[noise] verifying received static remote key

[David Wong]

>> 1) IF the remote static key is known because it was processed during a
>> pre-message pattern, it needs to be checked against the received one.
>> If they are different, the handshake needs to be aborted.
>
> That's an illegal pattern (section 7.1, pattern validity rule #2 - a
> key can only be sent once, including pre-messages).
>

Ah! I read this sentence in the spec several times and didn't really
grasp its meaning. In this case, it might be good to add in the
Write/ReadMessage() definition:

WriteMessage() → For "s": Appends EncryptAndHash(s.public_key) to the
buffer, only if it hasn't been previously sent in a pre-message.
ReadMessage() → For "s": Ignore this token if the remote static key
has already been set in a pre-message. Sets temp to the next DHLEN +
16 bytes of the message if HasKey() == True, or to the next DHLEN
bytes otherwise. Sets rs (which must be empty) to
DecryptAndHash(temp).

(Btw in my previous message I refered to these as N patterns. But in
reality these are K patterns.)

> We added a Security consideration in revision 33 to discuss this
> ("Authentication").

I strongly believe that this is not enough! This is one of the most,
if not the most, important security foundation of Noise. Without this
verification the whole security crumbles. Following, I propose the
addition of a `remoteKeyValid` boolean inside of a handshakeState.


> I'm not convinced about putting this in the pseudocode.  For one
> thing, you might perform the check at different points.  For example,
> if the handshake payload contains certificates, the check would not
> occur on processing the static but would be deferred until processing
> the payload.

If I understand the first point correctly, you will always have to
check a signature over a remote static key in order to validate it in
X patterns.
If so, this undefined verifyRemoteStaticKey() boolean function could
be mentioned after the DecryptAndHash step of a ReadMessage():

* Calls DecryptAndHash() on the remaining bytes of the message and
stores the output into payload_buffer.
* if a `rs` has been received (not null anymore) and the
`remoteKeyValid` boolean is false, call
verifyRemoteStaticKey(payload_buffer, rs)

You would also need to define a `remoteKeyValid` boolean in a
handshakeState along with s, e, rs and re.

When calling split, it would be also good to mention that it should
return an error if remoteKeyValid is set to false.

What do you think of this? I like the idea of having a remoteKeyValid
boolean value in the handshakeState to make sure that implementations
find a way to set it to true.