diff --git a/crypto/default/src/main/java/org/keycloak/crypto/def/BCOCSPProvider.java b/crypto/default/src/main/java/org/keycloak/crypto/def/BCOCSPProvider.java index 9ce2d0affbe..4bde6faa454 100644 --- a/crypto/default/src/main/java/org/keycloak/crypto/def/BCOCSPProvider.java +++ b/crypto/default/src/main/java/org/keycloak/crypto/def/BCOCSPProvider.java @@ -53,17 +53,16 @@ import org.bouncycastle.operator.DigestCalculatorProvider; import org.bouncycastle.operator.OperatorCreationException; import org.bouncycastle.operator.jcajce.JcaContentVerifierProviderBuilder; import org.bouncycastle.operator.jcajce.JcaDigestCalculatorProviderBuilder; +import org.keycloak.jose.jwe.JWEUtils; import org.keycloak.common.util.BouncyIntegration; import org.keycloak.models.KeycloakSession; import org.keycloak.utils.OCSPProvider; import java.io.IOException; -import java.math.BigInteger; import java.net.URI; import java.security.GeneralSecurityException; import java.security.NoSuchAlgorithmException; import java.security.NoSuchProviderException; -import java.security.SecureRandom; import java.security.cert.CRLReason; import java.security.cert.CertPath; import java.security.cert.CertPathValidatorException; @@ -120,26 +119,24 @@ public class BCOCSPProvider extends OCSPProvider { JcaCertificateID certificateID = new JcaCertificateID(digCalc, issuerCertificate, cert.getSerialNumber()); - // Create a nounce extension to protect against replay attacks - SecureRandom random = SecureRandom.getInstance("SHA1PRNG"); - BigInteger nounce = BigInteger.valueOf(Math.abs(random.nextInt())); - - DEROctetString derString = new DEROctetString(nounce.toByteArray()); - Extension nounceExtension = new Extension(OCSPObjectIdentifiers.id_pkix_ocsp_nonce, false, derString); - Extensions extensions = new Extensions(nounceExtension); - - OCSPReq ocspReq = new OCSPReqBuilder().addRequest(certificateID, extensions).build(); - URI responderURI = responderURIs.get(0); - logger.log(Level.INFO, "OCSP Responder {0}", responderURI); try { + // Create a nonce extension to protect against replay attacks + DEROctetString requestNonce = new DEROctetString(new DEROctetString(JWEUtils.generateSecret(16))); + Extension nonceExtension = new Extension(OCSPObjectIdentifiers.id_pkix_ocsp_nonce, false, requestNonce); + Extensions extensions = new Extensions(nonceExtension); + + OCSPReq ocspReq = new OCSPReqBuilder().addRequest(certificateID).setRequestExtensions(extensions).build(); + + logger.log(Level.INFO, "OCSP Responder {0}", responderURI); + OCSPResp resp = getResponse(session, ocspReq, responderURI); logger.log(Level.FINE, "Received a response from OCSP responder {0}, the response status is {1}", new Object[]{responderURI, resp.getStatus()}); switch (resp.getStatus()) { case OCSPResp.SUCCESSFUL: if (resp.getResponseObject() instanceof BasicOCSPResp) { - return processBasicOCSPResponse(issuerCertificate, responderCert, date, certificateID, nounce, (BasicOCSPResp)resp.getResponseObject()); + return processBasicOCSPResponse(issuerCertificate, responderCert, date, certificateID, requestNonce, (BasicOCSPResp)resp.getResponseObject()); } else { throw new CertPathValidatorException("OCSP responder returned an invalid or unknown OCSP response."); } @@ -172,7 +169,7 @@ public class BCOCSPProvider extends OCSPProvider { } } - private OCSPRevocationStatus processBasicOCSPResponse(X509Certificate issuerCertificate, X509Certificate responderCertificate, Date date, JcaCertificateID certificateID, BigInteger nounce, BasicOCSPResp basicOcspResponse) + private OCSPRevocationStatus processBasicOCSPResponse(X509Certificate issuerCertificate, X509Certificate responderCertificate, Date date, JcaCertificateID certificateID, DEROctetString requestNonce, BasicOCSPResp basicOcspResponse) throws OCSPException, NoSuchProviderException, NoSuchAlgorithmException, CertificateNotYetValidException, CertificateExpiredException, CertPathValidatorException { SingleResp expectedResponse = null; for (SingleResp singleResponse : basicOcspResponse.getResponses()) { @@ -183,7 +180,7 @@ public class BCOCSPProvider extends OCSPProvider { } if (expectedResponse != null) { - verifyResponse(basicOcspResponse, issuerCertificate, responderCertificate, nounce.toByteArray(), date); + verifyResponse(basicOcspResponse, issuerCertificate, responderCertificate, requestNonce, date); return singleResponseToRevocationStatus(expectedResponse); } else { throw new CertPathValidatorException("OCSP response does not include a response for a certificate supplied in the OCSP request"); @@ -201,7 +198,7 @@ public class BCOCSPProvider extends OCSPProvider { idLeft.getSerialNumber().equals(idRight.getSerialNumber()); } - private void verifyResponse(BasicOCSPResp basicOcspResponse, X509Certificate issuerCertificate, X509Certificate responderCertificate, byte[] requestNonce, Date date) throws NoSuchProviderException, NoSuchAlgorithmException, CertificateNotYetValidException, CertificateExpiredException, CertPathValidatorException { + private void verifyResponse(BasicOCSPResp basicOcspResponse, X509Certificate issuerCertificate, X509Certificate responderCertificate, DEROctetString requestNonce, Date date) throws NoSuchProviderException, NoSuchAlgorithmException, CertificateNotYetValidException, CertificateExpiredException, CertPathValidatorException { List certs = new ArrayList<>(Arrays.asList(basicOcspResponse.getCerts())); X509Certificate signingCert = null; @@ -332,7 +329,10 @@ public class BCOCSPProvider extends OCSPProvider { throw new CertPathValidatorException("Error verifying OCSP Response\'s signature"); } else { Extension responseNonce = basicOcspResponse.getExtension(OCSPObjectIdentifiers.id_pkix_ocsp_nonce); - if (responseNonce != null && requestNonce != null && !Arrays.equals(requestNonce, responseNonce.getExtnValue().getOctets())) { + if (responseNonce == null && requestNonce != null) { + logger.log(Level.FINE, "No OCSP nonce in response"); + } + if (responseNonce != null && requestNonce != null && !requestNonce.equals(responseNonce.getExtnValue())) { throw new CertPathValidatorException("Nonces do not match."); } else { // See Sun's OCSP implementation. diff --git a/crypto/fips1402/src/main/java/org/keycloak/crypto/fips/BCFIPSOCSPProvider.java b/crypto/fips1402/src/main/java/org/keycloak/crypto/fips/BCFIPSOCSPProvider.java index 37f8d26a14d..36ba06fb7ca 100644 --- a/crypto/fips1402/src/main/java/org/keycloak/crypto/fips/BCFIPSOCSPProvider.java +++ b/crypto/fips1402/src/main/java/org/keycloak/crypto/fips/BCFIPSOCSPProvider.java @@ -52,17 +52,16 @@ import org.bouncycastle.operator.DigestCalculatorProvider; import org.bouncycastle.operator.OperatorCreationException; import org.bouncycastle.operator.jcajce.JcaContentVerifierProviderBuilder; import org.bouncycastle.operator.jcajce.JcaDigestCalculatorProviderBuilder; +import org.keycloak.jose.jwe.JWEUtils; import org.keycloak.common.util.BouncyIntegration; import org.keycloak.models.KeycloakSession; import org.keycloak.utils.OCSPProvider; import java.io.IOException; -import java.math.BigInteger; import java.net.URI; import java.security.GeneralSecurityException; import java.security.NoSuchAlgorithmException; import java.security.NoSuchProviderException; -import java.security.SecureRandom; import java.security.cert.CRLReason; import java.security.cert.CertPath; import java.security.cert.CertPathValidatorException; @@ -92,7 +91,7 @@ public class BCFIPSOCSPProvider extends OCSPProvider { private final static Logger logger = Logger.getLogger(BCFIPSOCSPProvider.class.getName()); - protected OCSPResp getResponse(KeycloakSession session, OCSPReq ocspReq, URI responderUri) throws IOException, InterruptedException { + protected OCSPResp getResponse(KeycloakSession session, OCSPReq ocspReq, URI responderUri) throws IOException { byte[] data = getEncodedOCSPResponse(session, ocspReq.getEncoded(), responderUri); return new OCSPResp(data); } @@ -116,28 +115,27 @@ public class BCFIPSOCSPProvider extends OCSPProvider { DigestCalculatorProvider dcp = new JcaDigestCalculatorProviderBuilder().build(); DigestCalculator digCalc = dcp.get(CertificateID.HASH_SHA1); + JcaCertificateID certificateID = new JcaCertificateID(digCalc, issuerCertificate, cert.getSerialNumber()); - // Create a nounce extension to protect against replay attacks - SecureRandom random = SecureRandom.getInstance("SHA1PRNG"); - BigInteger nounce = BigInteger.valueOf(Math.abs(random.nextInt())); - - DEROctetString derString = new DEROctetString(nounce.toByteArray()); - Extension nounceExtension = new Extension(OCSPObjectIdentifiers.id_pkix_ocsp_nonce, false, derString); - Extensions extensions = new Extensions(nounceExtension); - - OCSPReq ocspReq = new OCSPReqBuilder().addRequest(certificateID, extensions).build(); - URI responderURI = responderURIs.get(0); - logger.log(Level.INFO, "OCSP Responder {0}", responderURI); try { + // Create a nonce extension to protect against replay attacks + DEROctetString requestNonce = new DEROctetString(new DEROctetString(JWEUtils.generateSecret(16))); + Extension nonceExtension = new Extension(OCSPObjectIdentifiers.id_pkix_ocsp_nonce, false, requestNonce); + Extensions extensions = new Extensions(nonceExtension); + + OCSPReq ocspReq = new OCSPReqBuilder().addRequest(certificateID).setRequestExtensions(extensions).build(); + + logger.log(Level.INFO, "OCSP Responder {0}", responderURI); + OCSPResp resp = getResponse(session, ocspReq, responderURI); logger.log(Level.FINE, "Received a response from OCSP responder {0}, the response status is {1}", new Object[]{responderURI, resp.getStatus()}); switch (resp.getStatus()) { case OCSPResp.SUCCESSFUL: if (resp.getResponseObject() instanceof BasicOCSPResp) { - return processBasicOCSPResponse(issuerCertificate, responderCert, date, certificateID, nounce, (BasicOCSPResp)resp.getResponseObject()); + return processBasicOCSPResponse(issuerCertificate, responderCert, date, certificateID, requestNonce, (BasicOCSPResp)resp.getResponseObject()); } else { throw new CertPathValidatorException("OCSP responder returned an invalid or unknown OCSP response."); } @@ -157,7 +155,7 @@ public class BCFIPSOCSPProvider extends OCSPProvider { throw new CertPathValidatorException("OCSP request is malformed. OCSP response error: " + resp.getStatus(), (Throwable) null, (CertPath) null, -1, CertPathValidatorException.BasicReason.UNSPECIFIED); } } - catch(IOException | InterruptedException e) { + catch(IOException e) { logger.log(Level.FINE, "OCSP Responder \"{0}\" failed to return a valid OCSP response\n{1}", new Object[] {responderURI, e.getMessage()}); throw new CertPathValidatorException("OCSP check failed", e); @@ -170,7 +168,7 @@ public class BCFIPSOCSPProvider extends OCSPProvider { } } - private OCSPRevocationStatus processBasicOCSPResponse(X509Certificate issuerCertificate, X509Certificate responderCertificate, Date date, JcaCertificateID certificateID, BigInteger nounce, BasicOCSPResp basicOcspResponse) + private OCSPRevocationStatus processBasicOCSPResponse(X509Certificate issuerCertificate, X509Certificate responderCertificate, Date date, JcaCertificateID certificateID, DEROctetString requestNonce, BasicOCSPResp basicOcspResponse) throws OCSPException, NoSuchProviderException, NoSuchAlgorithmException, CertificateNotYetValidException, CertificateExpiredException, CertPathValidatorException { SingleResp expectedResponse = null; for (SingleResp singleResponse : basicOcspResponse.getResponses()) { @@ -181,7 +179,7 @@ public class BCFIPSOCSPProvider extends OCSPProvider { } if (expectedResponse != null) { - verifyResponse(basicOcspResponse, issuerCertificate, responderCertificate, nounce.toByteArray(), date); + verifyResponse(basicOcspResponse, issuerCertificate, responderCertificate, requestNonce, date); return singleResponseToRevocationStatus(expectedResponse); } else { throw new CertPathValidatorException("OCSP response does not include a response for a certificate supplied in the OCSP request"); @@ -199,7 +197,7 @@ public class BCFIPSOCSPProvider extends OCSPProvider { idLeft.getSerialNumber().equals(idRight.getSerialNumber()); } - private void verifyResponse(BasicOCSPResp basicOcspResponse, X509Certificate issuerCertificate, X509Certificate responderCertificate, byte[] requestNonce, Date date) throws NoSuchProviderException, NoSuchAlgorithmException, CertificateNotYetValidException, CertificateExpiredException, CertPathValidatorException { + private void verifyResponse(BasicOCSPResp basicOcspResponse, X509Certificate issuerCertificate, X509Certificate responderCertificate, DEROctetString requestNonce, Date date) throws NoSuchProviderException, NoSuchAlgorithmException, CertificateNotYetValidException, CertificateExpiredException, CertPathValidatorException { List certs = new ArrayList<>(Arrays.asList(basicOcspResponse.getCerts())); X509Certificate signingCert = null; @@ -330,7 +328,10 @@ public class BCFIPSOCSPProvider extends OCSPProvider { throw new CertPathValidatorException("Error verifying OCSP Response\'s signature"); } else { Extension responseNonce = basicOcspResponse.getExtension(OCSPObjectIdentifiers.id_pkix_ocsp_nonce); - if (responseNonce != null && requestNonce != null && !Arrays.equals(requestNonce, responseNonce.getExtnValue().getOctets())) { + if (responseNonce == null && requestNonce != null) { + logger.log(Level.FINE, "No OCSP nonce in response"); + } + if (responseNonce != null && requestNonce != null && !requestNonce.equals(responseNonce.getExtnValue())) { throw new CertPathValidatorException("Nonces do not match."); } else { // See Sun's OCSP implementation. diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/OcspHandler.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/OcspHandler.java index ea83612df4e..2a72b950f92 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/OcspHandler.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/OcspHandler.java @@ -30,6 +30,8 @@ import java.util.Map; import com.google.common.collect.ImmutableMap; +import org.bouncycastle.asn1.ASN1OctetString; +import org.bouncycastle.asn1.DEROctetString; import org.bouncycastle.asn1.nist.NISTObjectIdentifiers; import org.bouncycastle.asn1.ocsp.OCSPObjectIdentifiers; import org.bouncycastle.asn1.pkcs.PKCSObjectIdentifiers; @@ -105,6 +107,27 @@ final class OcspHandler implements HttpHandler { } } + private Extension checkNonce(Extension nonce) { + if (nonce == null) { + return null; + } + try { + // check the nonce value is an octet string that encapsulates another octet string + // see: https://github.com/pyca/cryptography/issues/6404 + ASN1OctetString value = nonce.getExtnValue(); + if (!(value instanceof DEROctetString)) { + return null; + } + int length = ASN1OctetString.getInstance(value.getOctets()).getOctetsLength(); + if (length < 16 || length > 32) { + return null; + } + return nonce; + } catch (Exception e) { + return null; + } + } + @Override public void handleRequest(final HttpServerExchange exchange) throws Exception { if (exchange.isInIoThread()) { @@ -120,7 +143,7 @@ final class OcspHandler implements HttpHandler { final OCSPReq request = new OCSPReq(buffy); final Req[] requested = request.getRequestList(); - final Extension nonce = request.getExtension(OCSPObjectIdentifiers.id_pkix_ocsp_nonce); + final Extension nonce = checkNonce(request.getExtension(OCSPObjectIdentifiers.id_pkix_ocsp_nonce)); final DigestCalculator sha1Calculator = new JcaDigestCalculatorProviderBuilder().build() .get(AlgorithmIdentifier.getInstance(RespID.HASH_SHA1)); @@ -142,8 +165,11 @@ final class OcspHandler implements HttpHandler { new AlgorithmIdentifier(PKCSObjectIdentifiers.sha256WithRSAEncryption), new AlgorithmIdentifier(NISTObjectIdentifiers.id_sha256)).build(privateKey); - final OCSPResp response = new OCSPRespBuilder().build(OCSPResp.SUCCESSFUL, - responseBuilder.build(contentSigner, chain, new Date())); + // nonce is mandatory for testing that the nonce is properly set + final OCSPResp response = nonce != null + ? new OCSPRespBuilder().build(OCSPResp.SUCCESSFUL, + responseBuilder.build(contentSigner, chain, new Date())) + : new OCSPRespBuilder().build(OCSPResp.MALFORMED_REQUEST, null); final byte[] responseBytes = response.getEncoded();