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

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

Matthew Grooms wrote:

<snip>

> If you don't mind me asking, how does this cause a consistency problem
> when interfacing with the NetBSD kernel? Can ipsec-tools not rely on a
> set of NATT port extensions being returned as part of an SA for some
> platforms that also support NATT?

Here is what I found checking out the kernel code for the pfkey socket on
NetBSD and Linux (please keep in mind that this was not an exhaustive
analysis).

For SADB_ADDs and SADB_UPDATEs, the NetBSD kernel is taking the NATT port
information from the SADB_X_EXT_NAT_T_*PORT blobs and storing it in the
src and dst sockaddr port fields.  For SADB_DUMPs and SADB_GETSs, the NATT
port info from the SAa src and dst sockaddrs is returned via the
SADB_X_EXT_NAT_T_*PORT, in addition to still being in the sockaddrs.

On the other hand, Linux has a different place to store the NATT port
information.  It never lives in the SAa sockaddrs, this never gets returned
via that mechanism.  The NATT port info is handled only by the
SADB_X_EXT_NAT_T_*PORT blobs.

The ipsec-tools code seems to be expecting that the NATT port info is
contained within the src and dst addrs.

> 
>>>For what its worth, I think your patch contains the correct approach. 
>>
>>I had a very quick look at it. It will fix the Linux issue, but will
>>break again some configurations on Net/FreeBSD.
>>
> 
> 
> I should look at that patch again after its applied.

I think I have a better one now, read on :)

>>>I'm just wondering if this couldn't be solved at the libipsec level so 
>>>we don't have to include multiple special cases throughout racoon. I'm 
>>>sure Manu and Yvan will chime in as they have quite a bit more NATT 
>>>knowhow than I do.
>>
>>We will always need a CMPSADDR macro, because in racoon, if we support
>>multiple peers behind the same IP, the ports info is stored in the
>>addess structs.
>>
>>But at pfkey level, some cleanup is needed !
>>
> 
> 
> In my head, the suggestion was that the next lower pfkey API call could
> be possibly modified to return the data in a consistent fashion so that
> special casing the use of CMPSADDR could be avoided. I apparently don't
> understand the problem completely ( or the scope of libipsec ) :)

That sure seemed like a good idea to me as well.  The following patch get as
close as it can to the actual SA dump as it can in racoon and modifies the port
information there to meet the assumption in the rest of the code.  I'm sure
that there is cleaner way to do this, but I am not wanting to change to much of
the code at this point to get things working in my environment.  I'd appreciate
any feedback on this approach (and thanks again for the help so far).
It seems to be doing the trick for me here.  Note that this only "fixes" racoon,
I have not modified the code that setkey uses to display the SAs information.

--

Darrel


diff --git a/src/racoon/pfkey.c b/src/racoon/pfkey.c
index d775aa4..b3a003b 100644
--- a/src/racoon/pfkey.c
+++ b/src/racoon/pfkey.c
@@ -326,6 +326,54 @@ pfkey_dump_sadb(satype)
 				"failed to reallocate buffer to dump.\n");
 			goto fail;
 		}
+
+#if defined(__linux__) && defined(SADB_X_EXT_NAT_T_TYPE)
+		/*
+		 * NetBSD returns the NAT-T ports in the src and dst sockaddrs
+		 * in addition to the SADB_X_EXT_NAT_T_*PORT structs.
+		 *
+		 * Linux only returns them in the SADB_X_EXT_NAT_T_*PORT
+		 * structs.  The racoon codebase is making the assumption that
+		 * the NAT-T ports are reflected by the ports in the src and
+		 * dst sockaddrs.  We stick that information into those structs
+		 * here to meet the assumptions elsewhere.
+		 */
+		{
+		caddr_t mhp[SADB_EXT_MAX + 1];
+		struct sadb_sa *sa;
+		struct sockaddr *src, *dst;
+		struct sadb_x_nat_t_type *natt_type;
+		struct sadb_x_nat_t_port *natt_port;
+
+		if (pfkey_align(msg, mhp) || pfkey_check(mhp)) {
+			plog(LLV_ERROR, LOCATION, NULL,
+				"pfkey_check (%s)\n", ipsec_strerror());
+			break;
+		}
+
+		sa = (struct sadb_sa *)(mhp[SADB_EXT_SA]);
+		if (!sa ||
+		    !mhp[SADB_EXT_ADDRESS_SRC] ||
+		    !mhp[SADB_EXT_ADDRESS_DST]) {
+			break;
+		}
+
+		src = PFKEY_ADDR_SADDR(mhp[SADB_EXT_ADDRESS_SRC]);
+		dst = PFKEY_ADDR_SADDR(mhp[SADB_EXT_ADDRESS_DST]);
+
+		natt_type = (struct sadb_x_nat_t_type *)(mhp[SADB_X_EXT_NAT_T_TYPE]);
+
+		if (natt_type && natt_type->sadb_x_nat_t_type_type) {
+			/* fake up the src port */
+			natt_port = (struct sadb_x_nat_t_port *)(mhp[SADB_X_EXT_NAT_T_SPORT]);
+			((struct sockaddr_in *)src)->sin_port = natt_port->sadb_x_nat_t_port_port;
+			/* fake up the dst port */
+			natt_port = (struct sadb_x_nat_t_port *)(mhp[SADB_X_EXT_NAT_T_DPORT]);
+			((struct sockaddr_in *)dst)->sin_port = natt_port->sadb_x_nat_t_port_port;
+		}
+		}
+#endif /* __linux__ && SADB_X_EXT_NAT_T_TYPE */
+
 		memcpy(buf->v + bl, msg, ml);
 
 		if (msg->sadb_msg_seq == 0)