From 2911962cef11affacba3a355ace91cb2ff46127b Mon Sep 17 00:00:00 2001 From: andreer Date: Mon, 27 Jan 2020 11:09:58 +0100 Subject: add various validation of endpoint certificates --- .../hosted/controller/ApplicationController.java | 86 ++++++++++++++++++---- .../yahoo/vespa/hosted/controller/Controller.java | 9 ++- .../vespa/hosted/controller/ControllerTester.java | 3 +- 3 files changed, 80 insertions(+), 18 deletions(-) (limited to 'controller-server') 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 getEndpointCertificate(Instance instance) { + private Optional getEndpointCertificate(Instance instance, ZoneId zone) { // Re-use certificate if already provisioned - Optional endpointCertificateMetadata = curator.readEndpointCertificateMetadata(instance.id()); - if(endpointCertificateMetadata.isPresent()) - return endpointCertificateMetadata; + Optional 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 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 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 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 dnsNamesOf(ApplicationId applicationId) { + private List dnsNamesOf(ApplicationId applicationId, List zones) { List 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 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; -- cgit v1.2.3 From 42d9ece77530bd4a803677f62380e41462d37b50 Mon Sep 17 00:00:00 2001 From: andreer Date: Mon, 27 Jan 2020 12:50:47 +0100 Subject: move endpoint certificate handling out of applicationcontroller --- .../hosted/controller/ApplicationController.java | 111 +--------------- .../EndpointCertificateManager.java | 148 +++++++++++++++++++++ 2 files changed, 153 insertions(+), 106 deletions(-) create mode 100644 controller-server/src/main/java/com/yahoo/vespa/hosted/controller/endpointcertificates/EndpointCertificateManager.java (limited to 'controller-server') 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 7b5e0f45797..dbb3b13f69d 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,12 +12,8 @@ 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; @@ -32,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; @@ -56,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; @@ -65,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; @@ -79,7 +73,6 @@ 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; @@ -101,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; @@ -135,7 +127,7 @@ public class ApplicationController { private final Clock clock; private final DeploymentTrigger deploymentTrigger; private final ApplicationPackageValidator applicationPackageValidator; - private final SecretStore secretStore; + private final EndpointCertificateManager endpointCertificateManager; ApplicationController(Controller controller, CuratorDb curator, AccessControl accessControl, RotationsConfig rotationsConfig, @@ -153,7 +145,8 @@ public class ApplicationController { rotationRepository = new RotationRepository(rotationsConfig, this, curator); deploymentTrigger = new DeploymentTrigger(controller, clock); applicationPackageValidator = new ApplicationPackageValidator(controller); - this.secretStore = secretStore; + endpointCertificateManager = new EndpointCertificateManager(controller.zoneRegistry(), curator, secretStore, + controller.serviceRegistry().applicationCertificateProvider(), clock); // Update serialization format of all applications Once.after(Duration.ofMinutes(1), () -> { @@ -405,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), zone); - } else { - endpointCertificateMetadata = Optional.empty(); - } + endpointCertificateMetadata = endpointCertificateManager.getEndpointCertificate(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. @@ -573,95 +561,6 @@ public class ApplicationController { return Collections.unmodifiableSet(containerEndpoints); } - private Optional getEndpointCertificate(Instance instance, ZoneId zone) { - // Re-use certificate if already provisioned - Optional 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 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 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 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 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; - } - - private List dnsNamesOf(ApplicationId applicationId, List zones) { - List 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 = 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(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/endpointcertificates/EndpointCertificateManager.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/endpointcertificates/EndpointCertificateManager.java new file mode 100644 index 00000000000..22152917f60 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/endpointcertificates/EndpointCertificateManager.java @@ -0,0 +1,148 @@ +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.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 getEndpointCertificate(Instance instance, ZoneId zone) { + + if (!zoneRegistry.zones().directlyRouted().ids().contains(zone)) { + return Optional.empty(); + } + + // Re-use certificate if already provisioned + Optional 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 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 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 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; + } + + private List dnsNamesOf(ApplicationId applicationId, List zones) { + List 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); + } + +} -- cgit v1.2.3 From 9e4b9422124073188f9aab1c0498f9fd04f6c8c1 Mon Sep 17 00:00:00 2001 From: andreer Date: Mon, 27 Jan 2020 14:45:44 +0100 Subject: set up mocked unit test --- .../hosted/controller/ApplicationController.java | 2 +- .../EndpointCertificateManager.java | 6 ++-- .../EndpointCertificateManagerTest.java | 34 ++++++++++++++++++++++ 3 files changed, 37 insertions(+), 5 deletions(-) create mode 100644 controller-server/src/test/java/com/yahoo/vespa/hosted/controller/endpointcertificates/EndpointCertificateManagerTest.java (limited to 'controller-server') 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 dbb3b13f69d..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 @@ -398,7 +398,7 @@ public class ApplicationController { validateRun(application.get().require(instance), zone, platformVersion, applicationVersion); } - endpointCertificateMetadata = endpointCertificateManager.getEndpointCertificate(application.get().require(instance), zone); + 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. 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 22152917f60..4908735659f 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 @@ -51,11 +51,9 @@ public class EndpointCertificateManager { this.clock = clock; } - public Optional getEndpointCertificate(Instance instance, ZoneId zone) { + public Optional getEndpointCertificateMetadata(Instance instance, ZoneId zone) { - if (!zoneRegistry.zones().directlyRouted().ids().contains(zone)) { - return Optional.empty(); - } + if (!zoneRegistry.zones().directlyRouted().ids().contains(zone)) return Optional.empty(); // Re-use certificate if already provisioned Optional endpointCertificateMetadata = 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 = endpointCertificateManager.getEndpointCertificateMetadata(instance, id); + assertTrue(endpointCertificateMetadata.isPresent()); + } +} \ No newline at end of file -- cgit v1.2.3 From a6769f64b639152760d84ed4e6f3300f1f83d90b Mon Sep 17 00:00:00 2001 From: andreer Date: Mon, 27 Jan 2020 15:17:15 +0100 Subject: stricter SAN verification --- .../endpointcertificates/EndpointCertificateManager.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'controller-server') 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 4908735659f..c421c106a59 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 @@ -25,6 +25,7 @@ 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; @@ -98,14 +99,12 @@ public class EndpointCertificateManager { } X509Certificate endEntityCertificate = x509CertificateList.get(0); - List subjectAlternativeNames = X509CertificateUtils.getSubjectAlternativeNames(endEntityCertificate).stream() + Set subjectAlternativeNames = X509CertificateUtils.getSubjectAlternativeNames(endEntityCertificate).stream() .filter(san -> san.getType().equals(SubjectAlternativeName.Type.DNS_NAME)) - .map(SubjectAlternativeName::getValue).collect(Collectors.toList()); + .map(SubjectAlternativeName::getValue).collect(Collectors.toSet()); - System.out.println(subjectAlternativeNames); - - if (!subjectAlternativeNames.containsAll(dnsNamesOf(instance.id(), List.of(zone)))) - return logWarning("Certificate is missing SANs"); + if (!subjectAlternativeNames.equals(Set.copyOf(dnsNamesOf(instance.id(), List.of(zone))))) + return logWarning("The set of DNS SANs in the certificate has changed"); return true; // All good then, hopefully } catch (Exception e) { -- cgit v1.2.3 From 0a8a46202e974a853dd6690c9359a3d26450dd89 Mon Sep 17 00:00:00 2001 From: andreer Date: Mon, 27 Jan 2020 15:18:17 +0100 Subject: stricter SAN verification 2 --- .../controller/endpointcertificates/EndpointCertificateManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'controller-server') 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 c421c106a59..4c7305f08e5 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 @@ -104,7 +104,7 @@ public class EndpointCertificateManager { .map(SubjectAlternativeName::getValue).collect(Collectors.toSet()); if (!subjectAlternativeNames.equals(Set.copyOf(dnsNamesOf(instance.id(), List.of(zone))))) - return logWarning("The set of DNS SANs in the certificate has changed"); + return logWarning("The list of SANs in the certificate does not match what we expect"); return true; // All good then, hopefully } catch (Exception e) { -- cgit v1.2.3