[prev in list] [next in list] [prev in thread] [next in thread]
List: strongswan-announce
Subject: Re: [strongSwan-dev] [PATCH 0/2] Rekeying collisions vs. delete
From: Martin Willi <martin () strongswan ! org>
Date: 2010-06-02 10:00:50
Message-ID: 1275472850.2302.35.camel () martin
[Download RAW message or body]
Btw., attached the patches I pushed to master.
Regards
Martin
["0001-Wrap-getters-for-dpd-close-action-into-CHILD_SA-allo.patch" (0001-Wrap-getters-for-dpd-close-action-into-CHILD_SA-allo.patch)]
> From 4c401ea216b411a5c912190d71c54739515cab43 Mon Sep 17 00:00:00 2001
From: Martin Willi <martin@revosec.ch>
Date: Wed, 2 Jun 2010 11:40:38 +0200
Subject: [PATCH 1/3] Wrap getters for dpd/close action into CHILD_SA, allows us to \
override them
---
src/libcharon/sa/child_sa.c | 48 +++++++++++++++++++++++++++++++++++++++++++
src/libcharon/sa/child_sa.h | 28 +++++++++++++++++++++++++
2 files changed, 76 insertions(+), 0 deletions(-)
diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c
index 8fd2a8c..fb1ed34 100644
--- a/src/libcharon/sa/child_sa.c
+++ b/src/libcharon/sa/child_sa.c
@@ -128,6 +128,16 @@ struct private_child_sa_t {
ipsec_mode_t mode;
/**
+ * Action to enforce if peer closes the CHILD_SA
+ */
+ action_t close_action;
+
+ /**
+ * Action to enforce if peer is considered dead
+ */
+ action_t dpd_action;
+
+ /**
* selected proposal
*/
proposal_t *proposal;
@@ -272,6 +282,38 @@ static void set_ipcomp(private_child_sa_t *this, \
ipcomp_transform_t ipcomp) }
/**
+ * Implementation of child_sa_t.set_close_action.
+ */
+static void set_close_action(private_child_sa_t *this, action_t action)
+{
+ this->close_action = action;
+}
+
+/**
+ * Implementation of child_sa_t.get_close_action.
+ */
+static action_t get_close_action(private_child_sa_t *this)
+{
+ return this->close_action;
+}
+
+/**
+ * Implementation of child_sa_t.set_dpd_action.
+ */
+static void set_dpd_action(private_child_sa_t *this, action_t action)
+{
+ this->dpd_action = action;
+}
+
+/**
+ * Implementation of child_sa_t.get_dpd_action.
+ */
+static action_t get_dpd_action(private_child_sa_t *this)
+{
+ return this->dpd_action;
+}
+
+/**
* Implementation of child_sa_t.get_proposal
*/
static proposal_t* get_proposal(private_child_sa_t *this)
@@ -919,6 +961,10 @@ child_sa_t * child_sa_create(host_t *me, host_t* other,
this->public.has_encap = (bool(*)(child_sa_t*))has_encap;
this->public.get_ipcomp = (ipcomp_transform_t(*)(child_sa_t*))get_ipcomp;
this->public.set_ipcomp = (void(*)(child_sa_t*,ipcomp_transform_t))set_ipcomp;
+ this->public.get_close_action = (action_t(*)(child_sa_t*))get_close_action;
+ this->public.set_close_action = (void(*)(child_sa_t*,action_t))set_close_action;
+ this->public.get_dpd_action = (action_t(*)(child_sa_t*))get_dpd_action;
+ this->public.set_dpd_action = (void(*)(child_sa_t*,action_t))set_dpd_action;
this->public.alloc_spi = (u_int32_t(*)(child_sa_t*, protocol_id_t \
protocol))alloc_spi; this->public.alloc_cpi = (u_int16_t(*)(child_sa_t*))alloc_cpi;
this->public.install = (status_t(*)(child_sa_t*, chunk_t encr, chunk_t integ, \
u_int32_t spi, u_int16_t cpi, bool inbound, linked_list_t *my_ts_list, linked_list_t \
*other_ts_list))install; @@ -946,6 +992,8 @@ child_sa_t * child_sa_create(host_t *me, \
host_t* other, this->other_ts = linked_list_create();
this->protocol = PROTO_NONE;
this->mode = MODE_TUNNEL;
+ this->close_action = config->get_close_action(config);
+ this->dpd_action = config->get_dpd_action(config);
this->proposal = NULL;
this->rekey_time = 0;
this->expire_time = 0;
diff --git a/src/libcharon/sa/child_sa.h b/src/libcharon/sa/child_sa.h
index e6c6035..95bc297 100644
--- a/src/libcharon/sa/child_sa.h
+++ b/src/libcharon/sa/child_sa.h
@@ -208,6 +208,34 @@ struct child_sa_t {
void (*set_ipcomp)(child_sa_t *this, ipcomp_transform_t ipcomp);
/**
+ * Get the action to enforce if the remote peer closes the CHILD_SA.
+ *
+ * @return close action
+ */
+ action_t (*get_close_action)(child_sa_t *this);
+
+ /**
+ * Override the close action specified by the CHILD_SA config.
+ *
+ * @param close action to enforce
+ */
+ void (*set_close_action)(child_sa_t *this, action_t action);
+
+ /**
+ * Get the action to enforce if the peer is considered dead.
+ *
+ * @return dpd action
+ */
+ action_t (*get_dpd_action)(child_sa_t *this);
+
+ /**
+ * Override the DPD action specified by the CHILD_SA config.
+ *
+ * @param close action to enforce
+ */
+ void (*set_dpd_action)(child_sa_t *this, action_t action);
+
+ /**
* Get the selected proposal.
*
* @return selected proposal
--
1.7.0.4
["0002-Use-wrapped-getters-for-close-dpd-action.patch" (0002-Use-wrapped-getters-for-close-dpd-action.patch)]
>From fe02d99b9602c81a804892a67fde2890ef1f6aa6 Mon Sep 17 00:00:00 2001
From: Martin Willi <martin@revosec.ch>
Date: Wed, 2 Jun 2010 11:41:46 +0200
Subject: [PATCH 2/3] Use wrapped getters for close/dpd action
---
src/libcharon/sa/ike_sa.c | 14 +++++++-------
src/libcharon/sa/tasks/child_delete.c | 4 +++-
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/src/libcharon/sa/ike_sa.c b/src/libcharon/sa/ike_sa.c
index 023f074..0b77f5f 100644
--- a/src/libcharon/sa/ike_sa.c
+++ b/src/libcharon/sa/ike_sa.c
@@ -1636,14 +1636,13 @@ static status_t reestablish(private_ike_sa_t *this)
iterator = create_child_sa_iterator(this);
while (iterator->iterate(iterator, (void**)&child_sa))
{
- child_cfg = child_sa->get_config(child_sa);
if (this->state == IKE_DELETING)
{
- action = child_cfg->get_close_action(child_cfg);
+ action = child_sa->get_close_action(child_sa);
}
else
{
- action = child_cfg->get_dpd_action(child_cfg);
+ action = child_sa->get_dpd_action(child_sa);
}
switch (action)
{
@@ -1651,7 +1650,8 @@ static status_t reestablish(private_ike_sa_t *this)
restart = TRUE;
break;
case ACTION_ROUTE:
- charon->traps->install(charon->traps, this->peer_cfg, child_cfg);
+ charon->traps->install(charon->traps, this->peer_cfg,
+ child_sa->get_config(child_sa));
break;
default:
break;
@@ -1707,18 +1707,18 @@ static status_t reestablish(private_ike_sa_t *this)
iterator = create_child_sa_iterator(this);
while (iterator->iterate(iterator, (void**)&child_sa))
{
- child_cfg = child_sa->get_config(child_sa);
if (this->state == IKE_DELETING)
{
- action = child_cfg->get_close_action(child_cfg);
+ action = child_sa->get_close_action(child_sa);
}
else
{
- action = child_cfg->get_dpd_action(child_cfg);
+ action = child_sa->get_dpd_action(child_sa);
}
switch (action)
{
case ACTION_RESTART:
+ child_cfg = child_sa->get_config(child_sa);
DBG1(DBG_IKE, "restarting CHILD_SA %s",
child_cfg->get_name(child_cfg));
child_cfg->get_ref(child_cfg);
diff --git a/src/libcharon/sa/tasks/child_delete.c b/src/libcharon/sa/tasks/child_delete.c
index d7c6b05..b0cd30e 100644
--- a/src/libcharon/sa/tasks/child_delete.c
+++ b/src/libcharon/sa/tasks/child_delete.c
@@ -191,6 +191,7 @@ static status_t destroy_and_reestablish(private_child_delete_t *this)
child_cfg_t *child_cfg;
protocol_id_t protocol;
u_int32_t spi;
+ action_t action;
status_t status = SUCCESS;
iterator = this->child_sas->create_iterator(this->child_sas, TRUE);
@@ -205,10 +206,11 @@ static status_t destroy_and_reestablish(private_child_delete_t *this)
protocol = child_sa->get_protocol(child_sa);
child_cfg = child_sa->get_config(child_sa);
child_cfg->get_ref(child_cfg);
+ action = child_sa->get_close_action(child_sa);
this->ike_sa->destroy_child_sa(this->ike_sa, protocol, spi);
if (this->check_delete_action)
{ /* enforce child_cfg policy if deleted passively */
- switch (child_cfg->get_close_action(child_cfg))
+ switch (action)
{
case ACTION_RESTART:
child_cfg->get_ref(child_cfg);
--
1.7.0.4
["0003-Disable-close-action-for-a-redundant-CHILD_SA-result.patch" (0003-Disable-close-action-for-a-redundant-CHILD_SA-result.patch)]
>From 2f57e6da0e83a3e64e36dd2559b2579b9b1e32a2 Mon Sep 17 00:00:00 2001
From: Martin Willi <martin@revosec.ch>
Date: Wed, 2 Jun 2010 11:43:39 +0200
Subject: [PATCH 3/3] Disable close action for a redundant CHILD_SA resulting from a rekey collision
If a rekey collision is detected, the winning peer of the nonce compare
will delete the redundant CHILD_SA. The other peer should not enforce the
close action on this CHILD, as it would reestablish the redundat CHILD_SA.
Thanks to Thomas Egerer from secunet for pointing this out and the initial
patchset.
---
src/libcharon/sa/tasks/child_rekey.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/src/libcharon/sa/tasks/child_rekey.c b/src/libcharon/sa/tasks/child_rekey.c
index 5331419..fb3452e 100644
--- a/src/libcharon/sa/tasks/child_rekey.c
+++ b/src/libcharon/sa/tasks/child_rekey.c
@@ -234,9 +234,14 @@ static child_sa_t *handle_collision(private_child_rekey_t *this)
if (memcmp(this_nonce.ptr, other_nonce.ptr,
min(this_nonce.len, other_nonce.len)) < 0)
{
+ child_sa_t *child_sa;
+
DBG1(DBG_IKE, "CHILD_SA rekey collision won, "
"deleting rekeyed child");
to_delete = this->child_sa;
+ /* disable close action for the redundand child */
+ child_sa = other->child_create->get_child(other->child_create);
+ child_sa->set_close_action(child_sa, ACTION_NONE);
}
else
{
--
1.7.0.4
_______________________________________________
Dev mailing list
Dev@lists.strongswan.org
https://lists.strongswan.org/mailman/listinfo/dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic