On Sat, 2018-02-17 at 23:55 +0100, Niels Möller wrote:
> ni...@lysator.liu.se (Niels Möller) writes:
> > Daiki Ueno <u...@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_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.


nettle-bugs mailing list

Reply via email to