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

efi: Add support for shim binaries used during snapd spread tests #266

Merged

Conversation

chrisccoulson
Copy link
Collaborator

@chrisccoulson chrisccoulson commented Oct 26, 2023

Snapd re-signs production shim and grub binaries (previously signed by Microsoft and Canonical) with a snakeoil key, so recognize this as part of the Microsoft hierarchy during testing.

Some tests also recompile our patched shim v15, which we use an image digest match to recognize it as having patches backported from 15.2. Recognize any shim v15 signed with the snakeoil key as being supported during testing.

#268 will contain changes required for grub.

Snapd re-signs production shim and grub binaries (previously signed
by Microsoft and Canonical) with a snakeoil key, so recognize this
as part of the Microsoft hierarchy during testing.

Some tests also recompile our patched shim v15, which we use an image
digest match to recognize it as having patches backported from 15.2.
Recognize any shim v15 signed with the snakeoil key as being supported
during testing.

Some tests also replace the .vendor_cert section of a production shim
with an ephemeral certificate, and sign a production grub binary
previously signed by Canonical with the corresponding key, so recognize
these grubs as having the chainloader modifications during testing.
Grub detection will be fixed separately in canonical#268
@chrisccoulson chrisccoulson changed the title efi: Add support for binaries used during snapd spread tests efi: Add support for shim binaries used during snapd spread tests Nov 28, 2023
@chrisccoulson chrisccoulson marked this pull request as ready for review November 28, 2023 09:44
@chrisccoulson chrisccoulson force-pushed the efi-support-snapd-test-binaries branch from 9e9fdc6 to 80feeaa Compare November 28, 2023 09:44
Copy link
Collaborator

@pedronis pedronis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good but a couple of questions to double check some things

Comment on lines +130 to +132
bytes.Equal(authority.issuer, cert.RawIssuer) &&
bytes.Equal(authority.authorityKeyId, cert.AuthorityKeyId) &&
authority.signatureAlgorithm == cert.SignatureAlgorithm {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for non snakeoil keys both sides here are going to be empty?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One side will always have all fields. I did consider trying to accommodate this but it looked over complicated and it would only make a difference in an edge case - ie, a shim that embeds the Microsoft CA as a vendor certificate. In this case, it would be added twice, which isn't really a problem anyway.

// secureBootNamespaceRules, only during testing. This also supports the case where the
// specified authority is the signer.
func withSelfSignedSignerOnlyForTesting(subject, subjectKeyId []byte, publicKeyAlgorithm x509.PublicKeyAlgorithm, signatureAlgorithm x509.SignatureAlgorithm) secureBootNamespaceOption {
if !snapdenvTesting() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just to double check: these are relevant only on the policy generation side? also to manipulate this we need to influence a root-running service environment? also ultimately what can be booted with is controlled by secureboot?

maybe some of this consideration should go in a comment somewhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is only relevant when generating policies, and just required for image detection. You can still use an API to generate a policy for binaries signed by any key, it's just that some of the edge cases in the detection might not work and you could end up with the wrong digests.

Copy link
Collaborator

@pedronis pedronis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you

@chrisccoulson chrisccoulson merged commit 14fd3d6 into canonical:master Dec 1, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants