Received-SPF: pass (google.com: domain of linux-kernel+bounces-157887-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240423110148.13114-1-stsp2@yandex.ru> <4D2A1543-273F-417F-921B-E9F994FBF2E8@amacapital.net>
 <0e2e48be-86a8-418c-95b1-e8ca17469198@yandex.ru>
In-Reply-To: <0e2e48be-86a8-418c-95b1-e8ca17469198@yandex.ru>
From: Andy Lutomirski <luto@amacapital.net>
Date: Wed, 24 Apr 2024 17:43:02 -0700
Message-ID: <CALCETrWswr5jAzD9BkdCqLX=d8vReO8O9dVmZfL7HXdvwkft9g@mail.gmail.com>
Subject: Re: [PATCH v2 0/2] implement OA2_INHERIT_CRED flag for openat2()
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>, Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>, 
	Jeff Layton <jlayton@kernel.org>, Chuck Lever <chuck.lever@oracle.com>, 
	Alexander Aring <alex.aring@gmail.com>, linux-fsdevel@vger.kernel.org, 
	linux-api@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>, 
	=?UTF-8?Q?Christian_G=C3=B6ttsche?= <cgzones@googlemail.com>, 
	Aleksa Sarai <cyphar@cyphar.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Apr 24, 2024 at 3:57=E2=80=AFAM stsp <stsp2@yandex.ru> wrote:
>
> 23.04.2024 19:44, Andy Lutomirski =D0=BF=D0=B8=D1=88=D0=B5=D1=82:
> >> On Apr 23, 2024, at 4:02=E2=80=AFAM, Stas Sergeev <stsp2@yandex.ru> wr=
ote:
> >>
> >> =EF=BB=BFThis patch-set implements the OA2_INHERIT_CRED flag for opena=
t2() syscall.
> >> It is needed to perform an open operation with the creds that were in
> >> effect when the dir_fd was opened. This allows the process to pre-open
> >> some dirs and switch eUID (and other UIDs/GIDs) to the less-privileged
> >> user, while still retaining the possibility to open/create files withi=
n
> >> the pre-opened directory set.
> > I like the concept, as it=E2=80=99s a sort of move toward a capability =
system. But I think that making a dirfd into this sort of capability would =
need to be much more explicit. Right now, any program could do this entirel=
y by accident, and applying OA2_INHERIT_CRED to an fd fished out of /proc s=
eems hazardous.
>
> While I still don't quite understand
> the threat of /proc symlinks, I posted
> v4 which disallows them.
>

I like that, but you're blocking it the wrong way.  My concern is that
someone does dfd =3D open("/proc/PID/fd/3") and then openat(dfd, ...,
OA2_INHERIT_CRED);  IIRC open("/proc/PID/fd/3") is extremely magical
and returns the _same open file description_ (struct file) as PID's fd
3.

> > So perhaps if an open file description for a directory could have somet=
hing like FMODE_CRED, and if OA2_INHERIT_CRED also blocked .., magic links,=
 symlinks to anywhere above the dirfd (or maybe all symlinks) and absolute =
path lookups, then this would be okay.
>
> So I think this all is now done.

But you missed the FMODE_CRED part!

So here's the problem: right now, in current Linux, a dirfd pointing
to a directory that you can open anyway doesn't convey any new powers.
So, if I'm a regular program, and I do open("/etc", O_PATH), I get an
fd.  And if I get an fd pointing at /etc from somewhere else, I get
the same thing (possibly with different f_cred, but f_cred is largely
a hack to restrict things that would otherwise be insecure because
they were designed a bit wrong from the beginning).

But, with your patch, these fds suddenly convey a very strong
privilege: that of their f_cred *over the entire subtree to which they
refer*.  And you can attack it using exactly your intended use case:
if any program opens a dirfd and then drops privileges, well, oops, it
didn't actually fully drop privilege.

So I think that, if this whole concept has any chance of working well,
it needs to be opt-in *at the time of the original open*.  So a
privilege-carrying open would be an entirely new option like
O_CAPTURE_CREDS or FMODE_CREDS.  And OA2_INHERIT_CREDS is rejected if
the dirfd doesn't have that special mode.