Re: overlayfs access checks on underlying layers

[Stephen Smalley]

On 12/4/18 11:17 AM, Vivek Goyal wrote:
> On Tue, Dec 04, 2018 at 11:05:46AM -0500, Stephen Smalley wrote:
> > On 12/4/18 10:42 AM, Vivek Goyal wrote:
> > > On Tue, Dec 04, 2018 at 04:31:09PM +0100, Miklos Szeredi wrote:
> > > > On Tue, Dec 4, 2018 at 4:22 PM Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
> > > >
> > > > > Having said that, this still create little anomaly when mknod to client
> > > > > is not allowed on context label. So a device file, which is on lower
> > > > > and client can not open it for read/write on host, it can now be opened
> > > > > for read/write because mounter will allow access. So why it is different
> > > > > that regular copy up. Well, in regular copy up, we created a copy of
> > > > > the original object and allowed writing to that object (cp --preserve=all)
> > > > > model. But in case of device file, writes will go to same original
> > > > > object. (And not a separate copy).
> > > >
> > > > That's true.
> > > >
> > > > In that sense copy up of special file should result in upper having
> > > > the same label as of lower, right?
> > >
> > > I guess that might be reasonable (if this behavior is a concern). So even
> > > after copy up, client will not be able to read/write a device if it was
> > > not allowed on lower.
> > >
> > > Stephen, what do you think about retaining label of lower for device
> > > files during copy up. What about socket/fifo.
> >
> > We don't check client task access to the upper inode label, only to the
> > overlay, right?  So the client is still free to access the device through
> > the overlay even if we preserve the lower inode label on the upper inode?
> > What do we gain?
>
> That's only with latest code and Miklos said he will revert it for 4.20.
>
> IOW, I am assuming that we will continue to check access to a file
> on upper in the context of mounter. Otherwise, client will be able to access
> files on upper/ which even mounter can't access.

I was assuming we're talking about the proposed solution, where we check 
client access to the overlay (unchanged), mounter access to lower 
(unchanged), copy-up if denied (new), mounter access to upper (new in 
the sense that previously we didn't copy-up on denials).

In that situation, propagating the lower inode label to the upper inode 
only impacts the mounter checks, and in that case makes copy-up 
pointless - if it wasn't allowed to lower it won't be allowed to upper. 
 If it is allowed, then client task is free to access the device 
regardless as long as it has permissions to the overlay inode.  So I 
don't see what we gain by propagating the lower inode label to the upper 
inode in the context mount case, and it creates an inconsistency between 
special files and regular ones.

Did I miss something?