diff options
author | Dan Winship <danw@gnome.org> | 2014-11-14 11:46:19 -0500 |
---|---|---|
committer | Dan Winship <danw@gnome.org> | 2014-11-14 15:20:32 -0500 |
commit | ee18980ec95907513e6a176a0e17951da228a200 (patch) | |
tree | 8556523f5be1e12658cbe4e2d31a9ae2241891c0 | |
parent | 51c5368b71631416b50e42a2d76d7dffc80c78a1 (diff) | |
download | NetworkManager-danw/compat-props-bgo740140.tar.gz |
libnm-core: change how new and legacy properties are serializeddanw/compat-props-bgo740140
Although libnm filters out properties received from the daemon that it
doesn't understand, there may be other clients that do not. In
particular, a client might call GetSettings() on a connection, update
the ipv4.addresses property in the returned dictionary, and then pass
the dictionary to Update(). In that case, the updated dictionary would
contain ipv4.address-data, but it would not reflect the changes the
client intended to make.
Fix this by changing the client side to not send the legacy properties
(since we don't support new clients talking to old servers anyway),
and changing the daemon side to prefer the legacy properties to the
new ones if both are set.
-rw-r--r-- | libnm-core/nm-setting-ip-config.c | 27 | ||||
-rw-r--r-- | libnm-core/nm-setting-ip4-config.c | 37 | ||||
-rw-r--r-- | libnm-core/nm-setting-ip6-config.c | 38 | ||||
-rw-r--r-- | libnm-core/nm-setting-private.h | 8 | ||||
-rw-r--r-- | libnm-core/nm-setting.c | 40 | ||||
-rw-r--r-- | libnm-core/nm-utils.c | 2 | ||||
-rw-r--r-- | libnm-core/tests/test-general.c | 139 |
7 files changed, 243 insertions, 48 deletions
diff --git a/libnm-core/nm-setting-ip-config.c b/libnm-core/nm-setting-ip-config.c index 25f7c82f01..b76ff1bec1 100644 --- a/libnm-core/nm-setting-ip-config.c +++ b/libnm-core/nm-setting-ip-config.c @@ -2093,6 +2093,19 @@ get_property (GObject *object, guint prop_id, } static void +ip_gateway_set (NMSetting *setting, + GVariant *connection_dict, + const char *property, + GVariant *value) +{ + /* Don't set from 'gateway' if we're going to use the gateway in 'addresses' */ + if (_nm_setting_use_legacy_property (setting, connection_dict, "addresses", "gateway")) + return; + + g_object_set (setting, property, g_variant_get_string (value, NULL), NULL); +} + +static void nm_setting_ip_config_class_init (NMSettingIPConfigClass *setting_class) { GObjectClass *object_class = G_OBJECT_CLASS (setting_class); @@ -2171,6 +2184,11 @@ nm_setting_ip_config_class_init (NMSettingIPConfigClass *setting_class) G_TYPE_PTR_ARRAY, G_PARAM_READWRITE | NM_SETTING_PARAM_INFERRABLE | + /* "addresses" is a legacy D-Bus property, because the + * "addresses" GObject property normally gets set from + * the "address-data" D-Bus property... + */ + NM_SETTING_PARAM_LEGACY | G_PARAM_STATIC_STRINGS)); /** @@ -2187,6 +2205,13 @@ nm_setting_ip_config_class_init (NMSettingIPConfigClass *setting_class) NM_SETTING_PARAM_INFERRABLE | G_PARAM_STATIC_STRINGS)); + _nm_setting_class_override_property (parent_class, + NM_SETTING_IP_CONFIG_GATEWAY, + G_VARIANT_TYPE_STRING, + NULL, + ip_gateway_set, + NULL); + /** * NMSettingIPConfig:routes: * @@ -2200,6 +2225,8 @@ nm_setting_ip_config_class_init (NMSettingIPConfigClass *setting_class) G_TYPE_PTR_ARRAY, G_PARAM_READWRITE | NM_SETTING_PARAM_INFERRABLE | + /* See :addresses above Re: LEGACY */ + NM_SETTING_PARAM_LEGACY | G_PARAM_STATIC_STRINGS)); /** diff --git a/libnm-core/nm-setting-ip4-config.c b/libnm-core/nm-setting-ip4-config.c index 4f451f5eb4..008e87eea9 100644 --- a/libnm-core/nm-setting-ip4-config.c +++ b/libnm-core/nm-setting-ip4-config.c @@ -271,33 +271,26 @@ ip4_addresses_set (NMSetting *setting, char **labels, *gateway = NULL; int i; - s_ip4 = g_variant_lookup_value (connection_dict, NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_VARIANT_TYPE_SETTING); - /* If 'address-data' is set then ignore 'addresses' */ - if (g_variant_lookup (s_ip4, "address-data", "aa{sv}", NULL)) { - g_variant_unref (s_ip4); + if (!_nm_setting_use_legacy_property (setting, connection_dict, "addresses", "address-data")) return; - } addrs = nm_utils_ip4_addresses_from_variant (value, &gateway); + s_ip4 = g_variant_lookup_value (connection_dict, NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_VARIANT_TYPE_SETTING); if (g_variant_lookup (s_ip4, "address-labels", "^as", &labels)) { for (i = 0; i < addrs->len && labels[i]; i++) if (*labels[i]) nm_ip_address_set_attribute (addrs->pdata[i], "label", g_variant_new_string (labels[i])); g_strfreev (labels); } - - if (gateway && !g_variant_lookup (s_ip4, "gateway", "s", NULL)) { - g_object_set (setting, - NM_SETTING_IP_CONFIG_GATEWAY, gateway, - NULL); - } - g_free (gateway); - g_variant_unref (s_ip4); - g_object_set (setting, property, addrs, NULL); + g_object_set (setting, + NM_SETTING_IP_CONFIG_ADDRESSES, addrs, + NM_SETTING_IP_CONFIG_GATEWAY, gateway, + NULL); g_ptr_array_unref (addrs); + g_free (gateway); } static GVariant * @@ -361,6 +354,10 @@ ip4_address_data_set (NMSetting *setting, { GPtrArray *addrs; + /* Ignore 'address-data' if we're going to process 'addresses' */ + if (_nm_setting_use_legacy_property (setting, connection_dict, "addresses", "address-data")) + return; + addrs = nm_utils_ip_addresses_from_variant (value, AF_INET); g_object_set (setting, NM_SETTING_IP_CONFIG_ADDRESSES, addrs, NULL); g_ptr_array_unref (addrs); @@ -387,15 +384,9 @@ ip4_routes_set (NMSetting *setting, GVariant *value) { GPtrArray *routes; - GVariant *s_ip4; - s_ip4 = g_variant_lookup_value (connection_dict, NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_VARIANT_TYPE_SETTING); - /* If 'route-data' is set then ignore 'routes' */ - if (g_variant_lookup (s_ip4, "route-data", "aa{sv}", NULL)) { - g_variant_unref (s_ip4); + if (!_nm_setting_use_legacy_property (setting, connection_dict, "routes", "route-data")) return; - } - g_variant_unref (s_ip4); routes = nm_utils_ip4_routes_from_variant (value); g_object_set (setting, property, routes, NULL); @@ -425,6 +416,10 @@ ip4_route_data_set (NMSetting *setting, { GPtrArray *routes; + /* Ignore 'route-data' if we're going to process 'routes' */ + if (_nm_setting_use_legacy_property (setting, connection_dict, "routes", "route-data")) + return; + routes = nm_utils_ip_routes_from_variant (value, AF_INET); g_object_set (setting, NM_SETTING_IP_CONFIG_ROUTES, routes, NULL); g_ptr_array_unref (routes); diff --git a/libnm-core/nm-setting-ip6-config.c b/libnm-core/nm-setting-ip6-config.c index 50dbc3c723..7bfce8ef47 100644 --- a/libnm-core/nm-setting-ip6-config.c +++ b/libnm-core/nm-setting-ip6-config.c @@ -213,29 +213,19 @@ ip6_addresses_set (NMSetting *setting, GVariant *value) { GPtrArray *addrs; - GVariant *s_ip6; char *gateway = NULL; - s_ip6 = g_variant_lookup_value (connection_dict, NM_SETTING_IP6_CONFIG_SETTING_NAME, NM_VARIANT_TYPE_SETTING); - /* If 'address-data' is set then ignore 'addresses' */ - if (g_variant_lookup (s_ip6, "address-data", "aa{sv}", NULL)) { - g_variant_unref (s_ip6); + if (!_nm_setting_use_legacy_property (setting, connection_dict, "addresses", "address-data")) return; - } addrs = nm_utils_ip6_addresses_from_variant (value, &gateway); - if (gateway && !g_variant_lookup (s_ip6, "gateway", "s", NULL)) { - g_object_set (setting, - NM_SETTING_IP_CONFIG_GATEWAY, gateway, - NULL); - } - g_free (gateway); - - g_variant_unref (s_ip6); - - g_object_set (setting, property, addrs, NULL); + g_object_set (setting, + NM_SETTING_IP_CONFIG_ADDRESSES, addrs, + NM_SETTING_IP_CONFIG_GATEWAY, gateway, + NULL); g_ptr_array_unref (addrs); + g_free (gateway); } static GVariant * @@ -261,6 +251,10 @@ ip6_address_data_set (NMSetting *setting, { GPtrArray *addrs; + /* Ignore 'address-data' if we're going to process 'addresses' */ + if (_nm_setting_use_legacy_property (setting, connection_dict, "addresses", "address-data")) + return; + addrs = nm_utils_ip_addresses_from_variant (value, AF_INET6); g_object_set (setting, NM_SETTING_IP_CONFIG_ADDRESSES, addrs, NULL); g_ptr_array_unref (addrs); @@ -287,15 +281,9 @@ ip6_routes_set (NMSetting *setting, GVariant *value) { GPtrArray *routes; - GVariant *s_ip6; - s_ip6 = g_variant_lookup_value (connection_dict, NM_SETTING_IP6_CONFIG_SETTING_NAME, NM_VARIANT_TYPE_SETTING); - /* If 'route-data' is set then ignore 'routes' */ - if (g_variant_lookup (s_ip6, "route-data", "aa{sv}", NULL)) { - g_variant_unref (s_ip6); + if (!_nm_setting_use_legacy_property (setting, connection_dict, "routes", "route-data")) return; - } - g_variant_unref (s_ip6); routes = nm_utils_ip6_routes_from_variant (value); g_object_set (setting, property, routes, NULL); @@ -325,6 +313,10 @@ ip6_route_data_set (NMSetting *setting, { GPtrArray *routes; + /* Ignore 'route-data' if we're going to process 'routes' */ + if (_nm_setting_use_legacy_property (setting, connection_dict, "routes", "route-data")) + return; + routes = nm_utils_ip_routes_from_variant (value, AF_INET6); g_object_set (setting, NM_SETTING_IP_CONFIG_ROUTES, routes, NULL); g_ptr_array_unref (routes); diff --git a/libnm-core/nm-setting-private.h b/libnm-core/nm-setting-private.h index bb9f1d8ee1..2d34d509a5 100644 --- a/libnm-core/nm-setting-private.h +++ b/libnm-core/nm-setting-private.h @@ -86,6 +86,9 @@ gboolean _nm_setting_clear_secrets_with_flags (NMSetting *setting, */ #define NM_SETTING_PARAM_INFERRABLE (1 << (4 + G_PARAM_USER_SHIFT)) +/* This is a legacy property, which clients should not send to the daemon. */ +#define NM_SETTING_PARAM_LEGACY (1 << (5 + G_PARAM_USER_SHIFT)) + /* Ensure the setting's GType is registered at library load time */ #define NM_SETTING_REGISTER_TYPE(x) \ static void __attribute__((constructor)) register_setting (void) \ @@ -145,6 +148,11 @@ void _nm_setting_class_transform_property (NMSettingClass *setting_class, NMSettingPropertyTransformToFunc to_dbus, NMSettingPropertyTransformFromFunc from_dbus); +gboolean _nm_setting_use_legacy_property (NMSetting *setting, + GVariant *connection_dict, + const char *legacy_property, + const char *new_property); + GPtrArray *_nm_setting_need_secrets (NMSetting *setting); #endif /* NM_SETTING_PRIVATE_H */ diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index 278c21b1b2..40298d3c73 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -482,6 +482,42 @@ _nm_setting_class_transform_property (NMSettingClass *setting_class, to_dbus, from_dbus); } +gboolean +_nm_setting_use_legacy_property (NMSetting *setting, + GVariant *connection_dict, + const char *legacy_property, + const char *new_property) +{ + GVariant *setting_dict, *value; + + setting_dict = g_variant_lookup_value (connection_dict, nm_setting_get_name (NM_SETTING (setting)), NM_VARIANT_TYPE_SETTING); + g_return_val_if_fail (setting_dict != NULL, FALSE); + + /* If the new property isn't set, we have to use the legacy property. */ + value = g_variant_lookup_value (setting_dict, new_property, NULL); + if (!value) { + g_variant_unref (setting_dict); + return TRUE; + } + g_variant_unref (value); + + /* Otherwise, clients always prefer new properties sent from the daemon. */ + if (!_nm_utils_is_manager_process) { + g_variant_unref (setting_dict); + return FALSE; + } + + /* The daemon prefers the legacy property if it exists. */ + value = g_variant_lookup_value (setting_dict, legacy_property, NULL); + g_variant_unref (setting_dict); + + if (value) { + g_variant_unref (value); + return TRUE; + } else + return FALSE; +} + static GArray * nm_setting_class_ensure_properties (NMSettingClass *setting_class) { @@ -667,6 +703,10 @@ _nm_setting_to_dbus (NMSetting *setting, NMConnection *connection, NMConnectionS if (prop_spec && !(prop_spec->flags & G_PARAM_WRITABLE)) continue; + if ( prop_spec && (prop_spec->flags & NM_SETTING_PARAM_LEGACY) + && !_nm_utils_is_manager_process) + continue; + if ( (flags & NM_CONNECTION_SERIALIZE_NO_SECRETS) && (prop_spec && (prop_spec->flags & NM_SETTING_PARAM_SECRET))) continue; diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index dce475569e..539b787db0 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -271,6 +271,8 @@ nm_utils_deinit (void) } } +gboolean _nm_utils_is_manager_process; + /* ssid helpers */ /** diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 829fb3926a..de3b9f17e3 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -416,12 +416,14 @@ test_setting_ip4_config_labels (void) label = nm_ip_address_get_attribute (addr, "label"); g_assert (label == NULL); - /* The labels should appear in the D-Bus serialization under both - * 'address-labels' and 'address-data'. + /* If we serialize as the daemon, the labels should appear in the D-Bus + * serialization under both 'address-labels' and 'address-data'. */ conn = nmtst_create_minimal_connection ("label test", NULL, NM_SETTING_WIRED_SETTING_NAME, NULL); nm_connection_add_setting (conn, NM_SETTING (s_ip4)); + _nm_utils_is_manager_process = TRUE; dict = nm_connection_to_dbus (conn, NM_CONNECTION_SERIALIZE_ALL); + _nm_utils_is_manager_process = FALSE; g_object_unref (conn); setting_dict = g_variant_lookup_value (dict, NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_VARIANT_TYPE_SETTING); @@ -531,6 +533,126 @@ test_setting_ip4_config_labels (void) } static void +test_setting_ip4_config_address_data (void) +{ + NMSettingIPConfig *s_ip4; + NMIPAddress *addr; + GPtrArray *addrs; + NMConnection *conn; + GVariant *dict, *setting_dict, *value; + GError *error = NULL; + + s_ip4 = (NMSettingIPConfig *) nm_setting_ip4_config_new (); + g_object_set (G_OBJECT (s_ip4), + NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP4_CONFIG_METHOD_MANUAL, + NULL); + + /* addr 1 */ + addr = nm_ip_address_new (AF_INET, "1.1.1.1", 24, &error); + g_assert_no_error (error); + nm_ip_address_set_attribute (addr, "one", g_variant_new_string ("foo")); + nm_ip_address_set_attribute (addr, "two", g_variant_new_int32 (42)); + + nm_setting_ip_config_add_address (s_ip4, addr); + nm_ip_address_unref (addr); + nm_setting_verify (NM_SETTING (s_ip4), NULL, &error); + g_assert_no_error (error); + + /* addr 2 */ + addr = nm_ip_address_new (AF_INET, "2.2.2.2", 24, &error); + g_assert_no_error (error); + + nm_setting_ip_config_add_address (s_ip4, addr); + nm_ip_address_unref (addr); + nm_setting_verify (NM_SETTING (s_ip4), NULL, &error); + g_assert_no_error (error); + + /* The client-side D-Bus serialization should include the attributes in + * "address-data", and should not have an "addresses" property. + */ + conn = nmtst_create_minimal_connection ("address-data test", NULL, NM_SETTING_WIRED_SETTING_NAME, NULL); + nm_connection_add_setting (conn, NM_SETTING (s_ip4)); + dict = nm_connection_to_dbus (conn, NM_CONNECTION_SERIALIZE_ALL); + + setting_dict = g_variant_lookup_value (dict, NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_VARIANT_TYPE_SETTING); + g_assert (setting_dict != NULL); + + value = g_variant_lookup_value (setting_dict, "addresses", NULL); + g_assert (value == NULL); + + value = g_variant_lookup_value (setting_dict, "address-data", G_VARIANT_TYPE ("aa{sv}")); + addrs = nm_utils_ip_addresses_from_variant (value, AF_INET); + g_variant_unref (value); + g_assert (addrs != NULL); + g_assert_cmpint (addrs->len, ==, 2); + + addr = addrs->pdata[0]; + g_assert_cmpstr (nm_ip_address_get_address (addr), ==, "1.1.1.1"); + value = nm_ip_address_get_attribute (addr, "one"); + g_assert (value != NULL); + g_assert_cmpstr (g_variant_get_string (value, NULL), ==, "foo"); + value = nm_ip_address_get_attribute (addr, "two"); + g_assert (value != NULL); + g_assert_cmpint (g_variant_get_int32 (value), ==, 42); + + g_ptr_array_unref (addrs); + g_variant_unref (setting_dict); + g_variant_unref (dict); + + /* The daemon-side serialization should include both 'addresses' and 'address-data' */ + _nm_utils_is_manager_process = TRUE; + dict = nm_connection_to_dbus (conn, NM_CONNECTION_SERIALIZE_ALL); + _nm_utils_is_manager_process = FALSE; + + setting_dict = g_variant_lookup_value (dict, NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_VARIANT_TYPE_SETTING); + g_assert (setting_dict != NULL); + + value = g_variant_lookup_value (setting_dict, "addresses", G_VARIANT_TYPE ("aau")); + g_assert (value != NULL); + g_variant_unref (value); + + value = g_variant_lookup_value (setting_dict, "address-data", G_VARIANT_TYPE ("aa{sv}")); + g_assert (value != NULL); + g_variant_unref (value); + + g_variant_unref (setting_dict); + g_object_unref (conn); + + /* When we reserialize that dictionary as a client, 'address-data' will be preferred. */ + conn = nm_simple_connection_new_from_dbus (dict, &error); + g_assert_no_error (error); + + s_ip4 = nm_connection_get_setting_ip4_config (conn); + + addr = nm_setting_ip_config_get_address (s_ip4, 0); + g_assert_cmpstr (nm_ip_address_get_address (addr), ==, "1.1.1.1"); + value = nm_ip_address_get_attribute (addr, "one"); + g_assert (value != NULL); + g_assert_cmpstr (g_variant_get_string (value, NULL), ==, "foo"); + value = nm_ip_address_get_attribute (addr, "two"); + g_assert (value != NULL); + g_assert_cmpint (g_variant_get_int32 (value), ==, 42); + + /* But on the server side, 'addresses' will have precedence. */ + _nm_utils_is_manager_process = TRUE; + conn = nm_simple_connection_new_from_dbus (dict, &error); + _nm_utils_is_manager_process = FALSE; + g_assert_no_error (error); + g_variant_unref (dict); + + s_ip4 = nm_connection_get_setting_ip4_config (conn); + + addr = nm_setting_ip_config_get_address (s_ip4, 0); + g_assert_cmpstr (nm_ip_address_get_address (addr), ==, "1.1.1.1"); + value = nm_ip_address_get_attribute (addr, "one"); + g_assert (value == NULL); + value = nm_ip_address_get_attribute (addr, "two"); + g_assert (value == NULL); + + g_object_unref (conn); +} + +static void test_setting_gsm_apn_spaces (void) { NMSettingGsm *s_gsm; @@ -3416,7 +3538,9 @@ test_setting_ip4_gateway (void) GVariant *addr_var; GError *error = NULL; - /* When serializing, ipv4.gateway is copied to the first entry of ipv4.addresses */ + /* When serializing on the daemon side, ipv4.gateway is copied to the first + * entry of ipv4.addresses + */ conn = nmtst_create_minimal_connection ("test_setting_ip4_gateway", NULL, NM_SETTING_WIRED_SETTING_NAME, NULL); s_ip4 = (NMSettingIPConfig *) nm_setting_ip4_config_new (); @@ -3431,7 +3555,9 @@ test_setting_ip4_gateway (void) nm_setting_ip_config_add_address (s_ip4, addr); nm_ip_address_unref (addr); + _nm_utils_is_manager_process = TRUE; conn_dict = nm_connection_to_dbus (conn, NM_CONNECTION_SERIALIZE_ALL); + _nm_utils_is_manager_process = FALSE; g_object_unref (conn); ip4_dict = g_variant_lookup_value (conn_dict, NM_SETTING_IP4_CONFIG_SETTING_NAME, NM_VARIANT_TYPE_SETTING); @@ -3490,7 +3616,9 @@ test_setting_ip6_gateway (void) GVariant *gateway_var; GError *error = NULL; - /* When serializing, ipv6.gateway is copied to the first entry of ipv6.addresses */ + /* When serializing on the daemon side, ipv6.gateway is copied to the first + * entry of ipv6.addresses + */ conn = nmtst_create_minimal_connection ("test_setting_ip6_gateway", NULL, NM_SETTING_WIRED_SETTING_NAME, NULL); s_ip6 = (NMSettingIPConfig *) nm_setting_ip6_config_new (); @@ -3505,7 +3633,9 @@ test_setting_ip6_gateway (void) nm_setting_ip_config_add_address (s_ip6, addr); nm_ip_address_unref (addr); + _nm_utils_is_manager_process = TRUE; conn_dict = nm_connection_to_dbus (conn, NM_CONNECTION_SERIALIZE_ALL); + _nm_utils_is_manager_process = FALSE; g_object_unref (conn); ip6_dict = g_variant_lookup_value (conn_dict, NM_SETTING_IP6_CONFIG_SETTING_NAME, NM_VARIANT_TYPE_SETTING); @@ -3598,6 +3728,7 @@ int main (int argc, char **argv) g_test_add_func ("/core/general/test_setting_vpn_update_secrets", test_setting_vpn_update_secrets); g_test_add_func ("/core/general/test_setting_vpn_modify_during_foreach", test_setting_vpn_modify_during_foreach); g_test_add_func ("/core/general/test_setting_ip4_config_labels", test_setting_ip4_config_labels); + g_test_add_func ("/core/general/test_setting_ip4_config_address_data", test_setting_ip4_config_address_data); g_test_add_func ("/core/general/test_setting_gsm_apn_spaces", test_setting_gsm_apn_spaces); g_test_add_func ("/core/general/test_setting_gsm_apn_bad_chars", test_setting_gsm_apn_bad_chars); g_test_add_func ("/core/general/test_setting_gsm_apn_underscore", test_setting_gsm_apn_underscore); |