firewall-util: introduce context structure

for planned nft backend we have three choices:

- open/close a new nfnetlink socket for every operation
- keep a nfnetlink socket open internally
- expose a opaque fw_ctx and stash all internal data here.

Originally I opted for the 2nd option, but during review it was
suggested to avoid static storage duration because of perceived
problems with threaded applications.

This adds fw_ctx and new/free functions, then converts the existing api
and nspawn and networkd to use it.

diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c
index 3ec47e30a3412166d96fea6c39093ea406200e49..4137b2994527c570f67d5139538082bce32e4499 100644 (file)
--- a/src/network/networkd-address.c
+++ b/src/network/networkd-address.c
@@ -282,7 +282,7 @@ static int address_set_masquerade(Address *address, bool add) {
         if (r < 0)
                 return r;
 
-        r = fw_add_masquerade(add, AF_INET, &masked, address->prefixlen);
+        r = fw_add_masquerade(&address->link->manager->fw_ctx, add, AF_INET, &masked, address->prefixlen);
         if (r < 0)
                 return r;
 

diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c
index 8af17b1194612ba2a64f008899a376995f3f776c..4894d235b072b8318ead6db3def758e0509309ad 100644 (file)
--- a/src/network/networkd-manager.c
+++ b/src/network/networkd-manager.c
@@ -22,6 +22,7 @@
 #include "dns-domain.h"
 #include "fd-util.h"
 #include "fileio.h"
+#include "firewall-util.h"
 #include "local-addresses.h"
 #include "netlink-util.h"
 #include "network-internal.h"
@@ -912,6 +913,8 @@ void manager_free(Manager *m) {
 
         safe_close(m->ethtool_fd);
 
+        m->fw_ctx = fw_ctx_free(m->fw_ctx);
+
         free(m);
 }
 

diff --git a/src/network/networkd-manager.h b/src/network/networkd-manager.h
index b67116be59ba0e3c2cd0dca861a3fe5afbe416d8..25fb080dc9ba90101a0055eef02bad52b78c8434 100644 (file)
--- a/src/network/networkd-manager.h
+++ b/src/network/networkd-manager.h
@@ -9,6 +9,7 @@
 #include "sd-resolve.h"
 
 #include "dhcp-identifier.h"
+#include "firewall-util.h"
 #include "hashmap.h"
 #include "networkd-link.h"
 #include "networkd-network.h"
@@ -74,6 +75,8 @@ struct Manager {
 
         bool dhcp4_prefix_root_cannot_set_table:1;
         bool bridge_mdb_on_master_not_supported:1;
+
+        FirewallContext *fw_ctx;
 };
 
 int manager_new(Manager **ret);

diff --git a/src/network/networkd.c b/src/network/networkd.c
index b448d9b011788246183a9f8d437b634d7df08a47..ac2bc90d974f3ed8cbfb201714b1a543b42cf897 100644 (file)
--- a/src/network/networkd.c
+++ b/src/network/networkd.c
@@ -9,6 +9,7 @@
 
 #include "capability-util.h"
 #include "daemon-util.h"
+#include "firewall-util.h"
 #include "main-func.h"
 #include "mkdir.h"
 #include "networkd-conf.h"
@@ -92,6 +93,10 @@ static int run(int argc, char *argv[]) {
         if (r < 0)
                 return r;
 
+        r = fw_ctx_new(&m->fw_ctx);
+        if (r < 0)
+                log_warning_errno(r, "Could not initialize firewall, IPMasquerade= option not available: %m");
+
         r = manager_start(m);
         if (r < 0)
                 return log_error_errno(r, "Could not start manager: %m");

diff --git a/src/nspawn/nspawn-expose-ports.c b/src/nspawn/nspawn-expose-ports.c
index db076c50c0918ef72068a5e5043992e8d485f5f6..c368b2056354df9bbcb0fd898cee5f84bfc59f90 100644 (file)
--- a/src/nspawn/nspawn-expose-ports.c
+++ b/src/nspawn/nspawn-expose-ports.c
@@ -82,7 +82,7 @@ void expose_port_free_all(ExposePort *p) {
         }
 }
 
-int expose_port_flush(ExposePort* l, union in_addr_union *exposed) {
+int expose_port_flush(FirewallContext **fw_ctx, ExposePort* l, union in_addr_union *exposed) {
         ExposePort *p;
         int r, af = AF_INET;
 
@@ -97,7 +97,8 @@ int expose_port_flush(ExposePort* l, union in_addr_union *exposed) {
         log_debug("Lost IP address.");
 
         LIST_FOREACH(ports, p, l) {
-                r = fw_add_local_dnat(false,
+                r = fw_add_local_dnat(fw_ctx,
+                                      false,
                                       af,
                                       p->protocol,
                                       p->host_port,
@@ -112,7 +113,7 @@ int expose_port_flush(ExposePort* l, union in_addr_union *exposed) {
         return 0;
 }
 
-int expose_port_execute(sd_netlink *rtnl, ExposePort *l, union in_addr_union *exposed) {
+int expose_port_execute(sd_netlink *rtnl, FirewallContext **fw_ctx, ExposePort *l, union in_addr_union *exposed) {
         _cleanup_free_ struct local_address *addresses = NULL;
         union in_addr_union new_exposed;
         ExposePort *p;
@@ -136,7 +137,7 @@ int expose_port_execute(sd_netlink *rtnl, ExposePort *l, union in_addr_union *ex
                 addresses[0].scope < RT_SCOPE_LINK;
 
         if (!add)
-                return expose_port_flush(l, exposed);
+                return expose_port_flush(fw_ctx, l, exposed);
 
         new_exposed = addresses[0].address;
         if (in_addr_equal(af, exposed, &new_exposed))
@@ -150,7 +151,8 @@ int expose_port_execute(sd_netlink *rtnl, ExposePort *l, union in_addr_union *ex
 
         LIST_FOREACH(ports, p, l) {
 
-                r = fw_add_local_dnat(true,
+                r = fw_add_local_dnat(fw_ctx,
+                                      true,
                                       af,
                                       p->protocol,
                                       p->host_port,

diff --git a/src/nspawn/nspawn-expose-ports.h b/src/nspawn/nspawn-expose-ports.h
index d0c1cecbe87434f143087f26a859d1390517437c..c1677cb61b0fac58cd707018b0b9af26dd248ff3 100644 (file)
--- a/src/nspawn/nspawn-expose-ports.h
+++ b/src/nspawn/nspawn-expose-ports.h
@@ -3,6 +3,8 @@
 
 #include <inttypes.h>
 
+#include "firewall-util.h"
+
 #include "sd-event.h"
 #include "sd-netlink.h"
 
@@ -22,5 +24,5 @@ int expose_port_parse(ExposePort **l, const char *s);
 int expose_port_watch_rtnl(sd_event *event, int recv_fd, sd_netlink_message_handler_t handler, void *userdata, sd_netlink **ret);
 int expose_port_send_rtnl(int send_fd);
 
-int expose_port_execute(sd_netlink *rtnl, ExposePort *l, union in_addr_union *exposed);
-int expose_port_flush(ExposePort* l, union in_addr_union *exposed);
+int expose_port_execute(sd_netlink *rtnl, FirewallContext **fw_ctx, ExposePort *l, union in_addr_union *exposed);
+int expose_port_flush(FirewallContext **fw_ctx, ExposePort* l, union in_addr_union *exposed);

diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index cfbc8f11bf1fc23efa426bcb319313b535a31299..a6f64e84157e388c261a74e82095b9303220af6a 100644 (file)
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -2474,14 +2474,19 @@ static int setup_kmsg(int kmsg_socket) {
         return 0;
 }
 
+struct ExposeArgs {
+        union in_addr_union address;
+        struct FirewallContext *fw_ctx;
+};
+
 static int on_address_change(sd_netlink *rtnl, sd_netlink_message *m, void *userdata) {
-        union in_addr_union *exposed = userdata;
+        struct ExposeArgs *args = userdata;
 
         assert(rtnl);
         assert(m);
-        assert(exposed);
+        assert(args);
 
-        expose_port_execute(rtnl, arg_expose_ports, exposed);
+        expose_port_execute(rtnl, &args->fw_ctx, arg_expose_ports, &args->address);
         return 0;
 }
 
@@ -4466,7 +4471,7 @@ static int run_container(
                bool secondary,
                FDSet *fds,
                char veth_name[IFNAMSIZ], bool *veth_created,
-               union in_addr_union *exposed,
+               struct ExposeArgs *expose_args,
                int *master, pid_t *pid, int *ret) {
 
         static const struct sigaction sa = {
@@ -4895,11 +4900,11 @@ static int run_container(
         (void) sd_event_add_signal(event, NULL, SIGCHLD, on_sigchld, PID_TO_PTR(*pid));
 
         if (arg_expose_ports) {
-                r = expose_port_watch_rtnl(event, rtnl_socket_pair[0], on_address_change, exposed, &rtnl);
+                r = expose_port_watch_rtnl(event, rtnl_socket_pair[0], on_address_change, expose_args, &rtnl);
                 if (r < 0)
                         return r;
 
-                (void) expose_port_execute(rtnl, arg_expose_ports, exposed);
+                (void) expose_port_execute(rtnl, &expose_args->fw_ctx, arg_expose_ports, &expose_args->address);
         }
 
         rtnl_socket_pair[0] = safe_close(rtnl_socket_pair[0]);
@@ -5026,7 +5031,7 @@ static int run_container(
                 return 0; /* finito */
         }
 
-        expose_port_flush(arg_expose_ports, exposed);
+        expose_port_flush(&expose_args->fw_ctx, arg_expose_ports, &expose_args->address);
 
         (void) remove_veth_links(veth_name, arg_network_veth_extra);
         *veth_created = false;
@@ -5155,12 +5160,13 @@ static int run(int argc, char *argv[]) {
         _cleanup_fdset_free_ FDSet *fds = NULL;
         int r, n_fd_passed, ret = EXIT_SUCCESS;
         char veth_name[IFNAMSIZ] = "";
-        union in_addr_union exposed = {};
+        struct ExposeArgs expose_args = {};
         _cleanup_(release_lock_file) LockFile tree_global_lock = LOCK_FILE_INIT, tree_local_lock = LOCK_FILE_INIT;
         char tmprootdir[] = "/tmp/nspawn-root-XXXXXX";
         _cleanup_(loop_device_unrefp) LoopDevice *loop = NULL;
         _cleanup_(decrypted_image_unrefp) DecryptedImage *decrypted_image = NULL;
         _cleanup_(dissected_image_unrefp) DissectedImage *dissected_image = NULL;
+        _cleanup_(fw_ctx_freep) FirewallContext *fw_ctx = NULL;
         pid_t pid = 0;
 
         log_parse_environment();
@@ -5517,12 +5523,20 @@ static int run(int argc, char *argv[]) {
                 goto finish;
         }
 
+        if (arg_expose_ports) {
+                r = fw_ctx_new(&fw_ctx);
+                if (r < 0) {
+                        log_error_errno(r, "Cannot expose configured ports, firewall initialization failed: %m");
+                        goto finish;
+                }
+                expose_args.fw_ctx = fw_ctx;
+        }
         for (;;) {
                 r = run_container(dissected_image,
                                   secondary,
                                   fds,
                                   veth_name, &veth_created,
-                                  &exposed, &master,
+                                  &expose_args, &master,
                                   &pid, &ret);
                 if (r <= 0)
                         break;
@@ -5572,7 +5586,7 @@ finish:
                 (void) rm_rf(p, REMOVE_ROOT);
         }
 
-        expose_port_flush(arg_expose_ports, &exposed);
+        expose_port_flush(&fw_ctx, arg_expose_ports, &expose_args.address);
 
         if (veth_created)
                 (void) remove_veth_links(veth_name, arg_network_veth_extra);

diff --git a/src/shared/firewall-util-private.h b/src/shared/firewall-util-private.h
index d7cb19353db893fbcb4421bf3c7a7464f016be65..7f9efbc5135775a91fd1a514ae9de6fceb8173ce 100644 (file)
--- a/src/shared/firewall-util-private.h
+++ b/src/shared/firewall-util-private.h
@@ -6,6 +6,17 @@
 
 #include "in-addr-util.h"
 
+enum FirewallBackend {
+        FW_BACKEND_NONE,
+#if HAVE_LIBIPTC
+        FW_BACKEND_IPTABLES,
+#endif
+};
+
+struct FirewallContext {
+        enum FirewallBackend firewall_backend;
+};
+
 #if HAVE_LIBIPTC
 
 int fw_iptables_add_masquerade(

diff --git a/src/shared/firewall-util.c b/src/shared/firewall-util.c
index 107056514ee56ad6c13c1fd231bdfdb5b877f3c3..edfe5787b1d7d1d488302e9f040554463e1c9a2c 100644 (file)
--- a/src/shared/firewall-util.c
+++ b/src/shared/firewall-util.c
@@ -8,15 +8,6 @@
 #include "firewall-util.h"
 #include "firewall-util-private.h"
 
-enum FirewallBackend {
-        FW_BACKEND_NONE,
-#if HAVE_LIBIPTC
-        FW_BACKEND_IPTABLES,
-#endif
-};
-
-static enum FirewallBackend FirewallBackend;
-
 static enum FirewallBackend firewall_backend_probe(void) {
 #if HAVE_LIBIPTC
         return FW_BACKEND_IPTABLES;
@@ -25,16 +16,41 @@ static enum FirewallBackend firewall_backend_probe(void) {
 #endif
 }
 
+int fw_ctx_new(FirewallContext **ret) {
+        _cleanup_free_ FirewallContext *ctx = NULL;
+
+        ctx = new0(FirewallContext, 1);
+        if (!ctx)
+                return -ENOMEM;
+
+         *ret = TAKE_PTR(ctx);
+         return 0;
+}
+
+FirewallContext *fw_ctx_free(FirewallContext *ctx) {
+        return mfree(ctx);
+}
+
 int fw_add_masquerade(
+                FirewallContext **fw_ctx,
                 bool add,
                 int af,
                 const union in_addr_union *source,
                 unsigned source_prefixlen) {
+        FirewallContext *ctx;
+        int r;
 
-        if (FirewallBackend == FW_BACKEND_NONE)
-                FirewallBackend = firewall_backend_probe();
+        if (!*fw_ctx) {
+                r = fw_ctx_new(fw_ctx);
+                if (r < 0)
+                        return r;
+        }
 
-        switch (FirewallBackend) {
+        ctx = *fw_ctx;
+        if (ctx->firewall_backend == FW_BACKEND_NONE)
+                ctx->firewall_backend = firewall_backend_probe();
+
+        switch (ctx->firewall_backend) {
         case FW_BACKEND_NONE:
                 return -EOPNOTSUPP;
 #if HAVE_LIBIPTC
@@ -47,6 +63,7 @@ int fw_add_masquerade(
 }
 
 int fw_add_local_dnat(
+                FirewallContext **fw_ctx,
                 bool add,
                 int af,
                 int protocol,
@@ -54,11 +71,19 @@ int fw_add_local_dnat(
                 const union in_addr_union *remote,
                 uint16_t remote_port,
                 const union in_addr_union *previous_remote) {
+        FirewallContext *ctx;
+
+        if (!*fw_ctx) {
+                int ret = fw_ctx_new(fw_ctx);
+                if (ret < 0)
+                        return ret;
+        }
 
-        if (FirewallBackend == FW_BACKEND_NONE)
-                FirewallBackend = firewall_backend_probe();
+        ctx = *fw_ctx;
+        if (ctx->firewall_backend == FW_BACKEND_NONE)
+                ctx->firewall_backend = firewall_backend_probe();
 
-        switch (FirewallBackend) {
+        switch (ctx->firewall_backend) {
         case FW_BACKEND_NONE:
                 return -EOPNOTSUPP;
 #if HAVE_LIBIPTC

diff --git a/src/shared/firewall-util.h b/src/shared/firewall-util.h
index bb6dc5a0f08409c660262196a4233046b310febe..5180b429d3d7356ee246448fdbc53cc9a59f748d 100644 (file)
--- a/src/shared/firewall-util.h
+++ b/src/shared/firewall-util.h
@@ -6,13 +6,22 @@
 
 #include "in-addr-util.h"
 
+typedef struct FirewallContext FirewallContext;
+
+int fw_ctx_new(FirewallContext **ret);
+FirewallContext *fw_ctx_free(FirewallContext *fw_ctx);
+
+DEFINE_TRIVIAL_CLEANUP_FUNC(FirewallContext *, fw_ctx_free);
+
 int fw_add_masquerade(
+                FirewallContext **fw_ctx,
                 bool add,
                 int af,
                 const union in_addr_union *source,
                 unsigned source_prefixlen);
 
 int fw_add_local_dnat(
+                FirewallContext **fw_ctx,
                 bool add,
                 int af,
                 int protocol,

diff --git a/src/test/test-firewall-util.c b/src/test/test-firewall-util.c
index f223c0a4d91cb0bd55baef724bd11fc739fd7d96..14678c048d89c6cb96b87fcb02ff3dafa23916e0 100644 (file)
--- a/src/test/test-firewall-util.c
+++ b/src/test/test-firewall-util.c
@@ -7,48 +7,53 @@
 #define MAKE_IN_ADDR_UNION(a,b,c,d) (union in_addr_union) { .in.s_addr = htobe32((uint32_t) (a) << 24 | (uint32_t) (b) << 16 | (uint32_t) (c) << 8 | (uint32_t) (d))}
 
 int main(int argc, char *argv[]) {
+        _cleanup_(fw_ctx_freep) FirewallContext *ctx;
         int r;
         test_setup_logging(LOG_DEBUG);
         uint8_t prefixlen = 32;
 
-        r = fw_add_masquerade(true, AF_INET, NULL, 0);
+        r = fw_ctx_new(&ctx);
+        if (r < 0)
+                return log_error_errno(r, "Failed to init firewall: %m");
+
+        r = fw_add_masquerade(&ctx, true, AF_INET, NULL, 0);
         if (r == 0)
                 log_error("Expected failure: NULL source");
 
-        r = fw_add_masquerade(true, AF_INET, &MAKE_IN_ADDR_UNION(10,1,2,0), 0);
+        r = fw_add_masquerade(&ctx, true, AF_INET, &MAKE_IN_ADDR_UNION(10,1,2,0), 0);
         if (r == 0)
                 log_error("Expected failure: 0 prefixlen");
 
-        r = fw_add_masquerade(true, AF_INET, &MAKE_IN_ADDR_UNION(10,1,2,3), prefixlen);
+        r = fw_add_masquerade(&ctx, true, AF_INET, &MAKE_IN_ADDR_UNION(10,1,2,3), prefixlen);
         if (r < 0)
                 log_error_errno(r, "Failed to modify firewall: %m");
 
         prefixlen = 28;
-        r = fw_add_masquerade(true, AF_INET, &MAKE_IN_ADDR_UNION(10,0,2,0), prefixlen);
+        r = fw_add_masquerade(&ctx, true, AF_INET, &MAKE_IN_ADDR_UNION(10,0,2,0), prefixlen);
         if (r < 0)
                 log_error_errno(r, "Failed to modify firewall: %m");
 
-        r = fw_add_masquerade(false, AF_INET, &MAKE_IN_ADDR_UNION(10,0,2,0), prefixlen);
+        r = fw_add_masquerade(&ctx, false, AF_INET, &MAKE_IN_ADDR_UNION(10,0,2,0), prefixlen);
         if (r < 0)
                 log_error_errno(r, "Failed to modify firewall: %m");
 
-        r = fw_add_masquerade(false, AF_INET, &MAKE_IN_ADDR_UNION(10,1,2,3), 32);
+        r = fw_add_masquerade(&ctx, false, AF_INET, &MAKE_IN_ADDR_UNION(10,1,2,3), 32);
         if (r < 0)
                 log_error_errno(r, "Failed to modify firewall: %m");
 
-        r = fw_add_local_dnat(true, AF_INET, IPPROTO_TCP, 4711, &MAKE_IN_ADDR_UNION(1, 2, 3, 4), 815, NULL);
+        r = fw_add_local_dnat(&ctx, true, AF_INET, IPPROTO_TCP, 4711, &MAKE_IN_ADDR_UNION(1, 2, 3, 4), 815, NULL);
         if (r < 0)
                 log_error_errno(r, "Failed to modify firewall: %m");
 
-        r = fw_add_local_dnat(true, AF_INET, IPPROTO_TCP, 4711, &MAKE_IN_ADDR_UNION(1, 2, 3, 4), 815, NULL);
+        r = fw_add_local_dnat(&ctx, true, AF_INET, IPPROTO_TCP, 4711, &MAKE_IN_ADDR_UNION(1, 2, 3, 4), 815, NULL);
         if (r < 0)
                 log_error_errno(r, "Failed to modify firewall: %m");
 
-        r = fw_add_local_dnat(true, AF_INET, IPPROTO_TCP, 4711, &MAKE_IN_ADDR_UNION(1, 2, 3, 5), 815, &MAKE_IN_ADDR_UNION(1, 2, 3, 4));
+        r = fw_add_local_dnat(&ctx, true, AF_INET, IPPROTO_TCP, 4711, &MAKE_IN_ADDR_UNION(1, 2, 3, 5), 815, &MAKE_IN_ADDR_UNION(1, 2, 3, 4));
         if (r < 0)
                 log_error_errno(r, "Failed to modify firewall: %m");
 
-        r = fw_add_local_dnat(false, AF_INET, IPPROTO_TCP, 4711, &MAKE_IN_ADDR_UNION(1, 2, 3, 5), 815, NULL);
+        r = fw_add_local_dnat(&ctx, false, AF_INET, IPPROTO_TCP, 4711, &MAKE_IN_ADDR_UNION(1, 2, 3, 5), 815, NULL);
         if (r < 0)
                 log_error_errno(r, "Failed to modify firewall: %m");