pinctrl: Check requested IP in DHCPREQUEST messages

See RFC 2131, section 4.3.2. When handling a DHCPREQUEST message, the
server should validate that the client's requested IP matches the
offered IP. If not, the server should reply with a DHCPNAK. The client's
requested IP is either specified as the Requested IP Address (option
50), or as the ciaddr, depending on the client's state.

Signed-off-by: Gregory Smith <gasmith@nutanix.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>

2 files changed, 102 insertions, 25 deletions

diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 3cd2ad718..5aebbfc2c 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -608,14 +608,23 @@ pinctrl_handle_put_dhcp_opts(
      *| UDP HEADER  | DHCP HEADER  | 4 Byte DHCP Cookie | DHCP OPTIONS(var len)|
      * ------------------------------------------------------------------------
      */
-    if (dp_packet_l4_size(pkt_in) < (UDP_HEADER_LEN +
-        sizeof (struct dhcp_header) + sizeof(uint32_t) + 3)) {
+
+    const char *end = (char *)dp_packet_l4(pkt_in) + dp_packet_l4_size(pkt_in);
+    const char *in_dhcp_ptr = dp_packet_get_udp_payload(pkt_in);
+    if (!in_dhcp_ptr) {
         static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
         VLOG_WARN_RL(&rl, "Invalid or incomplete DHCP packet received");
         goto exit;
     }
 
-    struct dhcp_header const *in_dhcp_data = dp_packet_get_udp_payload(pkt_in);
+    const struct dhcp_header *in_dhcp_data
+        = (const struct dhcp_header *) in_dhcp_ptr;
+    in_dhcp_ptr += sizeof *in_dhcp_data;
+    if (in_dhcp_ptr > end) {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+        VLOG_WARN_RL(&rl, "Invalid or incomplete DHCP packet received");
+        goto exit;
+    }
     if (in_dhcp_data->op != DHCP_OP_REQUEST) {
         static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
         VLOG_WARN_RL(&rl, "Invalid opcode in the DHCP packet : %d",
@@ -626,41 +635,90 @@ pinctrl_handle_put_dhcp_opts(
     /* DHCP options follow the DHCP header. The first 4 bytes of the DHCP
      * options is the DHCP magic cookie followed by the actual DHCP options.
      */
-    const uint8_t *in_dhcp_opt =
-        (const uint8_t *)dp_packet_get_udp_payload(pkt_in) +
-        sizeof (struct dhcp_header);
-
     ovs_be32 magic_cookie = htonl(DHCP_MAGIC_COOKIE);
-    if (memcmp(in_dhcp_opt, &magic_cookie, sizeof(ovs_be32))) {
+    if (in_dhcp_ptr + sizeof magic_cookie > end ||
+        get_unaligned_be32((const void *) in_dhcp_ptr) != magic_cookie) {
         static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
         VLOG_WARN_RL(&rl, "DHCP magic cookie not present in the DHCP packet");
         goto exit;
     }
+    in_dhcp_ptr += sizeof magic_cookie;
+
+    const uint8_t *in_dhcp_msg_type = NULL;
+    ovs_be32 request_ip = in_dhcp_data->ciaddr;
+    while (in_dhcp_ptr < end) {
+        const struct dhcp_opt_header *in_dhcp_opt =
+            (const struct dhcp_opt_header *)in_dhcp_ptr;
+        if (in_dhcp_opt->code == DHCP_OPT_END) {
+            break;
+        }
+        if (in_dhcp_opt->code == DHCP_OPT_PAD) {
+            in_dhcp_ptr += 1;
+            continue;
+        }
+        in_dhcp_ptr += sizeof *in_dhcp_opt;
+        if (in_dhcp_ptr > end) {
+            break;
+        }
+        in_dhcp_ptr += in_dhcp_opt->len;
+        if (in_dhcp_ptr > end) {
+            break;
+        }
+
+        switch (in_dhcp_opt->code) {
+        case DHCP_OPT_MSG_TYPE:
+            if (in_dhcp_opt->len == 1) {
+                in_dhcp_msg_type = DHCP_OPT_PAYLOAD(in_dhcp_opt);
+            }
+            break;
+        case DHCP_OPT_REQ_IP:
+            if (in_dhcp_opt->len == 4) {
+                request_ip = get_unaligned_be32(DHCP_OPT_PAYLOAD(in_dhcp_opt));
+            }
+            break;
+        default:
+            break;
+        }
+    }
 
-    in_dhcp_opt += 4;
     /* Check that the DHCP Message Type (opt 53) is present or not with
-     * valid values - DHCP_MSG_DISCOVER or DHCP_MSG_REQUEST as the first
-     * DHCP option.
+     * valid values - DHCP_MSG_DISCOVER or DHCP_MSG_REQUEST.
      */
-    if (!(in_dhcp_opt[0] == DHCP_OPT_MSG_TYPE && in_dhcp_opt[1] == 1 && (
-            in_dhcp_opt[2] == DHCP_MSG_DISCOVER ||
-            in_dhcp_opt[2] == DHCP_MSG_REQUEST))) {
+    if (!in_dhcp_msg_type) {
         static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-        VLOG_WARN_RL(&rl, "Invalid DHCP message type : opt code = %d,"
-                     " opt value = %d", in_dhcp_opt[0], in_dhcp_opt[2]);
+        VLOG_WARN_RL(&rl, "Missing DHCP message type");
+        goto exit;
+    }
+    if (*in_dhcp_msg_type != DHCP_MSG_DISCOVER &&
+        *in_dhcp_msg_type != DHCP_MSG_REQUEST) {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+        VLOG_WARN_RL(&rl, "Invalid DHCP message type : %d", *in_dhcp_msg_type);
         goto exit;
     }
 
     uint8_t msg_type;
-    if (in_dhcp_opt[2] == DHCP_MSG_DISCOVER) {
+    if (*in_dhcp_msg_type == DHCP_MSG_DISCOVER) {
         msg_type = DHCP_MSG_OFFER;
     } else {
+        /* This is a DHCPREQUEST. If the client has requested an IP that
+         * does not match the offered IP address, reply with a NAK. The
+         * requested IP address may be supplied either via Requested IP Address
+         * (opt 50) or via ciaddr, depending on the client's state.
+         */
         msg_type = DHCP_MSG_ACK;
+        if (request_ip != *offer_ip) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+            VLOG_WARN_RL(&rl, "DHCPREQUEST requested IP "IP_FMT" does not "
+                         "match offer "IP_FMT, IP_ARGS(request_ip),
+                         IP_ARGS(*offer_ip));
+            msg_type = DHCP_MSG_NAK;
+        }
     }
 
     /* Frame the DHCP reply packet
      * Total DHCP options length will be options stored in the userdata +
-     * 16 bytes.
+     * 16 bytes. Note that the DHCP options stored in userdata are not included
+     * in DHCPNAK messages.
      *
      * --------------------------------------------------------------
      *| 4 Bytes (dhcp cookie) | 3 Bytes (option type) | DHCP options |
@@ -668,8 +726,10 @@ pinctrl_handle_put_dhcp_opts(
      *| 4 Bytes padding | 1 Byte (option end 0xFF ) | 4 Bytes padding|
      * --------------------------------------------------------------
      */
-    uint16_t new_l4_size = UDP_HEADER_LEN + DHCP_HEADER_LEN + \
-                           userdata->size + 16;
+    uint16_t new_l4_size = UDP_HEADER_LEN + DHCP_HEADER_LEN + 16;
+    if (msg_type != DHCP_MSG_NAK) {
+        new_l4_size += userdata->size;
+    }
     size_t new_packet_size = pkt_in->l4_ofs + new_l4_size;
 
     struct dp_packet pkt_out;
@@ -693,19 +753,26 @@ pinctrl_handle_put_dhcp_opts(
     struct dhcp_header *dhcp_data = dp_packet_put(
         &pkt_out, dp_packet_pull(pkt_in, DHCP_HEADER_LEN), DHCP_HEADER_LEN);
     dhcp_data->op = DHCP_OP_REPLY;
-    dhcp_data->yiaddr = *offer_ip;
+    dhcp_data->yiaddr = (msg_type == DHCP_MSG_NAK) ? 0 : *offer_ip;
     dp_packet_put(&pkt_out, &magic_cookie, sizeof(ovs_be32));
 
+    uint16_t out_dhcp_opts_size = 12;
+    if (msg_type != DHCP_MSG_NAK) {
+      out_dhcp_opts_size += userdata->size;
+    }
     uint8_t *out_dhcp_opts = dp_packet_put_zeros(&pkt_out,
-                                                 userdata->size + 12);
+                                                 out_dhcp_opts_size);
     /* DHCP option - type */
     out_dhcp_opts[0] = DHCP_OPT_MSG_TYPE;
     out_dhcp_opts[1] = 1;
     out_dhcp_opts[2] = msg_type;
     out_dhcp_opts += 3;
 
-    memcpy(out_dhcp_opts, userdata->data, userdata->size);
-    out_dhcp_opts += userdata->size;
+    if (msg_type != DHCP_MSG_NAK) {
+      memcpy(out_dhcp_opts, userdata->data, userdata->size);
+      out_dhcp_opts += userdata->size;
+    }
+
     /* Padding */
     out_dhcp_opts += 4;
     /* End */
@@ -727,7 +794,8 @@ pinctrl_handle_put_dhcp_opts(
     static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(20, 40);
     const struct eth_header *l2 = dp_packet_eth(&pkt_out);
     VLOG_INFO_RL(&rl, "DHCP%s "ETH_ADDR_FMT" "IP_FMT"",
-                 msg_type == DHCP_MSG_OFFER ? "OFFER" : "ACK",
+                 msg_type == DHCP_MSG_OFFER ? "OFFER" :
+                   (msg_type == DHCP_MSG_ACK ? "ACK": "NAK"),
                  ETH_ADDR_ARGS(l2->eth_src), IP_ARGS(*offer_ip));
 
     success = 1;
diff --git a/ovn/lib/ovn-l7.h b/ovn/lib/ovn-l7.h
index 08c3da54a..2412c9e06 100644
--- a/ovn/lib/ovn-l7.h
+++ b/ovn/lib/ovn-l7.h
@@ -138,6 +138,15 @@ dhcp_opts_destroy(struct hmap *dhcp_opts)
     gen_opts_destroy(dhcp_opts);
 }
 
+OVS_PACKED(
+struct dhcp_opt_header {
+    uint8_t code;
+    uint8_t len;
+});
+
+#define DHCP_OPT_PAYLOAD(hdr) \
+    (void *)((char *)hdr + sizeof(struct dhcp_opt_header))
+
 /* Used in the OpenFlow PACKET_IN userdata */
 struct dhcp_opt6_header {
     ovs_be16 opt_code;