keyfile: better handle cert/key files that don't exist (bgo #649807)

The keyfile code has to handle a few different formats of cert/key values,
and wasn't doing a good enough job of detecting plain paths as values.  By
default the writer will write out a plain path (ie, not prefixed with file://)
and the reader will handle that correctly, *unless* that file does not
exist, at which the reader assumed it was a byte array.  This caused the
read-in keyfile not to match the in-memory connection (since the in-memory
connection though the cert/key held a path, but the read-in one thought it
contained a blob) and this seems to eventually have triggered a write-out
with the new values (as a blob), which would then drop a .pem file into
system-connections/ containing the path that should have been in the
keyfile in the first place.

This all happened because we assumed that the given path for the cert or
key would actually be valid, which doesn't seem to be the case for a lot
of people.  Clearly these connections won't work (since the certificate or
key does not exist) but the keyfile plugin shouldn't be messing up the
connection's settings at the very least.

Fix that by handling the check of whether the cert/key data is a path or
not in a less restrictive manner and add some testcases to make sure that
everything works as we expect.

src/settings/plugins/keyfile/reader.c

@@ -40,6 +40,7 @@
 #include <ctype.h>
 
 #include "nm-dbus-glib-types.h"
+#include "nm-system-config-interface.h"
 #include "reader.h"
 #include "common.h"
 
@@ -835,6 +836,24 @@ get_cert_path (const char *keyfile_path, GByteArray *cert_path)
 
 #define SCHEME_PATH "file://"
 
+static const char *certext[] = { ".pem", ".cert", ".crt", ".cer", ".p12", ".der", ".key" };
+
+static gboolean
+has_cert_ext (GByteArray *array)
+{
+	int i;
+
+	for (i = 0; i < G_N_ELEMENTS (certext); i++) {
+		guint32 extlen = strlen (certext[i]);
+
+		if (array->len <= extlen)
+			continue;
+		if (memcmp (&array->data[array->len - extlen], certext[i], extlen) == 0)
+			return TRUE;
+	}
+	return FALSE;
+}
+
 static void
 cert_parser (NMSetting *setting, const char *key, GKeyFile *keyfile, const char *keyfile_path)
 {
@@ -859,9 +878,18 @@ cert_parser (NMSetting *setting, const char *key, GKeyFile *keyfile, const char
 		           && g_utf8_validate ((const char *) array->data, array->len, NULL)) {
 			GByteArray *val;
 			char *path;
+			gboolean exists;
+
+			/* Might be a bare path without the file:// prefix; in that case
+			 * if it's an absolute path, use that, otherwise treat it as a
+			 * relative path to the current directory.
+			 */
 
 			path = get_cert_path (keyfile_path, array);
-			if (g_file_test (path, G_FILE_TEST_EXISTS)) {
+			exists = g_file_test (path, G_FILE_TEST_EXISTS);
+			if (   exists
+			    || memchr (array->data, '/', array->len)
+			    || has_cert_ext (array)) {
 				/* Construct the proper value as required for the PATH scheme */
 				val = g_byte_array_sized_new (strlen (SCHEME_PATH) + array->len + 1);
 				g_byte_array_append (val, (const guint8 *) SCHEME_PATH, strlen (SCHEME_PATH));
@@ -870,6 +898,11 @@ cert_parser (NMSetting *setting, const char *key, GKeyFile *keyfile, const char
 				g_object_set (setting, key, val, NULL);
 				g_byte_array_free (val, TRUE);
 				success = TRUE;
+
+				/* Warn if the certificate didn't exist */
+				if (exists == FALSE) {
+					PLUGIN_WARN (KEYFILE_PLUGIN_NAME, "   certificate or key %s does not exist", path);
+				}
 			}
 			g_free (path);
 		}

src/settings/plugins/keyfile/tests/keyfiles/Makefile.am

@@ -8,7 +8,9 @@ KEYFILES = \
 	ATT_Data_Connect_Plain \
 	Test_String_SSID \
 	Test_Wired_TLS_Old \
-	Test_Wired_TLS_New
+	Test_Wired_TLS_New \
+	Test_Wired_TLS_Blob \
+	Test_Wired_TLS_Path_Missing
 
 CERTS = \
 	test-ca-cert.pem \

src/settings/plugins/keyfile/tests/keyfiles/Test_Wired_TLS_Blob

@@ -0,0 +1,22 @@
+
+[connection]
+id=Wired TLS
+uuid=5ee46013-9469-4c6a-a60a-0c7a1e1c7488
+type=802-3-ethernet
+
+[802-1x]
+eap=tls;
+identity=Bill Smith
+ca-cert=48;130;2;52;48;130;1;161;2;16;2;173;102;126;78;69;254;94;87;111;60;152;25;94;221;192;48;13;6;9;42;134;72;134;247;13;1;1;2;5;0;48;95;49;11;48;9;6;3;85;4;6;19;2;85;83;49;32;48;30;6;3;85;4;10;19;23;82;83;65;32;68;97;116;97;32;83;101;99;117;114;105;116;121;44;32;73;110;99;46;49;46;48;44;6;3;85;4;11;19;37;83;101;99;117;114;101;32;83;101;114;118;101;114;32;67;101;114;116;105;102;105;99;97;116;105;111;110;32;65;117;116;104;111;114;105;116;121;48;30;23;13;57;52;49;49;48;57;48;48;48;48;48;48;90;23;13;49;48;48;49;48;55;50;51;53;57;53;57;90;48;95;49;11;48;9;6;3;85;4;6;19;2;85;83;49;32;48;30;6;3;85;4;10;19;23;82;83;65;32;68;97;116;97;32;83;101;99;117;114;105;116;121;44;32;73;110;99;46;49;46;48;44;6;3;85;4;11;19;37;83;101;99;117;114;101;32;83;101;114;118;101;114;32;67;101;114;116;105;102;105;99;97;116;105;111;110;32;65;117;116;104;111;114;105;116;121;48;129;155;48;13;6;9;42;134;72;134;247;13;1;1;1;5;0;3;129;137;0;48;129;133;2;126;0;146;206;122;193;174;131;62;90;170;137;131;87;172;37;1;118;12;173;174;142;44;55;206;235;53;120;100;84;3;229;132;64;81;201;191;143;8;226;138;130;8;210;22;134;55;85;233;177;33;2;173;118;104;129;154;5;162;75;201;75;37;102;34;86;108;136;7;143;247;129;89;109;132;7;101;112;19;113;118;62;155;119;76;227;80;137;86;152;72;185;29;167;41;26;19;46;74;17;89;156;30;21;213;73;84;44;115;58;105;130;177;151;57;156;109;112;103;72;229;221;45;214;200;30;123;2;3;1;0;1;48;13;6;9;42;134;72;134;247;13;1;1;2;5;0;3;126;0;101;221;126;225;178;236;176;226;58;224;236;113;70;154;25;17;184;211;199;160;180;3;64;38;2;62;9;156;225;18;179;209;90;246;55;165;183;97;3;182;91;22;105;59;198;68;8;12;136;83;12;107;151;73;199;62;53;220;108;185;187;170;223;92;187;58;47;147;96;182;169;75;77;242;32;247;205;95;127;100;123;142;220;0;92;215;250;119;202;57;22;89;111;14;234;211;181;131;127;77;77;66;86;118;180;201;95;4;248;56;248;235;210;95;117;95;205;123;252;229;142;128;124;252;80;
+client-cert=102;105;108;101;58;47;47;47;104;111;109;101;47;100;99;98;119;47;68;101;115;107;116;111;112;47;99;101;114;116;105;110;102;114;97;47;99;108;105;101;110;116;46;112;101;109;0;
+private-key=102;105;108;101;58;47;47;47;104;111;109;101;47;100;99;98;119;47;68;101;115;107;116;111;112;47;99;101;114;116;105;110;102;114;97;47;99;108;105;101;110;116;46;112;101;109;0;
+private-key-password=12345testing
+
+[ipv4]
+method=auto
+
+[802-3-ethernet]
+duplex=full
+
+[ipv6]
+method=ignore

src/settings/plugins/keyfile/tests/keyfiles/Test_Wired_TLS_Path_Missing

@@ -0,0 +1,22 @@
+
+[connection]
+id=Wired TLS
+uuid=5ee46013-9469-4c6a-a60a-0c7a1e1c7488
+type=802-3-ethernet
+
+[802-1x]
+eap=tls;
+identity=Bill Smith
+ca-cert=/some/random/cert/path.pem
+client-cert=test-key-and-cert.pem
+private-key=test-key-and-cert.pem
+private-key-password=12345testing
+
+[ipv4]
+method=auto
+
+[802-3-ethernet]
+duplex=full
+
+[ipv6]
+method=ignore

src/settings/plugins/keyfile/tests/test-keyfile.c

@@ -1953,6 +1953,133 @@ test_write_gsm_connection (void)
 	g_object_unref (connection);
 }
 
+#define TEST_WIRED_TLS_BLOB_FILE TEST_KEYFILES_DIR"/Test_Wired_TLS_Blob"
+
+static void
+test_read_wired_8021x_tls_blob_connection (void)
+{
+	NMConnection *connection;
+	NMSetting *s_wired;
+	NMSetting8021x *s_8021x;
+	GError *error = NULL;
+	const char *tmp;
+	gboolean success;
+	const GByteArray *array;
+
+	connection = nm_keyfile_plugin_connection_from_file (TEST_WIRED_TLS_BLOB_FILE, &error);
+	if (connection == NULL) {
+		g_assert (error);
+		g_warning ("Failed to read %s: %s", TEST_WIRED_TLS_BLOB_FILE, error->message);
+		g_assert (connection);
+	}
+
+	success = nm_connection_verify (connection, &error);
+	if (!success) {
+		g_assert (error);
+		g_warning ("Failed to verify %s: %s", TEST_WIRED_TLS_BLOB_FILE, error->message);
+		g_assert (success);
+	}
+
+	/* ===== Wired Setting ===== */
+	s_wired = nm_connection_get_setting (connection, NM_TYPE_SETTING_WIRED);
+	g_assert (s_wired != NULL);
+
+	/* ===== 802.1x Setting ===== */
+	s_8021x = (NMSetting8021x *) nm_connection_get_setting (connection, NM_TYPE_SETTING_802_1X);
+	g_assert (s_8021x != NULL);
+
+	g_assert (nm_setting_802_1x_get_num_eap_methods (s_8021x) == 1);
+	tmp = nm_setting_802_1x_get_eap_method (s_8021x, 0);
+	g_assert (g_strcmp0 (tmp, "tls") == 0);
+
+	tmp = nm_setting_802_1x_get_identity (s_8021x);
+	g_assert (g_strcmp0 (tmp, "Bill Smith") == 0);
+
+	tmp = nm_setting_802_1x_get_private_key_password (s_8021x);
+	g_assert (g_strcmp0 (tmp, "12345testing") == 0);
+
+	g_assert_cmpint (nm_setting_802_1x_get_ca_cert_scheme (s_8021x), ==, NM_SETTING_802_1X_CK_SCHEME_BLOB);
+
+	/* Make sure it's not a path, since it's a blob */
+	tmp = nm_setting_802_1x_get_ca_cert_path (s_8021x);
+	g_assert (tmp == NULL);
+
+	/* Validate the path */
+	array = nm_setting_802_1x_get_ca_cert_blob (s_8021x);
+	g_assert (array != NULL);
+	g_assert_cmpint (array->len, ==, 568);
+
+	tmp = nm_setting_802_1x_get_client_cert_path (s_8021x);
+	g_assert_cmpstr (tmp, ==, "/home/dcbw/Desktop/certinfra/client.pem");
+
+	tmp = nm_setting_802_1x_get_private_key_path (s_8021x);
+	g_assert_cmpstr (tmp, ==, "/home/dcbw/Desktop/certinfra/client.pem");
+
+	g_object_unref (connection);
+}
+
+#define TEST_WIRED_TLS_PATH_MISSING_FILE TEST_KEYFILES_DIR"/Test_Wired_TLS_Path_Missing"
+
+static void
+test_read_wired_8021x_tls_bad_path_connection (void)
+{
+	NMConnection *connection;
+	NMSetting *s_wired;
+	NMSetting8021x *s_8021x;
+	GError *error = NULL;
+	const char *tmp;
+	char *tmp2;
+	gboolean success;
+
+	connection = nm_keyfile_plugin_connection_from_file (TEST_WIRED_TLS_PATH_MISSING_FILE, &error);
+	if (connection == NULL) {
+		g_assert (error);
+		g_warning ("Failed to read %s: %s", TEST_WIRED_TLS_PATH_MISSING_FILE, error->message);
+		g_assert (connection);
+	}
+
+	success = nm_connection_verify (connection, &error);
+	if (!success) {
+		g_assert (error);
+		g_warning ("Failed to verify %s: %s", TEST_WIRED_TLS_BLOB_FILE, error->message);
+		g_assert (success);
+	}
+
+	/* ===== Wired Setting ===== */
+	s_wired = nm_connection_get_setting (connection, NM_TYPE_SETTING_WIRED);
+	g_assert (s_wired != NULL);
+
+	/* ===== 802.1x Setting ===== */
+	s_8021x = (NMSetting8021x *) nm_connection_get_setting (connection, NM_TYPE_SETTING_802_1X);
+	g_assert (s_8021x != NULL);
+
+	g_assert (nm_setting_802_1x_get_num_eap_methods (s_8021x) == 1);
+	tmp = nm_setting_802_1x_get_eap_method (s_8021x, 0);
+	g_assert (g_strcmp0 (tmp, "tls") == 0);
+
+	tmp = nm_setting_802_1x_get_identity (s_8021x);
+	g_assert (g_strcmp0 (tmp, "Bill Smith") == 0);
+
+	tmp = nm_setting_802_1x_get_private_key_password (s_8021x);
+	g_assert (g_strcmp0 (tmp, "12345testing") == 0);
+
+	g_assert_cmpint (nm_setting_802_1x_get_ca_cert_scheme (s_8021x), ==, NM_SETTING_802_1X_CK_SCHEME_PATH);
+
+	tmp = nm_setting_802_1x_get_ca_cert_path (s_8021x);
+	g_assert_cmpstr (tmp, ==, "/some/random/cert/path.pem");
+
+	tmp2 = g_strdup_printf (TEST_KEYFILES_DIR "/test-key-and-cert.pem");
+
+	tmp = nm_setting_802_1x_get_client_cert_path (s_8021x);
+	g_assert_cmpstr (tmp, ==, tmp2);
+
+	tmp = nm_setting_802_1x_get_private_key_path (s_8021x);
+	g_assert_cmpstr (tmp, ==, tmp2);
+
+	g_free (tmp2);
+	g_object_unref (connection);
+}
+
 #define TEST_WIRED_TLS_OLD_FILE TEST_KEYFILES_DIR"/Test_Wired_TLS_Old"
 
 static void
@@ -2367,6 +2494,9 @@ int main (int argc, char **argv)
 	test_read_gsm_connection ();
 	test_write_gsm_connection ();
 
+	test_read_wired_8021x_tls_blob_connection ();
+	test_read_wired_8021x_tls_bad_path_connection ();
+
 	test_read_wired_8021x_tls_old_connection ();
 	test_read_wired_8021x_tls_new_connection ();
 	test_write_wired_8021x_tls_connection_path ();