From 9ca09d497f4bccd951def9f0cf6abe9838befd08 Mon Sep 17 00:00:00 2001 From: Xiaofei Bai Date: Mon, 14 Feb 2022 12:57:18 +0000 Subject: [PATCH 01/11] Add writing CertificateRequest msg on server side Signed-off-by: Xiaofei Bai --- library/ssl_tls13_server.c | 140 +++++++++++++++++++++++++++++++++++++ 1 file changed, 140 insertions(+) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index b2a5cfcf5..4e2a0dd23 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -25,6 +25,10 @@ #include "mbedtls/error.h" #include "mbedtls/platform.h" +#include "ssl_misc.h" +#include "ssl_tls13_keys.h" +#include "ssl_debug_helpers.h" + #if defined(MBEDTLS_ECP_C) #include "mbedtls/ecp.h" #endif /* MBEDTLS_ECP_C */ @@ -729,6 +733,136 @@ cleanup: return( ret ); } +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) +#define SSL_CERTIFICATE_REQUEST_SEND_REQUEST 0 +#define SSL_CERTIFICATE_REQUEST_SKIP 1 +/* Coordination: + * Check whether a CertificateRequest message should be written. + * Returns a negative code on failure, or + * - SSL_CERTIFICATE_REQUEST_SEND_REQUEST + * - SSL_CERTIFICATE_REQUEST_SKIP + * indicating if the writing of the CertificateRequest + * should be skipped or not. + */ +static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) +{ + int authmode; + + if( mbedtls_ssl_tls13_some_psk_enabled( ssl ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, ( "<= skip write certificate request" ) ); + return( SSL_CERTIFICATE_REQUEST_SKIP ); + } +#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) + if( ssl->handshake->sni_authmode != MBEDTLS_SSL_VERIFY_UNSET ) + authmode = ssl->handshake->sni_authmode; + else +#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ + authmode = ssl->conf->authmode; + + if( authmode == MBEDTLS_SSL_VERIFY_NONE ) + return( SSL_CERTIFICATE_REQUEST_SKIP ); + + return( SSL_CERTIFICATE_REQUEST_SEND_REQUEST ); +} + +/* + * struct { + * opaque certificate_request_context<0..2^8-1>; + * Extension extensions<2..2^16-1>; + * } CertificateRequest; + * + */ +static int ssl_tls13_write_certificate_request( mbedtls_ssl_context *ssl, + unsigned char *buf, + const unsigned char *end, + size_t *out_len ) +{ + int ret; + size_t ext_size = 0; + unsigned char *p = buf; + + *out_len = 0; + + /* Check if we have enough space: + * - certificate_request_context (1 byte) + * - extensions (2 bytes) + */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 3 ); + + /* + * Write certificate_request_context + */ + /* + * We use a zero length context for the normal handshake + * messages. For post-authentication handshake messages + * this request context would be set to a non-zero value. + */ + *p++ = 0x0; + + /* + * Write extensions + */ + /* The extensions must contain the signature_algorithms. */ + ret = mbedtls_ssl_write_sig_alg_ext( ssl, p + 2, end, &ext_size ); + if( ret != 0 ) + return( ret ); + + /* length field for all extensions */ + MBEDTLS_PUT_UINT16_BE( ext_size, p, 0 ); + p += 2 + ext_size; + + *out_len = p - buf; + + return( ret ); +} + +static int ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write certificate request" ) ); + + MBEDTLS_SSL_PROC_CHK_NEG( ssl_tls13_certificate_request_coordinate( ssl ) ); + + if( ret == SSL_CERTIFICATE_REQUEST_SEND_REQUEST ) + { + unsigned char *buf; + size_t buf_len, msg_len; + + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_start_handshake_msg( ssl, + MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, &buf, &buf_len ) ); + + MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_certificate_request( + ssl, buf, buf + buf_len, &msg_len ) ); + + mbedtls_ssl_tls13_add_hs_msg_to_checksum( + ssl, MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, buf, msg_len ); + + /* TODO: Logically this should come at the end, but the non-MPS msg + * layer impl'n of mbedtls_ssl_tls13_finish_handshake_msg() can fail. */ + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_finish_handshake_msg( + ssl, buf_len, msg_len ) ); + } + else if( ret == SSL_CERTIFICATE_REQUEST_SKIP ) + { + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip write certificate request" ) ); + ret = 0; + } + else + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_CERTIFICATE ); +cleanup: + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write certificate request" ) ); + return( ret ); +} +#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ + /* * Handler for MBEDTLS_SSL_SERVER_HELLO */ @@ -1195,6 +1329,12 @@ int mbedtls_ssl_tls13_handshake_server_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; +#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ + default: MBEDTLS_SSL_DEBUG_MSG( 1, ( "invalid state %d", ssl->state ) ); return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); From 5ee73d84a976a4469cf029fdbc14ba2ddff256b3 Mon Sep 17 00:00:00 2001 From: Xiaofei Bai Date: Mon, 14 Mar 2022 02:48:30 +0000 Subject: [PATCH 02/11] Address review comments Signed-off-by: Xiaofei Bai --- library/ssl_tls13_server.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 4e2a0dd23..a5a26d476 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -773,20 +773,20 @@ static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) * } CertificateRequest; * */ -static int ssl_tls13_write_certificate_request( mbedtls_ssl_context *ssl, - unsigned char *buf, - const unsigned char *end, - size_t *out_len ) +static int ssl_tls13_write_certificate_request_body( mbedtls_ssl_context *ssl, + unsigned char *buf, + const unsigned char *end, + size_t *out_len ) { - int ret; - size_t ext_size = 0; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + size_t extensions_len = 0; unsigned char *p = buf; *out_len = 0; /* Check if we have enough space: * - certificate_request_context (1 byte) - * - extensions (2 bytes) + * - extensions length (2 bytes) */ MBEDTLS_SSL_CHK_BUF_PTR( p, end, 3 ); @@ -804,20 +804,20 @@ static int ssl_tls13_write_certificate_request( mbedtls_ssl_context *ssl, * Write extensions */ /* The extensions must contain the signature_algorithms. */ - ret = mbedtls_ssl_write_sig_alg_ext( ssl, p + 2, end, &ext_size ); + ret = mbedtls_ssl_write_sig_alg_ext( ssl, p + 2, end, &extensions_len ); if( ret != 0 ) return( ret ); /* length field for all extensions */ - MBEDTLS_PUT_UINT16_BE( ext_size, p, 0 ); - p += 2 + ext_size; + MBEDTLS_PUT_UINT16_BE( extensions_len, p, 0 ); + p += 2 + extensions_len; *out_len = p - buf; - return( ret ); + return( 0 ); } -static int ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ) +static int ssl_tls13_write_certificate_request( mbedtls_ssl_context *ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -833,14 +833,12 @@ static int ssl_tls13_process_certificate_request( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_start_handshake_msg( ssl, MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, &buf, &buf_len ) ); - MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_certificate_request( + MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_certificate_request_body( ssl, buf, buf + buf_len, &msg_len ) ); mbedtls_ssl_tls13_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, buf, msg_len ); - /* TODO: Logically this should come at the end, but the non-MPS msg - * layer impl'n of mbedtls_ssl_tls13_finish_handshake_msg() can fail. */ MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_finish_handshake_msg( ssl, buf_len, msg_len ) ); } @@ -1331,7 +1329,7 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) case MBEDTLS_SSL_CERTIFICATE_REQUEST: - ret = ssl_tls13_process_certificate_request( ssl ); + ret = ssl_tls13_write_certificate_request( ssl ); break; #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ From eaf3651e31da1f5e1e8d2bb04f3426a3a5de5678 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Sun, 24 Apr 2022 09:07:44 +0000 Subject: [PATCH 03/11] Rebase and solve conflicts Change handshake_msg related functions Share the ssl_write_sig_alg_ext Change-Id: I3d342baac302aa1d87c6f3ef75d85c7dc030070c Signed-off-by: XiaokangQian --- library/ssl_client.c | 106 +------------------------------------ library/ssl_misc.h | 3 ++ library/ssl_tls.c | 104 ++++++++++++++++++++++++++++++++++++ library/ssl_tls13_server.c | 6 +-- 4 files changed, 111 insertions(+), 108 deletions(-) diff --git a/library/ssl_client.c b/library/ssl_client.c index 0c32f07bf..22ca57cab 100644 --- a/library/ssl_client.c +++ b/library/ssl_client.c @@ -308,110 +308,6 @@ static int ssl_write_supported_groups_ext( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C || MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ -#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) -/* - * Function for writing a signature algorithm extension. - * - * The `extension_data` field of signature algorithm contains a `SignatureSchemeList` - * value (TLS 1.3 RFC8446): - * enum { - * .... - * ecdsa_secp256r1_sha256( 0x0403 ), - * ecdsa_secp384r1_sha384( 0x0503 ), - * ecdsa_secp521r1_sha512( 0x0603 ), - * .... - * } SignatureScheme; - * - * struct { - * SignatureScheme supported_signature_algorithms<2..2^16-2>; - * } SignatureSchemeList; - * - * The `extension_data` field of signature algorithm contains a `SignatureAndHashAlgorithm` - * value (TLS 1.2 RFC5246): - * enum { - * none(0), md5(1), sha1(2), sha224(3), sha256(4), sha384(5), - * sha512(6), (255) - * } HashAlgorithm; - * - * enum { anonymous(0), rsa(1), dsa(2), ecdsa(3), (255) } - * SignatureAlgorithm; - * - * struct { - * HashAlgorithm hash; - * SignatureAlgorithm signature; - * } SignatureAndHashAlgorithm; - * - * SignatureAndHashAlgorithm - * supported_signature_algorithms<2..2^16-2>; - * - * The TLS 1.3 signature algorithm extension was defined to be a compatible - * generalization of the TLS 1.2 signature algorithm extension. - * `SignatureAndHashAlgorithm` field of TLS 1.2 can be represented by - * `SignatureScheme` field of TLS 1.3 - * - */ -static int ssl_write_sig_alg_ext( mbedtls_ssl_context *ssl, unsigned char *buf, - const 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; - - 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; - const uint16_t *sig_alg = mbedtls_ssl_get_sig_algs( ssl ); - if( sig_alg == NULL ) - return( MBEDTLS_ERR_SSL_BAD_CONFIG ); - - for( ; *sig_alg != MBEDTLS_TLS1_3_SIG_NONE; sig_alg++ ) - { - if( ! mbedtls_ssl_sig_alg_is_supported( ssl, *sig_alg ) ) - continue; - 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; - -#if defined(MBEDTLS_SSL_PROTO_TLS1_3) - ssl->handshake->extensions_present |= MBEDTLS_SSL_EXT_SIG_ALG; -#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ - return( 0 ); -} -#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ - static int ssl_write_client_hello_cipher_suites( mbedtls_ssl_context *ssl, unsigned char *buf, @@ -721,7 +617,7 @@ static int ssl_write_client_hello_body( mbedtls_ssl_context *ssl, #endif 0 ) { - ret = ssl_write_sig_alg_ext( ssl, p, end, &output_len ); + ret = mbedtls_ssl_write_sig_alg_ext( ssl, p, end, &output_len ); if( ret != 0 ) return( ret ); p += output_len; diff --git a/library/ssl_misc.h b/library/ssl_misc.h index e8acc238d..4d8c479d4 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -2290,4 +2290,7 @@ int mbedtls_ssl_validate_ciphersuite( mbedtls_ssl_protocol_version min_tls_version, mbedtls_ssl_protocol_version max_tls_version ); +int mbedtls_ssl_write_sig_alg_ext( mbedtls_ssl_context *ssl, unsigned char *buf, + const unsigned char *end, size_t *out_len ); + #endif /* ssl_misc.h */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 250bae90f..1a62eff03 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7998,4 +7998,108 @@ int mbedtls_ssl_validate_ciphersuite( return( 0 ); } +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) +/* + * Function for writing a signature algorithm extension. + * + * The `extension_data` field of signature algorithm contains a `SignatureSchemeList` + * value (TLS 1.3 RFC8446): + * enum { + * .... + * ecdsa_secp256r1_sha256( 0x0403 ), + * ecdsa_secp384r1_sha384( 0x0503 ), + * ecdsa_secp521r1_sha512( 0x0603 ), + * .... + * } SignatureScheme; + * + * struct { + * SignatureScheme supported_signature_algorithms<2..2^16-2>; + * } SignatureSchemeList; + * + * The `extension_data` field of signature algorithm contains a `SignatureAndHashAlgorithm` + * value (TLS 1.2 RFC5246): + * enum { + * none(0), md5(1), sha1(2), sha224(3), sha256(4), sha384(5), + * sha512(6), (255) + * } HashAlgorithm; + * + * enum { anonymous(0), rsa(1), dsa(2), ecdsa(3), (255) } + * SignatureAlgorithm; + * + * struct { + * HashAlgorithm hash; + * SignatureAlgorithm signature; + * } SignatureAndHashAlgorithm; + * + * SignatureAndHashAlgorithm + * supported_signature_algorithms<2..2^16-2>; + * + * The TLS 1.3 signature algorithm extension was defined to be a compatible + * generalization of the TLS 1.2 signature algorithm extension. + * `SignatureAndHashAlgorithm` field of TLS 1.2 can be represented by + * `SignatureScheme` field of TLS 1.3 + * + */ +int mbedtls_ssl_write_sig_alg_ext( mbedtls_ssl_context *ssl, unsigned char *buf, + const 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; + + 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; + const uint16_t *sig_alg = mbedtls_ssl_get_sig_algs( ssl ); + if( sig_alg == NULL ) + return( MBEDTLS_ERR_SSL_BAD_CONFIG ); + + for( ; *sig_alg != MBEDTLS_TLS1_3_SIG_NONE; sig_alg++ ) + { + if( ! mbedtls_ssl_sig_alg_is_supported( ssl, *sig_alg ) ) + continue; + 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; + +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) + ssl->handshake->extensions_present |= MBEDTLS_SSL_EXT_SIG_ALG; +#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ + return( 0 ); +} +#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ + #endif /* MBEDTLS_SSL_TLS_C */ diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index a5a26d476..31d7508f8 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -830,16 +830,16 @@ static int ssl_tls13_write_certificate_request( mbedtls_ssl_context *ssl ) unsigned char *buf; size_t buf_len, msg_len; - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_start_handshake_msg( ssl, + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_start_handshake_msg( ssl, MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, &buf, &buf_len ) ); MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_certificate_request_body( ssl, buf, buf + buf_len, &msg_len ) ); - mbedtls_ssl_tls13_add_hs_msg_to_checksum( + mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, buf, msg_len ); - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_finish_handshake_msg( + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_finish_handshake_msg( ssl, buf_len, msg_len ) ); } else if( ret == SSL_CERTIFICATE_REQUEST_SKIP ) From 9dc44506476d12fc91a18713dd174d4f2539110c Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 27 Apr 2022 02:08:02 +0000 Subject: [PATCH 04/11] Fix commets issue about coding styles Change-Id: I930a062e137562e0b129b9b9b191e5c864f8104d Signed-off-by: XiaokangQian --- library/ssl_tls13_server.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 31d7508f8..079a0a841 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -850,7 +850,8 @@ static int ssl_tls13_write_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_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_CERTIFICATE ); From 1f1f1e3372cbe6e4b09d2ac19b6c2a167d8a5c6b Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 27 Apr 2022 08:56:03 +0000 Subject: [PATCH 05/11] Temp change to align with client/server hello style Change-Id: I8befbbcb5d6f7fdb230022825dcb856e19d9bec0 Signed-off-by: XiaokangQian --- library/ssl_tls13_server.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 079a0a841..d911e9021 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -753,12 +753,7 @@ static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 3, ( "<= skip write certificate request" ) ); return( SSL_CERTIFICATE_REQUEST_SKIP ); } -#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) - if( ssl->handshake->sni_authmode != MBEDTLS_SSL_VERIFY_UNSET ) - authmode = ssl->handshake->sni_authmode; - else -#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ - authmode = ssl->conf->authmode; + authmode = ssl->conf->authmode; if( authmode == MBEDTLS_SSL_VERIFY_NONE ) return( SSL_CERTIFICATE_REQUEST_SKIP ); @@ -779,8 +774,9 @@ static int ssl_tls13_write_certificate_request_body( mbedtls_ssl_context *ssl, size_t *out_len ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - size_t extensions_len = 0; unsigned char *p = buf; + size_t extensions_len = 0; + unsigned char *p_extensions_len; *out_len = 0; @@ -804,13 +800,15 @@ static int ssl_tls13_write_certificate_request_body( mbedtls_ssl_context *ssl, * Write extensions */ /* The extensions must contain the signature_algorithms. */ - ret = mbedtls_ssl_write_sig_alg_ext( ssl, p + 2, end, &extensions_len ); + p_extensions_len = p; + p += 2; + ret = mbedtls_ssl_write_sig_alg_ext( ssl, p, end, &extensions_len ); if( ret != 0 ) return( ret ); /* length field for all extensions */ - MBEDTLS_PUT_UINT16_BE( extensions_len, p, 0 ); - p += 2 + extensions_len; + MBEDTLS_PUT_UINT16_BE( extensions_len, p_extensions_len, 0 ); + p += extensions_len; *out_len = p - buf; From 2f150e184f11c67b8aecb8abe52031f3d65d146b Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Fri, 29 Apr 2022 02:01:19 +0000 Subject: [PATCH 06/11] Update status and add test cases for client certificate request Change-Id: If9b9672540d2b427496b7297aa484b8bcfeb75c5 Signed-off-by: XiaokangQian --- library/ssl_tls13_server.c | 7 +++++++ tests/ssl-opt.sh | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index d911e9021..4a4400338 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1264,6 +1264,13 @@ static int ssl_tls13_write_encrypted_extensions( mbedtls_ssl_context *ssl ) mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_ENCRYPTED_EXTENSIONS, buf, msg_len ); + /* Update state */ + MBEDTLS_SSL_PROC_CHK_NEG( ssl_tls13_certificate_request_coordinate( ssl ) ); + if( ret == SSL_CERTIFICATE_REQUEST_SEND_REQUEST ) + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CERTIFICATE_REQUEST ); + else + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_CERTIFICATE ); + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_finish_handshake_msg( ssl, buf_len, msg_len ) ); diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 07ad1b3fd..fb1b4c705 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -11312,6 +11312,23 @@ run_test "TLS 1.3: Server side check - openssl" \ -s "=> parse client hello" \ -s "<= parse client hello" +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_config_enabled MBEDTLS_DEBUG_C +requires_config_enabled MBEDTLS_SSL_SRV_C +requires_openssl_tls1_3 +run_test "TLS 1.3: Server side check - openssl with cient authentication" \ + "$P_SRV debug_level=4 auth_mode=required crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ + "$O_NEXT_CLI -msg -debug -cert data_files/server5.crt -key data_files/server5.key -tls1_3" \ + 1 \ + -s "tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ + -s "tls13 server state: MBEDTLS_SSL_CERTIFICATE_REQUEST" \ + -s "tls13 server state: MBEDTLS_SSL_SERVER_CERTIFICATE" \ + -s "SSL - The requested feature is not available" \ + -s "=> parse client hello" \ + -s "<= parse client hello" + requires_gnutls_tls1_3 requires_gnutls_next_no_ticket requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 @@ -11329,6 +11346,24 @@ run_test "TLS 1.3: Server side check - gnutls" \ -s "=> parse client hello" \ -s "<= parse client hello" +requires_gnutls_tls1_3 +requires_gnutls_next_no_ticket +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_config_enabled MBEDTLS_DEBUG_C +requires_config_enabled MBEDTLS_SSL_SRV_C +run_test "TLS 1.3: Server side check - gnutls with cient authentication" \ + "$P_SRV debug_level=4 auth_mode=required crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ + "$G_NEXT_CLI localhost -d 4 --x509certfile data_files/server5.crt --x509keyfile data_files/server5.key --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:%NO_TICKETS:%DISABLE_TLS13_COMPAT_MODE -V" \ + 1 \ + -s "tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ + -s "tls13 server state: MBEDTLS_SSL_CERTIFICATE_REQUEST" \ + -s "tls13 server state: MBEDTLS_SSL_SERVER_CERTIFICATE" \ + -s "SSL - The requested feature is not available" \ + -s "=> parse client hello" \ + -s "<= parse client hello" + requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_DEBUG_C requires_config_enabled MBEDTLS_SSL_SRV_C From 45c22201b318185a48db9edf798e440a7ccad840 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Fri, 6 May 2022 06:54:09 +0000 Subject: [PATCH 07/11] Update test cases and encrypted extension state set Change-Id: Ie1acd10b61cefa9414169b276a0c5c5ff2f9eb79 Signed-off-by: XiaokangQian --- library/ssl_tls13_server.c | 19 +++++++++++-------- tests/ssl-opt.sh | 17 +++++++++++++++++ 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 4a4400338..01dff8c9c 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1264,21 +1264,24 @@ static int ssl_tls13_write_encrypted_extensions( mbedtls_ssl_context *ssl ) mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_ENCRYPTED_EXTENSIONS, buf, msg_len ); - /* Update state */ - MBEDTLS_SSL_PROC_CHK_NEG( ssl_tls13_certificate_request_coordinate( ssl ) ); - if( ret == SSL_CERTIFICATE_REQUEST_SEND_REQUEST ) - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CERTIFICATE_REQUEST ); - else - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_CERTIFICATE ); - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_finish_handshake_msg( ssl, buf_len, msg_len ) ); + /* Update state */ #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) if( mbedtls_ssl_tls13_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_PROC_CHK_NEG( ssl_tls13_certificate_request_coordinate( ssl ) ); + if( ret == SSL_CERTIFICATE_REQUEST_SEND_REQUEST ) + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CERTIFICATE_REQUEST ); + else + { + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_CERTIFICATE ); + ret = 0; + } + } #else mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_FINISHED ); #endif diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index fb1b4c705..762a963e8 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -11381,6 +11381,23 @@ run_test "TLS 1.3: Server side check - mbedtls" \ -s "=> parse client hello" \ -s "<= parse client hello" +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_config_enabled MBEDTLS_DEBUG_C +requires_config_enabled MBEDTLS_SSL_SRV_C +requires_config_enabled MBEDTLS_SSL_CLI_C +run_test "TLS 1.3: Server side check - mbedtls with cient authentication" \ + "$P_SRV debug_level=4 auth_mode=required crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ + "$P_CLI debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13" \ + 1 \ + -s "tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ + -s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ + -s "tls13 server state: MBEDTLS_SSL_SERVER_CERTIFICATE" \ + -c "client state: MBEDTLS_SSL_CERTIFICATE_REQUEST" \ + -s "SSL - The requested feature is not available" \ + -s "=> parse client hello" \ + -s "<= parse client hello" + for i in opt-testcases/*.sh do TEST_SUITE_NAME=${i##*/} From cec9ae62590195aa24e7e0c2b1a6b9a8f1553def Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Fri, 6 May 2022 07:28:50 +0000 Subject: [PATCH 08/11] Change the code places of CERTIFICATE_REQUEST Change-Id: I3aa293184fea4f960782675bdd520256c808bd4e Signed-off-by: XiaokangQian --- library/ssl_tls13_server.c | 203 ++++++++++++++++++------------------- 1 file changed, 100 insertions(+), 103 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 01dff8c9c..ac6d40e90 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -748,11 +748,6 @@ static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) { int authmode; - if( mbedtls_ssl_tls13_some_psk_enabled( ssl ) ) - { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "<= skip write certificate request" ) ); - return( SSL_CERTIFICATE_REQUEST_SKIP ); - } authmode = ssl->conf->authmode; if( authmode == MBEDTLS_SSL_VERIFY_NONE ) @@ -760,104 +755,6 @@ static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) return( SSL_CERTIFICATE_REQUEST_SEND_REQUEST ); } - -/* - * struct { - * opaque certificate_request_context<0..2^8-1>; - * Extension extensions<2..2^16-1>; - * } CertificateRequest; - * - */ -static int ssl_tls13_write_certificate_request_body( mbedtls_ssl_context *ssl, - unsigned char *buf, - const unsigned char *end, - size_t *out_len ) -{ - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - unsigned char *p = buf; - size_t extensions_len = 0; - unsigned char *p_extensions_len; - - *out_len = 0; - - /* Check if we have enough space: - * - certificate_request_context (1 byte) - * - extensions length (2 bytes) - */ - MBEDTLS_SSL_CHK_BUF_PTR( p, end, 3 ); - - /* - * Write certificate_request_context - */ - /* - * We use a zero length context for the normal handshake - * messages. For post-authentication handshake messages - * this request context would be set to a non-zero value. - */ - *p++ = 0x0; - - /* - * Write extensions - */ - /* The extensions must contain the signature_algorithms. */ - p_extensions_len = p; - p += 2; - ret = mbedtls_ssl_write_sig_alg_ext( ssl, p, end, &extensions_len ); - if( ret != 0 ) - return( ret ); - - /* length field for all extensions */ - MBEDTLS_PUT_UINT16_BE( extensions_len, p_extensions_len, 0 ); - p += extensions_len; - - *out_len = p - buf; - - return( 0 ); -} - -static int ssl_tls13_write_certificate_request( mbedtls_ssl_context *ssl ) -{ - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write certificate request" ) ); - - MBEDTLS_SSL_PROC_CHK_NEG( ssl_tls13_certificate_request_coordinate( ssl ) ); - - if( ret == SSL_CERTIFICATE_REQUEST_SEND_REQUEST ) - { - unsigned char *buf; - size_t buf_len, msg_len; - - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_start_handshake_msg( ssl, - MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, &buf, &buf_len ) ); - - MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_certificate_request_body( - ssl, buf, buf + buf_len, &msg_len ) ); - - mbedtls_ssl_add_hs_msg_to_checksum( - ssl, MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, buf, msg_len ); - - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_finish_handshake_msg( - ssl, buf_len, msg_len ) ); - } - else if( ret == SSL_CERTIFICATE_REQUEST_SKIP ) - { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip write certificate request" ) ); - ret = 0; - } - else - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; - goto cleanup; - } - - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_CERTIFICATE ); -cleanup: - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write certificate request" ) ); - return( ret ); -} #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ /* @@ -1292,6 +1189,106 @@ cleanup: return( ret ); } +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) +/* + * struct { + * opaque certificate_request_context<0..2^8-1>; + * Extension extensions<2..2^16-1>; + * } CertificateRequest; + * + */ +static int ssl_tls13_write_certificate_request_body( mbedtls_ssl_context *ssl, + unsigned char *buf, + const unsigned char *end, + size_t *out_len ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + unsigned char *p = buf; + size_t extensions_len = 0; + unsigned char *p_extensions_len; + + *out_len = 0; + + /* Check if we have enough space: + * - certificate_request_context (1 byte) + * - extensions length (2 bytes) + */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 3 ); + + /* + * Write certificate_request_context + */ + /* + * We use a zero length context for the normal handshake + * messages. For post-authentication handshake messages + * this request context would be set to a non-zero value. + */ + *p++ = 0x0; + + /* + * Write extensions + */ + /* The extensions must contain the signature_algorithms. */ + p_extensions_len = p; + p += 2; + ret = mbedtls_ssl_write_sig_alg_ext( ssl, p, end, &extensions_len ); + if( ret != 0 ) + return( ret ); + + /* length field for all extensions */ + MBEDTLS_PUT_UINT16_BE( extensions_len, p_extensions_len, 0 ); + p += extensions_len; + + *out_len = p - buf; + + return( 0 ); +} + +static int ssl_tls13_write_certificate_request( mbedtls_ssl_context *ssl ) +{ + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write certificate request" ) ); + + MBEDTLS_SSL_PROC_CHK_NEG( ssl_tls13_certificate_request_coordinate( ssl ) ); + + if( ret == SSL_CERTIFICATE_REQUEST_SEND_REQUEST ) + { + unsigned char *buf; + size_t buf_len, msg_len; + + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_start_handshake_msg( ssl, + MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, &buf, &buf_len ) ); + + MBEDTLS_SSL_PROC_CHK( ssl_tls13_write_certificate_request_body( + ssl, buf, buf + buf_len, &msg_len ) ); + + mbedtls_ssl_add_hs_msg_to_checksum( + ssl, MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, buf, msg_len ); + + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_finish_handshake_msg( + ssl, buf_len, msg_len ) ); + } + else if( ret == SSL_CERTIFICATE_REQUEST_SKIP ) + { + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip write certificate request" ) ); + ret = 0; + } + else + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; + goto cleanup; + } + + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_CERTIFICATE ); +cleanup: + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write certificate request" ) ); + return( ret ); +} +#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ + /* * TLS 1.3 State Machine -- server side */ From ec6efb98bc676addb4ecbf436ddad502a650c0b5 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Fri, 6 May 2022 09:53:10 +0000 Subject: [PATCH 09/11] Change variable name to output_len Change-Id: I0f8a40da9782b2ec7af7e6f1faf1ac5c7e589418 Signed-off-by: XiaokangQian --- library/ssl_tls13_server.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index ac6d40e90..191dc548d 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1204,7 +1204,7 @@ static int ssl_tls13_write_certificate_request_body( mbedtls_ssl_context *ssl, { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char *p = buf; - size_t extensions_len = 0; + size_t output_len = 0; unsigned char *p_extensions_len; *out_len = 0; @@ -1231,13 +1231,13 @@ static int ssl_tls13_write_certificate_request_body( mbedtls_ssl_context *ssl, /* The extensions must contain the signature_algorithms. */ p_extensions_len = p; p += 2; - ret = mbedtls_ssl_write_sig_alg_ext( ssl, p, end, &extensions_len ); + ret = mbedtls_ssl_write_sig_alg_ext( ssl, p, end, &output_len ); if( ret != 0 ) return( ret ); /* length field for all extensions */ - MBEDTLS_PUT_UINT16_BE( extensions_len, p_extensions_len, 0 ); - p += extensions_len; + MBEDTLS_PUT_UINT16_BE( output_len, p_extensions_len, 0 ); + p += output_len; *out_len = p - buf; From a987e1d2f8e0cafa6e5d1a47acc98b56eaf97ffc Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Sat, 7 May 2022 01:25:58 +0000 Subject: [PATCH 10/11] Change state machine after encrypted extension and update cases Change-Id: Ie84a2d52a08538afb8f6096af0c054bd55ed66cb Signed-off-by: XiaokangQian --- library/ssl_tls13_server.c | 55 ++++++++++++++++---------------------- tests/ssl-opt.sh | 9 ++++--- 2 files changed, 29 insertions(+), 35 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 191dc548d..ef9cd1796 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -733,30 +733,6 @@ cleanup: return( ret ); } -#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) -#define SSL_CERTIFICATE_REQUEST_SEND_REQUEST 0 -#define SSL_CERTIFICATE_REQUEST_SKIP 1 -/* Coordination: - * Check whether a CertificateRequest message should be written. - * Returns a negative code on failure, or - * - SSL_CERTIFICATE_REQUEST_SEND_REQUEST - * - SSL_CERTIFICATE_REQUEST_SKIP - * indicating if the writing of the CertificateRequest - * should be skipped or not. - */ -static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) -{ - int authmode; - - authmode = ssl->conf->authmode; - - if( authmode == MBEDTLS_SSL_VERIFY_NONE ) - return( SSL_CERTIFICATE_REQUEST_SKIP ); - - return( SSL_CERTIFICATE_REQUEST_SEND_REQUEST ); -} -#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ - /* * Handler for MBEDTLS_SSL_SERVER_HELLO */ @@ -1170,14 +1146,7 @@ static int ssl_tls13_write_encrypted_extensions( mbedtls_ssl_context *ssl ) mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_FINISHED ); else { - MBEDTLS_SSL_PROC_CHK_NEG( ssl_tls13_certificate_request_coordinate( ssl ) ); - if( ret == SSL_CERTIFICATE_REQUEST_SEND_REQUEST ) - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CERTIFICATE_REQUEST ); - else - { - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_CERTIFICATE ); - ret = 0; - } + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CERTIFICATE_REQUEST ); } #else mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_FINISHED ); @@ -1190,6 +1159,28 @@ cleanup: } #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) +#define SSL_CERTIFICATE_REQUEST_SEND_REQUEST 0 +#define SSL_CERTIFICATE_REQUEST_SKIP 1 +/* Coordination: + * Check whether a CertificateRequest message should be written. + * Returns a negative code on failure, or + * - SSL_CERTIFICATE_REQUEST_SEND_REQUEST + * - SSL_CERTIFICATE_REQUEST_SKIP + * indicating if the writing of the CertificateRequest + * should be skipped or not. + */ +static int ssl_tls13_certificate_request_coordinate( mbedtls_ssl_context *ssl ) +{ + int authmode; + + authmode = ssl->conf->authmode; + + if( authmode == MBEDTLS_SSL_VERIFY_NONE ) + return( SSL_CERTIFICATE_REQUEST_SKIP ); + + return( SSL_CERTIFICATE_REQUEST_SEND_REQUEST ); +} + /* * struct { * opaque certificate_request_context<0..2^8-1>; diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 762a963e8..591b6d39f 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -11316,7 +11316,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_DEBUG_C requires_config_enabled MBEDTLS_SSL_SRV_C requires_openssl_tls1_3 -run_test "TLS 1.3: Server side check - openssl with cient authentication" \ +run_test "TLS 1.3: Server side check - openssl with client authentication" \ "$P_SRV debug_level=4 auth_mode=required crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ "$O_NEXT_CLI -msg -debug -cert data_files/server5.crt -key data_files/server5.key -tls1_3" \ 1 \ @@ -11325,6 +11325,7 @@ run_test "TLS 1.3: Server side check - openssl with cient authentication" \ -s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ -s "tls13 server state: MBEDTLS_SSL_CERTIFICATE_REQUEST" \ -s "tls13 server state: MBEDTLS_SSL_SERVER_CERTIFICATE" \ + -s "=> write certificate request" \ -s "SSL - The requested feature is not available" \ -s "=> parse client hello" \ -s "<= parse client hello" @@ -11351,7 +11352,7 @@ requires_gnutls_next_no_ticket requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_DEBUG_C requires_config_enabled MBEDTLS_SSL_SRV_C -run_test "TLS 1.3: Server side check - gnutls with cient authentication" \ +run_test "TLS 1.3: Server side check - gnutls with client authentication" \ "$P_SRV debug_level=4 auth_mode=required crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ "$G_NEXT_CLI localhost -d 4 --x509certfile data_files/server5.crt --x509keyfile data_files/server5.key --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:%NO_TICKETS:%DISABLE_TLS13_COMPAT_MODE -V" \ 1 \ @@ -11360,6 +11361,7 @@ run_test "TLS 1.3: Server side check - gnutls with cient authentication" \ -s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ -s "tls13 server state: MBEDTLS_SSL_CERTIFICATE_REQUEST" \ -s "tls13 server state: MBEDTLS_SSL_SERVER_CERTIFICATE" \ + -s "=> write certificate request" \ -s "SSL - The requested feature is not available" \ -s "=> parse client hello" \ -s "<= parse client hello" @@ -11376,6 +11378,7 @@ run_test "TLS 1.3: Server side check - mbedtls" \ -s "tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ -s "tls13 server state: MBEDTLS_SSL_ENCRYPTED_EXTENSIONS" \ -s "tls13 server state: MBEDTLS_SSL_SERVER_CERTIFICATE" \ + -s "=> write certificate request" \ -c "client state: MBEDTLS_SSL_CERTIFICATE_REQUEST" \ -s "SSL - The requested feature is not available" \ -s "=> parse client hello" \ @@ -11385,7 +11388,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_DEBUG_C requires_config_enabled MBEDTLS_SSL_SRV_C requires_config_enabled MBEDTLS_SSL_CLI_C -run_test "TLS 1.3: Server side check - mbedtls with cient authentication" \ +run_test "TLS 1.3: Server side check - mbedtls with client authentication" \ "$P_SRV debug_level=4 auth_mode=required crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ "$P_CLI debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13" \ 1 \ From aad9b0a286434da1bf129a600ffed8a95de69725 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Mon, 9 May 2022 01:11:21 +0000 Subject: [PATCH 11/11] Update code base on comments Change-Id: Ibc5043154515d2801565a2b99741dfda1344211c Signed-off-by: XiaokangQian --- library/ssl_tls.c | 4 ---- library/ssl_tls13_server.c | 6 +----- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 1a62eff03..d18758537 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -8085,14 +8085,10 @@ int mbedtls_ssl_write_sig_alg_ext( mbedtls_ssl_context *ssl, unsigned char *buf, 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; #if defined(MBEDTLS_SSL_PROTO_TLS1_3) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index ef9cd1796..775443cd6 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -1140,14 +1140,11 @@ static int ssl_tls13_write_encrypted_extensions( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_finish_handshake_msg( ssl, buf_len, msg_len ) ); - /* Update state */ #if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) if( mbedtls_ssl_tls13_some_psk_enabled( ssl ) ) mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_FINISHED ); else - { mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CERTIFICATE_REQUEST ); - } #else mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_FINISHED ); #endif @@ -1226,9 +1223,8 @@ static int ssl_tls13_write_certificate_request_body( mbedtls_ssl_context *ssl, if( ret != 0 ) return( ret ); - /* length field for all extensions */ - MBEDTLS_PUT_UINT16_BE( output_len, p_extensions_len, 0 ); p += output_len; + MBEDTLS_PUT_UINT16_BE( p - p_extensions_len - 2, p_extensions_len, 0 ); *out_len = p - buf;