Olivier Crête [Tue, 31 May 2016 21:31:12 +0000 (17:31 -0400)]
WIP
Jakub Adam [Tue, 31 May 2016 09:27:03 +0000 (09:27 +0000)]
conncheck: fix pruning conn checks with TCP active sockets
TCP active socket makes a NiceSocket for each peer in conn_check_send()
and this new socket is then stored as CandidateCheckPair's 'sockptr'.
We thus have to look also at the 'sockptr' value when eliminating
sockets which have received HUP from connection checks.
Differential Revision: https://phabricator.freedesktop.org/D1034
Olivier Crête [Tue, 8 Mar 2016 20:37:05 +0000 (15:37 -0500)]
agent: Remove socket on read error
If a socket returned an error, remove it.
Olivier Crête [Sat, 16 Jan 2016 03:40:27 +0000 (22:40 -0500)]
component: Add API to cleanly remove a base socket
Olivier Crête [Sat, 27 Feb 2016 08:35:27 +0000 (03:35 -0500)]
agent: Fix udp-turn-over-tcp
The TCP-based turns don't come pre-parsed unlike
the UDP variants!
Olivier Crête [Wed, 27 Jan 2016 23:56:13 +0000 (18:56 -0500)]
agent: Add force-relay property to force messages through the relay
This allows implementing WebRTC privacy mode.
Olivier Crête [Wed, 10 Feb 2016 21:29:57 +0000 (16:29 -0500)]
conncheck: Start conncheck on server reply if needed
This only really applies in the force relay mode where there are
no local candidates.
Olivier Crête [Mon, 8 Feb 2016 21:44:47 +0000 (16:44 -0500)]
Replace g_malloc/g_new with g_alloca where possible
This should reduce the overhead a bit.
Fabrice Bellet [Wed, 20 Apr 2016 10:17:05 +0000 (10:17 +0000)]
conncheck: explain some corner cases
This patch give details why some exceptions to the ICE spec are needed.
Differential Revision: https://phabricator.freedesktop.org/D876
Fabrice Bellet [Fri, 27 May 2016 23:15:39 +0000 (19:15 -0400)]
conncheck: add a debug dump of the whole stream check list
https://phabricator.freedesktop.org/D814
Olivier Crête [Fri, 27 May 2016 22:50:59 +0000 (18:50 -0400)]
conncheck: fix the replay of early incoming connchecks
This patch fixes a bug in the way the list of incoming checks
is handled. The code purges this list too early, before all ichecks
for a given component are processed. It happens because the component
is computed from each pair of the check list, instead of being passed
as a fixed parameter of the function.
Differential Revision: https://phabricator.freedesktop.org/D882
Fabrice Bellet [Wed, 20 Apr 2016 09:23:14 +0000 (09:23 +0000)]
stun: fix ice role conflict handling
This patch fixes the role conflict handling in stun ICE usage,
according to RFC 5245, by adding including missing cases in the
test. The role switch not only depends of the comparison of the
stun ice-controlling/controlled attrib with the agent tie breaker
value, but it also depends on the current role of the agent.
This patch also changes the value returned by
stun_usage_ice_conncheck_create_reply() when a role conflict exists
but doesn't change the role of the agent, causing an error stun
response. Previously, this case could not be differenciated by the
caller from a case with no role conflict. Now by examinating the
return value, and whether the control param changed, the caller
can check the four possibles situations. The stun test suite is
updated to match this change.
Differential Revision: https://phabricator.freedesktop.org/D873
Olivier Crête [Fri, 27 May 2016 21:26:06 +0000 (17:26 -0400)]
conncheck: Make previous commit compile
https://phabricator.freedesktop.org/T3324
Olivier Crête [Mon, 8 Feb 2016 23:49:42 +0000 (18:49 -0500)]
discovery: Make sure each candidate has a unique priority
This should fix compliance with RFC 5245 Section 4.1.2
https://phabricator.freedesktop.org/T3324
Olivier Crête [Sun, 20 Sep 2015 20:53:26 +0000 (16:53 -0400)]
agent: Restrict transitions to gathering
Only allow transitions to gathering from disconnected or
failed states.
Olivier Crête [Fri, 27 May 2016 18:06:24 +0000 (14:06 -0400)]
conncheck: fix TCP active relay handling
TCP active relay candidates use UDP TURN for their underlying socket.
Since
0a6c779f1f, socket->fileno of UDP TURN sockets is always NULL,
which caused a wrong code path to be chosen in conn_check_send().
We have to update the if-expression accordingly.
Maniphest Tasks: T7442
Differential Revision: https://phabricator.freedesktop.org/D1017
Olivier Crête [Fri, 27 May 2016 15:29:22 +0000 (11:29 -0400)]
agent: ignore gathering failures on auto-generated IPs
Candidate gathering is stopped when discovery_add_local_host_candidate()
returns HOST_CANDIDATE_CANT_CREATE_SOCKET. This may be too radical
a measure when other local addresses can still be able to generate
usable candidates.
The issue was observed by a user who had an IPv6 address with tentative
flag on one of the interfaces. That single failing address was causing
the whole gathering process to end with no candidates found.
Still, don't do this if nice_agent_add_local_address() has been called.
In that case, the user really cares about the addresses and if there's
any problem, the process should fail.
https://phabricator.freedesktop.org/D1016
Olivier Crête [Tue, 1 Mar 2016 20:27:46 +0000 (15:27 -0500)]
candidate: Give lower priority to TCP relayed candidates
Olivier Crête [Mon, 29 Feb 2016 21:11:18 +0000 (16:11 -0500)]
udp-turn: Fix binding timeout leak
Olivier Crête [Thu, 25 Feb 2016 03:53:08 +0000 (22:53 -0500)]
conncheck: Update selected pair if necessary
Olivier Crête [Tue, 23 Feb 2016 00:36:58 +0000 (19:36 -0500)]
conncheck: Don't reset keepalive timer on next keepalive
If the keepalive is still being re-send, just let the retries do their
job. If they don't get a reply, then declare the attempt failed.
Olivier Crête [Thu, 26 May 2016 20:05:36 +0000 (16:05 -0400)]
conncheck: Separate valid and succeded states
RFC 5245 specifies that when a mapped-address differs from the address
from the request was sent, the mapped-address is used to select the
valid pair, but the source address of the check is used to select the
pair that succeeded, so they are not the same.
Olivier Crête [Sat, 20 Feb 2016 01:47:08 +0000 (20:47 -0500)]
udp-turn: Don't expose GSocket
UDP turn sockets should never be read frm directly.
Because they may share the same socket with the non-relay,
so the incoming data may not be relayed and then the NiceSocket
API doesn't allow returning the base socket as the source.
Olivier Crête [Thu, 18 Feb 2016 19:20:52 +0000 (14:20 -0500)]
conncheck: Make very frequent debug verbose-only
Olivier Crête [Tue, 16 Feb 2016 00:09:58 +0000 (19:09 -0500)]
debug: Enable based on G_MESSAGES_DEBUG
Olivier Crête [Fri, 12 Feb 2016 06:01:37 +0000 (01:01 -0500)]
agent: Don't emit signal in the middle of recv call
Olivier Crête [Thu, 11 Feb 2016 00:38:52 +0000 (19:38 -0500)]
agent: Update type of peer-reflexive candidate on trickled candidate
If a remote candidate matches an already discovered peer-reflexive candidate,
then the type can be updated to the real type and the foundation
can be set correctly.
Olivier Crête [Tue, 9 Feb 2016 17:52:45 +0000 (12:52 -0500)]
agent: Only try to use the address of the same family to connect to TURN
Using a IPv6 local address to connect to a IPv4 relay just creates an
extra discovery attempt that will not provide something useful.
Olivier Crête [Mon, 8 Feb 2016 00:48:07 +0000 (19:48 -0500)]
stun turn usage: Only send the username if short term creds or nonce present
This is recommended by the STUN RFC 5389.
Olivier Crête [Mon, 8 Feb 2016 00:41:52 +0000 (19:41 -0500)]
turn: Cache the nonce & realm to remove useless round trips
Instead of re-discovering the nonce and realm for every request, cache them
in th socket.
Olivier Crête [Tue, 9 Feb 2016 16:18:30 +0000 (11:18 -0500)]
conncheck: Stay READY if a new nominated pairs comes in
Olivier Crête [Tue, 9 Feb 2016 02:04:24 +0000 (21:04 -0500)]
conncheck: Deduplicate conncheck stopping code
Olivier Crête [Tue, 9 Feb 2016 00:41:28 +0000 (19:41 -0500)]
Reset to connecting if reconnected after failed
Olivier Crête [Thu, 14 Jan 2016 22:59:16 +0000 (17:59 -0500)]
agent: Add warning on ignored result
Fabrice Bellet [Mon, 4 Apr 2016 22:02:52 +0000 (23:02 +0100)]
conncheck: display controlling mode of stun requests
This patch makes the debug log more explicit about the agent
controlling role for each stun request sent. It helps to debug
role conflict resolution.
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Differential Revision: https://phabricator.freedesktop.org/D877
Fabrice Bellet [Mon, 4 Apr 2016 21:38:07 +0000 (22:38 +0100)]
agent: remove newline from debug output
Just a cosmetic fix.
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Differential Revision: https://phabricator.freedesktop.org/D872
Jakub Adam [Mon, 4 Apr 2016 20:52:29 +0000 (21:52 +0100)]
socket: refactor nice_socket_is_base_of()
• rename to nice_socket_is_based_on() and swap the order of arguments
accordingly; the implementation doesn't have to use the confusing
'return other->is_base_of()' pattern anymore
• fix potential NULL dereferences
The argument order in agent_recv_message_unlocked() was already wrongly
swapped in
1732c7d6 and thus this commit isn't changing it back because
that order has become the correct one.
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Differential Revision: https://phabricator.freedesktop.org/D866
Jakub Adam [Mon, 4 Apr 2016 20:46:05 +0000 (21:46 +0100)]
socket: fix wrong function called in nice_socket_is_base_of()
We have to call is_base_of "virtual function pointer" of 'other'
object, not 'sock', since 'other' is the structure whose base
NiceSocket we need to get from its private data.
For instance calling nice_socket_is_base_of() with 'sock' and 'other'
being respectively pseudo-SSL and UDP-TURN-over-TCP invoked is_base_of
variant for pseudo-SSL, casting other->priv into PseudoSSLPriv *, but
other->priv is actually TurnTcpPriv *. It must be called the other way
around.
https://phabricator.freedesktop.org/T7335
https://phabricator.freedesktop.org/T7336
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Reviewed-by: José Antonio Santos Cadenas <santoscadenas@gmail.com>
Reviewed-by: Philip Withnall <philip@tecnocode.co.uk>
Reviewed-by: José Antonio Santos Cadenas <santoscadenas@gmail.com>
Differential Revision: https://phabricator.freedesktop.org/D785
Fabrice Bellet [Mon, 4 Apr 2016 20:38:59 +0000 (21:38 +0100)]
conncheck: implement a "triggered queue" list
The checks should not be sent immediately in priv_conn_check_initiate(),
but be put into the "triggered queue", see "7.2.1.4 Triggered Checks".
This patch implements this triggered checks list, and uses it to enforce a
pacing of STUN transactions, no more than one per Ta ms, according to
"B.1. Pacing of STUN Transactions".
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Reviewed-by: Philip Withnall <philip@tecnocode.co.uk>
Differential Revision: https://phabricator.freedesktop.org/D802
Olivier Crête [Sun, 6 Mar 2016 20:16:18 +0000 (15:16 -0500)]
agent: Fix argument order
Fixes crash reported on https://phabricator.freedesktop.org/D786
Fabrice Bellet [Sat, 5 Mar 2016 18:46:48 +0000 (18:46 +0000)]
ice: fix the debug of the presence of the controlling/controlled attrib
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D807
Jakub Adam [Wed, 2 Mar 2016 00:01:19 +0000 (00:01 +0000)]
agent: fix relay candidate discovery on hosts having several IPs
When a message is received from a TURN server and we manage to find a
local relay candidate with matching stream and component IDs, we should
also check whether the message came from the candidate's respective
socket.
We should do this because there might still be some pending TURN
candidate discovery with the same server from a different local host IP
and the message may be a response to our allocate request. If
nice_udp_turn_socket_parse_recv_message() is passed such request, it can
make some wrong assumptions and modify it like in the case of reliable
UDP-TURN-OVER-TCP by removing (supposed) RFC4571 framing, which in turn
causes the reply to be unrecognized and discarded.
Because of this, any subsequent replies following the first successful
allocate response from that server couldn't create any additional relay
candidates.
Maniphest Tasks: https://phabricator.freedesktop.org/T7336
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D786
Philip Withnall [Tue, 1 Mar 2016 23:50:11 +0000 (23:50 +0000)]
build: Update autogen.sh from GNOME template
https://wiki.gnome.org/Projects/GnomeCommon/Migration#autogen.sh
Olivier Crête [Tue, 1 Mar 2016 23:29:10 +0000 (23:29 +0000)]
component.c: Fix memory leak
If nicesocket is not added to a component it will be leaked.
This is the case of active tcp sockets
Change-Id: I57fefffef71d35ce9871139ee1064181f6fe125b
Reviewed-by: José Antonio Santos Cadenas <santoscadenas@gmail.com>
Differential Revision: https://phabricator.freedesktop.org/D822
Philip Withnall [Tue, 1 Mar 2016 23:23:14 +0000 (23:23 +0000)]
agent: Use provided CandidatePair rather than re-finding a pair
In priv_update_selected_pair(), commit
57393333 changed the code to
re-find a CandidatePair matching the given lfoundation and rfoundation.
However, the foundation does not uniquely identify candidate pairs,
and if we’re aiming to set a specific candidate pair as the selected
pair, this could result in the wrong pair being selected.
This can happen when handling multiple similar candidate pairs, such as
when generating peer reflexive candidates from multiple sources.
See https://tools.ietf.org/html/rfc5245#section-2.4.
Originally spotted by Fabrice Bellet in
https://phabricator.freedesktop.org/T3557.
Reviewed-by: José Antonio Santos Cadenas <santoscadenas@gmail.com>
Differential Revision: https://phabricator.freedesktop.org/D742
Philip Withnall [Tue, 1 Mar 2016 23:05:20 +0000 (23:05 +0000)]
simple-example: transmission can begin earlier than in ready state
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D819
Philip Withnall [Tue, 1 Mar 2016 23:04:14 +0000 (23:04 +0000)]
test-new-dribble: wait until ragent reaches state completed
The test didn't let enough time for ragent to reach the completed state
after obtaining its remote candidates and switching to connecting state.
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D817
Philip Withnall [Tue, 1 Mar 2016 23:02:52 +0000 (23:02 +0000)]
conncheck: reorder the connection list when priorities are updated
The update of pairs priorities due to agent role change requires the
conncheck list to be reordered to reflect this modification.
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D806
Philip Withnall [Tue, 1 Mar 2016 23:01:14 +0000 (23:01 +0000)]
conncheck: fix keepalive stun agent initialisation
With this patch, we send keepalive binding requests using agent
compatibility flags, instead of RFC 3489 classic stun. The peer stun
agent will known how to handle it, and won't be confused by the
uncompatible RFC 3489 message, causing "no cookie" errors in the debug
log.
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D804
Philip Withnall [Tue, 1 Mar 2016 22:58:15 +0000 (22:58 +0000)]
conncheck: foundations are shared across streams
This patch fixes a bug where the foundation definition shouldn't take
into account the stream the pair belongs to. This is important, because
the ordinary checks algorithm will change pair state from Frozen to
Waiting, by selecting pairs from other streams sharing the same
foundation than already succeeded pairs.
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D815
Philip Withnall [Tue, 1 Mar 2016 22:37:33 +0000 (22:37 +0000)]
test-priority: ignore the local preference
The local preference depends on the rank of the IP address in the list
of all IP addresses available of the box running the test. As this value
is not fixed we ignore it in the test.
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D818
Philip Withnall [Tue, 1 Mar 2016 22:33:51 +0000 (22:33 +0000)]
conncheck: nominate only one matching pair
This patch fixes a bug in priv_mark_pair_nominated(), where the local
candidate was not passed to the function, so removing the possibility to
find which local candidate the check was sent to.
Reviewed-by: Philip Withnall <philip@tecnocode.co.uk>
Differential Revision: https://phabricator.freedesktop.org/D808
Philip Withnall [Tue, 1 Mar 2016 22:28:35 +0000 (22:28 +0000)]
conncheck: add more debug information
Add a more debug details, specifically in some places, it is interesting
to have the src and dst IP addresses of the pairs being checked, and
also to make the difference between log related to different stream ids.
Reviewed-by: Philip Withnall <philip@tecnocode.co.uk>
Differential Revision: https://phabricator.freedesktop.org/D803
Mike Ruprecht [Mon, 18 Jan 2016 12:42:46 +0000 (12:42 +0000)]
agent: Fix not setting UPnP timeout on second gather_candidates()
If the first call to nice_agent_gather_candidates() partially succeeds
(setting a UPnP agent and timeout), then fails before starting
gathering, a second call to nice_agent_gather_candidates() would fail to
set a new UPnP timeout because the UPnP initialisation block would be
skipped. That means gathering would never succeed due to timing out on
UPnP.
Fix that by setting the UPnP timeout whenever a new pending UPnP mapping
is added.
https://phabricator.freedesktop.org/T3534
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Jose Antonio Santos Cadenas [Thu, 3 Dec 2015 14:01:35 +0000 (15:01 +0100)]
configure.ac: Update glib version
As udp-bsd.ccode is using G_IO_ERROR_CONNECTION_CLOSED glib 2.44
is required.
Change-Id: I1bb63f2484c513c58eeec312ba0835164604c40c
Reviewed-by: Philip Withnall <philip@tecnocode.co.uk>
https://phabricator.freedesktop.org/T3492
Philip Withnall [Wed, 7 Oct 2015 18:03:58 +0000 (19:03 +0100)]
pseudotcp: Use labs() rather than abs() for handling long integers
This fixes a compiler warning and prevents a possible truncation.
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D345
Philip Withnall [Mon, 22 Jun 2015 10:30:31 +0000 (11:30 +0100)]
tests: Enable G_MESSAGES_DEBUG for all unit tests
Now that we’re using automake’s parallel test harness, it automatically
redirects all the debug log spew away from the console, so we should
always have it enabled.
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D292
Philip Withnall [Thu, 1 Oct 2015 16:52:08 +0000 (17:52 +0100)]
build: Set repository callsign in .arcconfig
This fixes `arc diff` to select the right repository when submitting
patches.
Philip Withnall [Wed, 30 Sep 2015 13:57:10 +0000 (14:57 +0100)]
agent: Correctly namespace Component and its methods
Remove all references to the old, unnamespaced versions. This should
cause no functional changes.
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D309
Philip Withnall [Wed, 30 Sep 2015 13:34:34 +0000 (14:34 +0100)]
agent: Correctly namespace Stream and its methods
Remove all references to the old, unnamespaced versions. This should
cause no functional changes.
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D308
Philip Withnall [Wed, 30 Sep 2015 13:11:14 +0000 (14:11 +0100)]
agent: Turn Component into a GObject
This makes it reference-counted. This will be useful for allowing
GDatagramBased and GIOStream objects to hold references to the stream
and component they are interested in, allowing removal of the global
NiceAgent lock previously needed to look up the component for every I/O
operation.
Deprecate all the old methods until it’s properly namespaced.
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D307
Philip Withnall [Wed, 30 Sep 2015 13:10:32 +0000 (14:10 +0100)]
agent: Turn Stream into a GObject
This makes it reference-counted. This will be useful for allowing
GDatagramBased and GIOStream objects to hold references to the stream
and component they are interested in, allowing removal of the global
NiceAgent lock previously needed to look up the component for every I/O
operation.
It also means that nice_stream_close() could eventually become
asynchronous, which would fix a few race conditions.
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D306
Philip Withnall [Wed, 30 Sep 2015 16:59:04 +0000 (17:59 +0100)]
tests: Update expected priority values in test-priority
This is a follow up to T3324, to update the test case to match the new
values generated.
Bug: https://phabricator.freedesktop.org/T3324
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D301
Philip Withnall [Wed, 30 Sep 2015 16:54:21 +0000 (17:54 +0100)]
tests: Use g_assert_cmpuint() to make test failures easier to diagnose
Now we can actually see the priority numbers which are unequal.
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D300
Philip Withnall [Wed, 30 Sep 2015 16:53:14 +0000 (17:53 +0100)]
tests: Set candidate addresses in test-priority
This avoids an assertion failure in nice_address_to_string() when the
addresses are compared for priority.
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D299
Philip Withnall [Wed, 30 Sep 2015 16:46:47 +0000 (17:46 +0100)]
agent: Remove redundant GLIB_CHECK_VERSION macros
We depend on GLib 2.36.0, which is a higher version than any of these
version checks cared about, so they were all trivially true or false.
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D298
Philip Withnall [Wed, 30 Sep 2015 16:44:44 +0000 (17:44 +0100)]
tests: Remove g_thread_init() calls
We depend on GLib 2.36.0; g_thread_init() has been deprecated since
2.32.0, when thread initialisation was changed to happen automatically.
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D297
Philip Withnall [Wed, 30 Sep 2015 16:41:36 +0000 (17:41 +0100)]
tests: Remove g_type_init() calls
We depend on GLib 2.36.0, which deprecated g_type_init() since GType
initialisation is now done automatically.
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D296
Jakub Adam [Fri, 11 Sep 2015 10:57:54 +0000 (11:57 +0100)]
conncheck: rename priv_process_response_check_for_peer_reflexive()
Renamed the function to priv_process_response_check_for_reflexive()
because it now checks also for server reflexive candidates.
Updated the documentation to indicate that the function never returns
NULL.
Maniphest Tasks: https://phabricator.freedesktop.org/T115
Differential Revision: https://phabricator.freedesktop.org/D243
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Jakub Adam [Fri, 11 Sep 2015 10:56:02 +0000 (11:56 +0100)]
conncheck: generate candidate pair for valid srflx candidate
In priv_process_response_check_for_peer_reflexive(), mere presence of a candidate in local_candidates doesn't mean there's also some candidate
pair in conncheck_list using it - for instance that candidate may be server reflexive, for which no check pairs are initially created (see
conn_check_add_for_candidate_pair()).
If we fail to find corresponding pair upon receiving such candidate's IP in a conncheck response's XOR-MAPPED-ADDRESS attribute, we shall add a
new one in a similar way we would add a new pair for a just discovered peer reflexive candidate.
Previous priv_process_response_check_for_peer_reflexive() implementation would return NULL, causing a CandidateCheckPair with local candidate of
type HOST to be wrongly selected even though the local host IP might not be directly accessible by the remote counterpart (e.g. it's an address
on a private network segment). In practice this was coming through as a duplex connection that libnice was reporting as properly established,
but only one direction of the communication was actually working.
Maniphest Tasks: https://phabricator.freedesktop.org/T115
Differential Revision: https://phabricator.freedesktop.org/D242
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Jakub Adam [Fri, 11 Sep 2015 10:33:51 +0000 (11:33 +0100)]
agent: check for base socket in _tcp_sock_is_writable()
The argument passed into the callback is always a base (TCP/UDP) socket,
which can't be directly compared with local candidate's sockptr (may be
TURN, http, or other socket wrapping another one). We're in fact
interested whether sock is a base socket of sockptr.
Maniphest Tasks: T114
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Differential Revision: https://phabricator.freedesktop.org/D241
Jakub Adam [Fri, 11 Sep 2015 10:29:39 +0000 (11:29 +0100)]
socket: add nice_socket_is_base_of()
This will be used in the next commit.
Maniphest Tasks: T114
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Differential Revision: https://phabricator.freedesktop.org/D240
Philip Withnall [Thu, 10 Jul 2014 07:32:04 +0000 (08:32 +0100)]
build: Bump GLib dependency to 2.36
This is needed for G_IO_ERROR_BROKEN_PIPE, which is used in the I/O
stream code.
Reported by Emanuele Bizzarri <emabiz76@gmail.com> on the mailing list.
Philip Withnall [Fri, 4 Sep 2015 07:14:08 +0000 (08:14 +0100)]
stun: Disable debug by default
To match debug_enable in agent/debug.c. Debug can still be enabled by
calling stun_debug_enable() or nice_debug_enable().
Spotted on the mailing list by Tom Chen.
Philip Withnall [Mon, 29 Jun 2015 15:30:12 +0000 (16:30 +0100)]
agent: Add assertions to check component state transitions are valid
There is no point in the NiceComponents having a state machine if the
state transition graph is not documented or enforced. Document and
enforce it.
http://phabricator.freedesktop.org/T120
Philip Withnall [Wed, 2 Sep 2015 15:44:45 +0000 (16:44 +0100)]
ms-ice: ensure distinct candidate priority for multihomed hosts
Summary:
Offering multiple host candidates with equal priorities could lead to unpredictable candidate pair selection by our counterparty.
Fixes call disconnection by MS Lync client after 30 seconds while VPN (2nd IP) was active on libnice host.
Maniphest Tasks: T3324
Reviewers: pwithnall
Projects: #libnice
Reviewed By: pwithnall
Subscribers: pwithnall
Differential Revision: https://phabricator.freedesktop.org/D234
Jakub Adam [Wed, 2 Sep 2015 15:43:02 +0000 (16:43 +0100)]
candidate: use distinct priority also for non-MS compatibilities
Maniphest Tasks: T3324
Reviewers: pwithnall
Projects: #libnice
Reviewed By: pwithnall
Differential Revision: https://phabricator.freedesktop.org/D239
Jakub Adam [Wed, 2 Sep 2015 15:41:49 +0000 (16:41 +0100)]
conncheck: give temporary candidate_priority a base_addr
Summary:
Fixes "(nice_address_to_string): should not be reached" errors when calling nice_candidate_ms_ice_priority() because of invalid NiceAddress.
Maniphest Tasks: T3324
Reviewers: pwithnall
Projects: #libnice
Reviewed By: pwithnall
Subscribers: pwithnall
Differential Revision: https://phabricator.freedesktop.org/D238
Jakub Adam [Wed, 2 Sep 2015 15:41:49 +0000 (16:41 +0100)]
ms-ice: ensure distinct candidate priority for multihomed hosts
Summary:
Offering multiple host candidates with equal priorities could lead to unpredictable candidate pair selection by our counterparty.
Fixes call disconnection by MS Lync client after 30 seconds while VPN (2nd IP) was active on libnice host.
Maniphest Tasks: T3324
Reviewers: pwithnall
Projects: #libnice
Reviewed By: pwithnall
Subscribers: pwithnall
Differential Revision: https://phabricator.freedesktop.org/D234
Jakub Adam [Wed, 2 Sep 2015 15:41:48 +0000 (16:41 +0100)]
discovery: assign candidate priority after base_addr is set
Summary: So that we can take the base address into account in the calculation.
Maniphest Tasks: T3324
Reviewers: pwithnall
Projects: #libnice
Reviewed By: pwithnall
Subscribers: pwithnall
Differential Revision: https://phabricator.freedesktop.org/D235
Philip Withnall [Wed, 2 Sep 2015 15:34:01 +0000 (16:34 +0100)]
ms-turn: don't wait for a reply to STUN_SEND request
Maniphest Tasks: T126
Reviewers: pwithnall
Projects: #libnice
Reviewed By: pwithnall
Subscribers: pwithnall
Differential Revision: https://phabricator.freedesktop.org/D223
Jakub Adam [Wed, 2 Sep 2015 15:32:05 +0000 (16:32 +0100)]
ms-turn: don't wait for a reply to STUN_SEND request
As per [MS-TURN] Section 2.2.1, TURN message type 0x0104 "Send request
response" isn't supported and the TURN server MUST NOT send them. Thus,
libnice should not remember Send requests in agent->sent_ids because
without replies coming, the number of allowed pending transaction gets
quickly exhausted, causing our data packets to be dropped until a
request timeout frees some space in the queue.
This behavior resulted in choppy reception of our audio on a Lync client
when connected via Lync Edge (TURN) Server.
Maniphest Tasks: T126
Reviewers: pwithnall
Projects: #libnice
Reviewed By: pwithnall
Subscribers: pwithnall
Differential Revision: https://phabricator.freedesktop.org/D223
Philip Withnall [Sat, 29 Aug 2015 22:26:00 +0000 (23:26 +0100)]
Creating TCP sockets with TCP_NODELAY option set to TRUE
Summary:
Disable Nagling for underlying TCP sockets used by libnice, because they
are typically used for streaming applications, or for pseudo-TCP; the
bandwidth in both cases is harmed by Nagling.
Based on a patch by Vadim Genkin.
Maniphest Tasks: T3317
Reviewers: vadimgenkin, pwithnall
Projects: #libnice
Reviewed By: pwithnall
Subscribers: pwithnall, vadimgenkin
Differential Revision: https://phabricator.freedesktop.org/D230
Philip Withnall [Sat, 29 Aug 2015 21:39:56 +0000 (22:39 +0100)]
Creating TCP sockets with TCP_NODELAY option set to TRUE
Disable Nagling for underlying TCP sockets used by libnice, because they
are typically used for streaming applications, or for pseudo-TCP; the
bandwidth in both cases is harmed by Nagling.
Based on a patch by Vadim Genkin.
Maniphest Tasks: T3317
Reviewers: pwithnall
Projects: #libnice
Subscribers: pwithnall, vadimgenkin
Differential Revision: https://phabricator.freedesktop.org/D230
Philip Withnall [Sat, 29 Aug 2015 19:05:00 +0000 (20:05 +0100)]
Fix agent leak in case component socket is reset remotely
Summary: The patch fixes the issue where agent reference count is not properly decremented causing instance leak in cases where component's socket is reset remotely.
Reviewers: #libnice, pwithnall
Projects: #libnice
Reviewed By: #libnice, pwithnall
Subscribers: pwithnall, maximgolunov
Differential Revision: https://phabricator.freedesktop.org/D236
Philip Withnall [Sat, 29 Aug 2015 19:04:03 +0000 (20:04 +0100)]
Fix agent leak in case component socket is reset remotely
Summary: The patch fixes the issue where agent reference count is not properly decremented causing instance leak in cases where component's socket is reset remotely.
Reviewers: #libnice, pwithnall
Projects: #libnice
Reviewed By: #libnice, pwithnall
Subscribers: pwithnall, maximgolunov
Differential Revision: https://phabricator.freedesktop.org/D236
Philip Withnall [Wed, 19 Aug 2015 08:24:27 +0000 (09:24 +0100)]
build: Update .gitignore
Add stun/usages/.dirstamp.
Philip Withnall [Wed, 19 Aug 2015 08:22:49 +0000 (09:22 +0100)]
build: Fix multiple definition of CLEANFILES
It’s already defined in common.mk.
Philip Withnall [Wed, 19 Aug 2015 08:20:01 +0000 (09:20 +0100)]
build: Add .arcconfig file
This completes the transition to Phabricator; everyone should be using
the same project settings now.
https://phabricator.freedesktop.org/tag/libnice/
Philip Withnall [Wed, 19 Aug 2015 08:19:18 +0000 (09:19 +0100)]
socket: Handle ECONNRESET as EWOULDBLOCK on Windows
Summary:
Some versions of Windows can return ECONNRESET for UDP recvmsg() calls
if they would otherwise block. Hence, handle the two equivalently; this
should not affect behaviour on Linux, which apparently does not return
ECONNRESET for UDP recvmsg() calls at all.
https://phabricator.freedesktop.org/T121
Maniphest Tasks: T121
Reviewers: ocrete
Projects: #libnice
Reviewed By: ocrete
Subscribers: stwiname, felixSchl
Differential Revision: https://phabricator.freedesktop.org/D227
Philip Withnall [Wed, 19 Aug 2015 08:16:53 +0000 (09:16 +0100)]
socket: Close base socket for a TCP passive socket when closing parent
Summary:
Otherwise the base socket will leak. Spotted by Vadim Genkin.
https://phabricator.freedesktop.org/T125
Maniphest Tasks: T125
Reviewers: ocrete
Projects: #libnice
Reviewed By: ocrete
Subscribers: vadimgenkin
Differential Revision: https://phabricator.freedesktop.org/D228
Philip Withnall [Tue, 18 Aug 2015 13:58:23 +0000 (14:58 +0100)]
socket: Close base socket for a TCP passive socket when closing parent
Otherwise the base socket will leak. Spotted by Vadim Genkin.
https://phabricator.freedesktop.org/T125
Philip Withnall [Tue, 18 Aug 2015 13:03:41 +0000 (14:03 +0100)]
agent: Remove unused inet_pton() function
Summary:
As spotted by Felix <felixschlitter@gmail.com>. This is a static
function which is totally unused in this compilation unit and is causing
build failures with `-Werror=unused-function`.
Maniphest Tasks: T123
Reviewers: felixSchl, ocrete
Projects: #libnice
Differential Revision: https://phabricator.freedesktop.org/D221
Philip Withnall [Tue, 30 Jun 2015 13:39:51 +0000 (14:39 +0100)]
agent: Remove unused inet_pton() function
As spotted by Felix <felixschlitter@gmail.com>. This is a static
function which is totally unused in this compilation unit and is causing
build failures with -Werror=unused-function.
http://phabricator.freedesktop.org/T123
Philip Withnall [Tue, 18 Aug 2015 12:33:23 +0000 (13:33 +0100)]
socket: Handle ECONNRESET as EWOULDBLOCK on Windows
Some versions of Windows can return ECONNRESET for UDP recvmsg() calls
if they would otherwise block. Hence, handle the two equivalently; this
should not affect behaviour on Linux, which apparently does not return
ECONNRESET for UDP recvmsg() calls at all.
https://phabricator.freedesktop.org/T121
Jakub Adam [Thu, 18 Jun 2015 07:05:21 +0000 (09:05 +0200)]
conncheck: set writable callback to socket from TCP active connect
A new socket created in nice_tcp_active_socket_connect() should have its
writable callback set, because it's possible for it to become a base
socket of a peer reflexive candidate, if some is discovered by
connection checks on that TCP active candidate.
Previously, when such prflx candidate became selected, without write_cb
on the socket the agent was never notified about it becoming writable
again after the socket's buffer got filled up. This caused the data flow
to hang permanently.
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
http://phabricator.freedesktop.org/T117
Philip Withnall [Tue, 30 Jun 2015 13:29:43 +0000 (14:29 +0100)]
pseudotcp: Only define errnos on Windows if not already defined
Recent versions of MinGW define at least ECONNABORTED and EAFNOSUPPORT,
so only define the various socket errnos if they are not defined
already.
Based on a patch by Alexey Pawlow <alexey.pawlow@gmail.com> and Felix
<felixschlitter@gmail.com>.
Reviewed-by: Olivier Crete <olivier.crete@collabora.com>
Reviewed-by: Felix Schlitter <felixschlitter@gmail.com>
http://phabricator.freedesktop.org/T122
Felix Schlitter [Mon, 29 Jun 2015 19:37:31 +0000 (07:37 +1200)]
Use G_GSIZE_FORMAT instead of %zu
The C runtime on windows does not implement a printf that understands
%zu and other C99 format specifiers. Use G_GSIZE_FORMAT instead. This
is further consistent with the rest of the code base that makes use of
G_GSIZE_FORMAT throughout.
https://github.com/libnice/libnice/pull/3
Olivier Crête [Thu, 18 Sep 2014 23:33:10 +0000 (19:33 -0400)]
Split "verbose" on-every-packet messages to a verbose log
This way, the regular log will only contain connection-time information.
Olivier Crête [Thu, 18 Sep 2014 23:33:06 +0000 (19:33 -0400)]
stun: Remove annoying non-error on non-STUN packet