From 46b281f1cc0ad60234b71842519cd40cb64dd41a Mon Sep 17 00:00:00 2001 From: Daniel Maslowski Date: Sun, 21 May 2023 23:24:06 +0200 Subject: [PATCH] BG suite: add multierror output with multiline print 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 --- cmd/core/bg-suite/cmd.go | 2 +- go.mod | 2 +- go.sum | 4 +- pkg/provisioning/bootguard/bootguard.go | 147 ++++++++++++++---------- pkg/test/bootguard_tests.go | 21 ++-- pkg/test/test.go | 2 +- 6 files changed, 102 insertions(+), 76 deletions(-) diff --git a/cmd/core/bg-suite/cmd.go b/cmd/core/bg-suite/cmd.go index 9a943c4a..a0880d0d 100644 --- a/cmd/core/bg-suite/cmd.go +++ b/cmd/core/bg-suite/cmd.go @@ -51,7 +51,7 @@ func (e *execTestsCmd) Run(ctx *context) error { ret := false data, err := os.ReadFile(e.Firmware) if err != nil { - return fmt.Errorf("can't read firmware file") + return fmt.Errorf("can't read firmware file %v", e.Firmware) } preset := test.PreSet{ Firmware: data, diff --git a/go.mod b/go.mod index 7e882d99..20fb47f6 100644 --- a/go.mod +++ b/go.mod @@ -26,12 +26,12 @@ require ( github.com/sirupsen/logrus v1.9.3 github.com/steakknife/hamming v0.0.0-20180906055917-c99c65617cd3 github.com/stretchr/testify v1.8.4 - github.com/tidwall/pretty v1.2.1 github.com/ulikunitz/xz v0.5.11 github.com/xaionaro-facebook/go-dmidecode v0.0.0-20220413144237-c42d5bef2498 github.com/xaionaro-go/bytesextra v0.0.0-20220103144954-846e454ddea9 github.com/xaionaro-go/unhash v0.0.0-20230427202706-0195a574c620 github.com/xaionaro-go/unsafetools v0.0.0-20210722164218-75ba48cf7b3c + go.uber.org/multierr v1.11.0 golang.org/x/exp v0.0.0-20230626212559-97b1e661b5df golang.org/x/term v0.15.0 gopkg.in/yaml.v3 v3.0.1 diff --git a/go.sum b/go.sum index 0d156bc6..9db03e3a 100644 --- a/go.sum +++ b/go.sum @@ -777,8 +777,6 @@ github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw= -github.com/tidwall/pretty v1.2.1 h1:qjsOFOWWQl+N3RsoF5/ssm1pHmJJwhjlSbZ51I6wMl4= -github.com/tidwall/pretty v1.2.1/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= github.com/tj/assert v0.0.0-20171129193455-018094318fb0/go.mod h1:mZ9/Rh9oLWpLLDRpvE+3b7gP/C2YyLFYxNmcLnPTMe0= github.com/tj/go-elastic v0.0.0-20171221160941-36157cbbebc2/go.mod h1:WjeM0Oo1eNAjXGDx2yma7uG2XoyRZTq1uv3M/o7imD0= github.com/tj/go-kinesis v0.0.0-20171128231115-08b17f58cb1b/go.mod h1:/yhzCV0xPfx6jb1bBgRFjl5lytqVqZXEaeqWP8lTEao= @@ -893,6 +891,8 @@ go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/ go.uber.org/multierr v1.3.0/go.mod h1:VgVr7evmIr6uPjLBxg28wmKNXyqE9akIJ5XnfpiKl+4= go.uber.org/multierr v1.5.0/go.mod h1:FeouvMocqHpRaaGuG9EjoKcStLC43Zu/fmqdUMPcKYU= go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= +go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= +go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= go.uber.org/tools v0.0.0-20190618225709-2cfd321de3ee/go.mod h1:vJERXedbb3MVM5f9Ejo0C68/HhF8uaILCdgjnY+goOA= go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q= go.uber.org/zap v1.13.0/go.mod h1:zwrFLgMcdUuIBviXEYEH1YKNaOBnKXsx2IPda5bBwHM= diff --git a/pkg/provisioning/bootguard/bootguard.go b/pkg/provisioning/bootguard/bootguard.go index c8f6484c..7127ece5 100644 --- a/pkg/provisioning/bootguard/bootguard.go +++ b/pkg/provisioning/bootguard/bootguard.go @@ -24,6 +24,7 @@ import ( "github.com/tidwall/pretty" log "github.com/sirupsen/logrus" + "go.uber.org/multierr" ) // Everything more secure than SHA-1 @@ -97,7 +98,7 @@ func NewVData(vdata VersionedData) (*BootGuard, error) { b.Version, _ = bgheader.DetectBGV(manifest) } if b.Version == 0 { - return nil, fmt.Errorf("NewVData: can't identify bootguard header") + return nil, fmt.Errorf("NewVData: can't identify BootGuard header") } b.VData = vdata return &b, nil @@ -127,7 +128,7 @@ func NewBPM(bpm io.ReadSeeker) (*BootGuard, error) { return nil, err } default: - return nil, fmt.Errorf("NewBPM: can't identify bootguard header") + return nil, fmt.Errorf("NewBPM: can't identify BootGuard header") } return &b, nil } @@ -156,7 +157,7 @@ func NewKM(km io.ReadSeeker) (*BootGuard, error) { return nil, err } default: - return nil, fmt.Errorf("NewKM: can't identify bootguard header") + return nil, fmt.Errorf("NewKM: can't identify BootGuard header") } return &b, nil } @@ -176,7 +177,7 @@ func NewBPMAndKM(bpm io.ReadSeeker, km io.ReadSeeker) (*BootGuard, error) { return nil, err } if bpmV != kmV { - return nil, fmt.Errorf("km and bpm version number differ") + return nil, fmt.Errorf("Key Manifest and Boot Policy Manifest version number differ") } b.Version = bpmV switch b.Version { @@ -203,7 +204,7 @@ func NewBPMAndKM(bpm io.ReadSeeker, km io.ReadSeeker) (*BootGuard, error) { return nil, err } default: - return nil, fmt.Errorf("NewBPMAndKM: can't identify bootguard header") + return nil, fmt.Errorf("NewBPMAndKM: can't identify BootGuard header") } return &b, nil } @@ -246,7 +247,7 @@ func NewBPMAndKMFromBIOS(biosFilepath string, jsonFilepath *os.File) (*BootGuard return nil, err } default: - return nil, fmt.Errorf("NewBPMAndKMFromBIOS: can't identify bootguard header") + return nil, fmt.Errorf("NewBPMAndKMFromBIOS: can't identify BootGuard header") } data, err := json.Marshal(b.VData) if err != nil { @@ -268,11 +269,11 @@ func (b *BootGuard) ValidateBPM() error { case bgheader.Version20: return b.VData.CBNTbpm.Validate() default: - return fmt.Errorf("ValidateBPM: can't identify bootguard header") + return fmt.Errorf("ValidateBPM: can't identify BootGuard header") } } -// ValidateKM reads from a binary source, parses into the key manifest structure +// ValidateKM reads from a binary source, parses into the Key Manifest structure // and validates the structure func (b *BootGuard) ValidateKM() error { switch b.Version { @@ -281,7 +282,7 @@ func (b *BootGuard) ValidateKM() error { case bgheader.Version20: return b.VData.CBNTkm.Validate() default: - return fmt.Errorf("ValidateKM: can't identify bootguard header") + return fmt.Errorf("ValidateKM: can't identify BootGuard header") } } @@ -293,11 +294,11 @@ func (b *BootGuard) PrintBPM() { case bgheader.Version20: b.VData.CBNTbpm.Print() default: - log.Error("PrintBPM: can't identify bootguard header") + log.Error("PrintBPM: can't identify BootGuard header") } } -// PrintKM prints the key manifest in human readable +// PrintKM prints the Key Manifest in human readable func (b *BootGuard) PrintKM() { switch b.Version { case bgheader.Version10: @@ -305,11 +306,10 @@ func (b *BootGuard) PrintKM() { case bgheader.Version20: b.VData.CBNTkm.Print() default: - log.Error("PrintKM: can't identify bootguard header") - } + log.Error("PrintKM: can't identify BootGuard header") } -// WriteKM returns a key manifest as bytes in format defined in #575623. +// WriteKM returns a Key Manifest as bytes in format defined in #575623. func (b *BootGuard) WriteKM() ([]byte, error) { var err error buf := new(bytes.Buffer) @@ -319,7 +319,7 @@ func (b *BootGuard) WriteKM() ([]byte, error) { case bgheader.Version20: _, err = b.VData.CBNTkm.WriteTo(buf) default: - log.Error("WriteKM: can't identify bootguard header") + log.Error("WriteKM: can't identify BootGuard header") } return buf.Bytes(), err } @@ -334,7 +334,7 @@ func (b *BootGuard) WriteBPM() ([]byte, error) { case bgheader.Version20: _, err = b.VData.CBNTbpm.WriteTo(buf) default: - log.Error("WriteBPM: can't identify bootguard header") + log.Error("WriteBPM: can't identify BootGuard header") } return buf.Bytes(), err } @@ -364,7 +364,7 @@ func (b *BootGuard) ReadJSON(filepath string) error { return nil } -// StitchKM returns a key manifest manifest as byte slice +// StitchKM returns a Key Manifest as byte slice func (b *BootGuard) StitchKM(pubKey crypto.PublicKey, signature []byte) ([]byte, error) { switch b.Version { case bgheader.Version10: @@ -384,7 +384,7 @@ func (b *BootGuard) StitchKM(pubKey crypto.PublicKey, signature []byte) ([]byte, return nil, err } default: - log.Error("StitchKM: can't identify bootguard header") + log.Error("StitchKM: can't identify BootGuard header") } return b.WriteKM() } @@ -413,7 +413,7 @@ func (b *BootGuard) StitchBPM(pubKey crypto.PublicKey, signature []byte) ([]byte return nil, err } default: - log.Error("StitchBPM: can't identify bootguard header") + log.Error("StitchBPM: can't identify BootGuard header") } return b.WriteBPM() } @@ -451,7 +451,7 @@ func (b *BootGuard) SignKM(signAlgo string, privkey crypto.PrivateKey) ([]byte, return nil, err } default: - log.Error("SignKM: can't identify bootguard header") + log.Error("SignKM: can't identify BootGuard header") } return b.WriteKM() } @@ -495,7 +495,7 @@ func (b *BootGuard) SignBPM(signAlgo, hashAlgo string, privkey crypto.PrivateKey return nil, err } default: - log.Error("SignBPM: can't identify bootguard header") + log.Error("SignBPM: can't identify BootGuard header") } return b.WriteBPM() } @@ -521,7 +521,7 @@ func (b *BootGuard) VerifyKM() error { return err } default: - log.Error("VerifyKM: can't identify bootguard header") + log.Error("VerifyKM: can't identify BootGuard header") } return nil } @@ -547,7 +547,7 @@ func (b *BootGuard) VerifyBPM() error { return err } default: - log.Error("VerifyBPM: can't identify bootguard header") + log.Error("VerifyBPM: can't identify BootGuard header") } return nil } @@ -623,7 +623,7 @@ func (b *BootGuard) CalculateNEMSize(image []byte, acm *tools.ACM) (uint16, erro } return uint16(cbntbootpolicy.NewSize4K(totalSize)), nil default: - return 0, fmt.Errorf("CalculateNEMSize: can't identify bootguard header") + return 0, fmt.Errorf("CalculateNEMSize: can't identify BootGuard header") } } @@ -681,7 +681,7 @@ func (b *BootGuard) GetBPMPubHash(pubkey crypto.PublicKey, hashAlgo string) erro } b.VData.CBNTkm.Hash = append(keyHashes, kH) default: - log.Error("can't identify bootguard header") + log.Error("can't identify BootGuard header") } return nil } @@ -765,7 +765,7 @@ func (b *BootGuard) GetIBBsDigest(image []byte, hashAlgo string) (digest []byte, } digest = hash.Sum(nil) default: - log.Error("can't identify bootguard header") + log.Error("can't identify BootGuard header") } return digest, nil } @@ -795,62 +795,70 @@ func (b *BootGuard) CreateIBBDigest(biosFilepath string) error { copy(b.VData.CBNTbpm.SE[0].DigestList.List[iterator].HashBuffer, d) } default: - log.Error("can't identify bootguard header") + log.Error("can't identify BootGuard header") } return nil } // BPMCryptoSecure verifies that BPM uses sane crypto algorithms func (b *BootGuard) BPMCryptoSecure() (bool, error) { + var errs error switch b.Version { case bgheader.Version10: hash := b.VData.BGbpm.SE[0].Digest.HashAlg if hash == bg.AlgSHA1 || hash.IsNull() { - return false, fmt.Errorf("signed IBB hash in BPM uses insecure hash algorithm SHA1/Null") + errs = multierr.Combine(errs, fmt.Errorf("signed IBB hash in BPM uses insecure hash algorithm SHA1 or has none")) } hash = b.VData.BGbpm.PMSE.Signature.HashAlg if hash == bg.AlgSHA1 || hash.IsNull() { - return false, fmt.Errorf("BPM signature uses insecure hash algorithm SHA1/Null") + errs = multierr.Combine(errs, fmt.Errorf("BPM signature uses insecure hash algorithm SHA1 or has none")) } case bgheader.Version20: for _, hash := range b.VData.CBNTbpm.SE[0].DigestList.List { if hash.HashAlg == cbnt.AlgSHA1 || hash.HashAlg.IsNull() { if b.VData.CBNTbpm.SE[0].DigestList.Size < 2 { - return false, fmt.Errorf("signed IBB hash list in BPM uses insecure hash algorithm SHA1/Null") + errs = multierr.Combine(errs, fmt.Errorf("signed IBB hash list in BPM uses insecure hash algorithm SHA1 or has none")) } } } hash := b.VData.CBNTbpm.PMSE.Signature.HashAlg if hash == cbnt.AlgSHA1 || hash.IsNull() { - return false, fmt.Errorf("BPM signature uses insecure hash algorithm SHA1/Null") + errs = multierr.Combine(errs, fmt.Errorf("BPM signature uses insecure hash algorithm SHA1 or has none")) } } + if errs != nil { + return false, fmt.Errorf("%+v", errs) + } return true, nil } // KMCryptoSecure verifies that KM uses sane crypto algorithms func (b *BootGuard) KMCryptoSecure() (bool, error) { + var errs error switch b.Version { case bgheader.Version10: hash := b.VData.BGkm.KeyAndSignature.Signature.HashAlg if hash == bg.AlgSHA1 || hash.IsNull() { - return false, fmt.Errorf("KM signature uses insecure hash algorithm SHA1/Null") + errs = multierr.Combine(errs, fmt.Errorf("Key Manifest signature uses insecure hash algorithm SHA1 or has none")) } hash = b.VData.BGkm.BPKey.HashAlg if hash == bg.AlgSHA1 || hash.IsNull() { - return false, fmt.Errorf("signed BPM hash in KM uses insecure hash algorithm SHA1/Null") + errs = multierr.Combine(errs, fmt.Errorf("signed BPM hash in Key Manifest uses insecure hash algorithm SHA1 or has none")) } case bgheader.Version20: hash := b.VData.CBNTkm.PubKeyHashAlg if hash == cbnt.AlgSHA1 || hash.IsNull() { - return false, fmt.Errorf("KM signature uses insecure hash algorithm SHA1/Null") + errs = multierr.Combine(errs, fmt.Errorf("Key Manifest signature uses insecure hash algorithm SHA1 or has none")) } for _, hash := range b.VData.CBNTkm.Hash { if hash.Digest.HashAlg == cbnt.AlgSHA1 || hash.Digest.HashAlg.IsNull() { - return false, fmt.Errorf("the KM hash %s uses insecure hash algorithm SHA1/Null", hash.Usage.String()) + errs = multierr.Combine(errs, fmt.Errorf("Key Manifest hash %s uses insecure hash algorithm SHA1 or has none", hash.Usage.String())) } } } + if errs != nil { + return false, fmt.Errorf("%+v", errs) + } return true, nil } @@ -870,7 +878,7 @@ func (b *BootGuard) KMHasBPMHash() (bool, error) { } } if !bpmHashFound { - return false, fmt.Errorf("couldn't find BPM hash in KM") + return false, fmt.Errorf("couldn't find BPM hash in Key Manifest") } return true, nil } @@ -881,14 +889,14 @@ func (b *BootGuard) BPMKeyMatchKMHash() (bool, error) { case bgheader.Version10: if b.VData.BGkm.BPKey.HashBufferTotalSize() > minHashTypeSize { if err := b.VData.BGkm.ValidateBPMKey(b.VData.BGbpm.PMSE.KeySignature); err != nil { - return false, fmt.Errorf("couldn't verify bpm hash in km") + return false, fmt.Errorf("couldn't verify BPM hash in Key Manifest") } } case bgheader.Version20: for _, hash := range b.VData.CBNTkm.Hash { if hash.Usage == cbntkey.UsageBPMSigningPKD { if err := b.VData.CBNTkm.ValidateBPMKey(b.VData.CBNTbpm.PMSE.KeySignature); err != nil { - return false, fmt.Errorf("couldn't verify bpm hash in km") + return false, fmt.Errorf("couldn't verify BPM hash in Key Manifest") } } } @@ -898,70 +906,83 @@ func (b *BootGuard) BPMKeyMatchKMHash() (bool, error) { // StrictSaneBPMSecurityProps verifies that BPM contains security properties more strictly func (b *BootGuard) StrictSaneBPMSecurityProps() (bool, error) { + var errs error + switch b.Version { case bgheader.Version10: flags := b.VData.BGbpm.SE[0].Flags if !flags.AuthorityMeasure() { - return false, fmt.Errorf("pcr-7 data should extended for OS security") + errs = multierr.Append(errs, fmt.Errorf("PCR-7 data should extended for OS security")) } if !flags.TPMFailureLeavesHierarchiesEnabled() { - return false, fmt.Errorf("tpm failure should lead to default measurements from PCR0 to PCR7") + errs = multierr.Append(errs, fmt.Errorf("TPM failure should lead to default measurements from PCR0 to PCR7")) } case bgheader.Version20: bgFlags := b.VData.CBNTbpm.SE[0].Flags if !bgFlags.AuthorityMeasure() { - return false, fmt.Errorf("pcr-7 data should extended for OS security") + errs = multierr.Append(errs, fmt.Errorf("PCR-7 data should extended for OS security")) } if !bgFlags.TPMFailureLeavesHierarchiesEnabled() { - return false, fmt.Errorf("tpm failure should lead to default measurements from PCR0 to PCR7") + errs = multierr.Append(errs, fmt.Errorf("TPM failure should lead to default measurements from PCR0 to PCR7")) } txtFlags := b.VData.CBNTbpm.TXTE.ControlFlags if txtFlags.MemoryScrubbingPolicy() != cbntbootpolicy.MemoryScrubbingPolicySACM { - return false, fmt.Errorf("S-ACM memory scrubbing should be used over the BIOS") + errs = multierr.Append(errs, fmt.Errorf("S-ACM memory scrubbing should be used over the BIOS")) } } - + if errs != nil { + return false, errs + } return b.SaneBPMSecurityProps() } // SaneBPMSecurityProps verifies that BPM contains security properties set accordingly to spec func (b *BootGuard) SaneBPMSecurityProps() (bool, error) { + var errs error + switch b.Version { case bgheader.Version10: flags := b.VData.BGbpm.SE[0].Flags if !flags.DMAProtection() { - return false, fmt.Errorf("dma protection should be enabled for bootguard") + errs = multierr.Combine(errs, fmt.Errorf("DMA protection should be enabled for BootGuard")) } if !flags.AuthorityMeasure() { - return false, fmt.Errorf("pcr-7 data should extended for OS security") + return false, fmt.Errorf("PCR-7 data should be extended for OS security") } if b.VData.BGbpm.SE[0].PBETValue.PBETValue() == 0 { - return false, fmt.Errorf("firmware shall not allowed to run infinitely after incident happened") + errs = multierr.Combine(errs, fmt.Errorf("firmware should not be allowed to run infinitely after incident happened")) } if len(b.VData.BGbpm.SE[0].IBBSegments) < 1 { - return false, fmt.Errorf("no ibb segments measured") + return false, fmt.Errorf("no IBB segments measured") } case bgheader.Version20: bgFlags := b.VData.CBNTbpm.SE[0].Flags if !bgFlags.DMAProtection() { if b.VData.CBNTbpm.SE[0].DMAProtBase0 == 0 && b.VData.CBNTbpm.SE[0].VTdBAR == 0 { - return false, fmt.Errorf("dma protection should be enabled for bootguard") + errs = multierr.Combine(errs, fmt.Errorf("DMA protection should be enabled for BootGuard")) } } if !bgFlags.AuthorityMeasure() { - return false, fmt.Errorf("pcr-7 data should extended for OS security") + return false, fmt.Errorf("PCR-7 data should be extended for OS security") } if b.VData.CBNTbpm.SE[0].PBETValue.PBETValue() == 0 { - return false, fmt.Errorf("firmware shall not allowed to run infinitely after incident happened") + errs = multierr.Combine(errs, fmt.Errorf("firmware should not be allowed to run infinitely after incident happened")) + } + pm := b.VData.BGbpm + if pm == nil { + errs = multierr.Combine(errs, fmt.Errorf("no BootGuard Boot Policy Manifest found")) } txtFlags := b.VData.CBNTbpm.TXTE.ControlFlags if !txtFlags.IsSACMRequestedToExtendStaticPCRs() { - return false, fmt.Errorf("S-ACM shall always extend static PCRs") + errs = multierr.Combine(errs, fmt.Errorf("S-ACM shall always extend static PCRs")) } if len(b.VData.CBNTbpm.SE[0].IBBSegments) < 1 { - return false, fmt.Errorf("no ibb segments measured") + return false, fmt.Errorf("no IBB segments measured") } } + if errs != nil { + return false, fmt.Errorf("%+v", errs) + } return true, nil } @@ -974,11 +995,11 @@ func (b *BootGuard) IBBsMatchBPMDigest(image []byte) (bool, error) { switch b.Version { case bgheader.Version10: if err := b.VData.BGbpm.ValidateIBB(firmware); err != nil { - return false, fmt.Errorf("bpm final ibb hash doesn't match selected measurements in image") + return false, fmt.Errorf("BPM final IBB hash doesn't match selected measurements in image") } case bgheader.Version20: if err := b.VData.CBNTbpm.ValidateIBB(firmware); err != nil { - return false, fmt.Errorf("bpm final ibb hash doesn't match selected measurements in image") + return false, fmt.Errorf("BPM final IBB hash doesn't match selected measurements in image") } } return true, nil @@ -986,28 +1007,32 @@ func (b *BootGuard) IBBsMatchBPMDigest(image []byte) (bool, error) { // ValidateMEAgainstManifests validates during runtime ME configuation with BootGuard KM & BPM manifests func (b *BootGuard) ValidateMEAgainstManifests(fws *FirmwareStatus6) (bool, error) { + var errs error switch b.Version { case bgheader.Version10: if fws.BPMSVN != uint32(b.VData.BGbpm.BPMSVN) { - return false, fmt.Errorf("bpm svn doesn't match me configuration") + errs = multierr.Combine(errs, fmt.Errorf("Boot Policy Manifest SVN doesn't match ME configuration")) } if fws.KMSVN != uint32(b.VData.BGkm.KMSVN) { - return false, fmt.Errorf("km svn doesn't match me configuration") + errs = multierr.Combine(errs, fmt.Errorf("Key Manifest SVN doesn't match ME configuration")) } if fws.KMID != uint32(b.VData.BGkm.KMID) { - return false, fmt.Errorf("km KMID doesn't match me configuration") + errs = multierr.Combine(errs, fmt.Errorf("Key Manifest ID doesn't match ME configuration")) } case bgheader.Version20: if fws.BPMSVN > uint32(b.VData.CBNTbpm.BPMSVN) { - return false, fmt.Errorf("bpm svn doesn't match me configuration") + errs = multierr.Combine(errs, fmt.Errorf("Boot Policy Manifest SVN doesn't match ME configuration")) } if fws.KMSVN != uint32(b.VData.CBNTkm.KMSVN) { - return false, fmt.Errorf("km svn doesn't match me configuration") + errs = multierr.Combine(errs, fmt.Errorf("Key Manifest SVN doesn't match ME configuration")) } if fws.KMID != uint32(b.VData.CBNTkm.KMID) { - return false, fmt.Errorf("km KMID doesn't match me configuration") + errs = multierr.Combine(errs, fmt.Errorf("Key Manifest ID doesn't match ME configuration")) } } + if errs != nil { + return false, fmt.Errorf("%+v", errs) + } return true, nil } diff --git a/pkg/test/bootguard_tests.go b/pkg/test/bootguard_tests.go index d57411e2..f3e96d0d 100644 --- a/pkg/test/bootguard_tests.go +++ b/pkg/test/bootguard_tests.go @@ -8,6 +8,7 @@ import ( "github.com/9elements/converged-security-suite/v2/pkg/tools" "github.com/9elements/go-linux-lowlevel-hw/pkg/hwapi" "github.com/linuxboot/fiano/pkg/intel/metadata/fit" + "go.uber.org/multierr" ) const ( @@ -29,7 +30,6 @@ var ( Name: "SACM meets sane BootGuard requirements", Required: true, function: BootGuardACM, - dependencies: []*Test{&testbootguardfit}, Status: Implemented, SpecificationChapter: "Chapter A. Authenticated Code Modules", SpecificiationTitle: IntelTXTSpecificationTitle, @@ -39,7 +39,6 @@ var ( Name: "Key Manifest meets sane BootGuard requirements", Required: true, function: BootGuardKM, - dependencies: []*Test{&testbootguardfit}, Status: Implemented, SpecificationChapter: "", SpecificiationTitle: IntelBootGuardSpecificationTitle, @@ -49,7 +48,6 @@ var ( Name: "Boot Policy Manifest meets sane BootGuard requirements", Required: true, function: BootGuardBPM, - dependencies: []*Test{&testbootguardfit}, Status: Implemented, SpecificationChapter: "", SpecificiationTitle: IntelBootGuardSpecificationTitle, @@ -59,7 +57,6 @@ var ( Name: "Verifies BPM and IBBs match firmware image", Required: true, function: BootGuardIBB, - dependencies: []*Test{&testbootguardfit}, Status: Implemented, SpecificationChapter: "", SpecificiationTitle: IntelBootGuardSpecificationTitle, @@ -225,19 +222,20 @@ func BootGuardBPM(hw hwapi.LowLevelHardwareInterfaces, p *PreSet) (bool, error, bpmReader = bytes.NewReader(entry.DataSegmentBytes) } } + var errs error b, err := bootguard.NewBPMAndKM(bpmReader, kmReader) if b == nil || err != nil { - return false, fmt.Errorf("couldn't parse KM and BPM"), err + errs = multierr.Combine(errs, fmt.Errorf("couldn't parse Key Manifest and Boot Policy Manifest\n")) } if err := b.ValidateBPM(); err != nil { - return false, fmt.Errorf("couldn't validate BPM"), err + errs = multierr.Combine(errs, fmt.Errorf("couldn't validate Boot Policy Manifest")) } if err := b.VerifyBPM(); err != nil { - return false, fmt.Errorf("couldn't verify BPM signature"), err + errs = multierr.Combine(errs, fmt.Errorf("couldn't verify Boot Policy Manifest signature")) } secure, err := b.BPMCryptoSecure() if !secure || err != nil { - return false, fmt.Errorf("bpm crypto parameters are insecure"), err + errs = multierr.Combine(errs, fmt.Errorf("Boot Policy Manifest crypto parameters are insecure")) } if p.Strict { secure, err = b.StrictSaneBPMSecurityProps() @@ -245,11 +243,14 @@ func BootGuardBPM(hw hwapi.LowLevelHardwareInterfaces, p *PreSet) (bool, error, secure, err = b.SaneBPMSecurityProps() } if !secure || err != nil { - return false, fmt.Errorf("bpm hasn't sane security properties"), err + errs = multierr.Combine(errs, fmt.Errorf("Boot Policy Manifest doesn't have sane security properties: %v", err)) } secure, err = b.BPMKeyMatchKMHash() if !secure || err != nil { - return false, fmt.Errorf("bpm doesn't match km hash"), err + errs = multierr.Combine(errs, fmt.Errorf("Boot Policy Manifest doesn't match Key Manifest hash: %v", err)) + } + if errs != nil { + return false, fmt.Errorf("Errors occurred"), fmt.Errorf("%+v", errs) } return true, nil, nil } diff --git a/pkg/test/test.go b/pkg/test/test.go index 0cf54ff9..d6fe763f 100644 --- a/pkg/test/test.go +++ b/pkg/test/test.go @@ -111,7 +111,7 @@ func (t *Test) Run(hw hwapi.LowLevelHardwareInterfaces, preset *PreSet) bool { } t.Result = ResultFail } else if testerror != nil && internalerror != nil { - t.ErrorText = testerror.Error() + ": " + internalerror.Error() + t.ErrorText = fmt.Sprintf("\n %v:\n %+v\n", testerror.Error(), internalerror) if t.SpecificiationTitle != "" || t.SpecificationDocumentID != "" { t.ErrorTextSpec = "Please have a look at " if t.SpecificiationTitle != "" {