From d5787f628c40624db4d9d2a970f7e672bb006852 Mon Sep 17 00:00:00 2001 From: Ghanan Gowripalan Date: Fri, 28 Aug 2020 14:37:53 -0700 Subject: Don't bind loopback to all IPs in an IPv6 subnet An earlier change considered the loopback bound to all addresses in an assigned subnet. This should have only be done for IPv4 to maintain compatability with Linux: ``` $ ip addr show dev lo 1: lo: mtu 65536 qdisc noqueue state UNKNOWN group ... link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 inet 127.0.0.1/8 scope host lo valid_lft forever preferred_lft forever inet6 ::1/128 scope host valid_lft forever preferred_lft forever $ ping 2001:db8::1 PING 2001:db8::1(2001:db8::1) 56 data bytes ^C --- 2001:db8::1 ping statistics --- 4 packets transmitted, 0 received, 100% packet loss, time 3062ms $ ping 2001:db8::2 PING 2001:db8::2(2001:db8::2) 56 data bytes ^C --- 2001:db8::2 ping statistics --- 3 packets transmitted, 0 received, 100% packet loss, time 2030ms $ sudo ip addr add 2001:db8::1/64 dev lo $ ping 2001:db8::1 PING 2001:db8::1(2001:db8::1) 56 data bytes 64 bytes from 2001:db8::1: icmp_seq=1 ttl=64 time=0.055 ms 64 bytes from 2001:db8::1: icmp_seq=2 ttl=64 time=0.074 ms 64 bytes from 2001:db8::1: icmp_seq=3 ttl=64 time=0.073 ms 64 bytes from 2001:db8::1: icmp_seq=4 ttl=64 time=0.071 ms ^C --- 2001:db8::1 ping statistics --- 4 packets transmitted, 4 received, 0% packet loss, time 3075ms rtt min/avg/max/mdev = 0.055/0.068/0.074/0.007 ms $ ping 2001:db8::2 PING 2001:db8::2(2001:db8::2) 56 data bytes From 2001:db8::1 icmp_seq=1 Destination unreachable: No route From 2001:db8::1 icmp_seq=2 Destination unreachable: No route From 2001:db8::1 icmp_seq=3 Destination unreachable: No route From 2001:db8::1 icmp_seq=4 Destination unreachable: No route ^C --- 2001:db8::2 ping statistics --- 4 packets transmitted, 0 received, +4 errors, 100% packet loss, time 3070ms ``` Test: integration_test.TestLoopbackAcceptAllInSubnet PiperOrigin-RevId: 329011566 --- pkg/tcpip/stack/nic.go | 6 +-- pkg/tcpip/tests/integration/loopback_test.go | 40 --------------- test/syscalls/linux/BUILD | 19 +------ .../linux/socket_ip_udp_unbound_netlink_util.cc | 58 ---------------------- .../linux/socket_ip_udp_unbound_netlink_util.h | 34 ------------- .../linux/socket_ipv4_udp_unbound_netlink.cc | 32 +++++++++++- .../linux/socket_ipv4_udp_unbound_netlink.h | 4 +- .../linux/socket_ipv6_udp_unbound_netlink.cc | 28 ++++------- .../linux/socket_ipv6_udp_unbound_netlink.h | 4 +- 9 files changed, 49 insertions(+), 176 deletions(-) delete mode 100644 test/syscalls/linux/socket_ip_udp_unbound_netlink_util.cc delete mode 100644 test/syscalls/linux/socket_ip_udp_unbound_netlink_util.h diff --git a/pkg/tcpip/stack/nic.go b/pkg/tcpip/stack/nic.go index 8e700990d..863ef6bee 100644 --- a/pkg/tcpip/stack/nic.go +++ b/pkg/tcpip/stack/nic.go @@ -676,10 +676,10 @@ func (n *NIC) getRefOrCreateTemp(protocol tcpip.NetworkProtocolNumber, address t } // A usable reference was not found, create a temporary one if requested by - // the caller or if the address is found in the NIC's subnets and the NIC is - // a loopback interface. + // the caller or if the IPv4 address is found in the NIC's subnets and the NIC + // is a loopback interface. createTempEP := spoofingOrPromiscuous - if !createTempEP && n.isLoopback() { + if !createTempEP && n.isLoopback() && protocol == header.IPv4ProtocolNumber { for _, r := range n.mu.endpoints { addr := r.addrWithPrefix() subnet := addr.Subnet() diff --git a/pkg/tcpip/tests/integration/loopback_test.go b/pkg/tcpip/tests/integration/loopback_test.go index 3a2f75837..1b18023c5 100644 --- a/pkg/tcpip/tests/integration/loopback_test.go +++ b/pkg/tcpip/tests/integration/loopback_test.go @@ -109,52 +109,12 @@ func TestLoopbackAcceptAllInSubnet(t *testing.T) { dstAddr: ipv6Addr.Address, expectRx: true, }, - { - name: "IPv6 bind to wildcard and send to assigned address", - addAddress: ipv6ProtocolAddress, - dstAddr: ipv6Addr.Address, - expectRx: true, - }, { name: "IPv6 bind to wildcard and send to other subnet-local address", addAddress: ipv6ProtocolAddress, dstAddr: otherIPv6Address, - expectRx: true, - }, - { - name: "IPv6 bind to wildcard send to other address", - addAddress: ipv6ProtocolAddress, - dstAddr: remoteIPv6Addr, - expectRx: false, - }, - { - name: "IPv6 bind to other subnet-local address and send to assigned address", - addAddress: ipv6ProtocolAddress, - bindAddr: otherIPv6Address, - dstAddr: ipv6Addr.Address, - expectRx: false, - }, - { - name: "IPv6 bind and send to other subnet-local address", - addAddress: ipv6ProtocolAddress, - bindAddr: otherIPv6Address, - dstAddr: otherIPv6Address, - expectRx: true, - }, - { - name: "IPv6 bind to assigned address and send to other subnet-local address", - addAddress: ipv6ProtocolAddress, - bindAddr: ipv6Addr.Address, - dstAddr: otherIPv6Address, expectRx: false, }, - { - name: "IPv6 bind and send to assigned address", - addAddress: ipv6ProtocolAddress, - bindAddr: ipv6Addr.Address, - dstAddr: ipv6Addr.Address, - expectRx: true, - }, } for _, test := range tests { diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index 5a323d331..fad3be7bf 100644 --- a/test/syscalls/linux/BUILD +++ b/test/syscalls/linux/BUILD @@ -2416,21 +2416,6 @@ cc_library( alwayslink = 1, ) -cc_library( - name = "socket_ip_udp_unbound_netlink_test_utils", - testonly = 1, - srcs = [ - "socket_ip_udp_unbound_netlink_util.cc", - ], - hdrs = [ - "socket_ip_udp_unbound_netlink_util.h", - ], - deps = [ - ":socket_test_util", - ], - alwayslink = 1, -) - cc_library( name = "socket_ipv4_udp_unbound_netlink_test_cases", testonly = 1, @@ -2441,8 +2426,8 @@ cc_library( "socket_ipv4_udp_unbound_netlink.h", ], deps = [ - ":socket_ip_udp_unbound_netlink_test_utils", ":socket_netlink_route_util", + ":socket_test_util", "//test/util:capability_util", gtest, ], @@ -2459,8 +2444,8 @@ cc_library( "socket_ipv6_udp_unbound_netlink.h", ], deps = [ - ":socket_ip_udp_unbound_netlink_test_utils", ":socket_netlink_route_util", + ":socket_test_util", "//test/util:capability_util", gtest, ], diff --git a/test/syscalls/linux/socket_ip_udp_unbound_netlink_util.cc b/test/syscalls/linux/socket_ip_udp_unbound_netlink_util.cc deleted file mode 100644 index 13ffafde7..000000000 --- a/test/syscalls/linux/socket_ip_udp_unbound_netlink_util.cc +++ /dev/null @@ -1,58 +0,0 @@ -// Copyright 2020 The gVisor Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "test/syscalls/linux/socket_ip_udp_unbound_netlink_util.h" - -namespace gvisor { -namespace testing { - -const size_t kSendBufSize = 200; - -void IPUDPUnboundSocketNetlinkTest::TestSendRecv(TestAddress sender_addr, - TestAddress receiver_addr) { - auto snd_sock = ASSERT_NO_ERRNO_AND_VALUE(NewSocket()); - auto rcv_sock = ASSERT_NO_ERRNO_AND_VALUE(NewSocket()); - - EXPECT_THAT( - bind(snd_sock->get(), reinterpret_cast(&sender_addr.addr), - sender_addr.addr_len), - SyscallSucceeds()); - - EXPECT_THAT( - bind(rcv_sock->get(), reinterpret_cast(&receiver_addr.addr), - receiver_addr.addr_len), - SyscallSucceeds()); - socklen_t receiver_addr_len = receiver_addr.addr_len; - ASSERT_THAT(getsockname(rcv_sock->get(), - reinterpret_cast(&receiver_addr.addr), - &receiver_addr_len), - SyscallSucceeds()); - EXPECT_EQ(receiver_addr_len, receiver_addr.addr_len); - char send_buf[kSendBufSize]; - RandomizeBuffer(send_buf, kSendBufSize); - EXPECT_THAT( - RetryEINTR(sendto)(snd_sock->get(), send_buf, kSendBufSize, 0, - reinterpret_cast(&receiver_addr.addr), - receiver_addr.addr_len), - SyscallSucceedsWithValue(kSendBufSize)); - - // Check that we received the packet. - char recv_buf[kSendBufSize] = {}; - ASSERT_THAT(RetryEINTR(recv)(rcv_sock->get(), recv_buf, kSendBufSize, 0), - SyscallSucceedsWithValue(kSendBufSize)); - EXPECT_EQ(0, memcmp(send_buf, recv_buf, kSendBufSize)); -} - -} // namespace testing -} // namespace gvisor diff --git a/test/syscalls/linux/socket_ip_udp_unbound_netlink_util.h b/test/syscalls/linux/socket_ip_udp_unbound_netlink_util.h deleted file mode 100644 index 157fb0939..000000000 --- a/test/syscalls/linux/socket_ip_udp_unbound_netlink_util.h +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright 2020 The gVisor Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef GVISOR_TEST_SYSCALLS_LINUX_SOCKET_IP_UDP_UNBOUND_NETLINK_UTIL_H_ -#define GVISOR_TEST_SYSCALLS_LINUX_SOCKET_IP_UDP_UNBOUND_NETLINK_UTIL_H_ - -#include "test/syscalls/linux/socket_test_util.h" - -namespace gvisor { -namespace testing { - -// Test fixture for tests that apply to IP UDP sockets. -class IPUDPUnboundSocketNetlinkTest : public SimpleSocketTest { - public: - // TestSendRecv tests sending and receiving a UDP packet from |sender_addr| to - // |receiver_addr|. - void TestSendRecv(TestAddress sender_addr, TestAddress receiver_addr); -}; - -} // namespace testing -} // namespace gvisor - -#endif // GVISOR_TEST_SYSCALLS_LINUX_SOCKET_IP_UDP_UNBOUND_NETLINK_UTIL_H_ diff --git a/test/syscalls/linux/socket_ipv4_udp_unbound_netlink.cc b/test/syscalls/linux/socket_ipv4_udp_unbound_netlink.cc index 696fbb189..79eb48afa 100644 --- a/test/syscalls/linux/socket_ipv4_udp_unbound_netlink.cc +++ b/test/syscalls/linux/socket_ipv4_udp_unbound_netlink.cc @@ -23,6 +23,8 @@ namespace gvisor { namespace testing { +constexpr size_t kSendBufSize = 200; + // Checks that the loopback interface considers itself bound to all IPs in an // associated subnet. TEST_P(IPv4UDPUnboundSocketNetlinkTest, JoinSubnet) { @@ -35,6 +37,9 @@ TEST_P(IPv4UDPUnboundSocketNetlinkTest, JoinSubnet) { EXPECT_NO_ERRNO(LinkAddLocalAddr(loopback_link.index, AF_INET, /*prefixlen=*/24, &addr, sizeof(addr))); + auto snd_sock = ASSERT_NO_ERRNO_AND_VALUE(NewSocket()); + auto rcv_sock = ASSERT_NO_ERRNO_AND_VALUE(NewSocket()); + // Send from an unassigned address but an address that is in the subnet // associated with the loopback interface. TestAddress sender_addr("V4NotAssignd1"); @@ -43,6 +48,10 @@ TEST_P(IPv4UDPUnboundSocketNetlinkTest, JoinSubnet) { EXPECT_EQ(1, inet_pton(AF_INET, "192.0.2.2", &(reinterpret_cast(&sender_addr.addr) ->sin_addr.s_addr))); + EXPECT_THAT( + bind(snd_sock->get(), reinterpret_cast(&sender_addr.addr), + sender_addr.addr_len), + SyscallSucceeds()); // Send the packet to an unassigned address but an address that is in the // subnet associated with the loopback interface. @@ -52,8 +61,29 @@ TEST_P(IPv4UDPUnboundSocketNetlinkTest, JoinSubnet) { EXPECT_EQ(1, inet_pton(AF_INET, "192.0.2.254", &(reinterpret_cast(&receiver_addr.addr) ->sin_addr.s_addr))); + EXPECT_THAT( + bind(rcv_sock->get(), reinterpret_cast(&receiver_addr.addr), + receiver_addr.addr_len), + SyscallSucceeds()); + socklen_t receiver_addr_len = receiver_addr.addr_len; + ASSERT_THAT(getsockname(rcv_sock->get(), + reinterpret_cast(&receiver_addr.addr), + &receiver_addr_len), + SyscallSucceeds()); + EXPECT_EQ(receiver_addr_len, receiver_addr.addr_len); + char send_buf[kSendBufSize]; + RandomizeBuffer(send_buf, kSendBufSize); + EXPECT_THAT( + RetryEINTR(sendto)(snd_sock->get(), send_buf, kSendBufSize, 0, + reinterpret_cast(&receiver_addr.addr), + receiver_addr.addr_len), + SyscallSucceedsWithValue(kSendBufSize)); - TestSendRecv(sender_addr, receiver_addr); + // Check that we received the packet. + char recv_buf[kSendBufSize] = {}; + ASSERT_THAT(RetryEINTR(recv)(rcv_sock->get(), recv_buf, kSendBufSize, 0), + SyscallSucceedsWithValue(kSendBufSize)); + EXPECT_EQ(0, memcmp(send_buf, recv_buf, kSendBufSize)); } } // namespace testing diff --git a/test/syscalls/linux/socket_ipv4_udp_unbound_netlink.h b/test/syscalls/linux/socket_ipv4_udp_unbound_netlink.h index fcfb3318e..73e7836d5 100644 --- a/test/syscalls/linux/socket_ipv4_udp_unbound_netlink.h +++ b/test/syscalls/linux/socket_ipv4_udp_unbound_netlink.h @@ -15,13 +15,13 @@ #ifndef GVISOR_TEST_SYSCALLS_LINUX_SOCKET_IPV4_UDP_UNBOUND_NETLINK_UTIL_H_ #define GVISOR_TEST_SYSCALLS_LINUX_SOCKET_IPV4_UDP_UNBOUND_NETLINK_UTIL_H_ -#include "test/syscalls/linux/socket_ip_udp_unbound_netlink_util.h" +#include "test/syscalls/linux/socket_test_util.h" namespace gvisor { namespace testing { // Test fixture for tests that apply to IPv4 UDP sockets. -using IPv4UDPUnboundSocketNetlinkTest = IPUDPUnboundSocketNetlinkTest; +using IPv4UDPUnboundSocketNetlinkTest = SimpleSocketTest; } // namespace testing } // namespace gvisor diff --git a/test/syscalls/linux/socket_ipv6_udp_unbound_netlink.cc b/test/syscalls/linux/socket_ipv6_udp_unbound_netlink.cc index 539a4ec55..2ee218231 100644 --- a/test/syscalls/linux/socket_ipv6_udp_unbound_netlink.cc +++ b/test/syscalls/linux/socket_ipv6_udp_unbound_netlink.cc @@ -23,13 +23,10 @@ namespace gvisor { namespace testing { -// Checks that the loopback interface considers itself bound to all IPs in an -// associated subnet. +// Checks that the loopback interface does not consider itself bound to all IPs +// in an associated subnet. TEST_P(IPv6UDPUnboundSocketNetlinkTest, JoinSubnet) { - // TODO(b/166440211): Only run this test on gvisor or remove if the loopback - // interface should not consider itself bound to all IPs in an IPv6 subnet. - SKIP_IF(!IsRunningOnGvisor() || - !ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_NET_ADMIN))); + SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_NET_ADMIN))); // Add an IP address to the loopback interface. Link loopback_link = ASSERT_NO_ERRNO_AND_VALUE(LoopbackLink()); @@ -38,25 +35,18 @@ TEST_P(IPv6UDPUnboundSocketNetlinkTest, JoinSubnet) { EXPECT_NO_ERRNO(LinkAddLocalAddr(loopback_link.index, AF_INET6, /*prefixlen=*/64, &addr, sizeof(addr))); - // Send from an unassigned address but an address that is in the subnet - // associated with the loopback interface. + // Binding to an unassigned address but an address that is in the subnet + // associated with the loopback interface should fail. TestAddress sender_addr("V6NotAssignd1"); sender_addr.addr.ss_family = AF_INET6; sender_addr.addr_len = sizeof(sockaddr_in6); EXPECT_EQ(1, inet_pton(AF_INET6, "2001:db8::2", reinterpret_cast(&sender_addr.addr) ->sin6_addr.s6_addr)); - - // Send the packet to an unassigned address but an address that is in the - // subnet associated with the loopback interface. - TestAddress receiver_addr("V6NotAssigned2"); - receiver_addr.addr.ss_family = AF_INET6; - receiver_addr.addr_len = sizeof(sockaddr_in6); - EXPECT_EQ(1, inet_pton(AF_INET6, "2001:db8::ffff:ffff:ffff:ffff", - reinterpret_cast(&receiver_addr.addr) - ->sin6_addr.s6_addr)); - - TestSendRecv(sender_addr, receiver_addr); + auto sock = ASSERT_NO_ERRNO_AND_VALUE(NewSocket()); + EXPECT_THAT(bind(sock->get(), reinterpret_cast(&sender_addr.addr), + sender_addr.addr_len), + SyscallFailsWithErrno(EADDRNOTAVAIL)); } } // namespace testing diff --git a/test/syscalls/linux/socket_ipv6_udp_unbound_netlink.h b/test/syscalls/linux/socket_ipv6_udp_unbound_netlink.h index 6a2b0a5be..88098be82 100644 --- a/test/syscalls/linux/socket_ipv6_udp_unbound_netlink.h +++ b/test/syscalls/linux/socket_ipv6_udp_unbound_netlink.h @@ -15,13 +15,13 @@ #ifndef GVISOR_TEST_SYSCALLS_LINUX_SOCKET_IPV6_UDP_UNBOUND_NETLINK_UTIL_H_ #define GVISOR_TEST_SYSCALLS_LINUX_SOCKET_IPV6_UDP_UNBOUND_NETLINK_UTIL_H_ -#include "test/syscalls/linux/socket_ip_udp_unbound_netlink_util.h" +#include "test/syscalls/linux/socket_test_util.h" namespace gvisor { namespace testing { // Test fixture for tests that apply to IPv6 UDP sockets. -using IPv6UDPUnboundSocketNetlinkTest = IPUDPUnboundSocketNetlinkTest; +using IPv6UDPUnboundSocketNetlinkTest = SimpleSocketTest; } // namespace testing } // namespace gvisor -- cgit v1.2.3