Skip to content

Commit

Permalink
http: removing the legacy trusted_forwarded_proto behavior (#9186)
Browse files Browse the repository at this point in the history
Risk Level: Low (removing legacy behavior, which was runtime-flipped off)
Testing: n/a
Docs Changes: n/a
Release Notes: n/a
Fixes #8842

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
  • Loading branch information
alyssawilk authored Dec 3, 2019
1 parent 911046a commit 2883067
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 33 deletions.
17 changes: 3 additions & 14 deletions source/common/http/conn_manager_utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,6 @@ Network::Address::InstanceConstSharedPtr ConnectionManagerUtility::mutateRequest
Network::Address::InstanceConstSharedPtr final_remote_address;
bool single_xff_address;
const uint32_t xff_num_trusted_hops = config.xffNumTrustedHops();
const bool trusted_forwarded_proto =
Runtime::runtimeFeatureEnabled("envoy.reloadable_features.trusted_forwarded_proto");

if (config.useRemoteAddress()) {
single_xff_address = request_headers.ForwardedFor() == nullptr;
Expand All @@ -113,18 +111,9 @@ Network::Address::InstanceConstSharedPtr ConnectionManagerUtility::mutateRequest
Utility::appendXff(request_headers, *connection.remoteAddress());
}
}
if (trusted_forwarded_proto) {
// If the prior hop is not a trusted proxy, overwrite any x-forwarded-proto value it set as
// untrusted. Alternately if no x-forwarded-proto header exists, add one.
if (xff_num_trusted_hops == 0 || request_headers.ForwardedProto() == nullptr) {
request_headers.setReferenceForwardedProto(connection.ssl()
? Headers::get().SchemeValues.Https
: Headers::get().SchemeValues.Http);
}
} else {
// Previously, before the trusted_forwarded_proto logic, Envoy would always overwrite the
// x-forwarded-proto header even if it was set by a trusted proxy. This code path is
// deprecated and will be removed.
// If the prior hop is not a trusted proxy, overwrite any x-forwarded-proto value it set as
// untrusted. Alternately if no x-forwarded-proto header exists, add one.
if (xff_num_trusted_hops == 0 || request_headers.ForwardedProto() == nullptr) {
request_headers.setReferenceForwardedProto(
connection.ssl() ? Headers::get().SchemeValues.Https : Headers::get().SchemeValues.Http);
}
Expand Down
1 change: 0 additions & 1 deletion source/common/runtime/runtime_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ constexpr const char* runtime_features[] = {
"envoy.reloadable_features.test_feature_true",
"envoy.reloadable_features.strict_header_validation",
"envoy.reloadable_features.buffer_filter_populate_content_length",
"envoy.reloadable_features.trusted_forwarded_proto",
"envoy.reloadable_features.outlier_detection_support_for_grpc_status",
"envoy.reloadable_features.connection_header_sanitization",
};
Expand Down
18 changes: 0 additions & 18 deletions test/common/http/conn_manager_utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -241,26 +241,8 @@ TEST_F(ConnectionManagerUtilityTest, SkipXffAppendPassThruUseRemoteAddress) {
EXPECT_EQ("198.51.100.1", headers.ForwardedFor()->value().getStringView());
}

TEST_F(ConnectionManagerUtilityTest, ForwardedProtoLegacyBehavior) {
TestScopedRuntime scoped_runtime;
Runtime::LoaderSingleton::getExisting()->mergeValues(
{{"envoy.reloadable_features.trusted_forwarded_proto", "false"}});

ON_CALL(config_, useRemoteAddress()).WillByDefault(Return(true));
ON_CALL(config_, xffNumTrustedHops()).WillByDefault(Return(1));
EXPECT_CALL(config_, skipXffAppend()).WillOnce(Return(true));
connection_.remote_address_ = std::make_shared<Network::Address::Ipv4Instance>("12.12.12.12");
ON_CALL(config_, useRemoteAddress()).WillByDefault(Return(true));
TestHeaderMapImpl headers{{"x-forwarded-proto", "https"}};

callMutateRequestHeaders(headers, Protocol::Http2);
EXPECT_EQ("http", headers.ForwardedProto()->value().getStringView());
}

TEST_F(ConnectionManagerUtilityTest, PreserveForwardedProtoWhenInternal) {
TestScopedRuntime scoped_runtime;
Runtime::LoaderSingleton::getExisting()->mergeValues(
{{"envoy.reloadable_features.trusted_forwarded_proto", "true"}});

ON_CALL(config_, useRemoteAddress()).WillByDefault(Return(true));
ON_CALL(config_, xffNumTrustedHops()).WillByDefault(Return(1));
Expand Down

0 comments on commit 2883067

Please sign in to comment.