From 9cab29408f9cdfae714a77358c437b0267ad34fb Mon Sep 17 00:00:00 2001 From: mullan Date: Wed, 19 Feb 2014 14:22:33 -0500 Subject: [PATCH] 8025708: Certificate Path Building problem with AKI serial number Reviewed-by: xuelei, juh --- .../certpath/AdaptableX509CertSelector.java | 189 ++++++++++++------ .../certpath/DistributionPointFetcher.java | 6 +- .../provider/certpath/ForwardBuilder.java | 4 +- .../certpath/PKIXCertPathValidator.java | 4 +- .../akiExt/AKISerialNumber.java | 141 +++++++++++++ 5 files changed, 274 insertions(+), 70 deletions(-) create mode 100644 test/java/security/cert/CertPathBuilder/akiExt/AKISerialNumber.java diff --git a/src/share/classes/sun/security/provider/certpath/AdaptableX509CertSelector.java b/src/share/classes/sun/security/provider/certpath/AdaptableX509CertSelector.java index 680795c3b..d05d22ff5 100644 --- a/src/share/classes/sun/security/provider/certpath/AdaptableX509CertSelector.java +++ b/src/share/classes/sun/security/provider/certpath/AdaptableX509CertSelector.java @@ -1,5 +1,5 @@ /* - * 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 Certificate 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; } } diff --git a/src/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java b/src/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java index ecf609b28..7f275fb80 100644 --- a/src/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java +++ b/src/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java @@ -1,5 +1,5 @@ /* - * 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); diff --git a/src/share/classes/sun/security/provider/certpath/ForwardBuilder.java b/src/share/classes/sun/security/provider/certpath/ForwardBuilder.java index 9523e9c46..16892c497 100644 --- a/src/share/classes/sun/security/provider/certpath/ForwardBuilder.java +++ b/src/share/classes/sun/security/provider/certpath/ForwardBuilder.java @@ -1,5 +1,5 @@ /* - * 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 diff --git a/src/share/classes/sun/security/provider/certpath/PKIXCertPathValidator.java b/src/share/classes/sun/security/provider/certpath/PKIXCertPathValidator.java index 728bc19f5..c4eaff3fe 100644 --- a/src/share/classes/sun/security/provider/certpath/PKIXCertPathValidator.java +++ b/src/share/classes/sun/security/provider/certpath/PKIXCertPathValidator.java @@ -1,5 +1,5 @@ /* - * 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 diff --git a/test/java/security/cert/CertPathBuilder/akiExt/AKISerialNumber.java b/test/java/security/cert/CertPathBuilder/akiExt/AKISerialNumber.java new file mode 100644 index 000000000..e8ab65d78 --- /dev/null +++ b/test/java/security/cert/CertPathBuilder/akiExt/AKISerialNumber.java @@ -0,0 +1,141 @@ +/* + * 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 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); + } +} -- GitLab