Re: [AppArmor 19/45] Add struct vfsmount parameters to vfs_rename()

[Bodo Eggert <7eggert@gmx.de>]

Al Viro <viro@ftp.linux.org.uk> wrote:
> On Fri, Oct 26, 2007 at 11:23:53AM -0700, John Johansen wrote:

>> In the current code, both vfsmounts are always identical, and so one of
>> the two should go, agreed.
>> 
>> The thought behind passing both vfsmounts was that they could differ but
>> point to the same super_block, in which case renames would still be
>> possible at least from a filesystem point of view. The essential
>> restriction here is that both files must be on the same device; the vfs
>> restriction of not allowing cross-mount renames is arbitrary.
> 
> It's called "access control".  Pathname-based one, BTW.  And yes, it's
> 100% deliberate.

I doubt anybody uses bind mounts & co instead of symlinks in order to
prevent rename() while still allowing to move files by either copying
or by using the source file in the bound directory. At least I expected
bind mounted directories to behave like symlinked ones, minus the problems
of symlinks. 

Since this feature only protects you from rename(src/foo,dst/foo) if
1) There is no way to access src and dst in the same mount space
2) src and dst are writebale by the attacker
3) Unlinking src/foo is OK
4) Renaming src/foo is OK as long as it's within the same mount as foo
5) Symlinking src/foo to dst/foo is OK
6) Creating dst/foo having a different owner is OK
7) Having dst/foo with the original content and owner from src/foo is _not_ OK
8) Moon crashes on earth
, I'd rather like to have a fast mv.

>> Cross-mount renames are not allowed currently, and granted, they may not
>> be very useful, either.
> 
> <raised brows>
> Excuse me, but IIRC LSM was supposed to _add_ restrictions, not to remove
> existing security checks.

Security checks as in "we built a steel door into that Chinese paper wall"?

As far as I understand, the restriction would not be removed by the LSM
explicitely allowing it, but by the fixed vfs then being able to handle
cross-mountpoint-renames. Maybe yo'll want to keep the ability for the users
who use bind mounts in order to not allow rename() ... both of them.-)

/me prepares for the impact of a large round object on earth.