Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
27 changes: 16 additions & 11 deletions source/common/network/listener_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,25 @@ Address::InstanceConstSharedPtr ListenerImpl::getLocalAddress(int fd) {
void ListenerImpl::listenCallback(evconnlistener*, evutil_socket_t fd, sockaddr* remote_addr,
int remote_addr_len, void* arg) {
ListenerImpl* listener = static_cast<ListenerImpl*>(arg);
ConnectionSocketPtr socket(new AcceptedSocketImpl(
fd,
// Get the local address from the new socket if the listener is listening on IP ANY
// (e.g., 0.0.0.0 for IPv4) (local_address_ is nullptr in this case).
!listener->local_address_ ? listener->getLocalAddress(fd) : listener->local_address_,
// The accept() call that filled in remote_addr doesn't fill in more than the sa_family field
// for Unix domain sockets; apparently there isn't a mechanism in the kernel to get the
// sockaddr_un associated with the client socket when starting from the server socket.
// We work around this by using our own name for the socket in this case.
// Get the local address from the new socket if the listener is listening on IP ANY
// (e.g., 0.0.0.0 for IPv4) (local_address_ is nullptr in this case).
const Address::InstanceConstSharedPtr& local_address =
listener->local_address_ ? listener->local_address_ : listener->getLocalAddress(fd);
// The accept() call that filled in remote_addr doesn't fill in more than the sa_family field
// for Unix domain sockets; apparently there isn't a mechanism in the kernel to get the
// sockaddr_un associated with the client socket when starting from the server socket.
// We work around this by using our own name for the socket in this case.
// Pass the 'v6only' parameter as true if the local_address is an IPv6 address. This has no effect
// if the socket is a v4 socket, but for v6 sockets this will create an IPv4 remote address if an
// IPv4 local_address was created from an IPv6 mapped IPv4 address.
const Address::InstanceConstSharedPtr& remote_address =
(remote_addr->sa_family == AF_UNIX)
? Address::peerAddressFromFd(fd)
: Address::addressFromSockAddr(*reinterpret_cast<const sockaddr_storage*>(remote_addr),
remote_addr_len)));
listener->cb_.onAccept(std::move(socket), listener->hand_off_restored_destination_connections_);
remote_addr_len,
local_address->ip()->version() == Address::IpVersion::v6);
listener->cb_.onAccept(std::make_unique<AcceptedSocketImpl>(fd, local_address, remote_address),
listener->hand_off_restored_destination_connections_);
}

ListenerImpl::ListenerImpl(Event::DispatcherImpl& dispatcher, Socket& socket, ListenerCallbacks& cb,
Expand Down
47 changes: 47 additions & 0 deletions test/common/network/listener_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -154,5 +154,52 @@ TEST_P(ListenerImplTest, WildcardListenerUseActualDst) {
dispatcher.run(Event::Dispatcher::RunType::Block);
}

// Test for the correct behavior when a listener is configured with an ANY address that allows
// receiving IPv4 connections on an IPv6 socket. In this case the address instances of both
// local and remote addresses of the connection should be IPv4 instances, as the connection really
// is an IPv4 connection.
TEST_P(ListenerImplTest, WildcardListenerIpv4Compat) {
Stats::IsolatedStoreImpl stats_store;
Event::DispatcherImpl dispatcher;
Network::TcpListenSocket socket(Network::Test::getAnyAddress(version_, true), true);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like a comment here (or at the top of the test) briefly explaining the circumstances this is testing.

It's confusing to me since this is the only place that leads to invoking Network::Utility::getAnyIpv6Address(false), so is this code testing a case that cannot occur outside of tests?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test mocks a case where a listener would be configured with an IPv6 any address and have it's socket_address configured with ipv4_compat = true. There are a couple of tests that do that, but in those the socket address comes from the listener configuraion, so they do not invoke Network::Utility::getAnyIpv6Address() at all. The effect is the same, though, when the listener is configured with the IPv6 ANY address ([::]).

Network::MockListenerCallbacks listener_callbacks;
Network::MockConnectionHandler connection_handler;

ASSERT_TRUE(socket.localAddress()->ip()->isAnyAddress());

// Do not redirect since use_original_dst is false.
Network::TestListenerImpl listener(dispatcher, socket, listener_callbacks, true, true);

auto listener_address = Network::Utility::getAddressWithPort(
*Network::Test::getCanonicalLoopbackAddress(version_), socket.localAddress()->ip()->port());
auto local_dst_address = Network::Utility::getAddressWithPort(
*Network::Utility::getCanonicalIpv4LoopbackAddress(), socket.localAddress()->ip()->port());
Network::ClientConnectionPtr client_connection = dispatcher.createClientConnection(
local_dst_address, Network::Address::InstanceConstSharedPtr(),
Network::Test::createRawBufferSocket(), nullptr);
client_connection->connect();

EXPECT_CALL(listener, getLocalAddress(_))
.WillOnce(Invoke(
[](int fd) -> Address::InstanceConstSharedPtr { return Address::addressFromFd(fd); }));

EXPECT_CALL(listener_callbacks, onAccept_(_, _))
.WillOnce(Invoke([&](Network::ConnectionSocketPtr& socket, bool) -> void {
Network::ConnectionPtr new_connection = dispatcher.createServerConnection(
std::move(socket), Network::Test::createRawBufferSocket());
listener_callbacks.onNewConnection(std::move(new_connection));
}));
EXPECT_CALL(listener_callbacks, onNewConnection_(_))
.WillOnce(Invoke([&](Network::ConnectionPtr& conn) -> void {
EXPECT_EQ(conn->localAddress()->ip()->version(), conn->remoteAddress()->ip()->version());
EXPECT_EQ(*conn->localAddress(), *local_dst_address);
client_connection->close(ConnectionCloseType::NoFlush);
conn->close(ConnectionCloseType::NoFlush);
dispatcher.exit();
}));

dispatcher.run(Event::Dispatcher::RunType::Block);
}

} // namespace Network
} // namespace Envoy
24 changes: 23 additions & 1 deletion test/test_common/network_utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,32 @@ Address::InstanceConstSharedPtr getCanonicalLoopbackAddress(Address::IpVersion v
return Network::Utility::getIpv6LoopbackAddress();
}

Address::InstanceConstSharedPtr getAnyAddress(const Address::IpVersion version) {
namespace {
// There is no portable way to initialize sockaddr_in6 with a static initializer, do it with a
// helper function instead.
sockaddr_in6 sockaddrIn6Any() {
sockaddr_in6 v6any = {};
v6any.sin6_family = AF_INET6;
v6any.sin6_addr = in6addr_any;

return v6any;
}
} // namespace

Address::InstanceConstSharedPtr getAnyAddress(const Address::IpVersion version, bool v4_compat) {
if (version == Address::IpVersion::v4) {
return Network::Utility::getIpv4AnyAddress();
}
if (v4_compat) {
// This will return an IPv6 ANY address ("[::]:0") like the getIpv6AnyAddress() below, but
// with the internal 'v6only' member set to false. This will allow a socket created from this
// address to accept IPv4 connections. IPv4 connections received on IPv6 sockets will have
// Ipv4-mapped IPv6 addresses, which we will then internally interpret as IPv4 addresses so
// that, for example, access logging will show IPv4 address format for IPv4 connections even
// if they were received on an IPv6 socket.
static Address::InstanceConstSharedPtr any(new Address::Ipv6Instance(sockaddrIn6Any(), false));
return any;
}
return Network::Utility::getIpv6AnyAddress();
}

Expand Down
6 changes: 5 additions & 1 deletion test/test_common/network_utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,13 @@ Address::InstanceConstSharedPtr getCanonicalLoopbackAddress(const Address::IpVer
/**
* Returns the any address for the specified IP version.
* @param version the IP version of the any address.
* @param v4_compat determines whether a v4-mapped addresses bound to a socket listening on the
* returned ANY address are to be treated as IPv4 or IPv6 addresses. Defaults to 'false',
* has no effect with IPv4 ANY address.
* @returns the any address for the specified IP version.
*/
Address::InstanceConstSharedPtr getAnyAddress(const Address::IpVersion version);
Address::InstanceConstSharedPtr getAnyAddress(const Address::IpVersion version,
bool v4_compat = false);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Document v4_compat, please.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.


/**
* This function tries to create a socket of type IpVersion version and bind to it. If
Expand Down