Bug 1546562: Teach TransportLayerDtls and TransportLayerSrtp to cope when TransportLayerIce fails, and then recovers. r=mjf
authorByron Campen [:bwc] <docfaraday@gmail.com>
Fri, 26 Apr 2019 14:49:12 +0000
changeset 530567 d8aabb74946258bdaa6964e7e41fdd409604ae73
parent 530566 e46a4bcb07b769fba96fa572590cc8711586a263
child 530568 0e93a381964bd65bf2785c7856b3d9806094f13e
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmjf
bugs1546562
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1546562: Teach TransportLayerDtls and TransportLayerSrtp to cope when TransportLayerIce fails, and then recovers. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D28840
media/mtransport/nricectx.cpp
media/mtransport/transportlayerdtls.cpp
media/mtransport/transportlayersrtp.cpp
--- a/media/mtransport/nricectx.cpp
+++ b/media/mtransport/nricectx.cpp
@@ -1025,16 +1025,20 @@ nsresult NrIceCtx::Finalize() {
   }
 
   return NS_OK;
 }
 
 void NrIceCtx::UpdateNetworkState(bool online) {
   MOZ_MTLOG(ML_INFO, "NrIceCtx(" << name_ << "): updating network state to "
                                  << (online ? "online" : "offline"));
+  if (connection_state_ == ICE_CTX_CLOSED) {
+    return;
+  }
+
   if (online) {
     nr_ice_peer_ctx_refresh_consent_all_streams(peer_);
   } else {
     nr_ice_peer_ctx_disconnect_all_streams(peer_);
   }
 }
 
 void NrIceCtx::SetConnectionState(ConnectionState state) {
--- a/media/mtransport/transportlayerdtls.cpp
+++ b/media/mtransport/transportlayerdtls.cpp
@@ -806,62 +806,68 @@ std::vector<uint16_t> TransportLayerDtls
   // longer offer this in Nightly builds.
   ciphers.push_back(kDtlsSrtpAes128CmHmacSha1_32);
 #endif
 
   return ciphers;
 }
 
 void TransportLayerDtls::StateChange(TransportLayer *layer, State state) {
-  if (state <= state_) {
-    MOZ_MTLOG(ML_ERROR, "Lower layer state is going backwards from ours");
-    TL_SET_STATE(TS_ERROR);
-    return;
-  }
-
   switch (state) {
     case TS_NONE:
       MOZ_ASSERT(false);  // Can't happen
       break;
 
     case TS_INIT:
       MOZ_MTLOG(ML_ERROR,
                 LAYER_INFO << "State change of lower layer to INIT forbidden");
       TL_SET_STATE(TS_ERROR);
       break;
 
     case TS_CONNECTING:
       MOZ_MTLOG(ML_INFO, LAYER_INFO << "Lower layer is connecting.");
       break;
 
     case TS_OPEN:
-      MOZ_MTLOG(ML_INFO, LAYER_INFO << "Lower layer is now open; starting TLS");
-      // Async, since the ICE layer might need to send a STUN response, and we
-      // don't want the handshake to start until that is sent.
-      TL_SET_STATE(TS_CONNECTING);
-      timer_->Cancel();
-      timer_->SetTarget(target_);
-      timer_->InitWithNamedFuncCallback(TimerCallback, this, 0,
-                                        nsITimer::TYPE_ONE_SHOT,
-                                        "TransportLayerDtls::TimerCallback");
+      if (timer_) {
+        MOZ_MTLOG(ML_INFO,
+                  LAYER_INFO << "Lower layer is now open; starting TLS");
+        timer_->Cancel();
+        timer_->SetTarget(target_);
+        // Async, since the ICE layer might need to send a STUN response, and we
+        // don't want the handshake to start until that is sent.
+        timer_->InitWithNamedFuncCallback(TimerCallback, this, 0,
+                                          nsITimer::TYPE_ONE_SHOT,
+                                          "TransportLayerDtls::TimerCallback");
+        TL_SET_STATE(TS_CONNECTING);
+      } else {
+        // We have already completed DTLS. Can happen if the ICE layer failed
+        // due to a loss of network, and then recovered.
+        TL_SET_STATE(TS_OPEN);
+      }
       break;
 
     case TS_CLOSED:
       MOZ_MTLOG(ML_INFO, LAYER_INFO << "Lower layer is now closed");
       TL_SET_STATE(TS_CLOSED);
       break;
 
     case TS_ERROR:
       MOZ_MTLOG(ML_ERROR, LAYER_INFO << "Lower layer experienced an error");
       TL_SET_STATE(TS_ERROR);
       break;
   }
 }
 
 void TransportLayerDtls::Handshake() {
+  if (!timer_) {
+    // We are done with DTLS, regardless of the state changes of lower layers
+    return;
+  }
+
   // Clear the retransmit timer
   timer_->Cancel();
 
   MOZ_ASSERT(state_ == TS_CONNECTING);
 
   SECStatus rv = SSL_ForceHandshake(ssl_fd_.get());
 
   if (rv == SECSuccess) {
@@ -878,16 +884,17 @@ void TransportLayerDtls::Handshake() {
       ssl_fd_ = nullptr;
       TL_SET_STATE(TS_ERROR);
       return;
     }
 
     TL_SET_STATE(TS_OPEN);
 
     RecordTlsTelemetry();
+    timer_ = nullptr;
   } else {
     int32_t err = PR_GetError();
     switch (err) {
       case SSL_ERROR_RX_MALFORMED_HANDSHAKE:
         MOZ_MTLOG(ML_ERROR, LAYER_INFO << "Malformed DTLS message; ignoring");
         // If this were TLS (and not DTLS), this would be fatal, but
         // here we're required to ignore bad messages, so fall through
         MOZ_FALLTHROUGH;
@@ -1038,17 +1045,17 @@ void TransportLayerDtls::GetDecryptedPac
         }
       }
     } while (rv > 0);
   }
 }
 
 void TransportLayerDtls::SetState(State state, const char *file,
                                   unsigned line) {
-  if (state > state_) {
+  if (timer_) {
     switch (state) {
       case TS_NONE:
       case TS_INIT:
         MOZ_ASSERT(false);
         break;
       case TS_CONNECTING:
         handshake_started_ = TimeStamp::Now();
         break;
@@ -1056,18 +1063,16 @@ void TransportLayerDtls::SetState(State 
       case TS_CLOSED:
       case TS_ERROR:
         timer_->Cancel();
         if (state_ == TS_CONNECTING) {
           RecordHandshakeCompletionTelemetry(state);
         }
         break;
     }
-  } else {
-    MOZ_ASSERT(false, "Invalid state transition");
   }
 
   TransportLayer::SetState(state, file, line);
 }
 
 TransportResult TransportLayerDtls::SendPacket(MediaPacket &packet) {
   CheckThread();
   if (state_ != TS_OPEN) {
--- a/media/mtransport/transportlayersrtp.cpp
+++ b/media/mtransport/transportlayersrtp.cpp
@@ -104,17 +104,17 @@ TransportResult TransportLayerSrtp::Send
   if (bytes == TE_WOULDBLOCK) {
     return TE_WOULDBLOCK;
   }
 
   return TE_ERROR;
 }
 
 void TransportLayerSrtp::StateChange(TransportLayer* layer, State state) {
-  if (state == TS_OPEN) {
+  if (state == TS_OPEN && !mSendSrtp) {
     TransportLayerDtls* dtls = static_cast<TransportLayerDtls*>(layer);
     MOZ_ASSERT(dtls);  // DTLS is mandatory
 
     uint16_t cipher_suite;
     nsresult res = dtls->GetSrtpCipher(&cipher_suite);
     if (NS_FAILED(res)) {
       MOZ_MTLOG(ML_DEBUG, "DTLS-SRTP disabled");
       TL_SET_STATE(TS_ERROR);