From 5548eb1e00d1e441dc63129d301fa6bc57b8d1ec Mon Sep 17 00:00:00 2001 From: Vinnie Ryan Date: Tue, 19 Nov 2013 17:55:43 +0000 Subject: [PATCH] 8015571: OCSP validation fails if ocsp.responderCertSubjectName is set Reviewed-by: mullan, xuelei --- .../sun/security/provider/certpath/OCSP.java | 10 +- .../provider/certpath/OCSPRequest.java | 9 +- .../provider/certpath/OCSPResponse.java | 128 +++++++++++++----- .../provider/certpath/RevocationChecker.java | 13 +- .../sun/security/x509/X509CertImpl.java | 14 ++ 5 files changed, 122 insertions(+), 52 deletions(-) diff --git a/jdk/src/share/classes/sun/security/provider/certpath/OCSP.java b/jdk/src/share/classes/sun/security/provider/certpath/OCSP.java index 2c375a869b7..dce8fd6b934 100644 --- a/jdk/src/share/classes/sun/security/provider/certpath/OCSP.java +++ b/jdk/src/share/classes/sun/security/provider/certpath/OCSP.java @@ -129,7 +129,8 @@ public final class OCSP { ("Exception while encoding OCSPRequest", e); } OCSPResponse ocspResponse = check(Collections.singletonList(certId), - responderURI, issuerCert, null, Collections.emptyList()); + responderURI, issuerCert, null, null, + Collections.emptyList()); return (RevocationStatus)ocspResponse.getSingleResponse(certId); } @@ -176,7 +177,7 @@ public final class OCSP { ("Exception while encoding OCSPRequest", e); } OCSPResponse ocspResponse = check(Collections.singletonList(certId), - responderURI, responderCert, date, extensions); + responderURI, issuerCert, responderCert, date, extensions); return (RevocationStatus) ocspResponse.getSingleResponse(certId); } @@ -185,6 +186,7 @@ public final class OCSP { * * @param certs the CertIds to be checked * @param responderURI the URI of the OCSP responder + * @param issuerCert the issuer's certificate * @param responderCert the OCSP responder's certificate * @param date the time the validity of the OCSP responder's certificate * should be checked against. If null, the current time is used. @@ -195,6 +197,7 @@ public final class OCSP { * encoding the OCSP Request or validating the OCSP Response */ static OCSPResponse check(List certIds, URI responderURI, + X509Certificate issuerCert, X509Certificate responderCert, Date date, List extensions) throws IOException, CertPathValidatorException @@ -284,7 +287,8 @@ public final class OCSP { } // verify the response - ocspResponse.verify(certIds, responderCert, date, request.getNonce()); + ocspResponse.verify(certIds, issuerCert, responderCert, date, + request.getNonce()); return ocspResponse; } diff --git a/jdk/src/share/classes/sun/security/provider/certpath/OCSPRequest.java b/jdk/src/share/classes/sun/security/provider/certpath/OCSPRequest.java index 085424393cd..6bded97293b 100644 --- a/jdk/src/share/classes/sun/security/provider/certpath/OCSPRequest.java +++ b/jdk/src/share/classes/sun/security/provider/certpath/OCSPRequest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2013, 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 @@ -76,7 +76,8 @@ import sun.security.util.*; class OCSPRequest { - private static final boolean dump = false; + private static final Debug debug = Debug.getInstance("certpath"); + private static final boolean dump = debug != null && Debug.isOn("ocsp"); // List of request CertIds private final List certIds; @@ -138,8 +139,8 @@ class OCSPRequest { if (dump) { HexDumpEncoder hexEnc = new HexDumpEncoder(); - System.out.println("OCSPRequest bytes are... "); - System.out.println(hexEnc.encode(bytes)); + debug.println("OCSPRequest bytes...\n\n" + + hexEnc.encode(bytes) + "\n"); } return bytes; diff --git a/jdk/src/share/classes/sun/security/provider/certpath/OCSPResponse.java b/jdk/src/share/classes/sun/security/provider/certpath/OCSPResponse.java index 955d63b57f3..113983796fe 100644 --- a/jdk/src/share/classes/sun/security/provider/certpath/OCSPResponse.java +++ b/jdk/src/share/classes/sun/security/provider/certpath/OCSPResponse.java @@ -132,7 +132,7 @@ public final class OCSPResponse { private static ResponseStatus[] rsvalues = ResponseStatus.values(); private static final Debug debug = Debug.getInstance("certpath"); - private static final boolean dump = false; + private static final boolean dump = debug != null && Debug.isOn("ocsp"); private static final ObjectIdentifier OCSP_BASIC_RESPONSE_OID = ObjectIdentifier.newInternal(new int[] { 1, 3, 6, 1, 5, 5, 7, 48, 1, 1}); private static final int CERT_STATUS_GOOD = 0; @@ -177,11 +177,14 @@ public final class OCSPResponse { private final ResponseStatus responseStatus; private final Map singleResponseMap; - private final List certs; private final AlgorithmId sigAlgId; private final byte[] signature; private final byte[] tbsResponseData; private final byte[] responseNonce; + private List certs; + private X509CertImpl signerCert = null; + private X500Principal responderName = null; + private KeyIdentifier responderKeyId = null; /* * Create an OCSP response from its ASN.1 DER encoding. @@ -189,8 +192,8 @@ public final class OCSPResponse { OCSPResponse(byte[] bytes) throws IOException { if (dump) { HexDumpEncoder hexEnc = new HexDumpEncoder(); - System.out.println("OCSPResponse bytes are..."); - System.out.println(hexEnc.encode(bytes)); + debug.println("OCSPResponse bytes...\n\n" + + hexEnc.encode(bytes) + "\n"); } DerValue der = new DerValue(bytes); if (der.tag != DerValue.tag_Sequence) { @@ -213,7 +216,7 @@ public final class OCSPResponse { if (responseStatus != ResponseStatus.SUCCESSFUL) { // no need to continue, responseBytes are not set. singleResponseMap = Collections.emptyMap(); - certs = Collections.emptyList(); + certs = new ArrayList(); sigAlgId = null; signature = null; tbsResponseData = null; @@ -288,16 +291,15 @@ public final class OCSPResponse { // responderID short tag = (byte)(seq.tag & 0x1f); if (tag == NAME_TAG) { + responderName = new X500Principal(seq.getData().toByteArray()); if (debug != null) { - X500Principal responderName = - new X500Principal(seq.getData().toByteArray()); - debug.println("OCSP Responder name: " + responderName); + debug.println("Responder's name: " + responderName); } } else if (tag == KEY_TAG) { + responderKeyId = new KeyIdentifier(seq.getData().getOctetString()); if (debug != null) { - byte[] responderKey = seq.getData().getOctetString(); - debug.println("OCSP Responder key: " + - Debug.toString(responderKey)); + debug.println("Responder's key ID: " + + Debug.toString(responderKeyId.getIdentifier())); } } else { throw new IOException("Bad encoding in responderID element of " + @@ -368,18 +370,25 @@ public final class OCSPResponse { certs = new ArrayList(derCerts.length); try { for (int i = 0; i < derCerts.length; i++) { - certs.add(new X509CertImpl(derCerts[i].toByteArray())); + X509CertImpl cert = + new X509CertImpl(derCerts[i].toByteArray()); + certs.add(cert); + + if (debug != null) { + debug.println("OCSP response cert #" + (i + 1) + ": " + + cert.getSubjectX500Principal()); + } } } catch (CertificateException ce) { throw new IOException("Bad encoding in X509 Certificate", ce); } } else { - certs = Collections.emptyList(); + certs = new ArrayList(); } } - void verify(List certIds, X509Certificate responderCert, - Date date, byte[] nonce) + void verify(List certIds, X509Certificate issuerCert, + X509Certificate responderCert, Date date, byte[] nonce) throws CertPathValidatorException { switch (responseStatus) { @@ -414,22 +423,58 @@ public final class OCSPResponse { } } + // Locate the signer cert + if (signerCert == null) { + // Add the Issuing CA cert and/or Trusted Responder cert to the list + // of certs from the OCSP response + certs.add((X509CertImpl) issuerCert); + if (responderCert != null) { + certs.add((X509CertImpl) responderCert); + } - // Check whether the cert returned by the responder is trusted - if (!certs.isEmpty()) { - X509CertImpl cert = certs.get(0); - // First check if the cert matches the expected responder cert - if (cert.equals(responderCert)) { + if (responderName != null) { + for (X509CertImpl cert : certs) { + if (cert.getSubjectX500Principal().equals(responderName)) { + signerCert = cert; + break; + } + } + } else if (responderKeyId != null) { + for (X509CertImpl cert : certs) { + KeyIdentifier certKeyId = cert.getSubjectKeyId(); + if (certKeyId != null && responderKeyId.equals(certKeyId)) { + signerCert = cert; + break; + } + } + } + } + + // Check whether the signer cert returned by the responder is trusted + if (signerCert != null) { + // Check if the response is signed by the issuing CA + if (signerCert.equals(issuerCert)) { + if (debug != null) { + debug.println("OCSP response is signed by the target's " + + "Issuing CA"); + } // cert is trusted, now verify the signed response - // Next check if the cert was issued by the responder cert - // which was set locally. - } else if (cert.getIssuerX500Principal().equals( - responderCert.getSubjectX500Principal())) { + // Check if the response is signed by a trusted responder + } else if (signerCert.equals(responderCert)) { + if (debug != null) { + debug.println("OCSP response is signed by a Trusted " + + "Responder"); + } + // cert is trusted, now verify the signed response + + // Check if the response is signed by an authorized responder + } else if (signerCert.getIssuerX500Principal().equals( + issuerCert.getSubjectX500Principal())) { // Check for the OCSPSigning key purpose try { - List keyPurposes = cert.getExtendedKeyUsage(); + List keyPurposes = signerCert.getExtendedKeyUsage(); if (keyPurposes == null || !keyPurposes.contains(KP_OCSP_SIGNING_OID)) { throw new CertPathValidatorException( @@ -446,16 +491,16 @@ public final class OCSPResponse { // Check algorithm constraints specified in security property // "jdk.certpath.disabledAlgorithms". AlgorithmChecker algChecker = new AlgorithmChecker( - new TrustAnchor(responderCert, null)); + new TrustAnchor(issuerCert, null)); algChecker.init(false); - algChecker.check(cert, Collections.emptySet()); + algChecker.check(signerCert, Collections.emptySet()); // check the validity try { if (date == null) { - cert.checkValidity(); + signerCert.checkValidity(); } else { - cert.checkValidity(date); + signerCert.checkValidity(date); } } catch (CertificateException e) { throw new CertPathValidatorException( @@ -471,7 +516,7 @@ public final class OCSPResponse { // extension id-pkix-ocsp-nocheck. // Extension noCheck = - cert.getExtension(PKIXExtensions.OCSPNoCheck_Id); + signerCert.getExtension(PKIXExtensions.OCSPNoCheck_Id); if (noCheck != null) { if (debug != null) { debug.println("Responder's certificate includes " + @@ -484,12 +529,15 @@ public final class OCSPResponse { // verify the signature try { - cert.verify(responderCert.getPublicKey()); - responderCert = cert; + signerCert.verify(issuerCert.getPublicKey()); + if (debug != null) { + debug.println("OCSP response is signed by an " + + "Authorized Responder"); + } // cert is trusted, now verify the signed response } catch (GeneralSecurityException e) { - responderCert = null; + signerCert = null; } } else { throw new CertPathValidatorException( @@ -500,12 +548,12 @@ public final class OCSPResponse { // Confirm that the signed response was generated using the public // key from the trusted responder cert - if (responderCert != null) { + if (signerCert != null) { // Check algorithm constraints specified in security property // "jdk.certpath.disabledAlgorithms". - AlgorithmChecker.check(responderCert.getPublicKey(), sigAlgId); + AlgorithmChecker.check(signerCert.getPublicKey(), sigAlgId); - if (!verifySignature(responderCert)) { + if (!verifySignature(signerCert)) { throw new CertPathValidatorException( "Error verifying OCSP Response's signature"); } @@ -555,7 +603,6 @@ public final class OCSPResponse { /* * Verify the signature of the OCSP response. - * The responder's cert is implicitly trusted. */ private boolean verifySignature(X509Certificate cert) throws CertPathValidatorException { @@ -593,6 +640,13 @@ public final class OCSPResponse { return singleResponseMap.get(certId); } + /* + * Returns the certificate for the authority that signed the OCSP response. + */ + X509Certificate getSignerCertificate() { + return signerCert; // set in verify() + } + /* * A class representing a single OCSP response. */ diff --git a/jdk/src/share/classes/sun/security/provider/certpath/RevocationChecker.java b/jdk/src/share/classes/sun/security/provider/certpath/RevocationChecker.java index f34e737102a..bdd01c66439 100644 --- a/jdk/src/share/classes/sun/security/provider/certpath/RevocationChecker.java +++ b/jdk/src/share/classes/sun/security/provider/certpath/RevocationChecker.java @@ -668,9 +668,6 @@ class RevocationChecker extends PKIXRevocationChecker { throw new CertPathValidatorException(ce); } - X509Certificate respCert = (responderCert == null) ? issuerCert - : responderCert; - // The algorithm constraints of the OCSP trusted responder certificate // does not need to be checked in this code. The constraints will be // checked when the responder's certificate is validated. @@ -702,8 +699,8 @@ class RevocationChecker extends PKIXRevocationChecker { nonce = ext.getValue(); } } - response.verify(Collections.singletonList(certId), respCert, - params.date(), nonce); + response.verify(Collections.singletonList(certId), issuerCert, + responderCert, params.date(), nonce); } else { URI responderURI = (this.responderURI != null) @@ -716,8 +713,8 @@ class RevocationChecker extends PKIXRevocationChecker { } response = OCSP.check(Collections.singletonList(certId), - responderURI, respCert, null, - ocspExtensions); + responderURI, issuerCert, responderCert, + null, ocspExtensions); } } catch (IOException e) { throw new CertPathValidatorException( @@ -733,7 +730,7 @@ class RevocationChecker extends PKIXRevocationChecker { if (revocationTime.before(params.date())) { Throwable t = new CertificateRevokedException( revocationTime, rs.getRevocationReason(), - respCert.getSubjectX500Principal(), + response.getSignerCertificate().getSubjectX500Principal(), rs.getSingleExtensions()); throw new CertPathValidatorException(t.getMessage(), t, null, -1, BasicReason.REVOKED); diff --git a/jdk/src/share/classes/sun/security/x509/X509CertImpl.java b/jdk/src/share/classes/sun/security/x509/X509CertImpl.java index b96674e2e1d..b3f448b09fd 100644 --- a/jdk/src/share/classes/sun/security/x509/X509CertImpl.java +++ b/jdk/src/share/classes/sun/security/x509/X509CertImpl.java @@ -1108,6 +1108,20 @@ public class X509CertImpl extends X509Certificate implements DerEncoder { return null; } + /** + * Returns the subject's key identifier, or null + */ + public KeyIdentifier getSubjectKeyId() { + SubjectKeyIdentifierExtension ski = getSubjectKeyIdentifierExtension(); + if (ski != null) { + try { + return (KeyIdentifier)ski.get( + SubjectKeyIdentifierExtension.KEY_ID); + } catch (IOException ioe) {} // not possible + } + return null; + } + /** * Get AuthorityKeyIdentifier extension * @return AuthorityKeyIdentifier object or null (if no such object