[PATCH libnetfilter_queue] BUG: src: Update UDP header length field after mangling

[PATCH libnetfilter_queue] BUG: src: Update UDP header length field after mangling

[Duncan Roe]

One would expect nfq_udp_mangle_ipv4() to take care of the length field in
the UDP header but it did not.
With this patch, it does.
This patch is very unlikely to adversely affect any existing userspace
software (that did its own length adjustment),
because UDP checksumming was broken
---
 src/extra/udp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/extra/udp.c b/src/extra/udp.c
index 8c44a66..6836230 100644
--- a/src/extra/udp.c
+++ b/src/extra/udp.c
@@ -140,6 +140,8 @@ nfq_udp_mangle_ipv4(struct pkt_buff *pkt,
 	iph = (struct iphdr *)pkt->network_header;
 	udph = (struct udphdr *)(pkt->network_header + iph->ihl*4);
 
+	udph->len = htons(ntohs(udph->len) + rep_len - match_len);
+
 	if (!nfq_ip_mangle(pkt, iph->ihl*4 + sizeof(struct udphdr),
 				match_offset, match_len, rep_buffer, rep_len))
 		return 0;
-- 
2.14.5

Re: [PATCH libnetfilter_queue] BUG: src: Update UDP header length field after mangling

[Duncan Roe]

Hi Pablo,

On Fri, Sep 27, 2019 at 10:56:45PM +1000, Duncan Roe wrote:
> One would expect nfq_udp_mangle_ipv4() to take care of the length field in
> the UDP header but it did not.
> With this patch, it does.
> This patch is very unlikely to adversely affect any existing userspace
> software (that did its own length adjustment),
> because UDP checksumming was broken
> ---
>  src/extra/udp.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/src/extra/udp.c b/src/extra/udp.c
> index 8c44a66..6836230 100644
> --- a/src/extra/udp.c
> +++ b/src/extra/udp.c
> @@ -140,6 +140,8 @@ nfq_udp_mangle_ipv4(struct pkt_buff *pkt,
>  	iph = (struct iphdr *)pkt->network_header;
>  	udph = (struct udphdr *)(pkt->network_header + iph->ihl*4);
>
> +	udph->len = htons(ntohs(udph->len) + rep_len - match_len);
> +
>  	if (!nfq_ip_mangle(pkt, iph->ihl*4 + sizeof(struct udphdr),
>  				match_offset, match_len, rep_buffer, rep_len))
>  		return 0;
> --
> 2.14.5
>
Please consider applying this fix. I have other patches banking up behind it.

There is no need for a corresponding TCP fix because the TCP header does not
contain a length field.

And, there is no IP4 / IP6 concern: udp.c is used by both.

(Also, git pull has stopped working for me: 80s delay then connection reset by
peer).

Cheers ... Duncan.

Re: [PATCH libnetfilter_queue] BUG: src: Update UDP header length field after mangling

[Pablo Neira Ayuso]

On Sun, Oct 06, 2019 at 10:02:26AM +1100, Duncan Roe wrote:
> Hi Pablo,
> 
> On Fri, Sep 27, 2019 at 10:56:45PM +1000, Duncan Roe wrote:
> > One would expect nfq_udp_mangle_ipv4() to take care of the length field in
> > the UDP header but it did not.
> > With this patch, it does.
> > This patch is very unlikely to adversely affect any existing userspace
> > software (that did its own length adjustment),
> > because UDP checksumming was broken
> > ---
> >  src/extra/udp.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/src/extra/udp.c b/src/extra/udp.c
> > index 8c44a66..6836230 100644
> > --- a/src/extra/udp.c
> > +++ b/src/extra/udp.c
> > @@ -140,6 +140,8 @@ nfq_udp_mangle_ipv4(struct pkt_buff *pkt,
> >  	iph = (struct iphdr *)pkt->network_header;
> >  	udph = (struct udphdr *)(pkt->network_header + iph->ihl*4);
> >
> > +	udph->len = htons(ntohs(udph->len) + rep_len - match_len);
> > +
> >  	if (!nfq_ip_mangle(pkt, iph->ihl*4 + sizeof(struct udphdr),
> >  				match_offset, match_len, rep_buffer, rep_len))
> >  		return 0;
> > --
> > 2.14.5
> >
> Please consider applying this fix. I have other patches banking up behind it.

Applied, thanks.