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

["David Stevens" <dlstevens@xxxxxxxxxx>]

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

I didn't try TAHI tests on it; my testing was with "ping6 -s ..." for ICMP
and a UDP echo program I wrote to cause fragmentation. Those were failing
before, and passed after the patch.

>> 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.

      I thought that's why you might have removed it, and I didn't get
the details from the code, but they are not set without the lines. I used
printk() to print the buffer, and it fails for all output digests without
these lines. I assume "authhdr" isn't pointing to the first skb's header
area at the point this is done, but I didn't track it down. I just know
without those lines, the seq_no and spi are not set in "pseudo_packet" and
the digest is incorrect.

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

      Yes, I believe so. The code only copies the header portion, not the
entire packet, and it only applies to fragments, of course, so I don't
expect
a large performance hit by removing the header from non-IPsec packets, too,
and that's faster and cleaner than trying to distinguish in the reassembly
code.
So the fix removes all fragment headers (even those not using AH) when
IPSEC6 is
configured.

                                                +-DLS