On 2017-02-23 18:06, Florian Westphal wrote: > Richard Guy Briggs <[email protected]> wrote: > > On 2017-02-23 11:57, Paul Moore wrote: > > > On Thu, Feb 23, 2017 at 10:51 AM, Richard Guy Briggs <[email protected]> > > > wrote: > > > > On 2017-02-23 06:20, Florian Westphal wrote: > > > >> Richard Guy Briggs <[email protected]> wrote: > > > >> > Simplify and eliminate flipping in and out of message fields, > > > >> > relying on nfmark > > > >> > the way we do for audit_key. > > > >> > > > > >> > +struct nfpkt_par { > > > >> > + int ipv; > > > >> > + const void *saddr; > > > >> > + const void *daddr; > > > >> > + u8 proto; > > > >> > +}; > > > >> > > > >> This is problematic, see below for why. > > > >> > > > >> > -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"); > > > >> > + if (!ih) > > > >> > return; > > > >> > > > >> Removing this "truncated" has the consequence that this can later log > > > >> "saddr=0.0.0.0 daddr=0.0.0.0" if we return here. > > > >> > > > >> This cannot happen for ip(6)tables because ip stack discards broken l3 > > > >> headers > > > >> before the netfilter hooks get called, but its possible with > > > >> NFPROTO_BRIDGE. > > > >> > > > >> Perhaps you will need to change audit_ip4/6 to return "false" when it > > > >> can't > > > >> get the l3 information now so we only log zero addresses when the > > > >> packet > > > >> really did contain them. > > > > > > > > Ok, to clarify the implications, are you saying that handing a NULL > > > > pointer to "saddr=%pI4" will print "0.0.0.0" rather than "(none)" or "?" > > No, if you pass pointers that would indeed log NULL. > > > > My initial reaction is that if the packet is so badly > > > truncated/malformed that we don't have a full IP header than we should > > > just refrain from logging the packet; it's too malformed/garbage to > > > offer any useful information and the normal packet processing should > > > result in the packet being discarded anyway. > > True for ip/ipv6, not sure about bridge though. > > > Which is why I wanted the ethertype, but that can be coded into the nfmark. > > Not following, sorry, are you saying users can/should use -j MARK > somehow?
Part of the discussed design and rationale for stripping many of the vanishing fields is that when setting up netfilter rules to invoke the AUDIT target, an accompanying nf mark should be used to indicate which rule caught that packet, since the chain name and rule number aren't available to the audit target. We would use the nf mark similarly to the way we use a rule key in the audit rules (see man auditctl). - RGB -- Richard Guy Briggs <[email protected]> Kernel Security Engineering, Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
