From de11799baf05ce91af9c4569018e25bd013b7dbb Mon Sep 17 00:00:00 2001 From: cyli Date: Fri, 9 Mar 2018 17:42:49 -0800 Subject: [PATCH] Stop encrypting the raft root CA key entirely based on env vars, only allow decrypting, since that feature was deprecated almost a year ago. Signed-off-by: cyli --- ca/certificates.go | 51 ++++------------------ ca/certificates_test.go | 87 +++++--------------------------------- manager/manager.go | 93 +++++++++++++---------------------------- manager/manager_test.go | 80 +++++++++++++++++++---------------- 4 files changed, 94 insertions(+), 217 deletions(-) diff --git a/ca/certificates.go b/ca/certificates.go index a5b31d9cce..f3607b1e97 100644 --- a/ca/certificates.go +++ b/ca/certificates.go @@ -642,21 +642,23 @@ func newLocalSigner(keyBytes, certBytes []byte, certExpiry time.Duration, rootPo return nil, errors.Wrap(err, "error while validating signing CA certificate against roots and intermediates") } + // We previously supported (1) encrypting the root CA key in raft using a passphrase provided by an environment variable, + // and (2) hitless passphrase rotation if the user wanted to provide us a previous passphrase as well as a current + // passphrase (we'd rotate the encryption). We now no longer support encrypting the key in raft, since we will + // rely on TLS in transit and raft log encryption at rest, but we do still support reading the passphrase from both + // of the environment variables in order to decrypt the root CA key. + // If the root CA key is not encrypted, then we're good whether or not the KEKs are provided. If the root CA key + // is encrypted in raft, and no KEKs are provided, we will fail to produce a valid signer. var ( - passphraseStr string passphrase, passphrasePrev []byte priv crypto.Signer ) - - // Attempt two distinct passphrases, so we can do a hitless passphrase rotation - if passphraseStr = os.Getenv(PassphraseENVVar); passphraseStr != "" { - passphrase = []byte(passphraseStr) + if p := os.Getenv(PassphraseENVVar); p != "" { + passphrase = []byte(p) } - if p := os.Getenv(PassphraseENVVarPrev); p != "" { passphrasePrev = []byte(p) } - // Attempt to decrypt the current private-key with the passphrases provided priv, err = keyutils.ParsePrivateKeyPEMWithPassword(keyBytes, passphrase) if err != nil { @@ -676,17 +678,6 @@ func newLocalSigner(keyBytes, certBytes []byte, certExpiry time.Duration, rootPo return nil, err } - // If the key was loaded from disk unencrypted, but there is a passphrase set, - // ensure it is encrypted, so it doesn't hit raft in plain-text - // we don't have to check for nil, because if we couldn't pem-decode the bytes, then parsing above would have failed - keyBlock, _ := pem.Decode(keyBytes) - if passphraseStr != "" && !keyutils.IsEncryptedPEMBlock(keyBlock) { - keyBytes, err = EncryptECPrivateKey(keyBytes, passphraseStr) - if err != nil { - return nil, errors.Wrap(err, "unable to encrypt signing CA key material") - } - } - return &LocalSigner{Cert: certBytes, Key: keyBytes, Signer: signer, parsedCert: parsedCerts[0], cryptoSigner: priv}, nil } @@ -977,30 +968,6 @@ func GenerateNewCSR() ([]byte, []byte, error) { return csr, key, err } -// EncryptECPrivateKey receives a PEM encoded private key and returns an encrypted -// AES256 version using a passphrase -// TODO: Make this method generic to handle RSA keys -func EncryptECPrivateKey(key []byte, passphraseStr string) ([]byte, error) { - passphrase := []byte(passphraseStr) - - keyBlock, _ := pem.Decode(key) - if keyBlock == nil { - // This RootCA does not have a valid signer. - return nil, errors.New("error while decoding PEM key") - } - - encryptedPEMBlock, err := keyutils.EncryptPEMBlock(keyBlock.Bytes, passphrase) - if err != nil { - return nil, err - } - - if encryptedPEMBlock.Headers == nil { - return nil, errors.New("unable to encrypt key - invalid PEM file produced") - } - - return pem.EncodeToMemory(encryptedPEMBlock), nil -} - // NormalizePEMs takes a bundle of PEM-encoded certificates in a certificate bundle, // decodes them, removes headers, and re-encodes them to make sure that they have // consistent whitespace. Note that this is intended to normalize x509 certificates diff --git a/ca/certificates_test.go b/ca/certificates_test.go index 20269829c9..fde5c040f8 100644 --- a/ca/certificates_test.go +++ b/ca/certificates_test.go @@ -28,7 +28,6 @@ import ( "github.com/docker/swarmkit/api" "github.com/docker/swarmkit/ca" "github.com/docker/swarmkit/ca/keyutils" - "github.com/docker/swarmkit/ca/pkcs8" cautils "github.com/docker/swarmkit/ca/testutils" "github.com/docker/swarmkit/connectionbroker" "github.com/docker/swarmkit/fips" @@ -45,8 +44,8 @@ import ( ) func init() { - os.Setenv(ca.PassphraseENVVar, "") - os.Setenv(ca.PassphraseENVVarPrev, "") + os.Unsetenv(ca.PassphraseENVVar) + os.Unsetenv(ca.PassphraseENVVarPrev) ca.RenewTLSExponentialBackoff = events.ExponentialBackoffConfig{ Base: 250 * time.Millisecond, @@ -232,21 +231,6 @@ some random garbage\n require.Error(t, err) } -func TestEncryptECPrivateKey(t *testing.T) { - tempBaseDir, err := ioutil.TempDir("", "swarm-ca-test-") - assert.NoError(t, err) - defer os.RemoveAll(tempBaseDir) - - _, key, err := ca.GenerateNewCSR() - assert.NoError(t, err) - encryptedKey, err := ca.EncryptECPrivateKey(key, "passphrase") - assert.NoError(t, err) - - keyBlock, _ := pem.Decode(encryptedKey) - assert.NotNil(t, keyBlock) - assert.True(t, pkcs8.IsEncryptedPEMBlock(keyBlock)) -} - func TestParseValidateAndSignCSR(t *testing.T) { rootCA, err := ca.CreateRootCA("rootCN") assert.NoError(t, err) @@ -955,6 +939,14 @@ func TestGetRemoteSignedCertificateConnectionErrors(t *testing.T) { } func TestNewRootCA(t *testing.T) { + // we expect nothing to happen with these environment variables - previously we supported encrypting + // the root CA key, and these passphrases would cause us to encrypt the CA key in raft using the + // provided encryption keys stored in these environment variables. + os.Setenv(ca.PassphraseENVVar, "password1") + defer os.Unsetenv(ca.PassphraseENVVar) + os.Setenv(ca.PassphraseENVVarPrev, "password2") + defer os.Unsetenv(ca.PassphraseENVVarPrev) + for _, pair := range []struct{ cert, key []byte }{ {cert: cautils.ECDSA256SHA256Cert, key: cautils.ECDSA256Key}, {cert: cautils.RSA2048SHA256Cert, key: cautils.RSA2048Key}, @@ -1332,65 +1324,6 @@ func TestRootCAWithCrossSignedIntermediates(t *testing.T) { checkValidateAgainstAllRoots(tlsCert) } -func TestNewRootCAWithPassphrase(t *testing.T) { - defer os.Setenv(ca.PassphraseENVVar, "") - defer os.Setenv(ca.PassphraseENVVarPrev, "") - - rootCA, err := ca.CreateRootCA("rootCN") - assert.NoError(t, err) - rcaSigner, err := rootCA.Signer() - assert.NoError(t, err) - - // Ensure that we're encrypting the Key bytes out of NewRoot if there - // is a passphrase set as an env Var - os.Setenv(ca.PassphraseENVVar, "password1") - newRootCA, err := ca.NewRootCA(rootCA.Certs, rcaSigner.Cert, rcaSigner.Key, ca.DefaultNodeCertExpiration, nil) - assert.NoError(t, err) - nrcaSigner, err := newRootCA.Signer() - assert.NoError(t, err) - assert.NotEqual(t, rcaSigner.Key, nrcaSigner.Key) - assert.Equal(t, rootCA.Certs, newRootCA.Certs) - assert.NotContains(t, string(rcaSigner.Key), string(nrcaSigner.Key)) - keyBlock, _ := pem.Decode(nrcaSigner.Key) - assert.NotNil(t, keyBlock) - assert.True(t, keyutils.IsEncryptedPEMBlock(keyBlock)) - - // Ensure that we're decrypting the Key bytes out of NewRoot if there - // is a passphrase set as an env Var - anotherNewRootCA, err := ca.NewRootCA(newRootCA.Certs, nrcaSigner.Cert, nrcaSigner.Key, ca.DefaultNodeCertExpiration, nil) - assert.NoError(t, err) - anrcaSigner, err := anotherNewRootCA.Signer() - assert.NoError(t, err) - assert.Equal(t, newRootCA, anotherNewRootCA) - assert.NotContains(t, string(rcaSigner.Key), string(anrcaSigner.Key)) - keyBlock, _ = pem.Decode(anrcaSigner.Key) - assert.NotNil(t, keyBlock) - assert.True(t, keyutils.IsEncryptedPEMBlock(keyBlock)) - - // Ensure that we cant decrypt the Key bytes out of NewRoot if there - // is a wrong passphrase set as an env Var - os.Setenv(ca.PassphraseENVVar, "password2") - anotherNewRootCA, err = ca.NewRootCA(newRootCA.Certs, nrcaSigner.Cert, nrcaSigner.Key, ca.DefaultNodeCertExpiration, nil) - assert.Error(t, err) - - // Ensure that we cant decrypt the Key bytes out of NewRoot if there - // is a wrong passphrase set as an env Var - os.Setenv(ca.PassphraseENVVarPrev, "password2") - anotherNewRootCA, err = ca.NewRootCA(newRootCA.Certs, nrcaSigner.Cert, nrcaSigner.Key, ca.DefaultNodeCertExpiration, nil) - assert.Error(t, err) - - // Ensure that we can decrypt the Key bytes out of NewRoot if there - // is a wrong passphrase set as an env Var, but a valid as Prev - os.Setenv(ca.PassphraseENVVarPrev, "password1") - anotherNewRootCA, err = ca.NewRootCA(newRootCA.Certs, nrcaSigner.Cert, nrcaSigner.Key, ca.DefaultNodeCertExpiration, nil) - assert.NoError(t, err) - assert.Equal(t, newRootCA, anotherNewRootCA) - assert.NotContains(t, string(rcaSigner.Key), string(anrcaSigner.Key)) - keyBlock, _ = pem.Decode(anrcaSigner.Key) - assert.NotNil(t, keyBlock) - assert.True(t, keyutils.IsEncryptedPEMBlock(keyBlock)) -} - type certTestCase struct { cert []byte errorStr string diff --git a/manager/manager.go b/manager/manager.go index 0e897bcd14..08ce1ae89c 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -806,36 +806,34 @@ func (m *Manager) watchForClusterChanges(ctx context.Context) error { return nil } -// rotateRootCAKEK will attempt to rotate the key-encryption-key for root CA key-material in raft. -// If there is no passphrase set in ENV, it returns. -// If there is plain-text root key-material, and a passphrase set, it encrypts it. -// If there is encrypted root key-material and it is using the current passphrase, it returns. -// If there is encrypted root key-material, and it is using the previous passphrase, it -// re-encrypts it with the current passphrase. -func (m *Manager) rotateRootCAKEK(ctx context.Context, clusterID string) error { - // If we don't have a KEK, we won't ever be rotating anything +// decryptRootCAKEK will attempt to decrypt the key-encryption-key for root CA key-material in raft. +// We previously supported (1) encrypting the root CA key in raft using a passphrase provided by an environment variable, +// and (2) hitless passphrase rotation if the user wanted to provide us a previous passphrase as well as a current +// passphrase (we'd rotate the encryption). We now no longer support encrypting the key in raft, since we will +// rely on TLS in transit and raft log encryption at rest, but this will read the passphrase from both +// of the environment variables in order to decrypt, and store decrypted, the root CA key. +// If the root CA key is not encrypted in raft, then we're good and nothing needs to be done, whether or not +// the KEKs are provided. If the root CA key is encrypted in raft, and no KEKs are provided, then this function +// will succeed, but if this node becomes the leader it will not be able to serve as the CA. +func (m *Manager) decryptRootCAKEK(ctx context.Context, clusterID string) error { + // If we don't have a KEK, we won't need to, or can't, decrypt anything strPassphrase := os.Getenv(ca.PassphraseENVVar) strPassphrasePrev := os.Getenv(ca.PassphraseENVVarPrev) if strPassphrase == "" && strPassphrasePrev == "" { return nil } if strPassphrase != "" { - log.G(ctx).Warn("Encrypting the root CA key in swarm using environment variables is deprecated. " + - "Support for decrypting or rotating the key will be removed in the future.") + log.G(ctx).Warn("Encrypting the root CA key in swarm using environment variables is no longer supported. " + + "It will be decrypted in swarm, and rely on raft at rest encryption and TLS in transit to protect it.") } passphrase := []byte(strPassphrase) passphrasePrev := []byte(strPassphrasePrev) s := m.raftNode.MemoryStore() - var ( - cluster *api.Cluster - err error - finalKey []byte - ) // Retrieve the cluster identified by ClusterID return s.Update(func(tx store.Tx) error { - cluster = store.GetCluster(tx, clusterID) + cluster := store.GetCluster(tx, clusterID) if cluster == nil { return fmt.Errorf("cluster not found: %s", clusterID) } @@ -854,57 +852,26 @@ func (m *Manager) rotateRootCAKEK(ctx context.Context, clusterID string) error { return fmt.Errorf("invalid PEM-encoded private key inside of cluster %s", clusterID) } - if keyutils.IsEncryptedPEMBlock(keyBlock) { - // PEM encryption does not have a digest, so sometimes decryption doesn't - // error even with the wrong passphrase. So actually try to parse it into a valid key. - _, err := keyutils.ParsePrivateKeyPEMWithPassword(privKeyPEM, []byte(passphrase)) - if err == nil { - // This key is already correctly encrypted with the correct KEK, nothing to do here - return nil - } + if !keyutils.IsEncryptedPEMBlock(keyBlock) { + return nil + } - // This key is already encrypted, but failed with current main passphrase. - // Let's try to decrypt with the previous passphrase, and parse into a valid key, for the - // same reason as above. - _, err = keyutils.ParsePrivateKeyPEMWithPassword(privKeyPEM, []byte(passphrasePrev)) + unencryptedDER, err := keyutils.DecryptPEMBlock(keyBlock, passphrase) + if err != nil { + unencryptedDER, err = keyutils.DecryptPEMBlock(keyBlock, passphrasePrev) if err != nil { - // We were not able to decrypt either with the main or backup passphrase, error return err } - // ok the above passphrase is correct, so decrypt the PEM block so we can re-encrypt - - // since the key was successfully decrypted above, there will be no error doing PEM - // decryption - unencryptedDER, _ := keyutils.DecryptPEMBlock(keyBlock, []byte(passphrasePrev)) - unencryptedKeyBlock := &pem.Block{ - Type: keyBlock.Type, - Bytes: unencryptedDER, - } - - // we were able to decrypt the key with the previous passphrase - if the current passphrase is empty, - // the we store the decrypted key in raft - finalKey = pem.EncodeToMemory(unencryptedKeyBlock) + } - // the current passphrase is not empty, so let's encrypt with the new one and store it in raft - if strPassphrase != "" { - finalKey, err = ca.EncryptECPrivateKey(finalKey, strPassphrase) - if err != nil { - log.G(ctx).WithError(err).Debugf("failed to rotate the key-encrypting-key for the root key material of cluster %s", clusterID) - return err - } - } - } else if strPassphrase != "" { - // If this key is not encrypted, and the passphrase is not nil, then we have to encrypt it - finalKey, err = ca.EncryptECPrivateKey(privKeyPEM, strPassphrase) - if err != nil { - log.G(ctx).WithError(err).Debugf("failed to rotate the key-encrypting-key for the root key material of cluster %s", clusterID) - return err - } - } else { - return nil // don't update if it's not encrypted and we don't want it encrypted + unencryptedKeyBlock := &pem.Block{ + Type: keyBlock.Type, + Bytes: unencryptedDER, } - log.G(ctx).Infof("Updating the encryption on the root key material of cluster %s", clusterID) - cluster.RootCA.CAKey = finalKey + cluster.RootCA.CAKey = pem.EncodeToMemory(unencryptedKeyBlock) + + log.G(ctx).Infof("Decrypting the root key material of cluster %s", clusterID) return store.UpdateCluster(tx, cluster) }) } @@ -1066,10 +1033,10 @@ func (m *Manager) becomeLeader(ctx context.Context) { return nil }) - // Attempt to rotate the key-encrypting-key of the root CA key-material - err := m.rotateRootCAKEK(ctx, clusterID) + // Attempt to decrypt the root CA key-material + err := m.decryptRootCAKEK(ctx, clusterID) if err != nil { - log.G(ctx).WithError(err).Error("root key-encrypting-key rotation failed") + log.G(ctx).WithError(err).Error("root key decryption failed") } m.replicatedOrchestrator = replicated.NewReplicatedOrchestrator(s) diff --git a/manager/manager_test.go b/manager/manager_test.go index 8fc4c30d37..69ffccaaeb 100644 --- a/manager/manager_test.go +++ b/manager/manager_test.go @@ -424,8 +424,12 @@ func TestManagerLockUnlock(t *testing.T) { <-done } -// Tests manager rotates encryption of root key data in the raft store -func TestManagerEncryptsDecryptsRootKeyMaterial(t *testing.T) { +// TestManagerDecryptsRootKeyMaterial ensures that on startup, if the root CA key was encrypted in raft, +// the manager would decrypt the key using either the current passphrase environment variable, or the +// previous passphrase environment variable, and write the decrypted CA key back to raft. If the key was +// encrypted using the previous passphrase environment variable, it will *not* be re-encrypted +// using the current passphrase environment variable (we no longer to encryption key rotation) +func TestManagerDecryptsRootKeyMaterial(t *testing.T) { tc := cautils.NewTestCA(t) defer tc.Stop() @@ -467,10 +471,13 @@ func TestManagerEncryptsDecryptsRootKeyMaterial(t *testing.T) { }() } + os.Setenv(ca.PassphraseENVVar, "kek") + defer os.Unsetenv(ca.PassphraseENVVar) + startManager() - var clusterID string - // wait for cluster data to be there + var cluster *api.Cluster + // wait for cluster data to be there, and make sure that the key is not encrypted err = testutils.PollFunc(nil, func() error { // using store.Update just because it returns an error, as opposed to store.View return m.raftNode.MemoryStore().Update(func(tx store.Tx) error { @@ -481,49 +488,32 @@ func TestManagerEncryptsDecryptsRootKeyMaterial(t *testing.T) { if len(clusters) != 1 { return fmt.Errorf("expected 1 cluster, got %d", len(clusters)) } - clusterID = clusters[0].ID + cluster = clusters[0] return nil }) }) - os.Setenv(ca.PassphraseENVVar, "kek") - defer os.Unsetenv(ca.PassphraseENVVar) + keyBlock, _ := pem.Decode(cluster.RootCA.CAKey) + require.NotNil(t, keyBlock) + require.False(t, keyutils.IsEncryptedPEMBlock(keyBlock)) - // restart - m.Stop(tc.Context, false) - <-done - startManager() + unencryptedDERBytes := keyBlock.Bytes - // wait for the key to be encrypted in the raft store - err = testutils.PollFunc(nil, func() error { - return m.raftNode.MemoryStore().Update(func(tx store.Tx) error { - cluster := store.GetCluster(tx, clusterID) - if cluster == nil { - return fmt.Errorf("cluster gone") - } - keyBlock, _ := pem.Decode(cluster.RootCA.CAKey) - if keyBlock == nil { - return fmt.Errorf("could not pem decode root key") - } - if !keyutils.IsEncryptedPEMBlock(keyBlock) { - return fmt.Errorf("root key material not encrypted yet") - } - _, err = keyutils.DecryptPEMBlock(keyBlock, []byte("kek")) - return err - }) - }) + // update the cluster CA key material to be encrypted with the current passphrase + keyBlock, err = keyutils.EncryptPEMBlock(unencryptedDERBytes, []byte("kek")) require.NoError(t, err) - os.Unsetenv(ca.PassphraseENVVar) - os.Setenv(ca.PassphraseENVVarPrev, "kek") - defer os.Unsetenv(ca.PassphraseENVVarPrev) + require.NoError(t, m.raftNode.MemoryStore().Update(func(tx store.Tx) error { + cluster = store.GetCluster(tx, cluster.ID) + cluster.RootCA.CAKey = pem.EncodeToMemory(keyBlock) + return store.UpdateCluster(tx, cluster) + })) // restart m.Stop(tc.Context, false) <-done startManager() - // wait for the key to be decrypted in the raft store pollDecrypted := func() error { return testutils.PollFunc(nil, func() error { // wait until we are leader first, because otherwise the raft node could still be catching @@ -532,7 +522,7 @@ func TestManagerEncryptsDecryptsRootKeyMaterial(t *testing.T) { return fmt.Errorf("node is not leader yet") } return m.raftNode.MemoryStore().Update(func(tx store.Tx) error { - cluster := store.GetCluster(tx, clusterID) + cluster := store.GetCluster(tx, cluster.ID) if cluster == nil { return fmt.Errorf("cluster gone") } @@ -549,11 +539,31 @@ func TestManagerEncryptsDecryptsRootKeyMaterial(t *testing.T) { } require.NoError(t, pollDecrypted()) + os.Setenv(ca.PassphraseENVVarPrev, "kek_old") + defer os.Unsetenv(ca.PassphraseENVVarPrev) + + // update the cluster CA key material to be encrypted with the previous passphrase + keyBlock, err = keyutils.EncryptPEMBlock(unencryptedDERBytes, []byte("kek_old")) + require.NoError(t, err) + + require.NoError(t, m.raftNode.MemoryStore().Update(func(tx store.Tx) error { + cluster = store.GetCluster(tx, cluster.ID) + cluster.RootCA.CAKey = pem.EncodeToMemory(keyBlock) + return store.UpdateCluster(tx, cluster) + })) + + // restart + m.Stop(tc.Context, false) + <-done + startManager() + + require.NoError(t, pollDecrypted()) + // update the key to that can be "decrypted" with both "" and "kek" as the password. This // doesn't actually match the root CA certificate, and hence the security config can't be // updated, but we're just checking that the CA key is decrypted. require.NoError(t, m.raftNode.MemoryStore().Update(func(tx store.Tx) error { - cluster := store.GetCluster(tx, clusterID) + cluster := store.GetCluster(tx, cluster.ID) if cluster == nil { return fmt.Errorf("cluster gone") }