From 37e5999ac3568173b57edfaa415e77c16c2eec49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 10 Jun 2022 09:25:01 +0200 Subject: [PATCH 01/20] Fix potential buffer overread with USE_PSA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using opaque keys for static ECDH is not supported in this branch (will be introduced in 3.2). In case we reach that point, error out cleanly instead of miscasting a pointer. Since opaque keys were introduced, mbedtls_pk_can_do() was no longer a precise enough check. Signed-off-by: Manuel Pégourié-Gonnard --- ChangeLog.d/buf-overread-use-psa-static-ecdh.txt | 6 ++++++ library/ssl_srv.c | 7 +++++-- 2 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 ChangeLog.d/buf-overread-use-psa-static-ecdh.txt diff --git a/ChangeLog.d/buf-overread-use-psa-static-ecdh.txt b/ChangeLog.d/buf-overread-use-psa-static-ecdh.txt new file mode 100644 index 000000000..023c73082 --- /dev/null +++ b/ChangeLog.d/buf-overread-use-psa-static-ecdh.txt @@ -0,0 +1,6 @@ +Security + * Fix a potential heap buffer overread in TLS 1.2 server-side when + MBEDTLS_USE_PSA_CRYPTO is enabled, an opaque key (created with + mbedtls_pk_setup_opaque()) is provisioned, and a static ECDH ciphersuite + is selected. This may result in an application crash. No path to + information leak has been identified. diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 1733ec931..a912ce141 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3239,15 +3239,18 @@ static int ssl_write_certificate_request( mbedtls_ssl_context *ssl ) static int ssl_get_ecdh_params_from_cert( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + mbedtls_pk_context *own_key = mbedtls_ssl_own_key( ssl ); - if( ! mbedtls_pk_can_do( mbedtls_ssl_own_key( ssl ), MBEDTLS_PK_ECKEY ) ) + /* We want to call mbedtls_pk_ec(), which only works on those types. */ + if( mbedtls_pk_get_type( own_key ) != MBEDTLS_PK_ECKEY && + mbedtls_pk_get_type( own_key ) != MBEDTLS_PK_ECKEY_DH ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "server key not ECDH capable" ) ); return( MBEDTLS_ERR_SSL_PK_TYPE_MISMATCH ); } if( ( ret = mbedtls_ecdh_get_params( &ssl->handshake->ecdh_ctx, - mbedtls_pk_ec( *mbedtls_ssl_own_key( ssl ) ), + mbedtls_pk_ec( *own_key ), MBEDTLS_ECDH_OURS ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, ( "mbedtls_ecdh_get_params" ), ret ); From 5b3f24f214e1e9397611397a6cbc51252eafe775 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 10 Jun 2022 09:34:20 +0200 Subject: [PATCH 02/20] Fix unchecked return value from internal function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_srv.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index a912ce141..f2fca12b7 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3681,7 +3681,12 @@ static int ssl_write_server_key_exchange( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_KEY_EXCHANGE_SOME_ECDH_ENABLED) if( mbedtls_ssl_ciphersuite_uses_ecdh( ciphersuite_info ) ) { - ssl_get_ecdh_params_from_cert( ssl ); + ret = ssl_get_ecdh_params_from_cert( ssl ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_get_ecdh_params_from_cert", ret ); + return( ret ); + } } #endif /* MBEDTLS_KEY_EXCHANGE_SOME_ECDH_ENABLED */ From 853f06732e88843574d8cbe1318446a051c038ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 10 Jun 2022 09:40:58 +0200 Subject: [PATCH 03/20] Clarify warning about mbedtls_pk_ec/rsa() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous wording "ensure it holds an XXX" context did not mean anything without looking at the source. Looking at the source, the criterion is: - for mbedtls_pk_rsa(), that the info structure uses rsa_alloc_wrap; - for mbedtls_pk_ec(), that it uses eckey_alloc_wrap or ecdsa_alloc_wrap, since mbedtls_ecdsa_context is a typedef for mbedtls_ecp_keypair. (Note that our test code uses mbedtls_pk_ec() on contexts of type MBEDTLS_PK_ECDSA.) Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/pk.h | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index 8f2abf2a6..3851146d8 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -221,8 +221,9 @@ typedef void mbedtls_pk_restart_ctx; /** * Quick access to an RSA context inside a PK context. * - * \warning You must make sure the PK context actually holds an RSA context - * before using this function! + * \warning This function can only be used when the type of the context, as + * returned by mbedtls_pk_get_type(), is #MBEDTLS_PK_RSA. + * Ensuring that is the caller's responsibility. */ static inline mbedtls_rsa_context *mbedtls_pk_rsa( const mbedtls_pk_context pk ) { @@ -234,8 +235,10 @@ static inline mbedtls_rsa_context *mbedtls_pk_rsa( const mbedtls_pk_context pk ) /** * Quick access to an EC context inside a PK context. * - * \warning You must make sure the PK context actually holds an EC context - * before using this function! + * \warning This function can only be used when the type of the context, as + * returned by mbedtls_pk_get_type(), is #MBEDTLS_PK_ECKEY, + * #MBEDTLS_PK_ECKEY_DH, or #MBEDTLS_PK_ECDSA. + * Ensuring that is the caller's responsibility. */ static inline mbedtls_ecp_keypair *mbedtls_pk_ec( const mbedtls_pk_context pk ) { From a4a4aab54294e47f10f2e742c3eb13851473e7ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 10 Jun 2022 09:48:38 +0200 Subject: [PATCH 04/20] Improve contract of mbedtls_pk_ec/rsa() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Trusting the caller to perform the appropriate check is both risky, and a bit user-unfriendly. Returning NULL on error seems both safer (dereferencing a NULL pointer is more likely to result in a clean crash, while mis-casting a pointer might have deeper, less predictable consequences) and friendlier (the caller can just check the return value for NULL, which is a common idiom). Only add that as an additional way of using the function, for the sake of backwards compatibility. Calls where we know the type of the context for sure (for example because we just set it up) were legal and safe, so they should remain legal without checking the result for NULL, which would be redundant. Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/pk.h | 68 +++++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 29 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index 3851146d8..5a27b4276 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -217,35 +217,6 @@ typedef struct typedef void mbedtls_pk_restart_ctx; #endif /* MBEDTLS_ECDSA_C && MBEDTLS_ECP_RESTARTABLE */ -#if defined(MBEDTLS_RSA_C) -/** - * Quick access to an RSA context inside a PK context. - * - * \warning This function can only be used when the type of the context, as - * returned by mbedtls_pk_get_type(), is #MBEDTLS_PK_RSA. - * Ensuring that is the caller's responsibility. - */ -static inline mbedtls_rsa_context *mbedtls_pk_rsa( const mbedtls_pk_context pk ) -{ - return( (mbedtls_rsa_context *) (pk).pk_ctx ); -} -#endif /* MBEDTLS_RSA_C */ - -#if defined(MBEDTLS_ECP_C) -/** - * Quick access to an EC context inside a PK context. - * - * \warning This function can only be used when the type of the context, as - * returned by mbedtls_pk_get_type(), is #MBEDTLS_PK_ECKEY, - * #MBEDTLS_PK_ECKEY_DH, or #MBEDTLS_PK_ECDSA. - * Ensuring that is the caller's responsibility. - */ -static inline mbedtls_ecp_keypair *mbedtls_pk_ec( const mbedtls_pk_context pk ) -{ - return( (mbedtls_ecp_keypair *) (pk).pk_ctx ); -} -#endif /* MBEDTLS_ECP_C */ - #if defined(MBEDTLS_PK_RSA_ALT_SUPPORT) /** * \brief Types for RSA-alt abstraction @@ -659,6 +630,45 @@ const char * mbedtls_pk_get_name( const mbedtls_pk_context *ctx ); */ mbedtls_pk_type_t mbedtls_pk_get_type( const mbedtls_pk_context *ctx ); +#if defined(MBEDTLS_RSA_C) +/** + * Quick access to an RSA context inside a PK context. + * + * \warning This function can only be used when the type of the context, as + * returned by mbedtls_pk_get_type(), is #MBEDTLS_PK_RSA. + * Ensuring that is the caller's responsibility. + * Alternatively, you can check whether this function returns NULL. + * + * \return The internal RSA context held by the PK context, or NULL. + */ +static inline mbedtls_rsa_context *mbedtls_pk_rsa( const mbedtls_pk_context pk ) +{ + return( mbedtls_pk_get_type( &pk ) != MBEDTLS_PK_RSA ? NULL : + (mbedtls_rsa_context *) (pk).pk_ctx ); +} +#endif /* MBEDTLS_RSA_C */ + +#if defined(MBEDTLS_ECP_C) +/** + * Quick access to an EC context inside a PK context. + * + * \warning This function can only be used when the type of the context, as + * returned by mbedtls_pk_get_type(), is #MBEDTLS_PK_ECKEY, + * #MBEDTLS_PK_ECKEY_DH, or #MBEDTLS_PK_ECDSA. + * Ensuring that is the caller's responsibility. + * Alternatively, you can check whether this function returns NULL. + * + * \return The internal EC context held by the PK context, or NULL. + */ +static inline mbedtls_ecp_keypair *mbedtls_pk_ec( const mbedtls_pk_context pk ) +{ + return( mbedtls_pk_get_type( &pk ) != MBEDTLS_PK_ECKEY && + mbedtls_pk_get_type( &pk ) != MBEDTLS_PK_ECKEY_DH && + mbedtls_pk_get_type( &pk ) != MBEDTLS_PK_ECDSA ? NULL : + (mbedtls_ecp_keypair *) (pk).pk_ctx ); +} +#endif /* MBEDTLS_ECP_C */ + #if defined(MBEDTLS_PK_PARSE_C) /** \ingroup pk_module */ /** From b9c7ea459e518eac4cdf8c0d79977f2571b4dcd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 14 Jun 2022 09:25:17 +0200 Subject: [PATCH 05/20] Improve a comment. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ECDSA keys work with mbedtls_pk_ec() too, but we don't want to accept them here, so the comment should reflect that the check is not just about ensuring pk_ec() works. Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_srv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index f2fca12b7..705a63241 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -3241,7 +3241,8 @@ static int ssl_get_ecdh_params_from_cert( mbedtls_ssl_context *ssl ) int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; mbedtls_pk_context *own_key = mbedtls_ssl_own_key( ssl ); - /* We want to call mbedtls_pk_ec(), which only works on those types. */ + /* Check if the key is a transparent ECDH key. + * This also ensures that it is safe to call mbedtls_pk_ec(). */ if( mbedtls_pk_get_type( own_key ) != MBEDTLS_PK_ECKEY && mbedtls_pk_get_type( own_key ) != MBEDTLS_PK_ECKEY_DH ) { From ab09c9eb790ab78fcabbab56b263d94d1c8f5c4a Mon Sep 17 00:00:00 2001 From: Przemyslaw Stekiel Date: Mon, 4 Oct 2021 11:13:22 +0200 Subject: [PATCH 06/20] Add key_opaque option to ssl_server2.c + test Signed-off-by: Przemyslaw Stekiel --- programs/ssl/ssl_server2.c | 32 +++++++++++++++++++++++++++++++- tests/ssl-opt.sh | 15 +++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index cc1751d69..f0d021d3c 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -82,6 +82,7 @@ int main( void ) #define DFL_CA_PATH "" #define DFL_CRT_FILE "" #define DFL_KEY_FILE "" +#define DFL_KEY_OPAQUE 0 #define DFL_KEY_PWD "" #define DFL_CRT_FILE2 "" #define DFL_KEY_FILE2 "" @@ -199,6 +200,13 @@ int main( void ) #else #define USAGE_IO "" #endif /* MBEDTLS_X509_CRT_PARSE_C */ +#if defined(MBEDTLS_USE_PSA_CRYPTO) && defined(MBEDTLS_X509_CRT_PARSE_C) +#define USAGE_KEY_OPAQUE \ + " key_opaque=%%d Handle your private key as if it were opaque\n" \ + " default: 0 (disabled)\n" +#else +#define USAGE_KEY_OPAQUE "" +#endif #if defined(MBEDTLS_SSL_ASYNC_PRIVATE) #define USAGE_SSL_ASYNC \ @@ -481,6 +489,7 @@ int main( void ) " cert_req_ca_list=%%d default: 1 (send ca list)\n" \ " options: 1 (send ca list), 0 (don't send)\n" \ USAGE_IO \ + USAGE_KEY_OPAQUE \ "\n" \ USAGE_PSK \ USAGE_CA_CALLBACK \ @@ -559,6 +568,7 @@ struct options const char *ca_path; /* the path with the CA certificate(s) reside */ const char *crt_file; /* the file with the server certificate */ const char *key_file; /* the file with the server key */ + int key_opaque; /* handle private key as if it were opaque */ const char *key_pwd; /* the password for the server key */ const char *crt_file2; /* the file with the 2nd server certificate */ const char *key_file2; /* the file with the 2nd server key */ @@ -1310,6 +1320,9 @@ int main( int argc, char *argv[] ) mbedtls_pk_context pkey; mbedtls_x509_crt srvcert2; mbedtls_pk_context pkey2; +#if defined(MBEDTLS_USE_PSA_CRYPTO) + psa_key_id_t key_slot = 0; /* invalid key slot */ +#endif int key_cert_init = 0, key_cert_init2 = 0; #if defined(MBEDTLS_SSL_ASYNC_PRIVATE) ssl_async_key_context_t ssl_async_keys; @@ -1480,6 +1493,7 @@ int main( int argc, char *argv[] ) opt.ca_path = DFL_CA_PATH; opt.crt_file = DFL_CRT_FILE; opt.key_file = DFL_KEY_FILE; + opt.key_opaque = DFL_KEY_OPAQUE; opt.key_pwd = DFL_KEY_PWD; opt.crt_file2 = DFL_CRT_FILE2; opt.key_file2 = DFL_KEY_FILE2; @@ -1611,6 +1625,10 @@ int main( int argc, char *argv[] ) opt.key_file = q; else if( strcmp( p, "key_pwd" ) == 0 ) opt.key_pwd = q; +#if defined(MBEDTLS_USE_PSA_CRYPTO) && defined(MBEDTLS_X509_CRT_PARSE_C) + else if( strcmp( p, "key_opaque" ) == 0 ) + opt.key_opaque = atoi( q ); +#endif else if( strcmp( p, "crt_file2" ) == 0 ) opt.crt_file2 = q; else if( strcmp( p, "key_file2" ) == 0 ) @@ -2501,12 +2519,24 @@ int main( int argc, char *argv[] ) (unsigned int) -ret ); goto exit; } +#if defined(MBEDTLS_USE_PSA_CRYPTO) + if( opt.key_opaque != 0 ) + { + if( ( ret = mbedtls_pk_wrap_as_opaque( &pkey2, &key_slot, + PSA_ALG_SHA_256 ) ) != 0 ) + { + mbedtls_printf( " failed\n ! " + "mbedtls_pk_wrap_as_opaque returned -0x%x\n\n", (unsigned int) -ret ); + goto exit; + } + } +#endif /* MBEDTLS_USE_PSA_CRYPTO */ key_cert_init2 = 2; #endif /* MBEDTLS_ECDSA_C */ #endif /* MBEDTLS_CERTS_C */ } - mbedtls_printf( " ok\n" ); + mbedtls_printf( " ok (key type: %s)\n", mbedtls_pk_get_name( &pkey2 ) ); #endif /* MBEDTLS_X509_CRT_PARSE_C */ #if defined(MBEDTLS_DHM_C) && defined(MBEDTLS_FS_IO) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index fb9746f25..739cbd8c9 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -1632,6 +1632,21 @@ run_test "Opaque key for client authentication" \ -S "error" \ -C "error" +# Test using an opaque private key for server authentication +requires_config_enabled MBEDTLS_USE_PSA_CRYPTO +requires_config_enabled MBEDTLS_X509_CRT_PARSE_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SHA256_C +run_test "Opaque key for server authentication" \ + "$P_SRV auth_mode=required key_opaque=1" \ + "$P_CLI crt_file=data_files/server5.crt \ + key_file=data_files/server5.key" \ + 0 \ + -c "Verifying peer X.509 certificate... ok" \ + -s "key type: Opaque" \ + -S "error" \ + -C "error" + # Test ciphersuites which we expect to be fully supported by PSA Crypto # and check that we don't fall back to Mbed TLS' internal crypto primitives. run_test_psa TLS-ECDHE-ECDSA-WITH-AES-128-CCM From 5b6c4c9552aa8d2973d480ab12f5a73473466a7f Mon Sep 17 00:00:00 2001 From: Przemyslaw Stekiel Date: Wed, 6 Oct 2021 11:31:49 +0200 Subject: [PATCH 07/20] add client/server opaque test Signed-off-by: Przemyslaw Stekiel --- tests/ssl-opt.sh | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 739cbd8c9..ef7238c5f 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -1647,6 +1647,23 @@ run_test "Opaque key for server authentication" \ -S "error" \ -C "error" +# Test using an opaque private key for client/server authentication +requires_config_enabled MBEDTLS_USE_PSA_CRYPTO +requires_config_enabled MBEDTLS_X509_CRT_PARSE_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SHA256_C +run_test "Opaque key for client/server authentication" \ + "$P_SRV auth_mode=required key_opaque=1" \ + "$P_CLI key_opaque=1 crt_file=data_files/server5.crt \ + key_file=data_files/server5.key" \ + 0 \ + -c "key type: Opaque" \ + -c "Verifying peer X.509 certificate... ok" \ + -s "key type: Opaque" \ + -s "Verifying peer X.509 certificate... ok" \ + -S "error" \ + -C "error" + # Test ciphersuites which we expect to be fully supported by PSA Crypto # and check that we don't fall back to Mbed TLS' internal crypto primitives. run_test_psa TLS-ECDHE-ECDSA-WITH-AES-128-CCM From 69e567c0e135668547c2dee50f1ba7fba5fc44cf Mon Sep 17 00:00:00 2001 From: Przemyslaw Stekiel Date: Thu, 7 Oct 2021 15:11:32 +0200 Subject: [PATCH 08/20] ssl_server2.c: fix build err (key_slot - unused variable) Signed-off-by: Przemyslaw Stekiel --- programs/ssl/ssl_server2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index f0d021d3c..05cf08516 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1320,7 +1320,7 @@ int main( int argc, char *argv[] ) mbedtls_pk_context pkey; mbedtls_x509_crt srvcert2; mbedtls_pk_context pkey2; -#if defined(MBEDTLS_USE_PSA_CRYPTO) +#if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_USE_PSA_CRYPTO) psa_key_id_t key_slot = 0; /* invalid key slot */ #endif int key_cert_init = 0, key_cert_init2 = 0; From 331c3421d1f0c6427c6b5d8083b80ecac749c646 Mon Sep 17 00:00:00 2001 From: Przemyslaw Stekiel Date: Thu, 21 Oct 2021 12:26:58 +0200 Subject: [PATCH 09/20] Address review comments Signed-off-by: Przemyslaw Stekiel --- programs/ssl/ssl_server2.c | 42 ++++++++++++++++++++++++++++---------- tests/ssl-opt.sh | 4 ++-- 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 05cf08516..29c696f33 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -202,7 +202,7 @@ int main( void ) #endif /* MBEDTLS_X509_CRT_PARSE_C */ #if defined(MBEDTLS_USE_PSA_CRYPTO) && defined(MBEDTLS_X509_CRT_PARSE_C) #define USAGE_KEY_OPAQUE \ - " key_opaque=%%d Handle your private key as if it were opaque\n" \ + " key_opaque=%%d Handle your private keys as if they were opaque\n" \ " default: 0 (disabled)\n" #else #define USAGE_KEY_OPAQUE "" @@ -1320,8 +1320,9 @@ int main( int argc, char *argv[] ) mbedtls_pk_context pkey; mbedtls_x509_crt srvcert2; mbedtls_pk_context pkey2; -#if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_USE_PSA_CRYPTO) +#if defined(MBEDTLS_USE_PSA_CRYPTO) psa_key_id_t key_slot = 0; /* invalid key slot */ + psa_key_id_t key_slot2 = 0; /* invalid key slot */ #endif int key_cert_init = 0, key_cert_init2 = 0; #if defined(MBEDTLS_SSL_ASYNC_PRIVATE) @@ -2519,24 +2520,39 @@ int main( int argc, char *argv[] ) (unsigned int) -ret ); goto exit; } + key_cert_init2 = 2; +#endif /* MBEDTLS_ECDSA_C */ + } + #if defined(MBEDTLS_USE_PSA_CRYPTO) if( opt.key_opaque != 0 ) { - if( ( ret = mbedtls_pk_wrap_as_opaque( &pkey2, &key_slot, - PSA_ALG_SHA_256 ) ) != 0 ) + if ( mbedtls_pk_get_type( &pkey ) == MBEDTLS_PK_ECKEY ) { - mbedtls_printf( " failed\n ! " - "mbedtls_pk_wrap_as_opaque returned -0x%x\n\n", (unsigned int) -ret ); - goto exit; + if( ( ret = mbedtls_pk_wrap_as_opaque( &pkey, &key_slot, + PSA_ALG_SHA_256 ) ) != 0 ) + { + mbedtls_printf( " failed\n ! " + "mbedtls_pk_wrap_as_opaque returned -0x%x\n\n", (unsigned int) -ret ); + goto exit; + } + } + + if ( mbedtls_pk_get_type( &pkey2 ) == MBEDTLS_PK_ECKEY ) + { + if( ( ret = mbedtls_pk_wrap_as_opaque( &pkey2, &key_slot2, + PSA_ALG_SHA_256 ) ) != 0 ) + { + mbedtls_printf( " failed\n ! " + "mbedtls_pk_wrap_as_opaque returned -0x%x\n\n", (unsigned int) -ret ); + goto exit; + } } } #endif /* MBEDTLS_USE_PSA_CRYPTO */ - key_cert_init2 = 2; -#endif /* MBEDTLS_ECDSA_C */ #endif /* MBEDTLS_CERTS_C */ - } - mbedtls_printf( " ok (key type: %s)\n", mbedtls_pk_get_name( &pkey2 ) ); + mbedtls_printf( " ok (key types: %s - %s)\n", mbedtls_pk_get_name( &pkey ), mbedtls_pk_get_name( &pkey2 ) ); #endif /* MBEDTLS_X509_CRT_PARSE_C */ #if defined(MBEDTLS_DHM_C) && defined(MBEDTLS_FS_IO) @@ -4031,6 +4047,10 @@ exit: mbedtls_pk_free( &pkey ); mbedtls_x509_crt_free( &srvcert2 ); mbedtls_pk_free( &pkey2 ); +#if defined(MBEDTLS_USE_PSA_CRYPTO) + psa_destroy_key( key_slot ); + psa_destroy_key( key_slot2 ); +#endif #endif #if defined(MBEDTLS_DHM_C) && defined(MBEDTLS_FS_IO) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index ef7238c5f..187f7a0f1 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -1643,7 +1643,7 @@ run_test "Opaque key for server authentication" \ key_file=data_files/server5.key" \ 0 \ -c "Verifying peer X.509 certificate... ok" \ - -s "key type: Opaque" \ + -s "key types: RSA - Opaque" \ -S "error" \ -C "error" @@ -1659,7 +1659,7 @@ run_test "Opaque key for client/server authentication" \ 0 \ -c "key type: Opaque" \ -c "Verifying peer X.509 certificate... ok" \ - -s "key type: Opaque" \ + -s "key types: RSA - Opaque" \ -s "Verifying peer X.509 certificate... ok" \ -S "error" \ -C "error" From 67fc488515d978542127147f06fba9efd422d9d8 Mon Sep 17 00:00:00 2001 From: Przemyslaw Stekiel Date: Tue, 26 Oct 2021 12:21:45 +0200 Subject: [PATCH 10/20] ssl_client2/ssl_server_2: use PSA_ALG_ANY_HASH as algorithm for opaque key Signed-off-by: Przemyslaw Stekiel --- programs/ssl/ssl_client2.c | 2 +- programs/ssl/ssl_server2.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index e08bde6dd..2b8c89d11 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -1672,7 +1672,7 @@ int main( int argc, char *argv[] ) if( opt.key_opaque != 0 ) { if( ( ret = mbedtls_pk_wrap_as_opaque( &pkey, &key_slot, - PSA_ALG_SHA_256 ) ) != 0 ) + PSA_ALG_ANY_HASH ) ) != 0 ) { mbedtls_printf( " failed\n ! " "mbedtls_pk_wrap_as_opaque returned -0x%x\n\n", (unsigned int) -ret ); diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 29c696f33..d45c0ed85 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -2530,7 +2530,7 @@ int main( int argc, char *argv[] ) if ( mbedtls_pk_get_type( &pkey ) == MBEDTLS_PK_ECKEY ) { if( ( ret = mbedtls_pk_wrap_as_opaque( &pkey, &key_slot, - PSA_ALG_SHA_256 ) ) != 0 ) + PSA_ALG_ANY_HASH ) ) != 0 ) { mbedtls_printf( " failed\n ! " "mbedtls_pk_wrap_as_opaque returned -0x%x\n\n", (unsigned int) -ret ); @@ -2541,7 +2541,7 @@ int main( int argc, char *argv[] ) if ( mbedtls_pk_get_type( &pkey2 ) == MBEDTLS_PK_ECKEY ) { if( ( ret = mbedtls_pk_wrap_as_opaque( &pkey2, &key_slot2, - PSA_ALG_SHA_256 ) ) != 0 ) + PSA_ALG_ANY_HASH ) ) != 0 ) { mbedtls_printf( " failed\n ! " "mbedtls_pk_wrap_as_opaque returned -0x%x\n\n", (unsigned int) -ret ); From b3de3fd68c6904d1d6b56a4f05707a662d5f9ae6 Mon Sep 17 00:00:00 2001 From: Przemyslaw Stekiel Date: Tue, 26 Oct 2021 12:25:27 +0200 Subject: [PATCH 11/20] ssl-opt.sh: adapt paramteters of key opaque cases Signed-off-by: Przemyslaw Stekiel --- tests/ssl-opt.sh | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 187f7a0f1..bea887b3d 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -1623,12 +1623,15 @@ requires_config_enabled MBEDTLS_X509_CRT_PARSE_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SHA256_C run_test "Opaque key for client authentication" \ - "$P_SRV auth_mode=required" \ + "$P_SRV auth_mode=required crt_file=data_files/server5.crt \ + key_file=data_files/server5.key" \ "$P_CLI key_opaque=1 crt_file=data_files/server5.crt \ key_file=data_files/server5.key" \ 0 \ -c "key type: Opaque" \ + -c "Ciphersuite is TLS-ECDHE-ECDSA" \ -s "Verifying peer X.509 certificate... ok" \ + -s "Ciphersuite is TLS-ECDHE-ECDSA" \ -S "error" \ -C "error" @@ -1638,12 +1641,15 @@ requires_config_enabled MBEDTLS_X509_CRT_PARSE_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SHA256_C run_test "Opaque key for server authentication" \ - "$P_SRV auth_mode=required key_opaque=1" \ + "$P_SRV auth_mode=required key_opaque=1 crt_file=data_files/server5.crt \ + key_file=data_files/server5.key" \ "$P_CLI crt_file=data_files/server5.crt \ key_file=data_files/server5.key" \ 0 \ -c "Verifying peer X.509 certificate... ok" \ - -s "key types: RSA - Opaque" \ + -c "Ciphersuite is TLS-ECDHE-ECDSA" \ + -s "key types: Opaque - invalid PK" \ + -s "Ciphersuite is TLS-ECDHE-ECDSA" \ -S "error" \ -C "error" @@ -1653,14 +1659,17 @@ requires_config_enabled MBEDTLS_X509_CRT_PARSE_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SHA256_C run_test "Opaque key for client/server authentication" \ - "$P_SRV auth_mode=required key_opaque=1" \ + "$P_SRV auth_mode=required key_opaque=1 crt_file=data_files/server5.crt \ + key_file=data_files/server5.key" \ "$P_CLI key_opaque=1 crt_file=data_files/server5.crt \ key_file=data_files/server5.key" \ 0 \ -c "key type: Opaque" \ -c "Verifying peer X.509 certificate... ok" \ - -s "key types: RSA - Opaque" \ + -c "Ciphersuite is TLS-ECDHE-ECDSA" \ + -s "key types: Opaque - invalid PK" \ -s "Verifying peer X.509 certificate... ok" \ + -s "Ciphersuite is TLS-ECDHE-ECDSA" \ -S "error" \ -C "error" From 938be422c6319de136096657a364e3f87d700efd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 14 Jun 2022 10:43:36 +0200 Subject: [PATCH 12/20] Add negative test for Opaque key & static ECDH MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit That's actually the only non-PSK key exchange that needs to be negative-tested: all the other key exchanges are either positive-tested or use RSA, for which we can't even create opaque keys in this branch. Signed-off-by: Manuel Pégourié-Gonnard --- tests/ssl-opt.sh | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index bea887b3d..da929c73a 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -1673,6 +1673,32 @@ run_test "Opaque key for client/server authentication" \ -S "error" \ -C "error" +# Opaque keys not supported for static ECDH +requires_config_enabled MBEDTLS_USE_PSA_CRYPTO +requires_config_enabled MBEDTLS_X509_CRT_PARSE_C +run_test "Opaque key: server: ECDH-ECDSA not supported" \ + "$P_SRV debug_level=1 key_opaque=1 + crt_file=data_files/server5.crt key_file=data_files/server5.key" \ + "$P_CLI force_ciphersuite=TLS-ECDH-ECDSA-WITH-AES-128-GCM-SHA256" \ + 1 \ + -s "server key not ECDH capable" \ + -s "ssl_get_ecdh_params_from_cert() returned" \ + -s "error" \ + -c "error" + +# Opaque keys not supported for static ECDH +requires_config_enabled MBEDTLS_USE_PSA_CRYPTO +requires_config_enabled MBEDTLS_X509_CRT_PARSE_C +run_test "Opaque key: server: ECDH-RSA not supported" \ + "$P_SRV debug_level=1 key_opaque=1 + crt_file=data_files/server5.crt key_file=data_files/server5.key" \ + "$P_CLI force_ciphersuite=TLS-ECDH-RSA-WITH-AES-128-GCM-SHA256" \ + 1 \ + -s "server key not ECDH capable" \ + -s "ssl_get_ecdh_params_from_cert() returned" \ + -s "error" \ + -c "error" + # Test ciphersuites which we expect to be fully supported by PSA Crypto # and check that we don't fall back to Mbed TLS' internal crypto primitives. run_test_psa TLS-ECDHE-ECDSA-WITH-AES-128-CCM From a49a00cc24cda6068f28dc347aa7cbd7e8dfa5d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 14 Jun 2022 10:45:19 +0200 Subject: [PATCH 13/20] Add negative tests for opaque mixed-PSK (client) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ssl_client2.c used to check that we force a ciphersuite that worked; that would have prevented testing so I removed it. The library should be robust even when the application tries something that doesn't work. Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_cli.c | 9 +++++++++ programs/ssl/ssl_client2.c | 11 ----------- tests/ssl-opt.sh | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 11 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index e6e6ef444..eeb55c348 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -3928,7 +3928,10 @@ ecdh_calc_secret: #if defined(MBEDTLS_USE_PSA_CRYPTO) /* Opaque PSKs are currently only supported for PSK-only suites. */ if( ssl_conf_has_static_raw_psk( ssl->conf ) == 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "opaque PSK not supported with RSA-PSK" ) ); return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + } #endif /* MBEDTLS_USE_PSA_CRYPTO */ if( ( ret = ssl_write_encrypted_pms( ssl, header_len, @@ -3943,7 +3946,10 @@ ecdh_calc_secret: #if defined(MBEDTLS_USE_PSA_CRYPTO) /* Opaque PSKs are currently only supported for PSK-only suites. */ if( ssl_conf_has_static_raw_psk( ssl->conf ) == 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "opaque PSK not supported with DHE-PSK" ) ); return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + } #endif /* MBEDTLS_USE_PSA_CRYPTO */ /* @@ -3980,7 +3986,10 @@ ecdh_calc_secret: #if defined(MBEDTLS_USE_PSA_CRYPTO) /* Opaque PSKs are currently only supported for PSK-only suites. */ if( ssl_conf_has_static_raw_psk( ssl->conf ) == 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "opaque PSK not supported with ECDHE-PSK" ) ); return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + } #endif /* MBEDTLS_USE_PSA_CRYPTO */ /* diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 2b8c89d11..4f076602a 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -1420,17 +1420,6 @@ int main( int argc, char *argv[] ) #if defined (MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) if( opt.psk_opaque != 0 ) { - /* Ensure that the chosen ciphersuite is PSK-only; we must know - * the ciphersuite in advance to set the correct policy for the - * PSK key slot. This limitation might go away in the future. */ - if( ciphersuite_info->key_exchange != MBEDTLS_KEY_EXCHANGE_PSK || - opt.min_version != MBEDTLS_SSL_MINOR_VERSION_3 ) - { - mbedtls_printf( "opaque PSKs are only supported in conjunction with forcing TLS 1.2 and a PSK-only ciphersuite through the 'force_ciphersuite' option.\n" ); - ret = 2; - goto usage; - } - /* Determine KDF algorithm the opaque PSK will be used in. */ #if defined(MBEDTLS_SHA512_C) if( ciphersuite_info->mac == MBEDTLS_MD_SHA384 ) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index da929c73a..545da0915 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -1699,6 +1699,41 @@ run_test "Opaque key: server: ECDH-RSA not supported" \ -s "error" \ -c "error" +# Opaque PSKs not supported for mixed PSK + +requires_config_enabled MBEDTLS_USE_PSA_CRYPTO +run_test "Opaque psk: client: ECDHE-PSK not supported" \ + "$P_SRV debug_level=1 psk=abc123 psk_identity=foo" \ + "$P_CLI debug_level=1 psk=abc123 psk_identity=foo psk_opaque=1 \ + force_version=tls12 \ + force_ciphersuite=TLS-ECDHE-PSK-WITH-AES-128-CBC-SHA" \ + 1 \ + -c "opaque PSK not supported with ECDHE-PSK" \ + -s "error" \ + -c "error" + +requires_config_enabled MBEDTLS_USE_PSA_CRYPTO +run_test "Opaque psk: client: DHE-PSK not supported" \ + "$P_SRV debug_level=1 psk=abc123 psk_identity=foo" \ + "$P_CLI debug_level=1 psk=abc123 psk_identity=foo psk_opaque=1 \ + force_version=tls12 \ + force_ciphersuite=TLS-DHE-PSK-WITH-AES-128-CBC-SHA" \ + 1 \ + -c "opaque PSK not supported with DHE-PSK" \ + -s "error" \ + -c "error" + +requires_config_enabled MBEDTLS_USE_PSA_CRYPTO +run_test "Opaque psk: client: RSA-PSK not supported" \ + "$P_SRV debug_level=1 psk=abc123 psk_identity=foo" \ + "$P_CLI debug_level=1 psk=abc123 psk_identity=foo psk_opaque=1 \ + force_version=tls12 \ + force_ciphersuite=TLS-RSA-PSK-WITH-AES-128-CBC-SHA" \ + 1 \ + -c "opaque PSK not supported with RSA-PSK" \ + -s "error" \ + -c "error" + # Test ciphersuites which we expect to be fully supported by PSA Crypto # and check that we don't fall back to Mbed TLS' internal crypto primitives. run_test_psa TLS-ECDHE-ECDSA-WITH-AES-128-CCM From d80d8a40ee24867037293ecc38e14ba0ee1836bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 14 Jun 2022 10:53:15 +0200 Subject: [PATCH 14/20] Add negative tests for opaque mixed-PSK (server) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_srv.c | 9 +++++++++ programs/ssl/ssl_server2.c | 11 ----------- tests/ssl-opt.sh | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 11 deletions(-) diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 705a63241..64e78a9ff 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -4270,7 +4270,10 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_USE_PSA_CRYPTO) /* Opaque PSKs are currently only supported for PSK-only. */ if( ssl_use_opaque_psk( ssl ) == 1 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "opaque PSK not supported with RSA-PSK" ) ); return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + } #endif if( ( ret = ssl_parse_encrypted_pms( ssl, p, end, 2 ) ) != 0 ) @@ -4305,7 +4308,10 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_USE_PSA_CRYPTO) /* Opaque PSKs are currently only supported for PSK-only. */ if( ssl_use_opaque_psk( ssl ) == 1 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "opaque PSK not supported with DHE-PSK" ) ); return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + } #endif if( p != end ) @@ -4342,7 +4348,10 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_USE_PSA_CRYPTO) /* Opaque PSKs are currently only supported for PSK-only. */ if( ssl_use_opaque_psk( ssl ) == 1 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "opaque PSK not supported with ECDHE-PSK" ) ); return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + } #endif MBEDTLS_SSL_DEBUG_ECDH( 3, &ssl->handshake->ecdh_ctx, diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index d45c0ed85..6169a3774 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -2162,17 +2162,6 @@ int main( int argc, char *argv[] ) #if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) if( opt.psk_opaque != 0 || opt.psk_list_opaque != 0 ) { - /* Ensure that the chosen ciphersuite is PSK-only; we must know - * the ciphersuite in advance to set the correct policy for the - * PSK key slot. This limitation might go away in the future. */ - if( ciphersuite_info->key_exchange != MBEDTLS_KEY_EXCHANGE_PSK || - opt.min_version != MBEDTLS_SSL_MINOR_VERSION_3 ) - { - mbedtls_printf( "opaque PSKs are only supported in conjunction with forcing TLS 1.2 and a PSK-only ciphersuite through the 'force_ciphersuite' option.\n" ); - ret = 2; - goto usage; - } - /* Determine KDF algorithm the opaque PSK will be used in. */ #if defined(MBEDTLS_SHA512_C) if( ciphersuite_info->mac == MBEDTLS_MD_SHA384 ) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 545da0915..b1a328804 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -1734,6 +1734,39 @@ run_test "Opaque psk: client: RSA-PSK not supported" \ -s "error" \ -c "error" +requires_config_enabled MBEDTLS_USE_PSA_CRYPTO +run_test "Opaque psk: server: ECDHE-PSK not supported" \ + "$P_SRV debug_level=1 psk=abc123 psk_identity=foo psk_opaque=1 \ + force_version=tls12 \ + force_ciphersuite=TLS-ECDHE-PSK-WITH-AES-128-CBC-SHA" \ + "$P_CLI debug_level=1 psk=abc123 psk_identity=foo" \ + 1 \ + -s "opaque PSK not supported with ECDHE-PSK" \ + -s "error" \ + -c "error" + +requires_config_enabled MBEDTLS_USE_PSA_CRYPTO +run_test "Opaque psk: server: DHE-PSK not supported" \ + "$P_SRV debug_level=1 psk=abc123 psk_identity=foo psk_opaque=1 \ + force_version=tls12 \ + force_ciphersuite=TLS-DHE-PSK-WITH-AES-128-CBC-SHA" \ + "$P_CLI debug_level=1 psk=abc123 psk_identity=foo" \ + 1 \ + -s "opaque PSK not supported with DHE-PSK" \ + -s "error" \ + -c "error" + +requires_config_enabled MBEDTLS_USE_PSA_CRYPTO +run_test "Opaque psk: server: RSA-PSK not supported" \ + "$P_SRV debug_level=1 psk=abc123 psk_identity=foo psk_opaque=1 \ + force_version=tls12 \ + force_ciphersuite=TLS-RSA-PSK-WITH-AES-128-CBC-SHA" \ + "$P_CLI debug_level=1 psk=abc123 psk_identity=foo" \ + 1 \ + -s "opaque PSK not supported with RSA-PSK" \ + -s "error" \ + -c "error" + # Test ciphersuites which we expect to be fully supported by PSA Crypto # and check that we don't fall back to Mbed TLS' internal crypto primitives. run_test_psa TLS-ECDHE-ECDSA-WITH-AES-128-CCM From 8641102bc16a8193d93721f222729e5a07252fc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 16 Jun 2022 09:50:04 +0200 Subject: [PATCH 15/20] Fix impact evaluation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- ChangeLog.d/buf-overread-use-psa-static-ecdh.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ChangeLog.d/buf-overread-use-psa-static-ecdh.txt b/ChangeLog.d/buf-overread-use-psa-static-ecdh.txt index 023c73082..84b9f790d 100644 --- a/ChangeLog.d/buf-overread-use-psa-static-ecdh.txt +++ b/ChangeLog.d/buf-overread-use-psa-static-ecdh.txt @@ -2,5 +2,5 @@ Security * Fix a potential heap buffer overread in TLS 1.2 server-side when MBEDTLS_USE_PSA_CRYPTO is enabled, an opaque key (created with mbedtls_pk_setup_opaque()) is provisioned, and a static ECDH ciphersuite - is selected. This may result in an application crash. No path to - information leak has been identified. + is selected. This may result in an application crash or potentially an + information leak. From 06e1fcdb45872a410165a9b4ab4c16bf41dced6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 16 Jun 2022 10:48:06 +0200 Subject: [PATCH 16/20] Add comments when can_do() is safe to use MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_cli.c | 2 ++ library/ssl_tls.c | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index eeb55c348..1140d9e3e 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -3000,6 +3000,8 @@ static int ssl_get_ecdh_params_from_cert( mbedtls_ssl_context *ssl ) peer_pk = &ssl->session_negotiate->peer_cert->pk; #endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ + /* This is a public key, so it can't be opaque, so can_do() is a good + * enough check to ensure pk_ec() is safe to use below. */ if( ! mbedtls_pk_can_do( peer_pk, MBEDTLS_PK_ECKEY ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "server key not ECDH capable" ) ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index b0379cdce..0099b4396 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2697,7 +2697,9 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, { const mbedtls_pk_context *pk = &chain->pk; - /* If certificate uses an EC key, make sure the curve is OK */ + /* If certificate uses an EC key, make sure the curve is OK. + * This is a public key, so it can't be opaque, so can_do() is a good + * enough check to ensure pk_ec() is safe to use here. */ if( mbedtls_pk_can_do( pk, MBEDTLS_PK_ECKEY ) && mbedtls_ssl_check_curve( ssl, mbedtls_pk_ec( *pk )->grp.id ) != 0 ) { From 08b2ebd2bef0c4903219bffeec9510afe8ca42cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 17 Jun 2022 10:11:15 +0200 Subject: [PATCH 17/20] Improve readability with less negation. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Err, I mean don't worsen readability by not using more negation. Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/pk.h | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index 5a27b4276..017557a83 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -643,8 +643,9 @@ mbedtls_pk_type_t mbedtls_pk_get_type( const mbedtls_pk_context *ctx ); */ static inline mbedtls_rsa_context *mbedtls_pk_rsa( const mbedtls_pk_context pk ) { - return( mbedtls_pk_get_type( &pk ) != MBEDTLS_PK_RSA ? NULL : - (mbedtls_rsa_context *) (pk).pk_ctx ); + return( mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_RSA ? + (mbedtls_rsa_context *) (pk).pk_ctx : + NULL ); } #endif /* MBEDTLS_RSA_C */ @@ -662,10 +663,11 @@ static inline mbedtls_rsa_context *mbedtls_pk_rsa( const mbedtls_pk_context pk ) */ static inline mbedtls_ecp_keypair *mbedtls_pk_ec( const mbedtls_pk_context pk ) { - return( mbedtls_pk_get_type( &pk ) != MBEDTLS_PK_ECKEY && - mbedtls_pk_get_type( &pk ) != MBEDTLS_PK_ECKEY_DH && - mbedtls_pk_get_type( &pk ) != MBEDTLS_PK_ECDSA ? NULL : - (mbedtls_ecp_keypair *) (pk).pk_ctx ); + return( mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_ECKEY || + mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_ECKEY_DH || + mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_ECDSA ? + (mbedtls_ecp_keypair *) (pk).pk_ctx : + NULL ); } #endif /* MBEDTLS_ECP_C */ From d904d66639aa82aac11d5964a597776df5485db3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 17 Jun 2022 10:24:00 +0200 Subject: [PATCH 18/20] Mark static int SSL functions CHECK_RETURN_CRITICAL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_ciphersuites.c | 1 + library/ssl_cli.c | 48 ++++++++++++++++++++++++++++++++++++++ library/ssl_cookie.c | 1 + library/ssl_msg.c | 42 +++++++++++++++++++++++++++++++++ library/ssl_srv.c | 39 +++++++++++++++++++++++++++++++ library/ssl_ticket.c | 2 ++ library/ssl_tls.c | 29 +++++++++++++++++++++++ 7 files changed, 162 insertions(+) diff --git a/library/ssl_ciphersuites.c b/library/ssl_ciphersuites.c index 3826ad27f..ceec77efb 100644 --- a/library/ssl_ciphersuites.c +++ b/library/ssl_ciphersuites.c @@ -2181,6 +2181,7 @@ const int *mbedtls_ssl_list_ciphersuites( void ) static int supported_ciphersuites[MAX_CIPHERSUITES]; static int supported_init = 0; +MBEDTLS_CHECK_RETURN_CRITICAL static int ciphersuite_is_removed( const mbedtls_ssl_ciphersuite_t *cs_info ) { (void)cs_info; diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 1140d9e3e..d7a5dac1b 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -53,6 +53,7 @@ #endif #if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_conf_has_static_psk( mbedtls_ssl_config const *conf ) { if( conf->psk_identity == NULL || @@ -73,6 +74,7 @@ static int ssl_conf_has_static_psk( mbedtls_ssl_config const *conf ) } #if defined(MBEDTLS_USE_PSA_CRYPTO) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_conf_has_static_raw_psk( mbedtls_ssl_config const *conf ) { if( conf->psk_identity == NULL || @@ -91,6 +93,7 @@ static int ssl_conf_has_static_raw_psk( mbedtls_ssl_config const *conf ) #endif /* MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */ #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_hostname_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -161,6 +164,7 @@ static int ssl_write_hostname_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ #if defined(MBEDTLS_SSL_RENEGOTIATION) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_renegotiation_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -204,6 +208,7 @@ static int ssl_write_renegotiation_ext( mbedtls_ssl_context *ssl, */ #if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_signature_algorithms_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -302,6 +307,7 @@ static int ssl_write_signature_algorithms_ext( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -373,6 +379,7 @@ static int ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl, return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_supported_point_formats_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -404,6 +411,7 @@ static int ssl_write_supported_point_formats_ext( mbedtls_ssl_context *ssl, MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ #if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_ecjpake_kkpp_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -477,6 +485,7 @@ static int ssl_write_ecjpake_kkpp_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_cid_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -523,6 +532,7 @@ static int ssl_write_cid_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_max_fragment_length_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -555,6 +565,7 @@ static int ssl_write_max_fragment_length_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ #if defined(MBEDTLS_SSL_TRUNCATED_HMAC) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_truncated_hmac_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -585,6 +596,7 @@ static int ssl_write_truncated_hmac_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_TRUNCATED_HMAC */ #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -616,6 +628,7 @@ static int ssl_write_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_ENCRYPT_THEN_MAC */ #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_extended_ms_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -647,6 +660,7 @@ static int ssl_write_extended_ms_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ #if defined(MBEDTLS_SSL_SESSION_TICKETS) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_session_ticket_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -689,6 +703,7 @@ static int ssl_write_session_ticket_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_SESSION_TICKETS */ #if defined(MBEDTLS_SSL_ALPN) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_alpn_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -748,6 +763,7 @@ static int ssl_write_alpn_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_ALPN */ #if defined(MBEDTLS_SSL_DTLS_SRTP) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_use_srtp_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, @@ -868,6 +884,7 @@ static int ssl_write_use_srtp_ext( mbedtls_ssl_context *ssl, /* * Generate random bytes for ClientHello */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_generate_random( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -917,6 +934,7 @@ static int ssl_generate_random( mbedtls_ssl_context *ssl ) * * \return 0 if valid, else 1 */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_validate_ciphersuite( const mbedtls_ssl_ciphersuite_t * suite_info, const mbedtls_ssl_context * ssl, @@ -960,6 +978,7 @@ static int ssl_validate_ciphersuite( return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -1450,6 +1469,7 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_renegotiation_info( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -1494,6 +1514,7 @@ static int ssl_parse_renegotiation_info( mbedtls_ssl_context *ssl, } #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_max_fragment_length_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -1520,6 +1541,7 @@ static int ssl_parse_max_fragment_length_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ #if defined(MBEDTLS_SSL_TRUNCATED_HMAC) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_truncated_hmac_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -1545,6 +1567,7 @@ static int ssl_parse_truncated_hmac_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_TRUNCATED_HMAC */ #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_cid_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -1601,6 +1624,7 @@ static int ssl_parse_cid_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -1627,6 +1651,7 @@ static int ssl_parse_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_ENCRYPT_THEN_MAC */ #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_extended_ms_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -1653,6 +1678,7 @@ static int ssl_parse_extended_ms_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ #if defined(MBEDTLS_SSL_SESSION_TICKETS) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_session_ticket_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -1679,6 +1705,7 @@ static int ssl_parse_session_ticket_ext( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_supported_point_formats_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -1724,6 +1751,7 @@ static int ssl_parse_supported_point_formats_ext( mbedtls_ssl_context *ssl, MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ #if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_ecjpake_kkpp( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -1758,6 +1786,7 @@ static int ssl_parse_ecjpake_kkpp( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ #if defined(MBEDTLS_SSL_ALPN) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_alpn_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) { @@ -1828,6 +1857,7 @@ static int ssl_parse_alpn_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_ALPN */ #if defined(MBEDTLS_SSL_DTLS_SRTP) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_use_srtp_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -1948,6 +1978,7 @@ static int ssl_parse_use_srtp_ext( mbedtls_ssl_context *ssl, * Parse HelloVerifyRequest. Only called after verifying the HS type. */ #if defined(MBEDTLS_SSL_PROTO_DTLS) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_hello_verify_request( mbedtls_ssl_context *ssl ) { const unsigned char *p = ssl->in_msg + mbedtls_ssl_hs_hdr_len( ssl ); @@ -2031,6 +2062,7 @@ static int ssl_parse_hello_verify_request( mbedtls_ssl_context *ssl ) } #endif /* MBEDTLS_SSL_PROTO_DTLS */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) { int ret, i; @@ -2591,6 +2623,7 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_server_dh_params( mbedtls_ssl_context *ssl, unsigned char **p, unsigned char *end ) @@ -2637,6 +2670,7 @@ static int ssl_parse_server_dh_params( mbedtls_ssl_context *ssl, defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_check_server_ecdh_params( const mbedtls_ssl_context *ssl ) { const mbedtls_ecp_curve_info *curve_info; @@ -2678,6 +2712,7 @@ static int ssl_check_server_ecdh_params( const mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_USE_PSA_CRYPTO) && \ ( defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) ) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_server_ecdh_params_psa( mbedtls_ssl_context *ssl, unsigned char **p, unsigned char *end ) @@ -2744,6 +2779,7 @@ static int ssl_parse_server_ecdh_params_psa( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_server_ecdh_params( mbedtls_ssl_context *ssl, unsigned char **p, unsigned char *end ) @@ -2783,6 +2819,7 @@ static int ssl_parse_server_ecdh_params( mbedtls_ssl_context *ssl, MBEDTLS_KEY_EXCHANGE_ECDHE_PSK_ENABLED */ #if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_server_psk_hint( mbedtls_ssl_context *ssl, unsigned char **p, unsigned char *end ) @@ -2829,6 +2866,7 @@ static int ssl_parse_server_psk_hint( mbedtls_ssl_context *ssl, /* * Generate a pre-master secret and encrypt it with the server's RSA key */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_encrypted_pms( mbedtls_ssl_context *ssl, size_t offset, size_t *olen, size_t pms_offset ) @@ -2916,6 +2954,7 @@ static int ssl_write_encrypted_pms( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDHE_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_signature_algorithm( mbedtls_ssl_context *ssl, unsigned char **p, unsigned char *end, @@ -2982,6 +3021,7 @@ static int ssl_parse_signature_algorithm( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_get_ecdh_params_from_cert( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -3035,6 +3075,7 @@ static int ssl_get_ecdh_params_from_cert( mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -3441,6 +3482,7 @@ exit: } #if ! defined(MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) { const mbedtls_ssl_ciphersuite_t *ciphersuite_info = @@ -3459,6 +3501,7 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } #else /* MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -3630,6 +3673,7 @@ exit: } #endif /* MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_server_hello_done( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -3669,6 +3713,7 @@ static int ssl_parse_server_hello_done( mbedtls_ssl_context *ssl ) return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -4101,6 +4146,7 @@ ecdh_calc_secret: } #if !defined(MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_certificate_verify( mbedtls_ssl_context *ssl ) { const mbedtls_ssl_ciphersuite_t *ciphersuite_info = @@ -4126,6 +4172,7 @@ static int ssl_write_certificate_verify( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } #else /* !MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_certificate_verify( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; @@ -4298,6 +4345,7 @@ sign: #endif /* MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED */ #if defined(MBEDTLS_SSL_SESSION_TICKETS) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_new_session_ticket( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; diff --git a/library/ssl_cookie.c b/library/ssl_cookie.c index 519d6d7a3..3781796b7 100644 --- a/library/ssl_cookie.c +++ b/library/ssl_cookie.c @@ -122,6 +122,7 @@ int mbedtls_ssl_cookie_setup( mbedtls_ssl_cookie_ctx *ctx, /* * Generate the HMAC part of a cookie */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_cookie_hmac( mbedtls_md_context_t *hmac_ctx, const unsigned char time[4], unsigned char **p, unsigned char *end, diff --git a/library/ssl_msg.c b/library/ssl_msg.c index dfcdc9327..217859cdc 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -91,6 +91,7 @@ int mbedtls_ssl_check_timer( mbedtls_ssl_context *ssl ) } #if defined(MBEDTLS_SSL_RECORD_CHECKING) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_record_header( mbedtls_ssl_context const *ssl, unsigned char *buf, size_t len, @@ -165,11 +166,16 @@ exit: static void ssl_buffering_free_slot( mbedtls_ssl_context *ssl, uint8_t slot ); static void ssl_free_buffered_record( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_load_buffered_message( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_load_buffered_record( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_buffer_message( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_buffer_future_record( mbedtls_ssl_context *ssl, mbedtls_record const *rec ); +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_next_record_is_in_datagram( mbedtls_ssl_context *ssl ); static size_t ssl_get_maximum_datagram_size( mbedtls_ssl_context const *ssl ) @@ -187,6 +193,7 @@ static size_t ssl_get_maximum_datagram_size( mbedtls_ssl_context const *ssl ) return( out_buf_len ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_get_remaining_space_in_datagram( mbedtls_ssl_context const *ssl ) { size_t const bytes_written = ssl->out_left; @@ -203,6 +210,7 @@ static int ssl_get_remaining_space_in_datagram( mbedtls_ssl_context const *ssl ) return( (int) ( mtu - bytes_written ) ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_get_remaining_payload_in_datagram( mbedtls_ssl_context const *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -254,6 +262,7 @@ static int ssl_get_remaining_payload_in_datagram( mbedtls_ssl_context const *ssl * Double the retransmit timeout value, within the allowed range, * returning -1 if the maximum value has already been reached. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_double_retransmit_timeout( mbedtls_ssl_context *ssl ) { uint32_t new_timeout; @@ -353,6 +362,7 @@ static size_t ssl_compute_padding_length( size_t len, * - A negative error code if `max_len` didn't offer enough space * for the expansion. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_build_inner_plaintext( unsigned char *content, size_t *content_size, size_t remaining, @@ -380,6 +390,7 @@ static int ssl_build_inner_plaintext( unsigned char *content, /* This function parses a (D)TLSInnerPlaintext structure. * See ssl_build_inner_plaintext() for details. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_inner_plaintext( unsigned char const *content, size_t *content_size, uint8_t *rec_type ) @@ -474,6 +485,7 @@ static void ssl_extract_add_data_from_record( unsigned char* add_data, /* * SSLv3.0 MAC functions */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_mac( mbedtls_md_context_t *md_ctx, const unsigned char *secret, const unsigned char *buf, size_t len, @@ -541,6 +553,7 @@ static int ssl_mac( mbedtls_md_context_t *md_ctx, #if defined(MBEDTLS_GCM_C) || \ defined(MBEDTLS_CCM_C) || \ defined(MBEDTLS_CHACHAPOLY_C) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_transform_aead_dynamic_iv_is_explicit( mbedtls_ssl_transform const *transform ) { @@ -1738,6 +1751,7 @@ int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, /* * Compression/decompression functions */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_compress_buf( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -1790,6 +1804,7 @@ static int ssl_compress_buf( mbedtls_ssl_context *ssl ) return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_decompress_buf( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -2149,6 +2164,7 @@ int mbedtls_ssl_flush_output( mbedtls_ssl_context *ssl ) /* * Append current handshake message to current outgoing flight */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_flight_append( mbedtls_ssl_context *ssl ) { mbedtls_ssl_flight_item *msg; @@ -2215,6 +2231,7 @@ void mbedtls_ssl_flight_free( mbedtls_ssl_flight_item *flight ) /* * Swap transform_out and out_ctr with the alternative ones */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_swap_epochs( mbedtls_ssl_context *ssl ) { mbedtls_ssl_transform *tmp_transform; @@ -2857,6 +2874,7 @@ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ) #if defined(MBEDTLS_SSL_PROTO_DTLS) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_hs_is_proper_fragment( mbedtls_ssl_context *ssl ) { if( ssl->in_msglen < ssl->in_hslen || @@ -2882,6 +2900,7 @@ static uint32_t ssl_get_hs_frag_off( mbedtls_ssl_context const *ssl ) ssl->in_msg[8] ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_check_hs_header( mbedtls_ssl_context const *ssl ) { uint32_t msg_len, frag_off, frag_len; @@ -2948,6 +2967,7 @@ static void ssl_bitmask_set( unsigned char *mask, size_t offset, size_t len ) /* * Check that bitmask is full */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_bitmask_check( unsigned char *mask, size_t len ) { size_t i; @@ -3147,6 +3167,7 @@ static inline uint64_t ssl_load_six_bytes( unsigned char *buf ) ( (uint64_t) buf[5] ) ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int mbedtls_ssl_dtls_record_replay_check( mbedtls_ssl_context *ssl, uint8_t *record_in_ctr ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -3239,6 +3260,7 @@ void mbedtls_ssl_dtls_replay_update( mbedtls_ssl_context *ssl ) * return MBEDTLS_ERR_SSL_HELLO_VERIFY_REQUIRED * - otherwise return a specific error code */ +MBEDTLS_CHECK_RETURN_CRITICAL MBEDTLS_STATIC_TESTABLE int mbedtls_ssl_check_dtls_clihlo_cookie( mbedtls_ssl_context *ssl, @@ -3397,6 +3419,7 @@ int mbedtls_ssl_check_dtls_clihlo_cookie( * includes the case of MBEDTLS_ERR_SSL_CLIENT_RECONNECT and of unexpected * errors, and is the right thing to do in both cases). */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_handle_possible_reconnect( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -3452,6 +3475,7 @@ static int ssl_handle_possible_reconnect( mbedtls_ssl_context *ssl ) } #endif /* MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE && MBEDTLS_SSL_SRV_C */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_check_record_type( uint8_t record_type ) { if( record_type != MBEDTLS_SSL_MSG_HANDSHAKE && @@ -3484,6 +3508,7 @@ static int ssl_check_record_type( uint8_t record_type ) * Point 2 is needed when the peer is resending, and we have already received * the first record from a datagram but are still waiting for the others. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_record_header( mbedtls_ssl_context const *ssl, unsigned char *buf, size_t len, @@ -3719,6 +3744,7 @@ static int ssl_parse_record_header( mbedtls_ssl_context const *ssl, #if defined(MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE) && defined(MBEDTLS_SSL_SRV_C) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_check_client_reconnect( mbedtls_ssl_context *ssl ) { unsigned int rec_epoch = ( ssl->in_ctr[0] << 8 ) | ssl->in_ctr[1]; @@ -3748,6 +3774,7 @@ static int ssl_check_client_reconnect( mbedtls_ssl_context *ssl ) /* * If applicable, decrypt record content */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_prepare_record_content( mbedtls_ssl_context *ssl, mbedtls_record *rec ) { @@ -3899,8 +3926,11 @@ static int ssl_prepare_record_content( mbedtls_ssl_context *ssl, */ /* Helper functions for mbedtls_ssl_read_record(). */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_consume_current_message( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_get_next_record( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_record_is_in_progress( mbedtls_ssl_context *ssl ); int mbedtls_ssl_read_record( mbedtls_ssl_context *ssl, @@ -3988,6 +4018,7 @@ int mbedtls_ssl_read_record( mbedtls_ssl_context *ssl, } #if defined(MBEDTLS_SSL_PROTO_DTLS) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_next_record_is_in_datagram( mbedtls_ssl_context *ssl ) { if( ssl->in_left > ssl->next_record_offset ) @@ -3996,6 +4027,7 @@ static int ssl_next_record_is_in_datagram( mbedtls_ssl_context *ssl ) return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_load_buffered_message( mbedtls_ssl_context *ssl ) { mbedtls_ssl_handshake_params * const hs = ssl->handshake; @@ -4093,6 +4125,7 @@ exit: return( ret ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_buffer_make_space( mbedtls_ssl_context *ssl, size_t desired ) { @@ -4135,6 +4168,7 @@ static int ssl_buffer_make_space( mbedtls_ssl_context *ssl, return( -1 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_buffer_message( mbedtls_ssl_context *ssl ) { int ret = 0; @@ -4339,6 +4373,7 @@ exit: } #endif /* MBEDTLS_SSL_PROTO_DTLS */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_consume_current_message( mbedtls_ssl_context *ssl ) { /* @@ -4426,6 +4461,7 @@ static int ssl_consume_current_message( mbedtls_ssl_context *ssl ) return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_record_is_in_progress( mbedtls_ssl_context *ssl ) { if( ssl->in_msglen > 0 ) @@ -4452,6 +4488,7 @@ static void ssl_free_buffered_record( mbedtls_ssl_context *ssl ) } } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_load_buffered_record( mbedtls_ssl_context *ssl ) { mbedtls_ssl_handshake_params * const hs = ssl->handshake; @@ -4509,6 +4546,7 @@ exit: return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_buffer_future_record( mbedtls_ssl_context *ssl, mbedtls_record const *rec ) { @@ -4567,6 +4605,7 @@ static int ssl_buffer_future_record( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_PROTO_DTLS */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_get_next_record( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -5317,6 +5356,7 @@ int mbedtls_ssl_get_record_expansion( const mbedtls_ssl_context *ssl ) /* * Check record counters and renegotiate if they're above the limit. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_check_ctr_renegotiate( mbedtls_ssl_context *ssl ) { size_t ep_len = mbedtls_ssl_ep_len( ssl ); @@ -5667,6 +5707,7 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) * Therefore, it is possible that the input message length is 0 and the * corresponding return code is 0 on success. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_real( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) { @@ -5738,6 +5779,7 @@ static int ssl_write_real( mbedtls_ssl_context *ssl, * remember whether we already did the split or not. */ #if defined(MBEDTLS_SSL_CBC_RECORD_SPLITTING) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_split( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) { diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 64e78a9ff..2efb13cc3 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -78,6 +78,7 @@ void mbedtls_ssl_conf_dtls_cookies( mbedtls_ssl_config *conf, #endif /* MBEDTLS_SSL_DTLS_HELLO_VERIFY */ #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_servername_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -147,6 +148,7 @@ static int ssl_parse_servername_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ #if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_conf_has_psk_or_cb( mbedtls_ssl_config const *conf ) { if( conf->f_psk != NULL ) @@ -167,6 +169,7 @@ static int ssl_conf_has_psk_or_cb( mbedtls_ssl_config const *conf ) } #if defined(MBEDTLS_USE_PSA_CRYPTO) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_use_opaque_psk( mbedtls_ssl_context const *ssl ) { if( ssl->conf->f_psk != NULL ) @@ -188,6 +191,7 @@ static int ssl_use_opaque_psk( mbedtls_ssl_context const *ssl ) #endif /* MBEDTLS_USE_PSA_CRYPTO */ #endif /* MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_renegotiation_info( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -239,6 +243,7 @@ static int ssl_parse_renegotiation_info( mbedtls_ssl_context *ssl, * This needs to be done at a later stage. * */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_signature_algorithms_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -317,6 +322,7 @@ static int ssl_parse_signature_algorithms_ext( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_supported_elliptic_curves( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -383,6 +389,7 @@ static int ssl_parse_supported_elliptic_curves( mbedtls_ssl_context *ssl, return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_supported_point_formats( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -425,6 +432,7 @@ static int ssl_parse_supported_point_formats( mbedtls_ssl_context *ssl, MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ #if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_ecjpake_kkpp( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -454,6 +462,7 @@ static int ssl_parse_ecjpake_kkpp( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_max_fragment_length_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -473,6 +482,7 @@ static int ssl_parse_max_fragment_length_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_cid_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -545,6 +555,7 @@ static int ssl_parse_cid_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ #if defined(MBEDTLS_SSL_TRUNCATED_HMAC) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_truncated_hmac_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -567,6 +578,7 @@ static int ssl_parse_truncated_hmac_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_TRUNCATED_HMAC */ #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -592,6 +604,7 @@ static int ssl_parse_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_ENCRYPT_THEN_MAC */ #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_extended_ms_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -617,6 +630,7 @@ static int ssl_parse_extended_ms_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ #if defined(MBEDTLS_SSL_SESSION_TICKETS) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_session_ticket_ext( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) @@ -691,6 +705,7 @@ static int ssl_parse_session_ticket_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_SESSION_TICKETS */ #if defined(MBEDTLS_SSL_ALPN) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_alpn_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) { @@ -779,6 +794,7 @@ static int ssl_parse_alpn_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_ALPN */ #if defined(MBEDTLS_SSL_DTLS_SRTP) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_use_srtp_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) @@ -907,6 +923,7 @@ static int ssl_parse_use_srtp_ext( mbedtls_ssl_context *ssl, * Return 0 if the given key uses one of the acceptable curves, -1 otherwise */ #if defined(MBEDTLS_ECDSA_C) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_check_key_curve( mbedtls_pk_context *pk, const mbedtls_ecp_curve_info **curves ) { @@ -928,6 +945,7 @@ static int ssl_check_key_curve( mbedtls_pk_context *pk, * Try picking a certificate for this ciphersuite, * return 0 on success and -1 on failure. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_pick_cert( mbedtls_ssl_context *ssl, const mbedtls_ssl_ciphersuite_t * ciphersuite_info ) { @@ -1032,6 +1050,7 @@ static int ssl_pick_cert( mbedtls_ssl_context *ssl, * Check if a given ciphersuite is suitable for use with our config/keys/etc * Sets ciphersuite_info only if the suite matches. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_ciphersuite_match( mbedtls_ssl_context *ssl, int suite_id, const mbedtls_ssl_ciphersuite_t **ciphersuite_info ) { @@ -1147,6 +1166,7 @@ static int ssl_ciphersuite_match( mbedtls_ssl_context *ssl, int suite_id, } #if defined(MBEDTLS_SSL_SRV_SUPPORT_SSLV2_CLIENT_HELLO) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_client_hello_v2( mbedtls_ssl_context *ssl ) { int ret, got_common_suite; @@ -1410,6 +1430,7 @@ have_ciphersuite_v2: /* This function doesn't alert on errors that happen early during ClientHello parsing because they might indicate that the client is not talking SSL/TLS at all and would not understand our alert. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_client_hello( mbedtls_ssl_context *ssl ) { int ret, got_common_suite; @@ -2699,6 +2720,7 @@ static void ssl_write_use_srtp_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_SSL_DTLS_SRTP */ #if defined(MBEDTLS_SSL_DTLS_HELLO_VERIFY) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_hello_verify_request( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -2819,6 +2841,7 @@ exit: mbedtls_ssl_session_free( &session_tmp ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) { #if defined(MBEDTLS_HAVE_TIME) @@ -3049,6 +3072,7 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) } #if !defined(MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_certificate_request( mbedtls_ssl_context *ssl ) { const mbedtls_ssl_ciphersuite_t *ciphersuite_info = @@ -3067,6 +3091,7 @@ static int ssl_write_certificate_request( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } #else /* !MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_certificate_request( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; @@ -3236,6 +3261,7 @@ static int ssl_write_certificate_request( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_KEY_EXCHANGE_ECDH_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_ECDH_ECDSA_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_get_ecdh_params_from_cert( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -3265,6 +3291,7 @@ static int ssl_get_ecdh_params_from_cert( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_KEY_EXCHANGE_WITH_SERVER_SIGNATURE_ENABLED) && \ defined(MBEDTLS_SSL_ASYNC_PRIVATE) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_resume_server_key_exchange( mbedtls_ssl_context *ssl, size_t *signature_len ) { @@ -3292,6 +3319,7 @@ static int ssl_resume_server_key_exchange( mbedtls_ssl_context *ssl, /* Prepare the ServerKeyExchange message, up to and including * calculating the signature if any, but excluding formatting the * signature and sending the message. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_prepare_server_key_exchange( mbedtls_ssl_context *ssl, size_t *signature_len ) { @@ -3661,6 +3689,7 @@ curve_matching_done: * that do not include a ServerKeyExchange message, do nothing. Either * way, if successful, move on to the next step in the SSL state * machine. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_server_key_exchange( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -3763,6 +3792,7 @@ static int ssl_write_server_key_exchange( mbedtls_ssl_context *ssl ) return( 0 ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_server_hello_done( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -3802,6 +3832,7 @@ static int ssl_write_server_hello_done( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_client_dh_public( mbedtls_ssl_context *ssl, unsigned char **p, const unsigned char *end ) { @@ -3845,6 +3876,7 @@ static int ssl_parse_client_dh_public( mbedtls_ssl_context *ssl, unsigned char * defined(MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED) #if defined(MBEDTLS_SSL_ASYNC_PRIVATE) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_resume_decrypt_pms( mbedtls_ssl_context *ssl, unsigned char *peer_pms, size_t *peer_pmslen, @@ -3862,6 +3894,7 @@ static int ssl_resume_decrypt_pms( mbedtls_ssl_context *ssl, } #endif /* MBEDTLS_SSL_ASYNC_PRIVATE */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_decrypt_encrypted_pms( mbedtls_ssl_context *ssl, const unsigned char *p, const unsigned char *end, @@ -3954,6 +3987,7 @@ static int ssl_decrypt_encrypted_pms( mbedtls_ssl_context *ssl, return( ret ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_encrypted_pms( mbedtls_ssl_context *ssl, const unsigned char *p, const unsigned char *end, @@ -4043,6 +4077,7 @@ static int ssl_parse_encrypted_pms( mbedtls_ssl_context *ssl, MBEDTLS_KEY_EXCHANGE_RSA_PSK_ENABLED */ #if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_client_psk_identity( mbedtls_ssl_context *ssl, unsigned char **p, const unsigned char *end ) { @@ -4103,6 +4138,7 @@ static int ssl_parse_client_psk_identity( mbedtls_ssl_context *ssl, unsigned cha } #endif /* MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -4418,6 +4454,7 @@ static int ssl_parse_client_key_exchange( mbedtls_ssl_context *ssl ) } #if !defined(MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) { const mbedtls_ssl_ciphersuite_t *ciphersuite_info = @@ -4436,6 +4473,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } #else /* !MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; @@ -4629,6 +4667,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_KEY_EXCHANGE_CERT_REQ_ALLOWED_ENABLED */ #if defined(MBEDTLS_SSL_SESSION_TICKETS) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_new_session_ticket( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; diff --git a/library/ssl_ticket.c b/library/ssl_ticket.c index 5680a0b16..e0126cc9d 100644 --- a/library/ssl_ticket.c +++ b/library/ssl_ticket.c @@ -66,6 +66,7 @@ void mbedtls_ssl_ticket_init( mbedtls_ssl_ticket_context *ctx ) /* * Generate/update a key */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_ticket_gen_key( mbedtls_ssl_ticket_context *ctx, unsigned char index ) { @@ -96,6 +97,7 @@ static int ssl_ticket_gen_key( mbedtls_ssl_ticket_context *ctx, /* * Rotate/generate keys if necessary */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_ticket_update_keys( mbedtls_ssl_ticket_context *ctx ) { #if !defined(MBEDTLS_HAVE_TIME) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 0099b4396..7badec51a 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -245,6 +245,7 @@ int mbedtls_ssl_session_copy( mbedtls_ssl_session *dst, } #if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) +MBEDTLS_CHECK_RETURN_CRITICAL static int resize_buffer( unsigned char **buffer, size_t len_new, size_t *len_old ) { unsigned char* resized_buffer = mbedtls_calloc( 1, len_new ); @@ -337,6 +338,7 @@ static void handle_buffer_resizing( mbedtls_ssl_context *ssl, int downsizing, * Key material generation */ #if defined(MBEDTLS_SSL_PROTO_SSL3) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl3_prf( const unsigned char *secret, size_t slen, const char *label, const unsigned char *random, size_t rlen, @@ -398,6 +400,7 @@ exit: #endif /* MBEDTLS_SSL_PROTO_SSL3 */ #if defined(MBEDTLS_SSL_PROTO_TLS1) || defined(MBEDTLS_SSL_PROTO_TLS1_1) +MBEDTLS_CHECK_RETURN_CRITICAL static int tls1_prf( const unsigned char *secret, size_t slen, const char *label, const unsigned char *random, size_t rlen, @@ -605,6 +608,7 @@ static psa_status_t setup_psa_key_derivation( psa_key_derivation_operation_t* de return( PSA_SUCCESS ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int tls_prf_generic( mbedtls_md_type_t md_type, const unsigned char *secret, size_t slen, const char *label, @@ -679,6 +683,7 @@ static int tls_prf_generic( mbedtls_md_type_t md_type, #else /* MBEDTLS_USE_PSA_CRYPTO */ +MBEDTLS_CHECK_RETURN_CRITICAL static int tls_prf_generic( mbedtls_md_type_t md_type, const unsigned char *secret, size_t slen, const char *label, @@ -770,6 +775,7 @@ exit: } #endif /* MBEDTLS_USE_PSA_CRYPTO */ #if defined(MBEDTLS_SHA256_C) +MBEDTLS_CHECK_RETURN_CRITICAL static int tls_prf_sha256( const unsigned char *secret, size_t slen, const char *label, const unsigned char *random, size_t rlen, @@ -781,6 +787,7 @@ static int tls_prf_sha256( const unsigned char *secret, size_t slen, #endif /* MBEDTLS_SHA256_C */ #if defined(MBEDTLS_SHA512_C) && !defined(MBEDTLS_SHA512_NO_SHA384) +MBEDTLS_CHECK_RETURN_CRITICAL static int tls_prf_sha384( const unsigned char *secret, size_t slen, const char *label, const unsigned char *random, size_t rlen, @@ -825,6 +832,7 @@ static void ssl_calc_finished_tls_sha384( mbedtls_ssl_context *, unsigned char * #if defined(MBEDTLS_KEY_EXCHANGE_PSK_ENABLED) && \ defined(MBEDTLS_USE_PSA_CRYPTO) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_use_opaque_psk( mbedtls_ssl_context const *ssl ) { if( ssl->conf->f_psk != NULL ) @@ -949,6 +957,7 @@ typedef int ssl_tls_prf_t(const unsigned char *, size_t, const char *, * - MBEDTLS_SSL_EXPORT_KEYS: ssl->conf->{f,p}_export_keys * - MBEDTLS_DEBUG_C: ssl->conf->{f,p}_dbg */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_populate_transform( mbedtls_ssl_transform *transform, int ciphersuite, const unsigned char master[48], @@ -1512,6 +1521,7 @@ end: * Outputs: * - the tls_prf, calc_verify and calc_finished members of handshake structure */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_set_handshake_prfs( mbedtls_ssl_handshake_params *handshake, int minor_ver, mbedtls_md_type_t hash ) @@ -1581,6 +1591,7 @@ static int ssl_set_handshake_prfs( mbedtls_ssl_handshake_params *handshake, * EMS: passed to calc_verify (debug + (SSL3) session_negotiate) * PSA-PSA: minor_ver, conf */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_compute_master( mbedtls_ssl_handshake_params *handshake, unsigned char *master, const mbedtls_ssl_context *ssl ) @@ -2109,6 +2120,7 @@ int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exch #endif /* MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */ #if defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_SSL_RENEGOTIATION) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_hello_request( mbedtls_ssl_context *ssl ); #if defined(MBEDTLS_SSL_PROTO_DTLS) @@ -2324,6 +2336,7 @@ write_msg: #if defined(MBEDTLS_SSL_RENEGOTIATION) && defined(MBEDTLS_SSL_CLI_C) #if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_check_peer_crt_unchanged( mbedtls_ssl_context *ssl, unsigned char *crt_buf, size_t crt_buf_len ) @@ -2339,6 +2352,7 @@ static int ssl_check_peer_crt_unchanged( mbedtls_ssl_context *ssl, return( memcmp( peer_crt->raw.p, crt_buf, peer_crt->raw.len ) ); } #else /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_check_peer_crt_unchanged( mbedtls_ssl_context *ssl, unsigned char *crt_buf, size_t crt_buf_len ) @@ -2373,6 +2387,7 @@ static int ssl_check_peer_crt_unchanged( mbedtls_ssl_context *ssl, * Once the certificate message is read, parse it into a cert chain and * perform basic checks, but leave actual verification to the caller */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_certificate_chain( mbedtls_ssl_context *ssl, mbedtls_x509_crt *chain ) { @@ -2522,6 +2537,7 @@ static int ssl_parse_certificate_chain( mbedtls_ssl_context *ssl, } #if defined(MBEDTLS_SSL_SRV_C) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_srv_check_client_no_crt_notification( mbedtls_ssl_context *ssl ) { if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) @@ -2571,6 +2587,7 @@ static int ssl_srv_check_client_no_crt_notification( mbedtls_ssl_context *ssl ) */ #define SSL_CERTIFICATE_EXPECTED 0 #define SSL_CERTIFICATE_SKIP 1 +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_certificate_coordinate( mbedtls_ssl_context *ssl, int authmode ) { @@ -2600,6 +2617,7 @@ static int ssl_parse_certificate_coordinate( mbedtls_ssl_context *ssl, return( SSL_CERTIFICATE_EXPECTED ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, int authmode, mbedtls_x509_crt *chain, @@ -2790,6 +2808,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl, } #if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_remember_peer_crt_digest( mbedtls_ssl_context *ssl, unsigned char *start, size_t len ) { @@ -2821,6 +2840,7 @@ static int ssl_remember_peer_crt_digest( mbedtls_ssl_context *ssl, return( ret ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_remember_peer_pubkey( mbedtls_ssl_context *ssl, unsigned char *start, size_t len ) { @@ -3799,6 +3819,7 @@ void mbedtls_ssl_session_init( mbedtls_ssl_session *session ) memset( session, 0, sizeof(mbedtls_ssl_session) ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_handshake_init( mbedtls_ssl_context *ssl ) { /* Clear old handshake information if present */ @@ -3876,6 +3897,7 @@ static int ssl_handshake_init( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_DTLS_HELLO_VERIFY) && defined(MBEDTLS_SSL_SRV_C) /* Dummy cookie callbacks for defaults */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_cookie_write_dummy( void *ctx, unsigned char **p, unsigned char *end, const unsigned char *cli_id, size_t cli_id_len ) @@ -3889,6 +3911,7 @@ static int ssl_cookie_write_dummy( void *ctx, return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); } +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_cookie_check_dummy( void *ctx, const unsigned char *cookie, size_t cookie_len, const unsigned char *cli_id, size_t cli_id_len ) @@ -4306,6 +4329,7 @@ void mbedtls_ssl_conf_cert_profile( mbedtls_ssl_config *conf, } /* Append a new keycert entry to a (possibly empty) list */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_append_key_cert( mbedtls_ssl_key_cert **head, mbedtls_x509_crt *cert, mbedtls_pk_context *key ) @@ -4474,6 +4498,7 @@ static void ssl_conf_remove_psk( mbedtls_ssl_config *conf ) * It checks that the provided identity is well-formed and attempts * to make a copy of it in the SSL config. * On failure, the PSK identity in the config remains unset. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_conf_set_psk_identity( mbedtls_ssl_config *conf, unsigned char const *psk_identity, size_t psk_identity_len ) @@ -5393,6 +5418,7 @@ static unsigned char ssl_serialized_session_header[] = { * verify_result is put before peer_cert so that all mandatory fields come * together in one block. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_session_save( const mbedtls_ssl_session *session, unsigned char omit_header, unsigned char *buf, @@ -5592,6 +5618,7 @@ int mbedtls_ssl_session_save( const mbedtls_ssl_session *session, * This internal version is wrapped by a public function that cleans up in * case of error, and has an extra option omit_header. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_session_load( mbedtls_ssl_session *session, unsigned char omit_header, const unsigned char *buf, @@ -5895,6 +5922,7 @@ int mbedtls_ssl_handshake( mbedtls_ssl_context *ssl ) /* * Write HelloRequest to request renegotiation on server */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_write_hello_request( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -6506,6 +6534,7 @@ static tls_prf_fn ssl_tls12prf_from_cs( int ciphersuite_id ) * This internal version is wrapped by a public function that cleans up in * case of error. */ +MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_context_load( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) From 5721059281eb189711ddc12122dd8ef560d643ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 17 Jun 2022 10:32:25 +0200 Subject: [PATCH 19/20] Mark internal int SSL functions CHECK_RETURN_CRITICAL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/ssl_internal.h | 35 ++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 83047c5bd..46ade67b9 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -934,16 +934,22 @@ void mbedtls_ssl_transform_free( mbedtls_ssl_transform *transform ); */ void mbedtls_ssl_handshake_free( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_handshake_client_step( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_handshake_server_step( mbedtls_ssl_context *ssl ); void mbedtls_ssl_handshake_wrapup( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_send_fatal_handshake_failure( mbedtls_ssl_context *ssl ); void mbedtls_ssl_reset_checksum( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_handle_message_type( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_prepare_handshake_record( mbedtls_ssl_context *ssl ); void mbedtls_ssl_update_handshake_status( mbedtls_ssl_context *ssl ); @@ -1023,27 +1029,39 @@ void mbedtls_ssl_update_handshake_status( mbedtls_ssl_context *ssl ); * following the above definition. * */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_read_record( mbedtls_ssl_context *ssl, unsigned update_hs_digest ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_fetch_input( mbedtls_ssl_context *ssl, size_t nb_want ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_flush_output( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_parse_certificate( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_write_certificate( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_parse_change_cipher_spec( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_write_change_cipher_spec( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_parse_finished( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_write_finished( mbedtls_ssl_context *ssl ); void mbedtls_ssl_optimize_checksum( mbedtls_ssl_context *ssl, const mbedtls_ssl_ciphersuite_t *ciphersuite_info ); #if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_psk_derive_premaster( mbedtls_ssl_context *ssl, mbedtls_key_exchange_type_t key_ex ); /** @@ -1108,14 +1126,18 @@ mbedtls_pk_type_t mbedtls_ssl_pk_alg_from_sig( unsigned char sig ); mbedtls_md_type_t mbedtls_ssl_md_alg_from_hash( unsigned char hash ); unsigned char mbedtls_ssl_hash_from_md_alg( int md ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_set_calc_verify_md( mbedtls_ssl_context *ssl, int md ); #if defined(MBEDTLS_ECP_C) +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_check_curve( const mbedtls_ssl_context *ssl, mbedtls_ecp_group_id grp_id ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_check_curve_tls_id( const mbedtls_ssl_context *ssl, uint16_t tls_id ); #endif #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_check_sig_hash( const mbedtls_ssl_context *ssl, mbedtls_md_type_t md ); #endif @@ -1171,6 +1193,7 @@ static inline mbedtls_x509_crt *mbedtls_ssl_own_cert( mbedtls_ssl_context *ssl ) * * Return 0 if everything is OK, -1 if not. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_check_cert_usage( const mbedtls_x509_crt *cert, const mbedtls_ssl_ciphersuite_t *ciphersuite, int cert_endpoint, @@ -1219,21 +1242,26 @@ static inline size_t mbedtls_ssl_hs_hdr_len( const mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_PROTO_DTLS) void mbedtls_ssl_send_flight_completed( mbedtls_ssl_context *ssl ); void mbedtls_ssl_recv_flight_completed( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_resend( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ); #endif /* Visible for testing purposes only */ #if defined(MBEDTLS_SSL_DTLS_ANTI_REPLAY) +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_dtls_replay_check( mbedtls_ssl_context const *ssl ); void mbedtls_ssl_dtls_replay_update( mbedtls_ssl_context *ssl ); #endif +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_session_copy( mbedtls_ssl_session *dst, const mbedtls_ssl_session *src ); #if defined(MBEDTLS_SSL_PROTO_SSL3) || defined(MBEDTLS_SSL_PROTO_TLS1) || \ defined(MBEDTLS_SSL_PROTO_TLS1_1) +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_get_key_exchange_md_ssl_tls( mbedtls_ssl_context *ssl, unsigned char *output, unsigned char *data, size_t data_len ); @@ -1243,6 +1271,7 @@ int mbedtls_ssl_get_key_exchange_md_ssl_tls( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_SSL_PROTO_TLS1) || defined(MBEDTLS_SSL_PROTO_TLS1_1) || \ defined(MBEDTLS_SSL_PROTO_TLS1_2) /* The hash buffer must have at least MBEDTLS_MD_MAX_SIZE bytes of length. */ +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_get_key_exchange_md_tls1_2( mbedtls_ssl_context *ssl, unsigned char *hash, size_t *hashlen, unsigned char *data, size_t data_len, @@ -1255,11 +1284,13 @@ int mbedtls_ssl_get_key_exchange_md_tls1_2( mbedtls_ssl_context *ssl, #endif void mbedtls_ssl_transform_init( mbedtls_ssl_transform *transform ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_encrypt_buf( mbedtls_ssl_context *ssl, mbedtls_ssl_transform *transform, mbedtls_record *rec, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_decrypt_buf( mbedtls_ssl_context const *ssl, mbedtls_ssl_transform *transform, mbedtls_record *rec ); @@ -1277,10 +1308,12 @@ static inline size_t mbedtls_ssl_ep_len( const mbedtls_ssl_context *ssl ) } #if defined(MBEDTLS_SSL_PROTO_DTLS) +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_resend_hello_request( mbedtls_ssl_context *ssl ); #endif /* MBEDTLS_SSL_PROTO_DTLS */ void mbedtls_ssl_set_timer( mbedtls_ssl_context *ssl, uint32_t millisecs ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_check_timer( mbedtls_ssl_context *ssl ); void mbedtls_ssl_reset_in_out_pointers( mbedtls_ssl_context *ssl ); @@ -1288,6 +1321,7 @@ void mbedtls_ssl_update_out_pointers( mbedtls_ssl_context *ssl, mbedtls_ssl_transform *transform ); void mbedtls_ssl_update_in_pointers( mbedtls_ssl_context *ssl ); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_session_reset_int( mbedtls_ssl_context *ssl, int partial ); #if defined(MBEDTLS_SSL_DTLS_ANTI_REPLAY) @@ -1297,6 +1331,7 @@ void mbedtls_ssl_dtls_replay_reset( mbedtls_ssl_context *ssl ); void mbedtls_ssl_handshake_wrapup_free_hs_transform( mbedtls_ssl_context *ssl ); #if defined(MBEDTLS_SSL_RENEGOTIATION) +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_start_renegotiation( mbedtls_ssl_context *ssl ); #endif /* MBEDTLS_SSL_RENEGOTIATION */ From ed36d20ea6423fcc545da200bf5a69d8542366c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 23 Jun 2022 09:43:39 +0200 Subject: [PATCH 20/20] Save code size by calling get_type only once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is an external function, so in the absence of link-time optimisation (LTO) the compiler can't know anything about it and has to call it the number of times it's called in the source code. This only matters for pk_ec, but change pk_rsa as well for the sake of uniformity. Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/pk.h | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h index 017557a83..c9a13f484 100644 --- a/include/mbedtls/pk.h +++ b/include/mbedtls/pk.h @@ -643,9 +643,13 @@ mbedtls_pk_type_t mbedtls_pk_get_type( const mbedtls_pk_context *ctx ); */ static inline mbedtls_rsa_context *mbedtls_pk_rsa( const mbedtls_pk_context pk ) { - return( mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_RSA ? - (mbedtls_rsa_context *) (pk).pk_ctx : - NULL ); + switch( mbedtls_pk_get_type( &pk ) ) + { + case MBEDTLS_PK_RSA: + return( (mbedtls_rsa_context *) (pk).pk_ctx ); + default: + return( NULL ); + } } #endif /* MBEDTLS_RSA_C */ @@ -663,11 +667,15 @@ static inline mbedtls_rsa_context *mbedtls_pk_rsa( const mbedtls_pk_context pk ) */ static inline mbedtls_ecp_keypair *mbedtls_pk_ec( const mbedtls_pk_context pk ) { - return( mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_ECKEY || - mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_ECKEY_DH || - mbedtls_pk_get_type( &pk ) == MBEDTLS_PK_ECDSA ? - (mbedtls_ecp_keypair *) (pk).pk_ctx : - NULL ); + switch( mbedtls_pk_get_type( &pk ) ) + { + case MBEDTLS_PK_ECKEY: + case MBEDTLS_PK_ECKEY_DH: + case MBEDTLS_PK_ECDSA: + return( (mbedtls_ecp_keypair *) (pk).pk_ctx ); + default: + return( NULL ); + } } #endif /* MBEDTLS_ECP_C */