use refreshed certificates when available (and feature flag is set)

5 files changed, 143 insertions, 32 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index fe1aaa001b1..3ade72b020c 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -17,6 +17,7 @@ import com.yahoo.vespa.athenz.api.AthenzPrincipal;
 import com.yahoo.vespa.athenz.api.AthenzService;
 import com.yahoo.vespa.athenz.api.AthenzUser;
 import com.yahoo.vespa.curator.Lock;
+import com.yahoo.vespa.flags.FlagSource;
 import com.yahoo.vespa.hosted.controller.api.ActivateResult;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.DeployOptions;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.ConfigChangeActions;
@@ -106,8 +107,10 @@ public class ApplicationController {
     private final ApplicationPackageValidator applicationPackageValidator;
     private final EndpointCertificateManager endpointCertificateManager;
 
-    ApplicationController(Controller controller, CuratorDb curator, AccessControl accessControl, Clock clock,
-                          SecretStore secretStore) {
+    ApplicationController(Controller controller, CuratorDb curator,
+	                  AccessControl accessControl, Clock clock,
+			  SecretStore secretStore, FlagSource flagSource) {
+
         this.controller = controller;
         this.curator = curator;
         this.accessControl = accessControl;
@@ -119,7 +122,7 @@ public class ApplicationController {
         deploymentTrigger = new DeploymentTrigger(controller, clock);
         applicationPackageValidator = new ApplicationPackageValidator(controller);
         endpointCertificateManager = new EndpointCertificateManager(controller.zoneRegistry(), curator, secretStore,
-                controller.serviceRegistry().applicationCertificateProvider(), clock);
+                controller.serviceRegistry().applicationCertificateProvider(), clock, flagSource);
 
         // Update serialization format of all applications
         Once.after(Duration.ofMinutes(1), () -> {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
index b4136e0de99..3fa017ef261 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java
@@ -103,8 +103,8 @@ public class Controller extends AbstractComponent implements ApplicationIdSource
         metrics = new ConfigServerMetrics(serviceRegistry.configServer());
         nameServiceForwarder = new NameServiceForwarder(curator);
         jobController = new JobController(this);
-        applicationController = new ApplicationController(this, curator, accessControl, clock, secretStore);
-        tenantController = new TenantController(this, curator, accessControl);
+	applicationController = new ApplicationController(this, curator, accessControl, clock, secretStore, flagSource);
+	tenantController = new TenantController(this, curator, accessControl);
         routingController = new RoutingController(this, Objects.requireNonNull(rotationsConfig, "RotationsConfig cannot be null"));
         auditLogger = new AuditLogger(curator, clock);
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/endpointcertificates/EndpointCertificateManager.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/endpointcertificates/EndpointCertificateManager.java
index ad8c0c73754..8ca741a1fa5 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/endpointcertificates/EndpointCertificateManager.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/endpointcertificates/EndpointCertificateManager.java
@@ -1,5 +1,6 @@
 package com.yahoo.vespa.hosted.controller.endpointcertificates;
 
+import com.google.common.collect.Sets;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.zone.ZoneApi;
@@ -8,6 +9,9 @@ import com.yahoo.container.jdisc.secretstore.SecretStore;
 import com.yahoo.log.LogLevel;
 import com.yahoo.security.SubjectAlternativeName;
 import com.yahoo.security.X509CertificateUtils;
+import com.yahoo.vespa.flags.FetchVector;
+import com.yahoo.vespa.flags.FlagSource;
+import com.yahoo.vespa.flags.Flags;
 import com.yahoo.vespa.hosted.controller.Instance;
 import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate;
 import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificateProvider;
@@ -23,14 +27,19 @@ import java.time.Clock;
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
+import java.util.OptionalInt;
 import java.util.Set;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 /**
+ * Looks up stored endpoint certificate metadata, provisions new certificates if none is found,
+ * and refreshes certificates if a newer version is available.
+ *
  * @author andreer
  */
 public class EndpointCertificateManager {
@@ -42,32 +51,58 @@ public class EndpointCertificateManager {
     private final SecretStore secretStore;
     private final ApplicationCertificateProvider applicationCertificateProvider;
     private final Clock clock;
+    private final FlagSource flagSource;
 
     public EndpointCertificateManager(ZoneRegistry zoneRegistry,
                                       CuratorDb curator,
                                       SecretStore secretStore,
                                       ApplicationCertificateProvider applicationCertificateProvider,
-                                      Clock clock) {
+                                      Clock clock, FlagSource flagSource) {
         this.zoneRegistry = zoneRegistry;
         this.curator = curator;
         this.secretStore = secretStore;
         this.applicationCertificateProvider = applicationCertificateProvider;
         this.clock = clock;
+        this.flagSource = flagSource;
     }
 
     public Optional<EndpointCertificateMetadata> getEndpointCertificateMetadata(Instance instance, ZoneId zone) {
 
         if (!zoneRegistry.zones().directlyRouted().ids().contains(zone)) return Optional.empty();
 
-        // Re-use certificate if already provisioned
-        Optional<EndpointCertificateMetadata> endpointCertificateMetadata =
+        // Re-use existing certificate if already provisioned
+        var endpointCertificateMetadata =
                 curator.readEndpointCertificateMetadata(instance.id())
-                        .or(() -> Optional.of(provisionEndpointCertificate(instance)));
+                        .orElse(provisionEndpointCertificate(instance));
+
+        // If feature flag set for application, look for and use refreshed certificate
+        var useRefreshedEndpointCertificate = Flags.USE_REFRESHED_ENDPOINT_CERTIFICATE.bindTo(flagSource);
+        if (useRefreshedEndpointCertificate.with(FetchVector.Dimension.APPLICATION_ID, instance.id().serializedForm()).value()) {
+            var latestAvailableVersion = greatestVersionInSecretStore(endpointCertificateMetadata);
+
+            if (latestAvailableVersion.isPresent() && latestAvailableVersion.getAsInt() > endpointCertificateMetadata.version()) {
+                var refreshedCertificateMetadata = new EndpointCertificateMetadata(
+                        endpointCertificateMetadata.keyName(),
+                        endpointCertificateMetadata.certName(),
+                        latestAvailableVersion.getAsInt()
+                );
+
+                if (verifyEndpointCertificate(refreshedCertificateMetadata, instance, zone, "Did not refresh, problems with refreshed certificate: "))
+                    return Optional.of(refreshedCertificateMetadata);
+            }
+        }
+
+        // Only log warnings
+        verifyEndpointCertificate(endpointCertificateMetadata, instance, zone, "Problems while verifying certificate: ");
+
+        return Optional.of(endpointCertificateMetadata);
+    }
 
-        // Only logs warnings for now
-        endpointCertificateMetadata.ifPresent(certificateMetadata -> verifyEndpointCertificate(certificateMetadata, instance, zone));
+    private OptionalInt greatestVersionInSecretStore(EndpointCertificateMetadata originalCertificateMetadata) {
+        var certVersions = new HashSet<>(secretStore.listSecretVersions(originalCertificateMetadata.certName()));
+        var keyVersions = new HashSet<>(secretStore.listSecretVersions(originalCertificateMetadata.keyName()));
 
-        return endpointCertificateMetadata;
+        return Sets.union(certVersions, keyVersions).stream().mapToInt(Integer::intValue).max();
     }
 
     private EndpointCertificateMetadata provisionEndpointCertificate(Instance instance) {
@@ -79,25 +114,25 @@ public class EndpointCertificateManager {
         return provisionedCertificateMetadata;
     }
 
-    private boolean verifyEndpointCertificate(EndpointCertificateMetadata endpointCertificateMetadata, Instance instance, ZoneId zone) {
+    private boolean verifyEndpointCertificate(EndpointCertificateMetadata endpointCertificateMetadata, Instance instance, ZoneId zone, String warningPrefix) {
         try {
             var pemEncodedEndpointCertificate = secretStore.getSecret(endpointCertificateMetadata.certName(), endpointCertificateMetadata.version());
 
-            if (pemEncodedEndpointCertificate == null) return logWarning("Certificate not found in secret store");
+            if (pemEncodedEndpointCertificate == null) return logWarning(warningPrefix, "Certificate not found in secret store");
 
             List<X509Certificate> x509CertificateList = X509CertificateUtils.certificateListFromPem(pemEncodedEndpointCertificate);
 
-            if (x509CertificateList.isEmpty()) return logWarning("Empty certificate list");
+            if (x509CertificateList.isEmpty()) return logWarning(warningPrefix, "Empty certificate list");
             if (x509CertificateList.size() < 2)
-                return logWarning("Only a single certificate found in chain - intermediate certificates likely missing");
+                return logWarning(warningPrefix, "Only a single certificate found in chain - intermediate certificates likely missing");
 
             Instant now = clock.instant();
             Instant firstExpiry = Instant.MAX;
             for (X509Certificate x509Certificate : x509CertificateList) {
                 Instant notBefore = x509Certificate.getNotBefore().toInstant();
                 Instant notAfter = x509Certificate.getNotAfter().toInstant();
-                if (now.isBefore(notBefore)) return logWarning("Certificate is not yet valid");
-                if (now.isAfter(notAfter)) return logWarning("Certificate has expired");
+                if (now.isBefore(notBefore)) return logWarning(warningPrefix, "Certificate is not yet valid");
+                if (now.isAfter(notAfter)) return logWarning(warningPrefix, "Certificate has expired");
                 if (notAfter.isBefore(firstExpiry)) firstExpiry = notAfter;
             }
 
@@ -107,7 +142,7 @@ public class EndpointCertificateManager {
                     .map(SubjectAlternativeName::getValue).collect(Collectors.toSet());
 
             if (!subjectAlternativeNames.equals(Set.copyOf(dnsNamesOf(instance.id(), List.of(zone)))))
-                return logWarning("The list of SANs in the certificate does not match what we expect");
+                return logWarning(warningPrefix, "The list of SANs in the certificate does not match what we expect");
 
             return true; // All good then, hopefully
         } catch (Exception e) {
@@ -116,8 +151,8 @@ public class EndpointCertificateManager {
         }
     }
 
-    private static boolean logWarning(String message) {
-        log.log(LogLevel.WARNING, message);
+    private static boolean logWarning(String warningPrefix, String message) {
+        log.log(LogLevel.WARNING, warningPrefix + message);
         return false;
     }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/endpointcertificates/EndpointCertificateManagerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/endpointcertificates/EndpointCertificateManagerTest.java
index 270f83ea26b..11572551490 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/endpointcertificates/EndpointCertificateManagerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/endpointcertificates/EndpointCertificateManagerTest.java
@@ -3,17 +3,31 @@ package com.yahoo.vespa.hosted.controller.endpointcertificates;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.zone.ZoneId;
+import com.yahoo.security.KeyAlgorithm;
+import com.yahoo.security.KeyUtils;
+import com.yahoo.security.SignatureAlgorithm;
+import com.yahoo.security.X509CertificateBuilder;
+import com.yahoo.security.X509CertificateUtils;
+import com.yahoo.vespa.flags.Flags;
+import com.yahoo.vespa.flags.InMemoryFlagSource;
 import com.yahoo.vespa.hosted.controller.Instance;
 import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificateMock;
 import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata;
 import com.yahoo.vespa.hosted.controller.integration.SecretStoreMock;
 import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock;
 import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb;
+import org.junit.Before;
 import org.junit.Test;
 
+import javax.security.auth.x500.X500Principal;
+import java.security.KeyPair;
+import java.security.cert.X509Certificate;
 import java.time.Clock;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
 import java.util.Optional;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 /**
@@ -21,19 +35,73 @@ import static org.junit.Assert.assertTrue;
  */
 public class EndpointCertificateManagerTest {
 
-    @Test
-    public void getEndpointCertificate() {
-        SecretStoreMock secretStore = new SecretStoreMock();
-        ZoneRegistryMock zoneRegistryMock = new ZoneRegistryMock(SystemName.main);
+    private final SecretStoreMock secretStore = new SecretStoreMock();
+    private final ZoneRegistryMock zoneRegistryMock = new ZoneRegistryMock(SystemName.main);
+    private final MockCuratorDb mockCuratorDb = new MockCuratorDb();
+    private final ApplicationCertificateMock applicationCertificateMock = new ApplicationCertificateMock();
+    private final InMemoryFlagSource inMemoryFlagSource = new InMemoryFlagSource();
+    private final Clock clock = Clock.systemUTC();
+    private final EndpointCertificateManager endpointCertificateManager = new EndpointCertificateManager(zoneRegistryMock, mockCuratorDb, secretStore, applicationCertificateMock, clock, inMemoryFlagSource);
+
+    private static final KeyPair testKeyPair = KeyUtils.generateKeypair(KeyAlgorithm.EC, 192);
+    private static final X509Certificate testCertificate = X509CertificateBuilder
+            .fromKeypair(
+                    testKeyPair,
+                    new X500Principal("CN=test"),
+                    Instant.now(), Instant.now().plus(5, ChronoUnit.MINUTES),
+                    SignatureAlgorithm.SHA256_WITH_ECDSA,
+                    X509CertificateBuilder.generateRandomSerialNumber())
+            .addSubjectAlternativeName("vt2ktgkqme5zlnp4tj4ttyor7fj3v7q5o.vespa.oath.cloud")
+            .addSubjectAlternativeName("default.default.global.vespa.oath.cloud")
+            .addSubjectAlternativeName("*.default.default.global.vespa.oath.cloud")
+            .addSubjectAlternativeName("default.default.us-east-1.test.vespa.oath.cloud")
+            .addSubjectAlternativeName("*.default.default.us-east-1.test.vespa.oath.cloud")
+            .build();
+
+    private final Instance testInstance = new Instance(ApplicationId.defaultId());
+    private final String testKeyName = "testKeyName";
+    private final String testCertName = "testCertName";
+    private ZoneId testZone;
+
+    @Before
+    public void setUp() {
         zoneRegistryMock.exclusiveRoutingIn(zoneRegistryMock.zones().all().zones());
-        MockCuratorDb mockCuratorDb = new MockCuratorDb();
-        ApplicationCertificateMock applicationCertificateMock = new ApplicationCertificateMock();
-        Clock clock = Clock.systemUTC();
-        EndpointCertificateManager endpointCertificateManager = new EndpointCertificateManager(zoneRegistryMock, mockCuratorDb, secretStore, applicationCertificateMock, clock);
-        ZoneId id = zoneRegistryMock.zones().directlyRouted().zones().stream().findFirst().get().getId();
-        Instance instance = new Instance(ApplicationId.defaultId());
-        Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificateManager.getEndpointCertificateMetadata(instance, id);
+        testZone = zoneRegistryMock.zones().directlyRouted().zones().stream().findFirst().get().getId();
+    }
+
+    @Test
+    public void provisions_new_certificate() {
+        Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificateManager.getEndpointCertificateMetadata(testInstance, testZone);
+        assertTrue(endpointCertificateMetadata.isPresent());
+        assertTrue(endpointCertificateMetadata.get().keyName().matches("vespa.tls.default.default.*-key"));
+        assertTrue(endpointCertificateMetadata.get().certName().matches("vespa.tls.default.default.*-cert"));
+        assertEquals(0, endpointCertificateMetadata.get().version());
+    }
+
+    @Test
+    public void reuses_stored_certificate_metadata() {
+        mockCuratorDb.writeEndpointCertificateMetadata(testInstance.id(), new EndpointCertificateMetadata(testKeyName, testCertName, 7));
+        Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificateManager.getEndpointCertificateMetadata(testInstance, testZone);
+        assertTrue(endpointCertificateMetadata.isPresent());
+        assertEquals(testKeyName, endpointCertificateMetadata.get().keyName());
+        assertEquals(testCertName, endpointCertificateMetadata.get().certName());
+        assertEquals(7, endpointCertificateMetadata.get().version());
+    }
+
+    @Test
+    public void uses_refreshed_certificate_when_available_and_valid() {
+        inMemoryFlagSource.withBooleanFlag(Flags.USE_REFRESHED_ENDPOINT_CERTIFICATE.id(), true);
+
+        secretStore.setSecret(testKeyName, "secret-key", 7);
+        secretStore.setSecret(testCertName, "cert", 7);
+        secretStore.setSecret(testKeyName, KeyUtils.toPem(testKeyPair.getPrivate()), 8);
+        secretStore.setSecret(testCertName, X509CertificateUtils.toPem(testCertificate)+X509CertificateUtils.toPem(testCertificate), 8);
+        mockCuratorDb.writeEndpointCertificateMetadata(testInstance.id(), new EndpointCertificateMetadata(testKeyName, testCertName, 7));
+        Optional<EndpointCertificateMetadata> endpointCertificateMetadata = endpointCertificateManager.getEndpointCertificateMetadata(testInstance, testZone);
         assertTrue(endpointCertificateMetadata.isPresent());
+        assertEquals(testKeyName, endpointCertificateMetadata.get().keyName());
+        assertEquals(testCertName, endpointCertificateMetadata.get().certName());
+        assertEquals(8, endpointCertificateMetadata.get().version());
     }
 
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/SecretStoreMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/SecretStoreMock.java
index a14b7b82d67..c088965c2ad 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/SecretStoreMock.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/SecretStoreMock.java
@@ -5,6 +5,7 @@ import com.yahoo.component.AbstractComponent;
 import com.yahoo.container.jdisc.secretstore.SecretStore;
 
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
 
@@ -45,4 +46,8 @@ public class SecretStoreMock extends AbstractComponent implements SecretStore {
         return secrets.getOrDefault(key, new TreeMap<>()).get(version);
     }
 
+    @Override
+    public List<Integer> listSecretVersions(String key) {
+        return List.copyOf(secrets.getOrDefault(key, new TreeMap<>()).keySet());
+    }
 }