linux-kernel - Re: [PATCH v2 1/1] OverlayFS: Fix checking permissions during lookup.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 28 Feb 2016 12:09:42 +0100
From:	Ignacy Gawędzki 
	<ignacy.gawedzki@...en-communications.fr>
To:	Vivek Goyal <vgoyal@...hat.com>
Cc:	linux-unionfs@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v2 1/1] OverlayFS: Fix checking permissions during lookup.

On Fri, Feb 26, 2016 at 02:41:43PM -0500, thus spake Vivek Goyal:
> CCing linux-fsdevel as it is a wider issue.
> 
> 
> On Wed, Feb 24, 2016 at 02:55:52PM +0100, Ignacy Gawędzki wrote:
> > Add alternate lookup_one_len_check function to fs/namei.c which does
> > what lookup_one_len did until now with a boolean argument telling
> > whether to check that the base directory is traversable.  Modify
> > original lookup_one_len function to call the former with true as the
> > last argument.
> > 
> > In function ovl_lookup_real, file fs/overlayfs/super.c, call
> > lookup_one_len_check with false as the last argument, so that failure
> > to traverse the base directory does not return -EACCES.  This should
> > make lookup resolution work properly in the following setup
> > 
> >   drwxr-xr-x lower/
> >   drwx------ lower/foo/
> >   drw-r--r-- lower/boo/bar
> >   drwxr-xr-x upper/
> >   drwxr-xr-x upper/foo/
> > 
> > when any user not being the owner of lower/foo is trying to access
> > foo/bar in the mounted overlay.
> 
> So what's the problem we are trying to solve. Why should we able to
> override the DAC checks of lower layer if same directory in upper
> is searchable for user but it is not searchable in lower layer. 

My point is that an overlay filesystem should have consistent
semantics.  The current state of affairs fails in this regard on two
points.  First, suppose you have the same lower as above, but upper is
empty.  Then initially, foo/ has permissions 700 and shouldn't be
traversable by anyone but user root and the owner of foo/.  But if user
root or the owner changes foo's permissions to 755, a foo directory is
created in the upper layer and we arrive at the exact same
configuration as above.  In this case, I don't think anyone would
expect other users be denied traversal of foo to access bar.  Second,
the state of the cache shouldn't have any influence on the way access
rights are enforced.  In the present case, traversal of foo is granted
to other users depending on whether the owner or root already accessed
bar.

Besides, according to Documentation/fs/overlay.txt:

  Only the lists of names from directories are merged.  Other content
  such as metadata and extended attributes are reported for the upper
  directory only.  These attributes of the lower directory are hidden.

which implies that lower's permissions of foo should be ignored if foo
exists in upper as well.

Cheers,

Ignacy

-- 
Ignacy Gawędzki
R&D Engineer
Green Communications