Tobias Brunner [Tue, 25 Aug 2020 11:54:08 +0000 (13:54 +0200)]
tls-crypto: Filter TLS cipher suites by min/max version
There is no point proposing legacy (or future) cipher suites depending on
the proposed TLS versions. It was actually possible to negotiate and use
cipher suites only defined for TLS 1.2 with earlier TLS versions.
bytinbit [Thu, 21 May 2020 07:37:38 +0000 (09:37 +0200)]
libtls: Implement TLS 1.3 handshake on client-side
The code is a minimal handshake with the HelloRetryRequest message
implementation missing.
Can be tested with an OpenSSL server running TLS 1.3. The server must
be at least version 1.1.1 (September 2018).
The key material, in particular the nonce/IV, is derived differently and
the IV is also generated in a different way. Additionally, the actual
content type is encrypted and there may be optional padding to mask the
actual size of the encrypted data.
Tobias Brunner [Thu, 11 Feb 2021 09:20:43 +0000 (10:20 +0100)]
appveyor: Set timezone on the build system
For some reason, setting the time zone via TZ to `GST-1GDT` in the utils test
doesn't work anymore (the DST zone is not considered, it's as if only `GST-1`
was configured).
Tobias Brunner [Wed, 10 Feb 2021 16:55:06 +0000 (17:55 +0100)]
openssl: Allocate our own buffer for i2d_* wrapper macro
If we pass a pointer to NULL, the memory allocated by OpenSSL has to be
freed with OPENSSL_free(). Otherwise, this can lead to random
crashes/freezes for Windows builds as seen on AppVeyor. To not
complicate things for callers of this macro, we allocate our own memory,
which we already do for other i2d_* calls.
Tobias Brunner [Fri, 18 Dec 2020 13:17:37 +0000 (14:17 +0100)]
openssl: Reset HMAC key if chunk_empty is passed
If no valid key is configured (e.g. because it's inadvertently uninitialized),
we should not just reuse the previous key.
The `key_set` flag is not necessary anymore because a non-NULL key is set
during initialization since 6b347d5232c7 ("openssl: Ensure underlying hash
algorithm is available during HMAC init").
Tobias Brunner [Tue, 24 Nov 2020 15:50:55 +0000 (16:50 +0100)]
android: Explicitly apply DNS servers to the TUN device
If the peer deletes the CHILD_SA, we recreate it due to the close
action. However, if we create a new TUN device, we do so with a new
VpnService.Builder object and on that the DNS servers were never applied.
The latter happened only on the fly in the attribute handler when an
IKE_SA was established. Now we do this explicitly when creating the TUN
device, like the virtual IPs and routes. While we could avoid the
recreation of the TUN device if the CHILD_SA is recreated, there is the
theoretical possibility that the remote traffic selectors change. This
way we also avoid adding stuff to the builder in different places.
Tobias Brunner [Wed, 27 Jan 2021 13:51:56 +0000 (14:51 +0100)]
swanctl: Don't print status message if nothing was loaded to stderr
This is not an error (as reflected by the returned status code) so we
should not print to stderr as output there might still be considered an
error (or at least an audit-worthy event) by some scripts.
Thomas Egerer [Thu, 28 Jan 2021 17:49:08 +0000 (17:49 +0000)]
plugin-loader: Add optional filter for plugin features
In some cases, the algorithms that have been compiled into a plugin have
to be disabled at runtime. Based on the array returned by the get_features()
function the optionally provided function can strip algorithms or even
callbacks or registrations from a plugin, giving us a handy and powerful way
for runtime feature configuration aside from the plugin list.
Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>
Tobias Brunner [Tue, 26 Jan 2021 18:33:24 +0000 (19:33 +0100)]
openssl: Fix potential crash with ECDH on Windows
Apparently, we should use OPENSSL_free() to release memory allocated by
OpenSSL. While it generally maps to free() that's apparently not the
case on Windows, where the ECP test vectors caused `ACCESS_VIOLATION
exception` crashes (not always the same vector).
Fixes: 74e02ff5e624 ("openssl: Mainly use EVP interface for ECDH")
Tobias Brunner [Wed, 20 Jan 2021 16:54:42 +0000 (17:54 +0100)]
Merge branch 'openssl-ecp'
Uses the EVP interface for ECDH with newer OpenSSL versions, which,
compared to the previous low-level use of EC_POINT_mul() supports
hardware offloading. We used this because of the ecp_x_coordinate_only
option, which is now removed as it's been obsolete for a long time and
complicated the code. There is still some legacy code for OpenSSL 1.0
and the old BoringSSL version we currently use for the Android app.
Tobias Brunner [Tue, 1 Dec 2020 10:45:05 +0000 (11:45 +0100)]
openssl: Mainly use EVP interface for ECDH
Functions like ECDH_compute_key() will be removed with OpenSSL 3 (which
will require additional changes as other functions will be deprecated or
removed too).
openssl: Use ECDH_compute_key() for 'x-coordinate only' setting
ECDH_compute_key() was not used because it only gives x-coordinate of
the result. However, the default setting, as per the errata mentioned,
is to use x-coordinate only.
Use ECDH_compute_key() for this setting as it additionally allows HW
offload of the computation using dynamic engine feature in OpenSSL.
EC_POINT_mul() doesn't allow HW offload.
Коренберг Марк [Wed, 15 Jul 2020 08:25:56 +0000 (13:25 +0500)]
identification: Change abbreviation for surname/serialNumber RDNs
To align with RFC 4519, section 2.31/32, the abbreviation for surname
is changed to "SN" that was previously used for serialNumber, which does
not have an abbreviation.
This mapping had its origins in the X.509 patch for FreeS/WAN that was
started in 2000. It was aligned with how OpenSSL did this in earlier
versions. However, there it was changed already in March 2002 (commit ffbe98b7630d604263cfb1118c67ca2617a8e222) to make it compatible with
RFC 2256 (predecessor of RFC 4519).
Tobias Brunner [Tue, 24 Nov 2020 08:24:25 +0000 (09:24 +0100)]
mem-pool: Be less strict when reassigning existing online leases
Also assign online leases to a peer connecting from the same endpoint
when it requests any virtual IP. This is mainly a workaround for
Windows clients that remember the virtual IPv6 address and re-request it
the next time the connection is initiated (even if it is not a
reauthentication) but don't do the same for virtual IPv4 addresses.
This can result in duplicate policies with different reqids because
these are allocated for unique sets of traffic selectors.
Tobias Brunner [Mon, 18 Jan 2021 12:31:01 +0000 (13:31 +0100)]
Merge branch 'ike-update-event'
This modifies the signature of the listener_t::ike_update() callback so
that both addresses are passed and it's only called once if both
addresses change (e.g. for an address family switch).
The callback is now also triggered for MOBIKE updates and the event is
exposed via vici.
Tobias Brunner [Tue, 27 Oct 2020 18:06:21 +0000 (19:06 +0100)]
ike-sa: Add flags to force updating hosts/CHILD_SAs
This allows more fine grained control over what's updated and does not
require multiple calls of the method. Plus we'll be able to use it in
the ike-mobike task.
Tobias Brunner [Tue, 15 Dec 2020 17:07:28 +0000 (18:07 +0100)]
testing: Add scenarios that use a CA with two intermediate CA certificates
Mainly to test TKM's ability for handling multiple CAs and that the
received intermediate CA certificates are passed in the right order.
But also added a regular scenario where two intermediate CA certificates
are sent by one of the clients.
Load CA certificate id mapping from config and pass the correct CA ID to
TKM when checking certificate chains. The mapping of CA certificate to
CA ID is done via SHA-1 hash of the CA certificates subjectPublicKey.
Andreas Steffen [Wed, 30 Dec 2020 09:16:57 +0000 (10:16 +0100)]
libimcv: Support symlinks introduced by usrmerge
Debian, Ubuntu, Fedora et. al. started to apply usrmerge to their
latest Linux distributions, i.e. /bin, /sbin, and /lib are now
symbolical links to /usr/bin, /usr/sbin, and /usr/lib, respectively.
Since executables and libraries are contained only once in Linux
packages (e.g. /bin/cp in coreutils but not /usr/bin/cp) this leads
to missing file measurments due to the symlinks when doing remote
attestation.
The new ita_attr_symlinks PA-TNC attribute fixes this problem by
collecting symbolic links pointing to directories on the client
platform.
Tobias Brunner [Mon, 14 Dec 2020 16:52:06 +0000 (17:52 +0100)]
Ignore verbose parser generator output file more generally
Depending on from where bison is called, the file might not end up in
the same directory as the .y file, but the location of the Makefile.
This has been seen on FreeBSD.
Tobias Brunner [Mon, 14 Dec 2020 10:36:21 +0000 (11:36 +0100)]
Replace two deprecated parser generator directives
There is a conflict between Flex's bison-bridge and Bison's api.prefix
options. Apparently, the former was added without consulting the Bison
devs and requires YYSTYPE, which is not added to the header anymore by
the latter. Instead, we just provide the proper definition of yyflex()
manually (as recommended by the Bison docs), so the option is not
required anymore.
Tobias Brunner [Thu, 26 Nov 2020 09:53:45 +0000 (10:53 +0100)]
github: Migrate from Travis CI to Github Actions
On travis-ci.com (travis-ci.org will be discontinued by the end of the
year) we are now charged for each minute. We only got 10000 credits in
a trial plan, which we used up with a few builds. Minutes also cost a
different amount of credits on different platforms: 10 on Linux,
but 50 on macOS (installing the dependencies on macOS alone took 12-15
minutes on Travis for some reason, takes about half on Github's runners).
No native Windows build yet as we have the same issue as on AppVeyor where
threading/streaming tests might get stuck. And there is also only a
single Windows platform to test on. Plus building/testing on Windows is
very slow (and getting ccache to work seems tricky).
The 'sw_collector' test case had to be disabled because we can't access
/usr/local/share on the Github build hosts (the process is just blocked
in readdir() and eventually times out).
Unfortunately, we can't test on different architectures anymore (in
particular ARM and the big-endian IBM Z/x390x).
Tobias Brunner [Thu, 3 Dec 2020 11:14:35 +0000 (12:14 +0100)]
imv-scanner: Fix potentially unsafe port filter attribute destruction
DESTROY_IF() checks if the given value is not NULL, before calling
destroy() on it, which does not work for sub-structs. If
port_filter_attr is NULL, this could crash.
Tobias Brunner [Tue, 1 Dec 2020 11:12:25 +0000 (12:12 +0100)]
child-rekey: Don't migrate child-create task if we already are deleting
If we are already deleting the old/redundant CHILD_SA, we must not
migrate the child-create task as that would destroy the new CHILD_SA we
already moved to the IKE_SA.
Tobias Brunner [Fri, 20 Nov 2020 14:02:30 +0000 (15:02 +0100)]
host-resolver: Don't wait for a reply if there are no threads
Without threads handling the resolution, there is no point waiting
for a reply. If no subsequent resolution successfully starts a
thread (there might not even be one), we'd wait indefinitely.
Tobias Brunner [Mon, 2 Nov 2020 14:09:13 +0000 (15:09 +0100)]
identification: Validate ASN.1 DN in from_data() constructor
The DN is otherwise not parsed until compared/printed. This avoids
false detections as ASN.1 DN if e.g. an email address starts with "0",
which is 0x30 = ASN.1 sequence tag, and the next character denotes
the exact length of the rest of the string (see the unit tests for an
example).
Tobias Brunner [Tue, 1 Dec 2020 14:54:35 +0000 (15:54 +0100)]
android: Don't default to marking VPN connections as metered
For apps targeting Android 10, where a method to change this was added, the
default changed so that all VPN connections are marked as metered. This means
certain background operations (e.g. syncing data) are not performed anymore
even when connected to a WiFi. By setting this to false, the metered state
of the VPN connection reflects that of the underlying networks.
Tobias Brunner [Thu, 29 Oct 2020 16:55:38 +0000 (17:55 +0100)]
testing: Add option to build all software recipes when building strongSwan
This is like building the root image but using a specific strongSwan
source tree, which is helpful if code changes depend on other software
packages (e.g. TKM-related or testing new crypto libraries). If the script
is called and the root image does not exist, the new option is enabled
automatically.
The option to build in a specific guest image is now also moved to an
explicit command line option so that the source dir path is the only
remaining positional argument (see --help for details).
Tobias Brunner [Mon, 9 Nov 2020 09:51:56 +0000 (10:51 +0100)]
testing: Improve building different revisions of Git-recipes
If we check out and build a certain revision of a dependency in a branch and
switch to another that requires a different revision and then switch back,
the previous approach installed the wrong revision as it would incorrectly
assume the required revision was already built and ready to install.
Tobias Brunner [Thu, 12 Nov 2020 13:20:04 +0000 (14:20 +0100)]
windows: Don't declare [v]asprintf()
None of our build environments seem to require these declarations. And
current versions of MinGW-w64 define them as inline functions in stdio.h
so these declarations clashed with that ("static declaration of '...'
follows non-static declaration").
Shmulik Ladkani [Mon, 2 Nov 2020 12:54:48 +0000 (14:54 +0200)]
controller: Always return SUCCESS when terminating IKE_SAs without callback
If no callback is specified, terminate_ike_execute() is invoked without the
listener waiting on the IKE state change.
Now, if 'force' is false, then ike_sa->delete() just queues an
IKE_DELETE task, and returns SUCCESS - indicating successful task
manager initiation.
However, terminate_ike_execute() ignored this success and set the
status to FAILED.
This is not ideal, as it will be the overall return code of
terminate_ike(), although no failure did occur. This eventually leads
vici's "terminate" to return "Command failed: terminating SA failed",
as seen in this example:
CommandException: Command failed: terminating SA failed
If we consider both queueing the task and actually destroying the IKS_SA
a success, we can just always return SUCCESS if we don't have a
callback. There is also no need to explicitly set the status to FAILED
if a listener is waiting as that's the default anyway.
Tobias Brunner [Tue, 3 Nov 2020 11:07:48 +0000 (12:07 +0100)]
gcrypt: Use a dummy buffer to initialize static allocations
In FIPS mode, libgcrypt uses a DRBG, which behaves differently when the
length passed to gcry_create_nonce() or gcry_randomize() is <= 0. It
expects a struct and explicitly checks that the passed pointer is not
NULL.