From aa2594a52e9bddb6a21f7353a2c0965eec3b3415 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 20 Feb 2025 18:42:13 +0100 Subject: [PATCH 01/20] Make ticket_alpn field private An omission in 3.x. Signed-off-by: Gilles Peskine --- include/mbedtls/ssl.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index e0c0eae4e..902907856 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1295,8 +1295,8 @@ struct mbedtls_ssl_session { #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION && MBEDTLS_SSL_CLI_C */ #if defined(MBEDTLS_SSL_EARLY_DATA) && defined(MBEDTLS_SSL_ALPN) && defined(MBEDTLS_SSL_SRV_C) - char *ticket_alpn; /*!< ALPN negotiated in the session - during which the ticket was generated. */ + char *MBEDTLS_PRIVATE(ticket_alpn); /*!< ALPN negotiated in the session + during which the ticket was generated. */ #endif #if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_CLI_C) From 86a66edcd021556e13cc4b714ab4dbc159770482 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 12 Feb 2025 23:11:09 +0100 Subject: [PATCH 02/20] Fix Doxygen markup Pacify `clang -Wdocumentation`. Signed-off-by: Gilles Peskine --- programs/ssl/ssl_test_lib.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/programs/ssl/ssl_test_lib.h b/programs/ssl/ssl_test_lib.h index a8387d719..3bbddd76e 100644 --- a/programs/ssl/ssl_test_lib.h +++ b/programs/ssl/ssl_test_lib.h @@ -243,8 +243,8 @@ int key_opaque_set_alg_usage(const char *alg1, const char *alg2, * - free the provided PK context and re-initilize it as an opaque PK context * wrapping the PSA key imported in the above step. * - * \param[in/out] pk On input the non-opaque PK context which contains the - * key to be wrapped. On output the re-initialized PK + * \param[in,out] pk On input, the non-opaque PK context which contains the + * key to be wrapped. On output, the re-initialized PK * context which represents the opaque version of the one * provided as input. * \param[in] psa_alg The primary algorithm that will be associated to the From eb63613347312ae8976016ac94884e34c058926f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 13 Feb 2025 12:58:24 +0100 Subject: [PATCH 03/20] Make guards more consistent between X.509-has-certs and SSL-has-certs Fix some build errors when MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED is false but MBEDTLS_X509_CRT_PARSE_C is enabled. This is not a particularly useful configuration, but for quick testing, it's convenient for it to work. Signed-off-by: Gilles Peskine --- programs/ssl/ssl_test_common_source.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/programs/ssl/ssl_test_common_source.c b/programs/ssl/ssl_test_common_source.c index 6c7eed5e5..e194b58df 100644 --- a/programs/ssl/ssl_test_common_source.c +++ b/programs/ssl/ssl_test_common_source.c @@ -315,7 +315,7 @@ uint16_t ssl_sig_algs_for_test[] = { }; #endif /* MBEDTLS_X509_CRT_PARSE_C */ -#if defined(MBEDTLS_X509_CRT_PARSE_C) +#if defined(MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED) /** Functionally equivalent to mbedtls_x509_crt_verify_info, see that function * for more info. */ @@ -350,9 +350,7 @@ static int x509_crt_verify_info(char *buf, size_t size, const char *prefix, return (int) (size - n); #endif /* MBEDTLS_X509_REMOVE_INFO */ } -#endif /* MBEDTLS_X509_CRT_PARSE_C */ -#if defined(MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED) static void mbedtls_print_supported_sig_algs(void) { mbedtls_printf("supported signature algorithms:\n"); From 58b399e81ed3bff008672a76b227ffbf2c3a288f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 13 Feb 2025 21:23:22 +0100 Subject: [PATCH 04/20] Automate MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK dependency Signed-off-by: Gilles Peskine --- tests/ssl-opt.sh | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 23b692c72..ce661fcc8 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -475,6 +475,11 @@ detect_required_features() { requires_certificate_authentication;; esac + case " $CMD_LINE " in + *\ ca_callback=1\ *) + requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK;; + esac + case " $CMD_LINE " in *"programs/ssl/dtls_client "*|\ *"programs/ssl/ssl_client1 "*) @@ -2217,7 +2222,6 @@ run_test "TLS: password protected server key, two certificates" \ "$P_CLI" \ 0 -requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "CA callback on client" \ "$P_SRV debug_level=3" \ "$P_CLI ca_callback=1 debug_level=3 " \ @@ -2226,7 +2230,6 @@ run_test "CA callback on client" \ -S "error" \ -C "error" -requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK requires_config_enabled MBEDTLS_X509_CRT_PARSE_C requires_hash_alg SHA_256 run_test "CA callback on server" \ @@ -6279,7 +6282,6 @@ run_test "Authentication: send alt hs DN hints in CertificateRequest" \ # Tests for auth_mode, using CA callback, these are duplicated from the authentication tests # When updating these tests, modify the matching authentication tests accordingly -requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: server badcert, client required" \ "$P_SRV crt_file=$DATA_FILES_PATH/server5-badsign.crt \ key_file=$DATA_FILES_PATH/server5.key" \ @@ -6291,7 +6293,6 @@ run_test "Authentication, CA callback: server badcert, client required" \ -c "! mbedtls_ssl_handshake returned" \ -c "X509 - Certificate verification failed" -requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: server badcert, client optional" \ "$P_SRV crt_file=$DATA_FILES_PATH/server5-badsign.crt \ key_file=$DATA_FILES_PATH/server5.key" \ @@ -6303,7 +6304,6 @@ run_test "Authentication, CA callback: server badcert, client optional" \ -C "! mbedtls_ssl_handshake returned" \ -C "X509 - Certificate verification failed" -requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: server badcert, client none" \ "$P_SRV crt_file=$DATA_FILES_PATH/server5-badsign.crt \ key_file=$DATA_FILES_PATH/server5.key" \ @@ -6322,7 +6322,6 @@ run_test "Authentication, CA callback: server badcert, client none" \ # occasion (to be fixed). If that bug's fixed, the test needs to be altered to use a # different means to have the server ignoring the client's supported curve list. -requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: server ECDH p256v1, client required, p256v1 unsupported" \ "$P_SRV debug_level=1 key_file=$DATA_FILES_PATH/server5.key \ crt_file=$DATA_FILES_PATH/server5.ku-ka.crt" \ @@ -6333,7 +6332,6 @@ run_test "Authentication, CA callback: server ECDH p256v1, client required, p -c "! Certificate verification flags" \ -C "bad server certificate (ECDH curve)" # Expect failure at earlier verification stage -requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: server ECDH p256v1, client optional, p256v1 unsupported" \ "$P_SRV debug_level=1 key_file=$DATA_FILES_PATH/server5.key \ crt_file=$DATA_FILES_PATH/server5.ku-ka.crt" \ @@ -6344,7 +6342,6 @@ run_test "Authentication, CA callback: server ECDH p256v1, client optional, p -c "! Certificate verification flags"\ -c "bad server certificate (ECDH curve)" # Expect failure only at ECDH params check -requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK requires_any_configs_enabled $TLS1_2_KEY_EXCHANGES_WITH_CERT run_test "Authentication, CA callback: client SHA384, server required" \ "$P_SRV ca_callback=1 debug_level=3 auth_mode=required" \ @@ -6356,7 +6353,6 @@ run_test "Authentication, CA callback: client SHA384, server required" \ -c "Supported Signature Algorithm found: 04 " \ -c "Supported Signature Algorithm found: 05 " -requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK requires_any_configs_enabled $TLS1_2_KEY_EXCHANGES_WITH_CERT run_test "Authentication, CA callback: client SHA256, server required" \ "$P_SRV ca_callback=1 debug_level=3 auth_mode=required" \ @@ -6368,7 +6364,6 @@ run_test "Authentication, CA callback: client SHA256, server required" \ -c "Supported Signature Algorithm found: 04 " \ -c "Supported Signature Algorithm found: 05 " -requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: client badcert, server required" \ "$P_SRV ca_callback=1 debug_level=3 auth_mode=required" \ "$P_CLI debug_level=3 crt_file=$DATA_FILES_PATH/server5-badsign.crt \ @@ -6390,7 +6385,6 @@ run_test "Authentication, CA callback: client badcert, server required" \ # detect that its write end of the connection is closed and abort # before reading the alert message. -requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: client cert not trusted, server required" \ "$P_SRV ca_callback=1 debug_level=3 auth_mode=required" \ "$P_CLI debug_level=3 crt_file=$DATA_FILES_PATH/server5-selfsigned.crt \ @@ -6408,7 +6402,6 @@ run_test "Authentication, CA callback: client cert not trusted, server requir -s "! mbedtls_ssl_handshake returned" \ -s "X509 - Certificate verification failed" -requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: client badcert, server optional" \ "$P_SRV ca_callback=1 debug_level=3 auth_mode=optional" \ "$P_CLI debug_level=3 crt_file=$DATA_FILES_PATH/server5-badsign.crt \ @@ -6429,7 +6422,6 @@ run_test "Authentication, CA callback: client badcert, server optional" \ requires_config_value_equals "MBEDTLS_X509_MAX_INTERMEDIATE_CA" $MAX_IM_CA requires_full_size_output_buffer -requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: server max_int chain, client default" \ "$P_SRV crt_file=$DATA_FILES_PATH/dir-maxpath/c09.pem \ key_file=$DATA_FILES_PATH/dir-maxpath/09.key" \ @@ -6440,7 +6432,6 @@ run_test "Authentication, CA callback: server max_int chain, client default" requires_config_value_equals "MBEDTLS_X509_MAX_INTERMEDIATE_CA" $MAX_IM_CA requires_full_size_output_buffer -requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: server max_int+1 chain, client default" \ "$P_SRV crt_file=$DATA_FILES_PATH/dir-maxpath/c10.pem \ key_file=$DATA_FILES_PATH/dir-maxpath/10.key" \ @@ -6451,7 +6442,6 @@ run_test "Authentication, CA callback: server max_int+1 chain, client default requires_config_value_equals "MBEDTLS_X509_MAX_INTERMEDIATE_CA" $MAX_IM_CA requires_full_size_output_buffer -requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: server max_int+1 chain, client optional" \ "$P_SRV crt_file=$DATA_FILES_PATH/dir-maxpath/c10.pem \ key_file=$DATA_FILES_PATH/dir-maxpath/10.key" \ @@ -6463,7 +6453,6 @@ run_test "Authentication, CA callback: server max_int+1 chain, client optiona requires_config_value_equals "MBEDTLS_X509_MAX_INTERMEDIATE_CA" $MAX_IM_CA requires_full_size_output_buffer -requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: client max_int+1 chain, server optional" \ "$P_SRV ca_callback=1 debug_level=3 ca_file=$DATA_FILES_PATH/dir-maxpath/00.crt auth_mode=optional" \ "$P_CLI crt_file=$DATA_FILES_PATH/dir-maxpath/c10.pem \ @@ -6474,7 +6463,6 @@ run_test "Authentication, CA callback: client max_int+1 chain, server optiona requires_config_value_equals "MBEDTLS_X509_MAX_INTERMEDIATE_CA" $MAX_IM_CA requires_full_size_output_buffer -requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: client max_int+1 chain, server required" \ "$P_SRV ca_callback=1 debug_level=3 ca_file=$DATA_FILES_PATH/dir-maxpath/00.crt auth_mode=required" \ "$P_CLI crt_file=$DATA_FILES_PATH/dir-maxpath/c10.pem \ @@ -6485,7 +6473,6 @@ run_test "Authentication, CA callback: client max_int+1 chain, server require requires_config_value_equals "MBEDTLS_X509_MAX_INTERMEDIATE_CA" $MAX_IM_CA requires_full_size_output_buffer -requires_config_enabled MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK run_test "Authentication, CA callback: client max_int chain, server required" \ "$P_SRV ca_callback=1 debug_level=3 ca_file=$DATA_FILES_PATH/dir-maxpath/00.crt auth_mode=required" \ "$P_CLI crt_file=$DATA_FILES_PATH/dir-maxpath/c09.pem \ From 95fe2a6df4efca8680200f8a0110fbeae4a795cb Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 20 Feb 2025 18:12:29 +0100 Subject: [PATCH 05/20] Add a flags field to mbedtls_ssl_context Signed-off-by: Gilles Peskine --- include/mbedtls/ssl.h | 6 ++++++ library/ssl_tls.c | 1 + 2 files changed, 7 insertions(+) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 902907856..7c3a3d943 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1669,6 +1669,12 @@ struct mbedtls_ssl_context { * Miscellaneous */ int MBEDTLS_PRIVATE(state); /*!< SSL handshake: current state */ + + /** Mask of `MBEDTLS_SSL_CONTEXT_FLAG_XXX`. + * This field is not saved by mbedtls_ssl_session_save(). + */ + uint32_t MBEDTLS_PRIVATE(flags); + #if defined(MBEDTLS_SSL_RENEGOTIATION) int MBEDTLS_PRIVATE(renego_status); /*!< Initial, in progress, pending? */ int MBEDTLS_PRIVATE(renego_records_seen); /*!< Records since renego request, or with DTLS, diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 60f2e1cd6..4744db3d4 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1411,6 +1411,7 @@ int mbedtls_ssl_session_reset_int(mbedtls_ssl_context *ssl, int partial) int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; ssl->state = MBEDTLS_SSL_HELLO_REQUEST; + ssl->flags = 0; ssl->tls_version = ssl->conf->max_tls_version; mbedtls_ssl_session_reset_msg_layer(ssl, partial); From e5054e495aa69f4556147fc250d9204e597e4ed9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 12 Feb 2025 21:50:53 +0100 Subject: [PATCH 06/20] mbedtls_ssl_set_hostname tests: baseline Test the current behavior. Signed-off-by: Gilles Peskine --- programs/ssl/ssl_client2.c | 39 ++++++++- tests/ssl-opt.sh | 157 +++++++++++++++++++++++++++++++++++++ 2 files changed, 192 insertions(+), 4 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index f009a3169..fa61c6cb1 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -68,6 +68,7 @@ int main(void) #define DFL_MAX_VERSION -1 #define DFL_SHA1 -1 #define DFL_AUTH_MODE -1 +#define DFL_SET_HOSTNAME 1 #define DFL_MFL_CODE MBEDTLS_SSL_MAX_FRAG_LEN_NONE #define DFL_TRUNC_HMAC -1 #define DFL_RECSPLIT -1 @@ -403,6 +404,9 @@ int main(void) #define USAGE2 \ " auth_mode=%%s default: (library default: none)\n" \ " options: none, optional, required\n" \ + " set_hostname=%%s call mbedtls_ssl_set_hostname()?" \ + " options: no, server_name, NULL\n" \ + " default: server_name (but ignored if certs disabled)\n" \ USAGE_IO \ USAGE_KEY_OPAQUE \ USAGE_CA_CALLBACK \ @@ -505,6 +509,8 @@ struct options { int max_version; /* maximum protocol version accepted */ int allow_sha1; /* flag for SHA-1 support */ int auth_mode; /* verify mode for connection */ + int set_hostname; /* call mbedtls_ssl_set_hostname()? */ + /* 0=no, 1=yes, -1=NULL */ unsigned char mfl_code; /* code for maximum fragment length */ int trunc_hmac; /* negotiate truncated hmac or not */ int recsplit; /* enable record splitting? */ @@ -953,6 +959,7 @@ int main(int argc, char *argv[]) opt.max_version = DFL_MAX_VERSION; opt.allow_sha1 = DFL_SHA1; opt.auth_mode = DFL_AUTH_MODE; + opt.set_hostname = DFL_SET_HOSTNAME; opt.mfl_code = DFL_MFL_CODE; opt.trunc_hmac = DFL_TRUNC_HMAC; opt.recsplit = DFL_RECSPLIT; @@ -1344,6 +1351,16 @@ usage: } else { goto usage; } + } else if (strcmp(p, "set_hostname") == 0) { + if (strcmp(q, "no") == 0) { + opt.set_hostname = 0; + } else if (strcmp(q, "server_name") == 0) { + opt.set_hostname = 1; + } else if (strcmp(q, "NULL") == 0) { + opt.set_hostname = -1; + } else { + goto usage; + } } else if (strcmp(p, "max_frag_len") == 0) { if (strcmp(q, "512") == 0) { opt.mfl_code = MBEDTLS_SSL_MAX_FRAG_LEN_512; @@ -2052,10 +2069,24 @@ usage: #endif /* MBEDTLS_SSL_DTLS_SRTP */ #if defined(MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED) - if ((ret = mbedtls_ssl_set_hostname(&ssl, opt.server_name)) != 0) { - mbedtls_printf(" failed\n ! mbedtls_ssl_set_hostname returned %d\n\n", - ret); - goto exit; + switch (opt.set_hostname) { + case -1: + if ((ret = mbedtls_ssl_set_hostname(&ssl, NULL)) != 0) { + mbedtls_printf(" failed\n ! mbedtls_ssl_set_hostname returned %d\n\n", + ret); + goto exit; + } + break; + case 0: + /* Skip the call */ + break; + default: + if ((ret = mbedtls_ssl_set_hostname(&ssl, opt.server_name)) != 0) { + mbedtls_printf(" failed\n ! mbedtls_ssl_set_hostname returned %d\n\n", + ret); + goto exit; + } + break; } #endif diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index ce661fcc8..e541a8198 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5938,6 +5938,163 @@ run_test "Authentication: server goodcert, client none, no trusted CA (1.2)" -C "X509 - Certificate verification failed" \ -C "SSL - No CA Chain is set, but required to operate" +# The next few tests check what happens if the server has a valid certificate +# that does not match its name (impersonation). + +run_test "Authentication: hostname match, client required" \ + "$P_SRV" \ + "$P_CLI auth_mode=required server_name=localhost debug_level=1" \ + 0 \ + -C "does not match with the expected CN" \ + -C "x509_verify_cert() returned -" \ + -C "! mbedtls_ssl_handshake returned" \ + -C "X509 - Certificate verification failed" + +run_test "Authentication: hostname mismatch (wrong), client required" \ + "$P_SRV" \ + "$P_CLI auth_mode=required server_name=wrong-name debug_level=1" \ + 1 \ + -c "does not match with the expected CN" \ + -c "x509_verify_cert() returned -" \ + -c "! mbedtls_ssl_handshake returned" \ + -c "X509 - Certificate verification failed" + +run_test "Authentication: hostname mismatch (empty), client required" \ + "$P_SRV" \ + "$P_CLI auth_mode=required server_name= debug_level=1" \ + 1 \ + -c "does not match with the expected CN" \ + -c "x509_verify_cert() returned -" \ + -c "! mbedtls_ssl_handshake returned" \ + -c "X509 - Certificate verification failed" + +run_test "Authentication: hostname mismatch (truncated), client required" \ + "$P_SRV" \ + "$P_CLI auth_mode=required server_name=localhos debug_level=1" \ + 1 \ + -c "does not match with the expected CN" \ + -c "x509_verify_cert() returned -" \ + -c "! mbedtls_ssl_handshake returned" \ + -c "X509 - Certificate verification failed" + +run_test "Authentication: hostname mismatch (last char), client required" \ + "$P_SRV" \ + "$P_CLI auth_mode=required server_name=localhoss debug_level=1" \ + 1 \ + -c "does not match with the expected CN" \ + -c "x509_verify_cert() returned -" \ + -c "! mbedtls_ssl_handshake returned" \ + -c "X509 - Certificate verification failed" + +run_test "Authentication: hostname mismatch (trailing), client required" \ + "$P_SRV" \ + "$P_CLI auth_mode=required server_name=localhostt debug_level=1" \ + 1 \ + -c "does not match with the expected CN" \ + -c "x509_verify_cert() returned -" \ + -c "! mbedtls_ssl_handshake returned" \ + -c "X509 - Certificate verification failed" + +run_test "Authentication: hostname mismatch, client optional" \ + "$P_SRV" \ + "$P_CLI auth_mode=optional server_name=wrong-name debug_level=1" \ + 0 \ + -c "does not match with the expected CN" \ + -c "x509_verify_cert() returned -" \ + -C "X509 - Certificate verification failed" + +run_test "Authentication: hostname mismatch, client none" \ + "$P_SRV" \ + "$P_CLI auth_mode=none server_name=wrong-name debug_level=1" \ + 0 \ + -C "does not match with the expected CN" \ + -C "x509_verify_cert() returned -" \ + -C "X509 - Certificate verification failed" + +run_test "Authentication: hostname null, client required" \ + "$P_SRV" \ + "$P_CLI auth_mode=required set_hostname=NULL debug_level=1" \ + 0 \ + -C "does not match with the expected CN" \ + -C "x509_verify_cert() returned -" \ + -C "! mbedtls_ssl_handshake returned" \ + -C "X509 - Certificate verification failed" + +run_test "Authentication: hostname null, client optional" \ + "$P_SRV" \ + "$P_CLI auth_mode=optional set_hostname=NULL debug_level=1" \ + 0 \ + -C "does not match with the expected CN" \ + -C "x509_verify_cert() returned -" \ + -C "X509 - Certificate verification failed" + +run_test "Authentication: hostname null, client none" \ + "$P_SRV" \ + "$P_CLI auth_mode=none set_hostname=NULL debug_level=1" \ + 0 \ + -C "does not match with the expected CN" \ + -C "x509_verify_cert() returned -" \ + -C "X509 - Certificate verification failed" + +run_test "Authentication: hostname unset, client required" \ + "$P_SRV" \ + "$P_CLI auth_mode=required set_hostname=no debug_level=1" \ + 0 \ + -C "does not match with the expected CN" \ + -C "x509_verify_cert() returned -" \ + -C "! mbedtls_ssl_handshake returned" \ + -C "X509 - Certificate verification failed" + +run_test "Authentication: hostname unset, client optional" \ + "$P_SRV" \ + "$P_CLI auth_mode=optional set_hostname=no debug_level=1" \ + 0 \ + -C "does not match with the expected CN" \ + -C "x509_verify_cert() returned -" \ + -C "X509 - Certificate verification failed" + +run_test "Authentication: hostname unset, client none" \ + "$P_SRV" \ + "$P_CLI auth_mode=none set_hostname=no debug_level=1" \ + 0 \ + -C "does not match with the expected CN" \ + -C "x509_verify_cert() returned -" \ + -C "X509 - Certificate verification failed" + +run_test "Authentication: hostname unset, client default, server picks cert, 1.2" \ + "$P_SRV force_version=tls12 force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-CCM-8" \ + "$P_CLI psk=73776f726466697368 psk_identity=foo set_hostname=no debug_level=1" \ + 0 \ + -C "does not match with the expected CN" \ + -C "x509_verify_cert() returned -" \ + -C "X509 - Certificate verification failed" + +requires_config_enabled MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED +run_test "Authentication: hostname unset, client default, server picks cert, 1.3" \ + "$P_SRV force_version=tls13 tls13_kex_modes=ephemeral" \ + "$P_CLI psk=73776f726466697368 psk_identity=foo set_hostname=no debug_level=1" \ + 0 \ + -C "does not match with the expected CN" \ + -C "x509_verify_cert() returned -" \ + -C "X509 - Certificate verification failed" + +run_test "Authentication: hostname unset, client default, server picks PSK, 1.2" \ + "$P_SRV force_version=tls12 force_ciphersuite=TLS-PSK-WITH-AES-128-CCM-8 psk=73776f726466697368 psk_identity=foo" \ + "$P_CLI psk=73776f726466697368 psk_identity=foo set_hostname=no debug_level=1" \ + 0 \ + -C "does not match with the expected CN" \ + -C "x509_verify_cert() returned -" \ + -C "X509 - Certificate verification failed" + +requires_config_enabled MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_ENABLED +run_test "Authentication: hostname unset, client default, server picks PSK, 1.3" \ + "$P_SRV force_version=tls13 tls13_kex_modes=psk psk=73776f726466697368 psk_identity=foo" \ + "$P_CLI psk=73776f726466697368 psk_identity=foo set_hostname=no debug_level=1" \ + 0 \ + -C "does not match with the expected CN" \ + -C "x509_verify_cert() returned -" \ + -C "X509 - Certificate verification failed" + # The purpose of the next two tests is to test the client's behaviour when receiving a server # certificate with an unsupported elliptic curve. This should usually not happen because # the client informs the server about the supported curves - it does, though, in the From 4ac4008fa09e0be09a8bfabbb43c966bcc54119f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 20 Feb 2025 18:13:58 +0100 Subject: [PATCH 07/20] Access ssl->hostname through abstractions in certificate verification New abstractions to access ssl->hostname: mbedtls_ssl_has_set_hostname_been_called(), mbedtls_ssl_free_hostname(). Use these abstractions to access the hostname with the opportunity for extra checks in mbedtls_ssl_verify_certificate(). No behavior change except for a new log message. Signed-off-by: Gilles Peskine --- library/ssl_tls.c | 66 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 56 insertions(+), 10 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 4744db3d4..dd1beb98b 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2516,6 +2516,36 @@ void mbedtls_ssl_conf_groups(mbedtls_ssl_config *conf, } #if defined(MBEDTLS_X509_CRT_PARSE_C) + +#if defined(MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED) +/** Whether mbedtls_ssl_set_hostname() has been called. + * + * \param[in] ssl SSL context + * + * \return \c 1 if mbedtls_ssl_set_hostname() has been called on \p ssl + * (including `mbedtls_ssl_set_hostname(ssl, NULL)`), + * otherwise \c 0. + */ +static int mbedtls_ssl_has_set_hostname_been_called( + const mbedtls_ssl_context *ssl) +{ + /* We can't tell the difference between the case where + * mbedtls_ssl_set_hostname() has not been called at all, and + * the case where it was last called with NULL. For the time + * being, we assume the latter, i.e. we behave as if there had + * been an implicit call to mbedtls_ssl_set_hostname(ssl, NULL). */ + return ssl->hostname != NULL; +} +#endif + +static void mbedtls_ssl_free_hostname(mbedtls_ssl_context *ssl) +{ + if (ssl->hostname != NULL) { + mbedtls_zeroize_and_free(ssl->hostname, strlen(ssl->hostname)); + } + ssl->hostname = NULL; +} + int mbedtls_ssl_set_hostname(mbedtls_ssl_context *ssl, const char *hostname) { /* Initialize to suppress unnecessary compiler warning */ @@ -2533,10 +2563,7 @@ int mbedtls_ssl_set_hostname(mbedtls_ssl_context *ssl, const char *hostname) /* Now it's clear that we will overwrite the old hostname, * so we can free it safely */ - - if (ssl->hostname != NULL) { - mbedtls_zeroize_and_free(ssl->hostname, strlen(ssl->hostname)); - } + mbedtls_ssl_free_hostname(ssl); /* Passing NULL as hostname shall clear the old one */ @@ -5295,9 +5322,7 @@ void mbedtls_ssl_free(mbedtls_ssl_context *ssl) } #if defined(MBEDTLS_X509_CRT_PARSE_C) - if (ssl->hostname != NULL) { - mbedtls_zeroize_and_free(ssl->hostname, strlen(ssl->hostname)); - } + mbedtls_ssl_free_hostname(ssl); #endif #if defined(MBEDTLS_SSL_DTLS_HELLO_VERIFY) && defined(MBEDTLS_SSL_SRV_C) @@ -8845,6 +8870,21 @@ int mbedtls_ssl_check_cert_usage(const mbedtls_x509_crt *cert, return ret; } +static int get_hostname_for_verification(mbedtls_ssl_context *ssl, + const char **hostname) +{ + if (!mbedtls_ssl_has_set_hostname_been_called(ssl)) { + MBEDTLS_SSL_DEBUG_MSG(1, ("Certificate verification without having set hostname")); + } + + *hostname = ssl->hostname; + if (*hostname == NULL) { + MBEDTLS_SSL_DEBUG_MSG(2, ("Certificate verification without CN verification")); + } + + return 0; +} + int mbedtls_ssl_verify_certificate(mbedtls_ssl_context *ssl, int authmode, mbedtls_x509_crt *chain, @@ -8870,7 +8910,13 @@ int mbedtls_ssl_verify_certificate(mbedtls_ssl_context *ssl, p_vrfy = ssl->conf->p_vrfy; } - int ret = 0; + const char *hostname = ""; + int ret = get_hostname_for_verification(ssl, &hostname); + if (ret != 0) { + MBEDTLS_SSL_DEBUG_RET(1, "get_hostname_for_verification", ret); + return ret; + } + int have_ca_chain_or_callback = 0; #if defined(MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK) if (ssl->conf->f_ca_cb != NULL) { @@ -8883,7 +8929,7 @@ int mbedtls_ssl_verify_certificate(mbedtls_ssl_context *ssl, ssl->conf->f_ca_cb, ssl->conf->p_ca_cb, ssl->conf->cert_profile, - ssl->hostname, + hostname, &ssl->session_negotiate->verify_result, f_vrfy, p_vrfy); } else @@ -8910,7 +8956,7 @@ int mbedtls_ssl_verify_certificate(mbedtls_ssl_context *ssl, chain, ca_chain, ca_crl, ssl->conf->cert_profile, - ssl->hostname, + hostname, &ssl->session_negotiate->verify_result, f_vrfy, p_vrfy, rs_ctx); } From 434016e2eb6812be245277ceda39a56713be284c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 20 Feb 2025 18:49:59 +0100 Subject: [PATCH 08/20] Keep track of whether mbedtls_ssl_set_hostname() has been called No behavior change apart from now emitting a different log message depending on whether mbedtls_ssl_set_hostname() has been called with NULL or not at all. Signed-off-by: Gilles Peskine --- include/mbedtls/ssl.h | 2 ++ library/ssl_misc.h | 6 ++++++ library/ssl_tls.c | 9 +++----- tests/ssl-opt.sh | 50 ++++++++++++++++++++++++++++++++----------- 4 files changed, 48 insertions(+), 19 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 7c3a3d943..fa46fa745 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1671,6 +1671,8 @@ struct mbedtls_ssl_context { int MBEDTLS_PRIVATE(state); /*!< SSL handshake: current state */ /** Mask of `MBEDTLS_SSL_CONTEXT_FLAG_XXX`. + * See `mbedtls_ssl_context_flags_t` in ssl_misc.h. + * * This field is not saved by mbedtls_ssl_session_save(). */ uint32_t MBEDTLS_PRIVATE(flags); diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 9f91861f6..2d5417281 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -51,6 +51,12 @@ extern const mbedtls_error_pair_t psa_to_ssl_errors[7]; #define MBEDTLS_SSL_ECP_RESTARTABLE_ENABLED #endif +/** Flag values for mbedtls_ssl_context::flags. */ +typedef enum { + /** Set if mbedtls_ssl_set_hostname() has been called. */ + MBEDTLS_SSL_CONTEXT_FLAG_HOSTNAME_SET = 1, +} mbedtls_ssl_context_flags_t; + #define MBEDTLS_SSL_INITIAL_HANDSHAKE 0 #define MBEDTLS_SSL_RENEGOTIATION_IN_PROGRESS 1 /* In progress */ #define MBEDTLS_SSL_RENEGOTIATION_DONE 2 /* Done or aborted */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index dd1beb98b..998cac2ce 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2529,12 +2529,7 @@ void mbedtls_ssl_conf_groups(mbedtls_ssl_config *conf, static int mbedtls_ssl_has_set_hostname_been_called( const mbedtls_ssl_context *ssl) { - /* We can't tell the difference between the case where - * mbedtls_ssl_set_hostname() has not been called at all, and - * the case where it was last called with NULL. For the time - * being, we assume the latter, i.e. we behave as if there had - * been an implicit call to mbedtls_ssl_set_hostname(ssl, NULL). */ - return ssl->hostname != NULL; + return (ssl->flags & MBEDTLS_SSL_CONTEXT_FLAG_HOSTNAME_SET) != 0; } #endif @@ -2580,6 +2575,8 @@ int mbedtls_ssl_set_hostname(mbedtls_ssl_context *ssl, const char *hostname) ssl->hostname[hostname_len] = '\0'; } + ssl->flags |= MBEDTLS_SSL_CONTEXT_FLAG_HOSTNAME_SET; + return 0; } #endif /* MBEDTLS_X509_CRT_PARSE_C */ diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index e541a8198..ecff16ec8 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5943,9 +5943,11 @@ run_test "Authentication: server goodcert, client none, no trusted CA (1.2)" run_test "Authentication: hostname match, client required" \ "$P_SRV" \ - "$P_CLI auth_mode=required server_name=localhost debug_level=1" \ + "$P_CLI auth_mode=required server_name=localhost debug_level=2" \ 0 \ -C "does not match with the expected CN" \ + -C "Certificate verification without having set hostname" \ + -C "Certificate verification without CN verification" \ -C "x509_verify_cert() returned -" \ -C "! mbedtls_ssl_handshake returned" \ -C "X509 - Certificate verification failed" @@ -5997,7 +5999,7 @@ run_test "Authentication: hostname mismatch (trailing), client required" \ run_test "Authentication: hostname mismatch, client optional" \ "$P_SRV" \ - "$P_CLI auth_mode=optional server_name=wrong-name debug_level=1" \ + "$P_CLI auth_mode=optional server_name=wrong-name debug_level=2" \ 0 \ -c "does not match with the expected CN" \ -c "x509_verify_cert() returned -" \ @@ -6005,93 +6007,115 @@ run_test "Authentication: hostname mismatch, client optional" \ run_test "Authentication: hostname mismatch, client none" \ "$P_SRV" \ - "$P_CLI auth_mode=none server_name=wrong-name debug_level=1" \ + "$P_CLI auth_mode=none server_name=wrong-name debug_level=2" \ 0 \ -C "does not match with the expected CN" \ + -C "Certificate verification without having set hostname" \ + -C "Certificate verification without CN verification" \ -C "x509_verify_cert() returned -" \ -C "X509 - Certificate verification failed" run_test "Authentication: hostname null, client required" \ "$P_SRV" \ - "$P_CLI auth_mode=required set_hostname=NULL debug_level=1" \ + "$P_CLI auth_mode=required set_hostname=NULL debug_level=2" \ 0 \ -C "does not match with the expected CN" \ + -C "Certificate verification without having set hostname" \ + -c "Certificate verification without CN verification" \ -C "x509_verify_cert() returned -" \ -C "! mbedtls_ssl_handshake returned" \ -C "X509 - Certificate verification failed" run_test "Authentication: hostname null, client optional" \ "$P_SRV" \ - "$P_CLI auth_mode=optional set_hostname=NULL debug_level=1" \ + "$P_CLI auth_mode=optional set_hostname=NULL debug_level=2" \ 0 \ -C "does not match with the expected CN" \ + -C "Certificate verification without having set hostname" \ + -c "Certificate verification without CN verification" \ -C "x509_verify_cert() returned -" \ -C "X509 - Certificate verification failed" run_test "Authentication: hostname null, client none" \ "$P_SRV" \ - "$P_CLI auth_mode=none set_hostname=NULL debug_level=1" \ + "$P_CLI auth_mode=none set_hostname=NULL debug_level=2" \ 0 \ -C "does not match with the expected CN" \ + -C "Certificate verification without having set hostname" \ + -C "Certificate verification without CN verification" \ -C "x509_verify_cert() returned -" \ -C "X509 - Certificate verification failed" run_test "Authentication: hostname unset, client required" \ "$P_SRV" \ - "$P_CLI auth_mode=required set_hostname=no debug_level=1" \ + "$P_CLI auth_mode=required set_hostname=no debug_level=2" \ 0 \ -C "does not match with the expected CN" \ + -c "Certificate verification without having set hostname" \ + -c "Certificate verification without CN verification" \ -C "x509_verify_cert() returned -" \ -C "! mbedtls_ssl_handshake returned" \ -C "X509 - Certificate verification failed" run_test "Authentication: hostname unset, client optional" \ "$P_SRV" \ - "$P_CLI auth_mode=optional set_hostname=no debug_level=1" \ + "$P_CLI auth_mode=optional set_hostname=no debug_level=2" \ 0 \ -C "does not match with the expected CN" \ + -c "Certificate verification without having set hostname" \ + -c "Certificate verification without CN verification" \ -C "x509_verify_cert() returned -" \ -C "X509 - Certificate verification failed" run_test "Authentication: hostname unset, client none" \ "$P_SRV" \ - "$P_CLI auth_mode=none set_hostname=no debug_level=1" \ + "$P_CLI auth_mode=none set_hostname=no debug_level=2" \ 0 \ -C "does not match with the expected CN" \ + -C "Certificate verification without having set hostname" \ + -C "Certificate verification without CN verification" \ -C "x509_verify_cert() returned -" \ -C "X509 - Certificate verification failed" run_test "Authentication: hostname unset, client default, server picks cert, 1.2" \ "$P_SRV force_version=tls12 force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-CCM-8" \ - "$P_CLI psk=73776f726466697368 psk_identity=foo set_hostname=no debug_level=1" \ + "$P_CLI psk=73776f726466697368 psk_identity=foo set_hostname=no debug_level=2" \ 0 \ -C "does not match with the expected CN" \ + -c "Certificate verification without having set hostname" \ + -c "Certificate verification without CN verification" \ -C "x509_verify_cert() returned -" \ -C "X509 - Certificate verification failed" requires_config_enabled MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED run_test "Authentication: hostname unset, client default, server picks cert, 1.3" \ "$P_SRV force_version=tls13 tls13_kex_modes=ephemeral" \ - "$P_CLI psk=73776f726466697368 psk_identity=foo set_hostname=no debug_level=1" \ + "$P_CLI psk=73776f726466697368 psk_identity=foo set_hostname=no debug_level=2" \ 0 \ -C "does not match with the expected CN" \ + -c "Certificate verification without having set hostname" \ + -c "Certificate verification without CN verification" \ -C "x509_verify_cert() returned -" \ -C "X509 - Certificate verification failed" run_test "Authentication: hostname unset, client default, server picks PSK, 1.2" \ "$P_SRV force_version=tls12 force_ciphersuite=TLS-PSK-WITH-AES-128-CCM-8 psk=73776f726466697368 psk_identity=foo" \ - "$P_CLI psk=73776f726466697368 psk_identity=foo set_hostname=no debug_level=1" \ + "$P_CLI psk=73776f726466697368 psk_identity=foo set_hostname=no debug_level=2" \ 0 \ -C "does not match with the expected CN" \ + -C "Certificate verification without having set hostname" \ + -C "Certificate verification without CN verification" \ -C "x509_verify_cert() returned -" \ -C "X509 - Certificate verification failed" requires_config_enabled MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_PSK_ENABLED run_test "Authentication: hostname unset, client default, server picks PSK, 1.3" \ "$P_SRV force_version=tls13 tls13_kex_modes=psk psk=73776f726466697368 psk_identity=foo" \ - "$P_CLI psk=73776f726466697368 psk_identity=foo set_hostname=no debug_level=1" \ + "$P_CLI psk=73776f726466697368 psk_identity=foo set_hostname=no debug_level=2" \ 0 \ -C "does not match with the expected CN" \ + -C "Certificate verification without having set hostname" \ + -C "Certificate verification without CN verification" \ -C "x509_verify_cert() returned -" \ -C "X509 - Certificate verification failed" From 59a51170727c0a903c9a6dcbd4707b500d9cdaa3 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 13 Feb 2025 13:46:03 +0100 Subject: [PATCH 09/20] Create error code for mbedtls_ssl_set_hostname not called Signed-off-by: Gilles Peskine --- include/mbedtls/ssl.h | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index fa46fa745..0eaec5c8c 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -169,6 +169,39 @@ #define MBEDTLS_ERR_SSL_VERSION_MISMATCH -0x5F00 /** Invalid value in SSL config */ #define MBEDTLS_ERR_SSL_BAD_CONFIG -0x5E80 +/* Error space gap */ +/** Attempt to verify a certificate without an expected hostname. + * This is usually insecure. + * + * In TLS clients, when a client authenticates a server through its + * certificate, the client normally checks three things: + * - the certificate chain must be valid; + * - the chain must start from a trusted CA; + * - the certificate must cover the server name that is expected by the client. + * + * Omitting any of these checks is generally insecure, and can allow a + * malicious server to impersonate a legitimate server. + * + * The third check may be safely skipped in some unusual scenarios, + * such as networks where eavesdropping is a risk but not active attacks, + * or a private PKI where the client equally trusts all servers that are + * accredited by the root CA. + * + * You should call mbedtls_ssl_set_hostname() with the expected server name + * before starting a TLS handshake on a client (unless the client is + * set up to only use PSK-based authentication, which does not rely on the + * host name). If you have determined that server name verification is not + * required for security in your scenario, call mbedtls_ssl_set_hostname() + * with \p NULL as the server name. + * + * This error is raised if all of the following conditions are met: + * + * - A TLS client is configured with the authentication mode + * #MBEDTLS_SSL_VERIFY_REQUIRED (default). + * - Certificate authentication is enabled. + * - The client does not call mbedtls_ssl_set_hostname(). + */ +#define MBEDTLS_ERR_SSL_CERTIFICATE_VERIFICATION_WITHOUT_HOSTNAME -0x5D80 /* * Constants from RFC 8446 for TLS 1.3 PSK modes From 488b91929dc20d186913eb896202d634c769dbc4 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 13 Feb 2025 14:39:02 +0100 Subject: [PATCH 10/20] Require calling mbedtls_ssl_set_hostname() for security In a TLS client, when using certificate authentication, the client should check that the certificate is valid for the server name that the client expects. Otherwise, in most scenarios, a malicious server can impersonate another server. Normally, the application code should call mbedtls_ssl_set_hostname(). However, it's easy to forget. So raise an error if mandatory certificate authentication is in effect and mbedtls_ssl_set_hostname() has not been called. Raise the new error code MBEDTLS_ERR_SSL_CERTIFICATE_VERIFICATION_WITHOUT_HOSTNAME, for easy identification. But don't raise the error if the backward compatibility option MBEDTLS_SSL_CLI_ALLOW_WEAK_CERTIFICATE_VERIFICATION_WITHOUT_HOSTNAME is enabled. Signed-off-by: Gilles Peskine --- library/ssl_tls.c | 4 ++++ tests/ssl-opt.sh | 17 ++++++++++------- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 998cac2ce..6c401b59b 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -8872,6 +8872,10 @@ static int get_hostname_for_verification(mbedtls_ssl_context *ssl, { if (!mbedtls_ssl_has_set_hostname_been_called(ssl)) { MBEDTLS_SSL_DEBUG_MSG(1, ("Certificate verification without having set hostname")); + if (mbedtls_ssl_conf_get_endpoint(ssl->conf) == MBEDTLS_SSL_IS_CLIENT && + ssl->conf->authmode == MBEDTLS_SSL_VERIFY_REQUIRED) { + return MBEDTLS_ERR_SSL_CERTIFICATE_VERIFICATION_WITHOUT_HOSTNAME; + } } *hostname = ssl->hostname; diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index ecff16ec8..8d417afb1 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -6049,12 +6049,13 @@ run_test "Authentication: hostname null, client none" \ run_test "Authentication: hostname unset, client required" \ "$P_SRV" \ "$P_CLI auth_mode=required set_hostname=no debug_level=2" \ - 0 \ + 1 \ -C "does not match with the expected CN" \ -c "Certificate verification without having set hostname" \ - -c "Certificate verification without CN verification" \ + -C "Certificate verification without CN verification" \ + -c "get_hostname_for_verification() returned -" \ -C "x509_verify_cert() returned -" \ - -C "! mbedtls_ssl_handshake returned" \ + -c "! mbedtls_ssl_handshake returned" \ -C "X509 - Certificate verification failed" run_test "Authentication: hostname unset, client optional" \ @@ -6080,10 +6081,11 @@ run_test "Authentication: hostname unset, client none" \ run_test "Authentication: hostname unset, client default, server picks cert, 1.2" \ "$P_SRV force_version=tls12 force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-CCM-8" \ "$P_CLI psk=73776f726466697368 psk_identity=foo set_hostname=no debug_level=2" \ - 0 \ + 1 \ -C "does not match with the expected CN" \ -c "Certificate verification without having set hostname" \ - -c "Certificate verification without CN verification" \ + -C "Certificate verification without CN verification" \ + -c "get_hostname_for_verification() returned -" \ -C "x509_verify_cert() returned -" \ -C "X509 - Certificate verification failed" @@ -6091,10 +6093,11 @@ requires_config_enabled MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED run_test "Authentication: hostname unset, client default, server picks cert, 1.3" \ "$P_SRV force_version=tls13 tls13_kex_modes=ephemeral" \ "$P_CLI psk=73776f726466697368 psk_identity=foo set_hostname=no debug_level=2" \ - 0 \ + 1 \ -C "does not match with the expected CN" \ -c "Certificate verification without having set hostname" \ - -c "Certificate verification without CN verification" \ + -C "Certificate verification without CN verification" \ + -c "get_hostname_for_verification() returned -" \ -C "x509_verify_cert() returned -" \ -C "X509 - Certificate verification failed" From 856a3706286b313cd0f22b07b9233348d53c620f Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 13 Feb 2025 17:28:49 +0100 Subject: [PATCH 11/20] Call mbedtls_ssl_set_hostname in the generic endpoint setup in unit tests Signed-off-by: Gilles Peskine --- tests/src/test_helpers/ssl_helpers.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/src/test_helpers/ssl_helpers.c b/tests/src/test_helpers/ssl_helpers.c index 44e07efb6..b89ca215f 100644 --- a/tests/src/test_helpers/ssl_helpers.c +++ b/tests/src/test_helpers/ssl_helpers.c @@ -855,6 +855,10 @@ int mbedtls_test_ssl_endpoint_init( ret = mbedtls_ssl_setup(&(ep->ssl), &(ep->conf)); TEST_ASSERT(ret == 0); + if (MBEDTLS_SSL_IS_CLIENT == endpoint_type) { + ret = mbedtls_ssl_set_hostname(&(ep->ssl), "localhost"); + } + #if defined(MBEDTLS_SSL_PROTO_DTLS) && defined(MBEDTLS_SSL_SRV_C) if (endpoint_type == MBEDTLS_SSL_IS_SERVER && dtls_context != NULL) { mbedtls_ssl_conf_dtls_cookies(&(ep->conf), NULL, NULL, NULL); From 640512eb90a129187aa24ae4f7e742224d63ad2e Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 13 Feb 2025 21:46:00 +0100 Subject: [PATCH 12/20] mbedtls_ssl_set_hostname tests: add tests with CA callback Signed-off-by: Gilles Peskine --- tests/ssl-opt.sh | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 8d417afb1..8a44687c5 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5952,6 +5952,18 @@ run_test "Authentication: hostname match, client required" \ -C "! mbedtls_ssl_handshake returned" \ -C "X509 - Certificate verification failed" +run_test "Authentication: hostname match, client required, CA callback" \ + "$P_SRV" \ + "$P_CLI auth_mode=required server_name=localhost debug_level=3 ca_callback=1" \ + 0 \ + -C "does not match with the expected CN" \ + -C "Certificate verification without having set hostname" \ + -C "Certificate verification without CN verification" \ + -c "use CA callback for X.509 CRT verification" \ + -C "x509_verify_cert() returned -" \ + -C "! mbedtls_ssl_handshake returned" \ + -C "X509 - Certificate verification failed" + run_test "Authentication: hostname mismatch (wrong), client required" \ "$P_SRV" \ "$P_CLI auth_mode=required server_name=wrong-name debug_level=1" \ @@ -6058,6 +6070,19 @@ run_test "Authentication: hostname unset, client required" \ -c "! mbedtls_ssl_handshake returned" \ -C "X509 - Certificate verification failed" +run_test "Authentication: hostname unset, client required, CA callback" \ + "$P_SRV" \ + "$P_CLI auth_mode=required set_hostname=no debug_level=3 ca_callback=1" \ + 1 \ + -C "does not match with the expected CN" \ + -c "Certificate verification without having set hostname" \ + -C "Certificate verification without CN verification" \ + -c "get_hostname_for_verification() returned -" \ + -C "use CA callback for X.509 CRT verification" \ + -C "x509_verify_cert() returned -" \ + -c "! mbedtls_ssl_handshake returned" \ + -C "X509 - Certificate verification failed" + run_test "Authentication: hostname unset, client optional" \ "$P_SRV" \ "$P_CLI auth_mode=optional set_hostname=no debug_level=2" \ From 825c3d075a7ac6e11505dfc4a59140282884e1a0 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 17 Feb 2025 17:41:54 +0100 Subject: [PATCH 13/20] Add a note about calling mbedtls_ssl_set_hostname to mbedtls_ssl_setup Signed-off-by: Gilles Peskine --- include/mbedtls/ssl.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 0eaec5c8c..b15bbb666 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2015,6 +2015,17 @@ void mbedtls_ssl_init(mbedtls_ssl_context *ssl); * \note The PSA crypto subsystem must have been initialized by * calling psa_crypto_init() before calling this function. * + * \note After setting up a client context, if certificate-based + * authentication is enabled, you should call + * mbedtls_ssl_set_hostname() to specifiy the expected + * name of the server. Otherwise, if server authentication + * is required (which is the case by default) and the + * selected key exchange involves a certificate (i.e. is not + * based on a pre-shared key), the certificate authentication + * will fail. See + * #MBEDTLS_ERR_SSL_CERTIFICATE_VERIFICATION_WITHOUT_HOSTNAME + * for more information. + * * \param ssl SSL context * \param conf SSL configuration to use * From 02e303ec8669d6691404b09a48bd0e6e0c4fad80 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 17 Feb 2025 17:49:20 +0100 Subject: [PATCH 14/20] Changelog entries for requiring mbedls_ssl_set_hostname() in TLS clients Signed-off-by: Gilles Peskine --- ChangeLog.d/mbedtls_ssl_set_hostname.txt | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 ChangeLog.d/mbedtls_ssl_set_hostname.txt diff --git a/ChangeLog.d/mbedtls_ssl_set_hostname.txt b/ChangeLog.d/mbedtls_ssl_set_hostname.txt new file mode 100644 index 000000000..f5f0fa7e0 --- /dev/null +++ b/ChangeLog.d/mbedtls_ssl_set_hostname.txt @@ -0,0 +1,15 @@ +Default behavior changes + * In TLS clients, if mbedtls_ssl_set_hostname() has not been called, + mbedtls_ssl_handshake() now fails with + MBEDTLS_ERR_SSL_CERTIFICATE_VERIFICATION_WITHOUT_HOSTNAME + if certificate-based authentication of the server is attempted. + This is because authenticating a server without knowing what name + to expect is usually insecure. + +Security + * Note that TLS clients should generally call mbedtls_ssl_set_hostname() + if they use certificate authentication (i.e. not pre-shared keys). + Otherwise, in many scenarios, the server could be impersonated. + The library will now prevent the handshake and return + MBEDTLS_ERR_SSL_CERTIFICATE_VERIFICATION_WITHOUT_HOSTNAME + if mbedtls_ssl_set_hostname() has not been called. From 96073fb997dd6d7ef978f30bb390738691577f69 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 20 Feb 2025 19:12:04 +0100 Subject: [PATCH 15/20] Improve documentation of mbedtls_ssl_set_hostname Signed-off-by: Gilles Peskine --- include/mbedtls/ssl.h | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index b15bbb666..0fe2399d3 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -3937,16 +3937,19 @@ void mbedtls_ssl_conf_sig_algs(mbedtls_ssl_config *conf, #if defined(MBEDTLS_X509_CRT_PARSE_C) /** * \brief Set or reset the hostname to check against the received - * server certificate. It sets the ServerName TLS extension, - * too, if that extension is enabled. (client-side only) + * peer certificate. On a client, this also sets the + * ServerName TLS extension, if that extension is enabled. + * On a TLS 1.3 client, this also sets the server name in + * the session resumption ticket, if that feature is enabled. * * \param ssl SSL context - * \param hostname the server hostname, may be NULL to clear hostname - - * \note Maximum hostname length MBEDTLS_SSL_MAX_HOST_NAME_LEN. + * \param hostname The server hostname. This may be \c NULL to clear + * the hostname. * - * \return 0 if successful, MBEDTLS_ERR_SSL_ALLOC_FAILED on - * allocation failure, MBEDTLS_ERR_SSL_BAD_INPUT_DATA on + * \note Maximum hostname length #MBEDTLS_SSL_MAX_HOST_NAME_LEN. + * + * \return 0 if successful, #MBEDTLS_ERR_SSL_ALLOC_FAILED on + * allocation failure, #MBEDTLS_ERR_SSL_BAD_INPUT_DATA on * too long input hostname. * * Hostname set to the one provided on success (cleared From eb2d29eb6bdce5b90e31ce2a8a4eb1826ee5d8b7 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 20 Feb 2025 19:12:16 +0100 Subject: [PATCH 16/20] Document the need to call mbedtls_ssl_set_hostname Signed-off-by: Gilles Peskine --- include/mbedtls/ssl.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 0fe2399d3..31540249d 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -3948,6 +3948,16 @@ void mbedtls_ssl_conf_sig_algs(mbedtls_ssl_config *conf, * * \note Maximum hostname length #MBEDTLS_SSL_MAX_HOST_NAME_LEN. * + * \note If the hostname is \c NULL on a client, then the server + * is not authenticated: it only needs to have a valid + * certificate, not a certificate matching its name. + * Therefore you should always call this function on a client, + * unless the connection is set up to only allow + * pre-shared keys, or in scenarios where server + * impersonation is not a concern. See the documentation of + * #MBEDTLS_ERR_SSL_CERTIFICATE_VERIFICATION_WITHOUT_HOSTNAME + * for more details. + * * \return 0 if successful, #MBEDTLS_ERR_SSL_ALLOC_FAILED on * allocation failure, #MBEDTLS_ERR_SSL_BAD_INPUT_DATA on * too long input hostname. From fd89acc7357c53b432141bb2d341ca104f77bc85 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 24 Feb 2025 18:45:49 +0100 Subject: [PATCH 17/20] ssl_session_reset: preserve HOSTNAME_SET flag When we don't reset `ssl->hostname`, we must not reset the `MBEDTLS_SSL_CONTEXT_FLAG_HOSTNAME_SET` flag either. Signed-off-by: Gilles Peskine --- library/ssl_misc.h | 10 ++++++++++ library/ssl_tls.c | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 2d5417281..fd01aacac 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -57,6 +57,16 @@ typedef enum { MBEDTLS_SSL_CONTEXT_FLAG_HOSTNAME_SET = 1, } mbedtls_ssl_context_flags_t; +/** Flags from ::mbedtls_ssl_context_flags_t to keep in + * mbedtls_ssl_session_reset(). + * + * The flags that are in this list are kept until explicitly updated or + * until mbedtls_ssl_free(). The flags that are not listed here are + * reset to 0 in mbedtls_ssl_session_reset(). + */ +#define MBEDTLS_SSL_CONTEXT_FLAGS_KEEP_AT_SESSION \ + (MBEDTLS_SSL_CONTEXT_FLAG_HOSTNAME_SET) + #define MBEDTLS_SSL_INITIAL_HANDSHAKE 0 #define MBEDTLS_SSL_RENEGOTIATION_IN_PROGRESS 1 /* In progress */ #define MBEDTLS_SSL_RENEGOTIATION_DONE 2 /* Done or aborted */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 6c401b59b..0b072e6a7 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1411,7 +1411,7 @@ int mbedtls_ssl_session_reset_int(mbedtls_ssl_context *ssl, int partial) int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; ssl->state = MBEDTLS_SSL_HELLO_REQUEST; - ssl->flags = 0; + ssl->flags &= MBEDTLS_SSL_CONTEXT_FLAGS_KEEP_AT_SESSION; ssl->tls_version = ssl->conf->max_tls_version; mbedtls_ssl_session_reset_msg_layer(ssl, partial); From 816b7126806f2faf63eb0b3b8207d5c6b071f10c Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Fri, 7 Mar 2025 17:20:59 +0000 Subject: [PATCH 18/20] TLS1.2: Check for failures in Finished calculation If the calc_finished function returns an error code, don't ignore it but instead return the error code to stop the handshake as the Finished message may be incorrect. Signed-off-by: David Horstmann --- library/ssl_tls.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 0b072e6a7..b740358c1 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7570,6 +7570,7 @@ int mbedtls_ssl_write_finished(mbedtls_ssl_context *ssl) ret = ssl->handshake->calc_finished(ssl, ssl->out_msg + 4, ssl->conf->endpoint); if (ret != 0) { MBEDTLS_SSL_DEBUG_RET(1, "calc_finished", ret); + return ret; } /* @@ -7683,6 +7684,7 @@ int mbedtls_ssl_parse_finished(mbedtls_ssl_context *ssl) ret = ssl->handshake->calc_finished(ssl, buf, ssl->conf->endpoint ^ 1); if (ret != 0) { MBEDTLS_SSL_DEBUG_RET(1, "calc_finished", ret); + return ret; } if ((ret = mbedtls_ssl_read_record(ssl, 1)) != 0) { From 5ea94e6cd1ab8a89ee75284144412e2495607cf7 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Tue, 11 Mar 2025 15:52:48 +0000 Subject: [PATCH 19/20] Add changelog entry for TLS 1.2 Finished fix Signed-off-by: David Horstmann --- ChangeLog.d/tls12-check-finished-calc.txt | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 ChangeLog.d/tls12-check-finished-calc.txt diff --git a/ChangeLog.d/tls12-check-finished-calc.txt b/ChangeLog.d/tls12-check-finished-calc.txt new file mode 100644 index 000000000..cd52d32ff --- /dev/null +++ b/ChangeLog.d/tls12-check-finished-calc.txt @@ -0,0 +1,6 @@ +Security + * Fix a vulnerability in the TLS 1.2 handshake. If memory allocation failed + or there was a cryptographic hardware failure when calculating the + Finished message, it could be calculated incorrectly. This would break + the security guarantees of the TLS handshake. + CVE-2025-27810 From bc7cd93b5f5f685d8b313b7e7f177a32b05bcdcc Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Tue, 25 Mar 2025 14:10:10 +0000 Subject: [PATCH 20/20] Add missing credit for set_hostname issue Correctly credit Daniel Stenberg as the reporter of the mbedtls_ssl_set_hostname() issue. This was previously missed. Signed-off-by: David Horstmann --- ChangeLog.d/mbedtls_ssl_set_hostname.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/ChangeLog.d/mbedtls_ssl_set_hostname.txt b/ChangeLog.d/mbedtls_ssl_set_hostname.txt index f5f0fa7e0..250a5baaf 100644 --- a/ChangeLog.d/mbedtls_ssl_set_hostname.txt +++ b/ChangeLog.d/mbedtls_ssl_set_hostname.txt @@ -13,3 +13,4 @@ Security The library will now prevent the handshake and return MBEDTLS_ERR_SSL_CERTIFICATE_VERIFICATION_WITHOUT_HOSTNAME if mbedtls_ssl_set_hostname() has not been called. + Reported by Daniel Stenberg.