From 0a2b4556c55eda27536ee563f60bcf5d69379479 Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Fri, 4 Oct 2024 15:12:48 +0400 Subject: [PATCH] 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 --- go.mod | 2 +- go.sum | 4 +- internal/integration/api/volumes.go | 4 +- internal/pkg/encryption/encryption.go | 96 +++++++++++++++------------ 4 files changed, 59 insertions(+), 47 deletions(-) diff --git a/go.mod b/go.mod index 761f4103b4..e021019941 100644 --- 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 index c5a6dcdc87..8d8a873687 100644 --- 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 index b6531158ba..ce80894a97 100644 --- 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 index 75729697be..47e51f90da 100644 --- 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,7 +313,8 @@ 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)) @@ -310,10 +322,10 @@ func (h *Handler) tryHandlers(ctx context.Context, logger *zap.Logger, cb func(c 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) {