libnm,core: enhance nm_utils_hexstr2bin()

Make the type return GBytes since most in-tree users want that.

Allow the function to accept many more formats as valid hex, including
bytes delimited by ':' and a leading '0x'.

clients/cli/settings.c

@@ -2343,7 +2343,7 @@ nmc_property_set_byte_array (NMSetting *setting, const char *prop, const char *v
 	char *val_strip;
 	const char *delimiters = " \t,";
 	long int val_int;
-	char *bin;
+	GBytes *bytes;
 	GByteArray *array = NULL;
 	gboolean success = TRUE;
 
@@ -2352,11 +2352,9 @@ nmc_property_set_byte_array (NMSetting *setting, const char *prop, const char *v
 	val_strip = g_strstrip (g_strdup (val));
 
 	/* First try hex string in the format of AAbbCCDd */
-	bin = nm_utils_hexstr2bin (val_strip, strlen (val_strip));
-	if (bin) {
-		array = g_byte_array_sized_new (strlen (val_strip)/2);
-		g_byte_array_append (array, (const guint8 *) bin, strlen (val_strip)/2);
-		g_free (bin);
+	bytes = nm_utils_hexstr2bin (val_strip);
+	if (bytes) {
+		array = g_bytes_unref_to_array (bytes);
 		goto done;
 	}
 

libnm-core/nm-utils.c

@@ -33,6 +33,7 @@
 #include "nm-glib-compat.h"
 #include "nm-setting-private.h"
 #include "crypto.h"
+#include "gsystem-local-alloc.h"
 
 #include "nm-setting-bond.h"
 #include "nm-setting-bridge.h"
@@ -2100,7 +2101,7 @@ nm_utils_rsa_key_encrypt_helper (const char *cipher,
 	if (!in_password) {
 		if (!crypto_randomize (pw_buf, sizeof (pw_buf), error))
 			return NULL;
-		in_password = tmp_password = nm_utils_bin2hexstr ((const char *) pw_buf, sizeof (pw_buf), -1);
+		in_password = tmp_password = nm_utils_bin2hexstr (pw_buf, sizeof (pw_buf), -1);
 	}
 
 	if (g_strcmp0 (cipher, CIPHER_AES_CBC) == 0)
@@ -2124,7 +2125,7 @@ nm_utils_rsa_key_encrypt_helper (const char *cipher,
 	g_string_append (pem, "Proc-Type: 4,ENCRYPTED\n");
 
 	/* Convert the salt to a hex string */
-	tmp = nm_utils_bin2hexstr ((const char *) salt, salt_len, salt_len * 2);
+	tmp = nm_utils_bin2hexstr (salt, salt_len, salt_len * 2);
 	g_string_append_printf (pem, "DEK-Info: %s,%s\n\n", cipher, tmp);
 	g_free (tmp);
 
@@ -2876,13 +2877,13 @@ _nm_utils_hwaddr_from_dbus (GVariant *dbus_value,
 
 /**
  * nm_utils_bin2hexstr:
- * @bytes: an array of bytes
+ * @src: an array of bytes
  * @len: the length of the @bytes array
  * @final_len: an index where to cut off the returned string, or -1
  *
- * Converts a byte-array @bytes into a hexadecimal string.
- * If @final_len is greater than -1, the returned string is terminated at
- * that index (returned_string[final_len] == '\0'),
+ * Converts the byte array @src into a hexadecimal string. If @final_len is
+ * greater than -1, the returned string is terminated at that index
+ * (returned_string[final_len] == '\0'),
  *
  * Return value: (transfer full): the textual form of @bytes
  */
@@ -2891,9 +2892,10 @@ _nm_utils_hwaddr_from_dbus (GVariant *dbus_value,
  *  copyright Red Hat, Inc. under terms of the LGPL.
  */
 char *
-nm_utils_bin2hexstr (const char *bytes, int len, int final_len)
+nm_utils_bin2hexstr (gconstpointer src, gsize len, int final_len)
 {
 	static char hex_digits[] = "0123456789abcdef";
+	const guint8 *bytes = src;
 	char *result;
 	int i;
 	gsize buflen = (len * 2) + 1;
@@ -2918,64 +2920,61 @@ nm_utils_bin2hexstr (const char *bytes, int len, int final_len)
 	return result;
 }
 
-/* From hostap, Copyright (c) 2002-2005, Jouni Malinen <jkmaline@cc.hut.fi> */
 /**
- * nm_utils_hex2byte:
- * @hex: a string representing a hex byte
+ * nm_utils_hexstr2bin:
+ * @hex: a string of hexadecimal characters with optional ':' separators
  *
- * Converts a hex string (2 characters) into its byte representation.
+ * Converts a hexadecimal string @hex into an array of bytes.  The optional
+ * separator ':' may be used between single or pairs of hexadecimal characters,
+ * eg "00:11" or "0:1".  Any "0x" at the beginning of @hex is ignored.  @hex
+ * may not start or end with ':'.
  *
- * Return value: a byte, or -1 if @hex doesn't represent a hex byte
+ * Return value: (transfer full): the converted bytes, or %NULL on error
  */
-int
-nm_utils_hex2byte (const char *hex)
+GBytes *
+nm_utils_hexstr2bin (const char *hex)
 {
+	guint i = 0, x = 0;
+	gs_free guint8 *c = NULL;
 	int a, b;
-	a = g_ascii_xdigit_value (*hex++);
-	if (a < 0)
-		return -1;
-	b = g_ascii_xdigit_value (*hex++);
-	if (b < 0)
-		return -1;
-	return (a << 4) | b;
-}
+	gboolean found_colon = FALSE;
 
-/**
- * nm_utils_hexstr2bin:
- * @hex: an hex string
- * @len: the length of the @hex string (it has to be even)
- *
- * Converts a hexadecimal string @hex into a byte-array. The returned array
- * length is @len/2.
- *
- * Return value: (transfer full): a array of bytes, or %NULL on error
- */
-char *
-nm_utils_hexstr2bin (const char *hex, size_t len)
-{
-	size_t       i;
-	int          a;
-	const char * ipos = hex;
-	char *       buf = NULL;
-	char *       opos;
+	g_return_val_if_fail (hex != NULL, NULL);
 
-	/* Length must be a multiple of 2 */
-	if ((len % 2) != 0)
-		return NULL;
+	if (strncasecmp (hex, "0x", 2) == 0)
+		hex += 2;
+	found_colon = !!strchr (hex, ':');
 
-	opos = buf = g_malloc0 ((len / 2) + 1);
-	for (i = 0; i < len; i += 2) {
-		a = nm_utils_hex2byte (ipos);
-		if (a < 0) {
-			g_free (buf);
+	c = g_malloc (strlen (hex) / 2 + 1);
+	for (;;) {
+		a = g_ascii_xdigit_value (hex[i++]);
+		if (a < 0)
+			return NULL;
+
+		if (hex[i] && hex[i] != ':') {
+			b = g_ascii_xdigit_value (hex[i++]);
+			if (b < 0)
+				return NULL;
+			c[x++] = ((guint) a << 4) | ((guint) b);
+		} else
+			c[x++] = (guint) a;
+
+		if (!hex[i])
+			break;
+		if (hex[i] == ':') {
+			if (!hex[i + 1]) {
+				/* trailing ':' is invalid */
+				return NULL;
+			}
+			i++;
+		} else if (found_colon) {
+			/* If colons exist, they must delimit 1 or 2 hex chars */
 			return NULL;
 		}
-		*opos++ = a;
-		ipos += 2;
 	}
-	return buf;
+
+	return g_bytes_new (c, x);
 }
-/* End from hostap */
 
 /**
  * nm_utils_iface_valid_name:

libnm-core/nm-utils.h

@@ -167,9 +167,8 @@ gboolean    nm_utils_hwaddr_matches   (gconstpointer hwaddr1,
                                        gconstpointer hwaddr2,
                                        gssize        hwaddr2_len);
 
-char *nm_utils_bin2hexstr (const char *bytes, int len, int final_len);
-int   nm_utils_hex2byte   (const char *hex);
-char *nm_utils_hexstr2bin (const char *hex, size_t len);
+char *nm_utils_bin2hexstr (gconstpointer src, gsize len, int final_len);
+GBytes *nm_utils_hexstr2bin (const char *hex);
 
 gboolean    nm_utils_iface_valid_name(const char *name);
 

libnm-core/tests/test-general.c

@@ -3550,6 +3550,41 @@ test_setting_ip6_gateway (void)
 	g_object_unref (conn);
 }
 
+typedef struct {
+	const char *str;
+	const guint8 expected[20];
+	const guint expected_len;
+} HexItem;
+
+static void
+test_hexstr2bin (void)
+{
+	static const HexItem items[] = {
+		{ "aaBBCCddDD10496a",     { 0xaa, 0xbb, 0xcc, 0xdd, 0xdd, 0x10, 0x49, 0x6a }, 8 },
+		{ "aa:bb:cc:dd:10:49:6a", { 0xaa, 0xbb, 0xcc, 0xdd, 0x10, 0x49, 0x6a },       7 },
+		{ "0xccddeeff",           { 0xcc, 0xdd, 0xee, 0xff },                         4 },
+		{ "1:2:66:77:80",         { 0x01, 0x02, 0x66, 0x77, 0x80 },                   5 },
+		{ "e",                    { 0x0e },                                           1 },
+		{ "aabb1199:" },
+		{ ":aabb1199" },
+		{ "aabb$$dd" },
+		{ "aab:ccc:ddd" },
+		{ "aab::ccc:ddd" },
+	};
+	GBytes *b;
+	guint i;
+
+	for (i = 0; i < G_N_ELEMENTS (items); i++) {
+		b = nm_utils_hexstr2bin (items[i].str);
+		if (items[i].expected_len) {
+			g_assert (b);
+			g_assert_cmpint (g_bytes_get_size (b), ==, items[i].expected_len);
+			g_assert (memcmp (g_bytes_get_data (b, NULL), items[i].expected, g_bytes_get_size (b)) == 0);
+		} else
+			g_assert (b == NULL);
+	}
+}
+
 NMTST_DEFINE ();
 
 int main (int argc, char **argv)
@@ -3641,6 +3676,8 @@ int main (int argc, char **argv)
 	g_test_add_func ("/core/general/test_setting_ip4_gateway", test_setting_ip4_gateway);
 	g_test_add_func ("/core/general/test_setting_ip6_gateway", test_setting_ip6_gateway);
 
+	g_test_add_func ("/core/general/hexstr2bin", test_hexstr2bin);
+
 	return g_test_run ();
 }
 

libnm/libnm.ver

@@ -756,7 +756,6 @@ global:
 	nm_utils_deinit;
 	nm_utils_escape_ssid;
 	nm_utils_file_is_pkcs12;
-	nm_utils_hex2byte;
 	nm_utils_hexstr2bin;
 	nm_utils_hwaddr_atoba;
 	nm_utils_hwaddr_aton;

libnm/nm-device.c

@@ -1249,6 +1249,19 @@ nm_device_get_available_connections (NMDevice *device)
 	return NM_DEVICE_GET_PRIVATE (device)->available_connections;
 }
 
+static inline guint8
+hex2byte (const char *hex)
+{
+	int a, b;
+	a = g_ascii_xdigit_value (*hex++);
+	if (a < 0)
+		return -1;
+	b = g_ascii_xdigit_value (*hex++);
+	if (b < 0)
+		return -1;
+	return (a << 4) | b;
+}
+
 static char *
 get_decoded_property (GUdevDevice *device, const char *property)
 {
@@ -1264,7 +1277,7 @@ get_decoded_property (GUdevDevice *device, const char *property)
 	n = unescaped = g_malloc0 (len + 1);
 	while (*p) {
 		if ((len >= 4) && (*p == '\\') && (*(p+1) == 'x')) {
-			*n++ = (char) nm_utils_hex2byte (p + 2);
+			*n++ = (char) hex2byte (p + 2);
 			p += 4;
 			len -= 4;
 		} else {

src/dhcp-manager/nm-dhcp-utils.c

@@ -709,46 +709,17 @@ GBytes *
 nm_dhcp_utils_client_id_string_to_bytes (const char *client_id)
 {
 	GBytes *bytes = NULL;
-	guint i = 0, x = 0;
 	guint len;
 	char *c;
-	int a;
 
 	g_return_val_if_fail (client_id && client_id[0], NULL);
 
-	/* Accept a binary client ID in hex digits with the ':' delimiter,
-	 * otherwise treat it as a string.
-	 */
-	len = strlen (client_id);
-	c = g_malloc0 (len / 2 + 1);
-	while (client_id[i]) {
-		a = g_ascii_xdigit_value (client_id[i++]);
-		if (a >= 0) {
-			if (client_id[i] != ':') {
-				c[x] = ((guint8) a << 4);
-				a = g_ascii_xdigit_value (client_id[i++]);
-			}
-			if (a >= 0)
-				c[x++] |= (guint8) a;
-		}
-		if (client_id[i]) {
-			if (client_id[i] != ':' || !client_id[i + 1]) {
-				/* missing or trailing ':' is invalid for hex-format */
-				a = -1;
-			}
-			i++;
-		}
-
-		if (a < 0) {
-			g_clear_pointer (&c, g_free);
-			break;
-		}
-	}
-
-	if (c) {
-		g_assert (x > 0);
-		bytes = g_bytes_new_take (c, x);
-	} else {
+	/* Try as hex encoded */
+	if (strchr (client_id, ':'))
+		bytes = nm_utils_hexstr2bin (client_id);
+	if (!bytes) {
+		/* Fall back to string */
+		len = strlen (client_id);
 		c = g_malloc (len + 1);
 		c[0] = 0;  /* type: non-hardware address per RFC 2132 section 9.14 */
 		memcpy (c + 1, client_id, len);

src/settings/plugins/ifcfg-rh/reader.c

@@ -3224,7 +3224,6 @@ make_wireless_setting (shvarFile *ifcfg,
                        GError **error)
 {
 	NMSettingWireless *s_wireless;
-	GBytes *bytes = NULL;
 	char *value = NULL;
 	gint64 chan = 0;
 
@@ -3256,19 +3255,19 @@ make_wireless_setting (shvarFile *ifcfg,
 
 	value = svGetValue (ifcfg, "ESSID", TRUE);
 	if (value) {
-		gsize ssid_len = 0, value_len = strlen (value);
-		char *p = value, *tmp;
-		char buf[33];
+		GBytes *bytes = NULL;
+		gsize ssid_len = 0;
+		gsize value_len = strlen (value);
 
-		ssid_len = value_len;
 		if (   (value_len >= 2)
 		    && (value[0] == '"')
 		    && (value[value_len - 1] == '"')) {
 			/* Strip the quotes and unescape */
-			p = value + 1;
+			char *p = value + 1;
+
 			value[value_len - 1] = '\0';
 			svUnescape (p);
-			ssid_len = strlen (p);
+			bytes = g_bytes_new (p, strlen (p));
 		} else if ((value_len > 2) && (strncmp (value, "0x", 2) == 0)) {
 			/* Hex representation */
 			if (value_len % 2) {
@@ -3279,34 +3278,27 @@ make_wireless_setting (shvarFile *ifcfg,
 				goto error;
 			}
 
-			p = value + 2;
-			while (*p) {
-				if (!g_ascii_isxdigit (*p)) {
-					g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION,
-					             "Invalid SSID '%s' character (looks like hex SSID but '%c' isn't a hex digit)",
-					             value, *p);
-					g_free (value);
-					goto error;
-				}
-				p++;
+			bytes = nm_utils_hexstr2bin (value);
+			if (!bytes) {
+				g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION,
+				             "Invalid SSID '%s' (looks like hex SSID but isn't)",
+				             value);
+				g_free (value);
+				goto error;
 			}
+		} else
+			bytes = g_bytes_new (value, value_len);
 
-			tmp = nm_utils_hexstr2bin (value + 2, value_len - 2);
-			ssid_len  = (value_len - 2) / 2;
-			memcpy (buf, tmp, ssid_len);
-			p = &buf[0];
-			g_free (tmp);
-		}
-
+		ssid_len = g_bytes_get_size (bytes);
 		if (ssid_len > 32 || ssid_len == 0) {
 			g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION,
 			             "Invalid SSID '%s' (size %zu not between 1 and 32 inclusive)",
 			             value, ssid_len);
+			g_bytes_unref (bytes);
 			g_free (value);
 			goto error;
 		}
 
-		bytes = g_bytes_new (p, ssid_len);
 		g_object_set (s_wireless, NM_SETTING_WIRELESS_SSID, bytes, NULL);
 		g_bytes_unref (bytes);
 		g_free (value);

src/settings/plugins/ifnet/connection_parser.c

@@ -43,8 +43,16 @@ connection_id_from_ifnet_name (const char *conn_name)
 	int name_len = strlen (conn_name);
 
 	/* Convert a hex-encoded conn_name (only used for wifi SSIDs) to human-readable one */
-	if ((name_len > 2) && (g_str_has_prefix (conn_name, "0x")))
-		return nm_utils_hexstr2bin (conn_name + 2, name_len - 2);
+	if ((name_len > 2) && (g_str_has_prefix (conn_name, "0x"))) {
+		GBytes *bytes = nm_utils_hexstr2bin (conn_name);
+		char *buf;
+
+		if (bytes) {
+			buf = g_strndup (g_bytes_get_data (bytes, NULL), g_bytes_get_size (bytes));
+			g_bytes_unref (bytes);
+			return buf;
+		}
+	}
 
 	return g_strdup (conn_name);
 }
@@ -882,7 +890,6 @@ make_wireless_connection_setting (const char *conn_name,
                                   NMSetting8021x **s_8021x,
                                   GError **error)
 {
-	GBytes *bytes;
 	const char *mac = NULL;
 	NMSettingWireless *wireless_setting = NULL;
 	gboolean adhoc = FALSE;
@@ -913,9 +920,8 @@ make_wireless_connection_setting (const char *conn_name,
 
 	/* handle ssid (hex and ascii) */
 	if (conn_name) {
+		GBytes *bytes;
 		gsize ssid_len = 0, value_len = strlen (conn_name);
-		const char *p;
-		char *tmp, *converted = NULL;
 
 		ssid_len = value_len;
 		if ((value_len > 2) && (g_str_has_prefix (conn_name, "0x"))) {
@@ -926,32 +932,27 @@ make_wireless_connection_setting (const char *conn_name,
 					     conn_name);
 				goto error;
 			}
-			// ignore "0x"
-			p = conn_name + 2;
-			if (!is_hex (p)) {
+
+			bytes = nm_utils_hexstr2bin (conn_name);
+			if (!bytes) {
 				g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION,
-					     "Invalid SSID '%s' character (looks like hex SSID but '%c' isn't a hex digit)",
-					     conn_name, *p);
+				             "Invalid SSID '%s' (looks like hex SSID but isn't)",
+				             conn_name);
 				goto error;
-
 			}
-			tmp = nm_utils_hexstr2bin (p, value_len - 2);
-			ssid_len = (value_len - 2) / 2;
-			converted = g_malloc0 (ssid_len + 1);
-			memcpy (converted, tmp, ssid_len);
-			g_free (tmp);
-		}
+		} else
+			bytes = g_bytes_new (conn_name, value_len);
 
+		ssid_len = g_bytes_get_size (bytes);
 		if (ssid_len > 32 || ssid_len == 0) {
 			g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION,
 				     "Invalid SSID '%s' (size %zu not between 1 and 32 inclusive)",
 				     conn_name, ssid_len);
 			goto error;
 		}
-		bytes = g_bytes_new (converted ? converted : conn_name, ssid_len);
+
 		g_object_set (wireless_setting, NM_SETTING_WIRELESS_SSID, bytes, NULL);
 		g_bytes_unref (bytes);
-		g_free (converted);
 	} else {
 		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION,
 			     "Missing SSID");
@@ -1095,7 +1096,7 @@ add_one_wep_key (const char *ssid,
 
 		}
 
-		converted = nm_utils_bin2hexstr (tmp, strlen (tmp), strlen (tmp) * 2);
+		converted = nm_utils_bin2hexstr (tmp, strlen (tmp), -1);
 		g_free (tmp);
 	} else {
 		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION,

src/supplicant-manager/nm-supplicant-config.c

@@ -542,8 +542,8 @@ add_wep_key (NMSupplicantConfig *self,
              const char *name,
              NMWepKeyType wep_type)
 {
-	char *value;
-	gboolean success;
+	GBytes *bytes;
+	gboolean success = FALSE;
 	size_t key_len = key ? strlen (key) : 0;
 
 	if (!key || !key_len)
@@ -552,9 +552,15 @@ add_wep_key (NMSupplicantConfig *self,
 	if (   (wep_type == NM_WEP_KEY_TYPE_UNKNOWN)
 	    || (wep_type == NM_WEP_KEY_TYPE_KEY)) {
 		if ((key_len == 10) || (key_len == 26)) {
-			value = nm_utils_hexstr2bin (key, strlen (key));
-			success = nm_supplicant_config_add_option (self, name, value, key_len / 2, TRUE);
-			g_free (value);
+			bytes = nm_utils_hexstr2bin (key);
+			if (bytes) {
+				success = nm_supplicant_config_add_option (self,
+				                                           name,
+				                                           g_bytes_get_data (bytes, NULL),
+				                                           g_bytes_get_size (bytes),
+				                                           TRUE);
+				g_bytes_unref (bytes);
+			}
 			if (!success) {
 				nm_log_warn (LOGD_SUPPLICANT, "Error adding %s to supplicant config.", name);
 				return FALSE;
@@ -590,8 +596,7 @@ nm_supplicant_config_add_setting_wireless_security (NMSupplicantConfig *self,
                                                     NMSetting8021x *setting_8021x,
                                                     const char *con_uuid)
 {
-	char *value;
-	gboolean success;
+	gboolean success = FALSE;
 	const char *key_mgmt, *auth_alg;
 	const char *psk;
 
@@ -612,10 +617,18 @@ nm_supplicant_config_add_setting_wireless_security (NMSupplicantConfig *self,
 		size_t psk_len = strlen (psk);
 
 		if (psk_len == 64) {
+			GBytes *bytes;
+
 			/* Hex PSK */
-			value = nm_utils_hexstr2bin (psk, psk_len);
-			success = nm_supplicant_config_add_option (self, "psk", value, psk_len / 2, TRUE);
-			g_free (value);
+			bytes = nm_utils_hexstr2bin (psk);
+			if (bytes) {
+				success = nm_supplicant_config_add_option (self,
+				                                           "psk",
+				                                           g_bytes_get_data (bytes, NULL),
+				                                           g_bytes_get_size (bytes),
+				                                           TRUE);
+				g_bytes_unref (bytes);
+			}
 			if (!success) {
 				nm_log_warn (LOGD_SUPPLICANT, "Error adding 'psk' to supplicant config.");
 				return FALSE;
@@ -653,6 +666,7 @@ nm_supplicant_config_add_setting_wireless_security (NMSupplicantConfig *self,
 		const char *wep1 = nm_setting_wireless_security_get_wep_key (setting, 1);
 		const char *wep2 = nm_setting_wireless_security_get_wep_key (setting, 2);
 		const char *wep3 = nm_setting_wireless_security_get_wep_key (setting, 3);
+		char *value;
 
 		if (!add_wep_key (self, wep0, "wep_key0", wep_type))
 			return FALSE;