From 3e779ff555f9ce27c23425162233ba0b8761ac5c Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Wed, 15 Oct 2014 17:19:56 +0200 Subject: [PATCH 01/28] libipsec: Remove unused reqid parameter from ipsec_sa_mgr_t.get_spi() --- .../android/jni/libandroidbridge/kernel/android_ipsec.c | 2 +- .../plugins/kernel_libipsec/kernel_libipsec_ipsec.c | 2 +- src/libipsec/ipsec_sa_mgr.c | 8 +++----- src/libipsec/ipsec_sa_mgr.h | 3 +-- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/frontends/android/jni/libandroidbridge/kernel/android_ipsec.c b/src/frontends/android/jni/libandroidbridge/kernel/android_ipsec.c index 24640a07d..84b3b600d 100644 --- a/src/frontends/android/jni/libandroidbridge/kernel/android_ipsec.c +++ b/src/frontends/android/jni/libandroidbridge/kernel/android_ipsec.c @@ -50,7 +50,7 @@ METHOD(kernel_ipsec_t, get_spi, status_t, private_kernel_android_ipsec_t *this, host_t *src, host_t *dst, u_int8_t protocol, u_int32_t reqid, u_int32_t *spi) { - return ipsec->sas->get_spi(ipsec->sas, src, dst, protocol, reqid, spi); + return ipsec->sas->get_spi(ipsec->sas, src, dst, protocol, spi); } METHOD(kernel_ipsec_t, get_cpi, status_t, diff --git a/src/libcharon/plugins/kernel_libipsec/kernel_libipsec_ipsec.c b/src/libcharon/plugins/kernel_libipsec/kernel_libipsec_ipsec.c index bd07a67a2..5ab0c9743 100644 --- a/src/libcharon/plugins/kernel_libipsec/kernel_libipsec_ipsec.c +++ b/src/libcharon/plugins/kernel_libipsec/kernel_libipsec_ipsec.c @@ -238,7 +238,7 @@ METHOD(kernel_ipsec_t, get_spi, status_t, private_kernel_libipsec_ipsec_t *this, host_t *src, host_t *dst, u_int8_t protocol, u_int32_t reqid, u_int32_t *spi) { - return ipsec->sas->get_spi(ipsec->sas, src, dst, protocol, reqid, spi); + return ipsec->sas->get_spi(ipsec->sas, src, dst, protocol, spi); } METHOD(kernel_ipsec_t, get_cpi, status_t, diff --git a/src/libipsec/ipsec_sa_mgr.c b/src/libipsec/ipsec_sa_mgr.c index 1db1776c0..d1850b048 100644 --- a/src/libipsec/ipsec_sa_mgr.c +++ b/src/libipsec/ipsec_sa_mgr.c @@ -396,12 +396,10 @@ static bool allocate_spi(private_ipsec_sa_mgr_t *this, u_int32_t spi) METHOD(ipsec_sa_mgr_t, get_spi, status_t, private_ipsec_sa_mgr_t *this, host_t *src, host_t *dst, u_int8_t protocol, - u_int32_t reqid, u_int32_t *spi) + u_int32_t *spi) { u_int32_t spi_new; - DBG2(DBG_ESP, "allocating SPI for reqid {%u}", reqid); - this->mutex->lock(this->mutex); if (!this->rng) { @@ -420,7 +418,7 @@ METHOD(ipsec_sa_mgr_t, get_spi, status_t, (u_int8_t*)&spi_new)) { this->mutex->unlock(this->mutex); - DBG1(DBG_ESP, "failed to allocate SPI for reqid {%u}", reqid); + DBG1(DBG_ESP, "failed to allocate SPI"); return FAILED; } /* make sure the SPI is valid (not in range 0-255) */ @@ -432,7 +430,7 @@ METHOD(ipsec_sa_mgr_t, get_spi, status_t, *spi = spi_new; - DBG2(DBG_ESP, "allocated SPI %.8x for reqid {%u}", ntohl(*spi), reqid); + DBG2(DBG_ESP, "allocated SPI %.8x", ntohl(*spi)); return SUCCESS; } diff --git a/src/libipsec/ipsec_sa_mgr.h b/src/libipsec/ipsec_sa_mgr.h index 8c234cefa..3e6efaa6d 100644 --- a/src/libipsec/ipsec_sa_mgr.h +++ b/src/libipsec/ipsec_sa_mgr.h @@ -45,12 +45,11 @@ struct ipsec_sa_mgr_t { * @param src source address of the SA * @param dst destination address of the SA * @param protocol protocol of the SA (only ESP supported) - * @param reqid reqid for the SA * @param spi the allocated SPI * @return SUCCESS of operation successful */ status_t (*get_spi)(ipsec_sa_mgr_t *this, host_t *src, host_t *dst, - u_int8_t protocol, u_int32_t reqid, u_int32_t *spi); + u_int8_t protocol, u_int32_t *spi); /** * Add a new SA From 2a1c9e20bd38572ba32ba57436d7064192680729 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Wed, 15 Oct 2014 17:22:55 +0200 Subject: [PATCH 02/28] kernel-interface: Remove reqid parameter from get_spi/get_cpi() methods The reqid is not strictly required, as we set the reqid with the update call when installing the negotiated SA. If we don't need a reqid at this stage, we can later allocate the reqid in the kernel backend once the SA parameters have been fully negotaited. This allows us to assign the same reqid for the same selectors to avoid conflicts on backends this is necessary. --- src/charon-tkm/src/tkm/tkm_kernel_ipsec.c | 5 ++-- .../libandroidbridge/kernel/android_ipsec.c | 4 ++-- .../kernel_libipsec/kernel_libipsec_ipsec.c | 4 ++-- .../plugins/kernel_wfp/kernel_wfp_ipsec.c | 4 ++-- .../plugins/load_tester/load_tester_ipsec.c | 4 ++-- src/libcharon/sa/child_sa.c | 4 ++-- src/libhydra/kernel/kernel_interface.c | 8 +++---- src/libhydra/kernel/kernel_interface.h | 10 ++++---- src/libhydra/kernel/kernel_ipsec.h | 10 ++++---- .../kernel_netlink/kernel_netlink_ipsec.c | 23 ++++++++----------- .../plugins/kernel_pfkey/kernel_pfkey_ipsec.c | 23 ++++++++----------- 11 files changed, 43 insertions(+), 56 deletions(-) diff --git a/src/charon-tkm/src/tkm/tkm_kernel_ipsec.c b/src/charon-tkm/src/tkm/tkm_kernel_ipsec.c index dbeea93f2..d0bf8df3b 100644 --- a/src/charon-tkm/src/tkm/tkm_kernel_ipsec.c +++ b/src/charon-tkm/src/tkm/tkm_kernel_ipsec.c @@ -60,7 +60,7 @@ struct private_tkm_kernel_ipsec_t { METHOD(kernel_ipsec_t, get_spi, status_t, private_tkm_kernel_ipsec_t *this, host_t *src, host_t *dst, - u_int8_t protocol, u_int32_t reqid, u_int32_t *spi) + u_int8_t protocol, u_int32_t *spi) { bool result; @@ -74,7 +74,6 @@ METHOD(kernel_ipsec_t, get_spi, status_t, } } - DBG1(DBG_KNL, "getting SPI for reqid {%u}", reqid); result = this->rng->get_bytes(this->rng, sizeof(u_int32_t), (u_int8_t *)spi); return result ? SUCCESS : FAILED; @@ -82,7 +81,7 @@ METHOD(kernel_ipsec_t, get_spi, status_t, METHOD(kernel_ipsec_t, get_cpi, status_t, private_tkm_kernel_ipsec_t *this, host_t *src, host_t *dst, - u_int32_t reqid, u_int16_t *cpi) + u_int16_t *cpi) { return NOT_SUPPORTED; } diff --git a/src/frontends/android/jni/libandroidbridge/kernel/android_ipsec.c b/src/frontends/android/jni/libandroidbridge/kernel/android_ipsec.c index 84b3b600d..d2a45da82 100644 --- a/src/frontends/android/jni/libandroidbridge/kernel/android_ipsec.c +++ b/src/frontends/android/jni/libandroidbridge/kernel/android_ipsec.c @@ -48,14 +48,14 @@ void expire(u_int32_t reqid, u_int8_t protocol, u_int32_t spi, bool hard) METHOD(kernel_ipsec_t, get_spi, status_t, private_kernel_android_ipsec_t *this, host_t *src, host_t *dst, - u_int8_t protocol, u_int32_t reqid, u_int32_t *spi) + u_int8_t protocol, u_int32_t *spi) { return ipsec->sas->get_spi(ipsec->sas, src, dst, protocol, spi); } METHOD(kernel_ipsec_t, get_cpi, status_t, private_kernel_android_ipsec_t *this, host_t *src, host_t *dst, - u_int32_t reqid, u_int16_t *cpi) + u_int16_t *cpi) { return NOT_SUPPORTED; } diff --git a/src/libcharon/plugins/kernel_libipsec/kernel_libipsec_ipsec.c b/src/libcharon/plugins/kernel_libipsec/kernel_libipsec_ipsec.c index 5ab0c9743..e277063d5 100644 --- a/src/libcharon/plugins/kernel_libipsec/kernel_libipsec_ipsec.c +++ b/src/libcharon/plugins/kernel_libipsec/kernel_libipsec_ipsec.c @@ -236,14 +236,14 @@ METHOD(kernel_ipsec_t, get_features, kernel_feature_t, METHOD(kernel_ipsec_t, get_spi, status_t, private_kernel_libipsec_ipsec_t *this, host_t *src, host_t *dst, - u_int8_t protocol, u_int32_t reqid, u_int32_t *spi) + u_int8_t protocol, u_int32_t *spi) { return ipsec->sas->get_spi(ipsec->sas, src, dst, protocol, spi); } METHOD(kernel_ipsec_t, get_cpi, status_t, private_kernel_libipsec_ipsec_t *this, host_t *src, host_t *dst, - u_int32_t reqid, u_int16_t *cpi) + u_int16_t *cpi) { return NOT_SUPPORTED; } diff --git a/src/libcharon/plugins/kernel_wfp/kernel_wfp_ipsec.c b/src/libcharon/plugins/kernel_wfp/kernel_wfp_ipsec.c index 39e37b1c9..61e820dea 100644 --- a/src/libcharon/plugins/kernel_wfp/kernel_wfp_ipsec.c +++ b/src/libcharon/plugins/kernel_wfp/kernel_wfp_ipsec.c @@ -1981,7 +1981,7 @@ static u_int permute(u_int x, u_int p) METHOD(kernel_ipsec_t, get_spi, status_t, private_kernel_wfp_ipsec_t *this, host_t *src, host_t *dst, - u_int8_t protocol, u_int32_t reqid, u_int32_t *spi) + u_int8_t protocol, u_int32_t *spi) { /* To avoid sequencial SPIs, we use a one-to-one permuation function on * an incrementing counter, that is a full period PRNG for the range we @@ -1998,7 +1998,7 @@ METHOD(kernel_ipsec_t, get_spi, status_t, METHOD(kernel_ipsec_t, get_cpi, status_t, private_kernel_wfp_ipsec_t *this, host_t *src, host_t *dst, - u_int32_t reqid, u_int16_t *cpi) + u_int16_t *cpi) { return NOT_SUPPORTED; } diff --git a/src/libcharon/plugins/load_tester/load_tester_ipsec.c b/src/libcharon/plugins/load_tester/load_tester_ipsec.c index 3f256ddd0..578fc5362 100644 --- a/src/libcharon/plugins/load_tester/load_tester_ipsec.c +++ b/src/libcharon/plugins/load_tester/load_tester_ipsec.c @@ -36,7 +36,7 @@ struct private_load_tester_ipsec_t { METHOD(kernel_ipsec_t, get_spi, status_t, private_load_tester_ipsec_t *this, host_t *src, host_t *dst, - u_int8_t protocol, u_int32_t reqid, u_int32_t *spi) + u_int8_t protocol, u_int32_t *spi) { *spi = (uint32_t)ref_get(&this->spi); return SUCCESS; @@ -44,7 +44,7 @@ METHOD(kernel_ipsec_t, get_spi, status_t, METHOD(kernel_ipsec_t, get_cpi, status_t, private_load_tester_ipsec_t *this, host_t *src, host_t *dst, - u_int32_t reqid, u_int16_t *cpi) + u_int16_t *cpi) { return FAILED; } diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c index 66be5fe61..9026681c2 100644 --- a/src/libcharon/sa/child_sa.c +++ b/src/libcharon/sa/child_sa.c @@ -603,7 +603,7 @@ METHOD(child_sa_t, alloc_spi, u_int32_t, { if (hydra->kernel_interface->get_spi(hydra->kernel_interface, this->other_addr, this->my_addr, - proto_ike2ip(protocol), this->reqid, + proto_ike2ip(protocol), &this->my_spi) == SUCCESS) { /* if we allocate a SPI, but then are unable to establish the SA, we @@ -619,7 +619,7 @@ METHOD(child_sa_t, alloc_cpi, u_int16_t, { if (hydra->kernel_interface->get_cpi(hydra->kernel_interface, this->other_addr, this->my_addr, - this->reqid, &this->my_cpi) == SUCCESS) + &this->my_cpi) == SUCCESS) { return this->my_cpi; } diff --git a/src/libhydra/kernel/kernel_interface.c b/src/libhydra/kernel/kernel_interface.c index 3fa28e054..f479b8723 100644 --- a/src/libhydra/kernel/kernel_interface.c +++ b/src/libhydra/kernel/kernel_interface.c @@ -155,24 +155,24 @@ METHOD(kernel_interface_t, get_features, kernel_feature_t, METHOD(kernel_interface_t, get_spi, status_t, private_kernel_interface_t *this, host_t *src, host_t *dst, - u_int8_t protocol, u_int32_t reqid, u_int32_t *spi) + u_int8_t protocol, u_int32_t *spi) { if (!this->ipsec) { return NOT_SUPPORTED; } - return this->ipsec->get_spi(this->ipsec, src, dst, protocol, reqid, spi); + return this->ipsec->get_spi(this->ipsec, src, dst, protocol, spi); } METHOD(kernel_interface_t, get_cpi, status_t, private_kernel_interface_t *this, host_t *src, host_t *dst, - u_int32_t reqid, u_int16_t *cpi) + u_int16_t *cpi) { if (!this->ipsec) { return NOT_SUPPORTED; } - return this->ipsec->get_cpi(this->ipsec, src, dst, reqid, cpi); + return this->ipsec->get_cpi(this->ipsec, src, dst, cpi); } METHOD(kernel_interface_t, add_sa, status_t, diff --git a/src/libhydra/kernel/kernel_interface.h b/src/libhydra/kernel/kernel_interface.h index cd550383c..f2901673f 100644 --- a/src/libhydra/kernel/kernel_interface.h +++ b/src/libhydra/kernel/kernel_interface.h @@ -104,24 +104,22 @@ struct kernel_interface_t { * @param src source address of SA * @param dst destination address of SA * @param protocol protocol for SA (ESP/AH) - * @param reqid unique ID for this SA * @param spi allocated spi - * @return SUCCESS if operation completed + * @return SUCCESS if operation completed */ status_t (*get_spi)(kernel_interface_t *this, host_t *src, host_t *dst, - u_int8_t protocol, u_int32_t reqid, u_int32_t *spi); + u_int8_t protocol, u_int32_t *spi); /** * Get a Compression Parameter Index (CPI) from the kernel. * * @param src source address of SA * @param dst destination address of SA - * @param reqid unique ID for the corresponding SA * @param cpi allocated cpi - * @return SUCCESS if operation completed + * @return SUCCESS if operation completed */ status_t (*get_cpi)(kernel_interface_t *this, host_t *src, host_t *dst, - u_int32_t reqid, u_int16_t *cpi); + u_int16_t *cpi); /** * Add an SA to the SAD. diff --git a/src/libhydra/kernel/kernel_ipsec.h b/src/libhydra/kernel/kernel_ipsec.h index eec7401e9..286f53ff3 100644 --- a/src/libhydra/kernel/kernel_ipsec.h +++ b/src/libhydra/kernel/kernel_ipsec.h @@ -58,24 +58,22 @@ struct kernel_ipsec_t { * @param src source address of SA * @param dst destination address of SA * @param protocol protocol for SA (ESP/AH) - * @param reqid unique ID for this SA * @param spi allocated spi - * @return SUCCESS if operation completed + * @return SUCCESS if operation completed */ status_t (*get_spi)(kernel_ipsec_t *this, host_t *src, host_t *dst, - u_int8_t protocol, u_int32_t reqid, u_int32_t *spi); + u_int8_t protocol, u_int32_t *spi); /** * Get a Compression Parameter Index (CPI) from the kernel. * * @param src source address of SA * @param dst destination address of SA - * @param reqid unique ID for the corresponding SA * @param cpi allocated cpi - * @return SUCCESS if operation completed + * @return SUCCESS if operation completed */ status_t (*get_cpi)(kernel_ipsec_t *this, host_t *src, host_t *dst, - u_int32_t reqid, u_int16_t *cpi); + u_int16_t *cpi); /** * Add an SA to the SAD. diff --git a/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c b/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c index cfe58bc6b..a9f9c9f59 100644 --- a/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c +++ b/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c @@ -1066,7 +1066,7 @@ METHOD(kernel_ipsec_t, get_features, kernel_feature_t, */ static status_t get_spi_internal(private_kernel_netlink_ipsec_t *this, host_t *src, host_t *dst, u_int8_t proto, u_int32_t min, u_int32_t max, - u_int32_t reqid, u_int32_t *spi) + u_int32_t *spi) { netlink_buf_t request; struct nlmsghdr *hdr, *out; @@ -1086,7 +1086,6 @@ static status_t get_spi_internal(private_kernel_netlink_ipsec_t *this, host2xfrm(dst, &userspi->info.id.daddr); userspi->info.id.proto = proto; userspi->info.mode = XFRM_MODE_TUNNEL; - userspi->info.reqid = reqid; userspi->info.family = src->get_family(src); userspi->min = min; userspi->max = max; @@ -1133,39 +1132,35 @@ static status_t get_spi_internal(private_kernel_netlink_ipsec_t *this, METHOD(kernel_ipsec_t, get_spi, status_t, private_kernel_netlink_ipsec_t *this, host_t *src, host_t *dst, - u_int8_t protocol, u_int32_t reqid, u_int32_t *spi) + u_int8_t protocol, u_int32_t *spi) { - DBG2(DBG_KNL, "getting SPI for reqid {%u}", reqid); - if (get_spi_internal(this, src, dst, protocol, - 0xc0000000, 0xcFFFFFFF, reqid, spi) != SUCCESS) + 0xc0000000, 0xcFFFFFFF, spi) != SUCCESS) { - DBG1(DBG_KNL, "unable to get SPI for reqid {%u}", reqid); + DBG1(DBG_KNL, "unable to get SPI"); return FAILED; } - DBG2(DBG_KNL, "got SPI %.8x for reqid {%u}", ntohl(*spi), reqid); + DBG2(DBG_KNL, "got SPI %.8x", ntohl(*spi)); return SUCCESS; } METHOD(kernel_ipsec_t, get_cpi, status_t, private_kernel_netlink_ipsec_t *this, host_t *src, host_t *dst, - u_int32_t reqid, u_int16_t *cpi) + u_int16_t *cpi) { u_int32_t received_spi = 0; - DBG2(DBG_KNL, "getting CPI for reqid {%u}", reqid); - if (get_spi_internal(this, src, dst, IPPROTO_COMP, - 0x100, 0xEFFF, reqid, &received_spi) != SUCCESS) + 0x100, 0xEFFF, &received_spi) != SUCCESS) { - DBG1(DBG_KNL, "unable to get CPI for reqid {%u}", reqid); + DBG1(DBG_KNL, "unable to get CPI"); return FAILED; } *cpi = htons((u_int16_t)ntohl(received_spi)); - DBG2(DBG_KNL, "got CPI %.4x for reqid {%u}", ntohs(*cpi), reqid); + DBG2(DBG_KNL, "got CPI %.4x", ntohs(*cpi)); return SUCCESS; } diff --git a/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c b/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c index 6b5678270..1d24b7b15 100644 --- a/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c +++ b/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c @@ -1518,7 +1518,7 @@ static bool receive_events(private_kernel_pfkey_ipsec_t *this, int fd, static status_t get_spi_internal(private_kernel_pfkey_ipsec_t *this, host_t *src, host_t *dst, u_int8_t proto, u_int32_t min, u_int32_t max, - u_int32_t reqid, u_int32_t *spi) + u_int32_t *spi) { unsigned char request[PFKEY_BUFFER_SIZE]; struct sadb_msg *msg, *out; @@ -1539,7 +1539,6 @@ static status_t get_spi_internal(private_kernel_pfkey_ipsec_t *this, sa2 = (struct sadb_x_sa2*)PFKEY_EXT_ADD_NEXT(msg); sa2->sadb_x_sa2_exttype = SADB_X_EXT_SA2; sa2->sadb_x_sa2_len = PFKEY_LEN(sizeof(struct sadb_spirange)); - sa2->sadb_x_sa2_reqid = reqid; PFKEY_EXT_ADD(msg, sa2); add_addr_ext(msg, src, SADB_EXT_ADDRESS_SRC, 0, 0, FALSE); @@ -1577,39 +1576,37 @@ static status_t get_spi_internal(private_kernel_pfkey_ipsec_t *this, METHOD(kernel_ipsec_t, get_spi, status_t, private_kernel_pfkey_ipsec_t *this, host_t *src, host_t *dst, - u_int8_t protocol, u_int32_t reqid, u_int32_t *spi) + u_int8_t protocol, u_int32_t *spi) { - DBG2(DBG_KNL, "getting SPI for reqid {%u}", reqid); - if (get_spi_internal(this, src, dst, protocol, - 0xc0000000, 0xcFFFFFFF, reqid, spi) != SUCCESS) + 0xc0000000, 0xcFFFFFFF, spi) != SUCCESS) { - DBG1(DBG_KNL, "unable to get SPI for reqid {%u}", reqid); + DBG1(DBG_KNL, "unable to get SPI"); return FAILED; } - DBG2(DBG_KNL, "got SPI %.8x for reqid {%u}", ntohl(*spi), reqid); + DBG2(DBG_KNL, "got SPI %.8x", ntohl(*spi)); return SUCCESS; } METHOD(kernel_ipsec_t, get_cpi, status_t, private_kernel_pfkey_ipsec_t *this, host_t *src, host_t *dst, - u_int32_t reqid, u_int16_t *cpi) + u_int16_t *cpi) { u_int32_t received_spi = 0; - DBG2(DBG_KNL, "getting CPI for reqid {%u}", reqid); + DBG2(DBG_KNL, "getting CPI"); if (get_spi_internal(this, src, dst, IPPROTO_COMP, - 0x100, 0xEFFF, reqid, &received_spi) != SUCCESS) + 0x100, 0xEFFF, &received_spi) != SUCCESS) { - DBG1(DBG_KNL, "unable to get CPI for reqid {%u}", reqid); + DBG1(DBG_KNL, "unable to get CPI"); return FAILED; } *cpi = htons((u_int16_t)ntohl(received_spi)); - DBG2(DBG_KNL, "got CPI %.4x for reqid {%u}", ntohs(*cpi), reqid); + DBG2(DBG_KNL, "got CPI %.4x", ntohs(*cpi)); return SUCCESS; } From 6ad787ca54ba8f51dce8311dc8bb012a04581b96 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Fri, 17 Oct 2014 14:31:32 +0200 Subject: [PATCH 03/28] kernel-pfkey: Don't update during add_sa(), but delete SPI allocation state --- .../plugins/kernel_pfkey/kernel_pfkey_ipsec.c | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c b/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c index 1d24b7b15..b46f19011 100644 --- a/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c +++ b/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c @@ -1522,7 +1522,6 @@ static status_t get_spi_internal(private_kernel_pfkey_ipsec_t *this, { unsigned char request[PFKEY_BUFFER_SIZE]; struct sadb_msg *msg, *out; - struct sadb_x_sa2 *sa2; struct sadb_spirange *range; pfkey_msg_t response; u_int32_t received_spi = 0; @@ -1536,11 +1535,6 @@ static status_t get_spi_internal(private_kernel_pfkey_ipsec_t *this, msg->sadb_msg_satype = proto2satype(proto); msg->sadb_msg_len = PFKEY_LEN(sizeof(struct sadb_msg)); - sa2 = (struct sadb_x_sa2*)PFKEY_EXT_ADD_NEXT(msg); - sa2->sadb_x_sa2_exttype = SADB_X_EXT_SA2; - sa2->sadb_x_sa2_len = PFKEY_LEN(sizeof(struct sadb_spirange)); - PFKEY_EXT_ADD(msg, sa2); - add_addr_ext(msg, src, SADB_EXT_ADDRESS_SRC, 0, 0, FALSE); add_addr_ext(msg, dst, SADB_EXT_ADDRESS_DST, 0, 0, FALSE); @@ -1641,6 +1635,23 @@ METHOD(kernel_ipsec_t, add_sa, status_t, mode = MODE_TRANSPORT; } + if (inbound) + { + /* As we didn't know the reqid during SPI allocation, we used reqid + * zero. Unfortunately we can't SADB_UPDATE to the new reqid, hence we + * have to delete the SPI allocation state manually. The reqid + * selector does not count for that, therefore we have to delete + * that state before installing the new SA to avoid deleting the + * the new state after installing it. */ + mark_t zeromark = {0, 0}; + + if (this->public.interface.del_sa(&this->public.interface, + src, dst, spi, protocol, 0, zeromark) != SUCCESS) + { + DBG1(DBG_KNL, "deleting SPI allocation SA failed"); + } + } + memset(&request, 0, sizeof(request)); DBG2(DBG_KNL, "adding SAD entry with SPI %.8x and reqid {%u}", @@ -1648,7 +1659,7 @@ METHOD(kernel_ipsec_t, add_sa, status_t, msg = (struct sadb_msg*)request; msg->sadb_msg_version = PF_KEY_V2; - msg->sadb_msg_type = inbound ? SADB_UPDATE : SADB_ADD; + msg->sadb_msg_type = SADB_ADD; msg->sadb_msg_satype = proto2satype(protocol); msg->sadb_msg_len = PFKEY_LEN(sizeof(struct sadb_msg)); From fd9417607c05ebd8dcb1e7394550b4129aa096e9 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Wed, 15 Oct 2014 18:02:46 +0200 Subject: [PATCH 04/28] libipsec: Remove unused src/dst_ts parameters from ipsec_sa_mgr_t.add_sa() --- .../android/jni/libandroidbridge/kernel/android_ipsec.c | 3 +-- .../plugins/kernel_libipsec/kernel_libipsec_ipsec.c | 3 +-- src/libipsec/ipsec_sa.c | 3 +-- src/libipsec/ipsec_sa.h | 6 +----- src/libipsec/ipsec_sa_mgr.c | 5 ++--- src/libipsec/ipsec_sa_mgr.h | 5 +---- 6 files changed, 7 insertions(+), 18 deletions(-) diff --git a/src/frontends/android/jni/libandroidbridge/kernel/android_ipsec.c b/src/frontends/android/jni/libandroidbridge/kernel/android_ipsec.c index d2a45da82..4ed5a426a 100644 --- a/src/frontends/android/jni/libandroidbridge/kernel/android_ipsec.c +++ b/src/frontends/android/jni/libandroidbridge/kernel/android_ipsec.c @@ -71,8 +71,7 @@ METHOD(kernel_ipsec_t, add_sa, status_t, { return ipsec->sas->add_sa(ipsec->sas, src, dst, spi, protocol, reqid, mark, tfc, lifetime, enc_alg, enc_key, int_alg, int_key, - mode, ipcomp, cpi, initiator, encap, esn, inbound, - src_ts, dst_ts); + mode, ipcomp, cpi, initiator, encap, esn, inbound); } METHOD(kernel_ipsec_t, update_sa, status_t, diff --git a/src/libcharon/plugins/kernel_libipsec/kernel_libipsec_ipsec.c b/src/libcharon/plugins/kernel_libipsec/kernel_libipsec_ipsec.c index e277063d5..cb4aa90e5 100644 --- a/src/libcharon/plugins/kernel_libipsec/kernel_libipsec_ipsec.c +++ b/src/libcharon/plugins/kernel_libipsec/kernel_libipsec_ipsec.c @@ -259,8 +259,7 @@ METHOD(kernel_ipsec_t, add_sa, status_t, { return ipsec->sas->add_sa(ipsec->sas, src, dst, spi, protocol, reqid, mark, tfc, lifetime, enc_alg, enc_key, int_alg, int_key, - mode, ipcomp, cpi, initiator, encap, esn, inbound, - src_ts, dst_ts); + mode, ipcomp, cpi, initiator, encap, esn, inbound); } METHOD(kernel_ipsec_t, update_sa, status_t, diff --git a/src/libipsec/ipsec_sa.c b/src/libipsec/ipsec_sa.c index 6ec8bd25e..3d0bbe169 100644 --- a/src/libipsec/ipsec_sa.c +++ b/src/libipsec/ipsec_sa.c @@ -275,8 +275,7 @@ ipsec_sa_t *ipsec_sa_create(u_int32_t spi, host_t *src, host_t *dst, u_int8_t protocol, u_int32_t reqid, mark_t mark, u_int32_t tfc, lifetime_cfg_t *lifetime, u_int16_t enc_alg, chunk_t enc_key, u_int16_t int_alg, chunk_t int_key, ipsec_mode_t mode, - u_int16_t ipcomp, u_int16_t cpi, bool encap, bool esn, bool inbound, - traffic_selector_t *src_ts, traffic_selector_t *dst_ts) + u_int16_t ipcomp, u_int16_t cpi, bool encap, bool esn, bool inbound) { private_ipsec_sa_t *this; diff --git a/src/libipsec/ipsec_sa.h b/src/libipsec/ipsec_sa.h index 5e69f18cf..8dad29ac5 100644 --- a/src/libipsec/ipsec_sa.h +++ b/src/libipsec/ipsec_sa.h @@ -197,8 +197,6 @@ struct ipsec_sa_t { * @param encap enable UDP encapsulation (must be TRUE) * @param esn Extended Sequence Numbers (currently not supported) * @param inbound TRUE if this is an inbound SA, FALSE otherwise - * @param src_ts source traffic selector - * @param dst_ts destination traffic selector * @return the IPsec SA, or NULL if the creation failed */ ipsec_sa_t *ipsec_sa_create(u_int32_t spi, host_t *src, host_t *dst, @@ -207,8 +205,6 @@ ipsec_sa_t *ipsec_sa_create(u_int32_t spi, host_t *src, host_t *dst, u_int16_t enc_alg, chunk_t enc_key, u_int16_t int_alg, chunk_t int_key, ipsec_mode_t mode, u_int16_t ipcomp, u_int16_t cpi, - bool encap, bool esn, bool inbound, - traffic_selector_t *src_ts, - traffic_selector_t *dst_ts); + bool encap, bool esn, bool inbound); #endif /** IPSEC_SA_H_ @}*/ diff --git a/src/libipsec/ipsec_sa_mgr.c b/src/libipsec/ipsec_sa_mgr.c index d1850b048..3496fc79c 100644 --- a/src/libipsec/ipsec_sa_mgr.c +++ b/src/libipsec/ipsec_sa_mgr.c @@ -439,8 +439,7 @@ METHOD(ipsec_sa_mgr_t, add_sa, status_t, u_int8_t protocol, u_int32_t reqid, mark_t mark, u_int32_t tfc, lifetime_cfg_t *lifetime, u_int16_t enc_alg, chunk_t enc_key, u_int16_t int_alg, chunk_t int_key, ipsec_mode_t mode, u_int16_t ipcomp, - u_int16_t cpi, bool initiator, bool encap, bool esn, bool inbound, - traffic_selector_t *src_ts, traffic_selector_t *dst_ts) + u_int16_t cpi, bool initiator, bool encap, bool esn, bool inbound) { ipsec_sa_entry_t *entry; ipsec_sa_t *sa_new; @@ -454,7 +453,7 @@ METHOD(ipsec_sa_mgr_t, add_sa, status_t, sa_new = ipsec_sa_create(spi, src, dst, protocol, reqid, mark, tfc, lifetime, enc_alg, enc_key, int_alg, int_key, mode, - ipcomp, cpi, encap, esn, inbound, src_ts, dst_ts); + ipcomp, cpi, encap, esn, inbound); if (!sa_new) { DBG1(DBG_ESP, "failed to create SAD entry"); diff --git a/src/libipsec/ipsec_sa_mgr.h b/src/libipsec/ipsec_sa_mgr.h index 3e6efaa6d..8d3f64fbb 100644 --- a/src/libipsec/ipsec_sa_mgr.h +++ b/src/libipsec/ipsec_sa_mgr.h @@ -73,8 +73,6 @@ struct ipsec_sa_mgr_t { * @param encap enable UDP encapsulation (must be TRUE) * @param esn Extended Sequence Numbers (currently not supported) * @param inbound TRUE if this is an inbound SA, FALSE otherwise - * @param src_ts source traffic selector - * @param dst_ts destination traffic selector * @return SUCCESS if operation completed */ status_t (*add_sa)(ipsec_sa_mgr_t *this, host_t *src, host_t *dst, @@ -83,8 +81,7 @@ struct ipsec_sa_mgr_t { u_int16_t enc_alg, chunk_t enc_key, u_int16_t int_alg, chunk_t int_key, ipsec_mode_t mode, u_int16_t ipcomp, u_int16_t cpi, bool initiator, bool encap, bool esn, - bool inbound, traffic_selector_t *src_ts, - traffic_selector_t *dst_ts); + bool inbound); /** * Update the hosts on an installed SA. From d05d85fe658321aff8d859eaf50704a21299b683 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Wed, 15 Oct 2014 18:03:31 +0200 Subject: [PATCH 05/28] kernel-interface: Pass full list of traffic selectors to add_sa() While we can handle the first selector only in BEET mode in kernel-netlink, passing the full list gives the backend more flexibility how to handle this information. --- src/charon-tkm/src/tkm/tkm_kernel_ipsec.c | 2 +- .../jni/libandroidbridge/kernel/android_ipsec.c | 2 +- .../kernel_libipsec/kernel_libipsec_ipsec.c | 2 +- .../plugins/kernel_wfp/kernel_wfp_ipsec.c | 2 +- .../plugins/load_tester/load_tester_ipsec.c | 2 +- src/libcharon/sa/child_sa.c | 14 ++++++-------- src/libhydra/kernel/kernel_interface.c | 2 +- src/libhydra/kernel/kernel_interface.h | 13 +++++-------- src/libhydra/kernel/kernel_ipsec.h | 13 +++++-------- .../plugins/kernel_netlink/kernel_netlink_ipsec.c | 8 +++++--- .../plugins/kernel_pfkey/kernel_pfkey_ipsec.c | 2 +- 11 files changed, 28 insertions(+), 34 deletions(-) diff --git a/src/charon-tkm/src/tkm/tkm_kernel_ipsec.c b/src/charon-tkm/src/tkm/tkm_kernel_ipsec.c index d0bf8df3b..da8e0ee64 100644 --- a/src/charon-tkm/src/tkm/tkm_kernel_ipsec.c +++ b/src/charon-tkm/src/tkm/tkm_kernel_ipsec.c @@ -93,7 +93,7 @@ METHOD(kernel_ipsec_t, add_sa, status_t, u_int16_t int_alg, chunk_t int_key, ipsec_mode_t mode, u_int16_t ipcomp, u_int16_t cpi, u_int32_t replay_window, bool _initiator, bool encap, bool esn, bool inbound, - traffic_selector_t* src_ts, traffic_selector_t* dst_ts) + linked_list_t* src_ts, linked_list_t* dst_ts) { esa_info_t esa; bool initiator; diff --git a/src/frontends/android/jni/libandroidbridge/kernel/android_ipsec.c b/src/frontends/android/jni/libandroidbridge/kernel/android_ipsec.c index 4ed5a426a..65166077e 100644 --- a/src/frontends/android/jni/libandroidbridge/kernel/android_ipsec.c +++ b/src/frontends/android/jni/libandroidbridge/kernel/android_ipsec.c @@ -67,7 +67,7 @@ METHOD(kernel_ipsec_t, add_sa, status_t, u_int16_t int_alg, chunk_t int_key, ipsec_mode_t mode, u_int16_t ipcomp, u_int16_t cpi, u_int32_t replay_window, bool initiator, bool encap, bool esn, bool inbound, - traffic_selector_t *src_ts, traffic_selector_t *dst_ts) + linked_list_t *src_ts, linked_list_t *dst_ts) { return ipsec->sas->add_sa(ipsec->sas, src, dst, spi, protocol, reqid, mark, tfc, lifetime, enc_alg, enc_key, int_alg, int_key, diff --git a/src/libcharon/plugins/kernel_libipsec/kernel_libipsec_ipsec.c b/src/libcharon/plugins/kernel_libipsec/kernel_libipsec_ipsec.c index cb4aa90e5..362b32746 100644 --- a/src/libcharon/plugins/kernel_libipsec/kernel_libipsec_ipsec.c +++ b/src/libcharon/plugins/kernel_libipsec/kernel_libipsec_ipsec.c @@ -255,7 +255,7 @@ METHOD(kernel_ipsec_t, add_sa, status_t, u_int16_t int_alg, chunk_t int_key, ipsec_mode_t mode, u_int16_t ipcomp, u_int16_t cpi, u_int32_t replay_window, bool initiator, bool encap, bool esn, bool inbound, - traffic_selector_t *src_ts, traffic_selector_t *dst_ts) + linked_list_t *src_ts, linked_list_t *dst_ts) { return ipsec->sas->add_sa(ipsec->sas, src, dst, spi, protocol, reqid, mark, tfc, lifetime, enc_alg, enc_key, int_alg, int_key, diff --git a/src/libcharon/plugins/kernel_wfp/kernel_wfp_ipsec.c b/src/libcharon/plugins/kernel_wfp/kernel_wfp_ipsec.c index 61e820dea..9fd6541a5 100644 --- a/src/libcharon/plugins/kernel_wfp/kernel_wfp_ipsec.c +++ b/src/libcharon/plugins/kernel_wfp/kernel_wfp_ipsec.c @@ -2107,7 +2107,7 @@ METHOD(kernel_ipsec_t, add_sa, status_t, u_int16_t int_alg, chunk_t int_key, ipsec_mode_t mode, u_int16_t ipcomp, u_int16_t cpi, u_int32_t replay_window, bool initiator, bool encap, bool esn, bool inbound, - traffic_selector_t *src_ts, traffic_selector_t *dst_ts) + linked_list_t *src_ts, linked_list_t *dst_ts) { host_t *local, *remote; entry_t *entry; diff --git a/src/libcharon/plugins/load_tester/load_tester_ipsec.c b/src/libcharon/plugins/load_tester/load_tester_ipsec.c index 578fc5362..68af4794d 100644 --- a/src/libcharon/plugins/load_tester/load_tester_ipsec.c +++ b/src/libcharon/plugins/load_tester/load_tester_ipsec.c @@ -56,7 +56,7 @@ METHOD(kernel_ipsec_t, add_sa, status_t, u_int16_t int_alg, chunk_t int_key, ipsec_mode_t mode, u_int16_t ipcomp, u_int16_t cpi, u_int32_t replay_window, bool initiator, bool encap, bool esn, bool inbound, - traffic_selector_t *src_ts, traffic_selector_t *dst_ts) + linked_list_t *src_ts, linked_list_t *dst_ts) { return SUCCESS; } diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c index 9026681c2..ed9e9b087 100644 --- a/src/libcharon/sa/child_sa.c +++ b/src/libcharon/sa/child_sa.c @@ -633,7 +633,7 @@ METHOD(child_sa_t, install, status_t, { u_int16_t enc_alg = ENCR_UNDEFINED, int_alg = AUTH_UNDEFINED, size; u_int16_t esn = NO_EXT_SEQ_NUMBERS; - traffic_selector_t *src_ts = NULL, *dst_ts = NULL; + linked_list_t *src_ts = NULL, *dst_ts = NULL; time_t now; lifetime_cfg_t *lifetime; u_int32_t tfc = 0; @@ -705,18 +705,16 @@ METHOD(child_sa_t, install, status_t, lifetime->time.rekey = 0; } - /* BEET requires the bound address from the traffic selectors. - * TODO: We add just the first traffic selector for now, as the - * kernel accepts a single TS per SA only */ + /* BEET requires the bound address from the traffic selectors */ if (inbound) { - my_ts->get_first(my_ts, (void**)&dst_ts); - other_ts->get_first(other_ts, (void**)&src_ts); + dst_ts = my_ts; + src_ts = other_ts; } else { - my_ts->get_first(my_ts, (void**)&src_ts); - other_ts->get_first(other_ts, (void**)&dst_ts); + src_ts = my_ts; + dst_ts = other_ts; } status = hydra->kernel_interface->add_sa(hydra->kernel_interface, diff --git a/src/libhydra/kernel/kernel_interface.c b/src/libhydra/kernel/kernel_interface.c index f479b8723..1cb001548 100644 --- a/src/libhydra/kernel/kernel_interface.c +++ b/src/libhydra/kernel/kernel_interface.c @@ -182,7 +182,7 @@ METHOD(kernel_interface_t, add_sa, status_t, u_int16_t int_alg, chunk_t int_key, ipsec_mode_t mode, u_int16_t ipcomp, u_int16_t cpi, u_int32_t replay_window, bool initiator, bool encap, bool esn, bool inbound, - traffic_selector_t *src_ts, traffic_selector_t *dst_ts) + linked_list_t *src_ts, linked_list_t *dst_ts) { if (!this->ipsec) { diff --git a/src/libhydra/kernel/kernel_interface.h b/src/libhydra/kernel/kernel_interface.h index f2901673f..a94c58a64 100644 --- a/src/libhydra/kernel/kernel_interface.h +++ b/src/libhydra/kernel/kernel_interface.h @@ -124,11 +124,8 @@ struct kernel_interface_t { /** * Add an SA to the SAD. * - * add_sa() may update an already allocated - * SPI (via get_spi). In this case, the replace - * flag must be set. - * This function does install a single SA for a - * single protocol in one direction. + * This function does install a single SA for a single protocol in one + * direction. * * @param src source address for this SA * @param dst destination address for this SA @@ -150,8 +147,8 @@ struct kernel_interface_t { * @param encap enable UDP encapsulation for NAT traversal * @param esn TRUE to use Extended Sequence Numbers * @param inbound TRUE if this is an inbound SA - * @param src_ts traffic selector with BEET source address - * @param dst_ts traffic selector with BEET destination address + * @param src_ts list of source traffic selectors + * @param dst_ts list of destination traffic selectors * @return SUCCESS if operation completed */ status_t (*add_sa) (kernel_interface_t *this, @@ -163,7 +160,7 @@ struct kernel_interface_t { ipsec_mode_t mode, u_int16_t ipcomp, u_int16_t cpi, u_int32_t replay_window, bool initiator, bool encap, bool esn, bool inbound, - traffic_selector_t *src_ts, traffic_selector_t *dst_ts); + linked_list_t *src_ts, linked_list_t *dst_ts); /** * Update the hosts on an installed SA. diff --git a/src/libhydra/kernel/kernel_ipsec.h b/src/libhydra/kernel/kernel_ipsec.h index 286f53ff3..f6705ff88 100644 --- a/src/libhydra/kernel/kernel_ipsec.h +++ b/src/libhydra/kernel/kernel_ipsec.h @@ -78,11 +78,8 @@ struct kernel_ipsec_t { /** * Add an SA to the SAD. * - * add_sa() may update an already allocated - * SPI (via get_spi). In this case, the replace - * flag must be set. - * This function does install a single SA for a - * single protocol in one direction. + * This function does install a single SA for a single protocol in one + * direction. * * @param src source address for this SA * @param dst destination address for this SA @@ -104,8 +101,8 @@ struct kernel_ipsec_t { * @param encap enable UDP encapsulation for NAT traversal * @param esn TRUE to use Extended Sequence Numbers * @param inbound TRUE if this is an inbound SA - * @param src_ts traffic selector with BEET source address - * @param dst_ts traffic selector with BEET destination address + * @param src_ts list of source traffic selectors + * @param dst_ts list of destination traffic selectors * @return SUCCESS if operation completed */ status_t (*add_sa) (kernel_ipsec_t *this, @@ -117,7 +114,7 @@ struct kernel_ipsec_t { ipsec_mode_t mode, u_int16_t ipcomp, u_int16_t cpi, u_int32_t replay_window, bool initiator, bool encap, bool esn, bool inbound, - traffic_selector_t *src_ts, traffic_selector_t *dst_ts); + linked_list_t *src_ts, linked_list_t *dst_ts); /** * Update the hosts on an installed SA. diff --git a/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c b/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c index a9f9c9f59..31bb4f656 100644 --- a/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c +++ b/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c @@ -1191,7 +1191,7 @@ METHOD(kernel_ipsec_t, add_sa, status_t, u_int16_t int_alg, chunk_t int_key, ipsec_mode_t mode, u_int16_t ipcomp, u_int16_t cpi, u_int32_t replay_window, bool initiator, bool encap, bool esn, bool inbound, - traffic_selector_t* src_ts, traffic_selector_t* dst_ts) + linked_list_t* src_ts, linked_list_t* dst_ts) { netlink_buf_t request; char *alg_name; @@ -1199,6 +1199,7 @@ METHOD(kernel_ipsec_t, add_sa, status_t, struct xfrm_usersa_info *sa; u_int16_t icv_size = 64; ipsec_mode_t original_mode = mode; + traffic_selector_t *first_src_ts, *first_dst_ts; status_t status = FAILED; /* if IPComp is used, we install an additional IPComp SA. if the cpi is 0 @@ -1244,9 +1245,10 @@ METHOD(kernel_ipsec_t, add_sa, status_t, * selector can be installed other traffic would get dropped */ break; } - if (src_ts && dst_ts) + if (src_ts->get_first(src_ts, (void**)&first_src_ts) == SUCCESS && + dst_ts->get_first(dst_ts, (void**)&first_dst_ts) == SUCCESS) { - sa->sel = ts2selector(src_ts, dst_ts); + sa->sel = ts2selector(first_src_ts, first_dst_ts); if (!this->proto_port_transport) { /* don't install proto/port on SA. This would break diff --git a/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c b/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c index b46f19011..348549bfa 100644 --- a/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c +++ b/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c @@ -1611,7 +1611,7 @@ METHOD(kernel_ipsec_t, add_sa, status_t, u_int16_t int_alg, chunk_t int_key, ipsec_mode_t mode, u_int16_t ipcomp, u_int16_t cpi, u_int32_t replay_window, bool initiator, bool encap, bool esn, bool inbound, - traffic_selector_t *src_ts, traffic_selector_t *dst_ts) + linked_list_t *src_ts, linked_list_t *dst_ts) { unsigned char request[PFKEY_BUFFER_SIZE]; struct sadb_msg *msg, *out; From 698ed656f7578fbef0769216579417b32d8eb57e Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Fri, 17 Oct 2014 15:44:06 +0200 Subject: [PATCH 06/28] child-sa: Remove the obsolete update logic The kernel backend uses an inbound parameter these days, where it makes no sense to pass the update flag. The kernel backend decides itself how it handles SA installation based on the inbound flag. --- src/libcharon/sa/child_sa.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c index ed9e9b087..6f5e046c5 100644 --- a/src/libcharon/sa/child_sa.c +++ b/src/libcharon/sa/child_sa.c @@ -639,7 +639,6 @@ METHOD(child_sa_t, install, status_t, u_int32_t tfc = 0; host_t *src, *dst; status_t status; - bool update = FALSE; /* now we have to decide which spi to use. Use self allocated, if "in", * or the one in the proposal, if not "in" (others). Additionally, @@ -648,10 +647,6 @@ METHOD(child_sa_t, install, status_t, { dst = this->my_addr; src = this->other_addr; - if (this->my_spi == spi) - { /* alloc_spi has been called, do an SA update */ - update = TRUE; - } this->my_spi = spi; this->my_cpi = cpi; } @@ -722,7 +717,7 @@ METHOD(child_sa_t, install, status_t, inbound ? this->mark_in : this->mark_out, tfc, lifetime, enc_alg, encr, int_alg, integ, this->mode, this->ipcomp, cpi, this->config->get_replay_window(this->config), - initiator, this->encap, esn, update, src_ts, dst_ts); + initiator, this->encap, esn, inbound, src_ts, dst_ts); free(lifetime); From 373a147fed5406acb9533ae3a8f2c7a01b26fab0 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 19 Feb 2015 18:01:38 +0100 Subject: [PATCH 07/28] traffic-selector: Add a compare function to sort traffic selectors --- .../selectors/traffic_selector.c | 109 ++++++++++++------ .../selectors/traffic_selector.h | 11 ++ 2 files changed, 86 insertions(+), 34 deletions(-) diff --git a/src/libstrongswan/selectors/traffic_selector.c b/src/libstrongswan/selectors/traffic_selector.c index 94b77467a..1cfa3ae48 100644 --- a/src/libstrongswan/selectors/traffic_selector.c +++ b/src/libstrongswan/selectors/traffic_selector.c @@ -449,41 +449,9 @@ METHOD(traffic_selector_t, get_subset, traffic_selector_t*, } METHOD(traffic_selector_t, equals, bool, - private_traffic_selector_t *this, traffic_selector_t *other_public) + private_traffic_selector_t *this, traffic_selector_t *other) { - private_traffic_selector_t *other; - - other = (private_traffic_selector_t*)other_public; - if (this->type != other->type) - { - return FALSE; - } - if (!(this->from_port == other->from_port && - this->to_port == other->to_port && - this->protocol == other->protocol)) - { - return FALSE; - } - switch (this->type) - { - case TS_IPV4_ADDR_RANGE: - if (memeq(this->from4, other->from4, sizeof(this->from4)) && - memeq(this->to4, other->to4, sizeof(this->to4))) - { - return TRUE; - } - break; - case TS_IPV6_ADDR_RANGE: - if (memeq(this->from6, other->from6, sizeof(this->from6)) && - memeq(this->to6, other->to6, sizeof(this->to6))) - { - return TRUE; - } - break; - default: - break; - } - return FALSE; + return traffic_selector_cmp(&this->public, other, NULL) == 0; } METHOD(traffic_selector_t, get_from_address, chunk_t, @@ -723,6 +691,79 @@ METHOD(traffic_selector_t, destroy, void, free(this); } +/** + * Compare two integers + */ +static int compare_int(int a, int b) +{ + return a - b; +} + +/* + * See header + */ +int traffic_selector_cmp(traffic_selector_t *a_pub, traffic_selector_t *b_pub, + void *opts) +{ + private_traffic_selector_t *a, *b; + int res; + + a = (private_traffic_selector_t*)a_pub; + b = (private_traffic_selector_t*)b_pub; + + /* IPv4 before IPv6 */ + res = compare_int(a->type, b->type); + if (res) + { + return res; + } + switch (a->type) + { + case TS_IPV4_ADDR_RANGE: + /* lower starting subnets first */ + res = memcmp(a->from4, b->from4, sizeof(a->from4)); + if (res) + { + return res; + } + /* larger subnets first */ + res = memcmp(b->to4, a->to4, sizeof(a->to4)); + if (res) + { + return res; + } + break; + case TS_IPV6_ADDR_RANGE: + res = memcmp(a->from6, b->from6, sizeof(a->from6)); + if (res) + { + return res; + } + res = memcmp(b->to6, a->to6, sizeof(a->to6)); + if (res) + { + return res; + } + break; + default: + return 1; + } + /* lower protocols first */ + res = compare_int(a->protocol, b->protocol); + if (res) + { + return res; + } + /* lower starting ports first */ + res = compare_int(a->from_port, b->from_port); + if (res) + { + return res; + } + /* larger port ranges first */ + return compare_int(b->to_port, a->to_port); +} + /* * see header */ diff --git a/src/libstrongswan/selectors/traffic_selector.h b/src/libstrongswan/selectors/traffic_selector.h index ab6813acc..93300460e 100644 --- a/src/libstrongswan/selectors/traffic_selector.h +++ b/src/libstrongswan/selectors/traffic_selector.h @@ -248,6 +248,17 @@ static inline u_int8_t traffic_selector_icmp_code(u_int16_t port) return port & 0xff; } +/** + * Compare two traffic selectors, usable as sort function + * + * @param a first selector to compare + * @param b second selector to compare + * @param opts optional sort options, currently unused + * @return > 0 if a > b, 0 if a == b, < 0 if a < b + */ +int traffic_selector_cmp(traffic_selector_t *a, traffic_selector_t *b, + void *opts); + /** * Create a new traffic selector using human readable params. * From fc978f192f1b33e65816b5d65bbeee28eb0dc8e2 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 19 Feb 2015 18:02:25 +0100 Subject: [PATCH 08/28] unit-tests: Add an initial set of traffic selector unit tests --- src/libstrongswan/tests/Makefile.am | 1 + .../tests/suites/test_traffic_selector.c | 279 ++++++++++++++++++ src/libstrongswan/tests/tests.h | 1 + 3 files changed, 281 insertions(+) create mode 100644 src/libstrongswan/tests/suites/test_traffic_selector.c diff --git a/src/libstrongswan/tests/Makefile.am b/src/libstrongswan/tests/Makefile.am index 85626cd44..8c081c673 100644 --- a/src/libstrongswan/tests/Makefile.am +++ b/src/libstrongswan/tests/Makefile.am @@ -29,6 +29,7 @@ tests_SOURCES = tests.h tests.c \ suites/test_enum.c \ suites/test_hashtable.c \ suites/test_identification.c \ + suites/test_traffic_selector.c \ suites/test_threading.c \ suites/test_process.c \ suites/test_watcher.c \ diff --git a/src/libstrongswan/tests/suites/test_traffic_selector.c b/src/libstrongswan/tests/suites/test_traffic_selector.c new file mode 100644 index 000000000..bb44bf5a0 --- /dev/null +++ b/src/libstrongswan/tests/suites/test_traffic_selector.c @@ -0,0 +1,279 @@ +/* + * Copyright (C) 2015 Martin Willi + * Copyright (C) 2015 revosec AG + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. See . + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + */ + +#include "test_suite.h" + +#include + + +static void verify(const char *str, traffic_selector_t *ts) +{ + char buf[512]; + + ck_assert(ts != NULL); + snprintf(buf, sizeof(buf), "%R", ts); + ts->destroy(ts); + ck_assert_str_eq(buf, str); +} + +START_TEST(test_create_from_string) +{ + verify("10.1.0.0/16[tcp/http]", + traffic_selector_create_from_string(IPPROTO_TCP, TS_IPV4_ADDR_RANGE, + "10.1.0.0", 80, "10.1.255.255", 80)); + verify("10.1.0.1..10.1.0.99[udp/1234-1235]", + traffic_selector_create_from_string(IPPROTO_UDP, TS_IPV4_ADDR_RANGE, + "10.1.0.1", 1234, "10.1.0.99", 1235)); + verify("fec1::/64", + traffic_selector_create_from_string(0, TS_IPV6_ADDR_RANGE, + "fec1::", 0, "fec1::ffff:ffff:ffff:ffff", 65535)); +} +END_TEST + +START_TEST(test_create_from_cidr) +{ + verify("10.1.0.0/16", + traffic_selector_create_from_cidr("10.1.0.0/16", 0, 0, 65535)); + verify("10.1.0.1/32[udp/1234-1235]", + traffic_selector_create_from_cidr("10.1.0.1/32", IPPROTO_UDP, + 1234, 1235)); +} +END_TEST + +START_TEST(test_create_from_bytes) +{ + verify("10.1.0.0/16", + traffic_selector_create_from_bytes(0, TS_IPV4_ADDR_RANGE, + chunk_from_chars(0x0a,0x01,0x00,0x00), 0, + chunk_from_chars(0x0a,0x01,0xff,0xff), 65535)); +} +END_TEST + +START_TEST(test_create_from_subnet) +{ + verify("10.1.0.0/16", + traffic_selector_create_from_subnet( + host_create_from_string("10.1.0.0", 0), 16, 0, 0, 65535)); +} +END_TEST + + +START_TEST(test_subset) +{ + traffic_selector_t *a, *b; + + a = traffic_selector_create_from_cidr("10.1.0.0/16", 0, 0, 65535); + b = traffic_selector_create_from_cidr("10.1.5.0/24", 0, 0, 65535); + verify("10.1.5.0/24", a->get_subset(a, b)); + a->destroy(a); + b->destroy(b); +} +END_TEST + +START_TEST(test_subset_port) +{ + traffic_selector_t *a, *b; + + a = traffic_selector_create_from_cidr("10.0.0.0/8", IPPROTO_TCP, 55, 60); + b = traffic_selector_create_from_cidr("10.2.7.16/30", 0, 0, 65535); + verify("10.2.7.16/30[tcp/55-60]", a->get_subset(a, b)); + a->destroy(a); + b->destroy(b); +} +END_TEST + +START_TEST(test_subset_equal) +{ + traffic_selector_t *a, *b; + + a = traffic_selector_create_from_cidr("10.1.0.0/16", IPPROTO_TCP, 80, 80); + b = traffic_selector_create_from_cidr("10.1.0.0/16", IPPROTO_TCP, 80, 80); + verify("10.1.0.0/16[tcp/http]", a->get_subset(a, b)); + a->destroy(a); + b->destroy(b); +} +END_TEST + +START_TEST(test_subset_nonet) +{ + traffic_selector_t *a, *b; + + a = traffic_selector_create_from_cidr("10.1.0.0/16", 0, 0, 65535); + b = traffic_selector_create_from_cidr("10.2.0.0/16", 0, 0, 65535); + ck_assert(a->get_subset(a, b) == NULL); + a->destroy(a); + b->destroy(b); +} +END_TEST + +START_TEST(test_subset_noport) +{ + traffic_selector_t *a, *b; + + a = traffic_selector_create_from_cidr("10.1.0.0/16", 0, 0, 9999); + b = traffic_selector_create_from_cidr("10.1.0.0/16", 0, 10000, 65535); + ck_assert(a->get_subset(a, b) == NULL); + a->destroy(a); + b->destroy(b); +} +END_TEST + +START_TEST(test_subset_noproto) +{ + traffic_selector_t *a, *b; + + a = traffic_selector_create_from_cidr("10.1.0.0/16", IPPROTO_TCP, 0, 65535); + b = traffic_selector_create_from_cidr("10.1.0.0/16", IPPROTO_UDP, 0, 65535); + ck_assert(a->get_subset(a, b) == NULL); + a->destroy(a); + b->destroy(b); +} +END_TEST + +START_TEST(test_subset_nofamily) +{ + traffic_selector_t *a, *b; + + a = traffic_selector_create_from_cidr("0.0.0.0/0", 0, 0, 65535); + b = traffic_selector_create_from_cidr("::/0", 0, 0, 65535); + ck_assert(a->get_subset(a, b) == NULL); + a->destroy(a); + b->destroy(b); +} +END_TEST + +struct { + char *net; + char *host; + bool inc; +} include_tests[] = { + { "0.0.0.0/0", "192.168.1.2", TRUE }, + { "::/0", "fec2::1", TRUE }, + { "fec2::/64", "fec2::afaf", TRUE }, + { "10.1.0.0/16", "10.1.0.1", TRUE }, + { "10.5.6.7/32", "10.5.6.7", TRUE }, + { "0.0.0.0/0", "fec2::1", FALSE }, + { "::/0", "1.2.3.4", FALSE }, + { "10.0.0.0/16", "10.1.0.0", FALSE }, + { "fec2::/64", "fec2:0:0:1::afaf", FALSE }, +}; + +START_TEST(test_includes) +{ + traffic_selector_t *ts; + host_t *h; + + ts = traffic_selector_create_from_cidr(include_tests[_i].net, 0, 0, 65535); + h = host_create_from_string(include_tests[_i].host, 0); + ck_assert(ts->includes(ts, h) == include_tests[_i].inc); + ts->destroy(ts); + h->destroy(h); +} +END_TEST + +struct { + int res; + struct { + char *net; + u_int8_t proto; + u_int16_t from_port; + u_int16_t to_port; + } a, b; +} cmp_tests[] = { + { 0, { "10.0.0.0/8", 0, 0, 65535 }, { "10.0.0.0/8", 0, 0, 65535 }, }, + { 0, { "10.0.0.0/8", 17, 123, 456 }, { "10.0.0.0/8", 17, 123, 456 }, }, + { 0, { "fec2::/64", 0, 0, 65535 }, { "fec2::/64", 0, 0, 65535 }, }, + { 0, { "fec2::/64", 4, 0, 65535 }, { "fec2::/64", 4, 0, 65535 }, }, + + { -1, { "1.0.0.0/8", 0, 0, 65535 }, { "2.0.0.0/8", 0, 0, 65535 }, }, + { 1, { "2.0.0.0/8", 0, 0, 65535 }, { "1.0.0.0/8", 0, 0, 65535 }, }, + { -1, { "1.0.0.0/8", 0, 0, 65535 }, { "1.0.0.0/16", 0, 0, 65535 }, }, + { 1, { "1.0.0.0/16", 0, 0, 65535 }, { "1.0.0.0/8", 0, 0, 65535 }, }, + + { -1, { "10.0.0.0/8", 0, 0, 65535 }, { "fec2::/64", 0, 0, 65535 }, }, + { 1, { "fec2::/64", 0, 0, 65535 }, { "10.0.0.0/8", 0, 0, 65535 }, }, + + { -1, { "10.0.0.0/8", 16, 123, 456 }, { "10.0.0.0/8", 17, 123, 456 }, }, + { 1, { "fec2::/64", 5, 0, 65535 }, { "fec2::/64", 4, 0, 65535 }, }, + + { -1, { "10.0.0.0/8", 17, 111, 456 }, { "10.0.0.0/8", 17, 222, 456 }, }, + { 1, { "fec2::/64", 17, 555, 65535 }, { "fec2::/64", 17, 444, 65535 },}, + + { -1, { "10.0.0.0/8", 17, 55, 65535 }, { "10.0.0.0/8", 17, 55, 666 }, }, + { 1, { "fec2::/64", 17, 55, 111 }, { "fec2::/64", 17, 55, 4567 }, }, + +}; + +START_TEST(test_cmp) +{ + traffic_selector_t *a, *b; + + a = traffic_selector_create_from_cidr( + cmp_tests[_i].a.net, cmp_tests[_i].a.proto, + cmp_tests[_i].a.from_port, cmp_tests[_i].a.to_port); + b = traffic_selector_create_from_cidr( + cmp_tests[_i].b.net, cmp_tests[_i].b.proto, + cmp_tests[_i].b.from_port, cmp_tests[_i].b.to_port); + switch (cmp_tests[_i].res) + { + case 0: + ck_assert(traffic_selector_cmp(a, b, NULL) == 0); + break; + case 1: + ck_assert(traffic_selector_cmp(a, b, NULL) > 0); + break; + case -1: + ck_assert(traffic_selector_cmp(a, b, NULL) < 0); + break; + } + a->destroy(a); + b->destroy(b); +} +END_TEST + +Suite *traffic_selector_suite_create() +{ + Suite *s; + TCase *tc; + + s = suite_create("traffic selector"); + + tc = tcase_create("create"); + tcase_add_test(tc, test_create_from_string); + tcase_add_test(tc, test_create_from_cidr); + tcase_add_test(tc, test_create_from_bytes); + tcase_add_test(tc, test_create_from_subnet); + suite_add_tcase(s, tc); + + tc = tcase_create("subset"); + tcase_add_test(tc, test_subset); + tcase_add_test(tc, test_subset_port); + tcase_add_test(tc, test_subset_equal); + tcase_add_test(tc, test_subset_nonet); + tcase_add_test(tc, test_subset_noport); + tcase_add_test(tc, test_subset_noproto); + tcase_add_test(tc, test_subset_nofamily); + suite_add_tcase(s, tc); + + tc = tcase_create("includes"); + tcase_add_loop_test(tc, test_includes, 0, countof(include_tests)); + suite_add_tcase(s, tc); + + tc = tcase_create("cmp"); + tcase_add_loop_test(tc, test_cmp, 0, countof(cmp_tests)); + suite_add_tcase(s, tc); + + return s; +} diff --git a/src/libstrongswan/tests/tests.h b/src/libstrongswan/tests/tests.h index 440e38a9a..d01158649 100644 --- a/src/libstrongswan/tests/tests.h +++ b/src/libstrongswan/tests/tests.h @@ -23,6 +23,7 @@ TEST_SUITE(linked_list_enumerator_suite_create) TEST_SUITE(hashtable_suite_create) TEST_SUITE(array_suite_create) TEST_SUITE(identification_suite_create) +TEST_SUITE(traffic_selector_suite_create) TEST_SUITE(threading_suite_create) TEST_SUITE(process_suite_create) TEST_SUITE(watcher_suite_create) From 5499473c332c7369496f48c28be4fd15703dad7b Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 19 Feb 2015 18:03:08 +0100 Subject: [PATCH 09/28] child-sa: Sort traffic selectors after adding CHILD_SA policies Having traffic selectors sorted properly makes comparing them much simpler. --- src/libcharon/sa/child_sa.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c index 6f5e046c5..2028aba73 100644 --- a/src/libcharon/sa/child_sa.c +++ b/src/libcharon/sa/child_sa.c @@ -799,12 +799,15 @@ METHOD(child_sa_t, add_policies, status_t, array_insert(this->my_ts, ARRAY_TAIL, my_ts->clone(my_ts)); } enumerator->destroy(enumerator); + array_sort(this->my_ts, (void*)traffic_selector_cmp, NULL); + enumerator = other_ts_list->create_enumerator(other_ts_list); while (enumerator->enumerate(enumerator, &other_ts)) { array_insert(this->other_ts, ARRAY_TAIL, other_ts->clone(other_ts)); } enumerator->destroy(enumerator); + array_sort(this->other_ts, (void*)traffic_selector_cmp, NULL); if (this->config->install_policy(this->config)) { From 6d4649573ec2d8d89ef1130d63192c672c4b707b Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 23 Oct 2014 11:19:03 +0200 Subject: [PATCH 10/28] traffic-selector: Add a hash() method --- src/libstrongswan/selectors/traffic_selector.c | 12 ++++++++++++ src/libstrongswan/selectors/traffic_selector.h | 8 ++++++++ 2 files changed, 20 insertions(+) diff --git a/src/libstrongswan/selectors/traffic_selector.c b/src/libstrongswan/selectors/traffic_selector.c index 1cfa3ae48..3b7f8c5a0 100644 --- a/src/libstrongswan/selectors/traffic_selector.c +++ b/src/libstrongswan/selectors/traffic_selector.c @@ -685,6 +685,17 @@ METHOD(traffic_selector_t, clone_, traffic_selector_t*, } } +METHOD(traffic_selector_t, hash, u_int, + private_traffic_selector_t *this, u_int hash) +{ + return chunk_hash_inc(get_from_address(this), + chunk_hash_inc(get_to_address(this), + chunk_hash_inc(chunk_from_thing(this->from_port), + chunk_hash_inc(chunk_from_thing(this->to_port), + chunk_hash_inc(chunk_from_thing(this->protocol), + hash))))); +} + METHOD(traffic_selector_t, destroy, void, private_traffic_selector_t *this) { @@ -974,6 +985,7 @@ static private_traffic_selector_t *traffic_selector_create(u_int8_t protocol, .set_address = _set_address, .to_subnet = _to_subnet, .clone = _clone_, + .hash = _hash, .destroy = _destroy, }, .from_port = from_port, diff --git a/src/libstrongswan/selectors/traffic_selector.h b/src/libstrongswan/selectors/traffic_selector.h index 93300460e..cf9a2861b 100644 --- a/src/libstrongswan/selectors/traffic_selector.h +++ b/src/libstrongswan/selectors/traffic_selector.h @@ -220,6 +220,14 @@ struct traffic_selector_t { */ bool (*to_subnet) (traffic_selector_t *this, host_t **net, u_int8_t *mask); + /** + * Create a hash value for the traffic selector. + * + * @param inc optional value for incremental hashing + * @return calculated hash value for the traffic selector + */ + u_int (*hash)(traffic_selector_t *this, u_int inc); + /** * Destroys the ts object */ From cc08ce83f06d2158b5306b6f15a2701873fd8c3f Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Wed, 12 Nov 2014 17:22:45 +0100 Subject: [PATCH 11/28] kernel-interface: Add reqid allocation and release functions To reassign reqids where appropriate, we explicitly allocate or confirm them centrally on the kernel-interface. Currently the state is stored in the kernel-interface wrapper for all backends, but we may add appropriate methods to each backend to implement a custom reqid allocation logic, if required. --- src/libhydra/kernel/kernel_interface.c | 292 +++++++++++++++++++++++++ src/libhydra/kernel/kernel_interface.h | 38 +++- 2 files changed, 329 insertions(+), 1 deletion(-) diff --git a/src/libhydra/kernel/kernel_interface.c b/src/libhydra/kernel/kernel_interface.c index 1cb001548..9452b8f84 100644 --- a/src/libhydra/kernel/kernel_interface.c +++ b/src/libhydra/kernel/kernel_interface.c @@ -43,6 +43,8 @@ #include #include #include +#include +#include typedef struct private_kernel_interface_t private_kernel_interface_t; @@ -114,6 +116,16 @@ struct private_kernel_interface_t { */ linked_list_t *listeners; + /** + * Reqid entries indexed by reqids + */ + hashtable_t *reqids; + + /** + * Reqid entries indexed by traffic selectors + */ + hashtable_t *reqids_by_ts; + /** * mutex for algorithm mappings */ @@ -175,6 +187,278 @@ METHOD(kernel_interface_t, get_cpi, status_t, return this->ipsec->get_cpi(this->ipsec, src, dst, cpi); } +/** + * Reqid mapping entry + */ +typedef struct { + /** allocated reqid */ + u_int32_t reqid; + /** references to this entry */ + u_int refs; + /** inbound mark used for SA */ + mark_t mark_in; + /** outbound mark used for SA */ + mark_t mark_out; + /** local traffic selectors */ + array_t *local; + /** remote traffic selectors */ + array_t *remote; +} reqid_entry_t; + +/** + * Destroy a reqid mapping entry + */ +static void reqid_entry_destroy(reqid_entry_t *entry) +{ + array_destroy_offset(entry->local, offsetof(traffic_selector_t, destroy)); + array_destroy_offset(entry->remote, offsetof(traffic_selector_t, destroy)); + free(entry); +} + +/** + * Hashtable hash function for reqid entries using reqid as key + */ +static u_int hash_reqid(reqid_entry_t *entry) +{ + return chunk_hash_inc(chunk_from_thing(entry->reqid), + chunk_hash_inc(chunk_from_thing(entry->mark_in), + chunk_hash(chunk_from_thing(entry->mark_out)))); +} + +/** + * Hashtable equals function for reqid entries using reqid as key + */ +static bool equals_reqid(reqid_entry_t *a, reqid_entry_t *b) +{ + return a->reqid == b->reqid && + a->mark_in.value == b->mark_in.value && + a->mark_in.mask == b->mark_in.mask && + a->mark_out.value == b->mark_out.value && + a->mark_out.mask == b->mark_out.mask; +} + +/** + * Hash an array of traffic selectors + */ +static u_int hash_ts_array(array_t *array, u_int hash) +{ + enumerator_t *enumerator; + traffic_selector_t *ts; + + enumerator = array_create_enumerator(array); + while (enumerator->enumerate(enumerator, &ts)) + { + hash = ts->hash(ts, hash); + } + enumerator->destroy(enumerator); + + return hash; +} + +/** + * Hashtable hash function for reqid entries using traffic selectors as key + */ +static u_int hash_reqid_by_ts(reqid_entry_t *entry) +{ + return hash_ts_array(entry->local, hash_ts_array(entry->remote, 0)); +} + +/** + * Compare two array with traffic selectors for equality + */ +static bool ts_array_equals(array_t *a, array_t *b) +{ + traffic_selector_t *tsa, *tsb; + enumerator_t *ae, *be; + bool equal = TRUE; + + if (array_count(a) != array_count(b)) + { + return FALSE; + } + + ae = array_create_enumerator(a); + be = array_create_enumerator(b); + while (equal && ae->enumerate(ae, &tsa) && be->enumerate(be, &tsb)) + { + equal = tsa->equals(tsa, tsb); + } + ae->destroy(ae); + be->destroy(be); + + return equal; +} + +/** + * Check if mark b matches to a, optionally with reqid match + */ +static bool mark_matches(mark_t a, mark_t b, u_int32_t reqid) +{ + if (a.value == b.value) + { + return TRUE; + } + if (a.value == MARK_REQID && b.value == reqid) + { + return TRUE; + } + return FALSE; +} + +/** + * Hashtable equals function for reqid entries using traffic selectors as key + */ +static bool equals_reqid_by_ts(reqid_entry_t *a, reqid_entry_t *b) +{ + if (ts_array_equals(a->local, b->local) && + ts_array_equals(a->remote, b->remote) && + a->mark_in.mask == b->mark_in.mask && + a->mark_out.mask == b->mark_out.mask) + { + if (mark_matches(a->mark_in, b->mark_in, a->reqid) && + mark_matches(a->mark_out, b->mark_out, a->reqid)) + { + return TRUE; + } + if (mark_matches(b->mark_in, a->mark_in, b->reqid) && + mark_matches(b->mark_out, a->mark_out, b->reqid)) + { + return TRUE; + } + } + return FALSE; +} + +/** + * Create an array from copied traffic selector list items + */ +static array_t *array_from_ts_list(linked_list_t *list) +{ + enumerator_t *enumerator; + traffic_selector_t *ts; + array_t *array; + + array = array_create(0, 0); + + enumerator = list->create_enumerator(list); + while (enumerator->enumerate(enumerator, &ts)) + { + array_insert(array, ARRAY_TAIL, ts->clone(ts)); + } + enumerator->destroy(enumerator); + + return array; +} + +METHOD(kernel_interface_t, alloc_reqid, status_t, + private_kernel_interface_t *this, + linked_list_t *local_ts, linked_list_t *remote_ts, + mark_t *mark_in, mark_t *mark_out, u_int32_t *reqid) +{ + static u_int32_t counter = 0; + reqid_entry_t *entry = NULL, *tmpl; + status_t status = SUCCESS; + + INIT(tmpl, + .local = array_from_ts_list(local_ts), + .remote = array_from_ts_list(remote_ts), + .mark_in = *mark_in, + .mark_out = *mark_out, + .reqid = *reqid, + ); + + this->mutex->lock(this->mutex); + if (tmpl->reqid) + { + /* search by reqid if given */ + if (tmpl->mark_in.value == MARK_REQID) + { + tmpl->mark_in.value = tmpl->reqid; + } + if (tmpl->mark_out.value == MARK_REQID) + { + tmpl->mark_out.value = tmpl->reqid; + } + entry = this->reqids->get(this->reqids, tmpl); + } + if (entry) + { + /* we don't require a traffic selector match for explicit reqids, + * as we wan't to reuse a reqid for trap-triggered policies that + * got narrowed during negotiation. */ + reqid_entry_destroy(tmpl); + } + else + { + /* search by traffic selectors. We do the search with MARK_REQID + * wildcards (if any), and update the marks if we find any match */ + entry = this->reqids_by_ts->get(this->reqids_by_ts, tmpl); + if (entry) + { + reqid_entry_destroy(tmpl); + } + else + { + /* none found, create a new entry, allocating a reqid */ + entry = tmpl; + entry->reqid = ++counter; + if (entry->mark_in.value == MARK_REQID) + { + entry->mark_in.value = entry->reqid; + } + if (entry->mark_out.value == MARK_REQID) + { + entry->mark_out.value = entry->reqid; + } + this->reqids_by_ts->put(this->reqids_by_ts, entry, entry); + this->reqids->put(this->reqids, entry, entry); + } + *reqid = entry->reqid; + } + *mark_in = entry->mark_in; + *mark_out = entry->mark_out; + entry->refs++; + this->mutex->unlock(this->mutex); + + return status; +} + +METHOD(kernel_interface_t, release_reqid, status_t, + private_kernel_interface_t *this, u_int32_t reqid, + mark_t mark_in, mark_t mark_out) +{ + reqid_entry_t *entry, tmpl = { + .reqid = reqid, + .mark_in = mark_in, + .mark_out = mark_out, + }; + + this->mutex->lock(this->mutex); + entry = this->reqids->remove(this->reqids, &tmpl); + if (entry) + { + if (--entry->refs == 0) + { + entry = this->reqids_by_ts->remove(this->reqids_by_ts, entry); + if (entry) + { + reqid_entry_destroy(entry); + } + } + else + { + this->reqids->put(this->reqids, entry, entry); + } + } + this->mutex->unlock(this->mutex); + + if (entry) + { + return SUCCESS; + } + return NOT_FOUND; +} + METHOD(kernel_interface_t, add_sa, status_t, private_kernel_interface_t *this, host_t *src, host_t *dst, u_int32_t spi, u_int8_t protocol, u_int32_t reqid, mark_t mark, @@ -733,6 +1017,8 @@ METHOD(kernel_interface_t, destroy, void, DESTROY_IF(this->ipsec); DESTROY_IF(this->net); DESTROY_FUNCTION_IF(this->ifaces_filter, (void*)free); + this->reqids->destroy(this->reqids); + this->reqids_by_ts->destroy(this->reqids_by_ts); this->listeners->destroy(this->listeners); this->mutex->destroy(this->mutex); free(this); @@ -751,6 +1037,8 @@ kernel_interface_t *kernel_interface_create() .get_features = _get_features, .get_spi = _get_spi, .get_cpi = _get_cpi, + .alloc_reqid = _alloc_reqid, + .release_reqid = _release_reqid, .add_sa = _add_sa, .update_sa = _update_sa, .query_sa = _query_sa, @@ -795,6 +1083,10 @@ kernel_interface_t *kernel_interface_create() .listeners = linked_list_create(), .mutex_algs = mutex_create(MUTEX_TYPE_DEFAULT), .algorithms = linked_list_create(), + .reqids = hashtable_create((hashtable_hash_t)hash_reqid, + (hashtable_equals_t)equals_reqid, 8), + .reqids_by_ts = hashtable_create((hashtable_hash_t)hash_reqid_by_ts, + (hashtable_equals_t)equals_reqid_by_ts, 8), ); ifaces = lib->settings->get_str(lib->settings, diff --git a/src/libhydra/kernel/kernel_interface.h b/src/libhydra/kernel/kernel_interface.h index a94c58a64..f25c10830 100644 --- a/src/libhydra/kernel/kernel_interface.h +++ b/src/libhydra/kernel/kernel_interface.h @@ -121,6 +121,42 @@ struct kernel_interface_t { status_t (*get_cpi)(kernel_interface_t *this, host_t *src, host_t *dst, u_int16_t *cpi); + /** + * Allocate or confirm a reqid to use for a given SA pair. + * + * Each returned reqid by a successful call to alloc_reqid() must be + * released using release_reqid(). + * + * The reqid parameter is an in/out parameter. If it points to non-zero, + * the reqid is confirmed and registered for use. If it points to zero, + * a reqid is allocated for the given selectors, and returned to reqid. + * + * The passed mark values get updated to the reqid value if they are set + * to the magic value MARK_REQID. + * + * @param local_ts traffic selectors of local side for SA + * @param remote_ts traffic selectors of remote side for SA + * @param mark_in inbound mark on SA + * @param mark_out outbound mark on SA + * @param reqid allocated reqid + * @return SUCCESS if reqid allocated + */ + status_t (*alloc_reqid)(kernel_interface_t *this, + linked_list_t *local_ts, linked_list_t *remote_ts, + mark_t *mark_in, mark_t *mark_out, + u_int32_t *reqid); + + /** + * Release a previously allocated reqid. + * + * @param reqid reqid to release + * @param mark_in inbound mark on SA + * @param mark_out outbound mark on SA + * @return SUCCESS if reqid released + */ + status_t (*release_reqid)(kernel_interface_t *this, u_int32_t reqid, + mark_t mark_in, mark_t mark_out); + /** * Add an SA to the SAD. * @@ -131,7 +167,7 @@ struct kernel_interface_t { * @param dst destination address for this SA * @param spi SPI allocated by us or remote peer * @param protocol protocol for this SA (ESP/AH) - * @param reqid unique ID for this SA + * @param reqid reqid for this SA * @param mark optional mark for this SA * @param tfc Traffic Flow Confidentiality padding for this SA * @param lifetime lifetime_cfg_t for this SA From 0da97f506094f25cdc51b06e332b148cfc65f24d Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Tue, 21 Oct 2014 11:36:18 +0200 Subject: [PATCH 12/28] child-sa: Delegate reqid allocation to the kernel interface --- src/libcharon/sa/child_sa.c | 61 ++++++++++++++++++++++++++++--------- 1 file changed, 46 insertions(+), 15 deletions(-) diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c index 2028aba73..af9e75470 100644 --- a/src/libcharon/sa/child_sa.c +++ b/src/libcharon/sa/child_sa.c @@ -100,6 +100,11 @@ struct private_child_sa_t { */ u_int32_t reqid; + /** + * Did we allocate/confirm and must release the reqid? + */ + bool reqid_allocated; + /** * inbound mark used for this child_sa */ @@ -676,6 +681,18 @@ METHOD(child_sa_t, install, status_t, this->proposal->get_algorithm(this->proposal, EXTENDED_SEQUENCE_NUMBERS, &esn, NULL); + if (!this->reqid_allocated) + { + status = hydra->kernel_interface->alloc_reqid(hydra->kernel_interface, + my_ts, other_ts, &this->mark_in, &this->mark_out, + &this->reqid); + if (status != SUCCESS) + { + return status; + } + this->reqid_allocated = TRUE; + } + lifetime = this->config->get_lifetime(this->config); now = time_monotonic(NULL); @@ -792,6 +809,19 @@ METHOD(child_sa_t, add_policies, status_t, traffic_selector_t *my_ts, *other_ts; status_t status = SUCCESS; + if (!this->reqid_allocated) + { + /* trap policy, get or confirm reqid */ + status = hydra->kernel_interface->alloc_reqid( + hydra->kernel_interface, my_ts_list, other_ts_list, + &this->mark_in, &this->mark_out, &this->reqid); + if (status != SUCCESS) + { + return status; + } + this->reqid_allocated = TRUE; + } + /* apply traffic selectors */ enumerator = my_ts_list->create_enumerator(my_ts_list); while (enumerator->enumerate(enumerator, &my_ts)) @@ -1100,6 +1130,15 @@ METHOD(child_sa_t, destroy, void, enumerator->destroy(enumerator); } + if (this->reqid_allocated) + { + if (hydra->kernel_interface->release_reqid(hydra->kernel_interface, + this->reqid, this->mark_in, this->mark_out) != SUCCESS) + { + DBG1(DBG_CHD, "releasing reqid %u failed", this->reqid); + } + } + array_destroy_offset(this->my_ts, offsetof(traffic_selector_t, destroy)); array_destroy_offset(this->other_ts, offsetof(traffic_selector_t, destroy)); this->my_addr->destroy(this->my_addr); @@ -1150,7 +1189,6 @@ static host_t* get_proxy_addr(child_cfg_t *config, host_t *ike, bool local) child_sa_t * child_sa_create(host_t *me, host_t* other, child_cfg_t *config, u_int32_t rekey, bool encap) { - static refcount_t reqid = 0; private_child_sa_t *this; INIT(this, @@ -1208,7 +1246,13 @@ child_sa_t * child_sa_create(host_t *me, host_t* other, if (!this->reqid) { - /* reuse old reqid if we are rekeying an existing CHILD_SA */ + /* reuse old reqid if we are rekeying an existing CHILD_SA. While the + * reqid cache would find the same reqid for our selectors, this does + * not work in a special case: If an SA is triggered by a trap policy, + * but the negotiated SA gets narrowed, we still must reuse the same + * reqid to succesfully "trigger" the SA on the kernel level. Rekeying + * such an SA requires an explicit reqid, as the cache currently knows + * the original selectors only for that reqid. */ if (rekey) { this->reqid = rekey; @@ -1216,22 +1260,9 @@ child_sa_t * child_sa_create(host_t *me, host_t* other, else { this->reqid = charon->traps->find_reqid(charon->traps, config); - if (!this->reqid) - { - this->reqid = ref_get(&reqid); - } } } - if (this->mark_in.value == MARK_REQID) - { - this->mark_in.value = this->reqid; - } - if (this->mark_out.value == MARK_REQID) - { - this->mark_out.value = this->reqid; - } - /* MIPv6 proxy transport mode sets SA endpoints to TS hosts */ if (config->get_mode(config) == MODE_TRANSPORT && config->use_proxy_mode(config)) From 4ec397b89431576813f9a7511bf297ee312a5878 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 23 Oct 2014 11:27:25 +0200 Subject: [PATCH 13/28] child-sa: Introduce a unique CHILD_SA identifier As the reqid is not that unique even among multiple IKE_SAs anymore, we need an identifier to uniquely identify a specific CHILD_SA instance. --- src/libcharon/sa/child_sa.c | 14 ++++++++++++++ src/libcharon/sa/child_sa.h | 10 ++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c index af9e75470..7625be1d6 100644 --- a/src/libcharon/sa/child_sa.c +++ b/src/libcharon/sa/child_sa.c @@ -105,6 +105,11 @@ struct private_child_sa_t { */ bool reqid_allocated; + /* + * Unique CHILD_SA identifier + */ + u_int32_t unique_id; + /** * inbound mark used for this child_sa */ @@ -234,6 +239,12 @@ METHOD(child_sa_t, get_reqid, u_int32_t, return this->reqid; } +METHOD(child_sa_t, get_unique_id, u_int32_t, + private_child_sa_t *this) +{ + return this->unique_id; +} + METHOD(child_sa_t, get_config, child_cfg_t*, private_child_sa_t *this) { @@ -1190,11 +1201,13 @@ child_sa_t * child_sa_create(host_t *me, host_t* other, child_cfg_t *config, u_int32_t rekey, bool encap) { private_child_sa_t *this; + static refcount_t unique_id = 0; INIT(this, .public = { .get_name = _get_name, .get_reqid = _get_reqid, + .get_unique_id = _get_unique_id, .get_config = _get_config, .get_state = _get_state, .set_state = _set_state, @@ -1236,6 +1249,7 @@ child_sa_t * child_sa_create(host_t *me, host_t* other, .close_action = config->get_close_action(config), .dpd_action = config->get_dpd_action(config), .reqid = config->get_reqid(config), + .unique_id = ref_get(&unique_id), .mark_in = config->get_mark(config, TRUE), .mark_out = config->get_mark(config, FALSE), .install_time = time_monotonic(NULL), diff --git a/src/libcharon/sa/child_sa.h b/src/libcharon/sa/child_sa.h index 30a0edf42..f0ec01658 100644 --- a/src/libcharon/sa/child_sa.h +++ b/src/libcharon/sa/child_sa.h @@ -125,6 +125,16 @@ struct child_sa_t { */ u_int32_t (*get_reqid)(child_sa_t *this); + /** + * Get the unique numerical identifier for this CHILD_SA. + * + * While the same reqid might be shared between multiple SAs, the unique_id + * is truly unique for all CHILD_SA instances. + * + * @return unique CHILD_SA identifier + */ + u_int32_t (*get_unique_id)(child_sa_t *this); + /** * Get the config used to set up this child sa. * From 85b238887d01c030a7d9240db2031601211a6283 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 13 Nov 2014 15:26:10 +0100 Subject: [PATCH 14/28] child-sa: Replace reqid based marks by "unique" marks As we now use the same reqid for multiple CHILD_SAs with the same selectors, having marks based on the reqid makes not that much sense anymore. Instead we use unique marks that use a custom identifier. This identifier is reused during rekeying, keeping the marks constant for any rule relying on it (for example installed by updown). This also simplifies handling of reqid allocation, as we do not have to query the marks that is not yet assigned for an unknown reqid. --- src/libcharon/plugins/ha/ha_dispatcher.c | 3 +- src/libcharon/sa/child_sa.c | 31 +++++++-- src/libcharon/sa/child_sa.h | 5 +- src/libcharon/sa/ikev1/task_manager_v1.c | 2 + src/libcharon/sa/ikev1/tasks/quick_mode.c | 30 ++++++++- src/libcharon/sa/ikev1/tasks/quick_mode.h | 8 +++ src/libcharon/sa/ikev2/tasks/child_create.c | 26 +++++++- src/libcharon/sa/ikev2/tasks/child_create.h | 8 +++ src/libcharon/sa/ikev2/tasks/child_rekey.c | 6 ++ src/libcharon/sa/trap_manager.c | 2 +- src/libhydra/kernel/kernel_interface.c | 70 ++++----------------- src/libhydra/kernel/kernel_interface.h | 5 +- src/libstrongswan/ipsec/ipsec_types.h | 4 +- 13 files changed, 126 insertions(+), 74 deletions(-) diff --git a/src/libcharon/plugins/ha/ha_dispatcher.c b/src/libcharon/plugins/ha/ha_dispatcher.c index e20e872c1..6e02733f9 100644 --- a/src/libcharon/plugins/ha/ha_dispatcher.c +++ b/src/libcharon/plugins/ha/ha_dispatcher.c @@ -718,7 +718,8 @@ static void process_child_add(private_ha_dispatcher_t *this, child_sa = child_sa_create(ike_sa->get_my_host(ike_sa), ike_sa->get_other_host(ike_sa), config, 0, - ike_sa->has_condition(ike_sa, COND_NAT_ANY)); + ike_sa->has_condition(ike_sa, COND_NAT_ANY), + 0, 0); child_sa->set_mode(child_sa, mode); child_sa->set_protocol(child_sa, PROTO_ESP); child_sa->set_ipcomp(child_sa, ipcomp); diff --git a/src/libcharon/sa/child_sa.c b/src/libcharon/sa/child_sa.c index 7625be1d6..fdeb605ee 100644 --- a/src/libcharon/sa/child_sa.c +++ b/src/libcharon/sa/child_sa.c @@ -695,7 +695,7 @@ METHOD(child_sa_t, install, status_t, if (!this->reqid_allocated) { status = hydra->kernel_interface->alloc_reqid(hydra->kernel_interface, - my_ts, other_ts, &this->mark_in, &this->mark_out, + my_ts, other_ts, this->mark_in, this->mark_out, &this->reqid); if (status != SUCCESS) { @@ -825,7 +825,7 @@ METHOD(child_sa_t, add_policies, status_t, /* trap policy, get or confirm reqid */ status = hydra->kernel_interface->alloc_reqid( hydra->kernel_interface, my_ts_list, other_ts_list, - &this->mark_in, &this->mark_out, &this->reqid); + this->mark_in, this->mark_out, &this->reqid); if (status != SUCCESS) { return status; @@ -1198,10 +1198,11 @@ static host_t* get_proxy_addr(child_cfg_t *config, host_t *ike, bool local) * Described in header. */ child_sa_t * child_sa_create(host_t *me, host_t* other, - child_cfg_t *config, u_int32_t rekey, bool encap) + child_cfg_t *config, u_int32_t rekey, bool encap, + u_int mark_in, u_int mark_out) { private_child_sa_t *this; - static refcount_t unique_id = 0; + static refcount_t unique_id = 0, unique_mark = 0, mark; INIT(this, .public = { @@ -1258,6 +1259,28 @@ child_sa_t * child_sa_create(host_t *me, host_t* other, this->config = config; config->get_ref(config); + if (mark_in) + { + this->mark_in.value = mark_in; + } + if (mark_out) + { + this->mark_out.value = mark_out; + } + if (this->mark_in.value == MARK_UNIQUE || + this->mark_out.value == MARK_UNIQUE) + { + mark = ref_get(&unique_mark); + if (this->mark_in.value == MARK_UNIQUE) + { + this->mark_in.value = mark; + } + if (this->mark_out.value == MARK_UNIQUE) + { + this->mark_out.value = mark; + } + } + if (!this->reqid) { /* reuse old reqid if we are rekeying an existing CHILD_SA. While the diff --git a/src/libcharon/sa/child_sa.h b/src/libcharon/sa/child_sa.h index f0ec01658..83b8c096c 100644 --- a/src/libcharon/sa/child_sa.h +++ b/src/libcharon/sa/child_sa.h @@ -394,9 +394,12 @@ struct child_sa_t { * @param config config to use for this CHILD_SA * @param reqid reqid of old CHILD_SA when rekeying, 0 otherwise * @param encap TRUE to enable UDP encapsulation (NAT traversal) + * @param mark_in explicit inbound mark value to use, 0 for config + * @param mark_out explicit outbound mark value to use, 0 for config * @return child_sa_t object */ child_sa_t * child_sa_create(host_t *me, host_t *other, child_cfg_t *config, - u_int32_t reqid, bool encap); + u_int32_t reqid, bool encap, + u_int mark_in, u_int mark_out); #endif /** CHILD_SA_H_ @}*/ diff --git a/src/libcharon/sa/ikev1/task_manager_v1.c b/src/libcharon/sa/ikev1/task_manager_v1.c index 0f8e8bc6d..3184db4ee 100644 --- a/src/libcharon/sa/ikev1/task_manager_v1.c +++ b/src/libcharon/sa/ikev1/task_manager_v1.c @@ -1647,6 +1647,8 @@ METHOD(task_manager_t, queue_child_rekey, void, task = quick_mode_create(this->ike_sa, cfg->get_ref(cfg), get_first_ts(child_sa, TRUE), get_first_ts(child_sa, FALSE)); task->use_reqid(task, child_sa->get_reqid(child_sa)); + task->use_marks(task, child_sa->get_mark(child_sa, TRUE).value, + child_sa->get_mark(child_sa, FALSE).value); task->rekey(task, child_sa->get_spi(child_sa, TRUE)); queue_task(this, &task->task); diff --git a/src/libcharon/sa/ikev1/tasks/quick_mode.c b/src/libcharon/sa/ikev1/tasks/quick_mode.c index 1133aab65..5fe04c036 100644 --- a/src/libcharon/sa/ikev1/tasks/quick_mode.c +++ b/src/libcharon/sa/ikev1/tasks/quick_mode.c @@ -155,6 +155,16 @@ struct private_quick_mode_t { */ u_int32_t reqid; + /** + * Explicit inbound mark value to use, if any + */ + u_int mark_in; + + /** + * Explicit inbound mark value to use, if any + */ + u_int mark_out; + /** * SPI of SA we rekey */ @@ -788,7 +798,8 @@ METHOD(task_t, build_i, status_t, this->child_sa = child_sa_create( this->ike_sa->get_my_host(this->ike_sa), this->ike_sa->get_other_host(this->ike_sa), - this->config, this->reqid, this->udp); + this->config, this->reqid, this->udp, + this->mark_in, this->mark_out); if (this->udp && this->mode == MODE_TRANSPORT) { @@ -972,6 +983,10 @@ static void check_for_rekeyed_child(private_quick_mode_t *this) { this->reqid = child_sa->get_reqid(child_sa); this->rekey = child_sa->get_spi(child_sa, TRUE); + this->mark_in = child_sa->get_mark(child_sa, + TRUE).value; + this->mark_out = child_sa->get_mark(child_sa, + FALSE).value; child_sa->set_state(child_sa, CHILD_REKEYING); DBG1(DBG_IKE, "detected rekeying of CHILD_SA %s{%u}", child_sa->get_name(child_sa), this->reqid); @@ -1097,7 +1112,8 @@ METHOD(task_t, process_r, status_t, this->child_sa = child_sa_create( this->ike_sa->get_my_host(this->ike_sa), this->ike_sa->get_other_host(this->ike_sa), - this->config, this->reqid, this->udp); + this->config, this->reqid, this->udp, + this->mark_in, this->mark_out); tsi = linked_list_create_with_items(this->tsi, NULL); tsr = linked_list_create_with_items(this->tsr, NULL); @@ -1307,6 +1323,13 @@ METHOD(quick_mode_t, use_reqid, void, this->reqid = reqid; } +METHOD(quick_mode_t, use_marks, void, + private_quick_mode_t *this, u_int in, u_int out) +{ + this->mark_in = in; + this->mark_out = out; +} + METHOD(quick_mode_t, rekey, void, private_quick_mode_t *this, u_int32_t spi) { @@ -1334,6 +1357,8 @@ METHOD(task_t, migrate, void, this->dh = NULL; this->spi_i = 0; this->spi_r = 0; + this->mark_in = 0; + this->mark_out = 0; if (!this->initiator) { @@ -1372,6 +1397,7 @@ quick_mode_t *quick_mode_create(ike_sa_t *ike_sa, child_cfg_t *config, .destroy = _destroy, }, .use_reqid = _use_reqid, + .use_marks = _use_marks, .rekey = _rekey, }, .ike_sa = ike_sa, diff --git a/src/libcharon/sa/ikev1/tasks/quick_mode.h b/src/libcharon/sa/ikev1/tasks/quick_mode.h index 0b80cb836..ee9b64d13 100644 --- a/src/libcharon/sa/ikev1/tasks/quick_mode.h +++ b/src/libcharon/sa/ikev1/tasks/quick_mode.h @@ -44,6 +44,14 @@ struct quick_mode_t { */ void (*use_reqid)(quick_mode_t *this, u_int32_t reqid); + /** + * Use specific mark values, overriding configuration. + * + * @param in inbound mark value + * @param out outbound mark value + */ + void (*use_marks)(quick_mode_t *this, u_int in, u_int out); + /** * Set the SPI of the old SA, if rekeying. * diff --git a/src/libcharon/sa/ikev2/tasks/child_create.c b/src/libcharon/sa/ikev2/tasks/child_create.c index e7a914875..5ec05376c 100644 --- a/src/libcharon/sa/ikev2/tasks/child_create.c +++ b/src/libcharon/sa/ikev2/tasks/child_create.c @@ -159,6 +159,16 @@ struct private_child_create_t { */ u_int32_t reqid; + /** + * Explicit inbound mark value + */ + u_int mark_in; + + /** + * Explicit outbound mark value + */ + u_int mark_out; + /** * CHILD_SA which gets established */ @@ -996,7 +1006,8 @@ METHOD(task_t, build_i, status_t, this->child_sa = child_sa_create(this->ike_sa->get_my_host(this->ike_sa), this->ike_sa->get_other_host(this->ike_sa), this->config, this->reqid, - this->ike_sa->has_condition(this->ike_sa, COND_NAT_ANY)); + this->ike_sa->has_condition(this->ike_sa, COND_NAT_ANY), + this->mark_in, this->mark_out); if (!allocate_spi(this)) { @@ -1241,7 +1252,8 @@ METHOD(task_t, build_r, status_t, this->child_sa = child_sa_create(this->ike_sa->get_my_host(this->ike_sa), this->ike_sa->get_other_host(this->ike_sa), this->config, this->reqid, - this->ike_sa->has_condition(this->ike_sa, COND_NAT_ANY)); + this->ike_sa->has_condition(this->ike_sa, COND_NAT_ANY), + this->mark_in, this->mark_out); if (this->ipcomp_received != IPCOMP_NONE) { @@ -1478,6 +1490,13 @@ METHOD(child_create_t, use_reqid, void, this->reqid = reqid; } +METHOD(child_create_t, use_marks, void, + private_child_create_t *this, u_int in, u_int out) +{ + this->mark_in = in; + this->mark_out = out; +} + METHOD(child_create_t, get_child, child_sa_t*, private_child_create_t *this) { @@ -1545,6 +1564,8 @@ METHOD(task_t, migrate, void, this->ipcomp_received = IPCOMP_NONE; this->other_cpi = 0; this->reqid = 0; + this->mark_in = 0; + this->mark_out = 0; this->established = FALSE; } @@ -1593,6 +1614,7 @@ child_create_t *child_create_create(ike_sa_t *ike_sa, .set_config = _set_config, .get_lower_nonce = _get_lower_nonce, .use_reqid = _use_reqid, + .use_marks = _use_marks, .task = { .get_type = _get_type, .migrate = _migrate, diff --git a/src/libcharon/sa/ikev2/tasks/child_create.h b/src/libcharon/sa/ikev2/tasks/child_create.h index d29ba3d98..46d9403ee 100644 --- a/src/libcharon/sa/ikev2/tasks/child_create.h +++ b/src/libcharon/sa/ikev2/tasks/child_create.h @@ -51,6 +51,14 @@ struct child_create_t { */ void (*use_reqid) (child_create_t *this, u_int32_t reqid); + /** + * Use specific mark values to override configuration. + * + * @param in inbound mark value + * @param out outbound mark value + */ + void (*use_marks)(child_create_t *this, u_int in, u_int out); + /** * Get the lower of the two nonces, used for rekey collisions. * diff --git a/src/libcharon/sa/ikev2/tasks/child_rekey.c b/src/libcharon/sa/ikev2/tasks/child_rekey.c index db872827d..213155a29 100644 --- a/src/libcharon/sa/ikev2/tasks/child_rekey.c +++ b/src/libcharon/sa/ikev2/tasks/child_rekey.c @@ -184,6 +184,9 @@ METHOD(task_t, build_i, status_t, } reqid = this->child_sa->get_reqid(this->child_sa); this->child_create->use_reqid(this->child_create, reqid); + this->child_create->use_marks(this->child_create, + this->child_sa->get_mark(this->child_sa, TRUE).value, + this->child_sa->get_mark(this->child_sa, FALSE).value); if (this->child_create->task.build(&this->child_create->task, message) != NEED_MORE) @@ -224,6 +227,9 @@ METHOD(task_t, build_r, status_t, /* let the CHILD_CREATE task build the response */ reqid = this->child_sa->get_reqid(this->child_sa); this->child_create->use_reqid(this->child_create, reqid); + this->child_create->use_marks(this->child_create, + this->child_sa->get_mark(this->child_sa, TRUE).value, + this->child_sa->get_mark(this->child_sa, FALSE).value); config = this->child_sa->get_config(this->child_sa); this->child_create->set_config(this->child_create, config->get_ref(config)); this->child_create->task.build(&this->child_create->task, message); diff --git a/src/libcharon/sa/trap_manager.c b/src/libcharon/sa/trap_manager.c index 7e55d6b0f..534d4d5ff 100644 --- a/src/libcharon/sa/trap_manager.c +++ b/src/libcharon/sa/trap_manager.c @@ -171,7 +171,7 @@ METHOD(trap_manager_t, install, u_int32_t, this->lock->unlock(this->lock); /* create and route CHILD_SA */ - child_sa = child_sa_create(me, other, child, reqid, FALSE); + child_sa = child_sa_create(me, other, child, reqid, FALSE, 0, 0); list = linked_list_create_with_items(me, NULL); my_ts = child->get_traffic_selectors(child, TRUE, NULL, list); diff --git a/src/libhydra/kernel/kernel_interface.c b/src/libhydra/kernel/kernel_interface.c index 9452b8f84..28821fc15 100644 --- a/src/libhydra/kernel/kernel_interface.c +++ b/src/libhydra/kernel/kernel_interface.c @@ -260,7 +260,9 @@ static u_int hash_ts_array(array_t *array, u_int hash) */ static u_int hash_reqid_by_ts(reqid_entry_t *entry) { - return hash_ts_array(entry->local, hash_ts_array(entry->remote, 0)); + return hash_ts_array(entry->local, hash_ts_array(entry->remote, + chunk_hash_inc(chunk_from_thing(entry->mark_in), + chunk_hash(chunk_from_thing(entry->mark_out))))); } /** @@ -289,44 +291,17 @@ static bool ts_array_equals(array_t *a, array_t *b) return equal; } -/** - * Check if mark b matches to a, optionally with reqid match - */ -static bool mark_matches(mark_t a, mark_t b, u_int32_t reqid) -{ - if (a.value == b.value) - { - return TRUE; - } - if (a.value == MARK_REQID && b.value == reqid) - { - return TRUE; - } - return FALSE; -} - /** * Hashtable equals function for reqid entries using traffic selectors as key */ static bool equals_reqid_by_ts(reqid_entry_t *a, reqid_entry_t *b) { - if (ts_array_equals(a->local, b->local) && - ts_array_equals(a->remote, b->remote) && - a->mark_in.mask == b->mark_in.mask && - a->mark_out.mask == b->mark_out.mask) - { - if (mark_matches(a->mark_in, b->mark_in, a->reqid) && - mark_matches(a->mark_out, b->mark_out, a->reqid)) - { - return TRUE; - } - if (mark_matches(b->mark_in, a->mark_in, b->reqid) && - mark_matches(b->mark_out, a->mark_out, b->reqid)) - { - return TRUE; - } - } - return FALSE; + return ts_array_equals(a->local, b->local) && + ts_array_equals(a->remote, b->remote) && + a->mark_in.value == b->mark_in.value && + a->mark_in.mask == b->mark_in.mask && + a->mark_out.value == b->mark_out.value && + a->mark_out.mask == b->mark_out.mask; } /** @@ -353,7 +328,7 @@ static array_t *array_from_ts_list(linked_list_t *list) METHOD(kernel_interface_t, alloc_reqid, status_t, private_kernel_interface_t *this, linked_list_t *local_ts, linked_list_t *remote_ts, - mark_t *mark_in, mark_t *mark_out, u_int32_t *reqid) + mark_t mark_in, mark_t mark_out, u_int32_t *reqid) { static u_int32_t counter = 0; reqid_entry_t *entry = NULL, *tmpl; @@ -362,8 +337,8 @@ METHOD(kernel_interface_t, alloc_reqid, status_t, INIT(tmpl, .local = array_from_ts_list(local_ts), .remote = array_from_ts_list(remote_ts), - .mark_in = *mark_in, - .mark_out = *mark_out, + .mark_in = mark_in, + .mark_out = mark_out, .reqid = *reqid, ); @@ -371,14 +346,6 @@ METHOD(kernel_interface_t, alloc_reqid, status_t, if (tmpl->reqid) { /* search by reqid if given */ - if (tmpl->mark_in.value == MARK_REQID) - { - tmpl->mark_in.value = tmpl->reqid; - } - if (tmpl->mark_out.value == MARK_REQID) - { - tmpl->mark_out.value = tmpl->reqid; - } entry = this->reqids->get(this->reqids, tmpl); } if (entry) @@ -390,8 +357,7 @@ METHOD(kernel_interface_t, alloc_reqid, status_t, } else { - /* search by traffic selectors. We do the search with MARK_REQID - * wildcards (if any), and update the marks if we find any match */ + /* search by traffic selectors */ entry = this->reqids_by_ts->get(this->reqids_by_ts, tmpl); if (entry) { @@ -402,21 +368,11 @@ METHOD(kernel_interface_t, alloc_reqid, status_t, /* none found, create a new entry, allocating a reqid */ entry = tmpl; entry->reqid = ++counter; - if (entry->mark_in.value == MARK_REQID) - { - entry->mark_in.value = entry->reqid; - } - if (entry->mark_out.value == MARK_REQID) - { - entry->mark_out.value = entry->reqid; - } this->reqids_by_ts->put(this->reqids_by_ts, entry, entry); this->reqids->put(this->reqids, entry, entry); } *reqid = entry->reqid; } - *mark_in = entry->mark_in; - *mark_out = entry->mark_out; entry->refs++; this->mutex->unlock(this->mutex); diff --git a/src/libhydra/kernel/kernel_interface.h b/src/libhydra/kernel/kernel_interface.h index f25c10830..9a86e78d6 100644 --- a/src/libhydra/kernel/kernel_interface.h +++ b/src/libhydra/kernel/kernel_interface.h @@ -131,9 +131,6 @@ struct kernel_interface_t { * the reqid is confirmed and registered for use. If it points to zero, * a reqid is allocated for the given selectors, and returned to reqid. * - * The passed mark values get updated to the reqid value if they are set - * to the magic value MARK_REQID. - * * @param local_ts traffic selectors of local side for SA * @param remote_ts traffic selectors of remote side for SA * @param mark_in inbound mark on SA @@ -143,7 +140,7 @@ struct kernel_interface_t { */ status_t (*alloc_reqid)(kernel_interface_t *this, linked_list_t *local_ts, linked_list_t *remote_ts, - mark_t *mark_in, mark_t *mark_out, + mark_t mark_in, mark_t mark_out, u_int32_t *reqid); /** diff --git a/src/libstrongswan/ipsec/ipsec_types.h b/src/libstrongswan/ipsec/ipsec_types.h index c1465e097..fa122af30 100644 --- a/src/libstrongswan/ipsec/ipsec_types.h +++ b/src/libstrongswan/ipsec/ipsec_types.h @@ -169,9 +169,9 @@ struct mark_t { }; /** - * Special mark value that uses the reqid of the CHILD_SA as mark + * Special mark value that uses a unique mark for each CHILD_SA */ -#define MARK_REQID (0xFFFFFFFF) +#define MARK_UNIQUE (0xFFFFFFFF) /** * Try to parse a mark_t from the given string of the form mark[/mask]. From e732fb11a915b5e760a8c1fbd1a5f12581912787 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 23 Oct 2014 15:42:21 +0200 Subject: [PATCH 15/28] child-sa-manager: Add a global manager storing CHILD_SA relations To quickly check out IKE_SAs and find associated CHILD_SAs, the child_sa_manager stores relations between CHILD_SAs and IKE_SAs. It provides CHILD_SA specific IKE_SA checkout functions wrapping the ike_sa_manager. --- src/libcharon/Android.mk | 2 +- src/libcharon/Makefile.am | 1 + src/libcharon/daemon.c | 2 + src/libcharon/daemon.h | 6 + src/libcharon/sa/child_sa_manager.c | 333 ++++++++++++++++++++++++++++ src/libcharon/sa/child_sa_manager.h | 89 ++++++++ 6 files changed, 432 insertions(+), 1 deletion(-) create mode 100644 src/libcharon/sa/child_sa_manager.c create mode 100644 src/libcharon/sa/child_sa_manager.h diff --git a/src/libcharon/Android.mk b/src/libcharon/Android.mk index 4212ee87a..ab71d4016 100644 --- a/src/libcharon/Android.mk +++ b/src/libcharon/Android.mk @@ -72,6 +72,7 @@ sa/ike_sa.c sa/ike_sa.h \ sa/ike_sa_id.c sa/ike_sa_id.h \ sa/keymat.h sa/keymat.c \ sa/ike_sa_manager.c sa/ike_sa_manager.h \ +sa/child_sa_manager.c sa/child_sa_manager.h \ sa/task_manager.h sa/task_manager.c \ sa/shunt_manager.c sa/shunt_manager.h \ sa/trap_manager.c sa/trap_manager.h \ @@ -238,4 +239,3 @@ LOCAL_PRELINK_MODULE := false LOCAL_SHARED_LIBRARIES += libstrongswan libhydra include $(BUILD_SHARED_LIBRARY) - diff --git a/src/libcharon/Makefile.am b/src/libcharon/Makefile.am index e98f5e137..e666950f1 100644 --- a/src/libcharon/Makefile.am +++ b/src/libcharon/Makefile.am @@ -70,6 +70,7 @@ sa/ike_sa.c sa/ike_sa.h \ sa/ike_sa_id.c sa/ike_sa_id.h \ sa/keymat.h sa/keymat.c \ sa/ike_sa_manager.c sa/ike_sa_manager.h \ +sa/child_sa_manager.c sa/child_sa_manager.h \ sa/task_manager.h sa/task_manager.c \ sa/shunt_manager.c sa/shunt_manager.h \ sa/trap_manager.c sa/trap_manager.h \ diff --git a/src/libcharon/daemon.c b/src/libcharon/daemon.c index 3ae7c4e6f..f3859f912 100644 --- a/src/libcharon/daemon.c +++ b/src/libcharon/daemon.c @@ -480,6 +480,7 @@ static void destroy(private_daemon_t *this) DESTROY_IF(this->kernel_handler); DESTROY_IF(this->public.traps); DESTROY_IF(this->public.shunts); + DESTROY_IF(this->public.child_sa_manager); DESTROY_IF(this->public.ike_sa_manager); DESTROY_IF(this->public.controller); DESTROY_IF(this->public.eap); @@ -606,6 +607,7 @@ METHOD(daemon_t, initialize, bool, { return FALSE; } + this->public.child_sa_manager = child_sa_manager_create(); /* Queue start_action job */ lib->processor->queue_job(lib->processor, (job_t*)start_action_job_create()); diff --git a/src/libcharon/daemon.h b/src/libcharon/daemon.h index 36242bb04..8ec1ec253 100644 --- a/src/libcharon/daemon.h +++ b/src/libcharon/daemon.h @@ -158,6 +158,7 @@ typedef struct daemon_t daemon_t; #include #include #include +#include #include #include #include @@ -214,6 +215,11 @@ struct daemon_t { */ ike_sa_manager_t *ike_sa_manager; + /** + * A child_sa_manager_t instance. + */ + child_sa_manager_t *child_sa_manager; + /** * Manager for triggering policies, called traps */ diff --git a/src/libcharon/sa/child_sa_manager.c b/src/libcharon/sa/child_sa_manager.c new file mode 100644 index 000000000..071a119da --- /dev/null +++ b/src/libcharon/sa/child_sa_manager.c @@ -0,0 +1,333 @@ +/* + * Copyright (C) 2014 Martin Willi + * Copyright (C) 2014 revosec AG + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. See . + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + */ + +#include "child_sa_manager.h" + +#include +#include +#include + +typedef struct private_child_sa_manager_t private_child_sa_manager_t; + +/** + * Private data of an child_sa_manager_t object. + */ +struct private_child_sa_manager_t { + + /** + * Public child_sa_manager_t interface. + */ + child_sa_manager_t public; + + /** + * CHILD_SAs by inbound SPI/dst, child_entry_t => child_entry_t + */ + hashtable_t *in; + + /** + * CHILD_SAs by outbound SPI/dst, child_entry_t => child_entry_t + */ + hashtable_t *out; + + /** + * CHILD_SAs by unique ID, child_entry_t => child_entry_t + */ + hashtable_t *ids; + + /** + * Mutex to access any hashtable + */ + mutex_t *mutex; +}; + +/** + * Hashtable entry for a known CHILD_SA + */ +typedef struct { + /** the associated IKE_SA */ + ike_sa_id_t *ike_id; + /** unique CHILD_SA identifier */ + u_int32_t unique_id; + /** inbound SPI */ + u_int32_t spi_in; + /** outbound SPI */ + u_int32_t spi_out; + /** inbound host address */ + host_t *host_in; + /** outbound host address and port */ + host_t *host_out; + /** IPsec protocol, AH|ESP */ + protocol_id_t proto; +} child_entry_t; + +/** + * Destroy a CHILD_SA entry + */ +static void child_entry_destroy(child_entry_t *entry) +{ + entry->ike_id->destroy(entry->ike_id); + entry->host_in->destroy(entry->host_in); + entry->host_out->destroy(entry->host_out); + free(entry); +} + +/** + * Hashtable hash function for inbound SAs + */ +static u_int hash_in(child_entry_t *entry) +{ + return chunk_hash_inc(chunk_from_thing(entry->spi_in), + chunk_hash_inc(entry->host_in->get_address(entry->host_in), + chunk_hash(chunk_from_thing(entry->proto)))); +} + +/** + * Hashtable equals function for inbound SAs + */ +static bool equals_in(child_entry_t *a, child_entry_t *b) +{ + return a->spi_in == b->spi_in && + a->proto == b->proto && + a->host_in->ip_equals(a->host_in, b->host_in); +} + +/** + * Hashtable hash function for outbound SAs + */ +static u_int hash_out(child_entry_t *entry) +{ + return chunk_hash_inc(chunk_from_thing(entry->spi_out), + chunk_hash_inc(entry->host_out->get_address(entry->host_out), + chunk_hash(chunk_from_thing(entry->proto)))); +} + +/** + * Hashtable equals function for outbound SAs + */ +static bool equals_out(child_entry_t *a, child_entry_t *b) +{ + return a->spi_out == b->spi_out && + a->proto == b->proto && + a->host_out->ip_equals(a->host_out, b->host_out); +} + +/** + * Hashtable hash function for SAs by unique ID + */ +static u_int hash_id(child_entry_t *entry) +{ + return chunk_hash(chunk_from_thing(entry->unique_id)); +} + +/** + * Hashtable equals function for SAs by unique ID + */ +static bool equals_id(child_entry_t *a, child_entry_t *b) +{ + return a->unique_id == b->unique_id; +} + +METHOD(child_sa_manager_t, add, void, + private_child_sa_manager_t *this, child_sa_t *child_sa, ike_sa_t *ike_sa) +{ + child_entry_t *entry; + host_t *in, *out; + ike_sa_id_t *id; + + id = ike_sa->get_id(ike_sa); + in = ike_sa->get_my_host(ike_sa); + out = ike_sa->get_other_host(ike_sa); + + INIT(entry, + .ike_id = id->clone(id), + .unique_id = child_sa->get_unique_id(child_sa), + .proto = child_sa->get_protocol(child_sa), + .spi_in = child_sa->get_spi(child_sa, TRUE), + .spi_out = child_sa->get_spi(child_sa, FALSE), + .host_in = in->clone(in), + .host_out = out->clone(out), + ); + + this->mutex->lock(this->mutex); + if (!this->in->get(this->in, entry) && + !this->out->get(this->out, entry)) + { + this->in->put(this->in, entry, entry); + this->out->put(this->out, entry, entry); + entry = this->ids->put(this->ids, entry, entry); + } + this->mutex->unlock(this->mutex); + + if (entry) + { + child_entry_destroy(entry); + } +} + +METHOD(child_sa_manager_t, remove_, void, + private_child_sa_manager_t *this, child_sa_t *child_sa) +{ + child_entry_t *entry, key = { + .unique_id = child_sa->get_unique_id(child_sa), + }; + + this->mutex->lock(this->mutex); + entry = this->ids->remove(this->ids, &key); + if (entry) + { + this->in->remove(this->in, entry); + this->out->remove(this->out, entry); + } + this->mutex->unlock(this->mutex); + + if (entry) + { + child_entry_destroy(entry); + } +} + +/** + * Check out an IKE_SA for a given CHILD_SA + */ +static ike_sa_t *checkout_ikesa(private_child_sa_manager_t *this, + ike_sa_id_t *id, u_int32_t unique_id, child_sa_t **child_sa) +{ + enumerator_t *enumerator; + child_sa_t *current; + ike_sa_t *ike_sa; + bool found = FALSE; + + ike_sa = charon->ike_sa_manager->checkout(charon->ike_sa_manager, id); + id->destroy(id); + if (ike_sa) + { + enumerator = ike_sa->create_child_sa_enumerator(ike_sa); + while (enumerator->enumerate(enumerator, ¤t)) + { + found = current->get_unique_id(current) == unique_id; + if (found) + { + if (child_sa) + { + *child_sa = current; + } + break; + } + } + enumerator->destroy(enumerator); + + if (found) + { + return ike_sa; + } + charon->ike_sa_manager->checkin(charon->ike_sa_manager, ike_sa); + } + return NULL; +} + +METHOD(child_sa_manager_t, checkout_by_id, ike_sa_t*, + private_child_sa_manager_t *this, u_int32_t unique_id, + child_sa_t **child_sa) +{ + ike_sa_id_t *id; + child_entry_t *entry, key = { + .unique_id = unique_id, + }; + + this->mutex->lock(this->mutex); + entry = this->ids->get(this->ids, &key); + if (entry) + { + id = entry->ike_id->clone(entry->ike_id); + } + this->mutex->unlock(this->mutex); + + if (entry) + { + return checkout_ikesa(this, id, unique_id, child_sa); + } + return NULL; +} + +METHOD(child_sa_manager_t, checkout, ike_sa_t*, + private_child_sa_manager_t *this, protocol_id_t protocol, u_int32_t spi, + host_t *dst, child_sa_t **child_sa) +{ + ike_sa_id_t *id; + u_int32_t unique_id; + child_entry_t *entry, key = { + .spi_in = spi, + .spi_out = spi, + .host_in = dst, + .host_out = dst, + .proto = protocol, + }; + + this->mutex->lock(this->mutex); + entry = this->in->get(this->in, &key); + if (!entry) + { + entry = this->out->get(this->out, &key); + } + if (entry) + { + unique_id = entry->unique_id; + id = entry->ike_id->clone(entry->ike_id); + } + this->mutex->unlock(this->mutex); + + if (entry) + { + return checkout_ikesa(this, id, unique_id, child_sa); + } + return NULL; +} + +METHOD(child_sa_manager_t, destroy, void, + private_child_sa_manager_t *this) +{ + this->in->destroy(this->in); + this->out->destroy(this->out); + this->ids->destroy(this->ids); + this->mutex->destroy(this->mutex); + free(this); +} + +/** + * See header + */ +child_sa_manager_t *child_sa_manager_create() +{ + private_child_sa_manager_t *this; + + INIT(this, + .public = { + .add = _add, + .remove = _remove_, + .checkout = _checkout, + .checkout_by_id = _checkout_by_id, + .destroy = _destroy, + }, + .in = hashtable_create((hashtable_hash_t)hash_in, + (hashtable_equals_t)equals_in, 8), + .out = hashtable_create((hashtable_hash_t)hash_out, + (hashtable_equals_t)equals_out, 8), + .ids = hashtable_create((hashtable_hash_t)hash_id, + (hashtable_equals_t)equals_id, 8), + .mutex = mutex_create(MUTEX_TYPE_DEFAULT), + ); + + return &this->public; +} diff --git a/src/libcharon/sa/child_sa_manager.h b/src/libcharon/sa/child_sa_manager.h new file mode 100644 index 000000000..4d57528e8 --- /dev/null +++ b/src/libcharon/sa/child_sa_manager.h @@ -0,0 +1,89 @@ +/* + * Copyright (C) 2014 Martin Willi + * Copyright (C) 2014 revosec AG + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. See . + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + */ + +/** + * @defgroup child_sa_manager child_sa_manager + * @{ @ingroup sa + */ + +#ifndef CHILD_SA_MANAGER_H_ +#define CHILD_SA_MANAGER_H_ + +#include +#include + +typedef struct child_sa_manager_t child_sa_manager_t; + +/** + * Handle CHILD_SA to IKE_SA relations + */ +struct child_sa_manager_t { + + /** + * Register a CHILD_SA/IKE_SA relation. + * + * @param child_sa CHILD_SA to register + * @param ike_sa IKE_SA owning the CHILD_SA + */ + void (*add)(child_sa_manager_t *this, child_sa_t *child_sa, ike_sa_t *ike_sa); + + /** + * Unregister a CHILD_SA/IKE_SA relation. + * + * @param child_sa CHILD_SA to unregister + */ + void (*remove)(child_sa_manager_t *this, child_sa_t *child_sa); + + /** + * Find a CHILD_SA and check out the associated IKE_SA by SPI. + * + * On success, the returned IKE_SA must be checked in after use to + * the IKE_SA manager. + * + * @param protocol IPsec protocol, AH|ESP + * @param spi SPI of CHILD_SA to check out + * @param dst SA destination host related to SPI + * @param child_sa returns CHILD_SA managed by IKE_SA + * @return IKE_SA, NULL if not found + */ + ike_sa_t *(*checkout)(child_sa_manager_t *this, + protocol_id_t protocol, u_int32_t spi, host_t *dst, + child_sa_t **child_sa); + + /** + * Find a CHILD_SA and check out the associated IKE_SA by unique_id. + * + * On success, the returned IKE_SA must be checked in after use to + * the IKE_SA manager. + * + * @param unique_id unique ID of CHILD_SA to check out + * @param child_sa returns CHILD_SA managed by IKE_SA + * @return IKE_SA, NULL if not found + */ + ike_sa_t *(*checkout_by_id)(child_sa_manager_t *this, u_int32_t unique_id, + child_sa_t **child_sa); + + /** + * Destroy a child_sa_manager_t. + */ + void (*destroy)(child_sa_manager_t *this); +}; + +/** + * Create a child_sa_manager instance. + */ +child_sa_manager_t *child_sa_manager_create(); + +#endif /** CHILD_SA_MANAGER_H_ @}*/ From 38227d0e081975c6bed5163af22edddb338c825f Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 23 Oct 2014 16:52:25 +0200 Subject: [PATCH 16/28] ike: Maintain per-IKE_SA CHILD_SAs in the global CHILD_SA manager --- src/libcharon/processing/jobs/migrate_job.c | 19 ++++-- src/libcharon/sa/ike_sa.c | 65 ++++++++++++++++++--- src/libcharon/sa/ikev2/tasks/ike_mobike.c | 27 ++++++--- 3 files changed, 92 insertions(+), 19 deletions(-) diff --git a/src/libcharon/processing/jobs/migrate_job.c b/src/libcharon/processing/jobs/migrate_job.c index 2ebfc6714..311c4abe9 100644 --- a/src/libcharon/processing/jobs/migrate_job.c +++ b/src/libcharon/processing/jobs/migrate_job.c @@ -82,6 +82,7 @@ METHOD(job_t, execute, job_requeue_t, enumerator_t *children, *enumerator; child_sa_t *child_sa; host_t *host; + status_t status; linked_list_t *vips; children = ike_sa->create_child_sa_enumerator(ike_sa); @@ -113,11 +114,21 @@ METHOD(job_t, execute, job_requeue_t, } enumerator->destroy(enumerator); - if (child_sa->update(child_sa, this->local, this->remote, vips, - ike_sa->has_condition(ike_sa, COND_NAT_ANY)) == NOT_SUPPORTED) + status = child_sa->update(child_sa, this->local, this->remote, vips, + ike_sa->has_condition(ike_sa, COND_NAT_ANY)); + switch (status) { - ike_sa->rekey_child_sa(ike_sa, child_sa->get_protocol(child_sa), - child_sa->get_spi(child_sa, TRUE)); + case NOT_SUPPORTED: + ike_sa->rekey_child_sa(ike_sa, child_sa->get_protocol(child_sa), + child_sa->get_spi(child_sa, TRUE)); + break; + case SUCCESS: + charon->child_sa_manager->remove(charon->child_sa_manager, + child_sa); + charon->child_sa_manager->add(charon->child_sa_manager, + child_sa, ike_sa); + default: + break; } charon->ike_sa_manager->checkin(charon->ike_sa_manager, ike_sa); vips->destroy(vips); diff --git a/src/libcharon/sa/ike_sa.c b/src/libcharon/sa/ike_sa.c index d92b9df8e..54f22e61e 100644 --- a/src/libcharon/sa/ike_sa.c +++ b/src/libcharon/sa/ike_sa.c @@ -964,6 +964,10 @@ METHOD(ike_sa_t, update_hosts, void, enumerator = array_create_enumerator(this->child_sas); while (enumerator->enumerate(enumerator, &child_sa)) { + charon->child_sa_manager->remove(charon->child_sa_manager, child_sa); + charon->child_sa_manager->add(charon->child_sa_manager, + child_sa, &this->public); + if (child_sa->update(child_sa, this->my_host, this->other_host, vips, has_condition(this, COND_NAT_ANY)) == NOT_SUPPORTED) { @@ -971,6 +975,7 @@ METHOD(ike_sa_t, update_hosts, void, child_sa->get_protocol(child_sa), child_sa->get_spi(child_sa, TRUE)); } + } enumerator->destroy(enumerator); @@ -1444,6 +1449,8 @@ METHOD(ike_sa_t, add_child_sa, void, private_ike_sa_t *this, child_sa_t *child_sa) { array_insert_create(&this->child_sas, ARRAY_TAIL, child_sa); + charon->child_sa_manager->add(charon->child_sa_manager, + child_sa, &this->public); } METHOD(ike_sa_t, get_child_sa, child_sa_t*, @@ -1471,16 +1478,58 @@ METHOD(ike_sa_t, get_child_count, int, return array_count(this->child_sas); } +/** + * Private data of a create_child_sa_enumerator() + */ +typedef struct { + /** implements enumerator */ + enumerator_t public; + /** inner array enumerator */ + enumerator_t *inner; + /** current item */ + child_sa_t *current; +} child_enumerator_t; + +METHOD(enumerator_t, child_enumerate, bool, + child_enumerator_t *this, child_sa_t **child_sa) +{ + if (this->inner->enumerate(this->inner, &this->current)) + { + *child_sa = this->current; + return TRUE; + } + return FALSE; +} + +METHOD(enumerator_t, child_enumerator_destroy, void, + child_enumerator_t *this) +{ + this->inner->destroy(this->inner); + free(this); +} + METHOD(ike_sa_t, create_child_sa_enumerator, enumerator_t*, private_ike_sa_t *this) { - return array_create_enumerator(this->child_sas); + child_enumerator_t *enumerator; + + INIT(enumerator, + .public = { + .enumerate = (void*)_child_enumerate, + .destroy = _child_enumerator_destroy, + }, + .inner = array_create_enumerator(this->child_sas), + ); + return &enumerator->public; } METHOD(ike_sa_t, remove_child_sa, void, private_ike_sa_t *this, enumerator_t *enumerator) { - array_remove_at(this->child_sas, enumerator); + child_enumerator_t *ce = (child_enumerator_t*)enumerator; + + charon->child_sa_manager->remove(charon->child_sa_manager, ce->current); + array_remove_at(this->child_sas, ce->inner); } METHOD(ike_sa_t, rekey_child_sa, status_t, @@ -1513,13 +1562,13 @@ METHOD(ike_sa_t, destroy_child_sa, status_t, child_sa_t *child_sa; status_t status = NOT_FOUND; - enumerator = array_create_enumerator(this->child_sas); + enumerator = create_child_sa_enumerator(this); while (enumerator->enumerate(enumerator, (void**)&child_sa)) { if (child_sa->get_protocol(child_sa) == protocol && child_sa->get_spi(child_sa, TRUE) == spi) { - array_remove_at(this->child_sas, enumerator); + remove_child_sa(this, enumerator); child_sa->destroy(child_sa); status = SUCCESS; break; @@ -1771,7 +1820,7 @@ METHOD(ike_sa_t, reestablish, status_t, #endif /* ME */ { /* handle existing CHILD_SAs */ - enumerator = array_create_enumerator(this->child_sas); + enumerator = create_child_sa_enumerator(this); while (enumerator->enumerate(enumerator, (void**)&child_sa)) { if (has_condition(this, COND_REAUTHENTICATING)) @@ -1780,7 +1829,7 @@ METHOD(ike_sa_t, reestablish, status_t, { case CHILD_ROUTED: { /* move routed child directly */ - array_remove_at(this->child_sas, enumerator); + remove_child_sa(this, enumerator); new->add_child_sa(new, child_sa); action = ACTION_NONE; break; @@ -2251,7 +2300,8 @@ METHOD(ike_sa_t, inherit_post, void, /* adopt all children */ while (array_remove(other->child_sas, ARRAY_HEAD, &child_sa)) { - array_insert_create(&this->child_sas, ARRAY_TAIL, child_sa); + charon->child_sa_manager->remove(charon->child_sa_manager, child_sa); + add_child_sa(this, child_sa); } /* move pending tasks to the new IKE_SA */ @@ -2305,6 +2355,7 @@ METHOD(ike_sa_t, destroy, void, * routes that the CHILD_SA tries to uninstall. */ while (array_remove(this->child_sas, ARRAY_TAIL, &child_sa)) { + charon->child_sa_manager->remove(charon->child_sa_manager, child_sa); child_sa->destroy(child_sa); } while (array_remove(this->my_vips, ARRAY_TAIL, &vip)) diff --git a/src/libcharon/sa/ikev2/tasks/ike_mobike.c b/src/libcharon/sa/ikev2/tasks/ike_mobike.c index d91fa5862..6295d7960 100644 --- a/src/libcharon/sa/ikev2/tasks/ike_mobike.c +++ b/src/libcharon/sa/ikev2/tasks/ike_mobike.c @@ -256,6 +256,7 @@ static void update_children(private_ike_mobike_t *this) enumerator_t *enumerator; child_sa_t *child_sa; linked_list_t *vips; + status_t status; host_t *host; vips = linked_list_create(); @@ -270,15 +271,25 @@ static void update_children(private_ike_mobike_t *this) enumerator = this->ike_sa->create_child_sa_enumerator(this->ike_sa); while (enumerator->enumerate(enumerator, (void**)&child_sa)) { - if (child_sa->update(child_sa, - this->ike_sa->get_my_host(this->ike_sa), - this->ike_sa->get_other_host(this->ike_sa), vips, - this->ike_sa->has_condition(this->ike_sa, - COND_NAT_ANY)) == NOT_SUPPORTED) + status = child_sa->update(child_sa, + this->ike_sa->get_my_host(this->ike_sa), + this->ike_sa->get_other_host(this->ike_sa), vips, + this->ike_sa->has_condition(this->ike_sa, COND_NAT_ANY)); + switch (status) { - this->ike_sa->rekey_child_sa(this->ike_sa, - child_sa->get_protocol(child_sa), - child_sa->get_spi(child_sa, TRUE)); + case NOT_SUPPORTED: + this->ike_sa->rekey_child_sa(this->ike_sa, + child_sa->get_protocol(child_sa), + child_sa->get_spi(child_sa, TRUE)); + break; + case SUCCESS: + charon->child_sa_manager->remove(charon->child_sa_manager, + child_sa); + charon->child_sa_manager->add(charon->child_sa_manager, + child_sa, this->ike_sa); + break; + default: + break; } } enumerator->destroy(enumerator); From adc1885bf74a74f9ae670dc9902c6c10d78da354 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Mon, 27 Oct 2014 13:51:59 +0100 Subject: [PATCH 17/28] vici: Include the CHILD_SA unique ID in list-sa event --- src/libcharon/plugins/vici/README.md | 1 + src/libcharon/plugins/vici/vici_query.c | 1 + 2 files changed, 2 insertions(+) diff --git a/src/libcharon/plugins/vici/README.md b/src/libcharon/plugins/vici/README.md index b74ef0f15..c25b99e1d 100644 --- a/src/libcharon/plugins/vici/README.md +++ b/src/libcharon/plugins/vici/README.md @@ -559,6 +559,7 @@ command. ] child-sas = { * = { + uniqueid = reqid = state = mode = diff --git a/src/libcharon/plugins/vici/vici_query.c b/src/libcharon/plugins/vici/vici_query.c index 54833abde..5a8ef25d8 100644 --- a/src/libcharon/plugins/vici/vici_query.c +++ b/src/libcharon/plugins/vici/vici_query.c @@ -63,6 +63,7 @@ static void list_child(private_vici_query_t *this, vici_builder_t *b, enumerator_t *enumerator; traffic_selector_t *ts; + b->add_kv(b, "uniqueid", "%u", child->get_unique_id(child)); b->add_kv(b, "reqid", "%u", child->get_reqid(child)); b->add_kv(b, "state", "%N", child_sa_state_names, child->get_state(child)); b->add_kv(b, "mode", "%N", ipsec_mode_names, child->get_mode(child)); From 53cf7fa60a45b8ae46440b9f03baab0e56fb7a52 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Mon, 27 Oct 2014 14:07:14 +0100 Subject: [PATCH 18/28] stroke: List CHILD_SA unique ID as the primary identifier, but print reqid, too --- src/libcharon/plugins/stroke/stroke_list.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/libcharon/plugins/stroke/stroke_list.c b/src/libcharon/plugins/stroke/stroke_list.c index 1aa49ce0d..490983100 100644 --- a/src/libcharon/plugins/stroke/stroke_list.c +++ b/src/libcharon/plugins/stroke/stroke_list.c @@ -214,11 +214,12 @@ static void log_child_sa(FILE *out, child_sa_t *child_sa, bool all) config = child_sa->get_config(child_sa); now = time_monotonic(NULL); - fprintf(out, "%12s{%d}: %N, %N%s", - child_sa->get_name(child_sa), child_sa->get_reqid(child_sa), + fprintf(out, "%12s{%d}: %N, %N%s, reqid %u", + child_sa->get_name(child_sa), child_sa->get_unique_id(child_sa), child_sa_state_names, child_sa->get_state(child_sa), ipsec_mode_names, child_sa->get_mode(child_sa), - config->use_proxy_mode(config) ? "_PROXY" : ""); + config->use_proxy_mode(config) ? "_PROXY" : "", + child_sa->get_reqid(child_sa)); if (child_sa->get_state(child_sa) == CHILD_INSTALLED) { @@ -238,7 +239,7 @@ static void log_child_sa(FILE *out, child_sa_t *child_sa, bool all) if (all) { fprintf(out, "\n%12s{%d}: ", child_sa->get_name(child_sa), - child_sa->get_reqid(child_sa)); + child_sa->get_unique_id(child_sa)); proposal = child_sa->get_proposal(child_sa); if (proposal) @@ -333,7 +334,7 @@ static void log_child_sa(FILE *out, child_sa_t *child_sa, bool all) other_ts = linked_list_create_from_enumerator( child_sa->create_ts_enumerator(child_sa, FALSE)); fprintf(out, "\n%12s{%d}: %#R=== %#R\n", - child_sa->get_name(child_sa), child_sa->get_reqid(child_sa), + child_sa->get_name(child_sa), child_sa->get_unique_id(child_sa), my_ts, other_ts); my_ts->destroy(my_ts); other_ts->destroy(other_ts); From e4a131b1cee111622c95e856809982c66cf8806c Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Mon, 27 Oct 2014 13:54:51 +0100 Subject: [PATCH 19/28] swanctl: List CHILD_SA unique ID as the primary identifier, but print reqid, too --- src/swanctl/commands/list_sas.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/swanctl/commands/list_sas.c b/src/swanctl/commands/list_sas.c index 35e7469a9..55297cff3 100644 --- a/src/swanctl/commands/list_sas.c +++ b/src/swanctl/commands/list_sas.c @@ -86,8 +86,8 @@ CALLBACK(child_sas, int, ret = vici_parse_cb(res, NULL, sa_values, sa_list, child); if (ret == 0) { - printf(" %s: #%s, %s, %s%s, %s:", - name, child->get(child, "reqid"), + printf(" %s: #%s, reqid %s, %s, %s%s, %s:", + name, child->get(child, "uniqueid"), child->get(child, "reqid"), child->get(child, "state"), child->get(child, "mode"), child->get(child, "encap") ? "-in-UDP" : "", child->get(child, "protocol")); From 971a91685da640363651e7a9a9edb9dfd94b7fe5 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Mon, 27 Oct 2014 14:03:32 +0100 Subject: [PATCH 20/28] controller: Use the CHILD_SA unique_id to terminate CHILD_SAs --- src/conftest/actions.c | 2 +- src/libcharon/control/controller.c | 30 +++++-------------- src/libcharon/control/controller.h | 4 +-- src/libcharon/plugins/stroke/stroke_control.c | 2 +- src/libcharon/plugins/vici/vici_config.c | 20 ++++++------- src/libcharon/plugins/vici/vici_control.c | 4 +-- 6 files changed, 24 insertions(+), 38 deletions(-) diff --git a/src/conftest/actions.c b/src/conftest/actions.c index 7532e95cf..3f937b118 100644 --- a/src/conftest/actions.c +++ b/src/conftest/actions.c @@ -236,7 +236,7 @@ static job_requeue_t close_child(char *config) { if (streq(config, child_sa->get_name(child_sa))) { - id = child_sa->get_reqid(child_sa); + id = child_sa->get_unique_id(child_sa); break; } } diff --git a/src/libcharon/control/controller.c b/src/libcharon/control/controller.c index 60d186e27..418b24763 100644 --- a/src/libcharon/control/controller.c +++ b/src/libcharon/control/controller.c @@ -534,17 +534,15 @@ METHOD(job_t, terminate_child_execute, job_requeue_t, interface_job_t *job) { interface_listener_t *listener = &job->listener; - u_int32_t reqid = listener->id; - enumerator_t *enumerator; + u_int32_t id = listener->id; child_sa_t *child_sa; ike_sa_t *ike_sa; - ike_sa = charon->ike_sa_manager->checkout_by_id(charon->ike_sa_manager, - reqid, TRUE); + ike_sa = charon->child_sa_manager->checkout_by_id(charon->child_sa_manager, + id, &child_sa); if (!ike_sa) { - DBG1(DBG_IKE, "unable to terminate, CHILD_SA with ID %d not found", - reqid); + DBG1(DBG_IKE, "unable to terminate, CHILD_SA with ID %d not found", id); listener->status = NOT_FOUND; /* release listener */ listener_done(listener); @@ -554,22 +552,10 @@ METHOD(job_t, terminate_child_execute, job_requeue_t, listener->ike_sa = ike_sa; listener->lock->unlock(listener->lock); - enumerator = ike_sa->create_child_sa_enumerator(ike_sa); - while (enumerator->enumerate(enumerator, (void**)&child_sa)) - { - if (child_sa->get_state(child_sa) != CHILD_ROUTED && - child_sa->get_reqid(child_sa) == reqid) - { - break; - } - child_sa = NULL; - } - enumerator->destroy(enumerator); - - if (!child_sa) + if (child_sa->get_state(child_sa) == CHILD_ROUTED) { DBG1(DBG_IKE, "unable to terminate, established " - "CHILD_SA with ID %d not found", reqid); + "CHILD_SA with ID %d not found", id); charon->ike_sa_manager->checkin(charon->ike_sa_manager, ike_sa); listener->status = NOT_FOUND; /* release listener */ @@ -596,7 +582,7 @@ METHOD(job_t, terminate_child_execute, job_requeue_t, } METHOD(controller_t, terminate_child, status_t, - controller_t *this, u_int32_t reqid, + controller_t *this, u_int32_t unique_id, controller_cb_t callback, void *param, u_int timeout) { interface_job_t *job; @@ -617,7 +603,7 @@ METHOD(controller_t, terminate_child, status_t, .param = param, }, .status = FAILED, - .id = reqid, + .id = unique_id, .lock = spinlock_create(), }, .public = { diff --git a/src/libcharon/control/controller.h b/src/libcharon/control/controller.h index 222285cde..02f4ebb2b 100644 --- a/src/libcharon/control/controller.h +++ b/src/libcharon/control/controller.h @@ -118,7 +118,7 @@ struct controller_t { * If a callback is provided the function is synchronous and thus blocks * until the CHILD_SA is properly deleted, or the call timed out. * - * @param reqid reqid of the CHILD_SA to terminate + * @param unique_id CHILD_SA unique ID to terminate * @param cb logging callback * @param param parameter to include in each call of cb * @param timeout timeout in ms to wait for callbacks, 0 to disable @@ -128,7 +128,7 @@ struct controller_t { * - NEED_MORE, if callback returned FALSE * - OUT_OF_RES if timed out */ - status_t (*terminate_child)(controller_t *this, u_int32_t reqid, + status_t (*terminate_child)(controller_t *this, u_int32_t unique_id, controller_cb_t callback, void *param, u_int timeout); diff --git a/src/libcharon/plugins/stroke/stroke_control.c b/src/libcharon/plugins/stroke/stroke_control.c index f770d7c9e..99d07f593 100644 --- a/src/libcharon/plugins/stroke/stroke_control.c +++ b/src/libcharon/plugins/stroke/stroke_control.c @@ -352,7 +352,7 @@ METHOD(stroke_control_t, terminate, void, if (streq(name, child_sa->get_name(child_sa))) { child_list->insert_last(child_list, - (void*)(uintptr_t)child_sa->get_reqid(child_sa)); + (void*)(uintptr_t)child_sa->get_unique_id(child_sa)); if (!all) { break; diff --git a/src/libcharon/plugins/vici/vici_config.c b/src/libcharon/plugins/vici/vici_config.c index 113d48084..43b3f0712 100644 --- a/src/libcharon/plugins/vici/vici_config.c +++ b/src/libcharon/plugins/vici/vici_config.c @@ -1551,8 +1551,8 @@ static void clear_start_action(private_vici_config_t *this, enumerator_t *enumerator, *children; child_sa_t *child_sa; ike_sa_t *ike_sa; - u_int32_t reqid = 0, *del; - array_t *reqids = NULL; + u_int32_t id = 0, *del; + array_t *ids = NULL; char *name; name = child_cfg->get_name(child_cfg); @@ -1568,23 +1568,23 @@ static void clear_start_action(private_vici_config_t *this, { if (streq(name, child_sa->get_name(child_sa))) { - reqid = child_sa->get_reqid(child_sa); - array_insert_create(&reqids, ARRAY_TAIL, &reqid); + id = child_sa->get_unique_id(child_sa); + array_insert_create(&ids, ARRAY_TAIL, &id); } } children->destroy(children); } enumerator->destroy(enumerator); - if (array_count(reqids)) + if (array_count(ids)) { - while (array_remove(reqids, ARRAY_HEAD, &del)) + while (array_remove(ids, ARRAY_HEAD, &del)) { DBG1(DBG_CFG, "closing '%s' #%u", name, *del); charon->controller->terminate_child(charon->controller, *del, NULL, NULL, 0); } - array_destroy(reqids); + array_destroy(ids); } break; case ACTION_ROUTE: @@ -1601,14 +1601,14 @@ static void clear_start_action(private_vici_config_t *this, { if (streq(name, child_sa->get_name(child_sa))) { - reqid = child_sa->get_reqid(child_sa); + id = child_sa->get_reqid(child_sa); break; } } enumerator->destroy(enumerator); - if (reqid) + if (id) { - charon->traps->uninstall(charon->traps, reqid); + charon->traps->uninstall(charon->traps, id); } break; } diff --git a/src/libcharon/plugins/vici/vici_control.c b/src/libcharon/plugins/vici/vici_control.c index 292a40032..01d503644 100644 --- a/src/libcharon/plugins/vici/vici_control.c +++ b/src/libcharon/plugins/vici/vici_control.c @@ -264,11 +264,11 @@ CALLBACK(terminate, vici_message_t*, { continue; } - if (child_id && child_sa->get_reqid(child_sa) != child_id) + if (child_id && child_sa->get_unique_id(child_sa) != child_id) { continue; } - current = child_sa->get_reqid(child_sa); + current = child_sa->get_unique_id(child_sa); array_insert(ids, ARRAY_TAIL, ¤t); } csas->destroy(csas); From f81a9497483a7282c11adf5705d9ea3e83f6fffd Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Mon, 27 Oct 2014 15:07:05 +0100 Subject: [PATCH 21/28] kernel-interface: Raise expires with a proto/SPI/dst tuple instead of reqid --- src/conftest/actions.c | 14 ++++---- .../libandroidbridge/kernel/android_ipsec.c | 6 ++-- src/libcharon/kernel/kernel_handler.c | 12 +++---- .../kernel_libipsec/kernel_libipsec_ipsec.c | 6 ++-- .../plugins/kernel_wfp/kernel_wfp_ipsec.c | 11 +++--- src/libcharon/plugins/stroke/stroke_control.c | 6 ++-- .../processing/jobs/delete_child_sa_job.c | 26 +++++++------- .../processing/jobs/delete_child_sa_job.h | 9 ++--- .../processing/jobs/rekey_child_sa_job.c | 27 ++++++++------- .../processing/jobs/rekey_child_sa_job.h | 10 ++---- src/libcharon/sa/ikev2/tasks/child_rekey.c | 4 +-- src/libhydra/kernel/kernel_interface.c | 7 ++-- src/libhydra/kernel/kernel_interface.h | 6 ++-- src/libhydra/kernel/kernel_listener.h | 6 ++-- .../kernel_netlink/kernel_netlink_ipsec.c | 19 ++++++----- .../plugins/kernel_pfkey/kernel_pfkey_ipsec.c | 19 ++++++----- src/libipsec/ipsec_event_listener.h | 6 ++-- src/libipsec/ipsec_event_relay.c | 34 +++++++++++++------ src/libipsec/ipsec_event_relay.h | 6 ++-- src/libipsec/ipsec_sa.c | 8 ++--- 20 files changed, 124 insertions(+), 118 deletions(-) diff --git a/src/conftest/actions.c b/src/conftest/actions.c index 3f937b118..474672ca1 100644 --- a/src/conftest/actions.c +++ b/src/conftest/actions.c @@ -117,19 +117,20 @@ static job_requeue_t rekey_child(char *config) enumerator_t *enumerator, *children; ike_sa_t *ike_sa; child_sa_t *child_sa; - u_int32_t reqid = 0, spi = 0; - protocol_id_t proto = PROTO_ESP; + u_int32_t spi, proto; + host_t *dst = NULL; enumerator = charon->controller->create_ike_sa_enumerator( charon->controller, TRUE); while (enumerator->enumerate(enumerator, &ike_sa)) { children = ike_sa->create_child_sa_enumerator(ike_sa); - while (children->enumerate(children, (void**)&child_sa)) + while (children->enumerate(children, &child_sa)) { if (streq(config, child_sa->get_name(child_sa))) { - reqid = child_sa->get_reqid(child_sa); + dst = ike_sa->get_my_host(ike_sa); + dst = dst->clone(dst); proto = child_sa->get_protocol(child_sa); spi = child_sa->get_spi(child_sa, TRUE); break; @@ -138,11 +139,12 @@ static job_requeue_t rekey_child(char *config) children->destroy(children); } enumerator->destroy(enumerator); - if (reqid) + if (dst) { DBG1(DBG_CFG, "starting rekey of CHILD_SA '%s'", config); lib->processor->queue_job(lib->processor, - (job_t*)rekey_child_sa_job_create(reqid, proto, spi)); + (job_t*)rekey_child_sa_job_create(proto, spi, dst)); + dst->destroy(dst); } else { diff --git a/src/frontends/android/jni/libandroidbridge/kernel/android_ipsec.c b/src/frontends/android/jni/libandroidbridge/kernel/android_ipsec.c index 65166077e..a0aefaa4e 100644 --- a/src/frontends/android/jni/libandroidbridge/kernel/android_ipsec.c +++ b/src/frontends/android/jni/libandroidbridge/kernel/android_ipsec.c @@ -40,10 +40,10 @@ struct private_kernel_android_ipsec_t { /** * Callback registrered with libipsec. */ -void expire(u_int32_t reqid, u_int8_t protocol, u_int32_t spi, bool hard) +static void expire(u_int8_t protocol, u_int32_t spi, host_t *dst, bool hard) { - hydra->kernel_interface->expire(hydra->kernel_interface, reqid, protocol, - spi, hard); + hydra->kernel_interface->expire(hydra->kernel_interface, protocol, + spi, dst, hard); } METHOD(kernel_ipsec_t, get_spi, status_t, diff --git a/src/libcharon/kernel/kernel_handler.c b/src/libcharon/kernel/kernel_handler.c index 059124e35..a6656e7d5 100644 --- a/src/libcharon/kernel/kernel_handler.c +++ b/src/libcharon/kernel/kernel_handler.c @@ -72,23 +72,23 @@ METHOD(kernel_listener_t, acquire, bool, } METHOD(kernel_listener_t, expire, bool, - private_kernel_handler_t *this, u_int32_t reqid, u_int8_t protocol, - u_int32_t spi, bool hard) + private_kernel_handler_t *this, u_int8_t protocol, u_int32_t spi, + host_t *dst, bool hard) { protocol_id_t proto = proto_ip2ike(protocol); - DBG1(DBG_KNL, "creating %s job for %N CHILD_SA with SPI %.8x and reqid {%u}", - hard ? "delete" : "rekey", protocol_id_names, proto, ntohl(spi), reqid); + DBG1(DBG_KNL, "creating %s job for CHILD_SA %N/0x%08x/%H", + hard ? "delete" : "rekey", protocol_id_names, proto, ntohl(spi), dst); if (hard) { lib->processor->queue_job(lib->processor, - (job_t*)delete_child_sa_job_create(reqid, proto, spi, hard)); + (job_t*)delete_child_sa_job_create(proto, spi, dst, hard)); } else { lib->processor->queue_job(lib->processor, - (job_t*)rekey_child_sa_job_create(reqid, proto, spi)); + (job_t*)rekey_child_sa_job_create(proto, spi, dst)); } return TRUE; } diff --git a/src/libcharon/plugins/kernel_libipsec/kernel_libipsec_ipsec.c b/src/libcharon/plugins/kernel_libipsec/kernel_libipsec_ipsec.c index 362b32746..e6c5d6a1d 100644 --- a/src/libcharon/plugins/kernel_libipsec/kernel_libipsec_ipsec.c +++ b/src/libcharon/plugins/kernel_libipsec/kernel_libipsec_ipsec.c @@ -222,10 +222,10 @@ static inline bool policy_entry_equals(policy_entry_t *a, /** * Expiration callback */ -static void expire(u_int32_t reqid, u_int8_t protocol, u_int32_t spi, bool hard) +static void expire(u_int8_t protocol, u_int32_t spi, host_t *dst, bool hard) { - hydra->kernel_interface->expire(hydra->kernel_interface, reqid, protocol, - spi, hard); + hydra->kernel_interface->expire(hydra->kernel_interface, protocol, + spi, dst, hard); } METHOD(kernel_ipsec_t, get_features, kernel_feature_t, diff --git a/src/libcharon/plugins/kernel_wfp/kernel_wfp_ipsec.c b/src/libcharon/plugins/kernel_wfp/kernel_wfp_ipsec.c index 9fd6541a5..86db9e643 100644 --- a/src/libcharon/plugins/kernel_wfp/kernel_wfp_ipsec.c +++ b/src/libcharon/plugins/kernel_wfp/kernel_wfp_ipsec.c @@ -2032,9 +2032,8 @@ static void expire_data_destroy(expire_data_t *data) static job_requeue_t expire_job(expire_data_t *data) { private_kernel_wfp_ipsec_t *this = data->this; - u_int32_t reqid = 0; u_int8_t protocol; - entry_t *entry; + entry_t *entry = NULL; sa_entry_t key = { .spi = data->spi, .dst = data->dst, @@ -2048,7 +2047,6 @@ static job_requeue_t expire_job(expire_data_t *data) if (entry) { protocol = entry->isa.protocol; - reqid = entry->reqid; if (entry->osa.dst) { key.dst = entry->osa.dst; @@ -2065,15 +2063,14 @@ static job_requeue_t expire_job(expire_data_t *data) if (entry) { protocol = entry->isa.protocol; - reqid = entry->reqid; } this->mutex->unlock(this->mutex); } - if (reqid) + if (entry) { - hydra->kernel_interface->expire(hydra->kernel_interface, - reqid, protocol, data->spi, data->hard); + hydra->kernel_interface->expire(hydra->kernel_interface, protocol, + data->spi, data->dst, data->hard); } return JOB_REQUEUE_NONE; diff --git a/src/libcharon/plugins/stroke/stroke_control.c b/src/libcharon/plugins/stroke/stroke_control.c index 99d07f593..0084fbf93 100644 --- a/src/libcharon/plugins/stroke/stroke_control.c +++ b/src/libcharon/plugins/stroke/stroke_control.c @@ -432,13 +432,13 @@ METHOD(stroke_control_t, rekey, void, while (children->enumerate(children, (void**)&child_sa)) { if ((name && streq(name, child_sa->get_name(child_sa))) || - (id && id == child_sa->get_reqid(child_sa))) + (id && id == child_sa->get_unique_id(child_sa))) { lib->processor->queue_job(lib->processor, (job_t*)rekey_child_sa_job_create( - child_sa->get_reqid(child_sa), child_sa->get_protocol(child_sa), - child_sa->get_spi(child_sa, TRUE))); + child_sa->get_spi(child_sa, TRUE), + ike_sa->get_my_host(ike_sa))); if (!all) { finished = TRUE; diff --git a/src/libcharon/processing/jobs/delete_child_sa_job.c b/src/libcharon/processing/jobs/delete_child_sa_job.c index 9afbac02b..0d85883be 100644 --- a/src/libcharon/processing/jobs/delete_child_sa_job.c +++ b/src/libcharon/processing/jobs/delete_child_sa_job.c @@ -30,11 +30,6 @@ struct private_delete_child_sa_job_t { */ delete_child_sa_job_t public; - /** - * reqid of the CHILD_SA - */ - u_int32_t reqid; - /** * protocol of the CHILD_SA (ESP/AH) */ @@ -45,6 +40,11 @@ struct private_delete_child_sa_job_t { */ u_int32_t spi; + /** + * SA destination address + */ + host_t *dst; + /** * Delete for an expired CHILD_SA */ @@ -54,6 +54,7 @@ struct private_delete_child_sa_job_t { METHOD(job_t, destroy, void, private_delete_child_sa_job_t *this) { + this->dst->destroy(this->dst); free(this); } @@ -62,12 +63,12 @@ METHOD(job_t, execute, job_requeue_t, { ike_sa_t *ike_sa; - ike_sa = charon->ike_sa_manager->checkout_by_id(charon->ike_sa_manager, - this->reqid, TRUE); + ike_sa = charon->child_sa_manager->checkout(charon->child_sa_manager, + this->protocol, this->spi, this->dst, NULL); if (ike_sa == NULL) { - DBG1(DBG_JOB, "CHILD_SA with reqid %d not found for delete", - this->reqid); + DBG1(DBG_JOB, "CHILD_SA %N/0x%08x/%H not found for delete", + protocol_id_names, this->protocol, htonl(this->spi), this->dst); } else { @@ -87,8 +88,8 @@ METHOD(job_t, get_priority, job_priority_t, /* * Described in header */ -delete_child_sa_job_t *delete_child_sa_job_create(u_int32_t reqid, - protocol_id_t protocol, u_int32_t spi, bool expired) +delete_child_sa_job_t *delete_child_sa_job_create(protocol_id_t protocol, + u_int32_t spi, host_t *dst, bool expired) { private_delete_child_sa_job_t *this; @@ -100,12 +101,11 @@ delete_child_sa_job_t *delete_child_sa_job_create(u_int32_t reqid, .destroy = _destroy, }, }, - .reqid = reqid, .protocol = protocol, .spi = spi, + .dst = dst->clone(dst), .expired = expired, ); return &this->public; } - diff --git a/src/libcharon/processing/jobs/delete_child_sa_job.h b/src/libcharon/processing/jobs/delete_child_sa_job.h index be6d578bc..6fa53644c 100644 --- a/src/libcharon/processing/jobs/delete_child_sa_job.h +++ b/src/libcharon/processing/jobs/delete_child_sa_job.h @@ -44,16 +44,13 @@ struct delete_child_sa_job_t { /** * Creates a job of type DELETE_CHILD_SA. * - * The CHILD_SA is identified by its reqid, protocol (AH/ESP) and its - * inbound SPI. - * - * @param reqid reqid of the CHILD_SA, as used in kernel * @param protocol protocol of the CHILD_SA * @param spi security parameter index of the CHILD_SA + * @param dst SA destination address * @param expired TRUE if CHILD_SA already expired * @return delete_child_sa_job_t object */ -delete_child_sa_job_t *delete_child_sa_job_create(u_int32_t reqid, - protocol_id_t protocol, u_int32_t spi, bool expired); +delete_child_sa_job_t *delete_child_sa_job_create(protocol_id_t protocol, + u_int32_t spi, host_t *dst, bool expired); #endif /** DELETE_CHILD_SA_JOB_H_ @}*/ diff --git a/src/libcharon/processing/jobs/rekey_child_sa_job.c b/src/libcharon/processing/jobs/rekey_child_sa_job.c index 1bf8dc0cb..8f17d39ab 100644 --- a/src/libcharon/processing/jobs/rekey_child_sa_job.c +++ b/src/libcharon/processing/jobs/rekey_child_sa_job.c @@ -24,16 +24,12 @@ typedef struct private_rekey_child_sa_job_t private_rekey_child_sa_job_t; * Private data of an rekey_child_sa_job_t object. */ struct private_rekey_child_sa_job_t { + /** * Public rekey_child_sa_job_t interface. */ rekey_child_sa_job_t public; - /** - * reqid of the child to rekey - */ - u_int32_t reqid; - /** * protocol of the CHILD_SA (ESP/AH) */ @@ -43,11 +39,17 @@ struct private_rekey_child_sa_job_t { * inbound SPI of the CHILD_SA */ u_int32_t spi; + + /** + * SA destination address + */ + host_t *dst; }; METHOD(job_t, destroy, void, private_rekey_child_sa_job_t *this) { + this->dst->destroy(this->dst); free(this); } @@ -56,12 +58,12 @@ METHOD(job_t, execute, job_requeue_t, { ike_sa_t *ike_sa; - ike_sa = charon->ike_sa_manager->checkout_by_id(charon->ike_sa_manager, - this->reqid, TRUE); + ike_sa = charon->child_sa_manager->checkout(charon->child_sa_manager, + this->protocol, this->spi, this->dst, NULL); if (ike_sa == NULL) { - DBG2(DBG_JOB, "CHILD_SA with reqid %d not found for rekeying", - this->reqid); + DBG1(DBG_JOB, "CHILD_SA %N/0x%08x/%H not found for rekey", + protocol_id_names, this->protocol, htonl(this->spi), this->dst); } else { @@ -80,9 +82,8 @@ METHOD(job_t, get_priority, job_priority_t, /* * Described in header */ -rekey_child_sa_job_t *rekey_child_sa_job_create(u_int32_t reqid, - protocol_id_t protocol, - u_int32_t spi) +rekey_child_sa_job_t *rekey_child_sa_job_create(protocol_id_t protocol, + u_int32_t spi, host_t *dst) { private_rekey_child_sa_job_t *this; @@ -94,9 +95,9 @@ rekey_child_sa_job_t *rekey_child_sa_job_create(u_int32_t reqid, .destroy = _destroy, }, }, - .reqid = reqid, .protocol = protocol, .spi = spi, + .dst = dst->clone(dst), ); return &this->public; diff --git a/src/libcharon/processing/jobs/rekey_child_sa_job.h b/src/libcharon/processing/jobs/rekey_child_sa_job.h index fcbe65a06..364bb5ae7 100644 --- a/src/libcharon/processing/jobs/rekey_child_sa_job.h +++ b/src/libcharon/processing/jobs/rekey_child_sa_job.h @@ -43,15 +43,11 @@ struct rekey_child_sa_job_t { /** * Creates a job of type REKEY_CHILD_SA. * - * The CHILD_SA is identified by its protocol (AH/ESP) and its - * inbound SPI. - * - * @param reqid reqid of the CHILD_SA to rekey * @param protocol protocol of the CHILD_SA * @param spi security parameter index of the CHILD_SA + * @param dst SA destination address * @return rekey_child_sa_job_t object */ -rekey_child_sa_job_t *rekey_child_sa_job_create(u_int32_t reqid, - protocol_id_t protocol, - u_int32_t spi); +rekey_child_sa_job_t *rekey_child_sa_job_create(protocol_id_t protocol, + u_int32_t spi, host_t *dst); #endif /** REKEY_CHILD_SA_JOB_H_ @}*/ diff --git a/src/libcharon/sa/ikev2/tasks/child_rekey.c b/src/libcharon/sa/ikev2/tasks/child_rekey.c index 213155a29..c806e19ca 100644 --- a/src/libcharon/sa/ikev2/tasks/child_rekey.c +++ b/src/libcharon/sa/ikev2/tasks/child_rekey.c @@ -96,9 +96,9 @@ static void schedule_delayed_rekey(private_child_rekey_t *this) retry = RETRY_INTERVAL - (random() % RETRY_JITTER); job = (job_t*)rekey_child_sa_job_create( - this->child_sa->get_reqid(this->child_sa), this->child_sa->get_protocol(this->child_sa), - this->child_sa->get_spi(this->child_sa, TRUE)); + this->child_sa->get_spi(this->child_sa, TRUE), + this->ike_sa->get_my_host(this->ike_sa)); DBG1(DBG_IKE, "CHILD_SA rekeying failed, trying again in %d seconds", retry); this->child_sa->set_state(this->child_sa, CHILD_INSTALLED); lib->scheduler->schedule_job(lib->scheduler, job, retry); diff --git a/src/libhydra/kernel/kernel_interface.c b/src/libhydra/kernel/kernel_interface.c index 28821fc15..b5ade37d1 100644 --- a/src/libhydra/kernel/kernel_interface.c +++ b/src/libhydra/kernel/kernel_interface.c @@ -815,17 +815,18 @@ METHOD(kernel_interface_t, acquire, void, } METHOD(kernel_interface_t, expire, void, - private_kernel_interface_t *this, u_int32_t reqid, u_int8_t protocol, - u_int32_t spi, bool hard) + private_kernel_interface_t *this, u_int8_t protocol, u_int32_t spi, + host_t *dst, bool hard) { kernel_listener_t *listener; enumerator_t *enumerator; + this->mutex->lock(this->mutex); enumerator = this->listeners->create_enumerator(this->listeners); while (enumerator->enumerate(enumerator, &listener)) { if (listener->expire && - !listener->expire(listener, reqid, protocol, spi, hard)) + !listener->expire(listener, protocol, spi, dst, hard)) { this->listeners->remove_at(this->listeners, enumerator); } diff --git a/src/libhydra/kernel/kernel_interface.h b/src/libhydra/kernel/kernel_interface.h index 9a86e78d6..2db53f504 100644 --- a/src/libhydra/kernel/kernel_interface.h +++ b/src/libhydra/kernel/kernel_interface.h @@ -559,13 +559,13 @@ struct kernel_interface_t { /** * Raise an expire event. * - * @param reqid reqid of the expired SA * @param protocol protocol of the expired SA * @param spi spi of the expired SA + * @param dst destination address of expired SA * @param hard TRUE if it is a hard expire, FALSE otherwise */ - void (*expire)(kernel_interface_t *this, u_int32_t reqid, - u_int8_t protocol, u_int32_t spi, bool hard); + void (*expire)(kernel_interface_t *this, u_int8_t protocol, u_int32_t spi, + host_t *dst, bool hard); /** * Raise a mapping event. diff --git a/src/libhydra/kernel/kernel_listener.h b/src/libhydra/kernel/kernel_listener.h index 4382a43fd..122453f72 100644 --- a/src/libhydra/kernel/kernel_listener.h +++ b/src/libhydra/kernel/kernel_listener.h @@ -49,14 +49,14 @@ struct kernel_listener_t { /** * Hook called if an exire event for an IPsec SA is received. * - * @param reqid reqid of the expired SA * @param protocol protocol of the expired SA * @param spi spi of the expired SA + * @param dst destination address of expired SA * @param hard TRUE if it is a hard expire, FALSE otherwise * @return TRUE to remain registered, FALSE to unregister */ - bool (*expire)(kernel_listener_t *this, u_int32_t reqid, - u_int8_t protocol, u_int32_t spi, bool hard); + bool (*expire)(kernel_listener_t *this, u_int8_t protocol, u_int32_t spi, + host_t *dst, bool hard); /** * Hook called if the NAT mappings of an IPsec SA changed. diff --git a/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c b/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c index 31bb4f656..f8077d836 100644 --- a/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c +++ b/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c @@ -870,25 +870,26 @@ static void process_expire(private_kernel_netlink_ipsec_t *this, struct nlmsghdr *hdr) { struct xfrm_user_expire *expire; - u_int32_t spi, reqid; + u_int32_t spi; u_int8_t protocol; + host_t *dst; expire = NLMSG_DATA(hdr); protocol = expire->state.id.proto; spi = expire->state.id.spi; - reqid = expire->state.reqid; DBG2(DBG_KNL, "received a XFRM_MSG_EXPIRE"); - if (protocol != IPPROTO_ESP && protocol != IPPROTO_AH) + if (protocol == IPPROTO_ESP || protocol == IPPROTO_AH) { - DBG2(DBG_KNL, "ignoring XFRM_MSG_EXPIRE for SA with SPI %.8x and " - "reqid {%u} which is not a CHILD_SA", ntohl(spi), reqid); - return; + dst = xfrm2host(expire->state.family, &expire->state.id.daddr, 0); + if (dst) + { + hydra->kernel_interface->expire(hydra->kernel_interface, protocol, + spi, dst, expire->hard != 0); + dst->destroy(dst); + } } - - hydra->kernel_interface->expire(hydra->kernel_interface, reqid, protocol, - spi, expire->hard != 0); } /** diff --git a/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c b/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c index 348549bfa..9b846864e 100644 --- a/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c +++ b/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c @@ -1296,7 +1296,8 @@ static void process_expire(private_kernel_pfkey_ipsec_t *this, { pfkey_msg_t response; u_int8_t protocol; - u_int32_t spi, reqid; + u_int32_t spi; + host_t *dst; bool hard; DBG2(DBG_KNL, "received an SADB_EXPIRE"); @@ -1309,18 +1310,18 @@ static void process_expire(private_kernel_pfkey_ipsec_t *this, protocol = satype2proto(msg->sadb_msg_satype); spi = response.sa->sadb_sa_spi; - reqid = response.x_sa2->sadb_x_sa2_reqid; hard = response.lft_hard != NULL; - if (protocol != IPPROTO_ESP && protocol != IPPROTO_AH) + if (protocol == IPPROTO_ESP || protocol == IPPROTO_AH) { - DBG2(DBG_KNL, "ignoring SADB_EXPIRE for SA with SPI %.8x and " - "reqid {%u} which is not a CHILD_SA", ntohl(spi), reqid); - return; + dst = host_create_from_sockaddr((sockaddr_t*)(response.dst + 1)); + if (dst) + { + hydra->kernel_interface->expire(hydra->kernel_interface, protocol, + spi, dst, hard); + dst->destroy(dst); + } } - - hydra->kernel_interface->expire(hydra->kernel_interface, reqid, protocol, - spi, hard); } #ifdef SADB_X_MIGRATE diff --git a/src/libipsec/ipsec_event_listener.h b/src/libipsec/ipsec_event_listener.h index c5c39b0f1..f15f6fe52 100644 --- a/src/libipsec/ipsec_event_listener.h +++ b/src/libipsec/ipsec_event_listener.h @@ -35,14 +35,12 @@ struct ipsec_event_listener_t { /** * Called when the lifetime of an IPsec SA expired * - * @param reqid reqid of the expired SA * @param protocol protocol of the expired SA * @param spi spi of the expired SA + * @param dst destination address of expired SA * @param hard TRUE if this is a hard expire, FALSE otherwise */ - void (*expire)(u_int32_t reqid, u_int8_t protocol, u_int32_t spi, - bool hard); - + void (*expire)(u_int8_t protocol, u_int32_t spi, host_t *dst, bool hard); }; #endif /** IPSEC_EVENT_LISTENER_H_ @}*/ diff --git a/src/libipsec/ipsec_event_relay.c b/src/libipsec/ipsec_event_relay.c index c6b2a550d..048063053 100644 --- a/src/libipsec/ipsec_event_relay.c +++ b/src/libipsec/ipsec_event_relay.c @@ -65,23 +65,26 @@ typedef struct { } type; /** - * Reqid of the SA, if any + * Protocol of the SA */ - u_int32_t reqid; + u_int8_t protocol; /** * SPI of the SA, if any */ u_int32_t spi; + /** + * SA destination address + */ + host_t *dst; + /** * Additional data for specific event types */ union { struct { - /** Protocol of the SA */ - u_int8_t protocol; /** TRUE in case of a hard expire */ bool hard; } expire; @@ -90,6 +93,15 @@ typedef struct { } ipsec_event_t; +/** + * Destroy IPsec event data + */ +static void ipsec_event_destroy(ipsec_event_t *event) +{ + event->dst->destroy(event->dst); + free(event); +} + /** * Dequeue events and relay them to listeners */ @@ -110,31 +122,31 @@ static job_requeue_t handle_events(private_ipsec_event_relay_t *this) case IPSEC_EVENT_EXPIRE: if (current->expire) { - current->expire(event->reqid, event->data.expire.protocol, - event->spi, event->data.expire.hard); + current->expire(event->protocol, event->spi, event->dst, + event->data.expire.hard); } break; } } enumerator->destroy(enumerator); this->lock->unlock(this->lock); - free(event); + ipsec_event_destroy(event); return JOB_REQUEUE_DIRECT; } METHOD(ipsec_event_relay_t, expire, void, - private_ipsec_event_relay_t *this, u_int32_t reqid, u_int8_t protocol, - u_int32_t spi, bool hard) + private_ipsec_event_relay_t *this, u_int8_t protocol, u_int32_t spi, + host_t *dst, bool hard) { ipsec_event_t *event; INIT(event, .type = IPSEC_EVENT_EXPIRE, - .reqid = reqid, + .protocol = protocol, .spi = spi, + .dst = dst->clone(dst), .data = { .expire = { - .protocol = protocol, .hard = hard, }, }, diff --git a/src/libipsec/ipsec_event_relay.h b/src/libipsec/ipsec_event_relay.h index c6935d546..1dddf121b 100644 --- a/src/libipsec/ipsec_event_relay.h +++ b/src/libipsec/ipsec_event_relay.h @@ -38,13 +38,13 @@ struct ipsec_event_relay_t { /** * Raise an expire event. * - * @param reqid reqid of the expired IPsec SA * @param protocol protocol (e.g ESP) of the expired SA * @param spi SPI of the expired SA + * @param dst destination address of expired SA * @param hard TRUE for a hard expire, FALSE otherwise */ - void (*expire)(ipsec_event_relay_t *this, u_int32_t reqid, - u_int8_t protocol, u_int32_t spi, bool hard); + void (*expire)(ipsec_event_relay_t *this, u_int8_t protocol, u_int32_t spi, + host_t *dst, bool hard); /** * Register a listener to events raised by this manager diff --git a/src/libipsec/ipsec_sa.c b/src/libipsec/ipsec_sa.c index 3d0bbe169..ccbbb1b3c 100644 --- a/src/libipsec/ipsec_sa.c +++ b/src/libipsec/ipsec_sa.c @@ -194,8 +194,8 @@ METHOD(ipsec_sa_t, expire, void, if (!this->hard_expired) { this->hard_expired = TRUE; - ipsec->events->expire(ipsec->events, this->reqid, this->protocol, - this->spi, TRUE); + ipsec->events->expire(ipsec->events, this->protocol, this->spi, + this->dst, TRUE); } } else @@ -203,8 +203,8 @@ METHOD(ipsec_sa_t, expire, void, if (!this->hard_expired && !this->soft_expired) { this->soft_expired = TRUE; - ipsec->events->expire(ipsec->events, this->reqid, this->protocol, - this->spi, FALSE); + ipsec->events->expire(ipsec->events, this->protocol, this->spi, + this->dst, FALSE); } } } From 84e391e08e1bc8dcb0060d87f6e99caa91987fd2 Mon Sep 17 00:00:00 2001 From: Reto Buerki Date: Mon, 17 Nov 2014 15:15:19 +0100 Subject: [PATCH 22/28] charon-tkm: Fix compilation of ees_callback.c Update the call to hydra->kernel_interface->expire to make ees_callback.c compile again. The required destination host argument is set to NULL for now. --- src/charon-tkm/src/ees/ees_callbacks.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/charon-tkm/src/ees/ees_callbacks.c b/src/charon-tkm/src/ees/ees_callbacks.c index 2d9653837..8650947f8 100644 --- a/src/charon-tkm/src/ees/ees_callbacks.c +++ b/src/charon-tkm/src/ees/ees_callbacks.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2012 Reto Buerki + * Copyright (C) 2012-2014 Reto Buerki * Copyright (C) 2012 Adrian-Ken Rueegsegger * Hochschule fuer Technik Rapperswil * @@ -33,8 +33,10 @@ void charon_esa_expire(result_type *res, const sp_id_type sp_id, const esp_spi_type spi_rem, const protocol_type protocol, const expiry_flag_type hard) { + host_t *dst = NULL; + DBG1(DBG_KNL, "ees: expire received for reqid {%d}", sp_id); - hydra->kernel_interface->expire(hydra->kernel_interface, sp_id, protocol, - spi_rem, hard != 0); + hydra->kernel_interface->expire(hydra->kernel_interface, protocol, + spi_rem, dst, hard != 0); *res = TKM_OK; } From 85ace154d85c69f708d494c810e4d77af4587a8c Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Mon, 27 Oct 2014 15:19:09 +0100 Subject: [PATCH 23/28] inactivity-job: Schedule job by CHILD_SA unique ID instead of reqid --- src/libcharon/processing/jobs/inactivity_job.c | 16 ++++++++-------- src/libcharon/processing/jobs/inactivity_job.h | 4 ++-- src/libcharon/sa/ikev1/tasks/quick_mode.c | 9 +++------ src/libcharon/sa/ikev2/tasks/child_create.c | 11 ++++------- 4 files changed, 17 insertions(+), 23 deletions(-) diff --git a/src/libcharon/processing/jobs/inactivity_job.c b/src/libcharon/processing/jobs/inactivity_job.c index 197733979..f0f90eedf 100644 --- a/src/libcharon/processing/jobs/inactivity_job.c +++ b/src/libcharon/processing/jobs/inactivity_job.c @@ -30,9 +30,9 @@ struct private_inactivity_job_t { inactivity_job_t public; /** - * Reqid of CHILD_SA to check + * Unique CHILD_SA identifier to check */ - u_int32_t reqid; + u_int32_t id; /** * Inactivity timeout @@ -57,8 +57,8 @@ METHOD(job_t, execute, job_requeue_t, ike_sa_t *ike_sa; u_int32_t reschedule = 0; - ike_sa = charon->ike_sa_manager->checkout_by_id(charon->ike_sa_manager, - this->reqid, TRUE); + ike_sa = charon->child_sa_manager->checkout_by_id(charon->child_sa_manager, + this->id, NULL); if (ike_sa) { enumerator_t *enumerator; @@ -69,9 +69,9 @@ METHOD(job_t, execute, job_requeue_t, status_t status = SUCCESS; enumerator = ike_sa->create_child_sa_enumerator(ike_sa); - while (enumerator->enumerate(enumerator, (void**)&child_sa)) + while (enumerator->enumerate(enumerator, &child_sa)) { - if (child_sa->get_reqid(child_sa) == this->reqid) + if (child_sa->get_unique_id(child_sa) == this->id) { time_t in, out, install, diff; @@ -136,7 +136,7 @@ METHOD(job_t, get_priority, job_priority_t, /** * See header */ -inactivity_job_t *inactivity_job_create(u_int32_t reqid, u_int32_t timeout, +inactivity_job_t *inactivity_job_create(u_int32_t unique_id, u_int32_t timeout, bool close_ike) { private_inactivity_job_t *this; @@ -149,7 +149,7 @@ inactivity_job_t *inactivity_job_create(u_int32_t reqid, u_int32_t timeout, .destroy = _destroy, }, }, - .reqid = reqid, + .id = unique_id, .timeout = timeout, .close_ike = close_ike, ); diff --git a/src/libcharon/processing/jobs/inactivity_job.h b/src/libcharon/processing/jobs/inactivity_job.h index 890f7704b..ff19fe560 100644 --- a/src/libcharon/processing/jobs/inactivity_job.h +++ b/src/libcharon/processing/jobs/inactivity_job.h @@ -42,12 +42,12 @@ struct inactivity_job_t { /** * Create a inactivity_job instance. * - * @param reqid reqid of CHILD_SA to check for inactivity + * @param unique_id unique CHILD_SA identifier to check for inactivity * @param timeout inactivity timeout in s * @param close_ike close IKE_SA if the last remaining CHILD_SA is inactive? * @return inactivity checking job */ -inactivity_job_t *inactivity_job_create(u_int32_t reqid, u_int32_t timeout, +inactivity_job_t *inactivity_job_create(u_int32_t unique_id, u_int32_t timeout, bool close_ike); #endif /** INACTIVITY_JOB_H_ @}*/ diff --git a/src/libcharon/sa/ikev1/tasks/quick_mode.c b/src/libcharon/sa/ikev1/tasks/quick_mode.c index 5fe04c036..8ca3ac72e 100644 --- a/src/libcharon/sa/ikev1/tasks/quick_mode.c +++ b/src/libcharon/sa/ikev1/tasks/quick_mode.c @@ -206,8 +206,8 @@ static void schedule_inactivity_timeout(private_quick_mode_t *this) close_ike = lib->settings->get_bool(lib->settings, "%s.inactivity_close_ike", FALSE, lib->ns); lib->scheduler->schedule_job(lib->scheduler, (job_t*) - inactivity_job_create(this->child_sa->get_reqid(this->child_sa), - timeout, close_ike), timeout); + inactivity_job_create(this->child_sa->get_unique_id(this->child_sa), + timeout, close_ike), timeout); } } @@ -406,10 +406,7 @@ static bool install(private_quick_mode_t *this) { charon->bus->child_updown(charon->bus, this->child_sa, TRUE); } - if (!this->rekey) - { - schedule_inactivity_timeout(this); - } + schedule_inactivity_timeout(this); this->child_sa = NULL; return TRUE; } diff --git a/src/libcharon/sa/ikev2/tasks/child_create.c b/src/libcharon/sa/ikev2/tasks/child_create.c index 5ec05376c..abee0da3c 100644 --- a/src/libcharon/sa/ikev2/tasks/child_create.c +++ b/src/libcharon/sa/ikev2/tasks/child_create.c @@ -296,7 +296,7 @@ static bool allocate_spi(private_child_create_t *this) */ static void schedule_inactivity_timeout(private_child_create_t *this) { - u_int32_t timeout; + u_int32_t timeout, id; bool close_ike; timeout = this->config->get_inactivity(this->config); @@ -304,9 +304,9 @@ static void schedule_inactivity_timeout(private_child_create_t *this) { close_ike = lib->settings->get_bool(lib->settings, "%s.inactivity_close_ike", FALSE, lib->ns); + id = this->child_sa->get_unique_id(this->child_sa); lib->scheduler->schedule_job(lib->scheduler, (job_t*) - inactivity_job_create(this->child_sa->get_reqid(this->child_sa), - timeout, close_ike), timeout); + inactivity_job_create(id, timeout, close_ike), timeout); } } @@ -693,10 +693,7 @@ static status_t select_and_install(private_child_create_t *this, this->ike_sa->add_child_sa(this->ike_sa, this->child_sa); this->established = TRUE; - if (!this->rekey) - { /* a rekeyed SA uses the same reqid, no need for a new job */ - schedule_inactivity_timeout(this); - } + schedule_inactivity_timeout(this); my_ts = linked_list_create_from_enumerator( this->child_sa->create_ts_enumerator(this->child_sa, TRUE)); From b125839a1a174936624ad99765ea451c55834a70 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Mon, 27 Oct 2014 15:38:47 +0100 Subject: [PATCH 24/28] kernel-interface: Raise mapping event with a proto/SPI/dst tuple --- src/libcharon/kernel/kernel_handler.c | 12 ++++--- src/libcharon/processing/jobs/update_sa_job.c | 33 ++++++++++++++----- src/libcharon/processing/jobs/update_sa_job.h | 8 +++-- src/libhydra/kernel/kernel_interface.c | 7 ++-- src/libhydra/kernel/kernel_interface.h | 7 ++-- src/libhydra/kernel/kernel_listener.h | 7 ++-- .../kernel_netlink/kernel_netlink_ipsec.c | 22 ++++++++----- .../plugins/kernel_pfkey/kernel_pfkey_ipsec.c | 20 ++++++----- 8 files changed, 75 insertions(+), 41 deletions(-) diff --git a/src/libcharon/kernel/kernel_handler.c b/src/libcharon/kernel/kernel_handler.c index a6656e7d5..96a51fabd 100644 --- a/src/libcharon/kernel/kernel_handler.c +++ b/src/libcharon/kernel/kernel_handler.c @@ -94,14 +94,16 @@ METHOD(kernel_listener_t, expire, bool, } METHOD(kernel_listener_t, mapping, bool, - private_kernel_handler_t *this, u_int32_t reqid, u_int32_t spi, - host_t *remote) + private_kernel_handler_t *this, u_int8_t protocol, u_int32_t spi, + host_t *dst, host_t *remote) { - DBG1(DBG_KNL, "NAT mappings of ESP CHILD_SA with SPI %.8x and reqid {%u} " - "changed, queuing update job", ntohl(spi), reqid); + protocol_id_t proto = proto_ip2ike(protocol); + + DBG1(DBG_KNL, "NAT mappings of CHILD_SA %N/0x%08x/%H changed, " + "queuing update job", protocol_id_names, proto, ntohl(spi), dst); lib->processor->queue_job(lib->processor, - (job_t*)update_sa_job_create(reqid, remote)); + (job_t*)update_sa_job_create(proto, spi, dst, remote)); return TRUE; } diff --git a/src/libcharon/processing/jobs/update_sa_job.c b/src/libcharon/processing/jobs/update_sa_job.c index e6d7da2c6..862506d90 100644 --- a/src/libcharon/processing/jobs/update_sa_job.c +++ b/src/libcharon/processing/jobs/update_sa_job.c @@ -27,15 +27,26 @@ typedef struct private_update_sa_job_t private_update_sa_job_t; * Private data of an update_sa_job_t Object */ struct private_update_sa_job_t { + /** * public update_sa_job_t interface */ update_sa_job_t public; /** - * reqid of the CHILD_SA + * protocol of the CHILD_SA (ESP/AH) */ - u_int32_t reqid; + protocol_id_t protocol; + + /** + * SPI of the CHILD_SA + */ + u_int32_t spi; + + /** + * Old SA destination address + */ + host_t *dst; /** * New SA address and port @@ -46,6 +57,7 @@ struct private_update_sa_job_t { METHOD(job_t, destroy, void, private_update_sa_job_t *this) { + this->dst->destroy(this->dst); this->new->destroy(this->new); free(this); } @@ -55,11 +67,12 @@ METHOD(job_t, execute, job_requeue_t, { ike_sa_t *ike_sa; - ike_sa = charon->ike_sa_manager->checkout_by_id(charon->ike_sa_manager, - this->reqid, TRUE); + ike_sa = charon->child_sa_manager->checkout(charon->child_sa_manager, + this->protocol, this->spi, this->dst, NULL); if (ike_sa == NULL) { - DBG1(DBG_JOB, "CHILD_SA with reqid %d not found for update", this->reqid); + DBG1(DBG_JOB, "CHILD_SA %N/0x%08x/%H not found for update", + protocol_id_names, this->protocol, htonl(this->spi), this->dst); } else { @@ -78,7 +91,8 @@ METHOD(job_t, get_priority, job_priority_t, /* * Described in header */ -update_sa_job_t *update_sa_job_create(u_int32_t reqid, host_t *new) +update_sa_job_t *update_sa_job_create(protocol_id_t protocol, + u_int32_t spi, host_t *dst, host_t *new) { private_update_sa_job_t *this; @@ -90,10 +104,11 @@ update_sa_job_t *update_sa_job_create(u_int32_t reqid, host_t *new) .destroy = _destroy, }, }, - .reqid = reqid, - .new = new, + .protocol = protocol, + .spi = spi, + .dst = dst->clone(dst), + .new = new->clone(new), ); return &this->public; } - diff --git a/src/libcharon/processing/jobs/update_sa_job.h b/src/libcharon/processing/jobs/update_sa_job.h index 55a3df83e..9c19f5b6e 100644 --- a/src/libcharon/processing/jobs/update_sa_job.h +++ b/src/libcharon/processing/jobs/update_sa_job.h @@ -26,6 +26,7 @@ typedef struct update_sa_job_t update_sa_job_t; #include #include #include +#include /** * Update the addresses of an IKE and its CHILD_SAs. @@ -41,10 +42,13 @@ struct update_sa_job_t { /** * Creates a job to update IKE and CHILD_SA addresses. * - * @param reqid reqid of the CHILD_SA + * @param protocol IPsec protocol of SA to update + * @param spi SPI of SA to update + * @param dst old destination host of SA to update * @param new new address and port * @return update_sa_job_t object */ -update_sa_job_t *update_sa_job_create(u_int32_t reqid, host_t *new); +update_sa_job_t *update_sa_job_create(protocol_id_t protocol, + u_int32_t spi, host_t *dst, host_t *new); #endif /** UPDATE_SA_JOB_H_ @}*/ diff --git a/src/libhydra/kernel/kernel_interface.c b/src/libhydra/kernel/kernel_interface.c index b5ade37d1..943b513b2 100644 --- a/src/libhydra/kernel/kernel_interface.c +++ b/src/libhydra/kernel/kernel_interface.c @@ -836,17 +836,18 @@ METHOD(kernel_interface_t, expire, void, } METHOD(kernel_interface_t, mapping, void, - private_kernel_interface_t *this, u_int32_t reqid, u_int32_t spi, - host_t *remote) + private_kernel_interface_t *this, u_int8_t protocol, u_int32_t spi, + host_t *dst, host_t *remote) { kernel_listener_t *listener; enumerator_t *enumerator; + this->mutex->lock(this->mutex); enumerator = this->listeners->create_enumerator(this->listeners); while (enumerator->enumerate(enumerator, &listener)) { if (listener->mapping && - !listener->mapping(listener, reqid, spi, remote)) + !listener->mapping(listener, protocol, spi, dst, remote)) { this->listeners->remove_at(this->listeners, enumerator); } diff --git a/src/libhydra/kernel/kernel_interface.h b/src/libhydra/kernel/kernel_interface.h index 2db53f504..2d484251f 100644 --- a/src/libhydra/kernel/kernel_interface.h +++ b/src/libhydra/kernel/kernel_interface.h @@ -570,12 +570,13 @@ struct kernel_interface_t { /** * Raise a mapping event. * - * @param reqid reqid of the SA + * @param protocol protocol of affected SA * @param spi spi of the SA + * @param dst original destination address of SA * @param remote new remote host */ - void (*mapping)(kernel_interface_t *this, u_int32_t reqid, u_int32_t spi, - host_t *remote); + void (*mapping)(kernel_interface_t *this, u_int8_t protocol, u_int32_t spi, + host_t *dst, host_t *remote); /** * Raise a migrate event. diff --git a/src/libhydra/kernel/kernel_listener.h b/src/libhydra/kernel/kernel_listener.h index 122453f72..8074356a4 100644 --- a/src/libhydra/kernel/kernel_listener.h +++ b/src/libhydra/kernel/kernel_listener.h @@ -61,13 +61,14 @@ struct kernel_listener_t { /** * Hook called if the NAT mappings of an IPsec SA changed. * - * @param reqid reqid of the SA + * @param protocol IPsec protocol of affected SA * @param spi spi of the SA + * @param dst old destinatino address of SA * @param remote new remote host * @return TRUE to remain registered, FALSE to unregister */ - bool (*mapping)(kernel_listener_t *this, u_int32_t reqid, u_int32_t spi, - host_t *remote); + bool (*mapping)(kernel_listener_t *this, u_int8_t protocol, u_int32_t spi, + host_t *dst, host_t *remote); /** * Hook called if a migrate event for a policy is received. diff --git a/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c b/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c index f8077d836..b4875ba58 100644 --- a/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c +++ b/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c @@ -973,23 +973,29 @@ static void process_mapping(private_kernel_netlink_ipsec_t *this, struct nlmsghdr *hdr) { struct xfrm_user_mapping *mapping; - u_int32_t spi, reqid; + u_int32_t spi; mapping = NLMSG_DATA(hdr); spi = mapping->id.spi; - reqid = mapping->reqid; DBG2(DBG_KNL, "received a XFRM_MSG_MAPPING"); if (mapping->id.proto == IPPROTO_ESP) { - host_t *host; - host = xfrm2host(mapping->id.family, &mapping->new_saddr, - mapping->new_sport); - if (host) + host_t *dst, *new; + + dst = xfrm2host(mapping->id.family, &mapping->id.daddr, 0); + if (dst) { - hydra->kernel_interface->mapping(hydra->kernel_interface, reqid, - spi, host); + new = xfrm2host(mapping->id.family, &mapping->new_saddr, + mapping->new_sport); + if (new) + { + hydra->kernel_interface->mapping(hydra->kernel_interface, + IPPROTO_ESP, spi, dst, new); + new->destroy(new); + } + dst->destroy(dst); } } } diff --git a/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c b/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c index 9b846864e..8b893f485 100644 --- a/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c +++ b/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c @@ -1388,9 +1388,9 @@ static void process_mapping(private_kernel_pfkey_ipsec_t *this, struct sadb_msg* msg) { pfkey_msg_t response; - u_int32_t spi, reqid; + u_int32_t spi; sockaddr_t *sa; - host_t *host; + host_t *dst, *new; DBG2(DBG_KNL, "received an SADB_X_NAT_T_NEW_MAPPING"); @@ -1408,7 +1408,6 @@ static void process_mapping(private_kernel_pfkey_ipsec_t *this, } spi = response.sa->sadb_sa_spi; - reqid = response.x_sa2->sadb_x_sa2_reqid; if (satype2proto(msg->sadb_msg_satype) != IPPROTO_ESP) { @@ -1416,6 +1415,7 @@ static void process_mapping(private_kernel_pfkey_ipsec_t *this, } sa = (sockaddr_t*)(response.dst + 1); + dst = host_create_from_sockaddr(sa); switch (sa->sa_family) { case AF_INET: @@ -1433,12 +1433,16 @@ static void process_mapping(private_kernel_pfkey_ipsec_t *this, default: break; } - - host = host_create_from_sockaddr(sa); - if (host) + if (dst) { - hydra->kernel_interface->mapping(hydra->kernel_interface, reqid, - spi, host); + new = host_create_from_sockaddr(sa); + if (new) + { + hydra->kernel_interface->mapping(hydra->kernel_interface, + IPPROTO_ESP, spi, dst, new); + new->destroy(new); + } + dst->destroy(dst); } } #endif /*SADB_X_NAT_T_NEW_MAPPING*/ From 246dcf8691f233acc57edaa019291cbed810ab4c Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Mon, 27 Oct 2014 16:03:05 +0100 Subject: [PATCH 25/28] migrate-job: Do CHILD_SA reqid lookup locally --- src/libcharon/processing/jobs/migrate_job.c | 45 +++++++++------------ src/libcharon/processing/jobs/migrate_job.h | 2 +- 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/src/libcharon/processing/jobs/migrate_job.c b/src/libcharon/processing/jobs/migrate_job.c index 311c4abe9..097dbdffd 100644 --- a/src/libcharon/processing/jobs/migrate_job.c +++ b/src/libcharon/processing/jobs/migrate_job.c @@ -70,30 +70,34 @@ METHOD(job_t, destroy, void, METHOD(job_t, execute, job_requeue_t, private_migrate_job_t *this) { - ike_sa_t *ike_sa = NULL; + enumerator_t *ike_sas, *children; + ike_sa_t *ike_sa; - if (this->reqid) + ike_sas = charon->ike_sa_manager->create_enumerator(charon->ike_sa_manager, + TRUE); + while (ike_sas->enumerate(ike_sas, &ike_sa)) { - ike_sa = charon->ike_sa_manager->checkout_by_id(charon->ike_sa_manager, - this->reqid, TRUE); - } - if (ike_sa) - { - enumerator_t *children, *enumerator; - child_sa_t *child_sa; - host_t *host; - status_t status; + child_sa_t *current, *child_sa = NULL; linked_list_t *vips; + status_t status; + host_t *host; children = ike_sa->create_child_sa_enumerator(ike_sa); - while (children->enumerate(children, (void**)&child_sa)) + while (children->enumerate(children, ¤t)) { - if (child_sa->get_reqid(child_sa) == this->reqid) + if (current->get_reqid(current) == this->reqid) { + child_sa = current; break; } } children->destroy(children); + + if (!child_sa) + { + continue; + } + DBG2(DBG_JOB, "found CHILD_SA with reqid {%d}", this->reqid); ike_sa->set_kmaddress(ike_sa, this->local, this->remote); @@ -106,13 +110,8 @@ METHOD(job_t, execute, job_requeue_t, host->set_port(host, IKEV2_UDP_PORT); ike_sa->set_other_host(ike_sa, host); - vips = linked_list_create(); - enumerator = ike_sa->create_virtual_ip_enumerator(ike_sa, TRUE); - while (enumerator->enumerate(enumerator, &host)) - { - vips->insert_last(vips, host); - } - enumerator->destroy(enumerator); + vips = linked_list_create_from_enumerator( + ike_sa->create_virtual_ip_enumerator(ike_sa, TRUE)); status = child_sa->update(child_sa, this->local, this->remote, vips, ike_sa->has_condition(ike_sa, COND_NAT_ANY)); @@ -130,13 +129,9 @@ METHOD(job_t, execute, job_requeue_t, default: break; } - charon->ike_sa_manager->checkin(charon->ike_sa_manager, ike_sa); vips->destroy(vips); } - else - { - DBG1(DBG_JOB, "no CHILD_SA found with reqid {%d}", this->reqid); - } + ike_sas->destroy(ike_sas); return JOB_REQUEUE_NONE; } diff --git a/src/libcharon/processing/jobs/migrate_job.h b/src/libcharon/processing/jobs/migrate_job.h index 30c0ad0ac..0f2b9aaad 100644 --- a/src/libcharon/processing/jobs/migrate_job.h +++ b/src/libcharon/processing/jobs/migrate_job.h @@ -46,7 +46,7 @@ struct migrate_job_t { * * We use the reqid or the traffic selectors to find a matching CHILD_SA. * - * @param reqid reqid of the CHILD_SA to acquire + * @param reqid reqid of the CHILD_SA to migrate * @param src_ts source traffic selector to be used in the policy * @param dst_ts destination traffic selector to be used in the policy * @param dir direction of the policy (in|out) From db80d0d2dac7da6fc7d81201728de5b870f09d06 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Mon, 27 Oct 2014 16:08:59 +0100 Subject: [PATCH 26/28] ike-sa-manager: Remove IKE_SA checkout by CHILD_SA reqid --- .../backend/android_service.c | 3 +- src/libcharon/control/controller.c | 2 +- src/libcharon/plugins/unity/unity_handler.c | 2 +- src/libcharon/sa/ike_sa_manager.c | 39 +++++-------------- src/libcharon/sa/ike_sa_manager.h | 10 ++--- 5 files changed, 16 insertions(+), 40 deletions(-) diff --git a/src/frontends/android/jni/libandroidbridge/backend/android_service.c b/src/frontends/android/jni/libandroidbridge/backend/android_service.c index fad070d95..64a79077e 100644 --- a/src/frontends/android/jni/libandroidbridge/backend/android_service.c +++ b/src/frontends/android/jni/libandroidbridge/backend/android_service.c @@ -426,8 +426,7 @@ CALLBACK(reestablish, job_requeue_t, { ike_sa_t *ike_sa; - ike_sa = charon->ike_sa_manager->checkout_by_id(charon->ike_sa_manager, - *id, FALSE); + ike_sa = charon->ike_sa_manager->checkout_by_id(charon->ike_sa_manager, *id); if (ike_sa) { if (ike_sa->reauth(ike_sa) == DESTROY_ME) diff --git a/src/libcharon/control/controller.c b/src/libcharon/control/controller.c index 418b24763..fd8349e2f 100644 --- a/src/libcharon/control/controller.c +++ b/src/libcharon/control/controller.c @@ -449,7 +449,7 @@ METHOD(job_t, terminate_ike_execute, job_requeue_t, ike_sa_t *ike_sa; ike_sa = charon->ike_sa_manager->checkout_by_id(charon->ike_sa_manager, - unique_id, FALSE); + unique_id); if (!ike_sa) { DBG1(DBG_IKE, "unable to terminate IKE_SA: ID %d not found", unique_id); diff --git a/src/libcharon/plugins/unity/unity_handler.c b/src/libcharon/plugins/unity/unity_handler.c index bcef0dc25..ff386c301 100644 --- a/src/libcharon/plugins/unity/unity_handler.c +++ b/src/libcharon/plugins/unity/unity_handler.c @@ -210,7 +210,7 @@ static job_requeue_t add_exclude_async(entry_t *entry) host_t *host; ike_sa = charon->ike_sa_manager->checkout_by_id(charon->ike_sa_manager, - entry->sa, FALSE); + entry->sa); if (ike_sa) { create_shunt_name(ike_sa, entry->ts, name, sizeof(name)); diff --git a/src/libcharon/sa/ike_sa_manager.c b/src/libcharon/sa/ike_sa_manager.c index 144cd7d3f..7ca72f3c1 100644 --- a/src/libcharon/sa/ike_sa_manager.c +++ b/src/libcharon/sa/ike_sa_manager.c @@ -1383,54 +1383,35 @@ METHOD(ike_sa_manager_t, checkout_by_config, ike_sa_t*, } METHOD(ike_sa_manager_t, checkout_by_id, ike_sa_t*, - private_ike_sa_manager_t *this, u_int32_t id, bool child) + private_ike_sa_manager_t *this, u_int32_t id) { - enumerator_t *enumerator, *children; + enumerator_t *enumerator; entry_t *entry; ike_sa_t *ike_sa = NULL; - child_sa_t *child_sa; u_int segment; - DBG2(DBG_MGR, "checkout IKE_SA by ID"); + DBG2(DBG_MGR, "checkout IKE_SA by ID %u", id); enumerator = create_table_enumerator(this); while (enumerator->enumerate(enumerator, &entry, &segment)) { if (wait_for_entry(this, entry, segment)) { - /* look for a child with such a reqid ... */ - if (child) - { - children = entry->ike_sa->create_child_sa_enumerator(entry->ike_sa); - while (children->enumerate(children, (void**)&child_sa)) - { - if (child_sa->get_reqid(child_sa) == id) - { - ike_sa = entry->ike_sa; - break; - } - } - children->destroy(children); - } - else /* ... or for a IKE_SA with such a unique id */ - { - if (entry->ike_sa->get_unique_id(entry->ike_sa) == id) - { - ike_sa = entry->ike_sa; - } - } - /* got one, return */ - if (ike_sa) + if (entry->ike_sa->get_unique_id(entry->ike_sa) == id) { + ike_sa = entry->ike_sa; entry->checked_out = TRUE; - DBG2(DBG_MGR, "IKE_SA %s[%u] successfully checked out", - ike_sa->get_name(ike_sa), ike_sa->get_unique_id(ike_sa)); break; } } } enumerator->destroy(enumerator); + if (ike_sa) + { + DBG2(DBG_MGR, "IKE_SA %s[%u] successfully checked out", + ike_sa->get_name(ike_sa), ike_sa->get_unique_id(ike_sa)); + } charon->bus->set_sa(charon->bus, ike_sa); return ike_sa; } diff --git a/src/libcharon/sa/ike_sa_manager.h b/src/libcharon/sa/ike_sa_manager.h index a68ae7763..f259d8e56 100644 --- a/src/libcharon/sa/ike_sa_manager.h +++ b/src/libcharon/sa/ike_sa_manager.h @@ -129,19 +129,15 @@ struct ike_sa_manager_t { /** * Check out an IKE_SA a unique ID. * - * Every IKE_SA and every CHILD_SA is uniquely identified by an ID. - * These checkout function uses, depending - * on the child parameter, the unique ID of the IKE_SA or the reqid - * of one of a IKE_SAs CHILD_SA. + * Every IKE_SA is uniquely identified by a numerical ID. This checkout + * function uses the unique ID of the IKE_SA to check it out. * * @param id unique ID of the object - * @param child TRUE to use CHILD, FALSE to use IKE_SA * @return * - checked out IKE_SA, if found * - NULL, if not found */ - ike_sa_t* (*checkout_by_id) (ike_sa_manager_t* this, u_int32_t id, - bool child); + ike_sa_t* (*checkout_by_id) (ike_sa_manager_t* this, u_int32_t id); /** * Check out an IKE_SA by the policy/connection name. From fa42367ca1af350ce11e12daf1d2ed1174d7cfbe Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Mon, 27 Oct 2014 16:16:13 +0100 Subject: [PATCH 27/28] unity: Reference IKE_SAs by the IKEv1 COOKIEs, improving lookup performance When handling thousands of IKE_SAs, the unique ID based lookup is rather slow, as we have no indexing. --- src/libcharon/plugins/unity/unity_handler.c | 22 +++++++++++---------- src/libcharon/plugins/unity/unity_handler.h | 5 +++-- src/libcharon/plugins/unity/unity_narrow.c | 4 ++-- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/libcharon/plugins/unity/unity_handler.c b/src/libcharon/plugins/unity/unity_handler.c index ff386c301..eb0ddba5d 100644 --- a/src/libcharon/plugins/unity/unity_handler.c +++ b/src/libcharon/plugins/unity/unity_handler.c @@ -50,8 +50,8 @@ struct private_unity_handler_t { * Traffic selector entry for networks to include under a given IKE_SA */ typedef struct { - /** associated IKE_SA, unique ID */ - u_int32_t sa; + /** associated IKE_SA COOKIEs */ + ike_sa_id_t *id; /** traffic selector to include/exclude */ traffic_selector_t *ts; } entry_t; @@ -61,6 +61,7 @@ typedef struct { */ static void entry_destroy(entry_t *this) { + this->id->destroy(this->id); this->ts->destroy(this->ts); free(this); } @@ -131,9 +132,10 @@ static bool add_include(private_unity_handler_t *this, chunk_t data) while (list->remove_first(list, (void**)&ts) == SUCCESS) { INIT(entry, - .sa = ike_sa->get_unique_id(ike_sa), + .id = ike_sa->get_id(ike_sa), .ts = ts, ); + entry->id = entry->id->clone(entry->id); this->mutex->lock(this->mutex); this->include->insert_last(this->include, entry); @@ -171,7 +173,7 @@ static bool remove_include(private_unity_handler_t *this, chunk_t data) enumerator = this->include->create_enumerator(this->include); while (enumerator->enumerate(enumerator, &entry)) { - if (entry->sa == ike_sa->get_unique_id(ike_sa) && + if (entry->id->equals(entry->id, ike_sa->get_id(ike_sa)) && ts->equals(ts, entry->ts)) { this->include->remove_at(this->include, enumerator); @@ -209,8 +211,7 @@ static job_requeue_t add_exclude_async(entry_t *entry) char name[128]; host_t *host; - ike_sa = charon->ike_sa_manager->checkout_by_id(charon->ike_sa_manager, - entry->sa); + ike_sa = charon->ike_sa_manager->checkout(charon->ike_sa_manager, entry->id); if (ike_sa) { create_shunt_name(ike_sa, entry->ts, name, sizeof(name)); @@ -267,9 +268,10 @@ static bool add_exclude(private_unity_handler_t *this, chunk_t data) while (list->remove_first(list, (void**)&ts) == SUCCESS) { INIT(entry, - .sa = ike_sa->get_unique_id(ike_sa), + .id = ike_sa->get_id(ike_sa), .ts = ts, ); + entry->id = entry->id->clone(entry->id); /* we can't install the shunt policy yet, as we don't know the virtual IP. * Defer installation using an async callback. */ @@ -402,7 +404,7 @@ typedef struct { /** mutex to unlock */ mutex_t *mutex; /** IKE_SA ID to filter for */ - u_int32_t id; + ike_sa_id_t *id; } include_filter_t; /** @@ -411,7 +413,7 @@ typedef struct { static bool include_filter(include_filter_t *data, entry_t **entry, traffic_selector_t **ts) { - if ((*entry)->sa == data->id) + if (data->id->equals(data->id, (*entry)->id)) { *ts = (*entry)->ts; return TRUE; @@ -429,7 +431,7 @@ static void destroy_filter(include_filter_t *data) } METHOD(unity_handler_t, create_include_enumerator, enumerator_t*, - private_unity_handler_t *this, u_int32_t id) + private_unity_handler_t *this, ike_sa_id_t *id) { include_filter_t *data; diff --git a/src/libcharon/plugins/unity/unity_handler.h b/src/libcharon/plugins/unity/unity_handler.h index 8656fd372..18efe293b 100644 --- a/src/libcharon/plugins/unity/unity_handler.h +++ b/src/libcharon/plugins/unity/unity_handler.h @@ -21,6 +21,7 @@ #ifndef UNITY_HANDLER_H_ #define UNITY_HANDLER_H_ +#include #include typedef struct unity_handler_t unity_handler_t; @@ -38,11 +39,11 @@ struct unity_handler_t { /** * Create an enumerator over Split-Include attributes received for an IKE_SA. * - * @param id IKE_SA unique ID to get Split-Includes for + * @param id IKE_SA ID to get Split-Includes for * @return enumerator over traffic_selector_t* */ enumerator_t* (*create_include_enumerator)(unity_handler_t *this, - u_int32_t id); + ike_sa_id_t *id); /** * Destroy a unity_handler_t. diff --git a/src/libcharon/plugins/unity/unity_narrow.c b/src/libcharon/plugins/unity/unity_narrow.c index 94089563d..227d24be8 100644 --- a/src/libcharon/plugins/unity/unity_narrow.c +++ b/src/libcharon/plugins/unity/unity_narrow.c @@ -75,7 +75,7 @@ static void narrow_initiator(private_unity_narrow_t *this, ike_sa_t *ike_sa, enumerator_t *enumerator; enumerator = this->handler->create_include_enumerator(this->handler, - ike_sa->get_unique_id(ike_sa)); + ike_sa->get_id(ike_sa)); while (enumerator->enumerate(enumerator, ¤t)) { if (orig == NULL) @@ -159,7 +159,7 @@ static bool has_split_includes(private_unity_narrow_t *this, ike_sa_t *ike_sa) bool has; enumerator = this->handler->create_include_enumerator(this->handler, - ike_sa->get_unique_id(ike_sa)); + ike_sa->get_id(ike_sa)); has = enumerator->enumerate(enumerator, &ts); enumerator->destroy(enumerator); From 246c969d8bc98194c300989d545d8fa40e246399 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Tue, 28 Oct 2014 10:54:38 +0100 Subject: [PATCH 28/28] ike: Consistently log CHILD_SAs with their unique_id instead of their reqid --- .../android/jni/libandroidbridge/backend/android_service.c | 2 +- src/libcharon/plugins/ha/ha_child.c | 2 +- src/libcharon/plugins/ha/ha_dispatcher.c | 2 +- src/libcharon/sa/ikev1/task_manager_v1.c | 3 ++- src/libcharon/sa/ikev1/tasks/quick_delete.c | 4 ++-- src/libcharon/sa/ikev1/tasks/quick_mode.c | 5 +++-- src/libcharon/sa/ikev2/tasks/child_create.c | 2 +- src/libcharon/sa/ikev2/tasks/child_delete.c | 4 ++-- 8 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/frontends/android/jni/libandroidbridge/backend/android_service.c b/src/frontends/android/jni/libandroidbridge/backend/android_service.c index 64a79077e..896bb0940 100644 --- a/src/frontends/android/jni/libandroidbridge/backend/android_service.c +++ b/src/frontends/android/jni/libandroidbridge/backend/android_service.c @@ -285,7 +285,7 @@ static bool setup_tun_device(private_android_service_t *this, int tunfd; DBG1(DBG_DMN, "setting up TUN device for CHILD_SA %s{%u}", - child_sa->get_name(child_sa), child_sa->get_reqid(child_sa)); + child_sa->get_name(child_sa), child_sa->get_unique_id(child_sa)); builder = charonservice->get_vpnservice_builder(charonservice); diff --git a/src/libcharon/plugins/ha/ha_child.c b/src/libcharon/plugins/ha/ha_child.c index c166d72ac..ed6ca7196 100644 --- a/src/libcharon/plugins/ha/ha_child.c +++ b/src/libcharon/plugins/ha/ha_child.c @@ -128,7 +128,7 @@ METHOD(listener_t, child_keys, bool, ike_sa->get_other_host(ike_sa), child_sa->get_spi(child_sa, FALSE)); DBG1(DBG_CFG, "handling HA CHILD_SA %s{%d} %#R=== %#R " "(segment in: %d%s, out: %d%s)", child_sa->get_name(child_sa), - child_sa->get_reqid(child_sa), local_ts, remote_ts, + child_sa->get_unique_id(child_sa), local_ts, remote_ts, seg_i, this->segments->is_active(this->segments, seg_i) ? "*" : "", seg_o, this->segments->is_active(this->segments, seg_o) ? "*" : ""); diff --git a/src/libcharon/plugins/ha/ha_dispatcher.c b/src/libcharon/plugins/ha/ha_dispatcher.c index 6e02733f9..983f9d8fc 100644 --- a/src/libcharon/plugins/ha/ha_dispatcher.c +++ b/src/libcharon/plugins/ha/ha_dispatcher.c @@ -836,7 +836,7 @@ static void process_child_add(private_ha_dispatcher_t *this, DBG1(DBG_CFG, "installed HA CHILD_SA %s{%d} %#R=== %#R " "(segment in: %d%s, out: %d%s)", child_sa->get_name(child_sa), - child_sa->get_reqid(child_sa), local_ts, remote_ts, + child_sa->get_unique_id(child_sa), local_ts, remote_ts, seg_i, this->segments->is_active(this->segments, seg_i) ? "*" : "", seg_o, this->segments->is_active(this->segments, seg_o) ? "*" : ""); child_sa->add_policies(child_sa, local_ts, remote_ts); diff --git a/src/libcharon/sa/ikev1/task_manager_v1.c b/src/libcharon/sa/ikev1/task_manager_v1.c index 3184db4ee..cb22bf606 100644 --- a/src/libcharon/sa/ikev1/task_manager_v1.c +++ b/src/libcharon/sa/ikev1/task_manager_v1.c @@ -1596,7 +1596,8 @@ static bool is_redundant(private_task_manager_t *this, child_sa_t *child_sa) child_sa->get_lifetime(child_sa, FALSE)) { DBG1(DBG_IKE, "deleting redundant CHILD_SA %s{%d}", - child_sa->get_name(child_sa), child_sa->get_reqid(child_sa)); + child_sa->get_name(child_sa), + child_sa->get_unique_id(child_sa)); redundant = TRUE; break; } diff --git a/src/libcharon/sa/ikev1/tasks/quick_delete.c b/src/libcharon/sa/ikev1/tasks/quick_delete.c index 499081caa..4206182a0 100644 --- a/src/libcharon/sa/ikev1/tasks/quick_delete.c +++ b/src/libcharon/sa/ikev1/tasks/quick_delete.c @@ -116,7 +116,7 @@ static bool delete_child(private_quick_delete_t *this, protocol_id_t protocol, { DBG0(DBG_IKE, "closing expired CHILD_SA %s{%d} " "with SPIs %.8x_i %.8x_o and TS %#R=== %#R", - child_sa->get_name(child_sa), child_sa->get_reqid(child_sa), + child_sa->get_name(child_sa), child_sa->get_unique_id(child_sa), ntohl(child_sa->get_spi(child_sa, TRUE)), ntohl(child_sa->get_spi(child_sa, FALSE)), my_ts, other_ts); } @@ -127,7 +127,7 @@ static bool delete_child(private_quick_delete_t *this, protocol_id_t protocol, DBG0(DBG_IKE, "closing CHILD_SA %s{%d} with SPIs " "%.8x_i (%llu bytes) %.8x_o (%llu bytes) and TS %#R=== %#R", - child_sa->get_name(child_sa), child_sa->get_reqid(child_sa), + child_sa->get_name(child_sa), child_sa->get_unique_id(child_sa), ntohl(child_sa->get_spi(child_sa, TRUE)), bytes_in, ntohl(child_sa->get_spi(child_sa, FALSE)), bytes_out, my_ts, other_ts); diff --git a/src/libcharon/sa/ikev1/tasks/quick_mode.c b/src/libcharon/sa/ikev1/tasks/quick_mode.c index 8ca3ac72e..828b166d0 100644 --- a/src/libcharon/sa/ikev1/tasks/quick_mode.c +++ b/src/libcharon/sa/ikev1/tasks/quick_mode.c @@ -385,7 +385,7 @@ static bool install(private_quick_mode_t *this) DBG0(DBG_IKE, "CHILD_SA %s{%d} established " "with SPIs %.8x_i %.8x_o and TS %#R=== %#R", this->child_sa->get_name(this->child_sa), - this->child_sa->get_reqid(this->child_sa), + this->child_sa->get_unique_id(this->child_sa), ntohl(this->child_sa->get_spi(this->child_sa, TRUE)), ntohl(this->child_sa->get_spi(this->child_sa, FALSE)), my_ts, other_ts); @@ -986,7 +986,8 @@ static void check_for_rekeyed_child(private_quick_mode_t *this) FALSE).value; child_sa->set_state(child_sa, CHILD_REKEYING); DBG1(DBG_IKE, "detected rekeying of CHILD_SA %s{%u}", - child_sa->get_name(child_sa), this->reqid); + child_sa->get_name(child_sa), + child_sa->get_unique_id(child_sa)); } policies->destroy(policies); break; diff --git a/src/libcharon/sa/ikev2/tasks/child_create.c b/src/libcharon/sa/ikev2/tasks/child_create.c index abee0da3c..1bb934002 100644 --- a/src/libcharon/sa/ikev2/tasks/child_create.c +++ b/src/libcharon/sa/ikev2/tasks/child_create.c @@ -703,7 +703,7 @@ static status_t select_and_install(private_child_create_t *this, DBG0(DBG_IKE, "CHILD_SA %s{%d} established " "with SPIs %.8x_i %.8x_o and TS %#R=== %#R", this->child_sa->get_name(this->child_sa), - this->child_sa->get_reqid(this->child_sa), + this->child_sa->get_unique_id(this->child_sa), ntohl(this->child_sa->get_spi(this->child_sa, TRUE)), ntohl(this->child_sa->get_spi(this->child_sa, FALSE)), my_ts, other_ts); diff --git a/src/libcharon/sa/ikev2/tasks/child_delete.c b/src/libcharon/sa/ikev2/tasks/child_delete.c index 2b1697423..f0b11e291 100644 --- a/src/libcharon/sa/ikev2/tasks/child_delete.c +++ b/src/libcharon/sa/ikev2/tasks/child_delete.c @@ -267,7 +267,7 @@ static void log_children(private_child_delete_t *this) { DBG0(DBG_IKE, "closing expired CHILD_SA %s{%d} " "with SPIs %.8x_i %.8x_o and TS %#R=== %#R", - child_sa->get_name(child_sa), child_sa->get_reqid(child_sa), + child_sa->get_name(child_sa), child_sa->get_unique_id(child_sa), ntohl(child_sa->get_spi(child_sa, TRUE)), ntohl(child_sa->get_spi(child_sa, FALSE)), my_ts, other_ts); } @@ -278,7 +278,7 @@ static void log_children(private_child_delete_t *this) DBG0(DBG_IKE, "closing CHILD_SA %s{%d} with SPIs %.8x_i " "(%llu bytes) %.8x_o (%llu bytes) and TS %#R=== %#R", - child_sa->get_name(child_sa), child_sa->get_reqid(child_sa), + child_sa->get_name(child_sa), child_sa->get_unique_id(child_sa), ntohl(child_sa->get_spi(child_sa, TRUE)), bytes_in, ntohl(child_sa->get_spi(child_sa, FALSE)), bytes_out, my_ts, other_ts);