From f80a206aa497c1a1ce2a7bfc024ea2080f357aeb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 10 May 2021 16:33:24 +0200 Subject: [PATCH 1/5] socket-bind: use lowercase "ipv4"/"ipv6" spelling In most of our codebase when we referenced "ipv4" and "ipv6" on the right-hand-side of an assignment, we lowercases it (on the left-hand-side we used CamelCase, and thus "IPv4" and "IPv6"). In particular all across the networkd codebase the various "per-protocol booleans" use the lower-case spelling. Hence, let's use lower-case for SocketBindAllow=/SocketBindDeny= too, just make sure things feel like they belong together better. (This work is not included in any released version, hence let's fix this now, before any fixes in this area would be API breakage) Follow-up for #17655 --- man/systemd.resource-control.xml | 6 +++--- src/core/cgroup.c | 5 +++-- src/core/load-fragment.c | 6 +++--- src/shared/bus-unit-util.c | 11 +++++------ src/systemctl/systemctl-show.c | 2 +- src/test/test-socket-bind.c | 4 ++-- 6 files changed, 17 insertions(+), 17 deletions(-) diff --git a/man/systemd.resource-control.xml b/man/systemd.resource-control.xml index d9b570e232..827f343a50 100644 --- a/man/systemd.resource-control.xml +++ b/man/systemd.resource-control.xml @@ -775,7 +775,7 @@ BPFProgram=bind6:/sys/fs/bpf/sock-addr-hook bind-rule := [address-family:]ip-ports - address-family := { IPv4 | IPv6 } + address-family := { ipv4 | ipv6 } ip-ports := { ip-port | ip-port-range | any } @@ -812,7 +812,7 @@ BPFProgram=bind6:/sys/fs/bpf/sock-addr-hook Examples:… # Allow binding IPv6 socket addresses with a port greater than or equal to 10000. [Service] -SocketBindAllow=IPv6:10000-65535 +SocketBindAllow=ipv6:10000-65535 SocketBindDeny=any … # Allow binding IPv4 and IPv6 socket addresses with 1234 and 4321 ports. @@ -823,7 +823,7 @@ SocketBindDeny=any … # Deny binding IPv6 socket addresses. [Service] -SocketBindDeny=IPv6:any +SocketBindDeny=ipv6:any … # Deny binding IPv4 and IPv6 socket addresses. [Service] diff --git a/src/core/cgroup.c b/src/core/cgroup.c index a44cf9368c..87e8cdf7ee 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -594,8 +594,9 @@ void cgroup_context_dump(Unit *u, FILE* f, const char *prefix) { } void cgroup_context_dump_socket_bind_item(const CGroupSocketBindItem *item, FILE *f) { - const char *family = item->address_family == AF_INET ? "IPv4:" : - item->address_family == AF_INET6 ? "IPv6:" : ""; + const char *family = + item->address_family == AF_INET ? "ipv4:" : + item->address_family == AF_INET6 ? "ipv6:" : ""; if (item->nr_ports == 0) fprintf(f, " %sany", family); diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 2399089492..a7d1cb6f12 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -5661,13 +5661,13 @@ int config_parse_cgroup_socket_bind( } if (rvalue) { - if (streq(word, "IPv4")) + if (streq(word, "ipv4")) af = AF_INET; - else if (streq(word, "IPv6")) + else if (streq(word, "ipv6")) af = AF_INET6; else { log_syntax(unit, LOG_WARNING, filename, line, 0, - "Only IPv4 and IPv6 protocols are supported, ignoring."); + "Only \"ipv4\" and \"ipv6\" protocols are supported, ignoring."); return 0; } diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 20d368efca..4d53aaa3da 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -879,14 +879,13 @@ static int bus_append_cgroup_property(sd_bus_message *m, const char *field, cons address_family = eq ? word : NULL; if (address_family) { - if (!STR_IN_SET(address_family, "IPv4", "IPv6")) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Only IPv4 and IPv6 protocols are supported"); - - if (streq(address_family, "IPv4")) + if (streq(address_family, "ipv4")) family = AF_INET; - else + else if (streq(address_family, "ipv6")) family = AF_INET6; + else + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Only \"ipv4\" and \"ipv6\" protocols are supported"); } user_port = eq ? eq : word; diff --git a/src/systemctl/systemctl-show.c b/src/systemctl/systemctl-show.c index 2df05464c6..d43f1cf094 100644 --- a/src/systemctl/systemctl-show.c +++ b/src/systemctl/systemctl-show.c @@ -1717,7 +1717,7 @@ static int print_property(const char *name, const char *expected_value, sd_bus_m if (r < 0) return bus_log_parse_error(r); while ((r = sd_bus_message_read(m, "(iqq)", &af, &nr_ports, &port_min)) > 0) { - family = af == AF_INET ? "IPv4:" : af == AF_INET6 ? "IPv6:" : ""; + family = af == AF_INET ? "ipv4:" : af == AF_INET6 ? "ipv6:" : ""; if (nr_ports == 0) bus_print_property_valuef(name, expected_value, flags, "%sany", family); else if (nr_ports == 1) diff --git a/src/test/test-socket-bind.c b/src/test/test-socket-bind.c index bfe5072bc3..16cfea7779 100644 --- a/src/test/test-socket-bind.c +++ b/src/test/test-socket-bind.c @@ -141,8 +141,8 @@ int main(int argc, char *argv[]) { assert_se(manager_startup(m, NULL, NULL) >= 0); assert_se(test_socket_bind(m, "socket_bind_test.service", netcat_path, "2000", STRV_MAKE("2000"), STRV_MAKE("any")) >= 0); - assert_se(test_socket_bind(m, "socket_bind_test.service", netcat_path, "2000", STRV_MAKE("IPv6:2001-2002"), STRV_MAKE("any")) >= 0); - assert_se(test_socket_bind(m, "socket_bind_test.service", netcat_path, "6666", STRV_MAKE("IPv4:6666", "6667"), STRV_MAKE("any")) >= 0); + assert_se(test_socket_bind(m, "socket_bind_test.service", netcat_path, "2000", STRV_MAKE("ipv6:2001-2002"), STRV_MAKE("any")) >= 0); + assert_se(test_socket_bind(m, "socket_bind_test.service", netcat_path, "6666", STRV_MAKE("ipv4:6666", "6667"), STRV_MAKE("any")) >= 0); assert_se(test_socket_bind(m, "socket_bind_test.service", netcat_path, "6666", STRV_MAKE("6667", "6668", ""), STRV_MAKE("any")) >= 0); assert_se(test_socket_bind(m, "socket_bind_test.service", netcat_path, "7777", STRV_MAKE_EMPTY, STRV_MAKE_EMPTY) >= 0); assert_se(test_socket_bind(m, "socket_bind_test.service", netcat_path, "8888", STRV_MAKE("any"), STRV_MAKE("any")) >= 0); From 23118193d23438f9fdb4dd31c9acc5d6bfcc393c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 10 May 2021 17:14:13 +0200 Subject: [PATCH 2/5] af-list: add helpers mapping AF_INET/AF_INET6 to "ipv4"/"ipv6" --- src/basic/af-list.c | 12 ++++++++++++ src/basic/af-list.h | 3 +++ 2 files changed, 15 insertions(+) diff --git a/src/basic/af-list.c b/src/basic/af-list.c index 7e819d6d11..a9ab891e20 100644 --- a/src/basic/af-list.c +++ b/src/basic/af-list.c @@ -38,3 +38,15 @@ int af_from_name(const char *name) { int af_max(void) { return ELEMENTSOF(af_names); } + +const char *af_to_ipv4_ipv6(int id) { + /* Pretty often we want to map the address family to the typically used protocol name for IPv4 + + * IPv6. Let's add special helpers for that. */ + return id == AF_INET ? "ipv4" : + id == AF_INET6 ? "ipv6" : NULL; +} + +int af_from_ipv4_ipv6(const char *af) { + return streq_ptr(af, "ipv4") ? AF_INET : + streq_ptr(af, "ipv6") ? AF_INET6 : AF_UNSPEC; +} diff --git a/src/basic/af-list.h b/src/basic/af-list.h index 688ac63df7..9592b9ed3c 100644 --- a/src/basic/af-list.h +++ b/src/basic/af-list.h @@ -22,4 +22,7 @@ static inline const char* af_to_name_short(int id) { return f + 3; } +const char* af_to_ipv4_ipv6(int id); +int af_from_ipv4_ipv6(const char *af); + int af_max(void); From a481753648da77758b13b82e8d82be8e332f515e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 10 May 2021 17:24:48 +0200 Subject: [PATCH 3/5] tree-wide: use af_to_ipv4_ipv6() + af_from_ipv4_ipv6() helpers at various places --- src/core/cgroup.c | 14 ++++++++------ src/core/load-fragment.c | 7 ++----- src/journal-remote/journal-remote.c | 3 ++- src/libsystemd/sd-bus/sd-bus.c | 8 +++----- src/shared/bus-unit-util.c | 8 +++----- src/systemctl/systemctl-show.c | 14 +++++++++----- 6 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 87e8cdf7ee..961f112eb2 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -4,6 +4,7 @@ #include "sd-messages.h" +#include "af-list.h" #include "alloc-util.h" #include "blockdev-util.h" #include "bpf-devices.h" @@ -594,17 +595,18 @@ void cgroup_context_dump(Unit *u, FILE* f, const char *prefix) { } void cgroup_context_dump_socket_bind_item(const CGroupSocketBindItem *item, FILE *f) { - const char *family = - item->address_family == AF_INET ? "ipv4:" : - item->address_family == AF_INET6 ? "ipv6:" : ""; + const char *family, *colon; + + family = strempty(af_to_ipv4_ipv6(item->address_family)); + colon = isempty(family) ? "" : ":"; if (item->nr_ports == 0) - fprintf(f, " %sany", family); + fprintf(f, " %s%sany", family, colon); else if (item->nr_ports == 1) - fprintf(f, " %s%" PRIu16, family, item->port_min); + fprintf(f, " %s%s%" PRIu16, family, colon, item->port_min); else { uint16_t port_max = item->port_min + item->nr_ports - 1; - fprintf(f, " %s%" PRIu16 "-%" PRIu16, family, item->port_min, port_max); + fprintf(f, " %s%s%" PRIu16 "-%" PRIu16, family, colon, item->port_min, port_max); } } diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index a7d1cb6f12..c2e55bb972 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -5661,11 +5661,8 @@ int config_parse_cgroup_socket_bind( } if (rvalue) { - if (streq(word, "ipv4")) - af = AF_INET; - else if (streq(word, "ipv6")) - af = AF_INET6; - else { + af = af_from_ipv4_ipv6(word); + if (af == AF_UNSPEC) { log_syntax(unit, LOG_WARNING, filename, line, 0, "Only \"ipv4\" and \"ipv6\" protocols are supported, ignoring."); return 0; diff --git a/src/journal-remote/journal-remote.c b/src/journal-remote/journal-remote.c index 9600e5f732..13461dbe41 100644 --- a/src/journal-remote/journal-remote.c +++ b/src/journal-remote/journal-remote.c @@ -8,6 +8,7 @@ #include "sd-daemon.h" +#include "af-list.h" #include "alloc-util.h" #include "def.h" #include "errno-util.h" @@ -498,7 +499,7 @@ static int accept_connection( log_debug("Accepted %s %s connection from %s", type, - socket_address_family(addr) == AF_INET ? "IP" : "IPv6", + af_to_ipv4_ipv6(socket_address_family(addr)), a); *hostname = b; diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 8bcf4f6c50..31f527c571 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -12,6 +12,7 @@ #include "sd-bus.h" +#include "af-list.h" #include "alloc-util.h" #include "bus-container.h" #include "bus-control.h" @@ -821,11 +822,8 @@ static int parse_tcp_address(sd_bus *b, const char **p, char **guid) { return -EINVAL; if (family) { - if (streq(family, "ipv4")) - hints.ai_family = AF_INET; - else if (streq(family, "ipv6")) - hints.ai_family = AF_INET6; - else + hints.ai_family = af_from_ipv4_ipv6(family); + if (hints.ai_family == AF_UNSPEC) return -EINVAL; } diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 4d53aaa3da..54d04aae50 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +#include "af-list.h" #include "alloc-util.h" #include "bus-error.h" #include "bus-unit-util.h" @@ -879,11 +880,8 @@ static int bus_append_cgroup_property(sd_bus_message *m, const char *field, cons address_family = eq ? word : NULL; if (address_family) { - if (streq(address_family, "ipv4")) - family = AF_INET; - else if (streq(address_family, "ipv6")) - family = AF_INET6; - else + family = af_from_ipv4_ipv6(address_family); + if (family == AF_UNSPEC) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Only \"ipv4\" and \"ipv6\" protocols are supported"); } diff --git a/src/systemctl/systemctl-show.c b/src/systemctl/systemctl-show.c index d43f1cf094..4d68e08c80 100644 --- a/src/systemctl/systemctl-show.c +++ b/src/systemctl/systemctl-show.c @@ -2,6 +2,7 @@ #include +#include "af-list.h" #include "bus-error.h" #include "bus-locator.h" #include "bus-map-properties.h" @@ -1710,22 +1711,25 @@ static int print_property(const char *name, const char *expected_value, sd_bus_m return 1; } else if (STR_IN_SET(name, "SocketBindAllow", "SocketBindDeny")) { uint16_t nr_ports, port_min; - const char *family; int af; r = sd_bus_message_enter_container(m, SD_BUS_TYPE_ARRAY, "(iqq)"); if (r < 0) return bus_log_parse_error(r); while ((r = sd_bus_message_read(m, "(iqq)", &af, &nr_ports, &port_min)) > 0) { - family = af == AF_INET ? "ipv4:" : af == AF_INET6 ? "ipv6:" : ""; + const char *family, *colon; + + family = strempty(af_to_ipv4_ipv6(af)); + colon = isempty(family) ? "" : ":"; + if (nr_ports == 0) - bus_print_property_valuef(name, expected_value, flags, "%sany", family); + bus_print_property_valuef(name, expected_value, flags, "%s%sany", family, colon); else if (nr_ports == 1) bus_print_property_valuef( - name, expected_value, flags, "%s%hu", family, port_min); + name, expected_value, flags, "%s%s%hu", family, colon, port_min); else bus_print_property_valuef( - name, expected_value, flags, "%s%hu-%hu", family, port_min, + name, expected_value, flags, "%s%s%hu-%hu", family, colon, port_min, (uint16_t) (port_min + nr_ports - 1)); } if (r < 0) From a67abc490b1833a127e9ebdf62204a6b484ec0d1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 10 May 2021 17:47:19 +0200 Subject: [PATCH 4/5] tree-wide: move variables to innermost scope --- src/core/cgroup.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 961f112eb2..15b84dea06 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -203,12 +203,10 @@ void cgroup_context_remove_bpf_foreign_program(CGroupContext *c, CGroupBPFForeig } void cgroup_context_remove_socket_bind(CGroupSocketBindItem **head) { - CGroupSocketBindItem *h; - assert(head); while (*head) { - h = *head; + CGroupSocketBindItem *h = *head; LIST_REMOVE(socket_bind_items, *head, h); free(h); } From 11ab01e439e53d791c01fe980516e161ea382a32 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 10 May 2021 17:47:32 +0200 Subject: [PATCH 5/5] cgroup: drop explicit NULL comparisons --- src/core/cgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 15b84dea06..5453b5ae96 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -1581,7 +1581,7 @@ static bool unit_get_needs_socket_bind(Unit *u) { if (!c) return false; - return c->socket_bind_allow != NULL || c->socket_bind_deny != NULL; + return c->socket_bind_allow || c->socket_bind_deny; } static CGroupMask unit_get_cgroup_mask(Unit *u) {