android: Make sure every listener at the VPN state service is only registered once
We register when the service connects but also in onStart() (as we
unregister in onStop() to avoid updates when not shown). So this could
theoretically cause the listener to get registered twice if the service
is connected before onStart() is called (it seems it usually isn't).
Tobias Brunner [Tue, 20 Jun 2017 10:50:36 +0000 (12:50 +0200)]
ikev1: Only delete redundant CHILD_SAs if configured
If we find a redundant CHILD_SA (the peer probably rekeyed the SA before
us) we might not want to delete the old SA because the peer might still
use it (same applies to old CHILD_SAs after rekeyings). So only delete
them if configured to do so.
Tobias Brunner [Wed, 29 Mar 2017 09:26:24 +0000 (11:26 +0200)]
gmp: Make sure the modulus is odd and the exponent not zero
Unlike mpz_powm() its secure replacement mpz_powm_sec() has the additional
requirement that the exponent must be > 0 and the modulus has to be odd.
Otherwise, it will crash with a floating-point exception.
Fixes: CVE-2017-9022 Fixes: 3e35a6e7a1b0 ("Use side-channel secured mpz_powm_sec of libgmp 5, if available")
Tobias Brunner [Fri, 26 May 2017 13:10:04 +0000 (15:10 +0200)]
sql: Remove redundant enumerator allocation
Interestingly, this doesn't show up in the regression tests because the
compiler removes the first assignment (and thus the allocation) due to
-O2 that's included in our default CFLAGS.
Tobias Brunner [Fri, 26 May 2017 14:24:35 +0000 (16:24 +0200)]
testing: Add wrapper around service command
When charon is started via service command LEAK_DETECTIVE_LOG is not set
because the command strips the environment. Since we only want the
variable to be set during the automated test runs we can't just set it
in /etc/default/charon. Instead, we do so in this wrapper when charon is
started and remove the variable again when it is stopped.
Tobias Brunner [Fri, 26 May 2017 11:57:57 +0000 (13:57 +0200)]
Merge branch 'variadic-enumerators'
This adds several changes to enumerator_t and linked_list_t to improve
portability. In particular to Apple's ARM64 iOS platform, whose calling
convention for variadic and regular functions are different. This means
that assigning a non-variadic function to a variadic function pointer,
as we did with our enumerator_t::enumerate() implementations and several
callbacks, will result in crashes as the called function will access the
arguments differently than the caller provided them.
To avoid this issue the enumerator_t interface is now fully variadic.
A new mandatory method is added, venumerate(), that takes a va_list with
the arguments provided while enumerating. enumerate() is replaced with
a generic implementation that prepares a va_list and calls the
enumerator's venumerate() implementation. As this allows passing the
arguments of one enumerator to another it avoids the five pointer hack
used by enumerator_create_nested() and enumerator_create_cleaner().
To simplify the implementation of venumerate() a helper macro is provided
that assigns values from a given va_list to local variables.
The signature of the callback passed to enumerator_create_filter() has
also changed significantly. It's now required to enumerate over the
original enumerator in the callback as this avoids the previous in/out
pointer hack. The arguments to the outer enumerator are provided in a
va_list.
Similar changes to avoid such five pointer hacks affect the signatures
of the callbacks for linked_list_t's invoke_function() and find_first()
methods. For the latter the return type also changed from status_t to
bool, which is important as SUCCESS is defined as 0, so checks for ==
SUCCESS will now fail.
Tobias Brunner [Tue, 9 May 2017 14:59:37 +0000 (16:59 +0200)]
enumerator: Add venumerate() method to enumerator_t that takes a va_list
This will allow us to implement e.g. enumerator_cleaner without having to
use that unportable 5 pointer forwarding or having to define a callback for
each instance.
A generic implementation for enumerate() is provided so only venumerate()
has to be implemented, which may be simplified by using the VA_ARGS_VGET()
macro.
Tobias Brunner [Fri, 26 May 2017 11:49:51 +0000 (13:49 +0200)]
testing: Fix ikev2/two-certs scenario
Since 6a8a44be88b0 the certificate received by the client is verified
first, before checking the cached certificates for any with matching
identities. So we usually don't have to attempt to verify the signature
with wrong certificates first and can avoid this message.
Tobias Brunner [Fri, 26 May 2017 09:23:12 +0000 (11:23 +0200)]
Merge branch 'sha-256-96'
Adds an option to locally configure 96-bit truncation for HMAC-SHA256
when negotiated using the official algorithm identifier. This is for
compatibility with peers that incorrectly use this shorter truncation
(like Linux does by default).
Tobias Brunner [Wed, 10 May 2017 17:15:53 +0000 (19:15 +0200)]
child-cfg: Optionally use 96-bit truncation for HMAC-SHA-256
The correct truncation is 128-bit but some implementations insist on
using 96-bit truncation. With strongSwan this can be negotiated using
an algorithm identifier from a private range. But this doesn't work
with third-party implementations. This adds an option to use 96-bit
truncation even if the official identifier is used.
Tobias Brunner [Tue, 23 May 2017 16:49:13 +0000 (18:49 +0200)]
Merge branch 'avoid-rekey-loss'
This changes the behavior during IKEv2 CHILD_SA rekeyings to avoid
traffic loss. When responding to a CREATE_CHILD_SA request to rekey a
CHILD_SA the responder already has everything available to install and
use the new CHILD_SA. However, this could lead to lost traffic as the
initiator won't be able to process inbound packets until it processed the
CREATE_CHILD_SA response and updated the inbound SA. To avoid this the
responder now only installs the new inbound SA and delays installing the
outbound SA until it receives the DELETE for the replaced CHILD_SA. The
messages transporting these DELETEs could reach the peer before packets
sent with the deleted outbound SAs reach the respective peer. To reduce
the chance of traffic loss due to this the inbound SA of the replaced
CHILD_SA is not removed for a configurable amount of seconds after
the DELETE has been processed.
Tobias Brunner [Tue, 21 Mar 2017 15:03:54 +0000 (16:03 +0100)]
child-delete: Delay the removal of the inbound SA of rekeyed CHILD_SAs
After deleting a rekeyed CHILD_SA we uninstall the outbound SA but don't
destroy the CHILD_SA (and the inbound SA) immediately. We delay it
a few seconds or until the SA expires to allow delayed packets to get
processed. The CHILD_SA remains in state CHILD_DELETING until it finally
gets destroyed.
Tobias Brunner [Wed, 1 Mar 2017 17:02:38 +0000 (18:02 +0100)]
ikev2: Delay installation of outbound SAs during rekeying on the responder
The responder has all the information needed to install both SAs before
the initiator does. So if the responder immediately installs the outbound
SA it might send packets using the new SA which the initiator is not yet
able to process. This can be avoided by delaying the installation of the
outbound SA until the replaced SA is deleted.
Tobias Brunner [Tue, 28 Feb 2017 14:03:12 +0000 (15:03 +0100)]
child-sa: Add methods that allow partial installation of CHILD_SA
Using install() for the inbound SA and register_outbound() for the
outbound SA followed by install_policies(), will delay the installation of
the outbound SA as well as the installation of the outbound policies
in the kernel until install_outbound() is called later.
Tobias Brunner [Tue, 23 May 2017 16:33:00 +0000 (18:33 +0200)]
Merge branch 'fuzzing'
Adds support for fuzzing the certificate parser provided by the default
plugins (x509, pem, gmp etc.) on Google's OSS-Fuzz infrastructure (or
generally with libFuzzer). Fixes several issues that were found while
fuzzing these plugins.
When building the libraries monolithically and statically the
plugin constructors are now hard-coded in each library so the plugin
code is not removed by the linker because it thinks none of their symbols
are ever referenced.
Tobias Brunner [Thu, 4 May 2017 14:16:33 +0000 (16:16 +0200)]
chunk: Correctly parse Base64 text where four = follow in a row
That's not correct Base64 but invalid data could trigger this. Since
outlen would get reduced four times, but is only ever increased three
times per iteration, this could result in an integer underflow and then
a potential buffer overflow.
Tobias Brunner [Wed, 3 May 2017 15:35:45 +0000 (17:35 +0200)]
processor: Move priority threads assignment to set_threads()
This avoids the evaluation of %N even if the thread pool is never used.
We need to avoid as many custom printf specifiers as possible when
fuzzing our code to avoid excessive log messages.
Add plugin constructor registration for all libraries that provide plugins
Unfortunately, we can't just add the generated C file to the sources in
Makefile.am as the linker would remove that object file when it notices
that no symbol in it is ever referenced. So we include it in the file
that contains the library initialization, which will definitely be
referenced by the executable.
This allows building an almost stand-alone static version of e.g. charon
when building with `--enable-monolithic --enable-static --disable-shared`
(without `--disable-shared` libtool will only build a version that links
the libraries dynamically). External libraries (e.g. gmp or openssl) are
not linked statically this way, though.