With SignInitrdPCRs=disabled in mkosi the TPM/PCR services are broken
which triggers the error path in the varlink call which means that we
get parameters set in the response. The wrong ownership assumption when
looking at the reply with _cleanup_ set caused a memory corruption that
surfaced in systemd-sysext with an ABRT.
This is a common mistake done with the API, fix it by not setting
_cleanup_(sd_json_variant_unrefp).
The lack of error handling for sym_i2d_X509_NAME/sym_i2d_ASN1_INTEGER
means that we get a NULL iovec that will cause an assertion failure
later. The use of regular free for OpenSSL is probably okay but since
the allocator can differ, one should use OPENSSL_free.
Add error handling and do an explicit copy instead of ownership transfer
between possibly different allocators.
The disabling of features or components with the flag
--component-suggested/--feature-suggested didn't disable the suggested
ones but instead disabled all other ones. This is rather unintuitive due
to how the flags are named and also not really needed because the
intended reconciliation outcome can instead be done by first disabling
everything and then enabling the suggested ones again which is easier to
reason about. For components the tricky part is that they default to
enabled and thus it's better to have the disable/enable commands with
--component-suggested operate only on suggested ones instead of touching
others like "legacy" components that don't explicity say whether they
are enabled and suggested or not.
Make running disablement of components/features with
--component-suggested/--feature-suggested undo a previous enablement
with the same flags. Document how one can align the system to only use
suggested components/features and not anything else by doing it in two
steps, first disabling everything and then enabling suggested ones. This
also makes it clearer now that all components that are not explicitly
enabled nor suggested will be disabled then.
The auto-enable service should activate suggested components and
features but enabled all features which includes the default components
unsuggested features and any unsuggested features of the suggested
components. This is unexpected behavior and we rather want this service
to be limited to suggested features.
Switch the service flag to suggested and make the wording more explicit
in the man page. Also fix the wrong statement that it operates on
enabled components, it operates on all components, also explicitly
disabled ones.
When a reference specifies a registry like ghcr.io or quay.io for which
we don't have a registry config file, the fallback forced the default
registry which is wrong.
Only use the default as fallback when there is no explicit registry.
When we have two match patterns, e.g., because the repository layout
changed and we look for old and new style files, we can have the case
that one pattern suggests a descend into a subdir but the other pattern
would be a direct match. This was missed and only the descent was done.
Yet the other way round also has to be covered: We can't just use the
direct match because it might be that it's rejected if the type doesn't
fit (directory vs. regular file). In this case we should still descent.
Remember that we want to descend but continue checking the other
pattern first to maybe get a direct match. Introduce a new combined
return code YES_AND_RETRY to give the full information to the caller
which now can decide to fallback to a descent. While at it, use the
stat info for the directory check. Also add tests to ensure that the
order of patterns doesn't matter and we handle the above corner cases.
While for sysupdate it's fine to consume a large set of all possible
update payloads in a single directory this is not so handy for managing
and serving the update payloads. Since this large update folder is not
where the build output directly gets written to one has to create
copies and later possibly delete this added set of files.
Support matching for filenames in subdirectories by having a new **/
match pattern prefix which matches any number of nested subdirectories
or no subdirectory at all. For simplicity it's only allowed at the start
of a pattern and not a regular wildcard as the rest because the main
use case is to descend into subdirectories and only do the pattern
matching for the basenames. This way one can create a SHA256SUMS file
in the top folder and have it include all update payloads from the
release-specific (or arch-specific) subdirectories. Something similar
was already supported for directory sources where the match pattern can
start with a subdirectory path. Do also support this for SHA256SUMS for
parity while we are at it. Having the new wildcard makes mirroring also
easier because one does not have to follow the exact subdirectory layout
and one can filter by folder instead of by filename. It also makes it
possible to point the same transfer files with the new wildcard to
either a SHA256SUMS file that uses release-specific (or arch-specific)
subdirectories and includes all versions or to a SHA256SUMS file as
generated from mkosi that does not use subdirectories because it only
has files for a single version.
With the upcoming UAPI.16 JSON format we will also be able to encode
subdirectories and it makes sense to add this to SHA256SUMS for being
able to convert them. It also supports custom URLs for each entry which
is more powerful than the (arbitrary) subdirectory feature used here but
subdirectories have the advantage that they don't break mirroring.
This change also fixes the existing subdirectory handling bugs where
everything greater than two subdirectory levels failed to work because
rel_joined instead of de->d_name got used, symlinks were followed, and
we would continue silently on non-ENOENT errors.
To be able to continue trying other tokens to unlock a disk the missing
NV index case was mapped to EREMOTE (foreign TPM) which was ok because
this mostly happens when trying to unlock on another system. Still it
might be useful to deal with NV index errors differently.
Give it its own EADDRNOTAVAIL error and handle it at all call sites.
The plugin already continues on parsing errors but the fallback path
not. The plugin swallows ENOMEM as well which is too much, though.
Continue on JSON parsing errors by mapping them to EUCLEAN in
cryptsetup_get_token_as_json and handle that in any call site, not just
the TPM fallback path but also others.
When we iterate over tokens we should not print mismatches as errors
but rather warnings. At the end there is still a summary with the
notice level (gated by found_some) which the user can relate to the
warnings.
Currently libcryptsetup's look treats ENOANO special because it's used
to signal PIN requirement. But the bad PCR set can also contain ENOANO
for a mismatch from a PolicyOR branch.
To continue iterating, remap the bad PCR set to EPERM early. This would
in theory also allow us to simplify the matching for the iteration
condition but we leave this as is for now to prevent a future
regression.
When we enroll two UKIs with different PCR pub keys into one LUKS slot/
token each, we can encounter the wrong one and should not give up
but continue iterating.
We already report foreign TPM keys (wrapped for different parent) but
this is not enough because when also a different template was used, we
don't get TPM2_RC_INTEGRITY but TPM2_RC_SIZE.
Also cover TPM2_RC_SIZE to report EREMOTE so that we can continue to
iterate over LUKS tokens instead of giving up.
When we have many LUKS slots and not all are for our TPM, we can get an
NV index error when it's missing or has wrong content.
Instead of fully erroring out, map these encounters to EREMOTE like we
do for a foreign TPM key.
When we encounter a key for a foreign TPM we report that as EREMOTE in
tpm2_load but not yet in tpm2_import. This causes cryptsetup to give up
on using the TPM instead of being able to continue with out tokens.
Do the same mapping as in tpm2_load in tpm2_import to report EREMOTE on
foreign keys.
When we enroll two UKIs with different PCR pub keys into one LUKS slot/
token each, then we can encounter the wrong one and should not give up
but continue iterating instead of requiring the passphrase. Similarly,
a pcrlock token might be for another UKI and we should continue the
search. Same for a token that is meant for another TPM (e.g., external
storage). While this is mainly about cryptsetup's automatic unlocking
from the initrd, it also matters for usage in the system, e.g., for
other storage and when cryptenroll should unlock using the TPM.
There are two code paths, one is the libcryptsetup plugin and the other
is the fallback when that's not available.
To let the libcryptsetup loop continue to iterate, remap the above
error conditions to EPERM. For the fallback path check all of them (no
remapping) and continue iteration. For better log output, include the
token ID to be able understand which token fails.
When we enroll two UKIs with different PCR pub keys into one LUKS slot/
token each, then we can encounter the wrong one and should report it
with a clearer error than the generic "Failed to unseal secret using
TPM2".
So when we don't have the right signature, report this as separate
error.
With no console= given the kernel will use the graphical console and if
we give one console= then that will be used instead. But sometimes we
want both a serial console and a graphical one to work. This would be
consistent with the EFI menu and sd-boot showing on both already. It
also makes the impact of a wrongly detected-but-missing VirtIO console
lower when we emit it alongside of the other consoles we detect.
Collect all detected consoles and emit them via console= with the same
priority we used before to select them. This means for the main console
there is no change but we get additional ones enabled. This helps with
boot output and having a login presented, yet the main console is still
special and gets the emergency output at boot (we should somehow surface
that on the additional consoles but that's another topic). If we only
see the graphical console (or none), we don't need to emit anything a
the kernel already selects it itself. This also avoids suppressing the
non-x86 kernel serial console detection. As mentioned above, the VirtIO
console being wrongly added is now also less impactful. But for non-x86
ACPI case we could detect the serial so that console=hvc0 won't stop the
kernel serial auto-detection.
When nested mounts appear under a sysext hierarchy like this:
mkdir -p /opt/trigger/
mount -t tmpfs tmpfs /opt/trigger
mkdir -p /opt/trigger/inner
mount -t tmpfs tmpfs /opt/trigger/inner
Then systemd-sysext merge will lose the inner mount because it uses a
regular bind mount with propagation and then unmounts the source,
unmounting all children with it which propagates (as found out in
https://github.com/flatcar/Flatcar/issues/2111).
To solve this, clone the sub mount with MS_PRIVATE to decouple sub
mounts from the original mount. Then attach the cloned mount instead of
doing regular bind mounts. For old kernels we still attach the cloned
mount but we fallback to cloning without MS_PRIVATE. This change also
affects mount_private_apivfs which is used for private /proc, /sys, and
cgroupfs but I think it makes sense there, too, instead of only doing
mount_setattr for sysext alone because, e.g., a container and the host
should not be leaking mount actions into each other for these mounts.
When nested mounts appear under a sysext hierarchy like this:
mkdir -p /opt/trigger/
mount -t tmpfs tmpfs /opt/trigger
mkdir -p /opt/trigger/inner
mount -t tmpfs tmpfs /opt/trigger/inner
Then systemd-sysext merge hit an assertion reported in
https://github.com/flatcar/Flatcar/issues/2111 because when it iterates
over the list of sub mounts it doesn't expect entries with NULL in the
path from the dropped entries.
Instead of having to deal with entries with path NULL, better sort the
holes from dropping to the end and then reduce the array length.
With vmspawn --console=gui and similar one has the case where the VirtIO
PCI device is present for something else than the console and the logic
in sd-stub added console=hvc0 even though it didn't exist. This caused
tty0 to be empty during boot.
Always prefer the graphical console before a potential virtio console.
The virtio console is still preferred over a serial console and we can
get the same problem there, e.g., qemu-guest-agent is used causing the
PCI device but without a virtio console and instead a serial console is
used. This is not solved here.
Some state files were prefixed with the machine name which protected
against collisions when running concurrent machines in a systemd unit
with RuntimeDirectory= set. However, the socket files tpm.sock and
control were not, which caused a race. Prefixing these socket files
does not work because the path gets too long when both the subdirectory
and the socket file within it get unique identifiers.
Instead of prefixing all files, we can rather always create a
subdirectory and use simple names within in. This makes paths shorter
in the normal case and protects against races with RuntimeDirectory=
were instead of directly reusing RUNTIME_DIRECTORY we also create the
normal per-machine subdirectory. Since this here is about runtime state
it should not impact any running VMs/containers. The ssh-proxy looks
only at the normal case and does not support RUNTIME_DIRECTORY, so no
impact there as well.
The test first checks that simple signatures work and that a foreign
signature is rejected. Then it also tests that sub key signatures are
accepted when the public sub key is included in the signature.
With gpg sub keys one can rotate signing keys while having a stable
trust anchor. So far one still had to ship the sub key out of band but
a newer gpg has the option to include the sub key in the signature and
import it automatically. This is safe if we only allow importing a sub
key signed by the top key we already have in the key ring.
Add the --auto-key-import argument to gpg to import subkeys but also
set --import-options=merge-only,import-clean to restrict what we import
to only be sub keys signed by the top key we have in the keyring and
discard any irrelevant parts. The ugly part is that we also have to
work on a temporary copy of the keyring because gpg wants to persist
the added key material but we don't what that here.
By default there is a fixed keyring in /usr or /etc. But when running
systemd-pull unprivileged in the user context or with a custom transfer
definition as in systemd-sysupdate --definitions=./... (e.g., for local
ParticleOS updates) it is limiting to require that all keys have to be
part of the OS keyring or otherwise no verification can be used. Also,
for testing it is valuable to point it at a different keyring.
Add a SYSTEMD_OPENPGP_KEYRING env var where the omission or empty
assignment sticks to the current behavior of the global OS keyrings but
a keyring path given will take precedence. While an env var can leak
down the process tree and is more difficult to secure for being the
trust anchor the advantage is that one can directly specify it in the
service unit as drop-in instead of having to patch the command
invocation. Anyway it's a niche use case and thus not part of the man
page.
Up to now we recommended to use TARGET.upholds/ symlinks to start units
when an extension is loaded. However, this has some drawbacks. First,
for services that should not be tried to be started all the time we have
to resort to hiding them through a target that gets uphold and then
uses regular .wants/ for the actual service. Second, we actually leak
services on extension unload even if the unit has disappeared with the
extension. Third, to affect a service through a drop-in or a config
change from a confext/sysext and that service is already running, we
need a way to restart/reload it instead of just starting it.
Similar to EXTENSION_RELOAD_MANAGER=1, add a EXTENSION_RESTART_UNITS=
and a EXTENSION_RELOAD_OR_RESTART_UNITS= setting to the
extension-release metadata file, carrying a whitespace-separated list
of units to restart/reload on merge/refresh/unmerge after the daemon
reload. Also detect when the unit has vanished which is normally the
case when the unit was part of the unmerged extension, and stop it
explicitly to prevent it leaking. When the extension itself ships the
binary it should use EXTENSION_RESTART_UNITS= to make sure the new
binary is picked up. Since starting through this setting does not work
when the extension is mounted from the initrd, extensions should still
ship at least a .wants/ symlink to start at boot but can also continue
to ship a .upholds/ symlink for backwards compatibility without any
drawback and still benefit from the unit stopping triggered by the new
setting. While there are cases where one could want to set
EXTENSION_RESTART_UNITS= without requiring a daemon reload (e.g., an
env var file change instead of a unit drop-in), we now do an implicit
daemon reload when we have to restart units so that we know we work on
the right state and we spare users remembering to set this setting in
addition to prevent running into this issue.
When systemd-pull encountered a file shorter than the compression magic
headers it looks for, then it would complete the download in the
analysis state and fail.
When we are still in the analysis state and the download is done, we
know there is no compression and we should leave the analysis state and
continue writing out to disk as usual.
To call into pull_job_write_(un)compressed and pull_job_open_disk from
pull_job_curl_on_finished it has to be defined after them. This is in
preparation for a bug fix for small files where we need to leave the
compression analysis state to finish the download successfully.
ASAN showed a use-after-free error for systemd-vmspawn's
machine_register call because the reply got accessed and freed again
through _cleanup. The same problem exists in two
verb_machine_control_one/unregister_machine.
Fix these call sites to not set up _cleanup.
This came up when running systemd-vmspawn with ASAN to fix another bug
and thus I had to fix this overrun here first: The dispatch tables were
missing the terminator, add it.
In 012d87c1fc/cc8f398202 it was made possible for fork_notify() to
return in the child but at that point all FDs were closed and the
_cleanup path from the return causes assertion failures due to invalid
FDs in notify_event_source/event, leading to a vmspawn failing to start
with a SIGABRT logged in coredump.
Instead of TAKE_PTR on a bunch of things which is fragile, rather avoid
the return and instead add an explicit callback handler and guarantee
to exit directly after it. A userdata argument is also added but not
used yet I think it's quite normal to have for a callback.
For qcow2 images it's not enough to grow the file. Since we probably
don't want to shell out to qemu-img either let's just error out to make
the user aware that growing needs to be done manually.
As soon as machinectl list-images has an mstack entry updatectl fails
because systemd-sysupdated crashes with an assertion failing because
the mstack case was not handled.
For now mstack is not supported as image for sysupdate to operate on
and we can skip it.
Fixes https://github.com/systemd/systemd/issues/41649
In ccecae0efd ("vmspawn: use machine name in runtime directory path")
support for RUNTIME_DIRECTORY was dropped which makes it difficult to
run systemd-vmspawn in a service unit which doesn't have write access
to the regular /run but should use its own managed RUNTIME_DIRECTORY.
What worked before was --keep-unit --system but we can't use
XDG_RUNTIME_DIR and --user because then --keep-unit breaks which
we need because it can't create a scope as there is no session.
Switch back to runtime_directory which handles RUNTIME_DIRECTORY and
tells us whether we should use it as is without later cleanup or if we
need to use the regular path where we create and delete the directory
ourselves.
When install_subdir encounters a mkosi.tools tree with a /bin to
/usr/bin symlink it fails to copy it because it dereferences but still
treats it like a file.
Work around the Meson bug by excluding the mkosi.tools tree from
installation like mkosi.local is excluded. We anyway don't want the
tools tree end up there.
The socket failed on boot due to a ordering problem caused by
systemd-sysusers running later and thus the socket group was still
missing. The socket unit can only start when systemd-sysusers created
the group.
Order the socket after systemd-sysusers.service to have the group
available.
Observed with the tpm2 provider and the tpm2tss engine was that the
auth process failed because the provider/engine could not ask for the
PIN through the callback, resulting in:
"Failed to load private key from ...: Input/output error"
Apparently the default UI method is not enough and the key setup
functions expect an explicit method.
Pass the existing UI method through as callback for the key setup.
The indirect discard in mkfs.btrfs on the loop device mapped to the
region on disk can hang and fail the first-boot creation of the rootfs.
Since there already is a discard done we anyway don't need to do it
twice. This might help for most cases to avoid the failure in
mkfs.btrfs.
Keep track if the direct discard worked and then skip the mkfs.btrfs
discard if it did. This still leaves the case where mkfs.btrfs can hang
when the direct discard couldn't succeed and mkfs.btrfs tries again but
since the conditions are rather the same it might be that this case is
not easy to trigger. If the problem still shows up and the kernel won't
be fixed soon we can still disable the mkfs discard for at least btrfs.
Kai Lüke