diff options
author | Thomas Haller <thaller@redhat.com> | 2017-05-05 12:01:17 +0200 |
---|---|---|
committer | Thomas Haller <thaller@redhat.com> | 2017-05-06 14:12:18 +0200 |
commit | 8ef57d0f7ef26247bf53b4d07f712a1bf4de4bb2 (patch) | |
tree | a38eb4bcbe6ad7bf0235a216feec76ab11ebf7e1 | |
parent | 095c6f5d534c7f072f9cba57d5ad232dbbfd4fe1 (diff) | |
download | NetworkManager-8ef57d0f7ef26247bf53b4d07f712a1bf4de4bb2.tar.gz |
keyfile: fix handling unsupported characters in keys
vpn.data, bond.options, and user.data encode their values directly as
keys in keyfile. However, keys for GKeyFile may not contain characters
like '='.
We need to escape such special characters, otherwise an assertion
is hit on the server:
$ nmcli connection modify "$VPN_NAME" +vpn.data 'aa[=value'
Another example of encountering the assertion is when setting user-data key
with an invalid character "my.this=key=is=causes=a=crash".
-rw-r--r-- | libnm-core/nm-keyfile-reader.c | 13 | ||||
-rw-r--r-- | libnm-core/nm-keyfile-utils.c | 202 | ||||
-rw-r--r-- | libnm-core/nm-keyfile-utils.h | 6 | ||||
-rw-r--r-- | libnm-core/nm-keyfile-writer.c | 6 | ||||
-rw-r--r-- | libnm-core/tests/test-keyfile.c | 64 |
5 files changed, 286 insertions, 5 deletions
diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c index 293f03b629..078c517e8e 100644 --- a/libnm-core/nm-keyfile-reader.c +++ b/libnm-core/nm-keyfile-reader.c @@ -724,18 +724,23 @@ read_hash_of_string (GKeyFile *file, NMSetting *setting, const char *key) return; for (iter = (const char *const*) keys; *iter; iter++) { + gs_free char *to_free = NULL; + const char *name; + value = nm_keyfile_plugin_kf_get_string (file, setting_name, *iter, NULL); if (!value) continue; + name = nm_keyfile_key_decode (*iter, &to_free); + if (NM_IS_SETTING_VPN (setting)) { /* Add any item that's not a class property to the data hash */ - if (!g_object_class_find_property (G_OBJECT_GET_CLASS (setting), *iter)) - nm_setting_vpn_add_data_item (NM_SETTING_VPN (setting), *iter, value); + if (!g_object_class_find_property (G_OBJECT_GET_CLASS (setting), name)) + nm_setting_vpn_add_data_item (NM_SETTING_VPN (setting), name, value); } if (NM_IS_SETTING_BOND (setting)) { - if (strcmp (*iter, "interface-name")) - nm_setting_bond_add_option (NM_SETTING_BOND (setting), *iter, value); + if (strcmp (name, "interface-name")) + nm_setting_bond_add_option (NM_SETTING_BOND (setting), name, value); } g_free (value); } diff --git a/libnm-core/nm-keyfile-utils.c b/libnm-core/nm-keyfile-utils.c index 13cf616161..1b7860d873 100644 --- a/libnm-core/nm-keyfile-utils.c +++ b/libnm-core/nm-keyfile-utils.c @@ -361,3 +361,205 @@ _nm_keyfile_has_values (GKeyFile *keyfile) groups = g_key_file_get_groups (keyfile, NULL); return groups && groups[0]; } + +/*****************************************************************************/ + +static const char * +_keyfile_key_encode (const char *name, + char **out_to_free) +{ + gsize len, i; + GString *str; + + nm_assert (name); + nm_assert (out_to_free && !*out_to_free); + + /* See g_key_file_is_key_name(). + * + * GKeyfile allows all UTF-8 characters (even non-well formed sequences), + * except: + * - no empty keys + * - no leading/trailing ' ' + * - no '=', '[', ']' + * + * We do something more strict here. All non-ASCII characters, all non-printable + * characters, and all invalid characters are escaped with "\\XX". + * + * We don't escape \\, unless it is followed by two hex digits. + */ + + if (!name[0]) { + /* empty keys are are backslash encoded. Note that usually + * \\00 is not a valid encode, the only exception is the empty + * word. */ + return "\\00"; + } + + /* find the first character that needs escaping. */ + i = 0; + if (name[0] != ' ') { + for (;; i++) { + const guchar ch = (guchar) name[i]; + + if (ch == '\0') + return name; + + if ( ch < 0x20 + || ch >= 127 + || NM_IN_SET (ch, '=', '[', ']') + || ( ch == '\\' + && g_ascii_isxdigit (name[i + 1]) + && g_ascii_isxdigit (name[i + 2])) + || ( ch == ' ' + && name[i + 1] == '\0')) + break; + } + } else if (name[1] == '\0') + return "\\20"; + + len = i + strlen (&name[i]); + nm_assert (len == strlen (name)); + str = g_string_sized_new (len + 15); + + if (name[0] == ' ') { + nm_assert (i == 0); + g_string_append (str, "\\20"); + i = 1; + } else + g_string_append_len (str, name, i); + + for (;; i++) { + const guchar ch = (guchar) name[i]; + + if (ch == '\0') + break; + + if ( ch < 0x20 + || ch >= 127 + || NM_IN_SET (ch, '=', '[', ']') + || ( ch == '\\' + && g_ascii_isxdigit (name[i + 1]) + && g_ascii_isxdigit (name[i + 2])) + || ( ch == ' ' + && name[i + 1] == '\0')) + g_string_append_printf (str, "\\%2X", ch); + else + g_string_append_c (str, (char) ch); + } + + return (*out_to_free = g_string_free (str, FALSE)); +} + +static const char * +_keyfile_key_decode (const char *key, + char **out_to_free) +{ + gsize i, len; + GString *str; + + nm_assert (key); + nm_assert (out_to_free && !*out_to_free); + + if (!key[0]) + return ""; + + for (i = 0; TRUE; i++) { + const char ch = key[i]; + + if (ch == '\0') + return key; + if ( ch == '\\' + && g_ascii_isxdigit (key[i + 1]) + && g_ascii_isxdigit (key[i + 2])) + break; + } + + len = i + strlen (&key[i]); + + if ( len == 3 + && nm_streq (key, "\\00")) + return ""; + + nm_assert (len == strlen (key)); + str = g_string_sized_new (len + 3); + + g_string_append_len (str, key, i); + for (;;) { + const char ch = key[i]; + char ch1, ch2; + unsigned v; + + if (ch == '\0') + break; + + if ( ch == '\\' + && g_ascii_isxdigit ((ch1 = key[i + 1])) + && g_ascii_isxdigit ((ch2 = key[i + 2]))) { + v = (g_ascii_xdigit_value (ch1) << 4) + g_ascii_xdigit_value (ch2); + if (v != 0) { + g_string_append_c (str, (char) v); + i += 3; + continue; + } + } + g_string_append_c (str, ch); + i++; + } + + return (*out_to_free = g_string_free (str, FALSE)); +} + +/*****************************************************************************/ + +const char * +nm_keyfile_key_encode (const char *name, + char **out_to_free) +{ + const char *key; + + key = _keyfile_key_encode (name, out_to_free); +#if NM_MORE_ASSERTS > 5 + nm_assert (key); + nm_assert (!*out_to_free || key == *out_to_free); + nm_assert (!*out_to_free || !nm_streq0 (name, key)); + { + gs_free char *to_free2 = NULL; + const char *name2; + + name2 = _keyfile_key_decode (key, &to_free2); + /* name2, the result of encode()+decode() is identical to name. + * That is because + * - encode() is a injective function. + * - decode() is a surjective function, however for output + * values of encode() is behaves injective too. */ + nm_assert (nm_streq0 (name2, name)); + } +#endif + return key; +} + +const char * +nm_keyfile_key_decode (const char *key, + char **out_to_free) +{ + const char *name; + + name = _keyfile_key_decode (key, out_to_free); +#if NM_MORE_ASSERTS > 5 + nm_assert (name); + nm_assert (!*out_to_free || name == *out_to_free); + { + gs_free char *to_free2 = NULL; + const char *key2; + + key2 = _keyfile_key_encode (name, &to_free2); + /* key2, the result of decode+encode may not be idential + * to the original key. That is, decode() is a surjective + * function mapping different keys to the same name. + * However, decode() behaves injective for input that + * are valid output of encode(). */ + nm_assert (key2); + } +#endif + return name; +} diff --git a/libnm-core/nm-keyfile-utils.h b/libnm-core/nm-keyfile-utils.h index 89fd3041a0..1f63af8c2c 100644 --- a/libnm-core/nm-keyfile-utils.h +++ b/libnm-core/nm-keyfile-utils.h @@ -81,5 +81,11 @@ gboolean nm_keyfile_plugin_kf_has_key (GKeyFile *kf, const char *key, GError **error); +const char *nm_keyfile_key_encode (const char *name, + char **out_to_free); + +const char *nm_keyfile_key_decode (const char *key, + char **out_to_free); + #endif /* __NM_KEYFILE_UTILS_H__ */ diff --git a/libnm-core/nm-keyfile-writer.c b/libnm-core/nm-keyfile-writer.c index 13a29ccdcc..6a3d9a9f46 100644 --- a/libnm-core/nm-keyfile-writer.c +++ b/libnm-core/nm-keyfile-writer.c @@ -300,8 +300,12 @@ write_hash_of_string (GKeyFile *file, } if (write_item) { + gs_free char *to_free = NULL; + data = g_hash_table_lookup (hash, property); - nm_keyfile_plugin_kf_set_string (file, group_name, property, data); + nm_keyfile_plugin_kf_set_string (file, group_name, + nm_keyfile_key_encode (property, &to_free), + data); } } } diff --git a/libnm-core/tests/test-keyfile.c b/libnm-core/tests/test-keyfile.c index 9944eb5ef8..f7525317a9 100644 --- a/libnm-core/tests/test-keyfile.c +++ b/libnm-core/tests/test-keyfile.c @@ -36,6 +36,69 @@ #define TEST_WIRED_TLS_CA_CERT TEST_CERT_DIR"/test-ca-cert.pem" #define TEST_WIRED_TLS_PRIVKEY TEST_CERT_DIR"/test-key-and-cert.pem" +/*****************************************************************************/ + +static void +do_test_encode_key_full (GKeyFile *kf, const char *name, const char *key, const char *key_decode_encode) +{ + gs_free char *to_free1 = NULL; + gs_free char *to_free2 = NULL; + const char *key2; + const char *name2; + + g_assert (key); + + if (name) { + key2 = nm_keyfile_key_encode (name, &to_free1); + g_assert (key2); + g_assert (NM_STRCHAR_ALL (key2, ch, (guchar) ch < 127)); + g_assert_cmpstr (key2, ==, key); + + /* try to add the encoded key to the keyfile. We expect + * no g_critical warning about invalid key. */ + g_key_file_set_value (kf, "group", key, "dummy"); + } + + name2 = nm_keyfile_key_decode (key, &to_free2); + if (name) + g_assert_cmpstr (name2, ==, name); + else { + key2 = nm_keyfile_key_encode (name2, &to_free1); + g_assert (key2); + g_assert (NM_STRCHAR_ALL (key2, ch, (guchar) ch < 127)); + if (key_decode_encode) + g_assert_cmpstr (key2, ==, key_decode_encode); + g_key_file_set_value (kf, "group", key2, "dummy"); + } +} + +#define do_test_encode_key_bijection(kf, name, key) do_test_encode_key_full (kf, ""name, ""key, NULL) +#define do_test_encode_key_identity(kf, name) do_test_encode_key_full (kf, ""name, ""name, NULL) +#define do_test_encode_key_decode_surjection(kf, key, key_decode_encode) do_test_encode_key_full (kf, NULL, ""key, ""key_decode_encode) + +static void +test_encode_key (void) +{ + gs_unref_keyfile GKeyFile *kf = g_key_file_new (); + + do_test_encode_key_identity (kf, "a"); + do_test_encode_key_bijection (kf, "", "\\00"); + do_test_encode_key_bijection (kf, " ", "\\20"); + do_test_encode_key_bijection (kf, "\\ ", "\\\\20"); + do_test_encode_key_identity (kf, "\\0"); + do_test_encode_key_identity (kf, "\\a"); + do_test_encode_key_identity (kf, "\\0g"); + do_test_encode_key_bijection (kf, "\\0f", "\\5C0f"); + do_test_encode_key_bijection (kf, "\\0f ", "\\5C0f\\20"); + do_test_encode_key_bijection (kf, " \\0f ", "\\20\\5C0f\\20"); + do_test_encode_key_bijection (kf, "\xF5", "\\F5"); + do_test_encode_key_bijection (kf, "\x7F", "\\7F"); + do_test_encode_key_bijection (kf, "\x1f", "\\1F"); + do_test_encode_key_bijection (kf, " ", "\\20\\20"); + do_test_encode_key_bijection (kf, " ", "\\20 \\20"); + do_test_encode_key_decode_surjection (kf, "f\\20c", "f c"); + do_test_encode_key_decode_surjection (kf, "\\20\\20\\20", "\\20 \\20"); +} /*****************************************************************************/ @@ -589,6 +652,7 @@ int main (int argc, char **argv) { nmtst_init (&argc, &argv, TRUE); + g_test_add_func ("/core/keyfile/encode_key", test_encode_key); g_test_add_func ("/core/keyfile/test_8021x_cert", test_8021x_cert); g_test_add_func ("/core/keyfile/test_8021x_cert_read", test_8021x_cert_read); g_test_add_func ("/core/keyfile/test_team_conf_read/valid", test_team_conf_read_valid); |