Merge remote-tracking branch 'origin/master' into fix-win-ee-use-after-free

MAINTAINERS.md

@@ -31,6 +31,7 @@ for general contribution guidelines.
 - [pfreixes](https://github.com/pfreixes), Skyscanner Ltd
 - [ran-su](https://github.com/ran-su), Google LLC
 - [sanjaypujare](https://github.com/sanjaypujare), Google LLC
+- [sastryvp](https://github.com/sastryvp), Google LLC
 - [sergiitk](https://github.com/sergiitk), Google LLC
 - [soheilhy](https://github.com/soheilhy), Google LLC
 - [stanley-cheung](https://github.com/stanley-cheung), Google LLC

bazel/experiments.bzl

@@ -28,6 +28,7 @@ EXPERIMENT_ENABLES = {
     "http2_stats_fix": "http2_stats_fix",
     "keepalive_fix": "keepalive_fix",
     "keepalive_server_fix": "keepalive_server_fix",
+    "max_pings_wo_data_throttle": "max_pings_wo_data_throttle",
     "monitoring_experiment": "monitoring_experiment",
     "multiping": "multiping",
     "peer_state_based_framing": "peer_state_based_framing",
@@ -57,10 +58,16 @@ EXPERIMENTS = {
         "dbg": {
         },
         "off": {
+            "core_end2end_test": [
+                "event_engine_client",
+            ],
             "endpoint_test": [
                 "tcp_frame_size_tuning",
                 "tcp_rcv_lowat",
             ],
+            "event_engine_client_test": [
+                "event_engine_client",
+            ],
             "flow_control_test": [
                 "multiping",
                 "peer_state_based_framing",
@@ -78,15 +85,11 @@ EXPERIMENTS = {
                 "event_engine_dns",
             ],
             "core_end2end_test": [
-                "event_engine_client",
                 "event_engine_listener",
             ],
             "cpp_lb_end2end_test": [
                 "pick_first_new",
             ],
-            "event_engine_client_test": [
-                "event_engine_client",
-            ],
             "event_engine_listener_test": [
                 "event_engine_listener",
             ],

bazel/grpc_deps.bzl

@@ -406,10 +406,10 @@ def grpc_deps():
     if "bazel_gazelle" not in native.existing_rules():
         http_archive(
             name = "bazel_gazelle",
-            sha256 = "de69a09dc70417580aabf20a28619bb3ef60d038470c7cf8442fafcf627c21cb",
+            sha256 = "d76bf7a60fd8b050444090dfa2837a4eaf9829e1165618ee35dceca5cbdf58d5",
             urls = [
-                "https://mirror.bazel.build/github.com/bazelbuild/bazel-gazelle/releases/download/v0.24.0/bazel-gazelle-v0.24.0.tar.gz",
-                "https://github.com/bazelbuild/bazel-gazelle/releases/download/v0.24.0/bazel-gazelle-v0.24.0.tar.gz",
+                "https://mirror.bazel.build/github.com/bazelbuild/bazel-gazelle/releases/download/v0.37.0/bazel-gazelle-v0.37.0.tar.gz",
+                "https://github.com/bazelbuild/bazel-gazelle/releases/download/v0.37.0/bazel-gazelle-v0.37.0.tar.gz",
             ],
         )
 

bazel/grpc_extra_deps.bzl

@@ -52,7 +52,7 @@ def grpc_extra_deps(ignore_version_differences = False):
     api_dependencies()
 
     go_rules_dependencies()
-    go_register_toolchains(version = "1.18")
+    go_register_toolchains(version = "1.20")
     gazelle_dependencies()
 
     # Pull-in the go 3rd party dependencies for protoc_gen_validate, which is

doc/grpc_xds_features.md

@@ -77,5 +77,5 @@ Aggregate Cluster Behavior Fixes | [A75](https://github.com/grpc/proposal/blob/m
 Pick First | [A62](https://github.com/grpc/proposal/blob/master/A62-pick-first.md) | v1.58.0 | v1.58.1 | v1.56.0 | |
 [StringMatcher for Header Matching](https://github.com/envoyproxy/envoy/blob/3fe4b8d335fa339ef6f17325c8d31f87ade7bb1a/api/envoy/config/route/v3/route_components.proto#L2280) | [A63](https://github.com/grpc/proposal/blob/master/A63-xds-string-matcher-in-header-matching.md) | v1.56.0 | v1.53.0 | v1.56.0 | v1.9.0 |
 LRS Custom Metrics Support | [A64](https://github.com/grpc/proposal/blob/master/A64-lrs-custom-metrics.md) | v1.54.0 | | | |
-mTLS Credentials in xDS Bootstrap File | [A65](https://github.com/grpc/proposal/blob/master/A65-xds-mtls-creds-in-bootstrap.md) | v1.57.0 | | v1.61.0 | |
+mTLS Credentials in xDS Bootstrap File | [A65](https://github.com/grpc/proposal/blob/master/A65-xds-mtls-creds-in-bootstrap.md) | v1.65.0 | | v1.61.0 | |
 Stateful Session Affinity | [A55](https://github.com/grpc/proposal/blob/master/A55-xds-stateful-session-affinity.md), [A60](https://github.com/grpc/proposal/blob/master/A60-xds-stateful-session-affinity-weighted-clusters.md), [A75](https://github.com/grpc/proposal/blob/master/A75-xds-aggregate-cluster-behavior-fixes.md) | v1.61.0 | | | |

include/grpc/impl/channel_arg_names.h

@@ -111,9 +111,11 @@
   "grpc.server_max_unrequested_time_in_server"
 /** Channel arg to override the http2 :scheme header */
 #define GRPC_ARG_HTTP2_SCHEME "grpc.http2_scheme"
-/** How many pings can the client send before needing to send a
-   data/header frame? (0 indicates that an infinite number of
-   pings can be sent without sending a data frame or header frame) */
+/** How many pings can the client send before needing to send a data/header
+   frame? (0 indicates that an infinite number of pings can be sent without
+   sending a data frame or header frame).
+   If experiment "max_pings_wo_data_throttle" is enabled, instead of pings being
+   completely blocked, they are throttled. */
 #define GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA \
   "grpc.http2.max_pings_without_data"
 /** How many misbehaving pings the server can bear before sending goaway and

src/core/ext/transport/chttp2/server/chttp2_server.cc

@@ -474,23 +474,18 @@ void Chttp2ServerListener::ActiveConnection::HandshakingState::OnHandshakeDone(
       // handshaker may have handed off the connection to some external
       // code, so we can just clean up here without creating a transport.
       if (args->endpoint != nullptr) {
-        Transport* transport =
-            grpc_create_chttp2_transport(args->args, args->endpoint, false);
+        RefCountedPtr<Transport> transport =
+            grpc_create_chttp2_transport(args->args, args->endpoint, false)
+                ->Ref();
         grpc_error_handle channel_init_err =
             self->connection_->listener_->server_->SetupTransport(
-                transport, self->accepting_pollset_, args->args,
-                grpc_chttp2_transport_get_socket_node(transport));
+                transport.get(), self->accepting_pollset_, args->args,
+                grpc_chttp2_transport_get_socket_node(transport.get()));
         if (channel_init_err.ok()) {
           // Use notify_on_receive_settings callback to enforce the
           // handshake deadline.
-          // Note: The reinterpret_cast<>s here are safe, because
-          // grpc_chttp2_transport is a C-style extension of
-          // Transport, so this is morally equivalent of a
-          // static_cast<> to a derived class.
-          // TODO(roth): Change to static_cast<> when we C++-ify the
-          // transport API.
           self->connection_->transport_ =
-              reinterpret_cast<grpc_chttp2_transport*>(transport)->Ref();
+              DownCast<grpc_chttp2_transport*>(transport.get())->Ref();
           self->Ref().release();  // Held by OnReceiveSettings().
           GRPC_CLOSURE_INIT(&self->on_receive_settings_, OnReceiveSettings,
                             self, grpc_schedule_on_exec_ctx);
@@ -520,9 +515,9 @@ void Chttp2ServerListener::ActiveConnection::HandshakingState::OnHandshakeDone(
                                            grpc_schedule_on_exec_ctx_);
             cleanup_connection = true;
           }
-          grpc_chttp2_transport_start_reading(transport, args->read_buffer,
-                                              &self->on_receive_settings_,
-                                              on_close);
+          grpc_chttp2_transport_start_reading(
+              transport.get(), args->read_buffer, &self->on_receive_settings_,
+              on_close);
           self->timer_handle_ = self->connection_->event_engine_->RunAfter(
               self->deadline_ - Timestamp::Now(),
               [self = self->Ref()]() mutable {

src/core/ext/transport/chttp2/transport/ping_rate_policy.cc

@@ -35,16 +35,18 @@
 namespace grpc_core {
 
 namespace {
-int g_default_max_pings_without_data = 2;
+int g_default_max_pings_without_data_sent = 2;
+constexpr Duration kThrottleIntervalWithoutDataSent = Duration::Minutes(1);
 absl::optional<int> g_default_max_inflight_pings;
 }  // namespace
 
 Chttp2PingRatePolicy::Chttp2PingRatePolicy(const ChannelArgs& args,
                                            bool is_client)
-    : max_pings_without_data_(
+    : max_pings_without_data_sent_(
           is_client
-              ? std::max(0, args.GetInt(GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA)
-                                .value_or(g_default_max_pings_without_data))
+              ? std::max(0,
+                         args.GetInt(GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA)
+                             .value_or(g_default_max_pings_without_data_sent))
               : 0),
       // Configuration via channel arg dominates, otherwise if the multiping
       // experiment is enabled we use 100, otherwise 1.
@@ -54,18 +56,15 @@ Chttp2PingRatePolicy::Chttp2PingRatePolicy(const ChannelArgs& args,
                               IsMultipingEnabled() ? 100 : 1)))) {}
 
 void Chttp2PingRatePolicy::SetDefaults(const ChannelArgs& args) {
-  g_default_max_pings_without_data =
+  g_default_max_pings_without_data_sent =
       std::max(0, args.GetInt(GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA)
-                      .value_or(g_default_max_pings_without_data));
+                      .value_or(g_default_max_pings_without_data_sent));
   g_default_max_inflight_pings = args.GetInt(GRPC_ARG_HTTP2_MAX_INFLIGHT_PINGS);
 }
 
 Chttp2PingRatePolicy::RequestSendPingResult
 Chttp2PingRatePolicy::RequestSendPing(Duration next_allowed_ping_interval,
                                       size_t inflight_pings) const {
-  if (max_pings_without_data_ != 0 && pings_before_data_required_ == 0) {
-    return TooManyRecentPings{};
-  }
   if (max_inflight_pings_ > 0 &&
       inflight_pings > static_cast<size_t>(max_inflight_pings_)) {
     return TooManyRecentPings{};
@@ -77,12 +76,29 @@ Chttp2PingRatePolicy::RequestSendPing(Duration next_allowed_ping_interval,
     return TooSoon{next_allowed_ping_interval, last_ping_sent_time_,
                    next_allowed_ping - now};
   }
+  // Throttle pings to 1 minute if we haven't sent any data recently
+  if (max_pings_without_data_sent_ != 0 &&
+      pings_before_data_sending_required_ == 0) {
+    if (IsMaxPingsWoDataThrottleEnabled()) {
+      const Timestamp next_allowed_ping =
+          last_ping_sent_time_ + kThrottleIntervalWithoutDataSent;
+      if (next_allowed_ping > now) {
+        return TooSoon{kThrottleIntervalWithoutDataSent, last_ping_sent_time_,
+                       next_allowed_ping - now};
+      }
+    } else {
+      return TooManyRecentPings{};
+    }
+  }
+
   return SendGranted{};
 }
 
 void Chttp2PingRatePolicy::SentPing() {
   last_ping_sent_time_ = Timestamp::Now();
-  if (pings_before_data_required_) --pings_before_data_required_;
+  if (pings_before_data_sending_required_ > 0) {
+    --pings_before_data_sending_required_;
+  }
 }
 
 void Chttp2PingRatePolicy::ReceivedDataFrame() {
@@ -90,13 +106,13 @@ void Chttp2PingRatePolicy::ReceivedDataFrame() {
 }
 
 void Chttp2PingRatePolicy::ResetPingsBeforeDataRequired() {
-  pings_before_data_required_ = max_pings_without_data_;
+  pings_before_data_sending_required_ = max_pings_without_data_sent_;
 }
 
 std::string Chttp2PingRatePolicy::GetDebugString() const {
   return absl::StrCat(
-      "max_pings_without_data: ", max_pings_without_data_,
-      ", pings_before_data_required: ", pings_before_data_required_,
+      "max_pings_without_data: ", max_pings_without_data_sent_,
+      ", pings_before_data_required: ", pings_before_data_sending_required_,
       ", last_ping_sent_time_: ", last_ping_sent_time_.ToString());
 }
 

src/core/ext/transport/chttp2/transport/ping_rate_policy.h

@@ -70,13 +70,14 @@ class Chttp2PingRatePolicy {
   void ReceivedDataFrame();
   std::string GetDebugString() const;
 
-  int TestOnlyMaxPingsWithoutData() const { return max_pings_without_data_; }
+  int TestOnlyMaxPingsWithoutData() const {
+    return max_pings_without_data_sent_;
+  }
 
  private:
-  const int max_pings_without_data_;
+  const int max_pings_without_data_sent_;
   const int max_inflight_pings_;
-  // No pings allowed before receiving a header or data frame.
-  int pings_before_data_required_ = 0;
+  int pings_before_data_sending_required_ = 0;
   Timestamp last_ping_sent_time_ = Timestamp::InfPast();
 };
 

src/core/lib/experiments/experiments.cc

@@ -59,6 +59,11 @@ const char* const description_keepalive_server_fix =
     "Allows overriding keepalive_permit_without_calls for servers. Refer "
     "https://github.com/grpc/grpc/pull/33917 for more information.";
 const char* const additional_constraints_keepalive_server_fix = "{}";
+const char* const description_max_pings_wo_data_throttle =
+    "Experiment to throttle pings to a period of 1 min when "
+    "GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA limit has reached (instead of "
+    "completely blocking).";
+const char* const additional_constraints_max_pings_wo_data_throttle = "{}";
 const char* const description_monitoring_experiment =
     "Placeholder experiment to prove/disprove our monitoring is working";
 const char* const additional_constraints_monitoring_experiment = "{}";
@@ -157,6 +162,9 @@ const ExperimentMetadata g_experiment_metadata[] = {
      additional_constraints_keepalive_fix, nullptr, 0, false, false},
     {"keepalive_server_fix", description_keepalive_server_fix,
      additional_constraints_keepalive_server_fix, nullptr, 0, false, false},
+    {"max_pings_wo_data_throttle", description_max_pings_wo_data_throttle,
+     additional_constraints_max_pings_wo_data_throttle, nullptr, 0, false,
+     true},
     {"monitoring_experiment", description_monitoring_experiment,
      additional_constraints_monitoring_experiment, nullptr, 0, true, true},
     {"multiping", description_multiping, additional_constraints_multiping,
@@ -241,6 +249,11 @@ const char* const description_keepalive_server_fix =
     "Allows overriding keepalive_permit_without_calls for servers. Refer "
     "https://github.com/grpc/grpc/pull/33917 for more information.";
 const char* const additional_constraints_keepalive_server_fix = "{}";
+const char* const description_max_pings_wo_data_throttle =
+    "Experiment to throttle pings to a period of 1 min when "
+    "GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA limit has reached (instead of "
+    "completely blocking).";
+const char* const additional_constraints_max_pings_wo_data_throttle = "{}";
 const char* const description_monitoring_experiment =
     "Placeholder experiment to prove/disprove our monitoring is working";
 const char* const additional_constraints_monitoring_experiment = "{}";
@@ -326,7 +339,7 @@ const ExperimentMetadata g_experiment_metadata[] = {
     {"client_privacy", description_client_privacy,
      additional_constraints_client_privacy, nullptr, 0, false, false},
     {"event_engine_client", description_event_engine_client,
-     additional_constraints_event_engine_client, nullptr, 0, true, true},
+     additional_constraints_event_engine_client, nullptr, 0, false, true},
     {"event_engine_dns", description_event_engine_dns,
      additional_constraints_event_engine_dns, nullptr, 0, true, false},
     {"event_engine_listener", description_event_engine_listener,
@@ -339,6 +352,9 @@ const ExperimentMetadata g_experiment_metadata[] = {
      additional_constraints_keepalive_fix, nullptr, 0, false, false},
     {"keepalive_server_fix", description_keepalive_server_fix,
      additional_constraints_keepalive_server_fix, nullptr, 0, false, false},
+    {"max_pings_wo_data_throttle", description_max_pings_wo_data_throttle,
+     additional_constraints_max_pings_wo_data_throttle, nullptr, 0, false,
+     true},
     {"monitoring_experiment", description_monitoring_experiment,
      additional_constraints_monitoring_experiment, nullptr, 0, true, true},
     {"multiping", description_multiping, additional_constraints_multiping,
@@ -423,6 +439,11 @@ const char* const description_keepalive_server_fix =
     "Allows overriding keepalive_permit_without_calls for servers. Refer "
     "https://github.com/grpc/grpc/pull/33917 for more information.";
 const char* const additional_constraints_keepalive_server_fix = "{}";
+const char* const description_max_pings_wo_data_throttle =
+    "Experiment to throttle pings to a period of 1 min when "
+    "GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA limit has reached (instead of "
+    "completely blocking).";
+const char* const additional_constraints_max_pings_wo_data_throttle = "{}";
 const char* const description_monitoring_experiment =
     "Placeholder experiment to prove/disprove our monitoring is working";
 const char* const additional_constraints_monitoring_experiment = "{}";
@@ -521,6 +542,9 @@ const ExperimentMetadata g_experiment_metadata[] = {
      additional_constraints_keepalive_fix, nullptr, 0, false, false},
     {"keepalive_server_fix", description_keepalive_server_fix,
      additional_constraints_keepalive_server_fix, nullptr, 0, false, false},
+    {"max_pings_wo_data_throttle", description_max_pings_wo_data_throttle,
+     additional_constraints_max_pings_wo_data_throttle, nullptr, 0, false,
+     true},
     {"monitoring_experiment", description_monitoring_experiment,
      additional_constraints_monitoring_experiment, nullptr, 0, true, true},
     {"multiping", description_multiping, additional_constraints_multiping,

src/core/lib/experiments/experiments.h

@@ -70,6 +70,7 @@ inline bool IsFreeLargeAllocatorEnabled() { return false; }
 inline bool IsHttp2StatsFixEnabled() { return true; }
 inline bool IsKeepaliveFixEnabled() { return false; }
 inline bool IsKeepaliveServerFixEnabled() { return false; }
+inline bool IsMaxPingsWoDataThrottleEnabled() { return false; }
 #define GRPC_EXPERIMENT_IS_INCLUDED_MONITORING_EXPERIMENT
 inline bool IsMonitoringExperimentEnabled() { return true; }
 inline bool IsMultipingEnabled() { return false; }
@@ -97,8 +98,7 @@ inline bool IsCallStatusOverrideOnCancellationEnabled() { return true; }
 inline bool IsCallV3Enabled() { return false; }
 inline bool IsCanaryClientPrivacyEnabled() { return false; }
 inline bool IsClientPrivacyEnabled() { return false; }
-#define GRPC_EXPERIMENT_IS_INCLUDED_EVENT_ENGINE_CLIENT
-inline bool IsEventEngineClientEnabled() { return true; }
+inline bool IsEventEngineClientEnabled() { return false; }
 #define GRPC_EXPERIMENT_IS_INCLUDED_EVENT_ENGINE_DNS
 inline bool IsEventEngineDnsEnabled() { return true; }
 #define GRPC_EXPERIMENT_IS_INCLUDED_EVENT_ENGINE_LISTENER
@@ -108,6 +108,7 @@ inline bool IsFreeLargeAllocatorEnabled() { return false; }
 inline bool IsHttp2StatsFixEnabled() { return true; }
 inline bool IsKeepaliveFixEnabled() { return false; }
 inline bool IsKeepaliveServerFixEnabled() { return false; }
+inline bool IsMaxPingsWoDataThrottleEnabled() { return false; }
 #define GRPC_EXPERIMENT_IS_INCLUDED_MONITORING_EXPERIMENT
 inline bool IsMonitoringExperimentEnabled() { return true; }
 inline bool IsMultipingEnabled() { return false; }
@@ -145,6 +146,7 @@ inline bool IsFreeLargeAllocatorEnabled() { return false; }
 inline bool IsHttp2StatsFixEnabled() { return true; }
 inline bool IsKeepaliveFixEnabled() { return false; }
 inline bool IsKeepaliveServerFixEnabled() { return false; }
+inline bool IsMaxPingsWoDataThrottleEnabled() { return false; }
 #define GRPC_EXPERIMENT_IS_INCLUDED_MONITORING_EXPERIMENT
 inline bool IsMonitoringExperimentEnabled() { return true; }
 inline bool IsMultipingEnabled() { return false; }
@@ -181,6 +183,7 @@ enum ExperimentIds {
   kExperimentIdHttp2StatsFix,
   kExperimentIdKeepaliveFix,
   kExperimentIdKeepaliveServerFix,
+  kExperimentIdMaxPingsWoDataThrottle,
   kExperimentIdMonitoringExperiment,
   kExperimentIdMultiping,
   kExperimentIdPeerStateBasedFraming,
@@ -243,6 +246,10 @@ inline bool IsKeepaliveFixEnabled() {
 inline bool IsKeepaliveServerFixEnabled() {
   return IsExperimentEnabled(kExperimentIdKeepaliveServerFix);
 }
+#define GRPC_EXPERIMENT_IS_INCLUDED_MAX_PINGS_WO_DATA_THROTTLE
+inline bool IsMaxPingsWoDataThrottleEnabled() {
+  return IsExperimentEnabled(kExperimentIdMaxPingsWoDataThrottle);
+}
 #define GRPC_EXPERIMENT_IS_INCLUDED_MONITORING_EXPERIMENT
 inline bool IsMonitoringExperimentEnabled() {
   return IsExperimentEnabled(kExperimentIdMonitoringExperiment);

src/core/lib/experiments/experiments.yaml

@@ -120,6 +120,14 @@
   owner: yashkt@google.com
   test_tags: []
   allow_in_fuzzing_config: false
+- name: max_pings_wo_data_throttle
+  description:
+    Experiment to throttle pings to a period of 1 min when
+    GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA limit has reached (instead of
+    completely blocking).
+  expiry: 2024/12/31
+  owner: yashkt@google.com
+  test_tags: []
 - name: monitoring_experiment
   description: Placeholder experiment to prove/disprove our monitoring is working
   expiry: never-ever

src/core/lib/experiments/rollouts.yaml

@@ -58,7 +58,7 @@
     # not tested on iOS at all
     ios: broken
     posix: false
-    windows: true
+    windows: false
 - name: event_engine_dns
   default:
     # not tested on iOS at all
@@ -79,6 +79,8 @@
   default: false
 - name: keepalive_server_fix
   default: false
+- name: max_pings_wo_data_throttle
+  default: false
 - name: monitoring_experiment
   default: true
 - name: peer_state_based_framing

src/core/lib/security/credentials/channel_creds_registry_init.cc

@@ -96,6 +96,8 @@ class TlsChannelCredsFactory : public ChannelCredsFactory<> {
     }
     options->set_watch_root_cert(!config->ca_certificate_file().empty());
     options->set_watch_identity_pair(!config->certificate_file().empty());
+    options->set_certificate_verifier(
+        MakeRefCounted<HostNameCertificateVerifier>());
     return MakeRefCounted<TlsCredentials>(std::move(options));
   }
 

test/core/end2end/tests/bad_ping.cc

@@ -87,6 +87,9 @@ CORE_END2END_TEST(RetryHttp2Test, BadPing) {
 // Try sending more pings than server allows, but server should be fine because
 // max_pings_without_data should limit pings sent out on wire.
 CORE_END2END_TEST(RetryHttp2Test, PingsWithoutData) {
+  if (IsMaxPingsWoDataThrottleEnabled()) {
+    GTEST_SKIP() << "pings are not limited if this experiment is enabled";
+  }
   // Only allow MAX_PING_STRIKES pings without data (DATA/HEADERS/WINDOW_UPDATE)
   // so that the transport will throttle the excess pings.
   InitClient(ChannelArgs()

test/core/transport/chttp2/ping_rate_policy_test.cc

@@ -19,6 +19,8 @@
 
 #include "gtest/gtest.h"
 
+#include "src/core/lib/experiments/experiments.h"
+
 namespace grpc_core {
 namespace {
 
@@ -47,6 +49,10 @@ TEST(PingRatePolicy, ServerCanSendAtStart) {
 }
 
 TEST(PingRatePolicy, ClientBlockedUntilDataSent) {
+  if (IsMaxPingsWoDataThrottleEnabled()) {
+    GTEST_SKIP()
+        << "Pings are not blocked if max_pings_wo_data_throttle is enabled.";
+  }
   Chttp2PingRatePolicy policy{ChannelArgs(), true};
   EXPECT_EQ(policy.RequestSendPing(Duration::Milliseconds(10), 0),
             TooManyRecentPings());
@@ -59,6 +65,35 @@ TEST(PingRatePolicy, ClientBlockedUntilDataSent) {
   EXPECT_EQ(policy.RequestSendPing(Duration::Zero(), 0), TooManyRecentPings());
 }
 
+TEST(PingRatePolicy, ClientThrottledUntilDataSent) {
+  if (!IsMaxPingsWoDataThrottleEnabled()) {
+    GTEST_SKIP()
+        << "Throttling behavior is enabled with max_pings_wo_data_throttle.";
+  }
+  Chttp2PingRatePolicy policy{ChannelArgs(), true};
+  // First ping is allowed.
+  EXPECT_EQ(policy.RequestSendPing(Duration::Milliseconds(10), 0),
+            SendGranted());
+  policy.SentPing();
+  // Second ping is throttled since no data has been sent.
+  auto result = policy.RequestSendPing(Duration::Zero(), 0);
+  EXPECT_TRUE(absl::holds_alternative<Chttp2PingRatePolicy::TooSoon>(result));
+  EXPECT_EQ(absl::get<Chttp2PingRatePolicy::TooSoon>(result).wait,
+            Duration::Minutes(1));
+  policy.ResetPingsBeforeDataRequired();
+  // After resetting pings before data required (data sent), we can send pings
+  // without being throttled.
+  EXPECT_EQ(policy.RequestSendPing(Duration::Zero(), 0), SendGranted());
+  policy.SentPing();
+  EXPECT_EQ(policy.RequestSendPing(Duration::Zero(), 0), SendGranted());
+  policy.SentPing();
+  // After reaching limit, we are throttled again.
+  result = policy.RequestSendPing(Duration::Zero(), 0);
+  EXPECT_TRUE(absl::holds_alternative<Chttp2PingRatePolicy::TooSoon>(result));
+  EXPECT_EQ(absl::get<Chttp2PingRatePolicy::TooSoon>(result).wait,
+            Duration::Minutes(1));
+}
+
 TEST(PingRatePolicy, RateThrottlingWorks) {
   Chttp2PingRatePolicy policy{ChannelArgs(), false};
   // Observe that we can fail if we send in a tight loop

test/cpp/end2end/xds/BUILD

@@ -216,6 +216,11 @@ grpc_cc_test(
     name = "xds_core_end2end_test",
     size = "large",
     srcs = ["xds_core_end2end_test.cc"],
+    data = [
+        "//src/core/tsi/test_creds:ca.pem",
+        "//src/core/tsi/test_creds:server1.key",
+        "//src/core/tsi/test_creds:server1.pem",
+    ],
     external_deps = [
         "gtest",
     ],

test/cpp/end2end/xds/xds_core_end2end_test.cc

@@ -160,6 +160,34 @@ TEST_P(XdsClientTest, XdsStreamErrorPropagation) {
               ::testing::HasSubstr("(node ID:xds_end2end_test)"));
 }
 
+//
+// XdsServerTlsTest: xDS server using TlsCreds
+//
+
+class XdsServerTlsTest : public XdsEnd2endTest {
+ protected:
+  void SetUp() override {
+    InitClient(MakeBootstrapBuilder().SetXdsChannelCredentials(
+                   "tls", absl::StrCat("{\"ca_certificate_file\": \"",
+                                       kCaCertPath, "\"}")),
+               /*lb_expected_authority=*/"",
+               /*xds_resource_does_not_exist_timeout_ms=*/0,
+               /*balancer_authority_override=*/"foo.test.google.fr");
+  }
+};
+
+INSTANTIATE_TEST_SUITE_P(
+    XdsTest, XdsServerTlsTest,
+    ::testing::Values(XdsTestType().set_xds_server_uses_tls_creds(true)),
+    &XdsTestType::Name);
+
+TEST_P(XdsServerTlsTest, Basic) {
+  CreateAndStartBackends(1);
+  EdsResourceArgs args({{"locality0", CreateEndpointsForBackends()}});
+  balancer_->ads_service()->SetEdsResource(BuildEdsResource(args));
+  CheckRpcSendOk(DEBUG_LOCATION);
+}
+
 //
 // GlobalXdsClientTest - tests that need to run with a global XdsClient
 // (this is the default in production)

test/cpp/end2end/xds/xds_end2end_test_lib.cc

@@ -318,6 +318,26 @@ XdsEnd2endTest::BalancerServerThread::BalancerServerThread(
           },
           debug_label)) {}
 
+std::shared_ptr<ServerCredentials>
+XdsEnd2endTest::BalancerServerThread::Credentials() {
+  if (GetParam().xds_server_uses_tls_creds()) {
+    std::string identity_cert =
+        grpc_core::testing::GetFileContents(kServerCertPath);
+    std::string private_key =
+        grpc_core::testing::GetFileContents(kServerKeyPath);
+    std::vector<experimental::IdentityKeyCertPair> identity_key_cert_pairs = {
+        {private_key, identity_cert}};
+    auto certificate_provider =
+        std::make_shared<grpc::experimental::StaticDataCertificateProvider>(
+            identity_key_cert_pairs);
+    grpc::experimental::TlsServerCredentialsOptions options(
+        certificate_provider);
+    options.watch_identity_key_cert_pairs();
+    return grpc::experimental::TlsServerCredentials(options);
+  }
+  return ServerThread::Credentials();
+}
+
 void XdsEnd2endTest::BalancerServerThread::RegisterAllServices(
     ServerBuilder* builder) {
   builder->RegisterService(ads_service_.get());
@@ -490,7 +510,8 @@ std::vector<int> XdsEnd2endTest::GetBackendPorts(size_t start_index,
 
 void XdsEnd2endTest::InitClient(absl::optional<XdsBootstrapBuilder> builder,
                                 std::string lb_expected_authority,
-                                int xds_resource_does_not_exist_timeout_ms) {
+                                int xds_resource_does_not_exist_timeout_ms,
+                                std::string balancer_authority_override) {
   if (!builder.has_value()) {
     builder = MakeBootstrapBuilder();
   }
@@ -509,6 +530,11 @@ void XdsEnd2endTest::InitClient(absl::optional<XdsBootstrapBuilder> builder,
         const_cast<char*>(GRPC_ARG_FAKE_SECURITY_EXPECTED_TARGETS),
         const_cast<char*>(lb_expected_authority.c_str())));
   }
+  if (!balancer_authority_override.empty()) {
+    xds_channel_args_to_add_.emplace_back(grpc_channel_arg_string_create(
+        const_cast<char*>(GRPC_ARG_DEFAULT_AUTHORITY),
+        const_cast<char*>(balancer_authority_override.c_str())));
+  }
   xds_channel_args_.num_args = xds_channel_args_to_add_.size();
   xds_channel_args_.args = xds_channel_args_to_add_.data();
   bootstrap_ = builder->Build();

test/cpp/end2end/xds/xds_end2end_test_lib.h

@@ -115,6 +115,11 @@ class XdsTestType {
     return *this;
   }
 
+  XdsTestType& set_xds_server_uses_tls_creds(bool value) {
+    xds_server_uses_tls_creds_ = value;
+    return *this;
+  }
+
   bool enable_load_reporting() const { return enable_load_reporting_; }
   bool enable_rds_testing() const { return enable_rds_testing_; }
   bool use_xds_credentials() const { return use_xds_credentials_; }
@@ -130,6 +135,7 @@ class XdsTestType {
   rbac_audit_condition() const {
     return rbac_audit_condition_;
   }
+  bool xds_server_uses_tls_creds() const { return xds_server_uses_tls_creds_; }
 
   std::string AsString() const {
     std::string retval = "V3";
@@ -158,6 +164,9 @@ class XdsTestType {
                                  RBAC_AuditLoggingOptions_AuditCondition_Name(
                                      rbac_audit_condition_));
     }
+    if (xds_server_uses_tls_creds_) {
+      retval += "XdsServerUsesTlsCreds";
+    }
     return retval;
   }
 
@@ -178,6 +187,7 @@ class XdsTestType {
   ::envoy::config::rbac::v3::RBAC_AuditLoggingOptions_AuditCondition
       rbac_audit_condition_ = ::envoy::config::rbac::v3::
           RBAC_AuditLoggingOptions_AuditCondition_NONE;
+  bool xds_server_uses_tls_creds_ = false;
 };
 
 // A base class for xDS end-to-end tests.
@@ -412,6 +422,11 @@ class XdsEnd2endTest : public ::testing::TestWithParam<XdsTestType>,
     AdsServiceImpl* ads_service() { return ads_service_.get(); }
     LrsServiceImpl* lrs_service() { return lrs_service_.get(); }
 
+    // If XdsTestType::xds_server_uses_tls_creds() is true,
+    // returns TlsServerCredentials.
+    // Otherwise, returns fake credentials.
+    std::shared_ptr<ServerCredentials> Credentials() override;
+
    private:
     const char* Type() override { return "Balancer"; }
     void RegisterAllServices(ServerBuilder* builder) override;
@@ -578,7 +593,8 @@ class XdsEnd2endTest : public ::testing::TestWithParam<XdsTestType>,
   // All tests must call this exactly once at the start of the test.
   void InitClient(absl::optional<XdsBootstrapBuilder> builder = absl::nullopt,
                   std::string lb_expected_authority = "",
-                  int xds_resource_does_not_exist_timeout_ms = 0);
+                  int xds_resource_does_not_exist_timeout_ms = 0,
+                  std::string balancer_authority_override = "");
 
   XdsBootstrapBuilder MakeBootstrapBuilder() {
     return XdsBootstrapBuilder().SetServers(

test/cpp/end2end/xds/xds_utils.cc

@@ -84,7 +84,7 @@ std::string XdsBootstrapBuilder::MakeXdsServersText(
       "          \"server_uri\": \"<SERVER_URI>\",\n"
       "          \"channel_creds\": [\n"
       "            {\n"
-      "              \"type\": \"<SERVER_CREDS_TYPE>\"\n"
+      "              \"type\": \"<SERVER_CREDS_TYPE>\"<SERVER_CREDS_CONFIG>\n"
       "            }\n"
       "          ],\n"
       "          \"server_features\": [<SERVER_FEATURES>]\n"
@@ -99,6 +99,11 @@ std::string XdsBootstrapBuilder::MakeXdsServersText(
         kXdsServerTemplate,
         {{"<SERVER_URI>", server_uri},
          {"<SERVER_CREDS_TYPE>", xds_channel_creds_type_},
+         {"<SERVER_CREDS_CONFIG>",
+          xds_channel_creds_config_.empty()
+              ? ""
+              : absl::StrCat(",\n              \"config\": ",
+                             xds_channel_creds_config_)},
          {"<SERVER_FEATURES>", absl::StrJoin(server_features, ", ")}}));
   }
   return absl::StrCat("      \"xds_servers\": [\n",

test/cpp/end2end/xds/xds_utils.h

@@ -43,8 +43,10 @@ class XdsBootstrapBuilder {
     servers_ = std::vector<std::string>(servers.begin(), servers.end());
     return *this;
   }
-  XdsBootstrapBuilder& SetXdsChannelCredentials(const std::string& type) {
+  XdsBootstrapBuilder& SetXdsChannelCredentials(
+      const std::string& type, const std::string& config = "") {
     xds_channel_creds_type_ = type;
+    xds_channel_creds_config_ = config;
     return *this;
   }
   XdsBootstrapBuilder& SetClientDefaultListenerResourceNameTemplate(
@@ -100,6 +102,7 @@ class XdsBootstrapBuilder {
   bool ignore_resource_deletion_ = false;
   std::vector<std::string> servers_;
   std::string xds_channel_creds_type_ = "fake";
+  std::string xds_channel_creds_config_;
   std::string client_default_listener_resource_name_template_;
   std::map<std::string /*key*/, PluginInfo> plugins_;
   std::map<std::string /*authority_name*/, AuthorityInfo> authorities_;

tools/internal_ci/helper_scripts/requirements.linux_perf.txt

@@ -1,5 +1,5 @@
 cryptography==3.4.6
 PyJWT==2.0.1
-requests==2.31.0
+requests==2.32.2
 scipy==1.5.4
 tabulate==0.8.9

tools/internal_ci/helper_scripts/requirements.macos.txt

@@ -3,4 +3,4 @@ cryptography==3.4.6
 PyJWT==2.0.1
 pyOpenSSL==20.0.1
 PyYAML==6.0
-requests==2.31.0
+requests==2.32.2