Skip to content

Commit

Permalink
more
Browse files Browse the repository at this point in the history
Signed-off-by: Matt Klein <[email protected]>
  • Loading branch information
mattklein123 committed Dec 19, 2020
1 parent 980eb6b commit d38f963
Show file tree
Hide file tree
Showing 44 changed files with 376 additions and 328 deletions.
3 changes: 2 additions & 1 deletion include/envoy/network/connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "envoy/network/address.h"
#include "envoy/network/filter.h"
#include "envoy/network/listen_socket.h"
#include "envoy/network/socket.h"
#include "envoy/ssl/connection.h"
#include "envoy/stream_info/stream_info.h"

Expand Down Expand Up @@ -189,6 +190,7 @@ class Connection : public Event::DeferredDeletable, public FilterManager {

// fixfix
virtual const SocketAddressProvider& addressProvider() const PURE;
virtual SocketAddressProviderConstSharedPtr addressProviderSharedPtr() const PURE;

/**
* Credentials of the peer of a socket as decided by SO_PEERCRED.
Expand Down Expand Up @@ -270,7 +272,6 @@ class Connection : public Event::DeferredDeletable, public FilterManager {
*/
virtual uint32_t bufferLimit() const PURE;


/**
* @return boolean telling if the connection is currently above the high watermark.
*/
Expand Down
4 changes: 2 additions & 2 deletions include/envoy/network/socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class SocketAddressProvider {
*/
virtual void setLocalAddress(const Address::InstanceConstSharedPtr& local_address) PURE;

/**
/**
* Restores the local address of the socket. On accepted sockets the local address defaults to the
* one at which the connection was received at, which is the same as the listener's address, if
* the listener is bound to a specific address. Call this to restore the address to a value
Expand Down Expand Up @@ -139,7 +139,7 @@ class Socket {
*/
virtual Socket::Type socketType() const PURE;

/**
/**
* @return the type (IP or pipe) of addresses used by the socket (subset of socket domain)
*/
virtual Address::Type addressType() const PURE;
Expand Down
1 change: 0 additions & 1 deletion source/common/http/async_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#include <string>
#include <vector>

#include "common/network/socket_impl.h"
#include "envoy/config/core/v3/base.pb.h"

#include "common/grpc/common.h"
Expand Down
16 changes: 6 additions & 10 deletions source/common/http/conn_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -618,12 +618,6 @@ ConnectionManagerImpl::ActiveStream::ActiveStream(ConnectionManagerImpl& connect
} else {
connection_manager_.stats_.named_.downstream_rq_http1_total_.inc();
}
// Initially, the downstream remote address is the source address of the
// downstream connection. That can change later in the request's lifecycle,
// based on XFF processing, but setting the downstream remote address here
// prevents surprises for logging code in edge cases.
filter_manager_.streamInfo().setDownstreamAddresses(
connection_manager_.read_callbacks_->connection());

filter_manager_.streamInfo().setDownstreamSslConnection(
connection_manager_.read_callbacks_->connection().ssl());
Expand Down Expand Up @@ -822,7 +816,7 @@ const Network::Connection* ConnectionManagerImpl::ActiveStream::connection() {
}

uint32_t ConnectionManagerImpl::ActiveStream::localPort() {
auto ip = connection()->localAddress()->ip();
auto ip = connection()->addressProvider().localAddress()->ip();
if (ip == nullptr) {
return 0;
}
Expand Down Expand Up @@ -1010,12 +1004,14 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(RequestHeaderMapPtr&& he

if (!state_.is_internally_created_) { // Only sanitize headers on first pass.
// Modify the downstream remote address depending on configuration and headers.
filter_manager_.streamInfo().setDownstreamRemoteAddress(
ASSERT(false); // fixfix
/*filter_manager_.streamInfo().setDownstreamRemoteAddress(
ConnectionManagerUtility::mutateRequestHeaders(
*request_headers_, connection_manager_.read_callbacks_->connection(),
connection_manager_.config_, *snapped_route_config_, connection_manager_.local_info_));
connection_manager_.config_, *snapped_route_config_,
connection_manager_.local_info_));*/
}
ASSERT(filter_manager_.streamInfo().downstreamRemoteAddress() != nullptr);
ASSERT(filter_manager_.streamInfo().downstreamAddressProvider().remoteAddress() != nullptr);

ASSERT(!cached_route_);
refreshCachedRoute();
Expand Down
8 changes: 4 additions & 4 deletions source/common/http/conn_manager_utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,13 @@ Network::Address::InstanceConstSharedPtr ConnectionManagerUtility::mutateRequest
// are but they didn't populate XFF properly, the trusted client address is the
// source address of the immediate downstream's connection to us.
if (final_remote_address == nullptr) {
final_remote_address = connection.remoteAddress();
final_remote_address = connection.addressProvider().remoteAddress();
}
if (!config.skipXffAppend()) {
if (Network::Utility::isLoopbackAddress(*connection.remoteAddress())) {
if (Network::Utility::isLoopbackAddress(*connection.addressProvider().remoteAddress())) {
Utility::appendXff(request_headers, config.localAddress());
} else {
Utility::appendXff(request_headers, *connection.remoteAddress());
Utility::appendXff(request_headers, *connection.addressProvider().remoteAddress());
}
}
// If the prior hop is not a trusted proxy, overwrite any x-forwarded-proto value it set as
Expand Down Expand Up @@ -157,7 +157,7 @@ Network::Address::InstanceConstSharedPtr ConnectionManagerUtility::mutateRequest
// After determining internal request status, if there is no final remote address, due to no XFF,
// busted XFF, etc., use the direct connection remote address for logging.
if (final_remote_address == nullptr) {
final_remote_address = connection.remoteAddress();
final_remote_address = connection.addressProvider().remoteAddress();
}

// Edge request is the request from external clients to front Envoy.
Expand Down
3 changes: 2 additions & 1 deletion source/common/http/filter_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,8 @@ class FilterManager : public ScopeTrackedObject,
connection_(connection), stream_id_(stream_id), proxy_100_continue_(proxy_100_continue),
buffer_limit_(buffer_limit), filter_chain_factory_(filter_chain_factory),
local_reply_(local_reply),
stream_info_(protocol, time_source, parent_filter_state, filter_state_life_span) {}
stream_info_(protocol, time_source, connection.addressProviderSharedPtr(),
parent_filter_state, filter_state_life_span) {}
~FilterManager() override {
ASSERT(state_.destroyed_);
ASSERT(state_.filter_call_state_ == 0);
Expand Down
3 changes: 3 additions & 0 deletions source/common/network/connection_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ class ConnectionImpl : public ConnectionImplBase, public TransportSocketCallback
const SocketAddressProvider& addressProvider() const override {
return socket_->addressProvider();
}
SocketAddressProviderConstSharedPtr addressProviderSharedPtr() const override {
return socket_->addressProviderSharedPtr();
}
absl::optional<UnixDomainSocketPeerCredentials> unixSocketPeerCredentials() const override;
Ssl::ConnectionInfoConstSharedPtr ssl() const override { return transport_socket_->ssl(); }
State state() const override;
Expand Down
2 changes: 1 addition & 1 deletion source/common/network/filter_matcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class ListenerFilterDstPortMatcher final : public ListenerFilterMatcher {
explicit ListenerFilterDstPortMatcher(const ::envoy::type::v3::Int32Range& range)
: start_(range.start()), end_(range.end()) {}
bool matches(ListenerFilterCallbacks& cb) const override {
const auto& address = cb.socket().localAddress();
const auto& address = cb.socket().addressProvider().localAddress();
// Match on destination port (only for IP addresses).
if (address->type() == Address::Type::Ip) {
const auto port = address->ip()->port();
Expand Down
6 changes: 3 additions & 3 deletions source/common/network/socket_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ namespace Network {
SocketImpl::SocketImpl(Socket::Type sock_type,
const Address::InstanceConstSharedPtr& address_for_io_handle,
const Address::InstanceConstSharedPtr& remote_address)
: io_handle_(ioHandleForAddr(sock_type, address_for_io_handle)), sock_type_(sock_type),
addr_type_(address_for_io_handle->type()),
address_provider_(std::make_shared<SocketAddressProviderImpl>(nullptr, remote_address)) {}
: io_handle_(ioHandleForAddr(sock_type, address_for_io_handle)),
address_provider_(std::make_shared<SocketAddressProviderImpl>(nullptr, remote_address)),
sock_type_(sock_type), addr_type_(address_for_io_handle->type()) {}

SocketImpl::SocketImpl(IoHandlePtr&& io_handle,
const Address::InstanceConstSharedPtr& local_address,
Expand Down
2 changes: 1 addition & 1 deletion source/common/network/socket_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,10 @@ class SocketImpl : public virtual Socket {
const Address::InstanceConstSharedPtr& remote_address);

const IoHandlePtr io_handle_;
const SocketAddressProviderSharedPtr address_provider_;
OptionsSharedPtr options_;
Socket::Type sock_type_;
Address::Type addr_type_;
const SocketAddressProviderSharedPtr address_provider_;
};

} // namespace Network
Expand Down
13 changes: 11 additions & 2 deletions source/common/stream_info/stream_info_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#include <chrono>
#include <cstdint>

#include "common/common/macros.h"
#include "common/network/socket_impl.h"
#include "envoy/common/time.h"
#include "envoy/config/core/v3/base.pb.h"
#include "envoy/http/header_map.h"
Expand Down Expand Up @@ -301,14 +303,21 @@ struct StreamInfoImpl : public StreamInfo {
std::string route_name_;

private:
// fixfix null check
static Network::SocketAddressProviderConstSharedPtr emptyDownstreamAddressProvider() {
MUTABLE_CONSTRUCT_ON_FIRST_USE(
Network::SocketAddressProviderConstSharedPtr,
std::make_shared<Network::SocketAddressProviderImpl>(nullptr, nullptr));
}

StreamInfoImpl(absl::optional<Http::Protocol> protocol, TimeSource& time_source,
const Network::SocketAddressProviderConstSharedPtr& downstream_address_provider,
FilterStateSharedPtr filter_state)
: time_source_(time_source), start_time_(time_source.systemTime()),
start_time_monotonic_(time_source.monotonicTime()), protocol_(protocol),
filter_state_(std::move(filter_state)),
downstream_address_provider_(downstream_address_provider),
downstream_address_provider_(downstream_address_provider != nullptr
? downstream_address_provider
: emptyDownstreamAddressProvider()),
request_id_extension_(Http::RequestIDExtensionFactory::noopInstance()) {}

uint64_t bytes_received_{};
Expand Down
17 changes: 11 additions & 6 deletions source/common/tcp_proxy/tcp_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,20 +56,24 @@ Config::RouteImpl::RouteImpl(

bool Config::RouteImpl::matches(Network::Connection& connection) const {
if (!source_port_ranges_.empty() &&
!Network::Utility::portInRangeList(*connection.remoteAddress(), source_port_ranges_)) {
!Network::Utility::portInRangeList(*connection.addressProvider().remoteAddress(),
source_port_ranges_)) {
return false;
}

if (!source_ips_.empty() && !source_ips_.contains(*connection.remoteAddress())) {
if (!source_ips_.empty() &&
!source_ips_.contains(*connection.addressProvider().remoteAddress())) {
return false;
}

if (!destination_port_ranges_.empty() &&
!Network::Utility::portInRangeList(*connection.localAddress(), destination_port_ranges_)) {
!Network::Utility::portInRangeList(*connection.addressProvider().localAddress(),
destination_port_ranges_)) {
return false;
}

if (!destination_ips_.empty() && !destination_ips_.contains(*connection.localAddress())) {
if (!destination_ips_.empty() &&
!destination_ips_.contains(*connection.addressProvider().localAddress())) {
return false;
}

Expand Down Expand Up @@ -425,8 +429,9 @@ Network::FilterStatus Filter::initializeUpstreamConnection() {
Network::ProxyProtocolFilterState::key())) {
read_callbacks_->connection().streamInfo().filterState()->setData(
Network::ProxyProtocolFilterState::key(),
std::make_unique<Network::ProxyProtocolFilterState>(Network::ProxyProtocolData{
downstreamConnection()->remoteAddress(), downstreamConnection()->localAddress()}),
std::make_unique<Network::ProxyProtocolFilterState>(
Network::ProxyProtocolData{downstreamConnection()->addressProvider().remoteAddress(),
downstreamConnection()->addressProvider().localAddress()}),
StreamInfo::FilterState::StateType::ReadOnly,
StreamInfo::FilterState::LifeSpan::Connection);
}
Expand Down
5 changes: 3 additions & 2 deletions source/common/tcp_proxy/tcp_proxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -266,8 +266,9 @@ class Filter : public Network::ReadFilter,
absl::optional<uint64_t> computeHashKey() override {
auto hash_policy = config_->hashPolicy();
if (hash_policy) {
return hash_policy->generateHash(downstreamConnection()->remoteAddress().get(),
downstreamConnection()->localAddress().get());
return hash_policy->generateHash(
downstreamConnection()->addressProvider().remoteAddress().get(),
downstreamConnection()->addressProvider().localAddress().get());
}

return {};
Expand Down
2 changes: 1 addition & 1 deletion source/common/tcp_proxy/upstream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ void TcpConnPool::onPoolReady(Tcp::ConnectionPool::ConnectionDataPtr&& conn_data

auto upstream = std::make_unique<TcpUpstream>(std::move(conn_data), upstream_callbacks_);
callbacks_->onGenericPoolReady(&connection.streamInfo(), std::move(upstream), host,
latched_data->connection().localAddress(),
latched_data->connection().addressProvider().localAddress(),
latched_data->connection().streamInfo().downstreamSslConnection());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ createConnectionSocket(Network::Address::InstanceConstSharedPtr& peer_addr,
}
connection_socket->bind(local_addr);
ASSERT(local_addr->ip());
local_addr = connection_socket->localAddress();
local_addr = connection_socket->addressProvider().localAddress();
if (!Network::Socket::applyOptions(connection_socket->options(), *connection_socket,
envoy::config::core::v3::SocketOption::STATE_BOUND)) {
ENVOY_LOG_MISC(error, "Fail to apply post-bind options");
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/upstreams/http/tcp/upstream_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ void TcpConnPool::onPoolReady(Envoy::Tcp::ConnectionPool::ConnectionDataPtr&& co
Network::Connection& latched_conn = conn_data->connection();
auto upstream =
std::make_unique<TcpUpstream>(&callbacks_->upstreamToDownstream(), std::move(conn_data));
callbacks_->onPoolReady(std::move(upstream), host, latched_conn.localAddress(),
callbacks_->onPoolReady(std::move(upstream), host, latched_conn.addressProvider().localAddress(),
latched_conn.streamInfo(), {});
}

Expand Down
10 changes: 0 additions & 10 deletions source/server/api_listener_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,19 +111,10 @@ class ApiListenerImplBase : public ApiListener,
void readDisable(bool) override {}
void detectEarlyCloseWhenReadDisabled(bool) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
bool readEnabled() const override { return true; }
const Network::Address::InstanceConstSharedPtr& remoteAddress() const override {
return parent_.parent_.address();
}
const Network::Address::InstanceConstSharedPtr& directRemoteAddress() const override {
return parent_.parent_.address();
}
absl::optional<Network::Connection::UnixDomainSocketPeerCredentials>
unixSocketPeerCredentials() const override {
return absl::nullopt;
}
const Network::Address::InstanceConstSharedPtr& localAddress() const override {
return parent_.parent_.address();
}
void setConnectionStats(const Network::Connection::ConnectionStats&) override {}
Ssl::ConnectionInfoConstSharedPtr ssl() const override { return nullptr; }
absl::string_view requestedServerName() const override { return EMPTY_STRING; }
Expand All @@ -132,7 +123,6 @@ class ApiListenerImplBase : public ApiListener,
void write(Buffer::Instance&, bool) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
void setBufferLimits(uint32_t) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
uint32_t bufferLimit() const override { return 65000; }
bool localAddressRestored() const override { return false; }
bool aboveHighWatermark() const override { return false; }
const Network::ConnectionSocket::OptionsSharedPtr& socketOptions() const override {
return options_;
Expand Down
6 changes: 3 additions & 3 deletions source/server/filter_chain_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ std::pair<T, std::vector<Network::Address::CidrRange>> makeCidrListEntry(const s

const Network::FilterChain*
FilterChainManagerImpl::findFilterChain(const Network::ConnectionSocket& socket) const {
const auto& address = socket.localAddress();
const auto& address = socket.addressProvider().localAddress();

const Network::FilterChain* best_match_filter_chain = nullptr;
// Match on destination port (only for IP addresses).
Expand Down Expand Up @@ -449,7 +449,7 @@ FilterChainManagerImpl::findFilterChain(const Network::ConnectionSocket& socket)

const Network::FilterChain* FilterChainManagerImpl::findFilterChainForDestinationIP(
const DestinationIPsTrie& destination_ips_trie, const Network::ConnectionSocket& socket) const {
auto address = socket.localAddress();
auto address = socket.addressProvider().localAddress();
if (address->type() != Network::Address::Type::Ip) {
address = fakeAddress();
}
Expand Down Expand Up @@ -570,7 +570,7 @@ const Network::FilterChain* FilterChainManagerImpl::findFilterChainForSourceType

const Network::FilterChain* FilterChainManagerImpl::findFilterChainForSourceIpAndPort(
const SourceIPsTrie& source_ips_trie, const Network::ConnectionSocket& socket) const {
auto address = socket.remoteAddress();
auto address = socket.addressProvider().remoteAddress();
if (address->type() != Network::Address::Type::Ip) {
address = fakeAddress();
}
Expand Down
2 changes: 1 addition & 1 deletion source/server/listener_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ ListenSocketFactoryImpl::ListenSocketFactoryImpl(ListenerComponentFactory& facto
}

if (socket_ && local_address_->ip() && local_address_->ip()->port() == 0) {
local_address_ = socket_->localAddress();
local_address_ = socket_->addressProvider().localAddress();
}
ENVOY_LOG(debug, "Set listener {} socket factory local address to {}", listener_name_,
local_address_->asString());
Expand Down
5 changes: 3 additions & 2 deletions test/common/http/codec_client_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -296,12 +296,13 @@ TEST_F(CodecClientTest, SSLConnectionInfo) {
class CodecNetworkTest : public Event::TestUsingSimulatedTime,
public testing::TestWithParam<Network::Address::IpVersion> {
public:
CodecNetworkTest() : api_(Api::createApiForTest()), stream_info_(api_->timeSource()) {
CodecNetworkTest() : api_(Api::createApiForTest()), stream_info_(api_->timeSource(), nullptr) {
dispatcher_ = api_->allocateDispatcher("test_thread");
auto socket = std::make_shared<Network::TcpListenSocket>(
Network::Test::getCanonicalLoopbackAddress(GetParam()), nullptr, true);
Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection(
socket->localAddress(), source_address_, Network::Test::createRawBufferSocket(), nullptr);
socket->addressProvider().localAddress(), source_address_,
Network::Test::createRawBufferSocket(), nullptr);
upstream_listener_ = dispatcher_->createListener(std::move(socket), listener_callbacks_, true,
ENVOY_TCP_BACKLOG_SIZE);
client_connection_ = client_connection.get();
Expand Down
8 changes: 4 additions & 4 deletions test/common/http/conn_manager_impl_fuzz_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -570,10 +570,10 @@ DEFINE_PROTO_FUZZER(const test::common::http::ConnManagerImplTestCase& input) {
ON_CALL(Const(filter_callbacks.connection_), ssl()).WillByDefault(Return(ssl_connection));
ON_CALL(filter_callbacks.connection_, close(_))
.WillByDefault(InvokeWithoutArgs([&connection_alive] { connection_alive = false; }));
filter_callbacks.connection_.local_address_ =
std::make_shared<Network::Address::Ipv4Instance>("127.0.0.1");
filter_callbacks.connection_.remote_address_ =
std::make_shared<Network::Address::Ipv4Instance>("0.0.0.0");
filter_callbacks.connection_.stream_info_.downstream_address_provider_->setLocalAddress(
std::make_shared<Network::Address::Ipv4Instance>("127.0.0.1"));
filter_callbacks.connection_.stream_info_.downstream_address_provider_->setRemoteAddress(
std::make_shared<Network::Address::Ipv4Instance>("0.0.0.0"));

ConnectionManagerImpl conn_manager(config, drain_close, random, http_context, runtime, local_info,
cluster_manager, overload_manager, config.time_system_);
Expand Down
Loading

0 comments on commit d38f963

Please sign in to comment.