Re: IKE

[Gary Allan <dragonfly@xxxxxxxxxxxx>]

Francis GUDIN wrote:
> On 2008-03-02, Matthew Dillon <dillon@apollo.backplane.com> wrote:
> >     Hmm.  I know very little about IKE but one thing I do know is that
> >     we don't really have any developers who would likely want to maintain
> >     it in BASE.  If it is well developed in pkgsrc then pkgsrc may be
> >     the only way to go, which probably means adapting our kernel APIs to
> >     fit the utilities in pkgsrc rather then the other way around.
>
> I understand, and in fact, I already gave up on isakmpd and started
> working on racoon (security/ipsec-tools): I have a set of patches and the
> only thing that left is radius support.
> I need to rebuild one of my machines' kernel in order to have a test setup,
> then I could push them to its maintainer for inclusion in pkgsrc.
> By the way, there already was a patchset at sf.net for DragonFly support:
>  http://tinyurl.com/ysgmjh
> Too bad I didn't find it before; it would have save me a few hours.
>
> Regards,

Hello,

I've looked at this in the past and experienced kernel panics with 
IPSEC. I think I traced it back to endian issues in ip_lengthcheck() but 
other users were unable to replicate the problem. If you come across the 
same issue the attached patch may help. I'm not sure if it will still 
cleanly apply but I was able to get DragonFly<->WinXP and FreeBSD IPSEC 
working with it.

Regards

Gary Allan

Index: netinet/ip_demux.c
===================================================================
RCS file: /home/dcvs/src/sys/netinet/ip_demux.c,v
retrieving revision 1.33
diff -u -r1.33 ip_demux.c
--- netinet/ip_demux.c	23 Mar 2005 07:47:40 -0000	1.33
+++ netinet/ip_demux.c	3 May 2006 20:21:01 -0000
@@ -98,6 +98,134 @@
 #endif
 }
 
+/* 
+ * Called from esp4_input and ah4_input
+ * Gotchas:
+ *   ip fields are in host byte-order and not network byte-order.
+ *   ip->ip_len does not include the length of the ip header.
+ */
+boolean_t
+ip_lengthcheck_ipsec(struct mbuf **mp)
+{
+	struct mbuf *m = *mp;
+	struct ip *ip;
+	int iphlen, iplen;
+	struct tcphdr *th;
+	int thoff;				/* TCP data offset */
+
+	/* The packet must be at least the size of an IP header. */
+	if (m->m_pkthdr.len < sizeof(struct ip)) {
+		ipstat.ips_tooshort++;
+		m_free(m);
+		return FALSE;
+	}
+
+	/* The fixed IP header must reside completely in the first mbuf. */
+	if (m->m_len < sizeof(struct ip)) {
+		m = m_pullup(m, sizeof(struct ip));
+		if (m == NULL) {
+			ipstat.ips_toosmall++;
+			return FALSE;
+		}
+	}
+
+	ip = mtod(m, struct ip *);
+
+	/* Bound check the packet's stated IP header length. */
+	iphlen = ip->ip_hl << 2;
+	if (iphlen < sizeof(struct ip)) {	/* minimum header length */
+		ipstat.ips_badhlen++;
+		m_free(m);
+		return FALSE;
+	}
+
+	/* The full IP header must reside completely in the one mbuf. */
+	if (m->m_len < iphlen) {
+		m = m_pullup(m, iphlen);
+		if (m == NULL) {
+			ipstat.ips_badhlen++;
+			return FALSE;
+		}
+		ip = mtod(m, struct ip *);
+	}
+
+	iplen = ip->ip_len + iphlen;
+
+	/*
+	 * Fragments other than the first fragment don't have much
+	 * length information.
+	 */
+	if (ip->ip_off & IP_OFFMASK)
+		goto ipcheckonly;
+
+	/*
+	 * The TCP/IP or UDP/IP header must be entirely contained within
+	 * the first fragment of a packet.  Packet filters will break if they
+	 * aren't.
+	 *
+	 * Since the packet will be trimmed to ip_len we must also make sure
+	 * the potentially trimmed down length is still sufficient to hold
+	 * the header(s).
+	 */
+	switch (ip->ip_p) {
+	case IPPROTO_TCP:
+		if (iplen < iphlen + sizeof(struct tcphdr)) {
+			++tcpstat.tcps_rcvshort;
+			m_free(m);
+			return FALSE;
+		}
+		if (m->m_len < iphlen + sizeof(struct tcphdr)) {
+			m = m_pullup(m, iphlen + sizeof(struct tcphdr));
+			if (m == NULL) {
+				tcpstat.tcps_rcvshort++;
+				return FALSE;
+			}
+			ip = mtod(m, struct ip *);
+		}
+		th = (struct tcphdr *)((caddr_t)ip + iphlen);
+		thoff = th->th_off << 2;
+		if (thoff < sizeof(struct tcphdr) ||
+		    thoff + iphlen > iplen) {
+			tcpstat.tcps_rcvbadoff++;
+			m_free(m);
+			return FALSE;
+		}
+		if (m->m_len < iphlen + thoff) {
+			m = m_pullup(m, iphlen + thoff);
+			if (m == NULL) {
+				tcpstat.tcps_rcvshort++;
+				return FALSE;
+			}
+		}
+		break;
+	case IPPROTO_UDP:
+		if (iplen < iphlen + sizeof(struct udphdr)) {
+			++udpstat.udps_hdrops;
+			m_free(m);
+			return FALSE;
+		}
+		if (m->m_len < iphlen + sizeof(struct udphdr)) {
+			m = m_pullup(m, iphlen + sizeof(struct udphdr));
+			if (m == NULL) {
+				udpstat.udps_hdrops++;
+				return FALSE;
+			}
+		}
+		break;
+	default:
+ipcheckonly:
+		if (iplen < iphlen) {
+			++ipstat.ips_badlen;
+			m_free(m);
+			return FALSE;
+		}
+		break;
+	}
+
+	*mp = m;
+	return TRUE;
+}
+
 boolean_t
 ip_lengthcheck(struct mbuf **mp)
 {
Index: netinet/ip_var.h
===================================================================
RCS file: /home/dcvs/src/sys/netinet/ip_var.h,v
retrieving revision 1.18
diff -u -r1.18 ip_var.h
--- netinet/ip_var.h	31 Jan 2006 19:05:40 -0000	1.18
+++ netinet/ip_var.h	3 May 2006 20:20:39 -0000
@@ -181,6 +181,8 @@
 struct lwkt_port *
 	 ip_mport(struct mbuf **);
 boolean_t
+	 ip_lengthcheck_ipsec(struct mbuf **);
+boolean_t
 	 ip_lengthcheck(struct mbuf **);
 int	 ip_output(struct mbuf *,
 	    struct mbuf *, struct route *, int, struct ip_moptions *,
Index: netinet6/ah_input.c
===================================================================
RCS file: /home/dcvs/src/sys/netinet6/ah_input.c,v
retrieving revision 1.11
diff -u -r1.11 ah_input.c
--- netinet6/ah_input.c	1 May 2006 16:25:41 -0000	1.11
+++ netinet6/ah_input.c	3 May 2006 20:19:44 -0000
@@ -536,7 +536,7 @@
 				ipsecstat.in_polvio++;
 				goto fail;
 			}
-			if (!ip_lengthcheck(&m)) {
+			if (!ip_lengthcheck_ipsec(&m)) {
 				m = NULL;	/* freed in ip_lengthcheck() */
 				goto fail;
 			}
Index: netinet6/esp_input.c
===================================================================
RCS file: /home/dcvs/src/sys/netinet6/esp_input.c,v
retrieving revision 1.11
diff -u -r1.11 esp_input.c
--- netinet6/esp_input.c	1 May 2006 16:26:54 -0000	1.11
+++ netinet6/esp_input.c	3 May 2006 20:20:00 -0000
@@ -434,7 +434,7 @@
 				ipsecstat.in_polvio++;
 				goto bad;
 			}
-			if (!ip_lengthcheck(&m)) {
+			if (!ip_lengthcheck_ipsec(&m)) {
 				m = NULL;	/* freed in ip_lengthcheck() */
 				goto bad;
 			}