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.
This commit is contained in:
Shmulik Ladkani 2020-11-02 14:54:48 +02:00 committed by Tobias Brunner
parent 70b0c730d0
commit 1607e538e9

View File

@ -569,17 +569,17 @@ METHOD(job_t, terminate_ike_execute, job_requeue_t,
listener->ike_sa = ike_sa; listener->ike_sa = ike_sa;
listener->lock->unlock(listener->lock); 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) if (ike_sa->delete(ike_sa, listener->options.force) != DESTROY_ME)
{ /* delete queued */ { /* delete queued */
listener->status = FAILED;
charon->ike_sa_manager->checkin(charon->ike_sa_manager, ike_sa); charon->ike_sa_manager->checkin(charon->ike_sa_manager, ike_sa);
} }
else else
{ {
if (!listener->logger.callback)
{
listener->status = SUCCESS;
}
charon->ike_sa_manager->checkin_and_destroy(charon->ike_sa_manager, charon->ike_sa_manager->checkin_and_destroy(charon->ike_sa_manager,
ike_sa); ike_sa);
} }