* netfilter-queue: Incorrect UDP checksum computation in nfq_udp_compute_checksum_ipv4
@ 2016-04-01 8:48 Mathias Koehrer
2016-04-01 10:44 ` Pablo Neira Ayuso
0 siblings, 1 reply; 4+ messages in thread
From: Mathias Koehrer @ 2016-04-01 8:48 UTC (permalink / raw)
To: netfilter
Hi all,
the function nfq_udp_compute_checksum_ipv4 (src/extra/udp.c) does not
compute the correct UDP checksum.
The issue is caused by the called function checksum_tcpudp_ipv4()
(src/extra/checksum.c) that uses the hard coded protocol id IPPROTO_TCP
which is fine for TCP but fails for UDP.
A possible solution might be to pass the protocol id (IPPROTO_TCP /
IPPROTOC_UDP) as parameter to the function checksum_tcpudp_ipv4().
The very same is also true for the IPv6 versions of these functions.
Any feedback is welcome.
Best regards
Mathias
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: netfilter-queue: Incorrect UDP checksum computation in nfq_udp_compute_checksum_ipv4
2016-04-01 8:48 netfilter-queue: Incorrect UDP checksum computation in nfq_udp_compute_checksum_ipv4 Mathias Koehrer
@ 2016-04-01 10:44 ` Pablo Neira Ayuso
2016-04-01 11:34 ` Mathias Koehrer
0 siblings, 1 reply; 4+ messages in thread
From: Pablo Neira Ayuso @ 2016-04-01 10:44 UTC (permalink / raw)
To: Mathias Koehrer; +Cc: netfilter
On Fri, Apr 01, 2016 at 10:48:02AM +0200, Mathias Koehrer wrote:
> Hi all,
>
> the function nfq_udp_compute_checksum_ipv4 (src/extra/udp.c) does not
> compute the correct UDP checksum.
> The issue is caused by the called function checksum_tcpudp_ipv4()
> (src/extra/checksum.c) that uses the hard coded protocol id IPPROTO_TCP
> which is fine for TCP but fails for UDP.
> A possible solution might be to pass the protocol id (IPPROTO_TCP /
> IPPROTOC_UDP) as parameter to the function checksum_tcpudp_ipv4().
>
> The very same is also true for the IPv6 versions of these functions.
>
> Any feedback is welcome.
Would you send us a patch to fix this? Thanks!
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: netfilter-queue: Incorrect UDP checksum computation in nfq_udp_compute_checksum_ipv4
2016-04-01 10:44 ` Pablo Neira Ayuso
@ 2016-04-01 11:34 ` Mathias Koehrer
2016-04-01 11:39 ` Mathias Koehrer
0 siblings, 1 reply; 4+ messages in thread
From: Mathias Koehrer @ 2016-04-01 11:34 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter
Hi Pablo,
>> the function nfq_udp_compute_checksum_ipv4 (src/extra/udp.c) does not
>> compute the correct UDP checksum.
>> The issue is caused by the called function checksum_tcpudp_ipv4()
>> (src/extra/checksum.c) that uses the hard coded protocol id IPPROTO_TCP
>> which is fine for TCP but fails for UDP.
>> A possible solution might be to pass the protocol id (IPPROTO_TCP /
>> IPPROTOC_UDP) as parameter to the function checksum_tcpudp_ipv4().
>>
>> The very same is also true for the IPv6 versions of these functions.
>>
>> Any feedback is welcome.
>
> Would you send us a patch to fix this? Thanks!
Here it is:
Regards
Mathias
Correct the computation of the UDP checksum
Index: libnetfilter_queue-1.0.2/src/extra/checksum.c
===================================================================
--- libnetfilter_queue-1.0.2.orig/src/extra/checksum.c
+++ libnetfilter_queue-1.0.2/src/extra/checksum.c
@@ -35,7 +35,7 @@ uint16_t checksum(uint32_t sum, uint16_t
return (uint16_t)(~sum);
}
-uint16_t checksum_tcpudp_ipv4(struct iphdr *iph)
+uint16_t checksum_tcpudp_ipv4(struct iphdr *iph, uint16_t protocol_id)
{
uint32_t sum = 0;
uint32_t iph_len = iph->ihl*4;
@@ -46,13 +46,13 @@ uint16_t checksum_tcpudp_ipv4(struct iph
sum += (iph->saddr) & 0xFFFF;
sum += (iph->daddr >> 16) & 0xFFFF;
sum += (iph->daddr) & 0xFFFF;
- sum += htons(IPPROTO_TCP);
+ sum += htons(protocol_id);
sum += htons(len);
return checksum(sum, (uint16_t *)payload, len);
}
-uint16_t checksum_tcpudp_ipv6(struct ip6_hdr *ip6h, void *transport_hdr)
+uint16_t checksum_tcpudp_ipv6(struct ip6_hdr *ip6h, void *transport_hdr, uint16_t protocol_id)
{
uint32_t sum = 0;
uint32_t hdr_len = (uint32_t *)transport_hdr - (uint32_t *)ip6h;
@@ -68,7 +68,7 @@ uint16_t checksum_tcpudp_ipv6(struct ip6
sum += (ip6h->ip6_dst.s6_addr16[i] >> 16) & 0xFFFF;
sum += (ip6h->ip6_dst.s6_addr16[i]) & 0xFFFF;
}
- sum += htons(IPPROTO_TCP);
+ sum += htons(protocol_id);
sum += htons(ip6h->ip6_plen);
return checksum(sum, (uint16_t *)payload, len);
Index: libnetfilter_queue-1.0.2/src/extra/tcp.c
===================================================================
--- libnetfilter_queue-1.0.2.orig/src/extra/tcp.c
+++ libnetfilter_queue-1.0.2/src/extra/tcp.c
@@ -91,7 +91,7 @@ nfq_tcp_compute_checksum_ipv4(struct tcp
{
/* checksum field in header needs to be zero for calculation. */
tcph->check = 0;
- tcph->check = checksum_tcpudp_ipv4(iph);
+ tcph->check = checksum_tcpudp_ipv4(iph, IPPROTO_TCP);
}
EXPORT_SYMBOL(nfq_tcp_compute_checksum_ipv4);
@@ -105,7 +105,7 @@ nfq_tcp_compute_checksum_ipv6(struct tcp
{
/* checksum field in header needs to be zero for calculation. */
tcph->check = 0;
- tcph->check = checksum_tcpudp_ipv6(ip6h, tcph);
+ tcph->check = checksum_tcpudp_ipv6(ip6h, tcph, IPPROTO_TCP);
}
EXPORT_SYMBOL(nfq_tcp_compute_checksum_ipv6);
Index: libnetfilter_queue-1.0.2/src/extra/udp.c
===================================================================
--- libnetfilter_queue-1.0.2.orig/src/extra/udp.c
+++ libnetfilter_queue-1.0.2/src/extra/udp.c
@@ -91,7 +91,7 @@ nfq_udp_compute_checksum_ipv4(struct udp
{
/* checksum field in header needs to be zero for calculation. */
udph->check = 0;
- udph->check = checksum_tcpudp_ipv4(iph);
+ udph->check = checksum_tcpudp_ipv4(iph, IPPROTO_UDP);
}
EXPORT_SYMBOL(nfq_udp_compute_checksum_ipv4);
@@ -110,7 +110,7 @@ nfq_udp_compute_checksum_ipv6(struct udp
{
/* checksum field in header needs to be zero for calculation. */
udph->check = 0;
- udph->check = checksum_tcpudp_ipv6(ip6h, udph);
+ udph->check = checksum_tcpudp_ipv6(ip6h, udph, IPPROTO_UDP);
}
EXPORT_SYMBOL(nfq_udp_compute_checksum_ipv6);
Index: libnetfilter_queue-1.0.2/src/internal.h
===================================================================
--- libnetfilter_queue-1.0.2.orig/src/internal.h
+++ libnetfilter_queue-1.0.2/src/internal.h
@@ -13,8 +13,8 @@ struct iphdr;
struct ip6_hdr;
uint16_t checksum(uint32_t sum, uint16_t *buf, int size);
-uint16_t checksum_tcpudp_ipv4(struct iphdr *iph);
-uint16_t checksum_tcpudp_ipv6(struct ip6_hdr *ip6h, void *transport_hdr);
+uint16_t checksum_tcpudp_ipv4(struct iphdr *iph, uint16_t protocol_id);
+uint16_t checksum_tcpudp_ipv6(struct ip6_hdr *ip6h, void *transport_hdr, uint16_t protocol_id);
struct pkt_buff {
uint8_t *mac_header;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: netfilter-queue: Incorrect UDP checksum computation in nfq_udp_compute_checksum_ipv4
2016-04-01 11:34 ` Mathias Koehrer
@ 2016-04-01 11:39 ` Mathias Koehrer
0 siblings, 0 replies; 4+ messages in thread
From: Mathias Koehrer @ 2016-04-01 11:39 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter
[-- Attachment #1: Type: text/plain, Size: 750 bytes --]
Hi Pablo,
>>> the function nfq_udp_compute_checksum_ipv4 (src/extra/udp.c) does not
>>> compute the correct UDP checksum.
>>> The issue is caused by the called function checksum_tcpudp_ipv4()
>>> (src/extra/checksum.c) that uses the hard coded protocol id IPPROTO_TCP
>>> which is fine for TCP but fails for UDP.
>>> A possible solution might be to pass the protocol id (IPPROTO_TCP /
>>> IPPROTOC_UDP) as parameter to the function checksum_tcpudp_ipv4().
>>>
>>> The very same is also true for the IPv6 versions of these functions.
>>>
>>> Any feedback is welcome.
>>
>> Would you send us a patch to fix this? Thanks!
> Here it is:
It looks as if the line-structure of the patch is broken.
I have attached the patch as file to this mail.
Mathias
[-- Attachment #2: fix-udp-checksum.patch --]
[-- Type: text/x-patch, Size: 3715 bytes --]
Correct the computation of the UDP checksum
Index: libnetfilter_queue-1.0.2/src/extra/checksum.c
===================================================================
--- libnetfilter_queue-1.0.2.orig/src/extra/checksum.c
+++ libnetfilter_queue-1.0.2/src/extra/checksum.c
@@ -35,7 +35,7 @@ uint16_t checksum(uint32_t sum, uint16_t
return (uint16_t)(~sum);
}
-uint16_t checksum_tcpudp_ipv4(struct iphdr *iph)
+uint16_t checksum_tcpudp_ipv4(struct iphdr *iph, uint16_t protocol_id)
{
uint32_t sum = 0;
uint32_t iph_len = iph->ihl*4;
@@ -46,13 +46,13 @@ uint16_t checksum_tcpudp_ipv4(struct iph
sum += (iph->saddr) & 0xFFFF;
sum += (iph->daddr >> 16) & 0xFFFF;
sum += (iph->daddr) & 0xFFFF;
- sum += htons(IPPROTO_TCP);
+ sum += htons(protocol_id);
sum += htons(len);
return checksum(sum, (uint16_t *)payload, len);
}
-uint16_t checksum_tcpudp_ipv6(struct ip6_hdr *ip6h, void *transport_hdr)
+uint16_t checksum_tcpudp_ipv6(struct ip6_hdr *ip6h, void *transport_hdr, uint16_t protocol_id)
{
uint32_t sum = 0;
uint32_t hdr_len = (uint32_t *)transport_hdr - (uint32_t *)ip6h;
@@ -68,7 +68,7 @@ uint16_t checksum_tcpudp_ipv6(struct ip6
sum += (ip6h->ip6_dst.s6_addr16[i] >> 16) & 0xFFFF;
sum += (ip6h->ip6_dst.s6_addr16[i]) & 0xFFFF;
}
- sum += htons(IPPROTO_TCP);
+ sum += htons(protocol_id);
sum += htons(ip6h->ip6_plen);
return checksum(sum, (uint16_t *)payload, len);
Index: libnetfilter_queue-1.0.2/src/extra/tcp.c
===================================================================
--- libnetfilter_queue-1.0.2.orig/src/extra/tcp.c
+++ libnetfilter_queue-1.0.2/src/extra/tcp.c
@@ -91,7 +91,7 @@ nfq_tcp_compute_checksum_ipv4(struct tcp
{
/* checksum field in header needs to be zero for calculation. */
tcph->check = 0;
- tcph->check = checksum_tcpudp_ipv4(iph);
+ tcph->check = checksum_tcpudp_ipv4(iph, IPPROTO_TCP);
}
EXPORT_SYMBOL(nfq_tcp_compute_checksum_ipv4);
@@ -105,7 +105,7 @@ nfq_tcp_compute_checksum_ipv6(struct tcp
{
/* checksum field in header needs to be zero for calculation. */
tcph->check = 0;
- tcph->check = checksum_tcpudp_ipv6(ip6h, tcph);
+ tcph->check = checksum_tcpudp_ipv6(ip6h, tcph, IPPROTO_TCP);
}
EXPORT_SYMBOL(nfq_tcp_compute_checksum_ipv6);
Index: libnetfilter_queue-1.0.2/src/extra/udp.c
===================================================================
--- libnetfilter_queue-1.0.2.orig/src/extra/udp.c
+++ libnetfilter_queue-1.0.2/src/extra/udp.c
@@ -91,7 +91,7 @@ nfq_udp_compute_checksum_ipv4(struct udp
{
/* checksum field in header needs to be zero for calculation. */
udph->check = 0;
- udph->check = checksum_tcpudp_ipv4(iph);
+ udph->check = checksum_tcpudp_ipv4(iph, IPPROTO_UDP);
}
EXPORT_SYMBOL(nfq_udp_compute_checksum_ipv4);
@@ -110,7 +110,7 @@ nfq_udp_compute_checksum_ipv6(struct udp
{
/* checksum field in header needs to be zero for calculation. */
udph->check = 0;
- udph->check = checksum_tcpudp_ipv6(ip6h, udph);
+ udph->check = checksum_tcpudp_ipv6(ip6h, udph, IPPROTO_UDP);
}
EXPORT_SYMBOL(nfq_udp_compute_checksum_ipv6);
Index: libnetfilter_queue-1.0.2/src/internal.h
===================================================================
--- libnetfilter_queue-1.0.2.orig/src/internal.h
+++ libnetfilter_queue-1.0.2/src/internal.h
@@ -13,8 +13,8 @@ struct iphdr;
struct ip6_hdr;
uint16_t checksum(uint32_t sum, uint16_t *buf, int size);
-uint16_t checksum_tcpudp_ipv4(struct iphdr *iph);
-uint16_t checksum_tcpudp_ipv6(struct ip6_hdr *ip6h, void *transport_hdr);
+uint16_t checksum_tcpudp_ipv4(struct iphdr *iph, uint16_t protocol_id);
+uint16_t checksum_tcpudp_ipv6(struct ip6_hdr *ip6h, void *transport_hdr, uint16_t protocol_id);
struct pkt_buff {
uint8_t *mac_header;
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-04-01 11:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-01 8:48 netfilter-queue: Incorrect UDP checksum computation in nfq_udp_compute_checksum_ipv4 Mathias Koehrer
2016-04-01 10:44 ` Pablo Neira Ayuso
2016-04-01 11:34 ` Mathias Koehrer
2016-04-01 11:39 ` Mathias Koehrer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox