From 947571efff70d8a1b66501c2aa761bd702d538ee Mon Sep 17 00:00:00 2001 From: Xiaofei Bai Date: Wed, 29 Sep 2021 09:12:03 +0000 Subject: [PATCH 01/12] add tls1_3 read certificate Signed-off-by: Xiaofei Bai --- library/ssl_misc.h | 6 + library/ssl_tls13_client.c | 7 +- library/ssl_tls13_generic.c | 367 ++++++++++++++++++++++++++++++++++++ 3 files changed, 379 insertions(+), 1 deletion(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 66fb26c624..dbef6aa212 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1627,6 +1627,12 @@ int mbedtls_ssl_tls13_start_handshake_msg( mbedtls_ssl_context *ssl, unsigned hs_type, unsigned char **buf, size_t *buflen ); + +/* + * Handler of TLS 1.3 server certificate message + */ +int mbedtls_ssl_tls13_process_certificate( mbedtls_ssl_context *ssl ); + /* * Write TLS 1.3 handshake message tail */ diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 5ed01aade2..44c7b1ecc9 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1542,7 +1542,12 @@ static int ssl_tls1_3_process_certificate_request( mbedtls_ssl_context *ssl ) */ static int ssl_tls1_3_process_server_certificate( mbedtls_ssl_context *ssl ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "%s hasn't been implemented", __func__ ) ); + int ret; + + ret = mbedtls_ssl_tls13_process_certificate( ssl ); + if( ret != 0) + return( ret ); + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CERTIFICATE_VERIFY ); return( 0 ); } diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index b3a4a09ddc..8cae7898bf 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -27,6 +27,10 @@ #include "mbedtls/debug.h" #include "ssl_misc.h" +#include +#include +#include + int mbedtls_ssl_tls1_3_fetch_handshake_msg( mbedtls_ssl_context *ssl, unsigned hs_type, @@ -215,6 +219,369 @@ int mbedtls_ssl_tls13_write_sig_alg_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ +/* + * + * STATE HANDLING: Incoming Certificate, client-side only currently. + * + */ + +/* + * Overview + */ + +/* Main state-handling entry point; orchestrates the other functions. */ +int mbedtls_ssl_tls13_process_certificate( mbedtls_ssl_context *ssl ); + +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) +/* Parse certificate chain send by the server. */ +static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ); +/* Validate certificate chain sent by the server. */ +static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ); + +#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ + +/* Update the state after handling the incoming certificate message. */ +static int ssl_tls13_process_certificate_postprocess( mbedtls_ssl_context *ssl ); + +/* + * Implementation + */ + +int mbedtls_ssl_tls13_process_certificate( mbedtls_ssl_context *ssl ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse certificate" ) ); + +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) + unsigned char *buf; + size_t buf_len; + + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls1_3_fetch_handshake_msg( + ssl, MBEDTLS_SSL_HS_CERTIFICATE, + &buf, &buf_len ) ); + + /* Parse the certificate chain sent by the peer. */ + MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_certificate( ssl, buf, buf + buf_len ) ); + /* Validate the certificate chain and set the verification results. */ + MBEDTLS_SSL_PROC_CHK( ssl_tls13_validate_certificate( ssl ) ); + + mbedtls_ssl_tls1_3_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_CERTIFICATE, + buf, buf_len ); + +#else + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); +#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ + + /* Update state */ + MBEDTLS_SSL_PROC_CHK( ssl_tls13_process_certificate_postprocess( ssl ) ); + +cleanup: + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse certificate" ) ); + return( ret ); +} + +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) +#if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) +/* + * Structure of Certificate message: + * + * enum { + * X509(0), + * RawPublicKey(2), + * (255) + * } CertificateType; + * + * struct { + * select (certificate_type) { + * case RawPublicKey: + * * From RFC 7250 ASN.1_subjectPublicKeyInfo * + * opaque ASN1_subjectPublicKeyInfo<1..2^24-1>; + * case X509: + * opaque cert_data<1..2^24-1>; + * }; + * Extension extensions<0..2^16-1>; + * } CertificateEntry; + * + * struct { + * opaque certificate_request_context<0..2^8-1>; + * CertificateEntry certificate_list<0..2^24-1>; + * } Certificate; + * + */ +static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + 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 ); + certificate_request_context_len = p[0]; + certificate_list_len = ( p[1] << 16 ) | ( p[2] << 8 ) | p[3]; + + /* 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 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate message" ) ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, + MBEDTLS_ERR_SSL_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } + + /* In case we tried to reuse a session but it failed */ + if( ssl->session_negotiate->peer_cert != NULL ) + { + mbedtls_x509_crt_free( ssl->session_negotiate->peer_cert ); + mbedtls_free( ssl->session_negotiate->peer_cert ); + } + + if( ( ssl->session_negotiate->peer_cert = + mbedtls_calloc( 1, sizeof( mbedtls_x509_crt ) ) ) == NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "alloc( %" MBEDTLS_PRINTF_SIZET " bytes ) failed", + sizeof( mbedtls_x509_crt ) ) ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_INTERNAL_ERROR, + MBEDTLS_ERR_SSL_ALLOC_FAILED ); + return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); + } + + mbedtls_x509_crt_init( ssl->session_negotiate->peer_cert ); + + p += 4; + certificate_list_end = p + certificate_list_len; + while ( p < certificate_list_end ) + { + size_t cert_data_len, extensions_len; + + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 3 ); + cert_data_len = ( ( size_t )p[0] << 16 ) | + ( ( size_t )p[1] << 8 ) | + ( ( size_t )p[2] ); + p += 3; + + /* In theory, the CRT can be up to 2^24 Bytes, but we don't support + * anything beyond 2^16 = 64K. Otherwise as in the TLS 1.2 code, + * check that we have a minimum of 128 bytes of data, this is not + * clear why we need that though. + */ + if( ( cert_data_len < 128 ) || ( cert_data_len >= 0x10000 ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad Certificate message" ) ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, + MBEDTLS_ERR_SSL_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } + + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, certificate_list_end, cert_data_len); + ret = mbedtls_x509_crt_parse_der( ssl->session_negotiate->peer_cert, + p, cert_data_len ); + + switch( ret ) + { + case 0: /*ok*/ + break; + case MBEDTLS_ERR_X509_UNKNOWN_SIG_ALG + MBEDTLS_ERR_OID_NOT_FOUND: + /* Ignore certificate with an unknown algorithm: maybe a + prior certificate was already trusted. */ + break; + + case MBEDTLS_ERR_X509_ALLOC_FAILED: + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_INTERNAL_ERROR, + MBEDTLS_ERR_X509_ALLOC_FAILED ); + MBEDTLS_SSL_DEBUG_RET( 1, " mbedtls_x509_crt_parse_der", ret ); + return( ret ); + + case MBEDTLS_ERR_X509_UNKNOWN_VERSION: + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT, + MBEDTLS_ERR_X509_UNKNOWN_VERSION ); + MBEDTLS_SSL_DEBUG_RET( 1, " mbedtls_x509_crt_parse_der", ret ); + return( ret ); + + default: + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_BAD_CERT, + ret ); + MBEDTLS_SSL_DEBUG_RET( 1, " mbedtls_x509_crt_parse_der", ret ); + return( ret ); + } + + p += cert_data_len; + + /* Certificate extensions length */ + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, certificate_list_end, 2 ); + extensions_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + p += 2; + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, certificate_list_end, extensions_len); + p += extensions_len; + } + + /* Check that all the message is consumed. */ + if( p != end ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad Certificate message" ) ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, \ + MBEDTLS_ERR_SSL_DECODE_ERROR ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } + + MBEDTLS_SSL_DEBUG_CRT( 3, "peer certificate", ssl->session_negotiate->peer_cert ); + + return( ret ); +} +#else +static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) +{ + ((void) ssl); + ((void) buf); + ((void) end); + return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); +} +#endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ +#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ + +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) +#if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) +static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) +{ + int ret = 0; + mbedtls_x509_crt *ca_chain; + mbedtls_x509_crl *ca_crl; + +#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) + if( ssl->handshake->sni_ca_chain != NULL ) + { + ca_chain = ssl->handshake->sni_ca_chain; + ca_crl = ssl->handshake->sni_ca_crl; + } + else +#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ + { + ca_chain = ssl->conf->ca_chain; + ca_crl = ssl->conf->ca_crl; + } + + /* + * Main check: verify certificate + */ + ret = mbedtls_x509_crt_verify_with_profile( + ssl->session_negotiate->peer_cert, + ca_chain, ca_crl, + ssl->conf->cert_profile, + ssl->hostname, + &ssl->session_negotiate->verify_result, + ssl->conf->f_vrfy, ssl->conf->p_vrfy ); + + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "x509_verify_cert", ret ); + } + + /* + * Secondary checks: always done, but change 'ret' only if it was 0 + */ + +#if defined(MBEDTLS_ECP_C) + { + const mbedtls_pk_context *pk = &ssl->session_negotiate->peer_cert->pk; + + /* If certificate uses an EC key, make sure the curve is OK */ + if( mbedtls_pk_can_do( pk, MBEDTLS_PK_ECKEY ) && + mbedtls_ssl_check_curve( ssl, mbedtls_pk_ec( *pk )->grp.id ) != 0 ) + { + ssl->session_negotiate->verify_result |= MBEDTLS_X509_BADCERT_BAD_KEY; + + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate ( EC key curve )" ) ); + if( ret == 0 ) + ret = MBEDTLS_ERR_SSL_BAD_CERTIFICATE; + } + } +#endif /* MBEDTLS_ECP_C */ + + if( mbedtls_ssl_check_cert_usage( ssl->session_negotiate->peer_cert, + ssl->handshake->ciphersuite_info, + !ssl->conf->endpoint, + &ssl->session_negotiate->verify_result ) != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate ( usage extensions )" ) ); + if( ret == 0 ) + ret = MBEDTLS_ERR_SSL_BAD_CERTIFICATE; + } + + + if( ca_chain == NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "got no CA chain" ) ); + ret = MBEDTLS_ERR_SSL_CA_CHAIN_REQUIRED; + } + + if( ret != 0 ) + { + /* The certificate may have been rejected for several reasons. + Pick one and send the corresponding alert. Which alert to send + may be a subject of debate in some cases. */ + if( ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_OTHER ) + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ACCESS_DENIED, ret ); + else if( ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_CN_MISMATCH ) + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_BAD_CERT, ret ); + else if( ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_KEY_USAGE ) + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT, ret ); + else if( ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_EXT_KEY_USAGE ) + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT, ret ); + else if( ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_NS_CERT_TYPE ) + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT, ret ); + else if( ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_BAD_PK ) + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT, ret ); + else if( ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_BAD_KEY ) + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT, ret ); + else if( ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_EXPIRED ) + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_CERT_EXPIRED, ret ); + else if( ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_REVOKED ) + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_CERT_REVOKED, ret ); + else if( ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_NOT_TRUSTED ) + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNKNOWN_CA, ret ); + else + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_CERT_UNKNOWN, ret ); + } + +#if defined(MBEDTLS_DEBUG_C) + if( ssl->session_negotiate->verify_result != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "! Certificate verification flags %08x", + (unsigned int) ssl->session_negotiate->verify_result ) ); + } + else + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "Certificate verification flags clear" ) ); + } +#endif /* MBEDTLS_DEBUG_C */ + + return( ret ); +} +#else /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ +static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) +{ + ((void) ssl); + return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); +} +#endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ +#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ + +static int ssl_tls13_process_certificate_postprocess( mbedtls_ssl_context *ssl ) +{ + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CERTIFICATE_VERIFY ); + return( 0 ); +} + #endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ #endif /* MBEDTLS_SSL_TLS_C */ From 79595acf3f078b3ab7efea088ced5060391be0e9 Mon Sep 17 00:00:00 2001 From: Xiaofei Bai Date: Tue, 26 Oct 2021 07:16:45 +0000 Subject: [PATCH 02/12] Update based on review comments. Signed-off-by: Xiaofei Bai --- library/ssl_tls13_generic.c | 105 ++++++++++++++---------------------- 1 file changed, 40 insertions(+), 65 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 8cae7898bf..6f2c3ec178 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -225,65 +225,10 @@ int mbedtls_ssl_tls13_write_sig_alg_ext( mbedtls_ssl_context *ssl, * */ -/* - * Overview - */ - -/* Main state-handling entry point; orchestrates the other functions. */ -int mbedtls_ssl_tls13_process_certificate( mbedtls_ssl_context *ssl ); - -#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) -/* Parse certificate chain send by the server. */ -static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, - const unsigned char *buf, - const unsigned char *end ); -/* Validate certificate chain sent by the server. */ -static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ); - -#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ - -/* Update the state after handling the incoming certificate message. */ -static int ssl_tls13_process_certificate_postprocess( mbedtls_ssl_context *ssl ); - /* * Implementation */ -int mbedtls_ssl_tls13_process_certificate( mbedtls_ssl_context *ssl ) -{ - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse certificate" ) ); - -#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) - unsigned char *buf; - size_t buf_len; - - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls1_3_fetch_handshake_msg( - ssl, MBEDTLS_SSL_HS_CERTIFICATE, - &buf, &buf_len ) ); - - /* Parse the certificate chain sent by the peer. */ - MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_certificate( ssl, buf, buf + buf_len ) ); - /* Validate the certificate chain and set the verification results. */ - MBEDTLS_SSL_PROC_CHK( ssl_tls13_validate_certificate( ssl ) ); - - mbedtls_ssl_tls1_3_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_CERTIFICATE, - buf, buf_len ); - -#else - MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); -#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ - - /* Update state */ - MBEDTLS_SSL_PROC_CHK( ssl_tls13_process_certificate_postprocess( ssl ) ); - -cleanup: - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse certificate" ) ); - return( ret ); -} - #if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) #if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) /* @@ -312,6 +257,8 @@ cleanup: * } Certificate; * */ + +/* Parse certificate chain send by the server. */ static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) @@ -325,11 +272,13 @@ static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 4 ); certificate_request_context_len = p[0]; certificate_list_len = ( p[1] << 16 ) | ( p[2] << 8 ) | p[3]; + 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( ( certificate_request_context_len != 0 ) || + ( certificate_list_len >= 0x10000 ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate message" ) ); MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, @@ -356,13 +305,12 @@ static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, mbedtls_x509_crt_init( ssl->session_negotiate->peer_cert ); - p += 4; certificate_list_end = p + certificate_list_len; - while ( p < certificate_list_end ) + while( p < certificate_list_end ) { size_t cert_data_len, extensions_len; - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 3 ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, certificate_list_end, 3 ); cert_data_len = ( ( size_t )p[0] << 16 ) | ( ( size_t )p[1] << 8 ) | ( ( size_t )p[2] ); @@ -374,14 +322,14 @@ static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, * clear why we need that though. */ if( ( cert_data_len < 128 ) || ( cert_data_len >= 0x10000 ) ) - { + { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad Certificate message" ) ); MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR, MBEDTLS_ERR_SSL_DECODE_ERROR ); return( MBEDTLS_ERR_SSL_DECODE_ERROR ); } - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, certificate_list_end, cert_data_len); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, certificate_list_end, cert_data_len ); ret = mbedtls_x509_crt_parse_der( ssl->session_negotiate->peer_cert, p, cert_data_len ); @@ -419,7 +367,7 @@ static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, MBEDTLS_SSL_CHK_BUF_READ_PTR( p, certificate_list_end, 2 ); extensions_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, certificate_list_end, extensions_len); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, certificate_list_end, extensions_len ); p += extensions_len; } @@ -451,6 +399,7 @@ static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) #if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) +/* Validate certificate chain sent by the server. */ static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) { int ret = 0; @@ -576,10 +525,36 @@ static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ #endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ -static int ssl_tls13_process_certificate_postprocess( mbedtls_ssl_context *ssl ) +int mbedtls_ssl_tls13_process_certificate( mbedtls_ssl_context *ssl ) { - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CERTIFICATE_VERIFY ); - return( 0 ); + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse certificate" ) ); + +#if defined(MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED) + unsigned char *buf; + size_t buf_len; + + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls1_3_fetch_handshake_msg( + ssl, MBEDTLS_SSL_HS_CERTIFICATE, + &buf, &buf_len ) ); + + /* Parse the certificate chain sent by the peer. */ + MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_certificate( ssl, buf, buf + buf_len ) ); + /* Validate the certificate chain and set the verification results. */ + MBEDTLS_SSL_PROC_CHK( ssl_tls13_validate_certificate( ssl ) ); + + mbedtls_ssl_tls1_3_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_CERTIFICATE, + buf, buf_len ); + +#else + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); +#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ + +cleanup: + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse certificate" ) ); + return( ret ); } #endif /* MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL */ From 10aeec0685c878601cc65aee4fc424be9f65e823 Mon Sep 17 00:00:00 2001 From: Xiaofei Bai Date: Tue, 26 Oct 2021 09:50:08 +0000 Subject: [PATCH 03/12] Fix a build error Signed-off-by: Xiaofei Bai --- library/ssl_tls13_generic.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 6f2c3ec178..0157726afb 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -546,14 +546,13 @@ int mbedtls_ssl_tls13_process_certificate( mbedtls_ssl_context *ssl ) mbedtls_ssl_tls1_3_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_CERTIFICATE, buf, buf_len ); -#else - MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); -#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ - cleanup: MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse certificate" ) ); +#else + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; +#endif /* MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED */ return( ret ); } From ff45602c7438b9900c44519f4a9c54d8d30c7a39 Mon Sep 17 00:00:00 2001 From: Xiaofei Bai Date: Thu, 28 Oct 2021 06:50:17 +0000 Subject: [PATCH 04/12] Add local variable verify_result Signed-off-by: Xiaofei Bai --- library/ssl_tls13_generic.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 0157726afb..026c94c7e7 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -405,6 +405,7 @@ static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) int ret = 0; mbedtls_x509_crt *ca_chain; mbedtls_x509_crl *ca_crl; + uint32_t verify_result = 0; #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) if( ssl->handshake->sni_ca_chain != NULL ) @@ -427,7 +428,7 @@ static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) ca_chain, ca_crl, ssl->conf->cert_profile, ssl->hostname, - &ssl->session_negotiate->verify_result, + &verify_result, ssl->conf->f_vrfy, ssl->conf->p_vrfy ); if( ret != 0 ) @@ -447,7 +448,7 @@ static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) if( mbedtls_pk_can_do( pk, MBEDTLS_PK_ECKEY ) && mbedtls_ssl_check_curve( ssl, mbedtls_pk_ec( *pk )->grp.id ) != 0 ) { - ssl->session_negotiate->verify_result |= MBEDTLS_X509_BADCERT_BAD_KEY; + verify_result |= MBEDTLS_X509_BADCERT_BAD_KEY; MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate ( EC key curve )" ) ); if( ret == 0 ) @@ -459,7 +460,7 @@ static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) if( mbedtls_ssl_check_cert_usage( ssl->session_negotiate->peer_cert, ssl->handshake->ciphersuite_info, !ssl->conf->endpoint, - &ssl->session_negotiate->verify_result ) != 0 ) + &verify_result ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate ( usage extensions )" ) ); if( ret == 0 ) @@ -478,35 +479,31 @@ static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) /* The certificate may have been rejected for several reasons. Pick one and send the corresponding alert. Which alert to send may be a subject of debate in some cases. */ - if( ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_OTHER ) + if( verify_result & MBEDTLS_X509_BADCERT_OTHER ) MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ACCESS_DENIED, ret ); - else if( ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_CN_MISMATCH ) + else if( verify_result & MBEDTLS_X509_BADCERT_CN_MISMATCH ) MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_BAD_CERT, ret ); - else if( ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_KEY_USAGE ) + else if( ( verify_result & MBEDTLS_X509_BADCERT_KEY_USAGE ) || + ( verify_result & MBEDTLS_X509_BADCERT_EXT_KEY_USAGE ) || + ( verify_result & MBEDTLS_X509_BADCERT_NS_CERT_TYPE ) || + ( verify_result & MBEDTLS_X509_BADCERT_BAD_PK ) || + ( verify_result & MBEDTLS_X509_BADCERT_BAD_KEY ) ) MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT, ret ); - else if( ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_EXT_KEY_USAGE ) - MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT, ret ); - else if( ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_NS_CERT_TYPE ) - MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT, ret ); - else if( ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_BAD_PK ) - MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT, ret ); - else if( ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_BAD_KEY ) - MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT, ret ); - else if( ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_EXPIRED ) + else if( verify_result & MBEDTLS_X509_BADCERT_EXPIRED ) MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_CERT_EXPIRED, ret ); - else if( ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_REVOKED ) + else if( verify_result & MBEDTLS_X509_BADCERT_REVOKED ) MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_CERT_REVOKED, ret ); - else if( ssl->session_negotiate->verify_result & MBEDTLS_X509_BADCERT_NOT_TRUSTED ) + else if( verify_result & MBEDTLS_X509_BADCERT_NOT_TRUSTED ) MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNKNOWN_CA, ret ); else MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_CERT_UNKNOWN, ret ); } #if defined(MBEDTLS_DEBUG_C) - if( ssl->session_negotiate->verify_result != 0 ) + if( verify_result != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "! Certificate verification flags %08x", - (unsigned int) ssl->session_negotiate->verify_result ) ); + verify_result ) ); } else { @@ -514,6 +511,7 @@ static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) } #endif /* MBEDTLS_DEBUG_C */ + ssl->session_negotiate->verify_result = verify_result; return( ret ); } #else /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ From 937ac673fae8a06143753ae4e1b1129fd0636af1 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 28 Oct 2021 17:39:28 +0800 Subject: [PATCH 05/12] Disable client cert for gnutls tests Signed-off-by: Jerry Yu --- tests/ssl-opt.sh | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index f9bfec2e1e..70b9f4b7a0 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -8763,6 +8763,7 @@ run_test "export keys functionality" \ # openssl feature tests: check if tls1.3 exists. requires_openssl_tls1_3 +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL run_test "TLS1.3: Test openssl tls1_3 feature" \ "$O_NEXT_SRV -tls1_3 -msg" \ "$O_NEXT_CLI -tls1_3 -msg" \ @@ -8774,8 +8775,9 @@ run_test "TLS1.3: Test openssl tls1_3 feature" \ requires_gnutls_tls1_3 requires_gnutls_next_no_ticket requires_gnutls_next_disable_tls13_compat +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL run_test "TLS1.3: Test gnutls tls1_3 feature" \ - "$G_NEXT_SRV --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:%NO_TICKETS:%DISABLE_TLS13_COMPAT_MODE" \ + "$G_NEXT_SRV --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:+CIPHER-ALL:%NO_TICKETS:%DISABLE_TLS13_COMPAT_MODE --disable-client-cert " \ "$G_NEXT_CLI localhost --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:%NO_TICKETS:%DISABLE_TLS13_COMPAT_MODE -V" \ 0 \ -s "Version: TLS1.3" \ @@ -8832,10 +8834,12 @@ run_test "TLS1.3: Test client hello msg work - openssl" \ -c "<= parse encrypted extensions" requires_gnutls_tls1_3 +requires_gnutls_next_no_ticket +requires_gnutls_next_disable_tls13_compat requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3_EXPERIMENTAL requires_config_disabled MBEDTLS_USE_PSA_CRYPTO run_test "TLS1.3: Test client hello msg work - gnutls" \ - "$G_NEXT_SRV --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:%DISABLE_TLS13_COMPAT_MODE --debug=4" \ + "$G_NEXT_SRV --debug=4 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:+CIPHER-ALL:%NO_TICKETS:%DISABLE_TLS13_COMPAT_MODE --disable-client-cert" \ "$P_CLI debug_level=3 min_version=tls1_3 max_version=tls1_3" \ 1 \ -c "SSL - The requested feature is not available" \ From a93ac116c80e7955b6c0d8892e7dfeb9fd31bb84 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Wed, 27 Oct 2021 16:31:48 +0800 Subject: [PATCH 06/12] Remove certificate_request state Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 44c7b1ecc9..7be69fac3a 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1523,20 +1523,19 @@ static int ssl_tls13_parse_encrypted_extensions( mbedtls_ssl_context *ssl, static int ssl_tls13_postprocess_encrypted_extensions( mbedtls_ssl_context *ssl ) { - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CERTIFICATE_REQUEST ); - return( 0 ); -} - -/* - * Handler for MBEDTLS_SSL_CERTIFICATE_REQUEST - */ -static int ssl_tls1_3_process_certificate_request( mbedtls_ssl_context *ssl ) -{ - MBEDTLS_SSL_DEBUG_MSG( 1, ( "%s hasn't been implemented", __func__ ) ); - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_CERTIFICATE ); +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) + if( mbedtls_ssl_tls1_3_some_psk_enabled( ssl ) ) + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_FINISHED ); + else + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_CERTIFICATE ); +#else + ((void) ssl); + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_FINISHED ); +#endif return( 0 ); } +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) /* * Handler for MBEDTLS_SSL_SERVER_CERTIFICATE */ @@ -1561,7 +1560,7 @@ static int ssl_tls1_3_process_certificate_verify( mbedtls_ssl_context *ssl ) mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_FINISHED ); return( 0 ); } - +#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ /* * Handler for MBEDTLS_SSL_SERVER_FINISHED */ @@ -1647,10 +1646,7 @@ int mbedtls_ssl_tls13_handshake_client_step( mbedtls_ssl_context *ssl ) ret = ssl_tls13_process_encrypted_extensions( ssl ); break; - case MBEDTLS_SSL_CERTIFICATE_REQUEST: - ret = ssl_tls1_3_process_certificate_request( ssl ); - break; - +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) case MBEDTLS_SSL_SERVER_CERTIFICATE: ret = ssl_tls1_3_process_server_certificate( ssl ); break; @@ -1658,6 +1654,7 @@ int mbedtls_ssl_tls13_handshake_client_step( mbedtls_ssl_context *ssl ) case MBEDTLS_SSL_CERTIFICATE_VERIFY: ret = ssl_tls1_3_process_certificate_verify( ssl ); break; +#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ case MBEDTLS_SSL_SERVER_FINISHED: ret = ssl_tls1_3_process_server_finished( ssl ); From 7aa71860221aaa27c5a7594bf9dbb8a3e2b199f2 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 28 Oct 2021 21:41:30 +0800 Subject: [PATCH 07/12] fix various issues Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 4 ++-- library/ssl_tls13_generic.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 7be69fac3a..49ca7f0b8b 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1501,7 +1501,7 @@ static int ssl_tls13_parse_encrypted_extensions( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "unsupported extension found: %u ", extension_type) ); MBEDTLS_SSL_PEND_FATAL_ALERT( - MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_EXT, \ + MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_EXT, MBEDTLS_ERR_SSL_UNSUPPORTED_EXTENSION ); return ( MBEDTLS_ERR_SSL_UNSUPPORTED_EXTENSION ); } @@ -1513,7 +1513,7 @@ static int ssl_tls13_parse_encrypted_extensions( mbedtls_ssl_context *ssl, if( p != end ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "EncryptedExtension lengths misaligned" ) ); - MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_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 026c94c7e7..598b2bc375 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -271,7 +271,7 @@ static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 4 ); certificate_request_context_len = p[0]; - certificate_list_len = ( p[1] << 16 ) | ( p[2] << 8 ) | p[3]; + certificate_list_len = MBEDTLS_GET_UINT24_BE( p, 0 ); p += 4; /* In theory, the certificate list can be up to 2^24 Bytes, but we don't From 83bb13101ab37dbe78135fade230b865d1105d22 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Thu, 28 Oct 2021 22:16:33 +0800 Subject: [PATCH 08/12] fix format warning Signed-off-by: Jerry Yu --- library/ssl_tls13_generic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 598b2bc375..18bb984178 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -503,7 +503,7 @@ static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) if( verify_result != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "! Certificate verification flags %08x", - verify_result ) ); + (unsigned int) verify_result ) ); } else { From b640bf6c15c87d0c2dea1f27632052f39236842e Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 29 Oct 2021 10:05:32 +0800 Subject: [PATCH 09/12] fix CI build fail Signed-off-by: Jerry Yu --- library/ssl_tls13_generic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 18bb984178..9e643d478d 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -271,7 +271,7 @@ static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 4 ); certificate_request_context_len = p[0]; - certificate_list_len = MBEDTLS_GET_UINT24_BE( 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 From d2674314a3a783cc4fb639512916eeacb4f9be0a Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 29 Oct 2021 10:08:19 +0800 Subject: [PATCH 10/12] Restore certificate_request state Signed-off-by: Jerry Yu --- library/ssl_tls13_client.c | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 49ca7f0b8b..f1a31cab6b 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1527,7 +1527,7 @@ static int ssl_tls13_postprocess_encrypted_extensions( mbedtls_ssl_context *ssl if( mbedtls_ssl_tls1_3_some_psk_enabled( ssl ) ) mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_FINISHED ); else - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_CERTIFICATE ); + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CERTIFICATE_REQUEST ); #else ((void) ssl); mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_FINISHED ); @@ -1536,6 +1536,34 @@ static int ssl_tls13_postprocess_encrypted_extensions( 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 = mbedtls_ssl_read_record( ssl, 0 ); + + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret ); + return( ret ); + } + + 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 ); + } + + ssl->keep_current_message = 1; + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_CERTIFICATE ); + + return( 0 ); +} + /* * Handler for MBEDTLS_SSL_SERVER_CERTIFICATE */ @@ -1647,6 +1675,10 @@ int mbedtls_ssl_tls13_handshake_client_step( mbedtls_ssl_context *ssl ) break; #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) + case MBEDTLS_SSL_CERTIFICATE_REQUEST: + ret = ssl_tls13_process_certificate_request( ssl ); + break; + case MBEDTLS_SSL_SERVER_CERTIFICATE: ret = ssl_tls1_3_process_server_certificate( ssl ); break; From 1df3db04676851fc1e2d3d3f252a9c9ed19c4430 Mon Sep 17 00:00:00 2001 From: Jerry Yu Date: Fri, 29 Oct 2021 10:18:43 +0800 Subject: [PATCH 11/12] Add certificate success check Signed-off-by: Jerry Yu --- tests/ssl-opt.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 70b9f4b7a0..993021013c 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -8831,6 +8831,7 @@ run_test "TLS1.3: Test client hello msg work - openssl" \ -c "server hello, chosen ciphersuite: ( 1301 ) - TLS1-3-AES-128-GCM-SHA256" \ -c "ECDH curve: x25519" \ -c "=> ssl_tls1_3_process_server_hello" \ + -c "Certificate verification flags clear" \ -c "<= parse encrypted extensions" requires_gnutls_tls1_3 @@ -8860,6 +8861,7 @@ run_test "TLS1.3: Test client hello msg work - gnutls" \ -c "server hello, chosen ciphersuite: ( 1301 ) - TLS1-3-AES-128-GCM-SHA256" \ -c "ECDH curve: x25519" \ -c "=> ssl_tls1_3_process_server_hello" \ + -c "Certificate verification flags clear" \ -c "<= parse encrypted extensions" # Test heap memory usage after handshake From f93cbd267443e27e9b5e3a53742fb0f20cf6a5ea Mon Sep 17 00:00:00 2001 From: Xiaofei Bai Date: Fri, 29 Oct 2021 02:39:30 +0000 Subject: [PATCH 12/12] fix some format issues Signed-off-by: Xiaofei Bai --- library/ssl_tls13_client.c | 2 +- library/ssl_tls13_generic.c | 14 ++++++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index f1a31cab6b..0fb09c4ced 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1572,7 +1572,7 @@ static int ssl_tls1_3_process_server_certificate( mbedtls_ssl_context *ssl ) int ret; ret = mbedtls_ssl_tls13_process_certificate( ssl ); - if( ret != 0) + if( ret != 0 ) return( ret ); mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CERTIFICATE_VERIFY ); diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 9e643d478d..c8601ce17e 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -311,9 +311,7 @@ static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl, size_t cert_data_len, extensions_len; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, certificate_list_end, 3 ); - cert_data_len = ( ( size_t )p[0] << 16 ) | - ( ( size_t )p[1] << 8 ) | - ( ( size_t )p[2] ); + cert_data_len = MBEDTLS_GET_UINT24_BE( p, 0 ); p += 3; /* In theory, the CRT can be up to 2^24 Bytes, but we don't support @@ -483,11 +481,11 @@ static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ACCESS_DENIED, ret ); else if( verify_result & MBEDTLS_X509_BADCERT_CN_MISMATCH ) MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_BAD_CERT, ret ); - else if( ( verify_result & MBEDTLS_X509_BADCERT_KEY_USAGE ) || - ( verify_result & MBEDTLS_X509_BADCERT_EXT_KEY_USAGE ) || - ( verify_result & MBEDTLS_X509_BADCERT_NS_CERT_TYPE ) || - ( verify_result & MBEDTLS_X509_BADCERT_BAD_PK ) || - ( verify_result & MBEDTLS_X509_BADCERT_BAD_KEY ) ) + else if( verify_result & ( MBEDTLS_X509_BADCERT_KEY_USAGE | + MBEDTLS_X509_BADCERT_EXT_KEY_USAGE | + MBEDTLS_X509_BADCERT_NS_CERT_TYPE | + MBEDTLS_X509_BADCERT_BAD_PK | + MBEDTLS_X509_BADCERT_BAD_KEY ) ) MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_CERT, ret ); else if( verify_result & MBEDTLS_X509_BADCERT_EXPIRED ) MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_CERT_EXPIRED, ret );