Received-SPF: pass (google.com: domain of linux-kernel+bounces-7448-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20230921061641.273654-1-mic@digikod.net> <20230921061641.273654-6-mic@digikod.net>
In-Reply-To: <20230921061641.273654-6-mic@digikod.net>
From: Paul Moore <paul@paul-moore.com>
Date: Wed, 20 Dec 2023 16:22:33 -0500
Message-ID: <CAHC9VhQJPXJQCmXPUxOE3wXArUcgfDi98FO=VhRBgBgdueyAOQ@mail.gmail.com>
Subject: Re: [RFC PATCH v1 5/7] landlock: Log file-related requests
To: =?UTF-8?B?TWlja2HDq2wgU2FsYcO8bg==?= <mic@digikod.net>
Cc: Eric Paris <eparis@redhat.com>, James Morris <jmorris@namei.org>, 
	"Serge E . Hallyn" <serge@hallyn.com>, Ben Scarlato <akhna@google.com>, 
	=?UTF-8?Q?G=C3=BCnther_Noack?= <gnoack@google.com>, 
	Jeff Xu <jeffxu@google.com>, Jorge Lucangeli Obes <jorgelo@google.com>, 
	Konstantin Meskhidze <konstantin.meskhidze@huawei.com>, Shervin Oloumi <enlightened@google.com>, 
	audit@vger.kernel.org, linux-kernel@vger.kernel.org, 
	linux-security-module@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Sep 21, 2023 at 2:17=E2=80=AFAM Micka=C3=ABl Sala=C3=BCn <mic@digik=
od.net> wrote:
>
> Add audit support for mkdir, mknod, symlink, unlink, rmdir, truncate,
> and open requests.
>
> Signed-off-by: Micka=C3=ABl Sala=C3=BCn <mic@digikod.net>
> ---
>  security/landlock/audit.c | 114 ++++++++++++++++++++++++++++++++++++++
>  security/landlock/audit.h |  32 +++++++++++
>  security/landlock/fs.c    |  62 ++++++++++++++++++---
>  3 files changed, 199 insertions(+), 9 deletions(-)
>
> diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> index d9589d07e126..148fc0fafef4 100644
> --- a/security/landlock/audit.c
> +++ b/security/landlock/audit.c
> @@ -14,6 +14,25 @@
>
>  atomic64_t ruleset_and_domain_counter =3D ATOMIC64_INIT(0);
>
> +static const char *op_to_string(enum landlock_operation operation)
> +{
> +       const char *const desc[] =3D {
> +               [0] =3D "",
> +               [LANDLOCK_OP_MKDIR] =3D "mkdir",
> +               [LANDLOCK_OP_MKNOD] =3D "mknod",
> +               [LANDLOCK_OP_SYMLINK] =3D "symlink",
> +               [LANDLOCK_OP_UNLINK] =3D "unlink",
> +               [LANDLOCK_OP_RMDIR] =3D "rmdir",
> +               [LANDLOCK_OP_TRUNCATE] =3D "truncate",
> +               [LANDLOCK_OP_OPEN] =3D "open",
> +       };

If you're going to be using a single AUDIT_LANDLOCK record type, do
you want to somehow encode that the above are access/permission
requests in the "op=3D" field name?

> +static void
> +log_request(const int error, struct landlock_request *const request,
> +           const struct landlock_ruleset *const domain,
> +           const access_mask_t access_request,
> +           const layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_F=
S])
> +{
> +       struct audit_buffer *ab;
> +
> +       if (WARN_ON_ONCE(!error))
> +               return;
> +       if (WARN_ON_ONCE(!request))
> +               return;
> +       if (WARN_ON_ONCE(!domain || !domain->hierarchy))
> +               return;
> +
> +       /* Uses GFP_ATOMIC to not sleep. */
> +       ab =3D audit_log_start(audit_context(), GFP_ATOMIC | __GFP_NOWARN=
,
> +                            AUDIT_LANDLOCK);
> +       if (!ab)
> +               return;
> +
> +       update_request(request, domain, access_request, layer_masks);
> +
> +       log_task(ab);
> +       audit_log_format(ab, " domain=3D%llu op=3D%s errno=3D%d missing-f=
s-accesses=3D",
> +                        request->youngest_domain,
> +                        op_to_string(request->operation), -error);
> +       log_accesses(ab, request->missing_access);
> +       audit_log_lsm_data(ab, &request->audit);
> +       audit_log_end(ab);
> +}

See my previous comments about record format consistency.

--
paul-moore.com