Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion efi/preinstall/check_pcr4.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ NextEvent:
}

switch phase {
case tcglogPhaseFirmwareLaunch, tcglogPhasePreOSThirdPartyDispatch, tcglogPhasePreOSThirdPartyDispatchUnterminated:
case tcglogPhaseFirmwareLaunch, tcglogPhasePreOSThirdPartyDispatch:
if ev.PCRIndex != internal_efi.BootManagerCodePCR {
// Not PCR4
continue NextEvent
Expand Down
97 changes: 57 additions & 40 deletions efi/preinstall/check_pcr7.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ type secureBootPolicyResult struct {
// corresponds to the initial boot loader image for the current boot. This is used to detect the
// launch of the OS, at which checks for PCR7 end. There are some limitations of this, ie, we may
// not detect LoadImage bugs that happen later on, but once the OS has loaded, it's impossible to
// tell whicj events come from firmware and which are under the control of OS components.
// tell which events come from firmware and which are under the control of OS components.
//
// This ensures that secure boot is enabled, else an error is returned, as WithSecureBootPolicyProfile
// only generates profiles compatible with secure boot being enabled.
Expand Down Expand Up @@ -461,12 +461,51 @@ NextEvent:
}

switch phase {
case tcglogPhaseMeasuringSecureBootConfig:
case tcglogPhaseFirmwareLaunch:
if ev.PCRIndex != internal_efi.SecureBootPolicyPCR {
// Not PCR7
continue NextEvent
}

switch ev.EventType {
case tcglog.EventTypeEFIAction:
// An EV_EFI_ACTION event measured to PCR7 may indicate some degraded condition
// that weakens device security. 2 known ones are:
// - "UEFI Debug Mode", which indicates the presence of a debugging endpoint.
// The TCG PC Client PFP spec says this goes before the secure boot config
// is measured.
// - "DMA Protection Disabled" to indicate that pre-boot DMA protection was
// disabled. This event isn't formally documented anywhere - it is mentioned
// in some tianocore documentation, although EDK2 doesn't have a reference
// implementation. This event can be permitted, which means that we can
// generate a policy that includes it. However, the tianocore documentation
// doesn't specify event ordering, so we need to accommodate any possible
// ordering of events.
//
// The presence of an EV_EFI_ACTION event other than "DMA Protection Disabled"
// will result in WithSecureBootPolicyProfile() creating an invalid policy,
// because it generally doesn't emit these measurements. Just return an error
// here to prevent the use of WithSecureBootPolicyProfile() unless it is a
// "DMA Protection Disabled" event and it is permitted.
//
// Note that "UEFI Debug Mode" and "DMA Protection Disabled" events are both
// caught by the host security checks, which run before this.
if permitDMAProtectionDisabledEvent && (bytes.Equal(ev.Data.Bytes(), []byte(tcglog.DMAProtectionDisabled)) ||
bytes.Equal(ev.Data.Bytes(), append([]byte(tcglog.DMAProtectionDisabled), 0x00))) {
// This event is detected by the host security checks which will result in a flag
// being added to the results so that it can be picked up by the code in
// profile. We don't need to do anything else here other than make sure
// this event only appears once.
permitDMAProtectionDisabledEvent = false // Don't allow this more than once.
continue NextEvent
}
fallthrough
default:
// Anything that isn't EV_EFI_ACTION ends up here.
return nil, fmt.Errorf("unexpected %v event %q before config", ev.EventType, ev.Data)
}
case tcglogPhaseMeasuringSecureBootConfig:
Comment thread
pedronis marked this conversation as resolved.
Comment thread
pedronis marked this conversation as resolved.
// ev.PCRIndex is always SecureBootPolicyPCR in this phase.
switch ev.EventType {
case tcglog.EventTypeEFIVariableDriverConfig:
if len(configs) == 0 {
Expand Down Expand Up @@ -518,27 +557,16 @@ NextEvent:
db = sigDb
}
case tcglog.EventTypeEFIAction:
// An EV_EFI_ACTION events with the string "UEFI Debug Mode" appears at the
// start of the log if a debugging endpoint is enabled. It's also possible that
// EV_EFI_ACTION events are used for other conditions in PCR7 that weaken device
// security (eg, the "DMA Protection Disabled" event).
//
// In general, it's not normal to see EV_EFI_ACTION events and these indicate some
// sort of abnormal condition that has a detrimental effect on device security.
// WithSecureBootPolicyProfile() will generate an invalid policy in this case because,
// with some exceptions, it doesn't emit them.
//
// Just return an error here to prevent the use of WithSecureBootPolicyProfile(). The
// "UEFI Debug Mode" and "DMA Protection Disabled" cases are already picked up by the
// firmware protection checks, so we don't need any special handling here.
//
// We do permit the "DMA Protection Disabled" case if required. In this case,
// WithSecureBootPolicyProfile() needs a separate option.
// See the doc notes for this event type in the tcglogPhaseFirmwareLaunch
// phase. This leg is here to accommodate a "DMA Protection Disabled" event
// as part of the secure boot configuration, just at the end of the signature
// database measurements.
if permitDMAProtectionDisabledEvent && (bytes.Equal(ev.Data.Bytes(), []byte(tcglog.DMAProtectionDisabled)) ||
bytes.Equal(ev.Data.Bytes(), append([]byte(tcglog.DMAProtectionDisabled), 0x00))) {
// This event is detected by the host security checks so we can skip it here.
// We'll emit a flag in the results which is picked up by the code in profile.go
// to add an option to permit this with WithSecureBootPolicyProfile().
// This event is detected by the host security checks which will result in a flag
// being added to the results so that it can be picked up by the code in
// profile. We don't need to do anything else here other than make sure
// this event only appears once.
permitDMAProtectionDisabledEvent = false // Don't allow this more than once.
continue NextEvent
}
Expand Down Expand Up @@ -594,23 +622,16 @@ NextEvent:
case tcglog.EventTypeSeparator:
// ok
case tcglog.EventTypeEFIAction:
// In general, it's not normal to see EV_EFI_ACTION events and these indicate some
// sort of abnormal condition that has a detrimental effect on device security.
// WithSecureBootPolicyProfile() will generate an invalid policy in this case because,
// with some exceptions, it doesn't emit them.
//
// Just return an error here to prevent the use of WithSecureBootPolicyProfile(). The
// "UEFI Debug Mode" and "DMA Protection Disabled" cases are already picked up by the
// firmware protection checks, so we don't need any special handling here.
//
// We do permit the "DMA Protection Disabled" case if required. In this case,
// WithSecureBootPolicyProfile() needs a separate option. Some firmware measures
// this after the EV_SEPARATOR in PCR7 but part of the pre-OS environment.
// See the doc notes for this event type in the tcglogPhaseFirmwareLaunch
// phase. This leg is here to accommodate a "DMA Protection Disabled" event
// as part of the secure boot configuration, just at the end of the signature
// database measurements.
if permitDMAProtectionDisabledEvent && (bytes.Equal(ev.Data.Bytes(), []byte(tcglog.DMAProtectionDisabled)) ||
bytes.Equal(ev.Data.Bytes(), append([]byte(tcglog.DMAProtectionDisabled), 0x00))) {
// This event is detected by the host security checks so we can skip it here.
// We'll emit a flag in the results which is picked up by the code in profile.go
// to add an option to permit this with WithSecureBootPolicyProfile().
// This event is detected by the host security checks which will result in a flag
// being added to the results so that it can be picked up by the code in
// profile. We don't need to do anything else here other than make sure
// this event only appears once.
permitDMAProtectionDisabledEvent = false // Don't allow this more than once.
continue NextEvent
}
Expand Down Expand Up @@ -723,10 +744,6 @@ NextEvent:
// Anything that isn't EV_EFI_VARIABLE_AUTHORITY ends up here.
return nil, fmt.Errorf("unexpected %v event %q whilst measuring verification", ev.EventType, ev.Data)
}
case tcglogPhasePreOSThirdPartyDispatchUnterminated:
Comment thread
pedronis marked this conversation as resolved.
if ev.PCRIndex == internal_efi.SecureBootPolicyPCR {
return nil, fmt.Errorf("unexpected %v event in PCR7 after measuring config but before transitioning to OS-present", ev.EventType)
}
}
}

Expand Down
122 changes: 120 additions & 2 deletions efi/preinstall/check_pcr7_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,67 @@ func (s *pcr7Suite) TestCheckSecureBootPolicyMeasurementsAndObtainAuthoritiesGoo
c.Check(err, IsNil)
}

func (s *pcr7Suite) TestCheckSecureBootPolicyMeasurementsAndObtainAuthoritiesGoodSecureBootSeparatorAfterPreOS(c *C) {
err := s.testCheckSecureBootPolicyMeasurementsAndObtainAuthorities(c, &testCheckSecureBootPolicyMeasurementsAndObtainAuthoritiesParams{
env: efitest.NewMockHostEnvironmentWithOpts(
efitest.WithMockVars(efitest.MockVars{
{Name: "AuditMode", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeNonVolatile | efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x0}},
{Name: "BootCurrent", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x3, 0x0}},
{Name: "BootOptionSupport", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x13, 0x03, 0x00, 0x00}},
{Name: "DeployedMode", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeNonVolatile | efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x1}},
{Name: "SetupMode", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x0}},
{Name: "OsIndicationsSupported", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x41, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}},
}.SetSecureBoot(true).SetPK(c, efitest.NewSignatureListX509(c, snakeoilCert, efi.MakeGUID(0x03f66fa4, 0x5eee, 0x479c, 0xa408, [...]uint8{0xc4, 0xdc, 0x0a, 0x33, 0xfc, 0xde})))),
efitest.WithLog(efitest.NewLog(c, &efitest.LogOptions{
Algorithms: []tpm2.HashAlgorithmId{tpm2.HashAlgorithmSHA256},
SecureBootSeparatorOrder: efitest.SecureBootSeparatorAfterPreOS,
})),
),
pcrAlg: tpm2.HashAlgorithmSHA256,
iblImage: &mockImage{
signatures: []*efi.WinCertificateAuthenticode{
efitest.ReadWinCertificateAuthenticodeDetached(c, shimUbuntuSig4),
},
},
expectedFlags: SecureBootPolicyResultFlags(0),
expectedUsedAuthorities: []*X509CertificateID{
NewX509CertificateID(testutil.ParseCertificate(c, msUefiCACert)),
},
})
c.Check(err, IsNil)
}

func (s *pcr7Suite) TestCheckSecureBootPolicyMeasurementsAndObtainAuthoritiesGoodWithDriverLaunchAndSecureBootSeparatorAfterPreOS(c *C) {
err := s.testCheckSecureBootPolicyMeasurementsAndObtainAuthorities(c, &testCheckSecureBootPolicyMeasurementsAndObtainAuthoritiesParams{
env: efitest.NewMockHostEnvironmentWithOpts(
efitest.WithMockVars(efitest.MockVars{
{Name: "AuditMode", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeNonVolatile | efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x0}},
{Name: "BootCurrent", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x3, 0x0}},
{Name: "BootOptionSupport", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x13, 0x03, 0x00, 0x00}},
{Name: "DeployedMode", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeNonVolatile | efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x1}},
{Name: "SetupMode", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x0}},
{Name: "OsIndicationsSupported", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x41, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}},
}.SetSecureBoot(true).SetPK(c, efitest.NewSignatureListX509(c, snakeoilCert, efi.MakeGUID(0x03f66fa4, 0x5eee, 0x479c, 0xa408, [...]uint8{0xc4, 0xdc, 0x0a, 0x33, 0xfc, 0xde})))),
efitest.WithLog(efitest.NewLog(c, &efitest.LogOptions{
Algorithms: []tpm2.HashAlgorithmId{tpm2.HashAlgorithmSHA256},
SecureBootSeparatorOrder: efitest.SecureBootSeparatorAfterPreOS,
IncludeDriverLaunch: true,
})),
),
pcrAlg: tpm2.HashAlgorithmSHA256,
iblImage: &mockImage{
signatures: []*efi.WinCertificateAuthenticode{
efitest.ReadWinCertificateAuthenticodeDetached(c, shimUbuntuSig4),
},
},
expectedFlags: SecureBootPolicyResultFlags(0),
expectedUsedAuthorities: []*X509CertificateID{
NewX509CertificateID(testutil.ParseCertificate(c, msUefiCACert)),
},
})
c.Check(err, IsNil)
}

func (s *pcr7Suite) TestCheckSecureBootPolicyMeasurementsAndObtainAuthoritiesGoodWithSysPrepLaunch(c *C) {
err := s.testCheckSecureBootPolicyMeasurementsAndObtainAuthorities(c, &testCheckSecureBootPolicyMeasurementsAndObtainAuthoritiesParams{
env: efitest.NewMockHostEnvironmentWithOpts(
Expand Down Expand Up @@ -360,6 +421,37 @@ func (s *pcr7Suite) TestCheckSecureBootPolicyMeasurementsAndObtainAuthoritiesWit
c.Check(err, IsNil)
}

func (s *pcr7Suite) TestCheckSecureBootPolicyMeasurementsAndObtainAuthoritiesWithPermittedDMAProtectionDisabledNullTerminated(c *C) {
err := s.testCheckSecureBootPolicyMeasurementsAndObtainAuthorities(c, &testCheckSecureBootPolicyMeasurementsAndObtainAuthoritiesParams{
env: efitest.NewMockHostEnvironmentWithOpts(
efitest.WithMockVars(efitest.MockVars{
{Name: "AuditMode", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeNonVolatile | efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x0}},
{Name: "BootCurrent", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x3, 0x0}},
{Name: "BootOptionSupport", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x13, 0x03, 0x00, 0x00}},
{Name: "DeployedMode", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeNonVolatile | efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x1}},
{Name: "SetupMode", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x0}},
{Name: "OsIndicationsSupported", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x41, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}},
}.SetSecureBoot(true).SetPK(c, efitest.NewSignatureListX509(c, snakeoilCert, efi.MakeGUID(0x03f66fa4, 0x5eee, 0x479c, 0xa408, [...]uint8{0xc4, 0xdc, 0x0a, 0x33, 0xfc, 0xde})))),
efitest.WithLog(efitest.NewLog(c, &efitest.LogOptions{
Algorithms: []tpm2.HashAlgorithmId{tpm2.HashAlgorithmSHA256},
DMAProtection: efitest.DMAProtectionDisabled | efitest.DMAProtectionDisabledEventNullTerminated,
})),
),
pcrAlg: tpm2.HashAlgorithmSHA256,
iblImage: &mockImage{
signatures: []*efi.WinCertificateAuthenticode{
efitest.ReadWinCertificateAuthenticodeDetached(c, shimUbuntuSig4),
},
},
permitDMAProtectionDisabled: true,
expectedFlags: SecureBootPolicyResultFlags(0),
expectedUsedAuthorities: []*X509CertificateID{
NewX509CertificateID(testutil.ParseCertificate(c, msUefiCACert)),
},
})
c.Check(err, IsNil)
}

func (s *pcr7Suite) TestCheckSecureBootPolicyMeasurementsAndObtainAuthoritiesWithPermittedDMAProtectionDisabledDifferentLocation(c *C) {
err := s.testCheckSecureBootPolicyMeasurementsAndObtainAuthorities(c, &testCheckSecureBootPolicyMeasurementsAndObtainAuthoritiesParams{
env: efitest.NewMockHostEnvironmentWithOpts(
Expand Down Expand Up @@ -761,7 +853,7 @@ func (s *pcr7Suite) TestCheckSecureBootPolicyMeasurementsAndObtainAuthoritiesBad
},
},
})
c.Check(err, ErrorMatches, `unexpected EV_EFI_ACTION event \"UEFI Debug Mode\" whilst measuring config`)
c.Check(err, ErrorMatches, `unexpected EV_EFI_ACTION event \"UEFI Debug Mode\" before config`)
}

func (s *pcr7Suite) TestCheckSecureBootPolicyMeasurementsAndObtainAuthoritiesBadMissingConfigMeasurement(c *C) {
Expand Down Expand Up @@ -1111,10 +1203,36 @@ func (s *pcr7Suite) TestCheckSecureBootPolicyMeasurementsAndObtainAuthoritiesBad
},
},
})
c.Check(err, ErrorMatches, `unexpected EV_EFI_ACTION event \"DMA Protection Disabled\" whilst measuring config`)
c.Check(err, ErrorMatches, `unexpected EV_EFI_ACTION event \"DMA Protection Disabled\" before config`)
}

func (s *pcr7Suite) TestCheckSecureBootPolicyMeasurementsAndObtainAuthoritiesBadDMAProtectionDisabledBeforeVerification(c *C) {
err := s.testCheckSecureBootPolicyMeasurementsAndObtainAuthorities(c, &testCheckSecureBootPolicyMeasurementsAndObtainAuthoritiesParams{
env: efitest.NewMockHostEnvironmentWithOpts(
efitest.WithMockVars(efitest.MockVars{
{Name: "AuditMode", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeNonVolatile | efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x0}},
{Name: "BootCurrent", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x3, 0x0}},
{Name: "BootOptionSupport", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x13, 0x03, 0x00, 0x00}},
{Name: "DeployedMode", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeNonVolatile | efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x1}},
{Name: "SetupMode", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x0}},
{Name: "OsIndicationsSupported", GUID: efi.GlobalVariable}: &efitest.VarEntry{Attrs: efi.AttributeBootserviceAccess | efi.AttributeRuntimeAccess, Payload: []byte{0x41, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}},
}.SetSecureBoot(true).SetPK(c, efitest.NewSignatureListX509(c, snakeoilCert, efi.MakeGUID(0x03f66fa4, 0x5eee, 0x479c, 0xa408, [...]uint8{0xc4, 0xdc, 0x0a, 0x33, 0xfc, 0xde})))),
efitest.WithLog(efitest.NewLog(c, &efitest.LogOptions{
Algorithms: []tpm2.HashAlgorithmId{tpm2.HashAlgorithmSHA256},
DMAProtection: efitest.DMAProtectionDisabled,
})),
),
pcrAlg: tpm2.HashAlgorithmSHA256,
iblImage: &mockImage{
signatures: []*efi.WinCertificateAuthenticode{
efitest.ReadWinCertificateAuthenticodeDetached(c, shimUbuntuSig4),
},
},
})
c.Check(err, ErrorMatches, `unexpected EV_EFI_ACTION event \"DMA Protection Disabled\" whilst measuring config`)
}

func (s *pcr7Suite) TestCheckSecureBootPolicyMeasurementsAndObtainAuthoritiesBadDMAProtectionDisabledAfterSeparator(c *C) {
err := s.testCheckSecureBootPolicyMeasurementsAndObtainAuthorities(c, &testCheckSecureBootPolicyMeasurementsAndObtainAuthoritiesParams{
env: efitest.NewMockHostEnvironmentWithOpts(
efitest.WithMockVars(efitest.MockVars{
Expand Down
Loading
Loading