From 1607e538e9d2f631e737d2a2040153e4771f8e28 Mon Sep 17 00:00:00 2001 From: Shmulik Ladkani Date: Mon, 2 Nov 2020 14:54:48 +0200 Subject: [PATCH] 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. --- src/libcharon/control/controller.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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); }