Re: [PATCH v15 21/23] Audit: Include object data for all security modules

[Paul Moore <paul@paul-moore.com>]

On Fri, Feb 21, 2020 at 7:06 PM Casey Schaufler <casey@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@tycho.nsa.gov>
> Signed-off-by: Casey Schaufler <casey@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 (why would anyone want to
do that?!) 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.

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.

-- 
paul moore
www.paul-moore.com


--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit