| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <88b13658-1a4c-4ebe-92b4-5be93c9356c9@schaufler-ca.com>
Date: Tue, 15 Jul 2025 13:40:18 -0700
From: Casey Schaufler <casey@...aufler-ca.com>
To: Paul Moore <paul@...l-moore.com>, eparis@...hat.com,
linux-security-module@...r.kernel.org, audit@...r.kernel.org
Cc: jmorris@...ei.org, serge@...lyn.com, keescook@...omium.org,
john.johansen@...onical.com, penguin-kernel@...ove.sakura.ne.jp,
stephen.smalley.work@...il.com, linux-kernel@...r.kernel.org,
selinux@...r.kernel.org, Casey Schaufler <casey@...aufler-ca.com>
Subject: Re: [PATCH v4 4/4] Audit: Add record for multiple object contexts
On 7/4/2025 1:18 PM, Casey Schaufler wrote:
> On 6/16/2025 1:54 PM, Paul Moore wrote:
>> On Jun 6, 2025 Casey Schaufler <casey@...aufler-ca.com> wrote:
>>> Create a new audit record AUDIT_MAC_OBJ_CONTEXTS.
>>> An example of the MAC_OBJ_CONTEXTS record is:
>>>
>>> type=MAC_OBJ_CONTEXTS
>>> msg=audit(1601152467.009:1050):
>>> obj_selinux=unconfined_u:object_r:user_home_t:s0
>>>
>>> When an audit event includes a AUDIT_MAC_OBJ_CONTEXTS record
>>> the "obj=" field in other records in the event will be "obj=?".
>>> An AUDIT_MAC_OBJ_CONTEXTS record is supplied when the system has
>>> multiple security modules that may make access decisions based
>>> on an object security context.
>>>
>>> Signed-off-by: Casey Schaufler <casey@...aufler-ca.com>
>>> ---
>>> include/linux/audit.h | 7 +++++
>>> include/uapi/linux/audit.h | 1 +
>>> kernel/audit.c | 58 +++++++++++++++++++++++++++++++++++++-
>>> kernel/auditsc.c | 45 ++++++++---------------------
>>> security/selinux/hooks.c | 3 +-
>>> security/smack/smack_lsm.c | 3 +-
>>> 6 files changed, 80 insertions(+), 37 deletions(-)
>> ..
>>
>>> diff --git a/kernel/audit.c b/kernel/audit.c
>>> index 0987b2f391cc..451c36965889 100644
>>> --- a/kernel/audit.c
>>> +++ b/kernel/audit.c
>>> @@ -2337,6 +2344,55 @@ int audit_log_task_context(struct audit_buffer *ab)
>>> }
>>> EXPORT_SYMBOL(audit_log_task_context);
>>>
>>> +int audit_log_obj_ctx(struct audit_buffer *ab, struct lsm_prop *prop)
>>> +{
>>> + int i;
>>> + int rc;
>>> + int error = 0;
>>> + char *space = "";
>>> + struct lsm_context ctx;
>>> +
>>> + if (audit_obj_secctx_cnt < 2) {
>>> + error = security_lsmprop_to_secctx(prop, &ctx, LSM_ID_UNDEF);
>>> + if (error < 0) {
>>> + if (error != -EINVAL)
>>> + goto error_path;
>>> + return error;
>>> + }
>>> + audit_log_format(ab, " obj=%s", ctx.context);
>>> + security_release_secctx(&ctx);
>>> + return 0;
>>> + }
>>> + audit_log_format(ab, " obj=?");
>>> + error = audit_buffer_aux_new(ab, AUDIT_MAC_OBJ_CONTEXTS);
>>> + if (error)
>>> + goto error_path;
>>> +
>>> + for (i = 0; i < audit_obj_secctx_cnt; i++) {
>>> + rc = security_lsmprop_to_secctx(prop, &ctx,
>>> + audit_obj_lsms[i]->id);
>>> + if (rc < 0) {
>>> + audit_log_format(ab, "%sobj_%s=?", space,
>>> + audit_obj_lsms[i]->name);
>>> + if (rc != -EINVAL)
>>> + audit_panic("error in audit_log_obj_ctx");
>>> + error = rc;
>> Do we need the same logic as in audit_log_subj_ctx()?
> I seriously debated the issue. Subjects always have data to put in
> the aux record. Objects may or may not, in the AppArmor case. Not having
> a subject context is an error, not having an object context is interesting,
> but not necessarily an error. Hence the different treatment. You can tell
> me I'm wrong, and I'll make them consistent.
>
>>> + } else {
>>> + audit_log_format(ab, "%sobj_%s=%s", space,
>>> + audit_obj_lsms[i]->name, ctx.context);
>>> + security_release_secctx(&ctx);
>>> + }
>>> + space = " ";
>>> + }
>>> +
>>> + audit_buffer_aux_end(ab);
>>> + return error;
>>> +
>>> +error_path:
>>> + audit_panic("error in audit_log_obj_ctx");
>>> + return error;
>>> +}
>>> +
>>> void audit_log_d_path_exe(struct audit_buffer *ab,
>>> struct mm_struct *mm)
>>> {
>>> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
>>> index 322d4e27f28e..0c28fa33d099 100644
>>> --- a/kernel/auditsc.c
>>> +++ b/kernel/auditsc.c
>>> @@ -1098,7 +1098,6 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
>>> char *comm)
>>> {
>>> struct audit_buffer *ab;
>>> - struct lsm_context ctx;
>>> int rc = 0;
>>>
>>> ab = audit_log_start(context, GFP_KERNEL, AUDIT_OBJ_PID);
>>> @@ -1108,15 +1107,9 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
>>> audit_log_format(ab, "opid=%d oauid=%d ouid=%d oses=%d", pid,
>>> from_kuid(&init_user_ns, auid),
>>> from_kuid(&init_user_ns, uid), sessionid);
>>> - if (lsmprop_is_set(prop)) {
>>> - if (security_lsmprop_to_secctx(prop, &ctx, LSM_ID_UNDEF) < 0) {
>>> - audit_log_format(ab, " obj=(none)");
>>> - rc = 1;
>>> - } else {
>>> - audit_log_format(ab, " obj=%s", ctx.context);
>>> - security_release_secctx(&ctx);
>>> - }
>>> - }
>>> + if (lsmprop_is_set(prop) && audit_log_obj_ctx(ab, prop))
>>> + rc = 1;
>> We should probably use the return value from audit_log_obj_ctx().
> Sure.
On further inspection, the callers of audit_log_obj_ctx() don't
do anything with the return code, and similar functions have their
returns treated the same way. Unless there's a major rework of the
audit code there isn't any value in "using" the return code.
>>> audit_log_format(ab, " ocomm=");
>>> audit_log_untrustedstring(ab, comm);
>>> audit_log_end(ab);
>> ..
>>
>>> @@ -1780,15 +1756,16 @@ static void audit_log_exit(void)
>>> axs->target_sessionid[i],
>>> &axs->target_ref[i],
>>> axs->target_comm[i]))
>>> - call_panic = 1;
>>> + call_panic = 1;
>>> }
>>>
>>> if (context->target_pid &&
>>> audit_log_pid_context(context, context->target_pid,
>>> context->target_auid, context->target_uid,
>>> context->target_sessionid,
>>> - &context->target_ref, context->target_comm))
>>> - call_panic = 1;
>>> + &context->target_ref,
>>> + context->target_comm))
>>> + call_panic = 1;
>> I appreciate the indent fixes, would you mind pulling this out and
>> submitting them separately?
> Sure.
>
>> --
>> paul-moore.com
Powered by blists - more mailing lists