AddPCRProfile: add WithAllowSecurityLevelDowngraded - #555
AddPCRProfile: add WithAllowSecurityLevelDowngraded#555frederic-hoerni wants to merge 5 commits into
Conversation
c315d4d to
5418be0
Compare
valentindavid
left a comment
valentindavid
left a comment
There was a problem hiding this comment.
Looks fine. Though I think it is mutually exclusive with "DMA Protection Disabled". But this is merge-able as is.
| if includeThunderboltSecurityLevel0 { | ||
| ctx.ExtendPCR(internal_efi.SecureBootPolicyPCR, e.Digests[ctx.PCRAlg()]) | ||
| } | ||
| allowThunderboltSecurityLevel0 = false // Only allow this event to appear once |
There was a problem hiding this comment.
I think we might be able to set allowInsufficientDMAProtection to false here. And also set allowThunderboltSecurityLevel0 to false in previous case. I think those 2 measurements (dma protection disabled and security level downgraded) are mutually exclusive, right?
There was a problem hiding this comment.
does this change need a test?
There was a problem hiding this comment.
I will add a test for the "mutually exclusive" part.
There was a problem hiding this comment.
test added: TestMeasureImageStartErrAllowThunderboltSecurityLevel0WithInsufficientDMA
| StartupLocality uint8 // specify a startup locality other than 0 | ||
| FirmwareDebugger bool // indicate a firmware debugger endpoint is enabled | ||
| DMAProtection DMAProtectionDisabledEventFlags // whether DMA protection is disabled | ||
| ThunderboltSecurityLevel0 ThunderboltSecurityLevel0EventFlags // whether security level is downgraded |
There was a problem hiding this comment.
changed to a bool.
| ), | ||
| ), | ||
| expected: []tpm2.PCRValues{ | ||
| // Not including DMA protection string // TODO |
We could. But given this is only very old firmware, maybe we should not do it. That probably will not add that many devices. If that opened for many devices that would be a bit different. |
This adds an option WithAllowThunderboltSecurityLevel0() to allow a measurement event EV_EFI_ACTION "Security Level is Downgraded to 0", which relates to the Thunderbolt security level. Some BIOS firmware on NUC8v5PNB devices measure this event and that breaks the computation of the PCR policy, with this error: cannot measure secure boot policy: unexpected event type (EV_EFI_ACTION) found in log This can be fixed on those devices either by configuring the BIOS to the most secure option for Thunderbolt, or by updating the firmware. Having a Thunderbolt security level of zero means that in some situations a malicious Thunderbolt device could access potentially sensitive data in your system’s memory. This is why this case is not supported by default. However, in some cases, users cannot configure nor update the firmware and need this option.
5418be0 to
c19422a
Compare
|
I took into account the remarks of the review. I will squash the related commits before merge. |
| if includeThunderboltSecurityLevel0 { | ||
| ctx.ExtendPCR(internal_efi.SecureBootPolicyPCR, e.Digests[ctx.PCRAlg()]) | ||
| } | ||
| allowThunderboltSecurityLevel0 = false // Only allow this event to appear once |
There was a problem hiding this comment.
does this change need a test?
3 participants
No description provided.