Missing check for message limits, issues with data message counter

[Lekensteyn]

The protocol defines the following message limits:

• Rekey-After-Messages 2^64 * 2^16 - 1
• Reject-After-Messages 2^64 − 2^4 − 1

Although unlikely in practice, the implementation must check that these message limits are not reached and avoid a counter wraparound. The implementation of ReceivingKeyCounterValidator in src/noise/session.rs must be updated to check this.

Right now a malicious implementation could produce the following messages sequence which is accepted by boringtun:

• counter=0 (sets next to 1 in mark_did_receive)
• counter=2^64 - 1 (sets next to 0 due to integer overflow)
• counter=0 (oops! counter reuse - replay accepted!)

Additionally, going from counter 0 to counter 2^64 will also result in a lot of unnecessary executions which facilitates a DoS:

        // Packets where dropped, or maybe reordered, skip them and mark unused
        let mut i = self.next; // 1
        while i < counter { // i = 1 .. 2^64 - 1
            self.clear_bit(i);
            i += 1;
        }

If the gap is too big, perhaps the whole bitmap should be cleared.

FWIW, the WireGuard kernel implementation uses a window of 2048 bits whereas this one uses 1024.

Another related issue is that sending_key_counter and sending_key_counter are an AtomicUsize which may be 32-bit on 32-bit architectures whereas the counter in the protocol is 64-bit.

[vkrasnov]

Although unlikely in practice

Impossible in practice.

malicious implementation could produce

But it would only compromise the connection with the malicious implementation itself, not any other connection, I don't really see the problem here.

If the gap is too big, perhaps the whole bitmap should be cleared.

I could have sworn that I implemented that, but apparently I didn't, good catch

[vkrasnov]

I have to merge the PR because it fixes a critical issue, but I will open a new issue that only addresses the limits. Or you are welcome to do so.