Raw socket should comply with selected source address
Raw sockets do not comply with route sourceaddr. Use set address if source is not set by the caller. Index: netinet/ip_output.c === RCS file: /cvs/src/sys/netinet/ip_output.c,v retrieving revision 1.377 diff -u -p -r1.377 ip_output.c --- netinet/ip_output.c 3 Dec 2021 17:18:34 - 1.377 +++ netinet/ip_output.c 16 Dec 2021 18:12:44 - @@ -110,6 +110,7 @@ ip_output(struct mbuf *m, struct mbuf *o struct route iproute; struct sockaddr_in *dst; struct tdb *tdb = NULL; + struct sockaddr *ip4_source = NULL; u_long mtu; #if NPF > 0 u_int orig_rtableid; @@ -237,8 +238,18 @@ reroute: dst = satosin(ro->ro_rt->rt_gateway); /* Set the source IP address */ - if (ip->ip_src.s_addr == INADDR_ANY && ia) - ip->ip_src = ia->ia_addr.sin_addr; + if (ip->ip_src.s_addr == INADDR_ANY && ia) { + ip4_source = rtable_getsource(ro->ro_tableid, AF_INET); + if (ip4_source != NULL) { + struct ifaddr *ifa; + if ((ifa = ifa_ifwithaddr(ip4_source, + ro->ro_tableid)) != NULL && + ISSET(ifa->ifa_ifp->if_flags, IFF_UP)) { + ip->ip_src = satosin(ip4_source)->sin_addr; + } + } else + ip->ip_src = ia->ia_addr.sin_addr; + } } #ifdef IPSEC
Re: Raw socket should comply with selected source address
On Thu, Dec 16, 2021 at 07:20:04PM +0100, Denis Fondras wrote: > Raw sockets do not comply with route sourceaddr. > > Use set address if source is not set by the caller. Which problem do you want to solve? Which setups do you break? bluhm > Index: netinet/ip_output.c > === > RCS file: /cvs/src/sys/netinet/ip_output.c,v > retrieving revision 1.377 > diff -u -p -r1.377 ip_output.c > --- netinet/ip_output.c 3 Dec 2021 17:18:34 - 1.377 > +++ netinet/ip_output.c 16 Dec 2021 18:12:44 - > @@ -110,6 +110,7 @@ ip_output(struct mbuf *m, struct mbuf *o > struct route iproute; > struct sockaddr_in *dst; > struct tdb *tdb = NULL; > + struct sockaddr *ip4_source = NULL; > u_long mtu; > #if NPF > 0 > u_int orig_rtableid; > @@ -237,8 +238,18 @@ reroute: > dst = satosin(ro->ro_rt->rt_gateway); > > /* Set the source IP address */ > - if (ip->ip_src.s_addr == INADDR_ANY && ia) > - ip->ip_src = ia->ia_addr.sin_addr; > + if (ip->ip_src.s_addr == INADDR_ANY && ia) { > + ip4_source = rtable_getsource(ro->ro_tableid, AF_INET); > + if (ip4_source != NULL) { > + struct ifaddr *ifa; > + if ((ifa = ifa_ifwithaddr(ip4_source, > + ro->ro_tableid)) != NULL && > + ISSET(ifa->ifa_ifp->if_flags, IFF_UP)) { > + ip->ip_src = > satosin(ip4_source)->sin_addr; > + } > + } else > + ip->ip_src = ia->ia_addr.sin_addr; > + } > } > > #ifdef IPSEC
Re: Raw socket should comply with selected source address
'route sourceaddr' support is incomplete. In particular it does not work in ping or traceroute. The original idea of this option is to replace the default src address allocation algorithm, with a static default, particularily on routers. It is only working for non-bound sockets, but it should also work for SOCK_RAW or potentially other configurations. I believe this was implimented too cynically, as a "oh we only need to fix these few cases", but we are better off applying the replacement algorithm for all unbound cases, then there is only algorithm A or B, not a mix of algorithms depending on the type of socket being used. It is really fun when you realize ping -v uses a dummy connect() to figure out a srcaddr to print, then the kernel chooses a different address for the sent packets. Alexander Bluhm wrote: > On Thu, Dec 16, 2021 at 07:20:04PM +0100, Denis Fondras wrote: > > Raw sockets do not comply with route sourceaddr. > > > > Use set address if source is not set by the caller. > > Which problem do you want to solve? > Which setups do you break? > > bluhm > > > Index: netinet/ip_output.c > > === > > RCS file: /cvs/src/sys/netinet/ip_output.c,v > > retrieving revision 1.377 > > diff -u -p -r1.377 ip_output.c > > --- netinet/ip_output.c 3 Dec 2021 17:18:34 - 1.377 > > +++ netinet/ip_output.c 16 Dec 2021 18:12:44 - > > @@ -110,6 +110,7 @@ ip_output(struct mbuf *m, struct mbuf *o > > struct route iproute; > > struct sockaddr_in *dst; > > struct tdb *tdb = NULL; > > + struct sockaddr *ip4_source = NULL; > > u_long mtu; > > #if NPF > 0 > > u_int orig_rtableid; > > @@ -237,8 +238,18 @@ reroute: > > dst = satosin(ro->ro_rt->rt_gateway); > > > > /* Set the source IP address */ > > - if (ip->ip_src.s_addr == INADDR_ANY && ia) > > - ip->ip_src = ia->ia_addr.sin_addr; > > + if (ip->ip_src.s_addr == INADDR_ANY && ia) { > > + ip4_source = rtable_getsource(ro->ro_tableid, AF_INET); > > + if (ip4_source != NULL) { > > + struct ifaddr *ifa; > > + if ((ifa = ifa_ifwithaddr(ip4_source, > > + ro->ro_tableid)) != NULL && > > + ISSET(ifa->ifa_ifp->if_flags, IFF_UP)) { > > + ip->ip_src = > > satosin(ip4_source)->sin_addr; > > + } > > + } else > > + ip->ip_src = ia->ia_addr.sin_addr; > > + } > > } > > > > #ifdef IPSEC >
Re: Raw socket should comply with selected source address
On Thu, Dec 16, 2021 at 11:48:58AM -0700, Theo de Raadt wrote: > 'route sourceaddr' support is incomplete. > In particular it does not work in ping or traceroute. Thanks for the explanation. > > On Thu, Dec 16, 2021 at 07:20:04PM +0100, Denis Fondras wrote: > > > Raw sockets do not comply with route sourceaddr. So it is about unconnected sockets. Should it be fixed there? > > > + struct sockaddr *ip4_source = NULL; Initialization = NULL is not necessay. > > > + ip4_source = rtable_getsource(ro->ro_tableid, AF_INET); > > > + if (ip4_source != NULL) { > > > + struct ifaddr *ifa; > > > + if ((ifa = ifa_ifwithaddr(ip4_source, I would not call a function from the IP output path that grabs kernel lock and loops over all interfaces and addresses. Should we store it at the socket and use it if inp_laddr is not set? struct art_root { struct sockaddr *source;/* [K] optional src addr to use */ Why does this field not have an ar_ prefix like all the others? Why does it need kernel lock? You don't have kernel lock here. It is set with kernel lock, cleared with net lock and unset with the routing socket lock. The memory of the sockaddr seems to be protected by ifaddr refcounting and net lock. Can we assume that a store to a pointer and also dereferencing it is atomic? > > > + ro->ro_tableid)) != NULL && > > > + ISSET(ifa->ifa_ifp->if_flags, IFF_UP)) { > > > + ip->ip_src = > > > satosin(ip4_source)->sin_addr; > > > + } > > > + } else > > > + ip->ip_src = ia->ia_addr.sin_addr; > > > + } > > > } > > > > > > #ifdef IPSEC > >
[PATCH] relayd client certificate validation again
Hi! Here comes the support for relayd client certificate validation. Full certificate chain, subject and issuer can be passed over in http headers. It supports mandatory validation and optional validation(if client chooses to provide certificate it will be validated). Part of my sample config. http protocol test { match header set "CS_SUBJECT" value "$CLIENT_CERT_SUBJECT" match header set "CS_ISSUER" value "$CLIENT_CERT_ISSUER" match header set "CS_CERT" value "$CLIENT_CERT_CHAIN" pass tls {client ca "/tmp/easyrsa3/pki/ca.crt" optional } } This uses code from the patches submitted by Ashe Connor. Rivo diff refs/heads/master refs/heads/relay-clc3 blob - a2f1c130d6b45e3082048218c11537dca485998a blob + 5070a7d48f58403f53d818231e1676db749aa9d7 --- usr.sbin/relayd/config.c +++ usr.sbin/relayd/config.c @@ -954,6 +954,15 @@ config_setrelay(struct relayd *env, struct relay *rlay rlay->rl_conf.name); return (-1); } + if (rlay->rl_tls_client_ca_fd != -1 && + config_setrelayfd(ps, id, n, 0, + rlay->rl_conf.id, RELAY_FD_CLIENTCACERT, + rlay->rl_tls_client_ca_fd) == -1) { + log_warn("%s: fd passing failed for " + "`%s'", __func__, + rlay->rl_conf.name); + return (-1); + } /* Prevent fd exhaustion in the parent. */ if (proc_flush_imsg(ps, id, n) == -1) { log_warn("%s: failed to flush " @@ -987,6 +996,10 @@ config_setrelay(struct relayd *env, struct relay *rlay close(rlay->rl_s); rlay->rl_s = -1; } + if (rlay->rl_tls_client_ca_fd != -1) { + close(rlay->rl_tls_client_ca_fd); + rlay->rl_tls_client_ca_fd = -1; + } if (rlay->rl_tls_cacert_fd != -1) { close(rlay->rl_tls_cacert_fd); rlay->rl_tls_cacert_fd = -1; @@ -1012,6 +1025,10 @@ config_setrelay(struct relayd *env, struct relay *rlay cert->cert_ocsp_fd = -1; } } + if (rlay->rl_tls_client_ca_fd != -1) { + close(rlay->rl_tls_client_ca_fd); + rlay->rl_tls_client_ca_fd = -1; + } return (0); } @@ -1034,6 +1051,7 @@ config_getrelay(struct relayd *env, struct imsg *imsg) rlay->rl_s = imsg->fd; rlay->rl_tls_ca_fd = -1; rlay->rl_tls_cacert_fd = -1; + rlay->rl_tls_client_ca_fd = -1; if (ps->ps_what[privsep_process] & CONFIG_PROTOS) { if (rlay->rl_conf.proto == EMPTY_ID) @@ -1163,6 +1181,9 @@ config_getrelayfd(struct relayd *env, struct imsg *ims case RELAY_FD_CAFILE: rlay->rl_tls_cacert_fd = imsg->fd; break; + case RELAY_FD_CLIENTCACERT: + rlay->rl_tls_client_ca_fd = imsg->fd; + break; } DPRINTF("%s: %s %d received relay fd %d type %d for relay %s", __func__, blob - 22beb857229a16e5b2c17a25a2944231d41e7e08 blob + fe5e8ff4dfed10e8f09e3226bdfe33f8bc031c8e --- usr.sbin/relayd/parse.y +++ usr.sbin/relayd/parse.y @@ -172,14 +172,14 @@ typedef struct { %token CODE COOKIE DEMOTE DIGEST DISABLE ERROR EXPECT PASS BLOCK EXTERNAL %token FILENAME FORWARD FROM HASH HEADER HEADERLEN HOST HTTP ICMP INCLUDE INET %token INET6 INTERFACE INTERVAL IP KEYPAIR LABEL LISTEN VALUE LOADBALANCE LOG -%token LOOKUP METHOD MODE NAT NO DESTINATION NODELAY NOTHING ON PARENT PATH +%token LOOKUP METHOD MODE NAT NO DESTINATION NODELAY NOTHING ON OPTIONAL PARENT PATH %token PFTAG PORT PREFORK PRIORITY PROTO QUERYSTR REAL REDIRECT RELAY REMOVE %token REQUEST RESPONSE RETRY QUICK RETURN ROUNDROBIN ROUTE SACK SCRIPT SEND %token SESSION SOCKET SPLICE SSL STICKYADDR STRIP STYLE TABLE TAG TAGGED TCP %token TIMEOUT TLS TO ROUTER RTLABEL TRANSPARENT URL WITH TTL RTABLE %token MATCH PARAMS RANDOM LEASTSTATES SRCHASH KEY CERTIFICATE PASSWORD ECDHE %token EDH TICKETS CONNECTION CONNECTIONS CONTEXT ERRORS STATE CHANGES CHECKS -%token WEBSOCKETS +%token WEBSOCKETS CLIENT %token STRING %token NUMBER %type context hostname interface table value path @@ -188,6 +188,7 @@ typedef struct { %type opttls opttlsclient %type redirect_proto relay_proto match %type action ruleaf key_option +%type clientcaopt %typeport %typehost %typeaddress rulesrc ruledst addrprefix @@ -244,6 +245,10 @@ opttlsclient : /*empty*/ { $$ = 0; } | WITH ssltls { $$ = 1; } ; +clientcaopt : /*empty*/ { $$ = 0; } + | OP
Re: [PATCH] relayd client certificate validation again
Hi, not to interrupt development … Can you make this completely optional from the servers perspective? I don’t want my endpoints validating anonymous client certificates when I run a public endpoint. I’ll just hack it out otherwise, but I think this opens a vector that should be completely optional from a relay service. > On Dec 16, 2021, at 4:25 PM, rivo nurges wrote: > > Hi! > > Here comes the support for relayd client certificate validation. > Full certificate chain, subject and issuer can be passed over in http headers. > It supports mandatory validation and optional validation(if client chooses to > provide certificate it will be validated). > > Part of my sample config. > > http protocol test { > match header set "CS_SUBJECT" value "$CLIENT_CERT_SUBJECT" > match header set "CS_ISSUER" value "$CLIENT_CERT_ISSUER" > match header set "CS_CERT" value "$CLIENT_CERT_CHAIN" > pass > tls {client ca "/tmp/easyrsa3/pki/ca.crt" optional } > } > > This uses code from the patches submitted by Ashe Connor. > > Rivo > > diff refs/heads/master refs/heads/relay-clc3 > blob - a2f1c130d6b45e3082048218c11537dca485998a > blob + 5070a7d48f58403f53d818231e1676db749aa9d7 > --- usr.sbin/relayd/config.c > +++ usr.sbin/relayd/config.c > @@ -954,6 +954,15 @@ config_setrelay(struct relayd *env, struct relay *rlay >rlay->rl_conf.name); >return (-1); >} > +if (rlay->rl_tls_client_ca_fd != -1 && > +config_setrelayfd(ps, id, n, 0, > +rlay->rl_conf.id, RELAY_FD_CLIENTCACERT, > +rlay->rl_tls_client_ca_fd) == -1) { > +log_warn("%s: fd passing failed for " > +"`%s'", __func__, > +rlay->rl_conf.name); > +return (-1); > +} >/* Prevent fd exhaustion in the parent. */ >if (proc_flush_imsg(ps, id, n) == -1) { >log_warn("%s: failed to flush " > @@ -987,6 +996,10 @@ config_setrelay(struct relayd *env, struct relay *rlay >close(rlay->rl_s); >rlay->rl_s = -1; >} > +if (rlay->rl_tls_client_ca_fd != -1) { > +close(rlay->rl_tls_client_ca_fd); > +rlay->rl_tls_client_ca_fd = -1; > +} >if (rlay->rl_tls_cacert_fd != -1) { >close(rlay->rl_tls_cacert_fd); >rlay->rl_tls_cacert_fd = -1; > @@ -1012,6 +1025,10 @@ config_setrelay(struct relayd *env, struct relay *rlay >cert->cert_ocsp_fd = -1; >} >} > +if (rlay->rl_tls_client_ca_fd != -1) { > +close(rlay->rl_tls_client_ca_fd); > +rlay->rl_tls_client_ca_fd = -1; > +} > return (0); > } > @@ -1034,6 +1051,7 @@ config_getrelay(struct relayd *env, struct imsg *imsg) >rlay->rl_s = imsg->fd; >rlay->rl_tls_ca_fd = -1; >rlay->rl_tls_cacert_fd = -1; > +rlay->rl_tls_client_ca_fd = -1; > if (ps->ps_what[privsep_process] & CONFIG_PROTOS) { >if (rlay->rl_conf.proto == EMPTY_ID) > @@ -1163,6 +1181,9 @@ config_getrelayfd(struct relayd *env, struct imsg *ims >case RELAY_FD_CAFILE: >rlay->rl_tls_cacert_fd = imsg->fd; >break; > +case RELAY_FD_CLIENTCACERT: > +rlay->rl_tls_client_ca_fd = imsg->fd; > +break; >} > DPRINTF("%s: %s %d received relay fd %d type %d for relay %s", __func__, > blob - 22beb857229a16e5b2c17a25a2944231d41e7e08 > blob + fe5e8ff4dfed10e8f09e3226bdfe33f8bc031c8e > --- usr.sbin/relayd/parse.y > +++ usr.sbin/relayd/parse.y > @@ -172,14 +172,14 @@ typedef struct { > %tokenCODE COOKIE DEMOTE DIGEST DISABLE ERROR EXPECT PASS BLOCK EXTERNAL > %tokenFILENAME FORWARD FROM HASH HEADER HEADERLEN HOST HTTP ICMP INCLUDE > INET > %tokenINET6 INTERFACE INTERVAL IP KEYPAIR LABEL LISTEN VALUE LOADBALANCE > LOG > -%tokenLOOKUP METHOD MODE NAT NO DESTINATION NODELAY NOTHING ON PARENT > PATH > +%tokenLOOKUP METHOD MODE NAT NO DESTINATION NODELAY NOTHING ON OPTIONAL > PARENT PATH > %tokenPFTAG PORT PREFORK PRIORITY PROTO QUERYSTR REAL REDIRECT RELAY > REMOVE > %tokenREQUEST RESPONSE RETRY QUICK RETURN ROUNDROBIN ROUTE SACK SCRIPT > SEND > %tokenSESSION SOCKET SPLICE SSL STICKYADDR STRIP STYLE TABLE TAG TAGGED > TCP > %tokenTIMEOUT TLS TO ROUTER RTLABEL TRANSPARENT URL WITH TTL RTABLE > %tokenMATCH PARAMS RANDOM LEASTSTATES SRCHASH KEY CERTIFICATE PASSWORD > ECDHE > %tokenEDH TICKETS CONNECTION CONNECTIONS CONTEXT ERRORS STATE CHANGES > CHECKS > -%tokenWEBSOCKETS > +%tokenWEBSOCKETS CLIENT > %tokenSTRING > %token NUMBER > %typecontext hostname interface table value path > @@ -188,6 +188,7 @@ typedef struct { > %typeopttls opttlsclient > %typeredirect_proto relay_proto match > %typeaction ruleaf key_option > +%typeclientcaopt > %typeport > %typehost > %typeaddress rulesrc ruledst addrprefix > @@
Re: [PATCH] relayd client certificate validation again
Hi, sorry for being a moron. I realize it’s already optional by not specifying client ca… sorry about the noise! > On Dec 16, 2021, at 9:35 PM, Brian Brombacher wrote: > > Hi, not to interrupt development … > > Can you make this completely optional from the servers perspective? I don’t > want my endpoints validating anonymous client certificates when I run a > public endpoint. > > I’ll just hack it out otherwise, but I think this opens a vector that should > be completely optional from a relay service. > > >> On Dec 16, 2021, at 4:25 PM, rivo nurges wrote: >> >> Hi! >> >> Here comes the support for relayd client certificate validation. >> Full certificate chain, subject and issuer can be passed over in http >> headers. >> It supports mandatory validation and optional validation(if client chooses to >> provide certificate it will be validated). >> >> Part of my sample config. >> >> http protocol test { >> match header set "CS_SUBJECT" value "$CLIENT_CERT_SUBJECT" >> match header set "CS_ISSUER" value "$CLIENT_CERT_ISSUER" >> match header set "CS_CERT" value "$CLIENT_CERT_CHAIN" >> pass >> tls {client ca "/tmp/easyrsa3/pki/ca.crt" optional } >> } >> >> This uses code from the patches submitted by Ashe Connor. >> >> Rivo >> >> diff refs/heads/master refs/heads/relay-clc3 >> blob - a2f1c130d6b45e3082048218c11537dca485998a >> blob + 5070a7d48f58403f53d818231e1676db749aa9d7 >> --- usr.sbin/relayd/config.c >> +++ usr.sbin/relayd/config.c >> @@ -954,6 +954,15 @@ config_setrelay(struct relayd *env, struct relay *rlay >> rlay->rl_conf.name); >> return (-1); >> } >> +if (rlay->rl_tls_client_ca_fd != -1 && >> +config_setrelayfd(ps, id, n, 0, >> +rlay->rl_conf.id, RELAY_FD_CLIENTCACERT, >> +rlay->rl_tls_client_ca_fd) == -1) { >> +log_warn("%s: fd passing failed for " >> +"`%s'", __func__, >> +rlay->rl_conf.name); >> +return (-1); >> +} >> /* Prevent fd exhaustion in the parent. */ >> if (proc_flush_imsg(ps, id, n) == -1) { >> log_warn("%s: failed to flush " >> @@ -987,6 +996,10 @@ config_setrelay(struct relayd *env, struct relay *rlay >> close(rlay->rl_s); >> rlay->rl_s = -1; >> } >> +if (rlay->rl_tls_client_ca_fd != -1) { >> +close(rlay->rl_tls_client_ca_fd); >> +rlay->rl_tls_client_ca_fd = -1; >> +} >> if (rlay->rl_tls_cacert_fd != -1) { >> close(rlay->rl_tls_cacert_fd); >> rlay->rl_tls_cacert_fd = -1; >> @@ -1012,6 +1025,10 @@ config_setrelay(struct relayd *env, struct relay *rlay >> cert->cert_ocsp_fd = -1; >> } >> } >> +if (rlay->rl_tls_client_ca_fd != -1) { >> +close(rlay->rl_tls_client_ca_fd); >> +rlay->rl_tls_client_ca_fd = -1; >> +} >> return (0); >> } >> @@ -1034,6 +1051,7 @@ config_getrelay(struct relayd *env, struct imsg *imsg) >> rlay->rl_s = imsg->fd; >> rlay->rl_tls_ca_fd = -1; >> rlay->rl_tls_cacert_fd = -1; >> +rlay->rl_tls_client_ca_fd = -1; >> if (ps->ps_what[privsep_process] & CONFIG_PROTOS) { >> if (rlay->rl_conf.proto == EMPTY_ID) >> @@ -1163,6 +1181,9 @@ config_getrelayfd(struct relayd *env, struct imsg *ims >> case RELAY_FD_CAFILE: >> rlay->rl_tls_cacert_fd = imsg->fd; >> break; >> +case RELAY_FD_CLIENTCACERT: >> +rlay->rl_tls_client_ca_fd = imsg->fd; >> +break; >> } >> DPRINTF("%s: %s %d received relay fd %d type %d for relay %s", __func__, >> blob - 22beb857229a16e5b2c17a25a2944231d41e7e08 >> blob + fe5e8ff4dfed10e8f09e3226bdfe33f8bc031c8e >> --- usr.sbin/relayd/parse.y >> +++ usr.sbin/relayd/parse.y >> @@ -172,14 +172,14 @@ typedef struct { >> %tokenCODE COOKIE DEMOTE DIGEST DISABLE ERROR EXPECT PASS BLOCK EXTERNAL >> %tokenFILENAME FORWARD FROM HASH HEADER HEADERLEN HOST HTTP ICMP INCLUDE >> INET >> %tokenINET6 INTERFACE INTERVAL IP KEYPAIR LABEL LISTEN VALUE LOADBALANCE >> LOG >> -%tokenLOOKUP METHOD MODE NAT NO DESTINATION NODELAY NOTHING ON PARENT >> PATH >> +%tokenLOOKUP METHOD MODE NAT NO DESTINATION NODELAY NOTHING ON OPTIONAL >> PARENT PATH >> %tokenPFTAG PORT PREFORK PRIORITY PROTO QUERYSTR REAL REDIRECT RELAY >> REMOVE >> %tokenREQUEST RESPONSE RETRY QUICK RETURN ROUNDROBIN ROUTE SACK SCRIPT >> SEND >> %tokenSESSION SOCKET SPLICE SSL STICKYADDR STRIP STYLE TABLE TAG TAGGED >> TCP >> %tokenTIMEOUT TLS TO ROUTER RTLABEL TRANSPARENT URL WITH TTL RTABLE >> %tokenMATCH PARAMS RANDOM LEASTSTATES SRCHASH KEY CERTIFICATE PASSWORD >> ECDHE >> %tokenEDH TICKETS CONNECTION CONNECTIONS CONTEXT ERRORS STATE CHANGES >> CHECKS >> -%tokenWEBSOCKETS >> +%tokenWEBSOCKETS CLIENT >> %tokenSTRING >> %token NUMBER >> %typecontext hostname interface table value path >> @