From 5d0d13f57010b4eadcd547a29ec5a1f88ac07668 Mon Sep 17 00:00:00 2001 From: Antonio Cardace Date: Fri, 29 May 2020 17:53:02 +0200 Subject: [PATCH 1/2] platform: add support for local routes Also update unit tests. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/407 https://bugzilla.redhat.com/show_bug.cgi?id=1821787 --- libnm-core/nm-core-internal.h | 2 +- .../nm-libnm-core-utils.c | 30 +++++++++++++ .../nm-libnm-core-utils.h | 4 ++ libnm-core/nm-setting-ip-config.c | 43 ++++++++++++++++++- libnm-core/nm-setting-ip-config.h | 1 + libnm-core/tests/test-general.c | 5 +++ src/nm-ip4-config.c | 20 ++++++++- src/platform/nm-linux-platform.c | 10 +++-- src/platform/nm-platform.c | 24 ++++++++++- src/platform/nm-platform.h | 40 +++++++++++++++++ src/platform/tests/test-route.c | 4 +- .../plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 38 +++++++++++++++- .../plugins/ifcfg-rh/nms-ifcfg-rh-writer.c | 8 ++++ .../route-test-wired-static-routes | 4 ++ .../route-test-wired-static-routes-legacy | 1 + .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 18 +++++++- .../tests/keyfiles/Test_Wired_Connection | 4 +- .../keyfile/tests/test-keyfile-settings.c | 9 +++- 18 files changed, 248 insertions(+), 17 deletions(-) diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index 6d7661b916..1d67b09408 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -347,7 +347,7 @@ const char **_nm_ip_address_get_attribute_names (const NMIPAddress *addr, gboole void _nm_setting_wired_clear_s390_options (NMSettingWired *setting); -gboolean _nm_ip_route_attribute_validate_all (const NMIPRoute *route); +gboolean _nm_ip_route_attribute_validate_all (const NMIPRoute *route, GError **error); const char **_nm_ip_route_get_attribute_names (const NMIPRoute *route, gboolean sorted, guint *out_length); GHashTable *_nm_ip_route_get_attributes (NMIPRoute *route); diff --git a/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.c b/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.c index df2f2e7769..8d2ea09cdf 100644 --- a/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.c +++ b/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.c @@ -6,6 +6,8 @@ #include "nm-common-macros.h" +#include + /*****************************************************************************/ gboolean @@ -178,3 +180,31 @@ nm_client_permission_result_to_string (NMClientPermissionResult permission) nm_assert_not_reached (); return NULL; } + +NM_UTILS_STRING_TABLE_LOOKUP_DEFINE ( + nm_utils_route_type_by_name, + guint8, + { nm_assert (name); }, + { return RTN_UNSPEC; }, + { "blackhole", RTN_BLACKHOLE }, + { "broadcast", RTN_BROADCAST }, + { "local", RTN_LOCAL }, + { "multicast", RTN_MULTICAST }, + { "nat", RTN_NAT }, + { "prohibit", RTN_PROHIBIT }, + { "throw", RTN_THROW }, + { "unicast", RTN_UNICAST }, + { "unreachable", RTN_UNREACHABLE }, +); + +NM_UTILS_ENUM2STR_DEFINE (nm_utils_route_type2str, guint8, + NM_UTILS_ENUM2STR (RTN_BLACKHOLE, "blackhole"), + NM_UTILS_ENUM2STR (RTN_BROADCAST, "broadcast"), + NM_UTILS_ENUM2STR (RTN_LOCAL, "local"), + NM_UTILS_ENUM2STR (RTN_MULTICAST, "multicast"), + NM_UTILS_ENUM2STR (RTN_NAT, "nat"), + NM_UTILS_ENUM2STR (RTN_PROHIBIT, "prohibit"), + NM_UTILS_ENUM2STR (RTN_THROW, "throw"), + NM_UTILS_ENUM2STR (RTN_UNICAST, "unicast"), + NM_UTILS_ENUM2STR (RTN_UNREACHABLE, "unreachable"), +); diff --git a/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.h b/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.h index a35a5e15bf..afc214d4f3 100644 --- a/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.h +++ b/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.h @@ -112,4 +112,8 @@ NMClientPermission nm_auth_permission_from_string (const char *str); NMClientPermissionResult nm_client_permission_result_from_string (const char *nm); const char *nm_client_permission_result_to_string (NMClientPermissionResult permission); +guint8 nm_utils_route_type_by_name (const char *name); + +const char *nm_utils_route_type2str (guint8 val, char *buf, gsize len); + #endif /* __NM_LIBNM_SHARED_UTILS_H__ */ diff --git a/libnm-core/nm-setting-ip-config.c b/libnm-core/nm-setting-ip-config.c index 09a3428362..c750e97e31 100644 --- a/libnm-core/nm-setting-ip-config.c +++ b/libnm-core/nm-setting-ip-config.c @@ -1214,6 +1214,7 @@ static const NMVariantAttributeSpec *const ip_route_attribute_spec[] = { NM_VARIANT_ATTRIBUTE_SPEC_DEFINE (NM_IP_ROUTE_ATTRIBUTE_SRC, G_VARIANT_TYPE_STRING, .v4 = TRUE, .v6 = TRUE, .str_type = 'a', ), NM_VARIANT_ATTRIBUTE_SPEC_DEFINE (NM_IP_ROUTE_ATTRIBUTE_TABLE, G_VARIANT_TYPE_UINT32, .v4 = TRUE, .v6 = TRUE, ), NM_VARIANT_ATTRIBUTE_SPEC_DEFINE (NM_IP_ROUTE_ATTRIBUTE_TOS, G_VARIANT_TYPE_BYTE, .v4 = TRUE, ), + NM_VARIANT_ATTRIBUTE_SPEC_DEFINE (NM_IP_ROUTE_ATTRIBUTE_TYPE, G_VARIANT_TYPE_STRING, .v4 = TRUE, .v6 = TRUE, .str_type = 'T', ), NM_VARIANT_ATTRIBUTE_SPEC_DEFINE (NM_IP_ROUTE_ATTRIBUTE_WINDOW, G_VARIANT_TYPE_UINT32, .v4 = TRUE, .v6 = TRUE, ), NULL, }; @@ -1339,6 +1340,18 @@ nm_ip_route_attribute_validate (const char *name, } break; } + case 'T': /* route type. */ + if (!NM_IN_SET (nm_utils_route_type_by_name (string), + RTN_UNICAST, + RTN_LOCAL)) { + g_set_error (error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("%s is not a valid route type"), + string); + return FALSE; + } + break; default: break; } @@ -1348,22 +1361,48 @@ nm_ip_route_attribute_validate (const char *name, } gboolean -_nm_ip_route_attribute_validate_all (const NMIPRoute *route) +_nm_ip_route_attribute_validate_all (const NMIPRoute *route, GError **error) { GHashTableIter iter; const char *key; GVariant *val; + guint8 u8; g_return_val_if_fail (route, FALSE); + g_return_val_if_fail (!error || !*error, FALSE); if (!route->attributes) return TRUE; g_hash_table_iter_init (&iter, route->attributes); while (g_hash_table_iter_next (&iter, (gpointer *) &key, (gpointer *) &val)) { - if (!nm_ip_route_attribute_validate (key, val, route->family, NULL, NULL)) + if (!nm_ip_route_attribute_validate (key, val, route->family, NULL, error)) return FALSE; } + + if ((val = g_hash_table_lookup (route->attributes, + NM_IP_ROUTE_ATTRIBUTE_TYPE))) { + nm_assert (g_variant_is_of_type (val, G_VARIANT_TYPE_STRING)); + u8 = nm_utils_route_type_by_name (g_variant_get_string (val, NULL)); + + if ( u8 == RTN_LOCAL + && route->family == AF_INET + && (val = g_hash_table_lookup (route->attributes, NM_IP_ROUTE_ATTRIBUTE_SCOPE))) { + nm_assert (g_variant_is_of_type (val, G_VARIANT_TYPE_BYTE)); + u8 = g_variant_get_byte (val); + + if (!NM_IN_SET(u8, + RT_SCOPE_HOST, + RT_SCOPE_NOWHERE)) { + g_set_error (error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("route scope is invalid")); + return FALSE; + } + } + } + return TRUE; } diff --git a/libnm-core/nm-setting-ip-config.h b/libnm-core/nm-setting-ip-config.h index ae77480c08..0c655d2604 100644 --- a/libnm-core/nm-setting-ip-config.h +++ b/libnm-core/nm-setting-ip-config.h @@ -164,6 +164,7 @@ gboolean nm_ip_route_attribute_validate (const char *name, #define NM_IP_ROUTE_ATTRIBUTE_SRC "src" #define NM_IP_ROUTE_ATTRIBUTE_TABLE "table" #define NM_IP_ROUTE_ATTRIBUTE_TOS "tos" +#define NM_IP_ROUTE_ATTRIBUTE_TYPE "type" #define NM_IP_ROUTE_ATTRIBUTE_WINDOW "window" /*****************************************************************************/ diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index a6fb700715..714dcd4236 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -2020,6 +2020,11 @@ test_setting_ip_route_attributes (void) TEST_ATTR ("src", string, "1.2.3.0/24", AF_INET, FALSE, TRUE); TEST_ATTR ("src", string, "fd01::12", AF_INET6, TRUE, TRUE); + TEST_ATTR ("type", string, "local", AF_INET, TRUE, TRUE); + TEST_ATTR ("type", string, "local", AF_INET6, TRUE, TRUE); + TEST_ATTR ("type", string, "unicast", AF_INET, TRUE, TRUE); + TEST_ATTR ("type", string, "unicast", AF_INET6, TRUE, TRUE); + #undef TEST_ATTR } diff --git a/src/nm-ip4-config.c b/src/nm-ip4-config.c index 0feb7b2c3c..62b41478f4 100644 --- a/src/nm-ip4-config.c +++ b/src/nm-ip4-config.c @@ -860,8 +860,26 @@ _nm_ip_config_merge_route_attributes (int addr_family, (dst) = (dflt); \ } G_STMT_END + if ( (variant = nm_ip_route_get_attribute (s_route, NM_IP_ROUTE_ATTRIBUTE_TYPE)) + && g_variant_is_of_type (variant, G_VARIANT_TYPE_STRING)) { + guint8 type; + + type = nm_utils_route_type_by_name (g_variant_get_string (variant, NULL)); + nm_assert (NM_IN_SET (type, + RTN_UNICAST, + RTN_LOCAL)); + + r->type_coerced = nm_platform_route_type_coerce (type); + } else + r->type_coerced = nm_platform_route_type_coerce (RTN_UNICAST); + GET_ATTR (NM_IP_ROUTE_ATTRIBUTE_TABLE, table, UINT32, uint32, 0); - r->table_coerced = nm_platform_route_table_coerce (table ?: (route_table ?: RT_TABLE_MAIN)); + + if ( !table + && r->type_coerced == nm_platform_route_type_coerce (RTN_LOCAL)) + r->table_coerced = nm_platform_route_table_coerce (RT_TABLE_LOCAL); + else + r->table_coerced = nm_platform_route_table_coerce (table ?: (route_table ?: RT_TABLE_MAIN)); if (addr_family == AF_INET) { guint8 scope; diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index e2c45c8874..710a6f91f1 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -3257,14 +3257,16 @@ _new_from_nl_route (struct nlmsghdr *nlh, gboolean id_only) rtm = nlmsg_data (nlh); /***************************************************************** - * only handle ~normal~ routes. + * only handle ~supported~ routes. *****************************************************************/ if (!NM_IN_SET (rtm->rtm_family, AF_INET, AF_INET6)) return NULL; - if (rtm->rtm_type != RTN_UNICAST) - return NULL; + if (!NM_IN_SET (rtm->rtm_type, + RTN_UNICAST, + RTN_LOCAL)) + return NULL; if (nlmsg_parse_arr (nlh, sizeof (struct rtmsg), @@ -4491,7 +4493,7 @@ _nl_msg_new_route (int nlmsg_type, .rtm_scope = is_v4 ? nm_platform_route_scope_inv (obj->ip4_route.scope_inv) : RT_SCOPE_NOWHERE, - .rtm_type = RTN_UNICAST, + .rtm_type = nm_platform_route_type_uncoerce (NMP_OBJECT_CAST_IP_ROUTE (obj)->type_coerced), .rtm_flags = obj->ip_route.r_rtm_flags & ((unsigned) (RTNH_F_ONLINK)), .rtm_dst_len = obj->ip_route.plen, .rtm_src_len = is_v4 diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index c023f43574..c8b8c6e869 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -4518,8 +4518,12 @@ _ip_route_scope_inv_get_normalized (const NMPlatformIP4Route *route) * so that the default equals zero (~(RT_SCOPE_NOWHERE)). **/ if (route->scope_inv == 0) { - return nm_platform_route_scope_inv (!route->gateway - ? RT_SCOPE_LINK : RT_SCOPE_UNIVERSE); + if (route->type_coerced == nm_platform_route_type_coerce (RTN_LOCAL)) + return nm_platform_route_scope_inv (RT_SCOPE_HOST); + else { + return nm_platform_route_scope_inv (!route->gateway + ? RT_SCOPE_LINK : RT_SCOPE_UNIVERSE); + } } return route->scope_inv; } @@ -6079,6 +6083,7 @@ nm_platform_ip4_route_to_string (const NMPlatformIP4Route *route, char *buf, gsi char s_network[INET_ADDRSTRLEN], s_gateway[INET_ADDRSTRLEN]; char s_pref_src[INET_ADDRSTRLEN]; char str_dev[TO_STRING_DEV_BUF_SIZE]; + char str_type[30]; char str_table[30]; char str_scope[30], s_source[50]; char str_tos[32], str_window[32], str_cwnd[32], str_initcwnd[32], str_initrwnd[32], str_mtu[32]; @@ -6093,6 +6098,7 @@ nm_platform_ip4_route_to_string (const NMPlatformIP4Route *route, char *buf, gsi _to_string_dev (NULL, route->ifindex, str_dev, sizeof (str_dev)); g_snprintf (buf, len, + "%s" /* type */ "%s" /* table */ "%s/%d" " via %s" @@ -6110,6 +6116,7 @@ nm_platform_ip4_route_to_string (const NMPlatformIP4Route *route, char *buf, gsi "%s" /* initrwnd */ "%s" /* mtu */ "", + route->type_coerced ? nm_sprintf_buf (str_type, "type %s ", nm_utils_route_type2str (nm_platform_route_type_uncoerce (route->type_coerced), NULL, 0)) : "", route->table_coerced ? nm_sprintf_buf (str_table, "table %u ", nm_platform_route_table_uncoerce (route->table_coerced, FALSE)) : "", s_network, route->plen, @@ -6152,6 +6159,7 @@ nm_platform_ip6_route_to_string (const NMPlatformIP6Route *route, char *buf, gsi char s_pref_src[INET6_ADDRSTRLEN]; char s_src_all[INET6_ADDRSTRLEN + 40]; char s_src[INET6_ADDRSTRLEN]; + char str_type[30]; char str_table[30]; char str_pref[40]; char str_pref2[30]; @@ -6178,6 +6186,7 @@ nm_platform_ip6_route_to_string (const NMPlatformIP6Route *route, char *buf, gsi _to_string_dev (NULL, route->ifindex, str_dev, sizeof (str_dev)); g_snprintf (buf, len, + "%s" /* type */ "%s" /* table */ "%s/%d" " via %s" @@ -6195,6 +6204,7 @@ nm_platform_ip6_route_to_string (const NMPlatformIP6Route *route, char *buf, gsi "%s" /* mtu */ "%s" /* pref */ "", + route->type_coerced ? nm_sprintf_buf (str_type, "type %s ", nm_utils_route_type2str (nm_platform_route_type_uncoerce (route->type_coerced), NULL, 0)) : "", route->table_coerced ? nm_sprintf_buf (str_table, "table %u ", nm_platform_route_table_uncoerce (route->table_coerced, FALSE)) : "", s_network, route->plen, @@ -7274,6 +7284,7 @@ nm_platform_ip4_route_hash_update (const NMPlatformIP4Route *obj, NMPlatformIPRo break; case NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID: nm_hash_update_vals (h, + obj->type_coerced, nm_platform_route_table_uncoerce (obj->table_coerced, TRUE), nm_utils_ip4_address_clear_host_address (obj->network, obj->plen), obj->plen, @@ -7301,6 +7312,7 @@ nm_platform_ip4_route_hash_update (const NMPlatformIP4Route *obj, NMPlatformIPRo break; case NM_PLATFORM_IP_ROUTE_CMP_TYPE_SEMANTICALLY: nm_hash_update_vals (h, + obj->type_coerced, nm_platform_route_table_uncoerce (obj->table_coerced, TRUE), obj->ifindex, nm_utils_ip4_address_clear_host_address (obj->network, obj->plen), @@ -7327,6 +7339,7 @@ nm_platform_ip4_route_hash_update (const NMPlatformIP4Route *obj, NMPlatformIPRo break; case NM_PLATFORM_IP_ROUTE_CMP_TYPE_FULL: nm_hash_update_vals (h, + obj->type_coerced, obj->table_coerced, obj->ifindex, obj->network, @@ -7369,6 +7382,7 @@ nm_platform_ip4_route_cmp (const NMPlatformIP4Route *a, const NMPlatformIP4Route NM_CMP_FIELD (a, b, tos); if (cmp_type == NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID) { NM_CMP_FIELD (a, b, ifindex); + NM_CMP_FIELD (a, b, type_coerced); NM_CMP_DIRECT (nmp_utils_ip_config_source_round_trip_rtprot (a->rt_source), nmp_utils_ip_config_source_round_trip_rtprot (b->rt_source)); NM_CMP_DIRECT (_ip_route_scope_inv_get_normalized (a), @@ -7392,6 +7406,7 @@ nm_platform_ip4_route_cmp (const NMPlatformIP4Route *a, const NMPlatformIP4Route break; case NM_PLATFORM_IP_ROUTE_CMP_TYPE_SEMANTICALLY: case NM_PLATFORM_IP_ROUTE_CMP_TYPE_FULL: + NM_CMP_FIELD (a, b, type_coerced); if (cmp_type == NM_PLATFORM_IP_ROUTE_CMP_TYPE_SEMANTICALLY) { NM_CMP_DIRECT (nm_platform_route_table_uncoerce (a->table_coerced, TRUE), nm_platform_route_table_uncoerce (b->table_coerced, TRUE)); @@ -7454,6 +7469,7 @@ nm_platform_ip6_route_hash_update (const NMPlatformIP6Route *obj, NMPlatformIPRo break; case NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID: nm_hash_update_vals (h, + obj->type_coerced, nm_platform_route_table_uncoerce (obj->table_coerced, TRUE), *nm_utils_ip6_address_clear_host_address (&a1, &obj->network, obj->plen), obj->plen, @@ -7466,6 +7482,7 @@ nm_platform_ip6_route_hash_update (const NMPlatformIP6Route *obj, NMPlatformIPRo break; case NM_PLATFORM_IP_ROUTE_CMP_TYPE_SEMANTICALLY: nm_hash_update_vals (h, + obj->type_coerced, nm_platform_route_table_uncoerce (obj->table_coerced, TRUE), obj->ifindex, *nm_utils_ip6_address_clear_host_address (&a1, &obj->network, obj->plen), @@ -7493,6 +7510,7 @@ nm_platform_ip6_route_hash_update (const NMPlatformIP6Route *obj, NMPlatformIPRo break; case NM_PLATFORM_IP_ROUTE_CMP_TYPE_FULL: nm_hash_update_vals (h, + obj->type_coerced, obj->table_coerced, obj->ifindex, obj->network, @@ -7537,11 +7555,13 @@ nm_platform_ip6_route_cmp (const NMPlatformIP6Route *a, const NMPlatformIP6Route NM_CMP_FIELD (a, b, src_plen); if (cmp_type == NM_PLATFORM_IP_ROUTE_CMP_TYPE_ID) { NM_CMP_FIELD (a, b, ifindex); + NM_CMP_FIELD (a, b, type_coerced); NM_CMP_FIELD_IN6ADDR (a, b, gateway); } break; case NM_PLATFORM_IP_ROUTE_CMP_TYPE_SEMANTICALLY: case NM_PLATFORM_IP_ROUTE_CMP_TYPE_FULL: + NM_CMP_FIELD (a, b, type_coerced); if (cmp_type == NM_PLATFORM_IP_ROUTE_CMP_TYPE_SEMANTICALLY) { NM_CMP_DIRECT (nm_platform_route_table_uncoerce (a->table_coerced, TRUE), nm_platform_route_table_uncoerce (b->table_coerced, TRUE)); diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index aa3551a6d1..3e6ef84cf0 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -468,6 +468,13 @@ typedef union { * table. Use nm_platform_route_table_coerce()/nm_platform_route_table_uncoerce(). */ \ guint32 table_coerced; \ \ + /* rtm_type. + * + * This is not the original type, if type_coerced is 0 then + * it means RTN_UNSPEC otherwise the type value is preserved. + * */ \ + guint8 type_coerced; \ + \ /*end*/ typedef struct { @@ -1285,6 +1292,39 @@ _nm_platform_uint8_inv (guint8 scope) return (guint8) ~scope; } +/** + * nm_platform_route_type_coerce: + * @table: the route type, in its original value. + * + * Returns: returns the coerced type, that can be stored in + * NMPlatformIPRoute.type_coerced. + */ +static inline guint8 +nm_platform_route_type_coerce (guint8 type) +{ + switch (type) { + case 0 /* RTN_UNSPEC */: + return 1; + case 1 /* RTN_UNICAST */: + return 0; + default: + return type; + } +} + +/** + * nm_platform_route_type_uncoerce: + * @table: the type table, in its coerced value + * + * Returns: reverts the coerced type in NMPlatformIPRoute.type_coerced + * to the original value as kernel understands it. + */ +static inline guint8 +nm_platform_route_type_uncoerce (guint8 type_coerced) +{ + return nm_platform_route_type_coerce (type_coerced); +} + gboolean nm_platform_get_use_udev (NMPlatform *self); gboolean nm_platform_get_log_with_ptr (NMPlatform *self); diff --git a/src/platform/tests/test-route.c b/src/platform/tests/test-route.c index f074ca69cf..19debfb501 100644 --- a/src/platform/tests/test-route.c +++ b/src/platform/tests/test-route.c @@ -328,13 +328,13 @@ test_ip6_route (void) g_assert (nm_platform_ip6_address_add (NM_PLATFORM_GET, ifindex, pref_src, 128, in6addr_any, NM_PLATFORM_LIFETIME_PERMANENT, NM_PLATFORM_LIFETIME_PERMANENT, 0)); - accept_signals (route_added, 0, 1); + accept_signals (route_added, 0, 2); _wait_for_ipv6_addr_non_tentative (NM_PLATFORM_GET, 200, ifindex, 1, &pref_src); /* Add route to gateway */ nmtstp_ip6_route_add (NM_PLATFORM_GET, ifindex, NM_IP_CONFIG_SOURCE_USER, gateway, 128, in6addr_any, in6addr_any, metric, mss); - accept_signal (route_added); + accept_signals (route_added, 0, 3); /* Add route */ g_assert (!nmtstp_ip6_route_get (NM_PLATFORM_GET, ifindex, &network, plen, metric, NULL, 0)); 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 d65ae55551..798b833602 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -800,6 +800,7 @@ typedef struct { union { guint8 uint8; guint32 uint32; + const char *str; struct { guint32 uint32; bool lock:1; @@ -815,6 +816,7 @@ typedef struct { enum { /* route attributes */ + PARSE_LINE_ATTR_ROUTE_TYPE, PARSE_LINE_ATTR_ROUTE_TABLE, PARSE_LINE_ATTR_ROUTE_SRC, PARSE_LINE_ATTR_ROUTE_FROM, @@ -844,6 +846,7 @@ enum { #define PARSE_LINE_TYPE_IFNAME 'i' #define PARSE_LINE_TYPE_FLAG 'f' #define PARSE_LINE_TYPE_ROUTE_SCOPE 'S' +#define PARSE_LINE_TYPE_STRING 's' /** * parse_route_line: @@ -875,6 +878,8 @@ parse_route_line (const char *line, GError **error) { static const ParseLineInfo parse_infos[] = { + [PARSE_LINE_ATTR_ROUTE_TYPE] = { .key = NM_IP_ROUTE_ATTRIBUTE_TYPE, + .type = PARSE_LINE_TYPE_STRING, }, [PARSE_LINE_ATTR_ROUTE_TABLE] = { .key = NM_IP_ROUTE_ATTRIBUTE_TABLE, .type = PARSE_LINE_TYPE_UINT32, }, [PARSE_LINE_ATTR_ROUTE_SRC] = { .key = NM_IP_ROUTE_ATTRIBUTE_SRC, @@ -1010,6 +1015,23 @@ parse_route_line (const char *line, } } + p_info = &parse_infos[PARSE_LINE_ATTR_ROUTE_TYPE]; + p_data = &parse_datas[PARSE_LINE_ATTR_ROUTE_TYPE]; + if ( !p_data->has + && NM_IN_STRSET (w, + "local", + "unicast", + "broadcast" + "multicast", + "throw", + "unreachable", + "prohibit", + "blackhole", + "nat")) { + p_data->has = TRUE; + goto parse_line_type_string; + } + /* "to" is also accepted unqualified... (once) */ p_info = &parse_infos[PARSE_LINE_ATTR_ROUTE_TO]; p_data = &parse_datas[PARSE_LINE_ATTR_ROUTE_TO]; @@ -1160,6 +1182,15 @@ parse_line_type_addr_with_prefix: i_words++; goto next; +parse_line_type_string: + s = words[i_words]; + if (!s) + goto err_word_missing_argument; + + p_data->v.str = s; + i_words++; + goto next; + err_word_missing_argument: g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, "Missing argument for \"%s\"", w); @@ -1253,13 +1284,18 @@ next: p_info->key, g_variant_new_boolean (TRUE)); break; + case PARSE_LINE_TYPE_STRING: + nm_ip_route_set_attribute (route, + p_info->key, + g_variant_new_string (p_data->v.str)); + break; default: nm_assert_not_reached (); break; } } - nm_assert (_nm_ip_route_attribute_validate_all (route)); + nm_assert (_nm_ip_route_attribute_validate_all (route, NULL)); NM_SET_OUT (out_route, g_steal_pointer (&route)); return 0; 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 199e8e4e64..e6526944c7 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c @@ -2099,7 +2099,15 @@ get_route_attributes_string (NMIPRoute *route, int family) str = g_string_new (""); + attr = nm_ip_route_get_attribute (route, NM_IP_ROUTE_ATTRIBUTE_TYPE); + if ( attr + && nm_ip_route_attribute_validate (NM_IP_ROUTE_ATTRIBUTE_TYPE, attr, family, NULL, NULL)) + g_string_append_printf (str, "%s ", g_variant_get_string (attr, NULL)); + for (i = 0; i < len; i++) { + if (nm_streq (names[i], NM_IP_ROUTE_ATTRIBUTE_TYPE)) + continue; + attr = nm_ip_route_get_attribute (route, names[i]); if (!nm_ip_route_attribute_validate (names[i], attr, family, NULL, NULL)) diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/route-test-wired-static-routes b/src/settings/plugins/ifcfg-rh/tests/network-scripts/route-test-wired-static-routes index 5d02c62ef3..9c05417ec2 100644 --- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/route-test-wired-static-routes +++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/route-test-wired-static-routes @@ -13,3 +13,7 @@ NETMASK2=255.255.255.255 GATEWAY2=192.168.1.8 METRIC2=3 OPTIONS2="mtu lock 9000 cwnd 12 src 1.1.1.1 tos 0x28 onlink window 30000 initcwnd lock 13 initrwnd 14 scope link" + +ADDRESS3=1.2.3.4 +NETMASK3=255.255.255.255 +OPTIONS3="local scope host" diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/route-test-wired-static-routes-legacy b/src/settings/plugins/ifcfg-rh/tests/network-scripts/route-test-wired-static-routes-legacy index faa247d86a..1fef7e9792 100644 --- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/route-test-wired-static-routes-legacy +++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/route-test-wired-static-routes-legacy @@ -6,3 +6,4 @@ 43.53.0.0/16 metric 3 via 7.7.7.7 dev eth2 cwnd 14 mtu lock 9000 initrwnd 20 window lock 10000 initcwnd lock 42 src 1.2.3.4 7.7.7.8/32 via (null) metric 18 +local 1.2.3.4 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 a5025f3b93..40d1bb8c53 100644 --- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -1323,7 +1323,7 @@ test_read_wired_static_routes (void) g_assert_cmpstr (nm_setting_ip_config_get_method (s_ip4), ==, NM_SETTING_IP4_CONFIG_METHOD_MANUAL); /* Routes */ - g_assert_cmpint (nm_setting_ip_config_get_num_routes (s_ip4), ==, 3); + g_assert_cmpint (nm_setting_ip_config_get_num_routes (s_ip4), ==, 4); ip4_route = nm_setting_ip_config_get_route (s_ip4, 0); g_assert (ip4_route); @@ -1367,6 +1367,13 @@ test_read_wired_static_routes (void) nmtst_assert_route_attribute_boolean (ip4_route, NM_IP_ROUTE_ATTRIBUTE_ONLINK, TRUE); nmtst_assert_route_attribute_byte (ip4_route, NM_IP_ROUTE_ATTRIBUTE_SCOPE, 253); + ip4_route = nm_setting_ip_config_get_route (s_ip4, 3); + g_assert (ip4_route); + g_assert_cmpstr (nm_ip_route_get_dest (ip4_route), ==, "1.2.3.4"); + g_assert_cmpint (nm_ip_route_get_prefix (ip4_route), ==, 32); + nmtst_assert_route_attribute_string (ip4_route, NM_IP_ROUTE_ATTRIBUTE_TYPE, "local"); + nmtst_assert_route_attribute_byte (ip4_route, NM_IP_ROUTE_ATTRIBUTE_SCOPE, 254); + g_object_unref (connection); } @@ -1402,7 +1409,7 @@ test_read_wired_static_routes_legacy (void) g_assert_cmpstr (nm_setting_ip_config_get_method (s_ip4), ==, NM_SETTING_IP4_CONFIG_METHOD_MANUAL); /* Routes */ - g_assert_cmpint (nm_setting_ip_config_get_num_routes (s_ip4), ==, 4); + g_assert_cmpint (nm_setting_ip_config_get_num_routes (s_ip4), ==, 5); /* Route #1 */ ip4_route = nm_setting_ip_config_get_route (s_ip4, 0); @@ -1443,6 +1450,13 @@ test_read_wired_static_routes_legacy (void) g_assert_cmpstr (nm_ip_route_get_next_hop (ip4_route), ==, NULL); g_assert_cmpint (nm_ip_route_get_metric (ip4_route), ==, 18); + + /* Route #5 */ + ip4_route = nm_setting_ip_config_get_route (s_ip4, 4); + g_assert (ip4_route != NULL); + g_assert_cmpstr (nm_ip_route_get_dest (ip4_route), ==, "1.2.3.4"); + nmtst_assert_route_attribute_string (ip4_route, NM_IP_ROUTE_ATTRIBUTE_TYPE, "local"); + g_object_unref (connection); } diff --git a/src/settings/plugins/keyfile/tests/keyfiles/Test_Wired_Connection b/src/settings/plugins/keyfile/tests/keyfiles/Test_Wired_Connection index 1e62f4b3ef..f9ccc003df 100644 --- a/src/settings/plugins/keyfile/tests/keyfiles/Test_Wired_Connection +++ b/src/settings/plugins/keyfile/tests/keyfiles/Test_Wired_Connection @@ -36,7 +36,9 @@ routes9=1.1.1.9/19,0.0.0.0,0 route10=1.1.1.10/21,,0 routes10=1.1.1.10/20,,0 routes11=1.1.1.11/21,,21 -routes11_options=cwnd=10,lock-cwnd=true,mtu=1430,src=7.7.7.7 +routes11_options=cwnd=10,lock-cwnd=true,mtu=1430,src=7.7.7.7,type=unicast +routes12=1.2.3.4/32 +routes12_options=type=local address30=1.2.3.30/24 addresses30=1.2.3.30/25 addresses31=1.2.3.31/25 diff --git a/src/settings/plugins/keyfile/tests/test-keyfile-settings.c b/src/settings/plugins/keyfile/tests/test-keyfile-settings.c index 1a9482e5b6..98820272c1 100644 --- a/src/settings/plugins/keyfile/tests/test-keyfile-settings.c +++ b/src/settings/plugins/keyfile/tests/test-keyfile-settings.c @@ -274,7 +274,7 @@ test_read_valid_wired_connection (void) g_assert_cmpstr (nm_setting_ip_config_get_gateway (s_ip4), ==, "2.3.4.6"); /* IPv4 routes */ - g_assert_cmpint (nm_setting_ip_config_get_num_routes (s_ip4), ==, 13); + g_assert_cmpint (nm_setting_ip_config_get_num_routes (s_ip4), ==, 14); check_ip_route (s_ip4, 0, "5.6.7.8", 32, NULL, -1); check_ip_route (s_ip4, 1, "1.2.3.0", 24, "2.3.4.8", 99); check_ip_route (s_ip4, 2, "1.1.1.2", 12, NULL, -1); @@ -288,6 +288,7 @@ test_read_valid_wired_connection (void) check_ip_route (s_ip4, 10, "1.1.1.10", 21, NULL, 0); check_ip_route (s_ip4, 11, "1.1.1.10", 20, NULL, 0); check_ip_route (s_ip4, 12, "1.1.1.11", 21, NULL, 21); + check_ip_route (s_ip4, 13, "1.2.3.4", 32, NULL, -1); /* Route attributes */ route = nm_setting_ip_config_get_route (s_ip4, 12); @@ -297,6 +298,12 @@ test_read_valid_wired_connection (void) nmtst_assert_route_attribute_uint32 (route, NM_IP_ROUTE_ATTRIBUTE_MTU, 1430); nmtst_assert_route_attribute_boolean (route, NM_IP_ROUTE_ATTRIBUTE_LOCK_CWND, TRUE); nmtst_assert_route_attribute_string (route, NM_IP_ROUTE_ATTRIBUTE_SRC, "7.7.7.7"); + nmtst_assert_route_attribute_string (route, NM_IP_ROUTE_ATTRIBUTE_TYPE, "unicast"); + + route = nm_setting_ip_config_get_route (s_ip4, 13); + g_assert (route); + + nmtst_assert_route_attribute_string (route, NM_IP_ROUTE_ATTRIBUTE_TYPE, "local"); s_ip6 = nm_connection_get_setting_ip6_config (connection); g_assert (s_ip6); From 7781f7843546aeea26ac8a7b67af7707768f1fc0 Mon Sep 17 00:00:00 2001 From: Antonio Cardace Date: Tue, 9 Jun 2020 11:29:42 +0200 Subject: [PATCH 2/2] setting-ip-config: validate route attributes in verify() It's better to verify these route attributes so that the user can be notified early if something is not supported or invalid. The downside is that some incorrect profiles (with invalid route attributes) that previously would work since this commit will not anymore as the incorrect bits don't get ignored but rejected instead. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/407 https://bugzilla.redhat.com/show_bug.cgi?id=1821787 --- libnm-core/nm-setting-ip-config.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/libnm-core/nm-setting-ip-config.c b/libnm-core/nm-setting-ip-config.c index c750e97e31..bc81129177 100644 --- a/libnm-core/nm-setting-ip-config.c +++ b/libnm-core/nm-setting-ip-config.c @@ -5059,6 +5059,7 @@ verify (NMSetting *setting, NMConnection *connection, GError **error) /* Validate routes */ for (i = 0; i < priv->routes->len; i++) { + gs_free_error GError *local = NULL; NMIPRoute *route = (NMIPRoute *) priv->routes->pdata[i]; if (nm_ip_route_get_family (route) != NM_SETTING_IP_CONFIG_GET_FAMILY (setting)) { @@ -5070,6 +5071,19 @@ verify (NMSetting *setting, NMConnection *connection, GError **error) g_prefix_error (error, "%s.%s: ", nm_setting_get_name (setting), NM_SETTING_IP_CONFIG_ROUTES); return FALSE; } + + if (!_nm_ip_route_attribute_validate_all (route, &local)) { + g_set_error (error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("invalid attribute: %s"), + local->message); + g_prefix_error (error, + "%s.%s: ", + nm_setting_get_name (setting), + NM_SETTING_IP_CONFIG_ROUTES); + return FALSE; + } } if (priv->routing_rules) {