From b69d1bffa81fef702efdbcc4cf28123f102ed096 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Thu, 2 Jun 2022 13:41:59 -0400 Subject: [PATCH] remove end-user algorithm selection (#13190) After internal design review, we decided to remove exposing algorithm choice to the end-user for the initial release. We'll solve nonce rotation by forcing rotations automatically on key GC (in a core job, not included in this changeset). Default to AES-256 GCM for the following criteria: * faster implementation when hardware acceleration is available * FIPS compliant * implementation in pure go * post-quantum resistance Also fixed a bug in the decoding from keystore and switched to a harder-to-misuse encoding method. --- api/keyring.go | 14 ++++++------ api/keyring_test.go | 12 +++++------ command/agent/keyring_endpoint.go | 9 +++----- command/agent/keyring_endpoint_test.go | 12 +++++------ nomad/encrypter.go | 21 +----------------- nomad/encrypter_test.go | 1 - nomad/keyring_endpoint.go | 3 +-- nomad/keyring_endpoint_test.go | 16 ++++++++------ nomad/leader.go | 3 +-- nomad/state/state_store.go | 5 ----- nomad/structs/extensions.go | 6 ++---- nomad/structs/secure_variables.go | 30 ++++++++------------------ 12 files changed, 42 insertions(+), 90 deletions(-) diff --git a/api/keyring.go b/api/keyring.go index e00e13cc9..b75cbc681 100644 --- a/api/keyring.go +++ b/api/keyring.go @@ -21,7 +21,6 @@ func (c *Client) Keyring() *Keyring { type EncryptionAlgorithm string const ( - EncryptionAlgorithmXChaCha20 EncryptionAlgorithm = "xchacha20" EncryptionAlgorithmAES256GCM EncryptionAlgorithm = "aes256-gcm" ) @@ -34,13 +33,12 @@ type RootKey struct { // RootKeyMeta is the metadata used to refer to a RootKey. type RootKeyMeta struct { - Active bool - KeyID string // UUID - Algorithm EncryptionAlgorithm - EncryptionsCount uint64 - CreateTime time.Time - CreateIndex uint64 - ModifyIndex uint64 + Active bool + KeyID string // UUID + Algorithm EncryptionAlgorithm + CreateTime time.Time + CreateIndex uint64 + ModifyIndex uint64 } // List lists all the keyring metadata diff --git a/api/keyring_test.go b/api/keyring_test.go index 119a40aec..6b6520241 100644 --- a/api/keyring_test.go +++ b/api/keyring_test.go @@ -33,16 +33,14 @@ func TestKeyring_CRUD(t *testing.T) { id := "fd77c376-9785-4c80-8e62-4ec3ab5f8b9a" buf := make([]byte, 32) rand.Read(buf) - encodedKey := make([]byte, base64.StdEncoding.EncodedLen(32)) - base64.StdEncoding.Encode(encodedKey, buf) + encodedKey := base64.StdEncoding.EncodeToString(buf) wm, err = kr.Update(&RootKey{ - Key: string(encodedKey), + Key: encodedKey, Meta: &RootKeyMeta{ - KeyID: id, - Active: true, - Algorithm: EncryptionAlgorithmAES256GCM, - EncryptionsCount: 100, + KeyID: id, + Active: true, + Algorithm: EncryptionAlgorithmAES256GCM, }}, nil) require.NoError(t, err) assertWriteMeta(t, wm) diff --git a/command/agent/keyring_endpoint.go b/command/agent/keyring_endpoint.go index 52395e213..ecbdd991a 100644 --- a/command/agent/keyring_endpoint.go +++ b/command/agent/keyring_endpoint.go @@ -68,8 +68,6 @@ func (s *HTTPServer) keyringRotateRequest(resp http.ResponseWriter, req *http.Re switch query.Get("algo") { case string(structs.EncryptionAlgorithmAES256GCM): args.Algorithm = structs.EncryptionAlgorithmAES256GCM - case string(structs.EncryptionAlgorithmXChaCha20): - args.Algorithm = structs.EncryptionAlgorithmXChaCha20 } if _, ok := query["full"]; ok { @@ -106,10 +104,9 @@ func (s *HTTPServer) keyringUpsertRequest(resp http.ResponseWriter, req *http.Re RootKey: &structs.RootKey{ Key: decodedKey, Meta: &structs.RootKeyMeta{ - Active: key.Meta.Active, - KeyID: key.Meta.KeyID, - Algorithm: structs.EncryptionAlgorithm(key.Meta.Algorithm), - EncryptionsCount: key.Meta.EncryptionsCount, + Active: key.Meta.Active, + KeyID: key.Meta.KeyID, + Algorithm: structs.EncryptionAlgorithm(key.Meta.Algorithm), }, }, } diff --git a/command/agent/keyring_endpoint_test.go b/command/agent/keyring_endpoint_test.go index a79a6bc0b..ef2ed6af2 100644 --- a/command/agent/keyring_endpoint_test.go +++ b/command/agent/keyring_endpoint_test.go @@ -55,19 +55,17 @@ func TestHTTP_Keyring_CRUD(t *testing.T) { keyMeta := rotateResp.Key keyBuf := make([]byte, 32) rand.Read(keyBuf) - encodedKey := make([]byte, base64.StdEncoding.EncodedLen(32)) - base64.StdEncoding.Encode(encodedKey, keyBuf) + encodedKey := base64.StdEncoding.EncodeToString(keyBuf) newID2 := uuid.Generate() key := &api.RootKey{ Meta: &api.RootKeyMeta{ - Active: true, - KeyID: newID2, - Algorithm: api.EncryptionAlgorithm(keyMeta.Algorithm), - EncryptionsCount: 500, + Active: true, + KeyID: newID2, + Algorithm: api.EncryptionAlgorithm(keyMeta.Algorithm), }, - Key: string(encodedKey), + Key: encodedKey, } reqBuf := encodeReq(key) diff --git a/nomad/encrypter.go b/nomad/encrypter.go index 6c41c437b..eb7d36191 100644 --- a/nomad/encrypter.go +++ b/nomad/encrypter.go @@ -14,7 +14,6 @@ import ( "sync" "github.com/hashicorp/go-msgpack/codec" - "golang.org/x/crypto/chacha20poly1305" "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/nomad/structs" @@ -132,7 +131,6 @@ func (e *Encrypter) addCipher(rootKey *structs.RootKey) error { return fmt.Errorf("missing metadata") } var aead cipher.AEAD - var err error switch rootKey.Meta.Algorithm { case structs.EncryptionAlgorithmAES256GCM: @@ -144,11 +142,6 @@ func (e *Encrypter) addCipher(rootKey *structs.RootKey) error { if err != nil { return fmt.Errorf("could not create cipher: %v", err) } - case structs.EncryptionAlgorithmXChaCha20: - aead, err = chacha20poly1305.NewX(rootKey.Key) - if err != nil { - return fmt.Errorf("could not create cipher: %v", err) - } default: return fmt.Errorf("invalid algorithm %s", rootKey.Meta.Algorithm) } @@ -226,19 +219,7 @@ func (e *Encrypter) loadKeyFromStore(path string) (*structs.RootKey, error) { return nil, err } - // Note: we expect to have null bytes for padding, but we don't - // want to use RawStdEncoding which breaks a lot of command line - // tools. So we'll truncate the key to the expected length. - var keyLen int - switch storedKey.Meta.Algorithm { - case structs.EncryptionAlgorithmXChaCha20, structs.EncryptionAlgorithmAES256GCM: - keyLen = 32 - default: - return nil, fmt.Errorf("invalid algorithm") - } - - key := make([]byte, keyLen) - _, err = base64.StdEncoding.Decode(key, []byte(storedKey.Key)[:keyLen]) + key, err := base64.StdEncoding.DecodeString(storedKey.Key) if err != nil { return nil, fmt.Errorf("could not decode key: %v", err) } diff --git a/nomad/encrypter_test.go b/nomad/encrypter_test.go index f9f62517a..b25bc5123 100644 --- a/nomad/encrypter_test.go +++ b/nomad/encrypter_test.go @@ -22,7 +22,6 @@ func TestEncrypter_LoadSave(t *testing.T) { algos := []structs.EncryptionAlgorithm{ structs.EncryptionAlgorithmAES256GCM, - structs.EncryptionAlgorithmXChaCha20, } for _, algo := range algos { diff --git a/nomad/keyring_endpoint.go b/nomad/keyring_endpoint.go index 74ea38f00..685c314fd 100644 --- a/nomad/keyring_endpoint.go +++ b/nomad/keyring_endpoint.go @@ -37,8 +37,7 @@ func (k *Keyring) Rotate(args *structs.KeyringRotateRootKeyRequest, reply *struc // TODO: implement full key rotation via a core job } if args.Algorithm == "" { - // TODO: set this default value from server config - args.Algorithm = structs.EncryptionAlgorithmXChaCha20 + args.Algorithm = structs.EncryptionAlgorithmAES256GCM } rootKey, err := structs.NewRootKey(args.Algorithm) diff --git a/nomad/keyring_endpoint_test.go b/nomad/keyring_endpoint_test.go index c6bed763d..4e18cb884 100644 --- a/nomad/keyring_endpoint_test.go +++ b/nomad/keyring_endpoint_test.go @@ -3,6 +3,7 @@ package nomad import ( "sync" "testing" + "time" msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc" "github.com/stretchr/testify/require" @@ -25,7 +26,7 @@ func TestKeyringEndpoint_CRUD(t *testing.T) { // Upsert a new key - key, err := structs.NewRootKey(structs.EncryptionAlgorithmXChaCha20) + key, err := structs.NewRootKey(structs.EncryptionAlgorithmAES256GCM) require.NoError(t, err) id := key.Meta.KeyID key.Meta.Active = true @@ -54,7 +55,7 @@ func TestKeyringEndpoint_CRUD(t *testing.T) { err = msgpackrpc.CallWithCodec(codec, "Keyring.Get", getReq, &getResp) require.NoError(t, err) require.Equal(t, updateResp.Index, getResp.Index) - require.Equal(t, structs.EncryptionAlgorithmXChaCha20, getResp.Key.Meta.Algorithm) + require.Equal(t, structs.EncryptionAlgorithmAES256GCM, getResp.Key.Meta.Algorithm) // Make a blocking query for List and wait for an Update. Note // that List/Get queries don't need ACL tokens in the test server @@ -77,7 +78,8 @@ func TestKeyringEndpoint_CRUD(t *testing.T) { require.NoError(t, err) }() - updateReq.RootKey.Meta.EncryptionsCount++ + updateReq.RootKey.Meta.CreateTime = time.Now() + err = msgpackrpc.CallWithCodec(codec, "Keyring.Update", updateReq, &updateResp) require.NoError(t, err) require.NotEqual(t, uint64(0), updateResp.Index) @@ -133,7 +135,7 @@ func TestKeyringEndpoint_InvalidUpdates(t *testing.T) { codec := rpcClient(t, srv) // Setup an existing key - key, err := structs.NewRootKey(structs.EncryptionAlgorithmXChaCha20) + key, err := structs.NewRootKey(structs.EncryptionAlgorithmAES256GCM) require.NoError(t, err) id := key.Meta.KeyID key.Meta.Active = true @@ -168,7 +170,7 @@ func TestKeyringEndpoint_InvalidUpdates(t *testing.T) { { key: &structs.RootKey{Meta: &structs.RootKeyMeta{ KeyID: id, - Algorithm: structs.EncryptionAlgorithmXChaCha20, + Algorithm: structs.EncryptionAlgorithmAES256GCM, }}, expectedErrMsg: "root key material is required", }, @@ -177,7 +179,7 @@ func TestKeyringEndpoint_InvalidUpdates(t *testing.T) { Key: []byte{0x01}, Meta: &structs.RootKeyMeta{ KeyID: id, - Algorithm: structs.EncryptionAlgorithmAES256GCM, + Algorithm: "whatever", }}, expectedErrMsg: "root key algorithm cannot be changed after a key is created", }, @@ -213,7 +215,7 @@ func TestKeyringEndpoint_Rotate(t *testing.T) { codec := rpcClient(t, srv) // Setup an existing key - key, err := structs.NewRootKey(structs.EncryptionAlgorithmXChaCha20) + key, err := structs.NewRootKey(structs.EncryptionAlgorithmAES256GCM) require.NoError(t, err) key.Meta.Active = true diff --git a/nomad/leader.go b/nomad/leader.go index 85e6338f3..27a4b188f 100644 --- a/nomad/leader.go +++ b/nomad/leader.go @@ -1703,8 +1703,7 @@ func (s *Server) initializeKeyring() error { s.logger.Named("core").Trace("initializing keyring") - // TODO: algorithm should be set from config - rootKey, err := structs.NewRootKey(structs.EncryptionAlgorithmXChaCha20) + rootKey, err := structs.NewRootKey(structs.EncryptionAlgorithmAES256GCM) rootKey.Meta.Active = true if err != nil { return fmt.Errorf("could not initialize keyring: %v", err) diff --git a/nomad/state/state_store.go b/nomad/state/state_store.go index bb8301c6b..23850eb44 100644 --- a/nomad/state/state_store.go +++ b/nomad/state/state_store.go @@ -6659,11 +6659,6 @@ func (s *StateStore) UpsertRootKeyMeta(index uint64, rootKeyMeta *structs.RootKe existing := raw.(*structs.RootKeyMeta) rootKeyMeta.CreateIndex = existing.CreateIndex rootKeyMeta.CreateTime = existing.CreateTime - - // prevent resetting the encryptions count - if existing.EncryptionsCount > rootKeyMeta.EncryptionsCount { - rootKeyMeta.EncryptionsCount = existing.EncryptionsCount - } isRotation = !existing.Active && rootKeyMeta.Active } else { rootKeyMeta.CreateIndex = index diff --git a/nomad/structs/extensions.go b/nomad/structs/extensions.go index a2ffbc2e0..fcda97927 100644 --- a/nomad/structs/extensions.go +++ b/nomad/structs/extensions.go @@ -87,15 +87,13 @@ func csiVolumeExt(v interface{}) interface{} { // to misuse. func rootKeyExt(v interface{}) interface{} { key := v.(*RootKey) - - encodedKey := make([]byte, base64.StdEncoding.EncodedLen(len(key.Key))) - base64.StdEncoding.Encode(encodedKey, key.Key) + encodedKey := base64.StdEncoding.EncodeToString(key.Key) return &struct { Meta *RootKeyMetaStub Key string }{ Meta: key.Meta.Stub(), - Key: string(encodedKey), + Key: encodedKey, } } diff --git a/nomad/structs/secure_variables.go b/nomad/structs/secure_variables.go index 893686b9c..83b75a7bb 100644 --- a/nomad/structs/secure_variables.go +++ b/nomad/structs/secure_variables.go @@ -9,8 +9,6 @@ import ( // accidentally swaps it out for math/rand via running goimports cryptorand "crypto/rand" - "golang.org/x/crypto/chacha20poly1305" - "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/helper/uuid" ) @@ -234,13 +232,6 @@ func NewRootKey(algorithm EncryptionAlgorithm) (*RootKey, error) { return nil, err } rootKey.Key = key - - case EncryptionAlgorithmXChaCha20: - key := make([]byte, chacha20poly1305.KeySize) - if _, err := cryptorand.Read(key); err != nil { - return nil, err - } - rootKey.Key = key } return rootKey, nil @@ -249,29 +240,27 @@ func NewRootKey(algorithm EncryptionAlgorithm) (*RootKey, error) { // RootKeyMeta is the metadata used to refer to a RootKey. It is // stored in raft. type RootKeyMeta struct { - Active bool - KeyID string // UUID - Algorithm EncryptionAlgorithm - EncryptionsCount uint64 - CreateTime time.Time - CreateIndex uint64 - ModifyIndex uint64 + Active bool + KeyID string // UUID + Algorithm EncryptionAlgorithm + CreateTime time.Time + CreateIndex uint64 + ModifyIndex uint64 } // NewRootKeyMeta returns a new RootKeyMeta with default values func NewRootKeyMeta() *RootKeyMeta { return &RootKeyMeta{ KeyID: uuid.Generate(), - Algorithm: EncryptionAlgorithmXChaCha20, + Algorithm: EncryptionAlgorithmAES256GCM, CreateTime: time.Now(), } } // RootKeyMetaStub is for serializing root key metadata to the // keystore, not for the List API. It excludes frequently-changing -// fields such as EncryptionsCount or ModifyIndex so we don't have to -// sync them to the on-disk keystore when the fields are already in -// raft. +// fields such as ModifyIndex so we don't have to sync them to the +// on-disk keystore when the fields are already in raft. type RootKeyMetaStub struct { KeyID string Algorithm EncryptionAlgorithm @@ -317,7 +306,6 @@ func (rkm *RootKeyMeta) Validate() error { type EncryptionAlgorithm string const ( - EncryptionAlgorithmXChaCha20 EncryptionAlgorithm = "xchacha20" EncryptionAlgorithmAES256GCM EncryptionAlgorithm = "aes256-gcm" )