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.

conncheck: Start conncheck on server reply if needed

This only really applies in the force relay mode where there are
no local candidates.

Replace g_malloc/g_new with g_alloca where possible

This should reduce the overhead a bit.

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

conncheck: add a debug dump of the whole stream check list

https://phabricator.freedesktop.org/D814

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

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

conncheck: Make previous commit compile

https://phabricator.freedesktop.org/T3324

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

agent: Restrict transitions to gathering

Only allow transitions to gathering from disconnected or
failed states.

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

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

candidate: Give lower priority to TCP relayed candidates

udp-turn: Fix binding timeout leak

conncheck: Update selected pair if necessary

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.

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.

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.

conncheck: Make very frequent debug verbose-only

debug: Enable based on G_MESSAGES_DEBUG

agent: Don't emit signal in the middle of recv call

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.

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.

stun turn usage: Only send the username if short term creds or nonce present

This is recommended by the STUN RFC 5389.

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.

conncheck: Stay READY if a new nominated pairs comes in

conncheck: Deduplicate conncheck stopping code

Reset to connecting if reconnected after failed

agent: Add warning on ignored result

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

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

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

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

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

agent: Fix argument order

Fixes crash reported on https://phabricator.freedesktop.org/D786

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

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

build: Update autogen.sh from GNOME template

https://wiki.gnome.org/Projects/GnomeCommon/Migration#autogen.sh

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

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

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

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

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

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

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

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

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

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

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>

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

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

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

build: Set repository callsign in .arcconfig

This fixes `arc diff` to select the right repository when submitting
patches.

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

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