<div dir="ltr"><div><div><div><div><div><div><div><div><div><div>Some more comments.<br><br></div>One thing I didn't see was prologues.  The entire first packet with the list of protocols being offered, and the selected protocol number, should be hashed into the HandshakeState of the final chosen protocol.<br><br></div>Here's the attack scenario: a MITM knows that a target's AESGCM implementation is poorly implemented and vulnerable to timing issues.  So they modify incoming connection requests to change all "ChaChaPoly" protocols into "ChaChaPoli", which forces the target to always select AESGCM ciphers.<br><br></div>But there's a chicken-and-egg problem.  The first handshake packet cannot be a prologue for any of the individual protocols, as you don't know the values to put in the prologue until after you generate it ... from the prologue.<br><br></div>Suggestion: When the responder selects its protocol of choice, it calls MixHash() twice on the SymmetricState associated with the HandshakeState of the chosen protocol before the next WriteMessage call:<br><br></div>- The first MixHash() call is passed the complete contents of the first negotiation packet, excluding Ps.<br></div>- The second MixHash() call is passed the single-byte index of the chosen protocol.<br><br></div><div>Similarly for the initiator once the response is received.<br><br></div><div>From the second handshake packet onwards, MAC values will incorporate the new "h" as associated data and the original negotiation will be strongly bound into the handshake.<br></div><div><br></div>This is a modification to how Noise protocols typically work, but I think it is justifiable.<br><br></div><div>It does require the initiator to hold onto the complete first packet until it receives a response, which may affect memory usage on embedded platforms.  This is due to the definition of MixHash(data): h = HASH(h || data).  If it was MixOther(data): h = HASH(data || h), then the initiator could optimistically start hashing the packet with all candidate hash algorithms and then add the "h" later once a specific protocol is chosen.<br><br></div><div>Perhaps we should add MixOther(data) to the API for HandshakeState?<br><br></div>Cheers,<br><br></div>Rhys.<br><br></div>P.S. I vaguely recall some discussion along this line before, so apologies if I'm repeating things from months ago.<br><br></div>