From e1e344213a241bfab90af7bec151d48100b3c475 Mon Sep 17 00:00:00 2001 From: Xiaofei Bai Date: Thu, 23 Dec 2021 12:09:05 +0000 Subject: [PATCH 01/12] Add TLS1.3 process certificate request Signed-off-by: Xiaofei Bai --- library/ssl_misc.h | 18 ++ library/ssl_tls13_client.c | 7 +- library/ssl_tls13_generic.c | 379 +++++++++++++++++++++++++++++++++++- 3 files changed, 397 insertions(+), 7 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 788fafdd92..1db6202520 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -556,6 +556,13 @@ 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) + int recv_sig_schemes_list[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; @@ -802,6 +809,12 @@ struct mbedtls_ssl_handshake_params represents an extension and defined as \c MBEDTLS_SSL_EXT_XXX */ +#if defined(MBEDTLS_ECDSA_C) + unsigned char cert_req_ctx_len; /*!< certificate request context + length */ + unsigned char* cert_req_ctx; /*!< certificate request context */ +#endif + union { unsigned char early [MBEDTLS_TLS1_3_MD_MAX_SIZE]; @@ -1687,6 +1700,11 @@ int mbedtls_ssl_tls13_start_handshake_msg( mbedtls_ssl_context *ssl, unsigned char **buf, size_t *buf_len ); +/* + * Handler of TLS 1.3 server certificate request message + */ +int mbedtls_ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ); + /* * Handler of TLS 1.3 server certificate message */ diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index d046495cb7..83ee98fbde 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1415,10 +1415,9 @@ static int ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ) if( ( ssl->in_msgtype == MBEDTLS_SSL_MSG_HANDSHAKE ) && ( ssl->in_msg[0] == MBEDTLS_SSL_HS_CERTIFICATE_REQUEST ) ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "CertificateRequest not supported" ) ); - MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE, - MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); - return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + ret = mbedtls_ssl_tls13_process_certificate_request( ssl ); + if( ret != 0 ) + return( ret ); } ssl->keep_current_message = 1; diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index a87af94dcc..90c20bde92 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -136,6 +136,379 @@ 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 ); +} + +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) +static int ssl_tls13_parse_signature_algorithms_ext( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) +{ + size_t sig_alg_list_size; /* size of receive signature algorithms list */ + const unsigned char *p; /* pointer to individual signature algorithm */ + const uint16_t *sig_alg; /* iterate through configured signature schemes */ + unsigned int signature_scheme; /* store received signature algorithm scheme */ + uint32_t common_idx = 0; /* iterate through received_signature_schemes_list */ + size_t buf_len = end - buf; + + MBEDTLS_SSL_CHK_BUF_READ_PTR( buf, end, 2 ); + + sig_alg_list_size = MBEDTLS_GET_UINT16_BE( buf, 0 ); + if( sig_alg_list_size + 2 != buf_len || + sig_alg_list_size % 2 != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad signature_algorithms extension" ) ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } + memset( ssl->handshake->recv_sig_schemes_list, 0, + sizeof( ssl->handshake->recv_sig_schemes_list ) ); + + for( p = buf + 2; p < end && common_idx + 1 < MBEDTLS_PK_SIGNATURE_MAX_SIZE; p += 2 ) + { + signature_scheme = MBEDTLS_GET_UINT16_BE( p, 0 ); + + MBEDTLS_SSL_DEBUG_MSG( 4, ( "received signature algorithm: 0x%x", + signature_scheme ) ); + + for( sig_alg = ssl->conf->tls13_sig_algs; + *sig_alg != MBEDTLS_TLS1_3_SIG_NONE; sig_alg++ ) + { + if( *sig_alg == signature_scheme ) + { + ssl->handshake->recv_sig_schemes_list[common_idx] = signature_scheme; + common_idx++; + break; + } + } + } + + if( common_idx == 0 ) + { + 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 ); + return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + } + + ssl->handshake->recv_sig_schemes_list[common_idx] = MBEDTLS_TLS1_3_SIG_NONE; + + return( 0 ); +} +#endif + +/* + * + * STATE HANDLING: CertificateRequest + * + */ + +/* + * Overview + */ + +/* Coordination: + * Deals with the ambiguity of not knowing if a CertificateRequest + * will be sent. Returns a negative code on failure, or + * - SSL_CERTIFICATE_REQUEST_EXPECT_REQUEST + * - SSL_CERTIFICATE_REQUEST_SKIP + * indicating if a Certificate Request is expected or not. + */ +#define SSL_CERTIFICATE_REQUEST_EXPECT_REQUEST 0 +#define SSL_CERTIFICATE_REQUEST_SKIP 1 + +/* + * Implementation + */ + +static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) +{ + int ret; + + if( mbedtls_ssl_tls13_psk_enabled( ssl ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "<= skip parse certificate request" ) ); + return( SSL_CERTIFICATE_REQUEST_SKIP ); + } + + if( ( ret = mbedtls_ssl_read_record( ssl, 0 ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret ); + return( ret ); + } + 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 ); + } + + return( SSL_CERTIFICATE_REQUEST_SKIP ); +} + +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) +static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + const unsigned char *p = buf; + const unsigned char *ext = NULL; + size_t ext_len = 0; + size_t context_len = 0; + + + /* + * struct { + * opaque certificate_request_context<0..2^8-1>; + * Extension extensions<2..2^16-1>; + * } CertificateRequest; + */ + + /* + * Parse certificate_request_context + */ + + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); + context_len = (size_t) p[0]; + /* skip context_len */ + p++; + + /* Fixed length fields are: + * - 1 for length of context + * - 2 for length of extensions + * ----- + * 3 bytes + */ + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 3 ); + + /* store context ( if necessary ) */ + if( context_len > 0 ) + { + MBEDTLS_SSL_DEBUG_BUF( 3, "Certificate Request Context", + p, context_len ); + + ssl->handshake->cert_req_ctx = mbedtls_calloc( context_len, 1 ); + if( ssl->handshake->cert_req_ctx == NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); + return ( MBEDTLS_ERR_SSL_ALLOC_FAILED ); + } + memcpy( ssl->handshake->cert_req_ctx, p, context_len ); + + /* jump over certificate_request_context */ + p += context_len; + } + + /* + * Parse extensions + */ + ext_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + + /* At least one extension needs to be present, + * namely signature_algorithms ext. */ + if( ext_len < 4 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, + MBEDTLS_ERR_SSL_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } + + /* skip total extension length */ + p += 2; + + ext = p; /* jump to extensions */ + while( ext_len ) + { + size_t ext_id = MBEDTLS_GET_UINT16_BE( ext, 0 ); + size_t ext_size = MBEDTLS_GET_UINT16_BE( ext, 1 ); + + if( ext_size + 4 > ext_len ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, + MBEDTLS_ERR_SSL_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } + + switch( ext_id ) + { + case MBEDTLS_TLS_EXT_SIG_ALG: + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "found signature_algorithms extension" ) ); + + if( ( ret = ssl_tls13_parse_signature_algorithms_ext( ssl, + ext + 4, ext + 4 + ext_size ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, + "ssl_tls13_parse_signature_algorithms_ext", ret ); + MBEDTLS_SSL_PEND_FATAL_ALERT( + MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, ret ); + return( ret ); + } + break; + + default: + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "unknown extension found: %" MBEDTLS_PRINTF_SIZET " ( ignoring )", + ext_id ) ); + break; + } + + ext_len -= 4 + ext_size; + ext += 4 + ext_size; + + if( ( ext_len > 0 ) && ( ext_len < 4 ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, + MBEDTLS_ERR_SSL_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } + } + + ssl->client_auth = 1; + return( 0 ); +} +#endif /* ( MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED ) */ + +/* Main entry point; orchestrates the other functions */ +int mbedtls_ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ) +{ + int ret = 0; + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse certificate request" ) ); + + /* Coordination step + * - Fetch record + * - Make sure it's either a CertificateRequest or a ServerHelloDone + */ + MBEDTLS_SSL_PROC_CHK_NEG( ssl_tls13_certificate_request_coordinate( ssl ) ); + +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) + if( ret == SSL_CERTIFICATE_REQUEST_EXPECT_REQUEST ) + { + unsigned char *buf; + size_t buf_len; + + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_fetch_handshake_msg( ssl, + MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, + &buf, &buf_len ) ); + + MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_certificate_request( ssl, + buf, buf + buf_len ) ); + + mbedtls_ssl_tls13_add_hs_msg_to_checksum( + ssl, MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, buf, buf_len ); + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ + if( ret == SSL_CERTIFICATE_REQUEST_SKIP ) + { + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate request" ) ); + } + else + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + + MBEDTLS_SSL_DEBUG_MSG( 3, ( "got %s certificate request", + ssl->client_auth ? "a" : "no" ) ); + +cleanup: + + /* In the MPS one would close the read-port here to + * ensure there's no overlap of reading and writing. */ + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse certificate request" ) ); + return( ret ); +} + /* * STATE HANDLING: Read CertificateVerify */ @@ -487,20 +860,20 @@ static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, const unsigned char *end ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - size_t certificate_request_context_len = 0; + size_t cert_req_ctx_len = 0; size_t certificate_list_len = 0; const unsigned char *p = buf; const unsigned char *certificate_list_end; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 4 ); - certificate_request_context_len = p[0]; + cert_req_ctx_len = p[0]; certificate_list_len = MBEDTLS_GET_UINT24_BE( p, 1 ); p += 4; /* In theory, the certificate list can be up to 2^24 Bytes, but we don't * support anything beyond 2^16 = 64K. */ - if( ( certificate_request_context_len != 0 ) || + if( ( cert_req_ctx_len != 0 ) || ( certificate_list_len >= 0x10000 ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate message" ) ); From 5d8598e09030e099a3757140fbc855fa621072ae Mon Sep 17 00:00:00 2001 From: Xiaofei Bai Date: Tue, 11 Jan 2022 05:56:06 +0000 Subject: [PATCH 02/12] update certificate request tests Signed-off-by: Xiaofei Bai --- tests/ssl-opt.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 7a1436fab9..0fabae907e 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -9193,7 +9193,8 @@ run_test "TLS 1.3: CertificateRequest check - openssl" \ "$O_NEXT_SRV -msg -tls1_3 -num_tickets 0 -no_resume_ephemeral -no_cache -Verify 10" \ "$P_CLI debug_level=4 force_version=tls13 " \ 1 \ - -c "CertificateRequest not supported" + -c "=> parse certificate request" \ + -c "<= parse certificate request" requires_gnutls_tls1_3 requires_gnutls_next_no_ticket @@ -9206,7 +9207,8 @@ run_test "TLS 1.3: CertificateRequest check - gnutls" \ "$G_NEXT_SRV --debug=4 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:+CIPHER-ALL:%NO_TICKETS" \ "$P_CLI debug_level=3 min_version=tls13 max_version=tls13" \ 1 \ - -c "CertificateRequest not supported" + -c "=> parse certificate request" \ + -c "<= parse certificate request" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE From a0ab777cfcd0993fd5b773f623ced7968f9740ee Mon Sep 17 00:00:00 2001 From: Xiaofei Bai Date: Sun, 16 Jan 2022 12:14:45 +0000 Subject: [PATCH 03/12] update based on comments. Signed-off-by: Xiaofei Bai --- library/ssl_misc.h | 7 +- library/ssl_tls13_client.c | 276 ++++++++++++++++++++++++++++++++- library/ssl_tls13_generic.c | 295 +----------------------------------- 3 files changed, 275 insertions(+), 303 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 1db6202520..c2ce07773b 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -809,10 +809,9 @@ struct mbedtls_ssl_handshake_params represents an extension and defined as \c MBEDTLS_SSL_EXT_XXX */ -#if defined(MBEDTLS_ECDSA_C) - unsigned char cert_req_ctx_len; /*!< certificate request context - length */ - unsigned char* cert_req_ctx; /*!< certificate request context */ +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) + unsigned char certificate_request_context_len; + unsigned char *certificate_request_context; #endif union diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 83ee98fbde..23b067079f 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1398,6 +1398,272 @@ static int ssl_tls13_postprocess_encrypted_extensions( mbedtls_ssl_context *ssl return( 0 ); } +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) +static int ssl_tls13_parse_signature_algorithms_ext( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) +{ + size_t sig_alg_list_size; /* size of receive signature algorithms list */ + const unsigned char *p; /* pointer to individual signature algorithm */ + const uint16_t *sig_alg; /* iterate through configured signature schemes */ + unsigned int signature_scheme; /* store received signature algorithm scheme */ + uint32_t common_idx = 0; /* iterate through received_signature_schemes_list */ + size_t buf_len = end - buf; + + MBEDTLS_SSL_CHK_BUF_READ_PTR( buf, end, 2 ); + + sig_alg_list_size = MBEDTLS_GET_UINT16_BE( buf, 0 ); + if( sig_alg_list_size + 2 != buf_len || + sig_alg_list_size % 2 != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad signature_algorithms extension" ) ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } + memset( ssl->handshake->recv_sig_schemes_list, 0, + sizeof( ssl->handshake->recv_sig_schemes_list ) ); + + for( p = buf + 2; p < end && common_idx + 1 < MBEDTLS_PK_SIGNATURE_MAX_SIZE; p += 2 ) + { + signature_scheme = MBEDTLS_GET_UINT16_BE( p, 0 ); + + MBEDTLS_SSL_DEBUG_MSG( 4, ( "received signature algorithm: 0x%x", + signature_scheme ) ); + + for( sig_alg = ssl->conf->tls13_sig_algs; + *sig_alg != MBEDTLS_TLS1_3_SIG_NONE; sig_alg++ ) + { + if( *sig_alg == signature_scheme ) + { + ssl->handshake->recv_sig_schemes_list[common_idx] = signature_scheme; + common_idx++; + break; + } + } + } + + if( common_idx == 0 ) + { + 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 ); + return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + } + + ssl->handshake->recv_sig_schemes_list[common_idx] = MBEDTLS_TLS1_3_SIG_NONE; + + return( 0 ); +} +#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ + +/* + * + * STATE HANDLING: CertificateRequest + * + */ + +/* + * Overview + */ + +/* Coordination: + * Deals with the ambiguity of not knowing if a CertificateRequest + * will be sent. Returns a negative code on failure, or + * - SSL_CERTIFICATE_REQUEST_EXPECT_REQUEST + * - SSL_CERTIFICATE_REQUEST_SKIP + * indicating if a Certificate Request is expected or not. + */ +#define SSL_CERTIFICATE_REQUEST_EXPECT_REQUEST 0 +#define SSL_CERTIFICATE_REQUEST_SKIP 1 + +/* + * Implementation + */ + +static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) +{ + int ret; + + if( mbedtls_ssl_tls13_psk_enabled( ssl ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "<= skip parse certificate request" ) ); + return( SSL_CERTIFICATE_REQUEST_SKIP ); + } + + if( ( ret = mbedtls_ssl_read_record( ssl, 0 ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret ); + return( ret ); + } + 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 ) + { + return( SSL_CERTIFICATE_REQUEST_EXPECT_REQUEST ); + } + + return( SSL_CERTIFICATE_REQUEST_SKIP ); +} + +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) +/* + * ssl_tls13_parse_certificate_request() + * Parse certificate request + * struct { + * opaque certificate_request_context<0..2^8-1>; + * Extension extensions<2..2^16-1>; + * } CertificateRequest; + */ +static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) +{ + 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; + + /* Parse certificate_request_context */ + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); + certificate_request_context_len = (size_t) p[0]; + p++; + + /* store context ( if necessary ) */ + if( certificate_request_context_len > 0 ) + { + MBEDTLS_SSL_DEBUG_BUF( 3, "Certificate Request Context", + p, certificate_request_context_len ); + + ssl->handshake->certificate_request_context = + mbedtls_calloc( certificate_request_context_len, 1 ); + if( ssl->handshake->certificate_request_context == NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); + return ( MBEDTLS_ERR_SSL_ALLOC_FAILED ); + } + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, certificate_request_context_len ); + memcpy( ssl->handshake->certificate_request_context, p, + certificate_request_context_len ); + + /* jump over certificate_request_context */ + p += certificate_request_context_len; + } + + /* + * Parse extensions + */ + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); + extensions_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + p += 2; + + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, extensions_len ); + extensions_end = p + extensions_len; + + while( p < extensions_end ) + { + unsigned int extension_type; + size_t extension_data_len; + + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, extensions_end, 4 ); + extension_type = MBEDTLS_GET_UINT16_BE( p, 0 ); + extension_data_len = MBEDTLS_GET_UINT16_BE( p, 2 ); + p += 4; + + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, extensions_end, extension_data_len ); + + switch( extension_type ) + { + case MBEDTLS_TLS_EXT_SIG_ALG: + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "found signature_algorithms extension" ) ); + + ret = ssl_tls13_parse_signature_algorithms_ext( ssl, p, + p + extension_data_len ); + if( ret != 0 ) + return( ret ); + break; + + default: + MBEDTLS_SSL_DEBUG_MSG( + 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 ); + } + + p += extension_data_len; + } + + ssl->client_auth = 1; + return( 0 ); +} +#endif /* ( MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED ) */ + +/* Main entry point; orchestrates the other functions */ +int mbedtls_ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ) +{ + int ret = 0; + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse certificate request" ) ); + + /* Coordination step + * - Fetch record + * - Make sure it's either a CertificateRequest or a ServerHelloDone + */ + MBEDTLS_SSL_PROC_CHK_NEG( ssl_tls13_certificate_request_coordinate( ssl ) ); + +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) + if( ret == SSL_CERTIFICATE_REQUEST_EXPECT_REQUEST ) + { + unsigned char *buf; + size_t buf_len; + + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_fetch_handshake_msg( ssl, + MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, + &buf, &buf_len ) ); + + MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_certificate_request( ssl, + buf, buf + buf_len ) ); + + mbedtls_ssl_tls13_add_hs_msg_to_checksum( + ssl, MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, buf, buf_len ); + } + else +#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ + if( ret == SSL_CERTIFICATE_REQUEST_SKIP ) + { + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate request" ) ); + } + else + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + + MBEDTLS_SSL_DEBUG_MSG( 3, ( "got %s certificate request", + ssl->client_auth ? "a" : "no" ) ); + +cleanup: + + /* In the MPS one would close the read-port here to + * ensure there's no overlap of reading and writing. */ + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse certificate request" ) ); + return( ret ); +} + #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) /* * Handler for MBEDTLS_SSL_CERTIFICATE_REQUEST @@ -1412,13 +1678,9 @@ static int ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ) return( ret ); } - if( ( ssl->in_msgtype == MBEDTLS_SSL_MSG_HANDSHAKE ) && - ( ssl->in_msg[0] == MBEDTLS_SSL_HS_CERTIFICATE_REQUEST ) ) - { - ret = mbedtls_ssl_tls13_process_certificate_request( ssl ); - if( ret != 0 ) - return( ret ); - } + ret = mbedtls_ssl_tls13_process_certificate_request( ssl ); + if( ret != 0 ) + return( ret ); ssl->keep_current_message = 1; mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_CERTIFICATE ); diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 90c20bde92..650f9baae6 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -220,295 +220,6 @@ int mbedtls_ssl_tls13_write_sig_alg_ext( mbedtls_ssl_context *ssl, return( 0 ); } -#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) -static int ssl_tls13_parse_signature_algorithms_ext( mbedtls_ssl_context *ssl, - const unsigned char *buf, - const unsigned char *end ) -{ - size_t sig_alg_list_size; /* size of receive signature algorithms list */ - const unsigned char *p; /* pointer to individual signature algorithm */ - const uint16_t *sig_alg; /* iterate through configured signature schemes */ - unsigned int signature_scheme; /* store received signature algorithm scheme */ - uint32_t common_idx = 0; /* iterate through received_signature_schemes_list */ - size_t buf_len = end - buf; - - MBEDTLS_SSL_CHK_BUF_READ_PTR( buf, end, 2 ); - - sig_alg_list_size = MBEDTLS_GET_UINT16_BE( buf, 0 ); - if( sig_alg_list_size + 2 != buf_len || - sig_alg_list_size % 2 != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad signature_algorithms extension" ) ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - } - memset( ssl->handshake->recv_sig_schemes_list, 0, - sizeof( ssl->handshake->recv_sig_schemes_list ) ); - - for( p = buf + 2; p < end && common_idx + 1 < MBEDTLS_PK_SIGNATURE_MAX_SIZE; p += 2 ) - { - signature_scheme = MBEDTLS_GET_UINT16_BE( p, 0 ); - - MBEDTLS_SSL_DEBUG_MSG( 4, ( "received signature algorithm: 0x%x", - signature_scheme ) ); - - for( sig_alg = ssl->conf->tls13_sig_algs; - *sig_alg != MBEDTLS_TLS1_3_SIG_NONE; sig_alg++ ) - { - if( *sig_alg == signature_scheme ) - { - ssl->handshake->recv_sig_schemes_list[common_idx] = signature_scheme; - common_idx++; - break; - } - } - } - - if( common_idx == 0 ) - { - 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 ); - return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); - } - - ssl->handshake->recv_sig_schemes_list[common_idx] = MBEDTLS_TLS1_3_SIG_NONE; - - return( 0 ); -} -#endif - -/* - * - * STATE HANDLING: CertificateRequest - * - */ - -/* - * Overview - */ - -/* Coordination: - * Deals with the ambiguity of not knowing if a CertificateRequest - * will be sent. Returns a negative code on failure, or - * - SSL_CERTIFICATE_REQUEST_EXPECT_REQUEST - * - SSL_CERTIFICATE_REQUEST_SKIP - * indicating if a Certificate Request is expected or not. - */ -#define SSL_CERTIFICATE_REQUEST_EXPECT_REQUEST 0 -#define SSL_CERTIFICATE_REQUEST_SKIP 1 - -/* - * Implementation - */ - -static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) -{ - int ret; - - if( mbedtls_ssl_tls13_psk_enabled( ssl ) ) - { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "<= skip parse certificate request" ) ); - return( SSL_CERTIFICATE_REQUEST_SKIP ); - } - - if( ( ret = mbedtls_ssl_read_record( ssl, 0 ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret ); - return( ret ); - } - 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 ); - } - - return( SSL_CERTIFICATE_REQUEST_SKIP ); -} - -#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) -static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, - const unsigned char *buf, - const unsigned char *end ) -{ - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - const unsigned char *p = buf; - const unsigned char *ext = NULL; - size_t ext_len = 0; - size_t context_len = 0; - - - /* - * struct { - * opaque certificate_request_context<0..2^8-1>; - * Extension extensions<2..2^16-1>; - * } CertificateRequest; - */ - - /* - * Parse certificate_request_context - */ - - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); - context_len = (size_t) p[0]; - /* skip context_len */ - p++; - - /* Fixed length fields are: - * - 1 for length of context - * - 2 for length of extensions - * ----- - * 3 bytes - */ - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 3 ); - - /* store context ( if necessary ) */ - if( context_len > 0 ) - { - MBEDTLS_SSL_DEBUG_BUF( 3, "Certificate Request Context", - p, context_len ); - - ssl->handshake->cert_req_ctx = mbedtls_calloc( context_len, 1 ); - if( ssl->handshake->cert_req_ctx == NULL ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); - return ( MBEDTLS_ERR_SSL_ALLOC_FAILED ); - } - memcpy( ssl->handshake->cert_req_ctx, p, context_len ); - - /* jump over certificate_request_context */ - p += context_len; - } - - /* - * Parse extensions - */ - ext_len = MBEDTLS_GET_UINT16_BE( p, 0 ); - - /* At least one extension needs to be present, - * namely signature_algorithms ext. */ - if( ext_len < 4 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) ); - MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, - MBEDTLS_ERR_SSL_DECODE_ERROR ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - } - - /* skip total extension length */ - p += 2; - - ext = p; /* jump to extensions */ - while( ext_len ) - { - size_t ext_id = MBEDTLS_GET_UINT16_BE( ext, 0 ); - size_t ext_size = MBEDTLS_GET_UINT16_BE( ext, 1 ); - - if( ext_size + 4 > ext_len ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) ); - MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, - MBEDTLS_ERR_SSL_DECODE_ERROR ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - } - - switch( ext_id ) - { - case MBEDTLS_TLS_EXT_SIG_ALG: - MBEDTLS_SSL_DEBUG_MSG( 3, - ( "found signature_algorithms extension" ) ); - - if( ( ret = ssl_tls13_parse_signature_algorithms_ext( ssl, - ext + 4, ext + 4 + ext_size ) ) != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, - "ssl_tls13_parse_signature_algorithms_ext", ret ); - MBEDTLS_SSL_PEND_FATAL_ALERT( - MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, ret ); - return( ret ); - } - break; - - default: - MBEDTLS_SSL_DEBUG_MSG( 3, - ( "unknown extension found: %" MBEDTLS_PRINTF_SIZET " ( ignoring )", - ext_id ) ); - break; - } - - ext_len -= 4 + ext_size; - ext += 4 + ext_size; - - if( ( ext_len > 0 ) && ( ext_len < 4 ) ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) ); - MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, - MBEDTLS_ERR_SSL_DECODE_ERROR ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - } - } - - ssl->client_auth = 1; - return( 0 ); -} -#endif /* ( MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED ) */ - -/* Main entry point; orchestrates the other functions */ -int mbedtls_ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ) -{ - int ret = 0; - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse certificate request" ) ); - - /* Coordination step - * - Fetch record - * - Make sure it's either a CertificateRequest or a ServerHelloDone - */ - MBEDTLS_SSL_PROC_CHK_NEG( ssl_tls13_certificate_request_coordinate( ssl ) ); - -#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) - if( ret == SSL_CERTIFICATE_REQUEST_EXPECT_REQUEST ) - { - unsigned char *buf; - size_t buf_len; - - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_fetch_handshake_msg( ssl, - MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, - &buf, &buf_len ) ); - - MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_certificate_request( ssl, - buf, buf + buf_len ) ); - - mbedtls_ssl_tls13_add_hs_msg_to_checksum( - ssl, MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, buf, buf_len ); - } - else -#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ - if( ret == SSL_CERTIFICATE_REQUEST_SKIP ) - { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate request" ) ); - } - else - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); - } - - MBEDTLS_SSL_DEBUG_MSG( 3, ( "got %s certificate request", - ssl->client_auth ? "a" : "no" ) ); - -cleanup: - - /* In the MPS one would close the read-port here to - * ensure there's no overlap of reading and writing. */ - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse certificate request" ) ); - return( ret ); -} - /* * STATE HANDLING: Read CertificateVerify */ @@ -860,20 +571,20 @@ static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, const unsigned char *end ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - size_t cert_req_ctx_len = 0; + size_t certificate_request_context_len = 0; size_t certificate_list_len = 0; const unsigned char *p = buf; const unsigned char *certificate_list_end; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 4 ); - cert_req_ctx_len = p[0]; + certificate_request_context_len = p[0]; certificate_list_len = MBEDTLS_GET_UINT24_BE( p, 1 ); p += 4; /* In theory, the certificate list can be up to 2^24 Bytes, but we don't * support anything beyond 2^16 = 64K. */ - if( ( cert_req_ctx_len != 0 ) || + if( ( certificate_request_context_len != 0 ) || ( certificate_list_len >= 0x10000 ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate message" ) ); From f6d3696edab66258defbc9b1d815a426bd03d1f9 Mon Sep 17 00:00:00 2001 From: Xiaofei Bai Date: Sun, 16 Jan 2022 14:54:35 +0000 Subject: [PATCH 04/12] fix test failures Signed-off-by: Xiaofei Bai --- library/ssl_tls13_client.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 23b067079f..4d2464211a 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1645,6 +1645,7 @@ int mbedtls_ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ) if( ret == SSL_CERTIFICATE_REQUEST_SKIP ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate request" ) ); + ret = 0; } else { @@ -1677,12 +1678,12 @@ static int ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret ); return( ret ); } + ssl->keep_current_message = 1; ret = mbedtls_ssl_tls13_process_certificate_request( ssl ); if( ret != 0 ) return( ret ); - ssl->keep_current_message = 1; mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_CERTIFICATE ); return( 0 ); From de3f13e0b83f28e481751f0573c50e2fb8e059d6 Mon Sep 17 00:00:00 2001 From: Xiaofei Bai Date: Tue, 18 Jan 2022 05:47:05 +0000 Subject: [PATCH 05/12] update based on comments Signed-off-by: Xiaofei Bai --- library/ssl_tls13_client.c | 46 ++++++++++---------------------------- 1 file changed, 12 insertions(+), 34 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 4d2464211a..b220e15ef2 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1455,6 +1455,7 @@ static int ssl_tls13_parse_signature_algorithms_ext( mbedtls_ssl_context *ssl, } #endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) /* * * STATE HANDLING: CertificateRequest @@ -1478,10 +1479,9 @@ static int ssl_tls13_parse_signature_algorithms_ext( mbedtls_ssl_context *ssl, /* * Implementation */ - static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) { - int ret; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; if( mbedtls_ssl_tls13_psk_enabled( ssl ) ) { @@ -1511,6 +1511,7 @@ static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) return( SSL_CERTIFICATE_REQUEST_SKIP ); } +#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ #if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) /* @@ -1609,19 +1610,18 @@ static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, ssl->client_auth = 1; return( 0 ); } -#endif /* ( MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED ) */ +#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ -/* Main entry point; orchestrates the other functions */ -int mbedtls_ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ) +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) +/* + * Handler for MBEDTLS_SSL_CERTIFICATE_REQUEST + */ +static int ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ) { - int ret = 0; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse certificate request" ) ); - /* Coordination step - * - Fetch record - * - Make sure it's either a CertificateRequest or a ServerHelloDone - */ MBEDTLS_SSL_PROC_CHK_NEG( ssl_tls13_certificate_request_coordinate( ssl ) ); #if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) @@ -1656,6 +1656,8 @@ int mbedtls_ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 3, ( "got %s certificate request", ssl->client_auth ? "a" : "no" ) ); + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_CERTIFICATE ); + cleanup: /* In the MPS one would close the read-port here to @@ -1665,30 +1667,6 @@ cleanup: return( ret ); } -#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) -/* - * Handler for MBEDTLS_SSL_CERTIFICATE_REQUEST - */ -static int ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ) -{ - int ret = mbedtls_ssl_read_record( ssl, 0 ); - - if( ret != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret ); - return( ret ); - } - ssl->keep_current_message = 1; - - ret = mbedtls_ssl_tls13_process_certificate_request( ssl ); - if( ret != 0 ) - return( ret ); - - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_CERTIFICATE ); - - return( 0 ); -} - /* * Handler for MBEDTLS_SSL_SERVER_CERTIFICATE */ From 69fcd397748d283dc956d71398173eb41b0d37ff Mon Sep 17 00:00:00 2001 From: Xiaofei Bai Date: Thu, 20 Jan 2022 08:25:00 +0000 Subject: [PATCH 06/12] Update CertificateRequest tests and the parsing function Signed-off-by: Xiaofei Bai --- library/ssl_misc.h | 8 ++- library/ssl_tls13_client.c | 127 +++++++++++------------------------- library/ssl_tls13_generic.c | 72 ++++++++++++++++++++ tests/ssl-opt.sh | 2 + 4 files changed, 119 insertions(+), 90 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index c2ce07773b..a8ac52c9fe 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -560,7 +560,7 @@ struct mbedtls_ssl_handshake_params * Handshake specific crypto variables */ #if defined(MBEDTLS_X509_CRT_PARSE_C) - int recv_sig_schemes_list[MBEDTLS_PK_SIGNATURE_MAX_SIZE]; + uint16_t sig_algs[MBEDTLS_PK_SIGNATURE_MAX_SIZE]; /*!< Received signature algorithms */ #endif /* MBEDTLS_X509_CRT_PARSE_C */ @@ -1747,6 +1747,12 @@ void mbedtls_ssl_tls13_add_hs_msg_to_checksum( mbedtls_ssl_context *ssl, int mbedtls_ssl_write_sig_alg_ext( mbedtls_ssl_context *ssl, unsigned char *buf, const unsigned char *end, size_t *out_len ); +/* + * Parse TLS 1.3 Signature Algorithm extension + */ +int mbedtls_ssl_tls13_parse_sig_alg_ext( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ); #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ /* Get handshake transcript */ diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index b220e15ef2..a47a3c41a4 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1398,74 +1398,14 @@ static int ssl_tls13_postprocess_encrypted_extensions( mbedtls_ssl_context *ssl return( 0 ); } -#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) -static int ssl_tls13_parse_signature_algorithms_ext( mbedtls_ssl_context *ssl, - const unsigned char *buf, - const unsigned char *end ) -{ - size_t sig_alg_list_size; /* size of receive signature algorithms list */ - const unsigned char *p; /* pointer to individual signature algorithm */ - const uint16_t *sig_alg; /* iterate through configured signature schemes */ - unsigned int signature_scheme; /* store received signature algorithm scheme */ - uint32_t common_idx = 0; /* iterate through received_signature_schemes_list */ - size_t buf_len = end - buf; - - MBEDTLS_SSL_CHK_BUF_READ_PTR( buf, end, 2 ); - - sig_alg_list_size = MBEDTLS_GET_UINT16_BE( buf, 0 ); - if( sig_alg_list_size + 2 != buf_len || - sig_alg_list_size % 2 != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad signature_algorithms extension" ) ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - } - memset( ssl->handshake->recv_sig_schemes_list, 0, - sizeof( ssl->handshake->recv_sig_schemes_list ) ); - - for( p = buf + 2; p < end && common_idx + 1 < MBEDTLS_PK_SIGNATURE_MAX_SIZE; p += 2 ) - { - signature_scheme = MBEDTLS_GET_UINT16_BE( p, 0 ); - - MBEDTLS_SSL_DEBUG_MSG( 4, ( "received signature algorithm: 0x%x", - signature_scheme ) ); - - for( sig_alg = ssl->conf->tls13_sig_algs; - *sig_alg != MBEDTLS_TLS1_3_SIG_NONE; sig_alg++ ) - { - if( *sig_alg == signature_scheme ) - { - ssl->handshake->recv_sig_schemes_list[common_idx] = signature_scheme; - common_idx++; - break; - } - } - } - - if( common_idx == 0 ) - { - 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 ); - return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); - } - - ssl->handshake->recv_sig_schemes_list[common_idx] = MBEDTLS_TLS1_3_SIG_NONE; - - return( 0 ); -} -#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ - #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) /* * * STATE HANDLING: CertificateRequest * */ - -/* - * Overview - */ - +#define SSL_CERTIFICATE_REQUEST_EXPECT_REQUEST 0 +#define SSL_CERTIFICATE_REQUEST_SKIP 1 /* Coordination: * Deals with the ambiguity of not knowing if a CertificateRequest * will be sent. Returns a negative code on failure, or @@ -1473,9 +1413,6 @@ static int ssl_tls13_parse_signature_algorithms_ext( mbedtls_ssl_context *ssl, * - SSL_CERTIFICATE_REQUEST_SKIP * indicating if a Certificate Request is expected or not. */ -#define SSL_CERTIFICATE_REQUEST_EXPECT_REQUEST 0 -#define SSL_CERTIFICATE_REQUEST_SKIP 1 - /* * Implementation */ @@ -1483,7 +1420,7 @@ static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - if( mbedtls_ssl_tls13_psk_enabled( ssl ) ) + if( ! mbedtls_ssl_tls13_some_ephemeral_enabled( ssl ) ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "<= skip parse certificate request" ) ); return( SSL_CERTIFICATE_REQUEST_SKIP ); @@ -1511,9 +1448,7 @@ static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) return( SSL_CERTIFICATE_REQUEST_SKIP ); } -#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) /* * ssl_tls13_parse_certificate_request() * Parse certificate request @@ -1531,11 +1466,13 @@ static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, 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; /* Parse certificate_request_context */ MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); certificate_request_context_len = (size_t) p[0]; - p++; + p += 1; /* store context ( if necessary ) */ if( certificate_request_context_len > 0 ) @@ -1543,18 +1480,16 @@ static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_BUF( 3, "Certificate Request Context", p, certificate_request_context_len ); - ssl->handshake->certificate_request_context = - mbedtls_calloc( certificate_request_context_len, 1 ); - if( ssl->handshake->certificate_request_context == NULL ) + handshake->certificate_request_context = + mbedtls_calloc( 1, certificate_request_context_len); + if( handshake->certificate_request_context == NULL ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); return ( MBEDTLS_ERR_SSL_ALLOC_FAILED ); } MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, certificate_request_context_len ); - memcpy( ssl->handshake->certificate_request_context, p, + memcpy( handshake->certificate_request_context, p, certificate_request_context_len ); - - /* jump over certificate_request_context */ p += certificate_request_context_len; } @@ -1584,12 +1519,17 @@ static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, { case MBEDTLS_TLS_EXT_SIG_ALG: MBEDTLS_SSL_DEBUG_MSG( 3, - ( "found signature_algorithms extension" ) ); - - ret = ssl_tls13_parse_signature_algorithms_ext( ssl, p, + ( "found signature algorithms extension" ) ); + ret = mbedtls_ssl_tls13_parse_sig_alg_ext( ssl, p, 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 )" ) ); break; default: @@ -1597,22 +1537,37 @@ 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 ); } - p += extension_data_len; } + /* Check that we consumed all the message. */ + if( p != end ) + { + 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 ); + } + /* Check that we found signature algorithms extension */ + if( n_sig_alg_ext == 0 ) + { + 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 ); + } ssl->client_auth = 1; return( 0 ); } -#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) /* * Handler for MBEDTLS_SSL_CERTIFICATE_REQUEST */ @@ -1624,7 +1579,6 @@ static int ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_PROC_CHK_NEG( ssl_tls13_certificate_request_coordinate( ssl ) ); -#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) if( ret == SSL_CERTIFICATE_REQUEST_EXPECT_REQUEST ) { unsigned char *buf; @@ -1640,9 +1594,7 @@ static int ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ) mbedtls_ssl_tls13_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, buf, buf_len ); } - else -#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ - if( ret == SSL_CERTIFICATE_REQUEST_SKIP ) + else if( ret == SSL_CERTIFICATE_REQUEST_SKIP ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate request" ) ); ret = 0; @@ -1660,9 +1612,6 @@ static int ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ) cleanup: - /* In the MPS one would close the read-port here to - * ensure there's no overlap of reading and writing. */ - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse certificate request" ) ); return( ret ); } diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 650f9baae6..aeefdc77f9 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -220,6 +220,78 @@ int mbedtls_ssl_tls13_write_sig_alg_ext( mbedtls_ssl_context *ssl, return( 0 ); } +/* mbedtls_ssl_tls13_parse_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; + */ +int mbedtls_ssl_tls13_parse_sig_alg_ext( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) +{ + const unsigned char *p = buf; + const uint16_t *sig_alg; + unsigned int signature_scheme; /* store received signature algorithm scheme */ + 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 ); + p += 2; + + memset( ssl->handshake->sig_algs, 0, sizeof( ssl->handshake->sig_algs ) ); + + while( p < end && common_idx + 1 < MBEDTLS_PK_SIGNATURE_MAX_SIZE ) + { + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); + signature_scheme = MBEDTLS_GET_UINT16_BE( p, 0 ); + p += 2; + + MBEDTLS_SSL_DEBUG_MSG( 4, ( "received signature algorithm: 0x%x", + signature_scheme ) ); + + for( sig_alg = ssl->conf->tls13_sig_algs; + *sig_alg != MBEDTLS_TLS1_3_SIG_NONE; sig_alg++ ) + { + if( *sig_alg == signature_scheme ) + { + ssl->handshake->sig_algs[common_idx] = signature_scheme; + common_idx++; + break; + } + } + } + /* Check that we consumed all the message. */ + if( p != end ) + { + 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 ); + } + + if( common_idx == 0 ) + { + 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 ); + return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + } + + ssl->handshake->sig_algs[common_idx] = MBEDTLS_TLS1_3_SIG_NONE; + + return( 0 ); +} + /* * STATE HANDLING: Read CertificateVerify */ diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 0fabae907e..3babab82f1 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -9194,6 +9194,7 @@ run_test "TLS 1.3: CertificateRequest check - openssl" \ "$P_CLI debug_level=4 force_version=tls13 " \ 1 \ -c "=> parse certificate request" \ + -c "got a certificate request" \ -c "<= parse certificate request" requires_gnutls_tls1_3 @@ -9208,6 +9209,7 @@ run_test "TLS 1.3: CertificateRequest check - gnutls" \ "$P_CLI debug_level=3 min_version=tls13 max_version=tls13" \ 1 \ -c "=> parse certificate request" \ + -c "got a certificate request" \ -c "<= parse certificate request" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 From 82f0a9a1dbd7761b5e600a0cc1bbe064cac0ea9a Mon Sep 17 00:00:00 2001 From: Xiaofei Bai Date: Wed, 26 Jan 2022 09:21:54 +0000 Subject: [PATCH 07/12] 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 a8ac52c9fe..093732e033 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 a47a3c41a4..b3272c90fa 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 aeefdc77f9..2f4d04e10f 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 ); } From f5b4d25cfaf39b1acdf49882ebbf6a85517392ea Mon Sep 17 00:00:00 2001 From: Xiaofei Bai Date: Fri, 28 Jan 2022 06:37:15 +0000 Subject: [PATCH 08/12] Add received_sig_algs member to struct mbedtls_ssl_handshake_params Signed-off-by: Xiaofei Bai --- library/ssl_misc.h | 6 +++++- library/ssl_tls13_generic.c | 29 ++++++++++++----------------- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 093732e033..049195eb37 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -268,7 +268,7 @@ #define MBEDTLS_SSL_MAX_CURVE_LIST_LEN 65535 #if defined(MBEDTLS_SSL_PROTO_TLS1_3) -#define MBEDTLS_SIG_ALGS_SIZE 20 +#define MBEDTLS_RECEIVED_SIG_ALGS_SIZE 20 #endif /* @@ -600,6 +600,10 @@ struct mbedtls_ssl_handshake_params mbedtls_ssl_sig_hash_set_t hash_algs; /*!< Set of suitable sig-hash pairs */ #endif +#if defined(MBEDTLS_X509_CRT_PARSE_C) + uint16_t received_sig_algs[MBEDTLS_RECEIVED_SIG_ALGS_SIZE]; +#endif + #if !defined(MBEDTLS_DEPRECATED_REMOVED) const uint16_t *group_list; const uint16_t *sig_algs; diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 2f4d04e10f..86699d9596 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -155,26 +155,25 @@ int mbedtls_ssl_tls13_parse_sig_alg_ext( mbedtls_ssl_context *ssl, const unsigned char *end ) { const unsigned char *p = buf; - size_t sig_algs_len = 0; - uint16_t *sig_algs; - const unsigned char *sig_algs_end; + size_t supported_sig_algs_len = 0; + const unsigned char *supported_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 ); + supported_sig_algs_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; - sig_algs = mbedtls_calloc( MBEDTLS_SIG_ALGS_SIZE, sizeof( uint16_t ) ); - if( sig_algs == NULL ) - return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); + memset( ssl->handshake->received_sig_algs, 0, + sizeof( ssl->handshake->received_sig_algs) ); - 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, supported_sig_algs_len ); + supported_sig_algs_end = p + supported_sig_algs_len; + while( p < supported_sig_algs_end && + common_idx + 1 < MBEDTLS_RECEIVED_SIG_ALGS_SIZE ) { - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, sig_algs_end, 2 ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, supported_sig_algs_end, 2 ); sig_alg = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; @@ -184,7 +183,7 @@ int mbedtls_ssl_tls13_parse_sig_alg_ext( mbedtls_ssl_context *ssl, if( mbedtls_ssl_sig_alg_is_offered( ssl, sig_alg ) && mbedtls_ssl_sig_alg_is_supported( ssl, sig_alg ) ) { - sig_algs[common_idx] = sig_alg; + ssl->handshake->received_sig_algs[common_idx] = sig_alg; common_idx += 1; } } @@ -195,7 +194,6 @@ 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 ); } @@ -204,13 +202,10 @@ 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 ); } - sig_algs[common_idx] = MBEDTLS_TLS1_3_SIG_NONE; - ssl->handshake->sig_algs = sig_algs; - ssl->handshake->sig_algs_heap_allocated = 1; + ssl->handshake->received_sig_algs[common_idx] = MBEDTLS_TLS1_3_SIG_NONE; return( 0 ); } From 6d42bb430c50fa5c4cd2177c61d363c14fd56d69 Mon Sep 17 00:00:00 2001 From: Xiaofei Bai Date: Fri, 28 Jan 2022 08:52:13 +0000 Subject: [PATCH 09/12] Update mbedtls_ssl_handshake_free() Signed-off-by: Xiaofei Bai --- library/ssl_misc.h | 2 +- library/ssl_tls.c | 9 ++++++++- library/ssl_tls13_client.c | 31 +++++++++++++++++++------------ 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 049195eb37..60c68d4d55 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -267,7 +267,7 @@ /* 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) +#if defined(MBEDTLS_X509_CRT_PARSE_C) #define MBEDTLS_RECEIVED_SIG_ALGS_SIZE 20 #endif diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 5e8b60b9bc..91767fb004 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5579,7 +5579,14 @@ void mbedtls_ssl_handshake_free( mbedtls_ssl_context *ssl ) mbedtls_free( (void*) handshake->sig_algs ); handshake->sig_algs = NULL; #endif /* MBEDTLS_DEPRECATED_REMOVED */ - +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) + if( ssl->handshake->certificate_request_context ) + { + mbedtls_free( (void*) handshake->certificate_request_context ); + } + handshake->certificate_request_context = NULL; + handshake->certificate_request_context_len = 0; +#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ #if defined(MBEDTLS_SSL_ASYNC_PRIVATE) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index b3272c90fa..87517874e6 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1473,7 +1473,7 @@ static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, mbedtls_ssl_handshake_params *handshake = ssl->handshake; handshake->certificate_request_context = - mbedtls_calloc( 1, certificate_request_context_len); + mbedtls_calloc( 1, certificate_request_context_len ); if( handshake->certificate_request_context == NULL ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); @@ -1523,7 +1523,11 @@ static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, { MBEDTLS_SSL_DEBUG_MSG( 3, ( "Duplicate signature algorithms extensions found" ) ); - goto error; + MBEDTLS_SSL_PEND_FATAL_ALERT( + MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, + MBEDTLS_ERR_SSL_DECODE_ERROR ); + mbedtls_ssl_handshake_free( ssl ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); } break; @@ -1541,24 +1545,27 @@ static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, { MBEDTLS_SSL_DEBUG_MSG( 1, ( "Signature algorithms extension length misaligned" ) ); - goto error; + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, + MBEDTLS_ERR_SSL_DECODE_ERROR ); + mbedtls_ssl_handshake_free( ssl ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); } /* Check that we found signature algorithms extension */ if( ! sig_alg_ext_found ) { - MBEDTLS_SSL_DEBUG_MSG( 3, - ( "no signature algorithms extension found" ) ); - goto error; + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "no signature algorithms extension found" ) ); + 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 ); + mbedtls_ssl_handshake_free( ssl ); + return( MBEDTLS_ERR_SSL_DECODE_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 ); } /* From 51f515a5035a3abc8493e98b42f12bbd6e6e061e Mon Sep 17 00:00:00 2001 From: Xiaofei Bai Date: Tue, 8 Feb 2022 07:28:04 +0000 Subject: [PATCH 10/12] update based on comments Signed-off-by: Xiaofei Bai --- library/ssl_misc.h | 10 ++-------- library/ssl_tls.c | 9 +-------- library/ssl_tls13_client.c | 29 ++++++++++++----------------- library/ssl_tls13_generic.c | 12 +++++++----- 4 files changed, 22 insertions(+), 38 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 60c68d4d55..f68bb77bc6 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -267,9 +267,7 @@ /* Maximum size in bytes of list in supported elliptic curve ext., RFC 4492 */ #define MBEDTLS_SSL_MAX_CURVE_LIST_LEN 65535 -#if defined(MBEDTLS_X509_CRT_PARSE_C) #define MBEDTLS_RECEIVED_SIG_ALGS_SIZE 20 -#endif /* * Check that we obey the standard's message size bounds @@ -600,7 +598,8 @@ struct mbedtls_ssl_handshake_params mbedtls_ssl_sig_hash_set_t hash_algs; /*!< Set of suitable sig-hash pairs */ #endif -#if defined(MBEDTLS_X509_CRT_PARSE_C) +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) && \ + defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) uint16_t received_sig_algs[MBEDTLS_RECEIVED_SIG_ALGS_SIZE]; #endif @@ -1700,11 +1699,6 @@ int mbedtls_ssl_tls13_start_handshake_msg( mbedtls_ssl_context *ssl, unsigned char **buf, size_t *buf_len ); -/* - * Handler of TLS 1.3 server certificate request message - */ -int mbedtls_ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ); - /* * Handler of TLS 1.3 server certificate message */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 91767fb004..5e8b60b9bc 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5579,14 +5579,7 @@ void mbedtls_ssl_handshake_free( mbedtls_ssl_context *ssl ) mbedtls_free( (void*) handshake->sig_algs ); handshake->sig_algs = NULL; #endif /* MBEDTLS_DEPRECATED_REMOVED */ -#if defined(MBEDTLS_SSL_PROTO_TLS1_3) - if( ssl->handshake->certificate_request_context ) - { - mbedtls_free( (void*) handshake->certificate_request_context ); - } - handshake->certificate_request_context = NULL; - handshake->certificate_request_context_len = 0; -#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ + #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ #if defined(MBEDTLS_SSL_ASYNC_PRIVATE) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 87517874e6..62ed098351 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1523,11 +1523,7 @@ static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, { MBEDTLS_SSL_DEBUG_MSG( 3, ( "Duplicate signature algorithms extensions found" ) ); - MBEDTLS_SSL_PEND_FATAL_ALERT( - MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, - MBEDTLS_ERR_SSL_DECODE_ERROR ); - mbedtls_ssl_handshake_free( ssl ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + goto error; } break; @@ -1544,11 +1540,8 @@ static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, if( p != end ) { 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 ); - mbedtls_ssl_handshake_free( ssl ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + ( "CertificateRequset misaligned" ) ); + goto error; } /* Check that we found signature algorithms extension */ if( ! sig_alg_ext_found ) @@ -1556,16 +1549,17 @@ static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "no signature algorithms extension found" ) ); 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 ); - mbedtls_ssl_handshake_free( ssl ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - + ( "ssl_tls13_parse_certificate_request" ) ); + 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 ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); } /* @@ -1602,7 +1596,8 @@ static int ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ) else { MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; + goto cleanup; } MBEDTLS_SSL_DEBUG_MSG( 3, ( "got %s certificate request", diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 86699d9596..e174d63984 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -166,12 +166,11 @@ int mbedtls_ssl_tls13_parse_sig_alg_ext( mbedtls_ssl_context *ssl, p += 2; memset( ssl->handshake->received_sig_algs, 0, - sizeof( ssl->handshake->received_sig_algs) ); + sizeof(ssl->handshake->received_sig_algs) ); MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, supported_sig_algs_len ); supported_sig_algs_end = p + supported_sig_algs_len; - while( p < supported_sig_algs_end && - common_idx + 1 < MBEDTLS_RECEIVED_SIG_ALGS_SIZE ) + while( p < supported_sig_algs_end ) { MBEDTLS_SSL_CHK_BUF_READ_PTR( p, supported_sig_algs_end, 2 ); sig_alg = MBEDTLS_GET_UINT16_BE( p, 0 ); @@ -180,8 +179,11 @@ int mbedtls_ssl_tls13_parse_sig_alg_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 4, ( "received signature algorithm: 0x%x", sig_alg ) ); - if( mbedtls_ssl_sig_alg_is_offered( ssl, sig_alg ) && - mbedtls_ssl_sig_alg_is_supported( ssl, sig_alg ) ) + if( ! mbedtls_ssl_sig_alg_is_offered( ssl, sig_alg ) || + ! mbedtls_ssl_sig_alg_is_supported( ssl, sig_alg ) ) + continue; + + if( common_idx + 1 < MBEDTLS_RECEIVED_SIG_ALGS_SIZE ) { ssl->handshake->received_sig_algs[common_idx] = sig_alg; common_idx += 1; From c234ecf6956399405fe66d3c392c402b2cadcb0b Mon Sep 17 00:00:00 2001 From: Xiaofei Bai Date: Tue, 8 Feb 2022 09:59:23 +0000 Subject: [PATCH 11/12] Update mbedtls_ssl_handshake_free() and address review comments. Signed-off-by: Xiaofei Bai --- library/ssl_tls.c | 7 ++++++- library/ssl_tls13_client.c | 14 ++++++-------- library/ssl_tls13_generic.c | 3 +-- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 5e8b60b9bc..d08fe3a0a8 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5579,7 +5579,12 @@ void mbedtls_ssl_handshake_free( mbedtls_ssl_context *ssl ) mbedtls_free( (void*) handshake->sig_algs ); handshake->sig_algs = NULL; #endif /* MBEDTLS_DEPRECATED_REMOVED */ - +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) + if( ssl->handshake->certificate_request_context ) + { + mbedtls_free( (void*) handshake->certificate_request_context ); + } +#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ #if defined(MBEDTLS_SSL_ASYNC_PRIVATE) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 62ed098351..a103c9da0e 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1468,6 +1468,7 @@ static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, if( certificate_request_context_len > 0 ) { + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, certificate_request_context_len ); MBEDTLS_SSL_DEBUG_BUF( 3, "Certificate Request Context", p, certificate_request_context_len ); @@ -1479,7 +1480,6 @@ static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); return ( MBEDTLS_ERR_SSL_ALLOC_FAILED ); } - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, certificate_request_context_len ); memcpy( handshake->certificate_request_context, p, certificate_request_context_len ); p += certificate_request_context_len; @@ -1523,7 +1523,7 @@ static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, { MBEDTLS_SSL_DEBUG_MSG( 3, ( "Duplicate signature algorithms extensions found" ) ); - goto error; + goto decode_error; } break; @@ -1540,23 +1540,21 @@ static int ssl_tls13_parse_certificate_request( mbedtls_ssl_context *ssl, if( p != end ) { MBEDTLS_SSL_DEBUG_MSG( 1, - ( "CertificateRequset misaligned" ) ); - goto error; + ( "CertificateRequest misaligned" ) ); + goto decode_error; } /* Check that we found signature algorithms extension */ if( ! sig_alg_ext_found ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "no signature algorithms extension found" ) ); - MBEDTLS_SSL_DEBUG_MSG( 1, - ( "ssl_tls13_parse_certificate_request" ) ); - goto error; + goto decode_error; } ssl->client_auth = 1; return( 0 ); -error: +decode_error: MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, MBEDTLS_ERR_SSL_DECODE_ERROR ); return( MBEDTLS_ERR_SSL_DECODE_ERROR ); diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index e174d63984..c8c2583ee9 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -158,9 +158,8 @@ int mbedtls_ssl_tls13_parse_sig_alg_ext( mbedtls_ssl_context *ssl, size_t supported_sig_algs_len = 0; const unsigned char *supported_sig_algs_end; uint16_t sig_alg; - uint32_t common_idx = 0; /* iterate through received signature schemes list */ + uint32_t common_idx = 0; - /* skip 2 bytes of signature algorithms length */ MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); supported_sig_algs_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; From 7c8b6a97b9e4d9b074cd4c1628503821b6ffa3b1 Mon Sep 17 00:00:00 2001 From: Xiaofei Bai Date: Tue, 8 Feb 2022 15:21:13 +0000 Subject: [PATCH 12/12] Update CertificateRequest skip condition Signed-off-by: Xiaofei Bai --- library/ssl_tls13_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index a103c9da0e..6d4cd52961 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1417,7 +1417,7 @@ static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - if( ! mbedtls_ssl_tls13_some_ephemeral_enabled( ssl ) ) + if( mbedtls_ssl_tls13_some_psk_enabled( ssl ) ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "<= skip parse certificate request" ) ); return( SSL_CERTIFICATE_REQUEST_SKIP );