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>
I was trying to followup the `--selinux-policy-from-base` work
to add a `cosa build --fast=overlay` for coreos-assembler,
but hit on the fact that using e.g. `--owner-uid` disables
commit optimizations.
A while ago, https://github.com/ostreedev/ostree/pull/1643 landed
which optimized this for the case where no modifications are provided.
But, we really need the SELinux policy bits, and it's super convenient
to run `ostree commit` as non-root.
It's fairly surprising actually that it's taken us so long to
iterate on a good interface for this "commit changes on top of a base"
model. In practice, many nontrivial cases really end up needing
to do a (hardlink) checkout, and that case is optimized.
But for this coreos-assembler work I want to directly overlay onto
a commit object another commit object.
That previous PR above added exactly the API we need, so let's
expose it in the CLI.
What you can see happening in the test is that we provide
`--owner-uid 42`, but that only applies to directories/files
that were added in the commit.
And now that I look at this, I think what we really want here
is to avoid changing directories that exist in the base, but
eh; in practice the main use here is for `--owner-uid 0` while
committing as non-root; and that works fine with this since
the baseline uid will be zero as well.
This is another attempt to avoid having duplicated menu entries caused by
GRUB having support to parse BLS snippets and the 15_ostree script adding
menu entries as well.
The previous attempt was in commit 985a141002 ("grub2: Exit gracefully if
the configuration has BLS enabled") but that lead to users not having menu
entries at all, due having an old GRUB version that was not able to parse
the BLS snippets.
This happened because the GRUB bootloader is never updated in the ESP as
a part of the OSTree upgrade transaction.
The logic is similar to the previous commit, the 15_ostree script exits if
able to determine that the bootloader can parse the BLS snippets directly.
But this time it will not only check that a BLS configuration was enabled,
but also that a /boot/grub2/.grub2-blscfg-supported file exists. This file
has to be created by a component outside of OSTree that also takes care of
updating GRUB to a version that has proper BLS support.
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.
This will be useful in the unit test added by the next commit. It just
passes OSTREE_REPO_PULL_FLAGS_MIRROR to the call to
ostree_repo_pull_from_remotes_async().
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>
Allow to sign the summary file with alternative signing mechanism.
Added new options:
- --sign-type -- select the engine (defaults to ed25519)
- --sign -- secret key to use for signing
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>
Option '--keys-dir' is used for redefinition of default directories with
public/revoked keys. If keys directory is set then default directories
are ignored and target directory is expected to contain following
structure for ed25519 signature mechanism:
dir/
trusted.ed25519 <- file with trusted keys
revoked.ed25519 <- file with revoked keys
trusted.ed25519.d/ <- directory with files containing trusted keys
revoked.ed25519.d/ <- directory with files containing revoked keys
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>
`ostree sign` is able to use several public keys provided via arguments
and via file with keys.
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>
This builtin allows to sign and verify commit with new signature
mechanism. At the moment it is possible to use 'dummy' and 'ed25519'
signing modules.
'dummy' module use any ASCII string from command line as a key for
commit's signing or verification.
Support of ed25519 signature is implemented with `libsoium` library.
Secret and public key should be provided in hex presentation via
command line.
Based on 'gpg-sign' source.
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
Rework the simple cases of "commit ." and "commit argv[1]" to
generate the more general "--tree=X --tree=Y" path, so that we
only have one primary control flow here.
Prep for a future patch around loading SELinux policy from
the first argument.
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.
This regressed in 2db79fb398
I noticed this while finally getting the installed tests to run
in FCOS via kola and `ostree admin pin 0` is now aborting because
we were returning TRUE, but no error set.
I don't see a reason to try to continue on if we hit an error;
the original reporter was requesting support for multiple arguments,
but not "ignore invalid requests".
See https://github.com/coreos/fedora-coreos-tracker/issues/343
When we added the read-only sysroot support it broke using "raw"
`ostree pull` and `ostree refs --create` and all of the core repo
CLIs that just operate on a repo and not a sysroot.
Fixing this is a bit ugly as it "layer crosses" things even more.
Extract a helper function that works in both cases.
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
When --autoexit is used with --daemonize and --log-file, the program
never exits when the root directory is deleted. I believe what happens
is that g_file_new_for_path triggers the glib worker context to be
started to talk to GVfs. Once the program forks, the parent exits and
the thread iterating the worker context is gone. The file monitor then
never receives any events because the inotify helper also runs from the
worker context.
Move the fork earlier just after parsing and validating the command line
arguments. In order to handle setup errors in the child, a pipe is
opened and the parents waits until the child writes a status byte to it.
If the byte is 0, the parent considers the child setup successful and
exits and the child carries on as a daemon. Notably, the child doesn't
reopen stderr to /dev/null until after this so that it can send error
messages there.
Fixes: #1941
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.
Colin Walters
OpenShift Merge Robot
Denis Pynkin
Javier Martinez Canillas
Felix Krull
Matthew Leeds
Jonathan Lebon
Dan Nicholson