From 6e94ec3476a279c0a130186209c50a2991ba4c84 Mon Sep 17 00:00:00 2001 From: Farah Juma Date: Mon, 20 Nov 2023 16:16:33 -0500 Subject: [PATCH 1/7] [JBEAP-26097] CVE-2023-6236: Compare the provider-url for a cached account against the provider-url required for a request to determine if a cached token can be used --- .../java/org/wildfly/security/http/oidc/Oidc.java | 14 ++++++++++++++ .../security/http/oidc/OidcCookieTokenStore.java | 4 ++-- .../security/http/oidc/OidcSessionTokenStore.java | 5 ++--- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/http/oidc/src/main/java/org/wildfly/security/http/oidc/Oidc.java b/http/oidc/src/main/java/org/wildfly/security/http/oidc/Oidc.java index 496749c97a1..319d91920aa 100644 --- a/http/oidc/src/main/java/org/wildfly/security/http/oidc/Oidc.java +++ b/http/oidc/src/main/java/org/wildfly/security/http/oidc/Oidc.java @@ -333,4 +333,18 @@ public static void logToken(String name, String token) { } } + protected static boolean checkCachedAccountMatchesRequest(OidcAccount account, OidcClientConfiguration deployment) { + if (deployment.getRealm() != null + && ! deployment.getRealm().equals(account.getOidcSecurityContext().getRealm())) { + log.debug("Account in session belongs to a different realm than for this request."); + return false; + } + if (deployment.getProviderUrl() != null + && ! deployment.getProviderUrl().equals(account.getOidcSecurityContext().getOidcClientConfiguration().getProviderUrl())) { + log.debug("Account in session belongs to a different provider than for this request."); + return false; + } + return true; + } + } diff --git a/http/oidc/src/main/java/org/wildfly/security/http/oidc/OidcCookieTokenStore.java b/http/oidc/src/main/java/org/wildfly/security/http/oidc/OidcCookieTokenStore.java index 3039192f0a7..f24fe9008f8 100644 --- a/http/oidc/src/main/java/org/wildfly/security/http/oidc/OidcCookieTokenStore.java +++ b/http/oidc/src/main/java/org/wildfly/security/http/oidc/OidcCookieTokenStore.java @@ -20,6 +20,7 @@ import static org.wildfly.security.http.oidc.ElytronMessages.log; import static org.wildfly.security.http.oidc.Oidc.OIDC_STATE_COOKIE; +import static org.wildfly.security.http.oidc.Oidc.checkCachedAccountMatchesRequest; import java.net.URISyntaxException; import java.util.List; @@ -72,8 +73,7 @@ public boolean isCached(RequestAuthenticator authenticator) { return false; } OidcAccount account = new OidcAccount(principal); - if (deployment.getRealm() != null && ! deployment.getRealm().equals(account.getOidcSecurityContext().getRealm())) { - log.debug("Account in session belongs to a different realm than for this request."); + if (! checkCachedAccountMatchesRequest(account, deployment)) { return false; } diff --git a/http/oidc/src/main/java/org/wildfly/security/http/oidc/OidcSessionTokenStore.java b/http/oidc/src/main/java/org/wildfly/security/http/oidc/OidcSessionTokenStore.java index c2e6838f7a7..cb6206177cd 100644 --- a/http/oidc/src/main/java/org/wildfly/security/http/oidc/OidcSessionTokenStore.java +++ b/http/oidc/src/main/java/org/wildfly/security/http/oidc/OidcSessionTokenStore.java @@ -19,6 +19,7 @@ package org.wildfly.security.http.oidc; import static org.wildfly.security.http.oidc.ElytronMessages.log; +import static org.wildfly.security.http.oidc.Oidc.checkCachedAccountMatchesRequest; import java.util.ArrayList; import java.util.Collection; @@ -88,9 +89,7 @@ public boolean isCached(RequestAuthenticator authenticator) { } OidcClientConfiguration deployment = httpFacade.getOidcClientConfiguration(); - - if (deployment.getRealm() != null && ! deployment.getRealm().equals(account.getOidcSecurityContext().getRealm())) { - log.debug("Account in session belongs to a different realm than for this request."); + if (! checkCachedAccountMatchesRequest(account, deployment)) { return false; } From 9e44f50d62243ed68ee2f46c24c8b39d12795722 Mon Sep 17 00:00:00 2001 From: Farah Juma Date: Tue, 21 Nov 2023 11:52:28 -0500 Subject: [PATCH 2/7] [JBEAP-26097] CVE-2023-6236: Add tests for multi-tenancy to ensure that a valid token from one tenant cannot be used to access another tenant --- .../http/oidc/KeycloakConfiguration.java | 71 +++- .../http/oidc/MultiTenantResolver.java | 67 ++++ .../security/http/oidc/OidcBaseTest.java | 78 ++++- .../wildfly/security/http/oidc/OidcTest.java | 305 +++++++++++++++++- .../http/impl/AbstractBaseHttpTest.java | 25 +- 5 files changed, 523 insertions(+), 23 deletions(-) create mode 100644 http/oidc/src/test/java/org/wildfly/security/http/oidc/MultiTenantResolver.java diff --git a/http/oidc/src/test/java/org/wildfly/security/http/oidc/KeycloakConfiguration.java b/http/oidc/src/test/java/org/wildfly/security/http/oidc/KeycloakConfiguration.java index 5dfa052ed28..9a663136b28 100644 --- a/http/oidc/src/test/java/org/wildfly/security/http/oidc/KeycloakConfiguration.java +++ b/http/oidc/src/test/java/org/wildfly/security/http/oidc/KeycloakConfiguration.java @@ -18,6 +18,9 @@ package org.wildfly.security.http.oidc; +import static org.wildfly.security.http.oidc.OidcBaseTest.TENANT1_REALM; +import static org.wildfly.security.http.oidc.OidcBaseTest.TENANT2_REALM; + import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -48,6 +51,16 @@ public class KeycloakConfiguration { private static final String BOB_PASSWORD = "bob123+"; public static final String ALLOWED_ORIGIN = "http://somehost"; + // the users below are for multi-tenancy tests specifically + public static final String TENANT1_USER = "tenant1_user"; + public static final String TENANT1_PASSWORD = "tenant1_password"; + public static final String TENANT2_USER = "tenant2_user"; + public static final String TENANT2_PASSWORD = "tenant2_password"; + public static final String CHARLIE = "charlie"; + public static final String CHARLIE_PASSWORD =" charlie123+"; + public static final String DAN = "dan"; + public static final String DAN_PASSWORD =" dan123+"; + /** * Configure RealmRepresentation as follows: *