From 6d07afaa8d8303839875c0af4b2a48ca853761c9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 14 Jul 2021 07:40:35 +0200 Subject: [PATCH] libnm: implement special setter for direct string property for ip address This is a normalization employed by NMSettingIPConfig.gateway. Also rework NMSettingIPConfig.set_property() to no longer assert against valid input. We want to pass there untrusted strings from D-Bus, asserting is a horrible idea. Instead, either normalize the string or keep the invalid text that will be rejected by verify(). --- .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 2 +- src/libnm-core-impl/nm-setting-ip-config.c | 16 +++++++--------- src/libnm-core-impl/nm-setting-ip4-config.c | 2 +- src/libnm-core-impl/nm-setting-ip6-config.c | 2 +- src/libnm-core-impl/nm-setting-private.h | 2 +- src/libnm-core-impl/nm-setting.c | 7 +++++++ src/libnm-core-impl/nm-utils-private.h | 2 ++ src/libnm-core-impl/nm-utils.c | 19 +++++++++++++++++++ src/libnm-core-impl/tests/test-setting.c | 3 ++- src/libnm-core-intern/nm-core-internal.h | 12 +++--------- 10 files changed, 44 insertions(+), 23 deletions(-) diff --git a/src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c index 8d07b15608..015aafed03 100644 --- a/src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -5589,7 +5589,7 @@ test_write_wired_static_ip6_only_gw(gconstpointer user_data) /* assert that the gateway was written and reloaded as expected */ if (!gateway6 || !strcmp(gateway6, "::")) { - g_assert(nm_setting_ip_config_get_gateway(s_ip6) == NULL); + g_assert_cmpstr(nm_setting_ip_config_get_gateway(s_ip6), ==, NULL); g_assert(written_ifcfg_gateway == NULL); } else { g_assert(nm_setting_ip_config_get_gateway(s_ip6) != NULL); diff --git a/src/libnm-core-impl/nm-setting-ip-config.c b/src/libnm-core-impl/nm-setting-ip-config.c index 7f524a24f7..d9e44fcce7 100644 --- a/src/libnm-core-impl/nm-setting-ip-config.c +++ b/src/libnm-core-impl/nm-setting-ip-config.c @@ -5795,10 +5795,12 @@ ip_gateway_set(const NMSettInfoSetting * sett_info, } GArray * -_nm_sett_info_property_override_create_array_ip_config(void) +_nm_sett_info_property_override_create_array_ip_config(int addr_family) { GArray *properties_override = _nm_sett_info_property_override_create_array(); + nm_assert_addr_family(addr_family); + _nm_properties_override_gobj( properties_override, obj_properties[PROP_METHOD], @@ -5814,8 +5816,7 @@ _nm_sett_info_property_override_create_array_ip_config(void) .to_dbus_fcn = _nm_setting_property_to_dbus_fcn_direct, .from_dbus_fcn = ip_gateway_set), .direct_offset = NM_STRUCT_OFFSET_ENSURE_TYPE(char *, NMSettingIPConfigPrivate, gateway), - /* The property setter for the gateway performs some normalization and is special! */ - .direct_has_special_setter = TRUE); + .direct_set_string_ip_address_addr_family = addr_family); _nm_properties_override_gobj( properties_override, @@ -5975,7 +5976,6 @@ set_property(GObject *object, guint prop_id, const GValue *value, GParamSpec *ps { NMSettingIPConfig * setting = NM_SETTING_IP_CONFIG(object); NMSettingIPConfigPrivate *priv = NM_SETTING_IP_CONFIG_GET_PRIVATE(setting); - const char * gateway; char ** strv; guint i; @@ -6021,12 +6021,10 @@ set_property(GObject *object, guint prop_id, const GValue *value, GParamSpec *ps (GDestroyNotify) nm_ip_address_unref); break; case PROP_GATEWAY: - gateway = g_value_get_string(value); - g_return_if_fail( - !gateway - || nm_utils_ipaddr_is_valid(NM_SETTING_IP_CONFIG_GET_FAMILY(setting), gateway)); g_free(priv->gateway); - priv->gateway = canonicalize_ip(NM_SETTING_IP_CONFIG_GET_FAMILY(setting), gateway, TRUE); + priv->gateway = + _nm_utils_ipaddr_canonical_or_invalid(NM_SETTING_IP_CONFIG_GET_FAMILY(setting), + g_value_get_string(value)); break; case PROP_ROUTES: g_ptr_array_unref(priv->routes); diff --git a/src/libnm-core-impl/nm-setting-ip4-config.c b/src/libnm-core-impl/nm-setting-ip4-config.c index 5995a582b7..ee91b35d34 100644 --- a/src/libnm-core-impl/nm-setting-ip4-config.c +++ b/src/libnm-core-impl/nm-setting-ip4-config.c @@ -654,7 +654,7 @@ nm_setting_ip4_config_class_init(NMSettingIP4ConfigClass *klass) GObjectClass * object_class = G_OBJECT_CLASS(klass); NMSettingClass * setting_class = NM_SETTING_CLASS(klass); NMSettingIPConfigClass *setting_ip_config_class = NM_SETTING_IP_CONFIG_CLASS(klass); - GArray *properties_override = _nm_sett_info_property_override_create_array_ip_config(); + GArray *properties_override = _nm_sett_info_property_override_create_array_ip_config(AF_INET); g_type_class_add_private(klass, sizeof(NMSettingIP4ConfigPrivate)); diff --git a/src/libnm-core-impl/nm-setting-ip6-config.c b/src/libnm-core-impl/nm-setting-ip6-config.c index 3c2dfc8595..3c382d54c4 100644 --- a/src/libnm-core-impl/nm-setting-ip6-config.c +++ b/src/libnm-core-impl/nm-setting-ip6-config.c @@ -650,7 +650,7 @@ nm_setting_ip6_config_class_init(NMSettingIP6ConfigClass *klass) GObjectClass * object_class = G_OBJECT_CLASS(klass); NMSettingClass * setting_class = NM_SETTING_CLASS(klass); NMSettingIPConfigClass *setting_ip_config_class = NM_SETTING_IP_CONFIG_CLASS(klass); - GArray *properties_override = _nm_sett_info_property_override_create_array_ip_config(); + GArray *properties_override = _nm_sett_info_property_override_create_array_ip_config(AF_INET6); g_type_class_add_private(klass, sizeof(NMSettingIP6ConfigPrivate)); diff --git a/src/libnm-core-impl/nm-setting-private.h b/src/libnm-core-impl/nm-setting-private.h index 1042a64d71..8576c52f1e 100644 --- a/src/libnm-core-impl/nm-setting-private.h +++ b/src/libnm-core-impl/nm-setting-private.h @@ -426,7 +426,7 @@ _nm_sett_info_property_override_create_array(void) return _nm_sett_info_property_override_create_array_sized(20); } -GArray *_nm_sett_info_property_override_create_array_ip_config(void); +GArray *_nm_sett_info_property_override_create_array_ip_config(int addr_family); void _nm_setting_class_commit(NMSettingClass * setting_class, NMMetaSettingType meta_type, diff --git a/src/libnm-core-impl/nm-setting.c b/src/libnm-core-impl/nm-setting.c index efe305403c..462efe30e8 100644 --- a/src/libnm-core-impl/nm-setting.c +++ b/src/libnm-core-impl/nm-setting.c @@ -685,6 +685,13 @@ _property_direct_set_string(const NMSettInfoProperty *property_info, char **dst, src, property_info->direct_set_string_mac_address_len)); } + if (property_info->direct_set_string_ip_address_addr_family != 0) { + return nm_utils_strdup_reset_take( + dst, + _nm_utils_ipaddr_canonical_or_invalid( + property_info->direct_set_string_ip_address_addr_family, + src)); + } return nm_utils_strdup_reset(dst, src); } diff --git a/src/libnm-core-impl/nm-utils-private.h b/src/libnm-core-impl/nm-utils-private.h index 78554c944a..aefb7682a9 100644 --- a/src/libnm-core-impl/nm-utils-private.h +++ b/src/libnm-core-impl/nm-utils-private.h @@ -42,6 +42,8 @@ void _nm_utils_bytes_from_dbus(GVariant *dbus_value, GValue *prop_value); char *_nm_utils_hwaddr_canonical_or_invalid(const char *mac, gssize length); +char *_nm_utils_ipaddr_canonical_or_invalid(int addr_family, const char *ip); + gboolean _nm_utils_hwaddr_link_local_valid(const char *mac); gboolean _nm_sriov_vf_parse_vlans(NMSriovVF *vf, const char *str, GError **error); diff --git a/src/libnm-core-impl/nm-utils.c b/src/libnm-core-impl/nm-utils.c index b1fe6639a2..7da6eb7083 100644 --- a/src/libnm-core-impl/nm-utils.c +++ b/src/libnm-core-impl/nm-utils.c @@ -3880,6 +3880,25 @@ _nm_utils_hwaddr_canonical_or_invalid(const char *mac, gssize length) return g_strdup(mac); } +char * +_nm_utils_ipaddr_canonical_or_invalid(int addr_family, const char *ip) +{ + NMIPAddr addr_bin; + + nm_assert_addr_family(addr_family); + + if (!ip) + return NULL; + + if (!nm_utils_parse_inaddr_bin(addr_family, ip, NULL, &addr_bin)) + return g_strdup(ip); + + if (nm_ip_addr_is_null(addr_family, &addr_bin)) + return NULL; + + return nm_utils_inet_ntop_dup(addr_family, &addr_bin); +} + /* * Determine if given Ethernet address is link-local * diff --git a/src/libnm-core-impl/tests/test-setting.c b/src/libnm-core-impl/tests/test-setting.c index d67d6a7a3f..240918f16c 100644 --- a/src/libnm-core-impl/tests/test-setting.c +++ b/src/libnm-core-impl/tests/test-setting.c @@ -4520,7 +4520,8 @@ test_setting_metadata(void) } g_assert(((sip->direct_set_string_mac_address_len != 0) - + (!!sip->direct_set_string_ascii_strdown)) + + (!!sip->direct_set_string_ascii_strdown) + + (sip->direct_set_string_ip_address_addr_family != 0)) <= 1); if (!sip->property_type->to_dbus_fcn) { diff --git a/src/libnm-core-intern/nm-core-internal.h b/src/libnm-core-intern/nm-core-internal.h index d32f782f75..b539fb1a73 100644 --- a/src/libnm-core-intern/nm-core-internal.h +++ b/src/libnm-core-intern/nm-core-internal.h @@ -758,15 +758,9 @@ struct _NMSettInfoProperty { * MAC address length. */ guint8 direct_set_string_mac_address_len : 5; - /* Currently, properties that set property_type->direct_type only have to_dbus_fcn() - * implemented "the direct way". For the property setter, they still call g_object_set(). - * In the future, also other operations, like from_dbus_fcn() should be implemented - * by direct access (thereby, bypassing g_object_set()). - * - * A "direct_has_special_setter" property does something unusual, that will require special attention - * in the future, when we implement more functionality regarding the setter. It has no effect, - * except of marking those properties and serve as a reminder that special care needs to be taken. */ - bool direct_has_special_setter : 1; + /* If non-zero, this is the addr-family (AF_INET/AF_INET6) for normalizing an IP + * address. */ + guint8 direct_set_string_ip_address_addr_family : 5; /* Usually, properties that are set to the default value for the GParamSpec * are not serialized to GVariant (and NULL is returned by to_dbus_data().