5099921 in.iked pfkey.c: should pull memset into extract_exts()

6258318 need port selectors with wildcard protocol
6325408 sadb code cleanup: inbound/outbound symmetry in *_add_sa_finish()
6326584 comedy of mismerges puts a quarter-twist into quick mode identities
6331916 Identity-based DELETE on machine with no-identity SA can cause kernel panic
6333693 in.iked needs better handling of port-only selectors

8 files changed, 138 insertions, 50 deletions

diff --git a/usr/src/cmd/cmd-inet/usr.sbin/ipseckey.c b/usr/src/cmd/cmd-inet/usr.sbin/ipseckey.c
index fbdf95a53f..ce3332bf07 100644
--- a/usr/src/cmd/cmd-inet/usr.sbin/ipseckey.c
+++ b/usr/src/cmd/cmd-inet/usr.sbin/ipseckey.c
@@ -2140,16 +2140,6 @@ doaddresses(uint8_t sadb_msg_type, uint8_t sadb_msg_satype, int cmd,
 	 * First, fill in port numbers and protocol in extensions.
 	 */
 
-	if ((proto == 0) && ((srcport != 0) || (dstport != 0))) {
-		warnx(gettext("WARNING: ports without proto is nonsensical."));
-		/*
-		 * Don't worry about it, it just may make the SA not match
-		 * any outbound traffic, or it perhaps could be perverted
-		 * by the kernel to cover both TCP and UDP traffic on the
-		 * same port (e.g. DNS).
-		 */
-	}
-
 	if (src != NULL) {
 		src->sadb_address_proto = proto;
 		sin6 = (struct sockaddr_in6 *)(src + 1);
@@ -3345,6 +3335,13 @@ dodelget(int cmd, int satype, char *argv[])
 
 	thiscmd = (cmd == CMD_GET) ? "get" : "delete";
 
+#define	ALLOC_ADDR_EXT(ext, exttype)			\
+	(ext) = (struct sadb_address *)nextext;		\
+	nextext = (uint64_t *)((ext) + 1);		\
+	nextext += SADB_8TO64(roundup(sa_len, 8));	\
+	(ext)->sadb_address_exttype = exttype;		\
+	(ext)->sadb_address_len = nextext - ((uint64_t *)ext);
+
 	/* Assume last element in argv is set to NULL. */
 	do {
 		token = parseextval(*argv, &next);
@@ -3412,11 +3409,9 @@ dodelget(int cmd, int satype, char *argv[])
 			argv++;
 
 			unspec_src = B_FALSE;
-			src = (struct sadb_address *)nextext;
-			nextext = (uint64_t *)(src + 1);
-			nextext += SADB_8TO64(roundup(sa_len, 8));
-			src->sadb_address_exttype = SADB_EXT_ADDRESS_SRC;
-			src->sadb_address_len = nextext - ((uint64_t *)src);
+
+			ALLOC_ADDR_EXT(src, SADB_EXT_ADDRESS_SRC);
+
 			if (srchp == &dummy.he) {
 				/*
 				 * Single address with -n flag.
@@ -3440,11 +3435,8 @@ dodelget(int cmd, int satype, char *argv[])
 			    (token == TOK_SRCADDR6));
 			argv++;
 
-			dst = (struct sadb_address *)nextext;
-			nextext = (uint64_t *)(dst + 1);
-			nextext += SADB_8TO64(roundup(sa_len, 8));
-			dst->sadb_address_exttype = SADB_EXT_ADDRESS_DST;
-			dst->sadb_address_len = nextext - ((uint64_t *)dst);
+			ALLOC_ADDR_EXT(dst, SADB_EXT_ADDRESS_DST);
+
 			if (dsthp == &dummy.he) {
 				/*
 				 * Single address with -n flag.
@@ -3465,6 +3457,20 @@ dodelget(int cmd, int satype, char *argv[])
 		}
 	} while (token != TOK_EOF);
 
+	if ((srcport != 0) && (src == NULL)) {
+		ALLOC_ADDR_EXT(src, SADB_EXT_ADDRESS_SRC);
+		sin6 = (struct sockaddr_in6 *)(src + 1);
+		bzero(sin6, sizeof (*sin6));
+		sin6->sin6_family = AF_INET6;
+	}
+
+	if ((dstport != 0) && (dst == NULL)) {
+		ALLOC_ADDR_EXT(dst, SADB_EXT_ADDRESS_DST);
+		sin6 = (struct sockaddr_in6 *)(dst + 1);
+		bzero(sin6, sizeof (*sin6));
+		sin6->sin6_family = AF_INET6;
+	}
+
 	/* So I have enough of the message to send it down! */
 	msg->sadb_msg_len = nextext - get_buffer;
 
diff --git a/usr/src/uts/common/inet/ip/ip_sadb.c b/usr/src/uts/common/inet/ip/ip_sadb.c
index b73d8d51f7..176164bede 100644
--- a/usr/src/uts/common/inet/ip/ip_sadb.c
+++ b/usr/src/uts/common/inet/ip/ip_sadb.c
@@ -498,6 +498,9 @@ ipsec_getassocbyspi(isaf_t *bucket, uint32_t spi, uint32_t *src, uint32_t *dst,
 		 * There is a weakness here in that a packet with all-zeroes
 		 * for an address will match regardless of the source address
 		 * stored in the packet.
+		 *
+		 * Note that port-level packet selectors, if present,
+		 * are checked in ipsec_check_ipsecin_unique().
 		 */
 		if (IPSA_ARE_ADDR_EQUAL(src, retval->ipsa_srcaddr, af) ||
 		    IPSA_IS_ADDR_UNSPEC(retval->ipsa_srcaddr, af) ||
diff --git a/usr/src/uts/common/inet/ip/ipdrop.c b/usr/src/uts/common/inet/ip/ipdrop.c
index b6ca7559db..d86a4350fa 100644
--- a/usr/src/uts/common/inet/ip/ipdrop.c
+++ b/usr/src/uts/common/inet/ip/ipdrop.c
@@ -20,7 +20,7 @@
  * CDDL HEADER END
  */
 /*
- * Copyright 2004 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2005 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
@@ -110,6 +110,10 @@ ip_drop_init(void)
 	    KSTAT_DATA_UINT64);
 	kstat_named_init(&ipdrops_spd_esp_badid, "spd_esp_badid",
 	    KSTAT_DATA_UINT64);
+	kstat_named_init(&ipdrops_spd_ah_innermismatch,
+	    "spd_ah_innermismatch", KSTAT_DATA_UINT64);
+	kstat_named_init(&ipdrops_spd_esp_innermismatch,
+	    "spd_esp_innermismatch", KSTAT_DATA_UINT64);
 
 	/* ESP-specific drop statistics. */
 
diff --git a/usr/src/uts/common/inet/ip/ipsecah.c b/usr/src/uts/common/inet/ip/ipsecah.c
index 36233e4b4c..a50c65c780 100644
--- a/usr/src/uts/common/inet/ip/ipsecah.c
+++ b/usr/src/uts/common/inet/ip/ipsecah.c
@@ -840,7 +840,7 @@ inbound_task(void *arg)
 static int
 ah_add_sa_finish(mblk_t *mp, sadb_msg_t *samsg, keysock_in_t *ksi)
 {
-	isaf_t *primary, *secondary, *inbound;
+	isaf_t *primary, *secondary, *inbound, *outbound;
 	sadb_sa_t *assoc = (sadb_sa_t *)ksi->ks_in_extv[SADB_EXT_SA];
 	sadb_address_t *dstext =
 	    (sadb_address_t *)ksi->ks_in_extv[SADB_EXT_ADDRESS_DST];
@@ -876,6 +876,7 @@ ah_add_sa_finish(mblk_t *mp, sadb_msg_t *samsg, keysock_in_t *ksi)
 	}
 
 	inbound = INBOUND_BUCKET(sp, assoc->sadb_sa_spi);
+	outbound = &sp->sdb_of[outhash];
 
 	switch (ksi->ks_in_dsttype) {
 	case KS_IN_ADDR_MBCAST:
@@ -883,7 +884,7 @@ ah_add_sa_finish(mblk_t *mp, sadb_msg_t *samsg, keysock_in_t *ksi)
 		/* FALLTHRU */
 	case KS_IN_ADDR_ME:
 		primary = inbound;
-		secondary = &sp->sdb_of[outhash];
+		secondary = outbound;
 		/*
 		 * If the source address is either one of mine, or unspecified
 		 * (which is best summed up by saying "not 'not mine'"),
@@ -897,7 +898,7 @@ ah_add_sa_finish(mblk_t *mp, sadb_msg_t *samsg, keysock_in_t *ksi)
 		is_inbound = B_TRUE;
 		break;
 	case KS_IN_ADDR_NOTME:
-		primary = &sp->sdb_of[outhash];
+		primary = outbound;
 		secondary = inbound;
 		/*
 		 * If the source address literally not mine (either
diff --git a/usr/src/uts/common/inet/ip/ipsecesp.c b/usr/src/uts/common/inet/ip/ipsecesp.c
index 4b668c3866..df0752a174 100644
--- a/usr/src/uts/common/inet/ip/ipsecesp.c
+++ b/usr/src/uts/common/inet/ip/ipsecesp.c
@@ -2795,7 +2795,7 @@ inbound_task(void *arg)
 static int
 esp_add_sa_finish(mblk_t *mp, sadb_msg_t *samsg, keysock_in_t *ksi)
 {
-	isaf_t *primary, *secondary, *inbound;
+	isaf_t *primary, *secondary, *inbound, *outbound;
 	sadb_sa_t *assoc = (sadb_sa_t *)ksi->ks_in_extv[SADB_EXT_SA];
 	sadb_address_t *dstext =
 	    (sadb_address_t *)ksi->ks_in_extv[SADB_EXT_ADDRESS_DST];
@@ -2828,14 +2828,17 @@ esp_add_sa_finish(mblk_t *mp, sadb_msg_t *samsg, keysock_in_t *ksi)
 		dstaddr = (uint32_t *)(&dst6->sin6_addr);
 		outhash = OUTBOUND_HASH_V6(sp, *(in6_addr_t *)dstaddr);
 	}
+
 	inbound = INBOUND_BUCKET(sp, assoc->sadb_sa_spi);
+	outbound = &sp->sdb_of[outhash];
+
 	switch (ksi->ks_in_dsttype) {
 	case KS_IN_ADDR_MBCAST:
 		clone = B_TRUE;	/* All mcast SAs can be bidirectional */
 		/* FALLTHRU */
 	case KS_IN_ADDR_ME:
 		primary = inbound;
-		secondary = &sp->sdb_of[outhash];
+		secondary = outbound;
 		/*
 		 * If the source address is either one of mine, or unspecified
 		 * (which is best summed up by saying "not 'not mine'"),
@@ -2849,7 +2852,7 @@ esp_add_sa_finish(mblk_t *mp, sadb_msg_t *samsg, keysock_in_t *ksi)
 		is_inbound = B_TRUE;
 		break;
 	case KS_IN_ADDR_NOTME:
-		primary = &sp->sdb_of[outhash];
+		primary = outbound;
 		secondary = inbound;
 		/*
 		 * If the source address literally not mine (either
diff --git a/usr/src/uts/common/inet/ip/sadb.c b/usr/src/uts/common/inet/ip/sadb.c
index b0b9176c01..786c7d2c9e 100644
--- a/usr/src/uts/common/inet/ip/sadb.c
+++ b/usr/src/uts/common/inet/ip/sadb.c
@@ -2068,6 +2068,7 @@ sadb_srcaddrfix(keysock_in_t *ksi)
 	struct sockaddr_in *src;
 	struct sockaddr_in6 *dst;
 	sadb_address_t *srcext, *dstext;
+	uint16_t sport;
 
 	if (ksi->ks_in_srctype != KS_IN_ADDR_UNSPEC ||
 	    ksi->ks_in_dsttype == KS_IN_ADDR_NOTTHERE)
@@ -2086,9 +2087,14 @@ sadb_srcaddrfix(keysock_in_t *ksi)
 	    src->sin_family == AF_INET)
 		return;
 
-	/* Convert "src" to AF_INET INADDR_ANY. */
+	/*
+	 * Convert "src" to AF_INET INADDR_ANY.  We rely on sin_port being
+	 * in the same place for sockaddr_in and sockaddr_in6.
+	 */
+	sport = src->sin_port;
 	bzero(src, sizeof (*src));
 	src->sin_family = AF_INET;
+	src->sin_port = sport;
 }
 
 /*
@@ -2145,9 +2151,11 @@ sadb_purge_cb(isaf_t *head, ipsa_t *entry, void *cookie)
 	    (ps->dst != NULL &&
 		!IPSA_ARE_ADDR_EQUAL(entry->ipsa_dstaddr, ps->dst, ps->af)) ||
 	    (ps->didstr != NULL &&
+		(entry->ipsa_dst_cid != NULL) &&
 		!(ps->didtype == entry->ipsa_dst_cid->ipsid_type &&
 		    strcmp(ps->didstr, entry->ipsa_dst_cid->ipsid_cid) == 0)) ||
 	    (ps->sidstr != NULL &&
+		(entry->ipsa_src_cid != NULL) &&
 		!(ps->sidtype == entry->ipsa_src_cid->ipsid_type &&
 		    strcmp(ps->sidstr, entry->ipsa_src_cid->ipsid_cid) == 0)) ||
 	    (ps->kmproto <= SADB_X_KMP_MAX && ps->kmproto != entry->ipsa_kmp)) {
@@ -2418,13 +2426,10 @@ sadb_set_unique(ipsa_t *sa, uint8_t proto,
 	uint16_t srcport = src->sin_port;
 	uint16_t dstport = dst->sin_port;
 
-	if (proto != IPPROTO_TCP && proto != IPPROTO_UDP) {
-		srcport = dstport = 0;
-	}
-
 	sa->ipsa_unique_id = SA_UNIQUE_ID(srcport, dstport, proto);
 	sa->ipsa_unique_mask = SA_UNIQUE_MASK(srcport, dstport, proto);
-	sa->ipsa_flags |= IPSA_F_UNIQUE;
+	if (sa->ipsa_unique_mask != 0)
+		sa->ipsa_flags |= IPSA_F_UNIQUE;
 }
 
 
@@ -2685,8 +2690,7 @@ sadb_common_add(queue_t *ip_q, queue_t *pfkey_q, mblk_t *mp, sadb_msg_t *samsg,
 	(void) drv_getparm(TIME, &newbie->ipsa_addtime);
 
 	/* Set unique value */
-	if (dstext->sadb_address_proto != 0)
-		sadb_set_unique(newbie, dstext->sadb_address_proto, src, dst);
+	sadb_set_unique(newbie, dstext->sadb_address_proto, src, dst);
 
 	if (kmcext != NULL) {
 		newbie->ipsa_kmp = kmcext->sadb_x_kmc_proto;
@@ -3833,9 +3837,8 @@ sadb_update_sa(mblk_t *mp, keysock_in_t *ksi,
 	}
 
 	if (outbound_target != NULL) {
-		if (dstext->sadb_address_proto != 0)
-			sadb_set_unique(outbound_target,
-			    dstext->sadb_address_proto, src, dst);
+		sadb_set_unique(outbound_target, dstext->sadb_address_proto,
+		    src, dst);
 		sadb_update_lifetimes(outbound_target, hard, soft);
 		if (kmp != 0)
 			outbound_target->ipsa_kmp = kmp;
@@ -3844,9 +3847,8 @@ sadb_update_sa(mblk_t *mp, keysock_in_t *ksi,
 	}
 
 	if (inbound_target != NULL) {
-		if (dstext->sadb_address_proto != 0)
-			sadb_set_unique(inbound_target,
-			    dstext->sadb_address_proto, src, dst);
+		sadb_set_unique(inbound_target, dstext->sadb_address_proto,
+		    src, dst);
 		sadb_update_lifetimes(inbound_target, hard, soft);
 		if (kmp != 0)
 			inbound_target->ipsa_kmp = kmp;
diff --git a/usr/src/uts/common/inet/ip/spd.c b/usr/src/uts/common/inet/ip/spd.c
index 8826b42a66..256c634149 100644
--- a/usr/src/uts/common/inet/ip/spd.c
+++ b/usr/src/uts/common/inet/ip/spd.c
@@ -31,10 +31,6 @@
  *
  * This module maintains the SPD and provides routines used by ip and ip6
  * to apply IPsec policy to inbound and outbound datagrams.
- *
- * XXX TODO LIST
- * Inbound policy:
- * Put policy failure logging back in here (as policy action flag bit)
  */
 
 #include <sys/types.h>
@@ -158,6 +154,9 @@ uint8_t ipsec_sortlist[IPSEC_NALGTYPES][IPSEC_MAX_ALGS];
 ipsec_algs_exec_mode_t ipsec_algs_exec_mode[IPSEC_NALGTYPES];
 static crypto_notify_handle_t prov_update_handle = NULL;
 
+int ipsec_hdr_pullup_needed = 0;
+int ipsec_weird_null_inbound_policy = 0;
+
 #define	ALGBITS_ROUND_DOWN(x, align)	(((x)/(align))*(align))
 #define	ALGBITS_ROUND_UP(x, align)	ALGBITS_ROUND_DOWN((x)+(align)-1, align)
 
@@ -1301,6 +1300,58 @@ ipsec_check_loopback_policy(queue_t *q, mblk_t *first_mp,
 	return (first_mp);
 }
 
+/*
+ * Check that packet's inbound ports & proto match the selectors
+ * expected by the SAs it traversed on the way in.
+ */
+static boolean_t
+ipsec_check_ipsecin_unique(ipsec_in_t *ii, mblk_t *mp,
+    ipha_t *ipha, ip6_t *ip6h,
+    const char **reason, kstat_named_t **counter)
+{
+	uint64_t pkt_unique, ah_mask, esp_mask;
+	ipsa_t *ah_assoc = ii->ipsec_in_ah_sa;
+	ipsa_t *esp_assoc = ii->ipsec_in_esp_sa;
+	ipsec_selector_t sel;
+
+	ASSERT((ah_assoc != NULL) || (esp_assoc != NULL));
+
+	ah_mask = (ah_assoc != NULL) ? ah_assoc->ipsa_unique_mask : 0;
+	esp_mask = (esp_assoc != NULL) ? esp_assoc->ipsa_unique_mask : 0;
+
+	if ((ah_mask == 0) && (esp_mask == 0))
+		return (B_TRUE);
+
+	if (!ipsec_init_inbound_sel(&sel, mp, ipha, ip6h)) {
+		/*
+		 * Technically not a policy mismatch, but it is
+		 * an internal failure.
+		 */
+		*reason = "ipsec_init_inbound_sel";
+		*counter = &ipdrops_spd_nomem;
+		return (B_FALSE);
+	}
+
+	pkt_unique = SA_UNIQUE_ID(sel.ips_remote_port, sel.ips_local_port,
+	    sel.ips_protocol);
+
+	if (ah_mask != 0) {
+		if (ah_assoc->ipsa_unique_id != (pkt_unique & ah_mask)) {
+			*reason = "AH inner header mismatch";
+			*counter = &ipdrops_spd_ah_innermismatch;
+			return (B_FALSE);
+		}
+	}
+	if (esp_mask != 0) {
+		if (esp_assoc->ipsa_unique_id != (pkt_unique & esp_mask)) {
+			*reason = "ESP inner header mismatch";
+			*counter = &ipdrops_spd_esp_innermismatch;
+			return (B_FALSE);
+		}
+	}
+	return (B_TRUE);
+}
+
 static boolean_t
 ipsec_check_ipsecin_action(ipsec_in_t *ii, mblk_t *mp, ipsec_action_t *ap,
     ipha_t *ipha, ip6_t *ip6h, const char **reason, kstat_named_t **counter)
@@ -1502,6 +1553,9 @@ ipsec_check_ipsecin_latch(ipsec_in_t *ii, mblk_t *mp, ipsec_latch_t *ipl,
 		return (B_FALSE);
 	}
 
+	if (!ipsec_check_ipsecin_unique(ii, mp, ipha, ip6h, reason, counter))
+		return (B_FALSE);
+
 	return (ipsec_check_ipsecin_action(ii, mp, ipl->ipl_in_action,
 	    ipha, ip6h, reason, counter));
 }
@@ -1553,6 +1607,10 @@ ipsec_check_ipsecin_policy(queue_t *q, mblk_t *first_mp, ipsec_policy_t *ipsp,
 		goto drop;
 	}
 
+	if (!ipsec_check_ipsecin_unique(ii, data_mp, ipha, ip6h,
+	    &reason, &counter))
+		goto drop;
+
 	/*
 	 * Ok, now loop through the possible actions and see if any
 	 * of them work for us.
@@ -2085,7 +2143,7 @@ ipsec_latch_inbound(ipsec_latch_t *ipl, ipsec_in_t *ii)
  * inbound datagram; called from IP in numerous places.
  *
  * Note that this is not a chokepoint for inbound policy checks;
- * see also ipsec_check_ipsecin_latch()
+ * see also ipsec_check_ipsecin_latch() and ipsec_check_global_policy()
  */
 mblk_t *
 ipsec_check_inbound_policy(mblk_t *first_mp, conn_t *connp,
@@ -2176,14 +2234,16 @@ ipsec_check_inbound_policy(mblk_t *first_mp, conn_t *connp,
 
 	if (ipl == NULL) {
 		/*
-		 * We don't have policies cached in the conn's
+		 * We don't have policies cached in the conn
 		 * for this stream. So, look at the global
 		 * policy. It will check against conn or global
 		 * depending on whichever is stronger.
 		 */
 		return (ipsec_check_global_policy(first_mp, connp,
 		    ipha, ip6h, mctl_present));
-	} else if (ipl->ipl_in_action != NULL) {
+	}
+
+	if (ipl->ipl_in_action != NULL) {
 		/* Policy is cached & latched; fast(er) path */
 		const char *reason;
 		kstat_named_t *counter;
@@ -2200,8 +2260,10 @@ ipsec_check_inbound_policy(mblk_t *first_mp, conn_t *connp,
 		    &spd_dropper);
 		BUMP_MIB(&ip_mib, ipsecInFailed);
 		return (NULL);
-	} else if (ipl->ipl_in_policy == NULL)
+	} else if (ipl->ipl_in_policy == NULL) {
+		ipsec_weird_null_inbound_policy++;
 		return (first_mp);
+	}
 
 	IPPOL_REFHOLD(ipl->ipl_in_policy);
 	first_mp = ipsec_check_ipsecin_policy(CONNP_TO_WQ(connp), first_mp,
@@ -2284,6 +2346,7 @@ ipsec_init_inbound_sel(ipsec_selector_t *sel, mblk_t *mp,
 		 * apart from IP or options?  If so, perhaps we should revisit
 		 * the spare_mp strategy.
 		 */
+		ipsec_hdr_pullup_needed++;
 		if (spare_mp == NULL &&
 		    (spare_mp = msgpullup(mp, -1)) == NULL) {
 			return (B_FALSE);
diff --git a/usr/src/uts/common/inet/ipdrop.h b/usr/src/uts/common/inet/ipdrop.h
index 386c6a93cb..492b5213ed 100644
--- a/usr/src/uts/common/inet/ipdrop.h
+++ b/usr/src/uts/common/inet/ipdrop.h
@@ -20,7 +20,7 @@
  * CDDL HEADER END
  */
 /*
- * Copyright 2004 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2005 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
@@ -90,6 +90,8 @@ struct ip_dropstats {
 	kstat_named_t ipds_spd_nomem;
 	kstat_named_t ipds_spd_ah_badid;
 	kstat_named_t ipds_spd_esp_badid;
+	kstat_named_t ipds_spd_ah_innermismatch;
+	kstat_named_t ipds_spd_esp_innermismatch;
 
 	/* ESP-specific drop statistics. */
 	kstat_named_t ipds_esp_nomem;
@@ -152,6 +154,10 @@ struct ip_dropstats {
 #define	ipdrops_spd_nomem		ip_drop_types->ipds_spd_nomem
 #define	ipdrops_spd_ah_badid		ip_drop_types->ipds_spd_ah_badid
 #define	ipdrops_spd_esp_badid		ip_drop_types->ipds_spd_esp_badid
+#define	ipdrops_spd_ah_innermismatch	\
+				ip_drop_types->ipds_spd_ah_innermismatch
+#define	ipdrops_spd_esp_innermismatch	\
+				ip_drop_types->ipds_spd_esp_innermismatch
 
 /* ESP-specific drop statistics. */
 #define	ipdrops_esp_nomem		ip_drop_types->ipds_esp_nomem