From: Zbigniew Jędrzejewski-Szmek Date: Sat, 5 Jul 2025 07:22:16 +0000 (+0200) Subject: shared/bus-unit-util: fix PrivateTmp=/PrivateUsers=/ProtectControlGroups= and Ex... X-Git-Tag: v258-rc1~150^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=41288f2daf2c72d0a8614571cfc3de91ecdee372;p=thirdparty%2Fsystemd.git shared/bus-unit-util: fix PrivateTmp=/PrivateUsers=/ProtectControlGroups= and Ex variants For some fields, we perform careful parsing and verification on the sender side. For other fields, we accept any string or strv. I think that actually this is fine: we should optimize for the correct case, i.e. the user runs a command that is valid. The server must perform parsing in all cases, so doing the verification on the sender side doesn't add value. When doing parsing locally, in case of invalid or unsupported input, we would generate the error message locally, so we would avoid the D-Bus call, but the message itself is not better and from the user's point of view, the result is the same. And by doing the parsing only on the server side, we deal better with the case where the sender has an older version of the software. By not doing verification, we implicitly "support" new values. And when the sender has a newer version that supports additional fields, that does not help as long as the server uses an older version. So in case of version mismatches, parsing on the server side is as good or better. Resolves https://github.com/systemd/systemd/issues/37174. --- diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index 48bee05bb3b..97bf9d264ec 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -2088,6 +2088,28 @@ static int bus_append_protect_hostname(sd_bus_message *m, const char *field, con return 1; } +static int bus_append_boolean_or_ex_string(sd_bus_message *m, const char *field, const char *eq) { + int r; + + r = parse_boolean(eq); + if (r >= 0) { + if (endswith(field, "Ex")) + field = strndupa_safe(field, strlen(field) - 2); + + r = sd_bus_message_append(m, "(sv)", field, "b", r); + } else { + if (!endswith(field, "Ex")) + field = strjoina(field, "Ex"); + + /* We allow any string through and let the server perform the verification. */ + r = sd_bus_message_append(m, "(sv)", field, "s", eq); + } + if (r < 0) + return bus_log_create_error(r); + + return 1; +} + static int bus_append_paths(sd_bus_message *m, const char *field, const char *eq) { return bus_append_trivial_array(m, "Paths", eq, "a(ss)", field, eq); @@ -2363,9 +2385,6 @@ static const BusProperty execute_properties[] = { { "SyslogIdentifier", bus_append_string }, { "ProtectSystem", bus_append_string }, { "ProtectHome", bus_append_string }, - { "PrivateTmpEx", bus_append_string }, - { "PrivateUsersEx", bus_append_string }, - { "ProtectControlGroupsEx", bus_append_string }, { "SELinuxContext", bus_append_string }, { "RootImage", bus_append_string }, { "RootVerity", bus_append_string }, @@ -2385,10 +2404,8 @@ static const BusProperty execute_properties[] = { { "TTYVHangup", bus_append_parse_boolean }, { "TTYReset", bus_append_parse_boolean }, { "TTYVTDisallocate", bus_append_parse_boolean }, - { "PrivateTmp", bus_append_parse_boolean }, { "PrivateDevices", bus_append_parse_boolean }, { "PrivateNetwork", bus_append_parse_boolean }, - { "PrivateUsers", bus_append_parse_boolean }, { "PrivateMounts", bus_append_parse_boolean }, { "PrivateIPC", bus_append_parse_boolean }, { "NoNewPrivileges", bus_append_parse_boolean }, @@ -2401,7 +2418,6 @@ static const BusProperty execute_properties[] = { { "ProtectKernelModules", bus_append_parse_boolean }, { "ProtectKernelLogs", bus_append_parse_boolean }, { "ProtectClock", bus_append_parse_boolean }, - { "ProtectControlGroups", bus_append_parse_boolean }, { "MountAPIVFS", bus_append_parse_boolean }, { "BindLogSockets", bus_append_parse_boolean }, { "CPUSchedulingResetOnFork", bus_append_parse_boolean }, @@ -2456,7 +2472,7 @@ static const BusProperty execute_properties[] = { { "LoadCredential", bus_append_load_credential }, { "LoadCredentialEncrypted", bus_append_load_credential }, { "ImportCredential", bus_append_import_credential }, - { "ImportCredentialEx", bus_append_import_credential }, + { "ImportCredentialEx", bus_append_import_credential }, /* compat */ { "LogExtraFields", bus_append_log_extra_fields }, { "LogFilterPatterns", bus_append_log_filter_patterns }, { "StandardInput", bus_append_standard_inputs }, @@ -2491,7 +2507,13 @@ static const BusProperty execute_properties[] = { { "CacheDirectory", bus_append_directory }, { "LogsDirectory", bus_append_directory }, { "ProtectHostname", bus_append_protect_hostname }, - { "ProtectHostnameEx", bus_append_protect_hostname }, + { "ProtectHostnameEx", bus_append_protect_hostname }, /* compat */ + { "PrivateTmp", bus_append_boolean_or_ex_string }, + { "PrivateTmpEx", bus_append_boolean_or_ex_string }, /* compat */ + { "ProtectControlGroups", bus_append_boolean_or_ex_string }, + { "ProtectControlGroupsEx", bus_append_boolean_or_ex_string }, /* compat */ + { "PrivateUsers", bus_append_boolean_or_ex_string }, + { "PrivateUsersEx", bus_append_boolean_or_ex_string }, /* compat */ { NULL, bus_try_append_resource_limit, dump_resource_limits }, {} diff --git a/src/test/test-bus-unit-util.c b/src/test/test-bus-unit-util.c index 677615a2d3e..2b21f119e54 100644 --- a/src/test/test-bus-unit-util.c +++ b/src/test/test-bus-unit-util.c @@ -271,11 +271,17 @@ TEST(execute_properties) { "ProtectHome=true", "ProtectHome=read-only", "ProtectHome=tmpfs", - - "PrivateTmpEx=true", - "PrivateTmpEx=false", + "PrivateTmp=true", + "PrivateTmp=fAlSe", + "PrivateTmpEx=0001", + "PrivateTmpEx=0000", + "PrivateTmp=asdf", + "PrivateTmpEx=asdf", + "PrivateUsers=no", + "PrivateUsers=1", "PrivateUsersEx=true", "PrivateUsersEx=false", + "PrivateUsers=whatever", "ProtectControlGroupsEx=true", "ProtectControlGroupsEx=false", @@ -314,10 +320,8 @@ TEST(execute_properties) { "TTYVHangup=true", "TTYReset=true", "TTYVTDisallocate=true", - "PrivateTmp=true", "PrivateDevices=true", "PrivateNetwork=true", - "PrivateUsers=true", "PrivateMounts=true", "PrivateIPC=true", "NoNewPrivileges=true",