Obey bounds of ASN.1 substructures

When parsing a substructure of an ASN.1 structure, no field within
the substructure must exceed the bounds of the substructure.
Concretely, the `end` pointer passed to the ASN.1 parsing routines
must be updated to point to the end of the substructure while parsing
the latter.

This was previously not the case for the routines
- x509_get_attr_type_and_value(),
- mbedtls_x509_get_crt_ext(),
- mbedtls_x509_get_crl_ext().
These functions kept using the end of the parent structure as the
`end` pointer and would hence allow substructure fields to cross
the substructure boundary. This could lead to successful parsing
of ill-formed X.509 CRTs.

This commit fixes this.

Care has to be taken when adapting `mbedtls_x509_get_crt_ext()`
and `mbedtls_x509_get_crl_ext()`, as the underlying function
`mbedtls_x509_get_ext()` returns `0` if no extensions are present
but doesn't set the variable which holds the bounds of the Extensions
structure in case the latter is present. This commit addresses
this by returning early from `mbedtls_x509_get_crt_ext()` and
`mbedtls_x509_get_crl_ext()` if parsing has reached the end of
the input buffer.

The following X.509 parsing tests need to be adapted:
- "TBSCertificate, issuer two inner set datas"
  This test exercises the X.509 CRT parser with a Subject name
  which has two empty `AttributeTypeAndValue` structures.
  This is supposed to fail with `MBEDTLS_ERR_ASN1_OUT_OF_DATA`
  because the parser should attempt to parse the first structure
  and fail because of a lack of data. Previously, it failed to
  obey the (0-length) bounds of the first AttributeTypeAndValue
  structure and would try to interpret the beginning of the second
  AttributeTypeAndValue structure as the first field of the first
  AttributeTypeAndValue structure, returning an UNEXPECTED_TAG error.
- "TBSCertificate, issuer, no full following string"
  This test exercises the parser's behaviour on an AttributeTypeAndValue
  structure which contains more data than expected; it should therefore
  fail with MBEDTLS_ERR_ASN1_LENGTH_MISMATCH. Because of the missing bounds
  check, it previously failed with UNEXPECTED_TAG because it interpreted
  the remaining byte in the first AttributeTypeAndValue structure as the
  first byte in the second AttributeTypeAndValue structure.
- "SubjectAltName repeated"
  This test should exercise two SubjectAltNames extensions in succession,
  but a wrong length values makes the second SubjectAltNames extension appear
  outside of the Extensions structure. With the new bounds in place, this
  therefore fails with a LENGTH_MISMATCH error. This commit adapts the test
  data to put the 2nd SubjectAltNames extension inside the Extensions
  structure, too.
This commit is contained in:
Hanno Becker 2019-02-12 17:22:36 +00:00
parent 024b53a856
commit 12f62fb82c
4 changed files with 20 additions and 3 deletions

View file

@ -361,6 +361,8 @@ static int x509_get_attr_type_value( unsigned char **p,
MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 )
return( MBEDTLS_ERR_X509_INVALID_NAME + ret );
end = *p + len;
if( ( end - *p ) < 1 )
return( MBEDTLS_ERR_X509_INVALID_NAME +
MBEDTLS_ERR_ASN1_OUT_OF_DATA );
@ -394,6 +396,12 @@ static int x509_get_attr_type_value( unsigned char **p,
val->p = *p;
*p += val->len;
if( *p != end )
{
return( MBEDTLS_ERR_X509_INVALID_NAME +
MBEDTLS_ERR_ASN1_LENGTH_MISMATCH );
}
cur->next = NULL;
return( 0 );

View file

@ -103,6 +103,9 @@ static int x509_get_crl_ext( unsigned char **p,
{
int ret;
if( *p == end )
return( 0 );
/*
* crlExtensions [0] EXPLICIT Extensions OPTIONAL
* -- if present, version MUST be v2
@ -115,6 +118,8 @@ static int x509_get_crl_ext( unsigned char **p,
return( ret );
}
end = ext->p + ext->len;
while( *p < end )
{
/*

View file

@ -891,6 +891,9 @@ static int x509_get_crt_ext( unsigned char **p,
size_t len;
unsigned char *end_ext_data, *end_ext_octet;
if( *p == end )
return( 0 );
if( ( ret = mbedtls_x509_get_ext( p, end, &crt->v3_ext, 3 ) ) != 0 )
{
if( ret == MBEDTLS_ERR_ASN1_UNEXPECTED_TAG )
@ -899,6 +902,7 @@ static int x509_get_crt_ext( unsigned char **p,
return( ret );
}
end = crt->v3_ext.p + crt->v3_ext.len;
while( *p < end )
{
/*

View file

@ -1096,7 +1096,7 @@ x509parse_crt:"30223020a0030201028204deadbeef300d06092a864886f70d01010b050030043
X509 Certificate ASN1 (TBSCertificate, issuer two inner set datas)
depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
x509parse_crt:"30243022a0030201028204deadbeef300d06092a864886f70d01010b05003006310430003000":"":MBEDTLS_ERR_X509_INVALID_NAME + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG
x509parse_crt:"30243022a0030201028204deadbeef300d06092a864886f70d01010b05003006310430003000":"":MBEDTLS_ERR_X509_INVALID_NAME + MBEDTLS_ERR_ASN1_OUT_OF_DATA
X509 Certificate ASN1 (TBSCertificate, issuer no oid data)
depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
@ -1112,7 +1112,7 @@ x509parse_crt:"30253023a0030201028204deadbeef300d06092a864886f70d01010b050030073
X509 Certificate ASN1 (TBSCertificate, issuer, no full following string)
depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
x509parse_crt:"302b3029a0030201028204deadbeef300d06092a864886f70d01010b0500300d310b3009060013045465737400":"":MBEDTLS_ERR_X509_INVALID_NAME+MBEDTLS_ERR_ASN1_UNEXPECTED_TAG
x509parse_crt:"302b3029a0030201028204deadbeef300d06092a864886f70d01010b0500300d310b3009060013045465737400":"":MBEDTLS_ERR_X509_INVALID_NAME+MBEDTLS_ERR_ASN1_LENGTH_MISMATCH
X509 Certificate ASN1 (TBSCertificate, valid issuer, no validity)
depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
@ -1264,7 +1264,7 @@ x509parse_crt:"3081de3081dba003020102020900ebdbcd14105e1839300906072a8648ce3d040
X509 Certificate ASN1 (SubjectAltName repeated)
depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_SHA1_C
x509parse_crt:"3081fd3081faa003020102020900a8b31ff37d09a37f300906072a8648ce3d0401300f310d300b0603550403130454657374301e170d3134313131313231333731365a170d3234313130383231333731365a300f310d300b06035504031304546573743059301306072a8648ce3d020106082a8648ce3d0301070342000437cc56d976091e5a723ec7592dff206eee7cf9069174d0ad14b5f768225962924ee500d82311ffea2fd2345d5d16bd8a88c26b770d55cd8a2a0efa01c8b4edffa321301f301d0603551d11041630148208666f6f2e7465737482086261722e74657374301d0603551d11041630148208666f6f2e7465737482086261722e74657374":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS
x509parse_crt:"3081fd3081faa003020102020900a8b31ff37d09a37f300906072a8648ce3d0401300f310d300b0603550403130454657374301e170d3134313131313231333731365a170d3234313130383231333731365a300f310d300b06035504031304546573743059301306072a8648ce3d020106082a8648ce3d0301070342000437cc56d976091e5a723ec7592dff206eee7cf9069174d0ad14b5f768225962924ee500d82311ffea2fd2345d5d16bd8a88c26b770d55cd8a2a0efa01c8b4edffa340303e301d0603551d11041630148208666f6f2e7465737482086261722e74657374301d0603551d11041630148208666f6f2e7465737482086261722e74657374":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS
X509 Certificate ASN1 (ExtKeyUsage repeated)
depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_SHA1_C