From 905d35b089854c735ee8d690e913414a5fc0c30a Mon Sep 17 00:00:00 2001 From: Ying Li Date: Mon, 2 Apr 2018 12:03:02 -0700 Subject: [PATCH 1/2] Propagate the FIPS bool to the raft DEK manager so that the DEK is encrypted using fernet. Signed-off-by: Ying Li --- manager/deks.go | 36 +- manager/deks_test.go | 727 +++++++++++++++++++++++----------------- manager/manager.go | 2 +- manager/manager_test.go | 13 +- 4 files changed, 453 insertions(+), 325 deletions(-) diff --git a/manager/deks.go b/manager/deks.go index edb5227904..f3d985a137 100644 --- a/manager/deks.go +++ b/manager/deks.go @@ -22,6 +22,10 @@ const ( type RaftDEKData struct { raft.EncryptionKeys NeedsRotation bool + + // The FIPS boolean is not serialized, but is internal state which indicates how + // the raft DEK headers should be encrypted (e.g. using FIPS compliant algorithms) + FIPS bool } // UnmarshalHeaders loads the state of the DEK manager given the current TLS headers @@ -32,13 +36,13 @@ func (r RaftDEKData) UnmarshalHeaders(headers map[string]string, kekData ca.KEKD ) if currentDEKStr, ok := headers[pemHeaderRaftDEK]; ok { - currentDEK, err = decodePEMHeaderValue(currentDEKStr, kekData.KEK) + currentDEK, err = decodePEMHeaderValue(currentDEKStr, kekData.KEK, r.FIPS) if err != nil { return nil, err } } if pendingDEKStr, ok := headers[pemHeaderRaftPendingDEK]; ok { - pendingDEK, err = decodePEMHeaderValue(pendingDEKStr, kekData.KEK) + pendingDEK, err = decodePEMHeaderValue(pendingDEKStr, kekData.KEK, r.FIPS) if err != nil { return nil, err } @@ -55,6 +59,7 @@ func (r RaftDEKData) UnmarshalHeaders(headers map[string]string, kekData ca.KEKD CurrentDEK: currentDEK, PendingDEK: pendingDEK, }, + FIPS: r.FIPS, }, nil } @@ -66,7 +71,7 @@ func (r RaftDEKData) MarshalHeaders(kekData ca.KEKData) (map[string]string, erro pemHeaderRaftPendingDEK: r.PendingDEK, } { if contents != nil { - dekStr, err := encodePEMHeaderValue(contents, kekData.KEK) + dekStr, err := encodePEMHeaderValue(contents, kekData.KEK, r.FIPS) if err != nil { return nil, err } @@ -88,6 +93,7 @@ func (r RaftDEKData) UpdateKEK(oldKEK, candidateKEK ca.KEKData) ca.PEMKeyHeaders return RaftDEKData{ EncryptionKeys: r.EncryptionKeys, NeedsRotation: true, + FIPS: r.FIPS, } } return r @@ -112,6 +118,7 @@ func compareKEKs(oldKEK, candidateKEK ca.KEKData) (bool, bool, error) { type RaftDEKManager struct { kw ca.KeyWriter rotationCh chan struct{} + FIPS bool } var errNoUpdateNeeded = fmt.Errorf("don't need to rotate or update") @@ -122,7 +129,7 @@ var errNotUsingRaftDEKData = fmt.Errorf("RaftDEKManager can no longer store and // NewRaftDEKManager returns a RaftDEKManager that uses the current key writer // and header manager -func NewRaftDEKManager(kw ca.KeyWriter) (*RaftDEKManager, error) { +func NewRaftDEKManager(kw ca.KeyWriter, fips bool) (*RaftDEKManager, error) { // If there is no current DEK, generate one and write it to disk err := kw.ViewAndUpdateHeaders(func(h ca.PEMKeyHeaders) (ca.PEMKeyHeaders, error) { dekData, ok := h.(RaftDEKData) @@ -132,6 +139,7 @@ func NewRaftDEKManager(kw ca.KeyWriter) (*RaftDEKManager, error) { EncryptionKeys: raft.EncryptionKeys{ CurrentDEK: encryption.GenerateSecretKey(), }, + FIPS: fips, }, nil } return nil, errNoUpdateNeeded @@ -141,6 +149,7 @@ func NewRaftDEKManager(kw ca.KeyWriter) (*RaftDEKManager, error) { } return &RaftDEKManager{ kw: kw, + FIPS: fips, rotationCh: make(chan struct{}, 1), }, nil } @@ -156,8 +165,9 @@ func (r *RaftDEKManager) NeedsRotation() bool { } // GetKeys returns the current set of DEKs. If NeedsRotation is true, and there -// is no existing PendingDEK, it will try to create one. If there are any errors -// doing so, just return the original. +// is no existing PendingDEK, it will try to create one. If it successfully creates +// and writes a PendingDEK, it sets NeedRotation to false. If there are any errors +// doing so, just return the original set of keys. func (r *RaftDEKManager) GetKeys() raft.EncryptionKeys { var newKeys, originalKeys raft.EncryptionKeys err := r.kw.ViewAndUpdateHeaders(func(h ca.PEMKeyHeaders) (ca.PEMKeyHeaders, error) { @@ -173,7 +183,10 @@ func (r *RaftDEKManager) GetKeys() raft.EncryptionKeys { CurrentDEK: data.CurrentDEK, PendingDEK: encryption.GenerateSecretKey(), } - return RaftDEKData{EncryptionKeys: newKeys}, nil + return RaftDEKData{ + EncryptionKeys: newKeys, + FIPS: data.FIPS, + }, nil }) if err != nil { return originalKeys @@ -202,6 +215,7 @@ func (r *RaftDEKManager) UpdateKeys(newKeys raft.EncryptionKeys) error { return RaftDEKData{ EncryptionKeys: newKeys, NeedsRotation: data.NeedsRotation, + FIPS: data.FIPS, }, nil }) } @@ -240,10 +254,10 @@ func (r *RaftDEKManager) MaybeUpdateKEK(candidateKEK ca.KEKData) (bool, bool, er return updated, unlockedToLocked, err } -func decodePEMHeaderValue(headerValue string, kek []byte) ([]byte, error) { +func decodePEMHeaderValue(headerValue string, kek []byte, fips bool) ([]byte, error) { var decrypter encryption.Decrypter = encryption.NoopCrypter if kek != nil { - _, decrypter = encryption.Defaults(kek, false) + _, decrypter = encryption.Defaults(kek, fips) } valueBytes, err := base64.StdEncoding.DecodeString(headerValue) if err != nil { @@ -256,10 +270,10 @@ func decodePEMHeaderValue(headerValue string, kek []byte) ([]byte, error) { return result, nil } -func encodePEMHeaderValue(headerValue []byte, kek []byte) (string, error) { +func encodePEMHeaderValue(headerValue []byte, kek []byte, fips bool) (string, error) { var encrypter encryption.Encrypter = encryption.NoopCrypter if kek != nil { - encrypter, _ = encryption.Defaults(kek, false) + encrypter, _ = encryption.Defaults(kek, fips) } encrypted, err := encryption.Encrypt(headerValue, encrypter) if err != nil { diff --git a/manager/deks_test.go b/manager/deks_test.go index 4873b05c62..63fdaf9141 100644 --- a/manager/deks_test.go +++ b/manager/deks_test.go @@ -3,6 +3,7 @@ package manager import ( "encoding/base64" "encoding/pem" + "fmt" "io/ioutil" "os" "testing" @@ -16,101 +17,121 @@ import ( // Tests updating a kek on a raftDEK object. func TestRaftDEKUpdateKEK(t *testing.T) { - startData := RaftDEKData{ - EncryptionKeys: raft.EncryptionKeys{CurrentDEK: []byte("first dek")}, + for _, fips := range []bool{true, false} { + startData := RaftDEKData{ + EncryptionKeys: raft.EncryptionKeys{CurrentDEK: []byte("first dek")}, + FIPS: fips, + } + startKEK := ca.KEKData{} + + // because UpdateKEK returns a PEMKeyHeaders interface, we need to cast to check + // values + updateDEKAndCast := func(dekdata RaftDEKData, oldKEK ca.KEKData, newKEK ca.KEKData) RaftDEKData { + result := dekdata.UpdateKEK(oldKEK, newKEK) + raftDekObj, ok := result.(RaftDEKData) + require.True(t, ok) + return raftDekObj + } + + // nothing changes if we are updating a kek and they're both nil + result := updateDEKAndCast(startData, startKEK, ca.KEKData{Version: 2}) + require.Equal(t, result, startData) + require.Equal(t, startData.FIPS, result.FIPS) // fips value should not have changed + + // when moving from unlocked to locked, a "needs rotation" header is generated but no + // pending header is generated + updatedKEK := ca.KEKData{KEK: []byte("something"), Version: 1} + result = updateDEKAndCast(startData, startKEK, updatedKEK) + require.NotEqual(t, startData, result) + require.True(t, result.NeedsRotation) + require.Equal(t, startData.CurrentDEK, result.CurrentDEK) + require.Nil(t, result.PendingDEK) + require.Equal(t, startData.FIPS, result.FIPS) // fips value should not have changed + + // this is whether or not pending exists + startData.PendingDEK = []byte("pending") + result = updateDEKAndCast(startData, startKEK, updatedKEK) + require.NotEqual(t, startData, result) + require.True(t, result.NeedsRotation) + require.Equal(t, startData.CurrentDEK, result.CurrentDEK) + require.Equal(t, startData.PendingDEK, result.PendingDEK) + require.Equal(t, startData.FIPS, result.FIPS) // fips value should not have changed + + // if we are going from locked to unlocked, nothing happens + result = updateDEKAndCast(startData, updatedKEK, startKEK) + require.Equal(t, startData, result) + require.False(t, result.NeedsRotation) + require.Equal(t, startData.FIPS, result.FIPS) // fips value should not have changed + + // if we are going to locked to another locked, nothing happens + result = updateDEKAndCast(startData, updatedKEK, ca.KEKData{KEK: []byte("other"), Version: 4}) + require.Equal(t, startData, result) + require.False(t, result.NeedsRotation) + require.Equal(t, startData.FIPS, result.FIPS) // fips value should not have changed } - startKEK := ca.KEKData{} - - // because UpdateKEK returns a PEMKeyHeaders interface, we need to cast to check - // values - updateDEKAndCast := func(dekdata RaftDEKData, oldKEK ca.KEKData, newKEK ca.KEKData) RaftDEKData { - result := dekdata.UpdateKEK(oldKEK, newKEK) - raftDekObj, ok := result.(RaftDEKData) - require.True(t, ok) - return raftDekObj - } - - // nothing changes if we are updating a kek and they're both nil - result := updateDEKAndCast(startData, startKEK, ca.KEKData{Version: 2}) - require.Equal(t, result, startData) - - // when moving from unlocked to locked, a "needs rotation" header is generated but no - // pending header is generated - updatedKEK := ca.KEKData{KEK: []byte("something"), Version: 1} - result = updateDEKAndCast(startData, startKEK, updatedKEK) - require.NotEqual(t, startData, result) - require.True(t, result.NeedsRotation) - require.Equal(t, startData.CurrentDEK, result.CurrentDEK) - require.Nil(t, result.PendingDEK) - - // this is whether or not pending exists - startData.PendingDEK = []byte("pending") - result = updateDEKAndCast(startData, startKEK, updatedKEK) - require.NotEqual(t, startData, result) - require.True(t, result.NeedsRotation) - require.Equal(t, startData.CurrentDEK, result.CurrentDEK) - require.Equal(t, startData.PendingDEK, result.PendingDEK) - - // if we are going from locked to unlocked, nothing happens - result = updateDEKAndCast(startData, updatedKEK, startKEK) - require.Equal(t, startData, result) - require.False(t, result.NeedsRotation) - - // if we are going to locked to another locked, nothing happens - result = updateDEKAndCast(startData, updatedKEK, ca.KEKData{KEK: []byte("other"), Version: 4}) - require.Equal(t, startData, result) - require.False(t, result.NeedsRotation) } func TestRaftDEKMarshalUnmarshal(t *testing.T) { - startData := RaftDEKData{ - EncryptionKeys: raft.EncryptionKeys{CurrentDEK: []byte("first dek")}, - } - kek := ca.KEKData{} - - headers, err := startData.MarshalHeaders(kek) - require.NoError(t, err) - require.Len(t, headers, 1) - - // can't unmarshal with the wrong kek - _, err = RaftDEKData{}.UnmarshalHeaders(headers, ca.KEKData{KEK: []byte("something")}) - require.Error(t, err) - - // we can unmarshal what was marshalled with the right kek - toData, err := RaftDEKData{}.UnmarshalHeaders(headers, kek) - require.NoError(t, err) - require.Equal(t, startData, toData) - - // try the other headers as well - startData.PendingDEK = []byte("Hello") - headers, err = startData.MarshalHeaders(kek) - require.NoError(t, err) - require.Len(t, headers, 2) - - // we can unmarshal what was marshalled - toData, err = RaftDEKData{}.UnmarshalHeaders(headers, kek) - require.NoError(t, err) - require.Equal(t, startData, toData) - - // try the other headers as well - startData.NeedsRotation = true - startData.PendingDEK = nil - headers, err = startData.MarshalHeaders(kek) - require.NoError(t, err) - require.Len(t, headers, 2) - - // we can unmarshal what was marshalled - toData, err = RaftDEKData{}.UnmarshalHeaders(headers, kek) - require.NoError(t, err) - require.Equal(t, startData, toData) - - // If there is a pending header, but no current header, set will fail - headers = map[string]string{ - pemHeaderRaftPendingDEK: headers[pemHeaderRaftDEK], + for _, fips := range []bool{true, false} { + startData := RaftDEKData{ + EncryptionKeys: raft.EncryptionKeys{CurrentDEK: []byte("first dek")}, + FIPS: fips, + } + kek := ca.KEKData{} + + headers, err := startData.MarshalHeaders(kek) + require.NoError(t, err) + require.Len(t, headers, 1) + + // can't unmarshal with the wrong kek + _, err = RaftDEKData{FIPS: fips}.UnmarshalHeaders(headers, ca.KEKData{KEK: []byte("something")}) + require.Error(t, err) + + // we can unmarshal what was marshalled with the right kek + toData, err := RaftDEKData{FIPS: fips}.UnmarshalHeaders(headers, kek) + require.NoError(t, err) + require.Equal(t, startData, toData) + casted, ok := toData.(RaftDEKData) + require.True(t, ok) + require.Equal(t, fips, casted.FIPS) // fips value should not have changed + + // try the other headers as well + startData.PendingDEK = []byte("Hello") + headers, err = startData.MarshalHeaders(kek) + require.NoError(t, err) + require.Len(t, headers, 2) + + // we can unmarshal what was marshalled + toData, err = RaftDEKData{FIPS: fips}.UnmarshalHeaders(headers, kek) + require.NoError(t, err) + require.Equal(t, startData, toData) + casted, ok = toData.(RaftDEKData) + require.True(t, ok) + require.Equal(t, fips, casted.FIPS) // fips value should not have changed + + // try the other headers as well + startData.NeedsRotation = true + startData.PendingDEK = nil + headers, err = startData.MarshalHeaders(kek) + require.NoError(t, err) + require.Len(t, headers, 2) + + // we can unmarshal what was marshalled + toData, err = RaftDEKData{FIPS: fips}.UnmarshalHeaders(headers, kek) + require.NoError(t, err) + require.Equal(t, startData, toData) + casted, ok = toData.(RaftDEKData) + require.True(t, ok) + require.Equal(t, fips, casted.FIPS) // fips value should not have changed + + // If there is a pending header, but no current header, set will fail + headers = map[string]string{ + pemHeaderRaftPendingDEK: headers[pemHeaderRaftDEK], + } + _, err = RaftDEKData{FIPS: fips}.UnmarshalHeaders(headers, kek) + require.Error(t, err) + require.Contains(t, err.Error(), "pending DEK, but no current DEK") } - _, err = RaftDEKData{}.UnmarshalHeaders(headers, kek) - require.Error(t, err) - require.Contains(t, err.Error(), "pending DEK, but no current DEK") } // NewRaftDEKManager creates a key if one doesn't exist @@ -123,38 +144,50 @@ func TestNewRaftDEKManager(t *testing.T) { cert, key, err := cautils.CreateRootCertAndKey("cn") require.NoError(t, err) - krw := ca.NewKeyReadWriter(paths.Node, nil, nil) - require.NoError(t, krw.Write(cert, key, nil)) + for _, fips := range []bool{true, false} { + krw := ca.NewKeyReadWriter(paths.Node, nil, nil) + require.NoError(t, krw.Write(cert, key, nil)) - keyBytes, err := ioutil.ReadFile(paths.Node.Key) - require.NoError(t, err) - require.NotContains(t, string(keyBytes), pemHeaderRaftDEK) // headers are not written + keyBytes, err := ioutil.ReadFile(paths.Node.Key) + require.NoError(t, err) + require.NotContains(t, string(keyBytes), pemHeaderRaftDEK) // headers are not written - dekManager, err := NewRaftDEKManager(krw) // this should create a new DEK and write it to the file - require.NoError(t, err) + dekManager, err := NewRaftDEKManager(krw, fips) // this should create a new DEK and write it to the file + require.NoError(t, err) - keyBytes, err = ioutil.ReadFile(paths.Node.Key) - require.NoError(t, err) - require.Contains(t, string(keyBytes), pemHeaderRaftDEK) // header is written now + keyBytes, err = ioutil.ReadFile(paths.Node.Key) + require.NoError(t, err) + require.Contains(t, string(keyBytes), pemHeaderRaftDEK) // header is written now - keys := dekManager.GetKeys() - require.NotNil(t, keys.CurrentDEK) - require.Nil(t, keys.PendingDEK) - require.False(t, dekManager.NeedsRotation()) + // ensure that the created raft DEK uses FIPS + h, _ := krw.GetCurrentState() + casted, ok := h.(RaftDEKData) + require.True(t, ok) + require.Equal(t, fips, casted.FIPS) - // If one exists, nothing is updated - dekManager, err = NewRaftDEKManager(krw) // this should create a new DEK and write it to the file - require.NoError(t, err) + keys := dekManager.GetKeys() + require.NotNil(t, keys.CurrentDEK) + require.Nil(t, keys.PendingDEK) + require.False(t, dekManager.NeedsRotation()) - keyBytes2, err := ioutil.ReadFile(paths.Node.Key) - require.NoError(t, err) - require.Equal(t, keyBytes, keyBytes2) + // If one exists, nothing is updated + dekManager, err = NewRaftDEKManager(krw, fips) // this should not have created a new dek + require.NoError(t, err) - require.Equal(t, keys, dekManager.GetKeys()) - require.False(t, dekManager.NeedsRotation()) + keyBytes2, err := ioutil.ReadFile(paths.Node.Key) + require.NoError(t, err) + require.Equal(t, keyBytes, keyBytes2) + + require.Equal(t, keys, dekManager.GetKeys()) + require.False(t, dekManager.NeedsRotation()) + } } -// NeedsRotate returns true if there is a PendingDEK or a NeedsRotation flag +// NeedsRotate returns true if there is a PendingDEK or a NeedsRotation flag. GetKeys() evaluates +// whether a PendingDEK is there, and if there's no pending DEK but there is a NeedsRotation flag, +// it creates a PendingDEK and removes the NeedsRotation flag. If both the PendingDEK and +// NeedsRotation flag are there, it does not remove the NeedsRotation flag, because that indicates +// that we basically need to do 2 rotations. func TestRaftDEKManagerNeedsRotateGetKeys(t *testing.T) { tempDir, err := ioutil.TempDir("", "manager-maybe-get-data-") require.NoError(t, err) @@ -162,80 +195,110 @@ func TestRaftDEKManagerNeedsRotateGetKeys(t *testing.T) { paths := ca.NewConfigPaths(tempDir) - // if there is no PendingDEK, and no NeedsRotation flag: NeedsRotation=false - keys := raft.EncryptionKeys{CurrentDEK: []byte("hello")} - dekManager, err := NewRaftDEKManager( - ca.NewKeyReadWriter(paths.Node, nil, RaftDEKData{EncryptionKeys: keys})) - require.NoError(t, err) - - require.False(t, dekManager.NeedsRotation()) - require.Equal(t, keys, dekManager.GetKeys()) - - // if there is a PendingDEK, and no NeedsRotation flag: NeedsRotation=true - keys = raft.EncryptionKeys{CurrentDEK: []byte("hello"), PendingDEK: []byte("another")} - dekManager, err = NewRaftDEKManager( - ca.NewKeyReadWriter(paths.Node, nil, RaftDEKData{EncryptionKeys: keys})) - require.NoError(t, err) - - require.True(t, dekManager.NeedsRotation()) - require.Equal(t, keys, dekManager.GetKeys()) - - // if there is a PendingDEK, and a NeedsRotation flag: NeedsRotation=true - keys = raft.EncryptionKeys{CurrentDEK: []byte("hello"), PendingDEK: []byte("another")} - dekManager, err = NewRaftDEKManager( - ca.NewKeyReadWriter(paths.Node, nil, RaftDEKData{ - EncryptionKeys: keys, - NeedsRotation: true, - })) - require.NoError(t, err) - - require.True(t, dekManager.NeedsRotation()) - require.Equal(t, keys, dekManager.GetKeys()) - - // if there no PendingDEK, and a NeedsRotation flag: NeedsRotation=true and - // GetKeys attempts to create a pending key and write it to disk. However, writing - // will error (because there is no key on disk atm), and then the original keys will - // be returned. - keys = raft.EncryptionKeys{CurrentDEK: []byte("hello")} - krw := ca.NewKeyReadWriter(paths.Node, nil, RaftDEKData{ - EncryptionKeys: keys, - NeedsRotation: true, - }) - dekManager, err = NewRaftDEKManager(krw) - require.NoError(t, err) - - require.True(t, dekManager.NeedsRotation()) - require.Equal(t, keys, dekManager.GetKeys()) - h, _ := krw.GetCurrentState() - dekData, ok := h.(RaftDEKData) - require.True(t, ok) - require.True(t, dekData.NeedsRotation) - - // if there no PendingDEK, and a NeedsRotation flag: NeedsRotation=true and - // GetKeys attempts to create a pending key and write it to disk. If successful, - // it returns the new keys - keys = raft.EncryptionKeys{CurrentDEK: []byte("hello")} - krw = ca.NewKeyReadWriter(paths.Node, nil, RaftDEKData{ - EncryptionKeys: keys, - NeedsRotation: true, - }) - dekManager, err = NewRaftDEKManager(krw) - - require.NoError(t, err) - cert, key, err := cautils.CreateRootCertAndKey("cn") - require.NoError(t, err) - require.NoError(t, krw.Write(cert, key, nil)) - - require.True(t, dekManager.NeedsRotation()) - updatedKeys := dekManager.GetKeys() - require.Equal(t, keys.CurrentDEK, updatedKeys.CurrentDEK) - require.NotNil(t, updatedKeys.PendingDEK) - require.True(t, dekManager.NeedsRotation()) - - h, _ = krw.GetCurrentState() - dekData, ok = h.(RaftDEKData) - require.True(t, ok) - require.False(t, dekData.NeedsRotation) + for _, fips := range []bool{true, false} { + for _, testcase := range []struct { + description string + dekData RaftDEKData + managerNeedsRotation bool + newDEKDataNeedsRotation bool + keyOnDisk bool + }{ + { + description: "if there is no PendingDEK, and no NeedsRotation flag: NeedsRotation()->false, DEKData.NeedsRotation->false", + keyOnDisk: true, + dekData: RaftDEKData{ + EncryptionKeys: raft.EncryptionKeys{CurrentDEK: []byte("hello")}, + NeedsRotation: false, + }, + managerNeedsRotation: false, + newDEKDataNeedsRotation: false, + }, + { + description: "if there is a PendingDEK, and no NeedsRotation flag: NeedsRotation()->true, DEKData.NeedsRotation->false", + keyOnDisk: true, + dekData: RaftDEKData{ + EncryptionKeys: raft.EncryptionKeys{ + CurrentDEK: []byte("hello"), + PendingDEK: []byte("another"), + }, + NeedsRotation: false, + }, + managerNeedsRotation: true, + newDEKDataNeedsRotation: false, + }, + { + description: "if there is a PendingDEK, and a NeedsRotation flag: NeedsRotation()->true, DEKData.NeedsRotation->true", + keyOnDisk: true, + dekData: RaftDEKData{ + EncryptionKeys: raft.EncryptionKeys{ + CurrentDEK: []byte("hello"), + PendingDEK: []byte("another"), + }, + NeedsRotation: true, + }, + managerNeedsRotation: true, + newDEKDataNeedsRotation: true, + }, + // These in these two cases, the original keys did not have pending keys. GetKeys + // should create them, but only if it can write the new pending key to the disk. + { + description: ` + if there no PendingDEK, and a NeedsRotation flag: NeedsRotation()->true and + GetKeys attempts to create a pending key and write it to disk. However, writing + will error (because there is no key on disk atm), and then the original keys will + be returned. So DEKData.NeedsRotation->true.`, + keyOnDisk: false, + dekData: RaftDEKData{ + EncryptionKeys: raft.EncryptionKeys{CurrentDEK: []byte("hello")}, + NeedsRotation: true, + }, + managerNeedsRotation: true, + newDEKDataNeedsRotation: true, + }, + { + description: ` + if there no PendingDEK, and there is a NeedsRotation flag: NeedsRotation()->true and + GetKeys attempts to create a pending key and write it to disk. Once a pending key is + created, the NeedsRotation flag can be set to false. So DEKData.NeedsRotation->false`, + keyOnDisk: true, + dekData: RaftDEKData{ + EncryptionKeys: raft.EncryptionKeys{CurrentDEK: []byte("hello")}, + NeedsRotation: true, + }, + managerNeedsRotation: true, + newDEKDataNeedsRotation: false, + }, + } { + // clear the directory + require.NoError(t, os.RemoveAll(tempDir)) + os.Mkdir(tempDir, 0777) + testcase.dekData.FIPS = fips + krw := ca.NewKeyReadWriter(paths.Node, nil, testcase.dekData) + if testcase.keyOnDisk { + cert, key, err := cautils.CreateRootCertAndKey("cn") + require.NoError(t, err) + require.NoError(t, krw.Write(cert, key, nil)) + } + dekManager, err := NewRaftDEKManager(krw, fips) + require.NoError(t, err) + + require.Equal(t, testcase.managerNeedsRotation, dekManager.NeedsRotation(), testcase.description) + + gotKeys := dekManager.GetKeys() + if testcase.dekData.NeedsRotation && testcase.dekData.EncryptionKeys.PendingDEK == nil && testcase.keyOnDisk { + require.Equal(t, testcase.dekData.EncryptionKeys.CurrentDEK, gotKeys.CurrentDEK, testcase.description) + require.NotNil(t, gotKeys.PendingDEK, testcase.description) + } else { + require.Equal(t, testcase.dekData.EncryptionKeys, gotKeys, testcase.description) + } + + h, _ := krw.GetCurrentState() + dekData, ok := h.(RaftDEKData) + require.True(t, ok) + require.Equal(t, testcase.newDEKDataNeedsRotation, dekData.NeedsRotation, + "(FIPS: %v) %s", fips, testcase.description) + } + } } func TestRaftDEKManagerUpdateKeys(t *testing.T) { @@ -251,42 +314,46 @@ func TestRaftDEKManagerUpdateKeys(t *testing.T) { CurrentDEK: []byte("key1"), PendingDEK: []byte("key2"), } - krw := ca.NewKeyReadWriter(paths.Node, nil, RaftDEKData{ - EncryptionKeys: keys, - NeedsRotation: true, - }) - require.NoError(t, krw.Write(cert, key, nil)) + for _, fips := range []bool{true, false} { + krw := ca.NewKeyReadWriter(paths.Node, nil, RaftDEKData{ + EncryptionKeys: keys, + NeedsRotation: true, + FIPS: fips, + }) + require.NoError(t, krw.Write(cert, key, nil)) - dekManager, err := NewRaftDEKManager(krw) - require.NoError(t, err) + dekManager, err := NewRaftDEKManager(krw, fips) + require.NoError(t, err) - newKeys := raft.EncryptionKeys{ - CurrentDEK: []byte("new current"), - } - require.NoError(t, dekManager.UpdateKeys(newKeys)) - // don't run GetKeys, because NeedsRotation is true and it'd just generate a new one + newKeys := raft.EncryptionKeys{ + CurrentDEK: []byte("new current"), + } + require.NoError(t, dekManager.UpdateKeys(newKeys)) + // don't run GetKeys, because NeedsRotation is true and it'd just generate a new one - h, _ := krw.GetCurrentState() - dekData, ok := h.(RaftDEKData) - require.True(t, ok) - require.True(t, dekData.NeedsRotation) + h, _ := krw.GetCurrentState() + dekData, ok := h.(RaftDEKData) + require.True(t, ok) + require.True(t, dekData.NeedsRotation) + require.Equal(t, fips, dekData.FIPS) - // UpdateKeys so there is no CurrentDEK: all the headers should be wiped out - require.NoError(t, dekManager.UpdateKeys(raft.EncryptionKeys{})) - require.Equal(t, raft.EncryptionKeys{}, dekManager.GetKeys()) - require.False(t, dekManager.NeedsRotation()) + // UpdateKeys so there is no CurrentDEK: all the headers should be wiped out + require.NoError(t, dekManager.UpdateKeys(raft.EncryptionKeys{})) + require.Equal(t, raft.EncryptionKeys{}, dekManager.GetKeys()) + require.False(t, dekManager.NeedsRotation()) - h, _ = krw.GetCurrentState() - require.Nil(t, h) + h, _ = krw.GetCurrentState() + require.Nil(t, h) - keyBytes, err := ioutil.ReadFile(paths.Node.Key) - require.NoError(t, err) - keyBlock, _ := pem.Decode(keyBytes) - require.NotNil(t, keyBlock) + keyBytes, err := ioutil.ReadFile(paths.Node.Key) + require.NoError(t, err) + keyBlock, _ := pem.Decode(keyBytes) + require.NotNil(t, keyBlock) - // the only header remaining should be the kek version - require.Len(t, keyBlock.Headers, 1) - require.Contains(t, keyBlock.Headers, "kek-version") + // the only header remaining should be the kek version + require.Len(t, keyBlock.Headers, 1) + require.Contains(t, keyBlock.Headers, "kek-version") + } } func TestRaftDEKManagerMaybeUpdateKEK(t *testing.T) { @@ -300,101 +367,112 @@ func TestRaftDEKManagerMaybeUpdateKEK(t *testing.T) { keys := raft.EncryptionKeys{CurrentDEK: []byte("current dek")} - // trying to update a KEK will error if the version is the same but the kek is different - krw := ca.NewKeyReadWriter(paths.Node, nil, RaftDEKData{EncryptionKeys: keys}) - require.NoError(t, krw.Write(cert, key, nil)) - dekManager, err := NewRaftDEKManager(krw) - require.NoError(t, err) - - keyBytes, err := ioutil.ReadFile(paths.Node.Key) - require.NoError(t, err) - - _, _, err = dekManager.MaybeUpdateKEK(ca.KEKData{KEK: []byte("locked now")}) - require.Error(t, err) - require.False(t, dekManager.NeedsRotation()) - - keyBytes2, err := ioutil.ReadFile(paths.Node.Key) - require.NoError(t, err) - require.Equal(t, keyBytes, keyBytes2) - - // trying to update a KEK from unlocked to lock will set NeedsRotation to true, as well as encrypt the TLS key - updated, unlockedToLocked, err := dekManager.MaybeUpdateKEK(ca.KEKData{KEK: []byte("locked now"), Version: 1}) - require.NoError(t, err) - require.True(t, updated) - require.True(t, unlockedToLocked) - // don't run GetKeys, because NeedsRotation is true and it'd just generate a new one - h, _ := krw.GetCurrentState() - dekData, ok := h.(RaftDEKData) - require.True(t, ok) - require.Equal(t, keys, dekData.EncryptionKeys) - require.True(t, dekData.NeedsRotation) - require.NotNil(t, <-dekManager.RotationNotify()) // we are notified of a new pending key - - keyBytes2, err = ioutil.ReadFile(paths.Node.Key) - require.NoError(t, err) - require.NotEqual(t, keyBytes, keyBytes2) - keyBytes = keyBytes2 - - readKRW := ca.NewKeyReadWriter(paths.Node, []byte("locked now"), RaftDEKData{}) - _, _, err = readKRW.Read() - require.NoError(t, err) - - // trying to update a KEK of a lower version will not update anything, but will not error - updated, unlockedToLocked, err = dekManager.MaybeUpdateKEK(ca.KEKData{}) - require.NoError(t, err) - require.False(t, unlockedToLocked) - require.False(t, updated) - // don't run GetKeys, because NeedsRotation is true and it'd just generate a new one - h, _ = krw.GetCurrentState() - dekData, ok = h.(RaftDEKData) - require.True(t, ok) - require.Equal(t, keys, dekData.EncryptionKeys) - require.True(t, dekData.NeedsRotation) - - keyBytes2, err = ioutil.ReadFile(paths.Node.Key) - require.NoError(t, err) - require.Equal(t, keyBytes, keyBytes2, string(keyBytes), string(keyBytes2)) - - // updating a kek to a higher version, but with the same kek, will also neither update anything nor error - updated, unlockedToLocked, err = dekManager.MaybeUpdateKEK(ca.KEKData{KEK: []byte("locked now"), Version: 100}) - require.NoError(t, err) - require.False(t, unlockedToLocked) - require.False(t, updated) - // don't run GetKeys, because NeedsRotation is true and it'd just generate a new one - h, _ = krw.GetCurrentState() - dekData, ok = h.(RaftDEKData) - require.True(t, ok) - require.Equal(t, keys, dekData.EncryptionKeys) - require.True(t, dekData.NeedsRotation) - - keyBytes2, err = ioutil.ReadFile(paths.Node.Key) - require.NoError(t, err) - require.Equal(t, keyBytes, keyBytes2) - - // going from locked to unlock does not result in the NeedsRotation flag, but does result in - // the key being decrypted - krw = ca.NewKeyReadWriter(paths.Node, []byte("kek"), RaftDEKData{EncryptionKeys: keys}) - require.NoError(t, krw.Write(cert, key, nil)) - dekManager, err = NewRaftDEKManager(krw) - require.NoError(t, err) - - keyBytes, err = ioutil.ReadFile(paths.Node.Key) - require.NoError(t, err) + for _, fips := range []bool{true, false} { + // trying to update a KEK will error if the version is the same but the kek is different + krw := ca.NewKeyReadWriter(paths.Node, nil, RaftDEKData{ + EncryptionKeys: keys, + FIPS: fips, + }) + require.NoError(t, krw.Write(cert, key, nil)) + dekManager, err := NewRaftDEKManager(krw, fips) + require.NoError(t, err) + + keyBytes, err := ioutil.ReadFile(paths.Node.Key) + require.NoError(t, err) + + _, _, err = dekManager.MaybeUpdateKEK(ca.KEKData{KEK: []byte("locked now")}) + require.Error(t, err) + require.False(t, dekManager.NeedsRotation()) + + keyBytes2, err := ioutil.ReadFile(paths.Node.Key) + require.NoError(t, err) + require.Equal(t, keyBytes, keyBytes2) + + // trying to update a KEK from unlocked to lock will set NeedsRotation to true, as well as encrypt the TLS key + updated, unlockedToLocked, err := dekManager.MaybeUpdateKEK(ca.KEKData{KEK: []byte("locked now"), Version: 1}) + require.NoError(t, err) + require.True(t, updated) + require.True(t, unlockedToLocked) + // don't run GetKeys, because NeedsRotation is true and it'd just generate a new one + h, _ := krw.GetCurrentState() + dekData, ok := h.(RaftDEKData) + require.True(t, ok) + require.Equal(t, keys, dekData.EncryptionKeys) + require.True(t, dekData.NeedsRotation) + require.Equal(t, fips, dekData.FIPS) + require.NotNil(t, <-dekManager.RotationNotify()) // we are notified of a new pending key + + keyBytes2, err = ioutil.ReadFile(paths.Node.Key) + require.NoError(t, err) + require.NotEqual(t, keyBytes, keyBytes2) + keyBytes = keyBytes2 + + readKRW := ca.NewKeyReadWriter(paths.Node, []byte("locked now"), RaftDEKData{FIPS: fips}) + _, _, err = readKRW.Read() + require.NoError(t, err) + + // trying to update a KEK of a lower version will not update anything, but will not error + updated, unlockedToLocked, err = dekManager.MaybeUpdateKEK(ca.KEKData{}) + require.NoError(t, err) + require.False(t, unlockedToLocked) + require.False(t, updated) + // don't run GetKeys, because NeedsRotation is true and it'd just generate a new one + h, _ = krw.GetCurrentState() + dekData, ok = h.(RaftDEKData) + require.True(t, ok) + require.Equal(t, keys, dekData.EncryptionKeys) + require.True(t, dekData.NeedsRotation) + require.Equal(t, fips, dekData.FIPS) + + keyBytes2, err = ioutil.ReadFile(paths.Node.Key) + require.NoError(t, err) + require.Equal(t, keyBytes, keyBytes2, string(keyBytes), string(keyBytes2)) + + // updating a kek to a higher version, but with the same kek, will also neither update anything nor error + updated, unlockedToLocked, err = dekManager.MaybeUpdateKEK(ca.KEKData{KEK: []byte("locked now"), Version: 100}) + require.NoError(t, err) + require.False(t, unlockedToLocked) + require.False(t, updated) + // don't run GetKeys, because NeedsRotation is true and it'd just generate a new one + h, _ = krw.GetCurrentState() + dekData, ok = h.(RaftDEKData) + require.True(t, ok) + require.Equal(t, keys, dekData.EncryptionKeys) + require.True(t, dekData.NeedsRotation) + require.Equal(t, fips, dekData.FIPS) - updated, unlockedToLocked, err = dekManager.MaybeUpdateKEK(ca.KEKData{Version: 2}) - require.NoError(t, err) - require.False(t, unlockedToLocked) - require.True(t, updated) - require.Equal(t, keys, dekManager.GetKeys()) - require.False(t, dekManager.NeedsRotation()) + keyBytes2, err = ioutil.ReadFile(paths.Node.Key) + require.NoError(t, err) + require.Equal(t, keyBytes, keyBytes2) - keyBytes2, err = ioutil.ReadFile(paths.Node.Key) - require.NoError(t, err) - require.NotEqual(t, keyBytes, keyBytes2) - - readKRW = ca.NewKeyReadWriter(paths.Node, nil, RaftDEKData{}) - _, _, err = readKRW.Read() - require.NoError(t, err) + // going from locked to unlock does not result in the NeedsRotation flag, but does result in + // the key being decrypted + krw = ca.NewKeyReadWriter(paths.Node, []byte("kek"), RaftDEKData{ + EncryptionKeys: keys, + FIPS: fips, + }) + require.NoError(t, krw.Write(cert, key, nil)) + dekManager, err = NewRaftDEKManager(krw, fips) + require.NoError(t, err) + + keyBytes, err = ioutil.ReadFile(paths.Node.Key) + require.NoError(t, err) + + updated, unlockedToLocked, err = dekManager.MaybeUpdateKEK(ca.KEKData{Version: 2}) + require.NoError(t, err) + require.False(t, unlockedToLocked) + require.True(t, updated) + require.Equal(t, keys, dekManager.GetKeys()) + require.False(t, dekManager.NeedsRotation()) + + keyBytes2, err = ioutil.ReadFile(paths.Node.Key) + require.NoError(t, err) + require.NotEqual(t, keyBytes, keyBytes2) + + readKRW = ca.NewKeyReadWriter(paths.Node, nil, RaftDEKData{FIPS: fips}) + _, _, err = readKRW.Read() + require.NoError(t, err) + } } // The TLS KEK and the KEK for the headers should be in sync, and so failing @@ -461,3 +539,34 @@ O0T3aXuZGYNyh//KqAoA3erCmh6HauMz84Y= _, _, err = krw.Read() require.NoError(t, err) } + +// If FIPS is enabled, the raft DEK will be encrypted using fernet, and not NACL secretbox. +func TestRaftDEKsFIPSEnabledUsesFernet(t *testing.T) { + tempDir, err := ioutil.TempDir("", "manager-dek-fips") + require.NoError(t, err) + defer os.RemoveAll(tempDir) + + paths := ca.NewConfigPaths(tempDir) + cert, key, err := cautils.CreateRootCertAndKey("cn") + require.NoError(t, err) + + // no particular reason not to use FIPS in the key writer to write the TLS key itself, + // except to demonstrate that these two functionalities are decoupled + keys := raft.EncryptionKeys{CurrentDEK: []byte("current dek")} + krw := ca.NewKeyReadWriter(paths.Node, nil, RaftDEKData{EncryptionKeys: keys, FIPS: true}) + require.NoError(t, krw.Write(cert, key, nil)) + + dekManager, err := NewRaftDEKManager(krw, true) // this should be able to read the dek data + require.NoError(t, err) + require.Equal(t, keys, dekManager.GetKeys()) + + // if we do not use FIPS to write the header in the first place, a FIPS DEK manager can't read it + // because it's NACL secretbox + keys = raft.EncryptionKeys{CurrentDEK: []byte("current dek")} + krw = ca.NewKeyReadWriter(paths.Node, nil, RaftDEKData{EncryptionKeys: keys}) + require.NoError(t, krw.Write(cert, key, nil)) + + dekManager, err = NewRaftDEKManager(krw, true) // this should be able to read the dek data + require.NoError(t, err) + fmt.Println(err) +} diff --git a/manager/manager.go b/manager/manager.go index 30ec887c67..814b1b54c1 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -213,7 +213,7 @@ func New(config *Config) (*Manager, error) { raftCfg.HeartbeatTick = int(config.HeartbeatTick) } - dekRotator, err := NewRaftDEKManager(config.SecurityConfig.KeyWriter()) + dekRotator, err := NewRaftDEKManager(config.SecurityConfig.KeyWriter(), false) if err != nil { return nil, err } diff --git a/manager/manager_test.go b/manager/manager_test.go index 3e15605ed7..f55b7f38a8 100644 --- a/manager/manager_test.go +++ b/manager/manager_test.go @@ -296,7 +296,7 @@ func TestManagerLockUnlock(t *testing.T) { require.NotNil(t, keyBlock) require.False(t, keyutils.IsEncryptedPEMBlock(keyBlock)) require.Len(t, keyBlock.Headers, 2) - currentDEK, err := decodePEMHeaderValue(keyBlock.Headers[pemHeaderRaftDEK], nil) + currentDEK, err := decodePEMHeaderValue(keyBlock.Headers[pemHeaderRaftDEK], nil, false) require.NoError(t, err) require.NotEmpty(t, currentDEK) @@ -349,7 +349,7 @@ func TestManagerLockUnlock(t *testing.T) { // a little bit, and is best effort only currentDEKString, ok := keyBlock.Headers[pemHeaderRaftDEK] require.True(t, ok) // there should never NOT be a current header - nowCurrentDEK, err := decodePEMHeaderValue(currentDEKString, unlockKeyResp.UnlockKey) + nowCurrentDEK, err := decodePEMHeaderValue(currentDEKString, unlockKeyResp.UnlockKey, false) require.NoError(t, err) // it should always be encrypted if bytes.Equal(currentDEK, nowCurrentDEK) { return fmt.Errorf("snapshot has not been finished yet") @@ -422,8 +422,13 @@ func TestManagerLockUnlock(t *testing.T) { return nil }, 1*time.Second)) - // the DEK should not have been rotated, just decrypted (which was tested previously) - unencryptedDEK, err := decodePEMHeaderValue(keyBlock.Headers[pemHeaderRaftDEK], nil) + // the new key should not be encrypted, and the DEK should also be unencrypted + // but not rotated + keyBlock, _ = pem.Decode(unlockedKey) + require.NotNil(t, keyBlock) + require.False(t, keyutils.IsEncryptedPEMBlock(keyBlock)) + + unencryptedDEK, err := decodePEMHeaderValue(keyBlock.Headers[pemHeaderRaftDEK], nil, false) require.NoError(t, err) require.NotNil(t, unencryptedDEK) require.Equal(t, currentDEK, unencryptedDEK) From ba11e512a26f2866ca34ecd887af31f77c472be6 Mon Sep 17 00:00:00 2001 From: Ying Li Date: Tue, 3 Apr 2018 10:50:46 -0700 Subject: [PATCH 2/2] Propagate the FIPS boolean from node.go to the manager and hence to the raft storage layer. Also propagate it to the RaftDEKData objects in node.go and to the RaftDEKManager in the manager. Signed-off-by: Ying Li --- integration/integration_test.go | 10 ++++++++-- manager/manager.go | 3 ++- manager/state/raft/raft.go | 3 +++ manager/state/raft/storage.go | 1 + node/node.go | 5 +++-- 5 files changed, 17 insertions(+), 5 deletions(-) diff --git a/integration/integration_test.go b/integration/integration_test.go index e1258e5ce7..5fc6ef0efb 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -888,9 +888,15 @@ func TestMixedFIPSClusterNonMandatoryFIPS(t *testing.T) { pollClusterReady(t, cl, 2, 3) - // swap which nodes are fips and which are not - all should start up just fine + // switch which worker nodes are fips and which are not - all should start up just fine + // on managers, if we enable fips on a previously non-fips node, it won't be able to read + // non-fernet raft logs for nodeID, n := range cl.nodes { - n.config.FIPS = !n.config.FIPS + if n.IsManager() { + n.config.FIPS = false + } else { + n.config.FIPS = !n.config.FIPS + } require.NoError(t, n.Pause(false)) require.NoError(t, cl.StartNode(nodeID)) } diff --git a/manager/manager.go b/manager/manager.go index 814b1b54c1..ef1f6acec2 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -213,7 +213,7 @@ func New(config *Config) (*Manager, error) { raftCfg.HeartbeatTick = int(config.HeartbeatTick) } - dekRotator, err := NewRaftDEKManager(config.SecurityConfig.KeyWriter(), false) + dekRotator, err := NewRaftDEKManager(config.SecurityConfig.KeyWriter(), config.FIPS) if err != nil { return nil, err } @@ -227,6 +227,7 @@ func New(config *Config) (*Manager, error) { ForceNewCluster: config.ForceNewCluster, TLSCredentials: config.SecurityConfig.ClientTLSCreds, KeyRotator: dekRotator, + FIPS: config.FIPS, } raftNode := raft.NewNode(newNodeOpts) diff --git a/manager/state/raft/raft.go b/manager/state/raft/raft.go index 3e3410fe04..ce011b7d98 100644 --- a/manager/state/raft/raft.go +++ b/manager/state/raft/raft.go @@ -192,6 +192,9 @@ type NodeOptions struct { // DisableStackDump prevents Run from dumping goroutine stacks when the // store becomes stuck. DisableStackDump bool + + // FIPS specifies whether the raft encryption should be FIPS compliant + FIPS bool } func init() { diff --git a/manager/state/raft/storage.go b/manager/state/raft/storage.go index f538317c06..547b775645 100644 --- a/manager/state/raft/storage.go +++ b/manager/state/raft/storage.go @@ -34,6 +34,7 @@ func (n *Node) readFromDisk(ctx context.Context) (*raftpb.Snapshot, storage.WALD n.raftLogger = &storage.EncryptedRaftLogger{ StateDir: n.opts.StateDir, EncryptionKey: keys.CurrentDEK, + FIPS: n.opts.FIPS, } if keys.PendingDEK != nil { n.raftLogger.EncryptionKey = keys.PendingDEK diff --git a/node/node.go b/node/node.go index e1772975a7..3ebc4e673f 100644 --- a/node/node.go +++ b/node/node.go @@ -782,7 +782,7 @@ func (n *Node) loadSecurityConfig(ctx context.Context, paths *ca.SecurityConfigP cancel func() error ) - krw := ca.NewKeyReadWriter(paths.Node, n.unlockKey, &manager.RaftDEKData{}) + krw := ca.NewKeyReadWriter(paths.Node, n.unlockKey, &manager.RaftDEKData{FIPS: n.config.FIPS}) // if FIPS is required, we want to make sure our key is stored in PKCS8 format if n.config.FIPS { krw.SetKeyFormatter(keyutils.FIPS) @@ -816,7 +816,7 @@ func (n *Node) loadSecurityConfig(ctx context.Context, paths *ca.SecurityConfigP if n.config.AutoLockManagers { n.unlockKey = encryption.GenerateSecretKey() } - krw = ca.NewKeyReadWriter(paths.Node, n.unlockKey, &manager.RaftDEKData{}) + krw = ca.NewKeyReadWriter(paths.Node, n.unlockKey, &manager.RaftDEKData{FIPS: n.config.FIPS}) rootCA, err = ca.CreateRootCA(ca.DefaultRootCN) if err != nil { return nil, nil, err @@ -995,6 +995,7 @@ func (n *Node) runManager(ctx context.Context, securityConfig *ca.SecurityConfig Availability: n.config.Availability, PluginGetter: n.config.PluginGetter, RootCAPaths: rootPaths, + FIPS: n.config.FIPS, }) if err != nil { return false, err