From 31c1586893d975b139af191329eaafe19965506f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 13 Sep 2017 09:38:11 +0200 Subject: [PATCH 01/35] Start separating handshake from record writing --- include/mbedtls/ssl_internal.h | 1 + library/ssl_cli.c | 12 +++--- library/ssl_srv.c | 20 ++++----- library/ssl_tls.c | 76 +++++++++++++++++++++++++++------- 4 files changed, 79 insertions(+), 30 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index d214703d7..68b5f3033 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -559,6 +559,7 @@ void mbedtls_ssl_update_handshake_status( mbedtls_ssl_context *ssl ); int mbedtls_ssl_read_record( mbedtls_ssl_context *ssl ); int mbedtls_ssl_fetch_input( mbedtls_ssl_context *ssl, size_t nb_want ); +int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ); int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl ); int mbedtls_ssl_flush_output( mbedtls_ssl_context *ssl ); diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 321d6367a..253c81f73 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1088,9 +1088,9 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) mbedtls_ssl_send_flight_completed( ssl ); #endif - if( ( ret = mbedtls_ssl_write_record( ssl ) ) != 0 ) + if( ( ret = mbedtls_ssl_write_handshake_msg( ssl ) ) != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_record", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_handshake_msg", ret ); return( ret ); } @@ -3075,9 +3075,9 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) ssl->state++; - if( ( ret = mbedtls_ssl_write_record( ssl ) ) != 0 ) + if( ( ret = mbedtls_ssl_write_handshake_msg( ssl ) ) != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_record", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_handshake_msg", ret ); return( ret ); } @@ -3260,9 +3260,9 @@ static int ssl_write_certificate_verify( mbedtls_ssl_context *ssl ) ssl->state++; - if( ( ret = mbedtls_ssl_write_record( ssl ) ) != 0 ) + if( ( ret = mbedtls_ssl_write_handshake_msg( ssl ) ) != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_record", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_handshake_msg", ret ); return( ret ); } diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 2872f1fb0..66de2e46c 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -2384,9 +2384,9 @@ static int ssl_write_hello_verify_request( mbedtls_ssl_context *ssl ) ssl->state = MBEDTLS_SSL_SERVER_HELLO_VERIFY_REQUEST_SENT; - if( ( ret = mbedtls_ssl_write_record( ssl ) ) != 0 ) + if( ( ret = mbedtls_ssl_write_handshake_msg( ssl ) ) != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_record", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_handshake_msg", ret ); return( ret ); } @@ -2624,7 +2624,7 @@ static int ssl_write_server_hello( mbedtls_ssl_context *ssl ) ssl->out_msgtype = MBEDTLS_SSL_MSG_HANDSHAKE; ssl->out_msg[0] = MBEDTLS_SSL_HS_SERVER_HELLO; - ret = mbedtls_ssl_write_record( ssl ); + ret = mbedtls_ssl_write_handshake_msg( ssl ); MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write server hello" ) ); @@ -2819,7 +2819,7 @@ static int ssl_write_certificate_request( mbedtls_ssl_context *ssl ) ssl->out_msg[4 + ct_len + sa_len] = (unsigned char)( total_dn_size >> 8 ); ssl->out_msg[5 + ct_len + sa_len] = (unsigned char)( total_dn_size ); - ret = mbedtls_ssl_write_record( ssl ); + ret = mbedtls_ssl_write_handshake_msg( ssl ); MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write certificate request" ) ); @@ -3336,9 +3336,9 @@ static int ssl_write_server_key_exchange( mbedtls_ssl_context *ssl ) ssl->state++; - if( ( ret = mbedtls_ssl_write_record( ssl ) ) != 0 ) + if( ( ret = mbedtls_ssl_write_handshake_msg( ssl ) ) != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_record", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_handshake_msg", ret ); return( ret ); } @@ -3363,9 +3363,9 @@ static int ssl_write_server_hello_done( mbedtls_ssl_context *ssl ) mbedtls_ssl_send_flight_completed( ssl ); #endif - if( ( ret = mbedtls_ssl_write_record( ssl ) ) != 0 ) + if( ( ret = mbedtls_ssl_write_handshake_msg( ssl ) ) != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_record", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_handshake_msg", ret ); return( ret ); } @@ -4227,9 +4227,9 @@ static int ssl_write_new_session_ticket( mbedtls_ssl_context *ssl ) */ ssl->handshake->new_session_ticket = 0; - if( ( ret = mbedtls_ssl_write_record( ssl ) ) != 0 ) + if( ( ret = mbedtls_ssl_write_handshake_msg( ssl ) ) != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_record", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_handshake_msg", ret ); return( ret ); } diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 3b047fc0b..464cf6933 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2927,19 +2927,41 @@ void mbedtls_ssl_send_flight_completed( mbedtls_ssl_context *ssl ) #endif /* MBEDTLS_SSL_PROTO_DTLS */ /* - * Record layer functions + * Handshake layer functions */ /* - * Write current record. - * Uses ssl->out_msgtype, ssl->out_msglen and bytes at ssl->out_msg. + * Write current handshake (including CCS) message. + * + * - fill in handshake headers + * - update handshake checksum + * - DTLS: save message for resending + * - then pass to the record layer + * + * Inputs: + * - ssl->out_msglen: 4 + actual handshake message len + * (4 is the size of handshake headers for TLS) + * - ssl->out_msg[0]: the handshake type (ClientHello, ServerHello, etc) + * - ssl->out_msg + 4: the handshake message body + * + * Outputs: + * - ssl->out_msglen: the length of the record contents + * (including handshake headers but excluding record headers) + * - ssl->out_msg: the record contents (handshake headers + content) */ -int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl ) +int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) { - int ret, done = 0, out_msg_type; + int ret, out_msg_type; size_t len = ssl->out_msglen; - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write record" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write handshake message" ) ); + + if( ssl->out_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE && + ssl->out_msgtype != MBEDTLS_SSL_MSG_CHANGE_CIPHER_SPEC ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } #if defined(MBEDTLS_SSL_PROTO_DTLS) if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && @@ -3028,6 +3050,32 @@ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl ) } #endif + ret = mbedtls_ssl_write_record( ssl ); + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write handshake message" ) ); + + return( ret ); +} + +/* + * Record layer functions + */ + +/* + * Write current record. + * + * Uses: + * - ssl->out_msgtype: type of the message (AppData, Handshake, Alert, CCS) + * - ssl->out_msglen: length of the record content (excl headers) + * - ssl->out_msg: record content + */ +int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl ) +{ + int ret, done = 0; + size_t len = ssl->out_msglen; + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write record" ) ); + #if defined(MBEDTLS_ZLIB_SUPPORT) if( ssl->transform_out != NULL && ssl->session_out->compression == MBEDTLS_SSL_COMPRESS_DEFLATE ) @@ -4542,9 +4590,9 @@ write_msg: ssl->state++; - if( ( ret = mbedtls_ssl_write_record( ssl ) ) != 0 ) + if( ( ret = mbedtls_ssl_write_handshake_msg( ssl ) ) != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_record", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_handshake_msg", ret ); return( ret ); } @@ -4955,9 +5003,9 @@ int mbedtls_ssl_write_change_cipher_spec( mbedtls_ssl_context *ssl ) ssl->state++; - if( ( ret = mbedtls_ssl_write_record( ssl ) ) != 0 ) + if( ( ret = mbedtls_ssl_write_handshake_msg( ssl ) ) != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_record", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_handshake_msg", ret ); return( ret ); } @@ -5583,9 +5631,9 @@ int mbedtls_ssl_write_finished( mbedtls_ssl_context *ssl ) mbedtls_ssl_send_flight_completed( ssl ); #endif - if( ( ret = mbedtls_ssl_write_record( ssl ) ) != 0 ) + if( ( ret = mbedtls_ssl_write_handshake_msg( ssl ) ) != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_record", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_handshake_msg", ret ); return( ret ); } @@ -6984,9 +7032,9 @@ static int ssl_write_hello_request( mbedtls_ssl_context *ssl ) ssl->out_msgtype = MBEDTLS_SSL_MSG_HANDSHAKE; ssl->out_msg[0] = MBEDTLS_SSL_HS_HELLO_REQUEST; - if( ( ret = mbedtls_ssl_write_record( ssl ) ) != 0 ) + if( ( ret = mbedtls_ssl_write_handshake_msg( ssl ) ) != 0 ) { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_record", ret ); + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_handshake_msg", ret ); return( ret ); } From 9c3a8caa928d2ea1679f3ec088b5afcfc533c185 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 13 Sep 2017 09:54:27 +0200 Subject: [PATCH 02/35] Clarify code a bit in write_handshake_msg() - take advantage of the fact that we're only called for first send - put all sanity checks at the top - rename and constify shortcut variables - improve comments --- library/ssl_tls.c | 64 ++++++++++++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 28 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 464cf6933..b66b4fec4 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2938,6 +2938,8 @@ void mbedtls_ssl_send_flight_completed( mbedtls_ssl_context *ssl ) * - DTLS: save message for resending * - then pass to the record layer * + * DTLS: only used when first writing the message, not for resending. + * * Inputs: * - ssl->out_msglen: 4 + actual handshake message len * (4 is the size of handshake headers for TLS) @@ -2951,11 +2953,15 @@ void mbedtls_ssl_send_flight_completed( mbedtls_ssl_context *ssl ) */ int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) { - int ret, out_msg_type; - size_t len = ssl->out_msglen; + int ret; + const size_t hs_len = ssl->out_msglen - 4; + const unsigned char hs_type = ssl->out_msg[0]; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> write handshake message" ) ); + /* + * Sanity checks + */ if( ssl->out_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE && ssl->out_msgtype != MBEDTLS_SSL_MSG_CHANGE_CIPHER_SPEC ) { @@ -2963,29 +2969,32 @@ int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } + if( ssl->out_msgtype == MBEDTLS_SSL_MSG_HANDSHAKE && + hs_type != MBEDTLS_SSL_HS_HELLO_REQUEST && + ssl->handshake == NULL ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } + #if defined(MBEDTLS_SSL_PROTO_DTLS) if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && ssl->handshake != NULL && ssl->handshake->retransmit_state == MBEDTLS_SSL_RETRANS_SENDING ) { - ; /* Skip special handshake treatment when resending */ + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); } - else #endif + + /* + * Fill handshake headers + */ if( ssl->out_msgtype == MBEDTLS_SSL_MSG_HANDSHAKE ) { - out_msg_type = ssl->out_msg[0]; - - if( out_msg_type != MBEDTLS_SSL_HS_HELLO_REQUEST && - ssl->handshake == NULL ) - { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); - } - - ssl->out_msg[1] = (unsigned char)( ( len - 4 ) >> 16 ); - ssl->out_msg[2] = (unsigned char)( ( len - 4 ) >> 8 ); - ssl->out_msg[3] = (unsigned char)( ( len - 4 ) ); + ssl->out_msg[1] = (unsigned char)( hs_len >> 16 ); + ssl->out_msg[2] = (unsigned char)( hs_len >> 8 ); + ssl->out_msg[3] = (unsigned char)( hs_len ); /* * DTLS has additional fields in the Handshake layer, @@ -3002,17 +3011,16 @@ int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) { MBEDTLS_SSL_DEBUG_MSG( 1, ( "DTLS handshake message too large: " "size %u, maximum %u", - (unsigned) ( ssl->in_hslen - 4 ), + (unsigned) ( hs_len ), (unsigned) ( MBEDTLS_SSL_OUT_CONTENT_LEN - 12 ) ) ); return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); } - memmove( ssl->out_msg + 12, ssl->out_msg + 4, len - 4 ); + memmove( ssl->out_msg + 12, ssl->out_msg + 4, hs_len ); ssl->out_msglen += 8; - len += 8; /* Write message_seq and update it, except for HelloRequest */ - if( out_msg_type != MBEDTLS_SSL_HS_HELLO_REQUEST ) + if( hs_type != MBEDTLS_SSL_HS_HELLO_REQUEST ) { ssl->out_msg[4] = ( ssl->handshake->out_msg_seq >> 8 ) & 0xFF; ssl->out_msg[5] = ( ssl->handshake->out_msg_seq ) & 0xFF; @@ -3024,23 +3032,22 @@ int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) ssl->out_msg[5] = 0; } - /* We don't fragment, so frag_offset = 0 and frag_len = len */ + /* Handshake hashes are computed without fragmentation, + * so set frag_offset = 0 and frag_len = hs_len for now */ memset( ssl->out_msg + 6, 0x00, 3 ); memcpy( ssl->out_msg + 9, ssl->out_msg + 1, 3 ); } #endif /* MBEDTLS_SSL_PROTO_DTLS */ - if( out_msg_type != MBEDTLS_SSL_HS_HELLO_REQUEST ) - ssl->handshake->update_checksum( ssl, ssl->out_msg, len ); + /* Update running hashes of hanshake messages seen */ + if( hs_type != MBEDTLS_SSL_HS_HELLO_REQUEST ) + ssl->handshake->update_checksum( ssl, ssl->out_msg, ssl->out_msglen ); } - /* Save handshake and CCS messages for resending */ + /* Save for resending */ #if defined(MBEDTLS_SSL_PROTO_DTLS) if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && - ssl->handshake != NULL && - ssl->handshake->retransmit_state != MBEDTLS_SSL_RETRANS_SENDING && - ( ssl->out_msgtype == MBEDTLS_SSL_MSG_CHANGE_CIPHER_SPEC || - ssl->out_msgtype == MBEDTLS_SSL_MSG_HANDSHAKE ) ) + hs_type != MBEDTLS_SSL_HS_HELLO_REQUEST ) { if( ( ret = ssl_flight_append( ssl ) ) != 0 ) { @@ -3050,6 +3057,7 @@ int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) } #endif + /* Actually send out */ ret = mbedtls_ssl_write_record( ssl ); MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write handshake message" ) ); From 87a346f64e0d73522c17c22c5f4982c291d52641 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 13 Sep 2017 12:45:21 +0200 Subject: [PATCH 03/35] Always save flight first, (re)send later This will allow fragmentation to always happen in the same place, always from a buffer distinct from ssl->out_msg, and with the same way of resuming after returning WANT_WRITE --- include/mbedtls/ssl_internal.h | 1 + library/ssl_cli.c | 11 ++++++- library/ssl_srv.c | 20 +++++++++++- library/ssl_tls.c | 59 ++++++++++++++++++++++++++-------- 4 files changed, 75 insertions(+), 16 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 68b5f3033..501202bb3 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -669,6 +669,7 @@ static inline size_t mbedtls_ssl_hs_hdr_len( const mbedtls_ssl_context *ssl ) void mbedtls_ssl_send_flight_completed( mbedtls_ssl_context *ssl ); void mbedtls_ssl_recv_flight_completed( mbedtls_ssl_context *ssl ); int mbedtls_ssl_resend( mbedtls_ssl_context *ssl ); +int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ); #endif /* Visible for testing purposes only */ diff --git a/library/ssl_cli.c b/library/ssl_cli.c index 253c81f73..4b17deaaa 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1094,6 +1094,15 @@ static int ssl_write_client_hello( mbedtls_ssl_context *ssl ) return( ret ); } +#if defined(MBEDTLS_SSL_PROTO_DTLS) + if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + ( ret = mbedtls_ssl_flight_transmit( ssl ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_flight_transmit", ret ); + return( ret ); + } +#endif + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write client hello" ) ); return( 0 ); @@ -3402,7 +3411,7 @@ int mbedtls_ssl_handshake_client_step( mbedtls_ssl_context *ssl ) if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && ssl->handshake->retransmit_state == MBEDTLS_SSL_RETRANS_SENDING ) { - if( ( ret = mbedtls_ssl_resend( ssl ) ) != 0 ) + if( ( ret = mbedtls_ssl_flight_transmit( ssl ) ) != 0 ) return( ret ); } #endif diff --git a/library/ssl_srv.c b/library/ssl_srv.c index 66de2e46c..eda50bb34 100644 --- a/library/ssl_srv.c +++ b/library/ssl_srv.c @@ -2390,6 +2390,15 @@ static int ssl_write_hello_verify_request( mbedtls_ssl_context *ssl ) return( ret ); } +#if defined(MBEDTLS_SSL_PROTO_DTLS) + if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + ( ret = mbedtls_ssl_flight_transmit( ssl ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_flight_transmit", ret ); + return( ret ); + } +#endif + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write hello verify request" ) ); return( 0 ); @@ -3369,6 +3378,15 @@ static int ssl_write_server_hello_done( mbedtls_ssl_context *ssl ) return( ret ); } +#if defined(MBEDTLS_SSL_PROTO_DTLS) + if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + ( ret = mbedtls_ssl_flight_transmit( ssl ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_flight_transmit", ret ); + return( ret ); + } +#endif + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write server hello done" ) ); return( 0 ); @@ -4258,7 +4276,7 @@ int mbedtls_ssl_handshake_server_step( mbedtls_ssl_context *ssl ) if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && ssl->handshake->retransmit_state == MBEDTLS_SSL_RETRANS_SENDING ) { - if( ( ret = mbedtls_ssl_resend( ssl ) ) != 0 ) + if( ( ret = mbedtls_ssl_flight_transmit( ssl ) ) != 0 ) return( ret ); } #endif diff --git a/library/ssl_tls.c b/library/ssl_tls.c index b66b4fec4..5f032232a 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2822,18 +2822,34 @@ static void ssl_swap_epochs( mbedtls_ssl_context *ssl ) /* * Retransmit the current flight of messages. + */ +int mbedtls_ssl_resend( mbedtls_ssl_context *ssl ) +{ + int ret = 0; + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> mbedtls_ssl_resend" ) ); + + ret = mbedtls_ssl_flight_transmit( ssl ); + + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= mbedtls_ssl_resend" ) ); + + return( ret ); +} + +/* + * Transmit or retransmit the current flight of messages. * * Need to remember the current message in case flush_output returns * WANT_WRITE, causing us to exit this function and come back later. * This function must be called until state is no longer SENDING. */ -int mbedtls_ssl_resend( mbedtls_ssl_context *ssl ) +int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ) { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> mbedtls_ssl_resend" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> mbedtls_ssl_flight_transmit" ) ); if( ssl->handshake->retransmit_state != MBEDTLS_SSL_RETRANS_SENDING ) { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "initialise resending" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "initialise fligh transmission" ) ); ssl->handshake->cur_msg = ssl->handshake->flight; ssl_swap_epochs( ssl ); @@ -2861,7 +2877,7 @@ int mbedtls_ssl_resend( mbedtls_ssl_context *ssl ) ssl->handshake->cur_msg = cur->next; - MBEDTLS_SSL_DEBUG_BUF( 3, "resent handshake message header", ssl->out_msg, 12 ); + MBEDTLS_SSL_DEBUG_BUF( 3, "handshake header", ssl->out_msg, 12 ); if( ( ret = mbedtls_ssl_write_record( ssl ) ) != 0 ) { @@ -2878,7 +2894,7 @@ int mbedtls_ssl_resend( mbedtls_ssl_context *ssl ) ssl_set_timer( ssl, ssl->handshake->retransmit_timeout ); } - MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= mbedtls_ssl_resend" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= mbedtls_ssl_flight_transmit" ) ); return( 0 ); } @@ -2931,14 +2947,15 @@ void mbedtls_ssl_send_flight_completed( mbedtls_ssl_context *ssl ) */ /* - * Write current handshake (including CCS) message. + * Write (DTLS: or queue) current handshake (including CCS) message. * * - fill in handshake headers * - update handshake checksum * - DTLS: save message for resending * - then pass to the record layer * - * DTLS: only used when first writing the message, not for resending. + * DTLS: except for HelloRequest, messages are only queued, and will only be + * actually sent when calling flight_transmit() or resend(). * * Inputs: * - ssl->out_msglen: 4 + actual handshake message len @@ -2946,7 +2963,7 @@ void mbedtls_ssl_send_flight_completed( mbedtls_ssl_context *ssl ) * - ssl->out_msg[0]: the handshake type (ClientHello, ServerHello, etc) * - ssl->out_msg + 4: the handshake message body * - * Outputs: + * Ouputs, ie state before passing to flight_append() or write_record(): * - ssl->out_msglen: the length of the record contents * (including handshake headers but excluding record headers) * - ssl->out_msg: the record contents (handshake headers + content) @@ -3044,7 +3061,7 @@ int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) ssl->handshake->update_checksum( ssl, ssl->out_msg, ssl->out_msglen ); } - /* Save for resending */ + /* Either send now, or just save to be sent (and resent) later */ #if defined(MBEDTLS_SSL_PROTO_DTLS) if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && hs_type != MBEDTLS_SSL_HS_HELLO_REQUEST ) @@ -3055,14 +3072,19 @@ int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) return( ret ); } } + else #endif - - /* Actually send out */ - ret = mbedtls_ssl_write_record( ssl ); + { + if( ( ret = mbedtls_ssl_write_record( ssl ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_write_record", ret ); + return( ret ); + } + } MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write handshake message" ) ); - return( ret ); + return( 0 ); } /* @@ -5645,6 +5667,15 @@ int mbedtls_ssl_write_finished( mbedtls_ssl_context *ssl ) return( ret ); } +#if defined(MBEDTLS_SSL_PROTO_DTLS) + if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + ( ret = mbedtls_ssl_flight_transmit( ssl ) ) != 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_flight_transmit", ret ); + return( ret ); + } +#endif + MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= write finished" ) ); return( 0 ); @@ -7207,7 +7238,7 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) if( ssl->handshake != NULL && ssl->handshake->retransmit_state == MBEDTLS_SSL_RETRANS_SENDING ) { - if( ( ret = mbedtls_ssl_resend( ssl ) ) != 0 ) + if( ( ret = mbedtls_ssl_flight_transmit( ssl ) ) != 0 ) return( ret ); } } From 28f4beab1c3f2df6a45000fa8985bf46736700b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 13 Sep 2017 14:00:05 +0200 Subject: [PATCH 04/35] Start implementing fragmentation --- include/mbedtls/ssl_internal.h | 5 ++- library/ssl_tls.c | 72 ++++++++++++++++++++++++++++++---- 2 files changed, 68 insertions(+), 9 deletions(-) diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h index 501202bb3..18982f89a 100644 --- a/include/mbedtls/ssl_internal.h +++ b/include/mbedtls/ssl_internal.h @@ -298,8 +298,9 @@ struct mbedtls_ssl_handshake_params uint32_t retransmit_timeout; /*!< Current value of timeout */ unsigned char retransmit_state; /*!< Retransmission state */ - mbedtls_ssl_flight_item *flight; /*!< Current outgoing flight */ - mbedtls_ssl_flight_item *cur_msg; /*!< Current message in flight */ + mbedtls_ssl_flight_item *flight; /*!< Current outgoing flight */ + mbedtls_ssl_flight_item *cur_msg; /*!< Current message in flight */ + unsigned char *cur_msg_p; /*!< Position in current message */ unsigned int in_flight_start_seq; /*!< Minimum message sequence in the flight being received */ mbedtls_ssl_transform *alt_transform_out; /*!< Alternative transform for diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 5f032232a..6e0f6b604 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2852,16 +2852,23 @@ int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_MSG( 2, ( "initialise fligh transmission" ) ); ssl->handshake->cur_msg = ssl->handshake->flight; + ssl->handshake->cur_msg_p = ssl->handshake->flight->p + 12; ssl_swap_epochs( ssl ); ssl->handshake->retransmit_state = MBEDTLS_SSL_RETRANS_SENDING; } + /* + * XXX: this should not be hardcoded. + * Currently UDP limit - HS header - Record header + * (Should account for encryption overhead (renegotiation, finished)?) + */ +#define HS_LIMIT ( 512 - 12 - 13 ) + while( ssl->handshake->cur_msg != NULL ) { int ret; - mbedtls_ssl_flight_item *cur = ssl->handshake->cur_msg; - + const mbedtls_ssl_flight_item * const cur = ssl->handshake->cur_msg; /* Swap epochs before sending Finished: we can't do it after * sending ChangeCipherSpec, in case write returns WANT_READ. * Must be done before copying, may change out_msg pointer */ @@ -2871,14 +2878,64 @@ int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ) ssl_swap_epochs( ssl ); } - memcpy( ssl->out_msg, cur->p, cur->len ); - ssl->out_msglen = cur->len; - ssl->out_msgtype = cur->type; + /* CCS is copied as is, while HS messages may need fragmentation */ + if( cur->type == MBEDTLS_SSL_MSG_CHANGE_CIPHER_SPEC ) + { + memcpy( ssl->out_msg, cur->p, cur->len ); + ssl->out_msglen = cur->len; + ssl->out_msgtype = cur->type; - ssl->handshake->cur_msg = cur->next; + /* Update position inside current message */ + ssl->handshake->cur_msg_p += cur->len; + } + else + { + const unsigned char * const p = ssl->handshake->cur_msg_p; + const size_t hs_len = cur->len - 12; + const size_t frag_off = p - ( cur->p + 12 ); + const size_t rem_len = hs_len - frag_off; + const size_t frag_len = rem_len > HS_LIMIT ? HS_LIMIT : rem_len; - MBEDTLS_SSL_DEBUG_BUF( 3, "handshake header", ssl->out_msg, 12 ); + /* Messages are stored with handshake headers as if not fragmented, + * copy beginning of headers then fill fragmentation fields. + * Handshake headers: type(1) len(3) seq(2) f_off(3) f_len(3) */ + memcpy( ssl->out_msg, cur->p, 6 ); + ssl->out_msg[6] = ( ( frag_off >> 16 ) & 0xff ); + ssl->out_msg[7] = ( ( frag_off >> 8 ) & 0xff ); + ssl->out_msg[8] = ( ( frag_off ) & 0xff ); + + ssl->out_msg[ 9] = ( ( frag_len >> 16 ) & 0xff ); + ssl->out_msg[10] = ( ( frag_len >> 8 ) & 0xff ); + ssl->out_msg[11] = ( ( frag_len ) & 0xff ); + + MBEDTLS_SSL_DEBUG_BUF( 3, "handshake header", ssl->out_msg, 12 ); + + /* Copy the handshame message content and set records fields */ + memcpy( ssl->out_msg + 12, p, frag_len ); + ssl->out_msglen = frag_len + 12; + ssl->out_msgtype = cur->type; + + /* Update position inside current message */ + ssl->handshake->cur_msg_p += frag_len; + } + + /* If done with the current message move to the next one if any */ + if( ssl->handshake->cur_msg_p >= cur->p + cur->len ) + { + if( cur->next != NULL ) + { + ssl->handshake->cur_msg = cur->next; + ssl->handshake->cur_msg_p = cur->next->p + 12; + } + else + { + ssl->handshake->cur_msg = NULL; + ssl->handshake->cur_msg_p = NULL; + } + } + + /* Actually send the message out */ if( ( ret = mbedtls_ssl_write_record( ssl ) ) != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_write_record", ret ); @@ -2886,6 +2943,7 @@ int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ) } } + /* Update state and set timer */ if( ssl->state == MBEDTLS_SSL_HANDSHAKE_OVER ) ssl->handshake->retransmit_state = MBEDTLS_SSL_RETRANS_FINISHED; else From 2cb17e201b7a9508471bc4716f3f65951a73ed6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 19 Sep 2017 13:00:47 +0200 Subject: [PATCH 05/35] Make handshake fragmentation follow max_frag_len Note: no interop tests in ssl-opt.sh for now, as some of them make us run into bugs in (the CI's default versions of) OpenSSL and GnuTLS, so interop tests will be added later once the situation is clarified. <- TODO --- library/ssl_tls.c | 32 ++++++++++----- tests/ssl-opt.sh | 102 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 11 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 6e0f6b604..86a279c0e 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2845,12 +2845,23 @@ int mbedtls_ssl_resend( mbedtls_ssl_context *ssl ) */ int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ) { +#if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) + const size_t max_record_content_len = mbedtls_ssl_get_max_frag_len( ssl ); +#else + const size_t max_record_content_len = MBEDTLS_SSL_OUT_CONTENT_LEN; +#endif + /* DTLS handshake headers are 12 bytes */ + const size_t max_hs_fragment_len = max_record_content_len - 12; + MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> mbedtls_ssl_flight_transmit" ) ); if( ssl->handshake->retransmit_state != MBEDTLS_SSL_RETRANS_SENDING ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "initialise fligh transmission" ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "max handshake fragment length: %u", + max_hs_fragment_len ) ); + ssl->handshake->cur_msg = ssl->handshake->flight; ssl->handshake->cur_msg_p = ssl->handshake->flight->p + 12; ssl_swap_epochs( ssl ); @@ -2858,13 +2869,6 @@ int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ) ssl->handshake->retransmit_state = MBEDTLS_SSL_RETRANS_SENDING; } - /* - * XXX: this should not be hardcoded. - * Currently UDP limit - HS header - Record header - * (Should account for encryption overhead (renegotiation, finished)?) - */ -#define HS_LIMIT ( 512 - 12 - 13 ) - while( ssl->handshake->cur_msg != NULL ) { int ret; @@ -2894,7 +2898,8 @@ int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ) const size_t hs_len = cur->len - 12; const size_t frag_off = p - ( cur->p + 12 ); const size_t rem_len = hs_len - frag_off; - const size_t frag_len = rem_len > HS_LIMIT ? HS_LIMIT : rem_len; + const size_t frag_len = rem_len > max_hs_fragment_len + ? max_hs_fragment_len : rem_len; /* Messages are stored with handshake headers as if not fragmented, * copy beginning of headers then fill fragmentation fields. @@ -7029,15 +7034,20 @@ size_t mbedtls_ssl_get_max_frag_len( const mbedtls_ssl_context *ssl ) */ max_len = ssl_mfl_code_to_length( ssl->conf->mfl_code ); - /* - * Check if a smaller max length was negotiated - */ + /* Check if a smaller max length was negotiated */ if( ssl->session_out != NULL && ssl_mfl_code_to_length( ssl->session_out->mfl_code ) < max_len ) { max_len = ssl_mfl_code_to_length( ssl->session_out->mfl_code ); } + /* During a handshake, use the value being negotiated */ + if( ssl->session_negotiate != NULL && + ssl_mfl_code_to_length( ssl->session_negotiate->mfl_code ) < max_len ) + { + max_len = ssl_mfl_code_to_length( ssl->session_negotiate->mfl_code ); + } + return max_len; } #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 937a27b76..0cf288f12 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -4877,6 +4877,108 @@ run_test "DTLS reassembly: fragmentation, nbio (openssl server)" \ -c "found fragmented DTLS handshake message" \ -C "error" +# Tests for sending fragmented handshake messages with DTLS +# +# Use client auth when we need the client to send large messages, +# and use large cert chains on both sides too (the long chains we have all use +# both RSA and ECDSA, but ideally we should have long chains with either). +# Sizes reached (UDP payload): +# - 2037B for server certificate +# - 1542B for client certificate +# - 1013B for newsessionticket +# - all others below 512B +# All those tests assume MAX_CONTENT_LEN is at least 2048 + +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH +run_test "DTLS fragmenting: none (for reference)" \ + "$P_SRV dtls=1 debug_level=2 auth_mode=required \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + max_frag_len=2048" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + max_frag_len=2048" \ + 0 \ + -S "found fragmented DTLS handshake message" \ + -C "found fragmented DTLS handshake message" \ + -C "error" + +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH +run_test "DTLS fragmenting: server only" \ + "$P_SRV dtls=1 debug_level=2 auth_mode=required \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + max_frag_len=1024" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + max_frag_len=2048" \ + 0 \ + -S "found fragmented DTLS handshake message" \ + -c "found fragmented DTLS handshake message" \ + -C "error" + +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH +run_test "DTLS fragmenting: server only (more)" \ + "$P_SRV dtls=1 debug_level=2 auth_mode=required \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + max_frag_len=512" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + max_frag_len=2048" \ + 0 \ + -S "found fragmented DTLS handshake message" \ + -c "found fragmented DTLS handshake message" \ + -C "error" + +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH +run_test "DTLS fragmenting: client-initiated, server only" \ + "$P_SRV dtls=1 debug_level=2 auth_mode=none \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + max_frag_len=2048" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + max_frag_len=512" \ + 0 \ + -S "found fragmented DTLS handshake message" \ + -c "found fragmented DTLS handshake message" \ + -C "error" + +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH +run_test "DTLS fragmenting: client-initiated, both" \ + "$P_SRV dtls=1 debug_level=2 auth_mode=required \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + max_frag_len=2048" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + max_frag_len=512" \ + 0 \ + -s "found fragmented DTLS handshake message" \ + -c "found fragmented DTLS handshake message" \ + -C "error" + # Tests for specific things with "unreliable" UDP connection not_with_valgrind # spurious resend due to timeout From 01ec4af0238e62cf296b7eeade42ca5835327879 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 21 Sep 2017 13:16:52 +0200 Subject: [PATCH 06/35] Add ChangeLog entry --- ChangeLog | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ChangeLog b/ChangeLog index 59561fd07..948e4c3da 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,6 +2,9 @@ mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS x.x.x branch released xxxx-xx-xx +Features + * Add support for fragmentation of outoing DTLS handshake messages. + Bugfix * Fixes an issue with MBEDTLS_CHACHAPOLY_C which would not compile if MBEDTLS_ARC4_C and MBEDTLS_CIPHER_NULL_CIPHER weren't also defined. #1890 From 0b1d9b2c75b6f220b4eb8f1447a5d487e277b081 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 21 Sep 2017 13:15:27 +0200 Subject: [PATCH 07/35] Declare ssl_conf_mtu() --- ChangeLog | 3 +++ include/mbedtls/ssl.h | 43 +++++++++++++++++++++++++++++++++++++++++++ library/ssl_tls.c | 7 +++++++ 3 files changed, 53 insertions(+) diff --git a/ChangeLog b/ChangeLog index 948e4c3da..7233d4d23 100644 --- a/ChangeLog +++ b/ChangeLog @@ -24,6 +24,9 @@ Changes * Improve compatibility with some alternative CCM implementations by using CCM test vectors from RAM. +INTERNAL NOTE: need to bump soversion of libmbedtls: +- added new member 'mtu' to public 'mbedtls_ssl_conf' structure + = mbed TLS 2.12.0 branch released 2018-07-25 Security diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 2d511a8ea..0283eee62 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -958,6 +958,10 @@ struct mbedtls_ssl_config unsigned int dhm_min_bitlen; /*!< min. bit length of the DHM prime */ #endif +#if defined(MBEDTLS_SSL_PROTO_DTLS) + uint16_t mtu; /*!< path mtu, used to fragment outoing messages */ +#endif + unsigned char max_major_ver; /*!< max. major version used */ unsigned char max_minor_ver; /*!< max. minor version used */ unsigned char min_major_ver; /*!< min. major version used */ @@ -2423,6 +2427,33 @@ void mbedtls_ssl_conf_cert_req_ca_list( mbedtls_ssl_config *conf, char cert_req_ca_list ); #endif /* MBEDTLS_SSL_SRV_C */ +#if defined(MBEDTLS_SSL_PROTO_DTLS) +/** + * \brief Set the Maximum Tranport Unit (MTU). + * This represents the maximum size of a datagram payload + * handled by the transport layer (usually UDP) as determined + * by the network link and stack. In practice, this controls + * the maximum size datagram the DTLS layer will pass to the + * \c f_send() callback set using \c mbedtls_ssl_set_bio(). + * + * \note This only controls the size of the packet we send. + * Client-side, you can request the server to use smaller + * records with \c mbedtls_conf_max_frag_len(). + * + * \note If both a MTU and a maximum fragment length have been + * configured (or negotiated with the peer), the lower limit + * is used. + * + * \note Values larger than \c MBEDTLS_SSL_OUT_CONTENT_LEN have no + * effect. This can only be used to decrease the maximum size + * of detagrams sent. + * + * \param conf SSL configuration + * \param mtu Value of the path MTU in bytes + */ +void mbedtls_ssl_conf_mtu( mbedtls_ssl_config *conf, uint16_t mtu ); +#endif /* MBEDTLS_SSL_PROTO_DTLS */ + #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) /** * \brief Set the maximum fragment length to emit and/or negotiate @@ -2433,6 +2464,18 @@ void mbedtls_ssl_conf_cert_req_ca_list( mbedtls_ssl_config *conf, * (Client: set maximum fragment length to emit *and* * negotiate with the server during handshake) * + * \note With TLS, this currently only affects ApplicationData (sent + * with \c mbedtls_ssl_read()), not handshake messages. + * With DTLS, this affects both ApplicationData and handshake. + * + * \note This sets the maximum length for a record's paylaod, + * excluding record overhead that will be added to it, see + * \c mbedtls_ssl_get_record_expansion(). + * + * \note For DTLS, it is also possible to set a limit for the total + * size of daragrams passed to the transport layer, including + * record overhead, see \c mbedtls_ssl_conf_mtu(). + * * \param conf SSL configuration * \param mfl_code Code for maximum fragment length (allowed values: * MBEDTLS_SSL_MAX_FRAG_LEN_512, MBEDTLS_SSL_MAX_FRAG_LEN_1024, diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 86a279c0e..4b124ba8f 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6750,6 +6750,13 @@ void mbedtls_ssl_conf_arc4_support( mbedtls_ssl_config *conf, char arc4 ) } #endif +#if defined(MBEDTLS_SSL_PROTO_DTLS) +void mbedtls_ssl_conf_mtu( mbedtls_ssl_config *conf, uint16_t mtu ) +{ + conf->mtu = mtu; +} +#endif + #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) int mbedtls_ssl_conf_max_frag_len( mbedtls_ssl_config *conf, unsigned char mfl_code ) { From 9468ff1966faea814edbd2600ad196dd98c96686 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 21 Sep 2017 13:49:50 +0200 Subject: [PATCH 08/35] Implement support for MTU setting --- include/mbedtls/ssl.h | 43 ++++++++++++++++++++++----- library/ssl_tls.c | 69 +++++++++++++++++++++++++++++++++++-------- 2 files changed, 92 insertions(+), 20 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 0283eee62..706e27284 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2430,6 +2430,7 @@ void mbedtls_ssl_conf_cert_req_ca_list( mbedtls_ssl_config *conf, #if defined(MBEDTLS_SSL_PROTO_DTLS) /** * \brief Set the Maximum Tranport Unit (MTU). + * Special value: 0 means unset (no limit). * This represents the maximum size of a datagram payload * handled by the transport layer (usually UDP) as determined * by the network link and stack. In practice, this controls @@ -2446,7 +2447,8 @@ void mbedtls_ssl_conf_cert_req_ca_list( mbedtls_ssl_config *conf, * * \note Values larger than \c MBEDTLS_SSL_OUT_CONTENT_LEN have no * effect. This can only be used to decrease the maximum size - * of detagrams sent. + * of datagrams sent. Values lower than record layer expansion + * are ignored. * * \param conf SSL configuration * \param mtu Value of the path MTU in bytes @@ -2738,6 +2740,9 @@ const char *mbedtls_ssl_get_version( const mbedtls_ssl_context *ssl ); * \brief Return the (maximum) number of bytes added by the record * layer: header + encryption/MAC overhead (inc. padding) * + * \note This function is not available (always returns an error) + * when record compression is enabled. + * * \param ssl SSL context * * \return Current maximum record expansion in bytes, or @@ -2752,12 +2757,8 @@ int mbedtls_ssl_get_record_expansion( const mbedtls_ssl_context *ssl ); * This is the value negotiated with peer if any, * or the locally configured value. * - * \note With DTLS, \c mbedtls_ssl_write() will return an error if - * called with a larger length value. - * With TLS, \c mbedtls_ssl_write() will fragment the input if - * necessary and return the number of bytes written; it is up - * to the caller to call \c mbedtls_ssl_write() again in - * order to send the remaining bytes if any. + * \sa mbedtls_ssl_conf_max_frag_len() + * \sa mbedtls_ssl_get_max_record_payload() * * \param ssl SSL context * @@ -2766,6 +2767,34 @@ int mbedtls_ssl_get_record_expansion( const mbedtls_ssl_context *ssl ); size_t mbedtls_ssl_get_max_frag_len( const mbedtls_ssl_context *ssl ); #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ +/** + * \brief Return the current maximum outgoing record payload in bytes. + * This takes into account the config.h setting \c + * MBEDTLS_SSL_OUT_CONTENT_LEN, the configured and negotiated + * max fragment length extension if used, and for DTLS the + * path MTU as configured and current record expansion. + * + * \note With DTLS, \c mbedtls_ssl_write() will return an error if + * called with a larger length value. + * With TLS, \c mbedtls_ssl_write() will fragment the input if + * necessary and return the number of bytes written; it is up + * to the caller to call \c mbedtls_ssl_write() again in + * order to send the remaining bytes if any. + * + * \note This function is not available (always returns an error) + * when record compression is enabled. + * + * \sa mbedtls_ssl_conf_mtu() + * \sa mbedtls_ssl_get_max_frag_len() + * \sa mbedtls_ssl_get_record_expansion() + * + * \param ssl SSL context + * + * \return Current maximum payload for an outgoing record, + * or a negative error code. + */ +int mbedtls_ssl_get_max_out_record_payload( const mbedtls_ssl_context *ssl ); + #if defined(MBEDTLS_X509_CRT_PARSE_C) /** * \brief Return the peer certificate from the current connection diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 4b124ba8f..7b2ab0fb0 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2845,16 +2845,20 @@ int mbedtls_ssl_resend( mbedtls_ssl_context *ssl ) */ int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ) { -#if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) - const size_t max_record_content_len = mbedtls_ssl_get_max_frag_len( ssl ); -#else - const size_t max_record_content_len = MBEDTLS_SSL_OUT_CONTENT_LEN; -#endif + const int ret_payload = mbedtls_ssl_get_max_out_record_payload( ssl ); + const size_t max_record_payload = (size_t) ret_payload; /* DTLS handshake headers are 12 bytes */ - const size_t max_hs_fragment_len = max_record_content_len - 12; + const size_t max_hs_fragment_len = max_record_payload - 12; MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> mbedtls_ssl_flight_transmit" ) ); + if( ret_payload < 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_get_max_out_record_payload", + ret_payload ); + return( ret_payload ); + } + if( ssl->handshake->retransmit_state != MBEDTLS_SSL_RETRANS_SENDING ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "initialise fligh transmission" ) ); @@ -7008,6 +7012,7 @@ int mbedtls_ssl_get_record_expansion( const mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_ZLIB_SUPPORT) if( ssl->session_out->compression != MBEDTLS_SSL_COMPRESS_NULL ) return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + } #endif switch( mbedtls_cipher_get_cipher_mode( &transform->cipher_ctx_enc ) ) @@ -7055,10 +7060,45 @@ size_t mbedtls_ssl_get_max_frag_len( const mbedtls_ssl_context *ssl ) max_len = ssl_mfl_code_to_length( ssl->session_negotiate->mfl_code ); } - return max_len; + return( max_len ); } #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ +int mbedtls_ssl_get_max_out_record_payload( const mbedtls_ssl_context *ssl ) +{ + size_t max_len = MBEDTLS_SSL_OUT_CONTENT_LEN; + +#if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) + const size_t mfl = mbedtls_ssl_get_max_frag_len( ssl ); + + if( max_len > mfl ) + max_len = mfl; +#endif + +#if defined(MBEDTLS_SSL_PROTO_DTLS) + if( ssl->conf->mtu != 0 ) + { + const size_t mtu = ssl->conf->mtu; + const int ret = mbedtls_ssl_get_record_expansion( ssl ); + const size_t overhead = (size_t) ret; + + if( ret < 0 ) + return( ret ); + + if( mtu <= overhead ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "MTU too low for record expansion" ) ); + return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE ); + } + + if( max_len > mtu - overhead ) + max_len = mtu - overhead; + } +#endif + + return( (int) max_len ); +} + #if defined(MBEDTLS_X509_CRT_PARSE_C) const mbedtls_x509_crt *mbedtls_ssl_get_peer_cert( const mbedtls_ssl_context *ssl ) { @@ -7610,12 +7650,15 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len ) static int ssl_write_real( mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len ) { - int ret; -#if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) - size_t max_len = mbedtls_ssl_get_max_frag_len( ssl ); -#else - size_t max_len = MBEDTLS_SSL_OUT_CONTENT_LEN; -#endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ + int ret = mbedtls_ssl_get_max_out_record_payload( ssl ); + const size_t max_len = (size_t) ret; + + if( ret < 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_get_max_out_record_payload", ret ); + return( ret ); + } + if( len > max_len ) { #if defined(MBEDTLS_SSL_PROTO_DTLS) From b747c6cf9ba594f207c0d52b0ed572a875ee034b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Sun, 12 Aug 2018 13:28:53 +0200 Subject: [PATCH 09/35] Add basic first tests for MTU setting For now, just check that it causes us to fragment. More tests are coming in follow-up commits to ensure we respect the exact value set, including when renegotiating. --- library/ssl_tls.c | 3 ++ programs/ssl/ssl_client2.c | 15 +++++++- programs/ssl/ssl_server2.c | 15 +++++++- tests/ssl-opt.sh | 76 ++++++++++++++++++++++++++++++++++++-- 4 files changed, 103 insertions(+), 6 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index ea46d85b3..b05d2883a 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2905,6 +2905,9 @@ int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ) const size_t frag_len = rem_len > max_hs_fragment_len ? max_hs_fragment_len : rem_len; + if( frag_off == 0 && frag_len != hs_len ) + MBEDTLS_SSL_DEBUG_MSG( 2, ( "fragmenting handshake message" ) ); + /* Messages are stored with handshake headers as if not fragmented, * copy beginning of headers then fill fragmentation fields. * Handshake headers: type(1) len(3) seq(2) f_off(3) f_len(3) */ diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 0dd9e3f7b..7cdc53a54 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -106,6 +106,7 @@ int main( void ) #define DFL_TRANSPORT MBEDTLS_SSL_TRANSPORT_STREAM #define DFL_HS_TO_MIN 0 #define DFL_HS_TO_MAX 0 +#define DFL_DTLS_MTU -1 #define DFL_FALLBACK -1 #define DFL_EXTENDED_MS -1 #define DFL_ETM -1 @@ -198,7 +199,8 @@ int main( void ) #define USAGE_DTLS \ " dtls=%%d default: 0 (TLS)\n" \ " hs_timeout=%%d-%%d default: (library default: 1000-60000)\n" \ - " range of DTLS handshake timeouts in millisecs\n" + " range of DTLS handshake timeouts in millisecs\n" \ + " mtu=%%d default: (library default: unlimited)\n" #else #define USAGE_DTLS "" #endif @@ -345,6 +347,7 @@ struct options int transport; /* TLS or DTLS? */ uint32_t hs_to_min; /* Initial value of DTLS handshake timer */ uint32_t hs_to_max; /* Max value of DTLS handshake timer */ + int dtls_mtu; /* UDP Maximum tranport unit for DTLS */ int fallback; /* is this a fallback connection? */ int extended_ms; /* negotiate extended master secret? */ int etm; /* negotiate encrypt then mac? */ @@ -617,6 +620,7 @@ int main( int argc, char *argv[] ) opt.transport = DFL_TRANSPORT; opt.hs_to_min = DFL_HS_TO_MIN; opt.hs_to_max = DFL_HS_TO_MAX; + opt.dtls_mtu = DFL_DTLS_MTU; opt.fallback = DFL_FALLBACK; opt.extended_ms = DFL_EXTENDED_MS; opt.etm = DFL_ETM; @@ -927,6 +931,12 @@ int main( int argc, char *argv[] ) if( opt.hs_to_min == 0 || opt.hs_to_max < opt.hs_to_min ) goto usage; } + else if( strcmp( p, "mtu" ) == 0 ) + { + opt.dtls_mtu = atoi( q ); + if( opt.dtls_mtu < 0 ) + goto usage; + } else if( strcmp( p, "recsplit" ) == 0 ) { opt.recsplit = atoi( q ); @@ -1327,6 +1337,9 @@ int main( int argc, char *argv[] ) if( opt.hs_to_min != DFL_HS_TO_MIN || opt.hs_to_max != DFL_HS_TO_MAX ) mbedtls_ssl_conf_handshake_timeout( &conf, opt.hs_to_min, opt.hs_to_max ); + + if( opt.dtls_mtu != DFL_DTLS_MTU ) + mbedtls_ssl_conf_mtu( &conf, opt.dtls_mtu ); #endif /* MBEDTLS_SSL_PROTO_DTLS */ #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 7654a6446..484f84fdd 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -150,6 +150,7 @@ int main( void ) #define DFL_ANTI_REPLAY -1 #define DFL_HS_TO_MIN 0 #define DFL_HS_TO_MAX 0 +#define DFL_DTLS_MTU -1 #define DFL_BADMAC_LIMIT -1 #define DFL_EXTENDED_MS -1 #define DFL_ETM -1 @@ -297,7 +298,8 @@ int main( void ) #define USAGE_DTLS \ " dtls=%%d default: 0 (TLS)\n" \ " hs_timeout=%%d-%%d default: (library default: 1000-60000)\n" \ - " range of DTLS handshake timeouts in millisecs\n" + " range of DTLS handshake timeouts in millisecs\n" \ + " mtu=%%d default: (library default: unlimited)\n" #else #define USAGE_DTLS "" #endif @@ -470,6 +472,7 @@ struct options int anti_replay; /* Use anti-replay for DTLS? -1 for default */ uint32_t hs_to_min; /* Initial value of DTLS handshake timer */ uint32_t hs_to_max; /* Max value of DTLS handshake timer */ + int dtls_mtu; /* UDP Maximum tranport unit for DTLS */ int badmac_limit; /* Limit of records with bad MAC */ } opt; @@ -1338,6 +1341,7 @@ int main( int argc, char *argv[] ) opt.anti_replay = DFL_ANTI_REPLAY; opt.hs_to_min = DFL_HS_TO_MIN; opt.hs_to_max = DFL_HS_TO_MAX; + opt.dtls_mtu = DFL_DTLS_MTU; opt.badmac_limit = DFL_BADMAC_LIMIT; opt.extended_ms = DFL_EXTENDED_MS; opt.etm = DFL_ETM; @@ -1684,6 +1688,12 @@ int main( int argc, char *argv[] ) if( opt.hs_to_min == 0 || opt.hs_to_max < opt.hs_to_min ) goto usage; } + else if( strcmp( p, "mtu" ) == 0 ) + { + opt.dtls_mtu = atoi( q ); + if( opt.dtls_mtu < 0 ) + goto usage; + } else if( strcmp( p, "sni" ) == 0 ) { opt.sni = q; @@ -2155,6 +2165,9 @@ int main( int argc, char *argv[] ) #if defined(MBEDTLS_SSL_PROTO_DTLS) if( opt.hs_to_min != DFL_HS_TO_MIN || opt.hs_to_max != DFL_HS_TO_MAX ) mbedtls_ssl_conf_handshake_timeout( &conf, opt.hs_to_min, opt.hs_to_max ); + + if( opt.dtls_mtu != DFL_DTLS_MTU ) + mbedtls_ssl_conf_mtu( &conf, opt.dtls_mtu ); #endif /* MBEDTLS_SSL_PROTO_DTLS */ #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 0cf288f12..3d61ac3a4 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -4911,7 +4911,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH -run_test "DTLS fragmenting: server only" \ +run_test "DTLS fragmenting: server only (max_frag_len)" \ "$P_SRV dtls=1 debug_level=2 auth_mode=required \ crt_file=data_files/server7_int-ca.crt \ key_file=data_files/server7.key \ @@ -4929,7 +4929,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH -run_test "DTLS fragmenting: server only (more)" \ +run_test "DTLS fragmenting: server only (more) (max_frag_len)" \ "$P_SRV dtls=1 debug_level=2 auth_mode=required \ crt_file=data_files/server7_int-ca.crt \ key_file=data_files/server7.key \ @@ -4947,7 +4947,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH -run_test "DTLS fragmenting: client-initiated, server only" \ +run_test "DTLS fragmenting: client-initiated, server only (max_frag_len)" \ "$P_SRV dtls=1 debug_level=2 auth_mode=none \ crt_file=data_files/server7_int-ca.crt \ key_file=data_files/server7.key \ @@ -4965,7 +4965,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SSL_MAX_FRAGMENT_LENGTH -run_test "DTLS fragmenting: client-initiated, both" \ +run_test "DTLS fragmenting: client-initiated, both (max_frag_len)" \ "$P_SRV dtls=1 debug_level=2 auth_mode=required \ crt_file=data_files/server7_int-ca.crt \ key_file=data_files/server7.key \ @@ -4979,6 +4979,74 @@ run_test "DTLS fragmenting: client-initiated, both" \ -c "found fragmented DTLS handshake message" \ -C "error" +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +run_test "DTLS fragmenting: none (for reference) (MTU)" \ + "$P_SRV dtls=1 debug_level=2 auth_mode=required \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + mtu=2048" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + mtu=2048" \ + 0 \ + -S "found fragmented DTLS handshake message" \ + -C "found fragmented DTLS handshake message" \ + -C "error" + +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +run_test "DTLS fragmenting: client (MTU)" \ + "$P_SRV dtls=1 debug_level=2 auth_mode=required \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + mtu=2048" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + mtu=512" \ + 0 \ + -s "found fragmented DTLS handshake message" \ + -C "found fragmented DTLS handshake message" \ + -C "error" + +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +run_test "DTLS fragmenting: server (MTU)" \ + "$P_SRV dtls=1 debug_level=2 auth_mode=required \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + mtu=512" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + mtu=2048" \ + 0 \ + -S "found fragmented DTLS handshake message" \ + -c "found fragmented DTLS handshake message" \ + -C "error" + +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +run_test "DTLS fragmenting: both (MTU)" \ + "$P_SRV dtls=1 debug_level=2 auth_mode=required \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + mtu=512" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + mtu=512" \ + 0 \ + -s "found fragmented DTLS handshake message" \ + -c "found fragmented DTLS handshake message" \ + -C "error" + # Tests for specific things with "unreliable" UDP connection not_with_valgrind # spurious resend due to timeout From 72c2707d9c0db616a1b7d089c8e033c7b03dc705 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 13 Aug 2018 12:37:51 +0200 Subject: [PATCH 10/35] Add tests for MTU with renegotiation This exercises our computation of record expansion. --- tests/ssl-opt.sh | 161 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 161 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 3d61ac3a4..833b5e37f 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5047,6 +5047,167 @@ run_test "DTLS fragmenting: both (MTU)" \ -c "found fragmented DTLS handshake message" \ -C "error" +# the proxy shouldn't drop or mess up anything, so we shouldn't need to resend +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +run_test "DTLS fragmenting: proxy MTU, simple handshake" \ + -p "$P_PXY mtu=512" \ + "$P_SRV dtls=1 debug_level=2 auth_mode=required \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + mtu=512" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + mtu=512" \ + 0 \ + -S "resend" \ + -C "resend" \ + -s "found fragmented DTLS handshake message" \ + -c "found fragmented DTLS handshake message" \ + -C "error" + +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SHA256_C +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA +requires_config_enabled MBEDTLS_SSL_RENEGOTIATION +requires_config_enabled MBEDTLS_CHACHAPOLY_C +run_test "DTLS fragmenting: proxy MTU, ChachaPoly renego" \ + -p "$P_PXY mtu=512" \ + "$P_SRV dtls=1 debug_level=2 auth_mode=required \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + exchanges=2 renegotiation=1 \ + force_ciphersuite=TLS-ECDHE-ECDSA-WITH-CHACHA20-POLY1305-SHA256 \ + mtu=512" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + exchanges=2 renegotiation=1 renegotiate=1 \ + mtu=512" \ + 0 \ + -S "resend" \ + -C "resend" \ + -s "found fragmented DTLS handshake message" \ + -c "found fragmented DTLS handshake message" \ + -C "error" + +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SHA256_C +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA +requires_config_enabled MBEDTLS_SSL_RENEGOTIATION +requires_config_enabled MBEDTLS_AES_C +requires_config_enabled MBEDTLS_GCM_C +run_test "DTLS fragmenting: proxy MTU, AES-GCM renego" \ + -p "$P_PXY mtu=512" \ + "$P_SRV dtls=1 debug_level=2 auth_mode=required \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + exchanges=2 renegotiation=1 \ + force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-GCM-SHA256 \ + mtu=512" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + exchanges=2 renegotiation=1 renegotiate=1 \ + mtu=512" \ + 0 \ + -S "resend" \ + -C "resend" \ + -s "found fragmented DTLS handshake message" \ + -c "found fragmented DTLS handshake message" \ + -C "error" + +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SHA256_C +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA +requires_config_enabled MBEDTLS_SSL_RENEGOTIATION +requires_config_enabled MBEDTLS_AES_C +requires_config_enabled MBEDTLS_CCM_C +run_test "DTLS fragmenting: proxy MTU, AES-CCM renego" \ + -p "$P_PXY mtu=512" \ + "$P_SRV dtls=1 debug_level=2 auth_mode=required \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + exchanges=2 renegotiation=1 \ + force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-CCM-8 \ + mtu=512" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + exchanges=2 renegotiation=1 renegotiate=1 \ + mtu=512" \ + 0 \ + -S "resend" \ + -C "resend" \ + -s "found fragmented DTLS handshake message" \ + -c "found fragmented DTLS handshake message" \ + -C "error" + +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SHA256_C +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA +requires_config_enabled MBEDTLS_SSL_RENEGOTIATION +requires_config_enabled MBEDTLS_AES_C +requires_config_enabled MBEDTLS_CIPHER_MODE_CBC +requires_config_enabled MBEDTLS_SSL_ENCRYPT_THEN_MAC +run_test "DTLS fragmenting: proxy MTU, AES-CBC EtM renego" \ + -p "$P_PXY mtu=512" \ + "$P_SRV dtls=1 debug_level=2 auth_mode=required \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + exchanges=2 renegotiation=1 \ + force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-CBC-SHA256 \ + mtu=512" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + exchanges=2 renegotiation=1 renegotiate=1 \ + mtu=512" \ + 0 \ + -S "resend" \ + -C "resend" \ + -s "found fragmented DTLS handshake message" \ + -c "found fragmented DTLS handshake message" \ + -C "error" + +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SHA256_C +requires_config_enabled MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA +requires_config_enabled MBEDTLS_SSL_RENEGOTIATION +requires_config_enabled MBEDTLS_AES_C +requires_config_enabled MBEDTLS_CIPHER_MODE_CBC +run_test "DTLS fragmenting: proxy MTU, AES-CBC non-EtM renego" \ + -p "$P_PXY mtu=512" \ + "$P_SRV dtls=1 debug_level=2 auth_mode=required \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + exchanges=2 renegotiation=1 \ + force_ciphersuite=TLS-ECDHE-ECDSA-WITH-AES-128-CBC-SHA256 etm=0 \ + mtu=512" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + exchanges=2 renegotiation=1 renegotiate=1 \ + mtu=512" \ + 0 \ + -S "resend" \ + -C "resend" \ + -s "found fragmented DTLS handshake message" \ + -c "found fragmented DTLS handshake message" \ + -C "error" + # Tests for specific things with "unreliable" UDP connection not_with_valgrind # spurious resend due to timeout From 7e89c17788ae6d134090c639bb0c96562df7f5a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 13 Aug 2018 12:45:26 +0200 Subject: [PATCH 11/35] Fix two typos in comments --- include/mbedtls/ssl.h | 2 +- library/ssl_tls.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 706e27284..a3b514cd4 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2470,7 +2470,7 @@ void mbedtls_ssl_conf_mtu( mbedtls_ssl_config *conf, uint16_t mtu ); * with \c mbedtls_ssl_read()), not handshake messages. * With DTLS, this affects both ApplicationData and handshake. * - * \note This sets the maximum length for a record's paylaod, + * \note This sets the maximum length for a record's payload, * excluding record overhead that will be added to it, see * \c mbedtls_ssl_get_record_expansion(). * diff --git a/library/ssl_tls.c b/library/ssl_tls.c index b05d2883a..b25d9bfe7 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2923,7 +2923,7 @@ int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ) MBEDTLS_SSL_DEBUG_BUF( 3, "handshake header", ssl->out_msg, 12 ); - /* Copy the handshame message content and set records fields */ + /* Copy the handshake message content and set records fields */ memcpy( ssl->out_msg + 12, p, frag_len ); ssl->out_msglen = frag_len + 12; ssl->out_msgtype = cur->type; From 19c62f90e4608fc57f382cdbe8799ffdb98c9dda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 16 Aug 2018 10:50:39 +0200 Subject: [PATCH 12/35] Add test for session resumption --- library/ssl_tls.c | 11 ++++++----- tests/ssl-opt.sh | 26 ++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index b25d9bfe7..530f283b4 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2861,10 +2861,7 @@ int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ) if( ssl->handshake->retransmit_state != MBEDTLS_SSL_RETRANS_SENDING ) { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "initialise fligh transmission" ) ); - - MBEDTLS_SSL_DEBUG_MSG( 2, ( "max handshake fragment length: %u", - max_hs_fragment_len ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "initialise flight transmission" ) ); ssl->handshake->cur_msg = ssl->handshake->flight; ssl->handshake->cur_msg_p = ssl->handshake->flight->p + 12; @@ -2906,7 +2903,11 @@ int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ) ? max_hs_fragment_len : rem_len; if( frag_off == 0 && frag_len != hs_len ) - MBEDTLS_SSL_DEBUG_MSG( 2, ( "fragmenting handshake message" ) ); + { + MBEDTLS_SSL_DEBUG_MSG( 2, ( "fragmenting handshake message (%u > %u)", + (unsigned) hs_len, + (unsigned) max_hs_fragment_len ) ); + } /* Messages are stored with handshake headers as if not fragmented, * copy beginning of headers then fill fragmentation fields. diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 833b5e37f..7028a0738 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5068,6 +5068,32 @@ run_test "DTLS fragmenting: proxy MTU, simple handshake" \ -c "found fragmented DTLS handshake message" \ -C "error" +# This ensures things still work after session_reset(), +# for example it would have caught #1941. +# It also exercises the "resumed hanshake" flow. +# Since we don't support reading fragmented ClientHello yet, +# up the MTU to 1450 (larger than ClientHello with session ticket, +# but still smaller than client's Certificate to ensure fragmentation). +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +run_test "DTLS fragmenting: proxy MTU, resumed handshake" \ + -p "$P_PXY mtu=1450" \ + "$P_SRV dtls=1 debug_level=2 auth_mode=required \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + mtu=1450" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + mtu=1450 reconnect=1" \ + 0 \ + -S "resend" \ + -C "resend" \ + -s "found fragmented DTLS handshake message" \ + -c "found fragmented DTLS handshake message" \ + -C "error" + requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C From 2d56f0d346efa628776f92dcc7fdf8c0da66e87a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 16 Aug 2018 11:09:03 +0200 Subject: [PATCH 13/35] Add test with unreliable connection --- tests/ssl-opt.sh | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 7028a0738..397c565fe 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5234,6 +5234,25 @@ run_test "DTLS fragmenting: proxy MTU, AES-CBC non-EtM renego" \ -c "found fragmented DTLS handshake message" \ -C "error" +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +client_needs_more_time 2 +run_test "DTLS fragmenting: proxy MTU + 3d" \ + -p "$P_PXY mtu=512 drop=8 delay=8 duplicate=8" \ + "$P_SRV dtls=1 debug_level=2 auth_mode=required \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + mtu=512" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + mtu=512" \ + 0 \ + -s "found fragmented DTLS handshake message" \ + -c "found fragmented DTLS handshake message" \ + -C "error" + # Tests for specific things with "unreliable" UDP connection not_with_valgrind # spurious resend due to timeout From 1218bc0f74a14436915e6c0807be0e3f752b9da4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 17 Aug 2018 10:51:26 +0200 Subject: [PATCH 14/35] Add simple interop tests (reliable connection) --- tests/ssl-opt.sh | 114 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 397c565fe..86e9f1e06 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5253,6 +5253,120 @@ run_test "DTLS fragmenting: proxy MTU + 3d" \ -c "found fragmented DTLS handshake message" \ -C "error" +# here and below we just want to test that the we fragment in a way that +# pleases other implementations, so we don't need the peer to fragment +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "DTLS fragmenting: gnutls server, DTLS 1.2" \ + "$G_SRV -u" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + mtu=512 force_version=dtls1_2" \ + 0 \ + -c "fragmenting handshake message" \ + -C "error" + +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_1 +run_test "DTLS fragmenting: gnutls server, DTLS 1.0" \ + "$G_SRV -u" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + mtu=512 force_version=dtls1_2" \ + 0 \ + -c "fragmenting handshake message" \ + -C "error" + +# gnutls-cli always tries IPv6 first, and doesn't fall back to IPv4 with DTLS +requires_ipv6 +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "DTLS fragmenting: gnutls client, DTLS 1.2" \ + "$P_SRV dtls=1 debug_level=2 server_addr=::1 \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + mtu=512 force_version=dtls1_2" \ + "$G_CLI -u" \ + 0 \ + -s "fragmenting handshake message" + +# gnutls-cli always tries IPv6 first, and doesn't fall back to IPv4 with DTLS +requires_ipv6 +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_1 +run_test "DTLS fragmenting: gnutls client, DTLS 1.0" \ + "$P_SRV dtls=1 debug_level=2 server_addr=::1 \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + mtu=512 force_version=dtls1" \ + "$G_CLI -u" \ + 0 \ + -s "fragmenting handshake message" + +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "DTLS fragmenting: openssl server, DTLS 1.2" \ + "$O_SRV -dtls1_2 -verify 10" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + mtu=512 force_version=dtls1_2" \ + 0 \ + -c "fragmenting handshake message" \ + -C "error" + +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_1 +run_test "DTLS fragmenting: openssl server, DTLS 1.0" \ + "$O_SRV -dtls1 -verify 10" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + mtu=512 force_version=dtls1" \ + 0 \ + -c "fragmenting handshake message" \ + -C "error" + +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 +run_test "DTLS fragmenting: openssl client, DTLS 1.2" \ + "$P_SRV dtls=1 debug_level=2 \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + mtu=512 force_version=dtls1_2" \ + "$O_CLI -dtls1_2" \ + 0 \ + -s "fragmenting handshake message" + +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_1 +run_test "DTLS fragmenting: openssl client, DTLS 1.0" \ + "$P_SRV dtls=1 debug_level=2 \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + mtu=512 force_version=dtls1" \ + "$O_CLI -dtls1" \ + 0 \ + -s "fragmenting handshake message" + # Tests for specific things with "unreliable" UDP connection not_with_valgrind # spurious resend due to timeout From 0794d49566224e4d7a61bc510503ad3c55907620 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 17 Aug 2018 10:54:24 +0200 Subject: [PATCH 15/35] Skip some tests with valgrind (spurious resend) --- tests/ssl-opt.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 86e9f1e06..beceafae6 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5048,6 +5048,7 @@ run_test "DTLS fragmenting: both (MTU)" \ -C "error" # the proxy shouldn't drop or mess up anything, so we shouldn't need to resend +not_with_valgrind # spurious resend due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C @@ -5074,6 +5075,7 @@ run_test "DTLS fragmenting: proxy MTU, simple handshake" \ # Since we don't support reading fragmented ClientHello yet, # up the MTU to 1450 (larger than ClientHello with session ticket, # but still smaller than client's Certificate to ensure fragmentation). +not_with_valgrind # spurious resend due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C @@ -5094,6 +5096,7 @@ run_test "DTLS fragmenting: proxy MTU, resumed handshake" \ -c "found fragmented DTLS handshake message" \ -C "error" +not_with_valgrind # spurious resend due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C @@ -5121,6 +5124,7 @@ run_test "DTLS fragmenting: proxy MTU, ChachaPoly renego" \ -c "found fragmented DTLS handshake message" \ -C "error" +not_with_valgrind # spurious resend due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C @@ -5149,6 +5153,7 @@ run_test "DTLS fragmenting: proxy MTU, AES-GCM renego" \ -c "found fragmented DTLS handshake message" \ -C "error" +not_with_valgrind # spurious resend due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C @@ -5177,6 +5182,7 @@ run_test "DTLS fragmenting: proxy MTU, AES-CCM renego" \ -c "found fragmented DTLS handshake message" \ -C "error" +not_with_valgrind # spurious resend due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C @@ -5206,6 +5212,7 @@ run_test "DTLS fragmenting: proxy MTU, AES-CBC EtM renego" \ -c "found fragmented DTLS handshake message" \ -C "error" +not_with_valgrind # spurious resend due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C From 38110dfc0e2f59604f6d39093471ae790323c5dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 17 Aug 2018 12:44:54 +0200 Subject: [PATCH 16/35] Add interop test with unreliable connection Adds a requirement for GNUTLS_NEXT (3.5.3 or above, in practice we should install 3.6.3) on the CI. See internal ref IOTSSL-2401 for analysis of the bugs and their impact on the tests. --- tests/ssl-opt.sh | 217 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 217 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index beceafae6..c27cc25c8 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -41,6 +41,28 @@ G_SRV="$GNUTLS_SERV --x509certfile data_files/server5.crt --x509keyfile data_fil G_CLI="echo 'GET / HTTP/1.0' | $GNUTLS_CLI --x509cafile data_files/test-ca_cat12.crt" TCP_CLIENT="$PERL scripts/tcp_client.pl" +# alternative versions of OpenSSL and GnuTLS (no default path) + +if [ -n "${OPENSSL_LEGACY:-}" ]; then + O_LEGACY_SRV="$OPENSSL_LEGACY s_server -www -cert data_files/server5.crt -key data_files/server5.key" + O_LEGACY_CLI="echo 'GET / HTTP/1.0' | $OPENSSL_LEGACY s_client" +else + O_LEGACY_SRV=false + O_LEGACY_CLI=false +fi + +if [ -n "${GNUTLS_NEXT_SERV}" ]; then + G_NEXT_SRV="$GNUTLS_NEXT_SERV --x509certfile data_files/server5.crt --x509keyfile data_files/server5.key" +else + G_NEXT_SRV=false +fi + +if [ -n "${GNUTLS_NEXT_CLI}" ]; then + G_NEXT_CLI="echo 'GET / HTTP/1.0' | $GNUTLS_NEXT_CLI --x509cafile data_files/test-ca_cat12.crt" +else + G_NEXT_CLI=false +fi + TESTS=0 FAILS=0 SKIPS=0 @@ -163,6 +185,34 @@ requires_gnutls() { fi } +# skip next test if GnuTLS-next isn't available +requires_gnutls_next() { + if [ -z "${GNUTLS_NEXT_AVAILABLE:-}" ]; then + if ( which "${GNUTLS_NEXT_CLI:-}" && which "${GNUTLS_NEXT_SERV:-}" ) >/dev/null 2>&1; then + GNUTLS_NEXT_AVAILABLE="YES" + else + GNUTLS_NEXT_AVAILABLE="NO" + fi + fi + if [ "$GNUTLS_NEXT_AVAILABLE" = "NO" ]; then + SKIP_NEXT="YES" + fi +} + +# skip next test if OpenSSL-legacy isn't available +requires_openssl_legacy() { + if [ -z "${OPENSSL_LEGACY_AVAILABLE:-}" ]; then + if which "${OPENSSL_LEGACY:-}" >/dev/null 2>&1; then + OPENSSL_LEGACY_AVAILABLE="YES" + else + OPENSSL_LEGACY_AVAILABLE="NO" + fi + fi + if [ "$OPENSSL_LEGACY_AVAILABLE" = "NO" ]; then + SKIP_NEXT="YES" + fi +} + # skip next test if IPv6 isn't available on this host requires_ipv6() { if [ -z "${HAS_IPV6:-}" ]; then @@ -717,6 +767,19 @@ O_CLI="$O_CLI -connect localhost:+SRV_PORT" G_SRV="$G_SRV -p $SRV_PORT" G_CLI="$G_CLI -p +SRV_PORT localhost" +if [ -n "${OPENSSL_LEGACY:-}" ]; then + O_LEGACY_SRV="$O_LEGACY_SRV -accept $SRV_PORT -dhparam data_files/dhparams.pem" + O_LEGACY_CLI="$O_LEGACY_CLI -connect localhost:+SRV_PORT" +fi + +if [ -n "${GNUTLS_NEXT_SERV}" ]; then + G_NEXT_SRV="$G_NEXT_SRV -p $SRV_PORT" +fi + +if [ -n "${GNUTLS_NEXT_CLI}" ]; then + G_NEXT_CLI="$G_NEXT_CLI -p +SRV_PORT localhost" +fi + # Allow SHA-1, because many of our test certificates use it P_SRV="$P_SRV allow_sha1=1" P_CLI="$P_CLI allow_sha1=1" @@ -5260,6 +5323,8 @@ run_test "DTLS fragmenting: proxy MTU + 3d" \ -c "found fragmented DTLS handshake message" \ -C "error" +# interop tests for DTLS fragmentating with reliable connection +# # here and below we just want to test that the we fragment in a way that # pleases other implementations, so we don't need the peer to fragment requires_config_enabled MBEDTLS_SSL_PROTO_DTLS @@ -5374,6 +5439,158 @@ run_test "DTLS fragmenting: openssl client, DTLS 1.0" \ 0 \ -s "fragmenting handshake message" +# interop tests for DTLS fragmentating with unreliable connection +# +# again we just want to test that the we fragment in a way that +# pleases other implementations, so we don't need the peer to fragment +requires_gnutls_next +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 +client_needs_more_time 2 +run_test "DTLS fragmenting: 3d, gnutls server, DTLS 1.2" \ + -p "$P_PXY drop=8 delay=8 duplicate=8" \ + "$G_NEXT_SRV -u" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + mtu=512 force_version=dtls1_2" \ + 0 \ + -c "fragmenting handshake message" \ + -C "error" + +requires_gnutls_next +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_1 +client_needs_more_time 2 +run_test "DTLS fragmenting: 3d, gnutls server, DTLS 1.0" \ + -p "$P_PXY drop=8 delay=8 duplicate=8" \ + "$G_NEXT_SRV -u" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + mtu=512 force_version=dtls1_2" \ + 0 \ + -c "fragmenting handshake message" \ + -C "error" + +## The two tests below are disabled due to a bug in GnuTLS client that causes +## handshake failures when the NewSessionTicket message is lost, see +## https://gitlab.com/gnutls/gnutls/issues/543 +## We can re-enable them when a fixed version fo GnuTLS is available +## and installed in our CI system. +## +## # gnutls-cli always tries IPv6 first, and doesn't fall back to IPv4 with DTLS +## requires_ipv6 +## requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +## requires_config_enabled MBEDTLS_RSA_C +## requires_config_enabled MBEDTLS_ECDSA_C +## requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 +## client_needs_more_time 2 +## run_test "DTLS fragmenting: 3d, gnutls client, DTLS 1.2" \ +## -p "$P_PXY drop=8 delay=8 duplicate=8" \ +## "$P_SRV dtls=1 debug_level=2 server_addr=::1 \ +## crt_file=data_files/server7_int-ca.crt \ +## key_file=data_files/server7.key \ +## mtu=512 force_version=dtls1_2" \ +## "$G_CLI -u" \ +## 0 \ +## -s "fragmenting handshake message" +## +## # gnutls-cli always tries IPv6 first, and doesn't fall back to IPv4 with DTLS +## requires_ipv6 +## requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +## requires_config_enabled MBEDTLS_RSA_C +## requires_config_enabled MBEDTLS_ECDSA_C +## requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_1 +## client_needs_more_time 2 +## run_test "DTLS fragmenting: 3d, gnutls client, DTLS 1.0" \ +## -p "$P_PXY drop=8 delay=8 duplicate=8" \ +## "$P_SRV dtls=1 debug_level=2 server_addr=::1 \ +## crt_file=data_files/server7_int-ca.crt \ +## key_file=data_files/server7.key \ +## mtu=512 force_version=dtls1" \ +## "$G_CLI -u" \ +## 0 \ +## -s "fragmenting handshake message" + +## Interop test with OpenSSL might triger a bug in recent versions (that +## probably won't be fixed before 1.1.1X), so we use an old version that +## doesn't have this bug, but unfortunately it doesn't have support for DTLS +## 1.2 either, so the DTLS 1.2 tests are commented for now. +## Bug report: https://github.com/openssl/openssl/issues/6902 +## They should be re-enabled (and the DTLS 1.0 switched back to a non-legacy +## version of OpenSSL once a fixed version of OpenSSL is available) +## +## requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +## requires_config_enabled MBEDTLS_RSA_C +## requires_config_enabled MBEDTLS_ECDSA_C +## requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 +## client_needs_more_time 2 +## run_test "DTLS fragmenting: 3d, openssl server, DTLS 1.2" \ +## -p "$P_PXY drop=8 delay=8 duplicate=8" \ +## "$O_SRV -dtls1_2 -verify 10" \ +## "$P_CLI dtls=1 debug_level=2 \ +## crt_file=data_files/server8_int-ca2.crt \ +## key_file=data_files/server8.key \ +## mtu=512 force_version=dtls1_2" \ +## 0 \ +## -c "fragmenting handshake message" \ +## -C "error" + +requires_openssl_legacy +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_1 +client_needs_more_time 2 +run_test "DTLS fragmenting: 3d, openssl server, DTLS 1.0" \ + -p "$P_PXY drop=8 delay=8 duplicate=8" \ + "$O_LEGACY_SRV -dtls1 -verify 10" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + mtu=512 force_version=dtls1" \ + 0 \ + -c "fragmenting handshake message" \ + -C "error" + +## see comment on the previous-previous test +## requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +## requires_config_enabled MBEDTLS_RSA_C +## requires_config_enabled MBEDTLS_ECDSA_C +## requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 +## client_needs_more_time 2 +## run_test "DTLS fragmenting: 3d, openssl client, DTLS 1.2" \ +## -p "$P_PXY drop=8 delay=8 duplicate=8" \ +## "$P_SRV dtls=1 debug_level=2 \ +## crt_file=data_files/server7_int-ca.crt \ +## key_file=data_files/server7.key \ +## mtu=512 force_version=dtls1_2" \ +## "$O_CLI -dtls1_2" \ +## 0 \ +## -s "fragmenting handshake message" + +# -nbio is added to prevent s_client from blocking in case of duplicated +# messages at the end of the handshake +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_1 +client_needs_more_time 2 +run_test "DTLS fragmenting: 3d, openssl client, DTLS 1.0" \ + -p "$P_PXY drop=8 delay=8 duplicate=8" \ + "$P_SRV dtls=1 debug_level=2 \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + mtu=512 force_version=dtls1" \ + "$O_LEGACY_CLI -nbio -dtls1" \ + 0 \ + -s "fragmenting handshake message" + # Tests for specific things with "unreliable" UDP connection not_with_valgrind # spurious resend due to timeout From 6e7aaca146da9b4945895986abbb91ae3068c811 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 20 Aug 2018 10:37:23 +0200 Subject: [PATCH 17/35] Move MTU setting to SSL context, not config This setting belongs to the individual connection, not to a configuration shared by many connections. (If a default value is desired, that can be handled by the application code that calls mbedtls_ssl_set_mtu().) There are at least two ways in which this matters: - per-connection settings can be adjusted if MTU estimates become available during the lifetime of the connection - it is at least conceivable that a server might recognize restricted clients based on range of IPs and immediately set a lower MTU for them. This is much easier to do with a per-connection setting than by maintaining multiple near-duplicated ssl_config objects that differ only by the MTU setting. --- ChangeLog | 5 ++- include/mbedtls/ssl.h | 74 ++++++++++++++++++++------------------ library/ssl_tls.c | 18 +++++----- programs/ssl/ssl_client2.c | 10 +++--- programs/ssl/ssl_server2.c | 8 +++-- 5 files changed, 63 insertions(+), 52 deletions(-) diff --git a/ChangeLog b/ChangeLog index bab69f676..a95cc6c59 100644 --- a/ChangeLog +++ b/ChangeLog @@ -3,7 +3,10 @@ mbed TLS ChangeLog (Sorted per branch, date) = mbed TLS x.x.x branch released xxxx-xx-xx Features - * Add support for fragmentation of outoing DTLS handshake messages. + * Add support for fragmentation of outgoing DTLS handshake messages. This + is controlled by the maximum fragment length as set locally or negotiated + with the peer, as well as new per-connection MTU option, set using + mbedtls_ssl_set_mtu(). Bugfix * Fixes an issue with MBEDTLS_CHACHAPOLY_C which would not compile if diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index a3b514cd4..69a2e8618 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -958,10 +958,6 @@ struct mbedtls_ssl_config unsigned int dhm_min_bitlen; /*!< min. bit length of the DHM prime */ #endif -#if defined(MBEDTLS_SSL_PROTO_DTLS) - uint16_t mtu; /*!< path mtu, used to fragment outoing messages */ -#endif - unsigned char max_major_ver; /*!< max. major version used */ unsigned char max_minor_ver; /*!< max. minor version used */ unsigned char min_major_ver; /*!< min. major version used */ @@ -1116,6 +1112,10 @@ struct mbedtls_ssl_context size_t out_msglen; /*!< record header: message length */ size_t out_left; /*!< amount of data not yet written */ +#if defined(MBEDTLS_SSL_PROTO_DTLS) + uint16_t mtu; /*!< path mtu, used to fragment outoing messages */ +#endif + #if defined(MBEDTLS_ZLIB_SUPPORT) unsigned char *compress_buf; /*!< zlib data buffer */ #endif @@ -1378,6 +1378,39 @@ void mbedtls_ssl_set_bio( mbedtls_ssl_context *ssl, mbedtls_ssl_recv_t *f_recv, mbedtls_ssl_recv_timeout_t *f_recv_timeout ); +#if defined(MBEDTLS_SSL_PROTO_DTLS) +/** + * \brief Set the Maximum Tranport Unit (MTU). + * Special value: 0 means unset (no limit). + * This represents the maximum size of a datagram payload + * handled by the transport layer (usually UDP) as determined + * by the network link and stack. In practice, this controls + * the maximum size datagram the DTLS layer will pass to the + * \c f_send() callback set using \c mbedtls_ssl_set_bio(). + * + * \note This can be called at any point during the connection, for + * example when a PMTU estimate becomes available from other + * sources, such as lower (or higher) protocol layers. + * + * \note This only controls the size of the packet we send. + * Client-side, you can request the server to use smaller + * records with \c mbedtls_conf_max_frag_len(). + * + * \note If both a MTU and a maximum fragment length have been + * configured (or negotiated with the peer), the lower limit + * is used. + * + * \note Values larger than \c MBEDTLS_SSL_OUT_CONTENT_LEN have no + * effect. This can only be used to decrease the maximum size + * of datagrams sent. Values lower than record layer expansion + * are ignored. + * + * \param ssl SSL context + * \param mtu Value of the path MTU in bytes + */ +void mbedtls_ssl_set_mtu( mbedtls_ssl_context *ssl, uint16_t mtu ); +#endif /* MBEDTLS_SSL_PROTO_DTLS */ + /** * \brief Set the timeout period for mbedtls_ssl_read() * (Default: no timeout.) @@ -2427,35 +2460,6 @@ void mbedtls_ssl_conf_cert_req_ca_list( mbedtls_ssl_config *conf, char cert_req_ca_list ); #endif /* MBEDTLS_SSL_SRV_C */ -#if defined(MBEDTLS_SSL_PROTO_DTLS) -/** - * \brief Set the Maximum Tranport Unit (MTU). - * Special value: 0 means unset (no limit). - * This represents the maximum size of a datagram payload - * handled by the transport layer (usually UDP) as determined - * by the network link and stack. In practice, this controls - * the maximum size datagram the DTLS layer will pass to the - * \c f_send() callback set using \c mbedtls_ssl_set_bio(). - * - * \note This only controls the size of the packet we send. - * Client-side, you can request the server to use smaller - * records with \c mbedtls_conf_max_frag_len(). - * - * \note If both a MTU and a maximum fragment length have been - * configured (or negotiated with the peer), the lower limit - * is used. - * - * \note Values larger than \c MBEDTLS_SSL_OUT_CONTENT_LEN have no - * effect. This can only be used to decrease the maximum size - * of datagrams sent. Values lower than record layer expansion - * are ignored. - * - * \param conf SSL configuration - * \param mtu Value of the path MTU in bytes - */ -void mbedtls_ssl_conf_mtu( mbedtls_ssl_config *conf, uint16_t mtu ); -#endif /* MBEDTLS_SSL_PROTO_DTLS */ - #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) /** * \brief Set the maximum fragment length to emit and/or negotiate @@ -2476,7 +2480,7 @@ void mbedtls_ssl_conf_mtu( mbedtls_ssl_config *conf, uint16_t mtu ); * * \note For DTLS, it is also possible to set a limit for the total * size of daragrams passed to the transport layer, including - * record overhead, see \c mbedtls_ssl_conf_mtu(). + * record overhead, see \c mbedtls_ssl_set_mtu(). * * \param conf SSL configuration * \param mfl_code Code for maximum fragment length (allowed values: @@ -2784,7 +2788,7 @@ size_t mbedtls_ssl_get_max_frag_len( const mbedtls_ssl_context *ssl ); * \note This function is not available (always returns an error) * when record compression is enabled. * - * \sa mbedtls_ssl_conf_mtu() + * \sa mbedtls_ssl_set_mtu() * \sa mbedtls_ssl_get_max_frag_len() * \sa mbedtls_ssl_get_record_expansion() * diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 530f283b4..7f85ddff1 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6270,6 +6270,13 @@ void mbedtls_ssl_set_bio( mbedtls_ssl_context *ssl, ssl->f_recv_timeout = f_recv_timeout; } +#if defined(MBEDTLS_SSL_PROTO_DTLS) +void mbedtls_ssl_set_mtu( mbedtls_ssl_context *ssl, uint16_t mtu ) +{ + ssl->mtu = mtu; +} +#endif + void mbedtls_ssl_conf_read_timeout( mbedtls_ssl_config *conf, uint32_t timeout ) { conf->read_timeout = timeout; @@ -6758,13 +6765,6 @@ void mbedtls_ssl_conf_arc4_support( mbedtls_ssl_config *conf, char arc4 ) } #endif -#if defined(MBEDTLS_SSL_PROTO_DTLS) -void mbedtls_ssl_conf_mtu( mbedtls_ssl_config *conf, uint16_t mtu ) -{ - conf->mtu = mtu; -} -#endif - #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) int mbedtls_ssl_conf_max_frag_len( mbedtls_ssl_config *conf, unsigned char mfl_code ) { @@ -7101,9 +7101,9 @@ int mbedtls_ssl_get_max_out_record_payload( const mbedtls_ssl_context *ssl ) #endif #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->mtu != 0 ) + if( ssl->mtu != 0 ) { - const size_t mtu = ssl->conf->mtu; + const size_t mtu = ssl->mtu; const int ret = mbedtls_ssl_get_record_expansion( ssl ); const size_t overhead = (size_t) ret; diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 7cdc53a54..e4a7412a9 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -1337,10 +1337,7 @@ int main( int argc, char *argv[] ) if( opt.hs_to_min != DFL_HS_TO_MIN || opt.hs_to_max != DFL_HS_TO_MAX ) mbedtls_ssl_conf_handshake_timeout( &conf, opt.hs_to_min, opt.hs_to_max ); - - if( opt.dtls_mtu != DFL_DTLS_MTU ) - mbedtls_ssl_conf_mtu( &conf, opt.dtls_mtu ); -#endif /* MBEDTLS_SSL_PROTO_DTLS */ +#endif #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) if( ( ret = mbedtls_ssl_conf_max_frag_len( &conf, opt.mfl_code ) ) != 0 ) @@ -1498,6 +1495,11 @@ int main( int argc, char *argv[] ) mbedtls_net_send, mbedtls_net_recv, opt.nbio == 0 ? mbedtls_net_recv_timeout : NULL ); +#if defined(MBEDTLS_SSL_PROTO_DTLS) + if( opt.dtls_mtu != DFL_DTLS_MTU ) + mbedtls_ssl_set_mtu( &ssl, opt.dtls_mtu ); +#endif + #if defined(MBEDTLS_TIMING_C) mbedtls_ssl_set_timer_cb( &ssl, &timer, mbedtls_timing_set_delay, mbedtls_timing_get_delay ); diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 484f84fdd..71ec85bd3 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -2165,9 +2165,6 @@ int main( int argc, char *argv[] ) #if defined(MBEDTLS_SSL_PROTO_DTLS) if( opt.hs_to_min != DFL_HS_TO_MIN || opt.hs_to_max != DFL_HS_TO_MAX ) mbedtls_ssl_conf_handshake_timeout( &conf, opt.hs_to_min, opt.hs_to_max ); - - if( opt.dtls_mtu != DFL_DTLS_MTU ) - mbedtls_ssl_conf_mtu( &conf, opt.dtls_mtu ); #endif /* MBEDTLS_SSL_PROTO_DTLS */ #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) @@ -2486,6 +2483,11 @@ int main( int argc, char *argv[] ) mbedtls_ssl_set_bio( &ssl, &client_fd, mbedtls_net_send, mbedtls_net_recv, opt.nbio == 0 ? mbedtls_net_recv_timeout : NULL ); +#if defined(MBEDTLS_SSL_PROTO_DTLS) + if( opt.dtls_mtu != DFL_DTLS_MTU ) + mbedtls_ssl_set_mtu( &ssl, opt.dtls_mtu ); +#endif + #if defined(MBEDTLS_TIMING_C) mbedtls_ssl_set_timer_cb( &ssl, &timer, mbedtls_timing_set_delay, mbedtls_timing_get_delay ); From 02f3a8a921ba4aec77238eaa43305cebf1520eb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 20 Aug 2018 10:49:28 +0200 Subject: [PATCH 18/35] Adjust timeout values for 3d test Use the same values as other 3d tests: this makes the test hopefully a bit faster than the default values, while not increasing the failure rate. While at it: - adjust "needs_more_time" setting for 3d interop tests (we can't set the timeout values for other implementations, so the test might be slow) - fix some supposedly DTLS 1.0 test that were using dtls1_2 on the command line --- tests/ssl-opt.sh | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index c27cc25c8..e966649d1 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5313,11 +5313,11 @@ run_test "DTLS fragmenting: proxy MTU + 3d" \ "$P_SRV dtls=1 debug_level=2 auth_mode=required \ crt_file=data_files/server7_int-ca.crt \ key_file=data_files/server7.key \ - mtu=512" \ + hs_timeout=250-10000 mtu=512" \ "$P_CLI dtls=1 debug_level=2 \ crt_file=data_files/server8_int-ca2.crt \ key_file=data_files/server8.key \ - mtu=512" \ + hs_timeout=250-10000 mtu=512" \ 0 \ -s "found fragmented DTLS handshake message" \ -c "found fragmented DTLS handshake message" \ @@ -5350,7 +5350,7 @@ run_test "DTLS fragmenting: gnutls server, DTLS 1.0" \ "$P_CLI dtls=1 debug_level=2 \ crt_file=data_files/server8_int-ca2.crt \ key_file=data_files/server8.key \ - mtu=512 force_version=dtls1_2" \ + mtu=512 force_version=dtls1" \ 0 \ -c "fragmenting handshake message" \ -C "error" @@ -5448,14 +5448,14 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 -client_needs_more_time 2 +client_needs_more_time 4 run_test "DTLS fragmenting: 3d, gnutls server, DTLS 1.2" \ -p "$P_PXY drop=8 delay=8 duplicate=8" \ "$G_NEXT_SRV -u" \ "$P_CLI dtls=1 debug_level=2 \ crt_file=data_files/server8_int-ca2.crt \ key_file=data_files/server8.key \ - mtu=512 force_version=dtls1_2" \ + hs_timeout=250-60000 mtu=512 force_version=dtls1_2" \ 0 \ -c "fragmenting handshake message" \ -C "error" @@ -5465,14 +5465,14 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_1 -client_needs_more_time 2 +client_needs_more_time 4 run_test "DTLS fragmenting: 3d, gnutls server, DTLS 1.0" \ -p "$P_PXY drop=8 delay=8 duplicate=8" \ "$G_NEXT_SRV -u" \ "$P_CLI dtls=1 debug_level=2 \ crt_file=data_files/server8_int-ca2.crt \ key_file=data_files/server8.key \ - mtu=512 force_version=dtls1_2" \ + hs_timeout=250-60000 mtu=512 force_version=dtls1" \ 0 \ -c "fragmenting handshake message" \ -C "error" @@ -5489,13 +5489,13 @@ run_test "DTLS fragmenting: 3d, gnutls server, DTLS 1.0" \ ## requires_config_enabled MBEDTLS_RSA_C ## requires_config_enabled MBEDTLS_ECDSA_C ## requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 -## client_needs_more_time 2 +## client_needs_more_time 4 ## run_test "DTLS fragmenting: 3d, gnutls client, DTLS 1.2" \ ## -p "$P_PXY drop=8 delay=8 duplicate=8" \ ## "$P_SRV dtls=1 debug_level=2 server_addr=::1 \ ## crt_file=data_files/server7_int-ca.crt \ ## key_file=data_files/server7.key \ -## mtu=512 force_version=dtls1_2" \ +## hs_timeout=250-60000 mtu=512 force_version=dtls1_2" \ ## "$G_CLI -u" \ ## 0 \ ## -s "fragmenting handshake message" @@ -5506,13 +5506,13 @@ run_test "DTLS fragmenting: 3d, gnutls server, DTLS 1.0" \ ## requires_config_enabled MBEDTLS_RSA_C ## requires_config_enabled MBEDTLS_ECDSA_C ## requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_1 -## client_needs_more_time 2 +## client_needs_more_time 4 ## run_test "DTLS fragmenting: 3d, gnutls client, DTLS 1.0" \ ## -p "$P_PXY drop=8 delay=8 duplicate=8" \ ## "$P_SRV dtls=1 debug_level=2 server_addr=::1 \ ## crt_file=data_files/server7_int-ca.crt \ ## key_file=data_files/server7.key \ -## mtu=512 force_version=dtls1" \ +## hs_timeout=250-60000 mtu=512 force_version=dtls1" \ ## "$G_CLI -u" \ ## 0 \ ## -s "fragmenting handshake message" @@ -5529,14 +5529,14 @@ run_test "DTLS fragmenting: 3d, gnutls server, DTLS 1.0" \ ## requires_config_enabled MBEDTLS_RSA_C ## requires_config_enabled MBEDTLS_ECDSA_C ## requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 -## client_needs_more_time 2 +## client_needs_more_time 4 ## run_test "DTLS fragmenting: 3d, openssl server, DTLS 1.2" \ ## -p "$P_PXY drop=8 delay=8 duplicate=8" \ ## "$O_SRV -dtls1_2 -verify 10" \ ## "$P_CLI dtls=1 debug_level=2 \ ## crt_file=data_files/server8_int-ca2.crt \ ## key_file=data_files/server8.key \ -## mtu=512 force_version=dtls1_2" \ +## hs_timeout=250-60000 mtu=512 force_version=dtls1_2" \ ## 0 \ ## -c "fragmenting handshake message" \ ## -C "error" @@ -5546,14 +5546,14 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_1 -client_needs_more_time 2 +client_needs_more_time 4 run_test "DTLS fragmenting: 3d, openssl server, DTLS 1.0" \ -p "$P_PXY drop=8 delay=8 duplicate=8" \ "$O_LEGACY_SRV -dtls1 -verify 10" \ "$P_CLI dtls=1 debug_level=2 \ crt_file=data_files/server8_int-ca2.crt \ key_file=data_files/server8.key \ - mtu=512 force_version=dtls1" \ + hs_timeout=250-60000 mtu=512 force_version=dtls1" \ 0 \ -c "fragmenting handshake message" \ -C "error" @@ -5563,13 +5563,13 @@ run_test "DTLS fragmenting: 3d, openssl server, DTLS 1.0" \ ## requires_config_enabled MBEDTLS_RSA_C ## requires_config_enabled MBEDTLS_ECDSA_C ## requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 -## client_needs_more_time 2 +## client_needs_more_time 4 ## run_test "DTLS fragmenting: 3d, openssl client, DTLS 1.2" \ ## -p "$P_PXY drop=8 delay=8 duplicate=8" \ ## "$P_SRV dtls=1 debug_level=2 \ ## crt_file=data_files/server7_int-ca.crt \ ## key_file=data_files/server7.key \ -## mtu=512 force_version=dtls1_2" \ +## hs_timeout=250-60000 mtu=512 force_version=dtls1_2" \ ## "$O_CLI -dtls1_2" \ ## 0 \ ## -s "fragmenting handshake message" @@ -5580,13 +5580,13 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_1 -client_needs_more_time 2 +client_needs_more_time 4 run_test "DTLS fragmenting: 3d, openssl client, DTLS 1.0" \ -p "$P_PXY drop=8 delay=8 duplicate=8" \ "$P_SRV dtls=1 debug_level=2 \ crt_file=data_files/server7_int-ca.crt \ key_file=data_files/server7.key \ - mtu=512 force_version=dtls1" \ + hs_timeout=250-60000 mtu=512 force_version=dtls1" \ "$O_LEGACY_CLI -nbio -dtls1" \ 0 \ -s "fragmenting handshake message" From 065a2a3472e6d24be99bfcde65931dbfa75f4c86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 20 Aug 2018 11:09:26 +0200 Subject: [PATCH 19/35] Fix some typos and links in comments and doc --- include/mbedtls/ssl.h | 6 +++--- library/ssl_tls.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 69a2e8618..1d392ab31 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1113,7 +1113,7 @@ struct mbedtls_ssl_context size_t out_left; /*!< amount of data not yet written */ #if defined(MBEDTLS_SSL_PROTO_DTLS) - uint16_t mtu; /*!< path mtu, used to fragment outoing messages */ + uint16_t mtu; /*!< path mtu, used to fragment outgoing messages */ #endif #if defined(MBEDTLS_ZLIB_SUPPORT) @@ -1394,13 +1394,13 @@ void mbedtls_ssl_set_bio( mbedtls_ssl_context *ssl, * * \note This only controls the size of the packet we send. * Client-side, you can request the server to use smaller - * records with \c mbedtls_conf_max_frag_len(). + * records with \c mbedtls_ssl_conf_max_frag_len(). * * \note If both a MTU and a maximum fragment length have been * configured (or negotiated with the peer), the lower limit * is used. * - * \note Values larger than \c MBEDTLS_SSL_OUT_CONTENT_LEN have no + * \note Values larger than #MBEDTLS_SSL_OUT_CONTENT_LEN have no * effect. This can only be used to decrease the maximum size * of datagrams sent. Values lower than record layer expansion * are ignored. diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 7f85ddff1..5f3abe597 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3034,7 +3034,7 @@ void mbedtls_ssl_send_flight_completed( mbedtls_ssl_context *ssl ) * - ssl->out_msg[0]: the handshake type (ClientHello, ServerHello, etc) * - ssl->out_msg + 4: the handshake message body * - * Ouputs, ie state before passing to flight_append() or write_record(): + * Outputs, ie state before passing to flight_append() or write_record(): * - ssl->out_msglen: the length of the record contents * (including handshake headers but excluding record headers) * - ssl->out_msg: the record contents (handshake headers + content) From 050dd6ad354f89f9e20ff94483a40526e520ccfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 20 Aug 2018 11:16:40 +0200 Subject: [PATCH 20/35] Improve documentation of ssl_set_mtu(). --- include/mbedtls/ssl.h | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 1d392ab31..f563437d1 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1392,18 +1392,25 @@ void mbedtls_ssl_set_bio( mbedtls_ssl_context *ssl, * example when a PMTU estimate becomes available from other * sources, such as lower (or higher) protocol layers. * - * \note This only controls the size of the packet we send. + * \note This only controls the size of the packets we send. * Client-side, you can request the server to use smaller * records with \c mbedtls_ssl_conf_max_frag_len(). * * \note If both a MTU and a maximum fragment length have been - * configured (or negotiated with the peer), the lower limit - * is used. + * configured (or negotiated with the peer), the resulting + * lower limit (after translating the MTU setting to a limit + * on the record content length) is used. * - * \note Values larger than #MBEDTLS_SSL_OUT_CONTENT_LEN have no - * effect. This can only be used to decrease the maximum size - * of datagrams sent. Values lower than record layer expansion - * are ignored. + * \note This can only be used to decrease the maximum size + * of datagrams sent. It cannot be used to increase the + * maximum size of records over the limit set by + * #MBEDTLS_SSL_OUT_CONTENT_LEN. + * + * \note Values lower than the current record layer expansion will + * result in an error when trying to send data. + * + * \note Using record compression together with a non-zero MTU value + * will result in an error when trying to send data. * * \param ssl SSL context * \param mtu Value of the path MTU in bytes From 58e9dc3d4bf2d30be1eddf96b161ab3571df03b7 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 17 Aug 2018 15:53:21 +0100 Subject: [PATCH 21/35] Allow GNUTLS_NEXT_CLI / GNUTLS_NEXT_SERV to be unset in ssl-opt.sh --- tests/ssl-opt.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index e966649d1..205cc5dd1 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -51,13 +51,13 @@ else O_LEGACY_CLI=false fi -if [ -n "${GNUTLS_NEXT_SERV}" ]; then +if [ -n "${GNUTLS_NEXT_SERV:-}" ]; then G_NEXT_SRV="$GNUTLS_NEXT_SERV --x509certfile data_files/server5.crt --x509keyfile data_files/server5.key" else G_NEXT_SRV=false fi -if [ -n "${GNUTLS_NEXT_CLI}" ]; then +if [ -n "${GNUTLS_NEXT_CLI:-}" ]; then G_NEXT_CLI="echo 'GET / HTTP/1.0' | $GNUTLS_NEXT_CLI --x509cafile data_files/test-ca_cat12.crt" else G_NEXT_CLI=false @@ -772,11 +772,11 @@ if [ -n "${OPENSSL_LEGACY:-}" ]; then O_LEGACY_CLI="$O_LEGACY_CLI -connect localhost:+SRV_PORT" fi -if [ -n "${GNUTLS_NEXT_SERV}" ]; then +if [ -n "${GNUTLS_NEXT_SERV:-}" ]; then G_NEXT_SRV="$G_NEXT_SRV -p $SRV_PORT" fi -if [ -n "${GNUTLS_NEXT_CLI}" ]; then +if [ -n "${GNUTLS_NEXT_CLI:-}" ]; then G_NEXT_CLI="$G_NEXT_CLI -p +SRV_PORT localhost" fi From 982931523551b8b5e7e5db1f95eebe0c47ebdb30 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 17 Aug 2018 16:10:47 +0100 Subject: [PATCH 22/35] Add missing dependency in ssl-opt.sh --- tests/ssl-opt.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 205cc5dd1..9ff0795bc 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5576,6 +5576,7 @@ run_test "DTLS fragmenting: 3d, openssl server, DTLS 1.0" \ # -nbio is added to prevent s_client from blocking in case of duplicated # messages at the end of the handshake +requires_openssl_legacy requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C From 4532329397dc3201c292a628d4e875a3e7ca6569 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 20 Aug 2018 11:52:24 +0200 Subject: [PATCH 23/35] Add proxy-enforcement to a MTU test --- tests/ssl-opt.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 9ff0795bc..f1c19828b 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5097,6 +5097,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C run_test "DTLS fragmenting: both (MTU)" \ + -p "$P_PXY mtu=512" \ "$P_SRV dtls=1 debug_level=2 auth_mode=required \ crt_file=data_files/server7_int-ca.crt \ key_file=data_files/server7.key \ From a1071a58a3606e755e1e9832300bd4a35493e42b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 20 Aug 2018 11:56:14 +0200 Subject: [PATCH 24/35] Compute record expansion at the right time Depends on the current transform, which might change when retransmitting a flight containing a Finished message, so compute it only after the transform is swapped. --- library/ssl_tls.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 5f3abe597..da21db237 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -2845,20 +2845,8 @@ int mbedtls_ssl_resend( mbedtls_ssl_context *ssl ) */ int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ) { - const int ret_payload = mbedtls_ssl_get_max_out_record_payload( ssl ); - const size_t max_record_payload = (size_t) ret_payload; - /* DTLS handshake headers are 12 bytes */ - const size_t max_hs_fragment_len = max_record_payload - 12; - MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> mbedtls_ssl_flight_transmit" ) ); - if( ret_payload < 0 ) - { - MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_get_max_out_record_payload", - ret_payload ); - return( ret_payload ); - } - if( ssl->handshake->retransmit_state != MBEDTLS_SSL_RETRANS_SENDING ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "initialise flight transmission" ) ); @@ -2895,6 +2883,10 @@ int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ) } else { + const int ret_payload = mbedtls_ssl_get_max_out_record_payload( ssl ); + const size_t max_record_payload = (size_t) ret_payload; + /* DTLS handshake headers are 12 bytes */ + const size_t max_hs_fragment_len = max_record_payload - 12; const unsigned char * const p = ssl->handshake->cur_msg_p; const size_t hs_len = cur->len - 12; const size_t frag_off = p - ( cur->p + 12 ); @@ -2902,6 +2894,13 @@ int mbedtls_ssl_flight_transmit( mbedtls_ssl_context *ssl ) const size_t frag_len = rem_len > max_hs_fragment_len ? max_hs_fragment_len : rem_len; + if( ret_payload < 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_get_max_out_record_payload", + ret_payload ); + return( ret_payload ); + } + if( frag_off == 0 && frag_len != hs_len ) { MBEDTLS_SSL_DEBUG_MSG( 2, ( "fragmenting handshake message (%u > %u)", From 615129839558690d2bca8fbdcc1ca885ee8d208e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 21 Aug 2018 09:40:07 +0200 Subject: [PATCH 25/35] Add missing requires_gnutls guards --- tests/ssl-opt.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index f1c19828b..4a6234803 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5332,6 +5332,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 +requires_gnutls run_test "DTLS fragmenting: gnutls server, DTLS 1.2" \ "$G_SRV -u" \ "$P_CLI dtls=1 debug_level=2 \ @@ -5346,6 +5347,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_1 +requires_gnutls run_test "DTLS fragmenting: gnutls server, DTLS 1.0" \ "$G_SRV -u" \ "$P_CLI dtls=1 debug_level=2 \ @@ -5362,6 +5364,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 +requires_gnutls run_test "DTLS fragmenting: gnutls client, DTLS 1.2" \ "$P_SRV dtls=1 debug_level=2 server_addr=::1 \ crt_file=data_files/server7_int-ca.crt \ @@ -5377,6 +5380,7 @@ requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_1 +requires_gnutls run_test "DTLS fragmenting: gnutls client, DTLS 1.0" \ "$P_SRV dtls=1 debug_level=2 server_addr=::1 \ crt_file=data_files/server7_int-ca.crt \ @@ -5486,6 +5490,7 @@ run_test "DTLS fragmenting: 3d, gnutls server, DTLS 1.0" \ ## ## # gnutls-cli always tries IPv6 first, and doesn't fall back to IPv4 with DTLS ## requires_ipv6 +## requires_gnutls ## requires_config_enabled MBEDTLS_SSL_PROTO_DTLS ## requires_config_enabled MBEDTLS_RSA_C ## requires_config_enabled MBEDTLS_ECDSA_C @@ -5503,6 +5508,7 @@ run_test "DTLS fragmenting: 3d, gnutls server, DTLS 1.0" \ ## ## # gnutls-cli always tries IPv6 first, and doesn't fall back to IPv4 with DTLS ## requires_ipv6 +## requires_gnutls ## requires_config_enabled MBEDTLS_SSL_PROTO_DTLS ## requires_config_enabled MBEDTLS_RSA_C ## requires_config_enabled MBEDTLS_ECDSA_C From f2f1d40d6d96fd5f7c0973d91b5620d30a6e0913 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 21 Aug 2018 09:53:22 +0200 Subject: [PATCH 26/35] Improve wording in ChangeLog and documentation --- ChangeLog | 2 +- include/mbedtls/ssl.h | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index a95cc6c59..3f144a7e9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -5,7 +5,7 @@ mbed TLS ChangeLog (Sorted per branch, date) Features * Add support for fragmentation of outgoing DTLS handshake messages. This is controlled by the maximum fragment length as set locally or negotiated - with the peer, as well as new per-connection MTU option, set using + with the peer, as well as by a new per-connection MTU option, set using mbedtls_ssl_set_mtu(). Bugfix diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index f563437d1..4471de507 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1392,9 +1392,11 @@ void mbedtls_ssl_set_bio( mbedtls_ssl_context *ssl, * example when a PMTU estimate becomes available from other * sources, such as lower (or higher) protocol layers. * - * \note This only controls the size of the packets we send. - * Client-side, you can request the server to use smaller - * records with \c mbedtls_ssl_conf_max_frag_len(). + * \note This setting only controls the size of the packets we send, + * and does not restrict the size of the datagrams we're + * willing to receive. Client-side, you can request the + * server to use smaller records with \c + * mbedtls_ssl_conf_max_frag_len(). * * \note If both a MTU and a maximum fragment length have been * configured (or negotiated with the peer), the resulting @@ -1402,7 +1404,8 @@ void mbedtls_ssl_set_bio( mbedtls_ssl_context *ssl, * on the record content length) is used. * * \note This can only be used to decrease the maximum size - * of datagrams sent. It cannot be used to increase the + * of datagrams (hence records, as records cannot span + * multiple datagrams) sent. It cannot be used to increase the * maximum size of records over the limit set by * #MBEDTLS_SSL_OUT_CONTENT_LEN. * From 000281e07d796576d615243b5883b243f22dc53f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 21 Aug 2018 11:20:58 +0200 Subject: [PATCH 27/35] Fix "unused parameter" warning in small configs --- library/ssl_tls.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index da21db237..faa9467e1 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7092,6 +7092,11 @@ int mbedtls_ssl_get_max_out_record_payload( const mbedtls_ssl_context *ssl ) { size_t max_len = MBEDTLS_SSL_OUT_CONTENT_LEN; +#if !defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) && \ + !defined(MBEDTLS_SSL_PROTO_DTLS) + (void) ssl; +#endif + #if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) const size_t mfl = mbedtls_ssl_get_max_frag_len( ssl ); From 661103595e90529a2a3fc0af3648331f02b1af30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 21 Aug 2018 11:55:40 +0200 Subject: [PATCH 28/35] Try to further clarify documentation --- include/mbedtls/ssl.h | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 4471de507..35f4d320a 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1388,6 +1388,10 @@ void mbedtls_ssl_set_bio( mbedtls_ssl_context *ssl, * the maximum size datagram the DTLS layer will pass to the * \c f_send() callback set using \c mbedtls_ssl_set_bio(). * + * \note The limit on datagram size is converted to a limit on + * record payload by subtracting the current overhead of + * encapsulation and encryption/authentication if any. + * * \note This can be called at any point during the connection, for * example when a PMTU estimate becomes available from other * sources, such as lower (or higher) protocol layers. @@ -1400,14 +1404,12 @@ void mbedtls_ssl_set_bio( mbedtls_ssl_context *ssl, * * \note If both a MTU and a maximum fragment length have been * configured (or negotiated with the peer), the resulting - * lower limit (after translating the MTU setting to a limit - * on the record content length) is used. + * lower limit on record payload (see first note) is used. * * \note This can only be used to decrease the maximum size - * of datagrams (hence records, as records cannot span - * multiple datagrams) sent. It cannot be used to increase the - * maximum size of records over the limit set by - * #MBEDTLS_SSL_OUT_CONTENT_LEN. + * of datagrams (hence records, see first note) sent. It + * cannot be used to increase the maximum size of records over + * the limit set by #MBEDTLS_SSL_OUT_CONTENT_LEN. * * \note Values lower than the current record layer expansion will * result in an error when trying to send data. From 2f2d9020cd4eaab26b4159fd87e1220211e35a63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 21 Aug 2018 12:17:54 +0200 Subject: [PATCH 29/35] Add delay in test to avoid race condition We previously observed random-looking failures from this test. I think they were caused by a race condition where the client tries to reconnect while the server is still closing the connection and has not yet returned to an accepting state. In that case, the server would fail to see and reply to the ClientHello, and the client would have to resend it. I believe logs of failing runs are compatible with this interpretation: - the proxy logs show the new ClientHello and the server's closing Alert are sent the same millisecond. - the client logs show the server's closing Alert is received after the new handshake has been started (discarding message from wrong epoch). The attempted fix is for the client to wait a bit before reconnecting, which should vastly enhance the probability of the server reaching its accepting state before the client tries to reconnect. The value of 1 second is arbitrary but should be more than enough even on loaded machines. The test was run locally 100 times in a row on a slightly loaded machine (an instance of all.sh running in parallel) without any failure after this fix. --- tests/ssl-opt.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 4a6234803..f811789e6 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5139,6 +5139,8 @@ run_test "DTLS fragmenting: proxy MTU, simple handshake" \ # Since we don't support reading fragmented ClientHello yet, # up the MTU to 1450 (larger than ClientHello with session ticket, # but still smaller than client's Certificate to ensure fragmentation). +# reco_delay avoids races where the client reconnects before the server has +# resumed listening, which would result in a spurious resend. not_with_valgrind # spurious resend due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C @@ -5152,7 +5154,7 @@ run_test "DTLS fragmenting: proxy MTU, resumed handshake" \ "$P_CLI dtls=1 debug_level=2 \ crt_file=data_files/server8_int-ca2.crt \ key_file=data_files/server8.key \ - mtu=1450 reconnect=1" \ + mtu=1450 reconnect=1 reco_delay=1" \ 0 \ -S "resend" \ -C "resend" \ From 3d183cefb5bbc3e37fa033c2c85fdcde127a296c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 22 Aug 2018 09:56:22 +0200 Subject: [PATCH 30/35] Allow client-side resend in proxy MTU tests From Hanno: When a server replies to a cookieless ClientHello with a HelloVerifyRequest, it is supposed to reset the connection and wait for a subsequent ClientHello which includes the cookie from the HelloVerifyRequest. In testing environments, it might happen that the reset of the server takes longer than for the client to replying to the HelloVerifyRequest with the ClientHello+Cookie. In this case, the ClientHello gets lost and the client will need retransmit. This may happen even if the underlying datagram transport is reliable. --- tests/ssl-opt.sh | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index f811789e6..8cf0c82a6 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5112,6 +5112,8 @@ run_test "DTLS fragmenting: both (MTU)" \ -C "error" # the proxy shouldn't drop or mess up anything, so we shouldn't need to resend +# OTOH the client might resend if the server is to slow to reset after sending +# a HelloVerifyRequest, so only check for no retransmission server-side not_with_valgrind # spurious resend due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C @@ -5128,7 +5130,6 @@ run_test "DTLS fragmenting: proxy MTU, simple handshake" \ mtu=512" \ 0 \ -S "resend" \ - -C "resend" \ -s "found fragmented DTLS handshake message" \ -c "found fragmented DTLS handshake message" \ -C "error" @@ -5157,7 +5158,6 @@ run_test "DTLS fragmenting: proxy MTU, resumed handshake" \ mtu=1450 reconnect=1 reco_delay=1" \ 0 \ -S "resend" \ - -C "resend" \ -s "found fragmented DTLS handshake message" \ -c "found fragmented DTLS handshake message" \ -C "error" @@ -5185,7 +5185,6 @@ run_test "DTLS fragmenting: proxy MTU, ChachaPoly renego" \ mtu=512" \ 0 \ -S "resend" \ - -C "resend" \ -s "found fragmented DTLS handshake message" \ -c "found fragmented DTLS handshake message" \ -C "error" @@ -5214,7 +5213,6 @@ run_test "DTLS fragmenting: proxy MTU, AES-GCM renego" \ mtu=512" \ 0 \ -S "resend" \ - -C "resend" \ -s "found fragmented DTLS handshake message" \ -c "found fragmented DTLS handshake message" \ -C "error" @@ -5243,7 +5241,6 @@ run_test "DTLS fragmenting: proxy MTU, AES-CCM renego" \ mtu=512" \ 0 \ -S "resend" \ - -C "resend" \ -s "found fragmented DTLS handshake message" \ -c "found fragmented DTLS handshake message" \ -C "error" @@ -5273,7 +5270,6 @@ run_test "DTLS fragmenting: proxy MTU, AES-CBC EtM renego" \ mtu=512" \ 0 \ -S "resend" \ - -C "resend" \ -s "found fragmented DTLS handshake message" \ -c "found fragmented DTLS handshake message" \ -C "error" @@ -5302,7 +5298,6 @@ run_test "DTLS fragmenting: proxy MTU, AES-CBC non-EtM renego" \ mtu=512" \ 0 \ -S "resend" \ - -C "resend" \ -s "found fragmented DTLS handshake message" \ -c "found fragmented DTLS handshake message" \ -C "error" From c1d54b74ec756186e373a266e4cfc453225b0708 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 22 Aug 2018 10:02:59 +0200 Subject: [PATCH 31/35] Add tests with non-blocking I/O Make sure we behave properly when f_send() or f_recv() return MBEDTLS_ERR_SSL_WANT_{WRITE,READ}. --- tests/ssl-opt.sh | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 8cf0c82a6..ec2717ad5 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5134,6 +5134,26 @@ run_test "DTLS fragmenting: proxy MTU, simple handshake" \ -c "found fragmented DTLS handshake message" \ -C "error" +not_with_valgrind # spurious resend due to timeout +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +run_test "DTLS fragmenting: proxy MTU, simple handshake, nbio" \ + -p "$P_PXY mtu=512" \ + "$P_SRV dtls=1 debug_level=2 auth_mode=required \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + mtu=512 nbio=2" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + mtu=512 nbio=2" \ + 0 \ + -S "resend" \ + -s "found fragmented DTLS handshake message" \ + -c "found fragmented DTLS handshake message" \ + -C "error" + # This ensures things still work after session_reset(), # for example it would have caught #1941. # It also exercises the "resumed hanshake" flow. @@ -5321,6 +5341,25 @@ run_test "DTLS fragmenting: proxy MTU + 3d" \ -c "found fragmented DTLS handshake message" \ -C "error" +requires_config_enabled MBEDTLS_SSL_PROTO_DTLS +requires_config_enabled MBEDTLS_RSA_C +requires_config_enabled MBEDTLS_ECDSA_C +client_needs_more_time 2 +run_test "DTLS fragmenting: proxy MTU + 3d, nbio" \ + -p "$P_PXY mtu=512 drop=8 delay=8 duplicate=8" \ + "$P_SRV dtls=1 debug_level=2 auth_mode=required \ + crt_file=data_files/server7_int-ca.crt \ + key_file=data_files/server7.key \ + hs_timeout=250-10000 mtu=512 nbio=2" \ + "$P_CLI dtls=1 debug_level=2 \ + crt_file=data_files/server8_int-ca2.crt \ + key_file=data_files/server8.key \ + hs_timeout=250-10000 mtu=512 nbio=2" \ + 0 \ + -s "found fragmented DTLS handshake message" \ + -c "found fragmented DTLS handshake message" \ + -C "error" + # interop tests for DTLS fragmentating with reliable connection # # here and below we just want to test that the we fragment in a way that From 68ae351dbec53e8e6b5eae3ff1392952055f1a2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 22 Aug 2018 10:24:31 +0200 Subject: [PATCH 32/35] Fix some whitespace in documentation --- include/mbedtls/ssl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 35f4d320a..090660733 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1398,7 +1398,7 @@ void mbedtls_ssl_set_bio( mbedtls_ssl_context *ssl, * * \note This setting only controls the size of the packets we send, * and does not restrict the size of the datagrams we're - * willing to receive. Client-side, you can request the + * willing to receive. Client-side, you can request the * server to use smaller records with \c * mbedtls_ssl_conf_max_frag_len(). * From 34aa187df6a914d94d56d8b3aeab5692a1a3d59c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 23 Aug 2018 19:07:15 +0200 Subject: [PATCH 33/35] Force IPv4 for gnutls-cli DTLS tests Depending on the settings of the local machine, gnutls-cli will either try IPv4 or IPv6 when trying to connect to localhost. With TLS, whatever it tries first, it will notice if any failure happens and try the other protocol if necessary. With DTLS it can't do that. Unfortunately for now there isn't really any good way to specify an address and hostname independently, though that might come soon: https://gitlab.com/gnutls/gnutls/issues/344 A work around is to specify an address directly and then use --insecure to ignore certificate hostname mismatch; that is OK for tests that are completely unrelated to certificate verification (such as the recent fragmenting tests) but unacceptable for others. For that reason, don't specify a default hostname for gnutls-cli, but instead let each test choose between `--insecure 127.0.0.1` and `localhost` (or `--insecure '::1'` if desired). Alternatives include: - having test certificates with 127.0.0.1 as the hostname, but having an IP as the CN is unusual, and we would need to change our test certs; - have our server open two sockets under the hood and listen on both IPv4 and IPv6 (that's what gnutls-serv does, and IMO it's a good thing) but that obviously requires development and testing (esp. for windows compatibility) - wait for a newer version of GnuTLS to be released, install it on the CI and developer machines, and use that in all tests - quite satisfying but can't be done now (and puts stronger requirements on test environment). --- tests/ssl-opt.sh | 52 ++++++++++++++++++++---------------------------- 1 file changed, 22 insertions(+), 30 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index ec2717ad5..e89d3a981 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -765,7 +765,7 @@ P_PXY="$P_PXY server_addr=127.0.0.1 server_port=$SRV_PORT listen_addr=127.0.0.1 O_SRV="$O_SRV -accept $SRV_PORT -dhparam data_files/dhparams.pem" O_CLI="$O_CLI -connect localhost:+SRV_PORT" G_SRV="$G_SRV -p $SRV_PORT" -G_CLI="$G_CLI -p +SRV_PORT localhost" +G_CLI="$G_CLI -p +SRV_PORT" if [ -n "${OPENSSL_LEGACY:-}" ]; then O_LEGACY_SRV="$O_LEGACY_SRV -accept $SRV_PORT -dhparam data_files/dhparams.pem" @@ -777,7 +777,7 @@ if [ -n "${GNUTLS_NEXT_SERV:-}" ]; then fi if [ -n "${GNUTLS_NEXT_CLI:-}" ]; then - G_NEXT_CLI="$G_NEXT_CLI -p +SRV_PORT localhost" + G_NEXT_CLI="$G_NEXT_CLI -p +SRV_PORT" fi # Allow SHA-1, because many of our test certificates use it @@ -2118,7 +2118,7 @@ run_test "Renego ext: gnutls server unsafe, client break legacy" \ requires_gnutls run_test "Renego ext: gnutls client strict, server default" \ "$P_SRV debug_level=3" \ - "$G_CLI --priority=NORMAL:%SAFE_RENEGOTIATION" \ + "$G_CLI --priority=NORMAL:%SAFE_RENEGOTIATION localhost" \ 0 \ -s "received TLS_EMPTY_RENEGOTIATION_INFO\|found renegotiation extension" \ -s "server hello, secure renegotiation extension" @@ -2126,7 +2126,7 @@ run_test "Renego ext: gnutls client strict, server default" \ requires_gnutls run_test "Renego ext: gnutls client unsafe, server default" \ "$P_SRV debug_level=3" \ - "$G_CLI --priority=NORMAL:%DISABLE_SAFE_RENEGOTIATION" \ + "$G_CLI --priority=NORMAL:%DISABLE_SAFE_RENEGOTIATION localhost" \ 0 \ -S "received TLS_EMPTY_RENEGOTIATION_INFO\|found renegotiation extension" \ -S "server hello, secure renegotiation extension" @@ -2134,7 +2134,7 @@ run_test "Renego ext: gnutls client unsafe, server default" \ requires_gnutls run_test "Renego ext: gnutls client unsafe, server break legacy" \ "$P_SRV debug_level=3 allow_legacy=-1" \ - "$G_CLI --priority=NORMAL:%DISABLE_SAFE_RENEGOTIATION" \ + "$G_CLI --priority=NORMAL:%DISABLE_SAFE_RENEGOTIATION localhost" \ 1 \ -S "received TLS_EMPTY_RENEGOTIATION_INFO\|found renegotiation extension" \ -S "server hello, secure renegotiation extension" @@ -2145,7 +2145,7 @@ requires_gnutls run_test "DER format: no trailing bytes" \ "$P_SRV crt_file=data_files/server5-der0.crt \ key_file=data_files/server5.key" \ - "$G_CLI " \ + "$G_CLI localhost" \ 0 \ -c "Handshake was completed" \ @@ -2153,7 +2153,7 @@ requires_gnutls run_test "DER format: with a trailing zero byte" \ "$P_SRV crt_file=data_files/server5-der1a.crt \ key_file=data_files/server5.key" \ - "$G_CLI " \ + "$G_CLI localhost" \ 0 \ -c "Handshake was completed" \ @@ -2161,7 +2161,7 @@ requires_gnutls run_test "DER format: with a trailing random byte" \ "$P_SRV crt_file=data_files/server5-der1b.crt \ key_file=data_files/server5.key" \ - "$G_CLI " \ + "$G_CLI localhost" \ 0 \ -c "Handshake was completed" \ @@ -2169,7 +2169,7 @@ requires_gnutls run_test "DER format: with 2 trailing random bytes" \ "$P_SRV crt_file=data_files/server5-der2.crt \ key_file=data_files/server5.key" \ - "$G_CLI " \ + "$G_CLI localhost" \ 0 \ -c "Handshake was completed" \ @@ -2177,7 +2177,7 @@ requires_gnutls run_test "DER format: with 4 trailing random bytes" \ "$P_SRV crt_file=data_files/server5-der4.crt \ key_file=data_files/server5.key" \ - "$G_CLI " \ + "$G_CLI localhost" \ 0 \ -c "Handshake was completed" \ @@ -2185,7 +2185,7 @@ requires_gnutls run_test "DER format: with 8 trailing random bytes" \ "$P_SRV crt_file=data_files/server5-der8.crt \ key_file=data_files/server5.key" \ - "$G_CLI " \ + "$G_CLI localhost" \ 0 \ -c "Handshake was completed" \ @@ -2193,7 +2193,7 @@ requires_gnutls run_test "DER format: with 9 trailing random bytes" \ "$P_SRV crt_file=data_files/server5-der9.crt \ key_file=data_files/server5.key" \ - "$G_CLI " \ + "$G_CLI localhost" \ 0 \ -c "Handshake was completed" \ @@ -3758,14 +3758,14 @@ run_test "Per-version suites: TLS 1.2" \ requires_gnutls run_test "ClientHello without extensions, SHA-1 allowed" \ "$P_SRV debug_level=3" \ - "$G_CLI --priority=NORMAL:%NO_EXTENSIONS:%DISABLE_SAFE_RENEGOTIATION" \ + "$G_CLI --priority=NORMAL:%NO_EXTENSIONS:%DISABLE_SAFE_RENEGOTIATION localhost" \ 0 \ -s "dumping 'client hello extensions' (0 bytes)" requires_gnutls run_test "ClientHello without extensions, SHA-1 forbidden in certificates on server" \ "$P_SRV debug_level=3 key_file=data_files/server2.key crt_file=data_files/server2.crt allow_sha1=0" \ - "$G_CLI --priority=NORMAL:%NO_EXTENSIONS:%DISABLE_SAFE_RENEGOTIATION" \ + "$G_CLI --priority=NORMAL:%NO_EXTENSIONS:%DISABLE_SAFE_RENEGOTIATION localhost" \ 0 \ -s "dumping 'client hello extensions' (0 bytes)" @@ -5394,35 +5394,31 @@ run_test "DTLS fragmenting: gnutls server, DTLS 1.0" \ -c "fragmenting handshake message" \ -C "error" -# gnutls-cli always tries IPv6 first, and doesn't fall back to IPv4 with DTLS -requires_ipv6 requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_2 requires_gnutls run_test "DTLS fragmenting: gnutls client, DTLS 1.2" \ - "$P_SRV dtls=1 debug_level=2 server_addr=::1 \ + "$P_SRV dtls=1 debug_level=2 \ crt_file=data_files/server7_int-ca.crt \ key_file=data_files/server7.key \ mtu=512 force_version=dtls1_2" \ - "$G_CLI -u" \ + "$G_CLI -u --insecure 127.0.0.1" \ 0 \ -s "fragmenting handshake message" -# gnutls-cli always tries IPv6 first, and doesn't fall back to IPv4 with DTLS -requires_ipv6 requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C requires_config_enabled MBEDTLS_SSL_PROTO_TLS1_1 requires_gnutls run_test "DTLS fragmenting: gnutls client, DTLS 1.0" \ - "$P_SRV dtls=1 debug_level=2 server_addr=::1 \ + "$P_SRV dtls=1 debug_level=2 \ crt_file=data_files/server7_int-ca.crt \ key_file=data_files/server7.key \ mtu=512 force_version=dtls1" \ - "$G_CLI -u" \ + "$G_CLI -u --insecure 127.0.0.1" \ 0 \ -s "fragmenting handshake message" @@ -5524,8 +5520,6 @@ run_test "DTLS fragmenting: 3d, gnutls server, DTLS 1.0" \ ## We can re-enable them when a fixed version fo GnuTLS is available ## and installed in our CI system. ## -## # gnutls-cli always tries IPv6 first, and doesn't fall back to IPv4 with DTLS -## requires_ipv6 ## requires_gnutls ## requires_config_enabled MBEDTLS_SSL_PROTO_DTLS ## requires_config_enabled MBEDTLS_RSA_C @@ -5534,16 +5528,14 @@ run_test "DTLS fragmenting: 3d, gnutls server, DTLS 1.0" \ ## client_needs_more_time 4 ## run_test "DTLS fragmenting: 3d, gnutls client, DTLS 1.2" \ ## -p "$P_PXY drop=8 delay=8 duplicate=8" \ -## "$P_SRV dtls=1 debug_level=2 server_addr=::1 \ +## "$P_SRV dtls=1 debug_level=2 \ ## crt_file=data_files/server7_int-ca.crt \ ## key_file=data_files/server7.key \ ## hs_timeout=250-60000 mtu=512 force_version=dtls1_2" \ -## "$G_CLI -u" \ +## "$G_CLI -u --insecure 127.0.0.1" \ ## 0 \ ## -s "fragmenting handshake message" ## -## # gnutls-cli always tries IPv6 first, and doesn't fall back to IPv4 with DTLS -## requires_ipv6 ## requires_gnutls ## requires_config_enabled MBEDTLS_SSL_PROTO_DTLS ## requires_config_enabled MBEDTLS_RSA_C @@ -5552,11 +5544,11 @@ run_test "DTLS fragmenting: 3d, gnutls server, DTLS 1.0" \ ## client_needs_more_time 4 ## run_test "DTLS fragmenting: 3d, gnutls client, DTLS 1.0" \ ## -p "$P_PXY drop=8 delay=8 duplicate=8" \ -## "$P_SRV dtls=1 debug_level=2 server_addr=::1 \ +## "$P_SRV dtls=1 debug_level=2 \ ## crt_file=data_files/server7_int-ca.crt \ ## key_file=data_files/server7.key \ ## hs_timeout=250-60000 mtu=512 force_version=dtls1" \ -## "$G_CLI -u" \ +## "$G_CLI -u --insecure 127.0.0.1" \ ## 0 \ ## -s "fragmenting handshake message" From c83d2b3e095e114d2cdaf7597bfc6cbb318ccf8d Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 22 Aug 2018 16:05:47 +0100 Subject: [PATCH 34/35] ssl_write_handshake_msg(): Allow alert on client-side SSLv3 In SSLv3, the client sends a NoCertificate alert in response to a CertificateRequest if it doesn't have a CRT. This previously lead to failure in ssl_write_handshake_msg() which only accepted handshake or CCS records. --- library/ssl_tls.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index faa9467e1..d22b0e228 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3049,11 +3049,19 @@ int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) /* * Sanity checks */ - if( ssl->out_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE && + if( ssl->out_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE && ssl->out_msgtype != MBEDTLS_SSL_MSG_CHANGE_CIPHER_SPEC ) { - MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); - return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + /* In SSLv3, the client might send a NoCertificate alert. */ +#if defined(MBEDTLS_SSL_PROTO_SSL3) && defined(MBEDTLS_SSL_CLI_C) + if( ! ( ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 && + ssl->out_msgtype == MBEDTLS_SSL_MSG_ALERT && + ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT ) ) +#endif /* MBEDTLS_SSL_PROTO_SSL3 && MBEDTLS_SSL_SRV_C */ + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) ); + return( MBEDTLS_ERR_SSL_INTERNAL_ERROR ); + } } if( ssl->out_msgtype == MBEDTLS_SSL_MSG_HANDSHAKE && From 081bd81865881b82fc5d04847189b01fe4df8c1e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 22 Aug 2018 16:07:59 +0100 Subject: [PATCH 35/35] ssl_write_handshake_msg(): Always append CCS messages to flights The previous code appended messages to flights only if their handshake type, as derived from the first byte in the message, was different from MBEDTLS_SSL_HS_HELLO_REQUEST. This check should only be performed for handshake records, while CCS records should immediately be appended. --- library/ssl_tls.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index d22b0e228..3a972a598 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3142,7 +3142,8 @@ int mbedtls_ssl_write_handshake_msg( mbedtls_ssl_context *ssl ) /* Either send now, or just save to be sent (and resent) later */ #if defined(MBEDTLS_SSL_PROTO_DTLS) if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && - hs_type != MBEDTLS_SSL_HS_HELLO_REQUEST ) + ( ssl->out_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE || + hs_type != MBEDTLS_SSL_HS_HELLO_REQUEST ) ) { if( ( ret = ssl_flight_append( ssl ) ) != 0 ) {