When flush() is called there might be threads in install() waiting for
trap policies to get installed (without holding the lock). We have to
wait until they updated the entries with the respective CHILD_SAs before
destroying the list.
We also have to prevent further trap policy installations (and wait until
threads in install() are really finished), otherwise we might end up
destroying CHILD_SA objects after the kernel interface implementations
have already been unloaded (avoiding this is the whole point of calling
flush() before unloading the plugins).
This fixes potential race conditions in case complete() or flush() is
executed before or concurrently with a thread that handles an acquire.
It will also simplify tracking multiple acquires created for the same
trap policy in the future.
Also fixes the behavior in some error situations.
This basically reverts f4e822c1b4 ("trap-manager: don't check-in
nonexisting IKE_SA if acquire fails"). As checkout_by_config() could
return an already existing and established IKE_SA we have to properly
destroy it, for instance, in case other threads are waiting to check
it out. checkin_and_destroy() should handle the case of a new SA
properly (it produces a log message on level 1, though).
The specific traffic selectors from the acquire events, which are derived
from the triggering packet, are usually prepended to those from the
config. Some implementations might not be able to handle these properly.
References #860.
As we now use the same reqid for multiple CHILD_SAs with the same selectors,
having marks based on the reqid makes not that much sense anymore. Instead we
use unique marks that use a custom identifier. This identifier is reused during
rekeying, keeping the marks constant for any rule relying on it (for example
installed by updown).
This also simplifies handling of reqid allocation, as we do not have to query
the marks that is not yet assigned for an unknown reqid.
Because the write lock was held while calling add_policies() on
child_sa_t, which finishes with a call to child_state_change() on bus_t,
a deadlock would ensue if CHILD_SAs are concurrently being established,
which also causes a call to child_state_change() that will require
the read lock in trap_manager_t.
No locks are now being held while creating the CHILD_SA and installing the
trap policies.
Before this change, modified configs that have been updated with ipsec reload,
could properly be started manually, but the old config would get used if
triggered via trap policies.
As we only can negotiate a single TS in IKEv1, don't prepend the
triggering packet TS, as we do in IKEv2. Otherwise we don't establish
the TS of the configuration, but only that of the triggering packet.
Fixes#207.
Conflicts:
configure.in
man/ipsec.conf.5.in
src/libcharon/daemon.c
src/libcharon/plugins/eap_ttls/eap_ttls_peer.c
src/libcharon/plugins/eap_radius/eap_radius_accounting.c
src/libcharon/plugins/eap_radius/eap_radius_forward.c
src/libcharon/plugins/farp/farp_listener.c
src/libcharon/sa/ike_sa.c
src/libcharon/sa/keymat.c
src/libcharon/sa/task_manager.c
src/libcharon/sa/trap_manager.c
src/libstrongswan/plugins/x509/x509_cert.c
src/libstrongswan/utils.h
Applied lost changes of moved files keymat.c and task_manager.c.
Updated listener_t.message hook signature in new plugins.
Also fixes a TOCTOU issue regarding the use of entry_t.pending.
The deadlock was caused because the rwlock was being locked while
waiting for an IKE_SA. Triggering the deadlock was a bit tricky, here
is the description by Thomas Egerer (the reporter of this issue):
"
The deadlock occurs when the following happens (in the given order):
a) an IKE_SA is built and a thread is processing the IKE_AUTH request,
which can take a bit longer when a smartcard is involved. This
causes the ike_sa_manager to lock a particular IKE_SA exclusively.
b) an acquire is triggered which causes the rwlock in the trap_manager
to be read-locked, the subsequent call to
ike_sa_manager->checkout_by_config has to wait until a) unlocks
it's ike_sa.
c) a child_cfg contained in the peer_cfg belonging to the ike_sa
a) has locked is routed causes the child_configs contained
in the peer config to be locked by c) while the actual routing
code within trap_manager tries to writelock it's rwlock.
That's about it. As soon as a) finishes authentication of the peer
and tries to find a matching child sa it will try to lock the child
configs of the peer config which is not possible since it has been
locked by c).
Thread | Resource locked | Resource desired
-------+--------------------------------+--------------------------------
(a) | ike_sa in ike_sa_manager | child_cfgs of peer_cfg
| |
(b) | rwlock in trap-manager (read) | ike_sa in ike_sa_manager
| |
(c) | child_cfgs of peer_cfg | rwlock in trap-manager (write)
"
With this patch thread (b) now does not hold the lock while waiting for
the IKE_SA. Thus (c) can get the write lock, and (a) can subsequently
lock the mutex in the peer_cfg which then finally allows (b) to checkout
the IKE_SA.
Tobias Brunner