lib/mbedtls
1
0
Fork 0
mirror of https://github.com/Mbed-TLS/mbedtls.git synced 2025-07-29 11:41:15 +03:00
Code Activity

Merge remote-tracking branch 'restricted/iotssl-1398' into development-restricted

Browse Source 
* restricted/iotssl-1398:
  Add ChangeLog entry
  Ensure application data records are not kept when fully processed
  Add hard assertion to mbedtls_ssl_read_record_layer
  Fix mbedtls_ssl_read
  Simplify retaining of messages for future processing
This commit is contained in:
Manuel Pégourié-Gonnard
2017-06-09 15:02:40 +02:00
parent a8e5a4730d bf4c2e3f79
commit 4a42f3c405
5 changed files with 326 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files

70
library/ssl_cli.c
View File

@ -1471,6 +1471,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 */
@ -2316,13 +2318,17 @@ static int ssl_parse_server_key_exchange( mbedtls_ssl_context *ssl )
if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_PSK ||
ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_RSA_PSK )
{
ssl->record_read = 1;
/* Current message is probably either
* CertificateRequest or ServerHelloDone */
ssl->keep_current_message = 1;
goto exit;
}
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server key exchange message" ) );
MBEDTLS_SSL_DEBUG_MSG( 1, ( "server key exchange message must "
"not be skipped" ) );
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE );
return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE );
}
@ -2640,38 +2646,32 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl )
return( 0 );
}
if( ssl->record_read == 0 )
if( ( ret = mbedtls_ssl_read_record( ssl ) ) != 0 )
{
if( ( ret = mbedtls_ssl_read_record( ssl ) ) != 0 )
{
MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret );
return( ret );
}
if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) );
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE );
return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE );
}
ssl->record_read = 1;
MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret );
return( ret );
}
ssl->client_auth = 0;
ssl->state++;
if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad certificate request message" ) );
mbedtls_ssl_send_alert_message( ssl, MBEDTLS_SSL_ALERT_LEVEL_FATAL,
MBEDTLS_SSL_ALERT_MSG_UNEXPECTED_MESSAGE );
return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE );
}
if( ssl->in_msg[0] == MBEDTLS_SSL_HS_CERTIFICATE_REQUEST )
ssl->client_auth++;
ssl->state++;
ssl->client_auth = ( ssl->in_msg[0] == MBEDTLS_SSL_HS_CERTIFICATE_REQUEST );
MBEDTLS_SSL_DEBUG_MSG( 3, ( "got %s certificate request",
ssl->client_auth ? "a" : "no" ) );
if( ssl->client_auth == 0 )
{
/* Current message is probably the ServerHelloDone */
ssl->keep_current_message = 1;
goto exit;
ssl->record_read = 0;
}
/*
* struct {
@ -2766,21 +2766,17 @@ static int ssl_parse_server_hello_done( mbedtls_ssl_context *ssl )
MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse server hello done" ) );
if( ssl->record_read == 0 )
if( ( ret = mbedtls_ssl_read_record( ssl ) ) != 0 )
{
if( ( ret = mbedtls_ssl_read_record( ssl ) ) != 0 )
{
MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret );
return( ret );
}
if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello done message" ) );
return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE );
}
MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret );
return( ret );
}
if( ssl->in_msgtype != MBEDTLS_SSL_MSG_HANDSHAKE )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "bad server hello done message" ) );
return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE );
}
ssl->record_read = 0;
if( ssl->in_hslen != mbedtls_ssl_hs_hdr_len( ssl ) ||
ssl->in_msg[0] != MBEDTLS_SSL_HS_SERVER_HELLO_DONE )

265
library/ssl_tls.c
View File

@ -3720,27 +3720,35 @@ int mbedtls_ssl_read_record( mbedtls_ssl_context *ssl )
MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> read record" ) );
do {
if( ssl->keep_current_message == 0 )
{
do {
if( ( ret = mbedtls_ssl_read_record_layer( ssl ) ) != 0 )
if( ( ret = mbedtls_ssl_read_record_layer( ssl ) ) != 0 )
{
MBEDTLS_SSL_DEBUG_RET( 1, ( "mbedtls_ssl_read_record_layer" ), ret );
return( ret );
}
ret = mbedtls_ssl_handle_message_type( ssl );
} while( MBEDTLS_ERR_SSL_NON_FATAL == ret );
if( 0 != ret )
{
MBEDTLS_SSL_DEBUG_RET( 1, ( "mbedtls_ssl_read_record_layer" ), ret );
return( ret );
}
ret = mbedtls_ssl_handle_message_type( ssl );
} while( MBEDTLS_ERR_SSL_NON_FATAL == ret );
if( 0 != ret )
{
MBEDTLS_SSL_DEBUG_RET( 1, ( "mbedtls_ssl_handle_message_type" ), ret );
return( ret );
if( ssl->in_msgtype == MBEDTLS_SSL_MSG_HANDSHAKE )
{
mbedtls_ssl_update_handshake_status( ssl );
}
}
if( ssl->in_msgtype == MBEDTLS_SSL_MSG_HANDSHAKE )
else
{
mbedtls_ssl_update_handshake_status( ssl );
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= reuse previously read message" ) );
ssl->keep_current_message = 0;
}
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= read record" ) );
@ -3752,31 +3760,116 @@ int mbedtls_ssl_read_record_layer( mbedtls_ssl_context *ssl )
{
int ret;
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.
* Internal reference IOTSSL-1321.
*
* (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 )
{
/* Hard assertion to be sure that no application data
* is in flight, as corrupting ssl->in_msglen during
* ssl->in_offt != NULL is fatal. */
if( ssl->in_offt != NULL )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) );
return( MBEDTLS_ERR_SSL_INTERNAL_ERROR );
}
/*
* Get next Handshake message in the current record
*/
ssl->in_msglen -= ssl->in_hslen;
memmove( ssl->in_msg, ssl->in_msg + ssl->in_hslen,
ssl->in_msglen );
/* 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.
* Additionally, ssl->in_hslen might be arbitrarily out of
* bounds after handling a DTLS message with an unexpected
* sequence number, see mbedtls_ssl_prepare_handshake_record.
*/
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 );
MBEDTLS_SSL_DEBUG_BUF( 4, "remaining content in record",
ssl->in_msg, ssl->in_msglen );
MBEDTLS_SSL_DEBUG_BUF( 4, "remaining content in record",
ssl->in_msg, ssl->in_msglen );
}
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;
}
/*
* 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 );
}
ssl->in_hslen = 0;
/* Need to fetch a new record */
/*
* Read the record header and parse it
*/
#if defined(MBEDTLS_SSL_PROTO_DTLS)
read_record_header:
#endif
/* 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 );
@ -3868,6 +3961,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;
}
@ -5603,7 +5702,8 @@ static int ssl_session_reset_int( mbedtls_ssl_context *ssl, int partial )
ssl->in_hslen = 0;
ssl->nb_zero = 0;
ssl->record_read = 0;
ssl->keep_current_message = 0;
ssl->out_msg = ssl->out_buf + 13;
ssl->out_msgtype = 0;
@ -6634,7 +6734,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 )
@ -6657,8 +6757,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 );
@ -6668,17 +6782,49 @@ 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 );
}
}
/*
* TODO
*
* The logic should be streamlined here:
*
* Instead of
*
* - Manually checking whether ssl->in_offt is NULL
* - Fetching a new record if yes
* - Setting ssl->in_offt if one finds an application record
* - Resetting keep_current_message after handling the application data
*
* one should
*
* - Adapt read_record to set ssl->in_offt automatically
* when a new application data record is processed.
* - Always call mbedtls_ssl_read_record here.
*
* This way, the logic of ssl_read would be much clearer:
*
* (1) Always call record layer and see what kind of record is on
* and have it ready for consumption (in particular, in_offt
* properly set for application data records).
* (2) If it's application data (either freshly fetched
* or something already being partially processed),
* serve the read request from it.
* (3) If it's something different from application data,
* handle it accordingly, e.g. potentially start a
* renegotiation.
*
* This will also remove the need to manually reset
* ssl->keep_current_message = 0 below.
*
*/
if( ssl->in_offt == NULL )
{
/* Start timer if not already running */
@ -6688,16 +6834,13 @@ 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_ssl_read_record( ssl ) ) != 0 )
{
if( ret == MBEDTLS_ERR_SSL_CONN_EOF )
return( 0 );
if( ret == MBEDTLS_ERR_SSL_CONN_EOF )
return( 0 );
MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret );
return( ret );
}
MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_read_record", ret );
return( ret );
}
if( ssl->in_msglen == 0 &&
@ -6721,10 +6864,16 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "received handshake message" ) );
/*
* - For client-side, expect SERVER_HELLO_REQUEST.
* - For server-side, expect CLIENT_HELLO.
* - Fail (TLS) or silently drop record (DTLS) in other cases.
*/
#if defined(MBEDTLS_SSL_CLI_C)
if( ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT &&
( ssl->in_msg[0] != MBEDTLS_SSL_HS_HELLO_REQUEST ||
ssl->in_hslen != mbedtls_ssl_hs_hdr_len( ssl ) ) )
ssl->in_hslen != mbedtls_ssl_hs_hdr_len( ssl ) ) )
{
MBEDTLS_SSL_DEBUG_MSG( 1, ( "handshake received (not HelloRequest)" ) );
@ -6735,7 +6884,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 )
{
@ -6748,13 +6899,19 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len )
#endif
return( MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE );
}
#endif
#endif /* MBEDTLS_SSL_SRV_C */
/* Determine whether renegotiation attempt should be accepted */
if( ssl->conf->disable_renegotiation == MBEDTLS_SSL_RENEGOTIATION_DISABLED ||
( ssl->secure_renegotiation == MBEDTLS_SSL_LEGACY_RENEGOTIATION &&
ssl->conf->allow_legacy_renegotiation ==
MBEDTLS_SSL_LEGACY_NO_RENEGOTIATION ) )
{
/*
* Refuse renegotiation
*/
MBEDTLS_SSL_DEBUG_MSG( 3, ( "refusing renegotiation, sending alert" ) );
#if defined(MBEDTLS_SSL_PROTO_SSL3)
@ -6789,6 +6946,10 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len )
}
else
{
/*
* Accept renegotiation request
*/
/* DTLS clients need to know renego is server-initiated */
#if defined(MBEDTLS_SSL_PROTO_DTLS)
if( ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM &&
@ -6798,25 +6959,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 );
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 )
@ -6864,7 +7018,7 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len )
}
}
#endif /* MBEDTLS_SSL_SRV_C && MBEDTLS_SSL_RENEGOTIATION */
#endif
#endif /* MBEDTLS_SSL_PROTO_DTLS */
}
n = ( len < ssl->in_msglen )
@ -6874,11 +7028,16 @@ int mbedtls_ssl_read( mbedtls_ssl_context *ssl, unsigned char *buf, size_t len )
ssl->in_msglen -= n;
if( ssl->in_msglen == 0 )
/* all bytes consumed */
{
/* all bytes consumed */
ssl->in_offt = NULL;
ssl->keep_current_message = 0;
}
else
{
/* more data available */
ssl->in_offt += n;
}
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= read" ) );