ikev2: Trigger ike_updown() event after all IKE-specific tasks ran
This makes sure the event is only triggered after the IKE_SA is fully
established and e.g. virtual IPs, additional peer addresses or
a modified reauth time (on the initiator) are assigned to it. This was
e.g. a problem for the selinux plugin if virtual IPs are used.
We use a separate task to trigger the event that's queued before the
child-create task so the event is triggered before the child_updown()
event. Same goes for the state change to IKE_ESTABLISHED.
A new condition is used to indicate the successful completion of all
authentication rounds, so we don't have to set the IKE_ESTABLISHED state
in the ike-auth task (it was used as condition in other tasks).
Since set_state() also sets the rekey and reauth times, this required
some minor changes in regards to how AUTH_LIFETIME notifies are handled.
ikev2: The ike-me task does not have to run before the ike-auth task
Since e334bd46b184 ("ike-auth: Move packet collection to post_build()
method") tasks and plugins can modify the IKE_SA_INIT message independent
of the ike-auth task.
socket-default: Use IPv6-only mode for IPv6 sockets
Otherwise, we can't open a dedicated IPv4 socket on the same port as the
IPv6 socket already is set up do receive IPv4 packets (unless we'd again
enable SO_REUSEADDR).
Fixes: 83da13371292 ("socket-default: Don't set SO_REUSEADDR on IKE sockets anymore")
The default is apparently "Connection: keep-alive", which somehow keeps
the socket around, which leaks file descriptors with every connection
that fetches OCSP and/or CRLs. Over time that could result in the number
of FDs reaching a limit e.g. imposed by FD_SET().
android: Fix "Format string ... is not valid format string..." error
The linter complained that two of the strings don't actually contain any
printf-specifiers (i.e. don't expect any arguments) and therefore
shouldn't be used with String.format().
socket-default: Don't set SO_REUSEADDR on IKE sockets anymore
This was originally required when pluto and charon both bound sockets to
the same port to send messages. Pluto also received messages on them but
charon didn't and used a raw socket instead. Since the removal of pluto
we don't need to set this option anymore, which might actually mask
mistakes like running charon and charon-systemd concurrently (that could
result in messages getting sent fine by both daemons but only received
by one).
Note that a failure to create/bind the sockets will not immediately
result in a shutdown of the daemon. Instead, there will be an error
once the receiver tries to read any messages and also whenever the sender
attempts to send a request.
Changes the type for EAP vendor IDs from uint32_t to pen_t, which has
explicitly been added to represent three-byte IANA-allocated Private
Enterprise Numbers (PEN), which the EAP RFC called "SMI Network
Management Private Enterprise Codes".
Martin Willi [Tue, 20 Sep 2022 05:47:25 +0000 (07:47 +0200)]
pki: Always and implicitly use base64 encoding for EST requests/response
Content-Transfer-Encoding is actually not a valid HTTP header, but a MIME
header, and must not be used. The original RFC7030 specifies this wrong,
and an errata discusses this issue.
The use of base64 encoding has been clarified in RFC8951, and the
recommendation is to always use/expect base64 encoding, but not send/expect
the Content-Transfer-Encoding header.
scepclient: Remove documentation about removal of scepclient
There should be no need for such a persistent documentation on a removed
component in the repository. The commit history is enough. And besides
that, there is user-facing documentation about it in the docs and the
changelog/NEWS.
However, if finalize() fails, e.g. because a previous call to add()
failed due to the size limit, it returns NULL. This then caused a
segmentation fault in raise_event() when it interacted with that value.
Tobias Brunner [Fri, 26 Aug 2022 14:14:30 +0000 (16:14 +0200)]
ike-sa-manager: Make sure flush() removes entries that might get added concurrently
Because flush() has to release the segment locks intermittently, threads
might add new entries (even with the change in the previous commit as the
IKE_SA might already be created, just not registered/checked in yet).
Since those entries are added to the front of the segment lists, the
enumerator in the previous step 2 didn't notice them and did not wait
for them to get checked in. However, step 3 and 4 then proceeded to
delete and destroy the entry and IKE_SA, which could lead to a crash
once the other thread attempts to check in the already destroyed IKE_SA.
This change combines the three loops of steps 2-4 but then loops over
the whole table until it's actually empty. This way we wait for and
destroy newly added entries.
Tobias Brunner [Fri, 26 Aug 2022 13:33:22 +0000 (15:33 +0200)]
ike-sa-manager: Prevent new IKE_SA from getting created when flush() is called
Without ability to create SPIs, other threads are prevented from creating
new IKE_SAs while we are flushing existing IKE_SAs. However, there could
still be IKE_SAs already created that might get checked in while the
segments are temporarily unlocked to wait for threads to check existing
SAs in.
Tobias Brunner [Fri, 26 Aug 2022 15:29:00 +0000 (17:29 +0200)]
ike-sa: Always set ike_cfg_t when setting peer_cfg_t
This is more consistent and e.g. allows to properly take into account
some settings that are also relevant during IKE_AUTH (e.g. childless).
We also already use the peer_cfg_t's ike_cfg_t when rekeying,
reauthenticating and reestablishing an IKE_SA (and e.g. for DSCP).
Also changed are some IKEv1 cases where get_ike_cfg() is called before
set_peer_cfg() without taking a reference to the ike_cfg_t that might
get replaced/destroyed (none of the cases were problematic, though, but
it also wasn't necessary to keep the ike_cfg_t around).
Tobias Brunner [Thu, 18 Aug 2022 10:04:39 +0000 (12:04 +0200)]
cred-encoding: Avoid potential use after free when caching encodings
The pattern currently is to call get_cache(), generate the encoding
if that failed and then store it with cache(). The latter adopts the
passed encoding and frees any stored encoding. However, the latter means
that if two threads concurrently fail to get a cached encoding and then
both generate and store one, one of the threads might use an encoding
that was freed by the other thread.
Since encodings are not expected to change, we can avoid this issue by
not replacing an existing cache entry and instead return that (while
freeing the passed value instead of the cached one).
kernel-netlink: Increase debug level of the "querying [...]" log messages
When watching the output of `swanctl -l` during debugging, the debug
messages in query_sa/policy() cause a lot of noise in the logs (level 2
for DBG_KNL still has actually useful information that we want to see
in the logs) and they're not very useful.
Compared to the messages in the functions above, the ones in update_sa()
and get_replay_state() are not seen often. But since there already is a
log message on level 2 in update_sa(), they're kinda redundant.
github: Enable AddressSanitizer if leak-detective is disabled
At least for the tests where it is available and works. It conflicts
with the instrumentation used by the coverage and fuzzing (and possibly
sonarcloud) tests, the toolchain for the Windows builds doesn't seem to
support it, and on FreeBSD the test executables hang due to a
compatibility issue with FreeBSD's qsort(), which has been fixed [1],
but that has not made it into the clang version in the base system.
For the custom OpenSSL build, debug symbols are enabled so we can
suppress some leaks properly.
Only the second was reported by the compiler (depending on the version
and similarly to the previous commit only with AddressSanitizer active).
The strncpy() call for UTUN_CONTROL_NAME was simply wrong.
kernel-netlink: Fix compiler warnings with strncpy()
Normally, GCC sees that we terminate the destination with a zero byte.
However, when using `-fsanitize=address`, there seems to be additional
instrumentation code after strncpy() so GCC produces warnings like
these:
unit-tests: Don't link files from libimcv into the test executable
This causes odr-violation errors with libasan as some symbols will be
defined twice, once in the linked libimcv and once in the test
executable itself.
Thomas Egerer [Fri, 2 Sep 2022 11:54:05 +0000 (11:54 +0000)]
unit-tests: Use allocated listener instead of stack object in exchange tests
When using the statement expression and a stack object along with
clang-11 and libasan, we get quite a lot of errors about reading
invalid memory. This is due to clang making the actual listener_t local
to the block, such that the access outside of the macros using
_assert_payload is (correctly) considered an error.
By using a heap allocated object, we can destroy it once the listener
returns FALSE (cleaning up properly), and since bus_t does not touch the
listener after that, we don't get any errors from libasan.
Martin Willi [Wed, 8 Apr 2015 08:18:31 +0000 (10:18 +0200)]
unit-tests: Disable AddressSanitizer for threading cleanup function
As the cleanup function reads from the correct address on the parent frame,
it is currently unclear why AddressSanitizer complains about that pointer
dereference.
Some third party IKEv2 products expect an RSA-PSS ASN.1
algorithmIdentifier with an explicit trailerField value (CONTEXT3)
instead of the DEFAULT value if the trailerField is missing.
The setting charon.rsa_pss_trailerfield = yes enables the explicit
encoding.
unit-tests: Let the TLS server thread close its own socket
Closing the socket from the main thread, while the server thread is
still in accept() (or is just about to enter it), seems to
occasionally cause a deadlock on macOS.
testing: Add missing css dir to distribution tarballs
Add the css dir to the EXTRA_DIST variable in the Makefile for the test
environment. This dir was missing when generating distribution tarballs.
Adding it enables successful builds of the test environment from the
dist tarballs.
Fixes: 63f35993d9fb ("testing: Use sans-serif font for test results")
Closes strongswan/strongswan#1266
Seems to be required on macOS (libtls tests didn't run before the recent
implicit enabling via pki). Other platforms apparently let accept() fail
if the socket is shutdown/closed in teardown_creds(), macOS apparently
doesn't do that.
Andreas Steffen [Mon, 29 Aug 2022 08:34:58 +0000 (10:34 +0200)]
pki: pki --req can use old certreq as template
When an X.509 certificate has to be renewed it is helpful to use
the old PKCS#10 certificate request as a template, so that the
distinguishedName (DN), the subjectAlternativeName (SAN) and
a certificate profile name don't have to be typed-in again.
The old public key in the existing certreq is replaced with the
new key and the signature is re-generated using the new private key.
Andreas Steffen [Mon, 22 Aug 2022 12:33:00 +0000 (14:33 +0200)]
libtls: unit tests with crypto libs need additional plugins
In order for libtls to run with the gcrypt libraryi, additionally the
random, pem, gcm, hmac, kdf, x509, constraints, and the curve2519
plugins are needed.
The botan library additionally need the hmac (for HMAC_MD5), x509 and
constraints plugins.
The wolfssl library additionally need the pkcs1, pkcs8, x509 and constraints
plugins.
Andreas Steffen [Fri, 19 Aug 2022 15:18:52 +0000 (17:18 +0200)]
libtls: unit tests run with default plugins
The gcm plugin has been added to the default plugins and all
certificate types are loaded to allow the libtls socket unit
tests to run with the strongSwan default plugins.
Andreas Steffen [Fri, 19 Aug 2022 15:09:02 +0000 (17:09 +0200)]
libtls: Send empty cert payload upon cert request
Currently when a TLS client doesn't have a certificate, it doesn't
send a certficiate payload upon receiving a certificate request
from the TLS server. According to the TLS 1.2 and 1.3 RFCs an
empty certificate payload must be sent.
Andreas Steffen [Wed, 10 Aug 2022 22:21:28 +0000 (00:21 +0200)]
pkcs10: Support of Microsoft CertTypeExtension
The msCertificateTypeExtension OID (1.3.6.1.4.1.311.20.2) can
be used in a PKCS#10 certificate request to define a certificate
profile. It consists of an UTF8 string.