diff --git a/ChangeLog b/ChangeLog index 68e7a0b60426..cec77d2afc30 100644 --- a/ChangeLog +++ b/ChangeLog @@ -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 diff --git a/library/x509.c b/library/x509.c index aaf23010505e..a562df7ca3a2 100644 --- a/library/x509.c +++ b/library/x509.c @@ -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 ); @@ -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 ) diff --git a/library/x509_crl.c b/library/x509_crl.c index 8450f87e033d..00f8545d7cd2 100644 --- a/library/x509_crl.c +++ b/library/x509_crl.c @@ -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 ) { diff --git a/library/x509_crt.c b/library/x509_crt.c index ebd118db01fa..97e1d72e3c72 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -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; @@ -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; @@ -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 ) { /* diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 46ca1d2dce8e..b64414a67f86 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -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 @@ -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 @@ -1120,7 +1120,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 @@ -1128,7 +1128,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 @@ -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