On Sat, 2018-02-17 at 23:55 +0100, Niels Möller wrote:
nisse@lysator.liu.se (Niels Möller) writes:
Daiki Ueno ueno@gnu.org writes:
I have incorporated the suggested changes here: https://gitlab.com/dueno/nettle/commits/wip/dueno/rsa-padding
Thanks!
I've added these changes on a branch merge-pss in the main repo, together with some smaller post-merge cleanups.
Some late comments.
In testsuite/Makefile.in, pss-mgf1-test.c is listed in TS_NETTLE_SOURCES. Should be moved to TS_HOGWEED_SOURCES, to not get link failured in builds without hogweed. Right?
Alternatively, we could move pss-mgf1.c from libhogweed to libnettle, but that doesn't seem very useful to me.
Both pss_mgf1 and pss_encode_mgf1 allocate the hash context using
TMP_DECL(state, uint8_t, NETTLE_MAX_HASH_CONTEXT_SIZE); ... TMP_ALLOC(state, hash->context_size);
That should work fine if we are using alloca, which provides suitable alignment, but if !HAVE_ALLOCA, we're only guaranteed uint8_t alignment.
What if TMP_ALLOC() allocates 16 bytes more and makes sure it returns a 16-byte aligned buffer? That is, return something passed from:
#define ALIGN16(x) \ ((void *)(((ptrdiff_t)(x)+(ptrdiff_t)0x0f)&~((ptrdiff_t)0x0f)))
(the 16-byte alignment is because that's the worse case alignment needed, e.g., for AESNI keys etc).
Alternatively, can we drop support for compilers lacking alloca, or substitute malloc rather than fixed size stack allocation? In the latter case, we'd need to augment TMP_* facilities with TMP_FREE to deallocate properly in that case (like we already do for TMP_GMP_ALLOC).
What about switching to C99 buffers like: uint8_t buffer[buffer_size]; and fallback to alloca otherwise?
That would still require additional allocation for alignment handling, but it would be standard C. The current macros would also be simplified by such a move.
regards, Nikos