diff options
author | Morten Tokle <mortent@yahooinc.com> | 2022-12-01 11:19:16 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-12-01 11:19:16 +0100 |
commit | 0da74401d91e543f2f2505fd650e170792d58b31 (patch) | |
tree | 98839ee137dd2c30c41390a466e99c9289e964f3 /config-model | |
parent | ced78a22d4eb39927845db9a2446b988ba1f5802 (diff) | |
parent | a17e5f4a90299dc9256d7b1f2aed7ebf51c7f082 (diff) |
Merge pull request #25059 from vespa-engine/mortent/validate-clients
Add client validations
Diffstat (limited to 'config-model')
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(); + } + +} |