Simplify the reverse PCB lookup logic. The PF_TAG_TRANSLATE_LOCALHOST
authorbluhm <bluhm@openbsd.org>
Fri, 1 Dec 2017 10:33:33 +0000 (10:33 +0000)
committerbluhm <bluhm@openbsd.org>
Fri, 1 Dec 2017 10:33:33 +0000 (10:33 +0000)
security check prevents that the user accidentally configures
redirect where a divert-to would be appropriate.  Instead of spreading
the logic into tcp and udp input, check the flag during PCB listen
lookup.  This also reduces parameters of in_pcblookup_listen().
OK visa@

sys/net/pf.c
sys/netinet/in_pcb.c
sys/netinet/in_pcb.h
sys/netinet/tcp_input.c
sys/netinet/tcp_usrreq.c
sys/netinet/udp_usrreq.c

index 01e60ba..20e0e1f 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: pf.c,v 1.1048 2017/11/28 16:05:46 bluhm Exp $ */
+/*     $OpenBSD: pf.c,v 1.1049 2017/12/01 10:33:33 bluhm Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -3224,7 +3224,7 @@ pf_socket_lookup(struct pf_pdesc *pd)
                inp = in_pcbhashlookup(tb, saddr->v4, sport, daddr->v4, dport,
                    pd->rdomain);
                if (inp == NULL) {
-                       inp = in_pcblookup_listen(tb, daddr->v4, dport, 0,
+                       inp = in_pcblookup_listen(tb, daddr->v4, dport,
                            NULL, pd->rdomain);
                        if (inp == NULL)
                                return (-1);
@@ -3235,7 +3235,7 @@ pf_socket_lookup(struct pf_pdesc *pd)
                inp = in6_pcbhashlookup(tb, &saddr->v6, sport, &daddr->v6,
                    dport, pd->rdomain);
                if (inp == NULL) {
-                       inp = in6_pcblookup_listen(tb, &daddr->v6, dport, 0,
+                       inp = in6_pcblookup_listen(tb, &daddr->v6, dport,
                            NULL, pd->rdomain);
                        if (inp == NULL)
                                return (-1);
@@ -6971,7 +6971,7 @@ done:
        /*
         * connections redirected to loopback should not match sockets
         * bound specifically to loopback due to security implications,
-        * see tcp_input() and in_pcblookup_listen().
+        * see in_pcblookup_listen().
         */
        if (pd.destchg)
                if ((pd.af == AF_INET && (ntohl(pd.dst->v4.s_addr) >>
index 6027e88..3acde7a 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: in_pcb.c,v 1.224 2017/08/11 19:53:02 bluhm Exp $      */
+/*     $OpenBSD: in_pcb.c,v 1.225 2017/12/01 10:33:33 bluhm Exp $      */
 /*     $NetBSD: in_pcb.c,v 1.25 1996/02/13 23:41:53 christos Exp $     */
 
 /*
@@ -1133,7 +1133,7 @@ in6_pcbhashlookup(struct inpcbtable *table, const struct in6_addr *faddr,
  */
 struct inpcb *
 in_pcblookup_listen(struct inpcbtable *table, struct in_addr laddr,
-    u_int lport_arg, int reverse, struct mbuf *m, u_int rdomain)
+    u_int lport_arg, struct mbuf *m, u_int rdomain)
 {
        struct inpcbhead *head;
        struct in_addr *key1, *key2;
@@ -1141,6 +1141,8 @@ in_pcblookup_listen(struct inpcbtable *table, struct in_addr laddr,
        u_int16_t lport = lport_arg;
 
        rdomain = rtable_l2(rdomain);   /* convert passed rtableid to rdomain */
+       key1 = &laddr;
+       key2 = &zeroin_addr;
 #if NPF > 0
        if (m && m->m_pkthdr.pf.flags & PF_TAG_DIVERTED) {
                struct pf_divert *divert;
@@ -1149,15 +1151,11 @@ in_pcblookup_listen(struct inpcbtable *table, struct in_addr laddr,
                        return (NULL);
                key1 = key2 = &divert->addr.v4;
                lport = divert->port;
-       } else
-#endif
-       if (reverse) {
+       } else if (m && m->m_pkthdr.pf.flags & PF_TAG_TRANSLATE_LOCALHOST) {
                key1 = &zeroin_addr;
                key2 = &laddr;
-       } else {
-               key1 = &laddr;
-               key2 = &zeroin_addr;
        }
+#endif
 
        head = INPCBHASH(table, &zeroin_addr, 0, key1, lport, rdomain);
        LIST_FOREACH(inp, head, inp_hash) {
@@ -1206,7 +1204,7 @@ in_pcblookup_listen(struct inpcbtable *table, struct in_addr laddr,
 #ifdef INET6
 struct inpcb *
 in6_pcblookup_listen(struct inpcbtable *table, struct in6_addr *laddr,
-    u_int lport_arg, int reverse, struct mbuf *m, u_int rtable)
+    u_int lport_arg, struct mbuf *m, u_int rtable)
 {
        struct inpcbhead *head;
        struct in6_addr *key1, *key2;
@@ -1214,6 +1212,8 @@ in6_pcblookup_listen(struct inpcbtable *table, struct in6_addr *laddr,
        u_int16_t lport = lport_arg;
 
        rtable = rtable_l2(rtable);     /* convert passed rtableid to rdomain */
+       key1 = laddr;
+       key2 = &zeroin6_addr;
 #if NPF > 0
        if (m && m->m_pkthdr.pf.flags & PF_TAG_DIVERTED) {
                struct pf_divert *divert;
@@ -1222,15 +1222,11 @@ in6_pcblookup_listen(struct inpcbtable *table, struct in6_addr *laddr,
                        return (NULL);
                key1 = key2 = &divert->addr.v6;
                lport = divert->port;
-       } else
-#endif
-       if (reverse) {
+       } else if (m && m->m_pkthdr.pf.flags & PF_TAG_TRANSLATE_LOCALHOST) {
                key1 = &zeroin6_addr;
                key2 = laddr;
-       } else {
-               key1 = laddr;
-               key2 = &zeroin6_addr;
        }
+#endif
 
        head = IN6PCBHASH(table, &zeroin6_addr, 0, key1, lport, rtable);
        LIST_FOREACH(inp, head, inp_hash) {
index c8ad4c4..fdddaba 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: in_pcb.h,v 1.105 2017/10/06 21:14:55 bluhm Exp $      */
+/*     $OpenBSD: in_pcb.h,v 1.106 2017/12/01 10:33:33 bluhm Exp $      */
 /*     $NetBSD: in_pcb.h,v 1.14 1996/02/13 23:42:00 christos Exp $     */
 
 /*
@@ -263,16 +263,15 @@ struct inpcb *
         in_pcbhashlookup(struct inpcbtable *, struct in_addr,
                               u_int, struct in_addr, u_int, u_int);
 struct inpcb *
-        in_pcblookup_listen(struct inpcbtable *, struct in_addr, u_int, int,
+        in_pcblookup_listen(struct inpcbtable *, struct in_addr, u_int,
            struct mbuf *, u_int);
 #ifdef INET6
 struct inpcb *
         in6_pcbhashlookup(struct inpcbtable *, const struct in6_addr *,
                               u_int, const struct in6_addr *, u_int, u_int);
 struct inpcb *
-        in6_pcblookup_listen(struct inpcbtable *,
-                              struct in6_addr *, u_int, int, struct mbuf *,
-                              u_int);
+        in6_pcblookup_listen(struct inpcbtable *, struct in6_addr *, u_int,
+           struct mbuf *, u_int);
 int     in6_pcbaddrisavail(struct inpcb *, struct sockaddr_in6 *, int,
            struct proc *);
 int     in6_pcbconnect(struct inpcb *, struct mbuf *);
index bf93c08..1112bce 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: tcp_input.c,v 1.352 2017/11/20 10:35:24 mpi Exp $     */
+/*     $OpenBSD: tcp_input.c,v 1.353 2017/12/01 10:33:33 bluhm Exp $   */
 /*     $NetBSD: tcp_input.c,v 1.23 1996/02/13 23:43:44 christos Exp $  */
 
 /*
@@ -548,22 +548,17 @@ findpcb:
                }
        }
        if (inp == NULL) {
-               int     inpl_reverse = 0;
-               if (m->m_pkthdr.pf.flags & PF_TAG_TRANSLATE_LOCALHOST)
-                       inpl_reverse = 1;
                tcpstat_inc(tcps_pcbhashmiss);
                switch (af) {
 #ifdef INET6
                case AF_INET6:
-                       inp = in6_pcblookup_listen(&tcbtable,
-                           &ip6->ip6_dst, th->th_dport, inpl_reverse, m,
-                           m->m_pkthdr.ph_rtableid);
+                       inp = in6_pcblookup_listen(&tcbtable, &ip6->ip6_dst,
+                           th->th_dport, m, m->m_pkthdr.ph_rtableid);
                        break;
 #endif /* INET6 */
                case AF_INET:
-                       inp = in_pcblookup_listen(&tcbtable,
-                           ip->ip_dst, th->th_dport, inpl_reverse, m,
-                           m->m_pkthdr.ph_rtableid);
+                       inp = in_pcblookup_listen(&tcbtable, ip->ip_dst,
+                           th->th_dport, m, m->m_pkthdr.ph_rtableid);
                        break;
                }
                /*
index 7f6a50f..02fdf20 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: tcp_usrreq.c,v 1.161 2017/11/30 15:24:50 nayden Exp $ */
+/*     $OpenBSD: tcp_usrreq.c,v 1.162 2017/12/01 10:33:33 bluhm Exp $  */
 /*     $NetBSD: tcp_usrreq.c,v 1.20 1996/02/13 23:44:16 christos Exp $ */
 
 /*
@@ -818,12 +818,12 @@ tcp_ident(void *oldp, size_t *oldlenp, void *newp, size_t newlen, int dodrop)
 #ifdef INET6
                case AF_INET6:
                        inp = in6_pcblookup_listen(&tcbtable,
-                           &l6, lin6->sin6_port, 0, NULL, tir.rdomain);
+                           &l6, lin6->sin6_port, NULL, tir.rdomain);
                        break;
 #endif
                case AF_INET:
                        inp = in_pcblookup_listen(&tcbtable,
-                           lin->sin_addr, lin->sin_port, 0, NULL, tir.rdomain);
+                           lin->sin_addr, lin->sin_port, NULL, tir.rdomain);
                        break;
                }
        }
index e7c490c..57a35fa 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: udp_usrreq.c,v 1.244 2017/11/20 10:35:24 mpi Exp $    */
+/*     $OpenBSD: udp_usrreq.c,v 1.245 2017/12/01 10:33:33 bluhm Exp $  */
 /*     $NetBSD: udp_usrreq.c,v 1.28 1996/03/16 23:54:03 christos Exp $ */
 
 /*
@@ -528,20 +528,15 @@ udp_input(struct mbuf **mp, int *offp, int proto, int af)
                    ip->ip_dst, uh->uh_dport, m->m_pkthdr.ph_rtableid);
        }
        if (inp == 0) {
-               int     inpl_reverse = 0;
-               if (m->m_pkthdr.pf.flags & PF_TAG_TRANSLATE_LOCALHOST)
-                       inpl_reverse = 1;
                udpstat_inc(udps_pcbhashmiss);
 #ifdef INET6
                if (ip6) {
-                       inp = in6_pcblookup_listen(&udbtable,
-                           &ip6->ip6_dst, uh->uh_dport, inpl_reverse, m,
-                           m->m_pkthdr.ph_rtableid);
+                       inp = in6_pcblookup_listen(&udbtable, &ip6->ip6_dst,
+                           uh->uh_dport, m, m->m_pkthdr.ph_rtableid);
                } else
 #endif /* INET6 */
-               inp = in_pcblookup_listen(&udbtable,
-                   ip->ip_dst, uh->uh_dport, inpl_reverse, m,
-                   m->m_pkthdr.ph_rtableid);
+               inp = in_pcblookup_listen(&udbtable, ip->ip_dst,
+                   uh->uh_dport, m, m->m_pkthdr.ph_rtableid);
                if (inp == 0) {
                        udpstat_inc(udps_noport);
                        if (m->m_flags & (M_BCAST | M_MCAST)) {
@@ -801,7 +796,7 @@ udp6_ctlinput(int cmd, struct sockaddr *sa, u_int rdomain, void *d)
                         * is really ours.
                         */
                        else if (in6_pcblookup_listen(&udbtable,
-                           &sa6_src.sin6_addr, uh.uh_sport, 0,
+                           &sa6_src.sin6_addr, uh.uh_sport, NULL,
                            rdomain))
                                valid = 1;
 #endif