fix: volume encryption with failing keyslots

Fix the flow when a failing key slot leads to repeated attempts to open the volume, while it's already open, but the failure was to sync other keys. Refactor the code to get rid of variable assignment in the outer block from closures. Fixes #9415 Signed-off-by: Andrey Smirnov <[email protected]>
siderolabs · Oct 7, 2024 · 0a2b455 · 0a2b455
1 parent 6affbd3
commit 0a2b455
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 47 deletions.
diff --git a/go.mod b/go.mod
@@ -145,7 +145,7 @@ require (
 	github.com/siderolabs/gen v0.5.0
 	github.com/siderolabs/go-api-signature v0.3.6
 	github.com/siderolabs/go-blockdevice v0.4.7
-	github.com/siderolabs/go-blockdevice/v2 v2.0.2
+	github.com/siderolabs/go-blockdevice/v2 v2.0.3
 	github.com/siderolabs/go-circular v0.2.0
 	github.com/siderolabs/go-cmd v0.1.1
 	github.com/siderolabs/go-copy v0.1.0

diff --git a/go.sum b/go.sum
@@ -587,8 +587,8 @@ github.com/siderolabs/go-api-signature v0.3.6 h1:wDIsXbpl7Oa/FXvxB6uz4VL9INA9fmr
 github.com/siderolabs/go-api-signature v0.3.6/go.mod h1:hoH13AfunHflxbXfh+NoploqV13ZTDfQ1mQJWNVSW9U=
 github.com/siderolabs/go-blockdevice v0.4.7 h1:2bk4WpEEflGxjrNwp57ye24Pr+cYgAiAeNMWiQOuWbQ=
 github.com/siderolabs/go-blockdevice v0.4.7/go.mod h1:4PeOuk71pReJj1JQEXDE7kIIQJPVe8a+HZQa+qjxSEA=
-github.com/siderolabs/go-blockdevice/v2 v2.0.2 h1:GIdOBrCLQ7X9jbr0P/+7paw5SIfp/LL+dx9mTOzmw8w=
-github.com/siderolabs/go-blockdevice/v2 v2.0.2/go.mod h1:74htzCV913UzaLZ4H+NBXkwWlYnBJIq5m/379ZEcu8w=
+github.com/siderolabs/go-blockdevice/v2 v2.0.3 h1:IEgDqd3H3gPphahrdvfAzU8RmD4r5eQdWC+vgFQQoEg=
+github.com/siderolabs/go-blockdevice/v2 v2.0.3/go.mod h1:74htzCV913UzaLZ4H+NBXkwWlYnBJIq5m/379ZEcu8w=
 github.com/siderolabs/go-circular v0.2.0 h1:Xca8zrjF/YsujLbwDSojkKzJe7ngetnpuIJn8N78DJI=
 github.com/siderolabs/go-circular v0.2.0/go.mod h1:rrYCwHLYWmxqrmZP+LjYtwB2a55lxzQi0Ztu1VpWZSc=
 github.com/siderolabs/go-cmd v0.1.1 h1:nTouZUSxLeiiEe7hFexSVvaTsY/3O8k1s08BxPRrsps=

diff --git a/internal/integration/api/volumes.go b/internal/integration/api/volumes.go
@@ -296,8 +296,8 @@ func (suite *VolumesSuite) lvmVolumeExists(node string) bool {
 	}
 
 	// we test with creating a volume group with two logical volumes
-	// one mirrored and one not, so we expect to see 6 volumes
-	return lvmVolumeCount == 6
+	// one mirrored and one not, so we expect to see at least 6 volumes
+	return lvmVolumeCount >= 6
 }
 
 func init() {

diff --git a/internal/pkg/encryption/encryption.go b/internal/pkg/encryption/encryption.go
@@ -88,34 +88,42 @@ type Handler struct {
 
 // Open encrypted partition.
 func (h *Handler) Open(ctx context.Context, logger *zap.Logger, devicePath, encryptedName string) (string, error) {
-	var (
-		path string
-		key  *encryption.Key
-	)
+	isOpen, path, err := h.encryptionProvider.IsOpen(ctx, devicePath, encryptedName)
+	if err != nil {
+		return "", err
+	}
 
-	if err := h.tryHandlers(ctx, logger, func(ctx context.Context, handler keys.Handler) error {
-		token, err := h.readToken(ctx, devicePath, handler.Slot())
-		if err != nil {
-			return err
-		}
+	var usedKey *encryption.Key
 
-		if key, err = handler.GetKey(ctx, token); err != nil {
-			return err
+	if !isOpen {
+		handler, key, _, err := h.tryHandlers(ctx, logger, func(ctx context.Context, handler keys.Handler) (*encryption.Key, token.Token, error) {
+			slotToken, err := h.readToken(ctx, devicePath, handler.Slot())
+			if err != nil {
+				return nil, nil, err
+			}
+
+			slotKey, err := handler.GetKey(ctx, slotToken)
+			if err != nil {
+				return nil, nil, err
+			}
+
+			return slotKey, slotToken, nil
+		})
+		if err != nil {
+			return "", err
 		}
 
 		path, err = h.encryptionProvider.Open(ctx, devicePath, encryptedName, key)
 		if err != nil {
-			return err
+			return "", err
 		}
 
 		logger.Info("opened encrypted device", zap.Int("slot", handler.Slot()), zap.String("type", fmt.Sprintf("%T", handler)))
 
-		return nil
-	}); err != nil {
-		return "", fmt.Errorf("failed to open encrypted device %s: %w", devicePath, err)
+		usedKey = key
 	}
 
-	if err := h.syncKeys(ctx, logger, devicePath, key); err != nil {
+	if err := h.syncKeys(ctx, logger, devicePath, usedKey); err != nil {
 		return "", err
 	}
 
@@ -125,6 +133,10 @@ func (h *Handler) Open(ctx context.Context, logger *zap.Logger, devicePath, encr
 // Close encrypted partition.
 func (h *Handler) Close(ctx context.Context, encryptedPath string) error {
 	if err := h.encryptionProvider.Close(ctx, encryptedPath); err != nil {
+		if errors.Is(err, encryption.ErrDeviceNotReady) {
+			return nil
+		}
+
 		return fmt.Errorf("error closing %s: %w", encryptedPath, err)
 	}
 
@@ -133,23 +145,10 @@ func (h *Handler) Close(ctx context.Context, encryptedPath string) error {
 
 // FormatAndEncrypt formats and encrypts the volume.
 func (h *Handler) FormatAndEncrypt(ctx context.Context, logger *zap.Logger, path string) error {
-	if len(h.keyHandlers) == 0 {
-		return errors.New("no encryption keys found")
-	}
-
-	var (
-		key   *encryption.Key
-		token token.Token
-		err   error
-	)
-
-	if err = h.tryHandlers(ctx, logger, func(ctx context.Context, h keys.Handler) error {
-		if key, token, err = h.NewKey(ctx); err != nil {
-			return err
-		}
-
-		return nil
-	}); err != nil {
+	_, key, token, err := h.tryHandlers(ctx, logger, func(ctx context.Context, h keys.Handler) (*encryption.Key, token.Token, error) {
+		return h.NewKey(ctx)
+	})
+	if err != nil {
 		return err
 	}
 
@@ -189,21 +188,21 @@ func (h *Handler) syncKeys(ctx context.Context, logger *zap.Logger, path string,
 		slot := strconv.Itoa(handler.Slot())
 		visited[slot] = true
 		// no need to update the key which we already detected as unchanged
-		if k.Slot == handler.Slot() {
+		if k != nil && k.Slot == handler.Slot() {
 			continue
 		}
 
 		// keyslot exists
 		if _, ok := keyslots.Keyslots[slot]; ok {
 			if err = h.updateKey(ctx, path, k, handler); err != nil {
-				return err
+				return fmt.Errorf("error updating key slot %s %T: %w", slot, handler, err)
 			}
 
 			logger.Info("updated encryption key", zap.Int("slot", handler.Slot()))
 		} else {
 			// keyslot does not exist so just add the key
 			if err = h.addKey(ctx, path, k, handler); err != nil {
-				return err
+				return fmt.Errorf("error adding key slot %s %T: %w", slot, handler, err)
 			}
 
 			logger.Info("added encryption key", zap.Int("slot", handler.Slot()))
@@ -219,7 +218,7 @@ func (h *Handler) syncKeys(ctx context.Context, logger *zap.Logger, path string,
 			}
 
 			if err = h.encryptionProvider.RemoveKey(ctx, path, int(s), k); err != nil {
-				return err
+				return fmt.Errorf("error removing key slot %s: %w", slot, err)
 			}
 
 			logger.Info("removed encryption key", zap.Int("slot", k.Slot))
@@ -291,8 +290,20 @@ func (h *Handler) addKey(ctx context.Context, path string, existingKey *encrypti
 	return nil
 }
 
-func (h *Handler) tryHandlers(ctx context.Context, logger *zap.Logger, cb func(ctx context.Context, h keys.Handler) error) error {
-	callback := func(ctx context.Context, h keys.Handler) error {
+// tryHandlers tries to get encryption keys from all available handlers.
+//
+// It returns the first handler that successfully returns a key.
+func (h *Handler) tryHandlers(
+	ctx context.Context, logger *zap.Logger,
+	cb func(ctx context.Context, h keys.Handler) (*encryption.Key, token.Token, error),
+) (
+	keys.Handler, *encryption.Key, token.Token, error,
+) {
+	if len(h.keyHandlers) == 0 {
+		return nil, nil, nil, errors.New("no encryption keys found")
+	}
+
+	callback := func(ctx context.Context, h keys.Handler) (*encryption.Key, token.Token, error) {
 		ctx, cancel := context.WithTimeout(ctx, keyHandlerTimeout)
 		defer cancel()
 
@@ -302,18 +313,19 @@ func (h *Handler) tryHandlers(ctx context.Context, logger *zap.Logger, cb func(c
 	var errs error
 
 	for _, h := range h.keyHandlers {
-		if err := callback(ctx, h); err != nil {
+		key, token, err := callback(ctx, h)
+		if err != nil {
 			errs = multierror.Append(errs, err)
 
 			logger.Warn("failed to call key handler", zap.Int("slot", h.Slot()), zap.Error(err))
 
 			continue
 		}
 
-		return nil
+		return h, key, token, nil
 	}
 
-	return fmt.Errorf("no handlers available to get encryption keys from: %w", errs)
+	return nil, nil, nil, fmt.Errorf("no handlers available to get encryption keys from: %w", errs)
 }
 
 func (h *Handler) readToken(ctx context.Context, path string, id int) (token.Token, error) {