lxd/device: Disable virtiofsd idmap for shifted volumes

[jonathan-conder]

Partially fixes #18686. My rationale is described there.

Without this, file ownership in VMs doesn't match containers. This is technically a breaking change, but I suspect not many people are using raw.idmap together with shifted volumes, otherwise #18561 probably would have been discovered sooner. If it's an issue I'm happy to gate this behind a new config option though.

Checklist

• I have read the contributing guidelines and attest that all commits in this PR are signed off, cryptographically signed, and follow this project's commit structure.
• I have checked and added or updated relevant documentation.

[Copilot]

Pull request overview

Adjusts VM virtio-fs behavior so virtiofsd does not apply instance raw.idmap when serving shifted storage volumes, aligning file ownership semantics between VMs and containers and addressing the ownership mismatch described in #18686.

Changes:

• Update VM disk device setup to avoid parsing/applying raw.idmap for security.shifted volumes when starting virtiofsd.
• Add an integration test ensuring shifted volumes in VMs preserve on-disk ownership (i.e. do not appear as nobody:nogroup) even when raw.idmap is set.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
lxd/device/disk.go	Skips applying raw.idmap to virtiofsd for security.shifted volumes during VM disk setup.
test/suites/vm.sh	Adds a regression test validating ownership consistency for shifted volumes shared into VMs over virtio-fs.

[jonathan-conder]

@tomponline Another approach would be to keep running virtiofsd in a user namespace anyway, but shift the mount that it looks at. e.g. if someone exploits a buffer overflow in virtiofsd itself they would be able to write files as root within the mount, but not to /etc/shadow or w/e.

[tomponline]

@tomponline Another approach would be to keep running virtiofsd in a user namespace anyway, but shift the mount that it looks at. e.g. if someone exploits a buffer overflow in virtiofsd itself they would be able to write files as root within the mount, but not to /etc/shadow or w/e.

Yes, effectively I think you're proposing to sandbox virtiofsd to give it permission to write/read files as root, but not to be able to run code as root. Right?

[jonathan-conder]

Yes, effectively I think you're proposing to sandbox virtiofsd to give it permission to write/read files as root, but not to be able to run code as root. Right?

Yeah. It would be limited to reading/writing those files within the shifted mount as well, can't touch /etc/shadow, etc.

That said, virtiofsd is already sandboxed. So the change would be to add shifting to the mount (reducing the level of security but matching how things work for containers)

[tomponline]

Right. Lets focus on getting the behaviours aligned and then work on sandboxing separately once the correct behaviour is established. Thanks

[tomponline]

Hi @jonathan-conder

Thanks for this PR.

I've been thinking about this, and I think it'd be worth exploring always running virtiofsd in a userns, rather than only doing it in certain scenarios.

The difference being that in the scenarios today where we dont run virtiofsd in a userns (as well as the new one you're adding here) we would pass through a full unrestricted idmap, e.g.

// Map UIDs 0 to 4294967295 1:1 with the host UIDs
		UidMappings: []syscall.SysProcIDMap{
			{
				ContainerID: 0,
				HostID:      0,
				Size:        maxIDRange,
			},
		},

		// Map GIDs 0 to 4294967295 1:1 with the host GIDs
		GidMappings: []syscall.SysProcIDMap{
			{
				ContainerID: 0,
				HostID:      0,
				Size:        maxIDRange,
			},
		},

This way virtiofsd is always restricted without CAP_SYS_ADMIN, Host Kernel Module Loading, and anything protected by init_user_ns checks.

[tomponline]

That said, virtiofsd is already sandboxed. So the change would be to add shifting to the mount (reducing the level of security but matching how things work for containers)

Virtiofsd uses --sandbox=namespace by default which sandboxed uses a mount namespace (amongst other things).

namespace: The program switches into a new file system
namespace (namespaces(7)) and invokes pivot_root(2) to make the shared directory
tree its root. A new mount (mount_namespaces(7)), pid (pid_namespaces(7)) and
net namespace (network_namespaces(7)) is also created to isolate the process.

So I don't think we need to introduce another mount namespace.

[tomponline]

That said, virtiofsd is already sandboxed. So the change would be to add shifting to the mount (reducing the level of security but matching how things work for containers)

Virtiofsd uses --sandbox=namespace by default which sandboxed uses a mount namespace (amongst other things).

namespace: The program switches into a new file system
namespace (namespaces(7)) and invokes pivot_root(2) to make the shared directory
tree its root. A new mount (mount_namespaces(7)), pid (pid_namespaces(7)) and
net namespace (network_namespaces(7)) is also created to isolate the process.

So I don't think we need to introduce another mount namespace.

Although it wouldn't hurt to make that explicit in LXD's calling code, as currently its relying on the default option of virtiofsd.

[jonathan-conder]

The difference being that in the scenarios today where we dont run virtiofsd in a userns (as well as the new one you're adding here) we would pass through a full unrestricted idmap, e.g.

// Map UIDs 0 to 4294967295 1:1 with the host UIDs
		UidMappings: []syscall.SysProcIDMap{
			{
				ContainerID: 0,
				HostID:      0,
				Size:        maxIDRange,
			},
		},

		// Map GIDs 0 to 4294967295 1:1 with the host GIDs
		GidMappings: []syscall.SysProcIDMap{
			{
				ContainerID: 0,
				HostID:      0,
				Size:        maxIDRange,
			},
		},

This way virtiofsd is always restricted without CAP_SYS_ADMIN, Host Kernel Module Loading, and anything protected by init_user_ns checks.

I have to admit I didn't realise that unrestricting the idmap like this was still a meaningful security boundary. Certainly seems fine to just drop all caps but remain UID 0 (although maybe file operations still need something like CAP_CHOWN, for example).

This approach seems fine to me, although I don't know that I can implement it myself without taking time away from my main tasks.

[tomponline]

WIP is over here #18918

[jonathan-conder]

Closing in favour of #18918