diff options
author | Florian Westphal <fw@strlen.de> | 2020-09-15 19:58:44 +0200 |
---|---|---|
committer | Florian Westphal <fw@strlen.de> | 2020-12-16 00:35:56 +0100 |
commit | 761cf19d7bc4b5950caff33965508d9fb7bbb547 (patch) | |
tree | 62a913a54080a2680138582a4753f5a9b52705ff | |
parent | nspawn: pass userdata pointer, not inet_addr union (diff) | |
download | systemd-761cf19d7bc4b5950caff33965508d9fb7bbb547.tar.xz systemd-761cf19d7bc4b5950caff33965508d9fb7bbb547.zip |
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.
-rw-r--r-- | src/network/networkd-address.c | 2 | ||||
-rw-r--r-- | src/network/networkd-manager.c | 3 | ||||
-rw-r--r-- | src/network/networkd-manager.h | 3 | ||||
-rw-r--r-- | src/network/networkd.c | 5 | ||||
-rw-r--r-- | src/nspawn/nspawn-expose-ports.c | 12 | ||||
-rw-r--r-- | src/nspawn/nspawn-expose-ports.h | 6 | ||||
-rw-r--r-- | src/nspawn/nspawn.c | 34 | ||||
-rw-r--r-- | src/shared/firewall-util-private.h | 11 | ||||
-rw-r--r-- | src/shared/firewall-util.c | 55 | ||||
-rw-r--r-- | src/shared/firewall-util.h | 9 | ||||
-rw-r--r-- | src/test/test-firewall-util.c | 25 |
11 files changed, 122 insertions, 43 deletions
diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index 3ec47e30a3..4137b29945 100644 --- 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 8af17b1194..4894d235b0 100644 --- 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 b67116be59..25fb080dc9 100644 --- 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 b448d9b011..ac2bc90d97 100644 --- 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 db076c50c0..c368b20563 100644 --- 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 d0c1cecbe8..c1677cb61b 100644 --- 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 cfbc8f11bf..a6f64e8415 100644 --- 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 d7cb19353d..7f9efbc513 100644 --- 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 107056514e..edfe5787b1 100644 --- 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 bb6dc5a0f0..5180b429d3 100644 --- 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 f223c0a4d9..14678c048d 100644 --- 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"); |