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)
    <ipython-input-9-5f95b5cea88f> in <module>()
    ----> 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 <tobias@strongswan.org>
Closes strongswan/strongswan#185.

diff --git a/src/libcharon/control/controller.c b/src/libcharon/control/controller.c
index 0c86275e206f74b85c4d976bdf521f0e469f9aa9..3baa9342ad688f16cb831a8cdb6df5eda23e7532 100644 (file)
--- 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);
        }