Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

upstream: require opt-in for the x-envoy-original-dst-host header. #4046

Merged
merged 2 commits into from
Aug 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion api/envoy/api/v2/cds.proto
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ service ClusterDiscoveryService {
// [#protodoc-title: Clusters]

// Configuration for a single upstream cluster.
// [#comment:next free field: 34]
// [#comment:next free field: 35]
message Cluster {
// Supplies the name of the cluster which must be unique across all clusters.
// The cluster name is used when emitting
Expand Down Expand Up @@ -394,6 +394,22 @@ message Cluster {
DeprecatedV1 deprecated_v1 = 2 [deprecated = true];
}

// Specific configuration for the
// :ref:`Original Destination <arch_overview_load_balancing_types_original_destination>`
// load balancing policy.
message OriginalDstLbConfig {
// When true, :ref:`x-envoy-orignal-dst-host
// <config_http_conn_man_headers_x-envoy-original-dst-host>` can be used to override destination
// address.
//
// .. attention::
//
// This header isn't sanitized by default, so enabling this feature allows HTTP clients to
// route traffic to arbitrary hosts and/or ports, which may have serious security
// consequences.
bool use_http_header = 1;
}

// Optional configuration for the load balancing algorithm selected by
// LbPolicy. Currently only
// :ref:`RING_HASH<envoy_api_enum_value_Cluster.LbPolicy.RING_HASH>`
Expand All @@ -404,6 +420,8 @@ message Cluster {
oneof lb_config {
// Optional configuration for the Ring Hash load balancing policy.
RingHashLbConfig ring_hash_lb_config = 23;
// Optional configuration for the Original Destination load balancing policy.
OriginalDstLbConfig original_dst_lb_config = 34;
}

// Common configuration for all load balancer implementations.
Expand Down
12 changes: 12 additions & 0 deletions docs/root/configuration/http_conn_man/headers.rst
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,18 @@ the header value to *true*.

This is a convenience to avoid having to parse and understand XFF.

.. _config_http_conn_man_headers_x-envoy-original-dst-host:

x-envoy-original-dst-host
-------------------------

The header used to override destination address when using the
:ref:`Original Destination <arch_overview_load_balancing_types_original_destination>`
load balancing policy.

It is ignored, unless the use of it is enabled via
:ref:`use_http_header <envoy_api_field_Cluster.OriginalDstLbConfig.use_http_header>`.

.. _config_http_conn_man_headers_x-forwarded-client-cert:

x-forwarded-client-cert
Expand Down
3 changes: 2 additions & 1 deletion docs/root/intro/arch_overview/load_balancing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ be used with original destination clusters.

Original destination host request header
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Envoy can also pick up the original destination from a HTTP header called ``x-envoy-original-destination-host``.
Envoy can also pick up the original destination from a HTTP header called
:ref:`x-envoy-orignal-dst-host <config_http_conn_man_headers_x-envoy-original-dst-host>`.
Please note that fully resolved IP address should be passed in this header. For example if a request has to be
routed to a host with IP address 10.195.16.237 at port 8888, the request header value should be set as
``10.195.16.237:8888``.
Expand Down
3 changes: 3 additions & 0 deletions docs/root/intro/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ Version history
* thrift_proxy: introduced thrift routing, moved configuration to correct location
* upstream: added configuration option to the subset load balancer to take locality weights into account when
selecting a host from a subset.
* upstream: require opt-in to use the :ref:`x-envoy-orignal-dst-host <config_http_conn_man_headers_x-envoy-original-dst-host>` header
for overriding destination address when using the :ref:`Original Destination <arch_overview_load_balancing_types_original_destination>`
load balancing policy.

1.7.0
===============
Expand Down
8 changes: 8 additions & 0 deletions include/envoy/upstream/upstream.h
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,14 @@ class ClusterInfo {
virtual const absl::optional<envoy::api::v2::Cluster::RingHashLbConfig>&
lbRingHashConfig() const PURE;

/**
* @return const absl::optional<envoy::api::v2::Cluster::OriginalDstLbConfig>& the configuration
* for the Original Destination load balancing policy, only used if type is set to
* ORIGINAL_DST_LB.
*/
virtual const absl::optional<envoy::api::v2::Cluster::OriginalDstLbConfig>&
lbOriginalDstConfig() const PURE;

/**
* @return Whether the cluster is currently in maintenance mode and should not be routed to.
* Different filters may handle this situation in different ways. The implementation
Expand Down
3 changes: 2 additions & 1 deletion source/common/upstream/cluster_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1062,7 +1062,8 @@ ClusterManagerImpl::ThreadLocalClusterManagerImpl::ClusterEntry::ClusterEntry(
case LoadBalancerType::OriginalDst: {
ASSERT(lb_factory_ == nullptr);
lb_.reset(new OriginalDstCluster::LoadBalancer(
priority_set_, parent.parent_.active_clusters_.at(cluster->name())->cluster_));
priority_set_, parent.parent_.active_clusters_.at(cluster->name())->cluster_,
cluster->lbOriginalDstConfig()));
break;
}
}
Expand Down
11 changes: 8 additions & 3 deletions source/common/upstream/original_dst_cluster.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ namespace Upstream {
// OriginalDstCluster::LoadBalancer is never configured with any other type of cluster,
// and throws an exception otherwise.

OriginalDstCluster::LoadBalancer::LoadBalancer(PrioritySet& priority_set, ClusterSharedPtr& parent)
OriginalDstCluster::LoadBalancer::LoadBalancer(
PrioritySet& priority_set, ClusterSharedPtr& parent,
const absl::optional<envoy::api::v2::Cluster::OriginalDstLbConfig>& config)
: priority_set_(priority_set), parent_(std::static_pointer_cast<OriginalDstCluster>(parent)),
info_(parent->info()) {
info_(parent->info()), use_http_header_(config ? config.value().use_http_header() : false) {
// priority_set_ is initially empty.
priority_set_.addMemberUpdateCb(
[this](uint32_t, const HostVector& hosts_added, const HostVector& hosts_removed) -> void {
Expand All @@ -44,7 +46,10 @@ HostConstSharedPtr OriginalDstCluster::LoadBalancer::chooseHost(LoadBalancerCont
if (context) {

// Check if override host header is present, if yes use it otherwise check local address.
Network::Address::InstanceConstSharedPtr dst_host = requestOverrideHost(context);
Network::Address::InstanceConstSharedPtr dst_host = nullptr;
if (use_http_header_) {
dst_host = requestOverrideHost(context);
}
if (dst_host == nullptr) {
const Network::Connection* connection = context->downstreamConnection();
// The local address of the downstream connection is the original destination address,
Expand Down
4 changes: 3 additions & 1 deletion source/common/upstream/original_dst_cluster.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ class OriginalDstCluster : public ClusterImplBase {
*/
class LoadBalancer : public Upstream::LoadBalancer {
public:
LoadBalancer(PrioritySet& priority_set, ClusterSharedPtr& parent);
LoadBalancer(PrioritySet& priority_set, ClusterSharedPtr& parent,
const absl::optional<envoy::api::v2::Cluster::OriginalDstLbConfig>& config);

// Upstream::LoadBalancer
HostConstSharedPtr chooseHost(LoadBalancerContext* context) override;
Expand Down Expand Up @@ -98,6 +99,7 @@ class OriginalDstCluster : public ClusterImplBase {
PrioritySet& priority_set_; // Thread local priority set.
std::weak_ptr<OriginalDstCluster> parent_; // Primary cluster managed by the main thread.
ClusterInfoConstSharedPtr info_;
const bool use_http_header_;
HostMap host_map_;
};

Expand Down
4 changes: 2 additions & 2 deletions source/common/upstream/upstream_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,8 @@ ClusterInfoImpl::ClusterInfoImpl(const envoy::api::v2::Cluster& config,
resource_managers_(config, runtime, name_),
maintenance_mode_runtime_key_(fmt::format("upstream.maintenance_mode.{}", name_)),
source_address_(getSourceAddress(config, bind_config)),
lb_ring_hash_config_(envoy::api::v2::Cluster::RingHashLbConfig(config.ring_hash_lb_config())),
added_via_api_(added_via_api),
lb_ring_hash_config_(config.ring_hash_lb_config()),
lb_original_dst_config_(config.original_dst_lb_config()), added_via_api_(added_via_api),
lb_subset_(LoadBalancerSubsetInfoImpl(config.lb_subset_config())),
metadata_(config.metadata()), common_lb_config_(config.common_lb_config()),
cluster_socket_options_(parseClusterSocketOptions(config, bind_config)),
Expand Down
5 changes: 5 additions & 0 deletions source/common/upstream/upstream_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,10 @@ class ClusterInfoImpl : public ClusterInfo {
lbRingHashConfig() const override {
return lb_ring_hash_config_;
}
const absl::optional<envoy::api::v2::Cluster::OriginalDstLbConfig>&
lbOriginalDstConfig() const override {
return lb_original_dst_config_;
}
bool maintenanceMode() const override;
uint64_t maxRequestsPerConnection() const override { return max_requests_per_connection_; }
const std::string& name() const override { return name_; }
Expand Down Expand Up @@ -412,6 +416,7 @@ class ClusterInfoImpl : public ClusterInfo {
const Network::Address::InstanceConstSharedPtr source_address_;
LoadBalancerType lb_type_;
absl::optional<envoy::api::v2::Cluster::RingHashLbConfig> lb_ring_hash_config_;
absl::optional<envoy::api::v2::Cluster::OriginalDstLbConfig> lb_original_dst_config_;
const bool added_via_api_;
LoadBalancerSubsetInfoImpl lb_subset_;
const envoy::api::v2::core::Metadata metadata_;
Expand Down
Loading