Fix an eventlog parsing issue with null terminator (#540)

Signed-off-by: Jiankun Lu <[email protected]>

server/eventlog.go

@@ -7,6 +7,7 @@ import (
 	"encoding/hex"
 	"errors"
 	"fmt"
+	"hash"
 
 	"github.com/google/go-attestation/attest"
 	"github.com/google/go-eventlog/register"
@@ -453,6 +454,7 @@ func getGrubState(hash crypto.Hash, events []*pb.Event) (*pb.GrubState, error) {
 	var files []*pb.GrubFile
 	var commands []string
 	for idx, event := range events {
+		hasher := hash.New()
 		index := event.GetPcrIndex()
 		if index != 8 && index != 9 {
 			continue
@@ -471,7 +473,6 @@ func getGrubState(hash crypto.Hash, events []*pb.Event) (*pb.GrubState, error) {
 			files = append(files, &pb.GrubFile{Digest: event.GetDigest(),
 				UntrustedFilename: event.GetData()})
 		} else if index == 8 {
-			hasher := hash.New()
 			suffixAt := -1
 			rawData := event.GetData()
 			for _, prefix := range validPrefixes {
@@ -483,14 +484,16 @@ func getGrubState(hash crypto.Hash, events []*pb.Event) (*pb.GrubState, error) {
 			if suffixAt == -1 {
 				return nil, fmt.Errorf("invalid prefix seen for PCR%d event: %s", index, rawData)
 			}
-			hasher.Write(rawData[suffixAt : len(rawData)-1])
-			if !bytes.Equal(event.Digest, hasher.Sum(nil)) {
-				// Older GRUBs measure "grub_cmd " with the null terminator.
-				// However, "grub_kernel_cmdline " measurements also ignore the null terminator.
-				hasher.Reset()
-				hasher.Write(rawData[suffixAt:])
-				if !bytes.Equal(event.Digest, hasher.Sum(nil)) {
-					return nil, fmt.Errorf("invalid digest seen for GRUB event log in event %d: %s", idx, hex.EncodeToString(event.Digest))
+
+			// Check the slice is not empty after the suffix, which ensures rawData[len(rawData)-1] is not part
+			// of the suffix.
+			if len(rawData[suffixAt:]) > 0 && rawData[len(rawData)-1] == '\x00' {
+				if err := verifyNullTerminatedDataDigest(hasher, rawData[suffixAt:], event.Digest); err != nil {
+					return nil, fmt.Errorf("invalid GRUB event (null-terminated) #%d: %v", idx, err)
+				}
+			} else {
+				if err := verifyDataDigest(hasher, rawData[suffixAt:], event.Digest); err != nil {
+					return nil, fmt.Errorf("invalid GRUB event #%d: %v", idx, err)
 				}
 			}
 			hasher.Reset()
@@ -503,6 +506,33 @@ func getGrubState(hash crypto.Hash, events []*pb.Event) (*pb.GrubState, error) {
 	return &pb.GrubState{Files: files, Commands: commands}, nil
 }
 
+// verifyNullTerminatedRawData checks the digest of the data.
+// Returns nil if digest match the hash of the data or the data without the last bytes (\x00).
+// The caller needs to make sure len(data) is at least 1, and data is ended with '\x00',
+// otherwise this function will return an error.
+func verifyNullTerminatedDataDigest(hasher hash.Hash, data []byte, digest []byte) error {
+	if len(data) == 0 || data[len(data)-1] != '\x00' {
+		return errors.New("given data is not null-terminated")
+	}
+	if err := verifyDataDigest(hasher, data, digest); err != nil {
+		if err := verifyDataDigest(hasher, data[:len(data)-1], digest); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// verifyDataDigest checks the digest of the data.
+func verifyDataDigest(hasher hash.Hash, data []byte, digest []byte) error {
+	hasher.Reset()
+	hasher.Write(data)
+	defer hasher.Reset()
+	if !bytes.Equal(digest, hasher.Sum(nil)) {
+		return fmt.Errorf("invalid digest: %s", hex.EncodeToString(digest))
+	}
+	return nil
+}
+
 func getEfiState(hash crypto.Hash, events []*pb.Event) (*pb.EfiState, error) {
 	// We pre-compute various event digests, and check if those event type have
 	// been modified. We only trust events that come before the

server/eventlog_test.go

@@ -1069,6 +1069,45 @@ func TestParseLinuxKernelState(t *testing.T) {
 	}
 }
 
+func TestNullTerminatedDataDigest(t *testing.T) {
+	rawdata := []byte("123456")
+	rawdataNullTerminated := []byte("123456\x00")
+	rawdataModifyLastByte := []byte("123456\xff")
+	hash := crypto.SHA256
+	hasher := hash.New()
+	hasher.Write(rawdata)
+	rawDigest := hasher.Sum(nil)
+	hasher.Reset()
+	hasher.Write(rawdataNullTerminated)
+	nullTerminatedDigest := hasher.Sum(nil)
+	hasher.Reset()
+
+	if err := verifyDataDigest(hasher, rawdata, rawDigest); err != nil {
+		t.Error(err)
+	}
+	if err := verifyDataDigest(hasher, rawdata, nullTerminatedDigest); err == nil {
+		t.Errorf("non null-terminated data should not match the null-terminated digest")
+	}
+
+	// "rawdata + '\x00'" can be verified with digest("rawdata") as well as digest("rawdata + '\x00'")
+	if err := verifyNullTerminatedDataDigest(hasher, rawdataNullTerminated, nullTerminatedDigest); err != nil {
+		t.Error(err)
+	}
+	if err := verifyNullTerminatedDataDigest(hasher, rawdataNullTerminated, rawDigest); err != nil {
+		t.Error(err)
+	}
+
+	if err := verifyNullTerminatedDataDigest(hasher, rawdata, nullTerminatedDigest); err == nil {
+		t.Errorf("non null-terminated data should always fail")
+	}
+	if err := verifyNullTerminatedDataDigest(hasher, rawdataModifyLastByte, nullTerminatedDigest); err == nil {
+		t.Errorf("manipulated null terminated data should fail")
+	}
+	if err := verifyNullTerminatedDataDigest(hasher, []byte{}, []byte{}); err == nil {
+		t.Errorf("len() == 0 should always fail")
+	}
+}
+
 func TestParseGrubState(t *testing.T) {
 	logs := []struct {
 		eventLog
@@ -1178,6 +1217,38 @@ func TestParseEfiState(t *testing.T) {
 	}
 }
 
+func TestGetGrubStateWithModifiedNullTerminator(t *testing.T) {
+	// Choose an eventlog with GRUB.
+	eventlog := UbuntuAmdSevGCE
+	// Just use the SHA256 bank.
+	events, err := parseReplayHelper(eventlog.RawLog, eventlog.Banks[1])
+	if err != nil {
+		t.Fatal(err)
+	}
+	cryptoHash, _ := tpm2.Algorithm(eventlog.Banks[1].Hash).Hash()
+
+	// Make sure the original events can parse successfully.
+	pbEvents := convertToPbEvents(cryptoHash, events)
+	if _, err := getGrubState(cryptoHash, pbEvents); err != nil {
+		t.Fatal(err)
+	}
+
+	// Change the null terminator.
+	for _, e := range events {
+		if e.Index == 8 {
+			if e.Data[len(e.Data)-1] == '\x00' {
+				e.Data[len(e.Data)-1] = '\xff'
+			}
+		}
+	}
+
+	// Parse again, make sure it will fail.
+	pbEvents = convertToPbEvents(cryptoHash, events)
+	if _, err := getGrubState(cryptoHash, pbEvents); err == nil {
+		t.Error("Expected getGrubState to fail after modifying the null terminator")
+	}
+}
+
 func decodeHex(hexStr string) []byte {
 	bytes, err := hex.DecodeString(hexStr)
 	if err != nil {