Always return a high-level error code from X.509 module

Some functions within the X.509 module return an ASN.1 low level
error code where instead this error code should be wrapped by a
high-level X.509 error code as in the bulk of the module.

Specifically, the following functions are affected:
- mbedtls_x509_get_ext()
- x509_get_version()
- x509_get_uid()

This commit modifies these functions to always return an
X.509 high level error code.

Care has to be taken when adapting `mbetls_x509_get_ext()`:
Currently, the callers `mbedtls_x509_crt_ext()` treat the
return code `MBEDTLS_ERR_ASN1_UNEXPECTED_TAG` specially to
gracefully detect and continue if the extension structure is not
present. Wrapping the ASN.1 error with
`MBEDTLS_ERR_X509_INVALID_EXTENSIONS` and adapting the check
accordingly would mean that an unexpected tag somewhere
down the extension parsing would be ignored by the caller.

The way out of this is the following: Luckily, the extension
structure is always the last field in the surrounding structure,
so if there is some data remaining, it must be an Extension
structure, so we don't need to deal with a tag mismatch gracefully
in the first place.

We may therefore wrap the return code from the initial call to
`mbedtls_asn1_get_tag()` in `mbedtls_x509_get_ext()` by
`MBEDTLS_ERR_X509_INVALID_EXTENSIONS` and simply remove
the special treatment of `MBEDTLS_ERR_ASN1_UNEXPECTED_TAG`
in the callers `x509_crl_get_ext()` and `x509_crt_get_ext()`.

This renders `mbedtls_x509_get_ext()` unsuitable if it ever
happened that an Extension structure is optional and does not
occur at the end of its surrounding structure, but for CRTs
and CRLs, it's fine.

The following tests need to be adapted:
- "TBSCertificate v3, issuerID wrong tag"
  The issuerID is optional, so if we look for its presence
  but find a different tag, we silently continue and try
  parsing the subjectID, and then the extensions. The tag '00'
  used in this test doesn't match either of these, and the
  previous code would hence return LENGTH_MISMATCH after
  unsucessfully trying issuerID, subjectID and Extensions.
  With the new code, any data remaining after issuerID and
  subjectID _must_ be Extension data, so we fail with
  UNEXPECTED_TAG when trying to parse the Extension data.
- "TBSCertificate v3, UIDs, invalid length"
  The test hardcodes the expectation of
  MBEDTLS_ERR_ASN1_INVALID_LENGTH, which needs to be
  wrapped in MBEDTLS_ERR_X509_INVALID_FORMAT now.

Fixes #2431.
This commit is contained in:
Hanno Becker 2019-02-12 11:52:10 +00:00
parent 12f62fb82c
commit 6ccfb18ab1
4 changed files with 12 additions and 25 deletions

View file

@ -708,22 +708,19 @@ int mbedtls_x509_get_sig_alg( const mbedtls_x509_buf *sig_oid, const mbedtls_x50
* be either manually updated or extensions should be parsed!)
*/
int mbedtls_x509_get_ext( unsigned char **p, const unsigned char *end,
mbedtls_x509_buf *ext, int tag )
mbedtls_x509_buf *ext, int tag )
{
int ret;
size_t len;
if( *p == end )
return( 0 );
ret = mbedtls_asn1_get_tag( p, end, &ext->len,
MBEDTLS_ASN1_CONTEXT_SPECIFIC | MBEDTLS_ASN1_CONSTRUCTED | tag );
if( ret != 0 )
return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret );
ext->tag = **p;
if( ( ret = mbedtls_asn1_get_tag( p, end, &ext->len,
MBEDTLS_ASN1_CONTEXT_SPECIFIC | MBEDTLS_ASN1_CONSTRUCTED | tag ) ) != 0 )
return( ret );
ext->p = *p;
end = *p + ext->len;
ext->tag = MBEDTLS_ASN1_CONTEXT_SPECIFIC | MBEDTLS_ASN1_CONSTRUCTED | tag;
ext->p = *p;
end = *p + ext->len;
/*
* Extensions ::= SEQUENCE SIZE (1..MAX) OF Extension

View file

@ -111,12 +111,7 @@ static int x509_get_crl_ext( unsigned char **p,
* -- if present, version MUST be v2
*/
if( ( ret = mbedtls_x509_get_ext( p, end, ext, 0 ) ) != 0 )
{
if( ret == MBEDTLS_ERR_ASN1_UNEXPECTED_TAG )
return( 0 );
return( ret );
}
end = ext->p + ext->len;

View file

@ -402,7 +402,7 @@ static int x509_get_version( unsigned char **p,
return( 0 );
}
return( ret );
return( MBEDTLS_ERR_X509_INVALID_FORMAT + ret );
}
end = *p + len;
@ -469,7 +469,7 @@ static int x509_get_uid( unsigned char **p,
if( ret == MBEDTLS_ERR_ASN1_UNEXPECTED_TAG )
return( 0 );
return( ret );
return( MBEDTLS_ERR_X509_INVALID_FORMAT + ret );
}
uid->p = *p;
@ -895,12 +895,7 @@ static int x509_get_crt_ext( unsigned char **p,
return( 0 );
if( ( ret = mbedtls_x509_get_ext( p, end, &crt->v3_ext, 3 ) ) != 0 )
{
if( ret == MBEDTLS_ERR_ASN1_UNEXPECTED_TAG )
return( 0 );
return( ret );
}
end = crt->v3_ext.p + crt->v3_ext.len;
while( *p < end )

View file

@ -1188,7 +1188,7 @@ x509parse_crt:"308183308180a0030201028204deadbeef300d06092a864886f70d01010b05003
X509 Certificate ASN1 (TBSCertificate v3, issuerID wrong tag)
depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
x509parse_crt:"308184308181a0030201028204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092a864886f70d010101050003190030160210ffffffffffffffffffffffffffffffff0202ffff00":"":MBEDTLS_ERR_X509_INVALID_FORMAT + MBEDTLS_ERR_ASN1_LENGTH_MISMATCH
x509parse_crt:"308184308181a0030201028204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092a864886f70d010101050003190030160210ffffffffffffffffffffffffffffffff0202ffff00":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG
X509 Certificate ASN1 (TBSCertificate v3, UIDs, no ext)
depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
@ -1196,7 +1196,7 @@ x509parse_crt:"308189308186a0030201028204deadbeef300d06092a864886f70d01010b05003
X509 Certificate ASN1 (TBSCertificate v3, UIDs, invalid length)
depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
x509parse_crt:"308189308186a0030201028204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092a864886f70d010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa185aaa201bb":"":MBEDTLS_ERR_ASN1_INVALID_LENGTH
x509parse_crt:"308189308186a0030201028204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092a864886f70d010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa185aaa201bb":"":MBEDTLS_ERR_X509_INVALID_FORMAT + MBEDTLS_ERR_ASN1_INVALID_LENGTH
X509 Certificate ASN1 (TBSCertificate v3, ext empty)
depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C