[ICMP] IP options lead to malformed reply
authorsashan <sashan@openbsd.org>
Tue, 30 Mar 2021 08:37:10 +0000 (08:37 +0000)
committersashan <sashan@openbsd.org>
Tue, 30 Mar 2021 08:37:10 +0000 (08:37 +0000)
icmp_send() must update IP header length if IP optaions are appended.
Such packet also has to be dispatched with IP_RAWOUTPUT flags.

Bug reported and fix co-designed by Dominik Schreilechner _at_ siemens _dot_ com

OK bluhm@

sys/netinet/ip_icmp.c
sys/netinet/ip_input.c
sys/netinet/ip_output.c
sys/netinet/ip_var.h

index a007aa6..fd7b016 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_icmp.c,v 1.185 2021/02/25 02:48:21 dlg Exp $       */
+/*     $OpenBSD: ip_icmp.c,v 1.186 2021/03/30 08:37:10 sashan Exp $    */
 /*     $NetBSD: ip_icmp.c,v 1.19 1996/02/13 23:42:22 christos Exp $    */
 
 /*
@@ -846,10 +846,21 @@ icmp_send(struct mbuf *m, struct mbuf *opts)
                printf("icmp_send dst %s src %s\n", dst, src);
        }
 #endif
-       if (opts != NULL)
+       /*
+        * ip_send() cannot handle IP options properly. So in case we have
+        * options fill out the IP header here and use ip_send_raw() instead.
+        */
+       if (opts != NULL) {
                m = ip_insertoptions(m, opts, &hlen);
-
-       ip_send(m);
+               ip = mtod(m, struct ip *);
+               ip->ip_hl = (hlen >> 2);
+               ip->ip_v = IPVERSION;
+               ip->ip_off &= htons(IP_DF);
+               ip->ip_id = htons(ip_randomid());
+               ipstat_inc(ips_localout);
+               ip_send_raw(m);
+       } else
+               ip_send(m);
 }
 
 u_int32_t
index 0ec3f72..e5b74e4 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_input.c,v 1.355 2021/03/10 10:21:48 jsg Exp $      */
+/*     $OpenBSD: ip_input.c,v 1.356 2021/03/30 08:37:10 sashan Exp $   */
 /*     $NetBSD: ip_input.c,v 1.30 1996/03/16 23:53:58 christos Exp $   */
 
 /*
@@ -139,6 +139,7 @@ struct cpumem *ipcounters;
 int ip_sysctl_ipstat(void *, size_t *, void *);
 
 static struct mbuf_queue       ipsend_mq;
+static struct mbuf_queue       ipsendraw_mq;
 
 extern struct niqueue          arpinq;
 
@@ -147,7 +148,11 @@ int        ip_dooptions(struct mbuf *, struct ifnet *);
 int    in_ouraddr(struct mbuf *, struct ifnet *, struct rtentry **);
 
 static void ip_send_dispatch(void *);
+static void ip_sendraw_dispatch(void *);
 static struct task ipsend_task = TASK_INITIALIZER(ip_send_dispatch, &ipsend_mq);
+static struct task ipsendraw_task =
+       TASK_INITIALIZER(ip_sendraw_dispatch, &ipsendraw_mq);
+
 /*
  * Used to save the IP options in case a protocol wants to respond
  * to an incoming packet over the same route if the packet got here
@@ -217,6 +222,7 @@ ip_init(void)
                DP_SET(rootonlyports.udp, defrootonlyports_udp[i]);
 
        mq_init(&ipsend_mq, 64, IPL_SOFTNET);
+       mq_init(&ipsendraw_mq, 64, IPL_SOFTNET);
 
 #ifdef IPSEC
        ipsec_init();
@@ -1777,7 +1783,7 @@ ip_savecontrol(struct inpcb *inp, struct mbuf **mp, struct ip *ip,
 }
 
 void
-ip_send_dispatch(void *xmq)
+ip_send_do_dispatch(void *xmq, int flags)
 {
        struct mbuf_queue *mq = xmq;
        struct mbuf *m;
@@ -1789,14 +1795,33 @@ ip_send_dispatch(void *xmq)
 
        NET_LOCK();
        while ((m = ml_dequeue(&ml)) != NULL) {
-               ip_output(m, NULL, NULL, 0, NULL, NULL, 0);
+               ip_output(m, NULL, NULL, flags, NULL, NULL, 0);
        }
        NET_UNLOCK();
 }
 
+void
+ip_sendraw_dispatch(void *xmq)
+{
+       ip_send_do_dispatch(xmq, IP_RAWOUTPUT);
+}
+
+void
+ip_send_dispatch(void *xmq)
+{
+       ip_send_do_dispatch(xmq, 0);
+}
+
 void
 ip_send(struct mbuf *m)
 {
        mq_enqueue(&ipsend_mq, m);
        task_add(net_tq(0), &ipsend_task);
 }
+
+void
+ip_send_raw(struct mbuf *m)
+{
+       mq_enqueue(&ipsendraw_mq, m);
+       task_add(net_tq(0), &ipsendraw_task);
+}
index c01a3e7..e9e4151 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_output.c,v 1.369 2021/03/20 01:15:28 dlg Exp $     */
+/*     $OpenBSD: ip_output.c,v 1.370 2021/03/30 08:37:11 sashan Exp $  */
 /*     $NetBSD: ip_output.c,v 1.28 1996/02/13 23:43:07 christos Exp $  */
 
 /*
@@ -765,6 +765,13 @@ ip_insertoptions(struct mbuf *m, struct mbuf *opt, int *phlen)
        optlen = opt->m_len - sizeof(p->ipopt_dst);
        if (optlen + ntohs(ip->ip_len) > IP_MAXPACKET)
                return (m);             /* XXX should fail */
+
+       /* check if options will fit to IP header */
+       if ((optlen + sizeof(struct ip)) > (0x0f << 2)) {
+               *phlen = sizeof(struct ip);
+               return (m);
+       }
+
        if (p->ipopt_dst.s_addr)
                ip->ip_dst = p->ipopt_dst;
        if (m->m_flags & M_EXT || m->m_data - optlen < m->m_pktdat) {
index 7ede24c..e2269e4 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: ip_var.h,v 1.87 2021/03/01 11:05:42 bluhm Exp $       */
+/*     $OpenBSD: ip_var.h,v 1.88 2021/03/30 08:37:11 sashan Exp $      */
 /*     $NetBSD: ip_var.h,v 1.16 1996/02/13 23:43:20 christos Exp $     */
 
 /*
@@ -240,6 +240,7 @@ struct mbuf *
 u_int16_t
         ip_randomid(void);
 void    ip_send(struct mbuf *);
+void    ip_send_raw(struct mbuf *);
 void    ip_slowtimo(void);
 struct mbuf *
         ip_srcroute(struct mbuf *);