From 9eb400262fb60d38a1d73fbf0ba26c506b8429db Mon Sep 17 00:00:00 2001 From: Jan Lieskovsky Date: Fri, 22 Mar 2019 11:29:34 +0100 Subject: [PATCH] KEYCLOAK-6055 Include X.509 certificate data in audit logs Signed-off-by: Jan Lieskovsky Co-authored-by: mposolda --- .../java/org/keycloak/events/Details.java | 3 ++ ...actX509ClientCertificateAuthenticator.java | 24 +++++++++++++ .../x509/ValidateX509CertificateUsername.java | 3 ++ .../X509ClientCertificateAuthenticator.java | 4 +++ .../x509/AbstractX509AuthenticationTest.java | 16 +++++++-- .../testsuite/x509/X509BrowserLoginTest.java | 36 ++++++------------- .../testsuite/x509/X509DirectGrantTest.java | 13 ++++--- 7 files changed, 67 insertions(+), 32 deletions(-) diff --git a/server-spi-private/src/main/java/org/keycloak/events/Details.java b/server-spi-private/src/main/java/org/keycloak/events/Details.java index 81c82919fa..3f5326c746 100755 --- a/server-spi-private/src/main/java/org/keycloak/events/Details.java +++ b/server-spi-private/src/main/java/org/keycloak/events/Details.java @@ -70,4 +70,7 @@ public interface Details { String EXISTING_USER = "previous_user"; + String X509_CERTIFICATE_SERIAL_NUMBER = "x509_cert_serial_number"; + String X509_CERTIFICATE_SUBJECT_DISTINGUISHED_NAME = "x509_cert_subject_distinguished_name"; + String X509_CERTIFICATE_ISSUER_DISTINGUISHED_NAME = "x509_cert_issuer_distinguished_name"; } diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/x509/AbstractX509ClientCertificateAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/x509/AbstractX509ClientCertificateAuthenticator.java index b410ff88a1..346b3a2bd4 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/x509/AbstractX509ClientCertificateAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/x509/AbstractX509ClientCertificateAuthenticator.java @@ -31,6 +31,7 @@ import org.bouncycastle.asn1.x500.style.BCStyle; import org.bouncycastle.cert.jcajce.JcaX509CertificateHolder; import org.keycloak.authentication.AuthenticationFlowContext; import org.keycloak.authentication.Authenticator; +import org.keycloak.events.Details; import org.keycloak.forms.login.LoginFormsProvider; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; @@ -233,6 +234,29 @@ public abstract class AbstractX509ClientCertificateAuthenticator implements Auth } return null; } + + + // Saving some notes for audit to authSession as the event may not be necessarily triggered in this HTTP request where the certificate was parsed + // For example if there is confirmation page enabled, it will be in the additional request + protected void saveX509CertificateAuditDataToAuthSession(AuthenticationFlowContext context, + X509Certificate cert) { + context.getAuthenticationSession().setAuthNote(Details.X509_CERTIFICATE_SERIAL_NUMBER, cert.getSerialNumber().toString()); + context.getAuthenticationSession().setAuthNote(Details.X509_CERTIFICATE_SUBJECT_DISTINGUISHED_NAME, cert.getSubjectDN().toString()); + context.getAuthenticationSession().setAuthNote(Details.X509_CERTIFICATE_ISSUER_DISTINGUISHED_NAME, cert.getIssuerDN().toString()); + } + + protected void recordX509CertificateAuditDataViaContextEvent(AuthenticationFlowContext context) { + recordX509DetailFromAuthSessionToEvent(context, Details.X509_CERTIFICATE_SERIAL_NUMBER); + recordX509DetailFromAuthSessionToEvent(context, Details.X509_CERTIFICATE_SUBJECT_DISTINGUISHED_NAME); + recordX509DetailFromAuthSessionToEvent(context, Details.X509_CERTIFICATE_ISSUER_DISTINGUISHED_NAME); + } + + private void recordX509DetailFromAuthSessionToEvent(AuthenticationFlowContext context, String detailName) { + String detailValue = context.getAuthenticationSession().getAuthNote(detailName); + context.getEvent().detail(detailName, detailValue); + } + + // Purely for unit testing public UserIdentityExtractor getUserIdentityExtractor(X509AuthenticatorConfigModel config) { return UserIdentityExtractorBuilder.fromConfig(config); diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/x509/ValidateX509CertificateUsername.java b/services/src/main/java/org/keycloak/authentication/authenticators/x509/ValidateX509CertificateUsername.java index 73e2f43367..ba0491730a 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/x509/ValidateX509CertificateUsername.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/x509/ValidateX509CertificateUsername.java @@ -53,6 +53,9 @@ public class ValidateX509CertificateUsername extends AbstractX509ClientCertifica return; } + saveX509CertificateAuditDataToAuthSession(context, certs[0]); + recordX509CertificateAuditDataViaContextEvent(context); + X509AuthenticatorConfigModel config = null; if (context.getAuthenticatorConfig() != null && context.getAuthenticatorConfig().getConfig() != null) { config = new X509AuthenticatorConfigModel(context.getAuthenticatorConfig()); diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/x509/X509ClientCertificateAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/x509/X509ClientCertificateAuthenticator.java index 5ba4e3c51d..c55252fa28 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/x509/X509ClientCertificateAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/x509/X509ClientCertificateAuthenticator.java @@ -66,6 +66,9 @@ public class X509ClientCertificateAuthenticator extends AbstractX509ClientCertif return; } + saveX509CertificateAuditDataToAuthSession(context, certs[0]); + recordX509CertificateAuditDataViaContextEvent(context); + X509AuthenticatorConfigModel config = null; if (context.getAuthenticatorConfig() != null && context.getAuthenticatorConfig().getConfig() != null) { config = new X509AuthenticatorConfigModel(context.getAuthenticatorConfig()); @@ -261,6 +264,7 @@ public class X509ClientCertificateAuthenticator extends AbstractX509ClientCertif return; } if (context.getUser() != null) { + recordX509CertificateAuditDataViaContextEvent(context); context.success(); return; } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/AbstractX509AuthenticationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/AbstractX509AuthenticationTest.java index b421e88854..0c4991c88f 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/AbstractX509AuthenticationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/AbstractX509AuthenticationTest.java @@ -18,6 +18,8 @@ package org.keycloak.testsuite.x509; +import org.hamcrest.CoreMatchers; +import org.hamcrest.Matchers; import org.jboss.arquillian.graphene.page.Page; import org.jboss.logging.Logger; import org.junit.AfterClass; @@ -524,10 +526,20 @@ public abstract class AbstractX509AuthenticationTest extends AbstractTestRealmKe Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType()); Assert.assertNotNull(oauth.getCurrentQuery().get(OAuth2Constants.CODE)); - events.expectLogin() + AssertEvents.ExpectedEvent expectedEvent = events.expectLogin() .user(userId) .detail(Details.USERNAME, attemptedUsername) - .removeDetail(Details.REDIRECT_URI) + .removeDetail(Details.REDIRECT_URI); + + addX509CertificateDetails(expectedEvent) .assertEvent(); } + + + protected AssertEvents.ExpectedEvent addX509CertificateDetails(AssertEvents.ExpectedEvent expectedEvent) { + return expectedEvent + .detail(Details.X509_CERTIFICATE_SERIAL_NUMBER, Matchers.not(Matchers.isEmptyOrNullString())) + .detail(Details.X509_CERTIFICATE_SUBJECT_DISTINGUISHED_NAME, Matchers.startsWith("EMAILADDRESS=test-user@localhost")) + .detail(Details.X509_CERTIFICATE_ISSUER_DISTINGUISHED_NAME, Matchers.startsWith("EMAILADDRESS=contact@keycloak.org")); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/X509BrowserLoginTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/X509BrowserLoginTest.java index b5cf8a59c6..cee4569384 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/X509BrowserLoginTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/X509BrowserLoginTest.java @@ -19,6 +19,7 @@ package org.keycloak.testsuite.x509; import org.jboss.arquillian.drone.api.annotation.Drone; +import org.keycloak.testsuite.AssertEvents; import org.keycloak.testsuite.util.PhantomJSBrowser; import org.junit.Assert; import org.junit.Before; @@ -67,28 +68,6 @@ public class X509BrowserLoginTest extends AbstractX509AuthenticationTest { replaceDefaultWebDriver(phantomJS); } - private void login(X509AuthenticatorConfigModel config, String userId, String username, String attemptedUsername) { - AuthenticatorConfigRepresentation cfg = newConfig("x509-browser-config", config.getConfig()); - String cfgId = createConfig(browserExecution.getId(), cfg); - Assert.assertNotNull(cfgId); - - loginConfirmationPage.open(); - - Assert.assertTrue(loginConfirmationPage.getSubjectDistinguishedNameText().startsWith("EMAILADDRESS=test-user@localhost")); - Assert.assertEquals(username, loginConfirmationPage.getUsernameText()); - - loginConfirmationPage.confirm(); - - Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType()); - Assert.assertNotNull(oauth.getCurrentQuery().get(OAuth2Constants.CODE)); - - events.expectLogin() - .user(userId) - .detail(Details.USERNAME, attemptedUsername) - .removeDetail(Details.REDIRECT_URI) - .assertEvent(); - } - @Test public void loginAsUserFromCertSubjectEmail() throws Exception { @@ -376,13 +355,15 @@ public class X509BrowserLoginTest extends AbstractX509AuthenticationTest { Assert.assertThat(loginPage.getError(), containsString("X509 certificate authentication's failed.")); - events.expectLogin() + AssertEvents.ExpectedEvent expectedEvent = events.expectLogin() .user((String) null) .session((String) null) .error("user_not_found") .detail(Details.USERNAME, "test-user@localhost") .removeDetail(Details.CONSENT) - .removeDetail(Details.REDIRECT_URI) + .removeDetail(Details.REDIRECT_URI); + + addX509CertificateDetails(expectedEvent) .assertEvent(); // Continue with form based login @@ -461,10 +442,13 @@ public class X509BrowserLoginTest extends AbstractX509AuthenticationTest { // the identity. Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType()); Assert.assertNotNull(oauth.getCurrentQuery().get(OAuth2Constants.CODE)); - events.expectLogin() + + AssertEvents.ExpectedEvent expectedEvent = events.expectLogin() .user(userId) .detail(Details.USERNAME, "test-user@localhost") - .removeDetail(Details.REDIRECT_URI) + .removeDetail(Details.REDIRECT_URI); + + addX509CertificateDetails(expectedEvent) .assertEvent(); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/X509DirectGrantTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/X509DirectGrantTest.java index 526d5ac9cb..1622232df0 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/X509DirectGrantTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/x509/X509DirectGrantTest.java @@ -31,6 +31,7 @@ import org.keycloak.representations.AccessToken; import org.keycloak.representations.RefreshToken; import org.keycloak.representations.idm.AuthenticatorConfigRepresentation; import org.keycloak.representations.idm.UserRepresentation; +import org.keycloak.testsuite.AssertEvents; import org.keycloak.testsuite.util.ContainerAssume; import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.PhantomJSBrowser; @@ -173,14 +174,16 @@ public class X509DirectGrantTest extends AbstractX509AuthenticationTest { assertEquals(401, response.getStatusCode()); - events.expectLogin() + AssertEvents.ExpectedEvent expectedEvent = events.expectLogin() .user((String) null) .session((String) null) .error("invalid_user_credentials") .client("resource-owner") .removeDetail(Details.CODE_ID) .removeDetail(Details.CONSENT) - .removeDetail(Details.REDIRECT_URI) + .removeDetail(Details.REDIRECT_URI); + + addX509CertificateDetails(expectedEvent) .assertEvent(); } @@ -308,7 +311,7 @@ public class X509DirectGrantTest extends AbstractX509AuthenticationTest { AccessToken accessToken = oauth.verifyToken(response.getAccessToken()); RefreshToken refreshToken = oauth.parseRefreshToken(response.getRefreshToken()); - events.expectLogin() + AssertEvents.ExpectedEvent expectedEvent = events.expectLogin() .client(clientId) .user(userId) .session(accessToken.getSessionState()) @@ -318,7 +321,9 @@ public class X509DirectGrantTest extends AbstractX509AuthenticationTest { .detail(Details.USERNAME, login) .removeDetail(Details.CODE_ID) .removeDetail(Details.REDIRECT_URI) - .removeDetail(Details.CONSENT) + .removeDetail(Details.CONSENT); + + addX509CertificateDetails(expectedEvent) .assertEvent(); }