yuzu-emu/mbedtls
1
0
Fork 0
mirror of https://github.com/yuzu-emu/mbedtls.git synced 2024-11-26 03:05:44 +01:00
Code Issues Packages Projects Releases Wiki Activity

Add redundancy (Hamming distance) to cert flags

Browse Source 
Before this commit, if a certificate only had one issue (for example, if the
"untrusted" bit was the only set in flags), an attacker that could flip this
single bit between the moment it's set and the moment flags are checked before
returning from mbedtls_x509_crt_verify() could make the entire verification
routine appear to succeed (return 0 with no bit set in flags).

Avoid that by making sure that flags always has either 0 or at least 9 bits
set during the execution of the function. However, to preserve the API, clear
the 8 extra bits before returning. This doesn't open the door to other
attacks, as fortunately the API already had redundancy: either both flags and
the return value are 0, or flags has bits set and the return value is non-zero
with at least 16 bits set (assuming 32-bit 2-complement ints).
This commit is contained in:
Manuel Pégourié-Gonnard 2019-11-12 10:31:12 +01:00
parent 81c1fc4132
commit ea7eab1fde
2 changed files with 36 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
include/mbedtls/x509.h
View File

@ -85,6 +85,8 @@
* \{
*/
/* Reminder: update x509_crt_verify_strings[] in library/x509_crt.c */
/* Reminder: update X509_BADCERT_FI_EXTRA in library/x509_crt.c if using more
* that 24 bits */
#define MBEDTLS_X509_BADCERT_EXPIRED 0x01 /**< The certificate validity has expired. */
#define MBEDTLS_X509_BADCERT_REVOKED 0x02 /**< The certificate has been revoked (is on a CRL). */
#define MBEDTLS_X509_BADCERT_CN_MISMATCH 0x04 /**< The certificate Common Name (CN) does not match with the expected CN. */

45
library/x509_crt.c
View File

@ -3328,6 +3328,23 @@ static unsigned x509_crt_verify_chain_len(
#endif /* MBEDTLS_X509_REMOVE_VERIFY_CALLBACK */
/*
* This is used in addition to the flag for a specific issue, to ensure that
* it is not possible for an active physical attacker to entirely clear the
* flags just by flipping a single bit. Take advantage of the fact that all
* values defined in include/mbedtls/x509.h so far are 24-bit or less, so the
* top byte is free.
*
* Currently this protection is not compatible with the vrfy callback (as it
* can observ and modify flags freely), so it's only enabled when the callback
* is disabled.
*/
#if defined(MBEDTLS_X509_REMOVE_VERIFY_CALLBACK)
#define X509_BADCERT_FI_EXTRA 0xff000000u
#else
#define X509_BADCERT_FI_EXTRA 0u
#endif
/*
* Build and verify a certificate chain
*
@ -3434,9 +3451,9 @@ find_parent:
#if !defined(MBEDTLS_X509_CRT_REMOVE_TIME)
/* Check time-validity (all certificates) */
if( mbedtls_x509_time_is_past( &child->valid_to ) )
*flags |= MBEDTLS_X509_BADCERT_EXPIRED;
*flags |= MBEDTLS_X509_BADCERT_EXPIRED | X509_BADCERT_FI_EXTRA;
if( mbedtls_x509_time_is_future( &child->valid_from ) )
*flags |= MBEDTLS_X509_BADCERT_FUTURE;
*flags |= MBEDTLS_X509_BADCERT_FUTURE | X509_BADCERT_FI_EXTRA;
#endif /* !MBEDTLS_X509_CRT_REMOVE_TIME */
/* Stop here for trusted roots (but not for trusted EE certs) */
@ -3456,10 +3473,10 @@ find_parent:
/* Check signature algorithm: MD & PK algs */
if( x509_profile_check_md_alg( profile, child->sig_md ) != 0 )
*flags |= MBEDTLS_X509_BADCERT_BAD_MD;
*flags |= MBEDTLS_X509_BADCERT_BAD_MD | X509_BADCERT_FI_EXTRA;
if( x509_profile_check_pk_alg( profile, child->sig_pk ) != 0 )
*flags |= MBEDTLS_X509_BADCERT_BAD_PK;
*flags |= MBEDTLS_X509_BADCERT_BAD_PK | X509_BADCERT_FI_EXTRA;
/* Special case: EE certs that are locally trusted */
if( x509_crt_verify_chain_len( ver_chain ) == 1 && self_issued &&
@ -3507,7 +3524,7 @@ find_parent:
/* No parent? We're done here */
if( parent_crt == NULL )
{
*flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED;
*flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED | X509_BADCERT_FI_EXTRA;
return( 0 );
}
@ -3531,11 +3548,11 @@ find_parent:
signature_is_good_fi = signature_is_good;
if( signature_is_good_fi != X509_SIGNATURE_IS_GOOD )
{
*flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED;
*flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED | X509_BADCERT_FI_EXTRA;
mbedtls_platform_enforce_volatile_reads();
if( signature_is_good_fi != X509_SIGNATURE_IS_GOOD )
*flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED;
*flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED | X509_BADCERT_FI_EXTRA;
}
{
@ -3546,7 +3563,7 @@ find_parent:
/* check size of signing key */
if( x509_profile_check_key( profile, parent_pk ) != 0 )
*flags |= MBEDTLS_X509_BADCERT_BAD_KEY;
*flags |= MBEDTLS_X509_BADCERT_BAD_KEY | X509_BADCERT_FI_EXTRA;
mbedtls_x509_crt_pk_release( parent_crt );
}
@ -3677,7 +3694,7 @@ static int x509_crt_verify_name( const mbedtls_x509_crt *crt,
if( ret != 0 )
ret = MBEDTLS_ERR_X509_FATAL_ERROR;
*flags |= MBEDTLS_X509_BADCERT_CN_MISMATCH;
*flags |= MBEDTLS_X509_BADCERT_CN_MISMATCH | X509_BADCERT_FI_EXTRA;
return( ret );
}
#endif /* !MBEDTLS_X509_REMOVE_HOSTNAME_VERIFICATION */
@ -3799,10 +3816,10 @@ int mbedtls_x509_crt_verify_restartable( mbedtls_x509_crt *crt,
pk_type = mbedtls_pk_get_type( pk );
if( x509_profile_check_pk_alg( profile, pk_type ) != 0 )
ee_flags |= MBEDTLS_X509_BADCERT_BAD_PK;
ee_flags |= MBEDTLS_X509_BADCERT_BAD_PK | X509_BADCERT_FI_EXTRA;
if( x509_profile_check_key( profile, pk ) != 0 )
ee_flags |= MBEDTLS_X509_BADCERT_BAD_KEY;
ee_flags |= MBEDTLS_X509_BADCERT_BAD_KEY | X509_BADCERT_FI_EXTRA;
mbedtls_x509_crt_pk_release( crt );
}
@ -3843,7 +3860,13 @@ exit:
}
if( *flags != 0 )
{
/* Preserve the API by removing internal extra bits - from now on the
* fact that flags is non-zero is also redundantly encoded by the
* return value from this function. */
*flags &= ~ X509_BADCERT_FI_EXTRA;
return( MBEDTLS_ERR_X509_CERT_VERIFY_FAILED );
}
return( 0 );
}