fix: require length and hashes for target metadata (#345)

* fix: require length and hashes Signed-off-by: Asra Ali <asraa@google.com> * update pr and add test Signed-off-by: Asra Ali <asraa@google.com> * Fix lint Signed-off-by: Asra Ali <asraa@google.com> * make metapathfilemeta private Signed-off-by: Asra Ali <asraa@google.com>
theupdateframework · Jul 20, 2022 · 2e6c621 · 2e6c621
1 parent af3c7d6
commit 2e6c621
Show file tree

Hide file tree

Showing 6 changed files with 89 additions and 41 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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))
 	}
 }