1berry/docker-cli
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Propagate the provided external CA certificate to the external CA object

Browse Source 
in swarm.

Also, fix some CLI command confusions:
1. If the --external-ca flag is provided, require a --ca-cert flag as well, otherwise
   the external CA is set but the CA certificate is actually rotated to an internal
   cert
2. If a --ca-cert flag is provided, require a --ca-key or --external-ca flag be
   provided as well, otherwise either the server will say that the request is
   invalid, or if there was previously an external CA corresponding to the cert, it
   will succeed.  While that works, it's better to require the user to explicitly
   set all the parameters of the new desired root CA.

This also changes the `swarm update` function to set the external CA's CACert field,
which while not strictly necessary, makes the CA list more explicit.

Signed-off-by: Ying Li <ying.li@docker.com>
This commit is contained in:
Ying Li 2018-07-02 16:53:23 -07:00
parent f5393c904a
commit 4243440e1f
5 changed files with 233 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
cli/command/swarm/ca.go
View File

@ -68,6 +68,17 @@ func runCA(dockerCli command.Cli, flags *pflag.FlagSet, opts caOptions) error {
return displayTrustRoot(dockerCli.Out(), swarmInspect)
}
if flags.Changed(flagExternalCA) && len(opts.externalCA.Value()) > 0 && !flags.Changed(flagCACert) {
return fmt.Errorf(
"rotating to an external CA requires the `--%s` flag to specify the external CA's cert - "+
"to add an external CA with the current root CA certificate, use the `update` command instead", flagCACert)
}
if flags.Changed(flagCACert) && len(opts.externalCA.Value()) == 0 && !flags.Changed(flagCAKey) {
return fmt.Errorf("the --%s flag requires that a --%s flag and/or --%s flag be provided as well",
flagCACert, flagCAKey, flagExternalCA)
}
updateSwarmSpec(&swarmInspect.Spec, flags, opts)
if err := client.SwarmUpdate(ctx, swarmInspect.Version, swarmInspect.Spec, swarm.UpdateFlags{}); err != nil {
return err
@ -80,20 +91,15 @@ func runCA(dockerCli command.Cli, flags *pflag.FlagSet, opts caOptions) error {
}
func updateSwarmSpec(spec *swarm.Spec, flags *pflag.FlagSet, opts caOptions) {
opts.mergeSwarmSpecCAFlags(spec, flags)
caCert := opts.rootCACert.Contents()
caKey := opts.rootCAKey.Contents()
opts.mergeSwarmSpecCAFlags(spec, flags, caCert)
spec.CAConfig.SigningCACert = caCert
spec.CAConfig.SigningCAKey = caKey
if caCert != "" {
spec.CAConfig.SigningCACert = caCert
}
if caKey != "" {
spec.CAConfig.SigningCAKey = caKey
}
if caKey == "" && caCert == "" {
spec.CAConfig.ForceRotate++
spec.CAConfig.SigningCACert = ""
spec.CAConfig.SigningCAKey = ""
}
}

258
cli/command/swarm/ca_test.go
View File

@ -13,6 +13,28 @@ import (
is "gotest.tools/assert/cmp"
)
const (
cert = `
-----BEGIN CERTIFICATE-----
MIIBuDCCAV4CCQDOqUYOWdqMdjAKBggqhkjOPQQDAzBjMQswCQYDVQQGEwJVUzEL
MAkGA1UECAwCQ0ExFjAUBgNVBAcMDVNhbiBGcmFuY2lzY28xDzANBgNVBAoMBkRv
Y2tlcjEPMA0GA1UECwwGRG9ja2VyMQ0wCwYDVQQDDARUZXN0MCAXDTE4MDcwMjIx
MjkxOFoYDzMwMTcxMTAyMjEyOTE4WjBjMQswCQYDVQQGEwJVUzELMAkGA1UECAwC
Q0ExFjAUBgNVBAcMDVNhbiBGcmFuY2lzY28xDzANBgNVBAoMBkRvY2tlcjEPMA0G
A1UECwwGRG9ja2VyMQ0wCwYDVQQDDARUZXN0MFkwEwYHKoZIzj0CAQYIKoZIzj0D
AQcDQgAEgvvZl5Vqpr1e+g5IhoU6TZHgRau+BZETVFTmqyWYajA/mooRQ1MZTozu
s9ZZZA8tzUhIqS36gsFuyIZ4YiAlyjAKBggqhkjOPQQDAwNIADBFAiBQ7pCPQrj8
8zaItMf0pk8j1NU5XrFqFEZICzvjzUJQBAIhAKq2gFwoTn8KH+cAAXZpAGJPmOsT
zsBT8gBAOHhNA6/2
-----END CERTIFICATE-----`
key = `
-----BEGIN EC PRIVATE KEY-----
MHcCAQEEICyheZpw70pbgO4hEuwhZTETWyTpNJmJ3TyFaWT6WTRkoAoGCCqGSM49
AwEHoUQDQgAEgvvZl5Vqpr1e+g5IhoU6TZHgRau+BZETVFTmqyWYajA/mooRQ1MZ
Tozus9ZZZA8tzUhIqS36gsFuyIZ4YiAlyg==
-----END EC PRIVATE KEY-----`
)
func swarmSpecWithFullCAConfig() *swarm.Spec {
return &swarm.Spec{
CAConfig: swarm.CAConfig{
@ -37,51 +59,79 @@ func TestDisplayTrustRootNoRoot(t *testing.T) {
assert.Error(t, err, "No CA information available")
}
type invalidCATestCases struct {
args []string
errorMsg string
}
func writeFile(data string) (string, error) {
tmpfile, err := ioutil.TempFile("", "testfile")
if err != nil {
return "", err
}
_, err = tmpfile.Write([]byte(data))
if err != nil {
return "", err
}
tmpfile.Close()
return tmpfile.Name(), nil
}
func TestDisplayTrustRootInvalidFlags(t *testing.T) {
// we need an actual PEMfile to test
tmpfile, err := ioutil.TempFile("", "pemfile")
tmpfile, err := writeFile(cert)
assert.NilError(t, err)
defer os.Remove(tmpfile.Name())
tmpfile.Write([]byte(`
-----BEGIN CERTIFICATE-----
MIIBajCCARCgAwIBAgIUe0+jYWhxN8fFOByC7yveIYgvx1kwCgYIKoZIzj0EAwIw
EzERMA8GA1UEAxMIc3dhcm0tY2EwHhcNMTcwNjI3MTUxNDAwWhcNMzcwNjIyMTUx
NDAwWjATMREwDwYDVQQDEwhzd2FybS1jYTBZMBMGByqGSM49AgEGCCqGSM49AwEH
A0IABGgbOZLd7b4b262+6m4ignIecbAZKim6djNiIS1Kl5IHciXYn7gnSpsayjn7
GQABpgkdPeM9TEQowmtR1qSnORujQjBAMA4GA1UdDwEB/wQEAwIBBjAPBgNVHRMB
Af8EBTADAQH/MB0GA1UdDgQWBBQ6Rtcn823/fxRZyheRDFpDzuBMpTAKBggqhkjO
PQQDAgNIADBFAiEAqD3Kb2rgsy6NoTk+zEgcUi/aGBCsvQDG3vML1PXN8j0CIBjj
4nDj+GmHXcnKa8wXx70Z8OZEpRQIiKDDLmcXuslp
-----END CERTIFICATE-----
`))
tmpfile.Close()
defer os.Remove(tmpfile)
errorTestCases := [][]string{
errorTestCases := []invalidCATestCases{
{
"--ca-cert=" + tmpfile.Name(),
args: []string{"--ca-cert=" + tmpfile},
errorMsg: "flag requires the `--rotate` flag to update the CA",
},
{
"--ca-key=" + tmpfile.Name(),
args: []string{"--ca-key=" + tmpfile},
errorMsg: "flag requires the `--rotate` flag to update the CA",
},
{ // to make sure we're not erroring because we didn't provide a CA key along with the CA cert
"--ca-cert=" + tmpfile.Name(),
"--ca-key=" + tmpfile.Name(),
args: []string{
"--ca-cert=" + tmpfile,
"--ca-key=" + tmpfile,
},
errorMsg: "flag requires the `--rotate` flag to update the CA",
},
{
"--cert-expiry=2160h0m0s",
args: []string{"--cert-expiry=2160h0m0s"},
errorMsg: "flag requires the `--rotate` flag to update the CA",
},
{
"--external-ca=protocol=cfssl,url=https://some.com/https/url",
args: []string{"--external-ca=protocol=cfssl,url=https://some.com/https/url"},
errorMsg: "flag requires the `--rotate` flag to update the CA",
},
{ // to make sure we're not erroring because we didn't provide a CA cert and external CA
"--ca-cert=" + tmpfile.Name(),
"--external-ca=protocol=cfssl,url=https://some.com/https/url",
args: []string{
"--ca-cert=" + tmpfile,
"--external-ca=protocol=cfssl,url=https://some.com/https/url",
},
errorMsg: "flag requires the `--rotate` flag to update the CA",
},
{
args: []string{
"--rotate",
"--external-ca=protocol=cfssl,url=https://some.com/https/url",
},
errorMsg: "rotating to an external CA requires the `--ca-cert` flag to specify the external CA's cert - " +
"to add an external CA with the current root CA certificate, use the `update` command instead",
},
{
args: []string{
"--rotate",
"--ca-cert=" + tmpfile,
},
errorMsg: "the --ca-cert flag requires that a --ca-key flag and/or --external-ca flag be provided as well",
},
}
for _, args := range errorTestCases {
for _, testCase := range errorTestCases {
cmd := newCACommand(
test.NewFakeCli(&fakeClient{
swarmInspectFunc: func() (swarm.Swarm, error) {
@ -94,9 +144,9 @@ PQQDAgNIADBFAiEAqD3Kb2rgsy6NoTk+zEgcUi/aGBCsvQDG3vML1PXN8j0CIBjj
}, nil
},
}))
assert.Check(t, cmd.Flags().Parse(args))
assert.Check(t, cmd.Flags().Parse(testCase.args))
cmd.SetOutput(ioutil.Discard)
assert.ErrorContains(t, cmd.Execute(), "flag requires the `--rotate` flag to update the CA")
assert.ErrorContains(t, cmd.Execute(), testCase.errorMsg)
}
}
@ -112,43 +162,139 @@ func TestDisplayTrustRoot(t *testing.T) {
assert.Check(t, is.Equal(trustRoot+"\n", buffer.String()))
}
type swarmUpdateRecorder struct {
spec swarm.Spec
}
func (s *swarmUpdateRecorder) swarmUpdate(sp swarm.Spec, _ swarm.UpdateFlags) error {
s.spec = sp
return nil
}
func swarmInspectFuncWithFullCAConfig() (swarm.Swarm, error) {
return swarm.Swarm{
ClusterInfo: swarm.ClusterInfo{
Spec: *swarmSpecWithFullCAConfig(),
},
}, nil
}
func TestUpdateSwarmSpecDefaultRotate(t *testing.T) {
spec := swarmSpecWithFullCAConfig()
flags := newCACommand(nil).Flags()
updateSwarmSpec(spec, flags, caOptions{})
s := &swarmUpdateRecorder{}
cli := test.NewFakeCli(&fakeClient{
swarmInspectFunc: swarmInspectFuncWithFullCAConfig,
swarmUpdateFunc: s.swarmUpdate,
})
cmd := newCACommand(cli)
cmd.SetArgs([]string{"--rotate", "--detach"})
cmd.SetOutput(cli.OutBuffer())
assert.NilError(t, cmd.Execute())
expected := swarmSpecWithFullCAConfig()
expected.CAConfig.ForceRotate = 2
expected.CAConfig.SigningCACert = ""
expected.CAConfig.SigningCAKey = ""
assert.Check(t, is.DeepEqual(expected, spec))
assert.Check(t, is.DeepEqual(*expected, s.spec))
}
func TestUpdateSwarmSpecPartial(t *testing.T) {
spec := swarmSpecWithFullCAConfig()
flags := newCACommand(nil).Flags()
updateSwarmSpec(spec, flags, caOptions{
rootCACert: PEMFile{contents: "cacert"},
func TestUpdateSwarmSpecCertAndKey(t *testing.T) {
certfile, err := writeFile(cert)
assert.NilError(t, err)
defer os.Remove(certfile)
keyfile, err := writeFile(key)
assert.NilError(t, err)
defer os.Remove(keyfile)
s := &swarmUpdateRecorder{}
cli := test.NewFakeCli(&fakeClient{
swarmInspectFunc: swarmInspectFuncWithFullCAConfig,
swarmUpdateFunc: s.swarmUpdate,
})
cmd := newCACommand(cli)
cmd.SetArgs([]string{
"--rotate",
"--detach",
"--ca-cert=" + certfile,
"--ca-key=" + keyfile,
"--cert-expiry=3m"})
cmd.SetOutput(cli.OutBuffer())
assert.NilError(t, cmd.Execute())
expected := swarmSpecWithFullCAConfig()
expected.CAConfig.SigningCACert = "cacert"
assert.Check(t, is.DeepEqual(expected, spec))
}
func TestUpdateSwarmSpecFullFlags(t *testing.T) {
flags := newCACommand(nil).Flags()
flags.Lookup(flagCertExpiry).Changed = true
spec := swarmSpecWithFullCAConfig()
updateSwarmSpec(spec, flags, caOptions{
rootCACert: PEMFile{contents: "cacert"},
rootCAKey: PEMFile{contents: "cakey"},
swarmCAOptions: swarmCAOptions{nodeCertExpiry: 3 * time.Minute},
})
expected := swarmSpecWithFullCAConfig()
expected.CAConfig.SigningCACert = "cacert"
expected.CAConfig.SigningCAKey = "cakey"
expected.CAConfig.SigningCACert = cert
expected.CAConfig.SigningCAKey = key
expected.CAConfig.NodeCertExpiry = 3 * time.Minute
assert.Check(t, is.DeepEqual(expected, spec))
assert.Check(t, is.DeepEqual(*expected, s.spec))
}
func TestUpdateSwarmSpecCertAndExternalCA(t *testing.T) {
certfile, err := writeFile(cert)
assert.NilError(t, err)
defer os.Remove(certfile)
s := &swarmUpdateRecorder{}
cli := test.NewFakeCli(&fakeClient{
swarmInspectFunc: swarmInspectFuncWithFullCAConfig,
swarmUpdateFunc: s.swarmUpdate,
})
cmd := newCACommand(cli)
cmd.SetArgs([]string{
"--rotate",
"--detach",
"--ca-cert=" + certfile,
"--external-ca=protocol=cfssl,url=https://some.external.ca"})
cmd.SetOutput(cli.OutBuffer())
assert.NilError(t, cmd.Execute())
expected := swarmSpecWithFullCAConfig()
expected.CAConfig.SigningCACert = cert
expected.CAConfig.SigningCAKey = ""
expected.CAConfig.ExternalCAs = []*swarm.ExternalCA{
{
Protocol: swarm.ExternalCAProtocolCFSSL,
URL: "https://some.external.ca",
CACert: cert,
Options: make(map[string]string),
},
}
assert.Check(t, is.DeepEqual(*expected, s.spec))
}
func TestUpdateSwarmSpecCertAndKeyAndExternalCA(t *testing.T) {
certfile, err := writeFile(cert)
assert.NilError(t, err)
defer os.Remove(certfile)
keyfile, err := writeFile(key)
assert.NilError(t, err)
defer os.Remove(keyfile)
s := &swarmUpdateRecorder{}
cli := test.NewFakeCli(&fakeClient{
swarmInspectFunc: swarmInspectFuncWithFullCAConfig,
swarmUpdateFunc: s.swarmUpdate,
})
cmd := newCACommand(cli)
cmd.SetArgs([]string{
"--rotate",
"--detach",
"--ca-cert=" + certfile,
"--ca-key=" + keyfile,
"--external-ca=protocol=cfssl,url=https://some.external.ca"})
cmd.SetOutput(cli.OutBuffer())
assert.NilError(t, cmd.Execute())
expected := swarmSpecWithFullCAConfig()
expected.CAConfig.SigningCACert = cert
expected.CAConfig.SigningCAKey = key
expected.CAConfig.ExternalCAs = []*swarm.ExternalCA{
{
Protocol: swarm.ExternalCAProtocolCFSSL,
URL: "https://some.external.ca",
CACert: cert,
Options: make(map[string]string),
},
}
assert.Check(t, is.DeepEqual(*expected, s.spec))
}

11
cli/command/swarm/opts.go
View File

@ -230,7 +230,7 @@ func addSwarmFlags(flags *pflag.FlagSet, opts *swarmOptions) {
addSwarmCAFlags(flags, &opts.swarmCAOptions)
}
func (opts *swarmOptions) mergeSwarmSpec(spec *swarm.Spec, flags *pflag.FlagSet) {
func (opts *swarmOptions) mergeSwarmSpec(spec *swarm.Spec, flags *pflag.FlagSet, caCert string) {
if flags.Changed(flagTaskHistoryLimit) {
spec.Orchestration.TaskHistoryRetentionLimit = &opts.taskHistoryLimit
}
@ -246,7 +246,7 @@ func (opts *swarmOptions) mergeSwarmSpec(spec *swarm.Spec, flags *pflag.FlagSet)
if flags.Changed(flagAutolock) {
spec.EncryptionConfig.AutoLockManagers = opts.autolock
}
opts.mergeSwarmSpecCAFlags(spec, flags)
opts.mergeSwarmSpecCAFlags(spec, flags, caCert)
}
type swarmCAOptions struct {
@ -254,17 +254,20 @@ type swarmCAOptions struct {
externalCA ExternalCAOption
}
func (opts *swarmCAOptions) mergeSwarmSpecCAFlags(spec *swarm.Spec, flags *pflag.FlagSet) {
func (opts *swarmCAOptions) mergeSwarmSpecCAFlags(spec *swarm.Spec, flags *pflag.FlagSet, caCert string) {
if flags.Changed(flagCertExpiry) {
spec.CAConfig.NodeCertExpiry = opts.nodeCertExpiry
}
if flags.Changed(flagExternalCA) {
spec.CAConfig.ExternalCAs = opts.externalCA.Value()
for _, ca := range spec.CAConfig.ExternalCAs {
ca.CACert = caCert
}
}
}
func (opts *swarmOptions) ToSpec(flags *pflag.FlagSet) swarm.Spec {
var spec swarm.Spec
opts.mergeSwarmSpec(&spec, flags)
opts.mergeSwarmSpec(&spec, flags, "")
return spec
}

2
cli/command/swarm/update.go
View File

@ -48,7 +48,7 @@ func runUpdate(dockerCli command.Cli, flags *pflag.FlagSet, opts swarmOptions) e
prevAutoLock := swarmInspect.Spec.EncryptionConfig.AutoLockManagers
opts.mergeSwarmSpec(&swarmInspect.Spec, flags)
opts.mergeSwarmSpec(&swarmInspect.Spec, flags, swarmInspect.ClusterInfo.TLSInfo.TrustRoot)
curAutoLock := swarmInspect.Spec.EncryptionConfig.AutoLockManagers

9
cli/command/swarm/update_test.go
View File

@ -10,6 +10,7 @@ import (
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/swarm"
"github.com/pkg/errors"
// Import builders to get the builder function as package function
. "github.com/docker/cli/internal/test/builders"
"gotest.tools/assert"
@ -82,6 +83,9 @@ func TestSwarmUpdateErrors(t *testing.T) {
}
func TestSwarmUpdate(t *testing.T) {
swarmInfo := Swarm()
swarmInfo.ClusterInfo.TLSInfo.TrustRoot = "trustroot"
testCases := []struct {
name string
args []string
@ -105,6 +109,9 @@ func TestSwarmUpdate(t *testing.T) {
flagAutolock: "true",
flagQuiet: "true",
},
swarmInspectFunc: func() (swarm.Swarm, error) {
return *swarmInfo, nil
},
swarmUpdateFunc: func(swarm swarm.Spec, flags swarm.UpdateFlags) error {
if *swarm.Orchestration.TaskHistoryRetentionLimit != 10 {
return errors.Errorf("historyLimit not correctly set")
@ -123,7 +130,7 @@ func TestSwarmUpdate(t *testing.T) {
if swarm.CAConfig.NodeCertExpiry != certExpiryDuration {
return errors.Errorf("certExpiry not correctly set")
}
if len(swarm.CAConfig.ExternalCAs) != 1 {
if len(swarm.CAConfig.ExternalCAs) != 1 || swarm.CAConfig.ExternalCAs[0].CACert != "trustroot" {
return errors.Errorf("externalCA not correctly set")
}
if *swarm.Raft.KeepOldSnapshots != 10 {