Chiebot-Mirror
/
grpc
8
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

NACK xDS updates when certificate provider instace names are unrecognized (#26614)

Browse Source 
* NACK xDS updates when certificate provider instace names are unrecognized

* Reviewer comments

* Reviewer comments

* Clang format

* Fix compilation error
pull/26659/head
Yash Tibrewal 4 years ago committed by GitHub
parent 0bd70a7e3e
commit c52005c161
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 104 additions and 40 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 2
      src/core/ext/xds/certificate_provider_store.h
  2. 90
      src/core/ext/xds/xds_api.cc
  3. 5
      src/core/ext/xds/xds_api.h
  4. 3
      src/core/ext/xds/xds_client.cc
  5. 44
      test/cpp/end2end/xds_end2end_test.cc

2
src/core/ext/xds/certificate_provider_store.h
Unescape View File

@ -101,7 +101,7 @@ class CertificateProviderStore
Mutex mu_;
// Map of plugin configurations
PluginDefinitionMap plugin_config_map_ ABSL_GUARDED_BY(mu_);
const PluginDefinitionMap plugin_config_map_;
// Underlying map for the providers.
std::map<absl::string_view, CertificateProviderWrapper*>
certificate_providers_map_ ABSL_GUARDED_BY(mu_);

90
src/core/ext/xds/xds_api.cc
Unescape View File

@ -868,10 +868,13 @@ bool IsEds(absl::string_view type_url) {
#endif
XdsApi::XdsApi(XdsClient* client, TraceFlag* tracer,
const XdsBootstrap::Node* node)
const XdsBootstrap::Node* node,
const CertificateProviderStore::PluginDefinitionMap*
certificate_provider_definition_map)
: client_(client),
tracer_(tracer),
node_(node),
certificate_provider_definition_map_(certificate_provider_definition_map),
build_version_(absl::StrCat("gRPC C-core ", GPR_PLATFORM_STRING, " ",
grpc_version_string(),
GRPC_XDS_USER_AGENT_NAME_SUFFIX_STRING,
@ -903,11 +906,13 @@ XdsApi::XdsApi(XdsClient* client, TraceFlag* tracer,
namespace {
struct EncodingContext {
XdsClient* client;
XdsClient* client; // Used only for logging. Unsafe for dereferencing.
TraceFlag* tracer;
upb_symtab* symtab;
upb_arena* arena;
bool use_v3;
const CertificateProviderStore::PluginDefinitionMap*
certificate_provider_definition_map;
};
// Works for both std::string and absl::string_view.
@ -1116,8 +1121,12 @@ grpc_slice XdsApi::CreateAdsRequest(
const std::string& version, const std::string& nonce,
grpc_error_handle error, bool populate_node) {
upb::Arena arena;
const EncodingContext context = {client_, tracer_, symtab_.ptr(), arena.ptr(),
server.ShouldUseV3()};
const EncodingContext context = {client_,
tracer_,
symtab_.ptr(),
arena.ptr(),
server.ShouldUseV3(),
certificate_provider_definition_map_};
// Create a request.
envoy_service_discovery_v3_DiscoveryRequest* request =
envoy_service_discovery_v3_DiscoveryRequest_new(arena.ptr());
@ -1803,24 +1812,32 @@ grpc_error_handle RouteConfigParse(
return GRPC_ERROR_NONE;
}
XdsApi::CommonTlsContext::CertificateProviderInstance
CertificateProviderInstanceParse(
grpc_error_handle CertificateProviderInstanceParse(
const EncodingContext& context,
const envoy_extensions_transport_sockets_tls_v3_CommonTlsContext_CertificateProviderInstance*
certificate_provider_instance_proto) {
return {
certificate_provider_instance_proto,
XdsApi::CommonTlsContext::CertificateProviderInstance*
certificate_provider_instance) {
*certificate_provider_instance = {
UpbStringToStdString(
envoy_extensions_transport_sockets_tls_v3_CommonTlsContext_CertificateProviderInstance_instance_name(
certificate_provider_instance_proto)),
UpbStringToStdString(
envoy_extensions_transport_sockets_tls_v3_CommonTlsContext_CertificateProviderInstance_certificate_name(
certificate_provider_instance_proto))};
if (context.certificate_provider_definition_map->find(
certificate_provider_instance->instance_name) ==
context.certificate_provider_definition_map->end()) {
return GRPC_ERROR_CREATE_FROM_COPIED_STRING(
absl::StrCat("Unrecognized certificate provider instance name: ",
certificate_provider_instance->instance_name)
.c_str());
}
return GRPC_ERROR_NONE;
}
grpc_error_handle CommonTlsContextParse(
const envoy_extensions_transport_sockets_tls_v3_CommonTlsContext*
common_tls_context_proto,
XdsApi::CommonTlsContext* common_tls_context) GRPC_MUST_USE_RESULT;
grpc_error_handle CommonTlsContextParse(
const EncodingContext& context,
const envoy_extensions_transport_sockets_tls_v3_CommonTlsContext*
common_tls_context_proto,
XdsApi::CommonTlsContext* common_tls_context) {
@ -1898,19 +1915,21 @@ grpc_error_handle CommonTlsContextParse(
envoy_extensions_transport_sockets_tls_v3_CommonTlsContext_CombinedCertificateValidationContext_validation_context_certificate_provider_instance(
combined_validation_context);
if (validation_context_certificate_provider_instance != nullptr) {
common_tls_context->combined_validation_context
.validation_context_certificate_provider_instance =
CertificateProviderInstanceParse(
validation_context_certificate_provider_instance);
grpc_error_handle error = CertificateProviderInstanceParse(
context, validation_context_certificate_provider_instance,
&common_tls_context->combined_validation_context
.validation_context_certificate_provider_instance);
if (error != GRPC_ERROR_NONE) return error;
}
}
auto* tls_certificate_certificate_provider_instance =
envoy_extensions_transport_sockets_tls_v3_CommonTlsContext_tls_certificate_certificate_provider_instance(
common_tls_context_proto);
if (tls_certificate_certificate_provider_instance != nullptr) {
common_tls_context->tls_certificate_certificate_provider_instance =
CertificateProviderInstanceParse(
tls_certificate_certificate_provider_instance);
grpc_error_handle error = CertificateProviderInstanceParse(
context, tls_certificate_certificate_provider_instance,
&common_tls_context->tls_certificate_certificate_provider_instance);
if (error != GRPC_ERROR_NONE) return error;
}
return GRPC_ERROR_NONE;
}
@ -2100,8 +2119,9 @@ grpc_error_handle DownstreamTlsContextParse(
envoy_extensions_transport_sockets_tls_v3_DownstreamTlsContext_common_tls_context(
downstream_tls_context_proto);
if (common_tls_context != nullptr) {
grpc_error_handle error = CommonTlsContextParse(
common_tls_context, &downstream_tls_context->common_tls_context);
grpc_error_handle error =
CommonTlsContextParse(context, common_tls_context,
&downstream_tls_context->common_tls_context);
if (error != GRPC_ERROR_NONE) return error;
}
auto* require_client_certificate =
@ -3058,7 +3078,7 @@ grpc_error_handle CdsResponseParse(
upstream_tls_context);
if (common_tls_context != nullptr) {
grpc_error_handle error = CommonTlsContextParse(
common_tls_context, &cds_update.common_tls_context);
context, common_tls_context, &cds_update.common_tls_context);
if (error != GRPC_ERROR_NONE) {
errors.push_back(grpc_error_add_child(
GRPC_ERROR_CREATE_FROM_COPIED_STRING(
@ -3416,8 +3436,12 @@ XdsApi::AdsParseResult XdsApi::ParseAdsResponse(
const std::set<absl::string_view>& expected_eds_service_names) {
AdsParseResult result;
upb::Arena arena;
const EncodingContext context = {client_, tracer_, symtab_.ptr(), arena.ptr(),
server.ShouldUseV3()};
const EncodingContext context = {client_,
tracer_,
symtab_.ptr(),
arena.ptr(),
server.ShouldUseV3(),
certificate_provider_definition_map_};
// Decode the response.
const envoy_service_discovery_v3_DiscoveryResponse* response =
envoy_service_discovery_v3_DiscoveryResponse_parse(
@ -3504,8 +3528,12 @@ grpc_slice SerializeLrsRequest(
grpc_slice XdsApi::CreateLrsInitialRequest(
const XdsBootstrap::XdsServer& server) {
upb::Arena arena;
const EncodingContext context = {client_, tracer_, symtab_.ptr(), arena.ptr(),
server.ShouldUseV3()};
const EncodingContext context = {client_,
tracer_,
symtab_.ptr(),
arena.ptr(),
server.ShouldUseV3(),
certificate_provider_definition_map_};
// Create a request.
envoy_service_load_stats_v3_LoadStatsRequest* request =
envoy_service_load_stats_v3_LoadStatsRequest_new(arena.ptr());
@ -3575,8 +3603,9 @@ void LocalityStatsPopulate(
grpc_slice XdsApi::CreateLrsRequest(
ClusterLoadReportMap cluster_load_report_map) {
upb::Arena arena;
const EncodingContext context = {client_, tracer_, symtab_.ptr(), arena.ptr(),
false};
const EncodingContext context = {
client_, tracer_, symtab_.ptr(),
arena.ptr(), false, certificate_provider_definition_map_};
// Create a request.
envoy_service_load_stats_v3_LoadStatsRequest* request =
envoy_service_load_stats_v3_LoadStatsRequest_new(arena.ptr());
@ -3909,8 +3938,9 @@ std::string XdsApi::AssembleClientConfig(
// Fill-in the node information
auto* node = envoy_service_status_v3_ClientConfig_mutable_node(client_config,
arena.ptr());
const EncodingContext context = {client_, tracer_, symtab_.ptr(), arena.ptr(),
true};
const EncodingContext context = {
client_, tracer_, symtab_.ptr(),
arena.ptr(), true, certificate_provider_definition_map_};
PopulateNode(context, node_, build_version_, user_agent_name_,
user_agent_version_, node);
// Dump each xDS-type config into PerXdsConfig

5
src/core/ext/xds/xds_api.h
Unescape View File

@ -628,7 +628,8 @@ class XdsApi {
std::set<std::string> resource_names_failed;
};
XdsApi(XdsClient* client, TraceFlag* tracer, const XdsBootstrap::Node* node);
XdsApi(XdsClient* client, TraceFlag* tracer, const XdsBootstrap::Node* node,
const CertificateProviderStore::PluginDefinitionMap* map);
// Creates an ADS request.
// Takes ownership of \a error.
@ -669,6 +670,8 @@ class XdsApi {
XdsClient* client_;
TraceFlag* tracer_;
const XdsBootstrap::Node* node_; // Do not own.
const CertificateProviderStore::PluginDefinitionMap*
certificate_provider_definition_map_; // Do not own.
upb::SymbolTable symtab_;
const std::string build_version_;
const std::string user_agent_name_;

3
src/core/ext/xds/xds_client.cc
Unescape View File

@ -1803,7 +1803,8 @@ XdsClient::XdsClient(std::unique_ptr<XdsBootstrap> bootstrap,
interested_parties_(grpc_pollset_set_create()),
certificate_provider_store_(MakeOrphanable<CertificateProviderStore>(
bootstrap_->certificate_providers())),
api_(this, &grpc_xds_client_trace, bootstrap_->node()) {
api_(this, &grpc_xds_client_trace, bootstrap_->node(),
&bootstrap_->certificate_providers()) {
if (GRPC_TRACE_FLAG_ENABLED(grpc_xds_client_trace)) {
gpr_log(GPR_INFO, "[xds_client %p] creating xds client", this);
}

44
test/cpp/end2end/xds_end2end_test.cc
Unescape View File

@ -8029,7 +8029,7 @@ TEST_P(
UpstreamTlsContext upstream_tls_context;
upstream_tls_context.mutable_common_tls_context()
->mutable_tls_certificate_certificate_provider_instance()
->set_instance_name(std::string("instance_name"));
->set_instance_name(std::string("fake_plugin1"));
transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context);
balancers_[0]->ads_service()->SetCdsResource(cluster);
CheckRpcSendFailure();
@ -8082,7 +8082,13 @@ TEST_P(XdsSecurityTest, UnknownRootCertificateProvider) {
->set_instance_name("unknown");
transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context);
balancers_[0]->ads_service()->SetCdsResource(cluster);
CheckRpcSendFailure(1, RpcOptions(), StatusCode::UNAVAILABLE);
CheckRpcSendFailure();
const auto response_state =
balancers_[0]->ads_service()->cds_response_state();
EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED);
EXPECT_THAT(response_state.error_message,
::testing::HasSubstr(
"Unrecognized certificate provider instance name: unknown"));
}
TEST_P(XdsSecurityTest, UnknownIdentityCertificateProvider) {
@ -8102,7 +8108,13 @@ TEST_P(XdsSecurityTest, UnknownIdentityCertificateProvider) {
->set_instance_name("fake_plugin1");
transport_socket->mutable_typed_config()->PackFrom(upstream_tls_context);
balancers_[0]->ads_service()->SetCdsResource(cluster);
CheckRpcSendFailure(1, RpcOptions(), StatusCode::UNAVAILABLE);
CheckRpcSendFailure();
const auto response_state =
balancers_[0]->ads_service()->cds_response_state();
EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED);
EXPECT_THAT(response_state.error_message,
::testing::HasSubstr(
"Unrecognized certificate provider instance name: unknown"));
g_fake1_cert_data_map = nullptr;
}
@ -9028,14 +9040,32 @@ TEST_P(XdsServerSecurityTest, UnknownIdentityCertificateProvider) {
SetLdsUpdate("", "", "unknown", "", false);
SendRpc([this]() { return CreateTlsChannel(); }, {}, {},
true /* test_expects_failure */);
do {
CheckRpcSendFailure();
} while (balancers_[0]->ads_service()->lds_response_state().state ==
AdsServiceImpl::ResponseState::SENT);
const auto response_state =
balancers_[0]->ads_service()->lds_response_state();
EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED);
EXPECT_THAT(response_state.error_message,
::testing::HasSubstr(
"Unrecognized certificate provider instance name: unknown"));
}
TEST_P(XdsServerSecurityTest, UnknownRootCertificateProvider) {
FakeCertificateProvider::CertDataMap fake1_cert_map = {
{"", {root_cert_, identity_pair_}}};
SetLdsUpdate("unknown", "", "fake_plugin1", "", false);
SendRpc([this]() { return CreateTlsChannel(); }, {}, {},
true /* test_expects_failure */);
do {
CheckRpcSendFailure();
} while (balancers_[0]->ads_service()->lds_response_state().state ==
AdsServiceImpl::ResponseState::SENT);
const auto response_state =
balancers_[0]->ads_service()->lds_response_state();
EXPECT_EQ(response_state.state, AdsServiceImpl::ResponseState::NACKED);
EXPECT_THAT(response_state.error_message,
::testing::HasSubstr(
"Unrecognized certificate provider instance name: unknown"));
}
TEST_P(XdsServerSecurityTest, CertificatesNotAvailable) {
@ -9767,7 +9797,7 @@ TEST_P(XdsServerFilterChainMatchTest,
for (int i = 1; i < 65536; i++) {
filter_chain->mutable_filter_chain_match()->add_source_ports(i);
}
// Add another filter chain with no source prefix range mentioned with a bad
// Add another filter chain with no source port mentioned with a bad
// DownstreamTlsContext configuration.
filter_chain = listener.add_filter_chains();
filter_chain->add_filters()->mutable_typed_config()->PackFrom(
@ -9777,7 +9807,7 @@ TEST_P(XdsServerFilterChainMatchTest,
DownstreamTlsContext downstream_tls_context;
downstream_tls_context.mutable_common_tls_context()
->mutable_tls_certificate_certificate_provider_instance()
->set_instance_name("unknown");
->set_instance_name("fake_plugin1");
transport_socket->mutable_typed_config()->PackFrom(downstream_tls_context);
balancers_[0]->ads_service()->SetLdsResource(listener);
// A successful RPC proves that the filter chain with matching source port

Write Preview
Loading…
Cancel
Save