From 2e6c62191fb27ac26b10dd7312d239ffd6c1e498 Mon Sep 17 00:00:00 2001 From: asraa Date: Wed, 20 Jul 2022 04:54:52 -0500 Subject: [PATCH] fix: require length and hashes for target metadata (#345) * fix: require length and hashes Signed-off-by: Asra Ali * update pr and add test Signed-off-by: Asra Ali * Fix lint Signed-off-by: Asra Ali * make metapathfilemeta private Signed-off-by: Asra Ali --- client/client.go | 14 ++++++------- data/types.go | 36 ++++++++++++++++---------------- repo.go | 4 ++-- repo_test.go | 53 ++++++++++++++++++++++++++++++++++++++++++++--- util/util.go | 10 +++++---- util/util_test.go | 13 ++++++------ 6 files changed, 89 insertions(+), 41 deletions(-) diff --git a/client/client.go b/client/client.go index 080deefd..67850969 100644 --- a/client/client.go +++ b/client/client.go @@ -669,7 +669,7 @@ func (c *Client) downloadMeta(name string, version int64, m data.FileMeta) ([]by } func (c *Client) downloadMetaFromSnapshot(name string, m data.SnapshotFileMeta) ([]byte, error) { - b, err := c.downloadMeta(name, m.Version, m.FileMeta) + b, err := c.downloadMeta(name, m.Version, data.FileMeta{Length: m.Length, Hashes: m.Hashes}) if err != nil { return nil, err } @@ -679,7 +679,7 @@ func (c *Client) downloadMetaFromSnapshot(name string, m data.SnapshotFileMeta) return nil, ErrDownloadFailed{name, err} } - meta, err := util.GenerateSnapshotFileMeta(bytes.NewReader(b), m.HashAlgorithms()...) + meta, err := util.GenerateSnapshotFileMeta(bytes.NewReader(b), m.Hashes.HashAlgorithms()...) if err != nil { return nil, err } @@ -693,7 +693,7 @@ func (c *Client) downloadMetaFromSnapshot(name string, m data.SnapshotFileMeta) } func (c *Client) downloadMetaFromTimestamp(name string, m data.TimestampFileMeta) ([]byte, error) { - b, err := c.downloadMeta(name, m.Version, m.FileMeta) + b, err := c.downloadMeta(name, m.Version, data.FileMeta{Length: m.Length, Hashes: m.Hashes}) if err != nil { return nil, err } @@ -703,7 +703,7 @@ func (c *Client) downloadMetaFromTimestamp(name string, m data.TimestampFileMeta return nil, ErrDownloadFailed{name, err} } - meta, err := util.GenerateTimestampFileMeta(bytes.NewReader(b), m.HashAlgorithms()...) + meta, err := util.GenerateTimestampFileMeta(bytes.NewReader(b), m.Hashes.HashAlgorithms()...) if err != nil { return nil, err } @@ -825,7 +825,7 @@ func (c *Client) localMetaFromSnapshot(name string, m data.SnapshotFileMeta) (js if !ok { return nil, false } - meta, err := util.GenerateSnapshotFileMeta(bytes.NewReader(b), m.HashAlgorithms()...) + meta, err := util.GenerateSnapshotFileMeta(bytes.NewReader(b), m.Hashes.HashAlgorithms()...) if err != nil { return nil, false } @@ -840,7 +840,7 @@ func (c *Client) hasTargetsMeta(m data.SnapshotFileMeta) bool { if !ok { return false } - meta, err := util.GenerateSnapshotFileMeta(bytes.NewReader(b), m.HashAlgorithms()...) + meta, err := util.GenerateSnapshotFileMeta(bytes.NewReader(b), m.Hashes.HashAlgorithms()...) if err != nil { return false } @@ -855,7 +855,7 @@ func (c *Client) hasMetaFromTimestamp(name string, m data.TimestampFileMeta) boo if !ok { return false } - meta, err := util.GenerateTimestampFileMeta(bytes.NewReader(b), m.HashAlgorithms()...) + meta, err := util.GenerateTimestampFileMeta(bytes.NewReader(b), m.Hashes.HashAlgorithms()...) if err != nil { return false } diff --git a/data/types.go b/data/types.go index 6ec2fc34..ebf7e5ca 100644 --- a/data/types.go +++ b/data/types.go @@ -147,29 +147,26 @@ func (r *Role) AddKeyIDs(ids []string) bool { return changed } -type Files map[string]FileMeta - -type FileMeta struct { - Length int64 `json:"length,omitempty"` - Hashes Hashes `json:"hashes,omitempty"` - Custom *json.RawMessage `json:"custom,omitempty"` -} +type Files map[string]TargetFileMeta type Hashes map[string]HexBytes -func (f FileMeta) HashAlgorithms() []string { - funcs := make([]string, 0, len(f.Hashes)) - for name := range f.Hashes { +func (f Hashes) HashAlgorithms() []string { + funcs := make([]string, 0, len(f)) + for name := range f { funcs = append(funcs, name) } return funcs } -type SnapshotFileMeta struct { - FileMeta - Version int64 `json:"version"` +type metapathFileMeta struct { + Length int64 `json:"length,omitempty"` + Hashes Hashes `json:"hashes,omitempty"` + Version int64 `json:"version"` } +type SnapshotFileMeta metapathFileMeta + type SnapshotFiles map[string]SnapshotFileMeta type Snapshot struct { @@ -190,14 +187,20 @@ func NewSnapshot() *Snapshot { } } +type FileMeta struct { + Length int64 `json:"length"` + Hashes Hashes `json:"hashes"` +} + type TargetFiles map[string]TargetFileMeta type TargetFileMeta struct { FileMeta + Custom *json.RawMessage `json:"custom,omitempty"` } func (f TargetFileMeta) HashAlgorithms() []string { - return f.FileMeta.HashAlgorithms() + return f.FileMeta.Hashes.HashAlgorithms() } type Targets struct { @@ -300,10 +303,7 @@ func NewTargets() *Targets { } } -type TimestampFileMeta struct { - FileMeta - Version int64 `json:"version"` -} +type TimestampFileMeta metapathFileMeta type TimestampFiles map[string]TimestampFileMeta diff --git a/repo.go b/repo.go index 887a04ad..90d16928 100644 --- a/repo.go +++ b/repo.go @@ -1027,7 +1027,7 @@ func (r *Repo) AddTargetsWithDigest(digest string, digestAlg string, length int6 // This is the targets role that needs to sign the target file. targetsRoleName := delegation.Delegatee.Name - meta := data.FileMeta{Length: length, Hashes: make(data.Hashes, 1)} + meta := data.TargetFileMeta{FileMeta: data.FileMeta{Length: length, Hashes: make(data.Hashes, 1)}} meta.Hashes[digestAlg], err = hex.DecodeString(digest) if err != nil { return err @@ -1044,7 +1044,7 @@ func (r *Repo) AddTargetsWithDigest(digest string, digestAlg string, length int6 // What does G2 mean? Copying and pasting this comment from elsewhere in this file. // G2 -> we no longer desire any readers to ever observe non-prefix targets. delete(targetsMeta.Targets, "/"+path) - targetsMeta.Targets[path] = data.TargetFileMeta{FileMeta: meta} + targetsMeta.Targets[path] = meta targetsMeta.Expires = expires.Round(time.Second) diff --git a/repo_test.go b/repo_test.go index a8f44052..8fcfabcd 100644 --- a/repo_test.go +++ b/repo_test.go @@ -1282,12 +1282,12 @@ func (rs *RepoSuite) TestHashAlgorithm(c *C) { c.Assert(err, IsNil) for name, file := range map[string]data.FileMeta{ "foo.txt": targets.Targets["foo.txt"].FileMeta, - "targets.json": snapshot.Meta["targets.json"].FileMeta, - "snapshot.json": timestamp.Meta["snapshot.json"].FileMeta, + "targets.json": {Length: snapshot.Meta["targets.json"].Length, Hashes: snapshot.Meta["targets.json"].Hashes}, + "snapshot.json": {Length: timestamp.Meta["snapshot.json"].Length, Hashes: timestamp.Meta["snapshot.json"].Hashes}, } { for _, hashAlgorithm := range test.expected { if _, ok := file.Hashes[hashAlgorithm]; !ok { - c.Fatalf("expected %s hash to contain hash func %s, got %s", name, hashAlgorithm, file.HashAlgorithms()) + c.Fatalf("expected %s hash to contain hash func %s, got %s", name, hashAlgorithm, file.Hashes.HashAlgorithms()) } } } @@ -2654,3 +2654,50 @@ func (rs *RepoSuite) TestSnapshotWithInvalidRoot(c *C) { c.Assert(r.Timestamp(), IsNil) c.Assert(r.Commit(), IsNil) } + +// Regression test: Do not omit length in target metadata files. +func (rs *RepoSuite) TestTargetMetadataLength(c *C) { + files := map[string][]byte{"foo.txt": []byte("")} + local := MemoryStore(make(map[string]json.RawMessage), files) + r, err := NewRepo(local) + c.Assert(err, IsNil) + + // Init should create targets.json, but not signed yet + r.Init(false) + + genKey(c, r, "root") + genKey(c, r, "targets") + genKey(c, r, "snapshot") + genKey(c, r, "timestamp") + c.Assert(r.AddTarget("foo.txt", nil), IsNil) + c.Assert(r.Snapshot(), IsNil) + c.Assert(r.Timestamp(), IsNil) + c.Assert(r.Commit(), IsNil) + + // Check length field of foo.txt exists. + meta, err := local.GetMeta() + c.Assert(err, IsNil) + targetsJSON, ok := meta["targets.json"] + if !ok { + c.Fatal("missing targets metadata") + } + s := &data.Signed{} + c.Assert(json.Unmarshal(targetsJSON, s), IsNil) + fmt.Fprintf(os.Stderr, string(s.Signed)) + var objMap map[string]json.RawMessage + c.Assert(json.Unmarshal(s.Signed, &objMap), IsNil) + targetsMap, ok := objMap["targets"] + if !ok { + c.Fatal("missing targets field in targets metadata") + } + c.Assert(json.Unmarshal(targetsMap, &objMap), IsNil) + targetsMap, ok = objMap["foo.txt"] + if !ok { + c.Fatal("missing foo.txt in targets") + } + c.Assert(json.Unmarshal(targetsMap, &objMap), IsNil) + targetsMap, ok = objMap["length"] + if !ok { + c.Fatal("missing length field in foo.txt file meta") + } +} diff --git a/util/util.go b/util/util.go index 7ccb0d32..0878c823 100644 --- a/util/util.go +++ b/util/util.go @@ -247,8 +247,9 @@ func GenerateSnapshotFileMeta(r io.Reader, hashAlgorithms ...string) (data.Snaps return data.SnapshotFileMeta{}, err } return data.SnapshotFileMeta{ - FileMeta: m, - Version: v, + Length: m.Length, + Hashes: m.Hashes, + Version: v, }, nil } @@ -268,8 +269,9 @@ func GenerateTimestampFileMeta(r io.Reader, hashAlgorithms ...string) (data.Time return data.TimestampFileMeta{}, err } return data.TimestampFileMeta{ - FileMeta: m, - Version: v, + Length: m.Length, + Hashes: m.Hashes, + Version: v, }, nil } diff --git a/util/util_test.go b/util/util_test.go index 2e687c46..ca691efe 100644 --- a/util/util_test.go +++ b/util/util_test.go @@ -118,10 +118,8 @@ func (UtilSuite) TestSnapshotFileMetaEqual(c *C) { fileMeta := func(version int64, length int64, hashes map[string]string) data.SnapshotFileMeta { return data.SnapshotFileMeta{ - FileMeta: data.FileMeta{ - Length: length, - Hashes: makeHashes(c, hashes), - }, + Length: length, + Hashes: makeHashes(c, hashes), Version: version, } } @@ -158,9 +156,10 @@ func (UtilSuite) TestSnapshotFileMetaEqual(c *C) { } for _, t := range testMetaFileCases(c) { - actual := data.SnapshotFileMeta{FileMeta: t.actual} - expected := data.SnapshotFileMeta{FileMeta: t.expected} - c.Assert(SnapshotFileMetaEqual(actual, expected), DeepEquals, t.err(t), Commentf("name = %s", t.name)) + actual := data.SnapshotFileMeta{Length: t.actual.Length, Hashes: t.actual.Hashes} + expected := data.SnapshotFileMeta{Length: t.expected.Length, Hashes: t.expected.Hashes} + c.Assert(SnapshotFileMetaEqual(actual, expected), DeepEquals, t.err(t), Commentf("name = %s %d %d", t.name, t.actual.Length, t.expected.Length)) + c.Assert(FileMetaEqual(t.actual, t.expected), DeepEquals, t.err(t), Commentf("name = %s", t.name)) } }