charon-tkm: Call esa_reset() when the inbound SA is deleted
authorTobias Brunner <tobias@strongswan.org>
Fri, 4 Aug 2017 12:02:42 +0000 (14:02 +0200)
committerTobias Brunner <tobias@strongswan.org>
Mon, 7 Aug 2017 08:46:00 +0000 (10:46 +0200)
After a rekeying the outbound SA and policy is deleted immediately, however,
the inbound SA is not removed until a few seconds later, so delayed packets
can still be processed.

This adds a flag to get_esa_id() that specifies the location of the
given SPI.

src/charon-tkm/src/tkm/tkm_kernel_ipsec.c
src/charon-tkm/src/tkm/tkm_kernel_sad.c
src/charon-tkm/src/tkm/tkm_kernel_sad.h
src/charon-tkm/tests/kernel_sad_tests.c
testing/tests/tkm/xfrmproxy-expire/evaltest.dat
testing/tests/tkm/xfrmproxy-expire/hosts/moon/etc/strongswan.conf
testing/tests/tkm/xfrmproxy-expire/hosts/moon/etc/tkm/tkm.conf
testing/tests/tkm/xfrmproxy-rekey/evaltest.dat
testing/tests/tkm/xfrmproxy-rekey/hosts/moon/etc/strongswan.conf
testing/tests/tkm/xfrmproxy-rekey/hosts/sun/etc/ipsec.conf

index 8ccae4c..5decde9 100644 (file)
@@ -216,7 +216,7 @@ METHOD(kernel_ipsec_t, del_sa, status_t,
        esa_id_type esa_id;
 
        esa_id = tkm->sad->get_esa_id(tkm->sad, id->src, id->dst,
-                                                                 id->spi, id->proto);
+                                                                 id->spi, id->proto, TRUE);
        if (esa_id)
        {
                DBG1(DBG_KNL, "deleting child SA (esa: %llu, spi: %x)", esa_id,
@@ -272,7 +272,7 @@ METHOD(kernel_ipsec_t, add_policy, status_t,
                        return FAILED;
                }
                esa_id = tkm->sad->get_esa_id(tkm->sad, data->src, data->dst,
-                                                                         spi, proto);
+                                                                         spi, proto, FALSE);
                if (!esa_id)
                {
                        DBG1(DBG_KNL, "unable to find esa ID for policy (spi: %x)",
index 92ea25e..acc3ff1 100644 (file)
@@ -107,16 +107,23 @@ CALLBACK(sad_entry_match, bool,
        const host_t *src, *dst;
        const uint32_t *spi;
        const uint8_t *proto;
+       const bool *local;
 
-       VA_ARGS_VGET(args, src, dst, spi, proto);
+       VA_ARGS_VGET(args, src, dst, spi, proto, local);
 
-       if (entry->src == NULL || entry->dst == NULL)
+       if (entry->src == NULL || entry->dst == NULL || entry->proto != *proto)
        {
                return FALSE;
        }
-       return src->ip_equals(entry->src, (host_t *)src) &&
-                  dst->ip_equals(entry->dst, (host_t *)dst) &&
-                  entry->spi_rem == *spi && entry->proto == *proto;
+       if (*local)
+       {
+               return entry->src->ip_equals(entry->src, (host_t *)dst) &&
+                          entry->dst->ip_equals(entry->dst, (host_t *)src) &&
+                          entry->spi_loc == *spi;
+       }
+       return entry->src->ip_equals(entry->src, (host_t *)src) &&
+                  entry->dst->ip_equals(entry->dst, (host_t *)dst) &&
+                  entry->spi_rem == *spi;
 }
 
 CALLBACK(sad_entry_match_dst, bool,
@@ -193,7 +200,8 @@ METHOD(tkm_kernel_sad_t, insert, bool,
 
 METHOD(tkm_kernel_sad_t, get_esa_id, esa_id_type,
        private_tkm_kernel_sad_t * const this, const host_t * const src,
-       const host_t * const dst, const uint32_t spi, const uint8_t proto)
+       const host_t * const dst, const uint32_t spi, const uint8_t proto,
+       const bool local)
 {
        esa_id_type id = 0;
        sad_entry_t *entry = NULL;
@@ -201,17 +209,18 @@ METHOD(tkm_kernel_sad_t, get_esa_id, esa_id_type,
        this->mutex->lock(this->mutex);
        const bool res = this->data->find_first(this->data, sad_entry_match,
                                                                                        (void**)&entry, src, dst, &spi,
-                                                                                       &proto);
+                                                                                       &proto, &local);
        if (res && entry)
        {
                id = entry->esa_id;
                DBG3(DBG_KNL, "returning ESA id %llu of SAD entry (src: %H, dst: %H, "
-                        "spi: %x, proto: %u)", id, src, dst, ntohl(spi), proto);
+                        "%sbound spi: %x, proto: %u)", id, src, dst, local ? "in" : "out",
+                        ntohl(spi), proto);
        }
        else
        {
-               DBG3(DBG_KNL, "no SAD entry found for src %H, dst %H, spi %x, proto %u",
-                        src, dst, ntohl(spi), proto);
+               DBG3(DBG_KNL, "no SAD entry found for src %H, dst %H, %sbound spi %x, "
+                        "proto %u", src, dst, local ? "in" : "out", ntohl(spi), proto);
        }
        this->mutex->unlock(this->mutex);
        return id;
index bcb3b43..3d9f5f3 100644 (file)
@@ -55,13 +55,14 @@ struct tkm_kernel_sad_t {
         *
         * @param src                   source address of CHILD SA
         * @param dst                   destination address of CHILD SA
-        * @param spi                   Remote SPI of CHILD SA
+        * @param spi                   SPI of CHILD SA
         * @param proto                 protocol of CHILD SA (ESP/AH)
+        * @param local                 whether the SPI is local or remote
         * @return                              ESA id of entry if found, 0 otherwise
         */
        esa_id_type (*get_esa_id)(tkm_kernel_sad_t * const this,
                                 const host_t * const src, const host_t * const dst,
-                                const uint32_t spi, const uint8_t proto);
+                                const uint32_t spi, const uint8_t proto, const bool local);
 
        /**
         * Get destination host for entry with given parameters.
index 5dbe0ae..39d8a79 100644 (file)
@@ -63,7 +63,20 @@ START_TEST(test_get_esa_id)
        tkm_kernel_sad_t *sad = tkm_kernel_sad_create();
        fail_unless(sad->insert(sad, 23, 54, addr, addr, 27, 42, 50),
                                "Error inserting SAD entry");
-       fail_unless(sad->get_esa_id(sad, addr, addr, 42, 50) == 23,
+       fail_unless(sad->get_esa_id(sad, addr, addr, 42, 50, FALSE) == 23,
+                               "Error getting esa id");
+       sad->destroy(sad);
+       addr->destroy(addr);
+}
+END_TEST
+
+START_TEST(test_get_esa_id_local)
+{
+       host_t *addr = host_create_from_string("127.0.0.1", 1024);
+       tkm_kernel_sad_t *sad = tkm_kernel_sad_create();
+       fail_unless(sad->insert(sad, 23, 54, addr, addr, 27, 42, 50),
+                               "Error inserting SAD entry");
+       fail_unless(sad->get_esa_id(sad, addr, addr, 27, 50, TRUE) == 23,
                                "Error getting esa id");
        sad->destroy(sad);
        addr->destroy(addr);
@@ -74,7 +87,7 @@ START_TEST(test_get_esa_id_nonexistent)
 {
        host_t *addr = host_create_from_string("127.0.0.1", 1024);
        tkm_kernel_sad_t *sad = tkm_kernel_sad_create();
-       fail_unless(sad->get_esa_id(sad, addr, addr, 42, 50) == 0,
+       fail_unless(sad->get_esa_id(sad, addr, addr, 42, 50, FALSE) == 0,
                                "Got esa id for nonexistent SAD entry");
        sad->destroy(sad);
        addr->destroy(addr);
@@ -148,6 +161,7 @@ Suite *make_kernel_sad_tests()
 
        tc = tcase_create("get_esa_id");
        tcase_add_test(tc, test_get_esa_id);
+       tcase_add_test(tc, test_get_esa_id_local);
        tcase_add_test(tc, test_get_esa_id_nonexistent);
        suite_add_tcase(s, tc);
 
index 05bf420..a3f4587 100644 (file)
@@ -2,20 +2,24 @@ moon::ipsec stroke status 2> /dev/null::conn1.*ESTABLISHED.*moon.strongswan.org.
 sun::ipsec status 2> /dev/null::host-host.*ESTABLISHED.*sun.strongswan.org.*moon.strongswan.org::YES
 moon::ipsec stroke status 2> /dev/null::conn1.*INSTALLED, TRANSPORT::YES
 sun::ipsec status 2> /dev/null::host-host.*INSTALLED, TRANSPORT::YES
-moon::ping -c 1 PH_IP_SUN::64 bytes from PH_IP_SUN: icmp_.eq=1::YES
-sun::tcpdump::IP moon.strongswan.org > sun.strongswan.org: ESP::YES
-sun::tcpdump::IP sun.strongswan.org > moon.strongswan.org: ESP::YES
+moon::sleep 2::wait for rekeying::NO
 moon::cat /var/log/daemon.log::ees: acquire received for reqid 1::YES
 moon::cat /var/log/daemon.log::ees: expire received for reqid 1, spi.*, dst 192.168.0.2::YES
 moon::cat /var/log/daemon.log::creating rekey job for CHILD_SA ESP/0x.*/192.168.0.2::YES
+moon::cat /var/log/daemon.log::deleting child SA (esa: 1, spi:.*)::NO
+moon::ping -c 1 PH_IP_SUN::64 bytes from PH_IP_SUN: icmp_.eq=1::YES
+sun::tcpdump::IP moon.strongswan.org > sun.strongswan.org: ESP::YES
+sun::tcpdump::IP sun.strongswan.org > moon.strongswan.org: ESP::YES
+moon::sleep 2::wait until inbound SA is deleted::NO
 moon::cat /var/log/daemon.log::deleting child SA (esa: 1, spi:.*)::YES
+moon::ping -c 1 PH_IP_SUN::64 bytes from PH_IP_SUN: icmp_.eq=1::YES
 moon::cat /tmp/tkm.log::RSA private key '/etc/tkm/moonKey.der' loaded::YES
 moon::cat /tmp/tkm.log::Adding policy \[ 1, 192.168.0.1 <-> 192.168.0.2 \]::YES
 moon::cat /tmp/tkm.log::Checked CA certificate of CC context 1::YES
 moon::cat /tmp/tkm.log::Authentication of ISA context 1 successful::YES
 moon::cat /tmp/tkm.log::Creating first new ESA context with ID 1 (Isa 1, Sp 1, Ea 1, Initiator TRUE, spi_loc.*, spi_rem.*)::YES
 moon::cat /tmp/tkm.log::Creating ESA context with ID 2 (Isa 1, Sp 1, Ea 1, Dh_Id 1, Nc_Loc_Id 1, Initiator TRUE, spi_loc.*, spi_rem.*)::YES
-moon::cat /tmp/tkm.log | grep 'Adding ESA \[ 1, 192.168.0.1 <-> 192.168.0.2, SPI_in.*, SPI_out.*, soft 2, hard 60 \]' | wc -l::2::YES
+moon::cat /tmp/tkm.log | grep 'Adding ESA \[ 1, 192.168.0.1 <-> 192.168.0.2, SPI_in.*, SPI_out.*, soft 4, hard 60 \]' | wc -l::2::YES
 moon::cat /tmp/tkm.log::Resetting ESA context 1::YES
 moon::cat /tmp/tkm.log::Deleting ESA \[ 1, 192.168.0.1 <=> 192.168.0.2, SPI_in.*, SPI_out.* \]::YES
 moon::cat /tmp/xfrm_proxy.log::Initiating ESA acquire for reqid 1::YES
index cc9d6e0..5b79af9 100644 (file)
@@ -1,6 +1,8 @@
 # /etc/strongswan.conf - strongSwan configuration file
 
 charon-tkm {
+  # remove rekeyed inbound SA a bit quicker for the test scenario
+  delete_rekeyed_delay = 2
   dh_mapping {
     15 = 1
     16 = 2
index 23e958a..62b103a 100644 (file)
@@ -14,7 +14,7 @@
             <ip>192.168.0.2</ip>
         </remote>
         <lifetime>
-            <soft>2</soft>
+            <soft>4</soft>
             <hard>60</hard>
         </lifetime>
     </policy>
index 328b485..15bdf3b 100644 (file)
@@ -2,11 +2,15 @@ moon::ipsec stroke status 2> /dev/null::conn1.*ESTABLISHED.*moon.strongswan.org.
 sun::ipsec status 2> /dev/null::host-host.*ESTABLISHED.*sun.strongswan.org.*moon.strongswan.org::YES
 moon::ipsec stroke status 2> /dev/null::conn1.*INSTALLED, TRANSPORT::YES
 sun::ipsec status 2> /dev/null::host-host.*INSTALLED, TRANSPORT::YES
+moon::sleep 2::wait for rekeying::NO
+sun::cat /var/log/daemon.log::creating rekey job for CHILD_SA ESP/0x.*/192.168.0.2::YES
 moon::ping -c 1 PH_IP_SUN::64 bytes from PH_IP_SUN: icmp_.eq=1::YES
 sun::tcpdump::IP moon.strongswan.org > sun.strongswan.org: ESP::YES
 sun::tcpdump::IP sun.strongswan.org > moon.strongswan.org: ESP::YES
-sun::cat /var/log/daemon.log::creating rekey job for CHILD_SA ESP/0x.*/192.168.0.2::YES
+moon::cat /var/log/daemon.log::deleting child SA (esa: 1, spi:.*)::NO
+moon::sleep 2::wait until inbound SA is deleted::NO
 moon::cat /var/log/daemon.log::deleting child SA (esa: 1, spi:.*)::YES
+moon::ping -c 1 PH_IP_SUN::64 bytes from PH_IP_SUN: icmp_.eq=1::YES
 moon::cat /tmp/tkm.log::RSA private key '/etc/tkm/moonKey.der' loaded::YES
 moon::cat /tmp/tkm.log::Adding policy \[ 1, 192.168.0.1 <-> 192.168.0.2 \]::YES
 moon::cat /tmp/tkm.log::Checked CA certificate of CC context 1::YES
index cc9d6e0..5b79af9 100644 (file)
@@ -1,6 +1,8 @@
 # /etc/strongswan.conf - strongSwan configuration file
 
 charon-tkm {
+  # remove rekeyed inbound SA a bit quicker for the test scenario
+  delete_rekeyed_delay = 2
   dh_mapping {
     15 = 1
     16 = 2
index 99ffb30..9dc6412 100644 (file)
@@ -5,7 +5,7 @@ config setup
 conn %default
        ikelifetime=60m
        keylife=10s
-       rekeymargin=8s
+       rekeymargin=6s
        rekeyfuzz=0%
        keyingtries=1
        keyexchange=ikev2