Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/pr/2484' into mbedtls-2.16
Browse files Browse the repository at this point in the history 
* origin/pr/2484:
  Correct placement of ChangeLog entry
  Improve documentation of mbedtls_x509_get_ext()
  Adapt ChangeLog
  Always return a high-level error code from X.509 module
  Obey bounds of ASN.1 substructures
  • Loading branch information
@Patater
Patater committed Jun 14, 2019
2 parents 8f27b44 + 5549f46 commit 418e761
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 33 deletions.
5 changes: 5 additions & 0 deletions ChangeLog
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ Bugfix
* Add DER-encoded test CRTs to library/certs.c, allowing
the example programs ssl_server2 and ssl_client2 to be run
if MBEDTLS_FS_IO and MBEDTLS_PEM_PARSE_C are unset. Fixes #2254.
* Fix missing bounds checks in X.509 parsing functions that could
lead to successful parsing of ill-formed X.509 CRTs. Fixes #2437.
* Fix multiple X.509 functions previously returning ASN.1 low-level error
codes to always wrap these codes into X.509 high level error codes before
returning. Fixes #2431.

Changes
* Return from various debugging routines immediately if the
Expand Down
35 changes: 19 additions & 16 deletions library/x509.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down Expand Up @@ -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 );
Expand Down Expand Up @@ -700,30 +708,25 @@ 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 );

ext->tag = **p;

if( ( ret = mbedtls_asn1_get_tag( p, end, &ext->len,
MBEDTLS_ASN1_CONTEXT_SPECIFIC | MBEDTLS_ASN1_CONSTRUCTED | tag ) ) != 0 )
return( ret );
/* Extension structure use EXPLICIT tagging. That is, the actual
* `Extensions` structure is wrapped by a tag-length pair using
* the respective context-specific tag. */
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->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
*
* Extension ::= SEQUENCE {
* extnID OBJECT IDENTIFIER,
* critical BOOLEAN DEFAULT FALSE,
* extnValue OCTET STRING }
*/
if( ( ret = mbedtls_asn1_get_tag( p, end, &len,
MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 )
Expand Down
10 changes: 5 additions & 5 deletions library/x509_crl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,17 +103,17 @@ 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
*/
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;

while( *p < end )
{
Expand Down
13 changes: 6 additions & 7 deletions library/x509_crt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ static int x509_get_version( unsigned char **p,
return( 0 );
}

return( ret );
return( MBEDTLS_ERR_X509_INVALID_FORMAT + ret );
}

end = *p + len;
Expand Down Expand Up @@ -460,7 +460,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;
Expand Down Expand Up @@ -699,14 +699,13 @@ static int x509_get_crt_ext( unsigned char **p,
size_t len;
unsigned char *end_ext_data, *end_ext_octet;

if( ( ret = mbedtls_x509_get_ext( p, end, &crt->v3_ext, 3 ) ) != 0 )
{
if( ret == MBEDTLS_ERR_ASN1_UNEXPECTED_TAG )
return( 0 );
if( *p == end )
return( 0 );

if( ( ret = mbedtls_x509_get_ext( p, end, &crt->v3_ext, 3 ) ) != 0 )
return( ret );
}

end = crt->v3_ext.p + crt->v3_ext.len;
while( *p < end )
{
/*
Expand Down
10 changes: 5 additions & 5 deletions tests/suites/test_suite_x509parse.data
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1028,7 +1028,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
Expand All @@ -1044,7 +1044,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
Expand Down Expand Up @@ -1120,15 +1120,15 @@ 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
x509parse_crt:"308189308186a0030201028204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092a864886f70d010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa101aaa201bb":"":MBEDTLS_ERR_X509_INVALID_ALG + MBEDTLS_ERR_ASN1_OUT_OF_DATA

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
Expand Down Expand Up @@ -1176,7 +1176,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
Expand Down

0 comments on commit 418e761

Please sign in to comment.