[dhcp4relay] Implement RFC 6607 VSS-Control

dhcp4relay/src/dhcp4relay.cpp

@@ -455,11 +455,20 @@ int prepare_vlan_sockets(relay_config &config) {
 uint8_t encode_tlv(uint8_t *buf, uint8_t t, uint8_t l, uint8_t *v) {
     *buf = t;
     *(buf + DHCP_SUB_OPT_TLV_LENGTH_OFFSET) = l;
-    memcpy((buf + DHCP_SUB_OPT_TLV_HEADER_LEN), v, l);
+    /* VSS-Control (suboption 152) has a zero-length payload; calling memcpy
+       with its null payload pointer is unsafe even when the copy length is zero. */
+    if (l > 0) {
+        memcpy((buf + DHCP_SUB_OPT_TLV_HEADER_LEN), v, l);
+    }
 
     return (l + DHCP_SUB_OPT_TLV_HEADER_LEN);
 }
 
+static bool is_vss_required(const relay_config &config, const std::string &client_vrf) {
+    return ((config.vrf_selection_opt == "enable") && (client_vrf != "default") &&
+            (config.vrf != client_vrf));
+}
+
 std::string get_mac_address(const std::string &ifname) {
     std::string path = "/sys/class/net/" + ifname + "/address";
     std::ifstream file(path);
@@ -535,15 +544,19 @@ void encode_relay_option(pcpp::DhcpLayer *dhcp_pkt, relay_config *config) {
     /* | 151 | vrf_len | 0 | vrf_name | */
     uint8_t vss_buf[32] = {0};
     /* Enable VSS only if client and server are in two different VRF's */
-    if ((config->vrf_selection_opt == "enable") && (vrf != "default") &&
-        (config->vrf != vrf)) {
+    if (is_vss_required(*config, vrf)) {
         uint8_t zero_encode = 0;
         memcpy(vss_buf, &zero_encode, sizeof(uint8_t));
         memcpy((vss_buf + 1), (uint8_t *)vrf.c_str(), (uint8_t)vrf.length());
 
         offset = encode_tlv((buf + buf_offset), OPTION82_SUBOPT_VIRTUAL_SUBNET,
                             (uint8_t)(vrf.length() + 1), vss_buf);
         buf_offset += offset;
+
+        /* RFC 6607 requires VSS-Control whenever VSS is included. */
+        offset = encode_tlv((buf + buf_offset), OPTION82_SUBOPT_VIRTUAL_SUBNET_CONTROL,
+                            0, nullptr);
+        buf_offset += offset;
     }
 
     /* We shouldn't append relay information if packet size is exceeding MTU size */
@@ -664,7 +677,7 @@ uint8_t *decode_tlv(const uint8_t *buf, uint8_t t, uint8_t &l, uint32_t options_
 
     while (temp && ((offset + DHCP_SUB_OPT_TLV_HEADER_LEN) <= options_total_size)) {
         len = *(temp + DHCP_SUB_OPT_TLV_LENGTH_OFFSET);
-        if ((offset + DHCP_SUB_OPT_TLV_LENGTH_OFFSET + len) > options_total_size) {
+        if ((offset + DHCP_SUB_OPT_TLV_HEADER_LEN + len) > options_total_size) {
             /* Malformed packet */
             SWSS_LOG_ERROR("[DHCPV4_INFO] Failed to decode relay agent sub-option %d"
                        " exceeded total option len %d offset %d sub-option len %d",
@@ -684,6 +697,66 @@ uint8_t *decode_tlv(const uint8_t *buf, uint8_t t, uint8_t &l, uint32_t options_
     return NULL;
 }
 
+static bool is_valid_tlv_buffer(const uint8_t *buf, uint32_t options_total_size) {
+    uint32_t offset = 0;
+
+    while (offset < options_total_size) {
+        if ((options_total_size - offset) < DHCP_SUB_OPT_TLV_HEADER_LEN) {
+            return false;
+        }
+
+        const auto len = buf[offset + DHCP_SUB_OPT_TLV_LENGTH_OFFSET];
+        if (len > (options_total_size - offset - DHCP_SUB_OPT_TLV_HEADER_LEN)) {
+            return false;
+        }
+
+        offset += DHCP_SUB_OPT_TLV_HEADER_LEN + len;
+    }
+
+    return true;
+}
+
+bool validate_vss_reply(const uint8_t *options_ptr, uint32_t options_size,
+                        const relay_config &config, const std::string &src_ip) {
+    auto client_vrf_itr = vlan_vrf_map.find(config.vlan);
+    if ((client_vrf_itr == vlan_vrf_map.end()) ||
+        !is_vss_required(config, client_vrf_itr->second)) {
+        return true;
+    }
+
+    if ((options_ptr == nullptr) || !is_valid_tlv_buffer(options_ptr, options_size)) {
+        SWSS_LOG_WARN("[DHCPV4_RELAY] Dropping server reply for %s from %s: "
+                      "malformed relay agent sub-options",
+                      config.vlan.c_str(), src_ip.c_str());
+        return false;
+    }
+
+    uint8_t vss_control_len = 0;
+    auto vss_control_ptr = decode_tlv(options_ptr, OPTION82_SUBOPT_VIRTUAL_SUBNET_CONTROL,
+                                      vss_control_len, options_size);
+    if (vss_control_ptr != nullptr) {
+        SWSS_LOG_WARN("[DHCPV4_RELAY] Dropping server reply for %s from %s: "
+                      "server did not process VSS sub-option",
+                      config.vlan.c_str(), src_ip.c_str());
+        return false;
+    }
+
+    const auto &client_vrf = client_vrf_itr->second;
+    uint8_t vss_len = 0;
+    auto vss_ptr = decode_tlv(options_ptr, OPTION82_SUBOPT_VIRTUAL_SUBNET,
+                              vss_len, options_size);
+    if ((vss_ptr == nullptr) || (vss_len != (client_vrf.length() + 1)) ||
+        (vss_ptr[0] != 0) ||
+        (memcmp(vss_ptr + 1, client_vrf.data(), client_vrf.length()) != 0)) {
+        SWSS_LOG_WARN("[DHCPV4_RELAY] Dropping server reply for %s from %s: "
+                      "missing or mismatched VSS sub-option",
+                      config.vlan.c_str(), src_ip.c_str());
+        return false;
+    }
+
+    return true;
+}
+
 /**
  * @code                void to_client(pcpp::DhcpLayer* dhcp_pkt, std::unordered_map<std::string,
                                         relay_config > *vlans, std::string src_ip);
@@ -790,6 +863,12 @@ void to_client(pcpp::DhcpLayer *dhcp_pkt, std::unordered_map<std::string, relay_
     auto config = config_itr->second;
 
     dhcp_cntr_table.increment_counter(config.vlan, "RX", (int)dhcp_pkt->getMessageType());
+
+    if (!validate_vss_reply((const uint8_t *)options_ptr, agent_option_size, config, src_ip)) {
+        dhcp_cntr_table.increment_counter(config.vlan, "TX", DHCPv4_MESSAGE_TYPE_DROP);
+        return;
+    }
+
     /* TODO: Also check it is matching remote ID*/
 
     memcpy(&target_addr.sin_addr, &broadcast_addr, sizeof(struct in_addr));

dhcp4relay/src/dhcp4relay.h

@@ -60,6 +60,7 @@ extern int config_pipe[2];
 #define OPTION82_SUBOPT_LINK_SELECTION 5
 #define OPTION82_SUBOPT_SERVER_OVERRIDE 11
 #define OPTION82_SUBOPT_VIRTUAL_SUBNET 151
+#define OPTION82_SUBOPT_VIRTUAL_SUBNET_CONTROL 152
 
 #define DHCP_ETHERNET_HDR_LEN 14
 #define DHCP_IP_HDR_LEN 20
@@ -322,3 +323,7 @@ void pkt_in_callback(evutil_socket_t fd, short event, void *arg);
 void config_event_callback(evutil_socket_t fd, short event, void *arg);
 uint8_t *decode_tlv(const uint8_t *buf, uint8_t t, uint8_t &l, uint32_t options_total_size);
 uint8_t encode_tlv(uint8_t *buf, uint8_t t, uint8_t l, uint8_t *v);
+#ifdef UNIT_TEST
+bool validate_vss_reply(const uint8_t *options_ptr, uint32_t options_size,
+                        const relay_config &config, const std::string &src_ip);
+#endif

dhcp4relay/test/mock_relay.cpp

@@ -100,6 +100,24 @@ TEST(EncodeDecodeTLV, EncodeAndDecode) {
     EXPECT_EQ(decoded_value[0], 0x11);
     EXPECT_EQ(decoded_value[1], 0x22);
     EXPECT_EQ(decoded_value[2], 0x33);
+
+    uint8_t empty_buffer[2] = {};
+    encoded_length = encode_tlv(empty_buffer, OPTION82_SUBOPT_VIRTUAL_SUBNET_CONTROL,
+                                0, nullptr);
+    EXPECT_EQ(encoded_length, 2);
+    EXPECT_EQ(empty_buffer[0], OPTION82_SUBOPT_VIRTUAL_SUBNET_CONTROL);
+    EXPECT_EQ(empty_buffer[1], 0);
+
+    decoded_value = decode_tlv(empty_buffer, OPTION82_SUBOPT_VIRTUAL_SUBNET_CONTROL,
+                               length, sizeof(empty_buffer));
+    ASSERT_NE(decoded_value, nullptr);
+    EXPECT_EQ(length, 0);
+
+    uint8_t truncated_buffer[2] = {OPTION82_SUBOPT_VIRTUAL_SUBNET, 1};
+    decoded_value = decode_tlv(truncated_buffer, OPTION82_SUBOPT_VIRTUAL_SUBNET,
+                               length, sizeof(truncated_buffer));
+    EXPECT_EQ(decoded_value, nullptr);
+    EXPECT_EQ(length, 0);
 }
 
 TEST(sock, sock_open) {
@@ -820,6 +838,13 @@ TEST(DHCPRelayTest, encode_relay_option) {
     memcpy((vss_buf + 1), (uint8_t*)vlan_vrf_map["Vlan10"].c_str(), (uint8_t)vlan_vrf_map["Vlan10"].length());
 
     EXPECT_EQ(memcmp(vss_buf, vrf_ptr, 6), 0);
+
+    uint8_t vss_control_len = 1;
+    auto vss_control_ptr = decode_tlv((const uint8_t *)options_ptr,
+                                      OPTION82_SUBOPT_VIRTUAL_SUBNET_CONTROL,
+                                      vss_control_len, agent_option_size);
+    ASSERT_NE(vss_control_ptr, nullptr);
+    EXPECT_EQ(vss_control_len, 0);
 }
 
 TEST(DHCPRelayTest, encode_relay_option_server_client_same_vrf) {
@@ -898,6 +923,54 @@ TEST(DHCPRelayTest, encode_relay_option_server_client_same_vrf) {
                            vrf_len, agent_option_size);
 
     EXPECT_EQ((uintptr_t)vrf_ptr, NULL);
+
+    uint8_t vss_control_len = 0;
+    auto vss_control_ptr = decode_tlv((const uint8_t *)options_ptr,
+                                      OPTION82_SUBOPT_VIRTUAL_SUBNET_CONTROL,
+                                      vss_control_len, agent_option_size);
+    EXPECT_EQ((uintptr_t)vss_control_ptr, NULL);
+}
+
+static uint32_t build_vss_reply_options(uint8_t *options, const std::string &vrf,
+                                        bool include_vss, bool include_vss_control) {
+    uint32_t offset = 0;
+    if (include_vss) {
+        std::vector<uint8_t> vss_data(vrf.length() + 1, 0);
+        memcpy(vss_data.data() + 1, vrf.data(), vrf.length());
+        offset += encode_tlv(options + offset, OPTION82_SUBOPT_VIRTUAL_SUBNET,
+                             static_cast<uint8_t>(vss_data.size()), vss_data.data());
+    }
+    if (include_vss_control) {
+        offset += encode_tlv(options + offset, OPTION82_SUBOPT_VIRTUAL_SUBNET_CONTROL,
+                             0, nullptr);
+    }
+    return offset;
+}
+
+TEST(DHCPRelayTest, validate_vss_reply) {
+    relay_config config = {};
+    config.vlan = "Vlan10";
+    config.vrf = "Vrf03";
+    config.vrf_selection_opt = "enable";
+    vlan_vrf_map["Vlan10"] = "Vrf01";
+
+    uint8_t options[32] = {};
+    auto options_size = build_vss_reply_options(options, "Vrf01", true, false);
+    EXPECT_TRUE(validate_vss_reply(options, options_size, config, "192.0.2.1"));
+
+    options_size = build_vss_reply_options(options, "Vrf01", true, true);
+    EXPECT_FALSE(validate_vss_reply(options, options_size, config, "192.0.2.1"));
+
+    options_size = build_vss_reply_options(options, "Vrf01", true, false);
+    options[options_size++] = OPTION82_SUBOPT_VIRTUAL_SUBNET_CONTROL;
+    options[options_size++] = 1;
+    EXPECT_FALSE(validate_vss_reply(options, options_size, config, "192.0.2.1"));
+
+    options_size = build_vss_reply_options(options, "Vrf02", true, false);
+    EXPECT_FALSE(validate_vss_reply(options, options_size, config, "192.0.2.1"));
+
+    options_size = build_vss_reply_options(options, "Vrf01", false, false);
+    EXPECT_FALSE(validate_vss_reply(options, options_size, config, "192.0.2.1"));
 }
 
 TEST(DHCPRelayTest, to_client) {
@@ -924,6 +997,8 @@ TEST(DHCPRelayTest, to_client) {
     relay_config config = {};
     config.phy_interface = "Ethernet12";
     config.vlan = "Vlan10";
+    config.vrf = "Vrf01";
+    config.client_sock = 1;
     config.link_selection_opt = "enable";
     config.server_id_override_opt = "enable";
     config.link_address.sin_addr.s_addr = inet_addr("192.168.10.10");