At least since Ubuntu 22.04 there are lots of (apparently non-fatal)
"DWARF error: could not find variable specification" errors generated
by bfd_find_nearest_line(). This also happens with addr2line, which
internally uses libbfd. No idea what the actual issue is (it still
happens with binutils 2.39 in Ubuntu 22.10), but since it doesn't seem
to affect the functionality we need this avoids filling the test logs
with useless messages.
A recent change added CALLBACK to PENUM_PAGE_FILE_CALLBACKW/A in psapi.h,
which conflicts with our own macro of the same name. Our compat/windows.h
header undefs the Windows definition, but that only works if Windows
headers are included before ours, which wasn't the case for psapi.h.
This avoids a conflicts as mysql80-client has a dependency on either
openldap24-client (FreeBSD 12) or openldap26-client (FreeBSD 13) so e.g.
installing openldap24-client on FreeBSD 13 causes
openldap26-client-2.6.3 conflicts with openldap24-client-2.4.59_4 on /usr/local/bin/ldapadd
Similarly, when installing openldap26-client on FreeBSD 12. So just let
the mysql80-client package decide which version is installed.
There are spurious crashes after test runs (during the reporting phase
of LeakSanitizer) that are triggered by seemingly unrelated code changes.
It seems to be related to how glibc handles dynamic TLS allocations.
Malicious servers could otherwise block the fetching thread indefinitely
after the initial TCP handshake (which has a default timeout of 10s
in the curl and winhttp plugins, the soup plugin actually has a default
overall timeout of 10s).
This avoids querying URLs of potentially untrusted certificates, e.g. if
an attacker sends a specially crafted end-entity and intermediate CA
certificate with a CDP that points to a server that completes the
TCP handshake but then does not send any further data, which will block
the fetcher thread (depending on the plugin) for as long as the default
timeout for TCP. Doing that multiple times will block all worker threads,
leading to a DoS attack.
The logging during the certificate verification obviously changes. The
following example shows the output of `pki --verify` for the current
strongswan.org certificate:
new:
using certificate "CN=www.strongswan.org"
using trusted intermediate ca certificate "C=US, O=Let's Encrypt, CN=R3"
using trusted ca certificate "C=US, O=Internet Security Research Group, CN=ISRG Root X1"
reached self-signed root ca with a path length of 1
checking certificate status of "CN=www.strongswan.org"
requesting ocsp status from 'http://r3.o.lencr.org' ...
ocsp response correctly signed by "C=US, O=Let's Encrypt, CN=R3"
ocsp response is valid: until Jul 27 12:59:58 2022
certificate status is good
checking certificate status of "C=US, O=Let's Encrypt, CN=R3"
ocsp response verification failed, no signer certificate 'C=US, O=Let's Encrypt, CN=R3' found
fetching crl from 'http://x1.c.lencr.org/' ...
using trusted certificate "C=US, O=Internet Security Research Group, CN=ISRG Root X1"
crl correctly signed by "C=US, O=Internet Security Research Group, CN=ISRG Root X1"
crl is valid: until Apr 18 01:59:59 2023
certificate status is good
certificate trusted, lifetimes valid, certificate not revoked
old:
using certificate "CN=www.strongswan.org"
using trusted intermediate ca certificate "C=US, O=Let's Encrypt, CN=R3"
checking certificate status of "CN=www.strongswan.org"
requesting ocsp status from 'http://r3.o.lencr.org' ...
ocsp response correctly signed by "C=US, O=Let's Encrypt, CN=R3"
ocsp response is valid: until Jul 27 12:59:58 2022
certificate status is good
using trusted ca certificate "C=US, O=Internet Security Research Group, CN=ISRG Root X1"
checking certificate status of "C=US, O=Let's Encrypt, CN=R3"
ocsp response verification failed, no signer certificate 'C=US, O=Let's Encrypt, CN=R3' found
fetching crl from 'http://x1.c.lencr.org/' ...
using trusted certificate "C=US, O=Internet Security Research Group, CN=ISRG Root X1"
crl correctly signed by "C=US, O=Internet Security Research Group, CN=ISRG Root X1"
crl is valid: until Apr 18 01:59:59 2023
certificate status is good
reached self-signed root ca with a path length of 1
certificate trusted, lifetimes valid, certificate not revoked
Note that this also fixes an issue with the previous dual-use of the
`trusted` flag. It not only indicated whether the chain is trusted but
also whether the current issuer is the root anchor (the corresponding
flag in the `cert_validator_t` interface is called `anchor`). This was
a problem when building multi-level trust chains for pre-trusted
end-entity certificates (i.e. where `trusted` is TRUE from the start).
This caused the main loop to get aborted after the first intermediate CA
certificate and the mentioned `anchor` flag wasn't correct in any calls
to `cert_validator_t` implementations.
Fixes: CVE-2022-40617
This forces the use of a semicolon after INIT() and makes existing ones,
which was the case for basically all instances, necessary so e.g.
sonarcloud won't complain about an empty statement after every one of
them.
By evaluating to the allocated object, it would theoretically also allow
constructs like this:
struct_t *this;
return INIT(this,
.a = x,
.b = y,
);
or this:
array_insert(a, ARRAY_TAIL, INIT(this,
.a = x,
.b = y,
));
If e.g. the hmac plugin is loaded before the third-party crypto lib
plugin, we might not use the latter's HMAC implementation in some
cases (e.g. in the libtls tests).
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.
Since e334bd46b1 ("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.
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: 83da133712 ("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().
Closesstrongswan/strongswan#1160
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().
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".
References strongswan/strongswan#581
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.
There are a lot of calls like this:
this->dispatcher->raise_event(this->dispatcher, "...", 0,
b->finalize(b));
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.
Closesstrongswan/strongswan#1278
This fixes a race condition during shutdown between the main thread
flushing the IKE_SA manager and worker threads still creating IKE_SAs.
Closesstrongswan/strongswan#1252
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.
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.
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).
Closesstrongswan/strongswan#1238
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).
Closesstrongswan/strongswan#1231
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.
[1] https://github.com/llvm/llvm-project/issues/46176
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.
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:
‘__builtin_strncpy’ specified bound 16 equals destination size [-Wstringop-truncation]
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.
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.
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.
Since the allocated data was smaller than sizeof(eap_mschapv2_header_t),
the following compile error was triggered (with newer GCC versions):
eap_mschapv2.c: In function 'process_peer_success':
eap_mschapv2.c:945:12: error: array subscript 'eap_mschapv2_header_t[0]' is partly outside array bounds of 'unsigned char[6]' [-Werror=array-bounds]
945 | eap->code = EAP_RESPONSE;
| ^~
In file included from /usr/include/stdlib.h:587,
from ../../../../src/libstrongswan/utils/printf_hook/printf_hook.h:26,
from ../../../../src/libstrongswan/library.h:101,
from ../../../../src/libcharon/sa/eap/eap_method.h:28,
from eap_mschapv2.h:27,
from eap_mschapv2.c:18:
eap_mschapv2.c:944:15: note: object of size 6 allocated by '__builtin_alloca'
944 | eap = alloca(len);
| ^~~~~~
Closesstrongswan/strongswan#1188Closesstrongswan/strongswan#1215
Tobias Brunner