From 056f2679b848d7adb48044534704f9d3d3dcc656 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 14 Feb 2018 16:18:41 +0100 Subject: [PATCH 01/10] dhcp: fix memleak parsing dhclient file with multiple dhcp-client-identifier lines --- src/dhcp/nm-dhcp-dhclient-utils.c | 3 +++ src/dhcp/nm-dhcp-dhclient.c | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/dhcp/nm-dhcp-dhclient-utils.c b/src/dhcp/nm-dhcp-dhclient-utils.c index 4df90d76a0..ae7691f5e8 100644 --- a/src/dhcp/nm-dhcp-dhclient-utils.c +++ b/src/dhcp/nm-dhcp-dhclient-utils.c @@ -279,6 +279,7 @@ nm_dhcp_dhclient_create_config (const char *interface, g_return_val_if_fail (!anycast_addr || nm_utils_hwaddr_valid (anycast_addr, ETH_ALEN), NULL); g_return_val_if_fail (NM_IN_SET (addr_family, AF_INET, AF_INET6), NULL); + nm_assert (!out_new_client_id || !*out_new_client_id); new_contents = g_string_new (_("# Created by NetworkManager\n")); fqdn_opts = g_ptr_array_sized_new (5); @@ -332,6 +333,8 @@ nm_dhcp_dhclient_create_config (const char *interface, continue; /* Otherwise capture and return the existing client id */ + if (out_new_client_id) + g_clear_pointer (out_new_client_id, g_bytes_unref); NM_SET_OUT (out_new_client_id, read_client_id (p)); } diff --git a/src/dhcp/nm-dhcp-dhclient.c b/src/dhcp/nm-dhcp-dhclient.c index acb14a014e..f114e92ea9 100644 --- a/src/dhcp/nm-dhcp-dhclient.c +++ b/src/dhcp/nm-dhcp-dhclient.c @@ -518,8 +518,10 @@ ip4_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const char *last priv->conf_file = create_dhclient_config (self, AF_INET, iface, uuid, client_id, dhcp_anycast_addr, hostname, timeout, use_fqdn, &new_client_id); if (priv->conf_file) { - if (new_client_id) + if (new_client_id) { + nm_assert (!client_id); nm_dhcp_client_set_client_id (client, new_client_id); + } success = dhclient_start (client, NULL, NULL, FALSE, NULL, 0); } else _LOGW ("error creating dhclient configuration file"); From badace72ddb16ee2c4a21c6248e8c80a9e30c82a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 14 Feb 2018 14:48:30 +0100 Subject: [PATCH 02/10] dhcp: chain up parent stop() for NMDhcpSystem client The parent's stop() implementation does nothing interesting for NMDhcpSystem. Still, call it, it's just unexpected to not chain up the parent implementation, if all other subclasses do it. In general, if the parent's implementation is not suitable to be called by the derived class, that should be handled differently then just not chaining up. Otherwise it's inconsistent and confusing. --- src/dhcp/nm-dhcp-dhclient.c | 1 - src/dhcp/nm-dhcp-dhcpcd.c | 1 - src/dhcp/nm-dhcp-systemd.c | 2 ++ 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dhcp/nm-dhcp-dhclient.c b/src/dhcp/nm-dhcp-dhclient.c index f114e92ea9..03ce4c3c73 100644 --- a/src/dhcp/nm-dhcp-dhclient.c +++ b/src/dhcp/nm-dhcp-dhclient.c @@ -564,7 +564,6 @@ stop (NMDhcpClient *client, gboolean release, const GByteArray *duid) NMDhcpDhclient *self = NM_DHCP_DHCLIENT (client); NMDhcpDhclientPrivate *priv = NM_DHCP_DHCLIENT_GET_PRIVATE (self); - /* Chain up to parent */ NM_DHCP_CLIENT_CLASS (nm_dhcp_dhclient_parent_class)->stop (client, release, duid); if (priv->conf_file) diff --git a/src/dhcp/nm-dhcp-dhcpcd.c b/src/dhcp/nm-dhcp-dhcpcd.c index 7a18d88b67..ff8efb20af 100644 --- a/src/dhcp/nm-dhcp-dhcpcd.c +++ b/src/dhcp/nm-dhcp-dhcpcd.c @@ -194,7 +194,6 @@ stop (NMDhcpClient *client, gboolean release, const GByteArray *duid) NMDhcpDhcpcd *self = NM_DHCP_DHCPCD (client); NMDhcpDhcpcdPrivate *priv = NM_DHCP_DHCPCD_GET_PRIVATE (self); - /* Chain up to parent */ NM_DHCP_CLIENT_CLASS (nm_dhcp_dhcpcd_parent_class)->stop (client, release, duid); if (priv->pid_file) { diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c index abb2c36cb8..213cd41d4d 100644 --- a/src/dhcp/nm-dhcp-systemd.c +++ b/src/dhcp/nm-dhcp-systemd.c @@ -1020,6 +1020,8 @@ stop (NMDhcpClient *client, gboolean release, const GByteArray *duid) NMDhcpSystemdPrivate *priv = NM_DHCP_SYSTEMD_GET_PRIVATE (self); int r = 0; + NM_DHCP_CLIENT_CLASS (nm_dhcp_systemd_parent_class)->stop (client, release, duid); + _LOGT ("dhcp-client%d: stop %p", priv->client4 ? '4' : '6', priv->client4 ? (gpointer) priv->client4 : (gpointer) priv->client6); From 8ff962d9e481b8bd3f8b29e3e217fc6c19bd1f31 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 14 Feb 2018 14:58:52 +0100 Subject: [PATCH 03/10] dhcp: cache info-only parameter in NMDhcpClient Optimally, NMDhcpClient would be stateless and all paramters would be passed on as argument. Clearly that is not feasable, because there are so many paramters, and in many cases they need to be cached for the lifetime of the client instance. Instead of passing info_only paramter to ip6_start() and cache it both in NMDhcpClient and NMDhcpSystemd, keep it in NMDhcpClient at one place. In the next commit, we will initialize info-only only once during the constructor, so it is immutable and somewhat stateless. --- src/dhcp/nm-dhcp-client.c | 10 ++++++++-- src/dhcp/nm-dhcp-client.h | 3 ++- src/dhcp/nm-dhcp-dhclient.c | 7 +++++-- src/dhcp/nm-dhcp-dhcpcanon.c | 1 - src/dhcp/nm-dhcp-dhcpcd.c | 1 - src/dhcp/nm-dhcp-systemd.c | 9 +++------ 6 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/dhcp/nm-dhcp-client.c b/src/dhcp/nm-dhcp-client.c index 8fbb3c64f9..317ddb5e6f 100644 --- a/src/dhcp/nm-dhcp-client.c +++ b/src/dhcp/nm-dhcp-client.c @@ -260,6 +260,14 @@ nm_dhcp_client_get_hostname (NMDhcpClient *self) return NM_DHCP_CLIENT_GET_PRIVATE (self)->hostname; } +gboolean +nm_dhcp_client_get_info_only (NMDhcpClient *self) +{ + g_return_val_if_fail (NM_IS_DHCP_CLIENT (self), FALSE); + + return NM_DHCP_CLIENT_GET_PRIVATE (self)->info_only; +} + gboolean nm_dhcp_client_get_use_fqdn (NMDhcpClient *self) { @@ -359,7 +367,6 @@ stop (NMDhcpClient *self, gboolean release, const GByteArray *duid) nm_dhcp_client_stop_pid (priv->pid, priv->iface); } priv->pid = -1; - priv->info_only = FALSE; } void @@ -626,7 +633,6 @@ nm_dhcp_client_start_ip6 (NMDhcpClient *self, return NM_DHCP_CLIENT_GET_CLASS (self)->ip6_start (self, dhcp_anycast_addr, ll_addr, - info_only, privacy, priv->duid, needed_prefixes); diff --git a/src/dhcp/nm-dhcp-client.h b/src/dhcp/nm-dhcp-client.h index 3583281dc3..6616f1406f 100644 --- a/src/dhcp/nm-dhcp-client.h +++ b/src/dhcp/nm-dhcp-client.h @@ -79,7 +79,6 @@ typedef struct { gboolean (*ip6_start) (NMDhcpClient *self, const char *anycast_addr, const struct in6_addr *ll_addr, - gboolean info_only, NMSettingIP6ConfigPrivacy privacy, const GByteArray *duid, guint needed_prefixes); @@ -134,6 +133,8 @@ GBytes *nm_dhcp_client_get_client_id (NMDhcpClient *self); const char *nm_dhcp_client_get_hostname (NMDhcpClient *self); +gboolean nm_dhcp_client_get_info_only (NMDhcpClient *self); + gboolean nm_dhcp_client_get_use_fqdn (NMDhcpClient *self); gboolean nm_dhcp_client_start_ip4 (NMDhcpClient *self, diff --git a/src/dhcp/nm-dhcp-dhclient.c b/src/dhcp/nm-dhcp-dhclient.c index 03ce4c3c73..e93ab9a099 100644 --- a/src/dhcp/nm-dhcp-dhclient.c +++ b/src/dhcp/nm-dhcp-dhclient.c @@ -533,7 +533,6 @@ static gboolean ip6_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const struct in6_addr *ll_addr, - gboolean info_only, NMSettingIP6ConfigPrivacy privacy, const GByteArray *duid, guint needed_prefixes) @@ -555,7 +554,11 @@ ip6_start (NMDhcpClient *client, return FALSE; } - return dhclient_start (client, info_only ? "-S" : "-N", duid, FALSE, NULL, needed_prefixes); + return dhclient_start (client, + nm_dhcp_client_get_info_only (NM_DHCP_CLIENT (self)) + ? "-S" + : "-N", + duid, FALSE, NULL, needed_prefixes); } static void diff --git a/src/dhcp/nm-dhcp-dhcpcanon.c b/src/dhcp/nm-dhcp-dhcpcanon.c index a03f9d05b5..5c8f0d7d26 100644 --- a/src/dhcp/nm-dhcp-dhcpcanon.c +++ b/src/dhcp/nm-dhcp-dhcpcanon.c @@ -179,7 +179,6 @@ static gboolean ip6_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const struct in6_addr *ll_addr, - gboolean info_only, NMSettingIP6ConfigPrivacy privacy, const GByteArray *duid, guint needed_prefixes) diff --git a/src/dhcp/nm-dhcp-dhcpcd.c b/src/dhcp/nm-dhcp-dhcpcd.c index ff8efb20af..1eb4398aa9 100644 --- a/src/dhcp/nm-dhcp-dhcpcd.c +++ b/src/dhcp/nm-dhcp-dhcpcd.c @@ -177,7 +177,6 @@ static gboolean ip6_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const struct in6_addr *ll_addr, - gboolean info_only, NMSettingIP6ConfigPrivacy privacy, const GByteArray *duid, guint needed_prefixes) diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c index 213cd41d4d..a7222e4863 100644 --- a/src/dhcp/nm-dhcp-systemd.c +++ b/src/dhcp/nm-dhcp-systemd.c @@ -60,8 +60,7 @@ typedef struct { guint request_count; - gboolean privacy; - gboolean info_only; + bool privacy:1; } NMDhcpSystemdPrivate; struct _NMDhcpSystemd { @@ -854,7 +853,7 @@ bound6_handle (NMDhcpSystemd *self) lease, options, TRUE, - priv->info_only, + nm_dhcp_client_get_info_only (NM_DHCP_CLIENT (self)), &error); if (ip6_config) { @@ -900,7 +899,6 @@ static gboolean ip6_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const struct in6_addr *ll_addr, - gboolean info_only, NMSettingIP6ConfigPrivacy privacy, const GByteArray *duid, guint needed_prefixes) @@ -918,7 +916,6 @@ ip6_start (NMDhcpClient *client, g_free (priv->lease_file); priv->lease_file = get_leasefile_path (AF_INET6, iface, nm_dhcp_client_get_uuid (client)); - priv->info_only = info_only; r = sd_dhcp6_client_new (&priv->client6); if (r < 0) { @@ -933,7 +930,7 @@ ip6_start (NMDhcpClient *client, _LOGT ("dhcp-client6: set %p", priv->client6); - if (info_only) + if (nm_dhcp_client_get_info_only (client)) sd_dhcp6_client_set_information_request (priv->client6, 1); /* NM stores the entire DUID which includes the uint16 "type", while systemd From 167a1d5f195de14c9d7d29804e34a4635546b0d8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 14 Feb 2018 15:14:10 +0100 Subject: [PATCH 04/10] dhcp: initialize use_fqdn and info_only paramters in constructor The two boolean properties do not need to be ever reset. It's nice to initialize such properties in the constructor and don't mutate them afterwards. Instead of adding two boolean GObject properties, add a new flags property that can encode these two values. In the end, properties are too cumbersome, let's combine them. --- src/dhcp/nm-dhcp-client.c | 28 +++++++++++++++++++--------- src/dhcp/nm-dhcp-client.h | 24 ++++++++++++++---------- src/dhcp/nm-dhcp-manager.c | 8 ++++++-- 3 files changed, 39 insertions(+), 21 deletions(-) diff --git a/src/dhcp/nm-dhcp-client.c b/src/dhcp/nm-dhcp-client.c index 317ddb5e6f..1fa8cca54c 100644 --- a/src/dhcp/nm-dhcp-client.c +++ b/src/dhcp/nm-dhcp-client.c @@ -52,15 +52,16 @@ enum { static guint signals[LAST_SIGNAL] = { 0 }; NM_GOBJECT_PROPERTIES_DEFINE_BASE ( - PROP_MULTI_IDX, PROP_ADDR_FAMILY, + PROP_FLAGS, + PROP_HWADDR, PROP_IFACE, PROP_IFINDEX, - PROP_HWADDR, - PROP_UUID, - PROP_ROUTE_TABLE, + PROP_MULTI_IDX, PROP_ROUTE_METRIC, + PROP_ROUTE_TABLE, PROP_TIMEOUT, + PROP_UUID, ); typedef struct _NMDhcpClientPrivate { @@ -502,7 +503,6 @@ nm_dhcp_client_start_ip4 (NMDhcpClient *self, const char *dhcp_client_id, const char *dhcp_anycast_addr, const char *hostname, - gboolean use_fqdn, const char *last_ip4_address) { NMDhcpClientPrivate *priv; @@ -523,7 +523,6 @@ nm_dhcp_client_start_ip4 (NMDhcpClient *self, g_clear_pointer (&priv->hostname, g_free); priv->hostname = g_strdup (hostname); - priv->use_fqdn = use_fqdn; return NM_DHCP_CLIENT_GET_CLASS (self)->ip4_start (self, dhcp_anycast_addr, last_ip4_address); } @@ -598,7 +597,6 @@ nm_dhcp_client_start_ip6 (NMDhcpClient *self, const char *dhcp_anycast_addr, const struct in6_addr *ll_addr, const char *hostname, - gboolean info_only, NMSettingIP6ConfigPrivacy privacy, guint needed_prefixes) { @@ -623,8 +621,6 @@ nm_dhcp_client_start_ip6 (NMDhcpClient *self, g_clear_pointer (&priv->hostname, g_free); priv->hostname = g_strdup (hostname); - priv->info_only = info_only; - if (priv->timeout == NM_DHCP_TIMEOUT_INFINITY) _LOGI ("activation: beginning transaction (no timeout)"); else @@ -929,8 +925,16 @@ set_property (GObject *object, guint prop_id, const GValue *value, GParamSpec *pspec) { NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE ((NMDhcpClient *) object); + guint flags; switch (prop_id) { + case PROP_FLAGS: + /* construct-only */ + flags = g_value_get_uint (value); + nm_assert ((flags & ~((guint) (NM_DHCP_CLIENT_FLAGS_INFO_ONLY | NM_DHCP_CLIENT_FLAGS_USE_FQDN))) == 0); + priv->info_only = NM_FLAGS_HAS (flags, NM_DHCP_CLIENT_FLAGS_INFO_ONLY); + priv->use_fqdn = NM_FLAGS_HAS (flags, NM_DHCP_CLIENT_FLAGS_USE_FQDN); + break; case PROP_MULTI_IDX: /* construct-only */ priv->multi_idx = g_value_get_pointer (value); @@ -1098,6 +1102,12 @@ nm_dhcp_client_class_init (NMDhcpClientClass *client_class) G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS); + obj_properties[PROP_FLAGS] = + g_param_spec_uint (NM_DHCP_CLIENT_FLAGS, "", "", + 0, G_MAXUINT32, 0, + G_PARAM_WRITABLE | G_PARAM_CONSTRUCT_ONLY | + G_PARAM_STATIC_STRINGS); + g_object_class_install_properties (object_class, _PROPERTY_ENUMS_LAST, obj_properties); signals[SIGNAL_STATE_CHANGED] = diff --git a/src/dhcp/nm-dhcp-client.h b/src/dhcp/nm-dhcp-client.h index 6616f1406f..f3b731cefc 100644 --- a/src/dhcp/nm-dhcp-client.h +++ b/src/dhcp/nm-dhcp-client.h @@ -34,15 +34,16 @@ #define NM_IS_DHCP_CLIENT_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), NM_TYPE_DHCP_CLIENT)) #define NM_DHCP_CLIENT_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_DHCP_CLIENT, NMDhcpClientClass)) -#define NM_DHCP_CLIENT_INTERFACE "iface" -#define NM_DHCP_CLIENT_ADDR_FAMILY "addr-family" -#define NM_DHCP_CLIENT_IFINDEX "ifindex" -#define NM_DHCP_CLIENT_HWADDR "hwaddr" -#define NM_DHCP_CLIENT_UUID "uuid" -#define NM_DHCP_CLIENT_ROUTE_TABLE "route-table" +#define NM_DHCP_CLIENT_ADDR_FAMILY "addr-family" +#define NM_DHCP_CLIENT_FLAGS "flags" +#define NM_DHCP_CLIENT_HWADDR "hwaddr" +#define NM_DHCP_CLIENT_IFINDEX "ifindex" +#define NM_DHCP_CLIENT_INTERFACE "iface" +#define NM_DHCP_CLIENT_MULTI_IDX "multi-idx" #define NM_DHCP_CLIENT_ROUTE_METRIC "route-metric" -#define NM_DHCP_CLIENT_TIMEOUT "timeout" -#define NM_DHCP_CLIENT_MULTI_IDX "multi-idx" +#define NM_DHCP_CLIENT_ROUTE_TABLE "route-table" +#define NM_DHCP_CLIENT_TIMEOUT "timeout" +#define NM_DHCP_CLIENT_UUID "uuid" #define NM_DHCP_CLIENT_SIGNAL_STATE_CHANGED "state-changed" #define NM_DHCP_CLIENT_SIGNAL_PREFIX_DELEGATED "prefix-delegated" @@ -67,6 +68,11 @@ typedef struct { CList dhcp_client_lst; } NMDhcpClient; +typedef enum { + NM_DHCP_CLIENT_FLAGS_INFO_ONLY = (1LL << 0), + NM_DHCP_CLIENT_FLAGS_USE_FQDN = (1LL << 1), +} NMDhcpClientFlags; + typedef struct { GObjectClass parent; @@ -141,14 +147,12 @@ gboolean nm_dhcp_client_start_ip4 (NMDhcpClient *self, const char *dhcp_client_id, const char *dhcp_anycast_addr, const char *hostname, - gboolean use_fqdn, const char *last_ip4_address); gboolean nm_dhcp_client_start_ip6 (NMDhcpClient *self, const char *dhcp_anycast_addr, const struct in6_addr *ll_addr, const char *hostname, - gboolean info_only, NMSettingIP6ConfigPrivacy privacy, guint needed_prefixes); diff --git a/src/dhcp/nm-dhcp-manager.c b/src/dhcp/nm-dhcp-manager.c index 020eeed578..0fcf1e2c83 100644 --- a/src/dhcp/nm-dhcp-manager.c +++ b/src/dhcp/nm-dhcp-manager.c @@ -205,15 +205,19 @@ client_start (NMDhcpManager *self, NM_DHCP_CLIENT_ROUTE_TABLE, (guint) route_table, NM_DHCP_CLIENT_ROUTE_METRIC, (guint) route_metric, NM_DHCP_CLIENT_TIMEOUT, (guint) timeout, + NM_DHCP_CLIENT_FLAGS, (guint) (0 + | (hostname_use_fqdn ? NM_DHCP_CLIENT_FLAGS_USE_FQDN : 0) + | (info_only ? NM_DHCP_CLIENT_FLAGS_INFO_ONLY : 0) + ), NULL); nm_assert (client && c_list_is_empty (&client->dhcp_client_lst)); c_list_link_tail (&priv->dhcp_client_lst_head, &client->dhcp_client_lst); g_signal_connect (client, NM_DHCP_CLIENT_SIGNAL_STATE_CHANGED, G_CALLBACK (client_state_changed), self); if (addr_family == AF_INET) - success = nm_dhcp_client_start_ip4 (client, dhcp_client_id, dhcp_anycast_addr, hostname, hostname_use_fqdn, last_ip4_address); + success = nm_dhcp_client_start_ip4 (client, dhcp_client_id, dhcp_anycast_addr, hostname, last_ip4_address); else - success = nm_dhcp_client_start_ip6 (client, dhcp_anycast_addr, ipv6_ll_addr, hostname, info_only, privacy, needed_prefixes); + success = nm_dhcp_client_start_ip6 (client, dhcp_anycast_addr, ipv6_ll_addr, hostname, privacy, needed_prefixes); if (!success) { remove_client_unref (self, client); From 1f08b01714c9b2cbf476c8cea41290469c6ac4ad Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 15 Feb 2018 11:25:04 +0100 Subject: [PATCH 05/10] platform: cleanup nm_platform_link_get_address() to return-early Avoid nested if-blocks, and instead check conditions and return early. --- src/platform/nm-platform.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 572f1c48fd..2c12359cfc 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -1340,30 +1340,26 @@ gconstpointer nm_platform_link_get_address (NMPlatform *self, int ifindex, size_t *length) { const NMPlatformLink *pllink; - gconstpointer a = NULL; - guint8 l = 0; _CHECK_SELF (self, klass, NULL); - if (length) - *length = 0; - g_return_val_if_fail (ifindex > 0, NULL); pllink = nm_platform_link_get (self, ifindex); - if (pllink && pllink->addr.len > 0) { - if (pllink->addr.len > NM_UTILS_HWADDR_LEN_MAX) { - if (length) - *length = 0; - g_return_val_if_reached (NULL); - } - a = pllink->addr.data; - l = pllink->addr.len; + + if ( !pllink + || pllink->addr.len <= 0) { + NM_SET_OUT (length, 0); + return NULL; } - if (length) - *length = l; - return a; + if (pllink->addr.len > NM_UTILS_HWADDR_LEN_MAX) { + NM_SET_OUT (length, 0); + g_return_val_if_reached (NULL); + } + + NM_SET_OUT (length, pllink->addr.len); + return pllink->addr.data; } /** From b0e9856196bc4b321228a586af4f2f7f0e0c7f15 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 14 Feb 2018 13:09:23 +0100 Subject: [PATCH 06/10] dhcp: refactor type of NMDhcpClient hwaddr to be GBytes GByteArray is a mutable array of bytes. For every practical purpose, the hwaddr property of NMDhcpClient is an immutable sequence of bytes. Thus, make it a GBytes. --- src/devices/nm-device.c | 43 ++++++++++++-------------------------- src/dhcp/nm-dhcp-client.c | 12 ++++------- src/dhcp/nm-dhcp-client.h | 2 +- src/dhcp/nm-dhcp-manager.c | 6 +++--- src/dhcp/nm-dhcp-manager.h | 4 ++-- src/dhcp/nm-dhcp-systemd.c | 37 +++++++++++++++++--------------- src/nm-iface-helper.c | 12 ++--------- src/platform/nm-platform.h | 14 +++++++++++++ 8 files changed, 59 insertions(+), 71 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index ae64229cda..01ef15b4f6 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -5745,8 +5745,7 @@ ipv4_dad_start (NMDevice *self, NMIP4Config **configs, ArpingCallback cb) ArpingData *data; guint timeout; gboolean ret, addr_found; - const guint8 *hw_addr; - size_t hw_addr_len = 0; + const guint8 *hwaddr_arr; GError *error = NULL; guint i; @@ -5762,13 +5761,12 @@ ipv4_dad_start (NMDevice *self, NMIP4Config **configs, ArpingCallback cb) } timeout = get_ipv4_dad_timeout (self); - hw_addr = nm_platform_link_get_address (nm_device_get_platform (self), - nm_device_get_ip_ifindex (self), - &hw_addr_len); + hwaddr_arr = nm_platform_link_get_address (nm_device_get_platform (self), + nm_device_get_ip_ifindex (self), + NULL); if ( !timeout - || !hw_addr - || !hw_addr_len + || !hwaddr_arr || !addr_found || nm_device_sys_iface_state_is_external_or_assume (self)) { @@ -6399,9 +6397,7 @@ dhcp4_start (NMDevice *self) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); NMSettingIPConfig *s_ip4; - const guint8 *hw_addr; - size_t hw_addr_len = 0; - GByteArray *tmp = NULL; + gs_unref_bytes GBytes *hwaddr = NULL; NMConnection *connection; connection = nm_device_get_applied_connection (self); @@ -6413,11 +6409,8 @@ dhcp4_start (NMDevice *self) nm_exported_object_clear_and_unexport (&priv->dhcp4.config); priv->dhcp4.config = nm_dhcp4_config_new (); - hw_addr = nm_platform_link_get_address (nm_device_get_platform (self), nm_device_get_ip_ifindex (self), &hw_addr_len); - if (hw_addr_len) { - tmp = g_byte_array_sized_new (hw_addr_len); - g_byte_array_append (tmp, hw_addr, hw_addr_len); - } + hwaddr = nm_platform_link_get_address_as_bytes (nm_device_get_platform (self), + nm_device_get_ip_ifindex (self)); /* Begin DHCP on the interface */ g_warn_if_fail (priv->dhcp4.client == NULL); @@ -6425,7 +6418,7 @@ dhcp4_start (NMDevice *self) nm_netns_get_multi_idx (nm_device_get_netns (self)), nm_device_get_ip_iface (self), nm_device_get_ip_ifindex (self), - tmp, + hwaddr, nm_connection_get_uuid (connection), nm_device_get_route_table (self, AF_INET, TRUE), nm_device_get_route_metric (self, AF_INET), @@ -6437,9 +6430,6 @@ dhcp4_start (NMDevice *self) priv->dhcp_anycast_address, NULL); - if (tmp) - g_byte_array_free (tmp, TRUE); - if (!priv->dhcp4.client) return NM_ACT_STAGE_RETURN_FAILURE; @@ -7128,9 +7118,7 @@ dhcp6_start_with_link_ready (NMDevice *self, NMConnection *connection) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); NMSettingIPConfig *s_ip6; - GByteArray *tmp = NULL; - const guint8 *hw_addr; - size_t hw_addr_len = 0; + gs_unref_bytes GBytes *hwaddr = NULL; const NMPlatformIP6Address *ll_addr = NULL; g_assert (connection); @@ -7145,17 +7133,14 @@ dhcp6_start_with_link_ready (NMDevice *self, NMConnection *connection) return FALSE; } - hw_addr = nm_platform_link_get_address (nm_device_get_platform (self), nm_device_get_ip_ifindex (self), &hw_addr_len); - if (hw_addr_len) { - tmp = g_byte_array_sized_new (hw_addr_len); - g_byte_array_append (tmp, hw_addr, hw_addr_len); - } + hwaddr = nm_platform_link_get_address_as_bytes (nm_device_get_platform (self), + nm_device_get_ip_ifindex (self)); priv->dhcp6.client = nm_dhcp_manager_start_ip6 (nm_dhcp_manager_get (), nm_device_get_multi_index (self), nm_device_get_ip_iface (self), nm_device_get_ip_ifindex (self), - tmp, + hwaddr, &ll_addr->address, nm_connection_get_uuid (connection), nm_device_get_route_table (self, AF_INET6, TRUE), @@ -7167,8 +7152,6 @@ dhcp6_start_with_link_ready (NMDevice *self, NMConnection *connection) (priv->dhcp6.mode == NM_NDISC_DHCP_LEVEL_OTHERCONF) ? TRUE : FALSE, nm_setting_ip6_config_get_ip6_privacy (NM_SETTING_IP6_CONFIG (s_ip6)), priv->dhcp6.needed_prefixes); - if (tmp) - g_byte_array_free (tmp, TRUE); if (priv->dhcp6.client) { priv->dhcp6.state_sigid = g_signal_connect (priv->dhcp6.client, diff --git a/src/dhcp/nm-dhcp-client.c b/src/dhcp/nm-dhcp-client.c index 1fa8cca54c..c758417f63 100644 --- a/src/dhcp/nm-dhcp-client.c +++ b/src/dhcp/nm-dhcp-client.c @@ -67,7 +67,7 @@ NM_GOBJECT_PROPERTIES_DEFINE_BASE ( typedef struct _NMDhcpClientPrivate { NMDedupMultiIndex *multi_idx; char * iface; - GByteArray * hwaddr; + GBytes * hwaddr; char * uuid; GByteArray * duid; GBytes * client_id; @@ -147,7 +147,7 @@ nm_dhcp_client_get_duid (NMDhcpClient *self) return NM_DHCP_CLIENT_GET_PRIVATE (self)->duid; } -const GByteArray * +GBytes * nm_dhcp_client_get_hw_addr (NMDhcpClient *self) { g_return_val_if_fail (NM_IS_DHCP_CLIENT (self), NULL); @@ -1018,11 +1018,7 @@ dispose (GObject *object) g_clear_pointer (&priv->hostname, g_free); g_clear_pointer (&priv->uuid, g_free); g_clear_pointer (&priv->client_id, g_bytes_unref); - - if (priv->hwaddr) { - g_byte_array_free (priv->hwaddr, TRUE); - priv->hwaddr = NULL; - } + g_clear_pointer (&priv->hwaddr, g_bytes_unref); if (priv->duid) { g_byte_array_free (priv->duid, TRUE); @@ -1068,7 +1064,7 @@ nm_dhcp_client_class_init (NMDhcpClientClass *client_class) obj_properties[PROP_HWADDR] = g_param_spec_boxed (NM_DHCP_CLIENT_HWADDR, "", "", - G_TYPE_BYTE_ARRAY, + G_TYPE_BYTES, G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS); diff --git a/src/dhcp/nm-dhcp-client.h b/src/dhcp/nm-dhcp-client.h index f3b731cefc..f1ce933f83 100644 --- a/src/dhcp/nm-dhcp-client.h +++ b/src/dhcp/nm-dhcp-client.h @@ -127,7 +127,7 @@ const char *nm_dhcp_client_get_uuid (NMDhcpClient *self); const GByteArray *nm_dhcp_client_get_duid (NMDhcpClient *self); -const GByteArray *nm_dhcp_client_get_hw_addr (NMDhcpClient *self); +GBytes *nm_dhcp_client_get_hw_addr (NMDhcpClient *self); guint32 nm_dhcp_client_get_route_table (NMDhcpClient *self); diff --git a/src/dhcp/nm-dhcp-manager.c b/src/dhcp/nm-dhcp-manager.c index 0fcf1e2c83..fc88f54978 100644 --- a/src/dhcp/nm-dhcp-manager.c +++ b/src/dhcp/nm-dhcp-manager.c @@ -158,7 +158,7 @@ client_start (NMDhcpManager *self, NMDedupMultiIndex *multi_idx, const char *iface, int ifindex, - const GByteArray *hwaddr, + GBytes *hwaddr, const char *uuid, guint32 route_table, guint32 route_metric, @@ -233,7 +233,7 @@ nm_dhcp_manager_start_ip4 (NMDhcpManager *self, NMDedupMultiIndex *multi_idx, const char *iface, int ifindex, - const GByteArray *hwaddr, + GBytes *hwaddr, const char *uuid, guint32 route_table, guint32 route_metric, @@ -289,7 +289,7 @@ nm_dhcp_manager_start_ip6 (NMDhcpManager *self, NMDedupMultiIndex *multi_idx, const char *iface, int ifindex, - const GByteArray *hwaddr, + GBytes *hwaddr, const struct in6_addr *ll_addr, const char *uuid, guint32 route_table, diff --git a/src/dhcp/nm-dhcp-manager.h b/src/dhcp/nm-dhcp-manager.h index 078117ffab..afb13b1d8b 100644 --- a/src/dhcp/nm-dhcp-manager.h +++ b/src/dhcp/nm-dhcp-manager.h @@ -49,7 +49,7 @@ NMDhcpClient * nm_dhcp_manager_start_ip4 (NMDhcpManager *manager, struct _NMDedupMultiIndex *multi_idx, const char *iface, int ifindex, - const GByteArray *hwaddr, + GBytes *hwaddr, const char *uuid, guint32 route_table, guint32 route_metric, @@ -65,7 +65,7 @@ NMDhcpClient * nm_dhcp_manager_start_ip6 (NMDhcpManager *manager, struct _NMDedupMultiIndex *multi_idx, const char *iface, int ifindex, - const GByteArray *hwaddr, + GBytes *hwaddr, const struct in6_addr *ll_addr, const char *uuid, guint32 route_table, diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c index a7222e4863..f0ad8c06f0 100644 --- a/src/dhcp/nm-dhcp-systemd.c +++ b/src/dhcp/nm-dhcp-systemd.c @@ -583,14 +583,16 @@ dhcp_event_cb (sd_dhcp_client *client, int event, gpointer user_data) } static guint16 -get_arp_type (const GByteArray *hwaddr) +get_arp_type (GBytes *hwaddr) { - if (hwaddr->len == ETH_ALEN) + switch (g_bytes_get_size (hwaddr)) { + case ETH_ALEN: return ARPHRD_ETHER; - else if (hwaddr->len == INFINIBAND_ALEN) + case INFINIBAND_ALEN: return ARPHRD_INFINIBAND; - else + default: return ARPHRD_NONE; + } } static gboolean @@ -599,7 +601,7 @@ ip4_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const char *last NMDhcpSystemd *self = NM_DHCP_SYSTEMD (client); NMDhcpSystemdPrivate *priv = NM_DHCP_SYSTEMD_GET_PRIVATE (self); const char *iface = nm_dhcp_client_get_iface (client); - const GByteArray *hwaddr; + GBytes *hwaddr; sd_dhcp_lease *lease = NULL; GBytes *override_client_id; const uint8_t *client_id = NULL; @@ -608,7 +610,6 @@ ip4_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const char *last const char *hostname; int r, i; gboolean success = FALSE; - guint16 arp_type; g_assert (priv->client4 == NULL); g_assert (priv->client6 == NULL); @@ -632,16 +633,14 @@ ip4_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const char *last hwaddr = nm_dhcp_client_get_hw_addr (client); if (hwaddr) { - arp_type= get_arp_type (hwaddr); - if (arp_type == ARPHRD_NONE) { - _LOGW ("failed to determine ARP type"); - goto error; - } + const uint8_t *data; + gsize len; + data = g_bytes_get_data (hwaddr, &len); r = sd_dhcp_client_set_mac (priv->client4, - hwaddr->data, - hwaddr->len, - arp_type); + data, + len, + get_arp_type (hwaddr)); if (r < 0) { _LOGW ("failed to set MAC address (%d)", r); goto error; @@ -906,7 +905,7 @@ ip6_start (NMDhcpClient *client, NMDhcpSystemd *self = NM_DHCP_SYSTEMD (client); NMDhcpSystemdPrivate *priv = NM_DHCP_SYSTEMD_GET_PRIVATE (self); const char *iface = nm_dhcp_client_get_iface (client); - const GByteArray *hwaddr; + GBytes *hwaddr; const char *hostname; int r, i; @@ -953,9 +952,13 @@ ip6_start (NMDhcpClient *client, hwaddr = nm_dhcp_client_get_hw_addr (client); if (hwaddr) { + const uint8_t *data; + gsize len; + + data = g_bytes_get_data (hwaddr, &len); r = sd_dhcp6_client_set_mac (priv->client6, - hwaddr->data, - hwaddr->len, + data, + len, get_arp_type (hwaddr)); if (r < 0) { _LOGW ("failed to set MAC address (%d)", r); diff --git a/src/nm-iface-helper.c b/src/nm-iface-helper.c index a7769ab3cd..996b060fd3 100644 --- a/src/nm-iface-helper.c +++ b/src/nm-iface-helper.c @@ -340,9 +340,7 @@ main (int argc, char *argv[]) gs_free char *pidfile = NULL; gs_unref_object NMDhcpClient *dhcp4_client = NULL; gs_unref_object NMNDisc *ndisc = NULL; - GByteArray *hwaddr = NULL; - size_t hwaddr_len = 0; - gconstpointer tmp; + gs_unref_bytes GBytes *hwaddr = NULL; gs_free NMUtilsIPv6IfaceId *iid = NULL; guint sd_id; char sysctl_path_buf[NM_UTILS_SYSCTL_IP_CONF_PATH_BUFSIZE]; @@ -429,11 +427,7 @@ main (int argc, char *argv[]) /* Set up platform interaction layer */ nm_linux_platform_setup (); - tmp = nm_platform_link_get_address (NM_PLATFORM_GET, gl.ifindex, &hwaddr_len); - if (tmp) { - hwaddr = g_byte_array_sized_new (hwaddr_len); - g_byte_array_append (hwaddr, tmp, hwaddr_len); - } + hwaddr = nm_platform_link_get_address_as_bytes (NM_PLATFORM_GET, gl.ifindex); if (global_opt.iid_str) { GBytes *bytes; @@ -521,8 +515,6 @@ main (int argc, char *argv[]) g_main_loop_run (gl.main_loop); - g_clear_pointer (&hwaddr, g_byte_array_unref); - if (pidfile && wrote_pidfile) unlink (pidfile); diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index 3cbf8c9990..a545485183 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -1080,7 +1080,21 @@ gboolean nm_platform_link_is_connected (NMPlatform *self, int ifindex); gboolean nm_platform_link_uses_arp (NMPlatform *self, int ifindex); guint32 nm_platform_link_get_mtu (NMPlatform *self, int ifindex); gboolean nm_platform_link_get_user_ipv6ll_enabled (NMPlatform *self, int ifindex); + gconstpointer nm_platform_link_get_address (NMPlatform *self, int ifindex, size_t *length); + +static inline GBytes * +nm_platform_link_get_address_as_bytes (NMPlatform *self, int ifindex) +{ + gconstpointer p; + gsize l; + + p = nm_platform_link_get_address (self, ifindex, &l); + return p + ? g_bytes_new (p, l) + : NULL; +} + int nm_platform_link_get_master (NMPlatform *self, int slave); gboolean nm_platform_link_can_assume (NMPlatform *self, int ifindex); From 578c4af907cd1f5a93aacb024f983b7eb5b40362 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 14 Feb 2018 13:09:23 +0100 Subject: [PATCH 07/10] dhcp: refactor type of NMDhcpClient duid to be GBytes GBytes is immutable. It's better suited to contain the duid parameter then a GByteArray. --- src/dhcp/nm-dhcp-client.c | 43 ++++++++++++----------------- src/dhcp/nm-dhcp-client.h | 8 +++--- src/dhcp/nm-dhcp-dhclient-utils.c | 23 +++++++++------ src/dhcp/nm-dhcp-dhclient-utils.h | 6 ++-- src/dhcp/nm-dhcp-dhclient.c | 12 ++++---- src/dhcp/nm-dhcp-dhcpcanon.c | 6 ++-- src/dhcp/nm-dhcp-dhcpcd.c | 4 +-- src/dhcp/nm-dhcp-systemd.c | 17 ++++++++---- src/dhcp/nm-dhcp-utils.c | 8 ++++-- src/dhcp/nm-dhcp-utils.h | 2 +- src/dhcp/tests/test-dhcp-dhclient.c | 30 +++++++++++--------- 11 files changed, 87 insertions(+), 72 deletions(-) diff --git a/src/dhcp/nm-dhcp-client.c b/src/dhcp/nm-dhcp-client.c index c758417f63..6396a925f5 100644 --- a/src/dhcp/nm-dhcp-client.c +++ b/src/dhcp/nm-dhcp-client.c @@ -69,7 +69,7 @@ typedef struct _NMDhcpClientPrivate { char * iface; GBytes * hwaddr; char * uuid; - GByteArray * duid; + GBytes * duid; GBytes * client_id; char * hostname; pid_t pid; @@ -139,7 +139,7 @@ nm_dhcp_client_get_uuid (NMDhcpClient *self) return NM_DHCP_CLIENT_GET_PRIVATE (self)->uuid; } -const GByteArray * +GBytes * nm_dhcp_client_get_duid (NMDhcpClient *self) { g_return_val_if_fail (NM_IS_DHCP_CLIENT (self), NULL); @@ -354,7 +354,7 @@ nm_dhcp_client_stop_pid (pid_t pid, const char *iface) } static void -stop (NMDhcpClient *self, gboolean release, const GByteArray *duid) +stop (NMDhcpClient *self, gboolean release, GBytes *duid) { NMDhcpClientPrivate *priv; @@ -527,10 +527,11 @@ nm_dhcp_client_start_ip4 (NMDhcpClient *self, return NM_DHCP_CLIENT_GET_CLASS (self)->ip4_start (self, dhcp_anycast_addr, last_ip4_address); } -static GByteArray * +static GBytes * generate_duid_from_machine_id (void) { - GByteArray *duid; + const int DUID_SIZE = 18; + guint8 *duid_buffer; GChecksum *sum; guint8 buffer[32]; /* SHA256 digest size */ gsize sumlen = sizeof (buffer); @@ -538,6 +539,7 @@ generate_duid_from_machine_id (void) uuid_t uuid; gs_free char *machine_id_s = NULL; gs_free char *str = NULL; + GBytes *duid; machine_id_s = nm_utils_machine_id_read (); if (nm_utils_machine_id_parse (machine_id_s, uuid)) { @@ -560,36 +562,31 @@ generate_duid_from_machine_id (void) * u16: type (DUID-UUID = 4) * u8[16]: UUID bytes */ - duid = g_byte_array_sized_new (18); - g_byte_array_append (duid, (guint8 *) &duid_type, sizeof (duid_type)); + duid_buffer = g_malloc (DUID_SIZE); + + G_STATIC_ASSERT_EXPR (sizeof (duid_type) == 2); + memcpy (&duid_buffer[0], &duid_type, 2); /* Since SHA256 is 256 bits, but UUID is 128 bits, we just take the first * 128 bits of the SHA256 as the DUID-UUID. */ - g_byte_array_append (duid, buffer, 16); + memcpy (&duid_buffer[2], buffer, 16); + duid = g_bytes_new_take (duid_buffer, DUID_SIZE); nm_log_dbg (LOGD_DHCP, "dhcp: generated DUID %s", (str = nm_dhcp_utils_duid_to_string (duid))); return duid; } -static GByteArray * +static GBytes * get_duid (NMDhcpClient *self) { - static GByteArray *duid = NULL; - GByteArray *copy = NULL; + static GBytes *duid = NULL; - if (G_UNLIKELY (duid == NULL)) { + if (G_UNLIKELY (!duid)) duid = generate_duid_from_machine_id (); - g_assert (duid); - } - if (G_LIKELY (duid)) { - copy = g_byte_array_sized_new (duid->len); - g_byte_array_append (copy, duid->data, duid->len); - } - - return copy; + return g_bytes_ref (duid); } gboolean @@ -1019,11 +1016,7 @@ dispose (GObject *object) g_clear_pointer (&priv->uuid, g_free); g_clear_pointer (&priv->client_id, g_bytes_unref); g_clear_pointer (&priv->hwaddr, g_bytes_unref); - - if (priv->duid) { - g_byte_array_free (priv->duid, TRUE); - priv->duid = NULL; - } + g_clear_pointer (&priv->duid, g_bytes_unref); G_OBJECT_CLASS (nm_dhcp_client_parent_class)->dispose (object); diff --git a/src/dhcp/nm-dhcp-client.h b/src/dhcp/nm-dhcp-client.h index f1ce933f83..25c433b9c3 100644 --- a/src/dhcp/nm-dhcp-client.h +++ b/src/dhcp/nm-dhcp-client.h @@ -86,12 +86,12 @@ typedef struct { const char *anycast_addr, const struct in6_addr *ll_addr, NMSettingIP6ConfigPrivacy privacy, - const GByteArray *duid, + GBytes *duid, guint needed_prefixes); void (*stop) (NMDhcpClient *self, gboolean release, - const GByteArray *duid); + GBytes *duid); /** * get_duid: @@ -102,7 +102,7 @@ typedef struct { * representation of the DUID. If no DUID is found, %NULL should be * returned. */ - GByteArray * (*get_duid) (NMDhcpClient *self); + GBytes *(*get_duid) (NMDhcpClient *self); /* Signals */ void (*state_changed) (NMDhcpClient *self, @@ -125,7 +125,7 @@ int nm_dhcp_client_get_ifindex (NMDhcpClient *self); const char *nm_dhcp_client_get_uuid (NMDhcpClient *self); -const GByteArray *nm_dhcp_client_get_duid (NMDhcpClient *self); +GBytes *nm_dhcp_client_get_duid (NMDhcpClient *self); GBytes *nm_dhcp_client_get_hw_addr (NMDhcpClient *self); diff --git a/src/dhcp/nm-dhcp-dhclient-utils.c b/src/dhcp/nm-dhcp-dhclient-utils.c index ae7691f5e8..1116e7514e 100644 --- a/src/dhcp/nm-dhcp-dhclient-utils.c +++ b/src/dhcp/nm-dhcp-dhclient-utils.c @@ -447,14 +447,20 @@ nm_dhcp_dhclient_create_config (const char *interface, /* Roughly follow what dhclient's quotify_buf() and pretty_escape() functions do */ char * -nm_dhcp_dhclient_escape_duid (const GByteArray *duid) +nm_dhcp_dhclient_escape_duid (GBytes *duid) { char *escaped; - const guint8 *s = duid->data; + const guint8 *s, *s0; + gsize len; char *d; - d = escaped = g_malloc0 ((duid->len * 4) + 1); - while (s < (duid->data + duid->len)) { + g_return_val_if_fail (duid, NULL); + + s0 = g_bytes_get_data (duid, &len); + s = s0; + + d = escaped = g_malloc ((len * 4) + 1); + while (s < (s0 + len)) { if (!g_ascii_isprint (*s)) { *d++ = '\\'; *d++ = '0' + ((*s >> 6) & 0x7); @@ -468,6 +474,7 @@ nm_dhcp_dhclient_escape_duid (const GByteArray *duid) } else *d++ = *s++; } + *d++ = '\0'; return escaped; } @@ -479,7 +486,7 @@ isoctal (const guint8 *p) && p[2] >= '0' && p[2] <= '7'); } -GByteArray * +GBytes * nm_dhcp_dhclient_unescape_duid (const char *duid) { GByteArray *unescaped; @@ -510,7 +517,7 @@ nm_dhcp_dhclient_unescape_duid (const char *duid) g_byte_array_append (unescaped, &p[i], 1); } - return unescaped; + return g_byte_array_free_to_bytes (unescaped); error: g_byte_array_free (unescaped, TRUE); @@ -519,10 +526,10 @@ error: #define DUID_PREFIX "default-duid \"" -GByteArray * +GBytes * nm_dhcp_dhclient_read_duid (const char *leasefile, GError **error) { - GByteArray *duid = NULL; + GBytes *duid = NULL; char *contents; char **line, **split, *p, *e; diff --git a/src/dhcp/nm-dhcp-dhclient-utils.h b/src/dhcp/nm-dhcp-dhclient-utils.h index 94de196362..d67a4f35d3 100644 --- a/src/dhcp/nm-dhcp-dhclient-utils.h +++ b/src/dhcp/nm-dhcp-dhclient-utils.h @@ -33,11 +33,11 @@ char *nm_dhcp_dhclient_create_config (const char *interface, const char *orig_contents, GBytes **out_new_client_id); -char *nm_dhcp_dhclient_escape_duid (const GByteArray *duid); +char *nm_dhcp_dhclient_escape_duid (GBytes *duid); -GByteArray *nm_dhcp_dhclient_unescape_duid (const char *duid); +GBytes *nm_dhcp_dhclient_unescape_duid (const char *duid); -GByteArray *nm_dhcp_dhclient_read_duid (const char *leasefile, GError **error); +GBytes *nm_dhcp_dhclient_read_duid (const char *leasefile, GError **error); gboolean nm_dhcp_dhclient_save_duid (const char *leasefile, const char *escaped_duid, diff --git a/src/dhcp/nm-dhcp-dhclient.c b/src/dhcp/nm-dhcp-dhclient.c index e93ab9a099..56b74d43be 100644 --- a/src/dhcp/nm-dhcp-dhclient.c +++ b/src/dhcp/nm-dhcp-dhclient.c @@ -338,7 +338,7 @@ create_dhclient_config (NMDhcpDhclient *self, static gboolean dhclient_start (NMDhcpClient *client, const char *mode_opt, - const GByteArray *duid, + GBytes *duid, gboolean release, pid_t *out_pid, int prefixes) @@ -534,7 +534,7 @@ ip6_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const struct in6_addr *ll_addr, NMSettingIP6ConfigPrivacy privacy, - const GByteArray *duid, + GBytes *duid, guint needed_prefixes) { NMDhcpDhclient *self = NM_DHCP_DHCLIENT (client); @@ -562,7 +562,7 @@ ip6_start (NMDhcpClient *client, } static void -stop (NMDhcpClient *client, gboolean release, const GByteArray *duid) +stop (NMDhcpClient *client, gboolean release, GBytes *duid) { NMDhcpDhclient *self = NM_DHCP_DHCLIENT (client); NMDhcpDhclientPrivate *priv = NM_DHCP_DHCLIENT_GET_PRIVATE (self); @@ -607,12 +607,12 @@ state_changed (NMDhcpClient *client, nm_dhcp_client_set_client_id (client, client_id); } -static GByteArray * +static GBytes * get_duid (NMDhcpClient *client) { NMDhcpDhclient *self = NM_DHCP_DHCLIENT (client); NMDhcpDhclientPrivate *priv = NM_DHCP_DHCLIENT_GET_PRIVATE (self); - GByteArray *duid = NULL; + GBytes *duid = NULL; char *leasefile; GError *error = NULL; @@ -646,7 +646,7 @@ get_duid (NMDhcpClient *client) } /* return our DUID, otherwise let the parent class make a default DUID */ - return duid ? duid : NM_DHCP_CLIENT_CLASS (nm_dhcp_dhclient_parent_class)->get_duid (client); + return duid ?: NM_DHCP_CLIENT_CLASS (nm_dhcp_dhclient_parent_class)->get_duid (client); } /*****************************************************************************/ diff --git a/src/dhcp/nm-dhcp-dhcpcanon.c b/src/dhcp/nm-dhcp-dhcpcanon.c index 5c8f0d7d26..66ce45c638 100644 --- a/src/dhcp/nm-dhcp-dhcpcanon.c +++ b/src/dhcp/nm-dhcp-dhcpcanon.c @@ -80,7 +80,7 @@ nm_dhcp_dhcpcanon_get_path (void) static gboolean dhcpcanon_start (NMDhcpClient *client, const char *mode_opt, - const GByteArray *duid, + GBytes *duid, gboolean release, pid_t *out_pid, int prefixes) @@ -180,7 +180,7 @@ ip6_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const struct in6_addr *ll_addr, NMSettingIP6ConfigPrivacy privacy, - const GByteArray *duid, + GBytes *duid, guint needed_prefixes) { NMDhcpDhcpcanon *self = NM_DHCP_DHCPCANON (client); @@ -189,7 +189,7 @@ ip6_start (NMDhcpClient *client, return FALSE; } static void -stop (NMDhcpClient *client, gboolean release, const GByteArray *duid) +stop (NMDhcpClient *client, gboolean release, GBytes *duid) { NMDhcpDhcpcanon *self = NM_DHCP_DHCPCANON (client); NMDhcpDhcpcanonPrivate *priv = NM_DHCP_DHCPCANON_GET_PRIVATE (self); diff --git a/src/dhcp/nm-dhcp-dhcpcd.c b/src/dhcp/nm-dhcp-dhcpcd.c index 1eb4398aa9..8e4ebb1968 100644 --- a/src/dhcp/nm-dhcp-dhcpcd.c +++ b/src/dhcp/nm-dhcp-dhcpcd.c @@ -178,7 +178,7 @@ ip6_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const struct in6_addr *ll_addr, NMSettingIP6ConfigPrivacy privacy, - const GByteArray *duid, + GBytes *duid, guint needed_prefixes) { NMDhcpDhcpcd *self = NM_DHCP_DHCPCD (client); @@ -188,7 +188,7 @@ ip6_start (NMDhcpClient *client, } static void -stop (NMDhcpClient *client, gboolean release, const GByteArray *duid) +stop (NMDhcpClient *client, gboolean release, GBytes *duid) { NMDhcpDhcpcd *self = NM_DHCP_DHCPCD (client); NMDhcpDhcpcdPrivate *priv = NM_DHCP_DHCPCD_GET_PRIVATE (self); diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c index f0ad8c06f0..db392c421b 100644 --- a/src/dhcp/nm-dhcp-systemd.c +++ b/src/dhcp/nm-dhcp-systemd.c @@ -899,7 +899,7 @@ ip6_start (NMDhcpClient *client, const char *dhcp_anycast_addr, const struct in6_addr *ll_addr, NMSettingIP6ConfigPrivacy privacy, - const GByteArray *duid, + GBytes *duid, guint needed_prefixes) { NMDhcpSystemd *self = NM_DHCP_SYSTEMD (client); @@ -908,11 +908,18 @@ ip6_start (NMDhcpClient *client, GBytes *hwaddr; const char *hostname; int r, i; + const guint16 *duid_arr; + gsize duid_len; g_assert (priv->client4 == NULL); g_assert (priv->client6 == NULL); g_return_val_if_fail (duid != NULL, FALSE); + G_STATIC_ASSERT_EXPR (sizeof (duid_arr[0]) == 2); + duid_arr = g_bytes_get_data (duid, &duid_len); + if (!duid_arr || duid_len < 2) + g_return_val_if_reached (FALSE); + g_free (priv->lease_file); priv->lease_file = get_leasefile_path (AF_INET6, iface, nm_dhcp_client_get_uuid (client)); @@ -936,9 +943,9 @@ ip6_start (NMDhcpClient *client, * wants the type passed separately from the following data. */ r = sd_dhcp6_client_set_duid (priv->client6, - ntohs (((const guint16 *) duid->data)[0]), - duid->data + 2, - duid->len - 2); + ntohs (duid_arr[0]), + &duid_arr[1], + duid_len - 2); if (r < 0) { _LOGW ("failed to set DUID (%d)", r); return FALSE; @@ -1014,7 +1021,7 @@ error: } static void -stop (NMDhcpClient *client, gboolean release, const GByteArray *duid) +stop (NMDhcpClient *client, gboolean release, GBytes *duid) { NMDhcpSystemd *self = NM_DHCP_SYSTEMD (client); NMDhcpSystemdPrivate *priv = NM_DHCP_SYSTEMD_GET_PRIVATE (self); diff --git a/src/dhcp/nm-dhcp-utils.c b/src/dhcp/nm-dhcp-utils.c index 50ca2abe2d..9185a135a2 100644 --- a/src/dhcp/nm-dhcp-utils.c +++ b/src/dhcp/nm-dhcp-utils.c @@ -721,11 +721,15 @@ error: } char * -nm_dhcp_utils_duid_to_string (const GByteArray *duid) +nm_dhcp_utils_duid_to_string (GBytes *duid) { + gconstpointer data; + gsize len; + g_return_val_if_fail (duid != NULL, NULL); - return _nm_utils_bin2str (duid->data, duid->len, FALSE); + data = g_bytes_get_data (duid, &len); + return _nm_utils_bin2str (data, len, FALSE); } /** diff --git a/src/dhcp/nm-dhcp-utils.h b/src/dhcp/nm-dhcp-utils.h index 32140f481d..5c127bd194 100644 --- a/src/dhcp/nm-dhcp-utils.h +++ b/src/dhcp/nm-dhcp-utils.h @@ -39,7 +39,7 @@ NMIP6Config *nm_dhcp_utils_ip6_config_from_options (struct _NMDedupMultiIndex *m NMPlatformIP6Address nm_dhcp_utils_ip6_prefix_from_options (GHashTable *options); -char * nm_dhcp_utils_duid_to_string (const GByteArray *duid); +char *nm_dhcp_utils_duid_to_string (GBytes *duid); GBytes * nm_dhcp_utils_client_id_string_to_bytes (const char *client_id); diff --git a/src/dhcp/tests/test-dhcp-dhclient.c b/src/dhcp/tests/test-dhcp-dhclient.c index 6b205cc02e..d092dba44f 100644 --- a/src/dhcp/tests/test-dhcp-dhclient.c +++ b/src/dhcp/tests/test-dhcp-dhclient.c @@ -585,23 +585,26 @@ test_existing_multiline_alsoreq (void) static void test_one_duid (const char *escaped, const guint8 *unescaped, guint len) { - GByteArray *t; + GBytes *t; char *w; + gsize t_len; + gconstpointer t_arr; t = nm_dhcp_dhclient_unescape_duid (escaped); g_assert (t); - g_assert_cmpint (t->len, ==, len); - g_assert_cmpint (memcmp (t->data, unescaped, len), ==, 0); - g_byte_array_free (t, TRUE); + t_arr = g_bytes_get_data (t, &t_len); + g_assert (t_arr); + g_assert_cmpint (t_len, ==, len); + g_assert_cmpint (memcmp (t_arr, unescaped, len), ==, 0); + g_bytes_unref (t); - t = g_byte_array_sized_new (len); - g_byte_array_append (t, unescaped, len); + t = g_bytes_new_static (unescaped, len); w = nm_dhcp_dhclient_escape_duid (t); g_assert (w); g_assert_cmpint (strlen (escaped), ==, strlen (w)); g_assert_cmpstr (escaped, ==, w); - g_byte_array_free (t, TRUE); + g_bytes_unref (t); g_free (w); } @@ -640,22 +643,23 @@ test_read_duid_from_leasefile (void) { const guint8 expected[] = { 0x00, 0x01, 0x00, 0x01, 0x18, 0x79, 0xa6, 0x13, 0x60, 0x67, 0x20, 0xec, 0x4c, 0x70 }; - GByteArray *duid; + gs_unref_bytes GBytes *duid = NULL; GError *error = NULL; + gconstpointer duid_arr; + gsize duid_len; duid = nm_dhcp_dhclient_read_duid (TESTDIR "/test-dhclient-duid.leases", &error); g_assert_no_error (error); g_assert (duid); - g_assert_cmpint (duid->len, ==, sizeof (expected)); - g_assert_cmpint (memcmp (duid->data, expected, duid->len), ==, 0); - - g_byte_array_free (duid, TRUE); + duid_arr = g_bytes_get_data (duid, &duid_len); + g_assert_cmpint (duid_len, ==, sizeof (expected)); + g_assert_cmpint (memcmp (duid_arr, expected, duid_len), ==, 0); } static void test_read_commented_duid_from_leasefile (void) { - GByteArray *duid; + GBytes *duid; GError *error = NULL; duid = nm_dhcp_dhclient_read_duid (TESTDIR "/test-dhclient-commented-duid.leases", &error); From 7de078a3940ec31bca553aced4e5d56bdc5c661c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 14 Feb 2018 15:26:40 +0100 Subject: [PATCH 08/10] dhcp: inject client-id in GBytes format from NMDevice to nm_dhcp_manager_start_ip4() Convert the string representation of ipv4.dhcp-client-id property already in NMDevice to a GBytes. Next, we will support more client ID modes, and we will need the NMDevice context to generate the client id. --- src/devices/nm-device.c | 21 +++++++++++++++++++-- src/dhcp/nm-dhcp-client.c | 18 ++---------------- src/dhcp/nm-dhcp-client.h | 4 +--- src/dhcp/nm-dhcp-manager.c | 5 +++-- src/dhcp/nm-dhcp-manager.h | 2 +- src/nm-iface-helper.c | 13 ++++++++++++- 6 files changed, 38 insertions(+), 25 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 01ef15b4f6..fb6c980c04 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -48,6 +48,7 @@ #include "ndisc/nm-ndisc.h" #include "ndisc/nm-lndp-ndisc.h" #include "dhcp/nm-dhcp-manager.h" +#include "dhcp/nm-dhcp-utils.h" #include "nm-act-request.h" #include "nm-proxy-config.h" #include "nm-ip4-config.h" @@ -6392,12 +6393,27 @@ get_dhcp_timeout (NMDevice *self, int addr_family) return timeout ?: NM_DHCP_TIMEOUT_DEFAULT; } +static GBytes * +dhcp4_get_client_id (NMDevice *self, NMConnection *connection) +{ + NMSettingIPConfig *s_ip4; + const char *client_id; + + s_ip4 = nm_connection_get_setting_ip4_config (connection); + client_id = nm_setting_ip4_config_get_dhcp_client_id (NM_SETTING_IP4_CONFIG (s_ip4)); + + return client_id + ? nm_dhcp_utils_client_id_string_to_bytes (client_id) + : NULL; +} + static NMActStageReturn dhcp4_start (NMDevice *self) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); NMSettingIPConfig *s_ip4; gs_unref_bytes GBytes *hwaddr = NULL; + gs_unref_bytes GBytes *client_id = NULL; NMConnection *connection; connection = nm_device_get_applied_connection (self); @@ -6412,7 +6428,8 @@ dhcp4_start (NMDevice *self) hwaddr = nm_platform_link_get_address_as_bytes (nm_device_get_platform (self), nm_device_get_ip_ifindex (self)); - /* Begin DHCP on the interface */ + client_id = dhcp4_get_client_id (self, connection); + g_warn_if_fail (priv->dhcp4.client == NULL); priv->dhcp4.client = nm_dhcp_manager_start_ip4 (nm_dhcp_manager_get (), nm_netns_get_multi_idx (nm_device_get_netns (self)), @@ -6425,7 +6442,7 @@ dhcp4_start (NMDevice *self) nm_setting_ip_config_get_dhcp_send_hostname (s_ip4), nm_setting_ip_config_get_dhcp_hostname (s_ip4), nm_setting_ip4_config_get_dhcp_fqdn (NM_SETTING_IP4_CONFIG (s_ip4)), - nm_setting_ip4_config_get_dhcp_client_id (NM_SETTING_IP4_CONFIG (s_ip4)), + client_id, get_dhcp_timeout (self, AF_INET), priv->dhcp_anycast_address, NULL); diff --git a/src/dhcp/nm-dhcp-client.c b/src/dhcp/nm-dhcp-client.c index 6396a925f5..96c0265388 100644 --- a/src/dhcp/nm-dhcp-client.c +++ b/src/dhcp/nm-dhcp-client.c @@ -239,20 +239,6 @@ nm_dhcp_client_set_client_id_bin (NMDhcpClient *self, _set_client_id (self, b, TRUE); } -void -nm_dhcp_client_set_client_id_str (NMDhcpClient *self, - const char *dhcp_client_id) -{ - g_return_if_fail (NM_IS_DHCP_CLIENT (self)); - g_return_if_fail (!dhcp_client_id || dhcp_client_id[0]); - - _set_client_id (self, - dhcp_client_id - ? nm_dhcp_utils_client_id_string_to_bytes (dhcp_client_id) - : NULL, - TRUE); -} - const char * nm_dhcp_client_get_hostname (NMDhcpClient *self) { @@ -500,7 +486,7 @@ nm_dhcp_client_watch_child (NMDhcpClient *self, pid_t pid) gboolean nm_dhcp_client_start_ip4 (NMDhcpClient *self, - const char *dhcp_client_id, + GBytes *client_id, const char *dhcp_anycast_addr, const char *hostname, const char *last_ip4_address) @@ -519,7 +505,7 @@ nm_dhcp_client_start_ip4 (NMDhcpClient *self, else _LOGI ("activation: beginning transaction (timeout in %u seconds)", (guint) priv->timeout); - nm_dhcp_client_set_client_id_str (self, dhcp_client_id); + nm_dhcp_client_set_client_id (self, client_id); g_clear_pointer (&priv->hostname, g_free); priv->hostname = g_strdup (hostname); diff --git a/src/dhcp/nm-dhcp-client.h b/src/dhcp/nm-dhcp-client.h index 25c433b9c3..f389f534c2 100644 --- a/src/dhcp/nm-dhcp-client.h +++ b/src/dhcp/nm-dhcp-client.h @@ -144,7 +144,7 @@ gboolean nm_dhcp_client_get_info_only (NMDhcpClient *self); gboolean nm_dhcp_client_get_use_fqdn (NMDhcpClient *self); gboolean nm_dhcp_client_start_ip4 (NMDhcpClient *self, - const char *dhcp_client_id, + GBytes *client_id, const char *dhcp_anycast_addr, const char *hostname, const char *last_ip4_address); @@ -185,8 +185,6 @@ void nm_dhcp_client_set_client_id_bin (NMDhcpClient *self, guint8 type, const guint8 *client_id, gsize len); -void nm_dhcp_client_set_client_id_str (NMDhcpClient *self, - const char *dhcp_client_id); /***************************************************************************** * Client data diff --git a/src/dhcp/nm-dhcp-manager.c b/src/dhcp/nm-dhcp-manager.c index fc88f54978..526dbfd796 100644 --- a/src/dhcp/nm-dhcp-manager.c +++ b/src/dhcp/nm-dhcp-manager.c @@ -163,7 +163,7 @@ client_start (NMDhcpManager *self, guint32 route_table, guint32 route_metric, const struct in6_addr *ipv6_ll_addr, - const char *dhcp_client_id, + GBytes *dhcp_client_id, guint32 timeout, const char *dhcp_anycast_addr, const char *hostname, @@ -181,6 +181,7 @@ client_start (NMDhcpManager *self, g_return_val_if_fail (NM_IS_DHCP_MANAGER (self), NULL); g_return_val_if_fail (ifindex > 0, NULL); g_return_val_if_fail (uuid != NULL, NULL); + g_return_val_if_fail (!dhcp_client_id || g_bytes_get_size (dhcp_client_id) >= 2, NULL); priv = NM_DHCP_MANAGER_GET_PRIVATE (self); @@ -240,7 +241,7 @@ nm_dhcp_manager_start_ip4 (NMDhcpManager *self, gboolean send_hostname, const char *dhcp_hostname, const char *dhcp_fqdn, - const char *dhcp_client_id, + GBytes *dhcp_client_id, guint32 timeout, const char *dhcp_anycast_addr, const char *last_ip_address) diff --git a/src/dhcp/nm-dhcp-manager.h b/src/dhcp/nm-dhcp-manager.h index afb13b1d8b..3cf8cf871b 100644 --- a/src/dhcp/nm-dhcp-manager.h +++ b/src/dhcp/nm-dhcp-manager.h @@ -56,7 +56,7 @@ NMDhcpClient * nm_dhcp_manager_start_ip4 (NMDhcpManager *manager, gboolean send_hostname, const char *dhcp_hostname, const char *dhcp_fqdn, - const char *dhcp_client_id, + GBytes *dhcp_client_id, guint32 timeout, const char *dhcp_anycast_addr, const char *last_ip_address); diff --git a/src/nm-iface-helper.c b/src/nm-iface-helper.c index 996b060fd3..c270333894 100644 --- a/src/nm-iface-helper.c +++ b/src/nm-iface-helper.c @@ -341,6 +341,7 @@ main (int argc, char *argv[]) gs_unref_object NMDhcpClient *dhcp4_client = NULL; gs_unref_object NMNDisc *ndisc = NULL; gs_unref_bytes GBytes *hwaddr = NULL; + gs_unref_bytes GBytes *client_id = NULL; gs_free NMUtilsIPv6IfaceId *iid = NULL; guint sd_id; char sysctl_path_buf[NM_UTILS_SYSCTL_IP_CONF_PATH_BUFSIZE]; @@ -441,6 +442,16 @@ main (int argc, char *argv[]) iid = g_bytes_unref_to_data (bytes, &ignored); } + if (global_opt.dhcp4_clientid) { + /* this string is just a plain hex-string. Unlike ipv4.dhcp-client-id, which + * is parsed via nm_dhcp_utils_client_id_string_to_bytes(). */ + client_id = nm_utils_hexstr2bin (global_opt.dhcp4_clientid); + if (!client_id || g_bytes_get_size (client_id) < 2) { + fprintf (stderr, _("(%s): Invalid DHCP client-id %s\n"), global_opt.ifname, global_opt.dhcp4_clientid); + return 1; + } + } + if (global_opt.dhcp4_address) { nm_platform_sysctl_set (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (AF_INET, sysctl_path_buf, global_opt.ifname, "promote_secondaries")), "1"); @@ -455,7 +466,7 @@ main (int argc, char *argv[]) !!global_opt.dhcp4_hostname, global_opt.dhcp4_hostname, global_opt.dhcp4_fqdn, - global_opt.dhcp4_clientid, + client_id, NM_DHCP_TIMEOUT_DEFAULT, NULL, global_opt.dhcp4_address); From f5bedd3655140e51020de720db576045281413be Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 14 Feb 2018 15:45:29 +0100 Subject: [PATCH 09/10] device: make ipv4.dhcp-client-id configurable via a global default --- clients/common/settings-docs.h.in | 2 +- libnm-core/nm-setting-ip4-config.c | 3 +++ man/NetworkManager.conf.xml | 3 +++ src/devices/nm-device.c | 8 ++++++++ 4 files changed, 15 insertions(+), 1 deletion(-) diff --git a/clients/common/settings-docs.h.in b/clients/common/settings-docs.h.in index 4a295a4877..c7c9cc29ed 100644 --- a/clients/common/settings-docs.h.in +++ b/clients/common/settings-docs.h.in @@ -211,7 +211,7 @@ #define DESCRIBE_DOC_NM_SETTING_IP_TUNNEL_TTL N_("The TTL to assign to tunneled packets. 0 is a special value meaning that packets inherit the TTL value.") #define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_ADDRESSES N_("Array of IP addresses.") #define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_DAD_TIMEOUT N_("Timeout in milliseconds used to check for the presence of duplicate IP addresses on the network. If an address conflict is detected, the activation will fail. A zero value means that no duplicate address detection is performed, -1 means the default value (either configuration ipvx.dad-timeout override or 3 seconds). A value greater than zero is a timeout in milliseconds.") -#define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_DHCP_CLIENT_ID N_("A string sent to the DHCP server to identify the local machine which the DHCP server may use to customize the DHCP lease and options. When the property is a hex string ('aa:bb:cc') it is interpreted as a binary client ID, in which case the first byte is assumed to be the 'type' field as per RFC 2132 section 9.14 and the remaining bytes may be an hardware address (e.g. '01:xx:xx:xx:xx:xx:xx' where 1 is the Ethernet ARP type and the rest is a MAC address). If the property is not a hex string it is considered as a non-hardware-address client ID and the 'type' field is set to 0.") +#define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_DHCP_CLIENT_ID N_("A string sent to the DHCP server to identify the local machine which the DHCP server may use to customize the DHCP lease and options. When the property is a hex string ('aa:bb:cc') it is interpreted as a binary client ID, in which case the first byte is assumed to be the 'type' field as per RFC 2132 section 9.14 and the remaining bytes may be an hardware address (e.g. '01:xx:xx:xx:xx:xx:xx' where 1 is the Ethernet ARP type and the rest is a MAC address). If the property is not a hex string it is considered as a non-hardware-address client ID and the 'type' field is set to 0. If unset, a globally configurable default is used. If still unset, the client-id from the last lease may be reused.") #define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_DHCP_FQDN N_("If the \"dhcp-send-hostname\" property is TRUE, then the specified FQDN will be sent to the DHCP server when acquiring a lease. This property and \"dhcp-hostname\" are mutually exclusive and cannot be set at the same time.") #define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_DHCP_HOSTNAME N_("If the \"dhcp-send-hostname\" property is TRUE, then the specified name will be sent to the DHCP server when acquiring a lease. This property and \"dhcp-fqdn\" are mutually exclusive and cannot be set at the same time.") #define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_DHCP_SEND_HOSTNAME N_("If TRUE, a hostname is sent to the DHCP server when acquiring a lease. Some DHCP servers use this hostname to update DNS databases, essentially providing a static hostname for the computer. If the \"dhcp-hostname\" property is NULL and this property is TRUE, the current persistent hostname of the computer is sent.") diff --git a/libnm-core/nm-setting-ip4-config.c b/libnm-core/nm-setting-ip4-config.c index 74c19b6a1c..a4e01c1675 100644 --- a/libnm-core/nm-setting-ip4-config.c +++ b/libnm-core/nm-setting-ip4-config.c @@ -719,6 +719,9 @@ nm_setting_ip4_config_class_init (NMSettingIP4ConfigClass *ip4_class) * ARP type and the rest is a MAC address). * If the property is not a hex string it is considered as a * non-hardware-address client ID and the 'type' field is set to 0. + * + * If unset, a globally configurable default is used. If still unset, the + * client-id from the last lease may be reused. **/ /* ---ifcfg-rh--- * property: dhcp-client-id diff --git a/man/NetworkManager.conf.xml b/man/NetworkManager.conf.xml index 074ddc0de1..9151b2414d 100644 --- a/man/NetworkManager.conf.xml +++ b/man/NetworkManager.conf.xml @@ -685,6 +685,9 @@ ipv6.ip6-privacy=0 ipv4.dad-timeout + + ipv4.dhcp-client-id + ipv4.dhcp-timeout If left unspecified, the default value for diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index fb6c980c04..7cdc0d4427 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -6398,10 +6398,18 @@ dhcp4_get_client_id (NMDevice *self, NMConnection *connection) { NMSettingIPConfig *s_ip4; const char *client_id; + gs_free char *client_id_default = NULL; s_ip4 = nm_connection_get_setting_ip4_config (connection); client_id = nm_setting_ip4_config_get_dhcp_client_id (NM_SETTING_IP4_CONFIG (s_ip4)); + if (!client_id) { + client_id_default = nm_config_data_get_connection_default (NM_CONFIG_GET_DATA, + "ipv4.dhcp-client-id", self); + if (client_id_default && client_id_default[0]) + client_id = client_id_default; + } + return client_id ? nm_dhcp_utils_client_id_string_to_bytes (client_id) : NULL; From 62a78639797244ef49f439ba2d8bd3332d31585b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 14 Feb 2018 17:01:56 +0100 Subject: [PATCH 10/10] dhcp: add support for special ipv4.dhcp-client-id types "mac", "perm-mac", and "stable" --- clients/common/settings-docs.h.in | 4 +- libnm-core/nm-setting-connection.c | 10 +++-- libnm-core/nm-setting-ip4-config.c | 10 +++++ src/devices/nm-device.c | 72 ++++++++++++++++++++++++++++-- 4 files changed, 87 insertions(+), 9 deletions(-) diff --git a/clients/common/settings-docs.h.in b/clients/common/settings-docs.h.in index c7c9cc29ed..7580054692 100644 --- a/clients/common/settings-docs.h.in +++ b/clients/common/settings-docs.h.in @@ -152,7 +152,7 @@ #define DESCRIBE_DOC_NM_SETTING_CONNECTION_READ_ONLY N_("FALSE if the connection can be modified using the provided settings service's D-Bus interface with the right privileges, or TRUE if the connection is read-only and cannot be modified.") #define DESCRIBE_DOC_NM_SETTING_CONNECTION_SECONDARIES N_("List of connection UUIDs that should be activated when the base connection itself is activated. Currently only VPN connections are supported.") #define DESCRIBE_DOC_NM_SETTING_CONNECTION_SLAVE_TYPE N_("Setting name of the device type of this slave's master connection (eg, \"bond\"), or NULL if this connection is not a slave.") -#define DESCRIBE_DOC_NM_SETTING_CONNECTION_STABLE_ID N_("Token to generate stable IDs for the connection. The stable-id is used for generating IPv6 stable private addresses with ipv6.addr-gen-mode=stable-privacy. It is also used to seed the generated cloned MAC address for ethernet.cloned-mac-address=stable and wifi.cloned-mac-address=stable. Note that also the interface name of the activating connection and a per-host secret key is included into the address generation so that the same stable-id on different hosts/devices yields different addresses. If the value is unset, an ID unique for the connection is used. Specifying a stable-id allows multiple connections to generate the same addresses. Another use is to generate IDs at runtime via dynamic substitutions. The '$' character is treated special to perform dynamic substitutions at runtime. Currently supported are \"${CONNECTION}\", \"${BOOT}\", \"${RANDOM}\". These effectively create unique IDs per-connection, per-boot, or every time. Any unrecognized patterns following '$' are treated verbatim, however are reserved for future use. You are thus advised to avoid '$' or escape it as \"$$\". For example, set it to \"${CONNECTION}/${BOOT}\" to create a unique id for this connection that changes with every reboot. Note that two connections only use the same effective id if their stable-id is also identical before performing dynamic substitutions.") +#define DESCRIBE_DOC_NM_SETTING_CONNECTION_STABLE_ID N_("Token to generate stable IDs for the connection. The stable-id is used for generating IPv6 stable private addresses with ipv6.addr-gen-mode=stable-privacy. It is also used to seed the generated cloned MAC address for ethernet.cloned-mac-address=stable and wifi.cloned-mac-address=stable. It is also used as DHCP client identifier with ipv4.dhcp-client-id=stable. Note that also the interface name of the activating connection and a per-host secret key is included into the address generation so that the same stable-id on different hosts/devices yields different addresses. If the value is unset, an ID unique for the connection is used. Specifying a stable-id allows multiple connections to generate the same addresses. Another use is to generate IDs at runtime via dynamic substitutions. The '$' character is treated special to perform dynamic substitutions at runtime. Currently supported are \"${CONNECTION}\", \"${BOOT}\", \"${RANDOM}\". These effectively create unique IDs per-connection, per-boot, or every time. Any unrecognized patterns following '$' are treated verbatim, however are reserved for future use. You are thus advised to avoid '$' or escape it as \"$$\". For example, set it to \"${CONNECTION}/${BOOT}\" to create a unique id for this connection that changes with every reboot. Note that two connections only use the same effective id if their stable-id is also identical before performing dynamic substitutions.") #define DESCRIBE_DOC_NM_SETTING_CONNECTION_TIMESTAMP N_("The time, in seconds since the Unix Epoch, that the connection was last _successfully_ fully activated. NetworkManager updates the connection timestamp periodically when the connection is active to ensure that an active connection has the latest timestamp. The property is only meant for reading (changes to this property will not be preserved).") #define DESCRIBE_DOC_NM_SETTING_CONNECTION_TYPE N_("Base type of the connection. For hardware-dependent connections, should contain the setting name of the hardware-type specific setting (ie, \"802-3-ethernet\" or \"802-11-wireless\" or \"bluetooth\", etc), and for non-hardware dependent connections like VPN or otherwise, should contain the setting name of that setting type (ie, \"vpn\" or \"bridge\", etc).") #define DESCRIBE_DOC_NM_SETTING_CONNECTION_UUID N_("A universally unique identifier for the connection, for example generated with libuuid. It should be assigned when the connection is created, and never changed as long as the connection still applies to the same network. For example, it should not be changed when the \"id\" property or NMSettingIP4Config changes, but might need to be re-created when the Wi-Fi SSID, mobile broadband network provider, or \"type\" property changes. The UUID must be in the format \"2815492f-7e56-435e-b2e9-246bd7cdc664\" (ie, contains only hexadecimal characters and \"-\").") @@ -211,7 +211,7 @@ #define DESCRIBE_DOC_NM_SETTING_IP_TUNNEL_TTL N_("The TTL to assign to tunneled packets. 0 is a special value meaning that packets inherit the TTL value.") #define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_ADDRESSES N_("Array of IP addresses.") #define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_DAD_TIMEOUT N_("Timeout in milliseconds used to check for the presence of duplicate IP addresses on the network. If an address conflict is detected, the activation will fail. A zero value means that no duplicate address detection is performed, -1 means the default value (either configuration ipvx.dad-timeout override or 3 seconds). A value greater than zero is a timeout in milliseconds.") -#define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_DHCP_CLIENT_ID N_("A string sent to the DHCP server to identify the local machine which the DHCP server may use to customize the DHCP lease and options. When the property is a hex string ('aa:bb:cc') it is interpreted as a binary client ID, in which case the first byte is assumed to be the 'type' field as per RFC 2132 section 9.14 and the remaining bytes may be an hardware address (e.g. '01:xx:xx:xx:xx:xx:xx' where 1 is the Ethernet ARP type and the rest is a MAC address). If the property is not a hex string it is considered as a non-hardware-address client ID and the 'type' field is set to 0. If unset, a globally configurable default is used. If still unset, the client-id from the last lease may be reused.") +#define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_DHCP_CLIENT_ID N_("A string sent to the DHCP server to identify the local machine which the DHCP server may use to customize the DHCP lease and options. When the property is a hex string ('aa:bb:cc') it is interpreted as a binary client ID, in which case the first byte is assumed to be the 'type' field as per RFC 2132 section 9.14 and the remaining bytes may be an hardware address (e.g. '01:xx:xx:xx:xx:xx:xx' where 1 is the Ethernet ARP type and the rest is a MAC address). If the property is not a hex string it is considered as a non-hardware-address client ID and the 'type' field is set to 0. The special values \"mac\" and \"perm-mac\" are supported, which use the current or permanent MAC address of the device to generate a client identifier with type ethernet type (01). Currently, these options only work for ethernet type of links. The special value \"stable\" is supported to generate a type 0 client identifier based on the stable-id (see connection.stable-id). If unset, a globally configure default is used. If still unset, the client-id from the last lease is reused. If unset, a globally configurable default is used. If still unset, the client-id from the last lease may be reused.") #define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_DHCP_FQDN N_("If the \"dhcp-send-hostname\" property is TRUE, then the specified FQDN will be sent to the DHCP server when acquiring a lease. This property and \"dhcp-hostname\" are mutually exclusive and cannot be set at the same time.") #define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_DHCP_HOSTNAME N_("If the \"dhcp-send-hostname\" property is TRUE, then the specified name will be sent to the DHCP server when acquiring a lease. This property and \"dhcp-fqdn\" are mutually exclusive and cannot be set at the same time.") #define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_DHCP_SEND_HOSTNAME N_("If TRUE, a hostname is sent to the DHCP server when acquiring a lease. Some DHCP servers use this hostname to update DNS databases, essentially providing a static hostname for the computer. If the \"dhcp-hostname\" property is NULL and this property is TRUE, the current persistent hostname of the computer is sent.") diff --git a/libnm-core/nm-setting-connection.c b/libnm-core/nm-setting-connection.c index 470209a176..e02fad35a1 100644 --- a/libnm-core/nm-setting-connection.c +++ b/libnm-core/nm-setting-connection.c @@ -1541,10 +1541,12 @@ nm_setting_connection_class_init (NMSettingConnectionClass *setting_class) * The stable-id is used for generating IPv6 stable private addresses * with ipv6.addr-gen-mode=stable-privacy. It is also used to seed the * generated cloned MAC address for ethernet.cloned-mac-address=stable - * and wifi.cloned-mac-address=stable. Note that also the interface name - * of the activating connection and a per-host secret key is included - * into the address generation so that the same stable-id on different - * hosts/devices yields different addresses. + * and wifi.cloned-mac-address=stable. It is also used as DHCP client + * identifier with ipv4.dhcp-client-id=stable. + * + * Note that also the interface name of the activating connection and a + * per-host secret key is included into the address generation so that the + * same stable-id on different hosts/devices yields different addresses. * * If the value is unset, an ID unique for the connection is used. * Specifying a stable-id allows multiple connections to generate the diff --git a/libnm-core/nm-setting-ip4-config.c b/libnm-core/nm-setting-ip4-config.c index a4e01c1675..e8558a1ceb 100644 --- a/libnm-core/nm-setting-ip4-config.c +++ b/libnm-core/nm-setting-ip4-config.c @@ -720,6 +720,16 @@ nm_setting_ip4_config_class_init (NMSettingIP4ConfigClass *ip4_class) * If the property is not a hex string it is considered as a * non-hardware-address client ID and the 'type' field is set to 0. * + * The special values "mac" and "perm-mac" are supported, which use the + * current or permanent MAC address of the device to generate a client identifier + * with type ethernet type (01). Currently, these options only work for ethernet + * type of links. + * + * The special value "stable" is supported to generate a type 0 client identifier based + * on the stable-id (see connection.stable-id). + * + * If unset, a globally configure default is used. If still unset, the + * client-id from the last lease is reused. * If unset, a globally configurable default is used. If still unset, the * client-id from the last lease may be reused. **/ diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 7cdc0d4427..ddd6bf7150 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -6399,6 +6400,8 @@ dhcp4_get_client_id (NMDevice *self, NMConnection *connection) NMSettingIPConfig *s_ip4; const char *client_id; gs_free char *client_id_default = NULL; + guint8 *client_id_buf; + gboolean is_mac; s_ip4 = nm_connection_get_setting_ip4_config (connection); client_id = nm_setting_ip4_config_get_dhcp_client_id (NM_SETTING_IP4_CONFIG (s_ip4)); @@ -6410,9 +6413,72 @@ dhcp4_get_client_id (NMDevice *self, NMConnection *connection) client_id = client_id_default; } - return client_id - ? nm_dhcp_utils_client_id_string_to_bytes (client_id) - : NULL; + if (!client_id) + return NULL; + + if ( (is_mac = nm_streq (client_id, "mac")) + || nm_streq (client_id, "perm-mac")) { + const char *hwaddr; + char addr_buf[NM_UTILS_HWADDR_LEN_MAX]; + gsize addr_len; + guint8 addr_type; + + hwaddr = is_mac + ? nm_device_get_hw_address (self) + : nm_device_get_permanent_hw_address (self); + if (!hwaddr) + return NULL; + + if (!_nm_utils_hwaddr_aton (hwaddr, addr_buf, sizeof (addr_buf), &addr_len)) + g_return_val_if_reached (NULL); + + switch (addr_len) { + case ETH_ALEN: + addr_type = ARPHRD_ETHER; + break; + default: + /* unsupported type. */ + return NULL; + } + + client_id_buf = g_malloc (addr_len + 1); + client_id_buf[0] = addr_type; + memcpy (&client_id_buf[1], addr_buf, addr_len); + return g_bytes_new_take (client_id_buf, addr_len + 1); + } + + if (nm_streq (client_id, "stable")) { + NMUtilsStableType stable_type; + const char *stable_id; + GChecksum *sum; + guint8 buf[20]; + gsize buf_size; + guint32 salted_header; + + stable_id = _get_stable_id (self, connection, &stable_type); + if (!stable_id) + g_return_val_if_reached (NULL); + + salted_header = htonl (2011610591 + stable_type); + + sum = g_checksum_new (G_CHECKSUM_SHA1); + + g_checksum_update (sum, (const guchar *) &salted_header, sizeof (salted_header)); + g_checksum_update (sum, (const guchar *) stable_id, strlen (stable_id)); + + buf_size = sizeof (buf); + g_checksum_get_digest (sum, buf, &buf_size); + nm_assert (buf_size == sizeof (buf)); + + g_checksum_free (sum); + + client_id_buf = g_malloc (1 + 15); + client_id_buf[0] = 0; + memcpy (&client_id_buf[0], buf, 15); + return g_bytes_new_take (client_id_buf, 1 + 15); + } + + return nm_dhcp_utils_client_id_string_to_bytes (client_id); } static NMActStageReturn