Bug 888274: Emit RFC5389 STUN. Accept MAPPED-ADDRESS in response regardless of what we send. r=abr a=bajaj on a CLOSED TREE
authorEKR <ekr@rtfm.com>
Fri, 28 Jun 2013 07:58:37 -0700
changeset 148386 cadb54f63c80
parent 148385 5954b0946800
child 148387 667742155902
child 148389 995480afdf3e
child 148393 3dbd75529f99
push id2776
push userrjesup@wgate.com
push dateMon, 19 Aug 2013 20:31:24 +0000
treeherdermozilla-beta@cadb54f63c80 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersabr, bajaj
bugs888274
milestone24.0
Bug 888274: Emit RFC5389 STUN. Accept MAPPED-ADDRESS in response regardless of what we send. r=abr a=bajaj on a CLOSED TREE
media/mtransport/third_party/nICEr/src/ice/ice_candidate.c
media/mtransport/third_party/nICEr/src/stun/stun_client_ctx.c
media/mtransport/third_party/nICEr/src/stun/stun_proc.c
--- a/media/mtransport/third_party/nICEr/src/ice/ice_candidate.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_candidate.c
@@ -546,17 +546,17 @@ static void nr_ice_srvrflx_start_stun_ti
     nr_ice_candidate *cand=cb_arg;
     int r,_status;
 
     cand->delay_timer=0;
 
 /* TODO: if the response is a BINDING-ERROR-RESPONSE, then restart
  * TODO: using NR_STUN_CLIENT_MODE_BINDING_REQUEST because the
  * TODO: server may not have understood the 0.96-style request */
-    if(r=nr_stun_client_start(cand->u.srvrflx.stun, NR_STUN_CLIENT_MODE_BINDING_REQUEST_STUND_0_96, nr_ice_srvrflx_stun_finished_cb, cand))
+    if(r=nr_stun_client_start(cand->u.srvrflx.stun, NR_STUN_CLIENT_MODE_BINDING_REQUEST_NO_AUTH, nr_ice_srvrflx_stun_finished_cb, cand))
       ABORT(r);
 
     if(r=nr_ice_ctx_remember_id(cand->ctx, cand->u.srvrflx.stun->request))
       ABORT(r);
 
     if(r=nr_ice_socket_register_stun_client(cand->isock,cand->u.srvrflx.stun,&cand->u.srvrflx.stun_handle))
       ABORT(r);
 
@@ -651,17 +651,17 @@ static void nr_ice_srvrflx_stun_finished
       nr_ice_socket_deregister(cand->isock,cand->u.srvrflx.stun_handle);
       cand->u.srvrflx.stun_handle=0;
     }
 
     switch(cand->u.srvrflx.stun->state){
       /* OK, we should have a mapped address */
       case NR_STUN_CLIENT_STATE_DONE:
         /* Copy the address */
-        nr_transport_addr_copy(&cand->addr, &cand->u.srvrflx.stun->results.stun_binding_response_stund_0_96.mapped_addr);
+        nr_transport_addr_copy(&cand->addr, &cand->u.srvrflx.stun->results.stun_binding_response.mapped_addr);
         nr_stun_client_ctx_destroy(&cand->u.srvrflx.stun);
         cand->state=NR_ICE_CAND_STATE_INITIALIZED;
         /* Execute the ready callback */
         cand->done_cb(0,0,cand->cb_arg);
         break;
 
       /* This failed, so go to the next STUN server if there is one */
       case NR_STUN_CLIENT_STATE_FAILED:
--- a/media/mtransport/third_party/nICEr/src/stun/stun_client_ctx.c
+++ b/media/mtransport/third_party/nICEr/src/stun/stun_client_ctx.c
@@ -568,18 +568,25 @@ int nr_stun_client_process_response(nr_s
             ABORT(R_BAD_DATA);
         if (! nr_stun_message_has_attribute(ctx->response, NR_STUN_ATTR_MESSAGE_INTEGRITY, 0))
             ABORT(R_BAD_DATA);
 
         mapped_addr = &ctx->results.stun_binding_response.mapped_addr;
         break;
 
     case NR_STUN_CLIENT_MODE_BINDING_REQUEST_NO_AUTH:
-        if (! nr_stun_message_has_attribute(ctx->response, NR_STUN_ATTR_XOR_MAPPED_ADDRESS, 0))
-            ABORT(R_BAD_DATA);
+        if (! nr_stun_message_has_attribute(ctx->response, NR_STUN_ATTR_XOR_MAPPED_ADDRESS, 0)) {
+            if (nr_stun_message_has_attribute(ctx->response, NR_STUN_ATTR_MAPPED_ADDRESS, 0)) {
+              /* Compensate for a bug in Google's STUN servers where they always respond with MAPPED-ADDRESS */
+              r_log(NR_LOG_STUN,LOG_DEBUG,"STUN-CLIENT(%s): No XOR-MAPPED-ADDRESS but MAPPED-ADDRESS. Falling back (though server is wrong).", ctx->label);
+            }
+            else {
+              ABORT(R_BAD_DATA);
+            }
+        }
 
         mapped_addr = &ctx->results.stun_binding_response.mapped_addr;
         break;
 
     case NR_STUN_CLIENT_MODE_BINDING_REQUEST_STUND_0_96:
         if (! nr_stun_message_has_attribute(ctx->response, NR_STUN_ATTR_MAPPED_ADDRESS, 0) && ! nr_stun_message_has_attribute(ctx->response, NR_STUN_ATTR_XOR_MAPPED_ADDRESS, 0))
             ABORT(R_BAD_DATA);
 
--- a/media/mtransport/third_party/nICEr/src/stun/stun_proc.c
+++ b/media/mtransport/third_party/nICEr/src/stun/stun_proc.c
@@ -194,30 +194,36 @@ nr_stun_process_indication(nr_stun_messa
 
     _status=0;
 #ifdef USE_STUN_PEDANTIC
   abort:
 #endif /* USE_STUN_PEDANTIC */
     return _status;
 }
 
-/* draft-ietf-behave-rfc3489bis-10.txt S 7.3.3 */
+/* RFC5389 S 7.3.3, except that we *also* allow a MAPPED_ADDRESS
+   to compensate for a bug in Google's STUN server where it
+   always returns MAPPED_ADDRESS.
+
+   Mozilla bug: 888274.
+ */
 int
 nr_stun_process_success_response(nr_stun_message *res)
 {
     int _status;
 
 #ifdef USE_STUN_PEDANTIC
     if (res->comprehension_required_unknown_attributes > 0)
         ABORT(R_REJECTED);
 #endif /* USE_STUN_PEDANTIC */
 
     if (NR_STUN_GET_TYPE_METHOD(res->header.type) == NR_METHOD_BINDING) {
-        if (! nr_stun_message_has_attribute(res, NR_STUN_ATTR_XOR_MAPPED_ADDRESS, 0)) {
-            r_log(NR_LOG_STUN, LOG_ERR, "Missing XOR-MAPPED-ADDRESS");
+        if (! nr_stun_message_has_attribute(res, NR_STUN_ATTR_XOR_MAPPED_ADDRESS, 0) &&
+            ! nr_stun_message_has_attribute(res, NR_STUN_ATTR_MAPPED_ADDRESS, 0)) {
+            r_log(NR_LOG_STUN, LOG_ERR, "Missing XOR-MAPPED-ADDRESS and MAPPED_ADDRESS");
             ABORT(R_REJECTED);
         }
     }
 
     _status=0;
  abort:
     return _status;
 }