Merge pull request #33414 from sberyozkin/session_cookie_encrypt_2.13

2.13: Encrypt OIDC session cookie value by default

.github/native-tests.json

@@ -68,7 +68,7 @@
         },
         {
             "category": "Security1",
-            "timeout": 50,
+            "timeout": 60,
             "test-modules": "elytron-security-oauth2, elytron-security, elytron-security-jdbc, elytron-undertow, elytron-security-ldap, bouncycastle, bouncycastle-jsse, bouncycastle-fips",
             "os-name": "ubuntu-latest"
         },

docs/src/main/asciidoc/security-openid-connect-web-authentication.adoc

@@ -571,7 +571,7 @@ Note that some endpoints do not require the access token. An access token is onl
 If the ID, access and refresh tokens are JWT tokens then combining all of them (if the strategy is the default `keep-all-tokens`) or only ID and refresh tokens (if the strategy is `id-refresh-token`) may produce a session cookie value larger than 4KB and the browsers may not be able to keep this cookie.
 In such cases, you can use `quarkus.oidc.token-state-manager.split-tokens=true` to have a unique session token per each of these tokens.
 
-You can also configure the default `TokenStateManager` to encrypt the tokens before storing them as cookie values which may be necessary if the tokens contain sensitive claim values.
+Note that `TokenStateManager` will encrypt the tokens before storing them in the session cookie.
 For example, here is how you configure it to split the tokens and encrypt them:
 
 [source, properties]
@@ -581,12 +581,14 @@ quarkus.oidc.client-id=quarkus-app
 quarkus.oidc.credentials.secret=secret
 quarkus.oidc.application-type=web-app
 quarkus.oidc.token-state-manager.split-tokens=true
-quarkus.oidc.token-state-manager.encryption-required=true
 quarkus.oidc.token-state-manager.encryption-secret=eUk1p7UB3nFiXZGUXi0uph1Y9p34YhBU
 ----
 
-The token encryption secret must be 32 characters long. Note that you only have to set `quarkus.oidc.token-state-manager.encryption-secret` if you prefer not to use
-`quarkus.oidc.credentials.secret` for encrypting the tokens or if `quarkus.oidc.credentials.secret` length is less than 32 characters.
+The token encryption secret must be at least 32 characters long. If this key is not configured then either `quarkus.oidc.credentials.secret` or `quarkus.oidc.credentials.jwt.secret` will be hashed to create an encryption key.
+
+`quarkus.oidc.token-state-manager.encryption-secret` should be configured if Quarkus authenticates to OpenId Connect Provider using either mTLS or `private_key_jwt` (where a private RSA or EC key is used to sign a JWT token) authentication methods, otherwise a random key will be generated which will be problematic if the Quarkus application is running in the cloud with multiple pods managing the requests.
+
+If you need you can disable encrypting the tokens in the session cookie with `quarkus.oidc.token-state-manager.encryption-required=false`.
 
 Register your own `io.quarkus.oidc.TokenStateManager' implementation as an `@ApplicationScoped` CDI bean if you need to customize the way the tokens are associated with the session cookie. For example, you may want to keep the tokens in a database and have only a database pointer stored in a session cookie. Note though that it may present some challenges in making the tokens available across multiple microservices nodes.
 

extensions/oidc-common/runtime/src/main/java/io/quarkus/oidc/common/runtime/OidcCommonUtils.java

@@ -276,6 +276,10 @@ public static String clientSecret(Credentials creds) {
         return creds.secret.orElse(creds.clientSecret.value.orElseGet(fromCredentialsProvider(creds.clientSecret.provider)));
     }
 
+    public static String jwtSecret(Credentials creds) {
+        return creds.jwt.secret.orElseGet(fromCredentialsProvider(creds.jwt.secretProvider));
+    }
+
     public static Secret.Method clientSecretMethod(Credentials creds) {
         return creds.clientSecret.method.orElseGet(() -> Secret.Method.BASIC);
     }
@@ -300,7 +304,7 @@ public String get() {
     public static Key clientJwtKey(Credentials creds) {
         if (creds.jwt.secret.isPresent() || creds.jwt.secretProvider.key.isPresent()) {
             return KeyUtils
-                    .createSecretKeyFromSecret(creds.jwt.secret.orElseGet(fromCredentialsProvider(creds.jwt.secretProvider)));
+                    .createSecretKeyFromSecret(jwtSecret(creds));
         } else {
             Key key = null;
             try {

extensions/oidc/deployment/src/test/java/io/quarkus/oidc/test/CodeFlowDevModeTestCase.java

@@ -97,7 +97,7 @@ public void testAccessAndRefreshTokenInjectionDevMode() throws IOException, Inte
 
             assertEquals("alice", page.getBody().asText());
 
-            assertEquals("custom", page.getWebClient().getCookieManager().getCookie("q_session").getValue().split("\\|")[3]);
+            assertEquals("custom", page.getWebClient().getCookieManager().getCookie("q_session").getValue().split("\\|")[1]);
 
             webClient.getOptions().setRedirectEnabled(false);
             WebResponse webResponse = webClient

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcTenantConfig.java

@@ -340,8 +340,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.
@@ -351,12 +351,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() {

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java

@@ -648,7 +648,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);
                                     }
                                 });

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/DefaultTokenStateManager.java

@@ -21,14 +21,19 @@ public class DefaultTokenStateManager implements TokenStateManager {
     @Override
     public Uni<String> createTokenState(RoutingContext routingContext, OidcTenantConfig oidcConfig,
             AuthorizationCodeTokens tokens, OidcRequestContext<String> requestContext) {
+
+        boolean encryptAll = !oidcConfig.tokenStateManager.splitTokens;
+
         StringBuilder sb = new StringBuilder();
-        sb.append(encryptToken(tokens.getIdToken(), routingContext, oidcConfig));
+        sb.append(encryptAll ? tokens.getIdToken() : encryptToken(tokens.getIdToken(), routingContext, oidcConfig));
         if (oidcConfig.tokenStateManager.strategy == OidcTenantConfig.TokenStateManager.Strategy.KEEP_ALL_TOKENS) {
             if (!oidcConfig.tokenStateManager.splitTokens) {
                 sb.append(CodeAuthenticationMechanism.COOKIE_DELIM)
-                        .append(encryptToken(tokens.getAccessToken(), routingContext, oidcConfig))
+                        .append(encryptAll ? tokens.getAccessToken()
+                                : encryptToken(tokens.getAccessToken(), routingContext, oidcConfig))
                         .append(CodeAuthenticationMechanism.COOKIE_DELIM)
-                        .append(encryptToken(tokens.getRefreshToken(), routingContext, oidcConfig));
+                        .append(encryptAll ? tokens.getRefreshToken()
+                                : encryptToken(tokens.getRefreshToken(), routingContext, oidcConfig));
             } else {
                 CodeAuthenticationMechanism.createCookie(routingContext,
                         oidcConfig,
@@ -48,7 +53,8 @@ public Uni<String> createTokenState(RoutingContext routingContext, OidcTenantCon
                 sb.append(CodeAuthenticationMechanism.COOKIE_DELIM)
                         .append("")
                         .append(CodeAuthenticationMechanism.COOKIE_DELIM)
-                        .append(encryptToken(tokens.getRefreshToken(), routingContext, oidcConfig));
+                        .append(encryptAll ? tokens.getRefreshToken()
+                                : encryptToken(tokens.getRefreshToken(), routingContext, oidcConfig));
             } else {
                 if (tokens.getRefreshToken() != null) {
                     CodeAuthenticationMechanism.createCookie(routingContext,
@@ -59,21 +65,26 @@ public Uni<String> createTokenState(RoutingContext routingContext, OidcTenantCon
                 }
             }
         }
-        return Uni.createFrom().item(sb.toString());
+        String state = encryptAll ? encryptToken(sb.toString(), routingContext, oidcConfig) : sb.toString();
+        return Uni.createFrom().item(state);
     }
 
     @Override
     public Uni<AuthorizationCodeTokens> getTokens(RoutingContext routingContext, OidcTenantConfig oidcConfig, String tokenState,
             OidcRequestContext<AuthorizationCodeTokens> requestContext) {
+        boolean decryptAll = !oidcConfig.tokenStateManager.splitTokens;
+
+        tokenState = decryptAll ? decryptToken(tokenState, routingContext, oidcConfig) : tokenState;
+
         String[] tokens = CodeAuthenticationMechanism.COOKIE_PATTERN.split(tokenState);
-        String idToken = decryptToken(tokens[0], routingContext, oidcConfig);
+        String idToken = decryptAll ? tokens[0] : decryptToken(tokens[0], routingContext, oidcConfig);
 
         String accessToken = null;
         String refreshToken = null;
         if (oidcConfig.tokenStateManager.strategy == OidcTenantConfig.TokenStateManager.Strategy.KEEP_ALL_TOKENS) {
             if (!oidcConfig.tokenStateManager.splitTokens) {
-                accessToken = decryptToken(tokens[1], routingContext, oidcConfig);
-                refreshToken = decryptToken(tokens[2], routingContext, oidcConfig);
+                accessToken = decryptAll ? tokens[1] : decryptToken(tokens[1], routingContext, oidcConfig);
+                refreshToken = decryptAll ? tokens[2] : decryptToken(tokens[2], routingContext, oidcConfig);
             } else {
                 Cookie atCookie = getAccessTokenCookie(routingContext, oidcConfig);
                 if (atCookie != null) {
@@ -86,7 +97,7 @@ public Uni<AuthorizationCodeTokens> getTokens(RoutingContext routingContext, Oid
             }
         } else if (oidcConfig.tokenStateManager.strategy == OidcTenantConfig.TokenStateManager.Strategy.ID_REFRESH_TOKENS) {
             if (!oidcConfig.tokenStateManager.splitTokens) {
-                refreshToken = decryptToken(tokens[2], routingContext, oidcConfig);
+                refreshToken = decryptAll ? tokens[2] : decryptToken(tokens[2], routingContext, oidcConfig);
             } else {
                 Cookie rtCookie = getRefreshTokenCookie(routingContext, oidcConfig);
                 if (rtCookie != null) {
@@ -129,7 +140,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());
@@ -141,7 +152,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());

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcUtils.java

@@ -422,7 +422,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);
@@ -434,7 +434,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 {

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/TenantConfigContext.java

@@ -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
@@ -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) {
@@ -59,16 +67,27 @@ 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");
+                encSecret = OidcCommonUtils.jwtSecret(config.credentials);
             }
-            if (encSecret.length() < 32) {
-                throw new RuntimeException("Secret key for encrypting tokens must be at least 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;
     }

integration-tests/oidc-code-flow/src/main/resources/application.properties

@@ -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}
@@ -143,7 +144,6 @@ quarkus.oidc.tenant-split-tokens.auth-server-url=${keycloak.url}/realms/quarkus
 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