This is an automated email from the ASF dual-hosted git repository. hanicz pushed a commit to branch v3.0.0 in repository https://gitbox.apache.org/repos/asf/knox.git
commit e065a13bce43248315d8570e1b4b15019b517e02 Author: hanicz <[email protected]> AuthorDate: Wed Aug 12 15:26:22 2026 +0200 KNOX-3413: KnoxToken passcode verification accepts a valid passcode for a different token (#1345) (cherry picked from commit 5342483a2c7ff229d8819d7df479980753640fc1) --- .../federation/jwt/filter/AbstractJWTFilter.java | 9 ++-- .../provider/federation/AbstractJWTFilterTest.java | 3 ++ .../federation/JWTFederationFilterTest.java | 2 +- .../federation/OAuthFlowsFederationFilterTest.java | 9 +++- ...okenIDAsHTTPBasicCredsFederationFilterTest.java | 50 ++++++++++++++++++++++ 5 files changed, 67 insertions(+), 6 deletions(-) diff --git a/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AbstractJWTFilter.java b/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AbstractJWTFilter.java index de4987caa..502928891 100644 --- a/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AbstractJWTFilter.java +++ b/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AbstractJWTFilter.java @@ -568,7 +568,7 @@ public abstract class AbstractJWTFilter implements Filter { final TokenMetadata tokenMetadata = tokenStateService == null ? null : tokenStateService.getTokenMetadata(tokenId); if (isTokenEnabled(tokenMetadata)) { if (isIdleTimeoutLimitNotExceeded(tokenMetadata)) { - if (hasSignatureBeenVerified(passcode) || validatePasscode(tokenId, passcode)) { + if (hasSignatureBeenVerified(passcodeVerificationCacheKey(tokenId, passcode)) || validatePasscode(tokenId, passcode)) { markLastUsedAt(tokenId, tokenMetadata); return true; } else { @@ -589,7 +589,7 @@ public abstract class AbstractJWTFilter implements Filter { // Explicitly evict the record of this token's signature verification (if present). // There is no value in keeping this record for expired tokens, and explicitly removing them may prevent // records for other valid tokens from being prematurely evicted from the cache. - removeSignatureVerificationRecord(passcode); + removeSignatureVerificationRecord(passcodeVerificationCacheKey(tokenId, passcode)); handleValidationError(request, response, HttpServletResponse.SC_UNAUTHORIZED, "Token has expired"); } } else { @@ -615,7 +615,7 @@ public abstract class AbstractJWTFilter implements Filter { final byte[] storedPasscode = tokenMetadata == null ? null : tokenMetadata.getPasscode().getBytes(UTF_8); final boolean validPasscode = Arrays.equals(tokenMAC.hash(tokenId, issueTime, userName, passcode).getBytes(UTF_8), storedPasscode); if (validPasscode) { - recordSignatureVerification(passcode); + recordSignatureVerification(passcodeVerificationCacheKey(tokenId, passcode)); } return validPasscode; } @@ -707,4 +707,7 @@ public abstract class AbstractJWTFilter implements Filter { protected abstract void handleValidationError(HttpServletRequest request, HttpServletResponse response, int status, String error) throws IOException; + private String passcodeVerificationCacheKey(final String tokenId, final String passcode) { + return tokenId + "::" + passcode; + } } diff --git a/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/AbstractJWTFilterTest.java b/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/AbstractJWTFilterTest.java index f025aa277..16f71d959 100644 --- a/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/AbstractJWTFilterTest.java +++ b/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/AbstractJWTFilterTest.java @@ -1520,4 +1520,7 @@ public abstract class AbstractJWTFilterTest { } } + protected String passcodeVerificationCacheKey(final String tokenId, final String passcode) { + return tokenId + "::" + passcode; + } } diff --git a/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/JWTFederationFilterTest.java b/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/JWTFederationFilterTest.java index 2ea5524b9..6c66d44a3 100644 --- a/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/JWTFederationFilterTest.java +++ b/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/JWTFederationFilterTest.java @@ -195,7 +195,7 @@ public class JWTFederationFilterTest extends AbstractJWTFilterTest { } EasyMock.replay(tokenStateService, tokenMetadata, request, response); - SignatureVerificationCache.getInstance(topologyName, filterConfig).recordSignatureVerification(passcode); + SignatureVerificationCache.getInstance(topologyName, filterConfig).recordSignatureVerification(passcodeVerificationCacheKey(tokenId, passcode)); final TestFilterChain chain = new TestFilterChain(); handler.doFilter(request, response, chain); diff --git a/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/OAuthFlowsFederationFilterTest.java b/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/OAuthFlowsFederationFilterTest.java index 5fc257e05..6fc873f68 100644 --- a/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/OAuthFlowsFederationFilterTest.java +++ b/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/OAuthFlowsFederationFilterTest.java @@ -278,7 +278,7 @@ public class OAuthFlowsFederationFilterTest extends TokenIDAsHTTPBasicCredsFeder // Wrap the request to simulate real-world scenario where wrappers hide parameter access final HttpServletRequest request = new TestServletRequestWrapper(mockRequest); - SignatureVerificationCache.getInstance(topologyName, filterConfig).recordSignatureVerification(passcode); + SignatureVerificationCache.getInstance(topologyName, filterConfig).recordSignatureVerification(passcodeVerificationCacheKey(tokenId, passcode)); final TestFilterChain chain = new TestFilterChain(); handler.doFilter(request, response, chain); @@ -387,6 +387,11 @@ public class OAuthFlowsFederationFilterTest extends TokenIDAsHTTPBasicCredsFeder public void testUnableToParseJWT() throws Exception { } + @Override + @Test + public void testPasscodeCannotBeReplayedAgainstDifferentTokenId() { + } + @Test public void testGetWireTokenUsingRefreshTokenFlow() throws Exception { final String refreshToken = "WTJ4cFpXNTBMV2xrTFRFeU16UTE6OlkyeHBaVzUwTFhObFkzSmxkQzB4TWpNME5RPT0="; @@ -470,7 +475,7 @@ public class OAuthFlowsFederationFilterTest extends TokenIDAsHTTPBasicCredsFeder // Wrap the request to simulate real-world scenario where wrappers hide parameter access final HttpServletRequest request = new TestServletRequestWrapper(mockRequest); - SignatureVerificationCache.getInstance("jwt-topology", filterConfig).recordSignatureVerification(passcode); + SignatureVerificationCache.getInstance("jwt-topology", filterConfig).recordSignatureVerification(passcodeVerificationCacheKey(tokenId, passcode)); final TestFilterChain chain = new TestFilterChain(); handler.doFilter(request, response, chain); diff --git a/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/TokenIDAsHTTPBasicCredsFederationFilterTest.java b/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/TokenIDAsHTTPBasicCredsFederationFilterTest.java index b92158f89..dadd41cff 100644 --- a/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/TokenIDAsHTTPBasicCredsFederationFilterTest.java +++ b/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/TokenIDAsHTTPBasicCredsFederationFilterTest.java @@ -20,6 +20,7 @@ package org.apache.knox.gateway.provider.federation; import static org.junit.Assert.fail; +import java.io.IOException; import java.nio.charset.StandardCharsets; import java.text.ParseException; import java.time.Instant; @@ -265,6 +266,55 @@ public class TokenIDAsHTTPBasicCredsFederationFilterTest extends JWTAsHTTPBasicC } } + @Test + public void testPasscodeCannotBeReplayedAgainstDifferentTokenId() throws Exception { + Properties props = getProperties(); + handler.init(new TestFilterConfig(props)); + + final long issueTime = System.currentTimeMillis() - TimeUnit.MINUTES.toMillis(5); + final Date expiry = new Date(System.currentTimeMillis() + TimeUnit.MINUTES.toMillis(5)); + + final SignedJWT attackerJwt = getJWT(AbstractJWTFilter.JWT_DEFAULT_ISSUER, "attacker", expiry, privateKey); + final String attackerPasscode = (String) attackerJwt.getJWTClaimsSet().getClaims().get(PASSCODE_CLAIM); + addTokenState(attackerJwt, issueTime, "attacker", attackerPasscode); + + final SignedJWT victimJwt = getJWT(AbstractJWTFilter.JWT_DEFAULT_ISSUER, "bob", expiry, privateKey); + final String victimPasscode = (String) victimJwt.getJWTClaimsSet().getClaims().get(PASSCODE_CLAIM); + addTokenState(victimJwt, issueTime, "bob", victimPasscode); + final String victimTokenId = getTokenId(victimJwt); + + final TestFilterChain seedChain = new TestFilterChain(); + handler.doFilter(newPasscodeRequest(generatePasscodeField(getTokenId(attackerJwt), attackerPasscode)), + newResponse(), seedChain); + Assert.assertTrue("Precondition: the attacker's own passcode should authenticate.", seedChain.doFilterCalled); + + final TestFilterChain attackChain = new TestFilterChain(); + handler.doFilter(newPasscodeRequest(generatePasscodeField(victimTokenId, attackerPasscode)), + newResponse(), attackChain); + + Assert.assertFalse("A passcode must not authenticate when paired with a different token id " + + "(identity-assertion / authentication bypass).", attackChain.doFilterCalled); + Assert.assertNull("No subject should have been established for the replayed passcode.", attackChain.getSubject()); + } + + private HttpServletRequest newPasscodeRequest(final String passcodeField) { + final HttpServletRequest request = EasyMock.createNiceMock(HttpServletRequest.class); + setTokenOnRequest(request, JWTFederationFilter.PASSCODE, passcodeField); + EasyMock.expect(request.getRequestURL()).andReturn(new StringBuffer(SERVICE_URL)).anyTimes(); + EasyMock.expect(request.getPathInfo()).andReturn("resource").anyTimes(); + EasyMock.expect(request.getQueryString()).andReturn(null).anyTimes(); + EasyMock.replay(request); + return request; + } + + private HttpServletResponse newResponse() throws IOException { + final HttpServletResponse response = EasyMock.createNiceMock(HttpServletResponse.class); + EasyMock.expect(response.encodeRedirectURL(SERVICE_URL)).andReturn(SERVICE_URL).anyTimes(); + EasyMock.expect(response.getOutputStream()).andAnswer(DummyServletOutputStream::new).anyTimes(); + EasyMock.replay(response); + return response; + } + @Override public void testJWTWithoutKnoxUUIDClaim() throws Exception { // Override to disable N/A test
