Re: [PATCH] isolation: keep CAP_DAC_OVERRIDE initially

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2567 bytes --]

On Tue, Oct 07, 2025 at 08:16:39AM -0400, Cole Robinson wrote:
> Reproducer that I'd expect to work
> 
>   $ cd $HOME
>   $ sudo passt --runas $UID --socket foo.sock
>   Failed to bind UNIX domain socket: Permission denied
> 
> A more practical example is for libguestfs apps when run as user=root.
> 
> + libguestfs connects to libvirt qemu:///system
> + libvirt qemu:///system defaults to user=qemu.
>   + chowns passt runtime dir to user=qemu
> + libguestfs instead requests the VM run as user=root
>   + patches in progress but we are blocked by this issue
> + passt is launched as root, but can't open socket in passt dir.
> 
> Obviously libvirt needs improvements too.
> But it seems like this is a defect as well.
> 
> Signed-off-by: Cole Robinson <crobinso@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

I was momentarily dismayed by keeping such a powerful
capability... then I realised we already keep CAP_SETUID and
CAP_SYS_ADMIN.  As the comment says, it's not clear if dropping the
relatively few and weak capabilities we do here is useful, but we
might as well - the "real" drop of capabilities is in isolate_user().

> ---
>  isolation.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/isolation.c b/isolation.c
> index bbcd23b..b25f349 100644
> --- a/isolation.c
> +++ b/isolation.c
> @@ -188,6 +188,9 @@ void isolate_initial(int argc, char **argv)
>  	 * We have to keep CAP_SETUID and CAP_SETGID at this stage, so
>  	 * that we can switch user away from root.
>  	 *
> +	 * CAP_DAC_OVERRIDE may be required for socket setup when combined
> +	 * with --runas.
> +	 *
>  	 * We have to keep some capabilities for the --netns-only case:
>  	 *  - CAP_SYS_ADMIN, so that we can setns() to the netns.
>  	 *  - Keep CAP_NET_ADMIN, so that we can configure interfaces
> @@ -198,7 +201,7 @@ void isolate_initial(int argc, char **argv)
>  	 * isolate_prefork().
>  	 */
>  	keep = BIT(CAP_NET_BIND_SERVICE) | BIT(CAP_SETUID) | BIT(CAP_SETGID) |
> -	       BIT(CAP_SYS_ADMIN) | BIT(CAP_NET_ADMIN);
> +	       BIT(CAP_SYS_ADMIN) | BIT(CAP_NET_ADMIN) | BIT(CAP_DAC_OVERRIDE);
>  
>  	/* Since Linux 5.12, if we want to update /proc/self/uid_map to create
>  	 * a mapping from UID 0, which only happens with pasta spawning a child
> -- 
> 2.51.0
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]