You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@knox.apache.org by pz...@apache.org on 2021/04/01 20:16:50 UTC
[knox] branch master updated: KNOX-2566 - JWT Token Signature
Verification Caching NPE (#427)
This is an automated email from the ASF dual-hosted git repository.
pzampino pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git
The following commit(s) were added to refs/heads/master by this push:
new bcce97b KNOX-2566 - JWT Token Signature Verification Caching NPE (#427)
bcce97b is described below
commit bcce97b7b83d0086bba113321c8bfff50f1b3e8a
Author: Phil Zampino <pz...@apache.org>
AuthorDate: Thu Apr 1 16:16:44 2021 -0400
KNOX-2566 - JWT Token Signature Verification Caching NPE (#427)
---
.../provider/federation/jwt/JWTMessages.java | 4 ++
.../federation/jwt/filter/AbstractJWTFilter.java | 28 +++++---
.../federation/jwt/filter/JWTFederationFilter.java | 3 +-
.../provider/federation/AbstractJWTFilterTest.java | 79 +++++++++++++++++++---
.../JWTAsHTTPBasicCredsFederationFilterTest.java | 3 +-
...okenIDAsHTTPBasicCredsFederationFilterTest.java | 32 +++++++--
6 files changed, 121 insertions(+), 28 deletions(-)
diff --git a/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/JWTMessages.java b/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/JWTMessages.java
index 688a2ab..8a01811 100644
--- a/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/JWTMessages.java
+++ b/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/JWTMessages.java
@@ -69,4 +69,8 @@ public interface JWTMessages {
text = "The configuration value ({0}) for maximum token verification cache is invalid; Using the default value." )
void invalidVerificationCacheMaxConfiguration(String value);
+ @Message( level = MessageLevel.ERROR,
+ text = "Missing token passcode." )
+ void missingTokenPasscode();
+
}
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 2e37526..45a3ce6 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
@@ -357,10 +357,12 @@ public abstract class AbstractJWTFilter implements Filter {
} else {
log.tokenHasExpired(displayableToken, displayableTokenId);
- // 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(tokenId);
+ if (tokenId != null) {
+ // 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(tokenId);
+ }
handleValidationError(request, response, HttpServletResponse.SC_BAD_REQUEST,
"Bad request: token has expired");
@@ -385,12 +387,18 @@ public abstract class AbstractJWTFilter implements Filter {
if (tokenStateService != null) {
try {
- if (tokenIsStillValid(tokenId)) {
- return true;
+ if (tokenId != null) {
+ if (tokenIsStillValid(tokenId)) {
+ return true;
+ } else {
+ log.tokenHasExpired(Tokens.getTokenIDDisplayText(tokenId));
+ handleValidationError(request, response, HttpServletResponse.SC_BAD_REQUEST,
+ "Bad request: token has expired");
+ }
} else {
- log.tokenHasExpired(Tokens.getTokenIDDisplayText(tokenId));
+ log.missingTokenPasscode();
handleValidationError(request, response, HttpServletResponse.SC_BAD_REQUEST,
- "Bad request: token has expired");
+ "Bad request: missing token passcode.");
}
} catch (UnknownTokenException e) {
log.unableToVerifyExpiration(e);
@@ -407,7 +415,7 @@ public abstract class AbstractJWTFilter implements Filter {
String tokenId = TokenUtils.getTokenId(token);
// Check if the token has already been verified
- verified = hasSignatureBeenVerified(tokenId);
+ verified = (tokenId != null) && hasSignatureBeenVerified(tokenId);
// If it has not yet been verified, then perform the verification now
if (!verified) {
@@ -436,7 +444,7 @@ public abstract class AbstractJWTFilter implements Filter {
}
}
- if (verified) { // If successful, record the verification for future reference
+ if (verified && tokenId != null) { // If successful, record the verification for future reference
recordSignatureVerification(tokenId);
}
}
diff --git a/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTFederationFilter.java b/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTFederationFilter.java
index a8d50df..aa82062 100644
--- a/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTFederationFilter.java
+++ b/gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/JWTFederationFilter.java
@@ -167,8 +167,9 @@ public class JWTFederationFilter extends AbstractJWTFilter {
final String credentials = new String(credDecoded, StandardCharsets.UTF_8);
final String[] values = credentials.split(":", 2);
String username = values[0];
+ String passcode = values[1].isEmpty() ? null : values[1];
if (TOKEN.equalsIgnoreCase(username) || PASSCODE.equalsIgnoreCase(username)) {
- parsed = Pair.of(TOKEN.equalsIgnoreCase(username) ? TokenType.JWT : TokenType.Passcode, values[1]);
+ parsed = Pair.of(TOKEN.equalsIgnoreCase(username) ? TokenType.JWT : TokenType.Passcode, passcode);
}
return parsed;
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 6d71a18..16b1811 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
@@ -64,6 +64,7 @@ import java.security.cert.Certificate;
import java.security.interfaces.RSAPrivateKey;
import java.security.interfaces.RSAPublicKey;
import java.text.MessageFormat;
+import java.time.Instant;
import java.util.Date;
import java.util.Enumeration;
import java.util.Locale;
@@ -115,6 +116,50 @@ public abstract class AbstractJWTFilterTest {
handler.destroy();
}
+ /**
+ * KNOX-2566
+ */
+ @Test
+ public void testJWTWithoutKnoxUUIDClaim() throws Exception {
+ doTestJWTWithoutKnoxUUIDClaim(Instant.now().toEpochMilli() + 5000);
+ }
+
+ /**
+ * KNOX-2566
+ * Covers the explicit removal of signature verification cache records for expired tokens.
+ */
+ @Test
+ public void testExpiredJWTWithoutKnoxUUIDClaim() throws Exception {
+ doTestJWTWithoutKnoxUUIDClaim(Instant.now().toEpochMilli() - 5000);
+ }
+
+ private void doTestJWTWithoutKnoxUUIDClaim(final long expiration) throws Exception {
+ Properties props = getProperties();
+ handler.init(new TestFilterConfig(props));
+
+ SignedJWT jwt = getJWT(AbstractJWTFilter.JWT_DEFAULT_ISSUER,
+ "alice",
+ "bar",
+ new Date(expiration),
+ new Date(),
+ privateKey,
+ JWSAlgorithm.RS256.getName(),
+ null); // null knox ID so the claim will be omitted from the token
+
+ 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.expect(response.getOutputStream()).andAnswer(DummyServletOutputStream::new).anyTimes();
+ EasyMock.replay(request, response);
+
+ TestFilterChain chain = new TestFilterChain();
+ handler.doFilter(request, response, chain);
+ }
+
@Test
public void testValidJWT() throws Exception {
try {
@@ -154,7 +199,6 @@ public abstract class AbstractJWTFilterTest {
SignedJWT jwt = getJWT(AbstractJWTFilter.JWT_DEFAULT_ISSUER, "alice",
new Date(new Date().getTime() + 5000), privateKey);
-
HttpServletRequest request = EasyMock.createNiceMock(HttpServletRequest.class);
setTokenOnRequest(request, jwt);
@@ -880,17 +924,30 @@ public abstract class AbstractJWTFilterTest {
}
protected SignedJWT getJWT(String issuer, String sub, String aud, Date expires, Date nbf, RSAPrivateKey privateKey,
- String signatureAlgorithm)
+ String signatureAlgorithm) throws Exception {
+ return getJWT(issuer, sub, aud, expires, nbf, privateKey, signatureAlgorithm, String.valueOf(UUID.randomUUID()));
+ }
+
+ protected SignedJWT getJWT(final String issuer,
+ final String sub,
+ final String aud,
+ final Date expires,
+ final Date nbf,
+ final RSAPrivateKey privateKey,
+ final String signatureAlgorithm,
+ final String knoxId)
throws Exception {
- JWTClaimsSet claims = new JWTClaimsSet.Builder()
- .issuer(issuer)
- .subject(sub)
- .audience(aud)
- .expirationTime(expires)
- .notBeforeTime(nbf)
- .claim("scope", "openid")
- .claim(JWTToken.KNOX_ID_CLAIM, String.valueOf(UUID.randomUUID()))
- .build();
+ JWTClaimsSet.Builder builder = new JWTClaimsSet.Builder();
+ builder.issuer(issuer)
+ .subject(sub)
+ .audience(aud)
+ .expirationTime(expires)
+ .notBeforeTime(nbf)
+ .claim("scope", "openid");
+ if (knoxId != null) {
+ builder.claim(JWTToken.KNOX_ID_CLAIM, knoxId);
+ }
+ JWTClaimsSet claims = builder.build();
JWSHeader header = new JWSHeader.Builder(JWSAlgorithm.parse(signatureAlgorithm)).build();
diff --git a/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/JWTAsHTTPBasicCredsFederationFilterTest.java b/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/JWTAsHTTPBasicCredsFederationFilterTest.java
index dbfe98f..0de8828 100644
--- a/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/JWTAsHTTPBasicCredsFederationFilterTest.java
+++ b/gateway-provider-security-jwt/src/test/java/org/apache/knox/gateway/provider/federation/JWTAsHTTPBasicCredsFederationFilterTest.java
@@ -77,7 +77,8 @@ public class JWTAsHTTPBasicCredsFederationFilterTest extends AbstractJWTFilterTe
protected void setTokenOnRequest(final HttpServletRequest request,
final String authUsername,
final String authPassword) {
- final byte[] basicAuth = (authUsername + ":" + authPassword).getBytes(StandardCharsets.UTF_8);
+ final byte[] basicAuth =
+ (authUsername + ":" + (authPassword != null ? authPassword : "")).getBytes(StandardCharsets.UTF_8);
final String authHeaderValue = "Basic " + Base64.getEncoder().encodeToString(basicAuth);
EasyMock.expect((Object)request.getHeader("Authorization")).andReturn(authHeaderValue);
}
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 a439143..157aabe 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
@@ -37,6 +37,7 @@ import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.text.ParseException;
+import java.time.Instant;
import java.util.Date;
import java.util.Locale;
import java.util.Map;
@@ -224,6 +225,16 @@ public class TokenIDAsHTTPBasicCredsFederationFilterTest extends JWTAsHTTPBasicC
}
@Override
+ public void testJWTWithoutKnoxUUIDClaim() throws Exception {
+ // Override to disable N/A test
+ }
+
+ @Override
+ public void testExpiredJWTWithoutKnoxUUIDClaim() throws Exception {
+ // Override to disable N/A test
+ }
+
+ @Override
public void testInvalidAudienceJWT() throws Exception {
// Override to disable N/A test
}
@@ -339,23 +350,34 @@ public class TokenIDAsHTTPBasicCredsFederationFilterTest extends JWTAsHTTPBasicC
if (expiration == null || expiration.isEmpty()) {
expiration = "0";
}
- tokenExpirations.put(TokenUtils.getTokenId(token), Long.parseLong(expiration));
+ addToken(TokenUtils.getTokenId(token), Instant.now().toEpochMilli(), Long.parseLong(expiration));
+ }
+
+ private void addToken(String tokenId, long expiration) {
+ tokenExpirations.put(tokenId, expiration);
}
@Override
public void addToken(String tokenId, long issueTime, long expiration) {
- tokenExpirations.put(tokenId, expiration);
+ addToken(tokenId, expiration);
}
@Override
public void addToken(String tokenId, long issueTime, long expiration, long maxLifetimeDuration) {
- tokenExpirations.put(tokenId, expiration);
+ addToken(tokenId, expiration);
}
@Override
public boolean isExpired(JWTToken token) throws UnknownTokenException {
- Long expiration = tokenExpirations.get(TokenUtils.getTokenId(token));
- return (new Date(expiration).before(new Date()));
+ Long expiration;
+ String tokenId = TokenUtils.getTokenId(token);
+ if (tokenId != null) {
+ expiration = tokenExpirations.get(tokenId);
+ } else {
+ expiration = Long.parseLong(token.getExpires());
+ }
+
+ return Instant.ofEpochMilli(expiration).isBefore(Instant.now());
}
@Override