Merge pull request #4477 from TRodziewicz/Remove__X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION

Remove MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION
2021-06-10 09:13:14 +02:00 · 2021-06-10 09:13:14 +02:00 · 44eea8f067
commit 44eea8f067
parent 364380e70c 0ea2576502
6 changed files with 24 additions and 23 deletions
--- a/ChangeLog.d/issue4378.txt
+++ b/ChangeLog.d/issue4378.txt
@ -0,0 +1,4 @@
+Removals
+   * Remove the  MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION config.h
+     option. The mbedtls_x509_crt_parse_der_with_ext_cb() is the way to go for
+     migration path. Fixes #4378.
--- a/docs/3.0-migration-guide.d/remove_MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION.md
+++ b/docs/3.0-migration-guide.d/remove_MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION.md
@ -0,0 +1,17 @@
+Remove the config option MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION
+--------------------------------------------------------------------------
+
+This change does not affect users of the default configuration; it only affect
+users who enable this option.
+
+The X.509 standard says that implementations must reject critical extensions that
+they don't recognize, and this is what Mbed TLS does by default. This option
+allowed to continue parsing those certificates but didn't provide a convenient
+way to handle those extensions.
+
+The migration path from that option is to use the
+`mbedtls_x509_crt_parse_der_with_ext_cb()` function which 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 and additionally the "certificate policies"
+extension if it contains any unsupported certificate policies.
--- a/include/mbedtls/config.h
+++ b/include/mbedtls/config.h
@ -1813,18 +1813,6 @@
 */
 #define MBEDTLS_VERSION_FEATURES

-/**
- * \def MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION
- *
- * If set, the X509 parser will not break-off when parsing an X509 certificate
- * and encountering an unknown critical extension.
- *
- * \warning Depending on your PKI use, enabling this can be a security risk!
- *
- * Uncomment to prevent an error.
- */
-//#define MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION
-
 /**
 * \def MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK
 *
--- a/library/x509_crt.c
+++ b/library/x509_crt.c
@ -818,8 +818,7 @@ static int x509_get_certificate_policies( unsigned char **p,
        {
            /*
             * Set the parsing return code but continue parsing, in case this
-             * extension is critical and MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION
-             * is configured.
+             * extension is critical.
             */
            parse_ret = MBEDTLS_ERR_X509_FEATURE_UNAVAILABLE;
        }
@ -961,14 +960,12 @@ static int x509_get_crt_ext( unsigned char **p,
            /* No parser found, skip extension */
            *p = end_ext_octet;

-#if !defined(MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION)
            if( is_critical )
            {
                /* Data is marked as critical: fail */
                return( MBEDTLS_ERROR_ADD( MBEDTLS_ERR_X509_INVALID_EXTENSIONS,
                        MBEDTLS_ERR_ASN1_UNEXPECTED_TAG ) );
            }
-#endif
            continue;
        }

@ -1027,11 +1024,9 @@ static int x509_get_crt_ext( unsigned char **p,
                        start_ext_octet, end_ext_octet ) == 0 )
                    break;

-#if !defined(MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION)
                if( is_critical )
                    return( ret );
                else
-#endif
                /*
                 * If MBEDTLS_ERR_X509_FEATURE_UNAVAILABLE is returned, then we
                 * cannot interpret or enforce the policy. However, it is up to
@ -1049,11 +1044,9 @@ static int x509_get_crt_ext( unsigned char **p,
             * supports, but there isn't an x509 parser for it,
             * skip the extension.
             */
-#if !defined(MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION)
            if( is_critical )
                return( MBEDTLS_ERR_X509_FEATURE_UNAVAILABLE );
            else
-#endif
                *p = end_ext_octet;
        }
    }
--- a/scripts/config.py
+++ b/scripts/config.py
@ -192,7 +192,6 @@ EXCLUDE_FROM_FULL = frozenset([
    'MBEDTLS_RSA_NO_CRT', # influences the use of RSA in X.509 and TLS
    'MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN', # build dependency (clang+memsan)
    'MBEDTLS_TEST_CONSTANT_FLOW_VALGRIND', # build dependency (valgrind headers)
-    'MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION', # influences the use of X.509 in TLS
    'MBEDTLS_X509_REMOVE_INFO', # removes a feature
 ])

--- a/tests/suites/test_suite_x509parse.data
+++ b/tests/suites/test_suite_x509parse.data
@ -1783,7 +1783,7 @@ depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
 x509parse_crt:"3081ad308197a0030201028204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092a864886f70d010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa100a200a311300f300d0603551d200406300430020601300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_OUT_OF_DATA

 X509 CRT ASN1 (TBSCertificate v3, inv CertificatePolicies, unknown critical policy)
-depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C:!MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION
+depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
 x509parse_crt:"3081b130819ba0030201028204deadbeef300d06092a864886f70d01010b0500300c310a30080600130454657374301c170c303930313031303030303030170c303931323331323335393539300c310a30080600130454657374302a300d06092a864886f70d010101050003190030160210ffffffffffffffffffffffffffffffff0202ffffa100a200a315301330110603551d20010101040730053003060100300d06092a864886f70d01010b0500030200ff":"":MBEDTLS_ERR_X509_FEATURE_UNAVAILABLE

 X509 CRT ASN1 (TBSCertificate v3, inv CertificatePolicies, policy qualifier invalid tag)
@ -2001,7 +2001,7 @@ depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED:MBEDTLS_SHA1_C:MBEDT
 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
+depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
 x509parse_crt:"308203353082021da00302010202104d3ebbb8a870f9c78c55a8a7e12fd516300d06092a864886f70d01010b05003010310e300c06035504030c0564756d6d79301e170d3230303432383137343234335a170d3230303632373137343234335a3010310e300c06035504030c0564756d6d7930820122300d06092a864886f70d01010105000382010f003082010a0282010100a51b75b3f7da2d60ea1b0fc077f0dbb2bbb6fe1b474028368af8dc2664672896efff171033b0aede0b323a89d5c6db4d517404bc97b65264e41b9e9e86a6f40ace652498d4b3b859544d1bacfd7f86325503eed046f517406545c0ffb5560f83446dedce0fcafcc41ac8495488a6aa912ae45192ef7e3efa20d0f7403b0baa62c7e2e5404c620c5793623132aa20f624f08d88fbf0985af39433f5a24d0b908e5219d8ba6a404d3ee8418203b62a40c8eb18837354d50281a6a2bf5012e505c419482787b7a81e5935613ceea0c6d93e86f76282b6aa406fb3a1796c56b32e8a22afc3f7a3c9daa8f0e2846ff0d50abfc862a52f6cf0aaece6066c860376f3ed0203010001a3818a308187300c0603551d13040530030101ff30130603551d110101ff04093007820564756d6d79301206082b0601050507011f0101ff0403040100300e0603551d0f0101ff040403020184301d0603551d0e04160414e6e451ec8d19d9677b2d272a9d73b939fa2d915a301f0603551d23041830168014e6e451ec8d19d9677b2d272a9d73b939fa2d915a300d06092a864886f70d01010b0500038201010056d06047b7f48683e2347ca726997d9700b4f2cf1d8bc0ef17addac8445d38ffd7f8079055ead878b6a74c8384d0e30150c8990aa74f59cda6ebcb49465d8991ffa16a4c927a26e4639d1875a3ac396c7455c7eda40dbe66054a03d27f961c15e86bd5b06db6b26572977bcda93453b6b6a88ef96b31996a7bd17323525b33050d28deec9c33a3f9765a11fb99d0e222bd39a6db3a788474c9ca347377688f837d42f5841667bffcbe6b473e6f229f286a0829963e591a99aa7f67e9d20c36ccd2ac84cb85b7a8b3396a6cbe59a573ffff726f373197c230de5c92a52c5bc87e29c20bdf6e89609764a60c649022aabd768f3557661b083ae00e6afc8a5bf2ed":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG

 X509 CRT ASN1 (Unsupported critical extension recognized by callback)