From 46cccb8f39bdebef6b7067052dfea550e7461309 Mon Sep 17 00:00:00 2001 From: TRodziewicz Date: Tue, 11 May 2021 13:13:51 +0200 Subject: [PATCH 1/3] _SSL_DTLS_BADMAC_LIMIT config.h option removed Signed-off-by: TRodziewicz --- include/mbedtls/check_config.h | 5 ----- include/mbedtls/config.h | 11 ----------- include/mbedtls/ssl.h | 7 ------- library/ssl_msg.c | 2 -- library/ssl_tls.c | 10 ---------- programs/ssl/ssl_context_info.c | 1 - programs/ssl/ssl_server2.c | 6 ------ tests/context-info.sh | 4 ---- 8 files changed, 46 deletions(-) diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h index 389ae2a71..ae23602a0 100644 --- a/include/mbedtls/check_config.h +++ b/include/mbedtls/check_config.h @@ -700,11 +700,6 @@ #error "MBEDTLS_SSL_CID_OUT_LEN_MAX too large (max 255)" #endif -#if defined(MBEDTLS_SSL_DTLS_BADMAC_LIMIT) && \ - ( !defined(MBEDTLS_SSL_TLS_C) || !defined(MBEDTLS_SSL_PROTO_DTLS) ) -#error "MBEDTLS_SSL_DTLS_BADMAC_LIMIT defined, but not all prerequisites" -#endif - #if defined(MBEDTLS_SSL_ENCRYPT_THEN_MAC) && \ !defined(MBEDTLS_SSL_PROTO_TLS1_2) #error "MBEDTLS_SSL_ENCRYPT_THEN_MAC defined, but not all prerequsites" diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 715c73ada..10140915e 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -1748,17 +1748,6 @@ */ #define MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE -/** - * \def MBEDTLS_SSL_DTLS_BADMAC_LIMIT - * - * Enable support for a limit of records with bad MAC. - * - * See mbedtls_ssl_conf_dtls_badmac_limit(). - * - * Requires: MBEDTLS_SSL_PROTO_DTLS - */ -#define MBEDTLS_SSL_DTLS_BADMAC_LIMIT - /** * \def MBEDTLS_SSL_SESSION_TICKETS * diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index bdc2b4797..6a908f271 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1151,9 +1151,7 @@ struct mbedtls_ssl_config that triggers renegotiation */ #endif -#if defined(MBEDTLS_SSL_DTLS_BADMAC_LIMIT) unsigned int badmac_limit; /*!< limit of records with a bad MAC */ -#endif #if defined(MBEDTLS_DHM_C) && defined(MBEDTLS_SSL_CLI_C) unsigned int dhm_min_bitlen; /*!< min. bit length of the DHM prime */ @@ -1226,10 +1224,7 @@ struct mbedtls_ssl_context int major_ver; /*!< equal to MBEDTLS_SSL_MAJOR_VERSION_3 */ int minor_ver; /*!< one of MBEDTLS_SSL_MINOR_VERSION_x macros */ - -#if defined(MBEDTLS_SSL_DTLS_BADMAC_LIMIT) unsigned badmac_seen; /*!< records with a bad MAC received */ -#endif /* MBEDTLS_SSL_DTLS_BADMAC_LIMIT */ #if defined(MBEDTLS_X509_CRT_PARSE_C) /** Callback to customize X.509 certificate chain verification */ @@ -2261,7 +2256,6 @@ int mbedtls_ssl_set_client_transport_id( mbedtls_ssl_context *ssl, void mbedtls_ssl_conf_dtls_anti_replay( mbedtls_ssl_config *conf, char mode ); #endif /* MBEDTLS_SSL_DTLS_ANTI_REPLAY */ -#if defined(MBEDTLS_SSL_DTLS_BADMAC_LIMIT) /** * \brief Set a limit on the number of records with a bad MAC * before terminating the connection. @@ -2286,7 +2280,6 @@ void mbedtls_ssl_conf_dtls_anti_replay( mbedtls_ssl_config *conf, char mode ); * many bogus packets. */ void mbedtls_ssl_conf_dtls_badmac_limit( mbedtls_ssl_config *conf, unsigned limit ); -#endif /* MBEDTLS_SSL_DTLS_BADMAC_LIMIT */ #if defined(MBEDTLS_SSL_PROTO_DTLS) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index a75b9190b..ff628b9f4 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -4499,14 +4499,12 @@ static int ssl_get_next_record( mbedtls_ssl_context *ssl ) return( ret ); } -#if defined(MBEDTLS_SSL_DTLS_BADMAC_LIMIT) if( ssl->conf->badmac_limit != 0 && ++ssl->badmac_seen >= ssl->conf->badmac_limit ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "too many records with bad MAC" ) ); return( MBEDTLS_ERR_SSL_INVALID_MAC ); } -#endif /* As above, invalid records cause * dismissal of the whole datagram. */ diff --git a/library/ssl_tls.c b/library/ssl_tls.c index e60c0726f..a7e5b4cd8 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3390,12 +3390,10 @@ void mbedtls_ssl_conf_dtls_anti_replay( mbedtls_ssl_config *conf, char mode ) } #endif -#if defined(MBEDTLS_SSL_DTLS_BADMAC_LIMIT) void mbedtls_ssl_conf_dtls_badmac_limit( mbedtls_ssl_config *conf, unsigned limit ) { conf->badmac_limit = limit; } -#endif #if defined(MBEDTLS_SSL_PROTO_DTLS) @@ -5418,11 +5416,7 @@ void mbedtls_ssl_session_free( mbedtls_ssl_session *session ) #define SSL_SERIALIZED_CONTEXT_CONFIG_DTLS_CONNECTION_ID 0u #endif /* MBEDTLS_SSL_DTLS_CONNECTION_ID */ -#if defined(MBEDTLS_SSL_DTLS_BADMAC_LIMIT) #define SSL_SERIALIZED_CONTEXT_CONFIG_DTLS_BADMAC_LIMIT 1u -#else -#define SSL_SERIALIZED_CONTEXT_CONFIG_DTLS_BADMAC_LIMIT 0u -#endif /* MBEDTLS_SSL_DTLS_BADMAC_LIMIT */ #if defined(MBEDTLS_SSL_DTLS_ANTI_REPLAY) #define SSL_SERIALIZED_CONTEXT_CONFIG_DTLS_ANTI_REPLAY 1u @@ -5639,7 +5633,6 @@ int mbedtls_ssl_context_save( mbedtls_ssl_context *ssl, /* * Saved fields from top-level ssl_context structure */ -#if defined(MBEDTLS_SSL_DTLS_BADMAC_LIMIT) used += 4; if( used <= buf_len ) { @@ -5648,7 +5641,6 @@ int mbedtls_ssl_context_save( mbedtls_ssl_context *ssl, *p++ = (unsigned char)( ( ssl->badmac_seen >> 8 ) & 0xFF ); *p++ = (unsigned char)( ( ssl->badmac_seen ) & 0xFF ); } -#endif /* MBEDTLS_SSL_DTLS_BADMAC_LIMIT */ #if defined(MBEDTLS_SSL_DTLS_ANTI_REPLAY) used += 16; @@ -5904,7 +5896,6 @@ static int ssl_context_load( mbedtls_ssl_context *ssl, /* * Saved fields from top-level ssl_context structure */ -#if defined(MBEDTLS_SSL_DTLS_BADMAC_LIMIT) if( (size_t)( end - p ) < 4 ) return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); @@ -5913,7 +5904,6 @@ static int ssl_context_load( mbedtls_ssl_context *ssl, ( (uint32_t) p[2] << 8 ) | ( (uint32_t) p[3] ); p += 4; -#endif /* MBEDTLS_SSL_DTLS_BADMAC_LIMIT */ #if defined(MBEDTLS_SSL_DTLS_ANTI_REPLAY) if( (size_t)( end - p ) < 16 ) diff --git a/programs/ssl/ssl_context_info.c b/programs/ssl/ssl_context_info.c index ec24fa8e0..855102de7 100644 --- a/programs/ssl/ssl_context_info.c +++ b/programs/ssl/ssl_context_info.c @@ -872,7 +872,6 @@ void print_deserialized_ssl_context( const uint8_t *ssl, size_t len ) print_if_bit( "MBEDTLS_SSL_SESSION_TICKETS and client", SESSION_CONFIG_CLIENT_TICKET_BIT, session_cfg_flag ); print_if_bit( "MBEDTLS_SSL_DTLS_CONNECTION_ID", CONTEXT_CONFIG_DTLS_CONNECTION_ID_BIT, context_cfg_flag ); - print_if_bit( "MBEDTLS_SSL_DTLS_BADMAC_LIMIT", CONTEXT_CONFIG_DTLS_BADMAC_LIMIT_BIT, context_cfg_flag ); print_if_bit( "MBEDTLS_SSL_DTLS_ANTI_REPLAY", CONTEXT_CONFIG_DTLS_ANTI_REPLAY_BIT, context_cfg_flag ); print_if_bit( "MBEDTLS_SSL_ALPN", CONTEXT_CONFIG_ALPN_BIT, context_cfg_flag ); diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 2cf2d73be..ef55a7c25 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -363,12 +363,8 @@ int main( void ) #define USAGE_ANTI_REPLAY "" #endif -#if defined(MBEDTLS_SSL_DTLS_BADMAC_LIMIT) #define USAGE_BADMAC_LIMIT \ " badmac_limit=%%d default: (library default: disabled)\n" -#else -#define USAGE_BADMAC_LIMIT "" -#endif #if defined(MBEDTLS_SSL_PROTO_DTLS) #define USAGE_DTLS \ @@ -2685,10 +2681,8 @@ int main( int argc, char *argv[] ) mbedtls_ssl_conf_dtls_anti_replay( &conf, opt.anti_replay ); #endif -#if defined(MBEDTLS_SSL_DTLS_BADMAC_LIMIT) if( opt.badmac_limit != DFL_BADMAC_LIMIT ) mbedtls_ssl_conf_dtls_badmac_limit( &conf, opt.badmac_limit ); -#endif } #endif /* MBEDTLS_SSL_PROTO_DTLS */ diff --git a/tests/context-info.sh b/tests/context-info.sh index 346529845..e02d33084 100755 --- a/tests/context-info.sh +++ b/tests/context-info.sh @@ -214,7 +214,6 @@ run_test "Default configuration, server" \ -u "MBEDTLS_SSL_ENCRYPT_THEN_MAC$" \ -u "MBEDTLS_SSL_SESSION_TICKETS$" \ -u "MBEDTLS_SSL_SESSION_TICKETS and client$" \ - -u "MBEDTLS_SSL_DTLS_BADMAC_LIMIT$" \ -u "MBEDTLS_SSL_DTLS_ANTI_REPLAY$" \ -u "MBEDTLS_SSL_ALPN$" \ -u "ciphersuite.* TLS-ECDHE-RSA-WITH-CHACHA20-POLY1305-SHA256$" \ @@ -238,7 +237,6 @@ run_test "Default configuration, client" \ -u "MBEDTLS_SSL_ENCRYPT_THEN_MAC$" \ -u "MBEDTLS_SSL_SESSION_TICKETS$" \ -u "MBEDTLS_SSL_SESSION_TICKETS and client$" \ - -u "MBEDTLS_SSL_DTLS_BADMAC_LIMIT$" \ -u "MBEDTLS_SSL_DTLS_ANTI_REPLAY$" \ -u "MBEDTLS_SSL_ALPN$" \ -u "ciphersuite.* TLS-ECDHE-RSA-WITH-CHACHA20-POLY1305-SHA256$" \ @@ -345,7 +343,6 @@ run_test "Minimal configuration, server" \ -n "MBEDTLS_SSL_ENCRYPT_THEN_MAC$" \ -n "MBEDTLS_SSL_SESSION_TICKETS$" \ -n "MBEDTLS_SSL_SESSION_TICKETS and client$" \ - -n "MBEDTLS_SSL_DTLS_BADMAC_LIMIT$" \ -n "MBEDTLS_SSL_DTLS_ANTI_REPLAY$" \ -n "MBEDTLS_SSL_ALPN$" \ @@ -357,7 +354,6 @@ run_test "Minimal configuration, client" \ -n "MBEDTLS_SSL_ENCRYPT_THEN_MAC$" \ -n "MBEDTLS_SSL_SESSION_TICKETS$" \ -n "MBEDTLS_SSL_SESSION_TICKETS and client$" \ - -n "MBEDTLS_SSL_DTLS_BADMAC_LIMIT$" \ -n "MBEDTLS_SSL_DTLS_ANTI_REPLAY$" \ -n "MBEDTLS_SSL_ALPN$" \ From 5c251c6a5e2175cd0570471d543acd842698b7f8 Mon Sep 17 00:00:00 2001 From: TRodziewicz Date: Tue, 11 May 2021 14:27:28 +0200 Subject: [PATCH 2/3] Add the ChangeLog file Signed-off-by: TRodziewicz --- ChangeLog.d/issue4403.txt | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 ChangeLog.d/issue4403.txt diff --git a/ChangeLog.d/issue4403.txt b/ChangeLog.d/issue4403.txt new file mode 100644 index 000000000..08ac60e69 --- /dev/null +++ b/ChangeLog.d/issue4403.txt @@ -0,0 +1,2 @@ +Removals + * Remove the MBEDTLS_SSL_DTLS_BADMAC_LIMIT config.h option. Fixes #4403. From a86c312d9266f9c6d6be67fba8d059ae6aa1b2fb Mon Sep 17 00:00:00 2001 From: TRodziewicz Date: Wed, 26 May 2021 15:29:36 +0200 Subject: [PATCH 3/3] Addition of the migration guide entry. Signed-off-by: TRodziewicz --- .../remove_SSL_DTLS_BADMAC_LIMIT_option.md | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 docs/3.0-migration-guide.d/remove_SSL_DTLS_BADMAC_LIMIT_option.md diff --git a/docs/3.0-migration-guide.d/remove_SSL_DTLS_BADMAC_LIMIT_option.md b/docs/3.0-migration-guide.d/remove_SSL_DTLS_BADMAC_LIMIT_option.md new file mode 100644 index 000000000..f81bdadba --- /dev/null +++ b/docs/3.0-migration-guide.d/remove_SSL_DTLS_BADMAC_LIMIT_option.md @@ -0,0 +1,11 @@ +Remove MBEDTLS_SSL_DTLS_BADMAC_LIMIT option +------------------------------------------- + +This change does not affect users who used the default `config.h`, as the option +MBEDTLS_SSL_DTLS_BADMAC_LIMIT was already on by default. + +This option was a trade-off between functionality and code size: it allowed +users who didn't need that feature to avoid paying the cost in code size, by +disabling it. + +This option is no longer present, but its functionality is now always enabled.