encrypter: Ignore wrapped key additions with zero wrapped keys. (#25791)

When a Nomad server restores its state via a snapshot and logs, it
is possible a legacy wrapped key object/log is found. This key
will not contain any wrapped keys and therefore should be ingored
within the encrypter.

It is theoretically possible without this change that a key which
generates zero decrypt tasks supersedes a running task and will
place itself in the tracked decrypt task tracker. This decrypt
task has no running work to remove its entry.

Author: jrasell

nomad/encrypter.go

@@ -368,6 +368,18 @@ func (e *Encrypter) AddUnwrappedKey(rootKey *structs.UnwrappedRootKey, isUpgrade
 // but it returns an error for ease of testing.
 func (e *Encrypter) AddWrappedKey(ctx context.Context, wrappedKeys *structs.RootKey) error {
 
+	// If the passed root key does not contain any wrapped keys, it has no
+	// decryption tasks to perform and therefore should not supersede or impact
+	// tasks running for the same keyID.
+	//
+	// This conditional will only be hit when the FSM is taking action on
+	// fsm.RootKeyMetaSnapshot and structs.RootKeyMetaUpsertRequestType types
+	// which represent legacy style keys. When support is removed for these in
+	// 1.12.0, we should reconsider this conditional too.
+	if len(wrappedKeys.WrappedKeys) == 0 {
+		return nil
+	}
+
 	logger := e.log.With("key_id", wrappedKeys.KeyID)
 
 	e.lock.Lock()
@@ -387,7 +399,7 @@ func (e *Encrypter) AddWrappedKey(ctx context.Context, wrappedKeys *structs.Root
 
 	if cancel, ok := e.decryptTasks[wrappedKeys.KeyID]; ok {
 		// stop any previous tasks for this same key ID under the assumption
-		// they're broken or being superceded, but don't remove the CancelFunc
+		// they're broken or being superseded, but don't remove the CancelFunc
 		// from the map yet so that other callers don't think we can continue
 		cancel()
 	}

nomad/encrypter_test.go

@@ -874,3 +874,39 @@ func TestEncrypter_decryptWrappedKeyTask(t *testing.T) {
 	err = encrypter.decryptWrappedKeyTask(ctx, cancel, KMSWrapper, provider, key.Meta, wrappedKey)
 	must.NoError(t, err)
 }
+
+func TestEncrypter_AddWrappedKey_noWrappedKeys(t *testing.T) {
+	ci.Parallel(t)
+
+	srv := &Server{
+		logger: testlog.HCLogger(t),
+		config: &Config{},
+	}
+
+	encrypter, err := NewEncrypter(srv, t.TempDir())
+	must.NoError(t, err)
+
+	// Fake life as a 1.6 server by writing only ed25519 keys and removing the
+	// RSAKey. Add this to the encrypter as if we are loading it as part of the
+	// FSM restore process which actions the trailing logs.
+	oldKey, err := structs.NewUnwrappedRootKey(structs.EncryptionAlgorithmAES256GCM)
+	must.NoError(t, err)
+
+	oldKey.RSAKey = nil
+	wrappedOldKey := structs.NewRootKey(oldKey.Meta)
+
+	shutdownCtx, shutdownCancel := context.WithCancel(context.Background())
+	t.Cleanup(shutdownCancel)
+
+	// Add the wrapped key and then wait for the encrypter to become ready. If
+	// it reaches the timeout, it means the encrypter has added a decryption
+	// task to its internal tracking without launching any decryptWrappedKeyTask
+	// routines that can remove this task entry.
+	must.NoError(t, encrypter.AddWrappedKey(shutdownCtx, wrappedOldKey))
+
+	timeoutCtx, timeoutCancel := context.WithTimeout(shutdownCtx, 3*time.Second)
+	t.Cleanup(timeoutCancel)
+
+	must.NoError(t, encrypter.IsReady(timeoutCtx))
+	must.MapLen(t, 0, encrypter.decryptTasks)
+}