nisse@lysator.liu.se (Niels Möller) writes:
Simon Josefsson simon@josefsson.org writes:
Did you notice that testsuite/meta-hash-test.c was modified as well to make sure the magic number is OK?
Yes, that's good, but I'd still prefer to have it defined in terms of sizeof.
A sizeof or sizeof-union could work too, but then nettle-internal.h would need more #include's.
Since it's for internal use only, I don't think that's a problem. And including sha.h should be sufficient I guess? (I still haven't checked, but I would think sha512_ctx is the largest one).
The sizes are:
md2: 84 md4: 92 md5: 92 ripemd160: 96 sha1: 96 sha224: 108 sha256: 108 sha384: 216 sha512: 216
Do you want me to use this approach in any updated patch?
- TMP_DECL (inner, uint8_t, NETTLE_MAX_HASH_CONTEXT_SIZE);
- TMP_DECL (outer, uint8_t, NETTLE_MAX_HASH_CONTEXT_SIZE);
- TMP_DECL (state, uint8_t, NETTLE_MAX_HASH_CONTEXT_SIZE);
[...]
Ah, no reason really. I wrote the inner/outer/state part later, after settling on the nettle_hash abstraction, so this was code inspired by hmac.c. I found the hmac interface a bit odd here, so there may be better ways to do this.
Hmm. I don't think using TMP_DECL like that is right. If HAVE_ALLOCA, then it's going to be a plain alloca, which is what we really want. The problem is the fallback case, when we don't use allloca. Then it expands to
uint8_t inner[NETTLE_MAX_HASH_CONTEXT_SIZE];
which may not be properly aligned.
We shouldn't use malloc here, so if we can't think up something completely different, I think nettle-internal.h needs to define some union type which makes the C compiler to provide sufficient space and proper alignment. Somewhat like sockaddr_storage.
And the hmac code uses TMP_DECL/TMP_ALLOC for input blocks and digests, not for the context structs.
(Maybe it should be designed differently? I'll send a separate reply on that).
I'm hoping this issue goes away with your new proposed interface, I think it does.
/Simon