From ffd8f7c8e401cfa2551b551f3fff54b11983f042 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Mon, 25 Jun 2018 16:16:25 -0600 Subject: [PATCH] Close Nimbus Information Leak This commit captures and remaps the exception that Nimbus throws when a PlainJWT is presented to it. While the surrounding classes are likely only used today by the oauth2Login flow, since they are public, we'll patch them at this point for anyone who may be using them directly. Fixes: gh-5457 --- .../jwt/NimbusJwtDecoderJwkSupport.java | 20 ++++++++++++++-- .../jwt/NimbusJwtDecoderJwkSupportTests.java | 24 ++++++++++++++++--- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoderJwkSupport.java b/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoderJwkSupport.java index 6a8f0eb34f..dc93bf0e05 100644 --- a/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoderJwkSupport.java +++ b/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoderJwkSupport.java @@ -26,6 +26,7 @@ import com.nimbusds.jose.util.ResourceRetriever; import com.nimbusds.jwt.JWT; import com.nimbusds.jwt.JWTClaimsSet; import com.nimbusds.jwt.JWTParser; +import com.nimbusds.jwt.SignedJWT; import com.nimbusds.jwt.proc.ConfigurableJWTProcessor; import com.nimbusds.jwt.proc.DefaultJWTProcessor; import org.springframework.security.oauth2.jose.jws.JwsAlgorithms; @@ -95,11 +96,26 @@ public final class NimbusJwtDecoderJwkSupport implements JwtDecoder { @Override public Jwt decode(String token) throws JwtException { + JWT jwt = this.parse(token); + if ( jwt instanceof SignedJWT ) { + return this.createJwt(token, jwt); + } + + throw new JwtException("Unsupported algorithm of " + jwt.getHeader().getAlgorithm()); + } + + private JWT parse(String token) { + try { + return JWTParser.parse(token); + } catch (Exception ex) { + throw new JwtException("An error occurred while attempting to decode the Jwt: " + ex.getMessage(), ex); + } + } + + private Jwt createJwt(String token, JWT parsedJwt) { Jwt jwt; try { - JWT parsedJwt = JWTParser.parse(token); - // Verify the signature JWTClaimsSet jwtClaimsSet = this.jwtProcessor.process(parsedJwt, null); diff --git a/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoderJwkSupportTests.java b/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoderJwkSupportTests.java index 177a52f91c..37ed64f0be 100644 --- a/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoderJwkSupportTests.java +++ b/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoderJwkSupportTests.java @@ -20,6 +20,7 @@ import com.nimbusds.jose.JWSHeader; import com.nimbusds.jwt.JWT; import com.nimbusds.jwt.JWTClaimsSet; import com.nimbusds.jwt.JWTParser; +import com.nimbusds.jwt.SignedJWT; import com.nimbusds.jwt.proc.DefaultJWTProcessor; import org.junit.Test; import org.junit.runner.RunWith; @@ -29,14 +30,19 @@ import org.springframework.security.oauth2.jose.jws.JwsAlgorithms; import static org.assertj.core.api.AssertionsForClassTypes.assertThatCode; import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy; -import static org.mockito.ArgumentMatchers.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; -import static org.powermock.api.mockito.PowerMockito.*; +import static org.powermock.api.mockito.PowerMockito.mockStatic; +import static org.powermock.api.mockito.PowerMockito.when; +import static org.powermock.api.mockito.PowerMockito.whenNew; /** * Tests for {@link NimbusJwtDecoderJwkSupport}. * * @author Joe Grandja + * @author Josh Cummings */ @RunWith(PowerMockRunner.class) @PrepareForTest({NimbusJwtDecoderJwkSupport.class, JWTParser.class}) @@ -44,6 +50,8 @@ public class NimbusJwtDecoderJwkSupportTests { private static final String JWK_SET_URL = "https://provider.com/oauth2/keys"; private static final String JWS_ALGORITHM = JwsAlgorithms.RS256; + private String unsignedToken = "eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0.eyJleHAiOi0yMDMzMjI0OTcsImp0aSI6IjEyMyIsInR5cCI6IkpXVCJ9."; + @Test public void constructorWhenJwkSetUrlIsNullThenThrowIllegalArgumentException() { assertThatThrownBy(() -> new NimbusJwtDecoderJwkSupport(null)) @@ -72,7 +80,7 @@ public class NimbusJwtDecoderJwkSupportTests { // gh-5168 @Test public void decodeWhenExpClaimNullThenDoesNotThrowException() throws Exception { - JWT jwt = mock(JWT.class); + SignedJWT jwt = mock(SignedJWT.class); JWSHeader header = new JWSHeader.Builder(JWSAlgorithm.parse(JWS_ALGORITHM)).build(); when(jwt.getHeader()).thenReturn(header); @@ -88,4 +96,14 @@ public class NimbusJwtDecoderJwkSupportTests { NimbusJwtDecoderJwkSupport jwtDecoder = new NimbusJwtDecoderJwkSupport(JWK_SET_URL, JWS_ALGORITHM); assertThatCode(() -> jwtDecoder.decode("encoded-jwt")).doesNotThrowAnyException(); } + + // gh-5457 + @Test + public void decodeWhenPlainJwtThenExceptionDoesNotMentionClass() throws Exception { + NimbusJwtDecoderJwkSupport jwtDecoder = new NimbusJwtDecoderJwkSupport(JWK_SET_URL, JWS_ALGORITHM); + + assertThatCode(() -> jwtDecoder.decode(this.unsignedToken)) + .isInstanceOf(JwtException.class) + .hasMessageContaining("Unsupported algorithm of none"); + } }