dhcp: preserve DHCPv4 client ID for later use

If we can, read the existing client ID from the leasefile and preserve
it for later use.

9 files changed, 327 insertions, 83 deletions

diff --git a/src/dhcp-manager/nm-dhcp-client.c b/src/dhcp-manager/nm-dhcp-client.c
index d6e308b959..f455efa732 100644
--- a/src/dhcp-manager/nm-dhcp-client.c
+++ b/src/dhcp-manager/nm-dhcp-client.c
@@ -45,6 +45,7 @@ typedef struct {
 	guint32      priority;
 	guint32      timeout;
 	GByteArray * duid;
+	GBytes *     client_id;
 
 	NMDhcpState  state;
 	pid_t        pid;
@@ -143,6 +144,29 @@ nm_dhcp_client_get_priority (NMDhcpClient *self)
 	return NM_DHCP_CLIENT_GET_PRIVATE (self)->priority;
 }
 
+GBytes *
+nm_dhcp_client_get_client_id (NMDhcpClient *self)
+{
+	g_return_val_if_fail (NM_IS_DHCP_CLIENT (self), NULL);
+
+	return NM_DHCP_CLIENT_GET_PRIVATE (self)->client_id;
+}
+
+void
+nm_dhcp_client_set_client_id (NMDhcpClient *self, GBytes *client_id)
+{
+	NMDhcpClientPrivate *priv;
+
+	g_return_if_fail (NM_IS_DHCP_CLIENT (self));
+
+	priv = NM_DHCP_CLIENT_GET_PRIVATE (self);
+
+	if (priv->client_id && client_id && g_bytes_equal (priv->client_id, client_id))
+		return;
+	g_clear_pointer (&priv->client_id, g_bytes_unref);
+	priv->client_id = client_id ? g_bytes_ref (client_id) : NULL;
+}
+
 /********************************************/
 
 static const char *state_table[NM_DHCP_STATE_MAX + 1] = {
@@ -374,7 +398,9 @@ nm_dhcp_client_start_ip4 (NMDhcpClient *self,
 	nm_log_info (LOGD_DHCP, "Activation (%s) Beginning DHCPv4 transaction (timeout in %d seconds)",
 	             priv->iface, priv->timeout);
 
-	return NM_DHCP_CLIENT_GET_CLASS (self)->ip4_start (self, dhcp_client_id, dhcp_anycast_addr, hostname);
+	nm_dhcp_client_set_client_id (self, dhcp_client_id ? nm_dhcp_utils_client_id_string_to_bytes (dhcp_client_id) : NULL);
+
+	return NM_DHCP_CLIENT_GET_CLASS (self)->ip4_start (self, dhcp_anycast_addr, hostname);
 }
 
 /* uuid_parse does not work for machine-id, so we use our own converter */
diff --git a/src/dhcp-manager/nm-dhcp-client.h b/src/dhcp-manager/nm-dhcp-client.h
index a9ce9bec37..fda14cf4a5 100644
--- a/src/dhcp-manager/nm-dhcp-client.h
+++ b/src/dhcp-manager/nm-dhcp-client.h
@@ -64,7 +64,6 @@ typedef struct {
 	/* Methods */
 
 	gboolean (*ip4_start)     (NMDhcpClient *self,
-	                           const char *dhcp_client_id,
 	                           const char *anycast_addr,
 	                           const char *hostname);
 
@@ -126,6 +125,8 @@ const GByteArray *nm_dhcp_client_get_hw_addr (NMDhcpClient *self);
 
 guint32 nm_dhcp_client_get_priority (NMDhcpClient *self);
 
+GBytes *nm_dhcp_client_get_client_id (NMDhcpClient *self);
+
 gboolean nm_dhcp_client_start_ip4 (NMDhcpClient *self,
                                    const char *dhcp_client_id,
                                    const char *dhcp_anycast_addr,
@@ -158,5 +159,7 @@ gboolean nm_dhcp_client_handle_event (gpointer unused,
                                       const char *reason,
                                       NMDhcpClient *self);
 
+void nm_dhcp_client_set_client_id (NMDhcpClient *self, GBytes *client_id);
+
 #endif /* __NETWORKMANAGER_DHCP_CLIENT_H__ */
 
diff --git a/src/dhcp-manager/nm-dhcp-dhclient-utils.c b/src/dhcp-manager/nm-dhcp-dhclient-utils.c
index a53e95b009..3c3c42c84a 100644
--- a/src/dhcp-manager/nm-dhcp-dhclient-utils.c
+++ b/src/dhcp-manager/nm-dhcp-dhclient-utils.c
@@ -26,14 +26,14 @@
 #include <arpa/inet.h>
 
 #include "nm-dhcp-dhclient-utils.h"
+#include "nm-dhcp-utils.h"
 #include "nm-ip4-config.h"
 #include "nm-utils.h"
 #include "nm-platform.h"
 #include "NetworkManagerUtils.h"
+#include "gsystem-local-alloc.h"
 
 #define CLIENTID_TAG            "send dhcp-client-identifier"
-#define CLIENTID_FORMAT         CLIENTID_TAG " \"%s\"; # added by NetworkManager"
-#define CLIENTID_FORMAT_OCTETS  CLIENTID_TAG " %s; # added by NetworkManager"
 
 #define HOSTNAME4_TAG    "send host-name"
 #define HOSTNAME4_FORMAT HOSTNAME4_TAG " \"%s\"; # added by NetworkManager"
@@ -73,40 +73,39 @@ add_hostname (GString *str, const char *format, const char *hostname)
 }
 
 static void
-add_ip4_config (GString *str, const char *dhcp_client_id, const char *hostname)
+add_ip4_config (GString *str, GBytes *client_id, const char *hostname)
 {
-	if (dhcp_client_id && *dhcp_client_id) {
-		gboolean is_octets = TRUE;
-		int i = 0;
+	if (client_id) {
+		const char *p;
+		gsize l;
+		guint i;
 
-		while (dhcp_client_id[i]) {
-			if (!g_ascii_isxdigit (dhcp_client_id[i])) {
-				is_octets = FALSE;
-				break;
-			}
-			i++;
-			if (!dhcp_client_id[i])
-				break;
-			if (g_ascii_isxdigit (dhcp_client_id[i])) {
-				i++;
-				if (!dhcp_client_id[i])
-					break;
-			}
-			if (dhcp_client_id[i] != ':') {
-				is_octets = FALSE;
+		p = g_bytes_get_data (client_id, &l);
+		g_assert (p);
+
+		/* Allow type 0 (non-hardware address) to be represented as a string
+		 * as long as all the characters are printable.
+		 */
+		for (i = 1; (p[0] == 0) && i < l; i++) {
+			if (!g_ascii_isprint (p[i]))
 				break;
-			}
-			i++;
 		}
 
-		/* If the client ID is just hex digits and : then don't use quotes,
-		 * because dhclient expects either a quoted ASCII string, or a byte
-		 * array formated as hex octets separated by :
-		 */
-		if (is_octets)
-			g_string_append_printf (str, CLIENTID_FORMAT_OCTETS "\n", dhcp_client_id);
-		else
-			g_string_append_printf (str, CLIENTID_FORMAT "\n", dhcp_client_id);
+		g_string_append (str, CLIENTID_TAG " ");
+		if (i < l) {
+			/* Unprintable; convert to a hex string */
+			for (i = 0; i < l; i++) {
+				if (i > 0)
+					g_string_append_c (str, ':');
+				g_string_append_printf (str, "%02x", (guint8) p[i]);
+			}
+		} else {
+			/* Printable; just add to the line minus the 'type' */
+			g_string_append_c (str, '"');
+			g_string_append_len (str, p + 1, l - 1);
+			g_string_append_c (str, '"');
+		}
+		g_string_append (str, "; # added by NetworkManager\n");
 	}
 
 	add_hostname (str, HOSTNAME4_FORMAT "\n", hostname);
@@ -134,14 +133,67 @@ add_ip6_config (GString *str, const char *hostname)
 	                 "send fqdn.server-update on;\n");
 }
 
+static GBytes *
+read_client_id (const char *str)
+{
+	gs_free char *s = NULL;
+	char *p;
+
+	g_assert (!strncmp (str, CLIENTID_TAG, STRLEN (CLIENTID_TAG)));
+
+	str += STRLEN (CLIENTID_TAG);
+	while (g_ascii_isspace (*str))
+		str++;
+
+	if (*str == '"') {
+		s = g_strdup (str + 1);
+		p = strrchr (s, '"');
+		if (p)
+			*p = '\0';
+		else
+			return NULL;
+	} else
+		s = g_strdup (str);
+
+	g_strchomp (s);
+	if (s[strlen (s) - 1] == ';')
+		s[strlen (s) - 1] = '\0';
+
+	return nm_dhcp_utils_client_id_string_to_bytes (s);
+}
+
+GBytes *
+nm_dhcp_dhclient_get_client_id_from_config_file (const char *path)
+{
+	gs_free char *contents = NULL;
+	gs_strfreev char **lines = NULL;
+	char **line;
+
+	g_return_val_if_fail (path != NULL, NULL);
+
+	if (!g_file_test (path, G_FILE_TEST_EXISTS))
+		return NULL;
+
+	if (!g_file_get_contents (path, &contents, NULL, NULL))
+		return NULL;
+
+	lines = g_strsplit_set (contents, "\n\r", 0);
+	for (line = lines; lines && *line; line++) {
+		if (!strncmp (*line, CLIENTID_TAG, STRLEN (CLIENTID_TAG)))
+			return read_client_id (*line);
+	}
+	return NULL;
+}
+
 char *
 nm_dhcp_dhclient_create_config (const char *interface,
                                 gboolean is_ip6,
-                                const char *dhcp_client_id,
+                                GBytes *client_id,
                                 const char *anycast_addr,
                                 const char *hostname,
                                 const char *orig_path,
-                                const char *orig_contents)
+                                const char *orig_contents,
+                                GBytes **out_new_client_id)
 {
 	GString *new_contents;
 	GPtrArray *alsoreq;
@@ -165,11 +217,15 @@ nm_dhcp_dhclient_create_config (const char *interface,
 			if (!strlen (g_strstrip (p)))
 				continue;
 
-			/* Override config file "dhcp-client-id" and use one from the
-			 * connection.
-			 */
-			if (dhcp_client_id && !strncmp (p, CLIENTID_TAG, strlen (CLIENTID_TAG)))
-				continue;
+			if (!strncmp (p, CLIENTID_TAG, strlen (CLIENTID_TAG))) {
+				/* Override config file "dhcp-client-id" and use one from the connection */
+				if (client_id)
+					continue;
+
+				/* Otherwise capture and return the existing client id */
+				if (out_new_client_id)
+					*out_new_client_id = read_client_id (p);
+			}
 
 			/* Override config file hostname and use one from the connection */
 			if (hostname) {
@@ -238,7 +294,7 @@ nm_dhcp_dhclient_create_config (const char *interface,
 		add_also_request (alsoreq, "dhcp6.domain-search");
 		add_also_request (alsoreq, "dhcp6.client-id");
 	} else {
-		add_ip4_config (new_contents, dhcp_client_id, hostname);
+		add_ip4_config (new_contents, client_id, hostname);
 		add_also_request (alsoreq, "rfc3442-classless-static-routes");
 		add_also_request (alsoreq, "ms-classless-static-routes");
 		add_also_request (alsoreq, "static-routes");
diff --git a/src/dhcp-manager/nm-dhcp-dhclient-utils.h b/src/dhcp-manager/nm-dhcp-dhclient-utils.h
index 3b786a65a6..a1828add4a 100644
--- a/src/dhcp-manager/nm-dhcp-dhclient-utils.h
+++ b/src/dhcp-manager/nm-dhcp-dhclient-utils.h
@@ -27,11 +27,12 @@
 
 char *nm_dhcp_dhclient_create_config (const char *interface,
                                       gboolean is_ip6,
-                                      const char *dhcp_client_id,
+                                      GBytes *client_id,
                                       const char *anycast_addr,
                                       const char *hostname,
                                       const char *orig_path,
-                                      const char *orig_contents);
+                                      const char *orig_contents,
+                                      GBytes **out_new_client_id);
 
 char *nm_dhcp_dhclient_escape_duid (const GByteArray *duid);
 
@@ -48,5 +49,7 @@ GSList *nm_dhcp_dhclient_read_lease_ip_configs (const char *iface,
                                                 gboolean ipv6,
                                                 GDateTime *now);
 
+GBytes *nm_dhcp_dhclient_get_client_id_from_config_file (const char *path);
+
 #endif /* __NETWORKMANAGER_DHCP_DHCLIENT_UTILS_H__ */
 
diff --git a/src/dhcp-manager/nm-dhcp-dhclient.c b/src/dhcp-manager/nm-dhcp-dhclient.c
index 8f1718f8a6..76e8d68c2c 100644
--- a/src/dhcp-manager/nm-dhcp-dhclient.c
+++ b/src/dhcp-manager/nm-dhcp-dhclient.c
@@ -44,6 +44,7 @@
 #include "nm-posix-signals.h"
 #include "NetworkManagerUtils.h"
 #include "nm-dhcp-listener.h"
+#include "gsystem-local-alloc.h"
 
 G_DEFINE_TYPE (NMDhcpDhclient, nm_dhcp_dhclient, NM_TYPE_DHCP_CLIENT)
 
@@ -152,10 +153,11 @@ static gboolean
 merge_dhclient_config (const char *iface,
                        const char *conf_file,
                        gboolean is_ip6,
-                       const char *dhcp_client_id,
+                       GBytes *client_id,
                        const char *anycast_addr,
                        const char *hostname,
                        const char *orig_path,
+                       GBytes **out_new_client_id,
                        GError **error)
 {
 	char *orig = NULL, *new;
@@ -174,7 +176,7 @@ merge_dhclient_config (const char *iface,
 		}
 	}
 
-	new = nm_dhcp_dhclient_create_config (iface, is_ip6, dhcp_client_id, anycast_addr, hostname, orig_path, orig);
+	new = nm_dhcp_dhclient_create_config (iface, is_ip6, client_id, anycast_addr, hostname, orig_path, orig, out_new_client_id);
 	g_assert (new);
 	success = g_file_set_contents (conf_file, new, -1, error);
 	g_free (new);
@@ -258,9 +260,10 @@ static char *
 create_dhclient_config (const char *iface,
                         gboolean is_ip6,
                         const char *uuid,
-                        const char *dhcp_client_id,
+                        GBytes *client_id,
                         const char *dhcp_anycast_addr,
-                        const char *hostname)
+                        const char *hostname,
+                        GBytes **out_new_client_id)
 {
 	char *orig = NULL, *new = NULL;
 	GError *error = NULL;
@@ -285,7 +288,7 @@ create_dhclient_config (const char *iface,
 	}
 
 	error = NULL;
-	success = merge_dhclient_config (iface, new, is_ip6, dhcp_client_id, dhcp_anycast_addr, hostname, orig, &error);
+	success = merge_dhclient_config (iface, new, is_ip6, client_id, dhcp_anycast_addr, hostname, orig, out_new_client_id, &error);
 	if (!success) {
 		nm_log_warn (LOGD_DHCP, "(%s): error creating dhclient%s configuration: %s",
 		             iface, is_ip6 ? "6" : "", error->message);
@@ -475,23 +478,28 @@ dhclient_start (NMDhcpClient *client,
 
 static gboolean
 ip4_start (NMDhcpClient *client,
-           const char *dhcp_client_id,
            const char *dhcp_anycast_addr,
            const char *hostname)
 {
 	NMDhcpDhclientPrivate *priv = NM_DHCP_DHCLIENT_GET_PRIVATE (client);
+	GBytes *client_id;
+	gs_unref_bytes GBytes *new_client_id = NULL;
 	const char *iface, *uuid;
+	gboolean success = FALSE;
 
 	iface = nm_dhcp_client_get_iface (client);
 	uuid = nm_dhcp_client_get_uuid (client);
+	client_id = nm_dhcp_client_get_client_id (client);
 
-	priv->conf_file = create_dhclient_config (iface, FALSE, uuid, dhcp_client_id, dhcp_anycast_addr, hostname);
-	if (!priv->conf_file) {
+	priv->conf_file = create_dhclient_config (iface, FALSE, uuid, client_id, dhcp_anycast_addr, hostname, &new_client_id);
+	if (priv->conf_file) {
+		if (new_client_id)
+			nm_dhcp_client_set_client_id (client, new_client_id);
+		success = dhclient_start (client, NULL, NULL, FALSE, NULL);
+	} else
 		nm_log_warn (LOGD_DHCP4, "(%s): error creating dhclient configuration file.", iface);
-		return FALSE;
-	}
 
-	return dhclient_start (client, NULL, NULL, FALSE, NULL);
+	return success;
 }
 
 static gboolean
@@ -508,7 +516,7 @@ ip6_start (NMDhcpClient *client,
 	iface = nm_dhcp_client_get_iface (client);
 	uuid = nm_dhcp_client_get_uuid (client);
 
-	priv->conf_file = create_dhclient_config (iface, TRUE, uuid, NULL, dhcp_anycast_addr, hostname);
+	priv->conf_file = create_dhclient_config (iface, TRUE, uuid, NULL, dhcp_anycast_addr, hostname, NULL);
 	if (!priv->conf_file) {
 		nm_log_warn (LOGD_DHCP6, "(%s): error creating dhclient6 configuration file.", iface);
 		return FALSE;
@@ -545,6 +553,24 @@ stop (NMDhcpClient *client, gboolean release, const GByteArray *duid)
 	}
 }
 
+static void
+state_changed (NMDhcpClient *client,
+	           NMDhcpState state,
+	           GObject *ip_config,
+	           GHashTable *options)
+{
+	NMDhcpDhclientPrivate *priv = NM_DHCP_DHCLIENT_GET_PRIVATE (client);
+	gs_unref_bytes GBytes *client_id = NULL;
+
+	if (nm_dhcp_client_get_client_id (client))
+		return;
+	if (state != NM_DHCP_STATE_BOUND)
+		return;
+
+	client_id = nm_dhcp_dhclient_get_client_id_from_config_file (priv->conf_file);
+	nm_dhcp_client_set_client_id (client, client_id);
+}
+
 static GByteArray *
 get_duid (NMDhcpClient *client)
 {
@@ -651,6 +677,7 @@ nm_dhcp_dhclient_class_init (NMDhcpDhclientClass *dhclient_class)
 	client_class->ip6_start = ip6_start;
 	client_class->stop = stop;
 	client_class->get_duid = get_duid;
+	client_class->state_changed = state_changed;
 }
 
 static void __attribute__((constructor))
diff --git a/src/dhcp-manager/nm-dhcp-dhcpcd.c b/src/dhcp-manager/nm-dhcp-dhcpcd.c
index c4d91b1247..5a3aee3f4d 100644
--- a/src/dhcp-manager/nm-dhcp-dhcpcd.c
+++ b/src/dhcp-manager/nm-dhcp-dhcpcd.c
@@ -74,7 +74,6 @@ dhcpcd_child_setup (gpointer user_data G_GNUC_UNUSED)
 
 static gboolean
 ip4_start (NMDhcpClient *client,
-           const char *dhcp_client_id,
            const char *dhcp_anycast_addr,
            const char *hostname)
 {
diff --git a/src/dhcp-manager/nm-dhcp-systemd.c b/src/dhcp-manager/nm-dhcp-systemd.c
index 6b06c45288..35b08a1d3b 100644
--- a/src/dhcp-manager/nm-dhcp-systemd.c
+++ b/src/dhcp-manager/nm-dhcp-systemd.c
@@ -402,6 +402,28 @@ nm_dhcp_systemd_get_lease_ip_configs (const char *iface,
 /************************************************************/
 
 static void
+_save_client_id (NMDhcpSystemd *self,
+                 uint8_t type,
+                 const uint8_t *client_id,
+                 size_t len)
+{
+	gs_unref_bytes GBytes *b = NULL;
+	gs_free char *buf = NULL;
+
+	g_return_if_fail (self != NULL);
+	g_return_if_fail (client_id != NULL);
+	g_return_if_fail (len > 0);
+
+	if (!nm_dhcp_client_get_client_id (NM_DHCP_CLIENT (self))) {
+		buf = g_malloc (len + 1);
+		buf[0] = type;
+		memcpy (buf + 1, client_id, len);
+		b = g_bytes_new (buf, len + 1);
+		nm_dhcp_client_set_client_id (NM_DHCP_CLIENT (self), b);
+	}
+}
+
+static void
 bound4_handle (NMDhcpSystemd *self)
 {
 	NMDhcpSystemdPrivate *priv = NM_DHCP_SYSTEMD_GET_PRIVATE (self);
@@ -428,9 +450,17 @@ bound4_handle (NMDhcpSystemd *self)
 	                                  TRUE,
 	                                  &error);
 	if (ip4_config) {
+		const uint8_t *client_id = NULL;
+		size_t client_id_len = 0;
+		uint8_t type = 0;
+
 		add_requests_to_options (options, dhcp4_requests);
 		sd_dhcp_lease_save (lease, priv->lease_file);
 
+		client_id = sd_dhcp_client_get_client_id(priv->client4, &type, &client_id_len);
+		if (client_id)
+			_save_client_id (self, type, client_id, client_id_len);
+
 		nm_dhcp_client_set_state (NM_DHCP_CLIENT (self),
 		                          NM_DHCP_STATE_BOUND,
 		                          G_OBJECT (ip4_config),
@@ -486,7 +516,6 @@ get_arp_type (const GByteArray *hwaddr)
 
 static gboolean
 ip4_start (NMDhcpClient *client,
-           const char *dhcp_client_id,
            const char *dhcp_anycast_addr,
            const char *hostname)
 {
@@ -494,6 +523,7 @@ ip4_start (NMDhcpClient *client,
 	const char *iface = nm_dhcp_client_get_iface (client);
 	const GByteArray *hwaddr;
 	sd_dhcp_lease *lease = NULL;
+	GBytes *override_client_id;
 	const uint8_t *client_id = NULL;
 	size_t client_id_len = 0;
 	struct in_addr last_addr;
@@ -560,25 +590,25 @@ ip4_start (NMDhcpClient *client,
 		}
 	}
 
-	if (dhcp_client_id) {
-		gs_unref_bytes GBytes *b = NULL;
-
-		b = nm_dhcp_utils_client_id_string_to_bytes (dhcp_client_id);
-		if (b) {
-			client_id = (const guint8 *) g_bytes_get_data (b, &client_id_len);
-			g_assert (client_id && client_id_len);
-			sd_dhcp_client_set_client_id (priv->client4,
-			                              client_id[0],
-			                              client_id + 1,
-			                              client_id_len - 1);
-		}
-	} else {
+	override_client_id = nm_dhcp_client_get_client_id (client);
+	if (override_client_id) {
+		client_id = g_bytes_get_data (override_client_id, &client_id_len);
+		g_assert (client_id && client_id_len);
+		sd_dhcp_client_set_client_id (priv->client4,
+		                              client_id[0],
+		                              client_id + 1,
+		                              client_id_len - 1);
+	} else if (lease) {
 		r = sd_dhcp_lease_get_client_id (lease, &client_id, &client_id_len);
 		if (r == 0 && client_id_len) {
 			sd_dhcp_client_set_client_id (priv->client4,
 			                              client_id[0],
 			                              client_id + 1,
 			                              client_id_len - 1);
+			_save_client_id (NM_DHCP_SYSTEMD (client),
+			                 client_id[0],
+			                 client_id + 1,
+			                 client_id_len - 1);
 		}
 	}
 
diff --git a/src/dhcp-manager/tests/Makefile.am b/src/dhcp-manager/tests/Makefile.am
index 1eb02d6323..9dc5dcb13d 100644
--- a/src/dhcp-manager/tests/Makefile.am
+++ b/src/dhcp-manager/tests/Makefile.am
@@ -20,6 +20,8 @@ noinst_PROGRAMS = \
 test_dhcp_dhclient_SOURCES = \
 	$(top_srcdir)/src/dhcp-manager/nm-dhcp-dhclient-utils.h \
 	$(top_srcdir)/src/dhcp-manager/nm-dhcp-dhclient-utils.c \
+	$(top_srcdir)/src/dhcp-manager/nm-dhcp-utils.h \
+	$(top_srcdir)/src/dhcp-manager/nm-dhcp-utils.c \
 	test-dhcp-dhclient.c
 
 test_dhcp_dhclient_LDADD = \
diff --git a/src/dhcp-manager/tests/test-dhcp-dhclient.c b/src/dhcp-manager/tests/test-dhcp-dhclient.c
index 2ddf6b5078..544bd89efe 100644
--- a/src/dhcp-manager/tests/test-dhcp-dhclient.c
+++ b/src/dhcp-manager/tests/test-dhcp-dhclient.c
@@ -23,30 +23,42 @@
 #include <unistd.h>
 #include <arpa/inet.h>
 
+#include "gsystem-local-alloc.h"
+#include "NetworkManagerUtils.h"
 #include "nm-dhcp-dhclient-utils.h"
+#include "nm-dhcp-utils.h"
 #include "nm-utils.h"
 #include "nm-ip4-config.h"
 #include "nm-platform.h"
 
-#define DEBUG 0
+#define DEBUG 1
 
 static void
 test_config (const char *orig,
              const char *expected,
              const char *hostname,
              const char *dhcp_client_id,
+             GBytes *expected_new_client_id,
              const char *iface,
              const char *anycast_addr)
 {
-	char *new;
+	gs_free char *new = NULL;
+	gs_unref_bytes GBytes *client_id = NULL;
+	gs_unref_bytes GBytes *new_client_id = NULL;
+
+	if (dhcp_client_id) {
+		client_id = nm_dhcp_utils_client_id_string_to_bytes (dhcp_client_id);
+		g_assert (client_id);
+	}
 
 	new = nm_dhcp_dhclient_create_config (iface,
 	                                      FALSE,
-	                                      dhcp_client_id,
+	                                      client_id,
 	                                      anycast_addr,
 	                                      hostname,
 	                                      "/path/to/dhclient.conf",
-	                                      orig);
+	                                      orig,
+	                                      &new_client_id);
 	g_assert (new != NULL);
 
 #if DEBUG
@@ -60,9 +72,13 @@ test_config (const char *orig,
 		           new, expected);
 	}
 #endif
-	g_assert (strlen (new) == strlen (expected));
-	g_assert (strcmp (new, expected) == 0);
-	g_free (new);
+	g_assert_cmpstr (new, ==, expected);
+
+	if (expected_new_client_id) {
+		g_assert (new_client_id);
+		g_assert (g_bytes_equal (new_client_id, expected_new_client_id));
+	 } else
+		g_assert (new_client_id == NULL);
 }
 
 /*******************************************/
@@ -84,11 +100,7 @@ static const char *orig_missing_expected = \
 static void
 test_orig_missing (void)
 {
-	test_config (NULL, orig_missing_expected,
-	             NULL,
-	             NULL,
-	             "eth0",
-	             NULL);
+	test_config (NULL, orig_missing_expected, NULL, NULL, NULL, "eth0", NULL);
 }
 
 /*******************************************/
@@ -119,6 +131,7 @@ test_override_client_id (void)
 	test_config (override_client_id_orig, override_client_id_expected,
 	             NULL,
 	             "11:22:33:44:55:66",
+	             NULL,
 	             "eth0",
 	             NULL);
 }
@@ -147,6 +160,7 @@ test_quote_client_id (void)
 	test_config (NULL, quote_client_id_expected,
 	             NULL,
 	             "1234",
+	             NULL,
 	             "eth0",
 	             NULL);
 }
@@ -175,6 +189,7 @@ test_ascii_client_id (void)
 	test_config (NULL, ascii_client_id_expected,
 	             NULL,
 	             "qb:cd:ef:12:34:56",
+	             NULL,
 	             "eth0",
 	             NULL);
 }
@@ -184,7 +199,7 @@ test_ascii_client_id (void)
 static const char *hex_single_client_id_expected = \
 	"# Created by NetworkManager\n"
 	"\n"
-	"send dhcp-client-identifier ab:cd:e:12:34:56; # added by NetworkManager\n"
+	"send dhcp-client-identifier ab:cd:0e:12:34:56; # added by NetworkManager\n"
 	"\n"
 	"option rfc3442-classless-static-routes code 121 = array of unsigned integer 8;\n"
 	"option ms-classless-static-routes code 249 = array of unsigned integer 8;\n"
@@ -203,6 +218,84 @@ test_hex_single_client_id (void)
 	test_config (NULL, hex_single_client_id_expected,
 	             NULL,
 	             "ab:cd:e:12:34:56",
+	             NULL,
+	             "eth0",
+	             NULL);
+}
+
+/*******************************************/
+
+static const char *existing_hex_client_id_orig = \
+	"send dhcp-client-identifier 00:30:04:20:7A:08;\n";
+
+static const char *existing_hex_client_id_expected = \
+	"# Created by NetworkManager\n"
+	"# Merged from /path/to/dhclient.conf\n"
+	"\n"
+	"send dhcp-client-identifier 00:30:04:20:7A:08;\n"
+	"\n"
+	"option rfc3442-classless-static-routes code 121 = array of unsigned integer 8;\n"
+	"option ms-classless-static-routes code 249 = array of unsigned integer 8;\n"
+	"option wpad code 252 = string;\n"
+	"\n"
+	"also request rfc3442-classless-static-routes;\n"
+	"also request ms-classless-static-routes;\n"
+	"also request static-routes;\n"
+	"also request wpad;\n"
+	"also request ntp-servers;\n"
+	"\n";
+
+static void
+test_existing_hex_client_id (void)
+{
+	gs_unref_bytes GBytes *new_client_id = NULL;
+	const guint8 bytes[] = { 0x00, 0x30, 0x04,0x20, 0x7A, 0x08 };
+
+	new_client_id = g_bytes_new (bytes, sizeof (bytes));
+	test_config (existing_hex_client_id_orig, existing_hex_client_id_expected,
+	             NULL,
+	             NULL,
+	             new_client_id,
+	             "eth0",
+	             NULL);
+}
+
+/*******************************************/
+
+#define EACID "qb:cd:ef:12:34:56"
+
+static const char *existing_ascii_client_id_orig = \
+	"send dhcp-client-identifier \"" EACID "\";\n";
+
+static const char *existing_ascii_client_id_expected = \
+	"# Created by NetworkManager\n"
+	"# Merged from /path/to/dhclient.conf\n"
+	"\n"
+	"send dhcp-client-identifier \"" EACID "\";\n"
+	"\n"
+	"option rfc3442-classless-static-routes code 121 = array of unsigned integer 8;\n"
+	"option ms-classless-static-routes code 249 = array of unsigned integer 8;\n"
+	"option wpad code 252 = string;\n"
+	"\n"
+	"also request rfc3442-classless-static-routes;\n"
+	"also request ms-classless-static-routes;\n"
+	"also request static-routes;\n"
+	"also request wpad;\n"
+	"also request ntp-servers;\n"
+	"\n";
+
+static void
+test_existing_ascii_client_id (void)
+{
+	gs_unref_bytes GBytes *new_client_id = NULL;
+	char buf[STRLEN (EACID) + 1] = { 0 };
+
+	memcpy (buf + 1, EACID, STRLEN (EACID));
+	new_client_id = g_bytes_new (buf, sizeof (buf));
+	test_config (existing_ascii_client_id_orig, existing_ascii_client_id_expected,
+	             NULL,
+	             NULL,
+	             new_client_id,
 	             "eth0",
 	             NULL);
 }
@@ -235,6 +328,7 @@ test_override_hostname (void)
 	test_config (override_hostname_orig, override_hostname_expected,
 	             "blahblah",
 	             NULL,
+	             NULL,
 	             "eth0",
 	             NULL);
 }
@@ -269,6 +363,7 @@ test_existing_alsoreq (void)
 	test_config (existing_alsoreq_orig, existing_alsoreq_expected,
 	             NULL,
 	             NULL,
+	             NULL,
 	             "eth0",
 	             NULL);
 }
@@ -307,6 +402,7 @@ test_existing_multiline_alsoreq (void)
 	test_config (existing_multiline_alsoreq_orig, existing_multiline_alsoreq_expected,
 	             NULL,
 	             NULL,
+	             NULL,
 	             "eth0",
 	             NULL);
 }
@@ -616,6 +712,8 @@ main (int argc, char **argv)
 	g_test_add_func ("/dhcp/dhclient/quote_client_id", test_quote_client_id);
 	g_test_add_func ("/dhcp/dhclient/ascii_client_id", test_ascii_client_id);
 	g_test_add_func ("/dhcp/dhclient/hex_single_client_id", test_hex_single_client_id);
+	g_test_add_func ("/dhcp/dhclient/existing-hex-client-id", test_existing_hex_client_id);
+	g_test_add_func ("/dhcp/dhclient/existing-ascii-client-id", test_existing_ascii_client_id);
 	g_test_add_func ("/dhcp/dhclient/override_hostname", test_override_hostname);
 	g_test_add_func ("/dhcp/dhclient/existing_alsoreq", test_existing_alsoreq);
 	g_test_add_func ("/dhcp/dhclient/existing_multiline_alsoreq", test_existing_multiline_alsoreq);