mirror of
https://github.com/yuzu-emu/mbedtls.git
synced 2024-12-25 17:25:33 +00:00
Fix possible close_notify/ClientHello confusion
The ssl-opt.sh test cases using session resumption tend to fail occasionally on the CI due to a race condition in how ssl_server2 and ssl_client2 handle the reconnection cycle. The server does the following in order: - S1 send application data - S2 send a close_notify alert - S3 close the client socket - S4 wait for a "new connection" (actually a new datagram) - S5 start a handshake The client does the following in order: - C1 wait for and read application data from the server - C2 send a close_notify alert - C3 close the server socket - C4 reset session data and re-open a server socket - C5 start a handshake If the client has been able to send the close_notify (C2) and if has been delivered to the server before if closes the client socket (S3), when the server reaches S4, the datagram that we start the new connection will be the ClientHello and everything will be fine. However if S3 wins the race and happens before the close_notify is delivered, in S4 the close_notify is what will be seen as the first datagram in a new connection, and then in S5 this will rightfully be rejected as not being a valid ClientHello and the server will close the connection (and go wait for another one). The client will then fail to read from the socket and exit non-zero and the ssl-opt.sh harness will correctly report this as a failure. In order to avoid this race condition in test using ssl_client2 and ssl_server2, this commits introduces a new command-line option skip_close_notify to ssl_client2 and uses it in all ssl-opt.sh tests that use session resumption with DTLS and ssl_server2. This works because ssl_server2 knows how many messages it expects in each direction and in what order, and closes the connection after that rather than relying on close_notify (which is also why there was a race in the first place). Tests that use another server (in practice there are two of them, using OpenSSL as a server) wouldn't work with skip_close_notify, as the server won't close the connection until the client sends a close_notify, but for the same reason they don't need it (there is no race between receiving close_notify and closing as the former is the cause of the later). An alternative approach would be to make ssl_server2 keep the connection open until it receives a close_notify. Unfortunately it creates problems for tests where we simulate a lossy network, as the close_notify could be lost (and the client can't retransmit it). We could modify udp_proxy with an option to never drop alert messages, but when TLS 1.3 comes that would no longer work as the type of messages will be encrypted. Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This commit is contained in:
Manuel Pégourié-Gonnard
parent
72371b2022
commit
5e261e958c
|
|
@ -108,6 +108,7 @@ int main( void )
|
|||
#define DFL_FALLBACK -1
|
||||
#define DFL_EXTENDED_MS -1
|
||||
#define DFL_ETM -1
|
||||
#define DFL_SKIP_CLOSE_NOTIFY 0
|
||||
|
||||
#define GET_REQUEST "GET %s HTTP/1.0\r\nExtra-header: "
|
||||
#define GET_REQUEST_END "\r\n\r\n"
|
||||
|
|
@ -256,6 +257,7 @@ int main( void )
|
|||
" options: 1 (non-blocking), 2 (added delays)\n" \
|
||||
" read_timeout=%%d default: 0 ms (no timeout)\n" \
|
||||
" max_resend=%%d default: 0 (no resend on timeout)\n" \
|
||||
" skip_close_notify=%%d default: 0 (send close_notify)\n" \
|
||||
"\n" \
|
||||
USAGE_DTLS \
|
||||
"\n" \
|
||||
|
|
@ -344,6 +346,7 @@ struct options
|
|||
int fallback; /* is this a fallback connection? */
|
||||
int extended_ms; /* negotiate extended master secret? */
|
||||
int etm; /* negotiate encrypt then mac? */
|
||||
int skip_close_notify; /* skip sending the close_notify alert */
|
||||
} opt;
|
||||
|
||||
static void my_debug( void *ctx, int level,
|
||||
|
|
@ -562,6 +565,7 @@ int main( int argc, char *argv[] )
|
|||
opt.fallback = DFL_FALLBACK;
|
||||
opt.extended_ms = DFL_EXTENDED_MS;
|
||||
opt.etm = DFL_ETM;
|
||||
opt.skip_close_notify = DFL_SKIP_CLOSE_NOTIFY;
|
||||
|
||||
for( i = 1; i < argc; i++ )
|
||||
{
|
||||
|
|
@ -864,6 +868,12 @@ int main( int argc, char *argv[] )
|
|||
if( opt.dhmlen < 0 )
|
||||
goto usage;
|
||||
}
|
||||
else if( strcmp( p, "skip_close_notify" ) == 0 )
|
||||
{
|
||||
opt.skip_close_notify = atoi( q );
|
||||
if( opt.skip_close_notify < 0 || opt.skip_close_notify > 1 )
|
||||
goto usage;
|
||||
}
|
||||
else
|
||||
goto usage;
|
||||
}
|
||||
|
|
@ -1733,10 +1743,25 @@ close_notify:
|
|||
mbedtls_printf( " . Closing the connection..." );
|
||||
fflush( stdout );
|
||||
|
||||
/* No error checking, the connection might be closed already */
|
||||
do ret = mbedtls_ssl_close_notify( &ssl );
|
||||
while( ret == MBEDTLS_ERR_SSL_WANT_WRITE );
|
||||
ret = 0;
|
||||
/*
|
||||
* Most of the time sending a close_notify before closing is the right
|
||||
* thing to do. However, when the server already knows how many messages
|
||||
* are expected and closes the connection by itself, this alert becomes
|
||||
* redundant. Sometimes with DTLS this redundancy becomes a problem by
|
||||
* leading to a race condition where the server might close the connection
|
||||
* before seeing the alert, and since UDP is connection-less when the
|
||||
* alert arrives it will be seen as a new connection, which will fail as
|
||||
* the alert is clearly not a valid ClientHello. This may cause spurious
|
||||
* failures in tests that use DTLS and resumption with ssl_server2 in
|
||||
* ssl-opt.sh, avoided by enabling skip_close_notify client-side.
|
||||
*/
|
||||
if( opt.skip_close_notify == 0 )
|
||||
{
|
||||
/* No error checking, the connection might be closed already */
|
||||
do ret = mbedtls_ssl_close_notify( &ssl );
|
||||
while( ret == MBEDTLS_ERR_SSL_WANT_WRITE );
|
||||
ret = 0;
|
||||
}
|
||||
|
||||
mbedtls_printf( " done\n" );
|
||||
|
||||
|
|
|
|||
|
|
@ -1394,7 +1394,7 @@ run_test "Session resume using tickets: openssl client" \
|
|||
|
||||
run_test "Session resume using tickets, DTLS: basic" \
|
||||
"$P_SRV debug_level=3 dtls=1 tickets=1" \
|
||||
"$P_CLI debug_level=3 dtls=1 tickets=1 reconnect=1" \
|
||||
"$P_CLI debug_level=3 dtls=1 tickets=1 reconnect=1 skip_close_notify=1" \
|
||||
0 \
|
||||
-c "client hello, adding session ticket extension" \
|
||||
-s "found session ticket extension" \
|
||||
|
|
@ -1408,7 +1408,7 @@ run_test "Session resume using tickets, DTLS: basic" \
|
|||
|
||||
run_test "Session resume using tickets, DTLS: cache disabled" \
|
||||
"$P_SRV debug_level=3 dtls=1 tickets=1 cache_max=0" \
|
||||
"$P_CLI debug_level=3 dtls=1 tickets=1 reconnect=1" \
|
||||
"$P_CLI debug_level=3 dtls=1 tickets=1 reconnect=1 skip_close_notify=1" \
|
||||
0 \
|
||||
-c "client hello, adding session ticket extension" \
|
||||
-s "found session ticket extension" \
|
||||
|
|
@ -1422,7 +1422,7 @@ run_test "Session resume using tickets, DTLS: cache disabled" \
|
|||
|
||||
run_test "Session resume using tickets, DTLS: timeout" \
|
||||
"$P_SRV debug_level=3 dtls=1 tickets=1 cache_max=0 ticket_timeout=1" \
|
||||
"$P_CLI debug_level=3 dtls=1 tickets=1 reconnect=1 reco_delay=2" \
|
||||
"$P_CLI debug_level=3 dtls=1 tickets=1 reconnect=1 skip_close_notify=1 reco_delay=2" \
|
||||
0 \
|
||||
-c "client hello, adding session ticket extension" \
|
||||
-s "found session ticket extension" \
|
||||
|
|
@ -1554,7 +1554,7 @@ run_test "Session resume using cache: openssl server" \
|
|||
|
||||
run_test "Session resume using cache, DTLS: tickets enabled on client" \
|
||||
"$P_SRV dtls=1 debug_level=3 tickets=0" \
|
||||
"$P_CLI dtls=1 debug_level=3 tickets=1 reconnect=1" \
|
||||
"$P_CLI dtls=1 debug_level=3 tickets=1 reconnect=1 skip_close_notify=1" \
|
||||
0 \
|
||||
-c "client hello, adding session ticket extension" \
|
||||
-s "found session ticket extension" \
|
||||
|
|
@ -1568,7 +1568,7 @@ run_test "Session resume using cache, DTLS: tickets enabled on client" \
|
|||
|
||||
run_test "Session resume using cache, DTLS: tickets enabled on server" \
|
||||
"$P_SRV dtls=1 debug_level=3 tickets=1" \
|
||||
"$P_CLI dtls=1 debug_level=3 tickets=0 reconnect=1" \
|
||||
"$P_CLI dtls=1 debug_level=3 tickets=0 reconnect=1 skip_close_notify=1" \
|
||||
0 \
|
||||
-C "client hello, adding session ticket extension" \
|
||||
-S "found session ticket extension" \
|
||||
|
|
@ -1582,7 +1582,7 @@ run_test "Session resume using cache, DTLS: tickets enabled on server" \
|
|||
|
||||
run_test "Session resume using cache, DTLS: cache_max=0" \
|
||||
"$P_SRV dtls=1 debug_level=3 tickets=0 cache_max=0" \
|
||||
"$P_CLI dtls=1 debug_level=3 tickets=0 reconnect=1" \
|
||||
"$P_CLI dtls=1 debug_level=3 tickets=0 reconnect=1 skip_close_notify=1" \
|
||||
0 \
|
||||
-S "session successfully restored from cache" \
|
||||
-S "session successfully restored from ticket" \
|
||||
|
|
@ -1591,7 +1591,7 @@ run_test "Session resume using cache, DTLS: cache_max=0" \
|
|||
|
||||
run_test "Session resume using cache, DTLS: cache_max=1" \
|
||||
"$P_SRV dtls=1 debug_level=3 tickets=0 cache_max=1" \
|
||||
"$P_CLI dtls=1 debug_level=3 tickets=0 reconnect=1" \
|
||||
"$P_CLI dtls=1 debug_level=3 tickets=0 reconnect=1 skip_close_notify=1" \
|
||||
0 \
|
||||
-s "session successfully restored from cache" \
|
||||
-S "session successfully restored from ticket" \
|
||||
|
|
@ -1600,7 +1600,7 @@ run_test "Session resume using cache, DTLS: cache_max=1" \
|
|||
|
||||
run_test "Session resume using cache, DTLS: timeout > delay" \
|
||||
"$P_SRV dtls=1 debug_level=3 tickets=0" \
|
||||
"$P_CLI dtls=1 debug_level=3 tickets=0 reconnect=1 reco_delay=0" \
|
||||
"$P_CLI dtls=1 debug_level=3 tickets=0 reconnect=1 skip_close_notify=1 reco_delay=0" \
|
||||
0 \
|
||||
-s "session successfully restored from cache" \
|
||||
-S "session successfully restored from ticket" \
|
||||
|
|
@ -1609,7 +1609,7 @@ run_test "Session resume using cache, DTLS: timeout > delay" \
|
|||
|
||||
run_test "Session resume using cache, DTLS: timeout < delay" \
|
||||
"$P_SRV dtls=1 debug_level=3 tickets=0 cache_timeout=1" \
|
||||
"$P_CLI dtls=1 debug_level=3 tickets=0 reconnect=1 reco_delay=2" \
|
||||
"$P_CLI dtls=1 debug_level=3 tickets=0 reconnect=1 skip_close_notify=1 reco_delay=2" \
|
||||
0 \
|
||||
-S "session successfully restored from cache" \
|
||||
-S "session successfully restored from ticket" \
|
||||
|
|
@ -1618,7 +1618,7 @@ run_test "Session resume using cache, DTLS: timeout < delay" \
|
|||
|
||||
run_test "Session resume using cache, DTLS: no timeout" \
|
||||
"$P_SRV dtls=1 debug_level=3 tickets=0 cache_timeout=0" \
|
||||
"$P_CLI dtls=1 debug_level=3 tickets=0 reconnect=1 reco_delay=2" \
|
||||
"$P_CLI dtls=1 debug_level=3 tickets=0 reconnect=1 skip_close_notify=1 reco_delay=2" \
|
||||
0 \
|
||||
-s "session successfully restored from cache" \
|
||||
-S "session successfully restored from ticket" \
|
||||
|
|
@ -5309,7 +5309,7 @@ run_test "DTLS proxy: 3d, min handshake, resumption" \
|
|||
"$P_SRV dtls=1 hs_timeout=250-10000 tickets=0 auth_mode=none \
|
||||
psk=abc123 debug_level=3" \
|
||||
"$P_CLI dtls=1 hs_timeout=250-10000 tickets=0 psk=abc123 \
|
||||
debug_level=3 reconnect=1 read_timeout=1000 max_resend=10 \
|
||||
debug_level=3 reconnect=1 skip_close_notify=1 read_timeout=1000 max_resend=10 \
|
||||
force_ciphersuite=TLS-PSK-WITH-AES-128-CCM-8" \
|
||||
0 \
|
||||
-s "a session has been resumed" \
|
||||
|
|
@ -5323,7 +5323,7 @@ run_test "DTLS proxy: 3d, min handshake, resumption, nbio" \
|
|||
"$P_SRV dtls=1 hs_timeout=250-10000 tickets=0 auth_mode=none \
|
||||
psk=abc123 debug_level=3 nbio=2" \
|
||||
"$P_CLI dtls=1 hs_timeout=250-10000 tickets=0 psk=abc123 \
|
||||
debug_level=3 reconnect=1 read_timeout=1000 max_resend=10 \
|
||||
debug_level=3 reconnect=1 skip_close_notify=1 read_timeout=1000 max_resend=10 \
|
||||
force_ciphersuite=TLS-PSK-WITH-AES-128-CCM-8 nbio=2" \
|
||||
0 \
|
||||
-s "a session has been resumed" \
|
||||
|
|
|
|||
Loading…
Reference in a new issue