validate the dhcp server-id in dhcp_msg_type so a forged dhcpnak with an absent or mismatched server identifier can no longer deconfigure the lease

this pretty much mirrors the guard already in dhcp_parse_ack

Author: gasbytes

src/test/unit/unit.c

@@ -1289,7 +1289,7 @@ Suite *wolf_suite(void)
 #ifdef IP_MULTICAST
     tcase_add_test(tc_core, test_poll_tx_udp_multicast_arp_skipped_uses_mcast_mac);
 #endif /* IP_MULTICAST */
-    /* --- unit_tests_dhcp_edges.c (43 tests) --- */
+    /* --- unit_tests_dhcp_edges.c (46 tests) --- */
     tcase_add_test(tc_core, test_dhcp_schedule_lease_timer_zero_lease_noop);
     tcase_add_test(tc_core, test_dhcp_schedule_lease_timer_null_noop);
     tcase_add_test(tc_core, test_dhcp_schedule_lease_timer_renew_gt_lease_clamped);
@@ -1307,6 +1307,9 @@ Suite *wolf_suite(void)
     tcase_add_test(tc_core, test_dhcp_msg_type_len_ne_1_not_returned);
     tcase_add_test(tc_core, test_dhcp_msg_type_pad_bytes_skipped);
     tcase_add_test(tc_core, test_dhcp_msg_type_truncated_option_returns_neg1);
+    tcase_add_test(tc_core, test_dhcp_msg_type_nak_wrong_server_id_rejected);
+    tcase_add_test(tc_core, test_dhcp_msg_type_nak_absent_server_id_rejected);
+    tcase_add_test(tc_core, test_dhcp_msg_type_nak_matching_server_id_accepted);
     tcase_add_test(tc_core, test_dhcp_parse_offer_type_ack_not_offer_rejected);
     tcase_add_test(tc_core, test_dhcp_parse_offer_subnet_mask_len_lt4_rejected);
     tcase_add_test(tc_core, test_dhcp_parse_offer_inner_truncated_opt2_rejected);

src/test/unit/unit_tests_dhcp_edges.c

@@ -371,8 +371,62 @@ START_TEST(test_dhcp_msg_type_truncated_option_returns_neg1)
 }
 END_TEST
 
+START_TEST(test_dhcp_msg_type_nak_wrong_server_id_rejected)
+{
+    struct wolfIP s;
+    struct dhcp_msg msg;
+    uint8_t *p;
+
+    wolfIP_init(&s);
+    s.dhcp_xid = 0xDEADBEEFU;
+    s.dhcp_server_ip = 0x0A000001U;          /* committed server */
+
+    build_dhcp_msg_base(&s, &msg, DHCP_NAK);
+    p = (uint8_t *)msg.options + 3;          /* after MSG_TYPE TLV */
+    append_opt4(&p, DHCP_OPTION_SERVER_ID, 0x0A000002U); /* different server */
+    append_end(&p);
+
+    ck_assert_int_eq(dhcp_msg_type(&s, &msg, sizeof(msg)), -1);
+}
+END_TEST
+
+START_TEST(test_dhcp_msg_type_nak_absent_server_id_rejected)
+{
+    struct wolfIP s;
+    struct dhcp_msg msg;
+
+    wolfIP_init(&s);
+    s.dhcp_xid = 0xDEADBEEFU;
+    s.dhcp_server_ip = 0x0A000001U;          /* committed server */
+
+    build_dhcp_msg_base(&s, &msg, DHCP_NAK); /* no SERVER_ID option */
+    ((uint8_t *)msg.options)[3] = DHCP_OPTION_END;
+
+    ck_assert_int_eq(dhcp_msg_type(&s, &msg, sizeof(msg)), -1);
+}
+END_TEST
+
+START_TEST(test_dhcp_msg_type_nak_matching_server_id_accepted)
+{
+    struct wolfIP s;
+    struct dhcp_msg msg;
+    uint8_t *p;
+
+    wolfIP_init(&s);
+    s.dhcp_xid = 0xDEADBEEFU;
+    s.dhcp_server_ip = 0x0A000001U;          /* committed server */
+
+    build_dhcp_msg_base(&s, &msg, DHCP_NAK);
+    p = (uint8_t *)msg.options + 3;
+    append_opt4(&p, DHCP_OPTION_SERVER_ID, 0x0A000001U); /* committed server */
+    append_end(&p);
+
+    ck_assert_int_eq(dhcp_msg_type(&s, &msg, sizeof(msg)), DHCP_NAK);
+}
+END_TEST
+
 /* -------------------------------------------------------------------------
- * dhcp_parse_offer — additional branches
+ * dhcp_parse_offer - additional branches
  * ---------------------------------------------------------------------- */
 
 START_TEST(test_dhcp_parse_offer_type_ack_not_offer_rejected)

src/wolfip.c

@@ -7592,6 +7592,9 @@ static int dhcp_msg_type(struct wolfIP *s, struct dhcp_msg *msg, uint32_t msg_le
 {
     uint8_t *opt = (uint8_t *)msg->options;
     uint8_t *opt_end;
+    int msg_type = -1;
+    int saw_server_id = 0;
+    uint32_t server_id = 0;
     if (msg_len < DHCP_HEADER_LEN)
         return -1;
     if (ee32(msg->magic) != DHCP_MAGIC)
@@ -7618,11 +7621,20 @@ static int dhcp_msg_type(struct wolfIP *s, struct dhcp_msg *msg, uint32_t msg_le
         len = opt[1];
         if (opt + 2 + len > opt_end)
             break;
-        if (code == DHCP_OPTION_MSG_TYPE && len == 1)
-            return opt[2];
+        if (code == DHCP_OPTION_MSG_TYPE && len == 1) {
+            msg_type = opt[2];
+        } else if (code == DHCP_OPTION_SERVER_ID && len >= 4) {
+            server_id = DHCP_OPT_data_to_u32((struct dhcp_option *)opt);
+            saw_server_id = 1;
+        }
         opt += 2 + len;
     }
-    return -1;
+    /* Reject a reply that does not carry the server identifier of the
+     * server we committed to during the OFFER phase. */
+    if (s->dhcp_server_ip != 0 &&
+        (!saw_server_id || server_id != s->dhcp_server_ip))
+        return -1;
+    return msg_type;
 }
 
 static int dhcp_parse_ack(struct wolfIP *s, struct dhcp_msg *msg, uint32_t msg_len)
@@ -8282,11 +8294,6 @@ static void arp_recv(struct wolfIP *s, unsigned int if_idx, void *buf, int len)
                     if (memcmp(s->arp.neighbors[idx].mac, sender_mac, 6) == 0)
                         s->arp.neighbors[idx].ts = s->last_tick;
                 }
-                /* Do not learn a new neighbor from an unsolicited REQUEST:
-                 * a same-LAN attacker can spoof sip to match an outstanding pending request and install
-                 * a forged MAC that then locks out the genuine reply. Only REPLYs
-                 * populate new cache entries; the pending slot is left intact so the genuine
-                 * reply still resolves. */
             }
         }
         eth_output_add_header(s, if_idx, arp->tma, &arp->eth, ETH_TYPE_ARP);