8025708: Certificate Path Building problem with AKI serial number

Reviewed-by: xuelei, juh

src/share/classes/sun/security/provider/certpath/AdaptableX509CertSelector.java

 /*
- * Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2014, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -26,13 +26,16 @@
 package sun.security.provider.certpath;
 
 import java.io.IOException;
-import java.util.Date;
-
+import java.math.BigInteger;
 import java.security.cert.Certificate;
 import java.security.cert.X509Certificate;
 import java.security.cert.X509CertSelector;
 import java.security.cert.CertificateException;
+import java.util.Arrays;
+import java.util.Date;
 
+import sun.security.util.Debug;
+import sun.security.util.DerInputStream;
 import sun.security.util.DerOutputStream;
 import sun.security.x509.SerialNumber;
 import sun.security.x509.KeyIdentifier;
@@ -40,26 +43,27 @@ import sun.security.x509.AuthorityKeyIdentifierExtension;
 
 /**
  * An adaptable X509 certificate selector for forward certification path
- * building.
+ * building. This selector overrides the default X509CertSelector matching
+ * rules for the subjectKeyIdentifier and serialNumber criteria, and adds
+ * additional rules for certificate validity.
  *
  * @since 1.7
  */
 class AdaptableX509CertSelector extends X509CertSelector {
+
+    private static final Debug debug = Debug.getInstance("certpath");
+
     // The start date of a validity period.
     private Date startDate;
 
     // The end date of a validity period.
     private Date endDate;
 
-    // Is subject key identifier sensitive?
-    private boolean isSKIDSensitive = false;
-
-    // Is serial number sensitive?
-    private boolean isSNSensitive = false;
+    // The subject key identifier
+    private byte[] ski;
 
-    AdaptableX509CertSelector() {
-        super();
-    }
+    // The serial number
+    private BigInteger serial;
 
     /**
      * Sets the criterion of the X509Certificate validity period.
@@ -86,51 +90,70 @@ class AdaptableX509CertSelector extends X509CertSelector {
     }
 
     /**
-     * Parse the authority key identifier extension.
+     * This selector overrides the subjectKeyIdentifier matching rules of
+     * X509CertSelector, so it throws IllegalArgumentException if this method
+     * is ever called.
+     */
+    @Override
+    public void setSubjectKeyIdentifier(byte[] subjectKeyID) {
+        throw new IllegalArgumentException();
+    }
+
+    /**
+     * This selector overrides the serialNumber matching rules of
+     * X509CertSelector, so it throws IllegalArgumentException if this method
+     * is ever called.
+     */
+    @Override
+    public void setSerialNumber(BigInteger serial) {
+        throw new IllegalArgumentException();
+    }
+
+    /**
+     * Sets the subjectKeyIdentifier and serialNumber criteria from the
+     * authority key identifier extension.
      *
-     * If the keyIdentifier field of the extension is non-null, set the
-     * subjectKeyIdentifier criterion. If the authorityCertSerialNumber
-     * field is non-null, set the serialNumber criterion.
+     * The subjectKeyIdentifier criterion is set to the keyIdentifier field
+     * of the extension, or null if it is empty. The serialNumber criterion
+     * is set to the authorityCertSerialNumber field, or null if it is empty.
      *
-     * Note that we will not set the subject criterion according to the
+     * Note that we do not set the subject criterion to the
      * authorityCertIssuer field of the extension. The caller MUST set
-     * the subject criterion before call match().
+     * the subject criterion before calling match().
      *
-     * @param akidext the authorityKeyIdentifier extension
+     * @param ext the authorityKeyIdentifier extension
+     * @throws IOException if there is an error parsing the extension
      */
-    void parseAuthorityKeyIdentifierExtension(
-            AuthorityKeyIdentifierExtension akidext) throws IOException {
-        if (akidext != null) {
-            KeyIdentifier akid = (KeyIdentifier)akidext.get(
-                    AuthorityKeyIdentifierExtension.KEY_ID);
-            if (akid != null) {
-                // Do not override the previous setting for initial selection.
-                if (isSKIDSensitive || getSubjectKeyIdentifier() == null) {
-                    DerOutputStream derout = new DerOutputStream();
-                    derout.putOctetString(akid.getIdentifier());
-                    super.setSubjectKeyIdentifier(derout.toByteArray());
+    void setSkiAndSerialNumber(AuthorityKeyIdentifierExtension ext)
+        throws IOException {
 
-                    isSKIDSensitive = true;
-                }
-            }
+        ski = null;
+        serial = null;
 
-            SerialNumber asn = (SerialNumber)akidext.get(
-                    AuthorityKeyIdentifierExtension.SERIAL_NUMBER);
+        if (ext != null) {
+            KeyIdentifier akid = (KeyIdentifier)ext.get(
+                AuthorityKeyIdentifierExtension.KEY_ID);
+            if (akid != null) {
+                DerOutputStream derout = new DerOutputStream();
+                derout.putOctetString(akid.getIdentifier());
+                ski = derout.toByteArray();
+            }
+            SerialNumber asn = (SerialNumber)ext.get(
+                AuthorityKeyIdentifierExtension.SERIAL_NUMBER);
             if (asn != null) {
-                // Do not override the previous setting for initial selection.
-                if (isSNSensitive || getSerialNumber() == null) {
-                    super.setSerialNumber(asn.getNumber());
-                    isSNSensitive = true;
-                }
+                serial = asn.getNumber();
             }
-
-            // the subject criterion should be set by the caller.
+            // the subject criterion should be set by the caller
         }
     }
 
     /**
      * Decides whether a <code>Certificate</code> should be selected.
      *
+     * This method overrides the matching rules for the subjectKeyIdentifier
+     * and serialNumber criteria and adds additional rules for certificate
+     * validity.
+     *
      * For the purpose of compatibility, when a certificate is of
      * version 1 and version 2, or the certificate does not include
      * a subject key identifier extension, the selection criterion
@@ -138,12 +161,28 @@ class AdaptableX509CertSelector extends X509CertSelector {
      */
     @Override
     public boolean match(Certificate cert) {
-        if (!(cert instanceof X509Certificate)) {
+        X509Certificate xcert = (X509Certificate)cert;
+
+        // match subject key identifier
+        if (!matchSubjectKeyID(xcert)) {
             return false;
         }
 
-        X509Certificate xcert = (X509Certificate)cert;
+        // In practice, a CA may replace its root certificate and require that
+        // the existing certificate is still valid, even if the AKID extension
+        // does not match the replacement root certificate fields.
+        //
+        // Conservatively, we only support the replacement for version 1 and
+        // version 2 certificate. As for version 3, the certificate extension
+        // may contain sensitive information (for example, policies), the
+        // AKID need to be respected to seek the exact certificate in case
+        // of key or certificate abuse.
         int version = xcert.getVersion();
+        if (serial != null && version > 2) {
+            if (!serial.equals(xcert.getSerialNumber())) {
+                return false;
+            }
+        }
 
         // Check the validity period for version 1 and 2 certificate.
         if (version < 3) {
@@ -154,7 +193,6 @@ class AdaptableX509CertSelector extends X509CertSelector {
                     return false;
                 }
             }
-
             if (endDate != null) {
                 try {
                     xcert.checkValidity(endDate);
@@ -164,26 +202,50 @@ class AdaptableX509CertSelector extends X509CertSelector {
             }
         }
 
-        // If no SubjectKeyIdentifier extension, don't bother to check it.
-        if (isSKIDSensitive &&
-            (version < 3 || xcert.getExtensionValue("2.5.29.14") == null)) {
-            setSubjectKeyIdentifier(null);
-        }
 
-        // In practice, a CA may replace its root certificate and require that
-        // the existing certificate is still valid, even if the AKID extension
-        // does not match the replacement root certificate fields.
-        //
-        // Conservatively, we only support the replacement for version 1 and
-        // version 2 certificate. As for version 2, the certificate extension
-        // may contain sensitive information (for example, policies), the
-        // AKID need to be respected to seek the exact certificate in case
-        // of key or certificate abuse.
-        if (isSNSensitive && version < 3) {
-            setSerialNumber(null);
+        if (!super.match(cert)) {
+            return false;
         }
 
-        return super.match(cert);
+        return true;
+    }
+
+    /*
+     * Match on subject key identifier extension value. These matching rules
+     * are identical to X509CertSelector except that if the certificate does
+     * not have a subject key identifier extension, it returns true.
+     */
+    private boolean matchSubjectKeyID(X509Certificate xcert) {
+        if (ski == null) {
+            return true;
+        }
+        try {
+            byte[] extVal = xcert.getExtensionValue("2.5.29.14");
+            if (extVal == null) {
+                if (debug != null) {
+                    debug.println("AdaptableX509CertSelector.match: "
+                        + "no subject key ID extension");
+                }
+                return true;
+            }
+            DerInputStream in = new DerInputStream(extVal);
+            byte[] certSubjectKeyID = in.getOctetString();
+            if (certSubjectKeyID == null ||
+                    !Arrays.equals(ski, certSubjectKeyID)) {
+                if (debug != null) {
+                    debug.println("AdaptableX509CertSelector.match: "
+                        + "subject key IDs don't match");
+                }
+                return false;
+            }
+        } catch (IOException ex) {
+            if (debug != null) {
+                debug.println("AdaptableX509CertSelector.match: "
+                    + "exception in subject key ID check");
+            }
+            return false;
+        }
+        return true;
     }
 
     @Override
@@ -198,6 +260,9 @@ class AdaptableX509CertSelector extends X509CertSelector {
             copy.endDate = (Date)endDate.clone();
         }
 
+        if (ski != null) {
+            copy.ski = ski.clone();
+        }
         return copy;
     }
 }

src/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java

 /*
- * Copyright (c) 2002, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2002, 2014, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -751,9 +751,7 @@ public class DistributionPointFetcher {
          * issued. [section 5.2.1, RFC 2459]
          */
         AuthorityKeyIdentifierExtension crlAKID = crl.getAuthKeyIdExtension();
-        if (crlAKID != null) {
-            issuerSelector.parseAuthorityKeyIdentifierExtension(crlAKID);
-        }
+        issuerSelector.setSkiAndSerialNumber(crlAKID);
 
         matched = issuerSelector.match(cert);
 

src/share/classes/sun/security/provider/certpath/ForwardBuilder.java

 /*
- * Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2014, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -269,7 +269,7 @@ class ForwardBuilder extends Builder {
              */
             AuthorityKeyIdentifierExtension akidext =
                     currentState.cert.getAuthorityKeyIdentifierExtension();
-            caSelector.parseAuthorityKeyIdentifierExtension(akidext);
+            caSelector.setSkiAndSerialNumber(akidext);
 
             /*
              * check the validity period

src/share/classes/sun/security/provider/certpath/PKIXCertPathValidator.java

 /*
- * Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2014, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -103,7 +103,7 @@ public final class PKIXCertPathValidator extends CertPathValidatorSpi {
              */
             try {
                 X509CertImpl firstCertImpl = X509CertImpl.toImpl(firstCert);
-                selector.parseAuthorityKeyIdentifierExtension(
+                selector.setSkiAndSerialNumber(
                             firstCertImpl.getAuthorityKeyIdentifierExtension());
             } catch (CertificateException | IOException e) {
                 // ignore

test/java/security/cert/CertPathBuilder/akiExt/AKISerialNumber.java

+/*
+ * Copyright (c) 2014, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * @test
+ * @bug 8025708
+ * @summary make sure a PKIX CertPathBuilder can build a path when an
+ *     intermediate CA certificate contains an AKI extension with a key
+ *     identifier and no serial number and the end-entity certificate contains
+ *     an AKI extension with both a key identifier and a serial number.
+ */
+
+import java.io.ByteArrayInputStream;
+import java.security.cert.*;
+import java.util.ArrayList;
+import java.util.Base64;
+import java.util.Collections;
+
+public class AKISerialNumber {
+
+    private static final String ROOT_CERT =
+        "MIICfTCCAeagAwIBAgIBATANBgkqhkiG9w0BAQUFADB3MQ0wCwYDVQQDEwRSb290\n" +
+        "MRYwFAYDVQQLEw1UZXN0IE9yZyBVbml0MREwDwYDVQQKEwhUZXN0IE9yZzEWMBQG\n" +
+        "A1UEBxMNVGVzdCBMb2NhbGl0eTEWMBQGA1UECBMNTWFzc2FjaHVzZXR0czELMAkG\n" +
+        "A1UEBhMCVVMwHhcNMTQwMjAxMDUwMDAwWhcNMjQwMjAxMDUwMDAwWjB3MQ0wCwYD\n" +
+        "VQQDEwRSb290MRYwFAYDVQQLEw1UZXN0IE9yZyBVbml0MREwDwYDVQQKEwhUZXN0\n" +
+        "IE9yZzEWMBQGA1UEBxMNVGVzdCBMb2NhbGl0eTEWMBQGA1UECBMNTWFzc2FjaHVz\n" +
+        "ZXR0czELMAkGA1UEBhMCVVMwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAJvL\n" +
+        "cZu6Rzf9IrduEDjJxEFv5uBvUNMlIAph7NhfmFH9puPW3Ksci4a5yTCzxI9VeVf3\n" +
+        "oYZ/UrZdF+mNZmS23RUh71X5tjMO+xew196M1xNpCRLbjcZ6i4tNdZYkdRIe8ejN\n" +
+        "sbBoD7OAvPbQqTygeG4jYjK6ODofSrba3BndNoFxAgMBAAGjGTAXMBUGA1UdEwEB\n" +
+        "/wQLMAkBAf8CBH////8wDQYJKoZIhvcNAQEFBQADgYEATvCqn69pNHv0zLiZAXk7\n" +
+        "3AKwAoza0wa+1S2rVuZGfBWbV7CxmBHbgcDDbU7/I8pQVkCwOHNkVFnBgNpMuAvU\n" +
+        "aDyrHSNS/av5d1yk5WAuGX2B9mSwZdhnAvtz2fsV1q9NptdF54EkIiKtQQmTGnr9\n" +
+        "TID8CFEk/qje+AB272B1UJw=\n";
+
+    /**
+     * This certificate contains an AuthorityKeyIdentifier with only the
+     * keyIdentifier field filled in.
+     */
+    private static final String INT_CERT_WITH_KEYID_AKI =
+        "MIICqTCCAhKgAwIBAgIBAjANBgkqhkiG9w0BAQUFADB3MQ0wCwYDVQQDEwRSb290\n" +
+        "MRYwFAYDVQQLEw1UZXN0IE9yZyBVbml0MREwDwYDVQQKEwhUZXN0IE9yZzEWMBQG\n" +
+        "A1UEBxMNVGVzdCBMb2NhbGl0eTEWMBQGA1UECBMNTWFzc2FjaHVzZXR0czELMAkG\n" +
+        "A1UEBhMCVVMwHhcNMTQwMjAxMDUwMDAwWhcNMjQwMjAxMDUwMDAwWjCBhDEaMBgG\n" +
+        "A1UEAxMRSW50ZXJtZWRpYXRlIENBIDIxFjAUBgNVBAsTDVRlc3QgT3JnIFVuaXQx\n" +
+        "ETAPBgNVBAoTCFRlc3QgT3JnMRYwFAYDVQQHEw1UZXN0IExvY2FsaXR5MRYwFAYD\n" +
+        "VQQIEw1NYXNzYWNodXNldHRzMQswCQYDVQQGEwJVUzCBnzANBgkqhkiG9w0BAQEF\n" +
+        "AAOBjQAwgYkCgYEAwKTZekCqb9F9T54s2IXjkQbmLIjQamMpkUlZNrpjjNq9CpTT\n" +
+        "POkfxv2UPwzTz3Ij4XFL/kJFBLm8NUOsS5xPJ62pGoZBPw9R0iMTsTce+Fpukqnr\n" +
+        "I+8jTRaAvr0tR3pqrE6uHKg7dWYN2SsWesDia/LHhwEN38yyWtSuTTLo4hcCAwEA\n" +
+        "AaM3MDUwHwYDVR0jBBgwFoAU6gZP1pO8v7+i8gsFf1gWTf/j3PkwEgYDVR0TAQH/\n" +
+        "BAgwBgEB/wIBADANBgkqhkiG9w0BAQUFAAOBgQAQxeQruav4AqQM4gmEfrHr5hOq\n" +
+        "mB2CNJ1ZqVfpDZ8GHijncKTpjNoXzzQtV23Ge+39JHOVBNWtk+aghB3iu6xGq7Qn\n" +
+        "HlBhg9meqHFqd3igDDD/jhABL2/bEo/M9rv6saYWDFZ8nCIEE6iTLTpRRko4W2Xb\n" +
+        "DyzMzMsO1kPNrJaxRg==\n";
+
+    /**
+     * This certificate contains an AuthorityKeyIdentifier with all 3 fields
+     * (keyIdentifier, authorityCertIssuer, and authorityCertSerialNumber)
+     * filled in.
+     */
+    private static final String EE_CERT_WITH_FULL_AKI =
+        "MIIDLjCCApegAwIBAgIBAzANBgkqhkiG9w0BAQUFADCBhDEaMBgGA1UEAxMRSW50\n" +
+        "ZXJtZWRpYXRlIENBIDIxFjAUBgNVBAsTDVRlc3QgT3JnIFVuaXQxETAPBgNVBAoT\n" +
+        "CFRlc3QgT3JnMRYwFAYDVQQHEw1UZXN0IExvY2FsaXR5MRYwFAYDVQQIEw1NYXNz\n" +
+        "YWNodXNldHRzMQswCQYDVQQGEwJVUzAeFw0xNDAyMDEwNTAwMDBaFw0yNDAyMDEw\n" +
+        "NTAwMDBaMH0xEzARBgNVBAMTCkVuZCBFbnRpdHkxFjAUBgNVBAsTDVRlc3QgT3Jn\n" +
+        "IFVuaXQxETAPBgNVBAoTCFRlc3QgT3JnMRYwFAYDVQQHEw1UZXN0IExvY2FsaXR5\n" +
+        "MRYwFAYDVQQIEw1NYXNzYWNodXNldHRzMQswCQYDVQQGEwJVUzCBnzANBgkqhkiG\n" +
+        "9w0BAQEFAAOBjQAwgYkCgYEAqady46PdwlKHVP1iaP11CxVyL6cDlPjpwhHCcIUv\n" +
+        "nKHbzdamqmHebDcWVBNN/I0TLNCl3ga7n8KyygSN379fG7haU8SNjpy4IDAXM0/x\n" +
+        "mwTWNTbKfJEkSoiqx1WUy2JTzRUMhgYPguQNECPxBXAdQrthZ7wQosv6Ro2ySP9O\n" +
+        "YqsCAwEAAaOBtTCBsjCBoQYDVR0jBIGZMIGWgBQdeoKxTvlTgW2KgprD69vgHV4X\n" +
+        "kKF7pHkwdzENMAsGA1UEAxMEUm9vdDEWMBQGA1UECxMNVGVzdCBPcmcgVW5pdDER\n" +
+        "MA8GA1UEChMIVGVzdCBPcmcxFjAUBgNVBAcTDVRlc3QgTG9jYWxpdHkxFjAUBgNV\n" +
+        "BAgTDU1hc3NhY2h1c2V0dHMxCzAJBgNVBAYTAlVTggECMAwGA1UdEwEB/wQCMAAw\n" +
+        "DQYJKoZIhvcNAQEFBQADgYEAuG4mM1nLF7STQWwmceELZEl49ntapH/RVoekknmd\n" +
+        "aNzcL4XQf6BTl8KFUXuThHaukQnGIzFbSZV0hrpSQ5fTN2cSZgD4Fji+HuNURmmd\n" +
+        "+Kayl0piHyO1FSbrty0TFhlVNvzKXjmMp6Jdn42KyGOSCoROQcvUWN6xkV3Hvrei\n" +
+        "0ZE=\n";
+
+    private static Base64.Decoder b64Decoder = Base64.getMimeDecoder();
+    private static CertificateFactory cf;
+
+    public static void main(String[] args) throws Exception {
+
+        cf = CertificateFactory.getInstance("X.509");
+
+        X509Certificate rootCert = getCertFromMimeEncoding(ROOT_CERT);
+        TrustAnchor anchor = new TrustAnchor(rootCert, null);
+
+        X509Certificate eeCert = getCertFromMimeEncoding(EE_CERT_WITH_FULL_AKI);
+        X509Certificate intCert = getCertFromMimeEncoding(INT_CERT_WITH_KEYID_AKI);
+
+        X509CertSelector sel = new X509CertSelector();
+        sel.setCertificate(eeCert);
+        PKIXBuilderParameters params = new PKIXBuilderParameters
+            (Collections.singleton(anchor), sel);
+        params.setRevocationEnabled(false);
+
+        ArrayList<X509Certificate> certs = new ArrayList<>();
+        certs.add(intCert);
+        certs.add(eeCert);
+        CollectionCertStoreParameters ccsp =
+            new CollectionCertStoreParameters(certs);
+        CertStore cs = CertStore.getInstance("Collection", ccsp);
+        params.addCertStore(cs);
+
+        CertPathBuilder cpb = CertPathBuilder.getInstance("PKIX");
+        CertPathBuilderResult res = cpb.build(params);
+    }
+
+    private static X509Certificate getCertFromMimeEncoding(String encoded)
+        throws CertificateException
+    {
+        byte[] bytes = b64Decoder.decode(encoded);
+        ByteArrayInputStream stream = new ByteArrayInputStream(bytes);
+        return (X509Certificate)cf.generateCertificate(stream);
+    }
+}