Skip to content

Commit 9631aeb

Browse files
raphaelgavacheraphaelgavache
authored
Update after reviews
* Add back lower case check * Initialize with size and comment * Simplify iterator initialization
1 parent ea6bcf8 commit 9631aeb

File tree

5 files changed

+54
-84
lines changed

5 files changed

+54
-84
lines changed

‎client/delegations.go‎

Lines changed: 22 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,16 @@ func (c *Client) getTargetFileMeta(file string) (data.TargetFileMeta, error) {
1414
return data.TargetFileMeta{}, err
1515
}
1616

17-
verifiers := map[string]verify.DelegationsVerifier{
18-
"root": {DB: c.db},
19-
}
17+
// verifiers is map of parent targets name to an associated DelegationsVerifier
18+
// that can verify all child targets pointed by delegatedRoles in the parent targets
19+
verifiers := make(map[string]verify.DelegationsVerifier)
2020

2121
// delegationsIterator covers 5.6.7
2222
// - pre-order depth-first search starting with the top targets
2323
// - filter delegations with paths or path_hash_prefixes matching searched file
2424
// - 5.6.7.1 cycles protection
2525
// - 5.6.7.2 terminations
26-
delegations := newDelegationsIterator(c.rootTargetDelegation(), "root", file)
26+
delegations := newDelegationsIterator(file)
2727
for i := 0; i < c.MaxDelegations; i++ {
2828
d, ok := delegations.next()
2929
if !ok {
@@ -80,7 +80,7 @@ func (c *Client) loadLocalSnapshot() (*data.Snapshot, error) {
8080
return snapshot, nil
8181
}
8282

83-
// loadDelegatedTargets downloads, decodes, verifies and stores delegated targets
83+
// loadDelegatedTargets downloads, decodes, verifies and stores targets
8484
func (c *Client) loadDelegatedTargets(snapshot *data.Snapshot, role string, verifier verify.DelegationsVerifier) (*data.Targets, error) {
8585
var err error
8686
fileName := role + ".json"
@@ -102,9 +102,16 @@ func (c *Client) loadDelegatedTargets(snapshot *data.Snapshot, role string, veri
102102
target := &data.Targets{}
103103
// 5.6.3 verify signature with parent public keys
104104
// 5.6.5 verify that the targets is not expired
105-
if err := verifier.Unmarshal(raw, target, role, fileMeta.Version); err != nil {
105+
// role "targets" is the topTargets verified by root roles loaded in the client db
106+
if role == "targets" {
107+
err = c.db.Unmarshal(raw, target, role, fileMeta.Version)
108+
} else {
109+
err = verifier.Unmarshal(raw, target, role, fileMeta.Version)
110+
}
111+
if err != nil {
106112
return nil, ErrDecodeFailed{fileName, err}
107113
}
114+
108115
// 5.6.4 check against snapshot version
109116
if target.Version != fileMeta.Version {
110117
return nil, ErrTargetsSnapshotVersionMismatch{
@@ -123,27 +130,6 @@ func (c *Client) loadDelegatedTargets(snapshot *data.Snapshot, role string, veri
123130
return target, nil
124131
}
125132

126-
func (c *Client) rootTargetDelegation() data.DelegatedRole {
127-
role := "targets"
128-
r := c.db.GetRole(role)
129-
if r == nil {
130-
return data.DelegatedRole{}
131-
}
132-
133-
keyIDs := make([]string, 0, len(r.KeyIDs))
134-
for id, _ := range r.KeyIDs {
135-
keyIDs = append(keyIDs, id)
136-
}
137-
138-
// root delegates the signing of all files to the top level targets
139-
return data.DelegatedRole{
140-
Name: role,
141-
KeyIDs: keyIDs,
142-
Threshold: r.Threshold,
143-
PathHashPrefixes: []string{""},
144-
}
145-
}
146-
147133
type delegation struct {
148134
parent string
149135
child data.DelegatedRole
@@ -160,15 +146,18 @@ type delegationsIterator struct {
160146
visited map[delegationID]struct{}
161147
}
162148

163-
func newDelegationsIterator(role data.DelegatedRole, parent string, file string) *delegationsIterator {
149+
// newDelegationsIterator initialises an iterator with a first step
150+
// on top level targets
151+
func newDelegationsIterator(file string) *delegationsIterator {
164152
i := &delegationsIterator{
165-
file: file,
166-
stack: make([]delegation, 0, 1),
153+
file: file,
154+
stack: []delegation{
155+
{
156+
child: data.DelegatedRole{Name: "targets"},
157+
},
158+
},
167159
visited: make(map[delegationID]struct{}),
168160
}
169-
170-
i.add([]data.DelegatedRole{role}, parent)
171-
172161
return i
173162
}
174163

‎client/delegations_test.go‎

Lines changed: 26 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,15 @@ var (
2323

2424
func TestDelegationsIterator(t *testing.T) {
2525
var iteratorTests = []struct {
26-
testName string
27-
roles map[string][]data.DelegatedRole
28-
rootDelegation data.DelegatedRole
29-
file string
30-
resultOrder []string
26+
testName string
27+
roles map[string][]data.DelegatedRole
28+
file string
29+
resultOrder []string
3130
}{
3231
{
3332
testName: "no termination",
3433
roles: map[string][]data.DelegatedRole{
35-
"a": {
34+
"targets": {
3635
{Name: "b", Paths: defaultPathPatterns},
3736
{Name: "e", Paths: defaultPathPatterns},
3837
},
@@ -52,14 +51,13 @@ func TestDelegationsIterator(t *testing.T) {
5251
{Name: "j", Paths: defaultPathPatterns},
5352
},
5453
},
55-
rootDelegation: data.DelegatedRole{Name: "a", Paths: defaultPathPatterns},
56-
file: "",
57-
resultOrder: []string{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j"},
54+
file: "",
55+
resultOrder: []string{"targets", "b", "c", "d", "e", "f", "g", "h", "i", "j"},
5856
},
5957
{
6058
testName: "terminated in b",
6159
roles: map[string][]data.DelegatedRole{
62-
"a": {
60+
"targets": {
6361
{Name: "b", Paths: defaultPathPatterns, Terminating: true},
6462
{Name: "e", Paths: defaultPathPatterns},
6563
},
@@ -68,14 +66,13 @@ func TestDelegationsIterator(t *testing.T) {
6866
{Name: "d", Paths: defaultPathPatterns},
6967
},
7068
},
71-
rootDelegation: data.DelegatedRole{Name: "a", Paths: defaultPathPatterns},
72-
file: "",
73-
resultOrder: []string{"a", "b", "c", "d"},
69+
file: "",
70+
resultOrder: []string{"targets", "b", "c", "d"},
7471
},
7572
{
7673
testName: "path does not match b",
7774
roles: map[string][]data.DelegatedRole{
78-
"a": {
75+
"targets": {
7976
{Name: "b", Paths: noMatchPathPatterns},
8077
{Name: "e", Paths: defaultPathPatterns},
8178
},
@@ -84,50 +81,47 @@ func TestDelegationsIterator(t *testing.T) {
8481
{Name: "d", Paths: defaultPathPatterns},
8582
},
8683
},
87-
rootDelegation: data.DelegatedRole{Name: "a", Paths: defaultPathPatterns},
88-
file: "",
89-
resultOrder: []string{"a", "e"},
84+
file: "",
85+
resultOrder: []string{"targets", "e"},
9086
},
9187
{
9288
testName: "cycle avoided 1",
9389
roles: map[string][]data.DelegatedRole{
94-
"a": {
90+
"targets": {
9591
{Name: "b", Paths: defaultPathPatterns},
9692
{Name: "e", Paths: defaultPathPatterns},
9793
},
9894
"b": {
99-
{Name: "a", Paths: defaultPathPatterns},
95+
{Name: "targets", Paths: defaultPathPatterns},
10096
{Name: "d", Paths: defaultPathPatterns},
10197
},
10298
},
103-
rootDelegation: data.DelegatedRole{Name: "a", Paths: defaultPathPatterns},
104-
file: "",
105-
resultOrder: []string{"a", "b", "a", "e", "d"},
99+
file: "",
100+
resultOrder: []string{"targets", "b", "targets", "e", "d"},
106101
},
107102
{
108103
testName: "cycle avoided 2",
109104
roles: map[string][]data.DelegatedRole{
110-
"a": {
111-
{Name: "a", Paths: defaultPathPatterns},
105+
"targets": {
106+
{Name: "targets", Paths: defaultPathPatterns},
112107
{Name: "b", Paths: defaultPathPatterns},
113108
},
114109
"b": {
115-
{Name: "a", Paths: defaultPathPatterns},
110+
{Name: "targets", Paths: defaultPathPatterns},
116111
{Name: "b", Paths: defaultPathPatterns},
117112
{Name: "c", Paths: defaultPathPatterns},
118113
},
119114
"c": {
120115
{Name: "c", Paths: defaultPathPatterns},
121116
},
122117
},
123-
rootDelegation: data.DelegatedRole{Name: "a", Paths: defaultPathPatterns},
124-
file: "",
125-
resultOrder: []string{"a", "a", "b", "a", "b", "c", "c"},
118+
file: "",
119+
resultOrder: []string{"targets", "targets", "b", "targets", "b", "c", "c"},
126120
},
127121
{
128122
testName: "diamond delegation",
129123
roles: map[string][]data.DelegatedRole{
130-
"a": {
124+
"targets": {
131125
{Name: "b", Paths: defaultPathPatterns},
132126
{Name: "c", Paths: defaultPathPatterns},
133127
},
@@ -138,15 +132,14 @@ func TestDelegationsIterator(t *testing.T) {
138132
{Name: "d", Paths: defaultPathPatterns},
139133
},
140134
},
141-
rootDelegation: data.DelegatedRole{Name: "a", Paths: defaultPathPatterns},
142-
file: "",
143-
resultOrder: []string{"a", "b", "d", "c", "d"},
135+
file: "",
136+
resultOrder: []string{"targets", "b", "d", "c", "d"},
144137
},
145138
}
146139

147140
for _, tt := range iteratorTests {
148141
t.Run(tt.testName, func(t *testing.T) {
149-
d := newDelegationsIterator(tt.rootDelegation, "root", tt.file)
142+
d := newDelegationsIterator(tt.file)
150143
var iterationOrder []string
151144
for {
152145
r, ok := d.next()
@@ -234,20 +227,6 @@ func TestTargetsNotFound(t *testing.T) {
234227
assert.Equal(t, ErrMissingRemoteMetadata{Name: "c.json"}, err)
235228
}
236229

237-
func TestRootDelegationMatchesAll(t *testing.T) {
238-
c := &Client{db: verify.NewDB()}
239-
c.db.AddRole("targets", &data.Role{Threshold: 1})
240-
d := c.rootTargetDelegation()
241-
242-
matchesPath, err := d.MatchesPath("a.txt")
243-
assert.NoError(t, err)
244-
assert.True(t, matchesPath)
245-
246-
matchesPath, err = d.MatchesPath("var/b//g")
247-
assert.NoError(t, err)
248-
assert.True(t, matchesPath)
249-
}
250-
251230
func TestUnverifiedTargets(t *testing.T) {
252231
verify.IsExpired = func(t time.Time) bool { return false }
253232
c, closer := initTestDelegationClient(t, "testdata/php-tuf-fixtures/TUFTestFixture3LevelDelegation")

‎data/types.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,8 @@ func (d *DelegatedRole) MatchesPath(file string) (bool, error) {
262262
return false, nil
263263
}
264264

265-
// validateFields enforces the spec 1.0.19 section 4.5:
265+
// validateFields enforces the spec
266+
// https://theupdateframework.github.io/specification/v1.0.19/index.html#file-formats-targets
266267
// 'role MUST specify only one of the "path_hash_prefixes" or "paths"'
267268
// Marshalling and unmarshalling JSON will fail and return
268269
// ErrPathsAndPathHashesSet if both fields are set and not empty.

‎verify/db.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func (d *DelegationsVerifier) Unmarshal(b []byte, v interface{}, role string, mi
4040
func NewDelegationsVerifier(d *data.Delegations) (DelegationsVerifier, error) {
4141
db := &DB{
4242
roles: make(map[string]*Role, len(d.Roles)),
43-
keys: make(map[string]*data.Key),
43+
keys: make(map[string]*data.Key, len(d.Keys)),
4444
}
4545
for _, r := range d.Roles {
4646
role := &data.Role{Threshold: r.Threshold, KeyIDs: r.KeyIDs}

‎verify/verify.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package verify
22

33
import (
44
"encoding/json"
5+
"strings"
56
"time"
67

78
cjson "github.com/tent/canonical-json-go"
@@ -27,12 +28,12 @@ func (db *DB) Verify(s *data.Signed, role string, minVersion int) error {
2728
if isTopLevelRole(role) {
2829
// Top-level roles can only sign metadata of the same type (e.g. snapshot
2930
// metadata must be signed by the snapshot role).
30-
if sm.Type != role {
31+
if strings.ToLower(sm.Type) != strings.ToLower(role) {
3132
return ErrWrongMetaType
3233
}
3334
} else {
3435
// Delegated (non-top-level) roles may only sign targets metadata.
35-
if sm.Type != "targets" {
36+
if strings.ToLower(sm.Type) != "targets" {
3637
return ErrWrongMetaType
3738
}
3839
}

0 commit comments

Comments
 (0)