[noise] Comments on revision 29

[Klaus Hartke]

Hi,

I've implemented revision 29 of Noise in C# and have a few comments:

* The specification is a bit opaque when read for the first time, but
section 5 is very clear and easy to translate into code. I was only a
bit confused by step "Calls MixHash() once for each public key listed
in the pre-messages from handshake_pattern" which is quite abstract
compared to the other steps. Maybe an example or more detailed steps
would be helpful for future readers.

* Section 10.3 specifies that "the 96-bit nonce is formed by encoding
32 bits of zeros followed by little-endian encoding of n" while
section 10.4 specifies that "the 96-bit nonce is formed by encoding 32
bits of zeros followed by big-endian encoding of n." Is there a reason
for the different endianness? Supporting two nonce formats is a bit
annoying when the cipher implementation expects a sequence of 12 bytes
and not a 64-bit unsigned integer.

* Section 5.2 says that "if protocol_name is less than or equal to
HASHLEN bytes in length, sets h equal to protocol_name with zero bytes
appended to make HASHLEN bytes. Otherwise sets h =
HASH(protocol_name)." Could this be simplified to always set h =
HASH(protocol_name)?

(The motivation is that "in protocol design, perfection has been
reached not when there is nothing left to add, but when there is
nothing left to take away." Why require implementations to support
both big-endian and little-endian nonces and both hashing and padding
the protocol name.)

* The specification refers to a "synthetic IV" in a few places.
"Synthetic IV" is not RFC 5116 terminology; does this mean that Noise
can be used with non-AEAD algorithms? If not, it's a bit confusing
where a "synthetic IV" might come from.

* The specification seems to be built on the assumption that HASHLEN =
CIPHERKEYLEN = 32 in most cases and provides workarounds where HASHLEN
= 64. For example, in section 5.2 MixKey uses HKDF to derive two
HASHLEN byte sequences from the input keying material and then
shortens one of them to CIPHERKEYLEN. This is a bit weird; I would
have expected something like this:

    prk = HKDF-Extract(ck, input_key_material)
    ck = HKDF-Expand(prk, "Noise Chaining Key", HASHLEN)
    temp_k = HKDF-Expand(prk, "Noise Handshake Key", CIPHERKEYLEN)

Similarly, Split:

    prk = HKDF-Extract(ck, zerolen)
    temp_k1 = HKDF-Expand(prk, "Noise Initiator Key", CIPHERKEYLEN)
    temp_k2 = HKDF-Expand(prk, "Noise Responder Key", CIPHERKEYLEN)

Invoking HKDF for each purpose separately like above would simplify my
implementation a lot and would decouple CIPHERKEYLEN from HASHLEN. In
any case I think it would be good to add CIPHERKEYLEN as a constant to
section 4.2 instead of hard-coding the key length to 32 bytes.

* I'm using libsodium which provides keyed and non-keyed Blake2b,
SHA-256, SHA-512, HMAC-SHA-256 and HMAC-SHA-512 but not HMAC-Blake2b.
Could keyed Blake2b be used instead of HMAC-Blake2b?

Klaus