From eb3da9012f462da2451edeb8d67c5b67c833a0b1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 13 Nov 2015 00:54:56 +0100 Subject: util-lib: optionally, when writing a string to a file, verify string on failure With this change, the idiom: r = write_string_file(p, buf, 0); if (r < 0) { if (verify_one_line_file(p, buf) > 0) r = 0; } gets reduced to: r = write_string_file(p, buf, WRITE_STRING_FILE_VERIFY_ON_FAILURE); i.e. when writing the string fails and the new flag WRITE_STRING_FILE_VERIFY_ON_FAILURE is specified we'll not return a failure immediately, but check the contents of the file. If it matches what we wanted to write we suppress the error and exit cleanly. --- src/network/networkd-link.c | 54 ++++++++++----------------------------------- 1 file changed, 12 insertions(+), 42 deletions(-) (limited to 'src/network') diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 00c57b2960..07910c2c3b 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1859,14 +1859,9 @@ static int link_set_ipv4_forward(Link *link) { p = strjoina("/proc/sys/net/ipv4/conf/", link->ifname, "/forwarding"); v = one_zero(link_ipv4_forward_enabled(link)); - r = write_string_file(p, v, 0); - if (r < 0) { - /* If the right value is set anyway, don't complain */ - if (verify_one_line_file(p, v) > 0) - return 0; - + r = write_string_file(p, v, WRITE_STRING_FILE_VERIFY_ON_FAILURE); + if (r < 0) log_link_warning_errno(link, r, "Cannot configure IPv4 forwarding for interface %s: %m", link->ifname); - } return 0; } @@ -1888,14 +1883,9 @@ static int link_set_ipv6_forward(Link *link) { p = strjoina("/proc/sys/net/ipv6/conf/", link->ifname, "/forwarding"); v = one_zero(link_ipv6_forward_enabled(link)); - r = write_string_file(p, v, 0); - if (r < 0) { - /* If the right value is set anyway, don't complain */ - if (verify_one_line_file(p, v) > 0) - return 0; - + r = write_string_file(p, v, WRITE_STRING_FILE_VERIFY_ON_FAILURE); + if (r < 0) log_link_warning_errno(link, r, "Cannot configure IPv6 forwarding for interface: %m"); - } return 0; } @@ -1917,14 +1907,9 @@ static int link_set_ipv6_privacy_extensions(Link *link) { p = strjoina("/proc/sys/net/ipv6/conf/", link->ifname, "/use_tempaddr"); xsprintf(buf, "%u", link->network->ipv6_privacy_extensions); - r = write_string_file(p, buf, 0); - if (r < 0) { - /* If the right value is set anyway, don't complain */ - if (verify_one_line_file(p, buf) > 0) - return 0; - + r = write_string_file(p, buf, WRITE_STRING_FILE_VERIFY_ON_FAILURE); + if (r < 0) log_link_warning_errno(link, r, "Cannot configure IPv6 privacy extension for interface: %m"); - } return 0; } @@ -1943,14 +1928,9 @@ static int link_set_ipv6_accept_ra(Link *link) { p = strjoina("/proc/sys/net/ipv6/conf/", link->ifname, "/accept_ra"); /* we handle router advertisments ourselves, tell the kernel to GTFO */ - r = write_string_file(p, "0", 0); - if (r < 0) { - /* If the right value is set anyway, don't complain */ - if (verify_one_line_file(p, "0") > 0) - return 0; - + r = write_string_file(p, "0", WRITE_STRING_FILE_VERIFY_ON_FAILURE); + if (r < 0) log_link_warning_errno(link, r, "Cannot disable kernel IPv6 accept_ra for interface: %m"); - } return 0; } @@ -1974,14 +1954,9 @@ static int link_set_ipv6_dad_transmits(Link *link) { xsprintf(buf, "%u", link->network->ipv6_dad_transmits); - r = write_string_file(p, buf, 0); - if (r < 0) { - /* If the right value is set anyway, don't complain */ - if (verify_one_line_file(p, buf) > 0) - return 0; - + r = write_string_file(p, buf, WRITE_STRING_FILE_VERIFY_ON_FAILURE); + if (r < 0) log_link_warning_errno(link, r, "Cannot set IPv6 dad transmits for interface: %m"); - } return 0; } @@ -2005,14 +1980,9 @@ static int link_set_ipv6_hop_limit(Link *link) { xsprintf(buf, "%u", link->network->ipv6_hop_limit); - r = write_string_file(p, buf, 0); - if (r < 0) { - /* If the right value is set anyway, don't complain */ - if (verify_one_line_file(p, buf) > 0) - return 0; - + r = write_string_file(p, buf, WRITE_STRING_FILE_VERIFY_ON_FAILURE); + if (r < 0) log_link_warning_errno(link, r, "Cannot set IPv6 hop limit for interface: %m"); - } return 0; } -- cgit v1.2.3-54-g00ecf From 66a6bd68976a20449bfcae0c55853d497a3f9b27 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 13 Nov 2015 12:30:57 +0100 Subject: networkd: fix a couple of format string types We really should use %i for ints, and %u for unsigneds, and be careful what we pick depending on the type we want to print. --- src/network/networkd-link.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) (limited to 'src/network') diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 07910c2c3b..a1401cc33f 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1901,11 +1901,11 @@ static int link_set_ipv6_privacy_extensions(Link *link) { return 0; s = link_ipv6_privacy_extensions(link); - if (s == _IPV6_PRIVACY_EXTENSIONS_INVALID) + if (s < 0) return 0; p = strjoina("/proc/sys/net/ipv6/conf/", link->ifname, "/use_tempaddr"); - xsprintf(buf, "%u", link->network->ipv6_privacy_extensions); + xsprintf(buf, "%u", (unsigned) link->network->ipv6_privacy_extensions); r = write_string_file(p, buf, WRITE_STRING_FILE_VERIFY_ON_FAILURE); if (r < 0) @@ -1927,7 +1927,7 @@ static int link_set_ipv6_accept_ra(Link *link) { p = strjoina("/proc/sys/net/ipv6/conf/", link->ifname, "/accept_ra"); - /* we handle router advertisments ourselves, tell the kernel to GTFO */ + /* We handle router advertisments ourselves, tell the kernel to GTFO */ r = write_string_file(p, "0", WRITE_STRING_FILE_VERIFY_ON_FAILURE); if (r < 0) log_link_warning_errno(link, r, "Cannot disable kernel IPv6 accept_ra for interface: %m"); @@ -1936,7 +1936,7 @@ static int link_set_ipv6_accept_ra(Link *link) { } static int link_set_ipv6_dad_transmits(Link *link) { - char buf[DECIMAL_STR_MAX(unsigned) + 1]; + char buf[DECIMAL_STR_MAX(int) + 1]; const char *p = NULL; int r; @@ -1951,8 +1951,7 @@ static int link_set_ipv6_dad_transmits(Link *link) { return 0; p = strjoina("/proc/sys/net/ipv6/conf/", link->ifname, "/dad_transmits"); - - xsprintf(buf, "%u", link->network->ipv6_dad_transmits); + xsprintf(buf, "%i", link->network->ipv6_dad_transmits); r = write_string_file(p, buf, WRITE_STRING_FILE_VERIFY_ON_FAILURE); if (r < 0) @@ -1962,7 +1961,7 @@ static int link_set_ipv6_dad_transmits(Link *link) { } static int link_set_ipv6_hop_limit(Link *link) { - char buf[DECIMAL_STR_MAX(unsigned) + 1]; + char buf[DECIMAL_STR_MAX(int) + 1]; const char *p = NULL; int r; @@ -1977,8 +1976,7 @@ static int link_set_ipv6_hop_limit(Link *link) { return 0; p = strjoina("/proc/sys/net/ipv6/conf/", link->ifname, "/hop_limit"); - - xsprintf(buf, "%u", link->network->ipv6_hop_limit); + xsprintf(buf, "%i", link->network->ipv6_hop_limit); r = write_string_file(p, buf, WRITE_STRING_FILE_VERIFY_ON_FAILURE); if (r < 0) -- cgit v1.2.3-54-g00ecf From d68e2e59b3103a2dd197401357dc619efa6e26db Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 13 Nov 2015 12:32:38 +0100 Subject: networkd: rearrange checks when to write something into sysctl a bit Move check whether ipv6 is available into link_ipv6_privacy_extensions() to keep it as internal and early as possible. Always check if there's a network attached to a link before we apply sysctls. We do this for most of the sysctl functions already, with this change we do it for all. --- src/network/networkd-link.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'src/network') diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index a1401cc33f..4a84e49699 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -147,6 +147,10 @@ bool link_ipv6_accept_ra_enabled(Link *link) { } static IPv6PrivacyExtensions link_ipv6_privacy_extensions(Link *link) { + + if (!socket_ipv6_is_supported()) + return _IPV6_PRIVACY_EXTENSIONS_INVALID; + if (link->flags & IFF_LOOPBACK) return _IPV6_PRIVACY_EXTENSIONS_INVALID; @@ -1896,10 +1900,6 @@ static int link_set_ipv6_privacy_extensions(Link *link) { const char *p = NULL; int r; - /* Make this a NOP if IPv6 is not available */ - if (!socket_ipv6_is_supported()) - return 0; - s = link_ipv6_privacy_extensions(link); if (s < 0) return 0; @@ -1925,6 +1925,9 @@ static int link_set_ipv6_accept_ra(Link *link) { if (link->flags & IFF_LOOPBACK) return 0; + if (!link->network) + return 0; + p = strjoina("/proc/sys/net/ipv6/conf/", link->ifname, "/accept_ra"); /* We handle router advertisments ourselves, tell the kernel to GTFO */ @@ -1947,6 +1950,9 @@ static int link_set_ipv6_dad_transmits(Link *link) { if (link->flags & IFF_LOOPBACK) return 0; + if (!link->network) + return 0; + if (link->network->ipv6_dad_transmits < 0) return 0; @@ -1972,6 +1978,9 @@ static int link_set_ipv6_hop_limit(Link *link) { if (link->flags & IFF_LOOPBACK) return 0; + if (!link->network) + return 0; + if (link->network->ipv6_hop_limit < 0) return 0; -- cgit v1.2.3-54-g00ecf From 765afd5c4dbc71940d6dd6007ecc3eaa5a0b2aa1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 13 Nov 2015 12:49:15 +0100 Subject: networkd: stop managing per-interface IP forwarding settings As it turns out the kernel does not support per-interface IPv6 packet forwarding controls (unlike as it does for IPv4), but only supports a global option (#1597). Also, the current per-interface management of the setting isn't really useful, as you want it to propagate to at least one more interface than the one you configure it on. This created much grief (#1411, #1808). Hence, let's roll this logic back and simplify this again, so that we can expose the same behaviour on IPv4 and IPv6 and things start to work automatically again for most folks: if a network with this setting set is set up we propagate the setting into the global setting, but this is strictly one-way: we never reset it again, and we do nothing for network interfaces where this setting is not enabled. Fixes: #1808, #1597. --- man/systemd.network.xml | 37 ++++++++++++++++---------------- src/network/networkd-link.c | 52 ++++++++++++++++++++++++++------------------- src/network/networkd-util.c | 10 ++++++++- 3 files changed, 57 insertions(+), 42 deletions(-) (limited to 'src/network') diff --git a/man/systemd.network.xml b/man/systemd.network.xml index 5994869d97..e6dedb027d 100644 --- a/man/systemd.network.xml +++ b/man/systemd.network.xml @@ -363,29 +363,28 @@ IPForward= - Configures IP forwarding for the network - interface. If enabled, incoming packets on the network - interface will be forwarded to other interfaces according to - the routing table. Takes either a boolean argument, or the - values ipv4 or ipv6, - which only enables IP forwarding for the specified address - family, or kernel, which preserves existing sysctl settings. - This controls the - net.ipv4.conf.<interface>.forwarding - and - net.ipv6.conf.<interface>.forwarding - sysctl options of the network interface (see Configures IP packet forwarding for the + system. If enabled, incoming packets on any network + interface will be forwarded to any other interfaces + according to the routing table. Takes either a boolean + argument, or the values ipv4 or + ipv6, which only enable IP packet + forwarding for the specified address family. This controls + the net.ipv4.ip_forward and + net.ipv6.conf.all.forwarding sysctl + options of the network interface (see ip-sysctl.txt for details about sysctl options). Defaults to no. - Note: unless this option is turned on, or set to kernel, - no IP forwarding is done on this interface, even if this is - globally turned on in the kernel, with the - net.ipv4.ip_forward, - net.ipv4.conf.all.forwarding, and - net.ipv6.conf.all.forwarding sysctl - options. + Note: this setting controls a global kernel option, + and does so one way only: if a network that has this setting + enabled is set up the global setting is turned on. However, + it is never turned off again, even after all networks with + this setting enabled are shut down again. + + To allow IP packet forwarding only between specific + network interfaces use a firewall. diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 4a84e49699..c37532bb73 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -111,16 +111,26 @@ static bool link_ipv4_forward_enabled(Link *link) { if (!link->network) return false; + if (link->network->ip_forward == _ADDRESS_FAMILY_BOOLEAN_INVALID) + return false; + return link->network->ip_forward & ADDRESS_FAMILY_IPV4; } static bool link_ipv6_forward_enabled(Link *link) { + + if (!socket_ipv6_is_supported()) + return false; + if (link->flags & IFF_LOOPBACK) return false; if (!link->network) return false; + if (link->network->ip_forward == _ADDRESS_FAMILY_BOOLEAN_INVALID) + return false; + return link->network->ip_forward & ADDRESS_FAMILY_IPV6; } @@ -1851,45 +1861,43 @@ static int link_enter_join_netdev(Link *link) { } static int link_set_ipv4_forward(Link *link) { - const char *p = NULL, *v; int r; - if (link->flags & IFF_LOOPBACK) - return 0; - - if (link->network->ip_forward == _ADDRESS_FAMILY_BOOLEAN_INVALID) + if (!link_ipv4_forward_enabled(link)) return 0; - p = strjoina("/proc/sys/net/ipv4/conf/", link->ifname, "/forwarding"); - v = one_zero(link_ipv4_forward_enabled(link)); + /* We propagate the forwarding flag from one interface to the + * global setting one way. This means: as long as at least one + * interface was configured at any time that had IP forwarding + * enabled the setting will stay on for good. We do this + * primarily to keep IPv4 and IPv6 packet forwarding behaviour + * somewhat in sync (see below). */ - r = write_string_file(p, v, WRITE_STRING_FILE_VERIFY_ON_FAILURE); + r = write_string_file("/proc/sys/net/ipv4/ip_forward", "1", WRITE_STRING_FILE_VERIFY_ON_FAILURE); if (r < 0) - log_link_warning_errno(link, r, "Cannot configure IPv4 forwarding for interface %s: %m", link->ifname); + log_link_warning_errno(link, r, "Cannot turn on IPv4 packet forwarding, ignoring: %m"); return 0; } static int link_set_ipv6_forward(Link *link) { - const char *p = NULL, *v = NULL; int r; - /* Make this a NOP if IPv6 is not available */ - if (!socket_ipv6_is_supported()) - return 0; - - if (link->flags & IFF_LOOPBACK) - return 0; - - if (link->network->ip_forward == _ADDRESS_FAMILY_BOOLEAN_INVALID) + if (!link_ipv6_forward_enabled(link)) return 0; - p = strjoina("/proc/sys/net/ipv6/conf/", link->ifname, "/forwarding"); - v = one_zero(link_ipv6_forward_enabled(link)); + /* On Linux, the IPv6 stack does not not know a per-interface + * packet forwarding setting: either packet forwarding is on + * for all, or off for all. We hence don't bother with a + * per-interface setting, but simply propagate the interface + * flag, if it is set, to the global flag, one-way. Note that + * while IPv4 would allow a per-interface flag, we expose the + * same behaviour there and also propagate the setting from + * one to all, to keep things simple (see above). */ - r = write_string_file(p, v, WRITE_STRING_FILE_VERIFY_ON_FAILURE); + r = write_string_file("/proc/sys/net/ipv6/conf/all/forwarding", "1", WRITE_STRING_FILE_VERIFY_ON_FAILURE); if (r < 0) - log_link_warning_errno(link, r, "Cannot configure IPv6 forwarding for interface: %m"); + log_link_warning_errno(link, r, "Cannot configure IPv6 packet forwarding, ignoring: %m"); return 0; } diff --git a/src/network/networkd-util.c b/src/network/networkd-util.c index df091393f6..2545621a93 100644 --- a/src/network/networkd-util.c +++ b/src/network/networkd-util.c @@ -79,10 +79,18 @@ int config_parse_address_family_boolean_with_kernel( assert(rvalue); assert(data); + /* This function is mostly obsolete now. It simply redirects + * "kernel" to "no". In older networkd versions we used to + * distuingish IPForward=off from IPForward=kernel, where the + * former would explicitly turn off forwarding while the + * latter would simply not touch the setting. But that logic + * is gone, hence silently accept the old setting, but turn it + * to "no". */ + s = address_family_boolean_from_string(rvalue); if (s < 0) { if (streq(rvalue, "kernel")) - s = _ADDRESS_FAMILY_BOOLEAN_INVALID; + s = ADDRESS_FAMILY_NO; else { log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to parse IPForward= option, ignoring: %s", rvalue); return 0; -- cgit v1.2.3-54-g00ecf