From ec385b22ad75c92a4bb4e238fb6bce2808b422c9 Mon Sep 17 00:00:00 2001 From: Joshua Lock Date: Tue, 21 Jun 2022 10:54:42 +0100 Subject: [PATCH 1/7] 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 --- util/util.go | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/util/util.go b/util/util.go index 5690b884..035e0971 100644 --- a/util/util.go +++ b/util/util.go @@ -86,6 +86,33 @@ 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} + } + + if len(hashes) != 0 { + 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 { From 0c35db7f6f142d272091e070a0af0c1c26893317 Mon Sep 17 00:00:00 2001 From: Joshua Lock Date: Tue, 21 Jun 2022 11:12:59 +0100 Subject: [PATCH 2/7] fix: verify snapshot's len and hashes before parsing Signed-off-by: Joshua Lock --- client/client.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/client/client.go b/client/client.go index b7e30b24..88b0bd4d 100644 --- a/client/client.go +++ b/client/client.go @@ -691,11 +691,16 @@ 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 + // 5.5.4 - Check against timestamp role's snapshot hash and version if err := util.TimestampFileMetaEqual(meta, m); err != nil { return nil, ErrDownloadFailed{name, err} } From 0f0388a6d1b553570c65ab2c2e479c0ff2f24559 Mon Sep 17 00:00:00 2001 From: Joshua Lock Date: Tue, 21 Jun 2022 11:14:58 +0100 Subject: [PATCH 3/7] fix: verify targets' len and hashes before parsing Signed-off-by: Joshua Lock --- client/client.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/client/client.go b/client/client.go index 88b0bd4d..5f298c0a 100644 --- a/client/client.go +++ b/client/client.go @@ -674,11 +674,16 @@ 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 + // 5.6.4 - Check against snapshot role's targets hash and version if err := util.SnapshotFileMetaEqual(meta, m); err != nil { return nil, ErrDownloadFailed{name, err} } From 2be28822b3f08bd7a70c488d1f055739fbd446a8 Mon Sep 17 00:00:00 2001 From: Joshua Lock Date: Tue, 21 Jun 2022 11:38:33 +0100 Subject: [PATCH 4/7] feat: export version comparison utility function Signed-off-by: Joshua Lock --- util/util.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/util/util.go b/util/util.go index 035e0971..f7ef7480 100644 --- a/util/util.go +++ b/util/util.go @@ -129,7 +129,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} } @@ -152,7 +152,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 } @@ -176,7 +176,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 } From d73f89fd3302b06a94fb0498a51d432774065a00 Mon Sep 17 00:00:00 2001 From: Joshua Lock Date: Tue, 21 Jun 2022 11:42:26 +0100 Subject: [PATCH 5/7] 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 --- client/client.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/client/client.go b/client/client.go index 5f298c0a..080deefd 100644 --- a/client/client.go +++ b/client/client.go @@ -683,10 +683,12 @@ func (c *Client) downloadMetaFromSnapshot(name string, m data.SnapshotFileMeta) if err != nil { return nil, err } - // 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 } @@ -705,10 +707,12 @@ func (c *Client) downloadMetaFromTimestamp(name string, m data.TimestampFileMeta if err != nil { return nil, err } - // 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 } From 39b9b0f1c495d2be5ff14317f0248dce5327e05d Mon Sep 17 00:00:00 2001 From: Joshua Lock Date: Thu, 23 Jun 2022 11:02:10 +0100 Subject: [PATCH 6/7] fix: remove spurious length check Signed-off-by: Joshua Lock --- util/util.go | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/util/util.go b/util/util.go index f7ef7480..7ccb0d32 100644 --- a/util/util.go +++ b/util/util.go @@ -92,24 +92,23 @@ func BytesMatchLenAndHashes(fetched []byte, length int64, hashes data.Hashes) er return ErrWrongLength{length, flen} } - if len(hashes) != 0 { - 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} - } + 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 } From af46f5d31d0aad033a629457fd50e8ac16cd9d19 Mon Sep 17 00:00:00 2001 From: Joshua Lock Date: Thu, 23 Jun 2022 14:08:29 +0100 Subject: [PATCH 7/7] 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 --- util/util_test.go | 78 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/util/util_test.go b/util/util_test.go index 4ee48946..cb36b3b4 100644 --- a/util/util_test.go +++ b/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)) + } +}