diff options
| -rw-r--r-- | NEWS | 28 | ||||
| -rw-r--r-- | man/systemd.network.xml | 37 | ||||
| -rw-r--r-- | src/basic/fileio.c | 83 | ||||
| -rw-r--r-- | src/basic/fileio.h | 3 | ||||
| -rw-r--r-- | src/libsystemd-network/sd-dhcp6-client.c | 4 | ||||
| -rw-r--r-- | src/network/networkd-dhcp6.c | 3 | ||||
| -rw-r--r-- | src/network/networkd-link.c | 142 | ||||
| -rw-r--r-- | src/network/networkd-util.c | 10 | ||||
| -rw-r--r-- | src/test/test-fileio.c | 21 |
9 files changed, 213 insertions, 118 deletions
@@ -92,6 +92,34 @@ CHANGES WITH 228: feature turned out to be little useful and little used, and has now been removed from the core and from systemctl. + * The dependency types RequiresOverridable= and + RequisiteOverridable= have been removed from systemd. They + have been used only very sparingly to our knowledge and + other options that provide a similar effect (such as + systemctl --mode=ignore-dependencies) are much more useful + and commonly used. Moreover, they were only half-way + implemented as the option to control behaviour regarding + these dependencies was never added to systemctl. By removing + these dependency types the execution engine becomes a bit + simpler. Unit files that use these dependencies should be + changed to use the non-Overridable dependency types + instead. In fact, when parsing unit files with these + options, that's what systemd will automatically convert them + too, but it will also warn, asking users to fix the unit + files accordingly. Removal of these dependency types should + only affect a negligible number of unit files in the wild. + + * Behaviour of networkd's IPForward= option changed + (again). It will no longer maintain a per-interface setting, + but propagate one way from interfaces where this is enabled + to the global kernel setting. The global setting will be + enabled when requested by a network that is set up, but + never be disabled again. This change was made to make sure + IPv4 and IPv6 behaviour regarding packet forwarding is + similar (as the Linux IPv6 stack does not support + per-interface control of this setting) and to minimize + surprises. + * In unit files the behaviour of %u, %U, %h, %s has changed. These specifiers will now unconditionally resolve to the various user database fields of the user that the 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 @@ </varlistentry> <varlistentry> <term><varname>IPForward=</varname></term> - <listitem><para>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 <literal>ipv4</literal> or <literal>ipv6</literal>, - which only enables IP forwarding for the specified address - family, or <literal>kernel</literal>, which preserves existing sysctl settings. - This controls the - <filename>net.ipv4.conf.<interface>.forwarding</filename> - and - <filename>net.ipv6.conf.<interface>.forwarding</filename> - sysctl options of the network interface (see <ulink + <listitem><para>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 <literal>ipv4</literal> or + <literal>ipv6</literal>, which only enable IP packet + forwarding for the specified address family. This controls + the <filename>net.ipv4.ip_forward</filename> and + <filename>net.ipv6.conf.all.forwarding</filename> sysctl + options of the network interface (see <ulink url="https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt">ip-sysctl.txt</ulink> for details about sysctl options). Defaults to <literal>no</literal>.</para> - <para>Note: unless this option is turned on, or set to <literal>kernel</literal>, - no IP forwarding is done on this interface, even if this is - globally turned on in the kernel, with the - <filename>net.ipv4.ip_forward</filename>, - <filename>net.ipv4.conf.all.forwarding</filename>, and - <filename>net.ipv6.conf.all.forwarding</filename> sysctl - options.</para> + <para>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.</para> + + <para>To allow IP packet forwarding only between specific + network interfaces use a firewall.</para> </listitem> </varlistentry> <varlistentry> diff --git a/src/basic/fileio.c b/src/basic/fileio.c index be6e327690..10aacdc56d 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -78,6 +78,7 @@ static int write_string_file_atomic(const char *fn, const char *line, bool enfor int write_string_file(const char *fn, const char *line, WriteStringFileFlags flags) { _cleanup_fclose_ FILE *f = NULL; + int q, r; assert(fn); assert(line); @@ -85,30 +86,58 @@ int write_string_file(const char *fn, const char *line, WriteStringFileFlags fla if (flags & WRITE_STRING_FILE_ATOMIC) { assert(flags & WRITE_STRING_FILE_CREATE); - return write_string_file_atomic(fn, line, !(flags & WRITE_STRING_FILE_AVOID_NEWLINE)); + r = write_string_file_atomic(fn, line, !(flags & WRITE_STRING_FILE_AVOID_NEWLINE)); + if (r < 0) + goto fail; + + return r; } if (flags & WRITE_STRING_FILE_CREATE) { f = fopen(fn, "we"); - if (!f) - return -errno; + if (!f) { + r = -errno; + goto fail; + } } else { int fd; /* We manually build our own version of fopen(..., "we") that * works without O_CREAT */ fd = open(fn, O_WRONLY|O_CLOEXEC|O_NOCTTY); - if (fd < 0) - return -errno; + if (fd < 0) { + r = -errno; + goto fail; + } f = fdopen(fd, "we"); if (!f) { + r = -errno; safe_close(fd); - return -errno; + goto fail; } } - return write_string_stream(f, line, !(flags & WRITE_STRING_FILE_AVOID_NEWLINE)); + r = write_string_stream(f, line, !(flags & WRITE_STRING_FILE_AVOID_NEWLINE)); + if (r < 0) + goto fail; + + return 0; + +fail: + if (!(flags & WRITE_STRING_FILE_VERIFY_ON_FAILURE)) + return r; + + f = safe_fclose(f); + + /* OK, the operation failed, but let's see if the right + * contents in place already. If so, eat up the error. */ + + q = verify_file(fn, line, !(flags & WRITE_STRING_FILE_AVOID_NEWLINE)); + if (q <= 0) + return r; + + return 0; } int read_one_line_file(const char *fn, char **line) { @@ -139,15 +168,41 @@ int read_one_line_file(const char *fn, char **line) { return 0; } -int verify_one_line_file(const char *fn, const char *line) { - _cleanup_free_ char *value = NULL; - int r; +int verify_file(const char *fn, const char *blob, bool accept_extra_nl) { + _cleanup_fclose_ FILE *f = NULL; + _cleanup_free_ char *buf = NULL; + size_t l, k; - r = read_one_line_file(fn, &value); - if (r < 0) - return r; + assert(fn); + assert(blob); + + l = strlen(blob); + + if (accept_extra_nl && endswith(blob, "\n")) + accept_extra_nl = false; + + buf = malloc(l + accept_extra_nl + 1); + if (!buf) + return -ENOMEM; - return streq(value, line); + f = fopen(fn, "re"); + if (!f) + return -errno; + + /* We try to read one byte more than we need, so that we know whether we hit eof */ + errno = 0; + k = fread(buf, 1, l + accept_extra_nl + 1, f); + if (ferror(f)) + return errno > 0 ? -errno : -EIO; + + if (k != l && k != l + accept_extra_nl) + return 0; + if (memcmp(buf, blob, l) != 0) + return 0; + if (k > l && buf[l] != '\n') + return 0; + + return 1; } int read_full_stream(FILE *f, char **contents, size_t *size) { diff --git a/src/basic/fileio.h b/src/basic/fileio.h index 5f2c941498..95e8698941 100644 --- a/src/basic/fileio.h +++ b/src/basic/fileio.h @@ -34,6 +34,7 @@ typedef enum { WRITE_STRING_FILE_CREATE = 1, WRITE_STRING_FILE_ATOMIC = 2, WRITE_STRING_FILE_AVOID_NEWLINE = 4, + WRITE_STRING_FILE_VERIFY_ON_FAILURE = 8, } WriteStringFileFlags; int write_string_stream(FILE *f, const char *line, bool enforce_newline); @@ -43,7 +44,7 @@ int read_one_line_file(const char *fn, char **line); int read_full_file(const char *fn, char **contents, size_t *size); int read_full_stream(FILE *f, char **contents, size_t *size); -int verify_one_line_file(const char *fn, const char *line); +int verify_file(const char *fn, const char *blob, bool accept_extra_nl); int parse_env_file(const char *fname, const char *separator, ...) _sentinel_; int load_env_file(FILE *f, const char *fname, const char *separator, char ***l); diff --git a/src/libsystemd-network/sd-dhcp6-client.c b/src/libsystemd-network/sd-dhcp6-client.c index e00ca2b00d..a443bb3a7a 100644 --- a/src/libsystemd-network/sd-dhcp6-client.c +++ b/src/libsystemd-network/sd-dhcp6-client.c @@ -259,12 +259,12 @@ int sd_dhcp6_client_set_request_option(sd_dhcp6_client *client, uint16_t option) int sd_dhcp6_client_get_lease(sd_dhcp6_client *client, sd_dhcp6_lease **ret) { assert_return(client, -EINVAL); - assert_return(ret, -EINVAL); if (!client->lease) return -ENOMSG; - *ret = client->lease; + if (ret) + *ret = client->lease; return 0; } diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c index 3ec9b84a93..d407b31b78 100644 --- a/src/network/networkd-dhcp6.c +++ b/src/network/networkd-dhcp6.c @@ -131,7 +131,8 @@ static void dhcp6_handler(sd_dhcp6_client *client, int event, void *userdata) { case SD_DHCP6_CLIENT_EVENT_STOP: case SD_DHCP6_CLIENT_EVENT_RESEND_EXPIRE: case SD_DHCP6_CLIENT_EVENT_RETRANS_MAX: - log_link_warning(link, "DHCPv6 lease lost"); + if (sd_dhcp6_client_get_lease(client, NULL) >= 0) + log_link_warning(link, "DHCPv6 lease lost"); link->dhcp6_configured = false; break; diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 00c57b2960..a415035887 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; } @@ -147,6 +157,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; @@ -492,9 +506,6 @@ static int link_stop_clients(Link *link) { assert(link->manager); assert(link->manager->event); - if (!link->network) - return 0; - if (link->dhcp_client) { k = sd_dhcp_client_stop(link->dhcp_client); if (k < 0) @@ -1847,55 +1858,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) + if (!link_ipv4_forward_enabled(link)) return 0; - if (link->network->ip_forward == _ADDRESS_FAMILY_BOOLEAN_INVALID) - return 0; + /* 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). */ - 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; - - log_link_warning_errno(link, r, "Cannot configure IPv4 forwarding for interface %s: %m", link->ifname); - } + 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 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()) + if (!link_ipv6_forward_enabled(link)) return 0; - if (link->flags & IFF_LOOPBACK) - return 0; + /* 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). */ - if (link->network->ip_forward == _ADDRESS_FAMILY_BOOLEAN_INVALID) - return 0; - - 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; - - log_link_warning_errno(link, r, "Cannot configure IPv6 forwarding for interface: %m"); - } + 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 packet forwarding, ignoring: %m"); return 0; } @@ -1906,25 +1905,16 @@ 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 == _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); - - 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; + xsprintf(buf, "%u", (unsigned) link->network->ipv6_privacy_extensions); + 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; } @@ -1940,23 +1930,21 @@ static int link_set_ipv6_accept_ra(Link *link) { if (link->flags & IFF_LOOPBACK) return 0; - p = strjoina("/proc/sys/net/ipv6/conf/", link->ifname, "/accept_ra"); + if (!link->network) + return 0; - /* 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; + 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", 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; } 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; @@ -1967,27 +1955,24 @@ 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; p = strjoina("/proc/sys/net/ipv6/conf/", link->ifname, "/dad_transmits"); + xsprintf(buf, "%i", link->network->ipv6_dad_transmits); - 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; } 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; @@ -1998,21 +1983,18 @@ 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; p = strjoina("/proc/sys/net/ipv6/conf/", link->ifname, "/hop_limit"); + xsprintf(buf, "%i", link->network->ipv6_hop_limit); - 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; } @@ -2500,7 +2482,7 @@ int link_ipv6ll_gained(Link *link) { link->ipv6ll_address = true; link_check_ready(link); - if (link->network) { + if (!IN_SET(link->state, LINK_STATE_PENDING, LINK_STATE_PENDING, LINK_STATE_UNMANAGED, LINK_STATE_FAILED)) { r = link_acquire_ipv6_conf(link); if (r < 0) { link_enter_failed(link); @@ -2516,7 +2498,7 @@ static int link_carrier_gained(Link *link) { assert(link); - if (link->network) { + if (!IN_SET(link->state, LINK_STATE_PENDING, LINK_STATE_PENDING, LINK_STATE_UNMANAGED, LINK_STATE_FAILED)) { r = link_acquire_conf(link); if (r < 0) { link_enter_failed(link); 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; diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c index e588681b86..bde3c7c3cf 100644 --- a/src/test/test-fileio.c +++ b/src/test/test-fileio.c @@ -363,6 +363,26 @@ static void test_write_string_file_no_create(void) { unlink(fn); } +static void test_write_string_file_verify(void) { + _cleanup_free_ char *buf = NULL, *buf2 = NULL; + int r; + + assert_se(read_one_line_file("/proc/cmdline", &buf) >= 0); + assert_se((buf2 = strjoin(buf, "\n", NULL))); + + r = write_string_file("/proc/cmdline", buf, 0); + assert_se(r == -EACCES || r == -EIO); + r = write_string_file("/proc/cmdline", buf2, 0); + assert_se(r == -EACCES || r == -EIO); + + assert_se(write_string_file("/proc/cmdline", buf, WRITE_STRING_FILE_VERIFY_ON_FAILURE) == 0); + assert_se(write_string_file("/proc/cmdline", buf2, WRITE_STRING_FILE_VERIFY_ON_FAILURE) == 0); + + r = write_string_file("/proc/cmdline", buf, WRITE_STRING_FILE_VERIFY_ON_FAILURE|WRITE_STRING_FILE_AVOID_NEWLINE); + assert_se(r == -EACCES || r == -EIO); + assert_se(write_string_file("/proc/cmdline", buf2, WRITE_STRING_FILE_VERIFY_ON_FAILURE|WRITE_STRING_FILE_AVOID_NEWLINE) == 0); +} + static void test_load_env_file_pairs(void) { char fn[] = "/tmp/test-load_env_file_pairs-XXXXXX"; int fd; @@ -419,6 +439,7 @@ int main(int argc, char *argv[]) { test_write_string_stream(); test_write_string_file(); test_write_string_file_no_create(); + test_write_string_file_verify(); test_load_env_file_pairs(); return 0; |