The verity user certs weren't looked up in the given --root= for
systemd-sysext which made it fail to set up extensions with a strict
image policy.
Look up verity user certs from inside the --root= when we operate on
images in it. The main use case where this matters is when the initrd
sets up the extensions for the final system and thus systemd-sysext
should do the same thing as it would do in the final system.
This has been supported since systemd v242 (specifically commit
7619cb32f0 if I’m not mistaken; added to NEWS in commit 4107452e51), but
the man page still claimed otherwise.
The `sg_adm` and `sg_mem` fields are not always set in shadow groups,
which can lead to issues with foreign tools like shadow's `sg` command.
Since other NSS implementations properly set these fields and it would
otherwise be impossible to access `administrators` and `members`
information from JSON files, it's bets to always fill these fields.
Even though `sg` is a nice example which should be already installed,
the issue itself can be reproduced with this simple program as well. It
relies on filled `sg_adm` and `sg_mem` fields just like `sg` does:
```
#include <err.h>
#include <gshadow.h>
#include <stdio.h>
int
main(int argc, char *argv[])
{
struct sgrp *s;
char **p;
if (argc != 2)
errx(1, "usage: poc group");
s = getsgnam(argv[1]);
printf("name: %s\n", s->sg_namp);
printf("admins:\n");
p = s->sg_adm;
while (*p != NULL) {
printf("- %s\n", *p);
p++;
}
printf("members:\n");
p = s->sg_mem;
while (*p != NULL) {
printf("- %s\n", *p);
p++;
}
}
```
Run it like this: `./poc root`
Proof of Concept (Arch Linux, which uses systemd with systemd-userdbd
and shadow's sg):
```
$ grep systemd /etc/nsswitch.conf
passwd: files systemd
group: files [SUCCESS=merge] systemd
shadow: files systemd
gshadow: files systemd
```
Issue with intrinsic groups:
Run as unprivileged user, who has no access to `/etc/gshadow` to trigger
nss-systemd (strace disables setuid of sg)
```
$ strace sg root
write(2, "sg: list.c:169: is_on_list: Asse"..., 61sg: list.c:169: is_on_list: Assertion `NULL != list' failed.
) = 61
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fa7e9c0c000
gettid() = 1882
getpid() = 1882
tgkill(1882, 1882, SIGABRT) = 0
--- SIGABRT {si_signo=SIGABRT, si_code=SI_TKILL, si_pid=1882, si_uid=1000} ---
+++ killed by SIGABRT (core dumped) +++
Aborted (core dumped) strace sg root
```
Issue with groups through systemd-userdbd:
1. Create a custom group (as root)
```
cat > /etc/userdb/sg-poc.group << EOF
{
"groupName": "sg-poc",
"gid": 6123,
"administrators": [
"root"
],
"members": [
"bin"
]
}
EOF
ln -s sg-poc.group /etc/userdb/6123.group
```
2. Verify that group actually exists
```
$ userdbctl group sg-poc
Group name: sg-poc
Disposition: regular
GID: 6123
Admins: root
Service: io.systemd.NameServiceSwitch
```
3. Run `sg` to switch into group `sg-poc` as regular user, this time
with setuid, i.e. no strace as before
```
$ sg sg-poc
sg: list.c:169: is_on_list: Assertion `NULL != list' failed.
Aborted (core dumped) sg sg-poc
```
In recent Fedora, preset-all fails:
[ 155s] Failed to preset unit: File '/buildroot/etc/systemd/user/dbus.service'
already exists and is a symlink to /usr/lib/systemd/user/dbus-broker.service
[ 155s] ‣ "systemctl --root=/buildroot --global preset-all" returned non-zero exit code 1.
Strictly speaking, this is an error in configuration. The presets specify that
both dbus-broker.service and dbus-daemon.service shall be enabled and they both
claim the 'dbus.service' alias. But this kind of error is very easy to make.
Failing the preset operation is too harsh, since in most cases the system will
work fine without an alias and changes in unrelated components can cause the
conflict.
Let's reuse the same logic that was added in
ad5fdd3912: when enabling the unit through
'preset' or 'preset-all', print the message, but suppress the error. When
enabling through 'enable', fail the operation.
Add a test for getsgnam_r to verify that sg_adm and sg_mem always point
to a NULL-terminated string vector.
Extend the gr_mem check of struct group for non-NULL values as well.
Fill sg_adm and sg_mem in nss_pack_group_record_shadow to stay
compatible with other NSS getsgnam implementations which set these
members to NULL terminated string arrays.
Tools like shadow's sg would trigger a NULL pointer dereference with
groups only found through nss-systemd otherwise.
The sg_adm and sg_mem fields are supposed to point to a NULL terminated
string array. If these are NULL, some foreign tools like shadow's sg
trigger NULL pointer dereferences (or fortunately their asset() calls).
Previously, if execution failed, we'd log at error level both from the
child and the parent, and we were using a bogus variable for the argument
name:
$ build/systemd-inhibit list
Failed to execute : No such file or directory
list failed with exit status 1.
In general, we can and should assume that the program the user is calling
is well behaved, so it'll log the error on its own if appropriate. So we
shouldn't log on "normal errors", but only if the child is terminated by
a signal.
And since the program name is controlled by the user, use quotes everywhere
to avoid ambiguity.
Now:
$ build/systemd-inhibit false
(nothing)
$ build/systemd-inhibit bash -c 'kill -SEGV $$'
src/basic/process-util.c:895: 'bash' terminated by signal SEGV.
menu-force and menu-hidden were added in 97f077df05,
menu-disable was added in 6efdd7fec5, a year later.
So we can assume that if the feature flag is set, the other string values are
supported too. The comment that there's no way check that was added later in
5b45fad4fc, but it was incorrect even at that
time.
Fixes https://github.com/systemd/systemd/issues/39167. As described in the
issue, we documented various string values in the BLI, but bootctl didn't use
the string values. At the time menu-force and menu-hidden were added, using
numerical values for compatibility made sense. But that stopped being needed
when a string value that didn't have a strictly equivalent numerical value and
a feature flag were added.
When converting a large number to menu-force, message is downgraded to debug,
since the severity of the issue is very minor. Debug messages are added in
other places when the requested setting is modified too.
We do the rm_rf_children() call only because in some cases we cannot
pivot_root() and hence the orginal root superblock stays pinned, and we
thus have to empty it to minimize its memory use. But if pivot_root()
worked (and the umount() for the old root), then there's really no need
to do this work.
Dropping this codepath is useful in context of Christian's recent work
to make the original initrd tmpfs unmountable, which means pivot_root()
will work, and thus there's no need to empty the tmpfs anymore, and we
can speed up boot a bit.
Fixes: #40250
With a service with DynamicUser= with static user or group, e.g.,
```
$ systemd-run -p DynamicUser=yes -p Group=disk sleep infinity
```
previously the lookup by name and ID through DBus/Varlink are inconsistent:
```
$ busctl call org.freedesktop.systemd1 /org/freedesktop/systemd1 org.freedesktop.systemd1.Manager LookupDynamicUserByUID "u" 6
Call failed: Dynamic user ID 6 does not exist.
$ busctl call org.freedesktop.systemd1 /org/freedesktop/systemd1 org.freedesktop.systemd1.Manager LookupDynamicUserByName "s" disk
u 6
$ userdbctl group 6
Group name: disk
Disposition: system
GID: 6
Passwords: 1
Service: io.systemd.NameServiceSwitch
$ userdbctl group disk
Group name: disk
Disposition: dynamic
GID: 6
Description: Dynamic Group
Service: io.systemd.DynamicUser
```
With this change, the results of these methods are consistent.
Fixes#40228.
If we're doing foreign UID range copying, we're going to be joining
a private user namespace before doing the copy. copy_tree() insists
on keeping all UIDs/GIDs the same when copying. Hence, all the
UIDs/GIDs of the files we're copying should be in the private UID
range, which means they need to be owned by the foreign UID range
and we always need to call mountfsd_mount_directory_fd(). So there's
no point in having a fallback path if the source directory is not
foreign UID range owned, we'd simply fail to copy it later. Hence,
insist on the source directory being foreign UID range owned.
The definition will use i because of the macro, so
let's use i in the declaration as well. We can't
use DECLARE_STRING_TABLE_LOOKUP_TO_STRING() because
sd-json.h is a libsystemd public header.
blkid.h and gmessages.h both use const for arguments that are passed
by value, which is pointless and triggers clang-tidy warnings, so exclude
them from processing.
Let's always use ret, ret_data and ret_size for output arguments.
size is better than length in this case because we're dealing with
arbitrary, possibly binary, data and not strings.
boot_id is already passed by value, and hence copied.
Since we don't apply const to function parameters
that are copied anywhere else, let's drop the const
here as well for consistency.
Kai Lueke
Yu Watanabe
Mike Yuan
Lucas Werkmeister
Zbigniew Jędrzejewski-Szmek
Tobias Stoeckmann
Lennart Poettering
David Santamaría Rogado
DaanDeMeyer
Antonio Alvarez Feijoo
Ulrich Ölmann