Bug 888274. Emit RFC5389 STUN. Accept MAPPED-ADDRESS in response regardless of what we send. r=abr
authorEKR <ekr@rtfm.com>
Fri, 28 Jun 2013 07:58:37 -0700
changeset 141348 2e3cdffc3e65
parent 141347 ec6bcb45443c
child 141349 4951cadc9980
push id25054
push usercbook@mozilla.com
push dateMon, 05 Aug 2013 09:19:53 +0000
treeherdermozilla-central@54434a926c5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersabr
bugs888274
milestone25.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 888274. Emit RFC5389 STUN. Accept MAPPED-ADDRESS in response regardless of what we send. r=abr
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;
 }