From 660101a2c97d17dd371d4419831fb60cd2ce3e12 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Fri, 12 Jul 2024 15:04:00 -0700 Subject: [PATCH] [xDS] implement system root cert support (#37185) As per gRFC A82 (https://github.com/grpc/proposal/pull/436). Closes #37185 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/37185 from markdroth:xds_system_root_certs 9ee1e825586688f1bc4aad5c5d915c30dc4369b8 PiperOrigin-RevId: 651896612 --- build_autogenerated.yaml | 5 +- src/core/BUILD | 1 + .../credentials/xds/xds_credentials.cc | 20 +-- .../load_balancing/xds/xds_cluster_impl.cc | 48 ++++--- src/core/server/xds_server_config_fetcher.cc | 20 ++- src/core/xds/grpc/xds_certificate_provider.cc | 3 +- src/core/xds/grpc/xds_certificate_provider.h | 4 +- src/core/xds/grpc/xds_cluster_parser.cc | 6 +- src/core/xds/grpc/xds_common_types.cc | 52 +++++--- src/core/xds/grpc/xds_common_types.h | 10 +- src/core/xds/grpc/xds_common_types_parser.cc | 48 +++++-- src/core/xds/grpc/xds_listener_parser.cc | 31 +++-- src/proto/grpc/testing/xds/v3/tls.proto | 9 ++ test/core/xds/BUILD | 3 + .../xds/xds_cluster_resource_type_test.cc | 43 +++++- test/core/xds/xds_common_types_test.cc | 126 +++++++++++++++--- .../xds/xds_listener_resource_type_test.cc | 68 ++++++++-- test/cpp/end2end/xds/xds_end2end_test.cc | 18 +++ 18 files changed, 390 insertions(+), 125 deletions(-) diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 17de2c2bf2d..87028ad7526 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -21497,7 +21497,8 @@ targets: gtest: true build: test language: c++ - headers: [] + headers: + - test/core/test_util/scoped_env_var.h src: - src/proto/grpc/testing/xds/v3/address.proto - src/proto/grpc/testing/xds/v3/aggregate_cluster.proto @@ -21592,6 +21593,7 @@ targets: build: test language: c++ headers: + - test/core/test_util/scoped_env_var.h - test/cpp/util/cli_call.h - test/cpp/util/cli_credentials.h - test/cpp/util/config_grpc_cli.h @@ -22081,6 +22083,7 @@ targets: build: test language: c++ headers: + - test/core/test_util/scoped_env_var.h - test/cpp/util/cli_call.h - test/cpp/util/cli_credentials.h - test/cpp/util/config_grpc_cli.h diff --git a/src/core/BUILD b/src/core/BUILD index dc761e5a8e9..1f3fe2c37ed 100644 --- a/src/core/BUILD +++ b/src/core/BUILD @@ -5252,6 +5252,7 @@ grpc_cc_library( deps = [ "grpc_matchers", "json", + "match", "validation_errors", ], ) diff --git a/src/core/lib/security/credentials/xds/xds_credentials.cc b/src/core/lib/security/credentials/xds/xds_credentials.cc index 7de65edb356..c2e5b925e6d 100644 --- a/src/core/lib/security/credentials/xds/xds_credentials.cc +++ b/src/core/lib/security/credentials/xds/xds_credentials.cc @@ -142,18 +142,22 @@ XdsCredentials::create_security_connector( auto xds_certificate_provider = args->GetObjectRef(); if (xds_certificate_provider != nullptr) { const bool watch_root = xds_certificate_provider->ProvidesRootCerts(); + const bool use_system_root_certs = + xds_certificate_provider->UseSystemRootCerts(); const bool watch_identity = xds_certificate_provider->ProvidesIdentityCerts(); - if (watch_root || watch_identity) { + if (watch_root || use_system_root_certs || watch_identity) { auto tls_credentials_options = MakeRefCounted(); - tls_credentials_options->set_certificate_provider( - xds_certificate_provider); - if (watch_root) { - tls_credentials_options->set_watch_root_cert(true); - } - if (watch_identity) { - tls_credentials_options->set_watch_identity_pair(true); + if (watch_root || watch_identity) { + tls_credentials_options->set_certificate_provider( + xds_certificate_provider); + if (watch_root) { + tls_credentials_options->set_watch_root_cert(true); + } + if (watch_identity) { + tls_credentials_options->set_watch_identity_pair(true); + } } tls_credentials_options->set_verify_server_cert(true); tls_credentials_options->set_certificate_verifier( diff --git a/src/core/load_balancing/xds/xds_cluster_impl.cc b/src/core/load_balancing/xds/xds_cluster_impl.cc index 0a04f73a5a0..c3b025f3ed1 100644 --- a/src/core/load_balancing/xds/xds_cluster_impl.cc +++ b/src/core/load_balancing/xds/xds_cluster_impl.cc @@ -674,23 +674,35 @@ XdsClusterImplLb::MaybeCreateCertificateProviderLocked( return nullptr; } // Configure root cert. - absl::string_view root_provider_instance_name = - cluster_resource.common_tls_context.certificate_validation_context - .ca_certificate_provider_instance.instance_name; - absl::string_view root_cert_name = - cluster_resource.common_tls_context.certificate_validation_context - .ca_certificate_provider_instance.certificate_name; + absl::string_view root_cert_name; RefCountedPtr root_cert_provider; - if (!root_provider_instance_name.empty()) { - root_cert_provider = - xds_client_->certificate_provider_store() - .CreateOrGetCertificateProvider(root_provider_instance_name); - if (root_cert_provider == nullptr) { - return absl::InternalError( - absl::StrCat("Certificate provider instance name: \"", - root_provider_instance_name, "\" not recognized.")); - } - } + bool use_system_root_certs = false; + absl::Status status = Match( + cluster_resource.common_tls_context.certificate_validation_context + .ca_certs, + [](const absl::monostate&) { + // No root cert configured. + return absl::OkStatus(); + }, + [&](const CommonTlsContext::CertificateProviderPluginInstance& + cert_provider) { + root_cert_name = cert_provider.certificate_name; + root_cert_provider = + xds_client_->certificate_provider_store() + .CreateOrGetCertificateProvider(cert_provider.instance_name); + if (root_cert_provider == nullptr) { + return absl::InternalError( + absl::StrCat("Certificate provider instance name: \"", + cert_provider.instance_name, "\" not recognized.")); + } + return absl::OkStatus(); + }, + [&](const CommonTlsContext::CertificateValidationContext:: + SystemRootCerts&) { + use_system_root_certs = true; + return absl::OkStatus(); + }); + if (!status.ok()) return status; // Configure identity cert. absl::string_view identity_provider_instance_name = cluster_resource.common_tls_context.tls_certificate_provider_instance @@ -715,8 +727,8 @@ XdsClusterImplLb::MaybeCreateCertificateProviderLocked( .match_subject_alt_names; // Create xds cert provider. return MakeRefCounted( - root_cert_provider, root_cert_name, identity_cert_provider, - identity_cert_name, san_matchers); + std::move(root_cert_provider), root_cert_name, use_system_root_certs, + std::move(identity_cert_provider), identity_cert_name, san_matchers); } void XdsClusterImplLb::MaybeUpdatePickerLocked() { diff --git a/src/core/server/xds_server_config_fetcher.cc b/src/core/server/xds_server_config_fetcher.cc index 3592baba176..a5fb02adc99 100644 --- a/src/core/server/xds_server_config_fetcher.cc +++ b/src/core/server/xds_server_config_fetcher.cc @@ -801,23 +801,21 @@ XdsServerConfigFetcher::ListenerWatcher::FilterChainMatchManager:: auto it = certificate_providers_map_.find(filter_chain); if (it != certificate_providers_map_.end()) return it->second; // Configure root cert. - absl::string_view root_provider_instance_name = - filter_chain->downstream_tls_context.common_tls_context - .certificate_validation_context.ca_certificate_provider_instance - .instance_name; - absl::string_view root_provider_cert_name = - filter_chain->downstream_tls_context.common_tls_context - .certificate_validation_context.ca_certificate_provider_instance - .certificate_name; + auto* ca_cert_provider = + absl::get_if( + &filter_chain->downstream_tls_context.common_tls_context + .certificate_validation_context.ca_certs); + absl::string_view root_provider_cert_name; RefCountedPtr root_cert_provider; - if (!root_provider_instance_name.empty()) { + if (ca_cert_provider != nullptr) { + root_provider_cert_name = ca_cert_provider->certificate_name; root_cert_provider = xds_client_->certificate_provider_store() - .CreateOrGetCertificateProvider(root_provider_instance_name); + .CreateOrGetCertificateProvider(ca_cert_provider->instance_name); if (root_cert_provider == nullptr) { return absl::NotFoundError( absl::StrCat("Certificate provider instance name: \"", - root_provider_instance_name, "\" not recognized.")); + ca_cert_provider->instance_name, "\" not recognized.")); } } // Configure identity cert. diff --git a/src/core/xds/grpc/xds_certificate_provider.cc b/src/core/xds/grpc/xds_certificate_provider.cc index 58ce2da4446..35c537bf362 100644 --- a/src/core/xds/grpc/xds_certificate_provider.cc +++ b/src/core/xds/grpc/xds_certificate_provider.cc @@ -108,13 +108,14 @@ class IdentityCertificatesWatcher final XdsCertificateProvider::XdsCertificateProvider( RefCountedPtr root_cert_provider, - absl::string_view root_cert_name, + absl::string_view root_cert_name, bool use_system_root_certs, RefCountedPtr identity_cert_provider, absl::string_view identity_cert_name, std::vector san_matchers) : distributor_(MakeRefCounted()), root_cert_provider_(std::move(root_cert_provider)), root_cert_name_(root_cert_name), + use_system_root_certs_(use_system_root_certs), identity_cert_provider_(std::move(identity_cert_provider)), identity_cert_name_(identity_cert_name), san_matchers_(std::move(san_matchers)), diff --git a/src/core/xds/grpc/xds_certificate_provider.h b/src/core/xds/grpc/xds_certificate_provider.h index 14747d99c14..6c64de04571 100644 --- a/src/core/xds/grpc/xds_certificate_provider.h +++ b/src/core/xds/grpc/xds_certificate_provider.h @@ -46,7 +46,7 @@ class XdsCertificateProvider final : public grpc_tls_certificate_provider { // ctor for client side XdsCertificateProvider( RefCountedPtr root_cert_provider, - absl::string_view root_cert_name, + absl::string_view root_cert_name, bool use_system_root_certs, RefCountedPtr identity_cert_provider, absl::string_view identity_cert_name, std::vector san_matchers); @@ -67,6 +67,7 @@ class XdsCertificateProvider final : public grpc_tls_certificate_provider { UniqueTypeName type() const override; bool ProvidesRootCerts() const { return root_cert_provider_ != nullptr; } + bool UseSystemRootCerts() const { return use_system_root_certs_; } bool ProvidesIdentityCerts() const { return identity_cert_provider_ != nullptr; } @@ -99,6 +100,7 @@ class XdsCertificateProvider final : public grpc_tls_certificate_provider { RefCountedPtr distributor_; RefCountedPtr root_cert_provider_; std::string root_cert_name_; + bool use_system_root_certs_ = false; RefCountedPtr identity_cert_provider_; std::string identity_cert_name_; std::vector san_matchers_; diff --git a/src/core/xds/grpc/xds_cluster_parser.cc b/src/core/xds/grpc/xds_cluster_parser.cc index 50e3807b9b9..c26c62c458e 100644 --- a/src/core/xds/grpc/xds_cluster_parser.cc +++ b/src/core/xds/grpc/xds_cluster_parser.cc @@ -101,9 +101,9 @@ CommonTlsContext UpstreamTlsContextParse( common_tls_context = CommonTlsContextParse(context, common_tls_context_proto, errors); } - if (common_tls_context.certificate_validation_context - .ca_certificate_provider_instance.instance_name.empty()) { - errors->AddError("no CA certificate provider instance configured"); + if (absl::holds_alternative( + common_tls_context.certificate_validation_context.ca_certs)) { + errors->AddError("no CA certs configured"); } return common_tls_context; } diff --git a/src/core/xds/grpc/xds_common_types.cc b/src/core/xds/grpc/xds_common_types.cc index 4617334dddb..bb169304c68 100644 --- a/src/core/xds/grpc/xds_common_types.cc +++ b/src/core/xds/grpc/xds_common_types.cc @@ -20,25 +20,9 @@ #include "absl/strings/str_format.h" #include "absl/strings/str_join.h" -namespace grpc_core { +#include "src/core/lib/gprpp/match.h" -// -// CommonTlsContext::CertificateValidationContext -// - -std::string CommonTlsContext::CertificateValidationContext::ToString() const { - std::vector contents; - contents.reserve(match_subject_alt_names.size()); - for (const auto& match : match_subject_alt_names) { - contents.push_back(match.ToString()); - } - return absl::StrFormat("{match_subject_alt_names=[%s]}", - absl::StrJoin(contents, ", ")); -} - -bool CommonTlsContext::CertificateValidationContext::Empty() const { - return match_subject_alt_names.empty(); -} +namespace grpc_core { // // CommonTlsContext::CertificateProviderPluginInstance @@ -61,6 +45,38 @@ bool CommonTlsContext::CertificateProviderPluginInstance::Empty() const { return instance_name.empty() && certificate_name.empty(); } +// +// CommonTlsContext::CertificateValidationContext +// + +std::string CommonTlsContext::CertificateValidationContext::ToString() const { + std::vector contents; + Match( + ca_certs, [](const absl::monostate&) {}, + [&](const CertificateProviderPluginInstance& cert_provider) { + contents.push_back( + absl::StrCat("ca_certs=cert_provider", cert_provider.ToString())); + }, + [&](const SystemRootCerts&) { + contents.push_back("ca_certs=system_root_certs{}"); + }); + if (!match_subject_alt_names.empty()) { + std::vector san_matchers; + san_matchers.reserve(match_subject_alt_names.size()); + for (const auto& match : match_subject_alt_names) { + san_matchers.push_back(match.ToString()); + } + contents.push_back(absl::StrCat("match_subject_alt_names=[", + absl::StrJoin(san_matchers, ", "), "]")); + } + return absl::StrCat("{", absl::StrJoin(contents, ", "), "}"); +} + +bool CommonTlsContext::CertificateValidationContext::Empty() const { + return absl::holds_alternative(ca_certs) && + match_subject_alt_names.empty(); +} + // // CommonTlsContext // diff --git a/src/core/xds/grpc/xds_common_types.h b/src/core/xds/grpc/xds_common_types.h index 250f81857b4..2b0c746a5d1 100644 --- a/src/core/xds/grpc/xds_common_types.h +++ b/src/core/xds/grpc/xds_common_types.h @@ -44,12 +44,16 @@ struct CommonTlsContext { }; struct CertificateValidationContext { - CertificateProviderPluginInstance ca_certificate_provider_instance; + struct SystemRootCerts { + bool operator==(const SystemRootCerts&) const { return true; } + }; + absl::variant + ca_certs; std::vector match_subject_alt_names; bool operator==(const CertificateValidationContext& other) const { - return ca_certificate_provider_instance == - other.ca_certificate_provider_instance && + return ca_certs == other.ca_certs && match_subject_alt_names == other.match_subject_alt_names; } diff --git a/src/core/xds/grpc/xds_common_types_parser.cc b/src/core/xds/grpc/xds_common_types_parser.cc index 24cf86fd051..3da948fe503 100644 --- a/src/core/xds/grpc/xds_common_types_parser.cc +++ b/src/core/xds/grpc/xds_common_types_parser.cc @@ -44,6 +44,7 @@ #include #include +#include "src/core/lib/gprpp/env.h" #include "src/core/util/json/json_reader.h" #include "src/core/util/upb_utils.h" #include "src/core/xds/grpc/xds_bootstrap_grpc.h" @@ -76,6 +77,14 @@ Duration ParseDuration(const google_protobuf_Duration* proto_duration, namespace { +bool XdsSystemRootCertsEnabled() { + auto value = GetEnv("GRPC_EXPERIMENTAL_XDS_SYSTEM_ROOT_CERTS"); + if (!value.has_value()) return false; + bool parsed_value; + bool parse_succeeded = gpr_parse_bool_value(value->c_str(), &parsed_value); + return parse_succeeded && parsed_value; +} + // CertificateProviderInstance is deprecated but we are still supporting it for // backward compatibility reasons. Note that we still parse the data into the // same CertificateProviderPluginInstance struct since the fields are the same. @@ -202,9 +211,17 @@ CertificateValidationContextParse( if (ca_certificate_provider_instance != nullptr) { ValidationErrors::ScopedField field(errors, ".ca_certificate_provider_instance"); - certificate_validation_context.ca_certificate_provider_instance = + certificate_validation_context.ca_certs = CertificateProviderPluginInstanceParse( context, ca_certificate_provider_instance, errors); + } else if (XdsSystemRootCertsEnabled()) { + auto* system_root_certs = + envoy_extensions_transport_sockets_tls_v3_CertificateValidationContext_system_root_certs( + certificate_validation_context_proto); + if (system_root_certs != nullptr) { + certificate_validation_context.ca_certs = + CommonTlsContext::CertificateValidationContext::SystemRootCerts(); + } } if (envoy_extensions_transport_sockets_tls_v3_CertificateValidationContext_verify_certificate_spki( certificate_validation_context_proto, nullptr) != nullptr) { @@ -263,23 +280,26 @@ CommonTlsContext CommonTlsContextParse( errors); } // If after parsing default_validation_context, - // common_tls_context->certificate_validation_context.ca_certificate_provider_instance - // is empty, fall back onto + // common_tls_context->certificate_validation_context.ca_certs does not + // contain a cert provider, fall back onto // 'validation_context_certificate_provider_instance' inside // 'combined_validation_context'. Note that this way of fetching root // certificates is deprecated and will be removed in the future. // TODO(yashykt): Remove this once it's no longer needed. - const auto* validation_context_certificate_provider_instance = - envoy_extensions_transport_sockets_tls_v3_CommonTlsContext_CombinedCertificateValidationContext_validation_context_certificate_provider_instance( - combined_validation_context); - if (common_tls_context.certificate_validation_context - .ca_certificate_provider_instance.Empty() && - validation_context_certificate_provider_instance != nullptr) { - ValidationErrors::ScopedField field( - errors, ".validation_context_certificate_provider_instance"); - common_tls_context.certificate_validation_context - .ca_certificate_provider_instance = CertificateProviderInstanceParse( - context, validation_context_certificate_provider_instance, errors); + if (!absl::holds_alternative< + CommonTlsContext::CertificateProviderPluginInstance>( + common_tls_context.certificate_validation_context.ca_certs)) { + const auto* validation_context_certificate_provider_instance = + envoy_extensions_transport_sockets_tls_v3_CommonTlsContext_CombinedCertificateValidationContext_validation_context_certificate_provider_instance( + combined_validation_context); + if (validation_context_certificate_provider_instance != nullptr) { + ValidationErrors::ScopedField field( + errors, ".validation_context_certificate_provider_instance"); + common_tls_context.certificate_validation_context.ca_certs = + CertificateProviderInstanceParse( + context, validation_context_certificate_provider_instance, + errors); + } } } else { auto* validation_context = diff --git a/src/core/xds/grpc/xds_listener_parser.cc b/src/core/xds/grpc/xds_listener_parser.cc index d19b516e729..1c8ef65c5cf 100644 --- a/src/core/xds/grpc/xds_listener_parser.cc +++ b/src/core/xds/grpc/xds_listener_parser.cc @@ -403,10 +403,16 @@ XdsListenerResource::DownstreamTlsContext DownstreamTlsContextParse( ValidationErrors::ScopedField field(errors, ".common_tls_context"); downstream_tls_context.common_tls_context = CommonTlsContextParse(context, common_tls_context, errors); - // Note: We can't be more specific about the field name for this - // error, because we don't know which fields they were found in + // Note: We can't be more specific about the field names for these + // errors, because we don't know which fields they were found in // inside of CommonTlsContext, so we make the error message a bit // more verbose to compensate. + if (absl::holds_alternative< + CommonTlsContext::CertificateValidationContext::SystemRootCerts>( + downstream_tls_context.common_tls_context + .certificate_validation_context.ca_certs)) { + errors->AddError("system_root_certs not supported"); + } if (!downstream_tls_context.common_tls_context .certificate_validation_context.match_subject_alt_names.empty()) { errors->AddError("match_subject_alt_names not supported on servers"); @@ -428,14 +434,19 @@ XdsListenerResource::DownstreamTlsContext DownstreamTlsContextParse( if (require_client_certificate != nullptr) { downstream_tls_context.require_client_certificate = google_protobuf_BoolValue_value(require_client_certificate); - if (downstream_tls_context.require_client_certificate && - downstream_tls_context.common_tls_context.certificate_validation_context - .ca_certificate_provider_instance.instance_name.empty()) { - ValidationErrors::ScopedField field(errors, - ".require_client_certificate"); - errors->AddError( - "client certificate required but no certificate " - "provider instance specified for validation"); + if (downstream_tls_context.require_client_certificate) { + auto* ca_cert_provider = + absl::get_if( + &downstream_tls_context.common_tls_context + .certificate_validation_context.ca_certs); + if (ca_cert_provider == nullptr || + ca_cert_provider->instance_name.empty()) { + ValidationErrors::ScopedField field(errors, + ".require_client_certificate"); + errors->AddError( + "client certificate required but no certificate provider " + "instance specified for validation"); + } } } if (ParseBoolValue( diff --git a/src/proto/grpc/testing/xds/v3/tls.proto b/src/proto/grpc/testing/xds/v3/tls.proto index b2fc4532d8b..e2648a2b089 100644 --- a/src/proto/grpc/testing/xds/v3/tls.proto +++ b/src/proto/grpc/testing/xds/v3/tls.proto @@ -45,12 +45,21 @@ message CertificateProviderPluginInstance { } message CertificateValidationContext { + message SystemRootCerts { + } + // Certificate provider instance for fetching TLS certificates. // // Only one of *trusted_ca* and *ca_certificate_provider_instance* may be specified. // [#not-implemented-hide:] CertificateProviderPluginInstance ca_certificate_provider_instance = 13; + // Use system root certs for validation. + // If present, system root certs are used only if neither of the ``trusted_ca`` + // or ``ca_certificate_provider_instance`` fields are set. + // [#not-implemented-hide:] + SystemRootCerts system_root_certs = 17; + // An optional list of base64-encoded SHA-256 hashes. If specified, Envoy will verify that the // SHA-256 of the DER-encoded Subject Public Key Information (SPKI) of the presented certificate // matches one of the specified values. diff --git a/test/core/xds/BUILD b/test/core/xds/BUILD index 51413628d61..09a6553037a 100644 --- a/test/core/xds/BUILD +++ b/test/core/xds/BUILD @@ -225,6 +225,7 @@ grpc_cc_test( "//src/proto/grpc/testing/xds/v3:typed_struct_proto", "//src/proto/grpc/testing/xds/v3:udpa_typed_struct_proto", "//test/core/test_util:grpc_test_util", + "//test/core/test_util:scoped_env_var", "//test/cpp/util:grpc_cli_utils", ], ) @@ -273,6 +274,7 @@ grpc_cc_test( "//src/proto/grpc/testing/xds/v3:tls_proto", "//src/proto/grpc/testing/xds/v3:typed_struct_proto", "//test/core/test_util:grpc_test_util", + "//test/core/test_util:scoped_env_var", "//test/cpp/util:grpc_cli_utils", ], ) @@ -319,6 +321,7 @@ grpc_cc_test( "//src/proto/grpc/testing/xds/v3:typed_struct_proto", "//src/proto/grpc/testing/xds/v3:wrr_locality_proto", "//test/core/test_util:grpc_test_util", + "//test/core/test_util:scoped_env_var", ], ) diff --git a/test/core/xds/xds_cluster_resource_type_test.cc b/test/core/xds/xds_cluster_resource_type_test.cc index 053e5d59bb1..7cf4c192681 100644 --- a/test/core/xds/xds_cluster_resource_type_test.cc +++ b/test/core/xds/xds_cluster_resource_type_test.cc @@ -65,6 +65,7 @@ #include "src/proto/grpc/testing/xds/v3/tls.pb.h" #include "src/proto/grpc/testing/xds/v3/typed_struct.pb.h" #include "src/proto/grpc/testing/xds/v3/wrr_locality.pb.h" +#include "test/core/test_util/scoped_env_var.h" #include "test/core/test_util/test_config.h" using envoy::config::cluster::v3::Cluster; @@ -932,12 +933,40 @@ TEST_F(TlsConfigTest, MinimumValidConfig) { EXPECT_EQ(*decode_result.name, "foo"); auto& resource = static_cast(**decode_result.resource); - EXPECT_EQ(resource.common_tls_context.certificate_validation_context - .ca_certificate_provider_instance.instance_name, - "provider1"); - EXPECT_EQ(resource.common_tls_context.certificate_validation_context - .ca_certificate_provider_instance.certificate_name, - "cert_name"); + auto* ca_cert_provider = + absl::get_if( + &resource.common_tls_context.certificate_validation_context.ca_certs); + ASSERT_NE(ca_cert_provider, nullptr); + EXPECT_EQ(ca_cert_provider->instance_name, "provider1"); + EXPECT_EQ(ca_cert_provider->certificate_name, "cert_name"); +} + +TEST_F(TlsConfigTest, SystemRootCerts) { + ScopedExperimentalEnvVar env_var("GRPC_EXPERIMENTAL_XDS_SYSTEM_ROOT_CERTS"); + Cluster cluster; + cluster.set_name("foo"); + cluster.set_type(cluster.EDS); + cluster.mutable_eds_cluster_config()->mutable_eds_config()->mutable_self(); + auto* transport_socket = cluster.mutable_transport_socket(); + transport_socket->set_name("envoy.transport_sockets.tls"); + UpstreamTlsContext upstream_tls_context; + auto* common_tls_context = upstream_tls_context.mutable_common_tls_context(); + auto* validation_context = common_tls_context->mutable_validation_context(); + validation_context->mutable_system_root_certs(); + transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context); + std::string serialized_resource; + ASSERT_TRUE(cluster.SerializeToString(&serialized_resource)); + auto* resource_type = XdsClusterResourceType::Get(); + auto decode_result = + resource_type->Decode(decode_context_, serialized_resource); + ASSERT_TRUE(decode_result.resource.ok()) << decode_result.resource.status(); + ASSERT_TRUE(decode_result.name.has_value()); + EXPECT_EQ(*decode_result.name, "foo"); + auto& resource = + static_cast(**decode_result.resource); + ASSERT_TRUE(absl::holds_alternative< + CommonTlsContext::CertificateValidationContext::SystemRootCerts>( + resource.common_tls_context.certificate_validation_context.ca_certs)); } // This is just one example of where CommonTlsContext::Parse() will @@ -1078,7 +1107,7 @@ TEST_F(TlsConfigTest, CaCertProviderUnset) { "field:transport_socket.typed_config.value[" "envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext]" ".common_tls_context " - "error:no CA certificate provider instance configured]") + "error:no CA certs configured]") << decode_result.resource.status(); } diff --git a/test/core/xds/xds_common_types_test.cc b/test/core/xds/xds_common_types_test.cc index 5a4fb324691..b19a885a79e 100644 --- a/test/core/xds/xds_common_types_test.cc +++ b/test/core/xds/xds_common_types_test.cc @@ -56,6 +56,7 @@ #include "src/proto/grpc/testing/xds/v3/tls.pb.h" #include "src/proto/grpc/testing/xds/v3/typed_struct.pb.h" #include "src/proto/grpc/testing/xds/v3/udpa_typed_struct.pb.h" +#include "test/core/test_util/scoped_env_var.h" #include "test/core/test_util/test_config.h" #include "test/cpp/util/config_grpc_cli.h" @@ -206,6 +207,24 @@ class CommonTlsConfigTest : public XdsCommonTypesTest { } }; +TEST_F(CommonTlsConfigTest, NoCaCerts) { + // Construct proto. + CommonTlsContextProto common_tls_context_proto; + // Convert to upb. + const auto* upb_proto = ConvertToUpb(common_tls_context_proto); + ASSERT_NE(upb_proto, nullptr); + // Run test. + auto common_tls_context = Parse(upb_proto); + ASSERT_TRUE(common_tls_context.ok()) << common_tls_context.status(); + EXPECT_TRUE(absl::holds_alternative( + common_tls_context->certificate_validation_context.ca_certs)); + EXPECT_THAT(common_tls_context->certificate_validation_context + .match_subject_alt_names, + ::testing::ElementsAre()); + EXPECT_TRUE(common_tls_context->tls_certificate_provider_instance.Empty()) + << common_tls_context->tls_certificate_provider_instance.ToString(); +} + TEST_F(CommonTlsConfigTest, CaCertProviderInCombinedValidationContext) { // Construct proto. CommonTlsContextProto common_tls_context_proto; @@ -221,12 +240,12 @@ TEST_F(CommonTlsConfigTest, CaCertProviderInCombinedValidationContext) { // Run test. auto common_tls_context = Parse(upb_proto); ASSERT_TRUE(common_tls_context.ok()) << common_tls_context.status(); - EXPECT_EQ(common_tls_context->certificate_validation_context - .ca_certificate_provider_instance.instance_name, - "provider1"); - EXPECT_EQ(common_tls_context->certificate_validation_context - .ca_certificate_provider_instance.certificate_name, - "cert_name"); + auto* ca_cert_provider = + absl::get_if( + &common_tls_context->certificate_validation_context.ca_certs); + ASSERT_NE(ca_cert_provider, nullptr); + EXPECT_EQ(ca_cert_provider->instance_name, "provider1"); + EXPECT_EQ(ca_cert_provider->certificate_name, "cert_name"); EXPECT_THAT(common_tls_context->certificate_validation_context .match_subject_alt_names, ::testing::ElementsAre()); @@ -247,12 +266,83 @@ TEST_F(CommonTlsConfigTest, CaCertProviderInValidationContext) { // Run test. auto common_tls_context = Parse(upb_proto); ASSERT_TRUE(common_tls_context.ok()) << common_tls_context.status(); - EXPECT_EQ(common_tls_context->certificate_validation_context - .ca_certificate_provider_instance.instance_name, - "provider1"); - EXPECT_EQ(common_tls_context->certificate_validation_context - .ca_certificate_provider_instance.certificate_name, - "cert_name"); + auto* ca_cert_provider = + absl::get_if( + &common_tls_context->certificate_validation_context.ca_certs); + ASSERT_NE(ca_cert_provider, nullptr); + EXPECT_EQ(ca_cert_provider->instance_name, "provider1"); + EXPECT_EQ(ca_cert_provider->certificate_name, "cert_name"); + EXPECT_THAT(common_tls_context->certificate_validation_context + .match_subject_alt_names, + ::testing::ElementsAre()); + EXPECT_TRUE(common_tls_context->tls_certificate_provider_instance.Empty()) + << common_tls_context->tls_certificate_provider_instance.ToString(); +} + +TEST_F(CommonTlsConfigTest, SystemRootCerts) { + ScopedExperimentalEnvVar env_var("GRPC_EXPERIMENTAL_XDS_SYSTEM_ROOT_CERTS"); + // Construct proto. + CommonTlsContextProto common_tls_context_proto; + common_tls_context_proto.mutable_validation_context() + ->mutable_system_root_certs(); + // Convert to upb. + const auto* upb_proto = ConvertToUpb(common_tls_context_proto); + ASSERT_NE(upb_proto, nullptr); + // Run test. + auto common_tls_context = Parse(upb_proto); + ASSERT_TRUE(common_tls_context.ok()) << common_tls_context.status(); + EXPECT_TRUE(absl::holds_alternative< + CommonTlsContext::CertificateValidationContext::SystemRootCerts>( + common_tls_context->certificate_validation_context.ca_certs)); + EXPECT_THAT(common_tls_context->certificate_validation_context + .match_subject_alt_names, + ::testing::ElementsAre()); + EXPECT_TRUE(common_tls_context->tls_certificate_provider_instance.Empty()) + << common_tls_context->tls_certificate_provider_instance.ToString(); +} + +TEST_F(CommonTlsConfigTest, CaCertProviderTakesPrecedenceOverSystemRootCerts) { + ScopedExperimentalEnvVar env_var("GRPC_EXPERIMENTAL_XDS_SYSTEM_ROOT_CERTS"); + // Construct proto. + CommonTlsContextProto common_tls_context_proto; + auto* cert_provider = common_tls_context_proto.mutable_validation_context() + ->mutable_ca_certificate_provider_instance(); + cert_provider->set_instance_name("provider1"); + cert_provider->set_certificate_name("cert_name"); + common_tls_context_proto.mutable_validation_context() + ->mutable_system_root_certs(); + // Convert to upb. + const auto* upb_proto = ConvertToUpb(common_tls_context_proto); + ASSERT_NE(upb_proto, nullptr); + // Run test. + auto common_tls_context = Parse(upb_proto); + ASSERT_TRUE(common_tls_context.ok()) << common_tls_context.status(); + auto* ca_cert_provider = + absl::get_if( + &common_tls_context->certificate_validation_context.ca_certs); + ASSERT_NE(ca_cert_provider, nullptr); + EXPECT_EQ(ca_cert_provider->instance_name, "provider1"); + EXPECT_EQ(ca_cert_provider->certificate_name, "cert_name"); + EXPECT_THAT(common_tls_context->certificate_validation_context + .match_subject_alt_names, + ::testing::ElementsAre()); + EXPECT_TRUE(common_tls_context->tls_certificate_provider_instance.Empty()) + << common_tls_context->tls_certificate_provider_instance.ToString(); +} + +TEST_F(CommonTlsConfigTest, SystemRootCertsIgnoredWithoutEnvVar) { + // Construct proto. + CommonTlsContextProto common_tls_context_proto; + common_tls_context_proto.mutable_validation_context() + ->mutable_system_root_certs(); + // Convert to upb. + const auto* upb_proto = ConvertToUpb(common_tls_context_proto); + ASSERT_NE(upb_proto, nullptr); + // Run test. + auto common_tls_context = Parse(upb_proto); + ASSERT_TRUE(common_tls_context.ok()) << common_tls_context.status(); + EXPECT_TRUE(absl::holds_alternative( + common_tls_context->certificate_validation_context.ca_certs)); EXPECT_THAT(common_tls_context->certificate_validation_context .match_subject_alt_names, ::testing::ElementsAre()); @@ -425,10 +515,8 @@ TEST_F(CommonTlsConfigTest, MatchSubjectAltNames) { EXPECT_EQ(match_subject_alt_names[4].type(), StringMatcher::Type::kSafeRegex); EXPECT_EQ(match_subject_alt_names[4].regex_matcher()->pattern(), "regex"); EXPECT_TRUE(match_subject_alt_names[4].case_sensitive()); - EXPECT_TRUE(common_tls_context->certificate_validation_context - .ca_certificate_provider_instance.Empty()) - << common_tls_context->certificate_validation_context - .ca_certificate_provider_instance.ToString(); + EXPECT_TRUE(absl::holds_alternative( + common_tls_context->certificate_validation_context.ca_certs)); EXPECT_TRUE(common_tls_context->tls_certificate_provider_instance.Empty()) << common_tls_context->tls_certificate_provider_instance.ToString(); } @@ -472,10 +560,8 @@ TEST_F(CommonTlsConfigTest, MatchSubjectAltNamesCaseInsensitive) { EXPECT_EQ(match_subject_alt_names[3].type(), StringMatcher::Type::kContains); EXPECT_EQ(match_subject_alt_names[3].string_matcher(), "contains"); EXPECT_FALSE(match_subject_alt_names[3].case_sensitive()); - EXPECT_TRUE(common_tls_context->certificate_validation_context - .ca_certificate_provider_instance.Empty()) - << common_tls_context->certificate_validation_context - .ca_certificate_provider_instance.ToString(); + EXPECT_TRUE(absl::holds_alternative( + common_tls_context->certificate_validation_context.ca_certs)); EXPECT_TRUE(common_tls_context->tls_certificate_provider_instance.Empty()) << common_tls_context->tls_certificate_provider_instance.ToString(); } diff --git a/test/core/xds/xds_listener_resource_type_test.cc b/test/core/xds/xds_listener_resource_type_test.cc index 718202d1e59..6106a8cda7e 100644 --- a/test/core/xds/xds_listener_resource_type_test.cc +++ b/test/core/xds/xds_listener_resource_type_test.cc @@ -65,6 +65,7 @@ #include "src/proto/grpc/testing/xds/v3/string.pb.h" #include "src/proto/grpc/testing/xds/v3/tls.pb.h" #include "src/proto/grpc/testing/xds/v3/typed_struct.pb.h" +#include "test/core/test_util/scoped_env_var.h" #include "test/core/test_util/test_config.h" using envoy::config::listener::v3::Listener; @@ -1559,11 +1560,56 @@ TEST_F(TcpListenerTest, DownstreamTlsContextWithCaCertProviderInstance) { tls_context.common_tls_context.tls_certificate_provider_instance; EXPECT_EQ(cert_provider_instance.instance_name, "provider1"); EXPECT_EQ(cert_provider_instance.certificate_name, "cert_name"); - auto& ca_cert_provider_instance = - tls_context.common_tls_context.certificate_validation_context - .ca_certificate_provider_instance; - EXPECT_EQ(ca_cert_provider_instance.instance_name, "provider1"); - EXPECT_EQ(ca_cert_provider_instance.certificate_name, "ca_cert_name"); + auto* ca_cert_provider = + absl::get_if( + &tls_context.common_tls_context.certificate_validation_context + .ca_certs); + ASSERT_NE(ca_cert_provider, nullptr); + EXPECT_EQ(ca_cert_provider->instance_name, "provider1"); + EXPECT_EQ(ca_cert_provider->certificate_name, "ca_cert_name"); +} + +TEST_F(TcpListenerTest, SystemRootCerts) { + ScopedExperimentalEnvVar env_var("GRPC_EXPERIMENTAL_XDS_SYSTEM_ROOT_CERTS"); + Listener listener; + listener.set_name("foo"); + HttpConnectionManager hcm; + auto* filter = hcm.add_http_filters(); + filter->set_name("router"); + filter->mutable_typed_config()->PackFrom(Router()); + auto* rds = hcm.mutable_rds(); + rds->set_route_config_name("rds_name"); + rds->mutable_config_source()->mutable_self(); + auto* filter_chain = listener.mutable_default_filter_chain(); + filter_chain->add_filters()->mutable_typed_config()->PackFrom(hcm); + auto* transport_socket = filter_chain->mutable_transport_socket(); + transport_socket->set_name("envoy.transport_sockets.tls"); + DownstreamTlsContext downstream_tls_context; + auto* common_tls_context = + downstream_tls_context.mutable_common_tls_context(); + auto* cert_provider = + common_tls_context->mutable_tls_certificate_provider_instance(); + cert_provider->set_instance_name("provider1"); + cert_provider->set_certificate_name("cert_name"); + common_tls_context->mutable_validation_context()->mutable_system_root_certs(); + transport_socket->mutable_typed_config()->PackFrom(downstream_tls_context); + auto* address = listener.mutable_address()->mutable_socket_address(); + address->set_address("127.0.0.1"); + address->set_port_value(443); + std::string serialized_resource; + ASSERT_TRUE(listener.SerializeToString(&serialized_resource)); + auto* resource_type = XdsListenerResourceType::Get(); + auto decode_result = + resource_type->Decode(decode_context_, serialized_resource); + EXPECT_EQ(decode_result.resource.status().code(), + absl::StatusCode::kInvalidArgument); + EXPECT_EQ(decode_result.resource.status().message(), + "errors validating server Listener: [" + "field:default_filter_chain.transport_socket.typed_config.value[" + "envoy.extensions.transport_sockets.tls.v3.DownstreamTlsContext]" + ".common_tls_context " + "error:system_root_certs not supported]") + << decode_result.resource.status(); } TEST_F(TcpListenerTest, ClientCertificateRequired) { @@ -1620,11 +1666,13 @@ TEST_F(TcpListenerTest, ClientCertificateRequired) { tls_context.common_tls_context.tls_certificate_provider_instance; EXPECT_EQ(cert_provider_instance.instance_name, "provider1"); EXPECT_EQ(cert_provider_instance.certificate_name, "cert_name"); - auto& ca_cert_provider_instance = - tls_context.common_tls_context.certificate_validation_context - .ca_certificate_provider_instance; - EXPECT_EQ(ca_cert_provider_instance.instance_name, "provider1"); - EXPECT_EQ(ca_cert_provider_instance.certificate_name, "ca_cert_name"); + auto* ca_cert_provider = + absl::get_if( + &tls_context.common_tls_context.certificate_validation_context + .ca_certs); + ASSERT_NE(ca_cert_provider, nullptr); + EXPECT_EQ(ca_cert_provider->instance_name, "provider1"); + EXPECT_EQ(ca_cert_provider->certificate_name, "ca_cert_name"); } // This is just one example of where CommonTlsContext::Parse() will diff --git a/test/cpp/end2end/xds/xds_end2end_test.cc b/test/cpp/end2end/xds/xds_end2end_test.cc index 1de043df574..0c6cfab5e84 100644 --- a/test/cpp/end2end/xds/xds_end2end_test.cc +++ b/test/cpp/end2end/xds/xds_end2end_test.cc @@ -488,6 +488,24 @@ TEST_P(XdsSecurityTest, CheckRpcSendOk(DEBUG_LOCATION, 1, RpcOptions().set_timeout_ms(5000)); } +TEST_P(XdsSecurityTest, UseSystemRootCerts) { + grpc_core::testing::ScopedExperimentalEnvVar env1( + "GRPC_EXPERIMENTAL_XDS_SYSTEM_ROOT_CERTS"); + grpc_core::testing::ScopedEnvVar env2("GRPC_DEFAULT_SSL_ROOTS_FILE_PATH", + kCaCertPath); + g_fake1_cert_data_map->Set({{"", {root_cert_, identity_pair_}}}); + auto cluster = default_cluster_; + auto* transport_socket = cluster.mutable_transport_socket(); + transport_socket->set_name("envoy.transport_sockets.tls"); + UpstreamTlsContext upstream_tls_context; + upstream_tls_context.mutable_common_tls_context() + ->mutable_validation_context() + ->mutable_system_root_certs(); + transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context); + balancer_->ads_service()->SetCdsResource(cluster); + CheckRpcSendOk(DEBUG_LOCATION, 1, RpcOptions().set_timeout_ms(5000)); +} + TEST_P(XdsSecurityTest, TestMtlsConfigurationWithNoSanMatchers) { g_fake1_cert_data_map->Set({{"", {root_cert_, identity_pair_}}}); UpdateAndVerifyXdsSecurityConfiguration("fake_plugin1", "", "fake_plugin1",