From 82b5d1c01832a79c65b002b1677aac7ed015cb52 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 13 Aug 2015 10:34:47 +0200 Subject: [PATCH 1/6] kernel-netlink: Only flush SAs of types we actually manage --- .../kernel_netlink/kernel_netlink_ipsec.c | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c b/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c index 2958b5942..8ea2914e0 100644 --- a/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c +++ b/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c @@ -2024,23 +2024,36 @@ METHOD(kernel_ipsec_t, flush_sas, status_t, netlink_buf_t request; struct nlmsghdr *hdr; struct xfrm_usersa_flush *flush; + struct { + u_int8_t proto; + char *name; + } protos[] = { + { IPPROTO_AH, "AH" }, + { IPPROTO_ESP, "ESP" }, + { IPPROTO_COMP, "IPComp" }, + }; + int i; memset(&request, 0, sizeof(request)); - DBG2(DBG_KNL, "flushing all SAD entries"); - hdr = &request.hdr; hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; hdr->nlmsg_type = XFRM_MSG_FLUSHSA; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_usersa_flush)); flush = NLMSG_DATA(hdr); - flush->proto = IPSEC_PROTO_ANY; - if (this->socket_xfrm->send_ack(this->socket_xfrm, hdr) != SUCCESS) + for (i = 0; i < countof(protos); i++) { - DBG1(DBG_KNL, "unable to flush SAD entries"); - return FAILED; + DBG2(DBG_KNL, "flushing all %s SAD entries", protos[i].name); + + flush->proto = protos[i].proto; + + if (this->socket_xfrm->send_ack(this->socket_xfrm, hdr) != SUCCESS) + { + DBG1(DBG_KNL, "unable to flush %s SAD entries", protos[i].name); + return FAILED; + } } return SUCCESS; } From 603e3b489bb8a448f0dbcad9406fbfb64523abe1 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 13 Aug 2015 11:01:50 +0200 Subject: [PATCH 2/6] kernel-pfkey: Only flush SAs of types we actually manage --- .../plugins/kernel_pfkey/kernel_pfkey_ipsec.c | 39 ++++++++++++------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c b/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c index f1b975e75..3583dfeba 100644 --- a/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c +++ b/src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.c @@ -2086,31 +2086,44 @@ METHOD(kernel_ipsec_t, flush_sas, status_t, { unsigned char request[PFKEY_BUFFER_SIZE]; struct sadb_msg *msg, *out; + struct { + u_int8_t proto; + char *name; + } protos[] = { + { SADB_SATYPE_AH, "AH" }, + { SADB_SATYPE_ESP, "ESP" }, + { SADB_X_SATYPE_IPCOMP, "IPComp" }, + }; size_t len; + int i; memset(&request, 0, sizeof(request)); - DBG2(DBG_KNL, "flushing all SAD entries"); - msg = (struct sadb_msg*)request; msg->sadb_msg_version = PF_KEY_V2; msg->sadb_msg_type = SADB_FLUSH; - msg->sadb_msg_satype = SADB_SATYPE_UNSPEC; msg->sadb_msg_len = PFKEY_LEN(sizeof(struct sadb_msg)); - if (pfkey_send(this, msg, &out, &len) != SUCCESS) + for (i = 0; i < countof(protos); i++) { - DBG1(DBG_KNL, "unable to flush SAD entries"); - return FAILED; - } - else if (out->sadb_msg_errno) - { - DBG1(DBG_KNL, "unable to flush SAD entries: %s (%d)", - strerror(out->sadb_msg_errno), out->sadb_msg_errno); + DBG2(DBG_KNL, "flushing all %s SAD entries", protos[i].name); + + msg->sadb_msg_satype = protos[i].proto; + if (pfkey_send(this, msg, &out, &len) != SUCCESS) + { + DBG1(DBG_KNL, "unable to flush %s SAD entries", protos[i].name); + return FAILED; + } + else if (out->sadb_msg_errno) + { + DBG1(DBG_KNL, "unable to flush %s SAD entries: %s (%d)", + protos[i].name, strerror(out->sadb_msg_errno), + out->sadb_msg_errno); + free(out); + return FAILED; + } free(out); - return FAILED; } - free(out); return SUCCESS; } From bd24f87d35f505a94814fd93b86816d69761527e Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 13 Aug 2015 11:08:41 +0200 Subject: [PATCH 3/6] starter: Don't flush policies in the kernel We can't control which policies we flush, so if policies are installed and used outside of strongSwan for other protocols we'd flush them too. And if installpolicies=no is used we probably shouldn't flush policies either. Luckily already existing policies are not treated as fatal errors anymore, so not flushing policies should not be that much of an issue (in case of a crash in dynamic setups, e.g. with virtual IPs, policies could be left behind even after restarting the connections and properly terminating the daemon). --- src/starter/netkey.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/starter/netkey.c b/src/starter/netkey.c index 2b500bab4..0b677fb79 100644 --- a/src/starter/netkey.c +++ b/src/starter/netkey.c @@ -65,6 +65,5 @@ void starter_netkey_cleanup(void) return; } hydra->kernel_interface->flush_sas(hydra->kernel_interface); - hydra->kernel_interface->flush_policies(hydra->kernel_interface); lib->plugins->unload(lib->plugins); } From d8fdd1018e1654b04b614354a493026a9dad30e5 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 21 Aug 2015 13:57:00 +0200 Subject: [PATCH 4/6] starter: Don't flush SAs in the kernel If starter is not used we don't do that either. And this allows us to move the stuff in libhydra back to libcharon. --- src/starter/netkey.c | 12 ------------ src/starter/netkey.h | 1 - src/starter/starter.c | 1 - 3 files changed, 14 deletions(-) diff --git a/src/starter/netkey.c b/src/starter/netkey.c index 0b677fb79..3eb6973a1 100644 --- a/src/starter/netkey.c +++ b/src/starter/netkey.c @@ -55,15 +55,3 @@ bool starter_netkey_init(void) DBG2(DBG_APP, "found netkey IPsec stack"); return TRUE; } - -void starter_netkey_cleanup(void) -{ - if (!lib->plugins->load(lib->plugins, - lib->settings->get_str(lib->settings, "starter.load", PLUGINS))) - { - DBG1(DBG_APP, "unable to load kernel plugins"); - return; - } - hydra->kernel_interface->flush_sas(hydra->kernel_interface); - lib->plugins->unload(lib->plugins); -} diff --git a/src/starter/netkey.h b/src/starter/netkey.h index c12924174..bc71af2ed 100644 --- a/src/starter/netkey.h +++ b/src/starter/netkey.h @@ -16,7 +16,6 @@ #define _STARTER_NETKEY_H_ extern bool starter_netkey_init (void); -extern void starter_netkey_cleanup (void); #endif /* _STARTER_NETKEY_H_ */ diff --git a/src/starter/starter.c b/src/starter/starter.c index a19298923..ab1ebdd5d 100644 --- a/src/starter/starter.c +++ b/src/starter/starter.c @@ -703,7 +703,6 @@ int main (int argc, char **argv) { starter_stop_charon(); } - starter_netkey_cleanup(); confread_free(cfg); unlink(starter_pid_file); cleanup(); From c91682d1b82959d6d18972d0a73736c8469c8e0f Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 21 Aug 2015 14:32:29 +0200 Subject: [PATCH 5/6] testing: Flush state and policies before every scenario Similar to conntrack we make sure we are working on a clean slate. --- testing/do-tests | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/testing/do-tests b/testing/do-tests index f12101926..5191d9007 100755 --- a/testing/do-tests +++ b/testing/do-tests @@ -364,6 +364,14 @@ do ssh $SSHCONF root@`eval echo \\\$ipv4_$host` 'conntrack -F' >/dev/null 2>&1 done + ########################################################################## + # flush IPsec state on all hosts + # + + for host in $STRONGSWANHOSTS + do + ssh $SSHCONF root@`eval echo \\\$ipv4_$host` 'ip xfrm state flush; ip xfrm policy flush' >/dev/null 2>&1 + done ########################################################################## # execute pre-test commands From 9086f060d35a71cd7d6a53006b57fe6c77a70156 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Fri, 21 Aug 2015 14:33:26 +0200 Subject: [PATCH 6/6] testing: Let test scenarios fail if IPsec SAs or policies are not removed The IKE daemon should delete all installed SAs and policies when everything works properly, so we fail the test if that's not the case. --- testing/do-tests | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/testing/do-tests b/testing/do-tests index 5191d9007..c01152c7b 100755 --- a/testing/do-tests +++ b/testing/do-tests @@ -726,6 +726,24 @@ do } }' $TESTDIR/posttest.dat` >> $CONSOLE_LOG 2>&1 + ########################################################################## + # check that IPsec state was cleaned up properly + # + + for host in $IPSECHOSTS + do + eval HOSTLOGIN=root@\$ipv4_${host} + IPSECSTATE=`ssh $SSHCONF $HOSTLOGIN 'ip xfrm state'` + IPSECPOLICY=`ssh $SSHCONF $HOSTLOGIN 'ip xfrm policy'` + if [ -n "$IPSECSTATE" -o -n "$IPSECPOLICY" ] + then + echo -e "\n$host# ip xfrm state [NO]" >> $CONSOLE_LOG + echo "$IPSECSTATE" >> $CONSOLE_LOG + echo -e "\n$host# ip xfrm policy [NO]" >> $CONSOLE_LOG + echo "$IPSECPOLICY" >> $CONSOLE_LOG + STATUS="failed" + fi + done ########################################################################## # get a copy of /var/log/auth.log