Skip to content

[fips] Propagate the FIPS boolean to the manager, raft storage layer, and raft DEK management #2586

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.

Sign up for GitHub

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

Merged
cyli merged 2 commits into from
May 7, 2018
Merged

[fips] Propagate the FIPS boolean to the manager, raft storage layer, and raft DEK management #2586

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
905d35b
Propagate the FIPS bool to the raft DEK manager so that the DEK is en…
cyli Apr 2, 2018
ba11e51
Propagate the FIPS boolean from node.go to the manager and hence to t…
cyli Apr 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions integration/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
36 changes: 25 additions & 11 deletions manager/deks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -55,6 +59,7 @@ func (r RaftDEKData) UnmarshalHeaders(headers map[string]string, kekData ca.KEKD
CurrentDEK: currentDEK,
PendingDEK: pendingDEK,
},
FIPS: r.FIPS,
}, nil
}

Expand All @@ -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
}
Expand All @@ -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
Expand All @@ -112,6 +118,7 @@ func compareKEKs(oldKEK, candidateKEK ca.KEKData) (bool, bool, error) {
type RaftDEKManager struct {
kw ca.KeyWriter
rotationCh chan struct{}
FIPS bool
Copy link
Copy Markdown
Contributor

@anshulpundir anshulpundir May 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add a comment on what this flag indicates.

}

var errNoUpdateNeeded = fmt.Errorf("don't need to rotate or update")
Expand All @@ -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)
Expand All @@ -132,6 +139,7 @@ func NewRaftDEKManager(kw ca.KeyWriter) (*RaftDEKManager, error) {
EncryptionKeys: raft.EncryptionKeys{
CurrentDEK: encryption.GenerateSecretKey(),
},
FIPS: fips,
}, nil
}
return nil, errNoUpdateNeeded
Expand All @@ -141,6 +149,7 @@ func NewRaftDEKManager(kw ca.KeyWriter) (*RaftDEKManager, error) {
}
return &RaftDEKManager{
kw: kw,
FIPS: fips,
rotationCh: make(chan struct{}, 1),
}, nil
}
Expand All @@ -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
Copy link
Copy Markdown
Contributor

@anshulpundir anshulpundir May 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name 'GetKeys' implies a read-only operation so I would suggest avoid doing operations within this function that have side effects.

Copy link
Copy Markdown
Contributor Author

@cyli cyli May 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be happy to revisit this at a later point, but that's probably beyond the scope of this PR.

// 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) {
Expand All @@ -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
Expand Down Expand Up @@ -202,6 +215,7 @@ func (r *RaftDEKManager) UpdateKeys(newKeys raft.EncryptionKeys) error {
return RaftDEKData{
EncryptionKeys: newKeys,
NeedsRotation: data.NeedsRotation,
FIPS: data.FIPS,
}, nil
})
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
Loading