On Mar 7, 2025 Casey Schaufler <casey@xxxxxxxxxxxxxxxx> wrote:
>
> Create a new audit record AUDIT_MAC_TASK_CONTEXTS.
> An example of the MAC_TASK_CONTEXTS (1423) record is:
>
> type=MAC_TASK_CONTEXTS[1423]
> msg=audit(1600880931.832:113)
> subj_apparmor=unconfined
> subj_smack=_
>
> When an audit event includes a AUDIT_MAC_TASK_CONTEXTS record
> the "subj=" field in other records in the event will be "subj=?".
> An AUDIT_MAC_TASK_CONTEXTS record is supplied when the system has
> multiple security modules that may make access decisions based
> on a subject security context.
>
> Signed-off-by: Casey Schaufler <casey@xxxxxxxxxxxxxxxx>
> ---
> include/linux/lsm_hooks.h | 1 +
> include/linux/security.h | 1 +
> include/uapi/linux/audit.h | 1 +
> kernel/audit.c | 45 ++++++++++++++++++++++++++++++++------
> security/apparmor/lsm.c | 1 +
> security/bpf/hooks.c | 1 +
> security/security.c | 3 +++
> security/selinux/hooks.c | 1 +
> security/smack/smack_lsm.c | 1 +
> 9 files changed, 48 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 090d1d3e19fe..e4d303ab1f20 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -81,6 +81,7 @@ struct lsm_static_calls_table {
> struct lsm_id {
> const char *name;
> u64 id;
> + bool subjctx;
> };
The new field needs to be documented in the comment block for the
struct, and while I'll reserve judgement until I see the description,
I believe this field either needs to be renamed, moved elsewhere, or
simply "disappeared" in favor of something else.
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 540894695c4b..79a9bf4a7cdd 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -168,6 +168,7 @@ struct lsm_prop {
>
> extern const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1];
> extern u32 lsm_active_cnt;
> +extern u32 lsm_subjctx_cnt;
I'm not loving this. More below, but I'd really like to hide some of
this detail inside a LSM API/hook if possible.
> extern const struct lsm_id *lsm_idlist[];
>
> /* These functions are in security/commoncap.c */
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index d9a069b4a775..5ebb5d80363d 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -146,6 +146,7 @@
> #define AUDIT_IPE_ACCESS 1420 /* IPE denial or grant */
> #define AUDIT_IPE_CONFIG_CHANGE 1421 /* IPE config change */
> #define AUDIT_IPE_POLICY_LOAD 1422 /* IPE policy load */
> +#define AUDIT_MAC_TASK_CONTEXTS 1423 /* Multiple LSM task contexts */
>
> #define AUDIT_FIRST_KERN_ANOM_MSG 1700
> #define AUDIT_LAST_KERN_ANOM_MSG 1799
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 293364bba961..59eaf69ee8ac 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -54,6 +54,7 @@
> #include <net/netlink.h>
> #include <linux/skbuff.h>
> #include <linux/security.h>
> +#include <linux/lsm_hooks.h>
> #include <linux/freezer.h>
> #include <linux/pid_namespace.h>
> #include <net/netns/generic.h>
> @@ -2241,21 +2242,51 @@ int audit_log_task_context(struct audit_buffer *ab)
> {
> struct lsm_prop prop;
> struct lsm_context ctx;
> + bool space = false;
> int error;
> + int i;
>
> security_current_getlsmprop_subj(&prop);
> if (!lsmprop_is_set(&prop))
> return 0;
>
> - error = security_lsmprop_to_secctx(&prop, &ctx, LSM_ID_UNDEF);
> - if (error < 0) {
> - if (error != -EINVAL)
> - goto error_path;
> + if (lsm_subjctx_cnt < 2) {
> + error = security_lsmprop_to_secctx(&prop, &ctx, LSM_ID_UNDEF);
> + if (error < 0) {
> + if (error != -EINVAL)
> + goto error_path;
> + return 0;
> + }
> + audit_log_format(ab, " subj=%s", ctx.context);
> + security_release_secctx(&ctx);
> return 0;
> }
> -
> - audit_log_format(ab, " subj=%s", ctx.context);
> - security_release_secctx(&ctx);
> + /* Multiple LSMs provide contexts. Include an aux record. */
> + audit_log_format(ab, " subj=?");
> + error = audit_buffer_aux_new(ab, AUDIT_MAC_TASK_CONTEXTS);
> + if (error)
> + goto error_path;
> +
> + for (i = 0; i < lsm_active_cnt; i++) {
> + if (!lsm_idlist[i]->subjctx)
> + continue;
> + error = security_lsmprop_to_secctx(&prop, &ctx,
> + lsm_idlist[i]->id);
> + if (error < 0) {
> + if (error == -EOPNOTSUPP)
> + continue;
> + audit_log_format(ab, "%ssubj_%s=?", space ? " " : "",
> + lsm_idlist[i]->name);
> + if (error != -EINVAL)
> + audit_panic("error in audit_log_task_context");
> + } else {
> + audit_log_format(ab, "%ssubj_%s=%s", space ? " " : "",
> + lsm_idlist[i]->name, ctx.context);
> + security_release_secctx(&ctx);
> + }
> + space = true;
> + }
> + audit_buffer_aux_end(ab);
> return 0;
I'd really like to see us develop something a bit cleaner than the
code above. It looks like there are two things that we're currently
missing in the LSM API: a count of the number of LSMs supplying data
in a lsm_prop struct, and a mechanism to iterate through a lsm_prop
and act on each LSM's data.
As far as the count is concerned, I think we can take a similar
approach to what we do with MAX_LSM_COUNT to generate a value at
build time, for example (pardon the lazy pseudo code):
>>> include/linux/lsm_count.h
#if IS_ENABLED(CONFIG_SECURITY_SMACK)
#define SMACK_ENABLED 1,
#define SMACK_LSMPROP 1,
#else
#define SMACK_ENABLED
#define SMACK_LSMPROP
#endif
#define MAX_LSM_PROPS \
COUNT_LSMS( \
SMACK_LSMPROP \
...) \
The iterator mechanism is a little more complex, but I think the
interface and resulting will be a lot cleaner, at least to in my mind.
>>> LSM hook
int security_lsmprop_walk(void *data,
void (*iter)(struct lsm_id *lsm, void *data,
char *secctx, u32 secid));
>>> example audit code
struct lsmprop_walk_data {
struct audit_buffer *ab;
unsigned int count;
};
void audit_lsmprop_walk(struct lsm_id *lsm, void *data,
char *secctx, u32 secid)
{
struct lsmprop_walk_data *walk = data;
audit_log_format(data->ab, "%ssubj_%s=%s",
(data->count++ ? " " : ""),
lsm->name, (secctx ? secctx : "?"));
if (secctx)
security_release_secctx(secctx);
}
int audit_log_task_context(...)
{
...
if (MAX_LSM_PROPS <= 1) {
security_lsmprop_to_secctx(...)
audit_log_format(...);
} else {
struct lsmprop_walk_data data = { .ab = ab, count = 0 };
audit_buffer_aux_new(ab, AUDIT_MAC_TASK_CONTEXTS);
security_lsmprop_walk(data, audit_lsmprop_walk);
audit_buffer_aux_end(
}
...
}
--
paul-moore.com
