feature: Tighten the constraints for act message sizes (discussion)

[julianknutsen]

The current design leaves excess bytes in the read_buffer across state transitions. This means that excess bytes would be processed as part of the next process_act(). This makes sense in the Responder::Act3 case since The Initiator has everything it needs to send the Init message and could just send it along directly after Act3. But, for Act1 & Act2 this isn't valid because the remote peer would need interleaved responses in order to generate the correct data. In those invalid cases, the code can fail early.

Is it worth being robust to handshake act sizes that are invalid prior to the next received bytes that would generate the failure?

from @ariard

At first, I thought it would add a padding oracle but in fact I don't see how an attacker would learn anything as the error would be triggered by him and remote honest party learn communication has been tampered by either receiving such error message or not receiving anything new. So I think it's okay but I would like to look through Noise rational here, BOLT8 might not leak all Noise assumptions.

Relevant patch:

feature: Tighten the constraints for act message sizes

Proper handshake peers won't send more bytes than required for
act1 and act2 and it is impossible to generate future acts
without first receiving an interleaved response. In those cases, the
peer is misbehaving and we can fail early.

0001-feature-Tighten-the-constraints-for-act-message-size.patch.txt

[TheBlueMatt]

I'm not sure it matters too much, as you point out unless the peer knows our private key they can't correctly complete the handshake here, but its probably worth being robust against it, just from a correctness point of view. Again, I believe this is correct on master.

[julianknutsen]

Closing this out. Implemented as part of arik-so#2.

Aside from the robustness which I think is nice to have, this behavior also gives us a guaranteed read buffer size during the initial act handling that allows for stack-based buffers. I'm not sure how much this performance really matters, but it makes the design a bit cleaner and minimizes the cases the code needs to handle.