summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Gundersen <teg@jklm.no>2015-11-13 14:18:32 +0100
committerTom Gundersen <teg@jklm.no>2015-11-13 14:18:32 +0100
commitcf0f970fe1c6101b45cae8277c221c10398b8e1e (patch)
tree37f8ac193821e520a3c271be7685c06d79300e07
parent7152869f0a4a4612022244064cc2b3905b1e3fc7 (diff)
parent765afd5c4dbc71940d6dd6007ecc3eaa5a0b2aa1 (diff)
Merge pull request #1879 from poettering/networkd-forward
stop managing per-interface IP forwarding settings
-rw-r--r--man/systemd.network.xml37
-rw-r--r--src/basic/fileio.c83
-rw-r--r--src/basic/fileio.h3
-rw-r--r--src/network/networkd-link.c135
-rw-r--r--src/network/networkd-util.c10
-rw-r--r--src/test/test-fileio.c21
6 files changed, 179 insertions, 110 deletions
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.&lt;interface&gt;.forwarding</filename>
- and
- <filename>net.ipv6.conf.&lt;interface&gt;.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/network/networkd-link.c b/src/network/networkd-link.c
index 00c57b2960..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;
}
@@ -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;
@@ -1847,55 +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)
+ 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())
- 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, 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 +1908,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 +1933,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 +1958,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 +1986,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;
}
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;