Re: [PATCH]: revised make xfrm_audit_log more generic patch

[Steve Grubb <sgrubb@redhat.com>]

Hi,

I think we need just one other minor tweak.

On Monday 23 July 2007 17:46:05 Joy Latten wrote:
> @@ -2154,44 +2168,23 @@ EXPORT_SYMBOL(xfrm_bundle_ok);
>  /* Audit addition and deletion of SAs and ipsec policy */
>  
>  void xfrm_audit_log(uid_t auid, u32 sid, int type, int result,
> -                   struct xfrm_policy *xp, struct xfrm_state *x)
> +                    u16 family, xfrm_address_t saddr, xfrm_address_t
> daddr, +                    __be32 spi, __be32 flowlabel, struct
> xfrm_sec_ctx *sctx, +                    char *buf)
>  {
> -
>         char *secctx;
>         u32 secctx_len;
> -       struct xfrm_sec_ctx *sctx = NULL;
>         struct audit_buffer *audit_buf;
> -       int family;
>         extern int audit_enabled;
>  
>         if (audit_enabled == 0)
>                 return;
>  
> -       BUG_ON((type == AUDIT_MAC_IPSEC_ADDSA ||
> -               type == AUDIT_MAC_IPSEC_DELSA) && !x);
> -       BUG_ON((type == AUDIT_MAC_IPSEC_ADDSPD ||
> -               type == AUDIT_MAC_IPSEC_DELSPD) && !xp);
> -
>         audit_buf = audit_log_start(current->audit_context, GFP_ATOMIC,
> type); if (audit_buf == NULL)
>                 return;
>  
> -       switch(type) {
> -       case AUDIT_MAC_IPSEC_ADDSA:
> -               audit_log_format(audit_buf, "SAD add: auid=%u", auid);
> -               break;
> -       case AUDIT_MAC_IPSEC_DELSA:
> -               audit_log_format(audit_buf, "SAD delete: auid=%u", auid);
> -               break;
> -       case AUDIT_MAC_IPSEC_ADDSPD:
> -               audit_log_format(audit_buf, "SPD add: auid=%u", auid);
> -               break;
> -       case AUDIT_MAC_IPSEC_DELSPD:
> -               audit_log_format(audit_buf, "SPD delete: auid=%u", auid);
> -               break;
> -       default:
> -               return;
> -       }
> +       audit_log_format(audit_buf, "%s: auid=%u", buf, auid);
>  
>         if (sid != 0 &&
>                 security_secid_to_secctx(sid, &secctx, &secctx_len) == 0)

The operation in buf will not be parsed by the user space tools. Let's 
use "op=%s " where you have "%s: " above. Audit record fields are name=value 
and fields separated by spaces. "op" is what we are using in other places to 
mean operation. 

I know its a change from the records above, but we previously had some detail 
about what operation was being performed by the record type and this did not 
matter so much. Now that we only have one event type, the meaning of the 
event being recorded needs to be parsable and in a field. 

It also wouldn't hurt to change the text being sent to this function to have a 
hyphen instead of a space, so "SPD delete" becomes "SPD-delete". This keeps 
the parser happy.

This patch otherwise looks good.

-Steve