Nikos Mavrogiannopoulos <[email protected]> writes:

>  The attached patch set introduces new APIs to access the TLS 1.0 and
> 1.2 PRFs as well as the HKDF (rfc5869) function used in TLS 1.3.

Thanks, a few initial comments.

Are the TLS-1.0 and TLS-1.2 PRFs in any use outside of TLS?

> There are few other updates to .gitlab-ci.yml (to use a more recent
> asan and ubsan versions),

Applied.

>  and a comment in .bootstrap for the
> testsuite/.test-rules.make which I always seem to forget and spend an
> hour figuring out what is going on. Ideally .test-rules.make should
> depend on Makefile.in.

Have you tested if it works to just add that dependency? 

An alternative might be to drop support for non-GNU make programs, then
one could probably write these as %-pattern rules directly in
Makefile.in.

> I'd also suggest to rename the .bootstrap to bootstrap.sh as the latter
> is quite a convention to find in projects (.bootstrap is not even
> listed in ls).

Makes sense. There are references to the name .bootstrap in README and
in .gitlab-ci.yml, any other place that would need updating?

> +void
> +hkdf_extract(void *mac_ctx,
> +          nettle_hmac_set_key_func *set_key,
> +          nettle_hash_update_func *update,
> +          nettle_hash_digest_func *digest,
> +          size_t digest_size,
> +          size_t salt_size, const uint8_t *salt,
> +          size_t secret_size, const uint8_t *secret,
> +          uint8_t *dst)
> +{
> +     set_key(mac_ctx, salt_size, salt);
> +
> +     update(mac_ctx, secret_size, secret);
> +     digest(mac_ctx, digest_size, dst);
> +}

This looks like a plain application of a mac, digest = MAC(salt,
secret), is this really useful?

Not sure about the typedef nettle_hmac_set_key_func, there's nothing
hmac specific, besides key size being variable? And it's identical to
nettle_hash_update_func, right?

Also, since it seems the key is fixed, both in this function and below, I
think it would be better to leave setting the mac key to the caller, and
pass in a mac ctx with key already set. That would also be more consistent
with the pbkdf2 code, and reduce the number of argument.

Note that the Nettle mac convention is that the sequence

  set_key
  update
  digest
  update
  digest
  ...
  update
  digest

works fine.

> +void
> +hkdf_expand(void *mac_ctx,
> +          nettle_hmac_set_key_func *set_key,
> +          nettle_hash_update_func *update,
> +          nettle_hash_digest_func *digest,
> +          size_t digest_size,
> +          size_t prk_size, const uint8_t *prk,
> +          size_t info_size, const uint8_t *info,
> +          size_t length, uint8_t *dst)
> +{
> +     uint8_t *Ttmp;
> +     ssize_t left;
> +     uint8_t i = 1;
> +     unsigned started = 0;
> +
> +     left = length;
> +
> +     while(left > 0) {
> +             /* T(i) */
> +             set_key(mac_ctx, prk_size, prk);
> +             if (started != 0) {
> +                     update(mac_ctx, digest_size, Ttmp);
> +             } else {
> +                     started = 1;
> +             }
> +             if (info_size)
> +                     update(mac_ctx, info_size, info);
> +             update(mac_ctx, 1, &i);
> +
> +             if (left < digest_size)
> +                     digest_size = left;
> +
> +             digest(mac_ctx, digest_size, dst);
> +             Ttmp = dst;
> +
> +             left -= digest_size;
> +             dst += digest_size;
> +             i++;
> +     }

I think this loop would clearer if Ttmp was replaced by (dst -
digest_size), and maybe it would make sense to take out the first and/or
final iterations.

Regards,
/Niels

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

Reply via email to