feat(dhcp): client identifier option #10

vulptex · 2022-02-21T12:28:42Z

Add DHCP option 61. This is used by a lot of DHCP servers in enterprise environment to determine the correctness of the client (whitelisting).

Frame 151: 350 bytes on wire (2800 bits), 350 bytes captured (2800 bits) on interface enp6s0, id 0
    Interface id: 0 (enp6s0)
        Interface name: enp6s0
    Encapsulation type: Ethernet (1)
    Arrival Time: Feb 21, 2022 13:32:00.464284021 CET
    [Time shift for this packet: 0.000000000 seconds]
    Epoch Time: 1645446720.464284021 seconds
    [Time delta from previous captured frame: 60.002995977 seconds]
    [Time delta from previous displayed frame: 60.002995977 seconds]
    [Time since reference or first frame: 4083.246598659 seconds]
    Frame Number: 151
    Frame Length: 350 bytes (2800 bits)
    Capture Length: 350 bytes (2800 bits)
    [Frame is marked: False]
    [Frame is ignored: False]
    [Protocols in frame: eth:ethertype:ip:udp:dhcp]
    [Coloring Rule Name: UDP]
    [Coloring Rule String: udp]
Ethernet II, Src: 7e:13:2a:ca:55:db (7e:13:2a:ca:55:db), Dst: Broadcast (ff:ff:ff:ff:ff:ff)
    Destination: Broadcast (ff:ff:ff:ff:ff:ff)
        Address: Broadcast (ff:ff:ff:ff:ff:ff)
        .... ..1. .... .... .... .... = LG bit: Locally administered address (this is NOT the factory default)
        .... ...1 .... .... .... .... = IG bit: Group address (multicast/broadcast)
    Source: 7e:13:2a:ca:55:db (7e:13:2a:ca:55:db)
        Address: 7e:13:2a:ca:55:db (7e:13:2a:ca:55:db)
        .... ..1. .... .... .... .... = LG bit: Locally administered address (this is NOT the factory default)
        .... ...0 .... .... .... .... = IG bit: Individual address (unicast)
    Type: IPv4 (0x0800)
Internet Protocol Version 4, Src: 0.0.0.0, Dst: 255.255.255.255
    0100 .... = Version: 4
    .... 0101 = Header Length: 20 bytes (5)
    Differentiated Services Field: 0x00 (DSCP: CS0, ECN: Not-ECT)
        0000 00.. = Differentiated Services Codepoint: Default (0)
        .... ..00 = Explicit Congestion Notification: Not ECN-Capable Transport (0)
    Total Length: 336
    Identification: 0x0012 (18)
    Flags: 0x00
        0... .... = Reserved bit: Not set
        .0.. .... = Don't fragment: Not set
        ..0. .... = More fragments: Not set
    ...0 0000 0000 0000 = Fragment Offset: 0
    Time to Live: 255
    Protocol: UDP (17)
    Header Checksum: 0xba8b [validation disabled]
    [Header checksum status: Unverified]
    Source Address: 0.0.0.0
    Destination Address: 255.255.255.255
User Datagram Protocol, Src Port: 68, Dst Port: 67
    Source Port: 68
    Destination Port: 67
    Length: 316
    Checksum: 0xe823 [unverified]
    [Checksum Status: Unverified]
    [Stream index: 0]
    [Timestamps]
        [Time since first frame: 4073.201294165 seconds]
        [Time since previous frame: 60.002995977 seconds]
    UDP payload (308 bytes)
Dynamic Host Configuration Protocol (Discover)
    Message type: Boot Request (1)
    Hardware type: Ethernet (0x01)
    Hardware address length: 6
    Hops: 0
    Transaction ID: 0x5851f42d
    Seconds elapsed: 0
    Bootp flags: 0x0000 (Unicast)
        0... .... .... .... = Broadcast flag: Unicast
        .000 0000 0000 0000 = Reserved flags: 0x0000
    Client IP address: 0.0.0.0
    Your (client) IP address: 0.0.0.0
    Next server IP address: 0.0.0.0
    Relay agent IP address: 0.0.0.0
    Client MAC address: 7e:13:2a:ca:55:db (7e:13:2a:ca:55:db)
    Client hardware address padding: 00000000000000000000
    Server host name not given
    Boot file name not given
    Magic cookie: DHCP
    Option: (53) DHCP Message Type (Discover)
        Length: 1
        DHCP: Discover (1)
    Option: (57) Maximum DHCP Message Size
        Length: 2
        Maximum DHCP Message Size: 1500
    Option: (12) Host Name
        Length: 27
        Host Name: MyDeviceRocks55db
    Option: (61) Client identifier
        Length: 7
        Hardware type: Ethernet (0x01)
        Client MAC address: 7e:13:2a:ca:55:db (7e:13:2a:ca:55:db)
    Option: (55) Parameter Request List
        Length: 4
        Parameter Request List Item: (1) Subnet Mask
        Parameter Request List Item: (3) Router
        Parameter Request List Item: (28) Broadcast Address
        Parameter Request List Item: (6) Domain Name Server
    Option: (255) End
        Option End: 255
    Padding: 00000000000000000000000000000000

vulptex · 2022-02-21T12:29:37Z

@yarrick ;-)

yarrick

Please remove the feat() part of the commit title, and add a description what is used as identifier (the RFC allows any data)

yarrick · 2022-03-03T21:04:44Z

src/core/ipv4/dhcp.c

@@ -1003,6 +1014,14 @@ dhcp_discover(struct netif *netif)
    options_out_len = dhcp_option(options_out_len, msg_out->options, DHCP_OPTION_MAX_MSG_SIZE, DHCP_OPTION_MAX_MSG_SIZE_LEN);
    options_out_len = dhcp_option_short(options_out_len, msg_out->options, DHCP_MAX_MSG_LEN(netif));

+#if LWIP_NETIF_HOSTNAME


Suggest to move this change (sending hostname in discover) to a separate commit

yarrick · 2022-03-03T21:07:48Z

src/core/ipv4/dhcp.c

@@ -1340,6 +1371,10 @@ dhcp_release_and_stop(struct netif *netif)
      options_out_len = dhcp_option(options_out_len, msg_out->options, DHCP_OPTION_SERVER_ID, 4);
      options_out_len = dhcp_option_long(options_out_len, msg_out->options, lwip_ntohl(ip4_addr_get_u32(ip_2_ip4(&server_ip_addr))));

+#if LWIP_DHCP_OPTION_CLIENT_IDENTIFIER
+    options_out_len = dhcp_option_client_identifier(options_out_len, msg_out->options, netif);


Indent looks wrong

yarrick · 2022-03-03T21:07:56Z

src/core/ipv4/dhcp.c

+dhcp_option_client_identifier(u16_t options_out_len, u8_t *options, struct netif *netif)
+{
+  size_t i;
+  options_out_len = dhcp_option(options_out_len, options, DHCP_OPTION_CLIENT_ID, DHCP_OPTION_CLIENT_ID_MAC_LEN);


Since dynamic length is used later (netif->hwaddr_len) it seems wrong to have a constant here

yarrick · 2022-03-03T21:08:00Z

src/core/ipv4/dhcp.c

+{
+  size_t i;
+  options_out_len = dhcp_option(options_out_len, options, DHCP_OPTION_CLIENT_ID, DHCP_OPTION_CLIENT_ID_MAC_LEN);
+  options_out_len = dhcp_option_byte(options_out_len, options, DHCP_OPTION_CLIENT_ID_MAC);


We already have LWIP_IANA_HWTYPE_ETHERNET defined

yarrick · 2022-03-03T21:08:09Z

src/include/lwip/opt.h

+ * LWIP_DHCP_OPTION_CLIENT_IDENTIFIER > 0: Send client identifier option with dhcp requests
+ */
+#if !defined LWIP_DHCP_OPTION_CLIENT_IDENTIFIER || defined __DOXYGEN__
+#define LWIP_DHCP_OPTION_CLIENT_IDENTIFIER      0


Maybe call it LWIP_DHCP_SEND_CLIENT_IDENTIFIER or similar, the other flags controlling options dont include option in their name

yarrick · 2022-03-03T21:08:17Z

src/include/lwip/prot/dhcp.h

@@ -160,7 +160,14 @@ typedef enum {
 #define DHCP_OPTION_T1              58 /* T1 renewal time */
 #define DHCP_OPTION_T2              59 /* T2 rebinding time */
 #define DHCP_OPTION_US              60
-#define DHCP_OPTION_CLIENT_ID       61
+
+#if LWIP_DHCP_OPTION_CLIENT_IDENTIFIER


No need for #if here with the two new constants gone

yarrick · 2022-03-03T21:08:21Z

src/include/lwip/prot/dhcp.h

@@ -171,6 +178,7 @@ typedef enum {
 #define DHCP_OVERLOAD_SNAME_FILE    3


+


vulptex · 2022-03-28T06:50:36Z

Lets close this for now.I need more free-time todo it properly.

Reproducer (in bash): base64 -d <<< "H4sIAP/9L2QCA+3WoQ2AMBSE4QoCTFHBBJfgSRF4RDfpRmgmYBpGQRBCk4ZiSfk/+fJMK+5dZRVpzSQzSs6oPierDV4y87WxLQLwE42SfNCdDyHJB9/xZwAARPbMJbUq4JJmu4JVT1cAAACfbGIqoqcMzy90eu+aBw2+N28WFgAA" | gunzip | test/fuzz/lwip_fuzz2 Crash log: ../../src/core/altcp_tcp.c:178:13: runtime error: member access within null pointer of type 'struct tcp_pcb' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../src/core/altcp_tcp.c:178:13 in AddressSanitizer:DEADLYSIGNAL ================================================================= ==192415==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000048 (pc 0x557065081703 bp 0x0aae0cb71204 sp 0x7ffd034dabc0 T0) ==192415==The signal is caused by a READ memory access. ==192415==Hint: address points to the zero page. #0 0x557065081703 in altcp_tcp_setup_callbacks /.../lwip/test/fuzz/../../src/core/altcp_tcp.c:178:19 #1 0x55706508206f in altcp_tcp_setup /.../lwip/test/fuzz/../../src/core/altcp_tcp.c:189:3 lwip-tcpip#2 0x55706508206f in altcp_tcp_accept /.../lwip/test/fuzz/../../src/core/altcp_tcp.c:84:5 lwip-tcpip#3 0x557065095592 in tcp_input /.../lwip/test/fuzz/../../src/core/tcp_in.c:380:9 lwip-tcpip#4 0x5570650e752f in ip4_input /.../lwip/test/fuzz/../../src/core/ipv4/ip4.c:743:9 lwip-tcpip#5 0x55706513d4de in ethernet_input /.../lwip/test/fuzz/../../src/netif/ethernet.c:186:9 lwip-tcpip#6 0x557064fe0959 in input_pkt /.../lwip/test/fuzz/fuzz_common.c:209:9 lwip-tcpip#7 0x557064fdeb6a in input_pkts /.../lwip/test/fuzz/fuzz_common.c:257:9 lwip-tcpip#8 0x557064fdeb6a in lwip_fuzztest /.../lwip/test/fuzz/fuzz_common.c:669:3 lwip-tcpip#9 0x7ff4f578e189 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 lwip-tcpip#10 0x7ff4f578e244 in __libc_start_main csu/../csu/libc-start.c:381:3 lwip-tcpip#11 0x557064f20420 in _start (/.../lwip/test/fuzz/lwip_fuzz2+0x81420) (BuildId: 8680a96430d5749c90111fe9c3a3d4f881a5dbcd) AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV /.../lwip/test/fuzz/../../src/core/altcp_tcp.c:178:19 in altcp_tcp_setup_callbacks ==192415==ABORTING Aborted

Reproducer (in bash): base64 -d <<< "H4sIAP/9L2QCA+3WoQ2AMBSE4QoCTFHBBJfgSRF4RDfpRmgmYBpGQRBCk4ZiSfk/+fJMK+5dZRVpzSQzSs6oPierDV4y87WxLQLwE42SfNCdDyHJB9/xZwAARPbMJbUq4JJmu4JVT1cAAACfbGIqoqcMzy90eu+aBw2+N28WFgAA" | gunzip | test/fuzz/lwip_fuzz2 Crash log: ../../src/core/altcp_tcp.c:178:13: runtime error: member access within null pointer of type 'struct tcp_pcb' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../src/core/altcp_tcp.c:178:13 in AddressSanitizer:DEADLYSIGNAL ================================================================= ==192415==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000048 (pc 0x557065081703 bp 0x0aae0cb71204 sp 0x7ffd034dabc0 T0) ==192415==The signal is caused by a READ memory access. ==192415==Hint: address points to the zero page. #0 0x557065081703 in altcp_tcp_setup_callbacks /.../lwip/test/fuzz/../../src/core/altcp_tcp.c:178:19 #1 0x55706508206f in altcp_tcp_setup /.../lwip/test/fuzz/../../src/core/altcp_tcp.c:189:3 #2 0x55706508206f in altcp_tcp_accept /.../lwip/test/fuzz/../../src/core/altcp_tcp.c:84:5 #3 0x557065095592 in tcp_input /.../lwip/test/fuzz/../../src/core/tcp_in.c:380:9 #4 0x5570650e752f in ip4_input /.../lwip/test/fuzz/../../src/core/ipv4/ip4.c:743:9 #5 0x55706513d4de in ethernet_input /.../lwip/test/fuzz/../../src/netif/ethernet.c:186:9 #6 0x557064fe0959 in input_pkt /.../lwip/test/fuzz/fuzz_common.c:209:9 #7 0x557064fdeb6a in input_pkts /.../lwip/test/fuzz/fuzz_common.c:257:9 #8 0x557064fdeb6a in lwip_fuzztest /.../lwip/test/fuzz/fuzz_common.c:669:3 #9 0x7ff4f578e189 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 #10 0x7ff4f578e244 in __libc_start_main csu/../csu/libc-start.c:381:3 #11 0x557064f20420 in _start (/.../lwip/test/fuzz/lwip_fuzz2+0x81420) (BuildId: 8680a96430d5749c90111fe9c3a3d4f881a5dbcd) AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV /.../lwip/test/fuzz/../../src/core/altcp_tcp.c:178:19 in altcp_tcp_setup_callbacks ==192415==ABORTING Aborted

feat(dhcp): client identifier option

2cffd30

vulptex mentioned this pull request Feb 21, 2022

Upstream changes/patches please (IDFGH-6818) espressif/esp-lwip#41

Closed

yarrick requested changes Mar 3, 2022

View reviewed changes

vulptex closed this Mar 28, 2022

david-cermak mentioned this pull request Apr 5, 2022

contrib/addons: Add example of using DHCP extra option hooks #12

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(dhcp): client identifier option #10

feat(dhcp): client identifier option #10

vulptex commented Feb 21, 2022 •

edited

Loading

vulptex commented Feb 21, 2022

yarrick left a comment

yarrick Mar 3, 2022

yarrick Mar 3, 2022

yarrick Mar 3, 2022

yarrick Mar 3, 2022

yarrick Mar 3, 2022

yarrick Mar 3, 2022

yarrick Mar 3, 2022

vulptex commented Mar 28, 2022

		@@ -171,6 +178,7 @@ typedef enum {
		#define DHCP_OVERLOAD_SNAME_FILE 3

feat(dhcp): client identifier option #10

feat(dhcp): client identifier option #10

Conversation

vulptex commented Feb 21, 2022 • edited Loading

vulptex commented Feb 21, 2022

yarrick left a comment

Choose a reason for hiding this comment

yarrick Mar 3, 2022

Choose a reason for hiding this comment

yarrick Mar 3, 2022

Choose a reason for hiding this comment

yarrick Mar 3, 2022

Choose a reason for hiding this comment

yarrick Mar 3, 2022

Choose a reason for hiding this comment

yarrick Mar 3, 2022

Choose a reason for hiding this comment

yarrick Mar 3, 2022

Choose a reason for hiding this comment

yarrick Mar 3, 2022

Choose a reason for hiding this comment

vulptex commented Mar 28, 2022

vulptex commented Feb 21, 2022 •

edited

Loading