[389-commits] [389-ds-base] 01/01: Ticket 49424 - Resolve csiphash alignment issues

This is an automated email from the git hooks/post-receive script. firstyear pushed a commit to branch 389-ds-base-1.3.7 in repository 389-ds-base. commit 92c56e903dd7560a2bf9a175a2f3783c958c2343 Author: William Brown <firstyear(a)redhat.com&gt; Date: Mon Oct 30 12:01:34 2017 +1000 Ticket 49424 - Resolve csiphash alignment issues Bug Description: On some platforms, uint64_t is not the same size as a void * - as well, if the input is not aligned correctly, then a number of nasty crashes can result Fix Description: Instead of relying on alignment to be correct, we should memcpy the data to inputs instead. https://pagure.io/389-ds-base/issue/49424 Author: wibrown Review by: lslebodn, cgrzemba, vashirov, mreynolds (Thanks!) (cherry picked from commit 751446440f5269a246e6e652a64e63aa5933734a) --- src/libsds/external/csiphash/csiphash.c | 52 +++++++++++++++++++-------------- src/libsds/test/test_sds_csiphash.c | 43 +++++++++++++++++++++------ 2 files changed, 64 insertions(+), 31 deletions(-) diff --git a/src/libsds/external/csiphash/csiphash.c b/src/libsds/external/csiphash/csiphash.c index 0089c82..2351db6 100644 --- a/src/libsds/external/csiphash/csiphash.c +++ b/src/libsds/external/csiphash/csiphash.c @@ -32,6 +32,9 @@ #include <inttypes.h> #include <stddef.h> /* for size_t */ +#include <stdlib.h> /* calloc,free */ +#include <string.h> /* memcpy */ + #include <config.h> #if defined(HAVE_SYS_ENDIAN_H) @@ -75,11 +78,24 @@ uint64_t sds_siphash13(const void *src, size_t src_sz, const char key[16]) { - const uint64_t *_key = (uint64_t *)key; + uint64_t _key[2] = {0}; + memcpy(_key, key, 16); uint64_t k0 = _le64toh(_key[0]); uint64_t k1 = _le64toh(_key[1]); uint64_t b = (uint64_t)src_sz << 56; - const uint64_t *in = (uint64_t *)src; + + size_t input_sz = (src_sz / sizeof(uint64_t)) + 1; + + /* Account for non-uint64_t alligned input */ + /* Could make this stack allocation */ + uint64_t *in = calloc(1, input_sz * sizeof(uint64_t)); + /* + * Because all crypto code sucks, they modify *in + * during operation, so we stash a copy of the ptr here. + * alternately, we could use stack allocated array, but gcc + * will complain about the vla being unbounded. + */ + uint64_t *in_ptr = memcpy(in, src, src_sz); uint64_t v0 = k0 ^ 0x736f6d6570736575ULL; uint64_t v1 = k1 ^ 0x646f72616e646f6dULL; @@ -96,27 +112,15 @@ sds_siphash13(const void *src, size_t src_sz, const char key[16]) v0 ^= mi; } + /* + * Because we allocate in as size + 1, we can over-read 0 + * for this buffer to be padded correctly. in here is a pointer to the + * excess data because the while loop above increments the in pointer + * to point to the excess once src_sz drops < 8. + */ uint64_t t = 0; - uint8_t *pt = (uint8_t *)&t; - uint8_t *m = (uint8_t *)in; - - switch (src_sz) { - case 7: - pt[6] = m[6]; /* FALLTHRU */ - case 6: - pt[5] = m[5]; /* FALLTHRU */ - case 5: - pt[4] = m[4]; /* FALLTHRU */ - case 4: - *((uint32_t *)&pt[0]) = *((uint32_t *)&m[0]); - break; - case 3: - pt[2] = m[2]; /* FALLTHRU */ - case 2: - pt[1] = m[1]; /* FALLTHRU */ - case 1: - pt[0] = m[0]; /* FALLTHRU */ - } + memcpy(&t, in, sizeof(uint64_t)); + b |= _le64toh(t); v3 ^= b; @@ -126,5 +130,9 @@ sds_siphash13(const void *src, size_t src_sz, const char key[16]) v2 ^= 0xff; // dround dROUND(v0, v1, v2, v3); + + free(in_ptr); + return (v0 ^ v1) ^ (v2 ^ v3); } + diff --git a/src/libsds/test/test_sds_csiphash.c b/src/libsds/test/test_sds_csiphash.c index cdb6b7f..cc9a6b2 100644 --- a/src/libsds/test/test_sds_csiphash.c +++ b/src/libsds/test/test_sds_csiphash.c @@ -25,23 +25,48 @@ static void test_siphash(void **state __attribute__((unused))) { - - // uint64_t value = 0; uint64_t hashout = 0; char key[16] = {0}; - uint64_t test_a = 15794382300316794652U; - uint64_t test_b = 13042610424265326907U; + uint64_t test_simple = 15794382300316794652U; - // Initial simple test + /* Initial simple test */ value = htole64(5); hashout = sds_siphash13(&value, sizeof(uint64_t), key); - assert_true(hashout == test_a); + assert_int_equal(hashout, test_simple); + + /* Test a range of input sizes to check endianness behaviour */ + + hashout = sds_siphash13("a", 1, key); + assert_int_equal(hashout, 0x407448d2b89b1813U); + + hashout = sds_siphash13("aa", 2, key); + assert_int_equal(hashout, 0x7910e0436ed8d1deU); + + hashout = sds_siphash13("aaa", 3, key); + assert_int_equal(hashout, 0xf752893a6c769652U); + + hashout = sds_siphash13("aaaa", 4, key); + assert_int_equal(hashout, 0x8b02350718d87164U); + + hashout = sds_siphash13("aaaaa", 5, key); + assert_int_equal(hashout, 0x92a991474c7eef2U); + + hashout = sds_siphash13("aaaaaa", 6, key); + assert_int_equal(hashout, 0xf0ab815a640277ccU); + + hashout = sds_siphash13("aaaaaaa", 7, key); + assert_int_equal(hashout, 0x33f3c6d7dbc82c0dU); + + hashout = sds_siphash13("aaaaaaaa", 8, key); + assert_int_equal(hashout, 0xc501b12e18428c92U); + + hashout = sds_siphash13("aaaaaaaabbbb", 12, key); + assert_int_equal(hashout, 0xcddca673069ade64U); - char *test = "abc"; - hashout = sds_siphash13(test, 4, key); - assert_true(hashout == test_b); + hashout = sds_siphash13("aaaaaaaabbbbbbbb", 16, key); + assert_int_equal(hashout, 0xdc54f0bfc0e1deb0U); } int -- To stop receiving notification emails like this one, please contact the administrator of this repository.