remove path restriction from getting delegations, use in future visitor

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

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)
 	}

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...)
 			}

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)
 }