Skip to content

Commit

Permalink
efi: Don't create profiles with invalid separators
Browse files Browse the repository at this point in the history 
We copy the EV_SEPARATOR events from the firmware log because it can
be one of two valid values. But we should be aborting early if any of
these separators indicate that an error occurred in firmware rather
than continuing to build a profile.
  • Loading branch information
@chrisccoulson
chrisccoulson committed Jun 11, 2024
1 parent 5c01e34 commit a4785d8
Show file tree
Hide file tree
Showing 2 changed files with 179 additions and 21 deletions.
57 changes: 44 additions & 13 deletions efi/fw_load_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package efi

import (
"bytes"
"encoding/binary"
"errors"
"fmt"

Expand All @@ -44,6 +45,23 @@ var newFwLoadHandler = func(log *tcglog.Log) imageLoadHandler {
return &fwLoadHandler{log: log}
}

func (h *fwLoadHandler) measureSeparator(ctx pcrBranchContext, pcr tpm2.Handle, event *tcglog.Event) error {
if event.EventType != tcglog.EventTypeSeparator {
return fmt.Errorf("unexpected event type %v", event.EventType)
}

data, ok := event.Data.(*tcglog.SeparatorEventData)
if !ok {
// if decoding fails, the resulting type is guaranteed to implement error.
return fmt.Errorf("cannot measure invalid separator event: %w", event.Data.(error))
}
if data.IsError() {
return fmt.Errorf("separator indicates that a firmware error occurred (error code from log: %d)", binary.LittleEndian.Uint32(data.Bytes()))
}
ctx.ExtendPCR(pcr, tpm2.Digest(event.Digests[ctx.PCRAlg()]))
return nil
}

func (h *fwLoadHandler) readAndMeasureSignatureDb(ctx pcrBranchContext, name efi.VariableDescriptor) ([]byte, error) {
db, _, err := ctx.Vars().ReadVar(name.Name, name.GUID)
if err != nil && err != efi.ErrVarNotExist {
Expand Down Expand Up @@ -107,7 +125,9 @@ func (h *fwLoadHandler) measureSecureBootPolicyPreOS(ctx pcrBranchContext) error
if foundSecureBootSeparator {
return errors.New("unexpected separator")
}
ctx.ExtendPCR(secureBootPolicyPCR, tpm2.Digest(e.Digests[ctx.PCRAlg()]))
if err := h.measureSeparator(ctx, secureBootPolicyPCR, e); err != nil {
return err
}
foundSecureBootSeparator = true
case e.PCRIndex == tcglog.PCRIndex(secureBootPolicyPCR) && e.EventType == tcglog.EventTypeEFIVariableAuthority:
// secure boot verification event - shouldn't see this before the end of secure
Expand Down Expand Up @@ -135,6 +155,9 @@ func (h *fwLoadHandler) measureSecureBootPolicyPreOS(ctx pcrBranchContext) error
}
}

if !foundSecureBootSeparator {
return errors.New("missing separator")
}
return nil
}

Expand Down Expand Up @@ -164,29 +187,31 @@ func (h *fwLoadHandler) measurePlatformFirmware(ctx pcrBranchContext) error {
donePcrReset = true
}

ctx.ExtendPCR(platformFirmwarePCR, tpm2.Digest(event.Digests[ctx.PCRAlg()]))
if event.EventType == tcglog.EventTypeSeparator {
break
return h.measureSeparator(ctx, platformFirmwarePCR, event)
}
ctx.ExtendPCR(platformFirmwarePCR, tpm2.Digest(event.Digests[ctx.PCRAlg()]))
}

return nil
return errors.New("missing separator")
}

func (h *fwLoadHandler) measureDriversAndApps(ctx pcrBranchContext) {
func (h *fwLoadHandler) measureDriversAndApps(ctx pcrBranchContext) error {
for _, event := range h.log.Events {
if event.PCRIndex != tcglog.PCRIndex(driversAndAppsPCR) {
continue
}

ctx.ExtendPCR(driversAndAppsPCR, tpm2.Digest(event.Digests[ctx.PCRAlg()]))
if event.EventType == tcglog.EventTypeSeparator {
break
return h.measureSeparator(ctx, driversAndAppsPCR, event)
}
ctx.ExtendPCR(driversAndAppsPCR, tpm2.Digest(event.Digests[ctx.PCRAlg()]))
}

return errors.New("missing separator")
}

func (h *fwLoadHandler) measureBootManagerCodePreOS(ctx pcrBranchContext) {
func (h *fwLoadHandler) measureBootManagerCodePreOS(ctx pcrBranchContext) error {
// Replay the log until the transition to the OS. Different firmware implementations and
// configurations perform different pre-OS measurements, and these events need to be preserved
// in the profile.
Expand Down Expand Up @@ -220,11 +245,13 @@ func (h *fwLoadHandler) measureBootManagerCodePreOS(ctx pcrBranchContext) {
continue
}

ctx.ExtendPCR(bootManagerCodePCR, tpm2.Digest(event.Digests[ctx.PCRAlg()]))
if event.EventType == tcglog.EventTypeSeparator {
break
return h.measureSeparator(ctx, bootManagerCodePCR, event)
}
ctx.ExtendPCR(bootManagerCodePCR, tpm2.Digest(event.Digests[ctx.PCRAlg()]))
}

return errors.New("missing separator")
}

// MeasureImageStart implements imageLoadHandler.MeasureImageStart.
Expand All @@ -245,14 +272,18 @@ func (h *fwLoadHandler) MeasureImageStart(ctx pcrBranchContext) error {

if ctx.PCRs().Contains(platformFirmwarePCR) {
if err := h.measurePlatformFirmware(ctx); err != nil {
return fmt.Errorf("cannot measure platform firmware policy: %w", err)
return fmt.Errorf("cannot measure platform firmware: %w", err)
}
}
if ctx.PCRs().Contains(driversAndAppsPCR) {
h.measureDriversAndApps(ctx)
if err := h.measureDriversAndApps(ctx); err != nil {
return fmt.Errorf("cannot measure drivers and apps: %w", err)
}
}
if ctx.PCRs().Contains(bootManagerCodePCR) {
h.measureBootManagerCodePreOS(ctx)
if err := h.measureBootManagerCodePreOS(ctx); err != nil {
return fmt.Errorf("cannot measure boot manager code: %w", err)
}
}
if ctx.PCRs().Contains(secureBootPolicyPCR) {
if err := h.measureSecureBootPolicyPreOS(ctx); err != nil {
Expand Down
143 changes: 135 additions & 8 deletions efi/fw_load_handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package efi_test

import (
"errors"
"fmt"
"io"

Expand Down Expand Up @@ -268,7 +269,7 @@ func (s *fwLoadHandlerSuite) TestMeasureImageStartDriversAndAppsProfile2(c *C) {
})
}

func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLog1(c *C) {
func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLogPCR7_1(c *C) {
// Insert a second EV_SEPARATOR event into PCR7
collector := NewRootVarsCollector(efitest.NewMockHostEnvironment(makeMockVars(c, withMsSecureBootConfig()), nil))
ctx := newMockPcrBranchContext(&mockPcrProfileContext{
Expand All @@ -292,7 +293,7 @@ func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLog1(c *C) {
c.Check(handler.MeasureImageStart(ctx), ErrorMatches, `cannot measure secure boot policy: unexpected separator`)
}

func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLog2(c *C) {
func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLogPCR7_2(c *C) {
// Prepend a verification event into PCR7 before the EV_SEPARATOR
collector := NewRootVarsCollector(efitest.NewMockHostEnvironment(makeMockVars(c, withMsSecureBootConfig()), nil))
ctx := newMockPcrBranchContext(&mockPcrProfileContext{
Expand All @@ -319,7 +320,7 @@ func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLog2(c *C) {
c.Check(handler.MeasureImageStart(ctx), ErrorMatches, `cannot measure secure boot policy: unexpected verification event`)
}

func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLog3(c *C) {
func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLogPCR7_3(c *C) {
// Append a configuration event into PCR7 after the EV_SEPARATOR
collector := NewRootVarsCollector(efitest.NewMockHostEnvironment(makeMockVars(c, withMsSecureBootConfig()), nil))
ctx := newMockPcrBranchContext(&mockPcrProfileContext{
Expand All @@ -346,7 +347,7 @@ func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLog3(c *C) {
c.Check(handler.MeasureImageStart(ctx), ErrorMatches, `cannot measure secure boot policy: unexpected configuration event`)
}

func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLog4(c *C) {
func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLogPCR7_4(c *C) {
// Insert an unexpected event type into PCR7
collector := NewRootVarsCollector(efitest.NewMockHostEnvironment(makeMockVars(c, withMsSecureBootConfig()), nil))
ctx := newMockPcrBranchContext(&mockPcrProfileContext{
Expand All @@ -373,7 +374,7 @@ func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLog4(c *C) {
c.Check(handler.MeasureImageStart(ctx), ErrorMatches, `cannot measure secure boot policy: unexpected event type \(EV_EFI_BOOT_SERVICES_APPLICATION\) found in log`)
}

func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLog5(c *C) {
func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLogPCR0_1(c *C) {
// Insert an invalid StartupLocality event data into the log
collector := NewRootVarsCollector(efitest.NewMockHostEnvironment(nil, nil))
ctx := newMockPcrBranchContext(&mockPcrProfileContext{
Expand All @@ -395,10 +396,10 @@ func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLog5(c *C) {
}

handler := NewFwLoadHandler(log)
c.Check(handler.MeasureImageStart(ctx), ErrorMatches, `cannot measure platform firmware policy: cannot decode EV_NO_ACTION event data: cannot decode StartupLocality data: EOF`)
c.Check(handler.MeasureImageStart(ctx), ErrorMatches, `cannot measure platform firmware: cannot decode EV_NO_ACTION event data: cannot decode StartupLocality data: EOF`)
}

func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLog6(c *C) {
func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLogPCR0_2(c *C) {
// Insert an extra StartupLocality event data into the log
collector := NewRootVarsCollector(efitest.NewMockHostEnvironment(nil, nil))
ctx := newMockPcrBranchContext(&mockPcrProfileContext{
Expand All @@ -424,7 +425,133 @@ func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLog6(c *C) {
}

handler := NewFwLoadHandler(log)
c.Check(handler.MeasureImageStart(ctx), ErrorMatches, `cannot measure platform firmware policy: log for PCR0 has an unexpected StartupLocality event`)
c.Check(handler.MeasureImageStart(ctx), ErrorMatches, `cannot measure platform firmware: log for PCR0 has an unexpected StartupLocality event`)
}

func (s *fwLoadHandlerSuite) testMeasureImageStartErrBadLogSeparatorError(c *C, pcr tpm2.Handle) error {
// Insert an invalid error separator event into the log for the specified pcr
collector := NewRootVarsCollector(efitest.NewMockHostEnvironment(nil, nil))
ctx := newMockPcrBranchContext(&mockPcrProfileContext{
alg: tpm2.HashAlgorithmSHA256,
pcrs: MakePcrFlags(pcr)}, nil, collector.Next())

log := efitest.NewLog(c, &efitest.LogOptions{
Algorithms: []tpm2.HashAlgorithmId{tpm2.HashAlgorithmSHA256, tpm2.HashAlgorithmSHA1}})
for i, event := range log.Events {
if event.PCRIndex == tcglog.PCRIndex(pcr) && event.EventType == tcglog.EventTypeSeparator {
// Overwrite the event data with a mock error event
log.Events[i].Data = tcglog.NewErrorSeparatorEventData([]byte{0x50, 0x10, 0x00, 0x00})
break
}
}

handler := NewFwLoadHandler(log)
return handler.MeasureImageStart(ctx)
}

func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLogSeparatorErrorPCR0(c *C) {
err := s.testMeasureImageStartErrBadLogSeparatorError(c, 0)
c.Check(err, ErrorMatches, `cannot measure platform firmware: separator indicates that a firmware error occurred \(error code from log: 4176\)`)
}

func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLogSeparatorErrorPCR2(c *C) {
err := s.testMeasureImageStartErrBadLogSeparatorError(c, 2)
c.Check(err, ErrorMatches, `cannot measure drivers and apps: separator indicates that a firmware error occurred \(error code from log: 4176\)`)
}

func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLogSeparatorErrorPCR4(c *C) {
err := s.testMeasureImageStartErrBadLogSeparatorError(c, 4)
c.Check(err, ErrorMatches, `cannot measure boot manager code: separator indicates that a firmware error occurred \(error code from log: 4176\)`)
}

func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLogSeparatorErrorPCR7(c *C) {
err := s.testMeasureImageStartErrBadLogSeparatorError(c, 7)
c.Check(err, ErrorMatches, `cannot measure secure boot policy: separator indicates that a firmware error occurred \(error code from log: 4176\)`)
}

func (s *fwLoadHandlerSuite) testMeasureImageStartErrBadLogInvalidSeparator(c *C, pcr tpm2.Handle) error {
// Insert an invalid separator event into the log for the specified PCR
collector := NewRootVarsCollector(efitest.NewMockHostEnvironment(nil, nil))
ctx := newMockPcrBranchContext(&mockPcrProfileContext{
alg: tpm2.HashAlgorithmSHA256,
pcrs: MakePcrFlags(pcr)}, nil, collector.Next())

log := efitest.NewLog(c, &efitest.LogOptions{
Algorithms: []tpm2.HashAlgorithmId{tpm2.HashAlgorithmSHA256, tpm2.HashAlgorithmSHA1}})
for i, event := range log.Events {
if event.PCRIndex == tcglog.PCRIndex(pcr) && event.EventType == tcglog.EventTypeSeparator {
// Overwrite the event data with a mock error event
log.Events[i].Data = &mockErrLogData{errors.New("data is the wrong size")}
break
}
}

handler := NewFwLoadHandler(log)
return handler.MeasureImageStart(ctx)
}

func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLogInvalidSeparatorPCR0(c *C) {
err := s.testMeasureImageStartErrBadLogInvalidSeparator(c, 0)
c.Check(err, ErrorMatches, `cannot measure platform firmware: cannot measure invalid separator event: data is the wrong size`)
}

func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLogInvalidSeparatorPCR2(c *C) {
err := s.testMeasureImageStartErrBadLogInvalidSeparator(c, 2)
c.Check(err, ErrorMatches, `cannot measure drivers and apps: cannot measure invalid separator event: data is the wrong size`)
}

func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLogInvalidSeparatorPCR4(c *C) {
err := s.testMeasureImageStartErrBadLogInvalidSeparator(c, 4)
c.Check(err, ErrorMatches, `cannot measure boot manager code: cannot measure invalid separator event: data is the wrong size`)
}

func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLogInvalidSeparatorPCR7(c *C) {
err := s.testMeasureImageStartErrBadLogInvalidSeparator(c, 7)
c.Check(err, ErrorMatches, `cannot measure secure boot policy: cannot measure invalid separator event: data is the wrong size`)
}

func (s *fwLoadHandlerSuite) testMeasureImageStartErrBadLogMissingSeparator(c *C, pcr tpm2.Handle) error {
// Remove the separator from the specified PCR
collector := NewRootVarsCollector(efitest.NewMockHostEnvironment(nil, nil))
ctx := newMockPcrBranchContext(&mockPcrProfileContext{
alg: tpm2.HashAlgorithmSHA256,
pcrs: MakePcrFlags(pcr)}, nil, collector.Next())

log := efitest.NewLog(c, &efitest.LogOptions{
Algorithms: []tpm2.HashAlgorithmId{tpm2.HashAlgorithmSHA256, tpm2.HashAlgorithmSHA1}})
for i, event := range log.Events {
if event.PCRIndex == tcglog.PCRIndex(pcr) && event.EventType == tcglog.EventTypeSeparator {
events := log.Events[:i]
if len(log.Events) > i+1 {
events = append(events, log.Events[i+1:]...)
}
log.Events = events
break
}
}

handler := NewFwLoadHandler(log)
return handler.MeasureImageStart(ctx)
}

func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLogMissingSeparatorPCR0(c *C) {
err := s.testMeasureImageStartErrBadLogMissingSeparator(c, 0)
c.Check(err, ErrorMatches, `cannot measure platform firmware: missing separator`)
}

func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLogMissingSeparatorPCR2(c *C) {
err := s.testMeasureImageStartErrBadLogMissingSeparator(c, 2)
c.Check(err, ErrorMatches, `cannot measure drivers and apps: missing separator`)
}

func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLogMissingSeparatorPCR4(c *C) {
err := s.testMeasureImageStartErrBadLogMissingSeparator(c, 4)
c.Check(err, ErrorMatches, `cannot measure boot manager code: missing separator`)
}

func (s *fwLoadHandlerSuite) TestMeasureImageStartErrBadLogMissingSeparatorPCR7(c *C) {
err := s.testMeasureImageStartErrBadLogMissingSeparator(c, 7)
c.Check(err, ErrorMatches, `cannot measure secure boot policy: unexpected verification event`)
}

type testFwMeasureImageLoadData struct {
Expand Down

0 comments on commit a4785d8

Please sign in to comment.