DTLS1.3: Fix missing retransmission due to failure to set timer. DTLS 1.3 considers itself connected earlier than DTLS 1.2 did - when second flight reaches the client. This CL fixes a bug that when client is connected (state_ != SSL_CONNECTING), it would not continue retransmitting. Continuous retransmission is needed when the third flight is lost multiple times. Or really anytime that DTLS request it :) This fixes the TODO in dtls_ice_integrationtest.cc in which dtls1.3 spuriously failed with certain (packet loss intensive) configurations. CREDITS: sergeysu@ that found and fixed the problem! Bug: chromium:441245658 Change-Id: I3302f6f384d7e4cda090184094a6fadaf7e4f129 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/406320 Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org> Reviewed-by: Harald Alvestrand <hta@webrtc.org> Commit-Queue: Jonas Oreland <jonaso@webrtc.org> Cr-Commit-Position: refs/heads/main@{#45439}
diff --git a/p2p/dtls/dtls_ice_integrationtest.cc b/p2p/dtls/dtls_ice_integrationtest.cc index c0011ca..b6b8d40 100644 --- a/p2p/dtls/dtls_ice_integrationtest.cc +++ b/p2p/dtls/dtls_ice_integrationtest.cc
@@ -127,11 +127,7 @@ BuiltInNetworkBehaviorConfig networkBehavior; networkBehavior.link_capacity = DataRate::KilobitsPerSec(220); - // TODO (webrtc:383141571) : Investigate why this testcase fails for - // DTLS 1.3 delay if networkBehavior.queue_delay_ms = 100ms. - // - unless both peers support dtls in stun, in which case it passes. - // - note: only for dtls1.3, it works for dtls1.2! - networkBehavior.queue_delay_ms = 50; + networkBehavior.queue_delay_ms = 100; networkBehavior.queue_length_packets = 30; networkBehavior.loss_percent = 50;
diff --git a/rtc_base/openssl_stream_adapter.cc b/rtc_base/openssl_stream_adapter.cc index 4acbb55..2b45d37 100644 --- a/rtc_base/openssl_stream_adapter.cc +++ b/rtc_base/openssl_stream_adapter.cc
@@ -852,10 +852,15 @@ // We check the timer even after SSL_CONNECTED, // but ContinueSSL() is only needed when SSL_CONNECTING if (state_ == SSL_CONNECTING) { + // Note: timeout is set inside ContinueSSL() ContinueSSL(); + } else if (state_ == SSL_CONNECTED) { + MaybeSetTimeout(); + } else { + RTC_DCHECK_NOTREACHED() << "state_: " << state_; } } else { - RTC_DCHECK_NOTREACHED(); + RTC_DCHECK_NOTREACHED() << "flag->alive() == false"; } // This callback will never run again (stopped above). return TimeDelta::PlusInfinity(); @@ -970,6 +975,12 @@ } } + MaybeSetTimeout(); + + return 0; +} + +void OpenSSLStreamAdapter::MaybeSetTimeout() { if (ssl_ != nullptr) { struct timeval timeout; if (DTLSv1_get_timeout(ssl_, &timeout)) { @@ -977,8 +988,6 @@ SetTimeout(delay); } } - - return 0; } void OpenSSLStreamAdapter::Error(absl::string_view context,
diff --git a/rtc_base/openssl_stream_adapter.h b/rtc_base/openssl_stream_adapter.h index 856d37c..c10e93e 100644 --- a/rtc_base/openssl_stream_adapter.h +++ b/rtc_base/openssl_stream_adapter.h
@@ -210,6 +210,8 @@ !peer_certificate_digest_value_.empty(); } + void MaybeSetTimeout(); + const std::unique_ptr<StreamInterface> stream_; absl::AnyInvocable<void(SSLHandshakeError)> handshake_error_;