add various validation of endpoint certificates

3 files changed, 80 insertions, 18 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..7b5e0f45797 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
@@ -12,7 +12,12 @@ import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.InstanceName;
 import com.yahoo.config.provision.TenantName;
+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.athenz.api.AthenzDomain;
 import com.yahoo.vespa.athenz.api.AthenzIdentity;
 import com.yahoo.vespa.athenz.api.AthenzPrincipal;
@@ -74,6 +79,7 @@ import com.yahoo.vespa.hosted.rotation.config.RotationsConfig;
 
 import java.net.URI;
 import java.security.Principal;
+import java.security.cert.X509Certificate;
 import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
@@ -129,10 +135,11 @@ public class ApplicationController {
     private final Clock clock;
     private final DeploymentTrigger deploymentTrigger;
     private final ApplicationPackageValidator applicationPackageValidator;
+    private final SecretStore secretStore;
 
     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 +153,7 @@ public class ApplicationController {
         rotationRepository = new RotationRepository(rotationsConfig, this, curator);
         deploymentTrigger = new DeploymentTrigger(controller, clock);
         applicationPackageValidator = new ApplicationPackageValidator(controller);
+        this.secretStore = secretStore;
 
         // Update serialization format of all applications
         Once.after(Duration.ofMinutes(1), () -> {
@@ -399,7 +407,7 @@ public class ApplicationController {
 
                 if (controller.zoneRegistry().zones().directlyRouted().ids().contains(zone)) {
                     // Provisions a new certificate if missing
-                    endpointCertificateMetadata = getEndpointCertificate(application.get().require(instance));
+                    endpointCertificateMetadata = getEndpointCertificate(application.get().require(instance), zone);
                 } else {
                     endpointCertificateMetadata = Optional.empty();
                 }
@@ -565,20 +573,72 @@ public class ApplicationController {
         return Collections.unmodifiableSet(containerEndpoints);
     }
 
-    private Optional<EndpointCertificateMetadata> getEndpointCertificate(Instance instance) {
+    private Optional<EndpointCertificateMetadata> getEndpointCertificate(Instance instance, ZoneId zone) {
         // Re-use certificate if already provisioned
-        Optional<EndpointCertificateMetadata> endpointCertificateMetadata = curator.readEndpointCertificateMetadata(instance.id());
-        if(endpointCertificateMetadata.isPresent())
-            return endpointCertificateMetadata;
+        Optional<EndpointCertificateMetadata> endpointCertificateMetadata =
+                curator.readEndpointCertificateMetadata(instance.id())
+                        .or(() -> Optional.of(provisionEndpointCertificate(instance)));
 
-        ApplicationCertificate newCertificate = controller.serviceRegistry().applicationCertificateProvider().requestCaSignedCertificate(instance.id(), dnsNamesOf(instance.id()));
+        // Only logs warnings for now
+        endpointCertificateMetadata.ifPresent(certificateMetadata -> verifyEndpointCertificate(certificateMetadata, instance, zone));
+
+        return endpointCertificateMetadata;
+    }
+
+    private EndpointCertificateMetadata provisionEndpointCertificate(Instance instance) {
+        List<ZoneId> directlyRoutedZones = controller.zoneRegistry().zones().directlyRouted().zones().stream().map(ZoneApi::getId).collect(Collectors.toUnmodifiableList());
+        ApplicationCertificate newCertificate = controller.serviceRegistry().applicationCertificateProvider()
+                .requestCaSignedCertificate(instance.id(), dnsNamesOf(instance.id(), directlyRoutedZones));
         EndpointCertificateMetadata provisionedCertificateMetadata = EndpointCertificateMetadataSerializer.fromTlsSecretsKeysString(newCertificate.secretsKeyNamePrefix());
         curator.writeEndpointCertificateMetadata(instance.id(), provisionedCertificateMetadata);
-        return Optional.of(provisionedCertificateMetadata);
+        return provisionedCertificateMetadata;
+    }
+
+    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);
+            List<String> subjectAlternativeNames = X509CertificateUtils.getSubjectAlternativeNames(endEntityCertificate).stream()
+                    .filter(san -> san.getType().equals(SubjectAlternativeName.Type.DNS_NAME))
+                    .map(SubjectAlternativeName::getValue).collect(Collectors.toList());
+
+            System.out.println(subjectAlternativeNames);
+
+            if (!subjectAlternativeNames.containsAll(dnsNamesOf(instance.id(), List.of(zone))))
+                return logWarning("Certificate is missing SANs");
+
+            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;
     }
 
-    /** Returns all valid DNS names of given application */
-    private List<String> dnsNamesOf(ApplicationId applicationId) {
+    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,
@@ -588,9 +648,9 @@ public class ApplicationController {
         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())
+        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)
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/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;