Skip to content

Commit

Permalink
preinstall: Add explicit checks for empty PCR banks
Browse files Browse the repository at this point in the history 
Whilst this has no consequence for FDE because we seal against a good
bank, it breaks measured boot as required by remove verifiers, as an
empty PCR bank provides a means for an adversary to spoof any host
platform of their choosing.

We reject systems with empty PCR banks by default, but with an opt-in to
permit it.
  • Loading branch information
@chrisccoulson
chrisccoulson committed Nov 13, 2024
1 parent e3f1bc2 commit e49aa0c
Show file tree
Hide file tree
Showing 6 changed files with 416 additions and 220 deletions.
32 changes: 27 additions & 5 deletions efi/preinstall/check_tcglog.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,14 @@ func (r *pcrResults) Err() error {
}
if !r.extended() {
// Return an error if the PCR hasn't been extended.
// This generally shouldn't happen because there should at
// least be a EV_SEPARATOR event, and if there isn't one, we
// trigger errors elsewhere related to the structure of the log.
return errors.New("PCR has not been extended by platform firmware")
}
if !bytes.Equal(r.pcrValue, r.logValue) {
// The PCR value is inconsistent with the log value.
return fmt.Errorf("PCR value mismatch (actual from TPM %#x, reconstructed from log %#x)", r.pcrValue, r.logValue)
return &PCRValueMismatchError{PCRValue: r.pcrValue, LogValue: r.logValue}
}
return nil
}
Expand Down Expand Up @@ -257,8 +260,7 @@ func checkFirmwareLogAgainstTPMForAlg(tpm *tpm2.TPMContext, log *tcglog.Log, alg
break
}
if !supported {
// The log doesn't contain the specified algorithm
return nil, errors.New("digest algorithm not present in log")
return nil, ErrPCRBankMissingFromLog
}

// Create the result tracker for PCRs 0-7
Expand Down Expand Up @@ -576,7 +578,7 @@ func (t *tcglogPhaseTracker) reachedOSPresent() bool {
// PCRs set, but the errors will be accessible on the returned results struct.
//
// The returned results struct indicates the best PCR bank to use and specifies the TPM startup locality as well.
func checkFirmwareLogAndChoosePCRBank(tpm *tpm2.TPMContext, log *tcglog.Log, mandatoryPcrs tpm2.HandleList) (results *pcrBankResults, err error) {
func checkFirmwareLogAndChoosePCRBank(tpm *tpm2.TPMContext, log *tcglog.Log, mandatoryPcrs tpm2.HandleList, permitEmptyPCRBanks bool) (results *pcrBankResults, err error) {
// Make sure it's a crypto-agile log
if !log.Spec.IsEFI_2() {
return nil, errors.New("invalid log spec")
Expand All @@ -586,7 +588,10 @@ func checkFirmwareLogAndChoosePCRBank(tpm *tpm2.TPMContext, log *tcglog.Log, man
// likely to get SHA-256 here - it's only in very recent devices that we have TPMs with
// SHA-384 support and corresponding firmware integration.
// We try to keep all errors enountered during selection here.
mainErr := new(NoSuitablePCRAlgorithmError)
mainErr := &NoSuitablePCRAlgorithmError{
BankErrs: make(map[tpm2.HashAlgorithmId]error),
PCRErrs: make(map[tpm2.HashAlgorithmId]map[tpm2.Handle]error),
}
var chosenResults *pcrBankResults
for _, alg := range supportedAlgs {
if chosenResults != nil {
Expand All @@ -596,6 +601,23 @@ func checkFirmwareLogAndChoosePCRBank(tpm *tpm2.TPMContext, log *tcglog.Log, man

results, err := checkFirmwareLogAgainstTPMForAlg(tpm, log, alg, mandatoryPcrs)
switch {
case errors.Is(err, ErrPCRBankMissingFromLog):
if !permitEmptyPCRBanks {
// Make sure that the TPM PCR bank is not enabled
pcrs, err := tpm.GetCapabilityPCRs()
if err != nil {
return nil, fmt.Errorf("cannot obtain active PCRs: %w", err)
}
for _, selection := range pcrs {
if selection.Hash == alg && len(selection.Select) > 0 {
// This bank is missing from the log but enabled on the TPM.
// This is very bad for remote attestation (not so bad for FDE), but treat
// this as a serious error nonetheless.
return nil, &EmptyPCRBankError{alg}
}
}
}
fallthrough
case err != nil:
// This entire bank is bad
mainErr.setBankErr(alg, err)
Expand Down
156 changes: 127 additions & 29 deletions efi/preinstall/check_tcglog_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ type testCheckFirmwareLogAndChoosePCRBankParams struct {
startupLocality uint8
disallowPreOSVerification bool
mandatoryPcrs tpm2.HandleList
permitEmptyPCRBanks bool

expectedAlg tpm2.HashAlgorithmId
}
Expand All @@ -189,7 +190,7 @@ func (s *tcglogSuite) testCheckFirmwareLogAndChoosePCRBank(c *C, params *testChe
DisallowPreOSVerification: params.disallowPreOSVerification,
})
s.resetTPMAndReplayLog(c, log, params.logAlgs...)
result, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, params.mandatoryPcrs)
result, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, params.mandatoryPcrs, params.permitEmptyPCRBanks)
c.Assert(err, IsNil)
c.Check(result.Alg, Equals, params.expectedAlg)
c.Check(result.StartupLocality, Equals, params.startupLocality)
Expand Down Expand Up @@ -285,7 +286,8 @@ func (s *tcglogSuite) TestCheckFirmwareLogAndChoosePCRBankSHA256WithEmptySHA384B
internal_efi.PlatformManufacturerPCR,
internal_efi.SecureBootPolicyPCR,
},
expectedAlg: tpm2.HashAlgorithmSHA256,
permitEmptyPCRBanks: true,
expectedAlg: tpm2.HashAlgorithmSHA256,
})
}

Expand Down Expand Up @@ -405,24 +407,27 @@ func (s *tcglogSuite) TestCheckFirmwareLogAndChoosePCRBankUnexpectedStartupLocal

_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, tpm2.HandleList{
internal_efi.PlatformFirmwarePCR,
})
}, false)
c.Check(err, ErrorMatches, `no suitable PCR algorithm available:
- TPM_ALG_SHA512: digest algorithm not present in log.
- TPM_ALG_SHA384: digest algorithm not present in log.
- TPM_ALG_SHA512: the PCR bank is missing from the TCG log.
- TPM_ALG_SHA384: the PCR bank is missing from the TCG log.
- TPM_ALG_SHA256\(PCR0\): PCR value mismatch \(actual from TPM 0xb0d6d5f50852be1524306ad88b928605c14338e56a1b8c0dc211a144524df2ef, reconstructed from log 0xa6602a7a403068b5556e78cc3f5b00c9c76d33d514093ca9b584dce7590e6c69\).
- TPM_ALG_SHA256\(PCR1\): unexpected StartupLocality event \(should be in PCR0\).
`)
var e *NoSuitablePCRAlgorithmError
c.Check(errors.As(err, &e), testutil.IsTrue)

// Test that we can access individual errors.
c.Check(e.UnwrapBankError(tpm2.HashAlgorithmSHA512), ErrorMatches, `digest algorithm not present in log`)
c.Check(e.UnwrapBankError(tpm2.HashAlgorithmSHA384), ErrorMatches, `digest algorithm not present in log`)
c.Check(e.UnwrapPCRError(tpm2.HashAlgorithmSHA384, internal_efi.PlatformFirmwarePCR), IsNil)
c.Check(e.UnwrapBankError(tpm2.HashAlgorithmSHA256), IsNil)
c.Check(e.UnwrapPCRError(tpm2.HashAlgorithmSHA256, internal_efi.PlatformFirmwarePCR), ErrorMatches, `PCR value mismatch \(actual from TPM 0xb0d6d5f50852be1524306ad88b928605c14338e56a1b8c0dc211a144524df2ef, reconstructed from log 0xa6602a7a403068b5556e78cc3f5b00c9c76d33d514093ca9b584dce7590e6c69\)`)
c.Check(e.UnwrapPCRError(tpm2.HashAlgorithmSHA256, internal_efi.PlatformConfigPCR), ErrorMatches, `unexpected StartupLocality event \(should be in PCR0\)`)
c.Check(e.UnwrapPCRError(tpm2.HashAlgorithmSHA256, internal_efi.DriversAndAppsPCR), IsNil)
c.Check(e.BankErrs[tpm2.HashAlgorithmSHA512], Equals, ErrPCRBankMissingFromLog)
c.Check(e.BankErrs[tpm2.HashAlgorithmSHA384], Equals, ErrPCRBankMissingFromLog)
c.Check(e.PCRErrs[tpm2.HashAlgorithmSHA384][internal_efi.PlatformFirmwarePCR], IsNil)
c.Check(e.BankErrs[tpm2.HashAlgorithmSHA256], IsNil)

var mismatchErr *PCRValueMismatchError
c.Check(e.PCRErrs[tpm2.HashAlgorithmSHA256][internal_efi.PlatformFirmwarePCR], ErrorMatches, `PCR value mismatch \(actual from TPM 0xb0d6d5f50852be1524306ad88b928605c14338e56a1b8c0dc211a144524df2ef, reconstructed from log 0xa6602a7a403068b5556e78cc3f5b00c9c76d33d514093ca9b584dce7590e6c69\)`)
c.Check(errors.As(e.PCRErrs[tpm2.HashAlgorithmSHA256][internal_efi.PlatformFirmwarePCR], &mismatchErr), testutil.IsTrue)
c.Check(e.PCRErrs[tpm2.HashAlgorithmSHA256][internal_efi.PlatformConfigPCR], ErrorMatches, `unexpected StartupLocality event \(should be in PCR0\)`)
c.Check(e.PCRErrs[tpm2.HashAlgorithmSHA256][internal_efi.DriversAndAppsPCR], IsNil)
}

func (s *tcglogSuite) TestCheckFirmwareLogAndChoosePCRBankOutOfPlaceStartupLocality(c *C) {
Expand Down Expand Up @@ -481,10 +486,10 @@ func (s *tcglogSuite) TestCheckFirmwareLogAndChoosePCRBankOutOfPlaceStartupLocal

_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, tpm2.HandleList{
internal_efi.PlatformFirmwarePCR,
})
}, false)
c.Check(err, ErrorMatches, `no suitable PCR algorithm available:
- TPM_ALG_SHA512: digest algorithm not present in log.
- TPM_ALG_SHA384: digest algorithm not present in log.
- TPM_ALG_SHA512: the PCR bank is missing from the TCG log.
- TPM_ALG_SHA384: the PCR bank is missing from the TCG log.
- TPM_ALG_SHA256\(PCR0\): unexpected StartupLocality event after measurements already made.
`)
var e *NoSuitablePCRAlgorithmError
Expand Down Expand Up @@ -524,10 +529,10 @@ func (s *tcglogSuite) TestCheckFirmwareLogAndChoosePCRBankInvalidStartupLocality

_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, tpm2.HandleList{
internal_efi.PlatformFirmwarePCR,
})
}, false)
c.Check(err, ErrorMatches, `no suitable PCR algorithm available:
- TPM_ALG_SHA512: digest algorithm not present in log.
- TPM_ALG_SHA384: digest algorithm not present in log.
- TPM_ALG_SHA512: the PCR bank is missing from the TCG log.
- TPM_ALG_SHA384: the PCR bank is missing from the TCG log.
- TPM_ALG_SHA256\(PCR0\): invalid StartupLocality value 2 - TPM2_Startup is only permitted from locality 0 or 3, or PCR0 can be initialized from locality 4 by a H-CRTM event before TPM2_Startup is called.
`)
var e *NoSuitablePCRAlgorithmError
Expand All @@ -550,10 +555,10 @@ func (s *tcglogSuite) TestCheckFirmwareLogAndChoosePCRBankPCRMismatchMandatory(c
})
_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, tpm2.HandleList{
internal_efi.PlatformFirmwarePCR,
})
}, false)
c.Check(err, ErrorMatches, `no suitable PCR algorithm available:
- TPM_ALG_SHA512: digest algorithm not present in log.
- TPM_ALG_SHA384: digest algorithm not present in log.
- TPM_ALG_SHA512: the PCR bank is missing from the TCG log.
- TPM_ALG_SHA384: the PCR bank is missing from the TCG log.
- TPM_ALG_SHA256\(PCR0\): PCR value mismatch \(actual from TPM 0xb0d6d5f50852be1524306ad88b928605c14338e56a1b8c0dc211a144524df2ef, reconstructed from log 0xa6602a7a403068b5556e78cc3f5b00c9c76d33d514093ca9b584dce7590e6c69\).
`)
var e *NoSuitablePCRAlgorithmError
Expand Down Expand Up @@ -584,6 +589,7 @@ func (s *tcglogSuite) TestCheckFirmwareLogAndChoosePCRBankPCRMismatchNonMandator
internal_efi.PlatformManufacturerPCR,
internal_efi.SecureBootPolicyPCR,
},
false,
)
c.Assert(err, IsNil)
c.Check(results.Alg, Equals, tpm2.HashAlgorithmSHA256)
Expand Down Expand Up @@ -623,6 +629,7 @@ func (s *tcglogSuite) TestCheckFirmwareLogAndChoosePCRBankPCRMismatchMandatoryIn
internal_efi.PlatformManufacturerPCR,
internal_efi.SecureBootPolicyPCR,
},
false,
)
c.Assert(err, IsNil)
c.Check(results.Alg, Equals, tpm2.HashAlgorithmSHA256)
Expand Down Expand Up @@ -658,6 +665,7 @@ func (s *tcglogSuite) TestCheckFirmwareLogAndChoosePCRBankPCRMismatchNonMandator
internal_efi.PlatformManufacturerPCR,
internal_efi.SecureBootPolicyPCR,
},
false,
)
c.Assert(err, IsNil)
c.Check(results.Alg, Equals, tpm2.HashAlgorithmSHA384)
Expand Down Expand Up @@ -716,6 +724,7 @@ func (s *tcglogSuite) TestCheckFirmwareLogAndChoosePCRBankSecureBootConfigJumpsT
internal_efi.PlatformManufacturerPCR,
internal_efi.SecureBootPolicyPCR,
},
false,
)
c.Assert(err, IsNil)
c.Check(results.Alg, Equals, tpm2.HashAlgorithmSHA256)
Expand All @@ -732,7 +741,7 @@ func (s *tcglogSuite) TestCheckFirmwareLogAndChoosePCRBankBadSpec(c *C) {
Minor: 2,
Errata: 0,
}
_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, nil)
_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, nil, false)
c.Check(err, ErrorMatches, `invalid log spec`)
}

Expand Down Expand Up @@ -765,7 +774,7 @@ func (s *tcglogSuite) TestCheckFirmwareLogAndChoosePCRBankPreOSMeasurementToNonT
log.Events = eventsCopy
s.resetTPMAndReplayLog(c, log, tpm2.HashAlgorithmSHA256)

_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, nil)
_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, nil, false)
c.Check(err, ErrorMatches, `measurements were made by firmware from pre-OS environment to non-TCG defined PCR 8`)
}

Expand All @@ -789,7 +798,7 @@ func (s *tcglogSuite) TestCheckFirmwareLogAndChoosePCRBankSeparatorDecodeError(c
}
s.resetTPMAndReplayLog(c, log, tpm2.HashAlgorithmSHA256)

_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, nil)
_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, nil, false)
c.Check(err, ErrorMatches, `invalid event data for EV_SEPARATOR event in PCR 7: some error`)
}

Expand All @@ -813,7 +822,7 @@ func (s *tcglogSuite) TestCheckFirmwareLogAndChoosePCRBankSeparatorError(c *C) {
}
s.resetTPMAndReplayLog(c, log, tpm2.HashAlgorithmSHA256)

_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, nil)
_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, nil, false)
c.Check(err, ErrorMatches, `EV_SEPARATOR event for PCR 7 indicates an error occurred \(error code in log: 67305985\)`)
}

Expand Down Expand Up @@ -849,7 +858,7 @@ func (s *tcglogSuite) TestCheckFirmwareLogAndChoosePCRBankUnexpectedSuccessfulSe

s.resetTPMAndReplayLog(c, log, tpm2.HashAlgorithmSHA256)

_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, nil)
_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, nil, false)
c.Check(err, ErrorMatches, `unexpected normal EV_SEPARATOR event in PCR 0`)
}

Expand Down Expand Up @@ -878,7 +887,7 @@ func (s *tcglogSuite) TestCheckFirmwareLogAndChoosePCRBankMissingSeparators(c *C
log.Events = eventsCopy
s.resetTPMAndReplayLog(c, log, tpm2.HashAlgorithmSHA256)

_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, nil)
_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, nil, false)
c.Check(err, ErrorMatches, `unexpected EV_EFI_VARIABLE_AUTHORITY event in PCR 7 whilst transitioning to OS-present \(expected EV_SEPARATOR\)`)
}

Expand All @@ -904,7 +913,7 @@ func (s *tcglogSuite) TestCheckFirmwareLogAndChoosePCRBankMultipleSeparatorsForS
log.Events = eventsCopy
s.resetTPMAndReplayLog(c, log, tpm2.HashAlgorithmSHA256)

_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, nil)
_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, nil, false)
c.Check(err, ErrorMatches, `more than one EV_SEPARATOR event exists for PCR 7`)
}

Expand All @@ -928,6 +937,95 @@ func (s *tcglogSuite) TestCheckFirmwareLogAndChoosePCRBankTruncatedLog(c *C) {
log.Events = eventsCopy
s.resetTPMAndReplayLog(c, log, tpm2.HashAlgorithmSHA256)

_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, nil)
_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log, nil, false)
c.Check(err, ErrorMatches, `reached the end of the log without seeing EV_SEPARATOR events in all TCG defined PCRs`)
}

func (s *tcglogSuite) TestCheckFirmwareLogAndChoosePCRBankEmptyPCRBankNotAllowed(c *C) {
s.RequireAlgorithm(c, tpm2.AlgorithmSHA384)
s.allocatePCRBanks(c, tpm2.HashAlgorithmSHA256, tpm2.HashAlgorithmSHA384)

log := efitest.NewLog(c, &efitest.LogOptions{
Algorithms: []tpm2.HashAlgorithmId{tpm2.HashAlgorithmSHA256},
StartupLocality: 3,
})
s.resetTPMAndReplayLog(c, log, tpm2.HashAlgorithmSHA256)

// This will make the PCR 0 calculation wrong
log = efitest.NewLog(c, &efitest.LogOptions{
Algorithms: []tpm2.HashAlgorithmId{tpm2.HashAlgorithmSHA256},
})
_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log,
tpm2.HandleList{
internal_efi.PlatformConfigPCR,
internal_efi.DriversAndAppsPCR,
internal_efi.DriversAndAppsConfigPCR,
internal_efi.BootManagerCodePCR,
internal_efi.BootManagerConfigPCR,
internal_efi.PlatformManufacturerPCR,
internal_efi.SecureBootPolicyPCR,
},
false,
)
c.Check(err, ErrorMatches, `the PCR bank for TPM_ALG_SHA384 is missing from the TCG log but is active on the TPM`)

var emptyPCRErr *EmptyPCRBankError
c.Check(errors.As(err, &emptyPCRErr), testutil.IsTrue)
}

func (s *tcglogSuite) TestCheckFirmwareLogAndChoosePCRBankEmptyPCRBankAllowed(c *C) {
s.RequireAlgorithm(c, tpm2.AlgorithmSHA384)
s.allocatePCRBanks(c, tpm2.HashAlgorithmSHA256, tpm2.HashAlgorithmSHA384)

log := efitest.NewLog(c, &efitest.LogOptions{
Algorithms: []tpm2.HashAlgorithmId{tpm2.HashAlgorithmSHA256},
StartupLocality: 3,
})
s.resetTPMAndReplayLog(c, log, tpm2.HashAlgorithmSHA256)

// This will make the PCR 0 calculation wrong
log = efitest.NewLog(c, &efitest.LogOptions{
Algorithms: []tpm2.HashAlgorithmId{tpm2.HashAlgorithmSHA256},
})
_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log,
tpm2.HandleList{
internal_efi.PlatformConfigPCR,
internal_efi.DriversAndAppsPCR,
internal_efi.DriversAndAppsConfigPCR,
internal_efi.BootManagerCodePCR,
internal_efi.BootManagerConfigPCR,
internal_efi.PlatformManufacturerPCR,
internal_efi.SecureBootPolicyPCR,
},
true,
)
c.Assert(err, IsNil)
}

func (s *tcglogSuite) TestCheckFirmwareLogAndChoosePCRBankSHA256WithEmptySHA384BankBad(c *C) {
s.RequireAlgorithm(c, tpm2.AlgorithmSHA384)
s.allocatePCRBanks(c, tpm2.HashAlgorithmSHA256, tpm2.HashAlgorithmSHA384)

log := efitest.NewLog(c, &efitest.LogOptions{
Algorithms: []tpm2.HashAlgorithmId{tpm2.HashAlgorithmSHA256},
StartupLocality: 3,
})
s.resetTPMAndReplayLog(c, log, tpm2.HashAlgorithmSHA256)

_, err := CheckFirmwareLogAndChoosePCRBank(s.TPM, log,
tpm2.HandleList{
internal_efi.PlatformConfigPCR,
internal_efi.DriversAndAppsPCR,
internal_efi.DriversAndAppsConfigPCR,
internal_efi.BootManagerCodePCR,
internal_efi.BootManagerConfigPCR,
internal_efi.PlatformManufacturerPCR,
internal_efi.SecureBootPolicyPCR,
},
false,
)
c.Check(err, ErrorMatches, `the PCR bank for TPM_ALG_SHA384 is missing from the TCG log but is active on the TPM`)

var bankErr *EmptyPCRBankError
c.Check(errors.As(err, &bankErr), testutil.IsTrue)
}
Loading

0 comments on commit e49aa0c

Please sign in to comment.