xDS resource validation: identify extensions by type_url instead of name (#31024)

* xDS resource validation: identify extensions by type_url instead of name

* fix build

src/core/ext/xds/xds_cluster.cc

@@ -26,6 +26,7 @@
 #include "absl/status/status.h"
 #include "absl/strings/str_cat.h"
 #include "absl/strings/str_join.h"
+#include "absl/strings/strip.h"
 #include "envoy/config/cluster/v3/circuit_breaker.upb.h"
 #include "envoy/config/cluster/v3/cluster.upb.h"
 #include "envoy/config/cluster/v3/cluster.upbdefs.h"
@@ -104,29 +105,32 @@ namespace {
 absl::StatusOr<CommonTlsContext> UpstreamTlsContextParse(
     const XdsResourceType::DecodeContext& context,
     const envoy_config_core_v3_TransportSocket* transport_socket) {
-  CommonTlsContext common_tls_context;
-  // Record Upstream tls context
-  absl::string_view name = UpbStringToAbsl(
-      envoy_config_core_v3_TransportSocket_name(transport_socket));
-  if (name != "envoy.transport_sockets.tls") {
-    return absl::InvalidArgumentError(
-        absl::StrCat("Unrecognized transport socket: ", name));
-  }
   auto* typed_config =
       envoy_config_core_v3_TransportSocket_typed_config(transport_socket);
-  if (typed_config != nullptr) {
+  if (typed_config == nullptr) {
+    return absl::InvalidArgumentError("transport_socket.typed_config not set");
+  }
+  absl::string_view type_url = absl::StripPrefix(
+      UpbStringToAbsl(google_protobuf_Any_type_url(typed_config)),
+      "type.googleapis.com/");
+  if (type_url !=
+      "envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext") {
+    return absl::InvalidArgumentError(
+        absl::StrCat("Unrecognized transport socket type: ", type_url));
+  }
   const upb_StringView encoded_upstream_tls_context =
       google_protobuf_Any_value(typed_config);
   auto* upstream_tls_context =
       envoy_extensions_transport_sockets_tls_v3_UpstreamTlsContext_parse(
-            encoded_upstream_tls_context.data,
+          encoded_upstream_tls_context.data, encoded_upstream_tls_context.size,
-            encoded_upstream_tls_context.size, context.arena);
+          context.arena);
   if (upstream_tls_context == nullptr) {
     return absl::InvalidArgumentError("Can't decode upstream tls context.");
   }
   auto* common_tls_context_proto =
       envoy_extensions_transport_sockets_tls_v3_UpstreamTlsContext_common_tls_context(
           upstream_tls_context);
+  CommonTlsContext common_tls_context;
   if (common_tls_context_proto != nullptr) {
     auto common_context =
         CommonTlsContext::Parse(context, common_tls_context_proto);
@@ -137,7 +141,6 @@ absl::StatusOr<CommonTlsContext> UpstreamTlsContextParse(
     }
     common_tls_context = std::move(*common_context);
   }
-  }
   if (common_tls_context.certificate_validation_context
           .ca_certificate_provider_instance.instance_name.empty()) {
     return absl::InvalidArgumentError(
@@ -243,31 +246,33 @@ absl::StatusOr<XdsClusterResource> CdsResourceParse(
     absl::Status status = CdsLogicalDnsParse(cluster, &cds_update);
     if (!status.ok()) errors.emplace_back(status.message());
   } else {
-    if (!envoy_config_cluster_v3_Cluster_has_cluster_type(cluster)) {
+    const auto* custom_cluster_type =
+        envoy_config_cluster_v3_Cluster_cluster_type(cluster);
+    if (custom_cluster_type == nullptr) {
       errors.push_back("DiscoveryType is not valid.");
     } else {
-      const envoy_config_cluster_v3_Cluster_CustomClusterType*
+      const auto* typed_config =
-          custom_cluster_type =
+          envoy_config_cluster_v3_Cluster_CustomClusterType_typed_config(
-              envoy_config_cluster_v3_Cluster_cluster_type(cluster);
-      upb_StringView type_name =
-          envoy_config_cluster_v3_Cluster_CustomClusterType_name(
               custom_cluster_type);
-      if (UpbStringToAbsl(type_name) != "envoy.clusters.aggregate") {
+      if (typed_config == nullptr) {
-        errors.emplace_back("DiscoveryType is not valid.");
+        errors.push_back("cluster_type.typed_config not set");
+      } else {
+        absl::string_view type_url = absl::StripPrefix(
+            UpbStringToAbsl(google_protobuf_Any_type_url(typed_config)),
+            "type.googleapis.com/");
+        if (type_url !=
+            "envoy.extensions.clusters.aggregate.v3.ClusterConfig") {
+          errors.push_back(
+              absl::StrCat("unknown cluster_type extension: ", type_url));
         } else {
           cds_update.cluster_type = XdsClusterResource::ClusterType::AGGREGATE;
           // Retrieve aggregate clusters.
-        const google_protobuf_Any* typed_config =
-            envoy_config_cluster_v3_Cluster_CustomClusterType_typed_config(
-                custom_cluster_type);
           const upb_StringView aggregate_cluster_config_upb_stringview =
               google_protobuf_Any_value(typed_config);
-        const envoy_extensions_clusters_aggregate_v3_ClusterConfig*
+          const auto* aggregate_cluster_config =
-            aggregate_cluster_config =
               envoy_extensions_clusters_aggregate_v3_ClusterConfig_parse(
                   aggregate_cluster_config_upb_stringview.data,
-                    aggregate_cluster_config_upb_stringview.size,
+                  aggregate_cluster_config_upb_stringview.size, context.arena);
-                    context.arena);
           if (aggregate_cluster_config == nullptr) {
             errors.emplace_back("Can't parse aggregate cluster.");
           } else {
@@ -284,6 +289,7 @@ absl::StatusOr<XdsClusterResource> CdsResourceParse(
         }
       }
     }
+  }
   // Check the LB policy.
   if (envoy_config_cluster_v3_Cluster_lb_policy(cluster) ==
       envoy_config_cluster_v3_Cluster_ROUND_ROBIN) {

src/core/ext/xds/xds_listener.cc

@@ -28,6 +28,7 @@
 #include "absl/strings/str_cat.h"
 #include "absl/strings/str_format.h"
 #include "absl/strings/str_join.h"
+#include "absl/strings/strip.h"
 #include "envoy/config/core/v3/address.upb.h"
 #include "envoy/config/core/v3/base.upb.h"
 #include "envoy/config/core/v3/config_source.upb.h"
@@ -505,28 +506,30 @@ absl::StatusOr<XdsListenerResource::DownstreamTlsContext>
 DownstreamTlsContextParse(
     const XdsResourceType::DecodeContext& context,
     const envoy_config_core_v3_TransportSocket* transport_socket) {
-  absl::string_view name = UpbStringToAbsl(
+  const auto* typed_config =
-      envoy_config_core_v3_TransportSocket_name(transport_socket));
-  if (name != "envoy.transport_sockets.tls") {
-    return absl::InvalidArgumentError(
-        absl::StrCat("Unrecognized transport socket: ", name));
-  }
-  std::vector<std::string> errors;
-  XdsListenerResource::DownstreamTlsContext downstream_tls_context;
-  auto* typed_config =
       envoy_config_core_v3_TransportSocket_typed_config(transport_socket);
   if (typed_config == nullptr) {
     return absl::InvalidArgumentError("transport socket typed config unset");
   }
+  absl::string_view type_url = absl::StripPrefix(
+      UpbStringToAbsl(google_protobuf_Any_type_url(typed_config)),
+      "type.googleapis.com/");
+  if (type_url !=
+      "envoy.extensions.transport_sockets.tls.v3.DownstreamTlsContext") {
+    return absl::InvalidArgumentError(
+        absl::StrCat("Unrecognized transport socket type: ", type_url));
+  }
   const upb_StringView encoded_downstream_tls_context =
       google_protobuf_Any_value(typed_config);
-  auto* downstream_tls_context_proto =
+  const auto* downstream_tls_context_proto =
       envoy_extensions_transport_sockets_tls_v3_DownstreamTlsContext_parse(
           encoded_downstream_tls_context.data,
           encoded_downstream_tls_context.size, context.arena);
   if (downstream_tls_context_proto == nullptr) {
     return absl::InvalidArgumentError("Can't decode downstream tls context.");
   }
+  std::vector<std::string> errors;
+  XdsListenerResource::DownstreamTlsContext downstream_tls_context;
   auto* common_tls_context =
       envoy_extensions_transport_sockets_tls_v3_DownstreamTlsContext_common_tls_context(
           downstream_tls_context_proto);
@@ -556,7 +559,6 @@ DownstreamTlsContextParse(
       envoy_extensions_transport_sockets_tls_v3_DownstreamTlsContext_LENIENT_STAPLING) {
     errors.emplace_back("ocsp_staple_policy: Only LENIENT_STAPLING supported");
   }
-
   if (downstream_tls_context.common_tls_context
           .tls_certificate_provider_instance.instance_name.empty()) {
     errors.emplace_back(
@@ -712,10 +714,10 @@ absl::StatusOr<FilterChain> FilterChainParse(
     if (typed_config == nullptr) {
       errors.emplace_back("No typed_config found in filter.");
     } else {
-      absl::string_view type_url =
+      absl::string_view type_url = absl::StripPrefix(
-          UpbStringToAbsl(google_protobuf_Any_type_url(typed_config));
+          UpbStringToAbsl(google_protobuf_Any_type_url(typed_config)),
+          "type.googleapis.com/");
       if (type_url !=
-          "type.googleapis.com/"
           "envoy.extensions.filters.network.http_connection_manager.v3."
           "HttpConnectionManager") {
         errors.emplace_back(absl::StrCat("Unsupported filter type ", type_url));

test/cpp/end2end/xds/xds_end2end_test.cc

@@ -431,24 +431,41 @@ class XdsSecurityTest : public XdsEnd2endTest {
   int backend_index_ = 0;
 };
 
+TEST_P(XdsSecurityTest, TransportSocketMissingTypedConfig) {
+  auto cluster = default_cluster_;
+  cluster.mutable_transport_socket();
+  balancer_->ads_service()->SetCdsResource(cluster);
+  const auto response_state =
+      WaitForCdsNack(DEBUG_LOCATION, RpcOptions().set_timeout_ms(5000));
+  ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK";
+  EXPECT_EQ(response_state->error_message,
+            "xDS response validation errors: [resource index 0: cluster_name: "
+            "INVALID_ARGUMENT: errors parsing CDS resource: ["
+            "Error parsing security configuration: "
+            "transport_socket.typed_config not set]]");
+}
+
 TEST_P(XdsSecurityTest, UnknownTransportSocket) {
   auto cluster = default_cluster_;
   auto* transport_socket = cluster.mutable_transport_socket();
-  transport_socket->set_name("unknown_transport_socket");
+  transport_socket->mutable_typed_config()->PackFrom(Listener());
   balancer_->ads_service()->SetCdsResource(cluster);
   const auto response_state =
       WaitForCdsNack(DEBUG_LOCATION, RpcOptions().set_timeout_ms(5000));
   ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK";
-  EXPECT_THAT(response_state->error_message,
+  EXPECT_EQ(response_state->error_message,
-              ::testing::HasSubstr(
+            "xDS response validation errors: [resource index 0: cluster_name: "
-                  "Unrecognized transport socket: unknown_transport_socket"));
+            "INVALID_ARGUMENT: errors parsing CDS resource: ["
+            "Error parsing security configuration: "
+            "Unrecognized transport socket type: "
+            "envoy.config.listener.v3.Listener]]");
 }
 
 TEST_P(XdsSecurityTest,
        TLSConfigurationWithoutValidationContextCertificateProviderInstance) {
   auto cluster = default_cluster_;
   auto* transport_socket = cluster.mutable_transport_socket();
-  transport_socket->set_name("envoy.transport_sockets.tls");
+  transport_socket->mutable_typed_config()->PackFrom(UpstreamTlsContext());
   balancer_->ads_service()->SetCdsResource(cluster);
   const auto response_state =
       WaitForCdsNack(DEBUG_LOCATION, RpcOptions().set_timeout_ms(5000));
@@ -1571,11 +1588,25 @@ class XdsServerSecurityTest : public XdsEnd2endTest {
   std::vector<std::string> client_authenticated_identity_;
 };
 
+TEST_P(XdsServerSecurityTest, TransportSocketTypedConfigUnset) {
+  Listener listener = default_server_listener_;
+  auto* filter_chain = listener.mutable_default_filter_chain();
+  filter_chain->mutable_transport_socket();
+  SetServerListenerNameAndRouteConfiguration(balancer_.get(), listener,
+                                             backends_[0]->port(),
+                                             default_server_route_config_);
+  backends_[0]->Start();
+  const auto response_state = WaitForLdsNack(DEBUG_LOCATION);
+  ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK";
+  EXPECT_THAT(response_state->error_message,
+              ::testing::HasSubstr("transport socket typed config unset"));
+}
+
 TEST_P(XdsServerSecurityTest, UnknownTransportSocket) {
   Listener listener = default_server_listener_;
   auto* filter_chain = listener.mutable_default_filter_chain();
   auto* transport_socket = filter_chain->mutable_transport_socket();
-  transport_socket->set_name("unknown_transport_socket");
+  transport_socket->mutable_typed_config()->PackFrom(Listener());
   SetServerListenerNameAndRouteConfiguration(balancer_.get(), listener,
                                              backends_[0]->port(),
                                              default_server_route_config_);
@@ -1583,8 +1614,8 @@ TEST_P(XdsServerSecurityTest, UnknownTransportSocket) {
   const auto response_state = WaitForLdsNack(DEBUG_LOCATION);
   ASSERT_TRUE(response_state.has_value()) << "timed out waiting for NACK";
   EXPECT_THAT(response_state->error_message,
-              ::testing::HasSubstr(
+              ::testing::HasSubstr("Unrecognized transport socket type: "
-                  "Unrecognized transport socket: unknown_transport_socket"));
+                                   "envoy.config.listener.v3.Listener"));
 }
 
 TEST_P(XdsServerSecurityTest, NacksRequireSNI) {