Opportunistic DoT for unwind(8)

classic Classic list List threaded Threaded
23 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Opportunistic DoT for unwind(8)

Otto Moerbeek
Hi,

The patch below add opportunistic DoT to unwind.  Some background
info:

The purpose of unwind is to provide secure DNS services even when
the available nameservers are broken or filtered like in many hotels.
To do that, it prefers DNSSEC whenever possible and changes to do
resolving by itself if needed.

DNSSEC only offers integrity and authenticity.  To protect
eavesdropping on the requests in transit, encryption is needed, as
offered by e.g. DNS over TLS (DoT) and DNS over HTTP (DoT). unwind
already supports DoT for forwarders but only when explicitly
configured to do so.

This diff makes unwind try DoT for forwarders and nameservers learned
via DHCP and fall back to plaintext if it does not work.

Which DoT servers to use for testing? Most quad-whatever open DNS
resolvers like 9.9.9.9 support DoT so you can specify them as a
forwarder.  But please note the privacy policies and realize they see
all your DNS queries.  You might consider reading [1] which offers DoT
on 136.144.215.158 as well and has a good privacy policy and is in
Dutch jurisdiction.  If you do want to run DoT yourself there's
dnsdist in ports.  Some ISPs offer DoT as well.

How to test?
============

Apply the diff in /usr/src and rebuild sbin/unwind and usr.sbin/unwindctl.

- read the unwind(8) man page about changing dhclient.conf if you use
  DHCP (this is not related to this diff but important anyway).
- make sure that if you have an existing working setup it does not
  break with this diff.
- check if DoT is used when appriopiate by using the unwindctl command.

Even if you do not want to run DoT at least make sure existing stuff
does not break.  I might need to add a config option to tune the
always on behaviour of opportunistic DoT, but only want to do that if
I have evidence it is needed.

Happy testing!

        -Otto

[1] https://doh.powerdns.org/doh/privacy.html


Index: sbin/unwind/parse.y
===================================================================
RCS file: /cvs/src/sbin/unwind/parse.y,v
retrieving revision 1.11
diff -u -p -r1.11 parse.y
--- sbin/unwind/parse.y 21 Oct 2019 07:16:09 -0000 1.11
+++ sbin/unwind/parse.y 23 Oct 2019 12:28:42 -0000
@@ -84,7 +84,6 @@ int check_pref_uniq(enum uw_resolver_ty
 
 static struct uw_conf *conf;
 static int errors;
-static struct uw_forwarder *uw_forwarder;
 
 void clear_config(struct uw_conf *xconf);
 struct sockaddr_storage *host_ip(const char *);
@@ -286,7 +285,9 @@ forwarderopts_l : forwarderopts_l forwa
 
 forwarderoptsl : STRING port authname dot {
  int ret, port;
+ struct uw_forwarder *uw_fwd;
  struct sockaddr_storage *ss;
+
  if ((ss = host_ip($1)) == NULL) {
  yyerror("%s is not an ip-address", $1);
  free($1);
@@ -304,37 +305,53 @@ forwarderoptsl : STRING port authname d
  else
  port = $2;
 
- if ((uw_forwarder = calloc(1,
-    sizeof(*uw_forwarder))) == NULL)
+ if ($3 != NULL && $4 == 0) {
+ yyerror("authentication name can only "
+    "be used with DoT");
+ free($1);
+ YYERROR;
+ }
+
+
+ if ((uw_fwd = calloc(1,
+    sizeof(*uw_fwd))) == NULL)
  err(1, NULL);
 
- if ($3 == NULL)
- ret = snprintf(uw_forwarder->name,
-    sizeof(uw_forwarder->name),
-    "%s@%d", $1, port);
- else
- ret = snprintf(uw_forwarder->name,
-    sizeof(uw_forwarder->name),
-    "%s@%d#%s", $1, port, $3);
+ if ($4 == DOT) {
+ if ($3 == NULL)
+ ret = snprintf(uw_fwd->name,
+    sizeof(uw_fwd->name),
+    "%s@%d", $1, port);
+ else
+ ret = snprintf(uw_fwd->name,
+    sizeof(uw_fwd->name),
+    "%s@%d#%s", $1, port, $3);
+ } else {
+ uw_fwd->port = $2;
+ /* complete string wil be done later */
+ ret = snprintf(uw_fwd->name,
+    sizeof(uw_fwd->name), "%s", $1);
+ }
  if (ret < 0 || (size_t)ret >=
-    sizeof(uw_forwarder->name)) {
- free(uw_forwarder);
+    sizeof(uw_fwd->name)) {
+ free(uw_fwd);
  yyerror("forwarder %s too long", $1);
  free($1);
  YYERROR;
  }
- free($1);
 
  if ($4 == DOT)
  SIMPLEQ_INSERT_TAIL(
     &conf->uw_dot_forwarder_list,
-    uw_forwarder, entry);
- else
+    uw_fwd, entry);
+ else {
  SIMPLEQ_INSERT_TAIL(
     &conf->uw_forwarder_list,
-    uw_forwarder, entry);
+    uw_fwd, entry);
+ }
+ free($1);
  }
- ;
+ ;
 
 port : PORT NUMBER { $$ = $2; }
  | /* empty */ { $$ = 0; }
@@ -872,12 +889,14 @@ symget(const char *nam)
 void
 clear_config(struct uw_conf *xconf)
 {
- while((uw_forwarder = SIMPLEQ_FIRST(&xconf->uw_forwarder_list)) !=
+ struct uw_forwarder *uw_forwarder;
+
+ while ((uw_forwarder = SIMPLEQ_FIRST(&xconf->uw_forwarder_list)) !=
     NULL) {
  SIMPLEQ_REMOVE_HEAD(&xconf->uw_forwarder_list, entry);
  free(uw_forwarder);
  }
- while((uw_forwarder = SIMPLEQ_FIRST(&xconf->uw_dot_forwarder_list)) !=
+ while ((uw_forwarder = SIMPLEQ_FIRST(&xconf->uw_dot_forwarder_list)) !=
     NULL) {
  SIMPLEQ_REMOVE_HEAD(&xconf->uw_dot_forwarder_list, entry);
  free(uw_forwarder);
Index: sbin/unwind/resolver.c
===================================================================
RCS file: /cvs/src/sbin/unwind/resolver.c,v
retrieving revision 1.46
diff -u -p -r1.46 resolver.c
--- sbin/unwind/resolver.c 19 Oct 2019 17:42:21 -0000 1.46
+++ sbin/unwind/resolver.c 23 Oct 2019 12:28:42 -0000
@@ -107,6 +107,8 @@ struct uw_resolver *create_resolver(enum
 void free_resolver(struct uw_resolver *);
 void set_forwarders(struct uw_resolver *,
      struct uw_forwarder_head *);
+void set_forwarders_oppdot(struct uw_resolver *,
+     struct uw_forwarder_head *, int);
 void resolver_check_timo(int, short, void *);
 void resolver_free_timo(int, short, void *);
 void check_resolver(struct uw_resolver *);
@@ -156,6 +158,7 @@ struct event_base *ev_base;
 
 enum uw_resolver_state global_state = DEAD;
 enum captive_portal_state captive_portal_state = PORTAL_UNCHECKED;
+int dhcp_oppdot, forw_oppdot;
 
 void
 resolver_sig_handler(int sig, short event, void *arg)
@@ -605,6 +608,7 @@ resolver_dispatch_main(int fd, short eve
  nconf = NULL;
  if (forwarders_changed) {
  log_debug("static forwarders changed");
+ forw_oppdot = 1;
  new_static_forwarders();
  }
  if (dot_forwarders_changed) {
@@ -726,7 +730,7 @@ parse_dhcp_forwarders(char *forwarders)
  if (forwarders != NULL) {
  while((ns = strsep(&forwarders, ",")) != NULL) {
  log_debug("%s: %s", __func__, ns);
- if ((uw_forwarder = malloc(sizeof(struct
+ if ((uw_forwarder = calloc(1, sizeof(struct
     uw_forwarder))) == NULL)
  fatal(NULL);
  if (strlcpy(uw_forwarder->name, ns,
@@ -740,6 +744,7 @@ parse_dhcp_forwarders(char *forwarders)
 
  if (check_forwarders_changed(&new_forwarder_list,
     &dhcp_forwarder_list)) {
+ dhcp_oppdot = 1;
  replace_forwarders(&new_forwarder_list, &dhcp_forwarder_list);
  new_forwarders();
  if (resolver_conf->captive_portal_auto)
@@ -886,10 +891,24 @@ create_resolver(enum uw_resolver_type ty
  case UW_RES_RECURSOR:
  break;
  case UW_RES_DHCP:
- set_forwarders(res, &dhcp_forwarder_list);
+ if (dhcp_oppdot) {
+ set_forwarders_oppdot(res, &dhcp_forwarder_list, 853);
+ ub_ctx_set_option(res->ctx, "tls-cert-bundle:",
+    tls_default_ca_cert_file());
+ ub_ctx_set_tls(res->ctx, 1);
+ } else
+ set_forwarders_oppdot(res, &dhcp_forwarder_list, 53);
  break;
  case UW_RES_FORWARDER:
- set_forwarders(res, &resolver_conf->uw_forwarder_list);
+ if (forw_oppdot) {
+ set_forwarders_oppdot(res,
+    &resolver_conf->uw_forwarder_list, 853);
+ ub_ctx_set_option(res->ctx, "tls-cert-bundle:",
+    tls_default_ca_cert_file());
+ ub_ctx_set_tls(res->ctx, 1);
+ } else
+ set_forwarders_oppdot(res,
+    &resolver_conf->uw_forwarder_list, 53);
  break;
  case UW_RES_DOT:
  set_forwarders(res, &resolver_conf->uw_dot_forwarder_list);
@@ -934,6 +953,22 @@ set_forwarders(struct uw_resolver *res,
 }
 
 void
+set_forwarders_oppdot(struct uw_resolver *res, struct uw_forwarder_head
+    *uw_forwarder_list, int def_port)
+{
+ struct uw_forwarder *uw_forwarder;
+
+ SIMPLEQ_FOREACH(uw_forwarder, uw_forwarder_list, entry) {
+ char name[1024];
+ int port = uw_forwarder->port;
+ if (port == 0)
+ port = def_port;
+ snprintf(name, sizeof(name), "%s@%d", uw_forwarder->name, port);
+ ub_ctx_set_fwd(res->ctx, name);
+ }
+}
+
+void
 resolver_check_timo(int fd, short events, void *arg)
 {
  check_resolver((struct uw_resolver *)arg);
@@ -1005,6 +1040,21 @@ check_resolver_done(void *arg, int rcode
 
  if (rcode == LDNS_RCODE_SERVFAIL) {
  data->res->state = DEAD;
+ if (data->res->type == UW_RES_DHCP) {
+ if (dhcp_oppdot) {
+ dhcp_oppdot = 0;
+ log_debug("dhcp_oppdot -> 0");
+ new_forwarders();
+ log_debug("dhcp_oppdot -> 0 done");
+ }
+ } else {
+ if (forw_oppdot) {
+ forw_oppdot = 0;
+ log_debug("forw_oppdot -> 0");
+ new_static_forwarders();
+ log_debug("forw_oppdot -> 0 done");
+ }
+ }
  goto out;
  }
 
@@ -1176,8 +1226,8 @@ best_resolver(void)
     uw_resolver_state_str[resolvers[UW_RES_FORWARDER]->state] : "NA",
     uw_resolver_type_str[UW_RES_DOT],
     resolvers[UW_RES_DOT] != NULL ?
-    uw_resolver_state_str[resolvers[UW_RES_DOT]->state] :
-    "NA", captive_portal_state_str[captive_portal_state]);
+    uw_resolver_state_str[resolvers[UW_RES_DOT]->state] : "NA",
+    captive_portal_state_str[captive_portal_state]);
 
  if (captive_portal_state == PORTAL_UNKNOWN || captive_portal_state ==
     BEHIND) {
@@ -1273,6 +1323,12 @@ send_resolver_info(struct uw_resolver *r
  cri.state = res->state;
  cri.type = res->type;
  cri.selected = selected;
+ if (cri.type == UW_RES_DHCP && dhcp_oppdot)
+ cri.oppdot = 1;
+ else if (cri.type == UW_RES_FORWARDER && forw_oppdot)
+ cri.oppdot = 1;
+ else
+ cri.oppdot = 0;
  resolver_imsg_compose_frontend(IMSG_CTL_RESOLVER_INFO, pid, &cri,
     sizeof(cri));
 }
Index: sbin/unwind/resolver.h
===================================================================
RCS file: /cvs/src/sbin/unwind/resolver.h,v
retrieving revision 1.6
diff -u -p -r1.6 resolver.h
--- sbin/unwind/resolver.h 2 Apr 2019 07:47:23 -0000 1.6
+++ sbin/unwind/resolver.h 23 Oct 2019 12:28:42 -0000
@@ -51,6 +51,7 @@ struct ctl_resolver_info {
  enum uw_resolver_state state;
  enum uw_resolver_type type;
  int selected;
+ int oppdot;
 };
 
 void resolver(int, int);
Index: sbin/unwind/unwind.h
===================================================================
RCS file: /cvs/src/sbin/unwind/unwind.h,v
retrieving revision 1.18
diff -u -p -r1.18 unwind.h
--- sbin/unwind/unwind.h 21 Oct 2019 07:16:09 -0000 1.18
+++ sbin/unwind/unwind.h 23 Oct 2019 12:28:42 -0000
@@ -127,6 +127,7 @@ enum imsg_type {
 struct uw_forwarder {
  SIMPLEQ_ENTRY(uw_forwarder) entry;
  char name[1024]; /* XXX */
+ uint16_t port;
 };
 
 SIMPLEQ_HEAD(uw_forwarder_head, uw_forwarder);
Index: usr.sbin/unwindctl/parser.c
===================================================================
RCS file: /cvs/src/usr.sbin/unwindctl/parser.c,v
retrieving revision 1.3
diff -u -p -r1.3 parser.c
--- usr.sbin/unwindctl/parser.c 3 Feb 2019 12:02:30 -0000 1.3
+++ usr.sbin/unwindctl/parser.c 23 Oct 2019 12:28:42 -0000
@@ -72,7 +72,7 @@ static const struct token t_status[] = {
  {NOTOKEN, "", NONE, NULL},
  {KEYWORD, "recursor", STATUS_RECURSOR, NULL},
  {KEYWORD, "dhcp", STATUS_DHCP, NULL},
- {KEYWORD, "static", STATUS_STATIC, NULL},
+ {KEYWORD, "forwarder", STATUS_STATIC, NULL},
  {KEYWORD, "DoT", STATUS_DOT, NULL},
  {ENDTOKEN, "", STATUS, NULL}
 };
Index: usr.sbin/unwindctl/unwindctl.8
===================================================================
RCS file: /cvs/src/usr.sbin/unwindctl/unwindctl.8,v
retrieving revision 1.4
diff -u -p -r1.4 unwindctl.8
--- usr.sbin/unwindctl/unwindctl.8 5 Feb 2019 20:41:12 -0000 1.4
+++ usr.sbin/unwindctl/unwindctl.8 23 Oct 2019 12:28:42 -0000
@@ -55,14 +55,14 @@ Enable very noisy debug logging.
 Reload the configuration file.
 .It Cm recheck portal
 Run the captive portal detection.
-.It Cm status Op Cm recursor | dhcp | DoT | static
+.It Cm status Op Cm recursor | dhcp | DoT | forwarder
 Show a status summary.
 If one of
 .Cm recursor ,
 .Cm Dhcp ,
 .Cm Dot ,
 Or
-.Cm Static
+.Cm forwarder
 is specified, more detailed information about that type of resolver is given
 including reasons why DNSSEC validation might be failing and a query time
 histogram.
Index: usr.sbin/unwindctl/unwindctl.c
===================================================================
RCS file: /cvs/src/usr.sbin/unwindctl/unwindctl.c,v
retrieving revision 1.6
diff -u -p -r1.6 unwindctl.c
--- usr.sbin/unwindctl/unwindctl.c 2 Apr 2019 07:47:23 -0000 1.6
+++ usr.sbin/unwindctl/unwindctl.c 23 Oct 2019 12:28:42 -0000
@@ -241,9 +241,10 @@ show_status_msg(struct imsg *imsg)
  break;
  case IMSG_CTL_RESOLVER_INFO:
  cri = imsg->data;
- printf("%8s %16s %s\n", cri->selected ? "*" : " ",
+ printf("%8s %16s %s%s\n", cri->selected ? "*" : " ",
     uw_resolver_type_str[cri->type],
-    uw_resolver_state_str[cri->state]);
+    uw_resolver_state_str[cri->state],
+    cri->oppdot ? " (OppDoT)" : "");
  break;
  case IMSG_CTL_RESOLVER_WHY_BOGUS:
  /* make sure this is a string */

Reply | Threaded
Open this post in threaded view
|

Re: Opportunistic DoT for unwind(8)

Otto Moerbeek
On Wed, Oct 23, 2019 at 02:40:53PM +0200, Otto Moerbeek wrote:

> [1] https://doh.powerdns.org/doh/privacy.html

Should be https://powerdns.org/doh/privacy.html

Reply | Threaded
Open this post in threaded view
|

Re: Opportunistic DoT for unwind(8)

Otto Moerbeek
In reply to this post by Otto Moerbeek
On Wed, Oct 23, 2019 at 02:40:53PM +0200, Otto Moerbeek wrote:

> Hi,
>
> The patch below add opportunistic DoT to unwind.  Some background
> info:
>
> The purpose of unwind is to provide secure DNS services even when
> the available nameservers are broken or filtered like in many hotels.
> To do that, it prefers DNSSEC whenever possible and changes to do
> resolving by itself if needed.
>
> DNSSEC only offers integrity and authenticity.  To protect
> eavesdropping on the requests in transit, encryption is needed, as
> offered by e.g. DNS over TLS (DoT) and DNS over HTTP (DoT). unwind

That second DoT should be DoH.

> already supports DoT for forwarders but only when explicitly
> configured to do so.
>
> This diff makes unwind try DoT for forwarders and nameservers learned
> via DHCP and fall back to plaintext if it does not work.
>
> Which DoT servers to use for testing? Most quad-whatever open DNS
> resolvers like 9.9.9.9 support DoT so you can specify them as a
> forwarder.  But please note the privacy policies and realize they see
> all your DNS queries.  You might consider reading [1] which offers DoT
> on 136.144.215.158 as well and has a good privacy policy and is in

Not that the mentioned privacy page talsk about DoH and does not
mention DoT, but the IP does offer DoT.

[1] https://powerdns.org/doh/privacy.html

        -Otto


Reply | Threaded
Open this post in threaded view
|

Re: Opportunistic DoT for unwind(8)

Kevin Chadwick-4

> The purpose of unwind is to provide secure DNS services even when
> the available nameservers are broken or filtered like in many hotels.
> To do that, it prefers DNSSEC whenever possible and changes to do
> resolving by itself if needed.
>
> DNSSEC only offers integrity and authenticity.  To protect
> eavesdropping on the requests in transit, encryption is needed, as
> offered by e.g. DNS over TLS (DoT) and DNS over HTTP (DoT). unwind

Before I jump aboard with DNSSECs failings in mind on my own networks rather
than the mentioned hotel scenario. I believe but I am still not certain that
services like PowerDNS have secure channels to the main primary DNS servers that
apparently do not scale for the rest of us? Otherwise I worry that the network
security target is a more singular centralised target compared to e.g. unbound.

Reply | Threaded
Open this post in threaded view
|

Re: Opportunistic DoT for unwind(8)

Paul de Weerd
On Thu, Oct 24, 2019 at 11:27:24AM +0100, Kevin Chadwick wrote:
|
| > The purpose of unwind is to provide secure DNS services even when
| > the available nameservers are broken or filtered like in many hotels.
| > To do that, it prefers DNSSEC whenever possible and changes to do
| > resolving by itself if needed.
| >
| > DNSSEC only offers integrity and authenticity.  To protect
| > eavesdropping on the requests in transit, encryption is needed, as
| > offered by e.g. DNS over TLS (DoT) and DNS over HTTP (DoT). unwind
|
| Before I jump aboard with DNSSECs failings in mind on my own networks rather
| than the mentioned hotel scenario. I believe but I am still not certain that
| services like PowerDNS have secure channels to the main primary DNS servers that
| apparently do not scale for the rest of us? Otherwise I worry that the network
| security target is a more singular centralised target compared to e.g. unbound.

These solutions (DoT / DoH, or the older DNSCrypt) encrypt DNS queries
from client to resolver, authorities are not available through these
protocols (yet).

This topic of DNS has lots of different attack vectors and risks
associated with it.  Slowly but surely, things are improving .. but
there's no big-bang solution that gets rid of all the issues in one
go.

If you want to use encrypted DNS from your client to your own resolver
then you can also do that.  Unbound is in base, look at the
tls-service-* and tls-port: options in unbound.conf(5).

The downside of using your own resolver (e.g. by running unbound on
your laptop), its traffic is more easily tied to a specific user.
There's an anonymizing power in using a bigger (shared) resolver (with
the downside that you then give your queries to a resolver that's
probably outside of your control - different risks and all that)

If you don't want to trust the freely available PowerDNS recursor then
that's your prerogative; it's just an easy option that's available
should you wish to test Otto's diff.

Cheers,

Paul 'WEiRD' de Weerd

--
>++++++++[<++++++++++>-]<+++++++.>+++[<------>-]<.>+++[<+
+++++++++++>-]<.>++[<------------>-]<+.--------------.[-]
                 http://www.weirdnet.nl/                 

Reply | Threaded
Open this post in threaded view
|

Re: Opportunistic DoT for unwind(8)

Otto Moerbeek
In reply to this post by Kevin Chadwick-4
On Thu, Oct 24, 2019 at 11:27:24AM +0100, Kevin Chadwick wrote:

>
> > The purpose of unwind is to provide secure DNS services even when
> > the available nameservers are broken or filtered like in many hotels.
> > To do that, it prefers DNSSEC whenever possible and changes to do
> > resolving by itself if needed.
> >
> > DNSSEC only offers integrity and authenticity.  To protect
> > eavesdropping on the requests in transit, encryption is needed, as
> > offered by e.g. DNS over TLS (DoT) and DNS over HTTP (DoT). unwind
>
> Before I jump aboard with DNSSECs failings in mind on my own networks rather
> than the mentioned hotel scenario. I believe but I am still not certain that
> services like PowerDNS have secure channels to the main primary DNS servers that
> apparently do not scale for the rest of us? Otherwise I worry that the network
> security target is a more singular centralised target compared to e.g. unbound.
>

Guess what the default config of unwind does: it runs a local resolver
and learns from DHCP. It will select either one, with DNSSEC working
preferred. DNSSEC operations if sometimes blocked by crappy
middleware. Unwind's resolver is basically a buit-in simple unbound,
the code uses libunbound.

So the default config of unwind addresss both your concerns.

        -Otto

Reply | Threaded
Open this post in threaded view
|

Re: Opportunistic DoT for unwind(8)

Otto Moerbeek
On Thu, Oct 24, 2019 at 12:24:22PM +0200, Otto Moerbeek wrote:

> On Thu, Oct 24, 2019 at 11:27:24AM +0100, Kevin Chadwick wrote:
>
> >
> > > The purpose of unwind is to provide secure DNS services even when
> > > the available nameservers are broken or filtered like in many hotels.
> > > To do that, it prefers DNSSEC whenever possible and changes to do
> > > resolving by itself if needed.
> > >
> > > DNSSEC only offers integrity and authenticity.  To protect
> > > eavesdropping on the requests in transit, encryption is needed, as
> > > offered by e.g. DNS over TLS (DoT) and DNS over HTTP (DoT). unwind
> >
> > Before I jump aboard with DNSSECs failings in mind on my own networks rather
> > than the mentioned hotel scenario. I believe but I am still not certain that
> > services like PowerDNS have secure channels to the main primary DNS servers that
> > apparently do not scale for the rest of us? Otherwise I worry that the network
> > security target is a more singular centralised target compared to e.g. unbound.
> >
>
> Guess what the default config of unwind does: it runs a local resolver
> and learns from DHCP. It will select either one, with DNSSEC working
> preferred. DNSSEC operations if sometimes blocked by crappy
> middleware. Unwind's resolver is basically a buit-in simple unbound,
> the code uses libunbound.
>
> So the default config of unwind addresss both your concerns.
>
> -Otto
>

To elaborate: unwind checks if DNSSEC works on any source and switches
back to non-validation if needed. The whole point of unwind is that it
will try to provide the best DNS service possibe in any circumstance.

        -Otto

Reply | Threaded
Open this post in threaded view
|

Re: Opportunistic DoT for unwind(8)

Stuart Henderson
In reply to this post by Paul de Weerd
On 2019/10/24 11:48, Paul de Weerd wrote:
> The downside of using your own resolver (e.g. by running unbound on
> your laptop), its traffic is more easily tied to a specific user.
> There's an anonymizing power in using a bigger (shared) resolver (with
> the downside that you then give your queries to a resolver that's
> probably outside of your control - different risks and all that)

And the downside of using a shared non-ISP resolver is that CDNs or
services using geolocation on the DNS requests won't have information about
your network location, so you may end up served by a suboptimal server or
CDN node (in the worst case you may end up with a very unsuitable one,
for example perhaps from a country halfway around the world).

The notable exception here of course is where the CDN also runs their own
public DNS resolver (hi Cloudflare!).

Reply | Threaded
Open this post in threaded view
|

Re: Opportunistic DoT for unwind(8)

Otto Moerbeek
In reply to this post by Otto Moerbeek
Hi,

I got *very* little feedback on this request for testing.

If not enough enough testing is done, I'll either abandon the diff or
commit it as-is, introducing bugs that could have been prevented. Both
are not good. So get going!

        -Otto

Reply | Threaded
Open this post in threaded view
|

Re: Opportunistic DoT for unwind(8)

Matthias Schmidt
Hi Otto,

* Otto Moerbeek wrote:
> Hi,
>
> I got *very* little feedback on this request for testing.
>
> If not enough enough testing is done, I'll either abandon the diff or
> commit it as-is, introducing bugs that could have been prevented. Both
> are not good. So get going!

I have your diff running for a couple of days and I can at least confirm
that it doesn't break an existing setup.  I have several upstream DoT
servers configured but no opportunistic DoT setup.

HTH and cheers

        Matthias

Reply | Threaded
Open this post in threaded view
|

Re: Opportunistic DoT for unwind(8)

Stuart Henderson
In reply to this post by Otto Moerbeek
On 2019/10/30 15:57, Otto Moerbeek wrote:

> Hi,
>
> I got *very* little feedback on this request for testing.
>
> If not enough enough testing is done, I'll either abandon the diff or
> commit it as-is, introducing bugs that could have been prevented. Both
> are not good. So get going!
>
> -Otto
>

I'm pointing it at a local dnsdist box via "forwarders { $ip_address }"
and querying unwind while watching tcpdump, I see it correctly using
TCP/853, and status correctly says

$ unwindctl status
captive portal is unchecked

selected             type status
       *        forwarder validating (OppDoT)
                 recursor validating



Comments:

- unwind doesn't have keepalives, so it's a new TCP session and TLS
handshake for every query, which can be bad in some cases (and could get
expensive with metered mobile data connections). for this reason it
would be helpful to have a way to disable it (though I suppose "block
out proto tcp to port 853" works at a pinch).

- several of the public DNS providers do include their IP in the certificate
so they could be validated even when picking them up opportunistically.
though I suppose with unwind this doesn't make a lot of difference as
it's just going to fallback to cleartext if TLS fails.

- might be useful to show OppDoT in the "best_resolver" line in debug logs?



Sample config from the dnsdist server below for anyone interested, this
is for 1.40rc5 but I think it'll work with the current ports version
(1.3.3) if you remove the addDOHLocal line.

--snip---------
addACL('0.0.0.0/0')
addACL('::/0')
newServer({address="44.33.22.11", name="upstream"})
addLocal('11.22.33.44:53',{doTCP=true, reusePort=true})
addTLSLocal("11.22.33.44", "/etc/ssl/xx.fullchain.pem", "/etc/ssl/private/xx.key",{ doTCP=true, reusePort=true })
addDOHLocal("11.22.33.44:5343", "/etc/ssl/xx.fullchain.pem", "/etc/ssl/private/xx.key", "/", {doTCP=true, reusePort=true})
pc = newPacketCache(10000, {maxTTL=86400, minTTL=0, temporaryFailureTTL=60, staleTTL=60, dontAge=false})
getPool(""):setCache(pc)
--snip------

Reply | Threaded
Open this post in threaded view
|

Re: Opportunistic DoT for unwind(8)

Remi Locherer
In reply to this post by Otto Moerbeek
Hi Otto,

On Wed, Oct 30, 2019 at 03:57:15PM +0100, Otto Moerbeek wrote:

> Hi,
>
> I got *very* little feedback on this request for testing.
>
> If not enough enough testing is done, I'll either abandon the diff or
> commit it as-is, introducing bugs that could have been prevented. Both
> are not good. So get going!
>
> -Otto
>

I applied your diff and tried with the following config:

$ unwind -nv
preference { recursor DoT forwarder dhcp }
forwarder {
        9.9.9.9
}
captive portal {
        url "http://captive.apple.com/"
        expected status 200
        expected response "<HTML><HEAD><TITLE>Success</TITLE></HEAD><BODY>Success</BODY></HTML>"
        auto yes
}
block list "/etc/unwind_blocklist.txt"
$

To force unwind to use 9.9.9.9 I tested with this pf rules:

$ doas pfctl -sr
doas ([hidden email]) password:
block return log all
pass log all flags S/SA
pass out log on egress inet from (vether0:network) to any flags S/SA nat-to (egress:0) round-robin
block return in on ! lo0 proto tcp from any to any port 6000:6010
block return out log inet proto tcp from any to ! 9.9.9.9 port = 53
block return out log inet proto udp from any to ! 9.9.9.9 port = 53
block return out log inet6 proto tcp from any to any port = 53
block return out log inet6 proto udp from any to any port = 53
block return out log proto tcp all user = 55
block return out log proto udp all user = 55
$

As expected I can now query 9.9.9.9 but 8.8.8.8 fails:

$ dig +short undeadly.org @9.9.9.9
94.142.241.173
typhoon ..c/examples$ dig +short undeadly.org @8.8.8.8
;; connection timed out; no servers could be reached
$

I expected that unwind would choose 9.9.9.9 with OppDoT. But unwind
selects dhcp which is correctly displayed as dead:

$ unwindctl status
captive portal is unknown

selected             type status
                 recursor dead
                forwarder validating
       *             dhcp dead
$

Port 853 on 9.9.9.9 is not blocked:

$ nc -zv 9.9.9.9 853
Connection to 9.9.9.9 853 port [tcp/domain-s] succeeded!
$ nc -zv -u 9.9.9.9 853
Connection to 9.9.9.9 853 port [udp/domain-s] succeeded!
$

Did I do something wrong in unwind.conf?

Remi

Reply | Threaded
Open this post in threaded view
|

Re: Opportunistic DoT for unwind(8)

Otto Moerbeek
On Wed, Oct 30, 2019 at 11:46:36PM +0100, Remi Locherer wrote:

> Hi Otto,
>
> On Wed, Oct 30, 2019 at 03:57:15PM +0100, Otto Moerbeek wrote:
> > Hi,
> >
> > I got *very* little feedback on this request for testing.
> >
> > If not enough enough testing is done, I'll either abandon the diff or
> > commit it as-is, introducing bugs that could have been prevented. Both
> > are not good. So get going!
> >
> > -Otto
> >
>
> I applied your diff and tried with the following config:
>
> $ unwind -nv
> preference { recursor DoT forwarder dhcp }
> forwarder {
>         9.9.9.9
> }
> captive portal {
>         url "http://captive.apple.com/"
>         expected status 200
>         expected response "<HTML><HEAD><TITLE>Success</TITLE></HEAD><BODY>Success</BODY></HTML>"
>         auto yes
> }
> block list "/etc/unwind_blocklist.txt"
> $
>
> To force unwind to use 9.9.9.9 I tested with this pf rules:
>
> $ doas pfctl -sr
> doas ([hidden email]) password:
> block return log all
> pass log all flags S/SA
> pass out log on egress inet from (vether0:network) to any flags S/SA nat-to (egress:0) round-robin
> block return in on ! lo0 proto tcp from any to any port 6000:6010
> block return out log inet proto tcp from any to ! 9.9.9.9 port = 53
> block return out log inet proto udp from any to ! 9.9.9.9 port = 53
> block return out log inet6 proto tcp from any to any port = 53
> block return out log inet6 proto udp from any to any port = 53
> block return out log proto tcp all user = 55
> block return out log proto udp all user = 55
> $
>
> As expected I can now query 9.9.9.9 but 8.8.8.8 fails:
>
> $ dig +short undeadly.org @9.9.9.9
> 94.142.241.173
> typhoon ..c/examples$ dig +short undeadly.org @8.8.8.8
> ;; connection timed out; no servers could be reached
> $
>
> I expected that unwind would choose 9.9.9.9 with OppDoT. But unwind
> selects dhcp which is correctly displayed as dead:
>
> $ unwindctl status
> captive portal is unknown
>
> selected             type status
>                  recursor dead
>                 forwarder validating
>        *             dhcp dead
> $
>
> Port 853 on 9.9.9.9 is not blocked:
>
> $ nc -zv 9.9.9.9 853
> Connection to 9.9.9.9 853 port [tcp/domain-s] succeeded!
> $ nc -zv -u 9.9.9.9 853
> Connection to 9.9.9.9 853 port [udp/domain-s] succeeded!
> $
>
> Did I do something wrong in unwind.conf?
>
> Remi

No, you found a bug that happens if the recursor is found dead. In
that case it would switch off OppDot for forwarders as well. Next
version of the diff will have a fix.


As for the unwindctl thing, I could not reproduce that one. Dould it
be that you did not build and install usr.sbin/unwindctl? It looks
like the messaging between unwind and unwindctl is off.

        -Otto

Reply | Threaded
Open this post in threaded view
|

Re: Opportunistic DoT for unwind(8)

Otto Moerbeek
In reply to this post by Stuart Henderson
On Wed, Oct 30, 2019 at 08:51:00PM +0000, Stuart Henderson wrote:

> On 2019/10/30 15:57, Otto Moerbeek wrote:
> > Hi,
> >
> > I got *very* little feedback on this request for testing.
> >
> > If not enough enough testing is done, I'll either abandon the diff or
> > commit it as-is, introducing bugs that could have been prevented. Both
> > are not good. So get going!
> >
> > -Otto
> >
>
> I'm pointing it at a local dnsdist box via "forwarders { $ip_address }"
> and querying unwind while watching tcpdump, I see it correctly using
> TCP/853, and status correctly says
>
> $ unwindctl status
> captive portal is unchecked
>
> selected             type status
>        *        forwarder validating (OppDoT)
>                  recursor validating
>
>
>

Thanks for testing.

> Comments:
>
> - unwind doesn't have keepalives, so it's a new TCP session and TLS
> handshake for every query, which can be bad in some cases (and could get
> expensive with metered mobile data connections). for this reason it
> would be helpful to have a way to disable it (though I suppose "block
> out proto tcp to port 853" works at a pinch).

unwind should cache thogh, can you observe that?

>
> - several of the public DNS providers do include their IP in the certificate
> so they could be validated even when picking them up opportunistically.
> though I suppose with unwind this doesn't make a lot of difference as
> it's just going to fallback to cleartext if TLS fails.

For any Dot mode the validity of the cert is checked, for OppDot the
trust check is only: is the cert signed by a trusted CA. We do not
know which DoT providers include a cert with an IP address, so we
cannot force a check for that. Besides that, I could not get
libunbound to accept a authentication IP like 9.9.9.9, only a name
like "quad9.net".

>
> - might be useful to show OppDoT in the "best_resolver" line in debug logs?
>
>
>
> Sample config from the dnsdist server below for anyone interested, this
> is for 1.40rc5 but I think it'll work with the current ports version
> (1.3.3) if you remove the addDOHLocal line.
>
> --snip---------
> addACL('0.0.0.0/0')
> addACL('::/0')
> newServer({address="44.33.22.11", name="upstream"})
> addLocal('11.22.33.44:53',{doTCP=true, reusePort=true})
> addTLSLocal("11.22.33.44", "/etc/ssl/xx.fullchain.pem", "/etc/ssl/private/xx.key",{ doTCP=true, reusePort=true })
> addDOHLocal("11.22.33.44:5343", "/etc/ssl/xx.fullchain.pem", "/etc/ssl/private/xx.key", "/", {doTCP=true, reusePort=true})
> pc = newPacketCache(10000, {maxTTL=86400, minTTL=0, temporaryFailureTTL=60, staleTTL=60, dontAge=false})
> getPool(""):setCache(pc)
> --snip------
>

Yes , that should work indeed.

        -Otto



Reply | Threaded
Open this post in threaded view
|

Re: Opportunistic DoT for unwind(8)

Stuart Henderson
On 2019/10/31 10:18, Otto Moerbeek wrote:
> On Wed, Oct 30, 2019 at 08:51:00PM +0000, Stuart Henderson wrote:
>
> > - unwind doesn't have keepalives, so it's a new TCP session and TLS
> > handshake for every query, which can be bad in some cases (and could get
> > expensive with metered mobile data connections). for this reason it
> > would be helpful to have a way to disable it (though I suppose "block
> > out proto tcp to port 853" works at a pinch).
>
> unwind should cache thogh, can you observe that?

Yes, it does cache. (for "every query" I meant "every query sent to
the forwarder")

> > - several of the public DNS providers do include their IP in the certificate
> > so they could be validated even when picking them up opportunistically.
> > though I suppose with unwind this doesn't make a lot of difference as
> > it's just going to fallback to cleartext if TLS fails.
>
> For any Dot mode the validity of the cert is checked, for OppDot the
> trust check is only: is the cert signed by a trusted CA. We do not
> know which DoT providers include a cert with an IP address, so we
> cannot force a check for that. Besides that, I could not get
> libunbound to accept a authentication IP like 9.9.9.9, only a name
> like "quad9.net".

Writing as a note to myself to check later when I have more time as
much as anything, is there a hold-off on re-checking if there is a
cert failure (or indeed if DoT port isn't answered), or does it
re-check for every query sent upstream. Also are there excessive
delays if port 853 packets are dropped rather than rejected.

Reply | Threaded
Open this post in threaded view
|

Re: Opportunistic DoT for unwind(8)

Florian Obser-2
On Thu, Oct 31, 2019 at 10:04:07AM +0000, Stuart Henderson wrote:

> Writing as a note to myself to check later when I have more time as
> much as anything, is there a hold-off on re-checking if there is a
> cert failure (or indeed if DoT port isn't answered), or does it
> re-check for every query sent upstream. Also are there excessive
> delays if port 853 packets are dropped rather than rejected.
>

Checking a resolving strategy is decoupled from using a strategy.

The global variable
struct uw_resolver *resolvers[UW_RES_NONE];
stores the available resolving strategy contexts.

best_resolvers() picks the best one according to res_state
(VALIDATING > RESOLVING > DEAD) while also consindering the
preference (default or from unwind.conf).

check_resolver / check_resolver_done are running non blocking via
libevent and set res_state in the the resolvers list.
So the checking never blocks resolving.

When a strategy is found to be DEAD we do an exponatial back-off
stopping saturating at ~ 17 minutes (1024 seconds).

(re-)checks are triggered when
- we got a SERVFAIL for a query
- we created a new resolver (config reload, new dhcp forwarders)
- we got past a captive portal
- we got a RTM_IFINFO on the routing socket (interface going up /down)

This is of course the most important part of unwind, resolving stuff
is trivial from unwind's point of view, just hand it of to libunbound.
Finding out to which libunbound is the tough part. There is still room
for improvement.

--
I'm not entirely sure you are real.

Reply | Threaded
Open this post in threaded view
|

Re: Opportunistic DoT for unwind(8)

Otto Moerbeek
In reply to this post by Otto Moerbeek
Hi,

So here's a new diff that incorporates the bug fix mentioned plus
debug printf line changes suggested by Stuart.

Please note that this is a diff on top of very recent current, i.e.
florian's work he committed today. That means that you need to be
up-to-date (including a recent libc update that was committed a few
days ago) to be able to test this version.

        -Otto

Index: sbin/unwind/parse.y
===================================================================
RCS file: /cvs/src/sbin/unwind/parse.y,v
retrieving revision 1.12
diff -u -p -r1.12 parse.y
--- sbin/unwind/parse.y 31 Oct 2019 12:51:43 -0000 1.12
+++ sbin/unwind/parse.y 31 Oct 2019 16:32:27 -0000
@@ -84,7 +84,6 @@ int check_pref_uniq(enum uw_resolver_ty
 
 static struct uw_conf *conf;
 static int errors;
-static struct uw_forwarder *uw_forwarder;
 
 void clear_config(struct uw_conf *xconf);
 struct sockaddr_storage *host_ip(const char *);
@@ -287,7 +286,9 @@ forwarderopts_l : forwarderopts_l forwa
 
 forwarderoptsl : STRING port authname dot {
  int ret, port;
+ struct uw_forwarder *uw_fwd;
  struct sockaddr_storage *ss;
+
  if ((ss = host_ip($1)) == NULL) {
  yyerror("%s is not an ip-address", $1);
  free($1);
@@ -305,37 +306,53 @@ forwarderoptsl : STRING port authname d
  else
  port = $2;
 
- if ((uw_forwarder = calloc(1,
-    sizeof(*uw_forwarder))) == NULL)
+ if ($3 != NULL && $4 == 0) {
+ yyerror("authentication name can only "
+    "be used with DoT");
+ free($1);
+ YYERROR;
+ }
+
+
+ if ((uw_fwd = calloc(1,
+    sizeof(*uw_fwd))) == NULL)
  err(1, NULL);
 
- if ($3 == NULL)
- ret = snprintf(uw_forwarder->name,
-    sizeof(uw_forwarder->name),
-    "%s@%d", $1, port);
- else
- ret = snprintf(uw_forwarder->name,
-    sizeof(uw_forwarder->name),
-    "%s@%d#%s", $1, port, $3);
+ if ($4 == DOT) {
+ if ($3 == NULL)
+ ret = snprintf(uw_fwd->name,
+    sizeof(uw_fwd->name),
+    "%s@%d", $1, port);
+ else
+ ret = snprintf(uw_fwd->name,
+    sizeof(uw_fwd->name),
+    "%s@%d#%s", $1, port, $3);
+ } else {
+ uw_fwd->port = $2;
+ /* complete string wil be done later */
+ ret = snprintf(uw_fwd->name,
+    sizeof(uw_fwd->name), "%s", $1);
+ }
  if (ret < 0 || (size_t)ret >=
-    sizeof(uw_forwarder->name)) {
- free(uw_forwarder);
+    sizeof(uw_fwd->name)) {
+ free(uw_fwd);
  yyerror("forwarder %s too long", $1);
  free($1);
  YYERROR;
  }
- free($1);
 
  if ($4 == DOT)
  SIMPLEQ_INSERT_TAIL(
     &conf->uw_dot_forwarder_list,
-    uw_forwarder, entry);
- else
+    uw_fwd, entry);
+ else {
  SIMPLEQ_INSERT_TAIL(
     &conf->uw_forwarder_list,
-    uw_forwarder, entry);
+    uw_fwd, entry);
+ }
+ free($1);
  }
- ;
+ ;
 
 port : PORT NUMBER { $$ = $2; }
  | /* empty */ { $$ = 0; }
@@ -874,12 +891,14 @@ symget(const char *nam)
 void
 clear_config(struct uw_conf *xconf)
 {
- while((uw_forwarder = SIMPLEQ_FIRST(&xconf->uw_forwarder_list)) !=
+ struct uw_forwarder *uw_forwarder;
+
+ while ((uw_forwarder = SIMPLEQ_FIRST(&xconf->uw_forwarder_list)) !=
     NULL) {
  SIMPLEQ_REMOVE_HEAD(&xconf->uw_forwarder_list, entry);
  free(uw_forwarder);
  }
- while((uw_forwarder = SIMPLEQ_FIRST(&xconf->uw_dot_forwarder_list)) !=
+ while ((uw_forwarder = SIMPLEQ_FIRST(&xconf->uw_dot_forwarder_list)) !=
     NULL) {
  SIMPLEQ_REMOVE_HEAD(&xconf->uw_dot_forwarder_list, entry);
  free(uw_forwarder);
Index: sbin/unwind/resolver.c
===================================================================
RCS file: /cvs/src/sbin/unwind/resolver.c,v
retrieving revision 1.48
diff -u -p -r1.48 resolver.c
--- sbin/unwind/resolver.c 31 Oct 2019 12:54:40 -0000 1.48
+++ sbin/unwind/resolver.c 31 Oct 2019 16:32:27 -0000
@@ -115,6 +115,8 @@ struct uw_resolver *create_resolver(enum
 void free_resolver(struct uw_resolver *);
 void set_forwarders(struct uw_resolver *,
      struct uw_forwarder_head *);
+void set_forwarders_oppdot(struct uw_resolver *,
+     struct uw_forwarder_head *, int);
 void resolver_check_timo(int, short, void *);
 void resolver_free_timo(int, short, void *);
 void check_resolver(struct uw_resolver *);
@@ -164,6 +166,7 @@ struct event_base *ev_base;
 
 enum uw_resolver_state global_state = DEAD;
 enum captive_portal_state captive_portal_state = PORTAL_UNCHECKED;
+int dhcp_oppdot, forw_oppdot;
 
 void
 resolver_sig_handler(int sig, short event, void *arg)
@@ -607,6 +610,7 @@ resolver_dispatch_main(int fd, short eve
  nconf = NULL;
  if (forwarders_changed) {
  log_debug("static forwarders changed");
+ forw_oppdot = 1;
  new_static_forwarders();
  }
  if (dot_forwarders_changed) {
@@ -768,7 +772,7 @@ parse_dhcp_forwarders(char *forwarders)
  if (forwarders != NULL) {
  while((ns = strsep(&forwarders, ",")) != NULL) {
  log_debug("%s: %s", __func__, ns);
- if ((uw_forwarder = malloc(sizeof(struct
+ if ((uw_forwarder = calloc(1, sizeof(struct
     uw_forwarder))) == NULL)
  fatal(NULL);
  if (strlcpy(uw_forwarder->name, ns,
@@ -782,6 +786,7 @@ parse_dhcp_forwarders(char *forwarders)
 
  if (check_forwarders_changed(&new_forwarder_list,
     &dhcp_forwarder_list)) {
+ dhcp_oppdot = 1;
  replace_forwarders(&new_forwarder_list, &dhcp_forwarder_list);
  new_forwarders();
  new_asr_forwarders();
@@ -985,10 +990,24 @@ create_resolver(enum uw_resolver_type ty
  case UW_RES_RECURSOR:
  break;
  case UW_RES_DHCP:
- set_forwarders(res, &dhcp_forwarder_list);
+ if (dhcp_oppdot) {
+ set_forwarders_oppdot(res, &dhcp_forwarder_list, 853);
+ ub_ctx_set_option(res->ctx, "tls-cert-bundle:",
+    tls_default_ca_cert_file());
+ ub_ctx_set_tls(res->ctx, 1);
+ } else
+ set_forwarders_oppdot(res, &dhcp_forwarder_list, 53);
  break;
  case UW_RES_FORWARDER:
- set_forwarders(res, &resolver_conf->uw_forwarder_list);
+ if (forw_oppdot) {
+ set_forwarders_oppdot(res,
+    &resolver_conf->uw_forwarder_list, 853);
+ ub_ctx_set_option(res->ctx, "tls-cert-bundle:",
+    tls_default_ca_cert_file());
+ ub_ctx_set_tls(res->ctx, 1);
+ } else
+ set_forwarders_oppdot(res,
+    &resolver_conf->uw_forwarder_list, 53);
  break;
  case UW_RES_DOT:
  set_forwarders(res, &resolver_conf->uw_dot_forwarder_list);
@@ -1034,6 +1053,22 @@ set_forwarders(struct uw_resolver *res,
 }
 
 void
+set_forwarders_oppdot(struct uw_resolver *res, struct uw_forwarder_head
+    *uw_forwarder_list, int def_port)
+{
+ struct uw_forwarder *uw_forwarder;
+
+ SIMPLEQ_FOREACH(uw_forwarder, uw_forwarder_list, entry) {
+ char name[1024];
+ int port = uw_forwarder->port;
+ if (port == 0)
+ port = def_port;
+ snprintf(name, sizeof(name), "%s@%d", uw_forwarder->name, port);
+ ub_ctx_set_fwd(res->ctx, name);
+ }
+}
+
+void
 resolver_check_timo(int fd, short events, void *arg)
 {
  check_resolver((struct uw_resolver *)arg);
@@ -1089,7 +1124,7 @@ check_resolver_done(void *arg, int rcode
 
  data = (struct check_resolver_data *)arg;
 
- log_debug("%s: rcode: %d", __func__, rcode);
+ log_debug("%s: %s rcode: %d", __func__, uw_resolver_type_str[data->res->type], rcode);
 
  prev_state = data->res->state;
 
@@ -1101,6 +1136,25 @@ check_resolver_done(void *arg, int rcode
 
  if (rcode == LDNS_RCODE_SERVFAIL) {
  data->res->state = DEAD;
+
+ switch (data->res->type) {
+ case UW_RES_DHCP:
+ if (dhcp_oppdot) {
+ dhcp_oppdot = 0;
+ log_debug("disabling DHCP OppDoT");
+ new_forwarders();
+ }
+ break;
+ case UW_RES_FORWARDER:
+ if (forw_oppdot) {
+ forw_oppdot = 0;
+ log_debug("disabling forwarder OppDoT");
+ new_static_forwarders();
+ }
+ break;
+ default:
+ break;
+ }
  goto out;
  }
 
@@ -1259,9 +1313,9 @@ struct uw_resolver*
 best_resolver(void)
 {
  struct uw_resolver *res = NULL;
- int i;
+ int i, oppdot;
 
- log_debug("%s: %s: %s, %s: %s, %s: %s, %s: %s, %s: %s, "
+ log_debug("%s: %s: %s, %s: %s%s, %s: %s%s, %s: %s, %s: %s, "
     "captive_portal: %s",
     __func__,
     uw_resolver_type_str[UW_RES_RECURSOR], resolvers[UW_RES_RECURSOR]
@@ -1269,9 +1323,11 @@ best_resolver(void)
     : "NA",
     uw_resolver_type_str[UW_RES_DHCP], resolvers[UW_RES_DHCP] != NULL ?
     uw_resolver_state_str[resolvers[UW_RES_DHCP]->state] : "NA",
+    dhcp_oppdot ? " (OppDot)" : "",
     uw_resolver_type_str[UW_RES_FORWARDER],
     resolvers[UW_RES_FORWARDER] != NULL ?
     uw_resolver_state_str[resolvers[UW_RES_FORWARDER]->state] : "NA",
+    forw_oppdot ? " (OppDot)" : "",
     uw_resolver_type_str[UW_RES_DOT],
     resolvers[UW_RES_DOT] != NULL ?
     uw_resolver_state_str[resolvers[UW_RES_DOT]->state] : "NA",
@@ -1295,8 +1351,10 @@ best_resolver(void)
     resolvers[resolver_conf->res_pref[i]]) < 0)
  res = resolvers[resolver_conf->res_pref[i]];
 out:
- log_debug("%s: %s state: %s", __func__, uw_resolver_type_str[res->type],
-    uw_resolver_state_str[res->state]);
+ oppdot = (res->type == UW_RES_DHCP && dhcp_oppdot) ||
+    (res->type == UW_RES_FORWARDER && forw_oppdot);
+ log_debug("%s: %s state: %s%s", __func__, uw_resolver_type_str[res->type],
+    uw_resolver_state_str[res->state], oppdot ? " (OppDoT)" : "");
  return (res);
 }
 
@@ -1376,6 +1434,12 @@ send_resolver_info(struct uw_resolver *r
  cri.state = res->state;
  cri.type = res->type;
  cri.selected = selected;
+ if (cri.type == UW_RES_DHCP && dhcp_oppdot)
+ cri.oppdot = 1;
+ else if (cri.type == UW_RES_FORWARDER && forw_oppdot)
+ cri.oppdot = 1;
+ else
+ cri.oppdot = 0;
  resolver_imsg_compose_frontend(IMSG_CTL_RESOLVER_INFO, pid, &cri,
     sizeof(cri));
 }
Index: sbin/unwind/resolver.h
===================================================================
RCS file: /cvs/src/sbin/unwind/resolver.h,v
retrieving revision 1.6
diff -u -p -r1.6 resolver.h
--- sbin/unwind/resolver.h 2 Apr 2019 07:47:23 -0000 1.6
+++ sbin/unwind/resolver.h 31 Oct 2019 16:32:27 -0000
@@ -51,6 +51,7 @@ struct ctl_resolver_info {
  enum uw_resolver_state state;
  enum uw_resolver_type type;
  int selected;
+ int oppdot;
 };
 
 void resolver(int, int);
Index: sbin/unwind/unwind.h
===================================================================
RCS file: /cvs/src/sbin/unwind/unwind.h,v
retrieving revision 1.20
diff -u -p -r1.20 unwind.h
--- sbin/unwind/unwind.h 31 Oct 2019 12:54:40 -0000 1.20
+++ sbin/unwind/unwind.h 31 Oct 2019 16:32:27 -0000
@@ -126,6 +126,7 @@ enum imsg_type {
 struct uw_forwarder {
  SIMPLEQ_ENTRY(uw_forwarder) entry;
  char name[1024]; /* XXX */
+ uint16_t port;
 };
 
 SIMPLEQ_HEAD(uw_forwarder_head, uw_forwarder);
Index: usr.sbin/unwindctl/parser.c
===================================================================
RCS file: /cvs/src/usr.sbin/unwindctl/parser.c,v
retrieving revision 1.4
diff -u -p -r1.4 parser.c
--- usr.sbin/unwindctl/parser.c 31 Oct 2019 12:51:43 -0000 1.4
+++ usr.sbin/unwindctl/parser.c 31 Oct 2019 16:32:27 -0000
@@ -72,7 +72,7 @@ static const struct token t_status[] = {
  {NOTOKEN, "", NONE, NULL},
  {KEYWORD, "recursor", STATUS_RECURSOR, NULL},
  {KEYWORD, "dhcp", STATUS_DHCP, NULL},
- {KEYWORD, "static", STATUS_STATIC, NULL},
+ {KEYWORD, "forwarder", STATUS_STATIC, NULL},
  {KEYWORD, "DoT", STATUS_DOT, NULL},
  {KEYWORD, "asr", STATUS_ASR, NULL},
  {ENDTOKEN, "", STATUS, NULL}
Index: usr.sbin/unwindctl/unwindctl.8
===================================================================
RCS file: /cvs/src/usr.sbin/unwindctl/unwindctl.8,v
retrieving revision 1.4
diff -u -p -r1.4 unwindctl.8
--- usr.sbin/unwindctl/unwindctl.8 5 Feb 2019 20:41:12 -0000 1.4
+++ usr.sbin/unwindctl/unwindctl.8 31 Oct 2019 16:32:27 -0000
@@ -55,14 +55,14 @@ Enable very noisy debug logging.
 Reload the configuration file.
 .It Cm recheck portal
 Run the captive portal detection.
-.It Cm status Op Cm recursor | dhcp | DoT | static
+.It Cm status Op Cm recursor | dhcp | DoT | forwarder
 Show a status summary.
 If one of
 .Cm recursor ,
 .Cm dhcp ,
 .Cm DoT ,
 or
-.Cm static
+.Cm forwarder
 is specified, more detailed information about that type of resolver is given
 including reasons why DNSSEC validation might be failing and a query time
 histogram.
Index: usr.sbin/unwindctl/unwindctl.c
===================================================================
RCS file: /cvs/src/usr.sbin/unwindctl/unwindctl.c,v
retrieving revision 1.7
diff -u -p -r1.7 unwindctl.c
--- usr.sbin/unwindctl/unwindctl.c 31 Oct 2019 12:51:43 -0000 1.7
+++ usr.sbin/unwindctl/unwindctl.c 31 Oct 2019 16:32:27 -0000
@@ -247,9 +247,10 @@ show_status_msg(struct imsg *imsg)
  break;
  case IMSG_CTL_RESOLVER_INFO:
  cri = imsg->data;
- printf("%8s %16s %s\n", cri->selected ? "*" : " ",
+ printf("%8s %16s %s%s\n", cri->selected ? "*" : " ",
     uw_resolver_type_str[cri->type],
-    uw_resolver_state_str[cri->state]);
+    uw_resolver_state_str[cri->state],
+    cri->oppdot ? " (OppDoT)" : "");
  break;
  case IMSG_CTL_RESOLVER_WHY_BOGUS:
  /* make sure this is a string */

Reply | Threaded
Open this post in threaded view
|

Re: Opportunistic DoT for unwind(8)

Remi Locherer
On Thu, Oct 31, 2019 at 08:14:04PM +0100, Otto Moerbeek wrote:
> Hi,
>
> So here's a new diff that incorporates the bug fix mentioned plus
> debug printf line changes suggested by Stuart.
>
> Please note that this is a diff on top of very recent current, i.e.
> florian's work he committed today. That means that you need to be
> up-to-date (including a recent libc update that was committed a few
> days ago) to be able to test this version.

I upgraded to a snapshot from today, updated the source and applied
your diff. Then I did the same test as last time using pf to block port 53
(block return out log inet proto {tcp udp} to !9.9.9.9 port 53).

Result: the non functional type asr is selected instead of the forwarder.

$ doas unwindctl status
captive portal is unknown

selected             type status
                 recursor dead
                forwarder validating (OppDoT)
                     dhcp unknown (OppDoT)
       *              asr dead
$
$ getent hosts undeadly.org
$ echo $?
2
$ dig +short undeadly.org @9.9.9.9
94.142.241.173
$

Without your patch unwind behaves similar regarding the type selection:

$ doas unwindctl status
captive portal is unknown

selected             type status
                 recursor dead
                forwarder validating
                     dhcp dead
       *              asr dead
$

Reply | Threaded
Open this post in threaded view
|

Re: Opportunistic DoT for unwind(8)

Florian Obser-2
On Fri, Nov 01, 2019 at 09:35:07PM +0100, Remi Locherer wrote:

> On Thu, Oct 31, 2019 at 08:14:04PM +0100, Otto Moerbeek wrote:
> > Hi,
> >
> > So here's a new diff that incorporates the bug fix mentioned plus
> > debug printf line changes suggested by Stuart.
> >
> > Please note that this is a diff on top of very recent current, i.e.
> > florian's work he committed today. That means that you need to be
> > up-to-date (including a recent libc update that was committed a few
> > days ago) to be able to test this version.
>
> I upgraded to a snapshot from today, updated the source and applied
> your diff. Then I did the same test as last time using pf to block port 53
> (block return out log inet proto {tcp udp} to !9.9.9.9 port 53).
>
> Result: the non functional type asr is selected instead of the forwarder.
>
> $ doas unwindctl status
> captive portal is unknown
>
> selected             type status
>                  recursor dead
>                 forwarder validating (OppDoT)
>                      dhcp unknown (OppDoT)
>        *              asr dead
> $
> $ getent hosts undeadly.org
> $ echo $?
> 2
> $ dig +short undeadly.org @9.9.9.9
> 94.142.241.173
> $
>
> Without your patch unwind behaves similar regarding the type selection:
>
> $ doas unwindctl status
> captive portal is unknown

^ you are creating a not supported configuration.

When we are behind a captive portal or don't know yet if we are behind
a captive portal resolving is forced to asr.

That might not be very wise if asr is dead but I currently don't see
how this can happen in practice except with a well aimed foot-gun.

>
> selected             type status
>                  recursor dead
>                 forwarder validating
>                      dhcp dead
>        *              asr dead
> $
>

--
I'm not entirely sure you are real.

Reply | Threaded
Open this post in threaded view
|

Re: Opportunistic DoT for unwind(8)

Florian Obser-2
On Fri, Nov 01, 2019 at 09:45:37PM +0100, Florian Obser wrote:

> On Fri, Nov 01, 2019 at 09:35:07PM +0100, Remi Locherer wrote:
> > On Thu, Oct 31, 2019 at 08:14:04PM +0100, Otto Moerbeek wrote:
> > > Hi,
> > >
> > > So here's a new diff that incorporates the bug fix mentioned plus
> > > debug printf line changes suggested by Stuart.
> > >
> > > Please note that this is a diff on top of very recent current, i.e.
> > > florian's work he committed today. That means that you need to be
> > > up-to-date (including a recent libc update that was committed a few
> > > days ago) to be able to test this version.
> >
> > I upgraded to a snapshot from today, updated the source and applied
> > your diff. Then I did the same test as last time using pf to block port 53
> > (block return out log inet proto {tcp udp} to !9.9.9.9 port 53).
> >
> > Result: the non functional type asr is selected instead of the forwarder.
> >
> > $ doas unwindctl status
> > captive portal is unknown
> >
> > selected             type status
> >                  recursor dead
> >                 forwarder validating (OppDoT)
> >                      dhcp unknown (OppDoT)
> >        *              asr dead
> > $
> > $ getent hosts undeadly.org
> > $ echo $?
> > 2
> > $ dig +short undeadly.org @9.9.9.9
> > 94.142.241.173
> > $
> >
> > Without your patch unwind behaves similar regarding the type selection:
> >
> > $ doas unwindctl status
> > captive portal is unknown
>
> ^ you are creating a not supported configuration.
>
> When we are behind a captive portal or don't know yet if we are behind
> a captive portal resolving is forced to asr.
>
> That might not be very wise if asr is dead but I currently don't see
> how this can happen in practice except with a well aimed foot-gun.

Actually, I have an idea how this can happen in practice, please try this:

diff --git resolver.c resolver.c
index f59860a5e98..5bbc63f60fa 100644
--- resolver.c
+++ resolver.c
@@ -1282,7 +1282,8 @@ best_resolver(void)
 
  if (captive_portal_state == PORTAL_UNKNOWN || captive_portal_state ==
     BEHIND) {
- if (resolvers[UW_RES_ASR] != NULL) {
+ if (resolvers[UW_RES_ASR] != NULL && resolvers[UW_RES_ASR]->
+                    state != DEAD) {
  res = resolvers[UW_RES_ASR];
  goto out;
  }


>
> >
> > selected             type status
> >                  recursor dead
> >                 forwarder validating
> >                      dhcp dead
> >        *              asr dead
> > $
> >
>
> --
> I'm not entirely sure you are real.
>

--
I'm not entirely sure you are real.

12