Skip to content

Commit

Permalink
Merge pull request #39063 from sberyozkin/oidc_cert_chain_cname
Browse files Browse the repository at this point in the history
Fix the OIDC token verification failure with the inlined cert chain
  • Loading branch information
sberyozkin authored Mar 1, 2024
2 parents efd8745 + 105fc00 commit 2fa75a1
Show file tree
Hide file tree
Showing 8 changed files with 132 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -176,14 +176,32 @@ public void setIncludeClientId(boolean includeClientId) {

/**
* Configuration of the certificate chain which can be used to verify tokens.
* If the certificate chain trusstore is configured, the tokens can be verified using the certificate
* If the certificate chain truststore is configured, the tokens can be verified using the certificate
* chain inlined in the Base64-encoded format as an `x5c` header in the token itself.
* <p/>
* The certificate chain inlined in the token is verified.
* Signature of every certificate in the chain but the root certificate is verified by the next certificate in the chain.
* Thumbprint of the root certificate in the chain must match a thumbprint of one of the certificates in the truststore.
* <p/>
* Additionally, a direct trust in the leaf chain certificate which will be used to verify the token signature must
* be established.
* By default, the leaf certificate's thumbprint must match a thumbprint of one of the certificates in the truststore.
* If the truststore does not have the leaf certificate imported, then the leaf certificate must be identified by its Common
* Name.
*/
@ConfigItem
public CertificateChain certificateChain = new CertificateChain();

@ConfigGroup
public static class CertificateChain {
/**
* Common name of the leaf certificate. It must be set if the {@link #trustStoreFile} does not have
* this certificate imported.
*
*/
@ConfigItem
public Optional<String> leafCertificateName = Optional.empty();

/**
* Truststore file which keeps thumbprints of the trusted certificates.
*/
Expand Down Expand Up @@ -233,6 +251,14 @@ public Optional<String> getTrustStoreFileType() {
public void setTrustStoreFileType(Optional<String> trustStoreFileType) {
this.trustStoreFileType = trustStoreFileType;
}

public Optional<String> getLeafCertificateName() {
return leafCertificateName;
}

public void setLeafCertificateName(String leafCertificateName) {
this.leafCertificateName = Optional.of(leafCertificateName);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.security.Key;
import java.security.cert.X509Certificate;
import java.util.List;
import java.util.Optional;
import java.util.Set;

import org.jboss.logging.Logger;
Expand All @@ -12,11 +13,13 @@

import io.quarkus.oidc.OidcTenantConfig.CertificateChain;
import io.quarkus.runtime.configuration.ConfigurationException;
import io.quarkus.security.runtime.X509IdentityProvider;
import io.vertx.ext.auth.impl.CertificateHelper;

public class CertChainPublicKeyResolver implements RefreshableVerificationKeyResolver {
private static final Logger LOG = Logger.getLogger(OidcProvider.class);
final Set<String> thumbprints;
final Optional<String> expectedLeafCertificateName;

public CertChainPublicKeyResolver(CertificateChain chain) {
if (chain.trustStorePassword.isEmpty()) {
Expand All @@ -25,6 +28,7 @@ public CertChainPublicKeyResolver(CertificateChain chain) {
}
this.thumbprints = TrustStoreUtils.getTrustedCertificateThumbprints(chain.trustStoreFile.get(),
chain.trustStorePassword.get(), chain.trustStoreCertAlias, chain.getTrustStoreFileType());
this.expectedLeafCertificateName = chain.leafCertificateName;
}

@Override
Expand All @@ -37,9 +41,29 @@ public Key resolveKey(JsonWebSignature jws, List<JsonWebStructure> nestingContex
LOG.debug("Token does not have an 'x5c' certificate chain header");
return null;
}
String thumbprint = TrustStoreUtils.calculateThumprint(chain.get(0));
if (!thumbprints.contains(thumbprint)) {
throw new UnresolvableKeyException("Certificate chain thumprint is invalid");
if (chain.size() == 0) {
LOG.debug("Token 'x5c' certificate chain is empty");
return null;
}
LOG.debug("Checking a thumbprint of the root chain certificate");
String rootThumbprint = TrustStoreUtils.calculateThumprint(chain.get(chain.size() - 1));
if (!thumbprints.contains(rootThumbprint)) {
LOG.error("Thumprint of the root chain certificate is invalid");
throw new UnresolvableKeyException("Thumprint of the root chain certificate is invalid");
}
if (expectedLeafCertificateName.isEmpty()) {
LOG.debug("Checking a thumbprint of the leaf chain certificate");
String thumbprint = TrustStoreUtils.calculateThumprint(chain.get(0));
if (!thumbprints.contains(thumbprint)) {
LOG.error("Thumprint of the leaf chain certificate is invalid");
throw new UnresolvableKeyException("Thumprint of the leaf chain certificate is invalid");
}
} else {
String leafCertificateName = X509IdentityProvider.getCommonName(chain.get(0).getSubjectX500Principal());
if (!expectedLeafCertificateName.get().equals(leafCertificateName)) {
LOG.errorf("Wrong leaf certificate common name: %s", leafCertificateName);
throw new UnresolvableKeyException("Wrong leaf certificate common name");
}
}
//TODO: support revocation lists
CertificateHelper.checkValidity(chain, null);
Expand All @@ -50,6 +74,8 @@ public Key resolveKey(JsonWebSignature jws, List<JsonWebStructure> nestingContex
root.verify(root.getPublicKey());
}
return chain.get(0).getPublicKey();
} catch (UnresolvableKeyException ex) {
throw ex;
} catch (Exception ex) {
throw new UnresolvableKeyException("Invalid certificate chain", ex);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ private Set<String> extractRoles(X509Certificate certificate, Map<String, Set<St
return Set.of();
}

private static String getCommonName(X500Principal principal) {
public static String getCommonName(X500Principal principal) {
try {
LdapName ldapDN = new LdapName(principal.getName());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,14 @@ public String bearerCertificateFullChain() {
return "granted:" + identity.getRoles();
}

@Path("bearer-certificate-full-chain-root-only")
@GET
@RolesAllowed("admin")
@Produces(MediaType.APPLICATION_JSON)
public String bearerCertificateFullChainRootOnly() {
return "granted:" + identity.getRoles();
}

@Path("bearer-kid-or-chain")
@GET
@RolesAllowed("admin")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ quarkus.oidc.code-flow-user-info-github-cached-in-idtoken.credentials.client-sec
quarkus.oidc.code-flow-user-info-github-cached-in-idtoken.credentials.client-secret.method=query
quarkus.oidc.code-flow-user-info-github-cached-in-idtoken.certificate-chain.trust-store-file=truststore.p12
quarkus.oidc.code-flow-user-info-github-cached-in-idtoken.certificate-chain.trust-store-password=storepassword
quarkus.oidc.code-flow-user-info-github-cached-in-idtoken.certificate-chain.trust-store-cert-alias=fullchain


quarkus.oidc.code-flow-token-introspection.provider=github
Expand Down Expand Up @@ -134,7 +133,6 @@ quarkus.oidc.bearer-kid-or-chain.token.audience=https://service.example.com
quarkus.oidc.bearer-kid-or-chain.allow-token-introspection-cache=false
quarkus.oidc.bearer-kid-or-chain.certificate-chain.trust-store-file=truststore.p12
quarkus.oidc.bearer-kid-or-chain.certificate-chain.trust-store-password=storepassword
quarkus.oidc.bearer-kid-or-chain.certificate-chain.trust-store-cert-alias=fullchain

quarkus.oidc.bearer-id.auth-server-url=${keycloak.url}/realms/quarkus/
quarkus.oidc.bearer-id.client-id=quarkus-app
Expand All @@ -156,7 +154,6 @@ quarkus.oidc.bearer-azure.token.lifespan-grace=2147483647
quarkus.oidc.bearer-azure.token.customizer-name=azure-access-token-customizer
quarkus.oidc.bearer-azure.certificate-chain.trust-store-file=truststore.p12
quarkus.oidc.bearer-azure.certificate-chain.trust-store-password=storepassword
quarkus.oidc.bearer-azure.certificate-chain.trust-store-cert-alias=fullchain

quarkus.oidc.bearer-role-claim-path.auth-server-url=${keycloak.url}/realms/quarkus/
quarkus.oidc.bearer-role-claim-path.client-id=quarkus-app
Expand All @@ -173,7 +170,14 @@ quarkus.oidc.bearer-no-introspection.token.allow-jwt-introspection=false

quarkus.oidc.bearer-certificate-full-chain.certificate-chain.trust-store-file=truststore.p12
quarkus.oidc.bearer-certificate-full-chain.certificate-chain.trust-store-password=storepassword
quarkus.oidc.bearer-certificate-full-chain.certificate-chain.trust-store-cert-alias=fullchain

quarkus.oidc.bearer-certificate-full-chain-root-only.certificate-chain.trust-store-file=truststore-rootcert.p12
quarkus.oidc.bearer-certificate-full-chain-root-only.certificate-chain.trust-store-password=storepassword
quarkus.oidc.bearer-certificate-full-chain-root-only.certificate-chain.leaf-certificate-name=www.quarkustest.com

quarkus.oidc.bearer-certificate-full-chain-root-only-wrongcname.certificate-chain.trust-store-file=truststore-rootcert.p12
quarkus.oidc.bearer-certificate-full-chain-root-only-wrongcname.certificate-chain.trust-store-password=storepassword
quarkus.oidc.bearer-certificate-full-chain-root-only-wrongcname.certificate-chain.leaf-certificate-name=www.quarkusio.com

quarkus.oidc.bearer-key-without-kid-thumbprint.auth-server-url=${keycloak.url}/realms/quarkus/
quarkus.oidc.bearer-key-without-kid-thumbprint.discovery-enabled=false
Expand Down
Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,12 @@ public void testAccessResourceAzure() throws Exception {
wireMockServer.stubFor(WireMock.get("/auth/azure/jwk")
.withHeader("Authorization", matching("Access token: " + azureToken))
.willReturn(WireMock.aResponse().withBody(azureJwk)));
// RestAssured.given().auth().oauth2(azureToken)
// .when().get("/api/admin/bearer-azure")
// .then()
// .statusCode(200)
// .body(Matchers.equalTo(
// "Name:[email protected],Issuer:https://sts.windows.net/e7861267-92c5-4a03-bdb2-2d3e491e7831/"));
RestAssured.given().auth().oauth2(azureToken)
.when().get("/api/admin/bearer-azure")
.then()
.statusCode(200)
.body(Matchers.equalTo(
"Name:[email protected],Issuer:https://sts.windows.net/e7861267-92c5-4a03-bdb2-2d3e491e7831/"));

String accessTokenWithCert = TestUtils.createTokenWithInlinedCertChain("alice-certificate");

Expand Down Expand Up @@ -217,7 +217,7 @@ public void testAccessAdminResourceWithFullCertChain() throws Exception {
.then()
.statusCode(401);

// Send the token with the valid certificates but which are are in the wrong order in the chain
// Send the token with the valid certificates but which are in the wrong order in the chain
accessToken = getAccessTokenWithCertChain(
List.of(intermediateCert, subjectCert, rootCert),
subjectPrivateKey);
Expand Down Expand Up @@ -246,6 +246,58 @@ public void testAccessAdminResourceWithFullCertChain() throws Exception {

}

@Test
public void testFullCertChainWithOnlyRootInTruststore() throws Exception {
X509Certificate rootCert = KeyUtils.getCertificate(ResourceUtils.readResource("/ca.cert.pem"));
X509Certificate intermediateCert = KeyUtils.getCertificate(ResourceUtils.readResource("/intermediate.cert.pem"));
X509Certificate subjectCert = KeyUtils.getCertificate(ResourceUtils.readResource("/www.quarkustest.com.cert.pem"));
PrivateKey subjectPrivateKey = KeyUtils.readPrivateKey("/www.quarkustest.com.key.pem");

// Send the token with the valid certificate chain
String accessToken = getAccessTokenWithCertChain(
List.of(subjectCert, intermediateCert, rootCert),
subjectPrivateKey);

RestAssured.given().auth().oauth2(accessToken)
.when().get("/api/admin/bearer-certificate-full-chain-root-only")
.then()
.statusCode(200)
.body(Matchers.containsString("admin"));

// Send the same token to the service expecting a different leaf certificate name
RestAssured.given().auth().oauth2(accessToken)
.when().get("/api/admin/bearer-certificate-full-chain-root-only-wrongcname")
.then()
.statusCode(401);

// Send the token with the valid certificates but which are in the wrong order in the chain
accessToken = getAccessTokenWithCertChain(
List.of(intermediateCert, subjectCert, rootCert),
subjectPrivateKey);
RestAssured.given().auth().oauth2(accessToken)
.when().get("/api/admin/bearer-certificate-full-chain-root-only")
.then()
.statusCode(401);

// Send the token with the valid certificates but with the intermediate one omitted from the chain
accessToken = getAccessTokenWithCertChain(
List.of(subjectCert, rootCert),
subjectPrivateKey);
RestAssured.given().auth().oauth2(accessToken)
.when().get("/api/admin/bearer-certificate-full-chain-root-only")
.then()
.statusCode(401);

// Send the token with the only the last valid certificate
accessToken = getAccessTokenWithCertChain(
List.of(subjectCert),
subjectPrivateKey);
RestAssured.given().auth().oauth2(accessToken)
.when().get("/api/admin/bearer-certificate-full-chain-root-only")
.then()
.statusCode(401);
}

@Test
public void testAccessAdminResourceWithKidOrChain() throws Exception {
// token with a matching kid, not x5c
Expand Down

0 comments on commit 2fa75a1

Please sign in to comment.