Browse Source

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
Josh Cummings 7 years ago
parent
commit
998d1a064b

+ 18 - 2
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;
+		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 {
-			JWT parsedJwt = JWTParser.parse(token);
+			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 {
 			// Verify the signature
 			JWTClaimsSet jwtClaimsSet = this.jwtProcessor.process(parsedJwt, null);
 

+ 21 - 3
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");
+	}
 }

+ 9 - 0
oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/NimbusReactiveJwtDecoderTests.java

@@ -41,6 +41,8 @@ public class NimbusReactiveJwtDecoderTests {
 
 	private String messageReadToken = "eyJraWQiOiJrZXktaWQtMSIsImFsZyI6IlJTMjU2In0.eyJzY29wZSI6Im1lc3NhZ2U6cmVhZCIsImV4cCI6OTIyMzM3MjAwNjA5NjM3NX0.bnQ8IJDXmQbmIXWku0YT1HOyV_3d0iQSA_0W2CmPyELhsxFETzBEEcZ0v0xCBiswDT51rwD83wbX3YXxb84fM64AhpU8wWOxLjha4J6HJX2JnlG47ydaAVD7eWGSYTavyyQ-CwUjQWrfMVcObFZLYG11ydzRYOR9-aiHcK3AobcTcS8jZFeI8EGQV_Cd3IJ018uFCf6VnXLv7eV2kRt08Go2RiPLW47ExvD7Dzzz_wDBKfb4pNem7fDvuzB3UPcp5m9QvLZicnbS_6AvDi6P1y_DFJf-1T5gkGmX5piDH1L1jg2Yl6tjmXbk5B3VhsyjJuXE6gzq1d-xie0Z1NVOxw";
 
+	private String unsignedToken = "eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0.eyJleHAiOi0yMDMzMjI0OTcsImp0aSI6IjEyMyIsInR5cCI6IkpXVCJ9.";
+
 	private String jwkSet =
 		"{\n"
 		+ "   \"keys\":[\n"
@@ -135,4 +137,11 @@ public class NimbusReactiveJwtDecoderTests {
 		assertThatCode(() -> this.decoder.decode("ew0KICAiYWxnIjogIkVTMjU2IiwNCiAgInR5cCI6ICJKV1QiDQp9.ew0KICAic3ViIjogIjEyMzQ1Njc4OTAiLA0KICAibmFtZSI6ICJKb2huIERvZSIsDQogICJpYXQiOiAxNTE2MjM5MDIyDQp9.").block())
 				.isInstanceOf(JwtException.class);
 	}
+
+	@Test
+	public void decodeWhenUnsignedTokenThenMessageDoesNotMentionClass() {
+		assertThatCode(() -> this.decoder.decode(this.unsignedToken).block())
+				.isInstanceOf(JwtException.class)
+				.hasMessage("Unsupported algorithm of none");
+	}
 }