12:45 p.m.
On 3/6/2020 6:31 PM, Paul Moore wrote:
On Fri, Feb 21, 2020 at 7:06 PM Casey Schaufler
<casey(a)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 <sds(a)tycho.nsa.gov>
> Signed-off-by: Casey Schaufler <casey(a)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.