[PATCH V4 2/2] audit: normalize NETFILTER_PKT
Paul Moore
paul at paul-moore.com
Thu Mar 23 18:44:36 UTC 2017
On Wed, Mar 22, 2017 at 3:05 AM, 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>
> ---
> 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-audit at 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 <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>
> ---
> 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-audit at redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
--
paul moore
www.paul-moore.com
More information about the Linux-audit
mailing list