making GetTargetsByName work with delegations

Signed-off-by: David Lawrence <david.lawrence@docker.com> (github: endophage)

client/client.go

@@ -440,8 +440,13 @@ func (r *NotaryRepository) ListTargets(roles ...string) ([]*Target, error) {
 	return targetList, nil
 }
 
-// GetTargetByName returns a target given a name
-func (r *NotaryRepository) GetTargetByName(name string) (*Target, error) {
+// GetTargetByName returns a target given a name. If no roles are passed
+// it uses the targets role and does a search of the entire delegation
+// graph, finding the first entry in a breadth first search of the delegations.
+// If roles are passed, they should be passed in ascending priority and
+// the target entry found in the subtree of the highest priority role
+// will be returned
+func (r *NotaryRepository) GetTargetByName(name string, roles ...string) (*Target, error) {
 	c, err := r.bootstrapClient()
 	if err != nil {
 		return nil, err
@@ -455,11 +460,25 @@ func (r *NotaryRepository) GetTargetByName(name string) (*Target, error) {
 		return nil, err
 	}
 
-	meta, err := c.TargetMeta(name)
+	if len(roles) == 0 {
+		roles = append(roles, data.CanonicalTargetsRole)
+	}
+	var (
+		meta *data.FileMeta
+	)
+	for i := len(roles) - 1; i >= 0; i-- {
+		meta, err = c.TargetMeta(roles[i], name)
+		if err != nil {
+			// important to error here otherwise there might be malicious
+			// behaviour that prevents a legitimate version of a target
+			// being found
+			return nil, err
+		} else if meta != nil {
+			break
+		}
+	}
 	if meta == nil {
 		return nil, fmt.Errorf("No trust data for %s", name)
-	} else if err != nil {
-		return nil, err
 	}
 
 	return &Target{Name: name, Hashes: meta.Hashes, Length: meta.Length}, nil

client/client_root_validation_test.go

@@ -17,7 +17,7 @@ var passphraseRetriever = func(string, string, bool, int) (string, bool, error)
 // sure the repository looks correct on disk.
 // We test this with both an RSA and ECDSA root key
 func TestValidateRoot(t *testing.T) {
-	logrus.SetLevel(logrus.DebugLevel)
+	logrus.SetLevel(logrus.ErrorLevel)
 	validateRootSuccessfully(t, data.ECDSAKey)
 	if !testing.Short() {
 		validateRootSuccessfully(t, data.RSAKey)

client/client_test.go

@@ -10,6 +10,7 @@ import (
 	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"reflect"
 	"sort"
 	"strings"
 	"testing"
@@ -748,9 +749,11 @@ func TestRemoveTargetErrorWritingChanges(t *testing.T) {
 func TestListTarget(t *testing.T) {
 	testListEmptyTargets(t, data.ECDSAKey)
 	testListTarget(t, data.ECDSAKey)
+	testListTargetWithDelegates(t, data.ECDSAKey)
 	if !testing.Short() {
 		testListEmptyTargets(t, data.RSAKey)
 		testListTarget(t, data.RSAKey)
+		testListTargetWithDelegates(t, data.RSAKey)
 	}
 }
 
@@ -795,6 +798,14 @@ func fakeServerData(t *testing.T, repo *NotaryRepository, mux *http.ServeMux,
 		"targets", data.DefaultExpires("targets"))
 	assert.NoError(t, err)
 
+	signedLevel1, err := savedTUFRepo.SignTargets(
+		"targets/level1",
+		data.DefaultExpires(data.CanonicalTargetsRole),
+	)
+	if _, ok := savedTUFRepo.Targets["targets/level1"]; ok {
+		assert.NoError(t, err)
+	}
+
 	signedSnapshot, err := savedTUFRepo.SignSnapshot(
 		data.DefaultExpires("snapshot"))
 	assert.NoError(t, err)
@@ -829,14 +840,9 @@ func fakeServerData(t *testing.T, repo *NotaryRepository, mux *http.ServeMux,
 
 	mux.HandleFunc("/v2/docker.com/notary/_trust/tuf/targets/level1.json",
 		func(w http.ResponseWriter, r *http.Request) {
-			signedTargets, err := savedTUFRepo.SignTargets(
-				"targets/level1",
-				data.DefaultExpires(data.CanonicalTargetsRole),
-			)
+			level1JSON, err := json.Marshal(signedLevel1)
 			assert.NoError(t, err)
-			targetsJSON, err := json.Marshal(signedTargets)
-			assert.NoError(t, err)
-			fmt.Fprint(w, string(targetsJSON))
+			fmt.Fprint(w, string(level1JSON))
 		})
 }
 
@@ -921,24 +927,18 @@ func testListTargetWithDelegates(t *testing.T, rootType string) {
 	err = repo.tufRepo.InitTimestamp()
 	assert.NoError(t, err, "error creating repository: %s", err)
 
+	latestTarget := addTarget(t, repo, "latest", "../fixtures/intermediate-ca.crt")
+	addTarget(t, repo, "current", "../fixtures/intermediate-ca.crt")
+
 	// setup delegated targets/level1 role
 	k, err := repo.CryptoService.Create("targets/level1", rootType)
 	assert.NoError(t, err)
 	r, err := data.NewRole("targets/level1", 1, []string{k.ID()}, nil, nil)
 	assert.NoError(t, err)
 	repo.tufRepo.UpdateDelegations(r, []data.PublicKey{k})
-
-	latestTarget := addTarget(t, repo, "latest", "../fixtures/intermediate-ca.crt")
-	addTarget(t, repo, "current", "../fixtures/intermediate-ca.crt")
-
 	delegatedTarget := addTarget(t, repo, "current", "../fixtures/root-ca.crt", "targets/level1")
 	otherTarget := addTarget(t, repo, "other", "../fixtures/root-ca.crt", "targets/level1")
 
-	_, ok := repo.tufRepo.Targets["targets/level1"].Signed.Targets["current"]
-	assert.True(t, ok)
-	_, ok = repo.tufRepo.Targets["targets/level1"].Signed.Targets["other"]
-	assert.True(t, ok)
-
 	// Apply the changelist. Normally, this would be done by Publish
 
 	// load the changelist for this repo
@@ -950,6 +950,11 @@ func testListTargetWithDelegates(t *testing.T, rootType string) {
 	err = applyChangelist(repo.tufRepo, cl)
 	assert.NoError(t, err, "could not apply changelist")
 
+	_, ok := repo.tufRepo.Targets["targets/level1"].Signed.Targets["current"]
+	assert.True(t, ok)
+	_, ok = repo.tufRepo.Targets["targets/level1"].Signed.Targets["other"]
+	assert.True(t, ok)
+
 	fakeServerData(t, repo, mux, keys)
 
 	targets, err := repo.ListTargets(data.CanonicalTargetsRole, "targets/level1")
@@ -970,13 +975,13 @@ func testListTargetWithDelegates(t *testing.T, rootType string) {
 	assert.NoError(t, err)
 	assert.Equal(t, latestTarget, newLatestTarget, "latest target does not match")
 
-	newCurrentTarget, err := repo.GetTargetByName("current")
+	newCurrentTarget, err := repo.GetTargetByName("current", "targets", "targets/level1")
 	assert.NoError(t, err)
 	assert.Equal(t, delegatedTarget, newCurrentTarget, "current target does not match")
 
-	newOtherTarget, err := repo.GetTargetByName("current")
+	newOtherTarget, err := repo.GetTargetByName("other")
 	assert.NoError(t, err)
-	assert.Equal(t, otherTarget, newOtherTarget, "current target does not match")
+	assert.True(t, reflect.DeepEqual(otherTarget, newOtherTarget), "other target does not match")
 }
 
 // TestValidateRootKey verifies that the public data in root.json for the root

client/helpers.go

@@ -37,10 +37,11 @@ func applyChangelist(repo *tuf.Repo, cl changelist.Changelist) error {
 		if err != nil {
 			return err
 		}
-		switch c.Scope() {
-		case changelist.ScopeTargets:
+		isDel := data.IsDelegation(c.Scope())
+		switch {
+		case c.Scope() == changelist.ScopeTargets || isDel:
 			err = applyTargetsChange(repo, c)
-		case changelist.ScopeRoot:
+		case c.Scope() == changelist.ScopeRoot:
 			err = applyRootChange(repo, c)
 		default:
 			logrus.Debug("scope not supported: ", c.Scope())
@@ -133,6 +134,9 @@ func changeTargetMeta(repo *tuf.Repo, c changelist.Change) error {
 		}
 		files := data.Files{c.Path(): *meta}
 		_, err = repo.AddTargets(c.Scope(), files)
+		if err != nil {
+			logrus.Errorf("couldn't add target to %s: %s", c.Scope(), err.Error())
+		}
 	case changelist.ActionDelete:
 		logrus.Debug("changelist remove: ", c.Path())
 		err = repo.RemoveTargets(c.Scope(), c.Path())

tuf/client/client.go

@@ -520,7 +520,7 @@ func (c Client) RoleTargetsPath(role string, hashSha256 string, consistent bool)
 
 // TargetMeta ensures the repo is up to date. It assumes downloadTargets
 // has already downloaded all delegated roles
-func (c Client) TargetMeta(path string) (*data.FileMeta, error) {
+func (c Client) TargetMeta(role, path string) (*data.FileMeta, error) {
 	c.Update()
 	var meta *data.FileMeta
 
@@ -528,14 +528,14 @@ func (c Client) TargetMeta(path string) (*data.FileMeta, error) {
 	pathHex := hex.EncodeToString(pathDigest[:])
 
 	// FIFO list of targets delegations to inspect for target
-	roles := []string{data.ValidRoles["targets"]}
-	var role string
+	roles := []string{role}
+	var curr string
 	for len(roles) > 0 {
 		// have to do these lines here because of order of execution in for statement
-		role = roles[0]
+		curr = roles[0]
 		roles = roles[1:]
 
-		meta = c.local.TargetMeta(role, path)
+		meta = c.local.TargetMeta(curr, path)
 		if meta != nil {
 			// we found the target!
 			return meta, nil

tuf/client/client_test.go

@@ -336,14 +336,45 @@ func TestDownloadTargetsDeepHappy(t *testing.T) {
 	remoteStorage := store.NewMemoryStore(nil, nil)
 	client := NewClient(repo, remoteStorage, kdb, localStorage)
 
-	k, err := cs.Create("targets/level1", data.ED25519Key)
-	assert.NoError(t, err)
-	r, err := data.NewRole("targets/level1", 1, []string{k.ID()}, nil, nil)
-	assert.NoError(t, err)
+	delegations := []string{
+		// left subtree
+		"targets/level1",
+		"targets/level1/a",
+		"targets/level1/a/i",
+		"targets/level1/a/ii",
+		"targets/level1/a/iii",
+		// right subtree
+		"targets/level2",
+		"targets/level2/b",
+		"targets/level2/b/i",
+		"targets/level2/b/i/0",
+		"targets/level2/b/i/1",
+	}
 
-	repo.UpdateDelegations(r, []data.PublicKey{k})
-	repo.InitTargets("targets/level1")
+	for _, r := range delegations {
+		// create role
+		k, err := cs.Create(r, data.ED25519Key)
+		assert.NoError(t, err)
+		role, err := data.NewRole(r, 1, []string{k.ID()}, nil, nil)
+		assert.NoError(t, err)
 
+		// add role to repo
+		repo.UpdateDelegations(role, []data.PublicKey{k})
+		repo.InitTargets(r)
+	}
+
+	// can only sign after adding all delegations
+	for _, r := range delegations {
+		// serialize and store role
+		signedOrig, err := repo.SignTargets(r, data.DefaultExpires("targets"))
+		assert.NoError(t, err)
+		orig, err := json.Marshal(signedOrig)
+		assert.NoError(t, err)
+		err = remoteStorage.SetMeta(r, orig)
+		assert.NoError(t, err)
+	}
+
+	// serialize and store targets after adding all delegations
 	signedOrig, err := repo.SignTargets("targets", data.DefaultExpires("targets"))
 	assert.NoError(t, err)
 	orig, err := json.Marshal(signedOrig)
@@ -351,26 +382,26 @@ func TestDownloadTargetsDeepHappy(t *testing.T) {
 	err = remoteStorage.SetMeta("targets", orig)
 	assert.NoError(t, err)
 
-	signedOrig, err = repo.SignTargets("targets/level1", data.DefaultExpires("targets"))
-	assert.NoError(t, err)
-	orig, err = json.Marshal(signedOrig)
-	assert.NoError(t, err)
-	err = remoteStorage.SetMeta("targets/level1", orig)
-	assert.NoError(t, err)
-
 	// call repo.SignSnapshot to update the targets role in the snapshot
 	repo.SignSnapshot(data.DefaultExpires("snapshot"))
 
 	delete(repo.Targets, "targets")
-	delete(repo.Targets, "targets/level1")
+	for _, r := range delegations {
+		delete(repo.Targets, r)
+		_, ok := repo.Targets[r]
+		assert.False(t, ok)
+	}
 
 	err = client.downloadTargets("targets")
 	assert.NoError(t, err)
 
 	_, ok := repo.Targets["targets"]
 	assert.True(t, ok)
-	_, ok = repo.Targets["targets/level1"]
-	assert.True(t, ok)
+
+	for _, r := range delegations {
+		_, ok = repo.Targets[r]
+		assert.True(t, ok)
+	}
 }
 
 func TestDownloadTargetChecksumMismatch(t *testing.T) {

tuf/data/roles.go

@@ -173,6 +173,9 @@ func (r Role) ValidKey(id string) bool {
 
 // CheckPaths checks if a given path is valid for the role
 func (r Role) CheckPaths(path string) bool {
+	if len(r.Paths) == 0 {
+		return true
+	}
 	for _, p := range r.Paths {
 		if strings.HasPrefix(path, p) {
 			return true
@@ -183,6 +186,9 @@ func (r Role) CheckPaths(path string) bool {
 
 // CheckPrefixes checks if a given hash matches the prefixes for the role
 func (r Role) CheckPrefixes(hash string) bool {
+	if len(r.PathHashPrefixes) == 0 {
+		return true
+	}
 	for _, p := range r.PathHashPrefixes {
 		if strings.HasPrefix(hash, p) {
 			return true

tuf/data/targets.go

@@ -3,6 +3,7 @@ package data
 import (
 	"crypto/sha256"
 	"encoding/hex"
+	"errors"
 
 	"github.com/jfrazelle/go/canonical/json"
 )
@@ -88,7 +89,7 @@ func (t *SignedTargets) AddTarget(path string, meta FileMeta) {
 // ensuring the keys either already exist, or are added to the map
 // of delegation keys
 func (t *SignedTargets) AddDelegation(role *Role, keys []*PublicKey) error {
-	return nil
+	return errors.New("Not Implemented")
 }
 
 // ToSigned partially serializes a SignedTargets for further signing

tuf/tuf.go

@@ -566,6 +566,12 @@ func (tr *Repo) SignRoot(expires time.Time) (*data.Signed, error) {
 // SignTargets signs the targets file for the given top level or delegated targets role
 func (tr *Repo) SignTargets(role string, expires time.Time) (*data.Signed, error) {
 	logrus.Debugf("sign targets called for role %s", role)
+	if _, ok := tr.Targets[role]; !ok {
+		return nil, data.ErrInvalidRole{
+			Role:   role,
+			Reason: "SignTargets called with non-existant targets role",
+		}
+	}
 	tr.Targets[role].Signed.Expires = expires
 	tr.Targets[role].Signed.Version++
 	signed, err := tr.Targets[role].ToSigned()