libnm/keyfile: merge branch 'th/keyfile-cert-parser' (#71)

https://github.com/NetworkManager/NetworkManager/pull/71

1 files changed, 139 insertions, 163 deletions

diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c
index f850638497..1e75cbf922 100644
--- a/libnm-core/nm-keyfile-reader.c
+++ b/libnm-core/nm-keyfile-reader.c
@@ -990,116 +990,6 @@ has_cert_ext (const char *path)
 	return FALSE;
 }
 
-static gboolean
-handle_as_scheme (KeyfileReaderInfo *info, GBytes *bytes, NMSetting *setting, const char *key)
-{
-	const char *data;
-	gsize data_len, bin_len;
-
-	data = g_bytes_get_data (bytes, &data_len);
-
-	g_return_val_if_fail (data && data_len > 0, FALSE);
-
-	/* to be a scheme, @data must be a zero terminated string, which is counted by @data_len */
-	if (data[data_len - 1] != '\0')
-		return FALSE;
-	data_len--;
-
-	/* It's the PATH scheme, can just set plain data.
-	 * In this case, @data_len includes */
-	if (   data_len >= NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH)
-	    && g_str_has_prefix (data, NM_KEYFILE_CERT_SCHEME_PREFIX_PATH)) {
-		if (nm_setting_802_1x_check_cert_scheme (data, data_len + 1, NULL) == NM_SETTING_802_1X_CK_SCHEME_PATH) {
-			const char *path = &data[NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH)];
-			gs_free char *path_free = NULL;
-
-			if (path[0] != '/') {
-				/* we want to read absolute paths because we use keyfile as exchange
-				 * between different processes which might not have the same cwd. */
-				path = path_free = get_cert_path (info->base_dir, (const guint8 *) path,
-				                                  data_len - NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH));
-			}
-
-			g_object_set (setting, key, bytes, NULL);
-			if (!g_file_test (path, G_FILE_TEST_EXISTS)) {
-				handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_INFO_MISSING_FILE,
-				             _("certificate or key file '%s' does not exist"),
-				             path);
-			}
-		} else {
-			handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN,
-			             _("invalid key/cert value path \"%s\""), data);
-		}
-		return TRUE;
-	}
-	if (   data_len >= NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_PKCS11)
-	    && g_str_has_prefix (data, NM_KEYFILE_CERT_SCHEME_PREFIX_PKCS11)) {
-		if (nm_setting_802_1x_check_cert_scheme (data, data_len + 1, NULL) == NM_SETTING_802_1X_CK_SCHEME_PKCS11) {
-			g_object_set (setting, key, bytes, NULL);
-		} else {
-			handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN,
-			             _("invalid PKCS#11 URI \"%s\""), data);
-		}
-		return TRUE;
-	}
-	if (   data_len > NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB)
-	    && g_str_has_prefix (data, NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB)) {
-		const char *cdata = data + NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB);
-		guchar *bin;
-		GBytes *bytes2;
-		gsize i;
-		gboolean valid_base64;
-
-		data_len -= NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB);
-
-		/* Let's be strict here. We expect valid base64, no funny stuff!!
-		 * We didn't write such invalid data ourselfes and refuse to read it as blob. */
-		if ((valid_base64 = (data_len % 4 == 0))) {
-			for (i = 0; i < data_len; i++) {
-				char c = cdata[i];
-
-				if (!(   (c >= 'a' && c <= 'z')
-				      || (c >= 'A' && c <= 'Z')
-				      || (c >= '0' && c <= '9')
-				      || (c == '+' || c == '/'))) {
-					if (c != '=' || i < data_len - 2)
-						valid_base64 = FALSE;
-					else {
-						for (; i < data_len; i++) {
-							if (cdata[i] != '=')
-								valid_base64 = FALSE;
-						}
-					}
-					break;
-				}
-			}
-		}
-		if (!valid_base64) {
-			handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN,
-			             _("invalid key/cert value data:;base64, is not base64"));
-			return TRUE;
-		}
-
-		bin = g_base64_decode (cdata, &bin_len);
-
-		g_return_val_if_fail (bin_len > 0, FALSE);
-		if (nm_setting_802_1x_check_cert_scheme (bin, bin_len, NULL) != NM_SETTING_802_1X_CK_SCHEME_BLOB) {
-			/* The blob probably starts with "file://". Setting the cert data will confuse NMSetting8021x.
-			 * In fact this is a limitation of NMSetting8021x which does not support setting blobs that start
-			 * with file://. Just warn and return TRUE to signal that we ~handled~ the setting. */
-			g_free (bin);
-			handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN,
-			             _("invalid key/cert value data:;base64,file://"));
-		} else {
-			bytes2 = g_bytes_new_take (bin, bin_len);
-			g_object_set (setting, key, bytes2, NULL);
-			g_bytes_unref (bytes2);
-		}
-		return TRUE;
-	}
-	return FALSE;
-}
-
 char *
 nm_keyfile_detect_unqualified_path_scheme (const char *base_dir,
                                            gconstpointer pdata,
@@ -1174,75 +1064,161 @@ out:
 	return path;
 }
 
-static gboolean
-handle_as_path (KeyfileReaderInfo *info,
-                GBytes *bytes,
-                NMSetting *setting,
-                const char *key)
-{
-	const guint8 *data;
-	gsize data_len;
-	char *path;
-	gboolean exists = FALSE;
-	GBytes *val;
-
-	data = g_bytes_get_data (bytes, &data_len);
-
-	path = nm_keyfile_detect_unqualified_path_scheme (info->base_dir, data, data_len, TRUE, &exists);
-	if (!path)
-		return FALSE;
-
-	/* Construct the proper value as required for the PATH scheme */
-	val = g_bytes_new_take (path, strlen (path) + 1);
-	g_object_set (setting, key, val, NULL);
-
-	/* Warn if the certificate didn't exist */
-	if (!exists) {
-		handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_INFO_MISSING_FILE,
-		             _("certificate or key file '%s' does not exist"),
-		             path);
-	}
-	g_bytes_unref (val);
-
-	return TRUE;
-}
+#define HAS_SCHEME_PREFIX(bin, bin_len, scheme) \
+	({ \
+		const char *const _bin = (bin); \
+		const gsize _bin_len = (bin_len); \
+		\
+		nm_assert (_bin && _bin_len > 0); \
+		\
+		(   _bin_len > NM_STRLEN (scheme) + 1 \
+		 && _bin[_bin_len - 1] == '\0' \
+		 && memcmp (_bin, scheme, NM_STRLEN (scheme)) == 0); \
+	})
 
 static void
 cert_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key)
 {
 	const char *setting_name = nm_setting_get_name (setting);
 	gs_unref_bytes GBytes *bytes = NULL;
-	gsize bin_len;
-	const char *bin;
+	const char *bin = NULL;
+	gsize bin_len = 0;
+	char *path;
+	gboolean path_exists;
 
 	bytes = get_bytes (info, setting_name, key, TRUE, FALSE);
-	if (bytes) {
-		/* Try as a path + scheme (ie, starts with "file://") */
-		if (handle_as_scheme (info, bytes, setting, key))
-			return;
-		if (info->error)
+	if (bytes)
+		bin = g_bytes_get_data (bytes, &bin_len);
+	if (bin_len == 0) {
+		if (!info->error) {
+			handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN,
+			             _("invalid key/cert value"));
+		}
+		return;
+	}
+
+	if (HAS_SCHEME_PREFIX (bin, bin_len, NM_KEYFILE_CERT_SCHEME_PREFIX_PATH)) {
+		const char *path2 = &bin[NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH)];
+		gs_free char *path2_free = NULL;
+
+		if (nm_setting_802_1x_check_cert_scheme (bin, bin_len, NULL) != NM_SETTING_802_1X_CK_SCHEME_PATH) {
+			handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN,
+			             _("invalid key/cert value path \"%s\""), bin);
 			return;
+		}
 
-		/* If not, it might be a plain path */
-		if (handle_as_path (info, bytes, setting, key))
+		g_object_set (setting, key, bytes, NULL);
+
+		if (path2[0] != '/') {
+			/* we want to read absolute paths because we use keyfile as exchange
+			 * between different processes which might not have the same cwd. */
+			path2_free = get_cert_path (info->base_dir, (const guint8 *) path2,
+			                            bin_len - NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_PATH) - 1);
+			path2 = path2_free;
+		}
+
+		if (!g_file_test (path2, G_FILE_TEST_EXISTS)) {
+			handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_INFO_MISSING_FILE,
+			             _("certificate or key file '%s' does not exist"),
+			             path2);
+		}
+		return;
+	}
+
+	if (HAS_SCHEME_PREFIX (bin, bin_len, NM_KEYFILE_CERT_SCHEME_PREFIX_PKCS11)) {
+		if (nm_setting_802_1x_check_cert_scheme (bin, bin_len, NULL) != NM_SETTING_802_1X_CK_SCHEME_PKCS11) {
+			handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN,
+			             _("invalid PKCS#11 URI \"%s\""), bin);
 			return;
-		if (info->error)
+		}
+
+		g_object_set (setting, key, bytes, NULL);
+		return;
+	}
+
+	if (HAS_SCHEME_PREFIX (bin, bin_len, NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB)) {
+		const char *cdata = bin + NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB);
+		gsize cdata_len = bin_len - NM_STRLEN (NM_KEYFILE_CERT_SCHEME_PREFIX_BLOB) - 1;
+		gs_free guchar *bin_decoded = NULL;
+		gsize bin_decoded_len = 0;
+		gsize i;
+		gboolean valid_base64;
+		gs_unref_bytes GBytes *val = NULL;
+
+		/* Let's be strict here. We expect valid base64, no funny stuff!!
+		 * We didn't write such invalid data ourselfes and refuse to read it as blob. */
+		if ((valid_base64 = (cdata_len % 4 == 0))) {
+			for (i = 0; i < cdata_len; i++) {
+				char c = cdata[i];
+
+				if (!(   (c >= 'a' && c <= 'z')
+				      || (c >= 'A' && c <= 'Z')
+				      || (c >= '0' && c <= '9')
+				      || (c == '+' || c == '/'))) {
+					if (c != '=' || i < cdata_len - 2)
+						valid_base64 = FALSE;
+					else {
+						for (; i < cdata_len; i++) {
+							if (cdata[i] != '=')
+								valid_base64 = FALSE;
+						}
+					}
+					break;
+				}
+			}
+		}
+		if (valid_base64)
+			bin_decoded = g_base64_decode (cdata, &bin_decoded_len);
+
+		if (bin_decoded_len == 0) {
+			handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN,
+			             _("invalid key/cert value data:;base64, is not base64"));
 			return;
+		}
 
-		bin = g_bytes_get_data (bytes, &bin_len);
-		if (nm_setting_802_1x_check_cert_scheme (bin, bin_len, NULL) != NM_SETTING_802_1X_CK_SCHEME_BLOB) {
-			/* The blob probably starts with "file://" but contains invalid characters for a path.
-			 * Setting the cert data will confuse NMSetting8021x.
-			 * In fact, NMSetting8021x does not support setting such binary data, so just warn and
-			 * continue. */
+		if (nm_setting_802_1x_check_cert_scheme (bin_decoded, bin_decoded_len, NULL) != NM_SETTING_802_1X_CK_SCHEME_BLOB) {
+			/* The blob probably starts with "file://". Setting the cert data will confuse NMSetting8021x.
+			 * In fact this is a limitation of NMSetting8021x which does not support setting blobs that start
+			 * with file://. Just warn and return TRUE to signal that we ~handled~ the setting. */
 			handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN,
-			             _("invalid key/cert value is not a valid blob"));
-		} else
-			g_object_set (setting, key, bytes, NULL);
-	} else if (!info->error) {
+			             _("invalid key/cert value data:;base64,file://"));
+			return;
+		}
+
+		val = g_bytes_new_take (g_steal_pointer (&bin_decoded), bin_decoded_len);
+		g_object_set (setting, key, val, NULL);
+		return;
+	}
+
+	/* If not, it might be a plain path */
+	path = nm_keyfile_detect_unqualified_path_scheme (info->base_dir, bin, bin_len, TRUE, &path_exists);
+	if (path) {
+		gs_unref_bytes GBytes *val = NULL;
+
+		/* Construct the proper value as required for the PATH scheme */
+		val = g_bytes_new_take (path, strlen (path) + 1);
+		g_object_set (setting, key, val, NULL);
+
+		/* Warn if the certificate didn't exist */
+		if (!path_exists) {
+			handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_INFO_MISSING_FILE,
+			             _("certificate or key file '%s' does not exist"),
+			             path);
+		}
+		return;
+	}
+
+	if (nm_setting_802_1x_check_cert_scheme (bin, bin_len, NULL) != NM_SETTING_802_1X_CK_SCHEME_BLOB) {
+		/* The blob probably starts with "file://" but contains invalid characters for a path.
+		 * Setting the cert data will confuse NMSetting8021x.
+		 * In fact, NMSetting8021x does not support setting such binary data, so just warn and
+		 * continue. */
 		handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN,
-		             _("invalid key/cert value"));
+		             _("invalid key/cert value is not a valid blob"));
+		return;
 	}
+
+	g_object_set (setting, key, bytes, NULL);
 }
 
 static void