Bug 904598 - Fix TURN long-term auth for Permissions Requests. r=abr

media/mtransport/nricectx.cpp

@@ -318,12 +318,12 @@ RefPtr<NrIceCtx> NrIceCtx::Create(const std::string& name,
     nr_crypto_vtbl = &nr_ice_crypto_nss_vtbl;
     initialized = true;
 
-    // Set the priorites for candidate type preferences
-    NR_reg_set_uchar((char *)"ice.pref.type.srv_rflx",100);
-    NR_reg_set_uchar((char *)"ice.pref.type.peer_rflx",105);
-    NR_reg_set_uchar((char *)"ice.pref.type.prflx",99);
-    NR_reg_set_uchar((char *)"ice.pref.type.host",125);
-    NR_reg_set_uchar((char *)"ice.pref.type.relayed",126);
+    // Set the priorites for candidate type preferences.
+    // These numbers come from RFC 5245 S. 4.1.2.2
+    NR_reg_set_uchar((char *)"ice.pref.type.srv_rflx", 100);
+    NR_reg_set_uchar((char *)"ice.pref.type.peer_rflx", 110);
+    NR_reg_set_uchar((char *)"ice.pref.type.host", 126);
+    NR_reg_set_uchar((char *)"ice.pref.type.relayed", 0);
 
     if (set_interface_priorities) {
       NR_reg_set_uchar((char *)"ice.pref.interface.rl0", 255);

media/mtransport/nricemediastream.cpp

@@ -46,6 +46,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #include "logging.h"
 #include "nsError.h"
+#include "mozilla/Scoped.h"
 
 // nICEr includes
 extern "C" {
@@ -72,6 +73,52 @@ namespace mozilla {
 
 MOZ_MTLOG_MODULE("mtransport")
 
+// Make an NrIceCandidate from the candidate |cand|.
+// This is not a member fxn because we want to hide the
+// defn of nr_ice_candidate but we pass by reference.
+static NrIceCandidate* MakeNrIceCandidate(const nr_ice_candidate& candc) {
+  ScopedDeletePtr<NrIceCandidate> out(new NrIceCandidate());
+
+  int r;
+  // Const-cast because the internal nICEr code isn't const-correct.
+  nr_ice_candidate *cand = const_cast<nr_ice_candidate *>(&candc);
+  char addr[INET6_ADDRSTRLEN + 1];
+
+  r = nr_transport_addr_get_addrstring(&cand->addr, addr, sizeof(addr));
+  if (r)
+    return nullptr;
+
+  int port;
+  r=nr_transport_addr_get_port(&cand->addr, &port);
+  if (r)
+    return nullptr;
+
+  NrIceCandidate::Type type;
+
+  switch(cand->type) {
+    case HOST:
+      type = NrIceCandidate::ICE_HOST;
+      break;
+    case SERVER_REFLEXIVE:
+      type = NrIceCandidate::ICE_SERVER_REFLEXIVE;
+      break;
+    case PEER_REFLEXIVE:
+      type = NrIceCandidate::ICE_PEER_REFLEXIVE;
+      break;
+    case RELAYED:
+      type = NrIceCandidate::ICE_RELAYED;
+      break;
+    default:
+      return nullptr;
+  }
+
+  out->host = addr;
+  out->port = port;
+  out->type = type;
+
+  return out.forget();
+}
+
 // NrIceMediaStream
 RefPtr<NrIceMediaStream>
 NrIceMediaStream::Create(NrIceCtx *ctx,
@@ -150,6 +197,38 @@ nsresult NrIceMediaStream::ParseTrickleCandidate(const std::string& candidate) {
   return NS_OK;
 }
 
+nsresult NrIceMediaStream::GetActivePair(int component,
+                                         NrIceCandidate **localp,
+                                         NrIceCandidate **remotep) {
+  int r;
+  nr_ice_candidate *local_int;
+  nr_ice_candidate *remote_int;
+
+  r = nr_ice_media_stream_get_active(ctx_->peer(),
+                                     stream_,
+                                     component,
+                                     &local_int, &remote_int);
+  if (r)
+    return NS_ERROR_FAILURE;
+
+  ScopedDeletePtr<NrIceCandidate> local(
+      MakeNrIceCandidate(*local_int));
+  if (!local)
+    return NS_ERROR_FAILURE;
+
+  ScopedDeletePtr<NrIceCandidate> remote(
+      MakeNrIceCandidate(*remote_int));
+  if (!remote)
+    return NS_ERROR_FAILURE;
+
+  if (localp)
+    *localp = local.forget();
+  if (remotep)
+    *remotep = remote.forget();
+
+  return NS_OK;
+}
+
 
 void NrIceMediaStream::EmitAllCandidates() {
   char **attrs = 0;
@@ -248,9 +327,16 @@ nsresult NrIceMediaStream::SendPacket(int component_id,
 
 
 void NrIceMediaStream::Ready() {
-  MOZ_MTLOG(ML_DEBUG, "Marking stream ready '" << name_ << "'");
-  state_ = ICE_OPEN;
-  SignalReady(this);
+  // This function is called whenever a stream becomes ready, but it
+  // gets fired multiple times when a stream gets nominated repeatedly.
+  if (state_ != ICE_OPEN) {
+    MOZ_MTLOG(ML_DEBUG, "Marking stream ready '" << name_ << "'");
+    state_ = ICE_OPEN;
+    SignalReady(this);
+  }
+  else {
+    MOZ_MTLOG(ML_DEBUG, "Stream ready callback fired again for '" << name_ << "'");
+  }
 }
 
 void NrIceMediaStream::Close() {

media/mtransport/nricemediastream.h

@@ -63,6 +63,21 @@ typedef struct nr_ice_media_stream_ nr_ice_media_stream;
 
 class NrIceCtx;
 
+/* A summary of a candidate, for use in asking which candidate
+   pair is active */
+struct NrIceCandidate {
+  enum Type {
+    ICE_HOST,
+    ICE_SERVER_REFLEXIVE,
+    ICE_PEER_REFLEXIVE,
+    ICE_RELAYED
+  };
+
+  std::string host;
+  uint16_t port;
+  Type type;
+};
+
 class NrIceMediaStream {
  public:
   static RefPtr<NrIceMediaStream> Create(NrIceCtx *ctx,
@@ -89,6 +104,14 @@ class NrIceMediaStream {
   // Parse trickle ICE candidate
   nsresult ParseTrickleCandidate(const std::string& candidate);
 
+  // Get the candidate pair currently active. It's the
+  // caller's responsibility to free these.
+  nsresult GetActivePair(int component,
+                         NrIceCandidate** local, NrIceCandidate** remote);
+
+  // The number of components
+  int components() const { return components_; }
+
   // The underlying nICEr stream
   nr_ice_media_stream *stream() { return stream_; }
   // Signals to indicate events. API users can (and should)
@@ -126,11 +149,7 @@ class NrIceMediaStream {
       ctx_(ctx),
       name_(name),
       components_(components),
-      stream_(nullptr)
-  {
-    // XXX: components_ will be used eventually;  placate clang in the meantime.
-    (void)components_;
-  }
+      stream_(nullptr) {}
 
   DISALLOW_COPY_ASSIGN(NrIceMediaStream);
 

media/mtransport/test/ice_unittest.cpp

@@ -77,7 +77,9 @@ class IceTestPeer : public sigslot::has_slots<> {
       fake_resolver_(),
       dns_resolver_(new NrIceResolver()),
       remote_(nullptr),
-      candidate_filter_(nullptr) {
+      candidate_filter_(nullptr),
+      expected_local_type_(NrIceCandidate::ICE_HOST),
+      expected_remote_type_(NrIceCandidate::ICE_HOST) {
     ice_ctx_->SignalGatheringCompleted.connect(this,
                                                &IceTestPeer::GatheringComplete);
     ice_ctx_->SignalCompleted.connect(this, &IceTestPeer::IceCompleted);
@@ -180,6 +182,11 @@ class IceTestPeer : public sigslot::has_slots<> {
     return candidates;
   }
 
+  void SetExpectedTypes(NrIceCandidate::Type local, NrIceCandidate::Type remote) {
+    expected_local_type_ = local;
+    expected_remote_type_ = remote;
+  }
+
   bool gathering_complete() { return gathering_complete_; }
   int ready_ct() { return ready_ct_; }
   bool is_ready(size_t stream) {
@@ -255,6 +262,57 @@ class IceTestPeer : public sigslot::has_slots<> {
     }
   }
 
+  void DumpCandidate(std::string which, const NrIceCandidate& cand) {
+    std::string type;
+
+    switch(cand.type) {
+      case NrIceCandidate::ICE_HOST:
+        type = "host";
+        break;
+      case NrIceCandidate::ICE_SERVER_REFLEXIVE:
+        type = "srflx";
+        break;
+      case NrIceCandidate::ICE_PEER_REFLEXIVE:
+        type = "prflx";
+        break;
+      case NrIceCandidate::ICE_RELAYED:
+        type = "relay";
+        break;
+      default:
+        FAIL();
+    };
+
+    std::cerr << which
+              << " --> "
+              << type
+              << " "
+              << cand.host
+              << ":"
+              << cand.port
+              << std::endl;
+  }
+
+  void DumpAndCheckActiveCandidates() {
+    std::cerr << "Active candidates:" << std::endl;
+    for (size_t i=0; i < streams_.size(); ++i) {
+      for (int j=0; j < streams_[i]->components(); ++j) {
+        std::cerr << "Stream " << i << " component " << j+1 << std::endl;
+
+        NrIceCandidate *local;
+        NrIceCandidate *remote;
+
+        nsresult res = streams_[i]->GetActivePair(j+1, &local, &remote);
+        ASSERT_TRUE(NS_SUCCEEDED(res));
+        DumpCandidate("Local  ", *local);
+        ASSERT_EQ(expected_local_type_, local->type);
+        DumpCandidate("Remote ", *remote);
+        ASSERT_EQ(expected_remote_type_, remote->type);
+        delete local;
+        delete remote;
+      }
+    }
+  }
+
   void Close() {
     test_utils->sts_target()->Dispatch(
       WrapRunnable(ice_ctx_, &NrIceCtx::destroy_peer_ctx),
@@ -286,8 +344,8 @@ class IceTestPeer : public sigslot::has_slots<> {
   }
 
   void StreamReady(NrIceMediaStream *stream) {
-    std::cerr << "Stream ready " << stream->name() << std::endl;
     ++ready_ct_;
+    std::cerr << "Stream ready " << stream->name() << " ct=" << ready_ct_ << std::endl;
   }
 
   void IceCompleted(NrIceCtx *ctx) {
@@ -335,6 +393,8 @@ class IceTestPeer : public sigslot::has_slots<> {
   nsRefPtr<NrIceResolver> dns_resolver_;
   IceTestPeer *remote_;
   CandidateFilter candidate_filter_;
+  NrIceCandidate::Type expected_local_type_;
+  NrIceCandidate::Type expected_remote_type_;
 };
 
 class IceGatherTest : public ::testing::Test {
@@ -401,9 +461,11 @@ class IceConnectTest : public ::testing::Test {
     p2_->SetTurnServer(addr, port, username, password);
   }
 
-  void SetCandidateFilter(CandidateFilter filter) {
+  void SetCandidateFilter(CandidateFilter filter, bool both=true) {
     p1_->SetCandidateFilter(filter);
-    p2_->SetCandidateFilter(filter);
+    if (both) {
+      p2_->SetCandidateFilter(filter);
+    }
   }
 
   void Connect() {
@@ -412,8 +474,21 @@ class IceConnectTest : public ::testing::Test {
 
     ASSERT_TRUE_WAIT(p1_->ready_ct() == 1 && p2_->ready_ct() == 1, 5000);
     ASSERT_TRUE_WAIT(p1_->ice_complete() && p2_->ice_complete(), 5000);
+
+    p1_->DumpAndCheckActiveCandidates();
+    p2_->DumpAndCheckActiveCandidates();
   }
 
+  void SetExpectedTypes(NrIceCandidate::Type local, NrIceCandidate::Type remote) {
+    p1_->SetExpectedTypes(local, remote);
+    p2_->SetExpectedTypes(local, remote);
+  }
+
+  void SetExpectedTypes(NrIceCandidate::Type local1, NrIceCandidate::Type remote1,
+                        NrIceCandidate::Type local2, NrIceCandidate::Type remote2) {
+    p1_->SetExpectedTypes(local1, remote1);
+    p2_->SetExpectedTypes(local2, remote2);
+  }
 
   void ConnectP1(TrickleMode mode = TRICKLE_NONE) {
     p1_->Connect(p2_, mode);
@@ -700,6 +775,8 @@ TEST_F(IceConnectTest, TestConnectTurnOnly) {
                 g_turn_user, g_turn_password);
   ASSERT_TRUE(Gather(true));
   SetCandidateFilter(IsRelayCandidate);
+  SetExpectedTypes(NrIceCandidate::Type::ICE_RELAYED,
+                   NrIceCandidate::Type::ICE_RELAYED);
   Connect();
 }
 
@@ -712,6 +789,8 @@ TEST_F(IceConnectTest, TestSendReceiveTurnOnly) {
                 g_turn_user, g_turn_password);
   ASSERT_TRUE(Gather(true));
   SetCandidateFilter(IsRelayCandidate);
+  SetExpectedTypes(NrIceCandidate::Type::ICE_RELAYED,
+                   NrIceCandidate::Type::ICE_RELAYED);
   Connect();
   SendReceive();
 }

media/mtransport/third_party/nICEr/src/ice/ice_component.c

@@ -799,10 +799,13 @@ int nr_ice_component_nominated_pair(nr_ice_component *comp, nr_ice_cand_pair *pa
 
     r_log(LOG_ICE,LOG_DEBUG,"ICE-PEER(%s)/STREAM(%s)/comp(%d): cancelling all pairs but %s (0x%p)",comp->stream->pctx->label,comp->stream->label,comp->component_id,pair->as_string,pair);
 
-    /* OK, we need to cancel off everything on this component */
+    /* Cancel checks in WAITING and FROZEN per ICE S 8.1.2 */
     p2=TAILQ_FIRST(&comp->stream->check_list);
     while(p2){
-      if((p2 != pair) && (p2->remote->component->component_id == comp->component_id)){
+      if((p2 != pair) &&
+         (p2->remote->component->component_id == comp->component_id) &&
+         ((p2->state == NR_ICE_PAIR_STATE_FROZEN) ||
+	  (p2->state == NR_ICE_PAIR_STATE_WAITING))) {
         r_log(LOG_ICE,LOG_DEBUG,"ICE-PEER(%s)/STREAM(%s)/comp(%d): cancelling pair %s (0x%p)",comp->stream->pctx->label,comp->stream->label,comp->component_id,p2->as_string,p2);
 
         if(r=nr_ice_candidate_pair_cancel(pair->pctx,p2))

media/mtransport/third_party/nICEr/src/ice/ice_media_stream.c

@@ -295,7 +295,6 @@ int nr_ice_media_stream_service_pre_answer_requests(nr_ice_peer_ctx *pctx, nr_ic
     nr_ice_component *pcomp;
     int r,_status;
     char *user = 0;
-    char *lufrag, *rufrag;
 
     if (serviced)
       *serviced = 0;
@@ -698,7 +697,6 @@ int nr_ice_media_stream_find_component(nr_ice_media_stream *str, int comp_id, nr
     return(_status);
   }
 
-
 int nr_ice_media_stream_send(nr_ice_peer_ctx *pctx, nr_ice_media_stream *str, int component, UCHAR *data, int len)
   {
     int r,_status;
@@ -710,7 +708,7 @@ int nr_ice_media_stream_send(nr_ice_peer_ctx *pctx, nr_ice_media_stream *str, in
 
     /* Do we have an active pair yet? We should... */
     if(!comp->active)
-      ABORT(R_BAD_ARGS);
+      ABORT(R_NOT_FOUND);
 
     /* OK, write to that pair, which means:
        1. Use the socket on our local side.
@@ -718,7 +716,7 @@ int nr_ice_media_stream_send(nr_ice_peer_ctx *pctx, nr_ice_media_stream *str, in
     */
     comp->keepalive_needed=0; /* Suppress keepalives */
     if(r=nr_socket_sendto(comp->active->local->osock,data,len,0,
-      &comp->active->remote->addr))
+                          &comp->active->remote->addr))
       ABORT(r);
 
     _status=0;
@@ -726,6 +724,25 @@ int nr_ice_media_stream_send(nr_ice_peer_ctx *pctx, nr_ice_media_stream *str, in
     return(_status);
   }
 
+int nr_ice_media_stream_get_active(nr_ice_peer_ctx *pctx, nr_ice_media_stream *str, int component, nr_ice_candidate **local, nr_ice_candidate **remote)
+  {
+    int r,_status;
+    nr_ice_component *comp;
+
+    /* First find the peer component */
+    if(r=nr_ice_peer_ctx_find_component(pctx, str, component, &comp))
+      ABORT(r);
+
+    if(!comp->active)
+      ABORT(R_NOT_FOUND);
+
+    if (local) *local = comp->active->local;
+    if (remote) *remote = comp->active->remote;
+
+    _status=0;
+  abort:
+    return(_status);
+  }
 
 int nr_ice_media_stream_addrs(nr_ice_peer_ctx *pctx, nr_ice_media_stream *str, int component, nr_transport_addr *local, nr_transport_addr *remote)
   {

media/mtransport/third_party/nICEr/src/ice/ice_media_stream.h

@@ -88,7 +88,8 @@ int nr_ice_media_stream_component_nominated(nr_ice_media_stream *stream,nr_ice_c
 int nr_ice_media_stream_component_failed(nr_ice_media_stream *stream,nr_ice_component *component);
 int nr_ice_media_stream_set_state(nr_ice_media_stream *str, int state);
 int nr_ice_media_stream_get_best_candidate(nr_ice_media_stream *str, int component, nr_ice_candidate **candp);
-int nr_ice_media_stream_send(nr_ice_peer_ctx *pctx,nr_ice_media_stream *str, int component, UCHAR *data, int len);
+int nr_ice_media_stream_send(nr_ice_peer_ctx *pctx, nr_ice_media_stream *str, int component, UCHAR *data, int len);
+int nr_ice_media_stream_get_active(nr_ice_peer_ctx *pctx, nr_ice_media_stream *str, int component, nr_ice_candidate **local, nr_ice_candidate **remote);
 int nr_ice_media_stream_find_component(nr_ice_media_stream *str, int comp_id, nr_ice_component **compp);
 int nr_ice_media_stream_addrs(nr_ice_peer_ctx *pctx, nr_ice_media_stream *str, int component, nr_transport_addr *local, nr_transport_addr *remote);
 int