Merge pull request #586 from ARMmbed/remove_peer_crt_after_handshake_no_digest-baremetal

[Baremetal] Don't store peer CRT digest if renegotiation is disabled
This commit is contained in:
Manuel Pégourié-Gonnard 2019-06-24 18:12:00 +02:00 committed by GitHub
commit 393338ca78
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 88 additions and 54 deletions

View File

@ -856,13 +856,13 @@ struct mbedtls_ssl_session
#if defined(MBEDTLS_X509_CRT_PARSE_C)
#if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)
mbedtls_x509_crt *peer_cert; /*!< peer X.509 cert chain */
#else /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
#elif defined(MBEDTLS_SSL_RENEGOTIATION)
/*! The digest of the peer's end-CRT. This must be kept to detect CRT
* changes during renegotiation, mitigating the triple handshake attack. */
unsigned char *peer_cert_digest;
size_t peer_cert_digest_len;
mbedtls_md_type_t peer_cert_digest_type;
#endif /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
#endif /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE && MBEDTLS_SSL_RENEGOTIATION */
#endif /* MBEDTLS_X509_CRT_PARSE_C */
uint32_t verify_result; /*!< verification result */

View File

@ -2288,7 +2288,7 @@ static int ssl_write_encrypted_pms( mbedtls_ssl_context *ssl,
int ret;
size_t len_bytes = ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ? 0 : 2;
unsigned char *p = ssl->handshake->premaster + pms_offset;
mbedtls_pk_context * peer_pk;
mbedtls_pk_context *peer_pk = NULL;
if( offset + len_bytes > MBEDTLS_SSL_OUT_CONTENT_LEN )
{
@ -2315,16 +2315,23 @@ static int ssl_write_encrypted_pms( mbedtls_ssl_context *ssl,
ssl->handshake->pmslen = 48;
#if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)
peer_pk = &ssl->handshake->peer_pubkey;
/* Because the peer CRT pubkey is embedded into the handshake
* params currently, and there's no 'is_init' functions for PK
* contexts, we need to break the abstraction and peek into
* the PK context to see if it has been initialized. */
if( ssl->handshake->peer_pubkey.pk_info != NULL )
peer_pk = &ssl->handshake->peer_pubkey;
#else /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
if( ssl->session_negotiate->peer_cert == NULL )
if( ssl->session_negotiate->peer_cert != NULL )
peer_pk = &ssl->session_negotiate->peer_cert->pk;
#endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
if( peer_pk == NULL )
{
/* Should never happen */
MBEDTLS_SSL_DEBUG_MSG( 1, ( "should never happen" ) );
return( MBEDTLS_ERR_SSL_INTERNAL_ERROR );
}
peer_pk = &ssl->session_negotiate->peer_cert->pk;
#endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
/*
* Now write it out, encrypted

View File

@ -4161,7 +4161,7 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl )
mbedtls_md_type_t md_alg;
const mbedtls_ssl_ciphersuite_t *ciphersuite_info =
ssl->handshake->ciphersuite_info;
mbedtls_pk_context * peer_pk;
mbedtls_pk_context *peer_pk = NULL;
MBEDTLS_SSL_DEBUG_MSG( 2, ( "=> parse certificate verify" ) );
@ -4172,21 +4172,30 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl )
return( 0 );
}
#if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)
if( ssl->session_negotiate->peer_cert == NULL )
/* Skip if we haven't received a certificate from the client.
* If MBEDTLS_SSL_KEEP_PEER_CERTIFICATE is set, this can be
* inferred from the setting of mbedtls_ssl_session::peer_cert.
* If MBEDTLS_SSL_KEEP_PEER_CERTIFICATE is not set, it can
* be inferred from whether we've held back the peer CRT's
* public key in mbedtls_ssl_handshake_params::peer_pubkey. */
#if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)
/* Because the peer CRT pubkey is embedded into the handshake
* params currently, and there's no 'is_init' functions for PK
* contexts, we need to break the abstraction and peek into
* the PK context to see if it has been initialized. */
if( ssl->handshake->peer_pubkey.pk_info != NULL )
peer_pk = &ssl->handshake->peer_pubkey;
#else /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
if( ssl->session_negotiate->peer_cert != NULL )
peer_pk = &ssl->session_negotiate->peer_cert->pk;
#endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
if( peer_pk == NULL )
{
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate verify" ) );
ssl->state++;
return( 0 );
}
#else /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
if( ssl->session_negotiate->peer_cert_digest == NULL )
{
MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= skip parse certificate verify" ) );
ssl->state++;
return( 0 );
}
#endif /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
/* Read the message without adding it to the checksum */
ret = mbedtls_ssl_read_record( ssl, 0 /* no checksum update */ );
@ -4208,17 +4217,6 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl )
i = mbedtls_ssl_hs_hdr_len( ssl );
#if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)
peer_pk = &ssl->handshake->peer_pubkey;
#else /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
if( ssl->session_negotiate->peer_cert == NULL )
{
/* Should never happen */
return( MBEDTLS_ERR_SSL_INTERNAL_ERROR );
}
peer_pk = &ssl->session_negotiate->peer_cert->pk;
#endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
/*
* struct {
* SignatureAndHashAlgorithm algorithm; -- TLS 1.2 only

View File

@ -395,7 +395,7 @@ int mbedtls_ssl_session_copy( mbedtls_ssl_session *dst,
return( ret );
}
}
#else /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
#elif defined(MBEDTLS_SSL_RENEGOTIATION)
if( src->peer_cert_digest != NULL )
{
dst->peer_cert_digest =
@ -408,7 +408,7 @@ int mbedtls_ssl_session_copy( mbedtls_ssl_session *dst,
dst->peer_cert_digest_type = src->peer_cert_digest_type;
dst->peer_cert_digest_len = src->peer_cert_digest_len;
}
#endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
#endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE && MBEDTLS_SSL_RENEGOTIATION */
#endif /* MBEDTLS_X509_CRT_PARSE_C */
@ -5999,7 +5999,7 @@ static void ssl_clear_peer_cert( mbedtls_ssl_session *session )
mbedtls_free( session->peer_cert );
session->peer_cert = NULL;
}
#else /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
#elif defined(MBEDTLS_SSL_RENEGOTIATION)
if( session->peer_cert_digest != NULL )
{
/* Zeroization is not necessary. */
@ -6008,7 +6008,9 @@ static void ssl_clear_peer_cert( mbedtls_ssl_session *session )
session->peer_cert_digest_type = MBEDTLS_MD_NONE;
session->peer_cert_digest_len = 0;
}
#endif /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
#else
((void) session);
#endif /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE && MBEDTLS_SSL_RENEGOTIATION */
}
#endif /* MBEDTLS_X509_CRT_PARSE_C */
@ -6184,7 +6186,7 @@ static int ssl_check_peer_crt_unchanged( mbedtls_ssl_context *ssl,
return( memcmp( peer_crt->raw.p, crt_buf, crt_buf_len ) );
}
#else /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
#elif defined(MBEDTLS_SSL_RENEGOTIATION)
static int ssl_check_peer_crt_unchanged( mbedtls_ssl_context *ssl,
unsigned char *crt_buf,
size_t crt_buf_len )
@ -6212,7 +6214,7 @@ static int ssl_check_peer_crt_unchanged( mbedtls_ssl_context *ssl,
return( memcmp( tmp_digest, peer_cert_digest, digest_len ) );
}
#endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
#endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE && MBEDTLS_SSL_RENEGOTIATION */
#endif /* MBEDTLS_SSL_RENEGOTIATION && MBEDTLS_SSL_CLI_C */
/*
@ -6594,6 +6596,8 @@ static int ssl_parse_certificate_verify( mbedtls_ssl_context *ssl,
}
#if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)
#if defined(MBEDTLS_SSL_RENEGOTIATION)
static int ssl_remember_peer_crt_digest( mbedtls_ssl_context *ssl,
unsigned char *start, size_t len )
{
@ -6624,6 +6628,7 @@ static int ssl_remember_peer_crt_digest( mbedtls_ssl_context *ssl,
return( ret );
}
#endif /* MBEDTLS_SSL_RENEGOTIATION */
static int ssl_remember_peer_pubkey( mbedtls_ssl_context *ssl,
unsigned char *start, size_t len )
@ -6738,16 +6743,21 @@ crt_verify:
#if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)
{
unsigned char *crt_start, *pk_start;
size_t crt_len, pk_len;
size_t pk_len;
unsigned char *pk_start;
/* We parse the CRT chain without copying, so
* these pointers point into the input buffer,
* and are hence still valid after freeing the
* CRT chain. */
#if defined(MBEDTLS_SSL_RENEGOTIATION)
unsigned char *crt_start;
size_t crt_len;
crt_start = chain->raw.p;
crt_len = chain->raw.len;
#endif /* MBEDTLS_SSL_RENEGOTIATION */
pk_start = chain->pk_raw.p;
pk_len = chain->pk_raw.len;
@ -6758,9 +6768,11 @@ crt_verify:
mbedtls_free( chain );
chain = NULL;
#if defined(MBEDTLS_SSL_RENEGOTIATION)
ret = ssl_remember_peer_crt_digest( ssl, crt_start, crt_len );
if( ret != 0 )
goto exit;
#endif /* MBEDTLS_SSL_RENEGOTIATION */
ret = ssl_remember_peer_pubkey( ssl, pk_start, pk_len );
if( ret != 0 )
@ -9280,7 +9292,7 @@ int mbedtls_ssl_session_save( const mbedtls_ssl_session *session,
}
}
#else /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
#elif defined(MBEDTLS_SSL_RENEGOTIATION)
/* Digest of peer certificate */
if( session->peer_cert_digest != NULL )
{
@ -9303,7 +9315,7 @@ int mbedtls_ssl_session_save( const mbedtls_ssl_session *session,
*p++ = 0;
}
}
#endif /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
#endif /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE && MBEDTLS_SSL_RENEGOTIATION */
#endif /* MBEDTLS_X509_CRT_PARSE_C */
/*
@ -9448,9 +9460,9 @@ static int ssl_session_load( mbedtls_ssl_session *session,
#if defined(MBEDTLS_X509_CRT_PARSE_C)
#if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)
session->peer_cert = NULL;
#else
#elif defined(MBEDTLS_SSL_RENEGOTIATION)
session->peer_cert_digest = NULL;
#endif /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
#endif /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE && MBEDTLS_SSL_RENEGOTIATION */
#endif
#if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_CLI_C)
session->ticket = NULL;
@ -9496,7 +9508,7 @@ static int ssl_session_load( mbedtls_ssl_session *session,
p += cert_len;
}
#else /* defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
#elif defined(MBEDTLS_SSL_RENEGOTIATION)
/* Deserialize CRT digest from the end of the ticket. */
if( 2 > (size_t)( end - p ) )
return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );
@ -9525,7 +9537,7 @@ static int ssl_session_load( mbedtls_ssl_session *session,
session->peer_cert_digest_len );
p += session->peer_cert_digest_len;
}
#endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
#endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE && MBEDTLS_SSL_RENEGOTIATION */
#endif /* MBEDTLS_X509_CRT_PARSE_C */
/*

View File

@ -901,6 +901,23 @@ component_test_asan_remove_peer_certificate () {
if_build_succeeded tests/compat.sh
}
component_test_asan_remove_peer_certificate_no_renego () {
msg "build: default config with MBEDTLS_SSL_KEEP_PEER_CERTIFICATE and MBEDTLS_SSL_RENEGOTIATION disabled (ASan build)"
scripts/config.pl unset MBEDTLS_SSL_KEEP_PEER_CERTIFICATE
scripts/config.pl unset MBEDTLS_SSL_RENEGOTIATION
CC=gcc cmake -D CMAKE_BUILD_TYPE:String=Asan .
make
msg "test: !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE"
make test
msg "test: ssl-opt.sh, !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE + !MBEDTLS_SSL_RENEGOTIATION"
if_build_succeeded tests/ssl-opt.sh
msg "test: compat.sh, !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE + !MBEDTLS_SSL_RENEGOTIATION"
if_build_succeeded tests/compat.sh
}
component_test_no_max_fragment_length_small_ssl_out_content_len () {
msg "build: no MFL extension, small SSL_OUT_CONTENT_LEN (ASan build)"
scripts/config.pl unset MBEDTLS_SSL_MAX_FRAGMENT_LENGTH

View File

@ -296,7 +296,14 @@ static int ssl_populate_session( mbedtls_ssl_session *session,
if( ret != 0 )
return( ret );
#if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)
#if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)
/* Move temporary CRT. */
session->peer_cert = mbedtls_calloc( 1, sizeof( *session->peer_cert ) );
if( session->peer_cert == NULL )
return( -1 );
*session->peer_cert = tmp_crt;
memset( &tmp_crt, 0, sizeof( tmp_crt ) );
#elif defined(MBEDTLS_SSL_RENEGOTIATION)
/* Calculate digest of temporary CRT. */
session->peer_cert_digest =
mbedtls_calloc( 1, MBEDTLS_SSL_PEER_CERT_DIGEST_DFL_LEN );
@ -312,14 +319,7 @@ static int ssl_populate_session( mbedtls_ssl_session *session,
MBEDTLS_SSL_PEER_CERT_DIGEST_DFL_TYPE;
session->peer_cert_digest_len =
MBEDTLS_SSL_PEER_CERT_DIGEST_DFL_LEN;
#else /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
/* Move temporary CRT. */
session->peer_cert = mbedtls_calloc( 1, sizeof( *session->peer_cert ) );
if( session->peer_cert == NULL )
return( -1 );
*session->peer_cert = tmp_crt;
memset( &tmp_crt, 0, sizeof( tmp_crt ) );
#endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
#endif /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE && MBEDTLS_SSL_RENEGOTIATION */
mbedtls_x509_crt_free( &tmp_crt );
}
@ -718,7 +718,7 @@ void ssl_serialize_session_save_load( int ticket_len, char *crt_file )
restored.peer_cert->raw.p,
original.peer_cert->raw.len ) == 0 );
}
#else /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
#elif defined(MBEDTLS_SSL_RENEGOTIATION)
TEST_ASSERT( original.peer_cert_digest_type ==
restored.peer_cert_digest_type );
TEST_ASSERT( original.peer_cert_digest_len ==
@ -731,7 +731,7 @@ void ssl_serialize_session_save_load( int ticket_len, char *crt_file )
restored.peer_cert_digest,
original.peer_cert_digest_len ) == 0 );
}
#endif /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */
#endif /* !MBEDTLS_SSL_KEEP_PEER_CERTIFICATE && MBEDTLS_SSL_RENEGOTIATION */
#endif /* MBEDTLS_X509_CRT_PARSE_C */
TEST_ASSERT( original.verify_result == restored.verify_result );