Skip to content

Conversation

@chrisccoulson
Copy link
Collaborator

@chrisccoulson chrisccoulson commented Dec 11, 2025

This updates PCRProfileOptionsFlags, such that:

  • There are now individual options to include each PCR if not already
    included (PCRProfileOptionLockTo*).
  • PCRProfileOptionsMostSecure includes all of the new
    PCRProfileOptionLockTo* options.
  • PCRProfileOptionTrustCAsForAddonDrivers and
    PCRProfileOptionTrustCAsForBootCode have been renamed to
    PCRProfileOptionTrustSecureBootAuthoritiesForAddonDrivers and
    PCRProfileOptionTrustSecureBootAuthoritiesForBootCode. They can only
    be used if the active CAs are not recognized. They can't be used to
    omit PCRs 2 or 4 from the profile if the CA is recognized and explicitly
    distrusted, so that users can't use these options to create insecure
    profiles.
  • PCRProfileOptionDistrustVARSuppliedNonHostCode is gone because it is
    superceded by PCRProfileOptionLockToDriversAndApps.

Fixes: FR-12150

@chrisccoulson chrisccoulson force-pushed the preinstall-cleanup-pcr-profile-options branch from fb26b17 to bec8f72 Compare December 11, 2025 21:15
@chrisccoulson
Copy link
Collaborator Author

Note that as with the other PRs that make some changes to PCRProfileOptionsFlags, there's no backwards compat here because PCRProfileOptionsDefault is the only value currently used by snapd, and that value remains unchanged.

@chrisccoulson chrisccoulson force-pushed the preinstall-cleanup-pcr-profile-options branch from bec8f72 to 68bfe14 Compare December 11, 2025 22:00
This updates PCRProfileOptionsFlags, such that:

- There are now individual options to include each PCR if not already
  included (PCRProfileOptionLockTo*).
- PCRProfileOptionsMostSecure includes all of the new
  PCRProfileOptionLockTo* options.
- PCRProfileOptionTrustCAsForAddonDrivers and
  PCRProfileOptionTrustCAsForBootCode have been renamed to
- PCRProfileOptionTrustSecureBootAuthoritiesForAddonDrivers and
  PCRProfileOptionTrustSecureBootAuthoritiesForBootCode. They can only
  be used if the active CAs are not recognized. They can't be used to
  omit PCRs 2 or 4 from the profile if the CA is recognized and explicitly
  distrusted, so that users can't use these options to create insecure
  profiles.
- PCRProfileOptionDistrustVARSuppliedNonHostCode is gone because it is
  superceded by PCRProfileOptionLockToDriversAndApps.

Fixes: FR-12150
@chrisccoulson chrisccoulson force-pushed the preinstall-cleanup-pcr-profile-options branch from 68bfe14 to 321eeb1 Compare December 11, 2025 22:37
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.

did a pass, some small comments

if flags&auth.Trust != flags {
return authoritiesNotTrusted
}
certFound = true
Copy link
Collaborator

Choose a reason for hiding this comment

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

isn't this more authFound than cert found?

}
trust &= certTrust
if !certFound {
return authoritiesTrustUnknown
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe this should have a comment that the authority of this cert is not in the data set


var str string
switch flag {
case PCRProfileOptionMostSecure:
Copy link
Collaborator

Choose a reason for hiding this comment

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

this value is now always exploded in the relevant flags?

{pcr: internal_efi.BootManagerConfigPCR, unsupportedFlag: NoBootManagerConfigProfileSupport},
{pcr: internal_efi.SecureBootPolicyPCR, unsupportedFlag: NoSecureBootPolicyProfileSupport, opt: secboot_efi.WithSecureBootPolicyProfile},
} {
if _, exists := pcrs[data.pcr]; exists {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think here s/exists/required/ would be a bit clearer

Copy link
Collaborator

@ernestl ernestl left a comment

Choose a reason for hiding this comment

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

Thanks

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.

3 participants