Re: unwind(8): only use available address families
On Tue, Jan 26, 2021 at 05:53:26PM +0100, Klemens Nanni wrote:
> On Tue, Jan 26, 2021 at 05:22:42PM +0100, Florian Obser wrote:
> > On Mon, Jan 25, 2021 at 07:05:40PM +0100, Florian Obser wrote:
> > > Unwind / libunbound goes pretty badly off the rails when an address
> > > family is not available, it still tries to talk to nameservers with an
> > > unreachable address family.
> > > I don't think it's libunbound's place to figure this out. It can't
> > > sensibly do a getifaddrs on every query...
> > > So let's help it out a bit.
> > >
> > > OK?
> >
> > This is better.
> >
> > - 100% less ioctl, leading to tighter pledge after clue-bat from
> > claudio while working in asr.
> > - also handle RTM_DESYNC, pointed out by deraadt
> Cool, it works as intended and is stable (after the other fix) for me.
>
> > I was also toying with the idea of counting arivals and departure of
> > IP addresses using the routing socket, but getting the account right
> > seems complicated. I don't think a call to getifaddrs will be
> > triggered that often.
> I think that's fair.
>
> Two things inline, with that
> OK kn
>
> > + for (ifa = ifap; ifa != NULL; ifa = ifa->ifa_next) {
> > + if (ifa->ifa_addr == NULL)
> > + continue;
> > + switch(ifa->ifa_addr->sa_family) {
> > + case AF_LINK:
> Can you add the same comment here as well as you did in asr?
> /* AF_LINK comes before inet / inet6 on an interface */
Sure, thanks.
>
> > + ifa_data = (struct if_data *)ifa->ifa_data;
> > + ifa_rtable = ifa_data->ifi_rdomain;
> > + break;
> > + case AF_INET:
> > + if (ifa_rtable != rtable)
> > + continue;
>
>
> > rtfilter = ROUTE_FILTER(RTM_IFINFO) | ROUTE_FILTER(RTM_PROPOSAL)
> > - | ROUTE_FILTER(RTM_IFANNOUNCE);
> > + | ROUTE_FILTER(RTM_IFANNOUNCE) | ROUTE_FILTER(RTM_NEWADDR)
> > + | ROUTE_FILTER(RTM_DELADDR);
> Looks like you missed `RTM_DESYNC' here.
Nope, see rtsock.c:
534 /* filter messages that the process does not want */
535 rtm = mtod(m, struct rt_msghdr *);
536 /* but RTM_DESYNC can't be filtered */
537 if (rtm->rtm_type != RTM_DESYNC) {
>
> > if (setsockopt(frontend_routesock, AF_ROUTE, ROUTE_MSGFILTER,
> > &rtfilter, sizeof(rtfilter)) == -1)
> > fatal("setsockopt(ROUTE_MSGFILTER)");
>
--
I'm not entirely sure you are real.
Re: unwind(8): only use available address families
On Tue, Jan 26, 2021 at 05:22:42PM +0100, Florian Obser wrote:
> On Mon, Jan 25, 2021 at 07:05:40PM +0100, Florian Obser wrote:
> > Unwind / libunbound goes pretty badly off the rails when an address
> > family is not available, it still tries to talk to nameservers with an
> > unreachable address family.
> > I don't think it's libunbound's place to figure this out. It can't
> > sensibly do a getifaddrs on every query...
> > So let's help it out a bit.
> >
> > OK?
>
> This is better.
>
> - 100% less ioctl, leading to tighter pledge after clue-bat from
> claudio while working in asr.
> - also handle RTM_DESYNC, pointed out by deraadt
Cool, it works as intended and is stable (after the other fix) for me.
> I was also toying with the idea of counting arivals and departure of
> IP addresses using the routing socket, but getting the account right
> seems complicated. I don't think a call to getifaddrs will be
> triggered that often.
I think that's fair.
Two things inline, with that
OK kn
> + for (ifa = ifap; ifa != NULL; ifa = ifa->ifa_next) {
> + if (ifa->ifa_addr == NULL)
> + continue;
> + switch(ifa->ifa_addr->sa_family) {
> + case AF_LINK:
Can you add the same comment here as well as you did in asr?
/* AF_LINK comes before inet / inet6 on an interface */
> + ifa_data = (struct if_data *)ifa->ifa_data;
> + ifa_rtable = ifa_data->ifi_rdomain;
> + break;
> + case AF_INET:
> + if (ifa_rtable != rtable)
> + continue;
> rtfilter = ROUTE_FILTER(RTM_IFINFO) | ROUTE_FILTER(RTM_PROPOSAL)
> - | ROUTE_FILTER(RTM_IFANNOUNCE);
> + | ROUTE_FILTER(RTM_IFANNOUNCE) | ROUTE_FILTER(RTM_NEWADDR)
> + | ROUTE_FILTER(RTM_DELADDR);
Looks like you missed `RTM_DESYNC' here.
> if (setsockopt(frontend_routesock, AF_ROUTE, ROUTE_MSGFILTER,
> &rtfilter, sizeof(rtfilter)) == -1)
> fatal("setsockopt(ROUTE_MSGFILTER)");
Re: unwind(8): only use available address families
On Mon, Jan 25, 2021 at 07:05:40PM +0100, Florian Obser wrote:
> Unwind / libunbound goes pretty badly off the rails when an address
> family is not available, it still tries to talk to nameservers with an
> unreachable address family.
> I don't think it's libunbound's place to figure this out. It can't
> sensibly do a getifaddrs on every query...
> So let's help it out a bit.
>
> OK?
This is better.
- 100% less ioctl, leading to tighter pledge after clue-bat from
claudio while working in asr.
- also handle RTM_DESYNC, pointed out by deraadt
I was also toying with the idea of counting arivals and departure of
IP addresses using the routing socket, but getting the account right
seems complicated. I don't think a call to getifaddrs will be
triggered that often.
diff --git frontend.c frontend.c
index 50dab6c70ca..b25fd76fe2e 100644
--- frontend.c
+++ frontend.c
@@ -32,6 +32,7 @@
#include
#include
+#include
#include
#include
#include
@@ -150,6 +151,7 @@ void parse_blocklist(int);
int bl_cmp(struct bl_node *, struct bl_node *);
voidfree_bl(void);
int pending_query_cnt(void);
+voidcheck_available_af(void);
struct uw_conf *frontend_conf;
static struct imsgev *iev_main;
@@ -212,7 +214,7 @@ frontend(int debug, int verbose)
setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid))
fatal("can't drop privileges");
- if (pledge("stdio unix recvfd", NULL) == -1)
+ if (pledge("stdio dns unix recvfd", NULL) == -1)
fatal("pledge");
event_init();
@@ -660,6 +662,7 @@ frontend_startup(void)
event_add(&ev_route, NULL);
frontend_imsg_compose_main(IMSG_STARTUP_DONE, 0, NULL, 0);
+ check_available_af();
}
void
@@ -1362,6 +1365,11 @@ handle_route_message(struct rt_msghdr *rtm, struct
sockaddr **rti_info)
frontend_imsg_compose_resolver(IMSG_REPLACE_DNS, 0,
&rdns_proposal, sizeof(rdns_proposal));
break;
+ case RTM_NEWADDR:
+ case RTM_DELADDR:
+ case RTM_DESYNC:
+ check_available_af();
+ break;
default:
break;
}
@@ -1765,3 +1773,66 @@ tcp_timeout(int fd, short events, void *arg)
{
free_pending_query(arg);
}
+
+void
+check_available_af()
+{
+ static int available_af = HAVE_IPV4 | HAVE_IPV6;
+ static int rtable = -1;
+ struct ifaddrs *ifap, *ifa;
+ struct if_data *ifa_data;
+ struct sockaddr_in *sin4;
+ struct sockaddr_in6 *sin6;
+ int new_available_af = 0, ifa_rtable = -1;
+
+ if (rtable == -1)
+ rtable = getrtable();
+
+ if (getifaddrs(&ifap) != 0) {
+ log_warn("getifaddrs");
+ return;
+ }
+
+ for (ifa = ifap; ifa != NULL; ifa = ifa->ifa_next) {
+ if (ifa->ifa_addr == NULL)
+ continue;
+ switch(ifa->ifa_addr->sa_family) {
+ case AF_LINK:
+ ifa_data = (struct if_data *)ifa->ifa_data;
+ ifa_rtable = ifa_data->ifi_rdomain;
+ break;
+ case AF_INET:
+ if (ifa_rtable != rtable)
+ continue;
+
+ sin4 = (struct sockaddr_in *)ifa->ifa_addr;
+ if ((ntohl(sin4->sin_addr.s_addr) >> 24) ==
+ IN_LOOPBACKNET)
+ continue;
+ new_available_af |= HAVE_IPV4;
+ break;
+ case AF_INET6:
+ if (ifa_rtable != rtable)
+ continue;
+
+ sin6 = (struct sockaddr_in6 *)ifa->ifa_addr;
+ if (IN6_IS_ADDR_LOOPBACK(&sin6->sin6_addr) ||
+ IN6_IS_ADDR_LINKLOCAL(&sin6->sin6_addr) ||
+ IN6_IS_ADDR_MC_LINKLOCAL(&sin6->sin6_addr) ||
+ IN6_IS_ADDR_MC_INTFACELOCAL(&sin6->sin6_addr))
+ continue;
+ new_available_af |= HAVE_IPV6;
+ break;
+ default:
+ break;
+ }
+ if (new_available_af == (HAVE_IPV4 | HAVE_IPV6))
+ break;
+ }
+ freeifaddrs(ifap);
+ if (new_available_af != available_af) {
+ available_af = new_available_af;
+ frontend_imsg_compose_resolver(IMSG_CHANGE_AFS, 0,
+ &available_af, sizeof(available_af));
+ }
+}
diff --git frontend.h frontend.h
index cd6c21875af..0c18c7524dc 100644
--- frontend.h
+++ frontend.h
@@ -17,6 +17,9 @@
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWAR
Re: unwind(8): only use available address families
On Mon, Jan 25, 2021 at 08:56:36PM +0100, Klemens Nanni wrote: > On Mon, Jan 25, 2021 at 07:05:40PM +0100, Florian Obser wrote: > > Unwind / libunbound goes pretty badly off the rails when an address > > family is not available, it still tries to talk to nameservers with an > > unreachable address family. > > I don't think it's libunbound's place to figure this out. It can't > > sensibly do a getifaddrs on every query... > > So let's help it out a bit. > Cool, this is pretty much what I had in mind after your reply to the > other thread. > > Have you tested this by adding some IPv4 address? unwind dies on me: > > $ ifconfig all inet | grep -w inet > inet 127.0.0.1 netmask 0xff00 > $ doas ifconfig tun0 1.2.3.4 > > check_available_af 3 - 2 > fatal in resolver: new_resolver: invalid resolver state: dead > frontend exiting > waiting for children to terminate > terminating > This has been fixed, and it was indeed unreleated to this diff. We also need to restart dead resolvers when we restart all resolvers because of a config change in libunbound. (e.g. changing of log level or new trust anchors). -- I'm not entirely sure you are real.
Re: unwind(8): only use available address families
On 25 January 2021 20:56:36 CET, Klemens Nanni wrote: >On Mon, Jan 25, 2021 at 07:05:40PM +0100, Florian Obser wrote: >> Unwind / libunbound goes pretty badly off the rails when an address >> family is not available, it still tries to talk to nameservers with >an >> unreachable address family. >> I don't think it's libunbound's place to figure this out. It can't >> sensibly do a getifaddrs on every query... >> So let's help it out a bit. >Cool, this is pretty much what I had in mind after your reply to the >other thread. > >Have you tested this by adding some IPv4 address? unwind dies on me: Yes, that's actually the only thing I tried, flipping v4 on and off. > > $ ifconfig all inet | grep -w inet > inet 127.0.0.1 netmask 0xff00 > $ doas ifconfig tun0 1.2.3.4 > > check_available_af 3 - 2 > fatal in resolver: new_resolver: invalid resolver state: dead Pretty sure this isn't a bug in this diff, it just gets exposed more easily. I also see that I left too much debug code in there... I'll have a look tomorrow. > frontend exiting > waiting for children to terminate > terminating -- Sent from a mobile device. Please excuse poor formating.
Re: unwind(8): only use available address families
On Mon, Jan 25, 2021 at 07:05:40PM +0100, Florian Obser wrote: > Unwind / libunbound goes pretty badly off the rails when an address > family is not available, it still tries to talk to nameservers with an > unreachable address family. > I don't think it's libunbound's place to figure this out. It can't > sensibly do a getifaddrs on every query... > So let's help it out a bit. Cool, this is pretty much what I had in mind after your reply to the other thread. Have you tested this by adding some IPv4 address? unwind dies on me: $ ifconfig all inet | grep -w inet inet 127.0.0.1 netmask 0xff00 $ doas ifconfig tun0 1.2.3.4 check_available_af 3 - 2 fatal in resolver: new_resolver: invalid resolver state: dead frontend exiting waiting for children to terminate terminating
unwind(8): only use available address families
Unwind / libunbound goes pretty badly off the rails when an address
family is not available, it still tries to talk to nameservers with an
unreachable address family.
I don't think it's libunbound's place to figure this out. It can't
sensibly do a getifaddrs on every query...
So let's help it out a bit.
OK?
(I'm not 100% sure where to place check_available_af(). Maybe this
should go to the main process.)
diff --git frontend.c frontend.c
index 50dab6c70ca..c5e5da91b9a 100644
--- frontend.c
+++ frontend.c
@@ -20,6 +20,7 @@
*/
#include
+#include
#include
#include
#include
@@ -32,6 +33,7 @@
#include
#include
+#include
#include
#include
#include
@@ -150,6 +152,8 @@ void parse_blocklist(int);
int bl_cmp(struct bl_node *, struct bl_node *);
voidfree_bl(void);
int pending_query_cnt(void);
+int get_ifrdomain(char *);
+voidcheck_available_af(void);
struct uw_conf *frontend_conf;
static struct imsgev *iev_main;
@@ -158,6 +162,7 @@ struct event ev_route;
int udp4sock = -1, udp6sock = -1;
int tcp4sock = -1, tcp6sock = -1;
int ta_fd = -1;
+int ioctlsock;
static struct trust_anchor_head trust_anchors, new_trust_anchors;
@@ -212,7 +217,10 @@ frontend(int debug, int verbose)
setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid))
fatal("can't drop privileges");
- if (pledge("stdio unix recvfd", NULL) == -1)
+ if ((ioctlsock = socket(AF_INET6, SOCK_DGRAM, 0)) == -1)
+ fatal("socket");
+
+ if (pledge("stdio unix recvfd route", NULL) == -1)
fatal("pledge");
event_init();
@@ -660,6 +668,7 @@ frontend_startup(void)
event_add(&ev_route, NULL);
frontend_imsg_compose_main(IMSG_STARTUP_DONE, 0, NULL, 0);
+ check_available_af();
}
void
@@ -1362,6 +1371,10 @@ handle_route_message(struct rt_msghdr *rtm, struct
sockaddr **rti_info)
frontend_imsg_compose_resolver(IMSG_REPLACE_DNS, 0,
&rdns_proposal, sizeof(rdns_proposal));
break;
+ case RTM_NEWADDR:
+ case RTM_DELADDR:
+ check_available_af();
+ break;
default:
break;
}
@@ -1765,3 +1778,72 @@ tcp_timeout(int fd, short events, void *arg)
{
free_pending_query(arg);
}
+
+int
+get_ifrdomain(char *if_name)
+{
+ struct ifreq ifr;
+
+ strlcpy(ifr.ifr_name, if_name, sizeof(ifr.ifr_name));
+ if (ioctl(ioctlsock, SIOCGIFRDOMAIN, (caddr_t)&ifr) == -1) {
+ log_warn("SIOCGIFRDOMAIN");
+ return -1;
+ }
+ return ifr.ifr_rdomainid;
+}
+
+void
+check_available_af()
+{
+ static int available_af = HAVE_IPV4 | HAVE_IPV6;
+ static int rtable = -1;
+ struct ifaddrs *ifap, *ifa;
+ struct sockaddr_in *sin4;
+ struct sockaddr_in6 *sin6;
+ int new_available_af = 0;
+
+ if (rtable == -1)
+ rtable = getrtable();
+
+ if (getifaddrs(&ifap) != 0) {
+ log_warn("getifaddrs");
+ return;
+ }
+
+ for (ifa = ifap; ifa != NULL; ifa = ifa->ifa_next) {
+ if (get_ifrdomain(ifa->ifa_name) != getrtable())
+ continue;
+ if (ifa->ifa_addr == NULL)
+ continue;
+ switch(ifa->ifa_addr->sa_family) {
+ case AF_INET:
+ sin4 = (struct sockaddr_in *)ifa->ifa_addr;
+ if ((ntohl(sin4->sin_addr.s_addr) >> 24) ==
+ IN_LOOPBACKNET)
+ continue;
+ new_available_af |= HAVE_IPV4;
+ break;
+ case AF_INET6:
+ sin6 = (struct sockaddr_in6 *)ifa->ifa_addr;
+ if (IN6_IS_ADDR_LOOPBACK(&sin6->sin6_addr) ||
+ IN6_IS_ADDR_LINKLOCAL(&sin6->sin6_addr) ||
+ IN6_IS_ADDR_MC_LINKLOCAL(&sin6->sin6_addr) ||
+ IN6_IS_ADDR_MC_INTFACELOCAL(&sin6->sin6_addr))
+ continue;
+ new_available_af |= HAVE_IPV6;
+ break;
+ default:
+ break;
+ }
+ if (new_available_af == (HAVE_IPV4 | HAVE_IPV6))
+ break;
+ }
+ freeifaddrs(ifap);
+ if (new_available_af != available_af) {
+ log_debug("%s %d - %d", __func__, new_available_af,
+ available_af);
+ available_af = new_available_af;
+ frontend_imsg_compose_resolver(IMSG_CHANGE_AFS, 0,
