Hi,
I would like to simplify the reverse pcb lookup logic. The
PF_TAG_TRANSLATE_LOCALHOST security check predates the divert
feature. It prevents that you accidentally use redirect where a
divert-to would be appropriate.
Instead of spreading the logic into tcp and udp input, we could
just check the flag during pcb listen lookup. This also reduces
parameters of in_pcblookup_listen().
I also cleaned up the #if, if, else if, else logic in
in_pcblookup_listen(). Set the default in front, do the special
pf things afterwards.
ok?
bluhm
Index: net/pf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
retrieving revision 1.1048
diff -u -p -r1.1048 pf.c
--- net/pf.c 28 Nov 2017 16:05:46 -0000 1.1048
+++ net/pf.c 30 Nov 2017 16:56:32 -0000
@@ -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: netinet/in_pcb.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.c,v
retrieving revision 1.224
diff -u -p -r1.224 in_pcb.c
--- netinet/in_pcb.c 11 Aug 2017 19:53:02 -0000 1.224
+++ netinet/in_pcb.c 30 Nov 2017 16:29:58 -0000
@@ -1133,7 +1133,7 @@ in6_pcbhashlookup(struct inpcbtable *tab
*/
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 *t
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 *t
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 *t
#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 *
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 *
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: netinet/in_pcb.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.h,v
retrieving revision 1.105
diff -u -p -r1.105 in_pcb.h
--- netinet/in_pcb.h 6 Oct 2017 21:14:55 -0000 1.105
+++ netinet/in_pcb.h 30 Nov 2017 15:58:42 -0000
@@ -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: netinet/tcp_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.352
diff -u -p -r1.352 tcp_input.c
--- netinet/tcp_input.c 20 Nov 2017 10:35:24 -0000 1.352
+++ netinet/tcp_input.c 30 Nov 2017 16:01:58 -0000
@@ -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: netinet/tcp_usrreq.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_usrreq.c,v
retrieving revision 1.160
diff -u -p -r1.160 tcp_usrreq.c
--- netinet/tcp_usrreq.c 20 Nov 2017 10:35:24 -0000 1.160
+++ netinet/tcp_usrreq.c 30 Nov 2017 16:30:22 -0000
@@ -818,12 +818,12 @@ tcp_ident(void *oldp, size_t *oldlenp, v
#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: netinet/udp_usrreq.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/udp_usrreq.c,v
retrieving revision 1.244
diff -u -p -r1.244 udp_usrreq.c
--- netinet/udp_usrreq.c 20 Nov 2017 10:35:24 -0000 1.244
+++ netinet/udp_usrreq.c 30 Nov 2017 16:32:32 -0000
@@ -528,20 +528,15 @@ udp_input(struct mbuf **mp, int *offp, i
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 *
* 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
