dissect: add support for GPT images with embedded dm-verity signatures #20691

poettering · 2021-09-09T16:20:26Z

bluca · 2021-09-09T16:53:31Z

I'd much rather avoid this - unless the appropriate sysctl is set, there's no harm in letting the kernel validate too, nothing should happen if the key is not available. If someone has the access rights to place images where they are loaded (sysext, etc) then they most likely can also place their own certs, which in some cases is not good.

uh this comment was inline to a specific check, but it's appearing as a generic comment in the PR? weird?

poettering · 2021-09-10T07:34:02Z

/cc @oniko, maybe this is interesting to you?

poettering · 2021-09-10T11:38:31Z

uh this comment was inline to a specific check, but it's appearing as a generic comment in the PR? weird?

I think to comment on individual commits of a PR in PR context you now need to click on the Commits tab above first. If you click on commits in the Conversation tab, then it ends up as comment on the commit object outside of PR context.

Pretty stupid if you ask me.

poettering · 2021-09-10T19:07:39Z

This should be ready for review now. PTAL!

@bluca i changed the logic now to do signature-based verity setup first via kernel and then fallback to userspace if that didn't work.

bluca · 2021-09-10T19:48:38Z

This should be ready for review now. PTAL!

@bluca i changed the logic now to do signature-based verity setup first via kernel and then fallback to userspace if that didn't work.

Nice, thank you!

The commit message needs updating then:

If one of the certificates found that way works then we'll attempt to
attach the DM device without passing the PKCS7 data to the kernel. If
none works then the behaviour as before is attempted: validation data is
passed to the kernel.

src/nspawn/nspawn.c

src/shared/dissect-image.c

bluca

Could you please add some checks in TEST-50-DISSECT? It should be pretty simple to do so, just need to generate the certificates, the rest is there already

This adds support for creating signed GPT disk images. If Verity=signed is set this will not only generate and insert Verity data into the image, but then use the resulting root hash, sign it and include it in an additional partition. It will also write the resulting PKCS7 signature out into a new .roothash.p7s file. This scheme is compatible with kernel 5.4's PKCS7 signature logic for dm-verity: the resulting .p7s file can be passed as-is to the kernel (or systemd's RootHashSignature= setting). The partition this embedds contains a simple JSON object containing three fields. The verity root hash, the PKCS7 data (i.e. the same data as in the .p7s file, but in base64), and SHA256 fingerprint of the signing key. This partition is supposed to be read by the image dissection logic of systemd, to implement signed single-file images. (The corresponding PR for systemd I am still working on). This opens up two avenues for image verification: 1. Everything in one file: the single, "unified" GPT disk image contains three partitions, for payload data, verity data and verity signature. 2. Split out: root hash and its signature are stored in two "sidecar" files. (Of course I'd personally always go the "unified" way, but given the RootHashSignature= logic exists already, and it's easy to support, let's support it.) This uses the key already used for doing secureboot image signing. Conceptually this makes a ton of sense: we sign the same stuff here after all: the contents of the image, supporting two different entrypoints to the image: one via UEFI booting the image, and once for attaching directly to an image from a running system. Admittedly, the "mkosi.secure-boot.key" and "mkosi.secure-boot.certificate" monikers for this key pair might be a bit suprising though. Corresponding PR in systemd is: systemd/systemd#20691

This adds support for actually using embedded signature data from partitions.

…params Just to make debugging easier.

Getting certificates for dm-verity roothash signing into the trusted kernel keychain is a royal PITA (means recompiling or rebooting with shim), hence let's add a minimal userspace PKCS7 validation as well. The mechanism is really simple and compatible with the verification the kernel does. The only difference is that the certificates are searched in /etc/verity.d/*.crt (and similar dirs in /usr/lib/, …). We'll first try validation by passing the PKCS#7 data to the kernel, but if that doesn't work we'll see if one of the certificates found that way works and then attempt to attach the image without passing the PKCS#7 data to the kernel. This makes it very easy to have fully validated GPT disk images. For example, just copy the 'mkosi.secure-boot.crt' file you have in your mkosi build dir to /etc/verity.d/ and things should just work.

…idation

poettering · 2021-09-28T15:06:21Z

Addressed all issues and extended the test case. The only interesting change is probably the latter. ptal!

bluca

LGTM

bluca · 2021-09-28T15:14:12Z

I've added the openssl binary to the list of depndencies of the focal tests just now, if it was too late for the run I'll just kick the jobs again later
@mrc0mmand do you need to do a similar change for the centos ones?

mrc0mmand · 2021-09-28T15:17:39Z

I've added the openssl binary to the list of depndencies of the focal tests just now, if it was too late for the run I'll just kick the jobs again later @mrc0mmand do you need to do a similar change for the centos ones?

iirc the openssl binary should be in CentOS CI everywhere by default, but let's see if the CIs agree with me (thanks for the heads-up).

bluca · 2021-09-30T15:10:51Z

@poettering it seems the test is failing every now and then:

[   69.364854] systemd[458]: run-u6.service: Failed to set up mount namespacing: Required key not available
[   69.373667] systemd[458]: run-u6.service: Failed at step NAMESPACE spawning mount: Required key not available
[   69.376950] systemd[1]: run-u6.service: Failed with result 'exit-code'.

https://autopkgtest.ubuntu.com/results/autopkgtest-focal-upstream-systemd-ci-systemd-ci/focal/amd64/s/systemd-upstream/20210930_141856_7cc4e@/artifacts.tar.gz

The binary is used in a test to generate some test certs, added by: systemd#20691

This adds support for creating signed GPT disk images. If Verity=signed is set this will not only generate and insert Verity data into the image, but then use the resulting root hash, sign it and include it in an additional partition. It will also write the resulting PKCS7 signature out into a new .roothash.p7s file. This scheme is compatible with kernel 5.4's PKCS7 signature logic for dm-verity: the resulting .p7s file can be passed as-is to the kernel (or systemd's RootHashSignature= setting). The partition this embedds contains a simple JSON object containing three fields. The verity root hash, the PKCS7 data (i.e. the same data as in the .p7s file, but in base64), and SHA256 fingerprint of the signing key. This partition is supposed to be read by the image dissection logic of systemd, to implement signed single-file images. (The corresponding PR for systemd I am still working on). This opens up two avenues for image verification: 1. Everything in one file: the single, "unified" GPT disk image contains three partitions, for payload data, verity data and verity signature. 2. Split out: root hash and its signature are stored in two "sidecar" files. (Of course I'd personally always go the "unified" way, but given the RootHashSignature= logic exists already, and it's easy to support, let's support it.) This uses the key already used for doing secureboot image signing. Conceptually this makes a ton of sense: we sign the same stuff here after all: the contents of the image, supporting two different entrypoints to the image: one via UEFI booting the image, and once for attaching directly to an image from a running system. Admittedly, the "mkosi.secure-boot.key" and "mkosi.secure-boot.certificate" monikers for this key pair might be a bit suprising though. Corresponding PR in systemd is: systemd/systemd#20691

If the packages are built without libssl simply skip the signature checks. Oct 06 21:21:32 H systemd[1]: systemd 249.1249.gcc4df1f787.0 running in system mode (+PAM +AUDIT +SELINUX +APPARMOR +IMA +SMACK +SECCOMP +GCRYPT +GNUTLS -OPENSSL ... Oct 06 21:22:21 H systemd[459]: Activation of signed Verity volume worked neither via the kernel nor in userspace, can't activate. Follow-up for systemd#20691

If the packages are built without libssl simply skip the signature checks. Oct 06 21:21:32 H systemd[1]: systemd 249.1249.gcc4df1f787.0 running in system mode (+PAM +AUDIT +SELINUX +APPARMOR +IMA +SMACK +SECCOMP +GCRYPT +GNUTLS -OPENSSL ... Oct 06 21:22:21 H systemd[459]: Activation of signed Verity volume worked neither via the kernel nor in userspace, can't activate. Follow-up for #20691

Enables additional tests and checks, added by: systemd#20691

dissect: add support for GPT images with embedded dm-verity signatures

poettering added dissect dont-merge 💣 new-feature labels Sep 9, 2021

poettering changed the title ~~dissect: add support for GPT images with embedded dm-verity signatures~~ Sep 9, 2021

github-actions bot added the resolve label Sep 9, 2021

poettering mentioned this pull request Sep 9, 2021

mkosi: add support for PKCS7 signed images systemd/mkosi#798

Merged

poettering force-pushed the gpt-sig branch from 4b68cff to d89cfed Compare September 10, 2021 09:05

github-actions bot added the documentation label Sep 10, 2021

poettering mentioned this pull request Sep 10, 2021

dissect: various small tweaks/tougher checks/refactoring of GPT dissection code #20703

Merged

poettering force-pushed the gpt-sig branch from d89cfed to 1cdce73 Compare September 10, 2021 19:03

poettering added needs-review and removed dont-merge 💣 labels Sep 10, 2021

bluca changed the title ~~WIP: dissect: add support for GPT images with embedded dm-verity signatures~~ Sep 10, 2021

bluca reviewed Sep 10, 2021

View reviewed changes

src/nspawn/nspawn.c Outdated Show resolved Hide resolved

src/shared/dissect-image.c Outdated Show resolved Hide resolved

bluca reviewed Sep 10, 2021

View reviewed changes

src/shared/dissect-image.c Outdated Show resolved Hide resolved

bluca reviewed Sep 10, 2021

View reviewed changes

src/shared/dissect-image.c Show resolved Hide resolved

bluca reviewed Sep 10, 2021

View reviewed changes

src/shared/dissect-image.c Show resolved Hide resolved

bluca requested changes Sep 10, 2021

View reviewed changes

bluca added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed please-review labels Sep 13, 2021

bluca added the needs-rebase label Sep 15, 2021

poettering mentioned this pull request Sep 20, 2021

efi-stub: when booting a kernel foo.efi then pack foo.efi.extra.d/*.{cred,raw} as an initrd #20789

Merged

poettering added 5 commits September 28, 2021 17:02

dissect-image: load embedded verity signature info from image

88b3300

This adds support for actually using embedded signature data from partitions.

dissect-image: add env var for disabling "sidecar" loading of verity …

d5fcc5b

…params Just to make debugging easier.

docs: document the new Verity signature partition type, and its UUIDs

7761799

docs: document the three new env vars for tweaking GPT dissection/val…

54dcf84

…idation

poettering force-pushed the gpt-sig branch from 987e21e to bc0d17e Compare September 28, 2021 15:06

poettering added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed good-to-merge/with-minor-suggestions labels Sep 28, 2021

poettering added 2 commits September 28, 2021 17:06

tests: extend TEST-50-DISSECT to look for verity signatures

2c424ee

update TODO

636c8a1

poettering force-pushed the gpt-sig branch from bc0d17e to 636c8a1 Compare September 28, 2021 15:07

poettering added please-review and removed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed labels Sep 28, 2021

bluca approved these changes Sep 28, 2021

View reviewed changes

bluca added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed please-review labels Sep 28, 2021

poettering merged commit ca3f47e into systemd:main Sep 28, 2021

ddstreet pushed a commit to ddstreet/systemd that referenced this pull request Oct 5, 2021

upstream-ci: add openssl to the dependencies

8b4d902

The binary is used in a test to generate some test certs, added by: systemd#20691

bluca mentioned this pull request Oct 6, 2021

test: make OpenSSL checks optional in TEST-50-DISSECT #20951

Merged

ddstreet pushed a commit to ddstreet/systemd that referenced this pull request Nov 6, 2021

upstream-ci: build with libssl-dev

e138f85

Enables additional tests and checks, added by: systemd#20691

behrmann pushed a commit to uapi-group/specifications that referenced this pull request Oct 18, 2022

Merge pull request systemd/systemd#20691 from poettering/gpt-sig

a957304

dissect: add support for GPT images with embedded dm-verity signatures

keszybz pushed a commit to uapi-group/specifications that referenced this pull request Oct 19, 2022

Merge pull request systemd/systemd#20691 from poettering/gpt-sig

2fceecf

dissect: add support for GPT images with embedded dm-verity signatures

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

dissect: add support for GPT images with embedded dm-verity signatures #20691

dissect: add support for GPT images with embedded dm-verity signatures #20691

Uh oh!

poettering commented Sep 9, 2021

bluca commented Sep 9, 2021

poettering commented Sep 10, 2021

poettering commented Sep 10, 2021

poettering commented Sep 10, 2021

bluca commented Sep 10, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bluca left a comment

poettering commented Sep 28, 2021

bluca left a comment

bluca commented Sep 28, 2021

mrc0mmand commented Sep 28, 2021

bluca commented Sep 30, 2021

Uh oh!

dissect: add support for GPT images with embedded dm-verity signatures #20691

dissect: add support for GPT images with embedded dm-verity signatures #20691

Uh oh!

Conversation

poettering commented Sep 9, 2021

bluca commented Sep 9, 2021

poettering commented Sep 10, 2021

poettering commented Sep 10, 2021

poettering commented Sep 10, 2021

bluca commented Sep 10, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bluca left a comment

Choose a reason for hiding this comment

poettering commented Sep 28, 2021

bluca left a comment

Choose a reason for hiding this comment

bluca commented Sep 28, 2021

mrc0mmand commented Sep 28, 2021

bluca commented Sep 30, 2021