HTTP: add allow_obs_text configuration to HTTP/2 and HTTP/3 protocol options (#43971)

<!--
!!!ATTENTION!!!

If you are fixing *any* crash or *any* potential security issue, *do
not*
open a pull request in this repo. Please report the issue via emailing
envoy-security@googlegroups.com where the issue will be triaged
appropriately.
Thank you in advance for helping to keep Envoy secure.

!!!ATTENTION!!!

For an explanation of how to fill out the fields, please see the
relevant section
in
[PULL_REQUESTS.md](https://github.com/envoyproxy/envoy/blob/main/PULL_REQUESTS.md)

!!!ATTENTION!!!

Please check the [use of generative AI
policy](https://github.com/envoyproxy/envoy/blob/main/CONTRIBUTING.md?plain=1#L41).

You may use generative AI only if you fully understand the code. You
need to disclose
this usage in the PR description to ensure transparency.
-->

Commit Message: HTTP: add allow_obs_text configuration to HTTP/2 and
HTTP/3 protocol options
Additional Description: This change introduces a new configuration knob,
allow_obs_text, to the Envoy HTTP/2 and HTTP/3 protocol options to
control the validation of 'obs-text' characters (0x80-0xFF) in header
field values. By default, this option is enabled to maintain consistency
with existing Shinkansen behaviors and to avoid breaking legacy clients
that rely on these characters. The implementation updates the underlying
codec logic for both protocols to respect this setting during header
validation. Comprehensive unit tests have been added for both HTTP/2
(OgHttp2) and HTTP/3 to verify that headers containing obsolete text are
accepted or rejected as expected based on the configuration.
Risk Level: low
Testing: added unit tests
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional [API
Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):]

---------

Signed-off-by: Ting Pan <panting@google.com>

Author: fishpan1209

api/envoy/config/core/v3/protocol.proto

@@ -541,7 +541,7 @@ message KeepaliveSettings {
       [(validate.rules).duration = {gte {nanos: 1000000}}];
 }
 
-// [#next-free-field: 20]
+// [#next-free-field: 21]
 message Http2ProtocolOptions {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.api.v2.core.Http2ProtocolOptions";
@@ -774,6 +774,12 @@ message Http2ProtocolOptions {
   //   request failures.
   google.protobuf.UInt32Value max_header_field_size_kb = 19
       [(validate.rules).uint32 = {lte: 256 gte: 64}];
+
+  // Whether to disallow obsolete text for oghttp2 in header field values.
+  // If not set, it defaults to false.
+  // From RFC 9110, https://www.rfc-editor.org/rfc/rfc9110.html#section-5.5:
+  // obs-text = %x80-FF
+  google.protobuf.BoolValue disallow_obs_text = 20;
 }
 
 // [#not-implemented-hide:]
@@ -785,7 +791,7 @@ message GrpcProtocolOptions {
 }
 
 // A message which allows using HTTP/3.
-// [#next-free-field: 9]
+// [#next-free-field: 10]
 message Http3ProtocolOptions {
   QuicProtocolOptions quic_protocol_options = 1;
 
@@ -827,6 +833,12 @@ message Http3ProtocolOptions {
   // Disables connection level flow control for HTTP/3 streams. This is useful in situations where the streams share the same connection
   // but originate from different end-clients, so that each stream can make progress independently at non-front-line proxies.
   bool disable_connection_flow_control_for_streams = 8;
+
+  // Whether to disallow obsolete text in header field values.
+  // If not set, it defaults to true for alignment with current behavior.
+  // As defined in RFC 9110, https://www.rfc-editor.org/rfc/rfc9110.html#section-5.5:
+  // an obs-text character is a character in the range %x80-FF
+  google.protobuf.BoolValue disallow_obs_text = 9;
 }
 
 // A message to control transformations to the :scheme header

source/common/http/http2/codec_impl.cc

@@ -2082,6 +2082,8 @@ ConnectionImpl::Http2Options::Http2Options(
   og_options_.max_header_field_size = max_headers_kb * 1024;
   og_options_.allow_extended_connect = http2_options.allow_connect();
   og_options_.allow_different_host_and_authority = true;
+  og_options_.allow_obs_text =
+      !PROTOBUF_GET_WRAPPED_OR_DEFAULT(http2_options, disallow_obs_text, false);
   if (!PROTOBUF_GET_WRAPPED_OR_DEFAULT(http2_options, enable_huffman_encoding, true)) {
     if (http2_options.has_hpack_table_size() && http2_options.hpack_table_size().value() == 0) {
       og_options_.compression_option = http2::adapter::OgHttp2Session::Options::DISABLE_COMPRESSION;

source/common/quic/envoy_quic_stream.h

@@ -9,6 +9,7 @@
 #include "envoy/http/codec.h"
 
 #include "source/common/http/codec_helper.h"
+#include "source/common/protobuf/utility.h"
 #include "source/common/quic/envoy_quic_simulated_watermark_buffer.h"
 #include "source/common/quic/envoy_quic_utils.h"
 
@@ -50,6 +51,9 @@ class EnvoyQuicStream : public virtual Http::StreamEncoder,
     if (http3_options_.disable_connection_flow_control_for_streams()) {
       quic_stream_.DisableConnectionFlowControlForThisStream();
     }
+    if (!PROTOBUF_GET_WRAPPED_OR_DEFAULT(http3_options_, disallow_obs_text, true)) {
+      header_validator_.SetObsTextOption(http2::adapter::ObsTextOption::kAllow);
+    }
   }
 
   ~EnvoyQuicStream() override = default;

test/common/http/http2/codec_impl_test.cc

@@ -135,6 +135,12 @@ class Http2CodecImplTestFixture {
     return stream_ids;
   }
 
+  static std::unique_ptr<ConnectionImpl::Http2Options>
+  createHttp2Options(const envoy::config::core::v3::Http2ProtocolOptions& http2_options,
+                     uint32_t max_headers_kb) {
+    return std::make_unique<ConnectionImpl::Http2Options>(http2_options, max_headers_kb);
+  }
+
   static Http2SettingsTuple smallWindowHttp2Settings() {
     return std::make_tuple(CommonUtility::OptionsLimits::DEFAULT_HPACK_TABLE_SIZE,
                            CommonUtility::OptionsLimits::DEFAULT_MAX_CONCURRENT_STREAMS,
@@ -211,11 +217,13 @@ class Http2CodecImplTestFixture {
 
     // Ensure that tests driveToCompletion(). Some tests set `expect_buffered_data_on_teardown_` to
     // indicate that they purposefully leave buffered data.
-    if (expect_buffered_data_on_teardown_) {
-      EXPECT_TRUE(client_wrapper_->buffer_.length() > 0 || server_wrapper_->buffer_.length() > 0);
-    } else {
-      EXPECT_EQ(client_wrapper_->buffer_.length(), 0);
-      EXPECT_EQ(server_wrapper_->buffer_.length(), 0);
+    if (client_wrapper_ != nullptr && server_wrapper_ != nullptr) {
+      if (expect_buffered_data_on_teardown_) {
+        EXPECT_TRUE(client_wrapper_->buffer_.length() > 0 || server_wrapper_->buffer_.length() > 0);
+      } else {
+        EXPECT_EQ(client_wrapper_->buffer_.length(), 0);
+        EXPECT_EQ(server_wrapper_->buffer_.length(), 0);
+      }
     }
   }
 
@@ -570,6 +578,59 @@ class Http2CodecImplTest : public ::testing::TestWithParam<Http2SettingsTestPara
   }
 };
 
+TEST_P(Http2CodecImplTest, DisallowObsTextBehaviorDisallow) {
+  if (skipForUhv() || http2_implementation_ != Http2Impl::Oghttp2) {
+    return;
+  }
+
+  // With disallow_obs_text = true, the request is rejected.
+  server_http2_options_.mutable_disallow_obs_text()->set_value(true);
+  initialize();
+
+  InSequence s;
+  TestRequestHeaderMapImpl request_headers;
+  HttpTestUtility::addDefaultHeaders(request_headers);
+
+  HeaderString header_name;
+  header_name.setCopy("custom-header");
+  HeaderString header_value;
+  setHeaderStringUnvalidated(header_value, "foo\x80");
+  request_headers.addViaMove(std::move(header_name), std::move(header_value));
+
+  // We don't expect onResetStream because the error might be detected before the stream is fully
+  // established on the server.
+  EXPECT_TRUE(request_encoder_->encodeHeaders(request_headers, true).ok());
+  driveToCompletion();
+  EXPECT_FALSE(server_wrapper_->status_.ok());
+  // Drain the buffer as we expect a connection error and some data might be left.
+  server_wrapper_->buffer_.drain(server_wrapper_->buffer_.length());
+}
+
+TEST_P(Http2CodecImplTest, DisallowObsTextBehaviorAllow) {
+  if (skipForUhv() || http2_implementation_ != Http2Impl::Oghttp2) {
+    return;
+  }
+
+  // With disallow_obs_text = false, the request is accepted.
+  server_http2_options_.mutable_disallow_obs_text()->set_value(false);
+  initialize();
+
+  InSequence s;
+  TestRequestHeaderMapImpl request_headers;
+  HttpTestUtility::addDefaultHeaders(request_headers);
+
+  HeaderString header_name;
+  header_name.setCopy("custom-header");
+  HeaderString header_value;
+  setHeaderStringUnvalidated(header_value, "foo\x80");
+  request_headers.addViaMove(std::move(header_name), std::move(header_value));
+
+  EXPECT_CALL(request_decoder_, decodeHeaders_(_, true));
+  EXPECT_TRUE(request_encoder_->encodeHeaders(request_headers, true).ok());
+  driveToCompletion();
+  EXPECT_TRUE(server_wrapper_->status_.ok());
+}
+
 TEST_P(Http2CodecImplTest, SimpleRequestResponse) {
   initialize();
 

test/common/quic/envoy_quic_server_stream_test.cc

@@ -984,5 +984,42 @@ TEST_F(EnvoyQuicServerStreamTest, DuplicatedPathHeader) {
   quic_stream_->OnStreamHeaderList(true, 0, header_list);
 }
 
+TEST_F(EnvoyQuicServerStreamTest, DisallowObsTextBehavior) {
+  EXPECT_CALL(stream_callbacks_, onResetStream(_, _)).Times(testing::AnyNumber());
+  {
+    // Default behavior is to disallow obs text.
+    envoy::config::core::v3::Http3ProtocolOptions options;
+    auto stream = std::make_unique<EnvoyQuicServerStream>(
+        stream_id_ + 12, &quic_session_, quic::BIDIRECTIONAL, stats_, options,
+        envoy::config::core::v3::HttpProtocolOptions::ALLOW);
+    // \x80 is obsolete text
+    EXPECT_EQ(Http::HeaderUtility::HeaderValidationResult::REJECT,
+              stream->validateHeader("custom-header", "foo\x80"));
+    EXPECT_EQ(Http::HeaderUtility::HeaderValidationResult::ACCEPT,
+              stream->validateHeader("custom-header", "foo"));
+  }
+  {
+    envoy::config::core::v3::Http3ProtocolOptions options;
+    options.mutable_disallow_obs_text()->set_value(true);
+    auto stream = std::make_unique<EnvoyQuicServerStream>(
+        stream_id_ + 16, &quic_session_, quic::BIDIRECTIONAL, stats_, options,
+        envoy::config::core::v3::HttpProtocolOptions::ALLOW);
+    // \x80 is obsolete text
+    EXPECT_EQ(Http::HeaderUtility::HeaderValidationResult::REJECT,
+              stream->validateHeader("custom-header", "foo\x80"));
+    EXPECT_EQ(Http::HeaderUtility::HeaderValidationResult::ACCEPT,
+              stream->validateHeader("custom-header", "foo"));
+  }
+  {
+    envoy::config::core::v3::Http3ProtocolOptions options;
+    options.mutable_disallow_obs_text()->set_value(false);
+    auto stream = std::make_unique<EnvoyQuicServerStream>(
+        stream_id_ + 20, &quic_session_, quic::BIDIRECTIONAL, stats_, options,
+        envoy::config::core::v3::HttpProtocolOptions::ALLOW);
+    EXPECT_EQ(Http::HeaderUtility::HeaderValidationResult::ACCEPT,
+              stream->validateHeader("custom-header", "foo\x80"));
+  }
+}
+
 } // namespace Quic
 } // namespace Envoy

test/integration/default_header_validator_integration_test.cc

@@ -80,7 +80,7 @@ class DownstreamUhvIntegrationTest : public HttpProtocolIntegrationTest {
     // All codecs allow the following characters that are outside of RFC "<>[]^`{}\|
     std::string additionally_allowed_characters(R"--("<>[]^`{}\|)--");
     if (downstream_protocol_ == Http::CodecType::HTTP2) {
-      // Both nghttp2 and oghttp2 allow extended ASCII >= 0x80 in path
+      // Both nghttp2 and oghttp2 allow extended ASCII >= 0x80 in path.
       additionally_allowed_characters += generateExtendedAsciiString();
     }
     return additionally_allowed_characters;