rbd: Add timeout for cryptsetup commands

internal/rbd/encryption.go

@@ -26,6 +26,7 @@ import (
 
 	kmsapi "github.com/ceph/ceph-csi/internal/kms"
 	"github.com/ceph/ceph-csi/internal/util"
+	"github.com/ceph/ceph-csi/internal/util/cryptsetup"
 	"github.com/ceph/ceph-csi/internal/util/lock"
 	"github.com/ceph/ceph-csi/internal/util/log"
 
@@ -475,9 +476,15 @@ func (rv *rbdVolume) RotateEncryptionKey(ctx context.Context) error {
 	// Lock params
 	lockName := rv.VolID + "-mutexlock"
 	lockDesc := "Key rotation mutex lock for " + rv.VolID
-	lockDuration := 3 * time.Minute
 	lockCookie := rv.VolID + "-enc-key-rotate"
 
+	// Keep this a little more than ExecutionTimeout to have some buffer
+	// for cleanup. If this lock is a part of some gRPC call, the client
+	// should always timeout after the lockDuration to avoid issues.
+	lockDuration := cryptsetup.ExecutionTimeout + 30*time.Second
+	timedCtx, cancel := context.WithTimeout(ctx, cryptsetup.ExecutionTimeout)
+	defer cancel()
+
 	// Acquire the exclusive lock based on vol id
 	lck := lock.NewLock(rv.ioctx, rv.VolID, lockName, lockCookie, lockDesc, lockDuration)
 	err = lck.LockExclusive(ctx)
@@ -500,8 +507,11 @@ func (rv *rbdVolume) RotateEncryptionKey(ctx context.Context) error {
 		return fmt.Errorf("failed to fetch the current passphrase for %q: %w", rv, err)
 	}
 
+	// Create a new luks wrapper
+	luks := cryptsetup.NewLUKSWrapper(timedCtx)
+
 	// Step 2: Add current key to slot 1
-	err = util.LuksAddKey(devicePath, oldPassphrase, oldPassphrase, luksSlot1)
+	err = luks.AddKey(devicePath, oldPassphrase, oldPassphrase, luksSlot1)
 	if err != nil {
 		return fmt.Errorf("failed to add curr key to luksSlot1: %w", err)
 	}
@@ -513,20 +523,20 @@ func (rv *rbdVolume) RotateEncryptionKey(ctx context.Context) error {
 		return fmt.Errorf("failed to generate a new passphrase: %w", err)
 	}
 
-	err = util.LuksAddKey(devicePath, oldPassphrase, newPassphrase, luksSlot0)
+	err = luks.AddKey(devicePath, oldPassphrase, newPassphrase, luksSlot0)
 	if err != nil {
 		return fmt.Errorf("failed to add the new key to luksSlot0: %w", err)
 	}
 
 	// Step 4: Add the new key to KMS
-	err = rv.blockEncryption.StoreCryptoPassphrase(ctx, rv.VolID, newPassphrase)
+	err = rv.blockEncryption.StoreCryptoPassphrase(timedCtx, rv.VolID, newPassphrase)
 	if err != nil {
 		return fmt.Errorf("failed to update the new key into the KMS: %w", err)
 	}
 
 	// Step 5: Remove the old key from slot 1
 	// We use the newPassphrase to authenticate LUKS here
-	err = util.LuksRemoveKey(devicePath, newPassphrase, luksSlot1)
+	err = luks.RemoveKey(devicePath, newPassphrase, luksSlot1)
 	if err != nil {
 		return fmt.Errorf("failed to remove the backup key from luksSlot1: %w", err)
 	}

internal/util/cephcmds.go

@@ -26,6 +26,7 @@ import (
 	"time"
 
 	"github.com/ceph/ceph-csi/internal/util/log"
+	"github.com/ceph/ceph-csi/internal/util/stripsecrets"
 
 	"github.com/ceph/go-ceph/rados"
 )
@@ -49,7 +50,7 @@ func ExecuteCommandWithNSEnter(ctx context.Context, netPath, program string, arg
 	}
 	//  nsenter --net=%s -- <program> <args>
 	args = append([]string{"--net=" + netPath, "--", program}, args...)
-	sanitizedArgs := StripSecretInArgs(args)
+	sanitizedArgs := stripsecrets.InArgs(args)
 	cmd := exec.Command(nsenter, args...) // #nosec:G204, commands executing not vulnerable.
 	cmd.Stdout = &stdoutBuf
 	cmd.Stderr = &stderrBuf
@@ -80,7 +81,7 @@ func ExecuteCommandWithNSEnter(ctx context.Context, netPath, program string, arg
 func ExecCommand(ctx context.Context, program string, args ...string) (string, string, error) {
 	var (
 		cmd           = exec.Command(program, args...) // #nosec:G204, commands executing not vulnerable.
-		sanitizedArgs = StripSecretInArgs(args)
+		sanitizedArgs = stripsecrets.InArgs(args)
 		stdoutBuf     bytes.Buffer
 		stderrBuf     bytes.Buffer
 	)
@@ -122,7 +123,7 @@ func ExecCommandWithTimeout(
 	error,
 ) {
 	var (
-		sanitizedArgs = StripSecretInArgs(args)
+		sanitizedArgs = stripsecrets.InArgs(args)
 		stdoutBuf     bytes.Buffer
 		stderrBuf     bytes.Buffer
 	)

internal/util/crypto.go

@@ -27,6 +27,7 @@ import (
 	"strings"
 
 	"github.com/ceph/ceph-csi/internal/kms"
+	"github.com/ceph/ceph-csi/internal/util/cryptsetup"
 	"github.com/ceph/ceph-csi/internal/util/log"
 )
 
@@ -49,6 +50,8 @@ var (
 	// DEKStore interface.
 	ErrDEKStoreNeeded = errors.New("DEKStore required, use " +
 		"VolumeEncryption.SetDEKStore()")
+
+	luks = cryptsetup.NewLUKSWrapper(context.Background())
 )
 
 type VolumeEncryption struct {
@@ -264,7 +267,7 @@ func VolumeMapper(volumeID string) (string, string) {
 // EncryptVolume encrypts provided device with LUKS.
 func EncryptVolume(ctx context.Context, devicePath, passphrase string) error {
 	log.DebugLog(ctx, "Encrypting device %q	 with LUKS", devicePath)
-	_, stdErr, err := LuksFormat(devicePath, passphrase)
+	_, stdErr, err := luks.Format(devicePath, passphrase)
 	if err != nil || stdErr != "" {
 		log.ErrorLog(ctx, "failed to encrypt device %q with LUKS (%v): %s", devicePath, err, stdErr)
 	}
@@ -275,7 +278,7 @@ func EncryptVolume(ctx context.Context, devicePath, passphrase string) error {
 // OpenEncryptedVolume opens volume so that it can be used by the client.
 func OpenEncryptedVolume(ctx context.Context, devicePath, mapperFile, passphrase string) error {
 	log.DebugLog(ctx, "Opening device %q with LUKS on %q", devicePath, mapperFile)
-	_, stdErr, err := LuksOpen(devicePath, mapperFile, passphrase)
+	_, stdErr, err := luks.Open(devicePath, mapperFile, passphrase)
 	if err != nil || stdErr != "" {
 		log.ErrorLog(ctx, "failed to open device %q (%v): %s", devicePath, err, stdErr)
 	}
@@ -286,7 +289,7 @@ func OpenEncryptedVolume(ctx context.Context, devicePath, mapperFile, passphrase
 // ResizeEncryptedVolume resizes encrypted volume so that it can be used by the client.
 func ResizeEncryptedVolume(ctx context.Context, mapperFile string) error {
 	log.DebugLog(ctx, "Resizing LUKS device %q", mapperFile)
-	_, stdErr, err := LuksResize(mapperFile)
+	_, stdErr, err := luks.Resize(mapperFile)
 	if err != nil || stdErr != "" {
 		log.ErrorLog(ctx, "failed to resize LUKS device %q (%v): %s", mapperFile, err, stdErr)
 	}
@@ -297,7 +300,7 @@ func ResizeEncryptedVolume(ctx context.Context, mapperFile string) error {
 // CloseEncryptedVolume closes encrypted volume so it can be detached.
 func CloseEncryptedVolume(ctx context.Context, mapperFile string) error {
 	log.DebugLog(ctx, "Closing LUKS device %q", mapperFile)
-	_, stdErr, err := LuksClose(mapperFile)
+	_, stdErr, err := luks.Close(mapperFile)
 	if err != nil || stdErr != "" {
 		log.ErrorLog(ctx, "failed to close LUKS device %q (%v): %s", mapperFile, err, stdErr)
 	}
@@ -320,7 +323,7 @@ func DeviceEncryptionStatus(ctx context.Context, devicePath string) (string, str
 		return devicePath, "", nil
 	}
 	mapPath := strings.TrimPrefix(devicePath, mapperFilePathPrefix+"/")
-	stdout, stdErr, err := LuksStatus(mapPath)
+	stdout, stdErr, err := luks.Status(mapPath)
 	if err != nil || stdErr != "" {
 		log.DebugLog(ctx, "%q is not an active LUKS device (%v): %s", devicePath, err, stdErr)
 

internal/util/cryptsetup.go → internal/util/cryptsetup/cryptsetup.go

@@ -14,26 +14,59 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package util
+package cryptsetup
 
 import (
 	"bytes"
+	"context"
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/ceph/ceph-csi/internal/util/file"
 	"github.com/ceph/ceph-csi/internal/util/log"
+	"github.com/ceph/ceph-csi/internal/util/stripsecrets"
 )
 
-// Limit memory used by Argon2i PBKDF to 32 MiB.
-const cryptsetupPBKDFMemoryLimit = 32 << 10 // 32768 KiB
+const (
+	// Maximum time to wait for cryptsetup commands to complete.
+	ExecutionTimeout = 2*time.Minute + 30*time.Second
+
+	// Limit memory used by Argon2i PBKDF to 32 MiB.
+	pkdbfMemoryLimit = 32 << 10 // 32768 KiB
+)
+
+// LuksWrapper is a struct that provides a context-aware wrapper around cryptsetup commands.
+type LUKSWrapper interface {
+	Format(devicePath, passphrase string) (string, string, error)
+	Open(devicePath, mapperFile, passphrase string) (string, string, error)
+	Close(mapperFile string) (string, string, error)
+	AddKey(devicePath, passphrase, newPassphrase, slot string) error
+	RemoveKey(devicePath, passphrase, slot string) error
+	Resize(mapperFile string) (string, string, error)
+	VerifyKey(devicePath, passphrase, slot string) (bool, error)
+	Status(mapperFile string) (string, string, error)
+}
+
+// luksWrapper is a type that implements LUKSWrapper interface
+// and provides a shared context for its methods.
+type luksWrapper struct {
+	ctx context.Context
+}
+
+// NewLUKSWrapper creates a new LUKSWrapper instance with the provided context.
+// The context is used to control the lifetime of the cryptsetup commands.
+func NewLUKSWrapper(ctx context.Context) LUKSWrapper {
+	return &luksWrapper{ctx: ctx}
+}
 
 // LuksFormat sets up volume as an encrypted LUKS partition.
-func LuksFormat(devicePath, passphrase string) (string, string, error) {
-	return execCryptsetupCommand(
+func (l *luksWrapper) Format(devicePath, passphrase string) (string, string, error) {
+	return l.execCryptsetupCommand(
 		&passphrase,
 		"-q",
 		"luksFormat",
@@ -42,36 +75,43 @@ func LuksFormat(devicePath, passphrase string) (string, string, error) {
 		"--hash",
 		"sha256",
 		"--pbkdf-memory",
-		strconv.Itoa(cryptsetupPBKDFMemoryLimit),
+		strconv.Itoa(pkdbfMemoryLimit),
 		devicePath,
 		"-d",
 		"/dev/stdin")
 }
 
 // LuksOpen opens LUKS encrypted partition and sets up a mapping.
-func LuksOpen(devicePath, mapperFile, passphrase string) (string, string, error) {
+func (l *luksWrapper) Open(devicePath, mapperFile, passphrase string) (string, string, error) {
 	// cryptsetup option --disable-keyring (introduced with cryptsetup v2.0.0)
 	// will be ignored with luks1
-	return execCryptsetupCommand(&passphrase, "luksOpen", devicePath, mapperFile, "--disable-keyring", "-d", "/dev/stdin")
+	return l.execCryptsetupCommand(
+		&passphrase,
+		"luksOpen",
+		devicePath,
+		mapperFile,
+		"--disable-keyring",
+		"-d",
+		"/dev/stdin")
 }
 
 // LuksResize resizes LUKS encrypted partition.
-func LuksResize(mapperFile string) (string, string, error) {
-	return execCryptsetupCommand(nil, "resize", mapperFile)
+func (l *luksWrapper) Resize(mapperFile string) (string, string, error) {
+	return l.execCryptsetupCommand(nil, "resize", mapperFile)
 }
 
 // LuksClose removes existing mapping.
-func LuksClose(mapperFile string) (string, string, error) {
-	return execCryptsetupCommand(nil, "luksClose", mapperFile)
+func (l *luksWrapper) Close(mapperFile string) (string, string, error) {
+	return l.execCryptsetupCommand(nil, "luksClose", mapperFile)
 }
 
 // LuksStatus returns encryption status of a provided device.
-func LuksStatus(mapperFile string) (string, string, error) {
-	return execCryptsetupCommand(nil, "status", mapperFile)
+func (l *luksWrapper) Status(mapperFile string) (string, string, error) {
+	return l.execCryptsetupCommand(nil, "status", mapperFile)
 }
 
 // LuksAddKey adds a new key to the specified slot.
-func LuksAddKey(devicePath, passphrase, newPassphrase, slot string) error {
+func (l *luksWrapper) AddKey(devicePath, passphrase, newPassphrase, slot string) error {
 	passFile, err := file.CreateTempFile("luks-", passphrase)
 	if err != nil {
 		return err
@@ -84,7 +124,7 @@ func LuksAddKey(devicePath, passphrase, newPassphrase, slot string) error {
 	}
 	defer os.Remove(newPassFile.Name())
 
-	_, stderr, err := execCryptsetupCommand(
+	_, stderr, err := l.execCryptsetupCommand(
 		nil,
 		"--verbose",
 		"--key-file="+passFile.Name(),
@@ -107,7 +147,7 @@ func LuksAddKey(devicePath, passphrase, newPassphrase, slot string) error {
 	if strings.Contains(stderr, fmt.Sprintf("Key slot %s is full", slot)) {
 		// The given slot already has a key
 		// Check if it is the one that we want to update with
-		exists, fErr := LuksVerifyKey(devicePath, newPassphrase, slot)
+		exists, fErr := l.VerifyKey(devicePath, newPassphrase, slot)
 		if fErr != nil {
 			return fErr
 		}
@@ -120,13 +160,13 @@ func LuksAddKey(devicePath, passphrase, newPassphrase, slot string) error {
 		// Else, we remove the key from the given slot and add the new one
 		// Note: we use existing passphrase here as we are not yet sure if
 		// the newPassphrase is present in the headers
-		fErr = LuksRemoveKey(devicePath, passphrase, slot)
+		fErr = l.RemoveKey(devicePath, passphrase, slot)
 		if fErr != nil {
 			return fErr
 		}
 
 		// Now the slot is free, add the new key to it
-		fErr = LuksAddKey(devicePath, passphrase, newPassphrase, slot)
+		fErr = l.AddKey(devicePath, passphrase, newPassphrase, slot)
 		if fErr != nil {
 			return fErr
 		}
@@ -140,14 +180,14 @@ func LuksAddKey(devicePath, passphrase, newPassphrase, slot string) error {
 }
 
 // LuksRemoveKey removes the key by killing the specified slot.
-func LuksRemoveKey(devicePath, passphrase, slot string) error {
+func (l *luksWrapper) RemoveKey(devicePath, passphrase, slot string) error {
 	keyFile, err := file.CreateTempFile("luks-", passphrase)
 	if err != nil {
 		return err
 	}
 	defer os.Remove(keyFile.Name())
 
-	_, stderr, err := execCryptsetupCommand(
+	_, stderr, err := l.execCryptsetupCommand(
 		nil,
 		"--verbose",
 		"--key-file="+keyFile.Name(),
@@ -166,15 +206,15 @@ func LuksRemoveKey(devicePath, passphrase, slot string) error {
 }
 
 // LuksVerifyKey verifies that a key exists in a given slot.
-func LuksVerifyKey(devicePath, passphrase, slot string) (bool, error) {
+func (l *luksWrapper) VerifyKey(devicePath, passphrase, slot string) (bool, error) {
 	// Create a temp file that we will use to open the device
 	keyFile, err := file.CreateTempFile("luks-", passphrase)
 	if err != nil {
 		return false, err
 	}
 	defer os.Remove(keyFile.Name())
 
-	_, stderr, err := execCryptsetupCommand(
+	_, stderr, err := l.execCryptsetupCommand(
 		nil,
 		"--verbose",
 		"--key-file="+keyFile.Name(),
@@ -199,11 +239,11 @@ func LuksVerifyKey(devicePath, passphrase, slot string) (bool, error) {
 	return true, nil
 }
 
-func execCryptsetupCommand(stdin *string, args ...string) (string, string, error) {
+func (l *luksWrapper) execCryptsetupCommand(stdin *string, args ...string) (string, string, error) {
 	var (
 		program       = "cryptsetup"
-		cmd           = exec.Command(program, args...) // #nosec:G204, commands executing not vulnerable.
-		sanitizedArgs = StripSecretInArgs(args)
+		cmd           = exec.CommandContext(l.ctx, program, args...) // #nosec:G204, commands executing not vulnerable.
+		sanitizedArgs = stripsecrets.InArgs(args)
 		stdoutBuf     bytes.Buffer
 		stderrBuf     bytes.Buffer
 	)
@@ -217,6 +257,10 @@ func execCryptsetupCommand(stdin *string, args ...string) (string, string, error
 	stdout := stdoutBuf.String()
 	stderr := stderrBuf.String()
 
+	if errors.Is(l.ctx.Err(), context.DeadlineExceeded) {
+		return stdout, stderr, fmt.Errorf("timeout occurred while running %s args: %v", program, sanitizedArgs)
+	}
+
 	if err != nil {
 		return stdout, stderr, fmt.Errorf("an error (%v)"+
 			" occurred while running %s args: %v", err, program, sanitizedArgs)

internal/util/stripsecrets.go → internal/util/stripsecrets/stripsecrets.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package util
+package stripsecrets
 
 import (
 	"strings"
@@ -30,10 +30,10 @@ const (
 	strippedSecret      = "secret=***stripped***"
 )
 
-// StripSecretInArgs strips values of either "--key"/"--keyfile" or "secret=".
+// InArgs strips values of either "--key"/"--keyfile" or "secret=".
 // `args` is left unchanged.
 // Expects only one occurrence of either "--key"/"--keyfile" or "secret=".
-func StripSecretInArgs(args []string) []string {
+func InArgs(args []string) []string {
 	out := make([]string, len(args))
 	copy(out, args)