Add expiration concept to data plane tokens

3 files changed, 75 insertions, 14 deletions

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 @@
             <artifactId>jetty-util</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>com.yahoo.vespa</groupId>
+            <artifactId>testutil</artifactId>
+            <version>${project.version}</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>
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<Client> allowedClients;
     private final TokenDomain tokenDomain;
+    private final Clock clock;
 
     @Inject
     public CloudDataPlaneFilter(CloudDataPlaneFilterConfig cfg,
                                 ComponentRegistry<DataplaneProxyCredentials> 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<Client> parseClients(CloudDataPlaneFilterConfig cfg, X509Certificate reverseProxyCert) {
+    private static List<Client> parseClients(CloudDataPlaneFilterConfig cfg, X509Certificate reverseProxyCert, Clock clock) {
+        var now = clock.instant();
         Set<String> ids = new HashSet<>();
         List<Client> 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<ErrorResponse> 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("<none>"));
         }
         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<Instant> expiration) {
+        static TokenVersion of(String id, String fingerprint, String accessHash, String expiration) {
+            return new TokenVersion(id, TokenFingerprint.ofHex(fingerprint), TokenCheckHash.ofHex(accessHash),
+                                    expiration.equals("<none>") ? 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) {