From 502d4b45102a0665aa0b769c41ad1c7bdb96478e Mon Sep 17 00:00:00 2001 From: Nicola Di Lieto Date: Sat, 25 Apr 2020 14:41:25 +0200 Subject: [PATCH 01/12] New mbedtls_x509_crt_parse_der_ext() routine This routine is functionally equivalent to mbedtls_x509_crt_parse_der(), but it accepts an additional callback function which it calls with every unsupported certificate extension. Proposed solution to https://github.com/ARMmbed/mbedtls/issues/3241 Signed-off-by: Nicola Di Lieto --- include/mbedtls/x509_crt.h | 56 ++++++++++++++++++++++++++++++++++++++ library/x509_crt.c | 30 +++++++++++++++----- 2 files changed, 79 insertions(+), 7 deletions(-) diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index e4fb13543..19de1e968 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -303,6 +303,62 @@ int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, const unsigned char *buf, size_t buflen ); +/** + * \brief The type of certificate extension callbacks. + * + * Callbacks of this type are passed to and used by the + * mbedtls_x509_crt_parse_der_ext() routine when it encounters + * an unsupported extension. + * + * \param crt Pointer to the certificate being parsed + * \param oid Extension's OID + * \param critical If the extension is critical (per the RFC's definition) + * \param p On entry \c *p points to the start of the extension ASN.1 + * data. On successful completion \c *p must point to the + * first byte after it. + * On error, the value of \c *p is undefined. + * \param end End of extension data. + * + * \note The callback must fail and return a negative error code if + * it can not parse or does not support the extension. + * + * \return \c 0 on success. + * \return A negative error code on failure. + */ +typedef int (*mbedtls_x509_crt_ext_cb_t)( mbedtls_x509_crt const *crt, + mbedtls_x509_buf const *oid, + int critical, + unsigned char **p, + const unsigned char *end ); + +/** + * \brief Parse a single DER formatted certificate and add it + * to the end of the provided chained list. + * + * \param chain The pointer to the start of the CRT chain to attach to. + * When parsing the first CRT in a chain, this should point + * to an instance of ::mbedtls_x509_crt initialized through + * mbedtls_x509_crt_init(). + * \param buf The buffer holding the DER encoded certificate. + * \param buflen The size in Bytes of \p buf. + * \param cb A callback invoked for every unsupported certificate + * extension. + * + * \note This call is functionally equivalent to + * mbedtls_x509_crt_parse_der(), but it calls the callback + * with every unsupported certificate extension. + * The callback must return a negative error code if it + * does not know how to handle such an extension. + * + * \return \c 0 if successful. + * \return A negative error code on failure. + */ +int mbedtls_x509_crt_parse_der_ext( mbedtls_x509_crt *chain, + const unsigned char *buf, + size_t buflen, + mbedtls_x509_crt_ext_cb_t cb + ); + /** * \brief Parse a single DER formatted certificate and add it * to the end of the provided chained list. This is a diff --git a/library/x509_crt.c b/library/x509_crt.c index 1e62ed5b0..9076b321b 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -892,7 +892,8 @@ static int x509_get_certificate_policies( unsigned char **p, */ static int x509_get_crt_ext( unsigned char **p, const unsigned char *end, - mbedtls_x509_crt *crt ) + mbedtls_x509_crt *crt, + mbedtls_x509_crt_ext_cb_t cb ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; size_t len; @@ -955,6 +956,10 @@ static int x509_get_crt_ext( unsigned char **p, if( ret != 0 ) { + /* Give the callback (if any) a chance to handle the extension */ + if (cb && cb(crt, &extn_oid, is_critical, p, end_ext_octet) == 0) + continue; + /* No parser found, skip extension */ *p = end_ext_octet; @@ -1061,7 +1066,8 @@ static int x509_get_crt_ext( unsigned char **p, static int x509_crt_parse_der_core( mbedtls_x509_crt *crt, const unsigned char *buf, size_t buflen, - int make_copy ) + int make_copy, + mbedtls_x509_crt_ext_cb_t cb ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; size_t len; @@ -1260,7 +1266,7 @@ static int x509_crt_parse_der_core( mbedtls_x509_crt *crt, if( crt->version == 3 ) #endif { - ret = x509_get_crt_ext( &p, end, crt ); + ret = x509_get_crt_ext( &p, end, crt, cb ); if( ret != 0 ) { mbedtls_x509_crt_free( crt ); @@ -1323,7 +1329,8 @@ static int x509_crt_parse_der_core( mbedtls_x509_crt *crt, static int mbedtls_x509_crt_parse_der_internal( mbedtls_x509_crt *chain, const unsigned char *buf, size_t buflen, - int make_copy ) + int make_copy, + mbedtls_x509_crt_ext_cb_t cb ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; mbedtls_x509_crt *crt = chain, *prev = NULL; @@ -1355,7 +1362,8 @@ static int mbedtls_x509_crt_parse_der_internal( mbedtls_x509_crt *chain, crt = crt->next; } - if( ( ret = x509_crt_parse_der_core( crt, buf, buflen, make_copy ) ) != 0 ) + ret = x509_crt_parse_der_core( crt, buf, buflen, make_copy, cb ); + if( ret != 0 ) { if( prev ) prev->next = NULL; @@ -1373,14 +1381,22 @@ int mbedtls_x509_crt_parse_der_nocopy( mbedtls_x509_crt *chain, const unsigned char *buf, size_t buflen ) { - return( mbedtls_x509_crt_parse_der_internal( chain, buf, buflen, 0 ) ); + return( mbedtls_x509_crt_parse_der_internal( chain, buf, buflen, 0, NULL ) ); +} + +int mbedtls_x509_crt_parse_der_ext( mbedtls_x509_crt *chain, + const unsigned char *buf, + size_t buflen, + mbedtls_x509_crt_ext_cb_t cb ) +{ + return( mbedtls_x509_crt_parse_der_internal( chain, buf, buflen, 1, cb ) ); } int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, const unsigned char *buf, size_t buflen ) { - return( mbedtls_x509_crt_parse_der_internal( chain, buf, buflen, 1 ) ); + return( mbedtls_x509_crt_parse_der_internal( chain, buf, buflen, 1, NULL ) ); } /* From 6e24980cc6c3bc024ddc114921e593495165a9c4 Mon Sep 17 00:00:00 2001 From: ndilieto <49833066+ndilieto@users.noreply.github.com> Date: Thu, 28 May 2020 06:17:38 +0000 Subject: [PATCH 02/12] Minor style and documentation improvements Co-authored-by: Gilles Peskine Signed-off-by: Nicola Di Lieto --- include/mbedtls/x509_crt.h | 32 ++++++++++++++++---------------- library/x509_crt.c | 2 +- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index 19de1e968..13687b5f0 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -310,14 +310,15 @@ int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, * mbedtls_x509_crt_parse_der_ext() routine when it encounters * an unsupported extension. * - * \param crt Pointer to the certificate being parsed - * \param oid Extension's OID - * \param critical If the extension is critical (per the RFC's definition) - * \param p On entry \c *p points to the start of the extension ASN.1 - * data. On successful completion \c *p must point to the - * first byte after it. - * On error, the value of \c *p is undefined. - * \param end End of extension data. + * \param crt The certificate being parsed. + * \param oid The OID of the extension. + * \param critical Whether the extension is critical. + * \param p On entry, \c *p points to the start of the extension value + * (the content of the OCTET STRING). + * On successful completion, \c *p must point to the + * first byte after the extension value. + * On error, the value of \c *p is not undefined. + * \param end End of extension value. * * \note The callback must fail and return a negative error code if * it can not parse or does not support the extension. @@ -326,10 +327,10 @@ int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, * \return A negative error code on failure. */ typedef int (*mbedtls_x509_crt_ext_cb_t)( mbedtls_x509_crt const *crt, - mbedtls_x509_buf const *oid, - int critical, - unsigned char **p, - const unsigned char *end ); + mbedtls_x509_buf const *oid, + int critical, + unsigned char **p, + const unsigned char *end ); /** * \brief Parse a single DER formatted certificate and add it @@ -354,10 +355,9 @@ typedef int (*mbedtls_x509_crt_ext_cb_t)( mbedtls_x509_crt const *crt, * \return A negative error code on failure. */ int mbedtls_x509_crt_parse_der_ext( mbedtls_x509_crt *chain, - const unsigned char *buf, - size_t buflen, - mbedtls_x509_crt_ext_cb_t cb - ); + const unsigned char *buf, + size_t buflen, + mbedtls_x509_crt_ext_cb_t cb ); /** * \brief Parse a single DER formatted certificate and add it diff --git a/library/x509_crt.c b/library/x509_crt.c index 9076b321b..a0d35ae3a 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -957,7 +957,7 @@ static int x509_get_crt_ext( unsigned char **p, if( ret != 0 ) { /* Give the callback (if any) a chance to handle the extension */ - if (cb && cb(crt, &extn_oid, is_critical, p, end_ext_octet) == 0) + if( cb != NULL && cb( crt, &extn_oid, is_critical, p, end_ext_octet ) == 0 ) continue; /* No parser found, skip extension */ From fde98f7773b15175d8f88fe28ac5b3e42f386077 Mon Sep 17 00:00:00 2001 From: Nicola Di Lieto Date: Thu, 28 May 2020 08:34:33 +0200 Subject: [PATCH 03/12] Rename mbedtls_x509_crt_parse_der_ext new name: mbedtls_x509_crt_parse_der_with_ext_cb Co-authored-by: Gilles Peskine Signed-off-by: Nicola Di Lieto --- include/mbedtls/x509_crt.h | 12 ++++++------ library/x509_crt.c | 8 ++++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index 13687b5f0..96129be36 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -307,8 +307,8 @@ int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, * \brief The type of certificate extension callbacks. * * Callbacks of this type are passed to and used by the - * mbedtls_x509_crt_parse_der_ext() routine when it encounters - * an unsupported extension. + * mbedtls_x509_crt_parse_der_with_ext_cb() routine when + * it encounters an unsupported extension. * * \param crt The certificate being parsed. * \param oid The OID of the extension. @@ -354,10 +354,10 @@ typedef int (*mbedtls_x509_crt_ext_cb_t)( mbedtls_x509_crt const *crt, * \return \c 0 if successful. * \return A negative error code on failure. */ -int mbedtls_x509_crt_parse_der_ext( mbedtls_x509_crt *chain, - const unsigned char *buf, - size_t buflen, - mbedtls_x509_crt_ext_cb_t cb ); +int mbedtls_x509_crt_parse_der_with_ext_cb( mbedtls_x509_crt *chain, + const unsigned char *buf, + size_t buflen, + mbedtls_x509_crt_ext_cb_t cb ); /** * \brief Parse a single DER formatted certificate and add it diff --git a/library/x509_crt.c b/library/x509_crt.c index a0d35ae3a..bf06872d4 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -1384,10 +1384,10 @@ int mbedtls_x509_crt_parse_der_nocopy( mbedtls_x509_crt *chain, return( mbedtls_x509_crt_parse_der_internal( chain, buf, buflen, 0, NULL ) ); } -int mbedtls_x509_crt_parse_der_ext( mbedtls_x509_crt *chain, - const unsigned char *buf, - size_t buflen, - mbedtls_x509_crt_ext_cb_t cb ) +int mbedtls_x509_crt_parse_der_with_ext_cb( mbedtls_x509_crt *chain, + const unsigned char *buf, + size_t buflen, + mbedtls_x509_crt_ext_cb_t cb ) { return( mbedtls_x509_crt_parse_der_internal( chain, buf, buflen, 1, cb ) ); } From fae25a13d931fa6c5c6afac59787ce4ba39b71d9 Mon Sep 17 00:00:00 2001 From: Nicola Di Lieto Date: Thu, 28 May 2020 08:55:08 +0200 Subject: [PATCH 04/12] mbedtls_x509_crt_ext_cb_t definition changed As suggested in https://github.com/ARMmbed/mbedtls/pull/3243#discussion_r431238005 Co-authored-by: Gilles Peskine Signed-off-by: Nicola Di Lieto --- include/mbedtls/x509_crt.h | 7 ++----- library/x509_crt.c | 7 ++++++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index 96129be36..28dfa515c 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -313,11 +313,8 @@ int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, * \param crt The certificate being parsed. * \param oid The OID of the extension. * \param critical Whether the extension is critical. - * \param p On entry, \c *p points to the start of the extension value + * \param p Pointer to the start of the extension value * (the content of the OCTET STRING). - * On successful completion, \c *p must point to the - * first byte after the extension value. - * On error, the value of \c *p is not undefined. * \param end End of extension value. * * \note The callback must fail and return a negative error code if @@ -329,7 +326,7 @@ int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, typedef int (*mbedtls_x509_crt_ext_cb_t)( mbedtls_x509_crt const *crt, mbedtls_x509_buf const *oid, int critical, - unsigned char **p, + const unsigned char *p, const unsigned char *end ); /** diff --git a/library/x509_crt.c b/library/x509_crt.c index bf06872d4..6fdee955b 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -957,8 +957,13 @@ static int x509_get_crt_ext( unsigned char **p, if( ret != 0 ) { /* Give the callback (if any) a chance to handle the extension */ - if( cb != NULL && cb( crt, &extn_oid, is_critical, p, end_ext_octet ) == 0 ) + if( cb != NULL ) { + ret = cb( crt, &extn_oid, is_critical, *p, end_ext_octet ); + if ( ret != 0 ) + return ( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret ); + *p = end_ext_octet; continue; + } /* No parser found, skip extension */ *p = end_ext_octet; From 4dbe5676af58ce7203b75a6d471d6c01fc5b3061 Mon Sep 17 00:00:00 2001 From: Nicola Di Lieto Date: Thu, 28 May 2020 09:18:42 +0200 Subject: [PATCH 05/12] mbedtls_x509_crt_parse_der_with_ext_cb enhancement added make_copy parameter as suggested in https://github.com/ARMmbed/mbedtls/pull/3243#discussion_r431233555 Co-authored-by: Gilles Peskine Signed-off-by: Nicola Di Lieto --- include/mbedtls/x509_crt.h | 43 +++++++++++++++++++++++--------------- library/x509_crt.c | 3 ++- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index 28dfa515c..fb91af289 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -330,30 +330,39 @@ typedef int (*mbedtls_x509_crt_ext_cb_t)( mbedtls_x509_crt const *crt, const unsigned char *end ); /** - * \brief Parse a single DER formatted certificate and add it - * to the end of the provided chained list. + * \brief Parse a single DER formatted certificate and add it + * to the end of the provided chained list. * - * \param chain The pointer to the start of the CRT chain to attach to. - * When parsing the first CRT in a chain, this should point - * to an instance of ::mbedtls_x509_crt initialized through - * mbedtls_x509_crt_init(). - * \param buf The buffer holding the DER encoded certificate. - * \param buflen The size in Bytes of \p buf. - * \param cb A callback invoked for every unsupported certificate - * extension. + * \param chain The pointer to the start of the CRT chain to attach to. + * When parsing the first CRT in a chain, this should point + * to an instance of ::mbedtls_x509_crt initialized through + * mbedtls_x509_crt_init(). + * \param buf The buffer holding the DER encoded certificate. + * \param buflen The size in Bytes of \p buf. + * \param make_copy When not zero this function makes an internal copy of the + * CRT buffer \p buf. In particular, \p buf may be destroyed + * or reused after this call returns. + * When zero this function avoids duplicating the CRT buffer + * by taking temporary ownership thereof until the CRT + * is destroyed (like mbedtls_x509_crt_parse_der_nocopy()) + * \param cb A callback invoked for every unsupported certificate + * extension. * - * \note This call is functionally equivalent to - * mbedtls_x509_crt_parse_der(), but it calls the callback - * with every unsupported certificate extension. - * The callback must return a negative error code if it - * does not know how to handle such an extension. + * \note This call is functionally equivalent to + * mbedtls_x509_crt_parse_der(), and/or + * mbedtls_x509_crt_parse_der_nocopy() + * but it calls the callback with every unsupported + * certificate extension. + * The callback must return a negative error code if it + * does not know how to handle such an extension. * - * \return \c 0 if successful. - * \return A negative error code on failure. + * \return \c 0 if successful. + * \return A negative error code on failure. */ int mbedtls_x509_crt_parse_der_with_ext_cb( mbedtls_x509_crt *chain, const unsigned char *buf, size_t buflen, + int no_copy, mbedtls_x509_crt_ext_cb_t cb ); /** diff --git a/library/x509_crt.c b/library/x509_crt.c index 6fdee955b..2e2fb24d5 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -1392,9 +1392,10 @@ int mbedtls_x509_crt_parse_der_nocopy( mbedtls_x509_crt *chain, int mbedtls_x509_crt_parse_der_with_ext_cb( mbedtls_x509_crt *chain, const unsigned char *buf, size_t buflen, + int make_copy, mbedtls_x509_crt_ext_cb_t cb ) { - return( mbedtls_x509_crt_parse_der_internal( chain, buf, buflen, 1, cb ) ); + return( mbedtls_x509_crt_parse_der_internal( chain, buf, buflen, make_copy, cb ) ); } int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, From 2c3a91739365305299f49da201552d35095a860f Mon Sep 17 00:00:00 2001 From: Nicola Di Lieto Date: Thu, 28 May 2020 17:20:42 +0200 Subject: [PATCH 06/12] Minor style improvement Co-authored-by: Hanno Becker Signed-off-by: Nicola Di Lieto --- library/x509_crt.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index 2e2fb24d5..554352291 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -957,10 +957,11 @@ static int x509_get_crt_ext( unsigned char **p, if( ret != 0 ) { /* Give the callback (if any) a chance to handle the extension */ - if( cb != NULL ) { + if( cb != NULL ) + { ret = cb( crt, &extn_oid, is_critical, *p, end_ext_octet ); - if ( ret != 0 ) - return ( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret ); + if( ret != 0 ) + return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret ); *p = end_ext_octet; continue; } From 5f6ebdebdbc2b86272e5a79ad485cfae00a7fc71 Mon Sep 17 00:00:00 2001 From: Nicola Di Lieto Date: Thu, 28 May 2020 19:00:47 +0200 Subject: [PATCH 07/12] Fix wrong parameter name in comment Detected by Travis https://travis-ci.org/github/ARMmbed/mbedtls/jobs/692213150 /home/travis/build/ARMmbed/mbedtls/include/mbedtls/x509_crt.h:333: warning: argument 'make_copy' of command @param is not found in the argument list of mbedtls_x509_crt_parse_der_with_ext_cb(mbedtls_x509_crt *chain, const unsigned char *buf, size_t buflen, int no_copy, mbedtls_x509_crt_ext_cb_t cb) Signed-off-by: Nicola Di Lieto --- include/mbedtls/x509_crt.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index fb91af289..cdcc65157 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -362,7 +362,7 @@ typedef int (*mbedtls_x509_crt_ext_cb_t)( mbedtls_x509_crt const *crt, int mbedtls_x509_crt_parse_der_with_ext_cb( mbedtls_x509_crt *chain, const unsigned char *buf, size_t buflen, - int no_copy, + int make_copy, mbedtls_x509_crt_ext_cb_t cb ); /** From 17bb60c0f1e5647c9fca8663e0f0a260f04d2164 Mon Sep 17 00:00:00 2001 From: Nicola Di Lieto Date: Thu, 28 May 2020 23:04:15 +0200 Subject: [PATCH 08/12] Tests for mbedtls_x509_crt_parse_der_with_ext_cb Signed-off-by: Nicola Di Lieto --- tests/suites/test_suite_x509parse.data | 8 +++ tests/suites/test_suite_x509parse.function | 82 ++++++++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 7012e8e36..f5345e293 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -1988,6 +1988,14 @@ X509 CRT ASN1 (RSA signature, EC key) depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED:MBEDTLS_SHA1_C:MBEDTLS_RSA_C x509parse_crt:"3081e430819f020104300d06092a864886f70d0101050500300f310d300b0603550403130454657374301e170d3133303731303135303233375a170d3233303730383135303233375a300f310d300b06035504031304546573743049301306072a8648ce3d020106082a8648ce3d03010103320004e962551a325b21b50cf6b990e33d4318fd16677130726357a196e3efe7107bcb6bdc6d9db2a4df7c964acfe81798433d300d06092a864886f70d01010505000331001a6c18cd1e457474b2d3912743f44b571341a7859a0122774a8e19a671680878936949f904c9255bdd6fffdb33a7e6d8":"cert. version \: 1\nserial number \: 04\nissuer name \: CN=Test\nsubject name \: CN=Test\nissued on \: 2013-07-10 15\:02\:37\nexpires on \: 2023-07-08 15\:02\:37\nsigned using \: RSA with SHA1\nEC key size \: 192 bits\n":0 +X509 CRT ASN1 (Unsupported critical extension) +depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C:!MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION +x509parse_crt:"308203353082021da00302010202104d3ebbb8a870f9c78c55a8a7e12fd516300d06092a864886f70d01010b05003010310e300c06035504030c0564756d6d79301e170d3230303432383137343234335a170d3230303632373137343234335a3010310e300c06035504030c0564756d6d7930820122300d06092a864886f70d01010105000382010f003082010a0282010100a51b75b3f7da2d60ea1b0fc077f0dbb2bbb6fe1b474028368af8dc2664672896efff171033b0aede0b323a89d5c6db4d517404bc97b65264e41b9e9e86a6f40ace652498d4b3b859544d1bacfd7f86325503eed046f517406545c0ffb5560f83446dedce0fcafcc41ac8495488a6aa912ae45192ef7e3efa20d0f7403b0baa62c7e2e5404c620c5793623132aa20f624f08d88fbf0985af39433f5a24d0b908e5219d8ba6a404d3ee8418203b62a40c8eb18837354d50281a6a2bf5012e505c419482787b7a81e5935613ceea0c6d93e86f76282b6aa406fb3a1796c56b32e8a22afc3f7a3c9daa8f0e2846ff0d50abfc862a52f6cf0aaece6066c860376f3ed0203010001a3818a308187300c0603551d13040530030101ff30130603551d110101ff04093007820564756d6d79301206082b0601050507011f0101ff0403040100300e0603551d0f0101ff040403020184301d0603551d0e04160414e6e451ec8d19d9677b2d272a9d73b939fa2d915a301f0603551d23041830168014e6e451ec8d19d9677b2d272a9d73b939fa2d915a300d06092a864886f70d01010b0500038201010056d06047b7f48683e2347ca726997d9700b4f2cf1d8bc0ef17addac8445d38ffd7f8079055ead878b6a74c8384d0e30150c8990aa74f59cda6ebcb49465d8991ffa16a4c927a26e4639d1875a3ac396c7455c7eda40dbe66054a03d27f961c15e86bd5b06db6b26572977bcda93453b6b6a88ef96b31996a7bd17323525b33050d28deec9c33a3f9765a11fb99d0e222bd39a6db3a788474c9ca347377688f837d42f5841667bffcbe6b473e6f229f286a0829963e591a99aa7f67e9d20c36ccd2ac84cb85b7a8b3396a6cbe59a573ffff726f373197c230de5c92a52c5bc87e29c20bdf6e89609764a60c649022aabd768f3557661b083ae00e6afc8a5bf2ed":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG + +X509 CRT ASN1 (Unsupported critical extension with callback) +depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C +x509parse_crt_cb:"308203353082021da00302010202104d3ebbb8a870f9c78c55a8a7e12fd516300d06092a864886f70d01010b05003010310e300c06035504030c0564756d6d79301e170d3230303432383137343234335a170d3230303632373137343234335a3010310e300c06035504030c0564756d6d7930820122300d06092a864886f70d01010105000382010f003082010a0282010100a51b75b3f7da2d60ea1b0fc077f0dbb2bbb6fe1b474028368af8dc2664672896efff171033b0aede0b323a89d5c6db4d517404bc97b65264e41b9e9e86a6f40ace652498d4b3b859544d1bacfd7f86325503eed046f517406545c0ffb5560f83446dedce0fcafcc41ac8495488a6aa912ae45192ef7e3efa20d0f7403b0baa62c7e2e5404c620c5793623132aa20f624f08d88fbf0985af39433f5a24d0b908e5219d8ba6a404d3ee8418203b62a40c8eb18837354d50281a6a2bf5012e505c419482787b7a81e5935613ceea0c6d93e86f76282b6aa406fb3a1796c56b32e8a22afc3f7a3c9daa8f0e2846ff0d50abfc862a52f6cf0aaece6066c860376f3ed0203010001a3818a308187300c0603551d13040530030101ff30130603551d110101ff04093007820564756d6d79301206082b0601050507011f0101ff0403040100300e0603551d0f0101ff040403020184301d0603551d0e04160414e6e451ec8d19d9677b2d272a9d73b939fa2d915a301f0603551d23041830168014e6e451ec8d19d9677b2d272a9d73b939fa2d915a300d06092a864886f70d01010b0500038201010056d06047b7f48683e2347ca726997d9700b4f2cf1d8bc0ef17addac8445d38ffd7f8079055ead878b6a74c8384d0e30150c8990aa74f59cda6ebcb49465d8991ffa16a4c927a26e4639d1875a3ac396c7455c7eda40dbe66054a03d27f961c15e86bd5b06db6b26572977bcda93453b6b6a88ef96b31996a7bd17323525b33050d28deec9c33a3f9765a11fb99d0e222bd39a6db3a788474c9ca347377688f837d42f5841667bffcbe6b473e6f229f286a0829963e591a99aa7f67e9d20c36ccd2ac84cb85b7a8b3396a6cbe59a573ffff726f373197c230de5c92a52c5bc87e29c20bdf6e89609764a60c649022aabd768f3557661b083ae00e6afc8a5bf2ed":"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 CRL ASN1 (Incorrect first tag) x509parse_crl:"":"":MBEDTLS_ERR_X509_INVALID_FORMAT diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function index f3e83d69e..c52af76f5 100644 --- a/tests/suites/test_suite_x509parse.function +++ b/tests/suites/test_suite_x509parse.function @@ -301,6 +301,17 @@ int verify_parse_san( mbedtls_x509_subject_alternative_name *san, return( 0 ); } + +int parse_crt_ext_cb( mbedtls_x509_crt const *crt, mbedtls_x509_buf const *oid, int critical, + const unsigned char *p, const unsigned char *end ) +{ + ( void ) crt; + ( void ) p; + ( void ) end; + if( MBEDTLS_OID_CMP( MBEDTLS_OID_PKIX "\x01\x1F", oid ) != 0 && critical != 0 ) + return( MBEDTLS_ERR_ASN1_UNEXPECTED_TAG ); + return( 0 ); +} #endif /* MBEDTLS_X509_CRT_PARSE_C */ /* END_HEADER */ @@ -771,6 +782,77 @@ void x509parse_crt( data_t * buf, char * result_str, int result ) TEST_ASSERT( strcmp( (char *) output, result_str ) == 0 ); } + mbedtls_x509_crt_free( &crt ); + mbedtls_x509_crt_init( &crt ); + memset( output, 0, 2000 ); + + TEST_ASSERT( mbedtls_x509_crt_parse_der_with_ext_cb( &crt, buf->x, buf->len, 0, NULL ) == ( result ) ); + if( ( result ) == 0 ) + { + res = mbedtls_x509_crt_info( (char *) output, 2000, "", &crt ); + + TEST_ASSERT( res != -1 ); + TEST_ASSERT( res != -2 ); + + TEST_ASSERT( strcmp( (char *) output, result_str ) == 0 ); + } + + mbedtls_x509_crt_free( &crt ); + mbedtls_x509_crt_init( &crt ); + memset( output, 0, 2000 ); + + TEST_ASSERT( mbedtls_x509_crt_parse_der_with_ext_cb( &crt, buf->x, buf->len, 1, NULL ) == ( result ) ); + if( ( result ) == 0 ) + { + res = mbedtls_x509_crt_info( (char *) output, 2000, "", &crt ); + + TEST_ASSERT( res != -1 ); + TEST_ASSERT( res != -2 ); + + TEST_ASSERT( strcmp( (char *) output, result_str ) == 0 ); + } + +exit: + mbedtls_x509_crt_free( &crt ); +} +/* END_CASE */ + +/* BEGIN_CASE depends_on:MBEDTLS_X509_CRT_PARSE_C */ +void x509parse_crt_cb( data_t * buf, char * result_str, int result ) +{ + mbedtls_x509_crt crt; + unsigned char output[2000]; + int res; + + mbedtls_x509_crt_init( &crt ); + memset( output, 0, 2000 ); + + TEST_ASSERT( mbedtls_x509_crt_parse_der_with_ext_cb( &crt, buf->x, buf->len, 0, parse_crt_ext_cb ) == ( result ) ); + if( ( result ) == 0 ) + { + res = mbedtls_x509_crt_info( (char *) output, 2000, "", &crt ); + + TEST_ASSERT( res != -1 ); + TEST_ASSERT( res != -2 ); + + TEST_ASSERT( strcmp( (char *) output, result_str ) == 0 ); + } + + mbedtls_x509_crt_free( &crt ); + mbedtls_x509_crt_init( &crt ); + memset( output, 0, 2000 ); + + TEST_ASSERT( mbedtls_x509_crt_parse_der_with_ext_cb( &crt, buf->x, buf->len, 1, parse_crt_ext_cb ) == ( result ) ); + if( ( result ) == 0 ) + { + res = mbedtls_x509_crt_info( (char *) output, 2000, "", &crt ); + + TEST_ASSERT( res != -1 ); + TEST_ASSERT( res != -2 ); + + TEST_ASSERT( strcmp( (char *) output, result_str ) == 0 ); + } + exit: mbedtls_x509_crt_free( &crt ); } From 5659e7e8896186fcae67af773708059894e60772 Mon Sep 17 00:00:00 2001 From: Nicola Di Lieto Date: Thu, 28 May 2020 23:41:38 +0200 Subject: [PATCH 09/12] Add opaque context to mbedtls_x509_crt_ext_cb_t Signed-off-by: Nicola Di Lieto --- include/mbedtls/x509_crt.h | 8 ++++++-- library/x509_crt.c | 24 +++++++++++++--------- tests/suites/test_suite_x509parse.function | 13 ++++++------ 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index cdcc65157..296b472a7 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -310,6 +310,7 @@ int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, * mbedtls_x509_crt_parse_der_with_ext_cb() routine when * it encounters an unsupported extension. * + * \param p_ctx An opaque context passed to the callback. * \param crt The certificate being parsed. * \param oid The OID of the extension. * \param critical Whether the extension is critical. @@ -323,7 +324,8 @@ int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, * \return \c 0 on success. * \return A negative error code on failure. */ -typedef int (*mbedtls_x509_crt_ext_cb_t)( mbedtls_x509_crt const *crt, +typedef int (*mbedtls_x509_crt_ext_cb_t)( void *p_ctx, + mbedtls_x509_crt const *crt, mbedtls_x509_buf const *oid, int critical, const unsigned char *p, @@ -347,6 +349,7 @@ typedef int (*mbedtls_x509_crt_ext_cb_t)( mbedtls_x509_crt const *crt, * is destroyed (like mbedtls_x509_crt_parse_der_nocopy()) * \param cb A callback invoked for every unsupported certificate * extension. + * \param p_ctx An opaque context passed to the callback. * * \note This call is functionally equivalent to * mbedtls_x509_crt_parse_der(), and/or @@ -363,7 +366,8 @@ int mbedtls_x509_crt_parse_der_with_ext_cb( mbedtls_x509_crt *chain, const unsigned char *buf, size_t buflen, int make_copy, - mbedtls_x509_crt_ext_cb_t cb ); + mbedtls_x509_crt_ext_cb_t cb, + void *p_ctx ); /** * \brief Parse a single DER formatted certificate and add it diff --git a/library/x509_crt.c b/library/x509_crt.c index 554352291..99d3be200 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -893,7 +893,8 @@ static int x509_get_certificate_policies( unsigned char **p, static int x509_get_crt_ext( unsigned char **p, const unsigned char *end, mbedtls_x509_crt *crt, - mbedtls_x509_crt_ext_cb_t cb ) + mbedtls_x509_crt_ext_cb_t cb, + void *p_ctx ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; size_t len; @@ -959,7 +960,7 @@ static int x509_get_crt_ext( unsigned char **p, /* Give the callback (if any) a chance to handle the extension */ if( cb != NULL ) { - ret = cb( crt, &extn_oid, is_critical, *p, end_ext_octet ); + ret = cb( p_ctx, crt, &extn_oid, is_critical, *p, end_ext_octet ); if( ret != 0 ) return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret ); *p = end_ext_octet; @@ -1073,7 +1074,8 @@ static int x509_crt_parse_der_core( mbedtls_x509_crt *crt, const unsigned char *buf, size_t buflen, int make_copy, - mbedtls_x509_crt_ext_cb_t cb ) + mbedtls_x509_crt_ext_cb_t cb, + void *p_ctx ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; size_t len; @@ -1272,7 +1274,7 @@ static int x509_crt_parse_der_core( mbedtls_x509_crt *crt, if( crt->version == 3 ) #endif { - ret = x509_get_crt_ext( &p, end, crt, cb ); + ret = x509_get_crt_ext( &p, end, crt, cb, p_ctx ); if( ret != 0 ) { mbedtls_x509_crt_free( crt ); @@ -1336,7 +1338,8 @@ static int mbedtls_x509_crt_parse_der_internal( mbedtls_x509_crt *chain, const unsigned char *buf, size_t buflen, int make_copy, - mbedtls_x509_crt_ext_cb_t cb ) + mbedtls_x509_crt_ext_cb_t cb, + void *p_ctx ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; mbedtls_x509_crt *crt = chain, *prev = NULL; @@ -1368,7 +1371,7 @@ static int mbedtls_x509_crt_parse_der_internal( mbedtls_x509_crt *chain, crt = crt->next; } - ret = x509_crt_parse_der_core( crt, buf, buflen, make_copy, cb ); + ret = x509_crt_parse_der_core( crt, buf, buflen, make_copy, cb, p_ctx ); if( ret != 0 ) { if( prev ) @@ -1387,23 +1390,24 @@ int mbedtls_x509_crt_parse_der_nocopy( mbedtls_x509_crt *chain, const unsigned char *buf, size_t buflen ) { - return( mbedtls_x509_crt_parse_der_internal( chain, buf, buflen, 0, NULL ) ); + return( mbedtls_x509_crt_parse_der_internal( chain, buf, buflen, 0, NULL, NULL ) ); } int mbedtls_x509_crt_parse_der_with_ext_cb( mbedtls_x509_crt *chain, const unsigned char *buf, size_t buflen, int make_copy, - mbedtls_x509_crt_ext_cb_t cb ) + mbedtls_x509_crt_ext_cb_t cb, + void *p_ctx ) { - return( mbedtls_x509_crt_parse_der_internal( chain, buf, buflen, make_copy, cb ) ); + return( mbedtls_x509_crt_parse_der_internal( chain, buf, buflen, make_copy, cb, p_ctx ) ); } int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, const unsigned char *buf, size_t buflen ) { - return( mbedtls_x509_crt_parse_der_internal( chain, buf, buflen, 1, NULL ) ); + return( mbedtls_x509_crt_parse_der_internal( chain, buf, buflen, 1, NULL, NULL ) ); } /* diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function index c52af76f5..0e2719d8e 100644 --- a/tests/suites/test_suite_x509parse.function +++ b/tests/suites/test_suite_x509parse.function @@ -302,9 +302,10 @@ int verify_parse_san( mbedtls_x509_subject_alternative_name *san, return( 0 ); } -int parse_crt_ext_cb( mbedtls_x509_crt const *crt, mbedtls_x509_buf const *oid, int critical, - const unsigned char *p, const unsigned char *end ) +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 ) { + ( void ) p_ctx; ( void ) crt; ( void ) p; ( void ) end; @@ -786,7 +787,7 @@ void x509parse_crt( data_t * buf, char * result_str, int result ) mbedtls_x509_crt_init( &crt ); memset( output, 0, 2000 ); - TEST_ASSERT( mbedtls_x509_crt_parse_der_with_ext_cb( &crt, buf->x, buf->len, 0, NULL ) == ( result ) ); + TEST_ASSERT( mbedtls_x509_crt_parse_der_with_ext_cb( &crt, buf->x, buf->len, 0, NULL, NULL ) == ( result ) ); if( ( result ) == 0 ) { res = mbedtls_x509_crt_info( (char *) output, 2000, "", &crt ); @@ -801,7 +802,7 @@ void x509parse_crt( data_t * buf, char * result_str, int result ) mbedtls_x509_crt_init( &crt ); memset( output, 0, 2000 ); - TEST_ASSERT( mbedtls_x509_crt_parse_der_with_ext_cb( &crt, buf->x, buf->len, 1, NULL ) == ( result ) ); + TEST_ASSERT( mbedtls_x509_crt_parse_der_with_ext_cb( &crt, buf->x, buf->len, 1, NULL, NULL ) == ( result ) ); if( ( result ) == 0 ) { res = mbedtls_x509_crt_info( (char *) output, 2000, "", &crt ); @@ -827,7 +828,7 @@ void x509parse_crt_cb( data_t * buf, char * result_str, int result ) mbedtls_x509_crt_init( &crt ); memset( output, 0, 2000 ); - TEST_ASSERT( mbedtls_x509_crt_parse_der_with_ext_cb( &crt, buf->x, buf->len, 0, parse_crt_ext_cb ) == ( result ) ); + TEST_ASSERT( mbedtls_x509_crt_parse_der_with_ext_cb( &crt, buf->x, buf->len, 0, parse_crt_ext_cb, NULL ) == ( result ) ); if( ( result ) == 0 ) { res = mbedtls_x509_crt_info( (char *) output, 2000, "", &crt ); @@ -842,7 +843,7 @@ void x509parse_crt_cb( data_t * buf, char * result_str, int result ) mbedtls_x509_crt_init( &crt ); memset( output, 0, 2000 ); - TEST_ASSERT( mbedtls_x509_crt_parse_der_with_ext_cb( &crt, buf->x, buf->len, 1, parse_crt_ext_cb ) == ( result ) ); + TEST_ASSERT( mbedtls_x509_crt_parse_der_with_ext_cb( &crt, buf->x, buf->len, 1, parse_crt_ext_cb, NULL ) == ( result ) ); if( ( result ) == 0 ) { res = mbedtls_x509_crt_info( (char *) output, 2000, "", &crt ); From 565b52bb727a81b82ad07a9bcca5ca033b554a24 Mon Sep 17 00:00:00 2001 From: Nicola Di Lieto Date: Fri, 29 May 2020 22:46:56 +0200 Subject: [PATCH 10/12] mbedtls_x509_crt_parse_der_with_ext_cb improvement Continue parsing when the callback fails to parse a non critical exception. Also document the behaviour more extensively and pass the callback error code to the caller unaltered. See https://github.com/ARMmbed/mbedtls/pull/3243#discussion_r432630548 and https://github.com/ARMmbed/mbedtls/pull/3243#discussion_r432630968 Signed-off-by: Nicola Di Lieto --- include/mbedtls/x509_crt.h | 16 +++++++++++++--- library/x509_crt.c | 4 ++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index 296b472a7..9a9b397d9 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -317,9 +317,14 @@ int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, * \param p Pointer to the start of the extension value * (the content of the OCTET STRING). * \param end End of extension value. - * - * \note The callback must fail and return a negative error code if - * it can not parse or does not support the extension. + * + * \note The callback must fail and return a negative error code + * if it can not parse or does not support the 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. * * \return \c 0 on success. * \return A negative error code on failure. @@ -358,6 +363,11 @@ typedef int (*mbedtls_x509_crt_ext_cb_t)( void *p_ctx, * certificate extension. * 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. * * \return \c 0 if successful. * \return A negative error code on failure. diff --git a/library/x509_crt.c b/library/x509_crt.c index 99d3be200..490b52454 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -961,8 +961,8 @@ static int x509_get_crt_ext( unsigned char **p, if( cb != NULL ) { ret = cb( p_ctx, crt, &extn_oid, is_critical, *p, end_ext_octet ); - if( ret != 0 ) - return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret ); + if( ret != 0 && is_critical ) + return( ret ); *p = end_ext_octet; continue; } From e58b4638e594a0ac16a19e40d96da09508f0a7ff Mon Sep 17 00:00:00 2001 From: Nicola Di Lieto Date: Fri, 29 May 2020 22:58:25 +0200 Subject: [PATCH 11/12] Unsupported extension tests in test_suite_x509parse All combinations of critical or not, recognized or not by the callback are now tested as requested in https://github.com/ARMmbed/mbedtls/pull/3243#discussion_r432647880 In addition pass the OID of the unsupported extension to be parsed to the callback using the opaque pointer, which makes the tests fail if the library forwards the wrong pointer to the callback, as requested in https://github.com/ARMmbed/mbedtls/pull/3243#discussion_r432647392 Signed-off-by: Nicola Di Lieto --- tests/suites/test_suite_x509parse.data | 14 +++++++++++++- tests/suites/test_suite_x509parse.function | 17 ++++++++++++----- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index f5345e293..37e759feb 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -1992,10 +1992,22 @@ X509 CRT ASN1 (Unsupported critical extension) depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C:!MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION x509parse_crt:"308203353082021da00302010202104d3ebbb8a870f9c78c55a8a7e12fd516300d06092a864886f70d01010b05003010310e300c06035504030c0564756d6d79301e170d3230303432383137343234335a170d3230303632373137343234335a3010310e300c06035504030c0564756d6d7930820122300d06092a864886f70d01010105000382010f003082010a0282010100a51b75b3f7da2d60ea1b0fc077f0dbb2bbb6fe1b474028368af8dc2664672896efff171033b0aede0b323a89d5c6db4d517404bc97b65264e41b9e9e86a6f40ace652498d4b3b859544d1bacfd7f86325503eed046f517406545c0ffb5560f83446dedce0fcafcc41ac8495488a6aa912ae45192ef7e3efa20d0f7403b0baa62c7e2e5404c620c5793623132aa20f624f08d88fbf0985af39433f5a24d0b908e5219d8ba6a404d3ee8418203b62a40c8eb18837354d50281a6a2bf5012e505c419482787b7a81e5935613ceea0c6d93e86f76282b6aa406fb3a1796c56b32e8a22afc3f7a3c9daa8f0e2846ff0d50abfc862a52f6cf0aaece6066c860376f3ed0203010001a3818a308187300c0603551d13040530030101ff30130603551d110101ff04093007820564756d6d79301206082b0601050507011f0101ff0403040100300e0603551d0f0101ff040403020184301d0603551d0e04160414e6e451ec8d19d9677b2d272a9d73b939fa2d915a301f0603551d23041830168014e6e451ec8d19d9677b2d272a9d73b939fa2d915a300d06092a864886f70d01010b0500038201010056d06047b7f48683e2347ca726997d9700b4f2cf1d8bc0ef17addac8445d38ffd7f8079055ead878b6a74c8384d0e30150c8990aa74f59cda6ebcb49465d8991ffa16a4c927a26e4639d1875a3ac396c7455c7eda40dbe66054a03d27f961c15e86bd5b06db6b26572977bcda93453b6b6a88ef96b31996a7bd17323525b33050d28deec9c33a3f9765a11fb99d0e222bd39a6db3a788474c9ca347377688f837d42f5841667bffcbe6b473e6f229f286a0829963e591a99aa7f67e9d20c36ccd2ac84cb85b7a8b3396a6cbe59a573ffff726f373197c230de5c92a52c5bc87e29c20bdf6e89609764a60c649022aabd768f3557661b083ae00e6afc8a5bf2ed":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG -X509 CRT ASN1 (Unsupported critical extension with callback) +X509 CRT ASN1 (Unsupported critical extension recognized by callback) depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C x509parse_crt_cb:"308203353082021da00302010202104d3ebbb8a870f9c78c55a8a7e12fd516300d06092a864886f70d01010b05003010310e300c06035504030c0564756d6d79301e170d3230303432383137343234335a170d3230303632373137343234335a3010310e300c06035504030c0564756d6d7930820122300d06092a864886f70d01010105000382010f003082010a0282010100a51b75b3f7da2d60ea1b0fc077f0dbb2bbb6fe1b474028368af8dc2664672896efff171033b0aede0b323a89d5c6db4d517404bc97b65264e41b9e9e86a6f40ace652498d4b3b859544d1bacfd7f86325503eed046f517406545c0ffb5560f83446dedce0fcafcc41ac8495488a6aa912ae45192ef7e3efa20d0f7403b0baa62c7e2e5404c620c5793623132aa20f624f08d88fbf0985af39433f5a24d0b908e5219d8ba6a404d3ee8418203b62a40c8eb18837354d50281a6a2bf5012e505c419482787b7a81e5935613ceea0c6d93e86f76282b6aa406fb3a1796c56b32e8a22afc3f7a3c9daa8f0e2846ff0d50abfc862a52f6cf0aaece6066c860376f3ed0203010001a3818a308187300c0603551d13040530030101ff30130603551d110101ff04093007820564756d6d79301206082b0601050507011f0101ff0403040100300e0603551d0f0101ff040403020184301d0603551d0e04160414e6e451ec8d19d9677b2d272a9d73b939fa2d915a301f0603551d23041830168014e6e451ec8d19d9677b2d272a9d73b939fa2d915a300d06092a864886f70d01010b0500038201010056d06047b7f48683e2347ca726997d9700b4f2cf1d8bc0ef17addac8445d38ffd7f8079055ead878b6a74c8384d0e30150c8990aa74f59cda6ebcb49465d8991ffa16a4c927a26e4639d1875a3ac396c7455c7eda40dbe66054a03d27f961c15e86bd5b06db6b26572977bcda93453b6b6a88ef96b31996a7bd17323525b33050d28deec9c33a3f9765a11fb99d0e222bd39a6db3a788474c9ca347377688f837d42f5841667bffcbe6b473e6f229f286a0829963e591a99aa7f67e9d20c36ccd2ac84cb85b7a8b3396a6cbe59a573ffff726f373197c230de5c92a52c5bc87e29c20bdf6e89609764a60c649022aabd768f3557661b083ae00e6afc8a5bf2ed":"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 extension not recognized by callback) +depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C +x509parse_crt_cb:"308203353082021da00302010202104d3ebbb8a870f9c78c55a8a7e12fd516300d06092a864886f70d01010b05003010310e300c06035504030c0564756d6d79301e170d3230303432383137343234335a170d3230303632373137343234335a3010310e300c06035504030c0564756d6d7930820122300d06092a864886f70d01010105000382010f003082010a0282010100a51b75b3f7da2d60ea1b0fc077f0dbb2bbb6fe1b474028368af8dc2664672896efff171033b0aede0b323a89d5c6db4d517404bc97b65264e41b9e9e86a6f40ace652498d4b3b859544d1bacfd7f86325503eed046f517406545c0ffb5560f83446dedce0fcafcc41ac8495488a6aa912ae45192ef7e3efa20d0f7403b0baa62c7e2e5404c620c5793623132aa20f624f08d88fbf0985af39433f5a24d0b908e5219d8ba6a404d3ee8418203b62a40c8eb18837354d50281a6a2bf5012e505c419482787b7a81e5935613ceea0c6d93e86f76282b6aa406fb3a1796c56b32e8a22afc3f7a3c9daa8f0e2846ff0d50abfc862a52f6cf0aaece6066c860376f3ed0203010001a3818a308187300c0603551d13040530030101ff30130603551d110101ff04093007820564756d6d79301206082b0601050507011e0101ff0403040100300e0603551d0f0101ff040403020184301d0603551d0e04160414e6e451ec8d19d9677b2d272a9d73b939fa2d915a301f0603551d23041830168014e6e451ec8d19d9677b2d272a9d73b939fa2d915a300d06092a864886f70d01010b0500038201010056d06047b7f48683e2347ca726997d9700b4f2cf1d8bc0ef17addac8445d38ffd7f8079055ead878b6a74c8384d0e30150c8990aa74f59cda6ebcb49465d8991ffa16a4c927a26e4639d1875a3ac396c7455c7eda40dbe66054a03d27f961c15e86bd5b06db6b26572977bcda93453b6b6a88ef96b31996a7bd17323525b33050d28deec9c33a3f9765a11fb99d0e222bd39a6db3a788474c9ca347377688f837d42f5841667bffcbe6b473e6f229f286a0829963e591a99aa7f67e9d20c36ccd2ac84cb85b7a8b3396a6cbe59a573ffff726f373197c230de5c92a52c5bc87e29c20bdf6e89609764a60c649022aabd768f3557661b083ae00e6afc8a5bf2ed":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG + +X509 CRT ASN1 (Unsupported non critical extension recognized by callback) +depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C +x509parse_crt_cb:"308203353082021da00302010202104d3ebbb8a870f9c78c55a8a7e12fd516300d06092a864886f70d01010b05003010310e300c06035504030c0564756d6d79301e170d3230303432383137343234335a170d3230303632373137343234335a3010310e300c06035504030c0564756d6d7930820122300d06092a864886f70d01010105000382010f003082010a0282010100a51b75b3f7da2d60ea1b0fc077f0dbb2bbb6fe1b474028368af8dc2664672896efff171033b0aede0b323a89d5c6db4d517404bc97b65264e41b9e9e86a6f40ace652498d4b3b859544d1bacfd7f86325503eed046f517406545c0ffb5560f83446dedce0fcafcc41ac8495488a6aa912ae45192ef7e3efa20d0f7403b0baa62c7e2e5404c620c5793623132aa20f624f08d88fbf0985af39433f5a24d0b908e5219d8ba6a404d3ee8418203b62a40c8eb18837354d50281a6a2bf5012e505c419482787b7a81e5935613ceea0c6d93e86f76282b6aa406fb3a1796c56b32e8a22afc3f7a3c9daa8f0e2846ff0d50abfc862a52f6cf0aaece6066c860376f3ed0203010001a3818a308187300c0603551d13040530030101ff30130603551d110101ff04093007820564756d6d79301206082b0601050507011f0101000403040100300e0603551d0f0101ff040403020184301d0603551d0e04160414e6e451ec8d19d9677b2d272a9d73b939fa2d915a301f0603551d23041830168014e6e451ec8d19d9677b2d272a9d73b939fa2d915a300d06092a864886f70d01010b0500038201010056d06047b7f48683e2347ca726997d9700b4f2cf1d8bc0ef17addac8445d38ffd7f8079055ead878b6a74c8384d0e30150c8990aa74f59cda6ebcb49465d8991ffa16a4c927a26e4639d1875a3ac396c7455c7eda40dbe66054a03d27f961c15e86bd5b06db6b26572977bcda93453b6b6a88ef96b31996a7bd17323525b33050d28deec9c33a3f9765a11fb99d0e222bd39a6db3a788474c9ca347377688f837d42f5841667bffcbe6b473e6f229f286a0829963e591a99aa7f67e9d20c36ccd2ac84cb85b7a8b3396a6cbe59a573ffff726f373197c230de5c92a52c5bc87e29c20bdf6e89609764a60c649022aabd768f3557661b083ae00e6afc8a5bf2ed":"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 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 CRL ASN1 (Incorrect first tag) x509parse_crl:"":"":MBEDTLS_ERR_X509_INVALID_FORMAT diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function index 0e2719d8e..54e515673 100644 --- a/tests/suites/test_suite_x509parse.function +++ b/tests/suites/test_suite_x509parse.function @@ -305,12 +305,14 @@ 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 ) { - ( void ) p_ctx; ( void ) crt; ( void ) p; ( void ) end; - if( MBEDTLS_OID_CMP( MBEDTLS_OID_PKIX "\x01\x1F", oid ) != 0 && critical != 0 ) - return( MBEDTLS_ERR_ASN1_UNEXPECTED_TAG ); + ( 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 ) + return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG ); return( 0 ); } #endif /* MBEDTLS_X509_CRT_PARSE_C */ @@ -822,13 +824,18 @@ exit: void x509parse_crt_cb( data_t * buf, char * result_str, int result ) { mbedtls_x509_crt crt; + mbedtls_x509_buf oid; unsigned char output[2000]; int res; + oid.tag = MBEDTLS_ASN1_OID; + oid.len = MBEDTLS_OID_SIZE(MBEDTLS_OID_PKIX "\x01\x1F"); + oid.p = (unsigned char *)MBEDTLS_OID_PKIX "\x01\x1F"; + mbedtls_x509_crt_init( &crt ); memset( output, 0, 2000 ); - TEST_ASSERT( mbedtls_x509_crt_parse_der_with_ext_cb( &crt, buf->x, buf->len, 0, parse_crt_ext_cb, NULL ) == ( result ) ); + TEST_ASSERT( mbedtls_x509_crt_parse_der_with_ext_cb( &crt, buf->x, buf->len, 0, parse_crt_ext_cb, &oid ) == ( result ) ); if( ( result ) == 0 ) { res = mbedtls_x509_crt_info( (char *) output, 2000, "", &crt ); @@ -843,7 +850,7 @@ void x509parse_crt_cb( data_t * buf, char * result_str, int result ) mbedtls_x509_crt_init( &crt ); memset( output, 0, 2000 ); - TEST_ASSERT( mbedtls_x509_crt_parse_der_with_ext_cb( &crt, buf->x, buf->len, 1, parse_crt_ext_cb, NULL ) == ( result ) ); + TEST_ASSERT( mbedtls_x509_crt_parse_der_with_ext_cb( &crt, buf->x, buf->len, 1, parse_crt_ext_cb, &oid ) == ( result ) ); if( ( result ) == 0 ) { res = mbedtls_x509_crt_info( (char *) output, 2000, "", &crt ); From 110a794e93795ebaffb21a1de91de9c572fb29c7 Mon Sep 17 00:00:00 2001 From: Nicola Di Lieto Date: Fri, 29 May 2020 23:27:47 +0200 Subject: [PATCH 12/12] Add ChangeLog.d/new-mbedtls_x509_crt_parse_der_with_ext_cb_routine.txt Signed-off-by: Nicola Di Lieto --- .../new-mbedtls_x509_crt_parse_der_with_ext_cb_routine.txt | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 ChangeLog.d/new-mbedtls_x509_crt_parse_der_with_ext_cb_routine.txt diff --git a/ChangeLog.d/new-mbedtls_x509_crt_parse_der_with_ext_cb_routine.txt b/ChangeLog.d/new-mbedtls_x509_crt_parse_der_with_ext_cb_routine.txt new file mode 100644 index 000000000..fdea746de --- /dev/null +++ b/ChangeLog.d/new-mbedtls_x509_crt_parse_der_with_ext_cb_routine.txt @@ -0,0 +1,5 @@ +Features + * Add new mbedtls_x509_crt_parse_der_with_ext_cb() routine which allows + parsing unsupported certificate extensions via user provided callback. + Contributed by Nicola Di Lieto in #3243 as + a solution to #3241.