[dwsuse]

Fourth step of privilege separation (#3879): nvme-cli uses the helper. When root runs nvme-cli on behalf of a user and the command does not need root, nvme-cli starts the helper, drops its privileges and opens the devices through the helper. Rebased onto master with #4185.

Still behind the privsep meson option (disabled by default, Linux only). The code is the same with the option disabled, except that it never starts a helper. setuid is not supported: a setuid nvme-cli keeps running as it was started and never trusts the environment.

Fix found by the privsep test runs: the callers of libnvme_get_nsid(), libnvme_reset_subsystem() and libnvme_rescan_ns() checked errno instead of the return value. A privsep handle leaves errno alone, so get-feature, set-feature and fdp set-events failed on a controller device, and ns get-id and format could report success after a failure.

Prep:

• privsep handles report libnvme_transport_handle_is_direct(), so nvme format still rescans the namespaces, updates the block size and resets the controller
• libnvme_set_ioctl_probing() reaches the helper: OPEN carries it in the former reserved word, the helper opens the handle in a context without probing and rejects unknown options
• an nvme-cli.conf which can't be read is reported with its name and error instead of failing silently. The file holds no secrets and must be readable by every user, as it is read after the drop; nvme-cli.conf(5) says so

Which commands drop: an explicit needs_root flag in struct command marks the commands which keep root: the commands without a device argument (fabrics, enumeration, keys, exclusions, configuration, registry, NBFT, and the deprecated key commands) and the device commands which also access PCI config space, the kernel log or root owned paths (Micron vs-pcie-stats, clear-pcie-correctable-errors, vs-internal-log, WDC and Sandisk reason-identifier, vs-telemetry-controller-option, registry update and delete). The list is what has to be addressed before all commands can drop. Commands with an MI device (mctp:<net>,<eid> in their arguments) keep root, as MI uses the MCTP sockets directly.

When: first in main(), on the command plugin_lookup() found (#4185), before the configuration file or the options of the command are parsed. A needs_root command does not run once the privileges are dropped, should a deprecated command forward to one.

To whom: SUDO_UID/SUDO_GID, else NVME_PRIVSEP_UID/NVME_PRIVSEP_GID, only when nvme-cli runs as root. Without them root keeps its privileges, so scripts and services are unchanged. The supplementary groups become the target group only (no NSS as root). If the helper cannot be started or the privileges cannot be dropped, nvme-cli fails instead of running privileged.

For the tests, NVME_PRIVSEP_TEST_HELPER names a helper which an unprivileged nvme-cli uses as well; it is ignored as root and in secure-execution mode.

Testing:

• libnvme tests: privsep handles are direct; a context without ioctl probing uses the 32 bit passthru in the helper; unknown OPEN options are rejected (each fails without the change)
• unit test: an nvme-cli.conf which can't be read is reported (fails without the change)
• CLI test (nvme-cli - privsep, needs the id plugin): device commands start the helper and the helper opens the device, commands which keep root, MI devices, help and version don't, a mctp: file name does not keep root; skipped when run as root
• the mock CLI tests (print, pif-sts, init-pi-tags, dry-run, sel, host-metadata, discovery-logs, sed) also run with the commands going through the helper (nvme-cli - <name>-privsep)
• full test suite with and without the option (gcc and clang, --werror), every commit on its own
• real root on a VM against an nvme-loop target: plain root, NVME_PRIVSEP_UID, SUDO_UID, missing helper, needs_root commands, MI, setuid nvme keeps root and ignores the environment, nvme-cli.conf read after the drop with the command line winning
• FreeBSD and Windows (x86_64, aarch64) builds

[igaw]

I am not totally happy with this approach but given that we retrofitting privilege separation, it's no surprise that there are exception necessary at this point. Ideally we could drop the root rights right at the beginning without parsing anything. But this is not possible right now. So I think it's a step towards this goal.

Thus let's get the device commands working and figure out how to handle the fabrics related one in a different step.

@martin-belanger do you agree with this approach do you think this is a good idea?

I am still not sure if the whole privsep is going to work as hoped. I'd say we mark this as experiment and if it turns out shitty, we can remove it again.

[Copilot]

🟡 Changes recommended

Helper integration introduces functional regressions and an argument-based path that unintentionally retains root privileges.

5 open findings

• Handle helper-backed devices in format rescan and reset guards · New
• Restrict privilege-separation bypass to the device argument · New
• Keep registry update and delete commands privileged · New
• Forward no-ioctl-probing setting to the helper · New
• Skip helper tests under root or run them unprivileged · New

What changed in this PR

Connects nvme-cli to the existing privilege-separation helper, allowing eligible commands to drop root privileges before parsing configuration and options. This remains Linux-only and disabled by default.

Changes:

• Adds early command selection, privilege dropping and helper-backed device access.
• Marks commands that must retain root privileges.
• Adds command-selection checks and helper-backed CLI tests.

File	Description
tests/​unit/​c/​test-plugin.c	Tests command-selection consistency.
tests/​unit/​c/​meson.build	Links privilege-separation support into tests.
tests/​cli/​nvme_privsep_test.py	Tests helper startup and device routing.
tests/​cli/​meson.build	Registers helper-backed CLI tests.
src/​privsep.h	Declares privilege-separation interfaces and stubs.
src/​privsep.c	Implements helper startup and privilege dropping.
src/​plugin.h	Adds command privilege metadata and lookup interface.
src/​plugin.c	Adds early lookup and execution checks.
src/​nvme.c	Initializes privilege separation before configuration loading.
src/​nvme-cmds-fabrics.c	Attaches contexts and preserves fabrics privileges.
src/​nvme-cmds-enumerate.c	Preserves enumeration privileges.
src/​nvme-cmds-deprecated.c	Preserves deprecated key-command privileges.
src/​meson.build	Conditionally builds privilege-separation support.
src/​global-ctx.c	Attaches shared contexts to the helper.
plugins/​zns/​zns.c	Preserves listing privileges.
plugins/​wdc/​wdc-nvme.c	Marks privileged telemetry commands.
plugins/​utils/​utils.c	Exempts metadata dumping from privilege dropping.
plugins/​solidigm/​solidigm-nvme.c	Exempts version reporting.
plugins/​seagate/​seagate-nvme.c	Exempts version reporting.
plugins/​sandisk/​sandisk-nvme.c	Marks privileged telemetry commands.
plugins/​registry/​registry-nvme.c	Preserves registry-list privileges.
plugins/​netapp/​netapp-nvme.c	Preserves enumeration privileges.
plugins/​nbft/​nbft-plugin.c	Preserves NBFT-reading privileges.
plugins/​micron/​micron-nvme.c	Marks privileged diagnostics and version commands.
plugins/​keys/​keys-plugin.c	Preserves key-management privileges.
plugins/​huawei/​huawei-nvme.c	Preserves listing privileges.
plugins/​exclusion/​exclusion-nvme.c	Preserves exclusion-management privileges.
plugins/​config/​config-nvme.c	Preserves configuration-management privileges.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[martin-belanger]

Thus let's get the device commands working and figure out how to handle the fabrics related one in a different step.

@martin-belanger do you agree with this approach do you think this is a good idea?

I agree with this approach. Getting the device commands working first makes sense, and so does marking it as an experiment.

For the fabrics commands, here is an idea for the later step. nvme connect, connect-all, disconnect and discover could send the request to nvme-discoverd over Varlink, like systemctl does with systemd. nvme-cli would then drop root at the start for these commands too, and nvme-discoverd does the work as root. It already does the connects, writes the registry, uses the keyring and reads the DH-HMAC-CHAP secrets. It would also be the only process that writes the registry. nvme-discoverd checks who the caller is with SO_PEERCRED (or polkit).

Limits:

• Root keeps the direct path, as in this PR. This keeps scripts, the initramfs and nvme-discoverd's own nvme connect units working without a daemon.
• nvme-discoverd gets one more source of input to parse as root, so it may need its own privilege separation later.
• The Varlink interface becomes an API we have to keep stable.

This is only a thought,

[dwsuse]

That is also an option, that the fabrics command are just forwarding the request to discocverd. This might be an option for a production system. I think we still want the feature to be able to write directly to the fabrics device for development work. FWIW, the current version of the privsep helper is able to handle the fabrics command (writing to the fabrics device). What is missing are all the registry features.

[Copilot]

🔵 Needs a closer look

The credential-changing lifecycle and new privilege boundary require final human security review.

7 open issues

• Restrict privilege-separation bypass to the device argument
• Handle helper-backed devices in format rescan and reset guards
• Report unreadable configuration errors after privilege drop · New
• Gate id-ctrl helper tests on the id plugin · New
• Skip helper tests under root or run them unprivileged
• Forward no-ioctl-probing setting to the helper
• Keep registry update and delete commands privileged

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

The credential transition and privileged-helper trust boundary warrant final human security review.

1 open issue

• Keep registry update and delete commands privileged

6 resolved since last review

• Restrict privilege-separation bypass to the device argument
• Handle helper-backed devices in format rescan and reset guards
• Gate id-ctrl helper tests on the id plugin
• Report unreadable configuration errors after privilege drop
• Skip helper tests under root or run them unprivileged
• Forward no-ioctl-probing setting to the helper

_{Review effort: Balanced}