Received-SPF: pass (google.com: domain of linux-kernel+bounces-158506-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Thu, 25 Apr 2024 14:08:24 +0200
From: Christian Brauner <brauner@kernel.org>
To: stsp <stsp2@yandex.ru>
Cc: linux-kernel@vger.kernel.org, Stefan Metzmacher <metze@samba.org>, 
	Eric Biederman <ebiederm@xmission.com>, Alexander Viro <viro@zeniv.linux.org.uk>, 
	Andy Lutomirski <luto@kernel.org>, Jan Kara <jack@suse.cz>, Jeff Layton <jlayton@kernel.org>, 
	Chuck Lever <chuck.lever@oracle.com>, Alexander Aring <alex.aring@gmail.com>, 
	David Laight <David.Laight@aculab.com>, linux-fsdevel@vger.kernel.org, linux-api@vger.kernel.org, 
	Paolo Bonzini <pbonzini@redhat.com>, Christian =?utf-8?B?R8O2dHRzY2hl?= <cgzones@googlemail.com>
Subject: Re: [PATCH v4 0/2] implement OA2_INHERIT_CRED flag for openat2()
Message-ID: <20240425-loslassen-hexen-a1664a579ea1@brauner>
References: <20240424105248.189032-1-stsp2@yandex.ru>
 <20240424-schummeln-zitieren-9821df7cbd49@brauner>
 <6b46528a-965f-410a-9e6f-9654c5e9dba2@yandex.ru>
 <20240425-ausfiel-beabsichtigen-a2ef9126ebda@brauner>
 <df51f2fd-385a-47bf-a072-a8988a801d52@yandex.ru>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <df51f2fd-385a-47bf-a072-a8988a801d52@yandex.ru>

> But I am sure you still don't understand
> what exactly the patch does, so why not
> to ask instead of asserting?
> You say uid/gid can be stolen, but no,
> it can't: the creds are reverted. Only
> fsuid/fsgid (and caps in v2 of the patch)
> actually affect openat2(), but nothing is
> "leaked" after openat2() finished.

I say "stolen" because the original opener has no say in this. You're
taking their fsuid/fsgid and groups and overriding creds for the
duration of the lookup and open. Something the original opener never
consented to. But let's call it "borrowed" if you're hung up on
terminology here.

But ultimately it's the same complaint: the original opener has no way
of controlling this behavior. Once ignored in my first reply, and now
again conveniently cut off again. Let alone all the other objections.

And fwiw, the same way you somehow feel like I haven't read your patch
it seems you to consistently underestimate the implications of this
change. Which is really strange because it's pretty obvious. It's
effectively temporary setuid/setgid with some light limitations.

Leaking directory descriptors across security boundaries becomes a lot
more interesting with this patch applied. Something which has happened
multiple times already and heavily figures in container escapes. And the
RESOLVE_BENEATH/IN_ROOT restrictions only stave off symlink (and magic
link) attacks. If a directory fd is leaked a container can take the
fsuid/fsgid/groups from the original opener of that directory and write
to disk with whatever it resolves to in that namespace's idmapping. It's
basically a nice way to puzzle together arbitrary fsuid/fsgid and
idmapping pairs in whatever namespace the opener happens to be in.

And again it also messes with LSMs because you're changing credentials
behind their back.

And to the "unsuspecting userspace" point you dismissed earlier.
Providing a dirfd to a lesser privileged process isn't horrendously
dangerous right now. But with this change it sure is. For stuff like
libpathrs or systemd's fdstore this change has immediate security
implications.