diff --git a/efi/fw_load_handler.go b/efi/fw_load_handler.go index 3cef06c5..4d0c6577 100644 --- a/efi/fw_load_handler.go +++ b/efi/fw_load_handler.go @@ -89,6 +89,20 @@ func (h *fwLoadHandler) measureAuthorizedSignatureDb(ctx pcrBranchContext) error return nil } +func (h *fwLoadHandler) measureSignatureDatabaseIfNotEmpty(ctx pcrBranchContext, name efi.VariableDescriptor) error { + switch db, _, err := efi.ReadVariable(ctx.VarContext(), name.Name, name.GUID); { + case errors.Is(err, efi.ErrVarNotExist): + return nil + case err != nil: + return fmt.Errorf("cannot read current variable: %w", err) + case len(db) == 0: + return nil + default: + ctx.MeasureVariable(internal_efi.SecureBootPolicyPCR, name.GUID, name.Name, db) + return nil + } +} + func (h *fwLoadHandler) measureSecureBootPolicyPreOS(ctx pcrBranchContext) error { // This hard-codes a profile that will only work on devices with secure boot enabled, // deployed mode on (where UEFI >= 2.5), without a UEFI debugger enabled and which @@ -202,6 +216,23 @@ func (h *fwLoadHandler) measureSecureBootPolicyPreOS(ctx pcrBranchContext) error return xerrors.Errorf("cannot measure dbx: %w", err) } + switch supported, err := efi.ReadOSIndicationsSupportedVariable(ctx.VarContext()); { + case errors.Is(err, efi.ErrVarNotExist): + // ignore + case err != nil: + return fmt.Errorf("cannot read OS indications: %w", err) + default: + if supported&efi.OSIndicationTimestampRevocation > 0 { + if err := h.measureSignatureDatabaseIfNotEmpty(ctx, Dbt); err != nil { + return fmt.Errorf("cannot measure dbt: %w", err) + } + } + if supported&efi.OSIndicationStartOSRecovery > 0 { + if err := h.measureSignatureDatabaseIfNotEmpty(ctx, Dbr); err != nil { + return fmt.Errorf("cannot measure dbr: %w", err) + } + } + } // TODO: Support optional dbt/dbr database // Include the user mode related measurements if the system is in user mode, it is diff --git a/efi/pcr_branch_context.go b/efi/pcr_branch_context.go index cbacdd5e..146e0574 100644 --- a/efi/pcr_branch_context.go +++ b/efi/pcr_branch_context.go @@ -20,6 +20,9 @@ package efi import ( + "context" + "errors" + efi "github.com/canonical/go-efilib" "github.com/canonical/go-tpm2" "github.com/canonical/tcglog-parser" @@ -32,7 +35,8 @@ import ( // context. type pcrBranchContext interface { pcrProfileContext - Params() loadParams // access the externally supplied parameters for this branch + Params() loadParams // access the externally supplied parameters for this branch + VarContext() context.Context Vars() varReadWriter // access the variable state for this branch FwContext() *fwContext // access the platform firmware state for this branch ShimContext() *shimContext // access the shim state for this branch @@ -43,6 +47,23 @@ type pcrBranchContext interface { MeasureVariable(pcr tpm2.Handle, guid efi.GUID, name string, data []byte) // measure the specified variable for this branch } +type pcrBranchVarCtx struct { + ctx *pcrBranchCtx +} + +func (c *pcrBranchVarCtx) Get(name string, guid efi.GUID) (efi.VariableAttributes, []byte, error) { + data, attrs, err := c.ctx.vars.ReadVar(name, guid) + return attrs, data, err +} + +func (c *pcrBranchVarCtx) Set(name string, guid efi.GUID, attrs efi.VariableAttributes, data []byte) error { + return c.ctx.vars.WriteVar(name, guid, attrs, data) +} + +func (c *pcrBranchVarCtx) List() ([]efi.VariableDescriptor, error) { + return nil, errors.New("not supported") +} + type pcrBranchCtx struct { pcrProfileContext branch *secboot_tpm2.PCRProtectionProfileBranch @@ -79,6 +100,10 @@ func (c *pcrBranchCtx) Params() loadParams { return c.params } +func (c *pcrBranchCtx) VarContext() context.Context { + return context.WithValue(context.Background(), efi.VarsBackendKey{}, &pcrBranchVarCtx{ctx: c}) +} + func (c *pcrBranchCtx) Vars() varReadWriter { return &c.vars } diff --git a/efi/preinstall/check_pcr7.go b/efi/preinstall/check_pcr7.go index 1b67ef9c..7ca14fe9 100644 --- a/efi/preinstall/check_pcr7.go +++ b/efi/preinstall/check_pcr7.go @@ -47,7 +47,10 @@ var ( // the signature databases decode properly. // // If the supplied parameter is for a signature database, the decoded signature database -// is returned, else nil is returned +// is returned, else nil is returned. +// +// TODO: This should optionally check the variable data from the log event against the +// actual variable contents (see https://github.com/canonical/secboot/issues/538). func checkSecureBootVariableData(data *tcglog.EFIVariableData) (sigDb efi.SignatureDatabase, err error) { switch data.UnicodeName { case "SecureBoot": @@ -286,6 +289,49 @@ func handleVariableAuthorityEvent(pcrAlg tpm2.HashAlgorithmId, db efi.SignatureD return matchedEsl.Type, esd.Data, nil } +type secureBootVariableDefinition struct { + name efi.VariableDescriptor + omitEmpty bool +} + +func checkVariableDriverConfigEventOrdering(expected *[]secureBootVariableDefinition, data *tcglog.EFIVariableData) error { + expectedTmp := *expected + for len(expectedTmp) > 0 { + config := expectedTmp[0] + expectedTmp = expectedTmp[1:] + + if data.VariableName == config.GUID && data.UnicodeName == config.Name { + *expected = expectedTmp + return nil + } + + if !e.omitEmpty { + return fmt.Errorf("unexpected EV_EFI_VARIABLE_DRIVER_CONFIG event ordering (expected %s-%v, got %s-%v)", + config.Name, config.GUID, data.UnicodeName, data.VariableName) + } + + // The next expected measurement is one that can be omitted if it is empty. + // TODO: Check that the actual variable contents are empty (see + // https://github.com/canonical/secboot/issues/538). + } + + return fmt.Errorf("unexpected EV_EFI_VARIABLE_DRIVER_CONFIG event ordering (%s-%v)", data.UnicodeName, data.VariableName) +} + +func canRemainingExpectedVariableDriverConfigEventsBeOmitted(expected []secureBootVariableDefinition) bool { + for _, e := range expected { + if !e.omitEmpty { + return false + } + + // This measurement is one that can be omitted if it is empty. + // TODO: Check that the actual variable contents are empty (see + // https://github.com/canonical/secboot/issues/538). + } + + return true +} + type secureBootPolicyResultFlags int const ( @@ -395,38 +441,31 @@ func checkSecureBootPolicyMeasurementsAndObtainAuthorities(ctx context.Context, } } - // Make sure this system doesn't support features that affect PCR7 and which we don't - // currently support. + // Determine whether timestamp revocation and/or OS recovery is supported. osIndicationsSupported, err := efi.ReadOSIndicationsSupportedVariable(varCtx) if err != nil { return nil, fmt.Errorf("cannot read OsIndicationsSupported variable: %w", err) } - if osIndicationsSupported&efi.OSIndicationTimestampRevocation > 0 { - // Timestamp verification relies on another database (dbt) which we currently don't support - // in WithSecureBootPolicyProfile(). It's theoretically possible we might see this in the - // wild and might have to add support for it in the future. - return nil, errors.New("generating secure boot profiles for systems with timestamp revocation (dbt) support is currently not supported") - } - if osIndicationsSupported&efi.OSIndicationStartOSRecovery > 0 { - // OS recovery relies on another database (dbr) which we currently don't support in - // WithSecureBootPolicyProfile(), but given this also depends on EFI_VARIABLE_AUTHENTICATION_3, - // it's unlikely we'll ever see this in the wild. - return nil, errors.New("generating secure boot profiles for systems with OS recovery support, which requires dbr support, is not supported") - } // TODO(chrisccoulson): Not sure if there's any indication that we might get SPDM related measurements, // which our profile generation for PCR7 currently doesn't support. // Make sure that the secure boot config in the log is measured in the // expected order, else WithSecureBootPolicyProfile() will generate an invalid policy, // because we hard code the order. The order here is what we expect to see. - configs := []efi.VariableDescriptor{ - {Name: "SecureBoot", GUID: efi.GlobalVariable}, - {Name: "PK", GUID: efi.GlobalVariable}, - {Name: "KEK", GUID: efi.GlobalVariable}, - {Name: "db", GUID: efi.ImageSecurityDatabaseGuid}, - {Name: "dbx", GUID: efi.ImageSecurityDatabaseGuid}, - // TODO: Add optional dbt / SPDM in the future. + configs := []secureBootVariableDefinition{ + {name: {Name: "SecureBoot", GUID: efi.GlobalVariable}}, + {name: efi.PKVariable}, + {name: efi.KEKVariable}, + {name: efi.DbVariable}, + {name: efi.DbxVariable}, + } + if osIndicationsSupported&efi.OSIndicationTimestampRevocation > 0 { + configs = append(configs, secureBootVariableDefinition{name: efi.DbtVariable, omitEmpty: true}) } + if osIndicationsSupported&efi.OSIndicationStartOSRecovery > 0 { + config = append(configs, secureBootVariableDefinition{name: efi.DbrVariable, omitEmpty: true}) + } + // TODO: Add optional SPDM variables in the future. var ( db efi.SignatureDatabase // The authorized signature database from the TCG log. @@ -501,10 +540,6 @@ NextEvent: return nil, errors.New("unexpected EV_EFI_VARIABLE_DRIVER_CONFIG event: all expected secure boot variable have been measured") } - // Pop the next secure boot config name - config := configs[0] - configs = configs[1:] - data, ok := ev.Data.(*tcglog.EFIVariableData) if !ok { // The data resulting from decode errors are guaranteed to implement the error interface @@ -512,10 +547,10 @@ NextEvent: } // Make sure this is the event we're expecting to be measured. If they're // measured in an unexpected order, then WithSecureBootPolicyProfile() will - // generate an invalid policy. - if data.VariableName != config.GUID || data.UnicodeName != config.Name { - return nil, fmt.Errorf("unexpected EV_EFI_VARIABLE_DRIVER_CONFIG event ordering (expected %s-%v, got %s-%v)", - config.Name, config.GUID, data.UnicodeName, data.VariableName) + // generate an invalid policy. This updates configs so that the next expected + // event is the first entry. + if err := checkVariableDriverConfigEventOrdering(&configs, data); err != nil { + return nil, err } // Compute the expected digest from the event data in the log and make @@ -569,9 +604,9 @@ NextEvent: } } case tcglogPhasePreOSThirdPartyDispatch: - if len(configs) > 0 { + if !canRemainingExpectedVariableDriverConfigEventsBeOmitted(configs) { // We've transitioned to a phase where components can be loaded and verified but we haven't - // measured all of the secure boot variables. We'll fail to generate a valid policy with + // measured all of the required secure boot variables. We'll fail to generate a valid policy with // WithSecureBootPolicyProfile() in this case. return nil, errors.New("EV_EFI_VARIABLE_DRIVER_CONFIG events for some secure boot variables missing from log") } @@ -639,9 +674,9 @@ NextEvent: } } case tcglogPhaseOSPresent: - if len(configs) > 0 { + if !canRemainingExpectedVariableDriverConfigEventsBeOmitted(configs) { // We've transitioned to a phase where components can be loaded and verified but we haven't - // measured all of the secure boot variables. We'll fail to generate a valid policy with + // measured all of the required secure boot variables. We'll fail to generate a valid policy with // WithSecureBootPolicyProfile() in this case. return nil, errors.New("EV_EFI_VARIABLE_DRIVER_CONFIG events for some secure boot variables missing from log") } diff --git a/efi/secureboot.go b/efi/secureboot.go index 28615747..44f1f1b3 100644 --- a/efi/secureboot.go +++ b/efi/secureboot.go @@ -33,16 +33,22 @@ import ( var ( // PK is the identity of the Platform Key variable. - PK = efi.VariableDescriptor{Name: "PK", GUID: efi.GlobalVariable} + PK = efi.PKVariable // KEK is the identity of the Key Exchange Key database. - KEK = efi.VariableDescriptor{Name: "KEK", GUID: efi.GlobalVariable} + KEK = efi.KEKVariable // Db is the identity of the authorized signature database. - Db = efi.VariableDescriptor{Name: "db", GUID: efi.ImageSecurityDatabaseGuid} + Db = efi.DbVariable // Dbx is the identity of the forbidden signature database. - Dbx = efi.VariableDescriptor{Name: "dbx", GUID: efi.ImageSecurityDatabaseGuid} + Dbx = efi.DbxVariable + + // Dbt is the identity of the authorized timestamp signature database. + Dbt = efi.DbtVariable + + // Dbr is the identity of the authorized recovery signature database. + Dbr = efi.DbrVariable ) // SignatureDBUpdate corresponds to an update to a signature database, such as dbx. diff --git a/go.mod b/go.mod index f031dd6d..c9ae28c3 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.18 require ( github.com/canonical/cpuid v0.0.0-20220614022739-219e067757cb - github.com/canonical/go-efilib v1.7.0 + github.com/canonical/go-efilib v1.8.0 github.com/canonical/go-kbkdf v0.0.0-20250104172618-3b1308f9acf9 github.com/canonical/go-password-validator v0.0.0-20250617132709-1b205303ca54 github.com/canonical/go-sp800.90a-drbg v0.0.0-20210314144037-6eeb1040d6c3 diff --git a/go.sum b/go.sum index 6622f560..90332774 100644 --- a/go.sum +++ b/go.sum @@ -7,6 +7,8 @@ github.com/canonical/go-efilib v1.6.0 h1:0ZFcIclzoMBl6sAaHrSO98YVPUvKAPfXm6kjofh github.com/canonical/go-efilib v1.6.0/go.mod h1:n0Ttsy1JuHAvqaFbZBs6PAzoiiJdfkHsAmDOEbexYEQ= github.com/canonical/go-efilib v1.7.0 h1:QLtWh/x9rtBjokext9c6e+hT/AR3X9D0U8DQN+/BVbM= github.com/canonical/go-efilib v1.7.0/go.mod h1:n0Ttsy1JuHAvqaFbZBs6PAzoiiJdfkHsAmDOEbexYEQ= +github.com/canonical/go-efilib v1.8.0 h1:VHWvbohcX1e/8NrXJhgqODCVLm69gHrFCIlY3CgFvMg= +github.com/canonical/go-efilib v1.8.0/go.mod h1:n0Ttsy1JuHAvqaFbZBs6PAzoiiJdfkHsAmDOEbexYEQ= github.com/canonical/go-kbkdf v0.0.0-20250104172618-3b1308f9acf9 h1:Twk1ZSTWRClfGShP16ePf2JIiayqWS4ix1rkAR6baag= github.com/canonical/go-kbkdf v0.0.0-20250104172618-3b1308f9acf9/go.mod h1:IneQ5/yQcfPXrGekEXpR6yeea55ZD24N5+kHzeDseOM= github.com/canonical/go-password-validator v0.0.0-20250617132709-1b205303ca54 h1:JO3wAsxjrvQDf/X3q4RLIdzDCWrFjzhwUmCKrhnrIO8=