From 6aa1111aeaa0b691cf13456c2951f495b5cea99b Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 13 Dec 2022 08:37:29 +0100
Subject: [PATCH 01/18] core/trivial: fix indentation

(cherry picked from commit 191a1c74bf19cd00c7f630a0bcdd47a15ad68829)
---
 src/core/dhcp/nm-dhcp-client.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/core/dhcp/nm-dhcp-client.c b/src/core/dhcp/nm-dhcp-client.c
index 1329b953e8..b0e9cee8df 100644
--- a/src/core/dhcp/nm-dhcp-client.c
+++ b/src/core/dhcp/nm-dhcp-client.c
@@ -1524,20 +1524,20 @@ maybe_add_option(NMDhcpClient *self, GHashTable *hash, const char *key, GVariant
     g_hash_table_insert(hash, g_strdup(key), str_value);
 
     /* dhclient has no special labels for private dhcp options: it uses "unknown_xyz"
-         * labels for that. We need to identify those to alias them to our "private_xyz"
-         * format unused in the internal dchp plugins.
-         */
+     * labels for that. We need to identify those to alias them to our "private_xyz"
+     * format unused in the internal dchp plugins.
+     */
     if ((priv_opt_num = label_is_unknown_xyz(key)) > 0) {
         gs_free guint8 *check_val = NULL;
         char           *hex_str   = NULL;
         gsize           len;
 
         /* dhclient passes values from dhcp private options in its own "string" format:
-             * if the raw values are printable as ascii strings, it will pass the string
-             * representation; if the values are not printable as an ascii string, it will
-             * pass a string displaying the hex values (hex string). Try to enforce passing
-             * always an hex string, converting string representation if needed.
-             */
+         * if the raw values are printable as ascii strings, it will pass the string
+         * representation; if the values are not printable as an ascii string, it will
+         * pass a string displaying the hex values (hex string). Try to enforce passing
+         * always an hex string, converting string representation if needed.
+         */
         check_val = nm_utils_hexstr2bin_alloc(str_value, FALSE, TRUE, ":", 0, &len);
         hex_str   = nm_utils_bin2hexstr_full(check_val ?: (guint8 *) str_value,
                                            check_val ? len : strlen(str_value),

From 49fdd3d4b16797bce4a69029a3be51beae537db9 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 13 Dec 2022 09:08:34 +0100
Subject: [PATCH 02/18] dhcp/trivial: fix naming for internal
 NM_DHCP_OPTION_DHCP6_{CLIENT,SERVER}_ID enums

(cherry picked from commit 9073628bd61a71ada5d87cfc0304c28d0bdc4291)
---
 src/core/dhcp/nm-dhcp-options.c | 4 ++--
 src/core/dhcp/nm-dhcp-options.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/core/dhcp/nm-dhcp-options.c b/src/core/dhcp/nm-dhcp-options.c
index 96fef81a40..172eb0492c 100644
--- a/src/core/dhcp/nm-dhcp-options.c
+++ b/src/core/dhcp/nm-dhcp-options.c
@@ -190,12 +190,12 @@ static const NMDhcpOption *const _sorted_options_4[G_N_ELEMENTS(_nm_dhcp_option_
 };
 
 const NMDhcpOption _nm_dhcp_option_dhcp6_options[] = {
-    REQ(NM_DHCP_OPTION_DHCP6_CLIENTID, "dhcp6_client_id", FALSE),
+    REQ(NM_DHCP_OPTION_DHCP6_CLIENT_ID, "dhcp6_client_id", FALSE),
 
     /* Don't request server ID by default; some servers don't reply to
      * Information Requests that request the Server ID.
      */
-    REQ(NM_DHCP_OPTION_DHCP6_SERVERID, "dhcp6_server_id", FALSE),
+    REQ(NM_DHCP_OPTION_DHCP6_SERVER_ID, "dhcp6_server_id", FALSE),
 
     REQ(NM_DHCP_OPTION_DHCP6_DNS_SERVERS, "dhcp6_name_servers", TRUE),
     REQ(NM_DHCP_OPTION_DHCP6_DOMAIN_LIST, "dhcp6_domain_search", TRUE),
diff --git a/src/core/dhcp/nm-dhcp-options.h b/src/core/dhcp/nm-dhcp-options.h
index 4c978c4f97..bdad8c94e8 100644
--- a/src/core/dhcp/nm-dhcp-options.h
+++ b/src/core/dhcp/nm-dhcp-options.h
@@ -157,8 +157,8 @@ typedef enum {
 } NMDhcpOptionDhcp4Options;
 
 typedef enum {
-    NM_DHCP_OPTION_DHCP6_CLIENTID     = 1,
-    NM_DHCP_OPTION_DHCP6_SERVERID     = 2,
+    NM_DHCP_OPTION_DHCP6_CLIENT_ID    = 1,
+    NM_DHCP_OPTION_DHCP6_SERVER_ID    = 2,
     NM_DHCP_OPTION_DHCP6_DNS_SERVERS  = 23,
     NM_DHCP_OPTION_DHCP6_DOMAIN_LIST  = 24,
     NM_DHCP_OPTION_DHCP6_SNTP_SERVERS = 31,

From 4f2d774b9c8691676011f0a9d9e70eadd327b43e Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 12 Dec 2022 19:15:09 +0100
Subject: [PATCH 03/18] dhcp: don't use
 nm_dhcp_client_get_effective_client_id() from systemd DHCPv6 client

The "effective-client-id" is handled wrongly. Step 1 to clean this up.

Note that NMDhcpClientPrivate.effective_client_id is only ever get/set
via the nm_dhcp_client_[gs]et_effective_client_id() functions.
Note that only a NMDhcpDhclient instance ever calls
nm_dhcp_client_set_effective_client_id().

Hence, for NMDhcpSystemd the effective-client-id is really just the DUID
from the config. Clean this up by not calling nm_dhcp_client_get_effective_client_id()
but use the config directly. There is no change in behavior here.

(cherry picked from commit 05ae48d64e3e5afd2fc8433ac965bc61b0a1f647)
---
 src/core/dhcp/nm-dhcp-systemd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/core/dhcp/nm-dhcp-systemd.c b/src/core/dhcp/nm-dhcp-systemd.c
index 49e21d97e5..5e9c2c3734 100644
--- a/src/core/dhcp/nm-dhcp-systemd.c
+++ b/src/core/dhcp/nm-dhcp-systemd.c
@@ -271,7 +271,7 @@ ip6_start(NMDhcpClient *client, const struct in6_addr *ll_addr, GError **error)
 
     /* TODO: honor nm_dhcp_client_get_anycast_address() */
 
-    duid = nm_dhcp_client_get_effective_client_id(client);
+    duid = client_config->client_id;
     if (!duid || !(duid_arr = g_bytes_get_data(duid, &duid_len)) || duid_len < 2) {
         nm_utils_error_set_literal(error, NM_UTILS_ERROR_UNKNOWN, "missing DUID");
         g_return_val_if_reached(FALSE);

From a4bce41fa02c2cca5bc6a6c638ee225808a0acdf Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 12 Dec 2022 19:20:19 +0100
Subject: [PATCH 04/18] dhcp: drop NMDhcpClientClass.get_duid() hook

Note that there are no callers of nm_dhcp_client_get_effective_client_id(),
hence calling the setter had no effect. This is a bug, that we will fix
later.

But before fixing the bug, change how this works. Drop the get_duid() hook.
It's only confusing and backward.

We will keep the nm_dhcp_client_[gs]et_effective_client_id() functions.
They will be used later.

(cherry picked from commit 28d7f9b7c4787db101e30a549c6b05aad2ce89c3)
---
 src/core/dhcp/nm-dhcp-client.c   | 14 +-------------
 src/core/dhcp/nm-dhcp-client.h   | 11 -----------
 src/core/dhcp/nm-dhcp-dhclient.c |  3 +--
 3 files changed, 2 insertions(+), 26 deletions(-)

diff --git a/src/core/dhcp/nm-dhcp-client.c b/src/core/dhcp/nm-dhcp-client.c
index b0e9cee8df..f21bcb656c 100644
--- a/src/core/dhcp/nm-dhcp-client.c
+++ b/src/core/dhcp/nm-dhcp-client.c
@@ -967,12 +967,6 @@ _dhcp_client_decline(NMDhcpClient         *self,
     return klass->decline(self, l3cd, error_message, error);
 }
 
-static GBytes *
-get_duid(NMDhcpClient *self)
-{
-    return NULL;
-}
-
 static gboolean
 ipv6_lladdr_timeout(gpointer user_data)
 {
@@ -1317,11 +1311,6 @@ nm_dhcp_client_start(NMDhcpClient *self, GError **error)
     IS_IPv4 = NM_IS_IPv4(priv->config.addr_family);
 
     if (!IS_IPv4) {
-        if (!priv->config.v6.enforce_duid)
-            own_client_id = NM_DHCP_CLIENT_GET_CLASS(self)->get_duid(self);
-
-        nm_dhcp_client_set_effective_client_id(self, own_client_id ?: priv->config.client_id);
-
         addr = ipv6_lladdr_find(self);
         if (!addr) {
             _LOGD("waiting for IPv6LL address");
@@ -1914,8 +1903,7 @@ nm_dhcp_client_class_init(NMDhcpClientClass *client_class)
     client_class->accept       = _accept;
     client_class->decline      = decline;
 
-    client_class->stop     = stop;
-    client_class->get_duid = get_duid;
+    client_class->stop = stop;
 
     obj_properties[PROP_CONFIG] =
         g_param_spec_pointer(NM_DHCP_CLIENT_CONFIG,
diff --git a/src/core/dhcp/nm-dhcp-client.h b/src/core/dhcp/nm-dhcp-client.h
index 51c6bc048f..f570e45a75 100644
--- a/src/core/dhcp/nm-dhcp-client.h
+++ b/src/core/dhcp/nm-dhcp-client.h
@@ -214,17 +214,6 @@ typedef struct {
     gboolean (*ip6_start)(NMDhcpClient *self, const struct in6_addr *ll_addr, GError **error);
 
     void (*stop)(NMDhcpClient *self, gboolean release);
-
-    /**
-     * get_duid:
-     * @self: the #NMDhcpClient
-     *
-     * Attempts to find an existing DHCPv6 DUID for this client in the DHCP
-     * client's persistent configuration.  Returned DUID should be the binary
-     * representation of the DUID.  If no DUID is found, %NULL should be
-     * returned.
-     */
-    GBytes *(*get_duid)(NMDhcpClient *self);
 } NMDhcpClientClass;
 
 GType nm_dhcp_client_get_type(void);
diff --git a/src/core/dhcp/nm-dhcp-dhclient.c b/src/core/dhcp/nm-dhcp-dhclient.c
index d0cd5ebdc7..98136ae68d 100644
--- a/src/core/dhcp/nm-dhcp-dhclient.c
+++ b/src/core/dhcp/nm-dhcp-dhclient.c
@@ -627,7 +627,7 @@ stop(NMDhcpClient *client, gboolean release)
     }
 }
 
-static GBytes *
+_nm_unused static GBytes *
 get_duid(NMDhcpClient *client)
 {
     NMDhcpDhclient           *self = NM_DHCP_DHCLIENT(client);
@@ -724,7 +724,6 @@ nm_dhcp_dhclient_class_init(NMDhcpDhclientClass *dhclient_class)
     client_class->ip4_start = ip4_start;
     client_class->ip6_start = ip6_start;
     client_class->stop      = stop;
-    client_class->get_duid  = get_duid;
 }
 
 const NMDhcpClientFactory _nm_dhcp_client_factory_dhclient = {

From fb0315902838467b0e3e06efc9939e96e88d3c8a Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 12 Dec 2022 20:29:30 +0100
Subject: [PATCH 05/18] dhcp: fix "ipv6.dhcp-duid=lease" for dhclient DHCPv6
 client

The "lease" mode is unusual, because it means to prefer the DUID
configuration from the DHCP plugin over the explicit configuration in
NetworkManager. It is only for the DHCPv6 DUID and not for the IPv4
client-id. It also is only special for the "dhclient" plugin, because
with the internal plugin, this always corresponds to a generated, stable
DUID.

Commit 58287cbcc0c8 ('core: rework IP configuration in NetworkManager
using layer 3 configuration') broke this. The commit refactored the code
to track the effective-client-id separately. Previously, the client-id which
was read from the dhclient lease, was overwriting NMDhcpClient.client_id. But
with the refactor, it broke because nm_dhcp_client_get_effective_client_id()
was never called.

Fix that.

Fixes: 58287cbcc0c8 ('core: rework IP configuration in NetworkManager using layer 3 configuration')
(cherry picked from commit bea72c3d6de5a294a89fc659e475fe9db0abf6ac)
---
 src/core/dhcp/nm-dhcp-dhclient-utils.c |  2 ++
 src/core/dhcp/nm-dhcp-dhclient.c       | 29 +++++++++++++++++++-------
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/src/core/dhcp/nm-dhcp-dhclient-utils.c b/src/core/dhcp/nm-dhcp-dhclient-utils.c
index 74e6b90507..72dbc7885e 100644
--- a/src/core/dhcp/nm-dhcp-dhclient-utils.c
+++ b/src/core/dhcp/nm-dhcp-dhclient-utils.c
@@ -399,6 +399,7 @@ nm_dhcp_dhclient_create_config(const char         *interface,
                 if (out_new_client_id)
                     nm_clear_pointer(out_new_client_id, g_bytes_unref);
                 NM_SET_OUT(out_new_client_id, read_client_id(p));
+                /* fall-through. We keep the line... */
             }
 
             /* Override config file hostname and use one from the connection */
@@ -656,6 +657,7 @@ nm_dhcp_dhclient_save_duid(const char *leasefile, GBytes *duid, GError **error)
     gsize                         len = 0;
 
     g_return_val_if_fail(leasefile != NULL, FALSE);
+
     if (!duid) {
         nm_utils_error_set_literal(error, NM_UTILS_ERROR_UNKNOWN, "missing duid");
         g_return_val_if_reached(FALSE);
diff --git a/src/core/dhcp/nm-dhcp-dhclient.c b/src/core/dhcp/nm-dhcp-dhclient.c
index 98136ae68d..fb1e7cca1e 100644
--- a/src/core/dhcp/nm-dhcp-dhclient.c
+++ b/src/core/dhcp/nm-dhcp-dhclient.c
@@ -82,6 +82,10 @@ G_DEFINE_TYPE(NMDhcpDhclient, nm_dhcp_dhclient, NM_TYPE_DHCP_CLIENT)
 
 /*****************************************************************************/
 
+static GBytes *read_duid_from_lease(NMDhcpDhclient *self);
+
+/*****************************************************************************/
+
 static const char *
 nm_dhcp_dhclient_get_path(void)
 {
@@ -332,6 +336,7 @@ static gboolean
 dhclient_start(NMDhcpClient *client,
                gboolean      set_mode,
                gboolean      release,
+               gboolean      set_duid,
                pid_t        *out_pid,
                GError      **error)
 {
@@ -410,8 +415,10 @@ dhclient_start(NMDhcpClient *client,
     }
 
     /* Save the DUID to the leasefile dhclient will actually use */
-    if (addr_family == AF_INET6) {
-        if (!nm_dhcp_dhclient_save_duid(priv->lease_file, client_config->client_id, &local)) {
+    if (set_duid && addr_family == AF_INET6) {
+        if (!nm_dhcp_dhclient_save_duid(priv->lease_file,
+                                        nm_dhcp_client_get_effective_client_id(client),
+                                        &local)) {
             nm_utils_error_set(error,
                                NM_UTILS_ERROR_UNKNOWN,
                                "failed to save DUID to '%s': %s",
@@ -560,6 +567,7 @@ ip6_start(NMDhcpClient *client, const struct in6_addr *ll_addr, GError **error)
     NMDhcpDhclient           *self = NM_DHCP_DHCLIENT(client);
     NMDhcpDhclientPrivate    *priv = NM_DHCP_DHCLIENT_GET_PRIVATE(self);
     const NMDhcpClientConfig *config;
+    gs_unref_bytes GBytes    *effective_client_id = NULL;
 
     config = nm_dhcp_client_get_config(client);
 
@@ -586,7 +594,12 @@ ip6_start(NMDhcpClient *client, const struct in6_addr *ll_addr, GError **error)
         return FALSE;
     }
 
-    return dhclient_start(client, TRUE, FALSE, NULL, error);
+    nm_assert(config->client_id);
+    if (!config->v6.enforce_duid)
+        effective_client_id = read_duid_from_lease(self);
+    nm_dhcp_client_set_effective_client_id(client, effective_client_id ?: config->client_id);
+
+    return dhclient_start(client, TRUE, FALSE, TRUE, NULL, error);
 }
 
 static void
@@ -620,18 +633,18 @@ stop(NMDhcpClient *client, gboolean release)
     if (release) {
         pid_t rpid = -1;
 
-        if (dhclient_start(client, FALSE, TRUE, &rpid, NULL)) {
+        if (dhclient_start(client, FALSE, TRUE, FALSE, &rpid, NULL)) {
             /* Wait a few seconds for the release to happen */
             nm_dhcp_client_stop_pid(rpid, nm_dhcp_client_get_iface(client));
         }
     }
 }
 
-_nm_unused static GBytes *
-get_duid(NMDhcpClient *client)
+static GBytes *
+read_duid_from_lease(NMDhcpDhclient *self)
 {
-    NMDhcpDhclient           *self = NM_DHCP_DHCLIENT(client);
-    NMDhcpDhclientPrivate    *priv = NM_DHCP_DHCLIENT_GET_PRIVATE(self);
+    NMDhcpClient             *client = NM_DHCP_CLIENT(self);
+    NMDhcpDhclientPrivate    *priv   = NM_DHCP_DHCLIENT_GET_PRIVATE(self);
     const NMDhcpClientConfig *client_config;
     GBytes                   *duid      = NULL;
     gs_free char             *leasefile = NULL;

From 2987bb7e8da6dbfd5040cefa70f590cd58565aa9 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 12 Dec 2022 20:38:34 +0100
Subject: [PATCH 06/18] dhcp: set effective-client-id for all DHCP plugins

(cherry picked from commit 84b90fbdd38aa6b39a7ac018e9216aecbb6f298c)
---
 src/core/dhcp/nm-dhcp-client.c   |  2 +-
 src/core/dhcp/nm-dhcp-dhclient.c | 10 +++++-----
 src/core/dhcp/nm-dhcp-nettools.c | 15 +++++++++++----
 src/core/dhcp/nm-dhcp-systemd.c  |  2 ++
 4 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/src/core/dhcp/nm-dhcp-client.c b/src/core/dhcp/nm-dhcp-client.c
index f21bcb656c..4a2263e76c 100644
--- a/src/core/dhcp/nm-dhcp-client.c
+++ b/src/core/dhcp/nm-dhcp-client.c
@@ -256,7 +256,7 @@ nm_dhcp_client_set_effective_client_id(NMDhcpClient *self, GBytes *client_id)
     g_bytes_unref(priv->effective_client_id);
     priv->effective_client_id = nm_g_bytes_ref(client_id);
 
-    _LOGT("%s: set %s",
+    _LOGT("%s: set effective %s",
           priv->config.addr_family == AF_INET6 ? "duid" : "client-id",
           priv->effective_client_id
               ? (tmp_str = nm_dhcp_utils_duid_to_string(priv->effective_client_id))
diff --git a/src/core/dhcp/nm-dhcp-dhclient.c b/src/core/dhcp/nm-dhcp-dhclient.c
index fb1e7cca1e..7e578d13ad 100644
--- a/src/core/dhcp/nm-dhcp-dhclient.c
+++ b/src/core/dhcp/nm-dhcp-dhclient.c
@@ -554,11 +554,11 @@ ip4_start(NMDhcpClient *client, GError **error)
         return FALSE;
     }
 
-    if (new_client_id) {
-        nm_assert(!client_config->client_id);
-        nm_dhcp_client_set_effective_client_id(client, new_client_id);
-    }
-    return dhclient_start(client, FALSE, FALSE, NULL, error);
+    /* Note that the effective-client-id for IPv4 here might be unknown/NULL. */
+    nm_assert(!new_client_id || !client_config->client_id);
+    nm_dhcp_client_set_effective_client_id(client, client_config->client_id ?: new_client_id);
+
+    return dhclient_start(client, FALSE, FALSE, FALSE, NULL, error);
 }
 
 static gboolean
diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c
index 05b7b52e95..5571b279b2 100644
--- a/src/core/dhcp/nm-dhcp-nettools.c
+++ b/src/core/dhcp/nm-dhcp-nettools.c
@@ -1100,7 +1100,7 @@ dhcp4_event_cb(int fd, GIOCondition condition, gpointer user_data)
 }
 
 static gboolean
-nettools_create(NMDhcpNettools *self, GError **error)
+nettools_create(NMDhcpNettools *self, GBytes **out_effective_client_id, GError **error)
 {
     NMDhcpNettoolsPrivate *priv = NM_DHCP_NETTOOLS_GET_PRIVATE(self);
     nm_auto(n_dhcp4_client_config_freep) NDhcp4ClientConfig *config = NULL;
@@ -1197,6 +1197,9 @@ nettools_create(NMDhcpNettools *self, GError **error)
 
     priv->event_source = nm_g_unix_fd_add_source(fd, G_IO_IN, dhcp4_event_cb, self);
 
+    *out_effective_client_id =
+        (client_id == client_id_new) ? g_steal_pointer(&client_id_new) : g_bytes_ref(client_id);
+
     return TRUE;
 }
 
@@ -1287,8 +1290,9 @@ static gboolean
 ip4_start(NMDhcpClient *client, GError **error)
 {
     nm_auto(n_dhcp4_client_probe_config_freep) NDhcp4ClientProbeConfig *config = NULL;
-    NMDhcpNettools           *self = NM_DHCP_NETTOOLS(client);
-    NMDhcpNettoolsPrivate    *priv = NM_DHCP_NETTOOLS_GET_PRIVATE(self);
+    NMDhcpNettools           *self                = NM_DHCP_NETTOOLS(client);
+    NMDhcpNettoolsPrivate    *priv                = NM_DHCP_NETTOOLS_GET_PRIVATE(self);
+    gs_unref_bytes GBytes    *effective_client_id = NULL;
     const NMDhcpClientConfig *client_config;
     gs_free char             *lease_file = NULL;
     struct in_addr            last_addr  = {0};
@@ -1299,7 +1303,7 @@ ip4_start(NMDhcpClient *client, GError **error)
     g_return_val_if_fail(!priv->probe, FALSE);
     g_return_val_if_fail(client_config, FALSE);
 
-    if (!nettools_create(self, error))
+    if (!nettools_create(self, &effective_client_id, error))
         return FALSE;
 
     r = n_dhcp4_client_probe_config_new(&config);
@@ -1445,6 +1449,9 @@ ip4_start(NMDhcpClient *client, GError **error)
     }
 
     _LOGT("dhcp-client4: start " NM_HASH_OBFUSCATE_PTR_FMT, NM_HASH_OBFUSCATE_PTR(priv->client));
+
+    nm_dhcp_client_set_effective_client_id(client, effective_client_id);
+
     return TRUE;
 }
 
diff --git a/src/core/dhcp/nm-dhcp-systemd.c b/src/core/dhcp/nm-dhcp-systemd.c
index 5e9c2c3734..9734b96f48 100644
--- a/src/core/dhcp/nm-dhcp-systemd.c
+++ b/src/core/dhcp/nm-dhcp-systemd.c
@@ -379,6 +379,8 @@ ip6_start(NMDhcpClient *client, const struct in6_addr *ll_addr, GError **error)
         return FALSE;
     }
 
+    nm_dhcp_client_set_effective_client_id(client, duid);
+
     return TRUE;
 }
 

From 63a6bc1bc92ce98488d2cc18ad72e20c1b50da73 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 13 Dec 2022 08:31:16 +0100
Subject: [PATCH 07/18] dhcp: add static-keys argument to
 nm_dhcp_option_create_options_dict()

This is so that we can use the same function also to create the
hash for dhclient plugin.

(cherry picked from commit 492818b52940f70d4fd7c553c07770030b56de2e)
---
 src/core/dhcp/nm-dhcp-nettools.c | 2 +-
 src/core/dhcp/nm-dhcp-options.c  | 4 ++--
 src/core/dhcp/nm-dhcp-options.h  | 5 +++--
 src/core/dhcp/nm-dhcp-systemd.c  | 2 +-
 4 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c
index 5571b279b2..04bedb878c 100644
--- a/src/core/dhcp/nm-dhcp-nettools.c
+++ b/src/core/dhcp/nm-dhcp-nettools.c
@@ -624,7 +624,7 @@ lease_to_ip4_config(NMDhcpNettools *self, NDhcp4ClientLease *lease, GError **err
 
     l3cd = nm_dhcp_client_create_l3cd(NM_DHCP_CLIENT(self));
 
-    options = nm_dhcp_option_create_options_dict();
+    options = nm_dhcp_option_create_options_dict(TRUE);
 
     if (!lease_parse_address(self, lease, l3cd, iface, options, &lease_address, error))
         return NULL;
diff --git a/src/core/dhcp/nm-dhcp-options.c b/src/core/dhcp/nm-dhcp-options.c
index 172eb0492c..a5bb06d1c7 100644
--- a/src/core/dhcp/nm-dhcp-options.c
+++ b/src/core/dhcp/nm-dhcp-options.c
@@ -460,7 +460,7 @@ nm_dhcp_option_add_requests_to_options(GHashTable *options, int addr_family)
 }
 
 GHashTable *
-nm_dhcp_option_create_options_dict(void)
+nm_dhcp_option_create_options_dict(gboolean static_keys)
 {
-    return g_hash_table_new_full(nm_str_hash, g_str_equal, NULL, g_free);
+    return g_hash_table_new_full(nm_str_hash, g_str_equal, static_keys ? NULL : g_free, g_free);
 }
diff --git a/src/core/dhcp/nm-dhcp-options.h b/src/core/dhcp/nm-dhcp-options.h
index bdad8c94e8..fcc6f9cd08 100644
--- a/src/core/dhcp/nm-dhcp-options.h
+++ b/src/core/dhcp/nm-dhcp-options.h
@@ -222,7 +222,8 @@ void nm_dhcp_option_add_option_in_addr(GHashTable *options,
                                        in_addr_t   value);
 void
 nm_dhcp_option_add_option_u64(GHashTable *options, int addr_family, guint option, guint64 value);
-void        nm_dhcp_option_add_requests_to_options(GHashTable *options, int addr_family);
-GHashTable *nm_dhcp_option_create_options_dict(void);
+void nm_dhcp_option_add_requests_to_options(GHashTable *options, int addr_family);
+
+GHashTable *nm_dhcp_option_create_options_dict(gboolean static_keys);
 
 #endif /* __NM_DHCP_OPTIONS_H__ */
diff --git a/src/core/dhcp/nm-dhcp-systemd.c b/src/core/dhcp/nm-dhcp-systemd.c
index 9734b96f48..0bed14a82a 100644
--- a/src/core/dhcp/nm-dhcp-systemd.c
+++ b/src/core/dhcp/nm-dhcp-systemd.c
@@ -86,7 +86,7 @@ lease_to_ip6_config(NMDhcpSystemd *self, sd_dhcp6_lease *lease, gint32 ts, GErro
 
     l3cd = nm_dhcp_client_create_l3cd(NM_DHCP_CLIENT(self));
 
-    options = nm_dhcp_option_create_options_dict();
+    options = nm_dhcp_option_create_options_dict(TRUE);
 
     if (!nm_dhcp_client_get_config(NM_DHCP_CLIENT(self))->v6.info_only) {
         gboolean has_any_addresses = FALSE;

From 2535395a8cf34ded6d4d5c8d2a25eec6ab1abe1d Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 13 Dec 2022 08:36:59 +0100
Subject: [PATCH 08/18] dhcp: use nm_dhcp_option_create_options_dict() in
 nm_dhcp_client_handle_event()

The point of using this trivial helper function is to have one function
that is related to the construction of the options dictionary, that we
can search for.

It answers the question, where do we create a option hash (at `git grep
nm_dhcp_option_create_options_dict`).

(cherry picked from commit ccbe76b81d2bb49d290b01dcf5e9f391279115c6)
---
 src/core/dhcp/nm-dhcp-client.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/core/dhcp/nm-dhcp-client.c b/src/core/dhcp/nm-dhcp-client.c
index 4a2263e76c..7f507e2e1c 100644
--- a/src/core/dhcp/nm-dhcp-client.c
+++ b/src/core/dhcp/nm-dhcp-client.c
@@ -1612,7 +1612,7 @@ nm_dhcp_client_handle_event(gpointer               unused,
         GVariant                      *value;
 
         /* Copy options */
-        str_options = g_hash_table_new_full(nm_str_hash, g_str_equal, g_free, g_free);
+        str_options = nm_dhcp_option_create_options_dict(FALSE);
         g_variant_iter_init(&iter, options);
         while (g_variant_iter_next(&iter, "{&sv}", &name, &value)) {
             maybe_add_option(self, str_options, name, value);

From 34d3898427bc12f87ccf606765c3f2627c4294cf Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 12 Dec 2022 16:21:31 +0100
Subject: [PATCH 09/18] dhcp: add and use nm_dhcp_client_create_options_dict()

This will be used to pre-fill the lease with client-specific options.

(cherry picked from commit c020f618ed9446ab163980b2456766a840ed7f65)
---
 src/core/dhcp/nm-dhcp-client.c   | 12 +++++++++++-
 src/core/dhcp/nm-dhcp-client.h   |  2 ++
 src/core/dhcp/nm-dhcp-nettools.c |  2 +-
 src/core/dhcp/nm-dhcp-systemd.c  |  2 +-
 4 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/src/core/dhcp/nm-dhcp-client.c b/src/core/dhcp/nm-dhcp-client.c
index 7f507e2e1c..ffd4bacbc9 100644
--- a/src/core/dhcp/nm-dhcp-client.c
+++ b/src/core/dhcp/nm-dhcp-client.c
@@ -237,6 +237,16 @@ nm_dhcp_client_create_l3cd(NMDhcpClient *self)
                                  NM_IP_CONFIG_SOURCE_DHCP);
 }
 
+GHashTable *
+nm_dhcp_client_create_options_dict(NMDhcpClient *self, gboolean static_keys)
+{
+    GHashTable *options;
+
+    options = nm_dhcp_option_create_options_dict(static_keys);
+
+    return options;
+}
+
 /*****************************************************************************/
 
 void
@@ -1612,7 +1622,7 @@ nm_dhcp_client_handle_event(gpointer               unused,
         GVariant                      *value;
 
         /* Copy options */
-        str_options = nm_dhcp_option_create_options_dict(FALSE);
+        str_options = nm_dhcp_client_create_options_dict(self, FALSE);
         g_variant_iter_init(&iter, options);
         while (g_variant_iter_next(&iter, "{&sv}", &name, &value)) {
             maybe_add_option(self, str_options, name, value);
diff --git a/src/core/dhcp/nm-dhcp-client.h b/src/core/dhcp/nm-dhcp-client.h
index f570e45a75..d57d446e9f 100644
--- a/src/core/dhcp/nm-dhcp-client.h
+++ b/src/core/dhcp/nm-dhcp-client.h
@@ -279,6 +279,8 @@ GBytes *nm_dhcp_client_get_effective_client_id(NMDhcpClient *self);
 
 NML3ConfigData *nm_dhcp_client_create_l3cd(NMDhcpClient *self);
 
+GHashTable *nm_dhcp_client_create_options_dict(NMDhcpClient *self, gboolean static_keys);
+
 /*****************************************************************************
  * Client data
  *****************************************************************************/
diff --git a/src/core/dhcp/nm-dhcp-nettools.c b/src/core/dhcp/nm-dhcp-nettools.c
index 04bedb878c..df88362e62 100644
--- a/src/core/dhcp/nm-dhcp-nettools.c
+++ b/src/core/dhcp/nm-dhcp-nettools.c
@@ -624,7 +624,7 @@ lease_to_ip4_config(NMDhcpNettools *self, NDhcp4ClientLease *lease, GError **err
 
     l3cd = nm_dhcp_client_create_l3cd(NM_DHCP_CLIENT(self));
 
-    options = nm_dhcp_option_create_options_dict(TRUE);
+    options = nm_dhcp_client_create_options_dict(NM_DHCP_CLIENT(self), TRUE);
 
     if (!lease_parse_address(self, lease, l3cd, iface, options, &lease_address, error))
         return NULL;
diff --git a/src/core/dhcp/nm-dhcp-systemd.c b/src/core/dhcp/nm-dhcp-systemd.c
index 0bed14a82a..7ce15d301e 100644
--- a/src/core/dhcp/nm-dhcp-systemd.c
+++ b/src/core/dhcp/nm-dhcp-systemd.c
@@ -86,7 +86,7 @@ lease_to_ip6_config(NMDhcpSystemd *self, sd_dhcp6_lease *lease, gint32 ts, GErro
 
     l3cd = nm_dhcp_client_create_l3cd(NM_DHCP_CLIENT(self));
 
-    options = nm_dhcp_option_create_options_dict(TRUE);
+    options = nm_dhcp_client_create_options_dict(NM_DHCP_CLIENT(self), TRUE);
 
     if (!nm_dhcp_client_get_config(NM_DHCP_CLIENT(self))->v6.info_only) {
         gboolean has_any_addresses = FALSE;

From 1f30005d5e198132a3629b465be5d13728b5a193 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 13 Dec 2022 09:05:06 +0100
Subject: [PATCH 10/18] dhcp: set the
 "dhcp_client_identifier"/"dhcp6_client_id" lease options

Also for the internal DHCP clients. And validate/normalize the setting
for the dhclient/dhcpcd/dhcdcanon plugins.

(cherry picked from commit ef5333e5cfb422ea562ce04304d90c17a7f10887)
---
 src/core/dhcp/nm-dhcp-client.c | 65 +++++++++++++++++++++++++++++-----
 src/core/dhcp/nm-dhcp-client.h |  4 +--
 2 files changed, 58 insertions(+), 11 deletions(-)

diff --git a/src/core/dhcp/nm-dhcp-client.c b/src/core/dhcp/nm-dhcp-client.c
index ffd4bacbc9..b6f9071e63 100644
--- a/src/core/dhcp/nm-dhcp-client.c
+++ b/src/core/dhcp/nm-dhcp-client.c
@@ -240,28 +240,50 @@ nm_dhcp_client_create_l3cd(NMDhcpClient *self)
 GHashTable *
 nm_dhcp_client_create_options_dict(NMDhcpClient *self, gboolean static_keys)
 {
-    GHashTable *options;
+    NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE(self);
+    GHashTable          *options;
+    GBytes              *effective_client_id;
 
     options = nm_dhcp_option_create_options_dict(static_keys);
 
+    effective_client_id = nm_dhcp_client_get_effective_client_id(self);
+    if (effective_client_id) {
+        guint         option = NM_IS_IPv4(priv->config.addr_family) ? NM_DHCP_OPTION_DHCP4_CLIENT_ID
+                                                                    : NM_DHCP_OPTION_DHCP6_CLIENT_ID;
+        gs_free char *str    = nm_dhcp_utils_duid_to_string(effective_client_id);
+
+        /* Note that for the nm-dhcp-helper based plugins (dhclient), the plugin
+         * may send the used client-id/DUID via the environment variables and
+         * overwrite them yet again. */
+
+        if (static_keys) {
+            nm_dhcp_option_add_option(options, priv->config.addr_family, option, str);
+        } else {
+            g_hash_table_insert(
+                options,
+                g_strdup(nm_dhcp_option_request_string(priv->config.addr_family, option)),
+                g_steal_pointer(&str));
+        }
+    }
+
     return options;
 }
 
 /*****************************************************************************/
 
-void
+gboolean
 nm_dhcp_client_set_effective_client_id(NMDhcpClient *self, GBytes *client_id)
 {
     NMDhcpClientPrivate *priv    = NM_DHCP_CLIENT_GET_PRIVATE(self);
     gs_free char        *tmp_str = NULL;
 
-    g_return_if_fail(NM_IS_DHCP_CLIENT(self));
-    g_return_if_fail(!client_id || g_bytes_get_size(client_id) >= 2);
+    g_return_val_if_fail(NM_IS_DHCP_CLIENT(self), FALSE);
+    g_return_val_if_fail(!client_id || g_bytes_get_size(client_id) >= 2, FALSE);
 
     priv = NM_DHCP_CLIENT_GET_PRIVATE(self);
 
     if (nm_g_bytes_equal0(priv->effective_client_id, client_id))
-        return;
+        return FALSE;
 
     g_bytes_unref(priv->effective_client_id);
     priv->effective_client_id = nm_g_bytes_ref(client_id);
@@ -271,6 +293,8 @@ nm_dhcp_client_set_effective_client_id(NMDhcpClient *self, GBytes *client_id)
           priv->effective_client_id
               ? (tmp_str = nm_dhcp_utils_duid_to_string(priv->effective_client_id))
               : "default");
+
+    return TRUE;
 }
 
 /*****************************************************************************/
@@ -1448,7 +1472,7 @@ nm_dhcp_client_stop(NMDhcpClient *self, gboolean release)
 /*****************************************************************************/
 
 static char *
-bytearray_variant_to_string(NMDhcpClient *self, GVariant *value, const char *key)
+bytearray_variant_to_string(GVariant *value)
 {
     const guint8 *array;
     char         *str;
@@ -1498,8 +1522,9 @@ label_is_unknown_xyz(const char *label)
 static void
 maybe_add_option(NMDhcpClient *self, GHashTable *hash, const char *key, GVariant *value)
 {
-    char *str_value;
-    int   priv_opt_num;
+    const int IS_IPv4 = NM_IS_IPv4(NM_DHCP_CLIENT_GET_PRIVATE(self)->config.addr_family);
+    char     *str_value;
+    int       priv_opt_num;
 
     if (!g_variant_is_of_type(value, G_VARIANT_TYPE_BYTESTRING))
         return;
@@ -1516,10 +1541,32 @@ maybe_add_option(NMDhcpClient *self, GHashTable *hash, const char *key, GVariant
     if (NM_STR_HAS_PREFIX(key, "private_") || !key[0])
         return;
 
-    str_value = bytearray_variant_to_string(self, value, key);
+    str_value = bytearray_variant_to_string(value);
     if (!str_value)
         return;
 
+    if ((IS_IPv4 && nm_streq(key, "dhcp_client_identifier"))
+        || (!IS_IPv4 && nm_streq(key, "dhcp6_client_id"))) {
+        gs_free char          *str   = g_steal_pointer(&str_value);
+        gs_unref_bytes GBytes *bytes = NULL;
+
+        /* Validate and normalize the client-id/DUID. */
+
+        bytes = nm_utils_hexstr2bin(str);
+        if (!bytes || g_bytes_get_size(bytes) < 2) {
+            /* Seems invalid. Ignore */
+            return;
+        }
+
+        if (!nm_dhcp_client_set_effective_client_id(self, bytes)) {
+            /* the client-id is identical and we already set it. Nothing to do. */
+            return;
+        }
+
+        /* The effective-client-id was (re)set. Update "hash" with the new value... */
+        str_value = nm_dhcp_utils_duid_to_string(bytes);
+    }
+
     g_hash_table_insert(hash, g_strdup(key), str_value);
 
     /* dhclient has no special labels for private dhcp options: it uses "unknown_xyz"
diff --git a/src/core/dhcp/nm-dhcp-client.h b/src/core/dhcp/nm-dhcp-client.h
index d57d446e9f..6f403b6154 100644
--- a/src/core/dhcp/nm-dhcp-client.h
+++ b/src/core/dhcp/nm-dhcp-client.h
@@ -274,8 +274,8 @@ const char        *nm_dhcp_client_get_iface(NMDhcpClient *self);
 NMDedupMultiIndex *nm_dhcp_client_get_multi_idx(NMDhcpClient *self);
 int                nm_dhcp_client_get_ifindex(NMDhcpClient *self);
 
-void    nm_dhcp_client_set_effective_client_id(NMDhcpClient *self, GBytes *client_id);
-GBytes *nm_dhcp_client_get_effective_client_id(NMDhcpClient *self);
+gboolean nm_dhcp_client_set_effective_client_id(NMDhcpClient *self, GBytes *client_id);
+GBytes  *nm_dhcp_client_get_effective_client_id(NMDhcpClient *self);
 
 NML3ConfigData *nm_dhcp_client_create_l3cd(NMDhcpClient *self);
 

From 919d66f049bedc2dbe06c8391bc59d0c533fedc4 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 14 Dec 2022 08:58:36 +0100
Subject: [PATCH 11/18] dhcp: don't destroy old value before setting new in
 nm_dhcp_client_set_effective_client_id()

Of course, the old "priv->effective_client_id" and the new
"client_id" instances are truly separate, that is, they don't
share data, and destroying "priv->effective_client_id" before
taking a reference on "client_id" causes no problem.

It's still a code smell. It makes the function unnecessarily unsafe
under (very unusual) circumstances.

(cherry picked from commit a3e4f764d1a668a5a806d6e80d575189722f4d14)
---
 src/core/dhcp/nm-dhcp-client.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/core/dhcp/nm-dhcp-client.c b/src/core/dhcp/nm-dhcp-client.c
index b6f9071e63..a77127ce33 100644
--- a/src/core/dhcp/nm-dhcp-client.c
+++ b/src/core/dhcp/nm-dhcp-client.c
@@ -274,8 +274,9 @@ nm_dhcp_client_create_options_dict(NMDhcpClient *self, gboolean static_keys)
 gboolean
 nm_dhcp_client_set_effective_client_id(NMDhcpClient *self, GBytes *client_id)
 {
-    NMDhcpClientPrivate *priv    = NM_DHCP_CLIENT_GET_PRIVATE(self);
-    gs_free char        *tmp_str = NULL;
+    NMDhcpClientPrivate   *priv              = NM_DHCP_CLIENT_GET_PRIVATE(self);
+    gs_free char          *tmp_str           = NULL;
+    gs_unref_bytes GBytes *client_id_to_free = NULL;
 
     g_return_val_if_fail(NM_IS_DHCP_CLIENT(self), FALSE);
     g_return_val_if_fail(!client_id || g_bytes_get_size(client_id) >= 2, FALSE);
@@ -285,7 +286,7 @@ nm_dhcp_client_set_effective_client_id(NMDhcpClient *self, GBytes *client_id)
     if (nm_g_bytes_equal0(priv->effective_client_id, client_id))
         return FALSE;
 
-    g_bytes_unref(priv->effective_client_id);
+    client_id_to_free         = g_steal_pointer(&priv->effective_client_id);
     priv->effective_client_id = nm_g_bytes_ref(client_id);
 
     _LOGT("%s: set effective %s",

From 771589e2762a4cae483892920f76a3d23de9be5b Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 14 Dec 2022 10:23:27 +0100
Subject: [PATCH 12/18] dhcp/trivial: rename DUID_PREFIX define to
 DEFAULT_DUID_PREFIX

(cherry picked from commit df0408f0f6a9c38e8c9a4aed43efa02cb23de9b6)
---
 src/core/dhcp/nm-dhcp-dhclient-utils.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/core/dhcp/nm-dhcp-dhclient-utils.c b/src/core/dhcp/nm-dhcp-dhclient-utils.c
index 72dbc7885e..ae6828ad0a 100644
--- a/src/core/dhcp/nm-dhcp-dhclient-utils.c
+++ b/src/core/dhcp/nm-dhcp-dhclient-utils.c
@@ -606,7 +606,7 @@ error:
     return NULL;
 }
 
-#define DUID_PREFIX "default-duid \""
+#define DEFAULT_DUID_PREFIX "default-duid \""
 
 /* Beware: @error may be unset even if the function returns %NULL. */
 GBytes *
@@ -627,10 +627,10 @@ nm_dhcp_dhclient_read_duid(const char *leasefile, GError **error)
         const char *p = nm_str_skip_leading_spaces(contents_v[i]);
         GBytes     *duid;
 
-        if (!NM_STR_HAS_PREFIX(p, DUID_PREFIX))
+        if (!NM_STR_HAS_PREFIX(p, DEFAULT_DUID_PREFIX))
             continue;
 
-        p += NM_STRLEN(DUID_PREFIX);
+        p += NM_STRLEN(DEFAULT_DUID_PREFIX);
 
         g_strchomp((char *) p);
 
@@ -678,7 +678,7 @@ nm_dhcp_dhclient_save_duid(const char *leasefile, GBytes *duid, GError **error)
     }
 
     s = g_string_sized_new(len + 50);
-    g_string_append_printf(s, DUID_PREFIX "%s\";\n", escaped_duid);
+    g_string_append_printf(s, DEFAULT_DUID_PREFIX "%s\";\n", escaped_duid);
 
     /* Preserve existing leasefile contents */
     if (lines) {
@@ -691,7 +691,7 @@ nm_dhcp_dhclient_save_duid(const char *leasefile, GBytes *duid, GError **error)
              * to update the lease file, otherwise skip the old DUID.
              */
             l = nm_str_skip_leading_spaces(str);
-            if (g_str_has_prefix(l, DUID_PREFIX)) {
+            if (g_str_has_prefix(l, DEFAULT_DUID_PREFIX)) {
                 gs_strfreev char **split = NULL;
 
                 split = g_strsplit(l, "\"", -1);

From c13cc6fb0f4c2b1bc4a3e52dbc050a214beb2fdc Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 14 Dec 2022 10:37:10 +0100
Subject: [PATCH 13/18] glib-aux: add "with_leading_zero" to
 nm_utils_bin2hexstr_full()

dhclient writes binary data as colon-separated hex strings
like nm_utils_bin2hexstr_full() does. But it only writes single
digits for values smaller than 0x10. Add an option to support
that mode.

However, there are many callers of nm_utils_bin2hexstr_full() already,
and they all don't care about the new option. Maybe this should this
not be a boolean argument, instead the function should accept a
flags argument. That is not done for now. Just add another "fuller"
variant. It's still easy to understand, because the "full" variant
is just a more limited functionality of "fuller".

(cherry picked from commit b23c505fca64718bcb28d76d5884110ae64461cb)
---
 src/libnm-glib-aux/nm-shared-utils.c | 47 +++++++++++++++++-----------
 src/libnm-glib-aux/nm-shared-utils.h | 21 ++++++++++---
 2 files changed, 45 insertions(+), 23 deletions(-)

diff --git a/src/libnm-glib-aux/nm-shared-utils.c b/src/libnm-glib-aux/nm-shared-utils.c
index d0885477fe..aca7d70865 100644
--- a/src/libnm-glib-aux/nm-shared-utils.c
+++ b/src/libnm-glib-aux/nm-shared-utils.c
@@ -4846,7 +4846,7 @@ nm_utils_memeqzero(gconstpointer data, gsize length)
 }
 
 /**
- * nm_utils_bin2hexstr_full:
+ * nm_utils_bin2hexstr_fuller:
  * @addr: pointer of @length bytes. If @length is zero, this may
  *   also be %NULL.
  * @length: number of bytes in @addr. May also be zero, in which
@@ -4854,12 +4854,17 @@ nm_utils_memeqzero(gconstpointer data, gsize length)
  * @delimiter: either '\0', otherwise the output string will have the
  *   given delimiter character between each two hex numbers.
  * @upper_case: if TRUE, use upper case ASCII characters for hex.
+ * @with_leading_zero: if TRUE, then the hex values from 0 to 0xf
+ *   are written as "00" to "0f", respectively. Otherwise, the leading
+ *   zero is dropped. With @with_leading_zero set to FALSE, the resulting
+ *   string may be shorter than expected. @delimiter must be set
+ *   if @with_leading_zero is FALSE.
  * @out: if %NULL, the function will allocate a new buffer of
- *   either (@length*2+1) or (@length*3) bytes, depending on whether
+ *   either (@length*2+1) or MAX(1, (@length*3)) bytes, depending on whether
  *   a @delimiter is specified. In that case, the allocated buffer will
  *   be returned and must be freed by the caller.
  *   If not %NULL, the buffer must already be preallocated and contain
- *   at least (@length*2+1) or (@length*3) bytes, depending on the delimiter.
+ *   at least (@length*2+1) or MAX(1, (@length*3)) bytes, depending on the delimiter.
  *   If @length is zero, then of course at least one byte will be allocated
  *   or @out (if given) must contain at least room for the trailing NUL byte.
  *
@@ -4869,37 +4874,43 @@ nm_utils_memeqzero(gconstpointer data, gsize length)
  *   an empty string is returned.
  */
 char *
-nm_utils_bin2hexstr_full(gconstpointer addr,
-                         gsize         length,
-                         char          delimiter,
-                         gboolean      upper_case,
-                         char         *out)
+nm_utils_bin2hexstr_fuller(gconstpointer addr,
+                           gsize         length,
+                           char          delimiter,
+                           gboolean      upper_case,
+                           gboolean      with_leading_zero,
+                           char         *out)
 {
     const guint8 *in     = addr;
     const char   *LOOKUP = upper_case ? "0123456789ABCDEF" : "0123456789abcdef";
     char         *out0;
 
-    if (out)
-        out0 = out;
-    else {
-        out0 = out =
-            g_new(char, length == 0 ? 1u : (delimiter == '\0' ? length * 2u + 1u : length * 3u));
-    }
+    nm_assert(with_leading_zero || delimiter != '\0');
 
-    /* @out must contain at least @length*3 bytes if @delimiter is set,
+    /* @out must contain at least (MAX(1, @length*3)) bytes if @delimiter is set,
      * otherwise, @length*2+1. */
 
+    if (!out)
+        out = g_new(char, length == 0 ? 1u : (delimiter == '\0' ? length * 2u + 1u : length * 3u));
+
+    out0 = out;
+
     if (length > 0) {
         nm_assert(in);
         for (;;) {
             const guint8 v = *in++;
+            guint8       v_hi;
 
-            *out++ = LOOKUP[v >> 4];
+            v_hi = (v >> 4);
+            if (v_hi != 0 || with_leading_zero) {
+                nm_assert(v_hi < 16);
+                *out++ = LOOKUP[v_hi];
+            }
             *out++ = LOOKUP[v & 0x0F];
             length--;
-            if (!length)
+            if (length == 0)
                 break;
-            if (delimiter)
+            if (delimiter != '\0')
                 *out++ = delimiter;
         }
     }
diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h
index fe7c59f3ce..53cf7f3e57 100644
--- a/src/libnm-glib-aux/nm-shared-utils.h
+++ b/src/libnm-glib-aux/nm-shared-utils.h
@@ -2842,11 +2842,22 @@ nm_ascii_is_regular(char ch)
     return ch >= ' ' && ch < 127;
 }
 
-char *nm_utils_bin2hexstr_full(gconstpointer addr,
-                               gsize         length,
-                               char          delimiter,
-                               gboolean      upper_case,
-                               char         *out);
+char *nm_utils_bin2hexstr_fuller(gconstpointer addr,
+                                 gsize         length,
+                                 char          delimiter,
+                                 gboolean      upper_case,
+                                 gboolean      with_leading_zero,
+                                 char         *out);
+
+static inline char *
+nm_utils_bin2hexstr_full(gconstpointer addr,
+                         gsize         length,
+                         char          delimiter,
+                         gboolean      upper_case,
+                         char         *out)
+{
+    return nm_utils_bin2hexstr_fuller(addr, length, delimiter, upper_case, TRUE, out);
+}
 
 char *_nm_utils_bin2hexstr(gconstpointer src, gsize len, int final_len);
 

From 874ade4f9d17233e1eee9cfd02568c8010f4969f Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 14 Dec 2022 11:53:42 +0100
Subject: [PATCH 14/18] dhcp/tests: refactor tests for
 nm_dhcp_dhclient_save_duid()

So much duplicate, boilerplate code. Get rid of it.

(cherry picked from commit 5ee2f3d1dc9d37803b40480dc54c3b37ca6a9780)
---
 src/core/dhcp/tests/test-dhcp-dhclient.c | 166 ++++++++---------------
 1 file changed, 59 insertions(+), 107 deletions(-)

diff --git a/src/core/dhcp/tests/test-dhcp-dhclient.c b/src/core/dhcp/tests/test-dhcp-dhclient.c
index aaa0dbc3e0..6f53e1ade3 100644
--- a/src/core/dhcp/tests/test-dhcp-dhclient.c
+++ b/src/core/dhcp/tests/test-dhcp-dhclient.c
@@ -895,122 +895,79 @@ test_read_commented_duid_from_leasefile(void)
 /*****************************************************************************/
 
 static void
-_save_duid(const char *path, const guint8 *duid_bin, gsize duid_len)
+_check_duid_impl(const guint8 *duid_bin,
+                 gsize         duid_len,
+                 gboolean      enforce_duid, /* Unused at the moment. */
+                 const char   *old_content,
+                 const char   *new_content)
 {
-    gs_unref_bytes GBytes *duid  = NULL;
-    GError                *error = NULL;
+    gs_free_error GError  *error    = NULL;
+    gs_free char          *contents = NULL;
     gboolean               success;
+    const char            *path = NM_BUILD_SRCDIR "/src/core/dhcp/tests/check-duid.lease";
+    gs_unref_bytes GBytes *duid = NULL;
+    gsize                  contents_len;
 
-    g_assert(path);
     g_assert(duid_bin);
     g_assert(duid_len > 0);
 
-    duid    = g_bytes_new(duid_bin, duid_len);
+    if (!nm_str_is_empty(old_content) || nmtst_get_rand_bool()) {
+        success = g_file_set_contents(path, old_content ?: "", -1, &error);
+        nmtst_assert_success(success, error);
+    } else
+        nmtst_file_unlink_if_exists(path);
+
+    duid = g_bytes_new(duid_bin, duid_len);
+
     success = nm_dhcp_dhclient_save_duid(path, duid, &error);
     nmtst_assert_success(success, error);
+
+    success = g_file_get_contents(path, &contents, &contents_len, &error);
+    nmtst_assert_success(success, error);
+    g_assert(contents);
+
+    nmtst_file_unlink(path);
+
+    if (!nm_streq0(new_content, contents))
+        g_error("FAILING:\n\nEXPECTED:\n%s\nACTUAL:\n%s\n\n", new_content, contents);
+
+    g_assert_cmpstr(new_content, ==, contents);
+    g_assert_cmpint(contents_len, ==, strlen(contents));
 }
 
+#define _DUID(...) ((const guint8[]){__VA_ARGS__})
+
+#define _check_duid(duid, enforce_duid, old_content, new_content) \
+    _check_duid_impl((duid), sizeof(duid), (enforce_duid), (old_content), (new_content))
+
 static void
 test_write_duid(void)
 {
-    const guint8 duid[] = {000, 001, 000, 001, 027, 'X', 0350, 'X', 0, '#', 025, 010, '~', 0254};
-    const char  *expected_contents =
-        "default-duid \"\\000\\001\\000\\001\\027X\\350X\\000#\\025\\010~\\254\";\n";
-    GError       *error    = NULL;
-    gs_free char *contents = NULL;
-    gboolean      success;
-    const char   *path = "test-dhclient-write-duid.leases";
+    _check_duid(_DUID(000, 001, 000, 001, 027, 'X', 0350, 'X', 0, '#', 025, 010, '~', 0254),
+                FALSE,
+                NULL,
+                "default-duid \"\\000\\001\\000\\001\\027X\\350X\\000#\\025\\010~\\254\";\n");
 
-    _save_duid(path, duid, G_N_ELEMENTS(duid));
+    _check_duid(
+        _DUID(000, 001, 000, 001, 023, 'o', 023, 'n', 000, '"', 0372, 0214, 0326, 0302),
+        FALSE,
+        "default-duid \"\\000\\001\\000\\001\\027X\\350X\\000#\\025\\010~\\254\";\n",
+        "default-duid \"\\000\\001\\000\\001\\023o\\023n\\000\\\"\\372\\214\\326\\302\";\n");
 
-    success = g_file_get_contents(path, &contents, NULL, &error);
-    nmtst_assert_success(success, error);
-
-    unlink(path);
-
-    g_assert_cmpstr(expected_contents, ==, contents);
-}
-
-static void
-test_write_existing_duid(void)
-{
-    const guint8 duid[] =
-        {000, 001, 000, 001, 023, 'o', 023, 'n', 000, '"', 0372, 0214, 0326, 0302};
-    const char *original_contents =
-        "default-duid \"\\000\\001\\000\\001\\027X\\350X\\000#\\025\\010~\\254\";\n";
-    const char *expected_contents =
-        "default-duid \"\\000\\001\\000\\001\\023o\\023n\\000\\\"\\372\\214\\326\\302\";\n";
-    GError       *error    = NULL;
-    gs_free char *contents = NULL;
-    gboolean      success;
-    const char   *path = "test-dhclient-write-existing-duid.leases";
-
-    success = g_file_set_contents(path, original_contents, -1, &error);
-    nmtst_assert_success(success, error);
-
-    /* Save other DUID; should be overwritten */
-    _save_duid(path, duid, G_N_ELEMENTS(duid));
-
-    /* reread original contents */
-    success = g_file_get_contents(path, &contents, NULL, &error);
-    nmtst_assert_success(success, error);
-
-    unlink(path);
-    g_assert_cmpstr(expected_contents, ==, contents);
-}
-
-static const guint8 DUID_BIN[] =
-    {000, 001, 000, 001, 023, 'o', 023, 'n', 000, '"', 0372, 0214, 0326, 0302};
-#define DUID "\\000\\001\\000\\001\\023o\\023n\\000\\\"\\372\\214\\326\\302"
-
-static void
-test_write_existing_commented_duid(void)
-{
-#define ORIG_CONTENTS "#default-duid \"\\000\\001\\000\\001\\027X\\350X\\000#\\025\\010~\\254\";\n"
-    const char   *expected_contents = "default-duid \"" DUID "\";\n" ORIG_CONTENTS;
-    GError       *error             = NULL;
-    gs_free char *contents          = NULL;
-    gboolean      success;
-    const char   *path = "test-dhclient-write-existing-commented-duid.leases";
-
-    success = g_file_set_contents(path, ORIG_CONTENTS, -1, &error);
-    nmtst_assert_success(success, error);
-
-    /* Save other DUID; should be saved on top */
-    _save_duid(path, DUID_BIN, G_N_ELEMENTS(DUID_BIN));
-
-    /* reread original contents */
-    success = g_file_get_contents(path, &contents, NULL, &error);
-    nmtst_assert_success(success, error);
-
-    unlink(path);
-    g_assert_cmpstr(expected_contents, ==, contents);
-#undef ORIG_CONTENTS
-}
-
-static void
-test_write_existing_multiline_duid(void)
-{
-#define ORIG_CONTENTS              \
-    "### Commented old DUID ###\n" \
-    "#default-duid \"\\000\\001\\000\\001\\027X\\350X\\000#\\025\\010~\\254\";\n"
-    const char                 *expected_contents = "default-duid \"" DUID "\";\n" ORIG_CONTENTS;
-    GError                     *error             = NULL;
-    gs_free char               *contents          = NULL;
-    gboolean                    success;
-    nmtst_auto_unlinkfile char *path =
-        g_strdup("test-dhclient-write-existing-multiline-duid.leases");
-
-    success = g_file_set_contents(path, ORIG_CONTENTS, -1, &error);
-    nmtst_assert_success(success, error);
-
-    _save_duid(path, DUID_BIN, G_N_ELEMENTS(DUID_BIN));
-
-    success = g_file_get_contents(path, &contents, NULL, &error);
-    nmtst_assert_success(success, error);
-
-    g_assert_cmpstr(expected_contents, ==, contents);
-#undef ORIG_CONTENTS
+    _check_duid(_DUID(000, 001, 000, 001, 023, 'o', 023, 'n', 000, '"', 0372, 0214, 0326, 0302),
+                FALSE,
+                "#default-duid \"\\000\\001\\000\\001\\027X\\350X\\000#\\025\\010~\\254\";\n",
+                "default-duid "
+                "\"\\000\\001\\000\\001\\023o\\023n\\000\\\"\\372\\214\\326\\302\";\n#default-duid "
+                "\"\\000\\001\\000\\001\\027X\\350X\\000#\\025\\010~\\254\";\n");
+    _check_duid(
+        _DUID(000, 001, 000, 001, 023, 'o', 023, 'n', 000, '"', 0372, 0214, 0326, 0302),
+        FALSE,
+        "### Commented old DUID ###\n#default-duid "
+        "\"\\000\\001\\000\\001\\027X\\350X\\000#\\025\\010~\\254\";\n",
+        "default-duid \"\\000\\001\\000\\001\\023o\\023n\\000\\\"\\372\\214\\326\\302\";\n### "
+        "Commented old DUID ###\n#default-duid "
+        "\"\\000\\001\\000\\001\\027X\\350X\\000#\\025\\010~\\254\";\n");
 }
 
 /*****************************************************************************/
@@ -1329,12 +1286,7 @@ main(int argc, char **argv)
     g_test_add_func("/dhcp/dhclient/read_commented_duid_from_leasefile",
                     test_read_commented_duid_from_leasefile);
 
-    g_test_add_func("/dhcp/dhclient/write_duid", test_write_duid);
-    g_test_add_func("/dhcp/dhclient/write_existing_duid", test_write_existing_duid);
-    g_test_add_func("/dhcp/dhclient/write_existing_commented_duid",
-                    test_write_existing_commented_duid);
-    g_test_add_func("/dhcp/dhclient/write_existing_multiline_duid",
-                    test_write_existing_multiline_duid);
+    g_test_add_func("/dhcp/dhclient/test_write_duid", test_write_duid);
 
     return g_test_run();
 }

From b48da72a71cd18bae00f52b86e9921c5478100e4 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 14 Dec 2022 12:26:14 +0100
Subject: [PATCH 15/18] dhcp/tests: add more tests for
 nm_dhcp_dhclient_save_duid()

(cherry picked from commit 7d1cfec0b8154cd359f5ea3d3c80488572fb51e6)
---
 src/core/dhcp/tests/test-dhcp-dhclient.c | 96 ++++++++++++++++++++++++
 1 file changed, 96 insertions(+)

diff --git a/src/core/dhcp/tests/test-dhcp-dhclient.c b/src/core/dhcp/tests/test-dhcp-dhclient.c
index 6f53e1ade3..6a94673d78 100644
--- a/src/core/dhcp/tests/test-dhcp-dhclient.c
+++ b/src/core/dhcp/tests/test-dhcp-dhclient.c
@@ -968,6 +968,102 @@ test_write_duid(void)
         "default-duid \"\\000\\001\\000\\001\\023o\\023n\\000\\\"\\372\\214\\326\\302\";\n### "
         "Commented old DUID ###\n#default-duid "
         "\"\\000\\001\\000\\001\\027X\\350X\\000#\\025\\010~\\254\";\n");
+
+    _check_duid(
+        _DUID(0xaa, 0xb, 0xcc, 0xd, 0xee, 0xf),
+        FALSE,
+        "default-duid \"\\252\\013\\314\\015\\356\\017\";\nlease6 {\n  interface \"eth1\";\n "
+        " ia-na f1:ce:00:01 {\n    starts 1671015678;\n    renew 60;\n    rebind 105;\n    "
+        "iaaddr 192:168:121::1:112c {\n      starts 1671015678;\n      preferred-life 120;\n "
+        "     max-life 120;\n    }\n  }\n  option fqdn.encoded true;\n  option "
+        "fqdn.server-update true;\n  option fqdn.no-client-update false;\n  option fqdn.fqdn "
+        "\"dff6de4fcb0f\";\n  option fqdn.hostname \"dff6de4fcb0f\";\n  option dhcp6.client-id "
+        "aa:b:cc:d:ee:f;\n  option dhcp6.server-id 0:1:0:1:2b:2c:4d:1d:0:0:0:0:0:0;\n  option "
+        "dhcp6.name-servers 192:168:121:0:ce0f:f1ff:fece:1;\n  option dhcp6.fqdn "
+        "1:c:64:66:66:36:64:65:34:66:63:62:30:66;\n  option dhcp6.status-code success "
+        "\"success\";\n}\n",
+        "default-duid \"\\252\\013\\314\\015\\356\\017\";\nlease6 {\n  interface \"eth1\";\n "
+        " ia-na f1:ce:00:01 {\n    starts 1671015678;\n    renew 60;\n    rebind 105;\n    "
+        "iaaddr 192:168:121::1:112c {\n      starts 1671015678;\n      preferred-life 120;\n "
+        "     max-life 120;\n    }\n  }\n  option fqdn.encoded true;\n  option "
+        "fqdn.server-update true;\n  option fqdn.no-client-update false;\n  option fqdn.fqdn "
+        "\"dff6de4fcb0f\";\n  option fqdn.hostname \"dff6de4fcb0f\";\n  option dhcp6.client-id "
+        "aa:b:cc:d:ee:f;\n  option dhcp6.server-id 0:1:0:1:2b:2c:4d:1d:0:0:0:0:0:0;\n  option "
+        "dhcp6.name-servers 192:168:121:0:ce0f:f1ff:fece:1;\n  option dhcp6.fqdn "
+        "1:c:64:66:66:36:64:65:34:66:63:62:30:66;\n  option dhcp6.status-code success "
+        "\"success\";\n}\n");
+
+    _check_duid(
+        _DUID(0xaa, 0xb, 0xcc, 0xd, 0xee, 0xf),
+        FALSE,
+        "default-duid \"\\252\\013\\314\\015\\356\\017\";\nlease6 {\n  interface \"eth1\";\n "
+        " ia-na f1:ce:00:01 {\n    starts 1671015678;\n    renew 60;\n    rebind 105;\n    "
+        "iaaddr 192:168:121::1:112c {\n      starts 1671015678;\n      preferred-life 120;\n "
+        "     max-life 120;\n    }\n  }\n  option fqdn.encoded true;\n  option "
+        "fqdn.server-update true;\n  option fqdn.no-client-update false;\n  option fqdn.fqdn "
+        "\"dff6de4fcb0f\";\n  option fqdn.hostname \"dff6de4fcb0f\";\n  option dhcp6.client-id "
+        "aa:b:cc:d:ee:f;\n  option dhcp6.server-id 0:1:0:1:2b:2c:4d:1d:0:0:0:0:0:0;\n  option "
+        "dhcp6.name-servers 192:168:121:0:ce0f:f1ff:fece:1;\n  option dhcp6.fqdn "
+        "1:c:64:66:66:36:64:65:34:66:63:62:30:66;\n  option dhcp6.status-code success "
+        "\"success\";\r\n}\n",
+        "default-duid \"\\252\\013\\314\\015\\356\\017\";\nlease6 {\n  interface \"eth1\";\n "
+        " ia-na f1:ce:00:01 {\n    starts 1671015678;\n    renew 60;\n    rebind 105;\n    "
+        "iaaddr 192:168:121::1:112c {\n      starts 1671015678;\n      preferred-life 120;\n "
+        "     max-life 120;\n    }\n  }\n  option fqdn.encoded true;\n  option "
+        "fqdn.server-update true;\n  option fqdn.no-client-update false;\n  option fqdn.fqdn "
+        "\"dff6de4fcb0f\";\n  option fqdn.hostname \"dff6de4fcb0f\";\n  option dhcp6.client-id "
+        "aa:b:cc:d:ee:f;\n  option dhcp6.server-id 0:1:0:1:2b:2c:4d:1d:0:0:0:0:0:0;\n  option "
+        "dhcp6.name-servers 192:168:121:0:ce0f:f1ff:fece:1;\n  option dhcp6.fqdn "
+        "1:c:64:66:66:36:64:65:34:66:63:62:30:66;\n  option dhcp6.status-code success "
+        "\"success\";\r\n}\n");
+
+    _check_duid(
+        _DUID(0xaa, 0xb, 0xcc, 0xd, 0xee, 0xe),
+        FALSE,
+        "default-duid \"\\252\\013\\314\\015\\356\\017\";\nlease6 {\n  interface \"eth1\";\n "
+        " ia-na f1:ce:00:01 {\n    starts 1671015678;\n    renew 60;\n    rebind 105;\n    "
+        "iaaddr 192:168:121::1:112c {\n      starts 1671015678;\n      preferred-life 120;\n "
+        "     max-life 120;\n    }\n  }\n  option fqdn.encoded true;\n  option "
+        "fqdn.server-update true;\n  option fqdn.no-client-update false;\n  option fqdn.fqdn "
+        "\"dff6de4fcb0f\";\n  option fqdn.hostname \"dff6de4fcb0f\";\n  option dhcp6.client-id "
+        "aa:b:cc:d:ee:f;\n  option dhcp6.server-id 0:1:0:1:2b:2c:4d:1d:0:0:0:0:0:0;\n  option "
+        "dhcp6.name-servers 192:168:121:0:ce0f:f1ff:fece:1;\n  option dhcp6.fqdn "
+        "1:c:64:66:66:36:64:65:34:66:63:62:30:66;\n  option dhcp6.status-code success "
+        "\"success\";\r\n}\n",
+        "default-duid \"\\252\\013\\314\\015\\356\\016\";\nlease6 {\n  interface \"eth1\";\n "
+        " ia-na f1:ce:00:01 {\n    starts 1671015678;\n    renew 60;\n    rebind 105;\n    "
+        "iaaddr 192:168:121::1:112c {\n      starts 1671015678;\n      preferred-life 120;\n "
+        "     max-life 120;\n    }\n  }\n  option fqdn.encoded true;\n  option "
+        "fqdn.server-update true;\n  option fqdn.no-client-update false;\n  option fqdn.fqdn "
+        "\"dff6de4fcb0f\";\n  option fqdn.hostname \"dff6de4fcb0f\";\n  option dhcp6.client-id "
+        "aa:b:cc:d:ee:f;\n  option dhcp6.server-id 0:1:0:1:2b:2c:4d:1d:0:0:0:0:0:0;\n  option "
+        "dhcp6.name-servers 192:168:121:0:ce0f:f1ff:fece:1;\n  option dhcp6.fqdn "
+        "1:c:64:66:66:36:64:65:34:66:63:62:30:66;\n  option dhcp6.status-code success "
+        "\"success\";\n\n}\n");
+
+    _check_duid(
+        _DUID(0xaa, 0xb, 0xcc, 0xd, 0xee, 0xe),
+        TRUE,
+        "default-duid \"\\252\\013\\314\\015\\356\\017\";\nlease6 {\n  interface \"eth1\";\n "
+        " ia-na f1:ce:00:01 {\n    starts 1671015678;\n    renew 60;\n    rebind 105;\n    "
+        "iaaddr 192:168:121::1:112c {\n      starts 1671015678;\n      preferred-life 120;\n "
+        "     max-life 120;\n    }\n  }\n  option fqdn.encoded true;\n  option "
+        "fqdn.server-update true;\n  option fqdn.no-client-update false;\n  option fqdn.fqdn "
+        "\"dff6de4fcb0f\";\n  option fqdn.hostname \"dff6de4fcb0f\";\n  option dhcp6.client-id "
+        "aa:b:cc:d:ee:f;\n  option dhcp6.server-id 0:1:0:1:2b:2c:4d:1d:0:0:0:0:0:0;\n  option "
+        "dhcp6.name-servers 192:168:121:0:ce0f:f1ff:fece:1;\n  option dhcp6.fqdn "
+        "1:c:64:66:66:36:64:65:34:66:63:62:30:66;\n  option dhcp6.status-code success "
+        "\"success\";\n}\n",
+        "default-duid \"\\252\\013\\314\\015\\356\\016\";\nlease6 {\n  interface \"eth1\";\n "
+        " ia-na f1:ce:00:01 {\n    starts 1671015678;\n    renew 60;\n    rebind 105;\n    "
+        "iaaddr 192:168:121::1:112c {\n      starts 1671015678;\n      preferred-life 120;\n "
+        "     max-life 120;\n    }\n  }\n  option fqdn.encoded true;\n  option "
+        "fqdn.server-update true;\n  option fqdn.no-client-update false;\n  option fqdn.fqdn "
+        "\"dff6de4fcb0f\";\n  option fqdn.hostname \"dff6de4fcb0f\";\n  option dhcp6.client-id "
+        "aa:b:cc:d:ee:f;\n  option dhcp6.server-id 0:1:0:1:2b:2c:4d:1d:0:0:0:0:0:0;\n  option "
+        "dhcp6.name-servers 192:168:121:0:ce0f:f1ff:fece:1;\n  option dhcp6.fqdn "
+        "1:c:64:66:66:36:64:65:34:66:63:62:30:66;\n  option dhcp6.status-code success "
+        "\"success\";\n}\n");
 }
 
 /*****************************************************************************/

From 9a2d2c8522e621f7e7503d3ac28447bc7f70687f Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 19 Dec 2022 09:56:35 +0100
Subject: [PATCH 16/18] dhcp/dhclient: avoid rewriting unchanged file in
 nm_dhcp_dhclient_save_duid()

It updates the file timestamp, which seems undesirable. Skip the update,
if the content didn't change.

(cherry picked from commit 0e63fe58a7ea38250d79214be22eb2eab3f524d6)
---
 src/core/dhcp/nm-dhcp-dhclient-utils.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/core/dhcp/nm-dhcp-dhclient-utils.c b/src/core/dhcp/nm-dhcp-dhclient-utils.c
index ae6828ad0a..c959430b4e 100644
--- a/src/core/dhcp/nm-dhcp-dhclient-utils.c
+++ b/src/core/dhcp/nm-dhcp-dhclient-utils.c
@@ -654,7 +654,8 @@ nm_dhcp_dhclient_save_duid(const char *leasefile, GBytes *duid, GError **error)
     gs_free const char          **lines        = NULL;
     nm_auto_free_gstring GString *s            = NULL;
     const char *const            *iter;
-    gsize                         len = 0;
+    gs_free char                 *contents     = NULL;
+    gsize                         contents_len = 0;
 
     g_return_val_if_fail(leasefile != NULL, FALSE);
 
@@ -667,9 +668,7 @@ nm_dhcp_dhclient_save_duid(const char *leasefile, GBytes *duid, GError **error)
     nm_assert(escaped_duid);
 
     if (g_file_test(leasefile, G_FILE_TEST_EXISTS)) {
-        gs_free char *contents = NULL;
-
-        if (!g_file_get_contents(leasefile, &contents, &len, error)) {
+        if (!g_file_get_contents(leasefile, &contents, &contents_len, error)) {
             g_prefix_error(error, "failed to read lease file %s: ", leasefile);
             return FALSE;
         }
@@ -677,7 +676,7 @@ nm_dhcp_dhclient_save_duid(const char *leasefile, GBytes *duid, GError **error)
         lines = nm_strsplit_set_with_empty(contents, "\n\r");
     }
 
-    s = g_string_sized_new(len + 50);
+    s = g_string_sized_new(contents_len + 50);
     g_string_append_printf(s, DEFAULT_DUID_PREFIX "%s\";\n", escaped_duid);
 
     /* Preserve existing leasefile contents */
@@ -709,6 +708,11 @@ nm_dhcp_dhclient_save_duid(const char *leasefile, GBytes *duid, GError **error)
         }
     }
 
+    if (contents && strlen(contents) == contents_len && nm_streq(contents, s->str)) {
+        /* The file is already as we want it. We are done. */
+        return TRUE;
+    }
+
     if (!g_file_set_contents(leasefile, s->str, -1, error)) {
         g_prefix_error(error, "failed to set DUID in lease file %s: ", leasefile);
         return FALSE;

From b7d343af05675d2635312c74400d257869de0cdd Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Mon, 19 Dec 2022 09:58:50 +0100
Subject: [PATCH 17/18] dhcp/dhclient: better handle "\r\n" line breaks in
 dhclient lease file

Splitting by any of "\r\n" and then joining the lines with "\n"
leads to double-newlines. That's certainly wrong.

Maybe we shouldn't care about "\r", I don't know why this was done. But
handle it differently.

(cherry picked from commit c990d6a81a5b0f267681304f215f28e3d2c03211)
---
 src/core/dhcp/nm-dhcp-dhclient-utils.c   | 36 ++++++++++++++----------
 src/core/dhcp/tests/test-dhcp-dhclient.c |  2 +-
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/src/core/dhcp/nm-dhcp-dhclient-utils.c b/src/core/dhcp/nm-dhcp-dhclient-utils.c
index c959430b4e..5bc135f02f 100644
--- a/src/core/dhcp/nm-dhcp-dhclient-utils.c
+++ b/src/core/dhcp/nm-dhcp-dhclient-utils.c
@@ -673,7 +673,7 @@ nm_dhcp_dhclient_save_duid(const char *leasefile, GBytes *duid, GError **error)
             return FALSE;
         }
 
-        lines = nm_strsplit_set_with_empty(contents, "\n\r");
+        lines = nm_strsplit_set_with_empty(contents, "\n");
     }
 
     s = g_string_sized_new(contents_len + 50);
@@ -684,27 +684,33 @@ nm_dhcp_dhclient_save_duid(const char *leasefile, GBytes *duid, GError **error)
         for (iter = lines; *iter; iter++) {
             const char *str = *iter;
             const char *l;
+            gboolean    ends_with_r;
+            gsize       l_len;
+            gsize       prefix_len;
 
-            /* If we find an uncommented DUID in the file, check if
-             * equal to the one we are going to write: if so, no need
-             * to update the lease file, otherwise skip the old DUID.
-             */
-            l = nm_str_skip_leading_spaces(str);
-            if (g_str_has_prefix(l, DEFAULT_DUID_PREFIX)) {
-                gs_strfreev char **split = NULL;
+            l          = nm_str_skip_leading_spaces(str);
+            l_len      = strlen(l);
+            prefix_len = l - str;
 
-                split = g_strsplit(l, "\"", -1);
-                if (split[0] && nm_streq0(split[1], escaped_duid))
-                    return TRUE;
+            ends_with_r = l_len > 0 && l[l_len - 1u] == '\r';
+            if (ends_with_r) {
+                ((char *) l)[--l_len] = '\0';
+            }
 
+            if (NM_STR_HAS_PREFIX(l, DEFAULT_DUID_PREFIX)) {
+                /* We always add our line on top. This line can be skipped. */
                 continue;
             }
 
-            if (str)
-                g_string_append(s, str);
-            /* avoid to add an extra '\n' at the end of file */
-            if ((iter[1]) != NULL)
+            g_string_append_len(s, str, prefix_len);
+            g_string_append(s, l);
+            if (ends_with_r) {
+                g_string_append_c(s, '\r');
                 g_string_append_c(s, '\n');
+            } else if ((iter[1]) != NULL) {
+                /* avoid to add an extra '\n' at the end of file */
+                g_string_append_c(s, '\n');
+            }
         }
     }
 
diff --git a/src/core/dhcp/tests/test-dhcp-dhclient.c b/src/core/dhcp/tests/test-dhcp-dhclient.c
index 6a94673d78..6daf689145 100644
--- a/src/core/dhcp/tests/test-dhcp-dhclient.c
+++ b/src/core/dhcp/tests/test-dhcp-dhclient.c
@@ -1039,7 +1039,7 @@ test_write_duid(void)
         "aa:b:cc:d:ee:f;\n  option dhcp6.server-id 0:1:0:1:2b:2c:4d:1d:0:0:0:0:0:0;\n  option "
         "dhcp6.name-servers 192:168:121:0:ce0f:f1ff:fece:1;\n  option dhcp6.fqdn "
         "1:c:64:66:66:36:64:65:34:66:63:62:30:66;\n  option dhcp6.status-code success "
-        "\"success\";\n\n}\n");
+        "\"success\";\r\n}\n");
 
     _check_duid(
         _DUID(0xaa, 0xb, 0xcc, 0xd, 0xee, 0xe),

From 6bc03e9c9517b396aae0f0d0a87bc52d598cf756 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 14 Dec 2022 10:37:15 +0100
Subject: [PATCH 18/18] dhcp/dhclient: fix honoring "ipv6.dhcp-duid" when
 explicitly set

Previously, we only set the "default-duid" line in the lease file. That
means, if the lease already contained a matching entry with a
"dhcp6.client-id" option, it was not honored. That is wrong.

If the profile has "ipv6.dhcp-duid" set, then we must use it and get
rid of those options from the lease.

It's easy to reproduce:

    PROFILE=eth1

    nmcli connection down "$PROFILE"
    rm -f /var/lib/NetworkManager/*lease
    nmcli connection modify "$PROFILE" ipv6.dhcp-duid "aa:bb:cc:dd:00:00:11"
    nmcli connection up "$PROFILE"
    # Verify the expected duid in /var/lib/NetworkManager/*lease and "/run/NetworkManager/devices/$IFINDEX"

    nmcli connection modify "$PROFILE" ipv6.dhcp-duid "aa:bb:cc:dd:00:00:22"
    nmcli connection up "$PROFILE"
    # Check the DUID again.

(cherry picked from commit 1d85608e1c13545556cd90d053d2b959adc354dd)
---
 src/core/dhcp/nm-dhcp-dhclient-utils.c   | 35 ++++++++++++++++++++++--
 src/core/dhcp/nm-dhcp-dhclient-utils.h   |  5 +++-
 src/core/dhcp/nm-dhcp-dhclient.c         |  1 +
 src/core/dhcp/tests/test-dhcp-dhclient.c |  6 ++--
 4 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/src/core/dhcp/nm-dhcp-dhclient-utils.c b/src/core/dhcp/nm-dhcp-dhclient-utils.c
index 5bc135f02f..ea8943fa1e 100644
--- a/src/core/dhcp/nm-dhcp-dhclient-utils.c
+++ b/src/core/dhcp/nm-dhcp-dhclient-utils.c
@@ -524,6 +524,20 @@ nm_dhcp_dhclient_create_config(const char         *interface,
     return g_string_free(g_steal_pointer(&new_contents), FALSE);
 }
 
+/* In the lease file, dhclient will write "option dhcp6.client-id $HEXSTR". This
+ * function does the same. */
+static char *
+nm_dhcp_dhclient_escape_duid_as_hex(GBytes *duid)
+{
+    const guint8 *s;
+    gsize         len;
+
+    nm_assert(duid);
+
+    s = g_bytes_get_data(duid, &len);
+    return nm_utils_bin2hexstr_fuller(s, len, ':', FALSE, FALSE, NULL);
+}
+
 /* Roughly follow what dhclient's quotify_buf() and pretty_escape() functions do */
 char *
 nm_dhcp_dhclient_escape_duid(GBytes *duid)
@@ -648,14 +662,18 @@ nm_dhcp_dhclient_read_duid(const char *leasefile, GError **error)
 }
 
 gboolean
-nm_dhcp_dhclient_save_duid(const char *leasefile, GBytes *duid, GError **error)
+nm_dhcp_dhclient_save_duid(const char *leasefile,
+                           GBytes     *duid,
+                           gboolean    enforce_duid,
+                           GError    **error)
 {
     gs_free char                 *escaped_duid = NULL;
     gs_free const char          **lines        = NULL;
     nm_auto_free_gstring GString *s            = NULL;
     const char *const            *iter;
-    gs_free char                 *contents     = NULL;
-    gsize                         contents_len = 0;
+    gs_free char                 *conflicting_duid_line = NULL;
+    gs_free char                 *contents              = NULL;
+    gsize                         contents_len          = 0;
 
     g_return_val_if_fail(leasefile != NULL, FALSE);
 
@@ -702,6 +720,17 @@ nm_dhcp_dhclient_save_duid(const char *leasefile, GBytes *duid, GError **error)
                 continue;
             }
 
+            if (enforce_duid & NM_STR_HAS_PREFIX(l, "option dhcp6.client-id ")) {
+                /* we want to use our duid. Skip the per-lease client-id. */
+                if (!conflicting_duid_line) {
+                    gs_free char *duid_hex = nm_dhcp_dhclient_escape_duid_as_hex(duid);
+
+                    conflicting_duid_line = g_strdup_printf("option dhcp6.client-id %s;", duid_hex);
+                }
+                /* We adjust the duid line and set what we want. */
+                l = conflicting_duid_line;
+            }
+
             g_string_append_len(s, str, prefix_len);
             g_string_append(s, l);
             if (ends_with_r) {
diff --git a/src/core/dhcp/nm-dhcp-dhclient-utils.h b/src/core/dhcp/nm-dhcp-dhclient-utils.h
index 0ce7dd90f0..6187cce0fb 100644
--- a/src/core/dhcp/nm-dhcp-dhclient-utils.h
+++ b/src/core/dhcp/nm-dhcp-dhclient-utils.h
@@ -29,6 +29,9 @@ GBytes *nm_dhcp_dhclient_unescape_duid(const char *duid);
 
 GBytes *nm_dhcp_dhclient_read_duid(const char *leasefile, GError **error);
 
-gboolean nm_dhcp_dhclient_save_duid(const char *leasefile, GBytes *duid, GError **error);
+gboolean nm_dhcp_dhclient_save_duid(const char *leasefile,
+                                    GBytes     *duid,
+                                    gboolean    enforce_duid,
+                                    GError    **error);
 
 #endif /* __NETWORKMANAGER_DHCP_DHCLIENT_UTILS_H__ */
diff --git a/src/core/dhcp/nm-dhcp-dhclient.c b/src/core/dhcp/nm-dhcp-dhclient.c
index 7e578d13ad..e4f40d7cb0 100644
--- a/src/core/dhcp/nm-dhcp-dhclient.c
+++ b/src/core/dhcp/nm-dhcp-dhclient.c
@@ -418,6 +418,7 @@ dhclient_start(NMDhcpClient *client,
     if (set_duid && addr_family == AF_INET6) {
         if (!nm_dhcp_dhclient_save_duid(priv->lease_file,
                                         nm_dhcp_client_get_effective_client_id(client),
+                                        client_config->v6.enforce_duid,
                                         &local)) {
             nm_utils_error_set(error,
                                NM_UTILS_ERROR_UNKNOWN,
diff --git a/src/core/dhcp/tests/test-dhcp-dhclient.c b/src/core/dhcp/tests/test-dhcp-dhclient.c
index 6daf689145..937ac2e3d6 100644
--- a/src/core/dhcp/tests/test-dhcp-dhclient.c
+++ b/src/core/dhcp/tests/test-dhcp-dhclient.c
@@ -897,7 +897,7 @@ test_read_commented_duid_from_leasefile(void)
 static void
 _check_duid_impl(const guint8 *duid_bin,
                  gsize         duid_len,
-                 gboolean      enforce_duid, /* Unused at the moment. */
+                 gboolean      enforce_duid,
                  const char   *old_content,
                  const char   *new_content)
 {
@@ -919,7 +919,7 @@ _check_duid_impl(const guint8 *duid_bin,
 
     duid = g_bytes_new(duid_bin, duid_len);
 
-    success = nm_dhcp_dhclient_save_duid(path, duid, &error);
+    success = nm_dhcp_dhclient_save_duid(path, duid, enforce_duid, &error);
     nmtst_assert_success(success, error);
 
     success = g_file_get_contents(path, &contents, &contents_len, &error);
@@ -1060,7 +1060,7 @@ test_write_duid(void)
         "     max-life 120;\n    }\n  }\n  option fqdn.encoded true;\n  option "
         "fqdn.server-update true;\n  option fqdn.no-client-update false;\n  option fqdn.fqdn "
         "\"dff6de4fcb0f\";\n  option fqdn.hostname \"dff6de4fcb0f\";\n  option dhcp6.client-id "
-        "aa:b:cc:d:ee:f;\n  option dhcp6.server-id 0:1:0:1:2b:2c:4d:1d:0:0:0:0:0:0;\n  option "
+        "aa:b:cc:d:ee:e;\n  option dhcp6.server-id 0:1:0:1:2b:2c:4d:1d:0:0:0:0:0:0;\n  option "
         "dhcp6.name-servers 192:168:121:0:ce0f:f1ff:fece:1;\n  option dhcp6.fqdn "
         "1:c:64:66:66:36:64:65:34:66:63:62:30:66;\n  option dhcp6.status-code success "
         "\"success\";\n}\n");