all: avoid byte ordering issue for IP4Config's Nameservers/WinsServers on D-Bus

Some properties in NetworkManager's D-Bus API are IPv4 addresses
in network byte order (big endian). That is problematic.

It is no problem, when the NetworkManager client runs on the same
host. That is the case with libnm, which does not support to be used
remotely for the time being.

It is a problem for an application that wants to access the D-Bus
interface of NetworkManager remotely. Possibly, such an application
would be implemented in two layers:

 - one layer merely remotes D-Bus, without specific knowledge of
   NetworkManager's API.

 - a higher layer which accesses the remote D-Bus interface of NetworkManager.
   Preferably it does so in an agnostic way, regardless of whether it runs
   locally or remotely.

When using a D-Bus library, all accesses to 32 bit integers are in
native endianness (regardless of how the integer is actually encoded
on the lower layers). Likewise, D-Bus does not support annotating integer
types in non-native endianness. There is no way to annotate an integer
type "u" to be anything but native order.
That means, when remoting D-Bus at some point the endianness must be
corrected.
But by looking at the D-Bus introspection alone, it is not possible
to know which property need correction and which don't. One would need
to understand the meaning of the properties.

That makes it problematic, because the higher layer of the application,
which knows that the "Nameservers" property is supposed to be in network
order, might not easily know, whether it must correct for endianness.

Deprecate IP4Config properties that are only accessible with a particular
endianness, and add new properties that expose the same data in an
agnostic way.

Note that I added "WinsServerData" to be a plain "as", while
"NameserverData" is of type "aa{sv}". There is no particularly strong
reason for these choices, except that I could imagine that it could be
useful to expose additional information in the future about nameservers
(e.g. are they received via DHCP or manual configuration?). On the other
hand, WINS information likely won't get extended in the future.

Also note, libnm was not modified to use the new D-Bus fields. The
endianness issue is no problem for libnm, so there is little reason to
change it (at this point).

https://bugzilla.redhat.com/show_bug.cgi?id=1153559
https://bugzilla.redhat.com/show_bug.cgi?id=1584584
This commit is contained in:
Thomas Haller 2018-07-13 20:15:33 +02:00
parent 62a4f2652f
commit 4eeb4b1bdd
4 changed files with 101 additions and 22 deletions

View file

@ -55,10 +55,20 @@
<!--
Nameservers:
The nameservers in use.
The nameservers in use. Deprecated: use NameserverData
-->
<property name="Nameservers" type="au" access="read"/>
<!--
NameserverData:
The nameservers in use. Currently only the value "address"
is recognized (with an IP address string).
Since: 1.14
-->
<property name="NameserverData" type="aa{sv}" access="read"/>
<!--
Domains:
@ -92,10 +102,19 @@
WinsServers:
The Windows Internet Name Service servers associated with the connection.
Each address is in network byte order.
Each address is in network byte order. Deprecated: use WinsServerData
-->
<property name="WinsServers" type="au" access="read"/>
<!--
WinsServerData:
The Windows Internet Name Service servers associated with the connection.
Since: 1.14
-->
<property name="WinsServerData" type="as" access="read"/>
<!--
PropertiesChanged:
@properties: A dictionary mapping property names to variant boxed values

View file

@ -173,9 +173,11 @@ init_dbus (NMObject *object)
{ "address-data", &priv->addresses, demarshal_ip_address_data },
{ NM_IP_CONFIG_ROUTES, &priv->routes, demarshal_ip_routes },
{ "route-data", &priv->routes, demarshal_ip_route_data },
/* Still use deprecated "Nameservers" property instead of "NameserverData" */
{ NM_IP_CONFIG_NAMESERVERS, &priv->nameservers, demarshal_ip_array },
{ NM_IP_CONFIG_DOMAINS, &priv->domains, },
{ NM_IP_CONFIG_SEARCHES, &priv->searches, },
/* Still use deprecated "WinsServers" property instead of "WinsServerData" */
{ NM_IP_CONFIG_WINS_SERVERS, &priv->wins, demarshal_ip_array },
{ NULL },
};

View file

@ -278,10 +278,12 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMIP4Config,
PROP_ROUTE_DATA,
PROP_ROUTES,
PROP_GATEWAY,
PROP_NAMESERVER_DATA,
PROP_NAMESERVERS,
PROP_DOMAINS,
PROP_SEARCHES,
PROP_DNS_OPTIONS,
PROP_WINS_SERVER_DATA,
PROP_WINS_SERVERS,
PROP_DNS_PRIORITY,
);
@ -2276,7 +2278,8 @@ nm_ip4_config_reset_nameservers (NMIP4Config *self)
if (priv->nameservers->len != 0) {
g_array_set_size (priv->nameservers, 0);
_notify (self, PROP_NAMESERVERS);
nm_gobject_notify_together (self, PROP_NAMESERVER_DATA,
PROP_NAMESERVERS);
}
}
@ -2293,7 +2296,8 @@ nm_ip4_config_add_nameserver (NMIP4Config *self, guint32 new)
return;
g_array_append_val (priv->nameservers, new);
_notify (self, PROP_NAMESERVERS);
nm_gobject_notify_together (self, PROP_NAMESERVER_DATA,
PROP_NAMESERVERS);
}
void
@ -2304,7 +2308,8 @@ nm_ip4_config_del_nameserver (NMIP4Config *self, guint i)
g_return_if_fail (i < priv->nameservers->len);
g_array_remove_index (priv->nameservers, i);
_notify (self, PROP_NAMESERVERS);
nm_gobject_notify_together (self, PROP_NAMESERVER_DATA,
PROP_NAMESERVERS);
}
guint
@ -2622,7 +2627,8 @@ nm_ip4_config_reset_wins (NMIP4Config *self)
if (priv->wins->len != 0) {
g_array_set_size (priv->wins, 0);
_notify (self, PROP_WINS_SERVERS);
nm_gobject_notify_together (self, PROP_WINS_SERVER_DATA,
PROP_WINS_SERVERS);
}
}
@ -2639,7 +2645,8 @@ nm_ip4_config_add_wins (NMIP4Config *self, guint32 wins)
return;
g_array_append_val (priv->wins, wins);
_notify (self, PROP_WINS_SERVERS);
nm_gobject_notify_together (self, PROP_WINS_SERVER_DATA,
PROP_WINS_SERVERS);
}
void
@ -2650,7 +2657,8 @@ nm_ip4_config_del_wins (NMIP4Config *self, guint i)
g_return_if_fail (i < priv->wins->len);
g_array_remove_index (priv->wins, i);
_notify (self, PROP_WINS_SERVERS);
nm_gobject_notify_together (self, PROP_WINS_SERVER_DATA,
PROP_WINS_SERVERS);
}
guint
@ -2913,6 +2921,7 @@ get_property (GObject *object, guint prop_id,
NMDedupMultiIter ipconf_iter;
const NMPlatformIP4Route *route;
GVariantBuilder builder_data, builder_legacy;
guint i;
switch (prop_id) {
case PROP_IFINDEX:
@ -2931,7 +2940,7 @@ get_property (GObject *object, guint prop_id,
head_entry = nm_ip4_config_lookup_addresses (self);
if (head_entry) {
gs_free const NMPObject **addresses = NULL;
guint naddr, i;
guint naddr;
addresses = (const NMPObject **) nm_dedup_multi_objs_to_array_head (head_entry, NULL, NULL, &naddr);
nm_assert (addresses && naddr);
@ -3067,6 +3076,26 @@ out_routes_cached:
} else
g_value_set_string (value, NULL);
break;
case PROP_NAMESERVER_DATA:
g_variant_builder_init (&builder_data, G_VARIANT_TYPE ("aa{sv}"));
for (i = 0; i < priv->nameservers->len; i++) {
GVariantBuilder nested_builder;
char addr_str[NM_UTILS_INET_ADDRSTRLEN];
nm_utils_inet4_ntop (g_array_index (priv->nameservers, in_addr_t, i),
addr_str);
g_variant_builder_init (&nested_builder, G_VARIANT_TYPE ("a{sv}"));
g_variant_builder_add (&nested_builder, "{sv}",
"address",
g_variant_new_string (addr_str));
g_variant_builder_add (&builder_data, "a{sv}", &nested_builder);
}
g_value_take_variant (value,
g_variant_builder_end (&builder_data));
break;
case PROP_NAMESERVERS:
g_value_take_variant (value,
g_variant_new_fixed_array (G_VARIANT_TYPE_UINT32,
@ -3086,6 +3115,19 @@ out_routes_cached:
case PROP_DNS_PRIORITY:
g_value_set_int (value, priv->dns_priority);
break;
case PROP_WINS_SERVER_DATA:
g_variant_builder_init (&builder_data, G_VARIANT_TYPE ("as"));
for (i = 0; i < priv->wins->len; i++) {
char addr_str[NM_UTILS_INET_ADDRSTRLEN];
g_variant_builder_add (&builder_data,
"s",
nm_utils_inet4_ntop (g_array_index (priv->wins, in_addr_t, i),
addr_str));
}
g_value_take_variant (value,
g_variant_builder_end (&builder_data));
break;
case PROP_WINS_SERVERS:
g_value_take_variant (value,
g_variant_new_fixed_array (G_VARIANT_TYPE_UINT32,
@ -3193,17 +3235,19 @@ static const NMDBusInterfaceInfoExtended interface_info_ip4_config = {
&nm_signal_info_property_changed_legacy,
),
.properties = NM_DEFINE_GDBUS_PROPERTY_INFOS (
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("Addresses", "aau", NM_IP4_CONFIG_ADDRESSES),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("AddressData", "aa{sv}", NM_IP4_CONFIG_ADDRESS_DATA),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("Gateway", "s", NM_IP4_CONFIG_GATEWAY),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("Routes", "aau", NM_IP4_CONFIG_ROUTES),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("RouteData", "aa{sv}", NM_IP4_CONFIG_ROUTE_DATA),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("Nameservers", "au", NM_IP4_CONFIG_NAMESERVERS),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("Domains", "as", NM_IP4_CONFIG_DOMAINS),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("Searches", "as", NM_IP4_CONFIG_SEARCHES),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("DnsOptions", "as", NM_IP4_CONFIG_DNS_OPTIONS),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("DnsPriority", "i", NM_IP4_CONFIG_DNS_PRIORITY),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("WinsServers", "au", NM_IP4_CONFIG_WINS_SERVERS),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("Addresses", "aau", NM_IP4_CONFIG_ADDRESSES),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("AddressData", "aa{sv}", NM_IP4_CONFIG_ADDRESS_DATA),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("Gateway", "s", NM_IP4_CONFIG_GATEWAY),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("Routes", "aau", NM_IP4_CONFIG_ROUTES),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("RouteData", "aa{sv}", NM_IP4_CONFIG_ROUTE_DATA),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE ("NameserverData", "aa{sv}", NM_IP4_CONFIG_NAMESERVER_DATA),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("Nameservers", "au", NM_IP4_CONFIG_NAMESERVERS),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("Domains", "as", NM_IP4_CONFIG_DOMAINS),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("Searches", "as", NM_IP4_CONFIG_SEARCHES),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("DnsOptions", "as", NM_IP4_CONFIG_DNS_OPTIONS),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("DnsPriority", "i", NM_IP4_CONFIG_DNS_PRIORITY),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE ("WinsServerData", "as", NM_IP4_CONFIG_WINS_SERVER_DATA),
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("WinsServers", "au", NM_IP4_CONFIG_WINS_SERVERS),
),
),
.legacy_property_changed = TRUE,
@ -3262,6 +3306,12 @@ nm_ip4_config_class_init (NMIP4ConfigClass *config_class)
NULL,
G_PARAM_READABLE |
G_PARAM_STATIC_STRINGS);
obj_properties[PROP_NAMESERVER_DATA] =
g_param_spec_variant (NM_IP4_CONFIG_NAMESERVER_DATA, "", "",
G_VARIANT_TYPE ("aa{sv}"),
NULL,
G_PARAM_READABLE |
G_PARAM_STATIC_STRINGS);
obj_properties[PROP_NAMESERVERS] =
g_param_spec_variant (NM_IP4_CONFIG_NAMESERVERS, "", "",
G_VARIANT_TYPE ("au"),
@ -3288,6 +3338,12 @@ nm_ip4_config_class_init (NMIP4ConfigClass *config_class)
G_MININT32, G_MAXINT32, 0,
G_PARAM_READABLE |
G_PARAM_STATIC_STRINGS);
obj_properties[PROP_WINS_SERVER_DATA] =
g_param_spec_variant (NM_IP4_CONFIG_WINS_SERVER_DATA, "", "",
G_VARIANT_TYPE ("as"),
NULL,
G_PARAM_READABLE |
G_PARAM_STATIC_STRINGS);
obj_properties[PROP_WINS_SERVERS] =
g_param_spec_variant (NM_IP4_CONFIG_WINS_SERVERS, "", "",
G_VARIANT_TYPE ("au"),

View file

@ -138,16 +138,18 @@ typedef struct _NMIP4ConfigClass NMIP4ConfigClass;
#define NM_IP4_CONFIG_ADDRESS_DATA "address-data"
#define NM_IP4_CONFIG_ROUTE_DATA "route-data"
#define NM_IP4_CONFIG_GATEWAY "gateway"
#define NM_IP4_CONFIG_NAMESERVERS "nameservers"
#define NM_IP4_CONFIG_NAMESERVER_DATA "nameserver-data"
#define NM_IP4_CONFIG_DOMAINS "domains"
#define NM_IP4_CONFIG_SEARCHES "searches"
#define NM_IP4_CONFIG_DNS_OPTIONS "dns-options"
#define NM_IP4_CONFIG_DNS_PRIORITY "dns-priority"
#define NM_IP4_CONFIG_WINS_SERVERS "wins-servers"
#define NM_IP4_CONFIG_WINS_SERVER_DATA "wins-server-data"
/* deprecated */
#define NM_IP4_CONFIG_ADDRESSES "addresses"
#define NM_IP4_CONFIG_ROUTES "routes"
#define NM_IP4_CONFIG_NAMESERVERS "nameservers"
#define NM_IP4_CONFIG_WINS_SERVERS "wins-servers"
GType nm_ip4_config_get_type (void);