Envoy ext_proc filter throw exception when received response timeout Duration is too large

api/envoy/extensions/filters/http/ext_proc/v3/ext_proc.proto

@@ -145,7 +145,10 @@ message ExternalProcessor {
   // to the processing mode) if the timer expires before a matching response
   // is received. There is no timeout when the filter is running in asynchronous
   // mode. Default is 200 milliseconds.
-  google.protobuf.Duration message_timeout = 7;
+  google.protobuf.Duration message_timeout = 7 [(validate.rules).duration = {
+    lte {seconds: 3600}
+    gte {}
+  }];
 
   // Optional additional prefix to use when emitting statistics. This allows to distinguish
   // emitted statistics between configured *ext_proc* filters in an HTTP filter chain.
@@ -167,7 +170,10 @@ message ExternalProcessor {
   // Specify the upper bound of
   // :ref:`override_message_timeout <envoy_v3_api_field_service.ext_proc.v3.ProcessingResponse.override_message_timeout>`
   // If not specified, by default it is 0, which will effectively disable the ``override_message_timeout`` API.
-  google.protobuf.Duration max_message_timeout = 10;
+  google.protobuf.Duration max_message_timeout = 10 [(validate.rules).duration = {
+    lte {seconds: 3600}
+    gte {}
+  }];
 
   // Prevents clearing the route-cache when the
   // :ref:`clear_route_cache <envoy_v3_api_field_service.ext_proc.v3.CommonResponse.clear_route_cache>`

source/common/protobuf/utility.cc

@@ -656,6 +656,16 @@ void validateDuration(const ProtobufWkt::Duration& duration, int64_t max_seconds
   }
 }
 
+bool validateDurationErrorNoThrow(const ProtobufWkt::Duration& duration,
+                                  int64_t max_seconds_value) {
+  if (duration.seconds() < 0 || duration.nanos() < 0 || duration.nanos() > 999999999 ||
+      duration.seconds() > max_seconds_value) {
+    ENVOY_LOG_MISC(warn, "Duration out-of-range: {}", duration.DebugString());
+    return true;
+  }
+  return false;
+}
+
 void validateDuration(const ProtobufWkt::Duration& duration) {
   validateDuration(duration, Protobuf::util::TimeUtil::kDurationMaxSeconds);
 }
@@ -669,13 +679,30 @@ void validateDurationAsMilliseconds(const ProtobufWkt::Duration& duration) {
   validateDuration(duration, kMaxInt64Nanoseconds);
 }
 
+bool validateDurationAsMillisecondsErrorNoThrow(const ProtobufWkt::Duration& duration) {
+  // Apply stricter max boundary to the `seconds` value to avoid overflow.
+  // Note that protobuf internally converts to nanoseconds.
+  // The kMaxInt64Nanoseconds = 9223372036, which is about 300 years.
+  constexpr int64_t kMaxInt64Nanoseconds =
+      std::numeric_limits<int64_t>::max() / (1000 * 1000 * 1000);
+  return validateDurationErrorNoThrow(duration, kMaxInt64Nanoseconds);
+}
+
 } // namespace
 
 uint64_t DurationUtil::durationToMilliseconds(const ProtobufWkt::Duration& duration) {
   validateDurationAsMilliseconds(duration);
   return Protobuf::util::TimeUtil::DurationToMilliseconds(duration);
 }
 
+uint64_t DurationUtil::durationToMillisecondsNoThrow(const ProtobufWkt::Duration& duration,
+                                                     bool& error) {
+  if ((error = validateDurationAsMillisecondsErrorNoThrow(duration))) {
+    return 0;
+  }
+  return Protobuf::util::TimeUtil::DurationToMilliseconds(duration);
+}
+
 uint64_t DurationUtil::durationToSeconds(const ProtobufWkt::Duration& duration) {
   validateDuration(duration);
   return Protobuf::util::TimeUtil::DurationToSeconds(duration);

source/common/protobuf/utility.h

@@ -683,6 +683,15 @@ class DurationUtil {
    */
   static uint64_t durationToMilliseconds(const ProtobufWkt::Duration& duration);
 
+  /**
+   * Same as DurationUtil::durationToMilliseconds but does not throw an exception.
+   * Instead, it returns error if duration is out-of-range.
+   * @param duration protobuf.
+   * @param error bool, set into true in case of duration out-of-range; otherwise false.
+   * @return duration in milliseconds.
+   */
+  static uint64_t durationToMillisecondsNoThrow(const ProtobufWkt::Duration& duration, bool& error);
+
   /**
    * Same as Protobuf::util::TimeUtil::DurationToSeconds but with extra validation logic.
    * Specifically, we ensure that the duration is positive.

source/extensions/filters/http/ext_proc/ext_proc.cc

@@ -527,12 +527,21 @@ void Filter::sendTrailers(ProcessorState& state, const Http::HeaderMap& trailers
   stats_.stream_msgs_sent_.inc();
 }
 
-void Filter::onNewTimeout(const uint32_t message_timeout_ms) {
+void Filter::onNewTimeout(const ProtobufWkt::Duration& override_message_timeout) {
+  bool error = false;
+  uint32_t message_timeout_ms =
+      DurationUtil::durationToMillisecondsNoThrow(override_message_timeout, error);
+  if (error) {
+    ENVOY_LOG(warn, "Ext_proc server new timeout setting is out of duration range. "
+                    "Ignoring the message.");
+    stats_.override_message_timeout_ignored_.inc();
+    return;
+  }
   // The new timeout has to be >=1ms and <= max_message_timeout configured in filter.
   const uint32_t min_timeout_ms = 1;
   const uint32_t max_timeout_ms = config_->maxMessageTimeout();
   if (message_timeout_ms < min_timeout_ms || message_timeout_ms > max_timeout_ms) {
-    ENVOY_LOG(warn, "Ext_proc server new timeout setting is out of range. "
+    ENVOY_LOG(warn, "Ext_proc server new timeout setting is out of config range. "
                     "Ignoring the message.");
     stats_.override_message_timeout_ignored_.inc();
     return;
@@ -559,7 +568,7 @@ void Filter::onReceiveMessage(std::unique_ptr<ProcessingResponse>&& r) {
 
   // Check whether the server is asking to extend the timer.
   if (response->has_override_message_timeout()) {
-    onNewTimeout(DurationUtil::durationToMilliseconds(response->override_message_timeout()));
+    onNewTimeout(response->override_message_timeout());
     return;
   }
 

source/extensions/filters/http/ext_proc/ext_proc.h

@@ -191,7 +191,7 @@ class Filter : public Logger::Loggable<Logger::Id::ext_proc>,
   void onGrpcClose() override;
 
   void onMessageTimeout();
-  void onNewTimeout(const uint32_t message_timeout_ms);
+  void onNewTimeout(const ProtobufWkt::Duration& override_message_timeout);
 
   void sendBufferedData(ProcessorState& state, ProcessorState::CallbackState new_state,
                         bool end_stream) {

test/common/protobuf/utility_test.cc

@@ -1705,6 +1705,63 @@ TEST(DurationUtilTest, OutOfRange) {
   }
 }
 
+TEST(DurationUtilTest, NoThrow) {
+  {
+    // In range test
+    ProtobufWkt::Duration duration;
+    duration.set_seconds(5);
+    duration.set_nanos(10000000);
+    bool error = false;
+    auto ms = DurationUtil::durationToMillisecondsNoThrow(duration, error);
+    EXPECT_FALSE(error);
+    EXPECT_TRUE(ms == 5010);
+  }
+
+  // Below are out-of-range tests
+  {
+    ProtobufWkt::Duration duration;
+    duration.set_seconds(-1);
+    bool error = false;
+    auto ms = DurationUtil::durationToMillisecondsNoThrow(duration, error);
+    EXPECT_TRUE(error);
+    EXPECT_TRUE(ms == 0);
+  }
+  {
+    ProtobufWkt::Duration duration;
+    duration.set_nanos(-1);
+    bool error = false;
+    auto ms = DurationUtil::durationToMillisecondsNoThrow(duration, error);
+    EXPECT_TRUE(error);
+    EXPECT_TRUE(ms == 0);
+  }
+  {
+    ProtobufWkt::Duration duration;
+    duration.set_nanos(1000000000);
+    bool error = false;
+    auto ms = DurationUtil::durationToMillisecondsNoThrow(duration, error);
+    EXPECT_TRUE(error);
+    EXPECT_TRUE(ms == 0);
+  }
+  {
+    ProtobufWkt::Duration duration;
+    duration.set_seconds(Protobuf::util::TimeUtil::kDurationMaxSeconds + 1);
+    bool error = false;
+    auto ms = DurationUtil::durationToMillisecondsNoThrow(duration, error);
+    EXPECT_TRUE(error);
+    EXPECT_TRUE(ms == 0);
+  }
+  {
+    ProtobufWkt::Duration duration;
+    constexpr int64_t kMaxInt64Nanoseconds =
+        std::numeric_limits<int64_t>::max() / (1000 * 1000 * 1000);
+    duration.set_seconds(kMaxInt64Nanoseconds + 1);
+    bool error = false;
+    auto ms = DurationUtil::durationToMillisecondsNoThrow(duration, error);
+    EXPECT_TRUE(error);
+    EXPECT_TRUE(ms == 0);
+  }
+}
+
 // Verify WIP accounting of the file based annotations. This test uses the strict validator to test
 // that code path.
 TEST_F(ProtobufUtilityTest, MessageInWipFile) {

test/extensions/filters/http/ext_proc/ext_proc_integration_test.cc

@@ -315,7 +315,7 @@ class ExtProcIntegrationTest : public HttpIntegrationTest,
 
   // ext_proc server sends back a response to tell Envoy to stop the
   // original timer and start a new timer.
-  void serverSendNewTimeout(const uint32_t timeout_ms) {
+  void serverSendNewTimeout(const uint64_t timeout_ms) {
     ProcessingResponse response;
     if (timeout_ms < 1000) {
       response.mutable_override_message_timeout()->set_nanos(timeout_ms * 1000000);
@@ -327,7 +327,7 @@ class ExtProcIntegrationTest : public HttpIntegrationTest,
 
   // The new timeout message is ignored by Envoy due to different reasons, like
   // new_timeout setting is out-of-range, or max_message_timeout is not configured.
-  void newTimeoutWrongConfigTest(const uint32_t timeout_ms) {
+  void newTimeoutWrongConfigTest(const uint64_t timeout_ms) {
     // Set envoy filter timeout to be 200ms.
     proto_config_.mutable_message_timeout()->set_nanos(200000000);
     // Config max_message_timeout proto to enable the new timeout API.
@@ -1960,4 +1960,13 @@ TEST_P(ExtProcIntegrationTest, RequestMessageNewTimeoutNegativeTestTimeoutNotAcc
   newTimeoutWrongConfigTest(500);
 }
 
+// Send the new timeout to be an extremely large number, which is out-of-range of duration.
+// Verify the code appropriately handled it.
+TEST_P(ExtProcIntegrationTest, RequestMessageNewTimeoutOutOfBounds) {
+  // Config max_message_timeout proto to 100ms to enable the new timeout API.
+  max_message_timeout_ms_ = 100;
+  const uint64_t override_message_timeout = 1000000000000000;
+  newTimeoutWrongConfigTest(override_message_timeout);
+}
+
 } // namespace Envoy