Possible BUG in ipv6_find_hdr function for fragmented packets

Possible BUG in ipv6_find_hdr function for fragmented packets

[Rahul Sharma]

Hello netdev,

I have observed a problem when I added an AH header before protocol
header (OSPFv3) while implementing authentication support for OSPFv3.

Problem: Fragmented packets which include authentication header don't
get reassembled in the kernel. This was because ipv6_find_hdr returns
ENOENT for the non-first fragment since AH is an extension header.

Firstly, this comment  "Note that non-1st fragment is special case
that "the protocol number of last header" is "next header" field in
Fragment header" ('last header' doesn't include AH or other extension
headers) before ipv6_find_hdr looks incorrect as per the description
of the fragmentation process in RFC2460. The rfc clearly states that
next header value in the fragments will be the first header of the
Fragmentable part of the original packet which could be AH (51) as in
our case.

This code looks like a problem:
if (_frag_off) {
253                                 if (target < 0 &&
254                                     ((!ipv6_ext_hdr(hp->nexthdr)) ||
255                                      hp->nexthdr == NEXTHDR_NONE)) {
256                                         if (fragoff)
257                                                 *fragoff = _frag_off;
258                                         return hp->nexthdr;
259                                 }
260                                 return -ENOENT;
261                         }

For non-first fragments, the 'next header' in the fragment header
would *always* be AUTH (or whatever extension header is the first
header in first fragment). But the above code will keep on returning
ENOENT for the non-first fragment in such cases.

Solution: I suggest we should get away with this check
((!ipv6_ext_hdr(hp->nexthdr)) ||hp->nexthdr == NEXTHDR_NONE))  and
simply return hp->nexthdr if the _frag_off is non zero. I tested it on
my machine and it works. Adding an special case for NEXTHDR_AUTH also
works for me.

Thanks,
Rahul

Re: Possible BUG in ipv6_find_hdr function for fragmented packets

[Hannes Frederic Sowa]

Hi Rahul,

On Mi, 2014-12-31 at 12:33 +0530, Rahul Sharma wrote:
> I have observed a problem when I added an AH header before protocol
> header (OSPFv3) while implementing authentication support for OSPFv3.
> 
> Problem: Fragmented packets which include authentication header don't
> get reassembled in the kernel. This was because ipv6_find_hdr returns
> ENOENT for the non-first fragment since AH is an extension header.
> 
> Firstly, this comment  "Note that non-1st fragment is special case
> that "the protocol number of last header" is "next header" field in
> Fragment header" ('last header' doesn't include AH or other extension
> headers) before ipv6_find_hdr looks incorrect as per the description
> of the fragmentation process in RFC2460. The rfc clearly states that
> next header value in the fragments will be the first header of the
> Fragmentable part of the original packet which could be AH (51) as in
> our case.
> 
> This code looks like a problem:
> if (_frag_off) {
> 253                                 if (target < 0 &&
> 254                                     ((!ipv6_ext_hdr(hp->nexthdr)) ||
> 255                                      hp->nexthdr == NEXTHDR_NONE)) {
> 256                                         if (fragoff)
> 257                                                 *fragoff = _frag_off;
> 258                                         return hp->nexthdr;
> 259                                 }
> 260                                 return -ENOENT;
> 261                         }
> 
> For non-first fragments, the 'next header' in the fragment header
> would *always* be AUTH (or whatever extension header is the first
> header in first fragment). But the above code will keep on returning
> ENOENT for the non-first fragment in such cases.
> 
> Solution: I suggest we should get away with this check
> ((!ipv6_ext_hdr(hp->nexthdr)) ||hp->nexthdr == NEXTHDR_NONE))  and
> simply return hp->nexthdr if the _frag_off is non zero. I tested it on
> my machine and it works. Adding an special case for NEXTHDR_AUTH also
> works for me.

The packets do get dropped in netfilter code? Do you have any idea were
specifically?

Your suggestion seems correct to me, can you provide a patch to fix
this?

Thanks,
Hannes

Re: Possible BUG in ipv6_find_hdr function for fragmented packets

[Rahul Sharma]

Hi Hannes

On Tue, Jan 6, 2015 at 7:01 PM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> Hi Rahul,
>
> On Mi, 2014-12-31 at 12:33 +0530, Rahul Sharma wrote:
>> I have observed a problem when I added an AH header before protocol
>> header (OSPFv3) while implementing authentication support for OSPFv3.
>>
>> Problem: Fragmented packets which include authentication header don't
>> get reassembled in the kernel. This was because ipv6_find_hdr returns
>> ENOENT for the non-first fragment since AH is an extension header.
>>
>> Firstly, this comment  "Note that non-1st fragment is special case
>> that "the protocol number of last header" is "next header" field in
>> Fragment header" ('last header' doesn't include AH or other extension
>> headers) before ipv6_find_hdr looks incorrect as per the description
>> of the fragmentation process in RFC2460. The rfc clearly states that
>> next header value in the fragments will be the first header of the
>> Fragmentable part of the original packet which could be AH (51) as in
>> our case.
>>
>> This code looks like a problem:
>> if (_frag_off) {
>> 253                                 if (target < 0 &&
>> 254                                     ((!ipv6_ext_hdr(hp->nexthdr)) ||
>> 255                                      hp->nexthdr == NEXTHDR_NONE)) {
>> 256                                         if (fragoff)
>> 257                                                 *fragoff = _frag_off;
>> 258                                         return hp->nexthdr;
>> 259                                 }
>> 260                                 return -ENOENT;
>> 261                         }
>>
>> For non-first fragments, the 'next header' in the fragment header
>> would *always* be AUTH (or whatever extension header is the first
>> header in first fragment). But the above code will keep on returning
>> ENOENT for the non-first fragment in such cases.
>>
>> Solution: I suggest we should get away with this check
>> ((!ipv6_ext_hdr(hp->nexthdr)) ||hp->nexthdr == NEXTHDR_NONE))  and
>> simply return hp->nexthdr if the _frag_off is non zero. I tested it on
>> my machine and it works. Adding an special case for NEXTHDR_AUTH also
>> works for me.
>
> The packets do get dropped in netfilter code? Do you have any idea were
> specifically?
>
> Your suggestion seems correct to me, can you provide a patch to fix
> this?
>
> Thanks,
> Hannes
>
>

Yes, the packets get dropped in the netfilter code. ip6table_raw_hook
was returning NF_DROP for the second fragment.
This was because of xt_action_param structure's hotdrop flag being set
to true for this fragment when ip6t_do_table tries to call
ip6_packet_match which in turn calls ipv6_find_hdr which was returning
ENOENT.

I have also emailed the patch.

Thanks

Possible BUG in ipv6_find_hdr function for fragmented packets

[Rahul Sharma]

Hello netfilter-devel,

I have observed a problem when I added an AH header before protocol
header (OSPFv3) while implementing authentication support for OSPFv3.

Problem: Fragmented packets which include authentication header don't
get reassembled in the kernel. This was because ipv6_find_hdr returns
ENOENT for the non-first fragment since AH is an extension header.

Firstly, this comment  "Note that non-1st fragment is special case
that "the protocol number of last header" is "next header" field in
Fragment header" ('last header' doesn't include AH or other extension
headers) before ipv6_find_hdr looks incorrect as per the description
of the fragmentation process in RFC2460. The rfc clearly states that
next header value in the fragments will be the first header of the
Fragmentable part of the original packet which could be AH (51) as in
our case.

This code looks like a problem:
if (_frag_off) {
253                                 if (target < 0 &&
254                                     ((!ipv6_ext_hdr(hp->nexthdr)) ||
255                                      hp->nexthdr == NEXTHDR_NONE)) {
256                                         if (fragoff)
257                                                 *fragoff = _frag_off;
258                                         return hp->nexthdr;
259                                 }
260                                 return -ENOENT;
261                         }

For non-first fragments, the 'next header' in the fragment header
would *always* be AUTH (or whatever extension header is the first
header in first fragment). But the above code will keep on returning
ENOENT for the non-first fragment in such cases.

Solution: I suggest we should get away with this check
((!ipv6_ext_hdr(hp->nexthdr)) ||hp->nexthdr == NEXTHDR_NONE))  and
simply return hp->nexthdr if the _frag_off is non zero. I tested it on
my machine and it works. Adding an special case for NEXTHDR_AUTH also
works for me.

Thanks,
Rahul