keyring: replicate RSA private key via GetKey RPC (#19350)

When we added a RSA key for signing Workload Identities, we added it to the
keystore serialization but did not also add it to the `GetKey` RPC. This means
that when a key is rotated, the RSA key will not come along. The Nomad leader
signs all Workload Identities, but external consumers of WI (like Consul or
Vault) will verify the WI against any of the servers. If the request to verify
hits a follower, the follower will not have the RSA private key and cannot use
the existing ed25519 key to verify WIs with the `RS256` algorithm.

Add the RSA key material to the `GetKey` RPC.

Also remove an extraneous write to disk that happens for each key each time we
restart the Nomad server.

Fixes: #19340

.changelog/19350.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+keyring: Fixed a bug where RSA keys were not replicated to followers
+```

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

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))
 	}
 }
 

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