Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bouncycastle does not retransmit the last server handshake flight on receipt of the retransmitted last client flight #1489

Closed
JonathanLennox opened this issue Sep 14, 2023 · 7 comments

Comments

@JonathanLennox
Copy link

When performing a DTLS handshake as the server, BouncyCastle does not respond to a retransmission of the client's second DTLS flight with a retransmission of the server flight.

See the attached PCAP (gzip'd because GitHub doesn't allow .pcap files).
Stuck-DTLS-handshake.pcap.gz

This is a handshake between Chrome 117.0.5938.62 and BouncyCastle 1.75, with artificial packet reordering introduced with the Linux tc tool (netem delay 100.0ms 75.0ms). A bug in Chrome's stack causes it not to handle the case where the packet containing the server's Encrypted "Finish" message precedes the packet containing the server's ChangeCipherSpec message, so Chrome retransmits its "Certificate, Client Key Exchange, Certificate Verify, Change Cipher Spec, Encrypted Handshake Message" flight (which happens to be aggregated into a single packet).

This problem appears to have been introduced in BouncyCastle 1.74; BouncyCastle 1.73 properly retransmits the server flight. 1.76 shows the same behavior as 1.75.

Tested with Chrome (as mentioned) using WebRTC, talking to the Jitsi Videobridge which is using (the various versions of ) BouncyCastle-Java.

@JonathanLennox
Copy link
Author

It looks like the problem was introduced in commit 2307836 by @peterdettman , and the issue is that DTLSRecordLayer.receivePendingRecord doesn't recognize the client's change_cipher_spec from the previous epoch as being part of the retransmitted handshake.

@JonathanLennox
Copy link
Author

JonathanLennox commented Sep 19, 2023

More specifically, when receivePendingRecord fails to parse the change_cipher_spec, it clears out the entirety of the rest of recordQueue, which includes the encrypted "Finish" message. Thus the "Finish" message is never processed, and so the server flight is not retransmitted.

@JonathanLennox
Copy link
Author

A fix is now in #1491.

@JonathanLennox
Copy link
Author

Note this is not just an artificial problem, though my reproducer is artificial - with Jitsi-Videobridge using BC 1.75 we had lots of reports in production of the DTLS-SRTP handshake not completing successfully and people not being able to join calls, which we concluded was because there was some random packet loss and/or reordering in the DTLS handshake which BC couldn't correctly recover from.

@JonathanLennox
Copy link
Author

The issue on the Chrome side is reported at https://bugs.chromium.org/p/webrtc/issues/detail?id=15495, but as their behavior is reasonable (they discard encrypted packets that outrace their corresponding key change) as they mention the BC issues are more serious.

@peterdettman
Copy link
Collaborator

Thanks very much for the report and the diagnosis. I will review the proposed fix.

@JonathanLennox
Copy link
Author

Fixed in #1491

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants