yuzu/mbedtls
1
0
Fork 0
mirror of https://github.com/yuzu-emu/mbedtls.git synced 2024-12-23 10:05:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

Remove CRT digest from SSL session if !RENEGO + !KEEP_PEER_CERT

Browse source 
If `MBEDTLS_SSL_KEEP_PEER_CERTIFICATE` is not set, `mbedtls_ssl_session`
contains the digest of the peer's certificate for the sole purpose of
detecting a CRT change on renegotiation. Hence, it is not needed if
renegotiation is disabled.

This commit removes the `peer_cert_digest` fields (and friends) from
`mbedtls_ssl_session` if
   `!MBEDTLS_SSL_KEEP_PEER_CERTIFICATE + !MBEDTLS_SSL_RENEGOTIATION`,
which is a sensible configuration for constrained devices.

Apart from straightforward replacements of
   `if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE)`
by
   `if !defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) && \
        defined(MBEDTLS_SSL_RENEGOTIATION)`,
there's one notable change: On the server-side, the CertificateVerify
parsing function is a no-op if the client hasn't sent a certificate.
So far, this was determined by either looking at the peer CRT or the
peer CRT digest in the SSL session structure (depending on the setting
of `MBEDTLS_SSL_KEEP_PEER_CERTIFICATE`), which now no longer works if
`MBEDTLS_SSL_KEEP_PEER_CERTIFICATE` is unset. Instead, this function
now checks whether the temporary copy of the peer's public key within
the handshake structure is initialized or not (which is also a
beneficial simplification in its own right, because the pubkey is
all the function needs anyway).
This commit is contained in:
Hanno Becker 2019-06-06 16:25:57 +01:00
parent 0528f82fa9
commit 5882dd0856
5 changed files with 71 additions and 54 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
include/mbedtls/ssl.h
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 */

17
library/ssl_cli.c
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

42
library/ssl_srv.c
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

40
library/ssl_tls.c
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 */
@ -5994,7 +5994,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. */
@ -6003,7 +6003,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 */
@ -6179,7 +6181,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 )
@ -6207,7 +6209,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 */
/*
@ -6589,6 +6591,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 )
{
@ -6619,6 +6623,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 )
@ -6733,16 +6738,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;
@ -6753,9 +6763,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 )
@ -9275,7 +9287,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 )
{
@ -9298,7 +9310,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 */
/*
@ -9443,9 +9455,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;
@ -9491,7 +9503,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 );
@ -9520,7 +9532,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 */
/*

22
tests/suites/test_suite_ssl.function
View file

@ -295,7 +295,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 );
@ -311,14 +318,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 );
}
@ -717,7 +717,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 ==
@ -730,7 +730,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 );