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 01/17] 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 02/17] 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 03/17] 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 04/17] 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 05/17] 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 06/17] 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 07/17] 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 08/17] 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 513815a38dd3e864531456d9537298de8b32d7ce Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 20 Aug 2018 11:56:09 +0100 Subject: [PATCH 09/17] Fix typo in debugging output --- library/ssl_tls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index cc470583a..05a2a9f01 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3388,7 +3388,7 @@ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ) flush = SSL_FORCE_FLUSH; else { - MBEDTLS_SSL_DEBUG_MSG( 2, ( "Stil %u bytes available in current datagram", (unsigned) remaining ) ); + MBEDTLS_SSL_DEBUG_MSG( 2, ( "Still %u bytes available in current datagram", (unsigned) remaining ) ); } } #endif /* MBEDTLS_SSL_PROTO_DTLS */ From 4e1a9c17f29f9b4af76d95202a0030c7aa46873b Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 20 Aug 2018 12:21:35 +0100 Subject: [PATCH 10/17] ssl-opt.sh: Preserve proxy log, too, if --preserve-logs is specified --- tests/ssl-opt.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 4fa8609f9..09728314d 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -682,6 +682,9 @@ run_test() { if [ "$PRESERVE_LOGS" -gt 0 ]; then mv $SRV_OUT o-srv-${TESTS}.log mv $CLI_OUT o-cli-${TESTS}.log + if [ -n "$PXY_CMD" ]; then + mv $PXY_OUT o-pxy-${TESTS}.log + fi fi rm -f $SRV_OUT $CLI_OUT $PXY_OUT From f362c297fa199fc4269d940e252b8933426fce2b Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 20 Aug 2018 12:40:23 +0100 Subject: [PATCH 11/17] ssl-opt.sh Add dependency on gnutls in two fragmentation tests --- tests/ssl-opt.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 09728314d..b6af4dff0 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5359,6 +5359,7 @@ run_test "DTLS fragmenting: proxy MTU + 3d" \ # # 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_gnutls requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C @@ -5373,6 +5374,7 @@ run_test "DTLS fragmenting: gnutls server, DTLS 1.2" \ -c "fragmenting handshake message" \ -C "error" +requires_gnutls requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C requires_config_enabled MBEDTLS_ECDSA_C From f61ff4e1d689388d76abb83f685f48a5c1c1f914 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 21 Aug 2018 13:17:47 +0100 Subject: [PATCH 12/17] ssl_server2: Remove redundant new line --- programs/ssl/ssl_server2.c | 1 - 1 file changed, 1 deletion(-) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 4378e4f25..8d414364a 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -2193,7 +2193,6 @@ int main( int argc, char *argv[] ) }; #endif - #if defined(MBEDTLS_SSL_TRUNCATED_HMAC) if( opt.trunc_hmac != DFL_TRUNC_HMAC ) mbedtls_ssl_conf_truncated_hmac( &conf, opt.trunc_hmac ); From ecff20554821ca9962c587fd9f55768f4d9fe787 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 21 Aug 2018 13:20:00 +0100 Subject: [PATCH 13/17] Remove stray bracket if MBEDTLS_ZLIB_SUPPORT is defined --- library/ssl_tls.c | 1 - 1 file changed, 1 deletion(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 0b3fea177..08ed75dc2 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7238,7 +7238,6 @@ 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 ) ) From 1f5a15d86dcc7350c5684b350e33b9d769b7cfd4 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 21 Aug 2018 13:31:31 +0100 Subject: [PATCH 14/17] Check retval of remaining_payload_in_datagram in ssl_write_record() --- library/ssl_tls.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 08ed75dc2..e888812f6 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3383,7 +3383,16 @@ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ) #if defined(MBEDTLS_SSL_PROTO_DTLS) if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) { - size_t remaining = ssl_get_remaining_payload_in_datagram( ssl ); + size_t remaining; + ret = ssl_get_remaining_payload_in_datagram( ssl ); + if( ret < 0 ) + { + MBEDTLS_SSL_DEBUG_RET( 1, "ssl_get_remaining_payload_in_datagram", + ret ); + return( ret ); + } + + remaining = (size_t) ret; if( remaining == 0 ) flush = SSL_FORCE_FLUSH; else From 47db877039d61ff28d2c3ce121acaed47e55b437 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 21 Aug 2018 13:32:13 +0100 Subject: [PATCH 15/17] ssl_write_record: Consider setting flush variable only if unset --- 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 e888812f6..e4ea5c2bc 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -3381,7 +3381,8 @@ int mbedtls_ssl_write_record( mbedtls_ssl_context *ssl, uint8_t force_flush ) } #if defined(MBEDTLS_SSL_PROTO_DTLS) - if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM ) + if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && + flush == SSL_DONT_FORCE_FLUSH ) { size_t remaining; ret = ssl_get_remaining_payload_in_datagram( ssl ); From 5bcf2b081f4ba0ec395478611d19e03f0793c7b6 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 21 Aug 2018 14:25:40 +0100 Subject: [PATCH 16/17] ssl-opt.sh: Allow spurious resend in DTLS session resumption test 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. This commit removes a guard in the ssl-opt.sh test 'DTLS fragmenting: proxy MTU, resumed handshake' which made the test fail in case the log showed a resend from the client. --- 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 886c44cfa..9b416fbb2 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5171,6 +5171,9 @@ 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). +# +# A resend on the client-side might happen if the server is +# slow to reset, therefore omitting '-C "resend"' below. not_with_valgrind # spurious resend due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C @@ -5187,7 +5190,6 @@ run_test "DTLS fragmenting: proxy MTU, resumed handshake" \ mtu=1450 reconnect=1" \ 0 \ -S "resend" \ - -C "resend" \ -s "found fragmented DTLS handshake message" \ -c "found fragmented DTLS handshake message" \ -C "error" From 175cb8fc699a1d755ba81976e53b91a131be445e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Tue, 21 Aug 2018 17:00:10 +0100 Subject: [PATCH 17/17] ssl-opt.sh: Allow resend in DTLS session resumption tests, cont'd This commit continues commit 47db877 by removing resend guards in the ssl-opt.sh tests 'DTLS fragmenting: proxy MTU, XXX' which sometimes made the tests fail in case the log showed a resend from the client. See 47db877 for more information. --- tests/ssl-opt.sh | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 9b416fbb2..ab53cc46c 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -5194,6 +5194,8 @@ run_test "DTLS fragmenting: proxy MTU, resumed handshake" \ -c "found fragmented DTLS handshake message" \ -C "error" +# A resend on the client-side might happen if the server is +# slow to reset, therefore omitting '-C "resend"' below. not_with_valgrind # spurious resend due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C @@ -5217,11 +5219,12 @@ 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" +# A resend on the client-side might happen if the server is +# slow to reset, therefore omitting '-C "resend"' below. not_with_valgrind # spurious resend due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C @@ -5246,11 +5249,12 @@ 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" +# A resend on the client-side might happen if the server is +# slow to reset, therefore omitting '-C "resend"' below. not_with_valgrind # spurious resend due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C @@ -5275,11 +5279,12 @@ 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" +# A resend on the client-side might happen if the server is +# slow to reset, therefore omitting '-C "resend"' below. not_with_valgrind # spurious resend due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C @@ -5305,11 +5310,12 @@ 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" +# A resend on the client-side might happen if the server is +# slow to reset, therefore omitting '-C "resend"' below. not_with_valgrind # spurious resend due to timeout requires_config_enabled MBEDTLS_SSL_PROTO_DTLS requires_config_enabled MBEDTLS_RSA_C @@ -5334,7 +5340,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"