Merge pull request #25059 from vespa-engine/mortent/validate-clients

Add client validations

6 files changed, 341 insertions, 6 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/CloudDataPlaneFilterValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/CloudDataPlaneFilterValidator.java
new file mode 100644
index 00000000000..60705ad9b51
--- /dev/null
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/CloudDataPlaneFilterValidator.java
@@ -0,0 +1,71 @@
+package com.yahoo.vespa.model.application.validation;
+
+import com.yahoo.config.application.api.ApplicationPackage;
+import com.yahoo.config.model.deploy.DeployState;
+import com.yahoo.io.IOUtils;
+import com.yahoo.io.reader.NamedReader;
+import com.yahoo.path.Path;
+import com.yahoo.security.X509CertificateUtils;
+import com.yahoo.vespa.model.VespaModel;
+
+import java.io.IOException;
+import java.security.cert.X509Certificate;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.logging.Logger;
+import java.util.stream.Collectors;
+
+public class CloudDataPlaneFilterValidator extends Validator {
+
+    private static final Logger log = Logger.getLogger(CloudDataPlaneFilterValidator.class.getName());
+
+    @Override
+    public void validate(VespaModel model, DeployState deployState) {
+        if (!deployState.isHosted()) return;
+        if (!deployState.zone().system().isPublic()) return;
+        if (!deployState.featureFlags().enableDataPlaneFilter()) return;
+
+        validateUniqueCertificates(deployState);
+    }
+
+    private void validateUniqueCertificates(DeployState deployState) {
+        List<NamedReader> certFiles = deployState.getApplicationPackage().getFiles(ApplicationPackage.SECURITY_DIR, ".pem");
+
+        Map<String, List<X509Certificate>> configuredCertificates = certFiles.stream()
+                .collect(Collectors.toMap(NamedReader::getName, CloudDataPlaneFilterValidator::readCertificates));
+
+        Set<X509Certificate> duplicates = new HashSet<>();
+        Set<X509Certificate> globalUniqueCerts = new HashSet<>();
+        for (Map.Entry<String, List<X509Certificate>> certificateFile : configuredCertificates.entrySet()) {
+            List<X509Certificate> duplicatesInFile = certificateFile.getValue().stream()
+                    .filter(i -> !globalUniqueCerts.add(i))
+                    .toList();
+            duplicates.addAll(duplicatesInFile);
+        }
+        if (!duplicates.isEmpty()) {
+            List<String> filesWithDuplicates = configuredCertificates.entrySet().stream()
+                    .filter(entry -> entry.getValue().stream().anyMatch(duplicates::contains))
+                    .map(Map.Entry::getKey)
+                    .map(Path::fromString)
+                    .map(Path::getName)
+                    .map(p -> ApplicationPackage.SECURITY_DIR.append(p).getRelative())
+                    .sorted()
+                    .toList();
+            throw new IllegalArgumentException("Duplicate certificate(s) detected in files: %s. Certificate subject of duplicates: %s"
+                    .formatted(filesWithDuplicates.toString(),
+                               duplicates.stream().map(cert -> cert.getSubjectX500Principal().getName()).toList().toString()));
+        }
+    }
+
+    private static List<X509Certificate> readCertificates(NamedReader reader) {
+        try {
+            return X509CertificateUtils.certificateListFromPem(IOUtils.readAll(reader));
+        } catch (IOException e) {
+            log.warning("Exception reading certificate list from application package. File: %s, exception message: %s"
+                                .formatted(reader.getName(), e.getMessage()));
+            throw new RuntimeException("Error reading certificates from application package", e);
+        }
+    }
+}
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java
index 68b5adcc84f..b81b3524c67 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java
@@ -86,6 +86,7 @@ public class Validation {
         new CloudWatchValidator().validate(model, deployState);
         new QuotaValidator().validate(model, deployState);
         new UriBindingsValidator().validate(model, deployState);
+        new CloudDataPlaneFilterValidator().validate(model, deployState);
 
         additionalValidators.forEach(v -> v.validate(model, deployState));
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/CertificateRemovalChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/CertificateRemovalChangeValidator.java
new file mode 100644
index 00000000000..50fd6572bcc
--- /dev/null
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/CertificateRemovalChangeValidator.java
@@ -0,0 +1,47 @@
+package com.yahoo.vespa.model.application.validation.change;
+
+import com.yahoo.config.application.api.ValidationId;
+import com.yahoo.config.application.api.ValidationOverrides;
+import com.yahoo.config.model.api.ConfigChangeAction;
+import com.yahoo.vespa.model.VespaModel;
+import com.yahoo.vespa.model.container.http.Client;
+
+import java.security.cert.X509Certificate;
+import java.time.Instant;
+import java.util.Collection;
+import java.util.List;
+
+public class CertificateRemovalChangeValidator implements ChangeValidator {
+    @Override
+    public List<ConfigChangeAction> validate(VespaModel current, VespaModel next, ValidationOverrides overrides, Instant now) {
+
+        current.getContainerClusters()
+                .forEach((clusterId, currentCluster) -> {
+                    validateClients(clusterId, currentCluster.getClients(), next.getContainerClusters().get(clusterId).getClients(), overrides, now);
+                });
+
+        return List.of();
+    }
+
+    void validateClients(String clusterId, List<Client> current, List<Client> next, ValidationOverrides overrides, Instant now) {
+
+        List<X509Certificate> currentCertificates = current.stream()
+                .map(Client::certificates)
+                .flatMap(Collection::stream)
+                .toList();
+        List<X509Certificate> nextCertificates = next.stream()
+                .map(Client::certificates)
+                .flatMap(Collection::stream)
+                .toList();
+
+        List<X509Certificate> missingCerts = currentCertificates.stream().filter(cert -> !nextCertificates.contains(cert)).toList();
+        if (!missingCerts.isEmpty()) {
+            overrides.invalid(ValidationId.certificateRemoval,
+                              "Data plane certificate(s) from cluster '" + clusterId + "' is removed " +
+                              "(removed certificates: " + missingCerts.stream().map(x509Certificate -> x509Certificate.getSubjectX500Principal().getName()).toList() + ") " +
+                              "This can cause client connection issues.",
+                              now);
+        }
+
+    }
+}
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
index 79d8512e447..007e8401c70 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
@@ -38,6 +38,7 @@ import com.yahoo.config.provision.NodeType;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.container.bundle.BundleInstantiationSpecification;
 import com.yahoo.container.logging.FileConnectionLog;
+import com.yahoo.io.IOUtils;
 import com.yahoo.osgi.provider.model.ComponentModel;
 import com.yahoo.path.Path;
 import com.yahoo.schema.OnnxModel;
@@ -100,9 +101,8 @@ import org.w3c.dom.Element;
 import org.w3c.dom.Node;
 
 import java.io.IOException;
-import java.io.InputStream;
+import java.io.Reader;
 import java.net.URI;
-import java.nio.charset.StandardCharsets;
 import java.security.cert.X509Certificate;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -529,10 +529,10 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
 
     private List<X509Certificate> getCertificates(ApplicationFile file) {
         try {
-            InputStream inputStream = file.createInputStream();
-            byte[] bytes = inputStream.readAllBytes();
-            inputStream.close();
-            return X509CertificateUtils.certificateListFromPem(new String(bytes, StandardCharsets.UTF_8));
+            Reader reader = file.createReader();
+            String certPem = IOUtils.readAll(reader);
+            reader.close();
+            return X509CertificateUtils.certificateListFromPem(certPem);
         } catch (IOException e) {
             throw new RuntimeException(e);
         }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/CloudDataPlaneFilterValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/CloudDataPlaneFilterValidatorTest.java
new file mode 100644
index 00000000000..ed39260bdd2
--- /dev/null
+++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/CloudDataPlaneFilterValidatorTest.java
@@ -0,0 +1,165 @@
+package com.yahoo.vespa.model.application.validation;
+
+import com.yahoo.config.application.api.ApplicationPackage;
+import com.yahoo.config.model.NullConfigModelRegistry;
+import com.yahoo.config.model.api.EndpointCertificateSecrets;
+import com.yahoo.config.model.deploy.DeployState;
+import com.yahoo.config.model.deploy.TestProperties;
+import com.yahoo.config.model.test.MockApplicationPackage;
+import com.yahoo.config.provision.Environment;
+import com.yahoo.config.provision.RegionName;
+import com.yahoo.config.provision.SystemName;
+import com.yahoo.config.provision.Zone;
+import com.yahoo.path.Path;
+import com.yahoo.security.KeyAlgorithm;
+import com.yahoo.security.KeyUtils;
+import com.yahoo.security.SignatureAlgorithm;
+import com.yahoo.security.X509CertificateBuilder;
+import com.yahoo.security.X509CertificateUtils;
+import com.yahoo.vespa.model.VespaModel;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import org.xml.sax.SAXException;
+
+import javax.security.auth.x500.X500Principal;
+import java.io.File;
+import java.io.IOException;
+import java.io.StringReader;
+import java.math.BigInteger;
+import java.security.KeyPair;
+import java.security.cert.X509Certificate;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class CloudDataPlaneFilterValidatorTest {
+
+    @TempDir
+    public File applicationFolder;
+
+    @Test
+    void validator_accepts_distinct_client_certificates() throws IOException, SAXException {
+        String certFile1 = "security/foo.pem";
+        String certFile2 = "security/bar.pem";
+        String servicesXml = """
+                        <services version='1.0'>
+                          <container version='1.0'>
+                            <clients>
+                              <client id="a" permissions="read,write">
+                                <certificate file="%s"/>
+                              </client>
+                              <client id="b" permissions="read,write">
+                                <certificate file="%s"/>
+                              </client>
+                            </clients>
+                          </container>
+                        </services>
+                """.formatted(certFile1, certFile2);
+
+        DeployState deployState = createDeployState(servicesXml,
+                                                    Map.of(
+                                                            certFile1, List.of(createCertificate("foo")),
+                                                            certFile2, List.of(createCertificate("bar"))));
+
+        VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState);
+        new CloudDataPlaneFilterValidator().validate(model, deployState);
+    }
+
+    @Test
+    void validator_rejects_duplicate_client_certificates_different_files() throws IOException, SAXException {
+        String certFile1 = "security/a.pem";
+        String certFile2 = "security/b.pem";
+        X509Certificate certificate = createCertificate("a");
+        String servicesXml = """
+                <services version='1.0'>
+                  <container version='1.0'>
+                    <clients>
+                      <client id="a" permissions="read,write">
+                        <certificate file="%s"/>
+                      </client>
+                      <client id="b" permissions="read,write">
+                        <certificate file="%s"/>
+                      </client>
+                    </clients>
+                  </container>
+                </services>
+                """.formatted(certFile1, certFile2);
+
+        DeployState deployState = createDeployState(servicesXml,
+                                                    Map.of(
+                                                            certFile1, List.of(certificate),
+                                                            certFile2, List.of(certificate)));
+
+        VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState);
+        IllegalArgumentException illegalArgumentException = Assertions.assertThrows(IllegalArgumentException.class, () ->
+                new CloudDataPlaneFilterValidator().validate(model, deployState));
+        assertEquals(
+                "Duplicate certificate(s) detected in files: [%s, %s]. Certificate subject of duplicates: [%s]".formatted(certFile1, certFile2, certificate.getSubjectX500Principal().getName()),
+                illegalArgumentException.getMessage());
+    }
+
+    @Test
+    void validator_rejects_duplicate_client_certificates_same_file() throws IOException, SAXException {
+        String certFile1 = "security/a.pem";
+        X509Certificate certificate = createCertificate("a");
+        String servicesXml = """
+                <services version='1.0'>
+                  <container version='1.0'>
+                    <clients>
+                      <client id="a" permissions="read,write">
+                        <certificate file="%s"/>
+                      </client>
+                    </clients>
+                  </container>
+                </services>
+                """.formatted(certFile1);
+
+        DeployState deployState = createDeployState(servicesXml,
+                                                    Map.of(certFile1, List.of(certificate, certificate)));
+
+        VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState);
+        IllegalArgumentException illegalArgumentException = Assertions.assertThrows(IllegalArgumentException.class, () ->
+                new CloudDataPlaneFilterValidator().validate(model, deployState));
+        assertEquals(
+                "Duplicate certificate(s) detected in files: [%s]. Certificate subject of duplicates: [%s]".formatted(certFile1, certificate.getSubjectX500Principal().getName()),
+                illegalArgumentException.getMessage());
+    }
+
+
+    private DeployState createDeployState(String servicesXml, Map<String, List<X509Certificate>> certificates) {
+
+        ApplicationPackage applicationPackage = new MockApplicationPackage.Builder()
+                .withRoot(applicationFolder)
+                .withServices(servicesXml)
+                .build();
+
+        applicationPackage.getFile(Path.fromString("security")).createDirectory();
+        certificates.forEach((file, certList) ->
+                                     applicationPackage.getFile(Path.fromString(file)).writeFile(new StringReader(X509CertificateUtils.toPem(certList))));
+
+        return new DeployState.Builder()
+                .applicationPackage(applicationPackage)
+                .properties(
+                        new TestProperties()
+                                .setEndpointCertificateSecrets(Optional.of(new EndpointCertificateSecrets("CERT", "KEY")))
+                                .setHostedVespa(true)
+                                .setEnableDataPlaneFilter(true))
+                .zone(new Zone(SystemName.PublicCd, Environment.dev, RegionName.defaultName()))
+                .build();
+    }
+
+    static X509Certificate createCertificate(String cn) throws IOException {
+        KeyPair keyPair = KeyUtils.generateKeypair(KeyAlgorithm.EC, 256);
+        X500Principal subject = new X500Principal("CN=" + cn);
+        return X509CertificateBuilder
+                .fromKeypair(
+                        keyPair, subject, Instant.now(), Instant.now().plus(1, ChronoUnit.DAYS), SignatureAlgorithm.SHA512_WITH_ECDSA, BigInteger.valueOf(1))
+                .build();
+    }
+
+}
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/CertificateRemovalChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/CertificateRemovalChangeValidatorTest.java
new file mode 100644
index 00000000000..f89c75362da
--- /dev/null
+++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/CertificateRemovalChangeValidatorTest.java
@@ -0,0 +1,51 @@
+package com.yahoo.vespa.model.application.validation.change;
+
+import com.yahoo.config.application.api.ValidationOverrides;
+import com.yahoo.security.X509CertificateUtils;
+import com.yahoo.vespa.model.container.http.Client;
+import org.junit.jupiter.api.Test;
+
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+public class CertificateRemovalChangeValidatorTest {
+
+    private static final String validationOverrides =
+            "<validation-overrides>\n" +
+            "    <allow until='2000-01-14' comment='test override'>certificate-removal</allow>\n" +
+            "</validation-overrides>\n";
+
+    @Test
+    void validate() {
+        Instant now = LocalDate.parse("2000-01-01", DateTimeFormatter.ISO_DATE).atStartOfDay().atZone(ZoneOffset.UTC).toInstant();
+
+        Client c1 = new Client("c1", List.of(), List.of(certificate("cn=c1")));
+        Client c2 = new Client("c2", List.of(), List.of(certificate("cn=c2")));
+        Client c3 = new Client("c3", List.of(), List.of(certificate("cn=c3")));
+
+        CertificateRemovalChangeValidator validator = new CertificateRemovalChangeValidator();
+
+        // Adding certs -> ok
+        validator.validateClients("clusterId", List.of(c1,c2), List.of(c1, c2, c3), ValidationOverrides.empty, now);
+
+        // Removing certs -> fails
+        assertThrows(ValidationOverrides.ValidationException.class,
+                     () ->validator.validateClients("clusterId", List.of(c1, c2, c3), List.of(c1, c3), ValidationOverrides.empty, now));
+
+        // Removing certs with validationoverrides -> ok
+        validator.validateClients("clusterId", List.of(c1, c2, c3), List.of(c1, c3), ValidationOverrides.fromXml(validationOverrides), now);
+
+    }
+
+    static X509Certificate certificate(String cn) {
+        return X509CertificateUtils.createSelfSigned(cn, Duration.ofHours(1)).certificate();
+    }
+
+}