linux-kernel - Re: [PATCH] vfs: relax linkat() AT_EMPTY_PATH - aka flink()

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: Fri, 12 Apr 2024 08:41:09 +0200
From: Christian Brauner <brauner@...nel.org>
To: Charles Mirabile <cmirabil@...hat.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>, 
	Alexander Viro <viro@...iv.linux.org.uk>, Jan Kara <jack@...e.cz>, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Andrew Lutomirski <luto@...nel.org>, Peter Anvin <hpa@...or.com>
Subject: Re: [PATCH] vfs: relax linkat() AT_EMPTY_PATH - aka flink() -
 requirements

On Thu, Apr 11, 2024 at 12:44:46PM -0400, Charles Mirabile wrote:
> On Thu, Apr 11, 2024 at 12:15 PM Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
> >
> > On Thu, 11 Apr 2024 at 02:05, Christian Brauner <brauner@...nel.org> wrote:
> > >
> > > I had a similar discussion a while back someone requested that we relax
> > > permissions so linkat can be used in containers.
> >
> > Hmm.
> >
> > Ok, that's different - it just wants root to be able to do it, but
> > "root" being just in the container itself.
> >
> > I don't think that's all that useful - I think one of the issues with
> > linkat(AT_EMPTY_PATH) is exactly that "it's only useful for root",
> > which means that it's effectively useless. Inside a container or out.
> >
> > Because very few loads run as root-only (and fewer still run with any
> > capability bits that aren't just "root or nothing").
> >
> > Before I did all this, I did a Debian code search for linkat with
> > AT_EMPTY_PATH, and it's almost non-existent. And I think it's exactly
> > because of this "when it's only useful for root, it's hardly useful at
> > all" issue.
> >
> > (Of course, my Debian code search may have been broken).
> >
> > So I suspect your special case is actually largely useless, and what
> > the container user actually wanted was what my patch does, but they
> > didn't think that was possible, so they asked to just extend the
> > "root" notion.
> >
> Yes, that is absolutely the case. When Christian poked holes in my
> initial submission, I started working on a v2 but haven't had a chance
> to send it because I wanted to make sure my arguments etc were
> airtight because I am well aware of just how long and storied the
> history of flink is. In the v2 that I didn't post yet, I extended the
> ability to link *any* file from only true root to also "root" within a
> container following the potentially suspect approach that christian
> suggested (I see where you are coming from with the unobvious approach

I'm sorry, what is suspect about my approach? Your patch in [1] lowered
the capability checks to ns_capable(current_user_ns(), CAP_DAC_READ_SEARCH).
That's very much wrong because it means root in an unprivileged
container could linkat() any file descriptor including any opened by
real root. The permission needs to be tied to the opener of the file.
Otherwise that's guaranteed to break security assumptions. Whereas my
patch avoids that.

[1]: https://lore.kernel.org/all/20231110170615.2168372-2-cmirabil@redhat.com