fix: verify length and hashes of fetched bytes before parsing (#325)

* feat: add func to util for comparing raw bytes

Add BytesMatchLenAndHashes() to verify that fetched bytes match the
expected length and hashes.
This is useful to check data before parsing.

Signed-off-by: Joshua Lock <[email protected]>

* fix: verify snapshot's len and hashes before parsing

Signed-off-by: Joshua Lock <[email protected]>

* fix: verify targets' len and hashes before parsing

Signed-off-by: Joshua Lock <[email protected]>

* feat: export version comparison utility function

Signed-off-by: Joshua Lock <[email protected]>

* fix: only check version after parsing fetched ss and ts

We check the length and hashes of the fetched bytes before parsing them,
therefore once the data are parsed into a FileMeta we only need to check
against the expected version. The length and hashes checks are no longer
required at this point, as they have already been done.

Signed-off-by: Joshua Lock <[email protected]>

* fix: remove spurious length check

Signed-off-by: Joshua Lock <[email protected]>

* chore: test newly exported util functions

Add tests for newly exported functions in the util package:
* VersionEqual() - simple test of version number comparison
* BytesMatchLenAndHashes() - check function returns appropriate errors
  when incorrect length, hashes, or both are passed

Signed-off-by: Joshua Lock <[email protected]>

Author: joshuagl

client/client.go

@@ -674,14 +674,21 @@ func (c *Client) downloadMetaFromSnapshot(name string, m data.SnapshotFileMeta)
 		return nil, err
 	}
 
+	// 5.6.2 – Check length and hashes of fetched bytes *before* parsing metadata
+	if err := util.BytesMatchLenAndHashes(b, m.Length, m.Hashes); err != nil {
+		return nil, ErrDownloadFailed{name, err}
+	}
+
 	meta, err := util.GenerateSnapshotFileMeta(bytes.NewReader(b), m.HashAlgorithms()...)
 	if err != nil {
 		return nil, err
 	}
-	// 5.6.2 and 5.6.4 - Check against snapshot role's targets hash and version
-	if err := util.SnapshotFileMetaEqual(meta, m); err != nil {
+
+	// 5.6.4 - Check against snapshot role's version
+	if err := util.VersionEqual(meta.Version, m.Version); err != nil {
 		return nil, ErrDownloadFailed{name, err}
 	}
+
 	return b, nil
 }
 
@@ -691,14 +698,21 @@ func (c *Client) downloadMetaFromTimestamp(name string, m data.TimestampFileMeta
 		return nil, err
 	}
 
+	// 5.2.2. – Check length and hashes of fetched bytes *before* parsing metadata
+	if err := util.BytesMatchLenAndHashes(b, m.Length, m.Hashes); err != nil {
+		return nil, ErrDownloadFailed{name, err}
+	}
+
 	meta, err := util.GenerateTimestampFileMeta(bytes.NewReader(b), m.HashAlgorithms()...)
 	if err != nil {
 		return nil, err
 	}
-	// 5.5.2 and 5.5.4 - Check against timestamp role's snapshot hash and version
-	if err := util.TimestampFileMetaEqual(meta, m); err != nil {
+
+	// 5.5.4 - Check against timestamp role's version
+	if err := util.VersionEqual(meta.Version, m.Version); err != nil {
 		return nil, ErrDownloadFailed{name, err}
 	}
+
 	return b, nil
 }
 

util/util.go

@@ -86,6 +86,32 @@ func FileMetaEqual(actual data.FileMeta, expected data.FileMeta) error {
 	return nil
 }
 
+func BytesMatchLenAndHashes(fetched []byte, length int64, hashes data.Hashes) error {
+	flen := int64(len(fetched))
+	if length != 0 && flen != length {
+		return ErrWrongLength{length, flen}
+	}
+
+	for alg, expected := range hashes {
+		var h hash.Hash
+		switch alg {
+		case "sha256":
+			h = sha256.New()
+		case "sha512":
+			h = sha512.New()
+		default:
+			return ErrUnknownHashAlgorithm{alg}
+		}
+		h.Write(fetched)
+		hash := h.Sum(nil)
+		if !hmac.Equal(hash, expected) {
+			return ErrWrongHash{alg, expected, hash}
+		}
+	}
+
+	return nil
+}
+
 func hashEqual(actual data.Hashes, expected data.Hashes) error {
 	hashChecked := false
 	for typ, hash := range expected {
@@ -102,7 +128,7 @@ func hashEqual(actual data.Hashes, expected data.Hashes) error {
 	return nil
 }
 
-func versionEqual(actual int64, expected int64) error {
+func VersionEqual(actual int64, expected int64) error {
 	if actual != expected {
 		return ErrWrongVersion{expected, actual}
 	}
@@ -125,7 +151,7 @@ func SnapshotFileMetaEqual(actual data.SnapshotFileMeta, expected data.SnapshotF
 		}
 	}
 	// 5.6.4 - Check against snapshot role's snapshot version
-	if err := versionEqual(actual.Version, expected.Version); err != nil {
+	if err := VersionEqual(actual.Version, expected.Version); err != nil {
 		return err
 	}
 
@@ -149,7 +175,7 @@ func TimestampFileMetaEqual(actual data.TimestampFileMeta, expected data.Timesta
 		}
 	}
 	// 5.5.4 - Check against timestamp role's snapshot version
-	if err := versionEqual(actual.Version, expected.Version); err != nil {
+	if err := VersionEqual(actual.Version, expected.Version); err != nil {
 		return err
 	}
 

util/util_test.go

@@ -2,7 +2,10 @@ package util
 
 import (
 	"bytes"
+	"crypto/sha256"
+	"crypto/sha512"
 	"encoding/hex"
+	"hash"
 	"testing"
 
 	"github.com/theupdateframework/go-tuf/data"
@@ -195,3 +198,78 @@ func (UtilSuite) TestHashedPaths(c *C) {
 		delete(expected, path)
 	}
 }
+
+func (UtilSuite) TestVersionEqual(c *C) {
+	c.Assert(VersionEqual(1, 1), IsNil)
+	c.Assert(VersionEqual(1, 3), Equals, ErrWrongVersion{3, 1})
+}
+
+func makeHash(b []byte, alg string) []byte {
+	var h hash.Hash
+
+	switch alg {
+	case "sha256":
+		h = sha256.New()
+	case "sha512":
+		h = sha512.New()
+	}
+	h.Write(b)
+	return h.Sum(nil)
+}
+
+func (UtilSuite) TestBytesMatchLenAndHashes(c *C) {
+	type test struct {
+		name   string
+		bytes  []byte
+		length int64
+		hashes data.Hashes
+		err    func(test) error
+	}
+
+	b := []byte{82, 253, 252, 7, 33, 130, 101, 79, 22, 63, 95, 15, 154, 98, 29, 114}
+	bhashes := data.Hashes{
+		"sha512": makeHash(b, "sha512"),
+		"sha256": makeHash(b, "sha256"),
+	}
+
+	tests := []test{
+		{
+			name:   "correct len and hashes",
+			bytes:  b,
+			length: 16,
+			hashes: bhashes,
+			err:    func(test) error { return nil },
+		},
+		{
+			name:   "incorrect len",
+			bytes:  b,
+			length: 32,
+			hashes: bhashes,
+			err:    func(test) error { return ErrWrongLength{32, 16} },
+		},
+		{
+			name:   "incorrect hashes",
+			bytes:  b,
+			length: 16,
+			hashes: data.Hashes{
+				"sha512": makeHash(b, "sha256"),
+				"sha256": makeHash(b, "sha512"),
+			},
+			err: func(test) error { return ErrWrongHash{"sha512", bhashes["sha256"], bhashes["sha512"]} },
+		},
+		{
+			name:   "incorrect len and hashes",
+			bytes:  b,
+			length: 32,
+			hashes: data.Hashes{
+				"sha512": makeHash(b, "sha256"),
+				"sha256": makeHash(b, "sha512"),
+			},
+			err: func(test) error { return ErrWrongLength{32, 16} },
+		},
+	}
+
+	for _, t := range tests {
+		c.Assert(BytesMatchLenAndHashes(t.bytes, t.length, t.hashes), DeepEquals, t.err(t), Commentf("name = %s", t.name))
+	}
+}