block: add volume_encryption flag for dm-crypt support #23

vermavis · 2025-04-16T04:55:10Z

This PR introduces host-side encryption support for block volumes using the dm-crypt module. Host encryption is disabled by default and can be enabled by setting the host_encryption parameter in the StorageClass.

When enabled, the NVMe device discovered via NVMe-TCP is encrypted on the client side using dm-crypt.

To use this feature, users must provide a Kubernetes secret referenced in the StorageClass containing a passphrase field.

Adds a `volume_encryption` flag to the storage class configuration. When enabled, this flag ensures that the NVMe device discovered via NVMe-TCP is encrypted using the `dm-crypt` module on the client side. Users must also provide a volume-specific Kubernetes Secret containing the encryption passphrase. This secret must be referenced in the StorageClass using the following parameters: csi.storage.k8s.io/volume-secret-name: <secret-name> csi.storage.k8s.io/volume-secret-namespace: <namespace> Example: kubectl create secret generic volume-secret --from-literal=passphrase='your-passphrase'

vermavis · 2025-04-17T05:48:23Z

@koreno we will look forward to your feedback on this. Please review 🙏

sagigrimberg · 2025-04-21T07:51:49Z

charts/vastblock/templates/storage-class.yaml

@@ -26,6 +26,7 @@

 {{- $tenant_name := pluck "tenantName" $options $.Values.storageClassDefaults | first | quote -}}
 {{- $volume_group := pluck "volumeGroup" $options $.Values.storageClassDefaults | first | quote -}}
+{{- $volume_encryption := pluck "volumeEncryption" $options $.Values.storageClassDefaults | first | quote -}}


need to name this host_encryption, to clarify this is host-side encryption and not storage side encryption

sagigrimberg · 2025-04-21T07:52:31Z

examples/block/sc-vol-encryption.yaml

+  vip_pool_name: vip1
+  transport_type: TCP
+  volume_encryption: "true"
+  csi.storage.k8s.io/volume-secret-name: volume-secret


host-encryption-secret-name

sagigrimberg · 2025-04-21T07:53:16Z

vast_csi/block_utils.py

@@ -30,6 +30,43 @@ def is_native_multipath_enabled():
    except Exception:
        return False

+def is_luks_device(device_path: str) -> bool:


How about moving this to luks_utils.py ?

sagigrimberg · 2025-04-21T07:54:26Z

vast_csi/block_utils.py

@@ -30,6 +30,43 @@ def is_native_multipath_enabled():
    except Exception:
        return False

+def is_luks_device(device_path: str) -> bool:
+    """
+    Check if the given device is a LUKS-encrypted volume.


lets remove the word volume from here

sagigrimberg · 2025-04-21T07:59:15Z

vast_csi/block_utils.py

+        bool: True if the device is using LUKS encryption (FSTYPE is crypto_LUKS), False otherwise.
+    """
+    try:
+        result = subprocess.run(


I'd use hostcmd here, there is a hidden assumption on the existence of utils in the image.

isn't it better to use cryptsetup status <device> ?

sagigrimberg · 2025-04-21T07:59:52Z

vast_csi/plugins/base.py

@@ -68,6 +68,7 @@ def wrapper(self, request, context):
            params = {fld.name: value for fld, value in request.ListFields()}
            # secrets are not logged and not the part of function signature.
            secrets = params.pop("secrets", {})
+            volume_secret = params.pop("volume_secret", {})


s/volume/host/

sagigrimberg · 2025-04-21T08:00:53Z

vast_csi/plugins/block.py

@@ -313,6 +315,7 @@ def ControllerPublishVolume(
        )
        vip_pool_name = volume_context.get("vip_pool_name")
        vip_pool_fqdn = volume_context.get("vip_pool_fqdn")
+        volume_encryption = volume_context.get("volume_encryption")


s/volume/host - same for the rest

sagigrimberg · 2025-04-21T08:03:29Z

vast_csi/plugins/block.py

@@ -479,6 +485,37 @@ def NodeStageVolume(
        device_path = device.DevicePath
        change_io_policy(device_name=device.Name, io_policy="round-robin")

+        if volume_encryption:
+            luks_device_name = f"crypt-{volume_id}"


lets name the volume something more meaningful like vast-csi-crypt-{volume_id}

Lets move it to a helper as other call sites may need to use it

sagigrimberg · 2025-04-21T08:06:29Z

vast_csi/plugins/block.py

+                raise Abort(INVALID_ARGUMENT, "Volume encryption detected, but 'volume_secret' not provided. Please provide a passphrase for the encrypted volume.")
+
+            if not os.path.exists(luks_device_path):
+                is_luks = is_luks_device() and is_luks_crypto()


Not passing device_path ?

sagigrimberg · 2025-04-21T08:07:23Z

vast_csi/plugins/block.py

+                if not is_luks:
+                    logger.info(f"Formatting device {device_path} with LUKS")
+                    try:
+                        hostcmd.run(["cryptsetup", "luksFormat", "--batch-mode", device_path], input=passphrase.encode())


--type=luks2 ?
--hash?
--cipher?
--key-size?

There is a known issue with luks memory usage:
https://gitlab.com/cryptsetup/cryptsetup/-/issues/372

I'd also limit the memory usage with --pbkdf-memory= as advised in the issue. Maybe something like 32/64 MB

sagigrimberg · 2025-04-21T08:14:02Z

vast_csi/plugins/block.py

+
+                logger.info(f"Opening encrypted device {device_path} as {luks_device_name}")
+                try:
+                    hostcmd.run(["cryptsetup", "open", device_path, luks_device_name], input=passphrase.encode())


NodeUnstageVolume will need to close the luks device as well

There are some parameters that may affect performance here.
--perf-same_cpu_crypt
--perf-submit_from_crypt_cpus
--perf-no_read_workqueue
--perf-no_write_workqueue

worth evaluating these

yes we could add performance tuning options as a follow-up to this

sagigrimberg · 2025-04-21T08:14:41Z

vast_csi/plugins/block.py

@@ -479,6 +485,37 @@ def NodeStageVolume(
        device_path = device.DevicePath
        change_io_policy(device_name=device.Name, io_policy="round-robin")

+        if volume_encryption:
+            luks_device_name = f"crypt-{volume_id}"


Lets move it to a helper as other call sites may need to use it

sagigrimberg · 2025-04-21T08:20:00Z

vast_csi/block_utils.py

+        bool: True if the device is using LUKS encryption (FSTYPE is crypto_LUKS), False otherwise.
+    """
+    try:
+        result = subprocess.run(


isn't it better to use cryptsetup status <device> ?

vlboiko · 2025-04-25T14:29:25Z

charts/vastblock/templates/storage-class.yaml

-  subsystem: {{ $subsystem }}
-{{- range $key, $value := dict "vip_pool_name" $vip_pool_name "vip_pool_fqdn" $vip_pool_fqdn "volume_group" $volume_group "transport_type" $transport_type "fsType" $fstype "tenant_name" $tenant_name }}
-  {{- if and $value (ne $value ( quote "" )) }}
-  {{ $key }}: {{ if (kindIs "int" $value) }}{{ $value | quote }}{{ else }}{{ $value }}{{ end }}


If I'm not mistaken this if (kindIs "int" $value)... prevents strings to be double quoted

vlboiko · 2025-04-25T14:34:17Z

charts/vastblock/templates/storage-class.yaml

+  {{- end }}
+  {{- $host_encryption := get $options "hostEncryption" }}
+  {{- if or (not $host_encryption) (eq $host_encryption "false") }}
+  host_encryption: "false"


host_encryption here is simple boo val represented as string. Although in values.yaml it is object with many keys. If I understand correctly it is temporary solution for testing right?
Later it can be json string via toJson func

agreed- a json with encryption related params as keys would be nice. For now, just a way to turn the knob and test out the flow.

vlboiko · 2025-04-25T14:35:00Z

vast_csi/builders/block.py

@@ -56,6 +57,7 @@ def from_parameters(
        vip_pool_fqdn = parameters.get("vip_pool_fqdn")
        vip_pool_name = parameters.get("vip_pool_name")
        volume_group = parameters.get("volume_group", "")
+        host_encryption = "true" if parameters.get("host_encryption", "false").lower() == "true" else "false"


It will be json right?

vlboiko · 2025-04-25T14:39:06Z

vast_csi/plugins/base.py

@@ -68,6 +68,7 @@ def wrapper(self, request, context):
            params = {fld.name: value for fld, value in request.ListFields()}
            # secrets are not logged and not the part of function signature.
            secrets = params.pop("secrets", {})
+            host_encryption_secret = params.pop("host_encryption_secret", {})


As we discussed in slack not possible to provide multiple secrets.
You might want to add passphrase to "main" secrets and take it from "secrets"

passprase = secrets.pop("passprase") ...

vlboiko · 2025-04-25T14:41:23Z

vast_csi/plugins/base.py

        if is_ephemeral:
            payload["vms_session"] = vms_session.serialize(salt=volume_id)
+
+        if is_host_encryption:


if ephemeral or is_host_entryption: ....

I didn't get why "vms_session" should be serialized in is_host_encryption is True?

It is limitation of ephemeral volumes that secrets are not passed via context. So we need to serialize session for deletion of such volumes.

Ah OK, thanks for catching! Will fix.

vlboiko · 2025-04-25T14:48:40Z

vast_csi/builders/block.py

@@ -321,6 +329,8 @@ def volume_context(self) -> dict:
            context["vip_pool_name"] = self.vip_pool_name
        elif self.vip_pool_fqdn:
            context["vip_pool_fqdn"] = self.vip_pool_fqdn_with_prefix
+        if self.host_encryption:


Here, logically if self.host_encryption is always true because
host_encryption can be "true" or "false". Both for python is True strings.

There is helper to convert "true"/"false" to bool
See: https://github.com/vast-data/vast-csi/blob/v2.6/vast_csi/plugins/csi.py#L330

vlboiko · 2025-04-29T11:47:46Z

charts/vastblock/templates/storage-class.yaml

@@ -58,6 +58,13 @@ parameters:
  {{ $key }}: {{ if (kindIs "int" $value) }}{{ $value | quote }}{{ else }}{{ $value }}{{ end }}
  {{- end }}
 {{- end }}
+{{- if $host_encryption }}


Maybe some generic template for parsing nested volumes might be helpful. Eg in
charts/vastblock/templates/shared/_helpers.tpl add:

{{- define "vastcsi.dictToYaml" -}} {{- $input := index . 0 -}} {{/* The map to render */}} {{- $prefix := index . 1 | default "" -}} {{/* Optional prefix for keys */}} {{- if not (kindIs "map" $input) }} {{- $errorMsg := printf "Invalid format. Expected a dictionary but got:\n%s" (toYaml $input) }} {{- fail $errorMsg }} {{- else }} {{- range $k, $v := $input }} {{- if and $v (ne $v "") }} {{ printf "%s%s: %s" $prefix $k ($v | quote) }} {{- end }} {{- end }} {{- end }} {{- end }}

and in storage-class.yaml replace block below with:

{{- include "vastcsi.dictToYaml" (list $host_encryption "host_encryption.") | indent 2 }}

What do you think?

vlboiko · 2025-04-29T11:57:28Z

examples/block/sc-with-host-encryption.yaml

+  subsystem: myblock
+  vip_pool_name: vippool-1
+  transport_type: TCP
+  hostEncryption:


Internally it is converted to host_encryption

vlboiko · 2025-04-29T12:12:55Z

vast_csi/plugins/block.py

@@ -479,6 +486,21 @@ def NodeStageVolume(
        device_path = device.DevicePath
        change_io_policy(device_name=device.Name, io_policy="round-robin")

+        host_encryption_present = any(key.startswith("host_encryption.") for key in volume_context)
+        if host_encryption_present:
+            host_encryption_secret = getattr(exit_stack, "host_encryption_secret", None)


why not create variable passphrase right here and not re-assign it twice?

vlboiko · 2025-04-29T12:16:35Z

vast_csi/plugins/block.py

+            host_encryption_secret = getattr(exit_stack, "host_encryption_secret", None)
+
+            if host_encryption_secret:
+                passphrase = host_encryption_secret


I'm a bit confused about this if/else

passphrase = host_encryption_secret so first "if host_encryption_secret" should do the trick. Why check again "if not passphrase:" <- Seems like this condition never happen

vlboiko · 2025-04-29T13:01:53Z

vast_csi/plugins/block.py

@@ -505,6 +527,7 @@ def NodeUnstageVolume(self, volume_id, staging_target_path):
        staging_target_path = local.path(staging_target_path)
        device_bind_path = get_device_bind_path(staging_target_path)
        staging_mount, target_mounts = MountInfo.get_mounts_by_source(src=device_bind_path)
+        host_encryption = None


Is there any usage of this variable?

sagigrimberg · 2025-04-30T07:47:08Z

vast_csi/plugins/block.py

@@ -494,7 +511,10 @@ def NodeStageVolume(
                with temporary_mount(
                        src=device_path, tgt_dir=staging_target_path, fs_type=fs_type
                ) as temp_mount:
-                    resize_device(device=device_path, target_mount=temp_mount, fs_type=fs_type)
+                    if host_encryption_present:


@vermavis just initialize passphrase to None in the start - and then you don't need any condition here, just call resize_device with passphrase=passphrase that may or may not be None

vlboiko · 2025-05-01T17:18:56Z

vast_csi/filesystem_utils.py

    """Perform resize of the filesystem."""
    if need_resize(device, target_mount, fs_type):
+        if passphrase:
+            luks_manager = LuksManager(logger, device_path=device)
+            luks_manager.luks_resize_device(passphrase)


sagigrimberg · 2025-05-04T08:02:34Z

vast_csi/luks_utils.py

+
+        # Check if already encrypted
+        is_luks = self._is_luks_device()
+        if is_luks and not self._is_luks_active():


Lets arrange this differently:

if not is_luks: // format + open elif not self._is_luks_active(): // open else: // error

sagigrimberg · 2025-05-04T08:05:30Z

vast_csi/luks_utils.py

+            if key.startswith(prefix)
+        }
+
+    def process_host_encryption(self, passphrase: str) -> None:


lets rename this init_host_encryption

sagigrimberg · 2025-05-04T08:06:10Z

vast_csi/luks_utils.py

+        except ProcessExecutionError:
+            return False
+
+    def luks_remove_device(self) -> None:


lets rename this _luks_close_device

Lets wrap that with a public function named fini_host_encryption and have it call private _luks_close_device

sagigrimberg · 2025-05-09T09:49:27Z

vast_csi/plugins/block.py

@@ -631,6 +649,13 @@ def NodeUnpublishVolume(self, volume_id, target_path, vms_session=None):
                volume_id=volume_id,
                vms_session=vms_session,
            )
+            # Check for host_encryption in meta and handle accordingly
+            luks_manager = LuksManager(logger, volume_id)


@vermavis why is this in NodeUnpublishVolume? We create the Luks device in NodeStageVolume, so we need to close the Luks device in NodeUnstageVolume no?

I would do this instead: in NodeUnstageVolume:

diff --git a/vast_csi/plugins/block.py b/vast_csi/plugins/block.py index c85f4c33a25f..9f28a12f17bf 100644 --- a/vast_csi/plugins/block.py +++ b/vast_csi/plugins/block.py @@ -25,7 +25,7 @@ from easypy.caching import cached_property from vast_csi.logging import logger from vast_csi.proto import csi_pb2_grpc as csi_grpc from vast_csi import csi_types as types -from vast_csi.luks_utils import LuksManager +from vast_csi.luks_utils import LuksManager, isLuksPath from vast_csi.csi_types import ( INVALID_ARGUMENT, ALREADY_EXISTS, @@ -535,6 +535,10 @@ class BlockNode(NodeBase, Instrumented): else: logger.info(f"Device not found at {device_bind_path}") remove_path_if_not_mounted(device_bind_path) + # If device is encrypted teardown accordingly + if isHostCryptPath(device_bind_path, volume_id) + with LuksManager(logger, volume_id) as lm: + lm.fini_host_encryption() return types.UnstageResp() def NodePublishVolume(

Where:

diff --git a/vast_csi/luks_utils.py b/vast_csi/luks_utils.py index 967f99b99ce5..6e30e5c87b56 100644 --- a/vast_csi/luks_utils.py +++ b/vast_csi/luks_utils.py @@ -9,14 +9,21 @@ from vast_csi.logging import logger from vast_csi.exceptions import Abort from vast_csi.csi_types import NOT_FOUND +def luksDevicePath(volume_name): + return f"/dev/mapper/{volume_name}" +def luksDeviceName(volume_id): + return f"vast-csi-crypt-{volume_id}" +def isHostCryptPath(device, volume_id): + return device == luksDevicePath(luksDeviceName(volume_id)) + class LuksManager: def __init__(self, logger, vol_id=None, device_path=None, vol_context=None): self.vol_id = vol_id self.encryption_config = self._parse_encryption_config(vol_context or {}) self.logger = logger self.device_path = device_path - self.luks_device_name = f"vast-csi-crypt-{vol_id}" - self.luks_device_path = f"/dev/mapper/{self.luks_device_name}" + self.luks_device_name = luksDeviceName(vol_id) + self.luks_device_path = luksDevicePath(luks_device_name)

sagigrimberg · 2025-05-09T09:58:51Z

vast_csi/luks_utils.py

+
+            hostcmd.test("-e", resolved_device)
+
+            device_type = hostcmd.lsblk("-no", "TYPE", resolved_device).strip()


Why not just test with _is_luks_device ?

If this is not sufficient, I would go with:
blkid <device> -s TYPE -o value

but I think self._is_luks_device will work just fine @vermavis

vermavis force-pushed the vermavis/test-luks branch 5 times, most recently from b976167 to cba368d Compare April 17, 2025 04:49

vermavis force-pushed the vermavis/test-luks branch from cba368d to 4b800a3 Compare April 17, 2025 05:09

koreno requested a review from vlboiko April 17, 2025 10:05

sagigrimberg reviewed Apr 21, 2025

View reviewed changes

sagigrimberg suggested changes Apr 21, 2025

View reviewed changes

vermavis force-pushed the vermavis/test-luks branch 5 times, most recently from c55882f to e5b6e5b Compare April 24, 2025 22:51

vlboiko reviewed Apr 25, 2025

View reviewed changes

vermavis force-pushed the vermavis/test-luks branch from 315d0c6 to f9fe8cf Compare April 28, 2025 21:15

vlboiko reviewed Apr 29, 2025

View reviewed changes

vermavis force-pushed the vermavis/test-luks branch 6 times, most recently from 4d8869c to aa12fce Compare April 29, 2025 23:49

sagigrimberg reviewed Apr 30, 2025

View reviewed changes

vermavis force-pushed the vermavis/test-luks branch 2 times, most recently from 4987220 to 7142d1f Compare April 30, 2025 15:34

vlboiko reviewed May 1, 2025

View reviewed changes

vermavis force-pushed the vermavis/test-luks branch 2 times, most recently from 757bba1 to d47fb7d Compare May 2, 2025 23:17

sagigrimberg reviewed May 4, 2025

View reviewed changes

vermavis force-pushed the vermavis/test-luks branch 5 times, most recently from 59e802d to 618e70c Compare May 8, 2025 17:02

sagigrimberg reviewed May 9, 2025

View reviewed changes

block: address CR feedback

4fb7a27

vermavis force-pushed the vermavis/test-luks branch from 618e70c to 4fb7a27 Compare May 9, 2025 16:42


		hostcmd.test("-e", resolved_device)

		device_type = hostcmd.lsblk("-no", "TYPE", resolved_device).strip()

block: add volume_encryption flag for dm-crypt support #23

Are you sure you want to change the base?

block: add volume_encryption flag for dm-crypt support #23

Conversation

vermavis commented Apr 16, 2025 • edited Loading

vermavis commented Apr 17, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sagigrimberg May 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vermavis commented Apr 16, 2025 •

edited

Loading

sagigrimberg May 4, 2025 •

edited

Loading