From 7041349e6b44f99afca2d613f7d24dfe023f836a Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 19 Jul 2026 16:14:55 +0000 Subject: [PATCH] [dhcp4relay]: Reject chained requests with a local giaddr Check nonzero giaddr values against all local IPv4 interface addresses and drop local matches or lookup failures before forwarding. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2b881aa9-3a3a-4aaf-b2ca-b941705b2438 Signed-off-by: Xichen96 --- dhcp4relay/src/dhcp4relay.cpp | 51 +++++++++++++++++++++++++++ dhcp4relay/test/mock_relay.cpp | 63 ++++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+) diff --git a/dhcp4relay/src/dhcp4relay.cpp b/dhcp4relay/src/dhcp4relay.cpp index bdd0979..3c66f4b 100644 --- a/dhcp4relay/src/dhcp4relay.cpp +++ b/dhcp4relay/src/dhcp4relay.cpp @@ -560,6 +560,37 @@ void encode_relay_option(pcpp::DhcpLayer *dhcp_pkt, relay_config *config) { return; } +enum class LocalAddressCheck { + ERROR, + NOT_LOCAL, + LOCAL +}; + +static LocalAddressCheck check_local_ipv4_address(uint32_t address) { + struct ifaddrs *ifaddrs_list = nullptr; + if (getifaddrs(&ifaddrs_list) == -1) { + SWSS_LOG_ERROR("[DHCPV4_RELAY] Unable to enumerate local IPv4 addresses: %s", + strerror(errno)); + return LocalAddressCheck::ERROR; + } + + LocalAddressCheck result = LocalAddressCheck::NOT_LOCAL; + for (struct ifaddrs *ifa = ifaddrs_list; ifa != nullptr; ifa = ifa->ifa_next) { + if (ifa->ifa_addr == nullptr || ifa->ifa_addr->sa_family != AF_INET) { + continue; + } + + auto *local_address = reinterpret_cast(ifa->ifa_addr); + if (local_address->sin_addr.s_addr == address) { + result = LocalAddressCheck::LOCAL; + break; + } + } + + freeifaddrs(ifaddrs_list); + return result; +} + /** * @code void from_client(pcpp::DhcpLayer* dhcp_pkt, relay_config *config) * @@ -594,6 +625,26 @@ void from_client(pcpp::DhcpLayer *dhcp_pkt, relay_config &config) { encode_relay_option(dhcp_pkt, &config); } } else { + const uint32_t giaddr = dhcp_pkt->getDhcpHeader()->gatewayIpAddress; + const LocalAddressCheck address_check = check_local_ipv4_address(giaddr); + if (address_check == LocalAddressCheck::ERROR) { + SWSS_LOG_ERROR("[DHCPV4_RELAY] Dropping chained packet because local giaddr validation failed on %s", + config.vlan.c_str()); + dhcp_cntr_table.increment_counter(config.vlan, "TX", DHCPv4_MESSAGE_TYPE_DROP); + return; + } + if (address_check == LocalAddressCheck::LOCAL) { + char giaddr_string[INET_ADDRSTRLEN] = {}; + struct in_addr giaddr_address = {giaddr}; + const char *formatted_giaddr = + inet_ntop(AF_INET, &giaddr_address, giaddr_string, sizeof(giaddr_string)); + SWSS_LOG_NOTICE("[DHCPV4_RELAY] Dropping chained packet with local giaddr %s on %s", + formatted_giaddr == nullptr ? "" : formatted_giaddr, + config.vlan.c_str()); + dhcp_cntr_table.increment_counter(config.vlan, "TX", DHCPv4_MESSAGE_TYPE_DROP); + return; + } + /* If the relay packet is from another relay, we should act based on configuration of agent_relay_mode. append - Forward the packet with appending our own relay option. diff --git a/dhcp4relay/test/mock_relay.cpp b/dhcp4relay/test/mock_relay.cpp index 02a44ab..b24627a 100644 --- a/dhcp4relay/test/mock_relay.cpp +++ b/dhcp4relay/test/mock_relay.cpp @@ -935,6 +935,17 @@ TEST(DHCPRelayTest, to_client) { to_client(&dhcpLayer, &vlans, "172.22.178.234"); } +static struct ifaddrs *expect_local_address_check( + const std::string &vlan_ip = "192.168.10.10", + const std::string &source_ip = "192.168.1.2") { + struct ifaddrs *mock_ifaddrs = CreateMockIfaddrs( + vlan_ip, "255.255.255.0", "Vlan10", source_ip, "Ethernet12"); + EXPECT_GLOBAL_CALL(getifaddrs, getifaddrs(_)) + .WillOnce(DoAll(testing::SetArgPointee<0>(mock_ifaddrs), Return(0))); + EXPECT_GLOBAL_CALL(freeifaddrs, freeifaddrs(_)).Times(1); + return mock_ifaddrs; +} + TEST(DHCPRelayTest, from_client) { pcpp::MacAddress clientMac(std::string("00:0e:86:11:c0:75")); @@ -964,6 +975,7 @@ TEST(DHCPRelayTest, from_client) { m_config.host_mac_addr = "12:32:54:24:95:36"; encode_relay_option(&dhcpLayer, &config); + struct ifaddrs *mock_ifaddrs = expect_local_address_check(); EXPECT_GLOBAL_CALL(send_udp, send_udp(_, _, _, _, _, _, _)).WillOnce([] (int sock, uint8_t* hdr, struct sockaddr_in target, uint32_t len, in_addr src_ip, bool use_src_ip, bool pad) { pcpp::dhcp_header* dhcp_hdr = (pcpp::dhcp_header*)hdr; @@ -973,6 +985,7 @@ TEST(DHCPRelayTest, from_client) { return true; }); from_client(&dhcpLayer, config); + FreeMockIfaddrs(mock_ifaddrs); } /* Helper: build a relay-of-relay packet (giaddr already set) with a pre-existing Option 82. */ @@ -997,6 +1010,48 @@ static relay_config make_relay_of_relay_config(const std::string &agent_relay_mo return config; } +TEST(DHCPRelayTest, from_client_relay_of_relay_local_giaddr_is_dropped) { + relay_config config = make_relay_of_relay_config("forward"); + + pcpp::MacAddress clientMac(std::string("00:0e:86:11:c0:75")); + pcpp::DhcpLayer dhcpLayer(pcpp::DHCP_DISCOVER, clientMac); + dhcpLayer.getDhcpHeader()->hops = 2; + dhcpLayer.getDhcpHeader()->gatewayIpAddress = inet_addr("192.168.1.2"); + encode_relay_option(&dhcpLayer, &config); + + struct ifaddrs *mock_ifaddrs = expect_local_address_check(); + EXPECT_GLOBAL_CALL(send_udp, send_udp(_, _, _, _, _, _, _)).Times(0); + from_client(&dhcpLayer, config); + + EXPECT_EQ(dhcpLayer.getDhcpHeader()->hops, 2); + EXPECT_EQ(dhcpLayer.getDhcpHeader()->gatewayIpAddress, inet_addr("192.168.1.2")); + FreeMockIfaddrs(mock_ifaddrs); +} + +TEST(DHCPRelayTest, from_client_relay_of_relay_nonlocal_giaddr_forwards) { + relay_config config = make_relay_of_relay_config("forward"); + + pcpp::MacAddress clientMac(std::string("00:0e:86:11:c0:75")); + pcpp::DhcpLayer dhcpLayer(pcpp::DHCP_DISCOVER, clientMac); + dhcpLayer.getDhcpHeader()->hops = 2; + dhcpLayer.getDhcpHeader()->gatewayIpAddress = inet_addr("203.0.113.10"); + encode_relay_option(&dhcpLayer, &config); + + struct ifaddrs *mock_ifaddrs = expect_local_address_check(); + EXPECT_GLOBAL_CALL(send_udp, send_udp(_, _, _, _, _, _, _)).WillOnce([] + (int, uint8_t *hdr, struct sockaddr_in, uint32_t, in_addr, bool, bool) { + pcpp::dhcp_header *dhcp_hdr = reinterpret_cast(hdr); + EXPECT_EQ(dhcp_hdr->hops, 3); + EXPECT_EQ(dhcp_hdr->gatewayIpAddress, inet_addr("203.0.113.10")); + return true; + }); + from_client(&dhcpLayer, config); + + EXPECT_EQ(dhcpLayer.getDhcpHeader()->hops, 3); + EXPECT_EQ(dhcpLayer.getDhcpHeader()->gatewayIpAddress, inet_addr("203.0.113.10")); + FreeMockIfaddrs(mock_ifaddrs); +} + /* agent_relay_mode=append: packet already has Option 82; we should append ours and forward. */ TEST(DHCPRelayTest, from_client_relay_of_relay_append) { relay_config config = make_relay_of_relay_config("append"); @@ -1008,6 +1063,7 @@ TEST(DHCPRelayTest, from_client_relay_of_relay_append) { dhcpLayer.getDhcpHeader()->gatewayIpAddress = inet_addr("192.168.1.1"); encode_relay_option(&dhcpLayer, &config); + struct ifaddrs *mock_ifaddrs = expect_local_address_check(); EXPECT_GLOBAL_CALL(send_udp, send_udp(_, _, _, _, _, _, _)).WillOnce([] (int, uint8_t* hdr, struct sockaddr_in, uint32_t, in_addr, bool, bool) { pcpp::dhcp_header* dhcp_hdr = (pcpp::dhcp_header*)hdr; @@ -1016,6 +1072,7 @@ TEST(DHCPRelayTest, from_client_relay_of_relay_append) { return true; }); from_client(&dhcpLayer, config); + FreeMockIfaddrs(mock_ifaddrs); } /* agent_relay_mode=replace: existing Option 82 stripped, ours added, packet forwarded. */ @@ -1028,6 +1085,7 @@ TEST(DHCPRelayTest, from_client_relay_of_relay_replace) { dhcpLayer.getDhcpHeader()->gatewayIpAddress = inet_addr("192.168.1.1"); encode_relay_option(&dhcpLayer, &config); + struct ifaddrs *mock_ifaddrs = expect_local_address_check(); EXPECT_GLOBAL_CALL(send_udp, send_udp(_, _, _, _, _, _, _)).WillOnce([] (int, uint8_t* hdr, struct sockaddr_in, uint32_t, in_addr, bool, bool) { pcpp::dhcp_header* dhcp_hdr = (pcpp::dhcp_header*)hdr; @@ -1036,6 +1094,7 @@ TEST(DHCPRelayTest, from_client_relay_of_relay_replace) { return true; }); from_client(&dhcpLayer, config); + FreeMockIfaddrs(mock_ifaddrs); } /* agent_relay_mode=forward: packet forwarded unchanged, Option 82 not modified. */ @@ -1048,6 +1107,7 @@ TEST(DHCPRelayTest, from_client_relay_of_relay_forward) { dhcpLayer.getDhcpHeader()->gatewayIpAddress = inet_addr("192.168.1.1"); encode_relay_option(&dhcpLayer, &config); + struct ifaddrs *mock_ifaddrs = expect_local_address_check(); EXPECT_GLOBAL_CALL(send_udp, send_udp(_, _, _, _, _, _, _)).WillOnce([] (int, uint8_t* hdr, struct sockaddr_in, uint32_t, in_addr, bool, bool) { pcpp::dhcp_header* dhcp_hdr = (pcpp::dhcp_header*)hdr; @@ -1056,6 +1116,7 @@ TEST(DHCPRelayTest, from_client_relay_of_relay_forward) { return true; }); from_client(&dhcpLayer, config); + FreeMockIfaddrs(mock_ifaddrs); } /* agent_relay_mode=discard: packet must be dropped, send_udp must NOT be called. */ @@ -1068,6 +1129,8 @@ TEST(DHCPRelayTest, from_client_relay_of_relay_discard) { dhcpLayer.getDhcpHeader()->gatewayIpAddress = inet_addr("192.168.1.1"); encode_relay_option(&dhcpLayer, &config); + struct ifaddrs *mock_ifaddrs = expect_local_address_check(); EXPECT_GLOBAL_CALL(send_udp, send_udp(_, _, _, _, _, _, _)).Times(0); from_client(&dhcpLayer, config); + FreeMockIfaddrs(mock_ifaddrs); }