From:   Steve Grubb <sgrubb@redhat.com>
To:     Richard Guy Briggs <rgb@redhat.com>,
        Paul Moore <paul@paul-moore.com>
Cc:     Linux-Audit Mailing List <linux-audit@redhat.com>,
        LKML <linux-kernel@vger.kernel.org>, io-uring@vger.kernel.org,
        Eric Paris <eparis@parisplace.org>, Stefan Roesch <shr@fb.com>,
        Christian Brauner <brauner@kernel.org>,
        Jens Axboe <axboe@kernel.dk>,
        Pavel Begunkov <asml.silence@gmail.com>
Subject: Re: [PATCH v1 2/2] io_uring,audit: do not log IORING_OP_*GETXATTR
Date:   Sat, 28 Jan 2023 12:26:31 -0500
Message-ID: <13202484.uLZWGnKmhe@x2>
Organization: Red Hat
In-Reply-To: <CAHC9VhQiy9vP7BdQk+SXG7gQKAqOAqbYtU+c9R0_ym0h4bgG7g@mail.gmail.com>
References: <cover.1674682056.git.rgb@redhat.com>
 <f602429ce0f419c2abc3ae5a0e705e1368ac5650.1674682056.git.rgb@redhat.com>
 <CAHC9VhQiy9vP7BdQk+SXG7gQKAqOAqbYtU+c9R0_ym0h4bgG7g@mail.gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

On Friday, January 27, 2023 5:43:02 PM EST Paul Moore wrote:
> On Fri, Jan 27, 2023 at 12:24 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > Getting XATTRs is not particularly interesting security-wise.
> > 
> > Suggested-by: Steve Grubb <sgrubb@redhat.com>
> > Fixes: a56834e0fafe ("io_uring: add fgetxattr and getxattr support")
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> > io_uring/opdef.c | 2 ++
> > 1 file changed, 2 insertions(+)
> 
> Depending on your security policy, fetching file data, including
> xattrs, can be interesting from a security perspective.  As an
> example, look at the SELinux file/getattr permission.
> 
> https://github.com/SELinuxProject/selinux-notebook/blob/main/src/object_cla
> sses_permissions.md#common-file-permissions

We're mostly interested in setting attributes because that changes policy. 
Reading them is not interesting unless the access fails with EPERM.

I was updating the user space piece recently and saw there was a bunch of 
"new" operations. I was commenting that we need to audit 5 or 6 of the "new" 
operations such as IORING_OP_MKDIRATor IORING_OP_SETXATTR. But now that I see 
the patch, it looks like they are auditable and we can just let a couple be 
skipped. IORING_OP_MADVISE is not interesting as it just gives hiints about 
the expected access patterns of memory. If there were an equivalent of 
mprotect, that would be of interest, but not madvise.

There are some I'm not sure about such as IORING_OP_MSG_RING and 
IORING_OP_URING_CMD. What do they do?

-Steve