From dde9f614a70d5c310e4b786cb9a30f09d5e79245 Mon Sep 17 00:00:00 2001 From: Ashwini Oruganti Date: Tue, 26 Sep 2017 11:43:52 -0700 Subject: [PATCH] trust: add signer-add and signer-remove command Signed-off-by: Riyaz Faizullabhoy --- cli/command/trust/client_test.go | 34 +++++- cli/command/trust/signer_add.go | 139 +++++++++++++++++++++++ cli/command/trust/signer_add_test.go | 140 ++++++++++++++++++++++++ cli/command/trust/signer_remove.go | 137 +++++++++++++++++++++++ cli/command/trust/signer_remove_test.go | 124 +++++++++++++++++++++ 5 files changed, 573 insertions(+), 1 deletion(-) create mode 100644 cli/command/trust/signer_add.go create mode 100644 cli/command/trust/signer_add_test.go create mode 100644 cli/command/trust/signer_remove.go create mode 100644 cli/command/trust/signer_remove_test.go diff --git a/cli/command/trust/client_test.go b/cli/command/trust/client_test.go index 3343209673..028083591f 100644 --- a/cli/command/trust/client_test.go +++ b/cli/command/trust/client_test.go @@ -316,7 +316,39 @@ func (l LoadedNotaryRepository) ListRoles() ([]client.RoleWithSignatures, error) Name: data.CanonicalTargetsRole, } - return []client.RoleWithSignatures{{Role: rootRole}, {Role: targetsRole}}, nil + aliceRole := data.Role{ + RootRole: data.RootRole{ + KeyIDs: []string{"A"}, + Threshold: 1, + }, + Name: data.RoleName("targets/alice"), + } + + bobRole := data.Role{ + RootRole: data.RootRole{ + KeyIDs: []string{"B"}, + Threshold: 1, + }, + Name: data.RoleName("targets/bob"), + } + + releasesRole := data.Role{ + RootRole: data.RootRole{ + KeyIDs: []string{"A", "B"}, + Threshold: 1, + }, + Name: data.RoleName("targets/releases"), + } + // have releases only signed off by Alice last + releasesSig := []data.Signature{{KeyID: "A"}} + + return []client.RoleWithSignatures{ + {Role: rootRole}, + {Role: targetsRole}, + {Role: aliceRole}, + {Role: bobRole}, + {Role: releasesRole, Signatures: releasesSig}, + }, nil } func (l LoadedNotaryRepository) ListTargets(roles ...data.RoleName) ([]*client.TargetWithRole, error) { diff --git a/cli/command/trust/signer_add.go b/cli/command/trust/signer_add.go new file mode 100644 index 0000000000..159cce6757 --- /dev/null +++ b/cli/command/trust/signer_add.go @@ -0,0 +1,139 @@ +package trust + +import ( + "context" + "fmt" + "io/ioutil" + "os" + "path" + "regexp" + "strings" + + "github.com/docker/cli/cli" + "github.com/docker/cli/cli/command" + "github.com/docker/cli/cli/trust" + "github.com/docker/cli/opts" + "github.com/docker/docker/api/types" + registrytypes "github.com/docker/docker/api/types/registry" + "github.com/docker/notary/client" + "github.com/docker/notary/tuf/data" + tufutils "github.com/docker/notary/tuf/utils" + "github.com/spf13/cobra" +) + +type signerAddOptions struct { + keys opts.ListOpts + signer string + images []string +} + +func newSignerAddCommand(dockerCli command.Cli) *cobra.Command { + var options signerAddOptions + cmd := &cobra.Command{ + Use: "signer-add [OPTIONS] NAME IMAGE [IMAGE...] ", + Short: "Add a signer", + Args: cli.RequiresMinArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + options.signer = args[0] + options.images = args[1:] + return addSigner(dockerCli, &options) + }, + } + flags := cmd.Flags() + options.keys = opts.NewListOpts(nil) + flags.VarP(&options.keys, "key", "k", "Path to the signer's public key(s)") + return cmd +} + +var validSignerName = regexp.MustCompile(`^[a-z0-9]+[a-z0-9\_\-]*$`).MatchString + +func addSigner(cli command.Cli, options *signerAddOptions) error { + signerName := options.signer + if !validSignerName(signerName) { + return fmt.Errorf("signer name \"%s\" must not contain uppercase or special characters", signerName) + } + if signerName == "releases" { + return fmt.Errorf("releases is a reserved keyword, please use a different signer name") + } + + if options.keys.Len() < 1 { + return fmt.Errorf("path to a valid public key must be provided using the `--key` flag") + } + + var errImages []string + for _, imageName := range options.images { + if err := addSignerToImage(cli, signerName, imageName, options.keys.GetAll()); err != nil { + fmt.Fprintln(cli.Out(), err.Error()) + errImages = append(errImages, imageName) + } else { + fmt.Fprintf(cli.Out(), "Successfully added signer: %s to %s\n", signerName, imageName) + } + } + if len(errImages) > 0 { + return fmt.Errorf("Failed to add signer to: %s", strings.Join(errImages, ", ")) + } + return nil +} + +func addSignerToImage(cli command.Cli, signerName string, imageName string, keyPaths []string) error { + fmt.Fprintf(cli.Out(), "\nAdding signer \"%s\" to %s...\n", signerName, imageName) + + ctx := context.Background() + authResolver := func(ctx context.Context, index *registrytypes.IndexInfo) types.AuthConfig { + return command.ResolveAuthConfig(ctx, cli, index) + } + imgRefAndAuth, err := trust.GetImageReferencesAndAuth(ctx, authResolver, imageName) + if err != nil { + return err + } + + notaryRepo, err := cli.NotaryClient(*imgRefAndAuth, trust.ActionsPushAndPull) + if err != nil { + return trust.NotaryError(imgRefAndAuth.Reference().Name(), err) + } + + if _, err = notaryRepo.ListTargets(); err != nil { + switch err.(type) { + case client.ErrRepoNotInitialized, client.ErrRepositoryNotExist: + fmt.Fprintf(cli.Out(), "Initializing signed repository for %s...\n", imageName) + if err := getOrGenerateRootKeyAndInitRepo(notaryRepo); err != nil { + return trust.NotaryError(imageName, err) + } + fmt.Fprintf(cli.Out(), "Successfully initialized %q\n", imageName) + default: + return trust.NotaryError(imageName, err) + } + } + + newSignerRoleName := data.RoleName(path.Join(data.CanonicalTargetsRole.String(), signerName)) + + signerPubKeys, err := ingestPublicKeys(keyPaths) + if err != nil { + return err + } + addStagedSigner(notaryRepo, newSignerRoleName, signerPubKeys) + + return notaryRepo.Publish() +} + +func ingestPublicKeys(pubKeyPaths []string) ([]data.PublicKey, error) { + pubKeys := []data.PublicKey{} + for _, pubKeyPath := range pubKeyPaths { + // Read public key bytes from PEM file + pubKeyBytes, err := ioutil.ReadFile(pubKeyPath) + if err != nil { + if os.IsNotExist(err) { + return nil, fmt.Errorf("file for public key does not exist: %s", pubKeyPath) + } + return nil, fmt.Errorf("unable to read public key from file: %s", pubKeyPath) + } + + // Parse PEM bytes into type PublicKey + pubKey, err := tufutils.ParsePEMPublicKey(pubKeyBytes) + if err != nil { + return nil, err + } + pubKeys = append(pubKeys, pubKey) + } + return pubKeys, nil +} diff --git a/cli/command/trust/signer_add_test.go b/cli/command/trust/signer_add_test.go new file mode 100644 index 0000000000..41880ef9b5 --- /dev/null +++ b/cli/command/trust/signer_add_test.go @@ -0,0 +1,140 @@ +package trust + +import ( + "fmt" + "io/ioutil" + "os" + "testing" + + "github.com/docker/cli/cli/config" + "github.com/docker/cli/internal/test" + "github.com/docker/cli/internal/test/testutil" + "github.com/stretchr/testify/assert" +) + +func TestTrustSignerAddErrors(t *testing.T) { + testCases := []struct { + name string + args []string + expectedError string + }{ + { + name: "not-enough-args", + expectedError: "requires at least 2 argument", + }, + { + name: "no-key", + args: []string{"foo", "bar"}, + expectedError: "path to a valid public key must be provided using the `--key` flag", + }, + { + name: "reserved-releases-signer-add", + args: []string{"releases", "my-image", "-k", "/path/to/key"}, + expectedError: "releases is a reserved keyword, please use a different signer name", + }, + { + name: "disallowed-chars", + args: []string{"ali/ce", "my-image", "-k", "/path/to/key"}, + expectedError: "signer name \"ali/ce\" must not contain uppercase or special characters", + }, + { + name: "no-upper-case", + args: []string{"Alice", "my-image", "-k", "/path/to/key"}, + expectedError: "signer name \"Alice\" must not contain uppercase or special characters", + }, + { + name: "start-with-letter", + args: []string{"_alice", "my-image", "-k", "/path/to/key"}, + expectedError: "signer name \"_alice\" must not contain uppercase or special characters", + }, + } + tmpDir, err := ioutil.TempDir("", "docker-sign-test-") + assert.NoError(t, err) + defer os.RemoveAll(tmpDir) + config.SetDir(tmpDir) + + for _, tc := range testCases { + cli := test.NewFakeCli(&fakeClient{}) + cli.SetNotaryClient(getOfflineNotaryRepository) + cmd := newSignerAddCommand(cli) + cmd.SetArgs(tc.args) + cmd.SetOutput(ioutil.Discard) + testutil.ErrorContains(t, cmd.Execute(), tc.expectedError) + } +} + +func TestSignerAddCommandNoTargetsKey(t *testing.T) { + tmpDir, err := ioutil.TempDir("", "docker-sign-test-") + assert.NoError(t, err) + defer os.RemoveAll(tmpDir) + config.SetDir(tmpDir) + + tmpfile, err := ioutil.TempFile("", "pemfile") + assert.NoError(t, err) + defer os.Remove(tmpfile.Name()) + + cli := test.NewFakeCli(&fakeClient{}) + cli.SetNotaryClient(getEmptyTargetsNotaryRepository) + cmd := newSignerAddCommand(cli) + cmd.SetArgs([]string{"--key", tmpfile.Name(), "alice", "alpine", "linuxkit/alpine"}) + + cmd.SetOutput(ioutil.Discard) + assert.EqualError(t, cmd.Execute(), "Failed to add signer to: alpine, linuxkit/alpine") + + assert.Contains(t, cli.OutBuffer().String(), "Adding signer \"alice\" to alpine...") + assert.Contains(t, cli.OutBuffer().String(), "no valid public key found") + + assert.Contains(t, cli.OutBuffer().String(), "Adding signer \"alice\" to linuxkit/alpine...") + assert.Contains(t, cli.OutBuffer().String(), "no valid public key found") +} + +func TestSignerAddCommandBadKeyPath(t *testing.T) { + tmpDir, err := ioutil.TempDir("", "docker-sign-test-") + assert.NoError(t, err) + defer os.RemoveAll(tmpDir) + config.SetDir(tmpDir) + + cli := test.NewFakeCli(&fakeClient{}) + cli.SetNotaryClient(getEmptyTargetsNotaryRepository) + cmd := newSignerAddCommand(cli) + cmd.SetArgs([]string{"--key", "/path/to/key.pem", "alice", "alpine"}) + + cmd.SetOutput(ioutil.Discard) + assert.EqualError(t, cmd.Execute(), "Failed to add signer to: alpine") + + expectedError := "\nAdding signer \"alice\" to alpine...\nfile for public key does not exist: /path/to/key.pem" + assert.Contains(t, cli.OutBuffer().String(), expectedError) +} + +func TestSignerAddCommandInvalidRepoName(t *testing.T) { + tmpDir, err := ioutil.TempDir("", "docker-sign-test-") + assert.NoError(t, err) + defer os.RemoveAll(tmpDir) + config.SetDir(tmpDir) + + cli := test.NewFakeCli(&fakeClient{}) + cli.SetNotaryClient(getUninitializedNotaryRepository) + cmd := newSignerAddCommand(cli) + imageName := "870d292919d01a0af7e7f056271dc78792c05f55f49b9b9012b6d89725bd9abd" + cmd.SetArgs([]string{"--key", "/path/to/key.pem", "alice", imageName}) + + cmd.SetOutput(ioutil.Discard) + assert.EqualError(t, cmd.Execute(), "Failed to add signer to: 870d292919d01a0af7e7f056271dc78792c05f55f49b9b9012b6d89725bd9abd") + expectedOutput := fmt.Sprintf("\nAdding signer \"alice\" to %s...\n"+ + "invalid repository name (%s), cannot specify 64-byte hexadecimal strings\n", + imageName, imageName) + + assert.Equal(t, expectedOutput, cli.OutBuffer().String()) +} + +func TestIngestPublicKeys(t *testing.T) { + // Call with a bad path + _, err := ingestPublicKeys([]string{"foo", "bar"}) + assert.EqualError(t, err, "file for public key does not exist: foo") + // Call with real file path + tmpfile, err := ioutil.TempFile("", "pemfile") + assert.NoError(t, err) + defer os.Remove(tmpfile.Name()) + _, err = ingestPublicKeys([]string{tmpfile.Name()}) + assert.EqualError(t, err, "no valid public key found") +} diff --git a/cli/command/trust/signer_remove.go b/cli/command/trust/signer_remove.go new file mode 100644 index 0000000000..6045b591dd --- /dev/null +++ b/cli/command/trust/signer_remove.go @@ -0,0 +1,137 @@ +package trust + +import ( + "context" + "fmt" + "os" + "strings" + + "github.com/docker/cli/cli" + "github.com/docker/cli/cli/command" + "github.com/docker/cli/cli/trust" + "github.com/docker/docker/api/types" + registrytypes "github.com/docker/docker/api/types/registry" + "github.com/docker/notary/client" + "github.com/docker/notary/tuf/data" + "github.com/spf13/cobra" +) + +type signerRemoveOptions struct { + forceYes bool +} + +func newSignerRemoveCommand(dockerCli command.Cli) *cobra.Command { + options := signerRemoveOptions{} + cmd := &cobra.Command{ + Use: "signer-remove [OPTIONS] NAME IMAGE [IMAGE...]", + Short: "Remove a signer", + Args: cli.RequiresMinArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + return removeSigner(dockerCli, args[0], args[1:], &options) + }, + } + flags := cmd.Flags() + flags.BoolVarP(&options.forceYes, "yes", "y", false, "Answer yes to removing most recent signer (no confirmation)") + return cmd +} + +func removeSigner(cli command.Cli, signer string, images []string, options *signerRemoveOptions) error { + var errImages []string + for _, image := range images { + if err := removeSingleSigner(cli, image, signer, options.forceYes); err != nil { + fmt.Fprintln(cli.Out(), err.Error()) + errImages = append(errImages, image) + } + } + if len(errImages) > 0 { + return fmt.Errorf("Error removing signer from: %s", strings.Join(errImages, ", ")) + } + return nil +} + +func isLastSignerForReleases(roleWithSig data.Role, allRoles []client.RoleWithSignatures) (bool, error) { + var releasesRoleWithSigs client.RoleWithSignatures + for _, role := range allRoles { + if role.Name == releasesRoleTUFName { + releasesRoleWithSigs = role + break + } + } + counter := len(releasesRoleWithSigs.Signatures) + if counter == 0 { + return false, fmt.Errorf("All signed tags are currently revoked, use docker trust sign to fix") + } + for _, signature := range releasesRoleWithSigs.Signatures { + for _, key := range roleWithSig.KeyIDs { + if signature.KeyID == key { + counter-- + } + } + } + return counter < releasesRoleWithSigs.Threshold, nil +} + +func removeSingleSigner(cli command.Cli, image, signerName string, forceYes bool) error { + fmt.Fprintf(cli.Out(), "\nRemoving signer \"%s\" from %s...\n", signerName, image) + + ctx := context.Background() + authResolver := func(ctx context.Context, index *registrytypes.IndexInfo) types.AuthConfig { + return command.ResolveAuthConfig(ctx, cli, index) + } + imgRefAndAuth, err := trust.GetImageReferencesAndAuth(ctx, authResolver, image) + if err != nil { + return err + } + + signerDelegation := data.RoleName("targets/" + signerName) + if signerDelegation == releasesRoleTUFName { + return fmt.Errorf("releases is a reserved keyword and cannot be removed") + } + notaryRepo, err := cli.NotaryClient(*imgRefAndAuth, trust.ActionsPushAndPull) + if err != nil { + return trust.NotaryError(imgRefAndAuth.Reference().Name(), err) + } + delegationRoles, err := notaryRepo.GetDelegationRoles() + if err != nil { + return fmt.Errorf("Error retrieving signers for %s", image) + } + var role data.Role + for _, delRole := range delegationRoles { + if delRole.Name == signerDelegation { + role = delRole + break + } + } + if role.Name == "" { + return fmt.Errorf("No signer %s for image %s", signerName, image) + } + allRoles, err := notaryRepo.ListRoles() + if err != nil { + return err + } + if ok, err := isLastSignerForReleases(role, allRoles); ok && !forceYes { + removeSigner := command.PromptForConfirmation(os.Stdin, cli.Out(), fmt.Sprintf("The signer \"%s\" signed the last released version of %s. "+ + "Removing this signer will make %s unpullable. "+ + "Are you sure you want to continue?", + signerName, image, image, + )) + + if !removeSigner { + fmt.Fprintf(cli.Out(), "\nAborting action.\n") + return nil + } + } else if err != nil { + fmt.Fprintln(cli.Out(), err.Error()) + } + if err = notaryRepo.RemoveDelegationKeys(releasesRoleTUFName, role.KeyIDs); err != nil { + return err + } + if err = notaryRepo.RemoveDelegationRole(signerDelegation); err != nil { + return err + } + if err = notaryRepo.Publish(); err != nil { + return err + } + fmt.Fprintf(cli.Out(), "Successfully removed %s from %s\n", signerName, image) + return nil +} diff --git a/cli/command/trust/signer_remove_test.go b/cli/command/trust/signer_remove_test.go new file mode 100644 index 0000000000..7ad841a82d --- /dev/null +++ b/cli/command/trust/signer_remove_test.go @@ -0,0 +1,124 @@ +package trust + +import ( + "io/ioutil" + "testing" + + "github.com/docker/cli/internal/test" + "github.com/docker/cli/internal/test/testutil" + "github.com/docker/notary/client" + "github.com/docker/notary/tuf/data" + "github.com/stretchr/testify/assert" +) + +func TestTrustSignerRemoveErrors(t *testing.T) { + testCases := []struct { + name string + args []string + expectedError string + }{ + { + name: "not-enough-args-0", + expectedError: "requires at least 2 arguments", + }, + { + name: "not-enough-args-1", + args: []string{"user"}, + expectedError: "requires at least 2 arguments", + }, + } + for _, tc := range testCases { + cmd := newSignerRemoveCommand( + test.NewFakeCli(&fakeClient{})) + cmd.SetArgs(tc.args) + cmd.SetOutput(ioutil.Discard) + testutil.ErrorContains(t, cmd.Execute(), tc.expectedError) + } + testCasesWithOutput := []struct { + name string + args []string + expectedError string + }{ + { + name: "not-an-image", + args: []string{"user", "notanimage"}, + expectedError: "Error retrieving signers for notanimage", + }, + { + name: "sha-reference", + args: []string{"user", "870d292919d01a0af7e7f056271dc78792c05f55f49b9b9012b6d89725bd9abd"}, + expectedError: "invalid repository name", + }, + { + name: "invalid-img-reference", + args: []string{"user", "ALPINE"}, + expectedError: "invalid reference format", + }, + } + for _, tc := range testCasesWithOutput { + cli := test.NewFakeCli(&fakeClient{}) + cli.SetNotaryClient(getOfflineNotaryRepository) + cmd := newSignerRemoveCommand(cli) + cmd.SetArgs(tc.args) + cmd.SetOutput(ioutil.Discard) + cmd.Execute() + assert.Contains(t, cli.OutBuffer().String(), tc.expectedError) + } + +} + +func TestRemoveSingleSigner(t *testing.T) { + cli := test.NewFakeCli(&fakeClient{}) + cli.SetNotaryClient(getLoadedNotaryRepository) + err := removeSingleSigner(cli, "signed-repo", "test", true) + assert.EqualError(t, err, "No signer test for image signed-repo") + assert.Contains(t, cli.OutBuffer().String(), "\nRemoving signer \"test\" from signed-repo...\n") + err = removeSingleSigner(cli, "signed-repo", "releases", true) + assert.EqualError(t, err, "releases is a reserved keyword and cannot be removed") + assert.Contains(t, cli.OutBuffer().String(), "\nRemoving signer \"releases\" from signed-repo...\n") +} + +func TestRemoveMultipleSigners(t *testing.T) { + cli := test.NewFakeCli(&fakeClient{}) + cli.SetNotaryClient(getLoadedNotaryRepository) + err := removeSigner(cli, "test", []string{"signed-repo", "signed-repo"}, &signerRemoveOptions{forceYes: true}) + assert.EqualError(t, err, "Error removing signer from: signed-repo, signed-repo") + assert.Contains(t, cli.OutBuffer().String(), + "\nRemoving signer \"test\" from signed-repo...\nNo signer test for image signed-repo\n\nRemoving signer \"test\" from signed-repo...\nNo signer test for image signed-repo") +} +func TestRemoveLastSignerWarning(t *testing.T) { + cli := test.NewFakeCli(&fakeClient{}) + cli.SetNotaryClient(getLoadedNotaryRepository) + err := removeSigner(cli, "alice", []string{"signed-repo"}, &signerRemoveOptions{forceYes: false}) + assert.NoError(t, err) + assert.Contains(t, cli.OutBuffer().String(), + "The signer \"alice\" signed the last released version of signed-repo. "+ + "Removing this signer will make signed-repo unpullable. "+ + "Are you sure you want to continue? [y/N]") +} + +func TestIsLastSignerForReleases(t *testing.T) { + role := data.Role{} + releaserole := client.RoleWithSignatures{} + releaserole.Name = releasesRoleTUFName + releaserole.Threshold = 1 + allrole := []client.RoleWithSignatures{releaserole} + lastsigner, _ := isLastSignerForReleases(role, allrole) + assert.Equal(t, false, lastsigner) + + role.KeyIDs = []string{"deadbeef"} + sig := data.Signature{} + sig.KeyID = "deadbeef" + releaserole.Signatures = []data.Signature{sig} + releaserole.Threshold = 1 + allrole = []client.RoleWithSignatures{releaserole} + lastsigner, _ = isLastSignerForReleases(role, allrole) + assert.Equal(t, true, lastsigner) + + sig.KeyID = "8badf00d" + releaserole.Signatures = []data.Signature{sig} + releaserole.Threshold = 1 + allrole = []client.RoleWithSignatures{releaserole} + lastsigner, _ = isLastSignerForReleases(role, allrole) + assert.Equal(t, false, lastsigner) +}