Zoltan Fridrich <[email protected]> writes:

> I am sending you an updated patch. I have resolved all of the issues except
> for the modulo.
> Can you please take a look again?

Thanks, looks in pretty pretty good shape now. Can you help out with
docs? It can be a followup change, code can be submitted firwst.

Docs should go in nettle.texinfo, probably in the "Key derivation
functions" section, even if output size is fixed (unlike other key
derivation functions which can also expand the key).

> +/* Takes length bytes long big number stored
> + * in little endian format and computes modulus
> + */
> +static size_t
> +block_to_int(size_t length, const uint8_t *block, size_t mod)
> +{
> +    size_t i = length, r = 0;
> +    while (i--) {
> +        r = (r << 8) + block[i];
> +        r %= mod;
> +    }
> +    return r;
> +}

This is quite clear. Might be interesting to run a profiler to see if it
is in any way critical for performance. It requires that mod is a least
8 bits smaller than a size_t, but it seems the only call passes the hash
digest size as the length, so that should be fine.

> +void
> +balloon(void *hash_ctx,
> +        nettle_hash_update_func *update,
> +        nettle_hash_digest_func *digest,
> +        size_t digest_size, size_t s_cost, size_t t_cost,
> +        size_t passwd_length, const uint8_t *passwd,
> +        size_t salt_length, const uint8_t *salt,
> +        uint8_t *buf, uint8_t *dst)
> +{
> +    uint8_t block[NETTLE_MAX_HASH_DIGEST_SIZE];
> +    const size_t BS = digest_size;

Would be good with an 

  assert (digest_size <= NETTLE_MAX_HASH_DIGEST_SIZE);

Or maybe better: Increase return value of balloon_itch, and have it the
block allocated at start or end of the passed in area. Something like

  void
  balloon(void *hash_ctx,
          nettle_hash_update_func *update,
          nettle_hash_digest_func *digest,
          size_t digest_size, size_t s_cost, size_t t_cost,
          size_t passwd_length, const uint8_t *passwd,
          size_t salt_length, const uint8_t *salt,
          uint8_t *scratch, uint8_t *dst)
  {
      const size_t BS = digest_size;
      uint8_t *block = scratch;
      uint8_t *buf = scratch + BS;
      ...
  }
   
  size_t
  balloon_itch(size_t digest_size, size_t s_cost)
  {
      return (s_cost + 1) * digest_size;
  }

> +    size_t i, j, k, cnt = 0;
> +
> +    hash(hash_ctx, update, digest, digest_size,
> +         cnt++, passwd_length, passwd, salt_length, salt, buf);
> +    for (i = 1; i < s_cost; ++i)
> +        hash(hash_ctx, update, digest, digest_size,
> +             cnt++, BS, buf + (i - 1) * BS, 0, NULL, buf + i * BS);
> +
> +    for (i = 0; i < t_cost; ++i) {

Formatting nit: GNU style used in Nettle puts braces on their own line,
indented halfway compared to the body of the block, like

    for (i = 0; i < t_cost; ++i) 
      {
        hash(hash_ctx, update, digest, digest_size, ...);
        ...
      }

> +        for (j = 0; j < s_cost; ++j) {
> +            hash(hash_ctx, update, digest, digest_size,
> +                 cnt++, BS, buf + (j ? j - 1 : s_cost - 1) * BS,
> +                 BS, buf + j * BS, buf + j * BS);
> +            for (k = 0; k < DELTA; ++k) {
> +                hash_ints(hash_ctx, update, digest, digest_size, i, j, k, 
> block);
> +                hash(hash_ctx, update, digest, digest_size,
> +                     cnt++, salt_length, salt, BS, block, block);
> +                hash(hash_ctx, update, digest, digest_size,
> +                     cnt++, BS, buf + j * BS,
> +                     BS, buf + block_to_int(BS, block, s_cost) * BS,
> +                     buf + j * BS);
> +            }
> +        }
> +    }
> +    memcpy(dst, buf + (s_cost - 1) * BS, BS);

For the docs, consider if it should be documented that calling the
function with dst == buf is allowed.

> +}
> +
> +void
> +balloon_sha1(size_t s_cost, size_t t_cost,
> +             size_t passwd_length, const uint8_t *passwd,
> +             size_t salt_length, const uint8_t *salt,
> +             uint8_t *buf, uint8_t *dst)
> +{

One could consider moving these wrapper functions to separate source
files, so that one can use, e.g, balloon_sha256, without getting a link
dependency also on sha512. But not essential for the first version.

> +/*
> + * Test vectors are taken from:
> + * <https://github.com/nachonavarro/balloon-hashing>
> + * <https://github.com/RustCrypto/password-hashes/tree/master/balloon-hash>
> + */
> +void
> +test_main(void)
> +{
> +    test_balloon(&nettle_sha256, 8, "hunter42", 11, "examplesalt", 1024, 3,
> +                 
> SHEX("716043dff777b44aa7b88dcbab12c078abecfac9d289c5b5195967aa63440dfb"));
> +    test_balloon(&nettle_sha256, 0, "", 4, "salt", 3, 3,
> +                 
> SHEX("5f02f8206f9cd212485c6bdf85527b698956701ad0852106f94b94ee94577378"));
> +    test_balloon(&nettle_sha256, 8, "password", 0, "", 3, 3,
> +                 
> SHEX("20aa99d7fe3f4df4bd98c655c5480ec98b143107a331fd491deda885c4d6a6cc"));
> +    test_balloon(&nettle_sha256, 1, "", 1, "", 3, 3,
> +                 
> SHEX("4fc7e302ffa29ae0eac31166cee7a552d1d71135f4e0da66486fb68a749b73a4"));
> +    test_balloon(&nettle_sha256, 8, "password", 4, "salt", 1, 1,
> +                 
> SHEX("eefda4a8a75b461fa389c1dcfaf3e9dfacbc26f81f22e6f280d15cc18c417545"));
> +}

Would be good with at least a single test for each of the balloon_shaXXX
(even if test vectors verified with other implementations aren't
available).

Regards,
/Niels

-- 
Niels Möller. PGP key CB4962D070D77D7FCB8BA36271D8F1FF368C6677.
Internet email is subject to wholesale government surveillance.
_______________________________________________
nettle-bugs mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to