]> git.ipfire.org Git - thirdparty/strongswan.git/commitdiff
ike-rekey: Remove collision task type checks
authorTobias Brunner <tobias@strongswan.org>
Thu, 11 Jun 2020 13:06:24 +0000 (15:06 +0200)
committerTobias Brunner <tobias@strongswan.org>
Wed, 29 Jun 2022 08:28:50 +0000 (10:28 +0200)
Since f67199378df9 ("ike-rekey: Handle undetected collisions also if
delete is delayed") we only ever track tasks of type TASK_IKE_REKEY, so
there is no need to check the type or use the generic task_t interface.

Also changed some of the comments to clarify collision handling.

src/libcharon/sa/ikev2/tasks/ike_rekey.c

index cca211fed5a825b16e1dd4487b2a8d2d5fe18663..f539ae21989f0dc556a51ead6b3d36917c574d89 100644 (file)
@@ -67,7 +67,7 @@ struct private_ike_rekey_t {
        /**
         * colliding task detected by the task manager
         */
-       task_t *collision;
+       private_ike_rekey_t *collision;
 
        /**
         * TRUE if rekeying can't be handled temporarily
@@ -313,12 +313,11 @@ METHOD(task_t, build_r, status_t,
  */
 static bool conclude_undetected_collision(private_ike_rekey_t *this)
 {
-       if (this->collision &&
-               this->collision->get_type(this->collision) == TASK_IKE_REKEY)
+       if (this->collision)
        {
                DBG1(DBG_IKE, "peer did not notice IKE_SA rekey collision, abort "
                         "active rekeying");
-               establish_new((private_ike_rekey_t*)this->collision);
+               establish_new(this->collision);
                return TRUE;
        }
        return FALSE;
@@ -355,19 +354,17 @@ METHOD(task_t, process_i, status_t,
                        break;
        }
 
-       /* check for collisions */
-       if (this->collision &&
-               this->collision->get_type(this->collision) == TASK_IKE_REKEY)
+       if (this->collision)
        {
-               private_ike_rekey_t *other = (private_ike_rekey_t*)this->collision;
+               private_ike_rekey_t *other = this->collision;
                host_t *host;
                chunk_t this_nonce, other_nonce;
 
                this_nonce = this->ike_init->get_lower_nonce(this->ike_init);
                other_nonce = other->ike_init->get_lower_nonce(other->ike_init);
 
-               /* if we have the lower nonce, delete rekeyed SA. If not, delete
-                * the redundant. */
+               /* the SA with the lowest nonce should be deleted, check if we or
+                * the peer created that */
                if (memcmp(this_nonce.ptr, other_nonce.ptr,
                                   min(this_nonce.len, other_nonce.len)) < 0)
                {
@@ -396,7 +393,8 @@ METHOD(task_t, process_i, status_t,
                        establish_new(other);
                        return SUCCESS;
                }
-               /* peer should delete this SA. Add a timeout just in case. */
+
+               /* peer should delete the SA it created, add a timeout just in case */
                job_t *job = (job_t*)delete_ike_sa_job_create(
                                                                        other->new_sa->get_id(other->new_sa), TRUE);
                lib->scheduler->schedule_job(lib->scheduler, job,
@@ -412,7 +410,7 @@ METHOD(task_t, process_i, status_t,
 
        establish_new(this);
 
-       /* rekeying successful, delete the IKE_SA using a subtask */
+       /* rekeying successful, delete this IKE_SA using a subtask */
        this->ike_delete = ike_delete_create(this->ike_sa, TRUE);
        this->public.task.build = _build_i_delete;
        this->public.task.process = _process_i_delete;
@@ -429,8 +427,7 @@ METHOD(task_t, get_type, task_type_t,
 METHOD(ike_rekey_t, did_collide, bool,
        private_ike_rekey_t *this)
 {
-       return this->collision &&
-                  this->collision->get_type(this->collision) == TASK_IKE_REKEY;
+       return this->collision != NULL;
 }
 
 METHOD(ike_rekey_t, collide, void,
@@ -444,7 +441,7 @@ METHOD(ike_rekey_t, collide, void,
                case TASK_IKE_DELETE:
                        conclude_undetected_collision(this);
                        other->destroy(other);
-                       return;
+                       break;
                case TASK_IKE_REKEY:
                {
                        private_ike_rekey_t *rekey = (private_ike_rekey_t*)other;
@@ -454,15 +451,16 @@ METHOD(ike_rekey_t, collide, void,
                                DBG1(DBG_IKE, "colliding exchange did not result in an IKE_SA, "
                                         "ignore");
                                other->destroy(other);
-                               return;
+                               break;
                        }
+                       DESTROY_IF(&this->collision->public.task);
+                       this->collision = rekey;
                        break;
                }
                default:
+                       /* shouldn't happen */
                        break;
        }
-       DESTROY_IF(this->collision);
-       this->collision = other;
 }
 
 /**
@@ -483,7 +481,7 @@ static void cleanup(private_ike_rekey_t *this)
        cur_sa = charon->bus->get_sa(charon->bus);
        DESTROY_IF(this->new_sa);
        charon->bus->set_sa(charon->bus, cur_sa);
-       DESTROY_IF(this->collision);
+       DESTROY_IF(&this->collision->public.task);
 }
 
 METHOD(task_t, migrate, void,