From 2edc73d84eb9c97ba610edaf75e48cfffd85d31d Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 30 Apr 2020 17:42:07 +0200 Subject: [PATCH] ike: Only track actually sent retransmits as outbound packets Retransmission jobs for old requests for which we already received a response previously left the impression that messages were sent more recently than was actually the case. task_manager_t always defined INVALID_STATE as possible return value if no retransmit was sent, this just was never actually returned. I guess we could further differentiate between actual invalid states (e.g. if we already received the response) and when we don't send a retransmit for other reasons e.g. because the IKE_SA became stale. --- src/libcharon/sa/ike_sa.c | 133 ++++++++++++----------- src/libcharon/sa/ike_sa.h | 9 +- src/libcharon/sa/ikev1/task_manager_v1.c | 2 +- src/libcharon/sa/ikev2/task_manager_v2.c | 7 +- 4 files changed, 79 insertions(+), 72 deletions(-) diff --git a/src/libcharon/sa/ike_sa.c b/src/libcharon/sa/ike_sa.c index e482d55eed..f1e3e8fe53 100644 --- a/src/libcharon/sa/ike_sa.c +++ b/src/libcharon/sa/ike_sa.c @@ -2426,81 +2426,86 @@ METHOD(ike_sa_t, retransmit, status_t, { return INVALID_STATE; } - this->stats[STAT_OUTBOUND] = time_monotonic(NULL); - if (this->task_manager->retransmit(this->task_manager, message_id) != SUCCESS) + switch (this->task_manager->retransmit(this->task_manager, message_id)) { - /* send a proper signal to brief interested bus listeners */ - switch (this->state) + case SUCCESS: + this->stats[STAT_OUTBOUND] = time_monotonic(NULL); + return SUCCESS; + case INVALID_STATE: + return INVALID_STATE; + default: + break; + } + /* send a proper signal to brief interested bus listeners */ + switch (this->state) + { + case IKE_CONNECTING: { - case IKE_CONNECTING: + /* retry IKE_SA_INIT/Main Mode if we have multiple keyingtries */ + uint32_t tries = this->peer_cfg->get_keyingtries(this->peer_cfg); + charon->bus->alert(charon->bus, ALERT_PEER_INIT_UNREACHABLE, + this->keyingtry); + this->keyingtry++; + if (tries == 0 || tries > this->keyingtry) { - /* retry IKE_SA_INIT/Main Mode if we have multiple keyingtries */ - uint32_t tries = this->peer_cfg->get_keyingtries(this->peer_cfg); - charon->bus->alert(charon->bus, ALERT_PEER_INIT_UNREACHABLE, - this->keyingtry); - this->keyingtry++; - if (tries == 0 || tries > this->keyingtry) - { - DBG1(DBG_IKE, "peer not responding, trying again (%d/%d)", - this->keyingtry + 1, tries); - reset(this, TRUE); - resolve_hosts(this); - return this->task_manager->initiate(this->task_manager); - } - DBG1(DBG_IKE, "establishing IKE_SA failed, peer not responding"); + DBG1(DBG_IKE, "peer not responding, trying again (%d/%d)", + this->keyingtry + 1, tries); + reset(this, TRUE); + resolve_hosts(this); + return this->task_manager->initiate(this->task_manager); + } + DBG1(DBG_IKE, "establishing IKE_SA failed, peer not responding"); + + if (this->version == IKEV1 && array_count(this->child_sas)) + { + enumerator_t *enumerator; + child_sa_t *child_sa; - if (this->version == IKEV1 && array_count(this->child_sas)) + /* if reauthenticating an IKEv1 SA failed (assumed for an SA + * in this state with CHILD_SAs), try again from scratch */ + DBG1(DBG_IKE, "reauthentication failed, trying to " + "reestablish IKE_SA"); + reestablish(this); + /* trigger down events for the CHILD_SAs, as no down event + * is triggered below for IKE SAs in this state */ + enumerator = array_create_enumerator(this->child_sas); + while (enumerator->enumerate(enumerator, &child_sa)) { - enumerator_t *enumerator; - child_sa_t *child_sa; - - /* if reauthenticating an IKEv1 SA failed (assumed for an SA - * in this state with CHILD_SAs), try again from scratch */ - DBG1(DBG_IKE, "reauthentication failed, trying to " - "reestablish IKE_SA"); - reestablish(this); - /* trigger down events for the CHILD_SAs, as no down event - * is triggered below for IKE SAs in this state */ - enumerator = array_create_enumerator(this->child_sas); - while (enumerator->enumerate(enumerator, &child_sa)) + if (child_sa->get_state(child_sa) != CHILD_REKEYED && + child_sa->get_state(child_sa) != CHILD_DELETED) { - if (child_sa->get_state(child_sa) != CHILD_REKEYED && - child_sa->get_state(child_sa) != CHILD_DELETED) - { - charon->bus->child_updown(charon->bus, child_sa, - FALSE); - } + charon->bus->child_updown(charon->bus, child_sa, + FALSE); } - enumerator->destroy(enumerator); } - break; + enumerator->destroy(enumerator); } - case IKE_DELETING: - DBG1(DBG_IKE, "proper IKE_SA delete failed, peer not responding"); - if (has_condition(this, COND_REAUTHENTICATING) && - !lib->settings->get_bool(lib->settings, - "%s.make_before_break", FALSE, lib->ns)) - { - DBG1(DBG_IKE, "delete during reauthentication failed, " - "trying to reestablish IKE_SA anyway"); - reestablish(this); - } - break; - case IKE_REKEYING: - DBG1(DBG_IKE, "rekeying IKE_SA failed, peer not responding"); - /* FALL */ - default: - reestablish(this); - break; - } - if (this->state != IKE_CONNECTING && - this->state != IKE_REKEYED) - { - charon->bus->ike_updown(charon->bus, &this->public, FALSE); + break; } - return DESTROY_ME; + case IKE_DELETING: + DBG1(DBG_IKE, "proper IKE_SA delete failed, peer not responding"); + if (has_condition(this, COND_REAUTHENTICATING) && + !lib->settings->get_bool(lib->settings, + "%s.make_before_break", FALSE, lib->ns)) + { + DBG1(DBG_IKE, "delete during reauthentication failed, " + "trying to reestablish IKE_SA anyway"); + reestablish(this); + } + break; + case IKE_REKEYING: + DBG1(DBG_IKE, "rekeying IKE_SA failed, peer not responding"); + /* FALL */ + default: + reestablish(this); + break; } - return SUCCESS; + if (this->state != IKE_CONNECTING && + this->state != IKE_REKEYED) + { + charon->bus->ike_updown(charon->bus, &this->public, FALSE); + } + return DESTROY_ME; } METHOD(ike_sa_t, set_auth_lifetime, status_t, diff --git a/src/libcharon/sa/ike_sa.h b/src/libcharon/sa/ike_sa.h index 56dd53efe2..b965a49c97 100644 --- a/src/libcharon/sa/ike_sa.h +++ b/src/libcharon/sa/ike_sa.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2006-2019 Tobias Brunner + * Copyright (C) 2006-2020 Tobias Brunner * Copyright (C) 2006 Daniel Roethlisberger * Copyright (C) 2005-2009 Martin Willi * Copyright (C) 2005 Jan Hutter @@ -872,10 +872,11 @@ struct ike_sa_t { * * @param message_id ID of the request to retransmit * @return - * - SUCCESS - * - NOT_FOUND if request doesn't have to be retransmitted + * - SUCCESS if retransmit was sent + * - INVALID_STATE if no retransmit required + * - DESTROY_ME if this IKE_SA MUST be deleted */ - status_t (*retransmit) (ike_sa_t *this, uint32_t message_id); + status_t (*retransmit)(ike_sa_t *this, uint32_t message_id); /** * Sends a DPD request to the peer. diff --git a/src/libcharon/sa/ikev1/task_manager_v1.c b/src/libcharon/sa/ikev1/task_manager_v1.c index 00f5e6f05e..70cebe7355 100644 --- a/src/libcharon/sa/ikev1/task_manager_v1.c +++ b/src/libcharon/sa/ikev1/task_manager_v1.c @@ -404,7 +404,7 @@ static status_t retransmit_packet(private_task_manager_t *this, uint32_t seqnr, METHOD(task_manager_t, retransmit, status_t, private_task_manager_t *this, uint32_t seqnr) { - status_t status = SUCCESS; + status_t status = INVALID_STATE; if (seqnr == this->initiating.seqnr && array_count(this->initiating.packets)) diff --git a/src/libcharon/sa/ikev2/task_manager_v2.c b/src/libcharon/sa/ikev2/task_manager_v2.c index 6e1d008511..9c7a5378d3 100644 --- a/src/libcharon/sa/ikev2/task_manager_v2.c +++ b/src/libcharon/sa/ikev2/task_manager_v2.c @@ -409,7 +409,7 @@ METHOD(task_manager_t, retransmit, status_t, "deferred"); this->ike_sa->set_condition(this->ike_sa, COND_STALE, TRUE); this->initiating.deferred = TRUE; - return SUCCESS; + return INVALID_STATE; } else if (mobike->is_probing(mobike)) { @@ -443,7 +443,7 @@ METHOD(task_manager_t, retransmit, status_t, "deferred"); this->ike_sa->set_condition(this->ike_sa, COND_STALE, TRUE); this->initiating.deferred = TRUE; - return SUCCESS; + return INVALID_STATE; } } @@ -451,8 +451,9 @@ METHOD(task_manager_t, retransmit, status_t, job = (job_t*)retransmit_job_create(this->initiating.mid, this->ike_sa->get_id(this->ike_sa)); lib->scheduler->schedule_job_ms(lib->scheduler, job, timeout); + return SUCCESS; } - return SUCCESS; + return INVALID_STATE; } METHOD(task_manager_t, initiate, status_t, -- 2.39.2