Re: [PATCH v8 2/2] overlayfs: override_creds=off option bypass creator_cred

[Mark Salyzyn <salyzyn@android.com>]

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

On 11/08/2018 12:01 PM, Vivek Goyal wrote:
> On Tue, Nov 06, 2018 at 03:01:15PM -0800, Mark Salyzyn wrote:
>> By default, all access to the upper, lower and work directories is the
>> recorded mounter's MAC and DAC credentials.  The incoming accesses are
>> checked against the caller's credentials.
> Ok, I am trying to think of scenarios where override_creds=off can
> provide any privilege escalation. How about following.
>
> $ mkdir lower lower/foo upper upper/foo work merged
> $ touch lower/foo/bar.txt
> $ chmod 700 lower/foo/
>
> # Mount overlay with override_creds=off
>
> $ mount -t overlay -o
> lowerdir=lower,upperdir=upper,workdir=work,override_creds=off none merged
>
> # Try to read lower/foo as unpriviliged user. Say "test"
> # su test
> # ls merged/foo/
> ls: cannot access 'merged/foo/': Operation not permitted
>
> # Now first try to do same operation as root and retry as test user.
> $ exit
> $ ls merged/foo
> bar.txt
> $ su test
> $ ls merged/foo
> bar.txt
>
> lower/foo/ is not readable by user "test". So it fails in first try. Later
> "root" accesses it and it populates cache in overlayfs. When test retries,
> it gets these entries from cache.
>
> With override_creds=on this is not a problem because overlay provides
> this as functionality as long as mounter as access to lower/foo/.
>
> But with override_creds=off, mounter is not providing any such
> functionality and we are exposing an issue where cache will make
> something available which is not normally available.
>
> I think it probably is a good idea to do something about it?
>
> Thanks
> Vivek
>
Good stuff.

That sounds like a bug in cache (!) to not recheck caller's credentials. 
Currently unsure how/where to force bypass of the cache (performance 
hit) as it is wired in throughout the code without a clear off switch, 
or rechecking of the credentials at access. This does need to be 
addressed to make this 'feature' more useful/trusted for non-MAC 
controlled, use cases.

This is not a problem in the Android usage case since DAC is simple and 
all can be read, execute bits might be controlled, the owners and perms 
are otherwise unremarkable in the affected arenas that are utilizing 
overlayfs. Not using it for a generic r/w backing except in rooted debug 
scenarios by developers, otherwise everything is r/o, MAC on the other 
hand is complex and heavily inspected. We do, however, want multi level 
security in that both DAC and MAC can be trusted and can protect each 
other from holes.

Sounds like the caveats in the documentation need to be expanded if _no_ 
solution for this kind of access pattern becomes apparent.

-- Mark

[-- Attachment #2: Type: text/html, Size: 4711 bytes --]