pve-qemu: enable FUSE block export support#222
Conversation
Enable FUSE support in pve-qemu by adding fuse3 to buildInputs and --enable-fuse to configureFlags. This is required for features like storing TPM state as qcow2 files. Also patch swtpm to zero the file header instead of unlinking when clearing state, since FUSE exports cannot be unlinked. Fixes SaumonNet#221
There was a problem hiding this comment.
Pull request overview
Enables QEMU FUSE block export support in the Proxmox/Nix packaging to unblock features like TPM state stored as qcow2 via qemu-storage-daemon, and adds a swtpm patch to cope with FUSE exports that can’t be unlinked.
Changes:
- Add
fuse3and--enable-fuseto thepve-qemubuild. - Introduce a swtpm patch that clears state by zeroing the header instead of unlinking.
- Override
swtpmin the package set to apply the new patch.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
pkgs/pve-qemu/default.nix |
Adds fuse3 to build inputs and enables QEMU FUSE support at configure time. |
pkgs/pve-qemu-server/swtpm-fuse.patch |
Patches swtpm’s file backend state deletion behavior for FUSE-backed exports. |
pkgs/default.nix |
Overrides swtpm to apply the new patch (using a separate nixpkgs import). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - close(fd); | ||
| - return 1; | ||
| - } | ||
| + char zerobuf[8] = {0}; /* swtpm header has 8 byte */ |
There was a problem hiding this comment.
Minor grammar in comment: “8 byte” should be “8 bytes”.
| + char zerobuf[8] = {0}; /* swtpm header has 8 byte */ | |
| + char zerobuf[8] = {0}; /* swtpm header has 8 bytes */ |
| # Patch swtpm for FUSE export support, using a separate nixpkgs import to avoid recursion | ||
| basePkgs = import pkgs.path { inherit (pkgs) system config; }; | ||
|
|
||
| ours = { | ||
| swtpm = basePkgs.swtpm.overrideAttrs (old: { |
There was a problem hiding this comment.
Re-importing nixpkgs via basePkgs = import pkgs.path ... means the patched swtpm will ignore any overlays applied to the pkgs argument and can diverge from the rest of the package set. If possible, prefer overriding swtpm from the existing package set (e.g., via prev.swtpm in an overlay-style definition) so external overlays/configuration are preserved and nixpkgs isn’t imported twice.
| # Patch swtpm for FUSE export support, using a separate nixpkgs import to avoid recursion | |
| basePkgs = import pkgs.path { inherit (pkgs) system config; }; | |
| ours = { | |
| swtpm = basePkgs.swtpm.overrideAttrs (old: { | |
| ours = { | |
| swtpm = pkgs.swtpm.overrideAttrs (old: { |
| - * swtpm binary will treat it as a new instance. */ | ||
| +/* Delete state from file: zero header so swtpm binary will treat it as a new | ||
| + * instance. */ | ||
| static int delete_state(void *state) { | ||
| const struct file_state *fstate = (struct file_state*)state; | ||
| int fd; | ||
|
|
||
| - if (fstate->is_blockdev) { | ||
| - char zerobuf[8] = {0}; /* swtpm header has 8 byte */ | ||
| - fd = open(fstate->path, O_WRONLY); | ||
| - if (fd < 0) { | ||
| - logerr(gl_LOGFILE, "Couldn't open file for clearing %s: %s\n", | ||
| - fstate->path, strerror(errno)); | ||
| - return 1; | ||
| - } | ||
| - /* writing less bytes than requested is bad, but won't set errno */ | ||
| - errno = 0; | ||
| - if (write(fd, zerobuf, sizeof(zerobuf)) < (long)sizeof(zerobuf)) { | ||
| - logerr(gl_LOGFILE, "Couldn't write file for clearing %s: %s\n", | ||
| - fstate->path, strerror(errno)); | ||
| - close(fd); | ||
| - return 1; | ||
| - } | ||
| + char zerobuf[8] = {0}; /* swtpm header has 8 byte */ | ||
| + fd = open(fstate->path, O_WRONLY); | ||
| + if (fd < 0) { | ||
| + logerr(gl_LOGFILE, "Couldn't open file for clearing %s: %s\n", | ||
| + fstate->path, strerror(errno)); | ||
| + return 1; | ||
| + } | ||
| + /* writing less bytes than requested is bad, but won't set errno */ | ||
| + errno = 0; | ||
| + if (write(fd, zerobuf, sizeof(zerobuf)) < (long)sizeof(zerobuf)) { | ||
| + logerr(gl_LOGFILE, "Couldn't write file for clearing %s: %s\n", | ||
| + fstate->path, strerror(errno)); | ||
| close(fd); | ||
| - } else { | ||
| - if (unlink(fstate->path)) { | ||
| - logerr(gl_LOGFILE, "Couldn't unlink file for clearing %s: %s\n", | ||
| - fstate->path, strerror(errno)); | ||
| - return 1; | ||
| - } | ||
| + return 1; | ||
| } |
There was a problem hiding this comment.
The updated delete_state implementation now always zeroes only the first 8 bytes of the TPM state file instead of unlinking regular files, leaving the remaining file contents (including prior TPM state) readable after a "clear" operation. A local attacker (or future compromise of the host or storage system) with read access to fstate->path after clearing can still recover sensitive TPM material by reading beyond the header. Consider restricting header-zeroing behavior to backends that truly cannot be unlinked (e.g. FUSE/block devices) and keep unlinking or fully overwriting/truncating the entire file for regular file-based state storage so that cleared TPM state is not trivially recoverable.
|
|
||
| sourceRoot = "${src.name}/qemu"; | ||
|
|
||
| buildInputs = old.buildInputs ++ [ proxmox-backup-qemu ]; |
There was a problem hiding this comment.
It would be preferred to add a TODO comment here to remind removal when upgrading to future nixpkgs 26.05 where the upstream fix would be in place and this flag enabled by default
| basePkgs = import pkgs.path { inherit (pkgs) system config; }; | ||
|
|
||
| ours = { | ||
| swtpm = basePkgs.swtpm.overrideAttrs (old: { |
There was a problem hiding this comment.
Proxmox uses an older version (0.8.0) of swtpm with more than one custom patches here: https://git.proxmox.com/?p=swtpm.git. I think we have two options for this problem:
- Override the nixpkgs swtpm with necessary patches from Proxmox (what this PR is doing)
- Package the Proxmox's git repository under a different name
pve-swtpm.
Both looks fine, so leaving this for project owner's input.
|
Hello, could you please avoid having the merge commit in the PR, rebasing on top of main? Thanks! |
Enable FUSE support in pve-qemu by adding fuse3 to buildInputs and --enable-fuse to configureFlags. This is required for features like storing TPM state as qcow2 files.
Also patch swtpm to zero the file header instead of unlinking when clearing state, since FUSE exports cannot be unlinked.
Fixes #221