Validate kid in Cloud Foundry token header

Instead of validating the signature against all the public keys,
we can validate it only against the public key with the kid that
matches the one in the token header.

Closes gh-8126

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryAuthorizationException.java

@@ -63,6 +63,8 @@ class CloudFoundryAuthorizationException extends RuntimeException {
 
 		INVALID_ISSUER(HttpStatus.UNAUTHORIZED),
 
+		INVALID_KEY_ID(HttpStatus.UNAUTHORIZED),
+
 		INVALID_SIGNATURE(HttpStatus.UNAUTHORIZED),
 
 		INVALID_TOKEN(HttpStatus.UNAUTHORIZED),

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityService.java

@@ -18,7 +18,7 @@ package org.springframework.boot.actuate.cloudfoundry;
 
 import java.net.URI;
 import java.net.URISyntaxException;
-import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -104,7 +104,7 @@ class CloudFoundrySecurityService {
 	 * Return all token keys known by the UAA.
 	 * @return a list of token keys
 	 */
-	public List<String> fetchTokenKeys() {
+	public Map<String, String> fetchTokenKeys() {
 		try {
 			return extractTokenKeys(this.restTemplate
 					.getForObject(getUaaUrl() + "/token_keys", Map.class));
@@ -115,11 +115,12 @@ class CloudFoundrySecurityService {
 		}
 	}
 
-	private List<String> extractTokenKeys(Map<?, ?> response) {
-		List<String> tokenKeys = new ArrayList<String>();
+	private Map<String, String> extractTokenKeys(Map<?, ?> response) {
+		Map<String, String> tokenKeys = new HashMap<String, String>();
 		List<?> keys = (List<?>) response.get("keys");
 		for (Object key : keys) {
-			tokenKeys.add((String) ((Map<?, ?>) key).get("value"));
+			Map<?, ?> tokenKey = (Map<?, ?>) key;
+			tokenKeys.put((String) (tokenKey).get("kid"), (String) (tokenKey).get("value"));
 		}
 		return tokenKeys;
 	}

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/Token.java

@@ -97,6 +97,11 @@ class Token {
 		return getRequired(this.claims, "scope", List.class);
 	}
 
+	@SuppressWarnings("unchecked")
+	public String getKeyId() {
+		return getRequired(this.header, "kid", String.class);
+	}
+
 	@SuppressWarnings("unchecked")
 	private <T> T getRequired(Map<String, Object> map, String key, Class<T> type) {
 		Object value = map.get(key);

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/TokenValidator.java

@@ -23,7 +23,7 @@ import java.security.PublicKey;
 import java.security.Signature;
 import java.security.spec.InvalidKeySpecException;
 import java.security.spec.X509EncodedKeySpec;
-import java.util.List;
+import java.util.Map;
 import java.util.concurrent.TimeUnit;
 
 import org.springframework.boot.actuate.cloudfoundry.CloudFoundryAuthorizationException.Reason;
@@ -38,7 +38,7 @@ class TokenValidator {
 
 	private final CloudFoundrySecurityService securityService;
 
-	private List<String> tokenKeys;
+	private Map<String, String> tokenKeys;
 
 	TokenValidator(CloudFoundrySecurityService cloudFoundrySecurityService) {
 		this.securityService = cloudFoundrySecurityService;
@@ -46,12 +46,14 @@ class TokenValidator {
 
 	public void validate(Token token) {
 		validateAlgorithm(token);
-		validateSignature(token);
+		validateKeyIdAndSignature(token);
 		validateExpiry(token);
 		validateIssuer(token);
 		validateAudience(token);
 	}
 
+
+
 	private void validateAlgorithm(Token token) {
 		String algorithm = token.getSignatureAlgorithm();
 		if (algorithm == null) {
@@ -65,19 +67,25 @@ class TokenValidator {
 		}
 	}
 
-	private void validateSignature(Token token) {
-		if (this.tokenKeys == null || !hasValidSignature(token)) {
+	private void validateKeyIdAndSignature(Token token) {
+		String keyId = token.getKeyId();
+		if (this.tokenKeys == null || !hasValidKeyId(keyId)) {
 			this.tokenKeys = this.securityService.fetchTokenKeys();
-			if (!hasValidSignature(token)) {
-				throw new CloudFoundryAuthorizationException(Reason.INVALID_SIGNATURE,
-						"RSA Signature did not match content");
+			if (!hasValidKeyId(keyId)) {
+				throw new CloudFoundryAuthorizationException(Reason.INVALID_KEY_ID,
+						"Key Id present in token header does not match");
 			}
 		}
+
+		if (!hasValidSignature(token, this.tokenKeys.get(keyId))) {
+			throw new CloudFoundryAuthorizationException(Reason.INVALID_SIGNATURE,
+					"RSA Signature did not match content");
+		}
 	}
 
-	private boolean hasValidSignature(Token token) {
-		for (String key : this.tokenKeys) {
-			if (hasValidSignature(token, key)) {
+	private boolean hasValidKeyId(String tokenKeyId) {
+		for (String keyId: this.tokenKeys.keySet()) {
+			if (tokenKeyId.equals(keyId)) {
 				return true;
 			}
 		}

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityServiceTests.java

@@ -16,7 +16,7 @@
 
 package org.springframework.boot.actuate.cloudfoundry;
 
-import java.util.List;
+import java.util.Map;
 
 import org.junit.Before;
 import org.junit.Rule;
@@ -162,13 +162,13 @@ public class CloudFoundrySecurityServiceTests {
 				+ "kqwIn7Glry9n9Suxygbf8g5AzpWcusZgDLIIZ7JTUldBb8qU2a0Dl4mvLZOn4wPo\n"
 				+ "jfj9Cw2QICsc5+Pwf21fP+hzf+1WSRHbnYv8uanRO0gZ8ekGaghM/2H6gqJbo2nI\n"
 				+ "JwIDAQAB\n-----END PUBLIC KEY-----";
-		String responseBody = "{\"keys\" : [ {\"value\" : \""
+		String responseBody = "{\"keys\" : [ {\"kid\":\"test-key\",\"value\" : \""
 				+ tokenKeyValue.replace("\n", "\\n") + "\"} ]}";
 		this.server.expect(requestTo(UAA_URL + "/token_keys"))
 				.andRespond(withSuccess(responseBody, MediaType.APPLICATION_JSON));
-		List<String> tokenKeys = this.securityService.fetchTokenKeys();
+		Map<String, String> tokenKeys = this.securityService.fetchTokenKeys();
 		this.server.verify();
-		assertThat(tokenKeys).containsExactly(tokenKeyValue);
+		assertThat(tokenKeys.get("test-key")).isEqualTo(tokenKeyValue);
 	}
 
 	@Test
@@ -178,7 +178,7 @@ public class CloudFoundrySecurityServiceTests {
 		String responseBody = "{\"keys\": []}";
 		this.server.expect(requestTo(UAA_URL + "/token_keys"))
 				.andRespond(withSuccess(responseBody, MediaType.APPLICATION_JSON));
-		List<String> tokenKeys = this.securityService.fetchTokenKeys();
+		Map<String, String> tokenKeys = this.securityService.fetchTokenKeys();
 		this.server.verify();
 		assertThat(tokenKeys).hasSize(0);
 	}

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/TokenTests.java

@@ -82,6 +82,7 @@ public class TokenTests {
 		assertThat(token.getExpiry()).isEqualTo(2147483647);
 		assertThat(token.getIssuer()).isEqualTo("http://localhost:8080/uaa/oauth/token");
 		assertThat(token.getSignatureAlgorithm()).isEqualTo("RS256");
+		assertThat(token.getKeyId()).isEqualTo("key-id");
 		assertThat(token.getContent()).isEqualTo(content.getBytes());
 		assertThat(token.getSignature())
 				.isEqualTo(Base64Utils.decodeFromString(signature));
@@ -100,7 +101,7 @@ public class TokenTests {
 
 	@Test
 	public void getIssuerWhenIssIsNullShouldThrowException() throws Exception {
-		String header = "{\"alg\": \"RS256\",  \"kid\": \"key-id\", \"typ\": \"JWT\"}";
+		String header = "{\"alg\": \"RS256\", \"kid\": \"key-id\", \"typ\": \"JWT\"}";
 		String claims = "{\"exp\": 2147483647}";
 		Token token = createToken(header, claims);
 		this.thrown
@@ -108,6 +109,16 @@ public class TokenTests {
 		token.getIssuer();
 	}
 
+	@Test
+	public void getKidWhenKidIsNullShouldThrowException() throws Exception {
+		String header = "{\"alg\": \"RS256\", \"typ\": \"JWT\"}";
+		String claims = "{\"exp\": 2147483647}";
+		Token token = createToken(header, claims);
+		this.thrown
+				.expect(AuthorizationExceptionMatcher.withReason(Reason.INVALID_TOKEN));
+		token.getKeyId();
+	}
+
 	@Test
 	public void getExpiryWhenExpIsNullShouldThrowException() throws Exception {
 		String header = "{\"alg\": \"RS256\",  \"kid\": \"key-id\", \"typ\": \"JWT\"}";

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/TokenValidatorTests.java

@@ -26,7 +26,7 @@ import java.security.Signature;
 import java.security.spec.InvalidKeySpecException;
 import java.security.spec.PKCS8EncodedKeySpec;
 import java.util.Collections;
-import java.util.List;
+import java.util.Map;
 
 import org.apache.commons.codec.binary.Base64;
 import org.junit.Before;
@@ -82,10 +82,10 @@ public class TokenValidatorTests {
 			+ "r3F7aM9YpErzeYLrl0GhQr9BVJxOvXcVd4kmY+XkiCcrkyS1cnghnllh+LCwQu1s\n"
 			+ "YwIDAQAB\n-----END PUBLIC KEY-----";
 
-	private static final List<String> INVALID_KEYS = Collections
-			.singletonList(INVALID_KEY);
+	private static final Map<String, String> INVALID_KEYS = Collections
+			.singletonMap("invalid-key", INVALID_KEY);
 
-	private static final List<String> VALID_KEYS = Collections.singletonList(VALID_KEY);
+	private static final Map<String, String> VALID_KEYS = Collections.singletonMap("valid-key", VALID_KEY);
 
 	@Before
 	public void setup() throws Exception {
@@ -94,25 +94,25 @@ public class TokenValidatorTests {
 	}
 
 	@Test
-	public void validateTokenWhenSignatureValidationFailsTwiceShouldThrowException()
+	public void validateTokenWhenKidValidationFailsTwiceShouldThrowException()
 			throws Exception {
 		ReflectionTestUtils.setField(this.tokenValidator, "tokenKeys", INVALID_KEYS);
 		given(this.securityService.fetchTokenKeys()).willReturn(INVALID_KEYS);
-		String header = "{\"alg\": \"RS256\",  \"kid\": \"key-id\",\"typ\": \"JWT\"}";
+		String header = "{\"alg\": \"RS256\",  \"kid\": \"valid-key\",\"typ\": \"JWT\"}";
 		String claims = "{\"exp\": 2147483647, \"iss\": \"http://localhost:8080/uaa/oauth/token\", \"scope\": [\"actuator.read\"]}";
 		this.thrown.expect(
-				AuthorizationExceptionMatcher.withReason(Reason.INVALID_SIGNATURE));
+				AuthorizationExceptionMatcher.withReason(Reason.INVALID_KEY_ID));
 		this.tokenValidator.validate(
 				new Token(getSignedToken(header.getBytes(), claims.getBytes())));
 	}
 
 	@Test
-	public void validateTokenWhenSignatureValidationSucceedsInTheSecondAttempt()
+	public void validateTokenWhenKidValidationSucceedsInTheSecondAttempt()
 			throws Exception {
 		ReflectionTestUtils.setField(this.tokenValidator, "tokenKeys", INVALID_KEYS);
 		given(this.securityService.fetchTokenKeys()).willReturn(VALID_KEYS);
 		given(this.securityService.getUaaUrl()).willReturn("http://localhost:8080/uaa");
-		String header = "{ \"alg\": \"RS256\",  \"kid\": \"key-id\",\"typ\": \"JWT\"}";
+		String header = "{ \"alg\": \"RS256\",  \"kid\": \"valid-key\",\"typ\": \"JWT\"}";
 		String claims = "{ \"exp\": 2147483647, \"iss\": \"http://localhost:8080/uaa/oauth/token\", \"scope\": [\"actuator.read\"]}";
 		this.tokenValidator.validate(
 				new Token(getSignedToken(header.getBytes(), claims.getBytes())));
@@ -123,7 +123,7 @@ public class TokenValidatorTests {
 	public void validateTokenShouldFetchTokenKeysIfNull() throws Exception {
 		given(this.securityService.fetchTokenKeys()).willReturn(VALID_KEYS);
 		given(this.securityService.getUaaUrl()).willReturn("http://localhost:8080/uaa");
-		String header = "{ \"alg\": \"RS256\",  \"kid\": \"key-id\",\"typ\": \"JWT\"}";
+		String header = "{ \"alg\": \"RS256\",  \"kid\": \"valid-key\",\"typ\": \"JWT\"}";
 		String claims = "{ \"exp\": 2147483647, \"iss\": \"http://localhost:8080/uaa/oauth/token\", \"scope\": [\"actuator.read\"]}";
 		this.tokenValidator.validate(
 				new Token(getSignedToken(header.getBytes(), claims.getBytes())));
@@ -131,17 +131,29 @@ public class TokenValidatorTests {
 	}
 
 	@Test
-	public void validateTokenWhenSignatureValidShouldNotFetchTokenKeys()
+	public void validateTokenWhenValidShouldNotFetchTokenKeys()
 			throws Exception {
 		ReflectionTestUtils.setField(this.tokenValidator, "tokenKeys", VALID_KEYS);
 		given(this.securityService.getUaaUrl()).willReturn("http://localhost:8080/uaa");
-		String header = "{ \"alg\": \"RS256\",  \"kid\": \"key-id\",\"typ\": \"JWT\"}";
+		String header = "{ \"alg\": \"RS256\",  \"kid\": \"valid-key\",\"typ\": \"JWT\"}";
 		String claims = "{ \"exp\": 2147483647, \"iss\": \"http://localhost:8080/uaa/oauth/token\", \"scope\": [\"actuator.read\"]}";
 		this.tokenValidator.validate(
 				new Token(getSignedToken(header.getBytes(), claims.getBytes())));
 		verify(this.securityService, Mockito.never()).fetchTokenKeys();
 	}
 
+	@Test
+	public void validateTokenWhenSignatureInvalidShouldThrowException() throws Exception {
+		ReflectionTestUtils.setField(this.tokenValidator, "tokenKeys", Collections.singletonMap("valid-key", INVALID_KEY));
+		given(this.securityService.getUaaUrl()).willReturn("http://localhost:8080/uaa");
+		String header = "{ \"alg\": \"RS256\",  \"kid\": \"valid-key\",\"typ\": \"JWT\"}";
+		String claims = "{ \"exp\": 2147483647, \"iss\": \"http://localhost:8080/uaa/oauth/token\", \"scope\": [\"actuator.read\"]}";
+		this.thrown.expect(
+				AuthorizationExceptionMatcher.withReason(Reason.INVALID_SIGNATURE));
+		this.tokenValidator.validate(
+				new Token(getSignedToken(header.getBytes(), claims.getBytes())));
+	}
+
 	@Test
 	public void validateTokenWhenTokenAlgorithmIsNotRS256ShouldThrowException()
 			throws Exception {
@@ -158,7 +170,7 @@ public class TokenValidatorTests {
 	public void validateTokenWhenExpiredShouldThrowException() throws Exception {
 		given(this.securityService.fetchTokenKeys()).willReturn(VALID_KEYS);
 		given(this.securityService.fetchTokenKeys()).willReturn(VALID_KEYS);
-		String header = "{ \"alg\": \"RS256\",  \"kid\": \"key-id\", \"typ\": \"JWT\"}";
+		String header = "{ \"alg\": \"RS256\",  \"kid\": \"valid-key\", \"typ\": \"JWT\"}";
 		String claims = "{ \"jti\": \"0236399c350c47f3ae77e67a75e75e7d\", \"exp\": 1477509977, \"scope\": [\"actuator.read\"]}";
 		this.thrown
 				.expect(AuthorizationExceptionMatcher.withReason(Reason.TOKEN_EXPIRED));
@@ -170,7 +182,7 @@ public class TokenValidatorTests {
 	public void validateTokenWhenIssuerIsNotValidShouldThrowException() throws Exception {
 		given(this.securityService.fetchTokenKeys()).willReturn(VALID_KEYS);
 		given(this.securityService.getUaaUrl()).willReturn("http://other-uaa.com");
-		String header = "{ \"alg\": \"RS256\",  \"kid\": \"key-id\", \"typ\": \"JWT\", \"scope\": [\"actuator.read\"]}";
+		String header = "{ \"alg\": \"RS256\",  \"kid\": \"valid-key\", \"typ\": \"JWT\", \"scope\": [\"actuator.read\"]}";
 		String claims = "{ \"exp\": 2147483647, \"iss\": \"http://localhost:8080/uaa/oauth/token\"}";
 		this.thrown
 				.expect(AuthorizationExceptionMatcher.withReason(Reason.INVALID_ISSUER));
@@ -183,7 +195,7 @@ public class TokenValidatorTests {
 			throws Exception {
 		given(this.securityService.fetchTokenKeys()).willReturn(VALID_KEYS);
 		given(this.securityService.getUaaUrl()).willReturn("http://localhost:8080/uaa");
-		String header = "{ \"alg\": \"RS256\",  \"kid\": \"key-id\", \"typ\": \"JWT\"}";
+		String header = "{ \"alg\": \"RS256\",  \"kid\": \"valid-key\", \"typ\": \"JWT\"}";
 		String claims = "{ \"exp\": 2147483647, \"iss\": \"http://localhost:8080/uaa/oauth/token\", \"scope\": [\"foo.bar\"]}";
 		this.thrown.expect(
 				AuthorizationExceptionMatcher.withReason(Reason.INVALID_AUDIENCE));