From bd4315ef373ad629ebab73c16b8222fe61f6a7f2 Mon Sep 17 00:00:00 2001 From: Luca Leonardo Scorcia Date: Sun, 25 Oct 2020 09:09:02 -0400 Subject: [PATCH] KEYCLOAK-16065 Replace last UrlConnection uses with HttpClientProvider --- .../x509/CertificateValidator.java | 51 ++++++--- .../authenticators/x509}/OCSPUtils.java | 101 ++++++++---------- 2 files changed, 77 insertions(+), 75 deletions(-) rename {common/src/main/java/org/keycloak/common/util => services/src/main/java/org/keycloak/authentication/authenticators/x509}/OCSPUtils.java (88%) diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/x509/CertificateValidator.java b/services/src/main/java/org/keycloak/authentication/authenticators/x509/CertificateValidator.java index 63bec8b189..b5dfe05dfe 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/x509/CertificateValidator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/x509/CertificateValidator.java @@ -18,8 +18,12 @@ package org.keycloak.authentication.authenticators.x509; -import org.keycloak.common.util.OCSPUtils; +import org.apache.http.HttpResponse; +import org.apache.http.client.HttpClient; +import org.apache.http.client.methods.HttpGet; + import org.keycloak.common.util.Time; +import org.keycloak.connections.httpclient.HttpClientProvider; import org.keycloak.models.Constants; import org.keycloak.models.KeycloakSession; import org.keycloak.saml.common.exceptions.ProcessingException; @@ -158,10 +162,12 @@ public class CertificateValidator { public static class BouncyCastleOCSPChecker extends OCSPChecker { + private final KeycloakSession session; private final String responderUri; private final X509Certificate responderCert; - BouncyCastleOCSPChecker(String responderUri, X509Certificate responderCert) { + BouncyCastleOCSPChecker(KeycloakSession session, String responderUri, X509Certificate responderCert) { + this.session = session; this.responderUri = responderUri; this.responderCert = responderCert; } @@ -180,7 +186,7 @@ public class CertificateValidator { // 1) signed by the issuer certificate, // 2) Includes the value of OCSPsigning in ExtendedKeyUsage v3 extension // 3) Certificate is valid at the time - ocspRevocationStatus = OCSPUtils.check(cert, issuerCertificate); + ocspRevocationStatus = OCSPUtils.check(session, cert, issuerCertificate); } else { URI uri; @@ -196,7 +202,7 @@ public class CertificateValidator { // OCSP responder's certificate is assumed to be the issuer's certificate // certificate. // responderUri overrides the contents (if any) of the certificate's AIA extension - ocspRevocationStatus = OCSPUtils.check(cert, issuerCertificate, uri, responderCert, null); + ocspRevocationStatus = OCSPUtils.check(session, cert, issuerCertificate, uri, responderCert, null); } return ocspRevocationStatus; } @@ -217,10 +223,10 @@ public class CertificateValidator { private final List delegates; - public CRLListLoader(String cRLConfigValue) { + public CRLListLoader(KeycloakSession session, String cRLConfigValue) { String[] delegatePaths = Constants.CFG_DELIMITER_PATTERN.split(cRLConfigValue); this.delegates = Arrays.stream(delegatePaths) - .map(CRLFileLoader::new) + .map(cRLPath -> new CRLFileLoader(session, cRLPath)) .collect(Collectors.toList()); } @@ -237,21 +243,25 @@ public class CertificateValidator { public static class CRLFileLoader extends CRLLoaderImpl { + private final KeycloakSession session; private final String cRLPath; private final LdapContext ldapContext; - public CRLFileLoader(String cRLPath) { + public CRLFileLoader(KeycloakSession session, String cRLPath) { + this.session = session; this.cRLPath = cRLPath; ldapContext = new LdapContext(); } - public CRLFileLoader(String cRLPath, LdapContext ldapContext) { + public CRLFileLoader(KeycloakSession session, String cRLPath, LdapContext ldapContext) { + this.session = session; this.cRLPath = cRLPath; this.ldapContext = ldapContext; if (ldapContext == null) throw new NullPointerException("Context cannot be null"); } + public Collection getX509CRLs() throws GeneralSecurityException { CertificateFactory cf = CertificateFactory.getInstance("X.509"); Collection crlColl = null; @@ -287,11 +297,18 @@ public class CertificateValidator { try { logger.debugf("Loading CRL from %s", remoteURI.toString()); - URLConnection conn = remoteURI.toURL().openConnection(); - conn.setDoInput(true); - conn.setUseCaches(false); - X509CRL crl = loadFromStream(cf, conn.getInputStream()); - return Collections.singleton(crl); + HttpClient httpClient = session.getProvider(HttpClientProvider.class).getHttpClient(); + HttpGet get = new HttpGet(remoteURI); + get.setHeader("Pragma", "no-cache"); + get.setHeader("Cache-Control", "no-cache, no-store"); + HttpResponse response = httpClient.execute(get); + InputStream content = response.getEntity().getContent(); + try { + X509CRL crl = loadFromStream(cf, content); + return Collections.singleton(crl); + } finally { + content.close(); + } } catch(IOException ex) { logger.errorf(ex.getMessage()); @@ -584,7 +601,7 @@ public class CertificateValidator { } for (String dp : distributionPoints) { logger.tracef("CRL Distribution point: \"%s\"", dp); - checkRevocationStatusUsingCRL(certs, new CRLFileLoader(dp), session); + checkRevocationStatusUsingCRL(certs, new CRLFileLoader(session, dp), session); } } @@ -756,7 +773,7 @@ public class CertificateValidator { public class GotCRLDP { public GotCRLRelativePath cRLrelativePath(String value) { if (value != null) - _crlLoader = new CRLListLoader(value); + _crlLoader = new CRLListLoader(session, value); return new GotCRLRelativePath(); } @@ -809,11 +826,11 @@ public class CertificateValidator { public CertificateValidator build(X509Certificate[] certs) { if (_crlLoader == null) { - _crlLoader = new CRLFileLoader(""); + _crlLoader = new CRLFileLoader(session, ""); } return new CertificateValidator(certs, _keyUsageBits, _extendedKeyUsage, _crlCheckingEnabled, _crldpEnabled, _crlLoader, _ocspEnabled, - new BouncyCastleOCSPChecker(_responderUri, _responderCert), session); + new BouncyCastleOCSPChecker(session, _responderUri, _responderCert), session); } } diff --git a/common/src/main/java/org/keycloak/common/util/OCSPUtils.java b/services/src/main/java/org/keycloak/authentication/authenticators/x509/OCSPUtils.java similarity index 88% rename from common/src/main/java/org/keycloak/common/util/OCSPUtils.java rename to services/src/main/java/org/keycloak/authentication/authenticators/x509/OCSPUtils.java index d2e05b5dfe..cd58554348 100644 --- a/common/src/main/java/org/keycloak/common/util/OCSPUtils.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/x509/OCSPUtils.java @@ -16,7 +16,15 @@ * */ -package org.keycloak.common.util; +package org.keycloak.authentication.authenticators.x509; + +import org.apache.http.HttpHeaders; +import org.apache.http.HttpResponse; +import org.apache.http.client.HttpClient; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.entity.ByteArrayEntity; +import org.apache.http.util.EntityUtils; import org.bouncycastle.asn1.ASN1InputStream; import org.bouncycastle.asn1.ASN1Sequence; @@ -46,11 +54,13 @@ import org.bouncycastle.operator.OperatorCreationException; import org.bouncycastle.operator.bc.BcDigestCalculatorProvider; import org.bouncycastle.operator.jcajce.JcaContentVerifierProviderBuilder; -import java.io.*; +import org.keycloak.common.util.BouncyIntegration; +import org.keycloak.connections.httpclient.HttpClientProvider; +import org.keycloak.models.KeycloakSession; + +import java.io.IOException; import java.math.BigInteger; -import java.net.HttpURLConnection; import java.net.URI; -import java.net.URL; import java.security.GeneralSecurityException; import java.security.NoSuchAlgorithmException; import java.security.NoSuchProviderException; @@ -99,7 +109,7 @@ public final class OCSPUtils { * @param responderCert a certificate that OCSP responder uses to sign OCSP responses * @return revocation status */ - public static OCSPRevocationStatus check(X509Certificate cert, X509Certificate issuerCertificate, URI responderURI, X509Certificate responderCert, Date date) throws CertPathValidatorException { + public static OCSPRevocationStatus check(KeycloakSession session, X509Certificate cert, X509Certificate issuerCertificate, URI responderURI, X509Certificate responderCert, Date date) throws CertPathValidatorException { if (cert == null) throw new IllegalArgumentException("cert cannot be null"); if (issuerCertificate == null) @@ -107,7 +117,7 @@ public final class OCSPUtils { if (responderURI == null) throw new IllegalArgumentException("responderURI cannot be null"); - return check(cert, issuerCertificate, Collections.singletonList(responderURI), responderCert, date); + return check(session, cert, issuerCertificate, Collections.singletonList(responderURI), responderCert, date); } /** * Requests certificate revocation status using OCSP. The OCSP responder URI @@ -117,7 +127,7 @@ public final class OCSPUtils { * @param date * @return revocation status */ - public static OCSPRevocationStatus check(X509Certificate cert, X509Certificate issuerCertificate, Date date, X509Certificate responderCert) throws CertPathValidatorException { + public static OCSPRevocationStatus check(KeycloakSession session, X509Certificate cert, X509Certificate issuerCertificate, Date date, X509Certificate responderCert) throws CertPathValidatorException { List responderURIs = null; try { responderURIs = getResponderURIs(cert); @@ -139,7 +149,7 @@ public final class OCSPUtils { logger.log(Level.FINE, "Malformed responder URI {0}", value); } } - return check(cert, issuerCertificate, Collections.unmodifiableList(uris), responderCert, date); + return check(session, cert, issuerCertificate, Collections.unmodifiableList(uris), responderCert, date); } /** * Requests certificate revocation status using OCSP. The OCSP responder URI @@ -148,59 +158,34 @@ public final class OCSPUtils { * @param issuerCertificate The issuer certificate * @return revocation status */ - public static OCSPRevocationStatus check(X509Certificate cert, X509Certificate issuerCertificate) throws CertPathValidatorException { - return check(cert, issuerCertificate, null, null); + public static OCSPRevocationStatus check(KeycloakSession session, X509Certificate cert, X509Certificate issuerCertificate) throws CertPathValidatorException { + return check(session, cert, issuerCertificate, null, null); } - private static OCSPResp getResponse(OCSPReq ocspReq, URI responderUri) throws IOException { - DataOutputStream dataOut = null; - InputStream in = null; - try { - byte[] array = ocspReq.getEncoded(); - URL urlt = responderUri.toURL(); - HttpURLConnection con = (HttpURLConnection) urlt.openConnection(); - con.setRequestMethod("POST"); - con.setConnectTimeout(OCSP_CONNECT_TIMEOUT); - con.setReadTimeout(OCSP_CONNECT_TIMEOUT); - con.setRequestProperty("Content-type", "application/ocsp-request"); - con.setRequestProperty("Content-length", String.valueOf(array.length)); -// con.setRequestProperty("Accept", "application/ocsp-response"); + private static OCSPResp getResponse(KeycloakSession session, OCSPReq ocspReq, URI responderUri) throws IOException { + HttpClient httpClient = session.getProvider(HttpClientProvider.class).getHttpClient(); + HttpPost post = new HttpPost(responderUri); + post.setHeader(HttpHeaders.CONTENT_TYPE, "application/ocsp-request"); - con.setDoOutput(true); - con.setDoInput(true); - OutputStream out = con.getOutputStream(); - dataOut = new DataOutputStream(new BufferedOutputStream(out)); - dataOut.write(array); - dataOut.flush(); + final RequestConfig params = RequestConfig.custom() + .setConnectTimeout(OCSP_CONNECT_TIMEOUT) + .setSocketTimeout(OCSP_CONNECT_TIMEOUT) + .build(); + post.setConfig(params); - if (con.getResponseCode() / 100 != 2) { - String errorMessage = String.format("Connection error, unable to obtain certificate revocation status using OCSP responder \"%s\", code \"%d\"", - responderUri.toString(), con.getResponseCode()); - throw new IOException(errorMessage); - } - //Get Response - in = (InputStream) con.getInputStream(); - int contentLen = con.getContentLength(); - if (contentLen == -1) { - contentLen = Integer.MAX_VALUE; - } - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - int bytesRead = 0; - byte[] buffer = new byte[2048]; - while ((bytesRead = in.read(buffer, 0, buffer.length)) >= 0) { - baos.write(buffer, 0, bytesRead); - } - baos.flush(); - byte[] data = baos.toByteArray(); - return new OCSPResp(data); - } finally { - if (dataOut != null) { - dataOut.close(); - } - if (in != null) { - in.close(); - } + post.setEntity(new ByteArrayEntity(ocspReq.getEncoded())); + + //Get Response + HttpResponse response = httpClient.execute(post); + + if (response.getStatusLine().getStatusCode() / 100 != 2) { + String errorMessage = String.format("Connection error, unable to obtain certificate revocation status using OCSP responder \"%s\", code \"%d\"", + responderUri.toString(), response.getStatusLine().getStatusCode()); + throw new IOException(errorMessage); } + + byte[] data = EntityUtils.toByteArray(response.getEntity()); + return new OCSPResp(data); } /** @@ -213,7 +198,7 @@ public final class OCSPUtils { * @return a revocation status * @throws CertPathValidatorException */ - private static OCSPRevocationStatus check(X509Certificate cert, X509Certificate issuerCertificate, List responderURIs, X509Certificate responderCert, Date date) throws CertPathValidatorException { + private static OCSPRevocationStatus check(KeycloakSession session, X509Certificate cert, X509Certificate issuerCertificate, List responderURIs, X509Certificate responderCert, Date date) throws CertPathValidatorException { if (responderURIs == null || responderURIs.size() == 0) throw new IllegalArgumentException("Need at least one responder"); try { @@ -236,7 +221,7 @@ public final class OCSPUtils { logger.log(Level.INFO, "OCSP Responder {0}", responderURI); try { - OCSPResp resp = getResponse(ocspReq, 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: