rbd: Add timeout for cryptsetup commands

This PR modifies the execCryptSetupCommand so that
the process is killed in an event of lock timeout.

Useful in cases where the volume lock is released but
the command is still running.

Signed-off-by: Niraj Yadav <niryadav@redhat.com>

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/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 (
-const cryptsetupPBKDFMemoryLimit = 32 << 10 // 32768 KiB
+	// 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) {
+func (l *luksWrapper) Format(devicePath, passphrase string) (string, string, error) {
-	return execCryptsetupCommand(
+	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) {
+func (l *luksWrapper) Resize(mapperFile string) (string, string, error) {
-	return execCryptsetupCommand(nil, "resize", mapperFile)
+	return l.execCryptsetupCommand(nil, "resize", mapperFile)
 }
 
 // LuksClose removes existing mapping.
-func LuksClose(mapperFile string) (string, string, error) {
+func (l *luksWrapper) Close(mapperFile string) (string, string, error) {
-	return execCryptsetupCommand(nil, "luksClose", mapperFile)
+	return l.execCryptsetupCommand(nil, "luksClose", mapperFile)
 }
 
 // LuksStatus returns encryption status of a provided device.
-func LuksStatus(mapperFile string) (string, string, error) {
+func (l *luksWrapper) Status(mapperFile string) (string, string, error) {
-	return execCryptsetupCommand(nil, "status", mapperFile)
+	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,7 +206,7 @@ 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 {
@@ -174,7 +214,7 @@ func LuksVerifyKey(devicePath, passphrase, slot string) (bool, error) {
 	}
 	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.
+		cmd           = exec.CommandContext(l.ctx, program, args...) // #nosec:G204, commands executing not vulnerable.
-		sanitizedArgs = StripSecretInArgs(args)
+		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/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)