The local pull path was erroring on any missing commit, but that
prevents a depth pull where the source repo has truncated history. As in
the remote case, this also tries to pull in a tombstone commit if the
source repo supports it.
Fixes: #2266
When pulling with depth, missing parent commits are ignored. However,
the check was applying to any commit, which means that it would succeed
even if the requested commit was missing. This might happen on a
corrupted remote repo or when using ref data from a stale summary.
To achieve this, the semantics of the `commit_to_depth` hash table is
changed slightly to only ever includes parent commits. This makes it
easy to detect when a parent commit is being referenced (although there
is a minor bug there when multiple refs are being pulled) while keeping
references to commits that need their `commitpartial` files cleaned up.
It also means that the table is only populated on depth pulls, which
saves some memory and processing in the common depth=0 case.
Fixes: #2265
The recent change in https://github.com/coreos/fedora-coreos-config/pull/659
broke some of our tests that do `mount -o remount,rw /sysroot` but
leave `/boot` read-only.
We had code for having `/boot` read-only before `/sysroot` but
in practice we had a file descriptor for `/sysroot` that we opened
before the remount that would happen later on.
Clean things up here so that in the library, we also remount
`/boot` at the same time we remount `/sysroot` if either are readonly.
Delete the legacy code for remounting `/boot` rw if we're not in
a mount namespace. I am fairly confident most users are either
using the `ostree` CLI, or they're using the mount namespace.
Just like we hold a fd for `/sysroot`, also do so for `/boot`
instead of opening and closing it in a few places.
This is a preparatory cleanup for further work.
In some cases such as backups or mirroring you may want to pull commits
from one repo to another even if there commits that have incorrect
bindings. Fixing the commits in the source repository to have correct
bindings may not be feasible, so provide a pull option to disable
verification.
For Endless we have several repositories that predate collection IDs and
ref bindings. Later these repositories gained collection IDs to support
the features they provide and ref bindings as the ostree tooling was
upgraded. These repositories contain released commits that were valid to
the clients they were targeting at the time. Correcting the bindings is
not really an option as it would mean invalidating the repository
history.
The cache shouldn't be affected by the user passing in some other
summary as it may not be the "official one".
I ran into this in flatpak where the passed summary was correct, but
the re-saving of the cache updated the mtime of the cached file which
led to later http If-Modified-Since calls failing to update.
As detailed in
gitlab.gnome.org/GNOME/glib/-/issues/600#note_877282, volatile
isn't actually needed in these contexts because the atomic operations
already give us strong enough guarantees. In GCC 11, this triggers a
diagnostic due to the volatile qualifier getting dropped anyway.
There is a WIP to do the same in glib:
https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1719
This obsoletes this downstream patch:
https://src.fedoraproject.org/rpms/ostree/c/b8c5a6fb
I got:
src/libostree/ostree-repo.c:5232: Warning: OSTree: ostree_repo_gpg_sign_data: unknown parameter 'out_signature' in documentation comment, should be 'out_signatures'
The timeout timer should always be one-shot, so let's just always
destroy it in the callback. The main context has its own ref on it, so
it won't be freed behind its back.
This *should* fix a leak that was brought up in
https://bugzilla.redhat.com/show_bug.cgi?id=1891761.
Reported-by: Milan Crha <mcrha@redhat.com>
...with the `sysroot.bootloader` configuration option. This can be useful
when converting a system to use `ostree` which doesn't currently have a
bootloader configuration that `ostree` can automatically detect, and is
also useful in combination with the `--sysroot` option when provisioning a
rootfs for systems other than the one you're running `ostree admin deploy`
on.
It's easier to extend and it centralises the config parsing. In other
places we will no longer need to use `g_str_equal` to match these values,
a `switch` statement will be sufficient.
If we have a commit id for all the refs we're pulling, and if we
don't need the summary to list all the refs when mirroring then the
only reason to download the summary is for the list of deltas.
With the new "indexed-deltas" property in the config file (and mirrored
to the summary file) we can detect when we don't need the summary for
deltas and completely avoid downloading it then.
Clients can use these during pull and avoid downloading the summary if
needed, or use the indexed-deltas instead of relying on the ones in
the summary which may be left out.
It is useful to be able to trigger this without having to regenerate
the summary. For example, if you are not using summaries, or ar generating
the summaries yourself.
When we update the summary file (and its list of deltas) we also update
all delta indexes. The index format is a single `a{sv}` variant identical
to the metadata-part of the summary with (currently) only the
`ostree.static-deltas` key.
Since we expect most delta indexes to change rarely, we avoid
unnecessary writes when reindexing. New indexes are compared to
existing ones and only the changed ones are written to disk. This
avoids unnecessary write load and mtime changes on the repo server.
This gets the subpath for a delta index file, which is of the form
"delta-indexes/$commit.index", that contains all the deltas going
to the particular commit.
This makes it testable, and increases its test coverage too 100% of
lines, as measured by `make coverage`.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
As `summary` and `summary.sig` aren’t immutable, HTTP requests to
download them can be optimised by sending the `If-None-Match` and
`If-Modified-Since` headers to avoid unnecessarily re-downloading them
if they haven’t changed since last being checked.
Hook them up to the new support for that in the fetcher.
The `ETag` and `Last-Modified` for each file in the cache are stored as
the `user.etag` xattr and the mtime, respectively. For flatpak, for
example, this affects the cached files in
`~/.local/share/flatpak/repo/tmp/cache/summaries`.
If xattrs aren’t supported, or if the server doesn’t support the caching
headers, the pull behaviour is unchanged from before.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Add support in the soup and curl fetchers to send the `If-None-Match`
and `If-Modified-Since` request headers, and pass on the `ETag` and
`Last-Modified` response headers.
This currently introduces no functional changes, but once call sites
provide the appropriate integration, this will allow HTTP caching to
happen with requests (typically with metadata requests, where the data
is not immutable due to being content-addressed). That should reduce
bandwidth requirements.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Alternative to https://github.com/ostreedev/ostree/pull/2197
Python's (usually) zero-sized `__init__.py` files can provoke
us hitting the hardlink limits on some filesystems (`EMLINK`).
At least one Fedora rpm-ostree user hit this.
The benefits of hardlinking here are quite marginal; lots
of hardlinks can behave suboptimally in particular filesystems
like BTRFS too.
This builds on prior code which made this an option, introduced
in 673cacd633
Now we just do it uncondtionally.
Also this provoked a different bug in a very obscure user mode checkout
case; when the "real" permissions were different from the "physical"
permissions, we would still hardlink. Fix the test case for this.
Otherwise, fall back to downloading and reading them from the `config`
file. See the previous commit for details.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #2165
Currently, they are set in the `config` file and cause that to be
downloaded on every pull. Given that the client is already pulling the
`summary` file, it makes sense to avoid an additional network round trip
and cache those options in the `summary` file.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Helps: #2165
In FCOS and RHCOS, the need to configure software in the initramfs has
come up multiple times. Sometimes, using kernel arguments suffices.
Other times, it really must be a configuration file. Rebuilding the
initramfs on the client-side however is a costly operation. Not only
does it add complexity to the update workflow, it also erodes a lot of
the value obtained from using the baked "blessed" initramfs from the
tree itself.
One elegant way to address this is to allow specifying multiple
initramfses. This is supported by most bootloaders (notably GRUB) and
results in each initrd being overlayed on top of each other.
This patch allows libostree clients to leverage this so that they can
avoid regenerating the initramfs entirely. libostree itself is agnostic
as to what kind and how much data overlay initrds contain. It's up to
the clients to enforce such boundaries.
To implement this, we add a new ostree_sysroot_stage_overlay_initrd
which takes a file descriptor and returns a checksum. Then users can
pass these checksums when calling the deploy APIs via the new array
option `overlay_initrds`. We copy these files into `/boot` and add them
to the BLS as another `initrd` entry.
And make the `override_kernel_argv` one of those options. This is mostly
a mechanical move here, no functional change otherwise.
Prep for adding a new option.
When support for devicetree was added, it created a problem
because old and new ostree versions would compute different
checksums for the "boot data". The scenario here is:
- Have system with ostree < 2020.4
- Reboot into system with ostree 2020.5
- Try to perform an operation that would retain
that previous booted deployment (common)
Currently ostree iterates over all the deployments
that will be retained and calls `install_deployment_kernel()`,
even for the booted one (which is a bit silly), but
just to verify that all boot data for the targeted
deployments are installed.
This then re-computes the checksum and we'd trip this
assertion.
In practice though, we don't strictly require them to match;
the only thing that will happen if they don't is that we'll
end up with another copy of the kernel/initramfs - and
that only temporarily until the previous deployment
gets GC'd.
Longer term, I think what we really want to do anyways
is probably closer to like a little ostree repo for `/boot`
so that we can e.g. still hardlink kernels there even if
the initramfs changes, or hardlink both kernel/initramfs
if just the devicetree changes, etc.
Closes: https://github.com/ostreedev/ostree/issues/2154
The extreme special case of "zero mode" files like `/etc/shadow`
comes up again. What we want is for "user mode" checkouts to
override it to make the file readable; otherwise when operating
as non-root without `CAP_DAC_OVERRIDE` it becomes very difficult
to work with.
Previously, we were hardlinking these files, but then it intersects
with *another* special case around zero sized files, which is
*also* true for `/etc/shadow`.
Trying to avoid hardlinking there unveiled this bug - when
we go to do a copy checkout, we need to override the mode.
Add a new function `ostree_repo_static_delta_execute_offline_with_signature`
which takes a signature engine to verify the delta before applying it.
The `ostree_repo_static_delta_execute_offline` is just a wrapper to this
new function, passing a NULL signature engine.
When this function is called without signature engine, but with a sign
delta, it will only fails if `sign-verify-deltas` is set to true in repo
core options.
This commits move signature existence check and delta signature
verification to share common parts between existing APIs and the new
function.
Signed-off-by: Frédéric Danis <frederic.danis@collabora.com>
This checks if the static delta file is signed or not to be able to
correctly get the superblock to dump.
Signed-off-by: Frédéric Danis <frederic.danis@collabora.com>
This checks if the static delta file is signed or not to be able to
correctly get the superblock to apply.
Signed-off-by: Frédéric Danis <frederic.danis@collabora.com>
This retrieves the signatures and pass the static delta block as an array
of bytes to ostree_sign_data_verify().
Signed-off-by: Frédéric Danis <frederic.danis@collabora.com>
While the commits contained in the single static-delta file are signed so
we can check them and operate on trusted data, the superblock isn't signed
in any way, so it end up operating on untrusted data to:
1. actually find where the trusted data is, and
2. check whether the update is fit for the current device by looking at
the collection id stored in the metadata
This commit generates signatures of all static data, and concatenate them
to the existing static delta format, i.e. as a GVariant layout `a{sv}ay`
where
- a{sv}: signatures
- ay: existing delta variant
Signed-off-by: Frédéric Danis <frederic.danis@collabora.com>
This was only used in one place, and (especially with the simplification
with GMainContextPopDefault) and the one caller doesn't really do
much more than call the helper. Additionally, what little it does (saving
the result in the cache) is inherently tied to how the helper work,
and will become even more so when we support summary indexes.
This is a preparatory cleanup for supporting summary indexes. It
doesn't change any behaviour and passes make check on its own.
This loads and makes a digest for a delta superblock. The previous
code was used when generating the deltas section in the summary
file. This changes nothing, but is in preparation for using similar
formats in a separate delta index file.
The change in cbf1aca1d5c08d2f40832d16670484ba878d95fb actually
only mmaps the signature file, not the summary. This change makes
use mmap both, as well as extract the cache loading into a helper
function that we will later use in more places.
ostree_repo_list_static_delta_names() tried to validate that
any second-level directory element was a directory, but there was
a cut-and-paste issue, and it used `dent->d_type` instead
of `sub_dent->d_type`.
This fixes the code, but all old ostree versions will break if
there are non-directories in a subdirectory of the deltas directory
in the repo, so be wary.
Looking at
https://github.com/coreos/coreos-assembler/issues/1703
a user is getting a bare:
`error: fsetxattr: Permission denied`
I don't think it's these code paths since a deploy
isn't happening but on inspection I noticed we didn't
have error prefixing here.
Don't mention deprecation in the description for
`ostree_sysroot_deploy_tree` since there are legitimate use cases for it
(e.g. to create the first deployment via `ostree admin deploy`).
Instead, make the comment clearly redirect to the staging API when
booted into the sysroot.
Tighten up how we handle kargs here so it's more clear. When we call
`sysroot_finalize_deployment`, any karg overrides have already been set
on the bootconfig object of the deployment. So re-setting it here is
redundant and confusing.
I was thinking a bit more recently about the "live" changes
stuff https://github.com/coreos/rpm-ostree/issues/639
(particularly since https://github.com/coreos/rpm-ostree/pull/2060 )
and I realized reading the last debates in that issue that
there's really a much simpler solution; do exactly the same
thing we do for `ostree admin unlock`, except mount it read-only
by default.
Then, anything that wants to modify it does the same thing
libostree does for `/sysroot` and `/boot` as of recently; create
a new mount namespace and do the modifications there.
The advantages of this are numerous. First, we already have
all of the code, it's basically just plumbing through a new
entry in the state enumeration and passing `MS_RDONLY` into
the `mount()` system call.
"live" changes here also naturally don't persist, unlike what
we are currently doing in rpm-ostree.
These allow the `summary` and `summary.sig` files to be cached at a
higher layer (for example, flatpak) between related pull operations (for
example, within a single flatpak transaction). This avoids
re-downloading `summary.sig` multiple times throughout a transaction,
which increases the transaction’s latency and introduces the possibility
for inconsistency between parts of the transaction if the server changes
its `summary` file part-way through.
In particular, this should speed up flatpak transactions on machines
with high latency network connections, where network round trips have a
high impact on the latency of an overall operation.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
We use a similar trick to having a `sysroot -> .` symlink on the real root
here to support both /boot on root as well as on a separate filesystem. No
matter how it's mounted `/boot/xyz` will always refer to the file you'd
expect.
This is nicer than my previous attempts at this because there's no
configuration nor auto-detection required.
The "new style" code generally avoids `goto err` because it conflicts
with `__attribute__((cleanup))`. This fixes a compiler warning.
Signed-off-by: Frédéric Danis <frederic.danis@collabora.com>
If `glnx_make_lock_file` falls back to `flock`, on NFS this uses POSIX
locks (`F_SETLK`). As such, we need to be able to handle `EACCES` as
well as `EAGAIN` (see `fnctl(2)`).
I think this is what coreos-ostree-importer has been hitting, which runs
on RHEL7 in the Fedora infra and does locking over an NFS share where
multiple apps could concurrently pull things into the repo.
This came in with 5af403be0c but
was never implemented.
I noticed this now because the Rust ostree bindings generate a
wrapper for it which the linker tries to use.
This is the dual of 1f3c8c5b3d
where we output more detail when signapi fails to validate.
Extend the API to return a string for success, which we output
to stdout.
This will help the test suite *and* end users validate that the expected
thing is happening.
In order to make this cleaner, split the "verified commit" set
in the pull code into GPG and signapi verified sets, and have
the signapi verified set contain the verification string.
We're not doing anything with the verification string in the
pull code *yet* but I plan to add something like
`ostree pull --verbose` which would finally print this.
To aid debuggability, when we find a commit that isn't signed
by our expected key, output a specific error message with the
key.
(And then add code to switch to just printing the count beyond 3
because the test suite injects 100 keys and hopefully no one
ever actually does that)
Add a standard key for this. We actually had a case in OpenShift
builds recently where a `ppc64le` image was pushed over an `x86_64`
one and this started failing at runtime with a not immediately
obvious error.
I'll probably end up changing rpm-ostree at least to use
the RPM architecture for this key and fail if it doesn't match
the booted value.
Possibly that should live in ostree but it would involve adding
architecture schema here, which gets into a big mess. Let's
just standardize the key.
xref e02ef2683d
Add support for a devicetree directory at /usr/lib/modules/$kver/dtb/.
In ARM world a general purpose distribution often suppports multiple
boards with a single operating system. However, OSTree currently only
supports a single device tree, which does not allow to use the same
OSTree on different ARM machines. In this scenario typically the boot
loader selects the effective device tree.
This adds device tree directory support for the new boot artefact
location under /usr/lib/modules. If the file `devicetree` does not
exist, then the folder dtb will be checked. All devicetrees are hashed
into the deployment hash. This makes sure that even a single devicetree
change leads to a new deployment and hence can be rolled back.
The loader configuration has a new key "devicetreepath" which contains
the path where devicetrees are stored. This is also written to the
U-Boot variable "fdtdir". The boot loader is expected to use this path
to load a particular machines device tree from.
Closes: #1900
Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
One OpenShift user saw this from rpm-ostree:
```
client(id:cli dbus:1.583 unit:machine-config-daemon-host.service uid:0) added; new total=1
Initiated txn UpdateDeployment for client(id:cli dbus:1.583 unit:machine-config-daemon-host.service uid:0): /org/projectatomic/rpmostree1/rhcos
Txn UpdateDeployment on /org/projectatomic/rpmostree1/rhcos failed: File header size 4294967295 exceeds size 0
```
which isn't very helpful. Let's add some error
prefixing here which would at least tell us which
object was corrupted.
The goal here is to move the code towards a model
where the *client* can explicitly specify which signature types
are acceptable.
We retain support for `sign-verify=true` for backwards compatibility.
But in that configuration, a missing public key is just "no signatures found".
With `sign-verify=ed25519` and no key configured, we can
explicitly say `No keys found for required signapi type ed25519`
which is much, much clearer.
Implementation side, rather than maintaining `gboolean sign_verify` *and*
`GPtrArray sign_verifiers`, just have the array. If it's `NULL` that means
not to verify.
Note that currently, an explicit list is an OR of signatures, not AND.
In practice...I think most people are going to be using a single entry
anyways.
There's a lot of historical baggage associated with GPG verification
and `ostree pull` versus `ostree pull-local`. In particular nowadays,
if you use a `file://` remote things are transparently optimized
to e.g. use reflinks if available.
So for anyone who doesn't trust the "remote" repository, you should
really go through through the regular
`ostree remote add --sign-verify=X file://`
path for example.
Having a mechanism to say "turn on signapi verification" *without*
providing keys goes back into the "global state" debate I brought
up in https://github.com/ostreedev/ostree/issues/2080
It's just much cleaner architecturally if there is exactly one
path to find keys: from a remote config.
So here in contrast to the GPG code, for `pull-local` we explictily
disable signapi validation, and the `ostree_repo_pull()` API just
surfaces flags to disable it, not enable it.
For the same reason as https://github.com/coreos/rpm-ostree/pull/2094.
What we care most about is that the new commit we pull is newer than the
one we're currently sitting on, not necessarily that it's newer than the
branch itself, which it might not be if e.g. we're trying to deploy a
commit older than the tip but still newer than the deployment (via
`--override-commit`).
The way `timestamp-check` works might be too restrictive in some
situations. Essentially, we need to support the case where users want to
pull an older commit than the current tip, but while still guaranteeing
that it is newer than some even older commit.
This will be used in Fedora CoreOS. For more information see:
https://github.com/coreos/rpm-ostree/pull/2094https://github.com/coreos/fedora-coreos-tracker/issues/481
Previously in the pull code, every time we went to verify
a commit we would re-initialize an `OstreeSign` instance
of each time, re-parse the remote configuration
and re-load its public keys etc.
In most cases this doesn't matter really because we're
pulling one commit, but if e.g. pulling a commit with
history would get a bit silly.
This changes things so that the pull code initializes the
verifiers once, and reuses them thereafter.
This is continuing towards changing the code to support
explicitly configured verifiers, xref
https://github.com/ostreedev/ostree/issues/2080
This cleans up the verification code; it was weird how
we'd get the list of known names and then try to create
an instance from it (and throw an error if that failed, which
couldn't happen).
The GI scanner decides if an `enum` is really a `bitfield` if it finds
any values that have left shifts. With an `enumeration`, the
introspecting language may error or convert to a different type if the
user tries to combine values. Change all Flags `enum`s to use
left-shifted values so that they're represented as `bitfield`s in the
GIR.
The primary bug here is that you can't combine `REFS_ONLY` and
`NO_PRUNE` when calling `OSTree.Repo.prune()` from an introspected
language.
This is an IABI break since the typelib will change from `enumeration`
to `bitfield`. `OstreeRepoImportFlags` is internal but the change is
included here to prepare for a subsequent name that would require bit
shifting to operate correctly as a flag.
Explicitly expose functions for querying the metadata format
and key name used by OstreeSign object:
- ostree_sign_metadata_format
- ostree_sign_metadata_key
This allows to use the same metadata format and key name
by 3-rd party applications using signapi.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
I've only noticed this by inspection. But I think it's possible for
`cleanup_txn_dir` to get called with the `staging-...-lock` file since
it matches the prefix.
Make the checking here stronger by verifying that it's a directory. If
it's not a directory (lockfile), then follow the default pruning expiry
logic so that we still cleanup stray lockfiles eventually.
`ostree-repo-pull.c` is rather monstrous; I plan to split it
up a bit. There's actually already a `pull-private.h` but
that's just for the binding verification API. I think that one
isn't really pull specific. Let's move it into the "catchall"
`repo.c`.
Previously we would pass the `verification-key` and `verification-file`
to all backends, ignoring errors from loading keys until we
found one that worked.
Instead, change the options to be `verification-<engine>-key`
and `verification-<engine>-file`, and then
rework this to use standard error handling; barf explicitly if
we can't load the public keys for example. Preserve
the semantics of accepting the first valid signature. The
first signature error is captured, the others are currently
compressed into a `(and %d more)` prefix.
And now that I look at this more closely there's a lot of
duplication between the two code paths in pull.c for verifying;
will dedup this next.
I'm mainly doing this to sanity check the CI state right now.
However, I also want to more cleanly/clearly distinguish
the "sign" code from the "gpg" code.
Rename one function to include `gpg`.
For the other...I think what it's really doing is using the remote
config, so change it to include `remote` in its name.
If GPG support is disabled in a build time we should to check if any of
options "gpg_verify" or "gpg_verify_summary" is set to TRUE instead
of checking if they are passed via options while pulling from remote.
Fixed the failure with assertion of `ostree find-remotes --pull --mirror`
calling (`tests/test-pull-collections.sh`) if libostree has been compiled
without GPG support.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
This code wasn't written with idiomatic GError usage; it's not standard
to construct an error up front and continually append to its
message. The exit from a function is usually `return TRUE`,
with error conditions before that.
Updating it to match style reveals what I think is a bug;
we were silently ignoring failure to parse key files.
When we're only pulling a subset of the refs available in the remote, it
doesn't make sense to copy the remote's summary (which may not be valid
for the local repo). This makes the check here match the one done
several lines above when we decide whether to error out if there's no
remote summary available.
This extends the fix in https://github.com/ostreedev/ostree/pull/935 for
the case of collection-refs.
Also, add a unit test for this issue, based on the existing one in
pull-test.sh.
Noticed this while writing tests for a core `ostree_sysroot_load()`
entrypoint. And decided to do the same for `ostree_repo_open()`,
and while there also noted we had a duplicate error prefixing
for the open (more recently `glnx_opendirat()` automatically
prefixes with the path).
Correctly return "error" from `ostree_repo_sign_commit()`
in case if GPG is not enabled.
Use glnx_* functions in signature related pull code for clear
error handling if GPG isn't enabled.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
Do not mask implementation anymore since we have a working
engines integrated with pulling mechanism.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
The "new style" code generally avoids `goto err` because it conflicts
with `__attribute__((cleanup))`. This fixes a compiler warning.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
The "new style" code generally avoids `goto err` because it conflicts
with `__attribute__((cleanup))`. This fixes a compiler warning.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
Improve error handling for signatures checks -- passthrough real
reasons from signature engines instead of using common messages.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
Return the collected errors from signing engines in case if verification
failed for the commit.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
Add more precise error handling for ed25519 initialization.
Check the initialization status at the beginning of every public
function provided by ed25519 engine.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
Change the API of supporting functions `_load_public_keys()` and
`_ostree_repo_sign_verify()` -- pass repo object and remote name
instead of OtPullData object. This allows to use these functions
not only in pull-related places.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
We don't need anymore stubs for verification options for remotes
in case if ostree built without GPG support.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
Usage of 'g_warning()' inside keys loading funcrion lead to false
failure: the key loading attempt for the wrong engine breaks the
pulling process instead of trying to use this key with correct engine.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
The initial implementation with single key for secret and public parts
doesn't allow to test pulling with several signing engines used.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
Skip public keys verification if key is marked as invalid key.
Allow to redefine system-wide directories for ed25519 verification.
Minor bugfixes.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
Add function `_load_public_keys()` to pre-load public keys according
remote's configuration. If no keys configured for remote, then use
system-wide configuration.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
Allow to add public and secret key for ed25519 module as based64 string.
This allows to use common API for pulling and builtins without knowledge
of used signature algorithm.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
Removed from public `ostree_sign_detached_metadata_append` function.
Renamed `metadata_verify` into `data_verify` to fit to real
functionality.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
Return `const char *` instead of copy of the string -- this allow to
avoid unneeded copying and memory leaks in some constructions.
Minor code cleanup and optimisations.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
If not provided key of file name with keys for remote, then try to use
system defaults:
- /etc/ostree/trusted.ed25519
- /etc/ostree/trusted.ed25519.d/*
- /usr/share/ostree/trusted.ed25519
- /usr/share/ostree/trusted.ed25519.d/*
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
If `verification-key` is set for remote it is used as a public key for
checking the commit pulled from that remote.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
API changes:
- added function `ostree_sign_add_pk()` for multiple public keys using.
- `ostree_sign_set_pk()` now substitutes all previously added keys.
- added function `ostree_sign_load_pk()` allowed to load keys from file.
- `ostree_sign_ed25519_load_pk()` able to load the raw keys list from file.
- use base64 encoded public and private ed25519 keys for CLI and keys file.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
Added the initial version of signing interface allowing to allowing to
sign and verify commits.
Implemented initial signing modules:
- dummy -- simple module allowing to sign/verify with ASCII string
- ed25519 -- module allowing to sign/verify commit with ed25519
(EdDSA) signature scheme provided by libsodium library.
Signed-off-by: Denis Pynkin <denis.pynkin@collabora.com>
The [dev-overlay](332c6ab3b9/src/cmd-dev-overlay)
script shipped in coreos-assembler mostly exists to deal
with the nontrivial logic around SELinux policy. Let's make
the use case of "commit some binaries overlaying a base tree, using
the base's selinux policy" just require a magical
`--selinux-policy-from-base` argument to `ostree commit`.
A new C API was added to implement this in the case of `--tree=ref`;
when the base directory is already checked out, we can just reuse
the existing logic that `--selinux-policy` was using.
Requires: https://github.com/ostreedev/ostree/pull/2039
Add G_IO_ERROR_PARTIAL_INPUT to the list of error codes caused by
transient networking errors which lead us to retry the request. When
attempting to install the spotify flatpak you often get the error
message "Connection terminated unexpectedly" and the download of the deb
file fails. In this case, libsoup is setting G_IO_ERROR_PARTIAL_INPUT
and sometimes a subsequent download attempt is successful, so we should
treat it as transient.
Ideally we would behave as wget does in this case and retry the download
picking up where we left off in the file rather than starting over, but
that would require changes to libsoup I think.
Sadly this patch does not fix the flatpak installation of spotify in the
face of such errors, because flatpak doesn't use libostree to download
extra data, but presumably it's possible we could encounter such an
error pulling from an ostree repo, so the patch is still correct.
These had been added assuming 2019.7 would be the next version, but now
it's 2020 and there's been a release. In the case of
`OstreeCommitSizesEntry`, I'd forgotten to move it forward from 2019.5
to 2019.7 in the time between when I started working on the feature and
it landed.
For repo structure directories like `objects`, `refs`, etc... we should
be more permissive and let the system's `umask` narrow down the
permission bits as wanted.
This came up in a context where we want to be able to have read/write
access on an OSTree repo on NFS from two separate OpenShift apps by
using supplemental groups[1] so we don't require SCCs for running as the
same UID (supplemental groups are part of the default restricted SCC).
[1] https://docs.openshift.com/container-platform/3.11/install_config/persistent_storage/persistent_storage_nfs.html#nfs-supplemental-groups
For some reason I haven't fully debugged (probably a recent
kernel change), in the case where the immutable bit isn't set,
trying to call `EXT2_IOC_SETFLAGS` without it set returns `EINVAL`.
Let's avoid calling the `ioctl()` if we don't have anything to do.
This fixes a slew of `make check` failures here in my toolbox
environment.
(kernel is `5.5.0-0.rc6.git0.1.fc32.x86_64` with `xfs`)
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".
OpenShift Merge Robot
Dan Nicholson
Colin Walters
William Manley
Luca BRUNO
Alexander Larsson
Jonathan Lebon
Philip Withnall
Felix Krull
Philip Withnall
Frédéric Danis
Phaedrus Leeds
Denis Pynkin
Stefan Agner
Dan Nicholson
John Hiesey
Philip Chimento
Robert Fairley
Umang Jain