Re: [Ipsec-tools-devel] SAs not gettin g purged when peer goes down

[Darrel G. <dgo...@tr...>]

Venkat Yekkirala wrote:
> Hello,
> 
> When we use NAT-Traversal with racoon on Linux 2.6.18
> we encounter a problem with the SAs not getting purged
> when the peer racoon goes down. Specifically, the following
> logic doesn't seem to be using the SA ports to compare them
> to the Phase1 ports while it seems like it should actually
> use the encap ports that are tied to the SA when comparing
> with the Phase1 ports.

To add a bit more information...  We are experiencing this behavior with
linux kernels from 2.6.14 - 2.6.18.  It also makes no difference whether or
not we are actually using NAT-T - it is simply a matter of compiling in the
support for NAT-T that causes the problem.

> In purge_remote():
> 
>                 /* check in/outbound SAs */
>                 if ((CMPSADDR(iph1->local, src) ||
> CMPSADDR(iph1->remote, dst)) &&
>                         (CMPSADDR(iph1->local, dst) ||
> CMPSADDR(iph1->remote, src))) {
>                         msg = next;
>                         continue;
>                 }

Without ENABLE_NATT, the above comparisons are using cmpsaddrwop (compares
addresses without ports).  This allows the IPsec-SAs associated with an
ISAKMP-SA to be purged along with the ISAKMP-SA.  I'm hoping that is the
correct behavior because that is indeed the behavior that we like ;)  When
ENABLE_NATT is defined, the comparison is changed to include the ports of
the SAs.  As Venkat mentioned, that is a comparison of the isakmp port (from
the ISAKMP-SA) and the ports specified in the IPsec-SA.  This port comparison
always fails because the IPsec-SAs always have the ports set to 0 (so we are
comparing 500 to 0 in the "normal" case).  If a remote goes down, we flush the
phase1 SA, but not the phase2 SAa because the ports do not match.  When the
remote comes back, we are still trying to use the old phase2s and communication
is never restored (in the not ENABLE_NATT case, communication is restored
because a new set of IPsec SAs are negotiated).  DPD monitoring is also
affected because it uses purge_remote to delete SAs from a dead peer.

> 
> The above boils down to a cmpsaddrstrict() comparison
> of the Phase1 address/port to the SA address/port. But
> shouldn't it really be between the Phase1 address/port
> to the SA address/encap_port?

In the case that it should be checking phase1 ports against the encapsulation
port of the phase2, I have included a patch below that addresses two instances
where the change should occur.  These two changes alleviate the problems that were
standing in my way, but I suspect that the other invocations of CMPSADDR should
also be examined.  I'd appreciate any feedback on the approach, including the
possibility that we are way off on what I think is/should be happening.  It may
be that I am just confused about the way ports are being treated...  In the case
of the latter, I'd appreciate a schooling on what racoon is doing.

diff --git a/src/racoon/isakmp.c b/src/racoon/isakmp.c
index d8b529c..452b6c1 100644
--- a/src/racoon/isakmp.c
+++ b/src/racoon/isakmp.c
@@ -3085,6 +3085,9 @@ purge_remote(iph1)
 	u_int proto_id;
 	struct ph2handle *iph2;
 	struct ph1handle *new_iph1;
+#if defined(ENABLE_NATT) && defined(SADB_X_EXT_NAT_T_TYPE)
+	struct sadb_x_nat_t_type *natt_type;
+#endif /* ENABLE_NATT && SADB_X_EXT_NAT_T_TYPE */
 
 	plog(LLV_INFO, LOCATION, NULL,
 		 "purging ISAKMP-SA spi=%s.\n",
@@ -3144,12 +3147,35 @@ purge_remote(iph1)
 		}
 
 		/* check in/outbound SAs */
-		if ((CMPSADDR(iph1->local, src) || CMPSADDR(iph1->remote, dst)) &&
-			(CMPSADDR(iph1->local, dst) || CMPSADDR(iph1->remote, src))) {
+		if ((cmpsaddrwop(iph1->local, src) || cmpsaddrwop(iph1->remote, dst)) &&
+			(cmpsaddrwop(iph1->local, dst) || cmpsaddrwop(iph1->remote, src))) {
 			msg = next;
 			continue;
 		}
 
+#if defined(ENABLE_NATT) && defined(SADB_X_EXT_NAT_T_TYPE)
+		/* check the isakmp port of the in/outbound SAs */
+		natt_type = (void *)mhp[SADB_X_EXT_NAT_T_TYPE];
+
+		if (natt_type && natt_type->sadb_x_nat_t_type_type) {
+			struct sadb_x_nat_t_port *natt_sport, *natt_dport;
+			u_short ph1_lp, ph1_rp, natt_sp, natt_dp;
+
+			natt_sport = (void *)mhp[SADB_X_EXT_NAT_T_SPORT];
+			natt_dport = (void *)mhp[SADB_X_EXT_NAT_T_DPORT];
+			natt_sp = natt_sport->sadb_x_nat_t_port_port;
+			natt_dp = natt_dport->sadb_x_nat_t_port_port;
+			ph1_lp = ((struct sockaddr_in *)iph1->local)->sin_port;
+			ph1_rp = ((struct sockaddr_in *)iph1->remote)->sin_port;
+	
+			if ((ph1_lp != natt_sp || ph1_rp != natt_dp) &&
+			    (ph1_lp != natt_dp || ph1_rp != natt_sp)) {
+				msg = next;
+				continue;
+			}
+		}
+#endif /* ENABLE_NATT && SADB_X_EXT_NAT_T_TYPE */
+
 		proto_id = pfkey2ipsecdoi_proto(msg->sadb_msg_satype);
 		iph2 = getph2bysaidx(src, dst, proto_id, sa->sadb_sa_spi);
 
diff --git a/src/racoon/isakmp_inf.c b/src/racoon/isakmp_inf.c
index 9112bfd..65c7ce8 100644
--- a/src/racoon/isakmp_inf.c
+++ b/src/racoon/isakmp_inf.c
@@ -967,6 +967,9 @@ purge_ipsec_spi(dst0, proto, spi, n)
 	struct ph2handle *iph2;
 	size_t i;
 	caddr_t mhp[SADB_EXT_MAX + 1];
+#ifdef SADB_X_EXT_NAT_T_TYPE
+	struct sadb_x_nat_t_type *natt_type;
+#endif /* ENABLE_NATT && SADB_X_EXT_NAT_T_TYPE */
 
 	buf = pfkey_dump_sadb(ipsecdoi2pfkey_proto(proto));
 	if (buf == NULL) {
@@ -1014,11 +1017,29 @@ purge_ipsec_spi(dst0, proto, spi, n)
 
 		/* don't delete inbound SAs at the moment */
 		/* XXX should we remove SAs with opposite direction as well? */
-		if (CMPSADDR(dst0, dst)) {
+		if (cmpsaddrwop(dst0, dst)) {
 			msg = next;
 			continue;
 		}
 
+#if defined(ENABLE_NATT) && defined(SADB_X_EXT_NAT_T_TYPE)
+		/* check the isakmp port of the in/outbound SAs */
+		natt_type = (void *)mhp[SADB_X_EXT_NAT_T_TYPE];
+
+		if (natt_type && natt_type->sadb_x_nat_t_type_type) {
+			struct sadb_x_nat_t_port *natt_dport;
+			u_short dp, natt_dp;
+
+			natt_dport = (void *)mhp[SADB_X_EXT_NAT_T_DPORT];
+			dp = ((struct sockaddr_in *)dst0)->sin_port;
+			natt_dp = natt_dport->sadb_x_nat_t_port_port;
+			if (dp != natt_dp) {
+				msg = next;
+				continue;
+			}
+		}
+#endif /* ENABLE_NATT && SADB_X_EXT_NAT_T_TYPE */
+
 		for (i = 0; i < n; i++) {
 			plog(LLV_DEBUG, LOCATION, NULL,
 				"check spi(packet)=%u spi(db)=%u.\n",

-- 

Darrel