xDS PSM Server Security: Nack unsupported require_sni and ocsp_staple_policy values (#26878 An error occurred )

* Nack unsupported require_sni and ocsp_staple_policy values

* Reviewer comments

src/core/ext/xds/xds_api.cc

@@ -2290,6 +2290,7 @@ grpc_error_handle DownstreamTlsContextParse(
   }
   auto* typed_config =
       envoy_config_core_v3_TransportSocket_typed_config(transport_socket);
+  std::vector<grpc_error_handle> errors;
   if (typed_config != nullptr) {
     const upb_strview encoded_downstream_tls_context =
         google_protobuf_Any_value(typed_config);
@@ -2308,7 +2309,7 @@ grpc_error_handle DownstreamTlsContextParse(
       grpc_error_handle error =
           CommonTlsContextParse(context, common_tls_context,
                                 &downstream_tls_context->common_tls_context);
-      if (error != GRPC_ERROR_NONE) return error;
+      if (error != GRPC_ERROR_NONE) errors.push_back(error);
     }
     auto* require_client_certificate =
         envoy_extensions_transport_sockets_tls_v3_DownstreamTlsContext_require_client_certificate(
@@ -2317,23 +2318,38 @@ grpc_error_handle DownstreamTlsContextParse(
       downstream_tls_context->require_client_certificate =
           google_protobuf_BoolValue_value(require_client_certificate);
     }
+    auto* require_sni =
+        envoy_extensions_transport_sockets_tls_v3_DownstreamTlsContext_require_sni(
+            downstream_tls_context_proto);
+    if (require_sni != nullptr &&
+        google_protobuf_BoolValue_value(require_sni)) {
+      errors.push_back(
+          GRPC_ERROR_CREATE_FROM_STATIC_STRING("require_sni: unsupported"));
+    }
+    if (envoy_extensions_transport_sockets_tls_v3_DownstreamTlsContext_ocsp_staple_policy(
+            downstream_tls_context_proto) !=
+        envoy_extensions_transport_sockets_tls_v3_DownstreamTlsContext_LENIENT_STAPLING) {
+      errors.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING(
+          "ocsp_staple_policy: Only LENIENT_STAPLING supported"));
+    }
   }
   if (downstream_tls_context->common_tls_context
           .tls_certificate_certificate_provider_instance.instance_name
           .empty()) {
-    return GRPC_ERROR_CREATE_FROM_STATIC_STRING(
+    errors.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING(
         "TLS configuration provided but no "
-        "tls_certificate_certificate_provider_instance found.");
+        "tls_certificate_certificate_provider_instance found."));
   }
   if (downstream_tls_context->require_client_certificate &&
       downstream_tls_context->common_tls_context.combined_validation_context
           .validation_context_certificate_provider_instance.instance_name
           .empty()) {
-    return GRPC_ERROR_CREATE_FROM_STATIC_STRING(
+    errors.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING(
         "TLS configuration requires client certificates but no certificate "
-        "provider instance specified for validation.");
+        "provider instance specified for validation."));
   }
-  return GRPC_ERROR_NONE;
+  return GRPC_ERROR_CREATE_FROM_VECTOR("Error parsing DownstreamTlsContext",
+                                       &errors);
 }
 
 grpc_error_handle CidrRangeParse(
@@ -2424,38 +2440,39 @@ grpc_error_handle FilterChainParse(
     const EncodingContext& context,
     const envoy_config_listener_v3_FilterChain* filter_chain_proto, bool is_v2,
     FilterChain* filter_chain) {
-  grpc_error_handle error = GRPC_ERROR_NONE;
+  std::vector<grpc_error_handle> errors;
   auto* filter_chain_match =
       envoy_config_listener_v3_FilterChain_filter_chain_match(
           filter_chain_proto);
   if (filter_chain_match != nullptr) {
-    error = FilterChainMatchParse(filter_chain_match,
+    grpc_error_handle error = FilterChainMatchParse(
-                                  &filter_chain->filter_chain_match);
+        filter_chain_match, &filter_chain->filter_chain_match);
-    if (error != GRPC_ERROR_NONE) return error;
+    if (error != GRPC_ERROR_NONE) errors.push_back(error);
   }
   // Parse the filters list. Currently we only support HttpConnectionManager.
   size_t size = 0;
   auto* filters =
       envoy_config_listener_v3_FilterChain_filters(filter_chain_proto, &size);
   if (size != 1) {
-    return GRPC_ERROR_CREATE_FROM_STATIC_STRING(
+    errors.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING(
         "FilterChain should have exactly one filter: HttpConnectionManager; no "
-        "other filter is supported at the moment");
+        "other filter is supported at the moment"));
-  }
+  } else {
-  auto* typed_config = envoy_config_listener_v3_Filter_typed_config(filters[0]);
+    auto* typed_config =
+        envoy_config_listener_v3_Filter_typed_config(filters[0]);
     if (typed_config == nullptr) {
-    return GRPC_ERROR_CREATE_FROM_STATIC_STRING(
+      errors.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING(
-        "No typed_config found in filter.");
+          "No typed_config found in filter."));
-  }
+    } else {
       absl::string_view type_url =
           UpbStringToAbsl(google_protobuf_Any_type_url(typed_config));
       if (type_url !=
           "type.googleapis.com/"
           "envoy.extensions.filters.network.http_connection_manager.v3."
           "HttpConnectionManager") {
-    return GRPC_ERROR_CREATE_FROM_COPIED_STRING(
+        errors.push_back(GRPC_ERROR_CREATE_FROM_COPIED_STRING(
-        absl::StrCat("Unsupported filter type ", type_url).c_str());
+            absl::StrCat("Unsupported filter type ", type_url).c_str()));
-  }
+      } else {
         const upb_strview encoded_http_connection_manager =
             google_protobuf_Any_value(typed_config);
         const auto* http_connection_manager =
@@ -2463,28 +2480,33 @@ grpc_error_handle FilterChainParse(
                 encoded_http_connection_manager.data,
                 encoded_http_connection_manager.size, context.arena);
         if (http_connection_manager == nullptr) {
-    return GRPC_ERROR_CREATE_FROM_STATIC_STRING(
+          errors.push_back(GRPC_ERROR_CREATE_FROM_STATIC_STRING(
               "Could not parse HttpConnectionManager config from filter "
-        "typed_config");
+              "typed_config"));
-  }
+        } else {
           filter_chain->filter_chain_data =
               std::make_shared<XdsApi::LdsUpdate::FilterChainData>();
-  error = HttpConnectionManagerParse(
+          grpc_error_handle error = HttpConnectionManagerParse(
               false /* is_client */, context, http_connection_manager, is_v2,
               &filter_chain->filter_chain_data->http_connection_manager);
-  if (error != GRPC_ERROR_NONE) return error;
+          if (error != GRPC_ERROR_NONE) errors.push_back(error);
+        }
+      }
+    }
+  }
   // 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) {
-      error = DownstreamTlsContextParse(
+      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 error;
+  return GRPC_ERROR_CREATE_FROM_VECTOR("Error parsing FilterChain", &errors);
 }
 
 grpc_error_handle AddressParse(

src/proto/grpc/testing/xds/v3/tls.proto

@@ -139,12 +139,43 @@ message UpstreamTlsContext {
 }
 
 message DownstreamTlsContext {
+  enum OcspStaplePolicy {
+    // OCSP responses are optional. If an OCSP response is absent
+    // or expired, the associated certificate will be used for
+    // connections without an OCSP staple.
+    LENIENT_STAPLING = 0;
+
+    // OCSP responses are optional. If an OCSP response is absent,
+    // the associated certificate will be used without an
+    // OCSP staple. If a response is provided but is expired,
+    // the associated certificate will not be used for
+    // subsequent connections. If no suitable certificate is found,
+    // the connection is rejected.
+    STRICT_STAPLING = 1;
+
+    // OCSP responses are required. Configuration will fail if
+    // a certificate is provided without an OCSP response. If a
+    // response expires, the associated certificate will not be
+    // used connections. If no suitable certificate is found, the
+    // connection is rejected.
+    MUST_STAPLE = 2;
+  }
+
   // Common TLS context settings.
   CommonTlsContext common_tls_context = 1;
 
   // If specified, Envoy will reject connections without a valid client
   // certificate.
   google.protobuf.BoolValue require_client_certificate = 2;
+
+  // If specified, Envoy will reject connections without a valid and matching SNI.
+  // [#not-implemented-hide:]
+  google.protobuf.BoolValue require_sni = 3;
+
+  // Config for whether to use certificates if they do not have
+  // an accompanying OCSP response or if the response expires at runtime.
+  // Defaults to LENIENT_STAPLING
+  OcspStaplePolicy ocsp_staple_policy = 8;
 }
 
 

test/cpp/end2end/xds_end2end_test.cc

@@ -9375,6 +9375,67 @@ TEST_P(XdsServerSecurityTest, UnknownTransportSocket) {
                   "Unrecognized transport socket: unknown_transport_socket"));
 }
 
+TEST_P(XdsServerSecurityTest, NacksRequireSNI) {
+  Listener listener;
+  listener.set_name(
+      absl::StrCat("grpc/server?xds.resource.listening_address=",
+                   ipv6_only_ ? "[::1]:" : "127.0.0.1:", backends_[0]->port()));
+  auto* socket_address = listener.mutable_address()->mutable_socket_address();
+  socket_address->set_address(ipv6_only_ ? "::1" : "127.0.0.1");
+  socket_address->set_port_value(backends_[0]->port());
+  auto* filter_chain = listener.add_filter_chains();
+  filter_chain->add_filters()->mutable_typed_config()->PackFrom(
+      HttpConnectionManager());
+  auto* transport_socket = filter_chain->mutable_transport_socket();
+  transport_socket->set_name("envoy.transport_sockets.tls");
+  DownstreamTlsContext downstream_tls_context;
+  downstream_tls_context.mutable_common_tls_context()
+      ->mutable_tls_certificate_certificate_provider_instance()
+      ->set_instance_name("fake_plugin1");
+  downstream_tls_context.mutable_require_sni()->set_value(true);
+  transport_socket->mutable_typed_config()->PackFrom(downstream_tls_context);
+  balancers_[0]->ads_service()->SetLdsResource(listener);
+  ASSERT_TRUE(WaitForLdsNack(StatusCode::DEADLINE_EXCEEDED))
+      << "timed out waiting for NACK";
+  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("require_sni: unsupported"));
+}
+
+TEST_P(XdsServerSecurityTest, NacksOcspStaplePolicyOtherThanLenientStapling) {
+  Listener listener;
+  listener.set_name(
+      absl::StrCat("grpc/server?xds.resource.listening_address=",
+                   ipv6_only_ ? "[::1]:" : "127.0.0.1:", backends_[0]->port()));
+  auto* socket_address = listener.mutable_address()->mutable_socket_address();
+  socket_address->set_address(ipv6_only_ ? "::1" : "127.0.0.1");
+  socket_address->set_port_value(backends_[0]->port());
+  auto* filter_chain = listener.add_filter_chains();
+  filter_chain->add_filters()->mutable_typed_config()->PackFrom(
+      HttpConnectionManager());
+  auto* transport_socket = filter_chain->mutable_transport_socket();
+  transport_socket->set_name("envoy.transport_sockets.tls");
+  DownstreamTlsContext downstream_tls_context;
+  downstream_tls_context.mutable_common_tls_context()
+      ->mutable_tls_certificate_certificate_provider_instance()
+      ->set_instance_name("fake_plugin1");
+  downstream_tls_context.set_ocsp_staple_policy(
+      envoy::extensions::transport_sockets::tls::v3::
+          DownstreamTlsContext_OcspStaplePolicy_STRICT_STAPLING);
+  transport_socket->mutable_typed_config()->PackFrom(downstream_tls_context);
+  balancers_[0]->ads_service()->SetLdsResource(listener);
+  ASSERT_TRUE(WaitForLdsNack(StatusCode::DEADLINE_EXCEEDED))
+      << "timed out waiting for NACK";
+  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(
+                  "ocsp_staple_policy: Only LENIENT_STAPLING supported"));
+}
+
 TEST_P(
     XdsServerSecurityTest,
     NacksRequiringClientCertificateWithoutValidationCertificateProviderInstance) {