I found a portability bug in haproxy's CRC32 implementation, while
adding adding CRC32 support to Postfix's haproxy2 code. The haproxy
code produces incorrect results on ix86. Which just happens to work
as long as everyone is using ix86.
The same bug appears to exist in other hash functions in
haproxy/src/hash.c.
My proposed solution: introduce a NEW type of TLV for checksums
that are generated with a more portable CRC32 implementation.
Details
=======
Haproxy CRC32 support adds a checksum to the haproxy protocol header.
This protects against random bitflips that happen before a proxy
header is written to a sender's TCP stack, and after a proxy header
is read from a receiver's TCP stack (during transport over TCP, the
proxy header is always protected by the TCP checksum).
Here's the code from haproxy/src/hash.c:
unsigned int hash_crc32(const char *key, int len)
{
unsigned int hash;
...
hash = ~0;
while (len--) {
hash ^= *key++;
...
}
...
}
The bug is that 'key' should be 'const unsigned char *'. Otherwise,
the result depends on the hardware and/or compiler.
Below is a small test program to illustrate these differences.
* On systems where 'char' is a signed type, *key is a signed
8-bit value in the range -128..+127. In the code above, this is
converted to int -128..+127, then converted to unsigned, where
the negative integers become very large positive numbers.
Example: the 8-bit value 0xff becomes the 32-bit value 0xffffffff.
This breaks the CRC32 computation, due to non-portable programming.
See test program below.
* On systems where 'char' is an unsigned type. *key is an unsigned
8-bit value in the range 0..255. In the code above, this is
converted to int 0..255, then converned to unsigned 0..255.
Example: the 8-bit value 0xff becomes the 32-bit value 0xff. This
does not break the CRC32 computation, but that is just the result
of dumb luck. Code that is portable would always work this way.
As luck has it, with ix86 systems, 'char' is a signed type. Therefore
the CRC32 computation is screwed up (both i386 and x86_64 a.k.a. amd64).
I wrote a small test program (not included) that compares the result
from haproxy's hash_crc32() implementation and some other crc32
implementations that I found on the web, plus an implementation
that I cobbled together. All portable implementations agree on the
result. hash_crc32() only some of the time agrees with the portable
implementations.
This makes Postfix support for haproxy CRC checks pointless. It
would have to compute two versions of CRC32: the correct one and
the one that is broken, otherwise CRC32 would only work between
similar systems.
Wietse
#include <stdlib.h>
#include <stdio.h>
int main(void)
{
{
char c = 0xff;
unsigned i = c;
printf("default char 0xff -> unsigned int 0x%x\n", i);
}
{
unsigned char c = 0xff;
unsigned i = c;
printf("unsigned char 0xff -> unsigned int 0x%x\n", i);
}
{
signed char c = 0xff;
unsigned i = c;
printf("signed char 0xff -> unsigned int 0x%x\n", i);
}
exit (0);
}
