Move dropping of unexpected AD records to after record decryption

With the introduction of the CID extension, the record content type
may change during decryption; we must therefore re-consider every
record content type check that happens before decryption, and either
move or duplicate it to ensure it also applies to records whose
real content type is only revealed during decryption.

This commit does this for the silent dropping of unexpected
ApplicationData records in DTLS. Previously, this was caught
in ssl_parse_record_header(), returning
MBEDTLS_ERR_SSL_UNEXPECTED_RECORD which in ssl_get_next_record()
would lead to silent skipping of the record.

When using CID, this check wouldn't trigger e.g. when delayed
encrypted ApplicationData records come on a CID-based connection
during a renegotiation.

This commit moves the check to mbedtls_ssl_handle_message_type()
and returns MBEDTLS_ERR_SSL_NON_FATAL if it triggers, which leads
so silent skipover in the caller mbedtls_ssl_read_record().
This commit is contained in:
Hanno Becker 2019-05-03 16:54:26 +01:00
parent f5970a0945
commit 74dd3a70d8

View file

@ -4510,20 +4510,6 @@ static int ssl_parse_record_header( mbedtls_ssl_context *ssl )
return( MBEDTLS_ERR_SSL_UNEXPECTED_RECORD ); return( MBEDTLS_ERR_SSL_UNEXPECTED_RECORD );
} }
#endif #endif
/* Drop unexpected ApplicationData records,
* except at the beginning of renegotiations */
if( ssl->in_msgtype == MBEDTLS_SSL_MSG_APPLICATION_DATA &&
ssl->state != MBEDTLS_SSL_HANDSHAKE_OVER
#if defined(MBEDTLS_SSL_RENEGOTIATION)
&& ! ( ssl->renego_status == MBEDTLS_SSL_RENEGOTIATION_IN_PROGRESS &&
ssl->state == MBEDTLS_SSL_SERVER_HELLO )
#endif
)
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "dropping unexpected ApplicationData" ) );
return( MBEDTLS_ERR_SSL_UNEXPECTED_RECORD );
}
} }
#endif /* MBEDTLS_SSL_PROTO_DTLS */ #endif /* MBEDTLS_SSL_PROTO_DTLS */
@ -5632,13 +5618,29 @@ int mbedtls_ssl_handle_message_type( mbedtls_ssl_context *ssl )
} }
#if defined(MBEDTLS_SSL_PROTO_DTLS) #if defined(MBEDTLS_SSL_PROTO_DTLS)
if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM && if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM )
ssl->handshake != NULL &&
ssl->state == MBEDTLS_SSL_HANDSHAKE_OVER )
{ {
ssl_handshake_wrapup_free_hs_transform( ssl ); /* Drop unexpected ApplicationData records,
} * except at the beginning of renegotiations */
if( ssl->in_msgtype == MBEDTLS_SSL_MSG_APPLICATION_DATA &&
ssl->state != MBEDTLS_SSL_HANDSHAKE_OVER
#if defined(MBEDTLS_SSL_RENEGOTIATION)
&& ! ( ssl->renego_status == MBEDTLS_SSL_RENEGOTIATION_IN_PROGRESS &&
ssl->state == MBEDTLS_SSL_SERVER_HELLO )
#endif #endif
)
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "dropping unexpected ApplicationData" ) );
return( MBEDTLS_ERR_SSL_NON_FATAL );
}
if( ssl->handshake != NULL &&
ssl->state == MBEDTLS_SSL_HANDSHAKE_OVER )
{
ssl_handshake_wrapup_free_hs_transform( ssl );
}
}
#endif /* MBEDTLS_SSL_DTLS */
return( 0 ); return( 0 );
} }