<div dir="ltr">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).<div> </div><div>We'll update our code.</div><div> <div> <div class="gmail_quote"><div dir="ltr">On Wed, Oct 14, 2015 at 12:29 PM Jason A. Donenfeld <<a href="mailto:Jason@zx2c4.com">Jason@zx2c4.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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 </blockquote></div></div></div></div>