trap-manager: Set seq. no. for CHILD_SAs not initiated by an acquire

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.

diff --git a/src/libcharon/sa/trap_manager.c b/src/libcharon/sa/trap_manager.c
index 7bd2c698f0f46f1e22521cfda3c592017c5432dd..814c8c8c0ae6024bb987d27dde81ecded81df5e7 100644 (file)
--- a/src/libcharon/sa/trap_manager.c
+++ b/src/libcharon/sa/trap_manager.c
@@ -126,10 +126,8 @@ typedef struct {
        uint32_t reqid;
        /** destination address (wildcard case) */
        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;
+       /** data from the kernel */
+       kernel_acquire_data_t *data;
        /** optional sequence number from the kernel if an acquire is retriggered */
        uint32_t new_seq;
 } acquire_t;
@@ -154,7 +152,7 @@ static void destroy_entry(entry_t *this)
 static void destroy_acquire(acquire_t *this)
 {
        DESTROY_IF(this->dst);
-       DESTROY_IF(this->label);
+       kernel_acquire_data_destroy(this->data);
        free(this);
 }
 
@@ -165,7 +163,7 @@ CALLBACK(acquire_by_reqid, bool,
        sec_label_t *label;
 
        VA_ARGS_VGET(args, reqid, label);
-       return this->reqid == reqid && sec_labels_equal(this->label, label);
+       return this->reqid == reqid && sec_labels_equal(this->data->label, label);
 }
 
 CALLBACK(acquire_by_dst, bool,
@@ -533,7 +531,6 @@ METHOD(trap_manager_t, acquire, void,
        child_cfg_t *child;
        ike_sa_t *ike_sa;
        host_t *host = NULL;
-       sec_label_t *label = NULL;
        uint32_t allocated_reqid, seq = 0;
        bool wildcard;
 
@@ -572,24 +569,22 @@ METHOD(trap_manager_t, acquire, void,
                        host->destroy(host);
                }
        }
-       else if (!this->acquires->find_first(this->acquires, acquire_by_reqid,
-                                                                                (void**)&acquire, reqid, data->label))
+       else
        {
-               label = data->label ? data->label->clone(data->label) : NULL;
+               this->acquires->find_first(this->acquires, acquire_by_reqid,
+                                                                  (void**)&acquire, reqid, data->label);
        }
        if (!acquire)
        {
-               seq = data->seq ?: ref_get_nonzero(&this->acquire_seq);
-
                INIT(acquire,
                        .dst = host,
                        .reqid = reqid,
-                       .label = label,
-                       .seq = seq,
+                       .data = kernel_acquire_data_clone(data),
                );
+               seq = data->seq = data->seq ?: ref_get_nonzero(&this->acquire_seq);
                this->acquires->insert_last(this->acquires, acquire);
        }
-       else if (data->seq && acquire->seq != data->seq)
+       else if (data->seq && data->seq != acquire->data->seq)
        {
                /* acquire got retriggered, update to latest sequence number */
                acquire->new_seq = data->seq;
@@ -698,8 +693,18 @@ static void update_acquire_seq(private_trap_manager_t *this, ike_sa_t *ike_sa,
        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)))
+       /* ignore trap policies */
+       if (!ike_sa)
+       {
+               return;
+       }
+       /* if a CHILD_SA was not triggered by an acquire (e.g. manually or via
+        * start action) and the kernel supports sequence numbers, we check if the
+        * negotiated TS match the triggering packet and set the sequence number
+        * to remove the temporary state in the kernel */
+       seq = child_sa->get_acquire_seq(child_sa);
+       if (!seq &&
+               !(charon->kernel->get_features(charon->kernel) & KERNEL_ACQUIRE_SEQ))
        {
                return;
        }
@@ -708,14 +713,25 @@ static void update_acquire_seq(private_trap_manager_t *this, ike_sa_t *ike_sa,
        enumerator = this->acquires->create_enumerator(this->acquires);
        while (enumerator->enumerate(enumerator, &acquire))
        {
-               if (!acquire->ike_sa || seq != acquire->seq)
+               if (!seq)
+               {
+                       /* if not triggered by an acquire, compare the TS from the packet
+                        * that triggered the current acquire, set the seq if they match */
+                       if (!child_sa_ts_match(child_sa, acquire->data->src,
+                                                                  acquire->data->dst))
+                       {
+                               continue;
+                       }
+                       child_sa->set_acquire_seq(child_sa, acquire->data->seq);
+               }
+               else if (!acquire->ike_sa || seq != acquire->data->seq)
                {
                        continue;
                }
                if (acquire->new_seq)
                {
                        child_sa->set_acquire_seq(child_sa, acquire->new_seq);
-                       acquire->seq = acquire->new_seq;
+                       acquire->data->seq = acquire->new_seq;
                        acquire->new_seq = 0;
                }
                break;
@@ -746,7 +762,7 @@ static void complete(private_trap_manager_t *this, ike_sa_t *ike_sa,
        {
                /* 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) ||
+               if (!acquire->ike_sa || (seq && seq != acquire->data->seq) ||
                        (!seq && acquire->ike_sa != ike_sa))
                {
                        continue;