Re: overlayfs access checks on underlying layers

[Stephen Smalley <sds@tycho.nsa.gov>]

On 11/29/18 6:04 AM, Miklos Szeredi wrote:
> On Wed, Nov 28, 2018 at 10:43 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>>
>> On 11/28/18 3:24 PM, Miklos Szeredi wrote:
>>> On Wed, Nov 28, 2018 at 8:32 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> 
> [...]
> 
>>>> Does the breaking commit (007ea44892e6) fix a real bug affecting users?
>>>>     If not, I'd recommend just reverting it.
>>>
>>> That is certainly an option, but...  this is all about context=
>>> mounts, right?  Which allows mounter to override MAC checks under the
>>> new mount?  On any mount, not just overlay, right?  So why is overlay
>>> special?
>>
>> With other filesystems, the files are only accessible under the context
>> specified by the mounter (and you can't mount it twice with differing
>> context mount options). With overlay, the file is simultaneously
>> accessible under both the context specified by the mounter via the
>> overlay and under its lower/upper context via the lower/upper dir.
>>
>> Generally we only use context mounts on other filesystems when they have
>> no label information at all (no security.selinux xattrs) or when they
>> are completely untrusted to provide that information; the context
>> specified by the mounter is the only basis for access control.  With
>> overlay, we are frequently dealing with labeled lower and upper
>> directories in a filesystem we trust.
>>
>> It seems like overlay has a goal of preventing the mounter from
>> escalating its access through an overlay mount.
> 
> Overlayfs main purpose is to bypass limited access to a read-only
> filesystem by copying up on a write access.  So bypassing access is
> built into the system, but it's done in a way that in the end it
> doesn't permit mounter to do more than it could otherwise do.  Or at
> least that's the intent, as you say.
> 
> To generalize that, we could say:  trigger a copy-up if access to
> lower layer object is denied.  That would extend the scope of the
> trigger from write access on file/dir to read/write on special files
> and execute on regular files, all of which could theoretically be
> denied on the lower object, yet allowed on the upper object without
> violating the above rule.

Possibly I misunderstood you, but I don't think we want to copy-up on 
permission denial, as that would still allow the mounter to read/write 
special files or execute regular files to which it would normally be 
denied access, because the copy would inherit the context specified by 
the mounter in the context mount case.  It still represents an 
escalation of privilege for the mounter.  In contrast, the copy-up on 
write behavior does not allow the mounter to do anything it could not do 
already (i.e. read from the lower, write to the upper).

> 
>>> I'd just like to see proper justification for why we should be doing
>>> those checks on underlying layer that simply don't belong there, IMO.
>>>    I'm sure you know better than I that it's not just about real bugs
>>> affecting users, it's about having a clear, well defined model to base
>>> the design on.   And by reverting the breaking commit, I don't see us
>>> getting closer to that.
>>
>> It seems like the NFS folks raised a number of concerns with the overlay
>> approach beyond just these two checks,
> 
> The concerns that NFS folks had was that overlayfs does not enforce
> permission checks (with creds of task) on underlying objects,
> resulting in possibly elevated access compared to directly accessing
> the NFS mount.    But I think there's no way to reconcile server
> permission checks with overlayfs, so we are left with the current
> model of only verifying the permission on the server against the
> mounter's creds.

Typically this will only work if the NFS files are 
world-readable/searchable, right, since usually the mounter will be root 
and the server will enable root squash on the export?  Is that 
sufficient for most use cases?

> 
>> and Android has their
>> override_creds=off use case.  Maybe the overlay model needs a more
>> significant rethinking than just these two cases.
> 
> Yes, it would be good if that override_creds=off use case was
> discussed.  AFAICS it trades less privilege in the mounter for more
> privilege in the task accessing the overlay.  If, or how, that is a
> good model for anything other than Android is not clear to me.
> 
> Thanks,
> Miklos
>