From c3c24be66bda44f970abcdad10af2ed82c92f002 Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Fri, 12 Feb 2016 11:39:40 -0800 Subject: [PATCH] remove path restriction from getting delegations, use in future visitor Signed-off-by: Riyaz Faizullabhoy --- tuf/data/roles.go | 14 ++------------ tuf/tuf.go | 12 ++---------- tuf/tuf_test.go | 30 +++++++++++++++++++++--------- 3 files changed, 25 insertions(+), 31 deletions(-) diff --git a/tuf/data/roles.go b/tuf/data/roles.go index b1ab6ff3e3..50249d870a 100644 --- a/tuf/data/roles.go +++ b/tuf/data/roles.go @@ -69,16 +69,6 @@ func ValidRole(name string) bool { return false } -// IsBaseRole checks if a role name is valid base role -func IsBaseRole(name string) bool { - for _, v := range BaseRoles { - if name == v { - return true - } - } - return false -} - // IsDelegation checks if the role is a delegation or a root role func IsDelegation(role string) bool { targetsBase := CanonicalTargetsRole + "/" @@ -153,9 +143,9 @@ func listKeyIDs(keyMap map[string]PublicKey) []string { return keyIDs } -// RestrictChild restricts the paths and path hash prefixes for the passed in delegation role, +// Restrict restricts the paths and path hash prefixes for the passed in delegation role, // returning a copy of the role with validated paths as if it was a direct child -func (d DelegationRole) RestrictChild(child DelegationRole) (DelegationRole, error) { +func (d DelegationRole) Restrict(child DelegationRole) (DelegationRole, error) { if !d.IsParentOf(child) { return DelegationRole{}, fmt.Errorf("%s is not a parent of %s", d.Name, child.Name) } diff --git a/tuf/tuf.go b/tuf/tuf.go index be9ef0413f..534eb45a1e 100644 --- a/tuf/tuf.go +++ b/tuf/tuf.go @@ -175,7 +175,7 @@ func (tr *Repo) RemoveBaseKeys(role string, keyIDs ...string) error { // GetBaseRole gets a base role from this repo's metadata func (tr *Repo) GetBaseRole(name string) (data.BaseRole, error) { - if !data.IsBaseRole(name) { + if !data.ValidRole(name) && !data.IsDelegation(name) { return data.BaseRole{}, data.ErrInvalidRole{Role: name, Reason: "invalid base role name"} } if tr.Root == nil { @@ -222,10 +222,6 @@ func (tr *Repo) GetDelegationRole(name string) (data.DelegationRole, error) { return data.DelegationRole{}, ErrNotLoaded{data.CanonicalTargetsRole} } - // Keep track of paths and path hash prefixes to restrict as we traverse the delegations tree, targets implicitly has "" - whiteListedPaths := []string{""} - whiteListedPathHashes := []string{""} - // Start with top level roles in targets delegationRoles := signedTargetData.Signed.Delegations.Roles var foundRole *data.Role @@ -233,10 +229,8 @@ func (tr *Repo) GetDelegationRole(name string) (data.DelegationRole, error) { delgRole := delegationRoles[0] delegationRoles = delegationRoles[1:] - // If this role is delegated above or is our desired role, restrict paths and traverse its child roles + // If this role is delegated above or is our desired role, traverse it if delgRole.Name == name || strings.HasPrefix(name, delgRole.Name+"/") { - delgRole.Paths = data.RestrictDelegationPathPrefixes(whiteListedPaths, delgRole.Paths) - delgRole.PathHashPrefixes = data.RestrictDelegationPathPrefixes(whiteListedPathHashes, delgRole.PathHashPrefixes) // If we found the role, we can exit the loop if delgRole.Name == name { @@ -245,8 +239,6 @@ func (tr *Repo) GetDelegationRole(name string) (data.DelegationRole, error) { } // If this is a parent role, keep traversing - whiteListedPaths = delgRole.Paths - whiteListedPathHashes = delgRole.PathHashPrefixes if delegationMeta, ok := tr.Targets[delgRole.Name]; ok { delegationRoles = append(delegationRoles, delegationMeta.Signed.Delegations.Roles...) } diff --git a/tuf/tuf_test.go b/tuf/tuf_test.go index 42ec8ec642..f7c97b5930 100644 --- a/tuf/tuf_test.go +++ b/tuf/tuf_test.go @@ -1096,10 +1096,10 @@ func TestGetDelegationRolesInvalidPaths(t *testing.T) { err = repo.UpdateDelegations(role, data.KeyList{testKey2}) assert.NoError(t, err) - // Getting this delegation should fail path verification, so it'll have 0 paths + // Getting this delegation does not actually restrict paths, unless we use the RestrictChild method delgRole, err := repo.GetDelegationRole("targets/test/b") assert.NoError(t, err) - assert.Empty(t, delgRole.Paths) + assert.Contains(t, delgRole.Paths, "invalidpath") delgRole, err = repo.GetDelegationRole("targets/test") assert.NoError(t, err) @@ -1131,10 +1131,10 @@ func TestGetDelegationRolesInvalidPathHashPrefix(t *testing.T) { err = repo.UpdateDelegations(role, data.KeyList{testKey2}) assert.NoError(t, err) - // Getting this delegation should fail path hash verification, so it'll be empty + // Getting this delegation does not actually restrict paths, unless we use the RestrictChild method delgRole, err := repo.GetDelegationRole("targets/test/b") assert.NoError(t, err) - assert.Empty(t, delgRole.PathHashPrefixes) + assert.Contains(t, delgRole.PathHashPrefixes, "invalidpathhash") delgRole, err = repo.GetDelegationRole("targets/test") assert.NoError(t, err) @@ -1187,7 +1187,7 @@ func TestDelegationRolesParent(t *testing.T) { assert.False(t, delgC.IsParentOf(delgC)) // Check that parents correctly restrict paths - restrictedDelgB, err := delgA.RestrictChild(delgB) + restrictedDelgB, err := delgA.Restrict(delgB) assert.NoError(t, err) assert.Contains(t, restrictedDelgB.Paths, "path/b") assert.Contains(t, restrictedDelgB.Paths, "anotherpath/b") @@ -1196,12 +1196,24 @@ func TestDelegationRolesParent(t *testing.T) { assert.Contains(t, restrictedDelgB.PathHashPrefixes, "anotherpathhash/b") assert.NotContains(t, restrictedDelgB.PathHashPrefixes, "b/invalidpathhash") - _, err = delgB.RestrictChild(delgA) + _, err = delgB.Restrict(delgA) assert.Error(t, err) - _, err = delgA.RestrictChild(delgC) + _, err = delgA.Restrict(delgC) assert.Error(t, err) - _, err = delgC.RestrictChild(delgB) + _, err = delgC.Restrict(delgB) assert.Error(t, err) - _, err = delgC.RestrictChild(delgA) + _, err = delgC.Restrict(delgA) assert.Error(t, err) + + // Make delgA have no paths and check that it changes delgB and delgC accordingly when chained + delgA.Paths = []string{} + delgA.PathHashPrefixes = []string{} + restrictedDelgB, err = delgA.Restrict(delgB) + assert.NoError(t, err) + assert.Empty(t, restrictedDelgB.Paths) + assert.Empty(t, restrictedDelgB.PathHashPrefixes) + restrictedDelgC, err := restrictedDelgB.Restrict(delgC) + assert.NoError(t, err) + assert.Empty(t, restrictedDelgC.Paths) + assert.Empty(t, restrictedDelgC.PathHashPrefixes) }