Skip to content

Commit 1ca9f9b

Browse files
prasanna8585liamjm
authored andcommitted
attest/internal: address review feedback on SignatureHeaderSize fix
- Use buf.Seek() instead of binary.Read() to skip vendor header bytes (no need to allocate vendorHeader since we do not use its contents) - Add sha256HashSize const instead of magic literal 32 - Reuse buildEFISignatureListData() in regression test - Use efiSignatureListHeaderSize instead of magic literal 28
1 parent e9b5cbf commit 1ca9f9b

2 files changed

Lines changed: 19 additions & 25 deletions

File tree

attest/internal/events.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -392,9 +392,8 @@ func parseEfiSignatureList(b []byte) ([]x509.Certificate, [][]byte, error) {
392392
// Without this, vendor bytes are misread as signature entries, allowing a
393393
// crafted event log to inject arbitrary hashes into the trusted hash list.
394394
if signatures.Header.SignatureHeaderSize > 0 {
395-
vendorHeader := make([]byte, signatures.Header.SignatureHeaderSize)
396-
if err := binary.Read(buf, binary.LittleEndian, &vendorHeader); err != nil {
397-
return nil, nil, fmt.Errorf("reading signature vendor header: %w", err)
395+
if _, err := buf.Seek(int64(signatures.Header.SignatureHeaderSize), io.SeekCurrent); err != nil {
396+
return nil, nil, fmt.Errorf("seeking past signature vendor header: %w", err)
398397
}
399398
}
400399

attest/internal/events_test.go

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -81,35 +81,30 @@ func TestParseEfiSignatureListNonZeroSignatureHeaderSize(t *testing.T) {
8181
}
8282

8383
const (
84-
sigSize = 48 // 16-byte GUID + 32-byte SHA256
85-
sigHeaderSize = 48 // vendor header exactly one entry worth
84+
sha256HashSize = 32 // SHA256 digest length in bytes
85+
sigSize = efiGUIDSize + sha256HashSize // 16-byte GUID + 32-byte hash
86+
sigHeaderSize = sigSize // vendor header = one fake entry worth
8687
)
8788

88-
// Vendor header: zero GUID + attacker-chosen hash (0xAA * 32)
89-
attackerHash := bytes.Repeat([]byte{0xAA}, 32)
89+
// Vendor header: zero GUID + attacker-chosen hash (0xAA * sha256HashSize)
90+
attackerHash := bytes.Repeat([]byte{0xAA}, sha256HashSize)
9091
vendorHeader := make([]byte, sigHeaderSize)
91-
copy(vendorHeader[16:], attackerHash)
92+
copy(vendorHeader[efiGUIDSize:], attackerHash)
9293

93-
// One legitimate entry: zero GUID + 0xBB * 32
94-
legitHash := bytes.Repeat([]byte{0xBB}, 32)
94+
// One legitimate entry: zero GUID + 0xBB * sha256HashSize
95+
legitHash := bytes.Repeat([]byte{0xBB}, sha256HashSize)
9596
legitEntry := make([]byte, sigSize)
96-
copy(legitEntry[16:], legitHash)
97+
copy(legitEntry[efiGUIDSize:], legitHash)
9798

98-
sigListSize := uint32(28 + sigHeaderSize + len(legitEntry)) // 124 bytes
99+
// Build the EFI_SIGNATURE_LIST using the existing helper, then append
100+
// the vendor header and legitimate entry manually.
101+
// sigListSize = fixed header + vendor header + one entry
102+
sigListSize := uint32(efiSignatureListHeaderSize + sigHeaderSize + len(legitEntry))
103+
data := buildEFISignatureListData(sigType, sigListSize, sigHeaderSize, sigSize, 0)
104+
data = append(data, vendorHeader...)
105+
data = append(data, legitEntry...)
99106

100-
writeU32 := func(buf *bytes.Buffer, v uint32) {
101-
b := []byte{byte(v), byte(v >> 8), byte(v >> 16), byte(v >> 24)}
102-
buf.Write(b)
103-
}
104-
var buf bytes.Buffer
105-
buf.Write(sigType[:])
106-
writeU32(&buf, sigListSize)
107-
writeU32(&buf, uint32(sigHeaderSize))
108-
writeU32(&buf, uint32(sigSize))
109-
buf.Write(vendorHeader)
110-
buf.Write(legitEntry)
111-
112-
_, hashes, err := parseEfiSignatureList(buf.Bytes())
107+
_, hashes, err := parseEfiSignatureList(data)
113108
if err != nil {
114109
// Acceptable: the bound check rejects the oversized SignatureHeaderSize.
115110
return

0 commit comments

Comments
 (0)