From 84da416082268a67be97ebb175673210ec4154da Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Wed, 19 Mar 2025 14:20:46 +0100 Subject: [PATCH] trap-manager: Use sequence numbers to identify acquires Either use the sequence number from the kernel (and potentially update it if the acquire was retriggered), or generate our own sequence numbers, which simplifies matching acquires to established/destroyed CHILD_SAs. --- src/libcharon/sa/trap_manager.c | 140 +++++++++++++++++++++----------- 1 file changed, 91 insertions(+), 49 deletions(-) diff --git a/src/libcharon/sa/trap_manager.c b/src/libcharon/sa/trap_manager.c index 1b85c66a5b..5dcb49b497 100644 --- a/src/libcharon/sa/trap_manager.c +++ b/src/libcharon/sa/trap_manager.c @@ -93,6 +93,11 @@ struct private_trap_manager_t { * Whether to ignore traffic selectors from acquires */ bool ignore_acquire_ts; + + /** + * Current acquire sequence number if not generated by the kernel + */ + refcount_t acquire_seq; }; /** @@ -123,6 +128,10 @@ typedef struct { host_t *dst; /** security label, if any */ sec_label_t *label; + /** sequence number from the kernel or generated by the trap manager */ + uint32_t seq; + /** optional sequence number from the kernel if an acquire is retriggered */ + uint32_t new_seq; } acquire_t; /** @@ -519,13 +528,14 @@ METHOD(trap_manager_t, acquire, void, { enumerator_t *enumerator; entry_t *entry, *found = NULL; - acquire_t *acquire; + acquire_t *acquire = NULL; peer_cfg_t *peer; child_cfg_t *child; ike_sa_t *ike_sa; - host_t *host; - uint32_t allocated_reqid; - bool wildcard, ignore = FALSE; + host_t *host = NULL; + sec_label_t *label = NULL; + uint32_t allocated_reqid, seq = 0; + bool wildcard; this->lock->read_lock(this->lock); enumerator = this->traps->create_enumerator(this->traps); @@ -550,44 +560,42 @@ METHOD(trap_manager_t, acquire, void, this->mutex->lock(this->mutex); if (wildcard) - { /* for wildcard acquires we check that we don't have a pending acquire + { + /* for wildcard acquires we check that we don't have a pending acquire * with the same peer */ uint8_t mask; data->dst->to_subnet(data->dst, &host, &mask); if (this->acquires->find_first(this->acquires, acquire_by_dst, - (void**)&acquire, host)) + (void**)&acquire, host)) { host->destroy(host); - ignore = TRUE; - } - else - { - INIT(acquire, - .dst = host, - .reqid = reqid, - ); - this->acquires->insert_last(this->acquires, acquire); } } - else + else if (!this->acquires->find_first(this->acquires, acquire_by_reqid, + (void**)&acquire, reqid, data->label)) { - if (this->acquires->find_first(this->acquires, acquire_by_reqid, - (void**)&acquire, reqid, data->label)) - { - ignore = TRUE; - } - else - { - INIT(acquire, - .reqid = reqid, - .label = data->label ? data->label->clone(data->label) : NULL, - ); - this->acquires->insert_last(this->acquires, acquire); - } + label = data->label ? data->label->clone(data->label) : NULL; + } + if (!acquire) + { + seq = data->seq ?: ref_get_nonzero(&this->acquire_seq); + + INIT(acquire, + .dst = host, + .reqid = reqid, + .label = label, + .seq = seq, + ); + this->acquires->insert_last(this->acquires, acquire); + } + else if (data->seq && acquire->seq != data->seq) + { + /* acquire got retriggered, update to latest sequence number */ + acquire->new_seq = data->seq; } this->mutex->unlock(this->mutex); - if (ignore) + if (!seq) { DBG1(DBG_CFG, "ignoring acquire for reqid %u, connection attempt " "pending", reqid); @@ -642,6 +650,7 @@ METHOD(trap_manager_t, acquire, void, .src = data->src, .dst = data->dst, .label = data->label, + .seq = seq, }; if (this->ignore_acquire_ts || ike_sa->get_version(ike_sa) == IKEV1) @@ -679,38 +688,68 @@ METHOD(trap_manager_t, acquire, void, } /** - * Complete the acquire, if successful or failed + * Update the sequence number of the CHILD_SA before installing it in case + * the acquire got retriggered. */ -static void complete(private_trap_manager_t *this, ike_sa_t *ike_sa, - child_sa_t *child_sa) +static void update_acquire_seq(private_trap_manager_t *this, ike_sa_t *ike_sa, + child_sa_t *child_sa) { enumerator_t *enumerator; acquire_t *acquire; + uint32_t seq; + + /* ignore trap policies and CHILD_SAs not triggered by an acquire */ + if (!ike_sa || !(seq = child_sa->get_acquire_seq(child_sa))) + { + return; + } this->mutex->lock(this->mutex); enumerator = this->acquires->create_enumerator(this->acquires); while (enumerator->enumerate(enumerator, &acquire)) { - if (!acquire->ike_sa || acquire->ike_sa != ike_sa) + if (!acquire->ike_sa || seq != acquire->seq) { continue; } - if (child_sa) + if (acquire->new_seq) { - if (acquire->dst) - { - /* since every wildcard acquire results in a separate IKE_SA - * there is no need to compare the destination address */ - } - else if (child_sa->get_reqid(child_sa) != acquire->reqid) - { - continue; - } - else if (!sec_labels_equal(acquire->label, - child_sa->get_label(child_sa))) - { - continue; - } + child_sa->set_acquire_seq(child_sa, acquire->new_seq); + acquire->seq = acquire->new_seq; + acquire->new_seq = 0; + } + break; + } + enumerator->destroy(enumerator); + this->mutex->unlock(this->mutex); +} + +/** + * Complete the acquire, if successful or failed. + */ +static void complete(private_trap_manager_t *this, ike_sa_t *ike_sa, + child_sa_t *child_sa) +{ + enumerator_t *enumerator; + acquire_t *acquire; + uint32_t seq = 0; + + /* ignore trap policies and CHILD_SAs not triggered by an acquire */ + if (!ike_sa || (child_sa && !(seq = child_sa->get_acquire_seq(child_sa)))) + { + return; + } + + this->mutex->lock(this->mutex); + enumerator = this->acquires->create_enumerator(this->acquires); + while (enumerator->enumerate(enumerator, &acquire)) + { + /* just look at the sequence number when handling a CHILD_SA event, + * otherwise, compare the IKE_SA */ + if (!acquire->ike_sa || (seq && seq != acquire->seq) || + (!seq && acquire->ike_sa != ike_sa)) + { + continue; } this->acquires->remove_at(this->acquires, enumerator); destroy_acquire(acquire); @@ -738,6 +777,9 @@ METHOD(listener_t, child_state_change, bool, { switch (state) { + case CHILD_INSTALLING: + update_acquire_seq(listener->traps, ike_sa, child_sa); + return TRUE; case CHILD_INSTALLED: case CHILD_DESTROYING: complete(listener->traps, ike_sa, child_sa); -- 2.47.2