From 82f0a9a1dbd7761b5e600a0cc1bbe064cac0ea9a Mon Sep 17 00:00:00 2001 From: Xiaofei Bai Date: Wed, 26 Jan 2022 09:21:54 +0000 Subject: [PATCH] Rebase and address review comments Signed-off-by: Xiaofei Bai --- library/ssl_misc.h | 11 ++-- library/ssl_tls13_client.c | 71 ++++++++++----------- library/ssl_tls13_generic.c | 124 +++++++----------------------------- 3 files changed, 59 insertions(+), 147 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index a8ac52c9f..093732e03 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -267,6 +267,10 @@ /* Maximum size in bytes of list in supported elliptic curve ext., RFC 4492 */ #define MBEDTLS_SSL_MAX_CURVE_LIST_LEN 65535 +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) +#define MBEDTLS_SIG_ALGS_SIZE 20 +#endif + /* * Check that we obey the standard's message size bounds */ @@ -556,13 +560,6 @@ struct mbedtls_ssl_handshake_params #if defined(MBEDTLS_SSL_PROTO_DTLS) unsigned char retransmit_state; /*!< Retransmission state */ #endif - /* - * Handshake specific crypto variables - */ -#if defined(MBEDTLS_X509_CRT_PARSE_C) - uint16_t sig_algs[MBEDTLS_PK_SIGNATURE_MAX_SIZE]; - /*!< Received signature algorithms */ -#endif /* MBEDTLS_X509_CRT_PARSE_C */ #if !defined(MBEDTLS_DEPRECATED_REMOVED) unsigned char group_list_heap_allocated; diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index a47a3c41a..b3272c90f 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1413,9 +1413,6 @@ static int ssl_tls13_postprocess_encrypted_extensions( mbedtls_ssl_context *ssl * - SSL_CERTIFICATE_REQUEST_SKIP * indicating if a Certificate Request is expected or not. */ -/* - * Implementation - */ static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -1433,15 +1430,8 @@ static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) } ssl->keep_current_message = 1; - if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) ); - MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE, - MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); - return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); - } - - if( ssl->in_msg[0] == MBEDTLS_SSL_HS_CERTIFICATE_REQUEST ) + if( ( ssl->in_msgtype == MBEDTLS_SSL_MSG_HANDSHAKE ) && + ( ssl->in_msg[0] == MBEDTLS_SSL_HS_CERTIFICATE_REQUEST ) ) { return( SSL_CERTIFICATE_REQUEST_EXPECT_REQUEST ); } @@ -1463,23 +1453,25 @@ static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; const unsigned char *p = buf; - const unsigned char *extensions_end; - size_t extensions_len = 0; size_t certificate_request_context_len = 0; - uint32_t n_sig_alg_ext = 0; - mbedtls_ssl_handshake_params *handshake = ssl->handshake; + size_t extensions_len = 0; + const unsigned char *extensions_end; + unsigned char sig_alg_ext_found = 0; - /* Parse certificate_request_context */ + /* ... + * opaque certificate_request_context<0..2^8-1> + * ... + */ MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); certificate_request_context_len = (size_t) p[0]; p += 1; - /* store context ( if necessary ) */ if( certificate_request_context_len > 0 ) { MBEDTLS_SSL_DEBUG_BUF( 3, "Certificate Request Context", p, certificate_request_context_len ); + mbedtls_ssl_handshake_params *handshake = ssl->handshake; handshake->certificate_request_context = mbedtls_calloc( 1, certificate_request_context_len); if( handshake->certificate_request_context == NULL ) @@ -1493,8 +1485,9 @@ static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, p += certificate_request_context_len; } - /* - * Parse extensions + /* ... + * Extension extensions<2..2^16-1>; + * ... */ MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); extensions_len = MBEDTLS_GET_UINT16_BE( p, 0 ); @@ -1524,12 +1517,14 @@ static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, p + extension_data_len ); if( ret != 0 ) return( ret ); - n_sig_alg_ext += 1; - break; - - case MBEDTLS_TLS_EXT_STATUS_REQUEST: - MBEDTLS_SSL_DEBUG_MSG( 3, - ( "found status request extension ( ignoring )" ) ); + if( ! sig_alg_ext_found ) + sig_alg_ext_found = 1; + else + { + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "Duplicate signature algorithms extensions found" ) ); + goto error; + } break; default: @@ -1537,10 +1532,7 @@ static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, 3, ( "unknown extension found: %u ( ignoring )", extension_type ) ); - MBEDTLS_SSL_PEND_FATAL_ALERT( - MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_EXT, - MBEDTLS_ERR_SSL_UNSUPPORTED_EXTENSION ); - return( MBEDTLS_ERR_SSL_UNSUPPORTED_EXTENSION ); + break; } p += extension_data_len; } @@ -1549,23 +1541,24 @@ static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, { MBEDTLS_SSL_DEBUG_MSG( 1, ( "Signature algorithms extension length misaligned" ) ); - MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, - MBEDTLS_ERR_SSL_DECODE_ERROR ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + goto error; } /* Check that we found signature algorithms extension */ - if( n_sig_alg_ext == 0 ) + if( ! sig_alg_ext_found ) { MBEDTLS_SSL_DEBUG_MSG( 3, - ( "no signature algorithms extension was found" ) ); - MBEDTLS_SSL_PEND_FATAL_ALERT( - MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_EXT, - MBEDTLS_ERR_SSL_UNSUPPORTED_EXTENSION ); - return( MBEDTLS_ERR_SSL_UNSUPPORTED_EXTENSION ); + ( "no signature algorithms extension found" ) ); + goto error; } ssl->client_auth = 1; return( 0 ); + +error: + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, + MBEDTLS_ERR_SSL_DECODE_ERROR ); + mbedtls_free( ssl->handshake->certificate_request_context ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); } /* diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index aeefdc77f..2f4d04e10 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -136,90 +136,6 @@ void mbedtls_ssl_tls13_add_hs_hdr_to_checksum( mbedtls_ssl_context *ssl, } #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) - -/* - * mbedtls_ssl_tls13_write_sig_alg_ext( ) - * - * enum { - * .... - * ecdsa_secp256r1_sha256( 0x0403 ), - * ecdsa_secp384r1_sha384( 0x0503 ), - * ecdsa_secp521r1_sha512( 0x0603 ), - * .... - * } SignatureScheme; - * - * struct { - * SignatureScheme supported_signature_algorithms<2..2^16-2>; - * } SignatureSchemeList; - * - * Only if we handle at least one key exchange that needs signatures. - */ -int mbedtls_ssl_tls13_write_sig_alg_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, - unsigned char *end, - size_t *out_len ) -{ - unsigned char *p = buf; - unsigned char *supported_sig_alg; /* Start of supported_signature_algorithms */ - size_t supported_sig_alg_len = 0; /* Length of supported_signature_algorithms */ - - *out_len = 0; - - /* Skip the extension on the client if all allowed key exchanges - * are PSK-based. */ -#if defined(MBEDTLS_SSL_CLI_C) - if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT && - !mbedtls_ssl_conf_tls13_some_ephemeral_enabled( ssl ) ) - { - return( 0 ); - } -#endif /* MBEDTLS_SSL_CLI_C */ - - MBEDTLS_SSL_DEBUG_MSG( 3, ( "adding signature_algorithms extension" ) ); - - /* Check if we have space for header and length field: - * - extension_type (2 bytes) - * - extension_data_length (2 bytes) - * - supported_signature_algorithms_length (2 bytes) - */ - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 6 ); - p += 6; - - /* - * Write supported_signature_algorithms - */ - supported_sig_alg = p; - for( const uint16_t *sig_alg = ssl->conf->tls13_sig_algs; - *sig_alg != MBEDTLS_TLS1_3_SIG_NONE; sig_alg++ ) - { - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); - MBEDTLS_PUT_UINT16_BE( *sig_alg, p, 0 ); - p += 2; - MBEDTLS_SSL_DEBUG_MSG( 3, ( "signature scheme [%x]", *sig_alg ) ); - } - - /* Length of supported_signature_algorithms */ - supported_sig_alg_len = p - supported_sig_alg; - if( supported_sig_alg_len == 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "No signature algorithms defined." ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); - } - - /* Write extension_type */ - MBEDTLS_PUT_UINT16_BE( MBEDTLS_TLS_EXT_SIG_ALG, buf, 0 ); - /* Write extension_data_length */ - MBEDTLS_PUT_UINT16_BE( supported_sig_alg_len + 2, buf, 2 ); - /* Write length of supported_signature_algorithms */ - MBEDTLS_PUT_UINT16_BE( supported_sig_alg_len, buf, 4 ); - - /* Output the total length of signature algorithms extension. */ - *out_len = p - buf; - - ssl->handshake->extensions_present |= MBEDTLS_SSL_EXT_SIG_ALG; - return( 0 ); -} - /* mbedtls_ssl_tls13_parse_sig_alg_ext() * * enum { @@ -239,34 +155,37 @@ int mbedtls_ssl_tls13_parse_sig_alg_ext( mbedtls_ssl_context *ssl, const unsigned char *end ) { const unsigned char *p = buf; - const uint16_t *sig_alg; - unsigned int signature_scheme; /* store received signature algorithm scheme */ + size_t sig_algs_len = 0; + uint16_t *sig_algs; + const unsigned char *sig_algs_end; + uint16_t sig_alg; uint32_t common_idx = 0; /* iterate through received signature schemes list */ /* skip 2 bytes of signature algorithms length */ MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); + sig_algs_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; - memset( ssl->handshake->sig_algs, 0, sizeof( ssl->handshake->sig_algs ) ); + sig_algs = mbedtls_calloc( MBEDTLS_SIG_ALGS_SIZE, sizeof( uint16_t ) ); + if( sig_algs == NULL ) + return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); - while( p < end && common_idx + 1 < MBEDTLS_PK_SIGNATURE_MAX_SIZE ) + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, sig_algs_len ); + sig_algs_end = p + sig_algs_len; + while( p < sig_algs_end && common_idx + 1 < MBEDTLS_SIG_ALGS_SIZE ) { - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); - signature_scheme = MBEDTLS_GET_UINT16_BE( p, 0 ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, sig_algs_end, 2 ); + sig_alg = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; MBEDTLS_SSL_DEBUG_MSG( 4, ( "received signature algorithm: 0x%x", - signature_scheme ) ); + sig_alg ) ); - for( sig_alg = ssl->conf->tls13_sig_algs; - *sig_alg != MBEDTLS_TLS1_3_SIG_NONE; sig_alg++ ) + if( mbedtls_ssl_sig_alg_is_offered( ssl, sig_alg ) && + mbedtls_ssl_sig_alg_is_supported( ssl, sig_alg ) ) { - if( *sig_alg == signature_scheme ) - { - ssl->handshake->sig_algs[common_idx] = signature_scheme; - common_idx++; - break; - } + sig_algs[common_idx] = sig_alg; + common_idx += 1; } } /* Check that we consumed all the message. */ @@ -276,6 +195,7 @@ int mbedtls_ssl_tls13_parse_sig_alg_ext( mbedtls_ssl_context *ssl, ( "Signature algorithms extension length misaligned" ) ); MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, MBEDTLS_ERR_SSL_DECODE_ERROR ); + mbedtls_free( sig_algs ); return( MBEDTLS_ERR_SSL_DECODE_ERROR ); } @@ -284,11 +204,13 @@ int mbedtls_ssl_tls13_parse_sig_alg_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "no signature algorithm in common" ) ); MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE, MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + mbedtls_free( sig_algs ); return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); } - ssl->handshake->sig_algs[common_idx] = MBEDTLS_TLS1_3_SIG_NONE; - + sig_algs[common_idx] = MBEDTLS_TLS1_3_SIG_NONE; + ssl->handshake->sig_algs = sig_algs; + ssl->handshake->sig_algs_heap_allocated = 1; return( 0 ); }