On 3/6/2020 6:31 PM, Paul Moore wrote: > On Fri, Feb 21, 2020 at 7:06 PM Casey Schaufler <ca...@schaufler-ca.com> > wrote: >> When there is more than one context displaying security >> module extend what goes into the audit record by supplimenting >> the "obj=" with an "obj_<lsm>=" for each such security >> module. >> >> Acked-by: Stephen Smalley <s...@tycho.nsa.gov> >> Signed-off-by: Casey Schaufler <ca...@schaufler-ca.com> >> Cc:linux-audit@redhat.com >> --- >> kernel/audit.h | 4 +- >> kernel/auditsc.c | 106 ++++++++++++++++++++++++----------------------- >> 2 files changed, 56 insertions(+), 54 deletions(-) > ... > >> diff --git a/kernel/auditsc.c b/kernel/auditsc.c >> index 68ae5fa843c1..7dab48661e31 100644 >> --- a/kernel/auditsc.c >> +++ b/kernel/auditsc.c >> @@ -956,13 +953,57 @@ static inline void audit_free_context(struct >> audit_context *context) >> kfree(context); >> } >> >> +static int audit_log_object_context(struct audit_buffer *ab, >> + struct lsmblob *blob) >> +{ >> + struct lsmcontext context; >> + const char *lsm; >> + int i; >> + >> + /* >> + * None of the installed modules have object labels. >> + */ >> + if (security_lsm_slot_name(0) == NULL) >> + return 0; >> + >> + if (blob->secid[0] != 0) { >> + if (security_secid_to_secctx(blob, &context, 0)) { >> + audit_log_format(ab, " obj=?"); >> + return 1; >> + } >> + audit_log_format(ab, " obj=%s", context.context); >> + security_release_secctx(&context); >> + } >> + >> + /* >> + * Don't do anything more unless there is more than one LSM >> + * with a security context to report. >> + */ >> + if (security_lsm_slot_name(1) == NULL) >> + return 0; >> + >> + for (i = 0; i < LSMBLOB_ENTRIES; i++) { >> + lsm = security_lsm_slot_name(i); >> + if (lsm == NULL) >> + break; >> + if (blob->secid[i] == 0) >> + continue; >> + if (security_secid_to_secctx(blob, &context, i)) { >> + audit_log_format(ab, " obj_%s=?", lsm); >> + continue; >> + } >> + audit_log_format(ab, " obj_%s=%s", lsm, context.context); >> + security_release_secctx(&context); >> + } >> + return 0; >> +} >> + >> static int audit_log_pid_context(struct audit_context *context, pid_t pid, >> kuid_t auid, kuid_t uid, >> unsigned int sessionid, >> struct lsmblob *blob, char *comm) >> { >> struct audit_buffer *ab; >> - struct lsmcontext lsmctx; >> int rc = 0; >> >> ab = audit_log_start(context, GFP_KERNEL, AUDIT_OBJ_PID); >> @@ -972,15 +1013,7 @@ 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 (lsmblob_is_set(blob)) { >> - if (security_secid_to_secctx(blob, &lsmctx, LSMBLOB_FIRST)) { >> - audit_log_format(ab, " obj=(none)"); >> - rc = 1; >> - } else { >> - audit_log_format(ab, " obj=%s", lsmctx.context); >> - security_release_secctx(&lsmctx); >> - } >> - } >> + rc = audit_log_object_context(ab, blob); >> audit_log_format(ab, " ocomm="); >> audit_log_untrustedstring(ab, comm); >> audit_log_end(ab); > I realized you don't hang around linux-audit
I do, but having implemented audit systems in the past I try not to interfere with someone else's approach for fear of getting sucked in to working on it. > (why would anyone want to do that?!) Keeping an eye on trends or possible Smack impact. > so let me tell you one of my most hated mantras: "new audit > fields MUST go at the end of the audit record". The "MUST" is in all > caps because either I'm being clever and reusing some IETF RFC > concepts, or I'm tired of arguing this point and feel like > capitalization is the best I can do for stress relief; maybe it is a > combination of the two. Feel free to pick whichever reason you find > most pleasing. I'll go with stress relief. Glad to be helpful. ;) > Either way, the "obj=" field should stay where it is, but the > "obj_XXX=" fields need to find their way to the end of the record. As Steve pointed out, there may be a bigger issue here. If the additional fields aren't going to fit in MAX_AUDIT_MESSAGE_LENGTH bytes another format may be required. I had hoped that perhaps obj_selinux= might count as a refinement to obj= and hence not be considered a new field, but it looks like that's not flying. -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit