From 511f7f2b9e32699a40c156421e4e666b2b62108e Mon Sep 17 00:00:00 2001 From: bluhm Date: Fri, 1 Dec 2017 10:33:33 +0000 Subject: [PATCH] Simplify the reverse PCB lookup logic. The PF_TAG_TRANSLATE_LOCALHOST 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 | 8 ++++---- sys/netinet/in_pcb.c | 26 +++++++++++--------------- sys/netinet/in_pcb.h | 9 ++++----- sys/netinet/tcp_input.c | 15 +++++---------- sys/netinet/tcp_usrreq.c | 6 +++--- sys/netinet/udp_usrreq.c | 17 ++++++----------- 6 files changed, 33 insertions(+), 48 deletions(-) diff --git a/sys/net/pf.c b/sys/net/pf.c index 01e60badeaa..20e0e1fc5e2 100644 --- a/sys/net/pf.c +++ b/sys/net/pf.c @@ -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) >> diff --git a/sys/netinet/in_pcb.c b/sys/netinet/in_pcb.c index 6027e880629..3acde7acf94 100644 --- a/sys/netinet/in_pcb.c +++ b/sys/netinet/in_pcb.c @@ -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) { diff --git a/sys/netinet/in_pcb.h b/sys/netinet/in_pcb.h index c8ad4c4ae2c..fdddaba3df3 100644 --- a/sys/netinet/in_pcb.h +++ b/sys/netinet/in_pcb.h @@ -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 *); diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c index bf93c0899da..1112bce418d 100644 --- a/sys/netinet/tcp_input.c +++ b/sys/netinet/tcp_input.c @@ -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; } /* diff --git a/sys/netinet/tcp_usrreq.c b/sys/netinet/tcp_usrreq.c index 7f6a50fe62f..02fdf2069f1 100644 --- a/sys/netinet/tcp_usrreq.c +++ b/sys/netinet/tcp_usrreq.c @@ -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; } } diff --git a/sys/netinet/udp_usrreq.c b/sys/netinet/udp_usrreq.c index e7c490cc107..57a35faf9e1 100644 --- a/sys/netinet/udp_usrreq.c +++ b/sys/netinet/udp_usrreq.c @@ -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 -- 2.20.1