Simon Josefsson <si...@josefsson.org> writes:

> ni...@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

-- 
Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26.
Internet email is subject to wholesale government surveillance.
_______________________________________________
nettle-bugs mailing list
nettle-bugs@lists.lysator.liu.se
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs

Reply via email to