Rework early incoming STUN requests handling
Submitted by Fabrice Bellet
Assigned to Fabrice Bellet
Patches D1889, D1890, and D1891 propose some simplification in the way early incoming STUN requests are handled by the conncheck algorithm. The concerned STUN requests are those stored in the incoming_checks list by the priv_store_pending_check() function. The reason for these patches is that I suspect that most code from conn_check_remote_credentials_set() is now dead-code since the recent modifications of the conncheck method.
These checks are delayed when the remote credentials information has not been received yet (stream->remote_ufrag == 0), and are processed later when the credentials become available, in conn_check_remote_credentials_set(). The idea behind this delay is that it's pointless to create a pair, and submit a new conncheck when no creds are available. The delayed work consist to do what should have been done in priv_reply_to_conn_check().
Such a case typically happens when the peer start sending us STUN requests before: 1/ remote candidates have been received, 2/ remote credentials have been received.
This case requires fixes at several places :
When the remote candidates are received, we may have to update some of our candidates, peer-reflexive ones, that have been identified by in early incoming request as such. It may require to change their foundation (from peer-reflexive to host for example, and to recompute their priority, including the pairs that may contain theses candidates) : this is what the patch modifies in priv_add_remote_candidate(). Previously, duplicated pairs (same local and remote candidates) were created in this circumstance.
When the remote credentials are received, incoming_checks per component should be processed, leading to the creation of triggered checks (new pairs will be created by this function if needed). I think the logic of this function was previously broken, when iterating on stream->conncheck_list first, instead of processing the incoming_checks directly: we may have situations where the conncheck_list is still empty (no remote candidate yet), and incoming_checks is not.
If the remote candidates are received, and the remote credentials are not available yet, then pairs will be formed, and 'll be enqueued to be checked. They will fail (this is what fixes D1891), and we may consider this is a normal behavior for what constitutes an application logic failure.
I tested theses patches successfully with the janus-gateway streaming test and a chrome and firefox client, when local addresses and turn relay are both allowed. Typically, stun requests from the local address of the browser are sent to the janus-gateway before the list of remote candidates. The above logic works as expected in this order : incoming stun reqeuest received, pending check stored, remote credentials received, related pairs created and enqueued, remote candidates received and merged, related pairs updated (prio and foundation).
The new-dribble-test has also been updated with patch D1890, by splitting the credentials swap function in two halves, that can be called independently. This split allows to simulate the behavior observed with the janus-gateway.
I could not test these patches with other products, especially OCS 2007/2007 R2, so if other people could verify that these modifications don't break them, it would be great.