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.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek
2025-07-05 09:22:16 +02:00
parent fb98c75e0e
commit 41288f2daf
2 changed files with 39 additions and 13 deletions

View File

@@ -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 },
{}

View File

@@ -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",