Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/netbsd-8]: src/sys/netipsec Pull up following revision(s) (requested by ...



details:   https://anonhg.NetBSD.org/src/rev/62da2f279ea5
branches:  netbsd-8
changeset: 434640:62da2f279ea5
user:      martin <martin%NetBSD.org@localhost>
date:      Thu Feb 15 14:28:38 2018 +0000

description:
Pull up following revision(s) (requested by maxv in ticket #551):
        sys/netipsec/xform_ipip.c: revision 1.56-1.63

Fix use-after-free. There is a path where the mbuf gets pulled up without
a proper mtod afterwards:

218     ipo = mtod(m, struct ip *);
281     m = m_pullup(m, hlen);
232     ipo->ip_src.s_addr

Found by Mootja.

Meanwhile it seems to me that 'ipo' should be set to NULL if the inner
packet is IPv6, but I'll revisit that later.
As I said in my last commit in this file, ipo should be set to NULL;
otherwise the 'local address spoofing' check below is always wrong on
IPv6.

Style and remove dead code.

dedup

Fix the IPIP_STAT_IBYTES stats; we did m_adj(m, iphlen) which substracted
iphlen, so no need to re-substract it again.

Remove broken MROUTING code, rename ipo->ip4, and simplify.

diffstat:

 sys/netipsec/xform_ipip.c |  244 +++++++++++++--------------------------------
 1 files changed, 71 insertions(+), 173 deletions(-)

diffs (truncated from 487 to 300 lines):

diff -r 4112af675339 -r 62da2f279ea5 sys/netipsec/xform_ipip.c
--- a/sys/netipsec/xform_ipip.c Thu Feb 15 08:27:24 2018 +0000
+++ b/sys/netipsec/xform_ipip.c Thu Feb 15 14:28:38 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: xform_ipip.c,v 1.49.2.2 2017/12/10 09:41:32 snj Exp $  */
+/*     $NetBSD: xform_ipip.c,v 1.49.2.3 2018/02/15 14:28:38 martin Exp $       */
 /*     $FreeBSD: src/sys/netipsec/xform_ipip.c,v 1.3.2.1 2003/01/24 05:11:36 sam Exp $ */
 /*     $OpenBSD: ip_ipip.c,v 1.25 2002/06/10 18:04:55 itojun Exp $ */
 
@@ -39,7 +39,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: xform_ipip.c,v 1.49.2.2 2017/12/10 09:41:32 snj Exp $");
+__KERNEL_RCSID(0, "$NetBSD: xform_ipip.c,v 1.49.2.3 2018/02/15 14:28:38 martin Exp $");
 
 /*
  * IP-inside-IP processing
@@ -74,10 +74,6 @@
 
 #include <netipsec/ipip_var.h>
 
-#ifdef MROUTING
-#include <netinet/ip_mroute.h>
-#endif
-
 #ifdef INET6
 #include <netinet/ip6.h>
 #include <netipsec/ipsec6.h>
@@ -88,84 +84,41 @@
 #include <netipsec/key.h>
 #include <netipsec/key_debug.h>
 
-typedef void   pr_in_input_t (struct mbuf *m, ...);
+/* XXX IPCOMP */
+#define        M_IPSEC (M_AUTHIPHDR|M_AUTHIPDGM|M_DECRYPTED)
 
-/*
- * We can control the acceptance of IP4 packets by altering the sysctl
- * net.inet.ipip.allow value.  Zero means drop them, all else is acceptance.
- */
-int    ipip_allow = 0;
+typedef void pr_in_input_t(struct mbuf *m, ...);
 
+int ipip_allow = 0;
 percpu_t *ipipstat_percpu;
 
-#ifdef SYSCTL_DECL
-SYSCTL_DECL(_net_inet_ipip);
-
-SYSCTL_INT(_net_inet_ipip, OID_AUTO,
-       ipip_allow,     CTLFLAG_RW,     &ipip_allow,    0, "");
-SYSCTL_STRUCT(_net_inet_ipip, IPSECCTL_STATS,
-       stats,          CTLFLAG_RD,     &ipipstat,      ipipstat, "");
-
-#endif
-
 void ipe4_attach(void);
 
-
-/* XXX IPCOMP */
-#define        M_IPSEC (M_AUTHIPHDR|M_AUTHIPDGM|M_DECRYPTED)
-
 static void _ipip_input(struct mbuf *m, int iphlen, struct ifnet *gifp);
 
 #ifdef INET6
-/*
- * Really only a wrapper for ipip_input(), for use with IPv6.
- */
 int
 ip4_input6(struct mbuf **m, int *offp, int proto, void *eparg __unused)
 {
-#if 0
-       /* If we do not accept IP-in-IP explicitly, drop.  */
-       if (!ipip_allow && ((*m)->m_flags & M_IPSEC) == 0) {
-               DPRINTF(("%s: dropped due to policy\n", __func__));
-               IPIP_STATINC(IPIP_STAT_PDROPS);
-               m_freem(*m);
-               return IPPROTO_DONE;
-       }
-#endif
        _ipip_input(*m, *offp, NULL);
        return IPPROTO_DONE;
 }
-#endif /* INET6 */
+#endif
 
 #ifdef INET
-/*
- * Really only a wrapper for ipip_input(), for use with IPv4.
- */
 void
 ip4_input(struct mbuf *m, int off, int proto, void *eparg __unused)
 {
-
-#if 0
-       /* If we do not accept IP-in-IP explicitly, drop.  */
-       if (!ipip_allow && (m->m_flags & M_IPSEC) == 0) {
-               DPRINTF(("%s: dropped due to policy\n", __func__));
-               IPIP_STATINC(IPIP_STAT_PDROPS);
-               m_freem(m);
-               return;
-       }
-#endif
-
        _ipip_input(m, off, NULL);
 }
-#endif /* INET */
+#endif
 
 /*
  * ipip_input gets called when we receive an IP{46} encapsulated packet,
  * either because we got it at a real interface, or because AH or ESP
  * were being used in tunnel mode (in which case the rcvif element will
- * contain the address of the encX interface associated with the tunnel.
+ * contain the address of the encX interface associated with the tunnel).
  */
-
 static void
 _ipip_input(struct mbuf *m, int iphlen, struct ifnet *gifp)
 {
@@ -173,7 +126,7 @@
        register struct ifnet *ifp;
        register struct ifaddr *ifa;
        pktqueue_t *pktq = NULL;
-       struct ip *ipo;
+       struct ip *ip4 = NULL;
 #ifdef INET6
        register struct sockaddr_in6 *sin6;
        struct ip6_hdr *ip6 = NULL;
@@ -189,21 +142,21 @@
 
        switch (v >> 4) {
 #ifdef INET
-        case 4:
+       case 4:
                hlen = sizeof(struct ip);
                break;
-#endif /* INET */
+#endif
 #ifdef INET6
-        case 6:
+       case 6:
                hlen = sizeof(struct ip6_hdr);
                break;
 #endif
-        default:
+       default:
                DPRINTF(("%s: bad protocol version 0x%x (%u) "
                    "for outer header\n", __func__, v, v>>4));
                IPIP_STATINC(IPIP_STAT_FAMILY);
                m_freem(m);
-               return /* EAFNOSUPPORT */;
+               return;
        }
 
        /* Bring the IP header in the first mbuf, if not there already */
@@ -215,24 +168,13 @@
                }
        }
 
-       ipo = mtod(m, struct ip *);
-
-#ifdef MROUTING
-       if (ipo->ip_v == IPVERSION && ipo->ip_p == IPPROTO_IPV4) {
-               if (IN_MULTICAST(((struct ip *)((char *) ipo + iphlen))->ip_dst.s_addr)) {
-                       ipip_mroute_input (m, iphlen);
-                       return;
-               }
-       }
-#endif /* MROUTING */
-
        /* Keep outer ecn field. */
        switch (v >> 4) {
 #ifdef INET
        case 4:
-               otos = ipo->ip_tos;
+               otos = mtod(m, struct ip *)->ip_tos;
                break;
-#endif /* INET */
+#endif
 #ifdef INET6
        case 6:
                otos = (ntohl(mtod(m, struct ip6_hdr *)->ip6_flow) >> 20) & 0xff;
@@ -256,14 +198,15 @@
 
        switch (v >> 4) {
 #ifdef INET
-        case 4:
+       case 4:
                hlen = sizeof(struct ip);
+               pktq = ip_pktq;
                break;
-#endif /* INET */
-
+#endif
 #ifdef INET6
-        case 6:
+       case 6:
                hlen = sizeof(struct ip6_hdr);
+               pktq = ip6_pktq;
                break;
 #endif
        default:
@@ -271,7 +214,7 @@
                    "for inner header\n", __func__, v, v >> 4));
                IPIP_STATINC(IPIP_STAT_FAMILY);
                m_freem(m);
-               return; /* EAFNOSUPPORT */
+               return;
        }
 
        /*
@@ -294,19 +237,19 @@
        /* Some sanity checks in the inner IP header */
        switch (v >> 4) {
 #ifdef INET
-       case 4:
-                ipo = mtod(m, struct ip *);
-               ip_ecn_egress(ip4_ipsec_ecn, &otos, &ipo->ip_tos);
-                break;
-#endif /* INET */
+       case 4:
+               ip4 = mtod(m, struct ip *);
+               ip_ecn_egress(ip4_ipsec_ecn, &otos, &ip4->ip_tos);
+               break;
+#endif
 #ifdef INET6
-       case 6:
-                ip6 = (struct ip6_hdr *) ipo;
+       case 6:
+               ip6 = mtod(m, struct ip6_hdr *);
                itos = (ntohl(ip6->ip6_flow) >> 20) & 0xff;
                ip_ecn_egress(ip6_ipsec_ecn, &otos, &itos);
                ip6->ip6_flow &= ~htonl(0xff << 20);
-               ip6->ip6_flow |= htonl((uint32_t) itos << 20);
-                break;
+               ip6->ip6_flow |= htonl((uint32_t)itos << 20);
+               break;
 #endif
        default:
                panic("%s: unknown ip version %u (inner)", __func__, v>>4);
@@ -320,7 +263,7 @@
                IFNET_READER_FOREACH(ifp) {
                        IFADDR_READER_FOREACH(ifa, ifp) {
 #ifdef INET
-                               if (ipo) {
+                               if (ip4) {
                                        if (ifa->ifa_addr->sa_family !=
                                            AF_INET)
                                                continue;
@@ -328,7 +271,7 @@
                                        sin = (struct sockaddr_in *) ifa->ifa_addr;
 
                                        if (sin->sin_addr.s_addr ==
-                                           ipo->ip_src.s_addr) {
+                                           ip4->ip_src.s_addr) {
                                                pserialize_read_exit(s);
                                                IPIP_STATINC(IPIP_STAT_SPOOF);
                                                m_freem(m);
@@ -359,8 +302,8 @@
                pserialize_read_exit(s);
        }
 
-       /* Statistics */
-       IPIP_STATADD(IPIP_STAT_IBYTES, m->m_pkthdr.len - iphlen);
+       /* Statistics: m->m_pkthdr.len is the length of the inner packet */
+       IPIP_STATADD(IPIP_STAT_IBYTES, m->m_pkthdr.len);
 
        /*
         * Interface pointer stays the same; if no IPsec processing has
@@ -370,21 +313,6 @@
         * untrusted packets.
         */
 
-       switch (v >> 4) {
-#ifdef INET
-       case 4:
-               pktq = ip_pktq;
-               break;
-#endif
-#ifdef INET6
-       case 6:
-               pktq = ip6_pktq;
-               break;
-#endif
-       default:
-               panic("%s: should never reach here", __func__);
-       }
-
        int s = splnet();
        if (__predict_false(!pktq_enqueue(pktq, m, 0))) {
                IPIP_STATINC(IPIP_STAT_QFULL);
@@ -394,26 +322,20 @@
 }
 
 int
-ipip_output(
-    struct mbuf *m,
-    const struct ipsecrequest *isr,



Home | Main Index | Thread Index | Old Index