diff options
Diffstat (limited to 'src/charon/sa')
-rw-r--r-- | src/charon/sa/child_sa.c | 73 | ||||
-rw-r--r-- | src/charon/sa/child_sa.h | 32 | ||||
-rw-r--r-- | src/charon/sa/transactions/create_child_sa.c | 121 | ||||
-rw-r--r-- | src/charon/sa/transactions/create_child_sa.h | 36 | ||||
-rw-r--r-- | src/charon/sa/transactions/delete_child_sa.c | 19 | ||||
-rw-r--r-- | src/charon/sa/transactions/transaction.h | 4 |
6 files changed, 236 insertions, 49 deletions
diff --git a/src/charon/sa/child_sa.c b/src/charon/sa/child_sa.c index 223def9f9..033114154 100644 --- a/src/charon/sa/child_sa.c +++ b/src/charon/sa/child_sa.c @@ -110,9 +110,14 @@ struct private_child_sa_t { u_int32_t hard_lifetime; /** - * has this CHILD_SA been rekeyed? + * transaction which is rekeying this CHILD_SA */ - bool rekeyed; + void *rekeying_transaction; + + /** + * has this child SA been rekeyed/is rekeying? + */ + bool is_rekeying; /** * Specifies if NAT traversal is used @@ -322,7 +327,7 @@ static status_t install(private_child_sa_t *this, proposal_t *proposal, prf_plus prf_plus, natt, mine); this->install_time = time(NULL); - + return status; } @@ -477,11 +482,28 @@ static status_t add_policies(private_child_sa_t *this, linked_list_t *my_ts_list } /** - * Implementation of child_sa_t.set_rekeyed. + * Implementation of child_sa_t.set_rekeying_transaction. + */ +static void set_rekeying_transaction(private_child_sa_t *this, void *transaction) +{ + this->rekeying_transaction = transaction; + this->is_rekeying = TRUE; +} + +/** + * Implementation of child_sa_t.get_rekeying_transaction. */ -static void set_rekeyed(private_child_sa_t *this) +static void* get_rekeying_transaction(private_child_sa_t *this) { - this->rekeyed = TRUE; + return this->rekeying_transaction; +} + +/** + * Implementation of child_sa_t.is_rekeying. + */ +static bool is_rekeying(private_child_sa_t *this) +{ + return this->is_rekeying; } /** @@ -685,16 +707,6 @@ static status_t update_sa_hosts(private_child_sa_t *this, host_t *new_me, host_t if (mine) { - src = this->me.addr; - dst = this->other.addr; - new_src = new_me; - new_dst = new_other; - src_changes = my_changes; - dst_changes = other_changes; - spi = this->me.spi; - } - else - { src = this->other.addr; dst = this->me.addr; new_src = new_other; @@ -703,6 +715,16 @@ static status_t update_sa_hosts(private_child_sa_t *this, host_t *new_me, host_t dst_changes = my_changes; spi = this->other.spi; } + else + { + src = this->me.addr; + dst = this->other.addr; + new_src = new_me; + new_dst = new_other; + src_changes = my_changes; + dst_changes = other_changes; + spi = this->me.spi; + } this->logger->log(this->logger, CONTROL|LEVEL1, "updating %s SA 0x%x, from %s:%d..%s:%d to %s:%d..%s:%d", @@ -783,7 +805,7 @@ static status_t update_policy_hosts(private_child_sa_t *this, host_t *new_me, ho static status_t update_hosts(private_child_sa_t *this, host_t *new_me, host_t *new_other, int my_changes, int other_changes) { - if (!my_changes || !other_changes) + if (!my_changes && !other_changes) { return SUCCESS; } @@ -857,7 +879,7 @@ static void destroy(private_child_sa_t *this) /* delete all policies in the kernel */ while (this->policies->remove_last(this->policies, (void**)&policy) == SUCCESS) { - if (!this->rekeyed) + if (!this->is_rekeying) { /* let rekeyed policies, as they are used by another child_sa */ charon->kernel_interface->del_policy(charon->kernel_interface, @@ -883,7 +905,9 @@ static void destroy(private_child_sa_t *this) free(policy); } this->policies->destroy(this->policies); - + + this->me.addr->destroy(this->me.addr); + this->other.addr->destroy(this->other.addr); free(this); } @@ -907,14 +931,16 @@ child_sa_t * child_sa_create(u_int32_t rekey, host_t *me, host_t* other, this->public.update_hosts = (status_t (*)(child_sa_t*,host_t*,host_t*,int,int))update_hosts; this->public.add_policies = (status_t (*)(child_sa_t*, linked_list_t*,linked_list_t*))add_policies; this->public.get_use_time = (status_t (*)(child_sa_t*,bool,time_t*))get_use_time; - this->public.set_rekeyed = (void (*)(child_sa_t*))set_rekeyed; + this->public.set_rekeying_transaction = (void (*)(child_sa_t*,void*))set_rekeying_transaction; + this->public.get_rekeying_transaction = (void* (*)(child_sa_t*))get_rekeying_transaction; + this->public.is_rekeying = (bool (*)(child_sa_t*))is_rekeying; this->public.log_status = (void (*)(child_sa_t*, logger_t*, char*))log_status; this->public.destroy = (void(*)(child_sa_t*))destroy; /* private data */ this->logger = logger_manager->get_logger(logger_manager, CHILD_SA); - this->me.addr = me; - this->other.addr = other; + this->me.addr = me->clone(me); + this->other.addr = other->clone(other); this->me.spi = 0; this->other.spi = 0; this->alloc_ah_spi = 0; @@ -926,7 +952,8 @@ child_sa_t * child_sa_create(u_int32_t rekey, host_t *me, host_t* other, this->reqid = rekey ? rekey : ++reqid; this->policies = linked_list_create(); this->protocol = PROTO_NONE; - this->rekeyed = FALSE; + this->rekeying_transaction = NULL; + this->is_rekeying = FALSE; return &this->public; } diff --git a/src/charon/sa/child_sa.h b/src/charon/sa/child_sa.h index 6c1ca0177..fba0c73a5 100644 --- a/src/charon/sa/child_sa.h +++ b/src/charon/sa/child_sa.h @@ -39,7 +39,7 @@ typedef struct child_sa_t child_sa_t; /** - * @brief Represents multiple IPsec SAs between two hosts. + * @brief Represents an IPsec SAs between two hosts. * * A child_sa_t contains two SAs. SAs for both * directions are managed in one child_sa_t object. Both @@ -168,15 +168,35 @@ struct child_sa_t { status_t (*get_use_time) (child_sa_t *this, bool inbound, time_t *use_time); /** - * @brief Mark this child_sa as rekeyed. + * @brief Set the transaction which rekeys this CHILD_SA. * - * Since an SA which rekeys a old SA shares the same policy, - * we must mark a child_sa as rekeyed. A so marked SA does - * not remove its policy, as the new SA uses it. + * Since either end may initiate CHILD_SA rekeying, we must detect + * such situations to handle them cleanly. A rekeying transaction + * registers itself to the CHILD_SA, and checks later if another + * transaction is in progress of a rekey. * * @param this calling object */ - void (*set_rekeyed) (child_sa_t *this); + void (*set_rekeying_transaction) (child_sa_t *this, void *transaction); + + /** + * @brief Get the transaction which rekeys this CHILD_SA. + * + * See set_rekeying_transactoin + * + * @param this calling object + */ + void* (*get_rekeying_transaction) (child_sa_t *this); + + /** + * @brief Is the CHILD SA rekeying/in progress of rekeying? + * + * This is a readonly parameter. It is set whenever the + * set_rekeying_transaction() method is called. + * + * @param this calling object + */ + bool (*is_rekeying) (child_sa_t *this); /** * @brief Log the status of a child_sa to a logger. diff --git a/src/charon/sa/transactions/create_child_sa.c b/src/charon/sa/transactions/create_child_sa.c index 6fce5e07b..30ccdef06 100644 --- a/src/charon/sa/transactions/create_child_sa.c +++ b/src/charon/sa/transactions/create_child_sa.c @@ -28,7 +28,6 @@ #include <encoding/payloads/sa_payload.h> #include <encoding/payloads/nonce_payload.h> #include <encoding/payloads/ts_payload.h> -#include <sa/child_sa.h> #include <sa/transactions/delete_child_sa.h> #include <utils/randomizer.h> @@ -116,6 +115,11 @@ struct private_create_child_sa_t { child_sa_t *rekeyed_sa; /** + * Have we lost the simultaneous rekeying nonce compare? + */ + bool lost; + + /** * source of randomness */ randomizer_t *randomizer; @@ -143,7 +147,7 @@ static u_int32_t requested(private_create_child_sa_t *this) } /** - * Implementation of transaction_t.rekeys_child. + * Implementation of create_child_sa_t.rekeys_child. */ static void rekeys_child(private_create_child_sa_t *this, child_sa_t *child_sa) { @@ -151,6 +155,15 @@ static void rekeys_child(private_create_child_sa_t *this, child_sa_t *child_sa) } /** + * Implementation of create_child_sa_t.cancel. + */ +static void cancel(private_create_child_sa_t *this) +{ + this->rekeyed_sa = NULL; + this->lost = TRUE; +} + +/** * Implementation of transaction_t.get_request. */ static status_t get_request(private_create_child_sa_t *this, message_t **result) @@ -158,6 +171,15 @@ static status_t get_request(private_create_child_sa_t *this, message_t **result) message_t *request; host_t *me, *other; + /* check if we are not already rekeying */ + if (this->rekeyed_sa && + this->rekeyed_sa->is_rekeying(this->rekeyed_sa)) + { + this->logger->log(this->logger, ERROR, + "rekeying a CHILD_SA which is already rekeying, aborted"); + return FAILED; + } + /* check if we already have built a message (retransmission) */ if (this->message) { @@ -249,8 +271,8 @@ static status_t get_request(private_create_child_sa_t *this, message_t **result) notify->set_spi(notify, this->rekeyed_sa->get_spi(this->rekeyed_sa, TRUE)); request->add_payload(request, (payload_t*)notify); - /* and mark sa with rekeying-in-progress */ - this->rekeyed_sa->set_rekeyed(this->rekeyed_sa); + /* register us as rekeying to detect multiple rekeying */ + this->rekeyed_sa->set_rekeying_transaction(this->rekeyed_sa, &this->public); } return SUCCESS; @@ -605,12 +627,21 @@ static status_t get_response(private_create_child_sa_t *this, message_t *request ts_response = ts_payload_create_from_traffic_selectors(FALSE, this->tsr); response->add_payload(response, (payload_t*)ts_response); } - /* CHILD_SA successfully created. Now we must check if it rekeys an old one, - * and if so, mark the old as rekeyed. It will get deleted from the other - * peer. */ + /* CHILD_SA successfully created. We set us as the rekeying transaction of + * this SA. If we already initiated rekeying of the same SA, we will detect + * this later in the conclude() call. */ if (this->rekeyed_sa) { - this->rekeyed_sa->set_rekeyed(this->rekeyed_sa); + if (this->rekeyed_sa->is_rekeying(this->rekeyed_sa)) + { + /* rekeying already in progress, register us, too */ + this->rekeyed_sa->set_rekeying_transaction(this->rekeyed_sa, &this->public); + } + else + { + /* no rekeying in progress. mark SA as rekeyed, but not conflicted */ + this->rekeyed_sa->set_rekeying_transaction(this->rekeyed_sa, NULL); + } } return SUCCESS; } @@ -628,6 +659,8 @@ static status_t conclude(private_create_child_sa_t *this, message_t *response, ts_payload_t *tsi_payload = NULL; ts_payload_t *tsr_payload = NULL; status_t status; + child_sa_t *new_child = NULL; + delete_child_sa_t *delete_child_sa; /* check message type */ if (response->get_exchange_type(response) != CREATE_CHILD_SA) @@ -726,6 +759,7 @@ static status_t conclude(private_create_child_sa_t *this, message_t *response, "CHILD_SA creation failed"); return FAILED; } + new_child = this->child_sa; if (install_child_sa(this, TRUE) != SUCCESS) { this->logger->log(this->logger, ERROR, @@ -733,14 +767,73 @@ static status_t conclude(private_create_child_sa_t *this, message_t *response, return FAILED; } } - /* CHILD_SA successfully created. If we have rekeyed an old one, delete it */ + /* CHILD_SA successfully created. If the other peer initiated rekeying + * in the meantime, we detect this by comparing the rekeying_transaction + * of the SA. If it changed, we are not alone. Then we must compare the nonces. + * If no simultaneous rekeying is going on, we just initiate the delete of + * the superseded SA. */ if (this->rekeyed_sa) { - delete_child_sa_t *delete_child_sa; + private_create_child_sa_t *other; + + other = (private_create_child_sa_t*) + this->rekeyed_sa->get_rekeying_transaction(this->rekeyed_sa); - delete_child_sa = delete_child_sa_create(this->ike_sa, - this->message_id + 1); - delete_child_sa->set_child_sa(delete_child_sa, this->rekeyed_sa); + /* we are not rekeying anymore, unregister us */ + this->rekeyed_sa->set_rekeying_transaction(this->rekeyed_sa, NULL); + + if (other != this) + { /* simlutaneous rekeying is going on, not so good */ + chunk_t this_lowest, other_lowest; + + /* check if this has a lower nonce the other */ + if (memcmp(this->nonce_i.ptr, this->nonce_r.ptr, + min(this->nonce_i.len, this->nonce_r.len)) < 0) + { + this_lowest = this->nonce_i; + } + else + { + this_lowest = this->nonce_r; + } + if (memcmp(other->nonce_i.ptr, other->nonce_r.ptr, + min(other->nonce_i.len, other->nonce_r.len)) < 0) + { + other_lowest = other->nonce_i; + } + else + { + other_lowest = other->nonce_r; + } + if (memcmp(this_lowest.ptr, other_lowest.ptr, + min(this_lowest.len, other_lowest.len)) < 0) + { + this->logger->log(this->logger, ERROR, + "detected simultaneous CHILD_SA rekeying, but ours is preferred"); + } + else + { + + this->logger->log(this->logger, ERROR, + "detected simultaneous CHILD_SA rekeying, deleting ours"); + this->lost = TRUE; + } + } + /* delete the old SA if we have won the rekeying nonce compare*/ + if (!this->lost) + { + other->rekeyed_sa->set_rekeying_transaction(other->rekeyed_sa, NULL); + delete_child_sa = delete_child_sa_create(this->ike_sa, this->message_id + 1); + delete_child_sa->set_child_sa(delete_child_sa, this->rekeyed_sa); + *next = (transaction_t*)delete_child_sa; + } + } + if (this->lost) + { + /* we have lost simlutaneous rekeying, delete the CHILD_SA we just have created */ + delete_child_sa = delete_child_sa_create(this->ike_sa, this->message_id + 1); + new_child->set_rekeying_transaction(new_child, NULL); + delete_child_sa->set_child_sa(delete_child_sa, new_child); *next = (transaction_t*)delete_child_sa; } return SUCCESS; @@ -788,6 +881,7 @@ create_child_sa_t *create_child_sa_create(ike_sa_t *ike_sa, u_int32_t message_id /* public functions */ this->public.rekeys_child = (void(*)(create_child_sa_t*,child_sa_t*))rekeys_child; + this->public.cancel = (void(*)(create_child_sa_t*))cancel; /* private data */ this->ike_sa = ike_sa; @@ -799,6 +893,7 @@ create_child_sa_t *create_child_sa_create(ike_sa_t *ike_sa, u_int32_t message_id this->nonce_r = CHUNK_INITIALIZER; this->child_sa = NULL; this->rekeyed_sa = NULL; + this->lost = FALSE; this->proposal = NULL; this->tsi = NULL; this->tsr = NULL; diff --git a/src/charon/sa/transactions/create_child_sa.h b/src/charon/sa/transactions/create_child_sa.h index b2bae7728..d3c05a620 100644 --- a/src/charon/sa/transactions/create_child_sa.h +++ b/src/charon/sa/transactions/create_child_sa.h @@ -20,9 +20,8 @@ * for more details. */ - -#ifndef CREATE_CHILD_SA_ -#define CREATE_CHILD_SA_ +#ifndef CREATE_CHILD_SA_H_ +#define CREATE_CHILD_SA_H_ #include <sa/ike_sa.h> #include <sa/child_sa.h> @@ -37,6 +36,23 @@ typedef struct create_child_sa_t create_child_sa_t; * Rekeying of an CHILD_SA works the same way as creating a new one, * but includes an additional REKEY_SA notify and deletes the old * one (in a separate transaction). + * + * ¦__________ _________¦ + * ¦ Cyq \/ Czq ¦ + * ¦__________/\_________¦ + * detect ¦__________ _________¦ detect + * ¦ Czp \/ Czp ¦ + * compare nonces, won ¦__________/\_________¦ compare nonces, lost + * ¦ ¦ + * delete old ¦__________ ¦ + * ¦ Dxq \__________¦ + * ¦ __________¦ + * ¦__________/ Dxp ¦ + * ¦ __________¦ delete created + * ¦__________/ Dzq ¦ + * ¦__________ ¦ + * ¦ Dzp \__________¦ + * * * @b Constructors: * - create_child_sa_create() @@ -61,6 +77,18 @@ struct create_child_sa_t { * @param child_sa CHILD_SA to rekey */ void (*rekeys_child) (create_child_sa_t* this, child_sa_t *child_sa); + + /** + * @brief Cancel a rekeying request. + * + * Cancelling a rekeying request will set a flag in the transaction. When + * the response for the transaction is received, the created CHILD_SA + * gets deleted afterwards. + * + * @param this calling object + * @param child_sa CHILD_SA to rekey + */ + void (*cancel) (create_child_sa_t* this); }; /** @@ -74,4 +102,4 @@ struct create_child_sa_t { */ create_child_sa_t *create_child_sa_create(ike_sa_t *ike_sa, u_int32_t message_id); -#endif /* CREATE_CHILD_SA_ */ +#endif /* CREATE_CHILD_SA_H_ */ diff --git a/src/charon/sa/transactions/delete_child_sa.c b/src/charon/sa/transactions/delete_child_sa.c index 760105204..621d2ab4c 100644 --- a/src/charon/sa/transactions/delete_child_sa.c +++ b/src/charon/sa/transactions/delete_child_sa.c @@ -24,6 +24,7 @@ #include <daemon.h> #include <encoding/payloads/delete_payload.h> +#include <sa/transactions/create_child_sa.h> typedef struct private_delete_child_sa_t private_delete_child_sa_t; @@ -151,7 +152,7 @@ static status_t process_delete(private_delete_child_sa_t *this, delete_payload_t protocol_id_t protocol; u_int32_t spi; iterator_t *iterator; - delete_payload_t *delete_response; + delete_payload_t *delete_response = NULL; /* get requested CHILD */ protocol = delete_request->get_protocol_id(delete_request); @@ -178,9 +179,23 @@ static status_t process_delete(private_delete_child_sa_t *this, delete_payload_t if (child_sa != NULL) { + create_child_sa_t *rekey; + this->logger->log(this->logger, CONTROL, - "received DELETE for %s CHILD_SA with SPI 0x%x, deleting", + "received DELETE for %s CHILD_SA with SPI 0x%x, deleting", mapping_find(protocol_id_m, protocol), ntohl(spi)); + + rekey = child_sa->get_rekeying_transaction(child_sa); + if (rekey) + { + /* we have received a delete for an SA which we are still rekeying. + * this means we have lost the nonce comparison, and the rekeying + * will fail. We set a flag in the transaction for this special case. + */ + this->logger->log(this->logger, CONTROL, + "DELETE received while rekeying, rekeying cancelled"); + rekey->cancel(rekey); + } /* delete it, with inbound spi */ spi = child_sa->get_spi(child_sa, TRUE); this->ike_sa->destroy_child_sa(this->ike_sa, protocol, spi); diff --git a/src/charon/sa/transactions/transaction.h b/src/charon/sa/transactions/transaction.h index ea84b0394..9bb600fe4 100644 --- a/src/charon/sa/transactions/transaction.h +++ b/src/charon/sa/transactions/transaction.h @@ -23,12 +23,14 @@ #ifndef TRANSACTION_H_ #define TRANSACTION_H_ + +typedef struct transaction_t transaction_t; + #include <types.h> #include <encoding/message.h> #include <sa/ike_sa.h> -typedef struct transaction_t transaction_t; /** * @brief This interface represents a transaction an established IKE_SA can do. |