sped updates - #287
Draft
fippo wants to merge 7 commits into
Draft
Conversation
Backport of upstream CL 452860 (patchset 5) onto m150_release. Avoids a scenario with DTLS 1.3 and PQC where the client sends the first part of a DTLS 1.3 flight and the server responds with no data to send. Returning an empty data attribute makes that response distinguishable from the closing handshake, which is signalled by an ack with no data attribute at all. Upstream CL: https://webrtc-review.googlesource.com/c/src/+/452860 Bug: webrtc:367395350
Previously if a ICE agent sent a binding request with a dtls data attribute and the peer including the ack attribute in its response it was assumed that the peer had handled the DTLS packet even if it was not included in the list of acknowledged packets. While this assumption holds for Chromium and libWebRTC, it is not necessarily true if the STUN and DTLS layers are separated. The sending side of this is not implemented and this was confirmed to to not happen in unit tests and benchmarks by adding a DCHECK testing the presence of the implicit ack in the actual acks. This might come back in the future and has some mixed implications for the benchmarks but that might be instability in the benchmarks rather than an actual effect. Bug: webrtc:367395350 Upstream CL: https://webrtc-review.googlesource.com/c/src/+/453161
drive-by: use more common `Buffer` instead of `BufferT<uint8_t>` Bug: webrtc:367395350 Upstream CL: https://webrtc-review.googlesource.com/c/src/+/497320
since it is possible for the DTLS 1.3 PQC server to only have received the first of two packets in the PQC flight 1 and hence not having anything to send. Bug: webrtc:367395350 Upstream CL: https://webrtc-review.googlesource.com/c/src/+/457540
This change introduces the `sped` ice-option for embedding dtls in stun. This allows the remote side to not negotiate it upfront. The existing in-band discovery is still done which remains important since STUN packets may arrive before the answer. This considers embedding an ICE option as defined in https://datatracker.ietf.org/doc/html/rfc8445#section-13 SDP munging of this new feature is disallowed. Re-upload of https://webrtc-review.googlesource.com/c/src/+/386820 Bug: webrtc:367395350 Upstream CL: https://webrtc-review.googlesource.com/c/src/+/428980
which means that we no longer speculative start without an indication that the peer supports this. Peer support is discovered by two triggers: * an offer/answer with the ice-option * a STUN packet with embedded DTLS which is passed to the DTLS transport as piggybacked If the peer does not support SPED this no longer configures the 24h timeout and behaves like the existing non-sped path. If the peer does support SPED then it is also assumed to support the API for adjusting the timeout mid-session. The timeout could be set lower though, e.g. to the signaling RTT. Bug: webrtc:367395350 Upstream CL: https://webrtc-review.googlesource.com/c/src/+/476202
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(tbd, ignore for the time being)