libnm/crypto: refactor nm_crypto_load_and_verify_certificate() and return GBytes

The GBytes has a suitable cleanup function, which zeros the certificate
from memory.

Also, all callers that require the certificate, actually later converted
it into a GBytes anyway. This way, they can re-used the same instance
(avoiding an additionaly copying of the data), and they will properly
clear the memory when freed.

5 files changed, 71 insertions, 73 deletions

diff --git a/libnm-core/nm-crypto.c b/libnm-core/nm-crypto.c
index 2f222aa3f0..7f84614cc7 100644
--- a/libnm-core/nm-crypto.c
+++ b/libnm-core/nm-crypto.c
@@ -644,15 +644,16 @@ extract_pem_cert_data (const guint8 *contents,
 	return TRUE;
 }
 
-GByteArray *
+gboolean
 nm_crypto_load_and_verify_certificate (const char *file,
                                        NMCryptoFileFormat *out_file_format,
+                                       GBytes **out_certificate,
                                        GError **error)
 {
 	nm_auto_clear_secret_ptr NMSecretPtr contents = { 0 };
 
-	g_return_val_if_fail (file != NULL, NULL);
-	g_return_val_if_fail (out_file_format != NULL, NULL);
+	g_return_val_if_fail (file, FALSE);
+	nm_assert (!error || !*error);
 
 	if (!_nm_crypto_init (error))
 		goto out;
@@ -670,23 +671,26 @@ nm_crypto_load_and_verify_certificate (const char *file,
 
 	/* Check for PKCS#12 */
 	if (nm_crypto_is_pkcs12_data (contents.bin, contents.len, NULL)) {
-		*out_file_format = NM_CRYPTO_FILE_FORMAT_PKCS12;
-		return to_gbyte_array_mem (contents.bin, contents.len);
+		NM_SET_OUT (out_file_format, NM_CRYPTO_FILE_FORMAT_PKCS12);
+		NM_SET_OUT (out_certificate, nm_secret_copy_to_gbytes (contents.bin, contents.len));
+		return TRUE;
 	}
 
 	/* Check for plain DER format */
 	if (contents.len > 2 && contents.bin[0] == 0x30 && contents.bin[1] == 0x82) {
 		if (_nm_crypto_verify_x509 (contents.bin, contents.len, NULL)) {
-			*out_file_format = NM_CRYPTO_FILE_FORMAT_X509;
-			return to_gbyte_array_mem (contents.bin, contents.len);
+			NM_SET_OUT (out_file_format, NM_CRYPTO_FILE_FORMAT_X509);
+			NM_SET_OUT (out_certificate, nm_secret_copy_to_gbytes (contents.bin, contents.len));
+			return TRUE;
 		}
 	} else {
 		nm_auto_clear_secret_ptr NMSecretPtr pem_cert = { 0 };
 
 		if (extract_pem_cert_data (contents.bin, contents.len, &pem_cert, NULL)) {
 			if (_nm_crypto_verify_x509 (pem_cert.bin, pem_cert.len, NULL)) {
-				*out_file_format = NM_CRYPTO_FILE_FORMAT_X509;
-				return to_gbyte_array_mem (contents.bin, contents.len);
+				NM_SET_OUT (out_file_format, NM_CRYPTO_FILE_FORMAT_X509);
+				NM_SET_OUT (out_certificate, nm_secret_copy_to_gbytes (contents.bin, contents.len));
+				return TRUE;
 			}
 		}
 	}
@@ -697,8 +701,9 @@ nm_crypto_load_and_verify_certificate (const char *file,
 	             _("Failed to recognize certificate"));
 
 out:
-	*out_file_format = NM_CRYPTO_FILE_FORMAT_UNKNOWN;
-	return NULL;
+	NM_SET_OUT (out_file_format, NM_CRYPTO_FILE_FORMAT_UNKNOWN);
+	NM_SET_OUT (out_certificate, NULL);
+	return FALSE;
 }
 
 gboolean
diff --git a/libnm-core/nm-crypto.h b/libnm-core/nm-crypto.h
index 3c073a9916..669f9660eb 100644
--- a/libnm-core/nm-crypto.h
+++ b/libnm-core/nm-crypto.h
@@ -60,9 +60,10 @@ GByteArray *nmtst_crypto_decrypt_openssl_private_key (const char *file,
                                                       NMCryptoKeyType *out_key_type,
                                                       GError **error);
 
-GByteArray *nm_crypto_load_and_verify_certificate (const char *file,
-                                                   NMCryptoFileFormat *out_file_format,
-                                                   GError **error);
+gboolean nm_crypto_load_and_verify_certificate (const char *file,
+                                                NMCryptoFileFormat *out_file_format,
+                                                GBytes **out_certificat,
+                                                GError **error);
 
 gboolean nm_crypto_is_pkcs12_file (const char *file, GError **error);
 
diff --git a/libnm-core/nm-setting-8021x.c b/libnm-core/nm-setting-8021x.c
index 30f4537e9c..4fae2c64e6 100644
--- a/libnm-core/nm-setting-8021x.c
+++ b/libnm-core/nm-setting-8021x.c
@@ -520,33 +520,41 @@ nm_setting_802_1x_check_cert_scheme (gconstpointer pdata, gsize length, GError *
 	return scheme;
 }
 
-static GByteArray *
+static GBytes *
 load_and_verify_certificate (const char *cert_path,
                              NMSetting8021xCKScheme scheme,
                              NMCryptoFileFormat *out_file_format,
                              GError **error)
 {
 	NMCryptoFileFormat format = NM_CRYPTO_FILE_FORMAT_UNKNOWN;
-	GByteArray *array;
+	gs_unref_bytes GBytes *cert = NULL;
+
+	if (!nm_crypto_load_and_verify_certificate (cert_path, &format, &cert, error)) {
+		NM_SET_OUT (out_file_format, NM_CRYPTO_FILE_FORMAT_UNKNOWN);
+		return NULL;
+	}
 
-	array = nm_crypto_load_and_verify_certificate (cert_path, &format, error);
+	nm_assert (format != NM_CRYPTO_FILE_FORMAT_UNKNOWN);
+	nm_assert (cert);
 
-	if (!array || !array->len || format == NM_CRYPTO_FILE_FORMAT_UNKNOWN) {
-		/* the array is empty or the format is already unknown. */
-		format = NM_CRYPTO_FILE_FORMAT_UNKNOWN;
-	} else if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) {
+	if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) {
+		const guint8 *bin;
+		gsize len;
+
+		bin = g_bytes_get_data (cert, &len);
 		/* If we load the file as blob, we must ensure that the binary data does not
 		 * start with file://. NMSetting8021x cannot represent blobs that start with
 		 * file://.
 		 * If that's the case, coerce the format to UNKNOWN. The callers will take care
 		 * of that and not set the blob. */
-		if (nm_setting_802_1x_check_cert_scheme (array->data, array->len, NULL) != NM_SETTING_802_1X_CK_SCHEME_BLOB)
-			format = NM_CRYPTO_FILE_FORMAT_UNKNOWN;
+		if (nm_setting_802_1x_check_cert_scheme (bin, len, NULL) != NM_SETTING_802_1X_CK_SCHEME_BLOB) {
+			NM_SET_OUT (out_file_format, NM_CRYPTO_FILE_FORMAT_UNKNOWN);
+			return NULL;
+		}
 	}
 
-	if (out_file_format)
-		*out_file_format = format;
-	return array;
+	NM_SET_OUT (out_file_format, format);
+	return g_steal_pointer (&cert);
 }
 
 /**
@@ -700,7 +708,7 @@ nm_setting_802_1x_set_ca_cert (NMSetting8021x *setting,
 {
 	NMSetting8021xPrivate *priv;
 	NMCryptoFileFormat format = NM_CRYPTO_FILE_FORMAT_UNKNOWN;
-	GByteArray *data;
+	gs_unref_bytes GBytes *cert = NULL;
 
 	g_return_val_if_fail (NM_IS_SETTING_802_1X (setting), FALSE);
 
@@ -730,17 +738,16 @@ nm_setting_802_1x_set_ca_cert (NMSetting8021x *setting,
 		return TRUE;
 	}
 
-	data = load_and_verify_certificate (value, scheme, &format, error);
-	if (data) {
+	cert = load_and_verify_certificate (value, scheme, &format, error);
+	if (cert) {
 		/* wpa_supplicant can only use raw x509 CA certs */
 		if (format == NM_CRYPTO_FILE_FORMAT_X509) {
 			if (out_format)
 				*out_format = NM_SETTING_802_1X_CK_FORMAT_X509;
 
-			if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) {
-				priv->ca_cert = g_byte_array_free_to_bytes (data);
-				data = NULL;
-			} else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH)
+			if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB)
+				priv->ca_cert = g_bytes_ref (cert);
+			else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH)
 				priv->ca_cert = path_to_scheme_value (value);
 			else
 				g_assert_not_reached ();
@@ -751,8 +758,6 @@ nm_setting_802_1x_set_ca_cert (NMSetting8021x *setting,
 			             _("CA certificate must be in X.509 format"));
 			g_prefix_error (error, "%s.%s: ", NM_SETTING_802_1X_SETTING_NAME, NM_SETTING_802_1X_CA_CERT);
 		}
-		if (data)
-			g_byte_array_unref (data);
 	}
 
 	g_object_notify (G_OBJECT (setting), NM_SETTING_802_1X_CA_CERT);
@@ -1104,7 +1109,7 @@ nm_setting_802_1x_set_client_cert (NMSetting8021x *setting,
 {
 	NMSetting8021xPrivate *priv;
 	NMCryptoFileFormat format = NM_CRYPTO_FILE_FORMAT_UNKNOWN;
-	GByteArray *data;
+	gs_unref_bytes GBytes *cert = NULL;
 
 	g_return_val_if_fail (NM_IS_SETTING_802_1X (setting), FALSE);
 
@@ -1134,8 +1139,8 @@ nm_setting_802_1x_set_client_cert (NMSetting8021x *setting,
 		return TRUE;
 	}
 
-	data = load_and_verify_certificate (value, scheme, &format, error);
-	if (data) {
+	cert = load_and_verify_certificate (value, scheme, &format, error);
+	if (cert) {
 		gboolean valid = FALSE;
 
 		switch (format) {
@@ -1159,16 +1164,13 @@ nm_setting_802_1x_set_client_cert (NMSetting8021x *setting,
 		}
 
 		if (valid) {
-			if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) {
-				priv->client_cert = g_byte_array_free_to_bytes (data);
-				data = NULL;
-			} else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH)
+			if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB)
+				priv->client_cert = g_bytes_ref (cert);
+			else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH)
 				priv->client_cert = path_to_scheme_value (value);
 			else
 				g_assert_not_reached ();
 		}
-		if (data)
-			g_byte_array_unref (data);
 	}
 
 	g_object_notify (G_OBJECT (setting), NM_SETTING_802_1X_CLIENT_CERT);
@@ -1459,7 +1461,7 @@ nm_setting_802_1x_set_phase2_ca_cert (NMSetting8021x *setting,
 {
 	NMSetting8021xPrivate *priv;
 	NMCryptoFileFormat format = NM_CRYPTO_FILE_FORMAT_UNKNOWN;
-	GByteArray *data;
+	gs_unref_bytes GBytes *cert = NULL;
 
 	g_return_val_if_fail (NM_IS_SETTING_802_1X (setting), FALSE);
 
@@ -1489,17 +1491,16 @@ nm_setting_802_1x_set_phase2_ca_cert (NMSetting8021x *setting,
 		return TRUE;
 	}
 
-	data = load_and_verify_certificate (value, scheme, &format, error);
-	if (data) {
+	cert = load_and_verify_certificate (value, scheme, &format, error);
+	if (cert) {
 		/* wpa_supplicant can only use raw x509 CA certs */
 		if (format == NM_CRYPTO_FILE_FORMAT_X509) {
 			if (out_format)
 				*out_format = NM_SETTING_802_1X_CK_FORMAT_X509;
 
-			if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) {
-				priv->phase2_ca_cert = g_byte_array_free_to_bytes (data);
-				data = NULL;
-			} else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH)
+			if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB)
+				priv->phase2_ca_cert = g_bytes_ref (cert);
+			else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH)
 				priv->phase2_ca_cert = path_to_scheme_value (value);
 			else
 				g_assert_not_reached ();
@@ -1510,8 +1511,6 @@ nm_setting_802_1x_set_phase2_ca_cert (NMSetting8021x *setting,
 			                     _("invalid certificate format"));
 			g_prefix_error (error, "%s.%s: ", NM_SETTING_802_1X_SETTING_NAME, NM_SETTING_802_1X_PHASE2_CA_CERT);
 		}
-		if (data)
-			g_byte_array_unref (data);
 	}
 
 	g_object_notify (G_OBJECT (setting), NM_SETTING_802_1X_PHASE2_CA_CERT);
@@ -1867,7 +1866,7 @@ nm_setting_802_1x_set_phase2_client_cert (NMSetting8021x *setting,
 {
 	NMSetting8021xPrivate *priv;
 	NMCryptoFileFormat format = NM_CRYPTO_FILE_FORMAT_UNKNOWN;
-	GByteArray *data;
+	gs_unref_bytes GBytes *cert = NULL;
 
 	g_return_val_if_fail (NM_IS_SETTING_802_1X (setting), FALSE);
 
@@ -1897,8 +1896,8 @@ nm_setting_802_1x_set_phase2_client_cert (NMSetting8021x *setting,
 		return TRUE;
 	}
 
-	data = load_and_verify_certificate (value, scheme, &format, error);
-	if (data) {
+	cert = load_and_verify_certificate (value, scheme, &format, error);
+	if (cert) {
 		gboolean valid = FALSE;
 
 		/* wpa_supplicant can only use raw x509 CA certs */
@@ -1923,16 +1922,13 @@ nm_setting_802_1x_set_phase2_client_cert (NMSetting8021x *setting,
 		}
 
 		if (valid) {
-			if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB) {
-				priv->phase2_client_cert = g_byte_array_free_to_bytes (data);
-				data = NULL;
-			} else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH)
+			if (scheme == NM_SETTING_802_1X_CK_SCHEME_BLOB)
+				priv->phase2_client_cert = g_bytes_ref (cert);
+			else if (scheme == NM_SETTING_802_1X_CK_SCHEME_PATH)
 				priv->phase2_client_cert = path_to_scheme_value (value);
 			else
 				g_assert_not_reached ();
 		}
-		if (data)
-			g_byte_array_unref (data);
 	}
 
 	g_object_notify (G_OBJECT (setting), NM_SETTING_802_1X_PHASE2_CLIENT_CERT);
diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c
index 21de5624a8..1542625c01 100644
--- a/libnm-core/nm-utils.c
+++ b/libnm-core/nm-utils.c
@@ -3060,18 +3060,15 @@ gboolean
 nm_utils_file_is_certificate (const char *filename)
 {
 	const char *extensions[] = { ".der", ".pem", ".crt", ".cer", NULL };
-	NMCryptoFileFormat file_format = NM_CRYPTO_FILE_FORMAT_UNKNOWN;
-	GByteArray *cert;
+	NMCryptoFileFormat file_format;
 
 	g_return_val_if_fail (filename != NULL, FALSE);
 
 	if (!file_has_extension (filename, extensions))
 		return FALSE;
 
-	cert = nm_crypto_load_and_verify_certificate (filename, &file_format, NULL);
-	if (cert)
-		g_byte_array_unref (cert);
-
+	if (!nm_crypto_load_and_verify_certificate (filename, &file_format, NULL, NULL))
+		return FALSE;
 	return file_format = NM_CRYPTO_FILE_FORMAT_X509;
 }
 
diff --git a/libnm-core/tests/test-crypto.c b/libnm-core/tests/test-crypto.c
index 937fc6c3b0..26ef08f56c 100644
--- a/libnm-core/tests/test-crypto.c
+++ b/libnm-core/tests/test-crypto.c
@@ -99,18 +99,17 @@ static void
 test_cert (gconstpointer test_data)
 {
 	gs_free char *path = NULL;
-	GByteArray *array;
+	gs_unref_bytes GBytes *cert = NULL;
 	NMCryptoFileFormat format = NM_CRYPTO_FILE_FORMAT_UNKNOWN;
 	GError *error = NULL;
+	gboolean success;
 
 	path = g_build_filename (TEST_CERT_DIR, (const char *) test_data, NULL);
 
-	array = nm_crypto_load_and_verify_certificate (path, &format, &error);
-	g_assert_no_error (error);
+	success = nm_crypto_load_and_verify_certificate (path, &format, &cert, &error);
+	nmtst_assert_success (success, error);
 	g_assert_cmpint (format, ==, NM_CRYPTO_FILE_FORMAT_X509);
 
-	g_byte_array_free (array, TRUE);
-
 	g_assert (nm_utils_file_is_certificate (path));
 }