(usagi-users 01465) Re: 5 bugs in AH + fragmentation ( and PATCH)

[Kazunori MIYAZAWA <miyazawa@xxxxxxxxxxxxxxxxx>]

Hello, 

I tested your patch, which makes the kernel pass TAHI test number 9.
It however doesn't pass test number 4 yet.

On Thu, 16 May 2002 01:38:11 -0700
"David Stevens" <dlstevens@xxxxxxxxxx> wrote:

> I've found 5 bugs relating to AH and fragmentation. A patch for all of them
> is listed below, relative to the 5/19 usagi snapshot.
> 
> Details:
> 1) On input processing, a reassembled packet normally will not be in one
>       contiguous buffer. Fix: changed "memcpy" to use skb_copy_bits()

I have not known it. I'm looking for why reassemled data is not correct.
I appreciate your information.

> 2) A recent change removed the lines:
> 
>       pseudo_authhdr->spi = authhdr->spi;
>       pseudo_authhdr->seq_no = authhdr->seq_no;
> 
>    from ipsec6_ah_calc(). Without them, the sequence number and spi are
>    not included in the digest calculation, and all output AH digests for
>    fragmented packets are incorrect. Fix: readd the lines

In ipsec6_ah_calc, the authhdr points an address of AH on skb.
memcpy at line 281 in ipsec6_utils.c copys packet to pseudo_packet including
AH with spi and seq_no.

I guess 2) is not needed.

> 3) The raw socket "getfrag" function maintains partial checksums in a
>       private header for fragments. AH uses these to assemble the full
>       packet for AH computation, and then the getfrag function is called
>       again on packet delivery. The second set of calls was using old
>       partial checksum state, resulting in incorrect ICMP checksums ("0").
>       Fix: reset the partial checksum to 0 when the packet is complete.
> 4) The UDP getfrag() function had a similar problem to 3) - similar fix.
> 5) Ordinarly, the fragment header is not removed from a reassembled input
>       packet. This results in incorrect digests when using AH, because
>       it incorrectly includes the fragment header. Fix: remove fragment
>       headers when CONFIG_IPV6_IPSEC is set.
> 

Why does current codes leave fragment header? Is it performance matter?
What do you think about it?

>                                           +-DLS
> Patch:
> 

<snip>

Thank you,

--Kazunori Miyazawa(YOKOGAWA Electric Corporation)