diff --git a/attest/internal/events.go b/attest/internal/events.go index 2270d38..c1f1a12 100644 --- a/attest/internal/events.go +++ b/attest/internal/events.go @@ -8,6 +8,7 @@ import ( "io" "unicode/utf16" + "github.com/google/certificate-transparency-go/asn1" "github.com/google/certificate-transparency-go/x509" ) @@ -78,6 +79,11 @@ const ( EFIVariableAuthority EventType = 0x800000e0 ) +// ErrSigMissingGUID is returned if an EFI_SIGNATURE_DATA structure was parsed +// successfully, however was missing the SignatureOwner GUID. This case is +// handled specially as a workaround for a bug relating to authority events. +var ErrSigMissingGUID = errors.New("signature data was missing owner GUID") + var eventTypeNames = map[EventType]string{ PrebootCert: "Preboot Cert", PostCode: "POST Code", @@ -218,10 +224,7 @@ func ParseUEFIVariableAuthority(r io.Reader) (UEFIVariableAuthority, error) { return UEFIVariableAuthority{}, err } certs, err := parseEfiSignature(v.VariableData) - if err != nil { - return UEFIVariableAuthority{}, err - } - return UEFIVariableAuthority{Certs: certs}, nil + return UEFIVariableAuthority{Certs: certs}, err } // efiSignatureData represents the EFI_SIGNATURE_DATA type. @@ -363,9 +366,21 @@ func parseEfiSignature(b []byte) ([]x509.Certificate, error) { if err := binary.Read(buf, binary.LittleEndian, &signature.SignatureData); err != nil { return certificates, err } + cert, err := x509.ParseCertificate(signature.SignatureData) if err == nil { certificates = append(certificates, *cert) + } else { + // A bug in shim may cause an event to be missing the SignatureOwner GUID. + // We handle this, but signal back to the caller using ErrSigMissingGUID. + if _, isStructuralErr := err.(asn1.StructuralError); isStructuralErr { + var err2 error + cert, err2 = x509.ParseCertificate(b) + if err2 == nil { + certificates = append(certificates, *cert) + err = ErrSigMissingGUID + } + } } return certificates, err } diff --git a/attest/secureboot.go b/attest/secureboot.go index 4e88fa7..fd07159 100644 --- a/attest/secureboot.go +++ b/attest/secureboot.go @@ -144,7 +144,18 @@ func ParseSecurebootState(events []Event) (*SecurebootState, error) { case internal.EFIVariableAuthority: a, err := internal.ParseUEFIVariableAuthority(bytes.NewReader(e.Data)) if err != nil { - return nil, fmt.Errorf("failed parsing EFI variable authority at event %d: %v", e.sequence, err) + // Workaround for: https://github.com/google/go-attestation/issues/157 + if err == internal.ErrSigMissingGUID { + // Versions of shim which do not carry + // https://github.com/rhboot/shim/commit/8a27a4809a6a2b40fb6a4049071bf96d6ad71b50 + // have an erroneous additional byte in the event, which breaks digest + // verification. If verification failed, we try removing the last byte. + if digestVerify != nil { + digestVerify = e.digestEquals(e.Data[:len(e.Data)-1]) + } + } else { + return nil, fmt.Errorf("failed parsing EFI variable authority at event %d: %v", e.sequence, err) + } } seenAuthority = true if digestVerify != nil { diff --git a/attest/secureboot_test.go b/attest/secureboot_test.go index 3c0fab2..c0e37c3 100644 --- a/attest/secureboot_test.go +++ b/attest/secureboot_test.go @@ -34,3 +34,79 @@ func TestSecureBoot(t *testing.T) { t.Errorf("secureboot.Enabled = %v, want %v", got, want) } } + +// See: https://github.com/google/go-attestation/issues/157 +func TestSecureBootBug157(t *testing.T) { + raw, err := ioutil.ReadFile("testdata/sb_cert_eventlog") + if err != nil { + t.Fatalf("reading test data: %v", err) + } + elr, err := ParseEventLog(raw) + if err != nil { + t.Fatalf("parsing event log: %v", err) + } + + pcrs := []PCR{ + {'\x00', []byte("Q\xc3#\xde\f\fiOF\x01\xcd\xd0+\xebX\xff\x13b\x9ft"), '\x03'}, + {'\x01', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x03'}, + {'\x02', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x03'}, + {'\x03', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x03'}, + {'\x04', []byte("\xb7q\x00\x8d\x17<\x02+\xc1oKM\x1a\u007f\x8b\x99\xed\x88\xee\xb1"), '\x03'}, + {'\x05', []byte("\xd79j\xc6\xe8\x87\xda\"ޠ;@\x95/p\xb8\xdbҩ\x96"), '\x03'}, + {'\x06', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x03'}, + {'\a', []byte("E\xa8b\x1d4\xa5}\xf2\xb2\xe7\xf1L\x92\xb9\x9a\xc8\xde}X\x05"), '\x03'}, + {'\b', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x03'}, + {'\t', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x03'}, + {'\n', []byte("\x82\x84\x10>\x06\xd4\x01\"\xbcd\xa0䡉\x1a\xf9\xec\xd4\\\xf6"), '\x03'}, + {'\v', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x03'}, + {'\f', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x03'}, + {'\r', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x03'}, + {'\x0e', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x03'}, + {'\x0f', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x03'}, + {'\x10', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x03'}, + {'\x11', []byte("\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff"), '\x03'}, + {'\x12', []byte("\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff"), '\x03'}, + {'\x13', []byte("\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff"), '\x03'}, + {'\x14', []byte("\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff"), '\x03'}, + {'\x15', []byte("\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff"), '\x03'}, + {'\x16', []byte("\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff"), '\x03'}, + {'\x17', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x03'}, + {'\x00', []byte("\xfc\xec\xb5j\xcc08b\xb3\x0e\xb3Bę\v\xebP\xb5ૉr$I\xc2٧?7\xb0\x19\xfe"), '\x05'}, + {'\x01', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x05'}, + {'\x02', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x05'}, + {'\x03', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x05'}, + {'\x04', []byte("\xa9)h\x80oy_\xa3D5\xd9\xf1\x18\x13hL\xa1\xe7\x05`w\xf7\x00\xbaI\xf2o\x99b\xf8m\x89"), '\x05'}, + {'\x05', []byte("̆\x18\xb7y2\xb4\xef\xda\x12\xccX\xba\xd9>\xcdѕ\x9d\xea)\xe5\xabyE%\xa6\x19\xf5\xba\xab\xee"), '\x05'}, + {'\x06', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x05'}, + {'\a', []byte("Q\xb3\x04\x88\xc9\xe6%]\x82+\xdc\x1b ٩,2\xbd\xe6\xc3\xe7\xbc\x02\xbc\xdd2\x82^\xb5\xef\x06\x9a"), '\x05'}, + {'\b', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x05'}, + {'\t', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x05'}, + {'\n', []byte("\xc3l\x9a\xb1\x10\x9b\xa0\x8a?dX!\x18\xf8G\x1a]i[\xc9#\xa0\xa2\xbd\x04]\xb1K\x97OB9"), '\x05'}, + {'\v', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x05'}, + {'\f', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x05'}, + {'\r', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x05'}, + {'\x0e', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x05'}, + {'\x0f', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x05'}, + {'\x10', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x05'}, + {'\x11', []byte("\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff"), '\x05'}, + {'\x12', []byte("\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff"), '\x05'}, + {'\x13', []byte("\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff"), '\x05'}, + {'\x14', []byte("\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff"), '\x05'}, + {'\x15', []byte("\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff"), '\x05'}, + {'\x16', []byte("\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff\xff"), '\x05'}, + {'\x17', []byte("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"), '\x05'}, + } + + events, err := elr.Verify(pcrs) + if err != nil { + t.Errorf("failed to verify log: %v", err) + } + + sbs, err := ParseSecurebootState(events) + if err != nil { + t.Errorf("failed parsing secureboot state: %v", err) + } + if got, want := len(sbs.PostSeparatorAuthority), 3; got != want { + t.Errorf("len(sbs.PostSeparatorAuthority) = %d, want %d", got, want) + } +} diff --git a/attest/testdata/sb_cert_eventlog b/attest/testdata/sb_cert_eventlog new file mode 100644 index 0000000..e3a64ce Binary files /dev/null and b/attest/testdata/sb_cert_eventlog differ