From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0AD05C47093 for ; Wed, 2 Jun 2021 01:40:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E0D1B613DD for ; Wed, 2 Jun 2021 01:40:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230090AbhFBBl7 (ORCPT ); Tue, 1 Jun 2021 21:41:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35010 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230143AbhFBBl6 (ORCPT ); Tue, 1 Jun 2021 21:41:58 -0400 Received: from mail-ej1-x62b.google.com (mail-ej1-x62b.google.com [IPv6:2a00:1450:4864:20::62b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E0978C061756 for ; Tue, 1 Jun 2021 18:40:15 -0700 (PDT) Received: by mail-ej1-x62b.google.com with SMTP id l1so1350315ejb.6 for ; Tue, 01 Jun 2021 18:40:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=paul-moore-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=7+6GengPTpnBNKRiprYjAxUmTuo80Mwv0XF47zfhAB0=; b=eHaMWPqA67Ms9cMf+MpCDlqXpy7gUU+QZLG79G7lMeuxyBVPP6W+UzN3hICWjKjiBJ 44peCH0PgM1siFi8Gc4aqT8FiLU/ehdSIKr0nBCH1uOLEJoZzDBKL2/MsPttMhJSGqeL heKa7CKjkZvPwSBHdRp7V8pLoZ3NIKPFNnJLrybRmV83gMVHIMG9st6DySEqrhg+/2nr Nt1dcVVMckoCjW6RMFbLosyOhYo5n1uhlUEhS/qG6jVenahGHkyqBFBNFyuZH/lDrEXZ Ufysh0UsjrPKikT6wQB8O9OUwbdDrkFBQIdmhfg06tut+RW2/WDitMIf6tve/CvEgA2O 1jbg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=7+6GengPTpnBNKRiprYjAxUmTuo80Mwv0XF47zfhAB0=; b=bByOGdwMNJ1DxWQIBBaGs4QY1XY45WhDrOrCsBeAosk2XXrOkXLV6FjGhHev3gwio2 a4bMJoiJML5Xpe5OBZ00VHeesCMl8ysmBDaVcGxsyRGM0h1q16FF3Ahu/0VKhYllU7LQ sch4tLjGK+gbZPQE7gq1F5BhVrZ92tZveWQgwSra/qIWJRP54eu3cYynEY07jK/VAroT TAAjCEfe755FbHPXQyX+fKNXN74/mURuxDTmFVeZc4VsRCSKTfLaXAfraBPBBEr2YNgy AW/zd5ezKPWbJcEgCYkMyCOjGvRWvw/54+cYvuc6C1GBqtEqTn7m+WanCUl+haEcLsDv b9rA== X-Gm-Message-State: AOAM530euP5JdqHZ7+1rukVqiXtMFKFMK/egLj0LZLZ/A/MWFjXQjqnp 51v2zWW/+VZQqCox9o/+HM0h29KaMaEjDOle9GHpm+YMMA== X-Google-Smtp-Source: ABdhPJzKGdNVsjgTej2xcBpuoKPd2C3Oqdgb3f4h5vrwkEShRjjAx36JJyUkFtyB1rEm/0O5DJTEmL545Nl/g7Jerlg= X-Received: by 2002:a17:906:2c54:: with SMTP id f20mr14631744ejh.91.1622598014322; Tue, 01 Jun 2021 18:40:14 -0700 (PDT) MIME-Version: 1.0 References: <162163367115.8379.8459012634106035341.stgit@sifl> <162163380685.8379.17381053199011043757.stgit@sifl> <20210528223544.GL447005@madcap2.tricolour.ca> <20210531134408.GL2268484@madcap2.tricolour.ca> In-Reply-To: <20210531134408.GL2268484@madcap2.tricolour.ca> From: Paul Moore Date: Tue, 1 Jun 2021 21:40:03 -0400 Message-ID: Subject: Re: [RFC PATCH 4/9] audit: add filtering for io_uring records To: Richard Guy Briggs Cc: linux-security-module@vger.kernel.org, selinux@vger.kernel.org, linux-audit@redhat.com, io-uring@vger.kernel.org, linux-fsdevel@vger.kernel.org, Kumar Kartikeya Dwivedi , Jens Axboe , Alexander Viro Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Mon, May 31, 2021 at 9:44 AM Richard Guy Briggs wrote: > On 2021-05-30 11:26, Paul Moore wrote: > > On Fri, May 28, 2021 at 6:36 PM Richard Guy Briggs wrote: > > > On 2021-05-21 17:50, Paul Moore wrote: ... > > > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > > > > index d8aa2c690bf9..4f6ab34020fb 100644 > > > > --- a/kernel/auditsc.c > > > > +++ b/kernel/auditsc.c > > > > @@ -799,6 +799,35 @@ static int audit_in_mask(const struct audit_krule *rule, unsigned long val) > > > > return rule->mask[word] & bit; > > > > } > > > > > > > > +/** > > > > + * audit_filter_uring - apply filters to an io_uring operation > > > > + * @tsk: associated task > > > > + * @ctx: audit context > > > > + */ > > > > +static void audit_filter_uring(struct task_struct *tsk, > > > > + struct audit_context *ctx) > > > > +{ > > > > + struct audit_entry *e; > > > > + enum audit_state state; > > > > + > > > > + if (auditd_test_task(tsk)) > > > > + return; > > > > > > Is this necessary? auditd and auditctl don't (intentionally) use any > > > io_uring functionality. Is it possible it might inadvertantly use some > > > by virtue of libc or other library calls now or in the future? > > > > I think the better question is what harm does it do? Yes, I'm not > > aware of an auditd implementation that currently makes use of > > io_uring, but it is also not inconceivable some future implementation > > might want to make use of it and given the disjoint nature of kernel > > and userspace development I don't want the kernel to block such > > developments. However, if you can think of a reason why having this > > check here is bad I'm listening (note: we are already in the slow path > > here so having the additional check isn't an issue as far as I'm > > concerned). > > > > As a reminder, auditd_test_task() only returns true/1 if the task is > > registered with the audit subsystem as an auditd connection, an > > auditctl process should not cause this function to return true. > > My main concern was overhead, since the whole goal of io_uring is speed. At the point where this test takes place we are already in the audit slow path as far as io_uring is concerned. I understand your concern, but the advantage of being able to effectively use io_uring in the future makes this worth keeping in my opinion. > The chances that audit does use this functionality in the future suggest > to me that it is best to leave this check in. Sounds like we are in agreement. We'll keep it for now. > > > > + rcu_read_lock(); > > > > + list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_URING_EXIT], > > > > + list) { > > > > + if (audit_in_mask(&e->rule, ctx->uring_op) && > > > > > > While this seems like the most obvious approach given the parallels > > > between syscalls and io_uring operations, as coded here it won't work > > > due to the different mappings of syscall numbers and io_uring > > > operations unless we re-use the auditctl -S field with raw io_uring > > > operation numbers in the place of syscall numbers. This should have > > > been obvious to me when I first looked at this patch. It became obvious > > > when I started looking at the userspace auditctl.c. > > > > FWIW, my intention was to treat io_uring opcodes exactly like we treat > > syscall numbers. Yes, this would potentially be an issue if we wanted > > to combine syscalls and io_uring opcodes into one filter, but why > > would we ever want to do that? Combining the two into one filter not > > only makes the filter lists longer than needed (we will always know if > > we are filtering on a syscall or io_uring op) and complicates the > > filter rule processing. > > > > Or is there a problem with this that I'm missing? > > No, I think you have a good understanding of it. I'm asking hard > questions to avoid missing something important. If we can reuse the > syscall infrastructure for this then that is extremely helpful (if not > lazy, which isn't necessarily a bad thing). It does mean that the > io_uring op dictionary will need to live in userspace audit the way it > is currently implemented .... Which I currently believe is the right thing to do. > > > The easy first step would be to use something like this: > > > auditctl -a uring,always -S 18,28 -F key=uring_open > > > to monitor file open commands only. The same is not yet possible for > > > the perm field, but there are so few io_uring ops at this point compared > > > with syscalls that it might be manageable. The arch is irrelevant since > > > io_uring operation numbers are identical across all hardware as far as I > > > can tell. Most of the rest of the fields should make sense if they do > > > for a syscall rule. > > > > I've never been a fan of audit's "perm" filtering; I've always felt > > there were better ways to handle that so I'm not overly upset that we > > are skipping that functionality with this initial support. If it > > becomes a problem in the future we can always add that support at a > > later date. > > Ok, I don't see a pressing need to add it initially, but should add a > check to block that field from being used to avoid the confusion of > unpredictable behaviour should someone try to add a perm filter to a > io_uring filter. That should be done protectively in the kernel and > proactively in userspace. Sure, that's reasonable. > > > Here's a sample of userspace code to support this > > > patch: > > > https://github.com/rgbriggs/audit-userspace/commit/a77baa1651b7ad841a220eb962d4cc92bc07dc96 > > > https://github.com/linux-audit/audit-userspace/compare/master...rgbriggs:ghau-iouring-filtering.v1.0 > > > > Great, thank you. I haven't grabbed a copy yet for testing, but I will. > > I've added a perm filter block as an additional patch in userspace and > updated the tree so that first commit is no longer the top of tree but > the branch name is current. > > I'll add a kernel perm filter check. > > I just noticed some list checking that is missing in tree and watch in > your patch. > > Suggested fixup patches to follow... I see them, thank you, comments will follow over there. Although to be honest I'm mostly focusing on the testing right now while we wait to hear back from Jens on what he is willing to accept regarding audit calls in io_issue_sqe(). If we can't do the _entry()/_exit() calls then this work is pretty much dead and we just have to deal with it in Kconfig. I might make one last, clean patchset and put it in a branch for the distros that want to carry the patchset, but it isn't clear to me that it is something I would want to maintain long term. Long running out of tree patches are generally A Bad Idea. > > > If we abuse the syscall infrastructure at first, we'd need a transition > > > plan to coordinate user and kernel switchover to seperate mechanisms for > > > the two to work together if the need should arise to have both syscall > > > and uring filters in the same rule. > > > > See my comments above, I don't currently see why we would ever want > > syscall and io_uring filtering to happen in the same rule. Please > > speak up if you can think of a reason why this would either be needed, > > or desirable for some reason. > > I think they can be seperate rules for now. Either a syscall rule > catching all io_uring ops can be added, or an io_uring rule can be added > to catch specific ops. The scenario I was thinking of was catching > syscalls of specific io_uring ops. Perhaps I'm misunderstand you, but that scenario really shouldn't exist. The io_uring ops function independently of syscalls; you can *submit* io_uring ops via io_uring_enter(), but they are not guaranteed to be dispatched synchronously (obviously), and given the cred shenanigans that can happen with io_uring there is no guarantee the filters would even be applicable. It isn't an issue of "can" the filters be separate, they *have* to be separate. -- paul moore www.paul-moore.com From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 254F6C47080 for ; Wed, 2 Jun 2021 01:40:32 +0000 (UTC) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A5739613CD for ; Wed, 2 Jun 2021 01:40:31 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A5739613CD Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=paul-moore.com Authentication-Results: mail.kernel.org; spf=tempfail smtp.mailfrom=linux-audit-bounces@redhat.com Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-582-DV6J68jhOKikCXeJv8i8aw-1; Tue, 01 Jun 2021 21:40:29 -0400 X-MC-Unique: DV6J68jhOKikCXeJv8i8aw-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 8B33C107ACCD; Wed, 2 Jun 2021 01:40:24 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 187175D6CF; Wed, 2 Jun 2021 01:40:24 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 4CF9C1800BB8; Wed, 2 Jun 2021 01:40:22 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 1521eK4e020813 for ; Tue, 1 Jun 2021 21:40:20 -0400 Received: by smtp.corp.redhat.com (Postfix) id 95C7021DE6FF; Wed, 2 Jun 2021 01:40:20 +0000 (UTC) Received: from mimecast-mx02.redhat.com (mimecast06.extmail.prod.ext.rdu2.redhat.com [10.11.55.22]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9161721DE6FD for ; Wed, 2 Jun 2021 01:40:18 +0000 (UTC) Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id DB5E5185A79C for ; Wed, 2 Jun 2021 01:40:17 +0000 (UTC) Received: from mail-ej1-f41.google.com (mail-ej1-f41.google.com [209.85.218.41]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-285-mMa-iNAcNReBZ0k0Q9i5uQ-1; Tue, 01 Jun 2021 21:40:15 -0400 X-MC-Unique: mMa-iNAcNReBZ0k0Q9i5uQ-1 Received: by mail-ej1-f41.google.com with SMTP id ce15so1360082ejb.4 for ; Tue, 01 Jun 2021 18:40:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=7+6GengPTpnBNKRiprYjAxUmTuo80Mwv0XF47zfhAB0=; b=cvOC3fHlQVAVpXtPas1s/cm8lI+xtJDhQq6aQYgXXsZum8x7rv1hZH4LM7JCVNczqH E19Qh7zmaNffwZEJABZNZk/vPLzT+p84K3irBtj3w6G+abc2YKOA3klyH3IfR5Hwm7Qy 2ifXx6XNrUX2YONtrWwHT1r8Wh+oJ6K2Eka6pVf32wfn1xdgRdPhXHHnjYAc9AuvUesX QyL08MzsUvBsenM+cwI4mCRTv0cQFtiaMs5Rvxclg2yz4nLCctQY61QV5oQ8DbEmCB/z TTFi6pGTiCWIQVFILNCOvAf096+giZm3zqDvggkrSY6GkBG1qLXyw6I/kSfeQ7ol3yGL 5m/A== X-Gm-Message-State: AOAM531yVBs5sMwl8gjy97Sv35pjZQiN0j9pIOWuGIBg7kiHpzuKQObH GTN6HpBm/TwqaW9QkF/1TKBlc2eY0feqXsb6VhlI X-Google-Smtp-Source: ABdhPJzKGdNVsjgTej2xcBpuoKPd2C3Oqdgb3f4h5vrwkEShRjjAx36JJyUkFtyB1rEm/0O5DJTEmL545Nl/g7Jerlg= X-Received: by 2002:a17:906:2c54:: with SMTP id f20mr14631744ejh.91.1622598014322; Tue, 01 Jun 2021 18:40:14 -0700 (PDT) MIME-Version: 1.0 References: <162163367115.8379.8459012634106035341.stgit@sifl> <162163380685.8379.17381053199011043757.stgit@sifl> <20210528223544.GL447005@madcap2.tricolour.ca> <20210531134408.GL2268484@madcap2.tricolour.ca> In-Reply-To: <20210531134408.GL2268484@madcap2.tricolour.ca> From: Paul Moore Date: Tue, 1 Jun 2021 21:40:03 -0400 Message-ID: Subject: Re: [RFC PATCH 4/9] audit: add filtering for io_uring records To: Richard Guy Briggs X-Mimecast-Impersonation-Protect: Policy=CLT - Impersonation Protection Definition; Similar Internal Domain=false; Similar Monitored External Domain=false; Custom External Domain=false; Mimecast External Domain=false; Newly Observed Domain=false; Internal User Name=false; Custom Display Name List=false; Reply-to Address Mismatch=false; Targeted Threat Dictionary=false; Mimecast Threat Dictionary=false; Custom Threat Dictionary=false X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-loop: linux-audit@redhat.com Cc: Jens Axboe , selinux@vger.kernel.org, linux-security-module@vger.kernel.org, linux-audit@redhat.com, Kumar Kartikeya Dwivedi , linux-fsdevel@vger.kernel.org, io-uring@vger.kernel.org, Alexander Viro X-BeenThere: linux-audit@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Linux Audit Discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=linux-audit-bounces@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On Mon, May 31, 2021 at 9:44 AM Richard Guy Briggs wrote: > On 2021-05-30 11:26, Paul Moore wrote: > > On Fri, May 28, 2021 at 6:36 PM Richard Guy Briggs wrote: > > > On 2021-05-21 17:50, Paul Moore wrote: ... > > > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > > > > index d8aa2c690bf9..4f6ab34020fb 100644 > > > > --- a/kernel/auditsc.c > > > > +++ b/kernel/auditsc.c > > > > @@ -799,6 +799,35 @@ static int audit_in_mask(const struct audit_krule *rule, unsigned long val) > > > > return rule->mask[word] & bit; > > > > } > > > > > > > > +/** > > > > + * audit_filter_uring - apply filters to an io_uring operation > > > > + * @tsk: associated task > > > > + * @ctx: audit context > > > > + */ > > > > +static void audit_filter_uring(struct task_struct *tsk, > > > > + struct audit_context *ctx) > > > > +{ > > > > + struct audit_entry *e; > > > > + enum audit_state state; > > > > + > > > > + if (auditd_test_task(tsk)) > > > > + return; > > > > > > Is this necessary? auditd and auditctl don't (intentionally) use any > > > io_uring functionality. Is it possible it might inadvertantly use some > > > by virtue of libc or other library calls now or in the future? > > > > I think the better question is what harm does it do? Yes, I'm not > > aware of an auditd implementation that currently makes use of > > io_uring, but it is also not inconceivable some future implementation > > might want to make use of it and given the disjoint nature of kernel > > and userspace development I don't want the kernel to block such > > developments. However, if you can think of a reason why having this > > check here is bad I'm listening (note: we are already in the slow path > > here so having the additional check isn't an issue as far as I'm > > concerned). > > > > As a reminder, auditd_test_task() only returns true/1 if the task is > > registered with the audit subsystem as an auditd connection, an > > auditctl process should not cause this function to return true. > > My main concern was overhead, since the whole goal of io_uring is speed. At the point where this test takes place we are already in the audit slow path as far as io_uring is concerned. I understand your concern, but the advantage of being able to effectively use io_uring in the future makes this worth keeping in my opinion. > The chances that audit does use this functionality in the future suggest > to me that it is best to leave this check in. Sounds like we are in agreement. We'll keep it for now. > > > > + rcu_read_lock(); > > > > + list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_URING_EXIT], > > > > + list) { > > > > + if (audit_in_mask(&e->rule, ctx->uring_op) && > > > > > > While this seems like the most obvious approach given the parallels > > > between syscalls and io_uring operations, as coded here it won't work > > > due to the different mappings of syscall numbers and io_uring > > > operations unless we re-use the auditctl -S field with raw io_uring > > > operation numbers in the place of syscall numbers. This should have > > > been obvious to me when I first looked at this patch. It became obvious > > > when I started looking at the userspace auditctl.c. > > > > FWIW, my intention was to treat io_uring opcodes exactly like we treat > > syscall numbers. Yes, this would potentially be an issue if we wanted > > to combine syscalls and io_uring opcodes into one filter, but why > > would we ever want to do that? Combining the two into one filter not > > only makes the filter lists longer than needed (we will always know if > > we are filtering on a syscall or io_uring op) and complicates the > > filter rule processing. > > > > Or is there a problem with this that I'm missing? > > No, I think you have a good understanding of it. I'm asking hard > questions to avoid missing something important. If we can reuse the > syscall infrastructure for this then that is extremely helpful (if not > lazy, which isn't necessarily a bad thing). It does mean that the > io_uring op dictionary will need to live in userspace audit the way it > is currently implemented .... Which I currently believe is the right thing to do. > > > The easy first step would be to use something like this: > > > auditctl -a uring,always -S 18,28 -F key=uring_open > > > to monitor file open commands only. The same is not yet possible for > > > the perm field, but there are so few io_uring ops at this point compared > > > with syscalls that it might be manageable. The arch is irrelevant since > > > io_uring operation numbers are identical across all hardware as far as I > > > can tell. Most of the rest of the fields should make sense if they do > > > for a syscall rule. > > > > I've never been a fan of audit's "perm" filtering; I've always felt > > there were better ways to handle that so I'm not overly upset that we > > are skipping that functionality with this initial support. If it > > becomes a problem in the future we can always add that support at a > > later date. > > Ok, I don't see a pressing need to add it initially, but should add a > check to block that field from being used to avoid the confusion of > unpredictable behaviour should someone try to add a perm filter to a > io_uring filter. That should be done protectively in the kernel and > proactively in userspace. Sure, that's reasonable. > > > Here's a sample of userspace code to support this > > > patch: > > > https://github.com/rgbriggs/audit-userspace/commit/a77baa1651b7ad841a220eb962d4cc92bc07dc96 > > > https://github.com/linux-audit/audit-userspace/compare/master...rgbriggs:ghau-iouring-filtering.v1.0 > > > > Great, thank you. I haven't grabbed a copy yet for testing, but I will. > > I've added a perm filter block as an additional patch in userspace and > updated the tree so that first commit is no longer the top of tree but > the branch name is current. > > I'll add a kernel perm filter check. > > I just noticed some list checking that is missing in tree and watch in > your patch. > > Suggested fixup patches to follow... I see them, thank you, comments will follow over there. Although to be honest I'm mostly focusing on the testing right now while we wait to hear back from Jens on what he is willing to accept regarding audit calls in io_issue_sqe(). If we can't do the _entry()/_exit() calls then this work is pretty much dead and we just have to deal with it in Kconfig. I might make one last, clean patchset and put it in a branch for the distros that want to carry the patchset, but it isn't clear to me that it is something I would want to maintain long term. Long running out of tree patches are generally A Bad Idea. > > > If we abuse the syscall infrastructure at first, we'd need a transition > > > plan to coordinate user and kernel switchover to seperate mechanisms for > > > the two to work together if the need should arise to have both syscall > > > and uring filters in the same rule. > > > > See my comments above, I don't currently see why we would ever want > > syscall and io_uring filtering to happen in the same rule. Please > > speak up if you can think of a reason why this would either be needed, > > or desirable for some reason. > > I think they can be seperate rules for now. Either a syscall rule > catching all io_uring ops can be added, or an io_uring rule can be added > to catch specific ops. The scenario I was thinking of was catching > syscalls of specific io_uring ops. Perhaps I'm misunderstand you, but that scenario really shouldn't exist. The io_uring ops function independently of syscalls; you can *submit* io_uring ops via io_uring_enter(), but they are not guaranteed to be dispatched synchronously (obviously), and given the cred shenanigans that can happen with io_uring there is no guarantee the filters would even be applicable. It isn't an issue of "can" the filters be separate, they *have* to be separate. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit