From 8f397268d31288ded3473db8b869b10df2e395dc Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Sat, 17 Nov 2018 21:18:01 +0000 Subject: [PATCH 01/10] Introduce macros for constants in SSL ticket implementation Signed-off-by: Ronald Cron --- library/ssl_ticket.c | 56 ++++++++++++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/library/ssl_ticket.c b/library/ssl_ticket.c index 8492c19a8..25eab49d2 100644 --- a/library/ssl_ticket.c +++ b/library/ssl_ticket.c @@ -54,6 +54,19 @@ void mbedtls_ssl_ticket_init( mbedtls_ssl_ticket_context *ctx ) #define MAX_KEY_BYTES 32 /* 256 bits */ +#define TICKET_KEY_NAME_BYTES 4 +#define TICKET_IV_BYTES 12 +#define TICKET_CRYPT_LEN_BYTES 2 +#define TICKET_AUTH_TAG_BYTES 16 + +#define TICKET_MIN_LEN ( TICKET_KEY_NAME_BYTES + \ + TICKET_IV_BYTES + \ + TICKET_CRYPT_LEN_BYTES + \ + TICKET_AUTH_TAG_BYTES ) +#define TICKET_ADD_DATA_LEN ( TICKET_KEY_NAME_BYTES + \ + TICKET_IV_BYTES + \ + TICKET_CRYPT_LEN_BYTES ) + /* * Generate/update a key */ @@ -278,6 +291,7 @@ static int ssl_load_session( mbedtls_ssl_session *session, * The key_name, iv, and length of encrypted_state are the additional * authenticated data. */ + int mbedtls_ssl_ticket_write( void *p_ticket, const mbedtls_ssl_session *session, unsigned char *start, @@ -289,9 +303,9 @@ int mbedtls_ssl_ticket_write( void *p_ticket, mbedtls_ssl_ticket_context *ctx = p_ticket; mbedtls_ssl_ticket_key *key; unsigned char *key_name = start; - unsigned char *iv = start + 4; - unsigned char *state_len_bytes = iv + 12; - unsigned char *state = state_len_bytes + 2; + unsigned char *iv = start + TICKET_KEY_NAME_BYTES; + unsigned char *state_len_bytes = iv + TICKET_IV_BYTES; + unsigned char *state = state_len_bytes + TICKET_CRYPT_LEN_BYTES; unsigned char *tag; size_t clear_len, ciph_len; @@ -302,7 +316,7 @@ int mbedtls_ssl_ticket_write( void *p_ticket, /* We need at least 4 bytes for key_name, 12 for IV, 2 for len 16 for tag, * in addition to session itself, that will be checked when writing it. */ - if( end - start < 4 + 12 + 2 + 16 ) + if( end - start < TICKET_MIN_LEN ) return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); #if defined(MBEDTLS_THREADING_C) @@ -317,9 +331,9 @@ int mbedtls_ssl_ticket_write( void *p_ticket, *ticket_lifetime = ctx->ticket_lifetime; - memcpy( key_name, key->name, 4 ); + memcpy( key_name, key->name, TICKET_KEY_NAME_BYTES ); - if( ( ret = ctx->f_rng( ctx->p_rng, iv, 12 ) ) != 0 ) + if( ( ret = ctx->f_rng( ctx->p_rng, iv, TICKET_IV_BYTES ) ) != 0 ) goto cleanup; /* Dump session state */ @@ -335,8 +349,11 @@ int mbedtls_ssl_ticket_write( void *p_ticket, /* Encrypt and authenticate */ tag = state + clear_len; if( ( ret = mbedtls_cipher_auth_encrypt( &key->ctx, - iv, 12, key_name, 4 + 12 + 2, - state, clear_len, state, &ciph_len, tag, 16 ) ) != 0 ) + iv, TICKET_IV_BYTES, + /* Additional data: key name, IV and length */ + key_name, TICKET_ADD_DATA_LEN, + state, clear_len, state, &ciph_len, + tag, TICKET_AUTH_TAG_BYTES ) ) != 0 ) { goto cleanup; } @@ -346,7 +363,7 @@ int mbedtls_ssl_ticket_write( void *p_ticket, goto cleanup; } - *tlen = 4 + 12 + 2 + 16 + ciph_len; + *tlen = TICKET_MIN_LEN + ciph_len; cleanup: #if defined(MBEDTLS_THREADING_C) @@ -385,17 +402,16 @@ int mbedtls_ssl_ticket_parse( void *p_ticket, mbedtls_ssl_ticket_context *ctx = p_ticket; mbedtls_ssl_ticket_key *key; unsigned char *key_name = buf; - unsigned char *iv = buf + 4; - unsigned char *enc_len_p = iv + 12; - unsigned char *ticket = enc_len_p + 2; + unsigned char *iv = buf + TICKET_KEY_NAME_BYTES; + unsigned char *enc_len_p = iv + TICKET_IV_BYTES; + unsigned char *ticket = enc_len_p + TICKET_CRYPT_LEN_BYTES; unsigned char *tag; size_t enc_len, clear_len; if( ctx == NULL || ctx->f_rng == NULL ) return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); - /* See mbedtls_ssl_ticket_write() */ - if( len < 4 + 12 + 2 + 16 ) + if( len < TICKET_MIN_LEN ) return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); #if defined(MBEDTLS_THREADING_C) @@ -409,7 +425,7 @@ int mbedtls_ssl_ticket_parse( void *p_ticket, enc_len = ( enc_len_p[0] << 8 ) | enc_len_p[1]; tag = ticket + enc_len; - if( len != 4 + 12 + 2 + enc_len + 16 ) + if( len != TICKET_MIN_LEN + enc_len ) { ret = MBEDTLS_ERR_SSL_BAD_INPUT_DATA; goto cleanup; @@ -425,9 +441,13 @@ int mbedtls_ssl_ticket_parse( void *p_ticket, } /* Decrypt and authenticate */ - if( ( ret = mbedtls_cipher_auth_decrypt( &key->ctx, iv, 12, - key_name, 4 + 12 + 2, ticket, enc_len, - ticket, &clear_len, tag, 16 ) ) != 0 ) + if( ( ret = mbedtls_cipher_auth_decrypt( &key->ctx, + iv, TICKET_IV_BYTES, + /* Additional data: key name, IV and length */ + key_name, TICKET_ADD_DATA_LEN, + ticket, enc_len, + ticket, &clear_len, + tag, TICKET_AUTH_TAG_BYTES ) ) != 0 ) { if( ret == MBEDTLS_ERR_CIPHER_AUTH_FAILED ) ret = MBEDTLS_ERR_SSL_INVALID_MAC; From 35f8a54405f331bfadc1fbea12d7758682548407 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 8 May 2017 11:06:19 +0100 Subject: [PATCH 02/10] Shorten lines in library/ssl_cli.c to at most 80 characters Signed-off-by: Ronald Cron --- library/ssl_cli.c | 535 +++++++++++++++++++++++++++++----------------- 1 file changed, 342 insertions(+), 193 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index c5c3af69d..67c619273 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -65,8 +65,9 @@ static void ssl_write_hostname_ext( mbedtls_ssl_context *ssl, if( ssl->hostname == NULL ) return; - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding server name extension: %s", - ssl->hostname ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "client hello, adding server name extension: %s", + ssl->hostname ) ); hostname_len = strlen( ssl->hostname ); @@ -137,7 +138,8 @@ static void ssl_write_renegotiation_ext( mbedtls_ssl_context *ssl, if( ssl->renego_status != MBEDTLS_SSL_RENEGOTIATION_IN_PROGRESS ) return; - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding renegotiation extension" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "client hello, adding renegotiation extension" ) ); if( end < p || (size_t)( end - p ) < 5 + ssl->verify_data_len ) { @@ -148,8 +150,10 @@ static void ssl_write_renegotiation_ext( mbedtls_ssl_context *ssl, /* * Secure renegotiation */ - *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_RENEGOTIATION_INFO >> 8 ) & 0xFF ); - *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_RENEGOTIATION_INFO ) & 0xFF ); + *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_RENEGOTIATION_INFO >> 8 ) + & 0xFF ); + *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_RENEGOTIATION_INFO ) + & 0xFF ); *p++ = 0x00; *p++ = ( ssl->verify_data_len + 1 ) & 0xFF; @@ -183,7 +187,8 @@ static void ssl_write_signature_algorithms_ext( mbedtls_ssl_context *ssl, if( ssl->conf->max_minor_ver != MBEDTLS_SSL_MINOR_VERSION_3 ) return; - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding signature_algorithms extension" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "client hello, adding signature_algorithms extension" ) ); for( md = ssl->conf->sig_hashes; *md != MBEDTLS_MD_NONE; md++ ) { @@ -268,12 +273,17 @@ static void ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl, *olen = 0; - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding supported_elliptic_curves extension" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "client hello, adding supported_elliptic_curves extension" ) ); #if defined(MBEDTLS_ECP_C) - for( grp_id = ssl->conf->curve_list; *grp_id != MBEDTLS_ECP_DP_NONE; grp_id++ ) + for( grp_id = ssl->conf->curve_list; + *grp_id != MBEDTLS_ECP_DP_NONE; + grp_id++ ) #else - for( info = mbedtls_ecp_curve_list(); info->grp_id != MBEDTLS_ECP_DP_NONE; info++ ) + for( info = mbedtls_ecp_curve_list(); + info->grp_id != MBEDTLS_ECP_DP_NONE; + info++ ) #endif { #if defined(MBEDTLS_ECP_C) @@ -281,7 +291,8 @@ static void ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl, #endif if( info == NULL ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "invalid curve in ssl configuration" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "invalid curve in ssl configuration" ) ); return; } @@ -297,9 +308,13 @@ static void ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl, elliptic_curve_len = 0; #if defined(MBEDTLS_ECP_C) - for( grp_id = ssl->conf->curve_list; *grp_id != MBEDTLS_ECP_DP_NONE; grp_id++ ) + for( grp_id = ssl->conf->curve_list; + *grp_id != MBEDTLS_ECP_DP_NONE; + grp_id++ ) #else - for( info = mbedtls_ecp_curve_list(); info->grp_id != MBEDTLS_ECP_DP_NONE; info++ ) + for( info = mbedtls_ecp_curve_list(); + info->grp_id != MBEDTLS_ECP_DP_NONE; + info++ ) #endif { #if defined(MBEDTLS_ECP_C) @@ -312,8 +327,10 @@ static void ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl, if( elliptic_curve_len == 0 ) return; - *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SUPPORTED_ELLIPTIC_CURVES >> 8 ) & 0xFF ); - *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SUPPORTED_ELLIPTIC_CURVES ) & 0xFF ); + *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SUPPORTED_ELLIPTIC_CURVES >> 8 ) + & 0xFF ); + *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SUPPORTED_ELLIPTIC_CURVES ) + & 0xFF ); *p++ = (unsigned char)( ( ( elliptic_curve_len + 2 ) >> 8 ) & 0xFF ); *p++ = (unsigned char)( ( ( elliptic_curve_len + 2 ) ) & 0xFF ); @@ -333,7 +350,8 @@ static void ssl_write_supported_point_formats_ext( mbedtls_ssl_context *ssl, *olen = 0; - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding supported_point_formats extension" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "client hello, adding supported_point_formats extension" ) ); if( end < p || (size_t)( end - p ) < 6 ) { @@ -341,8 +359,10 @@ static void ssl_write_supported_point_formats_ext( mbedtls_ssl_context *ssl, return; } - *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SUPPORTED_POINT_FORMATS >> 8 ) & 0xFF ); - *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SUPPORTED_POINT_FORMATS ) & 0xFF ); + *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SUPPORTED_POINT_FORMATS >> 8 ) + & 0xFF ); + *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SUPPORTED_POINT_FORMATS ) + & 0xFF ); *p++ = 0x00; *p++ = 2; @@ -371,7 +391,8 @@ static void ssl_write_ecjpake_kkpp_ext( mbedtls_ssl_context *ssl, if( mbedtls_ecjpake_check( &ssl->handshake->ecjpake_ctx ) != 0 ) return; - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding ecjpake_kkpp extension" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "client hello, adding ecjpake_kkpp extension" ) ); if( end - p < 4 ) { @@ -397,7 +418,8 @@ static void ssl_write_ecjpake_kkpp_ext( mbedtls_ssl_context *ssl, ssl->conf->f_rng, ssl->conf->p_rng ); if( ret != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1 , "mbedtls_ecjpake_write_round_one", ret ); + MBEDTLS_SSL_DEBUG_RET( 1 , + "mbedtls_ecjpake_write_round_one", ret ); return; } @@ -447,7 +469,8 @@ static void ssl_write_max_fragment_length_ext( mbedtls_ssl_context *ssl, return; } - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding max_fragment_length extension" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "client hello, adding max_fragment_length extension" ) ); if( end < p || (size_t)( end - p ) < 5 ) { @@ -455,8 +478,10 @@ static void ssl_write_max_fragment_length_ext( mbedtls_ssl_context *ssl, return; } - *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_MAX_FRAGMENT_LENGTH >> 8 ) & 0xFF ); - *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_MAX_FRAGMENT_LENGTH ) & 0xFF ); + *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_MAX_FRAGMENT_LENGTH >> 8 ) + & 0xFF ); + *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_MAX_FRAGMENT_LENGTH ) + & 0xFF ); *p++ = 0x00; *p++ = 1; @@ -481,7 +506,8 @@ static void ssl_write_truncated_hmac_ext( mbedtls_ssl_context *ssl, return; } - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding truncated_hmac extension" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "client hello, adding truncated_hmac extension" ) ); if( end < p || (size_t)( end - p ) < 4 ) { @@ -514,8 +540,8 @@ static void ssl_write_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, return; } - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding encrypt_then_mac " - "extension" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "client hello, adding encrypt_then_mac extension" ) ); if( end < p || (size_t)( end - p ) < 4 ) { @@ -548,8 +574,8 @@ static void ssl_write_extended_ms_ext( mbedtls_ssl_context *ssl, return; } - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding extended_master_secret " - "extension" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "client hello, adding extended_master_secret extension" ) ); if( end < p || (size_t)( end - p ) < 4 ) { @@ -557,8 +583,10 @@ static void ssl_write_extended_ms_ext( mbedtls_ssl_context *ssl, return; } - *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_EXTENDED_MASTER_SECRET >> 8 ) & 0xFF ); - *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_EXTENDED_MASTER_SECRET ) & 0xFF ); + *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_EXTENDED_MASTER_SECRET >> 8 ) + & 0xFF ); + *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_EXTENDED_MASTER_SECRET ) + & 0xFF ); *p++ = 0x00; *p++ = 0x00; @@ -582,7 +610,8 @@ static void ssl_write_session_ticket_ext( mbedtls_ssl_context *ssl, return; } - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding session ticket extension" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "client hello, adding session ticket extension" ) ); if( end < p || (size_t)( end - p ) < 4 + tlen ) { @@ -603,7 +632,8 @@ static void ssl_write_session_ticket_ext( mbedtls_ssl_context *ssl, return; } - MBEDTLS_SSL_DEBUG_MSG( 3, ( "sending session ticket of length %d", tlen ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "sending session ticket of length %d", tlen ) ); memcpy( p, ssl->session_negotiate->ticket, tlen ); @@ -724,9 +754,10 @@ static int ssl_generate_random( mbedtls_ssl_context *ssl ) * * \return 0 if valid, else 1 */ -static int ssl_validate_ciphersuite( const mbedtls_ssl_ciphersuite_t * suite_info, - const mbedtls_ssl_context * ssl, - int min_minor_ver, int max_minor_ver ) +static int ssl_validate_ciphersuite( + const mbedtls_ssl_ciphersuite_t * suite_info, + const mbedtls_ssl_context * ssl, + int min_minor_ver, int max_minor_ver ) { (void) ssl; if( suite_info == NULL ) @@ -789,8 +820,8 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) if( ssl->conf->max_major_ver == 0 ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "configured max major version is invalid, " - "consider using mbedtls_ssl_config_defaults()" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "configured max major version is invalid, consider using mbedtls_ssl_config_defaults()" ) ); return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); } @@ -804,8 +835,9 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) buf = ssl->out_msg; p = buf + 4; - mbedtls_ssl_write_version( ssl->conf->max_major_ver, ssl->conf->max_minor_ver, - ssl->conf->transport, p ); + mbedtls_ssl_write_version( ssl->conf->max_major_ver, + ssl->conf->max_minor_ver, + ssl->conf->transport, p ); p += 2; MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, max version: [%d:%d]", @@ -856,7 +888,8 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) if( ssl->session_negotiate->ticket != NULL && ssl->session_negotiate->ticket_len != 0 ) { - ret = ssl->conf->f_rng( ssl->conf->p_rng, ssl->session_negotiate->id, 32 ); + ret = ssl->conf->f_rng( ssl->conf->p_rng, + ssl->session_negotiate->id, 32 ); if( ret != 0 ) return( ret ); @@ -931,7 +964,8 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) *p++ = (unsigned char)( ciphersuites[i] ); } - MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, got %d ciphersuites (excluding SCSVs)", n ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "client hello, got %d ciphersuites (excluding SCSVs)", n ) ); /* * Add TLS_EMPTY_RENEGOTIATION_INFO_SCSV @@ -981,7 +1015,8 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, compress len.: %d", 2 ) ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, compress alg.: %d %d", - MBEDTLS_SSL_COMPRESS_DEFLATE, MBEDTLS_SSL_COMPRESS_NULL ) ); + MBEDTLS_SSL_COMPRESS_DEFLATE, + MBEDTLS_SSL_COMPRESS_NULL ) ); *p++ = 2; *p++ = MBEDTLS_SSL_COMPRESS_DEFLATE; @@ -1124,8 +1159,10 @@ static int ssl_parse_renegotiation_info( mbedtls_ssl_context *ssl, ssl->peer_verify_data, ssl->verify_data_len ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-matching renegotiation info" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } } @@ -1134,9 +1171,12 @@ static int ssl_parse_renegotiation_info( mbedtls_ssl_context *ssl, { if( len != 1 || buf[0] != 0x00 ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-zero length renegotiation info" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); + MBEDTLS_SSL_DEBUG_MSG( + 1, ( "non-zero length renegotiation info" ) ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } @@ -1159,9 +1199,12 @@ static int ssl_parse_max_fragment_length_ext( mbedtls_ssl_context *ssl, len != 1 || buf[0] != ssl->conf->mfl_code ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-matching max fragment length extension" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "non-matching max fragment length extension" ) ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } @@ -1177,9 +1220,12 @@ static int ssl_parse_truncated_hmac_ext( mbedtls_ssl_context *ssl, if( ssl->conf->trunc_hmac == MBEDTLS_SSL_TRUNC_HMAC_DISABLED || len != 0 ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-matching truncated HMAC extension" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "non-matching truncated HMAC extension" ) ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } @@ -1200,9 +1246,12 @@ static int ssl_parse_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 || len != 0 ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-matching encrypt-then-MAC extension" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "non-matching encrypt-then-MAC extension" ) ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } @@ -1223,9 +1272,12 @@ static int ssl_parse_extended_ms_ext( mbedtls_ssl_context *ssl, ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 || len != 0 ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-matching extended master secret extension" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "non-matching extended master secret extension" ) ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } @@ -1245,9 +1297,12 @@ static int ssl_parse_session_ticket_ext( mbedtls_ssl_context *ssl, if( ssl->conf->session_tickets == MBEDTLS_SSL_SESSION_TICKETS_DISABLED || len != 0 ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-matching session ticket extension" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "non-matching session ticket extension" ) ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } @@ -1328,8 +1383,10 @@ static int ssl_parse_ecjpake_kkpp( mbedtls_ssl_context *ssl, buf, len ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_read_round_one", ret ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); return( ret ); } @@ -1348,8 +1405,10 @@ static int ssl_parse_alpn_ext( mbedtls_ssl_context *ssl, if( ssl->conf->alpn_list == NULL ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-matching ALPN extension" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } @@ -1529,12 +1588,13 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) if( ssl->conf->renego_max_records >= 0 && ssl->renego_records_seen > ssl->conf->renego_max_records ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "renegotiation requested, " - "but not honored by server" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "renegotiation requested, but not honored by server" ) ); return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); } - MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-handshake message during renego" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "non-handshake message during renegotiation" ) ); ssl->keep_current_message = 1; return( MBEDTLS_ERR_SSL_WAITING_SERVER_HELLO_RENEGO ); @@ -1542,8 +1602,10 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_SSL_RENEGOTIATION */ MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); } @@ -1597,11 +1659,13 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) ssl->major_ver > ssl->conf->max_major_ver || ssl->minor_ver > ssl->conf->max_minor_ver ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "server version out of bounds - " - " min: [%d:%d], server: [%d:%d], max: [%d:%d]", - ssl->conf->min_major_ver, ssl->conf->min_minor_ver, - ssl->major_ver, ssl->minor_ver, - ssl->conf->max_major_ver, ssl->conf->max_minor_ver ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "server version out of bounds - min: [%d:%d], server: [%d:%d], max: [%d:%d]", + ssl->conf->min_major_ver, + ssl->conf->min_minor_ver, + ssl->major_ver, ssl->minor_ver, + ssl->conf->max_major_ver, + ssl->conf->max_minor_ver ) ); mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, MBEDTLS_SSL_ALERT_MSG_PROTOCOL_VERSION ); @@ -1638,8 +1702,10 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) ssl->in_hslen != mbedtls_ssl_hs_hdr_len( ssl ) + 40 + n + ext_len ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } } @@ -1678,26 +1744,32 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) if( comp != MBEDTLS_SSL_COMPRESS_NULL ) #endif/* MBEDTLS_ZLIB_SUPPORT */ { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "server hello, bad compression: %d", comp ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "server hello, bad compression: %d", comp ) ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); } /* * Initialize update checksum functions */ - ssl->transform_negotiate->ciphersuite_info = mbedtls_ssl_ciphersuite_from_id( i ); + ssl->transform_negotiate->ciphersuite_info = + mbedtls_ssl_ciphersuite_from_id( i ); if( ssl->transform_negotiate->ciphersuite_info == NULL ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "ciphersuite info for %04x not found", i ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "ciphersuite info for %04x not found", i ) ); mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, MBEDTLS_SSL_ALERT_MSG_INTERNAL_ERROR ); return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); } - mbedtls_ssl_optimize_checksum( ssl, ssl->transform_negotiate->ciphersuite_info ); + mbedtls_ssl_optimize_checksum( ssl, + ssl->transform_negotiate->ciphersuite_info ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, session id len.: %d", n ) ); MBEDTLS_SSL_DEBUG_BUF( 3, "server hello, session id", buf + 35, n ); @@ -1731,8 +1803,10 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) if( ( ret = mbedtls_ssl_derive_keys( ssl ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_derive_keys", ret ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_INTERNAL_ERROR ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_INTERNAL_ERROR ); return( ret ); } } @@ -1741,7 +1815,8 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) ssl->handshake->resume ? "a" : "no" ) ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, chosen ciphersuite: %04x", i ) ); - MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, compress alg.: %d", buf[37 + n] ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, compress alg.: %d", + buf[37 + n] ) ); /* * Perform cipher suite validation in same way as in ssl_write_client_hello. @@ -1752,8 +1827,10 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) if( ssl->conf->ciphersuite_list[ssl->minor_ver][i] == 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } @@ -1764,16 +1841,21 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) } } - suite_info = mbedtls_ssl_ciphersuite_from_id( ssl->session_negotiate->ciphersuite ); - if( ssl_validate_ciphersuite( suite_info, ssl, ssl->minor_ver, ssl->minor_ver ) != 0 ) + suite_info = mbedtls_ssl_ciphersuite_from_id( + ssl->session_negotiate->ciphersuite ); + if( ssl_validate_ciphersuite( suite_info, ssl, ssl->minor_ver, + ssl->minor_ver ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } - MBEDTLS_SSL_DEBUG_MSG( 3, ( "server hello, chosen ciphersuite: %s", suite_info->name ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "server hello, chosen ciphersuite: %s", suite_info->name ) ); #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) if( suite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA && @@ -1790,15 +1872,18 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } ssl->session_negotiate->compression = comp; ext = buf + 40 + n; - MBEDTLS_SSL_DEBUG_MSG( 2, ( "server hello, total extension length: %d", ext_len ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, + ( "server hello, total extension length: %d", ext_len ) ); while( ext_len ) { @@ -1810,8 +1895,9 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) if( ext_size + 4 > ext_len ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); + mbedtls_ssl_send_alert_message( + ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } @@ -1831,7 +1917,8 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) case MBEDTLS_TLS_EXT_MAX_FRAGMENT_LENGTH: - MBEDTLS_SSL_DEBUG_MSG( 3, ( "found max_fragment_length extension" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "found max_fragment_length extension" ) ); if( ( ret = ssl_parse_max_fragment_length_ext( ssl, ext + 4, ext_size ) ) != 0 ) @@ -1870,7 +1957,8 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) case MBEDTLS_TLS_EXT_EXTENDED_MASTER_SECRET: - MBEDTLS_SSL_DEBUG_MSG( 3, ( "found extended_master_secret extension" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "found extended_master_secret extension" ) ); if( ( ret = ssl_parse_extended_ms_ext( ssl, ext + 4, ext_size ) ) != 0 ) @@ -1897,7 +1985,8 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) case MBEDTLS_TLS_EXT_SUPPORTED_POINT_FORMATS: - MBEDTLS_SSL_DEBUG_MSG( 3, ( "found supported_point_formats extension" ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "found supported_point_formats extension" ) ); if( ( ret = ssl_parse_supported_point_formats_ext( ssl, ext + 4, ext_size ) ) != 0 ) @@ -1933,8 +2022,8 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_SSL_ALPN */ default: - MBEDTLS_SSL_DEBUG_MSG( 3, ( "unknown extension found: %d (ignoring)", - ext_id ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "unknown extension found: %d (ignoring)", ext_id ) ); } ext_len -= 4 + ext_size; @@ -1951,9 +2040,11 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) * Renegotiation security checks */ if( ssl->secure_renegotiation == MBEDTLS_SSL_LEGACY_RENEGOTIATION && - ssl->conf->allow_legacy_renegotiation == MBEDTLS_SSL_LEGACY_BREAK_HANDSHAKE ) + ssl->conf->allow_legacy_renegotiation == + MBEDTLS_SSL_LEGACY_BREAK_HANDSHAKE ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "legacy renegotiation, breaking off handshake" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "legacy renegotiation, breaking off handshake" ) ); handshake_failure = 1; } #if defined(MBEDTLS_SSL_RENEGOTIATION) @@ -1961,12 +2052,14 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) ssl->secure_renegotiation == MBEDTLS_SSL_SECURE_RENEGOTIATION && renegotiation_info_seen == 0 ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "renegotiation_info extension missing (secure)" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "renegotiation_info extension missing (secure)" ) ); handshake_failure = 1; } else if( ssl->renego_status == MBEDTLS_SSL_RENEGOTIATION_IN_PROGRESS && ssl->secure_renegotiation == MBEDTLS_SSL_LEGACY_RENEGOTIATION && - ssl->conf->allow_legacy_renegotiation == MBEDTLS_SSL_LEGACY_NO_RENEGOTIATION ) + ssl->conf->allow_legacy_renegotiation == + MBEDTLS_SSL_LEGACY_NO_RENEGOTIATION ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "legacy renegotiation not allowed" ) ); handshake_failure = 1; @@ -1975,15 +2068,18 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) ssl->secure_renegotiation == MBEDTLS_SSL_LEGACY_RENEGOTIATION && renegotiation_info_seen == 1 ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "renegotiation_info extension present (legacy)" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "renegotiation_info extension present (legacy)" ) ); handshake_failure = 1; } #endif /* MBEDTLS_SSL_RENEGOTIATION */ if( handshake_failure == 1 ) { - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_HELLO ); } @@ -1994,7 +2090,8 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_KEY_EXCHANGE_DHE_RSA_ENABLED) || \ defined(MBEDTLS_KEY_EXCHANGE_DHE_PSK_ENABLED) -static int ssl_parse_server_dh_params( mbedtls_ssl_context *ssl, unsigned char **p, +static int ssl_parse_server_dh_params( mbedtls_ssl_context *ssl, + unsigned char **p, unsigned char *end ) { int ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; @@ -2008,7 +2105,8 @@ static int ssl_parse_server_dh_params( mbedtls_ssl_context *ssl, unsigned char * * opaque dh_Ys<1..2^16-1>; * } ServerDHParams; */ - if( ( ret = mbedtls_dhm_read_params( &ssl->handshake->dhm_ctx, p, end ) ) != 0 ) + if( ( ret = mbedtls_dhm_read_params( &ssl->handshake->dhm_ctx, + p, end ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 2, ( "mbedtls_dhm_read_params" ), ret ); return( ret ); @@ -2104,7 +2202,8 @@ static int ssl_parse_server_ecdh_params( mbedtls_ssl_context *ssl, if( ssl_check_server_ecdh_params( ssl ) != 0 ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message (ECDHE curve)" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "bad server key exchange message (ECDHE curve)" ) ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); } @@ -2130,8 +2229,8 @@ static int ssl_parse_server_psk_hint( mbedtls_ssl_context *ssl, */ if( end - (*p) < 2 ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message " - "(psk_identity_hint length)" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "bad server key exchange message (psk_identity_hint length)" ) ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); } len = (*p)[0] << 8 | (*p)[1]; @@ -2139,8 +2238,8 @@ static int ssl_parse_server_psk_hint( mbedtls_ssl_context *ssl, if( end - (*p) < (int) len ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message " - "(psk_identity_hint length)" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "bad server key exchange message (psk_identity_hint length)" ) ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); } @@ -2182,8 +2281,9 @@ static int ssl_write_encrypted_pms( mbedtls_ssl_context *ssl, * opaque random[46]; * } PreMasterSecret; */ - mbedtls_ssl_write_version( ssl->conf->max_major_ver, ssl->conf->max_minor_ver, - ssl->conf->transport, p ); + mbedtls_ssl_write_version( ssl->conf->max_major_ver, + ssl->conf->max_minor_ver, + ssl->conf->transport, p ); if( ( ret = ssl->conf->f_rng( ssl->conf->p_rng, p + 2, 46 ) ) != 0 ) { @@ -2260,20 +2360,22 @@ static int ssl_parse_signature_algorithm( mbedtls_ssl_context *ssl, /* * Get hash algorithm */ - if( ( *md_alg = mbedtls_ssl_md_alg_from_hash( (*p)[0] ) ) == MBEDTLS_MD_NONE ) + if( ( *md_alg = mbedtls_ssl_md_alg_from_hash( (*p)[0] ) ) + == MBEDTLS_MD_NONE ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "Server used unsupported " - "HashAlgorithm %d", *(p)[0] ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "Server used unsupported HashAlgorithm %d", *(p)[0] ) ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); } /* * Get signature algorithm */ - if( ( *pk_alg = mbedtls_ssl_pk_alg_from_sig( (*p)[1] ) ) == MBEDTLS_PK_NONE ) + if( ( *pk_alg = mbedtls_ssl_pk_alg_from_sig( (*p)[1] ) ) + == MBEDTLS_PK_NONE ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "server used unsupported " - "SignatureAlgorithm %d", (*p)[1] ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "server used unsupported SignatureAlgorithm %d", (*p)[1] ) ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); } @@ -2282,13 +2384,15 @@ static int ssl_parse_signature_algorithm( mbedtls_ssl_context *ssl, */ if( mbedtls_ssl_check_sig_hash( ssl, *md_alg ) != 0 ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "server used HashAlgorithm %d that was not offered", - *(p)[0] ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "server used HashAlgorithm %d that was not offered", *(p)[0] ) ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); } - MBEDTLS_SSL_DEBUG_MSG( 2, ( "Server used SignatureAlgorithm %d", (*p)[1] ) ); - MBEDTLS_SSL_DEBUG_MSG( 2, ( "Server used HashAlgorithm %d", (*p)[0] ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "Server used SignatureAlgorithm %d", + (*p)[1] ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "Server used HashAlgorithm %d", + (*p)[0] ) ); *p += 2; return( 0 ); @@ -2366,8 +2470,10 @@ static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl ) if( ( ret = ssl_get_ecdh_params_from_cert( ssl ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "ssl_get_ecdh_params_from_cert", ret ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); return( ret ); } @@ -2397,8 +2503,10 @@ static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl ) if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); } @@ -2417,10 +2525,12 @@ static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl ) goto exit; } - MBEDTLS_SSL_DEBUG_MSG( 1, ( "server key exchange message must " - "not be skipped" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "server key exchange message must not be skipped" ) ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); } @@ -2444,8 +2554,10 @@ start_processing: if( ssl_parse_server_psk_hint( ssl, &p, end ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); } } /* FALLTROUGH */ @@ -2467,8 +2579,10 @@ start_processing: if( ssl_parse_server_dh_params( ssl, &p, end ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); } } @@ -2485,8 +2599,10 @@ start_processing: if( ssl_parse_server_ecdh_params( ssl, &p, end ) != 0 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); } } @@ -2502,8 +2618,10 @@ start_processing: if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecjpake_read_round_two", ret ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); } } @@ -2534,17 +2652,24 @@ start_processing: if( ssl_parse_signature_algorithm( ssl, &p, end, &md_alg, &pk_alg ) != 0 ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "bad server key exchange message" ) ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); } - if( pk_alg != mbedtls_ssl_get_ciphersuite_sig_pk_alg( ciphersuite_info ) ) + if( pk_alg != + mbedtls_ssl_get_ciphersuite_sig_pk_alg( ciphersuite_info ) ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "bad server key exchange message" ) ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_ILLEGAL_PARAMETER ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); } } @@ -2574,8 +2699,10 @@ start_processing: if( p > end - 2 ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); } sig_len = ( p[0] << 8 ) | p[1]; @@ -2584,8 +2711,10 @@ start_processing: if( p != end - sig_len ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE ); } @@ -2630,19 +2759,24 @@ start_processing: if( ssl->session_negotiate->peer_cert == NULL ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "certificate required" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); } /* * Verify signature */ - if( ! mbedtls_pk_can_do( &ssl->session_negotiate->peer_cert->pk, pk_alg ) ) + if( ! mbedtls_pk_can_do( &ssl->session_negotiate->peer_cert->pk, + pk_alg ) ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE ); return( MBEDTLS_ERR_SSL_PK_TYPE_MISMATCH ); } @@ -2658,8 +2792,10 @@ start_processing: #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) if( ret != MBEDTLS_ERR_ECP_IN_PROGRESS ) #endif - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_DECRYPT_ERROR ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_DECRYPT_ERROR ); MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_pk_verify", ret ); #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) if( ret == MBEDTLS_ERR_ECP_IN_PROGRESS ) @@ -2724,8 +2860,10 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); } @@ -2801,8 +2939,9 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_SSL_PROTO_TLS1_2) if( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_3 ) { - size_t sig_alg_len = ( ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 1 + n] << 8 ) - | ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 2 + n] ) ); + size_t sig_alg_len = + ( ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 1 + n] << 8 ) + | ( buf[mbedtls_ssl_hs_hdr_len( ssl ) + 2 + n] ) ); #if defined(MBEDTLS_DEBUG_C) unsigned char* sig_alg; size_t i; @@ -2820,11 +2959,14 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) * buf[...hdr_len + 3 + n + sig_alg_len], * which is one less than we need the buf to be. */ - if( ssl->in_hslen <= mbedtls_ssl_hs_hdr_len( ssl ) + 3 + n + sig_alg_len ) + if( ssl->in_hslen <= mbedtls_ssl_hs_hdr_len( ssl ) + + 3 + n + sig_alg_len ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_DECODE_ERROR ); return( MBEDTLS_ERR_SSL_BAD_HS_CERTIFICATE_REQUEST ); } @@ -2832,8 +2974,9 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl ) sig_alg = buf + mbedtls_ssl_hs_hdr_len( ssl ) + 3 + n; for( i = 0; i < sig_alg_len; i += 2 ) { - MBEDTLS_SSL_DEBUG_MSG( 3, ( "Supported Signature Algorithm found: %d" - ",%d", sig_alg[i], sig_alg[i + 1] ) ); + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "Supported Signature Algorithm found: %d,%d", + sig_alg[i], sig_alg[i + 1] ) ); } #endif @@ -2922,9 +3065,9 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) i = 6; ret = mbedtls_dhm_make_public( &ssl->handshake->dhm_ctx, - (int) mbedtls_mpi_size( &ssl->handshake->dhm_ctx.P ), - &ssl->out_msg[i], n, - ssl->conf->f_rng, ssl->conf->p_rng ); + (int) mbedtls_mpi_size( &ssl->handshake->dhm_ctx.P ), + &ssl->out_msg[i], n, + ssl->conf->f_rng, ssl->conf->p_rng ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_make_public", ret ); @@ -2935,10 +3078,10 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MPI( 3, "DHM: GX", &ssl->handshake->dhm_ctx.GX ); if( ( ret = mbedtls_dhm_calc_secret( &ssl->handshake->dhm_ctx, - ssl->handshake->premaster, - MBEDTLS_PREMASTER_SIZE, - &ssl->handshake->pmslen, - ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) + ssl->handshake->premaster, + MBEDTLS_PREMASTER_SIZE, + &ssl->handshake->pmslen, + ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_dhm_calc_secret", ret ); return( ret ); @@ -3001,10 +3144,10 @@ ecdh_calc_secret: n = ssl->handshake->ecrs_n; #endif if( ( ret = mbedtls_ecdh_calc_secret( &ssl->handshake->ecdh_ctx, - &ssl->handshake->pmslen, - ssl->handshake->premaster, - MBEDTLS_MPI_MAX_SIZE, - ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) + &ssl->handshake->pmslen, + ssl->handshake->premaster, + MBEDTLS_MPI_MAX_SIZE, + ssl->conf->f_rng, ssl->conf->p_rng ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ecdh_calc_secret", ret ); #if defined(MBEDTLS_SSL__ECP_RESTARTABLE) @@ -3039,15 +3182,17 @@ ecdh_calc_secret: if( i + 2 + n > MBEDTLS_SSL_OUT_CONTENT_LEN ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "psk identity too long or " - "SSL buffer too short" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "psk identity too long or SSL buffer too short" ) ); return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); } ssl->out_msg[i++] = (unsigned char)( n >> 8 ); ssl->out_msg[i++] = (unsigned char)( n ); - memcpy( ssl->out_msg + i, ssl->conf->psk_identity, ssl->conf->psk_identity_len ); + memcpy( ssl->out_msg + i, + ssl->conf->psk_identity, + ssl->conf->psk_identity_len ); i += ssl->conf->psk_identity_len; #if defined(MBEDTLS_KEY_EXCHANGE_PSK_ENABLED) @@ -3075,8 +3220,8 @@ ecdh_calc_secret: if( i + 2 + n > MBEDTLS_SSL_OUT_CONTENT_LEN ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "psk identity or DHM size too long" - " or SSL buffer too short" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "psk identity or DHM size too long or SSL buffer too short" ) ); return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); } @@ -3123,7 +3268,8 @@ ecdh_calc_secret: if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, ciphersuite_info->key_exchange ) ) != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_psk_derive_premaster", ret ); + MBEDTLS_SSL_DEBUG_RET( + 1, "mbedtls_ssl_psk_derive_premaster", ret ); return( ret ); } } @@ -3332,8 +3478,9 @@ sign: * Until we encounter a server that does not, we will take this * shortcut. * - * Reason: Otherwise we should have running hashes for SHA512 and SHA224 - * in order to satisfy 'weird' needs from the server side. + * Reason: Otherwise we should have running hashes for SHA512 and + * SHA224 in order to satisfy 'weird' needs from the server + * side. */ if( ssl->transform_negotiate->ciphersuite_info->mac == MBEDTLS_MD_SHA384 ) @@ -3423,8 +3570,10 @@ static int ssl_parse_new_session_ticket( mbedtls_ssl_context *ssl ) if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad new session ticket message" ) ); - mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, - MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); + mbedtls_ssl_send_alert_message( + ssl, + MBEDTLS_SSL_ALERT_LEVEL_FATAL, + MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE ); return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE ); } From 711eea30b93cf6a51492720d21a153c59d185a30 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 7 May 2020 10:54:43 +0200 Subject: [PATCH 03/10] Remove unnecessary MBEDTLS_ECP_C preprocessor condition The ssl_cli.c:ssl_write_supported_elliptic_curves_ext() function is compiled only if MBEDTLS_ECDH_C, MBEDTLS_ECDSA_C or MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED is defined which implies that MBEDTLS_ECP_C is defined. Thus remove the precompiler conditions on MBEDTLS_ECP_C in its code. Signed-off-by: Ronald Cron --- library/ssl_cli.c | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 67c619273..9f7f11b54 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -265,30 +265,18 @@ static void ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl, unsigned char *elliptic_curve_list = p + 6; size_t elliptic_curve_len = 0; const mbedtls_ecp_curve_info *info; -#if defined(MBEDTLS_ECP_C) const mbedtls_ecp_group_id *grp_id; -#else - ((void) ssl); -#endif *olen = 0; MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding supported_elliptic_curves extension" ) ); -#if defined(MBEDTLS_ECP_C) for( grp_id = ssl->conf->curve_list; *grp_id != MBEDTLS_ECP_DP_NONE; grp_id++ ) -#else - for( info = mbedtls_ecp_curve_list(); - info->grp_id != MBEDTLS_ECP_DP_NONE; - info++ ) -#endif { -#if defined(MBEDTLS_ECP_C) info = mbedtls_ecp_curve_info_from_grp_id( *grp_id ); -#endif if( info == NULL ) { MBEDTLS_SSL_DEBUG_MSG( 1, @@ -307,19 +295,11 @@ static void ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl, elliptic_curve_len = 0; -#if defined(MBEDTLS_ECP_C) for( grp_id = ssl->conf->curve_list; *grp_id != MBEDTLS_ECP_DP_NONE; grp_id++ ) -#else - for( info = mbedtls_ecp_curve_list(); - info->grp_id != MBEDTLS_ECP_DP_NONE; - info++ ) -#endif { -#if defined(MBEDTLS_ECP_C) info = mbedtls_ecp_curve_info_from_grp_id( *grp_id ); -#endif elliptic_curve_list[elliptic_curve_len++] = info->tls_id >> 8; elliptic_curve_list[elliptic_curve_len++] = info->tls_id & 0xFF; } From 7ea4b4d70ab5e0ac339502254597514e46e3b935 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 12 Apr 2017 14:54:42 +0100 Subject: [PATCH 04/10] Add macro for bounds checking This commit adds a macro for buffer bounds checks in the SSL module. It takes the buffer's current and end position as the first argument(s), followed by the needed space; if the available space is too small, it returns an SSL_BUFFER_TOO_SMALL error. Signed-off-by: Ronald Cron --- include/mbedtls/ssl_internal.h | 35 ++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index bd5ad94db..82beea553 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -236,6 +236,41 @@ #define MBEDTLS_TLS_EXT_SUPPORTED_POINT_FORMATS_PRESENT (1 << 0) #define MBEDTLS_TLS_EXT_ECJPAKE_KKPP_OK (1 << 1) +/** + * \brief This function checks if the remaining size in a buffer is + * greater or equal than a needed space. + * + * \param cur Pointer to the current position in the buffer. + * \param end Pointer to one past the end of the buffer. + * \param need Needed space in bytes. + * + * \return Non-zero if the needed space is available in the buffer, 0 + * otherwise. + */ +static inline int mbedtls_ssl_chk_buf_ptr( const uint8_t *cur, + const uint8_t *end, size_t need ) +{ + return( cur <= end && need <= (size_t)( end - cur ) ); +} + +/** + * \brief This macro checks if the remaining size in a buffer is + * greater or equal than a needed space. If it is not the case, + * it returns an SSL_BUFFER_TOO_SMALL error. + * + * \param cur Pointer to the current position in the buffer. + * \param end Pointer to one past the end of the buffer. + * \param need Needed space in bytes. + * + */ +#define MBEDTLS_SSL_CHK_BUF_PTR( cur, end, need ) \ + do { \ + if( mbedtls_ssl_chk_buf_ptr( ( cur ), ( end ), ( need ) ) == 0 ) \ + { \ + return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); \ + } \ + } while( 0 ) + #ifdef __cplusplus extern "C" { #endif From f8f61aad0fad258bcaa09c83ae8cc5a77734695f Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 12 Apr 2017 14:54:42 +0100 Subject: [PATCH 05/10] Uniformize bounds checks using new macro This commit uses the previously defined macro to uniformize bounds checks in several places. It also adds bounds checks to the ClientHello writing function that were previously missing. Also, the functions adding extensions to the ClientHello message can now fail if the buffer is too small or a different error condition occurs, and moreover they take an additional buffer end parameter to free them from the assumption that one is writing to the default output buffer. Signed-off-by: Ronald Cron --- ChangeLog.d/uniformize_bounds_checks.txt | 9 + library/ssl_cli.c | 409 ++++++++++++++--------- library/ssl_cookie.c | 6 +- library/ssl_ticket.c | 4 +- 4 files changed, 258 insertions(+), 170 deletions(-) create mode 100644 ChangeLog.d/uniformize_bounds_checks.txt diff --git a/ChangeLog.d/uniformize_bounds_checks.txt b/ChangeLog.d/uniformize_bounds_checks.txt new file mode 100644 index 000000000..210ab1051 --- /dev/null +++ b/ChangeLog.d/uniformize_bounds_checks.txt @@ -0,0 +1,9 @@ +Bugfix + * Add additional bounds checks in ssl_write_client_hello() preventing + output buffer overflow if the configuration declared a buffer that was + too small. +Changes + * Abort the ClientHello writing function as soon as some extension doesn't + fit into the record buffer. Previously, such extensions were silently + dropped. As a consequence, the TLS handshake now fails when the output + buffer is not large enough to hold the ClientHello. diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 9f7f11b54..c8fb0d5dc 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -52,18 +52,18 @@ #endif #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) -static void ssl_write_hostname_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, - size_t *olen ) +static int ssl_write_hostname_ext( mbedtls_ssl_context *ssl, + unsigned char *buf, + const unsigned char *end, + size_t *olen ) { unsigned char *p = buf; - const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_OUT_CONTENT_LEN; size_t hostname_len; *olen = 0; if( ssl->hostname == NULL ) - return; + return( 0 ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding server name extension: %s", @@ -71,11 +71,7 @@ static void ssl_write_hostname_ext( mbedtls_ssl_context *ssl, hostname_len = strlen( ssl->hostname ); - if( end < p || (size_t)( end - p ) < hostname_len + 9 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); - return; - } + MBEDTLS_SSL_CHK_BUF_PTR( p, end, hostname_len + 9 ); /* * Sect. 3, RFC 6066 (TLS Extensions Definitions) @@ -119,16 +115,18 @@ static void ssl_write_hostname_ext( mbedtls_ssl_context *ssl, memcpy( p, ssl->hostname, hostname_len ); *olen = hostname_len + 9; + + return( 0 ); } #endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */ #if defined(MBEDTLS_SSL_RENEGOTIATION) -static void ssl_write_renegotiation_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, - size_t *olen ) +static int ssl_write_renegotiation_ext( mbedtls_ssl_context *ssl, + unsigned char *buf, + const unsigned char *end, + size_t *olen ) { unsigned char *p = buf; - const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_OUT_CONTENT_LEN; *olen = 0; @@ -136,16 +134,12 @@ static void ssl_write_renegotiation_ext( mbedtls_ssl_context *ssl, * initial ClientHello, in which case also adding the renegotiation * info extension is NOT RECOMMENDED as per RFC 5746 Section 3.4. */ if( ssl->renego_status != MBEDTLS_SSL_RENEGOTIATION_IN_PROGRESS ) - return; + return( 0 ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding renegotiation extension" ) ); - if( end < p || (size_t)( end - p ) < 5 + ssl->verify_data_len ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); - return; - } + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 5 + ssl->verify_data_len ); /* * Secure renegotiation @@ -162,6 +156,8 @@ static void ssl_write_renegotiation_ext( mbedtls_ssl_context *ssl, memcpy( p, ssl->own_verify_data, ssl->verify_data_len ); *olen = 5 + ssl->verify_data_len; + + return( 0 ); } #endif /* MBEDTLS_SSL_RENEGOTIATION */ @@ -170,14 +166,15 @@ static void ssl_write_renegotiation_ext( mbedtls_ssl_context *ssl, */ #if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) -static void ssl_write_signature_algorithms_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, - size_t *olen ) +static int ssl_write_signature_algorithms_ext( mbedtls_ssl_context *ssl, + unsigned char *buf, + const unsigned char *end, + size_t *olen ) { unsigned char *p = buf; - const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_OUT_CONTENT_LEN; size_t sig_alg_len = 0; const int *md; + #if defined(MBEDTLS_RSA_C) || defined(MBEDTLS_ECDSA_C) unsigned char *sig_alg_list = buf + 6; #endif @@ -185,7 +182,7 @@ static void ssl_write_signature_algorithms_ext( mbedtls_ssl_context *ssl, *olen = 0; if( ssl->conf->max_minor_ver != MBEDTLS_SSL_MINOR_VERSION_3 ) - return; + return( 0 ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding signature_algorithms extension" ) ); @@ -200,11 +197,7 @@ static void ssl_write_signature_algorithms_ext( mbedtls_ssl_context *ssl, #endif } - if( end < p || (size_t)( end - p ) < sig_alg_len + 6 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); - return; - } + MBEDTLS_SSL_CHK_BUF_PTR( p, end, sig_alg_len + 6 ); /* * Prepare signature_algorithms extension (TLS 1.2) @@ -250,18 +243,20 @@ static void ssl_write_signature_algorithms_ext( mbedtls_ssl_context *ssl, *p++ = (unsigned char)( ( sig_alg_len ) & 0xFF ); *olen = 6 + sig_alg_len; + + return( 0 ); } #endif /* MBEDTLS_SSL_PROTO_TLS1_2 && MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED */ #if defined(MBEDTLS_ECDH_C) || defined(MBEDTLS_ECDSA_C) || \ defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) -static void ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, - size_t *olen ) +static int ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl, + unsigned char *buf, + const unsigned char *end, + size_t *olen ) { unsigned char *p = buf; - const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_OUT_CONTENT_LEN; unsigned char *elliptic_curve_list = p + 6; size_t elliptic_curve_len = 0; const mbedtls_ecp_curve_info *info; @@ -281,17 +276,15 @@ static void ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl, { MBEDTLS_SSL_DEBUG_MSG( 1, ( "invalid curve in ssl configuration" ) ); - return; + return( 0 ); } - elliptic_curve_len += 2; } - if( end < p || (size_t)( end - p ) < 6 + elliptic_curve_len ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); - return; - } + if( elliptic_curve_len == 0 ) + return( 0 ); + + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 6 + elliptic_curve_len ); elliptic_curve_len = 0; @@ -304,9 +297,6 @@ static void ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl, elliptic_curve_list[elliptic_curve_len++] = info->tls_id & 0xFF; } - if( elliptic_curve_len == 0 ) - return; - *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SUPPORTED_ELLIPTIC_CURVES >> 8 ) & 0xFF ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SUPPORTED_ELLIPTIC_CURVES ) @@ -319,25 +309,23 @@ static void ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl, *p++ = (unsigned char)( ( ( elliptic_curve_len ) ) & 0xFF ); *olen = 6 + elliptic_curve_len; + + return( 0 ); } -static void ssl_write_supported_point_formats_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, - size_t *olen ) +static int ssl_write_supported_point_formats_ext( mbedtls_ssl_context *ssl, + unsigned char *buf, + const unsigned char *end, + size_t *olen ) { unsigned char *p = buf; - const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_OUT_CONTENT_LEN; + (void) ssl; /* ssl used for debugging only */ *olen = 0; MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding supported_point_formats extension" ) ); - - if( end < p || (size_t)( end - p ) < 6 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); - return; - } + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 6 ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SUPPORTED_POINT_FORMATS >> 8 ) & 0xFF ); @@ -351,34 +339,32 @@ static void ssl_write_supported_point_formats_ext( mbedtls_ssl_context *ssl, *p++ = MBEDTLS_ECP_PF_UNCOMPRESSED; *olen = 6; + + return( 0 ); } #endif /* MBEDTLS_ECDH_C || MBEDTLS_ECDSA_C || MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ #if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) -static void ssl_write_ecjpake_kkpp_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, - size_t *olen ) +static int ssl_write_ecjpake_kkpp_ext( mbedtls_ssl_context *ssl, + unsigned char *buf, + const unsigned char *end, + size_t *olen ) { int ret; unsigned char *p = buf; - const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_OUT_CONTENT_LEN; size_t kkpp_len; *olen = 0; /* Skip costly extension if we can't use EC J-PAKE anyway */ if( mbedtls_ecjpake_check( &ssl->handshake->ecjpake_ctx ) != 0 ) - return; + return( 0 ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding ecjpake_kkpp extension" ) ); - if( end - p < 4 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); - return; - } + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 4 ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_ECJPAKE_KKPP >> 8 ) & 0xFF ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_ECJPAKE_KKPP ) & 0xFF ); @@ -394,20 +380,20 @@ static void ssl_write_ecjpake_kkpp_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "generating new ecjpake parameters" ) ); ret = mbedtls_ecjpake_write_round_one( &ssl->handshake->ecjpake_ctx, - p + 2, end - p - 2, &kkpp_len, - ssl->conf->f_rng, ssl->conf->p_rng ); + p + 2, end - p - 2, &kkpp_len, + ssl->conf->f_rng, ssl->conf->p_rng ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1 , "mbedtls_ecjpake_write_round_one", ret ); - return; + return( ret ); } ssl->handshake->ecjpake_cache = mbedtls_calloc( 1, kkpp_len ); if( ssl->handshake->ecjpake_cache == NULL ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "allocation failed" ) ); - return; + return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); } memcpy( ssl->handshake->ecjpake_cache, p + 2, kkpp_len ); @@ -418,12 +404,7 @@ static void ssl_write_ecjpake_kkpp_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "re-using cached ecjpake parameters" ) ); kkpp_len = ssl->handshake->ecjpake_cache_len; - - if( (size_t)( end - p - 2 ) < kkpp_len ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); - return; - } + MBEDTLS_SSL_CHK_BUF_PTR( p + 2, end, kkpp_len ); memcpy( p + 2, ssl->handshake->ecjpake_cache, kkpp_len ); } @@ -432,31 +413,28 @@ static void ssl_write_ecjpake_kkpp_ext( mbedtls_ssl_context *ssl, *p++ = (unsigned char)( ( kkpp_len ) & 0xFF ); *olen = kkpp_len + 4; + + return( 0 ); } #endif /* MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED */ #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) -static void ssl_write_max_fragment_length_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, - size_t *olen ) +static int ssl_write_max_fragment_length_ext( mbedtls_ssl_context *ssl, + unsigned char *buf, + const unsigned char *end, + size_t *olen ) { unsigned char *p = buf; - const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_OUT_CONTENT_LEN; *olen = 0; - if( ssl->conf->mfl_code == MBEDTLS_SSL_MAX_FRAG_LEN_NONE ) { - return; - } + if( ssl->conf->mfl_code == MBEDTLS_SSL_MAX_FRAG_LEN_NONE ) + return( 0 ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding max_fragment_length extension" ) ); - if( end < p || (size_t)( end - p ) < 5 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); - return; - } + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 5 ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_MAX_FRAGMENT_LENGTH >> 8 ) & 0xFF ); @@ -469,31 +447,28 @@ static void ssl_write_max_fragment_length_ext( mbedtls_ssl_context *ssl, *p++ = ssl->conf->mfl_code; *olen = 5; + + return( 0 ); } #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ #if defined(MBEDTLS_SSL_TRUNCATED_HMAC) -static void ssl_write_truncated_hmac_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, size_t *olen ) +static int ssl_write_truncated_hmac_ext( mbedtls_ssl_context *ssl, + unsigned char *buf, + const unsigned char *end, + size_t *olen ) { unsigned char *p = buf; - const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_OUT_CONTENT_LEN; *olen = 0; if( ssl->conf->trunc_hmac == MBEDTLS_SSL_TRUNC_HMAC_DISABLED ) - { - return; - } + return( 0 ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding truncated_hmac extension" ) ); - if( end < p || (size_t)( end - p ) < 4 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); - return; - } + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 4 ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_TRUNCATED_HMAC >> 8 ) & 0xFF ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_TRUNCATED_HMAC ) & 0xFF ); @@ -502,32 +477,29 @@ static void ssl_write_truncated_hmac_ext( mbedtls_ssl_context *ssl, *p++ = 0x00; *olen = 4; + + return( 0 ); } #endif /* MBEDTLS_SSL_TRUNCATED_HMAC */ #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) -static void ssl_write_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, size_t *olen ) +static int ssl_write_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, + unsigned char *buf, + const unsigned char *end, + size_t *olen ) { unsigned char *p = buf; - const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_OUT_CONTENT_LEN; *olen = 0; if( ssl->conf->encrypt_then_mac == MBEDTLS_SSL_ETM_DISABLED || ssl->conf->max_minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) - { - return; - } + return( 0 ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding encrypt_then_mac extension" ) ); - if( end < p || (size_t)( end - p ) < 4 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); - return; - } + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 4 ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_ENCRYPT_THEN_MAC >> 8 ) & 0xFF ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_ENCRYPT_THEN_MAC ) & 0xFF ); @@ -536,32 +508,29 @@ static void ssl_write_encrypt_then_mac_ext( mbedtls_ssl_context *ssl, *p++ = 0x00; *olen = 4; + + return( 0 ); } #endif /* MBEDTLS_SSL_ENCRYPT_THEN_MAC */ #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) -static void ssl_write_extended_ms_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, size_t *olen ) +static int ssl_write_extended_ms_ext( mbedtls_ssl_context *ssl, + unsigned char *buf, + const unsigned char *end, + size_t *olen ) { unsigned char *p = buf; - const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_OUT_CONTENT_LEN; *olen = 0; if( ssl->conf->extended_ms == MBEDTLS_SSL_EXTENDED_MS_DISABLED || ssl->conf->max_minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ) - { - return; - } + return( 0 ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding extended_master_secret extension" ) ); - if( end < p || (size_t)( end - p ) < 4 ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); - return; - } + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 4 ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_EXTENDED_MASTER_SECRET >> 8 ) & 0xFF ); @@ -572,32 +541,30 @@ static void ssl_write_extended_ms_ext( mbedtls_ssl_context *ssl, *p++ = 0x00; *olen = 4; + + return( 0 ); } #endif /* MBEDTLS_SSL_EXTENDED_MASTER_SECRET */ #if defined(MBEDTLS_SSL_SESSION_TICKETS) -static void ssl_write_session_ticket_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, size_t *olen ) +static int ssl_write_session_ticket_ext( mbedtls_ssl_context *ssl, + unsigned char *buf, + const unsigned char *end, + size_t *olen ) { unsigned char *p = buf; - const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_OUT_CONTENT_LEN; size_t tlen = ssl->session_negotiate->ticket_len; *olen = 0; if( ssl->conf->session_tickets == MBEDTLS_SSL_SESSION_TICKETS_DISABLED ) - { - return; - } + return( 0 ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding session ticket extension" ) ); - if( end < p || (size_t)( end - p ) < 4 + tlen ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); - return; - } + /* The addition is safe here since the ticket length is 16 bit. */ + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 4 + tlen ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SESSION_TICKET >> 8 ) & 0xFF ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_SESSION_TICKET ) & 0xFF ); @@ -608,9 +575,7 @@ static void ssl_write_session_ticket_ext( mbedtls_ssl_context *ssl, *olen = 4; if( ssl->session_negotiate->ticket == NULL || tlen == 0 ) - { - return; - } + return( 0 ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "sending session ticket of length %d", tlen ) ); @@ -618,35 +583,32 @@ static void ssl_write_session_ticket_ext( mbedtls_ssl_context *ssl, memcpy( p, ssl->session_negotiate->ticket, tlen ); *olen += tlen; + + return( 0 ); } #endif /* MBEDTLS_SSL_SESSION_TICKETS */ #if defined(MBEDTLS_SSL_ALPN) -static void ssl_write_alpn_ext( mbedtls_ssl_context *ssl, - unsigned char *buf, size_t *olen ) +static int ssl_write_alpn_ext( mbedtls_ssl_context *ssl, + unsigned char *buf, + const unsigned char *end, + size_t *olen ) { unsigned char *p = buf; - const unsigned char *end = ssl->out_msg + MBEDTLS_SSL_OUT_CONTENT_LEN; size_t alpnlen = 0; const char **cur; *olen = 0; if( ssl->conf->alpn_list == NULL ) - { - return; - } + return( 0 ); MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding alpn extension" ) ); for( cur = ssl->conf->alpn_list; *cur != NULL; cur++ ) alpnlen += (unsigned char)( strlen( *cur ) & 0xFF ) + 1; - if( end < p || (size_t)( end - p ) < 6 + alpnlen ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small" ) ); - return; - } + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 6 + alpnlen ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_ALPN >> 8 ) & 0xFF ); *p++ = (unsigned char)( ( MBEDTLS_TLS_EXT_ALPN ) & 0xFF ); @@ -678,6 +640,8 @@ static void ssl_write_alpn_ext( mbedtls_ssl_context *ssl, /* Extension length = olen - 2 (ext_type) - 2 (ext_len) */ buf[2] = (unsigned char)( ( ( *olen - 4 ) >> 8 ) & 0xFF ); buf[3] = (unsigned char)( ( ( *olen - 4 ) ) & 0xFF ); + + return( 0 ); } #endif /* MBEDTLS_SSL_ALPN */ @@ -772,8 +736,11 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) { int ret; size_t i, n, olen, ext_len = 0; + unsigned char *buf; unsigned char *p, *q; + const unsigned char *end; + unsigned char offer_compress; const int *ciphersuites; const mbedtls_ssl_ciphersuite_t *ciphersuite_info; @@ -805,16 +772,33 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); } + buf = ssl->out_msg; + end = buf + MBEDTLS_SSL_OUT_CONTENT_LEN; + /* - * 0 . 0 handshake type - * 1 . 3 handshake length + * Check if there's enough space for the first part of the ClientHello + * consisting of the 38 bytes described below, the session identifier (at + * most 32 bytes) and its length (1 byte). + * + * Use static upper bounds instead of the actual values + * to allow the compiler to optimize this away. + */ + MBEDTLS_SSL_CHK_BUF_PTR( buf, end, 38 + 1 + 32 ); + + /* + * The 38 first bytes of the ClientHello: + * 0 . 0 handshake type (written later) + * 1 . 3 handshake length (written later) * 4 . 5 highest version supported * 6 . 9 current UNIX time * 10 . 37 random bytes + * + * The current UNIX time (4 bytes) and following 28 random bytes are written + * by ssl_generate_random() into ssl->handshake->randbytes buffer and then + * copied from there into the output buffer. */ - buf = ssl->out_msg; - p = buf + 4; + p = buf + 4; mbedtls_ssl_write_version( ssl->conf->max_major_ver, ssl->conf->max_minor_ver, ssl->conf->transport, p ); @@ -837,7 +821,7 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) * 38 . 38 session id length * 39 . 39+n session id * 39+n . 39+n DTLS only: cookie length (1 byte) - * 40+n . .. DTSL only: cookie + * 40+n . .. DTLS only: cookie * .. . .. ciphersuitelist length (2 bytes) * .. . .. ciphersuitelist * .. . .. compression methods length (1 byte) @@ -879,6 +863,12 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) } #endif /* MBEDTLS_SSL_SESSION_TICKETS */ + /* + * The first check of the output buffer size above ( + * MBEDTLS_SSL_CHK_BUF_PTR( buf, end, 38 + 1 + 32 );) + * has checked that there is enough space in the output buffer for the + * session identifier length byte and the session identifier (n <= 32). + */ *p++ = (unsigned char) n; for( i = 0; i < n; i++ ) @@ -887,12 +877,27 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, session id len.: %d", n ) ); MBEDTLS_SSL_DEBUG_BUF( 3, "client hello, session id", buf + 39, n ); + /* + * With 'n' being the length of the session identifier + * + * 39+n . 39+n DTLS only: cookie length (1 byte) + * 40+n . .. DTLS only: cookie + * .. . .. ciphersuitelist length (2 bytes) + * .. . .. ciphersuitelist + * .. . .. compression methods length (1 byte) + * .. . .. compression methods + * .. . .. extensions length (2 bytes) + * .. . .. extensions + */ + /* * DTLS cookie */ #if defined(MBEDTLS_SSL_PROTO_DTLS) if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) { + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 1 ); + if( ssl->handshake->verify_cookie == NULL ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "no verify cookie to send" ) ); @@ -905,6 +910,9 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) ssl->handshake->verify_cookie_len ); *p++ = ssl->handshake->verify_cookie_len; + + MBEDTLS_SSL_CHK_BUF_PTR( p, end, + ssl->handshake->verify_cookie_len ); memcpy( p, ssl->handshake->verify_cookie, ssl->handshake->verify_cookie_len ); p += ssl->handshake->verify_cookie_len; @@ -920,6 +928,8 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) /* Skip writing ciphersuite length for now */ n = 0; q = p; + + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); p += 2; for( i = 0; ciphersuites[i] != 0; i++ ) @@ -939,6 +949,8 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) uses_ec |= mbedtls_ssl_ciphersuite_uses_ec( ciphersuite_info ); #endif + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); + n++; *p++ = (unsigned char)( ciphersuites[i] >> 8 ); *p++ = (unsigned char)( ciphersuites[i] ); @@ -955,6 +967,7 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) #endif { MBEDTLS_SSL_DEBUG_MSG( 3, ( "adding EMPTY_RENEGOTIATION_INFO_SCSV" ) ); + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); *p++ = (unsigned char)( MBEDTLS_SSL_EMPTY_RENEGOTIATION_INFO >> 8 ); *p++ = (unsigned char)( MBEDTLS_SSL_EMPTY_RENEGOTIATION_INFO ); n++; @@ -965,6 +978,8 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) if( ssl->conf->fallback == MBEDTLS_SSL_IS_FALLBACK ) { MBEDTLS_SSL_DEBUG_MSG( 3, ( "adding FALLBACK_SCSV" ) ); + + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); *p++ = (unsigned char)( MBEDTLS_SSL_FALLBACK_SCSV_VALUE >> 8 ); *p++ = (unsigned char)( MBEDTLS_SSL_FALLBACK_SCSV_VALUE ); n++; @@ -998,6 +1013,7 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_COMPRESS_DEFLATE, MBEDTLS_SSL_COMPRESS_NULL ) ); + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 3 ); *p++ = 2; *p++ = MBEDTLS_SSL_COMPRESS_DEFLATE; *p++ = MBEDTLS_SSL_COMPRESS_NULL; @@ -1008,27 +1024,45 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, compress alg.: %d", MBEDTLS_SSL_COMPRESS_NULL ) ); + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); *p++ = 1; *p++ = MBEDTLS_SSL_COMPRESS_NULL; } - // First write extensions, then the total length - // + /* First write extensions, then the total length */ + + MBEDTLS_SSL_CHK_BUF_PTR( p, end, 2 ); + #if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION) - ssl_write_hostname_ext( ssl, p + 2 + ext_len, &olen ); + if( ( ret = ssl_write_hostname_ext( ssl, p + 2 + ext_len, + end, &olen ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_hostname_ext", ret ); + return( ret ); + } ext_len += olen; #endif /* Note that TLS_EMPTY_RENEGOTIATION_INFO_SCSV is always added * even if MBEDTLS_SSL_RENEGOTIATION is not defined. */ #if defined(MBEDTLS_SSL_RENEGOTIATION) - ssl_write_renegotiation_ext( ssl, p + 2 + ext_len, &olen ); + if( ( ret = ssl_write_renegotiation_ext( ssl, p + 2 + ext_len, + end, &olen ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_renegotiation_ext", ret ); + return( ret ); + } ext_len += olen; #endif #if defined(MBEDTLS_SSL_PROTO_TLS1_2) && \ defined(MBEDTLS_KEY_EXCHANGE__WITH_CERT__ENABLED) - ssl_write_signature_algorithms_ext( ssl, p + 2 + ext_len, &olen ); + if( ( ret = ssl_write_signature_algorithms_ext( ssl, p + 2 + ext_len, + end, &olen ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_signature_algorithms_ext", ret ); + return( ret ); + } ext_len += olen; #endif @@ -1036,46 +1070,91 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) if( uses_ec ) { - ssl_write_supported_elliptic_curves_ext( ssl, p + 2 + ext_len, &olen ); + if( ( ret = ssl_write_supported_elliptic_curves_ext( ssl, p + 2 + ext_len, + end, &olen ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_supported_elliptic_curves_ext", ret ); + return( ret ); + } ext_len += olen; - ssl_write_supported_point_formats_ext( ssl, p + 2 + ext_len, &olen ); + if( ( ret = ssl_write_supported_point_formats_ext( ssl, p + 2 + ext_len, + end, &olen ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_supported_point_formats_ext", ret ); + return( ret ); + } ext_len += olen; } #endif #if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) - ssl_write_ecjpake_kkpp_ext( ssl, p + 2 + ext_len, &olen ); + if( ( ret = ssl_write_ecjpake_kkpp_ext( ssl, p + 2 + ext_len, + end, &olen ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_ecjpake_kkpp_ext", ret ); + return( ret ); + } ext_len += olen; #endif #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) - ssl_write_max_fragment_length_ext( ssl, p + 2 + ext_len, &olen ); + if( ( ret = ssl_write_max_fragment_length_ext( ssl, p + 2 + ext_len, + end, &olen ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_max_fragment_length_ext", ret ); + return( ret ); + } ext_len += olen; #endif #if defined(MBEDTLS_SSL_TRUNCATED_HMAC) - ssl_write_truncated_hmac_ext( ssl, p + 2 + ext_len, &olen ); + if( ( ret = ssl_write_truncated_hmac_ext( ssl, p + 2 + ext_len, + end, &olen ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_truncated_hmac_ext", ret ); + return( ret ); + } ext_len += olen; #endif #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) - ssl_write_encrypt_then_mac_ext( ssl, p + 2 + ext_len, &olen ); + if( ( ret = ssl_write_encrypt_then_mac_ext( ssl, p + 2 + ext_len, + end, &olen ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_encrypt_then_mac_ext", ret ); + return( ret ); + } ext_len += olen; #endif #if defined(MBEDTLS_SSL_EXTENDED_MASTER_SECRET) - ssl_write_extended_ms_ext( ssl, p + 2 + ext_len, &olen ); + if( ( ret = ssl_write_extended_ms_ext( ssl, p + 2 + ext_len, + end, &olen ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_extended_ms_ext", ret ); + return( ret ); + } ext_len += olen; #endif #if defined(MBEDTLS_SSL_ALPN) - ssl_write_alpn_ext( ssl, p + 2 + ext_len, &olen ); + if( ( ret = ssl_write_alpn_ext( ssl, p + 2 + ext_len, + end, &olen ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_alpn_ext", ret ); + return( ret ); + } ext_len += olen; #endif #if defined(MBEDTLS_SSL_SESSION_TICKETS) - ssl_write_session_ticket_ext( ssl, p + 2 + ext_len, &olen ); + if( ( ret = ssl_write_session_ticket_ext( ssl, p + 2 + ext_len, + end, &olen ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_session_ticket_ext", ret ); + return( ret ); + } ext_len += olen; #endif @@ -1083,10 +1162,12 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) ((void) olen); MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, total extension length: %d", - ext_len ) ); + ext_len ) ); if( ext_len > 0 ) { + /* No need to check for space here, because the extension + * writing functions already took care of that. */ *p++ = (unsigned char)( ( ext_len >> 8 ) & 0xFF ); *p++ = (unsigned char)( ( ext_len ) & 0xFF ); p += ext_len; diff --git a/library/ssl_cookie.c b/library/ssl_cookie.c index 56e9bdd2b..d87e5eef4 100644 --- a/library/ssl_cookie.c +++ b/library/ssl_cookie.c @@ -133,8 +133,7 @@ static int ssl_cookie_hmac( mbedtls_md_context_t *hmac_ctx, { unsigned char hmac_out[COOKIE_MD_OUTLEN]; - if( (size_t)( end - *p ) < COOKIE_HMAC_LEN ) - return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); + MBEDTLS_SSL_CHK_BUF_PTR( *p, end, COOKIE_HMAC_LEN ); if( mbedtls_md_hmac_reset( hmac_ctx ) != 0 || mbedtls_md_hmac_update( hmac_ctx, time, 4 ) != 0 || @@ -164,8 +163,7 @@ int mbedtls_ssl_cookie_write( void *p_ctx, if( ctx == NULL || cli_id == NULL ) return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); - if( (size_t)( end - *p ) < COOKIE_LEN ) - return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); + MBEDTLS_SSL_CHK_BUF_PTR( *p, end, COOKIE_LEN ); #if defined(MBEDTLS_HAVE_TIME) t = (unsigned long) mbedtls_time( NULL ); diff --git a/library/ssl_ticket.c b/library/ssl_ticket.c index 25eab49d2..332396707 100644 --- a/library/ssl_ticket.c +++ b/library/ssl_ticket.c @@ -35,6 +35,7 @@ #define mbedtls_free free #endif +#include "mbedtls/ssl_internal.h" #include "mbedtls/ssl_ticket.h" #include "mbedtls/platform_util.h" @@ -316,8 +317,7 @@ int mbedtls_ssl_ticket_write( void *p_ticket, /* We need at least 4 bytes for key_name, 12 for IV, 2 for len 16 for tag, * in addition to session itself, that will be checked when writing it. */ - if( end - start < TICKET_MIN_LEN ) - return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); + MBEDTLS_SSL_CHK_BUF_PTR( start, end, TICKET_MIN_LEN ); #if defined(MBEDTLS_THREADING_C) if( ( ret = mbedtls_mutex_lock( &ctx->mutex ) ) != 0 ) From d7296020a6e3e501614e0d9ce4f75c45d260d124 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 12 Apr 2017 14:54:42 +0100 Subject: [PATCH 06/10] Add error condition for bad user configurations This commit adds an error condition for bad user configurations and updates the number of SSL module errors in error.h. Signed-off-by: Ronald Cron --- include/mbedtls/error.h | 1 + include/mbedtls/ssl.h | 1 + library/error.c | 2 ++ 3 files changed, 4 insertions(+) diff --git a/include/mbedtls/error.h b/include/mbedtls/error.h index bee0fe485..aa1176a75 100644 --- a/include/mbedtls/error.h +++ b/include/mbedtls/error.h @@ -100,6 +100,7 @@ * ECP 4 10 (Started from top) * MD 5 5 * HKDF 5 1 (Started from top) + * SSL 5 1 (Started from 0x5E80) * CIPHER 6 8 * SSL 6 23 (Started from top) * SSL 7 32 diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 1adf9608c..f5cd97714 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -123,6 +123,7 @@ #define MBEDTLS_ERR_SSL_ASYNC_IN_PROGRESS -0x6500 /**< The asynchronous operation is not completed yet. */ #define MBEDTLS_ERR_SSL_EARLY_MESSAGE -0x6480 /**< Internal-only message signaling that a message arrived early. */ #define MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS -0x7000 /**< A cryptographic operation is in progress. Try again later. */ +#define MBEDTLS_ERR_SSL_BAD_CONFIG -0x5E80 /**< Invalid value in SSL config */ /* * Various constants diff --git a/library/error.c b/library/error.c index 04f425bb3..8aef23773 100644 --- a/library/error.c +++ b/library/error.c @@ -529,6 +529,8 @@ void mbedtls_strerror( int ret, char *buf, size_t buflen ) mbedtls_snprintf( buf, buflen, "SSL - Internal-only message signaling that a message arrived early" ); if( use_ret == -(MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS) ) mbedtls_snprintf( buf, buflen, "SSL - A cryptographic operation is in progress. Try again later" ); + if( use_ret == -(MBEDTLS_ERR_SSL_BAD_CONFIG) ) + mbedtls_snprintf( buf, buflen, "SSL - Invalid value in SSL config" ); #endif /* MBEDTLS_SSL_TLS_C */ #if defined(MBEDTLS_X509_USE_C) || defined(MBEDTLS_X509_CREATE_C) From f250380df375c46ad2a9304c917b5be405a9302d Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 12 Apr 2017 14:54:42 +0100 Subject: [PATCH 07/10] Return error in case of bad user configurations This commits adds returns with the SSL_BAD_CONFIG error code in case of bad user configurations. Signed-off-by: Ronald Cron --- include/mbedtls/ssl_internal.h | 6 ++++++ library/ssl_cli.c | 36 ++++++++++++++++++++++++++++++---- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 82beea553..daf3f6a2d 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -182,6 +182,12 @@ : ( MBEDTLS_SSL_IN_CONTENT_LEN ) \ ) +/* Maximum size in bytes of list in sig-hash algorithm ext., RFC 5246 */ +#define MBEDTLS_SSL_MAX_SIG_HASH_ALG_LIST_LEN 65534 + +/* Maximum size in bytes of list in supported elliptic curve ext., RFC 4492 */ +#define MBEDTLS_SSL_MAX_CURVE_LIST_LEN 65535 + /* * Check that we obey the standard's message size bounds */ diff --git a/library/ssl_cli.c b/library/ssl_cli.c index c8fb0d5dc..16b338cbf 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -187,6 +187,9 @@ static int ssl_write_signature_algorithms_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding signature_algorithms extension" ) ); + if( ssl->conf->sig_hashes == NULL ) + return( MBEDTLS_ERR_SSL_BAD_CONFIG ); + for( md = ssl->conf->sig_hashes; *md != MBEDTLS_MD_NONE; md++ ) { #if defined(MBEDTLS_ECDSA_C) @@ -195,8 +198,18 @@ static int ssl_write_signature_algorithms_ext( mbedtls_ssl_context *ssl, #if defined(MBEDTLS_RSA_C) sig_alg_len += 2; #endif + if( sig_alg_len > MBEDTLS_SSL_MAX_SIG_HASH_ALG_LIST_LEN ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "length in bytes of sig-hash-alg extension too big" ) ); + return( MBEDTLS_ERR_SSL_BAD_CONFIG ); + } } + /* Empty signature algorithms list, this is a configuration error. */ + if( sig_alg_len == 0 ) + return( MBEDTLS_ERR_SSL_BAD_CONFIG ); + MBEDTLS_SSL_CHK_BUF_PTR( p, end, sig_alg_len + 6 ); /* @@ -267,6 +280,9 @@ static int ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding supported_elliptic_curves extension" ) ); + if( ssl->conf->curve_list == NULL ) + return( MBEDTLS_ERR_SSL_BAD_CONFIG ); + for( grp_id = ssl->conf->curve_list; *grp_id != MBEDTLS_ECP_DP_NONE; grp_id++ ) @@ -276,13 +292,21 @@ static int ssl_write_supported_elliptic_curves_ext( mbedtls_ssl_context *ssl, { MBEDTLS_SSL_DEBUG_MSG( 1, ( "invalid curve in ssl configuration" ) ); - return( 0 ); + return( MBEDTLS_ERR_SSL_BAD_CONFIG ); } elliptic_curve_len += 2; + + if( elliptic_curve_len > MBEDTLS_SSL_MAX_CURVE_LIST_LEN ) + { + MBEDTLS_SSL_DEBUG_MSG( 3, + ( "malformed supported_elliptic_curves extension in config" ) ); + return( MBEDTLS_ERR_SSL_BAD_CONFIG ); + } } + /* Empty elliptic curve list, this is a configuration error. */ if( elliptic_curve_len == 0 ) - return( 0 ); + return( MBEDTLS_ERR_SSL_BAD_CONFIG ); MBEDTLS_SSL_CHK_BUF_PTR( p, end, 6 + elliptic_curve_len ); @@ -606,7 +630,7 @@ static int ssl_write_alpn_ext( mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG( 3, ( "client hello, adding alpn extension" ) ); for( cur = ssl->conf->alpn_list; *cur != NULL; cur++ ) - alpnlen += (unsigned char)( strlen( *cur ) & 0xFF ) + 1; + alpnlen += strlen( *cur ) + 1; MBEDTLS_SSL_CHK_BUF_PTR( p, end, 6 + alpnlen ); @@ -626,7 +650,11 @@ static int ssl_write_alpn_ext( mbedtls_ssl_context *ssl, for( cur = ssl->conf->alpn_list; *cur != NULL; cur++ ) { - *p = (unsigned char)( strlen( *cur ) & 0xFF ); + /* + * mbedtls_ssl_conf_set_alpn_protocols() checked that the length of + * protocol names is less than 255. + */ + *p = (unsigned char)strlen( *cur ); memcpy( p + 1, *cur, *p ); p += 1 + *p; } From 157cffebab106547939fb799bd5558d4747ec931 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 23 Apr 2020 16:41:44 +0200 Subject: [PATCH 08/10] Use defines to check alpn ext list validity Signed-off-by: Ronald Cron --- include/mbedtls/ssl.h | 3 +++ library/ssl_tls.c | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index f5cd97714..7f742ad4c 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -138,6 +138,9 @@ #define MBEDTLS_SSL_TRANSPORT_DATAGRAM 1 /*!< DTLS */ #define MBEDTLS_SSL_MAX_HOST_NAME_LEN 255 /*!< Maximum host name defined in RFC 1035 */ +#define MBEDTLS_SSL_MAX_ALPN_NAME_LEN 255 /*!< Maximum size in bytes of a protocol name in alpn ext., RFC 7301 */ + +#define MBEDTLS_SSL_MAX_ALPN_LIST_LEN 65535 /*!< Maximum size in bytes of list in alpn ext., RFC 7301 */ /* RFC 6066 section 4, see also mfl_code_to_length in ssl_tls.c * NONE must be zero so that memset()ing structure to zero works */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index cbec74fe8..9bc4fa81b 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7589,7 +7589,9 @@ int mbedtls_ssl_conf_alpn_protocols( mbedtls_ssl_config *conf, const char **prot cur_len = strlen( *p ); tot_len += cur_len; - if( cur_len == 0 || cur_len > 255 || tot_len > 65535 ) + if( ( cur_len == 0 ) || + ( cur_len > MBEDTLS_SSL_MAX_ALPN_NAME_LEN ) || + ( tot_len > MBEDTLS_SSL_MAX_ALPN_LIST_LEN ) ) return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); } From 32b629dc993bfd4820f85acf490d5ce05ba1dfce Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 11 Jun 2020 09:34:06 +0200 Subject: [PATCH 09/10] ssl_client: Align line breaking with MBEDTLS_SSL_DEBUG_* Signed-off-by: Ronald Cron --- library/ssl_cli.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 16b338cbf..9bbbaeb65 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1260,8 +1260,8 @@ static int ssl_parse_renegotiation_info( mbedtls_ssl_context *ssl, { if( len != 1 || buf[0] != 0x00 ) { - MBEDTLS_SSL_DEBUG_MSG( - 1, ( "non-zero length renegotiation info" ) ); + MBEDTLS_SSL_DEBUG_MSG( 1, + ( "non-zero length renegotiation info" ) ); mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL, @@ -3357,8 +3357,8 @@ ecdh_calc_secret: if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl, ciphersuite_info->key_exchange ) ) != 0 ) { - MBEDTLS_SSL_DEBUG_RET( - 1, "mbedtls_ssl_psk_derive_premaster", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, + "mbedtls_ssl_psk_derive_premaster", ret ); return( ret ); } } From 9581fa30503b6416bd3ef01813a9041bb1341f9c Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Thu, 11 Jun 2020 09:50:51 +0200 Subject: [PATCH 10/10] Align with check-like function return value convention By convention, in the project, functions that have a check or similar in the name return 0 if the check succeeds, non-zero otherwise. Align with this for mbedtls_ssl_chk_buf_ptr(). Signed-off-by: Ronald Cron --- include/mbedtls/ssl_internal.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index daf3f6a2d..e6d948ee9 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -250,13 +250,13 @@ * \param end Pointer to one past the end of the buffer. * \param need Needed space in bytes. * - * \return Non-zero if the needed space is available in the buffer, 0 + * \return Zero if the needed space is available in the buffer, non-zero * otherwise. */ static inline int mbedtls_ssl_chk_buf_ptr( const uint8_t *cur, const uint8_t *end, size_t need ) { - return( cur <= end && need <= (size_t)( end - cur ) ); + return( ( cur > end ) || ( need > (size_t)( end - cur ) ) ); } /** @@ -271,7 +271,7 @@ static inline int mbedtls_ssl_chk_buf_ptr( const uint8_t *cur, */ #define MBEDTLS_SSL_CHK_BUF_PTR( cur, end, need ) \ do { \ - if( mbedtls_ssl_chk_buf_ptr( ( cur ), ( end ), ( need ) ) == 0 ) \ + if( mbedtls_ssl_chk_buf_ptr( ( cur ), ( end ), ( need ) ) != 0 ) \ { \ return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); \ } \