From 876ca0b666b01fd88be644a1e31065c5707254f0 Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Wed, 12 Jul 2023 13:32:20 +0200 Subject: Add expiration concept to data plane tokens --- jdisc-security-filters/pom.xml | 6 +++ .../security/cloud/CloudDataPlaneFilter.java | 34 +++++++++++---- .../security/cloud/CloudDataPlaneFilterTest.java | 49 +++++++++++++++++++--- 3 files changed, 75 insertions(+), 14 deletions(-) (limited to 'jdisc-security-filters') diff --git a/jdisc-security-filters/pom.xml b/jdisc-security-filters/pom.xml index 652b864747d..3440f9089d7 100644 --- a/jdisc-security-filters/pom.xml +++ b/jdisc-security-filters/pom.xml @@ -63,6 +63,12 @@ jetty-util test + + com.yahoo.vespa + testutil + ${project.version} + test + diff --git a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/cloud/CloudDataPlaneFilter.java b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/cloud/CloudDataPlaneFilter.java index 96602fcd899..554c1d924a2 100644 --- a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/cloud/CloudDataPlaneFilter.java +++ b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/cloud/CloudDataPlaneFilter.java @@ -20,6 +20,8 @@ import com.yahoo.security.token.TokenFingerprint; import java.security.Principal; import java.security.cert.X509Certificate; +import java.time.Clock; +import java.time.Instant; import java.util.ArrayList; import java.util.EnumSet; import java.util.HashMap; @@ -53,21 +55,23 @@ public class CloudDataPlaneFilter extends JsonSecurityRequestFilterBase { private final boolean legacyMode; private final List allowedClients; private final TokenDomain tokenDomain; + private final Clock clock; @Inject public CloudDataPlaneFilter(CloudDataPlaneFilterConfig cfg, ComponentRegistry optionalReverseProxy) { - this(cfg, reverseProxyCert(optionalReverseProxy).orElse(null)); + this(cfg, reverseProxyCert(optionalReverseProxy).orElse(null), Clock.systemUTC()); } - CloudDataPlaneFilter(CloudDataPlaneFilterConfig cfg, X509Certificate reverseProxyCert) { + CloudDataPlaneFilter(CloudDataPlaneFilterConfig cfg, X509Certificate reverseProxyCert, Clock clock) { this.legacyMode = cfg.legacyMode(); this.tokenDomain = TokenDomain.of(cfg.tokenContext()); + this.clock = clock; if (legacyMode) { allowedClients = List.of(); log.fine(() -> "Legacy mode enabled"); } else { - allowedClients = parseClients(cfg, reverseProxyCert); + allowedClients = parseClients(cfg, reverseProxyCert, clock); } } @@ -76,7 +80,8 @@ public class CloudDataPlaneFilter extends JsonSecurityRequestFilterBase { return optionalReverseProxy.allComponents().stream().findAny().map(DataplaneProxyCredentials::certificate); } - private static List parseClients(CloudDataPlaneFilterConfig cfg, X509Certificate reverseProxyCert) { + private static List parseClients(CloudDataPlaneFilterConfig cfg, X509Certificate reverseProxyCert, Clock clock) { + var now = clock.instant(); Set ids = new HashSet<>(); List clients = new ArrayList<>(cfg.clients().size()); boolean hasClientRequiringCertificate = false; @@ -112,8 +117,14 @@ public class CloudDataPlaneFilter extends JsonSecurityRequestFilterBase { for (var token : c.tokens()) { for (int version = 0; version < token.checkAccessHashes().size(); version++) { var tokenVersion = TokenVersion.of( - token.id(), token.fingerprints().get(version), token.checkAccessHashes().get(version)); - tokens.put(tokenVersion.accessHash(), tokenVersion); + token.id(), token.fingerprints().get(version), token.checkAccessHashes().get(version), + token.expirations().get(version)); + var expiration = tokenVersion.expiration().orElse(null); + if (expiration != null && now.isAfter(expiration)) + log.fine(() -> "Ignoring expired version %s of token '%s' (expiration=%s)".formatted( + tokenVersion.fingerprint(), tokenVersion.id(), expiration)); + else + tokens.put(tokenVersion.accessHash(), tokenVersion); } } // Add reverse proxy certificate as required certificate for client definition @@ -128,6 +139,7 @@ public class CloudDataPlaneFilter extends JsonSecurityRequestFilterBase { @Override protected Optional filter(DiscFilterRequest req) { + var now = clock.instant(); var certs = req.getClientCertificateChain(); log.fine(() -> "Certificate chain contains %d elements".formatted(certs.size())); if (certs.isEmpty()) { @@ -164,6 +176,8 @@ public class CloudDataPlaneFilter extends JsonSecurityRequestFilterBase { if (requestTokenHash == null) continue; var matchedToken = c.tokens().get(requestTokenHash); if (matchedToken == null) continue; + var expiration = matchedToken.expiration().orElse(null); + if (expiration != null && now.isAfter(expiration)) continue; matchedTokens.add(matchedToken); } clientIds.add(c.id()); @@ -178,6 +192,7 @@ public class CloudDataPlaneFilter extends JsonSecurityRequestFilterBase { if (matchedToken != null) { addAccessLogEntry(req, "token.id", matchedToken.id()); addAccessLogEntry(req, "token.hash", matchedToken.fingerprint().toDelimitedHexString()); + addAccessLogEntry(req, "token.exp", matchedToken.expiration().map(Instant::toString).orElse("")); } log.fine(() -> "Client with ids=%s, permissions=%s" .formatted(clientIds, permissions.stream().map(Permission::asString).toList())); @@ -225,9 +240,10 @@ public class CloudDataPlaneFilter extends JsonSecurityRequestFilterBase { } } - private record TokenVersion(String id, TokenFingerprint fingerprint, TokenCheckHash accessHash) { - static TokenVersion of(String id, String fingerprint, String accessHash) { - return new TokenVersion(id, TokenFingerprint.ofHex(fingerprint), TokenCheckHash.ofHex(accessHash)); + private record TokenVersion(String id, TokenFingerprint fingerprint, TokenCheckHash accessHash, Optional expiration) { + static TokenVersion of(String id, String fingerprint, String accessHash, String expiration) { + return new TokenVersion(id, TokenFingerprint.ofHex(fingerprint), TokenCheckHash.ofHex(accessHash), + expiration.equals("") ? Optional.empty() : Optional.of(Instant.parse(expiration))); } } diff --git a/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/cloud/CloudDataPlaneFilterTest.java b/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/cloud/CloudDataPlaneFilterTest.java index d05baccc069..d9daf8b6f46 100644 --- a/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/cloud/CloudDataPlaneFilterTest.java +++ b/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/cloud/CloudDataPlaneFilterTest.java @@ -17,11 +17,15 @@ import com.yahoo.security.token.Token; import com.yahoo.security.token.TokenCheckHash; import com.yahoo.security.token.TokenDomain; import com.yahoo.security.token.TokenGenerator; +import com.yahoo.test.ManualClock; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import javax.security.auth.x500.X500Principal; import java.math.BigInteger; import java.security.cert.X509Certificate; +import java.time.Duration; +import java.time.Instant; import java.util.List; import java.util.Set; @@ -52,11 +56,16 @@ class CloudDataPlaneFilterTest { private static final String TOKEN_SEARCH_CLIENT = "token-search-client"; private static final String TOKEN_CONTEXT = "my-token-context"; private static final String TOKEN_ID = "my-token-id"; + private static final Instant TOKEN_EXPIRATION = EPOCH.plus(Duration.ofDays(1)); private static final Token VALID_TOKEN = TokenGenerator.generateToken(TokenDomain.of(TOKEN_CONTEXT), "vespa_token_", CHECK_HASH_BYTES); private static final Token UNKNOWN_TOKEN = TokenGenerator.generateToken(TokenDomain.of(TOKEN_CONTEXT), "vespa_token_", CHECK_HASH_BYTES); + private ManualClock clock; + + @BeforeEach void resetClock() { clock = new ManualClock(EPOCH); } + @Test void accepts_any_trusted_client_certificate_in_legacy_mode() { var req = FilterTestUtils.newRequestBuilder().withClientCertificate(LEGACY_CLIENT_CERT).build(); @@ -150,6 +159,7 @@ class CloudDataPlaneFilterTest { assertEquals(new ClientPrincipal(Set.of(TOKEN_SEARCH_CLIENT), Set.of(READ)), req.getUserPrincipal()); assertEquals(TOKEN_ID, entry.getKeyValues().get("token.id").get(0)); assertEquals(VALID_TOKEN.fingerprint().toDelimitedHexString(), entry.getKeyValues().get("token.hash").get(0)); + assertEquals(TOKEN_EXPIRATION.toString(), entry.getKeyValues().get("token.exp").get(0)); } @Test @@ -228,13 +238,40 @@ class CloudDataPlaneFilterTest { assertEquals(new ClientPrincipal(Set.of(FEED_CLIENT_ID), Set.of(WRITE)), req.getUserPrincipal()); } - private static CloudDataPlaneFilter newFilterWithLegacyMode() { + @Test + void fails_for_expired_token() { + var entry = new AccessLogEntry(); + var req = FilterTestUtils.newRequestBuilder() + .withMethod(Method.GET) + .withAccessLogEntry(entry) + .withClientCertificate(REVERSE_PROXY_CERT) + .withHeader("Authorization", "Bearer " + VALID_TOKEN.secretTokenString()) + .build(); + var filter = newFilterWithClientsConfig(); + + var responseHandler = new MockResponseHandler(); + filter.filter(req, responseHandler); + assertNull(responseHandler.getResponse()); + + clock.advance(Duration.ofDays(1)); + responseHandler = new MockResponseHandler(); + filter.filter(req, responseHandler); + assertNull(responseHandler.getResponse()); + + clock.advance(Duration.ofMillis(1)); + responseHandler = new MockResponseHandler(); + filter.filter(req, responseHandler); + assertNotNull(responseHandler.getResponse()); + assertEquals(FORBIDDEN, responseHandler.getResponse().getStatus()); + } + + private CloudDataPlaneFilter newFilterWithLegacyMode() { return new CloudDataPlaneFilter( new CloudDataPlaneFilterConfig.Builder() - .legacyMode(true).build(), (X509Certificate) null); + .legacyMode(true).build(), (X509Certificate) null, clock); } - private static CloudDataPlaneFilter newFilterWithClientsConfig() { + private CloudDataPlaneFilter newFilterWithClientsConfig() { return new CloudDataPlaneFilter( new CloudDataPlaneFilterConfig.Builder() .tokenContext(TOKEN_CONTEXT) @@ -251,11 +288,13 @@ class CloudDataPlaneFilterTest { .tokens(new CloudDataPlaneFilterConfig.Clients.Tokens.Builder() .id(TOKEN_ID) .checkAccessHashes(TokenCheckHash.of(VALID_TOKEN, 32).toHexString()) - .fingerprints(VALID_TOKEN.fingerprint().toDelimitedHexString())) + .fingerprints(VALID_TOKEN.fingerprint().toDelimitedHexString()) + .expirations(TOKEN_EXPIRATION.toString())) .permissions(READ.asString()) .id(TOKEN_SEARCH_CLIENT))) .build(), - REVERSE_PROXY_CERT); + REVERSE_PROXY_CERT, + clock); } private static X509Certificate certificate(String name) { -- cgit v1.2.3