Using fs-verity is natural for OSTree because it's file-based,
as opposed to block based (like dm-verity). This only covers
files - not symlinks or directories. And we clearly need to
have integrity for the deployment directories at least.
Also, what we likely need is an API that supports signing files
as they're committed.
So making this truly secure would need a lot more work. Nevertheless,
I think it's time to start experimenting with it. Among other things,
it does *finally* add an API that makes files immutable, which will
help against some accidental damage.
This is basic enablement work that is being driven by
Fedora CoreOS; see also https://github.com/coreos/coreos-assembler/pull/876
Currently `ostree_gpg_verify_result_require_valid_signature` always
returns an error that the key used for the signature is missing from the
keyring. However, all that's been determined is that there are no valid
signatures. The error could also be from an expired signature, an
expired key, a revoked key or an invalid signature.
Provide values for these missing errors and return them from
`ostree_gpg_verify_result_require_valid_signature`. The description of
each result is appended to the error message, but since the result can
contain more than one signature but only a single error can be returned,
the status of the last signature is used for the error code. See the
comment for rationale.
Related: flatpak/flatpak#1450
This function parses the object listing in the `ostree.sizes` metadata
and returns an array of `OstreeCommitSizesEntry` structures.
Unfortunately, for reasons I don't understand, the linker wants to
resolve `_ostree_read_varuint64` from `ostree-core.c` even though it's
not used by `test-checksum.c` at all.
Append a byte encoding the OSTree object type for each object in the
metadata. This allows the commit metadata to be fetched and then for the
program to see which objects it already has for an accurate calculation
of which objects need to be downloaded.
This slightly breaks the `ostree.sizes` `ay` metadata entries. However,
it's unlikely anyone was asserting the length of the entries since the
array currently ends in 2 variable length integers. As far as I know,
the only users of the sizes metadata are the ostree test suite and
Endless' eos-updater[1]. The former is updated here and the latter
already expects this format.
1. https://github.com/endlessm/eos-updater/
If the object was already in the repo then the sizes metadata entry was
skipped. Move the sizes entry creation after the data has been computed
but before the early return for an existing object.
The object sizes hash table was only being cleared when the repo was
finalized. That means that performing multiple commits while the repo
was open would reuse all the object sizes metadata for each commit.
Clear the hash table when the sizes metadata is setup and when it's
added to a commit. This still does not fix the issue all the way since
it does nothing to prevent the program from constructing multiple
commits simultaneously. To handle that, the object sizes hash table
should be attached to the MutableTree since that has the commit state.
However, the MutableTree is gone when the commit is actually created.
The hash table would have to be transferred to the root file when
writing the MutableTree. That would be an awkward addition to
OstreeRepoFile, though. Add a FIXME to capture that.
We want to support extending the read-only state to cover `/sysroot`
and `/boot`, since conceptually all of the data there should only
be written via libostree. Or at least for `/boot` should *mostly*
just be written by ostree.
This change needs to be opt-in though to avoid breaking anyone.
Add a `sysroot/readonly` key to the repository config which instructs
`ostree-remount.service` to ensure `/sysroot` is read-only. This
requires a bit of a dance because `/sysroot` is actually the same
filesystem as `/`; so we make `/etc` a writable bind mount in this case.
We also need to handle `/var` in the "OSTree default" case of a bind
mount; the systemd generator now looks at the writability state of
`/sysroot` and uses that to determine whether it should have the
`var.mount` unit happen before or after `ostree-remount.service.`
Also add an API to instruct the libostree shared library
that the caller has created a new mount namespace. This way
we can freely remount read-write.
This approach extends upon in a much better way previous work
we did to support remounting `/boot` read-write.
Closes: https://github.com/ostreedev/ostree/issues/1265
This allows copying the state from one OstreeAsyncProgress object to
another, atomically, without invoking the callback. This is needed in
libflatpak, in order to chain OstreeAsyncProgress objects so that you
can still receive progress updates when iterating a different
GMainContext than the one that the OstreeAsyncProgress object was
created under.
See https://github.com/flatpak/flatpak/pull/3211 for the application of
this API.
Define an `OstreeKernelArgsEntry` structure, which holds
both the key and the value. The kargs order array stores
entries for each key/value pair, instead of just the keys.
The hash table is used to locate entries, by storing
entries in a pointer array for each key. The same public
interface is preserved, while maintaining ordering
information of each key/value pair when
appending/replacing/deleting kargs.
Fixes: #1859
To allow for FIPS mode, we need to also install the HMAC file from
`/usr/lib/modules` to `/boot` alongside the kernel image where the
`fips` dracut module will find it. For details, see:
https://github.com/coreos/fedora-coreos-tracker/issues/302
Note I didn't include the file in the boot checksum since it's itself a
checksum of the kernel, so we don't really gain much here other than
potentially causing an unnecessary bootcsum bump.
I was hitting `SIGSEGV` when running `cosa build` and narrowed it down
to #1954. What's happening here is that because we're using the default
context, when we unref it in the out path, it may not actually destroy
the `GSource` if it (the context) is still ref'ed elsewhere. So then,
we'd still get events from it if subsequent operations iterated the
context.
This patch is mostly a revert of #1954, except that we still keep a ref
on the `GSource`. That way it is always safe to destroy it afterwards.
(And I've also added a comment to explain this better.)
We're creating the timer source and then passing ownership to the
context, but because we didn't free the pointer, we would still call
`g_source_destroy` in the exit path. We'd do this right after doing
`unref` on the context too, which would have already destroyed and
unref'ed the source.
Drop that and just restrict the scope of that variable down to make
things more obvious.
Just noticed this after reviewing #1953.
In glib 2.62 this has been changed to emitting a warning. Use G_STRFUNC
instead, which has been available for a long time and is already used in
other places in ostree.
zipl is a bit special in that it parses the BLS config files
directly *but* we need to run the command to update the "boot block".
Hence, we're not generating a separate config file like the other
backends. Instead, extend the bootloader interface with a `post_bls_sync`
method that is run in the same place we swap the `boot/loader` symlink.
We write a "stamp file" in `/boot` that says we need to run this command.
The reason we use stamp file is to prevent the case where the system is
interrupted after BLS file is updated, but before zipl is triggered,
then zipl boot records are not updated.
This opens the door to making things eventually-consistent/reconcilable
by later adding a systemd unit to run `zipl` if we're interrupted via
a systemd unit - I think we should eventually take this approach
everywhere rather than requiring `/boot/loader` to be a symlink.
Author: Colin Walters <walters@verbum.org>
Tested-by: Tuan Hoang <tmhoang@linux.ibm.com>
Co-Authored-By: Tuan Hoang <tmhoang@linux.ibm.com>
More "scan-build doesn't understand GError and our out-param conventions"
AKA "these errors would be impossible with Rust's sum type Result<> approach".
Got this error when trying to rebase libostree in RHEL:
```
Error: CLANG_WARNING: [#def1]
libostree-2019.2/src/libostree/ostree-repo-checkout.c:375:21: warning: Access to field 'disable_xattrs' results in a dereference of a null pointer (loaded from variable 'repo')
```
I think what's happening is it sees us effectively testing
`if (repo == NULL)` via the `while (current_repo)`. Let's
tell it we're sure it's non-null right after the loop.
Tiny release. Just want to get out the important bugfixes instead of
backporting patches (notably the gpg-agent stuff and
`ostree-finalize-staged.service` ordering).
Closes: #1927
Approved by: cgwalters
After the corruption has been fixed with "ostree fsck -a --delete", a
second run of the "ostree fsck" command will print X partial commits
not verified and exit with a zero.
The zero exit code makes it hard to detect if a repair operation needs
to be run. When ever fsck creates a partial commit it should add a
reason for the partial commit to the state file found in
state/<hash>.commitpartial. This will allow a future execution of the
fsck to still return an error indicating that the repository is still
in the damaged state, awaiting repair.
Additional reason codes could be added in the future for why a partial
commit exists.
Text from: https://github.com/ostreedev/ostree/pull/1880
====
cgwalters commented:
To restate, the core issue is that it's valid to have partial commits
for reasons other than fsck pruned bad objects, and libostree doesn't
have a way to distinguish.
Another option perhaps is to write e.g. fsck-partial into the
statefile state/<hash>.commitpartial which would mean "partial, and
expected to exist but was pruned by fsck" and fsck would continue to
error out until the commit was re-pulled. Right now the partial stamp
file is empty, so it'd be fully compatible to write a rationale into
it.
====
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
Closes: #1910
Approved by: cgwalters
If there are different deployments for the same commit version, the BLS
snippets will have the same title fields (but different version fields):
$ grep title *
ostree-1-testos.conf:title TestOS 42 20190902.0 (ostree)
ostree-2-testos.conf:title TestOS 42 20190902.0 (ostree)
ostree-3-testos.conf:title TestOS 42 20190902.0 (ostree)
But bootloaders could expect the title field to be unique for BLS files.
For example, the zipl bootloader used in the s390x architecture uses the
field to name the boot sections that are created from the BLS snippets.
So two BLS snippets having the same title would lead to zipl failing to
create the IPL boot sections because they would have duplicated names:
$ zipl
Using config file '/etc/zipl.conf'
Using BLS config file '/boot/loader/entries/ostree-3-testos.conf'
Using BLS config file '/boot/loader/entries/ostree-2-testos.conf'
Using BLS config file '/boot/loader/entries/ostree-1-testos.conf'
Error: Config file '/etc/zipl.conf': Line 0: section name 'TestOS 42 20190902.0 (ostree)' already specified
Avoid this by always including the deployment index along with the commit
version in the title field, so this will be unique even if there are BLS
files for deployments that use the same commit version:
$ grep title *
ostree-1-testos.conf:title TestOS 42 20190902.0 (ostree:2)
ostree-2-testos.conf:title TestOS 42 20190902.0 (ostree:1)
ostree-3-testos.conf:title TestOS 42 20190902.0 (ostree:0)
$ zipl
Using config file '/etc/zipl.conf'
Using BLS config file '/boot/loader/entries/ostree-3-testos.conf'
Using BLS config file '/boot/loader/entries/ostree-2-testos.conf'
Using BLS config file '/boot/loader/entries/ostree-1-testos.conf'
Building bootmap in '/boot'
Building menu 'zipl-automatic-menu'
Adding #1: IPL section 'TestOS 42 20190902.0 (ostree:0)' (default)
Adding #2: IPL section 'TestOS 42 20190902.0 (ostree:1)'
Adding #3: IPL section 'TestOS 42 20190902.0 (ostree:2)'
Preparing boot device: dasda (0120).
Done.
Closes: #1911
Approved by: cgwalters
Currently the BLS fragments fields write is non-determinisitc. The order
of the fields will depend on how the iterator of the options GHashTable
iterates over the key/value pairs.
But some bootloaders expect the fields to be written in a certain order.
For example the zipl bootloader (used in the s390x architecture) fails to
parse BLS files if the first field is not the 'title' field, since that's
used to name the zipl boot sections that are created from the BLS files.
Write the fields in a deterministic order, following what is used in the
example file of the BootLoaderspec document:
https://systemd.io/BOOT_LOADER_SPECIFICATION
Related: https://github.com/ostreedev/ostree/issues/1888Closes: #1904
Approved by: cgwalters
OSTree has some logic to preserve comment lines in the BLS fragments, but
the BLS fragments are always created on new deployments so the comments
are never carried.
Also, OSTree never writes BLS fragments with comments so these will only
be present in BLS files that were modified outside of OSTree. Something
that should be avoided in general.
Finally, there is a bug in the logic that causes BLS files to have lines
with only a newline character.
The ostree_bootconfig_parser_parse_at() function reads the bootconfig file
using glnx_fd_readall_utf8() but this function NUL terminates the returned
string with the file contents.
So when the string is later split using '\n' as delimiter, the last token
is set to '\0' and a wrong GVariant will be added to the lines GPtrArray
in the OstreeBootconfigParser struct.
This will lead to bootconfig files that contains lines with only a newline
character, since the key in the GVariant would be set to NUL and won't be
present in the options GHashTable of the OstreeBootconfigParser struct.
So let's just remove that logic since is never used and makes BLS files to
have wrong empty lines.
Before this patch:
$ tail -n 4 /boot/loader/entries/ostree-1-testos.conf | hexdump -C
00000000 74 69 74 6c 65 20 54 65 73 74 4f 53 20 34 32 20 |title TestOS 42 |
00000010 32 30 31 39 30 38 32 34 2e 30 20 28 6f 73 74 72 |20190824.0 (ostr|
00000020 65 65 29 0a 0a 0a 0a |ee)....|
00000027
After this patch:
$ tail -n 4 /boot/loader/entries/ostree-1-testos.conf | hexdump -C
00000000 76 65 72 73 69 6f 6e 20 31 0a 6f 70 74 69 6f 6e |version 1.option|
00000010 73 20 72 6f 6f 74 3d 4c 41 42 45 4c 3d 4d 4f 4f |s root=LABEL=MOO|
00000020 20 71 75 69 65 74 20 6f 73 74 72 65 65 3d 2f 6f | quiet ostree=/o|
00000030 73 74 72 65 65 2f 62 6f 6f 74 2e 31 2f 74 65 73 |stree/boot.1/tes|
00000040 74 6f 73 2f 61 65 34 36 34 39 36 38 30 64 33 65 |tos/ae4649680d3e|
00000050 38 33 62 32 34 65 34 37 66 38 64 66 31 30 38 31 |83b24e47f8df1081|
00000060 38 62 66 36 39 38 39 64 36 34 37 61 62 32 38 38 |8bf6989d647ab288|
00000070 64 31 63 30 39 38 30 36 65 34 61 33 36 61 34 65 |d1c09806e4a36a4e|
00000080 62 62 66 36 2f 30 0a 6c 69 6e 75 78 20 2f 6f 73 |bbf6/0.linux /os|
00000090 74 72 65 65 2f 74 65 73 74 6f 73 2d 61 65 34 36 |tree/testos-ae46|
000000a0 34 39 36 38 30 64 33 65 38 33 62 32 34 65 34 37 |49680d3e83b24e47|
000000b0 66 38 64 66 31 30 38 31 38 62 66 36 39 38 39 64 |f8df10818bf6989d|
000000c0 36 34 37 61 62 32 38 38 64 31 63 30 39 38 30 36 |647ab288d1c09806|
000000d0 65 34 61 33 36 61 34 65 62 62 66 36 2f 76 6d 6c |e4a36a4ebbf6/vml|
000000e0 69 6e 75 7a 2d 33 2e 36 2e 30 0a 74 69 74 6c 65 |inuz-3.6.0.title|
000000f0 20 54 65 73 74 4f 53 20 34 32 20 32 30 31 39 30 | TestOS 42 20190|
00000100 38 32 34 2e 30 20 28 6f 73 74 72 65 65 29 0a |824.0 (ostree).|
0000010f
Closes: #1904
Approved by: cgwalters
I've seen people confused by this error in the case where
`/boot` isn't mounted or the BLS fragments were deleted, etc.
If you understand ostree deeply it's clear but, let's do
better here and a direct error message for the case where
we can't find `/boot/loader` which is the majority of these.
The other case could happen if e.g. just the BLS fragment
for the booted deployment was deleted; let's reword that
one a bit too.
Closes: #1905
Approved by: rfairley
When running under qemu, unimplemented ioctls such as FIFREEZE
return ENOSYS, and this causes the deployment to fail.
Catch this and handle it like EOPNOTSUPP.
I'm not sure if qemu's behaviour is fully correct here (or if it should
return EOPNOTSUPP) but it's trivial to handle regardless.
Closes: #1901
Approved by: cgwalters
Add dummy stubs for GPG public functions to be compiled instead of
original code in case if support of GPG is disabled.
Need that to keep API backward compatibility.
Based on original code from file `ostree-gpg-verify-result.c`.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
Closes: #1889
Approved by: cgwalters
Colin Walters
Dan Nicholson
John Hiesey
Philip Chimento
Robert Fairley
Jonathan Lebon
OpenShift Merge Robot
Umang Jain
Philip Withnall
Jason Wessel
Javier Martinez Canillas
Daniel Drake
Denis Pynkin