tinc generating invalid packet checksums?

[Guus Sliepen]

On Sat, Sep 12, 2015 at 02:34:33AM -0400, Nathan Stratton Treadway wrote:

> On Thu, Sep 10, 2015 at 19:34:21 -0400, Nathan Stratton Treadway wrote:
> > It seems that when the cksum is incorrect, it is always off by 1.
> > 
> [...]
> > Am I correct in concluding that this cksum problem is a bug in Tinc? 

Yes.

> After investigating this further, I'm fairly certain that problem
> originates in the following lines of the clamp_mss() function in
> route.c:
> 
>      [...]
>      csum ^= 0xffff;
>      csum -= oldmss;
>      csum += newmss;
>      csum ^= 0xffff;
>      packet->data[50] = csum >> 8;
>      packet->data[51] = csum & 0xff;
> 
> Since the TCP checksum value needs to be computed using one's compliment
> arithmetic, the above code generates new values that are off by one from
> the correct checksum for the packet in cases where the calculations
> wrap around zero in one direction but not the other....

Indeed. I was wondering why I never found this bug myself. But a quick
calculation shows that, statistically, the checksum will be invalid with
a chance of |oldmss - newmss| / 65536. For people with an MTU of 1500,
that means only 0.08% of the SYN packets will end up with a wrong
checksum. However, with an MTU of 9000 it'll be wrong 11.5% of the time.

> According to Eqn 3 given in RFC 1624 (e.g. at
>   https://tools.ietf.org/html/rfc1624#section-3
> ) and the UpdateTTL() example function given RFC 1141,
> it looks like the calculation should instead be something like
>      uint32_t csum = packet->data[50] << 8 | packet->data[51];
>      [...]
>      csum ^= 0xffff;
>      csum += oldmss ^ 0xffff;
>      csum += newmss;
>      csum = (csum & 0xFFFF) + (csum >> 16);
>      csum += (csum >> 16);
>      csum ^= 0xffff;
>      packet->data[50] = csum >> 8;
>      packet->data[51] = csum & 0xff;

Thanks for digging into this and providing a fix. I've committed it to
git!

-- 
Met vriendelijke groet / with kind regards,
     Guus Sliepen <guus at tinc-vpn.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://www.tinc-vpn.org/pipermail/tinc/attachments/20150912/935991ce/attachment.sig>