diff --git a/.changelog/19350.txt b/.changelog/19350.txt new file mode 100644 index 00000000000..34a53e82a81 --- /dev/null +++ b/.changelog/19350.txt @@ -0,0 +1,3 @@ +```release-note:bug +keyring: Fixed a bug where RSA keys were not replicated to followers +``` diff --git a/nomad/encrypter.go b/nomad/encrypter.go index 6d397cd3619..9264612f74d 100644 --- a/nomad/encrypter.go +++ b/nomad/encrypter.go @@ -116,7 +116,7 @@ func (e *Encrypter) loadKeystore() error { return fmt.Errorf("root key ID %s must match key file %s", key.Meta.KeyID, path) } - err = e.AddKey(key) + err = e.addCipher(key) if err != nil { return fmt.Errorf("could not add key file %s to keystore: %w", path, err) } @@ -338,15 +338,15 @@ func (e *Encrypter) addCipher(rootKey *structs.RootKey) error { } // GetKey retrieves the key material by ID from the keyring -func (e *Encrypter) GetKey(keyID string) ([]byte, error) { +func (e *Encrypter) GetKey(keyID string) ([]byte, []byte, error) { e.lock.RLock() defer e.lock.RUnlock() keyset, err := e.keysetByIDLocked(keyID) if err != nil { - return nil, err + return nil, nil, err } - return keyset.rootKey.Key, nil + return keyset.rootKey.Key, keyset.rootKey.RSAKey, nil } // activeKeySetLocked returns the keyset that belongs to the key marked as @@ -582,7 +582,7 @@ func (krr *KeyringReplicator) run(ctx context.Context) { } keyMeta := raw.(*structs.RootKeyMeta) - if key, err := krr.encrypter.GetKey(keyMeta.KeyID); err == nil && len(key) > 0 { + if key, _, err := krr.encrypter.GetKey(keyMeta.KeyID); err == nil && len(key) > 0 { // the key material is immutable so if we've already got it // we can move on to the next key continue diff --git a/nomad/encrypter_test.go b/nomad/encrypter_test.go index 1f18509bb08..3992f8bb681 100644 --- a/nomad/encrypter_test.go +++ b/nomad/encrypter_test.go @@ -16,6 +16,7 @@ import ( "github.com/go-jose/go-jose/v3/jwt" msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc" + "github.com/shoenig/test" "github.com/shoenig/test/must" "github.com/stretchr/testify/require" @@ -56,7 +57,7 @@ func TestEncrypter_LoadSave(t *testing.T) { tmpDir := t.TempDir() encrypter, err := NewEncrypter(srv, tmpDir) - require.NoError(t, err) + must.NoError(t, err) algos := []structs.EncryptionAlgorithm{ structs.EncryptionAlgorithmAES256GCM, @@ -65,13 +66,21 @@ func TestEncrypter_LoadSave(t *testing.T) { for _, algo := range algos { t.Run(string(algo), func(t *testing.T) { key, err := structs.NewRootKey(algo) - require.NoError(t, err) - require.NoError(t, encrypter.saveKeyToStore(key)) + must.Greater(t, 0, len(key.RSAKey)) + must.NoError(t, err) + must.NoError(t, encrypter.saveKeyToStore(key)) + // startup code path gotKey, err := encrypter.loadKeyFromStore( filepath.Join(tmpDir, key.Meta.KeyID+".nks.json")) - require.NoError(t, err) - require.NoError(t, encrypter.addCipher(gotKey)) + must.NoError(t, err) + must.NoError(t, encrypter.addCipher(gotKey)) + must.Greater(t, 0, len(gotKey.RSAKey)) + must.NoError(t, encrypter.saveKeyToStore(key)) + + active, err := encrypter.keysetByIDLocked(key.Meta.KeyID) + must.NoError(t, err) + must.Greater(t, 0, len(active.rootKey.RSAKey)) }) } } @@ -122,8 +131,17 @@ func TestEncrypter_Restore(t *testing.T) { var rotateResp structs.KeyringRotateRootKeyResponse for i := 0; i < 4; i++ { err := msgpackrpc.CallWithCodec(codec, "Keyring.Rotate", rotateReq, &rotateResp) - require.NoError(t, err) + must.NoError(t, err) + } + + // Ensure all rotated keys are correct + srv.encrypter.lock.Lock() + test.MapLen(t, 5, srv.encrypter.keyring) + for _, keyset := range srv.encrypter.keyring { + test.Len(t, 32, keyset.rootKey.Key) + test.Greater(t, 0, len(keyset.rootKey.RSAKey)) } + srv.encrypter.lock.Unlock() shutdown() @@ -146,6 +164,14 @@ func TestEncrypter_Restore(t *testing.T) { return len(listResp.Keys) == 5 // 4 new + the bootstrap key }, time.Second*5, time.Second, "expected keyring to be restored") + srv.encrypter.lock.Lock() + test.MapLen(t, 5, srv.encrypter.keyring) + for _, keyset := range srv.encrypter.keyring { + test.Len(t, 32, keyset.rootKey.Key) + test.Greater(t, 0, len(keyset.rootKey.RSAKey)) + } + srv.encrypter.lock.Unlock() + for _, keyMeta := range listResp.Keys { getReq := &structs.KeyringGetRootKeyRequest{ @@ -157,10 +183,11 @@ func TestEncrypter_Restore(t *testing.T) { } var getResp structs.KeyringGetRootKeyResponse err := msgpackrpc.CallWithCodec(codec, "Keyring.Get", getReq, &getResp) - require.NoError(t, err) + must.NoError(t, err) gotKey := getResp.Key - require.Len(t, gotKey.Key, 32) + must.Len(t, 32, gotKey.Key) + test.Greater(t, 0, len(gotKey.RSAKey)) } } diff --git a/nomad/keyring_endpoint.go b/nomad/keyring_endpoint.go index b3869799ec5..34aaeb1e2d4 100644 --- a/nomad/keyring_endpoint.go +++ b/nomad/keyring_endpoint.go @@ -268,13 +268,14 @@ func (k *Keyring) Get(args *structs.KeyringGetRootKeyRequest, reply *structs.Key } // retrieve the key material from the keyring - key, err := k.encrypter.GetKey(keyMeta.KeyID) + key, rsaKey, err := k.encrypter.GetKey(keyMeta.KeyID) if err != nil { return err } rootKey := &structs.RootKey{ - Meta: keyMeta, - Key: key, + Meta: keyMeta, + Key: key, + RSAKey: rsaKey, } reply.Key = rootKey