Commit Graph
19790 Commits
Author SHA1 Message Date
Tobias Brunner e2fa67e8fc gmp: Avoid crash and timing leaks in PKCS#1 v1.5 decryption padding validation
This fixes a potential crash due to a null-pointer dereference if rsadp()
returns NULL (e.g. with an all-zero ciphertext).

And it also implements the PKCS#1 v1.5 decryption padding check in
constant time.

The timing leak caused by the previous implementation was measured at
~17.5 μs at 3 GHz, which could allow a Bleichenbacher-like attack in
LAN environments.  However, because of how RSA encryption is used in
strongSwan, this is not that much of an issue in practice.  The mechanism
is only used for two use cases.  One is SCEP/EST via PKCS#7 enveloped
data.  Fortunately, this can not be triggered in significant numbers by
an attacker.  The other use case is TLS as used by EAP methods (EAP-TLS,
EAP-PEAP/TTLS) during the authentication.  While the cipher suites that
use RSA encryption are still enabled by default, the TLS messages are
wrapped in EAP and encrypted by IKE, making any kind of attack difficult.

Note that the gmp plugin isn't enabled anymore by default.  And even
before that, most setups had the openssl plugin enabled, which has
priority over the gmp plugin.  So it's unlikely the plugin was used in
practice.

Fixes: d615ffdcf3 ("implement gmp_rsa_private_key.decrypt()")
Fixes: CVE-2026-35334
2026-04-21 16:48:56 +02:00
Tobias Brunner 90fd268088 utils: Add constant-time comparison primitives for uint32_t 2026-04-21 16:48:56 +02:00
Tobias Brunner 8dae5605a7 pkcs7: Avoid NULL pointer dereference when verifying padding in enveloped content
As the previous issue, this can be triggered via IKEv1 CERT payloads.

Fixes: d7aa09104f ("Implement PKCS#7 enveloped-data parsing and decryption")
Fixes: CVE-2026-35329
2026-04-21 16:48:56 +02:00
Tobias Brunner c6a2185c42 pkcs5: Fail decryption if data is empty
This avoids the previous bug early.
2026-04-21 16:48:56 +02:00
Tobias Brunner 4da84019cc pkcs5: Avoid NULL pointer dereference when verifying padding
Can be triggered via empty PKCS#7 encrypted content in IKEv1 CERT payload.

Fixes: 4076e3ee91 ("Extract PKCS#5 handling from pkcs8 plugin to separate helper class")
Fixes: CVE-2026-35329
2026-04-21 16:48:56 +02:00
Tobias Brunner 56c7f0d13d tls-server: Prevent infinite loop if supported versions are too short
If the extension doesn't contain a multiple of two bytes, the previous
code would get stuck in an infinite loop as `remaining()` continued to
return TRUE while `read_uint16()` failed to parse a value. Initiating
several connections with such an extension allows a DoS attack as no
threads would eventually be available to handle packets/events.

Fixes: 7fbe2e27ec ("tls-server: TLS 1.3 support for TLS server implementation")
Fixes: CVE-2026-35328
2026-04-21 16:48:56 +02:00
Tobias Brunner 64130ede5c constraints: Reject excluded directoryName (DN) name constraints
There is an issue similar to the one fixed with the previous commit when
using directoryName (DN) name constraints.  Some RDNs have to be matched
in a case-insensitive manner, which we e.g. do in
`identification.c::rdn_equals`.  By not doing it for name constraints,
a malicious intermediate CA could evade an excluded name constraint
just by modifying the case in such an RDN.

While we could use the mentioned function in `dn_matches`, this doesn't
properly fix the problem because the function is basically too strict.
Especially in regards to RDNs of type UTF8String, which are only compared
binary.  To match these properly, we'd have to implement the string
preparation described in RFC 5280, section 7.1 and the referenced RFCs.
Until that's the case, we reject excluded name constraints of type
directoryName as we are unable to enforce them.

Fixes: a2b340764f ("Implemented NameConstraint matching in constraints plugin")
Fixes: CVE-2026-35331
2026-04-21 16:48:56 +02:00
Tobias Brunner c66143db48 constraints: Match FQDN and email addresses case-insensitively
The case is generally ignored when matching such identities.  So this is
an issue with excluded name constraints where a malicious intermediate
CA could evade the constraints by issuing certificates with names that
just modify the case (e.g. strongSwan.org instead strongswan.org).

Note that it's likely that permitted name constraints are preferred over
excluded name constraints as it might be difficult to come up with a
conclusive list of names to exclude.

Fixes: a2b340764f ("Implemented NameConstraint matching in constraints plugin")
Fixes: CVE-2026-35331
2026-04-21 16:48:56 +02:00
Tobias Brunner 1e0643bef1 tls-server: Only accept non-empty ECDH public keys with TLS < 1.3
This prevents a crash due to a null-pointer dereference when processing
an empty ECDH public key.

The previous length check only applied in the `!ec` case, so in the `ec`
case, the access to `pub.ptr[0]` was unguarded.  If a crafted TLS
record ends with an empty ClientKeyExchange, then `read_data8` sets
`pub` to `chunk_empty`, causing a null-pointer dereference.

Note that if some data follows the empty ClientKeyExchange, this just
causes a 1-byte out-of-bounds read that has no further effect as the
TLS session is aborted immediately.  Either because the read value
doesn't equal TLS_ANSI_UNCOMPRESSED or because the empty public key
is rejected by `set_public_key()`.

The referenced commit that introduced the pointer access, added the
check for `pub.len` specifically to the `!ec` case, while the pointer
access was initially unconditional (probably because the code was just
copied from `tls_peer.c` which processes ECDH public keys in a separate
function, so there was no `ec` flag).  The latter was fixed a couple of
days later with 7b3c01845f ("Read the compression type byte for EC
groups, only").  However, that commit didn't change the length check.
Anyway, it's possible that the original intention was to add the check
to the `ec` case on the previous line, or that there was some confusion
with the parenthesis and something like the current code was intended to
begin with.

Fixes: e6cce7ff0d ("Prepend point format to ECDH public key")
Fixes: CVE-2026-35332
2026-04-21 16:48:56 +02:00
Lukas Johannes Möller e067d24293 libradius: Reject undersized attributes in enumerator
attribute_enumerate() accepts RADIUS attributes whose length byte is
smaller than sizeof(rattr_t) (2).  For length == 0, the iterator never
advances and traps callers — including verify() — in a non-advancing
loop.  For length == 1, misaligned packed-struct reads occur.

Add a separate check for this->next->length < sizeof(rattr_t) after
the existing truncation guard.  This mirrors radius_message_parse(),
which already distinguishes invalid length from truncation.

Signed-off-by: Lukas Johannes Möller <[email protected]>

Fixes: 4a6b84a934 ("reintegrated eap-radius branch into trunk")
Fixes: CVE-2026-35333
2026-04-21 16:48:56 +02:00
Lukas Johannes Möller aa5aaebc33 libsimaka: Reject zero-length EAP-SIM/AKA attributes
parse_attributes() accepts hdr->length == 0 in the AT_ENCR_DATA,
AT_RAND, AT_PADDING, default branches. The code then subtracts the
fixed attribute header size from the encoded length, which underflows
and exposes a wrapped payload length to later code.  In particular,
for the cases where add_attribute() is called, this causes a heap-based
buffer overflow (a buffer of 12 bytes is allocated to which the wrapped
length is written).  For AT_PADDING, the underflow is irrelevant as
add_attribute() is not called. Instead, this results in an infinite loop.

Reject zero-length attributes before subtracting the attribute header.

Signed-off-by: Lukas Johannes Möller <[email protected]>

Fixes: f8330d0395 ("Added a libsimaka library with shared message handling code for EAP-SIM/AKA")
Fixes: CVE-2026-35330
2026-04-21 16:48:56 +02:00
Tobias Brunner 19d73ef156 github: Move CI for Windows from AppVeyor to GitHub Actions
These are quite a bit faster than on AppVeyor (with ccache about a fifth,
without less than half - and they run concurrently).

We only keep the AppVeyor builds for now to test against those old
OpenSSL versions (1.1.1 and 1.0.2) for which there is still extended
support available.  Even simplified like that they still take longer
than the builds on GA.
2026-04-21 16:44:18 +02:00
Tobias Brunner e0fb06c9cc Revert "github: Remove commit ID from cache keys"
Without commit ID, no new caches are created as the key is always the
same.

This reverts commit 60f4c86780.
2026-04-21 11:05:19 +02:00
Tobias Brunner 60f4c86780 github: Remove commit ID from cache keys
This reduces the cache storage for active branches and since caches for
different branches are separate and we abort previous builds of the same
branch, this is not necessary to ensure caches can successfully be stored.
2026-04-17 15:18:11 +02:00
Tobias Brunner 1d36cae26a cirrus/github: Move CI for Alpine from Cirrus CI to GitHub Actions
Same as the previous commit.
2026-04-17 14:38:14 +02:00
Tobias Brunner 06b14b8988 cirrus/github: Move CI for FreeBSD from Cirrus CI to GitHub Actions
Cirrus CI will shut down in June, so we have to find a replacement to
test our build on FreeBSD.  This uses VMs on GitHub Actions.
2026-04-17 10:36:36 +02:00
Tobias Brunner cb27593ce0 kernel-netlink: Update family in SA selector if addresses change 2026-04-16 15:33:25 +02:00
Tobias Brunner e1a11e2c1c ike: Use correct format string for unique Child SA identifiers
Analogous to the previous commit.
2026-04-16 15:32:54 +02:00
Tobias Brunner 2a8fd3e4bf ike: Use correct format string for unique IKE SA identifiers
Would take a while until it became an issue, though.
2026-04-16 15:32:54 +02:00
Tobias Brunner 5334d93a45 delete-ike-sa-job: Log unique ID when deleting half-open IKE SA 2026-04-16 15:32:54 +02:00
Tobias Brunner 7f6fc50ba3 ike-init: Log the IKE SA's unique identifier when creating one as responder
The name will not be defined yet, but we have a similar message as
initiator so use the same format.
2026-04-16 15:32:54 +02:00
Tobias Brunner 1ee7e10940 testing: Fix typo regarding ML-KEM in description 2026-04-15 15:01:40 +02:00
Tobias Brunner 7862974e10 github: Use AWS-LC 1.72.0 for tests 2026-04-14 08:33:56 +02:00
Markus Theil c60a14ba23 botan: Fix typo in algorithm string for SHA3-224 RSA verification
Signed-off-by: Markus Theil <[email protected]>
2026-04-13 15:46:45 +02:00
Markus Theil e1091327b5 tun-device: Also close IPv6 socket on destruction
Signed-off-by: Markus Theil <[email protected]>
2026-04-13 15:13:14 +02:00
Markus Theil febe474926 encrypted-payload: Check for non-zero payload size
Signed-off-by: Markus Theil <[email protected]>
2026-04-13 15:12:55 +02:00
Markus Theil fdd06d99ec botan: Make RNG types configurable
This allows for usage of ESDM or jitterentropy as Botan RNG without
patching strongSwan.

Signed-off-by: Markus Theil <[email protected]>
2026-04-13 15:12:31 +02:00
Tobias Brunner 0d4a8cc9ba appveyor: Reduce build time and remove build against OpenSSL 1.1.0
We are still too close to the limit of 1 hour (at least with the 2019
image and the 2022 image is about the same), so reduce the build time by
not building libimcv natively, which saves about 10 minutes.

Also, only build against OpenSSL 1.0.2 (on the 2017 image) and 1.1.1 (on
the 2019 image) as these are the only versions for which OpenSSL provides
extended support.
2026-04-13 15:08:47 +02:00
Tobias Brunner 032d8adcd4 github: Continue crypto tests even if OpenSSL tests fail
These can sometimes fail because of sync issues with Ubuntu debug symbol
packages, let other crypto tests continue.
2026-04-09 09:53:36 +02:00
Tobias Brunner e8e34c4b33 Use wolfSSL 5.9.1 for tests 2026-04-09 09:53:36 +02:00
Tobias Brunner 99e9db478d unit-tests: Use longer input for ECDSA tests
wolfSSL 5.9.1 starts to enforce a minimum (and maximum) length for the
hash when signing.  Since we'll always require SHA-1, use 20 bytes as
input in the tests to succeed with SIGN_ECDSA_WITH_NULL.
2026-04-09 09:53:36 +02:00
Tobias Brunner b9d9f8ad95 fuzz: Remove unnecessary calls to set plugin dirs
All the plugins are linked statically into the binaries, so there
is no reason to set the directories that are only required when loading
them from files.
2026-04-08 15:57:17 +02:00
Tobias Brunner 444a1dc0e3 fuzz: Create fuzzers with default and custom crypto plugins
The pa_tnc fuzzer does not rely on any plugins and the pb_tnc fuzzer is
a bit special in that it does use code from the tnccs-20 plugin, but that
doesn't actually have to be loaded as such. The fuzzer directly calls
statically linked code from the plugin.
2026-04-08 15:56:42 +02:00
Tobias Brunner 521c6e05c5 github: Update actions so they don't use deprecated Node.js 20 2026-04-07 18:57:40 +02:00
Tobias Brunner b56b3d48b6 tls-server: Avoid allocating large buffer for cipher suites on stack
The `cipher_suites` field has a 16-bit length field, so up to 32k 2-byte
cipher suites could technically be proposed.  With `tls_cipher_suite_t`
typically being 4 bytes wide, the necessary allocation for the temporary
array can be up to 128 KiB.  Even though this should be fine on typical
systems, we avoid potentially overflowing the stack by using malloc()
instead of alloca().
2026-04-02 08:34:23 +02:00
Tobias Brunner a3c5fad1ff libipsec: Reject policies for anything but ESP tunnel mode
This is only relevant for trap policies as similar SAs are already
rejected when negotiated.
2026-04-02 08:34:20 +02:00
Tobias Brunner 2a63343968 trap-manager: Ignore acquires without selectors for wildcard traps
We need to know the actual destination address to process these.
2026-04-02 08:33:30 +02:00
Tobias Brunner 66fffdb1bf ike-init: Remove redundant check for more key exchanges during rekeying
We only call derive_keys() after a successful key_exchange_done() call
during rekeying, so this will always return FALSE.
2026-04-02 08:17:05 +02:00
Tobias Brunner 05807b1b1b openssl: Handle EdDSA keys from engines special again
Fixes: 052a939553 ("openssl: Add support for Ed25519 via AWS-LC")
2026-04-02 08:17:05 +02:00
Tobias Brunner 4af485d87d certreq: Avoid OOB read when enumerating hashes in OCSP CERTREQ
These certificate requests also contain SHA-1 hashes, which is assumed
in `ike_cert_pre.c::process_certreq()` when enumerating key IDs.

Because the parser allocates a separate chunk for the data and the
enumerator doesn't read beyond that chunk's length after the first
iteration, only lengths between 1 and 19 are problematic (0 doesn't
cause an enumeration because chunk_empty is assigned).

Whether the OOB read then can cause a segmentation fault depends on the
allocator, its alignment rules, and its minimum overhead.  For instance,
with glibc on a typical 64-bit system (8 bytes for pointers and size_t),
the alignment is 16 bytes and the minimum allocated size is 32 bytes,
with typically 24 that are technically available for data, even if only
0 bytes are allocated (as returned by `malloc_usable_size()`).  So with
an allocation between 1 and 19, we can always safely read 20 bytes.

Assuming that other allocators behave similar for small allocations, it
seems unlikely that this causes a crash.

Fixes: 15612b3a42 ("Add support for IKEv2 OCSP extensions (RFC 4806)")
2026-04-02 08:17:05 +02:00
Tobias Brunner 20b07f2cbf vici: Prevent uninitialized memory access if VICI_LIST_ITEM is too short
If there is only a single length byte, `value` is not changed and points
to uninitialized data.
2026-04-02 08:17:05 +02:00
Tobias Brunner 513f25ce12 tnccs_11: Avoid crash if TNCCS-ReasonStrings is empty or only contains empty nodes
`xmlNodeGetContent()` returns `NULL` if `child` is `NULL`, which causes
a segmentation fault in `chunk_from_str()`.

Fixes: ec1eab0319 ("fixed XML parsing of TNCCS 1.1 ReasonString message")
2026-04-02 08:17:05 +02:00
Tobias Brunner 65fcf08cdb eap-sim: Prevent infinite loop if version list contains more than one entry
Fixes: ac4dd5439b ("Migrated EAP-SIM to libsimaka, separated server/peer implementations")
2026-04-02 08:17:04 +02:00
Tobias Brunner 7b0190cb26 revocation: Fix memory leak if nonce in OCSP response doesn't match 2026-04-02 08:17:04 +02:00
Tobias Brunner 1e1dd2976d message: Drop fragments with total fragment count lower than before
The RFC only allows that the number of fragments increases (if the
sender reduces the MTU).

Not enforcing this before could cause early reassembly as the trigger was
that the number of received fragments matches the total count of the
current packet (which was a bit weird anyway).  Only an active MITM could
trigger this as individual fragments are encrypted and authenticated.
2026-04-02 08:17:04 +02:00
Tobias Brunner 06e5462a4b credential-factory: Enforce an upper limit when creating nested credentials
This mainly intended as defense-in-depth measure to avoid parsing
massively nested structures that could cause a call stack overflow due
to the massive recursion.  In particular PKCS#7 signed data is prone to
this as these can be nested basically infinitely.  When used in IKEv1 via
ENC_PKCS7_WRAPPED_X509 CERT payloads, our default of 10000 bytes for IKE
messages guards against this, but that's configurable and there might be
a chance for some bug that triggers problematic recursive parsing for
smaller input.

The upper limit is chosen arbitrarily, but there are currently no known
cases that require a depth of more than 10 levels.
2026-04-02 08:16:07 +02:00
Tobias Brunner 236ef93c50 tls-peer: Ensure TLS 1.3 CertificateRequest structure is valid
If nothing was read from the message, the previous code could result in
a crash depending on where `ext.ptr` pointed to, as determined by the
current stack contents.  Since TLS 1.3 is still disabled by default and
this is usually used for TLS-based EAP methods after validating the
IKEv2 server's certificate, the real world impact seems relatively low.

Fixes: 9ef46cfaf9 ("tls-peer: Mutual authentication support for TLS 1.3")
2026-04-02 08:14:29 +02:00
Tobias Brunner e454b4adb3 libsimaka: Prevent out-of-bounds read when parsing attributes with actual length field
These attributes contain a 16-bit length field for the actual length of
the data in bits or bytes, as compared to the length in 4-byte blocks in
the attribute header.  The previous code didn't correctly account for the
length of the fixed header (4 bytes) when it compared the parsed length
to the length in the header.  This could cause an out-of-bounds read of
up to four bytes beyond the end of the attribute/message.

Fixes: f8330d0395 ("Added a libsimaka library with shared message handling code for EAP-SIM/AKA")
2026-04-02 08:14:29 +02:00
Tobias Brunner 0b053e048d nm: Ignore all .gmo files 2026-04-02 07:54:04 +02:00
Tobias Brunner b377a41336 dhcp: Use correct getter for boolean force_server_address option 2026-04-01 19:33:09 +02:00