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 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.
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().
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
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.
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
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.
Closesstrongswan/strongswan#1271
