Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass "certificate policies" extension to callback #3419

Merged
merged 3 commits into from
Jun 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ChangeLog.d/pass-unsupported-policies-to-callback.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Features
* Pass the "certificate policies" extension to the callback supplied to
mbedtls_x509_crt_parse_der_with_ext_cb() if it contains unsupported
policies (#3419).
12 changes: 10 additions & 2 deletions include/mbedtls/x509_crt.h
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,11 @@ int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain,
*
* Callbacks of this type are passed to and used by the
* mbedtls_x509_crt_parse_der_with_ext_cb() routine when
* it encounters an unsupported extension.
* it encounters either an unsupported extension or a
* "certificate policies" extension containing any
* unsupported certificate policies.
ndilieto marked this conversation as resolved.
Show resolved Hide resolved
* Future versions of the library may invoke the callback
* in other cases, if and when the need arises.
*
* \param p_ctx An opaque context passed to the callback.
* \param crt The certificate being parsed.
Expand Down Expand Up @@ -360,14 +364,18 @@ typedef int (*mbedtls_x509_crt_ext_cb_t)( void *p_ctx,
* mbedtls_x509_crt_parse_der(), and/or
* mbedtls_x509_crt_parse_der_nocopy()
* but it calls the callback with every unsupported
* certificate extension.
* certificate extension and additionally the
* "certificate policies" extension if it contains any
* unsupported certificate policies.
* The callback must return a negative error code if it
* does not know how to handle such an extension.
* When the callback fails to parse a critical extension
* mbedtls_x509_crt_parse_der_with_ext_cb() also fails.
* When the callback fails to parse a non critical extension
* mbedtls_x509_crt_parse_der_with_ext_cb() simply skips
* the extension and continues parsing.
* Future versions of the library may invoke the callback
* in other cases, if and when the need arises.
*
* \return \c 0 if successful.
* \return A negative error code on failure.
Expand Down
10 changes: 9 additions & 1 deletion library/x509_crt.c
Original file line number Diff line number Diff line change
Expand Up @@ -894,7 +894,7 @@ static int x509_get_crt_ext( unsigned char **p,
{
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
size_t len;
unsigned char *end_ext_data, *end_ext_octet;
unsigned char *end_ext_data, *start_ext_octet, *end_ext_octet;

if( *p == end )
return( 0 );
Expand Down Expand Up @@ -940,6 +940,7 @@ static int x509_get_crt_ext( unsigned char **p,
MBEDTLS_ASN1_OCTET_STRING ) ) != 0 )
return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret );

start_ext_octet = *p;
end_ext_octet = *p + len;

if( end_ext_octet != end_ext_data )
Expand Down Expand Up @@ -1025,6 +1026,13 @@ static int x509_get_crt_ext( unsigned char **p,
if( ( ret = x509_get_certificate_policies( p, end_ext_octet,
&crt->certificate_policies ) ) != 0 )
{
/* Give the callback (if any) a chance to handle the extension
* if it contains unsupported policies */
if( ret == MBEDTLS_ERR_X509_FEATURE_UNAVAILABLE && cb != NULL &&
cb( p_ctx, crt, &extn_oid, is_critical,
start_ext_octet, end_ext_octet ) == 0 )
break;

#if !defined(MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION)
if( is_critical )
return( ret );
Expand Down
16 changes: 16 additions & 0 deletions tests/suites/test_suite_x509parse.data
Original file line number Diff line number Diff line change
Expand Up @@ -2032,6 +2032,22 @@ X509 CRT ASN1 (Unsupported non critical extension not recognized by callback)
depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
x509parse_crt_cb:"308203353082021da00302010202104d3ebbb8a870f9c78c55a8a7e12fd516300d06092a864886f70d01010b05003010310e300c06035504030c0564756d6d79301e170d3230303432383137343234335a170d3230303632373137343234335a3010310e300c06035504030c0564756d6d7930820122300d06092a864886f70d01010105000382010f003082010a0282010100a51b75b3f7da2d60ea1b0fc077f0dbb2bbb6fe1b474028368af8dc2664672896efff171033b0aede0b323a89d5c6db4d517404bc97b65264e41b9e9e86a6f40ace652498d4b3b859544d1bacfd7f86325503eed046f517406545c0ffb5560f83446dedce0fcafcc41ac8495488a6aa912ae45192ef7e3efa20d0f7403b0baa62c7e2e5404c620c5793623132aa20f624f08d88fbf0985af39433f5a24d0b908e5219d8ba6a404d3ee8418203b62a40c8eb18837354d50281a6a2bf5012e505c419482787b7a81e5935613ceea0c6d93e86f76282b6aa406fb3a1796c56b32e8a22afc3f7a3c9daa8f0e2846ff0d50abfc862a52f6cf0aaece6066c860376f3ed0203010001a3818a308187300c0603551d13040530030101ff30130603551d110101ff04093007820564756d6d79301206082b0601050507011e0101000403040100300e0603551d0f0101ff040403020184301d0603551d0e04160414e6e451ec8d19d9677b2d272a9d73b939fa2d915a301f0603551d23041830168014e6e451ec8d19d9677b2d272a9d73b939fa2d915a300d06092a864886f70d01010b0500038201010056d06047b7f48683e2347ca726997d9700b4f2cf1d8bc0ef17addac8445d38ffd7f8079055ead878b6a74c8384d0e30150c8990aa74f59cda6ebcb49465d8991ffa16a4c927a26e4639d1875a3ac396c7455c7eda40dbe66054a03d27f961c15e86bd5b06db6b26572977bcda93453b6b6a88ef96b31996a7bd17323525b33050d28deec9c33a3f9765a11fb99d0e222bd39a6db3a788474c9ca347377688f837d42f5841667bffcbe6b473e6f229f286a0829963e591a99aa7f67e9d20c36ccd2ac84cb85b7a8b3396a6cbe59a573ffff726f373197c230de5c92a52c5bc87e29c20bdf6e89609764a60c649022aabd768f3557661b083ae00e6afc8a5bf2ed":"cert. version \: 3\nserial number \: 4D\:3E\:BB\:B8\:A8\:70\:F9\:C7\:8C\:55\:A8\:A7\:E1\:2F\:D5\:16\nissuer name \: CN=dummy\nsubject name \: CN=dummy\nissued on \: 2020-04-28 17\:42\:43\nexpires on \: 2020-06-27 17\:42\:43\nsigned using \: RSA with SHA-256\nRSA key size \: 2048 bits\nbasic constraints \: CA=true\nsubject alt name \:\n dNSName \: dummy\nkey usage \: Digital Signature, Key Cert Sign\n":0

X509 CRT ASN1 (Unsupported critical policy recognized by callback)
depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
x509parse_crt_cb:"3081b130819ba0030201028204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092a864886f70d010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa100a200a315301330110603551d20010101040730053003060101300d06092a864886f70d01010b0500030200ff":"cert. version \: 3\nserial number \: DE\:AD\:BE\:EF\nissuer name \: ??=Test\nsubject name \: ??=Test\nissued on \: 2009-01-01 00\:00\:00\nexpires on \: 2009-12-31 23\:59\:59\nsigned using \: RSA with SHA-256\nRSA key size \: 128 bits\ncertificate policies \: ???\n":0

X509 CRT ASN1 (Unsupported critical policy not recognized by callback)
depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
x509parse_crt_cb:"3081b130819ba0030201028204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092a864886f70d010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa100a200a315301330110603551d20010101040730053003060100300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_X509_FEATURE_UNAVAILABLE

X509 CRT ASN1 (Unsupported non critical policy recognized by callback)
depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
x509parse_crt_cb:"3081b130819ba0030201028204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092a864886f70d010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa100a200a315301330110603551d20010100040730053003060101300d06092a864886f70d01010b0500030200ff":"cert. version \: 3\nserial number \: DE\:AD\:BE\:EF\nissuer name \: ??=Test\nsubject name \: ??=Test\nissued on \: 2009-01-01 00\:00\:00\nexpires on \: 2009-12-31 23\:59\:59\nsigned using \: RSA with SHA-256\nRSA key size \: 128 bits\ncertificate policies \: ???\n":0

X509 CRT ASN1 (Unsupported non critical policy not recognized by callback)
depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
x509parse_crt_cb:"3081b130819ba0030201028204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092a864886f70d010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa100a200a315301330110603551d20010100040730053003060100300d06092a864886f70d01010b0500030200ff":"cert. version \: 3\nserial number \: DE\:AD\:BE\:EF\nissuer name \: ??=Test\nsubject name \: ??=Test\nissued on \: 2009-01-01 00\:00\:00\nexpires on \: 2009-12-31 23\:59\:59\nsigned using \: RSA with SHA-256\nRSA key size \: 128 bits\ncertificate policies \: ???\n":0

X509 CRL ASN1 (Incorrect first tag)
x509parse_crl:"":"":MBEDTLS_ERR_X509_INVALID_FORMAT

Expand Down
87 changes: 81 additions & 6 deletions tests/suites/test_suite_x509parse.function
Original file line number Diff line number Diff line change
Expand Up @@ -303,17 +303,92 @@ int verify_parse_san( mbedtls_x509_subject_alternative_name *san,
}

int parse_crt_ext_cb( void *p_ctx, mbedtls_x509_crt const *crt, mbedtls_x509_buf const *oid,
int critical, const unsigned char *p, const unsigned char *end )
int critical, const unsigned char *cp, const unsigned char *end )
{
( void ) crt;
( void ) p;
( void ) end;
( void ) critical;
mbedtls_x509_buf *new_oid = (mbedtls_x509_buf *)p_ctx;
if( new_oid == NULL || new_oid->tag != oid->tag || new_oid->len != oid->len ||
memcmp(new_oid->p, oid->p, oid->len) != 0 )
if( oid->tag == MBEDTLS_ASN1_OID &&
MBEDTLS_OID_CMP( MBEDTLS_OID_CERTIFICATE_POLICIES, oid ) == 0 )
{
/* Handle unknown certificate policy */
int ret, parse_ret = 0;
size_t len;
unsigned char **p = (unsigned char **)&cp;

/* Get main sequence tag */
ret = mbedtls_asn1_get_tag( p, end, &len,
MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE );
if( ret != 0 )
return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret );

if( *p + len != end )
return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS +
MBEDTLS_ERR_ASN1_LENGTH_MISMATCH );

/*
* Cannot be an empty sequence.
*/
if( len == 0 )
return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS +
MBEDTLS_ERR_ASN1_LENGTH_MISMATCH );

while( *p < end )
{
const unsigned char *policy_end;

/*
* Get the policy sequence
*/
if( ( ret = mbedtls_asn1_get_tag( p, end, &len,
MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 )
return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret );

policy_end = *p + len;

if( ( ret = mbedtls_asn1_get_tag( p, policy_end, &len,
MBEDTLS_ASN1_OID ) ) != 0 )
return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret );

/*
* Recognize exclusively the policy with OID 1
*/
if( len != 1 || *p[0] != 1 )
ndilieto marked this conversation as resolved.
Show resolved Hide resolved
parse_ret = MBEDTLS_ERR_X509_FEATURE_UNAVAILABLE;

*p += len;

/*
* If there is an optional qualifier, then *p < policy_end
* Check the Qualifier len to verify it doesn't exceed policy_end.
*/
if( *p < policy_end )
{
if( ( ret = mbedtls_asn1_get_tag( p, policy_end, &len,
MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 )
return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret );
/*
* Skip the optional policy qualifiers.
*/
*p += len;
}

if( *p != policy_end )
return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS +
MBEDTLS_ERR_ASN1_LENGTH_MISMATCH );
}

if( *p != end )
return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS +
MBEDTLS_ERR_ASN1_LENGTH_MISMATCH );

return( parse_ret );
}
else if( new_oid != NULL && new_oid->tag == oid->tag && new_oid->len == oid->len &&
memcmp( new_oid->p, oid->p, oid->len ) == 0 )
return( 0 );
else
return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG );
return( 0 );
}
#endif /* MBEDTLS_X509_CRT_PARSE_C */
/* END_HEADER */
Expand Down