diff options
86 files changed, 1218 insertions, 335 deletions
diff --git a/config-model/pom.xml b/config-model/pom.xml index 25b733985f5..33f6657561c 100644 --- a/config-model/pom.xml +++ b/config-model/pom.xml @@ -105,6 +105,10 @@ <groupId>org.apache.httpcomponents</groupId> <artifactId>httpclient</artifactId> </exclusion> + <exclusion> + <groupId>org.apache.velocity</groupId> + <artifactId>velocity</artifactId> + </exclusion> </exclusions> </dependency> <dependency> diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java index 5714d41ef67..38037c8a522 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.model.admin; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.container.handler.ThreadpoolConfig; +import com.yahoo.search.config.QrStartConfig; import com.yahoo.vespa.model.container.ContainerCluster; import com.yahoo.vespa.model.container.component.Handler; @@ -27,6 +28,12 @@ public class LogserverContainerCluster extends ContainerCluster<LogserverContain builder.maxthreads(10); } + @Override + public void getConfig(QrStartConfig.Builder builder) { + super.getConfig(builder); + builder.jvm.heapsize(384); + } + protected boolean messageBusEnabled() { return false; } private void addLogHandler() { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java index 071666b5bc7..f81757ac568 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java @@ -20,6 +20,7 @@ import ai.vespa.metricsproxy.metric.dimensions.PublicDimensions; import ai.vespa.metricsproxy.rpc.RpcServer; import ai.vespa.metricsproxy.service.ConfigSentinelClient; import ai.vespa.metricsproxy.service.SystemPollerProvider; +import ai.vespa.metricsproxy.telegraf.TelegrafConfig; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.config.model.producer.AbstractConfigProducerRoot; @@ -67,6 +68,7 @@ public class MetricsProxyContainerCluster extends ContainerCluster<MetricsProxyC ApplicationDimensionsConfig.Producer, ConsumersConfig.Producer, MonitoringConfig.Producer, + TelegrafConfig.Producer, ThreadpoolConfig.Producer, MetricsNodesConfig.Producer { @@ -161,6 +163,25 @@ public class MetricsProxyContainerCluster extends ContainerCluster<MetricsProxyC } @Override + public void getConfig(TelegrafConfig.Builder builder) { + var userConsumers = getUserMetricsConsumers(); + for (var consumer : userConsumers.values()) { + for (var cloudWatch : consumer.cloudWatches()) { + var cloudWatchBuilder = new TelegrafConfig.CloudWatch.Builder(); + cloudWatchBuilder + .region(cloudWatch.region()) + .namespace(cloudWatch.namespace()) + .consumer(cloudWatch.consumer()); + cloudWatch.hostedAuth().ifPresent(hostedAuth -> cloudWatchBuilder + .accessKeyName(hostedAuth.accessKeyName) + .secretKeyName(hostedAuth.secretKeyName)); + cloudWatch.profile().ifPresent(cloudWatchBuilder::profile); + builder.cloudWatch(cloudWatchBuilder); + } + } + } + + @Override public void getConfig(ThreadpoolConfig.Builder builder) { builder.maxthreads(10); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/CloudWatch.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/CloudWatch.java new file mode 100644 index 00000000000..fd290409ea5 --- /dev/null +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/CloudWatch.java @@ -0,0 +1,49 @@ +package com.yahoo.vespa.model.admin.monitoring; + +import java.util.Optional; + +/** + * Helper object for CloudWatch configuration. + * + * @author gjoranv + */ +public class CloudWatch { + private final String region; + private final String namespace; + private final MetricsConsumer consumer; + + private HostedAuth hostedAuth; + private String profile; + + public CloudWatch(String region, String namespace, MetricsConsumer consumer) { + this.region = region; + this.namespace = namespace; + this.consumer = consumer; + } + + public String region() { return region; } + public String namespace() { return namespace; } + public String consumer() { return consumer.getId(); } + + public Optional<HostedAuth> hostedAuth() {return Optional.ofNullable(hostedAuth); } + public Optional<String> profile() { return Optional.ofNullable(profile); } + + public void setHostedAuth(HostedAuth hostedAuth) { + this.hostedAuth = hostedAuth; + } + + public void setProfile(String profile) { + this.profile = profile; + } + + public static class HostedAuth { + public final String accessKeyName; + public final String secretKeyName; + + public HostedAuth(String accessKeyName, String secretKeyName) { + this.accessKeyName = accessKeyName; + this.secretKeyName = secretKeyName; + } + } + +} diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/MetricsConsumer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/MetricsConsumer.java index 529ed6ecf67..9c752f3aa0d 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/MetricsConsumer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/MetricsConsumer.java @@ -2,9 +2,13 @@ package com.yahoo.vespa.model.admin.monitoring; import javax.annotation.concurrent.Immutable; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.Objects; +import static java.util.Collections.unmodifiableList; + /** * Represents an arbitrary metric consumer * @@ -16,6 +20,8 @@ public class MetricsConsumer { private final String id; private final MetricSet metricSet; + private final List<CloudWatch> cloudWatches = new ArrayList<>(); + /** * @param id The consumer * @param metricSet The metrics for this consumer @@ -38,4 +44,12 @@ public class MetricsConsumer { return metricSet.getMetrics(); } + public void addCloudWatch(CloudWatch cloudWatch) { + cloudWatches.add(cloudWatch); + } + + public List<CloudWatch> cloudWatches() { + return unmodifiableList(cloudWatches); + } + } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/builder/xml/CloudWatchBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/builder/xml/CloudWatchBuilder.java new file mode 100644 index 00000000000..4b9d5542aa9 --- /dev/null +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/builder/xml/CloudWatchBuilder.java @@ -0,0 +1,36 @@ +package com.yahoo.vespa.model.admin.monitoring.builder.xml; + +import com.yahoo.vespa.model.admin.monitoring.CloudWatch; +import com.yahoo.vespa.model.admin.monitoring.CloudWatch.HostedAuth; +import com.yahoo.vespa.model.admin.monitoring.MetricsConsumer; +import org.w3c.dom.Element; + +import static com.yahoo.config.model.builder.xml.XmlHelper.getOptionalChildValue; + +/** + * @author gjoranv + */ +public class CloudWatchBuilder { + + private static final String REGION_ATTRIBUTE = "region"; + private static final String NAMESPACE_ATTRIBUTE = "namespace"; + private static final String ACCESS_KEY_ELEMENT = "access-key-name"; + private static final String SECRET_KEY_ELEMENT = "secret-key-name"; + private static final String PROFILE_ELEMENT = "profile"; + + public static CloudWatch buildCloudWatch(Element cloudwatchElement, MetricsConsumer consumer) { + CloudWatch cloudWatch = new CloudWatch(cloudwatchElement.getAttribute(REGION_ATTRIBUTE), + cloudwatchElement.getAttribute(NAMESPACE_ATTRIBUTE), + consumer); + + getOptionalChildValue(cloudwatchElement, PROFILE_ELEMENT).ifPresent(cloudWatch::setProfile); + + getOptionalChildValue(cloudwatchElement, ACCESS_KEY_ELEMENT) + .ifPresent(accessKey -> cloudWatch.setHostedAuth( + new HostedAuth(accessKey, + getOptionalChildValue(cloudwatchElement, SECRET_KEY_ELEMENT) + .orElseThrow(() -> new IllegalArgumentException("Access key given without a secret key."))))); + return cloudWatch; + } + +} diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/builder/xml/MetricsBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/builder/xml/MetricsBuilder.java index f029dad01a9..b686288868f 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/builder/xml/MetricsBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/builder/xml/MetricsBuilder.java @@ -42,7 +42,11 @@ public class MetricsBuilder { throwIfIllegalConsumerId(metrics, consumerId); MetricSet metricSet = buildMetricSet(consumerId, consumerElement); - metrics.addConsumer(new MetricsConsumer(consumerId, metricSet)); + var consumer = new MetricsConsumer(consumerId, metricSet); + for (Element cloudwatchElement : XML.getChildren(consumerElement, "cloudwatch")) { + consumer.addCloudWatch(CloudWatchBuilder.buildCloudWatch(cloudwatchElement, consumer)); + } + metrics.addConsumer(consumer); } return metrics; } @@ -58,7 +62,7 @@ public class MetricsBuilder { private MetricSet buildMetricSet(String consumerId, Element consumerElement) { List<Metric> metrics = XML.getChildren(consumerElement, "metric").stream() - .map(metricElement -> metricFromElement(metricElement)) + .map(MetricsBuilder::metricFromElement) .collect(Collectors.toCollection(LinkedList::new)); List<MetricSet> metricSets = XML.getChildren(consumerElement, "metric-set").stream() diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/EndpointCertificateSecretsValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/EndpointCertificateSecretsValidator.java index f00ad0f0dbb..4b8bbd4ff08 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/EndpointCertificateSecretsValidator.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/EndpointCertificateSecretsValidator.java @@ -12,7 +12,7 @@ public class EndpointCertificateSecretsValidator extends Validator { @Override public void validate(VespaModel model, DeployState deployState) { if (deployState.endpointCertificateSecrets().isPresent() && deployState.endpointCertificateSecrets().get() == EndpointCertificateSecrets.MISSING) { - throw new CertificateNotReadyException("TLS enabled, but could not retrieve certificate yet"); + throw new CertificateNotReadyException("TLS enabled, but could not yet retrieve certificate for application " + deployState.getProperties().applicationId().serializedForm()); } } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java index 58679c63565..3632cb08da5 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java @@ -215,6 +215,7 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat super.getConfig(builder); builder.jvm.verbosegc(true) .availableProcessors(0) + .compressedClassSpaceSize(0) //TODO Reduce, next step is 512m .minHeapsize(1536) .heapsize(1536); if (getMemoryPercentage().isPresent()) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java index 6fa446bf365..3b132ab2342 100755 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java @@ -484,6 +484,7 @@ public abstract class ContainerCluster<CONTAINER extends Container> builder.jvm .verbosegc(false) .availableProcessors(2) + .compressedClassSpaceSize(32) .minHeapsize(32) .heapsize(512) .heapSizeAsPercentageOfPhysicalMemory(0) diff --git a/config-model/src/main/perl/vespa-deploy b/config-model/src/main/perl/vespa-deploy index 59a84f5b0c0..a128e4a8d4c 100755 --- a/config-model/src/main/perl/vespa-deploy +++ b/config-model/src/main/perl/vespa-deploy @@ -154,7 +154,7 @@ my $command = shift; $command ||= "help"; # The '--insecure' parameter is sadly required as it is not possible to disable or alter hostname verification with curl -my $curl_command = $VESPA_HOME . '/libexec/vespa/vespa-curl-wrapper --insecure -A vespa-deploy --silent --show-error --connect-timeout 30 --max-time 1200'; +my $curl_command = $VESPA_HOME . '/libexec/vespa/vespa-curl-wrapper -A vespa-deploy --silent --show-error --connect-timeout 30 --max-time 1200'; my $CURL_PUT = $curl_command . ' --write-out \%{http_code} --request PUT'; my $CURL_GET = $curl_command . ' --request GET'; diff --git a/config-model/src/main/resources/schema/admin.rnc b/config-model/src/main/resources/schema/admin.rnc index 7a3e2916f94..055f57dd7c0 100644 --- a/config-model/src/main/resources/schema/admin.rnc +++ b/config-model/src/main/resources/schema/admin.rnc @@ -82,10 +82,25 @@ Metrics = element metrics { element metric { attribute id { xsd:Name } & attribute display-name { xsd:Name }? - }* + }* & + Cloudwatch? }+ } +Cloudwatch = element cloudwatch { + attribute region { xsd:Name } & + attribute namespace { xsd:Name } & + ( + ( + element access-key-name { xsd:Name } & + element secret-key-name { xsd:Name } + ) + | + element profile { xsd:Name } + )? + +} + ClusterControllers = element cluster-controllers { attribute standalone-zookeeper { xsd:string }? & element cluster-controller { diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java index 9265e4437f1..e1b42854642 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java @@ -6,9 +6,9 @@ package com.yahoo.vespa.model.admin.metricsproxy; import ai.vespa.metricsproxy.core.ConsumersConfig; -import ai.vespa.metricsproxy.http.metrics.MetricsV1Handler; import ai.vespa.metricsproxy.http.application.ApplicationMetricsHandler; import ai.vespa.metricsproxy.http.application.MetricsNodesConfig; +import ai.vespa.metricsproxy.http.metrics.MetricsV1Handler; import ai.vespa.metricsproxy.http.prometheus.PrometheusHandler; import ai.vespa.metricsproxy.http.yamas.YamasHandler; import ai.vespa.metricsproxy.metric.dimensions.ApplicationDimensionsConfig; @@ -59,6 +59,8 @@ import static java.util.stream.Collectors.toList; import static org.hamcrest.CoreMatchers.endsWith; import static org.hamcrest.CoreMatchers.hasItem; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -105,10 +107,11 @@ public class MetricsProxyContainerClusterTest { assertEquals(512, qrStartConfig.jvm().heapsize()); assertEquals(0, qrStartConfig.jvm().heapSizeAsPercentageOfPhysicalMemory()); assertEquals(2, qrStartConfig.jvm().availableProcessors()); - assertEquals(false, qrStartConfig.jvm().verbosegc()); + assertFalse(qrStartConfig.jvm().verbosegc()); assertEquals("-XX:+UseG1GC -XX:MaxTenuringThreshold=15", qrStartConfig.jvm().gcopts()); assertEquals(512, qrStartConfig.jvm().stacksize()); assertEquals(0, qrStartConfig.jvm().directMemorySizeCache()); + assertEquals(32, qrStartConfig.jvm().compressedClassSpaceSize()); assertEquals(75, qrStartConfig.jvm().baseMaxDirectMemorySize()); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/TelegrafTest.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/TelegrafTest.java new file mode 100644 index 00000000000..144c45a7dd2 --- /dev/null +++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/TelegrafTest.java @@ -0,0 +1,87 @@ +package com.yahoo.vespa.model.admin.metricsproxy; + +import ai.vespa.metricsproxy.telegraf.TelegrafConfig; +import com.yahoo.vespa.model.VespaModel; +import org.junit.Test; + +import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.CLUSTER_CONFIG_ID; +import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.TestMode.hosted; +import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.getModel; +import static org.junit.Assert.assertEquals; + +/** + * @author gjoranv + */ +public class TelegrafTest { + + @Test + public void telegraf_config_is_generated_for_cloudwatch_in_services() { + String services = String.join("\n", + "<services>", + " <admin version='2.0'>", + " <adminserver hostalias='node1'/>", + " <metrics>", + " <consumer id='cloudwatch-consumer'>", + " <metric id='my-metric'/>", + " <cloudwatch region='us-east-1' namespace='my-namespace' >", + " <access-key-name>my-access-key</access-key-name>", + " <secret-key-name>my-secret-key</secret-key-name>", + " </cloudwatch>", + " </consumer>", + " </metrics>", + " </admin>", + "</services>" + ); + VespaModel hostedModel = getModel(services, hosted); + TelegrafConfig config = hostedModel.getConfig(TelegrafConfig.class, CLUSTER_CONFIG_ID); + var cloudWatch0 = config.cloudWatch(0); + assertEquals("cloudwatch-consumer", cloudWatch0.consumer()); + assertEquals("us-east-1", cloudWatch0.region()); + assertEquals("my-namespace", cloudWatch0.namespace()); + assertEquals("my-access-key", cloudWatch0.accessKeyName()); + assertEquals("my-secret-key", cloudWatch0.secretKeyName()); + assertEquals("", cloudWatch0.profile()); + } + + @Test + public void multiple_cloudwatches_are_allowed_for_the_same_consumer() { + String services = String.join("\n", + "<services>", + " <admin version='2.0'>", + " <adminserver hostalias='node1'/>", + " <metrics>", + " <consumer id='cloudwatch-consumer'>", + " <metric id='my-metric'/>", + " <cloudwatch region='us-east-1' namespace='namespace-1' >", + " <access-key-name>access-key-1</access-key-name>", + " <secret-key-name>secret-key-1</secret-key-name>", + " </cloudwatch>", + " <cloudwatch region='us-east-1' namespace='namespace-2' >", + " <profile>profile-2</profile>", + " </cloudwatch>", + " </consumer>", + " </metrics>", + " </admin>", + "</services>" + ); + VespaModel hostedModel = getModel(services, hosted); + TelegrafConfig config = hostedModel.getConfig(TelegrafConfig.class, CLUSTER_CONFIG_ID); + + var cloudWatch0 = config.cloudWatch(0); + assertEquals("cloudwatch-consumer", cloudWatch0.consumer()); + assertEquals("us-east-1", cloudWatch0.region()); + assertEquals("namespace-1", cloudWatch0.namespace()); + assertEquals("access-key-1", cloudWatch0.accessKeyName()); + assertEquals("secret-key-1", cloudWatch0.secretKeyName()); + assertEquals("", cloudWatch0.profile()); + + var cloudWatch1 = config.cloudWatch(1); + assertEquals("cloudwatch-consumer", cloudWatch1.consumer()); + assertEquals("us-east-1", cloudWatch1.region()); + assertEquals("namespace-2", cloudWatch1.namespace()); + assertEquals("", cloudWatch1.accessKeyName()); + assertEquals("", cloudWatch1.secretKeyName()); + assertEquals("profile-2", cloudWatch1.profile()); + } + +} diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/EndpointCertificateSecretsValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/EndpointCertificateSecretsValidatorTest.java index 21df39ebde8..318a0630c4d 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/EndpointCertificateSecretsValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/EndpointCertificateSecretsValidatorTest.java @@ -47,7 +47,7 @@ public class EndpointCertificateSecretsValidatorTest { VespaModel model = new VespaModel(new NullConfigModelRegistry(), deployState); exceptionRule.expect(CertificateNotReadyException.class); - exceptionRule.expectMessage("TLS enabled, but could not retrieve certificate yet"); + exceptionRule.expectMessage("TLS enabled, but could not yet retrieve certificate for application default:default:default"); new EndpointCertificateSecretsValidator().validate(model, deployState); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java index d1c5e344c24..ce565989c18 100755 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/ContainerClusterTest.java @@ -97,6 +97,7 @@ public class ContainerClusterTest { cluster.getConfig(qsB); QrStartConfig qsC= new QrStartConfig(qsB); assertEquals(expectedMemoryPercentage, qsC.jvm().heapSizeAsPercentageOfPhysicalMemory()); + assertEquals(0, qsC.jvm().compressedClassSpaceSize()); } @Test @@ -156,6 +157,7 @@ public class ContainerClusterTest { QrStartConfig qrStartConfig = new QrStartConfig(qrBuilder); assertEquals(32, qrStartConfig.jvm().minHeapsize()); assertEquals(512, qrStartConfig.jvm().heapsize()); + assertEquals(32, qrStartConfig.jvm().compressedClassSpaceSize()); assertEquals(0, qrStartConfig.jvm().heapSizeAsPercentageOfPhysicalMemory()); ThreadpoolConfig.Builder tpBuilder = new ThreadpoolConfig.Builder(); diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/DocprocBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/DocprocBuilderTest.java index 9ecd33f4273..eda90b03147 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/DocprocBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/DocprocBuilderTest.java @@ -28,7 +28,11 @@ import java.util.Map; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.core.IsNull.notNullValue; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertThat; + /** * @author einarmr @@ -204,24 +208,25 @@ public class DocprocBuilderTest extends DomBuilderTest { @Test public void testBundlesConfig() { - assertThat(bundlesConfig.bundle().size(), is(0)); + assertTrue(bundlesConfig.bundle().isEmpty()); } @Test public void testSchemaMappingConfig() { - assertThat(schemamappingConfig.fieldmapping().size(), is(0)); + assertTrue(schemamappingConfig.fieldmapping().isEmpty()); } @Test public void testQrStartConfig() { QrStartConfig.Jvm jvm = qrStartConfig.jvm(); - assertThat(jvm.server(), is(true)); - assertThat(jvm.verbosegc(), is(true)); - assertThat(jvm.gcopts(), is("-XX:+UseG1GC -XX:MaxTenuringThreshold=15")); - assertThat(jvm.minHeapsize(), is(1536)); - assertThat(jvm.heapsize(), is(1536)); - assertThat(jvm.stacksize(), is(512)); - assertThat(qrStartConfig.ulimitv(), is("")); + assertTrue(jvm.server()); + assertTrue(jvm.verbosegc()); + assertEquals("-XX:+UseG1GC -XX:MaxTenuringThreshold=15", jvm.gcopts()); + assertEquals(1536, jvm.minHeapsize()); + assertEquals(1536, jvm.heapsize()); + assertEquals(512, jvm.stacksize()); + assertTrue(qrStartConfig.ulimitv().isEmpty()); + assertEquals(0, jvm.compressedClassSpaceSize()); } } diff --git a/config-model/src/test/schema-test-files/services.xml b/config-model/src/test/schema-test-files/services.xml index 1bf42650123..b06c93d6406 100644 --- a/config-model/src/test/schema-test-files/services.xml +++ b/config-model/src/test/schema-test-files/services.xml @@ -15,14 +15,25 @@ <slobrok hostalias="rtc-1" /> </slobroks> <metrics> - <consumer id="my-consumer"> + <consumer id="cloudwatch-hosted"> <metric-set id="my-set" /> <metric id="my-metric"/> <metric id="my-metric2" display-name="my-metric3"/> <metric display-name="my-metric4" id="my-metric4.avg"/> + <cloudwatch region="us-east1" namespace="my-namespace"> + <access-key-name>my-access-key</access-key-name> + <secret-key-name>my-secret-key</secret-key-name> + </cloudwatch> </consumer> - <consumer id="my-consumer2"> - <metric-set id="my-set2" /> + <consumer id="cloudwatch-self-hosted-with-default-auth"> + <metric-set id="public" /> + <cloudwatch region="us-east1" namespace="my-namespace" /> + </consumer> + <consumer id="cloudwatch-self-hosted-with-profile"> + <metric id="my-custom-metric" /> + <cloudwatch region="us-east1" namespace="another-namespace"> + <profile>profile-in-credentials-file</profile> + </cloudwatch> </consumer> </metrics> <logforwarding> diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/RoutingMethod.java b/config-provisioning/src/main/java/com/yahoo/config/provision/zone/RoutingMethod.java index 892ac639198..765496bab56 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/zone/RoutingMethod.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/zone/RoutingMethod.java @@ -14,4 +14,6 @@ public enum RoutingMethod { /** Routing happens through a dedicated layer 4 load balancer */ exclusive, + /** Routing happens through a shared layer 4 load balancer */ + shared_layer_4 } diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java index d77206aee81..0966de940f1 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java @@ -70,12 +70,11 @@ public class ProxyServer implements Runnable { defaultTimingValues = tv; } - ProxyServer(Spec spec, ConfigSourceSet source, TimingValues timingValues, - MemoryCache memoryCache, ConfigSourceClient configClient) { + ProxyServer(Spec spec, ConfigSourceSet source, MemoryCache memoryCache, ConfigSourceClient configClient) { this.delayedResponses = new DelayedResponses(); this.configSource = source; log.log(LogLevel.DEBUG, "Using config source '" + source); - this.timingValues = timingValues; + this.timingValues = defaultTimingValues; this.memoryCache = memoryCache; this.rpcServer = createRpcServer(spec); this.configClient = createClient(rpcServer, delayedResponses, source, timingValues, memoryCache, configClient); @@ -181,8 +180,7 @@ public class ProxyServer implements Runnable { Event.started("configproxy"); ConfigSourceSet configSources = new ConfigSourceSet(properties.configSources); - ProxyServer proxyServer = new ProxyServer(new Spec(null, port), configSources, - defaultTimingValues(), new MemoryCache(), null); + ProxyServer proxyServer = new ProxyServer(new Spec(null, port), configSources, new MemoryCache(), null); // catch termination and interrupt signal proxyServer.setupSignalHandler(); Thread proxyserverThread = new Thread(proxyServer); diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java index ee843088086..47afbe83bb6 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java @@ -11,13 +11,14 @@ import com.yahoo.jrt.Supervisor; import com.yahoo.jrt.Target; import com.yahoo.jrt.Transport; import com.yahoo.log.LogLevel; -import com.yahoo.vespa.config.*; +import com.yahoo.vespa.config.ConfigCacheKey; +import com.yahoo.vespa.config.RawConfig; +import com.yahoo.vespa.config.TimingValues; import com.yahoo.vespa.config.protocol.JRTServerConfigRequest; import java.util.ArrayList; import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.concurrent.DelayQueue; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -42,7 +43,7 @@ class RpcConfigSourceClient implements ConfigSourceClient { private final TimingValues timingValues; private final ExecutorService exec; - private final Map<ConfigSourceSet, JRTConfigRequester> requesterPool; + private final JRTConfigRequester requester; RpcConfigSourceClient(RpcServer rpcServer, @@ -57,20 +58,7 @@ class RpcConfigSourceClient implements ConfigSourceClient { this.timingValues = timingValues; checkConfigSources(); exec = Executors.newCachedThreadPool(new DaemonThreadFactory("subscriber-")); - requesterPool = createRequesterPool(configSourceSet, timingValues); - } - - /** - * Creates a requester (connection) pool of one entry, to be used each time this {@link RpcConfigSourceClient} is used - * @param ccs a {@link ConfigSourceSet} - * @param timingValues a {@link TimingValues} - * @return requester map - */ - private Map<ConfigSourceSet, JRTConfigRequester> createRequesterPool(ConfigSourceSet ccs, TimingValues timingValues) { - Map<ConfigSourceSet, JRTConfigRequester> ret = new HashMap<>(); - if (ccs.getSources().isEmpty()) return ret; // unit test, just skip creating any requester - ret.put(ccs, new JRTConfigRequester(new JRTConnectionPool(ccs), timingValues)); - return ret; + requester = JRTConfigRequester.create(configSourceSet, timingValues); } /** @@ -153,7 +141,7 @@ class RpcConfigSourceClient implements ConfigSourceClient { } else { log.log(LogLevel.DEBUG, () -> "Could not find good config in cache, creating subscriber for: " + configCacheKey); UpstreamConfigSubscriber subscriber = new UpstreamConfigSubscriber(input, this, configSourceSet, - timingValues, requesterPool, memoryCache); + timingValues, requester, memoryCache); try { subscriber.subscribe(); activeSubscribers.put(configCacheKey, subscriber); @@ -183,25 +171,18 @@ class RpcConfigSourceClient implements ConfigSourceClient { activeSubscribers.clear(); } exec.shutdown(); - for (JRTConfigRequester requester : requesterPool.values()) { - requester.close(); - } + requester.close(); } @Override public String getActiveSourceConnection() { - if (requesterPool.get(configSourceSet) != null) { - return requesterPool.get(configSourceSet).getConnectionPool().getCurrent().getAddress(); - } else { - return ""; - } + return requester.getConnectionPool().getCurrent().getAddress(); } @Override public List<String> getSourceConnections() { ArrayList<String> ret = new ArrayList<>(); - final JRTConfigRequester jrtConfigRequester = requesterPool.get(configSourceSet); - if (jrtConfigRequester != null) { + if (configSourceSet != null) { ret.addAll(configSourceSet.getSources()); } return ret; diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/UpstreamConfigSubscriber.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/UpstreamConfigSubscriber.java index f8df16cb3d2..d8a8c5ce941 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/UpstreamConfigSubscriber.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/UpstreamConfigSubscriber.java @@ -1,16 +1,15 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.proxy; -import com.yahoo.config.subscription.ConfigSource; import com.yahoo.config.subscription.ConfigSourceSet; import com.yahoo.config.subscription.impl.GenericConfigHandle; import com.yahoo.config.subscription.impl.GenericConfigSubscriber; import com.yahoo.config.subscription.impl.JRTConfigRequester; import com.yahoo.log.LogLevel; import com.yahoo.vespa.config.ConfigKey; -import com.yahoo.yolean.Exceptions; import com.yahoo.vespa.config.RawConfig; import com.yahoo.vespa.config.TimingValues; +import com.yahoo.yolean.Exceptions; import java.util.Map; import java.util.logging.Logger; @@ -24,26 +23,26 @@ public class UpstreamConfigSubscriber implements Subscriber { private final RawConfig config; private final ConfigSourceClient configSourceClient; - private final ConfigSource configSourceSet; + private final ConfigSourceSet configSourceSet; private final TimingValues timingValues; - private final Map<ConfigSourceSet, JRTConfigRequester> requesterPool; + private final JRTConfigRequester requester; private final MemoryCache memoryCache; private GenericConfigSubscriber subscriber; private GenericConfigHandle handle; - UpstreamConfigSubscriber(RawConfig config, ConfigSourceClient configSourceClient, ConfigSource configSourceSet, - TimingValues timingValues, Map<ConfigSourceSet, JRTConfigRequester> requesterPool, + UpstreamConfigSubscriber(RawConfig config, ConfigSourceClient configSourceClient, ConfigSourceSet configSourceSet, + TimingValues timingValues, JRTConfigRequester requester, MemoryCache memoryCache) { this.config = config; this.configSourceClient = configSourceClient; this.configSourceSet = configSourceSet; this.timingValues = timingValues; - this.requesterPool = requesterPool; + this.requester = requester; this.memoryCache = memoryCache; } void subscribe() { - subscriber = new GenericConfigSubscriber(requesterPool); + subscriber = new GenericConfigSubscriber(Map.of(configSourceSet, requester)); ConfigKey<?> key = config.getKey(); handle = subscriber.subscribe(new ConfigKey<>(key.getName(), key.getConfigId(), key.getNamespace()), config.getDefContent(), configSourceSet, timingValues); diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServerTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServerTest.java index dc1c995fbb5..29bd38ea891 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServerTest.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServerTest.java @@ -264,7 +264,7 @@ public class ConfigProxyRpcServerTest { } private static ProxyServer createTestServer(ConfigSourceSet source) { - return new ProxyServer(null, source, ProxyServer.defaultTimingValues(), new MemoryCache(), null); + return new ProxyServer(null, source, new MemoryCache(), null); } private static class TestServer implements AutoCloseable { diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java index 712567774f1..bc35a8670a3 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java @@ -14,7 +14,11 @@ import org.junit.rules.TemporaryFolder; import java.util.Optional; import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; /** * @author hmusum @@ -222,7 +226,7 @@ public class ProxyServerTest { private static ProxyServer createTestServer(ConfigSourceSet source, ConfigSourceClient configSourceClient, MemoryCache memoryCache) { - return new ProxyServer(null, source, ProxyServer.defaultTimingValues(), memoryCache, configSourceClient); + return new ProxyServer(null, source, memoryCache, configSourceClient); } static RawConfig createConfigWithNextConfigGeneration(RawConfig config, int errorCode) { diff --git a/config/src/main/java/com/yahoo/config/subscription/ConfigSourceSet.java b/config/src/main/java/com/yahoo/config/subscription/ConfigSourceSet.java index c799186435c..7472439d6a4 100755 --- a/config/src/main/java/com/yahoo/config/subscription/ConfigSourceSet.java +++ b/config/src/main/java/com/yahoo/config/subscription/ConfigSourceSet.java @@ -3,7 +3,11 @@ package com.yahoo.config.subscription; import com.yahoo.log.LogLevel; -import java.util.*; +import java.util.Arrays; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; import java.util.logging.Logger; diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java index 6d08976b61c..49c5dcd343c 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java @@ -20,7 +20,6 @@ import com.yahoo.yolean.Exceptions; import java.time.Duration; import java.time.Instant; import java.util.concurrent.ScheduledThreadPoolExecutor; -import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; @@ -37,15 +36,17 @@ public class JRTConfigRequester implements RequestWaiter { private static final Logger log = Logger.getLogger(JRTConfigRequester.class.getName()); public static final ConfigSourceSet defaultSourceSet = ConfigSourceSet.createDefault(); + private static final JRTManagedConnectionPools managedPool = new JRTManagedConnectionPools(); private static final int TRACELEVEL = 6; private final TimingValues timingValues; private int fatalFailures = 0; // independent of transientFailures private int transientFailures = 0; // independent of fatalFailures - private final ScheduledThreadPoolExecutor scheduler = new ScheduledThreadPoolExecutor(1, new JRTSourceThreadFactory()); + private final ScheduledThreadPoolExecutor scheduler; private Instant suspendWarningLogged = Instant.MIN; private Instant noApplicationWarningLogged = Instant.MIN; private static final Duration delayBetweenWarnings = Duration.ofSeconds(60); private final ConnectionPool connectionPool; + private final ConfigSourceSet configSourceSet; static final float randomFraction = 0.2f; /* Time to be added to server timeout to create client timeout. This is the time allowed for the server to respond after serverTimeout has elapsed. */ private static final Double additionalTimeForClientTimeout = 10.0; @@ -56,11 +57,23 @@ public class JRTConfigRequester implements RequestWaiter { * @param connectionPool the connectionPool this requester should use * @param timingValues timeouts and delays used when sending JRT config requests */ - public JRTConfigRequester(ConnectionPool connectionPool, TimingValues timingValues) { + JRTConfigRequester(ConfigSourceSet configSourceSet, ScheduledThreadPoolExecutor scheduler, + ConnectionPool connectionPool, TimingValues timingValues) { + this.configSourceSet = configSourceSet; + this.scheduler = scheduler; this.connectionPool = connectionPool; this.timingValues = timingValues; } + /** Only for testing */ + public JRTConfigRequester(ConnectionPool connectionPool, TimingValues timingValues) { + this(null, new ScheduledThreadPoolExecutor(1), connectionPool, timingValues); + } + + public static JRTConfigRequester create(ConfigSourceSet sourceSet, TimingValues timingValues) { + return managedPool.acquire(sourceSet, timingValues); + } + /** * Requests the config for the {@link com.yahoo.config.ConfigInstance} on the given {@link ConfigSubscription} * @@ -273,18 +286,8 @@ public class JRTConfigRequester implements RequestWaiter { // Fake that we have logged to avoid printing warnings after this suspendWarningLogged = Instant.now(); noApplicationWarningLogged = Instant.now(); - - connectionPool.close(); - scheduler.shutdown(); - } - - private static class JRTSourceThreadFactory implements ThreadFactory { - @Override - public Thread newThread(Runnable runnable) { - Thread t = new Thread(runnable, String.format("jrt-config-requester-%d", System.currentTimeMillis())); - // We want a daemon thread to avoid hanging threads in case something goes wrong in the config system - t.setDaemon(true); - return t; + if (configSourceSet != null) { + managedPool.release(configSourceSet); } } diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java index 39e6c69f539..a94a135f9d8 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java @@ -28,7 +28,7 @@ import com.yahoo.vespa.config.protocol.Payload; public class JRTConfigSubscription<T extends ConfigInstance> extends ConfigSubscription<T> { private JRTConfigRequester requester; - private TimingValues timingValues; + private final TimingValues timingValues; // Last time we got an OK JRT callback private Instant lastOK = Instant.MIN; @@ -156,7 +156,7 @@ public class JRTConfigSubscription<T extends ConfigInstance> extends ConfigSubsc private JRTConfigRequester getRequester() { JRTConfigRequester requester = subscriber.requesters().get(sources); if (requester == null) { - requester = new JRTConfigRequester(new JRTConnectionPool(sources), timingValues); + requester = JRTConfigRequester.create(sources, timingValues); subscriber.requesters().put(sources, requester); } return requester; diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/JRTManagedConnectionPools.java b/config/src/main/java/com/yahoo/config/subscription/impl/JRTManagedConnectionPools.java new file mode 100644 index 00000000000..0a606416827 --- /dev/null +++ b/config/src/main/java/com/yahoo/config/subscription/impl/JRTManagedConnectionPools.java @@ -0,0 +1,63 @@ +package com.yahoo.config.subscription.impl; + +import com.yahoo.config.subscription.ConfigSourceSet; +import com.yahoo.vespa.config.JRTConnectionPool; +import com.yahoo.vespa.config.TimingValues; + +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; + +public class JRTManagedConnectionPools { + private static class JRTSourceThreadFactory implements ThreadFactory { + @Override + public Thread newThread(Runnable runnable) { + Thread t = new Thread(runnable, String.format("jrt-config-requester-%d", System.currentTimeMillis())); + // We want a daemon thread to avoid hanging threads in case something goes wrong in the config system + t.setDaemon(true); + return t; + } + } + private static class CountedPool { + long count; + final JRTConnectionPool pool; + final ScheduledThreadPoolExecutor scheduler = new ScheduledThreadPoolExecutor(1, new JRTSourceThreadFactory()); + CountedPool(JRTConnectionPool requester) { + this.pool = requester; + count = 0; + } + } + private Map<ConfigSourceSet, CountedPool> pools = new HashMap<>(); + + public JRTConfigRequester acquire(ConfigSourceSet sourceSet, TimingValues timingValues) { + CountedPool countedPool; + synchronized (pools) { + countedPool = pools.get(sourceSet); + if (countedPool == null) { + countedPool = new CountedPool(new JRTConnectionPool(sourceSet)); + pools.put(sourceSet, countedPool); + } + countedPool.count++; + } + return new JRTConfigRequester(sourceSet, countedPool.scheduler, countedPool.pool, timingValues); + } + public synchronized void release(ConfigSourceSet sourceSet) { + CountedPool countedPool; + synchronized (pools) { + countedPool = pools.get(sourceSet); + countedPool.count--; + if (countedPool.count > 0) return; + pools.remove(sourceSet); + } + + countedPool.pool.close(); + countedPool.scheduler.shutdown(); + try { + countedPool.scheduler.awaitTermination(30, TimeUnit.SECONDS); + } catch (InterruptedException e) { + throw new RuntimeException("Failed shutting down scheduler:", e); + } + } +} diff --git a/config/src/main/java/com/yahoo/vespa/config/JRTConnectionPool.java b/config/src/main/java/com/yahoo/vespa/config/JRTConnectionPool.java index a4da996effd..326c1287468 100644 --- a/config/src/main/java/com/yahoo/vespa/config/JRTConnectionPool.java +++ b/config/src/main/java/com/yahoo/vespa/config/JRTConnectionPool.java @@ -112,19 +112,6 @@ public class JRTConnectionPool implements ConnectionPool { return this; } - public String getAllSourceAddresses() { - StringBuilder sb = new StringBuilder(); - synchronized (connections) { - for (JRTConnection conn : connections.values()) { - sb.append(conn.getAddress()); - sb.append(","); - } - } - // Remove trailing "," - sb.deleteCharAt(sb.length() - 1); - return sb.toString(); - } - public String toString() { StringBuilder sb = new StringBuilder(); synchronized (connections) { diff --git a/config/src/main/java/com/yahoo/vespa/config/TimingValues.java b/config/src/main/java/com/yahoo/vespa/config/TimingValues.java index 780cf657009..5d5967e56c4 100644 --- a/config/src/main/java/com/yahoo/vespa/config/TimingValues.java +++ b/config/src/main/java/com/yahoo/vespa/config/TimingValues.java @@ -12,12 +12,11 @@ public class TimingValues { public static final long defaultNextConfigTimeout = 1000; // See getters below for an explanation of how these values are used and interpreted // All time values in milliseconds. - private long successTimeout = 600000; - private long errorTimeout = 20000; - private long initialTimeout = 15000; + private final long successTimeout; + private final long errorTimeout; + private final long initialTimeout; private long subscribeTimeout = 55000; private long configuredErrorTimeout = -1; // Don't ever timeout (and do not use error response) when we are already configured - private long nextConfigTimeout = defaultNextConfigTimeout; private long fixedDelay = 5000; private long unconfiguredDelay = 1000; @@ -26,6 +25,9 @@ public class TimingValues { private final Random rand; public TimingValues() { + successTimeout = 600000; + errorTimeout = 20000; + initialTimeout = 15000; this.rand = new Random(System.currentTimeMillis()); } @@ -100,20 +102,6 @@ public class TimingValues { } /** - * Returns initial timeout to use as server timeout when a config is requested for the first time. - * - * @return timeout in milliseconds. - */ - public long getInitialTimeout() { - return initialTimeout; - } - - public TimingValues setInitialTimeout(long t) { - initialTimeout = t; - return this; - } - - /** * Returns timeout to use as server timeout when subscribing for the first time. * * @return timeout in milliseconds. @@ -127,38 +115,12 @@ public class TimingValues { return this; } - /** - * Returns the time to retry getting config from the remote sources, until the next error response will - * be set as config. Counted from the last ok request was received. A negative value means that - * we will always retry getting config and never set an error response as config. - * - * @return timeout in milliseconds. - */ - public long getConfiguredErrorTimeout() { - return configuredErrorTimeout; - } - public TimingValues setConfiguredErrorTimeout(long t) { configuredErrorTimeout = t; return this; } /** - * Returns timeout used when calling {@link com.yahoo.config.subscription.ConfigSubscriber#nextConfig()} or - * {@link com.yahoo.config.subscription.ConfigSubscriber#nextGeneration()} - * - * @return timeout in milliseconds. - */ - public long getNextConfigTimeout() { - return nextConfigTimeout; - } - - public TimingValues setNextConfigTimeout(long t) { - nextConfigTimeout = t; - return this; - } - - /** * Returns time to wait until next attempt to get config after a failed request when the client has not * gotten a successful response to a config subscription (i.e, the client has not been configured). * A negative value means that there will never be a next attempt. If a negative value is set, the @@ -201,12 +163,6 @@ public class TimingValues { return maxDelayMultiplier; } - - public TimingValues setSuccessTimeout(long successTimeout) { - this.successTimeout = successTimeout; - return this; - } - /** * Returns fixed delay that is used when retrying getting config no matter if it was a success or an error * and independent of number of retries. @@ -228,10 +184,6 @@ public class TimingValues { return Math.round(val - (val * fraction) + (rand.nextFloat() * 2L * val * fraction)); } - Random getRandom() { - return rand; - } - @Override public String toString() { return "TimingValues [successTimeout=" + successTimeout diff --git a/config/src/test/java/com/yahoo/config/subscription/BasicTest.java b/config/src/test/java/com/yahoo/config/subscription/BasicTest.java index 3b8b7db6487..5b145d40b7f 100644 --- a/config/src/test/java/com/yahoo/config/subscription/BasicTest.java +++ b/config/src/test/java/com/yahoo/config/subscription/BasicTest.java @@ -1,13 +1,13 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.subscription; -import static org.junit.Assert.*; -import static org.hamcrest.CoreMatchers.is; import com.yahoo.foo.AppConfig; import org.junit.Test; +import static org.junit.Assert.assertEquals; + public class BasicTest { @@ -17,7 +17,8 @@ public class BasicTest { ConfigHandle<AppConfig> h = s.subscribe(AppConfig.class, "raw:times 0"); s.nextConfig(0); AppConfig c = h.getConfig(); - assertThat(c.times(), is(0)); + assertEquals(0, c.times()); + s.close(); } @Test @@ -26,6 +27,7 @@ public class BasicTest { ConfigHandle<AppConfig> h = s.subscribe(AppConfig.class, "raw:times 2"); s.nextGeneration(0); AppConfig c = h.getConfig(); - assertThat(c.times(), is(2)); + assertEquals(2, c.times()); + s.close(); } } diff --git a/config/src/test/java/com/yahoo/config/subscription/ConfigSetSubscriptionTest.java b/config/src/test/java/com/yahoo/config/subscription/ConfigSetSubscriptionTest.java index 21cdfbe7d30..db30e7b7389 100644 --- a/config/src/test/java/com/yahoo/config/subscription/ConfigSetSubscriptionTest.java +++ b/config/src/test/java/com/yahoo/config/subscription/ConfigSetSubscriptionTest.java @@ -128,6 +128,7 @@ public class ConfigSetSubscriptionTest { assertEquals(hA0.getConfig().times(), 8800); assertEquals(hA1.getConfig().times(), 890); assertEquals(hS.getConfig().stringVal(), "new StringVal"); + subscriber.close(); } @Test diff --git a/config/src/test/java/com/yahoo/config/subscription/ConfigSubscriptionTest.java b/config/src/test/java/com/yahoo/config/subscription/ConfigSubscriptionTest.java index 933a9fd130a..c8d4c081fc9 100644 --- a/config/src/test/java/com/yahoo/config/subscription/ConfigSubscriptionTest.java +++ b/config/src/test/java/com/yahoo/config/subscription/ConfigSubscriptionTest.java @@ -60,6 +60,7 @@ public class ConfigSubscriptionTest { assertEquals(c1, c1); assertNotEquals(c1, c2); + sub.close(); } @Test @@ -70,6 +71,7 @@ public class ConfigSubscriptionTest { sub.nextConfig(); assertTrue(handle.getConfig().boolval()); //assertTrue(sub.getSource() instanceof RawSource); + sub.close(); } // Test that subscription is closed and subscriptionHandles is empty if we get an exception diff --git a/config/src/test/java/com/yahoo/config/subscription/GenericConfigSubscriberTest.java b/config/src/test/java/com/yahoo/config/subscription/GenericConfigSubscriberTest.java index e9dc9cf7b98..9c83f2f3c9a 100644 --- a/config/src/test/java/com/yahoo/config/subscription/GenericConfigSubscriberTest.java +++ b/config/src/test/java/com/yahoo/config/subscription/GenericConfigSubscriberTest.java @@ -49,14 +49,17 @@ public class GenericConfigSubscriberTest { public void testGenericRequesterPooling() { ConfigSourceSet source1 = new ConfigSourceSet("tcp/foo:78"); ConfigSourceSet source2 = new ConfigSourceSet("tcp/bar:79"); - JRTConfigRequester req1 = new JRTConfigRequester(new JRTConnectionPool(source1), JRTConfigRequesterTest.getTestTimingValues()); - JRTConfigRequester req2 = new JRTConfigRequester(new JRTConnectionPool(source2), JRTConfigRequesterTest.getTestTimingValues()); + JRTConfigRequester req1 = JRTConfigRequester.create(source1, JRTConfigRequesterTest.getTestTimingValues()); + JRTConfigRequester req2 = JRTConfigRequester.create(source2, JRTConfigRequesterTest.getTestTimingValues()); Map<ConfigSourceSet, JRTConfigRequester> requesters = new LinkedHashMap<>(); requesters.put(source1, req1); requesters.put(source2, req2); GenericConfigSubscriber sub = new GenericConfigSubscriber(requesters); assertEquals(sub.requesters().get(source1).getConnectionPool().getCurrent().getAddress(), "tcp/foo:78"); assertEquals(sub.requesters().get(source2).getConnectionPool().getCurrent().getAddress(), "tcp/bar:79"); + for (JRTConfigRequester requester : requesters.values()) { + requester.close(); + } } @Test(expected=UnsupportedOperationException.class) diff --git a/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java b/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java index 757dd99f43b..4211345dff7 100644 --- a/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java +++ b/config/src/test/java/com/yahoo/config/subscription/impl/JRTConfigRequesterTest.java @@ -1,10 +1,12 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.subscription.impl; +import com.yahoo.config.subscription.ConfigSourceSet; import com.yahoo.foo.SimpletypesConfig; import com.yahoo.config.subscription.ConfigSubscriber; import com.yahoo.jrt.Request; import com.yahoo.vespa.config.ConfigKey; +import com.yahoo.vespa.config.ConnectionPool; import com.yahoo.vespa.config.ErrorCode; import com.yahoo.vespa.config.ErrorType; import com.yahoo.vespa.config.TimingValues; @@ -17,6 +19,8 @@ import java.util.Random; import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -349,4 +353,23 @@ public class JRTConfigRequesterTest { } } + @Test + public void testManagedPool() { + ConfigSourceSet sourceSet = ConfigSourceSet.createDefault(); + TimingValues timingValues = new TimingValues(); + JRTConfigRequester requester1 = JRTConfigRequester.create(sourceSet, timingValues); + JRTConfigRequester requester2 = JRTConfigRequester.create(sourceSet, timingValues); + assertNotSame(requester1, requester2); + assertSame(requester1.getConnectionPool(), requester2.getConnectionPool()); + ConnectionPool firstPool = requester1.getConnectionPool(); + requester1.close(); + requester2.close(); + requester1 = JRTConfigRequester.create(sourceSet, timingValues); + assertNotSame(firstPool, requester1.getConnectionPool()); + requester2 = JRTConfigRequester.create(new ConfigSourceSet("test-managed-pool-2"), timingValues); + assertNotSame(requester1.getConnectionPool(), requester2.getConnectionPool()); + requester1.close(); + requester2.close(); + } + } diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/ThreadedRequestHandler.java b/container-core/src/main/java/com/yahoo/container/jdisc/ThreadedRequestHandler.java index bfcecd61fa4..6c8cee8433c 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/ThreadedRequestHandler.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/ThreadedRequestHandler.java @@ -17,6 +17,7 @@ import com.yahoo.jdisc.handler.ResponseDispatch; import com.yahoo.jdisc.handler.ResponseHandler; import com.yahoo.log.LogLevel; +import java.net.URI; import java.time.Duration; import java.util.HashMap; import java.util.Map; @@ -91,6 +92,11 @@ public abstract class ThreadedRequestHandler extends AbstractRequestHandler { if (endpoint != null) { dimensions.put("endpoint", endpoint); } + URI uri = request.getUri(); + dimensions.put("scheme", uri.getScheme()); + dimensions.put("port", Integer.toString(uri.getPort())); + String handlerClassName = getClass().getName(); + dimensions.put("handler-name", handlerClassName); return this.metric.createContext(dimensions); } diff --git a/container-disc/src/main/sh/vespa-start-container-daemon.sh b/container-disc/src/main/sh/vespa-start-container-daemon.sh index 382843b5688..f097d6d72bc 100755 --- a/container-disc/src/main/sh/vespa-start-container-daemon.sh +++ b/container-disc/src/main/sh/vespa-start-container-daemon.sh @@ -64,6 +64,7 @@ configure_memory() { consider_fallback jvm_heapsize 1536 consider_fallback jvm_stacksize 512 consider_fallback jvm_baseMaxDirectMemorySize 75 + consider_fallback jvm_compressedClassSpaceSize 32 consider_fallback jvm_directMemorySizeCache 0 # Update jvm_heapsize only if percentage is explicitly set (default is 0). @@ -80,16 +81,20 @@ configure_memory() { fi # Safety measure against bad min vs max heapsize. - if ((jvm_minHeapsize > jvm_heapsize)); then + if ((jvm_minHeapsize > jvm_heapsize)); then jvm_minHeapsize=${jvm_heapsize} echo "Misconfigured heap size, jvm_minHeapsize(${jvm_minHeapsize} is larger than jvm_heapsize(${jvm_heapsize}). It has been capped." - fi + fi maxDirectMemorySize=$(( jvm_baseMaxDirectMemorySize + jvm_heapsize / 8 + jvm_directMemorySizeCache )) memory_options="-Xms${jvm_minHeapsize}m -Xmx${jvm_heapsize}m" memory_options="${memory_options} -XX:ThreadStackSize=${jvm_stacksize}" - memory_options="${memory_options} -XX:MaxDirectMemorySize=${maxDirectMemorySize}m" + memory_options="${memory_options} -XX:MaxDirectMemorySize=${maxDirectMemorySize}m" + + if ((jvm_compressedClassSpaceSize != 0)); then + memory_options="${memory_options} -XX:CompressedClassSpaceSize=${compressedClassSpaceSize}m" + fi if [ "${VESPA_USE_HUGEPAGES}" ]; then memory_options="${memory_options} -XX:+UseLargePages" diff --git a/container-search/src/main/resources/configdefinitions/qr-start.def b/container-search/src/main/resources/configdefinitions/qr-start.def index 031877ada81..95e9d4575dd 100644 --- a/container-search/src/main/resources/configdefinitions/qr-start.def +++ b/container-search/src/main/resources/configdefinitions/qr-start.def @@ -20,6 +20,9 @@ jvm.minHeapsize int default=1536 restart ## Stack size (in kilobytes) jvm.stacksize int default=512 restart +## CompressedOOps size in megabytes +jvm.compressedClassSpaceSize int default=32 restart + ## Base value of maximum direct memory size (in megabytes) jvm.baseMaxDirectMemorySize int default=75 restart diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/InstanceInformation.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/InstanceInformation.java index 5c279547e17..d33f9a45c82 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/InstanceInformation.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/InstanceInformation.java @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.controller.api.application.v4.model; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; +import com.yahoo.config.provision.zone.RoutingMethod; import com.yahoo.vespa.hosted.controller.api.identifiers.GitBranch; import com.yahoo.vespa.hosted.controller.api.identifiers.GitCommit; import com.yahoo.vespa.hosted.controller.api.identifiers.GitRepository; @@ -35,23 +36,31 @@ public class InstanceInformation { public String cluster; public boolean tls; public URI url; + public String scope; + public RoutingMethod routingMethod; @JsonCreator public Endpoint(@JsonProperty("cluster") String cluster , @JsonProperty("tls") boolean tls, - @JsonProperty("url") URI url) { + @JsonProperty("url") URI url, + @JsonProperty("scope") String scope, + @JsonProperty("routingMethod") RoutingMethod routingMethod) { this.cluster = cluster; this.tls = tls; this.url = url; + this.scope = scope; + this.routingMethod = routingMethod; } @Override public String toString() { - return "Endpoint {" + - "cluster=" + cluster+ - ", tls='" + tls + '\'' + - ", url='" + url+ '\'' + - '}'; + return "Endpoint{" + + "cluster='" + cluster + '\'' + + ", tls=" + tls + + ", url=" + url + + ", scope='" + scope + '\'' + + ", routingMethod=" + routingMethod + + '}'; } } } 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 9d666c6f7b5..4d09765f78b 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 @@ -28,7 +28,6 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.DeploymentId; import com.yahoo.vespa.hosted.controller.api.identifiers.Hostname; import com.yahoo.vespa.hosted.controller.api.integration.LogEntry; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ConfigServerException; -import com.yahoo.vespa.hosted.controller.api.integration.configserver.Log; import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node; import com.yahoo.vespa.hosted.controller.api.integration.configserver.PrepareResponse; import com.yahoo.vespa.hosted.controller.api.integration.configserver.ServiceConvergence; @@ -38,6 +37,7 @@ import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterCloud; import com.yahoo.vespa.hosted.controller.api.integration.deployment.TesterId; import com.yahoo.vespa.hosted.controller.api.integration.organization.DeploymentFailureMails; +import com.yahoo.vespa.hosted.controller.api.integration.organization.Mail; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.application.Deployment; import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; @@ -76,7 +76,6 @@ import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Nod import static com.yahoo.vespa.hosted.controller.api.integration.configserver.Node.State.reserved; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.aborted; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.deploymentFailed; -import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.endpointCertificateTimeout; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.error; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.installationFailed; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.outOfCapacity; @@ -672,22 +671,36 @@ public class InternalStepRunner implements StepRunner { return; try { - if (run.status() == outOfCapacity && run.id().type().isProduction()) - controller.serviceRegistry().mailer().send(mails.outOfCapacity(run.id(), recipients)); - if (run.status() == deploymentFailed) - controller.serviceRegistry().mailer().send(mails.deploymentFailure(run.id(), recipients)); - if (run.status() == installationFailed) - controller.serviceRegistry().mailer().send(mails.installationFailure(run.id(), recipients)); - if (run.status() == testFailure) - controller.serviceRegistry().mailer().send(mails.testFailure(run.id(), recipients)); - if (run.status() == error) - controller.serviceRegistry().mailer().send(mails.systemError(run.id(), recipients)); + mailOf(run, recipients).ifPresent(controller.serviceRegistry().mailer()::send); } catch (RuntimeException e) { logger.log(INFO, "Exception trying to send mail for " + run.id(), e); } } + private Optional<Mail> mailOf(Run run, List<String> recipients) { + switch (run.status()) { + case running: + case aborted: + case success: + return Optional.empty(); + case outOfCapacity: + return run.id().type().isProduction() ? Optional.of(mails.outOfCapacity(run.id(), recipients)) : Optional.empty(); + case deploymentFailed: + return Optional.of(mails.deploymentFailure(run.id(), recipients)); + case installationFailed: + return Optional.of(mails.installationFailure(run.id(), recipients)); + case testFailure: + return Optional.of(mails.testFailure(run.id(), recipients)); + case error: + case endpointCertificateTimeout: + return Optional.of(mails.systemError(run.id(), recipients)); + default: + logger.log(WARNING, "Don't know what mail to send for run status '" + run.status() + "'"); + return Optional.of(mails.systemError(run.id(), recipients)); + } + } + /** Returns the deployment of the real application in the zone of the given job, if it exists. */ private Optional<Deployment> deployment(ApplicationId id, JobType type) { return Optional.ofNullable(application(id).deployments().get(type.zone(controller.system()))); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java index 32c2d6ec3d1..0d1dca391bd 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java @@ -554,15 +554,16 @@ class JobControllerApiHandlerHelper { private static String nameOf(RunStatus status) { switch (status) { - case running: return "running"; - case aborted: return "aborted"; - case error: return "error"; - case testFailure: return "testFailure"; - case outOfCapacity: return "outOfCapacity"; - case installationFailed: return "installationFailed"; - case deploymentFailed: return "deploymentFailed"; - case success: return "success"; - default: throw new IllegalArgumentException("Unexpected status '" + status + "'"); + case running: return "running"; + case aborted: return "aborted"; + case error: return "error"; + case testFailure: return "testFailure"; + case endpointCertificateTimeout: return "endpointCertificateTimeout"; + case outOfCapacity: return "outOfCapacity"; + case installationFailed: return "installationFailed"; + case deploymentFailed: return "deploymentFailed"; + case success: return "success"; + default: throw new IllegalArgumentException("Unexpected status '" + status + "'"); } } diff --git a/fbench/src/fbench/fbench.cpp b/fbench/src/fbench/fbench.cpp index 91475ce2125..593ae30a0e5 100644 --- a/fbench/src/fbench/fbench.cpp +++ b/fbench/src/fbench/fbench.cpp @@ -86,10 +86,13 @@ FBench::init_crypto_engine(const std::string &ca_certs_file_name, return false; } bool load_failed = false; - vespalib::net::tls::TransportSecurityOptions - tls_opts(maybe_load(ca_certs_file_name, load_failed), - maybe_load(cert_chain_file_name, load_failed), - maybe_load(private_key_file_name, load_failed)); + auto ts_builder = vespalib::net::tls::TransportSecurityOptions::Params(). + ca_certs_pem(maybe_load(ca_certs_file_name, load_failed)). + cert_chain_pem(maybe_load(cert_chain_file_name, load_failed)). + private_key_pem(maybe_load(private_key_file_name, load_failed)). + authorized_peers(vespalib::net::tls::AuthorizedPeers::allow_all_authenticated()). + disable_hostname_validation(true); // TODO configurable or default false! + vespalib::net::tls::TransportSecurityOptions tls_opts(std::move(ts_builder)); if (load_failed) { fprintf(stderr, "failed to load transport security options\n"); return false; diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/ssl/impl/TlsContextBasedProviderTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/ssl/impl/TlsContextBasedProviderTest.java index 88db5c99de9..eb292199ea2 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/ssl/impl/TlsContextBasedProviderTest.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/ssl/impl/TlsContextBasedProviderTest.java @@ -5,6 +5,7 @@ import com.yahoo.security.KeyUtils; import com.yahoo.security.X509CertificateBuilder; import com.yahoo.security.tls.AuthorizationMode; import com.yahoo.security.tls.DefaultTlsContext; +import com.yahoo.security.tls.HostnameVerification; import com.yahoo.security.tls.PeerAuthentication; import com.yahoo.security.tls.TlsContext; import com.yahoo.security.tls.policy.AuthorizedPeers; @@ -51,7 +52,7 @@ public class TlsContextBasedProviderTest { BigInteger.ONE) .build(); return new DefaultTlsContext( - List.of(certificate), keyPair.getPrivate(), List.of(certificate), new AuthorizedPeers(Set.of()), AuthorizationMode.ENFORCE, PeerAuthentication.NEED); + List.of(certificate), keyPair.getPrivate(), List.of(certificate), new AuthorizedPeers(Set.of()), AuthorizationMode.ENFORCE, PeerAuthentication.NEED, HostnameVerification.ENABLED); } private static class SimpleTlsContextBasedProvider extends TlsContextBasedProvider { diff --git a/jrt/src/com/yahoo/jrt/TlsCryptoEngine.java b/jrt/src/com/yahoo/jrt/TlsCryptoEngine.java index 7474220d4e7..a363bf52155 100644 --- a/jrt/src/com/yahoo/jrt/TlsCryptoEngine.java +++ b/jrt/src/com/yahoo/jrt/TlsCryptoEngine.java @@ -21,7 +21,7 @@ public class TlsCryptoEngine implements CryptoEngine { @Override public TlsCryptoSocket createClientCryptoSocket(SocketChannel channel, Spec spec) { - SSLEngine sslEngine = tlsContext.createSslEngine(); + SSLEngine sslEngine = tlsContext.createSslEngine(spec.host(), spec.port()); sslEngine.setUseClientMode(true); return new TlsCryptoSocket(channel, sslEngine); } diff --git a/jrt/tests/com/yahoo/jrt/CryptoUtils.java b/jrt/tests/com/yahoo/jrt/CryptoUtils.java index e7e4eea568d..95ea581cb90 100644 --- a/jrt/tests/com/yahoo/jrt/CryptoUtils.java +++ b/jrt/tests/com/yahoo/jrt/CryptoUtils.java @@ -5,6 +5,7 @@ import com.yahoo.security.KeyUtils; import com.yahoo.security.X509CertificateBuilder; import com.yahoo.security.tls.AuthorizationMode; import com.yahoo.security.tls.DefaultTlsContext; +import com.yahoo.security.tls.HostnameVerification; import com.yahoo.security.tls.PeerAuthentication; import com.yahoo.security.tls.TlsContext; import com.yahoo.security.tls.policy.AuthorizedPeers; @@ -35,21 +36,23 @@ class CryptoUtils { static final KeyPair keyPair = KeyUtils.generateKeypair(EC); static final X509Certificate certificate = X509CertificateBuilder - .fromKeypair(keyPair, new X500Principal("CN=dummy"), EPOCH, Instant.now().plus(1, DAYS), SHA256_WITH_ECDSA, generateRandomSerialNumber()) + .fromKeypair(keyPair, new X500Principal("CN=localhost"), EPOCH, Instant.now().plus(1, DAYS), SHA256_WITH_ECDSA, generateRandomSerialNumber()) .build(); static final AuthorizedPeers authorizedPeers = new AuthorizedPeers( singleton( new PeerPolicy( - "dummy-policy", + "localhost-policy", singleton( - new Role("dummy-role")), + new Role("localhost-role")), singletonList( new RequiredPeerCredential( - Field.CN, new HostGlobPattern("dummy")))))); + Field.CN, new HostGlobPattern("localhost")))))); static TlsContext createTestTlsContext() { - return new DefaultTlsContext(singletonList(certificate), keyPair.getPrivate(), singletonList(certificate), authorizedPeers, AuthorizationMode.ENFORCE, PeerAuthentication.NEED); + return new DefaultTlsContext( + singletonList(certificate), keyPair.getPrivate(), singletonList(certificate), authorizedPeers, + AuthorizationMode.ENFORCE, PeerAuthentication.NEED, HostnameVerification.ENABLED); } } diff --git a/metrics-proxy/pom.xml b/metrics-proxy/pom.xml index f72ad75c6af..355f420c2a4 100644 --- a/metrics-proxy/pom.xml +++ b/metrics-proxy/pom.xml @@ -132,6 +132,10 @@ <groupId>org.apache.httpcomponents</groupId> <artifactId>httpclient</artifactId> </dependency> + <dependency> + <groupId>org.apache.velocity</groupId> + <artifactId>velocity</artifactId> + </dependency> <!-- test scope --> diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/telegraf/Telegraf.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/telegraf/Telegraf.java new file mode 100644 index 00000000000..bf4f0d4c49b --- /dev/null +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/telegraf/Telegraf.java @@ -0,0 +1,78 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package ai.vespa.metricsproxy.telegraf; + +import com.google.inject.Inject; +import com.yahoo.component.AbstractComponent; +import com.yahoo.system.execution.ProcessExecutor; +import org.apache.velocity.VelocityContext; +import org.apache.velocity.app.VelocityEngine; + +import java.io.FileWriter; +import java.io.InputStreamReader; +import java.io.Reader; +import java.io.Writer; + +import static com.yahoo.yolean.Exceptions.uncheck; + +/** + * @author olaa + */ +public class Telegraf extends AbstractComponent { + + private static final String TELEGRAF_CONFIG_PATH = "/etc/telegraf/telegraf.conf"; + private static final String TELEGRAF_CONFIG_TEMPLATE_PATH = "templates/telegraf.conf.vm"; + private final TelegrafRegistry telegrafRegistry; + + @Inject + public Telegraf(TelegrafRegistry telegrafRegistry, TelegrafConfig telegrafConfig) { + this.telegrafRegistry = telegrafRegistry; + telegrafRegistry.addInstance(this); + writeConfig(telegrafConfig, uncheck(() -> new FileWriter(TELEGRAF_CONFIG_PATH))); + restartTelegraf(); + } + + protected static void writeConfig(TelegrafConfig telegrafConfig, Writer writer) { + VelocityContext context = new VelocityContext(); + context.put("intervalSeconds", telegrafConfig.intervalSeconds()); + context.put("cloudwatchPlugins", telegrafConfig.cloudWatch()); + // TODO: Add node cert if hosted + + VelocityEngine velocityEngine = new VelocityEngine(); + velocityEngine.init(); + velocityEngine.evaluate(context, writer, "TelegrafConfigWriter", getTemplateReader()); + uncheck(writer::close); + } + + private void restartTelegraf() { + executeCommand("service telegraf restart"); + } + + private void stopTelegraf() { + executeCommand("service telegraf stop"); + } + + private void executeCommand(String command) { + ProcessExecutor processExecutor = new ProcessExecutor + .Builder(10) + .successExitCodes(0) + .build(); + uncheck(() -> processExecutor.execute(command)) + .orElseThrow(() -> new RuntimeException("Timed out running command: " + command)); + } + + @SuppressWarnings("ConstantConditions") + private static Reader getTemplateReader() { + return new InputStreamReader(Telegraf.class.getClassLoader() + .getResourceAsStream(TELEGRAF_CONFIG_TEMPLATE_PATH) + ); + + } + + @Override + public void deconstruct() { + telegrafRegistry.removeInstance(this); + if (telegrafRegistry.isEmpty()) { + stopTelegraf(); + } + } +} diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/telegraf/TelegrafRegistry.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/telegraf/TelegrafRegistry.java new file mode 100644 index 00000000000..429da5bb933 --- /dev/null +++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/telegraf/TelegrafRegistry.java @@ -0,0 +1,26 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package ai.vespa.metricsproxy.telegraf; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +/** + * @author olaa + */ +public class TelegrafRegistry { + + private static final List<Telegraf> telegrafInstances = Collections.synchronizedList(new ArrayList<>()); + + public void addInstance(Telegraf telegraf) { + telegrafInstances.add(telegraf); + } + + public void removeInstance(Telegraf telegraf) { + telegrafInstances.remove(telegraf); + } + + public boolean isEmpty() { + return telegrafInstances.isEmpty(); + } +} diff --git a/metrics-proxy/src/main/resources/configdefinitions/telegraf.def b/metrics-proxy/src/main/resources/configdefinitions/telegraf.def index f3b5db35d52..6abbd7921b5 100644 --- a/metrics-proxy/src/main/resources/configdefinitions/telegraf.def +++ b/metrics-proxy/src/main/resources/configdefinitions/telegraf.def @@ -5,9 +5,8 @@ package=ai.vespa.metricsproxy.telegraf intervalSeconds int default=60 -# The consumer to get metrics for -vespa.consumer string default="default" - +# The Vespa metrics consumer to get metrics for +cloudWatch[].consumer string cloudWatch[].region string default="us-east-1" cloudWatch[].namespace string diff --git a/metrics-proxy/src/main/resources/templates/telegraf.conf.vm b/metrics-proxy/src/main/resources/templates/telegraf.conf.vm new file mode 100644 index 00000000000..ff04dafe276 --- /dev/null +++ b/metrics-proxy/src/main/resources/templates/telegraf.conf.vm @@ -0,0 +1,39 @@ +# Configuration for telegraf agent +[agent] + interval = "${intervalSeconds}s" + round_interval = true + metric_batch_size = 1000 + metric_buffer_limit = 10000 + collection_jitter = "0s" + flush_interval = "${intervalSeconds}s" + flush_jitter = "0s" + precision = "" + +#foreach( $cloudwatch in $cloudwatchPlugins ) +# Configuration for AWS CloudWatch output. +[[outputs.cloudwatch]] + region = "$cloudwatch.region()" + namespace = "$cloudwatch.namespace()" +#if( $cloudwatch.accessKeyName() != "" ) + access_key = "$cloudwatch.accessKeyName()" + secret_key = "$cloudwatch.secretKeyName()" +#elseif( $cloudwatch.profile() != "" ) + profile = "$cloudwatch.profile()" +#end + tagexclude = ["vespa_consumer"] + [outputs.cloudwatch.tagpass] + vespa_consumer = ["$cloudwatch.consumer()"] + +# Configuration for Vespa input plugin +[[inputs.vespa]] + url = "http://localhost:19092/metrics/v2/values?consumer=$cloudwatch.consumer()" + [inputs.vespa.tags] + vespa_consumer = "$cloudwatch.consumer()" +#* TODO: Add node cert if hosted +#if( $isHosted ) + tls_cert = "${VESPA_CERTIFICATE_PATH}" + tls_key = "${VESPA_KEY_PATH}" + insecure_skip_verify = true +#end +*### +#end
\ No newline at end of file diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/telegraf/TelegrafTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/telegraf/TelegrafTest.java new file mode 100644 index 00000000000..00cbb4bf033 --- /dev/null +++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/telegraf/TelegrafTest.java @@ -0,0 +1,42 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package ai.vespa.metricsproxy.telegraf; + +import ai.vespa.metricsproxy.TestUtil; +import org.junit.Test; + +import java.io.StringWriter; + +import static org.junit.Assert.*; + +/** + * @author olaa + */ +public class TelegrafTest { + + + @Test + public void test_writing_correct_telegraf_plugin_config() { + TelegrafConfig telegrafConfig = new TelegrafConfig.Builder() + .cloudWatch( + new TelegrafConfig.CloudWatch.Builder() + .accessKeyName("accessKey1") + .namespace("namespace1") + .secretKeyName("secretKey1") + .region("us-east-1") + .consumer("consumer1") + ) + .cloudWatch( + new TelegrafConfig.CloudWatch.Builder() + .namespace("namespace2") + .profile("awsprofile") + .region("us-east-2") + .consumer("consumer2") + ) + .intervalSeconds(300) + .build(); + StringWriter stringWriter = new StringWriter(); + Telegraf.writeConfig(telegrafConfig, stringWriter); + String expectedConfig = TestUtil.getFileContents( "telegraf-config-with-two-cloudwatch-plugins.txt"); + assertEquals(expectedConfig, stringWriter.toString()); + } +} diff --git a/metrics-proxy/src/test/resources/telegraf-config-with-two-cloudwatch-plugins.txt b/metrics-proxy/src/test/resources/telegraf-config-with-two-cloudwatch-plugins.txt new file mode 100644 index 00000000000..0dec2775a05 --- /dev/null +++ b/metrics-proxy/src/test/resources/telegraf-config-with-two-cloudwatch-plugins.txt @@ -0,0 +1,41 @@ +# Configuration for telegraf agent +[agent] + interval = "300s" + round_interval = true + metric_batch_size = 1000 + metric_buffer_limit = 10000 + collection_jitter = "0s" + flush_interval = "300s" + flush_jitter = "0s" + precision = "" + +# Configuration for AWS CloudWatch output. +[[outputs.cloudwatch]] + region = "us-east-1" + namespace = "namespace1" + access_key = "accessKey1" + secret_key = "secretKey1" + tagexclude = ["vespa_consumer"] + [outputs.cloudwatch.tagpass] + vespa_consumer = ["consumer1"] + +# Configuration for Vespa input plugin +[[inputs.vespa]] + url = "http://localhost:19092/metrics/v2/values?consumer=consumer1" + [inputs.vespa.tags] + vespa_consumer = "consumer1" +# Configuration for AWS CloudWatch output. +[[outputs.cloudwatch]] + region = "us-east-2" + namespace = "namespace2" + profile = "awsprofile" + tagexclude = ["vespa_consumer"] + [outputs.cloudwatch.tagpass] + vespa_consumer = ["consumer2"] + +# Configuration for Vespa input plugin +[[inputs.vespa]] + url = "http://localhost:19092/metrics/v2/values?consumer=consumer2" + [inputs.vespa.tags] + vespa_consumer = "consumer2" + diff --git a/parent/pom.xml b/parent/pom.xml index b1ca2539ef5..68ee698e7a5 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -735,8 +735,8 @@ <properties> <antlr.version>3.5.2</antlr.version> <antlr4.version>4.5</antlr4.version> - <apache.httpclient.version>4.5.10</apache.httpclient.version> - <apache.httpcore.version>4.4.12</apache.httpcore.version> + <apache.httpclient.version>4.5.11</apache.httpclient.version> + <apache.httpcore.version>4.4.13</apache.httpcore.version> <asm.version>7.0</asm.version> <!-- Athenz dependencies. Make sure these dependencies match those in Vespa's internal repositories --> <athenz.version>1.8.44</athenz.version> diff --git a/security-tools/pom.xml b/security-tools/pom.xml index 38b14ce957f..195e2d06311 100644 --- a/security-tools/pom.xml +++ b/security-tools/pom.xml @@ -57,6 +57,7 @@ <exclude>META-INF/*.SF</exclude> <exclude>META-INF/*.DSA</exclude> <exclude>META-INF/*.RSA</exclude> + <exclude>META-INF/versions/*/module-info.class</exclude> </excludes> </filter> </filters> diff --git a/security-tools/src/main/java/com/yahoo/vespa/security/tool/securityenv/Main.java b/security-tools/src/main/java/com/yahoo/vespa/security/tool/securityenv/Main.java index 367d7b9dd83..c314d17e018 100644 --- a/security-tools/src/main/java/com/yahoo/vespa/security/tool/securityenv/Main.java +++ b/security-tools/src/main/java/com/yahoo/vespa/security/tool/securityenv/Main.java @@ -54,6 +54,9 @@ public class Main { MixedMode mixedMode = TransportSecurityUtils.getInsecureMixedMode(envVars); if (options.isPresent() && mixedMode != MixedMode.PLAINTEXT_CLIENT_MIXED_SERVER) { outputVariables.put(OutputVariable.TLS_ENABLED, "1"); + if (options.get().isHostnameValidationDisabled()) { + outputVariables.put(OutputVariable.DISABLE_HOSTNAME_VALIDATION, "1"); + } options.get().getCaCertificatesFile() .ifPresent(caCertFile -> outputVariables.put(OutputVariable.CA_CERTIFICATE, caCertFile.toString())); options.get().getCertificatesFile() diff --git a/security-tools/src/main/java/com/yahoo/vespa/security/tool/securityenv/OutputVariable.java b/security-tools/src/main/java/com/yahoo/vespa/security/tool/securityenv/OutputVariable.java index dd248d05aac..9a90a145f30 100644 --- a/security-tools/src/main/java/com/yahoo/vespa/security/tool/securityenv/OutputVariable.java +++ b/security-tools/src/main/java/com/yahoo/vespa/security/tool/securityenv/OutputVariable.java @@ -10,7 +10,8 @@ enum OutputVariable { TLS_ENABLED("VESPA_TLS_ENABLED", "Set to '1' if TLS is enabled in Vespa"), CA_CERTIFICATE("VESPA_TLS_CA_CERT", "Path to CA certificates file"), CERTIFICATE("VESPA_TLS_CERT", "Path to certificate file"), - PRIVATE_KEY("VESPA_TLS_PRIVATE_KEY", "Path to private key file"); + PRIVATE_KEY("VESPA_TLS_PRIVATE_KEY", "Path to private key file"), + DISABLE_HOSTNAME_VALIDATION("VESPA_TLS_HOSTNAME_VALIDATION_DISABLED", "Set to '1' if TLS hostname validation is disabled"); private final String variableName; private final String description; diff --git a/security-tools/src/main/sh/vespa-curl-wrapper b/security-tools/src/main/sh/vespa-curl-wrapper index e286e121f64..b4fd9224a8a 100755 --- a/security-tools/src/main/sh/vespa-curl-wrapper +++ b/security-tools/src/main/sh/vespa-curl-wrapper @@ -88,6 +88,11 @@ then CURL_PARAMETERS=("${CURL_PARAMETERS[@]/http:/https:}") fi +if [ -n "${VESPA_TLS_HOSTNAME_VALIDATION_DISABLED}" ] +then + CURL_PARAMETERS=("--insecure" "${CURL_PARAMETERS[@]}") +fi + if [ -n "${VESPA_TLS_CA_CERT}" ] then CURL_PARAMETERS=("--cacert" "${VESPA_TLS_CA_CERT}" "${CURL_PARAMETERS[@]}") diff --git a/security-tools/src/test/java/com/yahoo/vespa/security/tool/securityenv/MainTest.java b/security-tools/src/test/java/com/yahoo/vespa/security/tool/securityenv/MainTest.java index b563ebd14f4..45626820f4d 100644 --- a/security-tools/src/test/java/com/yahoo/vespa/security/tool/securityenv/MainTest.java +++ b/security-tools/src/test/java/com/yahoo/vespa/security/tool/securityenv/MainTest.java @@ -106,6 +106,7 @@ public class MainTest { TransportSecurityOptions options = new TransportSecurityOptions.Builder() .withCertificates(Paths.get("/path/to/certificate"), Paths.get("/path/to/key")) .withCaCertificates(Paths.get("/path/to/cacerts")) + .withHostnameValidationDisabled(true) .build(); Path configFile = tmpFolder.newFile().toPath(); options.toJsonFile(configFile); diff --git a/security-tools/src/test/resources/bash-output.txt b/security-tools/src/test/resources/bash-output.txt index c07c667af47..182dc177d42 100644 --- a/security-tools/src/test/resources/bash-output.txt +++ b/security-tools/src/test/resources/bash-output.txt @@ -2,3 +2,4 @@ VESPA_TLS_ENABLED="1"; export VESPA_TLS_ENABLED; VESPA_TLS_CA_CERT="/path/to/cacerts"; export VESPA_TLS_CA_CERT; VESPA_TLS_CERT="/path/to/certificate"; export VESPA_TLS_CERT; VESPA_TLS_PRIVATE_KEY="/path/to/key"; export VESPA_TLS_PRIVATE_KEY; +VESPA_TLS_HOSTNAME_VALIDATION_DISABLED="1"; export VESPA_TLS_HOSTNAME_VALIDATION_DISABLED; diff --git a/security-tools/src/test/resources/csh-output.txt b/security-tools/src/test/resources/csh-output.txt index 2b6716de92b..2e6cd886c26 100644 --- a/security-tools/src/test/resources/csh-output.txt +++ b/security-tools/src/test/resources/csh-output.txt @@ -2,3 +2,4 @@ setenv VESPA_TLS_ENABLED "1"; setenv VESPA_TLS_CA_CERT "/path/to/cacerts"; setenv VESPA_TLS_CERT "/path/to/certificate"; setenv VESPA_TLS_PRIVATE_KEY "/path/to/key"; +setenv VESPA_TLS_HOSTNAME_VALIDATION_DISABLED "1"; diff --git a/security-tools/src/test/resources/expected-help-output.txt b/security-tools/src/test/resources/expected-help-output.txt index 7d125fe15a2..33ad3b6d232 100644 --- a/security-tools/src/test/resources/expected-help-output.txt +++ b/security-tools/src/test/resources/expected-help-output.txt @@ -9,3 +9,5 @@ The output may include the following variables: - 'VESPA_TLS_CA_CERT': Path to CA certificates file - 'VESPA_TLS_CERT': Path to certificate file - 'VESPA_TLS_PRIVATE_KEY': Path to private key file + - 'VESPA_TLS_HOSTNAME_VALIDATION_DISABLED': Set to '1' if TLS hostname +validation is disabled diff --git a/security-tools/src/test/resources/no-security-output.txt b/security-tools/src/test/resources/no-security-output.txt index 3467f1316b5..257a2747ee2 100644 --- a/security-tools/src/test/resources/no-security-output.txt +++ b/security-tools/src/test/resources/no-security-output.txt @@ -2,3 +2,4 @@ unset VESPA_TLS_ENABLED; unset VESPA_TLS_CA_CERT; unset VESPA_TLS_CERT; unset VESPA_TLS_PRIVATE_KEY; +unset VESPA_TLS_HOSTNAME_VALIDATION_DISABLED; diff --git a/security-utils/src/main/java/com/yahoo/security/SslContextBuilder.java b/security-utils/src/main/java/com/yahoo/security/SslContextBuilder.java index d2b98fd20d9..f3932c84a17 100644 --- a/security-utils/src/main/java/com/yahoo/security/SslContextBuilder.java +++ b/security-utils/src/main/java/com/yahoo/security/SslContextBuilder.java @@ -35,6 +35,7 @@ public class SslContextBuilder { private TrustManagerFactory trustManagerFactory = TrustManagerUtils::createDefaultX509TrustManager; private KeyManagerFactory keyManagerFactory = KeyManagerUtils::createDefaultX509KeyManager; private X509ExtendedKeyManager keyManager; + private X509ExtendedTrustManager trustManager; public SslContextBuilder() {} @@ -121,15 +122,25 @@ public class SslContextBuilder { return this; } + /** + * Note: Callee is responsible for configuring the trust manager. + * Any truststore configured by {@link #withTrustStore(KeyStore)} or the other overloads will be ignored. + */ + public SslContextBuilder withTrustManager(X509ExtendedTrustManager trustManager) { + this.trustManager = trustManager; + return this; + } + public SSLContext build() { try { SSLContext sslContext = SSLContext.getInstance(TlsContext.SSL_CONTEXT_VERSION); - TrustManager[] trustManagers = new TrustManager[] { trustManagerFactory.createTrustManager(trustStoreSupplier.get()) }; + X509ExtendedTrustManager trustManager = this.trustManager != null + ? this.trustManager + : trustManagerFactory.createTrustManager(trustStoreSupplier.get()); X509ExtendedKeyManager keyManager = this.keyManager != null ? this.keyManager : keyManagerFactory.createKeyManager(keyStoreSupplier.get(), keyStorePassword); - KeyManager[] keyManagers = new KeyManager[] {keyManager}; - sslContext.init(keyManagers, trustManagers, null); + sslContext.init(new KeyManager[] {keyManager}, new TrustManager[] {trustManager}, null); return sslContext; } catch (GeneralSecurityException e) { throw new RuntimeException(e); diff --git a/security-utils/src/main/java/com/yahoo/security/tls/ConfigFileBasedTlsContext.java b/security-utils/src/main/java/com/yahoo/security/tls/ConfigFileBasedTlsContext.java index f746480b126..28854c59b2c 100644 --- a/security-utils/src/main/java/com/yahoo/security/tls/ConfigFileBasedTlsContext.java +++ b/security-utils/src/main/java/com/yahoo/security/tls/ConfigFileBasedTlsContext.java @@ -12,11 +12,9 @@ import com.yahoo.security.tls.policy.AuthorizedPeers; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLParameters; -import javax.net.ssl.X509ExtendedTrustManager; import java.io.IOException; import java.io.UncheckedIOException; import java.lang.ref.WeakReference; -import java.nio.file.Files; import java.nio.file.Path; import java.security.KeyStore; import java.time.Duration; @@ -110,12 +108,14 @@ public class ConfigFileBasedTlsContext implements TlsContext { MutableX509TrustManager mutableTrustManager, MutableX509KeyManager mutableKeyManager, PeerAuthentication peerAuthentication) { + + HostnameVerification hostnameVerification = options.isHostnameValidationDisabled() ? HostnameVerification.DISABLED : HostnameVerification.ENABLED; + PeerAuthorizerTrustManager authorizerTrustManager = options.getAuthorizedPeers() + .map(authorizedPeers -> new PeerAuthorizerTrustManager(authorizedPeers, mode, hostnameVerification, mutableTrustManager)) + .orElseGet(() -> new PeerAuthorizerTrustManager(new AuthorizedPeers(com.yahoo.vespa.jdk8compat.Set.of()), AuthorizationMode.DISABLE, hostnameVerification, mutableTrustManager)); SSLContext sslContext = new SslContextBuilder() .withKeyManager(mutableKeyManager) - .withTrustManagerFactory( - ignoredTruststore -> options.getAuthorizedPeers() - .map(authorizedPeers -> (X509ExtendedTrustManager) new PeerAuthorizerTrustManager(authorizedPeers, mode, mutableTrustManager)) - .orElseGet(() -> new PeerAuthorizerTrustManager(new AuthorizedPeers(com.yahoo.vespa.jdk8compat.Set.of()), AuthorizationMode.DISABLE, mutableTrustManager))) + .withTrustManager(authorizerTrustManager) .build(); List<String> acceptedCiphers = options.getAcceptedCiphers(); Set<String> ciphers = acceptedCiphers.isEmpty() ? TlsContext.ALLOWED_CIPHER_SUITES : new HashSet<>(acceptedCiphers); diff --git a/security-utils/src/main/java/com/yahoo/security/tls/DefaultTlsContext.java b/security-utils/src/main/java/com/yahoo/security/tls/DefaultTlsContext.java index c3f10a464a5..def3e49be4d 100644 --- a/security-utils/src/main/java/com/yahoo/security/tls/DefaultTlsContext.java +++ b/security-utils/src/main/java/com/yahoo/security/tls/DefaultTlsContext.java @@ -34,8 +34,9 @@ public class DefaultTlsContext implements TlsContext { List<X509Certificate> caCertificates, AuthorizedPeers authorizedPeers, AuthorizationMode mode, - PeerAuthentication peerAuthentication) { - this(createSslContext(certificates, privateKey, caCertificates, authorizedPeers, mode), peerAuthentication); + PeerAuthentication peerAuthentication, + HostnameVerification hostnameVerification) { + this(createSslContext(certificates, privateKey, caCertificates, authorizedPeers, mode, hostnameVerification), peerAuthentication); } public DefaultTlsContext(SSLContext sslContext, PeerAuthentication peerAuthentication) { @@ -120,7 +121,8 @@ public class DefaultTlsContext implements TlsContext { PrivateKey privateKey, List<X509Certificate> caCertificates, AuthorizedPeers authorizedPeers, - AuthorizationMode mode) { + AuthorizationMode mode, + HostnameVerification hostnameVerification) { SslContextBuilder builder = new SslContextBuilder(); if (!certificates.isEmpty()) { builder.withKeyStore(privateKey, certificates); @@ -129,12 +131,12 @@ public class DefaultTlsContext implements TlsContext { builder.withTrustStore(caCertificates); } if (authorizedPeers != null) { - builder.withTrustManagerFactory(truststore -> new PeerAuthorizerTrustManager(authorizedPeers, mode, truststore)); + builder.withTrustManagerFactory(truststore -> new PeerAuthorizerTrustManager(authorizedPeers, mode, hostnameVerification, truststore)); } else { - builder.withTrustManagerFactory(truststore -> new PeerAuthorizerTrustManager(new AuthorizedPeers(com.yahoo.vespa.jdk8compat.Set.of()), AuthorizationMode.DISABLE, truststore)); + builder.withTrustManagerFactory(truststore -> new PeerAuthorizerTrustManager( + new AuthorizedPeers(com.yahoo.vespa.jdk8compat.Set.of()), AuthorizationMode.DISABLE, hostnameVerification, truststore)); } return builder.build(); } - } diff --git a/security-utils/src/main/java/com/yahoo/security/tls/HostnameVerification.java b/security-utils/src/main/java/com/yahoo/security/tls/HostnameVerification.java new file mode 100644 index 00000000000..a41edc6dc44 --- /dev/null +++ b/security-utils/src/main/java/com/yahoo/security/tls/HostnameVerification.java @@ -0,0 +1,7 @@ +// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.security.tls; + +/** + * @author bjorncs + */ +public enum HostnameVerification { ENABLED, DISABLED } diff --git a/security-utils/src/main/java/com/yahoo/security/tls/authz/PeerAuthorizerTrustManager.java b/security-utils/src/main/java/com/yahoo/security/tls/authz/PeerAuthorizerTrustManager.java index 3ddd0861f39..03358190e8a 100644 --- a/security-utils/src/main/java/com/yahoo/security/tls/authz/PeerAuthorizerTrustManager.java +++ b/security-utils/src/main/java/com/yahoo/security/tls/authz/PeerAuthorizerTrustManager.java @@ -3,6 +3,7 @@ package com.yahoo.security.tls.authz; import com.yahoo.security.X509CertificateUtils; import com.yahoo.security.tls.AuthorizationMode; +import com.yahoo.security.tls.HostnameVerification; import com.yahoo.security.tls.TrustManagerUtils; import com.yahoo.security.tls.policy.AuthorizedPeers; @@ -14,7 +15,6 @@ import java.net.Socket; import java.security.KeyStore; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; -import java.util.Objects; import java.util.Optional; import java.util.logging.Logger; @@ -33,15 +33,23 @@ public class PeerAuthorizerTrustManager extends X509ExtendedTrustManager { private final PeerAuthorizer authorizer; private final X509ExtendedTrustManager defaultTrustManager; private final AuthorizationMode mode; + private final HostnameVerification hostnameVerification; - public PeerAuthorizerTrustManager(AuthorizedPeers authorizedPeers, AuthorizationMode mode, X509ExtendedTrustManager defaultTrustManager) { + public PeerAuthorizerTrustManager(AuthorizedPeers authorizedPeers, + AuthorizationMode mode, + HostnameVerification hostnameVerification, + X509ExtendedTrustManager defaultTrustManager) { this.authorizer = new PeerAuthorizer(authorizedPeers); this.mode = mode; + this.hostnameVerification = hostnameVerification; this.defaultTrustManager = defaultTrustManager; } - public PeerAuthorizerTrustManager(AuthorizedPeers authorizedPeers, AuthorizationMode mode, KeyStore truststore) { - this(authorizedPeers, mode, TrustManagerUtils.createDefaultX509TrustManager(truststore)); + public PeerAuthorizerTrustManager(AuthorizedPeers authorizedPeers, + AuthorizationMode mode, + HostnameVerification hostnameVerification, + KeyStore truststore) { + this(authorizedPeers, mode, hostnameVerification, TrustManagerUtils.createDefaultX509TrustManager(truststore)); } @Override @@ -58,28 +66,26 @@ public class PeerAuthorizerTrustManager extends X509ExtendedTrustManager { @Override public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket) throws CertificateException { - overrideHostnameVerification(socket); defaultTrustManager.checkClientTrusted(chain, authType, socket); authorizePeer(chain[0], authType, true, null); } @Override public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket) throws CertificateException { - overrideHostnameVerification(socket); + overrideHostnameVerificationForClient(socket); defaultTrustManager.checkServerTrusted(chain, authType, socket); authorizePeer(chain[0], authType, false, null); } @Override public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine sslEngine) throws CertificateException { - overrideHostnameVerification(sslEngine); defaultTrustManager.checkClientTrusted(chain, authType, sslEngine); authorizePeer(chain[0], authType, true, sslEngine); } @Override public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine sslEngine) throws CertificateException { - overrideHostnameVerification(sslEngine); + overrideHostnameVerificationForClient(sslEngine); defaultTrustManager.checkServerTrusted(chain, authType, sslEngine); authorizePeer(chain[0], authType, false, sslEngine); } @@ -121,31 +127,44 @@ public class PeerAuthorizerTrustManager extends X509ExtendedTrustManager { certificate.getSubjectX500Principal(), X509CertificateUtils.getSubjectAlternativeNames(certificate), authType, isVerifyingClient); } - private static void overrideHostnameVerification(SSLEngine engine) { + private void overrideHostnameVerificationForClient(SSLEngine engine) { SSLParameters params = engine.getSSLParameters(); - if (overrideHostnameVerification(params)) { + if (overrideHostnameVerificationForClient(params)) { engine.setSSLParameters(params); } } - private static void overrideHostnameVerification(Socket socket) { + private void overrideHostnameVerificationForClient(Socket socket) { if (socket instanceof SSLSocket) { SSLSocket sslSocket = (SSLSocket) socket; SSLParameters params = sslSocket.getSSLParameters(); - if (overrideHostnameVerification(params)) { + if (overrideHostnameVerificationForClient(params)) { sslSocket.setSSLParameters(params); } } } - // Disable the default hostname verification that is performed by underlying trust manager when 'HTTPS' is used as endpoint identification algorithm. - // Some http clients, notably the new http client in Java 11, does not allow user configuration of the endpoint algorithm or custom HostnameVerifier. - private static boolean overrideHostnameVerification(SSLParameters params) { - if (Objects.equals("HTTPS", params.getEndpointIdentificationAlgorithm())) { - params.setEndpointIdentificationAlgorithm(""); - return true; + // Overrides the endpoint identification algorithm specified in the ssl parameters of the ssl engine/socket. + // The underlying trust manager will perform hostname verification if endpoint identification algorithm is set to 'HTTPS'. + // Returns true if the parameter instance was modified + private boolean overrideHostnameVerificationForClient(SSLParameters params) { + String configuredAlgorithm = params.getEndpointIdentificationAlgorithm(); + switch (hostnameVerification) { + case ENABLED: + if (!"HTTPS".equals(configuredAlgorithm)) { + params.setEndpointIdentificationAlgorithm("HTTPS"); + return true; + } + return false; + case DISABLED: + if (configuredAlgorithm != null && !configuredAlgorithm.isEmpty()) { + params.setEndpointIdentificationAlgorithm(""); // disable any configured endpoint identification algorithm + return true; + } + return false; + default: + throw new IllegalStateException("Unknown host verification type: " + hostnameVerification); } - return false; } } diff --git a/security-utils/src/test/java/com/yahoo/security/tls/DefaultTlsContextTest.java b/security-utils/src/test/java/com/yahoo/security/tls/DefaultTlsContextTest.java index 727a64ae934..00928187f55 100644 --- a/security-utils/src/test/java/com/yahoo/security/tls/DefaultTlsContextTest.java +++ b/security-utils/src/test/java/com/yahoo/security/tls/DefaultTlsContextTest.java @@ -46,7 +46,9 @@ public class DefaultTlsContextTest { singletonList(new RequiredPeerCredential(RequiredPeerCredential.Field.CN, new HostGlobPattern("dummy")))))); DefaultTlsContext tlsContext = - new DefaultTlsContext(singletonList(certificate), keyPair.getPrivate(), singletonList(certificate), authorizedPeers, AuthorizationMode.ENFORCE, PeerAuthentication.NEED); + new DefaultTlsContext( + singletonList(certificate), keyPair.getPrivate(), singletonList(certificate), authorizedPeers, + AuthorizationMode.ENFORCE, PeerAuthentication.NEED, HostnameVerification.ENABLED); SSLEngine sslEngine = tlsContext.createSslEngine(); assertThat(sslEngine).isNotNull(); diff --git a/vbench/src/vbench/vbench/vbench.cpp b/vbench/src/vbench/vbench/vbench.cpp index 4f6efadfbdd..58854af705e 100644 --- a/vbench/src/vbench/vbench/vbench.cpp +++ b/vbench/src/vbench/vbench/vbench.cpp @@ -29,11 +29,13 @@ CryptoEngine::SP setup_crypto(const vespalib::slime::Inspector &tls) { if (!tls.valid()) { return std::make_shared<vespalib::NullCryptoEngine>(); } - vespalib::net::tls::TransportSecurityOptions - tls_opts(maybe_load(tls["ca-certificates"]), - maybe_load(tls["certificates"]), - maybe_load(tls["private-key"])); - return std::make_shared<vespalib::TlsCryptoEngine>(tls_opts); + auto ts_builder = vespalib::net::tls::TransportSecurityOptions::Params(). + ca_certs_pem(maybe_load(tls["ca-certificates"])). + cert_chain_pem(maybe_load(tls["certificates"])). + private_key_pem(maybe_load(tls["private-key"])). + authorized_peers(vespalib::net::tls::AuthorizedPeers::allow_all_authenticated()). + disable_hostname_validation(true); // TODO configurable or default false! + return std::make_shared<vespalib::TlsCryptoEngine>(vespalib::net::tls::TransportSecurityOptions(std::move(ts_builder))); } } // namespace vbench::<unnamed> diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/runner/Runner.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/runner/Runner.java index 0e202d1f348..59953fbe002 100644 --- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/runner/Runner.java +++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/runner/Runner.java @@ -70,7 +70,6 @@ public class Runner { Optional.ofNullable(commandLineArgs.getFile()), commandLineArgs.getAddRootElementToXml()); - int intervalOfLogging = commandLineArgs.getVerbose() ? commandLineArgs.getWhenVerboseEnabledPrintMessageForEveryXDocuments() : Integer.MAX_VALUE; @@ -86,13 +85,15 @@ public class Runner { if (commandLineArgs.getVerbose()) { System.err.println(feedClient.getStatsAsJson()); - double fileSizeMb = ((double) new File(commandLineArgs.getFile()).length()) / 1024.0 / 1024.0; double transferTimeSec = ((double) sendTotalTimeMs) / 1000.0; - System.err.println("Sent " + fileSizeMb + " MB in " + transferTimeSec + " seconds."); - System.err.println("Speed: " + ((fileSizeMb / transferTimeSec) * 8.0) + " Mbits/sec, + HTTP overhead " + - "(not taking compression into account)"); if (transferTimeSec > 0) { - System.err.printf("Docs/sec %.3f%n\n", numSent.get() / transferTimeSec); + System.err.printf("Docs/sec %.3f%n", numSent.get() / transferTimeSec); + } + if (commandLineArgs.getFile() != null) { + double fileSizeMb = ((double) new File(commandLineArgs.getFile()).length()) / 1024.0 / 1024.0; + System.err.println("Sent " + fileSizeMb + " MB in " + transferTimeSec + " seconds."); + System.err.println("Speed: " + ((fileSizeMb / transferTimeSec) * 8.0) + " Mbits/sec, + HTTP overhead " + + "(not taking compression into account)"); } } callback.printProgress(); diff --git a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunnerHandler.java b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunnerHandler.java index ed079442440..90f7a76b356 100644 --- a/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunnerHandler.java +++ b/vespa-testrunner-components/src/main/java/com/yahoo/vespa/hosted/testrunner/TestRunnerHandler.java @@ -61,8 +61,7 @@ public class TestRunnerHandler extends LoggingRequestHandler { private HttpResponse handleGET(HttpRequest request) { String path = request.getUri().getPath(); - // TODO: Migrate to /tester/v1/log when /tester/v1/log2 is not in use anymore (and remove /tester/v1/log2) - if (path.equals("/tester/v1/log") || path.equals("/tester/v1/log2")) { + if (path.equals("/tester/v1/log")) { return new SlimeJsonResponse(logToSlime(testRunner.getLog(request.hasProperty("after") ? Long.parseLong(request.getProperty("after")) : -1))); @@ -91,7 +90,7 @@ public class TestRunnerHandler extends LoggingRequestHandler { path = path.substring(0, path.length() - 1); int lastSlash = path.lastIndexOf("/"); if (lastSlash < 0) return path; - return path.substring(lastSlash + 1, path.length()); + return path.substring(lastSlash + 1); } static Slime logToSlime(Collection<LogRecord> log) { diff --git a/vespaclient/src/perl/lib/Yahoo/Vespa/Http.pm b/vespaclient/src/perl/lib/Yahoo/Vespa/Http.pm index 2dbf475f2a7..d907e89fa54 100644 --- a/vespaclient/src/perl/lib/Yahoo/Vespa/Http.pm +++ b/vespaclient/src/perl/lib/Yahoo/Vespa/Http.pm @@ -100,7 +100,10 @@ sub initialize { # () my $tls_enabled = $ENV{'VESPA_TLS_ENABLED'}; if (defined $tls_enabled and $tls_enabled eq '1') { $BROWSER->ssl_opts( SSL_version => 'TLSv12'); - $BROWSER->ssl_opts( verify_hostname => 0); + my $hostname_verification_disabled = $ENV{'VESPA_TLS_HOSTNAME_VALIDATION_DISABLED'}; + if (defined $hostname_verification_disabled and $hostname_verification_disabled eq '1') { + $BROWSER->ssl_opts( verify_hostname => 0); + } $BROWSER->ssl_opts( SSL_cipher_list => 'ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-CHACHA20-POLY1305:TLS13-AES-128-GCM-SHA256:TLS13-AES-256-GCM-SHA384:TLS13-CHACHA20-POLY1305-SHA256' ); } if (defined $ENV{'VESPA_TLS_CA_CERT'}) { diff --git a/vespalib/src/tests/net/tls/openssl_impl/openssl_impl_test.cpp b/vespalib/src/tests/net/tls/openssl_impl/openssl_impl_test.cpp index 78838ce2cd2..54c8c19fc64 100644 --- a/vespalib/src/tests/net/tls/openssl_impl/openssl_impl_test.cpp +++ b/vespalib/src/tests/net/tls/openssl_impl/openssl_impl_test.cpp @@ -57,11 +57,23 @@ void print_decode_result(const char* mode, const DecodeResult& res) { decode_state_to_str(res.state)); } +TransportSecurityOptions ts_from_pems(vespalib::stringref ca_certs_pem, + vespalib::stringref cert_chain_pem, + vespalib::stringref private_key_pem) +{ + auto ts_builder = TransportSecurityOptions::Params(). + ca_certs_pem(ca_certs_pem). + cert_chain_pem(cert_chain_pem). + private_key_pem(private_key_pem). + authorized_peers(AuthorizedPeers::allow_all_authenticated()); + return TransportSecurityOptions(std::move(ts_builder)); +} + struct Fixture { TransportSecurityOptions tls_opts; std::shared_ptr<TlsContext> tls_ctx; - std::unique_ptr<CryptoCodec> client; - std::unique_ptr<CryptoCodec> server; + std::unique_ptr<OpenSslCryptoCodecImpl> client; + std::unique_ptr<OpenSslCryptoCodecImpl> server; SmartBuffer client_to_server; SmartBuffer server_to_client; @@ -77,16 +89,21 @@ struct Fixture { static TransportSecurityOptions create_options_without_own_peer_cert() { auto source_opts = vespalib::test::make_tls_options_for_testing(); - return TransportSecurityOptions(source_opts.ca_certs_pem(), "", ""); + return ts_from_pems(source_opts.ca_certs_pem(), "", ""); } - static std::unique_ptr<CryptoCodec> create_openssl_codec( - const TransportSecurityOptions& opts, CryptoCodec::Mode mode) { + static std::unique_ptr<OpenSslCryptoCodecImpl> create_openssl_codec( + const TransportSecurityOptions& opts, CryptoCodec::Mode mode, const SocketSpec& peer_spec) { auto ctx = TlsContext::create_default_context(opts, AuthorizationMode::Enforce); - return create_openssl_codec(ctx, mode); + return create_openssl_codec(ctx, mode, peer_spec); + } + + static std::unique_ptr<OpenSslCryptoCodecImpl> create_openssl_codec( + const TransportSecurityOptions& opts, CryptoCodec::Mode mode) { + return create_openssl_codec(opts, mode, SocketSpec::invalid); } - static std::unique_ptr<CryptoCodec> create_openssl_codec( + static std::unique_ptr<OpenSslCryptoCodecImpl> create_openssl_codec( const TransportSecurityOptions& opts, std::shared_ptr<CertificateVerificationCallback> cert_verify_callback, CryptoCodec::Mode mode) { @@ -94,21 +111,30 @@ struct Fixture { return create_openssl_codec(ctx, mode); } - static std::unique_ptr<CryptoCodec> create_openssl_codec( - const std::shared_ptr<TlsContext>& ctx, CryptoCodec::Mode mode) { + static std::unique_ptr<OpenSslCryptoCodecImpl> create_openssl_codec( + const std::shared_ptr<TlsContext>& ctx, CryptoCodec::Mode mode, const SocketSpec& peer_spec) { auto ctx_impl = std::dynamic_pointer_cast<impl::OpenSslTlsContextImpl>(ctx); - return std::make_unique<impl::OpenSslCryptoCodecImpl>(std::move(ctx_impl), SocketAddress(), mode); + if (mode == CryptoCodec::Mode::Client) { + return OpenSslCryptoCodecImpl::make_client_codec(std::move(ctx_impl), peer_spec, SocketAddress()); + } else { + return OpenSslCryptoCodecImpl::make_server_codec(std::move(ctx_impl), SocketAddress()); + } } - EncodeResult do_encode(CryptoCodec& codec, Output& buffer, vespalib::stringref plaintext) { + static std::unique_ptr<OpenSslCryptoCodecImpl> create_openssl_codec( + const std::shared_ptr<TlsContext>& ctx, CryptoCodec::Mode mode) { + return create_openssl_codec(ctx, mode, SocketSpec::invalid); + } + + static EncodeResult do_encode(CryptoCodec& codec, Output& buffer, vespalib::stringref plaintext) { auto out = buffer.reserve(codec.min_encode_buffer_size()); auto enc_res = codec.encode(plaintext.data(), plaintext.size(), out.data, out.size); buffer.commit(enc_res.bytes_produced); return enc_res; } - DecodeResult do_decode(CryptoCodec& codec, Input& buffer, vespalib::string& out, - size_t max_bytes_produced, size_t max_bytes_consumed) { + static DecodeResult do_decode(CryptoCodec& codec, Input& buffer, vespalib::string& out, + size_t max_bytes_produced, size_t max_bytes_consumed) { auto in = buffer.obtain(); out.resize(max_bytes_produced); auto to_consume = std::min(in.size, max_bytes_consumed); @@ -382,13 +408,13 @@ l9pLv1vrujrPEC78cyIQe2x55wf3pRoaDg== -----END EC PRIVATE KEY-----)"; TEST_F("client with certificate signed by untrusted CA is rejected by server", Fixture) { - TransportSecurityOptions client_opts(unknown_ca_pem, untrusted_host_cert_pem, untrusted_host_key_pem); + auto client_opts = ts_from_pems(unknown_ca_pem, untrusted_host_cert_pem, untrusted_host_key_pem); f.client = f.create_openssl_codec(client_opts, CryptoCodec::Mode::Client); EXPECT_FALSE(f.handshake()); } TEST_F("server with certificate signed by untrusted CA is rejected by client", Fixture) { - TransportSecurityOptions server_opts(unknown_ca_pem, untrusted_host_cert_pem, untrusted_host_key_pem); + auto server_opts = ts_from_pems(unknown_ca_pem, untrusted_host_cert_pem, untrusted_host_key_pem); f.server = f.create_openssl_codec(server_opts, CryptoCodec::Mode::Server); EXPECT_FALSE(f.handshake()); } @@ -396,8 +422,8 @@ TEST_F("server with certificate signed by untrusted CA is rejected by client", F TEST_F("Can specify multiple trusted CA certs in transport options", Fixture) { auto& base_opts = f.tls_opts; auto multi_ca_pem = base_opts.ca_certs_pem() + "\n" + unknown_ca_pem; - TransportSecurityOptions multi_ca_using_ca_1(multi_ca_pem, untrusted_host_cert_pem, untrusted_host_key_pem); - TransportSecurityOptions multi_ca_using_ca_2(multi_ca_pem, base_opts.cert_chain_pem(), base_opts.private_key_pem()); + auto multi_ca_using_ca_1 = ts_from_pems(multi_ca_pem, untrusted_host_cert_pem, untrusted_host_key_pem); + auto multi_ca_using_ca_2 = ts_from_pems(multi_ca_pem, base_opts.cert_chain_pem(), base_opts.private_key_pem()); // Let client be signed by CA 1, server by CA 2. Both have the two CAs in their trust store // so this should allow for a successful handshake. f.client = f.create_openssl_codec(multi_ca_using_ca_1, CryptoCodec::Mode::Client); @@ -446,7 +472,7 @@ struct CertFixture : Fixture { return {std::move(cert), std::move(key)}; } - static std::unique_ptr<CryptoCodec> create_openssl_codec_with_authz_mode( + static std::unique_ptr<OpenSslCryptoCodecImpl> create_openssl_codec_with_authz_mode( const TransportSecurityOptions& opts, std::shared_ptr<CertificateVerificationCallback> cert_verify_callback, CryptoCodec::Mode codec_mode, @@ -455,33 +481,52 @@ struct CertFixture : Fixture { return create_openssl_codec(ctx, codec_mode); } + TransportSecurityOptions::Params ts_builder_from(const CertKeyWrapper& ck) const { + return TransportSecurityOptions::Params(). + ca_certs_pem(root_ca.cert->to_pem()). + cert_chain_pem(ck.cert->to_pem()). + private_key_pem(ck.key->private_to_pem()); + } + void reset_client_with_cert_opts(const CertKeyWrapper& ck, AuthorizedPeers authorized) { - TransportSecurityOptions client_opts(root_ca.cert->to_pem(), ck.cert->to_pem(), - ck.key->private_to_pem(), std::move(authorized)); - client = create_openssl_codec(client_opts, CryptoCodec::Mode::Client); + auto ts_params = ts_builder_from(ck).authorized_peers(std::move(authorized)); + client = create_openssl_codec(TransportSecurityOptions(std::move(ts_params)), CryptoCodec::Mode::Client); } void reset_client_with_cert_opts(const CertKeyWrapper& ck, std::shared_ptr<CertificateVerificationCallback> cert_cb) { - TransportSecurityOptions client_opts(root_ca.cert->to_pem(), ck.cert->to_pem(), ck.key->private_to_pem()); - client = create_openssl_codec(client_opts, std::move(cert_cb), CryptoCodec::Mode::Client); + auto ts_params = ts_builder_from(ck).authorized_peers(AuthorizedPeers::allow_all_authenticated()); + client = create_openssl_codec(TransportSecurityOptions(std::move(ts_params)), + std::move(cert_cb), CryptoCodec::Mode::Client); } void reset_server_with_cert_opts(const CertKeyWrapper& ck, AuthorizedPeers authorized) { - TransportSecurityOptions server_opts(root_ca.cert->to_pem(), ck.cert->to_pem(), - ck.key->private_to_pem(), std::move(authorized)); - server = create_openssl_codec(server_opts, CryptoCodec::Mode::Server); + auto ts_params = ts_builder_from(ck).authorized_peers(std::move(authorized)); + server = create_openssl_codec(TransportSecurityOptions(std::move(ts_params)), CryptoCodec::Mode::Server); } void reset_server_with_cert_opts(const CertKeyWrapper& ck, std::shared_ptr<CertificateVerificationCallback> cert_cb) { - TransportSecurityOptions server_opts(root_ca.cert->to_pem(), ck.cert->to_pem(), ck.key->private_to_pem()); - server = create_openssl_codec(server_opts, std::move(cert_cb), CryptoCodec::Mode::Server); + auto ts_params = ts_builder_from(ck).authorized_peers(AuthorizedPeers::allow_all_authenticated()); + server = create_openssl_codec(TransportSecurityOptions(std::move(ts_params)), + std::move(cert_cb), CryptoCodec::Mode::Server); } void reset_server_with_cert_opts(const CertKeyWrapper& ck, std::shared_ptr<CertificateVerificationCallback> cert_cb, AuthorizationMode authz_mode) { - TransportSecurityOptions server_opts(root_ca.cert->to_pem(), ck.cert->to_pem(), ck.key->private_to_pem()); - server = create_openssl_codec_with_authz_mode(server_opts, std::move(cert_cb), CryptoCodec::Mode::Server, authz_mode); + auto ts_params = ts_builder_from(ck).authorized_peers(AuthorizedPeers::allow_all_authenticated()); + server = create_openssl_codec_with_authz_mode(TransportSecurityOptions(std::move(ts_params)), + std::move(cert_cb), CryptoCodec::Mode::Server, authz_mode); + } + + void reset_client_with_peer_spec(const CertKeyWrapper& ck, + const SocketSpec& peer_spec, + bool disable_hostname_validation = false) + { + auto ts_params = ts_builder_from(ck). + authorized_peers(AuthorizedPeers::allow_all_authenticated()). + disable_hostname_validation(disable_hostname_validation); + client = create_openssl_codec(TransportSecurityOptions(std::move(ts_params)), + CryptoCodec::Mode::Client, peer_spec); } }; @@ -537,7 +582,7 @@ TEST_F("Exception during verification callback processing breaks handshake", Cer EXPECT_FALSE(f.handshake()); } -TEST_F("certificate verification callback observes CN and DNS SANs", CertFixture) { +TEST_F("Certificate verification callback observes CN and DNS SANs", CertFixture) { auto ck = f.create_ca_issued_peer_cert( {{"rockets.wile.example.com"}}, {{"DNS:crash.wile.example.com"}, {"DNS:burn.wile.example.com"}}); @@ -556,7 +601,7 @@ TEST_F("certificate verification callback observes CN and DNS SANs", CertFixture EXPECT_EQUAL("burn.wile.example.com", creds.dns_sans[1]); } -TEST_F("last occurring CN is given to verification callback if multiple CNs are present", CertFixture) { +TEST_F("Last occurring CN is given to verification callback if multiple CNs are present", CertFixture) { auto ck = f.create_ca_issued_peer_cert( {{"foo.wile.example.com"}, {"bar.wile.example.com"}, {"baz.wile.example.com"}}, {}); @@ -646,6 +691,51 @@ TEST_F("Disabled insecure authorization mode ignores verification result", CertF EXPECT_TRUE(f.handshake()); } +void reset_peers_with_client_peer_spec(CertFixture& f, + const SocketSpec& peer_spec, + bool disable_hostname_validation = false) +{ + auto client_ck = f.create_ca_issued_peer_cert({"hello.world.example.com"}, {}); + f.reset_client_with_peer_spec(client_ck, peer_spec, disable_hostname_validation); + // Since hostname validation is enabled by default, providing a peer spec also + // means that we must have a valid server name to present back (or the handshake fails). + auto server_ck = f.create_ca_issued_peer_cert({}, {{"DNS:*.example.com"}}); + f.reset_server_with_cert_opts(server_ck, AuthorizedPeers::allow_all_authenticated()); +} + +TEST_F("Client does not send SNI extension if hostname not provided in spec", CertFixture) { + reset_peers_with_client_peer_spec(f, SocketSpec::invalid); + + ASSERT_TRUE(f.handshake()); + auto maybe_sni = f.server->client_provided_sni_extension(); + EXPECT_FALSE(maybe_sni.has_value()); +} + +TEST_F("Client sends SNI extension with hostname provided in spec", CertFixture) { + reset_peers_with_client_peer_spec(f, SocketSpec::from_host_port("sni-test.example.com", 12345)); + + ASSERT_TRUE(f.handshake()); + auto maybe_sni = f.server->client_provided_sni_extension(); + ASSERT_TRUE(maybe_sni.has_value()); + EXPECT_EQUAL("sni-test.example.com", *maybe_sni); +} + +TEST_F("Client hostname validation passes handshake if server hostname matches certificate", CertFixture) { + reset_peers_with_client_peer_spec(f, SocketSpec::from_host_port("server-must-be-under.example.com", 12345), false); + EXPECT_TRUE(f.handshake()); +} + +TEST_F("Client hostname validation fails handshake if server hostname mismatches certificate", CertFixture) { + // Wildcards only apply to a single level, so this should fail as the server only has a cert for *.example.com + reset_peers_with_client_peer_spec(f, SocketSpec::from_host_port("nested.name.example.com", 12345), false); + EXPECT_FALSE(f.handshake()); +} + +TEST_F("Mismatching server cert vs hostname does not fail if hostname validation is disabled", CertFixture) { + reset_peers_with_client_peer_spec(f, SocketSpec::from_host_port("a.very.nested.name.example.com", 12345), true); + EXPECT_TRUE(f.handshake()); +} + TEST_F("Failure statistics are incremented on authorization failures", CertFixture) { reset_peers_with_server_authz_mode(f, AuthorizationMode::Enforce); auto server_before = ConnectionStatistics::get(true).snapshot(); diff --git a/vespalib/src/tests/net/tls/transport_options/transport_options_reading_test.cpp b/vespalib/src/tests/net/tls/transport_options/transport_options_reading_test.cpp index a54e2f29aa1..00459a4e69c 100644 --- a/vespalib/src/tests/net/tls/transport_options/transport_options_reading_test.cpp +++ b/vespalib/src/tests/net/tls/transport_options/transport_options_reading_test.cpp @@ -155,6 +155,47 @@ TEST("accepted cipher list is populated if specified") { EXPECT_EQUAL("bar", ciphers[1]); } +// FIXME this is temporary until we know enabling it by default won't break the world! +TEST("hostname validation is DISABLED by default when creating options from config file") { + const char* json = R"({"files":{"private-key":"dummy_privkey.txt", + "certificates":"dummy_certs.txt", + "ca-certificates":"dummy_ca_certs.txt"}})"; + EXPECT_TRUE(read_options_from_json_string(json)->disable_hostname_validation()); +} + +TEST("TransportSecurityOptions builder does not disable hostname validation by default") { + auto ts_builder = vespalib::net::tls::TransportSecurityOptions::Params(). + ca_certs_pem("foo"). + cert_chain_pem("bar"). + private_key_pem("fantonald"); + TransportSecurityOptions ts_opts(std::move(ts_builder)); + EXPECT_FALSE(ts_opts.disable_hostname_validation()); +} + +TEST("hostname validation can be explicitly disabled") { + const char* json = R"({"files":{"private-key":"dummy_privkey.txt", + "certificates":"dummy_certs.txt", + "ca-certificates":"dummy_ca_certs.txt"}, + "disable-hostname-validation": true})"; + EXPECT_TRUE(read_options_from_json_string(json)->disable_hostname_validation()); +} + +TEST("hostname validation can be explicitly enabled") { + const char* json = R"({"files":{"private-key":"dummy_privkey.txt", + "certificates":"dummy_certs.txt", + "ca-certificates":"dummy_ca_certs.txt"}, + "disable-hostname-validation": false})"; + EXPECT_FALSE(read_options_from_json_string(json)->disable_hostname_validation()); +} + +TEST("unknown fields are ignored at parse-time") { + const char* json = R"({"files":{"private-key":"dummy_privkey.txt", + "certificates":"dummy_certs.txt", + "ca-certificates":"dummy_ca_certs.txt"}, + "flipper-the-dolphin": "*weird dolphin noises*"})"; + EXPECT_TRUE(read_options_from_json_string(json).get() != nullptr); // And no exception thrown. +} + // TODO test parsing of multiple policies TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/vespalib/src/vespa/vespalib/net/socket_spec.cpp b/vespalib/src/vespa/vespalib/net/socket_spec.cpp index 06682086670..d1dd81a454a 100644 --- a/vespalib/src/vespa/vespalib/net/socket_spec.cpp +++ b/vespalib/src/vespa/vespalib/net/socket_spec.cpp @@ -41,7 +41,7 @@ SocketSpec::address(bool server) const return SocketAddress(); } -SocketSpec SocketSpec::invalid; +const SocketSpec SocketSpec::invalid; SocketSpec::SocketSpec(const vespalib::string &spec) : SocketSpec() diff --git a/vespalib/src/vespa/vespalib/net/socket_spec.h b/vespalib/src/vespa/vespalib/net/socket_spec.h index 01af382d638..4e3dddf6814 100644 --- a/vespalib/src/vespa/vespalib/net/socket_spec.h +++ b/vespalib/src/vespa/vespalib/net/socket_spec.h @@ -24,7 +24,7 @@ private: : _type(type), _node(node), _port(port) {} SocketAddress address(bool server) const; public: - static SocketSpec invalid; + static const SocketSpec invalid; explicit SocketSpec(const vespalib::string &spec); vespalib::string spec() const; SocketSpec replace_host(const vespalib::string &new_host) const; diff --git a/vespalib/src/vespa/vespalib/net/tls/crypto_codec.cpp b/vespalib/src/vespa/vespalib/net/tls/crypto_codec.cpp index c54990b3782..d3ac975d90a 100644 --- a/vespalib/src/vespa/vespalib/net/tls/crypto_codec.cpp +++ b/vespalib/src/vespa/vespalib/net/tls/crypto_codec.cpp @@ -6,12 +6,23 @@ namespace vespalib::net::tls { -std::unique_ptr<CryptoCodec> CryptoCodec::create_default_codec( - std::shared_ptr<TlsContext> ctx, const SocketAddress& peer_address, Mode mode) +std::unique_ptr<CryptoCodec> +CryptoCodec::create_default_client_codec(std::shared_ptr<TlsContext> ctx, + const SocketSpec& peer_spec, + const SocketAddress& peer_address) { auto ctx_impl = std::dynamic_pointer_cast<impl::OpenSslTlsContextImpl>(ctx); // only takes by const ref assert(ctx_impl); - return std::make_unique<impl::OpenSslCryptoCodecImpl>(std::move(ctx_impl), peer_address, mode); + return impl::OpenSslCryptoCodecImpl::make_client_codec(std::move(ctx_impl), peer_spec, peer_address); +} + +std::unique_ptr<CryptoCodec> +CryptoCodec::create_default_server_codec(std::shared_ptr<TlsContext> ctx, + const SocketAddress& peer_address) +{ + auto ctx_impl = std::dynamic_pointer_cast<impl::OpenSslTlsContextImpl>(ctx); // only takes by const ref + assert(ctx_impl); + return impl::OpenSslCryptoCodecImpl::make_server_codec(std::move(ctx_impl), peer_address); } } diff --git a/vespalib/src/vespa/vespalib/net/tls/crypto_codec.h b/vespalib/src/vespa/vespalib/net/tls/crypto_codec.h index 5d9684461d7..787485b47be 100644 --- a/vespalib/src/vespa/vespalib/net/tls/crypto_codec.h +++ b/vespalib/src/vespa/vespalib/net/tls/crypto_codec.h @@ -4,6 +4,8 @@ #include <vespa/vespalib/net/socket_address.h> #include <memory> +namespace vespalib { class SocketSpec; } + namespace vespalib::net::tls { struct HandshakeResult { @@ -179,9 +181,13 @@ public: * * Throws CryptoException if resources cannot be allocated for the codec. */ - static std::unique_ptr<CryptoCodec> create_default_codec(std::shared_ptr<TlsContext> ctx, - const SocketAddress& peer_address, - Mode mode); + static std::unique_ptr<CryptoCodec> + create_default_client_codec(std::shared_ptr<TlsContext> ctx, + const SocketSpec& peer_spec, + const SocketAddress& peer_address); + static std::unique_ptr<CryptoCodec> + create_default_server_codec(std::shared_ptr<TlsContext> ctx, + const SocketAddress& peer_address); }; } diff --git a/vespalib/src/vespa/vespalib/net/tls/impl/openssl_crypto_codec_impl.cpp b/vespalib/src/vespa/vespalib/net/tls/impl/openssl_crypto_codec_impl.cpp index 5315754d53a..6a79caa8264 100644 --- a/vespalib/src/vespa/vespalib/net/tls/impl/openssl_crypto_codec_impl.cpp +++ b/vespalib/src/vespa/vespalib/net/tls/impl/openssl_crypto_codec_impl.cpp @@ -172,9 +172,11 @@ void log_ssl_error(const char* source, const SocketAddress& peer_address, int ss } // anon ns OpenSslCryptoCodecImpl::OpenSslCryptoCodecImpl(std::shared_ptr<OpenSslTlsContextImpl> ctx, + const SocketSpec& peer_spec, const SocketAddress& peer_address, Mode mode) : _ctx(std::move(ctx)), + _peer_spec(peer_spec), _peer_address(peer_address), _ssl(::SSL_new(_ctx->native_context())), _mode(mode), @@ -219,6 +221,8 @@ OpenSslCryptoCodecImpl::OpenSslCryptoCodecImpl(std::shared_ptr<OpenSslTlsContext _output_bio = tmp_output_bio.release(); if (_mode == Mode::Client) { ::SSL_set_connect_state(_ssl.get()); + enable_hostname_validation_if_requested(); + set_server_name_indication_extension(); } else { ::SSL_set_accept_state(_ssl.get()); } @@ -230,6 +234,59 @@ OpenSslCryptoCodecImpl::OpenSslCryptoCodecImpl(std::shared_ptr<OpenSslTlsContext OpenSslCryptoCodecImpl::~OpenSslCryptoCodecImpl() = default; +std::unique_ptr<OpenSslCryptoCodecImpl> +OpenSslCryptoCodecImpl::make_client_codec(std::shared_ptr<OpenSslTlsContextImpl> ctx, + const SocketSpec& peer_spec, + const SocketAddress& peer_address) +{ + // Naked new due to private ctor + return std::unique_ptr<OpenSslCryptoCodecImpl>( + new OpenSslCryptoCodecImpl(std::move(ctx), peer_spec, peer_address, Mode::Client)); +} +std::unique_ptr<OpenSslCryptoCodecImpl> +OpenSslCryptoCodecImpl::make_server_codec(std::shared_ptr<OpenSslTlsContextImpl> ctx, + const SocketAddress& peer_address) +{ + // Naked new due to private ctor + return std::unique_ptr<OpenSslCryptoCodecImpl>( + new OpenSslCryptoCodecImpl(std::move(ctx), SocketSpec::invalid, peer_address, Mode::Server)); +} + +void OpenSslCryptoCodecImpl::enable_hostname_validation_if_requested() { + if (_peer_spec.valid() && !_ctx->transport_security_options().disable_hostname_validation()) { + auto* verify_param = SSL_get0_param(_ssl.get()); // Internal ptr, no refcount bump or alloc. We must not free. + LOG_ASSERT(verify_param != nullptr); + vespalib::string host = _peer_spec.host(); + if (X509_VERIFY_PARAM_set1_host(verify_param, host.c_str(), host.size()) != 1) { + throw CryptoException("X509_VERIFY_PARAM_set1_host() failed"); + } + // TODO should we set expected IP based on peer address as well? + } +} + +void OpenSslCryptoCodecImpl::set_server_name_indication_extension() { + if (_peer_spec.valid()) { + vespalib::string host = _peer_spec.host(); + // OpenSSL tries to cast const char* to void* in a macro, even on 1.1.1. GCC is not overly impressed, + // so to satiate OpenSSL's quirks we pre-cast away the constness. + auto* host_cstr_that_trusts_openssl_not_to_mess_up = const_cast<char*>(host.c_str()); + if (SSL_set_tlsext_host_name(_ssl.get(), host_cstr_that_trusts_openssl_not_to_mess_up) != 1) { + throw CryptoException("SSL_set_tlsext_host_name() failed"); + } + } +} + +std::optional<vespalib::string> OpenSslCryptoCodecImpl::client_provided_sni_extension() const { + if ((_mode != Mode::Server) || (SSL_get_servername_type(_ssl.get()) != TLSEXT_NAMETYPE_host_name)) { + return {}; + } + const char* sni_host_raw = SSL_get_servername(_ssl.get(), TLSEXT_NAMETYPE_host_name); + if (sni_host_raw == nullptr) { + return {}; + } + return vespalib::string(sni_host_raw); +} + HandshakeResult OpenSslCryptoCodecImpl::handshake(const char* from_peer, size_t from_peer_buf_size, char* to_peer, size_t to_peer_buf_size) noexcept { @@ -428,3 +485,5 @@ EncodeResult OpenSslCryptoCodecImpl::half_close(char* ciphertext, size_t ciphert // External references: // [0] http://openssl.6102.n7.nabble.com/nonblocking-implementation-question-tp1728p1732.html // [1] https://github.com/grpc/grpc/blob/master/src/core/tsi/ssl_transport_security.cc +// [2] https://wiki.openssl.org/index.php/Hostname_validation +// [3] https://wiki.openssl.org/index.php/SSL/TLS_Client diff --git a/vespalib/src/vespa/vespalib/net/tls/impl/openssl_crypto_codec_impl.h b/vespalib/src/vespa/vespalib/net/tls/impl/openssl_crypto_codec_impl.h index 14200de449a..ec8df853c16 100644 --- a/vespalib/src/vespa/vespalib/net/tls/impl/openssl_crypto_codec_impl.h +++ b/vespalib/src/vespa/vespalib/net/tls/impl/openssl_crypto_codec_impl.h @@ -3,6 +3,7 @@ #include "openssl_typedefs.h" #include <vespa/vespalib/net/socket_address.h> +#include <vespa/vespalib/net/socket_spec.h> #include <vespa/vespalib/net/tls/transport_security_options.h> #include <vespa/vespalib/net/tls/crypto_codec.h> #include <memory> @@ -46,17 +47,23 @@ class OpenSslCryptoCodecImpl : public CryptoCodec { // The context maintains shared verification callback state, so it must be // kept alive explictly for at least as long as any codecs. std::shared_ptr<OpenSslTlsContextImpl> _ctx; + SocketSpec _peer_spec; SocketAddress _peer_address; - SslPtr _ssl; - ::BIO* _input_bio; // Owned by _ssl - ::BIO* _output_bio; // Owned by _ssl - Mode _mode; + SslPtr _ssl; + ::BIO* _input_bio; // Owned by _ssl + ::BIO* _output_bio; // Owned by _ssl + Mode _mode; std::optional<DeferredHandshakeParams> _deferred_handshake_params; - std::optional<HandshakeResult> _deferred_handshake_result; + std::optional<HandshakeResult> _deferred_handshake_result; public: - OpenSslCryptoCodecImpl(std::shared_ptr<OpenSslTlsContextImpl> ctx, const SocketAddress& peer_address, Mode mode); ~OpenSslCryptoCodecImpl() override; + static std::unique_ptr<OpenSslCryptoCodecImpl> make_client_codec(std::shared_ptr<OpenSslTlsContextImpl> ctx, + const SocketSpec& peer_spec, + const SocketAddress& peer_address); + static std::unique_ptr<OpenSslCryptoCodecImpl> make_server_codec(std::shared_ptr<OpenSslTlsContextImpl> ctx, + const SocketAddress& peer_address); + /* * From RFC 8449 (Record Size Limit Extension for TLS), section 1: * "TLS versions 1.2 [RFC5246] and earlier permit senders to @@ -89,7 +96,20 @@ public: EncodeResult half_close(char* ciphertext, size_t ciphertext_size) noexcept override; const SocketAddress& peer_address() const noexcept { return _peer_address; } + /* + * If a client has sent a SNI extension field as part of the handshake, + * returns the raw string representation of this. It only makes sense to + * call this for codecs in server mode. + */ + std::optional<vespalib::string> client_provided_sni_extension() const; private: + OpenSslCryptoCodecImpl(std::shared_ptr<OpenSslTlsContextImpl> ctx, + const SocketSpec& peer_spec, + const SocketAddress& peer_address, + Mode mode); + + void enable_hostname_validation_if_requested(); + void set_server_name_indication_extension(); HandshakeResult do_handshake_and_consume_peer_input_bytes() noexcept; DecodeResult drain_and_produce_plaintext_from_ssl(char* plaintext, size_t plaintext_size) noexcept; // Precondition: read_result < 0 diff --git a/vespalib/src/vespa/vespalib/net/tls/peer_policies.h b/vespalib/src/vespa/vespalib/net/tls/peer_policies.h index 44cbe70fd92..c558708de8f 100644 --- a/vespalib/src/vespa/vespalib/net/tls/peer_policies.h +++ b/vespalib/src/vespa/vespalib/net/tls/peer_policies.h @@ -64,19 +64,24 @@ public: class AuthorizedPeers { // A peer will be authorized iff it matches _one or more_ policies. std::vector<PeerPolicy> _peer_policies; - bool _allow_all_if_empty = false; + bool _allow_all_if_empty; explicit AuthorizedPeers(bool allow_all_if_empty) : _peer_policies(), _allow_all_if_empty(allow_all_if_empty) {} public: - AuthorizedPeers() = default; + AuthorizedPeers() : _peer_policies(), _allow_all_if_empty(false) {} explicit AuthorizedPeers(std::vector<PeerPolicy> peer_policies_) : _peer_policies(std::move(peer_policies_)), _allow_all_if_empty(false) {} + AuthorizedPeers(const AuthorizedPeers&) = default; + AuthorizedPeers& operator=(const AuthorizedPeers&) = default; + AuthorizedPeers(AuthorizedPeers&&) noexcept = default; + AuthorizedPeers& operator=(AuthorizedPeers&&) noexcept = default; + static AuthorizedPeers allow_all_authenticated() { return AuthorizedPeers(true); } diff --git a/vespalib/src/vespa/vespalib/net/tls/tls_crypto_engine.cpp b/vespalib/src/vespa/vespalib/net/tls/tls_crypto_engine.cpp index d0475f3e88d..99862e83dbf 100644 --- a/vespalib/src/vespa/vespalib/net/tls/tls_crypto_engine.cpp +++ b/vespalib/src/vespa/vespalib/net/tls/tls_crypto_engine.cpp @@ -12,18 +12,16 @@ TlsCryptoEngine::TlsCryptoEngine(net::tls::TransportSecurityOptions tls_opts, ne } std::unique_ptr<TlsCryptoSocket> -TlsCryptoEngine::create_tls_client_crypto_socket(SocketHandle socket, const SocketSpec &) +TlsCryptoEngine::create_tls_client_crypto_socket(SocketHandle socket, const SocketSpec &peer_spec) { - auto mode = net::tls::CryptoCodec::Mode::Client; - auto codec = net::tls::CryptoCodec::create_default_codec(_tls_ctx, SocketAddress::peer_address(socket.get()), mode); + auto codec = net::tls::CryptoCodec::create_default_client_codec(_tls_ctx, peer_spec, SocketAddress::peer_address(socket.get())); return std::make_unique<net::tls::CryptoCodecAdapter>(std::move(socket), std::move(codec)); } std::unique_ptr<TlsCryptoSocket> TlsCryptoEngine::create_tls_server_crypto_socket(SocketHandle socket) { - auto mode = net::tls::CryptoCodec::Mode::Server; - auto codec = net::tls::CryptoCodec::create_default_codec(_tls_ctx, SocketAddress::peer_address(socket.get()), mode); + auto codec = net::tls::CryptoCodec::create_default_server_codec(_tls_ctx, SocketAddress::peer_address(socket.get())); return std::make_unique<net::tls::CryptoCodecAdapter>(std::move(socket), std::move(codec)); } diff --git a/vespalib/src/vespa/vespalib/net/tls/transport_security_options.cpp b/vespalib/src/vespa/vespalib/net/tls/transport_security_options.cpp index c9a5fa5a6e9..47b0e1e0a43 100644 --- a/vespalib/src/vespa/vespalib/net/tls/transport_security_options.cpp +++ b/vespalib/src/vespa/vespalib/net/tls/transport_security_options.cpp @@ -11,41 +11,57 @@ TransportSecurityOptions::TransportSecurityOptions(Params params) _cert_chain_pem(std::move(params._cert_chain_pem)), _private_key_pem(std::move(params._private_key_pem)), _authorized_peers(std::move(params._authorized_peers)), - _accepted_ciphers(std::move(params._accepted_ciphers)) + _accepted_ciphers(std::move(params._accepted_ciphers)), + _disable_hostname_validation(params._disable_hostname_validation) { } TransportSecurityOptions::TransportSecurityOptions(vespalib::string ca_certs_pem, vespalib::string cert_chain_pem, - vespalib::string private_key_pem) + vespalib::string private_key_pem, + AuthorizedPeers authorized_peers, + bool disable_hostname_validation) : _ca_certs_pem(std::move(ca_certs_pem)), _cert_chain_pem(std::move(cert_chain_pem)), _private_key_pem(std::move(private_key_pem)), - _authorized_peers(AuthorizedPeers::allow_all_authenticated()) + _authorized_peers(std::move(authorized_peers)), + _disable_hostname_validation(disable_hostname_validation) { } -TransportSecurityOptions::TransportSecurityOptions(vespalib::string ca_certs_pem, - vespalib::string cert_chain_pem, - vespalib::string private_key_pem, - AuthorizedPeers authorized_peers) - : _ca_certs_pem(std::move(ca_certs_pem)), - _cert_chain_pem(std::move(cert_chain_pem)), - _private_key_pem(std::move(private_key_pem)), - _authorized_peers(std::move(authorized_peers)) -{ +TransportSecurityOptions TransportSecurityOptions::copy_without_private_key() const { + return TransportSecurityOptions(_ca_certs_pem, _cert_chain_pem, "", + _authorized_peers, _disable_hostname_validation); } void secure_memzero(void* buf, size_t size) noexcept { OPENSSL_cleanse(buf, size); } -TransportSecurityOptions::Params::Params() = default; +TransportSecurityOptions::Params::Params() + : _ca_certs_pem(), + _cert_chain_pem(), + _private_key_pem(), + _authorized_peers(), + _accepted_ciphers(), + _disable_hostname_validation(false) +{ +} TransportSecurityOptions::Params::~Params() { secure_memzero(&_private_key_pem[0], _private_key_pem.size()); } +TransportSecurityOptions::Params::Params(const Params&) = default; + +TransportSecurityOptions::Params& +TransportSecurityOptions::Params::operator=(const TransportSecurityOptions::Params&) = default; + +TransportSecurityOptions::Params::Params(Params&&) noexcept = default; + +TransportSecurityOptions::Params& +TransportSecurityOptions::Params::operator=(TransportSecurityOptions::Params&&) noexcept = default; + TransportSecurityOptions::~TransportSecurityOptions() { secure_memzero(&_private_key_pem[0], _private_key_pem.size()); } diff --git a/vespalib/src/vespa/vespalib/net/tls/transport_security_options.h b/vespalib/src/vespa/vespalib/net/tls/transport_security_options.h index 39abb1ce4de..e0a48fc6cf5 100644 --- a/vespalib/src/vespa/vespalib/net/tls/transport_security_options.h +++ b/vespalib/src/vespa/vespalib/net/tls/transport_security_options.h @@ -14,18 +14,22 @@ class TransportSecurityOptions { vespalib::string _private_key_pem; AuthorizedPeers _authorized_peers; std::vector<vespalib::string> _accepted_ciphers; + bool _disable_hostname_validation; public: - TransportSecurityOptions() = default; - struct Params { vespalib::string _ca_certs_pem; vespalib::string _cert_chain_pem; vespalib::string _private_key_pem; AuthorizedPeers _authorized_peers; std::vector<vespalib::string> _accepted_ciphers; + bool _disable_hostname_validation; Params(); ~Params(); + Params(const Params&); + Params& operator=(const Params&); + Params(Params&&) noexcept; + Params& operator=(Params&&) noexcept; Params& ca_certs_pem(vespalib::stringref pem) { _ca_certs_pem = pem; return *this; } Params& cert_chain_pem(vespalib::stringref pem) { _cert_chain_pem = pem; return *this; } @@ -35,19 +39,14 @@ public: _accepted_ciphers = std::move(ciphers); return *this; } + Params& disable_hostname_validation(bool disable) { + _disable_hostname_validation = disable; + return *this; + } }; explicit TransportSecurityOptions(Params params); - TransportSecurityOptions(vespalib::string ca_certs_pem, - vespalib::string cert_chain_pem, - vespalib::string private_key_pem); - - TransportSecurityOptions(vespalib::string ca_certs_pem, - vespalib::string cert_chain_pem, - vespalib::string private_key_pem, - AuthorizedPeers authorized_peers); - ~TransportSecurityOptions(); const vespalib::string& ca_certs_pem() const noexcept { return _ca_certs_pem; } @@ -55,10 +54,16 @@ public: const vespalib::string& private_key_pem() const noexcept { return _private_key_pem; } const AuthorizedPeers& authorized_peers() const noexcept { return _authorized_peers; } - TransportSecurityOptions copy_without_private_key() const { - return TransportSecurityOptions(_ca_certs_pem, _cert_chain_pem, "", _authorized_peers); - } + TransportSecurityOptions copy_without_private_key() const; const std::vector<vespalib::string>& accepted_ciphers() const noexcept { return _accepted_ciphers; } + bool disable_hostname_validation() const noexcept { return _disable_hostname_validation; } + +private: + TransportSecurityOptions(vespalib::string ca_certs_pem, + vespalib::string cert_chain_pem, + vespalib::string private_key_pem, + AuthorizedPeers authorized_peers, + bool disable_hostname_validation); }; // Zeroes out `size` bytes in `buf` in a way that shall never be optimized diff --git a/vespalib/src/vespa/vespalib/net/tls/transport_security_options_reading.cpp b/vespalib/src/vespa/vespalib/net/tls/transport_security_options_reading.cpp index 6b6e65bef8a..80caa15e8b2 100644 --- a/vespalib/src/vespa/vespalib/net/tls/transport_security_options_reading.cpp +++ b/vespalib/src/vespa/vespalib/net/tls/transport_security_options_reading.cpp @@ -123,6 +123,12 @@ std::unique_ptr<TransportSecurityOptions> load_from_input(Input& input) { auto priv_key = load_file_referenced_by_field(files, "private-key"); auto authorized_peers = parse_authorized_peers(root["authorized-peers"]); auto accepted_ciphers = parse_accepted_ciphers(root["accepted-ciphers"]); + // FIXME this is temporary until we know it won't break a bunch of things! + // It's still possible to explicitly enable hostname validation by setting this to false. + bool disable_hostname_validation = true; + if (root["disable-hostname-validation"].valid()) { + disable_hostname_validation = root["disable-hostname-validation"].asBool(); + } auto options = std::make_unique<TransportSecurityOptions>( TransportSecurityOptions::Params() @@ -130,7 +136,8 @@ std::unique_ptr<TransportSecurityOptions> load_from_input(Input& input) { .cert_chain_pem(certs) .private_key_pem(priv_key) .authorized_peers(std::move(authorized_peers)) - .accepted_ciphers(std::move(accepted_ciphers))); + .accepted_ciphers(std::move(accepted_ciphers)) + .disable_hostname_validation(disable_hostname_validation)); secure_memzero(&priv_key[0], priv_key.size()); return options; } diff --git a/vespalib/src/vespa/vespalib/test/make_tls_options_for_testing.cpp b/vespalib/src/vespa/vespalib/test/make_tls_options_for_testing.cpp index dcd2ced8036..b31f2830976 100644 --- a/vespalib/src/vespa/vespalib/test/make_tls_options_for_testing.cpp +++ b/vespalib/src/vespa/vespalib/test/make_tls_options_for_testing.cpp @@ -70,7 +70,13 @@ namespace vespalib::test { SocketSpec local_spec("tcp/localhost:123"); vespalib::net::tls::TransportSecurityOptions make_tls_options_for_testing() { - return vespalib::net::tls::TransportSecurityOptions(ca_pem, cert_pem, key_pem); + auto ts_builder = vespalib::net::tls::TransportSecurityOptions::Params(). + ca_certs_pem(ca_pem). + cert_chain_pem(cert_pem). + private_key_pem(key_pem). + authorized_peers(vespalib::net::tls::AuthorizedPeers::allow_all_authenticated()). + disable_hostname_validation(true); // FIXME this is to avoid mass breakage of TLS'd networking tests. + return vespalib::net::tls::TransportSecurityOptions(std::move(ts_builder)); } } // namespace vespalib::test |