-
Notifications
You must be signed in to change notification settings - Fork 2.1k
docker trust: interact with signers and keys #566
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f47b1a3
5ab3ae7
dde9f61
604bc3f
4e797ea
532d223
2d8cc3c
b4ef2dd
079471e
e189a21
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| package trust | ||
|
|
||
| import ( | ||
| "io/ioutil" | ||
| "os" | ||
| "testing" | ||
|
|
||
| "github.com/docker/notary/client" | ||
| "github.com/docker/notary/passphrase" | ||
| "github.com/docker/notary/trustpinning" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| func TestGetOrGenerateNotaryKeyAndInitRepo(t *testing.T) { | ||
| tmpDir, err := ioutil.TempDir("", "notary-test-") | ||
| assert.NoError(t, err) | ||
| defer os.RemoveAll(tmpDir) | ||
|
|
||
| notaryRepo, err := client.NewFileCachedRepository(tmpDir, "gun", "https://localhost", nil, passphrase.ConstantRetriever(passwd), trustpinning.TrustPinConfig{}) | ||
| assert.NoError(t, err) | ||
|
|
||
| err = getOrGenerateRootKeyAndInitRepo(notaryRepo) | ||
| assert.EqualError(t, err, "client is offline") | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| package trust | ||
|
|
||
| import ( | ||
| "github.com/docker/cli/cli" | ||
| "github.com/docker/cli/cli/command" | ||
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| // newTrustKeyCommand returns a cobra command for `trust key` subcommands | ||
| func newTrustKeyCommand(dockerCli command.Streams) *cobra.Command { | ||
| cmd := &cobra.Command{ | ||
| Use: "key", | ||
| Short: "Manage keys for signing Docker images (experimental)", | ||
| Args: cli.NoArgs, | ||
| RunE: command.ShowHelp(dockerCli.Err()), | ||
| } | ||
| cmd.AddCommand( | ||
| newKeyGenerateCommand(dockerCli), | ||
| newKeyLoadCommand(dockerCli), | ||
| ) | ||
| return cmd | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| package trust | ||
|
|
||
| import ( | ||
| "encoding/pem" | ||
| "fmt" | ||
| "io/ioutil" | ||
| "os" | ||
| "path/filepath" | ||
| "regexp" | ||
| "strings" | ||
|
|
||
| "github.com/docker/cli/cli" | ||
| "github.com/docker/cli/cli/command" | ||
| "github.com/docker/cli/cli/trust" | ||
| "github.com/docker/notary" | ||
| "github.com/docker/notary/trustmanager" | ||
| "github.com/docker/notary/tuf/data" | ||
| tufutils "github.com/docker/notary/tuf/utils" | ||
| "github.com/pkg/errors" | ||
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| type keyGenerateOptions struct { | ||
| name string | ||
| directory string | ||
| } | ||
|
|
||
| func newKeyGenerateCommand(dockerCli command.Streams) *cobra.Command { | ||
| options := keyGenerateOptions{} | ||
| cmd := &cobra.Command{ | ||
| Use: "generate NAME", | ||
| Short: "Generate and load a signing key-pair", | ||
| Args: cli.ExactArgs(1), | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| options.name = args[0] | ||
| return setupPassphraseAndGenerateKeys(dockerCli, options) | ||
| }, | ||
| } | ||
| flags := cmd.Flags() | ||
| flags.StringVar(&options.directory, "dir", "", "Directory to generate key in, defaults to current directory") | ||
| return cmd | ||
| } | ||
|
|
||
| // 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 target dir on disk | ||
| func validateKeyArgs(keyName string, targetDir string) error { | ||
| if !validKeyName(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(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 targetDir == "" { | ||
| cwd, err := os.Getwd() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| targetDir = cwd | ||
| } | ||
| return validateAndGenerateKey(streams, opts.name, targetDir) | ||
| } | ||
|
|
||
| func validateAndGenerateKey(streams command.Streams, keyName string, workingDir string) error { | ||
| freshPassRetGetter := func() notary.PassRetriever { return trust.GetPassphraseRetriever(streams.In(), streams.Out()) } | ||
| if err := validateKeyArgs(keyName, workingDir); err != nil { | ||
| return err | ||
| } | ||
| 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 { | ||
| return err | ||
| } | ||
|
|
||
| pubPEM, err := generateKeyAndOutputPubPEM(keyName, privKeyFileStore) | ||
| if err != nil { | ||
| fmt.Fprintf(streams.Out(), err.Error()) | ||
| return errors.Wrapf(err, "failed to generate key for %s", keyName) | ||
| } | ||
|
|
||
| // Output the public key to a file in the CWD or specified dir | ||
| writtenPubFile, err := writePubKeyPEMToDir(pubPEM, keyName, workingDir) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| fmt.Fprintf(streams.Out(), "Successfully generated and loaded private key. Corresponding public key available: %s\n", writtenPubFile) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we print the location of the private key file as well? Something like: (or whatever information would be useful to the user)
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't want to expose the private key storage to users so I'd prefer not to (similar to your comment below on the |
||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func generateKeyAndOutputPubPEM(keyName string, privKeyStore trustmanager.KeyStore) (pem.Block, error) { | ||
| privKey, err := tufutils.GenerateKey(data.ECDSAKey) | ||
| if err != nil { | ||
| return pem.Block{}, err | ||
| } | ||
|
|
||
| privKeyStore.AddKey(trustmanager.KeyInfo{Role: data.RoleName(keyName)}, privKey) | ||
| if err != nil { | ||
| return pem.Block{}, err | ||
| } | ||
|
|
||
| pubKey := data.PublicKeyFromPrivate(privKey) | ||
| return pem.Block{ | ||
| Type: "PUBLIC KEY", | ||
| Headers: map[string]string{ | ||
| "role": keyName, | ||
| }, | ||
| Bytes: pubKey.Public(), | ||
| }, nil | ||
| } | ||
|
|
||
| func writePubKeyPEMToDir(pubPEM pem.Block, keyName, workingDir string) (string, error) { | ||
| // Output the public key to a file in the CWD or specified dir | ||
| pubFileName := strings.Join([]string{keyName, "pub"}, ".") | ||
| pubFilePath := filepath.Join(workingDir, pubFileName) | ||
| if err := ioutil.WriteFile(pubFilePath, pem.EncodeToMemory(&pubPEM), notary.PrivNoExecPerms); err != nil { | ||
| return "", errors.Wrapf(err, "failed to write public key to %s", pubFilePath) | ||
| } | ||
| return pubFilePath, nil | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, we should improve the error message in notary to include what the minimum length is:
https://github.com/docker/notary/blob/eb214782470e4913e5215df96e24aa59c36ca045/passphrase/passphrase.go#L146-L149
More in general (though out of bound for this PR); this check seems to give a bit of a false sense of "security", as it only checks some arbitrary length, but no other password policies. For example,
"password"is accepted just fine as a password (which, well, probably isn't really that more secure than "no password").Perhaps
trust.GetPassphraseRetriever()should accept avalidatefuncas argument, allowing consumers to provide their own function to enforce any password policy.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree that this is open for discussion. Let's open an issue in notary for this, separate from this PR: this affects Docker Content Trust today.