Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BG suite multierror support #362

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

orangecms
Copy link
Contributor

Signed-off-by: Daniel Maslowski info@orangecms.org

Copy link
Collaborator

@ChriMarMe ChriMarMe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of multierror.

Comment on lines 29 to 32
Name: "SACM meets sane BootGuard requirements",
Required: true,
function: BootGuardACM,
dependencies: []*Test{&testbootguardfit},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the alignemet before the change more :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comes from the Go formatter, which my IDE ran automatically... 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realized that this happened due to the comments. So, I just removed them. :)

pkg/test/bootguard_tests.go Outdated Show resolved Hide resolved
pkg/test/bootguard_tests.go Outdated Show resolved Hide resolved
pkg/test/bootguard_tests.go Outdated Show resolved Hide resolved
@orangecms
Copy link
Contributor Author

So I checked because I noticed that go-multierror was already in the dependencies...
https://www.libhunt.com/compare-multierr-vs-go-multierror

Which led me to this: Apparently, Go 1.20 added native multi error support.
https://www.reddit.com/r/golang/comments/z870te/multiple_error_wrapping_is_coming_in_go_120/

Anyway, I think introducing the Uber package is fine here, and we can rework things nicely again later.

@orangecms
Copy link
Contributor Author

orangecms commented Jan 30, 2024

And the CI error basically says that we should upgrade to Go 1.19 at least (I would go straight to 1.21).

# go.uber.org/multierr
/go/pkg/mod/go.uber.org/multierr@v1.11.0/error.go:209:20: undefined: atomic.Bool
note: module requires Go 1.19

Exited with code exit status 2

Huh, we do have 1.19 though... no idea what's wrong here... sigh

Edit/addendum: sooooo... since this uses atomics and Go doesn't offer them on all architectures we use, I'll see about using github.com/hashicorp/go-multierror here instead; hope that has everything necessary 😬

@orangecms orangecms changed the title WIP: BG suite multierror output BG suite multierror support Jan 30, 2024
@ChriMarMe
Copy link
Collaborator

ChriMarMe commented Jan 30, 2024

❤️

@@ -29,7 +30,6 @@ var (
Name: "SACM meets sane BootGuard requirements",
Required: true,
function: BootGuardACM,
dependencies: []*Test{&testbootguardfit},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Why remove the dependency?
  2. Why not in a seperate commit?

@orangecms
Copy link
Contributor Author

The dependency removal is the actual feature here; this before vs after should clarify it:

before

bg-suite exec-tests -f ~/firmware/MSI/Cyborg\ 15\ A12VE/E15K1IMS.30B/E15K1IMS.30B
For more information about the documents and chapters, run: bg-suite -m

All tests
_________
00 - FIT meets BootGuard requirements        : FAIL                 (couldn't find any BIOS Startup Module Entry in FIT)
01 - SACM meets sane BootGuard requirements  : DEPENDENCY_FAILED    (FIT meets BootGuard requirements failed)
02 - Key Manifest meets sane BootGuard requirements: DEPENDENCY_FAILED    (FIT meets BootGuard requirements failed)
03 - Boot Policy Manifest meets sane BootGuard requirements: DEPENDENCY_FAILED    (FIT meets BootGuard requirements failed)
04 - Verifies BPM and IBBs match firmware image: DEPENDENCY_FAILED    (FIT meets BootGuard requirements failed)
05 - [RUNTIME] Validates Intel ME specific configuration against KM/BPM in firmware image: DEPENDENCY_FAILED    (FIT meets BootGuard requirements failed)
06 - [RUNTIME] Verifies Intel ME Boot Guard configuration is sane and safe: FAIL                 (couldn't read HFSTS6: couldn't read HFSTS6 from PCI config space: couldn't find Intel ME device for runtime checks)
07 - [RUNTIME] BtG/TXT registers are sane    : FAIL                 (txt regs aren't valid: couldn't fetch TXT regs: open /dev/mem: permission denied)

after

bg-suite exec-tests -f "/home/dama/firmware/MSI/Cyborg 15 A12VE/E15K1IMS.30B/E15K1IMS.30B"
For more information about the documents and chapters, run: bg-suite -m

All tests
_________
00 - FIT meets BootGuard requirements        : FAIL                 (couldn't find any BIOS Startup Module Entry in FIT)
01 - SACM meets sane BootGuard requirements  : PASS
02 - Key Manifest meets sane BootGuard requirements: PASS
03 - Boot Policy Manifest meets sane BootGuard requirements: FAIL                 (
 multiple errors:
 bpm hasn't sane security properties: no BootGuard Boot Policy Manifest found
)
04 - Verifies BPM and IBBs match firmware image: PASS
05 - [RUNTIME] Validates Intel ME specific configuration against KM/BPM in firmware image: DEPENDENCY_FAILED    (FIT meets BootGuard requirements failed)
06 - [RUNTIME] Verifies Intel ME Boot Guard configuration is sane and safe: FAIL                 (couldn't read HFSTS6: couldn't read HFSTS6 from PCI config space: couldn't find Intel ME device for runtime checks)
07 - [RUNTIME] BtG/TXT registers are sane    : FAIL                 (
 txt regs aren't valid:
 couldn't fetch TXT regs: open /dev/mem: permission denied
)

@orangecms
Copy link
Contributor Author

grammar fixed:

bg-suite exec-tests -f "/home/dama/firmware/MSI/Cyborg 15 A12VE/E15K1IMS.30B/E15K1IMS.30B"
For more information about the documents and chapters, run: bg-suite -m

All tests
_________
00 - FIT meets BootGuard requirements        : FAIL                 (couldn't find any BIOS Startup Module Entry in FIT)
01 - SACM meets sane BootGuard requirements  : PASS
02 - Key Manifest meets sane BootGuard requirements: PASS
03 - Boot Policy Manifest meets sane BootGuard requirements: FAIL                 (
 Errors occurred:
 Boot Policy Manifest doesn't have sane security properties: no BootGuard Boot Policy Manifest found
)
04 - Verifies BPM and IBBs match firmware image: PASS
05 - [RUNTIME] Validates Intel ME specific configuration against KM/BPM in firmware image: DEPENDENCY_FAILED    (FIT meets BootGuard requirements failed)
06 - [RUNTIME] Verifies Intel ME Boot Guard configuration is sane and safe: FAIL                 (couldn't read HFSTS6: couldn't read HFSTS6 from PCI config space: couldn't find Intel ME device for runtime checks)
07 - [RUNTIME] BtG/TXT registers are sane    : FAIL                 (
 txt regs aren't valid:
 couldn't fetch TXT regs: open /dev/mem: permission denied
)

This also rewords many error messages to make them consistent:
- capitalize BootGuard, Key Manifest and Boot Policy Manifest
- spell out Key Manifest and Boot Policy Manifest
- uppercase acronyms such as DMA etc

Signed-off-by: Daniel Maslowski <info@orangecms.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants