* [RFC PATCH] audit: normalize NETFILTER_PKT @ 2017-01-27 13:11 Richard Guy Briggs 2017-01-30 14:53 ` Steve Grubb 0 siblings, 1 reply; 14+ messages in thread From: Richard Guy Briggs @ 2017-01-27 13:11 UTC (permalink / raw) To: Netfilter Developer Mailing List, linux-audit Cc: Richard Guy Briggs, Thomas Graf, Eric Paris, Paul Moore, Steve Grubb Eliminate flipping in and out of message fields. https://github.com/linux-audit/audit-kernel/issues/11 Signed-off-by: Richard Guy Briggs <rgb@redhat.com> --- net/netfilter/xt_AUDIT.c | 92 +++++++++++++++++++++++++++++++++------------- 1 files changed, 66 insertions(+), 26 deletions(-) diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c index 4973cbd..8089ec2 100644 --- a/net/netfilter/xt_AUDIT.c +++ b/net/netfilter/xt_AUDIT.c @@ -31,24 +31,41 @@ MODULE_ALIAS("ip6t_AUDIT"); MODULE_ALIAS("ebt_AUDIT"); MODULE_ALIAS("arpt_AUDIT"); +struct nfpkt_par { + int ipv; + int iptrunc; + const void *saddr; + const void *daddr; + u16 ipid; + u8 proto; + u8 frag; + int ptrunc; + u16 sport; + u16 dport; + u8 icmpt; + u8 icmpc; +}; + static void audit_proto(struct audit_buffer *ab, struct sk_buff *skb, - unsigned int proto, unsigned int offset) + unsigned int proto, unsigned int offset, struct nfpkt_par *apar) { switch (proto) { case IPPROTO_TCP: case IPPROTO_UDP: - case IPPROTO_UDPLITE: { + case IPPROTO_UDPLITE: + case IPPROTO_DCCP: + case IPPROTO_SCTP: { const __be16 *pptr; __be16 _ports[2]; pptr = skb_header_pointer(skb, offset, sizeof(_ports), _ports); if (pptr == NULL) { - audit_log_format(ab, " truncated=1"); + apar->ptrunc = 1; return; } + apar->sport = ntohs(pptr[0]); + apar->dport = ntohs(pptr[1]); - audit_log_format(ab, " sport=%hu dport=%hu", - ntohs(pptr[0]), ntohs(pptr[1])); } break; @@ -59,41 +76,43 @@ static void audit_proto(struct audit_buffer *ab, struct sk_buff *skb, iptr = skb_header_pointer(skb, offset, sizeof(_ih), &_ih); if (iptr == NULL) { - audit_log_format(ab, " truncated=1"); + apar->ptrunc = 1; return; } - - audit_log_format(ab, " icmptype=%hhu icmpcode=%hhu", - iptr[0], iptr[1]); + apar->icmpt = iptr[0]; + apar->icmpc = iptr[1]; } break; } } -static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb) +static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb, struct nfpkt_par *apar) { struct iphdr _iph; const struct iphdr *ih; + apar->ipv = 4; ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph); if (!ih) { - audit_log_format(ab, " truncated=1"); + apar->iptrunc = 1; return; } - audit_log_format(ab, " saddr=%pI4 daddr=%pI4 ipid=%hu proto=%hhu", - &ih->saddr, &ih->daddr, ntohs(ih->id), ih->protocol); + apar->saddr = &ih->saddr; + apar->daddr = &ih->daddr; + apar->ipid = ntohs(ih->id); + apar->proto = ih->protocol; if (ntohs(ih->frag_off) & IP_OFFSET) { - audit_log_format(ab, " frag=1"); + apar->frag = 1; return; } - audit_proto(ab, skb, ih->protocol, ih->ihl * 4); + audit_proto(ab, skb, ih->protocol, ih->ihl * 4, apar); } -static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb) +static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb, struct nfpkt_par *apar) { struct ipv6hdr _ip6h; const struct ipv6hdr *ih; @@ -101,9 +120,10 @@ static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb) __be16 frag_off; int offset; + apar->ipv = 6; ih = skb_header_pointer(skb, skb_network_offset(skb), sizeof(_ip6h), &_ip6h); if (!ih) { - audit_log_format(ab, " truncated=1"); + apar->iptrunc = 1; return; } @@ -111,11 +131,12 @@ static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb) offset = 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); + apar->saddr = &ih->saddr; + apar->daddr = &ih->daddr; + apar->proto = nexthdr; if (offset) - audit_proto(ab, skb, nexthdr, offset); + audit_proto(ab, skb, nexthdr, offset, apar); } static unsigned int @@ -123,6 +144,9 @@ audit_tg(struct sk_buff *skb, const struct xt_action_param *par) { const struct xt_audit_info *info = par->targinfo; struct audit_buffer *ab; + struct nfpkt_par apar = { + -1, -1, NULL, NULL, -1, -1, -1, -1, -1, -1, -1, -1 + }; if (audit_enabled == 0) goto errout; @@ -136,8 +160,7 @@ audit_tg(struct sk_buff *skb, const struct xt_action_param *par) par->in ? par->in->name : "?", par->out ? par->out->name : "?"); - if (skb->mark) - audit_log_format(ab, " mark=%#x", skb->mark); + audit_log_format(ab, " mark=%#x", skb->mark ?: -1); if (skb->dev && skb->dev->type == ARPHRD_ETHER) { audit_log_format(ab, " smac=%pM dmac=%pM macproto=0x%04x", @@ -147,25 +170,42 @@ audit_tg(struct sk_buff *skb, const struct xt_action_param *par) if (par->family == NFPROTO_BRIDGE) { switch (eth_hdr(skb)->h_proto) { case htons(ETH_P_IP): - audit_ip4(ab, skb); + audit_ip4(ab, skb, &apar); break; case htons(ETH_P_IPV6): - audit_ip6(ab, skb); + audit_ip6(ab, skb, &apar); break; } } + } else { + audit_log_format(ab, " smac=? dmac=? macproto=0xffff"); } switch (par->family) { case NFPROTO_IPV4: - audit_ip4(ab, skb); + audit_ip4(ab, skb, &apar); break; case NFPROTO_IPV6: - audit_ip6(ab, skb); + audit_ip6(ab, skb, &apar); + break; + } + + switch (apar.ipv) { + case 4: + audit_log_format(ab, " trunc=%d saddr=%pI4 daddr=%pI4 ipid=%hu proto=%hhu frag=%d", + apar.iptrunc, apar.saddr, apar.daddr, apar.ipid, apar.proto, apar.frag); + break; + case 6: + audit_log_format(ab, " trunc=%d saddr=%pI6c daddr=%pI6c ipid=-1 proto=%hhu frag=-1", + apar.iptrunc, apar.saddr, apar.daddr, apar.proto); break; + default: + audit_log_format(ab, " trunc=-1 saddr=? daddr=? ipid=-1 proto=-1 frag=-1"); } + audit_log_format(ab, " trunc=%d sport=%hu dport=%hu icmptype=%hhu icmpcode=%hhu", + apar.ptrunc, apar.sport, apar.dport, apar.icmpt, apar.icmpc); #ifdef CONFIG_NETWORK_SECMARK if (skb->secmark) -- 1.7.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [RFC PATCH] audit: normalize NETFILTER_PKT 2017-01-27 13:11 [RFC PATCH] audit: normalize NETFILTER_PKT Richard Guy Briggs @ 2017-01-30 14:53 ` Steve Grubb 2017-01-30 15:13 ` Richard Guy Briggs 0 siblings, 1 reply; 14+ messages in thread From: Steve Grubb @ 2017-01-30 14:53 UTC (permalink / raw) To: Richard Guy Briggs Cc: Netfilter Developer Mailing List, linux-audit, Thomas Graf, Eric Paris, Paul Moore On Fri, 27 Jan 2017 08:11:06 -0500 Richard Guy Briggs <rgb@redhat.com> wrote: > Eliminate flipping in and out of message fields. > > https://github.com/linux-audit/audit-kernel/issues/11 Do you have sample events that shows how this changes the record format? I like to review how the event looks when a patch changes or adds a record. Thanks, -Steve > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > --- > net/netfilter/xt_AUDIT.c | 92 > +++++++++++++++++++++++++++++++++------------- 1 files changed, 66 > insertions(+), 26 deletions(-) > > diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c > index 4973cbd..8089ec2 100644 > --- a/net/netfilter/xt_AUDIT.c > +++ b/net/netfilter/xt_AUDIT.c > @@ -31,24 +31,41 @@ MODULE_ALIAS("ip6t_AUDIT"); > MODULE_ALIAS("ebt_AUDIT"); > MODULE_ALIAS("arpt_AUDIT"); > > +struct nfpkt_par { > + int ipv; > + int iptrunc; > + const void *saddr; > + const void *daddr; > + u16 ipid; > + u8 proto; > + u8 frag; > + int ptrunc; > + u16 sport; > + u16 dport; > + u8 icmpt; > + u8 icmpc; > +}; > + > static void audit_proto(struct audit_buffer *ab, struct sk_buff *skb, > - unsigned int proto, unsigned int offset) > + unsigned int proto, unsigned int offset, > struct nfpkt_par *apar) { > switch (proto) { > case IPPROTO_TCP: > case IPPROTO_UDP: > - case IPPROTO_UDPLITE: { > + case IPPROTO_UDPLITE: > + case IPPROTO_DCCP: > + case IPPROTO_SCTP: { > const __be16 *pptr; > __be16 _ports[2]; > > pptr = skb_header_pointer(skb, offset, > sizeof(_ports), _ports); if (pptr == NULL) { > - audit_log_format(ab, " truncated=1"); > + apar->ptrunc = 1; > return; > } > + apar->sport = ntohs(pptr[0]); > + apar->dport = ntohs(pptr[1]); > > - audit_log_format(ab, " sport=%hu dport=%hu", > - ntohs(pptr[0]), ntohs(pptr[1])); > } > break; > > @@ -59,41 +76,43 @@ static void audit_proto(struct audit_buffer *ab, > struct sk_buff *skb, > iptr = skb_header_pointer(skb, offset, sizeof(_ih), > &_ih); if (iptr == NULL) { > - audit_log_format(ab, " truncated=1"); > + apar->ptrunc = 1; > return; > } > - > - audit_log_format(ab, " icmptype=%hhu icmpcode=%hhu", > - iptr[0], iptr[1]); > + apar->icmpt = iptr[0]; > + apar->icmpc = iptr[1]; > > } > break; > } > } > > -static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb) > +static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb, > struct nfpkt_par *apar) { > struct iphdr _iph; > const struct iphdr *ih; > > + apar->ipv = 4; > ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph); > if (!ih) { > - audit_log_format(ab, " truncated=1"); > + apar->iptrunc = 1; > return; > } > > - audit_log_format(ab, " saddr=%pI4 daddr=%pI4 ipid=%hu > proto=%hhu", > - &ih->saddr, &ih->daddr, ntohs(ih->id), ih->protocol); > + apar->saddr = &ih->saddr; > + apar->daddr = &ih->daddr; > + apar->ipid = ntohs(ih->id); > + apar->proto = ih->protocol; > > if (ntohs(ih->frag_off) & IP_OFFSET) { > - audit_log_format(ab, " frag=1"); > + apar->frag = 1; > return; > } > > - audit_proto(ab, skb, ih->protocol, ih->ihl * 4); > + audit_proto(ab, skb, ih->protocol, ih->ihl * 4, apar); > } > > -static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb) > +static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb, > struct nfpkt_par *apar) { > struct ipv6hdr _ip6h; > const struct ipv6hdr *ih; > @@ -101,9 +120,10 @@ static void audit_ip6(struct audit_buffer *ab, > struct sk_buff *skb) __be16 frag_off; > int offset; > > + apar->ipv = 6; > ih = skb_header_pointer(skb, skb_network_offset(skb), > sizeof(_ip6h), &_ip6h); if (!ih) { > - audit_log_format(ab, " truncated=1"); > + apar->iptrunc = 1; > return; > } > > @@ -111,11 +131,12 @@ static void audit_ip6(struct audit_buffer *ab, > struct sk_buff *skb) offset = 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); > + apar->saddr = &ih->saddr; > + apar->daddr = &ih->daddr; > + apar->proto = nexthdr; > > if (offset) > - audit_proto(ab, skb, nexthdr, offset); > + audit_proto(ab, skb, nexthdr, offset, apar); > } > > static unsigned int > @@ -123,6 +144,9 @@ audit_tg(struct sk_buff *skb, const struct > xt_action_param *par) { > const struct xt_audit_info *info = par->targinfo; > struct audit_buffer *ab; > + struct nfpkt_par apar = { > + -1, -1, NULL, NULL, -1, -1, -1, -1, -1, -1, -1, -1 > + }; > > if (audit_enabled == 0) > goto errout; > @@ -136,8 +160,7 @@ audit_tg(struct sk_buff *skb, const struct > xt_action_param *par) par->in ? par->in->name : "?", > par->out ? par->out->name : "?"); > > - if (skb->mark) > - audit_log_format(ab, " mark=%#x", skb->mark); > + audit_log_format(ab, " mark=%#x", skb->mark ?: -1); > > if (skb->dev && skb->dev->type == ARPHRD_ETHER) { > audit_log_format(ab, " smac=%pM dmac=%pM > macproto=0x%04x", @@ -147,25 +170,42 @@ audit_tg(struct sk_buff *skb, > const struct xt_action_param *par) if (par->family == NFPROTO_BRIDGE) > { switch (eth_hdr(skb)->h_proto) { > case htons(ETH_P_IP): > - audit_ip4(ab, skb); > + audit_ip4(ab, skb, &apar); > break; > > case htons(ETH_P_IPV6): > - audit_ip6(ab, skb); > + audit_ip6(ab, skb, &apar); > break; > } > } > + } else { > + audit_log_format(ab, " smac=? dmac=? > macproto=0xffff"); } > > switch (par->family) { > case NFPROTO_IPV4: > - audit_ip4(ab, skb); > + audit_ip4(ab, skb, &apar); > break; > > case NFPROTO_IPV6: > - audit_ip6(ab, skb); > + audit_ip6(ab, skb, &apar); > + break; > + } > + > + switch (apar.ipv) { > + case 4: > + audit_log_format(ab, " trunc=%d saddr=%pI4 > daddr=%pI4 ipid=%hu proto=%hhu frag=%d", > + apar.iptrunc, apar.saddr, apar.daddr, > apar.ipid, apar.proto, apar.frag); > + break; > + case 6: > + audit_log_format(ab, " trunc=%d saddr=%pI6c > daddr=%pI6c ipid=-1 proto=%hhu frag=-1", > + apar.iptrunc, apar.saddr, apar.daddr, > apar.proto); break; > + default: > + audit_log_format(ab, " trunc=-1 saddr=? daddr=? > ipid=-1 proto=-1 frag=-1"); } > + audit_log_format(ab, " trunc=%d sport=%hu dport=%hu > icmptype=%hhu icmpcode=%hhu", > + apar.ptrunc, apar.sport, apar.dport, apar.icmpt, > apar.icmpc); > #ifdef CONFIG_NETWORK_SECMARK > if (skb->secmark) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH] audit: normalize NETFILTER_PKT 2017-01-30 14:53 ` Steve Grubb @ 2017-01-30 15:13 ` Richard Guy Briggs 2017-01-31 12:57 ` Richard Guy Briggs 0 siblings, 1 reply; 14+ messages in thread From: Richard Guy Briggs @ 2017-01-30 15:13 UTC (permalink / raw) To: Steve Grubb Cc: Netfilter Developer Mailing List, linux-audit, Thomas Graf, Eric Paris, Paul Moore On 2017-01-30 15:53, Steve Grubb wrote: > On Fri, 27 Jan 2017 08:11:06 -0500 > Richard Guy Briggs <rgb@redhat.com> wrote: > > Eliminate flipping in and out of message fields. > > > > https://github.com/linux-audit/audit-kernel/issues/11 > > Do you have sample events that shows how this changes the record > format? I like to review how the event looks when a patch changes or > adds a record. I used the format that was proposed. Here are several samples from running this RFC patch through the RFC test script: ausearch --start 01/27/2017 -i -m netfilter_pkt ---- type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.649:183) : action=ACCEPT hook=OUTPUT len=84 inif=? outif=lo mark=0xa044a1d4 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=26581 proto=icmp frag=255 trunc=-1 sport=65535 dport=65535 icmptype=echo icmpcode=0 ---- type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.649:184) : action=ACCEPT hook=INPUT len=84 inif=lo outif=? mark=0xbadeac28 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=26581 proto=icmp frag=255 trunc=-1 sport=65535 dport=65535 icmptype=echo icmpcode=0 ---- type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.652:185) : action=ACCEPT hook=INPUT len=104 inif=lo outif=? mark=0xb404724 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=ipv6-icmp frag=-1 trunc=-1 sport=65535 dport=65535 icmptype=unknown icmp type (128) icmpcode=0 ---- type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.655:186) : action=ACCEPT hook=INPUT len=60 inif=lo outif=? mark=0xe2bd8098 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=29381 proto=tcp frag=255 trunc=-1 sport=51064 dport=42424 icmptype=unknown icmp type (255) icmpcode=255 ---- type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.657:187) : action=ACCEPT hook=INPUT len=80 inif=lo outif=? mark=0xf80a9dd7 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=tcp frag=-1 trunc=-1 sport=38188 dport=42424 icmptype=unknown icmp type (255) icmpcode=255 ---- type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.659:188) : action=ACCEPT hook=INPUT len=31 inif=lo outif=? mark=0xa6d8d4ac smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=46304 proto=udp frag=255 trunc=-1 sport=60095 dport=42424 icmptype=unknown icmp type (255) icmpcode=255 ---- type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.661:189) : action=ACCEPT hook=INPUT len=51 inif=lo outif=? mark=0x3f0d6054 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=udp frag=-1 trunc=-1 sport=43818 dport=42424 icmptype=unknown icmp type (255) icmpcode=255 ---- > -Steve > > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > > --- > > net/netfilter/xt_AUDIT.c | 92 > > +++++++++++++++++++++++++++++++++------------- 1 files changed, 66 > > insertions(+), 26 deletions(-) > > > > diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c > > index 4973cbd..8089ec2 100644 > > --- a/net/netfilter/xt_AUDIT.c > > +++ b/net/netfilter/xt_AUDIT.c > > @@ -31,24 +31,41 @@ MODULE_ALIAS("ip6t_AUDIT"); > > MODULE_ALIAS("ebt_AUDIT"); > > MODULE_ALIAS("arpt_AUDIT"); > > > > +struct nfpkt_par { > > + int ipv; > > + int iptrunc; > > + const void *saddr; > > + const void *daddr; > > + u16 ipid; > > + u8 proto; > > + u8 frag; > > + int ptrunc; > > + u16 sport; > > + u16 dport; > > + u8 icmpt; > > + u8 icmpc; > > +}; > > + > > static void audit_proto(struct audit_buffer *ab, struct sk_buff *skb, > > - unsigned int proto, unsigned int offset) > > + unsigned int proto, unsigned int offset, > > struct nfpkt_par *apar) { > > switch (proto) { > > case IPPROTO_TCP: > > case IPPROTO_UDP: > > - case IPPROTO_UDPLITE: { > > + case IPPROTO_UDPLITE: > > + case IPPROTO_DCCP: > > + case IPPROTO_SCTP: { > > const __be16 *pptr; > > __be16 _ports[2]; > > > > pptr = skb_header_pointer(skb, offset, > > sizeof(_ports), _ports); if (pptr == NULL) { > > - audit_log_format(ab, " truncated=1"); > > + apar->ptrunc = 1; > > return; > > } > > + apar->sport = ntohs(pptr[0]); > > + apar->dport = ntohs(pptr[1]); > > > > - audit_log_format(ab, " sport=%hu dport=%hu", > > - ntohs(pptr[0]), ntohs(pptr[1])); > > } > > break; > > > > @@ -59,41 +76,43 @@ static void audit_proto(struct audit_buffer *ab, > > struct sk_buff *skb, > > iptr = skb_header_pointer(skb, offset, sizeof(_ih), > > &_ih); if (iptr == NULL) { > > - audit_log_format(ab, " truncated=1"); > > + apar->ptrunc = 1; > > return; > > } > > - > > - audit_log_format(ab, " icmptype=%hhu icmpcode=%hhu", > > - iptr[0], iptr[1]); > > + apar->icmpt = iptr[0]; > > + apar->icmpc = iptr[1]; > > > > } > > break; > > } > > } > > > > -static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb) > > +static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb, > > struct nfpkt_par *apar) { > > struct iphdr _iph; > > const struct iphdr *ih; > > > > + apar->ipv = 4; > > ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph); > > if (!ih) { > > - audit_log_format(ab, " truncated=1"); > > + apar->iptrunc = 1; > > return; > > } > > > > - audit_log_format(ab, " saddr=%pI4 daddr=%pI4 ipid=%hu > > proto=%hhu", > > - &ih->saddr, &ih->daddr, ntohs(ih->id), ih->protocol); > > + apar->saddr = &ih->saddr; > > + apar->daddr = &ih->daddr; > > + apar->ipid = ntohs(ih->id); > > + apar->proto = ih->protocol; > > > > if (ntohs(ih->frag_off) & IP_OFFSET) { > > - audit_log_format(ab, " frag=1"); > > + apar->frag = 1; > > return; > > } > > > > - audit_proto(ab, skb, ih->protocol, ih->ihl * 4); > > + audit_proto(ab, skb, ih->protocol, ih->ihl * 4, apar); > > } > > > > -static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb) > > +static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb, > > struct nfpkt_par *apar) { > > struct ipv6hdr _ip6h; > > const struct ipv6hdr *ih; > > @@ -101,9 +120,10 @@ static void audit_ip6(struct audit_buffer *ab, > > struct sk_buff *skb) __be16 frag_off; > > int offset; > > > > + apar->ipv = 6; > > ih = skb_header_pointer(skb, skb_network_offset(skb), > > sizeof(_ip6h), &_ip6h); if (!ih) { > > - audit_log_format(ab, " truncated=1"); > > + apar->iptrunc = 1; > > return; > > } > > > > @@ -111,11 +131,12 @@ static void audit_ip6(struct audit_buffer *ab, > > struct sk_buff *skb) offset = 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); > > + apar->saddr = &ih->saddr; > > + apar->daddr = &ih->daddr; > > + apar->proto = nexthdr; > > > > if (offset) > > - audit_proto(ab, skb, nexthdr, offset); > > + audit_proto(ab, skb, nexthdr, offset, apar); > > } > > > > static unsigned int > > @@ -123,6 +144,9 @@ audit_tg(struct sk_buff *skb, const struct > > xt_action_param *par) { > > const struct xt_audit_info *info = par->targinfo; > > struct audit_buffer *ab; > > + struct nfpkt_par apar = { > > + -1, -1, NULL, NULL, -1, -1, -1, -1, -1, -1, -1, -1 > > + }; > > > > if (audit_enabled == 0) > > goto errout; > > @@ -136,8 +160,7 @@ audit_tg(struct sk_buff *skb, const struct > > xt_action_param *par) par->in ? par->in->name : "?", > > par->out ? par->out->name : "?"); > > > > - if (skb->mark) > > - audit_log_format(ab, " mark=%#x", skb->mark); > > + audit_log_format(ab, " mark=%#x", skb->mark ?: -1); > > > > if (skb->dev && skb->dev->type == ARPHRD_ETHER) { > > audit_log_format(ab, " smac=%pM dmac=%pM > > macproto=0x%04x", @@ -147,25 +170,42 @@ audit_tg(struct sk_buff *skb, > > const struct xt_action_param *par) if (par->family == NFPROTO_BRIDGE) > > { switch (eth_hdr(skb)->h_proto) { > > case htons(ETH_P_IP): > > - audit_ip4(ab, skb); > > + audit_ip4(ab, skb, &apar); > > break; > > > > case htons(ETH_P_IPV6): > > - audit_ip6(ab, skb); > > + audit_ip6(ab, skb, &apar); > > break; > > } > > } > > + } else { > > + audit_log_format(ab, " smac=? dmac=? > > macproto=0xffff"); } > > > > switch (par->family) { > > case NFPROTO_IPV4: > > - audit_ip4(ab, skb); > > + audit_ip4(ab, skb, &apar); > > break; > > > > case NFPROTO_IPV6: > > - audit_ip6(ab, skb); > > + audit_ip6(ab, skb, &apar); > > + break; > > + } > > + > > + switch (apar.ipv) { > > + case 4: > > + audit_log_format(ab, " trunc=%d saddr=%pI4 > > daddr=%pI4 ipid=%hu proto=%hhu frag=%d", > > + apar.iptrunc, apar.saddr, apar.daddr, > > apar.ipid, apar.proto, apar.frag); > > + break; > > + case 6: > > + audit_log_format(ab, " trunc=%d saddr=%pI6c > > daddr=%pI6c ipid=-1 proto=%hhu frag=-1", > > + apar.iptrunc, apar.saddr, apar.daddr, > > apar.proto); break; > > + default: > > + audit_log_format(ab, " trunc=-1 saddr=? daddr=? > > ipid=-1 proto=-1 frag=-1"); } > > + audit_log_format(ab, " trunc=%d sport=%hu dport=%hu > > icmptype=%hhu icmpcode=%hhu", > > + apar.ptrunc, apar.sport, apar.dport, apar.icmpt, > > apar.icmpc); > > #ifdef CONFIG_NETWORK_SECMARK > > if (skb->secmark) > - RGB -- Richard Guy Briggs <rgb@redhat.com> Kernel Security Engineering, Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH] audit: normalize NETFILTER_PKT 2017-01-30 15:13 ` Richard Guy Briggs @ 2017-01-31 12:57 ` Richard Guy Briggs 2017-01-31 16:13 ` Steve Grubb 0 siblings, 1 reply; 14+ messages in thread From: Richard Guy Briggs @ 2017-01-31 12:57 UTC (permalink / raw) To: Steve Grubb Cc: Netfilter Developer Mailing List, linux-audit, Thomas Graf, Eric Paris, Paul Moore On 2017-01-30 10:13, Richard Guy Briggs wrote: > On 2017-01-30 15:53, Steve Grubb wrote: > > On Fri, 27 Jan 2017 08:11:06 -0500 > > Richard Guy Briggs <rgb@redhat.com> wrote: > > > Eliminate flipping in and out of message fields. > > > > > > https://github.com/linux-audit/audit-kernel/issues/11 > > > > Do you have sample events that shows how this changes the record > > format? I like to review how the event looks when a patch changes or > > adds a record. > > I used the format that was proposed. Here are several samples from > running this RFC patch through the RFC test script: > > ausearch --start 01/27/2017 -i -m netfilter_pkt > ---- > type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.649:183) : action=ACCEPT hook=OUTPUT len=84 inif=? outif=lo mark=0xa044a1d4 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=26581 proto=icmp frag=255 trunc=-1 sport=65535 dport=65535 icmptype=echo icmpcode=0 > ---- > type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.649:184) : action=ACCEPT hook=INPUT len=84 inif=lo outif=? mark=0xbadeac28 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=26581 proto=icmp frag=255 trunc=-1 sport=65535 dport=65535 icmptype=echo icmpcode=0 > ---- > type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.652:185) : action=ACCEPT hook=INPUT len=104 inif=lo outif=? mark=0xb404724 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=ipv6-icmp frag=-1 trunc=-1 sport=65535 dport=65535 icmptype=unknown icmp type (128) icmpcode=0 > ---- > type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.655:186) : action=ACCEPT hook=INPUT len=60 inif=lo outif=? mark=0xe2bd8098 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=29381 proto=tcp frag=255 trunc=-1 sport=51064 dport=42424 icmptype=unknown icmp type (255) icmpcode=255 > ---- > type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.657:187) : action=ACCEPT hook=INPUT len=80 inif=lo outif=? mark=0xf80a9dd7 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=tcp frag=-1 trunc=-1 sport=38188 dport=42424 icmptype=unknown icmp type (255) icmpcode=255 > ---- > type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.659:188) : action=ACCEPT hook=INPUT len=31 inif=lo outif=? mark=0xa6d8d4ac smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=46304 proto=udp frag=255 trunc=-1 sport=60095 dport=42424 icmptype=unknown icmp type (255) icmpcode=255 > ---- > type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.661:189) : action=ACCEPT hook=INPUT len=51 inif=lo outif=? mark=0x3f0d6054 smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=udp frag=-1 trunc=-1 sport=43818 dport=42424 icmptype=unknown icmp type (255) icmpcode=255 > ---- Here are the raw messages: ---- time->Fri Jan 27 08:03:35 2017 type=NETFILTER_PKT msg=audit(1485522215.649:183): action=0 hook=3 len=84 inif=? outif=lo mark=0xa044a1d4 smac=? dmac=? macproto=0xffff trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=26581 proto=1 frag=255 trunc=-1 sport=65535 dport=65535 icmptype=8 icmpcode=0 ---- time->Fri Jan 27 08:03:35 2017 type=NETFILTER_PKT msg=audit(1485522215.649:184): action=0 hook=1 len=84 inif=lo outif=? mark=0xbadeac28 smac=? dmac=? macproto=0xffff trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=26581 proto=1 frag=255 trunc=-1 sport=65535 dport=65535 icmptype=8 icmpcode=0 ---- time->Fri Jan 27 08:03:35 2017 type=NETFILTER_PKT msg=audit(1485522215.652:185): action=0 hook=1 len=104 inif=lo outif=? mark=0xb404724 smac=? dmac=? macproto=0xffff trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=58 frag=-1 trunc=-1 sport=65535 dport=65535 icmptype=128 icmpcode=0 ---- time->Fri Jan 27 08:03:35 2017 type=NETFILTER_PKT msg=audit(1485522215.655:186): action=0 hook=1 len=60 inif=lo outif=? mark=0xe2bd8098 smac=? dmac=? macproto=0xffff trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=29381 proto=6 frag=255 trunc=-1 sport=51064 dport=42424 icmptype=255 icmpcode=255 ---- time->Fri Jan 27 08:03:35 2017 type=NETFILTER_PKT msg=audit(1485522215.657:187): action=0 hook=1 len=80 inif=lo outif=? mark=0xf80a9dd7 smac=? dmac=? macproto=0xffff trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=6 frag=-1 trunc=-1 sport=38188 dport=42424 icmptype=255 icmpcode=255 ---- time->Fri Jan 27 08:03:35 2017 type=NETFILTER_PKT msg=audit(1485522215.659:188): action=0 hook=1 len=31 inif=lo outif=? mark=0xa6d8d4ac smac=? dmac=? macproto=0xffff trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=46304 proto=17 frag=255 trunc=-1 sport=60095 dport=42424 icmptype=255 icmpcode=255 ---- time->Fri Jan 27 08:03:35 2017 type=NETFILTER_PKT msg=audit(1485522215.661:189): action=0 hook=1 len=51 inif=lo outif=? mark=0x3f0d6054 smac=? dmac=? macproto=0xffff trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=17 frag=-1 trunc=-1 sport=43818 dport=42424 icmptype=255 icmpcode=255 ---- > > -Steve > > > > > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > > > --- > > > net/netfilter/xt_AUDIT.c | 92 > > > +++++++++++++++++++++++++++++++++------------- 1 files changed, 66 > > > insertions(+), 26 deletions(-) > > > > > > diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c > > > index 4973cbd..8089ec2 100644 > > > --- a/net/netfilter/xt_AUDIT.c > > > +++ b/net/netfilter/xt_AUDIT.c > > > @@ -31,24 +31,41 @@ MODULE_ALIAS("ip6t_AUDIT"); > > > MODULE_ALIAS("ebt_AUDIT"); > > > MODULE_ALIAS("arpt_AUDIT"); > > > > > > +struct nfpkt_par { > > > + int ipv; > > > + int iptrunc; > > > + const void *saddr; > > > + const void *daddr; > > > + u16 ipid; > > > + u8 proto; > > > + u8 frag; > > > + int ptrunc; > > > + u16 sport; > > > + u16 dport; > > > + u8 icmpt; > > > + u8 icmpc; > > > +}; > > > + > > > static void audit_proto(struct audit_buffer *ab, struct sk_buff *skb, > > > - unsigned int proto, unsigned int offset) > > > + unsigned int proto, unsigned int offset, > > > struct nfpkt_par *apar) { > > > switch (proto) { > > > case IPPROTO_TCP: > > > case IPPROTO_UDP: > > > - case IPPROTO_UDPLITE: { > > > + case IPPROTO_UDPLITE: > > > + case IPPROTO_DCCP: > > > + case IPPROTO_SCTP: { > > > const __be16 *pptr; > > > __be16 _ports[2]; > > > > > > pptr = skb_header_pointer(skb, offset, > > > sizeof(_ports), _ports); if (pptr == NULL) { > > > - audit_log_format(ab, " truncated=1"); > > > + apar->ptrunc = 1; > > > return; > > > } > > > + apar->sport = ntohs(pptr[0]); > > > + apar->dport = ntohs(pptr[1]); > > > > > > - audit_log_format(ab, " sport=%hu dport=%hu", > > > - ntohs(pptr[0]), ntohs(pptr[1])); > > > } > > > break; > > > > > > @@ -59,41 +76,43 @@ static void audit_proto(struct audit_buffer *ab, > > > struct sk_buff *skb, > > > iptr = skb_header_pointer(skb, offset, sizeof(_ih), > > > &_ih); if (iptr == NULL) { > > > - audit_log_format(ab, " truncated=1"); > > > + apar->ptrunc = 1; > > > return; > > > } > > > - > > > - audit_log_format(ab, " icmptype=%hhu icmpcode=%hhu", > > > - iptr[0], iptr[1]); > > > + apar->icmpt = iptr[0]; > > > + apar->icmpc = iptr[1]; > > > > > > } > > > break; > > > } > > > } > > > > > > -static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb) > > > +static void audit_ip4(struct audit_buffer *ab, struct sk_buff *skb, > > > struct nfpkt_par *apar) { > > > struct iphdr _iph; > > > const struct iphdr *ih; > > > > > > + apar->ipv = 4; > > > ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph); > > > if (!ih) { > > > - audit_log_format(ab, " truncated=1"); > > > + apar->iptrunc = 1; > > > return; > > > } > > > > > > - audit_log_format(ab, " saddr=%pI4 daddr=%pI4 ipid=%hu > > > proto=%hhu", > > > - &ih->saddr, &ih->daddr, ntohs(ih->id), ih->protocol); > > > + apar->saddr = &ih->saddr; > > > + apar->daddr = &ih->daddr; > > > + apar->ipid = ntohs(ih->id); > > > + apar->proto = ih->protocol; > > > > > > if (ntohs(ih->frag_off) & IP_OFFSET) { > > > - audit_log_format(ab, " frag=1"); > > > + apar->frag = 1; > > > return; > > > } > > > > > > - audit_proto(ab, skb, ih->protocol, ih->ihl * 4); > > > + audit_proto(ab, skb, ih->protocol, ih->ihl * 4, apar); > > > } > > > > > > -static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb) > > > +static void audit_ip6(struct audit_buffer *ab, struct sk_buff *skb, > > > struct nfpkt_par *apar) { > > > struct ipv6hdr _ip6h; > > > const struct ipv6hdr *ih; > > > @@ -101,9 +120,10 @@ static void audit_ip6(struct audit_buffer *ab, > > > struct sk_buff *skb) __be16 frag_off; > > > int offset; > > > > > > + apar->ipv = 6; > > > ih = skb_header_pointer(skb, skb_network_offset(skb), > > > sizeof(_ip6h), &_ip6h); if (!ih) { > > > - audit_log_format(ab, " truncated=1"); > > > + apar->iptrunc = 1; > > > return; > > > } > > > > > > @@ -111,11 +131,12 @@ static void audit_ip6(struct audit_buffer *ab, > > > struct sk_buff *skb) offset = 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); > > > + apar->saddr = &ih->saddr; > > > + apar->daddr = &ih->daddr; > > > + apar->proto = nexthdr; > > > > > > if (offset) > > > - audit_proto(ab, skb, nexthdr, offset); > > > + audit_proto(ab, skb, nexthdr, offset, apar); > > > } > > > > > > static unsigned int > > > @@ -123,6 +144,9 @@ audit_tg(struct sk_buff *skb, const struct > > > xt_action_param *par) { > > > const struct xt_audit_info *info = par->targinfo; > > > struct audit_buffer *ab; > > > + struct nfpkt_par apar = { > > > + -1, -1, NULL, NULL, -1, -1, -1, -1, -1, -1, -1, -1 > > > + }; > > > > > > if (audit_enabled == 0) > > > goto errout; > > > @@ -136,8 +160,7 @@ audit_tg(struct sk_buff *skb, const struct > > > xt_action_param *par) par->in ? par->in->name : "?", > > > par->out ? par->out->name : "?"); > > > > > > - if (skb->mark) > > > - audit_log_format(ab, " mark=%#x", skb->mark); > > > + audit_log_format(ab, " mark=%#x", skb->mark ?: -1); > > > > > > if (skb->dev && skb->dev->type == ARPHRD_ETHER) { > > > audit_log_format(ab, " smac=%pM dmac=%pM > > > macproto=0x%04x", @@ -147,25 +170,42 @@ audit_tg(struct sk_buff *skb, > > > const struct xt_action_param *par) if (par->family == NFPROTO_BRIDGE) > > > { switch (eth_hdr(skb)->h_proto) { > > > case htons(ETH_P_IP): > > > - audit_ip4(ab, skb); > > > + audit_ip4(ab, skb, &apar); > > > break; > > > > > > case htons(ETH_P_IPV6): > > > - audit_ip6(ab, skb); > > > + audit_ip6(ab, skb, &apar); > > > break; > > > } > > > } > > > + } else { > > > + audit_log_format(ab, " smac=? dmac=? > > > macproto=0xffff"); } > > > > > > switch (par->family) { > > > case NFPROTO_IPV4: > > > - audit_ip4(ab, skb); > > > + audit_ip4(ab, skb, &apar); > > > break; > > > > > > case NFPROTO_IPV6: > > > - audit_ip6(ab, skb); > > > + audit_ip6(ab, skb, &apar); > > > + break; > > > + } > > > + > > > + switch (apar.ipv) { > > > + case 4: > > > + audit_log_format(ab, " trunc=%d saddr=%pI4 > > > daddr=%pI4 ipid=%hu proto=%hhu frag=%d", > > > + apar.iptrunc, apar.saddr, apar.daddr, > > > apar.ipid, apar.proto, apar.frag); > > > + break; > > > + case 6: > > > + audit_log_format(ab, " trunc=%d saddr=%pI6c > > > daddr=%pI6c ipid=-1 proto=%hhu frag=-1", > > > + apar.iptrunc, apar.saddr, apar.daddr, > > > apar.proto); break; > > > + default: > > > + audit_log_format(ab, " trunc=-1 saddr=? daddr=? > > > ipid=-1 proto=-1 frag=-1"); } > > > + audit_log_format(ab, " trunc=%d sport=%hu dport=%hu > > > icmptype=%hhu icmpcode=%hhu", > > > + apar.ptrunc, apar.sport, apar.dport, apar.icmpt, > > > apar.icmpc); > > > #ifdef CONFIG_NETWORK_SECMARK > > > if (skb->secmark) > > > > - RGB > > -- > Richard Guy Briggs <rgb@redhat.com> > Kernel Security Engineering, Base Operating Systems, Red Hat > Remote, Ottawa, Canada > Voice: +1.647.777.2635, Internal: (81) 32635 - RGB -- Richard Guy Briggs <rgb@redhat.com> Kernel Security Engineering, Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH] audit: normalize NETFILTER_PKT 2017-01-31 12:57 ` Richard Guy Briggs @ 2017-01-31 16:13 ` Steve Grubb 2017-01-31 19:44 ` Richard Guy Briggs 0 siblings, 1 reply; 14+ messages in thread From: Steve Grubb @ 2017-01-31 16:13 UTC (permalink / raw) To: Richard Guy Briggs Cc: Netfilter Developer Mailing List, linux-audit, Thomas Graf, Eric Paris, Paul Moore On Tue, 31 Jan 2017 07:57:23 -0500 Richard Guy Briggs <rgb@redhat.com> wrote: > On 2017-01-30 10:13, Richard Guy Briggs wrote: > > On 2017-01-30 15:53, Steve Grubb wrote: > > > On Fri, 27 Jan 2017 08:11:06 -0500 > > > Richard Guy Briggs <rgb@redhat.com> wrote: > > > > Eliminate flipping in and out of message fields. > > > > > > > > https://github.com/linux-audit/audit-kernel/issues/11 > > > > > > Do you have sample events that shows how this changes the record > > > format? I like to review how the event looks when a patch changes > > > or adds a record. > > > > I used the format that was proposed. Here are several samples from > > running this RFC patch through the RFC test script: > > > > ausearch --start 01/27/2017 -i -m netfilter_pkt > > ---- > > type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.649:183) : > > action=ACCEPT hook=OUTPUT len=84 inif=? outif=lo mark=0xa044a1d4 > > smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 > > daddr=127.0.0.1 ipid=26581 proto=icmp frag=255 trunc=-1 sport=65535 > > dport=65535 icmptype=echo icmpcode=0 ---- type=NETFILTER_PKT > > msg=audit(01/27/2017 08:03:35.649:184) : action=ACCEPT hook=INPUT > > len=84 inif=lo outif=? mark=0xbadeac28 smac=? dmac=? > > macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 > > ipid=26581 proto=icmp frag=255 trunc=-1 sport=65535 dport=65535 > > icmptype=echo icmpcode=0 ---- type=NETFILTER_PKT > > msg=audit(01/27/2017 08:03:35.652:185) : action=ACCEPT hook=INPUT > > len=104 inif=lo outif=? mark=0xb404724 smac=? dmac=? > > macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1 > > proto=ipv6-icmp frag=-1 trunc=-1 sport=65535 dport=65535 > > icmptype=unknown icmp type (128) icmpcode=0 ---- type=NETFILTER_PKT > > msg=audit(01/27/2017 08:03:35.655:186) : action=ACCEPT hook=INPUT > > len=60 inif=lo outif=? mark=0xe2bd8098 smac=? dmac=? > > macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 > > ipid=29381 proto=tcp frag=255 trunc=-1 sport=51064 dport=42424 > > icmptype=unknown icmp type (255) icmpcode=255 ---- > > type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.657:187) : > > action=ACCEPT hook=INPUT len=80 inif=lo outif=? mark=0xf80a9dd7 > > smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1 > > proto=tcp frag=-1 trunc=-1 sport=38188 dport=42424 icmptype=unknown > > icmp type (255) icmpcode=255 ---- type=NETFILTER_PKT > > msg=audit(01/27/2017 08:03:35.659:188) : action=ACCEPT hook=INPUT > > len=31 inif=lo outif=? mark=0xa6d8d4ac smac=? dmac=? > > macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 > > ipid=46304 proto=udp frag=255 trunc=-1 sport=60095 dport=42424 > > icmptype=unknown icmp type (255) icmpcode=255 ---- > > type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.661:189) : > > action=ACCEPT hook=INPUT len=51 inif=lo outif=? mark=0x3f0d6054 > > smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1 > > proto=udp frag=-1 trunc=-1 sport=43818 dport=42424 icmptype=unknown > > icmp type (255) icmpcode=255 ---- > > Here are the raw messages: > > ---- > time->Fri Jan 27 08:03:35 2017 > type=NETFILTER_PKT msg=audit(1485522215.649:183): action=0 hook=3 > len=84 inif=? outif=lo mark=0xa044a1d4 smac=? dmac=? macproto=0xffff > trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=26581 proto=1 frag=255 > trunc=-1 sport=65535 dport=65535 icmptype=8 icmpcode=0 ---- time->Fri > Jan 27 08:03:35 2017 type=NETFILTER_PKT > msg=audit(1485522215.649:184): action=0 hook=1 len=84 inif=lo outif=? > mark=0xbadeac28 smac=? dmac=? macproto=0xffff trunc=-1 > saddr=127.0.0.1 daddr=127.0.0.1 ipid=26581 proto=1 frag=255 trunc=-1 > sport=65535 dport=65535 icmptype=8 icmpcode=0 ---- time->Fri Jan 27 > 08:03:35 2017 type=NETFILTER_PKT msg=audit(1485522215.652:185): > action=0 hook=1 len=104 inif=lo outif=? mark=0xb404724 smac=? dmac=? > macproto=0xffff trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=58 frag=-1 > trunc=-1 sport=65535 dport=65535 icmptype=128 icmpcode=0 ---- > time->Fri Jan 27 08:03:35 2017 type=NETFILTER_PKT > msg=audit(1485522215.655:186): action=0 hook=1 len=60 inif=lo outif=? > mark=0xe2bd8098 smac=? dmac=? macproto=0xffff trunc=-1 > saddr=127.0.0.1 daddr=127.0.0.1 ipid=29381 proto=6 frag=255 trunc=-1 > sport=51064 dport=42424 icmptype=255 icmpcode=255 ---- time->Fri Jan > 27 08:03:35 2017 type=NETFILTER_PKT msg=audit(1485522215.657:187): > action=0 hook=1 len=80 inif=lo outif=? mark=0xf80a9dd7 smac=? dmac=? > macproto=0xffff trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=6 frag=-1 > trunc=-1 sport=38188 dport=42424 icmptype=255 icmpcode=255 ---- > time->Fri Jan 27 08:03:35 2017 type=NETFILTER_PKT > msg=audit(1485522215.659:188): action=0 hook=1 len=31 inif=lo outif=? > mark=0xa6d8d4ac smac=? dmac=? macproto=0xffff trunc=-1 > saddr=127.0.0.1 daddr=127.0.0.1 ipid=46304 proto=17 frag=255 trunc=-1 > sport=60095 dport=42424 icmptype=255 icmpcode=255 ---- time->Fri Jan > 27 08:03:35 2017 type=NETFILTER_PKT msg=audit(1485522215.661:189): > action=0 hook=1 len=51 inif=lo outif=? mark=0x3f0d6054 smac=? dmac=? > macproto=0xffff trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=17 frag=-1 > trunc=-1 sport=43818 dport=42424 icmptype=255 icmpcode=255 ---- I was curious about something. Auparse is trying to interpret the icmptype field for every event. This is not good. Which fields are valid for each kind of packet? Are there fields valid for all packets? Is the icmptype/code the only ones that don't apply in all cases? Thanks, -Steve > > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > > > > --- > > > > net/netfilter/xt_AUDIT.c | 92 > > > > +++++++++++++++++++++++++++++++++------------- 1 files changed, > > > > 66 insertions(+), 26 deletions(-) > > > > > > > > diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c > > > > index 4973cbd..8089ec2 100644 > > > > --- a/net/netfilter/xt_AUDIT.c > > > > +++ b/net/netfilter/xt_AUDIT.c > > > > @@ -31,24 +31,41 @@ MODULE_ALIAS("ip6t_AUDIT"); > > > > MODULE_ALIAS("ebt_AUDIT"); > > > > MODULE_ALIAS("arpt_AUDIT"); > > > > > > > > +struct nfpkt_par { > > > > + int ipv; > > > > + int iptrunc; > > > > + const void *saddr; > > > > + const void *daddr; > > > > + u16 ipid; > > > > + u8 proto; > > > > + u8 frag; > > > > + int ptrunc; > > > > + u16 sport; > > > > + u16 dport; > > > > + u8 icmpt; > > > > + u8 icmpc; > > > > +}; > > > > + > > > > static void audit_proto(struct audit_buffer *ab, struct > > > > sk_buff *skb, > > > > - unsigned int proto, unsigned int > > > > offset) > > > > + unsigned int proto, unsigned int > > > > offset, struct nfpkt_par *apar) { > > > > switch (proto) { > > > > case IPPROTO_TCP: > > > > case IPPROTO_UDP: > > > > - case IPPROTO_UDPLITE: { > > > > + case IPPROTO_UDPLITE: > > > > + case IPPROTO_DCCP: > > > > + case IPPROTO_SCTP: { > > > > const __be16 *pptr; > > > > __be16 _ports[2]; > > > > > > > > pptr = skb_header_pointer(skb, offset, > > > > sizeof(_ports), _ports); if (pptr == NULL) { > > > > - audit_log_format(ab, " truncated=1"); > > > > + apar->ptrunc = 1; > > > > return; > > > > } > > > > + apar->sport = ntohs(pptr[0]); > > > > + apar->dport = ntohs(pptr[1]); > > > > > > > > - audit_log_format(ab, " sport=%hu dport=%hu", > > > > - ntohs(pptr[0]), > > > > ntohs(pptr[1])); } > > > > break; > > > > > > > > @@ -59,41 +76,43 @@ static void audit_proto(struct audit_buffer > > > > *ab, struct sk_buff *skb, > > > > iptr = skb_header_pointer(skb, offset, > > > > sizeof(_ih), &_ih); if (iptr == NULL) { > > > > - audit_log_format(ab, " truncated=1"); > > > > + apar->ptrunc = 1; > > > > return; > > > > } > > > > - > > > > - audit_log_format(ab, " icmptype=%hhu > > > > icmpcode=%hhu", > > > > - iptr[0], iptr[1]); > > > > + apar->icmpt = iptr[0]; > > > > + apar->icmpc = iptr[1]; > > > > > > > > } > > > > break; > > > > } > > > > } > > > > > > > > -static void audit_ip4(struct audit_buffer *ab, struct sk_buff > > > > *skb) +static void audit_ip4(struct audit_buffer *ab, struct > > > > sk_buff *skb, struct nfpkt_par *apar) { > > > > struct iphdr _iph; > > > > const struct iphdr *ih; > > > > > > > > + apar->ipv = 4; > > > > ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph); > > > > if (!ih) { > > > > - audit_log_format(ab, " truncated=1"); > > > > + apar->iptrunc = 1; > > > > return; > > > > } > > > > > > > > - audit_log_format(ab, " saddr=%pI4 daddr=%pI4 ipid=%hu > > > > proto=%hhu", > > > > - &ih->saddr, &ih->daddr, ntohs(ih->id), > > > > ih->protocol); > > > > + apar->saddr = &ih->saddr; > > > > + apar->daddr = &ih->daddr; > > > > + apar->ipid = ntohs(ih->id); > > > > + apar->proto = ih->protocol; > > > > > > > > if (ntohs(ih->frag_off) & IP_OFFSET) { > > > > - audit_log_format(ab, " frag=1"); > > > > + apar->frag = 1; > > > > return; > > > > } > > > > > > > > - audit_proto(ab, skb, ih->protocol, ih->ihl * 4); > > > > + audit_proto(ab, skb, ih->protocol, ih->ihl * 4, apar); > > > > } > > > > > > > > -static void audit_ip6(struct audit_buffer *ab, struct sk_buff > > > > *skb) +static void audit_ip6(struct audit_buffer *ab, struct > > > > sk_buff *skb, struct nfpkt_par *apar) { > > > > struct ipv6hdr _ip6h; > > > > const struct ipv6hdr *ih; > > > > @@ -101,9 +120,10 @@ static void audit_ip6(struct audit_buffer > > > > *ab, struct sk_buff *skb) __be16 frag_off; > > > > int offset; > > > > > > > > + apar->ipv = 6; > > > > ih = skb_header_pointer(skb, skb_network_offset(skb), > > > > sizeof(_ip6h), &_ip6h); if (!ih) { > > > > - audit_log_format(ab, " truncated=1"); > > > > + apar->iptrunc = 1; > > > > return; > > > > } > > > > > > > > @@ -111,11 +131,12 @@ static void audit_ip6(struct audit_buffer > > > > *ab, struct sk_buff *skb) offset = 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); > > > > + apar->saddr = &ih->saddr; > > > > + apar->daddr = &ih->daddr; > > > > + apar->proto = nexthdr; > > > > > > > > if (offset) > > > > - audit_proto(ab, skb, nexthdr, offset); > > > > + audit_proto(ab, skb, nexthdr, offset, apar); > > > > } > > > > > > > > static unsigned int > > > > @@ -123,6 +144,9 @@ audit_tg(struct sk_buff *skb, const struct > > > > xt_action_param *par) { > > > > const struct xt_audit_info *info = par->targinfo; > > > > struct audit_buffer *ab; > > > > + struct nfpkt_par apar = { > > > > + -1, -1, NULL, NULL, -1, -1, -1, -1, -1, -1, > > > > -1, -1 > > > > + }; > > > > > > > > if (audit_enabled == 0) > > > > goto errout; > > > > @@ -136,8 +160,7 @@ audit_tg(struct sk_buff *skb, const struct > > > > xt_action_param *par) par->in ? par->in->name : "?", > > > > par->out ? par->out->name : "?"); > > > > > > > > - if (skb->mark) > > > > - audit_log_format(ab, " mark=%#x", skb->mark); > > > > + audit_log_format(ab, " mark=%#x", skb->mark ?: -1); > > > > > > > > if (skb->dev && skb->dev->type == ARPHRD_ETHER) { > > > > audit_log_format(ab, " smac=%pM dmac=%pM > > > > macproto=0x%04x", @@ -147,25 +170,42 @@ audit_tg(struct sk_buff > > > > *skb, const struct xt_action_param *par) if (par->family == > > > > NFPROTO_BRIDGE) { switch (eth_hdr(skb)->h_proto) { > > > > case htons(ETH_P_IP): > > > > - audit_ip4(ab, skb); > > > > + audit_ip4(ab, skb, &apar); > > > > break; > > > > > > > > case htons(ETH_P_IPV6): > > > > - audit_ip6(ab, skb); > > > > + audit_ip6(ab, skb, &apar); > > > > break; > > > > } > > > > } > > > > + } else { > > > > + audit_log_format(ab, " smac=? dmac=? > > > > macproto=0xffff"); } > > > > > > > > switch (par->family) { > > > > case NFPROTO_IPV4: > > > > - audit_ip4(ab, skb); > > > > + audit_ip4(ab, skb, &apar); > > > > break; > > > > > > > > case NFPROTO_IPV6: > > > > - audit_ip6(ab, skb); > > > > + audit_ip6(ab, skb, &apar); > > > > + break; > > > > + } > > > > + > > > > + switch (apar.ipv) { > > > > + case 4: > > > > + audit_log_format(ab, " trunc=%d saddr=%pI4 > > > > daddr=%pI4 ipid=%hu proto=%hhu frag=%d", > > > > + apar.iptrunc, apar.saddr, apar.daddr, > > > > apar.ipid, apar.proto, apar.frag); > > > > + break; > > > > + case 6: > > > > + audit_log_format(ab, " trunc=%d saddr=%pI6c > > > > daddr=%pI6c ipid=-1 proto=%hhu frag=-1", > > > > + apar.iptrunc, apar.saddr, apar.daddr, > > > > apar.proto); break; > > > > + default: > > > > + audit_log_format(ab, " trunc=-1 saddr=? daddr=? > > > > ipid=-1 proto=-1 frag=-1"); } > > > > + audit_log_format(ab, " trunc=%d sport=%hu dport=%hu > > > > icmptype=%hhu icmpcode=%hhu", > > > > + apar.ptrunc, apar.sport, apar.dport, > > > > apar.icmpt, apar.icmpc); > > > > #ifdef CONFIG_NETWORK_SECMARK > > > > if (skb->secmark) > > > > > > > - RGB > > > > -- > > Richard Guy Briggs <rgb@redhat.com> > > Kernel Security Engineering, Base Operating Systems, Red Hat > > Remote, Ottawa, Canada > > Voice: +1.647.777.2635, Internal: (81) 32635 > > - RGB > > -- > Richard Guy Briggs <rgb@redhat.com> > Kernel Security Engineering, Base Operating Systems, Red Hat > Remote, Ottawa, Canada > Voice: +1.647.777.2635, Internal: (81) 32635 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH] audit: normalize NETFILTER_PKT 2017-01-31 16:13 ` Steve Grubb @ 2017-01-31 19:44 ` Richard Guy Briggs 2017-02-03 23:44 ` Paul Moore 0 siblings, 1 reply; 14+ messages in thread From: Richard Guy Briggs @ 2017-01-31 19:44 UTC (permalink / raw) To: Steve Grubb Cc: Netfilter Developer Mailing List, linux-audit, Thomas Graf, Eric Paris, Paul Moore On 2017-01-31 17:13, Steve Grubb wrote: > On Tue, 31 Jan 2017 07:57:23 -0500 > Richard Guy Briggs <rgb@redhat.com> wrote: > > > On 2017-01-30 10:13, Richard Guy Briggs wrote: > > > On 2017-01-30 15:53, Steve Grubb wrote: > > > > On Fri, 27 Jan 2017 08:11:06 -0500 > > > > Richard Guy Briggs <rgb@redhat.com> wrote: > > > > > Eliminate flipping in and out of message fields. > > > > > > > > > > https://github.com/linux-audit/audit-kernel/issues/11 > > > > > > > > Do you have sample events that shows how this changes the record > > > > format? I like to review how the event looks when a patch changes > > > > or adds a record. > > > > > > I used the format that was proposed. Here are several samples from > > > running this RFC patch through the RFC test script: > > > > > > ausearch --start 01/27/2017 -i -m netfilter_pkt > > > ---- > > > type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.649:183) : > > > action=ACCEPT hook=OUTPUT len=84 inif=? outif=lo mark=0xa044a1d4 > > > smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 > > > daddr=127.0.0.1 ipid=26581 proto=icmp frag=255 trunc=-1 sport=65535 > > > dport=65535 icmptype=echo icmpcode=0 ---- type=NETFILTER_PKT > > > msg=audit(01/27/2017 08:03:35.649:184) : action=ACCEPT hook=INPUT > > > len=84 inif=lo outif=? mark=0xbadeac28 smac=? dmac=? > > > macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 > > > ipid=26581 proto=icmp frag=255 trunc=-1 sport=65535 dport=65535 > > > icmptype=echo icmpcode=0 ---- type=NETFILTER_PKT > > > msg=audit(01/27/2017 08:03:35.652:185) : action=ACCEPT hook=INPUT > > > len=104 inif=lo outif=? mark=0xb404724 smac=? dmac=? > > > macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1 > > > proto=ipv6-icmp frag=-1 trunc=-1 sport=65535 dport=65535 > > > icmptype=unknown icmp type (128) icmpcode=0 ---- type=NETFILTER_PKT > > > msg=audit(01/27/2017 08:03:35.655:186) : action=ACCEPT hook=INPUT > > > len=60 inif=lo outif=? mark=0xe2bd8098 smac=? dmac=? > > > macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 > > > ipid=29381 proto=tcp frag=255 trunc=-1 sport=51064 dport=42424 > > > icmptype=unknown icmp type (255) icmpcode=255 ---- > > > type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.657:187) : > > > action=ACCEPT hook=INPUT len=80 inif=lo outif=? mark=0xf80a9dd7 > > > smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1 > > > proto=tcp frag=-1 trunc=-1 sport=38188 dport=42424 icmptype=unknown > > > icmp type (255) icmpcode=255 ---- type=NETFILTER_PKT > > > msg=audit(01/27/2017 08:03:35.659:188) : action=ACCEPT hook=INPUT > > > len=31 inif=lo outif=? mark=0xa6d8d4ac smac=? dmac=? > > > macproto=UNKNOWN trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 > > > ipid=46304 proto=udp frag=255 trunc=-1 sport=60095 dport=42424 > > > icmptype=unknown icmp type (255) icmpcode=255 ---- > > > type=NETFILTER_PKT msg=audit(01/27/2017 08:03:35.661:189) : > > > action=ACCEPT hook=INPUT len=51 inif=lo outif=? mark=0x3f0d6054 > > > smac=? dmac=? macproto=UNKNOWN trunc=-1 saddr=::1 daddr=::1 ipid=-1 > > > proto=udp frag=-1 trunc=-1 sport=43818 dport=42424 icmptype=unknown > > > icmp type (255) icmpcode=255 ---- > > > > Here are the raw messages: > > > > ---- > > time->Fri Jan 27 08:03:35 2017 > > type=NETFILTER_PKT msg=audit(1485522215.649:183): action=0 hook=3 > > len=84 inif=? outif=lo mark=0xa044a1d4 smac=? dmac=? macproto=0xffff > > trunc=-1 saddr=127.0.0.1 daddr=127.0.0.1 ipid=26581 proto=1 frag=255 > > trunc=-1 sport=65535 dport=65535 icmptype=8 icmpcode=0 ---- time->Fri > > Jan 27 08:03:35 2017 type=NETFILTER_PKT > > msg=audit(1485522215.649:184): action=0 hook=1 len=84 inif=lo outif=? > > mark=0xbadeac28 smac=? dmac=? macproto=0xffff trunc=-1 > > saddr=127.0.0.1 daddr=127.0.0.1 ipid=26581 proto=1 frag=255 trunc=-1 > > sport=65535 dport=65535 icmptype=8 icmpcode=0 ---- time->Fri Jan 27 > > 08:03:35 2017 type=NETFILTER_PKT msg=audit(1485522215.652:185): > > action=0 hook=1 len=104 inif=lo outif=? mark=0xb404724 smac=? dmac=? > > macproto=0xffff trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=58 frag=-1 > > trunc=-1 sport=65535 dport=65535 icmptype=128 icmpcode=0 ---- > > time->Fri Jan 27 08:03:35 2017 type=NETFILTER_PKT > > msg=audit(1485522215.655:186): action=0 hook=1 len=60 inif=lo outif=? > > mark=0xe2bd8098 smac=? dmac=? macproto=0xffff trunc=-1 > > saddr=127.0.0.1 daddr=127.0.0.1 ipid=29381 proto=6 frag=255 trunc=-1 > > sport=51064 dport=42424 icmptype=255 icmpcode=255 ---- time->Fri Jan > > 27 08:03:35 2017 type=NETFILTER_PKT msg=audit(1485522215.657:187): > > action=0 hook=1 len=80 inif=lo outif=? mark=0xf80a9dd7 smac=? dmac=? > > macproto=0xffff trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=6 frag=-1 > > trunc=-1 sport=38188 dport=42424 icmptype=255 icmpcode=255 ---- > > time->Fri Jan 27 08:03:35 2017 type=NETFILTER_PKT > > msg=audit(1485522215.659:188): action=0 hook=1 len=31 inif=lo outif=? > > mark=0xa6d8d4ac smac=? dmac=? macproto=0xffff trunc=-1 > > saddr=127.0.0.1 daddr=127.0.0.1 ipid=46304 proto=17 frag=255 trunc=-1 > > sport=60095 dport=42424 icmptype=255 icmpcode=255 ---- time->Fri Jan > > 27 08:03:35 2017 type=NETFILTER_PKT msg=audit(1485522215.661:189): > > action=0 hook=1 len=51 inif=lo outif=? mark=0x3f0d6054 smac=? dmac=? > > macproto=0xffff trunc=-1 saddr=::1 daddr=::1 ipid=-1 proto=17 frag=-1 > > trunc=-1 sport=43818 dport=42424 icmptype=255 icmpcode=255 ---- > > I was curious about something. Auparse is trying to interpret the > icmptype field for every event. This is not good. Which fields are > valid for each kind of packet? Are there fields valid for all packets? > Is the icmptype/code the only ones that don't apply in all cases? Ok, this is important to know... You sound surprised. So if that field isn't valid for all cases of that event, then the event should be split or the "unset" value should be used as a hint to ignore it. This was the point of my earlier posting: https://www.redhat.com/archives/linux-audit/2017-January/msg00074.html There are still a number of questions from that thread that had no reply. Answering those questions would help inform this discussion, so if you could answer some of those questions in that first thread, I'd have a better chance of understanding what are the limitations of the parser and design/work around them. There is no packet for which all fields are valid. This is why using "unset" values in those fields was suggested, seemed to be accepted in discussion, and implemented. ICMP codes are obviously not valid on TCP or UDP packets, so maybe there should be a new message type(s?) such as NETFILTER_PKT_DATA for TCP/UDP/DCCP/SCTP and/or NETFILTER_PKT_CTRL for ICMP. For IPv4 vs IPv6 packets, IPv4 adds ipid= and frag= fields which are not necessarily valid for IPv6. Do you want NETFILTER_PKT_IPV{4,6}_{DATA,CTRL} message types? There's an optional ethernet bridge. Should messages with those fields use a new message type so that we have: NETFILTER_PKT{,_MAC}_IPV{4,6}_{DATA,CTRL} giving us 8 message types. There are other combinations too that could explode that message type count including non-TCP/UDP/DCCP/SCTP/ICMP transport layer protocols and even network layer protocols. Swinging fields in and out makes it very handy to use one message type for all of them and can save precious disk bandwidth, but the point was to normalize these messages. Is that still realistic and necessary? If so, we're trying to find a balance between message type explosion and disk bandwidth. We either need to make this fine-grained, ignore fields that aren't valid for that type, or swing fields in and out. Or maybe I have missed something fundamental, such as the presence of subsequent fields depends on the values of previous fields? > -Steve > > > > > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > > > > > --- > > > > > net/netfilter/xt_AUDIT.c | 92 > > > > > +++++++++++++++++++++++++++++++++------------- 1 files changed, > > > > > 66 insertions(+), 26 deletions(-) > > > > > > > > > > diff --git a/net/netfilter/xt_AUDIT.c b/net/netfilter/xt_AUDIT.c > > > > > index 4973cbd..8089ec2 100644 > > > > > --- a/net/netfilter/xt_AUDIT.c > > > > > +++ b/net/netfilter/xt_AUDIT.c > > > > > @@ -31,24 +31,41 @@ MODULE_ALIAS("ip6t_AUDIT"); > > > > > MODULE_ALIAS("ebt_AUDIT"); > > > > > MODULE_ALIAS("arpt_AUDIT"); > > > > > > > > > > +struct nfpkt_par { > > > > > + int ipv; > > > > > + int iptrunc; > > > > > + const void *saddr; > > > > > + const void *daddr; > > > > > + u16 ipid; > > > > > + u8 proto; > > > > > + u8 frag; > > > > > + int ptrunc; > > > > > + u16 sport; > > > > > + u16 dport; > > > > > + u8 icmpt; > > > > > + u8 icmpc; > > > > > +}; > > > > > + > > > > > static void audit_proto(struct audit_buffer *ab, struct > > > > > sk_buff *skb, > > > > > - unsigned int proto, unsigned int > > > > > offset) > > > > > + unsigned int proto, unsigned int > > > > > offset, struct nfpkt_par *apar) { > > > > > switch (proto) { > > > > > case IPPROTO_TCP: > > > > > case IPPROTO_UDP: > > > > > - case IPPROTO_UDPLITE: { > > > > > + case IPPROTO_UDPLITE: > > > > > + case IPPROTO_DCCP: > > > > > + case IPPROTO_SCTP: { > > > > > const __be16 *pptr; > > > > > __be16 _ports[2]; > > > > > > > > > > pptr = skb_header_pointer(skb, offset, > > > > > sizeof(_ports), _ports); if (pptr == NULL) { > > > > > - audit_log_format(ab, " truncated=1"); > > > > > + apar->ptrunc = 1; > > > > > return; > > > > > } > > > > > + apar->sport = ntohs(pptr[0]); > > > > > + apar->dport = ntohs(pptr[1]); > > > > > > > > > > - audit_log_format(ab, " sport=%hu dport=%hu", > > > > > - ntohs(pptr[0]), > > > > > ntohs(pptr[1])); } > > > > > break; > > > > > > > > > > @@ -59,41 +76,43 @@ static void audit_proto(struct audit_buffer > > > > > *ab, struct sk_buff *skb, > > > > > iptr = skb_header_pointer(skb, offset, > > > > > sizeof(_ih), &_ih); if (iptr == NULL) { > > > > > - audit_log_format(ab, " truncated=1"); > > > > > + apar->ptrunc = 1; > > > > > return; > > > > > } > > > > > - > > > > > - audit_log_format(ab, " icmptype=%hhu > > > > > icmpcode=%hhu", > > > > > - iptr[0], iptr[1]); > > > > > + apar->icmpt = iptr[0]; > > > > > + apar->icmpc = iptr[1]; > > > > > > > > > > } > > > > > break; > > > > > } > > > > > } > > > > > > > > > > -static void audit_ip4(struct audit_buffer *ab, struct sk_buff > > > > > *skb) +static void audit_ip4(struct audit_buffer *ab, struct > > > > > sk_buff *skb, struct nfpkt_par *apar) { > > > > > struct iphdr _iph; > > > > > const struct iphdr *ih; > > > > > > > > > > + apar->ipv = 4; > > > > > ih = skb_header_pointer(skb, 0, sizeof(_iph), &_iph); > > > > > if (!ih) { > > > > > - audit_log_format(ab, " truncated=1"); > > > > > + apar->iptrunc = 1; > > > > > return; > > > > > } > > > > > > > > > > - audit_log_format(ab, " saddr=%pI4 daddr=%pI4 ipid=%hu > > > > > proto=%hhu", > > > > > - &ih->saddr, &ih->daddr, ntohs(ih->id), > > > > > ih->protocol); > > > > > + apar->saddr = &ih->saddr; > > > > > + apar->daddr = &ih->daddr; > > > > > + apar->ipid = ntohs(ih->id); > > > > > + apar->proto = ih->protocol; > > > > > > > > > > if (ntohs(ih->frag_off) & IP_OFFSET) { > > > > > - audit_log_format(ab, " frag=1"); > > > > > + apar->frag = 1; > > > > > return; > > > > > } > > > > > > > > > > - audit_proto(ab, skb, ih->protocol, ih->ihl * 4); > > > > > + audit_proto(ab, skb, ih->protocol, ih->ihl * 4, apar); > > > > > } > > > > > > > > > > -static void audit_ip6(struct audit_buffer *ab, struct sk_buff > > > > > *skb) +static void audit_ip6(struct audit_buffer *ab, struct > > > > > sk_buff *skb, struct nfpkt_par *apar) { > > > > > struct ipv6hdr _ip6h; > > > > > const struct ipv6hdr *ih; > > > > > @@ -101,9 +120,10 @@ static void audit_ip6(struct audit_buffer > > > > > *ab, struct sk_buff *skb) __be16 frag_off; > > > > > int offset; > > > > > > > > > > + apar->ipv = 6; > > > > > ih = skb_header_pointer(skb, skb_network_offset(skb), > > > > > sizeof(_ip6h), &_ip6h); if (!ih) { > > > > > - audit_log_format(ab, " truncated=1"); > > > > > + apar->iptrunc = 1; > > > > > return; > > > > > } > > > > > > > > > > @@ -111,11 +131,12 @@ static void audit_ip6(struct audit_buffer > > > > > *ab, struct sk_buff *skb) offset = 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); > > > > > + apar->saddr = &ih->saddr; > > > > > + apar->daddr = &ih->daddr; > > > > > + apar->proto = nexthdr; > > > > > > > > > > if (offset) > > > > > - audit_proto(ab, skb, nexthdr, offset); > > > > > + audit_proto(ab, skb, nexthdr, offset, apar); > > > > > } > > > > > > > > > > static unsigned int > > > > > @@ -123,6 +144,9 @@ audit_tg(struct sk_buff *skb, const struct > > > > > xt_action_param *par) { > > > > > const struct xt_audit_info *info = par->targinfo; > > > > > struct audit_buffer *ab; > > > > > + struct nfpkt_par apar = { > > > > > + -1, -1, NULL, NULL, -1, -1, -1, -1, -1, -1, > > > > > -1, -1 > > > > > + }; > > > > > > > > > > if (audit_enabled == 0) > > > > > goto errout; > > > > > @@ -136,8 +160,7 @@ audit_tg(struct sk_buff *skb, const struct > > > > > xt_action_param *par) par->in ? par->in->name : "?", > > > > > par->out ? par->out->name : "?"); > > > > > > > > > > - if (skb->mark) > > > > > - audit_log_format(ab, " mark=%#x", skb->mark); > > > > > + audit_log_format(ab, " mark=%#x", skb->mark ?: -1); > > > > > > > > > > if (skb->dev && skb->dev->type == ARPHRD_ETHER) { > > > > > audit_log_format(ab, " smac=%pM dmac=%pM > > > > > macproto=0x%04x", @@ -147,25 +170,42 @@ audit_tg(struct sk_buff > > > > > *skb, const struct xt_action_param *par) if (par->family == > > > > > NFPROTO_BRIDGE) { switch (eth_hdr(skb)->h_proto) { > > > > > case htons(ETH_P_IP): > > > > > - audit_ip4(ab, skb); > > > > > + audit_ip4(ab, skb, &apar); > > > > > break; > > > > > > > > > > case htons(ETH_P_IPV6): > > > > > - audit_ip6(ab, skb); > > > > > + audit_ip6(ab, skb, &apar); > > > > > break; > > > > > } > > > > > } > > > > > + } else { > > > > > + audit_log_format(ab, " smac=? dmac=? > > > > > macproto=0xffff"); } > > > > > > > > > > switch (par->family) { > > > > > case NFPROTO_IPV4: > > > > > - audit_ip4(ab, skb); > > > > > + audit_ip4(ab, skb, &apar); > > > > > break; > > > > > > > > > > case NFPROTO_IPV6: > > > > > - audit_ip6(ab, skb); > > > > > + audit_ip6(ab, skb, &apar); > > > > > + break; > > > > > + } > > > > > + > > > > > + switch (apar.ipv) { > > > > > + case 4: > > > > > + audit_log_format(ab, " trunc=%d saddr=%pI4 > > > > > daddr=%pI4 ipid=%hu proto=%hhu frag=%d", > > > > > + apar.iptrunc, apar.saddr, apar.daddr, > > > > > apar.ipid, apar.proto, apar.frag); > > > > > + break; > > > > > + case 6: > > > > > + audit_log_format(ab, " trunc=%d saddr=%pI6c > > > > > daddr=%pI6c ipid=-1 proto=%hhu frag=-1", > > > > > + apar.iptrunc, apar.saddr, apar.daddr, > > > > > apar.proto); break; > > > > > + default: > > > > > + audit_log_format(ab, " trunc=-1 saddr=? daddr=? > > > > > ipid=-1 proto=-1 frag=-1"); } > > > > > + audit_log_format(ab, " trunc=%d sport=%hu dport=%hu > > > > > icmptype=%hhu icmpcode=%hhu", > > > > > + apar.ptrunc, apar.sport, apar.dport, > > > > > apar.icmpt, apar.icmpc); > > > > > #ifdef CONFIG_NETWORK_SECMARK > > > > > if (skb->secmark) > > > > > > > > > > - RGB > > > > > > -- > > > Richard Guy Briggs <rgb@redhat.com> > > > Kernel Security Engineering, Base Operating Systems, Red Hat > > > Remote, Ottawa, Canada > > > Voice: +1.647.777.2635, Internal: (81) 32635 > > > > - RGB > > > > -- > > Richard Guy Briggs <rgb@redhat.com> > > Kernel Security Engineering, Base Operating Systems, Red Hat > > Remote, Ottawa, Canada > > Voice: +1.647.777.2635, Internal: (81) 32635 > - RGB -- Richard Guy Briggs <rgb@redhat.com> Kernel Security Engineering, Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH] audit: normalize NETFILTER_PKT 2017-01-31 19:44 ` Richard Guy Briggs @ 2017-02-03 23:44 ` Paul Moore 2017-02-04 13:25 ` Steve Grubb 0 siblings, 1 reply; 14+ messages in thread From: Paul Moore @ 2017-02-03 23:44 UTC (permalink / raw) To: Richard Guy Briggs Cc: Steve Grubb, linux-audit, Netfilter Developer Mailing List, Thomas Graf On Tue, Jan 31, 2017 at 2:44 PM, Richard Guy Briggs <rgb@redhat.com> wrote: > On 2017-01-31 17:13, Steve Grubb wrote: ... >> I was curious about something. Auparse is trying to interpret the >> icmptype field for every event. This is not good. Which fields are >> valid for each kind of packet? Are there fields valid for all packets? >> Is the icmptype/code the only ones that don't apply in all cases? > > Ok, this is important to know... You sound surprised. So if that field > isn't valid for all cases of that event, then the event should be split > or the "unset" value should be used as a hint to ignore it. > > This was the point of my earlier posting: > https://www.redhat.com/archives/linux-audit/2017-January/msg00074.html > There are still a number of questions from that thread that had no > reply. Answering those questions would help inform this discussion, so > if you could answer some of those questions in that first thread, I'd > have a better chance of understanding what are the limitations of the > parser and design/work around them. > > There is no packet for which all fields are valid. This is why using > "unset" values in those fields was suggested, seemed to be accepted in > discussion, and implemented. ... > Swinging fields in and out makes it very handy to use one message type > for all of them and can save precious disk bandwidth, but the point was > to normalize these messages. Is that still realistic and necessary? If > so, we're trying to find a balance between message type explosion and > disk bandwidth. > > We either need to make this fine-grained, ignore fields that aren't > valid for that type, or swing fields in and out. Or maybe I have missed > something fundamental, such as the presence of subsequent fields depends > on the values of previous fields? I'm still trying to understand what purpose this record actually serves, and what requirements may exist. In an earlier thread somewhere Steve mentioned some broad requirements around data import/export, and I really wonder if the NETFILTER_PKT record provides anything useful here when it really isn't connecting the traffic to the sender/receiver without a lot of additional logging and post-processing smarts. If you were interested in data import/export I think auditing the socket syscalls would provide a much more useful set of records in the audit log. Considering that one of the primary motivations for the audit subsystem is to enable compliance with various security specifications, let's get the ones we know about listed in this thread and then figure out how best to meet those requirements. -- paul moore www.paul-moore.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH] audit: normalize NETFILTER_PKT 2017-02-03 23:44 ` Paul Moore @ 2017-02-04 13:25 ` Steve Grubb 2017-02-06 19:41 ` Paul Moore 0 siblings, 1 reply; 14+ messages in thread From: Steve Grubb @ 2017-02-04 13:25 UTC (permalink / raw) To: Paul Moore Cc: Richard Guy Briggs, linux-audit, Netfilter Developer Mailing List, Thomas Graf On Friday, February 3, 2017 6:44:16 PM EST Paul Moore wrote: > On Tue, Jan 31, 2017 at 2:44 PM, Richard Guy Briggs <rgb@redhat.com> wrote: > > On 2017-01-31 17:13, Steve Grubb wrote: > ... > > >> I was curious about something. Auparse is trying to interpret the > >> icmptype field for every event. This is not good. Which fields are > >> valid for each kind of packet? Are there fields valid for all packets? > >> Is the icmptype/code the only ones that don't apply in all cases? > > > > Ok, this is important to know... You sound surprised. So if that field > > isn't valid for all cases of that event, then the event should be split > > or the "unset" value should be used as a hint to ignore it. > > > > This was the point of my earlier posting: > > https://www.redhat.com/archives/linux-audit/2017-January/msg00074. > > html > > > > There are still a number of questions from that thread that had no > > reply. Answering those questions would help inform this discussion, so > > if you could answer some of those questions in that first thread, I'd > > have a better chance of understanding what are the limitations of the > > parser and design/work around them. > > > > There is no packet for which all fields are valid. This is why using > > "unset" values in those fields was suggested, seemed to be accepted in > > discussion, and implemented. > > ... > > > Swinging fields in and out makes it very handy to use one message type > > for all of them and can save precious disk bandwidth, but the point was > > to normalize these messages. Is that still realistic and necessary? If > > so, we're trying to find a balance between message type explosion and > > disk bandwidth. > > > > We either need to make this fine-grained, ignore fields that aren't > > valid for that type, or swing fields in and out. Or maybe I have missed > > something fundamental, such as the presence of subsequent fields depends > > on the values of previous fields? > > I'm still trying to understand what purpose this record actually > serves, and what requirements may exist. In an earlier thread > somewhere Steve mentioned some broad requirements around data > import/export, and I really wonder if the NETFILTER_PKT record > provides anything useful here when it really isn't connecting the > traffic to the sender/receiver without a lot of additional logging and > post-processing smarts. If you were interested in data import/export > I think auditing the socket syscalls would provide a much more useful > set of records in the audit log. The problem here is we cannot be selective enough through the syscall interface to get exactly what we want. For example, any auditing of connect and accept will also get af_unix traffic which is likely to be uid/gid lookups through sssd or glibc. Typically we want the IPv4/6 traffic. The netfilter rules are better suited to describing which packets are of interest. > Considering that one of the primary motivations for the audit > subsystem is to enable compliance with various security > specifications, let's get the ones we know about listed in this thread > and then figure out how best to meet those requirements. Common Criteria calls out for the ability to detect any attempt at information flow. Everything else leverages the CC requirements. -Steve ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH] audit: normalize NETFILTER_PKT 2017-02-04 13:25 ` Steve Grubb @ 2017-02-06 19:41 ` Paul Moore 2017-02-07 21:22 ` Richard Guy Briggs 0 siblings, 1 reply; 14+ messages in thread From: Paul Moore @ 2017-02-06 19:41 UTC (permalink / raw) To: Steve Grubb Cc: Richard Guy Briggs, linux-audit, Netfilter Developer Mailing List, Thomas Graf On Sat, Feb 4, 2017 at 8:25 AM, Steve Grubb <sgrubb@redhat.com> wrote: > On Friday, February 3, 2017 6:44:16 PM EST Paul Moore wrote: >> I'm still trying to understand what purpose this record actually >> serves, and what requirements may exist. In an earlier thread >> somewhere Steve mentioned some broad requirements around data >> import/export, and I really wonder if the NETFILTER_PKT record >> provides anything useful here when it really isn't connecting the >> traffic to the sender/receiver without a lot of additional logging and >> post-processing smarts. If you were interested in data import/export >> I think auditing the socket syscalls would provide a much more useful >> set of records in the audit log. > > The problem here is we cannot be selective enough through the syscall > interface to get exactly what we want. For example, any auditing of connect > and accept will also get af_unix traffic which is likely to be uid/gid lookups > through sssd or glibc. Typically we want the IPv4/6 traffic. The netfilter rules > are better suited to describing which packets are of interest. Okay, but how useful are these NETFILTER_PKT records, really? The only linkage you have back to the process on the local machine is via the addr/proto/port tuple and that seems far from ideal. >> Considering that one of the primary motivations for the audit >> subsystem is to enable compliance with various security >> specifications, let's get the ones we know about listed in this thread >> and then figure out how best to meet those requirements. > > Common Criteria calls out for the ability to detect any attempt at information > flow. Everything else leverages the CC requirements. Yes, you've mentioned this previously. This is good, but we need to make these requirements a bit more concrete; we need something we can use to arrive at a working implementation that satisfies these requirements. If this is purely about information flowing from A to B, would the source and destination addr/proto/port for TCP and UDP suffice? Do we need anything else? -- paul moore www.paul-moore.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH] audit: normalize NETFILTER_PKT 2017-02-06 19:41 ` Paul Moore @ 2017-02-07 21:22 ` Richard Guy Briggs 2017-02-08 4:02 ` Paul Moore 0 siblings, 1 reply; 14+ messages in thread From: Richard Guy Briggs @ 2017-02-07 21:22 UTC (permalink / raw) To: Paul Moore; +Cc: Netfilter Developer Mailing List, Thomas Graf, linux-audit On 2017-02-06 14:41, Paul Moore wrote: > On Sat, Feb 4, 2017 at 8:25 AM, Steve Grubb <sgrubb@redhat.com> wrote: > > On Friday, February 3, 2017 6:44:16 PM EST Paul Moore wrote: > >> I'm still trying to understand what purpose this record actually > >> serves, and what requirements may exist. In an earlier thread > >> somewhere Steve mentioned some broad requirements around data > >> import/export, and I really wonder if the NETFILTER_PKT record > >> provides anything useful here when it really isn't connecting the > >> traffic to the sender/receiver without a lot of additional logging and > >> post-processing smarts. If you were interested in data import/export > >> I think auditing the socket syscalls would provide a much more useful > >> set of records in the audit log. > > > > The problem here is we cannot be selective enough through the syscall > > interface to get exactly what we want. For example, any auditing of connect > > and accept will also get af_unix traffic which is likely to be uid/gid lookups > > through sssd or glibc. Typically we want the IPv4/6 traffic. The netfilter rules > > are better suited to describing which packets are of interest. > > Okay, but how useful are these NETFILTER_PKT records, really? The > only linkage you have back to the process on the local machine is via > the addr/proto/port tuple and that seems far from ideal. And even that could be spoofed easily and gathering more corroborating information would seem useful. Would the presence of the SOCKADDR record in any SYSCALL record be useful for somehow tagging a class of fd as being of interest? > >> Considering that one of the primary motivations for the audit > >> subsystem is to enable compliance with various security > >> specifications, let's get the ones we know about listed in this thread > >> and then figure out how best to meet those requirements. > > > > Common Criteria calls out for the ability to detect any attempt at information > > flow. Everything else leverages the CC requirements. > > Yes, you've mentioned this previously. This is good, but we need to > make these requirements a bit more concrete; we need something we can > use to arrive at a working implementation that satisfies these > requirements. > > If this is purely about information flowing from A to B, would the > source and destination addr/proto/port for TCP and UDP suffice? Do we > need anything else? > > -- > paul moore > www.paul-moore.com - RGB -- Richard Guy Briggs <rgb@redhat.com> Kernel Security Engineering, Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH] audit: normalize NETFILTER_PKT 2017-02-07 21:22 ` Richard Guy Briggs @ 2017-02-08 4:02 ` Paul Moore 2017-02-08 12:32 ` Richard Guy Briggs 0 siblings, 1 reply; 14+ messages in thread From: Paul Moore @ 2017-02-08 4:02 UTC (permalink / raw) To: Richard Guy Briggs Cc: linux-audit, Netfilter Developer Mailing List, Thomas Graf On Tue, Feb 7, 2017 at 4:22 PM, Richard Guy Briggs <rgb@redhat.com> wrote: > On 2017-02-06 14:41, Paul Moore wrote: >> On Sat, Feb 4, 2017 at 8:25 AM, Steve Grubb <sgrubb@redhat.com> wrote: >> > On Friday, February 3, 2017 6:44:16 PM EST Paul Moore wrote: >> >> I'm still trying to understand what purpose this record actually >> >> serves, and what requirements may exist. In an earlier thread >> >> somewhere Steve mentioned some broad requirements around data >> >> import/export, and I really wonder if the NETFILTER_PKT record >> >> provides anything useful here when it really isn't connecting the >> >> traffic to the sender/receiver without a lot of additional logging and >> >> post-processing smarts. If you were interested in data import/export >> >> I think auditing the socket syscalls would provide a much more useful >> >> set of records in the audit log. >> > >> > The problem here is we cannot be selective enough through the syscall >> > interface to get exactly what we want. For example, any auditing of connect >> > and accept will also get af_unix traffic which is likely to be uid/gid lookups >> > through sssd or glibc. Typically we want the IPv4/6 traffic. The netfilter rules >> > are better suited to describing which packets are of interest. >> >> Okay, but how useful are these NETFILTER_PKT records, really? The >> only linkage you have back to the process on the local machine is via >> the addr/proto/port tuple and that seems far from ideal. > > And even that could be spoofed easily and gathering more corroborating > information would seem useful. > > Would the presence of the SOCKADDR record in any SYSCALL record be > useful for somehow tagging a class of fd as being of interest? I don't think we want to create a SOCKADDR record for every syscall, but it seems reasonable that we may want to include it for targeted syscalls. Right now it looks like we create a SOCKADDR record whenever we copy a sockaddr struct across the kernel/userspace boundary, that should be sufficient, yes? -- paul moore security @ redhat ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH] audit: normalize NETFILTER_PKT 2017-02-08 4:02 ` Paul Moore @ 2017-02-08 12:32 ` Richard Guy Briggs 2017-02-08 23:11 ` Paul Moore 0 siblings, 1 reply; 14+ messages in thread From: Richard Guy Briggs @ 2017-02-08 12:32 UTC (permalink / raw) To: Paul Moore; +Cc: linux-audit, Netfilter Developer Mailing List, Thomas Graf On 2017-02-07 23:02, Paul Moore wrote: > On Tue, Feb 7, 2017 at 4:22 PM, Richard Guy Briggs <rgb@redhat.com> wrote: > > On 2017-02-06 14:41, Paul Moore wrote: > >> On Sat, Feb 4, 2017 at 8:25 AM, Steve Grubb <sgrubb@redhat.com> wrote: > >> > On Friday, February 3, 2017 6:44:16 PM EST Paul Moore wrote: > >> >> I'm still trying to understand what purpose this record actually > >> >> serves, and what requirements may exist. In an earlier thread > >> >> somewhere Steve mentioned some broad requirements around data > >> >> import/export, and I really wonder if the NETFILTER_PKT record > >> >> provides anything useful here when it really isn't connecting the > >> >> traffic to the sender/receiver without a lot of additional logging and > >> >> post-processing smarts. If you were interested in data import/export > >> >> I think auditing the socket syscalls would provide a much more useful > >> >> set of records in the audit log. > >> > > >> > The problem here is we cannot be selective enough through the syscall > >> > interface to get exactly what we want. For example, any auditing of connect > >> > and accept will also get af_unix traffic which is likely to be uid/gid lookups > >> > through sssd or glibc. Typically we want the IPv4/6 traffic. The netfilter rules > >> > are better suited to describing which packets are of interest. > >> > >> Okay, but how useful are these NETFILTER_PKT records, really? The > >> only linkage you have back to the process on the local machine is via > >> the addr/proto/port tuple and that seems far from ideal. > > > > And even that could be spoofed easily and gathering more corroborating > > information would seem useful. > > > > Would the presence of the SOCKADDR record in any SYSCALL record be > > useful for somehow tagging a class of fd as being of interest? > > I don't think we want to create a SOCKADDR record for every syscall, > but it seems reasonable that we may want to include it for targeted > syscalls. Right now it looks like we create a SOCKADDR record > whenever we copy a sockaddr struct across the kernel/userspace > boundary, that should be sufficient, yes? Yes, we certainly don't need it for every syscall. Since the sockaddr record is only created if it is available we could further flag or check the protocol to further process only the network-based sockaddrs and ignore the unix sockaddrs for this purpose. I'm picturing adding a flag to the fd, but that is making me a bit nervous about overstepping our usual code area. > paul moore - RGB -- Richard Guy Briggs <rgb@redhat.com> Kernel Security Engineering, Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH] audit: normalize NETFILTER_PKT 2017-02-08 12:32 ` Richard Guy Briggs @ 2017-02-08 23:11 ` Paul Moore 2017-02-08 23:26 ` Richard Guy Briggs 0 siblings, 1 reply; 14+ messages in thread From: Paul Moore @ 2017-02-08 23:11 UTC (permalink / raw) To: Richard Guy Briggs Cc: linux-audit, Netfilter Developer Mailing List, Thomas Graf On Wed, Feb 8, 2017 at 7:32 AM, Richard Guy Briggs <rgb@redhat.com> wrote: > On 2017-02-07 23:02, Paul Moore wrote: >> On Tue, Feb 7, 2017 at 4:22 PM, Richard Guy Briggs <rgb@redhat.com> wrote: >> > On 2017-02-06 14:41, Paul Moore wrote: >> >> On Sat, Feb 4, 2017 at 8:25 AM, Steve Grubb <sgrubb@redhat.com> wrote: >> >> > On Friday, February 3, 2017 6:44:16 PM EST Paul Moore wrote: >> >> >> I'm still trying to understand what purpose this record actually >> >> >> serves, and what requirements may exist. In an earlier thread >> >> >> somewhere Steve mentioned some broad requirements around data >> >> >> import/export, and I really wonder if the NETFILTER_PKT record >> >> >> provides anything useful here when it really isn't connecting the >> >> >> traffic to the sender/receiver without a lot of additional logging and >> >> >> post-processing smarts. If you were interested in data import/export >> >> >> I think auditing the socket syscalls would provide a much more useful >> >> >> set of records in the audit log. >> >> > >> >> > The problem here is we cannot be selective enough through the syscall >> >> > interface to get exactly what we want. For example, any auditing of connect >> >> > and accept will also get af_unix traffic which is likely to be uid/gid lookups >> >> > through sssd or glibc. Typically we want the IPv4/6 traffic. The netfilter rules >> >> > are better suited to describing which packets are of interest. >> >> >> >> Okay, but how useful are these NETFILTER_PKT records, really? The >> >> only linkage you have back to the process on the local machine is via >> >> the addr/proto/port tuple and that seems far from ideal. >> > >> > And even that could be spoofed easily and gathering more corroborating >> > information would seem useful. >> > >> > Would the presence of the SOCKADDR record in any SYSCALL record be >> > useful for somehow tagging a class of fd as being of interest? >> >> I don't think we want to create a SOCKADDR record for every syscall, >> but it seems reasonable that we may want to include it for targeted >> syscalls. Right now it looks like we create a SOCKADDR record >> whenever we copy a sockaddr struct across the kernel/userspace >> boundary, that should be sufficient, yes? > > Yes, we certainly don't need it for every syscall. Since the sockaddr > record is only created if it is available we could further flag or check > the protocol to further process only the network-based sockaddrs and > ignore the unix sockaddrs for this purpose. I'm picturing adding a flag > to the fd, but that is making me a bit nervous about overstepping our > usual code area. Let's keep it as-is, I would think there are other cases where having the address info for AF_UNIX (and others) might be helpful. -- paul moore www.paul-moore.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH] audit: normalize NETFILTER_PKT 2017-02-08 23:11 ` Paul Moore @ 2017-02-08 23:26 ` Richard Guy Briggs 0 siblings, 0 replies; 14+ messages in thread From: Richard Guy Briggs @ 2017-02-08 23:26 UTC (permalink / raw) To: Paul Moore; +Cc: linux-audit, Netfilter Developer Mailing List, Thomas Graf On 2017-02-08 18:11, Paul Moore wrote: > On Wed, Feb 8, 2017 at 7:32 AM, Richard Guy Briggs <rgb@redhat.com> wrote: > > On 2017-02-07 23:02, Paul Moore wrote: > >> On Tue, Feb 7, 2017 at 4:22 PM, Richard Guy Briggs <rgb@redhat.com> wrote: > >> > On 2017-02-06 14:41, Paul Moore wrote: > >> >> On Sat, Feb 4, 2017 at 8:25 AM, Steve Grubb <sgrubb@redhat.com> wrote: > >> >> > On Friday, February 3, 2017 6:44:16 PM EST Paul Moore wrote: > >> >> >> I'm still trying to understand what purpose this record actually > >> >> >> serves, and what requirements may exist. In an earlier thread > >> >> >> somewhere Steve mentioned some broad requirements around data > >> >> >> import/export, and I really wonder if the NETFILTER_PKT record > >> >> >> provides anything useful here when it really isn't connecting the > >> >> >> traffic to the sender/receiver without a lot of additional logging and > >> >> >> post-processing smarts. If you were interested in data import/export > >> >> >> I think auditing the socket syscalls would provide a much more useful > >> >> >> set of records in the audit log. > >> >> > > >> >> > The problem here is we cannot be selective enough through the syscall > >> >> > interface to get exactly what we want. For example, any auditing of connect > >> >> > and accept will also get af_unix traffic which is likely to be uid/gid lookups > >> >> > through sssd or glibc. Typically we want the IPv4/6 traffic. The netfilter rules > >> >> > are better suited to describing which packets are of interest. > >> >> > >> >> Okay, but how useful are these NETFILTER_PKT records, really? The > >> >> only linkage you have back to the process on the local machine is via > >> >> the addr/proto/port tuple and that seems far from ideal. > >> > > >> > And even that could be spoofed easily and gathering more corroborating > >> > information would seem useful. > >> > > >> > Would the presence of the SOCKADDR record in any SYSCALL record be > >> > useful for somehow tagging a class of fd as being of interest? > >> > >> I don't think we want to create a SOCKADDR record for every syscall, > >> but it seems reasonable that we may want to include it for targeted > >> syscalls. Right now it looks like we create a SOCKADDR record > >> whenever we copy a sockaddr struct across the kernel/userspace > >> boundary, that should be sufficient, yes? > > > > Yes, we certainly don't need it for every syscall. Since the sockaddr > > record is only created if it is available we could further flag or check > > the protocol to further process only the network-based sockaddrs and > > ignore the unix sockaddrs for this purpose. I'm picturing adding a flag > > to the fd, but that is making me a bit nervous about overstepping our > > usual code area. > > Let's keep it as-is, I would think there are other cases where having > the address info for AF_UNIX (and others) might be helpful. I wasn't suggesting removing the existing AUDIT_SOCKADDR support for AF_UNIX or other types of sockets, but rather when they are encountered by the audit subsystem flag the fd (if it isn't already identified as a network socket) as having more interesting information for network auditing. > paul moore - RGB -- Richard Guy Briggs <rgb@redhat.com> Kernel Security Engineering, Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635 ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-02-08 23:26 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-01-27 13:11 [RFC PATCH] audit: normalize NETFILTER_PKT Richard Guy Briggs 2017-01-30 14:53 ` Steve Grubb 2017-01-30 15:13 ` Richard Guy Briggs 2017-01-31 12:57 ` Richard Guy Briggs 2017-01-31 16:13 ` Steve Grubb 2017-01-31 19:44 ` Richard Guy Briggs 2017-02-03 23:44 ` Paul Moore 2017-02-04 13:25 ` Steve Grubb 2017-02-06 19:41 ` Paul Moore 2017-02-07 21:22 ` Richard Guy Briggs 2017-02-08 4:02 ` Paul Moore 2017-02-08 12:32 ` Richard Guy Briggs 2017-02-08 23:11 ` Paul Moore 2017-02-08 23:26 ` Richard Guy Briggs
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.