From 45c102a03dc1bcc6ca380288e04e77299d9b49ce Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Fri, 25 Aug 2017 14:49:40 -0700 Subject: [PATCH] trust: address review feedback, refactor to align with existing cli/command semantics Signed-off-by: Riyaz Faizullabhoy --- cli/command/image/trust.go | 51 +---------- cli/command/image/trust_test.go | 2 +- cli/command/trust/cmd.go | 2 +- cli/command/trust/helpers.go | 61 +++++++------ cli/command/trust/helpers_test.go | 3 + cli/command/trust/inspect.go | 76 ++++++++-------- cli/command/trust/inspect_test.go | 96 ++++++++++++++++----- cli/command/trust/revoke.go | 34 +++----- cli/command/trust/revoke_test.go | 4 +- cli/command/trust/sign.go | 43 +++++---- cli/command/trust/sign_test.go | 4 +- cli/trust/trust.go | 48 +++++++++++ docs/reference/commandline/trust_inspect.md | 4 +- docs/reference/commandline/trust_revoke.md | 12 +-- docs/reference/commandline/trust_sign.md | 6 +- 15 files changed, 253 insertions(+), 193 deletions(-) diff --git a/cli/command/image/trust.go b/cli/command/image/trust.go index 82bae2eb19..f667b0d632 100644 --- a/cli/command/image/trust.go +++ b/cli/command/image/trust.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "io" - "path" "sort" "github.com/docker/cli/cli/command" @@ -154,60 +153,12 @@ func PushTrustedReference(streams command.Streams, repoInfo *registry.Repository return nil } -// 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{}) - for fullKeyID := range repo.CryptoService.ListAllKeys() { - allCanonicalKeyIDs[path.Base(fullKeyID)] = struct{}{} - } - - allDelegationRoles, err := repo.GetDelegationRoles() - if err != nil { - return signableRoles, err - } - - // if there are no delegation roles, then just try to sign it into the targets role - if len(allDelegationRoles) == 0 { - signableRoles = append(signableRoles, data.CanonicalTargetsRole) - return signableRoles, nil - } - - // there are delegation roles, find every delegation role we have a key for, and - // attempt to sign into into all those roles. - for _, delegationRole := range allDelegationRoles { - // 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.String()) != data.CanonicalTargetsRole.String() || !delegationRole.CheckPaths(target.Name) { - continue - } - - for _, canonicalKeyID := range delegationRole.KeyIDs { - if _, ok := allCanonicalKeyIDs[canonicalKeyID]; ok { - signableRoles = append(signableRoles, delegationRole.Name) - break - } - } - } - - if len(signableRoles) == 0 { - 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) + signableRoles, err := trust.GetSignableRoles(repo, target) if err != nil { return err } diff --git a/cli/command/image/trust_test.go b/cli/command/image/trust_test.go index c080a90e46..1f2a7401f6 100644 --- a/cli/command/image/trust_test.go +++ b/cli/command/image/trust_test.go @@ -79,6 +79,6 @@ func TestGetSignableRolesError(t *testing.T) { notaryRepo, err := client.NewFileCachedNotaryRepository(tmpDir, "gun", "https://localhost", nil, passphrase.ConstantRetriever("password"), trustpinning.TrustPinConfig{}) target := client.Target{} - _, err = GetSignableRoles(notaryRepo, &target) + _, err = trust.GetSignableRoles(notaryRepo, &target) assert.EqualError(t, err, "client is offline") } diff --git a/cli/command/trust/cmd.go b/cli/command/trust/cmd.go index 8b079dcd0e..d1be0b59ad 100644 --- a/cli/command/trust/cmd.go +++ b/cli/command/trust/cmd.go @@ -10,7 +10,7 @@ import ( func NewTrustCommand(dockerCli command.Cli) *cobra.Command { cmd := &cobra.Command{ Use: "trust", - Short: "Sign images to establish trust", + Short: "Manage trust on Docker images", Args: cli.NoArgs, RunE: command.ShowHelp(dockerCli.Err()), } diff --git a/cli/command/trust/helpers.go b/cli/command/trust/helpers.go index 590000b2ae..e1115398ea 100644 --- a/cli/command/trust/helpers.go +++ b/cli/command/trust/helpers.go @@ -3,7 +3,6 @@ package trust import ( "context" "fmt" - "io" "strings" "github.com/docker/cli/cli/command" @@ -17,26 +16,53 @@ import ( const releasedRoleName = "Repo Admin" -func checkLocalImageExistence(ctx context.Context, cli command.Cli, imageName string) error { - _, _, err := cli.Client().ImageInspectWithRaw(ctx, imageName) - return err +// ImageRefAndAuth contains all reference information and the auth config for an image request +type ImageRefAndAuth struct { + authConfig *types.AuthConfig + reference reference.Named + repoInfo *registry.RepositoryInfo + tag string } -func getImageReferencesAndAuth(cli command.Cli, imgName string) (context.Context, reference.Named, *registry.RepositoryInfo, *types.AuthConfig, error) { +// AuthConfig returns the auth information (username, etc) for a given ImageRefAndAuth +func (imgRefAuth *ImageRefAndAuth) AuthConfig() *types.AuthConfig { + return imgRefAuth.authConfig +} + +// Reference returns the Image reference for a given ImageRefAndAuth +func (imgRefAuth *ImageRefAndAuth) Reference() reference.Named { + return imgRefAuth.reference +} + +// RepoInfo returns the repository information for a given ImageRefAndAuth +func (imgRefAuth *ImageRefAndAuth) RepoInfo() *registry.RepositoryInfo { + return imgRefAuth.repoInfo +} + +// Tag returns the Image tag for a given ImageRefAndAuth +func (imgRefAuth *ImageRefAndAuth) Tag() string { + return imgRefAuth.tag +} + +func getImageReferencesAndAuth(ctx context.Context, cli command.Cli, imgName string) (*ImageRefAndAuth, error) { ref, err := reference.ParseNormalizedNamed(imgName) if err != nil { - return nil, nil, nil, nil, err + return nil, err + } + + tag, err := getTag(ref) + if err != nil { + return nil, err } // Resolve the Repository name from fqn to RepositoryInfo repoInfo, err := registry.ParseRepositoryInfo(ref) if err != nil { - return nil, nil, nil, nil, err + return nil, err } - ctx := context.Background() authConfig := command.ResolveAuthConfig(ctx, cli, repoInfo.Index) - return ctx, ref, repoInfo, &authConfig, err + return &ImageRefAndAuth{&authConfig, ref, repoInfo, tag}, err } func getTag(ref reference.Named) (string, error) { @@ -66,25 +92,10 @@ func notaryRoleToSigner(tufRole data.RoleName) string { return strings.TrimPrefix(tufRole.String(), "targets/") } -func askConfirm(input io.Reader) bool { - var res string - if _, err := fmt.Fscanln(input, &res); err != nil { - return false - } - if strings.EqualFold(res, "y") || strings.EqualFold(res, "yes") { - return true - } - return false -} - func clearChangeList(notaryRepo *client.NotaryRepository) error { - cl, err := notaryRepo.GetChangelist() if err != nil { return err } - if err = cl.Clear(""); err != nil { - return err - } - return nil + return cl.Clear("") } diff --git a/cli/command/trust/helpers_test.go b/cli/command/trust/helpers_test.go index c4fc831b66..5ca9d319aa 100644 --- a/cli/command/trust/helpers_test.go +++ b/cli/command/trust/helpers_test.go @@ -11,15 +11,18 @@ func TestGetTag(t *testing.T) { ref, err := reference.ParseNormalizedNamed("ubuntu@sha256:45b23dee08af5e43a7fea6c4cf9c25ccf269ee113168c19722f87876677c5cb2") assert.NoError(t, err) tag, err := getTag(ref) + assert.Error(t, err) assert.EqualError(t, err, "cannot use a digest reference for IMAGE:TAG") ref, err = reference.ParseNormalizedNamed("alpine:latest") assert.NoError(t, err) tag, err = getTag(ref) + assert.NoError(t, err) assert.Equal(t, tag, "latest") ref, err = reference.ParseNormalizedNamed("alpine") assert.NoError(t, err) tag, err = getTag(ref) + assert.NoError(t, err) assert.Equal(t, tag, "") } diff --git a/cli/command/trust/inspect.go b/cli/command/trust/inspect.go index d0ecd0e9fe..de420052df 100644 --- a/cli/command/trust/inspect.go +++ b/cli/command/trust/inspect.go @@ -1,8 +1,10 @@ package trust import ( + "context" "encoding/hex" "fmt" + "io" "sort" "strings" @@ -56,27 +58,26 @@ func newInspectCommand(dockerCli command.Cli) *cobra.Command { } func lookupTrustInfo(cli command.Cli, remote string) error { - _, ref, repoInfo, authConfig, err := getImageReferencesAndAuth(cli, remote) + ctx := context.Background() + imgRefAndAuth, err := getImageReferencesAndAuth(ctx, cli, remote) if err != nil { return err } - notaryRepo, err := trust.GetNotaryRepository(cli, repoInfo, *authConfig, "pull") + tag := imgRefAndAuth.Tag() + notaryRepo, err := trust.GetNotaryRepository(cli, imgRefAndAuth.RepoInfo(), *imgRefAndAuth.AuthConfig(), "pull") if err != nil { - return trust.NotaryError(ref.Name(), err) + return trust.NotaryError(imgRefAndAuth.Reference().Name(), err) } if err = clearChangeList(notaryRepo); err != nil { return err } defer clearChangeList(notaryRepo) - tag, err := getTag(ref) - if err != nil { - return err - } + // Retrieve all released signatures, match them, and pretty print them allSignedTargets, err := notaryRepo.GetAllTargetMetadataByName(tag) if err != nil { - logrus.Debug(trust.NotaryError(ref.Name(), err)) + logrus.Debug(trust.NotaryError(imgRefAndAuth.Reference().Name(), err)) // print an empty table if we don't have signed targets, but have an initialized notary repo if _, ok := err.(client.ErrNoSuchTarget); !ok { return fmt.Errorf("No signatures or cannot access %s", remote) @@ -84,7 +85,7 @@ func lookupTrustInfo(cli command.Cli, remote string) error { } signatureRows := matchReleasedSignatures(allSignedTargets) if len(signatureRows) > 0 { - if err := printSignatures(cli, signatureRows); err != nil { + if err := printSignatures(cli.Out(), signatureRows); err != nil { return err } } else { @@ -92,11 +93,10 @@ func lookupTrustInfo(cli command.Cli, remote string) error { } // get the administrative roles - roleWithSigs, err := notaryRepo.ListRoles() + adminRolesWithSigs, err := notaryRepo.ListRoles() if err != nil { return fmt.Errorf("No signers for %s", remote) } - adminRoleToKeyIDs := getAdministrativeRolesToKeyMap(roleWithSigs) // get delegation roles with the canonical key IDs delegationRoles, err := notaryRepo.GetDelegationRoles() @@ -107,44 +107,38 @@ func lookupTrustInfo(cli command.Cli, remote string) error { // If we do not have additional signers, do not display if len(signerRoleToKeyIDs) > 0 { - fmt.Fprintf(cli.Out(), "\nList of signers and their KeyIDs:\n\n") - printSignerInfo(cli, signerRoleToKeyIDs) + fmt.Fprintf(cli.Out(), "\nList of signers and their keys:\n\n") + printSignerInfo(cli.Out(), signerRoleToKeyIDs) } // This will always have the root and targets information fmt.Fprintf(cli.Out(), "\nAdministrative keys for %s:\n", strings.Split(remote, ":")[0]) - printSortedAdminKeys(adminRoleToKeyIDs, cli) + printSortedAdminKeys(cli.Out(), adminRolesWithSigs) return nil } -func printSortedAdminKeys(adminRoleToKeyIDs map[string]string, cli command.Cli) { - keyNames := []string{} - for name := range adminRoleToKeyIDs { - keyNames = append(keyNames, name) - } - - sort.Strings(keyNames) - - for _, keyName := range keyNames { - fmt.Fprintf(cli.Out(), "%s:\t%s\n", keyName, adminRoleToKeyIDs[keyName]) +func printSortedAdminKeys(out io.Writer, adminRoles []client.RoleWithSignatures) { + sort.Slice(adminRoles, func(i, j int) bool { return adminRoles[i].Name > adminRoles[j].Name }) + for _, adminRole := range adminRoles { + fmt.Fprintf(out, "%s", formatAdminRole(adminRole)) } } -func getAdministrativeRolesToKeyMap(roleWithSigs []client.RoleWithSignatures) map[string]string { - adminRoleToKeyIDs := make(map[string]string) - for _, roleWithSig := range roleWithSigs { - sort.Strings(roleWithSig.KeyIDs) - switch roleWithSig.Name { - case data.CanonicalTargetsRole: - adminRoleToKeyIDs["Repository Key"] = strings.Join(roleWithSig.KeyIDs, ", ") - case data.CanonicalRootRole: - adminRoleToKeyIDs["Root Key"] = strings.Join(roleWithSig.KeyIDs, ", ") - default: - continue - } +func formatAdminRole(roleWithSigs client.RoleWithSignatures) string { + adminKeyList := roleWithSigs.KeyIDs + sort.Strings(adminKeyList) + + var role string + switch roleWithSigs.Name { + case data.CanonicalTargetsRole: + role = "Repository Key" + case data.CanonicalRootRole: + role = "Root Key" + default: + return "" } - return adminRoleToKeyIDs + return fmt.Sprintf("%s:\t%s\n", role, strings.Join(adminKeyList, ", ")) } func getDelegationRoleToKeyMap(rawDelegationRoles []data.Role) map[string][]string { @@ -191,9 +185,9 @@ func matchReleasedSignatures(allTargets []client.TargetSignedStruct) trustTagRow } // pretty print with ordered rows -func printSignatures(dockerCli command.Cli, signatureRows trustTagRowList) error { +func printSignatures(out io.Writer, signatureRows trustTagRowList) error { trustTagCtx := formatter.Context{ - Output: dockerCli.Out(), + Output: out, Format: formatter.NewTrustTagFormat(), } // convert the formatted type before printing @@ -212,9 +206,9 @@ func printSignatures(dockerCli command.Cli, signatureRows trustTagRowList) error return formatter.TrustTagWrite(trustTagCtx, formattedTags) } -func printSignerInfo(dockerCli command.Cli, roleToKeyIDs map[string][]string) error { +func printSignerInfo(out io.Writer, roleToKeyIDs map[string][]string) error { signerInfoCtx := formatter.Context{ - Output: dockerCli.Out(), + Output: out, Format: formatter.NewSignerInfoFormat(), Trunc: true, } diff --git a/cli/command/trust/inspect_test.go b/cli/command/trust/inspect_test.go index 578ab9627d..b109314bb3 100644 --- a/cli/command/trust/inspect_test.go +++ b/cli/command/trust/inspect_test.go @@ -19,7 +19,7 @@ type fakeClient struct { dockerClient.Client } -func TestTrustInfoErrors(t *testing.T) { +func TestTrustInspectCommandErrors(t *testing.T) { testCases := []struct { name string args []string @@ -69,7 +69,7 @@ func TestTrustInfoErrors(t *testing.T) { } } -func TestTrustInfo(t *testing.T) { +func TestTrustInspectCommandFullRepoWithoutSigners(t *testing.T) { cli := test.NewFakeCli(&fakeClient{}) cmd := newInspectCommand(cli) cmd.SetArgs([]string{"alpine"}) @@ -83,10 +83,12 @@ func TestTrustInfo(t *testing.T) { assert.Contains(t, cli.OutBuffer().String(), "Administrative keys for alpine:") assert.Contains(t, cli.OutBuffer().String(), "(Repo Admin)") // no delegations on this repo - assert.NotContains(t, cli.OutBuffer().String(), "List of signers and their KeyIDs:") + assert.NotContains(t, cli.OutBuffer().String(), "List of signers and their keys:") +} - cli = test.NewFakeCli(&fakeClient{}) - cmd = newInspectCommand(cli) +func TestTrustInspectCommandOneTagWithoutSigners(t *testing.T) { + cli := test.NewFakeCli(&fakeClient{}) + cmd := newInspectCommand(cli) cmd.SetArgs([]string{"alpine:3.5"}) assert.NoError(t, cmd.Execute()) assert.Contains(t, cli.OutBuffer().String(), "SIGNED TAG") @@ -100,10 +102,12 @@ func TestTrustInfo(t *testing.T) { assert.Contains(t, cli.OutBuffer().String(), "(Repo Admin)") // no delegations on this repo assert.NotContains(t, cli.OutBuffer().String(), "3.6") - assert.NotContains(t, cli.OutBuffer().String(), "List of signers and their KeyIDs:") + assert.NotContains(t, cli.OutBuffer().String(), "List of signers and their keys:") +} - cli = test.NewFakeCli(&fakeClient{}) - cmd = newInspectCommand(cli) +func TestTrustInspectCommandFullRepoWithSigners(t *testing.T) { + cli := test.NewFakeCli(&fakeClient{}) + cmd := newInspectCommand(cli) cmd.SetArgs([]string{"dockerorcadev/trust-fixture"}) assert.NoError(t, cmd.Execute()) @@ -112,7 +116,7 @@ func TestTrustInfo(t *testing.T) { assert.Contains(t, cli.OutBuffer().String(), "DIGEST") assert.Contains(t, cli.OutBuffer().String(), "SIGNERS") // Check for the signer headers - assert.Contains(t, cli.OutBuffer().String(), "List of signers and their KeyIDs:") + assert.Contains(t, cli.OutBuffer().String(), "List of signers and their keys:") assert.Contains(t, cli.OutBuffer().String(), "SIGNER") assert.Contains(t, cli.OutBuffer().String(), "KEYS") assert.Contains(t, cli.OutBuffer().String(), "Administrative keys for dockerorcadev/trust-fixture:") @@ -120,9 +124,11 @@ func TestTrustInfo(t *testing.T) { assert.Contains(t, cli.OutBuffer().String(), "Root Key") // all signers have names assert.NotContains(t, cli.OutBuffer().String(), "(Repo Admin)") +} - cli = test.NewFakeCli(&fakeClient{}) - cmd = newInspectCommand(cli) +func TestTrustInspectCommandUnsignedTagInSignedRepo(t *testing.T) { + cli := test.NewFakeCli(&fakeClient{}) + cmd := newInspectCommand(cli) cmd.SetArgs([]string{"dockerorcadev/trust-fixture:unsigned"}) assert.NoError(t, cmd.Execute()) @@ -132,7 +138,7 @@ func TestTrustInfo(t *testing.T) { assert.NotContains(t, cli.OutBuffer().String(), "DIGEST") assert.NotContains(t, cli.OutBuffer().String(), "SIGNERS") // Check for the signer headers - assert.Contains(t, cli.OutBuffer().String(), "List of signers and their KeyIDs:") + assert.Contains(t, cli.OutBuffer().String(), "List of signers and their keys:") assert.Contains(t, cli.OutBuffer().String(), "SIGNER") assert.Contains(t, cli.OutBuffer().String(), "KEYS") assert.Contains(t, cli.OutBuffer().String(), "Administrative keys for dockerorcadev/trust-fixture:") @@ -144,7 +150,7 @@ func TestTrustInfo(t *testing.T) { assert.NotContains(t, cli.OutBuffer().String(), "(Repo Admin)") } -func TestTUFToSigner(t *testing.T) { +func TestNotaryRoleToSigner(t *testing.T) { assert.Equal(t, releasedRoleName, notaryRoleToSigner(data.CanonicalTargetsRole)) assert.Equal(t, releasedRoleName, notaryRoleToSigner(trust.ReleasesRole)) assert.Equal(t, "signer", notaryRoleToSigner("targets/signer")) @@ -314,7 +320,7 @@ func TestMatchReleasedSignatureFromTargets(t *testing.T) { assert.Equal(t, hex.EncodeToString(releasedTgt.Hashes[notary.SHA256]), outputRow.HashHex) } -func TestGetSignerAndAdminRolesWithKeyIDs(t *testing.T) { +func TestGetSignerRolesWithKeyIDs(t *testing.T) { roles := []data.Role{ { RootRole: data.RootRole{ @@ -363,10 +369,6 @@ func TestGetSignerAndAdminRolesWithKeyIDs(t *testing.T) { "alice": {"key11"}, "bob": {"key71", "key72"}, } - expectedAdminRoleToKeyIDs := map[string]string{ - "Root Key": "key01, key41", - "Repository Key": "key31", - } var roleWithSigs []client.RoleWithSignatures for _, role := range roles { @@ -375,6 +377,60 @@ func TestGetSignerAndAdminRolesWithKeyIDs(t *testing.T) { } signerRoleToKeyIDs := getDelegationRoleToKeyMap(roles) assert.Equal(t, expectedSignerRoleToKeyIDs, signerRoleToKeyIDs) - adminRoleToKeyIDs := getAdministrativeRolesToKeyMap(roleWithSigs) - assert.Equal(t, expectedAdminRoleToKeyIDs, adminRoleToKeyIDs) +} + +func TestFormatAdminRole(t *testing.T) { + aliceRole := data.Role{ + RootRole: data.RootRole{ + KeyIDs: []string{"key11"}, + }, + Name: "targets/alice", + } + aliceRoleWithSigs := client.RoleWithSignatures{Role: aliceRole, Signatures: nil} + assert.Equal(t, "", formatAdminRole(aliceRoleWithSigs)) + + releasesRole := data.Role{ + RootRole: data.RootRole{ + KeyIDs: []string{"key11"}, + }, + Name: "targets/releases", + } + releasesRoleWithSigs := client.RoleWithSignatures{Role: releasesRole, Signatures: nil} + assert.Equal(t, "", formatAdminRole(releasesRoleWithSigs)) + + timestampRole := data.Role{ + RootRole: data.RootRole{ + KeyIDs: []string{"key11"}, + }, + Name: data.CanonicalTimestampRole, + } + timestampRoleWithSigs := client.RoleWithSignatures{Role: timestampRole, Signatures: nil} + assert.Equal(t, "", formatAdminRole(timestampRoleWithSigs)) + + snapshotRole := data.Role{ + RootRole: data.RootRole{ + KeyIDs: []string{"key11"}, + }, + Name: data.CanonicalSnapshotRole, + } + snapshotRoleWithSigs := client.RoleWithSignatures{Role: snapshotRole, Signatures: nil} + assert.Equal(t, "", formatAdminRole(snapshotRoleWithSigs)) + + rootRole := data.Role{ + RootRole: data.RootRole{ + KeyIDs: []string{"key11"}, + }, + Name: data.CanonicalRootRole, + } + rootRoleWithSigs := client.RoleWithSignatures{Role: rootRole, Signatures: nil} + assert.Equal(t, "Root Key:\tkey11\n", formatAdminRole(rootRoleWithSigs)) + + targetsRole := data.Role{ + RootRole: data.RootRole{ + KeyIDs: []string{"key99", "abc", "key11"}, + }, + Name: data.CanonicalTargetsRole, + } + targetsRoleWithSigs := client.RoleWithSignatures{Role: targetsRole, Signatures: nil} + assert.Equal(t, "Repository Key:\tabc, key11, key99\n", formatAdminRole(targetsRoleWithSigs)) } diff --git a/cli/command/trust/revoke.go b/cli/command/trust/revoke.go index b150f402cf..ca91d63a26 100644 --- a/cli/command/trust/revoke.go +++ b/cli/command/trust/revoke.go @@ -1,15 +1,16 @@ package trust import ( + "context" "fmt" "os" "github.com/docker/notary/client" "github.com/docker/notary/tuf/data" + "github.com/pkg/errors" "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" - "github.com/docker/cli/cli/command/image" "github.com/docker/cli/cli/trust" "github.com/spf13/cobra" ) @@ -29,34 +30,26 @@ func newRevokeCommand(dockerCli command.Cli) *cobra.Command { }, } flags := cmd.Flags() - flags.BoolVarP(&options.forceYes, "yes", "y", false, "Answer yes to the removal question (no confirmation)") + flags.BoolVarP(&options.forceYes, "yes", "y", false, "Do not prompt for confirmation") return cmd } func revokeTrust(cli command.Cli, remote string, options revokeOptions) error { - _, ref, repoInfo, authConfig, err := getImageReferencesAndAuth(cli, remote) + ctx := context.Background() + imgRefAndAuth, err := getImageReferencesAndAuth(ctx, cli, remote) if err != nil { return err } - tag, err := getTag(ref) - if err != nil { - return err - } - if tag == "" && !options.forceYes { - in := os.Stdin - fmt.Fprintf( - cli.Out(), - "Please confirm you would like to delete all signature data for %s? (y/n) ", - remote, - ) - deleteRemote := askConfirm(in) + tag := imgRefAndAuth.Tag() + if imgRefAndAuth.Tag() == "" && !options.forceYes { + deleteRemote := command.PromptForConfirmation(os.Stdin, cli.Out(), fmt.Sprintf("Please confirm you would like to delete all signature data for %s?", remote)) if !deleteRemote { fmt.Fprintf(cli.Out(), "\nAborting action.\n") return nil } } - notaryRepo, err := trust.GetNotaryRepository(cli, repoInfo, *authConfig, "push", "pull") + notaryRepo, err := trust.GetNotaryRepository(cli, imgRefAndAuth.RepoInfo(), *imgRefAndAuth.AuthConfig(), "push", "pull") if err != nil { return err } @@ -65,14 +58,14 @@ func revokeTrust(cli command.Cli, remote string, options revokeOptions) error { return err } defer clearChangeList(notaryRepo) - if err := revokeTestHelper(notaryRepo, tag); err != nil { - return fmt.Errorf("could not remove signature for %s: %s", remote, err) + if err := revokeSignature(notaryRepo, tag); err != nil { + return errors.Wrapf(err, "could not remove signature for %s", remote) } fmt.Fprintf(cli.Out(), "Successfully deleted signature for %s\n", remote) return nil } -func revokeTestHelper(notaryRepo *client.NotaryRepository, tag string) error { +func revokeSignature(notaryRepo *client.NotaryRepository, tag string) error { if tag != "" { // Revoke signature for the specified tag if err := revokeSingleSig(notaryRepo, tag); err != nil { @@ -99,7 +92,6 @@ func revokeSingleSig(notaryRepo *client.NotaryRepository, tag string) error { } func revokeAllSigs(notaryRepo *client.NotaryRepository) error { - releasedTargetWithRoleList, err := notaryRepo.ListTargets(trust.ReleasesRole, data.CanonicalTargetsRole) if err != nil { return err @@ -117,7 +109,7 @@ func revokeAllSigs(notaryRepo *client.NotaryRepository) error { // get all the roles that signed the target and removes it from all roles. func getSignableRolesForTargetAndRemove(releasedTarget client.Target, notaryRepo *client.NotaryRepository) error { - signableRoles, err := image.GetSignableRoles(notaryRepo, &releasedTarget) + signableRoles, err := trust.GetSignableRoles(notaryRepo, &releasedTarget) if err != nil { return err } diff --git a/cli/command/trust/revoke_test.go b/cli/command/trust/revoke_test.go index 68b6f76187..019a8e9788 100644 --- a/cli/command/trust/revoke_test.go +++ b/cli/command/trust/revoke_test.go @@ -14,7 +14,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestTrustRevokeErrors(t *testing.T) { +func TestTrustRevokeCommandErrors(t *testing.T) { testCases := []struct { name string args []string @@ -78,7 +78,7 @@ func TestNewRevokeTrustAllSigConfirmation(t *testing.T) { cmd.SetArgs([]string{"alpine"}) assert.NoError(t, cmd.Execute()) - assert.Contains(t, cli.OutBuffer().String(), "Please confirm you would like to delete all signature data for alpine? (y/n) \nAborting action.") + assert.Contains(t, cli.OutBuffer().String(), "Please confirm you would like to delete all signature data for alpine? [y/N] \nAborting action.") } func TestGetSignableRolesForTargetAndRemoveError(t *testing.T) { diff --git a/cli/command/trust/sign.go b/cli/command/trust/sign.go index 277e03864a..17225357da 100644 --- a/cli/command/trust/sign.go +++ b/cli/command/trust/sign.go @@ -1,6 +1,7 @@ package trust import ( + "context" "fmt" "path" "sort" @@ -12,6 +13,7 @@ import ( "github.com/docker/cli/cli/trust" "github.com/docker/notary/client" "github.com/docker/notary/tuf/data" + "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -28,26 +30,24 @@ func newSignCommand(dockerCli command.Cli) *cobra.Command { } func signImage(cli command.Cli, imageName string) error { - ctx, ref, repoInfo, authConfig, err := getImageReferencesAndAuth(cli, imageName) + ctx := context.Background() + imgRefAndAuth, err := getImageReferencesAndAuth(ctx, cli, imageName) if err != nil { return err } + tag := imgRefAndAuth.Tag() + if tag == "" { + return fmt.Errorf("No tag specified for %s", imageName) + } - notaryRepo, err := trust.GetNotaryRepository(cli, repoInfo, *authConfig, "push", "pull") + notaryRepo, err := trust.GetNotaryRepository(cli, imgRefAndAuth.RepoInfo(), *imgRefAndAuth.AuthConfig(), "push", "pull") if err != nil { - return trust.NotaryError(ref.Name(), err) + return trust.NotaryError(imgRefAndAuth.Reference().Name(), err) } if err = clearChangeList(notaryRepo); err != nil { return err } defer clearChangeList(notaryRepo) - tag, err := getTag(ref) - if err != nil { - return err - } - if tag == "" { - return fmt.Errorf("No tag specified for %s", imageName) - } // get the latest repository metadata so we can figure out which roles to sign if err = notaryRepo.Update(false); err != nil { @@ -58,18 +58,18 @@ func signImage(cli command.Cli, imageName string) error { return err } - userRole := data.RoleName(path.Join(data.CanonicalTargetsRole.String(), authConfig.Username)) + userRole := data.RoleName(path.Join(data.CanonicalTargetsRole.String(), imgRefAndAuth.AuthConfig().Username)) if err := initNotaryRepoWithSigners(notaryRepo, userRole); err != nil { - return trust.NotaryError(ref.Name(), err) + return trust.NotaryError(imgRefAndAuth.Reference().Name(), err) } - fmt.Fprintf(cli.Out(), "Created signer: %s\n", authConfig.Username) - fmt.Fprintf(cli.Out(), "Finished initializing %q\n", notaryRepo.GetGUN().String()) + fmt.Fprintf(cli.Out(), "Created signer: %s\n", imgRefAndAuth.AuthConfig().Username) + fmt.Fprintf(cli.Out(), "Finished initializing signed repository for %s\n", imageName) default: - return trust.NotaryError(repoInfo.Name.Name(), err) + return trust.NotaryError(imgRefAndAuth.RepoInfo().Name.Name(), err) } } - requestPrivilege := command.RegistryAuthenticationPrivilegedFunc(cli, repoInfo.Index, "push") + requestPrivilege := command.RegistryAuthenticationPrivilegedFunc(cli, imgRefAndAuth.RepoInfo().Index, "push") target, err := createTarget(notaryRepo, tag) if err != nil { switch err := err.(type) { @@ -78,7 +78,7 @@ func signImage(cli command.Cli, imageName string) error { if err := checkLocalImageExistence(ctx, cli, imageName); err != nil { return err } - return image.TrustedPush(ctx, cli, repoInfo, ref, *authConfig, requestPrivilege) + return image.TrustedPush(ctx, cli, imgRefAndAuth.RepoInfo(), imgRefAndAuth.Reference(), *imgRefAndAuth.AuthConfig(), requestPrivilege) default: return err } @@ -95,12 +95,17 @@ func signImage(cli command.Cli, imageName string) error { err = notaryRepo.Publish() } if err != nil { - return fmt.Errorf("failed to sign %q:%s - %s", repoInfo.Name.Name(), tag, err.Error()) + return errors.Wrapf(err, "failed to sign %q:%s", imgRefAndAuth.RepoInfo().Name.Name(), tag) } - fmt.Fprintf(cli.Out(), "Successfully signed %q:%s\n", repoInfo.Name.Name(), tag) + fmt.Fprintf(cli.Out(), "Successfully signed %q:%s\n", imgRefAndAuth.RepoInfo().Name.Name(), tag) return nil } +func checkLocalImageExistence(ctx context.Context, cli command.Cli, imageName string) error { + _, _, err := cli.Client().ImageInspectWithRaw(ctx, imageName) + return err +} + func createTarget(notaryRepo *client.NotaryRepository, tag string) (client.Target, error) { target := &client.Target{} var err error diff --git a/cli/command/trust/sign_test.go b/cli/command/trust/sign_test.go index 6849e1b1d9..758ff3fa64 100644 --- a/cli/command/trust/sign_test.go +++ b/cli/command/trust/sign_test.go @@ -21,7 +21,7 @@ import ( const passwd = "password" -func TestTrustSignErrors(t *testing.T) { +func TestTrustSignCommandErrors(t *testing.T) { testCases := []struct { name string args []string @@ -69,7 +69,7 @@ func TestTrustSignErrors(t *testing.T) { { name: "no-keys", args: []string{"ubuntu:latest"}, - expectedError: "failed to sign \"docker.io/library/ubuntu\":latest - you are not authorized to perform this operation: server returned 401.", + expectedError: "failed to sign \"docker.io/library/ubuntu\":latest: you are not authorized to perform this operation: server returned 401.", }, } // change to a tmpdir diff --git a/cli/trust/trust.go b/cli/trust/trust.go index cd40c5fc92..19a4bb9718 100644 --- a/cli/trust/trust.go +++ b/cli/trust/trust.go @@ -230,3 +230,51 @@ func NotaryError(repoName string, err error) error { return err } + +// 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{}) + for fullKeyID := range repo.CryptoService.ListAllKeys() { + allCanonicalKeyIDs[path.Base(fullKeyID)] = struct{}{} + } + + allDelegationRoles, err := repo.GetDelegationRoles() + if err != nil { + return signableRoles, err + } + + // if there are no delegation roles, then just try to sign it into the targets role + if len(allDelegationRoles) == 0 { + signableRoles = append(signableRoles, data.CanonicalTargetsRole) + return signableRoles, nil + } + + // there are delegation roles, find every delegation role we have a key for, and + // attempt to sign into into all those roles. + for _, delegationRole := range allDelegationRoles { + // 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.String()) != data.CanonicalTargetsRole.String() || !delegationRole.CheckPaths(target.Name) { + continue + } + + for _, canonicalKeyID := range delegationRole.KeyIDs { + if _, ok := allCanonicalKeyIDs[canonicalKeyID]; ok { + signableRoles = append(signableRoles, delegationRole.Name) + break + } + } + } + + if len(signableRoles) == 0 { + return signableRoles, errors.Errorf("no valid signing keys for delegation roles") + } + + return signableRoles, nil + +} diff --git a/docs/reference/commandline/trust_inspect.md b/docs/reference/commandline/trust_inspect.md index 9de9bad9f1..54c40455ee 100644 --- a/docs/reference/commandline/trust_inspect.md +++ b/docs/reference/commandline/trust_inspect.md @@ -58,7 +58,7 @@ $ docker trust inspect my-image:purple SIGNED TAG DIGEST SIGNERS purple 941d3dba358621ce3c41ef67b47cf80f701ff80cdf46b5cc86587eaebfe45557 alice, bob, carol -List of signers and their KeyIDs: +List of signers and their keys: SIGNER KEYS alice 47caae5b3e61, a85aab9d20a4 @@ -123,7 +123,7 @@ yellow 9cc65fc3126790e683d1b92f307a71f48f75fa7dd47a7b03145a123eaf0b purple 941d3dba358621ce3c41ef67b47cf80f701ff80cdf46b5cc86587eaebfe45557 alice, bob, carol orange d6c271baa6d271bcc24ef1cbd65abf39123c17d2e83455bdab545a1a9093fc1c alice -List of signers and their KeyIDs: +List of signers and their keys: SIGNER KEYS alice 47caae5b3e61, a85aab9d20a4 diff --git a/docs/reference/commandline/trust_revoke.md b/docs/reference/commandline/trust_revoke.md index b496b2909f..904c4767ed 100644 --- a/docs/reference/commandline/trust_revoke.md +++ b/docs/reference/commandline/trust_revoke.md @@ -22,7 +22,7 @@ Remove trust for an image Options: --help Print usage - -y, --yes Answer yes to the removal question (no confirmation) + -y, --yes Do not prompt for confirmation ``` ## Description @@ -42,7 +42,7 @@ SIGNED TAG DIGEST red 852cc04935f930a857b630edc4ed6131e91b22073bcc216698842e44f64d2943 alice blue f1c38dbaeeb473c36716f6494d803fbfbe9d8a76916f7c0093f227821e378197 alice, bob -List of signers and their KeyIDs: +List of signers and their keys: SIGNER KEYS alice 05e87edcaecb @@ -68,7 +68,7 @@ $ docker trust inspect example/trust-demo SIGNED TAG DIGEST SIGNERS blue f1c38dbaeeb473c36716f6494d803fbfbe9d8a76916f7c0093f227821e378197 alice, bob -List of signers and their KeyIDs: +List of signers and their keys: SIGNER KEYS alice 05e87edcaecb @@ -89,7 +89,7 @@ SIGNED TAG DIGEST red 852cc04935f930a857b630edc4ed6131e91b22073bcc216698842e44f64d2943 alice blue f1c38dbaeeb473c36716f6494d803fbfbe9d8a76916f7c0093f227821e378197 alice, bob -List of signers and their KeyIDs: +List of signers and their keys: SIGNER KEYS alice 05e87edcaecb @@ -104,7 +104,7 @@ When `alice`, one of the signers, runs `docker trust revoke`: ```bash $ docker trust revoke example/trust-demo -Please confirm you would like to delete all signature data for example/trust-demo? (y/n) y +Please confirm you would like to delete all signature data for example/trust-demo? [y/N] y Enter passphrase for delegation key with ID 27d42a8: Successfully deleted signature for example/trust-demo ``` @@ -117,7 +117,7 @@ $ docker trust inspect example/trust-demo No signatures for example/trust-demo -List of signers and their KeyIDs: +List of signers and their keys: SIGNER KEYS alice 05e87edcaecb diff --git a/docs/reference/commandline/trust_sign.md b/docs/reference/commandline/trust_sign.md index dfde693e80..51a3428015 100644 --- a/docs/reference/commandline/trust_sign.md +++ b/docs/reference/commandline/trust_sign.md @@ -84,7 +84,7 @@ $ docker trust inspect example/trust-demo No signatures for example/trust-demo -List of signers and their KeyIDs: +List of signers and their keys: SIGNER KEYS alice 05e87edcaecb @@ -119,7 +119,7 @@ $ docker trust inspect example/trust-demo SIGNED TAG DIGEST SIGNERS v1 74d4bfa917d55d53c7df3d2ab20a8d926874d61c3da5ef6de15dd2654fc467c4 alice -List of signers and their KeyIDs: +List of signers and their keys: SIGNER KEYS alice 05e87edcaecb @@ -168,7 +168,7 @@ $ docker trust inspect example/trust-demo SIGNED TAG DIGEST SIGNERS v1 8f6f460abf0436922df7eb06d28b3cdf733d2cac1a185456c26debbff0839c56 alice -List of signers and their KeyIDs: +List of signers and their keys: SIGNER KEYS alice 6d52b29d940f