Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ssl] Migrate to mbedtls 3 #290

Merged
merged 8 commits into from
Jul 3, 2024
Merged

[ssl] Migrate to mbedtls 3 #290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
622e91c
[ssl] Add mbedtls 3 compatibility
tobil4sk Jul 2, 2024
f3c8dba
[ci] Use mbedtls3 for dynamic mac build
tobil4sk Jul 2, 2024
d2ffb9c
[cmake] Update to mbedtls 3.6 for static builds
tobil4sk Jul 2, 2024
804f3c2
[ssl] Initialize PSA crypto when it is present.
Apprentice-Alchemist May 1, 2024
47d72ff
[ci] Build on ubuntu bionic instead of xenial
tobil4sk Jul 3, 2024
4e58a82
[ssl] Fix mbedtls compilation errors on windows
tobil4sk Jul 3, 2024
69c13f5
[ssl] Link bcrypt on windows
tobil4sk Jul 3, 2024
9010629
[ssl] Define mbedtls config file when compiling ssl.c
tobil4sk Jul 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Earthfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ devcontainer-update-ref:
build-env:
# We specifically use an old distro to build against an old glibc.
# https://repology.org/project/glibc/versions
FROM ubuntu:xenial
FROM ubuntu:bionic
RUN apt-get update \
&& apt-get install -qqy --no-install-recommends \
software-properties-common \
Expand Down Expand Up @@ -216,7 +216,7 @@ extract-package:
SAVE ARTIFACT /tmp/neko neko

test-static-package:
ARG IMAGE=ubuntu:xenial
ARG IMAGE=ubuntu:bionic
FROM $IMAGE
WORKDIR /tmp/neko
COPY +extract-package/neko .
Expand All @@ -234,5 +234,5 @@ test-static-package:
RUN nekotools

test-static-package-all-platforms:
ARG IMAGE=ubuntu:xenial
ARG IMAGE=ubuntu:bionic
BUILD --platform=linux/amd64 --platform=linux/arm64 +test-static-package --IMAGE="$IMAGE"
55 changes: 0 additions & 55 deletions cmake/patch_mbedtls.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,60 +1,5 @@
# Apply config adjustments similer to Debian's
# https://anonscm.debian.org/cgit/collab-maint/mbedtls.git/tree/debian/patches/01_config.patch

set(config ${MbedTLS_source}/include/mbedtls/config.h)

file(READ ${config} content)

if (WIN32)
# allow alternate threading implementation
string(REPLACE
"//#define MBEDTLS_THREADING_ALT"
"#define MBEDTLS_THREADING_ALT"
content "${content}"
)
# disable the TCP/IP networking routines
# such that it wouldn't interfere with the #include <windows.h> in our threading_alt.h
string(REPLACE
"#define MBEDTLS_NET_C"
"//#define MBEDTLS_NET_C"
content "${content}"
)

file(COPY ${source}/libs/ssl/threading_alt.h
DESTINATION ${MbedTLS_source}/include/mbedtls/
)
else()
# enable pthread mutexes
string(REPLACE
"//#define MBEDTLS_THREADING_PTHREAD"
"#define MBEDTLS_THREADING_PTHREAD"
content "${content}"
)
endif()

# enable the HAVEGE random generator
string(REPLACE
"//#define MBEDTLS_HAVEGE_C"
"#define MBEDTLS_HAVEGE_C"
content "${content}"
)
# enable support for (rare) MD2-signed X.509 certs
string(REPLACE
"//#define MBEDTLS_MD2_C"
"#define MBEDTLS_MD2_C"
content "${content}"
)
# enable support for (rare) MD4-signed X.509 certs
string(REPLACE
"//#define MBEDTLS_MD4_C"
"#define MBEDTLS_MD4_C"
content "${content}"
)
# allow use of mutexes within mbed TLS
string(REPLACE
"//#define MBEDTLS_THREADING_C"
"#define MBEDTLS_THREADING_C"
content "${content}"
)

file(WRITE ${config} "${content}")
2 changes: 1 addition & 1 deletion extra/Brewfile-STATIC_DEPS_NONE
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ brew "ninja"
brew "pkg-config"
brew "bdw-gc"
brew "mariadb-connector-c"
brew "mbedtls@2", link: true
brew "mbedtls"
brew "pcre2"
12 changes: 9 additions & 3 deletions libs/ssl/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ if (STATIC_MBEDTLS)
-DENABLE_PROGRAMS=OFF
-DENABLE_TESTING=OFF
-DUSE_STATIC_MBEDTLS_LIBRARY=ON
-DMBEDTLS_USER_CONFIG_FILE=${CMAKE_CURRENT_SOURCE_DIR}/mbedtls_config.h
)
if (UNIX)
list(APPEND MBEDTLS_CMAKE_ARGS
Expand All @@ -25,7 +26,7 @@ if (STATIC_MBEDTLS)
${CMAKE_BINARY_DIR}/libs/src/MbedTLS-build/library/${CMAKE_CFG_INTDIR}/mbedtls.lib
${CMAKE_BINARY_DIR}/libs/src/MbedTLS-build/library/${CMAKE_CFG_INTDIR}/mbedcrypto.lib
)
target_link_libraries(ssl.ndll ws2_32 Advapi32 Crypt32)
target_link_libraries(ssl.ndll ws2_32 Advapi32 Crypt32 bcrypt)
else()
set(MBEDTLS_LIBRARIES
${CMAKE_BINARY_DIR}/libs/src/MbedTLS-build/library/libmbedx509.a
Expand All @@ -35,8 +36,8 @@ if (STATIC_MBEDTLS)
endif()
ExternalProject_Add(MbedTLS
${EP_CONFIGS}
URL https://github.com/Mbed-TLS/mbedtls/archive/refs/tags/v2.28.3.tar.gz
URL_HASH SHA256=bdf7c5bbdc338da3edad89b2885d4f8668f9a6fffeba6ec17a60333e36dade6f
URL https://github.com/Mbed-TLS/mbedtls/releases/download/v3.6.0/mbedtls-3.6.0.tar.bz2
URL_HASH SHA256=3ecf94fcfdaacafb757786a01b7538a61750ebd85c4b024f56ff8ba1490fcd38
CMAKE_ARGS ${MBEDTLS_CMAKE_ARGS}
PATCH_COMMAND ${CMAKE_COMMAND} -Dsource=${CMAKE_SOURCE_DIR} -DMbedTLS_source=${CMAKE_BINARY_DIR}/libs/src/MbedTLS -P ${CMAKE_SOURCE_DIR}/cmake/patch_mbedtls.cmake
INSTALL_COMMAND echo skip install
Expand All @@ -57,6 +58,11 @@ target_include_directories(ssl.ndll
${MBEDTLS_INCLUDE_DIR}
)

target_compile_definitions(ssl.ndll
PRIVATE
-DMBEDTLS_USER_CONFIG_FILE="${CMAKE_CURRENT_SOURCE_DIR}/mbedtls_config.h"
)

if(APPLE)
find_library(SECURITY_LIBRARY Security REQUIRED)
find_library(COREFOUNDATION_LIBRARY CoreFoundation REQUIRED)
Expand Down
10 changes: 10 additions & 0 deletions libs/ssl/mbedtls_config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#ifdef _WIN32
#define MBEDTLS_THREADING_ALT
#endif
#ifndef _WIN32
#define MBEDTLS_THREADING_PTHREAD
#endif

#undef MBEDTLS_NET_C

#define MBEDTLS_THREADING_C
32 changes: 31 additions & 1 deletion libs/ssl/ssl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ typedef int SOCKET;
#include "mbedtls/oid.h"
#include "mbedtls/x509_crt.h"
#include "mbedtls/ssl.h"
#include "mbedtls/net.h"

#ifdef MBEDTLS_PSA_CRYPTO_C
#include <psa/crypto.h>
#endif

#define val_ssl(o) (mbedtls_ssl_context*)val_data(o)
#define val_conf(o) (mbedtls_ssl_config*)val_data(o)
Expand Down Expand Up @@ -508,7 +511,11 @@ static value cert_get_altnames( value cert ){
value l = NULL, first = NULL;
val_check_kind(cert, k_cert);
crt = val_cert(cert);
#if MBEDTLS_VERSION_MAJOR >= 3
if( mbedtls_x509_crt_has_ext_type( crt, MBEDTLS_X509_EXT_SUBJECT_ALT_NAME ) ){
#else
if( crt->ext_types & MBEDTLS_X509_EXT_SUBJECT_ALT_NAME ){
#endif
cur = &crt->subject_alt_names;

while( cur != NULL ){
Expand Down Expand Up @@ -627,7 +634,11 @@ static value key_from_der( value data, value pub ){
if( val_bool(pub) )
r = mbedtls_pk_parse_public_key( pk, (const unsigned char*)val_string(data), val_strlen(data) );
else
#if MBEDTLS_VERSION_MAJOR >= 3
r = mbedtls_pk_parse_key( pk, (const unsigned char*)val_string(data), val_strlen(data), NULL, 0, mbedtls_ctr_drbg_random, &ctr_drbg );
#else
r = mbedtls_pk_parse_key( pk, (const unsigned char*)val_string(data), val_strlen(data), NULL, 0 );
#endif
if( r != 0 ){
mbedtls_pk_free(pk);
return ssl_error(r);
Expand All @@ -653,10 +664,17 @@ static value key_from_pem(value data, value pub, value pass){
mbedtls_pk_init(pk);
if( val_bool(pub) )
r = mbedtls_pk_parse_public_key( pk, buf, len );
#if MBEDTLS_VERSION_MAJOR >= 3
else if( val_is_null(pass) )
r = mbedtls_pk_parse_key( pk, buf, len, NULL, 0, mbedtls_ctr_drbg_random, &ctr_drbg );
else
r = mbedtls_pk_parse_key( pk, buf, len, (const unsigned char*)val_string(pass), val_strlen(pass), mbedtls_ctr_drbg_random, &ctr_drbg );
#else
else if( val_is_null(pass) )
r = mbedtls_pk_parse_key( pk, buf, len, NULL, 0 );
else
r = mbedtls_pk_parse_key( pk, buf, len, (const unsigned char*)val_string(pass), val_strlen(pass) );
#endif
if( r != 0 ){
mbedtls_pk_free(pk);
return ssl_error(r);
Expand Down Expand Up @@ -706,9 +724,17 @@ static value dgst_sign(value data, value key, value alg){
if( (r = mbedtls_md( md, (const unsigned char *)val_string(data), val_strlen(data), hash )) != 0 )
return ssl_error(r);

#if MBEDTLS_VERSION_MAJOR >= 3
out = alloc_empty_string(MBEDTLS_PK_SIGNATURE_MAX_SIZE);
#else
out = alloc_empty_string(MBEDTLS_MPI_MAX_SIZE);
#endif
buf = (unsigned char *)val_string(out);
#if MBEDTLS_VERSION_MAJOR >= 3
if( (r = mbedtls_pk_sign( val_pkey(key), mbedtls_md_get_type(md), hash, mbedtls_md_get_size(md), buf, MBEDTLS_PK_SIGNATURE_MAX_SIZE, &olen, mbedtls_ctr_drbg_random, &ctr_drbg )) != 0 )
#else
if( (r = mbedtls_pk_sign( val_pkey(key), mbedtls_md_get_type(md), hash, 0, buf, &olen, mbedtls_ctr_drbg_random, &ctr_drbg )) != 0 )
#endif
return ssl_error(r);

buf[olen] = 0;
Expand Down Expand Up @@ -785,6 +811,10 @@ void ssl_main() {
mbedtls_entropy_init( &entropy );
mbedtls_ctr_drbg_init( &ctr_drbg );
mbedtls_ctr_drbg_seed( &ctr_drbg, mbedtls_entropy_func, &entropy, NULL, 0 );

#ifdef MBEDTLS_PSA_CRYPTO_C
psa_crypto_init();
#endif
}

DEFINE_PRIM( ssl_new, 1 );
Expand Down
1 change: 1 addition & 0 deletions libs/ssl/threading_alt.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#define WIN32_LEAN_AND_MEAN
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, I thought this define was to exclude some things. How does this "fix" anything?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Later on after including threading_alt.h, mbedtls files go on to include other headers (presumably winsock2.h). But since the full windows.h has already been included, this results in re-definitions and compilation errors. By setting WIN32_LEAN_AND_MEAN, a lot of the unnecessary definitions are excluded from windows.h which avoids these conflicts later on.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I mean this is a good define to make regardless, I just find it strange that it's strictly necessary for compilation. But then again it probably won't matter. Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just about the order of includes. Including windows.h in threading_alt.h like we do results in this ordering, which breaks compilation:

#include <windows.h> // threading_alt.h
// ...
#include <winsock2.h> // mbedtls sources

Since at the time when windows.h is included we haven't included winsock2.h yet, windows.h includes winsock.h instead which contains conflicting definitions. Then when winsock2.h is included we get redefinition errors.

If WIN32_LEAN_AND_MEAN is defined, then winsock.h is excluded from windows.h so we don't end up with these redefinition errors when including winsock2.h.

More details here: https://stackoverflow.com/questions/21399650/cannot-include-both-files-winsock2-windows-h

#include <windows.h>

typedef struct
Expand Down