Improper Client Certificate Validation for OAuth/OpenID clients (#20)
Co-authored-by: Stian Thorgersen <stianst@gmail.com>
This commit is contained in:
parent
1973d0f0d4
commit
51a9712e59
5 changed files with 45 additions and 23 deletions
|
@ -130,42 +130,38 @@ public class X509ClientAuthenticator extends AbstractClientAuthenticator {
|
|||
return;
|
||||
}
|
||||
|
||||
Optional<String> matchedCertificate;
|
||||
// Testing only 1st certificate in the chain to match with configured subject
|
||||
X509Certificate certificate = certs[0];
|
||||
boolean matchedCertificate = false;
|
||||
|
||||
if (clientCfg.getAllowRegexPatternComparison()) {
|
||||
Pattern subjectDNPattern = Pattern.compile(subjectDNRegexp);
|
||||
|
||||
matchedCertificate = Arrays.stream(certs)
|
||||
.map(certificate -> certificate.getSubjectDN().getName())
|
||||
.filter(subjectdn -> subjectDNPattern.matcher(subjectdn).matches())
|
||||
.findFirst();
|
||||
String subjectdn = certificate.getSubjectDN().getName();
|
||||
matchedCertificate = subjectDNPattern.matcher(subjectdn).matches();
|
||||
} else {
|
||||
// OIDC/OAuth2 does not use regex comparison as it expects exact DN given in the format according to RFC4514. See RFC8705 for the details.
|
||||
// We allow custom OIDs attributes to be "expanded" or not expanded in the given Subject DN
|
||||
X500Principal expectedDNPrincipal = new X500Principal(subjectDNRegexp, CUSTOM_OIDS_REVERSED);
|
||||
|
||||
matchedCertificate = Arrays.stream(certs)
|
||||
.filter(certificate -> expectedDNPrincipal.getName(X500Principal.RFC2253, CUSTOM_OIDS).equals(certificate.getSubjectX500Principal().getName(X500Principal.RFC2253, CUSTOM_OIDS)))
|
||||
.map(certificate -> certificate.getSubjectDN().getName())
|
||||
.findFirst();
|
||||
matchedCertificate = (expectedDNPrincipal.getName(X500Principal.RFC2253, CUSTOM_OIDS).equals(certificate.getSubjectX500Principal().getName(X500Principal.RFC2253, CUSTOM_OIDS)));
|
||||
}
|
||||
|
||||
if (!matchedCertificate.isPresent()) {
|
||||
if (!matchedCertificate) {
|
||||
// We do quite expensive operation here, so better check the logging level beforehand.
|
||||
if (logger.isDebugEnabled()) {
|
||||
logger.debug("[X509ClientCertificateAuthenticator:authenticate] Couldn't match any certificate for expected Subject DN '" + subjectDNRegexp + "' with allow regex pattern '" + clientCfg.getAllowRegexPatternComparison() + "'.");
|
||||
logger.debug("[X509ClientCertificateAuthenticator:authenticate] Available SubjectDNs: " +
|
||||
logger.debug("[X509ClientCertificateAuthenticator:authenticate] Checked Subject DN: " + certificate.getSubjectDN().getName());
|
||||
logger.debug("[X509ClientCertificateAuthenticator:authenticate] All SubjectDNs from the certificate chain: " +
|
||||
Arrays.stream(certs)
|
||||
.map(cert -> cert.getSubjectDN().getName())
|
||||
.collect(Collectors.toList()));
|
||||
}
|
||||
context.attempted();
|
||||
return;
|
||||
} else {
|
||||
logger.debug("[X509ClientCertificateAuthenticator:authenticate] Matched " + matchedCertificate.get() + " certificate.");
|
||||
logger.debug("[X509ClientCertificateAuthenticator:authenticate] Matched " + certificate.getSubjectDN().getName() + " certificate.");
|
||||
context.success();
|
||||
}
|
||||
|
||||
context.success();
|
||||
}
|
||||
|
||||
public String getDisplayType() {
|
||||
|
|
|
@ -28,6 +28,9 @@ public class MutualTLSUtils {
|
|||
|
||||
public static final String DEFAULT_KEYSTOREPATH = System.getProperty("client.certificate.keystore");
|
||||
public static final String DEFAULT_KEYSTOREPASSWORD = System.getProperty("client.certificate.keystore.passphrase");
|
||||
|
||||
// Subject DN of the 1st certificate corresponding to private-key in DEFAULT_KEYSTOREPATH keystore
|
||||
public static final String DEFAULT_KEYSTORE_SUBJECT_DN = "EMAILADDRESS=test-user@localhost, CN=test-user@localhost, OU=Keycloak, O=Red Hat, L=Westford, ST=MA, C=US";
|
||||
public static final String DEFAULT_TRUSTSTOREPATH = System.getProperty("client.truststore");
|
||||
public static final String DEFAULT_TRUSTSTOREPASSWORD = System.getProperty("client.truststore.passphrase");
|
||||
|
||||
|
|
|
@ -616,7 +616,7 @@ public class FAPI1Test extends AbstractClientPoliciesTest {
|
|||
clientRep.setImplicitFlowEnabled(true);
|
||||
OIDCAdvancedConfigWrapper clientConfig = OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep);
|
||||
clientConfig.setRequestUris(Collections.singletonList(TestApplicationResourceUrls.clientRequestUri()));
|
||||
clientConfig.setTlsClientAuthSubjectDn("EMAILADDRESS=contact@keycloak.org, CN=Keycloak Intermediate CA, OU=Keycloak, O=Red Hat, ST=MA, C=US");
|
||||
clientConfig.setTlsClientAuthSubjectDn(MutualTLSUtils.DEFAULT_KEYSTORE_SUBJECT_DN);
|
||||
clientConfig.setAllowRegexPatternComparison(false);
|
||||
});
|
||||
ClientResource clientResource = adminClient.realm(REALM_NAME).clients().get(clientUUID);
|
||||
|
|
|
@ -370,7 +370,7 @@ public class FAPICIBATest extends AbstractClientPoliciesTest {
|
|||
clientRep.setClientAuthenticatorType(X509ClientAuthenticator.PROVIDER_ID);
|
||||
OIDCAdvancedConfigWrapper clientConfig = OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep);
|
||||
clientConfig.setRequestUris(Collections.singletonList(TestApplicationResourceUrls.clientRequestUri()));
|
||||
clientConfig.setTlsClientAuthSubjectDn("EMAILADDRESS=contact@keycloak.org, CN=Keycloak Intermediate CA, OU=Keycloak, O=Red Hat, ST=MA, C=US");
|
||||
clientConfig.setTlsClientAuthSubjectDn(MutualTLSUtils.DEFAULT_KEYSTORE_SUBJECT_DN);
|
||||
clientConfig.setAllowRegexPatternComparison(false);
|
||||
setClientAuthMethodNeutralSettings(clientRep);
|
||||
});
|
||||
|
@ -414,7 +414,7 @@ public class FAPICIBATest extends AbstractClientPoliciesTest {
|
|||
clientRep.setClientAuthenticatorType(X509ClientAuthenticator.PROVIDER_ID);
|
||||
OIDCAdvancedConfigWrapper clientConfig = OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep);
|
||||
clientConfig.setRequestUris(Collections.singletonList(TestApplicationResourceUrls.clientRequestUri()));
|
||||
clientConfig.setTlsClientAuthSubjectDn("EMAILADDRESS=contact@keycloak.org, CN=Keycloak Intermediate CA, OU=Keycloak, O=Red Hat, ST=MA, C=US");
|
||||
clientConfig.setTlsClientAuthSubjectDn(MutualTLSUtils.DEFAULT_KEYSTORE_SUBJECT_DN);
|
||||
clientConfig.setAllowRegexPatternComparison(false);
|
||||
setClientAuthMethodNeutralSettings(clientRep);
|
||||
});
|
||||
|
@ -444,7 +444,7 @@ public class FAPICIBATest extends AbstractClientPoliciesTest {
|
|||
clientRep.setClientAuthenticatorType(X509ClientAuthenticator.PROVIDER_ID);
|
||||
OIDCAdvancedConfigWrapper clientConfig = OIDCAdvancedConfigWrapper.fromClientRepresentation(clientRep);
|
||||
clientConfig.setRequestUris(Collections.singletonList(TestApplicationResourceUrls.clientRequestUri()));
|
||||
clientConfig.setTlsClientAuthSubjectDn("EMAILADDRESS=contact@keycloak.org, CN=Keycloak Intermediate CA, OU=Keycloak, O=Red Hat, ST=MA, C=US");
|
||||
clientConfig.setTlsClientAuthSubjectDn(MutualTLSUtils.DEFAULT_KEYSTORE_SUBJECT_DN);
|
||||
clientConfig.setAllowRegexPatternComparison(false);
|
||||
setClientAuthMethodNeutralSettings(clientRep);
|
||||
});
|
||||
|
|
|
@ -9,7 +9,6 @@ import java.util.List;
|
|||
import java.util.Map;
|
||||
import java.util.function.Supplier;
|
||||
|
||||
import org.apache.commons.collections.map.UnmodifiableMap;
|
||||
import org.apache.http.NameValuePair;
|
||||
import org.apache.http.client.entity.UrlEncodedFormEntity;
|
||||
import org.apache.http.client.methods.HttpPost;
|
||||
|
@ -43,11 +42,15 @@ public class MutualTLSClientTest extends AbstractTestRealmKeycloakTest {
|
|||
private static final String CLIENT_ID = "confidential-x509";
|
||||
private static final String DISABLED_CLIENT_ID = "confidential-disabled-x509";
|
||||
private static final String EXACT_SUBJECT_DN_CLIENT_ID = "confidential-subjectdn-x509";
|
||||
|
||||
private static final String ISSUER_SUBJECT_DN_CLIENT_ID = "confidential-issuer-subjectdn-x509";
|
||||
private static final String OBB_SUBJECT_DN_CLIENT_ID = "obb-subjectdn-x509";
|
||||
private static final String USER = "keycloak-user@localhost";
|
||||
private static final String PASSWORD = "password";
|
||||
private static final String REALM = "test";
|
||||
private static final String EXACT_CERTIFICATE_SUBJECT_DN = "EMAILADDRESS=contact@keycloak.org, CN=Keycloak Intermediate CA, OU=Keycloak, O=Red Hat, ST=MA, C=US";
|
||||
|
||||
// This is DN of the issuer certificate, which signed certificate corresponding to EXACT_CERTIFICATE_SUBJECT_DN. This issuer certificate is present in the client.jks keystore on the 2nd position
|
||||
private static final String ISSUER_CERTIFICATE_SUBJECT_DN = "EMAILADDRESS=contact@keycloak.org, CN=Keycloak Intermediate CA, OU=Keycloak, O=Red Hat, ST=MA, C=US";
|
||||
|
||||
@Override
|
||||
public void configureTestRealm(RealmRepresentation testRealm) {
|
||||
|
@ -68,10 +71,19 @@ public class MutualTLSClientTest extends AbstractTestRealmKeycloakTest {
|
|||
exactSubjectDNConfiguration.setRedirectUris(Arrays.asList("https://localhost:8543/auth/realms/master/app/auth"));
|
||||
exactSubjectDNConfiguration.setClientAuthenticatorType(X509ClientAuthenticator.PROVIDER_ID);
|
||||
Map<String, String> attrs = new HashMap<>();
|
||||
attrs.put(X509ClientAuthenticator.ATTR_SUBJECT_DN, EXACT_CERTIFICATE_SUBJECT_DN);
|
||||
attrs.put(X509ClientAuthenticator.ATTR_SUBJECT_DN, MutualTLSUtils.DEFAULT_KEYSTORE_SUBJECT_DN);
|
||||
attrs.put(X509ClientAuthenticator.ATTR_ALLOW_REGEX_PATTERN_COMPARISON, "false");
|
||||
exactSubjectDNConfiguration.setAttributes(attrs);
|
||||
|
||||
ClientRepresentation issuerSubjectDNConfiguration = KeycloakModelUtils.createClient(testRealm, ISSUER_SUBJECT_DN_CLIENT_ID);
|
||||
issuerSubjectDNConfiguration.setServiceAccountsEnabled(Boolean.TRUE);
|
||||
issuerSubjectDNConfiguration.setRedirectUris(Arrays.asList("https://localhost:8543/auth/realms/master/app/auth"));
|
||||
issuerSubjectDNConfiguration.setClientAuthenticatorType(X509ClientAuthenticator.PROVIDER_ID);
|
||||
attrs = new HashMap<>();
|
||||
attrs.put(X509ClientAuthenticator.ATTR_SUBJECT_DN, ISSUER_CERTIFICATE_SUBJECT_DN);
|
||||
attrs.put(X509ClientAuthenticator.ATTR_ALLOW_REGEX_PATTERN_COMPARISON, "false");
|
||||
issuerSubjectDNConfiguration.setAttributes(attrs);
|
||||
|
||||
ClientRepresentation obbSubjectDNConfiguration = KeycloakModelUtils.createClient(testRealm, OBB_SUBJECT_DN_CLIENT_ID);
|
||||
obbSubjectDNConfiguration.setServiceAccountsEnabled(Boolean.TRUE);
|
||||
obbSubjectDNConfiguration.setRedirectUris(Arrays.asList("https://localhost:8543/auth/realms/master/app/auth"));
|
||||
|
@ -109,6 +121,18 @@ public class MutualTLSClientTest extends AbstractTestRealmKeycloakTest {
|
|||
assertTokenObtained(token);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testFailedClientInvocationWithIssuerCertificateAndSubjectDN() throws Exception {
|
||||
//given
|
||||
Supplier<CloseableHttpClient> clientWithProperCertificate = MutualTLSUtils::newCloseableHttpClientWithDefaultKeyStoreAndTrustStore;
|
||||
|
||||
//when (Certificate with the client's expected subjectDN is available in the certificate chain, but not on the 1st position. Hence authentication should not be successful)
|
||||
OAuthClient.AccessTokenResponse token = loginAndGetAccessTokenResponse(ISSUER_SUBJECT_DN_CLIENT_ID, clientWithProperCertificate);
|
||||
|
||||
//then
|
||||
assertTokenNotObtained(token);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testSuccessfulClientInvocationWithClientIdInQueryParams() throws Exception {
|
||||
//given//when
|
||||
|
@ -249,7 +273,6 @@ public class MutualTLSClientTest extends AbstractTestRealmKeycloakTest {
|
|||
* It test a scenario, where we do not follow the spec and specify client_id in Query Params (for in a form).
|
||||
*/
|
||||
private OAuthClient.AccessTokenResponse getAccessTokenResponseWithQueryParams(String clientId, CloseableHttpClient client) throws Exception {
|
||||
OAuthClient.AccessTokenResponse token;// This is a very simplified version of
|
||||
HttpPost post = new HttpPost(oauth.getAccessTokenUrl() + "?client_id=" + clientId);
|
||||
List<NameValuePair> parameters = new LinkedList<>();
|
||||
parameters.add(new BasicNameValuePair(OAuth2Constants.GRANT_TYPE, OAuth2Constants.AUTHORIZATION_CODE));
|
||||
|
|
Loading…
Reference in a new issue