Consider queued child-creating tasks when reloading configs that have
`start` as start action. Besides some possible corner cases it fixes
handling IKE_SAs that are current getting established and have no
established CHILD_SAs yet.
Closesstrongswan/strongswan#2418
This is particularly important for IKE_SAs that are not yet established,
which would get terminated as they have no established CHILD_SAs yet.
Fixes: 72f9a21b22 ("Merge branch 'vici-reload-actions'")
Otherwise, all configs would be considered to even some degree as the
ports usually match.
Closesstrongswan/strongswan#2441
Fixes: 9228a5109b ("ike-cfg: Consider port information in IKE config match")
This adds support for multiple key exchanges (no KEMs yet as none are
standardized so far). Work on this started over five years ago and went
through multiple iterations (first our own protocol, then standardized
extensions in different variations).
IKE_INTERMEDIATE exchanges, defined RFC 9242, are used to transport
multiple KE payloads between the IKE_SA_INIT and IKE_AUTH exchanges.
To rekey IKE and CHILD_SAs with multiple key exchanges, IKE_FOLLOWUP_KE
exchanges are used, as defined in RFC 9370.
In proposals, additional key exchange methods are configured via `keX_`
prefix, where X is a number between 1 and 7. For example, `ke1_ecp256`
adds ECP_256 as additional KE method. As with regular key exchanges,
peers have to agree on a method for each round unless no algorithms are
defined by both or `keX_none` is configured to make that round explicitly
optional.
Also changed is how rekey collisions are handled, which makes CHILD_SAs
properly trackable via child_rekey() hook.
Previously, it could happen that child_rekey() was triggered twice for
the same "old" SA. For listeners that would mean they'd loose track as
they'd be tracking a new SA that wasn't relevant anymore and for which
no updown event would ever get triggered (it was the redundant SA in a
collision). This new assert ensures that events are triggered in a
predictable way and listeners can track SAs properly.
As the winner of a rekey collision, we previously always triggered the
child_rekey() event once when creating the redundant SA on behalf of the
peer in the passive child-rekey task and then a second time when
creating the winning SA in the active task. However, both calls passed
the replaced CHILD_SA as "old". This made tracking CHILD_SAs impossible
because there was no transition from the redundant, "new" SA of the
first event to the "new", winning SA of the second. Of course, when the
second event was triggered, the redundant SA might not have existed
anymore because the peer is expected to delete it, which could happen
before the CREATE_CHILD_SA response arrives at the initiator.
This refactoring ensures that the child_rekey() event is triggered in
a way that makes the CHILD_SAs trackable in all reasonable (and even
some unreasonable) scenarios. The event is generally only triggered
once after installing the outbound SA for the new/winning CHILD_SA.
This can be when processing the CREATE_CHILD_SA in the active child-rekey
task, or when processing the DELETE for the old SA in a passive
child-delete task. There are some cases where the event is still
triggered twice, but it is now ensured that listeners can properly
transition to the winning SA.
Some corner cases are now also handled correctly, e.g. if a responder's
DELETE for the new CHILD_SA arrives before its CREATE_CHILD_SA response
that actually creates it on the initiator. Also handled properly are
responders of rekeyings that incorrectly send a DELETE for the old
CHILD_SA (previously this caused both, the new and the old SA, to get
deleted).
It also changes that payloads are built before installing the CHILD_SA
on the responder, that is, the KE payload is generated before keys are
derived, so that key_exchange_t::get_public_key() is called before
get_shared_secret(), or its internal equivalent, which could be relevant
for KE implementations that want to ensure that the key can't be
accessed again after the key derivation.
Initially, this is handled with a key derivation for each
IKE_INTERMEDIATE exchange. When rekeying, the keys are derived only
once all IKE_FOLLOWUP_KE exchanges are done.
The message ID of the first IKE_AUTH exchange is a safe-guard against
potential truncation attacks if IKE_INTERMEDIATE exchanges are not used
for multiple key exchanges but some other future use where the number of
exchanges might not depend on the selected proposal.
We currently only support these exchanges for additional key exchanges,
so once we have the final keys derived and the ike-init task is removed,
we don't expect any more of them.
The kernel now allows a 0 replay window with ESN for SAs that are
explicitly tagged as outbound SAs. But not just that, it actually
rejects outbound SAs with replay windows > 0. So we add a version check
to control the replay window size. Note that adding the attribute
unconditionally would be fine even for older kernels, but if somebody
backports the direction patches, the installation of outbound SAs might
fail if the replay window is not adjusted accordingly.
When targeting Android 14, we get a "Background activity launch blocked!"
exception when trying to start the connection in the background (closing
the drawer works). Which is apparently a bug:
https://issuetracker.google.com/issues/305035828
The workaround here is kinda ugly. In particular, because it's not
possible anymore since a few versions to open a dialog that allows users
to directly grant the required permission to the app. We can only open
the generic settings dialog where users have to search for the app and
grant the permission themselves (we could add a dialog with an explanation
similar to the one for the power whitelist if necessary). Hopefully this
gets fixed at some point (the current beta of Android 15 still has the
same bug, though).
This improves the behavior when reloading or unloading connections that
have `start` included in their `start_actiton`.
Closesstrongswan/strongswan#2324
The previous code had some issues because it handled each child config
separately. Not only was this quite inefficient because all IKE_SAs had
to be enumerated for every config, it also caused problems with the check
for other CHILD_SAs in order to decide whether to delete the IKE_SA or
not. Because CHILD_SAs are deleted with an INFORMATIONAL exchange, they
are not immediately gone. This caused a race condition and with more
than one child config and SAs the IKE_SA could be kept because it
could appear as if other, unrelated CHILD_SAs were still there.
Another race condition, which is fixed by the previous commit, occurred
when only changing child configs. Then it could happen that the code
deemed the IKE_SA empty and a delete for it was queued. If that happened
while the IKE_SA was deleting one of the CHILD_SAs (or was busy with some
other exchange), the IKE_SA was not switched to IKE_DELETING. So it
looked usable and create-child tasks for the updated configs might have
gotten queued. Unfortunately, once the ike-delete task is eventually
executed, these tasks would be gone and the replacement CHILD_SAs never
created. This commit additionally avoids actually deleting the IKE_SA
even if all child configs change or get removed if any new CHILD_SAs are
to be initiated.
The IKE_SA might be busy with a different task while a request to
terminate it is getting queued, we don't want to use such an IKE_SA to
initiate new CHILD_SAs as these tasks will get lost once the IKE_SA is
terminated.
The empty array of rules for `assert_message_empty()` and the resulting
size 0 triggers warnings like these:
allocation of insufficient size '0' for type 'listener_message_rule_t' with size '12'
Using calloc() with `nmemb` set to 0 triggers the same warning.
The number of elements is the first argument, their size the second.
The previous code triggered the following warning:
'calloc' sizes specified with 'sizeof' in the earlier argument and not in the later argument
Looks like a cipher suite without DHE was selected previously.
Could be a side-effect of dc1085734f ("testing: Remove unnecessary
FreeRADIUS dh_file option as recommended in the log").
Instead of just adding the offset internally, this way the reported
base address is always the first assignable address (e.g. for
192.168.0.0/24 vs. 192.168.0.1/24).
Closesstrongswan/strongswan#2264
If the regular daemon is running, it creates an unconditional routing
rule for the routing table. The rule that charon-nm tries to create,
which excludes marked IKE/ESP traffic to avoid a routing loop, then
can't be installed and we'd end up with said loop.
Closesstrongswan/strongswan#2230
Tobias Brunner