[noise] BLAKE2 as extractor with empty data

[Jean-Philippe Aumasson]

Thanks Jason for reporting this. We should allow "in" to be NULL if inlen
is zero. It's ok allow (inlen==0) && (keylen==0) though (we should err on
NULL pointers only if their length parameters is nonzero).

We'll update our code.


On Wed, Oct 14, 2015 at 12:29 PM Jason A. Donenfeld <Jason at zx2c4.com> wrote:

> Hi JP,
>
> Since you're on the noise list now, and we've (I've?) been talking
> about using BLAKE2 as a substitute for HMAC in the KDF, here's a
> considerable concern with noise:
>
> After various operations have been done, a function called Split() is
> called to begin a noise transport session. This function is currently
> defined as:
>
>     Split(): Creates two child CipherState objects by calling HKDF(ck,
> empty)
>     where empty is a zero-length byte sequence. The first child's k is
> set to the
>     first output from HKDF(), and the second child's k is set to the
> second output
>     from HKDF(). Both children's n value is set to zero. Both children
> are returned.
>
> Calling HKDF(ck, [empty]) might have some issues. Keeping with the
> HKDF extract-then-expand construction, but replacing HMAC with keyed
> BLAKE2, this amounts to:
>
>     temp_key = BLAKE2(chaining_key, [empty])
>     output1 = BLAKE2(temp_key, 0x01)
>     output2 = BLAKE2(temp_key, output1 || 0x02)
>     return (output1, output2)
>
> The problematic line is the first one:
>
>     temp_key = BLAKE2(chaining_key, [empty])
>
> Examining the reference implementation shows several checks at the
> beginning of the function:
>
> int blake2b( uint8_t *out, const void *in, const void *key, const
> uint8_t outlen, const uint64_t inlen, uint8_t keylen )
> {
>   blake2b_state S[1];
>   /* Verify parameters */
>   if ( NULL == in ) return -1;
>   if ( NULL == out ) return -1;
>   if( NULL == key ) keylen = 0;
>   if( keylen > 0 )
>     if( blake2b_init_key( S, outlen, key, keylen ) < 0 ) return -1;
>   else
>     if( blake2b_init( S, outlen ) < 0 ) return -1;
>   blake2b_update( S, ( const uint8_t * )in, inlen );
>   blake2b_final( S, out, outlen );
>   return 0;
> }
>
> Notably, the problem is with:
>
>   if ( NULL == in ) return -1;
>
> If there is an empty input, error out, it says. This would make BLAKE2
> unsuitable for the function above. It is possible, though, that this
> is an implementation error for keyed BLAKE2. The reason is that the
> call to "blake2b_init_key( S, outlen, key, keylen )" results in
> calling "blake2b_update" on the key. Thus, the only difference between
> calling BLAKE2 as a standard hash function and calling BLAKE2 as a
> keyed hash function with no data would be in the latter, the param
> block contains the keysize. Therefore I suspect the problematic line
> above would be better replaced with:
>
>   if ( NULL == in && ( NULL == key || 0 == keylen ) ) return -1;
>
> Would this be an acceptable modification? Or would it, in fact, break
> important assumptions underlying BLAKE2?
>
> Thanks,
> Jason
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://moderncrypto.org/mail-archive/noise/attachments/20151014/75be1814/attachment.html>