Chiebot-Mirror
/
grpc
mirror of https://github.com/grpc/grpc.git
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

xDS: Remove environmental variable guard for security (#27290)

Browse Source
pull/27300/head
Yash Tibrewal 3 years ago committed by GitHub
parent 7fd731f704
commit 362aff3458
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 88 additions and 201 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. 51
      src/core/ext/xds/xds_api.cc
  2. 5
      src/core/ext/xds/xds_api.h
  3. 18
      src/core/ext/xds/xds_bootstrap.cc
  4. 195
      test/core/xds/xds_bootstrap_test.cc
  5. 20
      test/cpp/end2end/xds_end2end_test.cc

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

@ -116,17 +116,6 @@ bool XdsAggregateAndLogicalDnsClusterEnabled() {
return parse_succeeded && parsed_value;
}
// TODO(yashykt): Check to see if xDS security is enabled. This will be
// removed once this feature is fully integration-tested and enabled by
// default.
bool XdsSecurityEnabled() {
char* value = gpr_getenv("GRPC_XDS_EXPERIMENTAL_SECURITY_SUPPORT");
bool parsed_value;
bool parse_succeeded = gpr_parse_bool_value(value, &parsed_value);
gpr_free(value);
return parse_succeeded && parsed_value;
}
//
// XdsApi::Route::HashPolicy
//
@ -2583,17 +2572,13 @@ grpc_error_handle FilterChainParse(
}
}
}
// Get the DownstreamTlsContext for the filter chain
if (XdsSecurityEnabled()) {
auto* transport_socket =
envoy_config_listener_v3_FilterChain_transport_socket(
filter_chain_proto);
if (transport_socket != nullptr) {
grpc_error_handle error = DownstreamTlsContextParse(
context, transport_socket,
&filter_chain->filter_chain_data->downstream_tls_context);
if (error != GRPC_ERROR_NONE) errors.push_back(error);
}
auto* transport_socket =
envoy_config_listener_v3_FilterChain_transport_socket(filter_chain_proto);
if (transport_socket != nullptr) {
grpc_error_handle error = DownstreamTlsContextParse(
context, transport_socket,
&filter_chain->filter_chain_data->downstream_tls_context);
if (error != GRPC_ERROR_NONE) errors.push_back(error);
}
return GRPC_ERROR_CREATE_FROM_VECTOR("Error parsing FilterChain", &errors);
}
@ -3138,18 +3123,16 @@ grpc_error_handle CdsResourceParse(
errors.push_back(
GRPC_ERROR_CREATE_FROM_STATIC_STRING("LB policy is not supported."));
}
if (XdsSecurityEnabled()) {
auto* transport_socket =
envoy_config_cluster_v3_Cluster_transport_socket(cluster);
if (transport_socket != nullptr) {
grpc_error_handle error = UpstreamTlsContextParse(
context, transport_socket, &cds_update->common_tls_context);
if (error != GRPC_ERROR_NONE) {
errors.push_back(
grpc_error_add_child(GRPC_ERROR_CREATE_FROM_STATIC_STRING(
"Error parsing security configuration"),
error));
}
auto* transport_socket =
envoy_config_cluster_v3_Cluster_transport_socket(cluster);
if (transport_socket != nullptr) {
grpc_error_handle error = UpstreamTlsContextParse(
context, transport_socket, &cds_update->common_tls_context);
if (error != GRPC_ERROR_NONE) {
errors.push_back(
grpc_error_add_child(GRPC_ERROR_CREATE_FROM_STATIC_STRING(
"Error parsing security configuration"),
error));
}
}
// Record LRS server name (if any).

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

@ -42,11 +42,6 @@
namespace grpc_core {
// TODO(yashykt): Check to see if xDS security is enabled. This will be
// removed once this feature is fully integration-tested and enabled by
// default.
bool XdsSecurityEnabled();
class XdsClient;
class XdsApi {

18
src/core/ext/xds/xds_bootstrap.cc
Unescape View File

@ -132,16 +132,14 @@ XdsBootstrap::XdsBootstrap(Json json, grpc_error_handle* error) {
std::move(*it->second.mutable_string_value());
}
}
if (XdsSecurityEnabled()) {
it = json.mutable_object()->find("certificate_providers");
if (it != json.mutable_object()->end()) {
if (it->second.type() != Json::Type::OBJECT) {
error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING(
"\"certificate_providers\" field is not an object"));
} else {
grpc_error_handle parse_error = ParseCertificateProviders(&it->second);
if (parse_error != GRPC_ERROR_NONE) error_list.push_back(parse_error);
}
it = json.mutable_object()->find("certificate_providers");
if (it != json.mutable_object()->end()) {
if (it->second.type() != Json::Type::OBJECT) {
error_list.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING(
"\"certificate_providers\" field is not an object"));
} else {
grpc_error_handle parse_error = ParseCertificateProviders(&it->second);
if (parse_error != GRPC_ERROR_NONE) error_list.push_back(parse_error);
}
}
*error = GRPC_ERROR_CREATE_FROM_VECTOR("errors parsing xds bootstrap file",

195
test/core/xds/xds_bootstrap_test.cc
Unescape View File

@ -34,41 +34,9 @@
namespace grpc_core {
namespace testing {
namespace {
class TestType {
public:
explicit TestType(bool parse_xds_certificate_providers)
: parse_xds_certificate_providers_(parse_xds_certificate_providers) {}
bool parse_xds_certificate_providers() const {
return parse_xds_certificate_providers_;
}
std::string AsString() const {
return parse_xds_certificate_providers_
? "WithCertificateProvidersParsing"
: "WithoutCertificateProvidersParsing";
}
private:
const bool parse_xds_certificate_providers_;
};
class XdsBootstrapTest : public ::testing::TestWithParam<TestType> {
public:
XdsBootstrapTest() {
if (GetParam().parse_xds_certificate_providers()) {
gpr_setenv("GRPC_XDS_EXPERIMENTAL_SECURITY_SUPPORT", "true");
} else {
gpr_unsetenv("GRPC_XDS_EXPERIMENTAL_SECURITY_SUPPORT");
}
grpc_init();
}
~XdsBootstrapTest() override { grpc_shutdown_blocking(); }
};
TEST_P(XdsBootstrapTest, Basic) {
TEST(XdsBootstrapTest, Basic) {
const char* json_str =
"{"
" \"xds_servers\": ["
@ -146,7 +114,7 @@ TEST_P(XdsBootstrapTest, Basic) {
"example/resource");
}
TEST_P(XdsBootstrapTest, ValidWithoutNode) {
TEST(XdsBootstrapTest, ValidWithoutNode) {
const char* json_str =
"{"
" \"xds_servers\": ["
@ -166,7 +134,7 @@ TEST_P(XdsBootstrapTest, ValidWithoutNode) {
EXPECT_EQ(bootstrap.node(), nullptr);
}
TEST_P(XdsBootstrapTest, InsecureCreds) {
TEST(XdsBootstrapTest, InsecureCreds) {
const char* json_str =
"{"
" \"xds_servers\": ["
@ -186,7 +154,7 @@ TEST_P(XdsBootstrapTest, InsecureCreds) {
EXPECT_EQ(bootstrap.node(), nullptr);
}
TEST_P(XdsBootstrapTest, GoogleDefaultCreds) {
TEST(XdsBootstrapTest, GoogleDefaultCreds) {
// Generate call creds file needed by GoogleDefaultCreds.
const char token_str[] =
"{ \"client_id\": \"32555999999.apps.googleusercontent.com\","
@ -222,7 +190,7 @@ TEST_P(XdsBootstrapTest, GoogleDefaultCreds) {
EXPECT_EQ(bootstrap.node(), nullptr);
}
TEST_P(XdsBootstrapTest, MissingChannelCreds) {
TEST(XdsBootstrapTest, MissingChannelCreds) {
const char* json_str =
"{"
" \"xds_servers\": ["
@ -240,7 +208,7 @@ TEST_P(XdsBootstrapTest, MissingChannelCreds) {
GRPC_ERROR_UNREF(error);
}
TEST_P(XdsBootstrapTest, NoKnownChannelCreds) {
TEST(XdsBootstrapTest, NoKnownChannelCreds) {
const char* json_str =
"{"
" \"xds_servers\": ["
@ -260,7 +228,7 @@ TEST_P(XdsBootstrapTest, NoKnownChannelCreds) {
GRPC_ERROR_UNREF(error);
}
TEST_P(XdsBootstrapTest, MissingXdsServers) {
TEST(XdsBootstrapTest, MissingXdsServers) {
grpc_error_handle error = GRPC_ERROR_NONE;
Json json = Json::Parse("{}", &error);
ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error);
@ -270,7 +238,7 @@ TEST_P(XdsBootstrapTest, MissingXdsServers) {
GRPC_ERROR_UNREF(error);
}
TEST_P(XdsBootstrapTest, TopFieldsWrongTypes) {
TEST(XdsBootstrapTest, TopFieldsWrongTypes) {
const char* json_str =
"{"
" \"xds_servers\":1,"
@ -287,15 +255,13 @@ TEST_P(XdsBootstrapTest, TopFieldsWrongTypes) {
"\"node\" field is not an object.*"
"\"server_listener_resource_name_"
"template\" field is not a string.*"));
if (GetParam().parse_xds_certificate_providers()) {
EXPECT_THAT(grpc_error_std_string(error),
::testing::ContainsRegex(
"\"certificate_providers\" field is not an object"));
}
EXPECT_THAT(grpc_error_std_string(error),
::testing::ContainsRegex(
"\"certificate_providers\" field is not an object"));
GRPC_ERROR_UNREF(error);
}
TEST_P(XdsBootstrapTest, XdsServerMissingServerUri) {
TEST(XdsBootstrapTest, XdsServerMissingServerUri) {
const char* json_str =
"{"
" \"xds_servers\":[{}]"
@ -311,7 +277,7 @@ TEST_P(XdsBootstrapTest, XdsServerMissingServerUri) {
GRPC_ERROR_UNREF(error);
}
TEST_P(XdsBootstrapTest, XdsServerUriAndCredsWrongTypes) {
TEST(XdsBootstrapTest, XdsServerUriAndCredsWrongTypes) {
const char* json_str =
"{"
" \"xds_servers\":["
@ -334,7 +300,7 @@ TEST_P(XdsBootstrapTest, XdsServerUriAndCredsWrongTypes) {
GRPC_ERROR_UNREF(error);
}
TEST_P(XdsBootstrapTest, ChannelCredsFieldsWrongTypes) {
TEST(XdsBootstrapTest, ChannelCredsFieldsWrongTypes) {
const char* json_str =
"{"
" \"xds_servers\":["
@ -364,7 +330,7 @@ TEST_P(XdsBootstrapTest, ChannelCredsFieldsWrongTypes) {
GRPC_ERROR_UNREF(error);
}
TEST_P(XdsBootstrapTest, NodeFieldsWrongTypes) {
TEST(XdsBootstrapTest, NodeFieldsWrongTypes) {
const char* json_str =
"{"
" \"node\":{"
@ -387,7 +353,7 @@ TEST_P(XdsBootstrapTest, NodeFieldsWrongTypes) {
GRPC_ERROR_UNREF(error);
}
TEST_P(XdsBootstrapTest, LocalityFieldsWrongType) {
TEST(XdsBootstrapTest, LocalityFieldsWrongType) {
const char* json_str =
"{"
" \"node\":{"
@ -411,7 +377,7 @@ TEST_P(XdsBootstrapTest, LocalityFieldsWrongType) {
GRPC_ERROR_UNREF(error);
}
TEST_P(XdsBootstrapTest, CertificateProvidersElementWrongType) {
TEST(XdsBootstrapTest, CertificateProvidersElementWrongType) {
const char* json_str =
"{"
" \"xds_servers\": ["
@ -428,18 +394,14 @@ TEST_P(XdsBootstrapTest, CertificateProvidersElementWrongType) {
Json json = Json::Parse(json_str, &error);
ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error);
XdsBootstrap bootstrap(std::move(json), &error);
if (GetParam().parse_xds_certificate_providers()) {
EXPECT_THAT(grpc_error_std_string(error),
::testing::ContainsRegex(
"errors parsing \"certificate_providers\" object.*"
"element \"plugin\" is not an object"));
} else {
EXPECT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error);
}
EXPECT_THAT(grpc_error_std_string(error),
::testing::ContainsRegex(
"errors parsing \"certificate_providers\" object.*"
"element \"plugin\" is not an object"));
GRPC_ERROR_UNREF(error);
}
TEST_P(XdsBootstrapTest, CertificateProvidersPluginNameWrongType) {
TEST(XdsBootstrapTest, CertificateProvidersPluginNameWrongType) {
const char* json_str =
"{"
" \"xds_servers\": ["
@ -458,19 +420,15 @@ TEST_P(XdsBootstrapTest, CertificateProvidersPluginNameWrongType) {
Json json = Json::Parse(json_str, &error);
ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error);
XdsBootstrap bootstrap(std::move(json), &error);
if (GetParam().parse_xds_certificate_providers()) {
EXPECT_THAT(grpc_error_std_string(error),
::testing::ContainsRegex(
"errors parsing \"certificate_providers\" object.*"
"errors parsing element \"plugin\".*"
"\"plugin_name\" field is not a string"));
} else {
EXPECT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error);
}
EXPECT_THAT(grpc_error_std_string(error),
::testing::ContainsRegex(
"errors parsing \"certificate_providers\" object.*"
"errors parsing element \"plugin\".*"
"\"plugin_name\" field is not a string"));
GRPC_ERROR_UNREF(error);
}
TEST_P(XdsBootstrapTest, CertificateProvidersUnrecognizedPluginName) {
TEST(XdsBootstrapTest, CertificateProvidersUnrecognizedPluginName) {
const char* json_str =
"{"
" \"xds_servers\": ["
@ -489,15 +447,11 @@ TEST_P(XdsBootstrapTest, CertificateProvidersUnrecognizedPluginName) {
Json json = Json::Parse(json_str, &error);
ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error);
XdsBootstrap bootstrap(std::move(json), &error);
if (GetParam().parse_xds_certificate_providers()) {
EXPECT_THAT(grpc_error_std_string(error),
::testing::ContainsRegex(
"errors parsing \"certificate_providers\" object.*"
"errors parsing element \"plugin\".*"
"Unrecognized plugin name: unknown"));
} else {
EXPECT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error);
}
EXPECT_THAT(grpc_error_std_string(error),
::testing::ContainsRegex(
"errors parsing \"certificate_providers\" object.*"
"errors parsing element \"plugin\".*"
"Unrecognized plugin name: unknown"));
GRPC_ERROR_UNREF(error);
}
@ -550,9 +504,7 @@ class FakeCertificateProviderFactory : public CertificateProviderFactory {
}
};
TEST_P(XdsBootstrapTest, CertificateProvidersFakePluginParsingError) {
CertificateProviderRegistry::RegisterCertificateProviderFactory(
absl::make_unique<FakeCertificateProviderFactory>());
TEST(XdsBootstrapTest, CertificateProvidersFakePluginParsingError) {
const char* json_str =
"{"
" \"xds_servers\": ["
@ -574,21 +526,15 @@ TEST_P(XdsBootstrapTest, CertificateProvidersFakePluginParsingError) {
Json json = Json::Parse(json_str, &error);
ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error);
XdsBootstrap bootstrap(std::move(json), &error);
if (GetParam().parse_xds_certificate_providers()) {
EXPECT_THAT(grpc_error_std_string(error),
::testing::ContainsRegex(
"errors parsing \"certificate_providers\" object.*"
"errors parsing element \"fake_plugin\".*"
"field:config field:value not of type number"));
} else {
EXPECT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error);
}
EXPECT_THAT(grpc_error_std_string(error),
::testing::ContainsRegex(
"errors parsing \"certificate_providers\" object.*"
"errors parsing element \"fake_plugin\".*"
"field:config field:value not of type number"));
GRPC_ERROR_UNREF(error);
}
TEST_P(XdsBootstrapTest, CertificateProvidersFakePluginParsingSuccess) {
CertificateProviderRegistry::RegisterCertificateProviderFactory(
absl::make_unique<FakeCertificateProviderFactory>());
TEST(XdsBootstrapTest, CertificateProvidersFakePluginParsingSuccess) {
const char* json_str =
"{"
" \"xds_servers\": ["
@ -611,24 +557,17 @@ TEST_P(XdsBootstrapTest, CertificateProvidersFakePluginParsingSuccess) {
ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error);
XdsBootstrap bootstrap(std::move(json), &error);
ASSERT_TRUE(error == GRPC_ERROR_NONE) << grpc_error_std_string(error);
if (GetParam().parse_xds_certificate_providers()) {
const CertificateProviderStore::PluginDefinition& fake_plugin =
bootstrap.certificate_providers().at("fake_plugin");
ASSERT_EQ(fake_plugin.plugin_name, "fake");
ASSERT_STREQ(fake_plugin.config->name(), "fake");
ASSERT_EQ(
static_cast<RefCountedPtr<FakeCertificateProviderFactory::Config>>(
fake_plugin.config)
->value(),
10);
} else {
EXPECT_TRUE(bootstrap.certificate_providers().empty());
}
const CertificateProviderStore::PluginDefinition& fake_plugin =
bootstrap.certificate_providers().at("fake_plugin");
ASSERT_EQ(fake_plugin.plugin_name, "fake");
ASSERT_STREQ(fake_plugin.config->name(), "fake");
ASSERT_EQ(static_cast<RefCountedPtr<FakeCertificateProviderFactory::Config>>(
fake_plugin.config)
->value(),
10);
}
TEST_P(XdsBootstrapTest, CertificateProvidersFakePluginEmptyConfig) {
CertificateProviderRegistry::RegisterCertificateProviderFactory(
absl::make_unique<FakeCertificateProviderFactory>());
TEST(XdsBootstrapTest, CertificateProvidersFakePluginEmptyConfig) {
const char* json_str =
"{"
" \"xds_servers\": ["
@ -648,35 +587,27 @@ TEST_P(XdsBootstrapTest, CertificateProvidersFakePluginEmptyConfig) {
ASSERT_EQ(error, GRPC_ERROR_NONE) << grpc_error_std_string(error);
XdsBootstrap bootstrap(std::move(json), &error);
ASSERT_TRUE(error == GRPC_ERROR_NONE) << grpc_error_std_string(error);
if (GetParam().parse_xds_certificate_providers()) {
const CertificateProviderStore::PluginDefinition& fake_plugin =
bootstrap.certificate_providers().at("fake_plugin");
ASSERT_EQ(fake_plugin.plugin_name, "fake");
ASSERT_STREQ(fake_plugin.config->name(), "fake");
ASSERT_EQ(
static_cast<RefCountedPtr<FakeCertificateProviderFactory::Config>>(
fake_plugin.config)
->value(),
0);
} else {
EXPECT_TRUE(bootstrap.certificate_providers().empty());
}
}
std::string TestTypeName(const ::testing::TestParamInfo<TestType>& info) {
return info.param.AsString();
const CertificateProviderStore::PluginDefinition& fake_plugin =
bootstrap.certificate_providers().at("fake_plugin");
ASSERT_EQ(fake_plugin.plugin_name, "fake");
ASSERT_STREQ(fake_plugin.config->name(), "fake");
ASSERT_EQ(static_cast<RefCountedPtr<FakeCertificateProviderFactory::Config>>(
fake_plugin.config)
->value(),
0);
}
INSTANTIATE_TEST_SUITE_P(XdsBootstrap, XdsBootstrapTest,
::testing::Values(TestType(false), TestType(true)),
&TestTypeName);
} // namespace
} // namespace testing
} // namespace grpc_core
int main(int argc, char** argv) {
::testing::InitGoogleTest(&argc, argv);
grpc::testing::TestEnvironment env(argc, argv);
grpc_init();
grpc_core::CertificateProviderRegistry::RegisterCertificateProviderFactory(
absl::make_unique<grpc_core::testing::FakeCertificateProviderFactory>());
int ret = RUN_ALL_TESTS();
grpc_shutdown();
return ret;
}

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

@ -8190,16 +8190,6 @@ TEST_P(CdsTest, RingHashPolicyHasInvalidRingSizeMinGreaterThanMax) {
class XdsSecurityTest : public BasicTest {
protected:
static void SetUpTestCase() {
gpr_setenv("GRPC_XDS_EXPERIMENTAL_SECURITY_SUPPORT", "true");
BasicTest::SetUpTestCase();
}
static void TearDownTestCase() {
BasicTest::TearDownTestCase();
gpr_unsetenv("GRPC_XDS_EXPERIMENTAL_SECURITY_SUPPORT");
}
void SetUp() override {
BasicTest::SetUp();
root_cert_ = ReadFile(kCaCertPath);
@ -9347,16 +9337,6 @@ class XdsServerSecurityTest : public XdsEnd2endTest {
XdsServerSecurityTest()
: XdsEnd2endTest(1, 1, 100, true /* use_xds_enabled_server */) {}
static void SetUpTestCase() {
gpr_setenv("GRPC_XDS_EXPERIMENTAL_SECURITY_SUPPORT", "true");
XdsEnd2endTest::SetUpTestCase();
}
static void TearDownTestCase() {
XdsEnd2endTest::TearDownTestCase();
gpr_unsetenv("GRPC_XDS_EXPERIMENTAL_SECURITY_SUPPORT");
}
void SetUp() override {
XdsEnd2endTest::SetUp();
root_cert_ = ReadFile(kCaCertPath);

Write Preview
Loading…
Cancel
Save