Skip to content

Commit

Permalink
Added support for ignoring all unknown key types
Browse files Browse the repository at this point in the history 
[#143216503] https://www.pivotaltracker.com/story/show/143216503

Signed-off-by: Bharath Sekar <bharath.sekar@ge.com>
  • Loading branch information
@Priyata25
Jeremy Coffield authored and Priyata25 committed May 2, 2017
1 parent 15e608f commit 0cee5ee
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 18 deletions.
5 changes: 5 additions & 0 deletions model/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwk/JsonWebKeyDeserializer.java
View file
Expand Up @@ -23,6 +23,7 @@
import org.cloudfoundry.identity.uaa.util.JsonUtils;

import java.io.IOException;
import java.util.Arrays;

/**
* See https://tools.ietf.org/html/rfc7517
Expand All @@ -31,6 +32,10 @@ public class JsonWebKeyDeserializer extends JsonDeserializer<JsonWebKey> {
@Override
public JsonWebKey deserialize(JsonParser p, DeserializationContext ctxt) throws IOException, JsonProcessingException {
JsonNode node = JsonUtils.readTree(p);
String kty = node.get("kty").asText("Unknown");
if(Arrays.stream(JsonWebKey.KeyType.values()).noneMatch(knownKeyType -> knownKeyType.name().equals(kty))) {
return null;
}
return new JsonWebKey(JsonUtils.getNodeAsMap(node));
}
}
10 changes: 4 additions & 6 deletions model/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwk/JsonWebKeySet.java
View file
Expand Up @@ -16,10 +16,7 @@
package org.cloudfoundry.identity.uaa.oauth.jwk;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.type.TypeReference;
import org.cloudfoundry.identity.uaa.util.JsonUtils;

import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.LinkedList;
Expand All @@ -36,9 +33,10 @@ public class JsonWebKeySet<T extends JsonWebKey> {
public JsonWebKeySet(@JsonProperty("keys") List<T> keys) {
Set<T> set = new LinkedHashSet<>();
//rules for how to override duplicates
for (T t : keys) {
set.remove(t);
set.add(t);
for (T key : keys) {
if(key == null) continue;
set.remove(key);
set.add(key);
}
this.keys = new LinkedList(set);
}
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/cloudfoundry/identity/uaa/oauth/jwt/ChainedSignatureVerifier.java
View file
Expand Up @@ -28,7 +28,7 @@ public class ChainedSignatureVerifier implements SignatureVerifier {
private final List<SignatureVerifier> delegates;

public ChainedSignatureVerifier(JsonWebKeySet<? extends JsonWebKey> keys) {
if(keys == null || keys.getKeys() == null) {
if(keys == null || keys.getKeys() == null || keys.getKeys().isEmpty()) {
throw new IllegalArgumentException("keys cannot be null or empty");
}
List<SignatureVerifier> ds = new ArrayList<>(keys.getKeys().size());
Expand Down
52 changes: 52 additions & 0 deletions server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwk/JsonWebKeySetTests.java
View file
Expand Up @@ -37,6 +37,16 @@ public class JsonWebKeySetTests {
" \"value\": \"-----BEGIN PUBLIC KEY-----\\nMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDHFr+KICms+tuT1OXJwhCUmR2d\\nKVy7psa8xzElSyzqx7oJyfJ1JZyOzToj9T5SfTIq396agbHJWVfYphNahvZ/7uMX\\nqHxf+ZH9BL1gk9Y6kCnbM5R60gfwjyW1/dQPjOzn9N394zd2FJoFHwdq9Qs0wBug\\nspULZVNRxq7veq/fzwIDAQAB\\n-----END PUBLIC KEY-----\"\n" +
"}";

public static final String unknownKeyJson = "{\n" +
" \"alg\": \"RS256\",\n" +
" \"e\": \"AQAB\",\n" +
" \"kid\": \"legacy\",\n" +
" \"kty\": \"GARBAGE\",\n" +
" \"n\": \"AMcWv4ogKaz625PU5cnCEJSZHZ0pXLumxrzHMSVLLOrHugnJ8nUlnI7NOiP1PlJ9Mirf3pqBsclZV9imE1qG9n_u4xeofF_5kf0EvWCT1jqQKdszlHrSB_CPJbX91A-M7Of03f3jN3YUmgUfB2r1CzTAG6CylQtlU1HGru96r9_P\",\n" +
" \"use\": \"sig\",\n" +
" \"value\": \"-----BEGIN PUBLIC KEY-----\\nMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDHFr+KICms+tuT1OXJwhCUmR2d\\nKVy7psa8xzElSyzqx7oJyfJ1JZyOzToj9T5SfTIq396agbHJWVfYphNahvZ/7uMX\\nqHxf+ZH9BL1gk9Y6kCnbM5R60gfwjyW1/dQPjOzn9N394zd2FJoFHwdq9Qs0wBug\\nspULZVNRxq7veq/fzwIDAQAB\\n-----END PUBLIC KEY-----\"\n" +
"}";

public static final String multiKeyJson = "{\n" +
" \"keys\": [\n" +
" {\n" +
Expand Down Expand Up @@ -67,6 +77,36 @@ public class JsonWebKeySetTests {
" ]\n" +
"}";

public static final String someUnknownKeysJson = "{\n" +
" \"keys\": [\n" +
" {\n" +
" \"alg\": \"RS256\",\n" +
" \"e\": \"AQAB\",\n" +
" \"kid\": \"legacy\",\n" +
" \"kty\": \"RSA\",\n" +
" \"n\": \"AMcWv4ogKaz625PU5cnCEJSZHZ0pXLumxrzHMSVLLOrHugnJ8nUlnI7NOiP1PlJ9Mirf3pqBsclZV9imE1qG9n_u4xeofF_5kf0EvWCT1jqQKdszlHrSB_CPJbX91A-M7Of03f3jN3YUmgUfB2r1CzTAG6CylQtlU1HGru96r9_P\",\n" +
" \"use\": \"sig\",\n" +
" \"value\": \"-----BEGIN PUBLIC KEY-----\\nMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDHFr+KICms+tuT1OXJwhCUmR2d\\nKVy7psa8xzElSyzqx7oJyfJ1JZyOzToj9T5SfTIq396agbHJWVfYphNahvZ/7uMX\\nqHxf+ZH9BL1gk9Y6kCnbM5R60gfwjyW1/dQPjOzn9N394zd2FJoFHwdq9Qs0wBug\\nspULZVNRxq7veq/fzwIDAQAB\\n-----END PUBLIC KEY-----\"\n" +
" },\n" +
" {\n" +
" \"alg\": \"RS256\",\n" +
" \"e\": \"AQAB\",\n" +
" \"kid\": \"legacy\",\n" +
" \"kty\": \"UNKNOWN1\",\n" +
" \"n\": \"AMcWv4ogKaz625PU5cnCEJSZHZ0pXLumxrzHMSVLLOrHugnJ8nUlnI7NOiP1PlJ9Mirf3pqBsclZV9imE1qG9n_u4xeofF_5kf0EvWCT1jqQKdszlHrSB_CPJbX91A-M7Of03f3jN3YUmgUfB2r1CzTAG6CylQtlU1HGru96r9_P\",\n" +
" \"use\": \"sig\",\n" +
" \"value\": \"-----BEGIN PUBLIC KEY-----\\nMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDHFr+KICms+tuT1OXJwhCUmR2d\\nKVy7psa8xzElSyzqx7oJyfJ1JZyOzToj9T5SfTIq396agbHJWVfYphNahvZ/7uMX\\nqHxf+ZH9BL1gk9Y6kCnbM5R60gfwjyW1/dQPjOzn9N394zd2FJoFHwdq9Qs0wBug\\nspULZVNRxq7veq/fzwIDAQAB\\n-----END PUBLIC KEY-----\"\n" +
" },\n" +
" {\n" +
" \"alg\": \"HMACSHA256\",\n" +
" \"k\": \"test-mac-key\",\n" +
" \"kid\": \"mac-id\",\n" +
" \"kty\": \"UNKNOWN2\",\n" +
" \"key_ops\": [\"sign\",\"verify\"]\n" +
" }\n" +
" ]\n" +
"}";


@Test
public void test_multi_key() {
Expand Down Expand Up @@ -108,4 +148,16 @@ public JsonWebKeySet<JsonWebKey> test_key(String json) {
return keys;
}

@Test
public void testUnknownKeyType() {
JsonWebKeySet<JsonWebKey> keys = JsonWebKeyHelper.deserialize(unknownKeyJson);
assertEquals(0, keys.getKeys().size());
}

@Test
public void testIgnoreUnknownKeyTypes() {
JsonWebKeySet<JsonWebKey> keys = JsonWebKeyHelper.deserialize(someUnknownKeysJson);
assertEquals(1, keys.getKeys().size());
}

}
4 changes: 1 addition & 3 deletions server/src/test/java/org/cloudfoundry/identity/uaa/oauth/jwk/RsaJsonWebKeyTests.java
View file
Expand Up @@ -127,8 +127,6 @@ public void ensure_that_duplicates_get_the_last_object() {
public void test_required_properties() {
Map<String,Object> map = new HashMap();
test_create_with_error(map);
map.put("kty", "invalid");
test_create_with_error(map);
map.put("kty", "RSA");
new VerificationKeyResponse(map);
}
Expand Down Expand Up @@ -231,4 +229,4 @@ public JsonWebKeySet<JsonWebKey> deserialize_azure_keys(String json) {
" ]\n" +
"}";

}
}
30 changes: 22 additions & 8 deletions .../src/test/java/org/cloudfoundry/identity/uaa/oauth/jwt/ChainedSignatureVerifierTests.java
View file
Expand Up @@ -18,6 +18,7 @@
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.util.JsonUtils;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mockito;
Expand All @@ -29,18 +30,16 @@

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import static org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKey.KeyType.*;
import static java.util.Collections.singletonMap;
import static org.cloudfoundry.identity.uaa.oauth.jwk.JsonWebKey.KeyType.MAC;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.mockito.Matchers.anyString;

public class ChainedSignatureVerifierTests {
private Signer signer;
Expand Down Expand Up @@ -147,12 +146,11 @@ public void unsupported_key_types_are_ignored() {
p.put("y", "test-ec-key-y");
p.put("use", "sig");
p.put("crv", "test-crv");
JsonWebKey ecKey = new JsonWebKey(p);
Map<String,Object> q = new HashMap<>();
q.put("kty","oct");
q.put("k", "octkeyvalue");
JsonWebKey octKey = new JsonWebKey(q);
verifier = new ChainedSignatureVerifier(new JsonWebKeySet<>(Arrays.asList(validKey, ecKey, octKey)));
JsonWebKeySet keySet = JsonUtils.convertValue(singletonMap("keys", Arrays.asList(validKey, p, q)), JsonWebKeySet.class);
verifier = new ChainedSignatureVerifier(keySet);
List<SignatureVerifier> delegates = new ArrayList((List<SignatureVerifier>) ReflectionTestUtils.getField(verifier, verifier.getClass(), "delegates"));
assertNotNull(delegates);
assertEquals(1, delegates.size());
Expand All @@ -162,6 +160,22 @@ public void unsupported_key_types_are_ignored() {
}
}

@Test(expected = IllegalArgumentException.class)
public void no_supported_key_types_causes_error() {
Map<String,Object> p = new HashMap<>();
p.put("kty","EC");
p.put("kid", "ecid");
p.put("x", "test-ec-key-x");
p.put("y", "test-ec-key-y");
p.put("use", "sig");
p.put("crv", "test-crv");
Map<String,Object> q = new HashMap<>();
q.put("kty","oct");
q.put("k", "octkeyvalue");
JsonWebKeySet keySet = JsonUtils.convertValue(singletonMap("keys", Arrays.asList(p, q)), JsonWebKeySet.class);
verifier = new ChainedSignatureVerifier(keySet);
}

@Test
public void test_multi_key_both_valid() {
Map<String,Object> p = new HashMap<>();
Expand All @@ -185,4 +199,4 @@ public void test_multi_key_both_valid() {
Mockito.verifyZeroInteractions(macSigner);
}

}
}

0 comments on commit 0cee5ee

Please sign in to comment.