From 2ab9742cb81a97972e9a05078c5c9f5585eb8558 Mon Sep 17 00:00:00 2001 From: Markus Strehle <11627201+strehle@users.noreply.github.com> Date: Fri, 16 Jun 2023 16:44:35 +0200 Subject: [PATCH] Support json key set in tokenKey (#2343) * Support json key set in tokenKey Why: OIDC IdP trust can be verified using tokenKeyUrl or tokenKey In case of tokenKeyUrl the jwks_uri is called to fetch all current keys in order to verify the JWT signature. In case of tokenKey there is currently only a single key possible. If the IdP has a dynamic key rotation, then trust breaks. * symmetric key fix * test renamed * jackson ignore method * sonar refactorings --- .../identity/uaa/oauth/jwk/JsonWebKeySet.java | 23 ++++++++++- .../uaa/oauth/jwk/JsonWebKeyTests.java | 2 + .../uaa/oauth/jwk/JsonWebKeyHelper.java | 25 ++++++++++++ .../ExternalOAuthAuthenticationManager.java | 15 ++----- .../uaa/oauth/jwk/JsonWebKeySetTests.java | 39 +++++++++++++++++++ 5 files changed, 90 insertions(+), 14 deletions(-) diff --git a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwk/JsonWebKeySet.java b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwk/JsonWebKeySet.java index 7bf774c053e..366f08a2cf4 100644 --- a/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwk/JsonWebKeySet.java +++ b/model/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwk/JsonWebKeySet.java @@ -15,12 +15,18 @@ package org.cloudfoundry.identity.uaa.oauth.jwk; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; +import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; import java.util.Set; /** @@ -28,9 +34,10 @@ */ public class JsonWebKeySet { + private static final String JSON_PROPERTY_KEYS = "keys"; private final List keys; - public JsonWebKeySet(@JsonProperty("keys") List keys) { + public JsonWebKeySet(@JsonProperty(JSON_PROPERTY_KEYS) List keys) { Set set = new LinkedHashSet<>(); //rules for how to override duplicates for (T key : keys) { @@ -38,10 +45,22 @@ public JsonWebKeySet(@JsonProperty("keys") List keys) { set.remove(key); set.add(key); } - this.keys = new LinkedList(set); + this.keys = new LinkedList<>(set); } public List getKeys() { return Collections.unmodifiableList(keys); } + + @JsonIgnore + public Map getKeySetMap() { + Map keySet = new HashMap<>(); + ArrayList> keyArray = new ArrayList<>(); + long keyCount = Optional.ofNullable(keys).orElseThrow(() -> new IllegalStateException("No keys found.")).stream() + .map(k -> keyArray.add(k.getKeyProperties())).filter(Objects::nonNull).count(); + if (keyCount > 0) { + keySet.put(JSON_PROPERTY_KEYS, keyArray); + } + return keySet; + } } diff --git a/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwk/JsonWebKeyTests.java b/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwk/JsonWebKeyTests.java index cde9b72ba18..de2090dc24f 100644 --- a/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwk/JsonWebKeyTests.java +++ b/model/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwk/JsonWebKeyTests.java @@ -4,6 +4,7 @@ import org.cloudfoundry.identity.uaa.util.JsonUtils; import org.junit.jupiter.api.Test; +import java.util.ArrayList; import java.util.Map; import static org.cloudfoundry.identity.uaa.test.ModelTestUtils.getResourceAsString; @@ -26,6 +27,7 @@ void testWebKeyPublic() { // then assertEquals(samlKeySet.getKeys().get(0).getKid(), jsonWebKey.getKid()); assertEquals(samlKeySet.getKeys().get(0).getX5t(), jsonWebKey.getX5t()); + assertEquals(3, ((ArrayList) samlKeySet.getKeySetMap().get("keys")).size()); } @Test diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwk/JsonWebKeyHelper.java b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwk/JsonWebKeyHelper.java index c61f69e5c75..23334776d8e 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwk/JsonWebKeyHelper.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwk/JsonWebKeyHelper.java @@ -21,12 +21,17 @@ import org.cloudfoundry.identity.uaa.util.JsonUtils; import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; +import static org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKey.KeyType.MAC; + public final class JsonWebKeyHelper { private static final Pattern PEM_DATA = Pattern.compile("-----BEGIN (.*)-----(.*)-----END (.*)-----", Pattern.DOTALL); private static final Pattern WS_DATA = Pattern.compile("\\s", Pattern.UNICODE_CHARACTER_CLASS); + private static final Pattern JSON_DATA = Pattern.compile("^\\{(.*)\\:(.*)\\}$", Pattern.DOTALL); private static final int PEM_TYPE = 1; private static final int PEM_CONTENT = 2; @@ -50,4 +55,24 @@ public static JWK getJsonWebKey(String pemData) throws JOSEException { String end = "\n-----END " + m.group(PEM_TYPE) + "-----"; return JWK.parseFromPEMEncodedObjects(begin + WS_DATA.matcher(m.group(PEM_CONTENT).trim()).replaceAll("\n") + end); } + + public static JsonWebKeySet parseConfiguration(String tokenKey) { + Matcher m = JSON_DATA.matcher(tokenKey.trim()); + if (m.matches()) { + return deserialize(tokenKey); + } else { + try { + if (PEM_DATA.matcher(tokenKey.trim()).matches()) { + return deserialize(getJsonWebKey(tokenKey).toJSONString()); + } else { + Map p = new HashMap<>(); + p.put("value", tokenKey); + p.put("kty", MAC.name()); + return new JsonWebKeySet<>(Collections.singletonList(new JsonWebKey(p))); + } + } catch(JOSEException e) { + throw new IllegalArgumentException(e); + } + } + } } diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java index 5ba42e21c98..1defc017d29 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/provider/oauth/ExternalOAuthAuthenticationManager.java @@ -24,6 +24,7 @@ import org.cloudfoundry.identity.uaa.oauth.KeyInfoService; import org.cloudfoundry.identity.uaa.oauth.TokenEndpointBuilder; import org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKey; +import org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKeyHelper; import org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKeySet; import org.cloudfoundry.identity.uaa.oauth.jwt.ChainedSignatureVerifier; import org.cloudfoundry.identity.uaa.oauth.jwt.Jwt; @@ -39,9 +40,9 @@ import org.cloudfoundry.identity.uaa.user.UaaUser; import org.cloudfoundry.identity.uaa.user.UaaUserPrototype; import org.cloudfoundry.identity.uaa.util.JsonUtils; +import org.cloudfoundry.identity.uaa.util.JwtTokenSignedByThisUAA; import org.cloudfoundry.identity.uaa.util.LinkedMaskingMultiValueMap; import org.cloudfoundry.identity.uaa.util.SessionUtils; -import org.cloudfoundry.identity.uaa.util.JwtTokenSignedByThisUAA; import org.cloudfoundry.identity.uaa.util.UaaStringUtils; import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder; import org.slf4j.Logger; @@ -91,8 +92,6 @@ import static java.util.Objects.isNull; import static java.util.Optional.of; import static java.util.Optional.ofNullable; -import static org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKey.KeyType.MAC; -import static org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKey.KeyType.RSA; import static org.cloudfoundry.identity.uaa.oauth.token.ClaimConstants.SUB; import static org.cloudfoundry.identity.uaa.oauth.token.TokenConstants.GRANT_TYPE_AUTHORIZATION_CODE; import static org.cloudfoundry.identity.uaa.provider.ExternalIdentityProviderDefinition.EMAIL_ATTRIBUTE_NAME; @@ -657,19 +656,11 @@ protected List getTokenKeyForUaaOrigin() { } - private static boolean isAssymetricKey(String key) { - return key.startsWith("-----BEGIN"); - } - private JsonWebKeySet getTokenKeyFromOAuth(AbstractExternalOAuthIdentityProviderDefinition config) { String tokenKey = config.getTokenKey(); if (StringUtils.hasText(tokenKey)) { - Map p = new HashMap<>(); - p.put("value", tokenKey); - p.put("kty", isAssymetricKey(tokenKey) ? RSA.name() : MAC.name()); - logger.debug("Key configured, returning."); - return new JsonWebKeySet<>(Collections.singletonList(new JsonWebKey(p))); + return JsonWebKeyHelper.parseConfiguration(tokenKey); } try { return oidcMetadataFetcher.fetchWebKeySet(config); diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwk/JsonWebKeySetTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwk/JsonWebKeySetTests.java index 54e60939fff..49c53c06743 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwk/JsonWebKeySetTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwk/JsonWebKeySetTests.java @@ -15,8 +15,10 @@ package org.cloudfoundry.identity.uaa.oauth.jwk; +import com.nimbusds.jose.jwk.JWKSet; import org.junit.Test; +import java.text.ParseException; import java.util.Arrays; import java.util.LinkedHashSet; @@ -24,6 +26,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; public class JsonWebKeySetTests { @@ -188,4 +191,40 @@ public void testIgnoreUnknownKeyTypes() { assertEquals(1, keys.getKeys().size()); } + @Test + public void testJsonKeySetParseJson() throws ParseException { + String jsonConfig = "{\"keys\":[{\"kty\":\"RSA\",\"e\":\"AQAB\",\"use\":\"sig\",\"kid\":\"key-1\",\"alg\":\"RS256\",\"n\":\"xMi4Z4FBfQEOdNYLmzxkYJvP02TSeapZMKMQo90JQRL07ttIKcDMP6pGcirOGSQWWBBpvdo5EnVOiNzViu9JCJP2IWbHJ4sRe0S1dySYdBRVV_ZkgWOrj7Cr2yT0ZVvCCzH7NAWmlA6LUV19Mnp-ugeGoxK-fsk8SRLS_Z9JdyxgOb3tPxdDas3MZweMZ6HqujoAAG9NASBGjFNXbhMckrEfecwm3OJzsjGFxhqXRqkTsGEHvzETMxfvSkTkldOzmErnjpwyoOPLrXcWIs1wvdXHakfVHSvyb3T4gm3ZfOOoUf6lrd2w1pF_PkA88NkjN2-W9fQmbUzNgVjEQiXo4w\"}]}"; + JsonWebKeySet keys = JsonWebKeyHelper.parseConfiguration(jsonConfig); + assertEquals(1, keys.getKeys().size()); + assertEquals(1, keys.getKeySetMap().size()); + JWKSet joseSet = JWKSet.parse(keys.getKeySetMap()); + assertNotNull(joseSet); + assertEquals(1, joseSet.size()); + } + + @Test + public void testJsonKeySetParsePublicKey() throws ParseException { + String publicKey = "-----BEGIN PUBLIC KEY-----MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAxMi4Z4FBfQEOdNYLmzxkYJvP02TSeapZMKMQo90JQRL07ttIKcDMP6pGcirOGSQWWBBpvdo5EnVOiNzViu9JCJP2IWbHJ4sRe0S1dySYdBRVV/ZkgWOrj7Cr2yT0ZVvCCzH7NAWmlA6LUV19Mnp+ugeGoxK+fsk8SRLS/Z9JdyxgOb3tPxdDas3MZweMZ6HqujoAAG9NASBGjFNXbhMckrEfecwm3OJzsjGFxhqXRqkTsGEHvzETMxfvSkTkldOzmErnjpwyoOPLrXcWIs1wvdXHakfVHSvyb3T4gm3ZfOOoUf6lrd2w1pF/PkA88NkjN2+W9fQmbUzNgVjEQiXo4wIDAQAB-----END PUBLIC KEY-----"; + JsonWebKeySet keys = JsonWebKeyHelper.parseConfiguration(publicKey); + assertEquals(1, keys.getKeys().size()); + assertEquals(1, keys.getKeySetMap().size()); + JWKSet joseSet = JWKSet.parse(keys.getKeySetMap()); + assertNotNull(joseSet); + assertEquals(1, joseSet.size()); + } + + @Test + public void testJsonKeySetParseFailurePEM() throws ParseException { + String publicKey = "-----BEGIN PUBLIC KEY-----tokenKey-----END PUBLIC KEY-----"; + assertThrows(IllegalArgumentException.class, () -> JsonWebKeyHelper.parseConfiguration(publicKey)); + } + + @Test + public void testJsonKeySetParseRawKey() { + String macKey = "tokenKey"; + JsonWebKeySet keys = JsonWebKeyHelper.parseConfiguration(macKey); + assertEquals(1, keys.getKeys().size()); + assertEquals(JsonWebKey.KeyType.MAC, keys.getKeys().get(0).getKty()); + assertEquals(macKey, keys.getKeys().get(0).getValue()); + } }