From 1cbf9d22a5f63f2c9c1d31221c5db63eaa6898a2 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 17 Jan 2020 08:30:24 +0100 Subject: [PATCH] n-dhcp4: accept options that are longer than requested If the server sends a packet with multiple instances of the same option, they are concatenated during n_dhcp4_incoming_linearize() and evaluated as a single option as per section 7 of RFC 3396. However, there are broken server implementations that send self-contained options in multiple copies. They are reassembled to form a single instance by the nettools client, which then fails to parse them because they have a length greater than the expected one. This problem can be reproduced by starting a server with: dnsmasq --bind-interfaces --interface veth1 -d --dhcp-range=172.25.1.100,172.25.1.200,1m --dhcp-option=54,172.25.1.1 In this way dnsmasq sends a duplicate option 54 (server-id) when the client requests it in the 'parameter request list' option, as dhcp=systemd and dhcp=nettools currently do. While this is a violation of the RFC by the server, both isc-dhcp and systemd-networkd client implementations have mechanisms to deal with this situation. dhclient simply takes the first bytes of the aggregated option. systemd-networkd doesn't follow RFC 3396 and doesn't aggregate multiple options; it considers only the last occurrence of each option. Change the parsing code to accept options that are longer than necessary. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/324 --- shared/n-dhcp4/src/n-dhcp4-incoming.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/shared/n-dhcp4/src/n-dhcp4-incoming.c b/shared/n-dhcp4/src/n-dhcp4-incoming.c index e7234c0a52..f739413b5c 100644 --- a/shared/n-dhcp4/src/n-dhcp4-incoming.c +++ b/shared/n-dhcp4/src/n-dhcp4-incoming.c @@ -326,7 +326,7 @@ static int n_dhcp4_incoming_query_u8(NDhcp4Incoming *message, uint8_t option, ui r = n_dhcp4_incoming_query(message, option, &data, &n_data); if (r) return r; - else if (n_data != sizeof(*data)) + else if (n_data < sizeof(*data)) return N_DHCP4_E_MALFORMED; *u8p = *data; @@ -342,7 +342,7 @@ static int n_dhcp4_incoming_query_u16(NDhcp4Incoming *message, uint8_t option, u r = n_dhcp4_incoming_query(message, option, &data, &n_data); if (r) return r; - else if (n_data != sizeof(be16)) + else if (n_data < sizeof(be16)) return N_DHCP4_E_MALFORMED; memcpy(&be16, data, sizeof(be16)); @@ -360,7 +360,7 @@ static int n_dhcp4_incoming_query_u32(NDhcp4Incoming *message, uint8_t option, u r = n_dhcp4_incoming_query(message, option, &data, &n_data); if (r) return r; - else if (n_data != sizeof(be32)) + else if (n_data < sizeof(be32)) return N_DHCP4_E_MALFORMED; memcpy(&be32, data, sizeof(be32)); @@ -378,7 +378,7 @@ static int n_dhcp4_incoming_query_in_addr(NDhcp4Incoming *message, uint8_t optio r = n_dhcp4_incoming_query(message, option, &data, &n_data); if (r) return r; - else if (n_data != sizeof(be32)) + else if (n_data < sizeof(be32)) return N_DHCP4_E_MALFORMED; memcpy(&be32, data, sizeof(be32));