Skip to content

Commit

Permalink
Encrypt OIDC session cookie value by default
Browse files Browse the repository at this point in the history
  • Loading branch information
@sberyozkin
sberyozkin committed Mar 29, 2023
1 parent 34da513 commit 23d0935
Show file tree
Hide file tree
Showing 10 changed files with 98 additions and 50 deletions.
8 changes: 4 additions & 4 deletions extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcTenantConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -374,8 +374,8 @@ public enum Strategy {
/**
* Requires that the tokens are encrypted before being stored in the cookies.
*/
@ConfigItem(defaultValueDocumentation = "false")
public Optional<Boolean> encryptionRequired = Optional.empty();
@ConfigItem(defaultValue = "true")
public boolean encryptionRequired = true;

/**
* Secret which will be used to encrypt the tokens.
Expand All @@ -385,12 +385,12 @@ public enum Strategy {
@ConfigItem
public Optional<String> encryptionSecret = Optional.empty();

public Optional<Boolean> isEncryptionRequired() {
public boolean isEncryptionRequired() {
return encryptionRequired;
}

public void setEncryptionRequired(boolean encryptionRequired) {
this.encryptionRequired = Optional.of(encryptionRequired);
this.encryptionRequired = encryptionRequired;
}

public Optional<String> getEncryptionSecret() {
Expand Down
9 changes: 4 additions & 5 deletions ...sions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,10 +287,7 @@ public Uni<Void> apply(SecurityIdentity identity) {
if (isBackChannelLogoutPendingAndValid(configContext, identity)
|| isFrontChannelLogoutValid(context, configContext,
identity)) {
return OidcUtils
.removeSessionCookie(context, configContext.oidcConfig,
sessionCookie.getName(),
resolver.getTokenStateManager())
return removeSessionCookie(context, configContext.oidcConfig)
.map(new Function<Void, Void>() {
@Override
public Void apply(Void t) {
Expand Down Expand Up @@ -742,7 +739,9 @@ public Throwable apply(Throwable tInner) {
LOG.debugf("Starting the final redirect");
return tInner;
}
LOG.errorf("ID token verification has failed: %s", tInner.getMessage());
String message = tInner.getCause() != null ? tInner.getCause().getMessage()
: tInner.getMessage();
LOG.errorf("ID token verification has failed: %s", message);
return new AuthenticationCompletionException(tInner);
}
});
Expand Down
4 changes: 2 additions & 2 deletions extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/DefaultTokenStateManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ private static String getRefreshTokenCookieName(OidcTenantConfig oidcConfig) {
}

private String encryptToken(String token, RoutingContext context, OidcTenantConfig oidcConfig) {
if (oidcConfig.tokenStateManager.encryptionRequired.orElse(false)) {
if (oidcConfig.tokenStateManager.encryptionRequired) {
TenantConfigContext configContext = context.get(TenantConfigContext.class.getName());
try {
return OidcUtils.encryptString(token, configContext.getTokenEncSecretKey());
Expand All @@ -141,7 +141,7 @@ private String encryptToken(String token, RoutingContext context, OidcTenantConf
}

private String decryptToken(String token, RoutingContext context, OidcTenantConfig oidcConfig) {
if (oidcConfig.tokenStateManager.encryptionRequired.orElse(false)) {
if (oidcConfig.tokenStateManager.encryptionRequired) {
TenantConfigContext configContext = context.get(TenantConfigContext.class.getName());
try {
return OidcUtils.decryptString(token, configContext.getTokenEncSecretKey());
Expand Down
4 changes: 2 additions & 2 deletions extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ public static String encryptJson(JsonObject json, SecretKey key) throws Exceptio

public static String encryptString(String jweString, SecretKey key) throws Exception {
JsonWebEncryption jwe = new JsonWebEncryption();
jwe.setAlgorithmHeaderValue(KeyEncryptionAlgorithm.A256KW.getAlgorithm());
jwe.setAlgorithmHeaderValue(KeyEncryptionAlgorithm.A256GCMKW.getAlgorithm());
jwe.setEncryptionMethodHeaderParameter(ContentEncryptionAlgorithm.A256GCM.getAlgorithm());
jwe.setKey(key);
jwe.setPlaintext(jweString);
Expand All @@ -467,7 +467,7 @@ public static JsonObject decryptJson(String jweString, Key key) throws Exception
}

public static String decryptString(String jweString, Key key) throws Exception {
return decryptString(jweString, key, KeyEncryptionAlgorithm.A256KW);
return decryptString(jweString, key, KeyEncryptionAlgorithm.A256GCMKW);
}

public static String decryptString(String jweString, Key key, KeyEncryptionAlgorithm algorithm) throws JoseException {
Expand Down
34 changes: 25 additions & 9 deletions extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/TenantConfigContext.java
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,20 @@
package io.quarkus.oidc.runtime;

import java.nio.charset.StandardCharsets;

import javax.crypto.KeyGenerator;
import javax.crypto.SecretKey;
import javax.crypto.spec.SecretKeySpec;

import org.jboss.logging.Logger;

import io.quarkus.oidc.OIDCException;
import io.quarkus.oidc.OidcTenantConfig;
import io.quarkus.oidc.common.runtime.OidcCommonUtils;
import io.smallrye.jwt.util.KeyUtils;

public class TenantConfigContext {
private static final Logger LOG = Logger.getLogger(TenantConfigContext.class);

/**
* OIDC Provider
Expand Down Expand Up @@ -39,8 +47,8 @@ public TenantConfigContext(OidcProvider client, OidcTenantConfig config, boolean
this.oidcConfig = config;
this.ready = ready;

pkceSecretKey = createPkceSecretKey(config);
tokenEncSecretKey = createTokenEncSecretKey(config);
pkceSecretKey = provider != null && provider.client != null ? createPkceSecretKey(config) : null;
tokenEncSecretKey = provider != null && provider.client != null ? createTokenEncSecretKey(config) : null;
}

private static SecretKey createPkceSecretKey(OidcTenantConfig config) {
Expand All @@ -59,16 +67,24 @@ private static SecretKey createPkceSecretKey(OidcTenantConfig config) {
}

private static SecretKey createTokenEncSecretKey(OidcTenantConfig config) {
if (config.tokenStateManager.encryptionRequired.orElse(false)) {
if (config.tokenStateManager.encryptionRequired) {
String encSecret = config.tokenStateManager.encryptionSecret
.orElse(OidcCommonUtils.clientSecret(config.credentials));
if (encSecret == null) {
throw new RuntimeException("Secret key for encrypting tokens is missing");
}
if (encSecret.length() != 32) {
throw new RuntimeException("Secret key for encrypting tokens must be 32 characters long");
try {
if (encSecret == null) {
LOG.warn("Secret key for encrypting tokens is missing, auto-generating it");
KeyGenerator keyGenerator = KeyGenerator.getInstance("AES");
keyGenerator.init(256);
return keyGenerator.generateKey();
}
byte[] secretBytes = encSecret.getBytes(StandardCharsets.UTF_8);
if (secretBytes.length < 32) {
LOG.warn("Secret key for encrypting tokens should be 32 characters long");
}
return new SecretKeySpec(OidcUtils.getSha256Digest(secretBytes), "AES");
} catch (Exception ex) {
throw new OIDCException(ex);
}
return KeyUtils.createSecretKeyFromSecret(encSecret);
}
return null;
}
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/oidc-code-flow/src/main/resources/application.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ quarkus.oidc.authentication.cookie-domain=localhost
quarkus.oidc.authentication.extra-params.max-age=60
quarkus.oidc.application-type=web-app
quarkus.oidc.authentication.cookie-suffix=test
quarkus.oidc.token-state-manager.encryption-required=false

# OIDC client configuration
quarkus.oidc-client.auth-server-url=${quarkus.oidc.auth-server-url}
Expand Down Expand Up @@ -145,7 +146,6 @@ quarkus.oidc.tenant-split-tokens.auth-server-url=${quarkus.oidc.auth-server-url}
quarkus.oidc.tenant-split-tokens.client-id=quarkus-app
quarkus.oidc.tenant-split-tokens.credentials.secret=secret
quarkus.oidc.tenant-split-tokens.token-state-manager.split-tokens=true
quarkus.oidc.tenant-split-tokens.token-state-manager.encryption-required=true
quarkus.oidc.tenant-split-tokens.token-state-manager.encryption-secret=eUk1p7UB3nFiXZGUXi0uph1Y9p34YhBU
quarkus.oidc.tenant-split-tokens.application-type=web-app

Expand Down
51 changes: 36 additions & 15 deletions integration-tests/oidc-code-flow/src/test/java/io/quarkus/it/keycloak/CodeFlowTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
import java.util.concurrent.Callable;
import java.util.concurrent.TimeUnit;

import javax.crypto.SecretKey;
import javax.crypto.spec.SecretKeySpec;

import org.hamcrest.Matchers;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -275,7 +278,15 @@ public void testCodeFlowForceHttpsRedirectUriWithQueryAndPkce() throws Exception

Cookie sessionCookie = getSessionCookie(webClient, "tenant-https_test");
assertNotNull(sessionCookie);
JsonObject idToken = OidcUtils.decodeJwtContent(sessionCookie.getValue().split("\\|")[0]);

String encryptedIdToken = sessionCookie.getValue().split("\\|")[0];

SecretKey key = new SecretKeySpec(OidcUtils
.getSha256Digest("secret".getBytes(StandardCharsets.UTF_8)),
"AES");
String encodedIdToken = OidcUtils.decryptString(encryptedIdToken, key);

JsonObject idToken = OidcUtils.decodeJwtContent(encodedIdToken);
String expiresAt = idToken.getInteger("exp").toString();
page = webClient.getPage(endpointLocationWithoutQueryUri.toURL());
String response = page.getBody().asNormalizedText();
Expand Down Expand Up @@ -850,7 +861,7 @@ public void testDefaultSessionManagerIdTokenOnly() throws IOException, Interrupt
assertEquals("tenant-idtoken-only:no refresh", page.getBody().asNormalizedText());

Cookie idTokenCookie = getSessionCookie(page.getWebClient(), "tenant-idtoken-only");
checkSingleTokenCookie(idTokenCookie, "ID");
checkSingleTokenCookie(idTokenCookie, "ID", "secret");

assertNull(getSessionAtCookie(webClient, "tenant-idtoken-only"));
assertNull(getSessionRtCookie(webClient, "tenant-idtoken-only"));
Expand All @@ -860,7 +871,7 @@ public void testDefaultSessionManagerIdTokenOnly() throws IOException, Interrupt
}

@Test
public void testDefaultSessionManagerIdRefreshTokens() throws IOException, InterruptedException {
public void testDefaultSessionManagerIdRefreshTokens() throws Exception {
try (final WebClient webClient = createWebClient()) {
HtmlPage page = webClient.getPage("http://localhost:8081/web-app/tenant-id-refresh-token");
assertNotNull(getStateCookie(webClient, "tenant-id-refresh-token"));
Expand All @@ -881,11 +892,16 @@ public void testDefaultSessionManagerIdRefreshTokens() throws IOException, Inter
assertEquals("tenant-id-refresh-token:RT injected", page.getBody().asNormalizedText());

Cookie idTokenCookie = getSessionCookie(page.getWebClient(), "tenant-id-refresh-token");

SecretKey key = new SecretKeySpec(OidcUtils
.getSha256Digest("secret".getBytes(StandardCharsets.UTF_8)),
"AES");

String[] parts = idTokenCookie.getValue().split("\\|");
assertEquals(3, parts.length);
assertEquals("ID", OidcUtils.decodeJwtContent(parts[0]).getString("typ"));
assertEquals("ID", OidcUtils.decodeJwtContent(OidcUtils.decryptString(parts[0], key)).getString("typ"));
assertEquals("", parts[1]);
assertEquals("Refresh", OidcUtils.decodeJwtContent(parts[2]).getString("typ"));
assertEquals("Refresh", OidcUtils.decodeJwtContent(OidcUtils.decryptString(parts[2], key)).getString("typ"));

assertNull(getSessionAtCookie(webClient, "tenant-id-refresh-token"));
assertNull(getSessionRtCookie(webClient, "tenant-id-refresh-token"));
Expand Down Expand Up @@ -916,14 +932,15 @@ public void testDefaultSessionManagerSplitTokens() throws IOException, Interrupt
page = webClient.getPage("http://localhost:8081/web-app/refresh/tenant-split-tokens");
assertEquals("tenant-split-tokens:RT injected", page.getBody().asNormalizedText());

final String decryptSecret = "eUk1p7UB3nFiXZGUXi0uph1Y9p34YhBU";
Cookie idTokenCookie = getSessionCookie(page.getWebClient(), "tenant-split-tokens");
checkSingleTokenCookie(idTokenCookie, "ID", true);
checkSingleTokenCookie(idTokenCookie, "ID", decryptSecret);

Cookie atTokenCookie = getSessionAtCookie(page.getWebClient(), "tenant-split-tokens");
checkSingleTokenCookie(atTokenCookie, "Bearer", true);
checkSingleTokenCookie(atTokenCookie, "Bearer", decryptSecret);

Cookie rtTokenCookie = getSessionRtCookie(page.getWebClient(), "tenant-split-tokens");
checkSingleTokenCookie(rtTokenCookie, "Refresh", true);
checkSingleTokenCookie(rtTokenCookie, "Refresh", decryptSecret);

// verify all the cookies are cleared after the session timeout
webClient.getOptions().setRedirectEnabled(false);
Expand Down Expand Up @@ -972,12 +989,12 @@ public void testDefaultSessionManagerIdRefreshSplitTokens() throws IOException,
assertEquals("tenant-split-id-refresh-token:RT injected", page.getBody().asNormalizedText());

Cookie idTokenCookie = getSessionCookie(page.getWebClient(), "tenant-split-id-refresh-token");
checkSingleTokenCookie(idTokenCookie, "ID");
checkSingleTokenCookie(idTokenCookie, "ID", "secret");

assertNull(getSessionAtCookie(page.getWebClient(), "tenant-split-id-refresh-token"));

Cookie rtTokenCookie = getSessionRtCookie(page.getWebClient(), "tenant-split-id-refresh-token");
checkSingleTokenCookie(rtTokenCookie, "Refresh");
checkSingleTokenCookie(rtTokenCookie, "Refresh", "secret");

// verify all the cookies are cleared after the session timeout
webClient.getOptions().setRedirectEnabled(false);
Expand Down Expand Up @@ -1005,26 +1022,30 @@ public Boolean call() throws Exception {
}

private void checkSingleTokenCookie(Cookie tokenCookie, String type) {
checkSingleTokenCookie(tokenCookie, type, false);
checkSingleTokenCookie(tokenCookie, type, null);

}

private void checkSingleTokenCookie(Cookie tokenCookie, String type, boolean decrypt) {
private void checkSingleTokenCookie(Cookie tokenCookie, String type, String decryptSecret) {
String[] cookieParts = tokenCookie.getValue().split("\\|");
assertEquals(1, cookieParts.length);
String token = cookieParts[0];
String[] tokenParts = token.split("\\.");
if (decrypt) {
if (decryptSecret != null) {
assertEquals(5, tokenParts.length);
try {
token = OidcUtils.decryptString(token, KeyUtils.createSecretKeyFromSecret("eUk1p7UB3nFiXZGUXi0uph1Y9p34YhBU"));
SecretKey key = new SecretKeySpec(OidcUtils
.getSha256Digest(decryptSecret.getBytes(StandardCharsets.UTF_8)),
"AES");
token = OidcUtils.decryptString(token, key);
tokenParts = token.split("\\.");
} catch (Exception ex) {
fail("Token decryption has failed");
}
}
assertEquals(3, tokenParts.length);
assertEquals(type, OidcUtils.decodeJwtContent(token).getString("typ"));
JsonObject json = OidcUtils.decodeJwtContent(token);
assertEquals(type, json.getString("typ"));
}

@Test
Expand Down
1 change: 1 addition & 0 deletions ...n-tests/oidc-tenancy/src/main/java/io/quarkus/it/keycloak/CustomTenantConfigResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ public OidcTenantConfig get() {
config.setJwksPath(jwksUri);
config.getToken().setIssuer("any");
config.tokenStateManager.setSplitTokens(true);
config.tokenStateManager.setEncryptionRequired(false);
config.getAuthentication().setSessionAgeExtension(Duration.ofMinutes(1));
config.getAuthentication().setIdTokenRequired(false);
return config;
Expand Down
2 changes: 1 addition & 1 deletion integration-tests/oidc-wiremock/src/main/resources/application.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ quarkus.oidc.code-flow-encrypted-id-token-pem.token.decryption-key-location=priv

quarkus.oidc.code-flow-form-post.auth-server-url=${keycloak.url}/realms/quarkus-form-post/
quarkus.oidc.code-flow-form-post.client-id=quarkus-web-app
quarkus.oidc.code-flow-form-post.credentials.secret=secret
quarkus.oidc.code-flow-form-post.credentials.secret=AyM1SysPpbyDfgZld3umj1qzKObwVMkoqQ-EstJQLr_T-1qS0gZH75aKtMN3Yj0iPS4hcgUuTwjAzZr1Z9CAow
quarkus.oidc.code-flow-form-post.application-type=web-app
quarkus.oidc.code-flow-form-post.authentication.response-mode=form_post
quarkus.oidc.code-flow-form-post.discovery-enabled=false
Expand Down
Loading

0 comments on commit 23d0935

Please sign in to comment.