From 7807f9f5c943d7242d4b5a70699247dee2fe557e Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Tue, 15 Feb 2022 10:04:37 +0000 Subject: [PATCH 01/27] Add client hello into server side Signed-off-by: XiaokangQian --- include/mbedtls/ssl.h | 12 +- library/ecp_internal.h | 45 ++ library/ssl_tls13_generic.c | 39 ++ library/ssl_tls13_server.c | 944 +++++++++++++++++++++++++++++++++++- 4 files changed, 1037 insertions(+), 3 deletions(-) create mode 100644 library/ecp_internal.h diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 147e76d401..476443c359 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -98,6 +98,8 @@ /* Error space gap */ /** Processing of the Certificate handshake message failed. */ #define MBEDTLS_ERR_SSL_BAD_CERTIFICATE -0x7A00 +/** Server needs to send a HelloRetryRequest */ +#define MBEDTLS_ERR_SSL_HRR_REQUIRED -0x7A80 /* Error space gap */ /* Error space gap */ /* Error space gap */ @@ -324,6 +326,9 @@ #define MBEDTLS_SSL_SRV_CIPHERSUITE_ORDER_CLIENT 1 #define MBEDTLS_SSL_SRV_CIPHERSUITE_ORDER_SERVER 0 +#define MBEDTLS_SSL_FORCE_RR_CHECK_OFF 0 +#define MBEDTLS_SSL_FORCE_RR_CHECK_ON 1 + /* * Default range for DTLS retransmission timer value, in milliseconds. * RFC 6347 4.2.4.1 says from 1 second to 60 seconds. @@ -488,6 +493,7 @@ #define MBEDTLS_SSL_ALERT_MSG_INAPROPRIATE_FALLBACK 86 /* 0x56 */ #define MBEDTLS_SSL_ALERT_MSG_USER_CANCELED 90 /* 0x5A */ #define MBEDTLS_SSL_ALERT_MSG_NO_RENEGOTIATION 100 /* 0x64 */ +#define MBEDTLS_SSL_ALERT_MSG_MISSING_EXTENSION 109 /* 0x6d -- new in TLS 1.3 */ #define MBEDTLS_SSL_ALERT_MSG_UNSUPPORTED_EXT 110 /* 0x6E */ #define MBEDTLS_SSL_ALERT_MSG_UNRECOGNIZED_NAME 112 /* 0x70 */ #define MBEDTLS_SSL_ALERT_MSG_UNKNOWN_PSK_IDENTITY 115 /* 0x73 */ @@ -641,6 +647,8 @@ typedef enum MBEDTLS_SSL_SERVER_NEW_SESSION_TICKET, MBEDTLS_SSL_SERVER_HELLO_VERIFY_REQUEST_SENT, #if defined(MBEDTLS_SSL_PROTO_TLS1_3) + MBEDTLS_SSL_HELLO_RETRY_REQUEST, + MBEDTLS_SSL_SECOND_CLIENT_HELLO, MBEDTLS_SSL_ENCRYPTED_EXTENSIONS, MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY, #if defined(MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE) @@ -1372,7 +1380,9 @@ struct mbedtls_ssl_config int (*MBEDTLS_PRIVATE(f_ticket_parse))( void *, mbedtls_ssl_session *, unsigned char *, size_t); void *MBEDTLS_PRIVATE(p_ticket); /*!< context for the ticket callbacks */ #endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_SRV_C */ - +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) + unsigned int MBEDTLS_PRIVATE(rr_config); +#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) size_t MBEDTLS_PRIVATE(cid_len); /*!< The length of CIDs for incoming DTLS records. */ #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ diff --git a/library/ecp_internal.h b/library/ecp_internal.h new file mode 100644 index 0000000000..ccd860f498 --- /dev/null +++ b/library/ecp_internal.h @@ -0,0 +1,45 @@ + +/** + * \file ecp_internal.h + * + * \brief ECC-related functions with external linkage but which are + * not part of the public API. + */ +/* + * Copyright The Mbed TLS Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#ifndef MBEDTLS_ECP_INTERNAL_H +#define MBEDTLS_ECP_INTERNAL_H + +#include "common.h" +#include "mbedtls/ecp.h" +#include "mbedtls/ecdh.h" + +static inline mbedtls_ecp_group_id mbedtls_ecp_named_group_to_id( + uint16_t named_curve ) +{ + const mbedtls_ecp_curve_info *curve_info; + curve_info = mbedtls_ecp_curve_info_from_tls_id( named_curve ); + if( curve_info == NULL ) + return( MBEDTLS_ECP_DP_NONE ); + return( curve_info->grp_id ); +} + +int mbedtls_ecdh_import_public_raw( mbedtls_ecdh_context *ctx, + const unsigned char *buf, + const unsigned char *end ); + +#endif /* MBEDTLS_ECP_INTERNAL_H */ diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 6623e7f705..758dbfd498 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -30,6 +30,7 @@ #include "mbedtls/constant_time.h" #include +#include "ecp_internal.h" #include "ssl_misc.h" #include "ssl_tls13_keys.h" #include "ssl_debug_helpers.h" @@ -1511,4 +1512,42 @@ int mbedtls_ssl_reset_transcript_for_hrr( mbedtls_ssl_context *ssl ) return( ret ); } +#define ECDH_VALIDATE_RET( cond ) \ + MBEDTLS_INTERNAL_VALIDATE_RET( cond, MBEDTLS_ERR_ECP_BAD_INPUT_DATA ) + +static int ecdh_import_public_raw( mbedtls_ecdh_context_mbed *ctx, + const unsigned char *buf, + const unsigned char *end ) +{ + return( mbedtls_ecp_point_read_binary( &ctx->grp, &ctx->Qp, + buf, end - buf ) ); +} + +int mbedtls_ecdh_import_public_raw( mbedtls_ecdh_context *ctx, + const unsigned char *buf, + const unsigned char *end ) +{ + ECDH_VALIDATE_RET( ctx != NULL ); + ECDH_VALIDATE_RET( buf != NULL ); + ECDH_VALIDATE_RET( end != NULL ); + +#if defined(MBEDTLS_ECDH_LEGACY_CONTEXT) + return( ecdh_read_tls13_params_internal( ctx, buf, end ) ); +#else + switch( ctx->var ) + { +#if defined(MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED) + case MBEDTLS_ECDH_VARIANT_EVEREST: + return( everest_import_public_raw( &ctx->ctx.everest_ecdh, + buf, end) ); +#endif + case MBEDTLS_ECDH_VARIANT_MBEDTLS_2_0: + return( ecdh_import_public_raw( &ctx->ctx.mbed_ecdh, + buf, end ) ); + default: + return MBEDTLS_ERR_ECP_BAD_INPUT_DATA; + } +#endif +} + #endif /* MBEDTLS_SSL_TLS_C && MBEDTLS_SSL_PROTO_TLS1_3 */ diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index b5f3ad738d..4e45583206 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -22,18 +22,958 @@ #if defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_SSL_PROTO_TLS1_3) #include "mbedtls/debug.h" +#include "mbedtls/platform.h" #include "ssl_misc.h" +#include "ssl_tls13_keys.h" #include "ssl_debug_helpers.h" +#include +#if defined(MBEDTLS_ECP_C) +#include "mbedtls/ecp.h" +#include "ecp_internal.h" +#endif /* MBEDTLS_ECP_C */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_3) + +/* From RFC 8446: + * struct { + * select (Handshake.msg_type) { + * case client_hello: + * ProtocolVersion versions<2..254>; + * case server_hello: // and HelloRetryRequest + * ProtocolVersion selected_version; + * }; + * } SupportedVersions; + */ +static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) +{ + size_t list_len; + int tls13_supported = 0; + int major_ver, minor_ver; + const unsigned char *p = buf; + const unsigned char *version_end; + + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); + + list_len = p[0]; + p += 1; + + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, list_len ); + if( list_len % 2 != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid supported version list length %" MBEDTLS_PRINTF_SIZET, + list_len ) ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } + + version_end = p + list_len; + while( p < version_end ) + { + mbedtls_ssl_read_version( &major_ver, &minor_ver, ssl->conf->transport, p ); + + /* In this implementation we only support TLS 1.3 and DTLS 1.3. */ + if( major_ver == MBEDTLS_SSL_MAJOR_VERSION_3 && + minor_ver == MBEDTLS_SSL_MINOR_VERSION_4 ) + { + tls13_supported = 1; + break; + } + + p += 2; + } + + if( tls13_supported == 0 ) + { + /* When we support runtime negotiation of TLS 1.2 and TLS 1.3, we need + * a graceful fallback to TLS 1.2 in this case. */ + + MBEDTLS_SSL_DEBUG_MSG( 1, ( "TLS 1.3 is not supported by the client" ) ); + + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_PROTOCOL_VERSION, + MBEDTLS_ERR_SSL_BAD_PROTOCOL_VERSION ); + return( MBEDTLS_ERR_SSL_BAD_PROTOCOL_VERSION ); + } + + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Negotiated version. Supported is [%d:%d]", + major_ver, minor_ver ) ); + + ssl->major_ver = major_ver; + ssl->minor_ver = minor_ver; + ssl->handshake->max_major_ver = ssl->major_ver; + ssl->handshake->max_minor_ver = ssl->minor_ver; + return( 0 ); +} + +#if ( defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) ) +/* This function parses the TLS 1.3 supported_groups extension and + * stores the received groups in ssl->handshake->curves. + * + * From RFC 8446: + * enum { + * ... (0xFFFF) + * } NamedGroup; + * struct { + * NamedGroup named_group_list<2..2^16-1>; + * } NamedGroupList; + */ +static int mbedtls_ssl_tls13_parse_supported_groups_ext( + mbedtls_ssl_context *ssl, + const unsigned char *buf, const unsigned char *end ) +{ + + size_t list_size, our_size; + const unsigned char *p = buf; + const mbedtls_ecp_curve_info *curve_info, **curves; + const unsigned char *extentions_end; + + MBEDTLS_SSL_DEBUG_BUF( 3, "supported_groups extension", p, end - buf ); + list_size = MBEDTLS_GET_UINT16_BE( p, 0 ); + p += 2; + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, list_size ); + if( list_size % 2 != 0 ) + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + + /* TODO: At the moment, this can happen when receiving a second + * ClientHello after an HRR. We should properly reset the + * state upon receiving an HRR, in which case we should + * not observe handshake->curves already being allocated. */ + if( ssl->handshake->curves != NULL ) + { + mbedtls_free( ssl->handshake->curves ); + ssl->handshake->curves = NULL; + } + + /* Don't allow our peer to make us allocate too much memory, + * and leave room for a final 0 */ + our_size = list_size / 2 + 1; + if( our_size > MBEDTLS_ECP_DP_MAX ) + our_size = MBEDTLS_ECP_DP_MAX; + + if( ( curves = mbedtls_calloc( our_size, sizeof( *curves ) ) ) == NULL ) + return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); + + extentions_end = p + list_size; + ssl->handshake->curves = curves; + + while ( p < extentions_end && our_size > 1 ) + { + uint16_t tls_grp_id = MBEDTLS_GET_UINT16_BE( p, 0 ); + curve_info = mbedtls_ecp_curve_info_from_tls_id( tls_grp_id ); + + /* mbedtls_ecp_curve_info_from_tls_id() uses the mbedtls_ecp_curve_info + * data structure (defined in ecp.c), which only includes the list of + * curves implemented. Hence, we only add curves that are also supported + * and implemented by the server. */ + if( curve_info != NULL ) + { + *curves++ = curve_info; + MBEDTLS_SSL_DEBUG_MSG( 4, ( "supported curve: %s", curve_info->name ) ); + our_size--; + } + + p += 2; + } + + return( 0 ); + +} +#endif /* MBEDTLS_ECDH_C || ( MBEDTLS_ECDSA_C */ + +/* TODO: Code for MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED missing */ +/* + * ssl_tls13_parse_key_shares_ext() verifies whether the information in the + * extension is correct and stores the provided key shares. Whether this is an + * acceptable key share depends on the selected ciphersuite. + * + * Possible return values are: + * - 0: Successful processing of the client provided key share extension. + * - MBEDTLS_ERR_SSL_HRR_REQUIRED: The key share provided by the client + * does not match a group supported by the server. A HelloRetryRequest will + * be needed. + * - Another negative return value for fatal errors. +*/ + +static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) +{ + int ret = 0; + unsigned char const *p = buf; + unsigned char const *extentions_end; + + size_t total_ext_len, cur_share_len; + int match_found = 0; + + /* From RFC 8446: + * + * struct { + * KeyShareEntry client_shares<0..2^16-1>; + * } KeyShareClientHello; + * + */ + + /* Read total legnth of KeyShareClientHello */ + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); + + total_ext_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + p += 2; + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, total_ext_len ); + + ssl->handshake->offered_group_id = 0; + extentions_end = p + total_ext_len; + + /* We try to find a suitable key share entry and copy it to the + * handshake context. Later, we have to find out whether we can do + * something with the provided key share or whether we have to + * dismiss it and send a HelloRetryRequest message. */ + + for( ; p < extentions_end; p += cur_share_len ) + { + uint16_t their_group; + mbedtls_ecp_group_id their_curve; + mbedtls_ecp_curve_info const *their_curve_info; + unsigned char const *end_of_share; + + /* + * struct { + * NamedGroup group; + * opaque key_exchange<1..2^16-1>; + * } KeyShareEntry; + */ + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, extentions_end, 4 ); + + their_group = MBEDTLS_GET_UINT16_BE( p, 0 ); + p += 2; + + cur_share_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + p += 2; + + end_of_share = p + cur_share_len; + + /* Continue parsing even if we have already found a match, + * for input validation purposes. */ + if( match_found == 1 ) + continue; + + /* + * NamedGroup matching + * + * For now, we only support ECDHE groups, but e.g. + * PQC KEMs will need to be added at a later stage. + */ + + /* Type 1: ECDHE shares + * + * - Check if we recognize the group + * - Check if it's supported + */ + + const mbedtls_ecp_curve_info *curve_info; + curve_info = mbedtls_ecp_curve_info_from_tls_id( their_group ); + if( curve_info == NULL ) + return( MBEDTLS_ECP_DP_NONE ); + their_curve = curve_info->grp_id; + if( mbedtls_ssl_check_curve( ssl, their_curve ) != 0 ) + continue; + + /* Type 2..X: Other kinds of shares */ + /* TO BE ADDED */ + + /* Skip if we no match succeeded. */ + if( their_curve == MBEDTLS_ECP_DP_NONE ) + { + MBEDTLS_SSL_DEBUG_MSG( 4, ( "Unrecognized NamedGroup %u", + (unsigned) their_group ) ); + continue; + } + + match_found = 1; + + /* KeyShare parsing + * + * Once we add more key share types, this needs to be a switch + * over the (type of) the named curve */ + + /* Type 1: ECDHE shares + * + * - Setup ECDHE context + * - Import client's public key + * - Apply further curve checks + */ + + their_curve_info = mbedtls_ecp_curve_info_from_grp_id( their_curve ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "ECDH curve: %s", their_curve_info->name ) ); + + ret = mbedtls_ecdh_setup( &ssl->handshake->ecdh_ctx, their_curve ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_setup()", ret ); + return( ret ); + } + + ret = mbedtls_ecdh_import_public_raw( &ssl->handshake->ecdh_ctx, + p, end_of_share ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_import_public_raw()", ret ); + return( ret ); + } + + ssl->handshake->offered_group_id = their_group; + } + + if( match_found == 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "no matching key share" ) ); + return( MBEDTLS_ERR_SSL_HRR_REQUIRED ); + } + return( 0 ); +} +#endif /* MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C */ + +#if defined(MBEDTLS_SSL_COOKIE_C) +static int ssl_tls13_parse_cookie_ext( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) +{ + int ret = 0; + size_t cookie_len; + unsigned char const *p = buf; + mbedtls_ssl_handshake_params *handshake = ssl->handshake; + + MBEDTLS_SSL_DEBUG_MSG( 3, ( "parse cookie extension" ) ); + + if( ssl->conf->f_cookie_check != NULL ) + { + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); + cookie_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + p += 2; + + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, cookie_len ); + + MBEDTLS_SSL_DEBUG_BUF( 3, "Received cookie", p, cookie_len ); + + if( ssl->conf->f_cookie_check( ssl->conf->p_cookie, + p, cookie_len, ssl->cli_id, + ssl->cli_id_len ) != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 2, ( "cookie verification failed" ) ); + handshake->verify_cookie_len = 1; + ret = MBEDTLS_ERR_SSL_HRR_REQUIRED; + } + else + { + MBEDTLS_SSL_DEBUG_MSG( 2, ( "cookie verification passed" ) ); + handshake->verify_cookie_len = 0; + } + } + else { + /* TBD: Check under what cases this is appropriate */ + MBEDTLS_SSL_DEBUG_MSG( 2, ( "cookie verification skipped" ) ); + } + + return( ret ); +} +#endif /* MBEDTLS_SSL_COOKIE_C */ + +/* + * + * STATE HANDLING: ClientHello + * + * There are three possible classes of outcomes when parsing the CH: + * + * 1) The CH was well-formed and matched the server's configuration. + * + * In this case, the server progresses to sending its ServerHello. + * + * 2) The CH was well-formed but didn't match the server's configuration. + * + * For example, the client might not have offered a key share which + * the server supports, or the server might require a cookie. + * + * In this case, the server sends a HelloRetryRequest. + * + * 3) The CH was ill-formed + * + * In this case, we abort the handshake. + * + */ + +/* + * Overview + */ + +/* Main entry point from the state machine; orchestrates the otherfunctions. */ +static int ssl_client_hello_process( mbedtls_ssl_context *ssl ); + +static int ssl_client_hello_parse( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ); + +/* Update the handshake state machine */ +/* TODO: At the moment, this doesn't update the state machine - why? */ +static int ssl_client_hello_postprocess( mbedtls_ssl_context *ssl, + int hrr_required ); + +/* + * Implementation + */ + +#define SSL_CLIENT_HELLO_OK 0 +#define SSL_CLIENT_HELLO_HRR_REQUIRED 1 + +static int ssl_client_hello_process( mbedtls_ssl_context *ssl ) +{ + + int ret = 0; + int hrr_required = SSL_CLIENT_HELLO_OK; + unsigned char* buf = NULL; + size_t buflen = 0; + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse client hello" ) ); + + ssl->major_ver = MBEDTLS_SSL_MAJOR_VERSION_3; + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_fetch_handshake_msg( + ssl, MBEDTLS_SSL_HS_CLIENT_HELLO, + &buf, &buflen ) ); + + mbedtls_ssl_tls13_add_hs_hdr_to_checksum( ssl, + MBEDTLS_SSL_HS_CLIENT_HELLO, + buflen ); + + MBEDTLS_SSL_PROC_CHK_NEG( ssl_client_hello_parse( ssl, buf, buf + buflen ) ); + hrr_required = ret; + + MBEDTLS_SSL_DEBUG_MSG( 1, ( "postprocess" ) ); + MBEDTLS_SSL_PROC_CHK( ssl_client_hello_postprocess( ssl, hrr_required ) ); + +cleanup: + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse client hello" ) ); + return( ret ); +} + +static void ssl_debug_print_client_hello_exts( mbedtls_ssl_context *ssl ) +{ + MBEDTLS_SSL_DEBUG_MSG( 3, ( "Supported Extensions:" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "- KEY_SHARE_EXTENSION ( %s )", + ( ( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_KEY_SHARE ) > 0 ) ? + "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "- PSK_KEY_EXCHANGE_MODES_EXTENSION ( %s )", + ( ( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_PSK_KEY_EXCHANGE_MODES ) > 0 ) ? + "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "- PRE_SHARED_KEY_EXTENSION ( %s )", + ( ( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_PRE_SHARED_KEY ) > 0 ) ? + "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "- SIGNATURE_ALGORITHM_EXTENSION ( %s )", + ( ( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_SIG_ALG ) > 0 ) ? + "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "- SUPPORTED_GROUPS_EXTENSION ( %s )", + ( ( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_SUPPORTED_GROUPS ) >0 ) ? + "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "- SUPPORTED_VERSION_EXTENSION ( %s )", + ( ( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_SUPPORTED_VERSIONS ) > 0 ) ? + "TRUE" : "FALSE" ) ); +#if defined ( MBEDTLS_SSL_SERVER_NAME_INDICATION ) + MBEDTLS_SSL_DEBUG_MSG( 3, ( "- SERVERNAME_EXTENSION ( %s )", + ( ( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_SERVERNAME ) > 0 ) ? + "TRUE" : "FALSE" ) ); +#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ +#if defined ( MBEDTLS_SSL_COOKIE_C ) + MBEDTLS_SSL_DEBUG_MSG( 3, ( "- COOKIE_EXTENSION ( %s )", + ( ( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_COOKIE ) >0 ) ? + "TRUE" : "FALSE" ) ); +#endif /* MBEDTLS_SSL_COOKIE_C */ +} + +static int ssl_client_hello_has_exts( mbedtls_ssl_context *ssl, + int ext_id_mask ) +{ + int masked = ssl->handshake->extensions_present & ext_id_mask; + return( masked == ext_id_mask ); +} + +static int ssl_client_hello_has_cert_extensions( mbedtls_ssl_context *ssl ) +{ + return( ssl_client_hello_has_exts( ssl, + MBEDTLS_SSL_EXT_SUPPORTED_GROUPS | + MBEDTLS_SSL_EXT_KEY_SHARE | + MBEDTLS_SSL_EXT_SIG_ALG ) ); +} + +static int ssl_check_certificate_key_exchange( mbedtls_ssl_context *ssl ) +{ + if( !mbedtls_ssl_conf_tls13_ephemeral_enabled( ssl ) ) + return( 0 ); + + if( !ssl_client_hello_has_cert_extensions( ssl ) ) + return( 0 ); + + ssl->handshake->tls13_kex_modes = MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL; + return( 1 ); +} + +static int ssl_client_hello_parse( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) +{ + int ret; + size_t i, j; + size_t comp_len, sess_len; + size_t cipher_suites_len; + size_t ext_len; + const unsigned char *ciph_offset; + const unsigned char *p = buf; + const unsigned char *extensions_end; + + const int* ciphersuites; + const mbedtls_ssl_ciphersuite_t* ciphersuite_info; + + int hrr_required = 0; + + ssl->handshake->extensions_present = MBEDTLS_SSL_EXT_NONE; + + /* + * ClientHello layer: + * 0 . 1 protocol version + * 2 . 33 random bytes ( starting with 4 bytes of Unix time ) + * 34 . 35 session id length ( 1 byte ) + * 35 . 34+x session id + * 35+x . 35+x DTLS only: cookie length ( 1 byte ) + * 36+x . .. DTLS only: cookie + * .. . .. ciphersuite list length ( 2 bytes ) + * .. . .. ciphersuite list + * .. . .. compression alg. list length ( 1 byte ) + * .. . .. compression alg. list + * .. . .. extensions length ( 2 bytes, optional ) + * .. . .. extensions ( optional ) + */ + + /* TBD: Needs to be updated due to mandatory extensions + * Minimal length ( with everything empty and extensions ommitted ) is + * 2 + 32 + 1 + 2 + 1 = 38 bytes. Check that first, so that we can + * read at least up to session id length without worrying. + */ + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 38 ); + + /* ... + * ProtocolVersion legacy_version = 0x0303; // TLS 1.2 + * ... + * with ProtocolVersion defined as: + * uint16 ProtocolVersion; + */ + if( !( p[0] == MBEDTLS_SSL_MAJOR_VERSION_3 && + p[1] == MBEDTLS_SSL_MINOR_VERSION_3 ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Unsupported version of TLS." ) ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_PROTOCOL_VERSION, + MBEDTLS_ERR_SSL_BAD_PROTOCOL_VERSION ); + ret = MBEDTLS_ERR_SSL_BAD_PROTOCOL_VERSION; + return ret; + } + p += 2; + + /* + * Save client random + */ + MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, random bytes", p, 32 ); + + memcpy( &ssl->handshake->randbytes[0], p, 32 ); + p += 32; /* skip random bytes */ + + /* + * Parse session ID + */ + sess_len = p[0]; + p++; /* skip session id length */ + + if( sess_len > 32 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) ); + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + } + + ssl->session_negotiate->id_len = sess_len; + + /* Note that this field is echoed even if + * the client's value corresponded to a cached pre-TLS 1.3 session + * which the server has chosen not to resume. A client which + * receives a legacy_session_id_echo field that does not match what + * it sent in the ClientHello MUST abort the handshake with an + * "illegal_parameter" alert. + */ + MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, session id length ( %" MBEDTLS_PRINTF_SIZET " )", sess_len ) ); + MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, session id", buf, sess_len ); + + memcpy( &ssl->session_negotiate->id[0], p, sess_len ); /* write session id */ + p += sess_len; + + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); + cipher_suites_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + p += 2; + + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, cipher_suites_len ); + + /* store pointer to ciphersuite list */ + ciph_offset = p; + + MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, ciphersuitelist", + p, cipher_suites_len ); + + /* skip ciphersuites for now */ + p += cipher_suites_len; + + /* + * For TLS 1.3 we are not using compression. + */ + comp_len = buf[0]; + p++; + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, comp_len ); + + MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, compression", + p, comp_len ); + + /* Determine whether we are indeed using null compression */ + if( ( comp_len != 1 ) && ( p[1] == 0 ) ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) ); + return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + } + + /* skip compression */ + p++; + + /* + * Check the extension length + */ + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); + + ext_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + p += 2; + extensions_end = p + ext_len; + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, ext_len ); + + MBEDTLS_SSL_DEBUG_BUF( 3, "client hello extensions", p, ext_len ); + + while( p < extensions_end ) + { + unsigned int extension_type; + size_t extension_data_len; + const unsigned char *extension_data_end; + + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, 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 ); + extension_data_end = p + extension_data_len; + + switch( extension_type ) + { +#if defined(MBEDTLS_SSL_COOKIE_C) + case MBEDTLS_TLS_EXT_COOKIE: + MBEDTLS_SSL_DEBUG_MSG( 3, ( "found cookie extension" ) ); + + ret = ssl_tls13_parse_cookie_ext( ssl, p, + extension_data_end ); + + /* if cookie verification failed then we return a hello retry + * message, or return success and set cookie extension present + */ + if( ret == MBEDTLS_ERR_SSL_HRR_REQUIRED ) + { + hrr_required = 1; + } + else if( ret == 0 ) + { + ssl->handshake->extensions_present |= MBEDTLS_SSL_EXT_COOKIE; + } + break; +#endif /* MBEDTLS_SSL_COOKIE_C */ + +#if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) + case MBEDTLS_TLS_EXT_SUPPORTED_GROUPS: + MBEDTLS_SSL_DEBUG_MSG( 3, ( "found supported group extension" ) ); + + /* Supported Groups Extension + * + * When sent by the client, the "supported_groups" extension + * indicates the named groups which the client supports, + * ordered from most preferred to least preferred. + */ + ret = mbedtls_ssl_tls13_parse_supported_groups_ext( ssl, p, + extension_data_end ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, + "mbedtls_ssl_parse_supported_groups_ext", ret ); + return( ret ); + } + + ssl->handshake->extensions_present |= MBEDTLS_SSL_EXT_SUPPORTED_GROUPS; + break; +#endif /* MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C */ + +#if ( defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) ) + case MBEDTLS_TLS_EXT_KEY_SHARE: + MBEDTLS_SSL_DEBUG_MSG( 3, ( "found key share extension" ) ); + + /* + * Key Share Extension + * + * When sent by the client, the "key_share" extension + * contains the endpoint's cryptographic parameters for + * ECDHE/DHE key establishment methods. + */ + ret = ssl_tls13_parse_key_shares_ext( ssl, p, extension_data_end ); + if( ret == MBEDTLS_ERR_SSL_HRR_REQUIRED ) + { + hrr_required = 1; + ret = 0; + } + + if( ret != 0 ) + return( ret ); + + ssl->handshake->extensions_present |= MBEDTLS_SSL_EXT_KEY_SHARE; + break; +#endif /* MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C */ + + case MBEDTLS_TLS_EXT_SUPPORTED_VERSIONS: + MBEDTLS_SSL_DEBUG_MSG( 3, ( "found supported versions extension" ) ); + + ret = ssl_tls13_parse_supported_versions_ext( + ssl, p, extension_data_end ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, + ( "ssl_tls13_parse_supported_versions_ext" ), ret ); + return( ret ); + } + ssl->handshake->extensions_present |= MBEDTLS_SSL_EXT_SUPPORTED_VERSIONS; + break; + +#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) + case MBEDTLS_TLS_EXT_SIG_ALG: + MBEDTLS_SSL_DEBUG_MSG( 3, ( "found signature_algorithms extension" ) ); + + ret = mbedtls_ssl_tls13_parse_sig_alg_ext( ssl, p, + extension_data_end ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "ssl_parse_supported_signature_algorithms_server_ext ( %d )", + ret ) ); + return( ret ); + } + ssl->handshake->extensions_present |= MBEDTLS_SSL_EXT_SIG_ALG; + break; +#endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ + + default: + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "unknown extension found: %ud ( ignoring )", + extension_type ) ); + } + + p += extension_data_len; + } + + /* Update checksum with either + * - The entire content of the CH message, if no PSK extension is present + * - The content up to but excluding the PSK extension, if present. + */ + ssl->handshake->update_checksum( ssl, buf, p - buf ); + /* + * Search for a matching ciphersuite + */ + ciphersuites = ssl->conf->ciphersuite_list; + ciphersuite_info = NULL; + for ( j = 0, p = ciph_offset; j < cipher_suites_len; j += 2, p += 2 ) + { + for ( i = 0; ciphersuites[i] != 0; i++ ) + { + if( p[0] != ( ( ciphersuites[i] >> 8 ) & 0xFF ) || + p[1] != ( ( ciphersuites[i] ) & 0xFF ) ) + continue; + + ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( + ciphersuites[i] ); + + if( ciphersuite_info == NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( + 1, + ( "mbedtls_ssl_ciphersuite_from_id: should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + + goto have_ciphersuite; + + } + } + + return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + +have_ciphersuite: + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "selected ciphersuite: %s", + ciphersuite_info->name ) ); + + ssl->session_negotiate->ciphersuite = ciphersuites[i]; + ssl->handshake->ciphersuite_info = ciphersuite_info; + + /* List all the extensions we have received */ + ssl_debug_print_client_hello_exts( ssl ); + + /* + * Determine the key exchange algorithm to use. + * There are three types of key exchanges supported in TLS 1.3: + * - (EC)DH with ECDSA, + * - (EC)DH with PSK, + * - plain PSK. + * + * The PSK-based key exchanges may additionally be used with 0-RTT. + * + * Our built-in order of preference is + * 1 ) Plain PSK Mode + * 2 ) (EC)DHE-PSK Mode + * 3 ) Certificate Mode + */ + + if( !ssl_check_certificate_key_exchange( ssl ) ) + { + MBEDTLS_SSL_DEBUG_MSG( + 1, + ( "ClientHello message misses mandatory extensions." ) ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_MISSING_EXTENSION , + MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + } + +#if defined(MBEDTLS_SSL_COOKIE_C) + /* If we failed to see a cookie extension, and we required it through the + * configuration settings ( rr_config ), then we need to send a HRR msg. + * Conceptually, this is similiar to having received a cookie that failed + * the verification check. + */ + if( ( ssl->conf->rr_config == MBEDTLS_SSL_FORCE_RR_CHECK_ON ) && + !( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_COOKIE ) ) + { + MBEDTLS_SSL_DEBUG_MSG( + 2, + ( "Cookie extension missing. Need to send a HRR." ) ); + hrr_required = 1; + } +#endif /* MBEDTLS_SSL_COOKIE_C */ + + if( hrr_required == 1 ) + return( SSL_CLIENT_HELLO_HRR_REQUIRED ); + + return( 0 ); +} + +static int ssl_client_hello_postprocess( mbedtls_ssl_context* ssl, + int hrr_required ) +{ + int ret = 0; + + if( ssl->handshake->hello_retry_request_count == 0 && + ssl->conf->rr_config == MBEDTLS_SSL_FORCE_RR_CHECK_ON ) + { + hrr_required = SSL_CLIENT_HELLO_HRR_REQUIRED; + } + + if( hrr_required == SSL_CLIENT_HELLO_HRR_REQUIRED ) + { + /* + * Create stateless transcript hash for HRR + */ + MBEDTLS_SSL_DEBUG_MSG( 4, ( "Reset transcript for HRR" ) ); + ret = mbedtls_ssl_reset_transcript_for_hrr( ssl ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_reset_transcript_for_hrr", + ret ); + return( ret ); + } + mbedtls_ssl_session_reset_msg_layer( ssl, 0 ); + + /* Transmit Hello Retry Request */ + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_HELLO_RETRY_REQUEST ); + return( 0 ); + } + + ret = mbedtls_ssl_tls13_key_schedule_stage_early( ssl ); + if( ret != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, + "mbedtls_ssl_tls1_3_key_schedule_stage_early", ret ); + return( ret ); + } + + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_HELLO ); + return( 0 ); + +} + +/* + * TLS and DTLS 1.3 State Maschine -- server side + */ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) { - ((void) ssl); + int ret = 0; + + if( ssl->state == MBEDTLS_SSL_HANDSHAKE_OVER || ssl->handshake == NULL ) + return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "tls13 server state: %s(%d)", mbedtls_ssl_states_str( ssl->state ), ssl->state ) ); - return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + if( ( ret = mbedtls_ssl_flush_output( ssl ) ) != 0 ) + return( ret ); + + switch( ssl->state ) + { + /* start state */ + case MBEDTLS_SSL_HELLO_REQUEST: + ssl->handshake->hello_retry_request_count = 0; + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_HELLO ); + + break; + + /* ----- READ CLIENT HELLO ----*/ + + case MBEDTLS_SSL_CLIENT_HELLO: + + ret = ssl_client_hello_process( ssl ); + if( ret != 0 ) + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_client_hello_process", ret ); + + break; + + case MBEDTLS_SSL_HANDSHAKE_WRAPUP: + MBEDTLS_SSL_DEBUG_MSG( 2, ( "handshake: done" ) ); + + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Switch to application keys for all traffic" ) ); + + mbedtls_ssl_set_inbound_transform ( ssl, ssl->transform_application ); + mbedtls_ssl_set_outbound_transform( ssl, ssl->transform_application ); + + mbedtls_ssl_tls13_handshake_wrapup( ssl ); + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_NEW_SESSION_TICKET ); + + break; + + default: + MBEDTLS_SSL_DEBUG_MSG( 1, ( "invalid state %d", ssl->state ) ); + return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + } + + return( ret ); } #endif /* MBEDTLS_SSL_SRV_C && MBEDTLS_SSL_PROTO_TLS1_3 */ From 5e4528cd1286db58f936f7eba86e137a49983281 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Thu, 17 Feb 2022 07:51:12 +0000 Subject: [PATCH 02/27] Add test cases for server side parse client hello Signed-off-by: XiaokangQian --- tests/ssl-opt.sh | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 18fff9d7ea..ec6cad8c96 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -10216,6 +10216,33 @@ run_test "TLS 1.3: HelloRetryRequest check, ciphersuite TLS_AES_256_GCM_SHA38 -c "Protocol is TLSv1.3" \ -c "HTTP/1.0 200 OK" +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 +requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE +requires_config_enabled MBEDTLS_DEBUG_C +requires_config_enabled MBEDTLS_SSL_CLI_C +requires_config_disabled MBEDTLS_USE_PSA_CRYPTO +requires_openssl_tls1_3 +run_test "TLS 1.3: Server side check, ciphersuite TLS_AES_256_GCM_SHA384 - openssl" \ + "$P_SRV debug_level=4 force_version=tls13 tickets=0" \ + "$O_NEXT_CLI -msg -tls1_3" \ + 1 \ + -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_SSL_TLS1_3_COMPATIBILITY_MODE +requires_config_enabled MBEDTLS_DEBUG_C +requires_config_enabled MBEDTLS_SSL_CLI_C +requires_config_disabled MBEDTLS_USE_PSA_CRYPTO +run_test "TLS 1.3: Server side check, ciphersuite TLS_AES_128_GCM_SHA256 - gnutls" \ + "$P_SRV debug_level=4 force_version=tls13 tickets=0" \ + "$G_NEXT_CLI -d 4 localhost --priority=NONE:+AES-128-GCM:+SHA256:+AEAD:+VERS-TLS1.3:%NO_TICKETS" \ + 1 \ + -s "=> parse client hello" \ + -s "<= parse client hello" + for i in opt-testcases/*.sh do TEST_SUITE_NAME=${i##*/} From a9c58419f255c1559a62e7e23aca5d48e06d9ccc Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Thu, 17 Feb 2022 09:41:26 +0000 Subject: [PATCH 03/27] Fix compile and test issues Signed-off-by: XiaokangQian --- library/ssl_tls13_server.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 4e45583206..0b37947567 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -22,7 +22,6 @@ #if defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_SSL_PROTO_TLS1_3) #include "mbedtls/debug.h" -#include "mbedtls/platform.h" #include "ssl_misc.h" #include "ssl_tls13_keys.h" @@ -33,7 +32,13 @@ #include "ecp_internal.h" #endif /* MBEDTLS_ECP_C */ -#if defined(MBEDTLS_SSL_PROTO_TLS1_3) +#if defined(MBEDTLS_PLATFORM_C) +#include "mbedtls/platform.h" +#else +#include +#define mbedtls_calloc calloc +#define mbedtls_free free +#endif /* MBEDTLS_PLATFORM_C */ /* From RFC 8446: * struct { @@ -181,6 +186,7 @@ static int mbedtls_ssl_tls13_parse_supported_groups_ext( } #endif /* MBEDTLS_ECDH_C || ( MBEDTLS_ECDSA_C */ +#if ( defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) ) /* TODO: Code for MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED missing */ /* * ssl_tls13_parse_key_shares_ext() verifies whether the information in the @@ -627,7 +633,7 @@ static int ssl_client_hello_parse( mbedtls_ssl_context *ssl, /* * For TLS 1.3 we are not using compression. */ - comp_len = buf[0]; + comp_len = p[0]; p++; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, comp_len ); @@ -635,7 +641,7 @@ static int ssl_client_hello_parse( mbedtls_ssl_context *ssl, p, comp_len ); /* Determine whether we are indeed using null compression */ - if( ( comp_len != 1 ) && ( p[1] == 0 ) ) + if( ( comp_len != 1 ) && ( p[0] == 0 ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) ); return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); From 7ac3ab34042ac5e5c2dc15b2e159a3a8b0e670f4 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Tue, 22 Feb 2022 04:03:26 +0000 Subject: [PATCH 04/27] Add hello retry request count for server Signed-off-by: XiaokangQian --- library/ssl_misc.h | 4 ++++ library/ssl_tls13_server.c | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 020f062808..f39f78dc48 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -585,6 +585,10 @@ struct mbedtls_ssl_handshake_params /*!< Number of Hello Retry Request messages received from the server. */ int hello_retry_request_count; #endif /* MBEDTLS_SSL_CLI_C */ +#if defined(MBEDTLS_SSL_SRV_C) + /*!< Number of Hello Retry Request messages sent by the server. */ + int hello_retry_requests_sent; +#endif /* MBEDTLS_SSL_SRV_C */ #if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 0b37947567..be1277834d 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -886,7 +886,7 @@ static int ssl_client_hello_postprocess( mbedtls_ssl_context* ssl, { int ret = 0; - if( ssl->handshake->hello_retry_request_count == 0 && + if( ssl->handshake->hello_retry_requests_sent == 0 && ssl->conf->rr_config == MBEDTLS_SSL_FORCE_RR_CHECK_ON ) { hrr_required = SSL_CLIENT_HELLO_HRR_REQUIRED; @@ -946,7 +946,7 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) { /* start state */ case MBEDTLS_SSL_HELLO_REQUEST: - ssl->handshake->hello_retry_request_count = 0; + ssl->handshake->hello_retry_requests_sent = 0; mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_HELLO ); break; From 3207a32b1ec0e632f4c7899250261d93d2890645 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 23 Feb 2022 03:15:27 +0000 Subject: [PATCH 05/27] Fix unused parameter issue and not defined cookie issue Signed-off-by: XiaokangQian --- include/mbedtls/ssl.h | 8 ++++++-- library/ssl_tls13_generic.c | 14 +++++++++++++- library/ssl_tls13_server.c | 8 +++----- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 476443c359..753a61d836 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1362,7 +1362,9 @@ struct mbedtls_ssl_config void *MBEDTLS_PRIVATE(p_psk); /*!< context for PSK callback */ #endif -#if defined(MBEDTLS_SSL_DTLS_HELLO_VERIFY) && defined(MBEDTLS_SSL_SRV_C) +#if (defined(MBEDTLS_SSL_DTLS_HELLO_VERIFY) || \ + (defined(MBEDTLS_SSL_COOKIE_C) && defined(MBEDTLS_SSL_PROTO_TLS1_3))) && \ + defined(MBEDTLS_SSL_SRV_C) /** Callback to create & write a cookie for ClientHello veirifcation */ int (*MBEDTLS_PRIVATE(f_cookie_write))( void *, unsigned char **, unsigned char *, const unsigned char *, size_t ); @@ -1703,7 +1705,9 @@ struct mbedtls_ssl_context /* * Information for DTLS hello verify */ -#if defined(MBEDTLS_SSL_DTLS_HELLO_VERIFY) && defined(MBEDTLS_SSL_SRV_C) +#if (defined(MBEDTLS_SSL_DTLS_HELLO_VERIFY) || \ + (defined(MBEDTLS_SSL_COOKIE_C) && defined(MBEDTLS_SSL_PROTO_TLS1_3))) && \ + defined(MBEDTLS_SSL_SRV_C) unsigned char *MBEDTLS_PRIVATE(cli_id); /*!< transport-level ID of the client */ size_t MBEDTLS_PRIVATE(cli_id_len); /*!< length of cli_id */ #endif /* MBEDTLS_SSL_DTLS_HELLO_VERIFY && MBEDTLS_SSL_SRV_C */ diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 758dbfd498..22d83deeb6 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1523,6 +1523,18 @@ static int ecdh_import_public_raw( mbedtls_ecdh_context_mbed *ctx, buf, end - buf ) ); } +#if defined(MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED) +static int everest_import_public_raw( mbedtls_x25519_context *ctx, + const unsigned char *buf, const unsigned char *end ) +{ + if( end - buf != MBEDTLS_X25519_KEY_SIZE_BYTES ) + return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); + + memcpy( ctx->peer_point, buf, MBEDTLS_X25519_KEY_SIZE_BYTES ); + return( 0 ); +} +#endif /* MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED */ + int mbedtls_ecdh_import_public_raw( mbedtls_ecdh_context *ctx, const unsigned char *buf, const unsigned char *end ) @@ -1532,7 +1544,7 @@ int mbedtls_ecdh_import_public_raw( mbedtls_ecdh_context *ctx, ECDH_VALIDATE_RET( end != NULL ); #if defined(MBEDTLS_ECDH_LEGACY_CONTEXT) - return( ecdh_read_tls13_params_internal( ctx, buf, end ) ); + return( ecdh_import_public_raw( ctx, buf, end ) ); #else switch( ctx->var ) { diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index be1277834d..007b9fa772 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -276,11 +276,7 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, * - Check if it's supported */ - const mbedtls_ecp_curve_info *curve_info; - curve_info = mbedtls_ecp_curve_info_from_tls_id( their_group ); - if( curve_info == NULL ) - return( MBEDTLS_ECP_DP_NONE ); - their_curve = curve_info->grp_id; + their_curve = mbedtls_ecp_named_group_to_id( their_group ); if( mbedtls_ssl_check_curve( ssl, their_curve ) != 0 ) continue; @@ -462,6 +458,8 @@ cleanup: static void ssl_debug_print_client_hello_exts( mbedtls_ssl_context *ssl ) { + ((void) ssl); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "Supported Extensions:" ) ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "- KEY_SHARE_EXTENSION ( %s )", ( ( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_KEY_SHARE ) > 0 ) ? From c5763b5efd34bfe058f05fb224c8553a96e67173 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Sat, 2 Apr 2022 03:34:37 +0000 Subject: [PATCH 06/27] Change some code style Change-Id: I67bb642e81693489345867ca87d7e9daa22f83ea Signed-off-by: XiaokangQian --- library/ssl_tls13_generic.c | 3 ++- library/ssl_tls13_server.c | 38 +++++++++++++++++++------------------ 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 22d83deeb6..bc992c913e 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1525,7 +1525,8 @@ static int ecdh_import_public_raw( mbedtls_ecdh_context_mbed *ctx, #if defined(MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED) static int everest_import_public_raw( mbedtls_x25519_context *ctx, - const unsigned char *buf, const unsigned char *end ) + const unsigned char *buf, + const unsigned char *end ) { if( end - buf != MBEDTLS_X25519_KEY_SIZE_BYTES ) return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 007b9fa772..847f12d969 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -92,7 +92,8 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, if( tls13_supported == 0 ) { /* When we support runtime negotiation of TLS 1.2 and TLS 1.3, we need - * a graceful fallback to TLS 1.2 in this case. */ + * a graceful fallback to TLS 1.2 in this case. + */ MBEDTLS_SSL_DEBUG_MSG( 1, ( "TLS 1.3 is not supported by the client" ) ); @@ -140,7 +141,7 @@ static int mbedtls_ssl_tls13_parse_supported_groups_ext( if( list_size % 2 != 0 ) return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - /* TODO: At the moment, this can happen when receiving a second + /* At the moment, this can happen when receiving a second * ClientHello after an HRR. We should properly reset the * state upon receiving an HRR, in which case we should * not observe handshake->curves already being allocated. */ @@ -151,7 +152,8 @@ static int mbedtls_ssl_tls13_parse_supported_groups_ext( } /* Don't allow our peer to make us allocate too much memory, - * and leave room for a final 0 */ + * and leave room for a final 0 + */ our_size = list_size / 2 + 1; if( our_size > MBEDTLS_ECP_DP_MAX ) our_size = MBEDTLS_ECP_DP_MAX; @@ -170,7 +172,8 @@ static int mbedtls_ssl_tls13_parse_supported_groups_ext( /* mbedtls_ecp_curve_info_from_tls_id() uses the mbedtls_ecp_curve_info * data structure (defined in ecp.c), which only includes the list of * curves implemented. Hence, we only add curves that are also supported - * and implemented by the server. */ + * and implemented by the server. + */ if( curve_info != NULL ) { *curves++ = curve_info; @@ -187,7 +190,6 @@ static int mbedtls_ssl_tls13_parse_supported_groups_ext( #endif /* MBEDTLS_ECDH_C || ( MBEDTLS_ECDSA_C */ #if ( defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) ) -/* TODO: Code for MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA_ENABLED missing */ /* * ssl_tls13_parse_key_shares_ext() verifies whether the information in the * extension is correct and stores the provided key shares. Whether this is an @@ -233,7 +235,8 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, /* We try to find a suitable key share entry and copy it to the * handshake context. Later, we have to find out whether we can do * something with the provided key share or whether we have to - * dismiss it and send a HelloRetryRequest message. */ + * dismiss it and send a HelloRetryRequest message. + */ for( ; p < extentions_end; p += cur_share_len ) { @@ -259,7 +262,8 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, end_of_share = p + cur_share_len; /* Continue parsing even if we have already found a match, - * for input validation purposes. */ + * for input validation purposes. + */ if( match_found == 1 ) continue; @@ -280,9 +284,6 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, if( mbedtls_ssl_check_curve( ssl, their_curve ) != 0 ) continue; - /* Type 2..X: Other kinds of shares */ - /* TO BE ADDED */ - /* Skip if we no match succeeded. */ if( their_curve == MBEDTLS_ECP_DP_NONE ) { @@ -296,7 +297,8 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, /* KeyShare parsing * * Once we add more key share types, this needs to be a switch - * over the (type of) the named curve */ + * over the (type of) the named curve + */ /* Type 1: ECDHE shares * @@ -371,8 +373,8 @@ static int ssl_tls13_parse_cookie_ext( mbedtls_ssl_context *ssl, handshake->verify_cookie_len = 0; } } - else { - /* TBD: Check under what cases this is appropriate */ + else + { MBEDTLS_SSL_DEBUG_MSG( 2, ( "cookie verification skipped" ) ); } @@ -415,7 +417,6 @@ static int ssl_client_hello_parse( mbedtls_ssl_context *ssl, const unsigned char *end ); /* Update the handshake state machine */ -/* TODO: At the moment, this doesn't update the state machine - why? */ static int ssl_client_hello_postprocess( mbedtls_ssl_context *ssl, int hrr_required ); @@ -542,7 +543,7 @@ static int ssl_client_hello_parse( mbedtls_ssl_context *ssl, * ClientHello layer: * 0 . 1 protocol version * 2 . 33 random bytes ( starting with 4 bytes of Unix time ) - * 34 . 35 session id length ( 1 byte ) + * 34 . 34 session id length ( 1 byte ) * 35 . 34+x session id * 35+x . 35+x DTLS only: cookie length ( 1 byte ) * 36+x . .. DTLS only: cookie @@ -554,7 +555,7 @@ static int ssl_client_hello_parse( mbedtls_ssl_context *ssl, * .. . .. extensions ( optional ) */ - /* TBD: Needs to be updated due to mandatory extensions + /* Needs to be updated due to mandatory extensions * Minimal length ( with everything empty and extensions ommitted ) is * 2 + 32 + 1 + 2 + 1 = 38 bytes. Check that first, so that we can * read at least up to session id length without worrying. @@ -584,13 +585,14 @@ static int ssl_client_hello_parse( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, random bytes", p, 32 ); memcpy( &ssl->handshake->randbytes[0], p, 32 ); - p += 32; /* skip random bytes */ + /* skip random bytes */ + p += 32; /* * Parse session ID */ sess_len = p[0]; - p++; /* skip session id length */ + p++; if( sess_len > 32 ) { From 8840888fbc0ccb3b29b1e5769a59fe9150c46ce0 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Sat, 2 Apr 2022 10:15:03 +0000 Subject: [PATCH 07/27] Fix some CI issues Change-Id: I68ee024f29b7b8dd586f2c45e91950657e76bad8 Signed-off-by: XiaokangQian --- library/ssl_tls13_generic.c | 14 +++++++++----- library/ssl_tls13_server.c | 17 ++++++----------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index bc992c913e..18a66ecb8b 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1515,6 +1515,7 @@ int mbedtls_ssl_reset_transcript_for_hrr( mbedtls_ssl_context *ssl ) #define ECDH_VALIDATE_RET( cond ) \ MBEDTLS_INTERNAL_VALIDATE_RET( cond, MBEDTLS_ERR_ECP_BAD_INPUT_DATA ) +#if !defined(MBEDTLS_ECDH_LEGACY_CONTEXT) static int ecdh_import_public_raw( mbedtls_ecdh_context_mbed *ctx, const unsigned char *buf, const unsigned char *end ) @@ -1522,6 +1523,7 @@ static int ecdh_import_public_raw( mbedtls_ecdh_context_mbed *ctx, return( mbedtls_ecp_point_read_binary( &ctx->grp, &ctx->Qp, buf, end - buf ) ); } +#endif /* MBEDTLS_ECDH_LEGACY_CONTEXT */ #if defined(MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED) static int everest_import_public_raw( mbedtls_x25519_context *ctx, @@ -1543,24 +1545,26 @@ int mbedtls_ecdh_import_public_raw( mbedtls_ecdh_context *ctx, ECDH_VALIDATE_RET( ctx != NULL ); ECDH_VALIDATE_RET( buf != NULL ); ECDH_VALIDATE_RET( end != NULL ); - #if defined(MBEDTLS_ECDH_LEGACY_CONTEXT) - return( ecdh_import_public_raw( ctx, buf, end ) ); + ((void) ctx); + ((void) buf); + ((void) end); + return ( 0 ); #else switch( ctx->var ) { #if defined(MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED) case MBEDTLS_ECDH_VARIANT_EVEREST: - return( everest_import_public_raw( &ctx->ctx.everest_ecdh, + return( everest_import_public_raw( &ctx->ctx.everest_ecdh.ctx, buf, end) ); -#endif +#endif /* MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED */ case MBEDTLS_ECDH_VARIANT_MBEDTLS_2_0: return( ecdh_import_public_raw( &ctx->ctx.mbed_ecdh, buf, end ) ); default: return MBEDTLS_ERR_ECP_BAD_INPUT_DATA; } -#endif +#endif /* MBEDTLS_ECDH_LEGACY_CONTEXT */ } #endif /* MBEDTLS_SSL_TLS_C && MBEDTLS_SSL_PROTO_TLS1_3 */ diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 847f12d969..934737a853 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -147,7 +147,7 @@ static int mbedtls_ssl_tls13_parse_supported_groups_ext( * not observe handshake->curves already being allocated. */ if( ssl->handshake->curves != NULL ) { - mbedtls_free( ssl->handshake->curves ); + //mbedtls_free( ssl->handshake->curves ); ssl->handshake->curves = NULL; } @@ -189,7 +189,7 @@ static int mbedtls_ssl_tls13_parse_supported_groups_ext( } #endif /* MBEDTLS_ECDH_C || ( MBEDTLS_ECDSA_C */ -#if ( defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) ) +#if defined(MBEDTLS_ECDH_C) /* * ssl_tls13_parse_key_shares_ext() verifies whether the information in the * extension is correct and stores the provided key shares. Whether this is an @@ -242,7 +242,6 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, { uint16_t their_group; mbedtls_ecp_group_id their_curve; - mbedtls_ecp_curve_info const *their_curve_info; unsigned char const *end_of_share; /* @@ -307,8 +306,7 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, * - Apply further curve checks */ - their_curve_info = mbedtls_ecp_curve_info_from_grp_id( their_curve ); - MBEDTLS_SSL_DEBUG_MSG( 2, ( "ECDH curve: %s", their_curve_info->name ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "ECDH curve: %ud", their_curve ) ); ret = mbedtls_ecdh_setup( &ssl->handshake->ecdh_ctx, their_curve ); if( ret != 0 ) @@ -335,7 +333,7 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, } return( 0 ); } -#endif /* MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C */ +#endif /* MBEDTLS_ECDH_C */ #if defined(MBEDTLS_SSL_COOKIE_C) static int ssl_tls13_parse_cookie_ext( mbedtls_ssl_context *ssl, @@ -345,7 +343,6 @@ static int ssl_tls13_parse_cookie_ext( mbedtls_ssl_context *ssl, int ret = 0; size_t cookie_len; unsigned char const *p = buf; - mbedtls_ssl_handshake_params *handshake = ssl->handshake; MBEDTLS_SSL_DEBUG_MSG( 3, ( "parse cookie extension" ) ); @@ -364,13 +361,11 @@ static int ssl_tls13_parse_cookie_ext( mbedtls_ssl_context *ssl, ssl->cli_id_len ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "cookie verification failed" ) ); - handshake->verify_cookie_len = 1; ret = MBEDTLS_ERR_SSL_HRR_REQUIRED; } else { MBEDTLS_SSL_DEBUG_MSG( 2, ( "cookie verification passed" ) ); - handshake->verify_cookie_len = 0; } } else @@ -722,7 +717,7 @@ static int ssl_client_hello_parse( mbedtls_ssl_context *ssl, break; #endif /* MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C */ -#if ( defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) ) +#if defined(MBEDTLS_ECDH_C) case MBEDTLS_TLS_EXT_KEY_SHARE: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found key share extension" ) ); @@ -745,7 +740,7 @@ static int ssl_client_hello_parse( mbedtls_ssl_context *ssl, ssl->handshake->extensions_present |= MBEDTLS_SSL_EXT_KEY_SHARE; break; -#endif /* MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C */ +#endif /* MBEDTLS_ECDH_C */ case MBEDTLS_TLS_EXT_SUPPORTED_VERSIONS: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found supported versions extension" ) ); From c4b8c99a38e2201598c839483c307ba5073fe1e8 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Thu, 7 Apr 2022 11:31:38 +0000 Subject: [PATCH 08/27] Rebase and solve conflicts and issues Change-Id: I17246c5b2f8a8ec4989c8b0b83b55cad0491b78a Signed-off-by: XiaokangQian --- library/ssl_tls.c | 24 ------------------------ library/ssl_tls13_server.c | 9 ++------- tests/ssl-opt.sh | 4 ++-- 3 files changed, 4 insertions(+), 33 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 4f1bac67df..cf64e81699 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -912,18 +912,6 @@ static int ssl_conf_version_check( const mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_PROTO_TLS1_3) if( mbedtls_ssl_conf_is_tls13_only( conf ) ) { - if( conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "DTLS 1.3 is not yet supported." ) ); - return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); - } - - if( conf->endpoint == MBEDTLS_SSL_IS_SERVER ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "TLS 1.3 server is not supported yet." ) ); - return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); - } - MBEDTLS_SSL_DEBUG_MSG( 4, ( "The SSL configuration is tls13 only." ) ); return( 0 ); } @@ -940,18 +928,6 @@ static int ssl_conf_version_check( const mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_PROTO_TLS1_2) && defined(MBEDTLS_SSL_PROTO_TLS1_3) if( mbedtls_ssl_conf_is_hybrid_tls12_tls13( conf ) ) { - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "DTLS not yet supported in Hybrid TLS 1.3 + TLS 1.2" ) ); - return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); - } - - if( conf->endpoint == MBEDTLS_SSL_IS_SERVER ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "TLS 1.3 server is not supported yet." ) ); - return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); - } - MBEDTLS_SSL_DEBUG_MSG( 4, ( "The SSL configuration is TLS 1.3 or TLS 1.2." ) ); return( 0 ); } diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 934737a853..9f55fe76ea 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -107,8 +107,6 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, ssl->major_ver = major_ver; ssl->minor_ver = minor_ver; - ssl->handshake->max_major_ver = ssl->major_ver; - ssl->handshake->max_minor_ver = ssl->minor_ver; return( 0 ); } @@ -436,10 +434,6 @@ static int ssl_client_hello_process( mbedtls_ssl_context *ssl ) ssl, MBEDTLS_SSL_HS_CLIENT_HELLO, &buf, &buflen ) ); - mbedtls_ssl_tls13_add_hs_hdr_to_checksum( ssl, - MBEDTLS_SSL_HS_CLIENT_HELLO, - buflen ); - MBEDTLS_SSL_PROC_CHK_NEG( ssl_client_hello_parse( ssl, buf, buf + buflen ) ); hrr_required = ret; @@ -786,7 +780,8 @@ static int ssl_client_hello_parse( mbedtls_ssl_context *ssl, * - The entire content of the CH message, if no PSK extension is present * - The content up to but excluding the PSK extension, if present. */ - ssl->handshake->update_checksum( ssl, buf, p - buf ); + mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_SERVER_HELLO, + buf, p - buf ); /* * Search for a matching ciphersuite */ diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index ec6cad8c96..a9948fde8b 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -10223,7 +10223,7 @@ requires_config_enabled MBEDTLS_SSL_CLI_C requires_config_disabled MBEDTLS_USE_PSA_CRYPTO requires_openssl_tls1_3 run_test "TLS 1.3: Server side check, ciphersuite TLS_AES_256_GCM_SHA384 - openssl" \ - "$P_SRV debug_level=4 force_version=tls13 tickets=0" \ + "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ "$O_NEXT_CLI -msg -tls1_3" \ 1 \ -s "=> parse client hello" \ @@ -10237,7 +10237,7 @@ requires_config_enabled MBEDTLS_DEBUG_C requires_config_enabled MBEDTLS_SSL_CLI_C requires_config_disabled MBEDTLS_USE_PSA_CRYPTO run_test "TLS 1.3: Server side check, ciphersuite TLS_AES_128_GCM_SHA256 - gnutls" \ - "$P_SRV debug_level=4 force_version=tls13 tickets=0" \ + "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ "$G_NEXT_CLI -d 4 localhost --priority=NONE:+AES-128-GCM:+SHA256:+AEAD:+VERS-TLS1.3:%NO_TICKETS" \ 1 \ -s "=> parse client hello" \ From 9b5d04b078de4acd8471eb0bfb7db28fea000720 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Sun, 10 Apr 2022 10:20:43 +0000 Subject: [PATCH 09/27] Share parse_key_share() between client and server Change-Id: I3fd2604296dc0e1e8380f5405429a6b0feb6e981 Signed-off-by: XiaokangQian --- library/ecp_internal.h | 45 ---------------------- library/ssl_misc.h | 8 ++++ library/ssl_tls13_client.c | 26 +------------ library/ssl_tls13_generic.c | 67 +++++++++----------------------- library/ssl_tls13_server.c | 76 ++++++++++++------------------------- 5 files changed, 52 insertions(+), 170 deletions(-) delete mode 100644 library/ecp_internal.h diff --git a/library/ecp_internal.h b/library/ecp_internal.h deleted file mode 100644 index ccd860f498..0000000000 --- a/library/ecp_internal.h +++ /dev/null @@ -1,45 +0,0 @@ - -/** - * \file ecp_internal.h - * - * \brief ECC-related functions with external linkage but which are - * not part of the public API. - */ -/* - * Copyright The Mbed TLS Contributors - * SPDX-License-Identifier: Apache-2.0 - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -#ifndef MBEDTLS_ECP_INTERNAL_H -#define MBEDTLS_ECP_INTERNAL_H - -#include "common.h" -#include "mbedtls/ecp.h" -#include "mbedtls/ecdh.h" - -static inline mbedtls_ecp_group_id mbedtls_ecp_named_group_to_id( - uint16_t named_curve ) -{ - const mbedtls_ecp_curve_info *curve_info; - curve_info = mbedtls_ecp_curve_info_from_tls_id( named_curve ); - if( curve_info == NULL ) - return( MBEDTLS_ECP_DP_NONE ); - return( curve_info->grp_id ); -} - -int mbedtls_ecdh_import_public_raw( mbedtls_ecdh_context *ctx, - const unsigned char *buf, - const unsigned char *end ); - -#endif /* MBEDTLS_ECP_INTERNAL_H */ diff --git a/library/ssl_misc.h b/library/ssl_misc.h index f39f78dc48..b23fc1df15 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -2176,4 +2176,12 @@ static inline int psa_ssl_status_to_mbedtls( psa_status_t status ) } #endif /* MBEDTLS_USE_PSA_CRYPTO || MBEDTLS_SSL_PROTO_TLS1_3 */ +#if defined(MBEDTLS_ECDH_C) + +int mbedtls_ssl_tls13_read_public_ecdhe_share( mbedtls_ssl_context *ssl, + const unsigned char *buf, + size_t buf_len ); + +#endif /* MBEDTLS_ECDH_C */ + #endif /* ssl_misc.h */ diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index b05d2f239a..198c20a2a3 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -417,30 +417,6 @@ cleanup: return( ret ); } -#if defined(MBEDTLS_ECDH_C) - -static int ssl_tls13_read_public_ecdhe_share( mbedtls_ssl_context *ssl, - const unsigned char *buf, - size_t buf_len ) -{ - uint8_t *p = (uint8_t*)buf; - mbedtls_ssl_handshake_params *handshake = ssl->handshake; - - /* Get size of the TLS opaque key_exchange field of the KeyShareEntry struct. */ - uint16_t peerkey_len = MBEDTLS_GET_UINT16_BE( p, 0 ); - p += 2; - - /* Check if key size is consistent with given buffer length. */ - if ( peerkey_len > ( buf_len - 2 ) ) - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - - /* Store peer's ECDH public key. */ - memcpy( handshake->ecdh_psa_peerkey, p, peerkey_len ); - handshake->ecdh_psa_peerkey_len = peerkey_len; - - return( 0 ); -} -#endif /* MBEDTLS_ECDH_C */ /* * ssl_tls13_parse_hrr_key_share_ext() @@ -565,7 +541,7 @@ static int ssl_tls13_parse_key_share_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 2, ( "ECDH curve: %s", curve_info->name ) ); - ret = ssl_tls13_read_public_ecdhe_share( ssl, p, end - p ); + ret = mbedtls_ssl_tls13_read_public_ecdhe_share( ssl, p, end - p ); if( ret != 0 ) return( ret ); } diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 18a66ecb8b..a6bcac3183 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -30,7 +30,6 @@ #include "mbedtls/constant_time.h" #include -#include "ecp_internal.h" #include "ssl_misc.h" #include "ssl_tls13_keys.h" #include "ssl_debug_helpers.h" @@ -1512,59 +1511,29 @@ int mbedtls_ssl_reset_transcript_for_hrr( mbedtls_ssl_context *ssl ) return( ret ); } -#define ECDH_VALIDATE_RET( cond ) \ - MBEDTLS_INTERNAL_VALIDATE_RET( cond, MBEDTLS_ERR_ECP_BAD_INPUT_DATA ) +#if defined(MBEDTLS_ECDH_C) -#if !defined(MBEDTLS_ECDH_LEGACY_CONTEXT) -static int ecdh_import_public_raw( mbedtls_ecdh_context_mbed *ctx, - const unsigned char *buf, - const unsigned char *end ) +int mbedtls_ssl_tls13_read_public_ecdhe_share( mbedtls_ssl_context *ssl, + const unsigned char *buf, + size_t buf_len ) { - return( mbedtls_ecp_point_read_binary( &ctx->grp, &ctx->Qp, - buf, end - buf ) ); -} -#endif /* MBEDTLS_ECDH_LEGACY_CONTEXT */ + uint8_t *p = (uint8_t*)buf; + mbedtls_ssl_handshake_params *handshake = ssl->handshake; -#if defined(MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED) -static int everest_import_public_raw( mbedtls_x25519_context *ctx, - const unsigned char *buf, - const unsigned char *end ) -{ - if( end - buf != MBEDTLS_X25519_KEY_SIZE_BYTES ) - return( MBEDTLS_ERR_ECP_BAD_INPUT_DATA ); + /* Get size of the TLS opaque key_exchange field of the KeyShareEntry struct. */ + uint16_t peerkey_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + p += 2; + + /* Check if key size is consistent with given buffer length. */ + if ( peerkey_len > ( buf_len - 2 ) ) + return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + + /* Store peer's ECDH public key. */ + memcpy( handshake->ecdh_psa_peerkey, p, peerkey_len ); + handshake->ecdh_psa_peerkey_len = peerkey_len; - memcpy( ctx->peer_point, buf, MBEDTLS_X25519_KEY_SIZE_BYTES ); return( 0 ); } -#endif /* MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED */ - -int mbedtls_ecdh_import_public_raw( mbedtls_ecdh_context *ctx, - const unsigned char *buf, - const unsigned char *end ) -{ - ECDH_VALIDATE_RET( ctx != NULL ); - ECDH_VALIDATE_RET( buf != NULL ); - ECDH_VALIDATE_RET( end != NULL ); -#if defined(MBEDTLS_ECDH_LEGACY_CONTEXT) - ((void) ctx); - ((void) buf); - ((void) end); - return ( 0 ); -#else - switch( ctx->var ) - { -#if defined(MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED) - case MBEDTLS_ECDH_VARIANT_EVEREST: - return( everest_import_public_raw( &ctx->ctx.everest_ecdh.ctx, - buf, end) ); -#endif /* MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED */ - case MBEDTLS_ECDH_VARIANT_MBEDTLS_2_0: - return( ecdh_import_public_raw( &ctx->ctx.mbed_ecdh, - buf, end ) ); - default: - return MBEDTLS_ERR_ECP_BAD_INPUT_DATA; - } -#endif /* MBEDTLS_ECDH_LEGACY_CONTEXT */ -} +#endif /* MBEDTLS_ECDH_C */ #endif /* MBEDTLS_SSL_TLS_C && MBEDTLS_SSL_PROTO_TLS1_3 */ diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 9f55fe76ea..f002595264 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -29,7 +29,6 @@ #include #if defined(MBEDTLS_ECP_C) #include "mbedtls/ecp.h" -#include "ecp_internal.h" #endif /* MBEDTLS_ECP_C */ #if defined(MBEDTLS_PLATFORM_C) @@ -238,9 +237,7 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, for( ; p < extentions_end; p += cur_share_len ) { - uint16_t their_group; - mbedtls_ecp_group_id their_curve; - unsigned char const *end_of_share; + uint16_t group; /* * struct { @@ -250,13 +247,11 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, */ MBEDTLS_SSL_CHK_BUF_READ_PTR( p, extentions_end, 4 ); - their_group = MBEDTLS_GET_UINT16_BE( p, 0 ); - p += 2; + group = MBEDTLS_GET_UINT16_BE( p, 0 ); + p += 2; cur_share_len = MBEDTLS_GET_UINT16_BE( p, 0 ); - p += 2; - - end_of_share = p + cur_share_len; + p += 2; /* Continue parsing even if we have already found a match, * for input validation purposes. @@ -268,60 +263,39 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, * NamedGroup matching * * For now, we only support ECDHE groups, but e.g. - * PQC KEMs will need to be added at a later stage. - */ - /* Type 1: ECDHE shares + * Type 1: ECDHE shares * * - Check if we recognize the group * - Check if it's supported */ - their_curve = mbedtls_ecp_named_group_to_id( their_group ); - if( mbedtls_ssl_check_curve( ssl, their_curve ) != 0 ) - continue; + if( mbedtls_ssl_tls13_named_group_is_ecdhe( group ) ) + { + const mbedtls_ecp_curve_info *curve_info = + mbedtls_ecp_curve_info_from_tls_id( group ); + if( curve_info == NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid TLS curve group id" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } - /* Skip if we no match succeeded. */ - if( their_curve == MBEDTLS_ECP_DP_NONE ) + match_found = 1; + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "ECDH curve: %s", curve_info->name ) ); + + ret = mbedtls_ssl_tls13_read_public_ecdhe_share( ssl, p, end - p ); + if( ret != 0 ) + return( ret ); + } + else { MBEDTLS_SSL_DEBUG_MSG( 4, ( "Unrecognized NamedGroup %u", - (unsigned) their_group ) ); + (unsigned) group ) ); continue; } - match_found = 1; - - /* KeyShare parsing - * - * Once we add more key share types, this needs to be a switch - * over the (type of) the named curve - */ - - /* Type 1: ECDHE shares - * - * - Setup ECDHE context - * - Import client's public key - * - Apply further curve checks - */ - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "ECDH curve: %ud", their_curve ) ); - - ret = mbedtls_ecdh_setup( &ssl->handshake->ecdh_ctx, their_curve ); - if( ret != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_setup()", ret ); - return( ret ); - } - - ret = mbedtls_ecdh_import_public_raw( &ssl->handshake->ecdh_ctx, - p, end_of_share ); - if( ret != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_import_public_raw()", ret ); - return( ret ); - } - - ssl->handshake->offered_group_id = their_group; + ssl->handshake->offered_group_id = group; } if( match_found == 0 ) From 4080a7f687481ff819e9933ebd2ce0a226da397b Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Mon, 11 Apr 2022 09:55:18 +0000 Subject: [PATCH 10/27] Change code style and some share functions Change variables and functions name style Refine supported_version Refine client hello parse Change-Id: Iabc1db51e791588f999c60db464326e2bdf7b2c4 Signed-off-by: XiaokangQian --- library/ssl_tls13_server.c | 180 ++++++++++++++++++++----------------- 1 file changed, 97 insertions(+), 83 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index f002595264..8a2f18ad1d 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -53,27 +53,27 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) { - size_t list_len; + size_t versions_len; int tls13_supported = 0; int major_ver, minor_ver; const unsigned char *p = buf; - const unsigned char *version_end; + const unsigned char *versions_end; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); - list_len = p[0]; + versions_len = p[0]; p += 1; - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, list_len ); - if( list_len % 2 != 0 ) + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, versions_len ); + if( versions_len % 2 != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid supported version list length %" MBEDTLS_PRINTF_SIZET, - list_len ) ); + versions_len ) ); return( MBEDTLS_ERR_SSL_DECODE_ERROR ); } - version_end = p + list_len; - while( p < version_end ) + versions_end = p + versions_len; + while( p < versions_end ) { mbedtls_ssl_read_version( &major_ver, &minor_ver, ssl->conf->transport, p ); @@ -90,8 +90,8 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, if( tls13_supported == 0 ) { - /* When we support runtime negotiation of TLS 1.2 and TLS 1.3, we need - * a graceful fallback to TLS 1.2 in this case. + /* Here we only support TLS 1.3, we need report "bad protocol" if it + * doesn't support TLS 1.2. */ MBEDTLS_SSL_DEBUG_MSG( 1, ( "TLS 1.3 is not supported by the client" ) ); @@ -126,16 +126,16 @@ static int mbedtls_ssl_tls13_parse_supported_groups_ext( const unsigned char *buf, const unsigned char *end ) { - size_t list_size, our_size; + size_t named_group_list_len, curve_list_len; const unsigned char *p = buf; const mbedtls_ecp_curve_info *curve_info, **curves; const unsigned char *extentions_end; MBEDTLS_SSL_DEBUG_BUF( 3, "supported_groups extension", p, end - buf ); - list_size = MBEDTLS_GET_UINT16_BE( p, 0 ); + named_group_list_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, list_size ); - if( list_size % 2 != 0 ) + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, named_group_list_len ); + if( named_group_list_len % 2 != 0 ) return( MBEDTLS_ERR_SSL_DECODE_ERROR ); /* At the moment, this can happen when receiving a second @@ -144,24 +144,24 @@ static int mbedtls_ssl_tls13_parse_supported_groups_ext( * not observe handshake->curves already being allocated. */ if( ssl->handshake->curves != NULL ) { - //mbedtls_free( ssl->handshake->curves ); + mbedtls_free( ssl->handshake->curves ); ssl->handshake->curves = NULL; } /* Don't allow our peer to make us allocate too much memory, * and leave room for a final 0 */ - our_size = list_size / 2 + 1; - if( our_size > MBEDTLS_ECP_DP_MAX ) - our_size = MBEDTLS_ECP_DP_MAX; + curve_list_len = named_group_list_len / 2 + 1; + if( curve_list_len > MBEDTLS_ECP_DP_MAX ) + curve_list_len = MBEDTLS_ECP_DP_MAX; - if( ( curves = mbedtls_calloc( our_size, sizeof( *curves ) ) ) == NULL ) + if( ( curves = mbedtls_calloc( curve_list_len, sizeof( *curves ) ) ) == NULL ) return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); - extentions_end = p + list_size; + extentions_end = p + named_group_list_len; ssl->handshake->curves = curves; - while ( p < extentions_end && our_size > 1 ) + while ( p < extentions_end && curve_list_len > 1 ) { uint16_t tls_grp_id = MBEDTLS_GET_UINT16_BE( p, 0 ); curve_info = mbedtls_ecp_curve_info_from_tls_id( tls_grp_id ); @@ -175,7 +175,7 @@ static int mbedtls_ssl_tls13_parse_supported_groups_ext( { *curves++ = curve_info; MBEDTLS_SSL_DEBUG_MSG( 4, ( "supported curve: %s", curve_info->name ) ); - our_size--; + curve_list_len--; } p += 2; @@ -208,7 +208,7 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, unsigned char const *p = buf; unsigned char const *extentions_end; - size_t total_ext_len, cur_share_len; + size_t total_extensions_len, key_share_len; int match_found = 0; /* From RFC 8446: @@ -222,12 +222,12 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, /* Read total legnth of KeyShareClientHello */ MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); - total_ext_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + total_extensions_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, total_ext_len ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, total_extensions_len ); ssl->handshake->offered_group_id = 0; - extentions_end = p + total_ext_len; + extentions_end = p + total_extensions_len; /* We try to find a suitable key share entry and copy it to the * handshake context. Later, we have to find out whether we can do @@ -235,7 +235,7 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, * dismiss it and send a HelloRetryRequest message. */ - for( ; p < extentions_end; p += cur_share_len ) + for( ; p < extentions_end; p += key_share_len ) { uint16_t group; @@ -250,7 +250,7 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, group = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; - cur_share_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + key_share_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; /* Continue parsing even if we have already found a match, @@ -377,15 +377,15 @@ static int ssl_tls13_parse_cookie_ext( mbedtls_ssl_context *ssl, */ /* Main entry point from the state machine; orchestrates the otherfunctions. */ -static int ssl_client_hello_process( mbedtls_ssl_context *ssl ); +static int ssl_tls13_process_client_hello( mbedtls_ssl_context *ssl ); -static int ssl_client_hello_parse( mbedtls_ssl_context *ssl, - const unsigned char *buf, - const unsigned char *end ); +static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ); /* Update the handshake state machine */ -static int ssl_client_hello_postprocess( mbedtls_ssl_context *ssl, - int hrr_required ); +static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context *ssl, + int hrr_required ); /* * Implementation @@ -394,7 +394,7 @@ static int ssl_client_hello_postprocess( mbedtls_ssl_context *ssl, #define SSL_CLIENT_HELLO_OK 0 #define SSL_CLIENT_HELLO_HRR_REQUIRED 1 -static int ssl_client_hello_process( mbedtls_ssl_context *ssl ) +static int ssl_tls13_process_client_hello( mbedtls_ssl_context *ssl ) { int ret = 0; @@ -408,11 +408,13 @@ static int ssl_client_hello_process( mbedtls_ssl_context *ssl ) ssl, MBEDTLS_SSL_HS_CLIENT_HELLO, &buf, &buflen ) ); - MBEDTLS_SSL_PROC_CHK_NEG( ssl_client_hello_parse( ssl, buf, buf + buflen ) ); + MBEDTLS_SSL_PROC_CHK_NEG( ssl_tls13_parse_client_hello( ssl, buf, + buf + buflen ) ); hrr_required = ret; MBEDTLS_SSL_DEBUG_MSG( 1, ( "postprocess" ) ); - MBEDTLS_SSL_PROC_CHK( ssl_client_hello_postprocess( ssl, hrr_required ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_client_hello( ssl, + hrr_required ) ); cleanup: @@ -420,7 +422,7 @@ cleanup: return( ret ); } -static void ssl_debug_print_client_hello_exts( mbedtls_ssl_context *ssl ) +static void ssl_tls13_debug_print_client_hello_exts( mbedtls_ssl_context *ssl ) { ((void) ssl); @@ -455,42 +457,59 @@ static void ssl_debug_print_client_hello_exts( mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_SSL_COOKIE_C */ } -static int ssl_client_hello_has_exts( mbedtls_ssl_context *ssl, +static int ssl_tls13_client_hello_has_exts( mbedtls_ssl_context *ssl, int ext_id_mask ) { int masked = ssl->handshake->extensions_present & ext_id_mask; return( masked == ext_id_mask ); } -static int ssl_client_hello_has_cert_extensions( mbedtls_ssl_context *ssl ) +static int ssl_tls13_client_hello_has_cert_extensions( mbedtls_ssl_context *ssl ) { - return( ssl_client_hello_has_exts( ssl, + return( ssl_tls13_client_hello_has_exts( ssl, MBEDTLS_SSL_EXT_SUPPORTED_GROUPS | MBEDTLS_SSL_EXT_KEY_SHARE | MBEDTLS_SSL_EXT_SIG_ALG ) ); } -static int ssl_check_certificate_key_exchange( mbedtls_ssl_context *ssl ) +static int ssl_tls13_check_certificate_key_exchange( mbedtls_ssl_context *ssl ) { if( !mbedtls_ssl_conf_tls13_ephemeral_enabled( ssl ) ) return( 0 ); - if( !ssl_client_hello_has_cert_extensions( ssl ) ) + if( !ssl_tls13_client_hello_has_cert_extensions( ssl ) ) return( 0 ); ssl->handshake->tls13_kex_modes = MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL; return( 1 ); } -static int ssl_client_hello_parse( mbedtls_ssl_context *ssl, - const unsigned char *buf, - const unsigned char *end ) +/* + * Structure of this message: + * + * uint16 ProtocolVersion; + * opaque Random[32]; + * + * uint8 CipherSuite[2]; // Cryptographic suite selector + * + * struct { + * ProtocolVersion legacy_version = 0x0303; // TLS v1.2 + * Random random; + * opaque legacy_session_id<0..32>; + * CipherSuite cipher_suites<2..2^16-2>; + * opaque legacy_compression_methods<1..2^8-1>; + * Extension extensions<8..2^16-1>; + * } ClientHello; + */ +static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, + const unsigned char *buf, + const unsigned char *end ) { int ret; size_t i, j; - size_t comp_len, sess_len; + size_t legacy_session_id_len; size_t cipher_suites_len; - size_t ext_len; + size_t extensions_len; const unsigned char *ciph_offset; const unsigned char *p = buf; const unsigned char *extensions_end; @@ -545,25 +564,26 @@ static int ssl_client_hello_parse( mbedtls_ssl_context *ssl, /* * Save client random */ - MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, random bytes", p, 32 ); + MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, random bytes", + p, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); - memcpy( &ssl->handshake->randbytes[0], p, 32 ); + memcpy( &ssl->handshake->randbytes[0], p, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); /* skip random bytes */ - p += 32; + p += MBEDTLS_SERVER_HELLO_RANDOM_LEN; /* * Parse session ID */ - sess_len = p[0]; + legacy_session_id_len = p[0]; p++; - if( sess_len > 32 ) + if( legacy_session_id_len > 32 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) ); return( MBEDTLS_ERR_SSL_DECODE_ERROR ); } - ssl->session_negotiate->id_len = sess_len; + ssl->session_negotiate->id_len = legacy_session_id_len; /* Note that this field is echoed even if * the client's value corresponded to a cached pre-TLS 1.3 session @@ -572,11 +592,11 @@ static int ssl_client_hello_parse( mbedtls_ssl_context *ssl, * it sent in the ClientHello MUST abort the handshake with an * "illegal_parameter" alert. */ - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, session id length ( %" MBEDTLS_PRINTF_SIZET " )", sess_len ) ); - MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, session id", buf, sess_len ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, session id length ( %" MBEDTLS_PRINTF_SIZET " )", legacy_session_id_len ) ); + MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, session id", buf, legacy_session_id_len ); - memcpy( &ssl->session_negotiate->id[0], p, sess_len ); /* write session id */ - p += sess_len; + memcpy( &ssl->session_negotiate->id[0], p, legacy_session_id_len ); /* write session id */ + p += legacy_session_id_len; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); cipher_suites_len = MBEDTLS_GET_UINT16_BE( p, 0 ); @@ -593,24 +613,18 @@ static int ssl_client_hello_parse( mbedtls_ssl_context *ssl, /* skip ciphersuites for now */ p += cipher_suites_len; - /* - * For TLS 1.3 we are not using compression. + /* ... + * uint8 legacy_compression_method = 0; + * ... */ - comp_len = p[0]; - p++; - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, comp_len ); - - MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, compression", - p, comp_len ); - - /* Determine whether we are indeed using null compression */ - if( ( comp_len != 1 ) && ( p[0] == 0 ) ) + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); + if( p[0] != 0 ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) ); - return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad legacy compression method" ) ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER, + MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + return ( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); } - - /* skip compression */ p++; /* @@ -618,12 +632,12 @@ static int ssl_client_hello_parse( mbedtls_ssl_context *ssl, */ MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); - ext_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + extensions_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; - extensions_end = p + ext_len; - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, ext_len ); + extensions_end = p + extensions_len; + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, extensions_len ); - MBEDTLS_SSL_DEBUG_BUF( 3, "client hello extensions", p, ext_len ); + MBEDTLS_SSL_DEBUG_BUF( 3, "client hello extensions", p, extensions_len ); while( p < extensions_end ) { @@ -796,7 +810,7 @@ have_ciphersuite: ssl->handshake->ciphersuite_info = ciphersuite_info; /* List all the extensions we have received */ - ssl_debug_print_client_hello_exts( ssl ); + ssl_tls13_debug_print_client_hello_exts( ssl ); /* * Determine the key exchange algorithm to use. @@ -813,7 +827,7 @@ have_ciphersuite: * 3 ) Certificate Mode */ - if( !ssl_check_certificate_key_exchange( ssl ) ) + if( !ssl_tls13_check_certificate_key_exchange( ssl ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, @@ -845,8 +859,8 @@ have_ciphersuite: return( 0 ); } -static int ssl_client_hello_postprocess( mbedtls_ssl_context* ssl, - int hrr_required ) +static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context* ssl, + int hrr_required ) { int ret = 0; @@ -919,9 +933,9 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) case MBEDTLS_SSL_CLIENT_HELLO: - ret = ssl_client_hello_process( ssl ); + ret = ssl_tls13_process_client_hello( ssl ); if( ret != 0 ) - MBEDTLS_SSL_DEBUG_RET( 1, "ssl_client_hello_process", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_tls13_process_client_hello", ret ); break; From ed582dd02383aaed5d38306581df8397a58ee41a Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 13 Apr 2022 08:21:05 +0000 Subject: [PATCH 11/27] Update based on comments Remove cookie support from server side Change code to align with coding styles Re-order functions of client_hello Change-Id: If31509ece402f8276e6cac37f261e0b166d05e18 Signed-off-by: XiaokangQian --- include/mbedtls/ssl.h | 7 +- library/ssl_tls.c | 12 ++ library/ssl_tls13_server.c | 272 +++++++++++-------------------------- tests/ssl-opt.sh | 14 +- 4 files changed, 97 insertions(+), 208 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 753a61d836..09fe01294a 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1362,9 +1362,7 @@ struct mbedtls_ssl_config void *MBEDTLS_PRIVATE(p_psk); /*!< context for PSK callback */ #endif -#if (defined(MBEDTLS_SSL_DTLS_HELLO_VERIFY) || \ - (defined(MBEDTLS_SSL_COOKIE_C) && defined(MBEDTLS_SSL_PROTO_TLS1_3))) && \ - defined(MBEDTLS_SSL_SRV_C) +#if defined(MBEDTLS_SSL_DTLS_HELLO_VERIFY) /** Callback to create & write a cookie for ClientHello veirifcation */ int (*MBEDTLS_PRIVATE(f_cookie_write))( void *, unsigned char **, unsigned char *, const unsigned char *, size_t ); @@ -1382,9 +1380,6 @@ struct mbedtls_ssl_config int (*MBEDTLS_PRIVATE(f_ticket_parse))( void *, mbedtls_ssl_session *, unsigned char *, size_t); void *MBEDTLS_PRIVATE(p_ticket); /*!< context for the ticket callbacks */ #endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_SRV_C */ -#if defined(MBEDTLS_SSL_PROTO_TLS1_3) - unsigned int MBEDTLS_PRIVATE(rr_config); -#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) size_t MBEDTLS_PRIVATE(cid_len); /*!< The length of CIDs for incoming DTLS records. */ #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index cf64e81699..54ea129617 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -912,6 +912,12 @@ static int ssl_conf_version_check( const mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_PROTO_TLS1_3) if( mbedtls_ssl_conf_is_tls13_only( conf ) ) { + if( conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "DTLS 1.3 is not yet supported." ) ); + return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + } + MBEDTLS_SSL_DEBUG_MSG( 4, ( "The SSL configuration is tls13 only." ) ); return( 0 ); } @@ -928,6 +934,12 @@ static int ssl_conf_version_check( const mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_PROTO_TLS1_2) && defined(MBEDTLS_SSL_PROTO_TLS1_3) if( mbedtls_ssl_conf_is_hybrid_tls12_tls13( conf ) ) { + if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "DTLS not yet supported in Hybrid TLS 1.3 + TLS 1.2" ) ); + return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + } + MBEDTLS_SSL_DEBUG_MSG( 4, ( "The SSL configuration is TLS 1.3 or TLS 1.2." ) ); return( 0 ); } diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 8a2f18ad1d..b172ec9b54 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -307,121 +307,6 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, } #endif /* MBEDTLS_ECDH_C */ -#if defined(MBEDTLS_SSL_COOKIE_C) -static int ssl_tls13_parse_cookie_ext( mbedtls_ssl_context *ssl, - const unsigned char *buf, - const unsigned char *end ) -{ - int ret = 0; - size_t cookie_len; - unsigned char const *p = buf; - - MBEDTLS_SSL_DEBUG_MSG( 3, ( "parse cookie extension" ) ); - - if( ssl->conf->f_cookie_check != NULL ) - { - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); - cookie_len = MBEDTLS_GET_UINT16_BE( p, 0 ); - p += 2; - - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, cookie_len ); - - MBEDTLS_SSL_DEBUG_BUF( 3, "Received cookie", p, cookie_len ); - - if( ssl->conf->f_cookie_check( ssl->conf->p_cookie, - p, cookie_len, ssl->cli_id, - ssl->cli_id_len ) != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "cookie verification failed" ) ); - ret = MBEDTLS_ERR_SSL_HRR_REQUIRED; - } - else - { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "cookie verification passed" ) ); - } - } - else - { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "cookie verification skipped" ) ); - } - - return( ret ); -} -#endif /* MBEDTLS_SSL_COOKIE_C */ - -/* - * - * STATE HANDLING: ClientHello - * - * There are three possible classes of outcomes when parsing the CH: - * - * 1) The CH was well-formed and matched the server's configuration. - * - * In this case, the server progresses to sending its ServerHello. - * - * 2) The CH was well-formed but didn't match the server's configuration. - * - * For example, the client might not have offered a key share which - * the server supports, or the server might require a cookie. - * - * In this case, the server sends a HelloRetryRequest. - * - * 3) The CH was ill-formed - * - * In this case, we abort the handshake. - * - */ - -/* - * Overview - */ - -/* Main entry point from the state machine; orchestrates the otherfunctions. */ -static int ssl_tls13_process_client_hello( mbedtls_ssl_context *ssl ); - -static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, - const unsigned char *buf, - const unsigned char *end ); - -/* Update the handshake state machine */ -static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context *ssl, - int hrr_required ); - -/* - * Implementation - */ - -#define SSL_CLIENT_HELLO_OK 0 -#define SSL_CLIENT_HELLO_HRR_REQUIRED 1 - -static int ssl_tls13_process_client_hello( mbedtls_ssl_context *ssl ) -{ - - int ret = 0; - int hrr_required = SSL_CLIENT_HELLO_OK; - unsigned char* buf = NULL; - size_t buflen = 0; - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse client hello" ) ); - - ssl->major_ver = MBEDTLS_SSL_MAJOR_VERSION_3; - MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_fetch_handshake_msg( - ssl, MBEDTLS_SSL_HS_CLIENT_HELLO, - &buf, &buflen ) ); - - MBEDTLS_SSL_PROC_CHK_NEG( ssl_tls13_parse_client_hello( ssl, buf, - buf + buflen ) ); - hrr_required = ret; - - MBEDTLS_SSL_DEBUG_MSG( 1, ( "postprocess" ) ); - MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_client_hello( ssl, - hrr_required ) ); - -cleanup: - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse client hello" ) ); - return( ret ); -} - static void ssl_tls13_debug_print_client_hello_exts( mbedtls_ssl_context *ssl ) { ((void) ssl); @@ -450,11 +335,6 @@ static void ssl_tls13_debug_print_client_hello_exts( mbedtls_ssl_context *ssl ) ( ( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_SERVERNAME ) > 0 ) ? "TRUE" : "FALSE" ) ); #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ -#if defined ( MBEDTLS_SSL_COOKIE_C ) - MBEDTLS_SSL_DEBUG_MSG( 3, ( "- COOKIE_EXTENSION ( %s )", - ( ( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_COOKIE ) >0 ) ? - "TRUE" : "FALSE" ) ); -#endif /* MBEDTLS_SSL_COOKIE_C */ } static int ssl_tls13_client_hello_has_exts( mbedtls_ssl_context *ssl, @@ -484,6 +364,29 @@ static int ssl_tls13_check_certificate_key_exchange( mbedtls_ssl_context *ssl ) return( 1 ); } +/* + * + * STATE HANDLING: ClientHello + * + * There are three possible classes of outcomes when parsing the CH: + * + * 1) The CH was well-formed and matched the server's configuration. + * + * In this case, the server progresses to sending its ServerHello. + * + * 2) The CH was well-formed but didn't match the server's configuration. + * + * For example, the client might not have offered a key share which + * the server supports, or the server might require a cookie. + * + * In this case, the server sends a HelloRetryRequest. + * + * 3) The CH was ill-formed + * + * In this case, we abort the handshake. + * + */ + /* * Structure of this message: * @@ -501,6 +404,10 @@ static int ssl_tls13_check_certificate_key_exchange( mbedtls_ssl_context *ssl ) * Extension extensions<8..2^16-1>; * } ClientHello; */ + +#define SSL_CLIENT_HELLO_OK 0 +#define SSL_CLIENT_HELLO_HRR_REQUIRED 1 + static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) @@ -510,11 +417,11 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, size_t legacy_session_id_len; size_t cipher_suites_len; size_t extensions_len; - const unsigned char *ciph_offset; + const unsigned char *cipher_suites_start; const unsigned char *p = buf; const unsigned char *extensions_end; - const int* ciphersuites; + const int* cipher_suites; const mbedtls_ssl_ciphersuite_t* ciphersuite_info; int hrr_required = 0; @@ -568,7 +475,6 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, p, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); memcpy( &ssl->handshake->randbytes[0], p, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); - /* skip random bytes */ p += MBEDTLS_SERVER_HELLO_RANDOM_LEN; /* @@ -592,10 +498,10 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, * it sent in the ClientHello MUST abort the handshake with an * "illegal_parameter" alert. */ - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, session id length ( %" MBEDTLS_PRINTF_SIZET " )", legacy_session_id_len ) ); - MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, session id", buf, legacy_session_id_len ); + MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, session id", + buf, legacy_session_id_len ); - memcpy( &ssl->session_negotiate->id[0], p, legacy_session_id_len ); /* write session id */ + memcpy( &ssl->session_negotiate->id[0], p, legacy_session_id_len ); p += legacy_session_id_len; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); @@ -605,12 +511,12 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, cipher_suites_len ); /* store pointer to ciphersuite list */ - ciph_offset = p; + cipher_suites_start = p; MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, ciphersuitelist", p, cipher_suites_len ); - /* skip ciphersuites for now */ + /* skip cipher_suites for now */ p += cipher_suites_len; /* ... @@ -628,10 +534,9 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, p++; /* - * Check the extension length + * Check the extensions length */ MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); - extensions_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; extensions_end = p + extensions_len; @@ -655,27 +560,6 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, switch( extension_type ) { -#if defined(MBEDTLS_SSL_COOKIE_C) - case MBEDTLS_TLS_EXT_COOKIE: - MBEDTLS_SSL_DEBUG_MSG( 3, ( "found cookie extension" ) ); - - ret = ssl_tls13_parse_cookie_ext( ssl, p, - extension_data_end ); - - /* if cookie verification failed then we return a hello retry - * message, or return success and set cookie extension present - */ - if( ret == MBEDTLS_ERR_SSL_HRR_REQUIRED ) - { - hrr_required = 1; - } - else if( ret == 0 ) - { - ssl->handshake->extensions_present |= MBEDTLS_SSL_EXT_COOKIE; - } - break; -#endif /* MBEDTLS_SSL_COOKIE_C */ - #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) case MBEDTLS_TLS_EXT_SUPPORTED_GROUPS: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found supported group extension" ) ); @@ -713,8 +597,8 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, ret = ssl_tls13_parse_key_shares_ext( ssl, p, extension_data_end ); if( ret == MBEDTLS_ERR_SSL_HRR_REQUIRED ) { - hrr_required = 1; - ret = 0; + MBEDTLS_SSL_DEBUG_MSG( 2, ( "HRR needed " ) ); + ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; } if( ret != 0 ) @@ -773,18 +657,17 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, /* * Search for a matching ciphersuite */ - ciphersuites = ssl->conf->ciphersuite_list; + cipher_suites = ssl->conf->ciphersuite_list; ciphersuite_info = NULL; - for ( j = 0, p = ciph_offset; j < cipher_suites_len; j += 2, p += 2 ) + for ( j = 0, p = cipher_suites_start; j < cipher_suites_len; j += 2, p += 2 ) { - for ( i = 0; ciphersuites[i] != 0; i++ ) + for ( i = 0; cipher_suites[i] != 0; i++ ) { - if( p[0] != ( ( ciphersuites[i] >> 8 ) & 0xFF ) || - p[1] != ( ( ciphersuites[i] ) & 0xFF ) ) + if( MBEDTLS_GET_UINT16_BE(p, 0) != cipher_suites[i] ) continue; ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( - ciphersuites[i] ); + cipher_suites[i] ); if( ciphersuite_info == NULL ) { @@ -806,7 +689,7 @@ have_ciphersuite: MBEDTLS_SSL_DEBUG_MSG( 2, ( "selected ciphersuite: %s", ciphersuite_info->name ) ); - ssl->session_negotiate->ciphersuite = ciphersuites[i]; + ssl->session_negotiate->ciphersuite = cipher_suites[i]; ssl->handshake->ciphersuite_info = ciphersuite_info; /* List all the extensions we have received */ @@ -837,35 +720,20 @@ have_ciphersuite: return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); } -#if defined(MBEDTLS_SSL_COOKIE_C) - /* If we failed to see a cookie extension, and we required it through the - * configuration settings ( rr_config ), then we need to send a HRR msg. - * Conceptually, this is similiar to having received a cookie that failed - * the verification check. - */ - if( ( ssl->conf->rr_config == MBEDTLS_SSL_FORCE_RR_CHECK_ON ) && - !( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_COOKIE ) ) - { - MBEDTLS_SSL_DEBUG_MSG( - 2, - ( "Cookie extension missing. Need to send a HRR." ) ); - hrr_required = 1; - } -#endif /* MBEDTLS_SSL_COOKIE_C */ - if( hrr_required == 1 ) return( SSL_CLIENT_HELLO_HRR_REQUIRED ); return( 0 ); } +/* Update the handshake state machine */ + static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context* ssl, int hrr_required ) { int ret = 0; - if( ssl->handshake->hello_retry_requests_sent == 0 && - ssl->conf->rr_config == MBEDTLS_SSL_FORCE_RR_CHECK_ON ) + if( ssl->handshake->hello_retry_requests_sent == 0 ) { hrr_required = SSL_CLIENT_HELLO_HRR_REQUIRED; } @@ -903,6 +771,38 @@ static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context* ssl, } +/* + * Main entry point from the state machine; orchestrates the otherfunctions. + */ + +static int ssl_tls13_process_client_hello( mbedtls_ssl_context *ssl ) +{ + + int ret = 0; + int hrr_required = SSL_CLIENT_HELLO_OK; + unsigned char* buf = NULL; + size_t buflen = 0; + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse client hello" ) ); + + ssl->major_ver = MBEDTLS_SSL_MAJOR_VERSION_3; + MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_fetch_handshake_msg( + ssl, MBEDTLS_SSL_HS_CLIENT_HELLO, + &buf, &buflen ) ); + + MBEDTLS_SSL_PROC_CHK_NEG( ssl_tls13_parse_client_hello( ssl, buf, + buf + buflen ) ); + hrr_required = ret; + + MBEDTLS_SSL_DEBUG_MSG( 1, ( "postprocess" ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_client_hello( ssl, + hrr_required ) ); + +cleanup: + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= parse client hello" ) ); + return( ret ); +} + /* * TLS and DTLS 1.3 State Maschine -- server side */ @@ -917,9 +817,6 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) mbedtls_ssl_states_str( ssl->state ), ssl->state ) ); - if( ( ret = mbedtls_ssl_flush_output( ssl ) ) != 0 ) - return( ret ); - switch( ssl->state ) { /* start state */ @@ -939,19 +836,6 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) break; - case MBEDTLS_SSL_HANDSHAKE_WRAPUP: - MBEDTLS_SSL_DEBUG_MSG( 2, ( "handshake: done" ) ); - - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Switch to application keys for all traffic" ) ); - - mbedtls_ssl_set_inbound_transform ( ssl, ssl->transform_application ); - mbedtls_ssl_set_outbound_transform( ssl, ssl->transform_application ); - - mbedtls_ssl_tls13_handshake_wrapup( ssl ); - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_NEW_SESSION_TICKET ); - - break; - default: MBEDTLS_SSL_DEBUG_MSG( 1, ( "invalid state %d", ssl->state ) ); return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index a9948fde8b..416b89f6c2 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -9638,14 +9638,6 @@ run_test "TLS 1.3: Test gnutls tls1_3 feature" \ -c "Version: TLS1.3" # TLS1.3 test cases -requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 -skip_handshake_stage_check -run_test "TLS 1.3: No server support" \ - "$P_SRV debug_level=2 force_version=tls13" \ - "$P_CLI debug_level=2 force_version=tls13" \ - 1 \ - -s "The requested feature is not available" - requires_openssl_tls1_3 requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE @@ -10226,6 +10218,9 @@ run_test "TLS 1.3: Server side check, ciphersuite TLS_AES_256_GCM_SHA384 - op "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ "$O_NEXT_CLI -msg -tls1_3" \ 1 \ + -s " tls13 server state: MBEDTLS_CLIENT_HELLO" \ + -s " tls13 server state: MBEDTLS_SERVER_HELLO" \ + -s ""SSL - The requested feature is not available"" \ -s "=> parse client hello" \ -s "<= parse client hello" @@ -10240,6 +10235,9 @@ run_test "TLS 1.3: Server side check, ciphersuite TLS_AES_128_GCM_SHA256 - gn "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ "$G_NEXT_CLI -d 4 localhost --priority=NONE:+AES-128-GCM:+SHA256:+AEAD:+VERS-TLS1.3:%NO_TICKETS" \ 1 \ + -s " tls13 server state: MBEDTLS_CLIENT_HELLO" \ + -s " tls13 server state: MBEDTLS_SERVER_HELLO" \ + -s ""SSL - The requested feature is not available"" \ -s "=> parse client hello" \ -s "<= parse client hello" From cfd925f3e8e2b27a6a5fe0b329c886d3e4f9cc93 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Thu, 14 Apr 2022 07:10:37 +0000 Subject: [PATCH 12/27] Fix comments and remove hrr related code Change-Id: Iab1fc5415b3b7f7b5bcb0a41a01f4234cc3497d6 Signed-off-by: XiaokangQian --- include/mbedtls/ssl.h | 3 +- library/ssl_misc.h | 4 --- library/ssl_tls13_generic.c | 5 +-- library/ssl_tls13_server.c | 59 ++++++-------------------------- tests/ssl-opt.sh | 8 ++--- tests/suites/test_suite_ssl.data | 2 +- 6 files changed, 18 insertions(+), 63 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 09fe01294a..3fbe6f3e2c 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -648,7 +648,6 @@ typedef enum MBEDTLS_SSL_SERVER_HELLO_VERIFY_REQUEST_SENT, #if defined(MBEDTLS_SSL_PROTO_TLS1_3) MBEDTLS_SSL_HELLO_RETRY_REQUEST, - MBEDTLS_SSL_SECOND_CLIENT_HELLO, MBEDTLS_SSL_ENCRYPTED_EXTENSIONS, MBEDTLS_SSL_CLIENT_CERTIFICATE_VERIFY, #if defined(MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE) @@ -1362,7 +1361,7 @@ struct mbedtls_ssl_config void *MBEDTLS_PRIVATE(p_psk); /*!< context for PSK callback */ #endif -#if defined(MBEDTLS_SSL_DTLS_HELLO_VERIFY) +#if defined(MBEDTLS_SSL_DTLS_HELLO_VERIFY) && defined(MBEDTLS_SSL_SRV_C) /** Callback to create & write a cookie for ClientHello veirifcation */ int (*MBEDTLS_PRIVATE(f_cookie_write))( void *, unsigned char **, unsigned char *, const unsigned char *, size_t ); diff --git a/library/ssl_misc.h b/library/ssl_misc.h index b23fc1df15..9a7c503259 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -585,10 +585,6 @@ struct mbedtls_ssl_handshake_params /*!< Number of Hello Retry Request messages received from the server. */ int hello_retry_request_count; #endif /* MBEDTLS_SSL_CLI_C */ -#if defined(MBEDTLS_SSL_SRV_C) - /*!< Number of Hello Retry Request messages sent by the server. */ - int hello_retry_requests_sent; -#endif /* MBEDTLS_SSL_SRV_C */ #if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index a6bcac3183..fabc5d1587 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1518,15 +1518,16 @@ int mbedtls_ssl_tls13_read_public_ecdhe_share( mbedtls_ssl_context *ssl, size_t buf_len ) { uint8_t *p = (uint8_t*)buf; + const uint8_t *end = buf + buf_len; mbedtls_ssl_handshake_params *handshake = ssl->handshake; /* Get size of the TLS opaque key_exchange field of the KeyShareEntry struct. */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); uint16_t peerkey_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; /* Check if key size is consistent with given buffer length. */ - if ( peerkey_len > ( buf_len - 2 ) ) - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); + MBEDTLS_SSL_CHK_BUF_PTR( p, end, peerkey_len ); /* Store peer's ECDH public key. */ memcpy( handshake->ecdh_psa_peerkey, p, peerkey_len ); diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index b172ec9b54..d181cd294b 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -41,12 +41,7 @@ /* From RFC 8446: * struct { - * select (Handshake.msg_type) { - * case client_hello: - * ProtocolVersion versions<2..254>; - * case server_hello: // and HelloRetryRequest - * ProtocolVersion selected_version; - * }; + * ProtocolVersion versions<2..254>; * } SupportedVersions; */ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, @@ -60,21 +55,15 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, const unsigned char *versions_end; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); - versions_len = p[0]; p += 1; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, versions_len ); - if( versions_len % 2 != 0 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid supported version list length %" MBEDTLS_PRINTF_SIZET, - versions_len ) ); - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - } versions_end = p + versions_len; while( p < versions_end ) { + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, versions_end, 2 ); mbedtls_ssl_read_version( &major_ver, &minor_ver, ssl->conf->transport, p ); /* In this implementation we only support TLS 1.3 and DTLS 1.3. */ @@ -121,7 +110,7 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, * NamedGroup named_group_list<2..2^16-1>; * } NamedGroupList; */ -static int mbedtls_ssl_tls13_parse_supported_groups_ext( +static int ssl_tls13_parse_supported_groups_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) { @@ -307,6 +296,7 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, } #endif /* MBEDTLS_ECDH_C */ +#if defined(MBEDTLS_SSL_DEBUG_C) static void ssl_tls13_debug_print_client_hello_exts( mbedtls_ssl_context *ssl ) { ((void) ssl); @@ -336,6 +326,7 @@ static void ssl_tls13_debug_print_client_hello_exts( mbedtls_ssl_context *ssl ) "TRUE" : "FALSE" ) ); #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ } +#endif /* MBEDTLS_SSL_DEBUG_C */ static int ssl_tls13_client_hello_has_exts( mbedtls_ssl_context *ssl, int ext_id_mask ) @@ -523,6 +514,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, * uint8 legacy_compression_method = 0; * ... */ + p += 1; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); if( p[0] != 0 ) { @@ -570,7 +562,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, * indicates the named groups which the client supports, * ordered from most preferred to least preferred. */ - ret = mbedtls_ssl_tls13_parse_supported_groups_ext( ssl, p, + ret = ssl_tls13_parse_supported_groups_ext( ssl, p, extension_data_end ); if( ret != 0 ) { @@ -693,7 +685,9 @@ have_ciphersuite: ssl->handshake->ciphersuite_info = ciphersuite_info; /* List all the extensions we have received */ +#if defined(MBEDTLS_SSL_DEBUG_C) ssl_tls13_debug_print_client_hello_exts( ssl ); +#endif /* MBEDTLS_SSL_DEBUG_C */ /* * Determine the key exchange algorithm to use. @@ -728,36 +722,10 @@ have_ciphersuite: /* Update the handshake state machine */ -static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context* ssl, - int hrr_required ) +static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context* ssl ) { int ret = 0; - if( ssl->handshake->hello_retry_requests_sent == 0 ) - { - hrr_required = SSL_CLIENT_HELLO_HRR_REQUIRED; - } - - if( hrr_required == SSL_CLIENT_HELLO_HRR_REQUIRED ) - { - /* - * Create stateless transcript hash for HRR - */ - MBEDTLS_SSL_DEBUG_MSG( 4, ( "Reset transcript for HRR" ) ); - ret = mbedtls_ssl_reset_transcript_for_hrr( ssl ); - if( ret != 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_reset_transcript_for_hrr", - ret ); - return( ret ); - } - mbedtls_ssl_session_reset_msg_layer( ssl, 0 ); - - /* Transmit Hello Retry Request */ - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_HELLO_RETRY_REQUEST ); - return( 0 ); - } - ret = mbedtls_ssl_tls13_key_schedule_stage_early( ssl ); if( ret != 0 ) { @@ -779,7 +747,6 @@ static int ssl_tls13_process_client_hello( mbedtls_ssl_context *ssl ) { int ret = 0; - int hrr_required = SSL_CLIENT_HELLO_OK; unsigned char* buf = NULL; size_t buflen = 0; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse client hello" ) ); @@ -791,11 +758,8 @@ static int ssl_tls13_process_client_hello( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_PROC_CHK_NEG( ssl_tls13_parse_client_hello( ssl, buf, buf + buflen ) ); - hrr_required = ret; - MBEDTLS_SSL_DEBUG_MSG( 1, ( "postprocess" ) ); - MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_client_hello( ssl, - hrr_required ) ); + MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_client_hello( ssl ) ); cleanup: @@ -821,7 +785,6 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) { /* start state */ case MBEDTLS_SSL_HELLO_REQUEST: - ssl->handshake->hello_retry_requests_sent = 0; mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_HELLO ); break; diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 416b89f6c2..dac868f49d 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -10218,9 +10218,7 @@ run_test "TLS 1.3: Server side check, ciphersuite TLS_AES_256_GCM_SHA384 - op "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ "$O_NEXT_CLI -msg -tls1_3" \ 1 \ - -s " tls13 server state: MBEDTLS_CLIENT_HELLO" \ - -s " tls13 server state: MBEDTLS_SERVER_HELLO" \ - -s ""SSL - The requested feature is not available"" \ + -s " tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ -s "=> parse client hello" \ -s "<= parse client hello" @@ -10235,9 +10233,7 @@ run_test "TLS 1.3: Server side check, ciphersuite TLS_AES_128_GCM_SHA256 - gn "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ "$G_NEXT_CLI -d 4 localhost --priority=NONE:+AES-128-GCM:+SHA256:+AEAD:+VERS-TLS1.3:%NO_TICKETS" \ 1 \ - -s " tls13 server state: MBEDTLS_CLIENT_HELLO" \ - -s " tls13 server state: MBEDTLS_SERVER_HELLO" \ - -s ""SSL - The requested feature is not available"" \ + -s " tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ -s "=> parse client hello" \ -s "<= parse client hello" diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 6d14086825..5aa8dd0b4b 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3279,7 +3279,7 @@ conf_version:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_TRANSPORT_DATAGRAM:MBEDTLS_SSL_VE Version config: unsupported server TLS 1.3 only depends_on:MBEDTLS_SSL_PROTO_TLS1_3 -conf_version:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_TRANSPORT_STREAM:MBEDTLS_SSL_VERSION_TLS1_3:MBEDTLS_SSL_VERSION_TLS1_3:MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE +conf_version:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_TRANSPORT_STREAM:3:4:3:4:0 Version config: unsupported server DTLS 1.3 only depends_on:MBEDTLS_SSL_PROTO_TLS1_3 From 8f9dfe41c00f0c42827b61ad16e1a57bb6960aed Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Fri, 15 Apr 2022 02:52:39 +0000 Subject: [PATCH 13/27] Fix comments about coding styles and test cases Change-Id: I70ebc05e9dd9fa084d7b0ce724a25464c3425e22 Signed-off-by: XiaokangQian --- include/mbedtls/ssl.h | 7 +-- library/ssl_tls.c | 7 +++ library/ssl_tls13_server.c | 81 ++++++++++++++------------------ tests/suites/test_suite_ssl.data | 2 +- 4 files changed, 44 insertions(+), 53 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 3fbe6f3e2c..d106137c92 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -326,9 +326,6 @@ #define MBEDTLS_SSL_SRV_CIPHERSUITE_ORDER_CLIENT 1 #define MBEDTLS_SSL_SRV_CIPHERSUITE_ORDER_SERVER 0 -#define MBEDTLS_SSL_FORCE_RR_CHECK_OFF 0 -#define MBEDTLS_SSL_FORCE_RR_CHECK_ON 1 - /* * Default range for DTLS retransmission timer value, in milliseconds. * RFC 6347 4.2.4.1 says from 1 second to 60 seconds. @@ -1699,9 +1696,7 @@ struct mbedtls_ssl_context /* * Information for DTLS hello verify */ -#if (defined(MBEDTLS_SSL_DTLS_HELLO_VERIFY) || \ - (defined(MBEDTLS_SSL_COOKIE_C) && defined(MBEDTLS_SSL_PROTO_TLS1_3))) && \ - defined(MBEDTLS_SSL_SRV_C) +#if defined(MBEDTLS_SSL_DTLS_HELLO_VERIFY) && defined(MBEDTLS_SSL_SRV_C) unsigned char *MBEDTLS_PRIVATE(cli_id); /*!< transport-level ID of the client */ size_t MBEDTLS_PRIVATE(cli_id_len); /*!< length of cli_id */ #endif /* MBEDTLS_SSL_DTLS_HELLO_VERIFY && MBEDTLS_SSL_SRV_C */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 54ea129617..11140569d6 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -940,6 +940,13 @@ static int ssl_conf_version_check( const mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); } + if( conf->endpoint == MBEDTLS_SSL_IS_SERVER ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "TLS 1.3 server is not supported yet." ) ); + return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + } + + MBEDTLS_SSL_DEBUG_MSG( 4, ( "The SSL configuration is TLS 1.3 or TLS 1.2." ) ); return( 0 ); } diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index d181cd294b..9ebab81c41 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -48,18 +48,17 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) { - size_t versions_len; - int tls13_supported = 0; - int major_ver, minor_ver; const unsigned char *p = buf; + size_t versions_len; const unsigned char *versions_end; + int major_ver, minor_ver; + int tls13_supported = 0; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); versions_len = p[0]; p += 1; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, versions_len ); - versions_end = p + versions_len; while( p < versions_end ) { @@ -79,10 +78,6 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, if( tls13_supported == 0 ) { - /* Here we only support TLS 1.3, we need report "bad protocol" if it - * doesn't support TLS 1.2. - */ - MBEDTLS_SSL_DEBUG_MSG( 1, ( "TLS 1.3 is not supported by the client" ) ); MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_PROTOCOL_VERSION, @@ -98,7 +93,7 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, return( 0 ); } -#if ( defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) ) +#if defined(MBEDTLS_ECDH_C) /* This function parses the TLS 1.3 supported_groups extension and * stores the received groups in ssl->handshake->curves. * @@ -114,23 +109,21 @@ static int ssl_tls13_parse_supported_groups_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) { - - size_t named_group_list_len, curve_list_len; const unsigned char *p = buf; + size_t named_group_list_len, curve_list_len; const mbedtls_ecp_curve_info *curve_info, **curves; - const unsigned char *extentions_end; + const unsigned char *named_group_list_end; MBEDTLS_SSL_DEBUG_BUF( 3, "supported_groups extension", p, end - buf ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); named_group_list_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, named_group_list_len ); - if( named_group_list_len % 2 != 0 ) - return( MBEDTLS_ERR_SSL_DECODE_ERROR ); - /* At the moment, this can happen when receiving a second - * ClientHello after an HRR. We should properly reset the - * state upon receiving an HRR, in which case we should - * not observe handshake->curves already being allocated. */ + /* At the moment, this can happen when receiving a second + * ClientHello after an HRR. We should properly reset the + * state upon receiving an HRR, in which case we should + * not observe handshake->curves already being allocated. */ if( ssl->handshake->curves != NULL ) { mbedtls_free( ssl->handshake->curves ); @@ -147,12 +140,14 @@ static int ssl_tls13_parse_supported_groups_ext( if( ( curves = mbedtls_calloc( curve_list_len, sizeof( *curves ) ) ) == NULL ) return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); - extentions_end = p + named_group_list_len; + named_group_list_end = p + named_group_list_len; ssl->handshake->curves = curves; - while ( p < extentions_end && curve_list_len > 1 ) + while ( p < named_group_list_end && curve_list_len > 1 ) { - uint16_t tls_grp_id = MBEDTLS_GET_UINT16_BE( p, 0 ); + uint16_t tls_grp_id; + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, named_group_list_end, 2 ); + tls_grp_id = MBEDTLS_GET_UINT16_BE( p, 0 ); curve_info = mbedtls_ecp_curve_info_from_tls_id( tls_grp_id ); /* mbedtls_ecp_curve_info_from_tls_id() uses the mbedtls_ecp_curve_info @@ -173,7 +168,7 @@ static int ssl_tls13_parse_supported_groups_ext( return( 0 ); } -#endif /* MBEDTLS_ECDH_C || ( MBEDTLS_ECDSA_C */ +#endif /* MBEDTLS_ECDH_C */ #if defined(MBEDTLS_ECDH_C) /* @@ -195,9 +190,9 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, { int ret = 0; unsigned char const *p = buf; - unsigned char const *extentions_end; + unsigned char const *client_shares_end; - size_t total_extensions_len, key_share_len; + size_t client_shares_len, key_exchange_len; int match_found = 0; /* From RFC 8446: @@ -208,15 +203,13 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, * */ - /* Read total legnth of KeyShareClientHello */ MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); - - total_extensions_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + client_shares_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, total_extensions_len ); + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, client_shares_len ); ssl->handshake->offered_group_id = 0; - extentions_end = p + total_extensions_len; + client_shares_end = p + client_shares_len; /* We try to find a suitable key share entry and copy it to the * handshake context. Later, we have to find out whether we can do @@ -224,7 +217,7 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, * dismiss it and send a HelloRetryRequest message. */ - for( ; p < extentions_end; p += key_share_len ) + for( ; p < client_shares_end; p += key_exchange_len ) { uint16_t group; @@ -234,12 +227,10 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, * opaque key_exchange<1..2^16-1>; * } KeyShareEntry; */ - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, extentions_end, 4 ); - + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, client_shares_end, 4 ); group = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; - - key_share_len = MBEDTLS_GET_UINT16_BE( p, 0 ); + key_exchange_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; /* Continue parsing even if we have already found a match, @@ -251,9 +242,9 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, /* * NamedGroup matching * - * For now, we only support ECDHE groups, but e.g. + * For now, we only support ECDHE groups. - * Type 1: ECDHE shares + * ECDHE shares * * - Check if we recognize the group * - Check if it's supported @@ -266,13 +257,11 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, if( curve_info == NULL ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid TLS curve group id" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + continue; } match_found = 1; - MBEDTLS_SSL_DEBUG_MSG( 2, ( "ECDH curve: %s", curve_info->name ) ); - ret = mbedtls_ssl_tls13_read_public_ecdhe_share( ssl, p, end - p ); if( ret != 0 ) return( ret ); @@ -296,7 +285,7 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, } #endif /* MBEDTLS_ECDH_C */ -#if defined(MBEDTLS_SSL_DEBUG_C) +#if defined(MBEDTLS_DEBUG_C) static void ssl_tls13_debug_print_client_hello_exts( mbedtls_ssl_context *ssl ) { ((void) ssl); @@ -326,13 +315,13 @@ static void ssl_tls13_debug_print_client_hello_exts( mbedtls_ssl_context *ssl ) "TRUE" : "FALSE" ) ); #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ } -#endif /* MBEDTLS_SSL_DEBUG_C */ +#endif /* MBEDTLS_DEBUG_C */ static int ssl_tls13_client_hello_has_exts( mbedtls_ssl_context *ssl, - int ext_id_mask ) + int exts_mask ) { - int masked = ssl->handshake->extensions_present & ext_id_mask; - return( masked == ext_id_mask ); + int masked = ssl->handshake->extensions_present & exts_mask; + return( masked == exts_mask ); } static int ssl_tls13_client_hello_has_cert_extensions( mbedtls_ssl_context *ssl ) @@ -685,9 +674,9 @@ have_ciphersuite: ssl->handshake->ciphersuite_info = ciphersuite_info; /* List all the extensions we have received */ -#if defined(MBEDTLS_SSL_DEBUG_C) +#if defined(MBEDTLS_DEBUG_C) ssl_tls13_debug_print_client_hello_exts( ssl ); -#endif /* MBEDTLS_SSL_DEBUG_C */ +#endif /* MBEDTLS_DEBUG_C */ /* * Determine the key exchange algorithm to use. diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 5aa8dd0b4b..94f79323b2 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3277,7 +3277,7 @@ Version config: unsupported client DTLS 1.3 only depends_on:MBEDTLS_SSL_PROTO_TLS1_3 conf_version:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_TRANSPORT_DATAGRAM:MBEDTLS_SSL_VERSION_TLS1_3:MBEDTLS_SSL_VERSION_TLS1_3:MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE -Version config: unsupported server TLS 1.3 only +Version config: supported server TLS 1.3 only depends_on:MBEDTLS_SSL_PROTO_TLS1_3 conf_version:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_TRANSPORT_STREAM:3:4:3:4:0 From f8ceb94fe77a1dc48a250c94e80b5350ce5105d5 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Fri, 15 Apr 2022 11:43:27 +0000 Subject: [PATCH 14/27] Fix the parse_sig_alg_ext fail issue Change-Id: Ib31e0929c5b6868ab6c3023b20472321fc07ba3c Signed-off-by: XiaokangQian --- library/ssl_tls13_generic.c | 8 ++++++-- library/ssl_tls13_server.c | 6 ++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index fabc5d1587..12b7223845 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -115,8 +115,12 @@ 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_supported( ssl, sig_alg ) +#if defined(MBEDTLS_SSL_CLI_C) + || ( ( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) + && ! mbedtls_ssl_sig_alg_is_offered( ssl, sig_alg ) ) +#endif /* MBEDTLS_SSL_CLI_C */ + ) continue; if( common_idx + 1 < MBEDTLS_RECEIVED_SIG_ALGS_SIZE ) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 9ebab81c41..5aed7ffb71 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -448,6 +448,12 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, } p += 2; + /* + * Only support TLS 1.3 currently, temporarily set the version. + */ + ssl->major_ver = MBEDTLS_SSL_MAJOR_VERSION_3; + ssl->minor_ver = MBEDTLS_SSL_MINOR_VERSION_4; + /* * Save client random */ From b67384d05ca601cf27bdf4d222b055603da955e5 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Tue, 19 Apr 2022 00:02:38 +0000 Subject: [PATCH 15/27] Fix coding style and comments styles Change-Id: Ifa37a3288fbb6b5206fc0640fa11fa36cb3189ff Signed-off-by: XiaokangQian --- library/ssl_tls13_server.c | 183 ++++++++++++++++++------------------- 1 file changed, 91 insertions(+), 92 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 5aed7ffb71..2122a6e070 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -64,6 +64,7 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, { MBEDTLS_SSL_CHK_BUF_READ_PTR( p, versions_end, 2 ); mbedtls_ssl_read_version( &major_ver, &minor_ver, ssl->conf->transport, p ); + p += 2; /* In this implementation we only support TLS 1.3 and DTLS 1.3. */ if( major_ver == MBEDTLS_SSL_MAJOR_VERSION_3 && @@ -72,11 +73,9 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, tls13_supported = 1; break; } - - p += 2; } - if( tls13_supported == 0 ) + if( !tls13_supported ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "TLS 1.3 is not supported by the client" ) ); @@ -188,10 +187,9 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) { - int ret = 0; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char const *p = buf; unsigned char const *client_shares_end; - size_t client_shares_len, key_exchange_len; int match_found = 0; @@ -232,6 +230,7 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, p += 2; key_exchange_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, client_shares_end, key_exchange_len ); /* Continue parsing even if we have already found a match, * for input validation purposes. @@ -240,16 +239,8 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, continue; /* - * NamedGroup matching - * * For now, we only support ECDHE groups. - - * ECDHE shares - * - * - Check if we recognize the group - * - Check if it's supported */ - if( mbedtls_ssl_tls13_named_group_is_ecdhe( group ) ) { const mbedtls_ecp_curve_info *curve_info = @@ -262,7 +253,7 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, match_found = 1; MBEDTLS_SSL_DEBUG_MSG( 2, ( "ECDH curve: %s", curve_info->name ) ); - ret = mbedtls_ssl_tls13_read_public_ecdhe_share( ssl, p, end - p ); + ret = mbedtls_ssl_tls13_read_public_ecdhe_share( ssl, p - 2, end - p + 2 ); if( ret != 0 ) return( ret ); } @@ -291,28 +282,39 @@ static void ssl_tls13_debug_print_client_hello_exts( mbedtls_ssl_context *ssl ) ((void) ssl); MBEDTLS_SSL_DEBUG_MSG( 3, ( "Supported Extensions:" ) ); - MBEDTLS_SSL_DEBUG_MSG( 3, ( "- KEY_SHARE_EXTENSION ( %s )", - ( ( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_KEY_SHARE ) > 0 ) ? - "TRUE" : "FALSE" ) ); - MBEDTLS_SSL_DEBUG_MSG( 3, ( "- PSK_KEY_EXCHANGE_MODES_EXTENSION ( %s )", - ( ( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_PSK_KEY_EXCHANGE_MODES ) > 0 ) ? - "TRUE" : "FALSE" ) ); - MBEDTLS_SSL_DEBUG_MSG( 3, ( "- PRE_SHARED_KEY_EXTENSION ( %s )", - ( ( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_PRE_SHARED_KEY ) > 0 ) ? - "TRUE" : "FALSE" ) ); - MBEDTLS_SSL_DEBUG_MSG( 3, ( "- SIGNATURE_ALGORITHM_EXTENSION ( %s )", - ( ( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_SIG_ALG ) > 0 ) ? - "TRUE" : "FALSE" ) ); - MBEDTLS_SSL_DEBUG_MSG( 3, ( "- SUPPORTED_GROUPS_EXTENSION ( %s )", - ( ( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_SUPPORTED_GROUPS ) >0 ) ? - "TRUE" : "FALSE" ) ); - MBEDTLS_SSL_DEBUG_MSG( 3, ( "- SUPPORTED_VERSION_EXTENSION ( %s )", - ( ( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_SUPPORTED_VERSIONS ) > 0 ) ? - "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "- KEY_SHARE_EXTENSION ( %s )", + ( ( ssl->handshake->extensions_present + & MBEDTLS_SSL_EXT_KEY_SHARE ) > 0 ) ? "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "- PSK_KEY_EXCHANGE_MODES_EXTENSION ( %s )", + ( ( ssl->handshake->extensions_present + & MBEDTLS_SSL_EXT_PSK_KEY_EXCHANGE_MODES ) > 0 ) ? + "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "- PRE_SHARED_KEY_EXTENSION ( %s )", + ( ( ssl->handshake->extensions_present + & MBEDTLS_SSL_EXT_PRE_SHARED_KEY ) > 0 ) ? "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "- SIGNATURE_ALGORITHM_EXTENSION ( %s )", + ( ( ssl->handshake->extensions_present + & MBEDTLS_SSL_EXT_SIG_ALG ) > 0 ) ? "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "- SUPPORTED_GROUPS_EXTENSION ( %s )", + ( ( ssl->handshake->extensions_present + & MBEDTLS_SSL_EXT_SUPPORTED_GROUPS ) >0 ) ? + "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "- SUPPORTED_VERSION_EXTENSION ( %s )", + ( ( ssl->handshake->extensions_present + & MBEDTLS_SSL_EXT_SUPPORTED_VERSIONS ) > 0 ) ? + "TRUE" : "FALSE" ) ); #if defined ( MBEDTLS_SSL_SERVER_NAME_INDICATION ) - MBEDTLS_SSL_DEBUG_MSG( 3, ( "- SERVERNAME_EXTENSION ( %s )", - ( ( ssl->handshake->extensions_present & MBEDTLS_SSL_EXT_SERVERNAME ) > 0 ) ? - "TRUE" : "FALSE" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "- SERVERNAME_EXTENSION ( %s )", + ( ( ssl->handshake->extensions_present + & MBEDTLS_SSL_EXT_SERVERNAME ) > 0 ) ? + "TRUE" : "FALSE" ) ); #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ } #endif /* MBEDTLS_DEBUG_C */ @@ -324,7 +326,8 @@ static int ssl_tls13_client_hello_has_exts( mbedtls_ssl_context *ssl, return( masked == exts_mask ); } -static int ssl_tls13_client_hello_has_cert_extensions( mbedtls_ssl_context *ssl ) +static int ssl_tls13_client_hello_has_exts_for_ephemeral_key_exchange( + mbedtls_ssl_context *ssl ) { return( ssl_tls13_client_hello_has_exts( ssl, MBEDTLS_SSL_EXT_SUPPORTED_GROUPS | @@ -332,15 +335,16 @@ static int ssl_tls13_client_hello_has_cert_extensions( mbedtls_ssl_context *ssl MBEDTLS_SSL_EXT_SIG_ALG ) ); } -static int ssl_tls13_check_certificate_key_exchange( mbedtls_ssl_context *ssl ) +static int ssl_tls13_check_ephemeral_key_exchange( mbedtls_ssl_context *ssl ) { if( !mbedtls_ssl_conf_tls13_ephemeral_enabled( ssl ) ) return( 0 ); - if( !ssl_tls13_client_hello_has_cert_extensions( ssl ) ) + if( !ssl_tls13_client_hello_has_exts_for_ephemeral_key_exchange( ssl ) ) return( 0 ); - ssl->handshake->tls13_kex_modes = MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL; + ssl->handshake->tls13_kex_modes = + MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL; return( 1 ); } @@ -348,20 +352,21 @@ static int ssl_tls13_check_certificate_key_exchange( mbedtls_ssl_context *ssl ) * * STATE HANDLING: ClientHello * - * There are three possible classes of outcomes when parsing the CH: + * There are three possible classes of outcomes when parsing the ClientHello: * - * 1) The CH was well-formed and matched the server's configuration. + * 1) The ClientHello was well-formed and matched the server's configuration. * * In this case, the server progresses to sending its ServerHello. * - * 2) The CH was well-formed but didn't match the server's configuration. + * 2) The ClientHello was well-formed but didn't match the server's + * configuration. * * For example, the client might not have offered a key share which * the server supports, or the server might require a cookie. * * In this case, the server sends a HelloRetryRequest. * - * 3) The CH was ill-formed + * 3) The ClientHello was ill-formed * * In this case, we abort the handshake. * @@ -372,7 +377,6 @@ static int ssl_tls13_check_certificate_key_exchange( mbedtls_ssl_context *ssl ) * * uint16 ProtocolVersion; * opaque Random[32]; - * * uint8 CipherSuite[2]; // Cryptographic suite selector * * struct { @@ -392,30 +396,26 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, const unsigned char *buf, const unsigned char *end ) { - int ret; - size_t i, j; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + const unsigned char *p = buf; size_t legacy_session_id_len; size_t cipher_suites_len; - size_t extensions_len; const unsigned char *cipher_suites_start; - const unsigned char *p = buf; + size_t extensions_len; const unsigned char *extensions_end; const int* cipher_suites; const mbedtls_ssl_ciphersuite_t* ciphersuite_info; - int hrr_required = 0; ssl->handshake->extensions_present = MBEDTLS_SSL_EXT_NONE; /* - * ClientHello layer: + * ClientHello layout: * 0 . 1 protocol version * 2 . 33 random bytes ( starting with 4 bytes of Unix time ) * 34 . 34 session id length ( 1 byte ) * 35 . 34+x session id - * 35+x . 35+x DTLS only: cookie length ( 1 byte ) - * 36+x . .. DTLS only: cookie * .. . .. ciphersuite list length ( 2 bytes ) * .. . .. ciphersuite list * .. . .. compression alg. list length ( 1 byte ) @@ -424,7 +424,8 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, * .. . .. extensions ( optional ) */ - /* Needs to be updated due to mandatory extensions + /* + * Needs to be updated due to mandatory extensions * Minimal length ( with everything empty and extensions ommitted ) is * 2 + 32 + 1 + 2 + 1 = 38 bytes. Check that first, so that we can * read at least up to session id length without worrying. @@ -443,8 +444,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 1, ( "Unsupported version of TLS." ) ); MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_PROTOCOL_VERSION, MBEDTLS_ERR_SSL_BAD_PROTOCOL_VERSION ); - ret = MBEDTLS_ERR_SSL_BAD_PROTOCOL_VERSION; - return ret; + return ( MBEDTLS_ERR_SSL_BAD_PROTOCOL_VERSION ); } p += 2; @@ -456,6 +456,12 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, /* * Save client random + * + * --- + * Random random; + * --- + * with Random defined as: + * opaque Random[32]; */ MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, random bytes", p, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); @@ -463,38 +469,38 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, memcpy( &ssl->handshake->randbytes[0], p, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); p += MBEDTLS_SERVER_HELLO_RANDOM_LEN; - /* - * Parse session ID + /* --- + * opaque legacy_session_id<0..32>; + * --- */ legacy_session_id_len = p[0]; p++; - if( legacy_session_id_len > 32 ) + if( legacy_session_id_len > sizeof( ssl->session_negotiate->id ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad client hello message" ) ); return( MBEDTLS_ERR_SSL_DECODE_ERROR ); } ssl->session_negotiate->id_len = legacy_session_id_len; - - /* Note that this field is echoed even if - * the client's value corresponded to a cached pre-TLS 1.3 session - * which the server has chosen not to resume. A client which - * receives a legacy_session_id_echo field that does not match what - * it sent in the ClientHello MUST abort the handshake with an - * "illegal_parameter" alert. - */ MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, session id", buf, legacy_session_id_len ); + /* + * Check we have enough data for the legacy session identifier + * and the ciphersuite list length. + */ + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, legacy_session_id_len + 2 ); memcpy( &ssl->session_negotiate->id[0], p, legacy_session_id_len ); p += legacy_session_id_len; - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); cipher_suites_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, cipher_suites_len ); + /* Check we have enough data for the ciphersuite list, the legacy + * compression methods and the length of the extensions. + */ + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, cipher_suites_len + 2 + 2 ); /* store pointer to ciphersuite list */ cipher_suites_start = p; @@ -506,24 +512,27 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, p += cipher_suites_len; /* ... - * uint8 legacy_compression_method = 0; + * opaque legacy_compression_methods<1..2^8-1>; * ... */ - p += 1; - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); - if( p[0] != 0 ) + if( p[0] != 1 || p[1] != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad legacy compression method" ) ); MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER, MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); return ( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); } - p++; + p += 2; - /* - * Check the extensions length + /* --- + * Extension extensions<8..2^16-1>; + * --- + * with Extension defined as: + * struct { + * ExtensionType extension_type; + * opaque extension_data<0..2^16-1>; + * } Extension; */ - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 2 ); extensions_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; extensions_end = p + extensions_len; @@ -547,7 +556,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, switch( extension_type ) { -#if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) +#if defined(MBEDTLS_ECDH_C) case MBEDTLS_TLS_EXT_SUPPORTED_GROUPS: MBEDTLS_SSL_DEBUG_MSG( 3, ( "found supported group extension" ) ); @@ -568,7 +577,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, ssl->handshake->extensions_present |= MBEDTLS_SSL_EXT_SUPPORTED_GROUPS; break; -#endif /* MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C */ +#endif /* MBEDTLS_ECDH_C */ #if defined(MBEDTLS_ECDH_C) case MBEDTLS_TLS_EXT_KEY_SHARE: @@ -599,7 +608,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "found supported versions extension" ) ); ret = ssl_tls13_parse_supported_versions_ext( - ssl, p, extension_data_end ); + ssl, p, extension_data_end ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, @@ -644,6 +653,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, /* * Search for a matching ciphersuite */ + size_t i, j; cipher_suites = ssl->conf->ciphersuite_list; ciphersuite_info = NULL; for ( j = 0, p = cipher_suites_start; j < cipher_suites_len; j += 2, p += 2 ) @@ -685,21 +695,10 @@ have_ciphersuite: #endif /* MBEDTLS_DEBUG_C */ /* - * Determine the key exchange algorithm to use. - * There are three types of key exchanges supported in TLS 1.3: - * - (EC)DH with ECDSA, - * - (EC)DH with PSK, - * - plain PSK. - * - * The PSK-based key exchanges may additionally be used with 0-RTT. - * - * Our built-in order of preference is - * 1 ) Plain PSK Mode - * 2 ) (EC)DHE-PSK Mode - * 3 ) Certificate Mode + * Here we only support the ephemeral or (EC)DHE key echange mode */ - if( !ssl_tls13_check_certificate_key_exchange( ssl ) ) + if( !ssl_tls13_check_ephemeral_key_exchange( ssl ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, From 3f84d5d0cd8c58ca92f448a664fc6a25c9a98697 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Tue, 19 Apr 2022 06:36:17 +0000 Subject: [PATCH 16/27] Update test cases and fix the test failure Change-Id: If93506fc3764d49836b229d51e4ad5b008cc3343 Signed-off-by: XiaokangQian --- library/ssl_tls13_server.c | 13 ++++++++++++- tests/ssl-opt.sh | 6 +++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 2122a6e070..9e0ea7950e 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -253,6 +253,12 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, match_found = 1; MBEDTLS_SSL_DEBUG_MSG( 2, ( "ECDH curve: %s", curve_info->name ) ); + ret = psa_crypto_init(); + if( ret != PSA_SUCCESS ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "psa_crypto_init()", ret ); + return( ret ); + } ret = mbedtls_ssl_tls13_read_public_ecdhe_share( ssl, p - 2, end - p + 2 ); if( ret != 0 ) return( ret ); @@ -648,7 +654,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, * - The entire content of the CH message, if no PSK extension is present * - The content up to but excluding the PSK extension, if present. */ - mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_SERVER_HELLO, + mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_CLIENT_HELLO, buf, p - buf ); /* * Search for a matching ciphersuite @@ -793,6 +799,11 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) break; + case MBEDTLS_SSL_SERVER_HELLO: + MBEDTLS_SSL_DEBUG_MSG( 1, ( "SSL - The requested feature is not available" ) ); + return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + + break; default: MBEDTLS_SSL_DEBUG_MSG( 1, ( "invalid state %d", ssl->state ) ); return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index dac868f49d..d870076884 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -10219,6 +10219,8 @@ run_test "TLS 1.3: Server side check, ciphersuite TLS_AES_256_GCM_SHA384 - op "$O_NEXT_CLI -msg -tls1_3" \ 1 \ -s " tls13 server state: MBEDTLS_SSL_CLIENT_HELLO" \ + -s " tls13 server state: MBEDTLS_SSL_SERVER_HELLO" \ + -s " SSL - The requested feature is not available" \ -s "=> parse client hello" \ -s "<= parse client hello" @@ -10231,9 +10233,11 @@ requires_config_enabled MBEDTLS_SSL_CLI_C requires_config_disabled MBEDTLS_USE_PSA_CRYPTO run_test "TLS 1.3: Server side check, ciphersuite TLS_AES_128_GCM_SHA256 - gnutls" \ "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ - "$G_NEXT_CLI -d 4 localhost --priority=NONE:+AES-128-GCM:+SHA256:+AEAD:+VERS-TLS1.3:%NO_TICKETS" \ + "$G_NEXT_CLI localhost -d 4 --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 " SSL - The requested feature is not available" \ -s "=> parse client hello" \ -s "<= parse client hello" From 84823779cea83da639a7576b96f10a59df19dcfc Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Tue, 19 Apr 2022 07:57:30 +0000 Subject: [PATCH 17/27] Only store the first group in ssl_tls13_parse_supported_groups_ext() Change-Id: I4427149aeb6eb453150e522e4c7b11187e2e3825 Signed-off-by: XiaokangQian --- library/ssl_tls13_server.c | 42 +++++++++----------------------------- 1 file changed, 10 insertions(+), 32 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 9e0ea7950e..20765560c8 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -109,8 +109,8 @@ static int ssl_tls13_parse_supported_groups_ext( const unsigned char *buf, const unsigned char *end ) { const unsigned char *p = buf; - size_t named_group_list_len, curve_list_len; - const mbedtls_ecp_curve_info *curve_info, **curves; + size_t named_group_list_len; + const mbedtls_ecp_curve_info *curve_info; const unsigned char *named_group_list_end; MBEDTLS_SSL_DEBUG_BUF( 3, "supported_groups extension", p, end - buf ); @@ -118,47 +118,25 @@ static int ssl_tls13_parse_supported_groups_ext( named_group_list_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, named_group_list_len ); - - /* At the moment, this can happen when receiving a second - * ClientHello after an HRR. We should properly reset the - * state upon receiving an HRR, in which case we should - * not observe handshake->curves already being allocated. */ - if( ssl->handshake->curves != NULL ) - { - mbedtls_free( ssl->handshake->curves ); - ssl->handshake->curves = NULL; - } - - /* Don't allow our peer to make us allocate too much memory, - * and leave room for a final 0 - */ - curve_list_len = named_group_list_len / 2 + 1; - if( curve_list_len > MBEDTLS_ECP_DP_MAX ) - curve_list_len = MBEDTLS_ECP_DP_MAX; - - if( ( curves = mbedtls_calloc( curve_list_len, sizeof( *curves ) ) ) == NULL ) - return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); - named_group_list_end = p + named_group_list_len; - ssl->handshake->curves = curves; - while ( p < named_group_list_end && curve_list_len > 1 ) + while ( p < named_group_list_end ) { uint16_t tls_grp_id; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, named_group_list_end, 2 ); tls_grp_id = MBEDTLS_GET_UINT16_BE( p, 0 ); curve_info = mbedtls_ecp_curve_info_from_tls_id( tls_grp_id ); - /* mbedtls_ecp_curve_info_from_tls_id() uses the mbedtls_ecp_curve_info - * data structure (defined in ecp.c), which only includes the list of - * curves implemented. Hence, we only add curves that are also supported - * and implemented by the server. - */ if( curve_info != NULL ) { - *curves++ = curve_info; + MBEDTLS_SSL_DEBUG_MSG( 4, ( "supported curve: %s", curve_info->name ) ); - curve_list_len--; + /* + * Here we only update offered_group_id field with the first + * offered group + */ + ssl->handshake->offered_group_id = tls_grp_id; + break; } p += 2; From 17f974c63e2b0755d18949797d269ecf69556c14 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Tue, 19 Apr 2022 09:57:41 +0000 Subject: [PATCH 18/27] Re-order the ciphersuite matching code in parse_client_hello Change-Id: I16d11bca42993d4abc2a1b19fa087366c591927c Signed-off-by: XiaokangQian --- library/ssl_tls13_server.c | 81 +++++++++++++++++++------------------- 1 file changed, 41 insertions(+), 40 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 20765560c8..5c926743ff 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -492,9 +492,48 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, ciphersuitelist", p, cipher_suites_len ); - /* skip cipher_suites for now */ - p += cipher_suites_len; + /* + * Search for a matching ciphersuite + */ + size_t ciphersuite_exist = 0; + cipher_suites = ssl->conf->ciphersuite_list; + ciphersuite_info = NULL; + for ( size_t j = 0; j < cipher_suites_len; + j += 2, p += 2 ) + { + for ( size_t i = 0; cipher_suites[i] != 0; i++ ) + { + if( MBEDTLS_GET_UINT16_BE(p, 0) == cipher_suites[i] ) + { + ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( + cipher_suites[i] ); + if( ciphersuite_info == NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( + 1, + ( "mbedtls_ssl_ciphersuite_from_id: should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + + ssl->session_negotiate->ciphersuite = cipher_suites[i]; + ssl->handshake->ciphersuite_info = ciphersuite_info; + ciphersuite_exist = 1; + + break; + + } + + } + } + + if( !ciphersuite_exist ) + return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "selected ciphersuite: %s", + ciphersuite_info->name ) ); + + p = cipher_suites_start + cipher_suites_len; /* ... * opaque legacy_compression_methods<1..2^8-1>; * ... @@ -634,44 +673,6 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, */ mbedtls_ssl_add_hs_msg_to_checksum( ssl, MBEDTLS_SSL_HS_CLIENT_HELLO, buf, p - buf ); - /* - * Search for a matching ciphersuite - */ - size_t i, j; - cipher_suites = ssl->conf->ciphersuite_list; - ciphersuite_info = NULL; - for ( j = 0, p = cipher_suites_start; j < cipher_suites_len; j += 2, p += 2 ) - { - for ( i = 0; cipher_suites[i] != 0; i++ ) - { - if( MBEDTLS_GET_UINT16_BE(p, 0) != cipher_suites[i] ) - continue; - - ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( - cipher_suites[i] ); - - if( ciphersuite_info == NULL ) - { - MBEDTLS_SSL_DEBUG_MSG( - 1, - ( "mbedtls_ssl_ciphersuite_from_id: should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); - } - - goto have_ciphersuite; - - } - } - - return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); - -have_ciphersuite: - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "selected ciphersuite: %s", - ciphersuite_info->name ) ); - - ssl->session_negotiate->ciphersuite = cipher_suites[i]; - ssl->handshake->ciphersuite_info = ciphersuite_info; /* List all the extensions we have received */ #if defined(MBEDTLS_DEBUG_C) From 08037553479dff14665ec646089dcf8c8912a161 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 20 Apr 2022 07:16:41 +0000 Subject: [PATCH 19/27] Update code base on review comments Refine named_group parsing Refine cipher_suites parsing Remove hrr related part Share code between client and server side Some code style changes Change-Id: Ia9ffd5ef9c0b64325f633241e0ea1669049fe33a Signed-off-by: XiaokangQian --- include/mbedtls/ssl.h | 2 - library/ssl_misc.h | 41 +++++++++++++ library/ssl_tls13_client.c | 18 +----- library/ssl_tls13_generic.c | 24 ++++++-- library/ssl_tls13_server.c | 111 +++++++++++++++++------------------- 5 files changed, 113 insertions(+), 83 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index d106137c92..16939565fe 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -98,8 +98,6 @@ /* Error space gap */ /** Processing of the Certificate handshake message failed. */ #define MBEDTLS_ERR_SSL_BAD_CERTIFICATE -0x7A00 -/** Server needs to send a HelloRetryRequest */ -#define MBEDTLS_ERR_SSL_HRR_REQUIRED -0x7A80 /* Error space gap */ /* Error space gap */ /* Error space gap */ diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 9a7c503259..f079e687d9 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -586,6 +586,11 @@ struct mbedtls_ssl_handshake_params int hello_retry_request_count; #endif /* MBEDTLS_SSL_CLI_C */ +#if defined(MBEDTLS_SSL_SRV_C) + /*!< selected_group of key_share extension in HelloRetryRequest message. */ + uint16_t hrr_selected_group; +#endif /* MBEDTLS_SSL_SRV_C */ + #if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED) mbedtls_ssl_sig_hash_set_t hash_algs; /*!< Set of suitable sig-hash pairs */ @@ -1856,6 +1861,39 @@ static inline int mbedtls_ssl_tls13_named_group_is_dhe( uint16_t named_group ) named_group <= MBEDTLS_SSL_IANA_TLS_GROUP_FFDHE8192 ); } +static inline int mbedtls_ssl_named_group_is_offered( + const mbedtls_ssl_context *ssl, uint16_t named_group ) +{ + const uint16_t *group_list = mbedtls_ssl_get_groups( ssl ); + + if( group_list == NULL ) + return( 0 ); + + for( ; *group_list != 0; group_list++ ) + { + if( *group_list == named_group ) + return( 1 ); + } + + return( 0 ); +} + +static inline int mbedtls_ssl_named_group_is_supported( uint16_t named_group ) +{ +#if defined(MBEDTLS_ECDH_C) + if( mbedtls_ssl_tls13_named_group_is_ecdhe( named_group ) ) + { + const mbedtls_ecp_curve_info *curve_info = + mbedtls_ecp_curve_info_from_tls_id( named_group ); + if( curve_info != NULL ) + return( 1 ); + } +#else + ((void) named_group); +#endif /* MBEDTLS_ECDH_C */ + return( 0 ); +} + /* * Return supported signature algorithms. * @@ -2180,4 +2218,7 @@ int mbedtls_ssl_tls13_read_public_ecdhe_share( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_ECDH_C */ +int mbedtls_ssl_tls13_cipher_suite_is_offered( mbedtls_ssl_context *ssl, + int cipher_suite ); + #endif /* ssl_misc.h */ diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 198c20a2a3..c40fb879b6 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -939,22 +939,6 @@ static int ssl_tls13_check_server_hello_session_id_echo( mbedtls_ssl_context *ss return( 0 ); } -static int ssl_tls13_cipher_suite_is_offered( mbedtls_ssl_context *ssl, - int cipher_suite ) -{ - const int *ciphersuite_list = ssl->conf->ciphersuite_list; - - /* Check whether we have offered this ciphersuite */ - for ( size_t i = 0; ciphersuite_list[i] != 0; i++ ) - { - if( ciphersuite_list[i] == cipher_suite ) - { - return( 1 ); - } - } - return( 0 ); -} - /* Parse ServerHello message and configure context * * struct { @@ -1054,7 +1038,7 @@ static int ssl_tls13_parse_server_hello( mbedtls_ssl_context *ssl, if( ( mbedtls_ssl_validate_ciphersuite( ssl, ciphersuite_info, ssl->tls_version, ssl->tls_version ) != 0 ) || - !ssl_tls13_cipher_suite_is_offered( ssl, cipher_suite ) ) + !mbedtls_ssl_tls13_cipher_suite_is_offered( ssl, cipher_suite ) ) { fatal_alert = MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER; } diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 12b7223845..1bcafe4927 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -115,12 +115,8 @@ 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_supported( ssl, sig_alg ) -#if defined(MBEDTLS_SSL_CLI_C) - || ( ( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) - && ! mbedtls_ssl_sig_alg_is_offered( ssl, sig_alg ) ) -#endif /* MBEDTLS_SSL_CLI_C */ - ) + if( ! mbedtls_ssl_sig_alg_is_supported( ssl, sig_alg ) || + ! mbedtls_ssl_sig_alg_is_offered( ssl, sig_alg ) ) continue; if( common_idx + 1 < MBEDTLS_RECEIVED_SIG_ALGS_SIZE ) @@ -1541,4 +1537,20 @@ int mbedtls_ssl_tls13_read_public_ecdhe_share( mbedtls_ssl_context *ssl, } #endif /* MBEDTLS_ECDH_C */ +int mbedtls_ssl_tls13_cipher_suite_is_offered( mbedtls_ssl_context *ssl, + int cipher_suite ) +{ + const int *ciphersuite_list = ssl->conf->ciphersuite_list; + + /* Check whether we have offered this ciphersuite */ + for ( size_t i = 0; ciphersuite_list[i] != 0; i++ ) + { + if( ciphersuite_list[i] == cipher_suite ) + { + return( 1 ); + } + } + return( 0 ); +} + #endif /* MBEDTLS_SSL_TLS_C && MBEDTLS_SSL_PROTO_TLS1_3 */ diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 5c926743ff..f2e7862e10 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -24,6 +24,7 @@ #include "mbedtls/debug.h" #include "ssl_misc.h" +#include "ssl_client.h" #include "ssl_tls13_keys.h" #include "ssl_debug_helpers.h" #include @@ -110,7 +111,6 @@ static int ssl_tls13_parse_supported_groups_ext( { const unsigned char *p = buf; size_t named_group_list_len; - const mbedtls_ecp_curve_info *curve_info; const unsigned char *named_group_list_end; MBEDTLS_SSL_DEBUG_BUF( 3, "supported_groups extension", p, end - buf ); @@ -119,27 +119,30 @@ static int ssl_tls13_parse_supported_groups_ext( p += 2; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, named_group_list_len ); named_group_list_end = p + named_group_list_len; + ssl->handshake->hrr_selected_group = 0; - while ( p < named_group_list_end ) + while( p < named_group_list_end ) { - uint16_t tls_grp_id; + uint16_t named_group; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, named_group_list_end, 2 ); - tls_grp_id = MBEDTLS_GET_UINT16_BE( p, 0 ); - curve_info = mbedtls_ecp_curve_info_from_tls_id( tls_grp_id ); + named_group = MBEDTLS_GET_UINT16_BE( p, 0 ); + p += 2; - if( curve_info != NULL ) + MBEDTLS_SSL_DEBUG_MSG( + 2, ( "got named group: %d", + named_group ) ); + + if( ! mbedtls_ssl_named_group_is_offered( ssl, named_group ) || + ! mbedtls_ssl_named_group_is_supported( named_group ) || + ssl->handshake->hrr_selected_group != 0 ) { - - MBEDTLS_SSL_DEBUG_MSG( 4, ( "supported curve: %s", curve_info->name ) ); - /* - * Here we only update offered_group_id field with the first - * offered group - */ - ssl->handshake->offered_group_id = tls_grp_id; - break; + continue; } - p += 2; + MBEDTLS_SSL_DEBUG_MSG( + 2, ( "add named group (%04x) into received list.", + named_group ) ); + ssl->handshake->hrr_selected_group = named_group; } return( 0 ); @@ -147,6 +150,8 @@ static int ssl_tls13_parse_supported_groups_ext( } #endif /* MBEDTLS_ECDH_C */ +#define SSL_TLS1_3_PARSE_KEY_SHARES_EXT_NO_MATCH 1 + #if defined(MBEDTLS_ECDH_C) /* * ssl_tls13_parse_key_shares_ext() verifies whether the information in the @@ -155,7 +160,7 @@ static int ssl_tls13_parse_supported_groups_ext( * * Possible return values are: * - 0: Successful processing of the client provided key share extension. - * - MBEDTLS_ERR_SSL_HRR_REQUIRED: The key share provided by the client + * - SSL_TLS1_3_PARSE_KEY_SHARES_EXT_NO_MATCH: The key shares provided by the client * does not match a group supported by the server. A HelloRetryRequest will * be needed. * - Another negative return value for fatal errors. @@ -237,7 +242,7 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_RET( 1, "psa_crypto_init()", ret ); return( ret ); } - ret = mbedtls_ssl_tls13_read_public_ecdhe_share( ssl, p - 2, end - p + 2 ); + ret = mbedtls_ssl_tls13_read_public_ecdhe_share( ssl, p - 2, key_exchange_len + 2 ); if( ret != 0 ) return( ret ); } @@ -254,7 +259,7 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, if( match_found == 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "no matching key share" ) ); - return( MBEDTLS_ERR_SSL_HRR_REQUIRED ); + return( SSL_TLS1_3_PARSE_KEY_SHARES_EXT_NO_MATCH ); } return( 0 ); } @@ -388,7 +393,6 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, size_t extensions_len; const unsigned char *extensions_end; - const int* cipher_suites; const mbedtls_ssl_ciphersuite_t* ciphersuite_info; int hrr_required = 0; @@ -409,7 +413,6 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, */ /* - * Needs to be updated due to mandatory extensions * Minimal length ( with everything empty and extensions ommitted ) is * 2 + 32 + 1 + 2 + 1 = 38 bytes. Check that first, so that we can * read at least up to session id length without worrying. @@ -438,20 +441,17 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, ssl->major_ver = MBEDTLS_SSL_MAJOR_VERSION_3; ssl->minor_ver = MBEDTLS_SSL_MINOR_VERSION_4; - /* - * Save client random - * - * --- + /* --- * Random random; * --- * with Random defined as: * opaque Random[32]; */ MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, random bytes", - p, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); + p, MBEDTLS_CLIENT_HELLO_RANDOM_LEN ); - memcpy( &ssl->handshake->randbytes[0], p, MBEDTLS_SERVER_HELLO_RANDOM_LEN ); - p += MBEDTLS_SERVER_HELLO_RANDOM_LEN; + memcpy( &ssl->handshake->randbytes[0], p, MBEDTLS_CLIENT_HELLO_RANDOM_LEN ); + p += MBEDTLS_CLIENT_HELLO_RANDOM_LEN; /* --- * opaque legacy_session_id<0..32>; @@ -486,45 +486,41 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, */ MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, cipher_suites_len + 2 + 2 ); - /* store pointer to ciphersuite list */ + /* --- + * CipherSuite cipher_suites<2..2^16-2>; + * --- + * with CipherSuite defined as: + * uint8 CipherSuite[2]; + */ cipher_suites_start = p; - MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, ciphersuitelist", p, cipher_suites_len ); - /* * Search for a matching ciphersuite */ size_t ciphersuite_exist = 0; - cipher_suites = ssl->conf->ciphersuite_list; + uint16_t cipher_suite; ciphersuite_info = NULL; for ( size_t j = 0; j < cipher_suites_len; j += 2, p += 2 ) { - for ( size_t i = 0; cipher_suites[i] != 0; i++ ) - { - if( MBEDTLS_GET_UINT16_BE(p, 0) == cipher_suites[i] ) - { - ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( - cipher_suites[i] ); + cipher_suite = MBEDTLS_GET_UINT16_BE( p, 0 ); + ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( + cipher_suite ); + /* + * Check whether this ciphersuite is valid and offered. + */ + if( ( mbedtls_ssl_validate_ciphersuite( + ssl, ciphersuite_info, ssl->minor_ver, ssl->minor_ver ) != 0 ) || + !mbedtls_ssl_tls13_cipher_suite_is_offered( ssl, cipher_suite ) ) + continue; - if( ciphersuite_info == NULL ) - { - MBEDTLS_SSL_DEBUG_MSG( - 1, - ( "mbedtls_ssl_ciphersuite_from_id: should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); - } + ssl->session_negotiate->ciphersuite = cipher_suite; + ssl->handshake->ciphersuite_info = ciphersuite_info; + ciphersuite_exist = 1; - ssl->session_negotiate->ciphersuite = cipher_suites[i]; - ssl->handshake->ciphersuite_info = ciphersuite_info; - ciphersuite_exist = 1; + break; - break; - - } - - } } if( !ciphersuite_exist ) @@ -614,7 +610,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, * ECDHE/DHE key establishment methods. */ ret = ssl_tls13_parse_key_shares_ext( ssl, p, extension_data_end ); - if( ret == MBEDTLS_ERR_SSL_HRR_REQUIRED ) + if( ret == SSL_TLS1_3_PARSE_KEY_SHARES_EXT_NO_MATCH ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "HRR needed " ) ); ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; @@ -703,7 +699,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context* ssl ) { - int ret = 0; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; ret = mbedtls_ssl_tls13_key_schedule_stage_early( ssl ); if( ret != 0 ) @@ -725,7 +721,7 @@ static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context* ssl ) static int ssl_tls13_process_client_hello( mbedtls_ssl_context *ssl ) { - int ret = 0; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; unsigned char* buf = NULL; size_t buflen = 0; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse client hello" ) ); @@ -751,7 +747,7 @@ cleanup: */ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) { - int ret = 0; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; if( ssl->state == MBEDTLS_SSL_HANDSHAKE_OVER || ssl->handshake == NULL ) return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); @@ -766,10 +762,9 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) case MBEDTLS_SSL_HELLO_REQUEST: mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_CLIENT_HELLO ); + ret = 0; break; - /* ----- READ CLIENT HELLO ----*/ - case MBEDTLS_SSL_CLIENT_HELLO: ret = ssl_tls13_process_client_hello( ssl ); From de33391fa009e5922d1157bdaac176b6f5eb3692 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 20 Apr 2022 08:49:42 +0000 Subject: [PATCH 20/27] Rebase and solve conflicts Change-Id: I7f838ff5b607fe5e6b68d74d0edc1def8fc9a744 Signed-off-by: XiaokangQian --- library/ssl_tls13_server.c | 24 ++++++++++-------------- tests/suites/test_suite_ssl.data | 2 +- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index f2e7862e10..2e0dd39c9c 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -52,7 +52,7 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, const unsigned char *p = buf; size_t versions_len; const unsigned char *versions_end; - int major_ver, minor_ver; + int tls_version; int tls13_supported = 0; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); @@ -64,12 +64,11 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, while( p < versions_end ) { MBEDTLS_SSL_CHK_BUF_READ_PTR( p, versions_end, 2 ); - mbedtls_ssl_read_version( &major_ver, &minor_ver, ssl->conf->transport, p ); + tls_version = mbedtls_ssl_read_version( p, ssl->conf->transport ); p += 2; /* In this implementation we only support TLS 1.3 and DTLS 1.3. */ - if( major_ver == MBEDTLS_SSL_MAJOR_VERSION_3 && - minor_ver == MBEDTLS_SSL_MINOR_VERSION_4 ) + if( tls_version == MBEDTLS_SSL_VERSION_TLS1_3 ) { tls13_supported = 1; break; @@ -85,11 +84,9 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_BAD_PROTOCOL_VERSION ); } - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Negotiated version. Supported is [%d:%d]", - major_ver, minor_ver ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, ( "Negotiated version. Supported is [%04x]", + tls_version ) ); - ssl->major_ver = major_ver; - ssl->minor_ver = minor_ver; return( 0 ); } @@ -425,8 +422,8 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, * with ProtocolVersion defined as: * uint16 ProtocolVersion; */ - if( !( p[0] == MBEDTLS_SSL_MAJOR_VERSION_3 && - p[1] == MBEDTLS_SSL_MINOR_VERSION_3 ) ) + if( mbedtls_ssl_read_version( p, ssl->conf->transport ) != + MBEDTLS_SSL_VERSION_TLS1_2 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "Unsupported version of TLS." ) ); MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_PROTOCOL_VERSION, @@ -438,8 +435,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, /* * Only support TLS 1.3 currently, temporarily set the version. */ - ssl->major_ver = MBEDTLS_SSL_MAJOR_VERSION_3; - ssl->minor_ver = MBEDTLS_SSL_MINOR_VERSION_4; + ssl->tls_version = MBEDTLS_SSL_VERSION_TLS1_3; /* --- * Random random; @@ -511,7 +507,8 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, * Check whether this ciphersuite is valid and offered. */ if( ( mbedtls_ssl_validate_ciphersuite( - ssl, ciphersuite_info, ssl->minor_ver, ssl->minor_ver ) != 0 ) || + ssl, ciphersuite_info, ssl->tls_version, + ssl->tls_version ) != 0 ) || !mbedtls_ssl_tls13_cipher_suite_is_offered( ssl, cipher_suite ) ) continue; @@ -726,7 +723,6 @@ static int ssl_tls13_process_client_hello( mbedtls_ssl_context *ssl ) size_t buflen = 0; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse client hello" ) ); - ssl->major_ver = MBEDTLS_SSL_MAJOR_VERSION_3; MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_fetch_handshake_msg( ssl, MBEDTLS_SSL_HS_CLIENT_HELLO, &buf, &buflen ) ); diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 94f79323b2..644c5ab387 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3279,7 +3279,7 @@ conf_version:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_TRANSPORT_DATAGRAM:MBEDTLS_SSL_VE Version config: supported server TLS 1.3 only depends_on:MBEDTLS_SSL_PROTO_TLS1_3 -conf_version:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_TRANSPORT_STREAM:3:4:3:4:0 +conf_version:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_TRANSPORT_STREAM:MBEDTLS_SSL_VERSION_TLS1_3:MBEDTLS_SSL_VERSION_TLS1_3:0 Version config: unsupported server DTLS 1.3 only depends_on:MBEDTLS_SSL_PROTO_TLS1_3 From 318dc763a6c165413c0b0a971015df402f2b8a1d Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 20 Apr 2022 09:43:51 +0000 Subject: [PATCH 21/27] Fix test failure issue and update code styles Change-Id: I0b08da1b083abdb19dc383e6f4b210f66659c109 Signed-off-by: XiaokangQian --- library/ssl_misc.h | 4 ++-- library/ssl_tls13_server.c | 32 ++++++++++++++------------------ programs/ssl/ssl_server2.c | 12 ++++++------ tests/ssl-opt.sh | 6 ++---- 4 files changed, 24 insertions(+), 30 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index f079e687d9..025732e03d 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -582,12 +582,12 @@ struct mbedtls_ssl_handshake_params #endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ #if defined(MBEDTLS_SSL_CLI_C) - /*!< Number of Hello Retry Request messages received from the server. */ + /** Number of Hello Retry Request messages received from the server. */ int hello_retry_request_count; #endif /* MBEDTLS_SSL_CLI_C */ #if defined(MBEDTLS_SSL_SRV_C) - /*!< selected_group of key_share extension in HelloRetryRequest message. */ + /** selected_group of key_share extension in HelloRetryRequest message. */ uint16_t hrr_selected_group; #endif /* MBEDTLS_SSL_SRV_C */ diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 2e0dd39c9c..447bc0e3dc 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -125,9 +125,7 @@ static int ssl_tls13_parse_supported_groups_ext( named_group = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; - MBEDTLS_SSL_DEBUG_MSG( - 2, ( "got named group: %d", - named_group ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "got named group: %d", named_group ) ); if( ! mbedtls_ssl_named_group_is_offered( ssl, named_group ) || ! mbedtls_ssl_named_group_is_supported( named_group ) || @@ -233,13 +231,8 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, match_found = 1; MBEDTLS_SSL_DEBUG_MSG( 2, ( "ECDH curve: %s", curve_info->name ) ); - ret = psa_crypto_init(); - if( ret != PSA_SUCCESS ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "psa_crypto_init()", ret ); - return( ret ); - } - ret = mbedtls_ssl_tls13_read_public_ecdhe_share( ssl, p - 2, key_exchange_len + 2 ); + ret = mbedtls_ssl_tls13_read_public_ecdhe_share( + ssl, p - 2, key_exchange_len + 2 ); if( ret != 0 ) return( ret ); } @@ -385,8 +378,8 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; const unsigned char *p = buf; size_t legacy_session_id_len; - size_t cipher_suites_len; const unsigned char *cipher_suites_start; + size_t cipher_suites_len; size_t extensions_len; const unsigned char *extensions_end; @@ -494,13 +487,12 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, /* * Search for a matching ciphersuite */ - size_t ciphersuite_exist = 0; - uint16_t cipher_suite; + int ciphersuite_match = 0; ciphersuite_info = NULL; for ( size_t j = 0; j < cipher_suites_len; j += 2, p += 2 ) { - cipher_suite = MBEDTLS_GET_UINT16_BE( p, 0 ); + uint16_t cipher_suite = MBEDTLS_GET_UINT16_BE( p, 0 ); ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( cipher_suite ); /* @@ -514,14 +506,18 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, ssl->session_negotiate->ciphersuite = cipher_suite; ssl->handshake->ciphersuite_info = ciphersuite_info; - ciphersuite_exist = 1; + ciphersuite_match = 1; break; } - if( !ciphersuite_exist ) - return( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + if( !ciphersuite_match ) + { + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER, + MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + return ( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + } MBEDTLS_SSL_DEBUG_MSG( 2, ( "selected ciphersuite: %s", ciphersuite_info->name ) ); @@ -562,7 +558,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, size_t extension_data_len; const unsigned char *extension_data_end; - MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 4 ); + 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; diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 948d2e016e..b5b0248aa5 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -65,7 +65,7 @@ int main( void ) #include #endif -#if defined(MBEDTLS_USE_PSA_CRYPTO) +#if defined(MBEDTLS_USE_PSA_CRYPTO) || defined(MBEDTLS_SSL_PROTO_TLS1_3) #include "test/psa_crypto_helpers.h" #endif @@ -1421,7 +1421,7 @@ int main( int argc, char *argv[] ) int i; char *p, *q; const int *list; -#if defined(MBEDTLS_USE_PSA_CRYPTO) +#if defined(MBEDTLS_USE_PSA_CRYPTO) || defined(MBEDTLS_SSL_PROTO_TLS1_3) psa_status_t status; #endif unsigned char eap_tls_keymaterial[16]; @@ -1487,7 +1487,7 @@ int main( int argc, char *argv[] ) mbedtls_ssl_cookie_init( &cookie_ctx ); #endif -#if defined(MBEDTLS_USE_PSA_CRYPTO) +#if defined(MBEDTLS_USE_PSA_CRYPTO) || defined(MBEDTLS_SSL_PROTO_TLS1_3) status = psa_crypto_init(); if( status != PSA_SUCCESS ) { @@ -4127,7 +4127,7 @@ exit: #endif /* MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED && MBEDTLS_USE_PSA_CRYPTO */ -#if defined(MBEDTLS_USE_PSA_CRYPTO) +#if defined(MBEDTLS_USE_PSA_CRYPTO) || defined(MBEDTLS_SSL_PROTO_TLS1_3) const char* message = mbedtls_test_helper_is_psa_leaking(); if( message ) { @@ -4139,8 +4139,8 @@ exit: /* For builds with MBEDTLS_TEST_USE_PSA_CRYPTO_RNG psa crypto * resources are freed by rng_free(). */ -#if defined(MBEDTLS_USE_PSA_CRYPTO) && \ - !defined(MBEDTLS_TEST_USE_PSA_CRYPTO_RNG) +#if ( defined(MBEDTLS_USE_PSA_CRYPTO) || defined(MBEDTLS_SSL_PROTO_TLS1_3) ) \ + && !defined(MBEDTLS_TEST_USE_PSA_CRYPTO_RNG) mbedtls_psa_crypto_free( ); #endif diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index d870076884..b0f5f4bead 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -10209,12 +10209,11 @@ run_test "TLS 1.3: HelloRetryRequest check, ciphersuite TLS_AES_256_GCM_SHA38 -c "HTTP/1.0 200 OK" requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 -requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE requires_config_enabled MBEDTLS_DEBUG_C requires_config_enabled MBEDTLS_SSL_CLI_C requires_config_disabled MBEDTLS_USE_PSA_CRYPTO requires_openssl_tls1_3 -run_test "TLS 1.3: Server side check, ciphersuite TLS_AES_256_GCM_SHA384 - openssl" \ +run_test "TLS 1.3: Server side check - openssl" \ "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ "$O_NEXT_CLI -msg -tls1_3" \ 1 \ @@ -10227,11 +10226,10 @@ run_test "TLS 1.3: Server side check, ciphersuite TLS_AES_256_GCM_SHA384 - op requires_gnutls_tls1_3 requires_gnutls_next_no_ticket requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 -requires_config_enabled MBEDTLS_SSL_TLS1_3_COMPATIBILITY_MODE requires_config_enabled MBEDTLS_DEBUG_C requires_config_enabled MBEDTLS_SSL_CLI_C requires_config_disabled MBEDTLS_USE_PSA_CRYPTO -run_test "TLS 1.3: Server side check, ciphersuite TLS_AES_128_GCM_SHA256 - gnutls" \ +run_test "TLS 1.3: Server side check - gnutls" \ "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ "$G_NEXT_CLI localhost -d 4 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:%NO_TICKETS:%DISABLE_TLS13_COMPAT_MODE -V" \ 1 \ From 75d40ef8cb241e312df1a4ee455ded6ed5105e2b Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Wed, 20 Apr 2022 11:05:24 +0000 Subject: [PATCH 22/27] Refine code base on review Remove useless hrr code Share validate_cipher_suit between client and server Fix test failure when tls13 only in server side Change-Id: I5d6a7932bd8448ebf542bc86cdcab8862bc28e9b Signed-off-by: XiaokangQian --- include/mbedtls/ssl.h | 16 +++++++++++++++ library/ssl_client.c | 39 ----------------------------------- library/ssl_client.h | 16 --------------- library/ssl_misc.h | 2 ++ library/ssl_tls.c | 42 ++++++++++++++++++++++++++++++++++++++ library/ssl_tls13_server.c | 11 +++------- 6 files changed, 63 insertions(+), 63 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 16939565fe..15e11db15e 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -4936,6 +4936,22 @@ int mbedtls_ssl_tls_prf( const mbedtls_tls_prf_types prf, const unsigned char *random, size_t rlen, unsigned char *dstbuf, size_t dlen ); +/** + * \brief Validate cipher suite against config in SSL context. + * + * \param ssl SSL context + * \param suite_info Cipher suite to validate + * \param min_tls_version Minimal TLS version to accept a cipher suite + * \param max_tls_version Maximal TLS version to accept a cipher suite + * + * \return 0 if valid, negative value otherwise. + */ +int mbedtls_ssl_validate_ciphersuite( + const mbedtls_ssl_context *ssl, + const mbedtls_ssl_ciphersuite_t *suite_info, + mbedtls_ssl_protocol_version min_tls_version, + mbedtls_ssl_protocol_version max_tls_version ); + #ifdef __cplusplus } #endif diff --git a/library/ssl_client.c b/library/ssl_client.c index f5b8be485c..79c5d9fbeb 100644 --- a/library/ssl_client.c +++ b/library/ssl_client.c @@ -413,45 +413,6 @@ static int ssl_write_sig_alg_ext( mbedtls_ssl_context *ssl, unsigned char *buf, } #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ -int mbedtls_ssl_validate_ciphersuite( - const mbedtls_ssl_context *ssl, - const mbedtls_ssl_ciphersuite_t *suite_info, - mbedtls_ssl_protocol_version min_tls_version, - mbedtls_ssl_protocol_version max_tls_version ) -{ - (void) ssl; - - if( suite_info == NULL ) - return( -1 ); - - if( ( suite_info->min_tls_version > max_tls_version ) || - ( suite_info->max_tls_version < min_tls_version ) ) - { - return( -1 ); - } - -#if defined(MBEDTLS_SSL_PROTO_TLS1_2) -#if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) - if( suite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECJPAKE && - mbedtls_ecjpake_check( &ssl->handshake->ecjpake_ctx ) != 0 ) - { - return( -1 ); - } -#endif - - /* Don't suggest PSK-based ciphersuite if no PSK is available. */ -#if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) - if( mbedtls_ssl_ciphersuite_uses_psk( suite_info ) && - mbedtls_ssl_conf_has_static_psk( ssl->conf ) == 0 ) - { - return( -1 ); - } -#endif /* MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */ -#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ - - return( 0 ); -} - static int ssl_write_client_hello_cipher_suites( mbedtls_ssl_context *ssl, unsigned char *buf, diff --git a/library/ssl_client.h b/library/ssl_client.h index 67fc5583f7..8e0c21634b 100644 --- a/library/ssl_client.h +++ b/library/ssl_client.h @@ -28,22 +28,6 @@ #include -/** - * \brief Validate cipher suite against config in SSL context. - * - * \param ssl SSL context - * \param suite_info Cipher suite to validate - * \param min_tls_version Minimal TLS version to accept a cipher suite - * \param max_tls_version Maximal TLS version to accept a cipher suite - * - * \return 0 if valid, negative value otherwise. - */ -int mbedtls_ssl_validate_ciphersuite( - const mbedtls_ssl_context *ssl, - const mbedtls_ssl_ciphersuite_t *suite_info, - mbedtls_ssl_protocol_version min_tls_version, - mbedtls_ssl_protocol_version max_tls_version ); - int mbedtls_ssl_write_client_hello( mbedtls_ssl_context *ssl ); #endif /* MBEDTLS_SSL_CLIENT_H */ diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 025732e03d..d2760826d1 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -589,6 +589,8 @@ struct mbedtls_ssl_handshake_params #if defined(MBEDTLS_SSL_SRV_C) /** selected_group of key_share extension in HelloRetryRequest message. */ uint16_t hrr_selected_group; + /** selected_group of key_share extension in ClientHello message. */ + uint16_t selected_group; #endif /* MBEDTLS_SSL_SRV_C */ #if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 11140569d6..d8d79d7998 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -4216,6 +4216,9 @@ int mbedtls_ssl_config_defaults( mbedtls_ssl_config *conf, #if defined(MBEDTLS_SSL_PROTO_TLS1_2) conf->min_tls_version = MBEDTLS_SSL_VERSION_TLS1_2; conf->max_tls_version = MBEDTLS_SSL_VERSION_TLS1_2; +#elif defined(MBEDTLS_SSL_PROTO_TLS1_3) + conf->min_tls_version = MBEDTLS_SSL_VERSION_TLS1_3; + conf->max_tls_version = MBEDTLS_SSL_VERSION_TLS1_3; #else return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); #endif @@ -7771,4 +7774,43 @@ static int ssl_session_load_tls12( mbedtls_ssl_session *session, } #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ +int mbedtls_ssl_validate_ciphersuite( + const mbedtls_ssl_context *ssl, + const mbedtls_ssl_ciphersuite_t *suite_info, + mbedtls_ssl_protocol_version min_tls_version, + mbedtls_ssl_protocol_version max_tls_version ) +{ + (void) ssl; + + if( suite_info == NULL ) + return( -1 ); + + if( ( suite_info->min_tls_version > max_tls_version ) || + ( suite_info->max_tls_version < min_tls_version ) ) + { + return( -1 ); + } + +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) +#if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) + if( suite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECJPAKE && + mbedtls_ecjpake_check( &ssl->handshake->ecjpake_ctx ) != 0 ) + { + return( -1 ); + } +#endif + + /* Don't suggest PSK-based ciphersuite if no PSK is available. */ +#if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) + if( mbedtls_ssl_ciphersuite_uses_psk( suite_info ) && + mbedtls_ssl_conf_has_static_psk( ssl->conf ) == 0 ) + { + return( -1 ); + } +#endif /* MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */ +#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ + + return( 0 ); +} + #endif /* MBEDTLS_SSL_TLS_C */ diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 447bc0e3dc..a8e523a774 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -24,7 +24,6 @@ #include "mbedtls/debug.h" #include "ssl_misc.h" -#include "ssl_client.h" #include "ssl_tls13_keys.h" #include "ssl_debug_helpers.h" #include @@ -116,7 +115,7 @@ static int ssl_tls13_parse_supported_groups_ext( p += 2; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, named_group_list_len ); named_group_list_end = p + named_group_list_len; - ssl->handshake->hrr_selected_group = 0; + ssl->handshake->selected_group = 0; while( p < named_group_list_end ) { @@ -129,7 +128,7 @@ static int ssl_tls13_parse_supported_groups_ext( if( ! mbedtls_ssl_named_group_is_offered( ssl, named_group ) || ! mbedtls_ssl_named_group_is_supported( named_group ) || - ssl->handshake->hrr_selected_group != 0 ) + ssl->handshake->selected_group != 0 ) { continue; } @@ -137,7 +136,7 @@ static int ssl_tls13_parse_supported_groups_ext( MBEDTLS_SSL_DEBUG_MSG( 2, ( "add named group (%04x) into received list.", named_group ) ); - ssl->handshake->hrr_selected_group = named_group; + ssl->handshake->selected_group = named_group; } return( 0 ); @@ -384,7 +383,6 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, const unsigned char *extensions_end; const mbedtls_ssl_ciphersuite_t* ciphersuite_info; - int hrr_required = 0; ssl->handshake->extensions_present = MBEDTLS_SSL_EXT_NONE; @@ -682,9 +680,6 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); } - if( hrr_required == 1 ) - return( SSL_CLIENT_HELLO_HRR_REQUIRED ); - return( 0 ); } From 0a1b54ed7328d20cd1d2856202e5f689f938004f Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Thu, 21 Apr 2022 03:01:38 +0000 Subject: [PATCH 23/27] Minor change the place of some functions Change-Id: I2626e68cf837d8ca4086cb35a8482cee315cde97 Signed-off-by: XiaokangQian --- include/mbedtls/ssl.h | 16 ---------------- library/ssl_misc.h | 33 +++++++++++++++++++++++++++++++-- library/ssl_tls13_generic.c | 16 ---------------- library/ssl_tls13_server.c | 12 ++++++------ 4 files changed, 37 insertions(+), 40 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 15e11db15e..16939565fe 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -4936,22 +4936,6 @@ int mbedtls_ssl_tls_prf( const mbedtls_tls_prf_types prf, const unsigned char *random, size_t rlen, unsigned char *dstbuf, size_t dlen ); -/** - * \brief Validate cipher suite against config in SSL context. - * - * \param ssl SSL context - * \param suite_info Cipher suite to validate - * \param min_tls_version Minimal TLS version to accept a cipher suite - * \param max_tls_version Maximal TLS version to accept a cipher suite - * - * \return 0 if valid, negative value otherwise. - */ -int mbedtls_ssl_validate_ciphersuite( - const mbedtls_ssl_context *ssl, - const mbedtls_ssl_ciphersuite_t *suite_info, - mbedtls_ssl_protocol_version min_tls_version, - mbedtls_ssl_protocol_version max_tls_version ); - #ifdef __cplusplus } #endif diff --git a/library/ssl_misc.h b/library/ssl_misc.h index d2760826d1..f83f5d0666 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -2220,7 +2220,36 @@ int mbedtls_ssl_tls13_read_public_ecdhe_share( mbedtls_ssl_context *ssl, #endif /* MBEDTLS_ECDH_C */ -int mbedtls_ssl_tls13_cipher_suite_is_offered( mbedtls_ssl_context *ssl, - int cipher_suite ); +static inline int mbedtls_ssl_tls13_cipher_suite_is_offered( + mbedtls_ssl_context *ssl, int cipher_suite ) +{ + const int *ciphersuite_list = ssl->conf->ciphersuite_list; + + /* Check whether we have offered this ciphersuite */ + for ( size_t i = 0; ciphersuite_list[i] != 0; i++ ) + { + if( ciphersuite_list[i] == cipher_suite ) + { + return( 1 ); + } + } + return( 0 ); +} + +/** + * \brief Validate cipher suite against config in SSL context. + * + * \param ssl SSL context + * \param suite_info Cipher suite to validate + * \param min_tls_version Minimal TLS version to accept a cipher suite + * \param max_tls_version Maximal TLS version to accept a cipher suite + * + * \return 0 if valid, negative value otherwise. + */ +int mbedtls_ssl_validate_ciphersuite( + const mbedtls_ssl_context *ssl, + const mbedtls_ssl_ciphersuite_t *suite_info, + mbedtls_ssl_protocol_version min_tls_version, + mbedtls_ssl_protocol_version max_tls_version ); #endif /* ssl_misc.h */ diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 1bcafe4927..4bee319dca 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1537,20 +1537,4 @@ int mbedtls_ssl_tls13_read_public_ecdhe_share( mbedtls_ssl_context *ssl, } #endif /* MBEDTLS_ECDH_C */ -int mbedtls_ssl_tls13_cipher_suite_is_offered( mbedtls_ssl_context *ssl, - int cipher_suite ) -{ - const int *ciphersuite_list = ssl->conf->ciphersuite_list; - - /* Check whether we have offered this ciphersuite */ - for ( size_t i = 0; ciphersuite_list[i] != 0; i++ ) - { - if( ciphersuite_list[i] == cipher_suite ) - { - return( 1 ); - } - } - return( 0 ); -} - #endif /* MBEDTLS_SSL_TLS_C && MBEDTLS_SSL_PROTO_TLS1_3 */ diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index a8e523a774..136d236217 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -51,7 +51,7 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, const unsigned char *p = buf; size_t versions_len; const unsigned char *versions_end; - int tls_version; + uint16_t tls_version; int tls13_supported = 0; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, 1 ); @@ -84,7 +84,7 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, } MBEDTLS_SSL_DEBUG_MSG( 1, ( "Negotiated version. Supported is [%04x]", - tls_version ) ); + (unsigned int)tls_version ) ); return( 0 ); } @@ -512,9 +512,9 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, if( !ciphersuite_match ) { - MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER, - MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); - return ( MBEDTLS_ERR_SSL_ILLEGAL_PARAMETER ); + MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE, + MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + return ( MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); } MBEDTLS_SSL_DEBUG_MSG( 2, ( "selected ciphersuite: %s", @@ -525,7 +525,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, * opaque legacy_compression_methods<1..2^8-1>; * ... */ - if( p[0] != 1 || p[1] != 0 ) + if( p[0] != 1 || p[1] != MBEDTLS_SSL_COMPRESS_NULL ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad legacy compression method" ) ); MBEDTLS_SSL_PEND_FATAL_ALERT( MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER, From 060d8675983645c1c207736c6b282645d29012a3 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Thu, 21 Apr 2022 09:24:56 +0000 Subject: [PATCH 24/27] Update parse_key_share in server side and version config Change-Id: Ic91c061027d0ee4dca2055df21809cbb4388f3ef Signed-off-by: XiaokangQian --- library/ssl_tls.c | 45 ++++++++++++++++++++------------------ library/ssl_tls13_server.c | 36 ++++++++++++++---------------- 2 files changed, 40 insertions(+), 41 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index d8d79d7998..26c009a9e0 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -4210,34 +4210,37 @@ int mbedtls_ssl_config_defaults( mbedtls_ssl_config *conf, conf->tls13_kex_modes = MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_ALL; #endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ - if( ( endpoint == MBEDTLS_SSL_IS_SERVER ) || - ( transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) ) - { #if defined(MBEDTLS_SSL_PROTO_TLS1_2) - conf->min_tls_version = MBEDTLS_SSL_VERSION_TLS1_2; - conf->max_tls_version = MBEDTLS_SSL_VERSION_TLS1_2; + conf->min_tls_version = MBEDTLS_SSL_VERSION_TLS1_2; #elif defined(MBEDTLS_SSL_PROTO_TLS1_3) - conf->min_tls_version = MBEDTLS_SSL_VERSION_TLS1_3; - conf->max_tls_version = MBEDTLS_SSL_VERSION_TLS1_3; -#else - return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + conf->min_tls_version = MBEDTLS_SSL_VERSION_TLS1_3; #endif +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) && defined(MBEDTLS_SSL_PROTO_TLS1_3) + if( transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM || + endpoint == MBEDTLS_SSL_IS_SERVER ) + { + /* DTLS 1.3 not supported yet + * server side hybrid mode not support yet + */ + conf->max_tls_version = MBEDTLS_SSL_VERSION_TLS1_2; } else { -#if defined(MBEDTLS_SSL_PROTO_TLS1_2) && defined(MBEDTLS_SSL_PROTO_TLS1_3) - conf->min_tls_version = MBEDTLS_SSL_VERSION_TLS1_2; conf->max_tls_version = MBEDTLS_SSL_VERSION_TLS1_3; -#elif defined(MBEDTLS_SSL_PROTO_TLS1_3) - conf->min_tls_version = MBEDTLS_SSL_VERSION_TLS1_3; - conf->max_tls_version = MBEDTLS_SSL_VERSION_TLS1_3; -#elif defined(MBEDTLS_SSL_PROTO_TLS1_2) - conf->min_tls_version = MBEDTLS_SSL_VERSION_TLS1_2; - conf->max_tls_version = MBEDTLS_SSL_VERSION_TLS1_2; -#else - return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); -#endif } +#elif defined(MBEDTLS_SSL_PROTO_TLS1_2) + conf->max_tls_version = MBEDTLS_SSL_VERSION_TLS1_2; +#elif defined(MBEDTLS_SSL_PROTO_TLS1_3) + if( transport != MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + { + conf->max_tls_version = MBEDTLS_SSL_VERSION_TLS1_3; + } + else + { + /* DTLS 1.3 not supported yet */ + return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + } +#endif /* * Preset-specific defaults @@ -7791,7 +7794,7 @@ int mbedtls_ssl_validate_ciphersuite( return( -1 ); } -#if defined(MBEDTLS_SSL_PROTO_TLS1_2) +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) && defined(MBEDTLS_SSL_CLI_C) #if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) if( suite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECJPAKE && mbedtls_ecjpake_check( &ssl->handshake->ecjpake_ctx ) != 0 ) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 136d236217..1a96eaf747 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -215,19 +215,19 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, if( match_found == 1 ) continue; + if( ! mbedtls_ssl_named_group_is_offered( ssl, group ) || + ! mbedtls_ssl_named_group_is_supported( group ) ) + { + continue; + } + const mbedtls_ecp_curve_info *curve_info = + mbedtls_ecp_curve_info_from_tls_id( group ); + /* * For now, we only support ECDHE groups. */ if( mbedtls_ssl_tls13_named_group_is_ecdhe( group ) ) { - const mbedtls_ecp_curve_info *curve_info = - mbedtls_ecp_curve_info_from_tls_id( group ); - if( curve_info == NULL ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Invalid TLS curve group id" ) ); - continue; - } - match_found = 1; MBEDTLS_SSL_DEBUG_MSG( 2, ( "ECDH curve: %s", curve_info->name ) ); ret = mbedtls_ssl_tls13_read_public_ecdhe_share( @@ -377,8 +377,9 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; const unsigned char *p = buf; size_t legacy_session_id_len; - const unsigned char *cipher_suites_start; + const unsigned char *cipher_suites; size_t cipher_suites_len; + const unsigned char *cipher_suites_end; size_t extensions_len; const unsigned char *extensions_end; @@ -479,7 +480,8 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, * with CipherSuite defined as: * uint8 CipherSuite[2]; */ - cipher_suites_start = p; + cipher_suites = p; + cipher_suites_end = p + cipher_suites_len; MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, ciphersuitelist", p, cipher_suites_len ); /* @@ -487,8 +489,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, */ int ciphersuite_match = 0; ciphersuite_info = NULL; - for ( size_t j = 0; j < cipher_suites_len; - j += 2, p += 2 ) + for ( ; p < cipher_suites_end; p += 2 ) { uint16_t cipher_suite = MBEDTLS_GET_UINT16_BE( p, 0 ); ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( @@ -520,7 +521,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 2, ( "selected ciphersuite: %s", ciphersuite_info->name ) ); - p = cipher_suites_start + cipher_suites_len; + p = cipher_suites + cipher_suites_len; /* ... * opaque legacy_compression_methods<1..2^8-1>; * ... @@ -730,7 +731,7 @@ cleanup: } /* - * TLS and DTLS 1.3 State Maschine -- server side + * TLS 1.3 State Maschine -- server side */ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) { @@ -760,14 +761,9 @@ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) break; - case MBEDTLS_SSL_SERVER_HELLO: - MBEDTLS_SSL_DEBUG_MSG( 1, ( "SSL - The requested feature is not available" ) ); - return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); - - break; default: MBEDTLS_SSL_DEBUG_MSG( 1, ( "invalid state %d", ssl->state ) ); - return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); } return( ret ); From 4e8cd7b903aecb431bd0792282ef056c6f5fa1fa Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Thu, 21 Apr 2022 09:48:09 +0000 Subject: [PATCH 25/27] Remove useless selected_group Change-Id: I5fb76b5bf4b22d0231c17314783781f9e7c309a3 Signed-off-by: XiaokangQian --- library/ssl_misc.h | 2 -- library/ssl_tls13_server.c | 14 ++++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index f83f5d0666..78b7154d90 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -589,8 +589,6 @@ struct mbedtls_ssl_handshake_params #if defined(MBEDTLS_SSL_SRV_C) /** selected_group of key_share extension in HelloRetryRequest message. */ uint16_t hrr_selected_group; - /** selected_group of key_share extension in ClientHello message. */ - uint16_t selected_group; #endif /* MBEDTLS_SSL_SRV_C */ #if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 1a96eaf747..5f9b5d0165 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -115,7 +115,7 @@ static int ssl_tls13_parse_supported_groups_ext( p += 2; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, named_group_list_len ); named_group_list_end = p + named_group_list_len; - ssl->handshake->selected_group = 0; + ssl->handshake->hrr_selected_group = 0; while( p < named_group_list_end ) { @@ -128,7 +128,7 @@ static int ssl_tls13_parse_supported_groups_ext( if( ! mbedtls_ssl_named_group_is_offered( ssl, named_group ) || ! mbedtls_ssl_named_group_is_supported( named_group ) || - ssl->handshake->selected_group != 0 ) + ssl->handshake->hrr_selected_group != 0 ) { continue; } @@ -136,7 +136,7 @@ static int ssl_tls13_parse_supported_groups_ext( MBEDTLS_SSL_DEBUG_MSG( 2, ( "add named group (%04x) into received list.", named_group ) ); - ssl->handshake->selected_group = named_group; + ssl->handshake->hrr_selected_group = named_group; } return( 0 ); @@ -220,20 +220,22 @@ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, { continue; } - const mbedtls_ecp_curve_info *curve_info = - mbedtls_ecp_curve_info_from_tls_id( group ); /* * For now, we only support ECDHE groups. */ if( mbedtls_ssl_tls13_named_group_is_ecdhe( group ) ) { - match_found = 1; + const mbedtls_ecp_curve_info *curve_info = + mbedtls_ecp_curve_info_from_tls_id( group ); + ((void) curve_info); MBEDTLS_SSL_DEBUG_MSG( 2, ( "ECDH curve: %s", curve_info->name ) ); ret = mbedtls_ssl_tls13_read_public_ecdhe_share( ssl, p - 2, key_exchange_len + 2 ); if( ret != 0 ) return( ret ); + + match_found = 1; } else { From 4d3a60475cd76aa493e7d087f54b4c655695c449 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Thu, 21 Apr 2022 13:46:17 +0000 Subject: [PATCH 26/27] Change default config version to development style Change-Id: I9c1088f235524211e727d03b96de8d82e60bd426 Signed-off-by: XiaokangQian --- library/ssl_tls.c | 56 ++++++++++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 26c009a9e0..57f4e46e65 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -4210,37 +4210,39 @@ int mbedtls_ssl_config_defaults( mbedtls_ssl_config *conf, conf->tls13_kex_modes = MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_ALL; #endif /* MBEDTLS_SSL_PROTO_TLS1_3 */ + if( transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + { #if defined(MBEDTLS_SSL_PROTO_TLS1_2) - conf->min_tls_version = MBEDTLS_SSL_VERSION_TLS1_2; -#elif defined(MBEDTLS_SSL_PROTO_TLS1_3) - conf->min_tls_version = MBEDTLS_SSL_VERSION_TLS1_3; -#endif -#if defined(MBEDTLS_SSL_PROTO_TLS1_2) && defined(MBEDTLS_SSL_PROTO_TLS1_3) - if( transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM || - endpoint == MBEDTLS_SSL_IS_SERVER ) - { - /* DTLS 1.3 not supported yet - * server side hybrid mode not support yet - */ + conf->min_tls_version = MBEDTLS_SSL_VERSION_TLS1_2; conf->max_tls_version = MBEDTLS_SSL_VERSION_TLS1_2; - } - else - { - conf->max_tls_version = MBEDTLS_SSL_VERSION_TLS1_3; - } -#elif defined(MBEDTLS_SSL_PROTO_TLS1_2) - conf->max_tls_version = MBEDTLS_SSL_VERSION_TLS1_2; -#elif defined(MBEDTLS_SSL_PROTO_TLS1_3) - if( transport != MBEDTLS_SSL_TRANSPORT_DATAGRAM ) - { - conf->max_tls_version = MBEDTLS_SSL_VERSION_TLS1_3; - } - else - { - /* DTLS 1.3 not supported yet */ +#else return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); - } #endif + } + else + { +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) && defined(MBEDTLS_SSL_PROTO_TLS1_3) + if( endpoint == MBEDTLS_SSL_IS_CLIENT ) + { + conf->min_tls_version = MBEDTLS_SSL_VERSION_TLS1_2; + conf->max_tls_version = MBEDTLS_SSL_VERSION_TLS1_3; + } + else + /* Hybrid TLS 1.2 / 1.3 is not supported on server side yet */ + { + conf->min_tls_version = MBEDTLS_SSL_VERSION_TLS1_2; + conf->max_tls_version = MBEDTLS_SSL_VERSION_TLS1_2; + } +#elif defined(MBEDTLS_SSL_PROTO_TLS1_3) + conf->min_tls_version = MBEDTLS_SSL_VERSION_TLS1_3; + conf->max_tls_version = MBEDTLS_SSL_VERSION_TLS1_3; +#elif defined(MBEDTLS_SSL_PROTO_TLS1_2) + conf->min_tls_version = MBEDTLS_SSL_VERSION_TLS1_2; + conf->max_tls_version = MBEDTLS_SSL_VERSION_TLS1_2; +#else + return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); +#endif + } /* * Preset-specific defaults From e8ff350698a40bbdbee876e126d8d54d46478ef2 Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Fri, 22 Apr 2022 02:34:40 +0000 Subject: [PATCH 27/27] Update code to align with tls13 coding standard Change-Id: I3c98b7d0db63aecc712a67f4e8da2cb9945c8f17 Signed-off-by: XiaokangQian --- library/ssl_tls13_server.c | 55 +++++++++++++++----------------- tests/ssl-opt.sh | 6 ++-- tests/suites/test_suite_ssl.data | 2 +- 3 files changed, 28 insertions(+), 35 deletions(-) diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 5f9b5d0165..8d1b1d81e5 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -90,8 +90,7 @@ static int ssl_tls13_parse_supported_versions_ext( mbedtls_ssl_context *ssl, } #if defined(MBEDTLS_ECDH_C) -/* This function parses the TLS 1.3 supported_groups extension and - * stores the received groups in ssl->handshake->curves. +/* * * From RFC 8446: * enum { @@ -149,15 +148,14 @@ static int ssl_tls13_parse_supported_groups_ext( #if defined(MBEDTLS_ECDH_C) /* * ssl_tls13_parse_key_shares_ext() verifies whether the information in the - * extension is correct and stores the provided key shares. Whether this is an - * acceptable key share depends on the selected ciphersuite. + * extension is correct and stores the first acceptable key share and its associated group. * * Possible return values are: * - 0: Successful processing of the client provided key share extension. * - SSL_TLS1_3_PARSE_KEY_SHARES_EXT_NO_MATCH: The key shares provided by the client * does not match a group supported by the server. A HelloRetryRequest will * be needed. - * - Another negative return value for fatal errors. + * - A negative value for fatal errors. */ static int ssl_tls13_parse_key_shares_ext( mbedtls_ssl_context *ssl, @@ -355,18 +353,18 @@ static int ssl_tls13_check_ephemeral_key_exchange( mbedtls_ssl_context *ssl ) /* * Structure of this message: * - * uint16 ProtocolVersion; - * opaque Random[32]; - * uint8 CipherSuite[2]; // Cryptographic suite selector + * uint16 ProtocolVersion; + * opaque Random[32]; + * uint8 CipherSuite[2]; // Cryptographic suite selector * - * struct { - * ProtocolVersion legacy_version = 0x0303; // TLS v1.2 - * Random random; - * opaque legacy_session_id<0..32>; - * CipherSuite cipher_suites<2..2^16-2>; - * opaque legacy_compression_methods<1..2^8-1>; - * Extension extensions<8..2^16-1>; - * } ClientHello; + * struct { + * ProtocolVersion legacy_version = 0x0303; // TLS v1.2 + * Random random; + * opaque legacy_session_id<0..32>; + * CipherSuite cipher_suites<2..2^16-2>; + * opaque legacy_compression_methods<1..2^8-1>; + * Extension extensions<8..2^16-1>; + * } ClientHello; */ #define SSL_CLIENT_HELLO_OK 0 @@ -392,7 +390,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, /* * ClientHello layout: * 0 . 1 protocol version - * 2 . 33 random bytes ( starting with 4 bytes of Unix time ) + * 2 . 33 random bytes * 34 . 34 session id length ( 1 byte ) * 35 . 34+x session id * .. . .. ciphersuite list length ( 2 bytes ) @@ -458,7 +456,7 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, ssl->session_negotiate->id_len = legacy_session_id_len; MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, session id", - buf, legacy_session_id_len ); + p, legacy_session_id_len ); /* * Check we have enough data for the legacy session identifier * and the ciphersuite list length. @@ -490,15 +488,15 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, * Search for a matching ciphersuite */ int ciphersuite_match = 0; - ciphersuite_info = NULL; for ( ; p < cipher_suites_end; p += 2 ) { - uint16_t cipher_suite = MBEDTLS_GET_UINT16_BE( p, 0 ); - ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( - cipher_suite ); + uint16_t cipher_suite; + MBEDTLS_SSL_CHK_BUF_READ_PTR( p, cipher_suites_end, 2 ); + cipher_suite = MBEDTLS_GET_UINT16_BE( p, 0 ); + ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( cipher_suite ); /* - * Check whether this ciphersuite is valid and offered. - */ + * Check whether this ciphersuite is valid and offered. + */ if( ( mbedtls_ssl_validate_ciphersuite( ssl, ciphersuite_info, ssl->tls_version, ssl->tls_version ) != 0 ) || @@ -548,8 +546,8 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, */ extensions_len = MBEDTLS_GET_UINT16_BE( p, 0 ); p += 2; - extensions_end = p + extensions_len; MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, extensions_len ); + extensions_end = p + extensions_len; MBEDTLS_SSL_DEBUG_BUF( 3, "client hello extensions", p, extensions_len ); @@ -686,8 +684,6 @@ static int ssl_tls13_parse_client_hello( mbedtls_ssl_context *ssl, return( 0 ); } -/* Update the handshake state machine */ - static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context* ssl ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -700,7 +696,6 @@ static int ssl_tls13_postprocess_client_hello( mbedtls_ssl_context* ssl ) return( ret ); } - mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_HELLO ); return( 0 ); } @@ -723,8 +718,8 @@ static int ssl_tls13_process_client_hello( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_PROC_CHK_NEG( ssl_tls13_parse_client_hello( ssl, buf, buf + buflen ) ); - MBEDTLS_SSL_DEBUG_MSG( 1, ( "postprocess" ) ); MBEDTLS_SSL_PROC_CHK( ssl_tls13_postprocess_client_hello( ssl ) ); + mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_SERVER_HELLO ); cleanup: @@ -733,7 +728,7 @@ cleanup: } /* - * TLS 1.3 State Maschine -- server side + * TLS 1.3 State Machine -- server side */ int mbedtls_ssl_tls13_handshake_server_step( mbedtls_ssl_context *ssl ) { diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index b0f5f4bead..b1a9d475f7 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -10210,8 +10210,7 @@ run_test "TLS 1.3: HelloRetryRequest check, ciphersuite TLS_AES_256_GCM_SHA38 requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_3 requires_config_enabled MBEDTLS_DEBUG_C -requires_config_enabled MBEDTLS_SSL_CLI_C -requires_config_disabled MBEDTLS_USE_PSA_CRYPTO +requires_config_enabled MBEDTLS_SSL_SRV_C requires_openssl_tls1_3 run_test "TLS 1.3: Server side check - openssl" \ "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ @@ -10227,8 +10226,7 @@ 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_CLI_C -requires_config_disabled MBEDTLS_USE_PSA_CRYPTO +requires_config_enabled MBEDTLS_SSL_SRV_C run_test "TLS 1.3: Server side check - gnutls" \ "$P_SRV debug_level=4 crt_file=data_files/server5.crt key_file=data_files/server5.key force_version=tls13 tickets=0" \ "$G_NEXT_CLI localhost -d 4 --priority=NORMAL:-VERS-ALL:+VERS-TLS1.3:%NO_TICKETS:%DISABLE_TLS13_COMPAT_MODE -V" \ diff --git a/tests/suites/test_suite_ssl.data b/tests/suites/test_suite_ssl.data index 644c5ab387..848a497cb3 100644 --- a/tests/suites/test_suite_ssl.data +++ b/tests/suites/test_suite_ssl.data @@ -3277,7 +3277,7 @@ Version config: unsupported client DTLS 1.3 only depends_on:MBEDTLS_SSL_PROTO_TLS1_3 conf_version:MBEDTLS_SSL_IS_CLIENT:MBEDTLS_SSL_TRANSPORT_DATAGRAM:MBEDTLS_SSL_VERSION_TLS1_3:MBEDTLS_SSL_VERSION_TLS1_3:MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE -Version config: supported server TLS 1.3 only +Version config: valid server TLS 1.3 only depends_on:MBEDTLS_SSL_PROTO_TLS1_3 conf_version:MBEDTLS_SSL_IS_SERVER:MBEDTLS_SSL_TRANSPORT_STREAM:MBEDTLS_SSL_VERSION_TLS1_3:MBEDTLS_SSL_VERSION_TLS1_3:0