From e3101560964d9bcbaca52691e5aae741d5b682b7 Mon Sep 17 00:00:00 2001 From: cyli Date: Tue, 24 Jan 2017 18:22:14 -0800 Subject: [PATCH] Update CreateRootCA to not automatically save - callers should instead also call SaveRootCA. Signed-off-by: cyli --- ca/certificates.go | 10 ++---- ca/certificates_test.go | 61 ++++++++++----------------------- ca/config.go | 2 +- ca/transport_test.go | 6 ++-- cmd/external-ca-example/main.go | 7 ++-- integration/integration_test.go | 5 +-- node/node.go | 5 ++- node/node_test.go | 10 ++++-- 8 files changed, 42 insertions(+), 64 deletions(-) diff --git a/ca/certificates.go b/ca/certificates.go index 88f05d3da5..21e7338f65 100644 --- a/ca/certificates.go +++ b/ca/certificates.go @@ -745,7 +745,7 @@ func GetRemoteCA(ctx context.Context, d digest.Digest, connBroker *connectionbro // CreateRootCA creates a Certificate authority for a new Swarm Cluster, potentially // overwriting any existing CAs. -func CreateRootCA(rootCN string, paths CertPaths) (RootCA, error) { +func CreateRootCA(rootCN string) (RootCA, error) { // Create a simple CSR for the CA using the default CA validator and policy req := cfcsr.CertificateRequest{ CN: rootCN, @@ -764,11 +764,6 @@ func CreateRootCA(rootCN string, paths CertPaths) (RootCA, error) { return RootCA{}, err } - // save the cert to disk - if err := saveRootCA(rootCA, paths); err != nil { - return RootCA{}, err - } - return rootCA, nil } @@ -872,7 +867,8 @@ func readCertValidity(kr KeyReader) (time.Time, time.Time, error) { } -func saveRootCA(rootCA RootCA, paths CertPaths) error { +// SaveRootCA saves a RootCA object to disk +func SaveRootCA(rootCA RootCA, paths CertPaths) error { // Make sure the necessary dirs exist and they are writable err := os.MkdirAll(filepath.Dir(paths.Cert), 0755) if err != nil { diff --git a/ca/certificates_test.go b/ca/certificates_test.go index d994e0cba7..8ab9383f0c 100644 --- a/ca/certificates_test.go +++ b/ca/certificates_test.go @@ -60,14 +60,17 @@ func TestMain(m *testing.M) { os.Exit(m.Run()) } -func TestCreateRootCA(t *testing.T) { +func TestCreateRootCASaveRootCA(t *testing.T) { tempBaseDir, err := ioutil.TempDir("", "swarm-ca-test-") assert.NoError(t, err) defer os.RemoveAll(tempBaseDir) paths := ca.NewConfigPaths(tempBaseDir) - _, err = ca.CreateRootCA("rootCN", paths.RootCA) + rootCA, err := ca.CreateRootCA("rootCN") + assert.NoError(t, err) + + err = ca.SaveRootCA(rootCA, paths.RootCA) assert.NoError(t, err) perms, err := permbits.Stat(paths.RootCA.Cert) @@ -80,13 +83,7 @@ func TestCreateRootCA(t *testing.T) { } func TestCreateRootCAExpiry(t *testing.T) { - tempBaseDir, err := ioutil.TempDir("", "swarm-ca-test-") - assert.NoError(t, err) - defer os.RemoveAll(tempBaseDir) - - paths := ca.NewConfigPaths(tempBaseDir) - - rootCA, err := ca.CreateRootCA("rootCN", paths.RootCA) + rootCA, err := ca.CreateRootCA("rootCN") assert.NoError(t, err) // Convert the certificate into an object to create a RootCA @@ -95,7 +92,6 @@ func TestCreateRootCAExpiry(t *testing.T) { duration, err := time.ParseDuration(ca.RootCAExpiration) assert.NoError(t, err) assert.True(t, time.Now().Add(duration).AddDate(0, -1, 0).Before(parsedCert.NotAfter)) - } func TestGetLocalRootCA(t *testing.T) { @@ -110,10 +106,12 @@ func TestGetLocalRootCA(t *testing.T) { assert.Equal(t, ca.ErrNoLocalRootCA, err) // Create the local Root CA to ensure that we can reload it correctly. - rootCA, err := ca.CreateRootCA("rootCN", paths.RootCA) + rootCA, err := ca.CreateRootCA("rootCN") assert.NoError(t, err) s, err := rootCA.Signer() assert.NoError(t, err) + err = ca.SaveRootCA(rootCA, paths.RootCA) + assert.NoError(t, err) // No private key here rootCA2, err := ca.GetLocalRootCA(paths.RootCA) @@ -168,8 +166,9 @@ func TestGetLocalRootCAInvalidKey(t *testing.T) { paths := ca.NewConfigPaths(tempBaseDir) // Create the local Root CA to ensure that we can reload it correctly. - _, err = ca.CreateRootCA("rootCN", paths.RootCA) + rootCA, err := ca.CreateRootCA("rootCN") require.NoError(t, err) + require.NoError(t, ca.SaveRootCA(rootCA, paths.RootCA)) // Write some garbage to the root key - this will cause the loading to fail require.NoError(t, ioutil.WriteFile(paths.RootCA.Key, []byte(`-----BEGIN EC PRIVATE KEY-----\n @@ -197,13 +196,7 @@ func TestEncryptECPrivateKey(t *testing.T) { } func TestParseValidateAndSignCSR(t *testing.T) { - tempBaseDir, err := ioutil.TempDir("", "swarm-ca-test-") - assert.NoError(t, err) - defer os.RemoveAll(tempBaseDir) - - paths := ca.NewConfigPaths(tempBaseDir) - - rootCA, err := ca.CreateRootCA("rootCN", paths.RootCA) + rootCA, err := ca.CreateRootCA("rootCN") assert.NoError(t, err) csr, _, err := ca.GenerateNewCSR() @@ -217,13 +210,7 @@ func TestParseValidateAndSignCSR(t *testing.T) { } func TestParseValidateAndSignMaliciousCSR(t *testing.T) { - tempBaseDir, err := ioutil.TempDir("", "swarm-ca-test-") - assert.NoError(t, err) - defer os.RemoveAll(tempBaseDir) - - paths := ca.NewConfigPaths(tempBaseDir) - - rootCA, err := ca.CreateRootCA("rootCN", paths.RootCA) + rootCA, err := ca.CreateRootCA("rootCN") assert.NoError(t, err) req := &cfcsr.CertificateRequest{ @@ -271,8 +258,7 @@ func TestGetRemoteCA(t *testing.T) { require.NoError(t, err) defer os.RemoveAll(tmpDir) paths := ca.NewConfigPaths(tmpDir) - - otherRootCA, err := ca.CreateRootCA("other", paths.RootCA) + otherRootCA, err := ca.CreateRootCA("other") require.NoError(t, err) comboCertBundle := append(tc.RootCA.Certs, otherRootCA.Certs...) @@ -555,11 +541,11 @@ func TestNewRootCABundle(t *testing.T) { paths := ca.NewConfigPaths(tempBaseDir) // make one rootCA - secondRootCA, err := ca.CreateRootCA("rootCN2", paths.RootCA) + firstRootCA, err := ca.CreateRootCA("rootCN1") assert.NoError(t, err) // make a second root CA - firstRootCA, err := ca.CreateRootCA("rootCN1", paths.RootCA) + secondRootCA, err := ca.CreateRootCA("rootCN2") assert.NoError(t, err) s, err := firstRootCA.Signer() require.NoError(t, err) @@ -585,13 +571,7 @@ func TestNewRootCABundle(t *testing.T) { } func TestNewRootCANonDefaultExpiry(t *testing.T) { - tempBaseDir, err := ioutil.TempDir("", "swarm-ca-test-") - assert.NoError(t, err) - defer os.RemoveAll(tempBaseDir) - - paths := ca.NewConfigPaths(tempBaseDir) - - rootCA, err := ca.CreateRootCA("rootCN", paths.RootCA) + rootCA, err := ca.CreateRootCA("rootCN") assert.NoError(t, err) s, err := rootCA.Signer() require.NoError(t, err) @@ -887,15 +867,10 @@ func TestRootCAWithCrossSignedIntermediates(t *testing.T) { } func TestNewRootCAWithPassphrase(t *testing.T) { - tempBaseDir, err := ioutil.TempDir("", "swarm-ca-test-") - assert.NoError(t, err) - defer os.RemoveAll(tempBaseDir) defer os.Setenv(ca.PassphraseENVVar, "") defer os.Setenv(ca.PassphraseENVVarPrev, "") - paths := ca.NewConfigPaths(tempBaseDir) - - rootCA, err := ca.CreateRootCA("rootCN", paths.RootCA) + rootCA, err := ca.CreateRootCA("rootCN") assert.NoError(t, err) rcaSigner, err := rootCA.Signer() assert.NoError(t, err) diff --git a/ca/config.go b/ca/config.go index b42124a5b5..12713dbf05 100644 --- a/ca/config.go +++ b/ca/config.go @@ -276,7 +276,7 @@ func DownloadRootCA(ctx context.Context, paths CertPaths, token string, connBrok } // Save root CA certificate to disk - if err = saveRootCA(rootCA, paths); err != nil { + if err = SaveRootCA(rootCA, paths); err != nil { return RootCA{}, err } diff --git a/ca/transport_test.go b/ca/transport_test.go index 9b17b165b8..257913a99d 100644 --- a/ca/transport_test.go +++ b/ca/transport_test.go @@ -17,7 +17,7 @@ func TestNewMutableTLS(t *testing.T) { paths := NewConfigPaths(tempdir) krw := NewKeyReadWriter(paths.Node, nil, nil) - rootCA, err := CreateRootCA("rootCN", paths.RootCA) + rootCA, err := CreateRootCA("rootCN") require.NoError(t, err) cert, err := rootCA.IssueAndSaveNewCertificates(krw, "CN", ManagerRole, "org") @@ -38,7 +38,7 @@ func TestGetAndValidateCertificateSubject(t *testing.T) { paths := NewConfigPaths(tempdir) krw := NewKeyReadWriter(paths.Node, nil, nil) - rootCA, err := CreateRootCA("rootCN", paths.RootCA) + rootCA, err := CreateRootCA("rootCN") require.NoError(t, err) cert, err := rootCA.IssueAndSaveNewCertificates(krw, "CN", ManagerRole, "org") @@ -58,7 +58,7 @@ func TestLoadNewTLSConfig(t *testing.T) { paths := NewConfigPaths(tempdir) krw := NewKeyReadWriter(paths.Node, nil, nil) - rootCA, err := CreateRootCA("rootCN", paths.RootCA) + rootCA, err := CreateRootCA("rootCN") require.NoError(t, err) // Create two different certs and two different TLS configs diff --git a/cmd/external-ca-example/main.go b/cmd/external-ca-example/main.go index 0d972b737f..5b3bd8b0f3 100644 --- a/cmd/external-ca-example/main.go +++ b/cmd/external-ca-example/main.go @@ -21,9 +21,12 @@ func main() { } // Initialize the Root CA. - rootCA, err := ca.CreateRootCA("external-ca-example", rootPaths) + rootCA, err := ca.CreateRootCA("external-ca-example") if err != nil { - logrus.Fatalf("unable to initialize Root CA: %s", err) + logrus.Fatalf("unable to initialize Root CA: %s", err.Error()) + } + if err := ca.SaveRootCA(rootCA, rootPaths); err != nil { + logrus.Fatalf("unable to save Root CA: %s", err.Error()) } // Create the initial manager node credentials. diff --git a/integration/integration_test.go b/integration/integration_test.go index 7735844674..81d1656bd8 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -489,10 +489,7 @@ func TestForceNewCluster(t *testing.T) { t.Parallel() // create an external CA so that we can use it to generate expired certificates - tempDir, err := ioutil.TempDir("", "external-ca") - require.NoError(t, err) - defer os.RemoveAll(tempDir) - rootCA, err := ca.CreateRootCA("externalRoot", ca.NewConfigPaths(tempDir).RootCA) + rootCA, err := ca.CreateRootCA("externalRoot") require.NoError(t, err) // start a new cluster with the external CA bootstrapped diff --git a/node/node.go b/node/node.go index 9c505b4acf..0551d5b93f 100644 --- a/node/node.go +++ b/node/node.go @@ -600,10 +600,13 @@ func (n *Node) loadSecurityConfig(ctx context.Context) (*ca.SecurityConfig, erro n.unlockKey = encryption.GenerateSecretKey() } krw = ca.NewKeyReadWriter(paths.Node, n.unlockKey, &manager.RaftDEKData{}) - rootCA, err = ca.CreateRootCA(ca.DefaultRootCN, paths.RootCA) + rootCA, err = ca.CreateRootCA(ca.DefaultRootCN) if err != nil { return nil, err } + if err := ca.SaveRootCA(rootCA, paths.RootCA); err != nil { + return nil, err + } log.G(ctx).Debug("generated CA key and certificate") } else if err == ca.ErrNoLocalRootCA { // from previous error loading the root CA from disk rootCA, err = ca.DownloadRootCA(ctx, paths.RootCA, n.config.JoinToken, n.connBroker) diff --git a/node/node_test.go b/node/node_test.go index cf18208218..4fb21f7fc5 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -55,8 +55,9 @@ func TestLoadSecurityConfigPartialCertsOnDisk(t *testing.T) { defer os.RemoveAll(tempdir) paths := ca.NewConfigPaths(filepath.Join(tempdir, "certificates")) - rootCA, err := ca.CreateRootCA(ca.DefaultRootCN, paths.RootCA) + rootCA, err := ca.CreateRootCA(ca.DefaultRootCN) require.NoError(t, err) + require.NoError(t, ca.SaveRootCA(rootCA, paths.RootCA)) node, err := New(&Config{ StateDir: tempdir, @@ -105,8 +106,10 @@ func TestLoadSecurityConfigLoadFromDisk(t *testing.T) { require.NoError(t, err) // Load successfully with valid passphrase - rootCA, err := ca.CreateRootCA(ca.DefaultRootCN, paths.RootCA) + rootCA, err := ca.CreateRootCA(ca.DefaultRootCN) require.NoError(t, err) + require.NoError(t, ca.SaveRootCA(rootCA, paths.RootCA)) + krw := ca.NewKeyReadWriter(paths.Node, []byte("passphrase"), nil) require.NoError(t, err) _, err = rootCA.IssueAndSaveNewCertificates(krw, identity.NewID(), ca.WorkerRole, identity.NewID()) @@ -134,8 +137,9 @@ func TestLoadSecurityConfigLoadFromDisk(t *testing.T) { require.Equal(t, ErrInvalidUnlockKey, err) // Invalid CA - rootCA, err = ca.CreateRootCA(ca.DefaultRootCN, paths.RootCA) + rootCA, err = ca.CreateRootCA(ca.DefaultRootCN) require.NoError(t, err) + require.NoError(t, ca.SaveRootCA(rootCA, paths.RootCA)) node, err = New(&Config{ StateDir: tempdir, JoinAddr: peer.Addr,