Version 0.1.8.1

Version 0.1.8

agent: Remove extra post condition

It prevent the GError from happening

udp-turn: Don't dereference priv before check that it's valid

agent: Pass all packets that are not acceptable STUN packets to the application

We attempted to not pass through some packets that looked like
valid STUN but were ot acceptable to us, but that dropped some application
packets.

agent: Re-emit gathering done after a new relay has been set

If the stream was ever gathering and a new relay has been set,
then re-emit the candidate=gathering-done signal

pseudotcp: Print unsigned int with %u not %d

agent: Add function to retrieve the current component state

agent: Make the stun-pacing-timer app configurable

There is no reason to only allow modifying it in the new().

pseudotcp: Set default RTO to 1 second

The newer RFC 6298 recommends 1 second instead of 3.

conncheck: Computer STUN retransmission timeout (RTO) dynamically

This is how it is specified in RFC 5245 section 16

discovery: Remove extra \n in g_debug()

agent: Only add pairs once on gathering done

Also don't try to re-add pairs that already have been added

conncheck: Insert the candidates sorted after setting the priority

conncheck: Insert peer reflexive pair sorted

The rest of the list is sorted, but this was just appended, making it likely
to be dropped if the list grows too long.

conncheck: Don't remove elements in the conncheck list while iterating it

priv_limit_conn_check_list_size() would remove elemtns from the conncheck_list
while the calling functions were iterating it. Now instead just mark them as
cancelled. Then later, at the outer function, free all cancelled elements to
prevent the list from growing out of bounds.

agent: Avoid leaking UPnP mappings between streams

Clean up the UPnP mappings of each stream when closing it

agent: Avoid restarting the GUPnP client on every gather

This would cause mappings to be dropped on every new gather, which is bad!
Instead, keep the same one with the mappings, and just drop the timer to ignore
new discovered mappings afterwards.

agent: Ignore UPnP mappings after the UPnP discovery has finished

component: On ICE restart reset selected pair priority to 0

This is to ensure that the result of the new negotiation will always
have priority.

agent: Add some preconditions to public API

To try and avoid NULL NiceCandidates entering our internal state.

build: Update .gitignore

outputstream: Fix double unref

agent: Remove socket source on HUP

agent: Declare the stream to be CONNECTED only if a pair is selected

Don't change the state if no pair is selected. Otherwise we get
a component that claims to be CONNECTED but has no selected pair.
Also, set the selected pair before announcing the state change.

nice.pc: Include ${includedir} directly

It doesn't make sense to tell applications to include <agent.h>,
including <nice/agent.h> is much safer. So tell the applications
to use the includedir directly.

agent: Document interaction between closing and removing streams

GIOStream and nice_agent_remove_stream() interact slightly subtly when
closing down the stream.

See: https://bugzilla.gnome.org/show_bug.cgi?id=735754

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.