[src/netbsd-7-0]: src/sys Pull up following revision(s) (requested by maxv in...

[martin <martin%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/2bb62d5a6458
branches:  netbsd-7-0
changeset: 801475:2bb62d5a6458
user:      martin <martin%NetBSD.org@localhost>
date:      Thu Apr 05 11:53:02 2018 +0000

description:
Pull up following revision(s) (requested by maxv in ticket #1594):

        sys/kern/uipc_mbuf.c: revision 1.182
        sys/netinet6/frag6.c: revision 1.67
        sys/netinet/ip_reass.c: revision 1.14
        sys/sys/mbuf.h: revision 1.179

Remove M_PKTHDR from secondary mbufs when reassembling packets.

This is a real problem, because I found at least one component that relies
on the fact that only the first mbuf has M_PKTHDR: far from here, in
m_splithdr, we don't update m->m_pkthdr.len if M_PKTHDR is found in a
secondary mbuf. (The initial intention there was to avoid updating
m_pkthdr.len twice, the assumption was that if M_PKTHDR is set then we're
dealing with the first mbuf.) Therefore, when handling fragmented IPsec
packets (in particular IPv6, IPv4 is a bit more complicated), we may end
up with an incorrect m_pkthdr.len after authentication or decryption. In
the case of ESP, this can lead to a remote crash on this instruction:
        m_copydata(m, m->m_pkthdr.len - 3, 3, lastthree);
m_pkthdr.len is bigger than the actual mbuf chain.

It seems possible to me to trigger this bug even if you don't have the ESP
key, because the fragmentation part is outside of the encrypted ESP
payload.

So if you MITM the target, and intercept an incoming ESP packet (which you
can't decrypt), you should be able to forge a new specially-crafted,
fragmented packet and stuff the ESP payload (still encrypted, as you
intercepted it) into it. The decryption succeeds and the target crashes.

diffstat:

 sys/kern/uipc_mbuf.c   |  14 ++++++++++++--
 sys/netinet/ip_reass.c |   8 +++++---
 sys/netinet6/frag6.c   |  11 +++++------
 sys/sys/mbuf.h         |   4 +++-
 4 files changed, 25 insertions(+), 12 deletions(-)

diffs (130 lines):

diff -r 4269890c5c08 -r 2bb62d5a6458 sys/kern/uipc_mbuf.c
--- a/sys/kern/uipc_mbuf.c      Thu Apr 05 11:44:57 2018 +0000
+++ b/sys/kern/uipc_mbuf.c      Thu Apr 05 11:53:02 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: uipc_mbuf.c,v 1.158.4.1 2015/02/09 09:46:01 martin Exp $       */
+/*     $NetBSD: uipc_mbuf.c,v 1.158.4.1.2.1 2018/04/05 11:53:02 martin Exp $   */
 
 /*-
  * Copyright (c) 1999, 2001 The NetBSD Foundation, Inc.
@@ -62,7 +62,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uipc_mbuf.c,v 1.158.4.1 2015/02/09 09:46:01 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uipc_mbuf.c,v 1.158.4.1.2.1 2018/04/05 11:53:02 martin Exp $");
 
 #include "opt_mbuftrace.h"
 #include "opt_nmbclusters.h"
@@ -453,6 +453,16 @@
        return (0);
 }
 
+void
+m_pkthdr_remove(struct mbuf *m)
+{
+       KASSERT(m->m_flags & M_PKTHDR);
+
+       m_tag_delete_chain(m, NULL);
+       m->m_flags &= ~M_PKTHDR;
+       memset(&m->m_pkthdr, 0, sizeof(m->m_pkthdr));
+}
+
 /*
  * Add mbuf to the end of a chain
  */
diff -r 4269890c5c08 -r 2bb62d5a6458 sys/netinet/ip_reass.c
--- a/sys/netinet/ip_reass.c    Thu Apr 05 11:44:57 2018 +0000
+++ b/sys/netinet/ip_reass.c    Thu Apr 05 11:53:02 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ip_reass.c,v 1.9 2014/02/25 18:30:12 pooka Exp $       */
+/*     $NetBSD: ip_reass.c,v 1.9.8.1 2018/04/05 11:53:02 martin Exp $  */
 
 /*
  * Copyright (c) 1982, 1986, 1988, 1993
@@ -46,7 +46,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ip_reass.c,v 1.9 2014/02/25 18:30:12 pooka Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ip_reass.c,v 1.9.8.1 2018/04/05 11:53:02 martin Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -393,6 +393,7 @@
                t = q->ipqe_m;
                nq = TAILQ_NEXT(q, ipqe_q);
                pool_cache_put(ipfren_cache, q);
+               m_pkthdr_remove(t);
                m_cat(m, t);
        }
 
@@ -410,7 +411,8 @@
        m->m_data -= (ip->ip_hl << 2);
 
        /* Fix up mbuf.  XXX This should be done elsewhere. */
-       if (m->m_flags & M_PKTHDR) {
+       {
+               KASSERT(m->m_flags & M_PKTHDR);
                int plen = 0;
                for (t = m; t; t = t->m_next) {
                        plen += t->m_len;
diff -r 4269890c5c08 -r 2bb62d5a6458 sys/netinet6/frag6.c
--- a/sys/netinet6/frag6.c      Thu Apr 05 11:44:57 2018 +0000
+++ b/sys/netinet6/frag6.c      Thu Apr 05 11:53:02 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: frag6.c,v 1.55.6.1 2018/01/30 18:31:53 martin Exp $    */
+/*     $NetBSD: frag6.c,v 1.55.6.2 2018/04/05 11:53:02 martin Exp $    */
 /*     $KAME: frag6.c,v 1.40 2002/05/27 21:40:31 itojun Exp $  */
 
 /*
@@ -31,7 +31,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: frag6.c,v 1.55.6.1 2018/01/30 18:31:53 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: frag6.c,v 1.55.6.2 2018/04/05 11:53:02 martin Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -405,6 +405,7 @@
                        t = t->m_next;
                t->m_next = IP6_REASS_MBUF(af6);
                m_adj(t->m_next, af6->ip6af_offset);
+               m_pkthdr_remove(t->m_next);
                kmem_intr_free(af6, sizeof(struct ip6asfrag));
                af6 = af6dwn;
        }
@@ -446,12 +447,10 @@
        kmem_intr_free(q6, sizeof(struct ip6q));
        frag6_nfragpackets--;
 
-       if (m->m_flags & M_PKTHDR) { /* Isn't it always true? */
+       {
+               KASSERT(m->m_flags & M_PKTHDR);
                int plen = 0;
                for (t = m; t; t = t->m_next) {
-                       /*
-                        * XXX XXX Why don't we remove M_PKTHDR?
-                        */
                        plen += t->m_len;
                }
                m->m_pkthdr.len = plen;
diff -r 4269890c5c08 -r 2bb62d5a6458 sys/sys/mbuf.h
--- a/sys/sys/mbuf.h    Thu Apr 05 11:44:57 2018 +0000
+++ b/sys/sys/mbuf.h    Thu Apr 05 11:53:02 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: mbuf.h,v 1.155 2014/05/17 23:27:59 rmind Exp $ */
+/*     $NetBSD: mbuf.h,v 1.155.4.1 2018/04/05 11:53:03 martin Exp $    */
 
 /*-
  * Copyright (c) 1996, 1997, 1999, 2001, 2007 The NetBSD Foundation, Inc.
@@ -819,6 +819,8 @@
 MALLOC_DECLARE(M_MBUF);
 MALLOC_DECLARE(M_SONAME);
 
+void   m_pkthdr_remove(struct mbuf *);
+
 struct mbuf *m_copym(struct mbuf *, int, int, int);
 struct mbuf *m_copypacket(struct mbuf *, int);
 struct mbuf *m_devget(char *, int, int, struct ifnet *,