Move warning about unsupported BPF firewall right before the firewall would be created

There's no need to warn about the firewall when parsing, because the unit might
not be started at all. Let's warn only when we're actually preparing to start
the firewall.

This changes behaviour:
- the warning is printed just once for all unit types, and not once
  for normal units and once for transient units.
- on repeat warnings, the message is not printed at all. There's already
  detailed debug info from bpf_firewall_compile(), so we don't need to repeat
  ourselves.
- when we are not root, let's say precisely that, not "lack of necessary privileges"
  and "the local system does not support BPF/cgroup firewalling".

Fixes #12673.

diff --git a/src/core/bpf-firewall.c b/src/core/bpf-firewall.c
index 723c7b4b4e1cc7a26118188271112b7de07397ac..32eb8700e356a680561aae97a18454d334d06e74 100644 (file)
--- a/src/core/bpf-firewall.c
+++ b/src/core/bpf-firewall.c
@@ -484,19 +484,17 @@ int bpf_firewall_compile(Unit *u) {
         supported = bpf_firewall_supported();
         if (supported < 0)
                 return supported;
-        if (supported == BPF_FIREWALL_UNSUPPORTED) {
-                log_unit_debug(u, "BPF firewalling not supported on this manager, proceeding without.");
-                return -EOPNOTSUPP;
-        }
-        if (supported != BPF_FIREWALL_SUPPORTED_WITH_MULTI && u->type == UNIT_SLICE) {
+        if (supported == BPF_FIREWALL_UNSUPPORTED)
+                return log_unit_debug_errno(u, SYNTHETIC_ERRNO(EOPNOTSUPP),
+                                            "BPF firewalling not supported on this manager, proceeding without.");
+        if (supported != BPF_FIREWALL_SUPPORTED_WITH_MULTI && u->type == UNIT_SLICE)
                 /* If BPF_F_ALLOW_MULTI is not supported we don't support any BPF magic on inner nodes (i.e. on slice
                  * units), since that would mean leaf nodes couldn't do any BPF anymore at all. Under the assumption
                  * that BPF is more interesting on leaf nodes we hence avoid it on inner nodes in that case. This is
                  * consistent with old systemd behaviour from before v238, where BPF wasn't supported in inner nodes at
                  * all, either. */
-                log_unit_debug(u, "BPF_F_ALLOW_MULTI is not supported on this manager, not doing BPF firewall on slice units.");
-                return -EOPNOTSUPP;
-        }
+                return log_unit_debug_errno(u, SYNTHETIC_ERRNO(EOPNOTSUPP),
+                                            "BPF_F_ALLOW_MULTI is not supported on this manager, not doing BPF firewall on slice units.");
 
         /* Note that when we compile a new firewall we first flush out the access maps and the BPF programs themselves,
          * but we reuse the the accounting maps. That way the firewall in effect always maps to the actual
@@ -766,3 +764,15 @@ int bpf_firewall_supported(void) {
                 return supported = BPF_FIREWALL_UNSUPPORTED;
         }
 }
+
+void emit_bpf_firewall_warning(Unit *u) {
+        static bool warned = false;
+
+        if (!warned) {
+                log_unit_warning(u, "unit configures an IP firewall, but %s.\n"
+                                    "(This warning is only shown for the first unit using IP firewalling.)",
+                                 getuid() != 0 ? "not running as root" :
+                                                 "the local system does not support BPF/cgroup firewalling");
+                warned = true;
+        }
+}

diff --git a/src/core/bpf-firewall.h b/src/core/bpf-firewall.h
index 7d38483dbd5288e725b9fee7f72a0217f7086568..10cafcc02e01bdf32778da10f43b23448de15064 100644 (file)
--- a/src/core/bpf-firewall.h
+++ b/src/core/bpf-firewall.h
@@ -18,3 +18,5 @@ int bpf_firewall_install(Unit *u);
 
 int bpf_firewall_read_accounting(int map_fd, uint64_t *ret_bytes, uint64_t *ret_packets);
 int bpf_firewall_reset_accounting(int map_fd);
+
+void emit_bpf_firewall_warning(Unit *u);

diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index 042a742fa9b4b7132545adc2858a200da90e3a61..a7263855dcac2b5d3f24025fd263a79e1d7f05d2 100644 (file)
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -1540,6 +1540,10 @@ CGroupMask unit_get_target_mask(Unit *u) {
          * hierarchy that shall be enabled for it. */
 
         mask = unit_get_own_mask(u) | unit_get_members_mask(u) | unit_get_siblings_mask(u);
+
+        if (mask & CGROUP_MASK_BPF_FIREWALL & ~u->manager->cgroup_supported)
+                emit_bpf_firewall_warning(u);
+
         mask &= u->manager->cgroup_supported;
         mask &= ~unit_get_ancestor_disable_mask(u);
 

diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c
index d75628c663b6623dfa6eb5773dbed3e0c92d092c..9f4fd06dc4094e6ca086107d9024ba77172fca7a 100644 (file)
--- a/src/core/dbus-cgroup.c
+++ b/src/core/dbus-cgroup.c
@@ -1426,21 +1426,6 @@ int bus_cgroup_set_property(
                                 return r;
 
                         unit_write_setting(u, flags, name, buf);
-
-                        if (*list) {
-                                r = bpf_firewall_supported();
-                                if (r < 0)
-                                        return r;
-                                if (r == BPF_FIREWALL_UNSUPPORTED) {
-                                        static bool warned = false;
-
-                                        log_full(warned ? LOG_DEBUG : LOG_WARNING,
-                                                 "Transient unit %s configures an IP firewall, but the local system does not support BPF/cgroup firewalling.\n"
-                                                 "Proceeding WITHOUT firewalling in effect! (This warning is only shown for the first started transient unit using IP firewalling.)", u->id);
-
-                                        warned = true;
-                                }
-                        }
                 }
 
                 return 1;

diff --git a/src/core/ip-address-access.c b/src/core/ip-address-access.c
index 1d431bb674d8f5e7648e54609c3534a8d6100223..36cec70c2c212f0b890735a3250bc34ceb99c818 100644 (file)
--- a/src/core/ip-address-access.c
+++ b/src/core/ip-address-access.c
@@ -134,21 +134,6 @@ int config_parse_ip_address_access(
 
         *list = ip_address_access_reduce(*list);
 
-        if (*list) {
-                r = bpf_firewall_supported();
-                if (r < 0)
-                        return r;
-                if (r == BPF_FIREWALL_UNSUPPORTED) {
-                        static bool warned = false;
-
-                        log_full(warned ? LOG_DEBUG : LOG_WARNING,
-                                 "File %s:%u configures an IP firewall (%s=%s), but the local system does not support BPF/cgroup based firewalling.\n"
-                                 "Proceeding WITHOUT firewalling in effect! (This warning is only shown for the first loaded unit using IP firewalling.)", filename, line, lvalue, rvalue);
-
-                        warned = true;
-                }
-        }
-
         return 0;
 }