add M_ZEROIZE as an mbuf flag, so copied PFKEY messages (with embedded keys)

are cleared as well; from hshoexer@, feedback and ok bluhm@, ok claudio@

7 files changed, 48 insertions, 35 deletions

diff --git a/sys/kern/uipc_mbuf.c b/sys/kern/uipc_mbuf.c
index 5348f38a930..4ec2bed41f1 100644
--- a/sys/kern/uipc_mbuf.c
+++ b/sys/kern/uipc_mbuf.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uipc_mbuf.c,v 1.166 2012/04/13 09:38:32 deraadt Exp $	*/
+/*	$OpenBSD: uipc_mbuf.c,v 1.167 2012/09/26 14:53:23 markus Exp $	*/
 /*	$NetBSD: uipc_mbuf.c,v 1.15.4.1 1996/06/13 17:11:44 cgd Exp $	*/
 
 /*
@@ -127,6 +127,7 @@ void	m_cltick(void *);
 void	m_extfree(struct mbuf *);
 struct mbuf *m_copym0(struct mbuf *, int, int, int, int);
 void	nmbclust_update(void);
+void	m_zero(struct mbuf *);
 
 
 const char *mclpool_warnmsg =
@@ -467,11 +468,17 @@ m_free_unlocked(struct mbuf *m)
 	struct mbuf *n;
 
 	mbstat.m_mtypes[m->m_type]--;
+	n = m->m_next;
+	if (m->m_flags & M_ZEROIZE) {
+		m_zero(m);
+		/* propagate M_ZEROIZE to the next mbuf in the chain */
+		if (n)
+			n->m_flags |= M_ZEROIZE;
+	}
 	if (m->m_flags & M_PKTHDR)
 		m_tag_delete_chain(m);
 	if (m->m_flags & M_EXT)
 		m_extfree(m);
-	n = m->m_next;
 	pool_put(&mbpool, m);
 
 	return (n);
@@ -1183,6 +1190,11 @@ m_devget(char *buf, int totlen, int off, struct ifnet *ifp,
 		if (top != NULL) {
 			MGET(m, M_DONTWAIT, MT_DATA);
 			if (m == NULL) {
+				/*
+				 * As we might get called by pfkey, make sure
+				 * we do not leak sensitive data.
+				 */
+				top->m_flags |= M_ZEROIZE;
 				m_freem(top);
 				return (NULL);
 			}
@@ -1225,20 +1237,18 @@ m_devget(char *buf, int totlen, int off, struct ifnet *ifp,
 void
 m_zero(struct mbuf *m)
 {
-	while (m) {
 #ifdef DIAGNOSTIC
-		if (M_READONLY(m))
-			panic("m_zero: M_READONLY");
+	if (M_READONLY(m))
+		panic("m_zero: M_READONLY");
 #endif /* DIAGNOSTIC */
-		if (m->m_flags & M_EXT)
-			memset(m->m_ext.ext_buf, 0, m->m_ext.ext_size);
-		else {
-			if (m->m_flags & M_PKTHDR)
-				memset(m->m_pktdat, 0, MHLEN);
-			else
-				memset(m->m_dat, 0, MLEN);
-		}
-		m = m->m_next;
+
+	if (m->m_flags & M_EXT)
+		explicit_bzero(m->m_ext.ext_buf, m->m_ext.ext_size);
+	else {
+		if (m->m_flags & M_PKTHDR)
+			explicit_bzero(m->m_pktdat, MHLEN);
+		else
+			explicit_bzero(m->m_dat, MLEN);
 	}
 }
 
@@ -1339,7 +1349,7 @@ m_print(void *v, int (*pr)(const char *, ...))
 	(*pr)("m_type: %hi\tm_flags: %b\n", m->m_type, m->m_flags,
 	    "\20\1M_EXT\2M_PKTHDR\3M_EOR\4M_CLUSTER\5M_PROTO1\6M_VLANTAG"
 	    "\7M_LOOP\10M_FILDROP\11M_BCAST\12M_MCAST\13M_CONF\14M_AUTH"
-	    "\15M_TUNNEL\16M_AUTH_AH\17M_LINK0");
+	    "\15M_TUNNEL\16M_ZEROIZE\17M_LINK0");
 	(*pr)("m_next: %p\tm_nextpkt: %p\n", m->m_next, m->m_nextpkt);
 	(*pr)("m_data: %p\tm_len: %u\n", m->m_data, m->m_len);
 	(*pr)("m_dat: %p m_pktdat: %p\n", m->m_dat, m->m_pktdat);
diff --git a/sys/net/pfkeyv2.c b/sys/net/pfkeyv2.c
index a3f35a5c3c1..c5bb109eaac 100644
--- a/sys/net/pfkeyv2.c
+++ b/sys/net/pfkeyv2.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: pfkeyv2.c,v 1.126 2012/09/20 10:25:03 blambert Exp $ */
+/* $OpenBSD: pfkeyv2.c,v 1.127 2012/09/26 14:53:23 markus Exp $ */
 
 /*
  *	@(#)COPYRIGHT	1.1 (NRL) 17 January 1995
@@ -139,6 +139,10 @@ pfdatatopacket(void *data, int len, struct mbuf **packet)
 {
 	if (!(*packet = m_devget(data, len, 0, NULL, NULL)))
 		return (ENOMEM);
+
+	/* Make sure, all data gets zeroized on free */
+	(*packet)->m_flags |= M_ZEROIZE;
+
 	return (0);
 }
 
@@ -281,8 +285,6 @@ pfkeyv2_sendmessage(void **headers, int mode, struct socket *socket,
 			    (s->rdomain == rdomain))
 				pfkey_sendup(s->socket, packet, 1);
 
-		/* Done, let's be a bit paranoid */
-		m_zero(packet);
 		m_freem(packet);
 		break;
 
@@ -919,12 +921,10 @@ pfkeyv2_send(struct socket *socket, void *message, int len)
 				pfkey_sendup(so->socket, packet, 1);
 		}
 
-		/* Paranoid */
-		m_zero(packet);
 		m_freem(packet);
 
-		/* Even more paranoid */
-		bzero(freeme, sizeof(struct sadb_msg) + len);
+		/* Paranoid */
+		explicit_bzero(freeme, sizeof(struct sadb_msg) + len);
 		free(freeme, M_PFKEY);
 		freeme = NULL;
 	}
@@ -1080,6 +1080,7 @@ pfkeyv2_send(struct socket *socket, void *message, int len)
 			import_tap(newsa, headers[SADB_X_EXT_TAP]);
 #endif
 
+			/* Exclude sensitive data from reply message. */
 			headers[SADB_EXT_KEY_AUTH] = NULL;
 			headers[SADB_EXT_KEY_ENCRYPT] = NULL;
 			headers[SADB_X_EXT_LOCAL_AUTH] = NULL;
@@ -1249,6 +1250,7 @@ pfkeyv2_send(struct socket *socket, void *message, int len)
 			import_tap(newsa, headers[SADB_X_EXT_TAP]);
 #endif
 
+			/* Exclude sensitive data from reply message. */
 			headers[SADB_EXT_KEY_AUTH] = NULL;
 			headers[SADB_EXT_KEY_ENCRYPT] = NULL;
 			headers[SADB_X_EXT_LOCAL_AUTH] = NULL;
@@ -1871,6 +1873,7 @@ realret:
 	if (freeme)
 		free(freeme, M_PFKEY);
 
+	explicit_bzero(message, len);
 	free(message, M_PFKEY);
 
 	return (rval);
diff --git a/sys/netinet/ip_ah.c b/sys/netinet/ip_ah.c
index c0ebf3b3870..b60215e965b 100644
--- a/sys/netinet/ip_ah.c
+++ b/sys/netinet/ip_ah.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_ah.c,v 1.102 2012/09/20 10:25:03 blambert Exp $ */
+/*	$OpenBSD: ip_ah.c,v 1.103 2012/09/26 14:53:23 markus Exp $ */
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr) and
@@ -1007,7 +1007,7 @@ ah_output(struct mbuf *m, struct tdb *tdb, struct mbuf **mp, int skip,
 
 			hdr.af = tdb->tdb_dst.sa.sa_family;
 			hdr.spi = tdb->tdb_spi;
-			hdr.flags |= M_AUTH | M_AUTH_AH;
+			hdr.flags |= M_AUTH;
 
 			bpf_mtap_hdr(encif->if_bpf, (char *)&hdr,
 			    ENC_HDRLEN, m, BPF_DIRECTION_OUT);
diff --git a/sys/netinet/ip_ether.c b/sys/netinet/ip_ether.c
index 178196bde5d..2b178795f5e 100644
--- a/sys/netinet/ip_ether.c
+++ b/sys/netinet/ip_ether.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_ether.c,v 1.58 2011/07/04 20:42:15 dhill Exp $  */
+/*	$OpenBSD: ip_ether.c,v 1.59 2012/09/26 14:53:23 markus Exp $  */
 /*
  * The author of this code is Angelos D. Keromytis (kermit@adk.gr)
  *
@@ -247,7 +247,7 @@ etherip_decap(struct mbuf *m, int iphlen)
 	m_copydata(m, 0, sizeof(eh), (void *) &eh);
 
 	/* Reset the flags based on the inner packet */
-	m->m_flags &= ~(M_BCAST|M_MCAST|M_AUTH|M_CONF|M_AUTH_AH);
+	m->m_flags &= ~(M_BCAST|M_MCAST|M_AUTH|M_CONF);
 	if (eh.ether_dhost[0] & 1) {
 		if (bcmp((caddr_t) etherbroadcastaddr,
 		    (caddr_t)eh.ether_dhost, sizeof(etherbroadcastaddr)) == 0)
diff --git a/sys/netinet/ipsec_input.c b/sys/netinet/ipsec_input.c
index 0a9d9fba602..0a2b1da7597 100644
--- a/sys/netinet/ipsec_input.c
+++ b/sys/netinet/ipsec_input.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ipsec_input.c,v 1.107 2012/09/20 10:25:03 blambert Exp $	*/
+/*	$OpenBSD: ipsec_input.c,v 1.108 2012/09/26 14:53:23 markus Exp $	*/
 /*
  * The authors of this code are John Ioannidis (ji@tla.org),
  * Angelos D. Keromytis (kermit@csd.uch.gr) and
@@ -650,7 +650,7 @@ ipsec_common_input_cb(struct mbuf *m, struct tdb *tdbp, int skip, int protoff,
 		if (tdbp->tdb_authalgxform)
 			m->m_flags |= M_AUTH;
 	} else if (sproto == IPPROTO_AH) {
-		m->m_flags |= M_AUTH | M_AUTH_AH;
+		m->m_flags |= M_AUTH;
 	} else if (sproto == IPPROTO_IPCOMP) {
 		m->m_flags |= M_COMP;
 	}
@@ -674,7 +674,7 @@ ipsec_common_input_cb(struct mbuf *m, struct tdb *tdbp, int skip, int protoff,
 
 			hdr.af = af;
 			hdr.spi = tdbp->tdb_spi;
-			hdr.flags = m->m_flags & (M_AUTH|M_CONF|M_AUTH_AH);
+			hdr.flags = m->m_flags & (M_AUTH|M_CONF);
 
 			bpf_mtap_hdr(encif->if_bpf, (char *)&hdr,
 			    ENC_HDRLEN, m, BPF_DIRECTION_IN);
diff --git a/sys/netinet6/nd6_rtr.c b/sys/netinet6/nd6_rtr.c
index d8ce6e06ed5..32e8246e366 100644
--- a/sys/netinet6/nd6_rtr.c
+++ b/sys/netinet6/nd6_rtr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nd6_rtr.c,v 1.64 2012/09/07 09:55:18 stsp Exp $	*/
+/*	$OpenBSD: nd6_rtr.c,v 1.65 2012/09/26 14:53:23 markus Exp $	*/
 /*	$KAME: nd6_rtr.c,v 1.97 2001/02/07 11:09:13 itojun Exp $	*/
 
 /*
@@ -1055,7 +1055,7 @@ prelist_update(struct nd_prefix *new, struct nd_defrouter *dr, struct mbuf *m)
 		 * Authenticity for NA consists authentication for
 		 * both IP header and IP datagrams, doesn't it ?
 		 */
-		auth = ((m->m_flags & M_AUTH_AH) && (m->m_flags & M_AUTH));
+		auth = (m->m_flags & M_AUTH);
 	}
 
 	if ((pr = nd6_prefix_lookup(new)) != NULL) {
diff --git a/sys/sys/mbuf.h b/sys/sys/mbuf.h
index eabe4c5a53b..482491e4bfa 100644
--- a/sys/sys/mbuf.h
+++ b/sys/sys/mbuf.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: mbuf.h,v 1.157 2012/09/19 17:50:17 yasuoka Exp $	*/
+/*	$OpenBSD: mbuf.h,v 1.158 2012/09/26 14:53:23 markus Exp $	*/
 /*	$NetBSD: mbuf.h,v 1.19 1996/02/09 18:25:14 christos Exp $	*/
 
 /*
@@ -168,13 +168,14 @@ struct mbuf {
 #define M_CONF		0x0400  /* payload was encrypted (ESP-transport) */
 #define M_AUTH		0x0800  /* payload was authenticated (AH or ESP auth) */
 #define M_TUNNEL	0x1000  /* IP-in-IP added by tunnel mode IPsec */
-#define M_AUTH_AH	0x2000  /* header was authenticated (AH) */
+#define M_ZEROIZE	0x2000  /* Zeroize data part on free */
 #define M_COMP		0x4000  /* header was decompressed */
 #define M_LINK0		0x8000	/* link layer specific flag */
 
 /* flags copied when copying m_pkthdr */
 #define	M_COPYFLAGS	(M_PKTHDR|M_EOR|M_PROTO1|M_BCAST|M_MCAST|M_CONF|M_COMP|\
-			 M_AUTH|M_LOOP|M_TUNNEL|M_LINK0|M_VLANTAG|M_FILDROP)
+			 M_AUTH|M_LOOP|M_TUNNEL|M_LINK0|M_VLANTAG|M_FILDROP|\
+			 M_ZEROIZE)
 
 /* Checksumming flags */
 #define	M_IPV4_CSUM_OUT		0x0001	/* IPv4 checksum needed */
@@ -417,7 +418,6 @@ void	m_copydata(struct mbuf *, int, int, caddr_t);
 void	m_cat(struct mbuf *, struct mbuf *);
 struct mbuf *m_devget(char *, int, int, struct ifnet *,
 	    void (*)(const void *, void *, size_t));
-void	m_zero(struct mbuf *);
 int	m_apply(struct mbuf *, int, int,
 	    int (*)(caddr_t, caddr_t, unsigned int), caddr_t);
 int	m_dup_pkthdr(struct mbuf *, struct mbuf *, int);