For system users we should lock things down, hence generate an empty
list.
This is mostly a safety precaution, but also hides really confusing
output of "userdbctl user" for an system user.
Follow-up for: a192250eda
The second check was searching the symbols into the same array, but
using the size of the other. This generated a SIGSEV when they
occassionally mismatched.
The auditing subsystem is still not virtualized for containers, hence
the two values don't really make sense inside them, they will just leak
information from outside into the container. Hence don't make use of the
data if we detect we are run inside of a container.
This has visible effects: logind will no longer try to reuse the
auditing session ids as its own session ids when run inside a container.
While are at it, modernize the calls in more ways:
1. switch to pidref behaviour, all but one of our uses are using pidref
anyway already.
2. use read_virtual_file() + proc_mounted()
3. reasonably distinguish ENOENT errors when reading the process proc
files: distinguish the case where /proc is not mounted, from the case
where the process is already gone, from where auditing is not enabled in
the kernel build.
The auditing subsystem is still not virtualized for containers, hence the two
values don't really make sense inside them, they will just leak
information from outside into the container. Hence don't make use of the
data if we detect we are run inside of a container.
This has visible effects: logind will no longer try to reuse the
auditing session ids as its own session ids when run inside a container.
While are at it, modernize the calls in more ways:
1. switch to pidref behaviour, all but one of our uses are using pidref
anyway already.
2. use read_virtual_file() + proc_mounted()
3. reasonable distinguish ENOENT errors when reading the process proc
files: distinguish the case where /proc is not mounted, from the case
where the process is already gone, from where auditing is not enabled
in the kernel build.
This makes use of the new TIOCGPTPEER pty ioctl() for directly opening a
PTY peer, without going via path names. This is nice because it closes a
race around allocating and opening the peer. And also has the nice
benefit that if we acquired an fd originating from some other
namespace/container, we can directly derive the peer fd from it, without
having to reenter the namespace again.
Let's make sure that sd_varlink_error() always returns an error code, so
that we can use it in a style "return sd_varlink_error(…);" everywhere,
which has two effects: return a good error reply to clients, and exit
the current stack frame with a failure code.
Interestingly sd_varlink_error_invalid_parameter() already worked like
this in some cases, but sd_varlink_error() itself didn't.
This is an alternative to the error handling tweak proposed in #34882,
but I think is a lot more generically useful, since it establishes a
pattern.
I checked our codebase, and this change should generally be OK without
breaking callsites, since the current callers (with exception of the
machined case from #34882) called sd_varlink_error() in the outermost
varlink method call dispatch stack frame, where this behaviour change
does not alter anything.
This is similar btw, how sd_bus_error_setf() and friends always return
error codes too, synthesized from its parameters.
This makes use of the infra introduced in 229d4a9806 to indicate visually on each prompt that we are in superuser mode temporarily.
pick ad5de3222f userdbctl: add some basic client-side filtering
One of the big mistakes of Linux is that when you create a file with
open() and O_CREAT and the file already exists as dangling symlink that
the symlink will be followed and the file created that it points to.
This has resulted in many vulnerabilities, and triggered the creation of
the O_MOFOLLOW flag, addressing the problem.
O_NOFOLLOW is less than ideal in many ways, but in particular one: when
actually creating a file it makes sense to set, because it is a problem
to follow final symlinks in that case. But if the file is already
existing, it actually does make sense to follow the symlinks. With
openat_report_new() we distinguish these two cases anyway (the whole
function exists only to distinguish the create and the exists-already
case after all), hence let's do something about this: let's simply never
create files "through symlinks".
This can be implemented very easily: just pass O_NOFOLLOW to the 2nd
openat() call, where we actually create files.
And then basically remove 0dd82dab91
again, because we don't need to care anymore, we already will see ELOOP
when we touch a symlink.
Note that this change means that openat_report_new() will thus start to
deviate from plain openat() behaviour in this one small detail: when
actually creating files we will *never* follow the symlink. That should
be a systematic improvement of security.
Fixes: #34088
We have two distinct implementations of the post hook.
1. For SELinux we just reset the selinux label we told the kernel
earlier to use for new inodes.
2. For SMACK we might apply an xattr to the specified file.
The two calls are quite different: the first call we want to call in all
cases (failure or success), the latter only if we actually managed to
create an inode, in which case it is called on the inode.
This PidRef just track some data, but cannot be used for any active
operation.
Background: for https://github.com/systemd/systemd/pull/34703 it makes
sense to track explicitly if some PidRef is not a local one, so that we
never attempt to for example "kill a remote process" and thus
acccidentally hit the wrong process (i.e. a local one by the same PID).
Zbigniew Jędrzejewski-Szmek
Lennart Poettering
Daan De Meyer
Federico Giovanardi
Yu Watanabe
anonymix007
Luca Boccassi
Andres Beltran
Luca Boccassi
Adrian Vovk
Mike Yuan
Integral