diff --git a/docs/documentation/release_notes/index.adoc b/docs/documentation/release_notes/index.adoc
index 03d9b2eb6dba..754fde73f7c4 100644
--- a/docs/documentation/release_notes/index.adoc
+++ b/docs/documentation/release_notes/index.adoc
@@ -16,6 +16,9 @@ include::topics/templates/release-header.adoc[]
== {project_name_full} 25.0.0
include::topics/25_0_0.adoc[leveloffset=2]
+== {project_name_full} 24.0.5
+include::topics/24_0_5.adoc[leveloffset=2]
+
== {project_name_full} 24.0.4
include::topics/24_0_4.adoc[leveloffset=2]
diff --git a/docs/documentation/release_notes/topics/24_0_5.adoc b/docs/documentation/release_notes/topics/24_0_5.adoc
new file mode 100644
index 000000000000..9af7da2adc84
--- /dev/null
+++ b/docs/documentation/release_notes/topics/24_0_5.adoc
@@ -0,0 +1,5 @@
+= Security issue with PAR clients using client_secret_post based authentication
+
+This release contains the fix of the important security issue affecting some OIDC confidential clients using PAR (Pushed authorization request). In case you use OIDC confidential clients together
+with PAR and you use client authentication based on `client_id` and `client_secret` sent as parameters in the HTTP request body (method `client_secret_post` specified in the OIDC specification), it is
+highly encouraged to rotate the client secrets of your clients after upgrading to this version.
diff --git a/services/src/main/java/org/keycloak/jose/jws/DefaultTokenManager.java b/services/src/main/java/org/keycloak/jose/jws/DefaultTokenManager.java
index 3775084824df..99eb998ce895 100644
--- a/services/src/main/java/org/keycloak/jose/jws/DefaultTokenManager.java
+++ b/services/src/main/java/org/keycloak/jose/jws/DefaultTokenManager.java
@@ -283,6 +283,8 @@ private String getEncryptedToken(TokenCategory category, String encodedToken) {
public String cekManagementAlgorithm(TokenCategory category) {
if (category == null) return null;
switch (category) {
+ case INTERNAL:
+ return Algorithm.AES;
case ID:
case LOGOUT:
return getCekManagementAlgorithm(OIDCConfigAttributes.ID_TOKEN_ENCRYPTED_RESPONSE_ALG);
@@ -310,6 +312,8 @@ public String encryptAlgorithm(TokenCategory category) {
switch (category) {
case ID:
return getEncryptAlgorithm(OIDCConfigAttributes.ID_TOKEN_ENCRYPTED_RESPONSE_ENC, JWEConstants.A128CBC_HS256);
+ case INTERNAL:
+ return JWEConstants.A128CBC_HS256;
case LOGOUT:
return getEncryptAlgorithm(OIDCConfigAttributes.ID_TOKEN_ENCRYPTED_RESPONSE_ENC);
case AUTHORIZATION_RESPONSE:
diff --git a/services/src/main/java/org/keycloak/protocol/RestartLoginCookie.java b/services/src/main/java/org/keycloak/protocol/RestartLoginCookie.java
index d676a5121feb..9432bfee92fc 100644
--- a/services/src/main/java/org/keycloak/protocol/RestartLoginCookie.java
+++ b/services/src/main/java/org/keycloak/protocol/RestartLoginCookie.java
@@ -23,13 +23,17 @@
import org.keycloak.TokenCategory;
import org.keycloak.cookie.CookieProvider;
import org.keycloak.cookie.CookieType;
+import org.keycloak.crypto.KeyUse;
import org.keycloak.models.ClientModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.services.managers.AuthenticationSessionManager;
import org.keycloak.sessions.AuthenticationSessionModel;
import org.keycloak.sessions.RootAuthenticationSessionModel;
+import org.keycloak.util.TokenUtil;
+import javax.crypto.SecretKey;
+import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.Map;
@@ -118,7 +122,7 @@ public RestartLoginCookie(AuthenticationSessionModel authSession) {
public static void setRestartCookie(KeycloakSession session, AuthenticationSessionModel authSession) {
RestartLoginCookie restart = new RestartLoginCookie(authSession);
- String encoded = session.tokens().encode(restart);
+ String encoded = encodeAndEncrypt(session, restart);
session.getProvider(CookieProvider.class).set(CookieType.AUTH_RESTART, encoded);
}
@@ -138,7 +142,7 @@ public static String getRestartCookie(KeycloakSession session){
public static AuthenticationSessionModel restartSession(KeycloakSession session, RealmModel realm,
RootAuthenticationSessionModel rootSession, String expectedClientId,
String encodedCookie) throws Exception {
- RestartLoginCookie cookie = session.tokens().decode(encodedCookie, RestartLoginCookie.class);
+ RestartLoginCookie cookie = decryptAndDecode(session, encodedCookie);
if (cookie == null) {
logger.debug("Failed to verify encoded RestartLoginCookie");
return null;
@@ -169,6 +173,36 @@ public static AuthenticationSessionModel restartSession(KeycloakSession session,
return authSession;
}
+ private static RestartLoginCookie decryptAndDecode(KeycloakSession session, String encodedToken) {
+ try {
+ String sigAlgorithm = session.tokens().signatureAlgorithm(TokenCategory.INTERNAL);
+ String algAlgorithm = session.tokens().cekManagementAlgorithm(TokenCategory.INTERNAL);
+ SecretKey encKey = session.keys().getActiveKey(session.getContext().getRealm(), KeyUse.ENC, algAlgorithm).getSecretKey();
+ SecretKey signKey = session.keys().getActiveKey(session.getContext().getRealm(), KeyUse.SIG, sigAlgorithm).getSecretKey();
+
+ byte[] contentBytes = TokenUtil.jweDirectVerifyAndDecode(encKey, signKey, encodedToken);
+ String jwt = new String(contentBytes, StandardCharsets.UTF_8);
+ return session.tokens().decode(jwt, RestartLoginCookie.class);
+ } catch (Exception e) {
+ // Might be the cookie from the older version
+ return session.tokens().decode(encodedToken, RestartLoginCookie.class);
+ }
+ }
+
+ private static String encodeAndEncrypt(KeycloakSession session, RestartLoginCookie cookie) {
+ try {
+ String sigAlgorithm = session.tokens().signatureAlgorithm(cookie.getCategory());
+ String algAlgorithm = session.tokens().cekManagementAlgorithm(cookie.getCategory());
+ SecretKey encKey = session.keys().getActiveKey(session.getContext().getRealm(), KeyUse.ENC, algAlgorithm).getSecretKey();
+ SecretKey signKey = session.keys().getActiveKey(session.getContext().getRealm(), KeyUse.SIG, sigAlgorithm).getSecretKey();
+
+ String encodedJwt = session.tokens().encode(cookie);
+ return TokenUtil.jweDirectEncode(encKey, signKey, encodedJwt.getBytes(StandardCharsets.UTF_8));
+ } catch (Exception e) {
+ throw new RuntimeException("Error encoding cookie.", e);
+ }
+ }
+
@Override
public TokenCategory getCategory() {
return TokenCategory.INTERNAL;
diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/request/AuthzEndpointRequestParser.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/request/AuthzEndpointRequestParser.java
index 8f696ea0ba7a..246d40978383 100644
--- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/request/AuthzEndpointRequestParser.java
+++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/request/AuthzEndpointRequestParser.java
@@ -73,6 +73,11 @@ public abstract class AuthzEndpointRequestParser {
// https://tools.ietf.org/html/rfc7636#section-6.1
KNOWN_REQ_PARAMS.add(OIDCLoginProtocol.CODE_CHALLENGE_PARAM);
KNOWN_REQ_PARAMS.add(OIDCLoginProtocol.CODE_CHALLENGE_METHOD_PARAM);
+
+ // Those are not OAuth/OIDC parameters, but they should never be added to the additionalRequestParameters
+ KNOWN_REQ_PARAMS.add(OAuth2Constants.CLIENT_ASSERTION_TYPE);
+ KNOWN_REQ_PARAMS.add(OAuth2Constants.CLIENT_ASSERTION);
+ KNOWN_REQ_PARAMS.add(OAuth2Constants.CLIENT_SECRET);
}
public void parseRequest(AuthorizationEndpointRequest request) {
diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/OAuthClient.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/OAuthClient.java
index cb5901161fb6..7e3c9cec6070 100644
--- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/OAuthClient.java
+++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/OAuthClient.java
@@ -1202,10 +1202,10 @@ public ParResponse doPushedAuthorizationRequest(String clientId, String clientSe
parameters.add(new BasicNameValuePair(OIDCLoginProtocol.RESPONSE_MODE_PARAM, responseMode));
}
if (clientId != null && clientSecret != null) {
- String authorization = BasicAuthHelper.createHeader(clientId, clientSecret);
- post.setHeader("Authorization", authorization);
+ parameters.add(new BasicNameValuePair(OAuth2Constants.CLIENT_ID, clientId));
+ parameters.add(new BasicNameValuePair(OAuth2Constants.CLIENT_SECRET, clientSecret));
}
- if (clientId != null) {
+ else if (clientId != null) {
parameters.add(new BasicNameValuePair(OAuth2Constants.CLIENT_ID, clientId));
}
if (redirectUri != null) {
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/RestartCookieTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/RestartCookieTest.java
index b9d783fc1a06..f80ee8bf7a9d 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/RestartCookieTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/RestartCookieTest.java
@@ -18,12 +18,20 @@
package org.keycloak.testsuite.forms;
import jakarta.ws.rs.core.Response;
+
import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.HashSet;
+import java.util.Set;
+
import org.jboss.arquillian.graphene.page.Page;
import org.junit.Rule;
import org.junit.Test;
+import org.keycloak.OAuth2Constants;
+import org.keycloak.TokenCategory;
import org.keycloak.common.util.MultivaluedHashMap;
import org.keycloak.crypto.Algorithm;
+import org.keycloak.crypto.KeyUse;
import org.keycloak.events.Details;
import org.keycloak.events.Errors;
import org.keycloak.jose.jws.JWSBuilder;
@@ -32,17 +40,26 @@
import org.keycloak.keys.KeyProvider;
import org.keycloak.models.KeyManager;
import org.keycloak.models.KeycloakSession;
+import org.keycloak.models.ParConfig;
import org.keycloak.models.RealmModel;
import org.keycloak.models.utils.DefaultKeyProviders;
import org.keycloak.protocol.RestartLoginCookie;
+import org.keycloak.protocol.oidc.endpoints.AuthorizationEndpoint;
import org.keycloak.representations.idm.ComponentRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.testsuite.AbstractTestRealmKeycloakTest;
import org.keycloak.testsuite.Assert;
import org.keycloak.testsuite.AssertEvents;
import org.keycloak.testsuite.pages.LoginPage;
+import org.keycloak.testsuite.util.ClientBuilder;
+import org.keycloak.testsuite.util.OAuthClient;
+import org.keycloak.util.TokenUtil;
import org.openqa.selenium.Cookie;
+import javax.crypto.SecretKey;
+
+import static org.junit.Assert.assertEquals;
+
/**
* @author Marek Posolda
*/
@@ -76,6 +93,16 @@ public class RestartCookieTest extends AbstractTestRealmKeycloakTest {
" }\n" +
"}";
+ public static final Set sensitiveNotes = new HashSet<>();
+ static {
+ sensitiveNotes.add(OAuth2Constants.CLIENT_ASSERTION_TYPE);
+ sensitiveNotes.add(OAuth2Constants.CLIENT_ASSERTION);
+ sensitiveNotes.add(OAuth2Constants.CLIENT_SECRET);
+ sensitiveNotes.add(AuthorizationEndpoint.LOGIN_SESSION_NOTE_ADDITIONAL_REQ_PARAMS_PREFIX + OAuth2Constants.CLIENT_ASSERTION_TYPE);
+ sensitiveNotes.add(AuthorizationEndpoint.LOGIN_SESSION_NOTE_ADDITIONAL_REQ_PARAMS_PREFIX + OAuth2Constants.CLIENT_ASSERTION);
+ sensitiveNotes.add(AuthorizationEndpoint.LOGIN_SESSION_NOTE_ADDITIONAL_REQ_PARAMS_PREFIX + OAuth2Constants.CLIENT_SECRET);
+ }
+
@Override
public void configureTestRealm(RealmRepresentation testRealm) {
}
@@ -99,6 +126,67 @@ protected void afterAbstractKeycloakTestRealmImport() {
}
}
+ @Test
+ public void testRestartCookie() {
+ loginPage.open();
+ String restartCookie = loginPage.getDriver().manage().getCookieNamed(RestartLoginCookie.KC_RESTART).getValue();
+ assertRestartCookie(restartCookie);
+ }
+
+ @Test
+ public void testRestartCookieWithPar() {
+ String clientId = "par-confidential-client";
+ adminClient.realm("test").clients().create(ClientBuilder.create()
+ .clientId("par-confidential-client")
+ .secret("secret")
+ .redirectUris(oauth.getRedirectUri() + "/*")
+ .attribute(ParConfig.REQUIRE_PUSHED_AUTHORIZATION_REQUESTS, "true")
+ .build());
+
+ oauth.clientId(clientId);
+ String requestUri = null;
+ try {
+ OAuthClient.ParResponse pResp = oauth.doPushedAuthorizationRequest(clientId, "secret");
+ assertEquals(201, pResp.getStatusCode());
+ requestUri = pResp.getRequestUri();
+ }
+ catch (Exception e) {
+ Assert.fail();
+ }
+
+ oauth.redirectUri(null);
+ oauth.scope(null);
+ oauth.responseType(null);
+ oauth.requestUri(requestUri);
+ String state = oauth.stateParamRandom().getState();
+ oauth.stateParamHardcoded(state);
+
+ oauth.openLoginForm();
+ String restartCookie = loginPage.getDriver().manage().getCookieNamed(RestartLoginCookie.KC_RESTART).getValue();
+ assertRestartCookie(restartCookie);
+ }
+
+ private void assertRestartCookie(String restartCookie) {
+ getTestingClient()
+ .server(TEST_REALM_NAME)
+ .run(session ->
+ {
+ try {
+ String sigAlgorithm = session.tokens().signatureAlgorithm(TokenCategory.INTERNAL);
+ String encAlgorithm = session.tokens().cekManagementAlgorithm(TokenCategory.INTERNAL);
+ SecretKey encKey = session.keys().getActiveKey(session.getContext().getRealm(), KeyUse.ENC, encAlgorithm).getSecretKey();
+ SecretKey signKey = session.keys().getActiveKey(session.getContext().getRealm(), KeyUse.SIG, sigAlgorithm).getSecretKey();
+
+ byte[] contentBytes = TokenUtil.jweDirectVerifyAndDecode(encKey, signKey, restartCookie);
+ String jwt = new String(contentBytes, StandardCharsets.UTF_8);
+ RestartLoginCookie restartLoginCookie = session.tokens().decode(jwt, RestartLoginCookie.class);
+ Assert.assertFalse(restartLoginCookie.getNotes().keySet().stream().anyMatch(sensitiveNotes::contains));
+ } catch (Exception e) {
+ Assert.fail();
+ }
+ });
+ }
+
// KEYCLOAK-5440 -- migration from Keycloak 3.1.0
@Test
public void testRestartCookieBackwardsCompatible_Keycloak25() throws IOException {
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/keys/FallbackKeyProviderTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/keys/FallbackKeyProviderTest.java
index 5212318c45e6..ba11ae46afec 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/keys/FallbackKeyProviderTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/keys/FallbackKeyProviderTest.java
@@ -82,7 +82,7 @@ public void fallbackAfterDeletingAllKeysInRealm() {
Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType());
providers = realmsResouce().realm("test").components().query(realmId, "org.keycloak.keys.KeyProvider");
- assertProviders(providers, "fallback-RS256", "fallback-" + Constants.INTERNAL_SIGNATURE_ALGORITHM);
+ assertProviders(providers, "fallback-RS256", "fallback-AES", "fallback-" + Constants.INTERNAL_SIGNATURE_ALGORITHM);
}
@Test