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?
