Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 14 Nov 2022 15:31:45 +0100
From:   Joel Granados <j.granados@samsung.com>
To:     Paul Moore <paul@paul-moore.com>
CC:     Jeffrey Vander Stoep <jeffv@google.com>,
        Gil Cukierman <cukie@google.com>, Jens Axboe <axboe@kernel.dk>,
        Pavel Begunkov <asml.silence@gmail.com>,
        James Morris <jmorris@namei.org>,
        "Serge E. Hallyn" <serge@hallyn.com>,
        Stephen Smalley <stephen.smalley.work@gmail.com>,
        Eric Paris <eparis@parisplace.org>, <kernel-team@android.com>,
        <linux-kernel@vger.kernel.org>, <io-uring@vger.kernel.org>,
        <linux-security-module@vger.kernel.org>, <selinux@vger.kernel.org>
Subject: Re: [PATCH v1 0/2] Add LSM access controls for io_uring_setup
Message-ID: <20221114143145.ha22rdxphhpgd53u@localhost>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg="pgp-sha512";
        protocol="application/pgp-signature"; boundary="2ctr4pslrx4mhrsa"
Content-Disposition: inline
In-Reply-To: <CAHC9VhRTWGuiMpJJiFrUpgsm7nQaNA-n1CYRMPS-24OLvzdA2A@mail.gmail.com>
CMS-TYPE: 201P
References: <20221107205754.2635439-1-cukie@google.com>
        <CAHC9VhTLBWkw2XzqdFx1LFVKDtaAL2pEfsmm+LEmS0OWM1mZgA@mail.gmail.com>
        <CABXk95ChjusTneWJgj5a58CZceZv0Ay-P-FwBcH2o4rO0g2Ggw@mail.gmail.com>
        <CAHC9VhRTWGuiMpJJiFrUpgsm7nQaNA-n1CYRMPS-24OLvzdA2A@mail.gmail.com>
        <CGME20221114143147eucas1p1902d9b4afc377fdda25910a5d083e3dc@eucas1p1.samsung.com>
Precedence: bulk

--2ctr4pslrx4mhrsa
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Nov 10, 2022 at 04:04:46PM -0500, Paul Moore wrote:
> On Thu, Nov 10, 2022 at 12:54 PM Jeffrey Vander Stoep <jeffv@google.com> =
wrote:
> > On Mon, Nov 7, 2022 at 10:17 PM Paul Moore <paul@paul-moore.com> wrote:
> > >
> > > On Mon, Nov 7, 2022 at 3:58 PM Gil Cukierman <cukie@google.com> wrote:
> > > >
> > > > This patchset provides the changes required for controlling access =
to
> > > > the io_uring_setup system call by LSMs. It does this by adding a new
> > > > hook to io_uring. It also provides the SELinux implementation for a=
 new
> > > > permission, io_uring { setup }, using the new hook.
> > > >
> > > > This is important because existing io_uring hooks only support limi=
ting
> > > > the sharing of credentials and access to the sensitive uring_cmd fi=
le
> > > > op. Users of LSMs may also want the ability to tightly control which
> > > > callers can retrieve an io_uring capable fd from the kernel, which =
is
> > > > needed for all subsequent io_uring operations.
> > >
> > > It isn't immediately obvious to me why simply obtaining a io_uring fd
> > > from io_uring_setup() would present a problem, as the security
> > > relevant operations that are possible with that io_uring fd *should*
> > > still be controlled by other LSM hooks.  Can you help me understand
> > > what security issue you are trying to resolve with this control?
> >
> > I think there are a few reasons why we want this particular hook.
> >
> > 1.  It aligns well with how other resources are managed by selinux
> > where access to the resource is the first control point (e.g. "create"
> > for files, sockets, or bpf_maps, "prog_load" for bpf programs, and
> > "open" for perf_event) and then additional functionality or
> > capabilities require additional permissions.
>=20
> [NOTE: there were two reply sections in your email, and while similar,
> they were not identical; I've trimmed the other for the sake of
> clarity]
>=20
> The resources you mention are all objects which contain some type of
> information (either user data, configuration, or program
> instructions), with the resulting fd being a handle to those objects.
> In the case of io_uring the fd is a handle to the io_uring
> interface/rings, which by itself does not contain any information
> which is not already controlled by other permissions.
>=20
> I/O operations which transfer data between the io_uring buffers and
> other system objects, e.g. IORING_OP_READV, are still subject to the
> same file access controls as those done by the application using
> syscalls.  Even the IORING_OP_OPENAT command goes through the standard
> VFS code path which means it will trigger the same access control
> checks as an open*() done by the application normally.
>=20
> The 'interesting' scenarios are those where the io_uring operation
> servicing credentials, aka personalities, differ from the task
> controlling the io_uring.  However in those cases we have the new
> io_uring controls to gate these delegated operations.  Passing an
> io_uring fd is subject to the fd/use permission like any other fd.
>=20
> Although perhaps the most relevant to your request is the fact that
> the io_uring inode is created using the new(ish) secure anon inode
> interface which ensures that the creating task has permission to
> create an io_uring.  This io_uring inode label also comes into play
> when a task attempts to mmap() the io_uring rings, a critical part of
> the io_uring API.
>=20
> If I'm missing something you believe to be important, please share the de=
tails.
>=20
> > 2. It aligns well with how resources are managed on Android. We often
> > do not grant direct access to resources (like memory buffers).
>=20
> Accessing the io_uring buffers requires a task to mmap() the io_uring
> fd which is controlled by the normal SELinux mmap() access controls.
>=20
> > 3. Attack surface management. One of the primary uses of selinux on
> > Android is to assess and limit attack surface (e.g.
> > https://twitter.com/jeffvanderstoep/status/1422771606309335043) . As
> > io_uring vulnerabilities have made their way through our vulnerability
> > management system, it's become apparent that it's complicated to
> > assess the impact. Is a use-after-free reachable? Creating
> > proof-of-concept exploits takes a lot of time, and often functionality
> > can be reached by multiple paths. How many of the known io_uring
> > vulnerabilities would be gated by the existing checks? How many future
> > ones will be gated by the existing checks? I don't know the answer to
> > either of these questions and it's not obvious. This hook makes that
> > initial assessment simple and effective.
>=20
> It should be possible to deny access to io_uring via the anonymous
> inode labels, the mmap() controls, and the fd/use permission.  If you
> find a way to do meaningful work with an io_uring fd that can't be
> controlled via an existing permission check please let me know.

Also interested in a more specific case. Sending reply so I get added to
the group response.
>=20
> --
> paul-moore.com

--2ctr4pslrx4mhrsa
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQGzBAABCgAdFiEErkcJVyXmMSXOyyeQupfNUreWQU8FAmNyUUoACgkQupfNUreW
QU+inAv/do7MKKHTSeQFTr+qTo1NYVoXryjw9rfz3xinweQV5XNVCvmzY1OO7Gkk
pnDb+FOXdVjDrp97L5lQfHM7jRsJY2Ia8x1KYW+zhn79rxH9BQxWs646Q6g7ORJz
6C9NlOMyoBIgssRaPqLMRe8E9ewIzfhgLpjn4yI23kl7uDkyVStGLTZtCarWTMi2
ffFN8DFmZm3hm42LUKElGqSlrDxR5eWDGBQYBknkFHwsHKj5C/cL1ch020decZwB
JSponYgw4OIAslxF+nWhTtIFBk40m+ji4QuxZv16bVxy1ei4yQayZYHelnT/Swvv
j4BR2S0ifeBp5HEprMRpPBKMxRe045J0IcHsDlg6d5Sz1039yLGcNySQnQwOND+t
dsVYv3f//2NIJF3eThgBi+KPAQS33USVk9qZujIFdcESRYfIoL0uA8el8TOop2gM
DaFkBKQqNrq3s01NUfuTrOoK7wrvalgngev67gVv0TAEQQebaWZovjgR/NHl16nI
ywSdStI+
=aESw
-----END PGP SIGNATURE-----

--2ctr4pslrx4mhrsa--