From d5746b36f9cb0ca3675a5237d1f805a2b9279555 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 13 Jan 2015 20:33:24 +0100 Subject: [PATCH 1/6] Fix warning --- 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 22bd6974c7..ac740585c6 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -4846,7 +4846,7 @@ int ssl_write( ssl_context *ssl, const unsigned char *buf, size_t len ) return( ssl_write_real( ssl, buf, len ) ); } - if( ssl->split_done == 0 ) + if( ssl->split_done != 1 ) { ssl->split_done = 1; if( ( ret = ssl_write_real( ssl, buf, 1 ) ) < 0 ) From a852cf4833cfd8c86757ba89e15f3645499e4eca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 13 Jan 2015 20:56:15 +0100 Subject: [PATCH 2/6] Fix issue with non-blocking I/O & record splitting --- library/ssl_tls.c | 15 ++++++--------- tests/ssl-opt.sh | 9 +++++++++ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index ac740585c6..6b9df33695 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -4846,19 +4846,16 @@ int ssl_write( ssl_context *ssl, const unsigned char *buf, size_t len ) return( ssl_write_real( ssl, buf, len ) ); } - if( ssl->split_done != 1 ) + if( ssl->split_done == 0 ) { - ssl->split_done = 1; - if( ( ret = ssl_write_real( ssl, buf, 1 ) ) < 0 ) + if( ( ret = ssl_write_real( ssl, buf, 1 ) ) <= 0 ) return( ret ); + ssl->split_done = 1; } - if( ssl->split_done == 1 ) - { - ssl->split_done = 0; - if( ( ret = ssl_write_real( ssl, buf + 1, len - 1 ) ) < 0 ) - return( ret ); - } + if( ( ret = ssl_write_real( ssl, buf + 1, len - 1 ) ) <= 0 ) + return( ret ); + ssl->split_done = 0; return( ret + 1 ); } diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index cd4f6a67f7..51c3fd0c6d 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -718,6 +718,15 @@ run_test "CBC Record splitting: TLS 1.0, splitting disabled" \ -S "Read from client: 1 bytes read" \ -S "122 bytes read" +run_test "CBC Record splitting: TLS 1.0, splitting, nbio" \ + "$P_SRV nbio=2" \ + "$P_CLI nbio=2 force_ciphersuite=TLS-RSA-WITH-AES-128-CBC-SHA \ + request_size=123 force_version=tls1" \ + 0 \ + -S "Read from client: 123 bytes read" \ + -s "Read from client: 1 bytes read" \ + -s "122 bytes read" + # Tests for Session Tickets run_test "Session resume using tickets: basic" \ From 78803c0567be807f333b196ae7defd4a47c40fbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 13 Jan 2015 21:20:22 +0100 Subject: [PATCH 3/6] Fix char signedness issue --- include/polarssl/ssl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/polarssl/ssl.h b/include/polarssl/ssl.h index 5f4b5f7508..5624ebe99b 100644 --- a/include/polarssl/ssl.h +++ b/include/polarssl/ssl.h @@ -836,7 +836,7 @@ struct _ssl_context unsigned char mfl_code; /*!< MaxFragmentLength chosen by us */ #endif /* POLARSSL_SSL_MAX_FRAGMENT_LENGTH */ #if defined(POLARSSL_SSL_CBC_RECORD_SPLITTING) - char split_done; /*!< flag for record splitting: + signed char split_done; /*!< flag for record splitting: -1 disabled, 0 todo, 1 done */ #endif From 687f89beabb56555061ab7e2a9fadfba10705c14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 13 Jan 2015 21:48:12 +0100 Subject: [PATCH 4/6] Don't check errors on ssl_close_notify() Depending on timing we might get different errors (conn_reset, write failed) and ignoring them all ends up being almost the same as just not checking errors. --- programs/ssl/ssl_client2.c | 20 +++----------------- programs/ssl/ssl_server2.c | 20 +++----------------- 2 files changed, 6 insertions(+), 34 deletions(-) diff --git a/programs/ssl/ssl_client2.c b/programs/ssl/ssl_client2.c index 07dee7f582..71f5efa301 100644 --- a/programs/ssl/ssl_client2.c +++ b/programs/ssl/ssl_client2.c @@ -1284,24 +1284,10 @@ send_request: close_notify: printf( " . Closing the connection..." ); - while( ( ret = ssl_close_notify( &ssl ) ) < 0 ) - { - if( ret == POLARSSL_ERR_NET_CONN_RESET ) - { - printf( " ok (already closed by peer)\n" ); - ret = 0; - goto reconnect; - } + /* Don't check for errors, the connection might already be closed */ + ssl_close_notify( &ssl ); - if( ret != POLARSSL_ERR_NET_WANT_READ && - ret != POLARSSL_ERR_NET_WANT_WRITE ) - { - printf( " failed\n ! ssl_close_notify returned %d\n\n", ret ); - goto reconnect; - } - } - - printf( " ok\n" ); + printf( " done\n" ); /* * 9. Reconnect? diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 2980cf5689..9ecbdf1e7a 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1763,24 +1763,10 @@ data_exchange: close_notify: printf( " . Closing the connection..." ); - while( ( ret = ssl_close_notify( &ssl ) ) < 0 ) - { - if( ret == POLARSSL_ERR_NET_CONN_RESET ) - { - printf( " ok (already closed by peer)\n" ); - ret = 0; - goto reset; - } + /* Don't check for errors, the connection might already be closed */ + ssl_close_notify( &ssl ); - if( ret != POLARSSL_ERR_NET_WANT_READ && - ret != POLARSSL_ERR_NET_WANT_WRITE ) - { - printf( " failed\n ! ssl_close_notify returned %d\n\n", ret ); - goto reset; - } - } - - printf( " ok\n" ); + printf( " done\n" ); goto reset; /* From a92ed4845ca242eb0cfd8c915570d9de82eb1142 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 14 Jan 2015 10:46:08 +0100 Subject: [PATCH 5/6] Fix stupid error in previous commit Since ret is no longer update by close_notify(), we need to reset it to 0 after a successful write. --- programs/ssl/ssl_server2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 9ecbdf1e7a..18513c9fd0 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1749,7 +1749,7 @@ data_exchange: buf[written] = '\0'; printf( " %d bytes written in %d fragments\n\n%s\n", written, frags, (char *) buf ); - + ret = 0; /* * 7b. Continue doing data exchanges? From 9835bc077a8880ec28c5aa27ca91f35cd480ec10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 14 Jan 2015 14:41:58 +0100 Subject: [PATCH 6/6] Fix racy test. With exchanges == renego period, sometimes the connection will be closed by the client before the server had time to read the ClientHello, making the test fail. The extra exchange avoids that. --- tests/ssl-opt.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh index 51c3fd0c6d..8bf7d97bff 100755 --- a/tests/ssl-opt.sh +++ b/tests/ssl-opt.sh @@ -1088,9 +1088,10 @@ run_test "Renegotiation: periodic, just below period" \ -S "SSL - An unexpected message was received from our peer" \ -S "failed" +# one extra exchange to be able to complete renego 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=3 renegotiation=1" \ + "$P_CLI debug_level=3 exchanges=4 renegotiation=1" \ 0 \ -c "client hello, adding renegotiation extension" \ -s "received TLS_EMPTY_RENEGOTIATION_INFO" \ @@ -1106,7 +1107,7 @@ run_test "Renegotiation: periodic, just above period" \ 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=6 renegotiation=1" \ + "$P_CLI debug_level=3 exchanges=7 renegotiation=1" \ 0 \ -c "client hello, adding renegotiation extension" \ -s "received TLS_EMPTY_RENEGOTIATION_INFO" \