lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 10 Oct 2016 18:33:14 +0000 (UTC)
From:   Chris Caputo <ccaputo@....net>
To:     Liping Zhang <zlpnobody@...il.com>
cc:     Vishwanath Pai <vpai@...mai.com>,
        Pablo Neira Ayuso <pablo@...filter.org>,
        Justin Piszcz <jpiszcz@...idpixels.com>,
        linux-kernel@...r.kernel.org,
        Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: kernel v4.8: iptables logs are truncated with the 4.8 kernel?

On Mon, 10 Oct 2016, Liping Zhang wrote:
> 2016-10-10 15:02 GMT+08:00 Chris Caputo <ccaputo@....net>:
> >   Program received signal SIGSEGV, Segmentation fault.
> >   0x00007ffff65fd18a in _interp_iphdr (pi=0x617f50, len=0) at ulogd_raw2packet_BASE.c:720
> >
> >   715     static int _interp_iphdr(struct ulogd_pluginstance *pi, uint32_t len)
> >   716     {
> >   717             struct ulogd_key *ret = pi->output.keys;
> >   718             struct iphdr *iph =
> >   719                     ikey_get_ptr(&pi->input.keys[INKEY_RAW_PCKT]);
> >   720             void *nexthdr = (uint32_t *)iph + iph->ihl;
> >
> > I believe 7643507fe8b5bd8ab7522f6a81058cc1209d2585 changed previous
> > behavior by not always copying IP header data to user space.
> >
> > On my machine IPv4 log packets result in a ulogd segfault while IPv6
> > packets do not.  I'm not sure of the cause of the difference.
> >
> > The corresponding userspace commit for the 209d2585 kernel change is:
> >
> >   https://git.netfilter.org/iptables/commit/?id=7070b1f3c88a0c3d4e315c00cca61f05b0fbc882
> >
> > This adds --nflog-size to iptables.  When --nflog-size is used with my
> > iptables NFLOG lines, the ulogd-2.0.5 segfaults cease.
> 
> What numbers did you specify after --nflog-size option?
> --nflog-size 0 or ...? If you want log the whole packet to
> the ulogd, please do not specify this nflog-size option.

Not specifying nflog-size does not appear to log the whole packet...

If "--nflog-size" is unspecified, and the iptables config is left 
unchanged when the kernel is upgraded to 4.8, ulogd-2.0.5 crashes.

If "--nflog-size 0" is used, ulogd-2.0.5 crashes.

If "--nflog-size" is used with size 1 or greater, ulogd-2.0.5 is fine.

> > I'm surprised to see a kernel change cause unexpected userspace segfaults,
> > so further investigation into a kernel fix would seem a good idea.
> 
> According to the original user's manual, nflog-range option was
> designed to be the number of bytes copied to userspace, but
> unfortunately there's a bug from the beginning and it never works,
> i.e. in kernel, it just ignored this option.
> 
> Try to change the current nflog-range option's semantics may
> cause unexpected results(maybe like this ulogd crash) ...
> 
> In order to keep compatibility, Vishwanath introduce a new
> nflog-size option and keep nflog-range unchanged. If you just
> upgrade the kernel, and do not change iptables rules, this
> problem will not happen.

I am reporting that the problem does happen simply with an upgrade to 
kernel 4.8 and no other changes.  When "--nflog-size" is unspecified or 
set to 0, the bug in ulogd-2.0.5 gets triggered.

I agree there is a bug in ulogd-2.0.5 that this kernel change exposed, but 
I am trying to explain that all ulogd users risk this segfault if they 
upgrade to kernel 4.8 and don't either update to a fixed ulogd (possibly 
using your patch below) or an unreleased iptables with iptables config 
changes to implement nflog-size on each NFLOG target.

> So I think this is ulogd's bug, in _interp_iphdr, it try to
> dereference the iphdr pointer before validation check, meanwhile
> this problem does not exist in ipv6 path.  Can you try this patch:
> 
> diff --git a/filter/raw2packet/ulogd_raw2packet_BASE.c
> b/filter/raw2packet/ulogd_raw2packet_BASE.c
> index 8a6180c..fd2665a 100644
> --- a/filter/raw2packet/ulogd_raw2packet_BASE.c
> +++ b/filter/raw2packet/ulogd_raw2packet_BASE.c
> @@ -717,7 +717,7 @@ static int _interp_iphdr(struct ulogd_pluginstance
> *pi, uint32_t len)
>         struct ulogd_key *ret = pi->output.keys;
>         struct iphdr *iph =
>                 ikey_get_ptr(&pi->input.keys[INKEY_RAW_PCKT]);
> -       void *nexthdr = (uint32_t *)iph + iph->ihl;
> +       void *nexthdr;
> 
>         if (len < sizeof(struct iphdr) || len <= (uint32_t)(iph->ihl * 4))
>                 return ULOGD_IRET_OK;
> @@ -734,6 +734,7 @@ static int _interp_iphdr(struct ulogd_pluginstance
> *pi, uint32_t len)
>         okey_set_u16(&ret[KEY_IP_ID], ntohs(iph->id));
>         okey_set_u16(&ret[KEY_IP_FRAGOFF], ntohs(iph->frag_off));
> 
> +       nexthdr = (uint32_t *)iph + iph->ihl;
>         switch (iph->protocol) {
>         case IPPROTO_TCP:
>                 _interp_tcp(pi, nexthdr, len);

I agree this will likely fix ulogd, but this misses the point about the 
new kernel defaulting to a zero size return when it used to return the 
packet.

Thanks,
Chris

Powered by blists - more mailing lists