Re: git: 417b35a97b76 - main - netinet: Add a sysctl to allow disabling connections to INADDR_ANY

On Wed, Aug 21, 2024 at 12:59:18AM +0000, Shawn Webb wrote:
> Hey Mark,
> 
> When I set the net.inet.ip.connect_inaddr_wild sysctl node to 0 and
> try running `nc -vv 0.0.0.0 22` (this VM has sshd enabled), the
> below-linked KASSERT fires:
> 
> https://cgit.freebsd.org/src/tree/sys/netinet/in_pcb.c#n2304
> 
> No KASSERT is tripped on the IPv6 code path--that works fine. Only
> IPv4 is impacted.

I had tested this when I wrote the patch, and just tried again now.  I
haven't been able to trigger the panic:

root@freebsd:~ # nc -4 -vv 0.0.0.0 22
nc: connect to 0.0.0.0 port 22 (tcp) failed: Network is unreachable

I believe the error is coming from in_pcbladdr().  But, I can bypass
that by rerunning the test in a classic jail with ip4.saddrsel=0.  So,
it seems it's best to explicitly catch that case and return an error:
https://reviews.freebsd.org/D46454

> -- 
> Shawn Webb
> Cofounder / Security Engineer
> HardenedBSD
> 
> Tor-ified Signal: +1 303-901-1600 / shawn_webb_opsec.50
> https://git.hardenedbsd.org/hardenedbsd/pubkeys/-/raw/master/Shawn_Webb/03A4CBEBB82EA5A67D9F3853FF2E67A277F8E1FA.pub.asc
> 
> On Tue, Aug 20, 2024 at 09:34:39PM UTC, Mark Johnston wrote:
> > The branch main has been updated by markj:
> > 
> > URL: https://cgit.FreeBSD.org/src/commit/?id=417b35a97b7669eb0bf417b43e97cccbedbce6f9
> > 
> > commit 417b35a97b7669eb0bf417b43e97cccbedbce6f9
> > Author:     Mark Johnston <markj@FreeBSD.org>
> > AuthorDate: 2024-08-20 21:31:57 +0000
> > Commit:     Mark Johnston <markj@FreeBSD.org>
> > CommitDate: 2024-08-20 21:31:57 +0000
> > 
> >     netinet: Add a sysctl to allow disabling connections to INADDR_ANY
> >     
> >     See the discussion in Bugzilla PR 280705 for context.
> >     
> >     PR:             280705
> >     MFC after:      1 week
> >     Differential Revision:  https://reviews.freebsd.org/D46259
> > ---
> >  sys/netinet/in_pcb.c   |  8 +++++++-
> >  sys/netinet6/in6_pcb.c | 12 +++++++++++-
> >  2 files changed, 18 insertions(+), 2 deletions(-)
> > 
> > diff --git a/sys/netinet/in_pcb.c b/sys/netinet/in_pcb.c
> > index 1a341d421f31..3fc90f1e12c2 100644
> > --- a/sys/netinet/in_pcb.c
> > +++ b/sys/netinet/in_pcb.c
> > @@ -234,6 +234,12 @@ in_pcbhashseed_init(void)
> >  VNET_SYSINIT(in_pcbhashseed_init, SI_SUB_PROTO_DOMAIN, SI_ORDER_FIRST,
> >      in_pcbhashseed_init, 0);
> >  
> > +VNET_DEFINE_STATIC(int, connect_inaddr_wild) = 1;
> > +#define	V_connect_inaddr_wild	VNET(connect_inaddr_wild)
> > +SYSCTL_INT(_net_inet_ip, OID_AUTO, connect_inaddr_wild,
> > +    CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(connect_inaddr_wild), 0,
> > +    "Allow connecting to INADDR_ANY or INADDR_BROADCAST for connect(2)");
> > +
> >  static void in_pcbremhash(struct inpcb *);
> >  
> >  /*
> > @@ -1309,7 +1315,7 @@ in_pcbconnect_setup(struct inpcb *inp, struct sockaddr_in *sin,
> >  		inp->inp_flowtype = hash_type;
> >  	}
> >  #endif
> > -	if (!CK_STAILQ_EMPTY(&V_in_ifaddrhead)) {
> > +	if (V_connect_inaddr_wild && !CK_STAILQ_EMPTY(&V_in_ifaddrhead)) {
> >  		/*
> >  		 * If the destination address is INADDR_ANY,
> >  		 * use the primary local address.
> > diff --git a/sys/netinet6/in6_pcb.c b/sys/netinet6/in6_pcb.c
> > index e6ec0f24c898..098b4e50483c 100644
> > --- a/sys/netinet6/in6_pcb.c
> > +++ b/sys/netinet6/in6_pcb.c
> > @@ -83,6 +83,7 @@
> >  #include <sys/socket.h>
> >  #include <sys/socketvar.h>
> >  #include <sys/sockio.h>
> > +#include <sys/sysctl.h>
> >  #include <sys/errno.h>
> >  #include <sys/time.h>
> >  #include <sys/priv.h>
> > @@ -97,6 +98,7 @@
> >  #include <net/if_types.h>
> >  #include <net/route.h>
> >  #include <net/route/nhop.h>
> > +#include <net/vnet.h>
> >  
> >  #include <netinet/in.h>
> >  #include <netinet/in_var.h>
> > @@ -112,6 +114,14 @@
> >  #include <netinet6/in6_fib.h>
> >  #include <netinet6/scope6_var.h>
> >  
> > +SYSCTL_DECL(_net_inet6);
> > +SYSCTL_DECL(_net_inet6_ip6);
> > +VNET_DEFINE_STATIC(int, connect_in6addr_wild) = 1;
> > +#define	V_connect_in6addr_wild	VNET(connect_in6addr_wild)
> > +SYSCTL_INT(_net_inet6_ip6, OID_AUTO, connect_in6addr_wild,
> > +    CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(connect_in6addr_wild), 0,
> > +    "Allow connecting to the unspecified address for connect(2)");
> > +
> >  int
> >  in6_pcbsetport(struct in6_addr *laddr, struct inpcb *inp, struct ucred *cred)
> >  {
> > @@ -351,7 +361,7 @@ in6_pcbladdr(struct inpcb *inp, struct sockaddr_in6 *sin6,
> >  	if ((error = sa6_embedscope(sin6, V_ip6_use_defzone)) != 0)
> >  		return(error);
> >  
> > -	if (!CK_STAILQ_EMPTY(&V_in6_ifaddrhead)) {
> > +	if (V_connect_in6addr_wild && !CK_STAILQ_EMPTY(&V_in6_ifaddrhead)) {
> >  		/*
> >  		 * If the destination address is UNSPECIFIED addr,
> >  		 * use the loopback addr, e.g ::1.
> >