yuzu/mbedtls
1
0
Fork 0
mirror of https://github.com/yuzu-emu/mbedtls.git synced 2025-01-23 13:51:03 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix mbedtls_ssl_read

Browse source 
Don't fetch a new record in mbedtls_ssl_read_record_layer as long as an application data record is being processed.
This commit is contained in:
Hanno Becker 2017-06-08 13:38:05 +01:00
parent 704f493730
commit 6a582e80f2
3 changed files with 198 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

74
include/mbedtls/ssl_internal.h
View file

@ -387,6 +387,80 @@ int mbedtls_ssl_send_fatal_handshake_failure( mbedtls_ssl_context *ssl );
void mbedtls_ssl_reset_checksum( mbedtls_ssl_context *ssl );
int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl );
/**
* \brief Update record layer
*
* This function roughly separates the implementation
* of the logic of (D)TLS from the implementation
* of the secure transport.
*
* \param ssl SSL context to use
*
* \return 0 or non-zero error code.
*
* \note A clarification on what is called 'record layer' here
* is in order, as many sensible definitions are possible:
*
* The record layer takes as input an untrusted underlying
* transport (stream or datagram) and transforms it into
* a serially multiplexed, secure transport, which
* conceptually provides the following:
*
* (1) Three datagram based, content-agnostic transports
* for handshake, alert and CCS messages.
* (2) One stream- or datagram-based transport
* for application data.
* (3) Functionality for changing the underlying transform
* securing the contents.
*
* The interface to this functionality is given as follows:
*
* a Updating
* [Currently implemented by mbedtls_ssl_read_record]
*
* Check if and on which of the four 'ports' data is pending:
* Nothing, a controlling datagram of type (1), or application
* data (2). In any case data is present, internal buffers
* provide access to the data for the user to process it.
* Consumption of type (1) datagrams is done automatically
* on the next update, invalidating that the internal buffers
* for previous datagrams, while consumption of application
* data (2) is user-controlled.
*
* b Reading of application data
* [Currently manual adaption of ssl->in_offt pointer]
*
* As mentioned in the last paragraph, consumption of data
* is different from the automatic consumption of control
* datagrams (1) because application data is treated as a stream.
*
* c Tracking availability of application data
* [Currently manually through decreasing ssl->in_msglen]
*
* For efficiency and to retain datagram semantics for
* application data in case of DTLS, the record layer
* provides functionality for checking how much application
* data is still available in the internal buffer.
*
* d Changing the transformation securing the communication.
*
* Given an opaque implementation of the record layer in the
* above sense, it should be possible to implement the logic
* of (D)TLS on top of it without the need to know anything
* about the record layer's internals. This is done e.g.
* in all the handshake handling functions, and in the
* application data reading function mbedtls_ssl_read.
*
* \note The above tries to give a conceptual picture of the
* record layer, but the current implementation deviates
* from it in some places. For example, our implementation of
* the update functionality through mbedtls_ssl_read_record
* discards datagrams depending on the current state, which
* wouldn't fall under the record layer's responsibility
* following the above definition.
*
*/
int mbedtls_ssl_read_record( mbedtls_ssl_context *ssl );
int mbedtls_ssl_fetch_input( mbedtls_ssl_context *ssl, size_t nb_want );

2
library/ssl_cli.c
View file

@ -1304,6 +1304,8 @@ static int ssl_parse_server_hello( mbedtls_ssl_context *ssl )
}
MBEDTLS_SSL_DEBUG_MSG( 1, ( "non-handshake message during renego" ) );
ssl->keep_current_message = 1;
return( MBEDTLS_ERR_SSL_WAITING_SERVER_HELLO_RENEGO );
}
#endif /* MBEDTLS_SSL_RENEGOTIATION */

129
library/ssl_tls.c
View file

@ -3725,13 +3725,59 @@ int mbedtls_ssl_read_record( mbedtls_ssl_context *ssl )
return( 0 );
}
if( ssl->in_hslen != 0 && ssl->in_hslen < ssl->in_msglen )
/*
* Step A
*
* Consume last content-layer message and potentially
* update in_msglen which keeps track of the contents'
* consumption state.
*
* (1) Handshake messages:
* Remove last handshake message, move content
* and adapt in_msglen.
*
* (2) Alert messages:
* Consume whole record content, in_msglen = 0.
*
* NOTE: This needs to be fixed, since like for
* handshake messages it is allowed to have
* multiple alerts witin a single record.
*
* (3) Change cipher spec:
* Consume whole record content, in_msglen = 0.
*
* (4) Application data:
* Don't do anything - the record layer provides
* the application data as a stream transport
* and consumes through mbedtls_ssl_read only.
*
*/
/* Case (1): Handshake messages */
if( ssl->in_hslen != 0 )
{
/*
* Get next Handshake message in the current record
*/
ssl->in_msglen -= ssl->in_hslen;
/* Notes:
* (1) in_hslen is *NOT* necessarily the size of the
* current handshake content: If DTLS handshake
* fragmentation is used, that's the fragment
* size instead. Using the total handshake message
* size here is FAULTY and should be changed at
* some point. Internal reference IOTSSL-1414.
* (2) While it doesn't seem to cause problems, one
* has to be very careful not to assume that in_hslen
* is always <= in_msglen in a sensible communication.
* Again, it's wrong for DTLS handshake fragmentation.
* The following check is therefore mandatory, and
* should not be treated as a silently corrected assertion.
*/
if( ssl->in_hslen < ssl->in_msglen )
{
ssl->in_msglen -= ssl->in_hslen;
memmove( ssl->in_msg, ssl->in_msg + ssl->in_hslen,
ssl->in_msglen );
@ -3743,13 +3789,43 @@ int mbedtls_ssl_read_record( mbedtls_ssl_context *ssl )
return( 0 );
}
else
{
ssl->in_msglen = 0;
}
ssl->in_hslen = 0;
}
/* Case (4): Application data */
else if( ssl->in_offt != NULL )
{
return( 0 );
}
/* Everything else (CCS & Alerts) */
else
{
ssl->in_msglen = 0;
}
/*
* Read the record header and parse it
* Step B
*
* Fetch and decode new record if current one is fully consumed.
*
*/
if( ssl->in_msglen > 0 )
{
/* There's something left to be processed in the current record. */
return( 0 );
}
/* Need to fetch a new record */
read_record_header:
/* Current record either fully processed or to be discarded. */
if( ( ret = mbedtls_ssl_fetch_input( ssl, mbedtls_ssl_hdr_len( ssl ) ) ) != 0 )
{
MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_fetch_input", ret );
@ -3841,6 +3917,12 @@ read_record_header:
}
#endif
/* As above, invalid records cause
* dismissal of the whole datagram. */
ssl->next_record_offset = 0;
ssl->in_left = 0;
MBEDTLS_SSL_DEBUG_MSG( 1, ( "discarding invalid record (mac)" ) );
goto read_record_header;
}
@ -6448,7 +6530,7 @@ static int ssl_check_ctr_renegotiate( mbedtls_ssl_context *ssl )
*/
int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len )
{
int ret, record_read = 0;
int ret;
size_t n;
if( ssl == NULL || ssl->conf == NULL )
@ -6471,8 +6553,22 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len )
}
#endif
/*
* Check if renegotiation is necessary and/or handshake is
* in process. If yes, perform/continue, and fall through
* if an unexpected packet is received while the client
* is waiting for the ServerHello.
*
* (There is no equivalent to the last condition on
* the server-side as it is not treated as within
* a handshake while waiting for the ClientHello
* after a renegotiation request.)
*/
#if defined(MBEDTLS_SSL_RENEGOTIATION)
if( ( ret = ssl_check_ctr_renegotiate( ssl ) ) != 0 )
ret = ssl_check_ctr_renegotiate( ssl );
if( ret != MBEDTLS_ERR_SSL_WAITING_SERVER_HELLO_RENEGO &&
ret != 0 )
{
MBEDTLS_SSL_DEBUG_RET( 1, "ssl_check_ctr_renegotiate", ret );
return( ret );
@ -6482,11 +6578,8 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len )
if( ssl->state != MBEDTLS_SSL_HANDSHAKE_OVER )
{
ret = mbedtls_ssl_handshake( ssl );
if( ret == MBEDTLS_ERR_SSL_WAITING_SERVER_HELLO_RENEGO )
{
record_read = 1;
}
else if( ret != 0 )
if( ret != MBEDTLS_ERR_SSL_WAITING_SERVER_HELLO_RENEGO &&
ret != 0 )
{
MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_handshake", ret );
return( ret );
@ -6502,8 +6595,6 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len )
ssl_set_timer( ssl, ssl->conf->read_timeout );
}
if( ! record_read )
{
if( ( ret = mbedtls_ssl_read_record( ssl ) ) != 0 )
{
if( ret == MBEDTLS_ERR_SSL_CONN_EOF )
@ -6512,7 +6603,6 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len )
MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret );
return( ret );
}
}
if( ssl->in_msglen == 0 &&
ssl->in_msgtype == MBEDTLS_SSL_MSG_APPLICATION_DATA )
@ -6549,7 +6639,9 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len )
#endif
return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE );
}
#endif /* MBEDTLS_SSL_CLI_C */
#if defined(MBEDTLS_SSL_SRV_C)
if( ssl->conf->endpoint == MBEDTLS_SSL_IS_SERVER &&
ssl->in_msg[0] != MBEDTLS_SSL_HS_CLIENT_HELLO )
{
@ -6612,25 +6704,18 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len )
}
#endif
ret = ssl_start_renegotiation( ssl );
if( ret == MBEDTLS_ERR_SSL_WAITING_SERVER_HELLO_RENEGO )
{
record_read = 1;
}
else if( ret != 0 )
if( ret != MBEDTLS_ERR_SSL_WAITING_SERVER_HELLO_RENEGO &&
ret != 0 )
{
MBEDTLS_SSL_DEBUG_RET( 1, "ssl_start_renegotiation", ret );
return( ret );
}
}
/* If a non-handshake record was read during renego, fallthrough,
* else tell the user they should call mbedtls_ssl_read() again */
if( ! record_read )
return( MBEDTLS_ERR_SSL_WANT_READ );
}
else if( ssl->renego_status == MBEDTLS_SSL_RENEGOTIATION_PENDING )
{
if( ssl->conf->renego_max_records >= 0 )
{
if( ++ssl->renego_records_seen > ssl->conf->renego_max_records )