diff options
19 files changed, 191 insertions, 187 deletions
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/CloudSslProvider.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/CloudSslProvider.java index 5fa893e9599..b231a4ad847 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/CloudSslProvider.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/CloudSslProvider.java @@ -26,7 +26,8 @@ public class CloudSslProvider extends SslProvider { private final String caCertificate; private final ClientAuth.Enum clientAuthentication; - public CloudSslProvider(String servername, String privateKey, String certificate, String caCertificatePath, String caCertificate, ClientAuth.Enum clientAuthentication, boolean enableTokenSupport) { + public CloudSslProvider(String servername, String privateKey, String certificate, String caCertificatePath, + String caCertificate, ClientAuth.Enum clientAuthentication, boolean enableTokenSupport) { super(COMPONENT_ID_PREFIX, servername, componentClass(enableTokenSupport), null); this.privateKey = privateKey; this.certificate = certificate; diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/HostedSslConnectorFactory.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/HostedSslConnectorFactory.java index 5bf348e5bb5..2b13cd21e99 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/HostedSslConnectorFactory.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/ssl/HostedSslConnectorFactory.java @@ -3,7 +3,6 @@ package com.yahoo.vespa.model.container.http.ssl; import com.yahoo.config.model.api.EndpointCertificateSecrets; import com.yahoo.jdisc.http.ConnectorConfig; -import com.yahoo.jdisc.http.ConnectorConfig.Ssl.ClientAuth; import com.yahoo.security.tls.TlsContext; import com.yahoo.vespa.model.container.http.ConnectorFactory; @@ -18,96 +17,78 @@ import java.util.List; */ public class HostedSslConnectorFactory extends ConnectorFactory { - private static final List<String> INSECURE_WHITELISTED_PATHS = List.of("/status.html"); - private static final String DEFAULT_HOSTED_TRUSTSTORE = "/opt/yahoo/share/ssl/certs/athenz_certificate_bundle.pem"; - - private final boolean enforceClientAuth; - private final boolean enforceHandshakeClientAuth; - private final Collection<String> tlsCiphersOverride; - private final boolean enableProxyProtocolMixedMode; + private final SslClientAuth clientAuth; + private final List<String> tlsCiphersOverride; + private final boolean proxyProtocolEnabled; + private final boolean proxyProtocolMixedMode; private final Duration endpointConnectionTtl; - /** - * Create connector factory that uses a certificate provided by the config-model / configserver and default hosted Vespa truststore. - */ - public static HostedSslConnectorFactory withProvidedCertificate( - String serverName, EndpointCertificateSecrets endpointCertificateSecrets, boolean enforceHandshakeClientAuth, - Collection<String> tlsCiphersOverride, boolean enableProxyProtocolMixedMode, int port, - Duration endpointConnectionTtl, boolean enableTokenSupport) { - CloudSslProvider sslProvider = createConfiguredDirectSslProvider( - serverName, endpointCertificateSecrets, DEFAULT_HOSTED_TRUSTSTORE, /*tlsCaCertificates*/null, enforceHandshakeClientAuth, enableTokenSupport); - return new HostedSslConnectorFactory(sslProvider, false, enforceHandshakeClientAuth, tlsCiphersOverride, - enableProxyProtocolMixedMode, port, endpointConnectionTtl); - } - - /** - * Create connector factory that uses a certificate provided by the config-model / configserver and a truststore configured by the application. - */ - public static HostedSslConnectorFactory withProvidedCertificateAndTruststore( - String serverName, EndpointCertificateSecrets endpointCertificateSecrets, String tlsCaCertificates, - Collection<String> tlsCiphersOverride, boolean enableProxyProtocolMixedMode, int port, - Duration endpointConnectionTtl, boolean enableTokenSupport) { - CloudSslProvider sslProvider = createConfiguredDirectSslProvider( - serverName, endpointCertificateSecrets, /*tlsCaCertificatesPath*/null, tlsCaCertificates, false, enableTokenSupport); - return new HostedSslConnectorFactory(sslProvider, true, false, tlsCiphersOverride, enableProxyProtocolMixedMode, - port, endpointConnectionTtl); - } - - /** - * Create connector factory that uses the default certificate and truststore provided by Vespa (through Vespa-global TLS configuration). - */ - public static HostedSslConnectorFactory withDefaultCertificateAndTruststore(String serverName, Collection<String> tlsCiphersOverride, - boolean enableProxyProtocolMixedMode, int port, - Duration endpointConnectionTtl) { - return new HostedSslConnectorFactory(new DefaultSslProvider(serverName), true, false, tlsCiphersOverride, - enableProxyProtocolMixedMode, port, endpointConnectionTtl); - } + public static Builder builder(String name, int listenPort) { return new Builder(name, listenPort); } - private HostedSslConnectorFactory(SslProvider sslProvider, boolean enforceClientAuth, - boolean enforceHandshakeClientAuth, Collection<String> tlsCiphersOverride, - boolean enableProxyProtocolMixedMode, int port, Duration endpointConnectionTtl) { - super(new Builder("tls"+port, port).sslProvider(sslProvider)); - this.enforceClientAuth = enforceClientAuth; - this.enforceHandshakeClientAuth = enforceHandshakeClientAuth; - this.tlsCiphersOverride = tlsCiphersOverride; - this.enableProxyProtocolMixedMode = enableProxyProtocolMixedMode; - this.endpointConnectionTtl = endpointConnectionTtl; + private HostedSslConnectorFactory(Builder builder) { + super(new ConnectorFactory.Builder("tls"+builder.port, builder.port).sslProvider(createSslProvider(builder))); + this.clientAuth = builder.clientAuth; + this.tlsCiphersOverride = List.copyOf(builder.tlsCiphersOverride); + this.proxyProtocolEnabled = builder.proxyProtocolEnabled; + this.proxyProtocolMixedMode = builder.proxyProtocolMixedMode; + this.endpointConnectionTtl = builder.endpointConnectionTtl; } - private static CloudSslProvider createConfiguredDirectSslProvider( - String serverName, EndpointCertificateSecrets endpointCertificateSecrets, String tlsCaCertificatesPath, String tlsCaCertificates, boolean enforceHandshakeClientAuth, boolean enableTokenSupport) { - var clientAuthentication = enforceHandshakeClientAuth ? ClientAuth.Enum.NEED_AUTH : ClientAuth.Enum.WANT_AUTH; + private static SslProvider createSslProvider(Builder builder) { + if (builder.endpointCertificate == null) return new DefaultSslProvider(builder.name); + var sslClientAuth = builder.clientAuth == SslClientAuth.NEED + ? ConnectorConfig.Ssl.ClientAuth.Enum.NEED_AUTH : ConnectorConfig.Ssl.ClientAuth.Enum.WANT_AUTH; return new CloudSslProvider( - serverName, - endpointCertificateSecrets.key(), - endpointCertificateSecrets.certificate(), - tlsCaCertificatesPath, - tlsCaCertificates, - clientAuthentication, - enableTokenSupport); + builder.name, builder.endpointCertificate.key(), builder.endpointCertificate.certificate(), + builder.tlsCaCertificatesPath, builder.tlsCaCertificatesPem, sslClientAuth, builder.tokenEndpoint); } @Override public void getConfig(ConnectorConfig.Builder connectorBuilder) { super.getConfig(connectorBuilder); - if (! enforceHandshakeClientAuth) { - connectorBuilder - .tlsClientAuthEnforcer(new ConnectorConfig.TlsClientAuthEnforcer.Builder() - .pathWhitelist(INSECURE_WHITELISTED_PATHS) - .enable(enforceClientAuth)); + if (clientAuth == SslClientAuth.WANT_WITH_ENFORCER) { + connectorBuilder.tlsClientAuthEnforcer( + new ConnectorConfig.TlsClientAuthEnforcer.Builder() + .pathWhitelist(List.of("/status.html")).enable(true)); } // Disables TLSv1.3 as it causes some browsers to prompt user for client certificate (when connector has 'want' auth) connectorBuilder.ssl.enabledProtocols(List.of("TLSv1.2")); - if (!tlsCiphersOverride.isEmpty()) { connectorBuilder.ssl.enabledCipherSuites(tlsCiphersOverride.stream().sorted().toList()); } else { connectorBuilder.ssl.enabledCipherSuites(TlsContext.ALLOWED_CIPHER_SUITES.stream().sorted().toList()); } - connectorBuilder - .proxyProtocol(new ConnectorConfig.ProxyProtocol.Builder().enabled(true).mixedMode(enableProxyProtocolMixedMode)) + .proxyProtocol(new ConnectorConfig.ProxyProtocol.Builder() + .enabled(proxyProtocolEnabled).mixedMode(proxyProtocolMixedMode)) .idleTimeout(Duration.ofSeconds(30).toSeconds()) .maxConnectionLife(endpointConnectionTtl != null ? endpointConnectionTtl.toSeconds() : 0); } + + public enum SslClientAuth { WANT, NEED, WANT_WITH_ENFORCER } + public static class Builder { + final String name; + final int port; + SslClientAuth clientAuth; + List<String> tlsCiphersOverride; + boolean proxyProtocolEnabled; + boolean proxyProtocolMixedMode; + Duration endpointConnectionTtl; + EndpointCertificateSecrets endpointCertificate; + String tlsCaCertificatesPem; + String tlsCaCertificatesPath; + boolean tokenEndpoint; + + private Builder(String name, int port) { this.name = name; this.port = port; } + public Builder clientAuth(SslClientAuth auth) { clientAuth = auth; return this; } + public Builder endpointConnectionTtl(Duration ttl) { endpointConnectionTtl = ttl; return this; } + public Builder tlsCiphersOverride(Collection<String> ciphers) { tlsCiphersOverride = List.copyOf(ciphers); return this; } + public Builder proxyProtocol(boolean enabled, boolean mixedMode) { proxyProtocolEnabled = enabled; proxyProtocolMixedMode = mixedMode; return this; } + public Builder endpointCertificate(EndpointCertificateSecrets cert) { this.endpointCertificate = cert; return this; } + public Builder tlsCaCertificatesPath(String path) { this.tlsCaCertificatesPath = path; return this; } + public Builder tlsCaCertificatesPem(String pem) { this.tlsCaCertificatesPem = pem; return this; } + public Builder tokenEndpoint(boolean enable) { this.tokenEndpoint = enable; return this; } + + public HostedSslConnectorFactory build() { return new HostedSslConnectorFactory(this); } + } } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index 00feb0a1c76..c97ea6671e8 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -16,7 +16,6 @@ import com.yahoo.config.model.ConfigModelContext; import com.yahoo.config.model.api.ApplicationClusterEndpoint; import com.yahoo.config.model.api.ConfigServerSpec; import com.yahoo.config.model.api.ContainerEndpoint; -import com.yahoo.config.model.api.EndpointCertificateSecrets; import com.yahoo.config.model.api.TenantSecretStore; import com.yahoo.config.model.application.provider.IncludeDirs; import com.yahoo.config.model.builder.xml.ConfigModelBuilder; @@ -95,6 +94,7 @@ import com.yahoo.vespa.model.container.http.Http; import com.yahoo.vespa.model.container.http.HttpFilterChain; import com.yahoo.vespa.model.container.http.JettyHttpServer; import com.yahoo.vespa.model.container.http.ssl.HostedSslConnectorFactory; +import com.yahoo.vespa.model.container.http.ssl.HostedSslConnectorFactory.SslClientAuth; import com.yahoo.vespa.model.container.http.xml.HttpBuilder; import com.yahoo.vespa.model.container.processing.ProcessingChains; import com.yahoo.vespa.model.container.search.ContainerSearch; @@ -109,7 +109,6 @@ import java.io.IOException; import java.io.Reader; import java.net.URI; import java.security.cert.X509Certificate; -import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -600,31 +599,36 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { .ifPresent(accessControl -> accessControl.configureDefaultHostedConnector(cluster.getHttp())); ; } - private void addAdditionalHostedConnector(DeployState deployState, ApplicationContainerCluster cluster) { + private void addAdditionalHostedConnector(DeployState state, ApplicationContainerCluster cluster) { JettyHttpServer server = cluster.getHttp().getHttpServer().get(); String serverName = server.getComponentId().getName(); // If the deployment contains certificate/private key reference, setup TLS port - HostedSslConnectorFactory connectorFactory; - Collection<String> tlsCiphersOverride = deployState.getProperties().tlsCiphersOverride(); - boolean proxyProtocolMixedMode = deployState.getProperties().featureFlags().enableProxyProtocolMixedMode(); - Duration endpointConnectionTtl = deployState.getProperties().endpointConnectionTtl(); - var port = getDataplanePort(deployState); - if (deployState.endpointCertificateSecrets().isPresent()) { - boolean authorizeClient = deployState.zone().system().isPublic(); + var builder = HostedSslConnectorFactory.builder(serverName, getDataplanePort(state)) + .proxyProtocol(true, state.getProperties().featureFlags().enableProxyProtocolMixedMode()) + .tlsCiphersOverride(state.getProperties().tlsCiphersOverride()) + .endpointConnectionTtl(state.getProperties().endpointConnectionTtl()); + var endpointCert = state.endpointCertificateSecrets().orElse(null); + if (endpointCert != null) { + builder.endpointCertificate(endpointCert); + boolean isPublic = state.zone().system().isPublic(); List<X509Certificate> clientCertificates = getClientCertificates(cluster); - if (authorizeClient && clientCertificates.isEmpty()) { - throw new IllegalArgumentException("Client certificate authority security/clients.pem is missing - " + - "see: https://cloud.vespa.ai/en/security/guide#data-plane"); + if (isPublic) { + if (clientCertificates.isEmpty()) + throw new IllegalArgumentException("Client certificate authority security/clients.pem is missing - " + + "see: https://cloud.vespa.ai/en/security/guide#data-plane"); + builder.tlsCaCertificatesPem(X509CertificateUtils.toPem(clientCertificates)) + .clientAuth(SslClientAuth.WANT_WITH_ENFORCER); + } else { + builder.tlsCaCertificatesPath("/opt/yahoo/share/ssl/certs/athenz_certificate_bundle.pem"); + var needAuth = cluster.getHttp().getAccessControl() + .map(accessControl -> accessControl.clientAuthentication) + .map(clientAuth -> clientAuth == AccessControl.ClientAuthentication.need) + .orElse(false); + builder.clientAuth(needAuth ? SslClientAuth.NEED : SslClientAuth.WANT); } - EndpointCertificateSecrets endpointCertificateSecrets = deployState.endpointCertificateSecrets().get(); - - boolean enforceHandshakeClientAuth = cluster.getHttp().getAccessControl() - .map(accessControl -> accessControl.clientAuthentication) - .map(clientAuth -> clientAuth == AccessControl.ClientAuthentication.need) - .orElse(false); - boolean enableTokenSupport = deployState.featureFlags().enableDataplaneProxy() + boolean enableTokenSupport = state.featureFlags().enableDataplaneProxy() && cluster.getClients().stream().anyMatch(c -> !c.tokens().isEmpty()); // Set up component to generate proxy cert if token support is enabled @@ -633,24 +637,16 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { cluster.addSimpleComponent(DataplaneProxyService.class); var dataplaneProxy = new DataplaneProxy( - getDataplanePort(deployState), - endpointCertificateSecrets.certificate(), - endpointCertificateSecrets.key()); + getDataplanePort(state), + endpointCert.certificate(), + endpointCert.key()); cluster.addComponent(dataplaneProxy); + builder.tokenEndpoint(true); } - - connectorFactory = authorizeClient - ? HostedSslConnectorFactory.withProvidedCertificateAndTruststore( - serverName, endpointCertificateSecrets, X509CertificateUtils.toPem(clientCertificates), - tlsCiphersOverride, proxyProtocolMixedMode, port, endpointConnectionTtl, enableTokenSupport) - : HostedSslConnectorFactory.withProvidedCertificate( - serverName, endpointCertificateSecrets, enforceHandshakeClientAuth, tlsCiphersOverride, - proxyProtocolMixedMode, port, endpointConnectionTtl, enableTokenSupport); } else { - connectorFactory = HostedSslConnectorFactory.withDefaultCertificateAndTruststore( - serverName, tlsCiphersOverride, proxyProtocolMixedMode, port, - endpointConnectionTtl); + builder.clientAuth(SslClientAuth.WANT_WITH_ENFORCER); } + var connectorFactory = builder.build(); cluster.getHttp().getAccessControl().ifPresent(accessControl -> accessControl.configureHostedConnector(connectorFactory)); server.addConnector(connectorFactory); } diff --git a/searchlib/src/tests/queryeval/filter_search/filter_search_test.cpp b/searchlib/src/tests/queryeval/filter_search/filter_search_test.cpp index 8f2f8f2e96b..e468560f4ec 100644 --- a/searchlib/src/tests/queryeval/filter_search/filter_search_test.cpp +++ b/searchlib/src/tests/queryeval/filter_search/filter_search_test.cpp @@ -67,7 +67,7 @@ struct LeafProxy : SimpleLeafBlueprint { } LeafProxy(std::unique_ptr<Blueprint> child_in) : SimpleLeafBlueprint(), child(std::move(child_in)) { init(); } - LeafProxy(const FieldSpec &field, std::unique_ptr<Blueprint> child_in) + LeafProxy(FieldSpecBase field, std::unique_ptr<Blueprint> child_in) : SimpleLeafBlueprint(field), child(std::move(child_in)) { init(); } SearchIteratorUP createLeafSearch(const TermFieldMatchDataArray &, bool) const override { abort(); } SearchIteratorUP createFilterSearch(bool strict, Constraint constraint) const override { diff --git a/searchlib/src/tests/queryeval/weighted_set_term/weighted_set_term_test.cpp b/searchlib/src/tests/queryeval/weighted_set_term/weighted_set_term_test.cpp index f93aa537625..9d0dd05e3e3 100644 --- a/searchlib/src/tests/queryeval/weighted_set_term/weighted_set_term_test.cpp +++ b/searchlib/src/tests/queryeval/weighted_set_term/weighted_set_term_test.cpp @@ -3,7 +3,6 @@ #include <vespa/vespalib/testkit/testapp.h> #include <vespa/searchlib/queryeval/weighted_set_term_search.h> -#include <vespa/searchlib/fef/fef.h> #include <vespa/searchlib/query/tree/simplequery.h> #include <vespa/searchlib/queryeval/field_spec.h> #include <vespa/searchlib/queryeval/blueprint.h> @@ -282,7 +281,7 @@ TEST("verify search iterator conformance with document weight iterator children" struct VerifyMatchData { struct MyBlueprint : search::queryeval::SimpleLeafBlueprint { VerifyMatchData &vmd; - MyBlueprint(VerifyMatchData &vmd_in, const FieldSpec & spec_in) + MyBlueprint(VerifyMatchData &vmd_in, FieldSpecBase spec_in) : SimpleLeafBlueprint(spec_in), vmd(vmd_in) {} [[nodiscard]] SearchIterator::UP createLeafSearch(const fef::TermFieldMatchDataArray &tfmda, bool) const override { EXPECT_EQUAL(tfmda.size(), 1u); @@ -301,7 +300,7 @@ struct VerifyMatchData { }; size_t child_cnt = 0; TermFieldMatchData *child_tfmd = nullptr; - search::queryeval::Blueprint::UP create(const FieldSpec &spec) { + search::queryeval::Blueprint::UP create(FieldSpecBase spec) { return std::make_unique<MyBlueprint>(*this, spec); } }; diff --git a/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp b/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp index ba791444dea..6cb5dbf7889 100644 --- a/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp @@ -74,6 +74,7 @@ using search::queryeval::ComplexLeafBlueprint; using search::queryeval::CreateBlueprintVisitorHelper; using search::queryeval::DotProductBlueprint; using search::queryeval::FieldSpec; +using search::queryeval::FieldSpecBase; using search::queryeval::FieldSpecBaseList; using search::queryeval::FilterWrapper; using search::queryeval::IRequestContext; @@ -129,27 +130,11 @@ private: Type _type; public: - AttributeFieldBlueprint(const FieldSpec &field, const IAttributeVector &attribute, - const string &query_stack, const SearchContextParams ¶ms) - : AttributeFieldBlueprint(field, attribute, QueryTermDecoder::decodeTerm(query_stack), params) - { } - AttributeFieldBlueprint(const FieldSpec &field, const IAttributeVector &attribute, - QueryTermSimple::UP term, const SearchContextParams ¶ms) - : SimpleLeafBlueprint(field), - _attr(attribute), - _query_term(term->getTermString()), - _search_context(attribute.createSearchContext(std::move(term), params)), - _type(OTHER) - { - uint32_t estHits = _search_context->approximateHits(); - HitEstimate estimate(estHits, estHits == 0); - setEstimate(estimate); - if (attribute.isFloatingPointType()) { - _type = FLOAT; - } else if (attribute.isIntegerType()) { - _type = INT; - } - } + AttributeFieldBlueprint(FieldSpecBase field, const IAttributeVector &attribute, + const string &query_stack, const SearchContextParams ¶ms); + AttributeFieldBlueprint(FieldSpecBase field, const IAttributeVector &attribute, + QueryTermSimple::UP term, const SearchContextParams ¶ms); + ~AttributeFieldBlueprint() override; SearchIteratorUP createLeafSearch(const TermFieldMatchDataArray &tfmda, bool strict) const override { assert(tfmda.size() == 1); @@ -181,6 +166,31 @@ public: bool getRange(vespalib::string &from, vespalib::string &to) const override; }; +AttributeFieldBlueprint::~AttributeFieldBlueprint() = default; + +AttributeFieldBlueprint::AttributeFieldBlueprint(FieldSpecBase field, const IAttributeVector &attribute, + const string &query_stack, const SearchContextParams ¶ms) + : AttributeFieldBlueprint(field, attribute, QueryTermDecoder::decodeTerm(query_stack), params) +{ } + +AttributeFieldBlueprint::AttributeFieldBlueprint(FieldSpecBase field, const IAttributeVector &attribute, + QueryTermSimple::UP term, const SearchContextParams ¶ms) + : SimpleLeafBlueprint(field), + _attr(attribute), + _query_term(term->getTermString()), + _search_context(attribute.createSearchContext(std::move(term), params)), + _type(OTHER) +{ + uint32_t estHits = _search_context->approximateHits(); + HitEstimate estimate(estHits, estHits == 0); + setEstimate(estimate); + if (attribute.isFloatingPointType()) { + _type = FLOAT; + } else if (attribute.isIntegerType()) { + _type = INT; + } +} + vespalib::string get_type(const IAttributeVector& attr) { @@ -866,7 +876,7 @@ CreateBlueprintVisitor::createShallowWeightedSet(WS *bp, MultiTerm &n, const Fie bp->reserve(n.getNumTerms()); Blueprint::HitEstimate estimate; for (uint32_t i(0); i < n.getNumTerms(); i++) { - FieldSpec childfs = bp->getNextChildField(fs); + FieldSpecBase childfs = bp->getNextChildField(fs); auto term = n.getAsString(i); bp->addTerm(std::make_unique<AttributeFieldBlueprint>(childfs, _attr, extractTerm(term.first, isInteger), scParams.useBitVector(childfs.isFilter())), term.second.percent(), estimate); } diff --git a/searchlib/src/vespa/searchlib/common/matching_elements_fields.h b/searchlib/src/vespa/searchlib/common/matching_elements_fields.h index cae28276eef..e4d61f8dedc 100644 --- a/searchlib/src/vespa/searchlib/common/matching_elements_fields.h +++ b/searchlib/src/vespa/searchlib/common/matching_elements_fields.h @@ -28,7 +28,7 @@ public: _fields.insert(field_name); } void add_mapping(const vespalib::string &field_name, - const vespalib::string &struct_field_name) { + const vespalib::string &struct_field_name) { _fields.insert(field_name); _struct_fields[struct_field_name] = field_name; } diff --git a/searchlib/src/vespa/searchlib/queryeval/create_blueprint_visitor_helper.cpp b/searchlib/src/vespa/searchlib/queryeval/create_blueprint_visitor_helper.cpp index bb44eaa0f3d..0719e4511be 100644 --- a/searchlib/src/vespa/searchlib/queryeval/create_blueprint_visitor_helper.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/create_blueprint_visitor_helper.cpp @@ -77,11 +77,12 @@ void CreateBlueprintVisitorHelper::createWeightedSet(std::unique_ptr<WS> bp, NODE &n) { bp->reserve(n.getNumTerms()); Blueprint::HitEstimate estimate; + FieldSpec childField(_field); for (size_t i = 0; i < n.getNumTerms(); ++i) { auto term = n.getAsString(i); query::SimpleStringTerm node(term.first, n.getView(), 0, term.second); // TODO Temporary - FieldSpec field = bp->getNextChildField(_field); - bp->addTerm(_searchable.createBlueprint(_requestContext, field, node), term.second.percent(), estimate); + childField.setBase(bp->getNextChildField(_field)); + bp->addTerm(_searchable.createBlueprint(_requestContext, childField, node), term.second.percent(), estimate); } bp->complete(estimate); setResult(std::move(bp)); diff --git a/searchlib/src/vespa/searchlib/queryeval/dot_product_blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/dot_product_blueprint.cpp index 3e85ae4d00a..795f5f1424a 100644 --- a/searchlib/src/vespa/searchlib/queryeval/dot_product_blueprint.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/dot_product_blueprint.cpp @@ -16,12 +16,6 @@ DotProductBlueprint::DotProductBlueprint(const FieldSpec &field) DotProductBlueprint::~DotProductBlueprint() = default; -FieldSpec -DotProductBlueprint::getNextChildField(const FieldSpec &outer) -{ - return FieldSpec(outer.getName(), outer.getFieldId(), _layout.allocTermField(outer.getFieldId()), false); -} - void DotProductBlueprint::reserve(size_t num_children) { _weights.reserve(num_children); diff --git a/searchlib/src/vespa/searchlib/queryeval/dot_product_blueprint.h b/searchlib/src/vespa/searchlib/queryeval/dot_product_blueprint.h index 18770691350..2704d76d3db 100644 --- a/searchlib/src/vespa/searchlib/queryeval/dot_product_blueprint.h +++ b/searchlib/src/vespa/searchlib/queryeval/dot_product_blueprint.h @@ -22,7 +22,9 @@ public: ~DotProductBlueprint() override; // used by create visitor - FieldSpec getNextChildField(const FieldSpec &outer); + FieldSpecBase getNextChildField(FieldSpecBase parent) { + return {parent.getFieldId(), _layout.allocTermField(parent.getFieldId()), false}; + } // used by create visitor void reserve(size_t num_children); diff --git a/searchlib/src/vespa/searchlib/queryeval/field_spec.h b/searchlib/src/vespa/searchlib/queryeval/field_spec.h index fd925fdf4ff..3fe43597602 100644 --- a/searchlib/src/vespa/searchlib/queryeval/field_spec.h +++ b/searchlib/src/vespa/searchlib/queryeval/field_spec.h @@ -44,9 +44,16 @@ public: : FieldSpecBase(fieldId, handle, isFilter_), _name(name) {} + FieldSpec(const vespalib::string & name, FieldSpecBase base) + : FieldSpecBase(base), + _name(name) + {} ~FieldSpec(); - const vespalib::string & getName() const { return _name; } + void setBase(FieldSpecBase base) { + static_cast<FieldSpecBase &>(*this) = base; + } + const vespalib::string & getName() const noexcept { return _name; } private: vespalib::string _name; // field name }; diff --git a/searchlib/src/vespa/searchlib/queryeval/wand/parallel_weak_and_blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/wand/parallel_weak_and_blueprint.cpp index e303e0b16d9..48a09f099a6 100644 --- a/searchlib/src/vespa/searchlib/queryeval/wand/parallel_weak_and_blueprint.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/wand/parallel_weak_and_blueprint.cpp @@ -12,12 +12,11 @@ namespace search::queryeval { -ParallelWeakAndBlueprint::ParallelWeakAndBlueprint(const FieldSpec &field, +ParallelWeakAndBlueprint::ParallelWeakAndBlueprint(FieldSpecBase field, uint32_t scoresToTrack, score_t scoreThreshold, double thresholdBoostFactor) : ComplexLeafBlueprint(field), - _field(field), _scores(scoresToTrack), _scoreThreshold(scoreThreshold), _thresholdBoostFactor(thresholdBoostFactor), @@ -28,13 +27,12 @@ ParallelWeakAndBlueprint::ParallelWeakAndBlueprint(const FieldSpec &field, { } -ParallelWeakAndBlueprint::ParallelWeakAndBlueprint(const FieldSpec &field, +ParallelWeakAndBlueprint::ParallelWeakAndBlueprint(FieldSpecBase field, uint32_t scoresToTrack, score_t scoreThreshold, double thresholdBoostFactor, uint32_t scoresAdjustFrequency) : ComplexLeafBlueprint(field), - _field(field), _scores(scoresToTrack), _scoreThreshold(scoreThreshold), _thresholdBoostFactor(thresholdBoostFactor), @@ -47,12 +45,6 @@ ParallelWeakAndBlueprint::ParallelWeakAndBlueprint(const FieldSpec &field, ParallelWeakAndBlueprint::~ParallelWeakAndBlueprint() = default; -FieldSpec -ParallelWeakAndBlueprint::getNextChildField(const FieldSpec &outer) -{ - return FieldSpec(outer.getName(), outer.getFieldId(), _layout.allocTermField(outer.getFieldId()), false); -} - void ParallelWeakAndBlueprint::reserve(size_t num_children) { _weights.reserve(num_children); diff --git a/searchlib/src/vespa/searchlib/queryeval/wand/parallel_weak_and_blueprint.h b/searchlib/src/vespa/searchlib/queryeval/wand/parallel_weak_and_blueprint.h index cb4d44f4497..a8d066ee689 100644 --- a/searchlib/src/vespa/searchlib/queryeval/wand/parallel_weak_and_blueprint.h +++ b/searchlib/src/vespa/searchlib/queryeval/wand/parallel_weak_and_blueprint.h @@ -21,7 +21,6 @@ class ParallelWeakAndBlueprint : public ComplexLeafBlueprint private: using score_t = wand::score_t; - const FieldSpec _field; mutable SharedWeakAndPriorityQueue _scores; const wand::score_t _scoreThreshold; double _thresholdBoostFactor; @@ -30,15 +29,14 @@ private: std::vector<int32_t> _weights; std::vector<Blueprint::UP> _terms; - ParallelWeakAndBlueprint(const ParallelWeakAndBlueprint &); - ParallelWeakAndBlueprint &operator=(const ParallelWeakAndBlueprint &); - public: - ParallelWeakAndBlueprint(const FieldSpec &field, + ParallelWeakAndBlueprint(const ParallelWeakAndBlueprint &) = delete; + ParallelWeakAndBlueprint &operator=(const ParallelWeakAndBlueprint &) = delete; + ParallelWeakAndBlueprint(FieldSpecBase field, uint32_t scoresToTrack, score_t scoreThreshold, double thresholdBoostFactor); - ParallelWeakAndBlueprint(const FieldSpec &field, + ParallelWeakAndBlueprint(FieldSpecBase field, uint32_t scoresToTrack, score_t scoreThreshold, double thresholdBoostFactor, @@ -52,7 +50,9 @@ public: double getThresholdBoostFactor() const { return _thresholdBoostFactor; } // Used by create visitor - FieldSpec getNextChildField(const FieldSpec &outer); + FieldSpecBase getNextChildField(FieldSpecBase parent) { + return {parent.getFieldId(), _layout.allocTermField(parent.getFieldId()), false}; + } // Used by create visitor void reserve(size_t num_children); diff --git a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_blueprint.h b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_blueprint.h index b40ab421890..0e3c82444d7 100644 --- a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_blueprint.h +++ b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_blueprint.h @@ -26,7 +26,7 @@ public: // used by create visitor // matches signature in dot product blueprint for common blueprint // building code. Hands out the same field spec to all children. - FieldSpec getNextChildField(const FieldSpec &) { return _children_field; } + FieldSpecBase getNextChildField(FieldSpecBase) { return _children_field; } // used by create visitor void reserve(size_t num_children); @@ -39,9 +39,6 @@ public: SearchIteratorUP createFilterSearch(bool strict, FilterConstraint constraint) const override; std::unique_ptr<MatchingElementsSearch> create_matching_elements_search(const MatchingElementsFields &fields) const override; void visitMembers(vespalib::ObjectVisitor &visitor) const override; - const vespalib::string &field_name() const { return _children_field.getName(); } - const std::vector<Blueprint::UP> &get_terms() const { return _terms; } - private: void fetchPostings(const ExecuteInfo &execInfo) override; }; diff --git a/security-utils/src/main/java/com/yahoo/security/SideChannelSafe.java b/security-utils/src/main/java/com/yahoo/security/SideChannelSafe.java index bd085f6f624..3a46891085f 100644 --- a/security-utils/src/main/java/com/yahoo/security/SideChannelSafe.java +++ b/security-utils/src/main/java/com/yahoo/security/SideChannelSafe.java @@ -1,6 +1,8 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.security; +import org.bouncycastle.util.Arrays; + /** * Utility functions for comparing the contents of arrays without leaking information about the * data contained within them via timing side-channels. This is done by avoiding any branches @@ -13,18 +15,11 @@ package com.yahoo.security; public class SideChannelSafe { /** - * @return true iff all bytes in the array are zero. An empty array always returns false - * since it technically can't contain any zeros at all. + * @return true iff all bytes in the array are zero. An empty array always returns true + * to be in line with BouncyCastle semantics. */ public static boolean allZeros(byte[] buf) { - if (buf.length == 0) { - return false; - } - byte accu = 0; - for (byte b : buf) { - accu |= b; - } - return (accu == 0); + return Arrays.areAllZeroes(buf, 0, buf.length); } /** @@ -32,23 +27,14 @@ public class SideChannelSafe { * about the contents of either of the arrays. * * <strong>Important:</strong> the <em>length</em> of the arrays is not considered secret, and - * will be leaked if arrays of differing sizes are given. + * <em>may</em> be leaked if arrays of differing sizes are given. * * @param lhs first array of bytes to compare * @param rhs second array of bytes to compare * @return true iff both arrays have the same size and are element-wise identical */ public static boolean arraysEqual(byte[] lhs, byte[] rhs) { - if (lhs.length != rhs.length) { - return false; - } - // Only use constant time bitwise ops. `accu` will be non-zero if at least one bit - // differed in any byte compared between the two arrays. - byte accu = 0; - for (int i = 0; i < lhs.length; ++i) { - accu |= (byte)(lhs[i] ^ rhs[i]); - } - return (accu == 0); + return Arrays.constantTimeAreEqual(lhs, rhs); } } diff --git a/security-utils/src/main/java/com/yahoo/security/token/TokenCheckHash.java b/security-utils/src/main/java/com/yahoo/security/token/TokenCheckHash.java index 2ff47081784..b67b120ba7b 100644 --- a/security-utils/src/main/java/com/yahoo/security/token/TokenCheckHash.java +++ b/security-utils/src/main/java/com/yahoo/security/token/TokenCheckHash.java @@ -1,6 +1,8 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.security.token; +import com.yahoo.security.SideChannelSafe; + import java.util.Arrays; import static com.yahoo.security.ArrayUtils.hex; @@ -18,8 +20,9 @@ public record TokenCheckHash(byte[] hashBytes) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; TokenCheckHash tokenCheckHash = (TokenCheckHash) o; - // We don't consider token hashes secret data, so no harm in data-dependent equals() - return Arrays.equals(hashBytes, tokenCheckHash.hashBytes); + // Although not considered secret information, avoid leaking the contents of + // the check-hashes themselves via timing channels. + return SideChannelSafe.arraysEqual(hashBytes, tokenCheckHash.hashBytes); } @Override diff --git a/security-utils/src/test/java/com/yahoo/security/SideChannelSafeTest.java b/security-utils/src/test/java/com/yahoo/security/SideChannelSafeTest.java index 7a66ed6eb7f..9731bbffc38 100644 --- a/security-utils/src/test/java/com/yahoo/security/SideChannelSafeTest.java +++ b/security-utils/src/test/java/com/yahoo/security/SideChannelSafeTest.java @@ -14,7 +14,7 @@ public class SideChannelSafeTest { @Test void all_zeros_checks_length_and_array_contents() { - assertFalse(SideChannelSafe.allZeros(new byte[0])); + assertTrue(SideChannelSafe.allZeros(new byte[0])); assertFalse(SideChannelSafe.allZeros(new byte[]{ 1 })); assertTrue(SideChannelSafe.allZeros(new byte[]{ 0 })); assertFalse(SideChannelSafe.allZeros(new byte[]{ 0, 0, 127, 0 })); diff --git a/storage/src/vespa/storage/storageserver/statemanager.cpp b/storage/src/vespa/storage/storageserver/statemanager.cpp index 654fe0e1f5d..c228229e4ef 100644 --- a/storage/src/vespa/storage/storageserver/statemanager.cpp +++ b/storage/src/vespa/storage/storageserver/statemanager.cpp @@ -17,6 +17,7 @@ #include <vespa/vespalib/util/exceptions.h> #include <vespa/vespalib/util/string_escape.h> #include <vespa/vespalib/util/stringfmt.h> +#include <vespa/vespalib/util/time.h> #include <fstream> #include <vespa/log/log.h> @@ -68,6 +69,10 @@ StateManager::StateManager(StorageComponentRegister& compReg, _threadLock(), _systemStateHistory(), _systemStateHistorySize(50), + _start_time(vespalib::steady_clock::now()), + _health_ping_time(), + _health_ping_warn_interval(5min), + _health_ping_warn_time(_start_time + _health_ping_warn_interval), _hostInfo(std::move(hostInfo)), _controllers_observed_explicit_node_state(), _noThreadTestMode(testMode), @@ -391,6 +396,8 @@ StateManager::onGetNodeState(const api::GetNodeStateCommand::SP& cmd) std::shared_ptr<api::GetNodeStateReply> reply; { std::unique_lock guard(_stateLock); + _health_ping_time = vespalib::steady_clock::now(); + _health_ping_warn_time = _health_ping_time.value() + _health_ping_warn_interval; const bool is_up_to_date = (_controllers_observed_explicit_node_state.find(cmd->getSourceIndex()) != _controllers_observed_explicit_node_state.end()); if ((cmd->getExpectedState() != nullptr) @@ -479,6 +486,28 @@ StateManager::run(framework::ThreadHandle& thread) } void +StateManager::warn_on_missing_health_ping() +{ + vespalib::steady_time now(vespalib::steady_clock::now()); + std::optional<vespalib::steady_time> health_ping_time; + { + std::lock_guard lock(_stateLock); + if (now <= _health_ping_warn_time) { + return; + } + health_ping_time = _health_ping_time; + _health_ping_warn_time = now + _health_ping_warn_interval; + } + if (health_ping_time.has_value()) { + vespalib::duration duration = now - health_ping_time.value(); + LOG(warning, "Last health ping from cluster controller was %1.1f seconds ago", vespalib::to_s(duration)); + } else { + vespalib::duration duration = now - _start_time; + LOG(warning, "No health pings from cluster controller since startup %1.1f seconds ago", vespalib::to_s(duration)); + } +} + +void StateManager::tick() { bool almost_immediate_replies = _requested_almost_immediate_node_state_replies.load(std::memory_order_relaxed); if (almost_immediate_replies) { @@ -487,6 +516,7 @@ StateManager::tick() { } else { sendGetNodeStateReplies(_component.getClock().getMonotonicTime()); } + warn_on_missing_health_ping(); } bool diff --git a/storage/src/vespa/storage/storageserver/statemanager.h b/storage/src/vespa/storage/storageserver/statemanager.h index 0b9a47c2515..3b1291b1c3f 100644 --- a/storage/src/vespa/storage/storageserver/statemanager.h +++ b/storage/src/vespa/storage/storageserver/statemanager.h @@ -65,6 +65,10 @@ class StateManager : public NodeStateUpdater, std::condition_variable _threadCond; std::deque<TimeSysStatePair> _systemStateHistory; uint32_t _systemStateHistorySize; + const vespalib::steady_time _start_time; + std::optional<vespalib::steady_time> _health_ping_time; + vespalib::duration _health_ping_warn_interval; + vespalib::steady_time _health_ping_warn_time; std::unique_ptr<HostInfo> _hostInfo; std::unique_ptr<framework::Thread> _thread; // Controllers that have observed a GetNodeState response sent _after_ @@ -84,6 +88,7 @@ public: void onClose() override; void tick(); + void warn_on_missing_health_ping(); void print(std::ostream& out, bool verbose, const std::string& indent) const override; void reportHtmlStatus(std::ostream&, const framework::HttpUrlPath&) const override; |