docker trust: interact with signers and keys #566

riyazdf · 2017-09-27T17:26:15Z

This PR introduces four more docker trust subcommands to interact with signers and keys:

docker trust signer add: add a signer to one or more repos
docker trust signer remove: remove a signer from one or more repos
docker trust key load: load a private key into the local storage to use for signing
docker trust key generate: generate a signing key-pair. The private key is automatically loaded into the local storage, and the public key can be used for signer add

cc @ashfall and @eiais who also implemented these features

This PR includes code and documentation and extends the functionality in #472. As such, it can be included in a followup release.

Original design doc here

thaJeztah · 2017-09-27T19:38:52Z

Just from the description; should we think of making signer and key top-level objects, to make it fit better with the existing UX (docker signer add, docker signer remove, docker key load, docker key generate), or alternatively docker trust-key load, docker trust-key generate

riyazdf · 2017-10-02T17:05:31Z

@thaJeztah: we kept key and signer under the trust command because the concepts probably won't be used elsewhere - that said I'm cool with factoring them out to docker signer/docker key or docker trust-signer/docker trust-key if you think that would be better.

For most users the flows will look something like:

# Add a signer and view to confirm
docker trust signer-add alice repo
docker trust view repo

# Load my signer key and sign with it
docker trust key-load alice.priv
docker trust sign repo

dnephin · 2017-10-02T18:23:15Z

I like it either as it is now, or as another level under docker trust: docker trust signer add , docker trust key load.

Moving them to the top level would be confusing.

thaJeztah · 2017-10-03T13:29:45Z

docker trust signer add

works for me; would that be more difficult to implement (as we currently don't have sub-sub commands)?

dnephin · 2017-10-03T14:37:43Z

Shouldn't be. All we need is a couple additional cobra.Command objects for signer and key and add the new commands to them instead of the Command for trust.

riyazdf · 2017-10-04T03:27:00Z

@dnephin @thaJeztah : sounds good! Are we okay with these being one of the only (if not the only) commands in docker/cli with four levels of subcommands?

If there's consensus I'm happy to make the change.

thaJeztah · 2017-10-04T10:18:26Z

I'm OK with it; it may be slightly awkward to have the commands nested this deep, but with the context given above, I think it's the best option (and most consistent with other commands as in <something> add)

ping @docker/core-cli-maintainers PTAL

albers · 2017-10-04T12:50:37Z

The completions are not equipped for another nesting level, so I'd prefer the less deeply nested forms.

vieux · 2017-10-05T22:02:02Z

I'm ok with docker trust signer add and docker trust key load

We almost went for docker swarm node update instead of a top leveldocker node so I'm not shocked by this depth

riyazdf · 2017-10-06T03:43:30Z

Ok! From discussions with @dnephin, @thaJeztah, @vdemeester, @stevvooe, and @vieux : it sounds like docker trust signer add et al. is the best path forward. I will update this PR shortly.

I'm not sure what the implications are for bash completions, is it possible to make it work with this level deep of commands?

albers · 2017-10-06T12:23:16Z

For bash completion it will be 0.5 to 3 days of work.
The current implementation uses global variables for the command and subcommand positions. It might be hard to turn this into a proper generic solution.

I'm also not sure about the implications on zsh and fish completion.
@sdurrheimer can zsh completion deal with an additional level of subcommands?

riyazdf · 2017-10-10T17:46:16Z

@albers: got it.

I've moved the commands around to be a level lower, now we have:

🐳 $ ./docker-darwin-amd64 trust

Usage:	docker trust COMMAND

Manage trust on Docker images (experimental)

Options:
      --help   Print usage

Management Commands:
  key         Manage keys for signing Docker images (experimental)
  signer      Manage entities who can sign Docker images (experimental)

Commands:
  revoke      Remove trust for an image
  sign        Sign an image
  view        Display detailed information about keys and signatures

Run 'docker trust COMMAND --help' for more information on a command.

dnephin

Thanks!

The UX generally seems good to me, although I'm not all that familiar with the workflow.

The design also generally seems good, the only major issue is the addition of 12 new unnecessary dependencies. I think this needs to be fixed in notary before we can merge.

Comments are mostly code review, small changes to match conventions and refactors to reduce arg counts.

dnephin · 2017-10-12T20:23:54Z

cli/command/trust/key_generate.go

+func setupPassphraseAndGenerateKeys(streams command.Streams, keyName string) error {
+	// always use a fresh passphrase for each key generation
+	freshPassRetGetter := func() notary.PassRetriever { return trust.GetPassphraseRetriever(streams.In(), streams.Out()) }
+	cwd, err := os.Getwd()


Maybe this could have a flag to set the target directory, and default to the working directory if the flag isn't set? I'm not sure where these keys would normally be placed.

agreed! Added.

dnephin · 2017-10-12T20:26:26Z

cli/command/trust/key_generate.go

+	return validateAndGenerateKey(streams, keyName, cwd, freshPassRetGetter)
+}
+
+func validateAndGenerateKey(streams command.Streams, keyName string, workingDir string, passphraseGetter func() notary.PassRetriever) error {


passphraseGetter seems like an unnecessary argument because it could be created in this function from streams.

keyName and directory could be part of an options struct (if it makes sense to allow setting of a target directory).

dnephin · 2017-10-12T20:29:44Z

cli/command/trust/key_generate.go

+	return nil
+}
+
+func generateKey(keyName, pubDir, privTrustDir string, passRet notary.PassRetriever) error {


privTrustDir and passRet are only used once as args to the same function, so they can be replaced with a single arg keyStore.

pubDir is only used once, at the very end of the function, and I believe the line where it's used is actually duplicated by the caller, so it seems like it could be removed as well, and this function could return the pubPEM. The file write could be done by the caller, which would remove the duplicate line, and the extra arg.

maybe something like:

func generatePublicPEM(keyName string, keyStore trust.KeyStore) *pem.Block { ... }

dnephin · 2017-10-12T20:52:21Z

cli/command/trust/key_load.go

+	return nil
+}
+
+func loadPrivKeyFromPath(privKeyImporters []utils.Importer, keyPath, keyName string, passRet notary.PassRetriever) error {


privKeyImporters, keyName, and passRet are all only used once, as args to the same function, on the very last line of this function. Instead of passing in these 3 args, the function could return a io.Reader, and ImportKeys can be called from the caller of this function.

func getPrivKeyFromPath(keyPath string) (error, io.Reader) { ... return bytes.NewReader(keyBytes), nil }

dnephin · 2017-10-12T20:57:33Z

cli/command/trust/key_load_test.go

+
+func TestLoadKeyFromPath(t *testing.T) {
+	for keyID, keyBytes := range testKeys {
+		testLoadKeyFromPath(t, keyID, keyBytes)


To make it easier to debug failures in this test I would suggest wrapping this call in a https://golang.org/pkg/testing/#T.Run and using the keyID as the name (or possibly a more descriptive string).

That will also let you replace the assert.NoError() calls with require.NoError() and still test both cases if there is a failure in the first (because t.Fatal() only exits the t.Run())

same below in TestLoadKeyTooPermissive

dnephin · 2017-10-12T21:07:02Z

cli/command/trust/signer_remove.go

+		Short: "Remove a signer",
+		Args:  cli.RequiresMinArgs(2),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			return removeSigner(dockerCli, args[0], args[1:], &options)


Most commands (including newSignerAddCommand above) pass args using the options struct instead of separate args. I think it would be good to do that here as well.

Also it shouldn't be necessary to use a pointer for options. The values should never be modified, so I believe we pass a struct (not a pointer) in all other commands.

dnephin · 2017-10-12T21:07:30Z

cli/command/trust/signer_remove.go

+		},
+	}
+	flags := cmd.Flags()
+	flags.BoolVarP(&options.forceYes, "yes", "y", false, "Answer yes to removing most recent signer (no confirmation)")


This should probably say something like "do not prompt for confirmation before ..." or something along those lines

dnephin · 2017-10-12T21:10:09Z

cli/command/trust/signer_remove.go

+	return counter < releasesRoleWithSigs.Threshold, nil
+}
+
+func removeSingleSigner(cli command.Cli, image, signerName string, forceYes bool) error {


When signerName is part of options I think signerName and forceYes can be replaced with options signerRemoveOptions

I left this function signature as is because otherwise there's a weird discrepancy between the unpacked image named and otherwise packed options.

I don't really see how this is a problem. It's quite common for a function to only use some fields or methods on a struct.

@dnephin: I don't think that is a safe practice: this is an inner helper function that only needs to know about one repo. Including the extra images in the slice is extraneous info, and I worry that a future contribution may misuse it.

That said, if you feel strongly about changing to the signerRemoveOptions, you are the maintainer and I will make the change - just let me know.

dnephin · 2017-10-12T21:11:05Z

cli/command/trust/signer_add.go

+
+var validSignerName = regexp.MustCompile(`^[a-z0-9]+[a-z0-9\_\-]*$`).MatchString
+
+func addSigner(cli command.Cli, options *signerAddOptions) error {


should accept the struct, not pointer to struct

dnephin · 2017-10-12T21:18:18Z

vendor.conf

+github.com/bugsnag/bugsnag-go 13fd6b8acda029830ef9904df6b63be0a83369d0
+github.com/bugsnag/osext 0dd3f918b21bec95ace9dc86c7e70266cfc5c702
+github.com/bugsnag/panicwrap e2c28503fcd0675329da73bf48b33404db873782
+github.com/BurntSushi/toml a368813c5e648fee92e5f6c30e3944ff9d5e8895


This is bringing in a whole lot of unnecessary dependencies.

I believe the problem is github.com/docker/notary/util. I think this illustrates one of the major problems with a utils package.

from https://blog.golang.org/package-names

Avoid meaningless package names. Packages named util ... provide clients with no sense of what the package contains. This makes it harder for clients to use the package and makes it harder for maintainers to keep the package focused. Over time, they accumulate dependencies that can make compilation significantly and unnecessarily slower, especially in large programs.

A generic package like utils tends to collect a bunch of unrelated functions/types that have completely different dependencies which means that using any of the functions results in the consumer having to depend on a bunch of unrelated things (sql libraryes, logging libraries, etc).

I think this needs to be fixed in notary and anything used in docker/cli needs to be moved out of utils so we can use it without bringing in a dozen unnecessary packages.

I'm looking into it - it's possible that this may not be so clean because the notary server-side services also use a key storage interface with connects with the DB (hence many of the imports). I'll get back to you with an update.

riyazdf · 2017-10-20T15:35:21Z

thanks @dnephin for the review! I'm looking into package refactors on docker/notary and have applied your other suggestions :)

riyazdf · 2017-10-23T09:25:07Z

@dnephin: great news! I have a fix in flight for upstream notary that will remove all vendoring except for an updated notary commit hash.

We would love to move this forward before the next release - please let me know if there are other review comments I can address :)

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

riyazdf · 2017-10-24T09:32:13Z

updated: the only vendoring is now limited to an updated docker/notary commit sha. Excited to move forward with this!

dnephin

Thanks! vendor looks good now. Left some comments about error handling and output streams.

dnephin · 2017-10-25T16:08:28Z

cli/command/trust/key_generate.go

+	pubPEM, err := generateKeyAndOutputPubPEM(keyName, privKeyFileStore)
+	if err != nil {
+		fmt.Fprintf(streams.Out(), err.Error())
+		return fmt.Errorf("Error generating key for: %s", keyName)


I don't think it's correct to print an error on stdout. This should return an error without the fmt.Fprintf()

return errors.Wrapf(err, "failed to generate key for %s", keyName)

dnephin · 2017-10-25T16:10:04Z

cli/command/trust/key_generate.go

+	pubFileName := strings.Join([]string{keyName, "pub"}, ".")
+	pubFilePath := filepath.Join(workingDir, pubFileName)
+	if err := ioutil.WriteFile(pubFilePath, pem.EncodeToMemory(&pubPEM), notary.PrivNoExecPerms); err != nil {
+		return "", fmt.Errorf("Error writing public key to location: %s", pubFilePath)


Maybe wrap the original error so it's easier to debug? Also errors are supposed to always start with lowercase so that they can be wrapped by other code.

return "', errors.Wrapf(err, "failed to write public key to %s", pubFilePath)

dnephin · 2017-10-25T16:11:41Z

cli/command/trust/key_load.go

+	passRet := trust.GetPassphraseRetriever(streams.In(), streams.Out())
+	keyBytes, err := getPrivKeyBytesFromPath(keyPath)
+	if err != nil {
+		return fmt.Errorf("error reading key from %s: %s", keyPath, err)


return errors.Wrapf(err, "error reading key from %s", keyPath)

dnephin · 2017-10-25T16:11:51Z

cli/command/trust/key_load.go

+		return fmt.Errorf("error reading key from %s: %s", keyPath, err)
+	}
+	if err := loadPrivKeyBytesToStore(keyBytes, privKeyImporters, keyPath, options.keyName, passRet); err != nil {
+		return fmt.Errorf("error importing key from %s: %s", keyPath, err)


errors.Wrapf()

dnephin · 2017-10-25T16:14:30Z

cli/command/trust/key_load.go

+		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)


What about 700, or 200? Maybe this could say "private key must not be readable by others" or something like that?

I think the ssh client says something like this: "It is required that your private key files are NOT accessible by others."

I don't think 200 or 700 make sense here, for 200 you'd have writable but not readable (I've never seen this done?), and with 700 the key would be executable (also don't see a use for this?).

I agree those would be silly permissions. My point is that the error message is misleading. The requirement is that it can't be readable by others, not that it must be one of those two permissions. The error message should match the requirement. The ssh client is a good example of this.

If I'm not mistaken this error message will also be used on windows. "set to 400 or 600" is meaningless to windows users.

Ah, I just noticed the code does actually check for specifically those permissions. I think this needs to be changed to just check that the specific bits are not set.

Something like this?

mode & 077 == 0

sounds good, updated!

dnephin · 2017-10-25T16:17:20Z

cli/command/trust/signer_add.go

+	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())


Should go to cli.Err() instead of stdout

dnephin · 2017-10-25T16:19:15Z

cli/command/trust/signer_add.go

+			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)


Maybe errors.Wrapf() to make it easier to debug with the original error.

dnephin · 2017-10-25T16:20:13Z

cli/command/trust/signer_remove.go

+	var errImages []string
+	for _, image := range options.images {
+		if err := removeSingleSigner(cli, image, options.signer, options.forceYes); err != nil {
+			fmt.Fprintln(cli.Out(), err.Error())


s/cli.Out()/cli.Err()/

dnephin · 2017-10-25T16:21:08Z

cli/command/trust/signer_remove.go

+	}
+	delegationRoles, err := notaryRepo.GetDelegationRoles()
+	if err != nil {
+		return fmt.Errorf("Error retrieving signers for %s", imageName)


Maybe errors.Wrapf()?

dnephin · 2017-10-25T16:23:21Z

cli/command/trust/signer_remove.go

+			return nil
+		}
+	} else if err != nil {
+		fmt.Fprintln(cli.Out(), err.Error())


Shouldn't this return the error here to exit early?

If not I guess just s/cli.Out/cli.Err/

thaJeztah · 2017-10-26T13:36:12Z

I also see there's still quite a few places that print "informational" information on stdout; that may be problematic if people want to script these things (perhaps not a concern, given that passwords have to be entered interactively)

riyazdf · 2017-10-26T15:32:42Z

@thaJeztah: thank you for the thorough review! I think I've addressed all comments.

As for the bug you found: I am not able to repro so maybe it is an odd tty issue in the container?

dnephin

LGTM

dnephin · 2017-10-27T17:29:19Z

cli/command/trust/signer_add.go

+	for _, imageName := range options.images {
+		fmt.Fprintf(cli.Out(), "Adding signer \"%s\" to %s...\n", signerName, imageName)
+		if err := addSignerToImage(cli, signerName, imageName, signerPubKeys); err != nil {
+			fmt.Fprintln(cli.Err(), err.Error()+"\n")


Fprintln() already adds a newline, but there's also a \n at the end of the message. Are you sure we want an extra blank line between these?

I added an extra blank line to separate the signer output if there are multiple ex:

$ 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 Adding signer "alice" to example/trust-demo2... Enter passphrase for repository key with ID ece554f: Successfully added signer: alice to example/trust-demo2

Let me know if you'd prefer for the blank line to be removed, but I think it helps improve readability

dnephin · 2017-10-27T17:29:56Z

cli/command/trust/signer_remove.go

+	for _, image := range options.images {
+		fmt.Fprintf(cli.Out(), "Removing signer \"%s\" from %s...\n", options.signer, image)
+		if err := removeSingleSigner(cli, image, options.signer, options.forceYes); err != nil {
+			fmt.Fprintln(cli.Err(), err.Error()+"\n")


dnephin · 2017-10-27T17:30:11Z

cli/command/trust/signer_remove.go

+			fmt.Fprintln(cli.Err(), err.Error()+"\n")
+			errImages = append(errImages, image)
+		} else {
+			fmt.Fprintf(cli.Out(), "Successfully removed %s from %s\n\n", options.signer, image)


same here, seems to be an extra blank line

thaJeztah

left some comments

thaJeztah · 2017-10-26T23:45:54Z

cli/command/trust/helpers.go

+	if err != nil {
+		return err
+	}
+	// Initialize the notary repository with a remotely managed snapshot


thanks; then it can be removed here 😉

thaJeztah · 2017-10-30T11:12:14Z

docs/reference/commandline/trust_key_generate.md

+# trust key generate
+
+```markdown
+Usage:  docker trust key generate NAME


This should match the actual output;

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

thaJeztah · 2017-10-30T11:15:43Z

cli/command/trust/key_load.go

+)
+
+const (
+	nonOwnerReadWritePerms = 0077


Should this be named nonOwnerReadWriteMask ? (e.g.)?

thaJeztah · 2017-10-30T11:17:09Z

cli/command/trust/key_load.go

+		},
+	}
+	flags := cmd.Flags()
+	flags.StringVarP(&options.keyName, "name", "n", "signer", "Name for the loaded key")


Sorry, didn't look at this one in my previous review; let's remove the shorthand -n for now (we can add it back later if we want to) (we don't have a -n shorthand for --name so far in our UX)

thaJeztah · 2017-10-30T11:18:00Z

docs/reference/commandline/trust_key_load.md

+# trust key load
+
+```markdown
+Usage:  docker trust key load [OPTIONS] KEY


Same here:

Usage: docker trust key load [OPTIONS] KEY Load a private key file for signing Options: --help Print usage -n, --name string Name for the loaded key (default "signer")

Or (when removing the -n shorthand);

Usage: docker trust key load [OPTIONS] KEY Load a private key file for signing Options: --help Print usage --name string Name for the loaded key (default "signer")

thaJeztah · 2017-10-30T12:42:33Z

docs/reference/commandline/trust_signer_remove.md

+# trust signer remove
+
+```markdown
+Usage:  docker trust signer remove [OPTIONS] NAME IMAGE [IMAGE...]


Usage: docker trust signer remove [OPTIONS] NAME IMAGE [IMAGE...] Remove a signer Options: --help Print usage -y, --yes Do not prompt for confirmation before removing the most recent signer

thaJeztah · 2017-10-30T12:43:17Z

docs/reference/commandline/trust_signer_remove.md

+To remove an existing signer, `alice`, from this repository: 
+
+```bash
+$ docker trust inspect example/trust-demo


s/inspect/view/

thaJeztah · 2017-10-30T12:45:22Z

docs/reference/commandline/trust_signer_remove.md

+To remove an existing signer, `alice`, from multiple repositories: 
+
+```bash
+$ docker trust inspect example/trust-demo


s/inspect/view/

thaJeztah · 2017-10-30T12:46:53Z

cli/command/trust/signer_remove.go

+
+func removeSigner(cli command.Cli, options signerRemoveOptions) error {
+	var errImages []string
+	for _, image := range options.images {


Looks like we don't require a :tag for these images as well

thaJeztah · 2017-10-30T12:47:42Z

docs/reference/commandline/trust_signer_add.md

+To add a new signer, `alice`, to this repository: 
+
+```bash
+$ docker trust inspect example/trust-demo


s/inspect/view/

(some other places as well)

thaJeztah · 2017-10-30T12:52:28Z

cli/command/trust/signer_remove.go

+		},
+	}
+	flags := cmd.Flags()
+	flags.BoolVarP(&options.forceYes, "yes", "y", false, "Do not prompt for confirmation before removing the most recent signer")


Noticed that for docker xx prune, we're using --force for this;

Usage: docker image prune [OPTIONS] Remove unused images Options: -a, --all Remove all unused images, not just dangling ones --filter filter Provide filter values (e.g. 'until=<timestamp>') -f, --force Do not prompt for confirmation --help Print usage

I'll update to force 👍

thaJeztah

changes LGTM, but can you squash the commits where applicable?

thaJeztah · 2017-10-30T15:42:04Z

cli/command/trust/key_load.go

 	return trustmanager.ImportKeys(bytes.NewReader(privKeyBytes), privKeyImporters, keyName, "", passRet)
 }
+
+func decodePrivKeyIfNecessary(privPemBytes []byte, passRet notary.PassRetriever) ([]byte, error) {


Would be good to have a unit-test for this one, but I'm okay adding that in a follow-up

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

riyazdf · 2017-10-30T16:03:40Z

thanks @thaJeztah: I've cut down to ~half the commits!

thaJeztah · 2017-10-30T16:03:52Z

Thanks! Discussed with @riyazdf and further squashing proved to be difficult, so I'm good with the current number of commits 👍

riyazdf requested review from mdlinville, thaJeztah and vdemeester as code owners September 27, 2017 17:26

GordonTheTurtle added the status/0-triage label Sep 27, 2017

thaJeztah added status/1-design-review and removed status/0-triage labels Sep 27, 2017

riyazdf force-pushed the docker-trust-2 branch 3 times, most recently from a3eeeaf to 0dc16d3 Compare October 10, 2017 17:24

dnephin suggested changes Oct 12, 2017

View reviewed changes

riyazdf added 2 commits October 24, 2017 11:23

vendor: update notary and deps for new trust commands

f47b1a3

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

trust: define new commands and helpers

5ab3ae7

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

riyazdf force-pushed the docker-trust-2 branch 2 times, most recently from 559ffc0 to 9e25404 Compare October 24, 2017 09:31

dnephin reviewed Oct 25, 2017

View reviewed changes

riyazdf force-pushed the docker-trust-2 branch 2 times, most recently from 07a62d8 to 3b45ad3 Compare October 27, 2017 10:13

dnephin approved these changes Oct 27, 2017

View reviewed changes

thaJeztah requested changes Oct 30, 2017

View reviewed changes

thaJeztah reviewed Oct 30, 2017

View reviewed changes

thaJeztah mentioned this pull request Oct 30, 2017

Move notary to its new location #652

Merged

thaJeztah approved these changes Oct 30, 2017

View reviewed changes

thaJeztah added status/2-code-review and removed status/1-design-review labels Oct 30, 2017

ashfall and others added 4 commits October 30, 2017 16:53

trust: add signer-add and signer-remove command

dde9f61

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

trust: key-load and key-generate code

604bc3f

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

docs: add docs for new trust subcommands

4e797ea

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

trust: move signer and key commands down one level

532d223

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

riyazdf force-pushed the docker-trust-2 branch from 4f805ef to 4ec0cc9 Compare October 30, 2017 15:54

riyazdf added 4 commits October 30, 2017 16:55

refactoring and adding tests for EC key types

2d8cc3c

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

revendor notary for updated import/export packages, update with rebase

b4ef2dd

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

update to stderr instead of stdout, update tests

079471e

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

review feedback: updating for windows, error paths

e189a21

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

riyazdf force-pushed the docker-trust-2 branch from 4ec0cc9 to e189a21 Compare October 30, 2017 15:56

thaJeztah added impact/changelog impact/documentation kind/experimental labels Oct 30, 2017

thaJeztah merged commit 0c4fa69 into docker:master Oct 30, 2017

GordonTheTurtle added this to the 17.11.0 milestone Oct 30, 2017

riyazdf deleted the docker-trust-2 branch October 30, 2017 16:04

thaJeztah mentioned this pull request Dec 6, 2017

Add support for custom passphrase-validation notaryproject/notary#1272

Open


		var validSignerName = regexp.MustCompile(`^[a-z0-9]+[a-z0-9\_\-]*$`).MatchString

		func addSigner(cli command.Cli, options *signerAddOptions) error {

docker trust: interact with signers and keys #566

docker trust: interact with signers and keys #566

Uh oh!

Conversation

riyazdf commented Sep 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Sep 27, 2017

Uh oh!

riyazdf commented Oct 2, 2017

Uh oh!

dnephin commented Oct 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Oct 3, 2017

Uh oh!

dnephin commented Oct 3, 2017

Uh oh!

riyazdf commented Oct 4, 2017

Uh oh!

thaJeztah commented Oct 4, 2017

Uh oh!

albers commented Oct 4, 2017

Uh oh!

vieux commented Oct 5, 2017

Uh oh!

riyazdf commented Oct 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

albers commented Oct 6, 2017

Uh oh!

riyazdf commented Oct 10, 2017

Uh oh!

dnephin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dnephin Oct 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dnephin Oct 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

riyazdf commented Oct 20, 2017

Uh oh!

riyazdf commented Oct 23, 2017

Uh oh!

riyazdf commented Oct 24, 2017

Uh oh!

dnephin left a comment

Choose a reason for hiding this comment

Uh oh!

riyazdf commented Sep 27, 2017 •

edited

Loading

dnephin commented Oct 2, 2017 •

edited

Loading

riyazdf commented Oct 6, 2017 •

edited

Loading

dnephin left a comment •

edited

Loading

dnephin Oct 12, 2017 •

edited

Loading

dnephin Oct 12, 2017 •

edited

Loading

dnephin Oct 25, 2017 •

edited

Loading