From 75e72d826ce9b117430f1d594a24d4954691f60c Mon Sep 17 00:00:00 2001
From: Michael Eischer <michael.eischer@fau.de>
Date: Sun, 4 Feb 2024 11:58:29 +0100
Subject: [PATCH] pack: verify integrity of pack file header

---
 internal/pack/pack.go               | 43 +++++++++++++++------
 internal/pack/pack_internal_test.go | 58 +++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+), 11 deletions(-)

diff --git a/internal/pack/pack.go b/internal/pack/pack.go
index 34ad9d071..34e87f1f9 100644
--- a/internal/pack/pack.go
+++ b/internal/pack/pack.go
@@ -1,6 +1,7 @@
 package pack
 
 import (
+	"bytes"
 	"context"
 	"encoding/binary"
 	"fmt"
@@ -74,7 +75,7 @@ func (p *Packer) Finalize() error {
 	p.m.Lock()
 	defer p.m.Unlock()
 
-	header, err := p.makeHeader()
+	header, err := makeHeader(p.blobs)
 	if err != nil {
 		return err
 	}
@@ -83,6 +84,11 @@ func (p *Packer) Finalize() error {
 	nonce := crypto.NewRandomNonce()
 	encryptedHeader = append(encryptedHeader, nonce...)
 	encryptedHeader = p.k.Seal(encryptedHeader, nonce, header, nil)
+	encryptedHeader = binary.LittleEndian.AppendUint32(encryptedHeader, uint32(len(encryptedHeader)))
+
+	if err := verifyHeader(p.k, encryptedHeader, p.blobs); err != nil {
+		return fmt.Errorf("detected data corruption while writing pack-file header: %w\nCorrupted data is either caused by hardware problems or bugs in restic. Please open an issue at https://github.com/restic/restic/issues/new/choose for further troubleshooting", err)
+	}
 
 	// append the header
 	n, err := p.wr.Write(encryptedHeader)
@@ -90,18 +96,33 @@ func (p *Packer) Finalize() error {
 		return errors.Wrap(err, "Write")
 	}
 
-	hdrBytes := len(encryptedHeader)
-	if n != hdrBytes {
+	if n != len(encryptedHeader) {
 		return errors.New("wrong number of bytes written")
 	}
+	p.bytes += uint(len(encryptedHeader))
 
-	// write length
-	err = binary.Write(p.wr, binary.LittleEndian, uint32(hdrBytes))
+	return nil
+}
+
+func verifyHeader(k *crypto.Key, header []byte, expected []restic.Blob) error {
+	// do not offer a way to skip the pack header verification, as pack headers are usually small enough
+	// to not result in a significant performance impact
+
+	decoded, hdrSize, err := List(k, bytes.NewReader(header), int64(len(header)))
 	if err != nil {
-		return errors.Wrap(err, "binary.Write")
+		return fmt.Errorf("header decoding failed: %w", err)
+	}
+	if hdrSize != uint32(len(header)) {
+		return fmt.Errorf("unexpected header size %v instead of %v", hdrSize, len(header))
+	}
+	if len(decoded) != len(expected) {
+		return fmt.Errorf("pack header size mismatch")
+	}
+	for i := 0; i < len(decoded); i++ {
+		if decoded[i] != expected[i] {
+			return fmt.Errorf("pack header entry mismatch got %v instead of %v", decoded[i], expected[i])
+		}
 	}
-	p.bytes += uint(hdrBytes + binary.Size(uint32(0)))
-
 	return nil
 }
 
@@ -111,10 +132,10 @@ func (p *Packer) HeaderOverhead() int {
 }
 
 // makeHeader constructs the header for p.
-func (p *Packer) makeHeader() ([]byte, error) {
-	buf := make([]byte, 0, len(p.blobs)*int(entrySize))
+func makeHeader(blobs []restic.Blob) ([]byte, error) {
+	buf := make([]byte, 0, len(blobs)*int(entrySize))
 
-	for _, b := range p.blobs {
+	for _, b := range blobs {
 		switch {
 		case b.Type == restic.DataBlob && b.UncompressedLength == 0:
 			buf = append(buf, 0)
diff --git a/internal/pack/pack_internal_test.go b/internal/pack/pack_internal_test.go
index c1a4867ea..2e7400ad0 100644
--- a/internal/pack/pack_internal_test.go
+++ b/internal/pack/pack_internal_test.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/binary"
 	"io"
+	"strings"
 	"testing"
 
 	"github.com/restic/restic/internal/crypto"
@@ -177,3 +178,60 @@ func TestReadRecords(t *testing.T) {
 		}
 	}
 }
+
+func TestUnpackedVerification(t *testing.T) {
+	// create random keys
+	k := crypto.NewRandomKey()
+	blobs := []restic.Blob{
+		{
+			BlobHandle:         restic.NewRandomBlobHandle(),
+			Length:             42,
+			Offset:             0,
+			UncompressedLength: 2 * 42,
+		},
+	}
+
+	type DamageType string
+	const (
+		damageData       DamageType = "data"
+		damageCiphertext DamageType = "ciphertext"
+		damageLength     DamageType = "length"
+	)
+
+	for _, test := range []struct {
+		damage DamageType
+		msg    string
+	}{
+		{"", ""},
+		{damageData, "pack header entry mismatch"},
+		{damageCiphertext, "ciphertext verification failed"},
+		{damageLength, "header decoding failed"},
+	} {
+		header, err := makeHeader(blobs)
+		rtest.OK(t, err)
+
+		if test.damage == damageData {
+			header[8] ^= 0x42
+		}
+
+		encryptedHeader := make([]byte, 0, crypto.CiphertextLength(len(header)))
+		nonce := crypto.NewRandomNonce()
+		encryptedHeader = append(encryptedHeader, nonce...)
+		encryptedHeader = k.Seal(encryptedHeader, nonce, header, nil)
+		encryptedHeader = binary.LittleEndian.AppendUint32(encryptedHeader, uint32(len(encryptedHeader)))
+
+		if test.damage == damageCiphertext {
+			encryptedHeader[8] ^= 0x42
+		}
+		if test.damage == damageLength {
+			encryptedHeader[len(encryptedHeader)-1] ^= 0x42
+		}
+
+		err = verifyHeader(k, encryptedHeader, blobs)
+		if test.msg == "" {
+			rtest.Assert(t, err == nil, "expected no error, got %v", err)
+		} else {
+			rtest.Assert(t, strings.Contains(err.Error(), test.msg), "expected error to contain %q, got %q", test.msg, err)
+		}
+	}
+}