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 <[email protected]> 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 <[email protected]> >>> --- >>> 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
