platform/upstream/libnice.git
7 years agoagent: Adjust the nice_agent_new_full() to use flags
Olivier Crête [Wed, 21 Jun 2017 20:55:32 +0000 (16:55 -0400)]
agent: Adjust the nice_agent_new_full() to use flags

This makes it easier to read and more extensible.

7 years agoagent: remove spurious newlines
Fabrice Bellet [Sun, 18 Jun 2017 08:12:58 +0000 (10:12 +0200)]
agent: remove spurious newlines

Differential Revision: https://phabricator.freedesktop.org/D1756

7 years agostun: fix gcc7 implicit fallthrough warning
Fabrice Bellet [Sun, 28 May 2017 20:20:36 +0000 (22:20 +0200)]
stun: fix gcc7 implicit fallthrough warning

Differential Revision: https://phabricator.freedesktop.org/D1754

7 years agoagent: add new pairs only for gathering streams
Fabrice Bellet [Fri, 15 Jul 2016 21:31:42 +0000 (23:31 +0200)]
agent: add new pairs only for gathering streams

At the end of the local candidate gathering process, we only create new
pairs for streams that are in gathering state.

Other stream that may be in ready state for example, due to a
previously succeeded conncheck process, may have accumulated some
couples (local,remote) candidates that have not resulted in the creation
a new pair during this previous conncheck process, and we don't want
these new pairs to be added now, because it would generate unneeded
transition changes for a stream unconcerned by this gathering.

Differential Revision: https://phabricator.freedesktop.org/D1755

7 years agoconncheck: fix the component failed transition
Fabrice Bellet [Tue, 21 Jun 2016 19:47:42 +0000 (21:47 +0200)]
conncheck: fix the component failed transition

This patch fixes the transition of a component from connecting to
failed, that previously occured due to the propagation of the
keep_timer_going variable, and to the final call to function
priv_update_check_list_failed_components(), after the global agent
timer was stopped.

Previously, the code almost never entered to failed state, because the
timer was going one, as long as the number of nominated pair was not
enough, and as long as there were valid pairs not yet nominated. Even
if all pair timers were over.

The definition of the Failed state of a conncheck list is somewhat
contradictory in the spec, depending on weather you read :

 * sect 5.7.4. "Computing States",
 "Failed:  In this state, the ICE checks have not completed successfully
 for this media stream."

 or

 * sect 7.1.3.3. "Check List and Timer State Updates",
 "If all of the pairs in the check list are now either in the Failed or
 Succeeded state: If there is not a pair in the valid list for each
 component of the media stream, the state of the check list is set to
 Failed."

Our understanding of the ICE spec is that, the proper way to enter failed
state instead in when all connchecks have no longer in-progress pairs.
All pairs are either in state succeeded, discovered, or failed. No timer
is still running, and we have no hope that the conncheck list changes
again, except if an unexpected STUN packet arrives later. All pairs in
frozen state is a special case, that is handled separately (sect
7.1.3.3).

A special grace delay is added before declaring a component in state
Failed. This delay is not part of the RFC, and it is aimed to limit the
cases when a conncheck list is reactivated just after it's been declared
failed, causing a user visible transition from connecting to failed, and
back from failed to connecting again. This is also required by the test
suite, that counts exactly the number of time each state is entered, and
doesn't expect these transcient failed states to happen (frequent due to
the nature of the testsuite, less frequent in real life).

Differential Revision: https://phabricator.freedesktop.org/D1111

7 years agoconncheck: remove cancelled pair state
Fabrice Bellet [Thu, 16 Jun 2016 15:32:39 +0000 (17:32 +0200)]
conncheck: remove cancelled pair state

Pairs with the state NICE_CHECK_CANCELLED are the pairs targeted for
removal after the nomination of a pair with an higher priority,
described in Section 8.1.2 "Updating States", item 2 of RFC 5245. They
include also pairs that overflow the conncheck list size, but this is a
somewhat more marginal situation. So we are mainly interested in the
first use case of this state.

This state mixes two different situations, that deserve a distinct
handling : on one side, there are waiting or frozen pairs that must be
removed, this is an immediate action that doesn't need a dedicated state
for that. And on the other side, there are in-progress pairs that
should no longer be retransmitted, because another pair with a higher
priority has already been nominated.

This patch removes the cancelled state, and adds a flag
retransmit_on_timeout to deal with this last situation. Note that this
case should not generate a triggered check, as per described in section
7.2.1.4, when the state of the pair is In-Progress or Failed, since this
pair of lower priority has no hope to replace the nominated one.

Differential Revision: https://phabricator.freedesktop.org/D1114

7 years agoconncheck: adjust recheck on timeout strategy
Fabrice Bellet [Tue, 14 Jun 2016 19:32:26 +0000 (21:32 +0200)]
conncheck: adjust recheck on timeout strategy

The pair recheck on timeout can easily cause repetitive rechecks in
a ping-pong effect, if both peers with the same behaviour try to
check the same pair almost simultaneously, and if the network rtt
is greater than the initial timer rto. The reply to the initial
stun request may arrive after the in-progress conncheck
cancellation (described in RFC 5245, sect 7.2.1.4). Cancellation
creates a new stun request, and forgets the initial one.
The conncheck timer is restarted with the same initial value,
so the same situation happens again later.

We choose to avoid resetting the timer in such situation. After enough
retransmissions, the timeout delay, that doubles after each timeout,
becomes longer than the rtt, and the stun reply can be handled.

Differential Revision: https://phabricator.freedesktop.org/D1115

7 years agoconncheck: do not recheck a just succeeded pair
Fabrice Bellet [Tue, 14 Jun 2016 19:20:49 +0000 (21:20 +0200)]
conncheck: do not recheck a just succeeded pair

We cancel the potential in-progress transaction cancellation, caused by
sect 7.2.1.4 "Triggered Checks", when we receive a valid reply before
transmission timeout, or just after timeout, when the pair is
temporarily put on the triggered check list on the way to be
rechecked. This situation is not covered by the RFC 5245.

Differential Revision: https://phabricator.freedesktop.org/D1119

7 years agoconncheck: fix a state transition case
Fabrice Bellet [Tue, 14 Jun 2016 19:12:16 +0000 (21:12 +0200)]
conncheck: fix a state transition case

When a new stun request hits a valid pair, of a failed component, we may
have a transition from state failed to connected. In this situation, we
do a logical progression failed -> connecting -> connected, like we do
in function priv_update_check_list_state_for_ready()

Similarily, when a new stun request hits a failed pair, of a failed
component, triggering a new conncheck for this pair may also cause the
component state to move back from failed to connecting state.

Differential Revision: https://phabricator.freedesktop.org/D1118

7 years agoconncheck: try to change earlier to state ready
Fabrice Bellet [Tue, 14 Jun 2016 19:04:49 +0000 (21:04 +0200)]
conncheck: try to change earlier to state ready

We check if we can move from state connected to ready just
after a pair expired its retransmission count. This pair
will be marked failed, and will no longer be in-progress.
The number of in-progress dropping down to zero is one
of the conditions needed to make the transition to ready,
per component (and not globally as it's the case in other
locations where this check function is called).

Differential Revision: https://phabricator.freedesktop.org/D1117

7 years agoconncheck: dont cancel a pair for triggered check
Fabrice Bellet [Mon, 11 Apr 2016 11:13:51 +0000 (13:13 +0200)]
conncheck: dont cancel a pair for triggered check

This patch adds another supplementary "corner" case, not covered by the
ICE spec, sect 8.1.2, "Updating States". A pair in waiting state and in
the triggered check list should be considered like an in-progress pair,
and cancelled only if its priority is lower than the priority of the
nominated pair. This is required in some aggressive nomination
situations for both peers to select the same pair, having the highest
priority.

Differential Revision: https://phabricator.freedesktop.org/D933

7 years agoconncheck: remove a useless pair recheck
Fabrice Bellet [Fri, 1 Apr 2016 15:31:44 +0000 (17:31 +0200)]
conncheck: remove a useless pair recheck

This exception to the ICE spec is no longer needed: when a pair is in
the succeeded state, there is no needed to recheck it again upon
reception of an incoming stun request on it.

Differential Revision: https://phabricator.freedesktop.org/D884

7 years agoconncheck: update the pair state in triggered check list
Fabrice Bellet [Tue, 12 Apr 2016 10:56:28 +0000 (12:56 +0200)]
conncheck: update the pair state in triggered check list

With this patch, we update the state of the pair to waiting when
it is put in the triggered check queue. We also take care to call
priv_schedule_triggered_check() before priv_mark_pair_nominated()
so a pair to be rechecked and put on the triggered check queue
will have a unique state to be tested in the following call to
priv_mark_pair_nominated() when evaluating its nomination attributes.

Differential Revision: https://phabricator.freedesktop.org/D883

7 years agoconncheck: new pairs never have the nominated flag preset
Fabrice Bellet [Tue, 12 Apr 2016 11:32:49 +0000 (13:32 +0200)]
conncheck: new pairs never have the nominated flag preset

This patch disables the possibility to set the nominated flag of a
candidate pair at creation time. This possibility was used when a new
pair is created from a new peer reflexive remote candidate, when the
agent is in controlled mode, and an stun request with USE-CANDIDATE is
received. In this case, since previous commit "conncheck: fix a
nomination corner case", we set the nominated flag when the stun
response of this new pair will arrive, and not before.  Consequently,
this flag is no longer required when the pair is created.

Differential Revision: https://phabricator.freedesktop.org/D881

7 years agoconncheck: fix a nomination corner case
Fabrice Bellet [Tue, 12 Apr 2016 11:30:04 +0000 (13:30 +0200)]
conncheck: fix a nomination corner case

This patch add two supplementary cases, not covered by the ICE spec,
sect 7.2.1.5 "Updating the Nominated Flag" when a controlled agent
receives a STUN request with the USE-CANDIDATE flag, for a pair that is
in the waiting state. We consider that this case is similar to the
in-progress state, and should be handled in the same way. We also accept
when the pair is in frozen state. This latter case happens in the
new-dribble test, when an agent replays incoming early connchecks.

Differential Revision: https://phabricator.freedesktop.org/D880

7 years agoconncheck: use the right pair when retriggering a check
Fabrice Bellet [Tue, 19 Apr 2016 16:16:26 +0000 (18:16 +0200)]
conncheck: use the right pair when retriggering a check

This patch completes the previous patch by adding a link back from the
discovered pair, to the parent pair that generated this check. This link
is needed by the ICE spec, to comply with section 8.1.1.1, "Regular
nomination", where the check to be retriggered is the initial check that
caused the discovery of the valid pair. When the valid pair is a
peer-reflexive pair, the retriggered check must target the succeeded
pair, and not the valid discovered pair.

Differential Revision: https://phabricator.freedesktop.org/D879

7 years agoconncheck: link succeeded and discovered pairs
Fabrice Bellet [Tue, 12 Apr 2016 11:25:16 +0000 (13:25 +0200)]
conncheck: link succeeded and discovered pairs

When the agent has the role of the stun server, is in controlled mode,
and receives a pair with the "use-candidate" attribute set, it must find
a matching succeded or discovered pair in its conncheck list. This is
described in ICE spec 7.2.1.5, "Updating the Nominated Flag", item #1.
When a matching pair is in succeeded state, the agent must nominate the
valid pair (a discovered pair) constructed from section 7.1.3.2.2,
that's been created from this succeeded one. To make this lookup, we
introduce a new "discovered_pair" member of the CandidateCheckPair
struct, that links the succeeded pair, and its discovered pair
if any.

Differential Revision: https://phabricator.freedesktop.org/D878

7 years agoconncheck: improve triggered check of in-progress pairs
Fabrice Bellet [Tue, 19 Apr 2016 15:59:27 +0000 (17:59 +0200)]
conncheck: improve triggered check of in-progress pairs

This patch update the way triggered checks of in-progress pairs are
handled, according to ICE spec, section 7.2.1.4. Previously the same
connection check was retransmitted with an updated timeout. This causes
problems when a controlling role switch occurs in this time frame.
This is the reason why a new connection check must be generated
reflecting the updated role. We introduce a new flag "recheck_on_timeout"
in the pair indicating that the pair must be rechecked at the next timer
expiration.

Differential Revision: https://phabricator.freedesktop.org/D875

7 years agoconncheck: invoke the debug dump in more places
Fabrice Bellet [Tue, 12 Apr 2016 11:20:38 +0000 (13:20 +0200)]
conncheck: invoke the debug dump in more places

Differential Revision: https://phabricator.freedesktop.org/D1123

7 years agoconncheck: improve the selection of the pairs to be checked
Fabrice Bellet [Tue, 19 Apr 2016 15:06:32 +0000 (17:06 +0200)]
conncheck: improve the selection of the pairs to be checked

This patch aims to implement more closely the algorithm described
in RFC 5245 indicating how pairs are transitionned from state Frozen
to Waiting. This is described in 7.1.3.2 when a check succeeded, and
correspond to modifications in function priv_conn_check_unfreeze_related().
This is also described in 5.7.4 when defining the initial state of the
pairs in a conncheck, and correspond to modifications in function
priv_conn_check_unfreeze_next().

This patch introduces the notion of active and frozen check list. It
allows us to define the timer restranmission delay as described in 16.1.

Another modification in priv_conn_check_tick_unlocked() is that every
stream in handled consecutively, and in an independant way. The pacing
was previously of a single STUN request emitted per callback, it is now
of a triggered check per callback OR a single STUN per callback AND per
stream per callback.

The description of ordinary checks per stream in 5.8 is detailled in
function priv_conn_check_tick_stream(), and a remaining of the code
used to nominate a pair by the controlling agent is put in a dedicated
function priv_conn_check_tick_stream_nominate()

Differential Revision: https://phabricator.freedesktop.org/D813

7 years agoconncheck: update pair valid property selectively
Fabrice Bellet [Mon, 7 Mar 2016 15:35:09 +0000 (16:35 +0100)]
conncheck: update pair valid property selectively

With this patch, we fix a corner case when the succeeded pair is a
peer-reflexive candidate pair, that already has been discovered
previously, In this case, the current pair -p- should not be marked
valid, because the valid flag is already set on the discovered pair.

Differential Revision: https://phabricator.freedesktop.org/D1124

7 years agotest-nomination: added a new test for the nomination mode
Fabrice Bellet [Wed, 22 Jun 2016 13:44:39 +0000 (15:44 +0200)]
test-nomination: added a new test for the nomination mode

Differential Revision: https://phabricator.freedesktop.org/D1107

7 years agoconncheck: implement ice regular nomination method
Fabrice Bellet [Tue, 19 Apr 2016 11:12:48 +0000 (13:12 +0200)]
conncheck: implement ice regular nomination method

This patch implements Regular Nomation as described in RFC5245
8.1.1.1. The controlling agent lets valid pairs accumulate, and
decides which pair to recheck with the use-candidate attribute set.
priv_mark_pair_nominated() follows 7.2.1.5, to update the nominated
pair when acting as a STUN server, and
priv_map_reply_to_conn_check_request() implements 7.1.3.2.4 to
update the nominated pair when acting as a STUN client. A new
property is also added to the agent to control the nomination
mode, which can be regular of aggressive, with default value
set to aggressive.

Two new flags are introduced in the CandidateCheckPair structure:

- use_candidate_on_next_check indicates the STUN client to add the
  use-candidate attribute when the pair will be checked. At this
  time, the nominated flag has not been set on this pair yet.

- mark_nominated_on_response_arrival indicates the STUN server
  to nominate the pair when its succesfull response to a
  previous triggered check will arrive (7.2.1.5, item #2)

Differential Revision: https://phabricator.freedesktop.org/D811

7 years agoconncheck: fix pair state transition when successful response is received
Fabrice Bellet [Tue, 12 Apr 2016 11:02:45 +0000 (13:02 +0200)]
conncheck: fix pair state transition when successful response is received

According the ICE RFC 5245, 7.1.3.2.3, the pair that *generated* a
successful check should go to state succeeded, not only the valid
pair built in section 7.1.3.2.2.

Differential Revision: https://phabricator.freedesktop.org/D810

7 years agoconncheck: peer reflexive candidates are not paired
Fabrice Bellet [Mon, 1 Feb 2016 10:10:21 +0000 (11:10 +0100)]
conncheck: peer reflexive candidates are not paired

This patch makes the code compliant with ICE RFC, 7.2.1.3 "Learning
Peer Reflexive Candidates" and 7.1.3.2.1 "Discovering Peer Reflexive
Candidates", where discovered candidates do not cause the creation
of new pairs to be checked.

Differential Revision: https://phabricator.freedesktop.org/D805

7 years agoconncheck: update selected pair when nominated flag is set
Fabrice Bellet [Mon, 6 Jun 2016 20:24:50 +0000 (22:24 +0200)]
conncheck: update selected pair when nominated flag is set

This modifies commit 8f1f615. It is better focused to update the
selected pair just after its nominated flag has been set. We also keep
the code homogeneous with other places, where the call to
priv_update_selected_pair() immediately follows the setting of
pair->nominated. Moreover in priv_update_check_list_state_for_ready(),
we would call priv_update_selected_pair() more times that necessary when
iterating on all nominated pairs.

Differential Revision: https://phabricator.freedesktop.org/D1125

7 years agostun timer: make properties for stun timer tunables
Fabrice Bellet [Thu, 9 Jun 2016 21:28:43 +0000 (23:28 +0200)]
stun timer: make properties for stun timer tunables

Three STUN binding request properties should be customisable. RFC 5245
describes the retransmission timer of the STUN transaction 'RTO', and
RFC 5389 describes the number of retransmissions to send until a
response is received 'Rc'. The third property is the 'RTO' when
a reliable connection is used.

RFC 5389 introduces a supplementary property 'Rm' as a multiplier used
to compute the final timeout RTO * Rm. However, this property is not
added in libnice, because this would require breaking the public API for
STUN. Currently, our STUN implementation hardcodes a division by two for
this final timeout.

Differential Revision: https://phabricator.freedesktop.org/D1109

7 years agoagent: Use base_addr to generate rport in SDP
Olivier Crête [Thu, 8 Jun 2017 20:34:21 +0000 (16:34 -0400)]
agent: Use base_addr to generate rport in SDP

Reported by Capricornus (zhushengliang)

https://phabricator.freedesktop.org/T7763

7 years agointerfaces: ignore predefined network interfaces
Fabrice Bellet [Thu, 21 Apr 2016 16:18:59 +0000 (18:18 +0200)]
interfaces: ignore predefined network interfaces

Some interfaces, like the one managed by libvirtd to provide a network
bridge to locally hosted virtual machines, can be completely ignored
when gathering ICE candidates. The motivation for adding this
possibility is that, ignoring them doesn't remove capabilities, and
improves the overall speed of the connection check method, by reducing
the number of pairs to be tested. This patch adds the possibility to
define such interfaces in the configuration script.

Differential Revision: https://phabricator.freedesktop.org/D948

7 years agoexamples: Stop installing the examples
Philip Withnall [Mon, 1 May 2017 07:51:40 +0000 (08:51 +0100)]
examples: Stop installing the examples

There’s no point in installing them; their benefit is in providing
example code to developers.

Debian doesn’t package them; Fedora packages them in a separate
subpackage which will have to disappear.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D1737

7 years agoagent: do not create a GSource for UDP TURN socket
Fabrice Bellet [Tue, 5 Apr 2016 19:32:39 +0000 (21:32 +0200)]
agent: do not create a GSource for UDP TURN socket

With this patch, we don't create a new GSource for udp-turn socket,
because it would duplicate the packets already received on the base UDP
socket, as the underlying GSocket is the same. This is a race condition,
because an UDP packet arriving on the base socket, may randomly be
handled by the GSource callback created for the base socket (udp-bsd) of
the callback created for the udp-turn socket. Moreover this callback
already knows how to parse UDP datagrams received from a known turn
server.

This patch also prevents a subtle bug, when a STUN request is received
directly from a peer, is handled by the udp turn socket. If the agent
already has a valid permission for this remote candidate, established
for another pair, it will happily send the STUN reply through the turn
relay. This generates a source address mismatch on the peer agent, when
it'll receive the STUN response from the turn relay instead of the
initial address the request has been sent to.

Differential Revision: https://phabricator.freedesktop.org/D932

7 years agostun timer: fix timeout of the last retransmission
Fabrice Bellet [Thu, 9 Jun 2016 20:22:33 +0000 (22:22 +0200)]
stun timer: fix timeout of the last retransmission

According to RFC 5389, section 7.2.1, a special timeout is applied to
the last retransmission (Rm * RTO), with Rm default value of 16, instead
of (64 * RTO), 2^6 when the number of transmissions Rc is set to 7.

As spotted by Olivier Crete, stun_timer_* is a public API, that cannot
be changed, and the initial delay (RTO) is not preserved in the
stun_timer_s struct. So we use a hack that implicitely guess Rm from the
number of transmissions Rc, by generalizing the default value of the
spec for Rm and Rc to other values of Rc passed in stun_timer_start(

According to the spec, with the default value of Rc=7, the last delay
should be (64 * RTO), and it is instead (16 * RTO). So the last delay
can be computed by dividing the penultimate delay by two, instead of
multiplying it by two.

Differential Revision: https://phabricator.freedesktop.org/D1108

7 years agoagent: Ignore remote candidate of non-accepted types
Olivier Crête [Tue, 11 Apr 2017 22:31:21 +0000 (18:31 -0400)]
agent: Ignore remote candidate of non-accepted types

If we disable ice-tcp or ice-udp, ignore the remote
candidates for those types.

7 years agoconncheck: Check the controlling state when the req was sent
Olivier Crête [Tue, 11 Apr 2017 20:42:55 +0000 (16:42 -0400)]
conncheck: Check the controlling state when the req was sent

It was checking when the pair was created, but the role may have
already changed when the request is sent.

7 years agotests_: Add test to verify that only packets from validated addresses pass
Olivier Crête [Wed, 5 Apr 2017 21:43:26 +0000 (17:43 -0400)]
tests_: Add test to verify that only packets from validated addresses pass

https://phabricator.freedesktop.org/T104

Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Differential Revision: https://phabricator.freedesktop.org/D1717

7 years agoagent: Drop packets not from validated addresses
Olivier Crête [Wed, 5 Apr 2017 01:27:39 +0000 (21:27 -0400)]
agent: Drop packets not from validated addresses

This is required by the WebRTC spec.

Remove test-mainloop as it doesnt even try to do
a negotiation.

https://phabricator.freedesktop.org/T104

Differential Revision: https://phabricator.freedesktop.org/D1716

7 years agocandidate: Add equality check function
Olivier Crête [Tue, 4 Apr 2017 18:41:51 +0000 (14:41 -0400)]
candidate: Add equality check function

Add a function that can check if two candidates point to the same place.

https://phabricator.freedesktop.org/T104

Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Differential Revision: https://phabricator.freedesktop.org/D1715

7 years agotest-credentials: Fix leak
Olivier Crête [Wed, 5 Apr 2017 21:01:35 +0000 (17:01 -0400)]
test-credentials: Fix leak

7 years agodebug: Use libnice-verbose, not libnice-nice-verbose
Olivier Crête [Wed, 5 Apr 2017 00:34:05 +0000 (20:34 -0400)]
debug: Use libnice-verbose, not libnice-nice-verbose

7 years agotests: Use automake test-driver for valgrind
Olivier Crête [Tue, 4 Apr 2017 22:42:57 +0000 (18:42 -0400)]
tests: Use automake test-driver for valgrind

This fixes the valgrind integration with the new test drivers.

7 years agoagent: Remove impossible case
Olivier Crête [Tue, 4 Apr 2017 20:16:46 +0000 (16:16 -0400)]
agent: Remove impossible case

7 years agoagent: Separate return from NiceSocket and internal enum
Olivier Crête [Tue, 4 Apr 2017 20:16:05 +0000 (16:16 -0400)]
agent: Separate return from NiceSocket and internal enum

The same variable was used for return values from NiceSocket and
for the internal enum, but 0 and -1 have different meanings in both.

7 years agoudp-turn: Add some const to internal APIs
Olivier Crête [Tue, 4 Apr 2017 19:24:43 +0000 (15:24 -0400)]
udp-turn: Add some const to internal APIs

7 years agoMake clang-analyzer happy
Olivier Crête [Tue, 4 Apr 2017 16:30:27 +0000 (12:30 -0400)]
Make clang-analyzer happy

Various little things, none of which should make a functional difference.

7 years agoagent: Don't set variable that won't be used
Olivier Crête [Tue, 4 Apr 2017 16:29:29 +0000 (12:29 -0400)]
agent: Don't set variable that won't be used

It exits the loop immediately, so no point to set the variable.
And it makes the clang static analyzer happy.

7 years agoconncheck: Use the right test for empty remote_frag
Olivier Crête [Tue, 4 Apr 2017 16:25:50 +0000 (12:25 -0400)]
conncheck: Use the right test for empty remote_frag

It's now an array, not a pointer, so needs to test to emptyness.

It's a bugfix on the previous commit, 59ce41df

7 years agoconncheck: consider answer received when remote credentials are set
Miguel París Díaz [Sat, 1 Apr 2017 00:20:38 +0000 (20:20 -0400)]
conncheck: consider answer received when remote credentials are set

Consider that the answer is received when remote credentials
are set instead of when a remote candidate is set,
which could not happen or could cause more delay for the
connection establishment.

Ported to git master by Olivier Crête

Differential Revision: https://phabricator.freedesktop.org/D1704

7 years agoVersion 0.1.14.1
Olivier Crête [Mon, 3 Apr 2017 18:30:10 +0000 (14:30 -0400)]
Version 0.1.14.1

7 years agoVersion 0.1.14 0.1.14
Olivier Crête [Mon, 3 Apr 2017 18:28:22 +0000 (14:28 -0400)]
Version 0.1.14

7 years agoREADME: Depends on GLib 2.44
Olivier Crête [Mon, 3 Apr 2017 18:20:51 +0000 (14:20 -0400)]
README: Depends on GLib 2.44

7 years agostun: Make hmac code NDEBUG safe
Olivier Crête [Mon, 3 Apr 2017 18:20:31 +0000 (14:20 -0400)]
stun: Make hmac code NDEBUG safe

7 years agostun: Remove double const on int
Olivier Crête [Mon, 3 Apr 2017 16:27:28 +0000 (12:27 -0400)]
stun: Remove double const on int

7 years agocandidate: Test against possible type
Olivier Crête [Mon, 3 Apr 2017 16:13:47 +0000 (12:13 -0400)]
candidate: Test against possible type

There was a confusion and it tested against a value not in the enum.

7 years agoconfigure: Remove missing-noreturn warning
Olivier Crête [Mon, 3 Apr 2017 16:12:54 +0000 (12:12 -0400)]
configure: Remove missing-noreturn warning

We don't have or call noreturn functions in practice and it makes
the stun test build fail on clang.

7 years agostun: Use unions fix alignment issues
Olivier Crête [Mon, 3 Apr 2017 16:11:55 +0000 (12:11 -0400)]
stun: Use unions fix alignment issues

This makes clang happy.

7 years agoconfigure: Make sure flag test really fails on unknown flag
Olivier Crête [Mon, 3 Apr 2017 15:55:11 +0000 (11:55 -0400)]
configure: Make sure flag test really fails on unknown flag

clang on recent macOS seems to only emit a warning on unknown flags
which makes this test fail and then when using Werror, it makes the
compiler test fail too.

7 years agostun: Rename rand function to make its strengh clear
Olivier Crête [Mon, 3 Apr 2017 15:02:00 +0000 (11:02 -0400)]
stun: Rename rand function to make its strengh clear

It's only nonce level randomness, not long term key level.

Differential Revision: https://phabricator.freedesktop.org/D1711

7 years agostun: Switch from gcrypt to gnutls
Olivier Crête [Sat, 1 Apr 2017 01:23:12 +0000 (21:23 -0400)]
stun: Switch from gcrypt to gnutls

GLib already uses it, instead of adding another dep.

Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Differential Revision: https://phabricator.freedesktop.org/D1705

7 years agoagent: Only try to use the address of the same family to connect to TURN
Jakub Adam [Sun, 2 Apr 2017 15:08:07 +0000 (17:08 +0200)]
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.

This commit fixes another place of TURN discovery creation which was
omitted in fc0d3744ebc03f8137866170594968ba61e6be30. In my case it cuts
down up to ~15 seconds from candidate gathering phase, making it almost
instantaneous.

Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D1709

7 years agoudp-bsd: Log g_socket_send_message() errors
Jakub Adam [Sun, 2 Apr 2017 14:38:21 +0000 (16:38 +0200)]
udp-bsd: Log g_socket_send_message() errors

Those may have previously been silently ignored.

Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Differential Revision: https://phabricator.freedesktop.org/D1708

7 years agoagent: Eliminate duplicate debug in agent_recv_message_unlocked()
Jakub Adam [Sat, 1 Apr 2017 14:55:48 +0000 (16:55 +0200)]
agent: Eliminate duplicate debug in agent_recv_message_unlocked()

There were two almost consecutive verbose debugs containing basically
identical information. Merge them into one message that is printed only
when something was actually received.

Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Differential Revision: https://phabricator.freedesktop.org/D1707

7 years agoagent: Improve debug in component_io_cb()
Jakub Adam [Sat, 1 Apr 2017 14:21:36 +0000 (16:21 +0200)]
agent: Improve debug in component_io_cb()

agent_recv_message_unlocked() always receives a single message and
returns a RecvStatus code.

Avoid weird debugs like "received -1 valid messages" (when retval is
RECV_WOULD_BLOCK) and print the message only when something was actually
received.

Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Differential Revision: https://phabricator.freedesktop.org/D1706

7 years agoconfigure: gcry_mac_* were added in 1.6
Olivier Crête [Sat, 1 Apr 2017 00:43:27 +0000 (20:43 -0400)]
configure: gcry_mac_* were added in 1.6

They're not present int he 1.5.0 release, so require
the newer one.

7 years agoMakefile: Build tests before docs
Olivier Crête [Sat, 1 Apr 2017 00:03:30 +0000 (20:03 -0400)]
Makefile: Build tests before docs

7 years agodoc: Improve gtkdoc-check environment
Olivier Crête [Fri, 31 Mar 2017 23:20:17 +0000 (19:20 -0400)]
doc: Improve gtkdoc-check environment

7 years agodoc: Add missing symbol
Olivier Crête [Fri, 31 Mar 2017 23:09:36 +0000 (19:09 -0400)]
doc: Add missing symbol

7 years agoconfigure: Actually require gtk-doc 1.10 for no-tmpl
Olivier Crête [Fri, 31 Mar 2017 23:06:36 +0000 (19:06 -0400)]
configure: Actually require gtk-doc 1.10 for no-tmpl

7 years agostun: Use libgcrypt for SHA1 support
Philip Withnall [Mon, 7 Mar 2016 10:05:27 +0000 (10:05 +0000)]
stun: Use libgcrypt for SHA1 support

Now that libstun depends on libgcrypt, we might as well use its SHA1 hash
support, rather than carrying around our own.

Differential Revision: https://phabricator.freedesktop.org/D1612

7 years agostun: Use libgcrypt for MD5 support
Philip Withnall [Mon, 7 Mar 2016 10:04:52 +0000 (10:04 +0000)]
stun: Use libgcrypt for MD5 support

Now that libstun depends on libgcrypt, we might as well use its MD5 hash
support, rather than carrying around our own.

Differential Revision: https://phabricator.freedesktop.org/D1611

7 years agostun: Use libgcrypt to provide secure random number generation
Philip Withnall [Mon, 7 Mar 2016 09:27:38 +0000 (09:27 +0000)]
stun: Use libgcrypt to provide secure random number generation

Previously, a custom Mersenne Twister PRNG was used, which is not
securely random. In addition, its seeding fell back to wall-clock time,
which is typically predictable.

This uses libgcrypt on Linux but retains the Windows code which uses the
Windows crypt API.

Differential Revision: https://phabricator.freedesktop.org/D1610

7 years agostun: Add libgcrypt dependency
Philip Withnall [Mon, 7 Mar 2016 09:27:00 +0000 (09:27 +0000)]
stun: Add libgcrypt dependency

This will shortly be used to implement secure random number generation.

Differential Revision: https://phabricator.freedesktop.org/D1609

7 years agostun: Remove outdated tests from test-hmac
Philip Withnall [Mon, 7 Mar 2016 10:53:47 +0000 (10:53 +0000)]
stun: Remove outdated tests from test-hmac

The SHA-1 and MD5 implementations in libnice are about to be removed, so
stop testing them explicitly. In addition, rework the remaining test to
use the stun_sha1() API which will remain.

Differential Revision: https://phabricator.freedesktop.org/D1608

7 years agostun: Fix cast-align compiler warning when casting sockaddr
Philip Withnall [Fri, 3 Mar 2017 10:19:20 +0000 (10:19 +0000)]
stun: Fix cast-align compiler warning when casting sockaddr

There should never be a problem with alignment at runtime, since we’re
casting the sockaddr to sockaddr_in or sockaddr_in6 based on its
declared sa_family — anything declared as AF_INET6 should have been
allocated as a sockaddr_in6, and hence have appropriate alignment (same
for AF_INET).

This fixes a compiler warning on ARM and other alignment-sensitive
architectures.

https://phabricator.freedesktop.org/T7718

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Reviewed-by: Olivier Crête <olivier.crete@collabora.com>
Differential Revision: https://phabricator.freedesktop.org/D1686

7 years agobuild: Add default-reviewers key to .arcconfig
Philip Withnall [Tue, 17 Jan 2017 00:03:45 +0000 (00:03 +0000)]
build: Add default-reviewers key to .arcconfig

This should cause all patches submitted using git-phab to be assigned to
the #libnice project for review, by default.

Signed-off-by: Philip Withnall <philip@tecnocode.co.uk>
7 years agoms-ice: limit legacy connchecks as per [MS-ICE2] 3.1.4.8.2
Jakub Adam [Wed, 29 Jun 2016 06:40:27 +0000 (06:40 +0000)]
ms-ice: limit legacy connchecks as per [MS-ICE2] 3.1.4.8.2

Client should stop sending connectivity checks with legacy FINGERPRINT
when it receives a conncheck message containing IMPLEMENTATION-VERSION
attribute.

Differential Revision: https://phabricator.freedesktop.org/D1139

7 years agoms-ice: legacy FINGERPRINT mode
Jakub Adam [Wed, 29 Jun 2016 06:40:12 +0000 (06:40 +0000)]
ms-ice: legacy FINGERPRINT mode

In order to preserve compatibility with clients which use custom CRC
lookup table from [MS-ICE2], whenever a connectivity check request or
reply is sent, an additional message is sent along. These two messages
differ only in FINGERPRINT attribute - one uses regular CRC lookup table
for calculation, the other uses the modified table.

When a message is received and FINGERPRINT doesn't pass validation using
regular CRC table, the receiver also tries to verify using the modified
table.

[MS-ICE2] 3.1.4.8.2 describes this procedure.

The commit fixes compatibility with older MSOC and Lync clients.

Differential Revision: https://phabricator.freedesktop.org/D1138

7 years agostun: add STUN_COMPATIBILITY_MSICE2
Jakub Adam [Wed, 29 Jun 2016 06:39:50 +0000 (06:39 +0000)]
stun: add STUN_COMPATIBILITY_MSICE2

Windows Live Messenger is a discontinued service. We can repurpose
STUN_COMPATIBILITY_WLM2009 as [MS-ICE2] compatibility.

The orignial WLM enumerator is kept for the sake of API compatibility.

Differential Revision: https://phabricator.freedesktop.org/D1137

7 years agoms-ice: calculate FINGERPRINT according to [MS-ICE2]
Jakub Adam [Wed, 29 Jun 2016 06:39:22 +0000 (06:39 +0000)]
ms-ice: calculate FINGERPRINT according to [MS-ICE2]

Connectivity checks that are fully conforming to [MS-ICE2] should
contain IMPLEMENTATION-VERSION attribute ([MS-ICE2] 2.2.2.2) equal to 2
and their FINGERPRINT should be calculated as described in RFC5389
section 15.5 (i.e. using standard CRC lookup table).

We need this because some Skype for Business clients no longer accept
messages whose FINGERPRINT contains a value calculated using Microsoft's
old custom CRC table (specified verbatim in [MS-ICE2] 3.1.4.8.2).

The change creates a compatibility breakage with legacy Lync clients
which will be fixed in following commits.

Differential Revision: https://phabricator.freedesktop.org/D1136

7 years agostun: add STUN_USAGE_ICE_COMPATIBILITY_MSICE2
Jakub Adam [Wed, 29 Jun 2016 06:39:02 +0000 (06:39 +0000)]
stun: add STUN_USAGE_ICE_COMPATIBILITY_MSICE2

Windows Live Messenger is a discontinued service. The only users of WLM
mode seem to be Lync clients, so STUN_USAGE_ICE_COMPATIBILITY_WLM2009
can be repurposed as [MS-ICE2] compatibility.

We keep the WLM enumerator for the sake of API compatibility.

Differential Revision: https://phabricator.freedesktop.org/D1135

8 years agoconncheck: invoke the debug dump in more places
Fabrice Bellet [Tue, 12 Apr 2016 11:20:38 +0000 (13:20 +0200)]
conncheck: invoke the debug dump in more places

Differential Revision: https://phabricator.freedesktop.org/D1123

8 years agoconncheck: fix pair socket assignment
Fabrice Bellet [Tue, 14 Jun 2016 18:44:55 +0000 (20:44 +0200)]
conncheck: fix pair socket assignment

This patch fixes a problem when a new pair having a peer-reflexive new
remote candidate is added while the transport type is udp. In this case
the new pair socket really should be the socket of the local candidate,
and not the remote (for example, the local candidate may be of relayed
type).

Differential Revision: https://phabricator.freedesktop.org/D1122

8 years agoconncheck: fix tick counter used for debug stats
Fabrice Bellet [Tue, 21 Jun 2016 19:32:11 +0000 (21:32 +0200)]
conncheck: fix tick counter used for debug stats

The tick counter variable used to display pairs statistics should be
per stream defined, to avoid side effects of a global variable, for
example always having an odd or even tick counter value when the agent
contains just two streams.

Differential Revision: https://phabricator.freedesktop.org/D1113

8 years agoagent: remove unused agent property
Fabrice Bellet [Tue, 21 Jun 2016 19:36:28 +0000 (21:36 +0200)]
agent: remove unused agent property

Differential Revision: https://phabricator.freedesktop.org/D1112

8 years agotest-turn: wait for gathering done sequentially
Fabrice Bellet [Tue, 21 Jun 2016 21:58:20 +0000 (23:58 +0200)]
test-turn: wait for gathering done sequentially

Fixes a bug in the logic of the wait loop, where only a single
gathering done was required to exit the loop, the other was caught
by the following assert.

Differential Revision: https://phabricator.freedesktop.org/D1110

8 years agoagent: Don't ask upnp to remove not yet allocated candidates
Olivier Crête [Tue, 21 Jun 2016 20:41:50 +0000 (16:41 -0400)]
agent: Don't ask upnp to remove not yet allocated candidates

This caused a warning in Farstream tests.

8 years agoagent: read from the correct TCP-TURN socket
Jakub Adam [Tue, 21 Jun 2016 08:43:49 +0000 (08:43 +0000)]
agent: read from the correct TCP-TURN socket

fileno of UDP-TURN NiceSocket is NULL since 0a6c779f and so we need
different means to identify the topmost socket.

Differential Revision: https://phabricator.freedesktop.org/D1100

8 years agoconncheck: dump timer status in the stream check list
Fabrice Bellet [Tue, 14 Jun 2016 18:34:52 +0000 (20:34 +0200)]
conncheck: dump timer status in the stream check list

Instead of printing the static pair priority values, it provides more
information to dump each pair timer state, next timeout, and
retranmission count, when debugging the whole connchecks list
content.

8 years agoconncheck: use strncmp instead of strcmp
Fabrice Bellet [Tue, 19 Apr 2016 18:46:34 +0000 (20:46 +0200)]
conncheck: use strncmp instead of strcmp

Differential Revision: https://phabricator.freedesktop.org/D931

8 years agotests: fix io-stream when built with optimizations
Fabrice Bellet [Thu, 16 Jun 2016 18:51:19 +0000 (20:51 +0200)]
tests: fix io-stream when built with optimizations

In construct like "while (foo);" when foo is modified outside of the
current thread, the variable should be declared volatile to suggest the
compiler to read its value without making code optimization.

8 years agostun: avoid expensive call to sprintf in debug-related code
Fabrice Bellet [Mon, 8 Feb 2016 19:20:06 +0000 (20:20 +0100)]
stun: avoid expensive call to sprintf in debug-related code

8 years agoagent: rework gathering failures on auto-generated IPs
Fabrice Bellet [Tue, 7 Jun 2016 08:52:02 +0000 (10:52 +0200)]
agent: rework gathering failures on auto-generated IPs

This patch reworks commit fc4d3aa "ignore gathering failures on
auto-generated IPs", that introduces a regression in the test-fullmode
check, when turn is on and use_loopback is off. The part of the test
that fails is when nice_agent_gather_candidates (ragent...) should
return false when the port range for the second component is already
busy, line 385.

In this case, agent->local_address is null, so the code path added by
commit fc4d3aa is taken, and the function will return true, even when
not local address has been gathered.

The proper fix is to swap the inner and outer loops (on components, and
on local addresses), and to go to error when all local addresses of a
given component have failed, and to return false only in this case.

8 years agoconncheck: state is connected when a pair is nominated
Fabrice Bellet [Mon, 6 Jun 2016 20:35:36 +0000 (22:35 +0200)]
conncheck: state is connected when a pair is nominated

This patch fixes is bug introduced in commit 1ab9d7c "conncheck:
Separate valid and succeded states", with the introduction of the valid
flag. The agent really should go to connected state when we have a
nominated pair, and not just a valid one.

8 years agotest-send-recv: reduce deadlock timeout
Fabrice Bellet [Thu, 16 Jun 2016 18:58:41 +0000 (20:58 +0200)]
test-send-recv: reduce deadlock timeout

8 years agopseudotcp-fuzzy: fix this test
Fabrice Bellet [Thu, 16 Jun 2016 22:31:48 +0000 (00:31 +0200)]
pseudotcp-fuzzy: fix this test

The header size should be 24 bytes only, if we don't want to fuzz the
payload too.  Moreover, the default lambda parameter is decreased to
one, to not fuzz the header too heavily, and consequently increase too
much the time for the test to complete, due to exponential
retransmission timeout when packets are corrupted.

8 years agopseudotcp: accept several FIN segments
Fabrice Bellet [Thu, 16 Jun 2016 22:28:16 +0000 (00:28 +0200)]
pseudotcp: accept several FIN segments

This modification allows to gracefully recover from a first
corrupted FIN segment.

8 years agobuild: fix build in alternate builddir
Fabrice Bellet [Tue, 19 Apr 2016 18:41:27 +0000 (20:41 +0200)]
build: fix build in alternate builddir

Differential Revision: https://phabricator.freedesktop.org/D930

8 years agoconncheck: Remove pairs before freeing candidate
Olivier Crête [Mon, 6 Jun 2016 22:31:22 +0000 (18:31 -0400)]
conncheck: Remove pairs before freeing candidate

Remove the whole pair before the candidate is
to be freed.

https://phabricator.freedesktop.org/T7460

8 years agostun timer: Do 7 retransmissions as recommended
Olivier Crête [Fri, 19 Feb 2016 20:01:03 +0000 (15:01 -0500)]
stun timer: Do 7 retransmissions as recommended

Also reduce the normal timeout to make the test bearable.

This is what RFC 5389 section 7.2.1

Differential Revision: https://phabricator.freedesktop.org/D1056
Maniphest Task: https://phabricator.freedesktop.org/T3339

8 years agotimer: Maximum retransmission should include the original one
Olivier Crête [Mon, 6 Jun 2016 20:21:54 +0000 (16:21 -0400)]
timer: Maximum retransmission should include the original one

We really care about the maximum transmissions, the first one counts.

8 years agopseudotcp: it's still a GObject
Olivier Crête [Fri, 3 Jun 2016 22:42:59 +0000 (18:42 -0400)]
pseudotcp: it's still a GObject