Merge pull request #11958 from vespa-engine/andreer/endpoint-certificate-validation

add various validation of endpoint certificates

5 files changed, 193 insertions, 53 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 66a7acc6ddf..c44c533e995 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
@@ -13,6 +13,7 @@ import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.InstanceName;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.config.provision.zone.ZoneId;
+import com.yahoo.container.jdisc.secretstore.SecretStore;
 import com.yahoo.vespa.athenz.api.AthenzDomain;
 import com.yahoo.vespa.athenz.api.AthenzIdentity;
 import com.yahoo.vespa.athenz.api.AthenzPrincipal;
@@ -27,7 +28,6 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.Hostname;
 import com.yahoo.vespa.hosted.controller.api.identifiers.InstanceId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.RevisionId;
-import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate;
 import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServer;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException;
@@ -51,7 +51,6 @@ import com.yahoo.vespa.hosted.controller.application.ApplicationPackageValidator
 import com.yahoo.vespa.hosted.controller.application.Deployment;
 import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics;
 import com.yahoo.vespa.hosted.controller.application.Endpoint;
-import com.yahoo.vespa.hosted.controller.application.EndpointId;
 import com.yahoo.vespa.hosted.controller.application.SystemApplication;
 import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId;
 import com.yahoo.vespa.hosted.controller.athenz.impl.AthenzFacade;
@@ -60,7 +59,7 @@ import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger;
 import com.yahoo.vespa.hosted.controller.deployment.Run;
 import com.yahoo.vespa.hosted.controller.deployment.Versions;
 import com.yahoo.vespa.hosted.controller.dns.NameServiceQueue.Priority;
-import com.yahoo.vespa.hosted.controller.persistence.EndpointCertificateMetadataSerializer;
+import com.yahoo.vespa.hosted.controller.endpointcertificates.EndpointCertificateManager;
 import com.yahoo.vespa.hosted.controller.routing.RoutingPolicies;
 import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
 import com.yahoo.vespa.hosted.controller.rotation.RotationLock;
@@ -95,7 +94,6 @@ import java.util.function.Consumer;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
 import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Node.State.active;
 import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Node.State.reserved;
@@ -129,10 +127,11 @@ public class ApplicationController {
     private final Clock clock;
     private final DeploymentTrigger deploymentTrigger;
     private final ApplicationPackageValidator applicationPackageValidator;
+    private final EndpointCertificateManager endpointCertificateManager;
 
     ApplicationController(Controller controller, CuratorDb curator,
                           AccessControl accessControl, RotationsConfig rotationsConfig,
-                          Clock clock) {
+                          Clock clock, SecretStore secretStore) {
         this.controller = controller;
         this.curator = curator;
         this.accessControl = accessControl;
@@ -146,6 +145,8 @@ public class ApplicationController {
         rotationRepository = new RotationRepository(rotationsConfig, this, curator);
         deploymentTrigger = new DeploymentTrigger(controller, clock);
         applicationPackageValidator = new ApplicationPackageValidator(controller);
+        endpointCertificateManager = new EndpointCertificateManager(controller.zoneRegistry(), curator, secretStore,
+                controller.serviceRegistry().applicationCertificateProvider(), clock);
 
         // Update serialization format of all applications
         Once.after(Duration.ofMinutes(1), () -> {
@@ -397,12 +398,7 @@ public class ApplicationController {
                     validateRun(application.get().require(instance), zone, platformVersion, applicationVersion);
                 }
 
-                if (controller.zoneRegistry().zones().directlyRouted().ids().contains(zone)) {
-                    // Provisions a new certificate if missing
-                    endpointCertificateMetadata = getEndpointCertificate(application.get().require(instance));
-                } else {
-                    endpointCertificateMetadata = Optional.empty();
-                }
+                endpointCertificateMetadata = endpointCertificateManager.getEndpointCertificateMetadata(application.get().require(instance), zone);
 
                 endpoints = registerEndpointsInDns(applicationPackage.deploymentSpec(), application.get().require(instanceId.instance()), zone);
             } // Release application lock while doing the deployment, which is a lengthy task.
@@ -565,43 +561,6 @@ public class ApplicationController {
         return Collections.unmodifiableSet(containerEndpoints);
     }
 
-    private Optional<EndpointCertificateMetadata> getEndpointCertificate(Instance instance) {
-        // Re-use certificate if already provisioned
-        Optional<EndpointCertificateMetadata> endpointCertificateMetadata = curator.readEndpointCertificateMetadata(instance.id());
-        if(endpointCertificateMetadata.isPresent())
-            return endpointCertificateMetadata;
-
-        ApplicationCertificate newCertificate = controller.serviceRegistry().applicationCertificateProvider().requestCaSignedCertificate(instance.id(), dnsNamesOf(instance.id()));
-        EndpointCertificateMetadata provisionedCertificateMetadata = EndpointCertificateMetadataSerializer.fromTlsSecretsKeysString(newCertificate.secretsKeyNamePrefix());
-        curator.writeEndpointCertificateMetadata(instance.id(), provisionedCertificateMetadata);
-        return Optional.of(provisionedCertificateMetadata);
-    }
-
-    /** Returns all valid DNS names of given application */
-    private List<String> dnsNamesOf(ApplicationId applicationId) {
-        List<String> endpointDnsNames = new ArrayList<>();
-
-        // We add first an endpoint name based on a hash of the applicationId,
-        // as the certificate provider requires the first CN to be < 64 characters long.
-        endpointDnsNames.add(Endpoint.createHashedCn(applicationId, controller.system()));
-
-        var globalDefaultEndpoint = Endpoint.of(applicationId).named(EndpointId.defaultId());
-        var rotationEndpoints = Endpoint.of(applicationId).wildcard();
-
-        var zoneLocalEndpoints = controller.zoneRegistry().zones().directlyRouted().zones().stream().flatMap(zone -> Stream.of(
-                Endpoint.of(applicationId).target(ClusterSpec.Id.from("default"), zone.getId()),
-                Endpoint.of(applicationId).wildcard(zone.getId())
-        ));
-
-        Stream.concat(Stream.of(globalDefaultEndpoint, rotationEndpoints), zoneLocalEndpoints)
-              .map(Endpoint.EndpointBuilder::directRouting)
-              .map(endpoint -> endpoint.on(Endpoint.Port.tls()))
-              .map(endpointBuilder -> endpointBuilder.in(controller.system()))
-              .map(Endpoint::dnsName).forEach(endpointDnsNames::add);
-
-        return Collections.unmodifiableList(endpointDnsNames);
-    }
-
     private ActivateResult unexpectedDeployment(ApplicationId application, ZoneId zone) {
         Log logEntry = new Log();
         logEntry.level = "WARNING";
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 d3e21f0d399..14b5c5e02c4 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
@@ -9,6 +9,7 @@ import com.yahoo.config.provision.CloudName;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.zone.ZoneApi;
+import com.yahoo.container.jdisc.secretstore.SecretStore;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.vespa.curator.Lock;
 import com.yahoo.vespa.flags.FlagSource;
@@ -80,14 +81,14 @@ public class Controller extends AbstractComponent implements ApplicationIdSource
      */
     @Inject
     public Controller(CuratorDb curator, RotationsConfig rotationsConfig, AccessControl accessControl, FlagSource flagSource,
-                      MavenRepository mavenRepository, ServiceRegistry serviceRegistry, Metric metric) {
+                      MavenRepository mavenRepository, ServiceRegistry serviceRegistry, Metric metric, SecretStore secretStore) {
         this(curator, rotationsConfig, accessControl, com.yahoo.net.HostName::getLocalhost, flagSource,
-             mavenRepository, serviceRegistry, metric);
+             mavenRepository, serviceRegistry, metric, secretStore);
     }
 
     public Controller(CuratorDb curator, RotationsConfig rotationsConfig, AccessControl accessControl,
                       Supplier<String> hostnameSupplier, FlagSource flagSource, MavenRepository mavenRepository,
-                      ServiceRegistry serviceRegistry, Metric metric) {
+                      ServiceRegistry serviceRegistry, Metric metric, SecretStore secretStore) {
 
         this.hostnameSupplier = Objects.requireNonNull(hostnameSupplier, "HostnameSupplier cannot be null");
         this.curator = Objects.requireNonNull(curator, "Curator cannot be null");
@@ -103,7 +104,7 @@ public class Controller extends AbstractComponent implements ApplicationIdSource
         jobController = new JobController(this);
         applicationController = new ApplicationController(this, curator, accessControl,
                                                           Objects.requireNonNull(rotationsConfig, "RotationsConfig cannot be null"),
-                                                          clock
+                                                          clock, secretStore
         );
         tenantController = new TenantController(this, curator, accessControl);
         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
new file mode 100644
index 00000000000..4c7305f08e5
--- /dev/null
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/endpointcertificates/EndpointCertificateManager.java
@@ -0,0 +1,145 @@
+package com.yahoo.vespa.hosted.controller.endpointcertificates;
+
+import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.ClusterSpec;
+import com.yahoo.config.provision.zone.ZoneApi;
+import com.yahoo.config.provision.zone.ZoneId;
+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.hosted.controller.Instance;
+import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate;
+import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificateProvider;
+import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata;
+import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry;
+import com.yahoo.vespa.hosted.controller.application.Endpoint;
+import com.yahoo.vespa.hosted.controller.application.EndpointId;
+import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
+import com.yahoo.vespa.hosted.controller.persistence.EndpointCertificateMetadataSerializer;
+
+import java.security.cert.X509Certificate;
+import java.time.Clock;
+import java.time.Instant;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import java.util.logging.Logger;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+public class EndpointCertificateManager {
+
+    private static final Logger log = Logger.getLogger(EndpointCertificateManager.class.getName());
+
+    private final ZoneRegistry zoneRegistry;
+    private final CuratorDb curator;
+    private final SecretStore secretStore;
+    private final ApplicationCertificateProvider applicationCertificateProvider;
+    private final Clock clock;
+
+    public EndpointCertificateManager(ZoneRegistry zoneRegistry,
+                                      CuratorDb curator,
+                                      SecretStore secretStore,
+                                      ApplicationCertificateProvider applicationCertificateProvider,
+                                      Clock clock) {
+        this.zoneRegistry = zoneRegistry;
+        this.curator = curator;
+        this.secretStore = secretStore;
+        this.applicationCertificateProvider = applicationCertificateProvider;
+        this.clock = clock;
+    }
+
+    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 =
+                curator.readEndpointCertificateMetadata(instance.id())
+                        .or(() -> Optional.of(provisionEndpointCertificate(instance)));
+
+        // Only logs warnings for now
+        endpointCertificateMetadata.ifPresent(certificateMetadata -> verifyEndpointCertificate(certificateMetadata, instance, zone));
+
+        return endpointCertificateMetadata;
+    }
+
+    private EndpointCertificateMetadata provisionEndpointCertificate(Instance instance) {
+        List<ZoneId> directlyRoutedZones = zoneRegistry.zones().directlyRouted().zones().stream().map(ZoneApi::getId).collect(Collectors.toUnmodifiableList());
+        ApplicationCertificate newCertificate = applicationCertificateProvider
+                .requestCaSignedCertificate(instance.id(), dnsNamesOf(instance.id(), directlyRoutedZones));
+        EndpointCertificateMetadata provisionedCertificateMetadata = EndpointCertificateMetadataSerializer.fromTlsSecretsKeysString(newCertificate.secretsKeyNamePrefix());
+        curator.writeEndpointCertificateMetadata(instance.id(), provisionedCertificateMetadata);
+        return provisionedCertificateMetadata;
+    }
+
+    private boolean verifyEndpointCertificate(EndpointCertificateMetadata endpointCertificateMetadata, Instance instance, ZoneId zone) {
+        try {
+            var pemEncodedEndpointCertificate = secretStore.getSecret(endpointCertificateMetadata.certName(), endpointCertificateMetadata.version());
+
+            if (pemEncodedEndpointCertificate == null) return logWarning("Certificate not found in secret store");
+
+            List<X509Certificate> x509CertificateList = X509CertificateUtils.certificateListFromPem(pemEncodedEndpointCertificate);
+
+            if (x509CertificateList.isEmpty()) return logWarning("Empty certificate list");
+            if (x509CertificateList.size() < 2)
+                return logWarning("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 (notAfter.isBefore(firstExpiry)) firstExpiry = notAfter;
+            }
+
+            X509Certificate endEntityCertificate = x509CertificateList.get(0);
+            Set<String> subjectAlternativeNames = X509CertificateUtils.getSubjectAlternativeNames(endEntityCertificate).stream()
+                    .filter(san -> san.getType().equals(SubjectAlternativeName.Type.DNS_NAME))
+                    .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 true; // All good then, hopefully
+        } catch (Exception e) {
+            log.log(LogLevel.WARNING, "Exception thrown when verifying endpoint certificate", e);
+            return false;
+        }
+    }
+
+    private static boolean logWarning(String message) {
+        log.log(LogLevel.WARNING, message);
+        return false;
+    }
+
+    private List<String> dnsNamesOf(ApplicationId applicationId, List<ZoneId> zones) {
+        List<String> endpointDnsNames = new ArrayList<>();
+
+        // We add first an endpoint name based on a hash of the applicationId,
+        // as the certificate provider requires the first CN to be < 64 characters long.
+        endpointDnsNames.add(Endpoint.createHashedCn(applicationId, zoneRegistry.system()));
+
+        var globalDefaultEndpoint = Endpoint.of(applicationId).named(EndpointId.defaultId());
+        var rotationEndpoints = Endpoint.of(applicationId).wildcard();
+
+        var zoneLocalEndpoints = zones.stream().flatMap(zone -> Stream.of(
+                Endpoint.of(applicationId).target(ClusterSpec.Id.from("default"), zone),
+                Endpoint.of(applicationId).wildcard(zone)
+        ));
+
+        Stream.concat(Stream.of(globalDefaultEndpoint, rotationEndpoints), zoneLocalEndpoints)
+                .map(Endpoint.EndpointBuilder::directRouting)
+                .map(endpoint -> endpoint.on(Endpoint.Port.tls()))
+                .map(endpointBuilder -> endpointBuilder.in(zoneRegistry.system()))
+                .map(Endpoint::dnsName).forEach(endpointDnsNames::add);
+
+        return Collections.unmodifiableList(endpointDnsNames);
+    }
+
+}
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
index c83463bc1ea..5318d6bdefd 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
@@ -33,6 +33,7 @@ import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId;
 import com.yahoo.vespa.hosted.controller.athenz.impl.AthenzFacade;
 import com.yahoo.vespa.hosted.controller.integration.ConfigServerMock;
 import com.yahoo.vespa.hosted.controller.integration.MetricsMock;
+import com.yahoo.vespa.hosted.controller.integration.SecretStoreMock;
 import com.yahoo.vespa.hosted.controller.integration.ServiceRegistryMock;
 import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock;
 import com.yahoo.vespa.hosted.controller.persistence.CuratorDb;
@@ -393,7 +394,7 @@ public final class ControllerTester {
                                                new InMemoryFlagSource(),
                                                new MockMavenRepository(),
                                                serviceRegistry,
-                                               new MetricsMock());
+                                               new MetricsMock(), new SecretStoreMock());
         // Calculate initial versions
         controller.updateVersionStatus(VersionStatus.compute(controller));
         return controller;
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
new file mode 100644
index 00000000000..231f8be2ed3
--- /dev/null
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/endpointcertificates/EndpointCertificateManagerTest.java
@@ -0,0 +1,34 @@
+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.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.Test;
+
+import java.time.Clock;
+import java.util.Optional;
+
+import static org.junit.Assert.assertTrue;
+
+public class EndpointCertificateManagerTest {
+
+    @Test
+    public void getEndpointCertificate() {
+        SecretStoreMock secretStore = new SecretStoreMock();
+        ZoneRegistryMock zoneRegistryMock = new ZoneRegistryMock(SystemName.main);
+        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);
+        assertTrue(endpointCertificateMetadata.isPresent());
+    }
+}
\ No newline at end of file