[noise] Comments on revision 29

[Trevor Perrin]

Hi Klaus,

Good comments, I'm making notes to clarify things.

If your implementation is open source, I'd love to take a look and link to it.

The time for making cosmetic tweaks to the crypto has probably passed
(we have a bunch of open-source implementations, and a billion-ish
commercial users), but I'll explain why we made some decisions:


On Fri, May 20, 2016 at 7:01 AM, Klaus Hartke <hartke at tzi.org> wrote:
>
> * 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.

Good point, I'll try to improve that.


> * 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?

GCM uses big-endian internally, ChaCha20 and Poly1305 use
little-endian internally.  So from the perspective of a concrete Noise
protocol, it's cleaner to have a consistent endian-ness in the crypto
code.

I'll explain that rationale in the spec.


> * 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)?

If you have a short protocol name you could skip the hash, which isn't
much savings but is something.

Since either way here is fine, we should probably just leave it as-is.


> * 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.

I interpreted AEAD as a concept, rather than a reference to a
particular RFC.  I guess that needs to be clarified, with references.


> * 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.

I don't see why that's simpler than making a single HKDF() call and
slicing two keys from the output.

You're also calling Expand with separate info values, which might not
be compatible with a generic "HKDF()" API.

Hugo's HKDF paper, Section 7, argues that the "feedback mode" for
generating successive outputs has some "heuristic" benefit, since "the
attacker has less knowledge on the inputs to the PRF".

The current Noise design always performs this feedback step, so that
key derivation of the second output is done consistently and
conservatively, regardless of HASHLEN.


> 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'd rather people not use shorter keys, and there's not much reason to
use longer keys, so why not keep it simple and just say 32?


> * 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?

We discussed this a lot.  For reasons of consistency, I'd prefer to
have a uniform interface to hash functions, and a uniform KDF design.
Also, the way we use the HMAC is not "just" as a MAC or PRF, so the
analysis isn't as simple as just swapping one MAC/PRF for another:

https://moderncrypto.org/mail-archive/noise/2016/000599.html

Trevor