diff mbox series

[RFC,v1,4/7] landlock: Log domain creation and enforcement

Message ID 20230921061641.273654-5-mic@digikod.net (mailing list archive)
State RFC
Delegated to: Paul Moore
Headers show
Series Landlock audit support | expand

Commit Message

Mickaël Salaün Sept. 21, 2023, 6:16 a.m. UTC
Add audit support for domain creation, i.e. task self-restriction.

Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 security/landlock/audit.c    | 24 ++++++++++++++++++++++++
 security/landlock/audit.h    |  8 ++++++++
 security/landlock/syscalls.c |  4 ++++
 3 files changed, 36 insertions(+)

Comments

Paul Moore Dec. 20, 2023, 9:22 p.m. UTC | #1
On Thu, Sep 21, 2023 at 2:17 AM Mickaël Salaün <mic@digikod.net> wrote:
>
> Add audit support for domain creation, i.e. task self-restriction.
>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>  security/landlock/audit.c    | 24 ++++++++++++++++++++++++
>  security/landlock/audit.h    |  8 ++++++++
>  security/landlock/syscalls.c |  4 ++++
>  3 files changed, 36 insertions(+)
>
> diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> index f58bd529784a..d9589d07e126 100644
> --- a/security/landlock/audit.c
> +++ b/security/landlock/audit.c
> @@ -84,6 +84,30 @@ void landlock_log_create_ruleset(struct landlock_ruleset *const ruleset)
>         audit_log_end(ab);
>  }
>
> +void landlock_log_restrict_self(struct landlock_ruleset *const domain,
> +                               struct landlock_ruleset *const ruleset)
> +{
> +       struct audit_buffer *ab;
> +
> +       WARN_ON_ONCE(domain->id);
> +       WARN_ON_ONCE(!ruleset->id);
> +
> +       ab = audit_log_start(audit_context(), GFP_ATOMIC, AUDIT_LANDLOCK);
> +       if (!ab)
> +               /* audit_log_lost() call */
> +               return;
> +
> +       domain->hierarchy->id =
> +               atomic64_inc_return(&ruleset_and_domain_counter);
> +       log_task(ab);
> +       audit_log_format(ab, " op=restrict-self domain=%llu ruleset=%llu",
> +                        domain->hierarchy->id, ruleset->id);

If domain creation and self restriction are the same, I would suggest
going with "op=create-domain" so it better matches "op=release-domain"
in patch 3/7.

Also see my previous comment about consistency between AUDIT_LANDLOCK records.

> +       audit_log_format(
> +               ab, " parent=%llu",
> +               domain->hierarchy->parent ? domain->hierarchy->parent->id : 0);
> +       audit_log_end(ab);
> +}

--
paul-moore.com
Mickaël Salaün Dec. 21, 2023, 6:45 p.m. UTC | #2
On Wed, Dec 20, 2023 at 04:22:22PM -0500, Paul Moore wrote:
> On Thu, Sep 21, 2023 at 2:17 AM Mickaël Salaün <mic@digikod.net> wrote:
> >
> > Add audit support for domain creation, i.e. task self-restriction.
> >
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ---
> >  security/landlock/audit.c    | 24 ++++++++++++++++++++++++
> >  security/landlock/audit.h    |  8 ++++++++
> >  security/landlock/syscalls.c |  4 ++++
> >  3 files changed, 36 insertions(+)
> >
> > diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> > index f58bd529784a..d9589d07e126 100644
> > --- a/security/landlock/audit.c
> > +++ b/security/landlock/audit.c
> > @@ -84,6 +84,30 @@ void landlock_log_create_ruleset(struct landlock_ruleset *const ruleset)
> >         audit_log_end(ab);
> >  }
> >
> > +void landlock_log_restrict_self(struct landlock_ruleset *const domain,
> > +                               struct landlock_ruleset *const ruleset)
> > +{
> > +       struct audit_buffer *ab;
> > +
> > +       WARN_ON_ONCE(domain->id);
> > +       WARN_ON_ONCE(!ruleset->id);
> > +
> > +       ab = audit_log_start(audit_context(), GFP_ATOMIC, AUDIT_LANDLOCK);
> > +       if (!ab)
> > +               /* audit_log_lost() call */
> > +               return;
> > +
> > +       domain->hierarchy->id =
> > +               atomic64_inc_return(&ruleset_and_domain_counter);
> > +       log_task(ab);
> > +       audit_log_format(ab, " op=restrict-self domain=%llu ruleset=%llu",
> > +                        domain->hierarchy->id, ruleset->id);
> 
> If domain creation and self restriction are the same, I would suggest
> going with "op=create-domain" so it better matches "op=release-domain"
> in patch 3/7.

OK, I'll do something more consistent.

> 
> Also see my previous comment about consistency between AUDIT_LANDLOCK records.
> 
> > +       audit_log_format(
> > +               ab, " parent=%llu",
> > +               domain->hierarchy->parent ? domain->hierarchy->parent->id : 0);
> > +       audit_log_end(ab);
> > +}
> 
> --
> paul-moore.com
>
diff mbox series

Patch

diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index f58bd529784a..d9589d07e126 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -84,6 +84,30 @@  void landlock_log_create_ruleset(struct landlock_ruleset *const ruleset)
 	audit_log_end(ab);
 }
 
+void landlock_log_restrict_self(struct landlock_ruleset *const domain,
+				struct landlock_ruleset *const ruleset)
+{
+	struct audit_buffer *ab;
+
+	WARN_ON_ONCE(domain->id);
+	WARN_ON_ONCE(!ruleset->id);
+
+	ab = audit_log_start(audit_context(), GFP_ATOMIC, AUDIT_LANDLOCK);
+	if (!ab)
+		/* audit_log_lost() call */
+		return;
+
+	domain->hierarchy->id =
+		atomic64_inc_return(&ruleset_and_domain_counter);
+	log_task(ab);
+	audit_log_format(ab, " op=restrict-self domain=%llu ruleset=%llu",
+			 domain->hierarchy->id, ruleset->id);
+	audit_log_format(
+		ab, " parent=%llu",
+		domain->hierarchy->parent ? domain->hierarchy->parent->id : 0);
+	audit_log_end(ab);
+}
+
 /*
  * This is useful to know when a domain or a ruleset will never show again in
  * the audit log.
diff --git a/security/landlock/audit.h b/security/landlock/audit.h
index 2666e9151627..bc17dc8ca6f1 100644
--- a/security/landlock/audit.h
+++ b/security/landlock/audit.h
@@ -16,6 +16,8 @@ 
 #ifdef CONFIG_AUDIT
 
 void landlock_log_create_ruleset(struct landlock_ruleset *const ruleset);
+void landlock_log_restrict_self(struct landlock_ruleset *const domain,
+				struct landlock_ruleset *const ruleset);
 void landlock_log_release_ruleset(const struct landlock_ruleset *const ruleset);
 
 #else /* CONFIG_AUDIT */
@@ -25,6 +27,12 @@  landlock_log_create_ruleset(struct landlock_ruleset *const ruleset)
 {
 }
 
+static inline void
+landlock_log_restrict_self(struct landlock_ruleset *const domain,
+			   struct landlock_ruleset *const ruleset)
+{
+}
+
 static inline void
 landlock_log_release_ruleset(const struct landlock_ruleset *const ruleset)
 {
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 373997a356e7..bfe5417a06c3 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -452,6 +452,10 @@  SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
 	landlock_put_ruleset(new_llcred->domain);
 	new_llcred->domain = new_dom;
 
+	// FIXME: Must be atomic between the ruleset merge and the audit log to
+	// be sure about the content of the domain.
+	// -> move mutex_lock() from merge_ruleset() into this function
+	landlock_log_restrict_self(new_dom, ruleset);
 	landlock_put_ruleset(ruleset);
 	return commit_creds(new_cred);