Skip to content

Commit

Permalink
KEYCLOAK-16065 Replace last UrlConnection uses with HttpClientProvider
Browse files Browse the repository at this point in the history
  • Loading branch information
@lscorcia @mposolda
lscorcia authored and mposolda committed Nov 20, 2020
1 parent 00ea64d commit bd4315e
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 77 deletions.
51 changes: 34 additions & 17 deletions ...s/src/main/java/org/keycloak/authentication/authenticators/x509/CertificateValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -158,10 +162,12 @@ public abstract static class CRLLoaderImpl {

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;
}
Expand All @@ -180,7 +186,7 @@ public OCSPUtils.OCSPRevocationStatus check(X509Certificate cert, X509Certificat
// 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;
Expand All @@ -196,7 +202,7 @@ public OCSPUtils.OCSPRevocationStatus check(X509Certificate cert, X509Certificat
// 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;
}
Expand All @@ -217,10 +223,10 @@ public static class CRLListLoader extends CRLLoaderImpl {

private final List<CRLLoaderImpl> 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());
}

Expand All @@ -237,21 +243,25 @@ public Collection<X509CRL> getX509CRLs() throws GeneralSecurityException {

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<X509CRL> getX509CRLs() throws GeneralSecurityException {
CertificateFactory cf = CertificateFactory.getInstance("X.509");
Collection<X509CRL> crlColl = null;
Expand Down Expand Up @@ -287,11 +297,18 @@ private Collection<X509CRL> loadFromURI(CertificateFactory cf, URI remoteURI) th
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());
Expand Down Expand Up @@ -584,7 +601,7 @@ private static void checkRevocationStatusUsingCRLDistributionPoints(X509Certific
}
for (String dp : distributionPoints) {
logger.tracef("CRL Distribution point: \"%s\"", dp);
checkRevocationStatusUsingCRL(certs, new CRLFileLoader(dp), session);
checkRevocationStatusUsingCRL(certs, new CRLFileLoader(session, dp), session);
}
}

Expand Down Expand Up @@ -756,7 +773,7 @@ public GotOCSP oCSPEnabled(boolean value) {
public class GotCRLDP {
public GotCRLRelativePath cRLrelativePath(String value) {
if (value != null)
_crlLoader = new CRLListLoader(value);
_crlLoader = new CRLListLoader(session, value);
return new GotCRLRelativePath();
}

Expand Down Expand Up @@ -809,11 +826,11 @@ public RevocationStatusCheckBuilder revocation() {

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);
}
}

Expand Down
105 changes: 45 additions & 60 deletions ...a/org/keycloak/common/util/OCSPUtils.java → ...cation/authenticators/x509/OCSPUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -46,11 +54,13 @@
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;
Expand Down Expand Up @@ -99,15 +109,15 @@ public interface OCSPRevocationStatus {
* @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)
throw new IllegalArgumentException("issuerCertificate cannot be null");
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
Expand All @@ -117,7 +127,7 @@ public static OCSPRevocationStatus check(X509Certificate cert, X509Certificate i
* @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<String> responderURIs = null;
try {
responderURIs = getResponderURIs(cert);
Expand All @@ -139,7 +149,7 @@ public static OCSPRevocationStatus check(X509Certificate cert, X509Certificate i
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
Expand All @@ -148,59 +158,34 @@ public static OCSPRevocationStatus check(X509Certificate cert, X509Certificate i
* @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");

con.setDoOutput(true);
con.setDoInput(true);
OutputStream out = con.getOutputStream();
dataOut = new DataOutputStream(new BufferedOutputStream(out));
dataOut.write(array);
dataOut.flush();

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();
}
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");

final RequestConfig params = RequestConfig.custom()
.setConnectTimeout(OCSP_CONNECT_TIMEOUT)
.setSocketTimeout(OCSP_CONNECT_TIMEOUT)
.build();
post.setConfig(params);

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);
}

/**
Expand All @@ -213,7 +198,7 @@ private static OCSPResp getResponse(OCSPReq ocspReq, URI responderUri) throws IO
* @return a revocation status
* @throws CertPathValidatorException
*/
private static OCSPRevocationStatus check(X509Certificate cert, X509Certificate issuerCertificate, List<URI> responderURIs, X509Certificate responderCert, Date date) throws CertPathValidatorException {
private static OCSPRevocationStatus check(KeycloakSession session, X509Certificate cert, X509Certificate issuerCertificate, List<URI> responderURIs, X509Certificate responderCert, Date date) throws CertPathValidatorException {
if (responderURIs == null || responderURIs.size() == 0)
throw new IllegalArgumentException("Need at least one responder");
try {
Expand All @@ -236,7 +221,7 @@ private static OCSPRevocationStatus check(X509Certificate cert, X509Certificate
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:
Expand Down

0 comments on commit bd4315e

Please sign in to comment.