fix(luks)!: read the master passphrase from the Secret only

docs/layer-stack.md

@@ -63,8 +63,8 @@ curl -XPOST http://localhost:3370/v1/resource-definitions \
 ### Per-RG via spawn template
 
 ```bash
+linstor encryption create-passphrase --passphrase '<32-byte-secret>'
 linstor rg create encrypted-rg
-linstor rg sp encrypted-rg DrbdOptions/Encryption/passphrase '<32-byte-secret>'
 linstor rg c --place-count 2 encrypted-rg
 linstor rg spawn encrypted-rg pvc-2 1G --layer-list LUKS DRBD
 ```
@@ -74,50 +74,124 @@ the REST shape mirrors upstream LINSTOR.)
 
 ### Per-RD via kubectl
 
+The cluster passphrase Secret has to exist first; `linstor encryption
+create-passphrase` writes it, or apply it yourself (GitOps):
+
 ```yaml
+apiVersion: v1
+kind: Secret
+metadata:
+  name: blockstor-cluster-passphrase   # or ControllerConfig spec.passphraseSecretRef.name
+  namespace: blockstor-system          # the controller's namespace
+type: Opaque
+stringData:
+  passphrase: "32-byte-secret"
+---
 apiVersion: blockstor.cozystack.io/v1alpha1
 kind: ResourceDefinition
 metadata: {name: pvc-encrypted}
 spec:
   layerStack: ["LUKS", "STORAGE"]
-  props:
-    DrbdOptions/Encryption/passphrase: "32-byte-secret"
   volumeDefinitions:
     - {volumeNumber: 0, sizeKib: 1048576}
 ```
 
 ## LUKS specifics
 
-- Passphrase is per-RD via `DrbdOptions/Encryption/passphrase`. The
-  upstream `linstor rd set-property` key.
-- The dispatcher folds the resolver-resolved passphrase onto the wire
-  as `DesiredResource.Props["LuksPassphrase"]`; the satellite reads it
-  from there.
-- Empty passphrase with `LUKS` in the stack fails the apply rather
-  than silently producing an unencrypted volume.
+- The passphrase is the cluster master passphrase, and it lives only in
+  the Secret `linstor encryption create-passphrase` writes
+  (`blockstor-cluster-passphrase` in the controller's namespace, unless
+  ControllerConfig `spec.passphraseSecretRef` names another). Creating
+  an RD with `LUKS` in its stack is refused until that Secret exists.
+- A passphrase is never read from a property. The REST API answers 400
+  to any write that would store `DrbdOptions/EncryptPassphrase`,
+  `DrbdOptions/Encryption/passphrase` or `LuksPassphrase` (any case, on
+  any object), and so does the bundled CLI's `set-property`: a property
+  sits in plaintext on its CR, so any dump of the CRs would carry the
+  key to every LUKS volume. Deleting such a property is still allowed.
+  This differs from upstream LINSTOR, where `linstor controller
+  set-property DrbdOptions/EncryptPassphrase` is accepted.
+- The satellite reads the Secret, drops any passphrase property that
+  reached a definition's props (one put there with kubectl, say), and
+  hands the Secret's value to the dispatcher, which puts it on the wire
+  as `DesiredResource.Props["LuksPassphrase"]` for the LUKS layer.
+- No passphrase with `LUKS` in the stack fails the apply rather than
+  silently producing an unencrypted volume.
 - Mapper name: `<rd>-<vol>-luks` → `/dev/mapper/<rd>-<vol>-luks`. Stable
   across reconciles so reopen on satellite restart re-uses it.
 - Volume grow: the satellite runs `cryptsetup resize` on the mapper
   after the storage layer has resized the underlying LV, before DRBD
   resizes the replicated device.
 
+### Upgrading a cluster that kept the passphrase in a property
+
+Before this release a passphrase property won over the Secret: `DrbdOptions/EncryptPassphrase` on the controller, or `DrbdOptions/Encryption/passphrase` on a group or a definition. LUKS volumes created while one was set were formatted with its value. From this release on the satellite opens them with the Secret's value only, and logs `ignoring LUKS passphrase properties` with the keys it dropped. Do this before the upgrade.
+
+1. Find the properties. Their values are redacted on the REST API, so read the CRs. The store keeps a `DrbdOptions/*` key in `spec.extraProps`, not in `spec.props`, and a volume's own props, where `vd modify-passphrase` wrote `DrbdOptions/Encrypt/Passphrase`, in `spec.volumeDefinitions[].props` of its definition, and a volume group's in `spec.volumeGroups[].props` of its group. Snapshots taken of a definition that carried the alias copied it into their own `spec.props`. So read all of them on groups, definitions, replicas and snapshots, and the controller's own:
+
+   ```sh
+   kubectl get controllerconfigs.blockstor.cozystack.io -o jsonpath='{.items[*].spec.extraProps}'
+   kubectl get resourcegroups.blockstor.cozystack.io,resourcedefinitions.blockstor.cozystack.io,resources.blockstor.cozystack.io,snapshots.blockstor.cozystack.io -o json \
+     | jq -r '.items[] | select([(.spec.props // {}), (.spec.extraProps // {}), (.spec.volumeDefinitions // [] | .[].props // {}), (.spec.volumeGroups // [] | .[].props // {})] | add | keys | any(test("passphrase"; "i"))) | "\(.kind)/\(.metadata.name)"'
+   ```
+
+2. Make the Secret open every LUKS volume:
+   - **A controller property and no Secret**: run `linstor encryption create-passphrase` with the property's exact value.
+   - **A controller property set from the start, and a Secret with another value**: no LUKS volume was formatted with the Secret's value, so overwrite the Secret with the property's value. The API refuses this (create answers 409 because a passphrase exists, modify answers 409 because LUKS volumes exist), so do it with kubectl:
+
+     ```sh
+     kubectl -n blockstor-system create secret generic blockstor-cluster-passphrase \
+       --from-literal=passphrase='<the property value>' --dry-run=client -o yaml | kubectl apply -f -
+     ```
+
+     The Secret is `blockstor-cluster-passphrase` in the controller's namespace unless the ControllerConfig's `spec.passphraseSecretRef` names another; use that name, and the controller's namespace, if you changed either.
+
+   - **Per-group, per-definition or per-replica values, or a Secret that some volumes were formatted with**: the volumes use different keys, so no single Secret value opens them all. Add the Secret's value to each LUKS device that was formatted with another key, on every node with a replica, with `cryptsetup luksAddKey <device>`; it asks for the key the device has now.
+
+3. Delete every key step 1 found, on the object it was found on, so the plaintext value leaves the CRs and the next backup. The controller can hold the canonical key and the alias both:
+
+   ```sh
+   linstor controller set-property DrbdOptions/EncryptPassphrase ""
+   linstor controller set-property DrbdOptions/Encryption/passphrase ""
+   linstor resource-group set-property <group> <key> ""
+   linstor resource-definition set-property <definition> <key> ""
+   linstor resource set-property <node> <resource> <key> ""
+   linstor volume-definition set-property <definition> <volume number> <key> ""
+   linstor volume-group set-property <group> <volume number> <key> ""
+   ```
+
+   A snapshot has no set-property. Remove the key from its CR, with `/` in the key written as `~1`, or delete the snapshot if it is no longer needed:
+
+   ```sh
+   kubectl patch snapshots.blockstor.cozystack.io <snapshot> --type json \
+     -p '[{"op":"remove","path":"/spec/props/DrbdOptions~1Encryption~1passphrase"}]'
+   ```
+
+   Groups, their volume groups and definitions no longer pass such a key on to what is cloned, spawned, snapshotted or restored from them, but the copies made before the upgrade carry it, so step 1 finds those too.
+
+A volume missed in step 2 fails to open on the next satellite restart or mapper close; the satellite log line names the property it was formatted with.
+
 ## What blockstor doesn't yet support
 
 - **Pluggable layer ordering**: the stack must be one of the four
   rows in the table above. Arbitrary orderings (e.g. `["LUKS","DRBD","STORAGE"]`
   with LUKS-over-DRBD instead of DRBD-over-LUKS) aren't yet validated
   or rendered.
-- **Per-volume LUKS keys**: every volume on a 2-volume RD currently
-  uses the same RD-level passphrase. Per-volume keys (with master-key
-  wrapping in the controller's KV store) is a follow-up.
+- **Per-volume LUKS keys**: every LUKS volume in the cluster uses the
+  cluster master passphrase. `linstor vd modify-passphrase` answers 400:
+  it used to store the value in the volume definition's properties,
+  in plaintext, where nothing read it. Per-volume keys (with master-key
+  wrapping) are a follow-up.
 - **Mid-stack changes**: editing `layerStack` after a Resource is
   active doesn't re-encrypt or unwrap existing data. Treat it as
   set-once-per-RD until migration support lands.
-- **Cluster-passphrase rotation**: the cluster passphrase
-  (`POST /v1/encryption/passphrase`) wraps RD passphrases in the
-  controller's KV store; rotating it doesn't re-encrypt LUKS headers.
-  Rotating the per-RD passphrase requires `cryptsetup luksChangeKey`
-  which isn't wired yet — operators should drop the RD and recreate.
+- **Cluster-passphrase rotation**: the cluster passphrase is the LUKS
+  key itself, not a wrapping key. `linstor encryption
+  modify-passphrase` (`PUT /v1/encryption/passphrase`) rewrites the
+  Secret, and nothing runs `cryptsetup luksChangeKey` on the existing
+  headers, so volumes formatted with the old value would stop opening.
+  It answers 409 while any definition has LUKS in its stack, naming
+  one of them; rotate only once there are none.
 
 ## Testing
 

docs/linstor-migration.md

@@ -10,7 +10,7 @@ Migrated: nodes, storage pools, resource groups, resource definitions (+ volume
 
 **NOT migrated — read the report on stderr for the exact list per cluster:**
 
-- **LUKS passphrases.** LUKS-encrypted volumes convert with their layer stack intact (an RD whose `layer_stack` column is empty takes the stack its replicas were built with, read from `LAYER_RESOURCE_IDS`), but the per-volume passphrase (encrypted with the LINSTOR master key) is not decrypted. You must provision `spec.encryption` for those RDs by hand before the encrypted volumes can be opened. Each affected RD is reported.
+- **LUKS passphrases.** LUKS-encrypted volumes convert with their layer stack intact (an RD whose `layer_stack` column is empty takes the stack its replicas were built with, read from `LAYER_RESOURCE_IDS`), but the per-volume key (encrypted with the LINSTOR master key) is not decrypted. blockstor opens every LUKS volume with the cluster passphrase kept in its Secret, so add that passphrase to each such volume as a key slot before adopting it: pre-flight step 7 says how. Each affected RD is reported.
 - **Definitions whose replicas disagree on the layer stack** are skipped with their replicas and reported: replicas built two ways, or with their layers in another order, under a definition with no stack of its own; a diskful replica the dump shows no layer chain for, under such a definition; or a replica built otherwise than the stack the definition names. Adopted as they are, such a replica would be brought up under a stack it was not built with, DRBD metadata over a LUKS volume's ciphertext for one. Settle the stack in LINSTOR and convert again.
 - **Backup-shipping remotes**, unknown/runtime flag bits, and DELETE'd / FAILED_DEPLOYMENT rows are skipped and reported. None affect live data availability.
 - **DRBD TCP ports** are absent from most LINSTOR 1.33 dumps — see "Capture live DRBD ports" below.
@@ -68,13 +68,32 @@ Migrated: nodes, storage pools, resource groups, resource definitions (+ volume
 
 6. **Rehearse on staging first.** Do a full dry-run adoption of a COPY of the pools (or a representative subset) on a non-production cluster: apply the manifests, confirm every StoragePool registers a provider (no `provider requires … in props` errors in the satellite log), every Resource reaches `UpToDate` with `out-of-sync=0` (adopted, not resynced), and no new empty zvols appear. Only after a clean staging rehearsal proceed to production.
 
+7. **Give every LUKS volume the blockstor cluster passphrase, while LINSTOR still has it open.** LINSTOR formats each LUKS volume with a key of its own, kept in its database encrypted with the master passphrase, and the converter does not decrypt it. blockstor opens every LUKS volume with the cluster passphrase in its Secret, so that passphrase has to be a key slot on each volume before adoption. Choose the cluster passphrase now, and put that exact value into blockstor's passphrase Secret before adoption (`linstor encryption create-passphrase` against blockstor, or the Secret by hand). Then, on a node where LINSTOR has the volume open:
+
+   ```bash
+   dmsetup ls --target crypt                    # find the mapper of the volume
+   dmsetup table --showkeys <mapper>            # field 5 is the volume key
+   ```
+
+   If field 5 is a hex string, it is the volume key itself. Write it as binary to a file on tmpfs and add the cluster passphrase with it; `luksAddKey` asks for the new passphrase:
+
+   ```bash
+   dmsetup table --showkeys <mapper> | awk '{print $5}' | xxd -r -p > /run/vk.bin
+   cryptsetup luksAddKey --volume-key-file /run/vk.bin <backing device>
+   shred -u /run/vk.bin
+   ```
+
+   The key slot lives in the LUKS header on that replica's backing device, so do this on every node with a diskful replica of the volume. Then check it, for each replica: `cryptsetup open --test-passphrase <backing device>` with the cluster passphrase.
+
+   If field 5 starts with `:` (`:64:logon:cryptsetup:...`), the volume was opened through the kernel keyring, which cryptsetup 2 does for LUKS2 by default, and the key cannot be read back from userspace. Such a volume cannot be given the passphrase this way: copy its data to a new volume created under blockstor instead, or keep it on LINSTOR. The volume key is the secret that decrypts the volume, so treat the file and the shell history like the passphrase itself.
+
 ## Convert
 
 ```bash
 linstor-migrate -in ./dump -drbd-ports ./drbd-ports.txt -out ./blockstor-manifests.yaml
 ```
 
-Read the report on stderr end to end. Every `warning:` line is a thing the converter refused to guess — decide per line whether it matters for your data before continuing. In particular resolve every `LUKS passphrase NOT migrated` and every `no DRBD port` line (the latter should be empty if the port capture was complete).
+Read the report on stderr end to end. Every `warning:` line is a thing the converter refused to guess — decide per line whether it matters for your data before continuing. In particular resolve every `LUKS key NOT migrated` and every `no DRBD port` line (the latter should be empty if the port capture was complete).
 
 ## Cutover
 
@@ -104,7 +123,7 @@ The manifests do not migrate a cluster by themselves — the control plane must
 - Spot-check ≥5 RDs: size, per-volume `/dev/drbd<minor>`, per-replica node-id and port match the pre-migration values. A changed minor or node-id is a stop-the-line defect.
 - Every diskful replica is `UpToDate`; no replica is unexpectedly `SyncTarget`.
 - Adopted snapshots list as present without any suspend-io having fired on the parent RDs (check the satellites did not log `suspend-io`).
-- For LUKS RDs: their volumes stay closed until you provision `spec.encryption`; confirm consumers of those volumes are quiesced until then.
+- For LUKS RDs: their volumes stay closed until the cluster passphrase is added to them as a key slot; confirm consumers of those volumes are quiesced until then.
 
 ## Rollback
 
@@ -114,7 +133,7 @@ Until CSI is repointed and confirmed, rollback is: scale blockstor to 0, scale `
 
 | Item | Impact | Mitigation |
 | --- | --- | --- |
-| LUKS passphrases not migrated | Encrypted volumes cannot be opened until `spec.encryption` is provisioned | Provision by hand; keep those consumers quiesced during migration |
+| LUKS keys not migrated | Encrypted volumes cannot be opened until the cluster passphrase is a key slot on them | Add it with `cryptsetup luksAddKey`; keep those consumers quiesced during migration |
 | DRBD port not in dump (LINSTOR 1.33) | Adoption reconnects the mesh on a new port if not captured | Capture with the pre-flight command and pass `-drbd-ports` |
 | Unknown flag bits dropped | Cosmetic runtime bookkeeping only | Reported; verified not to carry availability semantics |
 | Placement/data unchanged | The converter never moves data | Rebalance with blockstor after migration if desired |

internal/cli/encryption.go

@@ -317,20 +317,5 @@ func checkLUKSPrerequisite(ctx context.Context, run *runContext, layers []string
 		return nil
 	}
 
-	// The Secret is the primary mechanism, but a cluster provisioned
-	// before it existed carries the passphrase in the legacy controller
-	// prop, and the satellite still unlocks with it. REST accepts both;
-	// checking only the Secret makes the CLI refuse a stack that the
-	// REST door on the same cluster creates without complaint.
-	props, err := run.Store.ControllerProps().Get(ctx)
-	if err != nil {
-		return fmt.Errorf("read controller properties to check the %s prerequisite: %w",
-			apiv1.LayerKindLUKS, err)
-	}
-
-	if props[passphrase.PropKeyCanonical] != "" {
-		return nil
-	}
-
 	return fmt.Errorf("layer list includes %s: %w", apiv1.LayerKindLUKS, errLUKSWithoutPassphrase)
 }

internal/cli/luks_writers_test.go

@@ -5,6 +5,7 @@ package cli_test
 import (
 	"bytes"
 	"context"
+	"strings"
 	"testing"
 
 	corev1 "k8s.io/api/core/v1"
@@ -90,29 +91,41 @@ func TestLUKSPrerequisiteHoldsOnEveryLayerListWriter(t *testing.T) {
 	}
 }
 
-// The Secret is the primary mechanism, but a cluster provisioned before it
-// existed carries the passphrase in the legacy controller prop and the
-// satellite still unlocks with it. REST accepts both sources; a CLI that
-// reads only the Secret refuses on such a cluster what the REST door on the
-// same cluster accepts.
-func TestLUKSPrerequisiteAcceptsTheLegacyControllerProp(t *testing.T) {
+// The passphrase lives only in the Secret `encryption create-passphrase`
+// writes; nothing reads it from a property. A controller prop that carries
+// one (put there with kubectl, since REST refuses to store it) must not
+// satisfy the prerequisite, and the refusal must point at the Secret flow,
+// not at the property.
+func TestLUKSPrerequisiteIgnoresTheControllerProp(t *testing.T) {
 	t.Parallel()
 
 	app, backend, errBuf := newKubeAppWithStore(t)
 
 	err := backend.ControllerProps().PatchProps(t.Context(), func(props map[string]string) error {
-		props[passphrase.PropKeyCanonical] = "from-a-pre-Secret-cluster"
+		props[passphrase.PropKeyCanonical] = "plaintext-in-a-prop"
 
 		return nil
 	})
 	if err != nil {
-		t.Fatalf("seed the legacy prop: %v", err)
+		t.Fatalf("seed the controller prop: %v", err)
 	}
 
 	stack := apiv1.LayerKindDRBD + "," + apiv1.LayerKindLUKS + "," + apiv1.LayerKindStorage
 
-	if got := app.Run(t.Context(), []string{"rd", "c", "pvc-x", "--layer-list", stack}); got != 0 {
-		t.Errorf("exit = %d: the cluster has a passphrase in the legacy prop, which REST "+
-			"accepts (stderr: %s)", got, errBuf.String())
+	if got := app.Run(t.Context(), []string{"rd", "c", "pvc-x", "--layer-list", stack}); got == 0 {
+		t.Fatalf("exit 0: a passphrase in a controller prop satisfied the LUKS prerequisite "+
+			"(stderr: %s)", errBuf.String())
+	}
+
+	if !strings.Contains(errBuf.String(), "encryption create-passphrase") {
+		t.Errorf("the refusal does not point at `encryption create-passphrase`: %s", errBuf.String())
+	}
+
+	if strings.Contains(errBuf.String(), passphrase.PropKeyCanonical) {
+		t.Errorf("the refusal names the property, which is never read: %s", errBuf.String())
+	}
+
+	if _, err := backend.ResourceDefinitions().Get(t.Context(), "pvc-x"); err == nil {
+		t.Error("the refused create wrote the definition")
 	}
 }