From 1cbf9d22a5f63f2c9c1d31221c5db63eaa6898a2 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 17 Jan 2020 08:30:24 +0100 Subject: [PATCH 1/2] 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 e7234c0a5..f739413b5 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)); From 541db78259063b1bef10071449ebddfbac1fe519 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 17 Jan 2020 10:29:49 +0100 Subject: [PATCH 2/2] dhcp: don't add server-id option to the parameter request list The option is mandatory in the replies from server and so we don't need to ask for it. dhclient doesn't do it either. But especially, it seems that requesting the option causes some broken server implementations to send duplicate instances of the option. So, remove the option from the parameter request list of the internal nettools and systemd DHCP implementation. --- src/dhcp/nm-dhcp-options.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dhcp/nm-dhcp-options.c b/src/dhcp/nm-dhcp-options.c index 4c003f315..1d391f3e9 100644 --- a/src/dhcp/nm-dhcp-options.c +++ b/src/dhcp/nm-dhcp-options.c @@ -34,7 +34,7 @@ const NMDhcpOption _nm_dhcp_option_dhcp4_options[] = { REQ (NM_DHCP_OPTION_DHCP4_NIS_DOMAIN, "nis_domain", TRUE ), REQ (NM_DHCP_OPTION_DHCP4_NIS_SERVERS, "nis_servers", TRUE ), REQ (NM_DHCP_OPTION_DHCP4_NTP_SERVER, "ntp_servers", TRUE ), - REQ (NM_DHCP_OPTION_DHCP4_SERVER_ID, "dhcp_server_identifier", TRUE ), + REQ (NM_DHCP_OPTION_DHCP4_SERVER_ID, "dhcp_server_identifier", FALSE ), REQ (NM_DHCP_OPTION_DHCP4_DOMAIN_SEARCH_LIST, "domain_search", TRUE ), REQ (NM_DHCP_OPTION_DHCP4_PRIVATE_CLASSLESS_STATIC_ROUTE, "ms_classless_static_routes", TRUE ), REQ (NM_DHCP_OPTION_DHCP4_PRIVATE_PROXY_AUTODISCOVERY, "wpad", TRUE ),