diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryAuthorizationException.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryAuthorizationException.java index 9127e2b5e5..51762b5889 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundryAuthorizationException.java +++ b/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), diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityService.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityService.java index 723b53a7e9..f8660d9b84 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityService.java +++ b/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 fetchTokenKeys() { + public Map fetchTokenKeys() { try { return extractTokenKeys(this.restTemplate .getForObject(getUaaUrl() + "/token_keys", Map.class)); @@ -115,11 +115,12 @@ class CloudFoundrySecurityService { } } - private List extractTokenKeys(Map response) { - List tokenKeys = new ArrayList(); + private Map extractTokenKeys(Map response) { + Map tokenKeys = new HashMap(); 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; } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/Token.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/Token.java index 0a529dd581..3ec7860687 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/Token.java +++ b/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 getRequired(Map map, String key, Class type) { Object value = map.get(key); diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/TokenValidator.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/TokenValidator.java index 31a29e2056..3139077511 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/cloudfoundry/TokenValidator.java +++ b/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 tokenKeys; + private Map 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; } } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityServiceTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityServiceTests.java index 0f80c90fef..b6d2fcad48 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/CloudFoundrySecurityServiceTests.java +++ b/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 tokenKeys = this.securityService.fetchTokenKeys(); + Map 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 tokenKeys = this.securityService.fetchTokenKeys(); + Map tokenKeys = this.securityService.fetchTokenKeys(); this.server.verify(); assertThat(tokenKeys).hasSize(0); } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/TokenTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/TokenTests.java index 4807449713..098841abd7 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/TokenTests.java +++ b/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\"}"; diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/TokenValidatorTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/TokenValidatorTests.java index 58eda0aa9b..f1770fae1a 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/cloudfoundry/TokenValidatorTests.java +++ b/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 INVALID_KEYS = Collections - .singletonList(INVALID_KEY); + private static final Map INVALID_KEYS = Collections + .singletonMap("invalid-key", INVALID_KEY); - private static final List VALID_KEYS = Collections.singletonList(VALID_KEY); + private static final Map 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));