Bug 1131779: 403 on permission request no longer invalidates the whole TURN relay. r=bwc
authorNils Ohlmeier [:drno] <drno@ohlmeier.org>
Fri, 08 May 2015 11:59:52 -0700
changeset 274419 0cc4812a1cd79fb4dbd0c20360541d7ba416b813
parent 274418 4323557b111dd065048d2074f4e79ddeaac79678
child 274420 d732da9298fabae4350466e7817f634afa283cb9
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwc
bugs1131779
milestone40.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 1131779: 403 on permission request no longer invalidates the whole TURN relay. r=bwc
media/mtransport/test/turn_unittest.cpp
media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c
media/mtransport/third_party/nICEr/src/stun/turn_client_ctx.c
media/mtransport/third_party/nICEr/src/stun/turn_client_ctx.h
--- a/media/mtransport/test/turn_unittest.cpp
+++ b/media/mtransport/test/turn_unittest.cpp
@@ -55,16 +55,18 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 
 #include "mtransport_test_utils.h"
 #include "runnable_utils.h"
 
 #define GTEST_HAS_RTTI 0
 #include "gtest/gtest.h"
 #include "gtest_utils.h"
 
+#define USE_TURN
+
 // nICEr includes
 extern "C" {
 #include "nr_api.h"
 #include "registry.h"
 #include "async_timer.h"
 #include "r_crc32.h"
 #include "ice_util.h"
 #include "transport_addr.h"
@@ -206,26 +208,57 @@ class TurnClient : public ::testing::Tes
     relay_addr_ = addr.as_string;
 
     std::cerr << "Allocation succeeded with addr=" << relay_addr_ << std::endl;
   }
 
   void Deallocate_s() {
     ASSERT_TRUE(turn_ctx_);
 
+    std::cerr << "De-Allocating..." << std::endl;
     int r = nr_turn_client_deallocate(turn_ctx_);
     ASSERT_EQ(0, r);
   }
 
   void Deallocate() {
     RUN_ON_THREAD(test_utils->sts_target(),
                   WrapRunnable(this, &TurnClient::Deallocate_s),
                   NS_DISPATCH_SYNC);
   }
 
+  void RequestPermission_s(const std::string& target) {
+    nr_transport_addr addr;
+    int r;
+
+    // Expected pattern here is "IP4:127.0.0.1:3487"
+    ASSERT_EQ(0, target.compare(0, 4, "IP4:"));
+
+    size_t offset = target.rfind(':');
+    ASSERT_NE(std::string::npos, offset);
+
+    std::string host = target.substr(4, offset - 4);
+    std::string port = target.substr(offset + 1);
+
+    r = nr_ip4_str_port_to_transport_addr(host.c_str(),
+                                          atoi(port.c_str()),
+                                          IPPROTO_UDP,
+                                          &addr);
+    ASSERT_EQ(0, r);
+
+    r = nr_turn_client_ensure_perm(turn_ctx_, &addr);
+    ASSERT_EQ(0, r);
+  }
+
+  void RequestPermission(const std::string& target) {
+    RUN_ON_THREAD(test_utils->sts_target(),
+                  WrapRunnable(this, &TurnClient::RequestPermission_s, target),
+                  NS_DISPATCH_SYNC);
+
+  }
+
   void Readable(NR_SOCKET s, int how, void *arg) {
     // Re-arm
     std::cerr << "Socket is readable" << std::endl;
     NR_ASYNC_WAIT(s, how, socket_readable_cb, arg);
 
     UCHAR buf[8192];
     size_t len_s;
     nr_transport_addr addr;
@@ -273,52 +306,54 @@ class TurnClient : public ::testing::Tes
         std::cerr << "STUN message of unexpected type" << std::endl;
       } else {
         std::cerr << "Not a STUN message" << std::endl;
       }
       return;
     }
   }
 
-  void SendTo_s(const std::string& target, bool expect_success) {
+  void SendTo_s(const std::string& target, int expect_return) {
     nr_transport_addr addr;
     int r;
 
     // Expected pattern here is "IP4:127.0.0.1:3487"
     ASSERT_EQ(0, target.compare(0, 4, "IP4:"));
 
     size_t offset = target.rfind(':');
-    ASSERT_NE(offset, std::string::npos);
+    ASSERT_NE(std::string::npos, offset);
 
     std::string host = target.substr(4, offset - 4);
     std::string port = target.substr(offset + 1);
 
     r = nr_ip4_str_port_to_transport_addr(host.c_str(),
                                           atoi(port.c_str()),
                                           IPPROTO_UDP,
                                           &addr);
     ASSERT_EQ(0, r);
 
     unsigned char test[100];
     for (size_t i=0; i<sizeof(test); i++) {
       test[i] = i & 0xff;
     }
 
+    std::cerr << "Sending test message to " << target << " ..." << std::endl;
+
     r = nr_turn_client_send_indication(turn_ctx_,
                                             test, sizeof(test), 0,
                                             &addr);
-    if (expect_success) {
-      ASSERT_EQ(0, r);
+    if (expect_return >= 0) {
+      ASSERT_EQ(expect_return, r);
     }
   }
 
-  void SendTo(const std::string& target, bool expect_success=true) {
+  void SendTo(const std::string& target, int expect_return=0) {
     RUN_ON_THREAD(test_utils->sts_target(),
                   WrapRunnable(this, &TurnClient::SendTo_s, target,
-                               expect_success),
+                               expect_return),
                   NS_DISPATCH_SYNC);
   }
 
   int received() const { return received_; }
 
   static void socket_readable_cb(NR_SOCKET s, int how, void *arg) {
     static_cast<TurnClient *>(arg)->Readable(s, how, arg);
   }
@@ -368,39 +403,62 @@ TEST_F(TurnClient, SendToSelfTcp) {
   SetTcp();
   Allocate();
   SendTo(relay_addr_);
   ASSERT_TRUE_WAIT(received() == 100, 5000);
   SendTo(relay_addr_);
   ASSERT_TRUE_WAIT(received() == 200, 1000);
 }
 
+TEST_F(TurnClient, PermissionDenied) {
+  Allocate();
+  RequestPermission(relay_addr_);
+  PR_Sleep(1000);
+
+  /* Fake a 403 response */
+  nr_turn_permission *perm;
+  perm = STAILQ_FIRST(&turn_ctx_->permissions);
+  ASSERT_TRUE(perm);
+  while (perm) {
+    perm->stun->last_error_code = 403;
+    std::cerr << "Set 403's on permission" << std::endl;
+    perm = STAILQ_NEXT(perm, entry);
+  }
+
+  SendTo(relay_addr_, R_NOT_PERMITTED);
+  ASSERT_TRUE(received() == 0);
+
+  //TODO: We should check if we can still send to a second destination, but
+  //      we would need a second TURN client as one client can only handle one
+  //      allocation (maybe as part of bug 1128128 ?).
+}
+
 TEST_F(TurnClient, DeallocateReceiveFailure) {
   Allocate();
   SendTo(relay_addr_);
   ASSERT_TRUE_WAIT(received() == 100, 5000);
   Deallocate();
   turn_ctx_->state = NR_TURN_CLIENT_STATE_ALLOCATED;
-  SendTo(relay_addr_, true);
+  SendTo(relay_addr_);
   PR_Sleep(1000);
   ASSERT_TRUE(received() == 100);
 }
 
 TEST_F(TurnClient, DeallocateReceiveFailureTcp) {
   SetTcp();
   Allocate();
   SendTo(relay_addr_);
   ASSERT_TRUE_WAIT(received() == 100, 5000);
   Deallocate();
   turn_ctx_->state = NR_TURN_CLIENT_STATE_ALLOCATED;
   /* Either the connection got closed by the TURN server already, then the send
    * is going to fail, which we simply ignore. Or the connection is still alive
    * and we cand send the data, but it should not get forwarded to us. In either
    * case we should not receive more data. */
-  SendTo(relay_addr_, false);
+  SendTo(relay_addr_, -1);
   PR_Sleep(1000);
   ASSERT_TRUE(received() == 100);
 }
 
 TEST_F(TurnClient, AllocateDummyServer) {
   turn_server_ = kDummyTurnServer;
   Allocate(false);
 }
--- a/media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_candidate_pair.c
@@ -372,17 +372,16 @@ static void nr_ice_candidate_pair_restar
       NR_ASYNC_TIMER_SET(0,nr_ice_candidate_pair_stun_cb,pair, &pair->stun_cb_timer);
       _status=0;
     }
   }
 
 int nr_ice_candidate_pair_start(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair)
   {
     int r,_status;
-    UINT4 mode;
 
     /* Register the stun ctx for when responses come in*/
     if(r=nr_ice_socket_register_stun_client(pair->local->isock,pair->stun_client,&pair->stun_client_handle))
       ABORT(r);
 
     nr_ice_candidate_pair_restart(pctx, pair);
 
     _status=0;
--- a/media/mtransport/third_party/nICEr/src/stun/turn_client_ctx.c
+++ b/media/mtransport/third_party/nICEr/src/stun/turn_client_ctx.c
@@ -85,18 +85,16 @@ static int nr_turn_client_start_refresh_
                                               UINT4 lifetime);
 static int nr_turn_permission_create(nr_turn_client_ctx *ctx,
                                      nr_transport_addr *addr,
                                      nr_turn_permission **permp);
 static int nr_turn_permission_find(nr_turn_client_ctx *ctx,
                                    nr_transport_addr *addr,
                                    nr_turn_permission **permp);
 static int nr_turn_permission_destroy(nr_turn_permission **permp);
-static int nr_turn_client_ensure_perm(nr_turn_client_ctx *ctx,
-                                      nr_transport_addr *addr);
 static void nr_turn_client_refresh_cb(NR_SOCKET s, int how, void *arg);
 static void nr_turn_client_permissions_cb(NR_SOCKET s, int how, void *cb);
 static int nr_turn_client_send_stun_request(nr_turn_client_ctx *ctx,
                                             nr_stun_message *req,
                                             int flags);
 
 
 /* nr_turn_stun_ctx functions */
@@ -126,16 +124,17 @@ static int nr_turn_stun_ctx_create(nr_tu
   sctx->stun->auth_params.username=tctx->username;
   INIT_DATA(sctx->stun->auth_params.password,
             tctx->password->data, tctx->password->len);
 
   sctx->tctx=tctx;
   sctx->success_cb=success_cb;
   sctx->error_cb=error_cb;
   sctx->mode=mode;
+  sctx->last_error_code=0;
 
   /* Add ourselves to the tctx's list */
   STAILQ_INSERT_TAIL(&tctx->stun_ctxs, sctx, entry);
   *ctxp=sctx;
 
   _status=0;
 abort:
   if (_status) {
@@ -202,17 +201,17 @@ abort:
 
 static int nr_turn_stun_ctx_start(nr_turn_stun_ctx *ctx)
 {
   int r, _status;
   nr_turn_client_ctx *tctx = ctx->tctx;
 
   if ((r=nr_stun_client_reset(ctx->stun))) {
     r_log(NR_LOG_TURN, LOG_ERR, "TURN(%s): Couldn't reset STUN",
-          ctx->tctx->label);
+          tctx->label);
     ABORT(r);
   }
 
   if ((r=nr_stun_client_start(ctx->stun, ctx->mode, nr_turn_stun_ctx_cb, ctx))) {
     r_log(NR_LOG_TURN, LOG_ERR, "TURN(%s): Couldn't start STUN",
           tctx->label);
     ABORT(r);
   }
@@ -222,16 +221,18 @@ abort:
   return _status;
 }
 
 static void nr_turn_stun_ctx_cb(NR_SOCKET s, int how, void *arg)
 {
   int r, _status;
   nr_turn_stun_ctx *ctx = (nr_turn_stun_ctx *)arg;
 
+  ctx->last_error_code = ctx->stun->error_code;
+
   switch (ctx->stun->state) {
     case NR_STUN_CLIENT_STATE_DONE:
       /* Save the realm and nonce */
       if (ctx->stun->realm && (!ctx->tctx->realm || strcmp(ctx->stun->realm,
                                                            ctx->tctx->realm))) {
         RFREE(ctx->tctx->realm);
         ctx->tctx->realm = r_strdup(ctx->stun->realm);
         if (!ctx->tctx->realm)
@@ -592,16 +593,29 @@ static void nr_turn_client_error_cb(NR_S
   nr_turn_stun_ctx *ctx = (nr_turn_stun_ctx *)arg;
 
   r_log(NR_LOG_TURN, LOG_WARNING, "TURN(%s): mode %d, %s",
         ctx->tctx->label, ctx->mode, __FUNCTION__);
 
   nr_turn_client_failed(ctx->tctx);
 }
 
+static void nr_turn_client_permission_error_cb(NR_SOCKET s, int how, void *arg)
+{
+  nr_turn_stun_ctx *ctx = (nr_turn_stun_ctx *)arg;
+
+  if (ctx->last_error_code == 403) {
+    r_log(NR_LOG_TURN, LOG_WARNING, "TURN(%s): mode %d, permission denied",
+          ctx->tctx->label, ctx->mode);
+
+  } else{
+    nr_turn_client_error_cb(0, 0, ctx);
+  }
+}
+
 int nr_turn_client_allocate(nr_turn_client_ctx *ctx,
                             NR_async_cb finished_cb, void *cb_arg)
 {
   nr_turn_stun_ctx *stun = 0;
   int r,_status;
 
   assert(ctx->state == NR_TURN_CLIENT_STATE_INITTED);
 
@@ -884,17 +898,17 @@ abort:
      along with when they were last established.
    - Whenever someone sends a packet, we automatically create/
      refresh the permission.
 
    This means that permissions automatically time out if
    unused.
 
 */
-static int nr_turn_client_ensure_perm(nr_turn_client_ctx *ctx, nr_transport_addr *addr)
+int nr_turn_client_ensure_perm(nr_turn_client_ctx *ctx, nr_transport_addr *addr)
 {
   int r, _status;
   nr_turn_permission *perm = 0;
   UINT8 now;
   UINT8 turn_permission_refresh = (TURN_PERMISSION_LIFETIME_SECONDS -
                                    TURN_REFRESH_SLACK_SECONDS) * TURN_USECS_PER_S;
 
   if ((r=nr_turn_permission_find(ctx, addr, &perm))) {
@@ -944,17 +958,17 @@ static int nr_turn_permission_create(nr_
 
   if ((r=nr_transport_addr_copy(&perm->addr, addr)))
     ABORT(r);
 
   perm->last_used = 0;
 
   if ((r=nr_turn_stun_ctx_create(ctx, NR_TURN_CLIENT_MODE_PERMISSION_REQUEST,
                                  nr_turn_client_permissions_cb,
-                                 nr_turn_client_error_cb,
+                                 nr_turn_client_permission_error_cb,
                                  &perm->stun)))
     ABORT(r);
 
   /* We want to authenticate on the first packet */
   if ((r=nr_turn_stun_set_auth_params(perm->stun, ctx->realm, ctx->nonce)))
     ABORT(r);
 
   if ((r=nr_transport_addr_copy(
@@ -986,16 +1000,19 @@ static int nr_turn_permission_find(nr_tu
       break;
 
     perm = STAILQ_NEXT(perm, entry);
   }
 
   if (!perm) {
     ABORT(R_NOT_FOUND);
   }
+  if (perm->stun->last_error_code == 403) {
+    ABORT(R_NOT_PERMITTED);
+  }
   *permp = perm;
 
   _status=0;
 abort:
   return(_status);
 }
 
 static void nr_turn_client_permissions_cb(NR_SOCKET s, int how, void *arg)
--- a/media/mtransport/third_party/nICEr/src/stun/turn_client_ctx.h
+++ b/media/mtransport/third_party/nICEr/src/stun/turn_client_ctx.h
@@ -47,16 +47,17 @@ typedef struct nr_turn_stun_ctx_ {
   struct nr_turn_client_ctx_ *tctx;
   int mode;  /* From stun_client_ctx.h, NR_TURN_CLIENT_MODE_* */
   int retry_ct;
   nr_stun_client_ctx *stun;
   char *nonce;
   char *realm;
   NR_async_cb success_cb;
   NR_async_cb error_cb;
+  int last_error_code;
 
   STAILQ_ENTRY(nr_turn_stun_ctx_) entry;
 } nr_turn_stun_ctx;
 typedef STAILQ_HEAD(nr_turn_stun_ctx_head_, nr_turn_stun_ctx_)
     nr_turn_stun_ctx_head;
 
 /* Represents a single TURN permission */
 typedef struct nr_turn_permission_ {
@@ -124,10 +125,12 @@ int nr_turn_client_send_indication(nr_tu
                                    const UCHAR *msg, size_t len,
                                    int flags, nr_transport_addr *remote_addr);
 int nr_turn_client_parse_data_indication(nr_turn_client_ctx *ctx,
                                          nr_transport_addr *source_addr,
                                          UCHAR *msg, size_t len,
                                          UCHAR *newmsg, size_t *newlen,
                                          size_t newsize,
                                          nr_transport_addr *remote_addr);
+int nr_turn_client_ensure_perm(nr_turn_client_ctx *ctx,
+                               nr_transport_addr *addr);
 #endif