-
Notifications
You must be signed in to change notification settings - Fork 17
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
Changes from all commits
8419d5a
1569e7a
787a312
356e6cf
6499519
80feeaa
06ca8cc
8d5b857
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,9 +23,14 @@ import ( | |
"bytes" | ||
"crypto/x509" | ||
|
||
"github.com/snapcore/snapd/snapdenv" | ||
"golang.org/x/xerrors" | ||
) | ||
|
||
var ( | ||
snapdenvTesting = snapdenv.Testing | ||
) | ||
|
||
// vendorAuthorityGetter provides a way for an imageLoadHandler created by | ||
// secureBootNamespaceRules to supplement the CA's associated with a secure | ||
// boot namespace in the case where the associated image contains a delegated | ||
|
@@ -43,9 +48,14 @@ type secureBootAuthorityIdentity struct { | |
subject []byte | ||
subjectKeyId []byte | ||
publicKeyAlgorithm x509.PublicKeyAlgorithm | ||
|
||
issuer []byte | ||
authorityKeyId []byte | ||
signatureAlgorithm x509.SignatureAlgorithm | ||
} | ||
|
||
// withAuthority adds the specified secure boot authority to a secureBootNamespaceRules. | ||
// Note that this won't match if the specified authority directly signs things. | ||
func withAuthority(subject, subjectKeyId []byte, publicKeyAlgorithm x509.PublicKeyAlgorithm) secureBootNamespaceOption { | ||
return func(ns *secureBootNamespaceRules) { | ||
ns.authorities = append(ns.authorities, &secureBootAuthorityIdentity{ | ||
|
@@ -55,13 +65,42 @@ func withAuthority(subject, subjectKeyId []byte, publicKeyAlgorithm x509.PublicK | |
} | ||
} | ||
|
||
// withSelfSignedSignerOnlyForTesting adds the specified secure boot authority to a | ||
// secureBootNamespaceRules, only during testing. This also supports the case where the | ||
// specified authority directly signs things. This is used to ensure that binaries signed | ||
// by a production CA that are re-signed during testing by a testing-only CA are still | ||
// detected correctly, in the same way that the production binary would be. | ||
func withSelfSignedSignerOnlyForTesting(subject, subjectKeyId []byte, publicKeyAlgorithm x509.PublicKeyAlgorithm, signatureAlgorithm x509.SignatureAlgorithm) secureBootNamespaceOption { | ||
if !snapdenvTesting() { | ||
return func(_ *secureBootNamespaceRules) {} | ||
} | ||
return func(ns *secureBootNamespaceRules) { | ||
ns.authorities = append(ns.authorities, &secureBootAuthorityIdentity{ | ||
subject: subject, | ||
subjectKeyId: subjectKeyId, | ||
publicKeyAlgorithm: publicKeyAlgorithm, | ||
issuer: subject, | ||
authorityKeyId: subjectKeyId, | ||
signatureAlgorithm: signatureAlgorithm}) | ||
} | ||
} | ||
|
||
// withImageRule adds the specified rule to a secureBootNamespaceRules. | ||
func withImageRule(name string, match imagePredicate, create newImageLoadHandlerFn) secureBootNamespaceOption { | ||
return func(ns *secureBootNamespaceRules) { | ||
ns.rules = append(ns.rules, newImageRule(name, match, create)) | ||
} | ||
} | ||
|
||
// withImageRuleOnlyForTesting adds the specified rule to a secureBootNamespaceRules, | ||
// only during testing. | ||
func withImageRuleOnlyForTesting(name string, match imagePredicate, create newImageLoadHandlerFn) secureBootNamespaceOption { | ||
if !snapdenvTesting() { | ||
return func(_ *secureBootNamespaceRules) {} | ||
} | ||
return withImageRule(name, match, create) | ||
} | ||
|
||
type secureBootNamespaceOption func(*secureBootNamespaceRules) | ||
|
||
// secureBootNamespaceRules is used to construct an imageLoadHandler from a | ||
|
@@ -86,11 +125,17 @@ func newSecureBootNamespaceRules(name string, options ...secureBootNamespaceOpti | |
|
||
func (r *secureBootNamespaceRules) AddAuthorities(certs ...*x509.Certificate) { | ||
for _, cert := range certs { | ||
// Avoid adding duplicates. Note that this is only guaranteed to de-duplicate | ||
// those certificates added via this API, as the built-in certificates only | ||
// have a minimal set of fields populated and we don't try to handle that case. | ||
found := false | ||
for _, authority := range r.authorities { | ||
if bytes.Equal(authority.subject, cert.RawSubject) && | ||
bytes.Equal(authority.subjectKeyId, cert.SubjectKeyId) && | ||
authority.publicKeyAlgorithm == cert.PublicKeyAlgorithm { | ||
authority.publicKeyAlgorithm == cert.PublicKeyAlgorithm && | ||
bytes.Equal(authority.issuer, cert.RawIssuer) && | ||
bytes.Equal(authority.authorityKeyId, cert.AuthorityKeyId) && | ||
authority.signatureAlgorithm == cert.SignatureAlgorithm { | ||
Comment on lines
+136
to
+138
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for non snakeoil keys both sides here are going to be empty? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
found = true | ||
break | ||
} | ||
|
@@ -100,6 +145,9 @@ func (r *secureBootNamespaceRules) AddAuthorities(certs ...*x509.Certificate) { | |
subject: cert.RawSubject, | ||
subjectKeyId: cert.SubjectKeyId, | ||
publicKeyAlgorithm: cert.PublicKeyAlgorithm, | ||
issuer: cert.RawIssuer, | ||
authorityKeyId: cert.AuthorityKeyId, | ||
signatureAlgorithm: cert.SignatureAlgorithm, | ||
}) | ||
} | ||
} | ||
|
@@ -118,7 +166,10 @@ func (r *secureBootNamespaceRules) NewImageLoadHandler(image peImageHandle) (ima | |
cert := &x509.Certificate{ | ||
RawSubject: authority.subject, | ||
SubjectKeyId: authority.subjectKeyId, | ||
PublicKeyAlgorithm: authority.publicKeyAlgorithm} | ||
PublicKeyAlgorithm: authority.publicKeyAlgorithm, | ||
RawIssuer: authority.issuer, | ||
AuthorityKeyId: authority.authorityKeyId, | ||
SignatureAlgorithm: authority.signatureAlgorithm} | ||
for _, sig := range sigs { | ||
if !sig.CertLikelyTrustAnchor(cert) { | ||
continue | ||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.