From cc4d80fde37499d81e8003a78eb2832a0e73923e Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 12 May 2017 16:22:21 +0100 Subject: [PATCH 1/6] Backup errno in net_would_block Safe and restore the value of errno in net_would_block to be sure it's not affected by the guarding call to fcntl. Fixes #845. --- ChangeLog | 6 ++++++ library/net.c | 7 ++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index b46c72879..15a99e628 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS x.x.x branch released xxxx-xx-xx + +Bugfix + * Fix net_would_block to avoid modification by errno through fcntl call. + Found by nkolban. Fixes #845. + = mbed TLS 1.3.19 branch released 2017-03-08 Security diff --git a/library/net.c b/library/net.c index c7ce2584e..1e34901e3 100644 --- a/library/net.c +++ b/library/net.c @@ -396,13 +396,18 @@ static int net_would_block( int fd ) */ static int net_would_block( int fd ) { + int err = errno; + /* * Never return 'WOULD BLOCK' on a non-blocking socket */ if( ( fcntl( fd, F_GETFL ) & O_NONBLOCK ) != O_NONBLOCK ) + { + errno = err; return( 0 ); + } - switch( errno ) + switch( errno = err ) { #if defined EAGAIN case EAGAIN: From 268191a3057363193a7b28d86f42c20eceace390 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 25 Oct 2017 09:33:22 +0100 Subject: [PATCH 2/6] Swap branches accepting/refusing renegotiation in in ssl_read --- library/ssl_tls.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 54867da97..c72296679 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -4770,10 +4770,20 @@ int ssl_read( ssl_context *ssl, unsigned char *buf, size_t len ) } #endif - if( ssl->disable_renegotiation == SSL_RENEGOTIATION_DISABLED || - ( ssl->secure_renegotiation == SSL_LEGACY_RENEGOTIATION && - ssl->allow_legacy_renegotiation == - SSL_LEGACY_NO_RENEGOTIATION ) ) + if( ! ( ssl->disable_renegotiation == SSL_RENEGOTIATION_DISABLED || + ( ssl->secure_renegotiation == SSL_LEGACY_RENEGOTIATION && + ssl->allow_legacy_renegotiation == + SSL_LEGACY_NO_RENEGOTIATION ) ) ) + { + ret = ssl_start_renegotiation( ssl ); + if( ret != POLARSSL_ERR_SSL_WAITING_SERVER_HELLO_RENEGO && + ret != 0 ) + { + SSL_DEBUG_RET( 1, "ssl_start_renegotiation", ret ); + return( ret ); + } + } + else { SSL_DEBUG_MSG( 3, ( "ignoring renegotiation, sending alert" ) ); @@ -4807,16 +4817,6 @@ int ssl_read( ssl_context *ssl, unsigned char *buf, size_t len ) return( POLARSSL_ERR_SSL_INTERNAL_ERROR ); } } - else - { - ret = ssl_start_renegotiation( ssl ); - if( ret != POLARSSL_ERR_SSL_WAITING_SERVER_HELLO_RENEGO && - ret != 0 ) - { - SSL_DEBUG_RET( 1, "ssl_start_renegotiation", ret ); - return( ret ); - } - } return( POLARSSL_ERR_NET_WANT_READ ); } From bfd0991daae23f6084d9479eee0f306af16bc153 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 25 Oct 2017 09:34:48 +0100 Subject: [PATCH 3/6] Fix handling of HS msgs in ssl_read if renegotiation unused Previously, if `POLARSSL_SSL_RENEGOTIATION` was disabled, incoming handshake messages in `ssl_read` (expecting application data) lead to the connection being closed. This commit fixes this, restricting the `POLARSSL_SSL_RENEGOTIATION`-guard to the code-paths responsible for accepting renegotiation requests and aborting renegotiation attempts after too many unexpected records have been received. --- library/ssl_tls.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index c72296679..ee87bc0d4 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -4755,7 +4755,6 @@ int ssl_read( ssl_context *ssl, unsigned char *buf, size_t len ) } } -#if defined(POLARSSL_SSL_RENEGOTIATION) if( ssl->in_msgtype == SSL_MSG_HANDSHAKE ) { SSL_DEBUG_MSG( 1, ( "received handshake message" ) ); @@ -4770,6 +4769,7 @@ int ssl_read( ssl_context *ssl, unsigned char *buf, size_t len ) } #endif +#if defined(POLARSSL_SSL_RENEGOTIATION) if( ! ( ssl->disable_renegotiation == SSL_RENEGOTIATION_DISABLED || ( ssl->secure_renegotiation == SSL_LEGACY_RENEGOTIATION && ssl->allow_legacy_renegotiation == @@ -4784,6 +4784,7 @@ int ssl_read( ssl_context *ssl, unsigned char *buf, size_t len ) } } else +#endif /* POLARSSL_SSL_RENEGOTIATION */ { SSL_DEBUG_MSG( 3, ( "ignoring renegotiation, sending alert" ) ); @@ -4820,6 +4821,7 @@ int ssl_read( ssl_context *ssl, unsigned char *buf, size_t len ) return( POLARSSL_ERR_NET_WANT_READ ); } +#if defined(POLARSSL_SSL_RENEGOTIATION) else if( ssl->renegotiation == SSL_RENEGOTIATION_PENDING ) { ssl->renego_records_seen++; From e8f3d933e935832e34fa970c4b666191ada4308b Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 25 Oct 2017 09:38:00 +0100 Subject: [PATCH 4/6] Add dep'n on !DISABLE_RENEGOTIATION to renego tests in ssl-opt.sh --- tests/ssl-opt.sh | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 0f976682b..51d31fdfd 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -75,6 +75,13 @@ requires_config_enabled() { fi } +# skip next test if the flag is enabled in config.h +requires_config_disabled() { + if grep "^#define $1" $CONFIG_H > /dev/null; then + SKIP_NEXT="YES" + fi +} + # skip next test if OpenSSL can't send SSLv2 ClientHello requires_openssl_with_sslv2() { if [ -z "${OPENSSL_HAS_SSL2:-}" ]; then @@ -1073,6 +1080,7 @@ run_test "Renegotiation: none, for reference" \ -S "=> renegotiate" \ -S "write hello request" +requires_config_disabled POLARSSL_SSL_DISABLE_RENEGOTIATION run_test "Renegotiation: client-initiated" \ "$P_SRV debug_level=3 exchanges=2 renegotiation=1" \ "$P_CLI debug_level=3 exchanges=2 renegotiation=1 renegotiate=1" \ @@ -1086,6 +1094,7 @@ run_test "Renegotiation: client-initiated" \ -s "=> renegotiate" \ -S "write hello request" +requires_config_disabled POLARSSL_SSL_DISABLE_RENEGOTIATION run_test "Renegotiation: server-initiated" \ "$P_SRV debug_level=3 exchanges=2 renegotiation=1 renegotiate=1" \ "$P_CLI debug_level=3 exchanges=2 renegotiation=1" \ @@ -1102,6 +1111,7 @@ run_test "Renegotiation: server-initiated" \ # 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_disabled POLARSSL_SSL_DISABLE_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" \ @@ -1119,6 +1129,7 @@ run_test "Renegotiation: Signature Algorithms parsing, client-initiated" \ # 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_disabled POLARSSL_SSL_DISABLE_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" \ @@ -1133,6 +1144,7 @@ run_test "Renegotiation: Signature Algorithms parsing, server-initiated" \ -s "write hello request" \ -S "client hello v3, signature_algorithm ext: 2" # Is SHA-1 negotiated? +requires_config_disabled POLARSSL_SSL_DISABLE_RENEGOTIATION run_test "Renegotiation: double" \ "$P_SRV debug_level=3 exchanges=2 renegotiation=1 renegotiate=1" \ "$P_CLI debug_level=3 exchanges=2 renegotiation=1 renegotiate=1" \ @@ -1146,6 +1158,7 @@ run_test "Renegotiation: double" \ -s "=> renegotiate" \ -s "write hello request" +requires_config_disabled POLARSSL_SSL_DISABLE_RENEGOTIATION run_test "Renegotiation: client-initiated, server-rejected" \ "$P_SRV debug_level=3 exchanges=2 renegotiation=0" \ "$P_CLI debug_level=3 exchanges=2 renegotiation=1 renegotiate=1" \ @@ -1161,6 +1174,7 @@ run_test "Renegotiation: client-initiated, server-rejected" \ -c "SSL - Unexpected message at ServerHello in renegotiation" \ -c "failed" +requires_config_disabled POLARSSL_SSL_DISABLE_RENEGOTIATION run_test "Renegotiation: server-initiated, client-rejected, default" \ "$P_SRV debug_level=3 exchanges=2 renegotiation=1 renegotiate=1" \ "$P_CLI debug_level=3 exchanges=2 renegotiation=0" \ @@ -1176,6 +1190,7 @@ run_test "Renegotiation: server-initiated, client-rejected, default" \ -S "SSL - An unexpected message was received from our peer" \ -S "failed" +requires_config_disabled POLARSSL_SSL_DISABLE_RENEGOTIATION run_test "Renegotiation: server-initiated, client-rejected, not enforced" \ "$P_SRV debug_level=3 exchanges=2 renegotiation=1 renegotiate=1 \ renego_delay=-1" \ @@ -1193,6 +1208,7 @@ run_test "Renegotiation: server-initiated, client-rejected, not enforced" \ -S "failed" # delay 2 for 1 alert record + 1 application data record +requires_config_disabled POLARSSL_SSL_DISABLE_RENEGOTIATION run_test "Renegotiation: server-initiated, client-rejected, delay 2" \ "$P_SRV debug_level=3 exchanges=2 renegotiation=1 renegotiate=1 \ renego_delay=2" \ @@ -1209,6 +1225,7 @@ run_test "Renegotiation: server-initiated, client-rejected, delay 2" \ -S "SSL - An unexpected message was received from our peer" \ -S "failed" +requires_config_disabled POLARSSL_SSL_DISABLE_RENEGOTIATION run_test "Renegotiation: server-initiated, client-rejected, delay 0" \ "$P_SRV debug_level=3 exchanges=2 renegotiation=1 renegotiate=1 \ renego_delay=0" \ @@ -1224,6 +1241,7 @@ run_test "Renegotiation: server-initiated, client-rejected, delay 0" \ -s "write hello request" \ -s "SSL - An unexpected message was received from our peer" +requires_config_disabled POLARSSL_SSL_DISABLE_RENEGOTIATION run_test "Renegotiation: server-initiated, client-accepted, delay 0" \ "$P_SRV debug_level=3 exchanges=2 renegotiation=1 renegotiate=1 \ renego_delay=0" \ @@ -1240,6 +1258,7 @@ run_test "Renegotiation: server-initiated, client-accepted, delay 0" \ -S "SSL - An unexpected message was received from our peer" \ -S "failed" +requires_config_disabled POLARSSL_SSL_DISABLE_RENEGOTIATION run_test "Renegotiation: periodic, just below period" \ "$P_SRV debug_level=3 exchanges=9 renegotiation=1 renego_period=3" \ "$P_CLI debug_level=3 exchanges=2 renegotiation=1" \ @@ -1257,6 +1276,7 @@ run_test "Renegotiation: periodic, just below period" \ -S "failed" # one extra exchange to be able to complete renego +requires_config_disabled POLARSSL_SSL_DISABLE_RENEGOTIATION run_test "Renegotiation: periodic, just above period" \ "$P_SRV debug_level=3 exchanges=9 renegotiation=1 renego_period=3" \ "$P_CLI debug_level=3 exchanges=4 renegotiation=1" \ @@ -1273,6 +1293,7 @@ run_test "Renegotiation: periodic, just above period" \ -S "SSL - An unexpected message was received from our peer" \ -S "failed" +requires_config_disabled POLARSSL_SSL_DISABLE_RENEGOTIATION run_test "Renegotiation: periodic, two times period" \ "$P_SRV debug_level=3 exchanges=9 renegotiation=1 renego_period=3" \ "$P_CLI debug_level=3 exchanges=7 renegotiation=1" \ @@ -1289,6 +1310,7 @@ run_test "Renegotiation: periodic, two times period" \ -S "SSL - An unexpected message was received from our peer" \ -S "failed" +requires_config_disabled POLARSSL_SSL_DISABLE_RENEGOTIATION run_test "Renegotiation: periodic, above period, disabled" \ "$P_SRV debug_level=3 exchanges=9 renegotiation=0 renego_period=3" \ "$P_CLI debug_level=3 exchanges=4 renegotiation=1" \ @@ -1305,6 +1327,7 @@ run_test "Renegotiation: periodic, above period, disabled" \ -S "SSL - An unexpected message was received from our peer" \ -S "failed" +requires_config_disabled POLARSSL_SSL_DISABLE_RENEGOTIATION run_test "Renegotiation: nbio, client-initiated" \ "$P_SRV debug_level=3 nbio=2 exchanges=2 renegotiation=1" \ "$P_CLI debug_level=3 nbio=2 exchanges=2 renegotiation=1 renegotiate=1" \ @@ -1318,6 +1341,7 @@ run_test "Renegotiation: nbio, client-initiated" \ -s "=> renegotiate" \ -S "write hello request" +requires_config_disabled POLARSSL_SSL_DISABLE_RENEGOTIATION run_test "Renegotiation: nbio, server-initiated" \ "$P_SRV debug_level=3 nbio=2 exchanges=2 renegotiation=1 renegotiate=1" \ "$P_CLI debug_level=3 nbio=2 exchanges=2 renegotiation=1" \ @@ -1331,6 +1355,7 @@ run_test "Renegotiation: nbio, server-initiated" \ -s "=> renegotiate" \ -s "write hello request" +requires_config_disabled POLARSSL_SSL_DISABLE_RENEGOTIATION run_test "Renegotiation: openssl server, client-initiated" \ "$O_SRV" \ "$P_CLI debug_level=3 exchanges=1 renegotiation=1 renegotiate=1" \ @@ -1343,6 +1368,7 @@ run_test "Renegotiation: openssl server, client-initiated" \ -c "HTTP/1.0 200 [Oo][Kk]" requires_gnutls +requires_config_disabled POLARSSL_SSL_DISABLE_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" \ @@ -1355,6 +1381,7 @@ run_test "Renegotiation: gnutls server strict, client-initiated" \ -c "HTTP/1.0 200 [Oo][Kk]" requires_gnutls +requires_config_disabled POLARSSL_SSL_DISABLE_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" \ @@ -1367,6 +1394,7 @@ run_test "Renegotiation: gnutls server unsafe, client-initiated default" \ -C "HTTP/1.0 200 [Oo][Kk]" requires_gnutls +requires_config_disabled POLARSSL_SSL_DISABLE_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 \ @@ -1380,6 +1408,7 @@ run_test "Renegotiation: gnutls server unsafe, client-inititated no legacy" \ -C "HTTP/1.0 200 [Oo][Kk]" requires_gnutls +requires_config_disabled POLARSSL_SSL_DISABLE_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 \ From be812f68c5e2826a84905f65f5a1359a6e813d40 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 25 Oct 2017 09:49:13 +0100 Subject: [PATCH 5/6] Add build and ssl-opt.sh run for SSL_DISABLE_RENEGOTIATION to all.sh --- tests/scripts/all.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 133d98677..cb255c476 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -259,6 +259,19 @@ OPENSSL_CMD="$OPENSSL_LEGACY" tests/compat.sh -m 'ssl3' msg "build: SSLv3 - ssl-opt.sh (ASan build)" # ~ 6 min tests/ssl-opt.sh +msg "build: Default + POLARSSL_SSL_DISABLE_RENEGOTIATION (ASan build)" # ~ 6 min +cleanup +cp "$CONFIG_H" "$CONFIG_BAK" +scripts/config.pl set POLARSSL_SSL_DISABLE_RENEGOTIATION +CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan . +make + +msg "test: POLARSSL_SSL_DISABLE_RENEGOTIATION - main suites (inc. selftests) (ASan build)" # ~ 50s +make test + +msg "test: POLARSSL_SSL_DISABLE_RENEGOTIATION - ssl-opt.sh (ASan build)" # ~ 6 min +tests/ssl-opt.sh + msg "build: cmake, full config, clang" # ~ 50s cleanup cp "$CONFIG_H" "$CONFIG_BAK" From 18710eb10247d7881cd336688d462264253f892a Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Wed, 25 Oct 2017 09:50:22 +0100 Subject: [PATCH 6/6] Adapt ChangeLog --- ChangeLog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog b/ChangeLog index 6a1be9892..bbd1a54c7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -14,6 +14,8 @@ Bugfix * Fix leap year calculation in x509_date_is_valid() to ensure that invalid dates on leap years with 100 and 400 intervals are handled correctly. Found by Nicholas Wilson. #694 + * Fix handling of handshake messages in ssl_read in case + POLARSSL_SSL_DISABLE_RENEGOTIATION is set. Found by erja-gp. = mbed TLS 1.3.21 branch released 2017-08-10