Linux-Fsdevel Archive mirror
 help / color / mirror / Atom feed
From: stsp <stsp2@yandex.ru>
To: Christian Brauner <brauner@kernel.org>
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 Göttsche" <cgzones@googlemail.com>
Subject: Re: [PATCH v4 0/2] implement OA2_INHERIT_CRED flag for openat2()
Date: Thu, 25 Apr 2024 15:39:54 +0300	[thread overview]
Message-ID: <8b136077-af77-4371-9e67-7ae339efc3c1@yandex.ru> (raw)
In-Reply-To: <20240425-loslassen-hexen-a1664a579ea1@brauner>

25.04.2024 15:08, Christian Brauner пишет:
>> 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.

The initial idea was to keep this all
within a single-process boundary. It
wasn't coded that way though. :(


>   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.

Not a terminology: you were explicitly
talking about uid/gid, blaming a v2 of
my patch. But v2 was not any more
harmful than others and uid/gid cannot
be leaked even there.
But I don't mind: if now you realize v2
is not a leak for uid/gid, then we are on
the same track.

> But ultimately it's the same complaint: the original opener has no way
> of controlling this behavior.

It wasn't clear if the opener should
control that behaviour, or maybe instead
that all should be limited within a single
process?
Andy Lutomirski's explanation made it
clear that even if the openers are the
same, the control is still needed. So now
this argument is undeniable.


>   Once ignored in my first reply, and now
> again conveniently cut off again. Let alone all the other objections.

Sorry but your complains were about
stealing uid/gid in v2, which were clearly
wrong. But this is a matter of the past.

> 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.

Temporary cred override is quite common
within the current code AFAICS when I grep
it for override_creds() call.

> 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.

Yes, so the opt-in flag is undeniably needed.

> 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.

So am I getting your point correctly:
- process A uses the opt-in flag (eg O_CAPTURE_CREDS)
   and passes the fd to "unsuspecting userspace" B.
- process B is not going to use O_INHERIT_CREDS,
   but it now still can't drop his privs fully.

Is this what you mean?


      reply	other threads:[~2024-04-25 12:40 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-24 10:52 [PATCH v4 0/2] implement OA2_INHERIT_CRED flag for openat2() Stas Sergeev
2024-04-24 10:52 ` [PATCH 1/2] fs: reorganize path_openat() Stas Sergeev
2024-04-25  8:13   ` kernel test robot
2024-04-24 10:52 ` [PATCH 2/2] openat2: add OA2_INHERIT_CRED flag Stas Sergeev
2024-04-25  2:31   ` Al Viro
2024-04-25  7:24     ` stsp
2024-04-25  9:23     ` stsp
2024-04-25 13:50   ` kernel test robot
2024-04-25 14:02   ` Christian Brauner
2024-04-26 13:36     ` stsp
2024-04-24 16:09 ` [PATCH v4 0/2] implement OA2_INHERIT_CRED flag for openat2() Christian Brauner
2024-04-24 17:50   ` stsp
2024-04-25  9:54     ` Christian Brauner
2024-04-25 10:12       ` stsp
2024-04-25 12:08         ` Christian Brauner
2024-04-25 12:39           ` stsp [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8b136077-af77-4371-9e67-7ae339efc3c1@yandex.ru \
    --to=stsp2@yandex.ru \
    --cc=David.Laight@aculab.com \
    --cc=alex.aring@gmail.com \
    --cc=brauner@kernel.org \
    --cc=cgzones@googlemail.com \
    --cc=chuck.lever@oracle.com \
    --cc=ebiederm@xmission.com \
    --cc=jack@suse.cz \
    --cc=jlayton@kernel.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=metze@samba.org \
    --cc=pbonzini@redhat.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).