diff options
author | Jon Marius Venstad <jonmv@users.noreply.github.com> | 2023-04-24 10:41:39 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-04-24 10:41:39 +0200 |
commit | 3c409bf97a972757066599b63f7127c80ebee6f9 (patch) | |
tree | 80b00de89870b3bc2c325fd0c14a0dfc9709eaa4 | |
parent | cbc9101ce4f895b665b0e9a25c63f0ab7c5a7577 (diff) | |
parent | aa2a46e9043dbd7974f4eb3b291822bab94cd94c (diff) |
Merge pull request #26823 from vespa-engine/hmusum/fail-deployment-id-certificate-file-is-missing
Fail deployment if no certificate file exists in application package
4 files changed, 60 insertions, 27 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java index 4a8bc3cd09a..3ebaebf680a 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackage.java @@ -1,15 +1,10 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.application.pkg; -import com.google.common.hash.Funnel; -import com.google.common.hash.HashFunction; import com.google.common.hash.Hasher; import com.google.common.hash.Hashing; import com.google.common.hash.HashingOutputStream; import com.yahoo.component.Version; -import com.yahoo.vespa.archive.ArchiveStreamReader; -import com.yahoo.vespa.archive.ArchiveStreamReader.ArchiveFile; -import com.yahoo.vespa.archive.ArchiveStreamReader.Options; import com.yahoo.config.application.FileSystemWrapper; import com.yahoo.config.application.FileSystemWrapper.FileWrapper; import com.yahoo.config.application.XmlPreProcessor; @@ -23,10 +18,12 @@ import com.yahoo.config.provision.Tags; import com.yahoo.slime.Inspector; import com.yahoo.slime.Slime; import com.yahoo.slime.SlimeUtils; +import com.yahoo.vespa.archive.ArchiveStreamReader; +import com.yahoo.vespa.archive.ArchiveStreamReader.ArchiveFile; +import com.yahoo.vespa.archive.ArchiveStreamReader.Options; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.deployment.ZipBuilder; import com.yahoo.yolean.Exceptions; - import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStreamReader; @@ -44,10 +41,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; -import java.util.Random; import java.util.Set; -import java.util.SortedMap; -import java.util.TreeMap; import java.util.concurrent.ConcurrentSkipListMap; import java.util.function.Function; import java.util.function.Predicate; @@ -66,7 +60,8 @@ import static java.util.stream.Collectors.toMap; */ public class ApplicationPackage { - static final String trustedCertificatesFile = "security/clients.pem"; + static final String trustedCertificatesDir = "security/"; + static final String trustedCertificatesFile = trustedCertificatesDir + "clients.pem"; static final String buildMetaFile = "build-meta.json"; static final String deploymentFile = "deployment.xml"; static final String validationOverridesFile = "validation-overrides.xml"; @@ -90,7 +85,7 @@ public class ApplicationPackage { * it must not be further changed by the caller. */ public ApplicationPackage(byte[] zippedContent) { - this(zippedContent, false); + this(zippedContent, false, false); } /** @@ -99,9 +94,9 @@ public class ApplicationPackage { * it must not be further changed by the caller. * If 'requireFiles' is true, files needed by deployment orchestration must be present. */ - public ApplicationPackage(byte[] zippedContent, boolean requireFiles) { + public ApplicationPackage(byte[] zippedContent, boolean requireFiles, boolean checkCertificateFile) { this.zippedContent = Objects.requireNonNull(zippedContent, "The application package content cannot be null"); - this.files = new ZipArchiveCache(zippedContent, prePopulated); + this.files = new ZipArchiveCache(zippedContent, prePopulated, checkCertificateFile); Optional<DeploymentSpec> deploymentSpec = files.get(deploymentFile).map(bytes -> new String(bytes, UTF_8)).map(DeploymentSpec::fromXml); if (requireFiles && deploymentSpec.isEmpty()) @@ -253,10 +248,12 @@ public class ApplicationPackage { private final byte[] zip; private final Map<Path, Optional<byte[]>> cache; - public ZipArchiveCache(byte[] zip, Collection<String> prePopulated) { + public ZipArchiveCache(byte[] zip, Collection<String> prePopulated, boolean checkCertificateFile) { this.zip = zip; this.cache = new ConcurrentSkipListMap<>(); this.cache.putAll(read(prePopulated)); + if (checkCertificateFile) + verifyThatTrustedCertificateExists(); } public Optional<byte[]> get(String path) { @@ -274,17 +271,26 @@ public class ApplicationPackage { } private Map<Path, Optional<byte[]>> read(Collection<String> names) { - var entries = ZipEntries.from(zip, - names::contains, - maxSize, - true) - .asList().stream() - .collect(toMap(entry -> Paths.get(entry.name()).normalize(), - ZipEntries.ZipEntryWithContent::content)); + var entries = findZipFileEntries(names::contains); names.stream().map(Paths::get).forEach(path -> entries.putIfAbsent(path.normalize(), Optional.empty())); return entries; } + + private void verifyThatTrustedCertificateExists() { + // Any name is valid for certificate files + var entries = findZipFileEntries((entry) -> entry.contains(trustedCertificatesDir) && entry.endsWith(".pem")); + if (entries.size() == 0) + throw new IllegalArgumentException("No client certificate found in " + trustedCertificatesDir + " in application package" + + ", see https://cloud.vespa.ai/en/security/guide"); + } + + private Map<Path, Optional<byte[]>> findZipFileEntries(Predicate<String> names) { + return ZipEntries.from(zip, names, maxSize, true) + .asList().stream() + .collect(toMap(entry -> Paths.get(entry.name()).normalize(), + ZipEntries.ZipEntryWithContent::content)); + } } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index af09b9dc761..9224c53136d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -52,6 +52,8 @@ import com.yahoo.slime.SlimeUtils; import com.yahoo.text.Text; import com.yahoo.vespa.athenz.api.OAuthCredentials; import com.yahoo.vespa.athenz.client.zms.ZmsClientException; +import com.yahoo.vespa.flags.BooleanFlag; +import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.Instance; @@ -131,7 +133,6 @@ import com.yahoo.vespa.hosted.controller.tenant.TenantInfo; import com.yahoo.vespa.hosted.controller.versions.VersionStatus; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; import com.yahoo.yolean.Exceptions; - import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -166,6 +167,7 @@ import java.util.stream.Stream; import static com.yahoo.jdisc.Response.Status.BAD_REQUEST; import static com.yahoo.jdisc.Response.Status.CONFLICT; +import static com.yahoo.vespa.flags.FetchVector.Dimension.APPLICATION_ID; import static com.yahoo.yolean.Exceptions.uncheck; import static java.util.Comparator.comparingInt; import static java.util.Map.Entry.comparingByKey; @@ -187,6 +189,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { private final Controller controller; private final AccessControlRequests accessControlRequests; private final TestConfigSerializer testConfigSerializer; + private final BooleanFlag failDeploymentOnMissingCertificateFile; @Inject public ApplicationApiHandler(ThreadedHttpRequestHandler.Context parentCtx, @@ -196,6 +199,7 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { this.controller = controller; this.accessControlRequests = accessControlRequests; this.testConfigSerializer = new TestConfigSerializer(controller.system()); + this.failDeploymentOnMissingCertificateFile = Flags.FAIL_DEPLOYMENT_ON_MISSING_CERTIFICATE_FILE.bindTo(controller.flagSource()); } @Override @@ -3066,7 +3070,12 @@ public class ApplicationApiHandler extends AuditLoggingRequestHandler { throw new IllegalArgumentException("Source URL must include scheme and host"); }); - ApplicationPackage applicationPackage = new ApplicationPackage(dataParts.get(EnvironmentResource.APPLICATION_ZIP), true); + ApplicationPackage applicationPackage = + new ApplicationPackage(dataParts.get(EnvironmentResource.APPLICATION_ZIP), + true, + failDeploymentOnMissingCertificateFile + .with(APPLICATION_ID, ApplicationId.from(tenant, application, "default").serializedForm()) + .value()); byte[] testPackage = dataParts.getOrDefault(EnvironmentResource.APPLICATION_TEST_ZIP, new byte[0]); Submission submission = new Submission(applicationPackage, testPackage, sourceUrl, sourceRevision, authorEmail, description, risk); diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java index 7f578d3017e..e915a204e4b 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageTest.java @@ -5,7 +5,6 @@ import com.yahoo.config.application.api.DeploymentSpec; import com.yahoo.config.application.api.ValidationId; import com.yahoo.io.LazyInputStream; import org.junit.jupiter.api.Test; - import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -97,7 +96,7 @@ public class ApplicationPackageTest { "jdisc.xml", jdiscXml, "content/content.xml", contentXml, "content/nodes.xml", nodesXml), - unzip(new ApplicationPackage(zip, false).metaDataZip())); + unzip(new ApplicationPackage(zip).metaDataZip())); } @Test @@ -105,7 +104,7 @@ public class ApplicationPackageTest { byte[] zip = filesZip(Map.of("services.xml", servicesXml.getBytes(UTF_8))); try { - new ApplicationPackage(zip, false).metaDataZip(); + new ApplicationPackage(zip).metaDataZip(); fail("Should fail on missing include file"); } catch (RuntimeException e) { @@ -152,6 +151,21 @@ public class ApplicationPackageTest { assertEquals(originalPackage.bundleHash(), similarDeploymentXml.bundleHash()); } + @Test + void testCertificateFileExists() throws Exception { + getApplicationZip("with-certificate.zip", true); + } + + @Test + void testCertificateFileMissing() throws Exception { + try { + getApplicationZip("original.zip", true); + fail("Should fail on missing certificate file file"); + } catch (RuntimeException e) { + assertEquals("No client certificate found in security/ in application package, see https://cloud.vespa.ai/en/security/guide", e.getMessage()); + } + } + static Map<String, String> unzip(byte[] zip) { return ZipEntries.from(zip, __ -> true, 1 << 24, true) .asList().stream() @@ -160,7 +174,11 @@ public class ApplicationPackageTest { } private ApplicationPackage getApplicationZip(String path) throws IOException { - return new ApplicationPackage(Files.readAllBytes(Path.of("src/test/resources/application-packages/" + path)), true); + return getApplicationZip(path, false); + } + + private ApplicationPackage getApplicationZip(String path, boolean checkCertificateFile) throws IOException { + return new ApplicationPackage(Files.readAllBytes(Path.of("src/test/resources/application-packages/" + path)), true, checkCertificateFile); } static byte[] zip(Map<String, String> content) { diff --git a/controller-server/src/test/resources/application-packages/with-certificate.zip b/controller-server/src/test/resources/application-packages/with-certificate.zip Binary files differnew file mode 100644 index 00000000000..1540b96c7ef --- /dev/null +++ b/controller-server/src/test/resources/application-packages/with-certificate.zip |