Fix bug in handling of DTLS client hard reconnect

We keep track of the current epoch and record sequence number in out_ctr,
which was overwritten when writing the record containing the
HelloVerifyRequest starting from out_buf. We can avoid that by only using the
rest of the buffer.

Using MBEDTLS_SSL_MAX_CONTENT_LEN as the buffer size is still correct, as it
was a pretty conservative value when starting from out_buf.

Note: this bug was also fixed unknowingly in 2.13 by introducing a new buffer
that holds the current value of the sequence number (including epoch), while
working on datagram packing: 198594709b

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This commit is contained in:
Manuel Pégourié-Gonnard 2020-03-31 12:49:27 +02:00
parent 4bbbdc36bc
commit 6062b49d29
2 changed files with 15 additions and 3 deletions

View File

@ -2,6 +2,15 @@ mbed TLS ChangeLog (Sorted per branch, date)
= mbed TLS x.x.x branch released xxxx-xx-xx = mbed TLS x.x.x branch released xxxx-xx-xx
Security
* Fix bug in DTLS handling of new associations with the same parameters
(RFC 6347 section 4.2.8): after sending its HelloVerifyRequest, the
server would end up with corrupted state and only send invalid records to
the client. An attacker able to send forged UDP packets to the server
could use that to obtain a Denial of Service. This could only happen when
MBEDTLS_SSL_DTLS_CLIENT_PORT_REUSE was enabled in config.h (which it is
by default).
Bugfix Bugfix
* Fix compilation failure when both MBEDTLS_SSL_PROTO_DTLS and * Fix compilation failure when both MBEDTLS_SSL_PROTO_DTLS and
MBEDTLS_SSL_HW_RECORD_ACCEL are enabled. MBEDTLS_SSL_HW_RECORD_ACCEL are enabled.

View File

@ -3598,13 +3598,16 @@ static int ssl_handle_possible_reconnect( mbedtls_ssl_context *ssl )
int ret; int ret;
size_t len; size_t len;
/* Use out_msg as temporary buffer for writing out HelloVerifyRequest,
* because the output buffer's already around. Don't use out_buf though,
* as we don't want to overwrite out_ctr. */
ret = ssl_check_dtls_clihlo_cookie( ret = ssl_check_dtls_clihlo_cookie(
ssl->conf->f_cookie_write, ssl->conf->f_cookie_write,
ssl->conf->f_cookie_check, ssl->conf->f_cookie_check,
ssl->conf->p_cookie, ssl->conf->p_cookie,
ssl->cli_id, ssl->cli_id_len, ssl->cli_id, ssl->cli_id_len,
ssl->in_buf, ssl->in_left, ssl->in_buf, ssl->in_left,
ssl->out_buf, MBEDTLS_SSL_MAX_CONTENT_LEN, &len ); ssl->out_msg, MBEDTLS_SSL_MAX_CONTENT_LEN, &len );
MBEDTLS_SSL_DEBUG_RET( 2, "ssl_check_dtls_clihlo_cookie", ret ); MBEDTLS_SSL_DEBUG_RET( 2, "ssl_check_dtls_clihlo_cookie", ret );
@ -3613,11 +3616,11 @@ static int ssl_handle_possible_reconnect( mbedtls_ssl_context *ssl )
int send_ret; int send_ret;
MBEDTLS_SSL_DEBUG_MSG( 1, ( "sending HelloVerifyRequest" ) ); MBEDTLS_SSL_DEBUG_MSG( 1, ( "sending HelloVerifyRequest" ) );
MBEDTLS_SSL_DEBUG_BUF( 4, "output record sent to network", MBEDTLS_SSL_DEBUG_BUF( 4, "output record sent to network",
ssl->out_buf, len ); ssl->out_msg, len );
/* Don't check write errors as we can't do anything here. /* Don't check write errors as we can't do anything here.
* If the error is permanent we'll catch it later, * If the error is permanent we'll catch it later,
* if it's not, then hopefully it'll work next time. */ * if it's not, then hopefully it'll work next time. */
send_ret = ssl->f_send( ssl->p_bio, ssl->out_buf, len ); send_ret = ssl->f_send( ssl->p_bio, ssl->out_msg, len );
MBEDTLS_SSL_DEBUG_RET( 2, "ssl->f_send", send_ret ); MBEDTLS_SSL_DEBUG_RET( 2, "ssl->f_send", send_ret );
(void) send_ret; (void) send_ret;