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

Synchronous CLI command for root CA rotation #48

Merged
merged 3 commits into from May 16, 2017

Conversation

Projects
None yet
8 participants
@cyli
Contributor

cyli commented May 8, 2017

Provide command line tool to view and rotate swarm's currently CA root certificate.

Signed-off-by: Ying Li ying.li@docker.com

This code was originally in moby/moby#32993.

root@ubuntu:~#  docker swarm ca
-----BEGIN CERTIFICATE-----
MIIBazCCARCgAwIBAgIUJPzo67QC7g8Ebg2ansjkZ8CbmaswCgYIKoZIzj0EAwIw
EzERMA8GA1UEAxMIc3dhcm0tY2EwHhcNMTcwNTAzMTcxMDAwWhcNMzcwNDI4MTcx
MDAwWjATMREwDwYDVQQDEwhzd2FybS1jYTBZMBMGByqGSM49AgEGCCqGSM49AwEH
A0IABKL6/C0sihYEb935wVPRA8MqzPLn3jzou0OJRXHsCLcVExigrMdgmLCC+Va4
+sJ+SLVO1eQbvLHH8uuDdF/QOU6jQjBAMA4GA1UdDwEB/wQEAwIBBjAPBgNVHRMB
Af8EBTADAQH/MB0GA1UdDgQWBBSfUy5bjUnBAx/B0GkOBKp91XvxzjAKBggqhkjO
PQQDAgNJADBGAiEAnbvh0puOS5R/qvy1PMHY1iksYKh2acsGLtL/jAIvO4ACIQCi
lIwQqLkJ48SQqCjG1DBTSBsHmMSRT+6mE2My+Z3GKA==
-----END CERTIFICATE-----

root@ubuntu:~# docker swarm ca --help

Usage:	docker swarm ca [OPTIONS]

Manage root CA

Options:
      --ca-cert pem-file          Path to the PEM-formatted root CA certificate to use for the new cluster
      --ca-key pem-file           Path to the PEM-formatted root CA key to use for the new cluster
      --cert-expiry duration      Validity period for node certificates (ns|us|ms|s|m|h) (default 2160h0m0s)
  -d, --detach                    Exit immediately instead of waiting for the root rotation to converge
      --external-ca external-ca   Specifications of one or more certificate signing endpoints
      --help                      Print usage
  -q, --quiet                     Suppress progress output
      --rotate                    Rotate the swarm CA - if no certificate or key are provided, new ones will be generated

Sample progress bar here, copied styling from the synchronous service create progress bar: https://asciinema.org/a/2em3i7y1u50ajeut13es5m6uh

Note: I originally thought about just doing docker swarm update --rotate-ca cert=/path/...,key=/path/... but I wasn't sure how to also do tell the CLI to just generate a cert and key - it'd probably have to look like docker swarm update --rotate-ca "" which seems weird, hence the new command.

Also a suggestion from @NathanMcCauley: if we have service identities, and those could be rotated, do we want to use the same CLI command to view/rotate those as well?

@cyli cyli changed the title from WIP: CLI command for root CA rotation to WIP: Synchronous CLI command for root CA rotation May 9, 2017

@cyli cyli referenced this pull request May 10, 2017

Merged

Root CA rotation CLI docs #33152

@cyli cyli changed the title from WIP: Synchronous CLI command for root CA rotation to Synchronous CLI command for root CA rotation May 12, 2017

@cyli

This comment has been minimized.

Show comment
Hide comment
@cyli

cyli May 12, 2017

Contributor

Have re-vendored the latest version of moby/moby master instead of my fork, so this is ready for review.

Contributor

cyli commented May 12, 2017

Have re-vendored the latest version of moby/moby master instead of my fork, so this is ready for review.

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah May 15, 2017

Member

Looking at this again, I wonder if;

  • swarm ca should be a separate command; it's showing a single field in the swarm configuration, does this open the door to creating a separate command for each property? Is this information part of docker system info (or should we have docker swarm info? Possibly --rotate should be part of docker swarm update
  • if we do think ca should be its own "entity" / "object type" in the CLI, should it follow the ls / list / inspect etc subcommands? (will be a bit odd as well, given that there's only a single CA, not a list)
  • If we want to keep it similar to join-token, then swarm ca --rotate should print the ca after it was rotated

Happy to hear thoughts @aaronlehmann @diogomonica @dnephin

Member

thaJeztah commented May 15, 2017

Looking at this again, I wonder if;

  • swarm ca should be a separate command; it's showing a single field in the swarm configuration, does this open the door to creating a separate command for each property? Is this information part of docker system info (or should we have docker swarm info? Possibly --rotate should be part of docker swarm update
  • if we do think ca should be its own "entity" / "object type" in the CLI, should it follow the ls / list / inspect etc subcommands? (will be a bit odd as well, given that there's only a single CA, not a list)
  • If we want to keep it similar to join-token, then swarm ca --rotate should print the ca after it was rotated

Happy to hear thoughts @aaronlehmann @diogomonica @dnephin

@cyli

This comment has been minimized.

Show comment
Hide comment
@cyli

cyli May 15, 2017

Contributor

Possibly --rotate should be part of docker swarm update

I originally thought about just doing docker swarm update --rotate-ca cert=/path/...,key=/path/... but I wasn't sure how to also do tell the CLI to just generate a cert and key - it'd probably have to look like docker swarm update --rotate-ca "" which seems weird, hence the new command - but happy to have other opinions and suggestions instead - maybe the cert and key can be different flags?

The one argument I'd have for a separate command is that docker swarm update already has a bunch of flags, and the extra ones here (--ca-cert, --ca-key, -d and -q) for instance all seem to be bundled together, so maybe including them in a separate command makes sense?

Is this information part of docker system info (or should we have docker swarm info)?

Some of this info is currently on docker system info - I don't mind adding a docker swarm info, if that'd be useful - it seems like system info includes swarm info though.

If we want to keep it similar to join-token, then swarm ca --rotate should print the ca after it was rotated.

Happy to do that as well.

Contributor

cyli commented May 15, 2017

Possibly --rotate should be part of docker swarm update

I originally thought about just doing docker swarm update --rotate-ca cert=/path/...,key=/path/... but I wasn't sure how to also do tell the CLI to just generate a cert and key - it'd probably have to look like docker swarm update --rotate-ca "" which seems weird, hence the new command - but happy to have other opinions and suggestions instead - maybe the cert and key can be different flags?

The one argument I'd have for a separate command is that docker swarm update already has a bunch of flags, and the extra ones here (--ca-cert, --ca-key, -d and -q) for instance all seem to be bundled together, so maybe including them in a separate command makes sense?

Is this information part of docker system info (or should we have docker swarm info)?

Some of this info is currently on docker system info - I don't mind adding a docker swarm info, if that'd be useful - it seems like system info includes swarm info though.

If we want to keep it similar to join-token, then swarm ca --rotate should print the ca after it was rotated.

Happy to do that as well.

@diogomonica

This comment has been minimized.

Show comment
Hide comment
@diogomonica

diogomonica May 15, 2017

Contributor

I have a slight preference for having the ca subcommand

Contributor

diogomonica commented May 15, 2017

I have a slight preference for having the ca subcommand

@diogomonica

This comment has been minimized.

Show comment
Hide comment
@diogomonica

diogomonica May 15, 2017

Contributor

LGTM

Contributor

diogomonica commented May 15, 2017

LGTM

@cyli

This comment has been minimized.

Show comment
Hide comment
@cyli

cyli May 16, 2017

Contributor

Updated the --rotate command to print out the final CA certificate after the root rotation is complete.

Contributor

cyli commented May 16, 2017

Updated the --rotate command to print out the final CA certificate after the root rotation is complete.

@aaronlehmann

This comment has been minimized.

Show comment
Hide comment
@aaronlehmann

aaronlehmann May 16, 2017

Collaborator

LGTM

Collaborator

aaronlehmann commented May 16, 2017

LGTM

Show outdated Hide outdated cli/command/swarm/ca.go Outdated
@thaJeztah

left one comment, LGTM otherwise

Args: cli.NoArgs,
RunE: func(cmd *cobra.Command, args []string) error {
return runRotateCA(dockerCli, cmd.Flags(), opts)
},

This comment has been minimized.

@thaJeztah

thaJeztah May 16, 2017

Member

Can you add

Tags: map[string]string{"version": "1.30"},

like we have on cli/command/system/prune.go#L31, so that the command is hidden when talking to an older daemon, and produces an error that it requires a newer daemon

@thaJeztah

thaJeztah May 16, 2017

Member

Can you add

Tags: map[string]string{"version": "1.30"},

like we have on cli/command/system/prune.go#L31, so that the command is hidden when talking to an older daemon, and produces an error that it requires a newer daemon

cyli and others added some commits May 8, 2017

Provide command line tool to view and rotate swarm's currently CA roo…
…t certificate.

Signed-off-by: Ying Li <ying.li@docker.com>
Re-vendor docker/docker.
Signed-off-by: Ying Li <ying.li@docker.com>
Avoid loop when discarding contents of progress pipe
Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
@aaronlehmann

This comment has been minimized.

Show comment
Hide comment
@aaronlehmann

aaronlehmann May 16, 2017

Collaborator

@tonistiigi: Pushed a commit that simplifies the use of io.Copy

Collaborator

aaronlehmann commented May 16, 2017

@tonistiigi: Pushed a commit that simplifies the use of io.Copy

@tonistiigi

This comment has been minimized.

Show comment
Hide comment
@tonistiigi

tonistiigi May 16, 2017

Member

LGTM

Member

tonistiigi commented May 16, 2017

LGTM

@aaronlehmann aaronlehmann merged commit c17acee into docker:master May 16, 2017

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
dco-signed All commits are signed

@GordonTheTurtle GordonTheTurtle added this to the 17.06.0 milestone May 16, 2017

@cyli cyli deleted the cyli:root-rotation-cli branch May 16, 2017

@cyli

This comment has been minimized.

Show comment
Hide comment
@cyli

cyli May 16, 2017

Contributor

Thanks @aaronlehmann!

Contributor

cyli commented May 16, 2017

Thanks @aaronlehmann!

@BillMills

This comment has been minimized.

Show comment
Hide comment
@BillMills

BillMills Jun 19, 2017

Hi folks - Training wants to highlight this feature in our workshops, but I'm a bit confused; docker swarm ca --cert-expiry 720h spits a key back at me on 17.06.0-ce-rc2 but shows no change to the CA configuration / expiry duration key in the output of docker system info. I assumed (wrongly?) this command was equivalent to docker swarm update --cert-expiry 720h, which does show an updated expiry duration immediately. Thoughts?

BillMills commented Jun 19, 2017

Hi folks - Training wants to highlight this feature in our workshops, but I'm a bit confused; docker swarm ca --cert-expiry 720h spits a key back at me on 17.06.0-ce-rc2 but shows no change to the CA configuration / expiry duration key in the output of docker system info. I assumed (wrongly?) this command was equivalent to docker swarm update --cert-expiry 720h, which does show an updated expiry duration immediately. Thoughts?

@cyli

This comment has been minimized.

Show comment
Hide comment
@cyli

cyli Jun 19, 2017

Contributor

@BillMills Ah, apologies, that was intended to be used with the --rotate flag (e.g. you can rotate the CA cert and update the cert expiry to be used with that new cert at the same time. If you don't pass --rotate, it ignores all the other flags, since cert expiry could also be changed with docker swarm update --cert-expiry ....

Wondering if we should update the flags, if used by themselves, to specify that they are ignored unless --rotate is passed?

Contributor

cyli commented Jun 19, 2017

@BillMills Ah, apologies, that was intended to be used with the --rotate flag (e.g. you can rotate the CA cert and update the cert expiry to be used with that new cert at the same time. If you don't pass --rotate, it ignores all the other flags, since cert expiry could also be changed with docker swarm update --cert-expiry ....

Wondering if we should update the flags, if used by themselves, to specify that they are ignored unless --rotate is passed?

@dnephin

This comment has been minimized.

Show comment
Hide comment
@dnephin

dnephin Jun 19, 2017

Collaborator

A warning would be great. We might even want to consider it an error.

Collaborator

dnephin commented Jun 19, 2017

A warning would be great. We might even want to consider it an error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment