Tobias Brunner [Wed, 13 Mar 2024 14:34:48 +0000 (15:34 +0100)]
unit-tests: Point out if ECDSA public key was rejected after private keys was not
AWS-LC rejects public keys with explicitly encoded parameters but allows
private keys that use explicit encodings of the NIST curves. Since the
more important aspect is that public keys are rejected, this addition to
the warning message points that out.
Tobias Brunner [Fri, 23 Feb 2024 16:44:44 +0000 (17:44 +0100)]
constraints: Properly validate name constraints according to RFC 5280
The previous code was in a way too simple which resulted in it being too
strict. For instance, it enforced that intermediate CA certificates
inherited the name constraints of their parents. That's not required by
RFC 5280 and prevented e.g. adding constraints in an intermediate CA
certificate that's followed by another that doesn't contain any
name constraints. That's perfectly fine as the set of constraints
specified by the parent continue to apply to that CA certificate and
the children it issues.
Name constraints were previously also applied to all identities of a
matching type, which is way too strict except for some very simple
cases. It basically prevented multiple constraints of the same type
as e.g. an intermediate CA certificate that has permitted name constraints
for example.org and example.com couldn't issue acceptable certificates
because any SAN with one domain would get rejected by the other
constraint. According to RFC 5280 matching one constraint is enough.
Also fixed is an issue with name constraints for IP addresses which were
previously only supported for a single level.
Gerardo Ravago [Thu, 22 Feb 2024 18:47:58 +0000 (13:47 -0500)]
github: Add AWS-LC CI job
AWS-LC is an OpenSSL derivative which can be used with the openssl plugin.
This adds a CI job that resembles the openssl-3 test case. It downloads
the source tarball for an AWS-LC release, builds that source using
CMake/Ninja, and then builds/tests strongSwan using the same technique
used by openssl-3.
Etay Bogner [Wed, 6 Mar 2024 22:40:51 +0000 (00:40 +0200)]
starter: Use correct type for uniqueids field
Enum arguments (ARG_ENUM with .list != LST_bool) are assumed to be of
type/size int in assign_args() in args.c.
Fixes: 0644ebd3de62 ("implemented IKE_SA uniqueness using ipsec.conf uniqueids paramater additionally supports a "keep" value to keep the old IKE_SA")
Closes strongswan/strongswan#2148
Gerardo Ravago [Wed, 6 Mar 2024 15:45:00 +0000 (10:45 -0500)]
leak-detective: Add whitelist entries for AWS-LC
AWS-LC (and likely BoringSSL) uses thread specific data to store internal
library state which gets freed via a registered destructor when the thread
terminates. If this thread happens to be the main thread, which runs the
leak-detective evaluation, the detective won't observe the corresponding free
of the related memory and erroneously reports it as a leak.
The two places this happens are:
- `RAND_bytes` for storing internal RNG state.
- `ERR_put_error` for storing the per-thread OpenSSL error queue.
Gerardo Ravago [Mon, 4 Mar 2024 15:25:12 +0000 (10:25 -0500)]
openssl: Handle BoringSSL-style ASN1_INTEGERs in cert serials
OpenSSL stores the serial number for an X509 certificate as an
`ASN1_INTEGER` type. Within BoringSSL (and AWS-LC), the library
represents the value of zero as an empty array [1] which is different
from OpenSSL which represents it as the 1-byte array [0x00]. Though the
value of zero for the certificate serial number is illegal under
X.509 [2], we need to handle/encode it consistently within strongSwan.
From 18082ce2b061 ("certificates: Retrieve serial numbers in canonical
form"), we infer that the canonical representation of the zero serial
is [0x00]. To do this, we introduce `openssl_asn1_int2chunk` to
complement the existing string version that allows us to handle the
special case for zero instead of always returning a reference to the
library-dependent encodings.
Since ESN was negotiated via proposal, just configuring the SA without
ESN won't work as the ICV will be incorrect if the peer enabled ESN
on its SA. While the Linux kernel currently doesn't support disabling
replay protection for SAs that use ESN, this at least gets users an
explicit error not just dropped packets, and it will automatically work
if the kernel supports this combination at some point.
Tobias Brunner [Fri, 19 Jan 2024 17:29:20 +0000 (18:29 +0100)]
android: Expose static instance for Application object
While it seems to be possible to cast Context.getApplicationContext()
to the application class, there really is no documented reason why that
should actually be the same object.
Gerardo Ravago [Tue, 20 Feb 2024 16:54:01 +0000 (11:54 -0500)]
openssl: Condition out unsupported curves for AWS-LC
AWS-LC lacks support for a number of elliptic curve algorithms so this
adds some conditional macros to avoid registering the related plugin
features. Support for curves ed448 and x448 is completely absent and are
not planned for implementation as they are no longer recommended for use.
While ed25519 is supported by the library, a single missing API for
ASN.1 DER encoding of its private keys is missing which prevents its
use in strongSwan. Future work may remove this limitation, but for now
we will disable the functionality.
Gerardo Ravago [Thu, 15 Feb 2024 15:42:36 +0000 (10:42 -0500)]
openssl: Add conditional macros around SHA_CTX for AWS-LC
AWS-LC is a BoringSSL-based libcrypto implementation. SHA_CTX is declared with
the hash data specified as an array rather than as a field in upstream OpenSSL.
Since AWS-LC builds against C99, we are unable to handle this with anonymous
unions like BoringSSL. The workaround I propose is to add these conditional
macros around the accessors within openssl_sha1_prf. After this change,
everything builds successfully with AWS-LC headers.
Tobias Brunner [Fri, 16 Feb 2024 13:04:45 +0000 (14:04 +0100)]
Merge branch 'ref-overflows'
Different users in the strongSwan code base use the refcount helpers to
allocate incrementing unique values. So far the risk of overflows for
these unsigned 32-bit values has been considered mostly theoretical, as
it requires a longer uptime and a lot of activity to hit such an overflow.
At least for the Netlink sequence numbers, this is not only theoretical,
though, and an overflow has been hit on a productive setup. Unfortunately,
the consequences are rather unpleasant, as the response with a zero
sequence number can't be matched to the request. This results in the
offending thread to block indefinitely while holding the Netlink mutex.
So add a helper to allocate incrementing unique identifiers that checks
for overflows and never returns 0. Use it for Netlink sequence numbers
and some other potential users affected, namely those allocating
IKE_SA/CHILD_SA unique identifiers, marks and interface identifiers.
Martin Willi [Fri, 16 Feb 2024 09:59:11 +0000 (10:59 +0100)]
child-sa: Handle refcount overflow for unique mark/if_id allocation gracefully
The refcount_t for allocating unique marks and interface IDs may overflow or
hit the special value for unique marks/if_ids, in the worst case not setting it
on CHILD_SAs that should have one.
As (potentially two) marks/if_ids are allocated only for newly created CHILD_SAs,
but not for rekeying, this not very likely. Still, if a setup uses
aggressive re-authentication and or re-creates CHILD_SAs every minute,
a gateway with 100'000 tunnels may hit the overflow within a month uptime.
CHILD_SA unique identifier allocation starts at 1. If the counter overflows,
a unique ID of 0 is assigned to an CHILD_SA, which may have unclear
consequences.
Overflowing the unique ID counter is theoretical for most setups, but on
a Gateway terminating 100'000 tunnels and rekeying CHILD_SAs every 60s
overflows the counter after a month uptime. So avoid a 0 unique identifier
by using ref_get_nonzero().
IKE_SA unique identifier allocation starts at 1. If the counter overflows,
a unique ID of 0 is assigned to an IKE_SA, which may have unclear consequences.
Overflowing the unique ID counter is theoretical for most setups, but on
a Gateway terminating 100'000 tunnels and rekeying the IKE_SA every 60s
overflows the counter after a month uptime. So avoid a 0 unique identifier
by using ref_get_nonzero().
Martin Willi [Wed, 10 Jan 2024 15:54:17 +0000 (16:54 +0100)]
kernel-netlink: Handle Netlink sequence number counter overflows gracefully
A refcount variable is used to allocate sequential unique identifiers for
Netlink sequence numbers, subject to overflows. The risk of an overflow
has so far not been considered practical, as it requires 2^32 netlink
requests.
It seems that this issue is not only theoretical. A host with thousands
of tunnels doing aggressive rekeying and/or aggressive status checking
(via vici list-sas) may trigger the overflow after a few weeks uptime.
The consequences are rather devastating: Once the refcount overflows, a
Netlink request is sent with sequence number 0. This request is answered
by the kernel, but can't be matched to the request, resulting in the error:
"received unknown netlink seq 0, ignored". Without Netlink timeouts, the
thread indefinitely waits for a response while holding the Netlink mutex,
bringing all threads to a halt.
So at all costs avoid zero sequence numbers. Also, start at sequence number
1 instead of the arbitrary 201, so the same range is used on start and after
an overflow.
Tobias Brunner [Tue, 12 Dec 2023 17:08:05 +0000 (18:08 +0100)]
github: Use NDK version in build.gradle to build OpenSSL
Also fix the path to the sdkmanager (the old one was removed in the latest
images and the incorrect path caused a weird sudo error) and install
Java 17 as that's necessary for newer versions of the Gradle plugin.
Tobias Brunner [Tue, 12 Dec 2023 18:41:47 +0000 (19:41 +0100)]
android: Replace PowerMock with mechanism provided by newer Mockito versions
PowerMock isn't maintained anymore and causes issues with newer Java
versions. We only used it to mock static methods, which Mockito now
supports as well. Instead of using the try-with-resources construct,
this uses a @Before and @After method so we don't have to change all the
test methods.
Tobias Brunner [Tue, 12 Dec 2023 16:19:18 +0000 (17:19 +0100)]
android: Update Gradle plugin and build scripts and dependencies
This also references the NDK via ndkVersion and replaces the custom
ndk-build tasks. It also replaces the deprecated compileSdkVersion and
increases it because dependencies of updated dependencies require that.
targetSdkVersion is not yet updated because there might be some work
required for Android 14 compatibility.
Tobias Brunner [Mon, 15 Jan 2024 14:14:46 +0000 (15:14 +0100)]
github: Use newer gperf version on macOS
The gperf version that's already available on the system generates
function declarations with K&R syntax (separate arguments) for which newer
compilers produce a warning as C23 doesn't support that syntax anymore.
Tobias Brunner [Mon, 15 Jan 2024 12:39:32 +0000 (13:39 +0100)]
unit-tests: Use function pointers to test generic return_* helper functions
These functions are declared without arguments, passing arguments to them
causes warnings such as the following with newer compilers:
passing arguments to 'return_null' without a prototype is deprecated in all versions of C and is not supported in C2x [-Werror,-Wdeprecated-non-prototype]
We only use them via function pointers, which doesn't trigger any warnings
and hopefully continues to work.
Tobias Brunner [Wed, 10 Jan 2024 10:17:58 +0000 (11:17 +0100)]
curl: Fix issue with printf checks in newer curl versions
Newer curl versions (as used on macOS via Homebrew) add attributes like
__attribute__ ((format(printf, a, b)))
to their `curl_*printf*` functions, which fails if we redefine `printf`
as e.g. `builtin_printf` (pulled in via library.h). We could disable
these checks via CURL_NO_FMT_CHECKS, but reordering the headers should
do the trick as well.
Tobias Brunner [Wed, 10 Jan 2024 17:04:32 +0000 (18:04 +0100)]
Suppress compiler warnings with specific bison and compiler combinations
Bison generates code that only increases the yynerrs counter, it's never
read. This causes a warning in newer compilers (in particular clang).
Newer versions of bison mark yynerrs with __attribute__((unused)), but
at least on FreeBSD 14 that's not yet available.
Tobias Brunner [Mon, 8 Jan 2024 15:05:20 +0000 (16:05 +0100)]
leak-detective: Add implementation of malloc_usable_size()
systemd seems to use this and if we indirectly use libraries provided
by it, which can e.g. happen via getgrnam_r() and nss-systemd, this may
be called on pointers returned by leak detective's malloc(), which will
not point to the original start of the block and cause a segmentation
fault.