linode / linode-blockstorage-csi-driver

Container Storage Interface (CSI) Driver for Linode Block Storage
Apache License 2.0
64 stars 54 forks source link

Write encryptionKey to crypsetup stdin #159

Closed avestuk closed 6 months ago

avestuk commented 6 months ago

General:

Pull Request Guidelines:

  1. [ ] Does your submission pass tests?
  2. [ ] Have you added tests?
  3. [x] Are you addressing a single feature in this PR?
  4. [x] Are your commits atomic, addressing one change per commit?
  5. [x] Are you following the conventions of the language?
  6. [x] Have you saved your large formatting changes for a different PR, so we can focus on your work?
  7. [x] Have you explained your rationale for why this feature is needed?
  8. [x] Have you linked your PR to an open issue

Provides a fix for #158 by providing the encryption key on stdin rather than writing it to disk. This eliminates the requirement for /tmp to be a tmpfs as we just don't write the key to disk at all.

I've also made a few other changes to make the code a little more idiomatic.

Adding unit tests is doable - cryptsetup will create a loopback device if we target a file instead of a block device, we'd need to make further changes in order to make the functions unit testable. I'll raise a separate issue for doing this.

Can be tested using following manifests and the container ghcr.io/avestuk/linode-blockstorage-csi-driver:helm-v0.6.3-5-g763d4d2

apiVersion: v1
data:
  luksKey: aERFS0ZnRVpnbXB1cHBTaFBHN0hhaWxTRkJzeThNemx2bGhBTHZxazArMmpUcmNLckZtdHR0b0Y1SUdsTFZvTHQvanBhV25rL2tjbDdKeG5zWjN4UWpFY1l1bXY0V2t3T3Y3N3grYzJDL2t5eWxkVE5SYUNhVkhHOWZXOW42b2ljb1d6c3lVV2NtdTBkK0pPb3JHWjc5MmxzUzlRNWdYbENnNUJEMngxTW9WVnI4aFRRQXJGZlVYNk51SEYxbzB2L0VHSFUwQTVPNXdpTm5xcGREamY5cjU2clB0MEgyOTBOcjZZNUlqYjVSVElvSkZUNXd3NVhvY3J2TGxSL0dpWFJZZ3plSVNmYmZ5SXI4RnBmUkttalBUWmRMQlNYUE1NZEhKTmNQSWxSRytEZm5CYVRLa0lGd2lXWGp4WFpzczcxSUtpYkVNN1FmandrYTBLRnl1ZndBPT0=
kind: Secret
metadata:
  name: csi-encrypt-example-luks-key
  namespace: kube-system
type: Opaque
---
allowVolumeExpansion: true
apiVersion: storage.k8s.io/v1
kind: StorageClass
metadata:
  name: linode-block-storage-retain-luks
  namespace: kube-system
provisioner: linodebs.csi.linode.com
reclaimPolicy: Retain
parameters:
  linodebs.csi.linode.com/luks-encrypted: "true"
  linodebs.csi.linode.com/luks-cipher: "aes-xts-plain64"
  linodebs.csi.linode.com/luks-key-size: "512"
  csi.storage.k8s.io/node-stage-secret-namespace: kube-system
  csi.storage.k8s.io/node-stage-secret-name: csi-encrypt-example-luks-key
---
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: csi-example-pvcluks
spec:
  accessModes:
  - ReadWriteOnce
  resources:
    requests:
      storage: 10Gi
  storageClassName: linode-block-storage-retain-luks
---
kind: Pod
apiVersion: v1
metadata:
  name: csi-example-pod
spec:
  containers:
    - name: csi-example-container
      image: busybox
      volumeMounts:
      - mountPath: "/data"
        name: csi-example-volume
      command: [ "sleep", "1000000" ]
  volumes:
    - name: csi-example-volume
      persistentVolumeClaim:
        claimName: csi-example-pvcluks

Logs showing successful running of commands shown below

csi-linode-node-blg44 csi-linode-plugin I0312 12:21:51.224221       1 nodeserver.go:302] luksContext encryption enabled
csi-linode-node-blg44 csi-linode-plugin I0312 12:21:51.228082       1 nodeserver.go:308] luks volume now formatting: /dev/disk/by-id/scsi-0Linode_Volume_pvc8f834f890a3e4f54
csi-linode-node-blg44 csi-linode-plugin I0312 12:21:51.228137       1 luks.go:133] executing cryptsetup luksFormat command [-v --batch-mode --cipher aes-xts-plain64 --key-size 512 --key-file - luksFormat /dev/disk/by-id/scsi-0Linode_Volume_pvc8f834f890a3e4f54]
csi-linode-node-blg44 csi-linode-plugin I0312 12:22:02.409794       1 luks.go:155] luksOpen /dev/disk/by-id/scsi-0Linode_Volume_pvc8f834f890a3e4f54
csi-linode-node-blg44 csi-linode-plugin I0312 12:22:02.409977       1 luks.go:216] executing cryptsetup luksOpen command
csi-linode-node-blg44 csi-linode-plugin I0312 12:22:04.305369       1 luks.go:169] The LUKS volume name is pvc8f834f890a3e4f54
csi-linode-node-blg44 csi-linode-plugin I0312 12:22:04.305420       1 luks.go:189] executing cryptsetup close command
csi-linode-node-blg44 csi-linode-plugin I0312 12:22:04.529456       1 luks.go:216] executing cryptsetup luksOpen command
csi-linode-node-blg44 csi-linode-plugin I0312 12:22:06.446573       1 nodeserver.go:324] formatting and mounting the drive
csi-linode-node-blg44 csi-linode-plugin I0312 12:22:06.446607       1 mount_linux.go:405] Attempting to determine if disk "/dev/mapper/pvc8f834f890a3e4f54" is formatted using blkid with args: ([-p -s TYPE -s PTTYPE -o export /dev/mapper/pvc8f834f890a3e4f54])
csi-linode-node-blg44 csi-linode-plugin I0312 12:22:06.456620       1 mount_linux.go:408] Output: "", err: exit status 2
csi-linode-node-blg44 csi-linode-plugin I0312 12:22:06.456643       1 mount_linux.go:366] Disk "/dev/mapper/pvc8f834f890a3e4f54" appears to be unformatted, attempting to format as type: "ext4" with options: [-F -m0 /dev/mapper/pvc8f834f890a3e4f54]
csi-linode-node-blg44 csi-linode-plugin I0312 12:22:06.612352       1 mount_linux.go:376] Disk successfully formatted (mkfs): ext4 - /dev/mapper/pvc8f834f890a3e4f54 /var/lib/kubelet/plugins/kubernetes.io/csi/linodebs.csi.linode.com/85d5758976a0c19a22a7181ef751e09337459838984d7476c1a9cb4281d688e9/globalmount
csi-linode-node-blg44 csi-linode-plugin I0312 12:22:06.612655       1 mount_linux.go:394] Attempting to mount disk /dev/mapper/pvc8f834f890a3e4f54 in ext4 format at /var/lib/kubelet/plugins/kubernetes.io/csi/linodebs.csi.linode.com/85d5758976a0c19a22a7181ef751e09337459838984d7476c1a9cb4281d688e9/globalmount
csi-linode-node-blg44 csi-linode-plugin I0312 12:22:06.613505       1 mount_linux.go:146] Mounting cmd (mount) with arguments (-t ext4 -o defaults /dev/mapper/pvc8f834f890a3e4f54 /var/lib/kubelet/plugins/kubernetes.io/csi/linodebs.csi.linode.com/85d5758976a0c19a22a7181ef751e09337459838984d7476c1a9cb4281d688e9/globalmount)
avestuk commented 6 months ago

@srust There are no current e2e tests for LUKS and I believe that the e2e tests are currently broken - https://github.com/linode/linode-blockstorage-csi-driver/issues/129

Adding unit tests should be possible but it'll require more changes to functions and I wanted to keep this somewhat scope limited. I can commit to writing the unit tests but I'd suggest we not block on them.

EDIT: You can test directly using my prebuilt container off the last commit on this branch ghcr.io/avestuk/linode-blockstorage-csi-driver:helm-v0.6.3-5-g763d4d2

srust commented 6 months ago

Adding unit tests should be possible but it'll require more changes to functions and I wanted to keep this somewhat scope limited. I can commit to writing the unit tests but I'd suggest we not block on them.

Fair!