From edc7c7204c4bda8b1c3e3b18157ef1c31e9dd5fe Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 08:46:58 +0100 Subject: [PATCH 01/34] shared: remove "str" argument from nm_str_buf_reset() NMStrBuf's API is all about convenience. When you reset the buffer, is it convenient to immediately append a new string? It seems not. Make nm_str_buf_reset() simpler by doing only one thing. --- libnm-core/nm-utils.c | 3 ++- shared/nm-glib-aux/nm-str-buf.h | 7 +++---- src/core/ndisc/nm-lndp-ndisc.c | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index e19d5ed5fc..6ceef1e402 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -3623,7 +3623,8 @@ nm_utils_file_search_in_paths(const char * progname, if (!path[0]) continue; - nm_str_buf_reset(&strbuf, path); + nm_str_buf_reset(&strbuf); + nm_str_buf_append(&strbuf, path); nm_str_buf_ensure_trailing_c(&strbuf, '/'); s = nm_str_buf_append0(&strbuf, progname); diff --git a/shared/nm-glib-aux/nm-str-buf.h b/shared/nm-glib-aux/nm-str-buf.h index 6edd451ee5..32cfd4105d 100644 --- a/shared/nm-glib-aux/nm-str-buf.h +++ b/shared/nm-glib-aux/nm-str-buf.h @@ -292,8 +292,10 @@ nm_str_buf_append_c_len(NMStrBuf *strbuf, char ch, gsize len) } } +/*****************************************************************************/ + static inline void -nm_str_buf_reset(NMStrBuf *strbuf, const char *str) +nm_str_buf_reset(NMStrBuf *strbuf) { _nm_str_buf_assert(strbuf); @@ -304,9 +306,6 @@ nm_str_buf_reset(NMStrBuf *strbuf, const char *str) } strbuf->_priv_len = 0; } - - if (str) - nm_str_buf_append(strbuf, str); } /*****************************************************************************/ diff --git a/src/core/ndisc/nm-lndp-ndisc.c b/src/core/ndisc/nm-lndp-ndisc.c index d846411e0b..f773478f51 100644 --- a/src/core/ndisc/nm-lndp-ndisc.c +++ b/src/core/ndisc/nm-lndp-ndisc.c @@ -474,7 +474,7 @@ dns_servers_done: gsize padding; gsize len; - nm_str_buf_reset(&sbuf, NULL); + nm_str_buf_reset(&sbuf); for (i = 0; i < rdata->dns_domains->len; i++) { const NMNDiscDNSDomain *dns_domain = From e12e4ef84960a7d093b9c276069a052b6862aa6f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 09:55:40 +0100 Subject: [PATCH 02/34] shared: return NMStrBuf instance from nm_str_buf_reset() it can be useful for chaining, and it costs nothing to do this on an inline function. --- shared/nm-glib-aux/nm-str-buf.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/shared/nm-glib-aux/nm-str-buf.h b/shared/nm-glib-aux/nm-str-buf.h index 32cfd4105d..cb0d3fb189 100644 --- a/shared/nm-glib-aux/nm-str-buf.h +++ b/shared/nm-glib-aux/nm-str-buf.h @@ -294,7 +294,7 @@ nm_str_buf_append_c_len(NMStrBuf *strbuf, char ch, gsize len) /*****************************************************************************/ -static inline void +static inline NMStrBuf * nm_str_buf_reset(NMStrBuf *strbuf) { _nm_str_buf_assert(strbuf); @@ -306,6 +306,8 @@ nm_str_buf_reset(NMStrBuf *strbuf) } strbuf->_priv_len = 0; } + + return strbuf; } /*****************************************************************************/ From 1e15ea9dd528de2a73747a70cbb60f3ffe5f1dd3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 10:09:56 +0100 Subject: [PATCH 03/34] systemd: add nm_sd_dns_name_normalize() accessor --- shared/systemd/nm-sd-utils-shared.c | 18 ++++++++++++++++++ shared/systemd/nm-sd-utils-shared.h | 2 ++ 2 files changed, 20 insertions(+) diff --git a/shared/systemd/nm-sd-utils-shared.c b/shared/systemd/nm-sd-utils-shared.c index c93e89c630..f0504aa937 100644 --- a/shared/systemd/nm-sd-utils-shared.c +++ b/shared/systemd/nm-sd-utils-shared.c @@ -93,6 +93,24 @@ nm_sd_hostname_is_valid(const char *s, bool allow_trailing_dot) : (ValidHostnameFlags) 0); } +char * +nm_sd_dns_name_normalize(const char *s) +{ + nm_auto_free char *n = NULL; + int r; + + r = dns_name_normalize(s, 0, &n); + if (r < 0) + return NULL; + + nm_assert(n); + + /* usually we try not to mix malloc/g_malloc and free/g_free. In practice, + * they are the same. So here we return a buffer allocated with malloc(), + * and the caller should free it with g_free(). */ + return g_steal_pointer(&n); +} + /*****************************************************************************/ static gboolean diff --git a/shared/systemd/nm-sd-utils-shared.h b/shared/systemd/nm-sd-utils-shared.h index b4a1b189d3..45089c074d 100644 --- a/shared/systemd/nm-sd-utils-shared.h +++ b/shared/systemd/nm-sd-utils-shared.h @@ -28,6 +28,8 @@ nm_sd_dns_name_to_wire_format(const char *domain, guint8 *buffer, size_t len, gb int nm_sd_dns_name_is_valid(const char *s); gboolean nm_sd_hostname_is_valid(const char *s, bool allow_trailing_dot); +char *nm_sd_dns_name_normalize(const char *s); + /*****************************************************************************/ gboolean nm_sd_http_url_is_valid_https(const char *url); From f0a926871840a4ae470abed52a8ef2caff8b1092 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 09:26:41 +0100 Subject: [PATCH 04/34] dhcp: require options argument for nm_dhcp_option_add_option() It's not clear why the option argument would be optional. Also, it's not optional for nm_dhcp_option_take_option(). Add an nm_assert() to catch such wrong uses. --- src/core/dhcp/nm-dhcp-options.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-options.c b/src/core/dhcp/nm-dhcp-options.c index 09dd6f363f..c10077a431 100644 --- a/src/core/dhcp/nm-dhcp-options.c +++ b/src/core/dhcp/nm-dhcp-options.c @@ -219,7 +219,12 @@ nm_dhcp_option_take_option(GHashTable * options, guint option, char * value) { - nm_assert(options); + if (!options) { + nm_assert_not_reached(); + g_free(value); + return; + } + nm_assert(requests); nm_assert(value); nm_assert(g_utf8_validate(value, -1, NULL)); @@ -233,8 +238,7 @@ nm_dhcp_option_add_option(GHashTable * options, guint option, const char * value) { - if (options) - nm_dhcp_option_take_option(options, requests, option, g_strdup(value)); + nm_dhcp_option_take_option(options, requests, option, g_strdup(value)); } void @@ -243,11 +247,10 @@ nm_dhcp_option_add_option_u64(GHashTable * options, guint option, guint64 value) { - if (options) - nm_dhcp_option_take_option(options, - requests, - option, - g_strdup_printf("%" G_GUINT64_FORMAT, value)); + nm_dhcp_option_take_option(options, + requests, + option, + g_strdup_printf("%" G_GUINT64_FORMAT, value)); } void @@ -255,8 +258,10 @@ nm_dhcp_option_add_requests_to_options(GHashTable *options, const NMDhcpOption * { guint i; - if (!options) + if (!options) { + nm_assert_not_reached(); return; + } for (i = 0; requests[i].name; i++) { if (requests[i].include) From 41634d51993d04c7fc7c56c624f1e781ac4faa2d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 15:14:59 +0100 Subject: [PATCH 05/34] dhcp: add nm_dhcp_option_add_option_utf8safe_escape() helper --- src/core/dhcp/nm-dhcp-options.c | 14 ++++++++++++++ src/core/dhcp/nm-dhcp-options.h | 5 +++++ 2 files changed, 19 insertions(+) diff --git a/src/core/dhcp/nm-dhcp-options.c b/src/core/dhcp/nm-dhcp-options.c index c10077a431..c3ef8478e8 100644 --- a/src/core/dhcp/nm-dhcp-options.c +++ b/src/core/dhcp/nm-dhcp-options.c @@ -241,6 +241,20 @@ nm_dhcp_option_add_option(GHashTable * options, nm_dhcp_option_take_option(options, requests, option, g_strdup(value)); } +void +nm_dhcp_option_add_option_utf8safe_escape(GHashTable * options, + const NMDhcpOption *requests, + guint option, + const guint8 * data, + gsize n_data) +{ + gs_free char *to_free = NULL; + const char * escaped; + + escaped = nm_utils_buf_utf8safe_escape((char *) data, n_data, 0, &to_free); + nm_dhcp_option_add_option(options, requests, option, escaped ?: ""); +} + void nm_dhcp_option_add_option_u64(GHashTable * options, const NMDhcpOption *requests, diff --git a/src/core/dhcp/nm-dhcp-options.h b/src/core/dhcp/nm-dhcp-options.h index c2a403a4b5..9e6e4d1cf1 100644 --- a/src/core/dhcp/nm-dhcp-options.h +++ b/src/core/dhcp/nm-dhcp-options.h @@ -195,6 +195,11 @@ void nm_dhcp_option_add_option(GHashTable * options, const NMDhcpOption *requests, guint option, const char * value); +void nm_dhcp_option_add_option_utf8safe_escape(GHashTable * options, + const NMDhcpOption *requests, + guint option, + const guint8 * data, + gsize n_data); void nm_dhcp_option_add_option_u64(GHashTable * options, const NMDhcpOption *requests, guint option, From fc83acbd99f9d2558176b1546ba560696fd13974 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 15:14:10 +0100 Subject: [PATCH 06/34] dhcp: add nm_dhcp_option_add_option_in_addr() helper --- src/core/dhcp/nm-dhcp-options.c | 11 +++++++++++ src/core/dhcp/nm-dhcp-options.h | 4 ++++ 2 files changed, 15 insertions(+) diff --git a/src/core/dhcp/nm-dhcp-options.c b/src/core/dhcp/nm-dhcp-options.c index c3ef8478e8..e03fa23192 100644 --- a/src/core/dhcp/nm-dhcp-options.c +++ b/src/core/dhcp/nm-dhcp-options.c @@ -267,6 +267,17 @@ nm_dhcp_option_add_option_u64(GHashTable * options, g_strdup_printf("%" G_GUINT64_FORMAT, value)); } +void +nm_dhcp_option_add_option_in_addr(GHashTable * options, + const NMDhcpOption *requests, + guint option, + in_addr_t value) +{ + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; + + nm_dhcp_option_add_option(options, requests, option, _nm_utils_inet4_ntop(value, sbuf)); +} + void nm_dhcp_option_add_requests_to_options(GHashTable *options, const NMDhcpOption *requests) { diff --git a/src/core/dhcp/nm-dhcp-options.h b/src/core/dhcp/nm-dhcp-options.h index 9e6e4d1cf1..2ebf339ed7 100644 --- a/src/core/dhcp/nm-dhcp-options.h +++ b/src/core/dhcp/nm-dhcp-options.h @@ -200,6 +200,10 @@ void nm_dhcp_option_add_option_utf8safe_escape(GHashTable * option guint option, const guint8 * data, gsize n_data); +void nm_dhcp_option_add_option_in_addr(GHashTable * options, + const NMDhcpOption *requests, + guint option, + in_addr_t value); void nm_dhcp_option_add_option_u64(GHashTable * options, const NMDhcpOption *requests, guint option, From de14a376ffbd08b00732ef491cb7cc15b6ab3416 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 9 Feb 2021 18:10:12 +0100 Subject: [PATCH 07/34] dhcp/nettools: refactor parsing of DHCP lease (mtu) --- src/core/dhcp/nm-dhcp-nettools.c | 54 +++++++++----------------------- src/core/dhcp/nm-dhcp-utils.c | 32 +++++++++++++++++++ src/core/dhcp/nm-dhcp-utils.h | 5 +++ 3 files changed, 51 insertions(+), 40 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index ae1aeadfa4..14b788e38d 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -326,27 +326,6 @@ lease_get_in_addr(NDhcp4ClientLease *lease, guint8 option, struct in_addr *addrp return TRUE; } -static gboolean -lease_get_u16(NDhcp4ClientLease *lease, uint8_t option, uint16_t *u16p) -{ - uint8_t *data; - size_t n_data; - uint16_t be16; - int r; - - r = n_dhcp4_client_lease_query(lease, option, &data, &n_data); - if (r) - return FALSE; - - if (n_data != sizeof(be16)) - return FALSE; - - memcpy(&be16, data, sizeof(be16)); - - *u16p = ntohs(be16); - return TRUE; -} - static gboolean lease_parse_address(NDhcp4ClientLease *lease, NMIP4Config * ip4_config, @@ -710,24 +689,6 @@ lease_parse_routes(NDhcp4ClientLease *lease, } } -static void -lease_parse_mtu(NDhcp4ClientLease *lease, NMIP4Config *ip4_config, GHashTable *options) -{ - uint16_t mtu; - - if (!lease_get_u16(lease, NM_DHCP_OPTION_DHCP4_INTERFACE_MTU, &mtu)) - return; - - if (mtu < 68) - return; - - nm_dhcp_option_add_option_u64(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_INTERFACE_MTU, - mtu); - nm_ip4_config_set_mtu(ip4_config, mtu, NM_IP_CONFIG_SOURCE_DHCP); -} - static void lease_parse_metered(NDhcp4ClientLease *lease, NMIP4Config *ip4_config, GHashTable *options) { @@ -997,6 +958,10 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, { gs_unref_object NMIP4Config *ip4_config = NULL; gs_unref_hashtable GHashTable *options = NULL; + guint8 * l_data; + gsize l_data_len; + guint16 v_u16; + int r; g_return_val_if_fail(lease != NULL, NULL); @@ -1012,7 +977,16 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, lease_parse_address_list(lease, ip4_config, NM_DHCP_OPTION_DHCP4_DOMAIN_NAME_SERVER, options); lease_parse_domainname(lease, ip4_config, options); lease_parse_search_domains(lease, ip4_config, options); - lease_parse_mtu(lease, ip4_config, options); + + r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_INTERFACE_MTU, &l_data, &l_data_len); + if (r == 0 && nm_dhcp_lease_data_parse_mtu(l_data, l_data_len, &v_u16)) { + nm_dhcp_option_add_option_u64(options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_INTERFACE_MTU, + v_u16); + nm_ip4_config_set_mtu(ip4_config, v_u16, NM_IP_CONFIG_SOURCE_DHCP); + } + lease_parse_metered(lease, ip4_config, options); lease_parse_hostname(lease, options); diff --git a/src/core/dhcp/nm-dhcp-utils.c b/src/core/dhcp/nm-dhcp-utils.c index a6555bce24..bf8d5887e4 100644 --- a/src/core/dhcp/nm-dhcp-utils.c +++ b/src/core/dhcp/nm-dhcp-utils.c @@ -9,6 +9,7 @@ #include #include "nm-glib-aux/nm-dedup-multi.h" +#include "nm-std-aux/unaligned.h" #include "nm-dhcp-utils.h" #include "nm-utils.h" @@ -834,3 +835,34 @@ nm_dhcp_utils_get_dhcp6_event_id(GHashTable *lease) return g_strdup_printf("%s|%s", iaid, start); } + +/*****************************************************************************/ + +gboolean +nm_dhcp_lease_data_parse_u16(const guint8 *data, gsize n_data, uint16_t *out_val) +{ + if (n_data != 2) + return FALSE; + + *out_val = unaligned_read_be16(data); + return TRUE; +} + +gboolean +nm_dhcp_lease_data_parse_mtu(const guint8 *data, gsize n_data, uint16_t *out_val) +{ + uint16_t mtu; + + if (!nm_dhcp_lease_data_parse_u16(data, n_data, &mtu)) + return FALSE; + + if (mtu < 68) { + /* https://tools.ietf.org/html/rfc2132#section-5.1: + * + * The minimum legal value for the MTU is 68. */ + return FALSE; + } + + *out_val = mtu; + return TRUE; +} diff --git a/src/core/dhcp/nm-dhcp-utils.h b/src/core/dhcp/nm-dhcp-utils.h index f5bc53a215..52e1ea4fb1 100644 --- a/src/core/dhcp/nm-dhcp-utils.h +++ b/src/core/dhcp/nm-dhcp-utils.h @@ -40,4 +40,9 @@ char **nm_dhcp_parse_search_list(guint8 *data, size_t n_data); char *nm_dhcp_utils_get_dhcp6_event_id(GHashTable *lease); +/*****************************************************************************/ + +gboolean nm_dhcp_lease_data_parse_u16(const guint8 *data, gsize n_data, guint16 *out_val); +gboolean nm_dhcp_lease_data_parse_mtu(const guint8 *data, gsize n_data, guint16 *out_val); + #endif /* __NETWORKMANAGER_DHCP_UTILS_H__ */ From 89773b87393112c16f02f0d47e3f848354ed3e4f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 9 Feb 2021 21:12:35 +0100 Subject: [PATCH 08/34] dhcp/nettools: refactor parsing of DHCP lease (metered) --- src/core/dhcp/nm-dhcp-nettools.c | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 14b788e38d..6a8155f336 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -689,24 +689,6 @@ lease_parse_routes(NDhcp4ClientLease *lease, } } -static void -lease_parse_metered(NDhcp4ClientLease *lease, NMIP4Config *ip4_config, GHashTable *options) -{ - gboolean metered = FALSE; - uint8_t *data; - size_t n_data; - int r; - - r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_VENDOR_SPECIFIC, &data, &n_data); - if (r) - metered = FALSE; - else - metered = !!memmem(data, n_data, "ANDROID_METERED", NM_STRLEN("ANDROID_METERED")); - - /* TODO: expose the vendor specific option when present */ - nm_ip4_config_set_metered(ip4_config, metered); -} - static void lease_parse_ntps(NDhcp4ClientLease *lease, GHashTable *options) { @@ -961,6 +943,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, guint8 * l_data; gsize l_data_len; guint16 v_u16; + gboolean v_bool; int r; g_return_val_if_fail(lease != NULL, NULL); @@ -987,7 +970,13 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, nm_ip4_config_set_mtu(ip4_config, v_u16, NM_IP_CONFIG_SOURCE_DHCP); } - lease_parse_metered(lease, ip4_config, options); + r = n_dhcp4_client_lease_query(lease, + NM_DHCP_OPTION_DHCP4_VENDOR_SPECIFIC, + &l_data, + &l_data_len); + v_bool = + (r == 0) && memmem(l_data, l_data_len, "ANDROID_METERED", NM_STRLEN("ANDROID_METERED")); + nm_ip4_config_set_metered(ip4_config, v_bool); lease_parse_hostname(lease, options); lease_parse_ntps(lease, options); From 67110d171165e6d7adfc83d55fd28aed855af0d7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 08:44:27 +0100 Subject: [PATCH 09/34] dhcp/nettools: refactor parsing of DHCP lease (hostname) --- src/core/dhcp/nm-dhcp-nettools.c | 40 +++++++++++++------------------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 6a8155f336..aee092ef10 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -15,6 +15,7 @@ #include "nm-glib-aux/nm-dedup-multi.h" #include "nm-std-aux/unaligned.h" +#include "nm-glib-aux/nm-str-buf.h" #include "nm-utils.h" #include "nm-config.h" @@ -716,29 +717,6 @@ lease_parse_ntps(NDhcp4ClientLease *lease, GHashTable *options) str->str); } -static void -lease_parse_hostname(NDhcp4ClientLease *lease, GHashTable *options) -{ - nm_auto_free_gstring GString *str = NULL; - uint8_t * data; - size_t n_data; - int r; - - r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_HOST_NAME, &data, &n_data); - if (r) - return; - - str = g_string_new_len((char *) data, n_data); - - if (nm_utils_is_localhost(str->str)) - return; - - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_HOST_NAME, - str->str); -} - static void lease_parse_domainname(NDhcp4ClientLease *lease, NMIP4Config *ip4_config, GHashTable *options) { @@ -938,10 +916,12 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, GHashTable ** out_options, GError ** error) { + nm_auto_str_buf NMStrBuf sbuf = NM_STR_BUF_INIT(0, FALSE); gs_unref_object NMIP4Config *ip4_config = NULL; gs_unref_hashtable GHashTable *options = NULL; guint8 * l_data; gsize l_data_len; + const char * v_str; guint16 v_u16; gboolean v_bool; int r; @@ -978,7 +958,19 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, (r == 0) && memmem(l_data, l_data_len, "ANDROID_METERED", NM_STRLEN("ANDROID_METERED")); nm_ip4_config_set_metered(ip4_config, v_bool); - lease_parse_hostname(lease, options); + r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_HOST_NAME, &l_data, &l_data_len); + if (r == 0) { + nm_str_buf_reset(&sbuf); + nm_str_buf_append_len(&sbuf, (const char *) l_data, l_data_len); + v_str = nm_str_buf_get_str(&sbuf); + if (!nm_utils_is_localhost(v_str)) { + nm_dhcp_option_add_option(options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_HOST_NAME, + v_str); + } + } + lease_parse_ntps(lease, options); lease_parse_root_path(lease, options); lease_parse_wpad(lease, options); From 784932550ce1824505ff9893585ea8192939200a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 09:56:08 +0100 Subject: [PATCH 10/34] dhcp/nettools: validate and normalize Host Name Option (12) The hostname is in the end a string, which means it must be in a known, sensible encoding (UTF-8). Previously, we would not ensure the encoding, nor that the hostname was valid. Fix that. Follow what systemd does with lease_parse_domain(). See-also: https://tools.ietf.org/html/rfc2132#section-3.14 --- src/core/dhcp/nm-dhcp-nettools.c | 10 ++-- src/core/dhcp/nm-dhcp-utils.c | 86 +++++++++++++++++++++++++++++++- src/core/dhcp/nm-dhcp-utils.h | 4 ++ 3 files changed, 93 insertions(+), 7 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index aee092ef10..3bcf5a7ca6 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -921,7 +921,6 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, gs_unref_hashtable GHashTable *options = NULL; guint8 * l_data; gsize l_data_len; - const char * v_str; guint16 v_u16; gboolean v_bool; int r; @@ -960,14 +959,13 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_HOST_NAME, &l_data, &l_data_len); if (r == 0) { - nm_str_buf_reset(&sbuf); - nm_str_buf_append_len(&sbuf, (const char *) l_data, l_data_len); - v_str = nm_str_buf_get_str(&sbuf); - if (!nm_utils_is_localhost(v_str)) { + gs_free char *s = NULL; + + if (nm_dhcp_lease_data_parse_domain(l_data, l_data_len, &s)) { nm_dhcp_option_add_option(options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_HOST_NAME, - v_str); + s); } } diff --git a/src/core/dhcp/nm-dhcp-utils.c b/src/core/dhcp/nm-dhcp-utils.c index bf8d5887e4..508c3d0930 100644 --- a/src/core/dhcp/nm-dhcp-utils.c +++ b/src/core/dhcp/nm-dhcp-utils.c @@ -8,8 +8,9 @@ #include #include -#include "nm-glib-aux/nm-dedup-multi.h" #include "nm-std-aux/unaligned.h" +#include "nm-glib-aux/nm-dedup-multi.h" +#include "systemd/nm-sd-utils-shared.h" #include "nm-dhcp-utils.h" #include "nm-utils.h" @@ -866,3 +867,86 @@ nm_dhcp_lease_data_parse_mtu(const guint8 *data, gsize n_data, uint16_t *out_val *out_val = mtu; return TRUE; } + +gboolean +nm_dhcp_lease_data_parse_cstr(const guint8 *data, gsize n_data, gsize *out_new_len) +{ + /* WARNING: this function only validates that the string does not contain + * NUL characters (and ignores one trailing NUL). It does not check character + * encoding! */ + + if (n_data > 0) { + if (memchr(data, n_data - 1, '\0')) { + /* we accept one trailing NUL (not more). + * + * https://tools.ietf.org/html/rfc2132#section-2 + * https://github.com/systemd/systemd/issues/1337 */ + return FALSE; + } + + if (data[n_data - 1] == '\0') + n_data--; + } + + NM_SET_OUT(out_new_len, n_data); + return TRUE; +} + +char * +nm_dhcp_lease_data_parse_domain_validate(const char *str) +{ + gs_free char *s = NULL; + + s = nm_sd_dns_name_normalize(str); + if (!s) + return NULL; + + if (nm_str_is_empty(s) || (s[0] == '.' && s[1] == '\0')) { + /* root domains are not allowed. */ + return NULL; + } + + if (nm_utils_is_localhost(s)) + return NULL; + + if (!g_utf8_validate(s, -1, NULL)) { + /* the result must be valid UTF-8. */ + return NULL; + } + + return g_steal_pointer(&s); +} + +gboolean +nm_dhcp_lease_data_parse_domain(const guint8 *data, gsize n_data, char **out_val) +{ + gs_free char *str1_free = NULL; + const char * str1; + gs_free char *s = NULL; + + /* this is mostly the same as systemd's lease_parse_domain(). */ + + if (!nm_dhcp_lease_data_parse_cstr(data, n_data, &n_data)) + return FALSE; + + if (n_data == 0) { + /* empty domains are rejected. See + * https://tools.ietf.org/html/rfc2132#section-3.14 + * https://tools.ietf.org/html/rfc2132#section-3.17 + * + * Its minimum length is 1. + * + * Note that this is *after* we potentially stripped a trailing NUL. + */ + return FALSE; + } + + str1 = nm_strndup_a(300, (char *) data, n_data, &str1_free); + + s = nm_dhcp_lease_data_parse_domain_validate(str1); + if (!s) + return FALSE; + + *out_val = g_steal_pointer(&s); + return TRUE; +} diff --git a/src/core/dhcp/nm-dhcp-utils.h b/src/core/dhcp/nm-dhcp-utils.h index 52e1ea4fb1..379489a74d 100644 --- a/src/core/dhcp/nm-dhcp-utils.h +++ b/src/core/dhcp/nm-dhcp-utils.h @@ -42,7 +42,11 @@ char *nm_dhcp_utils_get_dhcp6_event_id(GHashTable *lease); /*****************************************************************************/ +char *nm_dhcp_lease_data_parse_domain_validate(const char *str); + gboolean nm_dhcp_lease_data_parse_u16(const guint8 *data, gsize n_data, guint16 *out_val); gboolean nm_dhcp_lease_data_parse_mtu(const guint8 *data, gsize n_data, guint16 *out_val); +gboolean nm_dhcp_lease_data_parse_cstr(const guint8 *data, gsize n_data, gsize *out_new_len); +gboolean nm_dhcp_lease_data_parse_domain(const guint8 *data, gsize n_data, char **out_val); #endif /* __NETWORKMANAGER_DHCP_UTILS_H__ */ From 8f7a2a1ea0dae4b81563974945d0ecb9827ac74b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 9 Feb 2021 21:12:35 +0100 Subject: [PATCH 11/34] dhcp/nettools: refactor parsing of DHCP lease (wpad) --- src/core/dhcp/nm-dhcp-nettools.c | 39 +++++++++++--------------------- 1 file changed, 13 insertions(+), 26 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 3bcf5a7ca6..90520dab3e 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -824,31 +824,6 @@ lease_parse_root_path(NDhcp4ClientLease *lease, GHashTable *options) str->str); } -static void -lease_parse_wpad(NDhcp4ClientLease *lease, GHashTable *options) -{ - gs_free char *wpad = NULL; - uint8_t * data; - size_t n_data; - int r; - - r = n_dhcp4_client_lease_query(lease, - NM_DHCP_OPTION_DHCP4_PRIVATE_PROXY_AUTODISCOVERY, - &data, - &n_data); - if (r) - return; - - nm_utils_buf_utf8safe_escape((char *) data, n_data, 0, &wpad); - if (wpad == NULL) - wpad = g_strndup((char *) data, n_data); - - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_PRIVATE_PROXY_AUTODISCOVERY, - wpad); -} - static void lease_parse_nis_domain(NDhcp4ClientLease *lease, NMIP4Config *ip4_config, GHashTable *options) { @@ -971,7 +946,19 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, lease_parse_ntps(lease, options); lease_parse_root_path(lease, options); - lease_parse_wpad(lease, options); + + r = n_dhcp4_client_lease_query(lease, + NM_DHCP_OPTION_DHCP4_PRIVATE_PROXY_AUTODISCOVERY, + &l_data, + &l_data_len); + if (r == 0) { + nm_dhcp_option_add_option_utf8safe_escape(options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_PRIVATE_PROXY_AUTODISCOVERY, + l_data, + l_data_len); + } + lease_parse_nis_domain(lease, ip4_config, options); lease_parse_address_list(lease, ip4_config, NM_DHCP_OPTION_DHCP4_NIS_SERVERS, options); lease_parse_address_list(lease, ip4_config, NM_DHCP_OPTION_DHCP4_NETBIOS_NAMESERVER, options); From eb16cb65633882546e0e3e50e95ff0df8facaad1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 11:49:09 +0100 Subject: [PATCH 12/34] dhcp/nettools: validate proxy-autodiscovery option (252) to not contain any NUL characters --- src/core/dhcp/nm-dhcp-nettools.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 90520dab3e..2c6d07775f 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -951,7 +951,12 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, NM_DHCP_OPTION_DHCP4_PRIVATE_PROXY_AUTODISCOVERY, &l_data, &l_data_len); - if (r == 0) { + if (r == 0 && nm_dhcp_lease_data_parse_cstr(l_data, l_data_len, &l_data_len)) { + /* https://tools.ietf.org/html/draft-ietf-wrec-wpad-01#section-4.4.1 + * + * We reject NUL characters inside the string (except one trailing NUL). + * Otherwise, we allow any encoding and backslash-escape the result to + * UTF-8. */ nm_dhcp_option_add_option_utf8safe_escape(options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_PRIVATE_PROXY_AUTODISCOVERY, From f2885cdf02be106ff9c59398f49588506e7f8c4b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 11:54:20 +0100 Subject: [PATCH 13/34] dhcp/nettools: refactor parsing of DHCP lease (root-path) --- src/core/dhcp/nm-dhcp-nettools.c | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 2c6d07775f..65095f0edb 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -805,25 +805,6 @@ lease_parse_search_domains(NDhcp4ClientLease *lease, NMIP4Config *ip4_config, GH str->str); } -static void -lease_parse_root_path(NDhcp4ClientLease *lease, GHashTable *options) -{ - nm_auto_free_gstring GString *str = NULL; - uint8_t * data; - size_t n_data; - int r; - - r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_ROOT_PATH, &data, &n_data); - if (r) - return; - - str = g_string_new_len((char *) data, n_data); - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_ROOT_PATH, - str->str); -} - static void lease_parse_nis_domain(NDhcp4ClientLease *lease, NMIP4Config *ip4_config, GHashTable *options) { @@ -945,7 +926,14 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, } lease_parse_ntps(lease, options); - lease_parse_root_path(lease, options); + + r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_ROOT_PATH, &l_data, &l_data_len); + if (r == 0) { + nm_dhcp_option_add_option(options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_ROOT_PATH, + g_strndup((char *) l_data, l_data_len)); + } r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_PRIVATE_PROXY_AUTODISCOVERY, From 0ef37431cfb52b79d849a55517ec58994f26f38e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 11:58:36 +0100 Subject: [PATCH 14/34] dhcp/nettools: validate root-path option (17) to not contain any NUL characters And make it UTF-8 (by backslash escaping). --- src/core/dhcp/nm-dhcp-nettools.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 65095f0edb..a46e5430b3 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -928,11 +928,22 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, lease_parse_ntps(lease, options); r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_ROOT_PATH, &l_data, &l_data_len); - if (r == 0) { - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_ROOT_PATH, - g_strndup((char *) l_data, l_data_len)); + if (r == 0 && nm_dhcp_lease_data_parse_cstr(l_data, l_data_len, &l_data_len)) { + /* https://tools.ietf.org/html/rfc2132#section-3.19 + * + * The path is formatted as a character string consisting of + * characters from the NVT ASCII character set. + * + * We still accept any character set and backslash escape it! */ + if (l_data_len == 0) { + /* "Its minimum length is 1." */ + } else { + nm_dhcp_option_add_option_utf8safe_escape(options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_ROOT_PATH, + l_data, + l_data_len); + } } r = n_dhcp4_client_lease_query(lease, From 0c93bff17912a1e47c297b83c4dca09c6dea7eba Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 11:54:20 +0100 Subject: [PATCH 15/34] dhcp/nettools: refactor parsing of DHCP lease (nis-domain) --- src/core/dhcp/nm-dhcp-nettools.c | 48 +++++++++++++------------------- 1 file changed, 20 insertions(+), 28 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index a46e5430b3..3e576b960f 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -805,33 +805,6 @@ lease_parse_search_domains(NDhcp4ClientLease *lease, NMIP4Config *ip4_config, GH str->str); } -static void -lease_parse_nis_domain(NDhcp4ClientLease *lease, NMIP4Config *ip4_config, GHashTable *options) -{ - gs_free char *str_free = NULL; - const char * str; - uint8_t * data; - size_t n_data; - guint i; - int r; - - r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_NIS_DOMAIN, &data, &n_data); - if (r) - return; - - for (i = 0; i < n_data; i++) { - if (!nm_is_ascii((char) data[i])) - return; - } - - str = nm_strndup_a(300, (const char *) data, n_data, &str_free); - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_NIS_DOMAIN, - str); - nm_ip4_config_set_nis_domain(ip4_config, str); -} - static void lease_parse_private_options(NDhcp4ClientLease *lease, GHashTable *options) { @@ -877,9 +850,11 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, gs_unref_hashtable GHashTable *options = NULL; guint8 * l_data; gsize l_data_len; + const char * v_str; guint16 v_u16; gboolean v_bool; int r; + gsize i; g_return_val_if_fail(lease != NULL, NULL); @@ -963,7 +938,24 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, l_data_len); } - lease_parse_nis_domain(lease, ip4_config, options); + r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_NIS_DOMAIN, &l_data, &l_data_len); + if (r == 0) { + gs_free char *str_free = NULL; + + for (i = 0; i < l_data_len; i++) { + if (!nm_is_ascii((char) l_data[i])) + goto nis_domain_done; + } + + v_str = nm_strndup_a(300, (const char *) l_data, l_data_len, &str_free); + nm_dhcp_option_add_option(options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_NIS_DOMAIN, + v_str); + nm_ip4_config_set_nis_domain(ip4_config, v_str); + } +nis_domain_done: + lease_parse_address_list(lease, ip4_config, NM_DHCP_OPTION_DHCP4_NIS_SERVERS, options); lease_parse_address_list(lease, ip4_config, NM_DHCP_OPTION_DHCP4_NETBIOS_NAMESERVER, options); lease_parse_private_options(lease, options); From 6c8a9e8bd6074f358fdae6044e22c8fcec18279b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 11:58:36 +0100 Subject: [PATCH 16/34] dhcp/nettools: validate nis-domain option (40) differently Previously, we would check that all characters are ASCII. But we would also accept NUL characters (and truncate on the first NUL). Now: - reject any NUL characters inside the string (except trailing NUL). - accept all characters, and if necessary backslash-encode non UTF-8. --- src/core/dhcp/nm-dhcp-nettools.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 3e576b960f..d165ea6b6d 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -854,7 +854,6 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, guint16 v_u16; gboolean v_bool; int r; - gsize i; g_return_val_if_fail(lease != NULL, NULL); @@ -939,22 +938,19 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, } r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_NIS_DOMAIN, &l_data, &l_data_len); - if (r == 0) { - gs_free char *str_free = NULL; + if (r == 0 && nm_dhcp_lease_data_parse_cstr(l_data, l_data_len, &l_data_len)) { + gs_free char *to_free = NULL; - for (i = 0; i < l_data_len; i++) { - if (!nm_is_ascii((char) l_data[i])) - goto nis_domain_done; - } + /* https://tools.ietf.org/html/rfc2132#section-8.1 */ + + v_str = nm_utils_buf_utf8safe_escape((char *) l_data, l_data_len, 0, &to_free); - v_str = nm_strndup_a(300, (const char *) l_data, l_data_len, &str_free); nm_dhcp_option_add_option(options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_NIS_DOMAIN, v_str); nm_ip4_config_set_nis_domain(ip4_config, v_str); } -nis_domain_done: lease_parse_address_list(lease, ip4_config, NM_DHCP_OPTION_DHCP4_NIS_SERVERS, options); lease_parse_address_list(lease, ip4_config, NM_DHCP_OPTION_DHCP4_NETBIOS_NAMESERVER, options); From 94c6f3c14ed5669b4190d6cf7134478a961923d6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 11:54:20 +0100 Subject: [PATCH 17/34] dhcp/nettools: refactor parsing of DHCP lease (domain-name) No change in behavior. --- src/core/dhcp/nm-dhcp-nettools.c | 65 ++++++++++++++++---------------- 1 file changed, 32 insertions(+), 33 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index d165ea6b6d..063c44c132 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -717,38 +717,6 @@ lease_parse_ntps(NDhcp4ClientLease *lease, GHashTable *options) str->str); } -static void -lease_parse_domainname(NDhcp4ClientLease *lease, NMIP4Config *ip4_config, GHashTable *options) -{ - nm_auto_free_gstring GString *str = NULL; - gs_strfreev char ** domains = NULL; - uint8_t * data; - size_t n_data; - int r; - - r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_DOMAIN_NAME, &data, &n_data); - if (r) - return; - - str = g_string_new_len((char *) data, n_data); - - /* Multiple domains sometimes stuffed into option 15 "Domain Name". */ - domains = g_strsplit(str->str, " ", 0); - nm_gstring_prepare(&str); - - for (char **d = domains; *d; d++) { - if (nm_utils_is_localhost(*d)) - return; - - g_string_append(nm_gstring_add_space_delimiter(str), *d); - nm_ip4_config_add_domain(ip4_config, *d); - } - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_DOMAIN_NAME, - str->str); -} - char ** nm_dhcp_parse_search_list(guint8 *data, size_t n_data) { @@ -867,7 +835,38 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, lease_parse_broadcast(lease, ip4_config, options); lease_parse_routes(lease, ip4_config, options, route_table, route_metric); lease_parse_address_list(lease, ip4_config, NM_DHCP_OPTION_DHCP4_DOMAIN_NAME_SERVER, options); - lease_parse_domainname(lease, ip4_config, options); + + r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_DOMAIN_NAME, &l_data, &l_data_len); + if (r == 0) { + gs_free const char **domains = NULL; + + nm_str_buf_reset(&sbuf); + nm_str_buf_append_len(&sbuf, (const char *) l_data, l_data_len); + + /* Multiple domains sometimes stuffed into option 15 "Domain Name". */ + domains = nm_utils_strsplit_set(nm_str_buf_get_str(&sbuf), " "); + + nm_str_buf_reset(&sbuf); + if (domains) { + gsize i; + + for (i = 0; domains[i]; i++) { + if (nm_utils_is_localhost(domains[i])) + goto domain_name_done; + + nm_str_buf_append_required_delimiter(&sbuf, ' '); + nm_str_buf_append(&sbuf, domains[i]); + nm_ip4_config_add_domain(ip4_config, domains[i]); + } + } + + nm_dhcp_option_add_option(options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_DOMAIN_NAME, + nm_str_buf_get_str(&sbuf)); + } +domain_name_done: + lease_parse_search_domains(lease, ip4_config, options); r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_INTERFACE_MTU, &l_data, &l_data_len); From a24b7287d828c715694edecc2f1d03f468639883 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 11:58:36 +0100 Subject: [PATCH 18/34] dhcp/nettools: validate domain-name option (15) differently --- src/core/dhcp/nm-dhcp-nettools.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 063c44c132..7ba4d9caf3 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -837,11 +837,11 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, lease_parse_address_list(lease, ip4_config, NM_DHCP_OPTION_DHCP4_DOMAIN_NAME_SERVER, options); r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_DOMAIN_NAME, &l_data, &l_data_len); - if (r == 0) { + if (r == 0 && nm_dhcp_lease_data_parse_cstr(l_data, l_data_len, &l_data_len)) { gs_free const char **domains = NULL; nm_str_buf_reset(&sbuf); - nm_str_buf_append_len(&sbuf, (const char *) l_data, l_data_len); + nm_str_buf_append_len0(&sbuf, (const char *) l_data, l_data_len); /* Multiple domains sometimes stuffed into option 15 "Domain Name". */ domains = nm_utils_strsplit_set(nm_str_buf_get_str(&sbuf), " "); @@ -851,21 +851,25 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, gsize i; for (i = 0; domains[i]; i++) { - if (nm_utils_is_localhost(domains[i])) - goto domain_name_done; + gs_free char *s = NULL; + + s = nm_dhcp_lease_data_parse_domain_validate(domains[i]); + if (!s) + continue; nm_str_buf_append_required_delimiter(&sbuf, ' '); - nm_str_buf_append(&sbuf, domains[i]); - nm_ip4_config_add_domain(ip4_config, domains[i]); + nm_str_buf_append(&sbuf, s); + nm_ip4_config_add_domain(ip4_config, s); } } - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_DOMAIN_NAME, - nm_str_buf_get_str(&sbuf)); + if (sbuf.len > 0) { + nm_dhcp_option_add_option(options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_DOMAIN_NAME, + nm_str_buf_get_str(&sbuf)); + } } -domain_name_done: lease_parse_search_domains(lease, ip4_config, options); From 6850e3640e4201316adf6eb1aa1c1bc00aaa9583 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 11:54:20 +0100 Subject: [PATCH 19/34] dhcp/nettools: refactor parsing of DHCP lease (broadcast) No change in behavior. --- src/core/dhcp/nm-dhcp-nettools.c | 27 ++++++++++----------------- src/core/dhcp/nm-dhcp-utils.c | 13 +++++++++++++ src/core/dhcp/nm-dhcp-utils.h | 1 + 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 7ba4d9caf3..2456c23a6c 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -512,22 +512,6 @@ lease_parse_server_id(NDhcp4ClientLease *lease, NMIP4Config *ip4_config, GHashTa addr_str); } -static void -lease_parse_broadcast(NDhcp4ClientLease *lease, NMIP4Config *ip4_config, GHashTable *options) -{ - struct in_addr addr; - char addr_str[NM_UTILS_INET_ADDRSTRLEN]; - - if (!lease_get_in_addr(lease, NM_DHCP_OPTION_DHCP4_BROADCAST, &addr)) - return; - - _nm_utils_inet4_ntop(addr.s_addr, addr_str); - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_BROADCAST, - addr_str); -} - static void lease_parse_routes(NDhcp4ClientLease *lease, NMIP4Config * ip4_config, @@ -821,6 +805,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, const char * v_str; guint16 v_u16; gboolean v_bool; + in_addr_t v_inaddr; int r; g_return_val_if_fail(lease != NULL, NULL); @@ -832,7 +817,15 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, return NULL; lease_parse_server_id(lease, ip4_config, options); - lease_parse_broadcast(lease, ip4_config, options); + + r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_BROADCAST, &l_data, &l_data_len); + if (r == 0 && nm_dhcp_lease_data_parse_in_addr(l_data, l_data_len, &v_inaddr)) { + nm_dhcp_option_add_option_in_addr(options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_BROADCAST, + v_inaddr); + } + lease_parse_routes(lease, ip4_config, options, route_table, route_metric); lease_parse_address_list(lease, ip4_config, NM_DHCP_OPTION_DHCP4_DOMAIN_NAME_SERVER, options); diff --git a/src/core/dhcp/nm-dhcp-utils.c b/src/core/dhcp/nm-dhcp-utils.c index 508c3d0930..b2064153d5 100644 --- a/src/core/dhcp/nm-dhcp-utils.c +++ b/src/core/dhcp/nm-dhcp-utils.c @@ -950,3 +950,16 @@ nm_dhcp_lease_data_parse_domain(const guint8 *data, gsize n_data, char **out_val *out_val = g_steal_pointer(&s); return TRUE; } + +gboolean +nm_dhcp_lease_data_parse_in_addr(const guint8 *data, gsize n_data, in_addr_t *out_val) +{ + /* - option 28, https://tools.ietf.org/html/rfc2132#section-5.3 + */ + + if (n_data != 4) + return FALSE; + + *out_val = unaligned_read_ne32(data); + return TRUE; +} diff --git a/src/core/dhcp/nm-dhcp-utils.h b/src/core/dhcp/nm-dhcp-utils.h index 379489a74d..30f850ce1e 100644 --- a/src/core/dhcp/nm-dhcp-utils.h +++ b/src/core/dhcp/nm-dhcp-utils.h @@ -48,5 +48,6 @@ gboolean nm_dhcp_lease_data_parse_u16(const guint8 *data, gsize n_data, guint16 gboolean nm_dhcp_lease_data_parse_mtu(const guint8 *data, gsize n_data, guint16 *out_val); gboolean nm_dhcp_lease_data_parse_cstr(const guint8 *data, gsize n_data, gsize *out_new_len); gboolean nm_dhcp_lease_data_parse_domain(const guint8 *data, gsize n_data, char **out_val); +gboolean nm_dhcp_lease_data_parse_in_addr(const guint8 *data, gsize n_data, in_addr_t *out_val); #endif /* __NETWORKMANAGER_DHCP_UTILS_H__ */ From 58b3b7ec3c064363a879e4e835974b75886cb8ed Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 11:54:20 +0100 Subject: [PATCH 20/34] dhcp/nettools: refactor parsing of DHCP lease (server-id) No change in behavior. --- src/core/dhcp/nm-dhcp-nettools.c | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 2456c23a6c..c2bb164244 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -496,22 +496,6 @@ lease_parse_address_list(NDhcp4ClientLease * lease, nm_dhcp_option_add_option(options, _nm_dhcp_option_dhcp4_options, option, str->str); } -static void -lease_parse_server_id(NDhcp4ClientLease *lease, NMIP4Config *ip4_config, GHashTable *options) -{ - struct in_addr addr; - char addr_str[NM_UTILS_INET_ADDRSTRLEN]; - - if (n_dhcp4_client_lease_get_server_identifier(lease, &addr)) - return; - - _nm_utils_inet4_ntop(addr.s_addr, addr_str); - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_SERVER_ID, - addr_str); -} - static void lease_parse_routes(NDhcp4ClientLease *lease, NMIP4Config * ip4_config, @@ -806,6 +790,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, guint16 v_u16; gboolean v_bool; in_addr_t v_inaddr; + struct in_addr v_inaddr_s; int r; g_return_val_if_fail(lease != NULL, NULL); @@ -816,7 +801,13 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, if (!lease_parse_address(lease, ip4_config, options, error)) return NULL; - lease_parse_server_id(lease, ip4_config, options); + r = n_dhcp4_client_lease_get_server_identifier(lease, &v_inaddr_s); + if (r == 0) { + nm_dhcp_option_add_option_in_addr(options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_SERVER_ID, + v_inaddr_s.s_addr); + } r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_BROADCAST, &l_data, &l_data_len); if (r == 0 && nm_dhcp_lease_data_parse_in_addr(l_data, l_data_len, &v_inaddr)) { From 30911a305f957f1795ca1f65d04d5076adaef974 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 15:08:00 +0100 Subject: [PATCH 21/34] dhcp/nettools: cleanup lease_parse_address() --- src/core/dhcp/nm-dhcp-nettools.c | 59 ++++++++++---------------------- src/core/dhcp/nm-dhcp-utils.c | 3 +- 2 files changed, 21 insertions(+), 41 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index c2bb164244..5f0a385386 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -305,43 +305,23 @@ lease_option_print_domain_name(GString * str, } } -static gboolean -lease_get_in_addr(NDhcp4ClientLease *lease, guint8 option, struct in_addr *addrp) -{ - struct in_addr addr; - uint8_t * data; - size_t n_data; - int r; - - r = n_dhcp4_client_lease_query(lease, option, &data, &n_data); - if (r) - return FALSE; - - if (!lease_option_next_in_addr(&addr, &data, &n_data)) - return FALSE; - - if (n_data != 0) - return FALSE; - - *addrp = addr; - return TRUE; -} - static gboolean lease_parse_address(NDhcp4ClientLease *lease, NMIP4Config * ip4_config, GHashTable * options, GError ** error) { - char addr_str[NM_UTILS_INET_ADDRSTRLEN]; struct in_addr a_address; - struct in_addr a_netmask; + in_addr_t a_netmask; struct in_addr a_next_server; guint32 a_plen; guint64 nettools_lifetime; guint32 a_lifetime; guint32 a_timestamp; guint64 a_expiry; + guint8 * l_data; + gsize l_data_len; + int r; n_dhcp4_client_lease_get_yiaddr(lease, &a_address); if (a_address.s_addr == INADDR_ANY) { @@ -396,24 +376,24 @@ lease_parse_address(NDhcp4ClientLease *lease, / NM_UTILS_NSEC_PER_SEC); } - if (!lease_get_in_addr(lease, NM_DHCP_OPTION_DHCP4_SUBNET_MASK, &a_netmask)) { + r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_SUBNET_MASK, &l_data, &l_data_len); + if (r != 0 || !nm_dhcp_lease_data_parse_in_addr(l_data, l_data_len, &a_netmask)) { nm_utils_error_set_literal(error, NM_UTILS_ERROR_UNKNOWN, "could not get netmask from lease"); return FALSE; } - _nm_utils_inet4_ntop(a_address.s_addr, addr_str); - a_plen = nm_utils_ip4_netmask_to_prefix(a_netmask.s_addr); + a_plen = nm_utils_ip4_netmask_to_prefix(a_netmask); - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_NM_IP_ADDRESS, - addr_str); - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_SUBNET_MASK, - _nm_utils_inet4_ntop(a_netmask.s_addr, addr_str)); + nm_dhcp_option_add_option_in_addr(options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_NM_IP_ADDRESS, + a_address.s_addr); + nm_dhcp_option_add_option_in_addr(options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_SUBNET_MASK, + a_netmask); nm_dhcp_option_add_option_u64(options, _nm_dhcp_option_dhcp4_options, @@ -429,11 +409,10 @@ lease_parse_address(NDhcp4ClientLease *lease, n_dhcp4_client_lease_get_siaddr(lease, &a_next_server); if (a_next_server.s_addr != INADDR_ANY) { - _nm_utils_inet4_ntop(a_next_server.s_addr, addr_str); - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_NM_NEXT_SERVER, - addr_str); + nm_dhcp_option_add_option_in_addr(options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_NM_NEXT_SERVER, + a_next_server.s_addr); } nm_ip4_config_add_address(ip4_config, diff --git a/src/core/dhcp/nm-dhcp-utils.c b/src/core/dhcp/nm-dhcp-utils.c index b2064153d5..b3b0678dc8 100644 --- a/src/core/dhcp/nm-dhcp-utils.c +++ b/src/core/dhcp/nm-dhcp-utils.c @@ -954,7 +954,8 @@ nm_dhcp_lease_data_parse_domain(const guint8 *data, gsize n_data, char **out_val gboolean nm_dhcp_lease_data_parse_in_addr(const guint8 *data, gsize n_data, in_addr_t *out_val) { - /* - option 28, https://tools.ietf.org/html/rfc2132#section-5.3 + /* - option 1, https://tools.ietf.org/html/rfc2132#section-3.3 + * - option 28, https://tools.ietf.org/html/rfc2132#section-5.3 */ if (n_data != 4) From f986d409f9829e99866aacb30fc47902169e871e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 15:48:56 +0100 Subject: [PATCH 22/34] dhcp/nettools: cleanup lease_parse_address_list() --- src/core/dhcp/nm-dhcp-nettools.c | 60 +++++++++++++++++++++----------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 5f0a385386..b899ede095 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -433,46 +433,53 @@ static void lease_parse_address_list(NDhcp4ClientLease * lease, NMIP4Config * ip4_config, NMDhcpOptionDhcp4Options option, - GHashTable * options) + GHashTable * options, + NMStrBuf * sbuf) { - nm_auto_free_gstring GString *str = NULL; - char addr_str[NM_UTILS_INET_ADDRSTRLEN]; - struct in_addr addr; - uint8_t * data; - size_t n_data; - int r; + guint8 *l_data; + gsize l_data_len; + int r; - r = n_dhcp4_client_lease_query(lease, option, &data, &n_data); - if (r) + r = n_dhcp4_client_lease_query(lease, option, &l_data, &l_data_len); + if (r != 0) return; - nm_gstring_prepare(&str); + if (l_data_len == 0 || l_data_len % 4 != 0) + return; - while (lease_option_next_in_addr(&addr, &data, &n_data)) { - _nm_utils_inet4_ntop(addr.s_addr, addr_str); - g_string_append(nm_gstring_add_space_delimiter(str), addr_str); + nm_str_buf_reset(sbuf); + + for (; l_data_len > 0; l_data_len -= 4, l_data += 4) { + char addr_str[NM_UTILS_INET_ADDRSTRLEN]; + const in_addr_t addr = unaligned_read_ne32(l_data); + + nm_str_buf_append_required_delimiter(sbuf, ' '); + nm_str_buf_append(sbuf, _nm_utils_inet4_ntop(addr, addr_str)); switch (option) { case NM_DHCP_OPTION_DHCP4_DOMAIN_NAME_SERVER: - if (addr.s_addr == 0 || nm_ip4_addr_is_localhost(addr.s_addr)) { + if (addr == 0 || nm_ip4_addr_is_localhost(addr)) { /* Skip localhost addresses, like also networkd does. * See https://github.com/systemd/systemd/issues/4524. */ continue; } - nm_ip4_config_add_nameserver(ip4_config, addr.s_addr); + nm_ip4_config_add_nameserver(ip4_config, addr); break; case NM_DHCP_OPTION_DHCP4_NIS_SERVERS: - nm_ip4_config_add_nis_server(ip4_config, addr.s_addr); + nm_ip4_config_add_nis_server(ip4_config, addr); break; case NM_DHCP_OPTION_DHCP4_NETBIOS_NAMESERVER: - nm_ip4_config_add_wins(ip4_config, addr.s_addr); + nm_ip4_config_add_wins(ip4_config, addr); break; default: nm_assert_not_reached(); } } - nm_dhcp_option_add_option(options, _nm_dhcp_option_dhcp4_options, option, str->str); + nm_dhcp_option_add_option(options, + _nm_dhcp_option_dhcp4_options, + option, + nm_str_buf_get_str(sbuf)); } static void @@ -797,7 +804,12 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, } lease_parse_routes(lease, ip4_config, options, route_table, route_metric); - lease_parse_address_list(lease, ip4_config, NM_DHCP_OPTION_DHCP4_DOMAIN_NAME_SERVER, options); + + lease_parse_address_list(lease, + ip4_config, + NM_DHCP_OPTION_DHCP4_DOMAIN_NAME_SERVER, + options, + &sbuf); r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_DOMAIN_NAME, &l_data, &l_data_len); if (r == 0 && nm_dhcp_lease_data_parse_cstr(l_data, l_data_len, &l_data_len)) { @@ -918,8 +930,14 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, nm_ip4_config_set_nis_domain(ip4_config, v_str); } - lease_parse_address_list(lease, ip4_config, NM_DHCP_OPTION_DHCP4_NIS_SERVERS, options); - lease_parse_address_list(lease, ip4_config, NM_DHCP_OPTION_DHCP4_NETBIOS_NAMESERVER, options); + lease_parse_address_list(lease, ip4_config, NM_DHCP_OPTION_DHCP4_NIS_SERVERS, options, &sbuf); + + lease_parse_address_list(lease, + ip4_config, + NM_DHCP_OPTION_DHCP4_NETBIOS_NAMESERVER, + options, + &sbuf); + lease_parse_private_options(lease, options); NM_SET_OUT(out_options, g_steal_pointer(&options)); From 2be43d79f7d9e78594ee9b932bdc8dfd53303fee Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 14:37:50 +0100 Subject: [PATCH 23/34] dhcp/nettools: refactor parsing of DHCP lease (ntps) --- src/core/dhcp/nm-dhcp-nettools.c | 31 +++---------------------------- 1 file changed, 3 insertions(+), 28 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index b899ede095..9f6f3497d9 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -471,6 +471,8 @@ lease_parse_address_list(NDhcp4ClientLease * lease, case NM_DHCP_OPTION_DHCP4_NETBIOS_NAMESERVER: nm_ip4_config_add_wins(ip4_config, addr); break; + case NM_DHCP_OPTION_DHCP4_NTP_SERVER: + break; default: nm_assert_not_reached(); } @@ -644,33 +646,6 @@ lease_parse_routes(NDhcp4ClientLease *lease, } } -static void -lease_parse_ntps(NDhcp4ClientLease *lease, GHashTable *options) -{ - nm_auto_free_gstring GString *str = NULL; - char addr_str[NM_UTILS_INET_ADDRSTRLEN]; - struct in_addr addr; - uint8_t * data; - size_t n_data; - int r; - - r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_NTP_SERVER, &data, &n_data); - if (r) - return; - - nm_gstring_prepare(&str); - - while (lease_option_next_in_addr(&addr, &data, &n_data)) { - _nm_utils_inet4_ntop(addr.s_addr, addr_str); - g_string_append(nm_gstring_add_space_delimiter(str), addr_str); - } - - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_NTP_SERVER, - str->str); -} - char ** nm_dhcp_parse_search_list(guint8 *data, size_t n_data) { @@ -877,7 +852,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, } } - lease_parse_ntps(lease, options); + lease_parse_address_list(lease, ip4_config, NM_DHCP_OPTION_DHCP4_NTP_SERVER, options, &sbuf); r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_ROOT_PATH, &l_data, &l_data_len); if (r == 0 && nm_dhcp_lease_data_parse_cstr(l_data, l_data_len, &l_data_len)) { From 94e474fa624542bc5eb11a894d265b4e15c2d132 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 15:08:00 +0100 Subject: [PATCH 24/34] dhcp/nettools: cleanup lease_parse_routes() --- src/core/dhcp/nm-dhcp-nettools.c | 159 ++++++++++++++++--------------- 1 file changed, 83 insertions(+), 76 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 9f6f3497d9..37f812c9fb 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -94,9 +94,9 @@ enum { }; static int -in_addr_class(struct in_addr addr) +in_addr_class(in_addr_t addr) { - switch (ntohl(addr.s_addr) >> 24) { + switch (ntohl(addr) >> 24) { case 0 ... 127: return NM_IN_ADDR_CLASS_A; case 128 ... 191: @@ -109,7 +109,7 @@ in_addr_class(struct in_addr addr) } static gboolean -lease_option_consume(void *out, size_t n_out, uint8_t **datap, size_t *n_datap) +lease_option_consume(uint8_t **datap, size_t *n_datap, void *out, size_t n_out) { if (*n_datap < n_out) return FALSE; @@ -121,27 +121,29 @@ lease_option_consume(void *out, size_t n_out, uint8_t **datap, size_t *n_datap) } static gboolean -lease_option_next_in_addr(struct in_addr *addrp, uint8_t **datap, size_t *n_datap) +lease_option_consume_in_addr(uint8_t **datap, size_t *n_datap, in_addr_t *addrp) { - return lease_option_consume(addrp, sizeof(struct in_addr), datap, n_datap); + return lease_option_consume(datap, n_datap, addrp, sizeof(struct in_addr)); } static gboolean -lease_option_next_route(struct in_addr *destp, - uint8_t * plenp, - struct in_addr *gatewayp, - gboolean classless, - uint8_t ** datap, - size_t * n_datap) +lease_option_consume_route(uint8_t ** datap, + size_t * n_datap, + gboolean classless, + in_addr_t *destp, + uint8_t * plenp, + in_addr_t *gatewayp) { - struct in_addr dest = {}, gateway; - uint8_t * data = *datap; - size_t n_data = *n_datap; - uint8_t plen; - uint8_t bytes; + in_addr_t dest; + in_addr_t gateway; + uint8_t * data = *datap; + size_t n_data = *n_datap; + uint8_t plen; if (classless) { - if (!lease_option_consume(&plen, sizeof(plen), &data, &n_data)) + uint8_t bytes; + + if (!lease_option_consume(&data, &n_data, &plen, sizeof(plen))) return FALSE; if (plen > 32) @@ -149,10 +151,11 @@ lease_option_next_route(struct in_addr *destp, bytes = plen == 0 ? 0 : ((plen - 1) / 8) + 1; - if (!lease_option_consume(&dest, bytes, &data, &n_data)) + dest = 0; + if (!lease_option_consume(&data, &n_data, &dest, bytes)) return FALSE; } else { - if (!lease_option_next_in_addr(&dest, &data, &n_data)) + if (!lease_option_consume_in_addr(&data, &n_data, &dest)) return FALSE; switch (in_addr_class(dest)) { @@ -170,9 +173,9 @@ lease_option_next_route(struct in_addr *destp, } } - dest.s_addr = nm_utils_ip4_address_clear_host_address(dest.s_addr, plen); + dest = nm_utils_ip4_address_clear_host_address(dest, plen); - if (!lease_option_next_in_addr(&gateway, &data, &n_data)) + if (!lease_option_consume_in_addr(&data, &n_data, &gateway)) return FALSE; *destp = dest; @@ -183,13 +186,15 @@ lease_option_next_route(struct in_addr *destp, return TRUE; } +/*****************************************************************************/ + static gboolean lease_option_print_label(GString *str, size_t n_label, uint8_t **datap, size_t *n_datap) { for (size_t i = 0; i < n_label; ++i) { uint8_t c; - if (!lease_option_consume(&c, sizeof(c), datap, n_datap)) + if (!lease_option_consume(datap, n_datap, &c, sizeof(c))) return FALSE; switch (c) { @@ -246,7 +251,7 @@ lease_option_print_domain_name(GString * str, return FALSE; for (;;) { - if (!lease_option_consume(&c, sizeof(c), domainp, n_domainp)) + if (!lease_option_consume(domainp, n_domainp, &c, sizeof(c))) return FALSE; switch (c & 0xC0) { @@ -282,7 +287,7 @@ lease_option_print_domain_name(GString * str, * two high bits are masked out. */ - if (!lease_option_consume(&c, sizeof(c), domainp, n_domainp)) + if (!lease_option_consume(domainp, n_domainp, &c, sizeof(c))) return FALSE; offset += c; @@ -305,6 +310,8 @@ lease_option_print_domain_name(GString * str, } } +/*****************************************************************************/ + static gboolean lease_parse_address(NDhcp4ClientLease *lease, NMIP4Config * ip4_config, @@ -489,39 +496,37 @@ lease_parse_routes(NDhcp4ClientLease *lease, NMIP4Config * ip4_config, GHashTable * options, guint32 route_table, - guint32 route_metric) + guint32 route_metric, + NMStrBuf * sbuf) { - nm_auto_free_gstring GString *str = NULL; - char dest_str[NM_UTILS_INET_ADDRSTRLEN]; - char gateway_str[NM_UTILS_INET_ADDRSTRLEN]; - const char * s; - struct in_addr dest, gateway; - uint8_t plen; - guint32 m; - gboolean has_router_from_classless = FALSE, has_classless = FALSE; - guint32 default_route_metric = route_metric; - uint8_t * data; - size_t n_data; - int r; + char dest_str[NM_UTILS_INET_ADDRSTRLEN]; + char gateway_str[NM_UTILS_INET_ADDRSTRLEN]; + in_addr_t dest; + in_addr_t gateway; + uint8_t plen; + guint32 m; + gboolean has_router_from_classless = FALSE; + gboolean has_classless = FALSE; + guint32 default_route_metric = route_metric; + guint8 * l_data; + gsize l_data_len; + int r; r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_CLASSLESS_STATIC_ROUTE, - &data, - &n_data); - if (!r) { - nm_gstring_prepare(&str); + &l_data, + &l_data_len); + if (r == 0) { + nm_str_buf_reset(sbuf); has_classless = TRUE; - while (lease_option_next_route(&dest, &plen, &gateway, TRUE, &data, &n_data)) { - _nm_utils_inet4_ntop(dest.s_addr, dest_str); - _nm_utils_inet4_ntop(gateway.s_addr, gateway_str); + while (lease_option_consume_route(&l_data, &l_data_len, TRUE, &dest, &plen, &gateway)) { + _nm_utils_inet4_ntop(dest, dest_str); + _nm_utils_inet4_ntop(gateway, gateway_str); - g_string_append_printf(nm_gstring_add_space_delimiter(str), - "%s/%d %s", - dest_str, - (int) plen, - gateway_str); + nm_str_buf_append_required_delimiter(sbuf, ' '); + nm_str_buf_append_printf(sbuf, "%s/%d %s", dest_str, (int) plen, gateway_str); if (plen == 0) { /* if there are multiple default routes, we add them with differing @@ -538,34 +543,32 @@ lease_parse_routes(NDhcp4ClientLease *lease, nm_ip4_config_add_route( ip4_config, &((const NMPlatformIP4Route){ - .network = dest.s_addr, + .network = dest, .plen = plen, - .gateway = gateway.s_addr, + .gateway = gateway, .rt_source = NM_IP_CONFIG_SOURCE_DHCP, .metric = m, .table_coerced = nm_platform_route_table_coerce(route_table), }), NULL); } + nm_dhcp_option_add_option(options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_CLASSLESS_STATIC_ROUTE, - str->str); + nm_str_buf_get_str(sbuf)); } - r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_STATIC_ROUTE, &data, &n_data); - if (!r) { - nm_gstring_prepare(&str); + r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_STATIC_ROUTE, &l_data, &l_data_len); + if (r == 0) { + nm_str_buf_reset(sbuf); - while (lease_option_next_route(&dest, &plen, &gateway, FALSE, &data, &n_data)) { - _nm_utils_inet4_ntop(dest.s_addr, dest_str); - _nm_utils_inet4_ntop(gateway.s_addr, gateway_str); + while (lease_option_consume_route(&l_data, &l_data_len, FALSE, &dest, &plen, &gateway)) { + _nm_utils_inet4_ntop(dest, dest_str); + _nm_utils_inet4_ntop(gateway, gateway_str); - g_string_append_printf(nm_gstring_add_space_delimiter(str), - "%s/%d %s", - dest_str, - (int) plen, - gateway_str); + nm_str_buf_append_required_delimiter(sbuf, ' '); + nm_str_buf_append_printf(sbuf, "%s/%d %s", dest_str, (int) plen, gateway_str); if (has_classless) { /* RFC 3443: if the DHCP server returns both a Classless Static Routes @@ -585,30 +588,31 @@ lease_parse_routes(NDhcp4ClientLease *lease, nm_ip4_config_add_route( ip4_config, &((const NMPlatformIP4Route){ - .network = dest.s_addr, + .network = dest, .plen = plen, - .gateway = gateway.s_addr, + .gateway = gateway, .rt_source = NM_IP_CONFIG_SOURCE_DHCP, .metric = route_metric, .table_coerced = nm_platform_route_table_coerce(route_table), }), NULL); } + nm_dhcp_option_add_option(options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_STATIC_ROUTE, - str->str); + nm_str_buf_get_str(sbuf)); } - r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_ROUTER, &data, &n_data); - if (!r) { - nm_gstring_prepare(&str); + r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_ROUTER, &l_data, &l_data_len); + if (r == 0) { + nm_str_buf_reset(sbuf); - while (lease_option_next_in_addr(&gateway, &data, &n_data)) { - s = _nm_utils_inet4_ntop(gateway.s_addr, gateway_str); - g_string_append(nm_gstring_add_space_delimiter(str), s); + while (lease_option_consume_in_addr(&l_data, &l_data_len, &gateway)) { + nm_str_buf_append_required_delimiter(sbuf, ' '); + nm_str_buf_append(sbuf, _nm_utils_inet4_ntop(gateway, gateway_str)); - if (gateway.s_addr == 0) { + if (gateway == 0) { /* silently skip 0.0.0.0 */ continue; } @@ -633,19 +637,22 @@ lease_parse_routes(NDhcp4ClientLease *lease, ip4_config, &((const NMPlatformIP4Route){ .rt_source = NM_IP_CONFIG_SOURCE_DHCP, - .gateway = gateway.s_addr, + .gateway = gateway, .table_coerced = nm_platform_route_table_coerce(route_table), .metric = m, }), NULL); } + nm_dhcp_option_add_option(options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_ROUTER, - str->str); + nm_str_buf_get_str(sbuf)); } } +/*****************************************************************************/ + char ** nm_dhcp_parse_search_list(guint8 *data, size_t n_data) { @@ -778,7 +785,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, v_inaddr); } - lease_parse_routes(lease, ip4_config, options, route_table, route_metric); + lease_parse_routes(lease, ip4_config, options, route_table, route_metric, &sbuf); lease_parse_address_list(lease, ip4_config, From 67dd25a396588070d12d0ab1e975679833f49842 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 16:38:21 +0100 Subject: [PATCH 25/34] shared,dhcp: add _nm_utils_ip4_get_default_prefix0() helper --- shared/nm-glib-aux/nm-shared-utils.c | 40 ++++++++++++++-------------- shared/nm-glib-aux/nm-shared-utils.h | 3 ++- src/core/dhcp/nm-dhcp-nettools.c | 36 +++---------------------- 3 files changed, 25 insertions(+), 54 deletions(-) diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c index 492a3d5098..3215a33b5b 100644 --- a/shared/nm-glib-aux/nm-shared-utils.c +++ b/shared/nm-glib-aux/nm-shared-utils.c @@ -786,27 +786,27 @@ nm_utils_ip6_address_same_prefix_cmp(const struct in6_addr *addr_a, return 0; } -/** - * _nm_utils_ip4_get_default_prefix: - * @ip: an IPv4 address (in network byte order) - * - * When the Internet was originally set up, various ranges of IP addresses were - * segmented into three network classes: A, B, and C. This function will return - * a prefix that is associated with the IP address specified defining where it - * falls in the predefined classes. - * - * Returns: the default class prefix for the given IP - **/ -/* The function is originally from ipcalc.c of Red Hat's initscripts. */ -guint32 -_nm_utils_ip4_get_default_prefix(guint32 ip) -{ - if (((ntohl(ip) & 0xFF000000) >> 24) <= 127) - return 8; /* Class A - 255.0.0.0 */ - else if (((ntohl(ip) & 0xFF000000) >> 24) <= 191) - return 16; /* Class B - 255.255.0.0 */ +/*****************************************************************************/ - return 24; /* Class C - 255.255.255.0 */ +guint32 +_nm_utils_ip4_get_default_prefix0(in_addr_t ip) +{ + /* The function is originally from ipcalc.c of Red Hat's initscripts. */ + switch (ntohl(ip) >> 24) { + case 0 ... 127: + return 8; /* Class A */ + case 128 ... 191: + return 16; /* Class B */ + case 192 ... 223: + return 24; /* Class C */ + } + return 0; +} + +guint32 +_nm_utils_ip4_get_default_prefix(in_addr_t ip) +{ + return _nm_utils_ip4_get_default_prefix0(ip) ?: 24; } gboolean diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h index c432b125b6..7d330458c1 100644 --- a/shared/nm-glib-aux/nm-shared-utils.h +++ b/shared/nm-glib-aux/nm-shared-utils.h @@ -699,7 +699,8 @@ nm_utils_escaped_tokens_options_escape_val(const char *val, char **out_to_free) /*****************************************************************************/ guint32 _nm_utils_ip4_prefix_to_netmask(guint32 prefix); -guint32 _nm_utils_ip4_get_default_prefix(guint32 ip); +guint32 _nm_utils_ip4_get_default_prefix0(in_addr_t ip); +guint32 _nm_utils_ip4_get_default_prefix(in_addr_t ip); gconstpointer nm_utils_ipx_address_clear_host_address(int family, gpointer dst, gconstpointer src, guint8 plen); diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 37f812c9fb..1381de47ae 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -86,27 +86,7 @@ set_error_nettools(GError **error, int r, const char *message) #define DHCP_MAX_FQDN_LENGTH 255 -enum { - NM_IN_ADDR_CLASS_A, - NM_IN_ADDR_CLASS_B, - NM_IN_ADDR_CLASS_C, - NM_IN_ADDR_CLASS_INVALID, -}; - -static int -in_addr_class(in_addr_t addr) -{ - switch (ntohl(addr) >> 24) { - case 0 ... 127: - return NM_IN_ADDR_CLASS_A; - case 128 ... 191: - return NM_IN_ADDR_CLASS_B; - case 192 ... 223: - return NM_IN_ADDR_CLASS_C; - default: - return NM_IN_ADDR_CLASS_INVALID; - } -} +/*****************************************************************************/ static gboolean lease_option_consume(uint8_t **datap, size_t *n_datap, void *out, size_t n_out) @@ -158,19 +138,9 @@ lease_option_consume_route(uint8_t ** datap, if (!lease_option_consume_in_addr(&data, &n_data, &dest)) return FALSE; - switch (in_addr_class(dest)) { - case NM_IN_ADDR_CLASS_A: - plen = 8; - break; - case NM_IN_ADDR_CLASS_B: - plen = 16; - break; - case NM_IN_ADDR_CLASS_C: - plen = 24; - break; - case NM_IN_ADDR_CLASS_INVALID: + plen = _nm_utils_ip4_get_default_prefix0(dest); + if (plen == 0) return FALSE; - } } dest = nm_utils_ip4_address_clear_host_address(dest, plen); From 6e0d2e5850594b136b5da9ff2f337a85e8afd8c1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 16:50:34 +0100 Subject: [PATCH 26/34] dhcp/nettools: move nm_dhcp_lease_data_parse_search_list() to nm-dhcp-utils.c --- src/core/dhcp/nm-dhcp-nettools.c | 155 +------------------------- src/core/dhcp/nm-dhcp-utils.c | 155 ++++++++++++++++++++++++++ src/core/dhcp/nm-dhcp-utils.h | 15 ++- src/core/dhcp/tests/test-dhcp-utils.c | 8 +- 4 files changed, 173 insertions(+), 160 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 1381de47ae..0329030fac 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -158,130 +158,6 @@ lease_option_consume_route(uint8_t ** datap, /*****************************************************************************/ -static gboolean -lease_option_print_label(GString *str, size_t n_label, uint8_t **datap, size_t *n_datap) -{ - for (size_t i = 0; i < n_label; ++i) { - uint8_t c; - - if (!lease_option_consume(datap, n_datap, &c, sizeof(c))) - return FALSE; - - switch (c) { - case 'a' ... 'z': - case 'A' ... 'Z': - case '0' ... '9': - case '-': - case '_': - g_string_append_c(str, c); - break; - case '.': - case '\\': - g_string_append_printf(str, "\\%c", c); - break; - default: - g_string_append_printf(str, "\\%3d", c); - } - } - - return TRUE; -} - -static gboolean -lease_option_print_domain_name(GString * str, - uint8_t * cache, - size_t * n_cachep, - uint8_t **datap, - size_t * n_datap) -{ - uint8_t * domain; - size_t n_domain, n_cache = *n_cachep; - uint8_t **domainp = datap; - size_t * n_domainp = n_datap; - gboolean first = TRUE; - uint8_t c; - - /* - * We are given two adjacent memory regions. The @cache contains alreday parsed - * domain names, and the @datap contains the remaining data to parse. - * - * A domain name is formed from a sequence of labels. Each label start with - * a length byte, where the two most significant bits are unset. A zero-length - * label indicates the end of the domain name. - * - * Alternatively, a label can be followed by an offset (indicated by the two - * most significant bits being set in the next byte that is read). The offset - * is an offset into the cache, where the next label of the domain name can - * be found. - * - * Note, that each time a jump to an offset is performed, the size of the - * cache shrinks, so this is guaranteed to terminate. - */ - if (cache + n_cache != *datap) - return FALSE; - - for (;;) { - if (!lease_option_consume(domainp, n_domainp, &c, sizeof(c))) - return FALSE; - - switch (c & 0xC0) { - case 0x00: /* label length */ - { - size_t n_label = c; - - if (n_label == 0) { - /* - * We reached the final label of the domain name. Adjust - * the cache to include the consumed data, and return. - */ - *n_cachep = *datap - cache; - return TRUE; - } - - if (!first) - g_string_append_c(str, '.'); - else - first = FALSE; - - if (!lease_option_print_label(str, n_label, domainp, n_domainp)) - return FALSE; - - break; - } - case 0xC0: /* back pointer */ - { - size_t offset = (c & 0x3F) << 16; - - /* - * The offset is given as two bytes (in big endian), where the - * two high bits are masked out. - */ - - if (!lease_option_consume(domainp, n_domainp, &c, sizeof(c))) - return FALSE; - - offset += c; - - if (offset >= n_cache) - return FALSE; - - domain = cache + offset; - n_domain = n_cache - offset; - n_cache = offset; - - domainp = &domain; - n_domainp = &n_domain; - - break; - } - default: - return FALSE; - } - } -} - -/*****************************************************************************/ - static gboolean lease_parse_address(NDhcp4ClientLease *lease, NMIP4Config * ip4_config, @@ -623,35 +499,6 @@ lease_parse_routes(NDhcp4ClientLease *lease, /*****************************************************************************/ -char ** -nm_dhcp_parse_search_list(guint8 *data, size_t n_data) -{ - GPtrArray *array = NULL; - guint8 * cache = data; - size_t n_cache = 0; - - for (;;) { - nm_auto_free_gstring GString *domain = NULL; - - nm_gstring_prepare(&domain); - - if (!lease_option_print_domain_name(domain, cache, &n_cache, &data, &n_data)) - break; - - if (!array) - array = g_ptr_array_new(); - - g_ptr_array_add(array, g_string_free(domain, FALSE)); - domain = NULL; - } - - if (array) { - g_ptr_array_add(array, NULL); - return (char **) g_ptr_array_free(array, FALSE); - } else - return NULL; -} - static void lease_parse_search_domains(NDhcp4ClientLease *lease, NMIP4Config *ip4_config, GHashTable *options) { @@ -666,7 +513,7 @@ lease_parse_search_domains(NDhcp4ClientLease *lease, NMIP4Config *ip4_config, GH if (r) return; - domains = nm_dhcp_parse_search_list(data, n_data); + domains = nm_dhcp_lease_data_parse_search_list(data, n_data); nm_gstring_prepare(&str); for (i = 0; domains && domains[i]; i++) { diff --git a/src/core/dhcp/nm-dhcp-utils.c b/src/core/dhcp/nm-dhcp-utils.c index b3b0678dc8..6cddb2d5f1 100644 --- a/src/core/dhcp/nm-dhcp-utils.c +++ b/src/core/dhcp/nm-dhcp-utils.c @@ -964,3 +964,158 @@ nm_dhcp_lease_data_parse_in_addr(const guint8 *data, gsize n_data, in_addr_t *ou *out_val = unaligned_read_ne32(data); return TRUE; } + +/*****************************************************************************/ + +static gboolean +lease_option_print_label(GString *str, size_t n_label, const uint8_t **datap, size_t *n_datap) +{ + gsize i; + + for (i = 0; i < n_label; ++i) { + uint8_t c = 0; + + if (!nm_dhcp_lease_data_consume(datap, n_datap, &c, sizeof(c))) + return FALSE; + + switch (c) { + case 'a' ... 'z': + case 'A' ... 'Z': + case '0' ... '9': + case '-': + case '_': + g_string_append_c(str, c); + break; + case '.': + case '\\': + g_string_append_printf(str, "\\%c", c); + break; + default: + g_string_append_printf(str, "\\%3d", c); + } + } + + return TRUE; +} + +static gboolean +lease_option_print_domain_name(GString * str, + const uint8_t * cache, + size_t * n_cachep, + const uint8_t **datap, + size_t * n_datap) +{ + const uint8_t * domain; + size_t n_domain, n_cache = *n_cachep; + const uint8_t **domainp = datap; + size_t * n_domainp = n_datap; + gboolean first = TRUE; + uint8_t c; + + /* + * We are given two adjacent memory regions. The @cache contains alreday parsed + * domain names, and the @datap contains the remaining data to parse. + * + * A domain name is formed from a sequence of labels. Each label start with + * a length byte, where the two most significant bits are unset. A zero-length + * label indicates the end of the domain name. + * + * Alternatively, a label can be followed by an offset (indicated by the two + * most significant bits being set in the next byte that is read). The offset + * is an offset into the cache, where the next label of the domain name can + * be found. + * + * Note, that each time a jump to an offset is performed, the size of the + * cache shrinks, so this is guaranteed to terminate. + */ + if (cache + n_cache != *datap) + return FALSE; + + for (;;) { + if (!nm_dhcp_lease_data_consume(domainp, n_domainp, &c, sizeof(c))) + return FALSE; + + switch (c & 0xC0) { + case 0x00: /* label length */ + { + size_t n_label = c; + + if (n_label == 0) { + /* + * We reached the final label of the domain name. Adjust + * the cache to include the consumed data, and return. + */ + *n_cachep = *datap - cache; + return TRUE; + } + + if (!first) + g_string_append_c(str, '.'); + else + first = FALSE; + + if (!lease_option_print_label(str, n_label, domainp, n_domainp)) + return FALSE; + + break; + } + case 0xC0: /* back pointer */ + { + size_t offset = (c & 0x3F) << 16; + + /* + * The offset is given as two bytes (in big endian), where the + * two high bits are masked out. + */ + + if (!nm_dhcp_lease_data_consume(domainp, n_domainp, &c, sizeof(c))) + return FALSE; + + offset += c; + + if (offset >= n_cache) + return FALSE; + + domain = cache + offset; + n_domain = n_cache - offset; + n_cache = offset; + + domainp = &domain; + n_domainp = &n_domain; + + break; + } + default: + return FALSE; + } + } +} + +char ** +nm_dhcp_lease_data_parse_search_list(const guint8 *data, gsize n_data) +{ + GPtrArray * array = NULL; + const guint8 *cache = data; + gsize n_cache = 0; + + for (;;) { + nm_auto_free_gstring GString *domain = NULL; + + nm_gstring_prepare(&domain); + + if (!lease_option_print_domain_name(domain, cache, &n_cache, &data, &n_data)) + break; + + if (!array) + array = g_ptr_array_new(); + + g_ptr_array_add(array, g_string_free(domain, FALSE)); + domain = NULL; + } + + if (array) { + g_ptr_array_add(array, NULL); + return (char **) g_ptr_array_free(array, FALSE); + } else + return NULL; +} diff --git a/src/core/dhcp/nm-dhcp-utils.h b/src/core/dhcp/nm-dhcp-utils.h index 30f850ce1e..45f74b35d1 100644 --- a/src/core/dhcp/nm-dhcp-utils.h +++ b/src/core/dhcp/nm-dhcp-utils.h @@ -36,12 +36,22 @@ gboolean nm_dhcp_utils_get_leasefile_path(int addr_family, const char *uuid, char ** out_leasefile_path); -char **nm_dhcp_parse_search_list(guint8 *data, size_t n_data); - char *nm_dhcp_utils_get_dhcp6_event_id(GHashTable *lease); /*****************************************************************************/ +static inline gboolean +nm_dhcp_lease_data_consume(const uint8_t **datap, size_t *n_datap, void *out, size_t n_out) +{ + if (*n_datap < n_out) + return FALSE; + + memcpy(out, *datap, n_out); + *datap += n_out; + *n_datap -= n_out; + return TRUE; +} + char *nm_dhcp_lease_data_parse_domain_validate(const char *str); gboolean nm_dhcp_lease_data_parse_u16(const guint8 *data, gsize n_data, guint16 *out_val); @@ -49,5 +59,6 @@ gboolean nm_dhcp_lease_data_parse_mtu(const guint8 *data, gsize n_data, guint16 gboolean nm_dhcp_lease_data_parse_cstr(const guint8 *data, gsize n_data, gsize *out_new_len); gboolean nm_dhcp_lease_data_parse_domain(const guint8 *data, gsize n_data, char **out_val); gboolean nm_dhcp_lease_data_parse_in_addr(const guint8 *data, gsize n_data, in_addr_t *out_val); +char ** nm_dhcp_lease_data_parse_search_list(const guint8 *data, gsize n_data); #endif /* __NETWORKMANAGER_DHCP_UTILS_H__ */ diff --git a/src/core/dhcp/tests/test-dhcp-utils.c b/src/core/dhcp/tests/test-dhcp-utils.c index 02fe640660..a45e7f118f 100644 --- a/src/core/dhcp/tests/test-dhcp-utils.c +++ b/src/core/dhcp/tests/test-dhcp-utils.c @@ -202,7 +202,7 @@ test_parse_search_list(void) char ** domains; data = (guint8[]){0x05, 'l', 'o', 'c', 'a', 'l', 0x00}; - domains = nm_dhcp_parse_search_list(data, 7); + domains = nm_dhcp_lease_data_parse_search_list(data, 7); g_assert(domains); g_assert_cmpint(g_strv_length(domains), ==, 1); g_assert_cmpstr(domains[0], ==, "local"); @@ -211,7 +211,7 @@ test_parse_search_list(void) data = (guint8[]){0x04, 't', 'e', 's', 't', 0x07, 'e', 'x', 'a', 'm', 'p', 'l', 'e', 0x03, 'c', 'o', 'm', 0x00, 0xc0, 0x05, 0x03, 'a', 'b', 'c', 0xc0, 0x0d, 0x06, 'f', 'o', 'o', 'b', 'a', 'r', 0x00}; - domains = nm_dhcp_parse_search_list(data, 34); + domains = nm_dhcp_lease_data_parse_search_list(data, 34); g_assert(domains); g_assert_cmpint(g_strv_length(domains), ==, 4); g_assert_cmpstr(domains[0], ==, "test.example.com"); @@ -226,7 +226,7 @@ test_parse_search_list(void) 'a', 'd', }; - domains = nm_dhcp_parse_search_list(data, 4); + domains = nm_dhcp_lease_data_parse_search_list(data, 4); g_assert(!domains); data = (guint8[]){ @@ -241,7 +241,7 @@ test_parse_search_list(void) 'a', 'd', }; - domains = nm_dhcp_parse_search_list(data, 10); + domains = nm_dhcp_lease_data_parse_search_list(data, 10); g_assert(domains); g_assert_cmpint(g_strv_length(domains), ==, 1); g_assert_cmpstr(domains[0], ==, "okay"); From ce72563a8cdf41cbb357628ed1e2ee6e1c5cb9d8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 17:06:37 +0100 Subject: [PATCH 27/34] dhcp/nettools: cleanup nm_dhcp_lease_data_parse_search_list() --- src/core/dhcp/nm-dhcp-utils.c | 64 +++++++++++++++++------------------ 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-utils.c b/src/core/dhcp/nm-dhcp-utils.c index 6cddb2d5f1..0ae0dcb96f 100644 --- a/src/core/dhcp/nm-dhcp-utils.c +++ b/src/core/dhcp/nm-dhcp-utils.c @@ -10,6 +10,7 @@ #include "nm-std-aux/unaligned.h" #include "nm-glib-aux/nm-dedup-multi.h" +#include "nm-glib-aux/nm-str-buf.h" #include "systemd/nm-sd-utils-shared.h" #include "nm-dhcp-utils.h" @@ -968,7 +969,7 @@ nm_dhcp_lease_data_parse_in_addr(const guint8 *data, gsize n_data, in_addr_t *ou /*****************************************************************************/ static gboolean -lease_option_print_label(GString *str, size_t n_label, const uint8_t **datap, size_t *n_datap) +lease_option_print_label(NMStrBuf *sbuf, size_t n_label, const uint8_t **datap, size_t *n_datap) { gsize i; @@ -984,33 +985,34 @@ lease_option_print_label(GString *str, size_t n_label, const uint8_t **datap, si case '0' ... '9': case '-': case '_': - g_string_append_c(str, c); + nm_str_buf_append_c(sbuf, c); break; case '.': case '\\': - g_string_append_printf(str, "\\%c", c); + nm_str_buf_append_c2(sbuf, '\\', c); break; default: - g_string_append_printf(str, "\\%3d", c); + nm_str_buf_append_printf(sbuf, "\\%3d", c); } } return TRUE; } -static gboolean -lease_option_print_domain_name(GString * str, - const uint8_t * cache, +static char * +lease_option_print_domain_name(const uint8_t * cache, size_t * n_cachep, const uint8_t **datap, size_t * n_datap) { - const uint8_t * domain; - size_t n_domain, n_cache = *n_cachep; - const uint8_t **domainp = datap; - size_t * n_domainp = n_datap; - gboolean first = TRUE; - uint8_t c; + nm_auto_str_buf NMStrBuf sbuf = NM_STR_BUF_INIT(NM_UTILS_GET_NEXT_REALLOC_SIZE_40, FALSE); + const uint8_t * domain; + size_t n_domain; + size_t n_cache = *n_cachep; + const uint8_t ** domainp = datap; + size_t * n_domainp = n_datap; + gboolean first = TRUE; + uint8_t c; /* * We are given two adjacent memory regions. The @cache contains alreday parsed @@ -1029,11 +1031,11 @@ lease_option_print_domain_name(GString * str, * cache shrinks, so this is guaranteed to terminate. */ if (cache + n_cache != *datap) - return FALSE; + return NULL; for (;;) { if (!nm_dhcp_lease_data_consume(domainp, n_domainp, &c, sizeof(c))) - return FALSE; + return NULL; switch (c & 0xC0) { case 0x00: /* label length */ @@ -1046,16 +1048,16 @@ lease_option_print_domain_name(GString * str, * the cache to include the consumed data, and return. */ *n_cachep = *datap - cache; - return TRUE; + return nm_str_buf_finalize(&sbuf, NULL); } if (!first) - g_string_append_c(str, '.'); + nm_str_buf_append_c(&sbuf, '.'); else first = FALSE; - if (!lease_option_print_label(str, n_label, domainp, n_domainp)) - return FALSE; + if (!lease_option_print_label(&sbuf, n_label, domainp, n_domainp)) + return NULL; break; } @@ -1069,12 +1071,12 @@ lease_option_print_domain_name(GString * str, */ if (!nm_dhcp_lease_data_consume(domainp, n_domainp, &c, sizeof(c))) - return FALSE; + return NULL; offset += c; if (offset >= n_cache) - return FALSE; + return NULL; domain = cache + offset; n_domain = n_cache - offset; @@ -1086,7 +1088,7 @@ lease_option_print_domain_name(GString * str, break; } default: - return FALSE; + return NULL; } } } @@ -1099,23 +1101,21 @@ nm_dhcp_lease_data_parse_search_list(const guint8 *data, gsize n_data) gsize n_cache = 0; for (;;) { - nm_auto_free_gstring GString *domain = NULL; + gs_free char *s = NULL; - nm_gstring_prepare(&domain); - - if (!lease_option_print_domain_name(domain, cache, &n_cache, &data, &n_data)) + s = lease_option_print_domain_name(cache, &n_cache, &data, &n_data); + if (!s) break; if (!array) array = g_ptr_array_new(); - g_ptr_array_add(array, g_string_free(domain, FALSE)); - domain = NULL; + g_ptr_array_add(array, g_steal_pointer(&s)); } - if (array) { - g_ptr_array_add(array, NULL); - return (char **) g_ptr_array_free(array, FALSE); - } else + if (!array) return NULL; + + g_ptr_array_add(array, NULL); + return (char **) g_ptr_array_free(array, FALSE); } From 8366fd87b9ec7a10b552e26fa639e949ce7b9580 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 17:12:00 +0100 Subject: [PATCH 28/34] dhcp/nettools: make data pointer const --- src/core/dhcp/nm-dhcp-nettools.c | 146 ++++++++++++++----------------- src/core/dhcp/nm-dhcp-utils.h | 6 ++ 2 files changed, 73 insertions(+), 79 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 0329030fac..0a5fdc8094 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -82,6 +82,15 @@ set_error_nettools(GError **error, int r, const char *message) nm_utils_error_set(error, r, "%s (code %d)", message, r); } +static inline int +_client_lease_query(NDhcp4ClientLease *lease, + uint8_t option, + const uint8_t ** datap, + size_t * n_datap) +{ + return n_dhcp4_client_lease_query(lease, option, (guint8 **) datap, n_datap); +} + /*****************************************************************************/ #define DHCP_MAX_FQDN_LENGTH 255 @@ -89,41 +98,23 @@ set_error_nettools(GError **error, int r, const char *message) /*****************************************************************************/ static gboolean -lease_option_consume(uint8_t **datap, size_t *n_datap, void *out, size_t n_out) +lease_option_consume_route(const uint8_t **datap, + size_t * n_datap, + gboolean classless, + in_addr_t * destp, + uint8_t * plenp, + in_addr_t * gatewayp) { - if (*n_datap < n_out) - return FALSE; - - memcpy(out, *datap, n_out); - *datap += n_out; - *n_datap -= n_out; - return TRUE; -} - -static gboolean -lease_option_consume_in_addr(uint8_t **datap, size_t *n_datap, in_addr_t *addrp) -{ - return lease_option_consume(datap, n_datap, addrp, sizeof(struct in_addr)); -} - -static gboolean -lease_option_consume_route(uint8_t ** datap, - size_t * n_datap, - gboolean classless, - in_addr_t *destp, - uint8_t * plenp, - in_addr_t *gatewayp) -{ - in_addr_t dest; - in_addr_t gateway; - uint8_t * data = *datap; - size_t n_data = *n_datap; - uint8_t plen; + in_addr_t dest; + in_addr_t gateway; + const uint8_t *data = *datap; + size_t n_data = *n_datap; + uint8_t plen; if (classless) { uint8_t bytes; - if (!lease_option_consume(&data, &n_data, &plen, sizeof(plen))) + if (!nm_dhcp_lease_data_consume(&data, &n_data, &plen, sizeof(plen))) return FALSE; if (plen > 32) @@ -132,10 +123,10 @@ lease_option_consume_route(uint8_t ** datap, bytes = plen == 0 ? 0 : ((plen - 1) / 8) + 1; dest = 0; - if (!lease_option_consume(&data, &n_data, &dest, bytes)) + if (!nm_dhcp_lease_data_consume(&data, &n_data, &dest, bytes)) return FALSE; } else { - if (!lease_option_consume_in_addr(&data, &n_data, &dest)) + if (!nm_dhcp_lease_data_consume_in_addr(&data, &n_data, &dest)) return FALSE; plen = _nm_utils_ip4_get_default_prefix0(dest); @@ -145,7 +136,7 @@ lease_option_consume_route(uint8_t ** datap, dest = nm_utils_ip4_address_clear_host_address(dest, plen); - if (!lease_option_consume_in_addr(&data, &n_data, &gateway)) + if (!nm_dhcp_lease_data_consume_in_addr(&data, &n_data, &gateway)) return FALSE; *destp = dest; @@ -172,7 +163,7 @@ lease_parse_address(NDhcp4ClientLease *lease, guint32 a_lifetime; guint32 a_timestamp; guint64 a_expiry; - guint8 * l_data; + const guint8 * l_data; gsize l_data_len; int r; @@ -229,7 +220,7 @@ lease_parse_address(NDhcp4ClientLease *lease, / NM_UTILS_NSEC_PER_SEC); } - r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_SUBNET_MASK, &l_data, &l_data_len); + r = _client_lease_query(lease, NM_DHCP_OPTION_DHCP4_SUBNET_MASK, &l_data, &l_data_len); if (r != 0 || !nm_dhcp_lease_data_parse_in_addr(l_data, l_data_len, &a_netmask)) { nm_utils_error_set_literal(error, NM_UTILS_ERROR_UNKNOWN, @@ -289,11 +280,11 @@ lease_parse_address_list(NDhcp4ClientLease * lease, GHashTable * options, NMStrBuf * sbuf) { - guint8 *l_data; - gsize l_data_len; - int r; + const guint8 *l_data; + gsize l_data_len; + int r; - r = n_dhcp4_client_lease_query(lease, option, &l_data, &l_data_len); + r = _client_lease_query(lease, option, &l_data, &l_data_len); if (r != 0) return; @@ -345,23 +336,23 @@ lease_parse_routes(NDhcp4ClientLease *lease, guint32 route_metric, NMStrBuf * sbuf) { - char dest_str[NM_UTILS_INET_ADDRSTRLEN]; - char gateway_str[NM_UTILS_INET_ADDRSTRLEN]; - in_addr_t dest; - in_addr_t gateway; - uint8_t plen; - guint32 m; - gboolean has_router_from_classless = FALSE; - gboolean has_classless = FALSE; - guint32 default_route_metric = route_metric; - guint8 * l_data; - gsize l_data_len; - int r; + char dest_str[NM_UTILS_INET_ADDRSTRLEN]; + char gateway_str[NM_UTILS_INET_ADDRSTRLEN]; + in_addr_t dest; + in_addr_t gateway; + uint8_t plen; + guint32 m; + gboolean has_router_from_classless = FALSE; + gboolean has_classless = FALSE; + guint32 default_route_metric = route_metric; + const guint8 *l_data; + gsize l_data_len; + int r; - r = n_dhcp4_client_lease_query(lease, - NM_DHCP_OPTION_DHCP4_CLASSLESS_STATIC_ROUTE, - &l_data, - &l_data_len); + r = _client_lease_query(lease, + NM_DHCP_OPTION_DHCP4_CLASSLESS_STATIC_ROUTE, + &l_data, + &l_data_len); if (r == 0) { nm_str_buf_reset(sbuf); @@ -405,7 +396,7 @@ lease_parse_routes(NDhcp4ClientLease *lease, nm_str_buf_get_str(sbuf)); } - r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_STATIC_ROUTE, &l_data, &l_data_len); + r = _client_lease_query(lease, NM_DHCP_OPTION_DHCP4_STATIC_ROUTE, &l_data, &l_data_len); if (r == 0) { nm_str_buf_reset(sbuf); @@ -450,11 +441,11 @@ lease_parse_routes(NDhcp4ClientLease *lease, nm_str_buf_get_str(sbuf)); } - r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_ROUTER, &l_data, &l_data_len); + r = _client_lease_query(lease, NM_DHCP_OPTION_DHCP4_ROUTER, &l_data, &l_data_len); if (r == 0) { nm_str_buf_reset(sbuf); - while (lease_option_consume_in_addr(&l_data, &l_data_len, &gateway)) { + while (nm_dhcp_lease_data_consume_in_addr(&l_data, &l_data_len, &gateway)) { nm_str_buf_append_required_delimiter(sbuf, ' '); nm_str_buf_append(sbuf, _nm_utils_inet4_ntop(gateway, gateway_str)); @@ -533,8 +524,8 @@ lease_parse_private_options(NDhcp4ClientLease *lease, GHashTable *options) for (i = NM_DHCP_OPTION_DHCP4_PRIVATE_224; i <= NM_DHCP_OPTION_DHCP4_PRIVATE_254; i++) { gs_free char *option_string = NULL; - guint8 * data; - gsize n_data; + const guint8 *l_data; + gsize l_data_len; int r; /* We manage private options 249 (private classless static route) and 252 (wpad) in a special @@ -544,11 +535,11 @@ lease_parse_private_options(NDhcp4ClientLease *lease, GHashTable *options) NM_DHCP_OPTION_DHCP4_PRIVATE_PROXY_AUTODISCOVERY)) continue; - r = n_dhcp4_client_lease_query(lease, i, &data, &n_data); + r = _client_lease_query(lease, i, &l_data, &l_data_len); if (r) continue; - option_string = nm_utils_bin2hexstr_full(data, n_data, ':', FALSE, NULL); + option_string = nm_utils_bin2hexstr_full(l_data, l_data_len, ':', FALSE, NULL); nm_dhcp_option_take_option(options, _nm_dhcp_option_dhcp4_options, i, @@ -569,7 +560,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, nm_auto_str_buf NMStrBuf sbuf = NM_STR_BUF_INIT(0, FALSE); gs_unref_object NMIP4Config *ip4_config = NULL; gs_unref_hashtable GHashTable *options = NULL; - guint8 * l_data; + const guint8 * l_data; gsize l_data_len; const char * v_str; guint16 v_u16; @@ -594,7 +585,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, v_inaddr_s.s_addr); } - r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_BROADCAST, &l_data, &l_data_len); + r = _client_lease_query(lease, NM_DHCP_OPTION_DHCP4_BROADCAST, &l_data, &l_data_len); if (r == 0 && nm_dhcp_lease_data_parse_in_addr(l_data, l_data_len, &v_inaddr)) { nm_dhcp_option_add_option_in_addr(options, _nm_dhcp_option_dhcp4_options, @@ -610,7 +601,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, options, &sbuf); - r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_DOMAIN_NAME, &l_data, &l_data_len); + r = _client_lease_query(lease, NM_DHCP_OPTION_DHCP4_DOMAIN_NAME, &l_data, &l_data_len); if (r == 0 && nm_dhcp_lease_data_parse_cstr(l_data, l_data_len, &l_data_len)) { gs_free const char **domains = NULL; @@ -647,7 +638,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, lease_parse_search_domains(lease, ip4_config, options); - r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_INTERFACE_MTU, &l_data, &l_data_len); + r = _client_lease_query(lease, NM_DHCP_OPTION_DHCP4_INTERFACE_MTU, &l_data, &l_data_len); if (r == 0 && nm_dhcp_lease_data_parse_mtu(l_data, l_data_len, &v_u16)) { nm_dhcp_option_add_option_u64(options, _nm_dhcp_option_dhcp4_options, @@ -656,15 +647,12 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, nm_ip4_config_set_mtu(ip4_config, v_u16, NM_IP_CONFIG_SOURCE_DHCP); } - r = n_dhcp4_client_lease_query(lease, - NM_DHCP_OPTION_DHCP4_VENDOR_SPECIFIC, - &l_data, - &l_data_len); + r = _client_lease_query(lease, NM_DHCP_OPTION_DHCP4_VENDOR_SPECIFIC, &l_data, &l_data_len); v_bool = (r == 0) && memmem(l_data, l_data_len, "ANDROID_METERED", NM_STRLEN("ANDROID_METERED")); nm_ip4_config_set_metered(ip4_config, v_bool); - r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_HOST_NAME, &l_data, &l_data_len); + r = _client_lease_query(lease, NM_DHCP_OPTION_DHCP4_HOST_NAME, &l_data, &l_data_len); if (r == 0) { gs_free char *s = NULL; @@ -678,7 +666,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, lease_parse_address_list(lease, ip4_config, NM_DHCP_OPTION_DHCP4_NTP_SERVER, options, &sbuf); - r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_ROOT_PATH, &l_data, &l_data_len); + r = _client_lease_query(lease, NM_DHCP_OPTION_DHCP4_ROOT_PATH, &l_data, &l_data_len); if (r == 0 && nm_dhcp_lease_data_parse_cstr(l_data, l_data_len, &l_data_len)) { /* https://tools.ietf.org/html/rfc2132#section-3.19 * @@ -697,10 +685,10 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, } } - r = n_dhcp4_client_lease_query(lease, - NM_DHCP_OPTION_DHCP4_PRIVATE_PROXY_AUTODISCOVERY, - &l_data, - &l_data_len); + r = _client_lease_query(lease, + NM_DHCP_OPTION_DHCP4_PRIVATE_PROXY_AUTODISCOVERY, + &l_data, + &l_data_len); if (r == 0 && nm_dhcp_lease_data_parse_cstr(l_data, l_data_len, &l_data_len)) { /* https://tools.ietf.org/html/draft-ietf-wrec-wpad-01#section-4.4.1 * @@ -714,7 +702,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, l_data_len); } - r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_NIS_DOMAIN, &l_data, &l_data_len); + r = _client_lease_query(lease, NM_DHCP_OPTION_DHCP4_NIS_DOMAIN, &l_data, &l_data_len); if (r == 0 && nm_dhcp_lease_data_parse_cstr(l_data, l_data_len, &l_data_len)) { gs_free char *to_free = NULL; @@ -874,9 +862,9 @@ dhcp4_event_handle(NMDhcpNettools *self, NDhcp4ClientEvent *event) } static gboolean -dhcp4_event_cb(int fd, GIOCondition condition, gpointer data) +dhcp4_event_cb(int fd, GIOCondition condition, gpointer user_data) { - NMDhcpNettools * self = data; + NMDhcpNettools * self = user_data; NMDhcpNettoolsPrivate *priv = NM_DHCP_NETTOOLS_GET_PRIVATE(self); NDhcp4ClientEvent * event; int r; diff --git a/src/core/dhcp/nm-dhcp-utils.h b/src/core/dhcp/nm-dhcp-utils.h index 45f74b35d1..69715f90fe 100644 --- a/src/core/dhcp/nm-dhcp-utils.h +++ b/src/core/dhcp/nm-dhcp-utils.h @@ -52,6 +52,12 @@ nm_dhcp_lease_data_consume(const uint8_t **datap, size_t *n_datap, void *out, si return TRUE; } +static inline gboolean +nm_dhcp_lease_data_consume_in_addr(const uint8_t **datap, size_t *n_datap, in_addr_t *addrp) +{ + return nm_dhcp_lease_data_consume(datap, n_datap, addrp, sizeof(struct in_addr)); +} + char *nm_dhcp_lease_data_parse_domain_validate(const char *str); gboolean nm_dhcp_lease_data_parse_u16(const guint8 *data, gsize n_data, guint16 *out_val); From 4707cf5fabc99e2f985230871fb042b01a824ee8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 15:48:56 +0100 Subject: [PATCH 29/34] dhcp/nettools: cleanup lease_parse_search_domains() --- src/core/dhcp/nm-dhcp-nettools.c | 36 +++++++++++++++----------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 0a5fdc8094..8c260e02e9 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -488,33 +488,31 @@ lease_parse_routes(NDhcp4ClientLease *lease, } } -/*****************************************************************************/ - static void lease_parse_search_domains(NDhcp4ClientLease *lease, NMIP4Config *ip4_config, GHashTable *options) { - nm_auto_free_gstring GString *str = NULL; - uint8_t * data; - size_t n_data; - gs_strfreev char ** domains = NULL; - guint i; - int r; + gs_strfreev char **domains = NULL; + const guint8 * l_data; + gsize l_data_len; + guint i; + int r; - r = n_dhcp4_client_lease_query(lease, NM_DHCP_OPTION_DHCP4_DOMAIN_SEARCH_LIST, &data, &n_data); - if (r) + r = _client_lease_query(lease, NM_DHCP_OPTION_DHCP4_DOMAIN_SEARCH_LIST, &l_data, &l_data_len); + if (r != 0) return; - domains = nm_dhcp_lease_data_parse_search_list(data, n_data); - nm_gstring_prepare(&str); + domains = nm_dhcp_lease_data_parse_search_list(l_data, l_data_len); - for (i = 0; domains && domains[i]; i++) { - g_string_append(nm_gstring_add_space_delimiter(str), domains[i]); + if (!domains || !domains[0]) + return; + + for (i = 0; domains[i]; i++) nm_ip4_config_add_search(ip4_config, domains[i]); - } - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_DOMAIN_SEARCH_LIST, - str->str); + + nm_dhcp_option_take_option(options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_DOMAIN_SEARCH_LIST, + g_strjoinv(" ", domains)); } static void From 3b8882b9784e5cd75d8537aa7c357075180e02d9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 10 Feb 2021 17:33:05 +0100 Subject: [PATCH 30/34] dhcp/nettools: use NMStrBuf in lease_save() --- src/core/dhcp/nm-dhcp-nettools.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index 8c260e02e9..ee755c4b3a 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -734,25 +734,24 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, static void lease_save(NMDhcpNettools *self, NDhcp4ClientLease *lease, const char *lease_file) { - struct in_addr a_address; - nm_auto_free_gstring GString *new_contents = NULL; - char sbuf[NM_UTILS_INET_ADDRSTRLEN]; + struct in_addr a_address; + nm_auto_str_buf NMStrBuf sbuf = NM_STR_BUF_INIT(NM_UTILS_GET_NEXT_REALLOC_SIZE_104, FALSE); + char addr_str[NM_UTILS_INET_ADDRSTRLEN]; gs_free_error GError *error = NULL; nm_assert(lease); nm_assert(lease_file); - new_contents = g_string_new("# This is private data. Do not parse.\n"); - n_dhcp4_client_lease_get_yiaddr(lease, &a_address); if (a_address.s_addr == INADDR_ANY) return; - g_string_append_printf(new_contents, - "ADDRESS=%s\n", - _nm_utils_inet4_ntop(a_address.s_addr, sbuf)); + nm_str_buf_append(&sbuf, "# This is private data. Do not parse.\n"); + nm_str_buf_append_printf(&sbuf, + "ADDRESS=%s\n", + _nm_utils_inet4_ntop(a_address.s_addr, addr_str)); - if (!g_file_set_contents(lease_file, new_contents->str, -1, &error)) + if (!g_file_set_contents(lease_file, nm_str_buf_get_str_unsafe(&sbuf), sbuf.len, &error)) _LOGW("error saving lease to %s: %s", lease_file, error->message); } From 53f137af6e7549d2712f50d10e07243ab1ad8db1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 11 Feb 2021 08:25:46 +0100 Subject: [PATCH 31/34] dhcp/nettools: accept any number of trailing NULs in string options https://tools.ietf.org/html/rfc2132#section-2 says: Options containing NVT ASCII data SHOULD NOT include a trailing NULL; however, the receiver of such options MUST be prepared to delete trailing nulls if they exist. It speaks in plurals. --- src/core/dhcp/nm-dhcp-nettools.c | 2 +- src/core/dhcp/nm-dhcp-utils.c | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index ee755c4b3a..f137983316 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -690,7 +690,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, if (r == 0 && nm_dhcp_lease_data_parse_cstr(l_data, l_data_len, &l_data_len)) { /* https://tools.ietf.org/html/draft-ietf-wrec-wpad-01#section-4.4.1 * - * We reject NUL characters inside the string (except one trailing NUL). + * We reject NUL characters inside the string (except trailing NULs). * Otherwise, we allow any encoding and backslash-escape the result to * UTF-8. */ nm_dhcp_option_add_option_utf8safe_escape(options, diff --git a/src/core/dhcp/nm-dhcp-utils.c b/src/core/dhcp/nm-dhcp-utils.c index 0ae0dcb96f..646411e201 100644 --- a/src/core/dhcp/nm-dhcp-utils.c +++ b/src/core/dhcp/nm-dhcp-utils.c @@ -873,20 +873,20 @@ gboolean nm_dhcp_lease_data_parse_cstr(const guint8 *data, gsize n_data, gsize *out_new_len) { /* WARNING: this function only validates that the string does not contain - * NUL characters (and ignores one trailing NUL). It does not check character + * NUL characters (and ignores trailing NULs). It does not check character * encoding! */ + while (n_data > 0 && data[n_data - 1] == '\0') + n_data--; + if (n_data > 0) { - if (memchr(data, n_data - 1, '\0')) { - /* we accept one trailing NUL (not more). + if (memchr(data, n_data, '\0')) { + /* we accept trailing NUL, but none in between. * * https://tools.ietf.org/html/rfc2132#section-2 * https://github.com/systemd/systemd/issues/1337 */ return FALSE; } - - if (data[n_data - 1] == '\0') - n_data--; } NM_SET_OUT(out_new_len, n_data); @@ -937,7 +937,7 @@ nm_dhcp_lease_data_parse_domain(const guint8 *data, gsize n_data, char **out_val * * Its minimum length is 1. * - * Note that this is *after* we potentially stripped a trailing NUL. + * Note that this is *after* we potentially stripped trailing NULs. */ return FALSE; } From 1cbe926c2048fa375aae9629217cf21ee2ebbc42 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 11 Feb 2021 10:21:46 +0100 Subject: [PATCH 32/34] dhcp: rework DHCP options to not carry around option array Previously, we would pass around the list of options. However, - that isn't too nice to read. Also, usually when we want to treat IP address families generically, then we have an addr_family argument. Having to first resolve the addr_family to another set of variables is inconvenient. - the option array itself doesn't have enough information. For example, we don't know how many elements there are, we don't know which address family it is (unless we compare it to one of the two well known lists). For example, I'd like to do a binary search for the option. But that's not immediately possible, because the length is unknown. - in practice, there are only two address families: AF_INET and AF_INET6. It is extremely unlikely that we will require a third DHCP options list, and even if we had that, the addr_family argument still abstracts them nicely. We also don't need two different lists for one DHCP type. While that would currently be possible (and afterwards not anymore), it would be wrong to do. - also add a new accessor nm_dhcp_option_find() to find the NMDhcpOption instance by option number. --- src/core/dhcp/nm-dhcp-nettools.c | 60 +++++--------- src/core/dhcp/nm-dhcp-options.c | 110 +++++++++++++------------- src/core/dhcp/nm-dhcp-options.h | 62 +++++++++------ src/core/dhcp/nm-dhcp-systemd.c | 104 +++++++----------------- src/core/dhcp/tests/test-dhcp-utils.c | 45 +++++++++++ 5 files changed, 185 insertions(+), 196 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c index f137983316..116e1bdb24 100644 --- a/src/core/dhcp/nm-dhcp-nettools.c +++ b/src/core/dhcp/nm-dhcp-nettools.c @@ -231,30 +231,27 @@ lease_parse_address(NDhcp4ClientLease *lease, a_plen = nm_utils_ip4_netmask_to_prefix(a_netmask); nm_dhcp_option_add_option_in_addr(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_NM_IP_ADDRESS, a_address.s_addr); nm_dhcp_option_add_option_in_addr(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_SUBNET_MASK, a_netmask); nm_dhcp_option_add_option_u64(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_IP_ADDRESS_LEASE_TIME, (guint64) a_lifetime); if (a_expiry != G_MAXUINT64) { - nm_dhcp_option_add_option_u64(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_NM_EXPIRY, - a_expiry); + nm_dhcp_option_add_option_u64(options, AF_INET, NM_DHCP_OPTION_DHCP4_NM_EXPIRY, a_expiry); } n_dhcp4_client_lease_get_siaddr(lease, &a_next_server); if (a_next_server.s_addr != INADDR_ANY) { nm_dhcp_option_add_option_in_addr(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_NM_NEXT_SERVER, a_next_server.s_addr); } @@ -322,10 +319,7 @@ lease_parse_address_list(NDhcp4ClientLease * lease, } } - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - option, - nm_str_buf_get_str(sbuf)); + nm_dhcp_option_add_option(options, AF_INET, option, nm_str_buf_get_str(sbuf)); } static void @@ -391,7 +385,7 @@ lease_parse_routes(NDhcp4ClientLease *lease, } nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_CLASSLESS_STATIC_ROUTE, nm_str_buf_get_str(sbuf)); } @@ -436,7 +430,7 @@ lease_parse_routes(NDhcp4ClientLease *lease, } nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_STATIC_ROUTE, nm_str_buf_get_str(sbuf)); } @@ -482,7 +476,7 @@ lease_parse_routes(NDhcp4ClientLease *lease, } nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_ROUTER, nm_str_buf_get_str(sbuf)); } @@ -510,7 +504,7 @@ lease_parse_search_domains(NDhcp4ClientLease *lease, NMIP4Config *ip4_config, GH nm_ip4_config_add_search(ip4_config, domains[i]); nm_dhcp_option_take_option(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_DOMAIN_SEARCH_LIST, g_strjoinv(" ", domains)); } @@ -538,10 +532,7 @@ lease_parse_private_options(NDhcp4ClientLease *lease, GHashTable *options) continue; option_string = nm_utils_bin2hexstr_full(l_data, l_data_len, ':', FALSE, NULL); - nm_dhcp_option_take_option(options, - _nm_dhcp_option_dhcp4_options, - i, - g_steal_pointer(&option_string)); + nm_dhcp_option_take_option(options, AF_INET, i, g_steal_pointer(&option_string)); } } @@ -578,7 +569,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, r = n_dhcp4_client_lease_get_server_identifier(lease, &v_inaddr_s); if (r == 0) { nm_dhcp_option_add_option_in_addr(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_SERVER_ID, v_inaddr_s.s_addr); } @@ -586,7 +577,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, r = _client_lease_query(lease, NM_DHCP_OPTION_DHCP4_BROADCAST, &l_data, &l_data_len); if (r == 0 && nm_dhcp_lease_data_parse_in_addr(l_data, l_data_len, &v_inaddr)) { nm_dhcp_option_add_option_in_addr(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_BROADCAST, v_inaddr); } @@ -628,7 +619,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, if (sbuf.len > 0) { nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_DOMAIN_NAME, nm_str_buf_get_str(&sbuf)); } @@ -638,10 +629,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, r = _client_lease_query(lease, NM_DHCP_OPTION_DHCP4_INTERFACE_MTU, &l_data, &l_data_len); if (r == 0 && nm_dhcp_lease_data_parse_mtu(l_data, l_data_len, &v_u16)) { - nm_dhcp_option_add_option_u64(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_INTERFACE_MTU, - v_u16); + nm_dhcp_option_add_option_u64(options, AF_INET, NM_DHCP_OPTION_DHCP4_INTERFACE_MTU, v_u16); nm_ip4_config_set_mtu(ip4_config, v_u16, NM_IP_CONFIG_SOURCE_DHCP); } @@ -655,10 +643,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, gs_free char *s = NULL; if (nm_dhcp_lease_data_parse_domain(l_data, l_data_len, &s)) { - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_HOST_NAME, - s); + nm_dhcp_option_add_option(options, AF_INET, NM_DHCP_OPTION_DHCP4_HOST_NAME, s); } } @@ -676,7 +661,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, /* "Its minimum length is 1." */ } else { nm_dhcp_option_add_option_utf8safe_escape(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_ROOT_PATH, l_data, l_data_len); @@ -694,7 +679,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, * Otherwise, we allow any encoding and backslash-escape the result to * UTF-8. */ nm_dhcp_option_add_option_utf8safe_escape(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_PRIVATE_PROXY_AUTODISCOVERY, l_data, l_data_len); @@ -708,10 +693,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, v_str = nm_utils_buf_utf8safe_escape((char *) l_data, l_data_len, 0, &to_free); - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_NIS_DOMAIN, - v_str); + nm_dhcp_option_add_option(options, AF_INET, NM_DHCP_OPTION_DHCP4_NIS_DOMAIN, v_str); nm_ip4_config_set_nis_domain(ip4_config, v_str); } @@ -781,7 +763,7 @@ bound4_handle(NMDhcpNettools *self, NDhcp4ClientLease *lease, gboolean extended) return; } - nm_dhcp_option_add_requests_to_options(options, _nm_dhcp_option_dhcp4_options); + nm_dhcp_option_add_requests_to_options(options, AF_INET); lease_save(self, lease, priv->lease_file); nm_dhcp_client_set_state(NM_DHCP_CLIENT(self), @@ -1107,7 +1089,7 @@ ip4_start(NMDhcpClient *client, } /* Add requested options */ - for (i = 0; _nm_dhcp_option_dhcp4_options[i].name; i++) { + for (i = 0; i < (int) G_N_ELEMENTS(_nm_dhcp_option_dhcp4_options); i++) { if (_nm_dhcp_option_dhcp4_options[i].include) { nm_assert(_nm_dhcp_option_dhcp4_options[i].option_num <= 255); n_dhcp4_client_probe_config_request_option(config, diff --git a/src/core/dhcp/nm-dhcp-options.c b/src/core/dhcp/nm-dhcp-options.c index e03fa23192..a8b27ddb8d 100644 --- a/src/core/dhcp/nm-dhcp-options.c +++ b/src/core/dhcp/nm-dhcp-options.c @@ -7,11 +7,11 @@ #include "nm-dhcp-options.h" -#define REQPREFIX "requested_" +/*****************************************************************************/ -#define REQ(_num, _name, _include) \ - { \ - .name = REQPREFIX ""_name, .option_num = _num, .include = _include, \ +#define REQ(_num, _name, _include) \ + { \ + .name = NM_DHCP_OPTION_REQPREFIX ""_name, .option_num = _num, .include = _include, \ } const NMDhcpOption _nm_dhcp_option_dhcp4_options[] = { @@ -167,8 +167,7 @@ const NMDhcpOption _nm_dhcp_option_dhcp4_options[] = { REQ(NM_DHCP_OPTION_DHCP4_NM_IP_ADDRESS, "ip_address", FALSE), REQ(NM_DHCP_OPTION_DHCP4_NM_EXPIRY, "expiry", FALSE), REQ(NM_DHCP_OPTION_DHCP4_NM_NEXT_SERVER, "next_server", FALSE), - - {0}}; +}; const NMDhcpOption _nm_dhcp_option_dhcp6_options[] = { REQ(NM_DHCP_OPTION_DHCP6_CLIENTID, "dhcp6_client_id", FALSE), @@ -194,103 +193,102 @@ const NMDhcpOption _nm_dhcp_option_dhcp6_options[] = { REQ(NM_DHCP_OPTION_DHCP6_NM_RENEW, "renew", FALSE), REQ(NM_DHCP_OPTION_DHCP6_NM_REBIND, "rebind", FALSE), REQ(NM_DHCP_OPTION_DHCP6_NM_IAID, "iaid", FALSE), +}; - {0}}; +#undef REQ -const char * -nm_dhcp_option_request_string(const NMDhcpOption *requests, guint option) +const NMDhcpOption * +nm_dhcp_option_find(int addr_family, guint option) { - guint i = 0; + const int IS_IPv4 = NM_IS_IPv4(addr_family); + const NMDhcpOption *const options = + IS_IPv4 ? _nm_dhcp_option_dhcp4_options : _nm_dhcp_option_dhcp6_options; + int n_options = IS_IPv4 ? G_N_ELEMENTS(_nm_dhcp_option_dhcp4_options) + : G_N_ELEMENTS(_nm_dhcp_option_dhcp6_options); + int i; - while (requests[i].name) { - if (requests[i].option_num == option) - return requests[i].name + NM_STRLEN(REQPREFIX); - i++; + for (i = 0; i < n_options; i++) { + const NMDhcpOption *opt = &options[i]; + + if (opt->option_num == option) + return opt; } /* Option should always be found */ - nm_assert_not_reached(); - return NULL; + return nm_assert_unreachable_val(NULL); } void -nm_dhcp_option_take_option(GHashTable * options, - const NMDhcpOption *requests, - guint option, - char * value) +nm_dhcp_option_take_option(GHashTable *options, int addr_family, guint option, char *value) { + nm_assert_addr_family(addr_family); + nm_assert(value); + nm_assert(g_utf8_validate(value, -1, NULL)); + if (!options) { nm_assert_not_reached(); g_free(value); return; } - nm_assert(requests); - nm_assert(value); - nm_assert(g_utf8_validate(value, -1, NULL)); - - g_hash_table_insert(options, (gpointer) nm_dhcp_option_request_string(requests, option), value); + g_hash_table_insert(options, + (gpointer) nm_dhcp_option_request_string(addr_family, option), + value); } void -nm_dhcp_option_add_option(GHashTable * options, - const NMDhcpOption *requests, - guint option, - const char * value) +nm_dhcp_option_add_option(GHashTable *options, int addr_family, guint option, const char *value) { - nm_dhcp_option_take_option(options, requests, option, g_strdup(value)); + nm_dhcp_option_take_option(options, addr_family, option, g_strdup(value)); } void -nm_dhcp_option_add_option_utf8safe_escape(GHashTable * options, - const NMDhcpOption *requests, - guint option, - const guint8 * data, - gsize n_data) +nm_dhcp_option_add_option_utf8safe_escape(GHashTable * options, + int addr_family, + guint option, + const guint8 *data, + gsize n_data) { gs_free char *to_free = NULL; const char * escaped; escaped = nm_utils_buf_utf8safe_escape((char *) data, n_data, 0, &to_free); - nm_dhcp_option_add_option(options, requests, option, escaped ?: ""); + nm_dhcp_option_add_option(options, addr_family, option, escaped ?: ""); } void -nm_dhcp_option_add_option_u64(GHashTable * options, - const NMDhcpOption *requests, - guint option, - guint64 value) +nm_dhcp_option_add_option_u64(GHashTable *options, int addr_family, guint option, guint64 value) { nm_dhcp_option_take_option(options, - requests, + addr_family, option, g_strdup_printf("%" G_GUINT64_FORMAT, value)); } void -nm_dhcp_option_add_option_in_addr(GHashTable * options, - const NMDhcpOption *requests, - guint option, - in_addr_t value) +nm_dhcp_option_add_option_in_addr(GHashTable *options, + int addr_family, + guint option, + in_addr_t value) { char sbuf[NM_UTILS_INET_ADDRSTRLEN]; - nm_dhcp_option_add_option(options, requests, option, _nm_utils_inet4_ntop(value, sbuf)); + nm_dhcp_option_add_option(options, addr_family, option, _nm_utils_inet4_ntop(value, sbuf)); } void -nm_dhcp_option_add_requests_to_options(GHashTable *options, const NMDhcpOption *requests) +nm_dhcp_option_add_requests_to_options(GHashTable *options, int addr_family) { - guint i; + const int IS_IPv4 = NM_IS_IPv4(addr_family); + const NMDhcpOption *const all_options = + IS_IPv4 ? _nm_dhcp_option_dhcp4_options : _nm_dhcp_option_dhcp6_options; + int n_options = IS_IPv4 ? G_N_ELEMENTS(_nm_dhcp_option_dhcp4_options) + : G_N_ELEMENTS(_nm_dhcp_option_dhcp6_options); + int i; - if (!options) { - nm_assert_not_reached(); - return; - } - - for (i = 0; requests[i].name; i++) { - if (requests[i].include) - g_hash_table_insert(options, (gpointer) requests[i].name, g_strdup("1")); + for (i = 0; i < n_options; i++) { + if (all_options[i].include) + g_hash_table_insert(options, (gpointer) all_options[i].name, g_strdup("1")); } } diff --git a/src/core/dhcp/nm-dhcp-options.h b/src/core/dhcp/nm-dhcp-options.h index 2ebf339ed7..585f118795 100644 --- a/src/core/dhcp/nm-dhcp-options.h +++ b/src/core/dhcp/nm-dhcp-options.h @@ -177,38 +177,50 @@ typedef enum { } NMDhcpOptionDhcp6Options; +#define NM_DHCP_OPTION_REQPREFIX "requested_" + typedef struct { const char *name; uint16_t option_num; bool include; } NMDhcpOption; -extern const NMDhcpOption _nm_dhcp_option_dhcp4_options[]; -extern const NMDhcpOption _nm_dhcp_option_dhcp6_options[]; +extern const NMDhcpOption _nm_dhcp_option_dhcp4_options[142]; +extern const NMDhcpOption _nm_dhcp_option_dhcp6_options[16]; -const char *nm_dhcp_option_request_string(const NMDhcpOption *requests, guint option); -void nm_dhcp_option_take_option(GHashTable * options, - const NMDhcpOption *requests, - guint option, - char * value); -void nm_dhcp_option_add_option(GHashTable * options, - const NMDhcpOption *requests, - guint option, - const char * value); -void nm_dhcp_option_add_option_utf8safe_escape(GHashTable * options, - const NMDhcpOption *requests, - guint option, - const guint8 * data, - gsize n_data); -void nm_dhcp_option_add_option_in_addr(GHashTable * options, - const NMDhcpOption *requests, - guint option, - in_addr_t value); -void nm_dhcp_option_add_option_u64(GHashTable * options, - const NMDhcpOption *requests, - guint option, - guint64 value); -void nm_dhcp_option_add_requests_to_options(GHashTable *options, const NMDhcpOption *requests); +static inline const char * +nm_dhcp_option_get_name(const NMDhcpOption *option) +{ + nm_assert(option); + nm_assert(option->name); + nm_assert(NM_STR_HAS_PREFIX(option->name, NM_DHCP_OPTION_REQPREFIX)); + + return &option->name[NM_STRLEN(NM_DHCP_OPTION_REQPREFIX)]; +} + +const NMDhcpOption *nm_dhcp_option_find(int addr_family, guint option); + +static inline const char * +nm_dhcp_option_request_string(int addr_family, guint option) +{ + return nm_dhcp_option_get_name(nm_dhcp_option_find(addr_family, option)); +} + +void nm_dhcp_option_take_option(GHashTable *options, int addr_family, guint option, char *value); +void +nm_dhcp_option_add_option(GHashTable *options, int addr_family, guint option, const char *value); +void nm_dhcp_option_add_option_utf8safe_escape(GHashTable * options, + int addr_family, + guint option, + const guint8 *data, + gsize n_data); +void nm_dhcp_option_add_option_in_addr(GHashTable *options, + int addr_family, + guint option, + in_addr_t value); +void +nm_dhcp_option_add_option_u64(GHashTable *options, int addr_family, guint option, guint64 value); +void nm_dhcp_option_add_requests_to_options(GHashTable *options, int addr_family); GHashTable *nm_dhcp_option_create_options_dict(void); #endif /* __NM_DHCP_OPTIONS_H__ */ diff --git a/src/core/dhcp/nm-dhcp-systemd.c b/src/core/dhcp/nm-dhcp-systemd.c index d558a62628..b92a9073fa 100644 --- a/src/core/dhcp/nm-dhcp-systemd.c +++ b/src/core/dhcp/nm-dhcp-systemd.c @@ -138,32 +138,26 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, options = out_options ? nm_dhcp_option_create_options_dict() : NULL; _nm_utils_inet4_ntop(a_address.s_addr, addr_str); - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_NM_IP_ADDRESS, - addr_str); + nm_dhcp_option_add_option(options, AF_INET, NM_DHCP_OPTION_DHCP4_NM_IP_ADDRESS, addr_str); a_plen = nm_utils_ip4_netmask_to_prefix(a_netmask.s_addr); nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_SUBNET_MASK, _nm_utils_inet4_ntop(a_netmask.s_addr, addr_str)); nm_dhcp_option_add_option_u64(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_IP_ADDRESS_LEASE_TIME, a_lifetime); nm_dhcp_option_add_option_u64(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_NM_EXPIRY, (guint64)(ts_time + a_lifetime)); if (sd_dhcp_lease_get_next_server(lease, &a_next_server) == 0) { _nm_utils_inet4_ntop(a_next_server.s_addr, addr_str); - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_NM_NEXT_SERVER, - addr_str); + nm_dhcp_option_add_option(options, AF_INET, NM_DHCP_OPTION_DHCP4_NM_NEXT_SERVER, addr_str); } nm_ip4_config_add_address(ip4_config, @@ -179,18 +173,12 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, if (sd_dhcp_lease_get_server_identifier(lease, &server_id) >= 0) { _nm_utils_inet4_ntop(server_id.s_addr, addr_str); - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_SERVER_ID, - addr_str); + nm_dhcp_option_add_option(options, AF_INET, NM_DHCP_OPTION_DHCP4_SERVER_ID, addr_str); } if (sd_dhcp_lease_get_broadcast(lease, &broadcast) >= 0) { _nm_utils_inet4_ntop(broadcast.s_addr, addr_str); - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_BROADCAST, - addr_str); + nm_dhcp_option_add_option(options, AF_INET, NM_DHCP_OPTION_DHCP4_BROADCAST, addr_str); } num = sd_dhcp_lease_get_dns(lease, &addr_list); @@ -208,7 +196,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, nm_ip4_config_add_nameserver(ip4_config, addr_list[i].s_addr); } nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_DOMAIN_NAME_SERVER, str->str); } @@ -221,7 +209,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, nm_ip4_config_add_search(ip4_config, search_domains[i]); } nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_DOMAIN_SEARCH_LIST, str->str); } @@ -230,10 +218,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, gs_strfreev char **domains = NULL; char ** d; - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_DOMAIN_NAME, - s); + nm_dhcp_option_add_option(options, AF_INET, NM_DHCP_OPTION_DHCP4_DOMAIN_NAME, s); /* Multiple domains sometimes stuffed into option 15 "Domain Name". * As systemd escapes such characters, split them at \\032. */ @@ -243,10 +228,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, } if (sd_dhcp_lease_get_hostname(lease, &s) >= 0) { - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_HOST_NAME, - s); + nm_dhcp_option_add_option(options, AF_INET, NM_DHCP_OPTION_DHCP4_HOST_NAME, s); } num = sd_dhcp_lease_get_routes(lease, &routes); @@ -348,12 +330,12 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, if (str_classless && str_classless->len > 0) nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_CLASSLESS_STATIC_ROUTE, str_classless->str); if (str_static && str_static->len > 0) nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_STATIC_ROUTE, str_static->str); } @@ -400,17 +382,11 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, }), NULL); } - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_ROUTER, - str->str); + nm_dhcp_option_add_option(options, AF_INET, NM_DHCP_OPTION_DHCP4_ROUTER, str->str); } if (sd_dhcp_lease_get_mtu(lease, &mtu) >= 0 && mtu) { - nm_dhcp_option_add_option_u64(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_INTERFACE_MTU, - mtu); + nm_dhcp_option_add_option_u64(options, AF_INET, NM_DHCP_OPTION_DHCP4_INTERFACE_MTU, mtu); nm_ip4_config_set_mtu(ip4_config, mtu, NM_IP_CONFIG_SOURCE_DHCP); } @@ -421,38 +397,29 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, _nm_utils_inet4_ntop(addr_list[i].s_addr, addr_str); g_string_append(nm_gstring_add_space_delimiter(str), addr_str); } - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_NTP_SERVER, - str->str); + nm_dhcp_option_add_option(options, AF_INET, NM_DHCP_OPTION_DHCP4_NTP_SERVER, str->str); } if (sd_dhcp_lease_get_root_path(lease, &s) >= 0) { - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_ROOT_PATH, - s); + nm_dhcp_option_add_option(options, AF_INET, NM_DHCP_OPTION_DHCP4_ROOT_PATH, s); } if (sd_dhcp_lease_get_t1(lease, &renewal) >= 0) { nm_dhcp_option_add_option_u64(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_RENEWAL_T1_TIME, renewal); } if (sd_dhcp_lease_get_t2(lease, &rebinding) >= 0) { nm_dhcp_option_add_option_u64(options, - _nm_dhcp_option_dhcp4_options, + AF_INET, NM_DHCP_OPTION_DHCP4_REBINDING_T2_TIME, rebinding); } if (sd_dhcp_lease_get_timezone(lease, &s) >= 0) { - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_NEW_TZDB_TIMEZONE, - s); + nm_dhcp_option_add_option(options, AF_INET, NM_DHCP_OPTION_DHCP4_NEW_TZDB_TIMEZONE, s); } if (sd_dhcp_lease_get_vendor_specific(lease, &data, &data_len) >= 0) @@ -473,10 +440,7 @@ lease_to_ip4_config(NMDedupMultiIndex *multi_idx, g_free(option_string); continue; } - nm_dhcp_option_take_option(options, - _nm_dhcp_option_dhcp4_options, - private_options[i].code, - option_string); + nm_dhcp_option_take_option(options, AF_INET, private_options[i].code, option_string); } } NM_SET_OUT(out_options, g_steal_pointer(&options)); @@ -518,7 +482,7 @@ bound4_handle(NMDhcpSystemd *self, gboolean extended) return; } - nm_dhcp_option_add_requests_to_options(options, _nm_dhcp_option_dhcp4_options); + nm_dhcp_option_add_requests_to_options(options, AF_INET); dhcp_lease_save(lease, priv->lease_file); nm_dhcp_client_set_state(NM_DHCP_CLIENT(self), @@ -702,7 +666,7 @@ ip4_start(NMDhcpClient *client, } /* Add requested options */ - for (i = 0; _nm_dhcp_option_dhcp4_options[i].name; i++) { + for (i = 0; i < (int) G_N_ELEMENTS(_nm_dhcp_option_dhcp4_options); i++) { if (_nm_dhcp_option_dhcp4_options[i].include) { nm_assert(_nm_dhcp_option_dhcp4_options[i].option_num <= 255); r = sd_dhcp_client_set_request_option(sd_client, @@ -821,10 +785,7 @@ lease_to_ip6_config(NMDedupMultiIndex *multi_idx, g_string_append(nm_gstring_add_space_delimiter(str), addr_str); }; if (str->len) - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp6_options, - NM_DHCP_OPTION_DHCP6_NM_IP_ADDRESS, - str->str); + nm_dhcp_option_add_option(options, AF_INET6, NM_DHCP_OPTION_DHCP6_NM_IP_ADDRESS, str->str); if (!info_only && nm_ip6_config_get_num_addresses(ip6_config) == 0) { g_set_error_literal(error, @@ -842,10 +803,7 @@ lease_to_ip6_config(NMDedupMultiIndex *multi_idx, g_string_append(nm_gstring_add_space_delimiter(str), addr_str); nm_ip6_config_add_nameserver(ip6_config, &dns[i]); } - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp6_options, - NM_DHCP_OPTION_DHCP6_DNS_SERVERS, - str->str); + nm_dhcp_option_add_option(options, AF_INET6, NM_DHCP_OPTION_DHCP6_DNS_SERVERS, str->str); } num = sd_dhcp6_lease_get_domains(lease, &domains); @@ -855,17 +813,11 @@ lease_to_ip6_config(NMDedupMultiIndex *multi_idx, g_string_append(nm_gstring_add_space_delimiter(str), domains[i]); nm_ip6_config_add_search(ip6_config, domains[i]); } - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp6_options, - NM_DHCP_OPTION_DHCP6_DOMAIN_LIST, - str->str); + nm_dhcp_option_add_option(options, AF_INET6, NM_DHCP_OPTION_DHCP6_DOMAIN_LIST, str->str); } if (sd_dhcp6_lease_get_fqdn(lease, &s) >= 0) { - nm_dhcp_option_add_option(options, - _nm_dhcp_option_dhcp6_options, - NM_DHCP_OPTION_DHCP6_FQDN, - s); + nm_dhcp_option_add_option(options, AF_INET6, NM_DHCP_OPTION_DHCP6_FQDN, s); } NM_SET_OUT(out_options, g_steal_pointer(&options)); @@ -1020,7 +972,7 @@ ip6_start(NMDhcpClient * client, } /* Add requested options */ - for (i = 0; _nm_dhcp_option_dhcp6_options[i].name; i++) { + for (i = 0; i < (int) G_N_ELEMENTS(_nm_dhcp_option_dhcp6_options); i++) { if (_nm_dhcp_option_dhcp6_options[i].include) { r = sd_dhcp6_client_set_request_option(sd_client, _nm_dhcp_option_dhcp6_options[i].option_num); diff --git a/src/core/dhcp/tests/test-dhcp-utils.c b/src/core/dhcp/tests/test-dhcp-utils.c index a45e7f118f..9b54e2cd02 100644 --- a/src/core/dhcp/tests/test-dhcp-utils.c +++ b/src/core/dhcp/tests/test-dhcp-utils.c @@ -13,10 +13,13 @@ #include "nm-utils.h" #include "dhcp/nm-dhcp-utils.h" +#include "dhcp/nm-dhcp-options.h" #include "platform/nm-platform.h" #include "nm-test-utils-core.h" +/*****************************************************************************/ + static NMIP4Config * _ip4_config_from_options(int ifindex, const char *iface, GHashTable *options, guint32 route_metric) { @@ -740,6 +743,46 @@ test_client_id_from_string(void) COMPARE_ID(endcolon, TRUE, endcolon, strlen(endcolon)); } +/*****************************************************************************/ + +static void +test_dhcp_opt_list(gconstpointer test_data) +{ + const gboolean IS_IPv4 = (GPOINTER_TO_INT(test_data) == 0); + const int addr_family = IS_IPv4 ? AF_INET : AF_INET6; + const NMDhcpOption *const options = + IS_IPv4 ? _nm_dhcp_option_dhcp4_options : _nm_dhcp_option_dhcp6_options; + const guint n = (IS_IPv4 ? G_N_ELEMENTS(_nm_dhcp_option_dhcp4_options) + : G_N_ELEMENTS(_nm_dhcp_option_dhcp6_options)); + guint i; + guint j; + + g_assert(options); + g_assert(n > 0); + + for (i = 0; i < n; i++) { + const NMDhcpOption *const opt = &options[i]; + + g_assert_cmpstr(opt->name, !=, NULL); + g_assert(NM_STR_HAS_PREFIX(opt->name, NM_DHCP_OPTION_REQPREFIX)); + + for (j = 0; j < i; j++) { + const NMDhcpOption *const opt2 = &options[j]; + + g_assert_cmpstr(opt->name, !=, opt2->name); + g_assert_cmpint(opt->option_num, !=, opt2->option_num); + } + } + + for (i = 0; i < n; i++) { + const NMDhcpOption *const opt = &options[i]; + + g_assert(opt == nm_dhcp_option_find(addr_family, opt->option_num)); + } +} + +/*****************************************************************************/ + NMTST_DEFINE(); int @@ -776,6 +819,8 @@ main(int argc, char **argv) g_test_add_func("/dhcp/client-id-from-string", test_client_id_from_string); g_test_add_func("/dhcp/vendor-option-metered", test_vendor_option_metered); g_test_add_func("/dhcp/parse-search-list", test_parse_search_list); + g_test_add_data_func("/dhcp/test_dhcp_opt_list/IPv4", GINT_TO_POINTER(0), test_dhcp_opt_list); + g_test_add_data_func("/dhcp/test_dhcp_opt_list/IPv6", GINT_TO_POINTER(1), test_dhcp_opt_list); return g_test_run(); } From 24abf13239446d01e414e476bff2c78f64121d35 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 11 Feb 2021 11:36:44 +0100 Subject: [PATCH 33/34] dhcp: binary search in nm_dhcp_option_find() Let's use binary search. Test patch: diff --git a/src/core/dhcp/tests/test-dhcp-utils.c b/src/core/dhcp/tests/test-dhcp-utils.c index 9b54e2cd0228..007993341672 100644 --- a/src/core/dhcp/tests/test-dhcp-utils.c +++ b/src/core/dhcp/tests/test-dhcp-utils.c @@ -788,6 +788,24 @@ NMTST_DEFINE(); int main(int argc, char **argv) { + int i; + guint idx; + guint c; + + idx = 0; + c = 0; + for (i = 0; i < 1000000; i++) { + const guint option = _nm_dhcp_option_dhcp4_options[idx % G_N_ELEMENTS(_nm_dhcp_option_dhcp4_options)].option_num; + + idx += 2010055757; + + if (nm_dhcp_option_find(AF_INET, option)->name) + c++; + } + g_print(">%u\n", c); + + return 0; + nmtst_init_assert_logging(&argc, &argv, "WARN", "DEFAULT"); g_test_add_func("/dhcp/generic-options", test_generic_options); Build: CFLAGS='-O2' ./autogen.sh --with-more-asserts=0 make -j 10 src/core/dhcp/tests/test-dhcp-utils && \ src/core/dhcp/tests/test-dhcp-utils && \ perf stat -r 200 -B src/core/dhcp/tests/test-dhcp-utils Before: Performance counter stats for 'src/core/dhcp/tests/test-dhcp-utils' (200 runs): 82.83 msec task-clock:u # 0.994 CPUs utilized ( +- 0.21% ) 0 context-switches:u # 0.000 K/sec 0 cpu-migrations:u # 0.000 K/sec 579 page-faults:u # 0.007 M/sec ( +- 0.03% ) 264,676,245 cycles:u # 3.195 GHz ( +- 0.06% ) 544,792,266 instructions:u # 2.06 insn per cycle ( +- 0.00% ) 151,624,848 branches:u # 1830.472 M/sec ( +- 0.00% ) 1,083,780 branch-misses:u # 0.71% of all branches ( +- 0.01% ) 0.083328 +- 0.000178 seconds time elapsed ( +- 0.21% ) After: Performance counter stats for 'src/core/dhcp/tests/test-dhcp-utils' (200 runs): 39.21 msec task-clock:u # 0.987 CPUs utilized ( +- 0.57% ) 0 context-switches:u # 0.000 K/sec 0 cpu-migrations:u # 0.000 K/sec 579 page-faults:u # 0.015 M/sec ( +- 0.03% ) 115,396,123 cycles:u # 2.943 GHz ( +- 0.23% ) 137,664,630 instructions:u # 1.19 insn per cycle ( +- 0.00% ) 25,866,025 branches:u # 659.597 M/sec ( +- 0.00% ) 1,919,616 branch-misses:u # 7.42% of all branches ( +- 0.12% ) 0.039717 +- 0.000227 seconds time elapsed ( +- 0.57% ) --- src/core/dhcp/nm-dhcp-options.c | 172 ++++++++++++++++++++++++++++++-- 1 file changed, 162 insertions(+), 10 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-options.c b/src/core/dhcp/nm-dhcp-options.c index a8b27ddb8d..3537cd147e 100644 --- a/src/core/dhcp/nm-dhcp-options.c +++ b/src/core/dhcp/nm-dhcp-options.c @@ -7,6 +7,8 @@ #include "nm-dhcp-options.h" +#include "nm-glib-aux/nm-str-buf.h" + /*****************************************************************************/ #define REQ(_num, _name, _include) \ @@ -169,6 +171,23 @@ const NMDhcpOption _nm_dhcp_option_dhcp4_options[] = { REQ(NM_DHCP_OPTION_DHCP4_NM_NEXT_SERVER, "next_server", FALSE), }; +static const NMDhcpOption *const _sorted_options_4[G_N_ELEMENTS(_nm_dhcp_option_dhcp4_options)] = { +#define A(idx) (&_nm_dhcp_option_dhcp4_options[(idx)]) + A(0), A(1), A(8), A(18), A(19), A(2), A(20), A(21), A(22), A(23), A(24), A(3), + A(25), A(26), A(4), A(27), A(17), A(28), A(29), A(30), A(31), A(32), A(33), A(34), + A(35), A(5), A(36), A(6), A(37), A(38), A(39), A(40), A(9), A(41), A(42), A(43), + A(44), A(45), A(46), A(10), A(11), A(12), A(47), A(48), A(49), A(50), A(51), A(52), + A(13), A(53), A(54), A(55), A(57), A(58), A(59), A(60), A(61), A(62), A(63), A(64), + A(65), A(66), A(67), A(68), A(69), A(70), A(71), A(72), A(73), A(74), A(75), A(76), + A(77), A(78), A(79), A(80), A(81), A(82), A(83), A(84), A(85), A(86), A(87), A(56), + A(88), A(89), A(90), A(91), A(92), A(93), A(14), A(7), A(94), A(95), A(96), A(97), + A(98), A(99), A(100), A(101), A(102), A(103), A(104), A(105), A(106), A(107), A(108), A(109), + A(110), A(111), A(112), A(113), A(114), A(115), A(116), A(117), A(118), A(119), A(120), A(121), + A(122), A(123), A(124), A(125), A(126), A(127), A(128), A(129), A(130), A(131), A(132), A(133), + A(134), A(15), A(135), A(136), A(16), A(137), A(138), A(139), A(140), A(141), +#undef A +}; + const NMDhcpOption _nm_dhcp_option_dhcp6_options[] = { REQ(NM_DHCP_OPTION_DHCP6_CLIENTID, "dhcp6_client_id", FALSE), @@ -197,27 +216,160 @@ const NMDhcpOption _nm_dhcp_option_dhcp6_options[] = { #undef REQ +static const NMDhcpOption *const _sorted_options_6[G_N_ELEMENTS(_nm_dhcp_option_dhcp6_options)] = { +#define A(idx) (&_nm_dhcp_option_dhcp6_options[(idx)]) + A(0), + A(1), + A(2), + A(3), + A(4), + A(5), + A(6), + A(7), + A(8), + A(9), + A(10), + A(11), + A(12), + A(13), + A(14), + A(15), +#undef A +}; + +/*****************************************************************************/ + +static int +_sorted_options_generate_sort(gconstpointer pa, gconstpointer pb, gpointer user_data) +{ + const NMDhcpOption *const *a = pa; + const NMDhcpOption *const *b = pb; + + NM_CMP_DIRECT((*a)->option_num, (*b)->option_num); + return nm_assert_unreachable_val(0); +} + +static char * +_sorted_options_generate(const NMDhcpOption *base, const NMDhcpOption *const *sorted, guint n) +{ + gs_free const NMDhcpOption **sort2 = NULL; + NMStrBuf sbuf = NM_STR_BUF_INIT(0, FALSE); + guint i; + + sort2 = nm_memdup(sorted, n * sizeof(sorted[0])); + + g_qsort_with_data(sort2, n, sizeof(sort2[0]), _sorted_options_generate_sort, NULL); + + for (i = 0; i < n; i++) { + if (i > 0) + nm_str_buf_append(&sbuf, ", "); + nm_str_buf_append_printf(&sbuf, "A(%d)", (int) (sort2[i] - base)); + } + + return nm_str_buf_finalize(&sbuf, NULL); +} + +_nm_unused static void +_ASSERT_sorted(int IS_IPv4, const NMDhcpOption *const *const sorted, int n) + +{ + const NMDhcpOption *const options = + IS_IPv4 ? _nm_dhcp_option_dhcp4_options : _nm_dhcp_option_dhcp6_options; + int i; + int j; + gs_free char *sorted_msg = NULL; + + for (i = 0; i < n; i++) { + const NMDhcpOption *opt = sorted[i]; + + g_assert(opt); + g_assert(opt >= options); + g_assert(opt < &options[n]); + + for (j = 0; j < i; j++) { + const NMDhcpOption *opt2 = sorted[j]; + + if (opt == opt2) { + g_error("%s:%d: the _sorted_options_%c at [%d] (opt=%u, %s) is duplicated at " + "[%d] (SORT: %s)", + __FILE__, + __LINE__, + IS_IPv4 ? '4' : '6', + i, + opt->option_num, + opt->name, + j, + (sorted_msg = _sorted_options_generate(options, sorted, n))); + } + } + + if (i > 0) { + const NMDhcpOption *opt2 = sorted[i - 1]; + + if (opt2->option_num >= opt->option_num) { + g_error("%s:%d: the _sorted_options_%c at [%d] (opt=%u, %s) should come before " + "[%d] (opt=%u, %s) (SORT: %s)", + __FILE__, + __LINE__, + IS_IPv4 ? '4' : '6', + i, + opt->option_num, + opt->name, + i - 1, + opt2->option_num, + opt2->name, + (sorted_msg = _sorted_options_generate(options, sorted, n))); + } + } + } +} + +/*****************************************************************************/ + const NMDhcpOption * nm_dhcp_option_find(int addr_family, guint option) { - const int IS_IPv4 = NM_IS_IPv4(addr_family); - const NMDhcpOption *const options = - IS_IPv4 ? _nm_dhcp_option_dhcp4_options : _nm_dhcp_option_dhcp6_options; - int n_options = IS_IPv4 ? G_N_ELEMENTS(_nm_dhcp_option_dhcp4_options) - : G_N_ELEMENTS(_nm_dhcp_option_dhcp6_options); - int i; + const int IS_IPv4 = NM_IS_IPv4(addr_family); + const NMDhcpOption *const *const sorted = IS_IPv4 ? _sorted_options_4 : _sorted_options_6; + const int n = IS_IPv4 ? G_N_ELEMENTS(_nm_dhcp_option_dhcp4_options) + : G_N_ELEMENTS(_nm_dhcp_option_dhcp6_options); + int imin = 0; + int imax = n - 1; + int imid = (n - 1) / 2; - for (i = 0; i < n_options; i++) { - const NMDhcpOption *opt = &options[i]; +#if NM_MORE_ASSERTS > 10 + nm_assert(n < G_MAXINT / 2); + if (IS_IPv4 && !NM_MORE_ASSERT_ONCE(10)) { + /* already checked */ + } else if (!IS_IPv4 && !NM_MORE_ASSERT_ONCE(10)) { + /* already checked */ + } else + _ASSERT_sorted(IS_IPv4, sorted, n); +#endif - if (opt->option_num == option) - return opt; + for (;;) { + const guint o = sorted[imid]->option_num; + + if (G_UNLIKELY(o == option)) + return sorted[imid]; + + if (o < option) + imin = imid + 1; + else + imax = imid - 1; + + if (G_UNLIKELY(imin > imax)) + break; + + imid = (imin + imax) / 2; } /* Option should always be found */ return nm_assert_unreachable_val(NULL); } +/*****************************************************************************/ + void nm_dhcp_option_take_option(GHashTable *options, int addr_family, guint option, char *value) { From 4b874019addeac0b1f88d7cb9f46b9021c540563 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 11 Feb 2021 09:33:45 +0100 Subject: [PATCH 34/34] dhcp: downgrade logging messages for DHCP to Granted, for debugging this information is useful. However, to actually debug an issue thoroughly, level=TRACE is anyway required. There is simply no way how we can log useful debug information and not flood logging messages for regular use. For example, logging the DHCP lease options can easily print 30 lines. And this, every time you get a lease update (e.g. every 30 minutes) and for every interface that does DHCP. It's simply too verbose. Downgrade the logging level. Yes, now our default level is even less useful to understand what is going on. But the majority of time, users don't care so not spamming the log is more important. However, we still log the DHCP event (and the IP address) with level. --- src/core/dhcp/nm-dhcp-client.c | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-client.c b/src/core/dhcp/nm-dhcp-client.c index f5047244ed..c38c814ead 100644 --- a/src/core/dhcp/nm-dhcp-client.c +++ b/src/core/dhcp/nm-dhcp-client.c @@ -20,6 +20,7 @@ #include "NetworkManagerUtils.h" #include "nm-utils.h" #include "nm-dhcp-utils.h" +#include "nm-dhcp-options.h" #include "platform/nm-platform.h" #include "nm-dhcp-client-logging.h" @@ -437,8 +438,7 @@ nm_dhcp_client_set_state(NMDhcpClient *self, NMIPConfig * ip_config, GHashTable * options) { - NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE(self); - gs_free char * event_id = NULL; + NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE(self); if (NM_IN_SET(new_state, NM_DHCP_STATE_BOUND, NM_DHCP_STATE_EXTENDED)) { g_return_if_fail(NM_IS_IP_CONFIG_ADDR_FAMILY(ip_config, priv->addr_family)); @@ -462,23 +462,36 @@ nm_dhcp_client_set_state(NMDhcpClient *self, && !NM_IN_SET(new_state, NM_DHCP_STATE_BOUND, NM_DHCP_STATE_EXTENDED)) return; - if (_LOGI_ENABLED()) { + if (_LOGD_ENABLED()) { gs_free const char **keys = NULL; guint i, nkeys; keys = nm_utils_strdict_get_keys(options, TRUE, &nkeys); for (i = 0; i < nkeys; i++) { - _LOGI("option %-20s => '%s'", keys[i], (char *) g_hash_table_lookup(options, keys[i])); + _LOGD("option %-20s => '%s'", keys[i], (char *) g_hash_table_lookup(options, keys[i])); } } - if (priv->addr_family == AF_INET6) - event_id = nm_dhcp_utils_get_dhcp6_event_id(options); + if (_LOGT_ENABLED() && priv->addr_family == AF_INET6) { + gs_free char *event_id = NULL; - _LOGI("state changed %s -> %s%s%s%s", - state_to_string(priv->state), - state_to_string(new_state), - NM_PRINT_FMT_QUOTED(event_id, ", event ID=\"", event_id, "\"", "")); + event_id = nm_dhcp_utils_get_dhcp6_event_id(options); + if (event_id) + _LOGT("event-id: \"%s\"", event_id); + } + + if (_LOGI_ENABLED()) { + const char *req_str = + NM_IS_IPv4(priv->addr_family) + ? nm_dhcp_option_request_string(AF_INET, NM_DHCP_OPTION_DHCP4_NM_IP_ADDRESS) + : nm_dhcp_option_request_string(AF_INET6, NM_DHCP_OPTION_DHCP6_NM_IP_ADDRESS); + const char *addr = nm_g_hash_table_lookup(options, req_str); + + _LOGI("state changed %s -> %s%s%s%s", + state_to_string(priv->state), + state_to_string(new_state), + NM_PRINT_FMT_QUOTED(addr, ", address=", addr, "", "")); + } priv->state = new_state; g_signal_emit(G_OBJECT(self), signals[SIGNAL_STATE_CHANGED], 0, new_state, ip_config, options);