Merge remote-tracking branch 'upstream-public/pr/1141' into development
diff --git a/ChangeLog b/ChangeLog
index 2b9aa14..6e404ff 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -48,6 +48,8 @@
* Fix bugs in RSA test suite under MBEDTLS_NO_PLATFORM_ENTROPY. #1023 #1024
* Fix net_would_block to avoid modification by errno through fcntl call.
Found by nkolban. Fixes #845.
+ * Fix handling of handshake messages in mbedtls_ssl_read in case
+ MBEDTLS_SSL_RENEGOTIATION is disabled. Found by erja-gp.
Changes
* Extend cert_write example program by options to set the CRT version
diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h
index 47c7196..69e997f 100644
--- a/include/mbedtls/config.h
+++ b/include/mbedtls/config.h
@@ -1155,6 +1155,13 @@
* misuse/misunderstand.
*
* Comment this to disable support for renegotiation.
+ *
+ * \note Even if this option is disabled, both client and server are aware
+ * of the Renegotiation Indication Extension (RFC 5746) used to
+ * prevent the SSL renegotiation attack (see RFC 5746 Sect. 1).
+ * (See \c mbedtls_ssl_conf_legacy_renegotiation for the
+ * configuration of this extension).
+ *
*/
#define MBEDTLS_SSL_RENEGOTIATION
diff --git a/library/ssl_cli.c b/library/ssl_cli.c
index 9227433..554348f 100644
--- a/library/ssl_cli.c
+++ b/library/ssl_cli.c
@@ -134,6 +134,9 @@
*olen = 0;
+ /* We're always including an TLS_EMPTY_RENEGOTIATION_INFO_SCSV in the
+ * initial ClientHello, in which case also adding the renegotiation
+ * info extension is NOT RECOMMENDED as per RFC 5746 Section 3.4. */
if( ssl->renego_status != MBEDTLS_SSL_RENEGOTIATION_IN_PROGRESS )
return;
@@ -971,6 +974,8 @@
ext_len += olen;
#endif
+ /* Note that TLS_EMPTY_RENEGOTIATION_INFO_SCSV is always added
+ * even if MBEDTLS_SSL_RENEGOTIATION is not defined. */
#if defined(MBEDTLS_SSL_RENEGOTIATION)
ssl_write_renegotiation_ext( ssl, p + 2 + ext_len, &olen );
ext_len += olen;
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index e19dfc9..2690e46 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -6883,7 +6883,6 @@
}
}
-#if defined(MBEDTLS_SSL_RENEGOTIATION)
if( ssl->in_msgtype == MBEDTLS_SSL_MSG_HANDSHAKE )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "received handshake message" ) );
@@ -6925,12 +6924,35 @@
}
#endif /* MBEDTLS_SSL_SRV_C */
+#if defined(MBEDTLS_SSL_RENEGOTIATION)
/* Determine whether renegotiation attempt should be accepted */
+ if( ! ( ssl->conf->disable_renegotiation == MBEDTLS_SSL_RENEGOTIATION_DISABLED ||
+ ( ssl->secure_renegotiation == MBEDTLS_SSL_LEGACY_RENEGOTIATION &&
+ ssl->conf->allow_legacy_renegotiation ==
+ MBEDTLS_SSL_LEGACY_NO_RENEGOTIATION ) ) )
+ {
+ /*
+ * Accept renegotiation request
+ */
- if( ssl->conf->disable_renegotiation == MBEDTLS_SSL_RENEGOTIATION_DISABLED ||
- ( ssl->secure_renegotiation == MBEDTLS_SSL_LEGACY_RENEGOTIATION &&
- ssl->conf->allow_legacy_renegotiation ==
- MBEDTLS_SSL_LEGACY_NO_RENEGOTIATION ) )
+ /* DTLS clients need to know renego is server-initiated */
+#if defined(MBEDTLS_SSL_PROTO_DTLS)
+ if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM &&
+ ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT )
+ {
+ ssl->renego_status = MBEDTLS_SSL_RENEGOTIATION_PENDING;
+ }
+#endif
+ ret = ssl_start_renegotiation( ssl );
+ if( ret != MBEDTLS_ERR_SSL_WAITING_SERVER_HELLO_RENEGO &&
+ ret != 0 )
+ {
+ MBEDTLS_SSL_DEBUG_RET( 1, "ssl_start_renegotiation", ret );
+ return( ret );
+ }
+ }
+ else
+#endif /* MBEDTLS_SSL_RENEGOTIATION */
{
/*
* Refuse renegotiation
@@ -6968,31 +6990,10 @@
return( MBEDTLS_ERR_SSL_INTERNAL_ERROR );
}
}
- else
- {
- /*
- * Accept renegotiation request
- */
-
- /* DTLS clients need to know renego is server-initiated */
-#if defined(MBEDTLS_SSL_PROTO_DTLS)
- if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM &&
- ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT )
- {
- ssl->renego_status = MBEDTLS_SSL_RENEGOTIATION_PENDING;
- }
-#endif
- ret = ssl_start_renegotiation( ssl );
- if( ret != MBEDTLS_ERR_SSL_WAITING_SERVER_HELLO_RENEGO &&
- ret != 0 )
- {
- MBEDTLS_SSL_DEBUG_RET( 1, "ssl_start_renegotiation", ret );
- return( ret );
- }
- }
return( MBEDTLS_ERR_SSL_WANT_READ );
}
+#if defined(MBEDTLS_SSL_RENEGOTIATION)
else if( ssl->renego_status == MBEDTLS_SSL_RENEGOTIATION_PENDING )
{
if( ssl->conf->renego_max_records >= 0 )
diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh
index 258141d..838b541 100755
--- a/tests/scripts/all.sh
+++ b/tests/scripts/all.sh
@@ -326,6 +326,19 @@
msg "build: SSLv3 - ssl-opt.sh (ASan build)" # ~ 6 min
tests/ssl-opt.sh
+msg "build: Default + !MBEDTLS_SSL_RENEGOTIATION (ASan build)" # ~ 6 min
+cleanup
+cp "$CONFIG_H" "$CONFIG_BAK"
+scripts/config.pl unset MBEDTLS_SSL_RENEGOTIATION
+CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan .
+make
+
+msg "test: !MBEDTLS_SSL_RENEGOTIATION - main suites (inc. selftests) (ASan build)" # ~ 50s
+make test
+
+msg "test: !MBEDTLS_SSL_RENEGOTIATION - ssl-opt.sh (ASan build)" # ~ 6 min
+tests/ssl-opt.sh
+
msg "build: cmake, full config, clang, C99" # ~ 50s
cleanup
cp "$CONFIG_H" "$CONFIG_BAK"
diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh
index 5dc47a3..326dcad 100755
--- a/tests/ssl-opt.sh
+++ b/tests/ssl-opt.sh
@@ -1501,6 +1501,7 @@
# Tests for renegotiation
+# Renegotiation SCSV always added, regardless of SSL_RENEGOTIATION
run_test "Renegotiation: none, for reference" \
"$P_SRV debug_level=3 exchanges=2 auth_mode=optional" \
"$P_CLI debug_level=3 exchanges=2" \
@@ -1514,6 +1515,7 @@
-S "=> renegotiate" \
-S "write hello request"
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: client-initiated" \
"$P_SRV debug_level=3 exchanges=2 renegotiation=1 auth_mode=optional" \
"$P_CLI debug_level=3 exchanges=2 renegotiation=1 renegotiate=1" \
@@ -1527,6 +1529,7 @@
-s "=> renegotiate" \
-S "write hello request"
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: server-initiated" \
"$P_SRV debug_level=3 exchanges=2 renegotiation=1 auth_mode=optional renegotiate=1" \
"$P_CLI debug_level=3 exchanges=2 renegotiation=1" \
@@ -1543,6 +1546,7 @@
# Checks that no Signature Algorithm with SHA-1 gets negotiated. Negotiating SHA-1 would mean that
# the server did not parse the Signature Algorithm extension. This test is valid only if an MD
# algorithm stronger than SHA-1 is enabled in config.h
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: Signature Algorithms parsing, client-initiated" \
"$P_SRV debug_level=3 exchanges=2 renegotiation=1 auth_mode=optional" \
"$P_CLI debug_level=3 exchanges=2 renegotiation=1 renegotiate=1" \
@@ -1560,6 +1564,7 @@
# Checks that no Signature Algorithm with SHA-1 gets negotiated. Negotiating SHA-1 would mean that
# the server did not parse the Signature Algorithm extension. This test is valid only if an MD
# algorithm stronger than SHA-1 is enabled in config.h
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: Signature Algorithms parsing, server-initiated" \
"$P_SRV debug_level=3 exchanges=2 renegotiation=1 auth_mode=optional renegotiate=1" \
"$P_CLI debug_level=3 exchanges=2 renegotiation=1" \
@@ -1574,6 +1579,7 @@
-s "write hello request" \
-S "client hello v3, signature_algorithm ext: 2" # Is SHA-1 negotiated?
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: double" \
"$P_SRV debug_level=3 exchanges=2 renegotiation=1 auth_mode=optional renegotiate=1" \
"$P_CLI debug_level=3 exchanges=2 renegotiation=1 renegotiate=1" \
@@ -1587,6 +1593,7 @@
-s "=> renegotiate" \
-s "write hello request"
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: client-initiated, server-rejected" \
"$P_SRV debug_level=3 exchanges=2 renegotiation=0 auth_mode=optional" \
"$P_CLI debug_level=3 exchanges=2 renegotiation=1 renegotiate=1" \
@@ -1602,6 +1609,7 @@
-c "SSL - Unexpected message at ServerHello in renegotiation" \
-c "failed"
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: server-initiated, client-rejected, default" \
"$P_SRV debug_level=3 exchanges=2 renegotiation=1 renegotiate=1 auth_mode=optional" \
"$P_CLI debug_level=3 exchanges=2 renegotiation=0" \
@@ -1617,6 +1625,7 @@
-S "SSL - An unexpected message was received from our peer" \
-S "failed"
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: server-initiated, client-rejected, not enforced" \
"$P_SRV debug_level=3 exchanges=2 renegotiation=1 renegotiate=1 \
renego_delay=-1 auth_mode=optional" \
@@ -1634,6 +1643,7 @@
-S "failed"
# delay 2 for 1 alert record + 1 application data record
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: server-initiated, client-rejected, delay 2" \
"$P_SRV debug_level=3 exchanges=2 renegotiation=1 renegotiate=1 \
renego_delay=2 auth_mode=optional" \
@@ -1650,6 +1660,7 @@
-S "SSL - An unexpected message was received from our peer" \
-S "failed"
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: server-initiated, client-rejected, delay 0" \
"$P_SRV debug_level=3 exchanges=2 renegotiation=1 renegotiate=1 \
renego_delay=0 auth_mode=optional" \
@@ -1665,6 +1676,7 @@
-s "write hello request" \
-s "SSL - An unexpected message was received from our peer"
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: server-initiated, client-accepted, delay 0" \
"$P_SRV debug_level=3 exchanges=2 renegotiation=1 renegotiate=1 \
renego_delay=0 auth_mode=optional" \
@@ -1681,6 +1693,7 @@
-S "SSL - An unexpected message was received from our peer" \
-S "failed"
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: periodic, just below period" \
"$P_SRV debug_level=3 exchanges=9 renegotiation=1 renego_period=3 auth_mode=optional" \
"$P_CLI debug_level=3 exchanges=2 renegotiation=1" \
@@ -1698,6 +1711,7 @@
-S "failed"
# one extra exchange to be able to complete renego
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: periodic, just above period" \
"$P_SRV debug_level=3 exchanges=9 renegotiation=1 renego_period=3 auth_mode=optional" \
"$P_CLI debug_level=3 exchanges=4 renegotiation=1" \
@@ -1714,6 +1728,7 @@
-S "SSL - An unexpected message was received from our peer" \
-S "failed"
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: periodic, two times period" \
"$P_SRV debug_level=3 exchanges=9 renegotiation=1 renego_period=3 auth_mode=optional" \
"$P_CLI debug_level=3 exchanges=7 renegotiation=1" \
@@ -1730,6 +1745,7 @@
-S "SSL - An unexpected message was received from our peer" \
-S "failed"
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: periodic, above period, disabled" \
"$P_SRV debug_level=3 exchanges=9 renegotiation=0 renego_period=3 auth_mode=optional" \
"$P_CLI debug_level=3 exchanges=4 renegotiation=1" \
@@ -1746,6 +1762,7 @@
-S "SSL - An unexpected message was received from our peer" \
-S "failed"
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: nbio, client-initiated" \
"$P_SRV debug_level=3 nbio=2 exchanges=2 renegotiation=1 auth_mode=optional" \
"$P_CLI debug_level=3 nbio=2 exchanges=2 renegotiation=1 renegotiate=1" \
@@ -1759,6 +1776,7 @@
-s "=> renegotiate" \
-S "write hello request"
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: nbio, server-initiated" \
"$P_SRV debug_level=3 nbio=2 exchanges=2 renegotiation=1 renegotiate=1 auth_mode=optional" \
"$P_CLI debug_level=3 nbio=2 exchanges=2 renegotiation=1" \
@@ -1772,6 +1790,7 @@
-s "=> renegotiate" \
-s "write hello request"
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: openssl server, client-initiated" \
"$O_SRV -www" \
"$P_CLI debug_level=3 exchanges=1 renegotiation=1 renegotiate=1" \
@@ -1784,6 +1803,7 @@
-c "HTTP/1.0 200 [Oo][Kk]"
requires_gnutls
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: gnutls server strict, client-initiated" \
"$G_SRV --priority=NORMAL:%SAFE_RENEGOTIATION" \
"$P_CLI debug_level=3 exchanges=1 renegotiation=1 renegotiate=1" \
@@ -1796,6 +1816,7 @@
-c "HTTP/1.0 200 [Oo][Kk]"
requires_gnutls
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: gnutls server unsafe, client-initiated default" \
"$G_SRV --priority=NORMAL:%DISABLE_SAFE_RENEGOTIATION" \
"$P_CLI debug_level=3 exchanges=1 renegotiation=1 renegotiate=1" \
@@ -1808,6 +1829,7 @@
-C "HTTP/1.0 200 [Oo][Kk]"
requires_gnutls
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: gnutls server unsafe, client-inititated no legacy" \
"$G_SRV --priority=NORMAL:%DISABLE_SAFE_RENEGOTIATION" \
"$P_CLI debug_level=3 exchanges=1 renegotiation=1 renegotiate=1 \
@@ -1821,6 +1843,7 @@
-C "HTTP/1.0 200 [Oo][Kk]"
requires_gnutls
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: gnutls server unsafe, client-inititated legacy" \
"$G_SRV --priority=NORMAL:%DISABLE_SAFE_RENEGOTIATION" \
"$P_CLI debug_level=3 exchanges=1 renegotiation=1 renegotiate=1 \
@@ -1833,6 +1856,7 @@
-C "error" \
-c "HTTP/1.0 200 [Oo][Kk]"
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: DTLS, client-initiated" \
"$P_SRV debug_level=3 dtls=1 exchanges=2 renegotiation=1" \
"$P_CLI debug_level=3 dtls=1 exchanges=2 renegotiation=1 renegotiate=1" \
@@ -1846,6 +1870,7 @@
-s "=> renegotiate" \
-S "write hello request"
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: DTLS, server-initiated" \
"$P_SRV debug_level=3 dtls=1 exchanges=2 renegotiation=1 renegotiate=1" \
"$P_CLI debug_level=3 dtls=1 exchanges=2 renegotiation=1 \
@@ -1860,6 +1885,7 @@
-s "=> renegotiate" \
-s "write hello request"
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: DTLS, renego_period overflow" \
"$P_SRV debug_level=3 dtls=1 exchanges=4 renegotiation=1 renego_period=18446462598732840962 auth_mode=optional" \
"$P_CLI debug_level=3 dtls=1 exchanges=4 renegotiation=1" \
@@ -1871,9 +1897,10 @@
-s "record counter limit reached: renegotiate" \
-c "=> renegotiate" \
-s "=> renegotiate" \
- -s "write hello request" \
+ -s "write hello request"
requires_gnutls
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "Renegotiation: DTLS, gnutls server, client-initiated" \
"$G_SRV -u --mtu 4096" \
"$P_CLI debug_level=3 dtls=1 exchanges=1 renegotiation=1 renegotiate=1" \
@@ -3782,6 +3809,7 @@
-C "error"
requires_gnutls
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "DTLS reassembly: fragmentation, renego (gnutls server)" \
"$G_SRV -u --mtu 256" \
"$P_CLI debug_level=3 dtls=1 renegotiation=1 renegotiate=1" \
@@ -3795,6 +3823,7 @@
-s "Extra-header:"
requires_gnutls
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "DTLS reassembly: fragmentation, nbio, renego (gnutls server)" \
"$G_SRV -u --mtu 256" \
"$P_CLI debug_level=3 nbio=2 dtls=1 renegotiation=1 renegotiate=1" \
@@ -4039,6 +4068,7 @@
-c "HTTP/1.0 200 OK"
client_needs_more_time 4
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "DTLS proxy: 3d, min handshake, client-initiated renego" \
-p "$P_PXY drop=5 delay=5 duplicate=5" \
"$P_SRV dtls=1 hs_timeout=250-10000 tickets=0 auth_mode=none \
@@ -4053,6 +4083,7 @@
-c "HTTP/1.0 200 OK"
client_needs_more_time 4
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "DTLS proxy: 3d, min handshake, client-initiated renego, nbio" \
-p "$P_PXY drop=5 delay=5 duplicate=5" \
"$P_SRV dtls=1 hs_timeout=250-10000 tickets=0 auth_mode=none \
@@ -4067,6 +4098,7 @@
-c "HTTP/1.0 200 OK"
client_needs_more_time 4
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "DTLS proxy: 3d, min handshake, server-initiated renego" \
-p "$P_PXY drop=5 delay=5 duplicate=5" \
"$P_SRV dtls=1 hs_timeout=250-10000 tickets=0 auth_mode=none \
@@ -4082,6 +4114,7 @@
-c "HTTP/1.0 200 OK"
client_needs_more_time 4
+requires_config_enabled MBEDTLS_SSL_RENEGOTIATION
run_test "DTLS proxy: 3d, min handshake, server-initiated renego, nbio" \
-p "$P_PXY drop=5 delay=5 duplicate=5" \
"$P_SRV dtls=1 hs_timeout=250-10000 tickets=0 auth_mode=none \