BUG/MAJOR: hashes: fix the signedness of the hash inputs
Wietse Venema reported in the thread below that we have a signedness
issue with our hashes implementations: due to the use of const char*
for the input key that's often text, the crc32, sdbm, djb2, and wt6
algorithms return a platform-dependent value for binary input keys
containing bytes with bit 7 set. This means that an ARM or PPC
platform will hash binary inputs differently from an x86 typically.
Worse, some algorithms are well defined in the industry (like CRC32)
and do not provide the expected result on x86, possibly causing
interoperability issues (e.g. a user-agent would fail to compare the
CRC32 of a message body against the one computed by haproxy).
Fortunately, and contrary to the first impression, the CRC32c variant
used in the PROXY protocol processing is not affected. Thus the impact
remains very limited (the vast majority of input keys are text-based,
such as user-agent headers for exmaple).
This patch addresses the issue by fixing all hash functions' prototypes
(even those not affected, for API consistency). A reg test will follow
in another patch.
The vast majority of users do not use these hashes. And among those
using them, very few will pass them on binary inputs. However, for the
rare ones doing it, this fix MAY have an impact during the upgrade. For
example if the package is upgraded on one LB then on another one, and
the CRC32 of a binary input is used as a stick table key (why?) then
these CRCs will not match between both nodes. Similarly, if
"hash-type ... crc32" is used, LB inconsistency may appear during the
transition. For this reason it is preferable to apply the patch on all
nodes using such hashes at the same time. Systems upgraded via their
distros will likely observe the least impact since they're expected to
be upgraded within a short time frame.
And it is important for distros NOT to skip this fix, in order to avoid
distributing an incompatible implementation of a hash. This is the
reason why this patch is tagged as MAJOR, eventhough it's extremely
unlikely that anyone will ever notice a change at all.
This patch must be backported to all supported branches since the
hashes were introduced in 1.5-dev20 (commit 98634f0c). Some parts
may be dropped since implemented later.
Link to Wietse's report:
https://marc.info/?l=postfix-users&m=157879464518535&w=2
(cherry picked from commit 340b07e8686ed0095291e937628d064bdcc7a3dd)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit e1275ddb8b427c88fb3cb3d8f7cd6ec576ce1e2d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/include/common/hash.h b/include/common/hash.h
index 78fd87b..c17f8c9 100644
--- a/include/common/hash.h
+++ b/include/common/hash.h
@@ -24,10 +24,10 @@
#include <inttypes.h>
-unsigned int hash_djb2(const char *key, int len);
-unsigned int hash_wt6(const char *key, int len);
-unsigned int hash_sdbm(const char *key, int len);
-unsigned int hash_crc32(const char *key, int len);
-uint32_t hash_crc32c(const char *key, int len);
+unsigned int hash_djb2(const void *input, int len);
+unsigned int hash_wt6(const void *input, int len);
+unsigned int hash_sdbm(const void *input, int len);
+unsigned int hash_crc32(const void *input, int len);
+uint32_t hash_crc32c(const void *input, int len);
#endif /* _COMMON_HASH_H_ */