[PATCH V3] audit: normalize NETFILTER_PKT

[Paul Moore]

On Thu, Mar 2, 2017 at 9:00 PM, Richard Guy Briggs <rgb at redhat.com> wrote:
> On 2017-03-02 19:16, Paul Moore wrote:
>> On Wed, Mar 1, 2017 at 5:34 PM, Richard Guy Briggs <rgb at redhat.com> wrote:
>> > On 2017-03-01 17:19, Paul Moore wrote:
>> >> On Wed, Mar 1, 2017 at 11:28 AM, Richard Guy Briggs <rgb at redhat.com> wrote:
>> >> > On 2017-02-28 17:22, Paul Moore wrote:
>> >> >> On Sun, Feb 26, 2017 at 3:49 PM, Richard Guy Briggs <rgb at redhat.com> wrote:
>> >> >> > Eliminate flipping in and out of message fields, dropping fields in the process.
>> >> >> >
>> >> >> > Sample raw message format IPv4 UDP:
>> >> >> > type=NETFILTER_PKT msg=audit(1487874761.386:228):  mark=0xae8a2732 saddr=127.0.0.1 daddr=127.0.0.1 proto=17^]
>> >> >> > Sample raw message format IPv6 ICMP6:
>> >> >> > type=NETFILTER_PKT msg=audit(1487874761.381:227):  mark=0x223894b7 saddr=::1 daddr=::1 proto=58^]
>> >> >> >
>> >> >> > Issue: https://github.com/linux-audit/audit-kernel/issues/11
>> >> >> > Test case: https://github.com/linux-audit/audit-testsuite/issues/43
>> >> >> >
>> >> >> > Signed-off-by: Richard Guy Briggs <rgb at redhat.com>
>> >> >> > ---
>> >> >> >  net/netfilter/xt_AUDIT.c |  122 ++++++++++-----------------------------------
>> >> >> >  1 files changed, 27 insertions(+), 95 deletions(-)
>> >> >> >
>> >> >> > diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c
>> >> >> > index 4973cbd..945fa29 100644
>> >> >> > --- a/net/netfilter/xt_AUDIT.c
>> >> >> > +++ b/net/netfilter/xt_AUDIT.c
>> >> >> > @@ -31,146 +31,78 @@ MODULE_ALIAS("ip6t_AUDIT");

...

>> >> >> >  static unsigned int
>> >> >> >  audit_tg(struct sk_buff *skb, const struct xt_action_param *par)
>> >> >> >  {
>> >> >> > -       const struct xt_audit_info *info = par->targinfo;
>> >> >> >         struct audit_buffer *ab;
>> >> >> > +       int fam = -1;
>> >> >> >
>> >> >> >         if (audit_enabled == 0)
>> >> >> >                 goto errout;
>> >> >> > -
>> >> >> >         ab = audit_log_start(NULL, GFP_ATOMIC, AUDIT_NETFILTER_PKT);
>> >> >> >         if (ab == NULL)
>> >> >> >                 goto errout;
>> >> >> >
>> >> >> > -       audit_log_format(ab, "action=%hhu hook=%u len=%u inif=%s outif=%s",
>> >> >> > -                        info->type, par->hooknum, skb->len,
>> >> >> > -                        par->in ? par->in->name : "?",
>> >> >> > -                        par->out ? par->out->name : "?");
>> >> >> > -
>> >> >> > -       if (skb->mark)
>> >> >> > -               audit_log_format(ab, " mark=%#x", skb->mark);
>> >> >> > -
>> >> >> > -       if (skb->dev && skb->dev->type == ARPHRD_ETHER) {
>> >> >> > -               audit_log_format(ab, " smac=%pM dmac=%pM macproto=0x%04x",
>> >> >> > -                                eth_hdr(skb)->h_source, eth_hdr(skb)->h_dest,
>> >> >> > -                                ntohs(eth_hdr(skb)->h_proto));
>> >> >> > +       audit_log_format(ab, "mark=%#x", skb->mark ?: -1);
>> >> >>
>> >> >> How do Steve's userspace tools like the unset/-1 value represented
>> >> >> when it is a hex value: -1 or 0xffffffff?
>> >> >
>> >> > My understanding is they are set up to cope with this.
>> >>
>> >> How does userspace distinguish between an unset nfmark and a nfmark of
>> >> 0xffffffff?
>> >
>> > It never had to deal specifically with nfmark previously because it
>> > wasn't included if it was blank.  Generally other values that are -1 are
>> > interpreted by the audit userspace tools as unset (session id, auid,
>> > etc...)
>>
>> Yes, I know, let me get straight to the point: should we use "mark=-1"
>> when the nfmark is unset instead of "mark=0xffffffff"?
>
> I'd prefer to keep the format as it was, explicitly labelled hex.  The
> other fields that are printed as unset, -1, come out in the logs as
> MAX_UINT: "auid=4294967295 ses=4294967295", so I don't see any reason to
> change that convention.  Once that field is known by userspace tools,
> they can interpret (-i) that as -1.

Perhaps I'm missing something here, but let me ask again, how does
userspace distinguish between an unset nfmark and a nfmark of
0xffffffff?

-- 
paul moore
www.paul-moore.com