diff --git a/cli/command/trust/helpers.go b/cli/command/trust/helpers.go index a395af44e2..60d38815d3 100644 --- a/cli/command/trust/helpers.go +++ b/cli/command/trust/helpers.go @@ -11,12 +11,13 @@ import ( const releasedRoleName = "Repo Admin" const releasesRoleTUFName = "targets/releases" -// check if a role name is "released": either targets/releases or targets TUF roles +// isReleasedTarget checks if a role name is "released": +// either targets/releases or targets TUF roles func isReleasedTarget(role data.RoleName) bool { return role == data.CanonicalTargetsRole || role == trust.ReleasesRole } -// convert TUF role name to a human-understandable signer name +// notaryRoleToSigner converts TUF role name to a human-understandable signer name func notaryRoleToSigner(tufRole data.RoleName) string { // don't show a signer for "targets" or "targets/releases" if isReleasedTarget(data.RoleName(tufRole.String())) { @@ -25,6 +26,7 @@ func notaryRoleToSigner(tufRole data.RoleName) string { return strings.TrimPrefix(tufRole.String(), "targets/") } +// clearChangelist clears the notary staging changelist. func clearChangeList(notaryRepo client.Repository) error { cl, err := notaryRepo.GetChangelist() if err != nil { @@ -33,12 +35,13 @@ func clearChangeList(notaryRepo client.Repository) error { return cl.Clear("") } +// getOrGenerateRootKeyAndInitRepo initializes the notary repository +// with a remotely managed snapshot key. The initialization will use +// an existing root key if one is found, else a new one will be generated. func getOrGenerateRootKeyAndInitRepo(notaryRepo client.Repository) error { rootKey, err := getOrGenerateNotaryKey(notaryRepo, data.CanonicalRootRole) if err != nil { return err } - // Initialize the notary repository with a remotely managed snapshot - // key return notaryRepo.Initialize([]string{rootKey.ID()}, data.CanonicalSnapshotRole) } diff --git a/cli/command/trust/key_generate.go b/cli/command/trust/key_generate.go index 913083be8f..d12f21ce77 100644 --- a/cli/command/trust/key_generate.go +++ b/cli/command/trust/key_generate.go @@ -28,7 +28,7 @@ type keyGenerateOptions struct { func newKeyGenerateCommand(dockerCli command.Streams) *cobra.Command { options := keyGenerateOptions{} cmd := &cobra.Command{ - Use: "generate NAME [NAME...]", + Use: "generate NAME", Short: "Generate and load a signing key-pair", Args: cli.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { @@ -37,30 +37,33 @@ func newKeyGenerateCommand(dockerCli command.Streams) *cobra.Command { }, } flags := cmd.Flags() - flags.StringVarP(&options.directory, "dir", "d", "", "Directory to generate key in, defaults to current directory") + flags.StringVar(&options.directory, "dir", "", "Directory to generate key in, defaults to current directory") return cmd } -// key names can use alphanumeric + _ + - characters -var validKeyName = regexp.MustCompile(`^[a-zA-Z0-9\_]+[a-zA-Z0-9\_\-]*$`).MatchString +// key names can use lowercase alphanumeric + _ + - characters +var validKeyName = regexp.MustCompile(`^[a-z0-9][a-z0-9\_\-]*$`).MatchString // validate that all of the key names are unique and are alphanumeric + _ + - -// and that we do not already have public key files in the current dir on disk -func validateKeyArgs(keyName string, cwdPath string) error { +// and that we do not already have public key files in the target dir on disk +func validateKeyArgs(keyName string, targetDir string) error { if !validKeyName(keyName) { - return fmt.Errorf("key name \"%s\" must not contain special characters", keyName) + return fmt.Errorf("key name \"%s\" must start with lowercase alphanumeric characters and can include \"-\" or \"_\" after the first character", keyName) } pubKeyFileName := keyName + ".pub" - if _, err := os.Stat(filepath.Join(cwdPath, pubKeyFileName)); err == nil { - return fmt.Errorf("public key file already exists: \"%s\"", pubKeyFileName) + if _, err := os.Stat(targetDir); err != nil { + return fmt.Errorf("public key path does not exist: \"%s\"", targetDir) + } + targetPath := filepath.Join(targetDir, pubKeyFileName) + if _, err := os.Stat(targetPath); err == nil { + return fmt.Errorf("public key file already exists: \"%s\"", targetPath) } return nil } func setupPassphraseAndGenerateKeys(streams command.Streams, opts keyGenerateOptions) error { targetDir := opts.directory - // if the target dir is empty, default to CWD if targetDir == "" { cwd, err := os.Getwd() if err != nil { @@ -76,7 +79,7 @@ func validateAndGenerateKey(streams command.Streams, keyName string, workingDir if err := validateKeyArgs(keyName, workingDir); err != nil { return err } - fmt.Fprintf(streams.Out(), "\nGenerating key for %s...\n", keyName) + fmt.Fprintf(streams.Out(), "Generating key for %s...\n", keyName) // Automatically load the private key to local storage for use privKeyFileStore, err := trustmanager.NewKeyFileStore(trust.GetTrustDirectory(), freshPassRetGetter()) if err != nil { diff --git a/cli/command/trust/key_generate_test.go b/cli/command/trust/key_generate_test.go index 2aa0d256a9..4843f6af4c 100644 --- a/cli/command/trust/key_generate_test.go +++ b/cli/command/trust/key_generate_test.go @@ -2,6 +2,7 @@ package trust import ( "encoding/pem" + "fmt" "io/ioutil" "os" "path/filepath" @@ -120,14 +121,18 @@ func TestValidateKeyArgs(t *testing.T) { err = validateKeyArgs("a/b", pubKeyCWD) assert.Error(t, err) - assert.Equal(t, err.Error(), "key name \"a/b\" must not contain special characters") + assert.Equal(t, err.Error(), "key name \"a/b\" must start with lowercase alphanumeric characters and can include \"-\" or \"_\" after the first character") err = validateKeyArgs("-", pubKeyCWD) assert.Error(t, err) - assert.Equal(t, err.Error(), "key name \"-\" must not contain special characters") + assert.Equal(t, err.Error(), "key name \"-\" must start with lowercase alphanumeric characters and can include \"-\" or \"_\" after the first character") assert.NoError(t, ioutil.WriteFile(filepath.Join(pubKeyCWD, "a.pub"), []byte("abc"), notary.PrivExecPerms)) err = validateKeyArgs("a", pubKeyCWD) assert.Error(t, err) - assert.Equal(t, err.Error(), "public key file already exists: \"a.pub\"") + assert.Equal(t, err.Error(), fmt.Sprintf("public key file already exists: \"%s/a.pub\"", pubKeyCWD)) + + err = validateKeyArgs("a", "/random/dir/") + assert.Error(t, err) + assert.Equal(t, err.Error(), "public key path does not exist: \"/random/dir/\"") } diff --git a/cli/command/trust/key_load.go b/cli/command/trust/key_load.go index 9dc77b9996..86e9990fdb 100644 --- a/cli/command/trust/key_load.go +++ b/cli/command/trust/key_load.go @@ -2,6 +2,7 @@ package trust import ( "bytes" + "encoding/pem" "fmt" "io/ioutil" "os" @@ -18,8 +19,7 @@ import ( ) const ( - ownerReadOnlyPerms = 0400 - ownerReadAndWritePerms = 0600 + nonOwnerReadWriteMask = 0077 ) type keyLoadOptions struct { @@ -29,7 +29,7 @@ type keyLoadOptions struct { func newKeyLoadCommand(dockerCli command.Streams) *cobra.Command { var options keyLoadOptions cmd := &cobra.Command{ - Use: "load [OPTIONS] KEY", + Use: "load [OPTIONS] KEYFILE", Short: "Load a private key file for signing", Args: cli.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { @@ -37,11 +37,15 @@ func newKeyLoadCommand(dockerCli command.Streams) *cobra.Command { }, } flags := cmd.Flags() - flags.StringVarP(&options.keyName, "name", "n", "signer", "Name for the loaded key") + flags.StringVar(&options.keyName, "name", "signer", "Name for the loaded key") return cmd } func loadPrivKey(streams command.Streams, keyPath string, options keyLoadOptions) error { + // validate the key name if provided + if options.keyName != "" && !validKeyName(options.keyName) { + return fmt.Errorf("key name \"%s\" must start with lowercase alphanumeric characters and can include \"-\" or \"_\" after the first character", options.keyName) + } trustDir := trust.GetTrustDirectory() keyFileStore, err := storage.NewPrivateKeyFileStorage(trustDir, notary.KeyExtension) if err != nil { @@ -49,13 +53,13 @@ func loadPrivKey(streams command.Streams, keyPath string, options keyLoadOptions } privKeyImporters := []trustmanager.Importer{keyFileStore} - fmt.Fprintf(streams.Out(), "\nLoading key from \"%s\"...\n", keyPath) + fmt.Fprintf(streams.Out(), "Loading key from \"%s\"...\n", keyPath) // Always use a fresh passphrase retriever for each import passRet := trust.GetPassphraseRetriever(streams.In(), streams.Out()) keyBytes, err := getPrivKeyBytesFromPath(keyPath) if err != nil { - return errors.Wrapf(err, "error reading key from %s", keyPath) + return errors.Wrapf(err, "refusing to load key from %s", keyPath) } if err := loadPrivKeyBytesToStore(keyBytes, privKeyImporters, keyPath, options.keyName, passRet); err != nil { return errors.Wrapf(err, "error importing key from %s", keyPath) @@ -69,8 +73,8 @@ func getPrivKeyBytesFromPath(keyPath string) ([]byte, error) { if err != nil { return nil, err } - if fileInfo.Mode() != ownerReadOnlyPerms && fileInfo.Mode() != ownerReadAndWritePerms { - return nil, fmt.Errorf("private key permission from %s should be set to 400 or 600", keyPath) + if fileInfo.Mode()&nonOwnerReadWriteMask != 0 { + return nil, fmt.Errorf("private key file %s must not be readable or writable by others", keyPath) } from, err := os.OpenFile(keyPath, os.O_RDONLY, notary.PrivExecPerms) @@ -83,9 +87,29 @@ func getPrivKeyBytesFromPath(keyPath string) ([]byte, error) { } func loadPrivKeyBytesToStore(privKeyBytes []byte, privKeyImporters []trustmanager.Importer, keyPath, keyName string, passRet notary.PassRetriever) error { - if _, _, err := tufutils.ExtractPrivateKeyAttributes(privKeyBytes); err != nil { + var err error + if _, _, err = tufutils.ExtractPrivateKeyAttributes(privKeyBytes); err != nil { return fmt.Errorf("provided file %s is not a supported private key - to add a signer's public key use docker trust signer add", keyPath) } + if privKeyBytes, err = decodePrivKeyIfNecessary(privKeyBytes, passRet); err != nil { + return errors.Wrapf(err, "cannot load key from provided file %s", keyPath) + } // Make a reader, rewind the file pointer return trustmanager.ImportKeys(bytes.NewReader(privKeyBytes), privKeyImporters, keyName, "", passRet) } + +func decodePrivKeyIfNecessary(privPemBytes []byte, passRet notary.PassRetriever) ([]byte, error) { + pemBlock, _ := pem.Decode(privPemBytes) + _, containsDEKInfo := pemBlock.Headers["DEK-Info"] + if containsDEKInfo || pemBlock.Type == "ENCRYPTED PRIVATE KEY" { + // if we do not have enough information to properly import, try to decrypt the key + if _, ok := pemBlock.Headers["path"]; !ok { + privKey, _, err := trustmanager.GetPasswdDecryptBytes(passRet, privPemBytes, "", "encrypted") + if err != nil { + return []byte{}, fmt.Errorf("could not decrypt key") + } + privPemBytes = privKey.Private() + } + } + return privPemBytes, nil +} diff --git a/cli/command/trust/key_load_test.go b/cli/command/trust/key_load_test.go index 8631d25ae3..5959a81192 100644 --- a/cli/command/trust/key_load_test.go +++ b/cli/command/trust/key_load_test.go @@ -40,8 +40,14 @@ func TestTrustKeyLoadErrors(t *testing.T) { { name: "not-a-key", args: []string{"iamnotakey"}, - expectedError: "error reading key from iamnotakey: stat iamnotakey: no such file or directory", - expectedOutput: "\nLoading key from \"iamnotakey\"...\n", + expectedError: "refusing to load key from iamnotakey: stat iamnotakey: no such file or directory", + expectedOutput: "Loading key from \"iamnotakey\"...\n", + }, + { + name: "bad-key-name", + args: []string{"iamnotakey", "--name", "KEYNAME"}, + expectedError: "key name \"KEYNAME\" must start with lowercase alphanumeric characters and can include \"-\" or \"_\" after the first character", + expectedOutput: "", }, } tmpDir, err := ioutil.TempDir("", "docker-key-load-test-") @@ -178,21 +184,21 @@ func testLoadKeyTooPermissive(t *testing.T, privKeyFixture []byte) { // import the key to our keyStorageDir _, err = getPrivKeyBytesFromPath(privKeyFilepath) assert.Error(t, err) - assert.Contains(t, fmt.Sprintf("private key permission from %s should be set to 400 or 600", privKeyFilepath), err.Error()) + assert.Contains(t, fmt.Sprintf("private key file %s must not be readable or writable by others", privKeyFilepath), err.Error()) privKeyFilepath = filepath.Join(privKeyDir, "privkey667.pem") assert.NoError(t, ioutil.WriteFile(privKeyFilepath, privKeyFixture, 0677)) _, err = getPrivKeyBytesFromPath(privKeyFilepath) assert.Error(t, err) - assert.Contains(t, fmt.Sprintf("private key permission from %s should be set to 400 or 600", privKeyFilepath), err.Error()) + assert.Contains(t, fmt.Sprintf("private key file %s must not be readable or writable by others", privKeyFilepath), err.Error()) privKeyFilepath = filepath.Join(privKeyDir, "privkey777.pem") assert.NoError(t, ioutil.WriteFile(privKeyFilepath, privKeyFixture, 0777)) _, err = getPrivKeyBytesFromPath(privKeyFilepath) assert.Error(t, err) - assert.Contains(t, fmt.Sprintf("private key permission from %s should be set to 400 or 600", privKeyFilepath), err.Error()) + assert.Contains(t, fmt.Sprintf("private key file %s must not be readable or writable by others", privKeyFilepath), err.Error()) privKeyFilepath = filepath.Join(privKeyDir, "privkey400.pem") assert.NoError(t, ioutil.WriteFile(privKeyFilepath, privKeyFixture, 0400)) @@ -208,9 +214,9 @@ func testLoadKeyTooPermissive(t *testing.T, privKeyFixture []byte) { } var pubKeyFixture = []byte(`-----BEGIN PUBLIC KEY----- - MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEUIH9AYtrcDFzZrFJBdJZkn21d+4c - H3nzy2O6Q/ct4BjOBKa+WCdRtPo78bA+C/7t81ADQO8Jqaj59W50rwoqDQ== - -----END PUBLIC KEY-----`) +MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEUIH9AYtrcDFzZrFJBdJZkn21d+4c +H3nzy2O6Q/ct4BjOBKa+WCdRtPo78bA+C/7t81ADQO8Jqaj59W50rwoqDQ== +-----END PUBLIC KEY-----`) func TestLoadPubKeyFailure(t *testing.T) { pubKeyDir, err := ioutil.TempDir("", "key-load-test-pubkey-") diff --git a/cli/command/trust/signer_add.go b/cli/command/trust/signer_add.go index 830fb9eda4..8a9418f02b 100644 --- a/cli/command/trust/signer_add.go +++ b/cli/command/trust/signer_add.go @@ -3,6 +3,7 @@ package trust import ( "context" "fmt" + "io" "io/ioutil" "os" "path" @@ -24,62 +25,64 @@ import ( type signerAddOptions struct { keys opts.ListOpts signer string - images []string + repos []string } func newSignerAddCommand(dockerCli command.Cli) *cobra.Command { var options signerAddOptions cmd := &cobra.Command{ - Use: "add [OPTIONS] NAME IMAGE [IMAGE...] ", + Use: "add OPTIONS NAME REPOSITORY [REPOSITORY...] ", Short: "Add a signer", Args: cli.RequiresMinArgs(2), RunE: func(cmd *cobra.Command, args []string) error { options.signer = args[0] - options.images = args[1:] + options.repos = 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)") + flags.Var(&options.keys, "key", "Path to the signer's public key file") return cmd } -var validSignerName = regexp.MustCompile(`^[a-z0-9]+[a-z0-9\_\-]*$`).MatchString +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) + return fmt.Errorf("signer name \"%s\" must start with lowercase alphanumeric characters and can include \"-\" or \"_\" after the first character", 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") + if options.keys.Len() == 0 { + return fmt.Errorf("path to a 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.Err(), err.Error()) - errImages = append(errImages, imageName) + signerPubKeys, err := ingestPublicKeys(options.keys.GetAll()) + if err != nil { + return err + } + var errRepos []string + for _, repoName := range options.repos { + fmt.Fprintf(cli.Out(), "Adding signer \"%s\" to %s...\n", signerName, repoName) + if err := addSignerToRepo(cli, signerName, repoName, signerPubKeys); err != nil { + fmt.Fprintln(cli.Err(), err.Error()+"\n") + errRepos = append(errRepos, repoName) } else { - fmt.Fprintf(cli.Out(), "Successfully added signer: %s to %s\n", signerName, imageName) + fmt.Fprintf(cli.Out(), "Successfully added signer: %s to %s\n\n", signerName, repoName) } } - if len(errImages) > 0 { - return fmt.Errorf("Failed to add signer to: %s", strings.Join(errImages, ", ")) + if len(errRepos) > 0 { + return fmt.Errorf("Failed to add signer to: %s", strings.Join(errRepos, ", ")) } 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) - +func addSignerToRepo(cli command.Cli, signerName string, repoName string, signerPubKeys []data.PublicKey) error { ctx := context.Background() - imgRefAndAuth, err := trust.GetImageReferencesAndAuth(ctx, image.AuthResolver(cli), imageName) + imgRefAndAuth, err := trust.GetImageReferencesAndAuth(ctx, image.AuthResolver(cli), repoName) if err != nil { return err } @@ -92,22 +95,18 @@ func addSignerToImage(cli command.Cli, signerName string, imageName string, keyP 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) + fmt.Fprintf(cli.Out(), "Initializing signed repository for %s...\n", repoName) if err := getOrGenerateRootKeyAndInitRepo(notaryRepo); err != nil { - return trust.NotaryError(imageName, err) + return trust.NotaryError(repoName, err) } - fmt.Fprintf(cli.Out(), "Successfully initialized %q\n", imageName) + fmt.Fprintf(cli.Out(), "Successfully initialized %q\n", repoName) default: - return trust.NotaryError(imageName, err) + return trust.NotaryError(repoName, err) } } newSignerRoleName := data.RoleName(path.Join(data.CanonicalTargetsRole.String(), signerName)) - signerPubKeys, err := ingestPublicKeys(keyPaths) - if err != nil { - return err - } if err := addStagedSigner(notaryRepo, newSignerRoleName, signerPubKeys); err != nil { return errors.Wrapf(err, "could not add signer to repo: %s", strings.TrimPrefix(newSignerRoleName.String(), "targets/")) } @@ -118,19 +117,23 @@ func addSignerToImage(cli command.Cli, signerName string, imageName string, keyP 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) + // Read public key bytes from PEM file, limit to 1 KiB + pubKeyFile, err := os.OpenFile(pubKeyPath, os.O_RDONLY, 0666) if err != nil { - if os.IsNotExist(err) { - return nil, fmt.Errorf("file for public key does not exist: %s", pubKeyPath) - } - return nil, errors.Wrapf(err, "unable to read public key from file: %s", pubKeyPath) + return nil, errors.Wrap(err, "unable to read public key from file") + } + defer pubKeyFile.Close() + // limit to + l := io.LimitReader(pubKeyFile, 1<<20) + pubKeyBytes, err := ioutil.ReadAll(l) + if err != nil { + return nil, errors.Wrap(err, "unable to read public key from file") } // Parse PEM bytes into type PublicKey pubKey, err := tufutils.ParsePEMPublicKey(pubKeyBytes) if err != nil { - return nil, err + return nil, errors.Wrapf(err, "could not parse public key from file: %s", pubKeyPath) } pubKeys = append(pubKeys, pubKey) } diff --git a/cli/command/trust/signer_add_test.go b/cli/command/trust/signer_add_test.go index 8b61c0c31d..075613f84b 100644 --- a/cli/command/trust/signer_add_test.go +++ b/cli/command/trust/signer_add_test.go @@ -4,11 +4,13 @@ import ( "fmt" "io/ioutil" "os" + "path/filepath" "testing" "github.com/docker/cli/cli/config" "github.com/docker/cli/internal/test" "github.com/docker/cli/internal/test/testutil" + "github.com/docker/notary" "github.com/stretchr/testify/assert" ) @@ -25,27 +27,27 @@ func TestTrustSignerAddErrors(t *testing.T) { { name: "no-key", args: []string{"foo", "bar"}, - expectedError: "path to a valid public key must be provided using the `--key` flag", + expectedError: "path to a public key must be provided using the `--key` flag", }, { name: "reserved-releases-signer-add", - args: []string{"releases", "my-image", "-k", "/path/to/key"}, + args: []string{"releases", "my-image", "--key", "/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", + args: []string{"ali/ce", "my-image", "--key", "/path/to/key"}, + expectedError: "signer name \"ali/ce\" must start with lowercase alphanumeric characters and can include \"-\" or \"_\" after the first character", }, { name: "no-upper-case", - args: []string{"Alice", "my-image", "-k", "/path/to/key"}, - expectedError: "signer name \"Alice\" must not contain uppercase or special characters", + args: []string{"Alice", "my-image", "--key", "/path/to/key"}, + expectedError: "signer name \"Alice\" must start with lowercase alphanumeric characters and can include \"-\" or \"_\" after the first character", }, { name: "start-with-letter", - args: []string{"_alice", "my-image", "-k", "/path/to/key"}, - expectedError: "signer name \"_alice\" must not contain uppercase or special characters", + args: []string{"_alice", "my-image", "--key", "/path/to/key"}, + expectedError: "signer name \"_alice\" must start with lowercase alphanumeric characters and can include \"-\" or \"_\" after the first character", }, } tmpDir, err := ioutil.TempDir("", "docker-sign-test-") @@ -79,13 +81,7 @@ func TestSignerAddCommandNoTargetsKey(t *testing.T) { 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.ErrBuffer().String(), "no valid public key found") - - assert.Contains(t, cli.OutBuffer().String(), "Adding signer \"alice\" to linuxkit/alpine...") - assert.Contains(t, cli.ErrBuffer().String(), "no valid public key found") + assert.EqualError(t, cmd.Execute(), fmt.Sprintf("could not parse public key from file: %s: no valid public key found", tmpfile.Name())) } func TestSignerAddCommandBadKeyPath(t *testing.T) { @@ -100,10 +96,7 @@ func TestSignerAddCommandBadKeyPath(t *testing.T) { 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 := "file for public key does not exist: /path/to/key.pem" - assert.Contains(t, cli.ErrBuffer().String(), expectedError) + assert.EqualError(t, cmd.Execute(), "unable to read public key from file: open /path/to/key.pem: no such file or directory") } func TestSignerAddCommandInvalidRepoName(t *testing.T) { @@ -112,15 +105,21 @@ func TestSignerAddCommandInvalidRepoName(t *testing.T) { defer os.RemoveAll(tmpDir) config.SetDir(tmpDir) + pubKeyDir, err := ioutil.TempDir("", "key-load-test-pubkey-") + assert.NoError(t, err) + defer os.RemoveAll(pubKeyDir) + pubKeyFilepath := filepath.Join(pubKeyDir, "pubkey.pem") + assert.NoError(t, ioutil.WriteFile(pubKeyFilepath, pubKeyFixture, notary.PrivNoExecPerms)) + cli := test.NewFakeCli(&fakeClient{}) cli.SetNotaryClient(getUninitializedNotaryRepository) cmd := newSignerAddCommand(cli) imageName := "870d292919d01a0af7e7f056271dc78792c05f55f49b9b9012b6d89725bd9abd" - cmd.SetArgs([]string{"--key", "/path/to/key.pem", "alice", imageName}) + cmd.SetArgs([]string{"--key", pubKeyFilepath, "alice", imageName}) cmd.SetOutput(ioutil.Discard) assert.EqualError(t, cmd.Execute(), "Failed to add signer to: 870d292919d01a0af7e7f056271dc78792c05f55f49b9b9012b6d89725bd9abd") - expectedErr := fmt.Sprintf("invalid repository name (%s), cannot specify 64-byte hexadecimal strings\n", imageName) + expectedErr := fmt.Sprintf("invalid repository name (%s), cannot specify 64-byte hexadecimal strings\n\n", imageName) assert.Equal(t, expectedErr, cli.ErrBuffer().String()) } @@ -128,11 +127,11 @@ func TestSignerAddCommandInvalidRepoName(t *testing.T) { 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") + assert.EqualError(t, err, "unable to read public key from file: open foo: no such file or directory") // 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") + assert.EqualError(t, err, fmt.Sprintf("could not parse public key from file: %s: no valid public key found", tmpfile.Name())) } diff --git a/cli/command/trust/signer_remove.go b/cli/command/trust/signer_remove.go index 0f13dad41f..2ac4224434 100644 --- a/cli/command/trust/signer_remove.go +++ b/cli/command/trust/signer_remove.go @@ -18,37 +18,40 @@ import ( type signerRemoveOptions struct { signer string - images []string + repos []string forceYes bool } func newSignerRemoveCommand(dockerCli command.Cli) *cobra.Command { options := signerRemoveOptions{} cmd := &cobra.Command{ - Use: "remove [OPTIONS] NAME IMAGE [IMAGE...]", + Use: "remove [OPTIONS] NAME REPOSITORY [REPOSITORY...]", Short: "Remove a signer", Args: cli.RequiresMinArgs(2), RunE: func(cmd *cobra.Command, args []string) error { options.signer = args[0] - options.images = args[1:] + options.repos = args[1:] return removeSigner(dockerCli, options) }, } flags := cmd.Flags() - flags.BoolVarP(&options.forceYes, "yes", "y", false, "Do not prompt for confirmation before removing the most recent signer") + flags.BoolVarP(&options.forceYes, "force", "f", false, "Do not prompt for confirmation before removing the most recent signer") return cmd } func removeSigner(cli command.Cli, options signerRemoveOptions) error { - var errImages []string - for _, image := range options.images { - if err := removeSingleSigner(cli, image, options.signer, options.forceYes); err != nil { - fmt.Fprintln(cli.Err(), err.Error()) - errImages = append(errImages, image) + var errRepos []string + for _, repo := range options.repos { + fmt.Fprintf(cli.Out(), "Removing signer \"%s\" from %s...\n", options.signer, repo) + if err := removeSingleSigner(cli, repo, options.signer, options.forceYes); err != nil { + fmt.Fprintln(cli.Err(), err.Error()+"\n") + errRepos = append(errRepos, repo) + } else { + fmt.Fprintf(cli.Out(), "Successfully removed %s from %s\n\n", options.signer, repo) } } - if len(errImages) > 0 { - return fmt.Errorf("Error removing signer from: %s", strings.Join(errImages, ", ")) + if len(errRepos) > 0 { + return fmt.Errorf("Error removing signer from: %s", strings.Join(errRepos, ", ")) } return nil } @@ -75,11 +78,9 @@ func isLastSignerForReleases(roleWithSig data.Role, allRoles []client.RoleWithSi return counter < releasesRoleWithSigs.Threshold, nil } -func removeSingleSigner(cli command.Cli, imageName, signerName string, forceYes bool) error { - fmt.Fprintf(cli.Out(), "\nRemoving signer \"%s\" from %s...\n", signerName, imageName) - +func removeSingleSigner(cli command.Cli, repoName, signerName string, forceYes bool) error { ctx := context.Background() - imgRefAndAuth, err := trust.GetImageReferencesAndAuth(ctx, image.AuthResolver(cli), imageName) + imgRefAndAuth, err := trust.GetImageReferencesAndAuth(ctx, image.AuthResolver(cli), repoName) if err != nil { return err } @@ -94,7 +95,7 @@ func removeSingleSigner(cli command.Cli, imageName, signerName string, forceYes } delegationRoles, err := notaryRepo.GetDelegationRoles() if err != nil { - return errors.Wrapf(err, "error retrieving signers for %s", imageName) + return errors.Wrapf(err, "error retrieving signers for %s", repoName) } var role data.Role for _, delRole := range delegationRoles { @@ -104,7 +105,7 @@ func removeSingleSigner(cli command.Cli, imageName, signerName string, forceYes } } if role.Name == "" { - return fmt.Errorf("No signer %s for image %s", signerName, imageName) + return fmt.Errorf("No signer %s for repository %s", signerName, repoName) } allRoles, err := notaryRepo.ListRoles() if err != nil { @@ -114,7 +115,7 @@ func removeSingleSigner(cli command.Cli, imageName, signerName string, 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, imageName, imageName, + signerName, repoName, repoName, )) if !removeSigner { @@ -133,6 +134,5 @@ func removeSingleSigner(cli command.Cli, imageName, signerName string, forceYes if err = notaryRepo.Publish(); err != nil { return err } - fmt.Fprintf(cli.Out(), "Successfully removed %s from %s\n", signerName, imageName) return nil } diff --git a/cli/command/trust/signer_remove_test.go b/cli/command/trust/signer_remove_test.go index fea6c65731..f0ab695c89 100644 --- a/cli/command/trust/signer_remove_test.go +++ b/cli/command/trust/signer_remove_test.go @@ -71,26 +71,25 @@ 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") + assert.EqualError(t, err, "No signer test for repository signed-repo") 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, signerRemoveOptions{signer: "test", images: []string{"signed-repo", "signed-repo"}, forceYes: true}) + err := removeSigner(cli, signerRemoveOptions{signer: "test", repos: []string{"signed-repo", "signed-repo"}, forceYes: true}) assert.EqualError(t, err, "Error removing signer from: signed-repo, signed-repo") assert.Contains(t, cli.ErrBuffer().String(), - "No signer test for image signed-repo") + "No signer test for repository signed-repo") + assert.Contains(t, cli.OutBuffer().String(), "Removing signer \"test\" from signed-repo...\n") } func TestRemoveLastSignerWarning(t *testing.T) { cli := test.NewFakeCli(&fakeClient{}) cli.SetNotaryClient(getLoadedNotaryRepository) - err := removeSigner(cli, signerRemoveOptions{signer: "alice", images: []string{"signed-repo"}, forceYes: false}) + err := removeSigner(cli, signerRemoveOptions{signer: "alice", repos: []string{"signed-repo"}, forceYes: false}) assert.NoError(t, err) assert.Contains(t, cli.OutBuffer().String(), "The signer \"alice\" signed the last released version of signed-repo. "+ diff --git a/docs/reference/commandline/trust_key_generate.md b/docs/reference/commandline/trust_key_generate.md index 811b4e7763..ad2758d0b1 100644 --- a/docs/reference/commandline/trust_key_generate.md +++ b/docs/reference/commandline/trust_key_generate.md @@ -20,6 +20,9 @@ Usage: docker trust key generate NAME Generate and load a signing key-pair +Options: + --dir string Directory to generate key in, defaults to current directory + --help Print usage ``` ## Description @@ -50,3 +53,17 @@ All passphrase requests to sign with the key will be referred to by the provided The public key component `alice.pub` will be available in the current working directory, and can be used directly by `docker trust signer add`. + +Provide the `--dir` argument to specify a directory to generate the key in: + +```bash +$ docker trust key generate alice --dir /foo + +Generating key for alice... +Enter passphrase for new alice key with ID 17acf3c: +Repeat passphrase for new alice key with ID 17acf3c: +Successfully generated and loaded private key. Corresponding public key available: alice.pub +$ ls /foo +alice.pub + +``` diff --git a/docs/reference/commandline/trust_key_load.md b/docs/reference/commandline/trust_key_load.md index f1c3d05ab8..6a29dd8874 100644 --- a/docs/reference/commandline/trust_key_load.md +++ b/docs/reference/commandline/trust_key_load.md @@ -16,10 +16,13 @@ keywords: "Key, notary, trust" # trust key load ```markdown -Usage: docker trust key load [OPTIONS] KEY +Usage: docker trust key load [OPTIONS] KEYFILE -Load a signing key +Load a private key file for signing +Options: + --help Print usage + --name string Name for the loaded key (default "signer") ``` ## Description diff --git a/docs/reference/commandline/trust_signer_add.md b/docs/reference/commandline/trust_signer_add.md index cb5ca77faf..bda45196f7 100644 --- a/docs/reference/commandline/trust_signer_add.md +++ b/docs/reference/commandline/trust_signer_add.md @@ -16,10 +16,13 @@ keywords: "signer, notary, trust" # trust signer add ```markdown -Usage: docker trust signer add [OPTIONS] NAME IMAGE [IMAGE...] +Usage: docker trust signer add [OPTIONS] NAME REPOSITORY [REPOSITORY...] -Add a signer to one or more repositories +Add a signer +Options: + --help Print usage + -k, --key list Path to the signer's public key file ``` ## Description @@ -35,7 +38,7 @@ Add a signer to one or more repositories To add a new signer, `alice`, to this repository: ```bash -$ docker trust inspect example/trust-demo +$ docker trust view example/trust-demo No signatures for example/trust-demo @@ -54,16 +57,15 @@ Add `alice` with `docker trust signer add`: ```bash $ docker trust signer add alice example/trust-demo --key alice.crt - Adding signer "alice" to example/trust-demo... Enter passphrase for repository key with ID 642692c: Successfully added signer: alice to example/trust-demo ``` -`docker trust inspect` now lists `alice` as a valid signer: +`docker trust view` now lists `alice` as a valid signer: ```bash -$ docker trust inspect example/trust-demo +$ docker trust view example/trust-demo No signatures for example/trust-demo @@ -84,7 +86,7 @@ Root Key: 3cb2228f6561e58f46dbc4cda4fcaff9d5ef22e865a94636f82450d1d2234949 When adding a signer on a repo for the first time, `docker trust signer add` sets up a new repo if it doesn't exist. ```bash -$ docker trust inspect example/trust-demo +$ docker trust view example/trust-demo No signatures or cannot access example/trust-demo ``` @@ -101,7 +103,7 @@ $ docker trust signer add alice example/trust-demo --key alice.crt ``` ```bash -$ docker trust inspect example/trust-demo +$ docker trust view example/trust-demo No signatures for example/trust-demo @@ -122,7 +124,7 @@ Root Key: 748121c14bd1461f6c58cb3ef39087c8fdc7633bb11a98af844fd9a04e208103 To add a signer, `alice`, to multiple repositories: ```bash -$ docker trust inspect example/trust-demo +$ docker trust view example/trust-demo SIGNED TAG DIGEST SIGNERS v1 74d4bfa917d55d53c7df3d2ab20a8d926874d61c3da5ef6de15dd2654fc467c4 bob @@ -136,7 +138,7 @@ Repository Key: ecc457614c9fc399da523a5f4e24fe306a0a6ee1cc79a10e4555b3c6ab02f71e Root Key: 3cb2228f6561e58f46dbc4cda4fcaff9d5ef22e865a94636f82450d1d2234949 ``` ```bash -$ docker trust inspect example/trust-demo2 +$ docker trust view example/trust-demo2 SIGNED TAG DIGEST SIGNERS v1 74d4bfa917d55d53c7df3d2ab20a8d926874d61c3da5ef6de15dd2654fc467c4 bob @@ -152,8 +154,7 @@ Root Key: 3cb2228f6561e58f46dbc4cda4fcaff9d5ef22e865a94636f82450d1d2234949 Add `alice` to both repositories with a single `docker trust signer add` command: ```bash -$ docker trust signer add alice example/trust-demo example/trust-demo2 -k alice.crt - +$ docker trust signer add alice example/trust-demo example/trust-demo2 --key alice.crt Adding signer "alice" to example/trust-demo... Enter passphrase for repository key with ID 95b9e55: Successfully added signer: alice to example/trust-demo @@ -162,11 +163,11 @@ Adding signer "alice" to example/trust-demo2... Enter passphrase for repository key with ID ece554f: Successfully added signer: alice to example/trust-demo2 ``` -`docker trust inspect` now lists `alice` as a valid signer of both `example/trust-demo` and `example/trust-demo2`: +`docker trust view` now lists `alice` as a valid signer of both `example/trust-demo` and `example/trust-demo2`: ```bash -$ docker trust inspect example/trust-demo +$ docker trust view example/trust-demo SIGNED TAG DIGEST SIGNERS v1 74d4bfa917d55d53c7df3d2ab20a8d926874d61c3da5ef6de15dd2654fc467c4 bob @@ -181,7 +182,7 @@ Repository Key: 95b9e5514c9fc399da523a5f4e24fe306a0a6ee1cc79a10e4555b3c6ab02f71e Root Key: 3cb2228f6561e58f46dbc4cda4fcaff9d5ef22e865a94636f82450d1d2234949 ``` ```bash -$ docker trust inspect example/trust-demo2 +$ docker trust view example/trust-demo2 SIGNED TAG DIGEST SIGNERS v1 74d4bfa917d55d53c7df3d2ab20a8d926874d61c3da5ef6de15dd2654fc467c4 bob @@ -200,8 +201,7 @@ Root Key: 3cb2228f6561e58f46dbc4cda4fcaff9d5ef22e865a94636f82450d1d2234949 `docker trust signer add` adds signers to repositories on a best effort basis, so it will continue to add the signer to subsequent repositories if one attempt fails: ```bash -$ docker trust signer add alice example/unauthorized example/authorized -k alice.crt - +$ docker trust signer add alice example/unauthorized example/authorized --key alice.crt Adding signer "alice" to example/unauthorized... you are not authorized to perform this operation: server returned 401. diff --git a/docs/reference/commandline/trust_signer_remove.md b/docs/reference/commandline/trust_signer_remove.md index 3da59a3c28..6afe160040 100644 --- a/docs/reference/commandline/trust_signer_remove.md +++ b/docs/reference/commandline/trust_signer_remove.md @@ -16,10 +16,13 @@ keywords: "signer, notary, trust" # trust signer remove ```markdown -Usage: docker trust signer remove [OPTIONS] NAME IMAGE [IMAGE...] +Usage: docker trust signer remove [OPTIONS] NAME REPOSITORY [REPOSITORY...] -Remove a signer from one or more repositories +Remove a signer +Options: + -f, --force Do not prompt for confirmation before removing the most recent signer + --help Print usage ``` ## Description @@ -35,7 +38,7 @@ Remove a signer from one or more repositories To remove an existing signer, `alice`, from this repository: ```bash -$ docker trust inspect example/trust-demo +$ docker trust view example/trust-demo No signatures for example/trust-demo @@ -55,15 +58,16 @@ Remove `alice` with `docker trust signer remove`: ```bash $ docker trust signer remove alice example/trust-demo + Removing signer "alice" from image example/trust-demo... Enter passphrase for repository key with ID 642692c: Successfully removed alice from example/trust-demo ``` -`docker trust inspect` now does not list `alice` as a valid signer: +`docker trust view` now does not list `alice` as a valid signer: ```bash -$ docker trust inspect example/trust-demo +$ docker trust view example/trust-demo No signatures for example/trust-demo @@ -83,7 +87,7 @@ Root Key: 3cb2228f6561e58f46dbc4cda4fcaff9d5ef22e865a94636f82450d1d2234949 To remove an existing signer, `alice`, from multiple repositories: ```bash -$ docker trust inspect example/trust-demo +$ docker trust view example/trust-demo SIGNED TAG DIGEST SIGNERS v1 74d4bfa917d55d53c7df3d2ab20a8d926874d61c3da5ef6de15dd2654fc467c4 alice, bob @@ -98,7 +102,7 @@ Repository Key: 95b9e5514c9fc399da523a5f4e24fe306a0a6ee1cc79a10e4555b3c6ab02f71e Root Key: 3cb2228f6561e58f46dbc4cda4fcaff9d5ef22e865a94636f82450d1d2234949 ``` ```bash -$ docker trust inspect example/trust-demo2 +$ docker trust view example/trust-demo2 SIGNED TAG DIGEST SIGNERS v1 74d4bfa917d55d53c7df3d2ab20a8d926874d61c3da5ef6de15dd2654fc467c4 alice, bob @@ -116,14 +120,17 @@ Remove `alice` from both images with a single `docker trust signer remove` comma ```bash $ docker trust signer remove alice example/trust-demo example/trust-demo2 +Removing signer "alice" from image example/trust-demo... Enter passphrase for repository key with ID 95b9e55: Successfully removed alice from example/trust-demo + +Removing signer "alice" from image example/trust-demo2... Enter passphrase for repository key with ID ece554f: Successfully removed alice from example/trust-demo2 ``` -`docker trust inspect` no longer lists `alice` as a valid signer of either `example/trust-demo` or `example/trust-demo2`: +`docker trust view` no longer lists `alice` as a valid signer of either `example/trust-demo` or `example/trust-demo2`: ```bash -$ docker trust inspect example/trust-demo +$ docker trust view example/trust-demo SIGNED TAG DIGEST SIGNERS v1 74d4bfa917d55d53c7df3d2ab20a8d926874d61c3da5ef6de15dd2654fc467c4 bob @@ -137,7 +144,7 @@ Repository Key: ecc457614c9fc399da523a5f4e24fe306a0a6ee1cc79a10e4555b3c6ab02f71e Root Key: 3cb2228f6561e58f46dbc4cda4fcaff9d5ef22e865a94636f82450d1d2234949 ``` ```bash -$ docker trust inspect example/trust-demo2 +$ docker trust view example/trust-demo2 SIGNED TAG DIGEST SIGNERS v1 74d4bfa917d55d53c7df3d2ab20a8d926874d61c3da5ef6de15dd2654fc467c4 bob @@ -155,7 +162,6 @@ Root Key: 3cb2228f6561e58f46dbc4cda4fcaff9d5ef22e865a94636f82450d1d2234949 ```bash $ docker trust signer remove alice example/unauthorized example/authorized - Removing signer "alice" from image example/unauthorized... No signer alice for image example/unauthorized