nisse@lysator.liu.se (Niels Möller) writes:
Simon Josefsson simon@josefsson.org writes:
Preparing an updated patch was easy, here it is and should apply to git master. This improves on a few minor issues compared to the old patch.
Looks solid to me.
I was at first considering if one could do it without the struct nettle_hash abstraction, but since that is used for the hmac functions (I think I tried without, and that turned out to be too inconvenient), I think it makes sense to use it here as well.
Yes, this was the trickiest part to resolve when I implemented it. However, it mimics the hmac interface, so at least there is some consistency.
--- a/nettle-internal.h +++ b/nettle-internal.h @@ -48,6 +48,7 @@ do { if (size > (sizeof(name) / sizeof(name[0]))) abort(); } while (0) #define NETTLE_MAX_BIGNUM_SIZE ((NETTLE_MAX_BIGNUM_BITS + 7)/8) #define NETTLE_MAX_HASH_BLOCK_SIZE 128 #define NETTLE_MAX_HASH_DIGEST_SIZE 64 +#define NETTLE_MAX_HASH_CONTEXT_SIZE 216 #define NETTLE_MAX_SEXP_ASSOC 17 #define NETTLE_MAX_CIPHER_BLOCK_SIZE 32
I'm a bit uncomfortable with that magic number. If sha512_ctx is the largest one, writing sizeof(struct sha512_ctx) is clearer. Or one could even go for sizeof(union { struct struct sha512_ctx sha512; struct foo_ctx foo; ... }).
Did you notice that testsuite/meta-hash-test.c was modified as well to make sure the magic number is OK? A sizeof or sizeof-union could work too, but then nettle-internal.h would need more #include's.
+Derive symmetric key from a password according to PKCS #5 PBKDF2. The +PRF is the HMAC familly with @var{hash} indicating the underlying hash
s/familly/family/
Right.
+void +pbkdf2_hmac (unsigned Plen, const uint8_t * P,
unsigned Slen, const uint8_t * S,const struct nettle_hash *hash,unsigned int c, unsigned dkLen, uint8_t * DK)+{
- unsigned int hLen = hash->digest_size;
- char U[NETTLE_MAX_HASH_DIGEST_SIZE];
- char T[NETTLE_MAX_HASH_DIGEST_SIZE];
[...]
- 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);
Any good reason to allocate the digests and the contexts in different ways? (One issue with the current hash and hmac interfaces is that all three of inner, outer and state include a buffer, while we really need only one).
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.
/Simon