Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

many: rebase fde branch #14637

Open
wants to merge 31 commits into
base: master
Choose a base branch
from

Conversation

valentindavid
Copy link
Contributor

No description provided.

valentindavid and others added 29 commits October 17, 2024 09:54
We also make the FDE state manager install the the backend function to
be associated with the state.
It can happen that an update of assets in seed partition is interupted
and shim is old, but grub is new. That means that the new grub has to
be signed with a vendor key from shim.

Secboot will refuse to reseal if we have a potential impossible boot
chain. So we should not test that impossible case.
Because we will need to enroll multiple keys, we need to make the
first key at volume creation a bootstrap key that we remove in the
end.  This commit does not implement it, but it does add the
abstraction where to allow us to do it.
When using FDE with hooks or tpm, modifying the run model in the boot
partition should result in disk that does not unlock. A recovery must
be used in that case.
Also update github.com/mvo5/goconfigparser to latest.
`by-partuuid` does not make much sense because it uselessly assumes
that it is a partition. Conceptually we should not care about it.  It
also makes the resolution more complex as we need to fetch information
about the device which we do not really need at this point.

It is more common to resolve by filesystem UUID than part UUID. For
instance cryptsetup accepts path as
`UUID=deadbeef-dead-dead-dead-deaddeafbeef`. But it does not accept
this kind of syntax for partitions.
… process being root

Add a new access checker which verifies that the request is coming from
a root user and if the process is a snap, a required interface is
connected, with that snap present on the slot side.

Signed-off-by: Maciej Borzecki <[email protected]>
…ureboot key DBs

Add a new endpoint for integration with a local manager of EFI
secureboot key databases.

Signed-off-by: Maciej Borzecki <[email protected]>
This factors and simplifies the TPM and FDE Hook code together. This
does not yet factor the key file base one.
* overlord/fdestate: keep FDE state up to date

StartUp() initializes the empty profiles, and reseal updates them.

* secboot: reexeport secboot's kernel-key-not-found error

Signed-off-by: Maciej Borzecki <[email protected]>

* overlord/fdestate: use correct mount point for ubuntu-data

Signed-off-by: Maciej Borzecki <[email protected]>

* overlord/fdestate: skip key verification when key not in keyring

For interim compatibilty, the key used to unlock ubuntu-save may not be
present in the kernel keyring, so allow key digest verification step to
be skipped in such scenario.

Signed-off-by: Maciej Borzecki <[email protected]>

* secboot: use secboot marshallers instead of encoding/json for PCR profiles

Signed-off-by: Maciej Borzecki <[email protected]>

---------

Signed-off-by: Maciej Borzecki <[email protected]>
Co-authored-by: Maciej Borzecki <[email protected]>
@github-actions github-actions bot added Needs Documentation -auto- Label automatically added which indicates the change needs documentation Run Nested -auto- Label automatically added in case nested tests need to be executed labels Oct 17, 2024
@valentindavid valentindavid added the Run nested The PR also runs tests inluded in nested suite label Oct 17, 2024
@valentindavid valentindavid reopened this Oct 17, 2024
@valentindavid valentindavid force-pushed the valentindavid/fde-branch-rebased branch from 3c716ac to 6dcf4a7 Compare October 17, 2024 08:42
@valentindavid valentindavid force-pushed the valentindavid/fde-branch-rebased branch from 6dcf4a7 to fd3e847 Compare October 17, 2024 10:23
Copy link

codecov bot commented Oct 17, 2024

Codecov Report

Attention: Patch coverage is 71.80883% with 466 lines in your changes missing coverage. Please review.

Project coverage is 78.87%. Comparing base (2e47491) to head (24018ee).
Report is 27 commits behind head on master.

Files with missing lines Patch % Lines
overlord/fdestate/fdestate.go 63.59% 62 Missing and 13 partials ⚠️
secboot/encrypt_sb.go 66.88% 34 Missing and 16 partials ⚠️
overlord/fdestate/backend/reseal.go 66.18% 33 Missing and 14 partials ⚠️
boot/seal.go 77.77% 33 Missing and 13 partials ⚠️
secboot/secboot_sb.go 65.90% 42 Missing and 3 partials ⚠️
secboot/secboot_tpm.go 74.50% 28 Missing and 11 partials ⚠️
overlord/fdestate/backend/seal.go 81.09% 27 Missing and 11 partials ⚠️
daemon/api_system_secureboot.go 73.68% 14 Missing and 6 partials ⚠️
overlord/devicestate/handlers_install.go 61.22% 11 Missing and 8 partials ⚠️
secboot/secboot_dummy.go 0.00% 18 Missing ⚠️
... and 10 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #14637      +/-   ##
==========================================
- Coverage   78.89%   78.87%   -0.02%     
==========================================
  Files        1083     1092       +9     
  Lines      146377   147404    +1027     
==========================================
+ Hits       115479   116270     +791     
- Misses      23695    23886     +191     
- Partials     7203     7248      +45     
Flag Coverage Δ
unittests 78.87% <71.80%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⛔ Blocked Needs Documentation -auto- Label automatically added which indicates the change needs documentation Run Nested -auto- Label automatically added in case nested tests need to be executed Run nested The PR also runs tests inluded in nested suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants