Fix bug in skip_date() (MBEDTLS_X509_CRT_REMOVE_TIME)

Asserting `*p == end` right after setting `end = *p + len` will always fail
unless `len == 0`, which is never the case with properly-formed certificates.

The function x509_skip_dates() is modelled after x509_get_dates() which between
setting `end` and comparing it to `*p` calls mbedtls_x509_get_time() which
advances `*p` to the expected value, which is why this test works in
get_dates().

Since `skip_dates()` has `skip`, not `validate` in its name, and the entire
point of `MBEDTLS_X509_CRT_REMOVE_TIME` is to save code, we don't want to
call the relatively large functions needed to properly parse (and validate)
dates before throwing the parsed dates away, we can just fast-forward to the
end of the sequence.

This makes updating `end` and comparing it to `*p` after the fast-forward
redundant, as the comparison will always be true (unlike the case where we
actually parse the contents of the sequence).

This bug was found by `all.sh test_baremetal` - no need for a new test.
This commit is contained in:
Manuel Pégourié-Gonnard 2019-07-30 14:11:25 +02:00
parent 62daad3b9a
commit 0d1db20490

View file

@ -743,11 +743,8 @@ static int x509_skip_dates( unsigned char **p,
MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 )
return( MBEDTLS_ERR_X509_INVALID_DATE + ret );
end = *p + len;
if( *p != end )
return( MBEDTLS_ERR_X509_INVALID_DATE +
MBEDTLS_ERR_ASN1_LENGTH_MISMATCH );
/* skip contents of the sequence */
*p += len;
return( 0 );
}