* [PATCH libnetfilter_queue] src: fix IPv6 header handling
@ 2021-01-13 9:47 Etan Kissling
2021-01-19 13:25 ` Etan Kissling
0 siblings, 1 reply; 5+ messages in thread
From: Etan Kissling @ 2021-01-13 9:47 UTC (permalink / raw)
To: netfilter-devel; +Cc: laforge
This corrects issues in IPv6 header handling that sometimes resulted in an endless loop.
Signed-off-by: Etan Kissling <etan_kissling@apple.com>
---
| 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
--git a/src/extra/ipv6.c b/src/extra/ipv6.c
index 42c5e25..1eb822f 100644
--- a/src/extra/ipv6.c
+++ b/src/extra/ipv6.c
@@ -72,7 +72,8 @@ int nfq_ip6_set_transport_header(struct pkt_buff *pktb, struct ip6_hdr *ip6h,
uint32_t hdrlen;
/* No more extensions, we're done. */
- if (nexthdr == IPPROTO_NONE) {
+ if (nexthdr == IPPROTO_TCP || nexthdr == IPPROTO_UDP || nexthdr == IPPROTO_ESP ||
+ nexthdr == IPPROTO_ICMPV6 || nexthdr == IPPROTO_NONE) {
cur = NULL;
break;
}
@@ -107,7 +108,7 @@ int nfq_ip6_set_transport_header(struct pkt_buff *pktb, struct ip6_hdr *ip6h,
} else if (nexthdr == IPPROTO_AH)
hdrlen = (ip6_ext->ip6e_len + 2) << 2;
else
- hdrlen = ip6_ext->ip6e_len;
+ hdrlen = (ip6_ext->ip6e_len + 1) << 3;
nexthdr = ip6_ext->ip6e_nxt;
cur += hdrlen;
--
2.21.1 (Apple Git-122.3)
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH libnetfilter_queue] src: fix IPv6 header handling
2021-01-13 9:47 [PATCH libnetfilter_queue] src: fix IPv6 header handling Etan Kissling
@ 2021-01-19 13:25 ` Etan Kissling
0 siblings, 0 replies; 5+ messages in thread
From: Etan Kissling @ 2021-01-19 13:25 UTC (permalink / raw)
To: netfilter-devel; +Cc: Pablo Neira Ayuso
This corrects issues in IPv6 header handling that sometimes resulted
in an endless loop.
Signed-off-by: Etan Kissling <etan_kissling@apple.com>
---
| 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
--git a/src/extra/ipv6.c b/src/extra/ipv6.c
index 42c5e25..c088240 100644
--- a/src/extra/ipv6.c
+++ b/src/extra/ipv6.c
@@ -72,7 +72,8 @@ int nfq_ip6_set_transport_header(struct pkt_buff *pktb, struct ip6_hdr *ip6h,
uint32_t hdrlen;
/* No more extensions, we're done. */
- if (nexthdr == IPPROTO_NONE) {
+ if (nexthdr == IPPROTO_TCP || nexthdr == IPPROTO_UDP || nexthdr == IPPROTO_ESP ||
+ nexthdr == IPPROTO_ICMPV6 || nexthdr == IPPROTO_NONE) {
cur = NULL;
break;
}
@@ -107,7 +108,7 @@ int nfq_ip6_set_transport_header(struct pkt_buff *pktb, struct ip6_hdr *ip6h,
} else if (nexthdr == IPPROTO_AH)
hdrlen = (ip6_ext->ip6e_len + 2) << 2;
else
- hdrlen = ip6_ext->ip6e_len;
+ hdrlen = (ip6_ext->ip6e_len + 1) << 3;
nexthdr = ip6_ext->ip6e_nxt;
cur += hdrlen;
--
2.21.1 (Apple Git-122.3)
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH libnetfilter_queue] src: fix IPv6 header handling
2021-02-09 16:30 ` Pablo Neira Ayuso
@ 2021-02-17 15:09 ` Etan Kissling
0 siblings, 0 replies; 5+ messages in thread
From: Etan Kissling @ 2021-02-17 15:09 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
On 09.02.21, 17:33, "Pablo Neira Ayuso" <pablo@netfilter.org> wrote:
Thanks for looking into this.
> Note: nfq_ip6_set_transport_header() is very much similar to
> ipv6_skip_exthdr() in the Linux kernel, see net/ipv6/exthdrs_core.c
I submitted a revised version that uses similar logic as in the kernel
to exit the loop once extension headers are fully processed.
> > uint32_t hdrlen;
> >
> > /* No more extensions, we're done. */
> > - if (nexthdr == IPPROTO_NONE) {
> > + if (nexthdr == IPPROTO_TCP || nexthdr == IPPROTO_UDP || nexthdr == IPPROTO_ESP ||
> > + nexthdr == IPPROTO_ICMPV6 || nexthdr == IPPROTO_NONE) {
> > cur = NULL;
> > break;
> > }
> > @@ -107,7 +108,7 @@ int nfq_ip6_set_transport_header(struct pkt_buff *pktb, struct ip6_hdr *ip6h,
> > } else if (nexthdr == IPPROTO_AH)
> > hdrlen = (ip6_ext->ip6e_len + 2) << 2;
> > else
> > - hdrlen = ip6_ext->ip6e_len;
> > + hdrlen = (ip6_ext->ip6e_len + 1) << 3;
>
> This looks correct, IPv6 optlen is miscalculated.
>
> The chunk above to stop the iteration, so I think the chunk that fixes
> optlen is sufficient to fix the bug.
The optlen fix is not sufficient when a non-existent 'target' is given.
For example, if a UDP packet is passed with 'target' IPPROTO_TCP,
the loop will go on and attempt to interpret the UDP packet body
as another IP extension header. Instead, by exiting the loop also
when all extension headers are processed, the function will now return
0 in that case, as documented in the header file.
Thanks
Etan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH libnetfilter_queue] src: fix IPv6 header handling
2021-01-13 9:58 Etan Kissling
@ 2021-02-09 16:30 ` Pablo Neira Ayuso
2021-02-17 15:09 ` Etan Kissling
0 siblings, 1 reply; 5+ messages in thread
From: Pablo Neira Ayuso @ 2021-02-09 16:30 UTC (permalink / raw)
To: Etan Kissling; +Cc: netfilter-devel
Hi Etan,
On Wed, Jan 13, 2021 at 10:58:52AM +0100, Etan Kissling wrote:
> diff --git a/src/extra/ipv6.c b/src/extra/ipv6.c
> index 42c5e25..1eb822f 100644
> --- a/src/extra/ipv6.c
> +++ b/src/extra/ipv6.c
> @@ -72,7 +72,8 @@ int nfq_ip6_set_transport_header(struct pkt_buff *pktb, struct ip6_hdr *ip6h,
Note: nfq_ip6_set_transport_header() is very much similar to
ipv6_skip_exthdr() in the Linux kernel, see net/ipv6/exthdrs_core.c
> uint32_t hdrlen;
>
> /* No more extensions, we're done. */
> - if (nexthdr == IPPROTO_NONE) {
> + if (nexthdr == IPPROTO_TCP || nexthdr == IPPROTO_UDP || nexthdr == IPPROTO_ESP ||
> + nexthdr == IPPROTO_ICMPV6 || nexthdr == IPPROTO_NONE) {
> cur = NULL;
> break;
> }
> @@ -107,7 +108,7 @@ int nfq_ip6_set_transport_header(struct pkt_buff *pktb, struct ip6_hdr *ip6h,
> } else if (nexthdr == IPPROTO_AH)
> hdrlen = (ip6_ext->ip6e_len + 2) << 2;
> else
> - hdrlen = ip6_ext->ip6e_len;
> + hdrlen = (ip6_ext->ip6e_len + 1) << 3;
This looks correct, IPv6 optlen is miscalculated.
The chunk above to stop the iteration, so I think the chunk that fixes
optlen is sufficient to fix the bug.
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH libnetfilter_queue] src: fix IPv6 header handling
@ 2021-01-13 9:58 Etan Kissling
2021-02-09 16:30 ` Pablo Neira Ayuso
0 siblings, 1 reply; 5+ messages in thread
From: Etan Kissling @ 2021-01-13 9:58 UTC (permalink / raw)
To: netfilter-devel
This corrects issues in IPv6 header handling that sometimes resulted
in an endless loop.
Signed-off-by: Etan Kissling <etan_kissling@apple.com>
---
| 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
--git a/src/extra/ipv6.c b/src/extra/ipv6.c
index 42c5e25..1eb822f 100644
--- a/src/extra/ipv6.c
+++ b/src/extra/ipv6.c
@@ -72,7 +72,8 @@ int nfq_ip6_set_transport_header(struct pkt_buff *pktb, struct ip6_hdr *ip6h,
uint32_t hdrlen;
/* No more extensions, we're done. */
- if (nexthdr == IPPROTO_NONE) {
+ if (nexthdr == IPPROTO_TCP || nexthdr == IPPROTO_UDP || nexthdr == IPPROTO_ESP ||
+ nexthdr == IPPROTO_ICMPV6 || nexthdr == IPPROTO_NONE) {
cur = NULL;
break;
}
@@ -107,7 +108,7 @@ int nfq_ip6_set_transport_header(struct pkt_buff *pktb, struct ip6_hdr *ip6h,
} else if (nexthdr == IPPROTO_AH)
hdrlen = (ip6_ext->ip6e_len + 2) << 2;
else
- hdrlen = ip6_ext->ip6e_len;
+ hdrlen = (ip6_ext->ip6e_len + 1) << 3;
nexthdr = ip6_ext->ip6e_nxt;
cur += hdrlen;
--
2.21.1 (Apple Git-122.3)
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-02-17 15:14 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-13 9:47 [PATCH libnetfilter_queue] src: fix IPv6 header handling Etan Kissling
2021-01-19 13:25 ` Etan Kissling
2021-01-13 9:58 Etan Kissling
2021-02-09 16:30 ` Pablo Neira Ayuso
2021-02-17 15:09 ` Etan Kissling
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.