Merge pull request #25310 from vespa-engine/mortent/remove-controller-cert-validation

Remove controller certificate validation

6 files changed, 0 insertions, 95 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 8dbec87828a..5becf5dad54 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
@@ -80,7 +80,6 @@ public class ApplicationPackage {
     private final Optional<Version> compileVersion;
     private final Optional<Instant> buildTime;
     private final Optional<Version> parentVersion;
-    private final List<X509Certificate> trustedCertificates;
 
     /**
      * Creates an application package from its zipped content.
@@ -113,8 +112,6 @@ public class ApplicationPackage {
         this.buildTime = buildMetaObject.flatMap(object -> parse(object, "buildTime", field -> Instant.ofEpochMilli(field.asLong())));
         this.parentVersion = buildMetaObject.flatMap(object -> parse(object, "parentVersion", field -> Version.fromString(field.asString())));
 
-        this.trustedCertificates = files.get(trustedCertificatesFile).map(bytes -> X509CertificateUtils.certificateListFromPem(new String(bytes, UTF_8))).orElse(List.of());
-
         this.bundleHash = calculateBundleHash(zippedContent);
 
         preProcessAndPopulateCache();
@@ -150,11 +147,6 @@ public class ApplicationPackage {
     /** Returns the parent version used to compile the package, if known. */
     public Optional<Version> parentVersion() { return parentVersion; }
 
-    /** Returns the list of certificates trusted by this application, or an empty list if no trust configured. */
-    public List<X509Certificate> trustedCertificates() {
-        return trustedCertificates;
-    }
-
     private static <Type> Optional<Type> parse(Inspector buildMetaObject, String fieldName, Function<Inspector, Type> mapper) {
         Inspector field = buildMetaObject.field(fieldName);
         if ( ! field.valid() || field.type() == NIX)
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageStream.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageStream.java
index 021064417ac..f947926eb9c 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageStream.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageStream.java
@@ -42,24 +42,6 @@ public class ApplicationPackageStream {
     private final Supplier<InputStream> in;
     private final AtomicReference<ApplicationPackage> truncatedPackage = new AtomicReference<>();
 
-    public static Supplier<Replacer> addingCertificate(Optional<X509Certificate> certificate) {
-        return certificate.map(cert -> Replacer.of(Map.of(ApplicationPackage.trustedCertificatesFile,
-                                                          trustBytes -> append(trustBytes, cert))))
-                          .orElse(Replacer.of(Map.of()));
-    }
-
-    static InputStream append(InputStream trustIn, X509Certificate cert) {
-        try {
-            List<X509Certificate> trusted = trustIn == null ? new ArrayList<>()
-                                                            : new ArrayList<>(certificateListFromPem(new String(trustIn.readAllBytes(), UTF_8)));
-            trusted.add(cert);
-            return new ByteArrayInputStream(X509CertificateUtils.toPem(trusted).getBytes(UTF_8));
-        }
-        catch (IOException e) {
-            throw new UncheckedIOException(e);
-        }
-    }
-
     /** Stream that effectively copies the input stream to its {@link #truncatedPackage()} when exhausted. */
     public ApplicationPackageStream(Supplier<InputStream> in) {
         this(in, () -> __ -> true, Map.of());
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java
index aeac28782a6..bce9d44f8c6 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/pkg/ApplicationPackageValidator.java
@@ -59,7 +59,6 @@ public class ApplicationPackageValidator {
         validateEndpointRegions(applicationPackage.deploymentSpec());
         validateEndpointChange(application, applicationPackage, instant);
         validateCompactedEndpoint(applicationPackage);
-        validateSecurityClientsPem(applicationPackage);
         validateDeprecatedElements(applicationPackage);
         validateCloudAccounts(application, applicationPackage);
     }
@@ -95,13 +94,6 @@ public class ApplicationPackageValidator {
         }
     }
 
-    /** Verify that we have the security/clients.pem file for public systems */
-    private void validateSecurityClientsPem(ApplicationPackage applicationPackage) {
-        if (!controller.system().isPublic() || applicationPackage.deploymentSpec().steps().isEmpty()) return;
-        if (applicationPackage.trustedCertificates().isEmpty())
-            throw new IllegalArgumentException("Missing required file 'security/clients.pem'");
-    }
-
     /** Verify that each of the production zones listed in the deployment spec exist in this system */
     private void validateSteps(DeploymentSpec deploymentSpec) {
         for (var spec : deploymentSpec.instances()) {
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 8ac8b87ac45..7f578d3017e 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
@@ -4,13 +4,8 @@ package com.yahoo.vespa.hosted.controller.application.pkg;
 import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.config.application.api.ValidationId;
 import com.yahoo.io.LazyInputStream;
-import com.yahoo.security.KeyAlgorithm;
-import com.yahoo.security.KeyUtils;
-import com.yahoo.security.SignatureAlgorithm;
-import com.yahoo.security.X509CertificateBuilder;
 import org.junit.jupiter.api.Test;
 
-import javax.security.auth.x500.X500Principal;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
@@ -18,26 +13,17 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.PrintStream;
 import java.io.SequenceInputStream;
-import java.math.BigInteger;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.security.KeyPair;
-import java.security.cert.X509Certificate;
 import java.time.Instant;
-import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.HashMap;
 import java.util.LinkedHashMap;
-import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.function.Predicate;
 import java.util.function.UnaryOperator;
 import java.util.stream.Collectors;
-import java.util.stream.IntStream;
 
 import static com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackage.filesZip;
-import static com.yahoo.vespa.hosted.controller.application.pkg.ApplicationPackageStream.addingCertificate;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -93,7 +79,6 @@ public class ApplicationPackageTest {
     void test_createEmptyForDeploymentRemoval() {
         ApplicationPackage app = ApplicationPackage.deploymentRemoval();
         assertEquals(DeploymentSpec.empty, app.deploymentSpec());
-        assertEquals(List.of(), app.trustedCertificates());
 
         for (ValidationId validationId : ValidationId.values()) {
             assertTrue(app.validationOverrides().allows(validationId, Instant.now()));
@@ -178,30 +163,6 @@ public class ApplicationPackageTest {
         return new ApplicationPackage(Files.readAllBytes(Path.of("src/test/resources/application-packages/" + path)), true);
     }
 
-    @Test
-    void test_replacement() throws IOException {
-        byte[] zip = zip(Map.of());
-        List<X509Certificate> certificates = IntStream.range(0, 3)
-                                                      .mapToObj(i -> {
-                                                          KeyPair keyPair = KeyUtils.generateKeypair(KeyAlgorithm.EC, 256);
-                                                          X500Principal subject = new X500Principal("CN=subject" + i);
-                                                          return X509CertificateBuilder.fromKeypair(keyPair,
-                                                                                                    subject,
-                                                                                                    Instant.now(),
-                                                                                                    Instant.now().plusSeconds(1),
-                                                                                                    SignatureAlgorithm.SHA512_WITH_ECDSA,
-                                                                                                    BigInteger.valueOf(1))
-                                                                                       .build();
-                                                      }).toList();
-
-        assertEquals(List.of(), new ApplicationPackage(zip).trustedCertificates());
-        for (int i = 0; i < certificates.size(); i++) {
-            InputStream in = new ByteArrayInputStream(zip);
-            zip = new ApplicationPackageStream(() -> in, () -> __ -> false, addingCertificate(Optional.of(certificates.get(i)))).zipStream().readAllBytes();
-            assertEquals(certificates.subList(0, i + 1), new ApplicationPackage(zip).trustedCertificates());
-        }
-    }
-
     static byte[] zip(Map<String, String> content) {
         return filesZip(content.entrySet().stream().collect(Collectors.toMap(entry -> entry.getKey(),
                                                                              entry -> entry.getValue().getBytes(UTF_8))));
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
index c5ddc850611..2c2cc333f9c 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
@@ -510,10 +510,8 @@ public class InternalStepRunnerTest {
         assertEquals(unfinished, tester.jobs().run(id).stepStatuses().get(Step.deployTester));
         assertEquals(unfinished, tester.jobs().run(id).stepStatuses().get(Step.deployReal));
 
-        List<X509Certificate> oldApplicationTruststore = new ArrayList<>(DeploymentContext.publicApplicationPackage().trustedCertificates());
         List<X509Certificate> oldTesterCert = List.of(tester.jobs().run(id).testerCertificate().get());
 
-        assertEquals(oldApplicationTruststore, tester.configServer().application(app.instanceId(), id.type().zone()).get().applicationPackage().trustedCertificates());
         assertEquals(oldTesterCert, tester.configServer().additionalCertificates(app.deploymentIdIn(id.type().zone())));
 
         tester.configServer().throwOnNextPrepare(null);
@@ -522,12 +520,9 @@ public class InternalStepRunnerTest {
         assertEquals(succeeded, tester.jobs().run(id).stepStatuses().get(Step.deployTester));
         assertEquals(succeeded, tester.jobs().run(id).stepStatuses().get(Step.deployReal));
 
-        List<X509Certificate> newApplicationTruststore = new ArrayList<>(DeploymentContext.publicApplicationPackage().trustedCertificates());
         List<X509Certificate> newTesterCert = List.of(tester.jobs().run(id).testerCertificate().get());
-        assertEquals(newApplicationTruststore, tester.configServer().application(app.instanceId(), id.type().zone()).get().applicationPackage().trustedCertificates());
         assertEquals(newTesterCert, tester.configServer().additionalCertificates(app.deploymentIdIn(id.type().zone())));
 
-        assertEquals(oldApplicationTruststore, newApplicationTruststore);
         assertNotEquals(oldTesterCert, newTesterCert);
     }
 
@@ -537,11 +532,8 @@ public class InternalStepRunnerTest {
         app = tester.newDeploymentContext();
         RunId id = app.startSystemTestTests();
 
-        List<X509Certificate> trusted = new ArrayList<>(DeploymentContext.publicApplicationPackage().trustedCertificates());
-        assertEquals(trusted, tester.configServer().application(app.instanceId(), id.type().zone()).get().applicationPackage().trustedCertificates());
 
         assertEquals(List.of(tester.jobs().run(id).testerCertificate().get()), tester.configServer().additionalCertificates(app.deploymentIdIn(id.type().zone())));
-        assertEquals(trusted, tester.configServer().application(app.instanceId(), id.type().zone()).get().applicationPackage().trustedCertificates());
 
         tester.clock().advance(InternalStepRunner.Timeouts.of(system()).testerCertificate().plus(Duration.ofSeconds(1)));
         tester.runner().run();
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java
index a1e70b77948..03322d7c962 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiCloudTest.java
@@ -67,20 +67,6 @@ public class ApplicationApiCloudTest extends ControllerContainerCloudTest {
     }
 
     @Test
-    void test_missing_security_clients_pem() {
-        var application = prodBuilder().build();
-
-        var deployRequest = request("/application/v4/tenant/scoober/application/albums/submit", POST)
-                .data(createApplicationSubmissionData(application, 0))
-                .roles(Set.of(Role.developer(tenantName)));
-
-        tester.assertResponse(
-                deployRequest,
-                "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Missing required file 'security/clients.pem'\"}",
-                400);
-    }
-
-    @Test
     void tenant_info_profile() {
         tester.flagSource().withBooleanFlag(Flags.ENABLED_MAIL_VERIFICATION.id(), true);
         var request = request("/application/v4/tenant/scoober/info/profile", GET)