AuroraMiddleware/mbedtls
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Merge pull request #939 from ronald-cron-arm/tls13-add-missing-overread-check

Browse Source 
TLS 1.3: Add missing overread check
This commit is contained in:
Paul Elliott 2022-06-29 17:01:14 +01:00 committed by GitHub
commit f6a56cf5ff
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 306 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
ChangeLog.d/tls13-add-missing-overread-check.txt Normal file
View File

@ -0,0 +1,8 @@
Security
* Fix a buffer overread in TLS 1.3 Certificate parsing. An unauthenticated
client or server could cause an MbedTLS server or client to overread up
to 64 kBytes of data and potentially overread the input buffer by that
amount minus the size of the input buffer. As overread data undergoes
various checks, the likelihood of reaching the boundary of the input
buffer is rather small but increases as its size
MBEDTLS_SSL_IN_CONTENT_LEN decreases.

25
library/ssl_misc.h
View File

@ -381,11 +381,36 @@ static inline size_t mbedtls_ssl_get_input_buflen( const mbedtls_ssl_context *ct
* \return Zero if the needed space is available in the buffer, non-zero
* otherwise.
*/
#if ! defined(MBEDTLS_TEST_HOOKS)
static inline int mbedtls_ssl_chk_buf_ptr( const uint8_t *cur,
const uint8_t *end, size_t need )
{
return( ( cur > end ) || ( need > (size_t)( end - cur ) ) );
}
#else
typedef struct
{
const uint8_t *cur;
const uint8_t *end;
size_t need;
} mbedtls_ssl_chk_buf_ptr_args;
void mbedtls_ssl_set_chk_buf_ptr_fail_args(
const uint8_t *cur, const uint8_t *end, size_t need );
void mbedtls_ssl_reset_chk_buf_ptr_fail_args( void );
int mbedtls_ssl_cmp_chk_buf_ptr_fail_args( mbedtls_ssl_chk_buf_ptr_args *args );
static inline int mbedtls_ssl_chk_buf_ptr( const uint8_t *cur,
const uint8_t *end, size_t need )
{
if( ( cur > end ) || ( need > (size_t)( end - cur ) ) )
{
mbedtls_ssl_set_chk_buf_ptr_fail_args( cur, end, need );
return( 1 );
}
return( 0 );
}
#endif /* MBEDTLS_TEST_HOOKS */
/**
* \brief This macro checks if the remaining size in a buffer is

26
library/ssl_tls.c
View File

@ -58,6 +58,30 @@
#include "mbedtls/oid.h"
#endif
#if defined(MBEDTLS_TEST_HOOKS)
static mbedtls_ssl_chk_buf_ptr_args chk_buf_ptr_fail_args;
void mbedtls_ssl_set_chk_buf_ptr_fail_args(
const uint8_t *cur, const uint8_t *end, size_t need )
{
chk_buf_ptr_fail_args.cur = cur;
chk_buf_ptr_fail_args.end = end;
chk_buf_ptr_fail_args.need = need;
}
void mbedtls_ssl_reset_chk_buf_ptr_fail_args( void )
{
memset( &chk_buf_ptr_fail_args, 0, sizeof( chk_buf_ptr_fail_args ) );
}
int mbedtls_ssl_cmp_chk_buf_ptr_fail_args( mbedtls_ssl_chk_buf_ptr_args *args )
{
return( ( chk_buf_ptr_fail_args.cur != args->cur ) ||
( chk_buf_ptr_fail_args.end != args->end ) ||
( chk_buf_ptr_fail_args.need != args->need ) );
}
#endif /* MBEDTLS_TEST_HOOKS */
#if defined(MBEDTLS_SSL_PROTO_DTLS)
#if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID)
@ -1103,6 +1127,8 @@ void mbedtls_ssl_session_reset_msg_layer( mbedtls_ssl_context *ssl,
memset( ssl->in_buf, 0, in_buf_len );
}
ssl->send_alert = 0;
/* Reset outgoing message writing */
ssl->out_msgtype = 0;
ssl->out_msglen = 0;

23
library/ssl_tls13_generic.c
View File

@ -31,6 +31,7 @@
#include <string.h>
#include "ssl_misc.h"
#include "ssl_tls13_invasive.h"
#include "ssl_tls13_keys.h"
#include "ssl_debug_helpers.h"
@ -391,9 +392,10 @@ cleanup:
/* Parse certificate chain send by the server. */
MBEDTLS_CHECK_RETURN_CRITICAL
static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl,
const unsigned char *buf,
const unsigned char *end )
MBEDTLS_STATIC_TESTABLE
int mbedtls_ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl,
const unsigned char *buf,
const unsigned char *end )
{
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
size_t certificate_request_context_len = 0;
@ -444,6 +446,7 @@ static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl,
mbedtls_x509_crt_init( ssl->session_negotiate->peer_cert );
MBEDTLS_SSL_CHK_BUF_READ_PTR( p, end, certificate_list_len );
certificate_list_end = p + certificate_list_len;
while( p < certificate_list_end )
{
@ -524,9 +527,10 @@ exit:
}
#else
MBEDTLS_CHECK_RETURN_CRITICAL
static int ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl,
const unsigned char *buf,
const unsigned char *end )
MBEDTLS_STATIC_TESTABLE
int mbedtls_ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl,
const unsigned char *buf,
const unsigned char *end )
{
((void) ssl);
((void) buf);
@ -657,7 +661,8 @@ static int ssl_tls13_validate_certificate( mbedtls_ssl_context *ssl )
* with details encoded in the verification flags. All other kinds
* of error codes, including those from the user provided f_vrfy
* functions, are treated as fatal and lead to a failure of
* ssl_tls13_parse_certificate even if verification was optional. */
* mbedtls_ssl_tls13_parse_certificate even if verification was optional.
*/
if( authmode == MBEDTLS_SSL_VERIFY_OPTIONAL &&
( ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED ||
ret == MBEDTLS_ERR_SSL_BAD_CERTIFICATE ) )
@ -735,8 +740,8 @@ int mbedtls_ssl_tls13_process_certificate( mbedtls_ssl_context *ssl )
&buf, &buf_len ) );
/* Parse the certificate chain sent by the peer. */
MBEDTLS_SSL_PROC_CHK( ssl_tls13_parse_certificate( ssl, buf,
buf + buf_len ) );
MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_parse_certificate( ssl, buf,
buf + buf_len ) );
/* Validate the certificate chain and set the verification results. */
MBEDTLS_SSL_PROC_CHK( ssl_tls13_validate_certificate( ssl ) );

4
library/ssl_tls13_invasive.h
View File

@ -80,6 +80,10 @@ psa_status_t mbedtls_psa_hkdf_expand( psa_algorithm_t hash_alg,
const unsigned char *info, size_t info_len,
unsigned char *okm, size_t okm_len );
MBEDTLS_CHECK_RETURN_CRITICAL
int mbedtls_ssl_tls13_parse_certificate( mbedtls_ssl_context *ssl,
const unsigned char *buf,
const unsigned char *end );
#endif /* MBEDTLS_TEST_HOOKS */
#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */

12
tests/scripts/all.sh
View File

@ -2904,6 +2904,18 @@ component_test_tls13_only () {
if_build_succeeded tests/ssl-opt.sh
}
component_test_tls13_only_with_hooks () {
msg "build: default config with MBEDTLS_SSL_PROTO_TLS1_3 and MBEDTLS_TEST_HOOKS, without MBEDTLS_SSL_PROTO_TLS1_2"
scripts/config.py set MBEDTLS_TEST_HOOKS
make CFLAGS="'-DMBEDTLS_USER_CONFIG_FILE=\"../tests/configs/tls13-only.h\"'"
msg "test: default config with MBEDTLS_SSL_PROTO_TLS1_3 enabled, without MBEDTLS_SSL_PROTO_TLS1_2"
if_build_succeeded make test
msg "ssl-opt.sh (TLS 1.3)"
if_build_succeeded tests/ssl-opt.sh
}
component_test_tls13 () {
msg "build: default config with MBEDTLS_SSL_PROTO_TLS1_3 enabled, without padding"
scripts/config.py set MBEDTLS_SSL_PROTO_TLS1_3

3
tests/suites/test_suite_ssl.data
View File

@ -3381,3 +3381,6 @@ cookie_parsing:"16fefd0000000000000000002f010000de000000000000011efefd7b72727272
Cookie parsing: one byte overread
cookie_parsing:"16fefd0000000000000000002F010000de000000000000011efefd7b7272727272727272727272727272727272727272727272727272727272727d0001":MBEDTLS_ERR_SSL_DECODE_ERROR
TLS 1.3 srv Certificate msg - wrong vector lengths
tls13_server_certificate_msg_invalid_vector_len

214
tests/suites/test_suite_ssl.function
View File

@ -105,6 +105,23 @@ void init_handshake_options( handshake_test_options *opts )
opts->cli_log_fun = NULL;
opts->resize_buffers = 1;
}
#if defined(MBEDTLS_TEST_HOOKS)
static void set_chk_buf_ptr_args(
mbedtls_ssl_chk_buf_ptr_args *args,
unsigned char *cur, unsigned char *end, size_t need )
{
args->cur = cur;
args->end = end;
args->need = need;
}
static void reset_chk_buf_ptr_args( mbedtls_ssl_chk_buf_ptr_args *args )
{
memset( args, 0, sizeof( *args ) );
}
#endif /* MBEDTLS_TEST_HOOKS */
/*
* Buffer structure for custom I/O callbacks.
*/
@ -2307,6 +2324,119 @@ exit:
}
#endif /* MBEDTLS_X509_CRT_PARSE_C && MBEDTLS_ENTROPY_C && MBEDTLS_CTR_DRBG_C */
#if defined(MBEDTLS_TEST_HOOKS)
/*
* Tweak vector lengths in a TLS 1.3 Certificate message
*
* \param[in] buf Buffer containing the Certificate message to tweak
* \param[in]]out] end End of the buffer to parse
* \param tweak Tweak identifier (from 1 to the number of tweaks).
* \param[out] expected_result Error code expected from the parsing function
* \param[out] args Arguments of the MBEDTLS_SSL_CHK_BUF_READ_PTR call that
* is expected to fail. All zeroes if no
* MBEDTLS_SSL_CHK_BUF_READ_PTR failure is expected.
*/
int tweak_tls13_certificate_msg_vector_len(
unsigned char *buf, unsigned char **end, int tweak,
int *expected_result, mbedtls_ssl_chk_buf_ptr_args *args )
{
/*
* The definition of the tweaks assume that the certificate list contains only
* one certificate.
*/
/*
* struct {
* opaque cert_data<1..2^24-1>;
* Extension extensions<0..2^16-1>;
* } CertificateEntry;
*
* struct {
* opaque certificate_request_context<0..2^8-1>;
* CertificateEntry certificate_list<0..2^24-1>;
* } Certificate;
*/
unsigned char *p_certificate_request_context_len = buf;
size_t certificate_request_context_len = buf[0];
unsigned char *p_certificate_list_len = buf + 1 + certificate_request_context_len;
unsigned char *certificate_list = p_certificate_list_len + 3;
size_t certificate_list_len = MBEDTLS_GET_UINT24_BE( p_certificate_list_len, 0 );
unsigned char *p_cert_data_len = certificate_list;
unsigned char *cert_data = p_cert_data_len + 3;
size_t cert_data_len = MBEDTLS_GET_UINT24_BE( p_cert_data_len, 0 );
unsigned char *p_extensions_len = cert_data + cert_data_len;
unsigned char *extensions = p_extensions_len + 2;
size_t extensions_len = MBEDTLS_GET_UINT16_BE( p_extensions_len, 0 );
*expected_result = MBEDTLS_ERR_SSL_DECODE_ERROR;
switch( tweak )
{
case 1:
/* Failure when checking if the certificate request context length and
* certificate list length can be read
*/
*end = buf + 3;
set_chk_buf_ptr_args( args, buf, *end, 4 );
break;
case 2:
/* Invalid certificate request context length.
*/
*p_certificate_request_context_len =
certificate_request_context_len + 1;
reset_chk_buf_ptr_args( args );
break;
case 3:
/* Failure when checking if certificate_list data can be read. */
MBEDTLS_PUT_UINT24_BE( certificate_list_len + 1,
p_certificate_list_len, 0 );
set_chk_buf_ptr_args( args, certificate_list, *end,
certificate_list_len + 1 );
break;
case 4:
/* Failure when checking if the cert_data length can be read. */
MBEDTLS_PUT_UINT24_BE( 2, p_certificate_list_len, 0 );
set_chk_buf_ptr_args( args, p_cert_data_len, certificate_list + 2, 3 );
break;
case 5:
/* Failure when checking if cert_data data can be read. */
MBEDTLS_PUT_UINT24_BE( certificate_list_len - 3 + 1,
p_cert_data_len, 0 );
set_chk_buf_ptr_args( args, cert_data,
certificate_list + certificate_list_len,
certificate_list_len - 3 + 1 );
break;
case 6:
/* Failure when checking if the extensions length can be read. */
MBEDTLS_PUT_UINT24_BE( certificate_list_len - extensions_len - 1,
p_certificate_list_len, 0 );
set_chk_buf_ptr_args( args, p_extensions_len,
certificate_list + certificate_list_len - extensions_len - 1, 2 );
break;
case 7:
/* Failure when checking if extensions data can be read. */
MBEDTLS_PUT_UINT16_BE( extensions_len + 1, p_extensions_len, 0 );
set_chk_buf_ptr_args( args, extensions,
certificate_list + certificate_list_len, extensions_len + 1 );
break;
default:
return( -1 );
}
return( 0 );
}
#endif /* MBEDTLS_TEST_HOOKS */
/* END_HEADER */
/* BEGIN_DEPENDENCIES
@ -5708,3 +5838,87 @@ exit:
USE_PSA_DONE( );
}
/* END_CASE */
/* BEGIN_CASE depends_on:MBEDTLS_TEST_HOOKS:MBEDTLS_SSL_PROTO_TLS1_3:!MBEDTLS_SSL_PROTO_TLS1_2:MBEDTLS_SSL_CLI_C:MBEDTLS_SSL_SRV_C:MBEDTLS_X509_CRT_PARSE_C:MBEDTLS_ECP_DP_SECP384R1_ENABLED */
void tls13_server_certificate_msg_invalid_vector_len( )
{
int ret = -1;
mbedtls_endpoint client_ep, server_ep;
unsigned char *buf, *end;
size_t buf_len;
int step = 0;
int expected_result;
mbedtls_ssl_chk_buf_ptr_args expected_chk_buf_ptr_args;
/*
* Test set-up
*/
USE_PSA_INIT( );
ret = mbedtls_endpoint_init( &client_ep, MBEDTLS_SSL_IS_CLIENT,
MBEDTLS_PK_ECDSA, NULL, NULL, NULL, NULL );
TEST_EQUAL( ret, 0 );
ret = mbedtls_endpoint_init( &server_ep, MBEDTLS_SSL_IS_SERVER,
MBEDTLS_PK_ECDSA, NULL, NULL, NULL, NULL );
TEST_EQUAL( ret, 0 );
ret = mbedtls_mock_socket_connect( &(client_ep.socket),
&(server_ep.socket), 1024 );
TEST_EQUAL( ret, 0 );
while( 1 )
{
mbedtls_test_set_step( ++step );
ret = mbedtls_move_handshake_to_state( &(server_ep.ssl),
&(client_ep.ssl),
MBEDTLS_SSL_CERTIFICATE_VERIFY );
TEST_EQUAL( ret, 0 );
ret = mbedtls_ssl_flush_output( &(server_ep.ssl) );
TEST_EQUAL( ret, 0 );
ret = mbedtls_move_handshake_to_state( &(client_ep.ssl),
&(server_ep.ssl),
MBEDTLS_SSL_SERVER_CERTIFICATE );
TEST_EQUAL( ret, 0 );
ret = mbedtls_ssl_tls13_fetch_handshake_msg( &(client_ep.ssl),
MBEDTLS_SSL_HS_CERTIFICATE,
&buf, &buf_len );
TEST_EQUAL( ret, 0 );
end = buf + buf_len;
/*
* Tweak server Certificate message and parse it.
*/
ret = tweak_tls13_certificate_msg_vector_len(
buf, &end, step, &expected_result, &expected_chk_buf_ptr_args );
if( ret != 0 )
break;
ret = mbedtls_ssl_tls13_parse_certificate( &(client_ep.ssl), buf, end );
TEST_EQUAL( ret, expected_result );
TEST_ASSERT( mbedtls_ssl_cmp_chk_buf_ptr_fail_args(
&expected_chk_buf_ptr_args ) == 0 );
mbedtls_ssl_reset_chk_buf_ptr_fail_args( );
ret = mbedtls_ssl_session_reset( &(client_ep.ssl) );
TEST_EQUAL( ret, 0 );
ret = mbedtls_ssl_session_reset( &(server_ep.ssl) );
TEST_EQUAL( ret, 0 );
}
exit:
mbedtls_ssl_reset_chk_buf_ptr_fail_args( );
mbedtls_endpoint_free( &client_ep, NULL );
mbedtls_endpoint_free( &server_ep, NULL );
USE_PSA_DONE( );
}
/* END_CASE */