From cded6d518455ddb1ad8d0a899a10f054dfa9d9f6 Mon Sep 17 00:00:00 2001 From: Francesco Giudici Date: Fri, 19 Jul 2019 10:07:15 +0200 Subject: dhcp: nettools: retrieve and expose the expiry time also fix the lease time: it should be the time in seconds that the lease lasts --- src/dhcp/nm-dhcp-nettools.c | 49 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 8 deletions(-) diff --git a/src/dhcp/nm-dhcp-nettools.c b/src/dhcp/nm-dhcp-nettools.c index 1f183f362a..dddfdbf2ee 100644 --- a/src/dhcp/nm-dhcp-nettools.c +++ b/src/dhcp/nm-dhcp-nettools.c @@ -355,10 +355,13 @@ lease_parse_address (NDhcp4ClientLease *lease, { char addr_str[NM_UTILS_INET_ADDRSTRLEN]; const gint64 ts = nm_utils_get_monotonic_timestamp_ns (); + const gint64 ts_clock_boottime = nm_utils_monotonic_timestamp_as_boottime (ts, 1); struct in_addr a_address; struct in_addr a_netmask; guint32 a_plen; - guint64 a_lifetime; + guint64 nettools_lifetime; + gint64 a_lifetime; + gint64 a_expiry; n_dhcp4_client_lease_get_yiaddr (lease, &a_address); if (a_address.s_addr == INADDR_ANY) { @@ -367,7 +370,29 @@ lease_parse_address (NDhcp4ClientLease *lease, } /* n_dhcp4_client_lease_get_lifetime() never fails */ - n_dhcp4_client_lease_get_lifetime (lease, &a_lifetime); + n_dhcp4_client_lease_get_lifetime (lease, &nettools_lifetime); + /* FIXME: n_dhcp4_client_lease_get_lifetime() returns the time in nsec of CLOCK_BOOTTIME. + * We want to retrieve the original lifetime value in seconds, so we approximate it in a_lifetime. + * Use a nettools API to retrieve the original value as passed by the server. + */ + if (nettools_lifetime == G_MAXUINT64) { + a_lifetime = NM_PLATFORM_LIFETIME_PERMANENT; + a_expiry = NM_PLATFORM_LIFETIME_PERMANENT; + } else { + gint64 ts_time = time (NULL); + + a_lifetime = ((gint64) nettools_lifetime - ts_clock_boottime) / NM_UTILS_NS_PER_SECOND; + /* A lease time of 0 is allowed on some dhcp servers, so, let's accept it. */ + if (a_lifetime < 0) + a_lifetime = 0; + else if (a_lifetime > NM_PLATFORM_LIFETIME_PERMANENT) + a_lifetime = NM_PLATFORM_LIFETIME_PERMANENT - 1; + + if (ts_time > NM_PLATFORM_LIFETIME_PERMANENT - a_lifetime) + a_expiry = NM_PLATFORM_LIFETIME_PERMANENT - 1; + else + a_expiry = ts_time + a_lifetime; + } if (!lease_get_in_addr (lease, NM_DHCP_OPTION_DHCP4_SUBNET_MASK, &a_netmask)) { nm_utils_error_set_literal (error, NM_UTILS_ERROR_UNKNOWN, "could not get netmask from lease"); @@ -387,12 +412,20 @@ lease_parse_address (NDhcp4ClientLease *lease, NM_DHCP_OPTION_DHCP4_SUBNET_MASK, nm_utils_inet4_ntop (a_netmask.s_addr, addr_str)); - LOG_LEASE (LOGD_DHCP4, "expires in %u seconds", - (guint) ((a_lifetime - ts)/1000000000)); + LOG_LEASE (LOGD_DHCP4, "%s '%u' seconds (at %lld)", + nm_dhcp_option_request_string (_nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_IP_ADDRESS_LEASE_TIME), + (guint) a_lifetime, + (long long) a_expiry); nm_dhcp_option_add_option_u64 (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_IP_ADDRESS_LEASE_TIME, - (guint64) (a_lifetime / 1000000000)); + (guint64) a_lifetime); + + nm_dhcp_option_add_option_u64 (options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_NM_EXPIRY, + (guint64) a_expiry); nm_ip4_config_add_address (ip4_config, &((const NMPlatformIP4Address) { @@ -400,9 +433,9 @@ lease_parse_address (NDhcp4ClientLease *lease, .peer_address = a_address.s_addr, .plen = a_plen, .addr_source = NM_IP_CONFIG_SOURCE_DHCP, - .timestamp = ts / 1000000000, - .lifetime = (a_lifetime - ts) / 1000000000, - .preferred = (a_lifetime - ts) / 1000000000, + .timestamp = ts / NM_UTILS_NS_PER_SECOND, + .lifetime = a_lifetime, + .preferred = a_lifetime, })); return TRUE; -- cgit v1.2.1 From dbd9ed1c627018a5f91fef25ee81817be19d2606 Mon Sep 17 00:00:00 2001 From: Francesco Giudici Date: Thu, 25 Jul 2019 11:03:40 +0200 Subject: dhcp: nettools: retrieve and expose the private dhcp options --- src/dhcp/nm-dhcp-nettools.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/dhcp/nm-dhcp-nettools.c b/src/dhcp/nm-dhcp-nettools.c index dddfdbf2ee..a6c6cf8fd1 100644 --- a/src/dhcp/nm-dhcp-nettools.c +++ b/src/dhcp/nm-dhcp-nettools.c @@ -856,6 +856,43 @@ lease_parse_wpad (NDhcp4ClientLease *lease, str->str); } +static void +lease_parse_private_options (NDhcp4ClientLease *lease, + const char *iface, + GHashTable *options) +{ + int i; + + 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; + int r; + + /* We manage private options 249 (private classless static route) and 252 (wpad) in a special + * way, so skip them as we here just manage all (the other) private options as raw data */ + if (NM_IN_SET (i, NM_DHCP_OPTION_DHCP4_PRIVATE_CLASSLESS_STATIC_ROUTE, + NM_DHCP_OPTION_DHCP4_PRIVATE_PROXY_AUTODISCOVERY)) + continue; + + r = n_dhcp4_client_lease_query (lease, i, &data, &n_data); + if (r) + continue; + + option_string = nm_utils_bin2hexstr_full (data, n_data, ':', FALSE, NULL); + LOG_LEASE (LOGD_DHCP4, "%s '%s'", + nm_dhcp_option_request_string (_nm_dhcp_option_dhcp4_options, i), + option_string); + if (options) { + nm_dhcp_option_take_option (options, + _nm_dhcp_option_dhcp4_options, + i, + g_steal_pointer (&option_string)); + } + + } +} + static NMIP4Config * lease_to_ip4_config (NMDedupMultiIndex *multi_idx, const char *iface, @@ -888,6 +925,7 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, lease_parse_ntps (lease, iface, options); lease_parse_root_path (lease, iface, options); lease_parse_wpad (lease, iface, options); + lease_parse_private_options (lease, iface, options); NM_SET_OUT (out_options, g_steal_pointer (&options)); return g_steal_pointer (&ip4_config); -- cgit v1.2.1 From 862177f6b655ec027c4bdb0826040de1bb5d86d6 Mon Sep 17 00:00:00 2001 From: Francesco Giudici Date: Mon, 29 Jul 2019 17:43:18 +0200 Subject: dhcp: log the dhcp options got in the lease once for all plugin Each plugin logged the options: just do that on dhcp state change and do in common code. Log the options at INFO level for all the plugins. This partially reverts the effects on the internal plugin of the commit: 97ce488f5f50 ('dhcp/internal: decrease logging level when retrieving dhcp options') --- src/dhcp/nm-dhcp-client.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/dhcp/nm-dhcp-client.c b/src/dhcp/nm-dhcp-client.c index 56660ff796..b9c673ddad 100644 --- a/src/dhcp/nm-dhcp-client.c +++ b/src/dhcp/nm-dhcp-client.c @@ -406,6 +406,17 @@ nm_dhcp_client_set_state (NMDhcpClient *self, if ((priv->state == new_state) && (new_state != NM_DHCP_STATE_BOUND)) return; + if (_LOGI_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])); + } + } + if ( priv->addr_family == AF_INET6 && new_state == NM_DHCP_STATE_BOUND) { char *start, *iaid; @@ -840,15 +851,6 @@ nm_dhcp_client_handle_event (gpointer unused, g_variant_unref (value); } - if (nm_logging_enabled (LOGL_DEBUG, LOGD_DHCP6)) { - GHashTableIter hash_iter; - gpointer key, val; - - g_hash_table_iter_init (&hash_iter, str_options); - while (g_hash_table_iter_next (&hash_iter, &key, &val)) - _LOGD ("option '%s'=>'%s'", (const char *) key, (const char *) val); - } - /* Create the IP config */ if (g_hash_table_size (str_options) > 0) { if (priv->addr_family == AF_INET) { -- cgit v1.2.1 From fff39e4a8873be6a962748ed740b8a0716bf983b Mon Sep 17 00:00:00 2001 From: Francesco Giudici Date: Mon, 29 Jul 2019 17:58:16 +0200 Subject: dhcp: internal: drop plugin logging of the lease options as these are already logged in common dhcp-client code. --- src/dhcp/nm-dhcp-systemd.c | 108 +++++++++------------------------------------ 1 file changed, 20 insertions(+), 88 deletions(-) diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c index 34aade0792..b943bead06 100644 --- a/src/dhcp/nm-dhcp-systemd.c +++ b/src/dhcp/nm-dhcp-systemd.c @@ -67,13 +67,6 @@ G_DEFINE_TYPE (NMDhcpSystemd, nm_dhcp_systemd, NM_TYPE_DHCP_CLIENT) /*****************************************************************************/ -#define LOG_LEASE(domain, ...) \ -G_STMT_START { \ - if (log_lease) { \ - _LOG2D ((domain), (iface), " "__VA_ARGS__); \ - } \ -} G_STMT_END - static NMIP4Config * lease_to_ip4_config (NMDedupMultiIndex *multi_idx, const char *iface, @@ -81,7 +74,6 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, sd_dhcp_lease *lease, guint32 route_table, guint32 route_metric, - gboolean log_lease, GHashTable **out_options, GError **error) { @@ -136,25 +128,17 @@ 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); - LOG_LEASE (LOGD_DHCP4, "%s '%s'", - nm_dhcp_option_request_string (_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_NM_IP_ADDRESS, addr_str); + nm_dhcp_option_add_option (options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_NM_IP_ADDRESS, + addr_str); a_plen = nm_utils_ip4_netmask_to_prefix (a_netmask.s_addr); - LOG_LEASE (LOGD_DHCP4, "%s '%u'", - nm_dhcp_option_request_string (_nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_SUBNET_MASK), - (guint) a_plen); 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)); - LOG_LEASE (LOGD_DHCP4, "%s '%u' seconds (at %lld)", - nm_dhcp_option_request_string (_nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_IP_ADDRESS_LEASE_TIME), - (guint) a_lifetime, - (long long) (ts_time + a_lifetime)); nm_dhcp_option_add_option_u64 (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_IP_ADDRESS_LEASE_TIME, @@ -177,9 +161,6 @@ 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); - LOG_LEASE (LOGD_DHCP4, "%s '%s'", - nm_dhcp_option_request_string (_nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_SERVER_ID), - addr_str); nm_dhcp_option_add_option (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_SERVER_ID, @@ -188,9 +169,6 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, if (sd_dhcp_lease_get_broadcast (lease, &broadcast) >= 0) { nm_utils_inet4_ntop (broadcast.s_addr, addr_str); - LOG_LEASE (LOGD_DHCP4, "%s '%s'", - nm_dhcp_option_request_string (_nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_BROADCAST), - addr_str); nm_dhcp_option_add_option (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_BROADCAST, @@ -212,9 +190,6 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, } nm_ip4_config_add_nameserver (ip4_config, addr_list[i].s_addr); } - LOG_LEASE (LOGD_DHCP4, "%s '%s'", - nm_dhcp_option_request_string (_nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_DOMAIN_NAME_SERVER), - str->str); nm_dhcp_option_add_option (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_DOMAIN_NAME_SERVER, @@ -228,9 +203,6 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, g_string_append (nm_gstring_add_space_delimiter (str), search_domains[i]); nm_ip4_config_add_search (ip4_config, search_domains[i]); } - LOG_LEASE (LOGD_DHCP4, "%s '%s'", - nm_dhcp_option_request_string (_nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_DOMAIN_SEARCH_LIST), - str->str); nm_dhcp_option_add_option (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_DOMAIN_SEARCH_LIST, @@ -241,10 +213,10 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, gs_strfreev char **domains = NULL; char **d; - LOG_LEASE (LOGD_DHCP4, "%s '%s'", - nm_dhcp_option_request_string (_nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_DOMAIN_NAME), - s); - nm_dhcp_option_add_option (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_DOMAIN_NAME, s); + nm_dhcp_option_add_option (options, + _nm_dhcp_option_dhcp4_options, + 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. */ @@ -254,10 +226,10 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, } if (sd_dhcp_lease_get_hostname (lease, &s) >= 0) { - LOG_LEASE (LOGD_DHCP4, "%s '%s'", - nm_dhcp_option_request_string (_nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_HOST_NAME), - 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, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_HOST_NAME, + s); } num = sd_dhcp_lease_get_routes (lease, &routes); @@ -310,14 +282,6 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, nm_utils_inet4_ntop (network_net, network_net_str); nm_utils_inet4_ntop (r_gateway.s_addr, gateway_str); - LOG_LEASE (LOGD_DHCP4, - "%sstatic_route %s/%d gw %s", - option == NM_DHCP_OPTION_DHCP4_CLASSLESS_STATIC_ROUTE - ? "rfc3442_classless_" - : "", - network_net_str, - (int) r_plen, - gateway_str); g_string_append_printf (nm_gstring_add_space_delimiter ( option == NM_DHCP_OPTION_DHCP4_CLASSLESS_STATIC_ROUTE ? str_classless : str_static), @@ -419,17 +383,14 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, }), NULL); } - LOG_LEASE (LOGD_DHCP4, "%s '%s'", - nm_dhcp_option_request_string (_nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_ROUTER), - str->str); - nm_dhcp_option_add_option (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_ROUTER, str->str); + nm_dhcp_option_add_option (options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_ROUTER, + str->str); } if ( sd_dhcp_lease_get_mtu (lease, &mtu) >= 0 && mtu) { - LOG_LEASE (LOGD_DHCP4, "%s '%u'", - nm_dhcp_option_request_string (_nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_INTERFACE_MTU), - mtu); nm_dhcp_option_add_option_u64 (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_INTERFACE_MTU, @@ -444,9 +405,6 @@ 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); } - LOG_LEASE (LOGD_DHCP4, "%s '%s'", - nm_dhcp_option_request_string (_nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_NTP_SERVER), - str->str); nm_dhcp_option_add_option (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_NTP_SERVER, @@ -454,16 +412,13 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, } if (sd_dhcp_lease_get_root_path (lease, &s) >= 0) { - LOG_LEASE (LOGD_DHCP4, "%s '%s'", - nm_dhcp_option_request_string (_nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_ROOT_PATH), - s); - nm_dhcp_option_add_option (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_ROOT_PATH, s); + nm_dhcp_option_add_option (options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_ROOT_PATH, + s); } if (sd_dhcp_lease_get_t1 (lease, &renewal) >= 0) { - LOG_LEASE (LOGD_DHCP4, "%s '%u'", - nm_dhcp_option_request_string (_nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_RENEWAL_T1_TIME), - renewal); nm_dhcp_option_add_option_u64 (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_RENEWAL_T1_TIME, @@ -471,9 +426,6 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, } if (sd_dhcp_lease_get_t2 (lease, &rebinding) >= 0) { - LOG_LEASE (LOGD_DHCP4, "%s '%u'", - nm_dhcp_option_request_string (_nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_REBINDING_T2_TIME), - rebinding); nm_dhcp_option_add_option_u64 (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_REBINDING_T2_TIME, @@ -481,9 +433,6 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, } if (sd_dhcp_lease_get_timezone (lease, &s) >= 0) { - LOG_LEASE (LOGD_DHCP4, "%s '%s'", - nm_dhcp_option_request_string (_nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_NEW_TZDB_TIMEZONE), - s); nm_dhcp_option_add_option (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_NEW_TZDB_TIMEZONE, @@ -502,9 +451,6 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, option_string = nm_utils_bin2hexstr_full (private_options[i].data, private_options[i].data_len, ':', FALSE, NULL); - LOG_LEASE (LOGD_DHCP4, "%s '%s'", - nm_dhcp_option_request_string (_nm_dhcp_option_dhcp4_options, private_options[i].code), - option_string); if (!options) { g_free (option_string); continue; @@ -546,7 +492,6 @@ bound4_handle (NMDhcpSystemd *self) lease, nm_dhcp_client_get_route_table (NM_DHCP_CLIENT (self)), nm_dhcp_client_get_route_metric (NM_DHCP_CLIENT (self)), - TRUE, &options, &error); if (!ip4_config) { @@ -762,7 +707,6 @@ lease_to_ip6_config (NMDedupMultiIndex *multi_idx, const char *iface, int ifindex, sd_dhcp6_lease *lease, - gboolean log_lease, gboolean info_only, GHashTable **out_options, GError **error) @@ -786,7 +730,6 @@ lease_to_ip6_config (NMDedupMultiIndex *multi_idx, sd_dhcp6_lease_reset_address_iter (lease); nm_gstring_prepare (&str); while (sd_dhcp6_lease_get_address (lease, &tmp_addr, &lft_pref, &lft_valid) >= 0) { - char sbuf[400]; const NMPlatformIP6Address address = { .plen = 128, .address = tmp_addr, @@ -800,10 +743,6 @@ lease_to_ip6_config (NMDedupMultiIndex *multi_idx, nm_utils_inet6_ntop (&tmp_addr, addr_str); g_string_append (nm_gstring_add_space_delimiter (str), addr_str); - - LOG_LEASE (LOGD_DHCP6, "%s '%s'", - nm_dhcp_option_request_string (_nm_dhcp_option_dhcp6_options, NM_DHCP_OPTION_DHCP6_NM_IP_ADDRESS), - nm_platform_ip6_address_to_string (&address, sbuf, sizeof (sbuf))); }; if (str->len) nm_dhcp_option_add_option (options, @@ -828,9 +767,6 @@ 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]); } - LOG_LEASE (LOGD_DHCP6, "%s '%s'", - nm_dhcp_option_request_string (_nm_dhcp_option_dhcp6_options, NM_DHCP_OPTION_DHCP6_DNS_SERVERS), - str->str); nm_dhcp_option_add_option (options, _nm_dhcp_option_dhcp6_options, NM_DHCP_OPTION_DHCP6_DNS_SERVERS, @@ -844,9 +780,6 @@ 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]); } - LOG_LEASE (LOGD_DHCP6, "%s '%s'", - nm_dhcp_option_request_string (_nm_dhcp_option_dhcp6_options, NM_DHCP_OPTION_DHCP6_DOMAIN_LIST), - str->str); nm_dhcp_option_add_option (options, _nm_dhcp_option_dhcp6_options, NM_DHCP_OPTION_DHCP6_DOMAIN_LIST, @@ -880,7 +813,6 @@ bound6_handle (NMDhcpSystemd *self) iface, nm_dhcp_client_get_ifindex (NM_DHCP_CLIENT (self)), lease, - TRUE, nm_dhcp_client_get_info_only (NM_DHCP_CLIENT (self)), &options, &error); -- cgit v1.2.1 From 6945ecb80423eb6c1eb4cc4e0408ce2b0356e9a0 Mon Sep 17 00:00:00 2001 From: Francesco Giudici Date: Tue, 30 Jul 2019 14:22:21 +0200 Subject: dhcp: nettools: drop plugin logging of the lease options as these are already logged in common dhcp-client code. Moreover, now the log level of the lease options will move from INFO to DEBUG. --- src/dhcp/nm-dhcp-nettools.c | 55 ++++++++++++++------------------------------- 1 file changed, 17 insertions(+), 38 deletions(-) diff --git a/src/dhcp/nm-dhcp-nettools.c b/src/dhcp/nm-dhcp-nettools.c index a6c6cf8fd1..8d1ca13678 100644 --- a/src/dhcp/nm-dhcp-nettools.c +++ b/src/dhcp/nm-dhcp-nettools.c @@ -341,11 +341,6 @@ lease_get_u16 (NDhcp4ClientLease *lease, return TRUE; } -#define LOG_LEASE(domain, ...) \ - G_STMT_START { \ - _LOG2I ((domain), (iface), " "__VA_ARGS__); \ - } G_STMT_END - static gboolean lease_parse_address (NDhcp4ClientLease *lease, const char *iface, @@ -402,7 +397,6 @@ lease_parse_address (NDhcp4ClientLease *lease, nm_utils_inet4_ntop (a_address.s_addr, addr_str); a_plen = nm_utils_ip4_netmask_to_prefix (a_netmask.s_addr); - LOG_LEASE (LOGD_DHCP4, "address %s/%u", addr_str, a_plen); nm_dhcp_option_add_option (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_NM_IP_ADDRESS, @@ -412,11 +406,6 @@ lease_parse_address (NDhcp4ClientLease *lease, NM_DHCP_OPTION_DHCP4_SUBNET_MASK, nm_utils_inet4_ntop (a_netmask.s_addr, addr_str)); - LOG_LEASE (LOGD_DHCP4, "%s '%u' seconds (at %lld)", - nm_dhcp_option_request_string (_nm_dhcp_option_dhcp4_options, - NM_DHCP_OPTION_DHCP4_IP_ADDRESS_LEASE_TIME), - (guint) a_lifetime, - (long long) a_expiry); nm_dhcp_option_add_option_u64 (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_IP_ADDRESS_LEASE_TIME, @@ -474,7 +463,6 @@ lease_parse_domain_name_servers (NDhcp4ClientLease *lease, nm_ip4_config_add_nameserver (ip4_config, addr.s_addr); } - LOG_LEASE (LOGD_DHCP4, "nameserver '%s'", str->str); nm_dhcp_option_add_option (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_DOMAIN_NAME_SERVER, @@ -513,11 +501,6 @@ lease_parse_routes (NDhcp4ClientLease *lease, nm_utils_inet4_ntop (dest.s_addr, dest_str); nm_utils_inet4_ntop (gateway.s_addr, gateway_str); - LOG_LEASE (LOGD_DHCP4, - "classless static route %s/%d gw %s", - dest_str, - (int) plen, - gateway_str); g_string_append_printf (nm_gstring_add_space_delimiter (str), "%s/%d %s", dest_str, @@ -562,11 +545,6 @@ lease_parse_routes (NDhcp4ClientLease *lease, nm_utils_inet4_ntop (dest.s_addr, dest_str); nm_utils_inet4_ntop (gateway.s_addr, gateway_str); - LOG_LEASE (LOGD_DHCP4, - "static route %s/%d gw %s", - dest_str, - (int) plen, - gateway_str); g_string_append_printf (nm_gstring_add_space_delimiter (str), "%s/%d %s", dest_str, @@ -643,7 +621,6 @@ lease_parse_routes (NDhcp4ClientLease *lease, }), NULL); } - LOG_LEASE (LOGD_DHCP4, "router %s", str->str); nm_dhcp_option_add_option (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_ROUTER, @@ -665,7 +642,6 @@ lease_parse_mtu (NDhcp4ClientLease *lease, if (mtu < 68) return; - LOG_LEASE (LOGD_DHCP4, "mtu %u", mtu); nm_dhcp_option_add_option_u64 (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_INTERFACE_MTU, @@ -691,7 +667,7 @@ lease_parse_metered (NDhcp4ClientLease *lease, metered = !!memmem (data, n_data, "ANDROID_METERED", NM_STRLEN ("ANDROID_METERED")); } - LOG_LEASE (LOGD_DHCP4, "%s", metered ? "metered" : "unmetered"); + /* TODO: expose the vendor specific option when present */ nm_ip4_config_set_metered (ip4_config, metered); } @@ -718,8 +694,10 @@ lease_parse_ntps (NDhcp4ClientLease *lease, g_string_append (nm_gstring_add_space_delimiter (str), addr_str); } - LOG_LEASE (LOGD_DHCP4, "ntp server '%s'", str->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, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_NTP_SERVER, + str->str); } static void @@ -741,8 +719,10 @@ lease_parse_hostname (NDhcp4ClientLease *lease, if (is_localhost(str->str)) return; - LOG_LEASE (LOGD_DHCP4, "hostname '%s'", str->str); - nm_dhcp_option_add_option (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_HOST_NAME, str->str); + nm_dhcp_option_add_option (options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_HOST_NAME, + str->str); } static void @@ -774,8 +754,10 @@ lease_parse_domainname (NDhcp4ClientLease *lease, g_string_append (nm_gstring_add_space_delimiter (str), *d); nm_ip4_config_add_domain (ip4_config, *d); } - LOG_LEASE (LOGD_DHCP4, "domain name '%s'", str->str); - nm_dhcp_option_add_option (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_DOMAIN_NAME, str->str); + nm_dhcp_option_add_option (options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_DOMAIN_NAME, + str->str); } static void @@ -808,7 +790,6 @@ lease_parse_search_domains (NDhcp4ClientLease *lease, g_string_append (nm_gstring_add_space_delimiter (str), domain->str); nm_ip4_config_add_search (ip4_config, domain->str); } - LOG_LEASE (LOGD_DHCP4, "domain search '%s'", str->str); nm_dhcp_option_add_option (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_DOMAIN_SEARCH_LIST, @@ -830,8 +811,10 @@ lease_parse_root_path (NDhcp4ClientLease *lease, return; str = g_string_new_len ((char *)data, n_data); - LOG_LEASE (LOGD_DHCP4, "root path '%s'", str->str); - nm_dhcp_option_add_option (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_ROOT_PATH, str->str); + nm_dhcp_option_add_option (options, + _nm_dhcp_option_dhcp4_options, + NM_DHCP_OPTION_DHCP4_ROOT_PATH, + str->str); } static void @@ -849,7 +832,6 @@ lease_parse_wpad (NDhcp4ClientLease *lease, return; str = g_string_new_len ((char *)data, n_data); - LOG_LEASE (LOGD_DHCP4, "wpad '%s'", str->str); nm_dhcp_option_add_option (options, _nm_dhcp_option_dhcp4_options, NM_DHCP_OPTION_DHCP4_PRIVATE_PROXY_AUTODISCOVERY, @@ -880,9 +862,6 @@ lease_parse_private_options (NDhcp4ClientLease *lease, continue; option_string = nm_utils_bin2hexstr_full (data, n_data, ':', FALSE, NULL); - LOG_LEASE (LOGD_DHCP4, "%s '%s'", - nm_dhcp_option_request_string (_nm_dhcp_option_dhcp4_options, i), - option_string); if (options) { nm_dhcp_option_take_option (options, _nm_dhcp_option_dhcp4_options, -- cgit v1.2.1 From 3a10c477864ed4d07d0e4c67537b67add52926f0 Mon Sep 17 00:00:00 2001 From: Francesco Giudici Date: Tue, 30 Jul 2019 14:37:38 +0200 Subject: dhcp: nettools: drop the 'iface' parameter when no more used Drop it from the functions for extracting the dhcp options from the lease: it was just used for the logging, but now we log all the options once, at the end of the process. --- src/dhcp/nm-dhcp-nettools.c | 38 +++++++++++++------------------------- 1 file changed, 13 insertions(+), 25 deletions(-) diff --git a/src/dhcp/nm-dhcp-nettools.c b/src/dhcp/nm-dhcp-nettools.c index 8d1ca13678..594145a84d 100644 --- a/src/dhcp/nm-dhcp-nettools.c +++ b/src/dhcp/nm-dhcp-nettools.c @@ -343,7 +343,6 @@ lease_get_u16 (NDhcp4ClientLease *lease, static gboolean lease_parse_address (NDhcp4ClientLease *lease, - const char *iface, NMIP4Config *ip4_config, GHashTable *options, GError **error) @@ -432,7 +431,6 @@ lease_parse_address (NDhcp4ClientLease *lease, static void lease_parse_domain_name_servers (NDhcp4ClientLease *lease, - const char *iface, NMIP4Config *ip4_config, GHashTable *options) { @@ -471,7 +469,6 @@ lease_parse_domain_name_servers (NDhcp4ClientLease *lease, static void lease_parse_routes (NDhcp4ClientLease *lease, - const char *iface, NMIP4Config *ip4_config, GHashTable *options, guint32 route_table, @@ -630,7 +627,6 @@ lease_parse_routes (NDhcp4ClientLease *lease, static void lease_parse_mtu (NDhcp4ClientLease *lease, - const char *iface, NMIP4Config *ip4_config, GHashTable *options) { @@ -651,7 +647,6 @@ lease_parse_mtu (NDhcp4ClientLease *lease, static void lease_parse_metered (NDhcp4ClientLease *lease, - const char *iface, NMIP4Config *ip4_config, GHashTable *options) { @@ -673,7 +668,6 @@ lease_parse_metered (NDhcp4ClientLease *lease, static void lease_parse_ntps (NDhcp4ClientLease *lease, - const char *iface, GHashTable *options) { nm_auto_free_gstring GString *str = NULL; @@ -702,7 +696,6 @@ lease_parse_ntps (NDhcp4ClientLease *lease, static void lease_parse_hostname (NDhcp4ClientLease *lease, - const char *iface, GHashTable *options) { nm_auto_free_gstring GString *str = NULL; @@ -727,7 +720,6 @@ lease_parse_hostname (NDhcp4ClientLease *lease, static void lease_parse_domainname (NDhcp4ClientLease *lease, - const char *iface, NMIP4Config *ip4_config, GHashTable *options) { @@ -762,7 +754,6 @@ lease_parse_domainname (NDhcp4ClientLease *lease, static void lease_parse_search_domains (NDhcp4ClientLease *lease, - const char *iface, NMIP4Config *ip4_config, GHashTable *options) { @@ -798,7 +789,6 @@ lease_parse_search_domains (NDhcp4ClientLease *lease, static void lease_parse_root_path (NDhcp4ClientLease *lease, - const char *iface, GHashTable *options) { nm_auto_free_gstring GString *str = NULL; @@ -819,7 +809,6 @@ lease_parse_root_path (NDhcp4ClientLease *lease, static void lease_parse_wpad (NDhcp4ClientLease *lease, - const char *iface, GHashTable *options) { nm_auto_free_gstring GString *str = NULL; @@ -840,7 +829,6 @@ lease_parse_wpad (NDhcp4ClientLease *lease, static void lease_parse_private_options (NDhcp4ClientLease *lease, - const char *iface, GHashTable *options) { int i; @@ -890,21 +878,21 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, ip4_config = nm_ip4_config_new (multi_idx, ifindex); options = out_options ? nm_dhcp_option_create_options_dict () : NULL; - if (!lease_parse_address (lease, iface, ip4_config, options, error)) + if (!lease_parse_address (lease, ip4_config, options, error)) return NULL; - lease_parse_routes (lease, iface, ip4_config, options, route_table, route_metric); - lease_parse_domain_name_servers (lease, iface, ip4_config, options); - lease_parse_domainname (lease, iface, ip4_config, options); - lease_parse_search_domains (lease, iface, ip4_config, options); - lease_parse_mtu (lease, iface, ip4_config, options); - lease_parse_metered (lease, iface, ip4_config, options); - - lease_parse_hostname (lease, iface, options); - lease_parse_ntps (lease, iface, options); - lease_parse_root_path (lease, iface, options); - lease_parse_wpad (lease, iface, options); - lease_parse_private_options (lease, iface, options); + lease_parse_routes (lease, ip4_config, options, route_table, route_metric); + lease_parse_domain_name_servers (lease, ip4_config, options); + lease_parse_domainname (lease, ip4_config, options); + lease_parse_search_domains (lease, ip4_config, options); + lease_parse_mtu (lease, ip4_config, options); + lease_parse_metered (lease, ip4_config, options); + + lease_parse_hostname (lease, options); + lease_parse_ntps (lease, options); + lease_parse_root_path (lease, options); + lease_parse_wpad (lease, options); + lease_parse_private_options (lease, options); NM_SET_OUT (out_options, g_steal_pointer (&options)); return g_steal_pointer (&ip4_config); -- cgit v1.2.1 From 86973eae1f86fd7f36e23ade9fe8892efd8c1846 Mon Sep 17 00:00:00 2001 From: Francesco Giudici Date: Mon, 2 Sep 2019 17:30:59 +0200 Subject: dhcp: nettools: add utf8 checking on the wpad option and assert values exported as dhcp options are utf8 compliant --- src/dhcp/nm-dhcp-nettools.c | 9 ++++++--- src/dhcp/nm-dhcp-options.c | 1 + 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/dhcp/nm-dhcp-nettools.c b/src/dhcp/nm-dhcp-nettools.c index 594145a84d..12b14c28ca 100644 --- a/src/dhcp/nm-dhcp-nettools.c +++ b/src/dhcp/nm-dhcp-nettools.c @@ -811,7 +811,7 @@ static void lease_parse_wpad (NDhcp4ClientLease *lease, GHashTable *options) { - nm_auto_free_gstring GString *str = NULL; + gs_free char *wpad = NULL; uint8_t *data; size_t n_data; int r; @@ -820,11 +820,14 @@ lease_parse_wpad (NDhcp4ClientLease *lease, if (r) return; - str = g_string_new_len ((char *)data, n_data); + 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, - str->str); + wpad); } static void diff --git a/src/dhcp/nm-dhcp-options.c b/src/dhcp/nm-dhcp-options.c index 0cefd51b1e..b41ca2c941 100644 --- a/src/dhcp/nm-dhcp-options.c +++ b/src/dhcp/nm-dhcp-options.c @@ -222,6 +222,7 @@ nm_dhcp_option_take_option (GHashTable *options, nm_assert (options); 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), -- cgit v1.2.1 From 07b3ecbb7a52e959713587c933e5ea1775183a6b Mon Sep 17 00:00:00 2001 From: Francesco Giudici Date: Mon, 2 Sep 2019 17:44:07 +0200 Subject: dhcp: nettools: drop useless "options" GHashTable checks --- src/dhcp/nm-dhcp-nettools.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/dhcp/nm-dhcp-nettools.c b/src/dhcp/nm-dhcp-nettools.c index 12b14c28ca..86c3311f40 100644 --- a/src/dhcp/nm-dhcp-nettools.c +++ b/src/dhcp/nm-dhcp-nettools.c @@ -853,13 +853,10 @@ lease_parse_private_options (NDhcp4ClientLease *lease, continue; option_string = nm_utils_bin2hexstr_full (data, n_data, ':', FALSE, NULL); - if (options) { - nm_dhcp_option_take_option (options, - _nm_dhcp_option_dhcp4_options, - i, - g_steal_pointer (&option_string)); - } - + nm_dhcp_option_take_option (options, + _nm_dhcp_option_dhcp4_options, + i, + g_steal_pointer (&option_string)); } } @@ -879,7 +876,7 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, g_return_val_if_fail (lease != NULL, NULL); ip4_config = nm_ip4_config_new (multi_idx, ifindex); - options = out_options ? nm_dhcp_option_create_options_dict () : NULL; + options = nm_dhcp_option_create_options_dict (); if (!lease_parse_address (lease, ip4_config, options, error)) return NULL; -- cgit v1.2.1