Skip to content

Commit

Permalink
refactoring
Browse files Browse the repository at this point in the history
  • Loading branch information
tsaarni committed Oct 30, 2024
1 parent 87eabea commit d2a004a
Show file tree
Hide file tree
Showing 13 changed files with 538 additions and 295 deletions.
33 changes: 18 additions & 15 deletions docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,7 @@ services:
keycloak:
image: quay.io/keycloak/keycloak:${KEYCLOAK_VERSION}

entrypoint: /bin/bash
command:
- -cxe
- |
/opt/keycloak/bin/kc.sh import --file /input/src/test/resources/integration-test/keycloak-realm.json
/opt/keycloak/bin/kc.sh start \
--https-certificate-file=/input/target/certs/keycloak.pem \
--https-certificate-key-file=/input/target/certs/keycloak-key.pem \
--https-trust-store-file=/input/target/certs/client-ca-truststore.p12 \
--https-trust-store-password=password \
--https-client-auth=request \
--spi-x509cert-lookup-provider=envoy \
--spi-x509cert-lookup-envoy-cert-path-verify='[[ "CN=authorized-client" ]]' \
--log-level=INFO,io.github.nordix.keycloak.services.x509:debug
#
# Notes:
#
# - Kecyloak 22 does not support
Expand All @@ -45,6 +31,22 @@ services:
#
# Keycloak 26 and newer suppport also following
# --truststore-paths=/input/target/certs/client-ca.pem
#

entrypoint: /bin/bash
command:
- -cxe
- |
/opt/keycloak/bin/kc.sh import --file /input/src/test/resources/integration-test/keycloak-realm.json
/opt/keycloak/bin/kc.sh start \
--https-certificate-file=/input/target/certs/keycloak.pem \
--https-certificate-key-file=/input/target/certs/keycloak-key.pem \
--https-trust-store-file=/input/target/certs/client-ca-truststore.p12 \
--https-trust-store-password=password \
--https-client-auth=request \
--spi-x509cert-lookup-provider=envoy \
--spi-x509cert-lookup-envoy-cert-path-verify='[[ "CN=envoy-client" ]]' \
--log-level=INFO,io.github.nordix.keycloak.services.x509:debug
environment:
- KEYCLOAK_ADMIN=admin
Expand All @@ -58,4 +60,5 @@ services:

# Expose Keycloak's HTTPS port to allow test suite to do direct requests.
ports:
- "10080:8080"
- "10443:8443"
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.security.cert.X509Certificate;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.StringTokenizer;

import javax.security.auth.x500.X500Principal;
Expand All @@ -23,54 +24,111 @@
import org.keycloak.services.x509.X509ClientCertificateLookup;

/**
* Envoy X509 client certificate lookup.
*
* Extracts the client certificate chain from the HTTP request forwarded by Envoy.
*/
public class EnvoyProxySslClientCertificateLookup implements X509ClientCertificateLookup {

private static Logger logger = Logger.getLogger(EnvoyProxySslClientCertificateLookup.class);

protected final static String XFCC_HEADER = "x-forwarded-client-cert";
protected final static String XFCC_HEADER_CERT_KEY = "Cert";
protected final static String XFCC_HEADER_CHAIN_KEY = "Chain";
protected static final String XFCC_HEADER = "x-forwarded-client-cert";
protected static final String XFCC_HEADER_CERT_KEY = "Cert";
protected static final String XFCC_HEADER_CHAIN_KEY = "Chain";

// Each element in the list is a list of subject names expected in the client certificate chain.
// <leaf certificate subject, intermediate certificate subject, ...>
private List<List<X500Principal>> validCertPaths = null;

/**
* Constructor for creating an instance of EnvoyProxySslClientCertificateLookup.
*/
public EnvoyProxySslClientCertificateLookup() {
}

/**
* Constructor for creating an instance of EnvoyProxySslClientCertificateLookup.
*
* @param validCertPaths The certificate paths to validate the client certificate chain.
*/
EnvoyProxySslClientCertificateLookup(List<List<X500Principal>> validCertPaths) {
this.validCertPaths = validCertPaths;
}

@Override
public void close() {
// Intentionally left empty.
}

@Override
public X509Certificate[] getCertificateChain(HttpRequest httpRequest) throws GeneralSecurityException {
String xfcc = httpRequest.getHttpHeaders().getRequestHeaders().getFirst(XFCC_HEADER);

// Check if client certificate path validation is enabled.
if (validCertPaths == null) {
// No certificate path validation.
// Extract the client certificate chain from the XFCC header, if present.
return extractCertificateChainFromXfcc(xfcc);
}

// Client certificate path validation is enabled.

X509Certificate[] clientChain = httpRequest.getClientCertificateChain();

if (validCertPaths.isEmpty()) {
return clientChain;
}

// Check if the request was sent over TLS.
if (clientChain == null || clientChain.length == 0) {
logger.debug("No client certificate chain found in the TLS layer.");
return null;
}

// Is request coming from Envoy?
boolean isEnvoy = checkClientCertPath(httpRequest);
X509Certificate[] clientCertificateChain = httpRequest.getClientCertificateChain();

// XFCC is not present.
if (xfcc == null) {
// Logic when path validation is enabled:
//
// Request is coming from Envoy but no XFCC header
// - Envoy should always send XFCC header if client sent a certificate.
// - Do not return Envoy's client certificate, it would allow unauthenticated clients to impersonate Envoy.
//
// Request is not coming from Envoy (fallback to TLS layer):
// - Return the client certificate chain from the TLS layer, if any.
// - Allows clients sending requests directly (bypassing Envoy) to authenticate.
return isEnvoy ? null : clientCertificateChain;
}

// XFCC is present.

// Request is coming from Envoy: extract the client certificate chain from the XFCC header.
if (isEnvoy) {
return extractCertificateChainFromXfcc(xfcc);
}

// Request contains XFCC header but is not coming from Envoy:
// - Clients sending requests directly, bypassing Envoy, should not send XFCC
// headers.
String subjectName = (clientCertificateChain != null && clientCertificateChain.length > 0)
? clientCertificateChain[0].getSubjectX500Principal().getName()
: "unknown, no client certificate";

logger.infov(
"Ignoring x-forwarded-client-cert header from client that does not match the expected certificate path. "
+ "Client: {0}, Expected paths: {1}",
subjectName, validCertPaths);

return null;
}

/**
* Extracts the client certificate chain from the HTTP request forwarded by Envoy.
*
* The Envoy XFCC header value is a comma (",") separated string.
* Each substring is an XFCC element, which holds information added by a single proxy.
* Each XFCC element is a semicolon (";") separated list of key-value pairs.
* The Envoy XFCC header value is a comma (",") separated string. Each substring is an XFCC element, which holds
* information added by a single proxy. Each XFCC element is a semicolon (";") separated list of key-value pairs.
* Each key-value pair is separated by an equal sign ("=").
*
* Example:
*
* x-forwarded-client-cert: key1="url encoded value 1";key2="url encoded value 2";...
* x-forwarded-client-cert: key1="url encoded value 1";key2="url encoded value 2";...
*
* Following keys are supported by this implementation:
*
* 1. Cert - The entire client certificate in URL encoded PEM format.
*
* 2. Chain - The entire client certificate chain (including the leaf certificate) in URL encoded PEM format.
*
* For Envoy documentation, see
Expand All @@ -79,19 +137,9 @@ public void close() {
* @param httpRequest The HTTP request forwarded by Envoy.
* @return The client certificate chain extracted from the HTTP request.
*/
@Override
public X509Certificate[] getCertificateChain(HttpRequest httpRequest) throws GeneralSecurityException {
// Before processing the XFCC header:
// 1. Check if TLS level authorization is configured.
// 2. Check if the TLS level client certificate chain matches the configured valid certificate paths.
if (validCertPaths != null && !validCertPaths.isEmpty() && !xfccAuthorized(httpRequest)) {
// Request is not coming from authorized client, fall back to the client certificate chain in the TLS layer.
logger.debug("The client certificate chain does not match the configured valid certificate paths. Falling back to the TLS layer client certificate chain.");
return httpRequest.getClientCertificateChain();
}

String xfcc = httpRequest.getHttpHeaders().getRequestHeaders().getFirst(XFCC_HEADER);
public X509Certificate[] extractCertificateChainFromXfcc(String xfcc) {
if (xfcc == null) {
logger.debug("No x-forwarded-client-cert header found.");
return null;
}

Expand All @@ -106,57 +154,55 @@ public X509Certificate[] getCertificateChain(HttpRequest httpRequest) throws Gen

X509Certificate[] certs = null;

StringTokenizer st = new StringTokenizer(xfcc, ";");
while (st.hasMoreTokens()) {
String token = st.nextToken();
int index = token.indexOf("=");
if (index != -1) {
String key = token.substring(0, index).trim();
String value = token.substring(index + 1).trim();

if (key.equals(XFCC_HEADER_CHAIN_KEY)) {
// Chain contains the entire chain including the leaf certificate so we can stop processing the header.
certs = PemUtils.decodeCertificates(decodeValue(value));
break;
} else if (key.equals(XFCC_HEADER_CERT_KEY)) {
// Cert contains only the leaf certificate. We need to continue processing the header in case Chain is present.
certs = PemUtils.decodeCertificates(decodeValue(value));
try {
StringTokenizer st = new StringTokenizer(xfcc, ";");
while (st.hasMoreTokens()) {
String token = st.nextToken();
int index = token.indexOf("=");
if (index != -1) {
String key = token.substring(0, index).trim();
String value = token.substring(index + 1).trim();

if (key.equals(XFCC_HEADER_CHAIN_KEY)) {
// Chain contains the entire chain including the leaf certificate so we can stop processing the
// header.
certs = PemUtils.decodeCertificates(decodeValue(value));
break;
} else if (key.equals(XFCC_HEADER_CERT_KEY)) {
// Cert contains only the leaf certificate. We need to continue processing the header in case
// Chain
// is present.
certs = PemUtils.decodeCertificates(decodeValue(value));
}
}
}
}

logger.debugv("Returning certificate chain with {0} certificates", certs != null ? certs.length : 0);
if (certs != null && logger.isDebugEnabled()) {
for (X509Certificate cert : certs) {
logger.debugv("Subject: {0}, Issuer: {1}", cert.getSubjectX500Principal(), cert.getIssuerX500Principal());
logger.debugv("Returning certificate chain with {0} certificates", certs != null ? certs.length : 0);
if (certs != null && logger.isDebugEnabled()) {
for (X509Certificate cert : certs) {
logger.debugv("Subject: {0}, Issuer: {1}", cert.getSubjectX500Principal(),
cert.getIssuerX500Principal());
}
}
}

return certs;
}

private boolean xfccAuthorized(HttpRequest httpRequest) {
X509Certificate[] clientChain = httpRequest.getClientCertificateChain();
if (clientChain == null || clientChain.length == 0) {
logger.debug("No client certificate chain found in the TLS layer.");
return false;
} catch (Exception e) {
logger.warnv("Failed to extract client certificate from x-forwarded-client-cert header: {0}", e.getMessage());
throw new SecurityException(
"Failed to extract client certificate from x-forwarded-client-cert header", e);
}

return isClientCertPathValid(clientChain);
return certs;
}

/**
* Validates the client certificate chain against the configured valid certificate paths.
*/
private boolean isClientCertPathValid(X509Certificate[] clientCerts) {
if (validCertPaths.isEmpty()) {
logger.debug("Skipping client certificate chain validation as no certificate paths are configured.");
return true;
}
private boolean checkClientCertPath(HttpRequest httpRequest) {
X509Certificate[] clientChain = httpRequest.getClientCertificateChain();

// Create a list of subject names from the client certificate chain.
List<X500Principal> path = new ArrayList<>();
for (X509Certificate cert : clientCerts) {
for (X509Certificate cert : clientChain) {
path.add(cert.getSubjectX500Principal());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
package io.github.nordix.keycloak.services.x509;

import java.io.IOException;
import java.util.ArrayList;
import java.util.List;

import javax.security.auth.x500.X500Principal;
Expand All @@ -21,24 +22,22 @@
import org.keycloak.services.x509.X509ClientCertificateLookupFactory;

import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JsonDeserializer;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.module.SimpleModule;


/**
* Factory for creating EnvoyProxySslClientCertificateLookup instances.
*/
public class EnvoyProxySslClientCertificateLookupFactory implements X509ClientCertificateLookupFactory {

private static Logger logger = Logger.getLogger(EnvoyProxySslClientCertificateLookupFactory.class);

private final static String PROVIDER = "envoy";
private static final String PROVIDER = "envoy";

private List<List<X500Principal>> validCertPaths;
private List<List<X500Principal>> validCertPaths = null;

@Override
public void init(Scope config) {
Expand All @@ -51,11 +50,11 @@ public void init(Scope config) {
mapper.registerModule(module);

try {
validCertPaths = mapper.readValue(pathsJson, new TypeReference<List<List<X500Principal>>>() {});
validCertPaths = mapper.readValue(pathsJson, new TypeReference<List<List<X500Principal>>>() {
});
} catch (Exception e) {
throw new RuntimeException("Failed to parse cert-paths", e);
}

}
}

Expand All @@ -66,10 +65,12 @@ public X509ClientCertificateLookup create(KeycloakSession session) {

@Override
public void postInit(KeycloakSessionFactory factory) {
// Intentionally left empty.
}

@Override
public void close() {
// Intentionally left empty.
}

@Override
Expand All @@ -79,7 +80,7 @@ public String getId() {

public class X500PrincipalDeserializer extends JsonDeserializer<X500Principal> {
@Override
public X500Principal deserialize(JsonParser p, DeserializationContext ctxt) throws IOException, JsonProcessingException {
public X500Principal deserialize(JsonParser p, DeserializationContext ctxt) throws IOException {
return new X500Principal(p.getValueAsString());
}
}
Expand Down
Loading

0 comments on commit d2a004a

Please sign in to comment.