* 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