Was apparently forgotten when support was added to the attr plugin
with 98a3ba8a5a ("attr: Add p-cscf keyword for P-CSCF server addresses").
For consistency, using an underscore like the `split*` options and not a
dash like in the attr plugin.
References strongswan/strongswan#2396
Apparently, some clients (e.g. native Android) just send an empty
EAP-Identity response. We silently ignored that previously and then
used the IKE identity for the actual EAP method. This change tries to
do something similar (i.e. don't fail if the response is empty), but by
assuming the IKE identity as EAP-Identity, we match that and possibly
can switch configs.
Closesstrongswan/strongswan#2833
Fixes: 2f2e4abe3c ("ikev2: Add support to switch peer configs based on EAP-Identities")
This now adds some state (basically a message buffer), but simplifies
error handling as we don't have to handle two potential failure paths
and could avoid some potential issues by still calling the blocking
read_all().
It also fixes a memory leak when clients disconnect.
Performing a stream read_all call (which is a blocking read) from
within the accept callback has the issue that if a whitelist client is
still connected whilst a shutdown of the charon deamon is triggered
then that shutdown won't complete gracefully due to the accept task
never exiting.
So fix shutting down gracefully by using the socket watcher rather than
a blocking read upon connection accept. Fall back to a blocking read
for partial messages to avoid the complexity associated (i.e. storing
state) for incomplete reads, which shouldn't block and cause the
original problem if the client only sends whole messages.
As we set the remote port to 0, we'd get a mapping change message with
every packet. Setting the threshold avoids all kernel messages after the
first, which we suppress explicitly as well.
This might make debugging easier and also ensures that a possible
fallback SA without CPU ID is established first when reestablishing
an IKE_SA. Because even if such an SA is established first initially,
that might change later depending on when per-CPU SAs are rekeyed.
Not sure if this combination does make sense as the plugin itself would
be a major bottleneck.
Similar to the connmark plugin, PREROUTING rules list SPIs or UDP ports,
which would be necessary for all SAs while the OUTPUT rules would only be
required once.
The combination probably doesn't make much sense.
The OUTPUT rules would definitely only be required once, while the INPUT
and PREROUTING rules list individual SPIs and/or UDP ports, which would
be necessary for all SAs.
By the way, the rules in PREROUTING might actually not be necessary
anymore if the set_mark_in option was used for such SAs.
Besides the previous key exchange method, this will allow us to also
reuse the previous traffic selectors. Some data is still passed in
separate methods as some are set even when there is no previous SA and
others are not set in all cases.
The interface for queue_child() now optionally takes the previous
Child SA to handle both recreations and initiations from scratch.
This fixes cases where `start_action = trap|start` is used and an acquire
is triggered while the SA is initiated (granted if narrowing is expected,
that's not a recommended configuration as the responder can only use
the first config when there is no packet TS). The resulting second
create-child task will potentially get dropped by the duplicate check,
so the temporary state won't get removed and traffic is blocked until
that expires, neither can acquires get triggered for traffic that doesn't
match the initial SA's policies.
Tobias Brunner