libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases 1 Packages Activity Actions

Encrypted KC_RESTART cookie and removed sensitive notes

Browse source 
Closes #keycloak/keycloak-private#162

Signed-off-by: Giuseppe Graziano <g.graziano94@gmail.com>
This commit is contained in:
Giuseppe Graziano 2024-05-21 08:29:17 +02:00 committed by Alexander Schwartz
parent dc59613e8a
commit d5e82356f9
8 changed files with 145 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
docs/documentation/release_notes/index.adoc
View file

@ -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]

5
docs/documentation/release_notes/topics/24_0_5.adoc Normal file
View file

@ -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.

4
services/src/main/java/org/keycloak/jose/jws/DefaultTokenManager.java
View file

@ -283,6 +283,8 @@ public class DefaultTokenManager implements TokenManager {
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 class DefaultTokenManager implements TokenManager {
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:

38
services/src/main/java/org/keycloak/protocol/RestartLoginCookie.java
View file

@ -23,13 +23,17 @@ import org.keycloak.Token;
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 class RestartLoginCookie implements Token {
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 class RestartLoginCookie implements Token {
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 class RestartLoginCookie implements Token {
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;

5
services/src/main/java/org/keycloak/protocol/oidc/endpoints/request/AuthzEndpointRequestParser.java
View file

@ -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) {

6
testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/OAuthClient.java
View file

@ -1202,10 +1202,10 @@ public class OAuthClient {
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) {

88
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/RestartCookieTest.java
View file

@ -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.GeneratedHmacKeyProviderFactory;
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 <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
@ -76,6 +93,16 @@ public class RestartCookieTest extends AbstractTestRealmKeycloakTest {
" }\n" +
"}";
public static final Set<String> 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 @@ public class RestartCookieTest extends AbstractTestRealmKeycloakTest {
}
}
@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 {

2
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/keys/FallbackKeyProviderTest.java
View file

@ -82,7 +82,7 @@ public class FallbackKeyProviderTest extends AbstractKeycloakTest {
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