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=-18.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS 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 2D91CC47091 for ; Sun, 30 May 2021 15:27:03 +0000 (UTC) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.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 9B72160FE9 for ; Sun, 30 May 2021 15:27:02 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9B72160FE9 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-331-ZMF74tHzMjGMKWXL1G0dZA-1; Sun, 30 May 2021 11:26:59 -0400 X-MC-Unique: ZMF74tHzMjGMKWXL1G0dZA-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 9DD4B180FD65; Sun, 30 May 2021 15:26:55 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 2BA435DEC1; Sun, 30 May 2021 15:26:53 +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 B47FD55345; Sun, 30 May 2021 15:26:51 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 14UFQlCY027059 for ; Sun, 30 May 2021 11:26:47 -0400 Received: by smtp.corp.redhat.com (Postfix) id 118911004037; Sun, 30 May 2021 15:26:47 +0000 (UTC) Received: from mimecast-mx02.redhat.com (mimecast05.extmail.prod.ext.rdu2.redhat.com [10.11.55.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0D6B61111A72 for ; Sun, 30 May 2021 15:26:40 +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 0D21982A682 for ; Sun, 30 May 2021 15:26:40 +0000 (UTC) Received: from mail-ej1-f52.google.com (mail-ej1-f52.google.com [209.85.218.52]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-205-P24ldwdDOKCL8io3qym4wQ-1; Sun, 30 May 2021 11:26:37 -0400 X-MC-Unique: P24ldwdDOKCL8io3qym4wQ-1 Received: by mail-ej1-f52.google.com with SMTP id e12so12768932ejt.3 for ; Sun, 30 May 2021 08:26:37 -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=e83Xdcrb8ArQTK3BfJ+SKjS0Z61QsbiftdtWKkLx9t0=; b=kQ/x5vcsi6Zmc5NcXprRc6mLgS3XtssY9VKxDdvF2Hj5mq4vjkWURIVayy9dft5s1Y uc2XVSl3yFjkMyGEkxgtJPqhLGHcfbyN7N9p/DLkhoWJ4h2E3eX5ssqYeMVvBleef7+p qGE6HYeuDyrd5Rgl6aiES+wJNgAW8q6DraTD8N7QZrq+1JlIFEnobcdczS85Sezhl03Y x3t8jpaXQanh7Zhmk2YmKbcAaGT0gd7ap03oZtuTjJK/cY7qaKFlgWYqhTIcUhsALCRe Jl86lNadrRzmyX39AnkeroLOola/JZ7Lv7MZrY7qWqQ5bfOVlitlNLUET0Vt9GfUzqs6 RAyA== X-Gm-Message-State: AOAM530a1W9P9Cp0IpCdCIBj4PzlqS3Hxgsw/8yrPJ7R0z5L9bWeZZBM 6gp1OOsUEaHs7EoWiJDNysNdkdy4ISiG67NjInZ3 X-Google-Smtp-Source: ABdhPJwPsCNauSlDJZZCjOpy5U50sK0NxibF5HbYEJWlsb/BX8Aa7YBzBA+a2fJHbO4WCKa8o/7Q2nAhnogkBmdUNtg= X-Received: by 2002:a17:906:4111:: with SMTP id j17mr9037573ejk.488.1622388396174; Sun, 30 May 2021 08:26:36 -0700 (PDT) MIME-Version: 1.0 References: <162163367115.8379.8459012634106035341.stgit@sifl> <162163380685.8379.17381053199011043757.stgit@sifl> <20210528223544.GL447005@madcap2.tricolour.ca> In-Reply-To: <20210528223544.GL447005@madcap2.tricolour.ca> From: Paul Moore Date: Sun, 30 May 2021 11:26:25 -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.3 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.14 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 Fri, May 28, 2021 at 6:36 PM Richard Guy Briggs wrote: > On 2021-05-21 17:50, Paul Moore wrote: > > WARNING - This is a work in progress and should not be merged > > anywhere important. It is almost surely not complete, and while it > > probably compiles it likely hasn't been booted and will do terrible > > things. You have been warned. > > > > This patch adds basic audit io_uring filtering, using as much of the > > existing audit filtering infrastructure as possible. In order to do > > this we reuse the audit filter rule's syscall mask for the io_uring > > operation and we create a new filter for io_uring operations as > > AUDIT_FILTER_URING_EXIT/audit_filter_list[7]. > > > > > > > > Signed-off-by: Paul Moore > > --- > > include/uapi/linux/audit.h | 3 +- > > kernel/auditfilter.c | 4 ++- > > kernel/auditsc.c | 65 ++++++++++++++++++++++++++++++++++---------- > > 3 files changed, 55 insertions(+), 17 deletions(-) ... > > 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. > > + 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? > 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. I currently fear that just getting io_uring and audit to coexist is going to be a large enough problem in the immediate future. > 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. > 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. > It might be wise to deliberately not support auditctl "-w" (and the > exported audit_add_watch() function) since that is currently hardcoded > to the AUDIT_FILTER_EXIT list and is the old watch form [replaced by > audit_rule_fieldpair_data()] anyways that is more likely to be > deprecated. It also appears to make sense not to support autrace (at > least initially). I'm going to be honest with you and simply say that I've run out of my email/review time in front of the computer on this holiday weekend (blame the lockdown/bpf/lsm discussion ) and I need to go for today, but this is something I'll take a look it this coming week. Hopefully the comments above give us something to think/talk about in the meantime. Regardless, thanks for your help on the userspace side of the filtering, that should make testing a lot easier moving forward. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit