On Wed, Mar 22, 2017 at 3:05 AM, Richard Guy Briggs <r...@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 <r...@redhat.com> > --- > v4: > Write out nfmark unmodified rather than trying to indicate "unset". > Collapse/simplify switch/case statements. > v3: > Don't store interim values, but print immediately. > v2: > Trim down to 4 fields. Add raw samples. > > net/netfilter/xt_AUDIT.c | 124 > ++++++++++------------------------------------ > 1 files changed, 27 insertions(+), 97 deletions(-)
Looks good, merged to audit/next. > diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c > index cdb7cee..582ee54 100644 > --- a/net/netfilter/xt_AUDIT.c > +++ b/net/netfilter/xt_AUDIT.c > @@ -31,146 +31,76 @@ MODULE_ALIAS("ip6t_AUDIT"); > MODULE_ALIAS("ebt_AUDIT"); > MODULE_ALIAS("arpt_AUDIT"); > > -static void audit_proto(struct audit_buffer *ab, struct sk_buff *skb, > - unsigned int proto, unsigned int offset) > -{ > - switch (proto) { > - case IPPROTO_TCP: > - case IPPROTO_UDP: > - case IPPROTO_UDPLITE: { > - const __be16 *pptr; > - __be16 _ports[2]; > - > - pptr = skb_header_pointer(skb, offset, sizeof(_ports), > _ports); > - if (pptr == NULL) { > - audit_log_format(ab, " truncated=1"); > - return; > - } > - > - audit_log_format(ab, " sport=%hu dport=%hu", > - ntohs(pptr[0]), ntohs(pptr[1])); > - } > - break; > - > - case IPPROTO_ICMP: > - case IPPROTO_ICMPV6: { > - const u8 *iptr; > - u8 _ih[2]; > - > - iptr = skb_header_pointer(skb, offset, sizeof(_ih), &_ih); > - if (iptr == NULL) { > - audit_log_format(ab, " truncated=1"); > - return; > - } > - > - audit_log_format(ab, " icmptype=%hhu icmpcode=%hhu", > - iptr[0], iptr[1]); > - > - } > - break; > - } > -} > - > -static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb) > +static bool audit_ip4(struct audit_buffer *ab, struct sk_buff *skb) > { > struct iphdr _iph; > const struct iphdr *ih; > > ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_iph), > &_iph); > - if (!ih) { > - audit_log_format(ab, " truncated=1"); > - return; > - } > + if (!ih) > + return false; > > - audit_log_format(ab, " saddr=%pI4 daddr=%pI4 ipid=%hu proto=%hhu", > - &ih->saddr, &ih->daddr, ntohs(ih->id), ih->protocol); > + audit_log_format(ab, " saddr=%pI4 daddr=%pI4 proto=%hhu", > + &ih->saddr, &ih->daddr, ih->protocol); > > - if (ntohs(ih->frag_off) & IP_OFFSET) { > - audit_log_format(ab, " frag=1"); > - return; > - } > - > - audit_proto(ab, skb, ih->protocol, ih->ihl * 4); > + return true; > } > > -static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb) > +static bool audit_ip6(struct audit_buffer *ab, struct sk_buff *skb) > { > struct ipv6hdr _ip6h; > const struct ipv6hdr *ih; > u8 nexthdr; > __be16 frag_off; > - int offset; > > ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_ip6h), > &_ip6h); > - if (!ih) { > - audit_log_format(ab, " truncated=1"); > - return; > - } > + if (!ih) > + return false; > > nexthdr = ih->nexthdr; > - offset = ipv6_skip_exthdr(skb, skb_network_offset(skb) + > sizeof(_ip6h), > - &nexthdr, &frag_off); > + ipv6_skip_exthdr(skb, skb_network_offset(skb) + sizeof(_ip6h), > &nexthdr, &frag_off); > > audit_log_format(ab, " saddr=%pI6c daddr=%pI6c proto=%hhu", > &ih->saddr, &ih->daddr, nexthdr); > > - if (offset) > - audit_proto(ab, skb, nexthdr, offset); > + return true; > } > > 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)); > - > - if (par->family == NFPROTO_BRIDGE) { > - switch (eth_hdr(skb)->h_proto) { > - case htons(ETH_P_IP): > - audit_ip4(ab, skb); > - break; > - > - case htons(ETH_P_IPV6): > - audit_ip6(ab, skb); > - break; > - } > - } > - } > + audit_log_format(ab, "mark=%#x", skb->mark); > > switch (par->family) { > + case NFPROTO_BRIDGE: > + switch (eth_hdr(skb)->h_proto) { > + case htons(ETH_P_IP): > + fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1; > + break; > + case htons(ETH_P_IPV6): > + fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1; > + break; > + } > + break; > case NFPROTO_IPV4: > - audit_ip4(ab, skb); > + fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1; > break; > - > case NFPROTO_IPV6: > - audit_ip6(ab, skb); > + fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1; > break; > } > > -#ifdef CONFIG_NETWORK_SECMARK > - if (skb->secmark) > - audit_log_secctx(ab, skb->secmark); > -#endif > + if (fam == -1) > + audit_log_format(ab, " saddr=? daddr=? proto=-1"); > > audit_log_end(ab); > > -- > 1.7.1 > > -- > Linux-audit mailing list > linux-au...@redhat.com > https://www.redhat.com/mailman/listinfo/linux-audit -- paul moore www.paul-moore.com On Wed, Mar 22, 2017 at 3:05 AM, Richard Guy Briggs <r...@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 <r...@redhat.com> > --- > v4: > Write out nfmark unmodified rather than trying to indicate "unset". > Collapse/simplify switch/case statements. > v3: > Don't store interim values, but print immediately. > v2: > Trim down to 4 fields. Add raw samples. > > net/netfilter/xt_AUDIT.c | 124 > ++++++++++------------------------------------ > 1 files changed, 27 insertions(+), 97 deletions(-) > > diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c > index cdb7cee..582ee54 100644 > --- a/net/netfilter/xt_AUDIT.c > +++ b/net/netfilter/xt_AUDIT.c > @@ -31,146 +31,76 @@ MODULE_ALIAS("ip6t_AUDIT"); > MODULE_ALIAS("ebt_AUDIT"); > MODULE_ALIAS("arpt_AUDIT"); > > -static void audit_proto(struct audit_buffer *ab, struct sk_buff *skb, > - unsigned int proto, unsigned int offset) > -{ > - switch (proto) { > - case IPPROTO_TCP: > - case IPPROTO_UDP: > - case IPPROTO_UDPLITE: { > - const __be16 *pptr; > - __be16 _ports[2]; > - > - pptr = skb_header_pointer(skb, offset, sizeof(_ports), > _ports); > - if (pptr == NULL) { > - audit_log_format(ab, " truncated=1"); > - return; > - } > - > - audit_log_format(ab, " sport=%hu dport=%hu", > - ntohs(pptr[0]), ntohs(pptr[1])); > - } > - break; > - > - case IPPROTO_ICMP: > - case IPPROTO_ICMPV6: { > - const u8 *iptr; > - u8 _ih[2]; > - > - iptr = skb_header_pointer(skb, offset, sizeof(_ih), &_ih); > - if (iptr == NULL) { > - audit_log_format(ab, " truncated=1"); > - return; > - } > - > - audit_log_format(ab, " icmptype=%hhu icmpcode=%hhu", > - iptr[0], iptr[1]); > - > - } > - break; > - } > -} > - > -static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb) > +static bool audit_ip4(struct audit_buffer *ab, struct sk_buff *skb) > { > struct iphdr _iph; > const struct iphdr *ih; > > ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_iph), > &_iph); > - if (!ih) { > - audit_log_format(ab, " truncated=1"); > - return; > - } > + if (!ih) > + return false; > > - audit_log_format(ab, " saddr=%pI4 daddr=%pI4 ipid=%hu proto=%hhu", > - &ih->saddr, &ih->daddr, ntohs(ih->id), ih->protocol); > + audit_log_format(ab, " saddr=%pI4 daddr=%pI4 proto=%hhu", > + &ih->saddr, &ih->daddr, ih->protocol); > > - if (ntohs(ih->frag_off) & IP_OFFSET) { > - audit_log_format(ab, " frag=1"); > - return; > - } > - > - audit_proto(ab, skb, ih->protocol, ih->ihl * 4); > + return true; > } > > -static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb) > +static bool audit_ip6(struct audit_buffer *ab, struct sk_buff *skb) > { > struct ipv6hdr _ip6h; > const struct ipv6hdr *ih; > u8 nexthdr; > __be16 frag_off; > - int offset; > > ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_ip6h), > &_ip6h); > - if (!ih) { > - audit_log_format(ab, " truncated=1"); > - return; > - } > + if (!ih) > + return false; > > nexthdr = ih->nexthdr; > - offset = ipv6_skip_exthdr(skb, skb_network_offset(skb) + > sizeof(_ip6h), > - &nexthdr, &frag_off); > + ipv6_skip_exthdr(skb, skb_network_offset(skb) + sizeof(_ip6h), > &nexthdr, &frag_off); > > audit_log_format(ab, " saddr=%pI6c daddr=%pI6c proto=%hhu", > &ih->saddr, &ih->daddr, nexthdr); > > - if (offset) > - audit_proto(ab, skb, nexthdr, offset); > + return true; > } > > 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)); > - > - if (par->family == NFPROTO_BRIDGE) { > - switch (eth_hdr(skb)->h_proto) { > - case htons(ETH_P_IP): > - audit_ip4(ab, skb); > - break; > - > - case htons(ETH_P_IPV6): > - audit_ip6(ab, skb); > - break; > - } > - } > - } > + audit_log_format(ab, "mark=%#x", skb->mark); > > switch (par->family) { > + case NFPROTO_BRIDGE: > + switch (eth_hdr(skb)->h_proto) { > + case htons(ETH_P_IP): > + fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1; > + break; > + case htons(ETH_P_IPV6): > + fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1; > + break; > + } > + break; > case NFPROTO_IPV4: > - audit_ip4(ab, skb); > + fam = audit_ip4(ab, skb) ? NFPROTO_IPV4 : -1; > break; > - > case NFPROTO_IPV6: > - audit_ip6(ab, skb); > + fam = audit_ip6(ab, skb) ? NFPROTO_IPV6 : -1; > break; > } > > -#ifdef CONFIG_NETWORK_SECMARK > - if (skb->secmark) > - audit_log_secctx(ab, skb->secmark); > -#endif > + if (fam == -1) > + audit_log_format(ab, " saddr=? daddr=? proto=-1"); > > audit_log_end(ab); > > -- > 1.7.1 > > -- > Linux-audit mailing list > linux-au...@redhat.com > https://www.redhat.com/mailman/listinfo/linux-audit -- paul moore www.paul-moore.com -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html