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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B5294ECAAD5 for ; Thu, 1 Sep 2022 01:47:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232580AbiIABr0 (ORCPT ); Wed, 31 Aug 2022 21:47:26 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52716 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232771AbiIABrW (ORCPT ); Wed, 31 Aug 2022 21:47:22 -0400 Received: from mail-oa1-x2f.google.com (mail-oa1-x2f.google.com [IPv6:2001:4860:4864:20::2f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0A585AA4C0 for ; Wed, 31 Aug 2022 18:47:21 -0700 (PDT) Received: by mail-oa1-x2f.google.com with SMTP id 586e51a60fabf-11f34610d4aso17562116fac.9 for ; Wed, 31 Aug 2022 18:47:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=paul-moore-com.20210112.gappssmtp.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date; bh=TlN3L7u+wXIinFsuYqdK27x2WtBpHcllYjmImCLuHAQ=; b=K1OeB1YszbZTqWguTLKSaKYwSr3mt4ww03N3PZsfia1GxvfsN1Vr3pwWf8GoktXXX9 ozwgxAa2ZfMOrzcUVIVZhdGYMC5BPpCdf0WfGLz/ywPkLp7kp6goP7wt0TDbmv6L/Kka L3+hS3CC6sEp3kgIonBvcts8dAPIWeJbM7fmr7qoxpUNeXcs03/ewiiWBknnzftkoHNZ lAvbk/LdDbeP8Mm7B+F0SKaaYRE9cKWDdoXHcKhfvM7mmPF/BPyGa+o8SYQgqjf/hRpn O2zu+/QyMKgNre9l59JqIxrboEDqP/k9kWl26cd4a8eTjU1tbRf953RGX9S9YViz+mmy sN2w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=TlN3L7u+wXIinFsuYqdK27x2WtBpHcllYjmImCLuHAQ=; b=FWebCtZCjq/hiIwecK6wXAb3AJam0/RCY/lgw1+ngGB3y4mKhfHTtbJHxEwEfXfLBN gS8N4oMbOFjX8r+dXNDcAjHIPkqTNxfmUQQa7PfHpDaXMtFxo/hhNnkSvycAZQSqPyCy +4hrw/i8otOFvJgNR5wo5mlGNRY88qNo5WKlKgG2IeL2rCiz5Rgs4AHxdXmTgAD8W+nY 3hJ2Zck/Va0hY/WmvAqN/Ky2YdYpurahCK95N0euy+GKLyrs0yfu1o6Rqupcr6nxA3Mr oEM53cgi1xStyEoI7rpkTh766HVJ++5zgrDoaxYUvTpw8x+/yjRCGNZ61oJulL9XduLi jHLw== X-Gm-Message-State: ACgBeo1TQKDTzbZAmIJh6DEhZBK4vl0oo9vOqw9tBxxG01GAL8KfUvM8 51oIJT4ipU/Mh8AYlpTa/3x5KhhSCVpJOU386tXU X-Google-Smtp-Source: AA6agR5gFuM289TBrDS25LgdOwxYckt1TdnxQ0qi8OPF99N5aCotlFsRQ/Axj8r2vSdySNWWcR+s751K/eVL4geaf8s= X-Received: by 2002:a05:6871:796:b0:11e:b92e:731e with SMTP id o22-20020a056871079600b0011eb92e731emr2981881oap.41.1661996840312; Wed, 31 Aug 2022 18:47:20 -0700 (PDT) MIME-Version: 1.0 References: <12063373.O9o76ZdvQC@x2> <5600292.DvuYhMxLoT@x2> In-Reply-To: <5600292.DvuYhMxLoT@x2> From: Paul Moore Date: Wed, 31 Aug 2022 21:47:09 -0400 Message-ID: Subject: Re: [PATCH v4 3/4] fanotify,audit: Allow audit to use the full permission event response To: Steve Grubb Cc: Richard Guy Briggs , Linux-Audit Mailing List , LKML , linux-fsdevel@vger.kernel.org, Eric Paris , Jan Kara , Amir Goldstein Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Aug 31, 2022 at 7:55 PM Steve Grubb wrote: > On Wednesday, August 31, 2022 6:19:40 PM EDT Richard Guy Briggs wrote: > > On 2022-08-31 17:25, Steve Grubb wrote: > > > On Wednesday, August 31, 2022 5:07:25 PM EDT Richard Guy Briggs wrote: > > > > > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > > > > > > index 433418d73584..f000fec52360 100644 > > > > > > --- a/kernel/auditsc.c > > > > > > +++ b/kernel/auditsc.c > > > > > > @@ -64,6 +64,7 @@ > > > > > > #include > > > > > > #include > > > > > > #include // struct open_how > > > > > > +#include > > > > > > > > > > > > #include "audit.h" > > > > > > > > > > > > @@ -2899,10 +2900,34 @@ void __audit_log_kern_module(char *name) > > > > > > context->type = AUDIT_KERN_MODULE; > > > > > > } > > > > > > > > > > > > -void __audit_fanotify(u32 response) > > > > > > +void __audit_fanotify(u32 response, size_t len, char *buf) > > > > > > { > > > > > > - audit_log(audit_context(), GFP_KERNEL, > > > > > > - AUDIT_FANOTIFY, "resp=%u", response); > > > > > > + struct fanotify_response_info_audit_rule *friar; > > > > > > + size_t c = len; > > > > > > + char *ib = buf; > > > > > > + > > > > > > + if (!(len && buf)) { > > > > > > + audit_log(audit_context(), GFP_KERNEL, > > > > > > AUDIT_FANOTIFY, > > > > > > + "resp=%u fan_type=0 fan_info=?", > > > > > > response); > > > > > > + return; > > > > > > + } > > > > > > + while (c >= sizeof(struct fanotify_response_info_header)) { > > > > > > + friar = (struct fanotify_response_info_audit_rule > > > > > > *)buf; > > > > > > > > > > Since the only use of this at the moment is the > > > > > fanotify_response_info_rule, why not pass the > > > > > fanotify_response_info_rule struct directly into this function? We > > > > > can always change it if we need to in the future without affecting > > > > > userspace, and it would simplify the code. > > > > > > > > Steve, would it make any sense for there to be more than one > > > > FAN_RESPONSE_INFO_AUDIT_RULE header in a message? Could there be more > > > > than one rule that contributes to a notify reason? If not, would it be > > > > reasonable to return -EINVAL if there is more than one? > > > > > > I don't see a reason for sending more than one header. What is more > > > probable is the need to send additional data in that header. I was > > > thinking of maybe bit mapping it in the rule number. But I'd suggest > > > padding the struct just in case it needs expanding some day. > > > > This doesn't exactly answer my question about multiple rules > > contributing to one decision. > > I don't forsee that. > > > The need for more as yet undefined information sounds like a good reason > > to define a new header if that happens. > > It's much better to pad the struct so that the size doesn't change. > > > At this point, is it reasonable to throw an error if more than one RULE > > header appears in a message? > > It is a write syscall. I'd silently discard everything else and document that > in the man pages. But the fanotify maintainers should really weigh in on > this. > > > The way I had coded this last patchset was to allow for more than one RULE > > header and each one would get its own record in the event. > > I do not forsee a need for this. > > > How many rules total are likely to exist? > > Could be a thousand. But I already know some missing information we'd like to > return to user space in an audit event, so the bit mapping on the rule number > might happen. I'd suggest padding one u32 for future use. A better way to handle an expansion like that would be to have a length/version field at the top of the struct that could be used to determine the size and layout of the struct. However, to be clear, my original suggestion of passing the fanotify_response_info_rule struct internally didn't require any additional future proofing as it is an internal implementation detail and not something that is exposed to userspace; the function arguments could be changed in the future and not break userspace. I'm not quite sure how we ended up on this topic ... -- 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 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 6B536ECAAD1 for ; Thu, 1 Sep 2022 03:09:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1662001763; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=3mMMAX0J7xNtRnoYVM2+BjltpfKRQu7Yop7AV9fJCEY=; b=NppWLLiE4cEKdLXfEmRAET1WL8HSOAEp+nFtG+qoWrNrePa+lk7N0lCM9YQ4lECOVJj4Sq ycUcFqcQ3S0KWIRQCoeojqlz7ICtTthek5M9V7u8PvJ9haBhPcvGxz6nbJds+6VdXgfUct OsPXewaKxMYUotH1aEQfMy6rLp9IEV8= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-141-KB5nO5vbN8e--HTQO0k4LA-1; Wed, 31 Aug 2022 23:09:19 -0400 X-MC-Unique: KB5nO5vbN8e--HTQO0k4LA-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 41BD71018AA4; Thu, 1 Sep 2022 03:09:18 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com [10.30.29.100]) by smtp.corp.redhat.com (Postfix) with ESMTP id DAC34492C3B; Thu, 1 Sep 2022 03:09:16 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (localhost [IPv6:::1]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id B4EA81946A45; Thu, 1 Sep 2022 03:09:16 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id 900391946A40 for ; Thu, 1 Sep 2022 01:47:23 +0000 (UTC) Received: by smtp.corp.redhat.com (Postfix) id 7D9484010D43; Thu, 1 Sep 2022 01:47:23 +0000 (UTC) Received: from mimecast-mx02.redhat.com (mimecast02.extmail.prod.ext.rdu2.redhat.com [10.11.55.18]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 790404010FA0 for ; Thu, 1 Sep 2022 01:47:23 +0000 (UTC) Received: from us-smtp-1.mimecast.com (us-smtp-2.mimecast.com [205.139.110.61]) (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 4D2A5964081 for ; Thu, 1 Sep 2022 01:47:23 +0000 (UTC) Received: from mail-oa1-f43.google.com (mail-oa1-f43.google.com [209.85.160.43]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-78-YBjB2RlwPASi3OzMfWO28w-1; Wed, 31 Aug 2022 21:47:21 -0400 X-MC-Unique: YBjB2RlwPASi3OzMfWO28w-1 Received: by mail-oa1-f43.google.com with SMTP id 586e51a60fabf-11f0fa892aeso20108888fac.7 for ; Wed, 31 Aug 2022 18:47:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=TlN3L7u+wXIinFsuYqdK27x2WtBpHcllYjmImCLuHAQ=; b=78xtnbKADTNfn5UaqIw7SPMlJY5WSwy8eNgGX3oHL8VzyNQUUIy5m7hwnQdc4WxxsF qxVeqzY641GRsrZE76c2ijpYXjGs27hGJ/k2itbSm6aNdlzCfz1RrfSo4mF39pvsbUNh 3g7P20/ERu/HCMYmdsBXmqcu65bWMSNw4qFCWU8SMOThYPSCr6EtAMjxxH1F7khHYvgc ebyLgyh9kbUrhqo388jmOMU4Afh2x9/0ZW6sKEFvBrB/3gS1FHljoyyFBm5FbeNHHdl+ iAFKt75SphdOh4beUy5w3gHf+J6stGXCBX7VS6HczZL33EBHTUPAsj576ZX9VLrPebfH SqQQ== X-Gm-Message-State: ACgBeo0vDfzOCf1h9nbQTkRZ+T1rIXq2mi6SQZ61SMwbrByIuYjiEiPw GOY/B8YC7aXJPDERtm1mGqVVBx0Vh9pBsbYc27oa X-Google-Smtp-Source: AA6agR5gFuM289TBrDS25LgdOwxYckt1TdnxQ0qi8OPF99N5aCotlFsRQ/Axj8r2vSdySNWWcR+s751K/eVL4geaf8s= X-Received: by 2002:a05:6871:796:b0:11e:b92e:731e with SMTP id o22-20020a056871079600b0011eb92e731emr2981881oap.41.1661996840312; Wed, 31 Aug 2022 18:47:20 -0700 (PDT) MIME-Version: 1.0 References: <12063373.O9o76ZdvQC@x2> <5600292.DvuYhMxLoT@x2> In-Reply-To: <5600292.DvuYhMxLoT@x2> From: Paul Moore Date: Wed, 31 Aug 2022 21:47:09 -0400 Message-ID: Subject: Re: [PATCH v4 3/4] fanotify,audit: Allow audit to use the full permission event response To: Steve Grubb 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.84 on 10.11.54.2 X-BeenThere: linux-audit@redhat.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux Audit Discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jan Kara , Richard Guy Briggs , Amir Goldstein , LKML , Linux-Audit Mailing List , linux-fsdevel@vger.kernel.org, Eric Paris Errors-To: linux-audit-bounces@redhat.com Sender: "Linux-audit" X-Scanned-By: MIMEDefang 2.85 on 10.11.54.10 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On Wed, Aug 31, 2022 at 7:55 PM Steve Grubb wrote: > On Wednesday, August 31, 2022 6:19:40 PM EDT Richard Guy Briggs wrote: > > On 2022-08-31 17:25, Steve Grubb wrote: > > > On Wednesday, August 31, 2022 5:07:25 PM EDT Richard Guy Briggs wrote: > > > > > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > > > > > > index 433418d73584..f000fec52360 100644 > > > > > > --- a/kernel/auditsc.c > > > > > > +++ b/kernel/auditsc.c > > > > > > @@ -64,6 +64,7 @@ > > > > > > #include > > > > > > #include > > > > > > #include // struct open_how > > > > > > +#include > > > > > > > > > > > > #include "audit.h" > > > > > > > > > > > > @@ -2899,10 +2900,34 @@ void __audit_log_kern_module(char *name) > > > > > > context->type = AUDIT_KERN_MODULE; > > > > > > } > > > > > > > > > > > > -void __audit_fanotify(u32 response) > > > > > > +void __audit_fanotify(u32 response, size_t len, char *buf) > > > > > > { > > > > > > - audit_log(audit_context(), GFP_KERNEL, > > > > > > - AUDIT_FANOTIFY, "resp=%u", response); > > > > > > + struct fanotify_response_info_audit_rule *friar; > > > > > > + size_t c = len; > > > > > > + char *ib = buf; > > > > > > + > > > > > > + if (!(len && buf)) { > > > > > > + audit_log(audit_context(), GFP_KERNEL, > > > > > > AUDIT_FANOTIFY, > > > > > > + "resp=%u fan_type=0 fan_info=?", > > > > > > response); > > > > > > + return; > > > > > > + } > > > > > > + while (c >= sizeof(struct fanotify_response_info_header)) { > > > > > > + friar = (struct fanotify_response_info_audit_rule > > > > > > *)buf; > > > > > > > > > > Since the only use of this at the moment is the > > > > > fanotify_response_info_rule, why not pass the > > > > > fanotify_response_info_rule struct directly into this function? We > > > > > can always change it if we need to in the future without affecting > > > > > userspace, and it would simplify the code. > > > > > > > > Steve, would it make any sense for there to be more than one > > > > FAN_RESPONSE_INFO_AUDIT_RULE header in a message? Could there be more > > > > than one rule that contributes to a notify reason? If not, would it be > > > > reasonable to return -EINVAL if there is more than one? > > > > > > I don't see a reason for sending more than one header. What is more > > > probable is the need to send additional data in that header. I was > > > thinking of maybe bit mapping it in the rule number. But I'd suggest > > > padding the struct just in case it needs expanding some day. > > > > This doesn't exactly answer my question about multiple rules > > contributing to one decision. > > I don't forsee that. > > > The need for more as yet undefined information sounds like a good reason > > to define a new header if that happens. > > It's much better to pad the struct so that the size doesn't change. > > > At this point, is it reasonable to throw an error if more than one RULE > > header appears in a message? > > It is a write syscall. I'd silently discard everything else and document that > in the man pages. But the fanotify maintainers should really weigh in on > this. > > > The way I had coded this last patchset was to allow for more than one RULE > > header and each one would get its own record in the event. > > I do not forsee a need for this. > > > How many rules total are likely to exist? > > Could be a thousand. But I already know some missing information we'd like to > return to user space in an audit event, so the bit mapping on the rule number > might happen. I'd suggest padding one u32 for future use. A better way to handle an expansion like that would be to have a length/version field at the top of the struct that could be used to determine the size and layout of the struct. However, to be clear, my original suggestion of passing the fanotify_response_info_rule struct internally didn't require any additional future proofing as it is an internal implementation detail and not something that is exposed to userspace; the function arguments could be changed in the future and not break userspace. I'm not quite sure how we ended up on this topic ... -- paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://listman.redhat.com/mailman/listinfo/linux-audit