From cb1cc80cf5d7257e5fc0071114778257e1bde352 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Sat, 17 Nov 2018 22:27:38 +0000 Subject: [PATCH] Use PSA-based ciphers for record protections in TLS-1.2 only Reasons: - For the first release, we attempt to support TLS-1.2 only, - At least TLS-1.0 is known to not work at the moment, as for CBC ciphersuites the code in mbedtls_ssl_decrypt_buf() and mbedtls_ssl_encrypt_buf() assumes that mbedtls_cipher_crypt() updates the structure field for the IV in the cipher context, which the PSA-based implementation currently doesn't. --- library/ssl_tls.c | 85 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 64 insertions(+), 21 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index acfb3de82..e6a4222a2 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -610,6 +610,9 @@ static void ssl_calc_finished_tls_sha384( mbedtls_ssl_context *, unsigned char * int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) { int ret = 0; +#if defined(MBEDTLS_USE_PSA_CRYPTO) + int psa_fallthrough; +#endif /* MBEDTLS_USE_PSA_CRYPTO */ unsigned char tmp[64]; unsigned char keyblk[256]; unsigned char *key1; @@ -1032,20 +1035,41 @@ int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) #endif #if defined(MBEDTLS_USE_PSA_CRYPTO) - ret = mbedtls_cipher_setup_psa( &transform->cipher_ctx_enc, - cipher_info, taglen ); - if( ret != 0 && ret != MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE ) + + /* Only use PSA-based ciphers for TLS-1.2. + * That's relevant at least for TLS-1.0, where + * we assume that mbedtls_cipher_crypt() updates + * the structure field for the IV, which the PSA-based + * implementation currently doesn't. */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_cipher_setup_psa", ret ); - return( ret ); + ret = mbedtls_cipher_setup_psa( &transform->cipher_ctx_enc, + cipher_info, taglen ); + if( ret != 0 && ret != MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_cipher_setup_psa", ret ); + return( ret ); + } + + if( ret == 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Successfully setup PSA-based encryption cipher context" ) ); + psa_fallthrough = 0; + } + else + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Failed to setup PSA-based cipher context for record encryption - fall through to default setup." ) ); + psa_fallthrough = 1; + } } - - if( ret == 0 ) - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Successfully setup PSA-based encryption cipher context" ) ); else - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Failed to setup PSA-based cipher context for record encryption - fall through to default setup." ) ); + psa_fallthrough = 1; +#else + psa_fallthrough = 1; +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ - if( ret != 0 ) + if( psa_fallthrough == 1 ) #endif /* MBEDTLS_USE_PSA_CRYPTO */ if( ( ret = mbedtls_cipher_setup( &transform->cipher_ctx_enc, cipher_info ) ) != 0 ) @@ -1055,21 +1079,40 @@ int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) } #if defined(MBEDTLS_USE_PSA_CRYPTO) - ret = mbedtls_cipher_setup_psa( &transform->cipher_ctx_dec, - cipher_info, taglen ); - - if( ret != 0 && ret != MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE ) + /* Only use PSA-based ciphers for TLS-1.2. + * That's relevant at least for TLS-1.0, where + * we assume that mbedtls_cipher_crypt() updates + * the structure field for the IV, which the PSA-based + * implementation currently doesn't. */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) + if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_cipher_setup_psa", ret ); - return( ret ); + ret = mbedtls_cipher_setup_psa( &transform->cipher_ctx_dec, + cipher_info, taglen ); + if( ret != 0 && ret != MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_cipher_setup_psa", ret ); + return( ret ); + } + + if( ret == 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Successfully setup PSA-based decryption cipher context" ) ); + psa_fallthrough = 0; + } + else + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Failed to setup PSA-based cipher context for record decryption - fall through to default setup." ) ); + psa_fallthrough = 1; + } } - - if( ret == 0 ) - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Successfully setup PSA-based decryption cipher context" ) ); else - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Failed to setup PSA-based cipher context for record decryption - fall through to default setup." ) ); + psa_fallthrough = 1; +#else + psa_fallthrough = 1; +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ - if( ret != 0 ) + if( psa_fallthrough == 1 ) #endif /* MBEDTLS_USE_PSA_CRYPTO */ if( ( ret = mbedtls_cipher_setup( &transform->cipher_ctx_dec, cipher_info ) ) != 0 )