diff options
author | Andreas Eriksen <andreer@verizonmedia.com> | 2021-02-04 16:18:12 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-02-04 16:18:12 +0100 |
commit | e244a4b57e0641c2882104cf114fd2fb2058d608 (patch) | |
tree | c572e070d51f985db20f1fa17212d725f03a0cc8 /controller-server | |
parent | 21dcc03fb48657b38d490d3708dc9a473c1735e6 (diff) |
eliminate validate-endpoint-certificates feature flag (#16385)
Diffstat (limited to 'controller-server')
7 files changed, 28 insertions, 105 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 5eb7fb6e03d..d064bb17b2c 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 @@ -11,7 +11,6 @@ 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; @@ -78,13 +77,11 @@ import java.security.Principal; import java.time.Clock; import java.time.Duration; import java.time.Instant; -import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.EnumSet; import java.util.HashMap; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; @@ -92,20 +89,15 @@ import java.util.Optional; import java.util.Set; import java.util.TreeMap; import java.util.function.Consumer; -import java.util.function.Function; -import java.util.function.Predicate; import java.util.logging.Level; import java.util.logging.Logger; 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; import static java.util.Comparator.naturalOrder; -import static java.util.function.Function.identity; -import static java.util.function.Predicate.not; import static java.util.stream.Collectors.joining; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toMap; -import static java.util.stream.Collectors.toUnmodifiableMap; /** * A singleton owned by the Controller which contains the methods and state for controlling applications. @@ -134,7 +126,7 @@ public class ApplicationController { private final BillingController billingController; ApplicationController(Controller controller, CuratorDb curator, AccessControl accessControl, Clock clock, - SecretStore secretStore, FlagSource flagSource, BillingController billingController) { + FlagSource flagSource, BillingController billingController) { this.controller = controller; this.curator = curator; @@ -148,8 +140,12 @@ public class ApplicationController { deploymentTrigger = new DeploymentTrigger(controller, clock); applicationPackageValidator = new ApplicationPackageValidator(controller); - endpointCertificateManager = new EndpointCertificateManager(controller.zoneRegistry(), curator, secretStore, - controller.serviceRegistry().endpointCertificateProvider(), clock, flagSource); + endpointCertificateManager = new EndpointCertificateManager( + controller.zoneRegistry(), + curator, + controller.serviceRegistry().endpointCertificateProvider(), + controller.serviceRegistry().endpointCertificateValidator(), + clock); // 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 ffe80866086..e12c1903426 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 @@ -110,7 +110,7 @@ public class Controller extends AbstractComponent { metrics = new ConfigServerMetrics(serviceRegistry.configServer()); nameServiceForwarder = new NameServiceForwarder(curator); jobController = new JobController(this); - applicationController = new ApplicationController(this, curator, accessControl, clock, secretStore, flagSource, serviceRegistry.billingController()); + applicationController = new ApplicationController(this, curator, accessControl, clock, flagSource, serviceRegistry.billingController()); tenantController = new TenantController(this, curator, accessControl, flagSource); 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/certificate/EndpointCertificateException.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificateException.java deleted file mode 100644 index 321eb783dab..00000000000 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificateException.java +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.controller.certificate; - -public class EndpointCertificateException extends RuntimeException { - - private final Type type; - - public EndpointCertificateException(Type type, String message) { - super(message); - this.type = type; - } - - public EndpointCertificateException(Type type, String message, Throwable cause) { - super(message, cause); - this.type = type; - } - - public Type type() { - return type; - } - - public enum Type { - CERT_NOT_AVAILABLE, - VERIFICATION_FAILURE - } -} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificateManager.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificateManager.java index 87531240752..86ba25b7ad7 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificateManager.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificateManager.java @@ -9,16 +9,10 @@ import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.zone.RoutingMethod; import com.yahoo.config.provision.zone.ZoneApi; import com.yahoo.config.provision.zone.ZoneId; -import com.yahoo.container.jdisc.secretstore.SecretNotFoundException; -import com.yahoo.container.jdisc.secretstore.SecretStore; -import com.yahoo.security.SubjectAlternativeName; -import com.yahoo.security.X509CertificateUtils; -import com.yahoo.vespa.flags.BooleanFlag; -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.EndpointCertificateMetadata; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateProvider; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateValidator; 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; @@ -26,7 +20,6 @@ import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import org.jetbrains.annotations.NotNull; import java.nio.charset.Charset; -import java.security.cert.X509Certificate; import java.time.Clock; import java.time.Duration; import java.time.Instant; @@ -43,7 +36,7 @@ import java.util.stream.Collectors; /** * Looks up stored endpoint certificate metadata, provisions new certificates if none is found, * re-provisions if zone is not covered, and uses refreshed certificates if a newer version is available. - * + * <p> * See also EndpointCertificateMaintainer, which handles refreshes, deletions and triggers deployments * * @author andreer @@ -54,22 +47,20 @@ public class EndpointCertificateManager { private final ZoneRegistry zoneRegistry; private final CuratorDb curator; - private final SecretStore secretStore; private final EndpointCertificateProvider endpointCertificateProvider; private final Clock clock; - private final BooleanFlag validateEndpointCertificates; + private final EndpointCertificateValidator endpointCertificateValidator; public EndpointCertificateManager(ZoneRegistry zoneRegistry, CuratorDb curator, - SecretStore secretStore, EndpointCertificateProvider endpointCertificateProvider, - Clock clock, FlagSource flagSource) { + EndpointCertificateValidator endpointCertificateValidator, + Clock clock) { this.zoneRegistry = zoneRegistry; this.curator = curator; - this.secretStore = secretStore; this.endpointCertificateProvider = endpointCertificateProvider; this.clock = clock; - this.validateEndpointCertificates = Flags.VALIDATE_ENDPOINT_CERTIFICATES.bindTo(flagSource); + this.endpointCertificateValidator = endpointCertificateValidator; } public Optional<EndpointCertificateMetadata> getEndpointCertificateMetadata(Instance instance, ZoneId zone, Optional<DeploymentInstanceSpec> instanceSpec) { @@ -101,11 +92,11 @@ public class EndpointCertificateManager { var reprovisionedCertificateMetadata = provisionEndpointCertificate(instance, currentCertificateMetadata, zone, instanceSpec); curator.writeEndpointCertificateMetadata(instance.id(), reprovisionedCertificateMetadata); // Verification is unlikely to succeed in this case, as certificate must be available first - controller will retry - validateEndpointCertificate(reprovisionedCertificateMetadata, instance, zone); + endpointCertificateValidator.validate(reprovisionedCertificateMetadata, instance.id().serializedForm(), zone, requiredSansForZone); return Optional.of(reprovisionedCertificateMetadata); } - validateEndpointCertificate(currentCertificateMetadata.get(), instance, zone); + endpointCertificateValidator.validate(currentCertificateMetadata.get(), instance.id().serializedForm(), zone, requiredSansForZone); return currentCertificateMetadata; } @@ -145,53 +136,6 @@ public class EndpointCertificateManager { return endpointCertificateProvider.requestCaSignedCertificate(instance.id(), List.copyOf(requiredNames), currentMetadata); } - private void validateEndpointCertificate(EndpointCertificateMetadata endpointCertificateMetadata, Instance instance, ZoneId zone) { - if (validateEndpointCertificates.value()) - try { - var pemEncodedEndpointCertificate = secretStore.getSecret(endpointCertificateMetadata.certName(), endpointCertificateMetadata.version()); - - if (pemEncodedEndpointCertificate == null) - throw new EndpointCertificateException(EndpointCertificateException.Type.VERIFICATION_FAILURE, "Secret store returned null for certificate"); - - List<X509Certificate> x509CertificateList = X509CertificateUtils.certificateListFromPem(pemEncodedEndpointCertificate); - - if (x509CertificateList.isEmpty()) - throw new EndpointCertificateException(EndpointCertificateException.Type.VERIFICATION_FAILURE, "Empty certificate list"); - if (x509CertificateList.size() < 2) - throw new EndpointCertificateException(EndpointCertificateException.Type.VERIFICATION_FAILURE, "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)) - throw new EndpointCertificateException(EndpointCertificateException.Type.VERIFICATION_FAILURE, "Certificate is not yet valid"); - if (now.isAfter(notAfter)) - throw new EndpointCertificateException(EndpointCertificateException.Type.VERIFICATION_FAILURE, "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()); - - var dnsNamesOfZone = dnsNamesOf(instance.id(), zone); - if (!subjectAlternativeNames.containsAll(dnsNamesOfZone)) - throw new EndpointCertificateException(EndpointCertificateException.Type.VERIFICATION_FAILURE, "Certificate is missing required SANs for zone " + zone.value()); - - } catch (SecretNotFoundException s) { - // Normally because the cert is in the process of being provisioned - this will cause a retry in InternalStepRunner - throw new EndpointCertificateException(EndpointCertificateException.Type.CERT_NOT_AVAILABLE, "Certificate not found in secret store"); - } catch (EndpointCertificateException e) { - log.log(Level.WARNING, "Certificate validation failure for " + instance.id().serializedForm(), e); - throw e; - } catch (Exception e) { - log.log(Level.WARNING, "Certificate validation failure for " + instance.id().serializedForm(), e); - throw new EndpointCertificateException(EndpointCertificateException.Type.VERIFICATION_FAILURE, "Certificate validation failure for app " + instance.id().serializedForm(), e); - } - } private List<String> dnsNamesOf(ApplicationId applicationId, ZoneId zone) { List<String> endpointDnsNames = new ArrayList<>(); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java index e789974ea13..77aefc90481 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunner.java @@ -43,7 +43,7 @@ import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.Endpoint; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; -import com.yahoo.vespa.hosted.controller.certificate.EndpointCertificateException; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateException; import com.yahoo.vespa.hosted.controller.config.ControllerConfig; import com.yahoo.vespa.hosted.controller.maintenance.JobRunner; import com.yahoo.vespa.hosted.controller.routing.RoutingPolicyId; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificateManagerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificateManagerTest.java index 0d70fe48a77..be77f355e35 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificateManagerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/certificate/EndpointCertificateManagerTest.java @@ -16,6 +16,7 @@ import com.yahoo.vespa.flags.InMemoryFlagSource; import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMock; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMetadata; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateValidatorImpl; import com.yahoo.vespa.hosted.controller.integration.SecretStoreMock; import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock; import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb; @@ -47,8 +48,9 @@ public class EndpointCertificateManagerTest { private final EndpointCertificateMock endpointCertificateMock = new EndpointCertificateMock(); private final InMemoryFlagSource inMemoryFlagSource = new InMemoryFlagSource(); private static final Clock clock = Clock.fixed(Instant.EPOCH, java.time.ZoneId.systemDefault()); + private final EndpointCertificateValidatorImpl endpointCertificateValidator = new EndpointCertificateValidatorImpl(secretStore, clock); private final EndpointCertificateManager endpointCertificateManager = - new EndpointCertificateManager(zoneRegistryMock, mockCuratorDb, secretStore, endpointCertificateMock, clock, inMemoryFlagSource); + new EndpointCertificateManager(zoneRegistryMock, mockCuratorDb, endpointCertificateMock, endpointCertificateValidator, clock); private static final List<String> expectedSans = List.of( "vt2ktgkqme5zlnp4tj4ttyor7fj3v7q5o.vespa.oath.cloud", @@ -103,7 +105,6 @@ public class EndpointCertificateManagerTest { public void setUp() { zoneRegistryMock.exclusiveRoutingIn(zoneRegistryMock.zones().all().zones()); testZone = zoneRegistryMock.zones().directlyRouted().in(Environment.prod).zones().stream().findFirst().orElseThrow().getId(); - inMemoryFlagSource.withBooleanFlag(Flags.VALIDATE_ENDPOINT_CERTIFICATES.id(), true); } @Test diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java index fd0e7c20896..1b7970f6fcb 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/ServiceRegistryMock.java @@ -15,6 +15,8 @@ import com.yahoo.vespa.hosted.controller.api.integration.aws.ResourceTagger; import com.yahoo.vespa.hosted.controller.api.integration.billing.BillingController; import com.yahoo.vespa.hosted.controller.api.integration.billing.MockBillingController; import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateMock; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateValidator; +import com.yahoo.vespa.hosted.controller.api.integration.certificates.EndpointCertificateValidatorMock; import com.yahoo.vespa.hosted.controller.api.integration.dns.MemoryNameService; import com.yahoo.vespa.hosted.controller.api.integration.entity.MemoryEntityService; import com.yahoo.vespa.hosted.controller.api.integration.organization.MockContactRetriever; @@ -44,6 +46,7 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg private final MemoryGlobalRoutingService memoryGlobalRoutingService = new MemoryGlobalRoutingService(); private final MockMailer mockMailer = new MockMailer(); private final EndpointCertificateMock endpointCertificateMock = new EndpointCertificateMock(); + private final EndpointCertificateValidatorMock endpointCertificateValidatorMock = new EndpointCertificateValidatorMock(); private final MockMeteringClient mockMeteringClient = new MockMeteringClient(); private final MockContactRetriever mockContactRetriever = new MockContactRetriever(); private final MockIssueHandler mockIssueHandler = new MockIssueHandler(); @@ -103,6 +106,11 @@ public class ServiceRegistryMock extends AbstractComponent implements ServiceReg } @Override + public EndpointCertificateValidator endpointCertificateValidator() { + return endpointCertificateValidatorMock; + } + + @Override public MockMeteringClient meteringService() { return mockMeteringClient; } |