Re: [PATCH v14 2/5] overlayfs: check CAP_DAC_READ_SEARCH before issuing exportfs_decode_fh

Re: [PATCH v14 2/5] overlayfs: check CAP_DAC_READ_SEARCH before issuing exportfs_decode_fh

[Amir Goldstein]

+ ebiederm and nfsd folks

On Wed, Oct 23, 2019 at 11:08 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
>
>
> On Tue, Oct 22, 2019 at 10:46 PM Mark Salyzyn <salyzyn@android.com> wrote:
> >
> > Assumption never checked, should fail if the mounter creds are not
> > sufficient.
>
> A bit more explanation would be nice.  Like a pointer to the explanation given in the open_by_handle_at(2) code where this check was presumably taken from.
>

Well, it's not that simple (TM).
If you are considering unprivileged overlay mounts, then this should be
ns_capable() check, even though open_by_handle_at(2) does not
currently allow userspace nfsd to decode file handles.

Unlike open_by_handle_at(2), overlayfs (currently) never exposes file
data via decoded origin fh. AFAIK, it only exposes the origin st_ino
st_dev and some nlink related accounting.

I have been trying to understand from code if nfsd exports are allowed
from non privileged containers and couldn't figure it out (?).
If non privileged container is allowed to export nosubtreecheck export
then non privileged container root can already decode file handles...

Thanks,
Amir.

Re: [PATCH v14 2/5] overlayfs: check CAP_DAC_READ_SEARCH before issuing exportfs_decode_fh

[J. Bruce Fields]

On Sun, Oct 27, 2019 at 09:24:52AM +0200, Amir Goldstein wrote:
> Well, it's not that simple (TM).
> If you are considering unprivileged overlay mounts, then this should be
> ns_capable() check, even though open_by_handle_at(2) does not
> currently allow userspace nfsd to decode file handles.
> 
> Unlike open_by_handle_at(2), overlayfs (currently) never exposes file
> data via decoded origin fh. AFAIK, it only exposes the origin st_ino
> st_dev and some nlink related accounting.
> 
> I have been trying to understand from code if nfsd exports are allowed
> from non privileged containers and couldn't figure it out (?).
> If non privileged container is allowed to export nosubtreecheck export
> then non privileged container root can already decode file handles...

I don't see any special checks in nfsctl_transaction_write() or
write_threads().  I guess it's just depending on the (0600) file
permissions.  I'm vague on how file permissions work in containers.

The issue with filehandles is that they allow you to bypass directory
lookup permissions.  Keeping a file private by denying permission to
look it up doesn't sound like a good idea to me, honestly, but it does
work on local posix filesystems, so we don't want to break that.

Filehandles are generally pretty easy to guess, and can't be revoked, so
we're more worried about using them (with open_by_handle_at()) than
reading them (with name_to_handle_at()), but we try to prevent the
latter as well.

--b.