[WIP] Static-delta's superblock signature support #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fdanis-oss
force-pushed
the
wip/fda/sign_delta_metadata
branch
from
843d9e9
to
21402ce
Compare
fdanis-oss
force-pushed
the
wip/d4s/no_gpg
branch
from
9dc9169
to
ceda082
Compare
fdanis-oss
force-pushed
the
wip/fda/sign_delta_metadata
branch
3 times, most recently
from
fb087b5
to
03e141e
Compare
d4s
suggested changes
Nov 28, 2019
fdanis-oss
force-pushed
the
wip/fda/sign_delta_metadata
branch
from
03e141e
to
cb98a74
Compare
d4s
suggested changes
Nov 29, 2019
| ostree_checksum_to_bytes_v (to); | ||
|
|
||
| /* NOTE: Build GVariant used to sign the superblock, i.e. all the superblock | ||
| * data except inline commits. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, | ||
| "no signature for '%s' in static-delta superblock", | ||
| signature_key); | ||
| return FALSE; |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
This tests if there is a signature for a specific 'sign' engine, passing another engine should fail
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| { | ||
| g_autoptr(GVariant) key_v = g_variant_get_child_value (item, 0); | ||
| const char *str = g_variant_get_string (key_v, NULL); | ||
| if (g_str_has_prefix (str, "deltas/") && !g_str_has_suffix (str, "/commitmeta")) |
There was a problem hiding this comment.
hm.... so why deltas and commitmeta are not a part of protected data?
There was a problem hiding this comment.
deltas are already signed, not really useful to signed them again, and commitmeta is part of protected data
There was a problem hiding this comment.
Is there any chance what these parts are not signed but included?
There was a problem hiding this comment.
yes, it behaves in the same way then if commits are not inlined
and it uses (very) less memory to create the GVariant to sign/check
fdanis-oss
force-pushed
the
wip/fda/sign_delta_metadata
branch
from
cb98a74
to
8ffdae1
Compare
d4s
suggested changes
Nov 30, 2019
| g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, | ||
| "no signature for '%s' in static-delta superblock", | ||
| signature_key); | ||
| return FALSE; |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
fdanis-oss
force-pushed
the
wip/fda/sign_delta_metadata
branch
from
8ffdae1
to
9b3d678
Compare
fdanis-oss
changed the title
fdanis-oss
force-pushed
the
wip/fda/sign_delta_metadata
branch
2 times, most recently
from
a1a91d6
to
4514ad1
Compare
d4s
approved these changes
Dec 3, 2019
fdanis-oss
force-pushed
the
wip/d4s/no_gpg
branch
from
2c57561
to
fe3d401
Compare
fdanis-oss
force-pushed
the
wip/fda/sign_delta_metadata
branch
from
4514ad1
to
2eafb69
Compare
"Brown paper bag" release that actually sets the `is_release_build=yes` flag and also fixes the `Since:` on a few new functions.
Release 2020.2
fdanis-oss
force-pushed
the
wip/d4s/no_gpg
branch
from
fe3d401
to
4b94e73
Compare
fdanis-oss
force-pushed
the
wip/fda/sign_delta_metadata
branch
from
2eafb69
to
d22a84c
Compare
Use the new custom steps. I think we could simplify things further by using `fcosBuild` but let's start with this for now.
ci: migrate to new coreos-ci project
This one is very relevant for ostree.
ci: Test kola --upgrades
We were using `--no-test-exit-error` for upgrade tests but weren't actually checking for test failures after. Instead of running kola directly, just use the `fcosKola` custom step which automatically takes care of e.g. running tests in parallel and archiving results.
ci: use `fcosKola` for running kola tests
See coreos/fedora-coreos-tracker#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.
main: Also automatically remount rw /sysroot for `ostree pull` etc.
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 985a141 ("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.
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, ostreedev#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.
lib: Squash two gtk-doc warnings
commit: Add --base argument
tests/pull-sizes: Disable xattrs everywhere
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>
He did a lot of work on signing and will continue that.
grub2: Don't add menu entries if GRUB supports parsing BLS snippets
…ons-gpg_check lib/repo-pull: fix GPG check while pulling remote
OWNERS: add d4s to reviewers
pull: Update key loading function to match error style
I don't want to even have to think about people using this in production.
Only enable "dummy" signature type with opt-in env variable
This was changed recently and broke us since we do explicitly call `fcosKola` instead of implicitly via `fcosBuild`. Adapt to the new semantics.
ci: Adapt to use new fcosKola semantics
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.
lib/pull: Two cosmetic internal function renames
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.
pull: Cleanup signature verification functions
fdanis-oss
force-pushed
the
wip/fda/sign_delta_metadata
branch
2 times, most recently
from
42fcbfd
to
bb28e28
Compare
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>
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 a signature of all superblock data (serialized),
except inline commits and the signature, then add the signature to the
superblock itself as an additional metadata field.
Signed-off-by: Frédéric Danis <frederic.danis@collabora.com>
Add signing ability to "static-delta generate" builtin. Signed-off-by: Frédéric Danis <frederic.danis@collabora.com>
This build superblock data except inline commits and the static-delta superblock signatures, then verify it. Signed-off-by: Frédéric Danis <frederic.danis@collabora.com>
Signed-off-by: Frédéric Danis <frederic.danis@collabora.com>
Add tests to generate signed deltas and verify them using 'dummy' signature engine. Signed-off-by: Frédéric Danis <frederic.danis@collabora.com>
Add tests to generate signed deltas and verify them using 'ed25519' signature engine. Signed-off-by: Frédéric Danis <frederic.danis@collabora.com>
fdanis-oss
force-pushed
the
wip/fda/sign_delta_metadata
branch
from
bb28e28
to
fcb814a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While the commits contained in the single
static-deltafile are signed so we can check them and operate on trusted data, thesuperblockisn't signed in any way, so it end up operating on untrusted data to:These commits generate a signature of all
superblockdata (serialized), except inline commits and the signature, then add the signature to thesuperblockitself as an additional metadata field.New APIs are added to check if a
superblockis signed, and to verify its signature.This is part of the Add support for alternative signature systems ostreedev#1233 and is based on top of Alternative signing system ostreedev#1878
Fixes: ostreedev#1977