From 5846e6e5d5bea128960d282738dff211b4d36d0f Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Thu, 24 Aug 2017 15:42:21 -0700 Subject: [PATCH] trust: update existing code for new vendoring, refactor for docker trust code sharing Signed-off-by: Riyaz Faizullabhoy --- cli/command/image/push.go | 2 +- cli/command/image/trust.go | 41 ++++++++++++++++++++++----------- cli/command/image/trust_test.go | 27 ++++++++++++++++++++++ cli/trust/trust.go | 8 +++---- 4 files changed, 59 insertions(+), 19 deletions(-) diff --git a/cli/command/image/push.go b/cli/command/image/push.go index cc95897bd0..741ef67c59 100644 --- a/cli/command/image/push.go +++ b/cli/command/image/push.go @@ -48,7 +48,7 @@ func runPush(dockerCli command.Cli, remote string) error { requestPrivilege := command.RegistryAuthenticationPrivilegedFunc(dockerCli, repoInfo.Index, "push") if command.IsTrusted() { - return trustedPush(ctx, dockerCli, repoInfo, ref, authConfig, requestPrivilege) + return TrustedPush(ctx, dockerCli, repoInfo, ref, authConfig, requestPrivilege) } responseBody, err := imagePushPrivileged(ctx, dockerCli, authConfig, ref, requestPrivilege) diff --git a/cli/command/image/trust.go b/cli/command/image/trust.go index 8d94164e5a..82bae2eb19 100644 --- a/cli/command/image/trust.go +++ b/cli/command/image/trust.go @@ -28,8 +28,8 @@ type target struct { size int64 } -// trustedPush handles content trust pushing of an image -func trustedPush(ctx context.Context, cli command.Cli, repoInfo *registry.RepositoryInfo, ref reference.Named, authConfig types.AuthConfig, requestPrivilege types.RequestPrivilegeFunc) error { +// TrustedPush handles content trust pushing of an image +func TrustedPush(ctx context.Context, cli command.Cli, repoInfo *registry.RepositoryInfo, ref reference.Named, authConfig types.AuthConfig, requestPrivilege types.RequestPrivilegeFunc) error { responseBody, err := imagePushPrivileged(ctx, cli, authConfig, ref, requestPrivilege) if err != nil { return err @@ -136,7 +136,7 @@ func PushTrustedReference(streams command.Streams, repoInfo *registry.Repository err = repo.AddTarget(target, data.CanonicalTargetsRole) case nil: // already initialized and we have successfully downloaded the latest metadata - err = addTargetToAllSignableRoles(repo, target) + err = AddTargetToAllSignableRoles(repo, target) default: return trust.NotaryError(repoInfo.Name.Name(), err) } @@ -154,12 +154,10 @@ func PushTrustedReference(streams command.Streams, repoInfo *registry.Repository return nil } -// Attempt to add the image target to all the top level delegation roles we can -// (based on whether we have the signing key and whether the role's path allows -// us to). -// If there are no delegation roles, we add to the targets role. -func addTargetToAllSignableRoles(repo *client.NotaryRepository, target *client.Target) error { - var signableRoles []string +// GetSignableRoles returns a list of roles for which we have valid signing +// keys, given a notary repository and a target +func GetSignableRoles(repo *client.NotaryRepository, target *client.Target) ([]data.RoleName, error) { + var signableRoles []data.RoleName // translate the full key names, which includes the GUN, into just the key IDs allCanonicalKeyIDs := make(map[string]struct{}) @@ -169,12 +167,13 @@ func addTargetToAllSignableRoles(repo *client.NotaryRepository, target *client.T allDelegationRoles, err := repo.GetDelegationRoles() if err != nil { - return err + return signableRoles, err } // if there are no delegation roles, then just try to sign it into the targets role if len(allDelegationRoles) == 0 { - return repo.AddTarget(target, data.CanonicalTargetsRole) + signableRoles = append(signableRoles, data.CanonicalTargetsRole) + return signableRoles, nil } // there are delegation roles, find every delegation role we have a key for, and @@ -183,7 +182,7 @@ func addTargetToAllSignableRoles(repo *client.NotaryRepository, target *client.T // We do not support signing any delegation role that isn't a direct child of the targets role. // Also don't bother checking the keys if we can't add the target // to this role due to path restrictions - if path.Dir(delegationRole.Name) != data.CanonicalTargetsRole || !delegationRole.CheckPaths(target.Name) { + if path.Dir(delegationRole.Name.String()) != data.CanonicalTargetsRole.String() || !delegationRole.CheckPaths(target.Name) { continue } @@ -196,7 +195,21 @@ func addTargetToAllSignableRoles(repo *client.NotaryRepository, target *client.T } if len(signableRoles) == 0 { - return errors.Errorf("no valid signing keys for delegation roles") + return signableRoles, errors.Errorf("no valid signing keys for delegation roles") + } + + return signableRoles, nil + +} + +// AddTargetToAllSignableRoles attempts to add the image target to all the top level delegation roles we can +// (based on whether we have the signing key and whether the role's path allows +// us to). +// If there are no delegation roles, we add to the targets role. +func AddTargetToAllSignableRoles(repo *client.NotaryRepository, target *client.Target) error { + signableRoles, err := GetSignableRoles(repo, target) + if err != nil { + return err } return repo.AddTarget(target, signableRoles...) @@ -348,7 +361,7 @@ func TrustedReference(ctx context.Context, cli command.Cli, ref reference.NamedT // Only list tags in the top level targets role or the releases delegation role - ignore // all other delegation roles if t.Role != trust.ReleasesRole && t.Role != data.CanonicalTargetsRole { - return nil, trust.NotaryError(repoInfo.Name.Name(), errors.Errorf("No trust data for %s", ref.Tag())) + return nil, trust.NotaryError(repoInfo.Name.Name(), client.ErrNoSuchTarget(ref.Tag())) } r, err := convertTarget(t.Target) if err != nil { diff --git a/cli/command/image/trust_test.go b/cli/command/image/trust_test.go index 0a57f9b3bf..c080a90e46 100644 --- a/cli/command/image/trust_test.go +++ b/cli/command/image/trust_test.go @@ -1,12 +1,17 @@ package image import ( + "io/ioutil" "os" "testing" "github.com/docker/cli/cli/trust" registrytypes "github.com/docker/docker/api/types/registry" "github.com/docker/docker/registry" + "github.com/docker/notary/client" + "github.com/docker/notary/passphrase" + "github.com/docker/notary/trustpinning" + "github.com/stretchr/testify/assert" ) func unsetENV() { @@ -55,3 +60,25 @@ func TestNonOfficialTrustServer(t *testing.T) { t.Fatalf("Expected server to be %s, got %s", expectedStr, output) } } + +func TestAddTargetToAllSignableRolesError(t *testing.T) { + tmpDir, err := ioutil.TempDir("", "notary-test-") + assert.NoError(t, err) + defer os.RemoveAll(tmpDir) + + notaryRepo, err := client.NewFileCachedNotaryRepository(tmpDir, "gun", "https://localhost", nil, passphrase.ConstantRetriever("password"), trustpinning.TrustPinConfig{}) + target := client.Target{} + err = AddTargetToAllSignableRoles(notaryRepo, &target) + assert.EqualError(t, err, "client is offline") +} + +func TestGetSignableRolesError(t *testing.T) { + tmpDir, err := ioutil.TempDir("", "notary-test-") + assert.NoError(t, err) + defer os.RemoveAll(tmpDir) + + notaryRepo, err := client.NewFileCachedNotaryRepository(tmpDir, "gun", "https://localhost", nil, passphrase.ConstantRetriever("password"), trustpinning.TrustPinConfig{}) + target := client.Target{} + _, err = GetSignableRoles(notaryRepo, &target) + assert.EqualError(t, err, "client is offline") +} diff --git a/cli/trust/trust.go b/cli/trust/trust.go index 13192fe112..cd40c5fc92 100644 --- a/cli/trust/trust.go +++ b/cli/trust/trust.go @@ -33,7 +33,7 @@ import ( var ( // ReleasesRole is the role named "releases" - ReleasesRole = path.Join(data.CanonicalTargetsRole, "releases") + ReleasesRole = data.RoleName(path.Join(data.CanonicalTargetsRole.String(), "releases")) ) func trustDirectory() string { @@ -164,9 +164,9 @@ func GetNotaryRepository(streams command.Streams, repoInfo *registry.RepositoryI modifiers = append(modifiers, auth.NewAuthorizer(challengeManager, tokenHandler, basicHandler)) tr := transport.NewTransport(base, modifiers...) - return client.NewNotaryRepository( + return client.NewFileCachedNotaryRepository( trustDirectory(), - repoInfo.Name.Name(), + data.GUN(repoInfo.Name.Name()), server, tr, getPassphraseRetriever(streams), @@ -193,7 +193,7 @@ func getPassphraseRetriever(streams command.Streams) notary.PassRetriever { return v, numAttempts > 1, nil } // For non-root roles, we can also try the "default" alias if it is specified - if v := env["default"]; v != "" && alias != data.CanonicalRootRole { + if v := env["default"]; v != "" && alias != data.CanonicalRootRole.String() { return v, numAttempts > 1, nil } return baseRetriever(keyName, alias, createNew, numAttempts)