Repository: knox Updated Branches: refs/heads/master 60900b955 -> c7cbd46a2
KNOX-962 - Add signature validation tests for the JWT filters Project: http://git-wip-us.apache.org/repos/asf/knox/repo Commit: http://git-wip-us.apache.org/repos/asf/knox/commit/c7cbd46a Tree: http://git-wip-us.apache.org/repos/asf/knox/tree/c7cbd46a Diff: http://git-wip-us.apache.org/repos/asf/knox/diff/c7cbd46a Branch: refs/heads/master Commit: c7cbd46a2dcc3cc8a87b948ad5468577ed25651b Parents: 60900b9 Author: Colm O hEigeartaigh <cohei...@apache.org> Authored: Wed Sep 6 11:02:52 2017 +0100 Committer: Colm O hEigeartaigh <cohei...@apache.org> Committed: Wed Sep 6 11:02:52 2017 +0100 ---------------------------------------------------------------------- .../jwt/filter/AbstractJWTFilter.java | 29 ++--- .../federation/AbstractJWTFilterTest.java | 128 +++++++++++++++---- .../federation/JWTFederationFilterTest.java | 14 +- .../federation/SSOCookieProviderTest.java | 57 +-------- 4 files changed, 133 insertions(+), 95 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/knox/blob/c7cbd46a/gateway-provider-security-jwt/src/main/java/org/apache/hadoop/gateway/provider/federation/jwt/filter/AbstractJWTFilter.java ---------------------------------------------------------------------- diff --git a/gateway-provider-security-jwt/src/main/java/org/apache/hadoop/gateway/provider/federation/jwt/filter/AbstractJWTFilter.java b/gateway-provider-security-jwt/src/main/java/org/apache/hadoop/gateway/provider/federation/jwt/filter/AbstractJWTFilter.java index 8627b3f..e938480 100644 --- a/gateway-provider-security-jwt/src/main/java/org/apache/hadoop/gateway/provider/federation/jwt/filter/AbstractJWTFilter.java +++ b/gateway-provider-security-jwt/src/main/java/org/apache/hadoop/gateway/provider/federation/jwt/filter/AbstractJWTFilter.java @@ -63,7 +63,6 @@ public abstract class AbstractJWTFilter implements Filter { static JWTMessages log = MessagesFactory.get( JWTMessages.class ); protected List<String> audiences; protected JWTokenAuthority authority; - protected String verificationPEM = null; protected RSAPublicKey publicKey = null; private static AuditService auditService = AuditServiceFactory.getAuditService(); private static Auditor auditor = auditService.getAuditor( @@ -74,7 +73,7 @@ public abstract class AbstractJWTFilter implements Filter { throws IOException, ServletException; /** - * + * */ public AbstractJWTFilter() { super(); @@ -128,7 +127,7 @@ public abstract class AbstractJWTFilter implements Filter { */ protected boolean validateAudiences(JWTToken jwtToken) { boolean valid = false; - + String[] tokenAudienceList = jwtToken.getAudienceClaims(); // if there were no expected audiences configured then just // consider any audience acceptable @@ -195,18 +194,18 @@ public abstract class AbstractJWTFilter implements Filter { Set<Principal> principals = new HashSet<>(); Principal p = new PrimaryPrincipal(principal); principals.add(p); - - // The newly constructed Sets check whether this Subject has been set read-only - // before permitting subsequent modifications. The newly created Sets also prevent + + // The newly constructed Sets check whether this Subject has been set read-only + // before permitting subsequent modifications. The newly created Sets also prevent // illegal modifications by ensuring that callers have sufficient permissions. // - // To modify the Principals Set, the caller must have AuthPermission("modifyPrincipals"). - // To modify the public credential Set, the caller must have AuthPermission("modifyPublicCredentials"). + // To modify the Principals Set, the caller must have AuthPermission("modifyPrincipals"). + // To modify the public credential Set, the caller must have AuthPermission("modifyPublicCredentials"). // To modify the private credential Set, the caller must have AuthPermission("modifyPrivateCredentials"). javax.security.auth.Subject subject = new javax.security.auth.Subject(true, principals, emptySet, emptySet); return subject; } - + protected boolean validateToken(HttpServletRequest request, HttpServletResponse response, FilterChain chain, JWTToken token) throws IOException, ServletException { @@ -221,7 +220,7 @@ public abstract class AbstractJWTFilter implements Filter { } catch (TokenServiceException e) { log.unableToVerifyToken(e); } - + if (verified) { // confirm that issue matches intended target - which for this filter must be KNOXSSO if (token.getIssuer().equals("KNOXSSO")) { @@ -235,13 +234,13 @@ public abstract class AbstractJWTFilter implements Filter { } else { log.failedToValidateAudience(); - handleValidationError(request, response, HttpServletResponse.SC_BAD_REQUEST, + handleValidationError(request, response, HttpServletResponse.SC_BAD_REQUEST, "Bad request: missing required token audience"); } } else { log.tokenHasExpired(); - handleValidationError(request, response, HttpServletResponse.SC_BAD_REQUEST, + handleValidationError(request, response, HttpServletResponse.SC_BAD_REQUEST, "Bad request: token has expired"); } } @@ -256,8 +255,8 @@ public abstract class AbstractJWTFilter implements Filter { return false; } - - protected abstract void handleValidationError(HttpServletRequest request, HttpServletResponse response, int status, + + protected abstract void handleValidationError(HttpServletRequest request, HttpServletResponse response, int status, String error) throws IOException; - + } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/knox/blob/c7cbd46a/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/AbstractJWTFilterTest.java ---------------------------------------------------------------------- diff --git a/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/AbstractJWTFilterTest.java b/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/AbstractJWTFilterTest.java index 471c7d3..1893647 100644 --- a/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/AbstractJWTFilterTest.java +++ b/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/AbstractJWTFilterTest.java @@ -26,6 +26,7 @@ import java.security.KeyPair; import java.security.KeyPairGenerator; import java.security.NoSuchAlgorithmException; import java.security.Principal; +import java.security.PublicKey; import java.security.cert.Certificate; import java.security.interfaces.RSAPrivateKey; import java.security.interfaces.RSAPublicKey; @@ -66,7 +67,7 @@ import com.nimbusds.jose.*; import com.nimbusds.jwt.JWTClaimsSet; import com.nimbusds.jwt.SignedJWT; import com.nimbusds.jose.crypto.RSASSASigner; -import com.nimbusds.jose.util.Base64URL; +import com.nimbusds.jose.crypto.RSASSAVerifier; public abstract class AbstractJWTFilterTest { private static final String SERVICE_URL = "https://localhost:8888/resource"; @@ -109,7 +110,7 @@ public abstract class AbstractJWTFilterTest { public void teardown() throws Exception { handler.destroy(); } - + @Test public void testValidJWT() throws Exception { try { @@ -120,7 +121,7 @@ public abstract class AbstractJWTFilterTest { HttpServletRequest request = EasyMock.createNiceMock(HttpServletRequest.class); setTokenOnRequest(request, jwt); - + EasyMock.expect(request.getRequestURL()).andReturn( new StringBuffer(SERVICE_URL)).anyTimes(); EasyMock.expect(request.getQueryString()).andReturn(null); @@ -139,7 +140,7 @@ public abstract class AbstractJWTFilterTest { fail("Should NOT have thrown a ServletException."); } } - + @Test public void testValidAudienceJWT() throws Exception { try { @@ -151,7 +152,7 @@ public abstract class AbstractJWTFilterTest { HttpServletRequest request = EasyMock.createNiceMock(HttpServletRequest.class); setTokenOnRequest(request, jwt); - + EasyMock.expect(request.getRequestURL()).andReturn( new StringBuffer(SERVICE_URL)).anyTimes(); EasyMock.expect(request.getQueryString()).andReturn(null); @@ -184,7 +185,7 @@ public abstract class AbstractJWTFilterTest { HttpServletRequest request = EasyMock.createNiceMock(HttpServletRequest.class); setTokenOnRequest(request, jwt); - + EasyMock.expect(request.getRequestURL()).andReturn( new StringBuffer(SERVICE_URL)).anyTimes(); EasyMock.expect(request.getQueryString()).andReturn(null); @@ -206,7 +207,7 @@ public abstract class AbstractJWTFilterTest { public void testValidVerificationPEM() throws Exception { try { Properties props = getProperties(); - + // System.out.println("+" + pem + "+"); props.put(getAudienceProperty(), "bar"); @@ -248,7 +249,7 @@ public abstract class AbstractJWTFilterTest { HttpServletRequest request = EasyMock.createNiceMock(HttpServletRequest.class); setTokenOnRequest(request, jwt); - + EasyMock.expect(request.getRequestURL()).andReturn( new StringBuffer(SERVICE_URL)).anyTimes(); EasyMock.expect(request.getQueryString()).andReturn(null); @@ -265,7 +266,7 @@ public abstract class AbstractJWTFilterTest { fail("Should NOT have thrown a ServletException."); } } - + @Test public void testValidJWTNoExpiration() throws Exception { try { @@ -276,7 +277,7 @@ public abstract class AbstractJWTFilterTest { HttpServletRequest request = EasyMock.createNiceMock(HttpServletRequest.class); setTokenOnRequest(request, jwt); - + EasyMock.expect(request.getRequestURL()).andReturn( new StringBuffer(SERVICE_URL)).anyTimes(); EasyMock.expect(request.getQueryString()).andReturn(null); @@ -295,14 +296,14 @@ public abstract class AbstractJWTFilterTest { fail("Should NOT have thrown a ServletException."); } } - + @Test public void testUnableToParseJWT() throws Exception { try { Properties props = getProperties(); handler.init(new TestFilterConfig(props)); - SignedJWT jwt = getJWT("bob",new Date(new Date().getTime() + 5000), privateKey, props); + SignedJWT jwt = getJWT("bob", new Date(new Date().getTime() + 5000), privateKey, props); HttpServletRequest request = EasyMock.createNiceMock(HttpServletRequest.class); setGarbledTokenOnRequest(request, jwt); @@ -317,7 +318,82 @@ public abstract class AbstractJWTFilterTest { TestFilterChain chain = new TestFilterChain(); handler.doFilter(request, response, chain); - Assert.assertTrue("doFilterCalled should not be false.", !chain.doFilterCalled); + Assert.assertTrue("doFilterCalled should not be true.", !chain.doFilterCalled); + Assert.assertTrue("No Subject should be returned.", chain.subject == null); + } catch (ServletException se) { + fail("Should NOT have thrown a ServletException."); + } + } + + @Test + public void testFailedSignatureValidationJWT() throws Exception { + try { + // Create a private key to sign the token + KeyPairGenerator kpg = KeyPairGenerator.getInstance("RSA"); + kpg.initialize(1024); + + KeyPair kp = kpg.genKeyPair(); + + Properties props = getProperties(); + handler.init(new TestFilterConfig(props)); + + SignedJWT jwt = getJWT("bob", new Date(new Date().getTime() + 5000), + (RSAPrivateKey)kp.getPrivate(), props); + + HttpServletRequest request = EasyMock.createNiceMock(HttpServletRequest.class); + setTokenOnRequest(request, jwt); + + EasyMock.expect(request.getRequestURL()).andReturn( + new StringBuffer(SERVICE_URL)).anyTimes(); + EasyMock.expect(request.getQueryString()).andReturn(null); + HttpServletResponse response = EasyMock.createNiceMock(HttpServletResponse.class); + EasyMock.expect(response.encodeRedirectURL(SERVICE_URL)).andReturn( + SERVICE_URL).anyTimes(); + EasyMock.replay(request); + + TestFilterChain chain = new TestFilterChain(); + handler.doFilter(request, response, chain); + Assert.assertTrue("doFilterCalled should not be true.", !chain.doFilterCalled); + Assert.assertTrue("No Subject should be returned.", chain.subject == null); + } catch (ServletException se) { + fail("Should NOT have thrown a ServletException."); + } + } + + @Test + public void testInvalidVerificationPEM() throws Exception { + try { + Properties props = getProperties(); + + KeyPairGenerator kpg = KeyPairGenerator.getInstance("RSA"); + kpg.initialize(1024); + + KeyPair KPair = kpg.generateKeyPair(); + String dn = buildDistinguishedName(InetAddress.getLocalHost().getHostName()); + Certificate cert = X509CertificateUtil.generateCertificate(dn, KPair, 365, "SHA1withRSA"); + byte[] data = cert.getEncoded(); + Base64 encoder = new Base64( 76, "\n".getBytes( "ASCII" ) ); + String failingPem = new String(encoder.encodeToString( data ).getBytes( "ASCII" )).trim(); + + props.put(getAudienceProperty(), "bar"); + props.put(getVerificationPemProperty(), failingPem); + handler.init(new TestFilterConfig(props)); + + SignedJWT jwt = getJWT("alice", new Date(new Date().getTime() + 50000), privateKey, props); + + HttpServletRequest request = EasyMock.createNiceMock(HttpServletRequest.class); + setTokenOnRequest(request, jwt); + + EasyMock.expect(request.getRequestURL()).andReturn( + new StringBuffer(SERVICE_URL)).anyTimes(); + EasyMock.expect(request.getQueryString()).andReturn(null); + HttpServletResponse response = EasyMock.createNiceMock(HttpServletResponse.class); + EasyMock.expect(response.encodeRedirectURL(SERVICE_URL)).andReturn(SERVICE_URL); + EasyMock.replay(request); + + TestFilterChain chain = new TestFilterChain(); + handler.doFilter(request, response, chain); + Assert.assertTrue("doFilterCalled should not be true.", chain.doFilterCalled == false); Assert.assertTrue("No Subject should be returned.", chain.subject == null); } catch (ServletException se) { fail("Should NOT have thrown a ServletException."); @@ -348,7 +424,6 @@ public abstract class AbstractJWTFilterTest { JWSHeader header = new JWSHeader.Builder(JWSAlgorithm.RS256).build(); SignedJWT signedJWT = new SignedJWT(header, claims); - Base64URL sigInput = Base64URL.encode(signedJWT.getSigningInput()); JWSSigner signer = new RSASSASigner(privateKey); signedJWT.sign(signer); @@ -396,11 +471,17 @@ public abstract class AbstractJWTFilterTest { public Enumeration<String> getInitParameterNames() { return null; } - + } - + protected static class TestJWTokenAuthority implements JWTokenAuthority { + private PublicKey verifyingKey; + + public TestJWTokenAuthority(PublicKey verifyingKey) { + this.verifyingKey = verifyingKey; + } + /* (non-Javadoc) * @see org.apache.hadoop.gateway.services.security.token.JWTokenAuthority#issueToken(javax.security.auth.Subject, java.lang.String) */ @@ -435,7 +516,8 @@ public abstract class AbstractJWTFilterTest { */ @Override public boolean verifyToken(JWTToken token) throws TokenServiceException { - return true; + JWSVerifier verifier = new RSASSAVerifier((RSAPublicKey) verifyingKey); + return token.verify(verifier); } /* (non-Javadoc) @@ -465,12 +547,12 @@ public abstract class AbstractJWTFilterTest { @Override public boolean verifyToken(JWTToken token, RSAPublicKey publicKey) throws TokenServiceException { - // TODO Auto-generated method stub - return true; + JWSVerifier verifier = new RSASSAVerifier(publicKey); + return token.verify(verifier); } - + } - + protected static class TestFilterChain implements FilterChain { boolean doFilterCalled = false; Subject subject = null; @@ -482,9 +564,9 @@ public abstract class AbstractJWTFilterTest { public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException { doFilterCalled = true; - + subject = Subject.getSubject( AccessController.getContext() ); } - + } } http://git-wip-us.apache.org/repos/asf/knox/blob/c7cbd46a/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/JWTFederationFilterTest.java ---------------------------------------------------------------------- diff --git a/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/JWTFederationFilterTest.java b/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/JWTFederationFilterTest.java index 8d41423..d19d999 100644 --- a/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/JWTFederationFilterTest.java +++ b/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/JWTFederationFilterTest.java @@ -29,19 +29,19 @@ import org.junit.Before; import com.nimbusds.jwt.SignedJWT; public class JWTFederationFilterTest extends AbstractJWTFilterTest { - + @Before public void setup() throws Exception, NoSuchAlgorithmException { super.setup(); handler = new TestJWTFederationFilter(); - ((TestJWTFederationFilter) handler).setTokenService(new TestJWTokenAuthority()); + ((TestJWTFederationFilter) handler).setTokenService(new TestJWTokenAuthority(publicKey)); } - + protected void setTokenOnRequest(HttpServletRequest request, SignedJWT jwt) { String token = "Bearer " + jwt.serialize(); EasyMock.expect(request.getHeader("Authorization")).andReturn(token); } - + protected void setGarbledTokenOnRequest(HttpServletRequest request, SignedJWT jwt) { String token = "Bearer " + "ljm" + jwt.serialize(); EasyMock.expect(request.getHeader("Authorization")).andReturn(token); @@ -50,18 +50,18 @@ public class JWTFederationFilterTest extends AbstractJWTFilterTest { protected String getAudienceProperty() { return TestJWTFederationFilter.KNOX_TOKEN_AUDIENCES; } - + private static class TestJWTFederationFilter extends JWTFederationFilter { public void setTokenService(JWTokenAuthority ts) { authority = ts; } - + } @Override protected String getVerificationPemProperty() { return TestJWTFederationFilter.TOKEN_VERIFICATION_PEM; }; - + } http://git-wip-us.apache.org/repos/asf/knox/blob/c7cbd46a/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/SSOCookieProviderTest.java ---------------------------------------------------------------------- diff --git a/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/SSOCookieProviderTest.java b/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/SSOCookieProviderTest.java index 27a3f31..85f7d59 100644 --- a/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/SSOCookieProviderTest.java +++ b/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/SSOCookieProviderTest.java @@ -42,21 +42,19 @@ import com.nimbusds.jwt.SignedJWT; public class SSOCookieProviderTest extends AbstractJWTFilterTest { private static final String SERVICE_URL = "https://localhost:8888/resource"; - private static final String REDIRECT_LOCATION = - "https://localhost:8443/authserver?originalUrl=" + SERVICE_URL; - + @Before public void setup() throws Exception, NoSuchAlgorithmException { super.setup(); handler = new TestSSOCookieFederationProvider(); - ((TestSSOCookieFederationProvider) handler).setTokenService(new TestJWTokenAuthority()); + ((TestSSOCookieFederationProvider) handler).setTokenService(new TestJWTokenAuthority(publicKey)); } - + protected void setTokenOnRequest(HttpServletRequest request, SignedJWT jwt) { Cookie cookie = new Cookie("hadoop-jwt", jwt.serialize()); EasyMock.expect(request.getCookies()).andReturn(new Cookie[] { cookie }); } - + protected void setGarbledTokenOnRequest(HttpServletRequest request, SignedJWT jwt) { Cookie cookie = new Cookie("hadoop-jwt", "ljm" + jwt.serialize()); EasyMock.expect(request.getCookies()).andReturn(new Cookie[] { cookie }); @@ -65,7 +63,7 @@ public class SSOCookieProviderTest extends AbstractJWTFilterTest { protected String getAudienceProperty() { return TestSSOCookieFederationProvider.SSO_EXPECTED_AUDIENCES; } - + @Test public void testCustomCookieNameJWT() throws Exception { try { @@ -112,47 +110,6 @@ public class SSOCookieProviderTest extends AbstractJWTFilterTest { se.getMessage().contains("authentication provider URL is missing"); } } - -/* - @Test - public void testFailedSignatureValidationJWT() throws Exception { - try { - - // Create a public key that doesn't match the one needed to - // verify the signature - in order to make it fail verification... - KeyPairGenerator kpg = KeyPairGenerator.getInstance("RSA"); - kpg.initialize(2048); - - KeyPair kp = kpg.genKeyPair(); - RSAPublicKey publicKey = (RSAPublicKey) kp.getPublic(); - - handler.setPublicKey(publicKey); - - Properties props = getProperties(); - handler.init(props); - - SignedJWT jwt = getJWT("bob", new Date(new Date().getTime() + 5000), - privateKey); - - Cookie cookie = new Cookie("hadoop-jwt", jwt.serialize()); - HttpServletRequest request = Mockito.mock(HttpServletRequest.class); - Mockito.when(request.getCookies()).thenReturn(new Cookie[] { cookie }); - Mockito.when(request.getRequestURL()).thenReturn( - new StringBuffer(SERVICE_URL)).anyTimes(); - HttpServletResponse response = Mockito.mock(HttpServletResponse.class); - Mockito.when(response.encodeRedirectURL(SERVICE_URL)).thenReturn( - SERVICE_URL); - - AuthenticationToken token = handler.alternateAuthenticate(request, - response); - Mockito.verify(response).sendRedirect(REDIRECT_LOCATION); - } catch (ServletException se) { - fail("alternateAuthentication should NOT have thrown a ServletException"); - } catch (AuthenticationException ae) { - fail("alternateAuthentication should NOT have thrown a AuthenticationException"); - } - } -*/ @Test public void testOrigURLWithQueryString() throws Exception { @@ -185,7 +142,7 @@ public class SSOCookieProviderTest extends AbstractJWTFilterTest { Assert.assertNotNull("LoginURL should not be null.", loginURL); Assert.assertEquals("https://localhost:8443/authserver?originalUrl=" + SERVICE_URL, loginURL); } - + @Override protected String getVerificationPemProperty() { @@ -196,7 +153,7 @@ public class SSOCookieProviderTest extends AbstractJWTFilterTest { public String testConstructLoginURL(HttpServletRequest req) { return constructLoginURL(req); } - + public void setTokenService(JWTokenAuthority ts) { authority = ts; }