From e7b32fb2b89e242d8da032c28ca4e134b2b5d965 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 4 May 2017 14:42:00 +0200 Subject: [PATCH 1/7] keyfile: fix memleak in read_hash_of_string() Fixes: 10661abe174862c71603cb385e20fee5a6671997 (cherry picked from commit cb33e3f3c2fa606f88415f13bec5cf8f79d3ef0c) --- libnm-core/nm-keyfile-reader.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c index 080988f08d..8e4a2ee5fd 100644 --- a/libnm-core/nm-keyfile-reader.c +++ b/libnm-core/nm-keyfile-reader.c @@ -715,7 +715,8 @@ mac_address_parser_INFINIBAND (KeyfileReaderInfo *info, NMSetting *setting, cons static void read_hash_of_string (GKeyFile *file, NMSetting *setting, const char *key) { - char **keys, **iter; + gs_strfreev char **keys = NULL; + const char *const*iter; char *value; const char *setting_name = nm_setting_get_name (setting); @@ -723,7 +724,7 @@ read_hash_of_string (GKeyFile *file, NMSetting *setting, const char *key) if (!keys || !*keys) return; - for (iter = keys; *iter; iter++) { + for (iter = (const char *const*) keys; *iter; iter++) { value = nm_keyfile_plugin_kf_get_string (file, setting_name, *iter, NULL); if (!value) continue; @@ -739,7 +740,6 @@ read_hash_of_string (GKeyFile *file, NMSetting *setting, const char *key) } g_free (value); } - g_strfreev (keys); } static gsize From 2ab5537b2090ebf6a873bb3ee2fc33c11774199b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 6 May 2017 13:33:21 +0200 Subject: [PATCH 2/7] keyfile: minor refactoring dropping temporary variable in mac_address_parser() (cherry picked from commit 095c6f5d534c7f072f9cba57d5ad232dbbfd4fe1) --- libnm-core/nm-keyfile-reader.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c index 8e4a2ee5fd..293f03b629 100644 --- a/libnm-core/nm-keyfile-reader.c +++ b/libnm-core/nm-keyfile-reader.c @@ -668,7 +668,6 @@ mac_address_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key buf_arr = g_new (guint8, buf_len); for (i = 0; i < length; i++) { int val = tmp_list[i]; - const guint8 v = (guint8) (val & 0xFF); if (val < 0 || val > 255) { handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN, @@ -676,7 +675,7 @@ mac_address_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key val); return; } - buf_arr[i] = v; + buf_arr[i] = (guint8) val; } } } From f38878c997bee8ee48eb94582b768370aa5e6e47 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 5 May 2017 12:01:17 +0200 Subject: [PATCH 3/7] 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". (cherry picked from commit 8ef57d0f7ef26247bf53b4d07f712a1bf4de4bb2) --- libnm-core/nm-keyfile-reader.c | 13 +- libnm-core/nm-keyfile-utils.c | 202 ++++++++++++++++++++++++++++++++ libnm-core/nm-keyfile-utils.h | 6 + libnm-core/nm-keyfile-writer.c | 6 +- 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); From c429951c4600cf89c71a671d4bff2868d2cfe528 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 4 May 2017 14:44:18 +0200 Subject: [PATCH 4/7] libnm: track invalid user data separately and reject during verify() nm_setting_user_set_data() rejects invalid keys and values, and can fail. This API is correct never to fail, like the get_data() only returns valid user-data. However, the g_object_set() API allows to set the hash directly but it cannot report errors for invalid values. This API is used to initialize the value from D-Bus or keyfile, hence it is wrong to emit g_critial() assertions for untrusted data. It would also be wrong to silently drop all invalid date, because then the user cannot get an error message to understand what happend. The correct but cumbersome solution is to remember the invalid values separately, so that verify() can report the setting as invalid. (cherry picked from commit 1dbbf6fb0327053ea6cb014acbcf1d1655869e89) --- libnm-core/nm-setting-user.c | 180 +++++++++++++++++++++++++---------- 1 file changed, 131 insertions(+), 49 deletions(-) diff --git a/libnm-core/nm-setting-user.c b/libnm-core/nm-setting-user.c index fd955f36d8..71d738910a 100644 --- a/libnm-core/nm-setting-user.c +++ b/libnm-core/nm-setting-user.c @@ -35,6 +35,8 @@ * arbitrary user data to #NMConnection objects. **/ +#define MAX_NUM_KEYS 256 + /*****************************************************************************/ NM_GOBJECT_PROPERTIES_DEFINE (NMSettingUser, @@ -43,6 +45,7 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMSettingUser, typedef struct { GHashTable *data; + GHashTable *data_invalid; const char **keys; } NMSettingUserPrivate; @@ -282,7 +285,8 @@ nm_setting_user_get_data (NMSettingUser *setting, const char *key) * nm_setting_user_set_data: * @setting: the #NMSettingUser instance * @key: the key to set - * @val: the value to set or %NULL to clear a key. + * @val: (allow-none): the value to set or %NULL to clear a key. + * @error: (allow-none): optional error argument * * Since: 1.8 * @@ -298,6 +302,7 @@ nm_setting_user_set_data (NMSettingUser *setting, { NMSettingUser *self = setting; NMSettingUserPrivate *priv; + gboolean changed = FALSE; g_return_val_if_fail (NM_IS_SETTING (self), FALSE); g_return_val_if_fail (!error || !*error, FALSE); @@ -305,41 +310,52 @@ nm_setting_user_set_data (NMSettingUser *setting, if (!nm_setting_user_check_key (key, error)) return FALSE; + if ( val + && !nm_setting_user_check_val (val, error)) + return FALSE; + priv = NM_SETTING_USER_GET_PRIVATE (self); if (!val) { if ( priv->data && g_hash_table_remove (priv->data, key)) { nm_clear_g_free (&priv->keys); - _notify (self, PROP_DATA); + changed = TRUE; } - return TRUE; + goto out; } - if (!nm_setting_user_check_val (val, error)) - return FALSE; - - if (!priv->data) - priv->data = _create_data_hash (); - else { + if (priv->data) { const char *key2, *val2; - if (g_hash_table_size (priv->data) > 256) { - /* limit the number of valid keys */ - g_set_error_literal (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY, - _("maximum number of user data entries reached")); - return FALSE; - } - if (g_hash_table_lookup_extended (priv->data, key, (gpointer *) &key2, (gpointer *) &val2)) { if (nm_streq (val, val2)) - return TRUE; - } else + goto out; + } else { + if (g_hash_table_size (priv->data) >= MAX_NUM_KEYS) { + /* limit the number of valid keys */ + g_set_error_literal (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("maximum number of user data entries reached")); + return FALSE; + } + nm_clear_g_free (&priv->keys); - } + } + } else + priv->data = _create_data_hash (); g_hash_table_insert (priv->data, g_strdup (key), g_strdup (val)); - _notify (self, PROP_DATA); + changed = TRUE; + +out: + if (priv->data_invalid) { + /* setting a value purges all invalid values that were set + * via GObject property. */ + changed = TRUE; + g_clear_pointer (&priv->data_invalid, g_hash_table_unref); + } + if (changed) + _notify (self, PROP_DATA); return TRUE; } @@ -348,11 +364,68 @@ nm_setting_user_set_data (NMSettingUser *setting, static gboolean verify (NMSetting *setting, NMConnection *connection, GError **error) { - g_set_error_literal (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_FAILED, - _("setting user-data is not yet implemented")); - return FALSE; + NMSettingUser *self = NM_SETTING_USER (setting); + NMSettingUserPrivate *priv = NM_SETTING_USER_GET_PRIVATE (self); + + if (priv->data_invalid) { + const char *key, *val; + GHashTableIter iter; + gs_free_error GError *local = NULL; + + g_hash_table_iter_init (&iter, priv->data_invalid); + while (g_hash_table_iter_next (&iter, (gpointer *) &key, (gpointer *) &val)) { + if (!nm_setting_user_check_key (key, &local)) { + g_set_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_FAILED, + _("invalid key \"%s\": %s"), + key, local->message); + } else if (!nm_setting_user_check_val (val, &local)) { + g_set_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_FAILED, + _("invalid value for \"%s\": %s"), + key, local->message); + } else { + nm_assert_not_reached (); + continue; + } + g_prefix_error (error, "%s.%s: ", NM_SETTING_USER_SETTING_NAME, NM_SETTING_USER_DATA); + return FALSE; + } + nm_assert_not_reached (); + } + + if ( priv->data + && g_hash_table_size (priv->data) > MAX_NUM_KEYS) { + g_set_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("maximum number of user data entries reached (%u instead of %u)"), + g_hash_table_size (priv->data), (unsigned) MAX_NUM_KEYS); + g_prefix_error (error, "%s.%s: ", NM_SETTING_USER_SETTING_NAME, NM_SETTING_USER_DATA); + return FALSE; + } + + return TRUE; } +static gboolean +hash_table_equal (GHashTable *a, GHashTable *b) +{ + guint n; + GHashTableIter iter; + const char *key, *value, *valu2; + + n = a ? g_hash_table_size (a) : 0; + if (n != (b ? g_hash_table_size (b) : 0)) + return FALSE; + if (n > 0) { + g_hash_table_iter_init (&iter, a); + while (g_hash_table_iter_next (&iter, (gpointer *) &key, (gpointer *) &value)) { + if (!g_hash_table_lookup_extended (b, key, NULL, (gpointer *) &valu2)) + return FALSE; + if (!nm_streq (value, valu2)) + return FALSE; + } + } + return TRUE; + +} static gboolean compare_property (NMSetting *setting, @@ -361,9 +434,6 @@ compare_property (NMSetting *setting, NMSettingCompareFlags flags) { NMSettingUserPrivate *priv, *pri2; - guint n; - GHashTableIter iter; - const char *key, *value, *valu2; g_return_val_if_fail (NM_IS_SETTING_USER (setting), FALSE); g_return_val_if_fail (NM_IS_SETTING_USER (other), FALSE); @@ -374,18 +444,12 @@ compare_property (NMSetting *setting, priv = NM_SETTING_USER_GET_PRIVATE (NM_SETTING_USER (setting)); pri2 = NM_SETTING_USER_GET_PRIVATE (NM_SETTING_USER (other)); - n = priv->data ? g_hash_table_size (priv->data) : 0; - if (n != (pri2->data ? g_hash_table_size (pri2->data) : 0)) + if (!hash_table_equal (priv->data, pri2->data)) return FALSE; - if (n > 0) { - g_hash_table_iter_init (&iter, priv->data); - while (g_hash_table_iter_next (&iter, (gpointer *) &key, (gpointer *) &value)) { - if (!g_hash_table_lookup_extended (pri2->data, key, NULL, (gpointer *) &valu2)) - return FALSE; - if (!nm_streq (value, valu2)) - return FALSE; - } - } + + if (!hash_table_equal (priv->data_invalid, pri2->data_invalid)) + return FALSE; + return TRUE; call_parent: @@ -412,6 +476,11 @@ get_property (GObject *object, guint prop_id, while (g_hash_table_iter_next (&iter, (gpointer *) &key, (gpointer *) &val)) g_hash_table_insert (data, g_strdup (key), g_strdup (val)); } + if (priv->data_invalid) { + g_hash_table_iter_init (&iter, priv->data_invalid); + while (g_hash_table_iter_next (&iter, (gpointer *) &key, (gpointer *) &val)) + g_hash_table_insert (data, g_strdup (key), g_strdup (val)); + } g_value_take_boxed (value, data); break; default: @@ -432,27 +501,38 @@ set_property (GObject *object, guint prop_id, switch (prop_id) { case PROP_DATA: + nm_clear_g_free (&priv->keys); + data = g_value_get_boxed (value); if (!data || !g_hash_table_size (data)) { g_clear_pointer (&priv->data, g_hash_table_unref); - nm_clear_g_free (&priv->keys); + g_clear_pointer (&priv->data_invalid, g_hash_table_unref); return; } - g_hash_table_iter_init (&iter, priv->data); - while (g_hash_table_iter_next (&iter, (gpointer *) &key, (gpointer *) &val)) { - if (!nm_setting_user_check_key (key, NULL)) - g_return_if_reached (); - if (!nm_setting_user_check_val (val, NULL)) - g_return_if_reached (); - } - nm_clear_g_free (&priv->keys); + if (priv->data) g_hash_table_remove_all (priv->data); else priv->data = _create_data_hash (); - g_hash_table_iter_init (&iter, priv->data); - while (g_hash_table_iter_next (&iter, (gpointer *) &key, (gpointer *) &val)) - g_hash_table_insert (priv->data, (gpointer) key, (gpointer) val); + + if (priv->data_invalid) + g_hash_table_remove_all (priv->data_invalid); + + g_hash_table_iter_init (&iter, data); + while (g_hash_table_iter_next (&iter, (gpointer *) &key, (gpointer *) &val)) { + if ( nm_setting_user_check_key (key, NULL) + && nm_setting_user_check_val (val, NULL)) + g_hash_table_insert (priv->data, g_strdup (key), g_strdup (val)); + else { + if (!priv->data_invalid) + priv->data_invalid = _create_data_hash (); + g_hash_table_insert (priv->data_invalid, g_strdup (key), g_strdup (val)); + } + } + if ( priv->data_invalid + && !g_hash_table_size (priv->data_invalid)) + g_clear_pointer (&priv->data_invalid, g_hash_table_unref); + break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); @@ -488,6 +568,8 @@ finalize (GObject *object) g_free (priv->keys); if (priv->data) g_hash_table_unref (priv->data); + if (priv->data_invalid) + g_hash_table_unref (priv->data_invalid); G_OBJECT_CLASS (nm_setting_user_parent_class)->finalize (object); } From a9ee1dcd5cf4e937e5f6b3182783f7e1b5e03762 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 4 May 2017 14:50:07 +0200 Subject: [PATCH 5/7] libnm/keyfile: properly read user data from keyfile Hack keyfile reader support for NMSettingUser. Writer support already works. (cherry picked from commit 22fd7d2e39327c9f288dd8f17529c1898bc20e47) --- libnm-core/nm-keyfile-reader.c | 60 ++++++++++++++++++++-------- libnm-core/tests/test-keyfile.c | 69 +++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 16 deletions(-) diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c index 078c517e8e..eb257eeb1a 100644 --- a/libnm-core/nm-keyfile-reader.c +++ b/libnm-core/nm-keyfile-reader.c @@ -35,6 +35,8 @@ #include "nm-core-internal.h" #include "nm-keyfile-utils.h" +#include "nm-setting-user.h" + typedef struct { NMConnection *connection; GKeyFile *keyfile; @@ -716,33 +718,56 @@ read_hash_of_string (GKeyFile *file, NMSetting *setting, const char *key) { gs_strfreev char **keys = NULL; const char *const*iter; - char *value; const char *setting_name = nm_setting_get_name (setting); + gboolean is_vpn; keys = nm_keyfile_plugin_kf_get_keys (file, setting_name, NULL, NULL); if (!keys || !*keys) return; - for (iter = (const char *const*) keys; *iter; iter++) { - gs_free char *to_free = NULL; - const char *name; + if ( (is_vpn = NM_IS_SETTING_VPN (setting)) + || NM_IS_SETTING_BOND (setting)) { + for (iter = (const char *const*) keys; *iter; iter++) { + gs_free char *to_free = NULL; + gs_free char *value = NULL; + const char *name; - value = nm_keyfile_plugin_kf_get_string (file, setting_name, *iter, NULL); - if (!value) - continue; + value = nm_keyfile_plugin_kf_get_string (file, setting_name, *iter, NULL); + if (!value) + continue; - name = nm_keyfile_key_decode (*iter, &to_free); + 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), name)) - nm_setting_vpn_add_data_item (NM_SETTING_VPN (setting), name, value); + if (is_vpn) { + /* Add any item that's not a class property to the data hash */ + if (!g_object_class_find_property (G_OBJECT_GET_CLASS (setting), name)) + nm_setting_vpn_add_data_item (NM_SETTING_VPN (setting), name, value); + } else { + if (strcmp (name, "interface-name")) + nm_setting_bond_add_option (NM_SETTING_BOND (setting), name, value); + } } - if (NM_IS_SETTING_BOND (setting)) { - if (strcmp (name, "interface-name")) - nm_setting_bond_add_option (NM_SETTING_BOND (setting), name, value); + return; + } + + if (NM_IS_SETTING_USER (setting)) { + gs_unref_hashtable GHashTable *data = NULL; + + data = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free); + for (iter = (const char *const*) keys; *iter; iter++) { + gs_free char *to_free = NULL; + char *value = 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); + g_hash_table_insert (data, + g_steal_pointer (&to_free) ?: g_strdup (name), + value); } - g_free (value); + g_object_set (setting, NM_SETTING_USER_DATA, data, NULL); } } @@ -1460,6 +1485,9 @@ read_one_setting_value (NMSetting *setting, if (NM_IS_SETTING_VPN (setting)) check_for_key = FALSE; + if (NM_IS_SETTING_USER (setting)) + check_for_key = FALSE; + /* Bonding 'options' don't have the exact key name. The options are right under [bond] group. */ if (NM_IS_SETTING_BOND (setting)) check_for_key = FALSE; diff --git a/libnm-core/tests/test-keyfile.c b/libnm-core/tests/test-keyfile.c index f7525317a9..2dc929afd8 100644 --- a/libnm-core/tests/test-keyfile.c +++ b/libnm-core/tests/test-keyfile.c @@ -28,6 +28,7 @@ #include "nm-setting-wired.h" #include "nm-setting-8021x.h" #include "nm-setting-team.h" +#include "nm-setting-user.h" #include "nm-setting-proxy.h" #include "nm-utils/nm-test-utils.h" @@ -646,6 +647,73 @@ test_team_conf_read_invalid (void) /*****************************************************************************/ +static void +test_user_1 (void) +{ + gs_unref_keyfile GKeyFile *keyfile = NULL; + gs_unref_object NMConnection *con = NULL; + NMSettingUser *s_user; + + con = nmtst_create_connection_from_keyfile ( + "[connection]\n" + "id=t\n" + "type=ethernet\n" + "\n" + "[user]\n" + "my-value.x=value1\n" + "", + "/test_user_1/invalid", NULL); + g_assert (con); + s_user = NM_SETTING_USER (nm_connection_get_setting (con, NM_TYPE_SETTING_USER)); + g_assert (s_user); + g_assert_cmpstr (nm_setting_user_get_data (s_user, "my-value.x"), ==, "value1"); + + CLEAR (&con, &keyfile); + + con = nmtst_create_minimal_connection ("user-2", "8b85fb8d-3070-48ba-93d9-53eee231d9a2", NM_SETTING_WIRED_SETTING_NAME, NULL); + s_user = NM_SETTING_USER (nm_setting_user_new ()); + +#define _USER_SET_DATA(s_user, key, val) \ + G_STMT_START { \ + GError *_error = NULL; \ + gboolean _success; \ + \ + _success = nm_setting_user_set_data ((s_user), (key), (val), &_error); \ + nmtst_assert_success (_success, _error); \ + } G_STMT_END + +#define _USER_SET_DATA_X(s_user, key) \ + _USER_SET_DATA (s_user, key, "val="key"") + + _USER_SET_DATA (s_user, "my.val1", ""); + _USER_SET_DATA_X (s_user, "my.val2"); + _USER_SET_DATA_X (s_user, "my.v__al3"); + _USER_SET_DATA_X (s_user, "my._v"); + _USER_SET_DATA_X (s_user, "my.v+"); + _USER_SET_DATA_X (s_user, "my.Av"); + _USER_SET_DATA_X (s_user, "MY.AV"); + _USER_SET_DATA_X (s_user, "MY.8V"); + _USER_SET_DATA_X (s_user, "MY.8-V"); + _USER_SET_DATA_X (s_user, "MY.8_V"); + _USER_SET_DATA_X (s_user, "MY.8+V"); + _USER_SET_DATA_X (s_user, "MY.8/V"); + _USER_SET_DATA_X (s_user, "MY.8=V"); + _USER_SET_DATA_X (s_user, "MY.-"); + _USER_SET_DATA_X (s_user, "MY._"); + _USER_SET_DATA_X (s_user, "MY.+"); + _USER_SET_DATA_X (s_user, "MY./"); + _USER_SET_DATA_X (s_user, "MY.="); + _USER_SET_DATA_X (s_user, "my.keys.1"); + _USER_SET_DATA_X (s_user, "my.other.KEY.42"); + + nm_connection_add_setting (con, NM_SETTING (s_user)); + nmtst_connection_normalize (con); + + _keyfile_convert (&con, &keyfile, NULL, NULL, NULL, NULL, NULL, NULL, FALSE); +} + +/*****************************************************************************/ + NMTST_DEFINE (); int main (int argc, char **argv) @@ -657,6 +725,7 @@ int main (int argc, char **argv) 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); g_test_add_func ("/core/keyfile/test_team_conf_read/invalid", test_team_conf_read_invalid); + g_test_add_func ("/core/keyfile/test_user/1", test_user_1); return g_test_run (); } From ead512e6c8dfbcc532f5b902a5a10ccd92371dfc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 4 May 2017 16:26:02 +0200 Subject: [PATCH 6/7] ifcfg: add read/write support for user-data The user data values are encoded in shell variables named prefix "NM_USER_". The variable name is an encoded form of the data key, consisting only of upper-case letters, digits, and underscore. The alternative would be something like NM_USER_1_KEY=my.keys.1 NM_USER_1_VAL='some value' NM_USER_2_KEY=my.other.KEY.42 NM_USER_2_VAL='other value' contary to NM_USER_MY__KEYS__1='some value' NM_USER_MY__OTHER___K_E_Y__42='other value' The advantage of the former, numbered scheme is that it may be easier to find the key of a user-data entry. With the current implementation, the shell script would have to decode the key, like the ifcfg-rh plugin does. However, user data keys are opaque identifers for values. Usually, you are not concerned with a certain name of the key, you already know it. Hence, you don't need to write a shell script to decode the key name, instead, you can use it directly: if [ -z ${NM_USER_MY__OTHER___K_E_Y__42+x} ]; then do_something_with_key "$NM_USER_MY__OTHER___K_E_Y__42" fi Otherwise, you'd first have to search write a shell script to search for the interesting key -- in this example "$NM_USER_2_KEY", before being able to access the value "$NM_USER_2_VAL". (cherry picked from commit 79be44d99020bbaf23b8226b7e459f321348c0b1) --- Makefile.am | 3 +- .../plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 55 ++++++++- .../plugins/ifcfg-rh/nms-ifcfg-rh-utils.c | 114 ++++++++++++++++++ .../plugins/ifcfg-rh/nms-ifcfg-rh-utils.h | 4 +- .../plugins/ifcfg-rh/nms-ifcfg-rh-writer.c | 37 ++++++ src/settings/plugins/ifcfg-rh/shvar.c | 51 +++++++- src/settings/plugins/ifcfg-rh/shvar.h | 4 + .../ifcfg-Test_User_1.cexpected | 34 ++++++ .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 59 +++++++++ 9 files changed, 356 insertions(+), 5 deletions(-) create mode 100644 src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-Test_User_1.cexpected diff --git a/Makefile.am b/Makefile.am index 8b90d50c21..709d79b701 100644 --- a/Makefile.am +++ b/Makefile.am @@ -2154,7 +2154,8 @@ EXTRA_DIST += \ src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-random_wifi_connection.cexpected \ src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-random_wifi_connection_2.cexpected \ src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-team-slave-enp31s0f1-142.cexpected \ - src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-static-routes-legacy.cexpected + src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-static-routes-legacy.cexpected \ + src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-Test_User_1.cexpected # make target dependencies can't have colons in their names, which ends up # meaning that we can't add the alias files to EXTRA_DIST. They are instead diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c index ee96feea89..164f684453 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -48,6 +48,7 @@ #include "nm-setting-bridge.h" #include "nm-setting-bridge-port.h" #include "nm-setting-dcb.h" +#include "nm-setting-user.h" #include "nm-setting-proxy.h" #include "nm-setting-generic.h" #include "nm-core-internal.h" @@ -1023,6 +1024,54 @@ error: return success; } +static NMSetting * +make_user_setting (shvarFile *ifcfg, GError **error) +{ + gboolean has_user_data = FALSE; + gs_unref_object NMSettingUser *s_user = NULL; + gs_unref_hashtable GHashTable *keys = NULL; + GHashTableIter iter; + const char *key; + nm_auto_free_gstring GString *str = NULL; + + keys = svGetKeys (ifcfg); + if (!keys) + return NULL; + + g_hash_table_iter_init (&iter, keys); + while (g_hash_table_iter_next (&iter, (gpointer *) &key, NULL)) { + const char *value; + gs_free char *value_to_free = NULL; + + if (!g_str_has_prefix (key, "NM_USER_")) + continue; + + value = svGetValue (ifcfg, key, &value_to_free); + + if (!value) + continue; + + if (!str) + str = g_string_sized_new (100); + else + g_string_set_size (str, 0); + + if (!nms_ifcfg_rh_utils_user_key_decode (key + NM_STRLEN ("NM_USER_"), str)) + continue; + + if (!s_user) + s_user = NM_SETTING_USER (nm_setting_user_new ()); + + if (nm_setting_user_set_data (s_user, str->str, + value, NULL)) + has_user_data = TRUE; + } + + return has_user_data + ? g_steal_pointer (&s_user) + : NULL; +} + static NMSetting * make_proxy_setting (shvarFile *ifcfg, GError **error) { @@ -5183,7 +5232,7 @@ connection_from_file_full (const char *filename, gs_unref_object NMConnection *connection = NULL; gs_free char *type = NULL; char *devtype, *bootproto; - NMSetting *s_ip4, *s_ip6, *s_proxy, *s_port, *s_dcb = NULL; + NMSetting *s_ip4, *s_ip6, *s_proxy, *s_port, *s_dcb = NULL, *s_user; const char *ifcfg_name = NULL; gboolean has_ip4_defroute = FALSE; @@ -5423,6 +5472,10 @@ connection_from_file_full (const char *filename, if (s_proxy) nm_connection_add_setting (connection, s_proxy); + s_user = make_user_setting (parsed, error); + if (s_user) + nm_connection_add_setting (connection, s_user); + /* Bridge port? */ s_port = make_bridge_port_setting (parsed); if (s_port) diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.c index 0dd49fb4a7..e82ef60c63 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.c @@ -365,3 +365,117 @@ utils_detect_ifcfg_path (const char *path, gboolean only_ifcfg) return NULL; return utils_get_ifcfg_path (path); } + +void +nms_ifcfg_rh_utils_user_key_encode (const char *key, GString *str_buffer) +{ + gsize i; + + nm_assert (key); + nm_assert (str_buffer); + + for (i = 0; key[i]; i++) { + char ch = key[i]; + + /* we encode the key in only upper case letters, digits, and underscore. + * As we expect lower-case letters to be more common, we encode lower-case + * letters as upper case, and upper-case letters with a leading underscore. */ + + if (ch >= '0' && ch <= '9') { + g_string_append_c (str_buffer, ch); + continue; + } + if (ch >= 'a' && ch <= 'z') { + g_string_append_c (str_buffer, ch - 'a' + 'A'); + continue; + } + if (ch == '.') { + g_string_append (str_buffer, "__"); + continue; + } + if (ch >= 'A' && ch <= 'Z') { + g_string_append_c (str_buffer, '_'); + g_string_append_c (str_buffer, ch); + continue; + } + g_string_append_printf (str_buffer, "_%03o", (unsigned) ch); + } +} + +gboolean +nms_ifcfg_rh_utils_user_key_decode (const char *name, GString *str_buffer) +{ + gsize i; + + nm_assert (name); + nm_assert (str_buffer); + + if (!name[0]) + return FALSE; + + for (i = 0; name[i]; ) { + char ch = name[i]; + + if (ch >= '0' && ch <= '9') { + g_string_append_c (str_buffer, ch); + i++; + continue; + } + if (ch >= 'A' && ch <= 'Z') { + g_string_append_c (str_buffer, ch - 'A' + 'a'); + i++; + continue; + } + + if (ch == '_') { + ch = name[i + 1]; + if (ch == '_') { + g_string_append_c (str_buffer, '.'); + i += 2; + continue; + } + if (ch >= 'A' && ch <= 'Z') { + g_string_append_c (str_buffer, ch); + i += 2; + continue; + } + if (ch >= '0' && ch <= '7') { + char ch2, ch3; + unsigned v; + + ch2 = name[i + 2]; + if (!(ch2 >= '0' && ch2 <= '7')) + return FALSE; + + ch3 = name[i + 3]; + if (!(ch3 >= '0' && ch3 <= '7')) + return FALSE; + +#define OCTAL_VALUE(ch) ((unsigned) ((ch) - '0')) + v = (OCTAL_VALUE (ch) << 6) + + (OCTAL_VALUE (ch2) << 3) + + OCTAL_VALUE (ch3); + if ( v > 0xFF + || v == 0) + return FALSE; + ch = (char) v; + if ( (ch >= 'A' && ch <= 'Z') + || (ch >= '0' && ch <= '9') + || (ch == '.') + || (ch >= 'a' && ch <= 'z')) { + /* such characters are not expected to be encoded via + * octal representation. The encoding is invalid. */ + return FALSE; + } + g_string_append_c (str_buffer, ch); + i += 4; + continue; + } + return FALSE; + } + + return FALSE; + } + + return TRUE; +} diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.h b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.h index d209a0673c..2b2c27558f 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.h +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-utils.h @@ -54,5 +54,7 @@ gboolean utils_is_ifcfg_alias_file (const char *alias, const char *ifcfg); char *utils_detect_ifcfg_path (const char *path, gboolean only_ifcfg); -#endif /* _UTILS_H_ */ +void nms_ifcfg_rh_utils_user_key_encode (const char *key, GString *str_buffer); +gboolean nms_ifcfg_rh_utils_user_key_decode (const char *name, GString *str_buffer); +#endif /* _UTILS_H_ */ diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c index 7959f45c4c..d6f33c4953 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c @@ -42,6 +42,7 @@ #include "nm-setting-ip6-config.h" #include "nm-setting-pppoe.h" #include "nm-setting-vlan.h" +#include "nm-setting-user.h" #include "nm-setting-team.h" #include "nm-setting-team-port.h" #include "nm-utils.h" @@ -2021,6 +2022,39 @@ write_proxy_setting (NMConnection *connection, shvarFile *ifcfg, GError **error) return TRUE; } +static gboolean +write_user_setting (NMConnection *connection, shvarFile *ifcfg, GError **error) +{ + NMSettingUser *s_user; + guint i, len; + const char *const*keys; + + s_user = NM_SETTING_USER (nm_connection_get_setting (connection, NM_TYPE_SETTING_USER)); + + svUnsetValuesWithPrefix (ifcfg, "NM_USER_"); + + if (!s_user) + return TRUE; + + keys = nm_setting_user_get_keys (s_user, &len); + if (len) { + nm_auto_free_gstring GString *str = g_string_sized_new (100); + + for (i = 0; i < len; i++) { + const char *key = keys[i]; + + g_string_set_size (str, 0); + g_string_append (str, "NM_USER_"); + nms_ifcfg_rh_utils_user_key_encode (key, str); + svSetValue (ifcfg, + str->str, + nm_setting_user_get_data (s_user, key)); + } + } + + return TRUE; +} + static gboolean write_ip4_setting (NMConnection *connection, shvarFile *ifcfg, GError **error) { @@ -2910,6 +2944,9 @@ write_connection (NMConnection *connection, if (!write_proxy_setting (connection, ifcfg, error)) return FALSE; + if (!write_user_setting (connection, ifcfg, error)) + return FALSE; + svUnsetValue (ifcfg, "DHCP_HOSTNAME"); svUnsetValue (ifcfg, "DHCP_FQDN"); diff --git a/src/settings/plugins/ifcfg-rh/shvar.c b/src/settings/plugins/ifcfg-rh/shvar.c index 25a012cf3c..9fce5aa117 100644 --- a/src/settings/plugins/ifcfg-rh/shvar.c +++ b/src/settings/plugins/ifcfg-rh/shvar.c @@ -41,7 +41,7 @@ /*****************************************************************************/ -typedef struct { +struct _shvarLine { /* There are three cases: * * 1) the line is not a valid variable assignment (that is, it doesn't @@ -61,7 +61,9 @@ typedef struct { char *line; const char *key; char *key_with_prefix; -} shvarLine; +}; + +typedef struct _shvarLine shvarLine; struct _shvarFile { char *fileName; @@ -879,6 +881,30 @@ shlist_find (const GList *current, const char *key) /*****************************************************************************/ +GHashTable * +svGetKeys (shvarFile *s) +{ + GHashTable *keys = NULL; + const GList *current; + const shvarLine *line; + + nm_assert (s); + + for (current = s->lineList; current; current = current->next) { + line = current->data; + if (line->key && line->line) { + /* we don't clone the keys. The keys are only valid + * until @s gets modified. */ + if (!keys) + keys = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, NULL); + g_hash_table_add (keys, (gpointer) line->key); + } + } + return keys; +} + +/*****************************************************************************/ + static const char * _svGetValue (shvarFile *s, const char *key, char **to_free) { @@ -1133,6 +1159,27 @@ svUnsetValue (shvarFile *s, const char *key) svSetValue (s, key, NULL); } +void +svUnsetValuesWithPrefix (shvarFile *s, const char *prefix) +{ + GList *current; + + g_return_if_fail (s); + g_return_if_fail (prefix); + + for (current = s->lineList; current; current = current->next) { + shvarLine *line = current->data; + + ASSERT_shvarLine (line); + if ( line->key + && g_str_has_prefix (line->key, prefix)) { + if (nm_clear_g_free (&line->line)) + s->modified = TRUE; + } + ASSERT_shvarLine (line); + } +} + /*****************************************************************************/ /* Write the current contents iff modified. Returns FALSE on error diff --git a/src/settings/plugins/ifcfg-rh/shvar.h b/src/settings/plugins/ifcfg-rh/shvar.h index 8eaf9b878e..9d8c2364a3 100644 --- a/src/settings/plugins/ifcfg-rh/shvar.h +++ b/src/settings/plugins/ifcfg-rh/shvar.h @@ -56,6 +56,8 @@ char *svGetValueStr_cp (shvarFile *s, const char *key); gint svParseBoolean (const char *value, gint def); +GHashTable *svGetKeys (shvarFile *s); + /* return TRUE if resolves to any truth value (e.g. "yes", "y", "true") * return FALSE if resolves to any non-truth value (e.g. "no", "n", "false") * return otherwise @@ -76,6 +78,8 @@ void svSetValueInt64 (shvarFile *s, const char *key, gint64 value); void svUnsetValue (shvarFile *s, const char *key); +void svUnsetValuesWithPrefix (shvarFile *s, const char *prefix); + /* Write the current contents iff modified. Returns FALSE on error * and TRUE on success. Do not write if no values have been modified. * The mode argument is only used if creating the file, not if diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-Test_User_1.cexpected b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-Test_User_1.cexpected new file mode 100644 index 0000000000..a48be78a23 --- /dev/null +++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-Test_User_1.cexpected @@ -0,0 +1,34 @@ +TYPE=Ethernet +PROXY_METHOD=none +BROWSER_ONLY=no +NM_USER__M_Y___053=val=MY.+ +NM_USER__M_Y___055=val=MY.- +NM_USER__M_Y___057=val=MY./ +NM_USER__M_Y__8_053_V=val=MY.8+V +NM_USER__M_Y__8_055_V=val=MY.8-V +NM_USER__M_Y__8_057_V=val=MY.8/V +NM_USER__M_Y__8_075_V=val=MY.8=V +NM_USER__M_Y__8_V=val=MY.8V +NM_USER__M_Y__8_137_V=val=MY.8_V +NM_USER__M_Y___075=val=MY.= +NM_USER__M_Y___A_V=val=MY.AV +NM_USER__M_Y___137=val=MY._ +NM_USER_MY___AV=val=my.Av +NM_USER_MY___137V=val=my._v +NM_USER_MY__KEYS__1=val=my.keys.1 +NM_USER_MY__OTHER___K_E_Y__42=val=my.other.KEY.42 +NM_USER_MY__V_053=val=my.v+ +NM_USER_MY__V_137_137AL3=val=my.v__al3 +NM_USER_MY__VAL1= +NM_USER_MY__VAL2=val=my.val2 +BOOTPROTO=dhcp +DEFROUTE=yes +IPV4_FAILURE_FATAL=no +IPV6INIT=yes +IPV6_AUTOCONF=yes +IPV6_DEFROUTE=yes +IPV6_FAILURE_FATAL=no +IPV6_ADDR_GEN_MODE=stable-privacy +NAME="Test User 1" +UUID=${UUID} +ONBOOT=yes diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c index 057d72d5f6..babb068db5 100644 --- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -33,6 +33,7 @@ #include "nm-utils.h" #include "nm-setting-connection.h" #include "nm-setting-wired.h" +#include "nm-setting-user.h" #include "nm-setting-wireless.h" #include "nm-setting-wireless-security.h" #include "nm-setting-ip4-config.h" @@ -1096,6 +1097,62 @@ test_read_wired_obsolete_gateway_n (void) g_object_unref (connection); } +static void +test_user_1 (void) +{ + nmtst_auto_unlinkfile char *testfile = NULL; + gs_unref_object NMConnection *connection = NULL; + gs_unref_object NMConnection *reread = NULL; + NMSettingUser *s_user; + + connection = nmtst_create_minimal_connection ("Test User 1", NULL, NM_SETTING_WIRED_SETTING_NAME, NULL); + s_user = NM_SETTING_USER (nm_setting_user_new ()); + +#define _USER_SET_DATA(s_user, key, val) \ + G_STMT_START { \ + GError *_error = NULL; \ + gboolean _success; \ + \ + _success = nm_setting_user_set_data ((s_user), (key), (val), &_error); \ + nmtst_assert_success (_success, _error); \ + } G_STMT_END + +#define _USER_SET_DATA_X(s_user, key) \ + _USER_SET_DATA (s_user, key, "val="key"") + + _USER_SET_DATA (s_user, "my.val1", ""); + _USER_SET_DATA_X (s_user, "my.val2"); + _USER_SET_DATA_X (s_user, "my.v__al3"); + _USER_SET_DATA_X (s_user, "my._v"); + _USER_SET_DATA_X (s_user, "my.v+"); + _USER_SET_DATA_X (s_user, "my.Av"); + _USER_SET_DATA_X (s_user, "MY.AV"); + _USER_SET_DATA_X (s_user, "MY.8V"); + _USER_SET_DATA_X (s_user, "MY.8-V"); + _USER_SET_DATA_X (s_user, "MY.8_V"); + _USER_SET_DATA_X (s_user, "MY.8+V"); + _USER_SET_DATA_X (s_user, "MY.8/V"); + _USER_SET_DATA_X (s_user, "MY.8=V"); + _USER_SET_DATA_X (s_user, "MY.-"); + _USER_SET_DATA_X (s_user, "MY._"); + _USER_SET_DATA_X (s_user, "MY.+"); + _USER_SET_DATA_X (s_user, "MY./"); + _USER_SET_DATA_X (s_user, "MY.="); + _USER_SET_DATA_X (s_user, "my.keys.1"); + _USER_SET_DATA_X (s_user, "my.other.KEY.42"); + + nm_connection_add_setting (connection, NM_SETTING (s_user)); + + _writer_new_connec_exp (connection, + TEST_SCRATCH_DIR "/network-scripts/", + TEST_IFCFG_DIR "/network-scripts/ifcfg-Test_User_1.cexpected", + &testfile); + + reread = _connection_from_file (testfile, NULL, TYPE_ETHERNET, NULL); + + nmtst_assert_connection_equals (connection, TRUE, reread, FALSE); +} + static void test_read_wired_never_default (void) { @@ -9310,6 +9367,8 @@ int main (int argc, char **argv) nmtst_add_test_func (TPATH "wired/read/manual/3", test_read_wired_ipv4_manual, TEST_IFCFG_DIR "/network-scripts/ifcfg-test-wired-ipv4-manual-3", "System test-wired-ipv4-manual-3"); nmtst_add_test_func (TPATH "wired/read/manual/4", test_read_wired_ipv4_manual, TEST_IFCFG_DIR "/network-scripts/ifcfg-test-wired-ipv4-manual-4", "System test-wired-ipv4-manual-4"); + g_test_add_func (TPATH "user/1", test_user_1); + g_test_add_func (TPATH "wired/ipv6-manual", test_read_wired_ipv6_manual); nmtst_add_test_func (TPATH "wired-ipv6-only/0", test_read_wired_ipv6_only, TEST_IFCFG_DIR"/network-scripts/ifcfg-test-wired-ipv6-only", "System test-wired-ipv6-only"); From adcbcb15e578adc45ab8b948bd2cdbf62854476f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 4 May 2017 09:58:45 +0200 Subject: [PATCH 7/7] examples: add setting-user-data.py Add an example python script to show and set setting's user-data. This is useful, as nmcli still doesn't support user data. (cherry picked from commit 447c766f526ec8bc4f5aa109a5e261cf060d1042) --- Makefile.examples | 13 +- examples/python/gi/setting-user-data.py | 246 ++++++++++++++++++++++++ 2 files changed, 253 insertions(+), 6 deletions(-) create mode 100755 examples/python/gi/setting-user-data.py diff --git a/Makefile.examples b/Makefile.examples index 172adc17ff..63370e2cbc 100644 --- a/Makefile.examples +++ b/Makefile.examples @@ -163,16 +163,17 @@ EXTRA_DIST += \ examples/python/dbus/create-bond.py \ examples/python/dbus/wifi-active-ap.py\ \ - examples/python/gi/list-connections.py \ + examples/python/gi/README \ + examples/python/gi/add_connection.py \ + examples/python/gi/deactivate-all.py \ examples/python/gi/device-state-ip4config.py \ examples/python/gi/firewall-zone.py \ - examples/python/gi/show-wifi-networks.py \ - examples/python/gi/get_ips.py \ - examples/python/gi/add_connection.py \ examples/python/gi/get-active-connections.py \ + examples/python/gi/get_ips.py \ + examples/python/gi/list-connections.py \ + examples/python/gi/setting-user-data.py \ + examples/python/gi/show-wifi-networks.py \ examples/python/gi/update-ip4-method.py \ - examples/python/gi/deactivate-all.py \ - examples/python/gi/README \ \ examples/python/python-networkmanager/README \ \ diff --git a/examples/python/gi/setting-user-data.py b/examples/python/gi/setting-user-data.py new file mode 100755 index 0000000000..79fb98cdbf --- /dev/null +++ b/examples/python/gi/setting-user-data.py @@ -0,0 +1,246 @@ +#!/usr/bin/env python +# -*- Mode: Python; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- +# vim: ft=python ts=4 sts=4 sw=4 et ai + +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# +# Copyright 2017 Red Hat, Inc. + +# +# set and show user-data for a connection: +# +# - Show all user data for all connections: +# $ ./examples/python/gi/setting-user-data.py +# - Filter to show only connections with matching id or uuid +# $ ./examples/python/gi/setting-user-data.py id my-connection +# $ ./examples/python/gi/setting-user-data.py uuid 123e4567-e89b-12d3-a456-426655440000 +# - id and uuid can be repeated to select multiple connections +# $ ./examples/python/gi/setting-user-data.py id my-connection1 id my-other-connection +# +# - Specify the user-data keys you want to see +# $ ./examples/python/gi/setting-user-data.py id my-connection my.user.field.1 +# $ ./examples/python/gi/setting-user-data.py id my-connection my.user.field.1 my.other.userfield +# - Prefix the field name with ~ to use a regex +# $ ./examples/python/gi/setting-user-data.py '~^my\.user\.' +# +# - set the fields, you need to select exactly one connection +# $ ./examples/python/gi/setting-user-data.py set id "$NAME" my.field.1 my-value1 +# - delete a user-setting +# $ ./examples/python/gi/setting-user-data.py set id "$NAME" -d my.field.1 +# - set/delete multiple user data values at once +# $ ./examples/python/gi/setting-user-data.py set id "$NAME" my.field.1 my-value1 -d my.other.field +# +# - libnm already client side rejects invalid values, like +# $ ./examples/python/gi/setting-user-data.py set id "$NAME" invalid_name 'has-no-dot' +# - to allow client side to specify invalid values and send them to the +# server, pass --set-gobject +# $ ./examples/python/gi/setting-user-data.py set id "$NAME" invalid_name 'has-no-dot' --set-gobject +# + +import sys +import re + +import gi +gi.require_version('NM', '1.0') +from gi.repository import NM + +def pr(v): + import pprint + pprint.pprint(v, indent=4, depth=5, width=60) + +def parse_args(): + args = { + 'set': False, + 'set-gobject': False, + 'filter': [], + 'data': [] + } + i = 1 + while i < len(sys.argv): + a = sys.argv[i] + if i == 1: + if a in ['s', 'set']: + args['set'] = True + i += 1 + continue + elif a in ['g', 'get']: + args['set'] = False + i += 1 + continue + if a in ['id', 'uuid']: + args['filter'].append((a, sys.argv[i+1])) + i += 2 + continue + + if a in ['--set-gobject']: + args['set-gobject'] = True + i += 1 + continue + + if a == 'data': + i += 1 + a = sys.argv[i] + if args['set']: + if a == '-d': + args['data'].append((sys.argv[i+1], None)) + else: + args['data'].append((a, sys.argv[i+1])) + i += 2 + else: + args['data'].append(a) + i += 1 + + return args + +def connection_to_str(connection): + return '%s (%s)' % (connection.get_id(), connection.get_uuid()) + +def connections_filter(connections, filter_data): + connections = list(sorted(connections, key=connection_to_str)) + if not filter_data: + return connections + # we preserve the order of the selected connections. And + # if connections are selected multiple times, we return + # them multiple times. + l = [] + for f in filter_data: + if f[0] == 'id': + for c in connections: + if f[1] == c.get_id(): + l.append(c) + else: + assert(f[0] == 'uuid') + for c in connections: + if f[1] == c.get_uuid(): + l.append(c) + return l + +def print_user_data(connection, data_allow_regex, data, prefix=''): + s_u = connection.get_setting(NM.SettingUser) + n = 'none' + keys_len = 0 + keys = [] + if s_u is not None: + all_keys = s_u.get_keys() + keys_len = len(all_keys) + if data: + for d in data: + if data_allow_regex and len(d) > 0 and d[0] == '~': + r = re.compile(d[1:]) + keys.extend([k for k in all_keys if r.match(k)]) + else: + keys.append (d) + else: + keys.extend(all_keys) + n = '%s' % (keys_len) + + print('%s%s [%s]' % (prefix, connection_to_str(connection), n)) + dd = { } + if s_u is not None: + dd = s_u.get_property(NM.SETTING_USER_DATA) + for k in keys: + if s_u is not None: + v = s_u.get_data(k) + if v is None: + if k in dd: + print('%s INVALID: "%s" = "%s"' % (prefix, k, dd[k])) + else: + print('%s MISSING: "%s"' % (prefix, k)) + else: + assert(v == dd.get(k, None)) + print('%s SET: "%s" = "%s"' % (prefix, k, v)) + else: + print('%s MISSING: "%s"' % (prefix, k)) + + +def do_get(connections, data): + first_line = True + connections = list(connections) + if not connections: + print('no matching connections (use id|uuid argument)') + sys.exit(1) + for c in connections: + if first_line: + first_line = False + else: + print('') + print_user_data(c, True, data) + +def do_set(connection, data, set_gobject): + print_user_data(connection, False, + [d[0] for d in data], + prefix = 'BEFORE: ') + print('') + s_u = connection.get_setting(NM.SettingUser) + if s_u is None: + connection.add_setting(NM.SettingUser()) + s_u = connection.get_setting(NM.SettingUser) + for d in data: + key = d[0] + val = d[1] + if val is None: + print(' DEL: "%s"' % (key)) + else: + print(' SET: "%s" = "%s"' % (key, val)) + if set_gobject: + d = s_u.get_property(NM.SETTING_USER_DATA) + if val is None: + d.pop(key, None) + else: + d[key] = val + s_u.set_property(NM.SETTING_USER_DATA, d) + else: + try: + s_u.set_data(key, val) + except Exception as e: + if val is None: + print('error deleting key "%s": %s' % (key, e)) + else: + print('error setting key "%s" = "%s": %s' % (key, val, e)) + sys.exit(1) + + + try: + connection.commit_changes(True, None) + except Exception as e: + print('failure to commit connection: %s' % (e)) + sys.exit(1) + + print('') + print_user_data(connection, False, + [d[0] for d in data], + prefix = 'AFTER: ') + +############################################################################### + +if __name__ == '__main__': + args = parse_args() + nm_client = NM.Client.new(None) + + connections = connections_filter(nm_client.get_connections(), args['filter']) + + if args['set']: + if not args['data']: + print('Requires one or more arguments to set or delete') + sys.exit(1) + if len(connections) != 1: + print('To set the user-data of a connection, exactly one connection must be selected via id|uuid. Instead, %s connection matched ([%s])' % + (len(connections), ', '.join([connection_to_str(c) for c in connections]))) + sys.exit(1) + do_set(connections[0], args['data'], args['set-gobject']) + else: + do_get(connections, args['data']) +