Re: [PATCH nf-next 3/3] netfilter: nft_log: fix snaplen does not truncate packets

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Wed, Jul 20, 2016 at 05:00:45PM +0800, Liping Zhang wrote:
> Hi Pablo,
> 
> 2016-07-20 16:25 GMT+08:00 Pablo Neira Ayuso <pablo@netfilter.org>:
> > On Wed, Jul 20, 2016 at 07:00:13AM +0800, Liping Zhang wrote:
> >> I find that nftables already support this feature, the following command mean to truncate packets
> >> to 100 bytes before logging to the userspace:
> >>   #nft add rule filter input log group 0 snaplen 100
> >>
> >> Before my patch, it does not work.
> >> And after apply my patch, it works as expected.
> >
> > If I git grep NF_LOG_F_COPY_LEN from the nftables.git tree, I don't
> > see any reference to this flag being set.
> >
> 
> I use this NF_LOG_F_COPY_LEN flag as internal, and when the user specify
> the NFTA_LOG_SNAPLEN attrribute, it will be enabled automatically.
> See my codes:
> 
>                if (tb[NFTA_LOG_SNAPLEN] != NULL) {
> +                       li->u.ulog.flags |= NF_LOG_F_COPY_LEN;
> 
> -               if (li->u.ulog.copy_len) {
> +               if (li->u.ulog.flags & NF_LOG_F_COPY_LEN) {
>                          if (nla_put_be32(skb, NFTA_LOG_SNAPLEN,
>                                          htonl(li->u.ulog.copy_len)))
> 
> So this flag will not be setted from userspace explicitly and will not
> be dumped to the userspace.

That's fine the way it is then. Thanks for clarifying this.

> > Then, nft_log kernel has been actually working fine since the
> > beginning. It would be good anyway if we set this flag on from
> > userspace to leave things in consistent state.
> 
> Do you mean this is something similar to "nflog-size" and
> "nflog-range" in xt_NFLOG?
> "nft log snaplen 100" cannot work since the beginning, so we should
> keep it unchanged? And
> maybe we should introduce a new option like "nft log snapsize 100"?

In the particular case of nftables, given we're still below the 1.0
stage, we can just classify this as a bugfix, I wouldn't bother much
on this.

The only thing to care is to provide the right translation of
nflog-size to snaplen and you actually contributed this patch already,
so everything is fine :).