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

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

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

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

agent: Ignore remote candidate of non-accepted types

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

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.

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

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

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

test-credentials: Fix leak

debug: Use libnice-verbose, not libnice-nice-verbose

tests: Use automake test-driver for valgrind

This fixes the valgrind integration with the new test drivers.

agent: Remove impossible case

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.

udp-turn: Add some const to internal APIs

Make clang-analyzer happy

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

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.

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

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

Version 0.1.14.1

Version 0.1.14

README: Depends on GLib 2.44

stun: Make hmac code NDEBUG safe

stun: Remove double const on int

candidate: Test against possible type

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

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.

stun: Use unions fix alignment issues

This makes clang happy.

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.

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

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

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

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

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

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

configure: gcry_mac_* were added in 1.6

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

Makefile: Build tests before docs

doc: Improve gtkdoc-check environment

doc: Add missing symbol

configure: Actually require gtk-doc 1.10 for no-tmpl

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

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

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

stun: Add libgcrypt dependency

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

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

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

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

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>

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

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

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

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

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

conncheck: invoke the debug dump in more places

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

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

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

agent: remove unused agent property

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

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

agent: Don't ask upnp to remove not yet allocated candidates

This caused a warning in Farstream tests.

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

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.

conncheck: use strncmp instead of strcmp

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

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.

stun: avoid expensive call to sprintf in debug-related code

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.

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.

test-send-recv: reduce deadlock timeout

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.

pseudotcp: accept several FIN segments

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

build: fix build in alternate builddir

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

conncheck: Remove pairs before freeing candidate

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

https://phabricator.freedesktop.org/T7460

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

timer: Maximum retransmission should include the original one

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

pseudotcp: it's still a GObject

pseudotcp: Make sure duplicate ack representing losses have no data

If they have data in them, they won't be recognized as duplicate acks by
the sender.

pseudotcp: Implement NewReno timestamp heuristic

This allows the sender to enter fast retransmit after a timeout because
it can now detect that three duplicate acks are caused by a packet loss.

As specific in RFC 6582 section 4.2.

pseudotcp: Set min RTO to 1 second

This is recommended by RFC 6298

pseudotcp: Implement full NewReno

pseudotcp: Make debug more useful

pseudotcp: Separate default and maximum MTU

Accept packets much beyond the default MTU, but
set a reasonable default MTU for sending of 1400

pseudotcp: close local socket on initial transmission error

This is required as no retransmissions will happen

pseudotcp: Export more symbols for PseudoTCP

pseudotcp: Make structs definitions private

pseudotcp: Correct behaviour of buffer size methods when part-closed

Correct the behaviour of pseudo_tcp_socket_get_available_bytes() and
pseudo_tcp_get_available_send_space() when the socket is not in
TCP_ESTABLISHED state. It’s still permissible to send and receive up
until the local side calls pseudo_tcp_socket_close(), which means we
may be in state TCP_ESTABLISHED *or TCP_CLOSE_WAIT*.

pseudotcp: Fix EOS checks in high packet loss situations

The state tracking previously assumed that if a FIN packet was sent, the
other side received it and the preceding packets, and hence it was
correct to sent an RST if an unexpected packet (such as a delayed
SYN-ACK) was received.

In cases where there is high packet loss, this won’t work. For example,
peer A sends a SYN, it is received and peer B replies with a SYN-ACK
which is also received; then peer A sends its data and a FIN, which are
both dropped. Since it hasn’t received anything since the original SYN,
peer B resends its SYN-ACK. If that is received, peer A was incorrectly
treating it as an erroneous packet, and would then send a RST. In actual
fact, it should take this as a signal that the data and FIN packets were
dropped, and should resend them.

TODO: Add unit tests

pseudotcp: Propagate error codes from transmit() to callers

Otherwise we can’t easily differentiate between different transmission
failures; for example: underlying socket failures, versus retransmission
timeouts.

pseudotcp: Add more debug info on closing down a pseudo-TCP socket

pseudotcp: Fix pseudo_tcp_socket_recv() in state TCP_CLOSE_WAIT

Previously, pseudo_tcp_socket_recv() would start returning 0 (EOS) as
soon as a FIN segment was received from the peer, even if there was
unread data already in the receive buffer.

Instead, the unread data should all be accessible before
pseudo_tcp_socket_recv() starts returning 0.

pseudotcp: Fix retransmission of segments before handling a FIN

Previously, if peer A transmitted one or more data segments (1),
followed by a FIN segment (2) to peer B, and segments 1 were
dropped, peer B would not request retransmission of them and would
instead continue with the FIN handshake. This effectively meant
segments 1 were lost without peer B realising.

Fix this by only handling the FIN segment once its sequence number is
acknowledged in the receive window.

socket: Assert trying to use free'd socket

Cleanly returnign makes no sense and may hide
worse problems.

component: Fix set TCP selected remote candidate

https://phabricator.freedesktop.org/T7407

agent: Parse TURN packet on the right socket

https://phabricator.freedesktop.org/T99

tests: Add TURN test

This test depends on rfc5766-turn-server which must
be installed for this test to run.

conncheck: mark discovered pairs with TCP passive as valid

Doing so similarly to priv_process_response_check_for_reflexive(),
which also sets valid flag on discovered peer reflexive pairs.

Fixes a regression in previously working scenario.
Differential Revision: https://phabricator.freedesktop.org/D1035

test-icetcp: don't be sensitive to the signal order

"new-selected-pair" may be emitted after "component-state-changed"
to READY, by which time the main loop might have gotten quit in
cb_component_state_changed(). Consequently, cb_new_selected_pair() could
miss to register the selected pair, ultimately leading to an assertion
failure in main().

We should wait for both selected pair and state change events to occur
before stopping the main loop.

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

Revert "WIP"

This reverts commit 01519677ba4d8df46e2c07bc20a5ef03ee2d9c3a.

WIP

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

agent: Remove socket on read error

If a socket returned an error, remove it.

component: Add API to cleanly remove a base socket

agent: Fix udp-turn-over-tcp

The TCP-based turns don't come pre-parsed unlike
the UDP variants!

agent: Add force-relay property to force messages through the relay

This allows implementing WebRTC privacy mode.