tests: Add more helgrind suppressions

agent: Delay some assignments until the agent is locked

This makes helgrind slightly happier.

agent: Warn if a Component is freed without first being closed

agent: Close Streams before freeing them

This could result in leaking the stream’s un-closed state otherwise, in
any case where a NiceAgent is finalised without
nice_agent_remove_stream() being called on all its streams.

tests: Add a Valgrind suppression file

To be used with `make check-valgrind`.

tests: Fix various memory leaks in the unit tests

The theory being that if we can get check-valgrind to pass, we could
acutally use it to find more memory leaks and other errors in the agent.

agent: Check STUN buffer is non-NULL before getting its message ID

This should fix a crash, as detected by Valgrind:
  ==28354== Invalid read of size 2
  ==28354==    at 0x4C2B5B0: memcpy@@GLIBC_2.14 (in
    /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
  ==28354==    by 0x50C17E2: stun_message_id (stunmessage.c:658)
  ==28354==    by 0x509E4E7: candidate_check_pair_fail (conncheck.c:254)
  ==28354==    by 0x50A4EDB: conn_check_prune_socket (conncheck.c:3145)
  ==28354==    by 0x509B6F8: component_io_cb (agent.c:3951)

agent: Fix a leak of a GCancellable and its GSource

The GSource holds a reference to the GCancellable, so needs to be
explicitly removed from the GMainContext when it’s finised with.

agent: Fix a minor leak in an error handling path

tests: Move a closure from the heap to the stack

There is no need for this to be heap-allocated.

conncheck: Don't use CandidateRefresh after freeing it

socket: Fix a leak on the slow TURN packet handling path

agent: Eliminate a memset() for local variable initialisation

Stack variables can be initialised to zero by explicit assignment.

This introduces no functional changes.

agent: Use g_slist_free_full() to reduce code

This introduces no functional changes.

agent: Add some preconditions to internal API

To try and avoid NULL NiceCandidates entering our internal state.

agent: Add some preconditions to public API

To try and avoid NULL NiceCandidates entering our internal state.

agent: Clear GSource timeout also for GUPnP timeout

Fix regressions introduced by the following patch:
Clear existing GSource timeouts before adding new ones

agent: Drop valid but unmatched STUN packets

Rather than passing them through to the application. Invalid STUN
packets are still passed through, but it causes unnecessary noise and
corruption for higher-level applications to receive STUN packets they
weren’t expecting.

This is permitted by RFC 5389, §7.3.0:
    If any errors are detected, the message is silently discarded.
    In the case when STUN is being multiplexed with another protocol,
    an error may indicate that this is not really a STUN message; in
    this case, the agent should try to parse the message as a different
    protocol.
where I interpret ‘error’ to mean ‘validation error’ rather than (e.g.)
‘unrecognised attribute’ or ‘unmatched response’ where the STUN packet
is otherwise perfectly formed.

agent: Remove dangling pointers on NiceSocket destruction

If a NiceSocket is destroyed, various pointers are currently left
dangling to it in the conncheck state. These can cause crashes if (for
example) a CandidateCheckPair with such a dangling pointer is then used;
the GSocket methods will fail.

Fix this by explicitly removing the socket and all NiceCandidates which
wrap it from various areas of the state.

agent: Factor out state transition to FAILED for CandidateCheckPairs

This introduces no functional changes.

agent: Factor out free function for IncomingCheck

agent: Improve comments for container element types

To allow enhanced grepping for what structs point to other structs.

agent: Clear existing GSource timeouts before adding new ones

Modify agent_timeout_add_with_context() to force destroying and freeing
of an existing GSource before overwriting it with a new one (probably
with an updated timeout period).

This fixes a case in priv_map_reply_to_relay_refresh() where the TURN
candidate refresh timer was being overwritten with a new one, without
the old one being destroyed. This lead to two timeouts existing, only
one of which would be destroyed when the CandidateRefresh struct was
freed, leaking the other one (in the main context) and allowing it to be
later dispatched with a dangling CandidateRefresh pointer.

The modification to agent_timeout_add_with_context() should prevent this
happening in new code in future.

agent: Add names to timer GSources

Modify the agent_timeout_add_with_context() utility function to
automatically add names to the timer GSources it creates. This makes
them a little easier to identify when debugging.

socket: Return early from socket functions if the socket is closed

Explicitly check whether the socket is closed (universally represented
as sock->priv == NULL) before doing anything else in the socket methods.
This should safely return from unusual situations where the socket has
been closed and part-destroyed but still ends up having send() or recv()
called on it.

socket: Fix return values of socket_send_messages in socks5.c

The return value is the number of messages sent, or -1 on error — not a
boolean.

stun: Make a warning message more prominent

Since dropping a STUN message due to having insufficient buffer space in
libstun can cause ICE negotiation to fail. So we want it to be more
obvious in the logs.

agent: Put one StunAgent per Component

Otherwise we risk running out of space in the space limited saved ids
list. So the easiest way to do that is to put one StunAgent in each
component. It may be advisable to just give up on not allocating
memory inside libstun and just use a regular hash table in there
instead.

conncheck: Fetch stream once instead of twice

agent: Agent can reach EOS after having received something

The agent can reach EOS after performing one or more non-empty reads in
its read loop, and then hitting EOS on the underlying stream. That means
reached_eos is TRUE, but n_valid_messages is non-zero.

Weaken the postcondition to reflect this.

agent: Fix locking when calling component_free()

As mentioned in the previous commit message, component_free() requires
the agent lock to be released, but component_close() requires it to be
held. Fix the locking in nice_agent_remove_stream() to do that.

agent: Split component_free() into component_[close|free]()

Closing a component and freeing it are slightly conceptually different.
As it happens, freeing it can result in the disposal of the component's
NiceIOStream, which requires the agent lock to be released — but closing
the rest of the Component requires the agent lock to be held. Splitting
the function up simplifies locking handling in other parts of the code
(see next commit).

agent: Fix initialisation of the agent mutex for old GLib versions

The mutex is now non-recursive. This is left over from commit 1deee693.

tests: Fix test-build-io-stream for EOS changes

From commit 7b6935c66738d855c84fba291d47ece6ce1c43e2.

agent: Add missing bracket

Bad merge in commit 67179ffd8fb1a39936c54ce611e8deeec8705ac3.

agent: Add missing stream-closed checks for pseudo-TCP

This got lost in rebasing the FIN–ACK work. component->tcp is now only
NULL before the pseudo-TCP stream is initially created — afterwards, it
is always non-NULL, but pseudo_tcp_stream_is_closed() returns TRUE if
the stream has closed (gracefully or on errors). This allows
differentiation between the states: TCP support was never initialised;
and TCP support was initialised but is now closed.

agent: Return 0 on EOS from nice_input_stream_read()

We should return 0 instead of G_IO_ERROR_CLOSED for consistency with
recv() and read().

pseudotcp: Ensure shutdown member is not overwritten

Add a few safeguards to ensure that once priv->shutdown is set, it is
not overwritten with a different value.

pseudotcp: Fix EOS return from recv() in non-FIN–ACK mode

Change pseudo_tcp_socket_recv() to return 0 if in non-FIN–ACK mode and
the socket has been shut down. This makes the behaviour of FIN–ACK and
non-FIN–ACK modes consistent.

This is a behaviour change from before the introduction of FIN–ACK mode
— previously, pseudo_tcp_socket_recv() would return -1 and set the
ENOTCONN error if called after the socket had been shut down (i.e. after
pseudo_tcp_socket_get_next_clock() had returned FALSE). The new
behaviour will hopefully not break anything.

pseudotcp: Ensure socket is closed after shutdown in non-FIN–ACK mode

If the socket is closed (with pseudo_tcp_socket_close()) with FIN–ACK
unsupported, it should transition to the CLOSED state when
pseudo_tcp_socket_get_next_clock() returns FALSE (as that’s the
old-style indication that the socket has closed).

This is a behaviour change from before the introduction of FIN–ACK:
previously, the socket would stay in the ESTABLISHED state. However,
this should not have been easily detectable by consumers of the API,
as pseudo_tcp_socket_is_closed() did not exist then.

pseudotcp: Add a PseudoTcpSocket:support-fin-ack property

This allows FIN–ACK support to be disabled entirely. This is mostly for
testing purposes, since TCP_OPT_FIN_ACK is negotiated when establishing
the connection, and is disabled if the other side doesn’t support it.

This includes an interoperability test.

tests: Fix race conditions in test-thread

As found by tsan.

agent: Close pseudo-TCP streams when closing G[IO|Input|Output]Streams

This means that users of the Nice[Input|Output|IO]Stream API can easily
close TCP connections without having to hack around with libnice
internals.

docs: Ignore some undocumentable/non-public API

pseudotcp: Add pseudo_tcp_socket_shutdown() support

This is analogous to the UNIX shutdown() function, allowing either or
both sides of a pseudo-TCP connection to be shut down.

pseudotcp: Shorten the TIME-WAIT state timeout dramatically

The TIME-WAIT timeout is typically 2×MSL (on the order of 60 seconds),
which is needed to be able to reject delayed segments from closed
conversations. However, the underlying socket layer for pseudo-TCP takes
care of channel numbering so that segments don’t end up being sent to
closed conversations. Therefore, the TIME-WAIT state can be eliminated
(by shortening the timeout). The code for the state is kept around so
that the pseudo-TCP implementation matches the TCP specification
correctly, which will simplify maintenance.

tests: Add a new segment-by-segment test framework for pseudo-TCP

This explicitly removes all timers and main loop considerations from the
pseudo-TCP testing, which considerably simplifies arranging packet swaps
and mistimings for testing purposes.

This test suite includes a few tests for the FIN–ACK support.

pseudotcp: Add optional FIN–ACK and RST support

In order to detect cases where the peer closes its connection without an
explicit in-band close message (e.g. in protocols such as Telnet where
there is none), pseudo-TCP needs to grow support for a shutdown
handshake, following the TCP FIN–ACK specification. Arguably it should
have had this all along, but Jingle apparently doesn’t need it.

This adds support for FIN–ACK to the pseudo-TCP implementation. It is
backwards-compatible, only being used if the TCP_OPT_FIN_ACK option is
specified in the SYN segment.

If enabled, full-duplex closes are supported, and the standard method
for notifying a peer of the other end closing its connection (returning
0 from recv()) is used.

Also allow rapidly tearing down a connection, discarding unsent and
unreceived data, by sending an RST segment. This preserves the ability to
do a forced socket closure with pseudo_tcp_socket_close(sock, TRUE).

It also permits graceful socket shutdown in the case where the final ACK
is lost, and one peer gets stuck in the LAST-ACK state: that peer will
eventually re-transmit its FIN segment. The other peer, in the CLOSED
state, will respond with a RST segment, and the first peer will then
reach CLOSED.

References (most useful first):
 • http://tools.ietf.org/html/rfc793#section-3.5
 • http://tools.ietf.org/html/rfc1122#page-87
 • http://vincent.bernat.im/en/blog/2014-tcp-time-wait-state-linux.html
 • http://tools.ietf.org/html/rfc675
Diagram:
 •
http://en.wikipedia.org/wiki/Transmission_Control_Protocol#mediaviewer/File:TCP_CLOSE.svg

agent: Handle EPIPE from pseudo_tcp_socket_send()

As with send(), pseudo_tcp_socket_send() may return EPIPE if the local
side of the connection has been closed using close() or shutdown(). It
currently doesn’t, but will do once pseudo-TCP FIN–ACK support has been
implemented.

agent: Don’t clear the PseudoTcpSocket on error

Instead, keep the closed object around. This allows differentiation
between the states: TCP support was never initialised; and TCP support
was initialised but is now closed.

pseudotcp: Fix a typo in some documentation

pseudotcp: Factor out state change functions for ESTABLISHED and CLOSED

Both state changes need some follow-up code executed immediately after,
which was duplicated in a number of places. Factor that out. This
introduces no behavioural changes.

pseudotcp: Split out some state checks

This clarifies the code a little, and does not introduce functional
changes.

pseudotcp: Tidy up buffer size handling in queue_connect_message()

This removes some hard-coded offsets. It does not introduce any
functional changes.

pseudotcp: Store segment flags in SSegment

Rather than reconstructing the flag state when sending a segment, just
store the flags explicitly. This does not introduce any behavioural
changes.

pseudotcp: Allow the ‘current’ time to be set

This is needed for the upcoming new test suite for pseudo-TCP. It
shouldn’t be used in normal code — only in tests.

Ideally, the pseudo-TCP code should originally never have called
g_get_monotonic_time() itself, and should have always taken a time
parameter from the caller; then it would be more testable.
Unfortunately, API guarantees prevent this from being changed now.

pseudotcp: Add socket state to debug messages

This should make debugging a little clearer.

pseudotcp: Put TCP flags in an enum

The on-the-wire flags (FIN, RST, ACK, etc.) should be in an enum to
clarify the code a little. This introduces no functional changes.

pseudotcp: Put TCP options in an enum

This tidies things up a little. No functional changes.

tests: Improve debug output of pseudotcp tests slightly

tests: Add setlocale() call to test-pseudotcp.c

This ensures UTF-8 output is printed correctly.

tests: Add a logging domain for the unit tests

This clarifies the log output a little.

socket: Gracefully return from send() if the socket is closed

agent: Ensure Component.selected_pair is cleaned when freeing candidates

When freeing candidates (component_free_socket_sources()), the sockets
which back Component.selected_pair.[local|remote] are closed and their
addresses destroyed. Component.selected_pair should be cleared as well
to allow calling code to condition on (Component.selected_pair == NULL)
to see if it’s invalid.

agent: Factor out common clean up code in Component

This introduces no functional changes.

agent: Tidy up a few variable references

This simplifies the code a little. No functional changes.

agent: Rearchitect message handling to use GErrors for EWOULDBLOCK

Previously, an EWOULDBLOCK return value from the low-level socket calls
(including PseudoTcpSocket) would be represented by a zero number of
bytes (or messages) read by the agent. This conflicts with the use of
zero to represent end of stream (EOS) for pseudo-TCP connections, where
the sender has indicated that they are not going to send any more bytes.

So, now use GError (G_IO_ERROR_WOULD_BLOCK) to represent EWOULDBLOCK,
just like the GSocket functions. Zero is reserved exclusively for if:
 • the number of requested bytes/messages is zero; or
 • reliable mode is enabled and EOS is reached.

This does change the documented behaviour of the NiceAgent send/recv
API, but only by allowing a new behaviour (returning zero) rather than
by changing an existing one, so it should be OK.

pseudotcp: Fix the type of an argument to apply_option()

We’re dealing with unsigned bytes here, not chars. This will become
important when adding new TcpOptions numbered from 254 downwards.

pseudotcp: Add pseudo_tcp_socket_is_closed_remotely()

New convenience API to check if the remote end of a pseudo-TCP socket
has been closed (but not necessarily the local end). This is currently a
stub implementation, but will be used fully once pseudo-TCP FIN–ACK
support lands.

pseudotcp: Add pseudo_tcp_socket_is_closed()

New convenience API to check if the socket is in state TCP_CLOSED.

pseudotcp: Factor out state changes and validate them

By validating state changes, we can verify that the implementation
conforms to the RFC 793 state machine at runtime. This will become more
important when FIN–ACK support is added, adding more states.

agent: Factor out peer reflexive priority calculation

agent: Add "keepalive-conncheck" property to make keepalives be conncheck

This is to use keepalives to check if the connection is still ongoing. If
they don't get a reply, then we can assume that the connection has died.

This needs to not happen in the case of ICE-TCP.

nicesink: Check for signal present before removing it

The signal is only connected when setting an agent, if no agent
has been set, there will be no signal.

stun: Fix definition of ssize_t on Windows

ssize_t should definitely be signed, not unsigned.

build: Add README.win32 to EXTRA_DIST

As spotted by B Andrei on the mailing list.

socket: Remove dead code

At that point, (len > 0), since there are conditions above handling the
(len == 0) and (len < 0) cases, both of which break from the loop.

Coverity issue: #29325

nicesink: Block until component is writable if in reliable mode

Fix bug in the ice-tcp unit test

conncheck: Go to connected only if the component was connected or ready

Otherwise, it claimed it was connected when the component was in the FAILED
state, when it should really have gone to connecting.

outputstream: Fix ref leak

component: Fix ref leak

agent: Expand documentation for nice_interfaces_get_ip_for_interface()

Set candidate pairs from discovered peer-reflx tcp-passive candidates into DISCOVERED state so it doesn't fail if the other side doesn't reply to our binding request

Fix retransmission timers for reliable sockets
The retransmissions would timeout after 600ms which caused the TCP connections
to timeout before it's able to connect. TCP sockets need to timeout
after 8 seconds instead.

Revert "agent: Separate reliability from ice-tcp vs ice-udp"

This reverts commit 18e5dff4f25b12522e857c13d3ef3bdb40212246.

Change test-icetcp to be more robust against ready->connected->ready state changes

Cast the current time for pseudotcp timeout calculation into guint32
The pseudo_tcp_socket_get_next_clock returns the next clock in terms
of g_get_monotonic_clock / 1000, however, it gets cropped to guint32.
When we try to find the interval, by substracting the current time as
a guint64, it will return a negative value, which prevents an integer
overflow by setting the timeout to 0. This causes the notify_clock
timeout to be called all the time by the mainloop, taking 100% of CPU.

agent: Print libnice debug messages when setting G_MESSAGES_DEBUG appropriately

This way all the debug can be controlled from one place.

Also disable -Wsuggest-attribute=format with -Werror because gcc 4.8.3
erroneously suggested it.

pseudotcp: Use different g_log domain for pseudotcp

Also add one for pseudotcp-verbose

debug: Make debug messages ready for g_log()

Send thing one line at a time, so no explicit \n

Also make it possible to set a log handler explicitly

Fix timer for TURN CreatePermission.
The TURN CreatePermission is a list of multiple permissions but the timer
is created and overwrites the old one, so some of them don't get triggered
at the right time.
This patch was suggested by Livio Madaro. We create a timer for the minimal
amount of time, and trigger the retransmissions on the permissions that
timed out, then reschedule for the next retransmissions.

Check error code 438 for TURN Nonce expired

Do not access component if it was destroyed when calling the io callback
When calling component_emit_io_callback, the agent mutex will be unlocked before
the callback is called, this can cause another thread to remove the
stream and thus destroying the component. We must verify that the
source was not destroyed after calling component_emit_io_callback
and avoid accessing the component in that case.
Bug report by Stephan Thamm in the mailing list.

Use sockaddr for stun_usage_bind_process in unit test

agent: Use AC_CHECK_HEADERS to check for netdb.h

Rather than assuming it is not available on Windows.

interfaces: Add missing brackets

agent: Fix infinite loop when the peer closes a TCP connection remotely

tcp-passive: Relay the tcp writable callback up

Otherwise, the caller gets a callback from an unexpected source.

agent: Fix an old-style function definition in the Windows code