diff --git a/internal/volumes/cryptsetup.go b/internal/volumes/cryptsetup.go index 0ed29842..25894c96 100644 --- a/internal/volumes/cryptsetup.go +++ b/internal/volumes/cryptsetup.go @@ -5,11 +5,13 @@ import ( "errors" "fmt" "log/slog" + "os" "os/exec" "strings" + "syscall" ) -const cryptsetupExecuable = "cryptsetup" +const cryptsetupExecutable = "cryptsetup" type CryptSetup struct { logger *slog.Logger @@ -19,15 +21,16 @@ func NewCryptSetup(logger *slog.Logger) *CryptSetup { return &CryptSetup{logger: logger} } -func (cs *CryptSetup) IsActive(ctx context.Context, luksDeviceName string) (bool, error) { - output, code, err := command(ctx, cryptsetupExecuable, "status", luksDeviceName) +func (cs *CryptSetup) Status(ctx context.Context, luksDeviceName string) (LUKSDeviceStatus, error) { + output, code, err := cryptsetup(ctx, "status", luksDeviceName) if err != nil { if code == 4 { - return false, nil + return LUKSDeviceStatus{Active: false}, nil } - return false, fmt.Errorf("unable to check LUKS device %s activity: %s", luksDeviceName, output) + return LUKSDeviceStatus{Active: false}, fmt.Errorf("unable to check LUKS device %s status: %w", luksDeviceName, err) } - return true, nil + + return NewLUKSDeviceStatus(output), nil } func (cs *CryptSetup) Format(ctx context.Context, devicePath string, passphrase string) error { @@ -35,7 +38,7 @@ func (cs *CryptSetup) Format(ctx context.Context, devicePath string, passphrase "formatting LUKS device", "devicePath", devicePath, ) - output, _, err := commandWithStdin(ctx, passphrase, cryptsetupExecuable, "luksFormat", "--type", "luks1", devicePath) + output, _, err := cryptsetupWithStdin(ctx, passphrase, "luksFormat", "--type", "luks1", devicePath) if err != nil { return fmt.Errorf("unable to format device %s with LUKS: %s", devicePath, output) } @@ -43,19 +46,45 @@ func (cs *CryptSetup) Format(ctx context.Context, devicePath string, passphrase } func (cs *CryptSetup) Open(ctx context.Context, devicePath string, luksDeviceName string, passphrase string) error { - active, err := cs.IsActive(ctx, luksDeviceName) + status, err := cs.Status(ctx, luksDeviceName) if err != nil { return err } - if active { - return nil + if status.Active { + if sameDevice(status.Device, devicePath) { + cs.logger.Debug( + "LUKS device already active", + "devicePath", devicePath, + "luksDeviceName", luksDeviceName, + ) + return nil + } + + // A volume detached without being unpublished leaves behind a mapper whose backing + // device is gone. Reusing it would format and mount a dead device. + cs.logger.Warn( + "LUKS device is not backed by the expected device, closing it", + "devicePath", devicePath, + "luksDeviceName", luksDeviceName, + "backingDevice", status.Device, + ) + if err := cs.Close(ctx, luksDeviceName); err != nil { + return fmt.Errorf( + "LUKS device %s is backed by %s instead of %s: %w", + luksDeviceName, + status.Device, + devicePath, + err, + ) + } } + cs.logger.Info( "opening LUKS device", "devicePath", devicePath, "luksDeviceName", luksDeviceName, ) - output, _, err := commandWithStdin(ctx, passphrase, cryptsetupExecuable, "luksOpen", "--allow-discards", devicePath, luksDeviceName) + output, _, err := cryptsetupWithStdin(ctx, passphrase, "luksOpen", "--allow-discards", devicePath, luksDeviceName) if err != nil { return fmt.Errorf("unable to open LUKS device %s: %s", devicePath, output) } @@ -63,18 +92,18 @@ func (cs *CryptSetup) Open(ctx context.Context, devicePath string, luksDeviceNam } func (cs *CryptSetup) Close(ctx context.Context, luksDeviceName string) error { - active, err := cs.IsActive(ctx, luksDeviceName) + status, err := cs.Status(ctx, luksDeviceName) if err != nil { return err } - if !active { + if !status.Active { return nil } cs.logger.Info( "closing LUKS device", "luksDeviceName", luksDeviceName, ) - output, _, err := command(ctx, cryptsetupExecuable, "luksClose", luksDeviceName) + output, _, err := cryptsetup(ctx, "luksClose", luksDeviceName) if err != nil { return fmt.Errorf("unable to close LUKS device %s: %s", luksDeviceName, output) } @@ -86,7 +115,7 @@ func (cs *CryptSetup) Resize(ctx context.Context, luksDeviceName string) error { "resizing LUKS device", "luksDeviceName", luksDeviceName, ) - output, _, err := command(ctx, cryptsetupExecuable, "resize", luksDeviceName) + output, _, err := cryptsetup(ctx, "resize", luksDeviceName) if err != nil { return fmt.Errorf("unable to resize LUKS device %s: %s", luksDeviceName, output) } @@ -102,12 +131,12 @@ func GenerateLUKSDevicePath(luksDeviceName string) string { return "/dev/mapper/" + luksDeviceName } -func command(ctx context.Context, name string, args ...string) (string, int, error) { - return commandWithStdin(ctx, "", name, args...) +func cryptsetup(ctx context.Context, args ...string) (string, int, error) { + return cryptsetupWithStdin(ctx, "", args...) } -func commandWithStdin(ctx context.Context, stdin string, name string, args ...string) (string, int, error) { - cmd := exec.CommandContext(ctx, name, args...) +func cryptsetupWithStdin(ctx context.Context, stdin string, args ...string) (string, int, error) { + cmd := exec.CommandContext(ctx, cryptsetupExecutable, args...) if stdin != "" { cmd.Stdin = strings.NewReader(stdin) } @@ -122,3 +151,38 @@ func commandWithStdin(ctx context.Context, stdin string, name string, args ...st } return output, 0, nil } + +type LUKSDeviceStatus struct { + Active bool + Device string +} + +func NewLUKSDeviceStatus(output string) LUKSDeviceStatus { + // An inactive device is reported through the exit code, not the output. + status := LUKSDeviceStatus{Active: true} + for line := range strings.SplitSeq(output, "\n") { + key, value, ok := strings.Cut(strings.TrimSpace(line), ":") + if ok && key == "device" { + status.Device = strings.TrimSpace(value) + } + } + return status +} + +// sameDevice reports whether a and b refer to the same device node. +// +// os.ModeDevice is set for block and character device files only. Both paths +// must be device nodes because Rdev (the major:minor device number) is only +// meaningful for them: for regular files and directories it is 0, so two +// unrelated non-device paths would otherwise compare as equal. +func sameDevice(a string, b string) bool { + infoA, err := os.Stat(a) + if err != nil || infoA.Mode()&os.ModeDevice == 0 { + return false + } + infoB, err := os.Stat(b) + if err != nil || infoB.Mode()&os.ModeDevice == 0 { + return false + } + return infoA.Sys().(*syscall.Stat_t).Rdev == infoB.Sys().(*syscall.Stat_t).Rdev +} diff --git a/internal/volumes/cryptsetup_test.go b/internal/volumes/cryptsetup_test.go new file mode 100644 index 00000000..6bbc8e82 --- /dev/null +++ b/internal/volumes/cryptsetup_test.go @@ -0,0 +1,112 @@ +package volumes + +import ( + "os" + "path/filepath" + "testing" +) + +func TestNewLUKSDeviceStatus(t *testing.T) { + testCases := []struct { + Name string + Output string + Want LUKSDeviceStatus + }{ + { + Name: "healthy device", + Output: `/dev/mapper/pv-1 is active and is in use. + type: LUKS1 + cipher: aes-xts-plain64 + keysize: 512 bits + device: /dev/sdb + sector size: 512 + offset: 4096 sectors + size: 2093056 sectors + mode: read/write`, + Want: LUKSDeviceStatus{Active: true, Device: "/dev/sdb"}, + }, + { + Name: "backing device disappeared", + Output: `/dev/mapper/pv-1 is active. + type: n/a + cipher: aes-xts-plain64 + keysize: 512 bits + device: (null) + mode: read/write`, + Want: LUKSDeviceStatus{Active: true, Device: "(null)"}, + }, + { + Name: "device line missing", + Output: `/dev/mapper/pv-1 is active. + type: LUKS1 + cipher: aes-xts-plain64`, + Want: LUKSDeviceStatus{Active: true}, + }, + { + Name: "empty output", + Output: "", + Want: LUKSDeviceStatus{Active: true}, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.Name, func(t *testing.T) { + if got := NewLUKSDeviceStatus(testCase.Output); got != testCase.Want { + t.Errorf("NewLUKSDeviceStatus() = %+v, want %+v", got, testCase.Want) + } + }) + } +} + +func TestSameDevice(t *testing.T) { + regularFile := filepath.Join(t.TempDir(), "file") + if err := os.WriteFile(regularFile, nil, 0o600); err != nil { + t.Fatal(err) + } + + testCases := []struct { + Name string + A string + B string + Want bool + }{ + { + Name: "same device", + A: "/dev/null", + B: "/dev/null", + Want: true, + }, + { + Name: "different devices", + A: "/dev/null", + B: "/dev/zero", + Want: false, + }, + { + Name: "backing device disappeared", + A: "(null)", + B: "/dev/null", + Want: false, + }, + { + Name: "backing device missing", + A: "", + B: "/dev/null", + Want: false, + }, + { + Name: "regular files", + A: regularFile, + B: regularFile, + Want: false, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.Name, func(t *testing.T) { + if got := sameDevice(testCase.A, testCase.B); got != testCase.Want { + t.Errorf("sameDevice(%q, %q) = %v, want %v", testCase.A, testCase.B, got, testCase.Want) + } + }) + } +} diff --git a/internal/volumes/resize.go b/internal/volumes/resize.go index 2af14bc7..eaf70d7f 100644 --- a/internal/volumes/resize.go +++ b/internal/volumes/resize.go @@ -46,11 +46,11 @@ func (l *LinuxResizeService) Resize(ctx context.Context, volumePath string, volu ) luksDeviceName := GenerateLUKSDeviceName(devicePath) - active, err := l.cryptSetup.IsActive(ctx, luksDeviceName) + status, err := l.cryptSetup.Status(ctx, luksDeviceName) if err != nil { return err } - if active { + if status.Active { luksDevicePath := GenerateLUKSDevicePath(luksDeviceName) if err := l.cryptSetup.Resize(ctx, luksDeviceName); err != nil { return err diff --git a/test/integration/cryptsetup_test.go b/test/integration/cryptsetup_test.go index 36a2440b..fa366566 100644 --- a/test/integration/cryptsetup_test.go +++ b/test/integration/cryptsetup_test.go @@ -5,6 +5,7 @@ package integration import ( "log/slog" "os" + "strings" "testing" "github.com/hetznercloud/csi-driver/internal/volumes" @@ -55,3 +56,112 @@ func TestCryptSetup(t *testing.T) { t.Fatal(err) } } + +func TestStaleLUKSMapperIsReplaced(t *testing.T) { + if !runTestInDockerImage(t, true) { + return + } + + ctx := t.Context() + logger := slog.New(slog.NewTextHandler(os.Stdout, nil)) + cryptSetup := volumes.NewCryptSetup(logger) + passphrase := "passphrase" + luksDeviceName := "stale" + + staleDevice := createLoopDevice(t, "stale-previous") + currentDevice := createLoopDevice(t, "stale-current") + for _, device := range []string{staleDevice, currentDevice} { + if err := cryptSetup.Format(ctx, device, passphrase); err != nil { + t.Fatal(err) + } + } + + if err := cryptSetup.Open(ctx, staleDevice, luksDeviceName, passphrase); err != nil { + t.Fatal(err) + } + defer runCmd("cryptsetup", "luksClose", luksDeviceName) + + for range 2 { + if err := cryptSetup.Open(ctx, currentDevice, luksDeviceName, passphrase); err != nil { + t.Fatal(err) + } + + status, err := cryptSetup.Status(ctx, luksDeviceName) + if err != nil { + t.Fatal(err) + } + if !status.Active || status.Device != currentDevice { + t.Fatalf("expected %s to be backed by %s, got %+v", luksDeviceName, currentDevice, status) + } + } +} + +func TestStaleLUKSMapperInUseIsKept(t *testing.T) { + if !runTestInDockerImage(t, true) { + return + } + + ctx := t.Context() + logger := slog.New(slog.NewTextHandler(os.Stdout, nil)) + cryptSetup := volumes.NewCryptSetup(logger) + passphrase := "passphrase" + luksDeviceName := "stale-in-use" + + staleDevice := createLoopDevice(t, "stale-in-use-previous") + currentDevice := createLoopDevice(t, "stale-in-use-current") + for _, device := range []string{staleDevice, currentDevice} { + if err := cryptSetup.Format(ctx, device, passphrase); err != nil { + t.Fatal(err) + } + } + + if err := cryptSetup.Open(ctx, staleDevice, luksDeviceName, passphrase); err != nil { + t.Fatal(err) + } + defer runCmd("cryptsetup", "luksClose", luksDeviceName) + + luksDevicePath := volumes.GenerateLUKSDevicePath(luksDeviceName) + if _, err := runCmd("mkfs.ext4", luksDevicePath); err != nil { + t.Fatal(err) + } + mountPath := t.TempDir() + if _, err := runCmd("mount", "-t", "ext4", luksDevicePath, mountPath); err != nil { + t.Fatal(err) + } + defer runCmd("umount", mountPath) + + err := cryptSetup.Open(ctx, currentDevice, luksDeviceName, passphrase) + if err == nil { + t.Fatalf("expected opening %s on %s to fail while it is mounted", luksDeviceName, currentDevice) + } + t.Log(err) + + status, err := cryptSetup.Status(ctx, luksDeviceName) + if err != nil { + t.Fatal(err) + } + if !status.Active || status.Device != staleDevice { + t.Fatalf("expected %s to still be backed by %s, got %+v", luksDeviceName, staleDevice, status) + } +} + +func createLoopDevice(t *testing.T, name string) string { + t.Helper() + + file := "/dev-" + name + if _, err := runCmd("dd", "if=/dev/zero", "of="+file, "bs=1M", "count=32"); err != nil { + t.Fatal(err) + } + + output, err := runCmd("losetup", "-f") + if err != nil { + t.Fatal(err) + } + device := strings.TrimSpace(output) + if _, err := runCmd("losetup", device, file); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _, _ = runCmd("losetup", "-d", device) }) + + return device +}