Commit 5f995123 authored by Thomas Haller's avatar Thomas Haller

core: prevent invalid routes in NMIP4Config/NMIP6Config

Kernel requires that the host part of a route (based on network/plen)
is zero. Routes with non-zero host part don't really exist.

In settings (NMIPRoute), we don't enforce that. Hence we must ensure
that we don't let such invalid routes into NMIP4Config/NMIP6Config.

Also at other places where we obtain routes from untrusted sources,
we must sanitize them first.

Also add an assertion to catch such bugs.
parent 5e5aa39c
......@@ -344,12 +344,13 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx,
if (sd_dhcp_route_get_destination (routes[i], &a) < 0)
continue;
route.network = a.s_addr;
if ( sd_dhcp_route_get_destination_prefix_length (routes[i], &plen) < 0
|| plen > 32)
continue;
route.plen = plen;
route.network = nm_utils_ip4_address_clear_host_address (a.s_addr, plen);
if (sd_dhcp_route_get_gateway (routes[i], &a) < 0)
continue;
......
......@@ -86,7 +86,7 @@ ip4_process_dhcpcd_rfc3442_routes (const char *iface,
} else {
_LOG2I (LOGD_DHCP4, iface, " classless static route %s/%d gw %s", *r, rt_cidr, *(r + 1));
memset (&route, 0, sizeof (route));
route.network = rt_addr;
route.network = nm_utils_ip4_address_clear_host_address (rt_addr, rt_cidr);
route.plen = rt_cidr;
route.gateway = rt_route;
route.rt_source = NM_IP_CONFIG_SOURCE_DHCP;
......@@ -144,8 +144,7 @@ process_dhclient_rfc3442_route (const char **octets,
goto error;
}
g_free (str_addr);
tmp_addr &= nm_utils_ip4_prefix_to_netmask ((guint32) tmp);
route->network = tmp_addr;
route->network = nm_utils_ip4_address_clear_host_address (tmp_addr, tmp);
}
/* Handle next hop */
......@@ -327,6 +326,8 @@ process_classful_routes (const char *iface,
route.rt_source = NM_IP_CONFIG_SOURCE_DHCP;
route.metric = priority;
route.network = nm_utils_ip4_address_clear_host_address (route.network, route.plen);
nm_ip4_config_add_route (ip4_config, &route);
_LOG2I (LOGD_DHCP, iface, " static route %s",
nm_platform_ip4_route_to_string (&route, NULL, 0));
......
......@@ -46,6 +46,16 @@ G_STATIC_ASSERT (G_MAXUINT >= 0xFFFFFFFF);
/*****************************************************************************/
static gboolean
_route_valid (const NMPlatformIP4Route *r)
{
return r
&& r->plen <= 32
&& r->network == nm_utils_ip4_address_clear_host_address (r->network, r->plen);
}
/*****************************************************************************/
gboolean
nm_ip_config_obj_id_equal_ip4_address (const NMPlatformIP4Address *a,
const NMPlatformIP4Address *b)
......@@ -851,6 +861,8 @@ nm_ip4_config_merge_setting (NMIP4Config *self, NMSettingIPConfig *setting, guin
route.metric = nm_ip_route_get_metric (s_route);
route.rt_source = NM_IP_CONFIG_SOURCE_USER;
route.network = nm_utils_ip4_address_clear_host_address (route.network, route.plen);
merge_route_attributes (s_route, &route);
_add_route (self, NULL, &route);
}
......@@ -1987,6 +1999,10 @@ _add_route (NMIP4Config *self, const NMPObject *obj_new, const NMPlatformIP4Rout
{
NMIP4ConfigPrivate *priv = NM_IP4_CONFIG_GET_PRIVATE (self);
nm_assert ((!new) != (!obj_new));
nm_assert (!new || _route_valid (new));
nm_assert (!obj_new || _route_valid (NMP_OBJECT_CAST_IP4_ROUTE (obj_new)));
if (_nm_ip_config_add_obj (priv->multi_idx,
&priv->idx_ip4_routes_,
priv->ifindex,
......@@ -2746,6 +2762,8 @@ out_addresses_cached:
nm_ip_config_iter_ip4_route_for_each (&ipconf_iter, self, &route) {
GVariantBuilder route_builder;
nm_assert (_route_valid (route));
g_variant_builder_init (&route_builder, G_VARIANT_TYPE ("a{sv}"));
g_variant_builder_add (&route_builder, "{sv}",
"dest",
......
......@@ -41,6 +41,20 @@
/*****************************************************************************/
static gboolean
_route_valid (const NMPlatformIP6Route *r)
{
struct in6_addr n;
return r
&& r->plen <= 128
&& (memcmp (&r->network,
nm_utils_ip6_address_clear_host_address (&n, &r->network, r->plen),
sizeof (n)) == 0);
}
/*****************************************************************************/
typedef struct {
bool never_default:1;
guint32 mss;
......@@ -671,6 +685,8 @@ nm_ip6_config_merge_setting (NMIP6Config *self, NMSettingIPConfig *setting, guin
route.metric = nm_ip_route_get_metric (s_route);
route.rt_source = NM_IP_CONFIG_SOURCE_USER;
nm_utils_ip6_address_clear_host_address (&route.network, &route.network, route.plen);
merge_route_attributes (s_route, &route);
_add_route (self, NULL, &route);
}
......@@ -1703,6 +1719,10 @@ _add_route (NMIP6Config *self, const NMPObject *obj_new, const NMPlatformIP6Rout
{
NMIP6ConfigPrivate *priv = NM_IP6_CONFIG_GET_PRIVATE (self);
nm_assert ((!new) != (!obj_new));
nm_assert (!new || _route_valid (new));
nm_assert (!obj_new || _route_valid (NMP_OBJECT_CAST_IP6_ROUTE (obj_new)));
if (_nm_ip_config_add_obj (priv->multi_idx,
&priv->idx_ip6_routes_,
priv->ifindex,
......@@ -2327,6 +2347,8 @@ out_addresses_cached:
nm_ip_config_iter_ip6_route_for_each (&ipconf_iter, self, &route) {
GVariantBuilder route_builder;
nm_assert (_route_valid (route));
g_variant_builder_init (&route_builder, G_VARIANT_TYPE ("a{sv}"));
g_variant_builder_add (&route_builder, "{sv}",
"dest",
......
......@@ -75,7 +75,7 @@ test_subtract (void)
const NMPlatformIP4Route *test_route;
const char *expected_addr = "192.168.1.12";
guint32 expected_addr_plen = 24;
const char *expected_route_dest = "8.7.6.5";
const char *expected_route_dest = "8.0.0.0";
guint32 expected_route_plen = 8;
const char *expected_route_next_hop = "192.168.1.1";
guint32 expected_ns1 = nmtst_inet4_from_string ("8.8.8.8");
......@@ -232,7 +232,7 @@ test_add_route_with_source (void)
a = nmtst_ip4_config_new (1);
/* Test that a higher priority source is not overwritten */
route = *nmtst_platform_ip4_route ("1.2.3.4", 24, "1.2.3.1");
route = *nmtst_platform_ip4_route ("1.2.3.0", 24, "1.2.3.1");
route.rt_source = NM_IP_CONFIG_SOURCE_USER;
nm_ip4_config_add_route (a, &route);
......
......@@ -37,8 +37,8 @@ build_test_config (void)
config = nmtst_ip6_config_new (1);
nm_ip6_config_add_address (config, nmtst_platform_ip6_address ("abcd:1234:4321::cdde", "1:2:3:4::5", 64));
nm_ip6_config_add_route (config, nmtst_platform_ip6_route ("abcd:1234:4321::", 24, "abcd:1234:4321:cdde::2", NULL));
nm_ip6_config_add_route (config, nmtst_platform_ip6_route ("2001:abba::", 16, "2001:abba::2234", NULL));
nm_ip6_config_add_route (config, nmtst_platform_ip6_route ("abcd:1200::", 24, "abcd:1234:4321:cdde::2", NULL));
nm_ip6_config_add_route (config, nmtst_platform_ip6_route ("2001::", 16, "2001:abba::2234", NULL));
nm_ip6_config_set_gateway (config, nmtst_inet6_from_string ("3001:abba::3234"));
......@@ -60,7 +60,7 @@ test_subtract (void)
const NMPlatformIP6Route *test_route;
const char *expected_addr = "1122:3344:5566::7788";
guint32 expected_addr_plen = 96;
const char *expected_route_dest = "9991:8882:7773::";
const char *expected_route_dest = "9991:8800::";
guint32 expected_route_plen = 24;
const char *expected_route_next_hop = "1119:2228:3337:4446::5555";
struct in6_addr expected_ns1;
......@@ -139,7 +139,7 @@ test_compare_with_source (void)
nm_ip6_config_add_address (b, &addr);
/* Route */
route = *nmtst_platform_ip6_route ("abcd:1234:4321::", 24, "abcd:1234:4321:cdde::2", NULL);
route = *nmtst_platform_ip6_route ("abcd:1200::", 24, "abcd:1234:4321:cdde::2", NULL);
route.rt_source = NM_IP_CONFIG_SOURCE_USER;
nm_ip6_config_add_route (a, &route);
......@@ -203,7 +203,7 @@ test_add_route_with_source (void)
a = nmtst_ip6_config_new (1);
/* Test that a higher priority source is not overwritten */
route = *nmtst_platform_ip6_route ("abcd:1234:4321::", 24, "abcd:1234:4321:cdde::2", NULL);
route = *nmtst_platform_ip6_route ("abcd:1200::", 24, "abcd:1234:4321:cdde::2", NULL);
route.rt_source = NM_IP_CONFIG_SOURCE_USER;
nm_ip6_config_add_route (a, &route);
......
......@@ -1531,6 +1531,7 @@ nm_vpn_connection_ip4_config_get (NMVpnConnection *self, GVariant *dict)
if (plen > 32 || plen == 0)
break;
route.plen = plen;
route.network = nm_utils_ip4_address_clear_host_address (route.network, plen);
/* Ignore host routes to the VPN gateway since NM adds one itself
* below. Since NM knows more about the routing situation than
......
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