Re: [PATCH] virtiofsd: Use clone() and not unshare(), support non-root

[Stefan Hajnoczi]

On Thu, May 21, 2020 at 11:43:44AM +0100, Daniel P. Berrangé wrote:
> On Thu, May 21, 2020 at 11:19:23AM +0100, Stefan Hajnoczi wrote:
> > On Thu, May 07, 2020 at 10:28:32AM +0100, Daniel P. Berrangé wrote:
> > > If the person in the host launching virtiofsd is non-root, then
> > > user namespaces mean they can offer the guest the full range of
> > > POSIX APIs wrt access control & file ownership, since they're
> > > no longer restricted to their single host UID when inside the
> > > container.
> > 
> > What installs the uid_map/gid_map for virtiofsd?
> > 
> > My machine has /etc/subuid and /etc/subgid, but how would this come into
> > play with these patches applied?
> 
> AFAICT, nothing is setting up the mapping, hence my question in the first
> review of this patch, that this patch seems incomplete.
> 
> The subuid/subgid files are essentially just reserving a range of IDs
> for each user. Something then needs to read & honour those IDs.
> 
> The rules on setting up a mapping are complex though, to avoid a user
> from creating a new user namespace, and defining a mapping that lets
> them elevate privileges to read files in the host they wouldn't otherwise
> be permitted to access.
> 
> IIUC, applying the range of IDs from subuid/subgid will require the
> process defining the mapping to have CAP_SETUID *outside* the container.
> As an unprivileged host user, you won't have that, so I think the only
> thing you can do is setup a mapping for your own UID/GID, which would
> allow the container to read/write files owned by the host user that
> started it. That should be ok for virtiofsd's needs at least for simple
> file sharing. If virtiofsd needs to expose its full range of features
> though, something privileged in the host needs to setup a full mapping
> based on subuid/subgid
> 
> BTW, "man user_namespaces" gives many more details on UID mapping
> rules.
> 
> > What happens when an unprivileged user who is not listed in /etc/subuid
> > runs virtiofsd?
> 
> The UID map inside the container will be empty, which should result in
> all files being remapped to (uid_t)-1 or whatever is shown in the
> /proc/sys/kernel/overflow{u,g}id files.
> 
> So virtiofsd would not have any write access, and only have read access
> to files which have world-read bit set.  

Okay. Enabling user namespaces for virtiofsd is valuable. I think the
behavior should be documented though so users know what needs to be
configured. That is missing from this patch series.

Stefan

signature.asc
Description: PGP signature