From 4676ee5d5b480f920fb844abdc198daf64ac1c8f Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 19 Jul 2026 16:14:55 +0000 Subject: [PATCH 1/2] [dhcp4relay]: Forward chained requests without relay information Apply agent relay mode only when a chained request actually contains Option 82, preserving giaddr and normal hop processing otherwise. 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 | 6 +++++- dhcp4relay/test/mock_relay.cpp | 23 +++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/dhcp4relay/src/dhcp4relay.cpp b/dhcp4relay/src/dhcp4relay.cpp index bdd0979..7019df5 100644 --- a/dhcp4relay/src/dhcp4relay.cpp +++ b/dhcp4relay/src/dhcp4relay.cpp @@ -601,7 +601,11 @@ void from_client(pcpp::DhcpLayer *dhcp_pkt, relay_config &config) { forward - Forward the packet unchanged (no Option 82 modification). discard - Discard the incoming packet (default). */ - if (config.agent_relay_mode == "append") { + auto agent_option = dhcp_pkt->getOptionData(pcpp::DHCPOPT_DHCP_AGENT_OPTIONS); + if (agent_option.isNull()) { + SWSS_LOG_DEBUG("[DHCPV4_RELAY] Forwarding chained packet without Option 82 on %s", + config.vlan.c_str()); + } else if (config.agent_relay_mode == "append") { encode_relay_option(dhcp_pkt, &config); } else if (config.agent_relay_mode == "replace") { dhcp_pkt->removeOption(pcpp::DHCPOPT_DHCP_AGENT_OPTIONS); diff --git a/dhcp4relay/test/mock_relay.cpp b/dhcp4relay/test/mock_relay.cpp index 02a44ab..ff77356 100644 --- a/dhcp4relay/test/mock_relay.cpp +++ b/dhcp4relay/test/mock_relay.cpp @@ -997,6 +997,29 @@ static relay_config make_relay_of_relay_config(const std::string &agent_relay_mo return config; } +TEST(DHCPRelayTest, from_client_relay_of_relay_without_option82_forwards) { + relay_config config = make_relay_of_relay_config("discard"); + + 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.1"); + + 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("192.168.1.1")); + return true; + }); + + from_client(&dhcpLayer, config); + + EXPECT_EQ(dhcpLayer.getDhcpHeader()->hops, 3); + EXPECT_EQ(dhcpLayer.getDhcpHeader()->gatewayIpAddress, inet_addr("192.168.1.1")); + EXPECT_TRUE(dhcpLayer.getOptionData(pcpp::DHCPOPT_DHCP_AGENT_OPTIONS).isNull()); +} + /* 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"); From 0c4588b730c845799e8f3e0e3898c4b284672be9 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 19 Jul 2026 17:50:25 +0000 Subject: [PATCH 2/2] [dhcp4relay]: Safely detect chained Option 82 Walk DHCP options with cookie, length, PAD, and END validation so bytes after END cannot make a no-option chained request subject to relay-agent mode. 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 | 49 ++++++++++++++++++++++++++++++++-- dhcp4relay/test/mock_relay.cpp | 32 ++++++++++++++++++++++ 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/dhcp4relay/src/dhcp4relay.cpp b/dhcp4relay/src/dhcp4relay.cpp index 7019df5..71b5e6e 100644 --- a/dhcp4relay/src/dhcp4relay.cpp +++ b/dhcp4relay/src/dhcp4relay.cpp @@ -560,6 +560,52 @@ void encode_relay_option(pcpp::DhcpLayer *dhcp_pkt, relay_config *config) { return; } +static bool has_dhcp_option(const pcpp::DhcpLayer *dhcp_pkt, + pcpp::DhcpOptionTypes option_type) { + const pcpp::dhcp_header *dhcp_header = dhcp_pkt->getDhcpHeader(); + if (dhcp_header == nullptr || dhcp_header->magicNumber != DHCP_MAGIC_NUMBER) { + return false; + } + + size_t remaining = dhcp_pkt->getHeaderLen(); + if (remaining <= sizeof(pcpp::dhcp_header)) { + return false; + } + + const uint8_t *option = reinterpret_cast(dhcp_header) + + sizeof(pcpp::dhcp_header); + remaining -= sizeof(pcpp::dhcp_header); + + while (remaining > 0) { + const uint8_t option_code = *option++; + --remaining; + + if (option_code == static_cast(pcpp::DHCPOPT_PAD)) { + continue; + } + if (option_code == static_cast(pcpp::DHCPOPT_END)) { + return false; + } + if (remaining == 0) { + return false; + } + + const uint8_t option_len = *option++; + --remaining; + if (option_len > remaining) { + return false; + } + if (option_code == static_cast(option_type)) { + return true; + } + + option += option_len; + remaining -= option_len; + } + + return false; +} + /** * @code void from_client(pcpp::DhcpLayer* dhcp_pkt, relay_config *config) * @@ -601,8 +647,7 @@ void from_client(pcpp::DhcpLayer *dhcp_pkt, relay_config &config) { forward - Forward the packet unchanged (no Option 82 modification). discard - Discard the incoming packet (default). */ - auto agent_option = dhcp_pkt->getOptionData(pcpp::DHCPOPT_DHCP_AGENT_OPTIONS); - if (agent_option.isNull()) { + if (!has_dhcp_option(dhcp_pkt, pcpp::DHCPOPT_DHCP_AGENT_OPTIONS)) { SWSS_LOG_DEBUG("[DHCPV4_RELAY] Forwarding chained packet without Option 82 on %s", config.vlan.c_str()); } else if (config.agent_relay_mode == "append") { diff --git a/dhcp4relay/test/mock_relay.cpp b/dhcp4relay/test/mock_relay.cpp index ff77356..7e268bb 100644 --- a/dhcp4relay/test/mock_relay.cpp +++ b/dhcp4relay/test/mock_relay.cpp @@ -1020,6 +1020,38 @@ TEST(DHCPRelayTest, from_client_relay_of_relay_without_option82_forwards) { EXPECT_TRUE(dhcpLayer.getOptionData(pcpp::DHCPOPT_DHCP_AGENT_OPTIONS).isNull()); } +TEST(DHCPRelayTest, from_client_relay_of_relay_ignores_option82_after_end) { + relay_config config = make_relay_of_relay_config("discard"); + const uint8_t options[] = { + static_cast(pcpp::DHCPOPT_DHCP_MESSAGE_TYPE), 1, + static_cast(pcpp::DHCP_DISCOVER), + static_cast(pcpp::DHCPOPT_PAD), + static_cast(pcpp::DHCPOPT_END), + static_cast(pcpp::DHCPOPT_DHCP_AGENT_OPTIONS), 2, 1, 0, + }; + const size_t packet_len = sizeof(pcpp::dhcp_header) + sizeof(options); + auto *packet = new uint8_t[packet_len](); + auto *dhcp_header = reinterpret_cast(packet); + dhcp_header->hops = 2; + dhcp_header->gatewayIpAddress = inet_addr("192.168.1.1"); + dhcp_header->magicNumber = DHCP_MAGIC_NUMBER; + memcpy(packet + sizeof(pcpp::dhcp_header), options, sizeof(options)); + pcpp::DhcpLayer dhcpLayer(packet, packet_len, nullptr, nullptr); + + EXPECT_GLOBAL_CALL(send_udp, send_udp(_, _, _, _, _, _, _)).WillOnce([] + (int, uint8_t *hdr, struct sockaddr_in, uint32_t, in_addr, bool, bool) { + auto *forwarded_header = reinterpret_cast(hdr); + EXPECT_EQ(forwarded_header->hops, 3); + EXPECT_EQ(forwarded_header->gatewayIpAddress, inet_addr("192.168.1.1")); + return true; + }); + + from_client(&dhcpLayer, config); + + EXPECT_EQ(dhcpLayer.getDhcpHeader()->hops, 3); + EXPECT_EQ(dhcpLayer.getHeaderLen(), packet_len); +} + /* 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");