From: Shmulik Ladkani Date: Mon, 2 Nov 2020 12:54:48 +0000 (+0200) Subject: controller: Always return SUCCESS when terminating IKE_SAs without callback X-Git-Tag: 5.9.1~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1607e538e9d2f631e737d2a2040153e4771f8e28;p=thirdparty%2Fstrongswan.git controller: Always return SUCCESS when terminating IKE_SAs without callback If no callback is specified, terminate_ike_execute() is invoked without the listener waiting on the IKE state change. Now, if 'force' is false, then ike_sa->delete() just queues an IKE_DELETE task, and returns SUCCESS - indicating successful task manager initiation. However, terminate_ike_execute() ignored this success and set the status to FAILED. This is not ideal, as it will be the overall return code of terminate_ike(), although no failure did occur. This eventually leads vici's "terminate" to return "Command failed: terminating SA failed", as seen in this example: In [9]: list(session.terminate({'ike-id': 2960, 'timeout': -1})) --------------------------------------------------------------------------- CommandException Traceback (most recent call last) in () ----> 1 list(session.terminate({'ike-id': 2960, 'timeout': -1})) vici/session.pyc in streamed_request(self, command, event_stream_type, message) 136 raise CommandException( 137 "Command failed: {errmsg}".format( --> 138 errmsg=command_response["errmsg"] 139 ) 140 ) CommandException: Command failed: terminating SA failed If we consider both queueing the task and actually destroying the IKS_SA a success, we can just always return SUCCESS if we don't have a callback. There is also no need to explicitly set the status to FAILED if a listener is waiting as that's the default anyway. Co-authored-by: Tobias Brunner Closes strongswan/strongswan#185. --- diff --git a/src/libcharon/control/controller.c b/src/libcharon/control/controller.c index 0c86275e20..3baa9342ad 100644 --- a/src/libcharon/control/controller.c +++ b/src/libcharon/control/controller.c @@ -569,17 +569,17 @@ METHOD(job_t, terminate_ike_execute, job_requeue_t, listener->ike_sa = ike_sa; listener->lock->unlock(listener->lock); + if (!listener->logger.callback) + { /* if we don't wait for the result, either outcome below is a success */ + listener->status = SUCCESS; + } + if (ike_sa->delete(ike_sa, listener->options.force) != DESTROY_ME) { /* delete queued */ - listener->status = FAILED; charon->ike_sa_manager->checkin(charon->ike_sa_manager, ike_sa); } else { - if (!listener->logger.callback) - { - listener->status = SUCCESS; - } charon->ike_sa_manager->checkin_and_destroy(charon->ike_sa_manager, ike_sa); }