Simon Josefsson simon@josefsson.org writes:
nisse@lysator.liu.se (Niels Möller) writes:
That the HMAC interface takes this parameter:
const struct nettle_hash *hash
but the new PBKDF2 interface would take these:
void *mac_ctx, unsigned digest_size, nettle_hash_update_func *update, nettle_hash_digest_func *digest
which feels low-level, especially considering that those parameters are captured through the nettle_hash abstraction together with hard-coding PBKDF2 for HMAC.
The "low-level" interface is the preferred style in Nettle. See cbc_encrypt/cbc_decrypt for other examples. It's intended that the nettle_hash abstraction should be optional, and not used internally. But then it's used with hmac anyway, iirc that's because the alternative interface got *too* clumsy.
However, I suppose this complexity could be hidden with a utility function 'pbkdf2_hmac' that is similar to my original function signature.
Do you think a "half-general" pbkdf2_hmac function really is useful? I think specific wrapper functions, pbkdf2_hmac_sha256, etc, would be more useful.
pbkdf2-test.c:27:3: warning: passing argument 3 of 'nettle_pbkdf2' from incompatible pointer type [enabled by default] ../pbkdf2.h:40:1: note: expected 'void (*)(void *, unsigned int, const uint8_t *)' but argument is of type 'void (*)(struct hmac_sha1_ctx *, unsigned int, const uint8_t *)'
The reason is that pbkdf2 has this signature: typedef void nettle_hash_update_func(void *ctx, unsigned length, const uint8_t *src);
which is not compatible with any instantiation like this one:
void sha1_update(struct sha1_ctx *ctx, unsigned length, const uint8_t *data);
This is a problem shared with almost every function in nettle accepting function pointer arguments. I think we have to live with casts one way or the other; I don't see any good solution.
Casting the parameter would solve this. Is there any other way to resolve the warning? Do you think casting is an acceptable solution here? The problem seems to be that the casting needs to happen in the application not in the library.
At least specific functions like pbkdf2_hmac_sha256 functions will not expose the problem to users. It's also possible to write some macros to do the cast but preserve some type checking, something like
#define PBKDF2(ctx, update, digest, ...) \ (0 ? (update(ctx, 0, NULL), digest(ctx, 0, NULL)) \ : pbkdf(ctx, (nettle_hash_update_func *) update, (nettle_hash_digest_func *) digest, ..,))
The CBC_ENCRYPT and CBC_DECRYPT macros do this, and it seems to work fine, but it's not particularly pretty.
Any other ideas?
Thanks for the implementation, I'll try to get it integrated soon. Some minor comments (which I can fix):
--- 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
This is no longer needed, right?
+void +pbkdf2 (void *mac_ctx, unsigned digest_size,
- nettle_hash_update_func *update,
- nettle_hash_digest_func *digest,
- unsigned length, uint8_t *dst,
- unsigned iterations,
- unsigned salt_length, const uint8_t *salt)
+{
- char U[NETTLE_MAX_HASH_DIGEST_SIZE];
- char T[NETTLE_MAX_HASH_DIGEST_SIZE];
It would make sense to use TMP_ALLOC for those. And I think I'd use uint8_t rather than char (unless there's some reason for char I'm missing?).
tmp[0] = (i & 0xff000000) >> 24;tmp[1] = (i & 0x00ff0000) >> 16;tmp[2] = (i & 0x0000ff00) >> 8;tmp[3] = (i & 0x000000ff) >> 0;
There's WRITE_UINT32 for this.
Regards, /Niels