Message-ID: <CAHC9VhR7EWDm5afnzRbVO6jkqtwtBe-UbHk+b-bED7uwisu_bA@mail.gmail.com>
Date: Mon, 6 Jan 2025 16:56:50 -0500
From: Paul Moore <paul@...l-moore.com>
To: Mickaël Salaün <mic@...ikod.net>
Cc: Eric Paris <eparis@...hat.com>, Günther Noack <gnoack@...gle.com>, 
	"Serge E . Hallyn" <serge@...lyn.com>, Ben Scarlato <akhna@...gle.com>, 
	Casey Schaufler <casey@...aufler-ca.com>, Charles Zaffery <czaffery@...lox.com>, 
	Francis Laniel <flaniel@...ux.microsoft.com>, James Morris <jmorris@...ei.org>, 
	Jann Horn <jannh@...gle.com>, Jeff Xu <jeffxu@...gle.com>, 
	Jorge Lucangeli Obes <jorgelo@...gle.com>, Kees Cook <kees@...nel.org>, 
	Konstantin Meskhidze <konstantin.meskhidze@...wei.com>, Matt Bobrowski <mattbobrowski@...gle.com>, 
	Mikhail Ivanov <ivanov.mikhail1@...wei-partners.com>, Phil Sutter <phil@....cc>, 
	Praveen K Paladugu <prapal@...ux.microsoft.com>, Robert Salvet <robert.salvet@...lox.com>, 
	Shervin Oloumi <enlightened@...gle.com>, Song Liu <song@...nel.org>, 
	Tahera Fahimi <fahimitahera@...il.com>, audit@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org
Subject: Re: [PATCH v3 10/23] landlock: Log domain properties and release

On Mon, Jan 6, 2025 at 9:51 AM Mickaël Salaün <mic@...ikod.net> wrote:
> On Sat, Jan 04, 2025 at 08:23:51PM -0500, Paul Moore wrote:
> > On Nov 22, 2024 =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <mic@...ikod.net> wrote:

...

> > > Audit event sample for a first denial:
> > >
> > >   type=LL_DENY msg=audit(1732186800.349:44): domain=195ba459b blockers=ptrace opid=1 ocomm="systemd"
> > >   type=LL_DOM_INFO msg=audit(1732186800.349:44): domain=195ba459b creation=1732186800.345 pid=300 uid=0 exe="/root/sandboxer" comm="sandboxer"
> > >   type=SYSCALL msg=audit(1732186800.349:44): arch=c000003e syscall=101 success=no [...] pid=300 auid=0
> >
> > As mentioned in patch 9/23, I don't want subsystems external to audit
> > to access the audit timestamp information, so the "creation=" field
> > in the audit event would need to be removed.  Assuming that the timestamp
> > was used either to reference the original domain creation and/or simply
> > provide some additional information for analysis, all of that information
> > should already be in the audit log, assuming of course that you are
> > logging domain creation (which you should, at least as an option).
>
> As explained in this patch, we don't want to (and cannot realistically)
> log domain creations.  That would make the audit support for Landlock
> unusable.  Moreover, these information is useless and only add noise
> unless there is a denial, hence this asynchronous approach.

That's fine, just know that it doesn't change my thoughts on exposing
the audit timestamp.

> However,
> users may want to log some syscalls, including landlock_restrict_self(),
> and it would make audit logs more consistent using the same timestamp as
> the Landlock domain creation time.  I'm wondering why exposing this
> timestamp to the kernel would be an issue whereas it is already exposed
> to user space.

Currently there are no other users of the audit timestamp besides
audit.  Making the audit timestamp available to other subsystems makes
the timestamp less flexible over the long term as it would become, in
a way, part of the API that audit provides to other in-kernel users.

I still have hopes to rework a large chunk of the audit subsystem, and
keeping the interfaces between audit and the other in-kernel
subsystems makes that easier.

> If you're really opposed to it I can create a new unrelated timestamp
> specific to Landlock.

Yes, at this point in time I don't want to support exporting the audit
timestamp outside of audit.  My guess is that you probably want to use
some identifier, other than a timestamp, when trying to link Landlock
events (presumably the domain ID would do this?), but I don't pretend
to know the details of Landlock very well right now.

> > Also, is there a good reason why the LL_DOM_INFO information can't be
> > recorded in the LL_DENY (or LL_ACCESS) record?  I think that would be
> > preferable.
>
> The goal of the standalone LL_DOM_INFO record type is to limit useless
> log verbosity.  Including this information in LL_DENY would have two
> downsides:
> - it would increases the length of *all* LL_DENY messages

Are you ever going to emit a LL_ACCESS/LL_DENY record without a
LL_DOM_INFO record?

> - it would make it more difficult to extend this new mixed messages with
>   access-related informations (e.g. file property) and domain-related
>   informations (and associate them with either the object or the
>   domain).

How?  Please elaborate on this.

> > > Audit event sample for logged domains deletion:
> > >
> > >   type=LL_DOM_DROP msg=audit(1732186800.393:45): domain=195ba459b
> > >
> > > Cc: Günther Noack <gnoack@...gle.com>
> > > Cc: Paul Moore <paul@...l-moore.com>
> > > Signed-off-by: Mickaël Salaün <mic@...ikod.net>
> > > Link: https://lore.kernel.org/r/20241122143353.59367-11-mic@digikod.net
> > > ---
> > > Questions:
> > > - Should we also log the creator's loginuid?
> > > - Should we also log the creator's sessionid?
> >
> > Creation of a Landlock domain can only happen through the Landlock
> > syscalls, yes?  If so, that information should already be logged in
> > the associated syscall record (see the "auid=" and "ses=" fields )and
> > we generally try to avoid duplicating information across records in
> > the same audit event.
>
> The specificity of Landlock compared to existing supported systems is
> that we cannot log domain creation for the reason I explain before.

Can you provide a link to that explanation?  I'm sure you explained it
well, but I missed it when going over the patchset with a focus on
audit.

If the Landlock domain is created independent from any user/process
action, it likely doesn't make sense to log either the loginuid or
sessionid since the domain creation is happening independently from a
user session.

-- 
paul-moore.com

Powered by blists - more mailing lists