Commit c785a7df authored by Dan Winship's avatar Dan Winship

libnm-core: change how new and legacy properties are serialized

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 daemon side to prefer the legacy properties
to the new ones if both are set, and changing the client side to not
send the legacy properties (since we don't support new clients talking
to old servers anyway).
parent 543416e5
......@@ -2092,6 +2092,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)
{
......@@ -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));
/**
......
......@@ -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);
......
......@@ -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);
......
......@@ -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 */
......@@ -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;
......
......@@ -271,6 +271,8 @@ nm_utils_deinit (void)
}
}
gboolean _nm_utils_is_manager_process;
/* ssid helpers */
/**
......
......@@ -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);
......@@ -530,6 +532,126 @@ test_setting_ip4_config_labels (void)
g_object_unref (conn);
}
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)
{
......@@ -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);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment