x509: CRL: reject unsupported critical extensions

ChangeLog

@@ -17,6 +17,8 @@ Security
      implementation allowed an offline 2^80 brute force attack on the
      HMAC key of a single, uninterrupted connection (with no
      resumption of the session).
+   * Fix CRL parsing to reject CRLs containing unsupported critical
+     extensions. Found by Falko Strenzke and Evangelos Karatsiolis.
 
 Features
    * Extend PKCS#8 interface by introducing support for the entire SHA

library/x509_crl.c

@@ -95,16 +95,22 @@ static int x509_crl_get_version( unsigned char **p,
 }
 
 /*
- * X.509 CRL v2 extensions (no extensions parsed yet.)
+ * X.509 CRL v2 extensions
+ *
+ * We currently don't parse any extension's content, but we do check that the
+ * list of extensions is well-formed and abort on critical extensions (that
+ * are unsupported as we don't support any extension so far)
  */
 static int x509_get_crl_ext( unsigned char **p,
                              const unsigned char *end,
                              mbedtls_x509_buf *ext )
 {
     int ret;
-    size_t len = 0;
 
-    /* Get explicit tag */
+    /*
+     * 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 )
@@ -115,11 +121,54 @@ static int x509_get_crl_ext( unsigned char **p,
 
     while( *p < end )
     {
+        /*
+         * Extension  ::=  SEQUENCE  {
+         *      extnID      OBJECT IDENTIFIER,
+         *      critical    BOOLEAN DEFAULT FALSE,
+         *      extnValue   OCTET STRING  }
+         */
+        int is_critical = 0;
+        const unsigned char *end_ext_data;
+        size_t len;
+
+        /* Get enclosing sequence tag */
         if( ( ret = mbedtls_asn1_get_tag( p, end, &len,
                 MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ) != 0 )
             return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret );
 
+        end_ext_data = *p + len;
+
+        /* Get OID (currently ignored) */
+        if( ( ret = mbedtls_asn1_get_tag( p, end_ext_data, &len,
+                                          MBEDTLS_ASN1_OID ) ) != 0 )
+        {
+            return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret );
+        }
         *p += len;
+
+        /* Get optional critical */
+        if( ( ret = mbedtls_asn1_get_bool( p, end_ext_data,
+                                           &is_critical ) ) != 0 &&
+            ( ret != MBEDTLS_ERR_ASN1_UNEXPECTED_TAG ) )
+        {
+            return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret );
+        }
+
+        /* Data should be octet string type */
+        if( ( ret = mbedtls_asn1_get_tag( p, end_ext_data, &len,
+                MBEDTLS_ASN1_OCTET_STRING ) ) != 0 )
+            return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret );
+
+        /* Ignore data so far and just check its length */
+        *p += len;
+        if( *p != end_ext_data )
+            return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS +
+                    MBEDTLS_ERR_ASN1_LENGTH_MISMATCH );
+
+        /* Abort on (unsupported) critical extensions */
+        if( is_critical )
+            return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS +
+                    MBEDTLS_ERR_ASN1_UNEXPECTED_TAG );
     }
 
     if( *p != end )

tests/data_files/Makefile

@@ -46,6 +46,9 @@ test-ca-sha256.crt: $(test_ca_key_file_rsa) $(test_ca_config_file) test-ca.csr
 	$(OPENSSL) req -x509 -config $(test_ca_config_file) -key $(test_ca_key_file_rsa) -passin "pass:$(test_ca_pwd_rsa)" -set_serial 0 -days 3653 -sha256 -in test-ca.csr -out $@
 all_final += test-ca-sha256.crt
 
+crl-idp.pem: $(test_ca_crt) $(test_ca_key_file_rsa) $(test_ca_config_file)
+	$(OPENSSL) ca -gencrl -batch -cert $(test_ca_crt) -keyfile $(test_ca_key_file_rsa) -key $(test_ca_pwd_rsa) -config $(test_ca_config_file) -name test_ca -md sha256 -crldays 3653 -crlexts crl_ext_idp -out $@
+
 cli_crt_key_file_rsa = cli-rsa.key
 cli_crt_extensions_file = cli.opensslconf
 

tests/data_files/crl-idp.pem

@@ -0,0 +1,12 @@
+-----BEGIN X509 CRL-----
+MIIBszCBnAIBATANBgkqhkiG9w0BAQsFADA7MQswCQYDVQQGEwJOTDERMA8GA1UE
+ChMIUG9sYXJTU0wxGTAXBgNVBAMTEFBvbGFyU1NMIFRlc3QgQ0EXDTE4MDMxNDA3
+MzE0OFoXDTI4MDMxNDA3MzE0OFqgLTArMCkGA1UdHAEB/wQfMB2gG6AZhhdodHRw
+Oi8vcGtpLmV4YW1wbGUuY29tLzANBgkqhkiG9w0BAQsFAAOCAQEAs/vp1Ybq9Lj/
+YM+O2uBqhRNdt494GYSYcZcltbQDwLgDwsFQ9S+q5zBtanhxiF3C6dyDoWS6xyY3
+dkdO9kK2YAQLNaFBCsKRrI9vGKuF5/1uIr0a8cQcqVzyRI9uK0KgGEk9/APGtqob
+nj/nt2ryGC+yEh20FmvwFn1vN5xaWK3uUIJCNDTZe+KQn150iAU/mWZG2xDdSXgm
+JtpTrY6toBgTwDGyus2wIDvAF6rBc1lRoR0BPuTR1fcUPMvr8jceZqG+xuH+vmkU
+j1B4Tu+K27ZmZMlhltfgwLzcgH9Ee1TgWPN2QqMzeZW/vNMyIIvWAWk2cFyCJj6r
+16/9upL64w==
+-----END X509 CRL-----

tests/data_files/test-ca.opensslconf

@@ -11,3 +11,12 @@ commonName = PolarSSL Test CA
 subjectKeyIdentifier=hash
 authorityKeyIdentifier=keyid:always,issuer:always
 basicConstraints = CA:true
+
+[test_ca]
+database = /dev/null
+
+[crl_ext_idp]
+issuingDistributionPoint=critical, @idpdata
+
+[idpdata]
+fullname=URI:http://pki.example.com/

tests/suites/test_suite_x509parse.data

@@ -202,6 +202,10 @@ X509 CRL Malformed Input (trailing spaces at end of file)
 depends_on:MBEDTLS_PEM_PARSE_C
 mbedtls_x509_crl_parse:"data_files/crl-malformed-trailing-spaces.pem":MBEDTLS_ERR_PEM_NO_HEADER_FOOTER_PRESENT
 
+X509 CRL Unsupported critical extension (issuingDistributionPoint)
+depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_SHA256_C
+mbedtls_x509_crl_parse:"data_files/crl-idp.pem":MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG
+
 X509 CSR Information RSA with MD4
 depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_MD4_C
 mbedtls_x509_csr_info:"data_files/server1.req.md4":"CSR version   \: 1\nsubject name  \: C=NL, O=PolarSSL, CN=PolarSSL Server 1\nsigned using  \: RSA with MD4\nRSA key size  \: 2048 bits\n"