Re: httpd(8) fix tls comparison of servers

2021-02-15 Thread Florian Obser
OK florian

On Mon, Feb 15, 2021 at 12:41:31PM +0100, Claudio Jeker wrote:
> For SNI all TLS servers need to run with the same config. The config
> parser has an extra step for this. The problem is it also compares the
> TLS config params with non-TLS servers when a server block has both
> listen * port 80 and listen * tls port 443.
> 
> The following diff fixes that and also removes the unused last argument of
> server_tls_cmp().
> -- 
> :wq Claudio
> 
> Index: httpd.h
> ===
> RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
> retrieving revision 1.154
> diff -u -p -r1.154 httpd.h
> --- httpd.h   27 Jan 2021 07:21:52 -  1.154
> +++ httpd.h   13 Feb 2021 08:32:34 -
> @@ -622,7 +622,7 @@ intcmdline_symset(char *);
>  
>  /* server.c */
>  void  server(struct privsep *, struct privsep_proc *);
> -int   server_tls_cmp(struct server *, struct server *, int);
> +int   server_tls_cmp(struct server *, struct server *);
>  int   server_tls_load_ca(struct server *);
>  int   server_tls_load_crl(struct server *);
>  int   server_tls_load_keypair(struct server *);
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> retrieving revision 1.124
> diff -u -p -r1.124 parse.y
> --- parse.y   22 Jan 2021 13:07:17 -  1.124
> +++ parse.y   13 Feb 2021 09:02:18 -
> @@ -333,7 +333,8 @@ server: SERVER optmatch STRING{
>   free(srv);
>   YYERROR;
>   }
> - if (server_tls_cmp(s, srv, 0) != 0) {
> + if (srv->srv_conf.flags & SRVFLAG_TLS &&
> + server_tls_cmp(s, srv) != 0) {
>   yyerror("server \"%s\": tls "
>   "configuration mismatch on same "
>   "address/port",
> Index: server.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/server.c,v
> retrieving revision 1.124
> diff -u -p -r1.124 server.c
> --- server.c  2 Jan 2021 18:35:07 -   1.124
> +++ server.c  15 Feb 2021 11:38:08 -
> @@ -127,7 +127,7 @@ server_privinit(struct server *srv)
>  }
>  
>  int
> -server_tls_cmp(struct server *s1, struct server *s2, int match_keypair)
> +server_tls_cmp(struct server *s1, struct server *s2)
>  {
>   struct server_config*sc1, *sc2;
>  
> @@ -146,13 +146,6 @@ server_tls_cmp(struct server *s1, struct
>   return (-1);
>   if (strcmp(sc1->tls_ecdhe_curves, sc2->tls_ecdhe_curves) != 0)
>   return (-1);
> -
> - if (match_keypair) {
> - if (strcmp(sc1->tls_cert_file, sc2->tls_cert_file) != 0)
> - return (-1);
> - if (strcmp(sc1->tls_key_file, sc2->tls_key_file) != 0)
> - return (-1);
> - }
>  
>   return (0);
>  }
> 

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



Re: Unbound: add support for pf tables to ipset module

2021-02-07 Thread Florian Obser
What sthen said, and I also have zero interest in maintaining what
comes down to a fork of unbound.

(Bit besides the point, I don't think the diff applies.)

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



unwind(8): improve DNS64 detection

2021-02-06 Thread Florian Obser
I noticed that sometimes DNS64 detection is not working correctly on
boot. Eventually I tracked it down to this:
Feb  6 08:56:22 x1 unwind[7139]: check_dns64_done: bad packet: too short: -1

The problem is that we are checking for dns64 while we might not yet
have a route to the nameserver provided by the router advertisement or
our IPv6 addresses are all tentative. Since we only re-schedule the
check when the network changes we are stuck. Instead we can check for
the presence of DNS64 when we know that DNS works, which is being
rescheduled correctly.

OK?


diff --git resolver.c resolver.c
index 302f381a0dd..95c32369830 100644
--- resolver.c
+++ resolver.c
@@ -1142,6 +1142,8 @@ new_resolver(enum uw_resolver_type type, enum 
uw_resolver_state state)
/* FALLTHROUGH */
case RESOLVING:
resolvers[type]->state = state;
+   if (type == UW_RES_ASR)
+   check_dns64();
break;
}
 }
@@ -2053,7 +2055,6 @@ replace_autoconf_forwarders(struct imsg_rdns_proposal 
*rdns_proposal)
new_resolver(UW_RES_ASR, UNKNOWN);
new_resolver(UW_RES_DHCP, UNKNOWN);
new_resolver(UW_RES_ODOT_DHCP, UNKNOWN);
-   check_dns64();
} else {
while ((tmp = TAILQ_FIRST(_forwarder_list)) != NULL) {
TAILQ_REMOVE(_forwarder_list, tmp, entry);



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



Re: unwind(8): open DNSSEC trustanchor late

2021-02-06 Thread Florian Obser
On Sat, Feb 06, 2021 at 01:23:35AM +0100, Jeremie Courreges-Anglas wrote:
> On Fri, Jan 29 2021, Florian Obser  wrote:
> > Last piece of the puzzle...
> >
> > Re-try to open DNSSEC trust anchor file if /var is not mounted yet.
> > With this we are able to start unwind before the network is up and
> > partitions are mounted.
> 
> Sorry for being late to the party, I just upgraded to the latest snaps
> and DNS broke.  Reverting this diff unbreaks unwind(8) operations.
> 

Could you be more specific what broke?

I was going to revert at least half of that diff anyway because I
think it's just too ugly what we are doing here and I'm persuing a
different solution to the overall problem.

> My unwind.conf:
> 
>   preference { recursor }
> 
> (Can't reproduce this problem with an empty config file.)
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

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



nsd 4.3.5

2021-02-01 Thread Florian Obser


4.3.5

BUG FIXES:
- Fix #143: xfrd no hysteresis with NOT IMPLEMENTED rcode.
- Fix #144: Typo fix in nsd.conf.5.in.
- For #145: Fix that service of remaining TCP and TLS connections
  does not allow new queries to be made, the connection is closed.
  Only existing queries and zone transfers are answered, new ones
  are rejected by a close of the channel.
- Fix that nsd-control has timeout when connection is down.
- remove windows socket ifdefs from nsd-control.
- Fix #148: CNAME need not be followed after a synthesized CNAME
  for a CNAME query.
- Fix configure.ac for autoconf 2.70.
- Fix #150: TXT record validation difference with BIND.
- Fix #151: DNAME not applied more than once to resolve the query.
- Fix #152: '*' in Rdata causes the return code to be NOERROR instead
  of NX.

Tests, OKs?

diff --git acx_nlnetlabs.m4 acx_nlnetlabs.m4
index 31e43d67e87..d33352f17b8 100644
--- acx_nlnetlabs.m4
+++ acx_nlnetlabs.m4
@@ -2,7 +2,9 @@
 # Copyright 2009, Wouter Wijngaards, NLnet Labs.   
 # BSD licensed.
 #
-# Version 35
+# Version 37
+# 2021-01-05 fix defun for aclocal
+# 2021-01-05 autoconf 2.70 autoupdate and fixes, no AC_TRY_COMPILE
 # 2020-08-24 Use EVP_sha256 instead of HMAC_Update (for openssl-3.0.0).
 # 2016-03-21 Check -ldl -pthread for libcrypto for ldns and openssl 1.1.0.
 # 2016-03-21 Use HMAC_Update instead of HMAC_CTX_Init (for openssl-1.1.0).
@@ -447,15 +449,12 @@ AC_DEFUN([ACX_CHECK_FORMAT_ATTRIBUTE],
 AC_MSG_CHECKING(whether the C compiler (${CC-cc}) accepts the "format" 
attribute)
 AC_CACHE_VAL(ac_cv_c_format_attribute,
 [ac_cv_c_format_attribute=no
-AC_TRY_COMPILE(
-[#include 
+AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include 
 void f (char *format, ...) __attribute__ ((format (printf, 1, 2)));
 void (*pf) (char *format, ...) __attribute__ ((format (printf, 1, 2)));
-], [
+]], [[
f ("%s", "str");
-],
-[ac_cv_c_format_attribute="yes"],
-[ac_cv_c_format_attribute="no"])
+]])],[ac_cv_c_format_attribute="yes"],[ac_cv_c_format_attribute="no"])
 ])
 
 AC_MSG_RESULT($ac_cv_c_format_attribute)
@@ -484,14 +483,11 @@ AC_DEFUN([ACX_CHECK_UNUSED_ATTRIBUTE],
 AC_MSG_CHECKING(whether the C compiler (${CC-cc}) accepts the "unused" 
attribute)
 AC_CACHE_VAL(ac_cv_c_unused_attribute,
 [ac_cv_c_unused_attribute=no
-AC_TRY_COMPILE(
-[#include 
+AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include 
 void f (char *u __attribute__((unused)));
-], [
+]], [[
f ("x");
-],
-[ac_cv_c_unused_attribute="yes"],
-[ac_cv_c_unused_attribute="no"])
+]])],[ac_cv_c_unused_attribute="yes"],[ac_cv_c_unused_attribute="no"])
 ])
 
 dnl Setup ATTR_UNUSED config.h parts.
@@ -548,7 +544,7 @@ dnl as a requirement so that is gets called before LIBTOOL
 dnl because libtools 'AC_REQUIRE' names are right after this one, before
 dnl this function contents.
 AC_REQUIRE([ACX_LIBTOOL_C_PRE])
-AC_PROG_LIBTOOL
+LT_INIT
 ])
 
 dnl Detect if u_char type is defined, otherwise define it.
@@ -677,14 +673,14 @@ AC_DEFUN([ACX_SSL_CHECKS], [
 AC_MSG_CHECKING([for EVP_sha256 in -lcrypto])
 LIBS="$LIBS -lcrypto"
 LIBSSL_LIBS="$LIBSSL_LIBS -lcrypto"
-AC_TRY_LINK(, [
+AC_LINK_IFELSE([AC_LANG_PROGRAM([[]], [[
 int EVP_sha256(void);
 (void)EVP_sha256();
-  ], [
+  ]])],[
 AC_MSG_RESULT(yes)
 AC_DEFINE([HAVE_EVP_SHA256], 1,
   [If you have EVP_sha256])
-  ], [
+  ],[
 AC_MSG_RESULT(no)
 # check if -lwsock32 or -lgdi32 are needed.
 BAKLIBS="$LIBS"
@@ -692,10 +688,10 @@ AC_DEFUN([ACX_SSL_CHECKS], [
LIBS="$LIBS -lgdi32 -lws2_32"
LIBSSL_LIBS="$LIBSSL_LIBS -lgdi32 -lws2_32"
 AC_MSG_CHECKING([if -lcrypto needs -lgdi32])
-AC_TRY_LINK([], [
+AC_LINK_IFELSE([AC_LANG_PROGRAM([[]], [[
 int EVP_sha256(void);
 (void)EVP_sha256();
-  ],[
+  ]])],[
 AC_DEFINE([HAVE_EVP_SHA256], 1,
 [If you have EVP_sha256])
 AC_MSG_RESULT(yes) 
@@ -706,10 +702,10 @@ AC_DEFUN([ACX_SSL_CHECKS], [
 LIBS="$LIBS -ldl"
 LIBSSL_LIBS="$LIBSSL_LIBS -ldl"
 AC_MSG_CHECKING([if -lcrypto needs -ldl])
-AC_TRY_LINK([], [
+AC_LINK_IFELSE([AC_LANG_PROGRAM([[]], [[
 int EVP_sha256(void);
 (void)EVP_sha256();
-  ],[
+  ]])],[
 AC_DEFINE([HAVE_EVP_SHA256], 1,
 [If you have EVP_sha256])
 AC_MSG_RESULT(yes) 
@@ -720,10 +716,10 @@ AC_DEFUN([ACX_SSL_CHECKS], [

Re: unwind(8): use SO_BINDANY

2021-01-29 Thread Florian Obser
Hold off on this for now, claudio pointed out that I might not be
supposed to use SO_BINDANY like this.

On Fri, Jan 29, 2021 at 04:51:46PM +0100, Florian Obser wrote:
> I want to start unwind earlier, around the time when slaacd comes up,
> the network is not up at that point. Set SO_BINDANY to be able to
> already bind upd/53 and tcp/53 on localhost.
> This will make integration with dhclient easier (I hope).
> 
> diff --git unwind.c unwind.c
> index 00c600560e4..9bfc4dcf3b8 100644
> --- unwind.c
> +++ unwind.c
> @@ -746,6 +746,9 @@ open_ports(void)
>   if (setsockopt(udp4sock, SOL_SOCKET, SO_SNDBUF, ,
>   sizeof(bsize)) == -1)
>   log_warn("setting SO_SNDBUF on socket");
> + if (setsockopt(udp4sock, SOL_SOCKET, SO_BINDANY, ,
> + sizeof(opt)) == -1)
> + log_warn("setting SO_BINDANY on socket");
>   if (bind(udp4sock, res0->ai_addr, res0->ai_addrlen)
>   == -1) {
>   close(udp4sock);
> @@ -767,6 +770,9 @@ open_ports(void)
>   if (setsockopt(udp6sock, SOL_SOCKET, SO_SNDBUF, ,
>   sizeof(bsize)) == -1)
>   log_warn("setting SO_SNDBUF on socket");
> + if (setsockopt(udp6sock, SOL_SOCKET, SO_BINDANY, ,
> + sizeof(opt)) == -1)
> + log_warn("setting SO_BINDANY on socket");
>   if (bind(udp6sock, res0->ai_addr, res0->ai_addrlen)
>   == -1) {
>   close(udp6sock);
> @@ -791,6 +797,9 @@ open_ports(void)
>   if (setsockopt(tcp4sock, SOL_SOCKET, SO_SNDBUF, ,
>   sizeof(bsize)) == -1)
>   log_warn("setting SO_SNDBUF on socket");
> + if (setsockopt(tcp4sock, SOL_SOCKET, SO_BINDANY, ,
> + sizeof(opt)) == -1)
> + log_warn("setting SO_BINDANY on socket");
>   if (bind(tcp4sock, res0->ai_addr, res0->ai_addrlen)
>   == -1) {
>   close(tcp4sock);
> @@ -817,6 +826,9 @@ open_ports(void)
>   if (setsockopt(tcp6sock, SOL_SOCKET, SO_SNDBUF, ,
>   sizeof(bsize)) == -1)
>   log_warn("setting SO_SNDBUF on socket");
> + if (setsockopt(tcp6sock, SOL_SOCKET, SO_BINDANY, ,
> + sizeof(opt)) == -1)
> + log_warn("setting SO_BINDANY on socket");
>   if (bind(tcp6sock, res0->ai_addr, res0->ai_addrlen)
>   == -1) {
>   close(tcp6sock);
> 
> 
> -- 
> I'm not entirely sure you are real.
> 

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



Re: rc(8): start unwind earlier

2021-01-29 Thread Florian Obser
On Fri, Jan 29, 2021 at 04:56:47PM +0100, Antoine Jacoutot wrote:
> On Fri, Jan 29, 2021 at 04:53:34PM +0100, Florian Obser wrote:
> > Start unwind earlier.
> > 
> > OK?
> > 
> > diff --git rc rc
> > index 94465add54f..7b5f835f0af 100644
> > --- rc
> > +++ rc
> > @@ -442,6 +442,7 @@ fill_baddynamic tcp
> >  sysctl_conf
> >  
> >  start_daemon slaacd >/dev/null 2>&1
> > +start_daemon unwind >/dev/null 2>&1
> 
> you could do it all in 1 line :-)
> start_daemon slaacd unwind >/dev/null 2>&1

thanks, fixed in my tree.

> 
> >  
> >  echo 'starting network'
> >  
> > @@ -454,8 +455,6 @@ sh /etc/netstart
> >  mount -s /usr >/dev/null 2>&1
> >  mount -s /var >/dev/null 2>&1
> >  
> > -start_daemon unwind >/dev/null 2>&1
> > -
> >  # Load pf rules and bring up pfsync interface.
> >  if [[ $pf != NO ]]; then
> > if [[ -f /etc/pf.conf ]]; then
> > 
> > 
> > -- 
> > I'm not entirely sure you are real.
> > 
> 
> -- 
> Antoine
> 

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



rc(8): start unwind earlier

2021-01-29 Thread Florian Obser
Start unwind earlier.

OK?

diff --git rc rc
index 94465add54f..7b5f835f0af 100644
--- rc
+++ rc
@@ -442,6 +442,7 @@ fill_baddynamic tcp
 sysctl_conf
 
 start_daemon slaacd >/dev/null 2>&1
+start_daemon unwind >/dev/null 2>&1
 
 echo 'starting network'
 
@@ -454,8 +455,6 @@ sh /etc/netstart
 mount -s /usr >/dev/null 2>&1
 mount -s /var >/dev/null 2>&1
 
-start_daemon unwind >/dev/null 2>&1
-
 # Load pf rules and bring up pfsync interface.
 if [[ $pf != NO ]]; then
if [[ -f /etc/pf.conf ]]; then


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



unwind(8): open DNSSEC trustanchor late

2021-01-29 Thread Florian Obser
Last piece of the puzzle...

Re-try to open DNSSEC trust anchor file if /var is not mounted yet.
With this we are able to start unwind before the network is up and
partitions are mounted.

diff --git frontend.c frontend.c
index 18d91dfbeb2..5ca733762c9 100644
--- frontend.c
+++ frontend.c
@@ -258,8 +258,6 @@ frontend(int debug, int verbose)
TAILQ_INIT(_anchors);
TAILQ_INIT(_trust_anchors);
 
-   add_new_ta(_anchors, KSK2017);
-
event_dispatch();
 
frontend_shutdown();
@@ -448,10 +446,21 @@ frontend_dispatch_main(int fd, short event, void *bula)
control_listen(fd);
break;
case IMSG_TAFD:
-   if ((ta_fd = imsg.fd) != -1)
+   if ((ta_fd = imsg.fd) == -1)
+   fatalx("%s: expected to receive imsg trust "
+   "anchor fd but didn't receive any",
+   __func__);
+   if (TAILQ_EMPTY(_anchors)) {
+   /*
+* We did not receive a trustanchor from DNS,
+* maybe the built-in one is out of date, try
+* with the one from disk.
+*/
parse_trust_anchor(_anchors, ta_fd);
-   if (!TAILQ_EMPTY(_anchors))
-   send_trust_anchors(_anchors);
+   if (!TAILQ_EMPTY(_anchors))
+   send_trust_anchors(_anchors);
+   } else
+   write_trust_anchors(_anchors, ta_fd);
break;
case IMSG_BLFD:
if ((fd = imsg.fd) == -1)
diff --git resolver.c resolver.c
index 006632e0303..d7f0bc7684b 100644
--- resolver.c
+++ resolver.c
@@ -423,6 +423,8 @@ resolver(int debug, int verbose)
TAILQ_INIT(_trust_anchors);
TAILQ_INIT(_queries);
 
+   add_new_ta(_anchors, KSK2017);
+
event_dispatch();
 
resolver_shutdown();
diff --git unwind.c unwind.c
index 9bfc4dcf3b8..9a354cf9ade 100644
--- unwind.c
+++ unwind.c
@@ -49,6 +49,8 @@
 #include "control.h"
 
 #defineTRUST_ANCHOR_FILE   "/var/db/unwind.key"
+#defineWAIT_TA_FD_TIMEOUT  5
+#defineWAIT_TA_FD_MAX_RETRY3
 
 enum uw_process {
PROC_MAIN,
@@ -74,6 +76,8 @@ int   main_sendall(enum imsg_type, void *, uint16_t);
 void   open_ports(void);
 void   solicit_dns_proposals(void);
 void   send_blocklist_fd(void);
+void   open_trustanchor(void);
+void   open_trustanchor_timeout(int, short, void *);
 
 struct uw_conf *main_conf;
 static struct imsgev   *iev_frontend;
@@ -83,6 +87,7 @@ pid_t  frontend_pid;
 pid_t   resolver_pid;
 uint32_tcmd_opts;
 int routesock;
+struct eventta_timo_ev;
 
 void
 main_sig_handler(int sig, short event, void *arg)
@@ -125,7 +130,7 @@ main(int argc, char *argv[])
int  ch, debug = 0, resolver_flag = 0, frontend_flag = 0;
int  frontend_routesock, rtfilter;
int  pipe_main2frontend[2], pipe_main2resolver[2];
-   int  control_fd, ta_fd;
+   int  control_fd;
char*csock, *saved_argv0;
 
csock = UNWIND_SOCKET;
@@ -280,12 +285,6 @@ main(int argc, char *argv[])
fatal("route socket");
shutdown(SHUT_RD, routesock);
 
-   if ((ta_fd = open(TRUST_ANCHOR_FILE, O_RDWR | O_CREAT, 0644)) == -1)
-   log_warn("%s", TRUST_ANCHOR_FILE);
-
-   /* receiver handles failed open correctly */
-   main_imsg_compose_frontend_fd(IMSG_TAFD, 0, ta_fd);
-
main_imsg_compose_frontend_fd(IMSG_CONTROLFD, 0, control_fd);
main_imsg_compose_frontend_fd(IMSG_ROUTESOCK, 0, frontend_routesock);
main_imsg_send_config(main_conf);
@@ -293,9 +292,17 @@ main(int argc, char *argv[])
if (main_conf->blocklist_file != NULL)
send_blocklist_fd();
 
-   if (pledge("stdio rpath sendfd", NULL) == -1)
+   /* this is the best we can do, when we startup /var is not mounted */
+   if (unveil("/var", "rwc") == -1)
+   fatal("unveil");
+   if (unveil("/", "r") == -1)
+   fatal("unveil");
+   if (pledge("stdio rpath wpath cpath sendfd", NULL) == -1)
fatal("pledge");
 
+   evtimer_set(_timo_ev, open_trustanchor_timeout, NULL);
+   open_trustanchor();
+
main_imsg_compose_frontend(IMSG_STARTUP, 0, NULL, 0);
main_imsg_compose_resolver(IMSG_STARTUP, 0, NULL, 0);
 
@@ -971,3 +978,31 @@ imsg_receive_config(struct imsg *imsg, struct uw_conf 
**xconf)
break;
}
 }
+
+void

unwind(8): use SO_BINDANY

2021-01-29 Thread Florian Obser
I want to start unwind earlier, around the time when slaacd comes up,
the network is not up at that point. Set SO_BINDANY to be able to
already bind upd/53 and tcp/53 on localhost.
This will make integration with dhclient easier (I hope).

diff --git unwind.c unwind.c
index 00c600560e4..9bfc4dcf3b8 100644
--- unwind.c
+++ unwind.c
@@ -746,6 +746,9 @@ open_ports(void)
if (setsockopt(udp4sock, SOL_SOCKET, SO_SNDBUF, ,
sizeof(bsize)) == -1)
log_warn("setting SO_SNDBUF on socket");
+   if (setsockopt(udp4sock, SOL_SOCKET, SO_BINDANY, ,
+   sizeof(opt)) == -1)
+   log_warn("setting SO_BINDANY on socket");
if (bind(udp4sock, res0->ai_addr, res0->ai_addrlen)
== -1) {
close(udp4sock);
@@ -767,6 +770,9 @@ open_ports(void)
if (setsockopt(udp6sock, SOL_SOCKET, SO_SNDBUF, ,
sizeof(bsize)) == -1)
log_warn("setting SO_SNDBUF on socket");
+   if (setsockopt(udp6sock, SOL_SOCKET, SO_BINDANY, ,
+   sizeof(opt)) == -1)
+   log_warn("setting SO_BINDANY on socket");
if (bind(udp6sock, res0->ai_addr, res0->ai_addrlen)
== -1) {
close(udp6sock);
@@ -791,6 +797,9 @@ open_ports(void)
if (setsockopt(tcp4sock, SOL_SOCKET, SO_SNDBUF, ,
sizeof(bsize)) == -1)
log_warn("setting SO_SNDBUF on socket");
+   if (setsockopt(tcp4sock, SOL_SOCKET, SO_BINDANY, ,
+   sizeof(opt)) == -1)
+   log_warn("setting SO_BINDANY on socket");
if (bind(tcp4sock, res0->ai_addr, res0->ai_addrlen)
== -1) {
close(tcp4sock);
@@ -817,6 +826,9 @@ open_ports(void)
if (setsockopt(tcp6sock, SOL_SOCKET, SO_SNDBUF, ,
sizeof(bsize)) == -1)
log_warn("setting SO_SNDBUF on socket");
+   if (setsockopt(tcp6sock, SOL_SOCKET, SO_BINDANY, ,
+   sizeof(opt)) == -1)
+   log_warn("setting SO_BINDANY on socket");
if (bind(tcp6sock, res0->ai_addr, res0->ai_addrlen)
== -1) {
close(tcp6sock);


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



unwind(8): recheck on libunbound config changes

2021-01-29 Thread Florian Obser
Some libunbound configuration changes can change the quality of a
resolver so we have to schedule a re-check.

OK?

diff --git resolver.c resolver.c
index feeb6c2f27a..006632e0303 100644
--- resolver.c
+++ resolver.c
@@ -175,7 +175,7 @@ void replace_forwarders(struct 
uw_forwarder_head *,
 voidresolver_ref(struct uw_resolver *);
 voidresolver_unref(struct uw_resolver *);
 int resolver_cmp(const void *, const void *);
-voidrestart_ub_resolvers(void);
+voidrestart_ub_resolvers(int);
 voidshow_status(pid_t);
 voidshow_autoconf(pid_t);
 voidshow_mem(pid_t);
@@ -501,7 +501,7 @@ resolver_dispatch_frontend(int fd, short event, void *bula)
memcpy(, imsg.data, sizeof(verbose));
if (log_getdebug() && (log_getverbose() & OPT_VERBOSE3)
!= (verbose & OPT_VERBOSE3))
-   restart_ub_resolvers();
+   restart_ub_resolvers(0);
log_setverbose(verbose);
break;
case IMSG_QUERY:
@@ -545,7 +545,7 @@ resolver_dispatch_frontend(int fd, short event, void *bula)
break;
case IMSG_NEW_TAS_DONE:
if (merge_tas(_trust_anchors, _anchors))
-   restart_ub_resolvers();
+   restart_ub_resolvers(1);
break;
case IMSG_NETWORK_CHANGED:
clock_gettime(CLOCK_MONOTONIC, _network_change);
@@ -577,7 +577,7 @@ resolver_dispatch_frontend(int fd, short event, void *bula)
sizeof(new_available_afs));
if (new_available_afs != available_afs) {
available_afs = new_available_afs;
-   restart_ub_resolvers();
+   restart_ub_resolvers(1);
}
break;
default:
@@ -1754,14 +1754,20 @@ resolver_cmp(const void *_a, const void *_b)
 }
 
 void
-restart_ub_resolvers(void)
+restart_ub_resolvers(int recheck)
 {
-   int  i;
+   int  i;
+   enum uw_resolver_state   state;
 
-   for (i = 0; i < UW_RES_NONE; i++)
-   if (i != UW_RES_ASR)
-   new_resolver(i, resolvers[i] != NULL ?
-   resolvers[i]->state : UNKNOWN);
+   for (i = 0; i < UW_RES_NONE; i++) {
+   if (i == UW_RES_ASR)
+   continue;
+   if (recheck || resolvers[i] == NULL)
+   state = UNKNOWN;
+   else
+   state = resolvers[i]->state;
+   new_resolver(i, state);
+   }
 }
 
 void

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



unwind(8): ignore old check results

2021-01-27 Thread Florian Obser


A new resolver can be created while we currently run a check with the
old configuration. We will then request another check that runs in
parallel to the old check. If the new check finishes earlier, the
current check result will be overwritten by an outdated check result
which is likely wrong.
While here fix some whitespace.

I have only been able to trigger this artificially but I think it can
happen in the wild if we learn new autoconf forwarders at the wrong
time.

OK?

diff --git resolver.c resolver.c
index 49f51d8a9ab..7d65d511432 100644
--- resolver.c
+++ resolver.c
@@ -1513,8 +1513,6 @@ check_resolver(struct uw_resolver *resolver_to_check)
evtimer_add(_to_check->check_ev,
_to_check->check_tv);
}
-
-
 }
 
 void
@@ -1529,6 +1527,12 @@ check_resolver_done(struct uw_resolver *res, void *arg, 
int rcode,
 
checked_resolver->check_running--;
 
+   if (checked_resolver != resolvers[checked_resolver->type]) {
+   log_debug("%s: %s: ignoring late check result", __func__,
+   uw_resolver_type_str[checked_resolver->type]);
+   goto out;
+   }
+
prev_state = checked_resolver->state;
 
if (answer_len < LDNS_HEADER_SIZE) {


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



Re: unwind(8): only use available address families

2021-01-26 Thread Florian Obser
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,
> > , sizeof(rtfilter)) == -1)
> > fatal("setsockopt(ROUTE_MSGFILTER)");
> 

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



Re: unwind(8): only use available address families

2021-01-26 Thread Florian Obser
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(_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,
_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() != 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_addr) ||
+   IN6_IS_ADDR_LINKLOCAL(>sin6_addr) ||
+   IN6_IS_ADDR_MC_LINKLOCAL(>sin6_addr) ||
+   IN6_IS_ADDR_MC_INTFACELOCAL(>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,
+   _af, sizeof(available_af));
+   }
+}
diff --git frontend.h frontend.h
index cd6c21875af..0c18c7524dc 100644
--- frontend.h
+++ frontend

Re: unwind(8): only use available address families

2021-01-26 Thread Florian Obser
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

2021-01-25 Thread Florian Obser



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.



unwind(8): only use available address families

2021-01-25 Thread Florian Obser
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(_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,
_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)) == -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() != 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_addr) ||
+   IN6_IS_ADDR_LINKLOCAL(>sin6_addr) ||
+   IN6_IS_ADDR_MC_LINKLOCAL(>sin6_addr) ||
+   IN6_IS_ADDR_MC_INTFACELOCAL(>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,
+   _af, 

unwind(8): disable logging to syslog from libunbound

2021-01-25 Thread Florian Obser
We are not getting anything useful for us out of it and it can be
quite noisy when we are missing IPv4 or IPv6 addresses as pointed out
by kn@.
It is still available when logging to stderr when running with -d.

OK?

Also shown a revert for a local diff we are carrying, I'll commit that
seperately.

diff --git libunbound/util/log.c libunbound/util/log.c
index e8e987963c5..dfbb2334994 100644
--- libunbound/util/log.c
+++ libunbound/util/log.c
@@ -109,20 +109,16 @@ log_init(const char* filename, int use_syslog, const 
char* chrootdir)
fclose(cl);
}
 #ifdef HAVE_SYSLOG_H
-#if 0  /* unwind handles syslog for us */
if(logging_to_syslog) {
closelog();
logging_to_syslog = 0;
}
-#endif
if(use_syslog) {
/* do not delay opening until first write, because we may
 * chroot and no longer be able to access dev/log and so on */
/* the facility is LOG_DAEMON by default, but
 * --with-syslog-facility=LOCAL[0-7] can override it */
-#if 0  /* unwind handles syslog for us */
openlog(ident, LOG_NDELAY, UB_SYSLOG_FACILITY);
-#endif
logging_to_syslog = 1;
lock_basic_unlock(_lock);
return;
diff --git resolver.c resolver.c
index e800d38280a..a3521e8025f 100644
--- resolver.c
+++ resolver.c
@@ -498,7 +498,7 @@ resolver_dispatch_frontend(int fd, short event, void *bula)
"%lu", __func__,
IMSG_DATA_SIZE(imsg));
memcpy(, imsg.data, sizeof(verbose));
-   if ((log_getverbose() & OPT_VERBOSE3)
+   if (log_getdebug() && (log_getverbose() & OPT_VERBOSE3)
!= (verbose & OPT_VERBOSE3))
restart_ub_resolvers();
log_setverbose(verbose);
@@ -1257,13 +1257,14 @@ create_resolver(enum uw_resolver_type type)
 
if (!log_getdebug()) {
if((err = ub_ctx_set_option(res->ctx, "use-syslog:",
-   "yes")) != 0) {
+   "no")) != 0) {
ub_ctx_delete(res->ctx);
free(res);
-   log_warnx("error setting use-syslog: yes: %s",
+   log_warnx("error setting use-syslog: no: %s",
ub_strerror(err));
return (NULL);
}
+   ub_ctx_debugout(res->ctx, NULL);
}
 
break;
diff --git unwind.8 unwind.8
index f1c8c5cc827..8b784a1399a 100644
--- unwind.8
+++ unwind.8
@@ -92,6 +92,8 @@ Produce more verbose output.
 Multiple
 .Fl v
 options increase the verbosity.
+Debug output from libunbound is only available when logging to
+.Em stderr .
 .El
 .Sh FILES
 .Bl -tag -width "/var/db/unwind.keyXXX" -compact



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



Re: unwind: silence "udp connect failed" errors

2021-01-24 Thread Florian Obser
On Sun, Jan 24, 2021 at 01:06:31PM +0100, Klemens Nanni wrote:
> On Sun, Jan 24, 2021 at 12:52:50PM +0100, Theo Buehler wrote:
> > Probably better to sync first with the corresponding unbound commit
> > https://cvsweb.openbsd.org/src/usr.sbin/unbound/services/outside_network.c#rev1.21
> > then adjust udp_connect_needs_log() as needed.
> Good call, thanks.
> 
> Here's the combined diff that syncs with unbound and adds EADDRNOTAVAIL
> in the same fashion.
> 
> In case that is OK, I'd commit sync and addition separately.
> 
> Feedback? OK?

I consider the libunbound directory off-limits, it is not a fork and
it should not be a fork.

We currently carry two diffs in there and I want to get rid of them
(see at the end).

Especialy the local syslog one is stupid. I think we are doing
something wrong or libunbound does something wrong, not sure.

Anyway, I think what we should actually do is disable logging in
libunbound unless we crank logging to debug in unwind. There is
nothing interesting comming out of libunbound for us.

This leaves the 2nd problem, why is it going off the rails so badly if
there is no v4? Is it also doing this for v6 when that's not present?
Try resolving dnssec-failed.org, it's delegated to comcast which uses
5 nameservers all with v4 and v6 addresses. Since dnssec validation is
intentionally broken unwind tries to talk to *all* nameservers, the
log does not stop scrolling. It's so bad it actually times out on IPv6
only.

Is this a bug in (lib)unbound or is it doing what it's supposed to do?

It is possible that from the point of view of (lib)unbound it is
behaving perfectly fine, if you don't have an address family you are
supposed to hit the available button in unbound.conf.

If that is the case we should do the same, automatically. I mean
that's the whole point of unwind, figure out what works and use that.
Meaning if we detect that we don't have working IPv4 set use-ipv4: no.

commit d273f78b8643bdb01f621260eb323123b774e431
Author: florian 
Date:   Fri Dec 6 13:08:48 2019 +

Stop fiddling with openlog / closelog in libunbound. unwind handles
this. We need to find a way to properly upstream this.
OK otto

diff --git libunbound/util/log.c libunbound/util/log.c
index dfbb2334994..e8e987963c5 100644
--- libunbound/util/log.c
+++ libunbound/util/log.c
@@ -109,16 +109,20 @@ log_init(const char* filename, int use_syslog, const 
char* chrootdir)
fclose(cl);
}
 #ifdef HAVE_SYSLOG_H
+#if 0  /* unwind handles syslog for us */
if(logging_to_syslog) {
closelog();
logging_to_syslog = 0;
}
+#endif
if(use_syslog) {
/* do not delay opening until first write, because we may
 * chroot and no longer be able to access dev/log and so on */
/* the facility is LOG_DAEMON by default, but
 * --with-syslog-facility=LOCAL[0-7] can override it */
+#if 0  /* unwind handles syslog for us */
openlog(ident, LOG_NDELAY, UB_SYSLOG_FACILITY);
+#endif
logging_to_syslog = 1;
lock_basic_unlock(_lock);
return;

commit 81b0d744ff77e26ea69cee28aed10081d3973fe8
Author: otto 
Date:   Sat Dec 14 19:56:26 2019 +

Be less aggressive pre-allocating memory; ok florian@

diff --git libunbound/util/alloc.c libunbound/util/alloc.c
index 7e9618931ca..e9613b10dcd 100644
--- libunbound/util/alloc.c
+++ libunbound/util/alloc.c
@@ -113,7 +113,7 @@ alloc_init(struct alloc_cache* alloc, struct alloc_cache* 
super,
alloc->last_id -= 1;/* for compiler portability. */
alloc->last_id |= alloc->next_id;
alloc->next_id += 1;/* because id=0 is special. */
-   alloc->max_reg_blocks = 100;
+   alloc->max_reg_blocks = 10;
alloc->num_reg_blocks = 0;
alloc->reg_list = NULL;
alloc->cleanup = NULL;


> 
> 
> Index: libunbound/services/outside_network.c
> ===
> RCS file: /cvs/src/sbin/unwind/libunbound/services/outside_network.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 outside_network.c
> --- libunbound/services/outside_network.c 11 Dec 2020 12:21:40 -  
> 1.9
> +++ libunbound/services/outside_network.c 24 Jan 2021 12:03:38 -
> @@ -1745,6 +1745,36 @@ select_id(struct outside_network* outnet
>   return 1;
>  }
>  
> +/** return true is UDP connect error needs to be logged */
> +static int udp_connect_needs_log(int err)
> +{
> + switch(err) {
> + case ECONNREFUSED:
> +#  ifdef ENETUNREACH
> + case ENETUNREACH:
> +#  endif
> +#  ifdef EHOSTDOWN
> + case EHOSTDOWN:
> +#  endif
> +#  ifdef EHOSTUNREACH
> + case EHOSTUNREACH:
> +#  endif
> +#  ifdef ENETDOWN
> + case ENETDOWN:
> +#  endif
> +#  ifdef EADDRNOTAVAIL
> + case EADDRNOTAVAIL:
> +#  endif
> + if(verbosity >= VERB_ALGO)
> + 

Re: unwind(8): Implement DNS64 synthesis.

2021-01-24 Thread Florian Obser
On Sun, Jan 24, 2021 at 11:12:49AM +0100, Klemens Nanni wrote:
> What I'm seeing here is that unwind forwards the very first query to my
> gateway (learned via SLAAC), that one succeeds, but all successive 
> queries of A only domains do not work... that's what makes the query in
> my previous mail work.  I can always reproduce it like that:
> 
>   $ doas rcctl restart unwind 
>   unwind(ok)
>   unwind(ok)
>   $ dig +noall +question +answer github.com 
>   ;github.com.IN  
>   github.com. 44  IN  64:ff9b::8c52:7904
>   $ dig +noall +question +answer github.com 
>   ;github.com.IN  
> 
> Here's the output of `unwind -dv' (with 'udp connect failed' log spam
> for IPv4 addresses removed):
> 
>   check_resolver_done: dhcp: unknown
>   check_resolver_done: stub: unknown
>   check_resolver_done: oDoT-dhcp rcode: SERVFAIL
>   check_resolver_done: recursor: unknown
>   [127.0.0.1]:22827: github.com. IN  ?
>   try_next_resolver[+0ms]: recursor[validating] github.com. IN 
>   try_next_resolver[+6ms]: dhcp[validating] github.com. IN 
>   try_next_resolver[+6ms]: stub[resolving] github.com. IN 
>   try_next_resolver: could not find (any more) working resolvers
>   resolve_done[stub]: github.com. IN  rcode: NOERROR[0], elapsed: 
> 1ms, running: 1
>   check_resolver_done: oDoT-dhcp rcode: SERVFAIL
>   [127.0.0.1]:5226: github.com. IN  ?
>   try_next_resolver[+0ms]: dhcp[validating] github.com. IN 
>   resolve_done[dhcp]: github.com. IN  rcode: NOERROR[0], elapsed: 
> 0ms, running: 1
> 

Are you sure you are running with the config you think you are running
with? I can not reproduce and from the logging, especially the
check_resolver_done bits it very much looks like you are running
without any config at all.

$ obj/unwind -nvf ./unwind.conf
preference { recursor }
$ doas obj/unwind -dvf ./unwind.conf
check_resolver_done: recursor: unknown
[::1]:39989: ripe.net. IN  ?
try_next_resolver[+0ms]: recursor[validating] ripe.net. IN 
try_next_resolver: could not find (any more) working resolvers
resolve_done[recursor]: ripe.net. IN  rcode: NOERROR[0], elapsed: 403ms, 
running: 1


> > > 
> > > So it seems unwind will effectively always synthesize, no matter what
> > > `preference {}' is used.
> > > 
> > > I also verified that unwind always asks my autoconf resolver for
> > > ipv4only.arpa by using tcpdump and running with this diff
> > 
> > Yes, that's intentional. That's how it learns the NAT64 prefix.
> Asking because my assumption was that, if I don't configured `stub' or
> `dhcp' in unwind.conf, the NAT64 would/should not be discovered.
> As in: Why does unwind do LAN specific DNS64 synthesis if I told it not
> to use any LAN specific resolvers.
> 
> I'll assume my logic is flawed here.

[...]

> I assumed unwind would not do DNS64 at all in cases like
> `preference { resolver }'.

I don't know. So you find yourself in a IPv6 only network which does
provide a NAT64 service, you'd rather not be able to reach IPv4-only
services?

Note that from the point of view of DNS you are talking to the right
destination, we will only do synthesis if we are not receiving a bogus
answer.

>From my point of view detecting presence of NAT64 and doing synthesis
is independent of the resolving strategy preference. We are not asking
dhcp or the stub to do resolving, we fire up one query to detect the
NAT64 prefix.
Are you saying you need to switch to turn off DNS64, or is it fine
like this and you were just surprised?

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



Re: unwind(8): Implement DNS64 synthesis.

2021-01-24 Thread Florian Obser
On Sun, Jan 24, 2021 at 09:35:26AM +0100, Klemens Nanni wrote:
> On Thu, Jan 21, 2021 at 05:16:24PM +0100, Florian Obser wrote:
> > When unwind(8) learns new autoconf resolvers (from dhcp or router
> > advertisements) it checks if a DNS64 is present in this network
> > location and tries to recover the IPv6 prefix used according to
> > RFC7050.
> I noticed that unwind learns autoconf resolvers regardless of "stub" or
> "dhcp" being present in unwind.conf:
> 
>   $ doas rcctl restart unwind
>   unwind(ok)
>   unwind(ok)
>   $ unwind -nv
>   preference { recursor }
>   $ unwindctl status autoconf
>   autoconfiguration forwarders:
>   SLAAC[trunk0]: fd00::1

Yes, the config is present, it's just not being used.
You can also define forwarders or DoT forwarders and not use them.

>   $ dig +short @::1 github.com. 
>   64:ff9b::8c52:7903
> 
> This happens without your DNS64 diff already.

This does not make any sense, is your gateway intercepting DNS
queries?

> 
> So it seems unwind will effectively always synthesize, no matter what
> `preference {}' is used.
> 
> I also verified that unwind always asks my autoconf resolver for
> ipv4only.arpa by using tcpdump and running with this diff

Yes, that's intentional. That's how it learns the NAT64 prefix.

They way NAT64/DNS64 is normally setup is that the gateway performes
NAT64 and Router Advertisements contain nameservers that perform
DNS64. So we ask those for a name that is known to not have a 
record to force them to perform synthesis.

As in, we have to ask those, we can't just do our own recurion and
hope that someone intercepts our query. See check_dns64, that
intentionally uses ASR to learn the prefix.

> 
> Index: libunbound/dns64/dns64.c
> ===
> RCS file: /cvs/src/sbin/unwind/libunbound/dns64/dns64.c,v
> retrieving revision 1.4
> diff -u -p -U0 -r1.4 dns64.c
> --- libunbound/dns64/dns64.c  29 Aug 2020 08:22:42 -  1.4
> +++ libunbound/dns64/dns64.c  24 Jan 2021 07:58:10 -
> @@ -65 +65 @@
> -static const char DEFAULT_DNS64_PREFIX[] = "64:ff9b::/96";
> +static const char DEFAULT_DNS64_PREFIX[] = "2001:db9::/96";
> 
> Is that intentional?

that code is not being used.

> 
> > The learned autoconf resolvers are then prevented from upgrading to
> > the validating state since DNS64 breaks DNSSEC.
> > 
> > unwind(8) can now perform its own synthesis. If a query for a 
> > record results in no answer we re-send the query for A and if that
> > leads to an answer we synthesize an  answer using the learned
> > prefixes.
> Using different configurations/preferences has been working just fine
> for me in IPv6 only networks, i.e. unwind properly picks up the DNS64
> prefix (if for example changed with unbound.conf(5)'s `dns64-prefix').
> 

So are you saying this works or does not? It kinda sounds like you
claim it already worked without the diff?

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



unwind(8): Implement DNS64 synthesis.

2021-01-21 Thread Florian Obser
When unwind(8) learns new autoconf resolvers (from dhcp or router
advertisements) it checks if a DNS64 is present in this network
location and tries to recover the IPv6 prefix used according to
RFC7050.

The learned autoconf resolvers are then prevented from upgrading to
the validating state since DNS64 breaks DNSSEC.

unwind(8) can now perform its own synthesis. If a query for a 
record results in no answer we re-send the query for A and if that
leads to an answer we synthesize an  answer using the learned
prefixes.

This depends on the previous two diffs and adds two files so apply
with -p0

Tests, OKs?



diff --git Makefile Makefile
index bd2bd7d9890..8d66fb9ddd2 100644
--- Makefile
+++ Makefile
@@ -2,6 +2,7 @@
 
 PROG=  unwind
 SRCS=  control.c resolver.c frontend.c log.c unwind.c parse.y printconf.c
+SRCS+= dns64_synth.c
 MAN=   unwind.8 unwind.conf.5
 
 .include "${.CURDIR}/libunbound/Makefile.inc"
diff --git dns64_synth.c dns64_synth.c
new file mode 100644
index 000..00a7d150765
--- /dev/null
+++ dns64_synth.c
@@ -0,0 +1,176 @@
+/* $OpenBSD$   */
+
+/*
+ * dns64/dns64.h - DNS64 module
+ *
+ * Copyright (c) 2009, Viagénie. All rights reserved.
+ *
+ * This software is open source.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * Redistributions of source code must retain the above copyright notice,
+ * this list of conditions and the following disclaimer.
+ *
+ * Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ *
+ * Neither the name of the NLNET LABS nor the names of its contributors may
+ * be used to endorse or promote products derived from this software without
+ * specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/*
+ * Copyright (c) 2021 Florian Obser 
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include 
+#include 
+
+#include 
+#include 
+
+#include 
+
+#include "libunbound/config.h"
+#include "libunbound/util/regional.h"
+#include "libunbound/util/data/msgencode.h"
+#include "libunbound/util/data/msgparse.h"
+#include "libunbound/util/data/msgreply.h"
+
+#include "log.h"
+#include "unwind.h"
+#include "frontend.h"
+#include "dns64_synth.h"
+
+extern struct dns64_prefix *dns64_prefixes;
+extern int  dns64_prefix_count;
+
+static void
+synthesize_(const struct in6_addr *in6, int prefixlen, const uint8_t *a,
+size_t a_len, uint8_t *, size_t _len)
+{
+   size_t i, pos;
+   memcpy(, in6->s6_addr, sizeof(in6->s6_addr));
+   for (i = 0, pos = prefixlen / 8; i < a_len && pos < _len;
+   i++, pos++) {
+   if (pos == 8)
+   [pos++] = 0;
+   [pos] = a[i];
+   }
+}
+
+/*
+ * Copied from unbound/dns64/dns64.c and slightly extended to work with more
+ * than one IPv6 prefix.
+ */
+
+void
+dns64_synth__data(const struct ub_packed_rrset_key* fk, const struct
+packed_rrset_data* fd, struct ub_packed_rrset_key *dk, struct
+packed_rrset_data **dd_out, struct regional *region)
+{
+   struct packed_rrset_data*dd;
+   size_t  

unwind(8): refactor resolv_conf creation for asr

2021-01-21 Thread Florian Obser
Move resolv_conf string generation for ASR to function; makes
upcomming DNS64 diff simpler.

OK?

diff --git resolver.c resolver.c
index d42d19c1087..2634b95c01f 100644
--- resolver.c
+++ resolver.c
@@ -195,6 +195,7 @@ int  running_query_cnt(void);
 int*resolvers_to_restart(struct uw_conf *,
 struct uw_conf *);
 const char *query_imsg2str(struct query_imsg *);
+char   *gen_resolv_conf(void);
 
 struct uw_conf *resolver_conf;
 static struct imsgev   *iev_frontend;
@@ -1172,10 +1173,9 @@ create_resolver(enum uw_resolver_type type)
 {
struct uw_resolver  *res;
struct trust_anchor *ta;
-   struct uw_forwarder *uw_forwarder;
size_t   i;
int  err;
-   char*resolv_conf = NULL, *tmp = NULL;
+   char*resolv_conf;
 
if ((res = calloc(1, sizeof(*res))) == NULL) {
log_warn("%s", __func__);
@@ -1193,18 +1193,11 @@ create_resolver(enum uw_resolver_type type)
free(res);
return (NULL);
}
-   TAILQ_FOREACH(uw_forwarder, _forwarder_list, entry) {
-   tmp = resolv_conf;
-   if (asprintf(_conf, "%snameserver %s\n", tmp ==
-   NULL ? "" : tmp, uw_forwarder->ip) == -1) {
-   free(tmp);
-   free(res);
-   log_warnx("could not create asr context");
-   return (NULL);
-   }
-   free(tmp);
+   if ((resolv_conf = gen_resolv_conf()) == NULL) {
+   free(res);
+   log_warnx("could not create asr context");
+   return (NULL);
}
-
if ((res->asr_ctx = asr_resolver_from_string(resolv_conf)) ==
NULL) {
free(res);
@@ -2146,3 +2139,21 @@ query_imsg2str(struct query_imsg *query_imsg)
qtype_buf);
return buf;
 }
+
+char *
+gen_resolv_conf()
+{
+   struct uw_forwarder *uw_forwarder;
+   char*resolv_conf = NULL, *tmp = NULL;
+
+   TAILQ_FOREACH(uw_forwarder, _forwarder_list, entry) {
+   tmp = resolv_conf;
+   if (asprintf(_conf, "%snameserver %s\n", tmp ==
+   NULL ? "" : tmp, uw_forwarder->ip) == -1) {
+   free(tmp);
+   return (NULL);
+   }
+   free(tmp);
+   }
+   return resolv_conf;
+}

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



unwind(8): SECURE answer & upgrade to validating answer

2021-01-21 Thread Florian Obser
Don't just blindly upgrade to VALIDATING if we see a SECURE answer.
This can happen if things improve after we check a strategy, for
example ntpd corrected the time.

Let's go through the check_resolver() / new_resolver() code path
which will also hook up the resovler to the shared cache.

diff --git resolver.c resolver.c
index f5a1f3e1f59..d42d19c1087 100644
--- resolver.c
+++ resolver.c
@@ -1008,8 +1008,8 @@ resolve_done(struct uw_resolver *res, void *arg, int 
rcode,
if (result->rcode == LDNS_RCODE_SERVFAIL)
goto servfail;
 
-   if (sec == SECURE)
-   res->state = VALIDATING;
+   if (sec == SECURE && res->state != VALIDATING && res->stop != -1)
+   check_resolver(res);
 
if (res->state == VALIDATING && sec == BOGUS) {
answer_header->bogus = !force_acceptbogus;


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



Re: dig(1): replace inet_net_pton(3)

2021-01-20 Thread Florian Obser
On Wed, Jan 20, 2021 at 11:38:40AM +0100, Claudio Jeker wrote:
> On Tue, Jan 19, 2021 at 07:49:29PM +0100, Florian Obser wrote:
> > When we converted isc_sockaddr_t to sockaddr_storage we also moved to
> > inet_net_pton(3). It turns out that was a mistake, at least it's not
> > portable for AF_INET6. Effectively revert that part and hand-roll it
> > using inet_pton(3).
> > 
> > OK?
> 
> I thought inet_net_pton() for AF_INET would still be ok. Handling the

Or we haven't found the problems with AF_INET yet.
The code in lib/libc/net/inet_net_pton is rather grotesque.

The semantics are also mind-boggingly stupid.
 All numbers supplied as "parts" in a `.' notation may be decimal, octal,
 or hexadecimal, as specified in the C language (i.e., a leading 0x or 0X
 implies hexadecimal; otherwise, a leading 0 implies octal; otherwise, the
 number is interpreted as decimal).

"0x0a" is valid and is 10.0.0.0/8
"0x0a0b0c0d" is valid and is 10.11.12.13/32
"0xC0" is valid and is 192.0.0.0/24 because of course the damn thing
understands about network classes.

> AF_INET case by hand here seems rather unpleasant.
> 
> Since this code uses sockaddr to store the result why not use
> getaddrinfo().
> 
> In bgpd the code does more or less this:
> 1. extract the /prefixlen like you do.
> 2. use getnameinfo() on the prefix
> 3. if that fails use inet_net_pton(AF_INET) to parse short forms (like 10/8)

I think inet_net_pton plain needs killing. If we were to agree that a
sensible way to write IPv4 addresses is to use decimal numbers in the
range 0-255 seperated by dots (allowing short form) we could easily
write an AF-independent replacement using getnameinfo.

Nobody uses this except for us it seems. Ports is clean, I found one
hit in the FreeBSD base system in sbin/pfctl/pfctl_parser using only
AF_INET. NetBSD additionally enables it in ftpd.

Anyway, not a hill I'm going to die on.

> 
> Note: I think the bgpd code could also use some further cleanup.
> 
> I'm not sure if it makes sense to force this code to be portable by not
> using inet_net_pton(3). The code is currently non-portable because it uses
> the len fields of the sockaddr structs. So why bother about this function?

I'm restoring previous behaviour.

> In general if we plan to make a portable version of our dig the framework
> can provide a working inet_net_pton(3) version.
> 
> > p.s. it is kinda telling that isc, who introduced the API is (no
> > longer?) using it.
> > 
> > diff --git dighost.c dighost.c
> > index 2d2a52c86e2..2995b7e1602 100644
> > --- dighost.c
> > +++ dighost.c
> > @@ -935,10 +935,16 @@ parse_netprefix(struct sockaddr_storage **sap, int 
> > *plen, const char *value) {
> > struct sockaddr_storage *sa = NULL;
> > struct in_addr *in4;
> > struct in6_addr *in6;
> > -   int prefix_length;
> > +   int prefix_length = -1, i;
> > +   char *sep;
> > +   char buf[INET6_ADDRSTRLEN + sizeof("/128")];
> > +   const char *errstr;
> >  
> > REQUIRE(sap != NULL && *sap == NULL);
> >  
> > +   if (strlcpy(buf, value, sizeof(buf)) >= sizeof(buf))
> > +   fatal("invalid prefix '%s'\n", value);
> > +
> > sa = calloc(1, sizeof(*sa));
> > if (sa == NULL)
> > fatal("out of memory");
> > @@ -952,14 +958,36 @@ parse_netprefix(struct sockaddr_storage **sap, int 
> > *plen, const char *value) {
> > goto done;
> > }
> >  
> > -   if ((prefix_length = inet_net_pton(AF_INET6, value, in6, sizeof(*in6)))
> > -   != -1) {
> > +   sep = strchr(buf, '/');
> > +   if (sep != NULL) {
> > +   *sep++ = '\0';
> > +   prefix_length = strtonum(sep, 0, 128, );
> > +   if (errstr != NULL)
> > +   fatal("invalid address '%s'", value);
> > +   }
> > +
> > +   if (inet_pton(AF_INET6, buf, in6) == 1) {
> > sa->ss_len = sizeof(struct sockaddr_in6);
> > sa->ss_family = AF_INET6;
> > -   } else if ((prefix_length = inet_net_pton(AF_INET, value, in4,
> > -   sizeof(*in4))) != -1) {
> > +   if (prefix_length > 128 || prefix_length == -1)
> 
> prefix_length can't be bigger than 128. But I see why it makes sense to do
> the same here :)
> 
> > +   prefix_length = 128;
> > +   } else if (inet_pton(AF_INET, buf, in4) == 1) {
> > sa->ss_len = sizeof(struct sockaddr_in);
> > sa->ss_family = AF_INET;
> > +   if (prefix_length > 32 || prefix_length == -1)
> > + 

dig(1): replace inet_net_pton(3)

2021-01-19 Thread Florian Obser
When we converted isc_sockaddr_t to sockaddr_storage we also moved to
inet_net_pton(3). It turns out that was a mistake, at least it's not
portable for AF_INET6. Effectively revert that part and hand-roll it
using inet_pton(3).

OK?

p.s. it is kinda telling that isc, who introduced the API is (no
longer?) using it.

diff --git dighost.c dighost.c
index 2d2a52c86e2..2995b7e1602 100644
--- dighost.c
+++ dighost.c
@@ -935,10 +935,16 @@ parse_netprefix(struct sockaddr_storage **sap, int *plen, 
const char *value) {
struct sockaddr_storage *sa = NULL;
struct in_addr *in4;
struct in6_addr *in6;
-   int prefix_length;
+   int prefix_length = -1, i;
+   char *sep;
+   char buf[INET6_ADDRSTRLEN + sizeof("/128")];
+   const char *errstr;
 
REQUIRE(sap != NULL && *sap == NULL);
 
+   if (strlcpy(buf, value, sizeof(buf)) >= sizeof(buf))
+   fatal("invalid prefix '%s'\n", value);
+
sa = calloc(1, sizeof(*sa));
if (sa == NULL)
fatal("out of memory");
@@ -952,14 +958,36 @@ parse_netprefix(struct sockaddr_storage **sap, int *plen, 
const char *value) {
goto done;
}
 
-   if ((prefix_length = inet_net_pton(AF_INET6, value, in6, sizeof(*in6)))
-   != -1) {
+   sep = strchr(buf, '/');
+   if (sep != NULL) {
+   *sep++ = '\0';
+   prefix_length = strtonum(sep, 0, 128, );
+   if (errstr != NULL)
+   fatal("invalid address '%s'", value);
+   }
+
+   if (inet_pton(AF_INET6, buf, in6) == 1) {
sa->ss_len = sizeof(struct sockaddr_in6);
sa->ss_family = AF_INET6;
-   } else if ((prefix_length = inet_net_pton(AF_INET, value, in4,
-   sizeof(*in4))) != -1) {
+   if (prefix_length > 128 || prefix_length == -1)
+   prefix_length = 128;
+   } else if (inet_pton(AF_INET, buf, in4) == 1) {
sa->ss_len = sizeof(struct sockaddr_in);
sa->ss_family = AF_INET;
+   if (prefix_length > 32 || prefix_length == -1)
+   prefix_length = 32;
+   } else if (prefix_length != -1) {
+   if (prefix_length > 32)
+   prefix_length = 32;
+   for (i = 0; i < 3 ; i++) {
+   if (strlcat(buf, ".0", sizeof(buf)) > sizeof(buf))
+   fatal("invalid address '%s'", value);
+   if (inet_pton(AF_INET, buf, in4) == 1) {
+   sa->ss_len = sizeof(struct sockaddr_in);
+   sa->ss_family = AF_INET;
+   goto done;
+   }
+   }
} else
fatal("invalid address '%s'", value);
 


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



Re: -fno-common fixes for slaacd, unwind & rad

2021-01-18 Thread Florian Obser
On Mon, Jan 18, 2021 at 06:19:48PM +0100, Claudio Jeker wrote:
> On Mon, Jan 18, 2021 at 05:31:21PM +0100, Florian Obser wrote:
> > - move ctl_conns to control.c and control_state to frontend.c,
> >   control_state needs to be extern because it's shared between
> >   frontend.c and control.c. (I see that claudio fixed this differently
> >   in ospfd)
> 
> It is pretty similar, the only change is that I also removed control_state
> from the frontend code and just pass the fd to control_listen(). This way
> the full control_state becomes internal.
> 

I think I like yours better, one less extern, but I need to have a
closer look.

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



rad(8): get rid of inet_net_{ntop,pton}(3)

2021-01-18 Thread Florian Obser



This is not an api that seems to have caught on (especially the
AF_INET6 variant), maybe we can get rid of it entirely.

(I also suspect that the AF_INET6 version is broken on FreeBSD and
NetBSD as well as mac osx.)

OK?

diff --git parse.y parse.y
index 8f32f11250d..36e5af8be76 100644
--- parse.y
+++ parse.y
@@ -260,17 +260,28 @@ ra_ifaceoptsl : NO AUTO PREFIX {
| PREFIX STRING {
struct in6_addr  addr;
int  prefixlen;
+   char*p;
+   const char  *errstr;
 
memset(, 0, sizeof(addr));
-   prefixlen = inet_net_pton(AF_INET6, $2, ,
-   sizeof(addr));
-   if (prefixlen == -1) {
-   yyerror("error parsing prefix");
+   p = strchr($2, '/');
+   if (p != NULL) {
+   *p++ = '\0';
+   prefixlen = strtonum(p, 0, 128, );
+   if (errstr != NULL) {
+   yyerror("error parsing prefix "
+   "\"%s/%s\"", $2, p);
+   free($2);
+   YYERROR;
+   }
+   } else
+   prefixlen = 64;
+   if(inet_pton(AF_INET6, $2, ) == 0) {
+   yyerror("error parsing prefix \"%s/%d\"", $2,
+   prefixlen);
free($2);
YYERROR;
}
-   if (prefixlen == 128 && strchr($2, '/') == NULL)
-   prefixlen = 64;
mask_prefix(, prefixlen);
ra_prefix_conf = conf_get_ra_prefix(, prefixlen);
} ra_prefix_block {
diff --git printconf.c printconf.c
index d42890da518..feabd83de94 100644
--- printconf.c
+++ printconf.c
@@ -102,7 +102,7 @@ print_config(struct rad_conf *conf)
 {
struct ra_iface_conf*iface;
struct ra_prefix_conf   *prefix;
-   char buf[INET6_ADDRSTRLEN], *bufp;
+   char buf[INET6_ADDRSTRLEN];
 
print_ra_options("", >ra_options);
printf("\n");
@@ -120,9 +120,9 @@ print_config(struct rad_conf *conf)
printf("\tno auto prefix\n");
 
SIMPLEQ_FOREACH(prefix, >ra_prefix_list, entry) {
-   bufp = inet_net_ntop(AF_INET6, >prefix,
-   prefix->prefixlen, buf, sizeof(buf));
-   printf("\tprefix %s {\n", bufp);
+   printf("\tprefix %s/%d {\n", inet_ntop(AF_INET6,
+   >prefix, buf, sizeof(buf)),
+   prefix->prefixlen);
print_prefix_options("\t\t", prefix);
printf("\t}\n");
}


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



-fno-common fixes for slaacd, unwind & rad

2021-01-18 Thread Florian Obser
This is my take on -fno-common fixes.

slaacd, unwind and rad are based on the same template so the fixes
were similar

- remove global $daemon_process, just use a const string for
  setproctitle
- move ctl_conns to control.c and control_state to frontend.c,
  control_state needs to be extern because it's shared between
  frontend.c and control.c. (I see that claudio fixed this differently
  in ospfd)
- give imsgevs unique names by using proc1_to_proc2 variable names
- other fixes unique per daemon


commit 3f23b6c514f2740f3ad559142935474c8063e60c
Author: Florian Obser 
Date:   Sun Jan 17 19:44:17 2021 +0100

No need for a global slaacd_process; unbreaks -fno-common.
Problem reported by mortimer

diff --git sbin/slaacd/engine.c sbin/slaacd/engine.c
index 52380f64439..3911e6b956a 100644
--- sbin/slaacd/engine.c
+++ sbin/slaacd/engine.c
@@ -369,9 +369,8 @@ engine(int debug, int verbose)
if (chdir("/") == -1)
fatal("chdir(\"/\")");
 
-   slaacd_process = PROC_ENGINE;
-   setproctitle("%s", log_procnames[slaacd_process]);
-   log_procinit(log_procnames[slaacd_process]);
+   setproctitle("%s", "engine");
+   log_procinit("engine");
 
if (setgroups(1, >pw_gid) ||
setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) ||
diff --git sbin/slaacd/frontend.c sbin/slaacd/frontend.c
index ff6539b9ff5..eca8dbea3be 100644
--- sbin/slaacd/frontend.c
+++ sbin/slaacd/frontend.c
@@ -155,9 +155,8 @@ frontend(int debug, int verbose)
if (chdir("/") == -1)
fatal("chdir(\"/\")");
 
-   slaacd_process = PROC_FRONTEND;
-   setproctitle("%s", log_procnames[slaacd_process]);
-   log_procinit(log_procnames[slaacd_process]);
+   setproctitle("%s", "frontend");
+   log_procinit("frontend");
 
if ((ioctlsock = socket(AF_INET6, SOCK_DGRAM | SOCK_CLOEXEC, 0)) == -1)
fatal("socket");
diff --git sbin/slaacd/slaacd.c sbin/slaacd/slaacd.c
index faf4edad31c..7fc9458e7e8 100644
--- sbin/slaacd/slaacd.c
+++ sbin/slaacd/slaacd.c
@@ -54,12 +54,18 @@
 #include "engine.h"
 #include "control.h"
 
+enum slaacd_process {
+   PROC_MAIN,
+   PROC_ENGINE,
+   PROC_FRONTEND
+};
+
 __dead voidusage(void);
 __dead voidmain_shutdown(void);
 
 void   main_sig_handler(int, short, void *);
 
-static pid_t   start_child(int, char *, int, int, int);
+static pid_t   start_child(enum slaacd_process, char *, int, int, int);
 
 void   main_dispatch_frontend(int, short, void *);
 void   main_dispatch_engine(int, short, void *);
@@ -197,9 +203,7 @@ main(int argc, char *argv[])
frontend_pid = start_child(PROC_FRONTEND, saved_argv0,
pipe_main2frontend[1], debug, verbose);
 
-   slaacd_process = PROC_MAIN;
-
-   log_procinit(log_procnames[slaacd_process]);
+   log_procinit("main");
 
if ((routesock = socket(AF_ROUTE, SOCK_RAW | SOCK_CLOEXEC |
SOCK_NONBLOCK, AF_INET6)) == -1)
@@ -319,7 +323,7 @@ main_shutdown(void)
 }
 
 static pid_t
-start_child(int p, char *argv0, int fd, int debug, int verbose)
+start_child(enum slaacd_process p, char *argv0, int fd, int debug, int verbose)
 {
char*argv[7];
int  argc = 0;
diff --git sbin/slaacd/slaacd.h sbin/slaacd/slaacd.h
index 16b47434b0f..7f14f95dda5 100644
--- sbin/slaacd/slaacd.h
+++ sbin/slaacd/slaacd.h
@@ -31,12 +31,6 @@
 
 #defineIMSG_DATA_SIZE(imsg)((imsg).hdr.len - IMSG_HEADER_SIZE)
 
-static const char * const log_procnames[] = {
-   "main",
-   "engine",
-   "frontend"
-};
-
 struct imsgev {
struct imsgbuf   ibuf;
void(*handler)(int, short, void *);
@@ -84,12 +78,6 @@ enum imsg_type {
    IMSG_DUP_ADDRESS,
 };
 
-enum {
-   PROC_MAIN,
-   PROC_ENGINE,
-   PROC_FRONTEND
-} slaacd_process;
-
 enum rpref {
LOW,
MEDIUM,

commit 721a0aea760c19e47df05d9a6a9f1e68e557ca34
Author: Florian Obser 
Date:   Sun Jan 17 19:54:17 2021 +0100

There is only one ctl_conns; unbreaks -fno-common.
Problem reported by mortimer

diff --git sbin/slaacd/control.c sbin/slaacd/control.c
index 48fec3dd71a..ea6c0c27d07 100644
--- sbin/slaacd/control.c
+++ sbin/slaacd/control.c
@@ -41,6 +41,12 @@
 
 #defineCONTROL_BACKLOG 5
 
+struct ctl_conn {
+   TAILQ_ENTRY(ctl_conn)   entry;
+   struct imsgev   iev;
+};
+TAILQ_HEAD(ctl_conns, ctl_conn) ctl_conns;
+
 struct ctl_conn*control_connbyfd(int);
 struct ctl_conn*control_connbypid(pid_t);
 voidcontrol_close(int);
@@ -91,7 +97,7 @@ control_init(char *path)
 int
 control_listen(void)
 {
-
+   TAILQ_INIT(_conns);
if (listen(control_state.fd, CONTROL_BACKLOG) == -1) {
log_warn("%s: listen", _

Re: unwind(8): tcp transport

2021-01-10 Thread Florian Obser
On Fri, Jan 08, 2021 at 06:44:41PM +0100, Florian Obser wrote:
> This is on top of the "unwind(8): respect DO flag" diff I just sent to
> tech.
> 
> This is a bit rough around the edges, but if you feel lucky...
> 
> Currently unwind(8) only accepts queries over udp. We can get away
> with that since we are only listening on localhost and localhost has a
> large mtu, but it's technically not correct and even over localhost we
> can't have answers larger than somewhere around 40 kbytes.

Updated diff, this is now also good to go.

Tests, OKs?

diff --git frontend.c frontend.c
index 0462f37a258..c9b30f618e5 100644
--- frontend.c
+++ frontend.c
@@ -74,6 +74,10 @@
 #define COMPRESSED_RR_SIZE 12
 #define MINIMIZE_ANSWER1
 
+#define FD_RESERVE 5
+#define TCP_TIMEOUT15
+#define DEFAULT_TCP_SIZE   512
+
 struct udp_ev {
struct event ev;
uint8_t  query[65536];
@@ -82,6 +86,11 @@ struct udp_ev {
struct sockaddr_storage  from;
 } udp4ev, udp6ev;
 
+struct tcp_accept_ev {
+   struct event ev;
+   struct event pause;
+} tcp4ev, tcp6ev;
+
 struct pending_query {
TAILQ_ENTRY(pending_query)   entry;
struct sockaddr_storage  from;
@@ -91,8 +100,12 @@ struct pending_query {
struct query_infoqinfo;
struct msg_parse*qmsg;
struct edns_data edns;
+   struct event ev;/* for tcp */
+   struct event resp_ev;   /* for tcp */
+   struct event tmo_ev;/* for tcp */
uint64_t imsg_id;
int  fd;
+   int  tcp;
 };
 
 TAILQ_HEAD(, pending_query) pending_queries;
@@ -108,6 +121,12 @@ voidfrontend_startup(void);
 voidudp_receive(int, short, void *);
 voidhandle_query(struct pending_query *);
 voidfree_pending_query(struct pending_query *);
+voidtcp_accept(int, short, void *);
+int accept_reserve(int, struct sockaddr *, socklen_t *);
+voidaccept_paused(int, short, void *);
+voidtcp_request(int, short, void *);
+voidtcp_response(int, short, void *);
+voidtcp_timeout(int, short, void *);
 int check_query(sldns_buffer*);
 voidnoerror_answer(struct pending_query *);
 voidchaos_answer(struct pending_query *);
@@ -133,6 +152,7 @@ struct imsgev   *iev_main;
 struct imsgev  *iev_resolver;
 struct eventev_route;
 int udp4sock = -1, udp6sock = -1, routesock = -1;
+int tcp4sock = -1, tcp6sock = -1;
 int ta_fd = -1;
 
 static struct trust_anchor_head trust_anchors, new_trust_anchors;
@@ -371,6 +391,30 @@ frontend_dispatch_main(int fd, short event, void *bula)
udp_receive, );
event_add(, NULL);
break;
+   case IMSG_TCP4SOCK:
+   if (tcp4sock != -1)
+   fatalx("%s: received unexpected tcp4sock",
+   __func__);
+   if ((tcp4sock = imsg.fd) == -1)
+   fatalx("%s: expected to receive imsg "
+   "TCP4 fd but didn't receive any", __func__);
+   event_set(, tcp4sock, EV_READ | EV_PERSIST,
+   tcp_accept, );
+   event_add(, NULL);
+   evtimer_set(, accept_paused, );
+   break;
+   case IMSG_TCP6SOCK:
+   if (tcp6sock != -1)
+   fatalx("%s: received unexpected tcp6sock",
+   __func__);
+   if ((tcp6sock = imsg.fd) == -1)
+   fatalx("%s: expected to receive imsg "
+   "TCP6 fd but didn't receive any", __func__);
+   event_set(, tcp6sock, EV_READ | EV_PERSIST,
+   tcp_accept, );
+   event_add(, NULL);
+   evtimer_set(, accept_paused, );
+   break;
case IMSG_ROUTESOCK:
if (routesock != -1)
fatalx("%s: received unexpected routesock",
@@ -577,6 +621,16 @@ free_pending_query(struct pending_query *pq)
regional_destroy(pq->region);
sldns_buffer_free(pq

Re: unwind(8): respect DO flag

2021-01-10 Thread Florian Obser
On Fri, Jan 08, 2021 at 06:37:35PM +0100, Florian Obser wrote:
> Rewrite query parsing and answer formating using libunbound provided
> functions.
> With this we can filter out DNSSEC RRsets if the client did not ask
> for them.
> We will also be able to send truncated answers to indicate to the
> client to switch to tcp (disabled for now since we have no tcp support
> yet).
> 
> Tests, OKs?
> 

I've recut this diff & the tcp diff.

This fixes another memory leak with the usage of reply_info_parse()
and contains some minor code shuffling that came up will workin on
tcp. E.g. free_pending_query now unhooks the query from the tailq so
we must ensure to add the struct as soon as we allocate it
sucessfully.

Tests, OKs?

diff --git frontend.c frontend.c
index 9f91396c4c3..0462f37a258 100644
--- frontend.c
+++ frontend.c
@@ -48,7 +48,11 @@
 #include "libunbound/sldns/sbuffer.h"
 #include "libunbound/sldns/str2wire.h"
 #include "libunbound/sldns/wire2str.h"
+#include "libunbound/util/alloc.h"
+#include "libunbound/util/net_help.h"
+#include "libunbound/util/regional.h"
 #include "libunbound/util/data/dname.h"
+#include "libunbound/util/data/msgencode.h"
 #include "libunbound/util/data/msgparse.h"
 #include "libunbound/util/data/msgreply.h"
 
@@ -68,6 +72,7 @@
  * 2 octets RDLENGTH
  */
 #define COMPRESSED_RR_SIZE 12
+#define MINIMIZE_ANSWER1
 
 struct udp_ev {
struct event ev;
@@ -81,14 +86,13 @@ struct pending_query {
TAILQ_ENTRY(pending_query)   entry;
struct sockaddr_storage  from;
struct sldns_buffer *qbuf;
-   ssize_t  len;
+   struct sldns_buffer *abuf;
+   struct regional *region;
+   struct query_infoqinfo;
+   struct msg_parse*qmsg;
+   struct edns_data edns;
uint64_t imsg_id;
int  fd;
-   int  bogus;
-   int  rcode_override;
-   int  answer_len;
-   int  received;
-   uint8_t *answer;
 };
 
 TAILQ_HEAD(, pending_query) pending_queries;
@@ -102,8 +106,12 @@ __dead void frontend_shutdown(void);
 voidfrontend_sig_handler(int, short, void *);
 voidfrontend_startup(void);
 voidudp_receive(int, short, void *);
+voidhandle_query(struct pending_query *);
+voidfree_pending_query(struct pending_query *);
 int check_query(sldns_buffer*);
+voidnoerror_answer(struct pending_query *);
 voidchaos_answer(struct pending_query *);
+voiderror_answer(struct pending_query *, int rcode);
 voidsend_answer(struct pending_query *);
 voidroute_receive(int, short, void *);
 voidhandle_route_message(struct rt_msghdr *,
@@ -471,37 +479,38 @@ frontend_dispatch_resolver(int fd, short event, void 
*bula)
}
 
if (answer_header->srvfail) {
-   free(pq->answer);
-   pq->answer_len = 0;
-   pq->answer = NULL;
-   pq->rcode_override = LDNS_RCODE_SERVFAIL;
+   error_answer(pq, LDNS_RCODE_SERVFAIL);
send_answer(pq);
break;
}
 
-   if (pq->answer == NULL) {
-   pq->answer = malloc(answer_header->answer_len);
-   if (pq->answer == NULL) {
-   pq->answer_len = 0;
-   pq->rcode_override =
-   LDNS_RCODE_SERVFAIL;
-   send_answer(pq);
-   break;
-   }
-   pq->answer_len = answer_header->answer_len;
-   pq->received = 0;
-   pq->bogus = answer_header->bogus;
+   if (answer_header->bogus && !(pq->qmsg->flags &
+   BIT_CD)) {
+   error_answer(pq, LDNS_RCODE_SERVFAIL);
+   send_answer(pq);
+   break;
+   }
+
+   if (sldns_buffer_position(pq->

Re: unwind(8): respect DO flag

2021-01-09 Thread Florian Obser
On Fri, Jan 08, 2021 at 06:37:35PM +0100, Florian Obser wrote:
> Rewrite query parsing and answer formating using libunbound provided
> functions.
> With this we can filter out DNSSEC RRsets if the client did not ask
> for them.
> We will also be able to send truncated answers to indicate to the
> client to switch to tcp (disabled for now since we have no tcp support
> yet).
> 
> Tests, OKs?
> 

It's better to leak less memory...

diff --git frontend.c frontend.c
index 9f91396c4c3..60dafd81a79 100644
--- frontend.c
+++ frontend.c
@@ -48,7 +48,11 @@
 #include "libunbound/sldns/sbuffer.h"
 #include "libunbound/sldns/str2wire.h"
 #include "libunbound/sldns/wire2str.h"
+#include "libunbound/util/alloc.h"
+#include "libunbound/util/net_help.h"
+#include "libunbound/util/regional.h"
 #include "libunbound/util/data/dname.h"
+#include "libunbound/util/data/msgencode.h"
 #include "libunbound/util/data/msgparse.h"
 #include "libunbound/util/data/msgreply.h"
 
@@ -81,14 +85,13 @@ struct pending_query {
TAILQ_ENTRY(pending_query)   entry;
struct sockaddr_storage  from;
struct sldns_buffer *qbuf;
-   ssize_t  len;
+   struct sldns_buffer *abuf;
+   struct regional *region;
+   struct query_infoqinfo;
+   struct msg_parse*qmsg;
+   struct edns_data edns;
uint64_t imsg_id;
int  fd;
-   int  bogus;
-   int  rcode_override;
-   int  answer_len;
-   int  received;
-   uint8_t *answer;
 };
 
 TAILQ_HEAD(, pending_query) pending_queries;
@@ -102,8 +105,12 @@ __dead void frontend_shutdown(void);
 voidfrontend_sig_handler(int, short, void *);
 voidfrontend_startup(void);
 voidudp_receive(int, short, void *);
+voidhandle_query(struct pending_query *);
+voidfree_pending_query(struct pending_query *);
 int check_query(sldns_buffer*);
+voidnoerror_answer(struct pending_query *);
 voidchaos_answer(struct pending_query *);
+voiderror_answer(struct pending_query *, int rcode);
 voidsend_answer(struct pending_query *);
 voidroute_receive(int, short, void *);
 voidhandle_route_message(struct rt_msghdr *,
@@ -471,37 +478,38 @@ frontend_dispatch_resolver(int fd, short event, void 
*bula)
}
 
if (answer_header->srvfail) {
-   free(pq->answer);
-   pq->answer_len = 0;
-   pq->answer = NULL;
-   pq->rcode_override = LDNS_RCODE_SERVFAIL;
+   error_answer(pq, LDNS_RCODE_SERVFAIL);
send_answer(pq);
break;
}
 
-   if (pq->answer == NULL) {
-   pq->answer = malloc(answer_header->answer_len);
-   if (pq->answer == NULL) {
-   pq->answer_len = 0;
-   pq->rcode_override =
-   LDNS_RCODE_SERVFAIL;
-   send_answer(pq);
-   break;
-   }
-   pq->answer_len = answer_header->answer_len;
-   pq->received = 0;
-   pq->bogus = answer_header->bogus;
+   if (answer_header->bogus && !(pq->qmsg->flags &
+   BIT_CD)) {
+   error_answer(pq, LDNS_RCODE_SERVFAIL);
+   send_answer(pq);
+   break;
+   }
+
+   if (sldns_buffer_position(pq->abuf) == 0 &&
+   !sldns_buffer_set_capacity(pq->abuf,
+   answer_header->answer_len)) {
+   error_answer(pq, LDNS_RCODE_SERVFAIL);
+   send_answer(pq);
+   break;
}
 
-   if (pq->received + data_len > pq->answer_len)
+   if (sldns_buffer

Re: tpm(4): don't use tvtohz(9)

2021-01-08 Thread Florian Obser
On Fri, Jan 08, 2021 at 11:48:33AM -0600, Scott Cheloha wrote:
> On Fri, Jan 08, 2021 at 05:41:24PM +0100, Mark Kettenis wrote:
> > > Date: Fri, 8 Jan 2021 10:27:38 -0600
> > > From: Scott Cheloha 
> > > 
> > > On Wed, Jan 06, 2021 at 11:26:27PM +0100, Mark Kettenis wrote:
> > > > > Date: Wed, 6 Jan 2021 16:16:27 -0600
> > > > > From: Scott Cheloha 
> > > > > 
> > > > > On Wed, Jan 06, 2021 at 12:16:13PM -0600, Scott Cheloha wrote:
> > > > > > As mentioned in a prior mail, tpm(4) is the last user of tvtohz(9) 
> > > > > > in
> > > > > > the tree.
> > > > > > 
> > > > > > However, we don't need to use tvtohz(9) in tpm(4) at all.  
> > > > > > Converting
> > > > > > from milliseconds to ticks is trivial.  Using an intermediary 
> > > > > > timeval
> > > > > > is just pointless indirection.
> > > > > > 
> > > > > > With this committed I will be able to remove both tvtohz(9) and
> > > > > > tstohz(9) from the tree in a subsequent patch.
> > > > > 
> > > > > Whoops, made a mistake in the prior diff.
> > > > > 
> > > > > We want to MAX() the result up to 1 to avoid dividing to zero and
> > > > > blocking indefinitely in e.g. tsleep(9).  Previous diff MIN'd the
> > > > > result down to 1, which is very wrong.
> > > > 
> > > > To be honest I'd just zap the function completely and instead simply do:
> > > > 
> > > > to = TPM_ACCESS_TMO / 10;
> > > > 
> > > > and
> > > > 
> > > > to = TPM_BURST_TMO / 10;
> > > 
> > > That won't work on custom kernels where HZ is not 100, no?
> > 
> > HZ is irrelevant here.  These TPM_XXX_TMO defines specify a timeout in
> > microseconds and DELAY(10) just delays for 10us.  So you just need to
> > figure out how many times you need to do the 10us delay to reach the
> > timeout specified by TPM_XXX_TMO.
> 
> Whoops, yes, you're right.
> 
> > Also, this driver is only supported on amd64/i386 at this point (but
> > might show up on arm64 at some point).
> > 
> > > > There is no magic happening here.  The code is just doing a hardware
> > > > register poll loop in 10us steps.
> > > > 
> > > > Hmm, actually it seems the code is broken and uses steps of 10
> > > > microseconds instead of milliseconds.  So instead it should probably
> > > > use:
> > > > 
> > > > to = TPM_ACCESS_TMO * 100;
> > > > 
> > > > and
> > > > 
> > > > to = TPM_BURST_TMO * 100;
> > > 
> > > This problem came up in a different thread:
> > > 
> > > https://marc.info/?l=openbsd-tech=160833962329381=2
> > > 
> > > jcs@ said, and I paraphrase, "tpm(4) sucks, we should merge NetBSD's
> > > latest code, which is much nicer now."
> > > 
> > > That sounds like a much taller order than I can fill, so I'm just
> > > trying to remove the tvtohz(9) call without changing any behavior,
> > > even if the behavior looks wrong (like using the wrong units).
> > > 
> > > I don't even have a tpm(4) device to test.  Until I have one I'm
> > > reluctant to do things like expanding the delay time in these loops.
> > > As of now the driver "works" for some subset of people, which is not
> > > nothing.
> > 
> > But having illogical code is a problem as well.  So I think we should
> > at least attempt to fix it the right way.  All it takes is to find
> > someone with a laptop where the driver attaches and have them
> > suspend/resume it.
> 
> We can try that, sure.  Here's the patch:
> 
> - Remove tpm_tmotohz().
> 
> - tpm_waitfor() does DELAY(1), so in that case convert from
>   milliseconds to microseconds.  Note in the function argument
>   that we expect milliseconds, not "tries".
> 
> - In the other cases we do DELAY(10), so we only multiply by 100.
>   Leave a comment explaining what we're doing.
> 
> Unsure who can test.  We need a suspend/resume test with this patch.
> Probably an older machine.  Newer machines tend to have TPM 2.0 chips.

My x1 gen 2 seems to have a TPM 1.2 chip. Or can emulate one?
The bios is confusing. I had it disabled until now.
It seems to be able to suspend/resume just fine with this.

It shows up like this:

tpm0 at acpi0 TPM_ addr 0xfed4/0x5000, device 0x104a rev 0x4e

No idea what this does, I'm going to disable it again, but let me know
if you want further tests.

OpenBSD 6.8-current (GENERIC.MP) #103: Fri Jan  8 20:11:51 CET 2021
flor...@x1.home.narrans.de:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 8233394176 (7851MB)
avail mem = 7968542720 (7599MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xdcd3d000 (61 entries)
bios0: vendor LENOVO version "GRET40WW (1.17 )" date 09/02/2014
bios0: LENOVO 20A7006VUS
acpi0 at bios0: ACPI 5.0
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP SLIC DBGP ECDT HPET APIC MCFG SSDT SSDT SSDT SSDT SSDT 
SSDT SSDT PCCT SSDT TCPA UEFI MSDM ASF! BATB FPDT UEFI DMAR
acpi0: wakeup devices LID_(S4) SLPB(S3) IGBE(S4) EXP2(S4) XHCI(S3) EHC1(S3)
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpiec0 at acpi0
acpihpet0 at acpi0: 14318179 Hz
acpimadt0 at acpi0 

unwind(8): tcp transport

2021-01-08 Thread Florian Obser
This is on top of the "unwind(8): respect DO flag" diff I just sent to
tech.

This is a bit rough around the edges, but if you feel lucky...

Currently unwind(8) only accepts queries over udp. We can get away
with that since we are only listening on localhost and localhost has a
large mtu, but it's technically not correct and even over localhost we
can't have answers larger than somewhere around 40 kbytes.

The accept_reserve stuff is a bit weird since we don't need additional
FDs to service a request, at least not in the frontend process. I
haven't made up my mind yet if i should simplify it or keep it
diffable to slowcgi.

I'm also not fond of the memmove stuff I need to do to the
sldns_buffers. I'll probably try to do without.
I think that will come down to
sldns_buffer_flip(...);
if (tcp)
sldns_buffer_skip(..., 2);
maybe...

Anyway, tests are welcome.

diff --git frontend.c frontend.c
index 8cb73dffa57..f87b0d1646d 100644
--- frontend.c
+++ frontend.c
@@ -73,6 +73,11 @@
  */
 #define COMPRESSED_RR_SIZE 12
 
+#define FD_RESERVE 5
+#define FD_NEEDED  0   /* no additional FDs needed */
+#define TCP_TIMEOUT15
+#define DEFAULT_TCP_SIZE   512
+
 struct udp_ev {
struct event ev;
uint8_t  query[65536];
@@ -81,6 +86,11 @@ struct udp_ev {
struct sockaddr_storage  from;
 } udp4ev, udp6ev;
 
+struct tcp_accept_ev {
+   struct event ev;
+   struct event pause;
+} tcp4ev, tcp6ev;
+
 struct pending_query {
TAILQ_ENTRY(pending_query)   entry;
struct sockaddr_storage  from;
@@ -90,8 +100,13 @@ struct pending_query {
struct query_infoqinfo;
struct msg_parse*qmsg;
struct edns_data edns;
+   struct event ev;/* for tcp */
+   struct event resp_ev;   /* for tcp */
+   struct event tmo_ev;/* for tcp */
uint64_t imsg_id;
int  fd;
+   int  tcp;
+   int  tcp_len;
 };
 
 TAILQ_HEAD(, pending_query) pending_queries;
@@ -107,6 +122,13 @@ voidfrontend_startup(void);
 voidudp_receive(int, short, void *);
 voidhandle_query(struct pending_query *);
 voidfree_pending_query(struct pending_query *);
+voidtcp_accept(int, short, void *);
+int accept_reserve(int, struct sockaddr *, socklen_t *,
+int, volatile int *);
+voidaccept_paused(int, short, void *);
+voidtcp_request(int, short, void *);
+voidtcp_response(int, short, void *);
+voidtcp_timeout(int, short, void *);
 int check_query(sldns_buffer*);
 voidnoerror_answer(struct pending_query *);
 voidchaos_answer(struct pending_query *);
@@ -132,7 +154,9 @@ struct imsgev   *iev_main;
 struct imsgev  *iev_resolver;
 struct eventev_route;
 int udp4sock = -1, udp6sock = -1, routesock = -1;
+int tcp4sock = -1, tcp6sock = -1;
 int ta_fd = -1;
+int tcp_inflight = 0;
 
 static struct trust_anchor_head trust_anchors, new_trust_anchors;
 
@@ -370,6 +394,30 @@ frontend_dispatch_main(int fd, short event, void *bula)
udp_receive, );
event_add(, NULL);
break;
+   case IMSG_TCP4SOCK:
+   if (tcp4sock != -1)
+   fatalx("%s: received unexpected tcp4sock",
+   __func__);
+   if ((tcp4sock = imsg.fd) == -1)
+   fatalx("%s: expected to receive imsg "
+   "TCP4 fd but didn't receive any", __func__);
+   event_set(, tcp4sock, EV_READ | EV_PERSIST,
+   tcp_accept, );
+   event_add(, NULL);
+   evtimer_set(, accept_paused, );
+   break;
+   case IMSG_TCP6SOCK:
+   if (tcp6sock != -1)
+   fatalx("%s: received unexpected tcp6sock",
+   __func__);
+   if ((tcp6sock = imsg.fd) == -1)
+   fatalx("%s: expected to receive imsg "
+   "TCP6 fd but didn't receive any", __func__);
+   event_set(, tcp6sock, EV_READ | EV_PERSIST,
+   tcp_accept, );
+   event_add(, NULL);
+  

unwind(8): respect DO flag

2021-01-08 Thread Florian Obser
Rewrite query parsing and answer formating using libunbound provided
functions.
With this we can filter out DNSSEC RRsets if the client did not ask
for them.
We will also be able to send truncated answers to indicate to the
client to switch to tcp (disabled for now since we have no tcp support
yet).

Tests, OKs?

diff --git frontend.c frontend.c
index 9f91396c4c3..8cb73dffa57 100644
--- frontend.c
+++ frontend.c
@@ -48,7 +48,11 @@
 #include "libunbound/sldns/sbuffer.h"
 #include "libunbound/sldns/str2wire.h"
 #include "libunbound/sldns/wire2str.h"
+#include "libunbound/util/alloc.h"
+#include "libunbound/util/net_help.h"
+#include "libunbound/util/regional.h"
 #include "libunbound/util/data/dname.h"
+#include "libunbound/util/data/msgencode.h"
 #include "libunbound/util/data/msgparse.h"
 #include "libunbound/util/data/msgreply.h"
 
@@ -81,14 +85,13 @@ struct pending_query {
TAILQ_ENTRY(pending_query)   entry;
struct sockaddr_storage  from;
struct sldns_buffer *qbuf;
-   ssize_t  len;
+   struct sldns_buffer *abuf;
+   struct regional *region;
+   struct query_infoqinfo;
+   struct msg_parse*qmsg;
+   struct edns_data edns;
uint64_t imsg_id;
int  fd;
-   int  bogus;
-   int  rcode_override;
-   int  answer_len;
-   int  received;
-   uint8_t *answer;
 };
 
 TAILQ_HEAD(, pending_query) pending_queries;
@@ -102,8 +105,12 @@ __dead void frontend_shutdown(void);
 voidfrontend_sig_handler(int, short, void *);
 voidfrontend_startup(void);
 voidudp_receive(int, short, void *);
+voidhandle_query(struct pending_query *);
+voidfree_pending_query(struct pending_query *);
 int check_query(sldns_buffer*);
+voidnoerror_answer(struct pending_query *);
 voidchaos_answer(struct pending_query *);
+voiderror_answer(struct pending_query *, int rcode);
 voidsend_answer(struct pending_query *);
 voidroute_receive(int, short, void *);
 voidhandle_route_message(struct rt_msghdr *,
@@ -471,37 +478,38 @@ frontend_dispatch_resolver(int fd, short event, void 
*bula)
}
 
if (answer_header->srvfail) {
-   free(pq->answer);
-   pq->answer_len = 0;
-   pq->answer = NULL;
-   pq->rcode_override = LDNS_RCODE_SERVFAIL;
+   error_answer(pq, LDNS_RCODE_SERVFAIL);
send_answer(pq);
break;
}
 
-   if (pq->answer == NULL) {
-   pq->answer = malloc(answer_header->answer_len);
-   if (pq->answer == NULL) {
-   pq->answer_len = 0;
-   pq->rcode_override =
-   LDNS_RCODE_SERVFAIL;
-   send_answer(pq);
-   break;
-   }
-   pq->answer_len = answer_header->answer_len;
-   pq->received = 0;
-   pq->bogus = answer_header->bogus;
+   if (answer_header->bogus && !(pq->qmsg->flags &
+   BIT_CD)) {
+   error_answer(pq, LDNS_RCODE_SERVFAIL);
+   send_answer(pq);
+   break;
+   }
+
+   if (sldns_buffer_position(pq->abuf) == 0 &&
+   !sldns_buffer_set_capacity(pq->abuf,
+   answer_header->answer_len)) {
+   error_answer(pq, LDNS_RCODE_SERVFAIL);
+   send_answer(pq);
+   break;
}
 
-   if (pq->received + data_len > pq->answer_len)
+   if (sldns_buffer_position(pq->abuf) + data_len >
+   sldns_buffer_capacity(pq->abuf))
fatalx("%s: IMSG_ANSWER answer too big: %d",
__func__, data_len);
+   sldns_buffer_write(pq->abuf, data, data_len);
 
-   memcpy(pq->answer + pq->received, data, data_len);
- 

Re: unbound: missing null check

2021-01-06 Thread Florian Obser
On Wed, Jan 06, 2021 at 10:11:01AM +0100, Anton Lindqvist wrote:
> Hi,
> I have a unbound forward zone configured on my router for my $DAYJOB.
> The address associated with the zone is only accessible when the router
> is connected to a VPN. If the VPN connection is absent, trying to
> resolve any domain that must be handled by the zone crashes unbound.
> Turns out there's a missing NULL check in comm_point_send_udp_msg().
> The same routine already has `if (addr) {} else {}' branches so I guess
> protecting the call to log_addr() using the same conditional is
> reasonable.
> 
> Should this instead be upstreamed? Comments? OK?

yes, please upstream it.

Could you also do sbin/unwind/libunbound/util/netevent.c?

OK florian

> 
> Index: util/netevent.c
> ===
> RCS file: /cvs/src/usr.sbin/unbound/util/netevent.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 netevent.c
> --- util/netevent.c   10 Dec 2020 21:44:58 -  1.27
> +++ util/netevent.c   6 Jan 2021 09:03:59 -
> @@ -379,8 +379,9 @@ comm_point_send_udp_msg(struct comm_poin
>   if(!udp_send_errno_needs_log(addr, addrlen))
>   return 0;
>   verbose(VERB_OPS, "sendto failed: %s", sock_strerror(errno));
> - log_addr(VERB_OPS, "remote address is", 
> - (struct sockaddr_storage*)addr, addrlen);
> + if(addr)
> + log_addr(VERB_OPS, "remote address is",
> + (struct sockaddr_storage*)addr, addrlen);
>   return 0;
>   } else if((size_t)sent != sldns_buffer_remaining(packet)) {
>   log_err("sent %d in place of %d bytes", 
> 

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



Re: use getnameinfo in bgpd to print addresses

2021-01-04 Thread Florian Obser
OK

On Mon, Jan 04, 2021 at 10:46:39AM +0100, Claudio Jeker wrote:
> In bgpd most prefixes and addresses are stored as struct bgpd_addr. When
> it is printed it uses inet_ntop() which is not ideal since it does not
> handle IPv6 scoped_id. Instead convert to a struct sockaddr and use
> log_sockaddr() which in turn uses getnameinfo.
> 
> Ideally the same should be done for the VPN address types but that is a
> bit more complex.
> 
> -- 
> :wq Claudio
> 
> Index: bgpd.h
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.408
> diff -u -p -r1.408 bgpd.h
> --- bgpd.h30 Dec 2020 07:29:56 -  1.408
> +++ bgpd.h4 Jan 2021 09:34:44 -
> @@ -1371,7 +1371,7 @@ int  aid2afi(u_int8_t, u_int16_t *, u_i
>  int   afi2aid(u_int16_t, u_int8_t, u_int8_t *);
>  sa_family_t   aid2af(u_int8_t);
>  int   af2aid(sa_family_t, u_int8_t, u_int8_t *);
> -struct sockaddr  *addr2sa(struct bgpd_addr *, u_int16_t, socklen_t *);
> +struct sockaddr  *addr2sa(const struct bgpd_addr *, u_int16_t, socklen_t 
> *);
>  void  sa2addr(struct sockaddr *, struct bgpd_addr *, u_int16_t *);
>  const char *  get_baudrate(unsigned long long, char *);
>  
> Index: util.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/util.c,v
> retrieving revision 1.55
> diff -u -p -r1.55 util.c
> --- util.c21 Oct 2020 06:53:54 -  1.55
> +++ util.c4 Jan 2021 09:33:30 -
> @@ -39,14 +39,12 @@ log_addr(const struct bgpd_addr *addr)
>  {
>   static char buf[74];
>   chartbuf[40];
> + socklen_t   len;
>  
>   switch (addr->aid) {
>   case AID_INET:
>   case AID_INET6:
> - if (inet_ntop(aid2af(addr->aid), >ba, buf,
> - sizeof(buf)) == NULL)
> - return ("?");
> - return (buf);
> + return log_sockaddr(addr2sa(addr, 0, ), len);
>   case AID_VPN_IPv4:
>   if (inet_ntop(AF_INET, >vpn4.addr, tbuf,
>   sizeof(tbuf)) == NULL)
> @@ -838,7 +836,7 @@ af2aid(sa_family_t af, u_int8_t safi, u_
>  }
>  
>  struct sockaddr *
> -addr2sa(struct bgpd_addr *addr, u_int16_t port, socklen_t *len)
> +addr2sa(const struct bgpd_addr *addr, u_int16_t port, socklen_t *len)
>  {
>   static struct sockaddr_storage   ss;
>   struct sockaddr_in  *sa_in = (struct sockaddr_in *)
> 

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



acme-client(1): backup certs

2021-01-02 Thread Florian Obser


Create .1 backup files when acme-client is going to overwrite a
certificate file.

This files are not terribly big and it's convenient to keep one
previous file around for example if one adds or removes domains to the
certificate and then wants to revoke the previous one.

(Note that it's kinda difficult to revoke the old certificate with
acme-client currently. The whole revoke machinery needs to be
overhauled. I have ideas...)

Comments, OKs?

diff --git acme-client.conf.5 acme-client.conf.5
index 3c5fd1c2362..3fdd40a5eb0 100644
--- acme-client.conf.5
+++ acme-client.conf.5
@@ -149,6 +149,11 @@ The filename of the certificate that will be issued.
 This is optional if
 .Ar domain full chain certificate
 is specified.
+A backup with name
+.Ar file.1
+is created if
+.Ar file
+exists.
 .It Ic domain chain certificate Ar file
 The filename in which to store the certificate chain
 that will be returned by the certificate authority.
@@ -156,6 +161,11 @@ It needs to be in the same directory as the
 .Ar domain certificate
 (or in a subdirectory) and can be specified as a relative or absolute path.
 This setting is optional.
+A backup with name
+.Ar file.1
+is created if
+.Ar file
+exists.
 .It Ic domain full chain certificate Ar file
 The filename in which to store the full certificate chain
 that will be returned by the certificate authority.
@@ -170,6 +180,11 @@ in one file, and is required by most browsers.
 This is optional if
 .Ar domain certificate
 is specified.
+A backup with name
+.Ar file.1
+is created if
+.Ar file
+exists.
 .It Ic sign with Ar authority
 The certificate authority (as declared above in the
 .Sx AUTHORITIES
diff --git fileproc.c fileproc.c
index b7cdff5525d..cc3aa293712 100644
--- fileproc.c
+++ fileproc.c
@@ -34,6 +34,19 @@ serialise(const char *real, const char *v, size_t vsz, const 
char *v2, size_t v2
int   fd;
char *tmp;
 
+   /* create backup hardlink */
+   if (asprintf(, "%s.1", real) == -1) {
+   warn("asprintf");
+   return 0;
+   }
+   (void) unlink(tmp);
+   if (link(real, tmp) == -1 && errno != ENOENT) {
+   warn("link");
+   free(tmp);
+   return 0;
+   }
+   free(tmp);
+
/*
 * Write into backup location, overwriting.
 * Then atomically do the rename.

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



Re: fix examples/acme-client.conf

2020-12-30 Thread Florian Obser



On 31 December 2020 00:42:26 CET, Chris Bennett 
 wrote:
>Hi,
>after spending several hours trying to find out what the problem was
>with getting SSL to work properly again in Apache, I finally found the
>problem.
>
>The -current and src versions are the same in /etc/examples, but
>acme-client has changed. I looked in both places to see if I missed a
>change. Turns out the man page mentioned it, but nothing in example.
>
>
>Not sure if I picked good names, but I would really like to get this
>little addition. RTFM still applies, but if there is an examples file,
>it ought to represent new changes, IMHO.

I'm not following, what new changes? Getting the chain certificate(s) out of 
acme-client has been there since the beginning.
Also I suspect if you need the chains separate from the domain cert you want 
the single domain cert and not the full chain domain cert.

In any case the point of this and the httpd example file is that you can put 
them into place, do s/example.com/your.domain/ and be up and running.

>
>Chris Bennett
>
>Index: acme-client.conf
>===
>RCS file: /cvs/src/etc/examples/acme-client.conf,v
>retrieving revision 1.4
>diff -u -p -u -p -r1.4 acme-client.conf
>--- acme-client.conf   17 Sep 2020 09:13:06 -  1.4
>+++ acme-client.conf   30 Dec 2020 23:35:03 -
>@@ -26,6 +26,7 @@ authority buypass-test {
> domain example.com {
>   alternative names { secure.example.com }
>   domain key "/etc/ssl/private/example.com.key"
>+  domain chain certificate "/etc/ssl/example.com.chain.pem"
>   domain full chain certificate "/etc/ssl/example.com.fullchain.pem"
>   sign with letsencrypt
> }

-- 
Sent from a mobile device. Please excuse poor formating.



ping(8): flood ping and counts

2020-12-27 Thread Florian Obser
I tried to fix this before in 2018.
Claudio had pointed out that a flood ping with a count given would
wait forever on a lossy link so we made sure a timeout was set.

However we would also exit after the first answer received if we had
already send out all flood pings.

Flooding something in farawayistan we are able to send out the all 10
pings before the first answer comes back:

$ time doas ping -f -c10 target.host
PING target.host (xxx.xxx.xxx.xxx): 56 data bytes
..
--- target.host ping statistics ---
10 packets transmitted, 1 packets received, 90.0% packet loss
round-trip min/avg/max/std-dev = 241.168/241.168/241.168/0.000 ms
0m00.26s real 0m00.00s user 0m00.00s system

The 90% loss is just wrong, all those packets come in immediately
after the first one, we just don't read them.

$ time doas obj/ping -f -c10 target.host
PING target.host (xxx.xxx.xxx.xxx): 56 data bytes
..
--- target.host ping statistics ---
10 packets transmitted, 10 packets received, 0.0% packet loss
round-trip min/avg/max/std-dev = 238.791/241.277/250.184/3.469 ms
0m00.42s real 0m00.00s user 0m00.00s system


OK?

diff --git ping.c ping.c
index 2af4ded0b6b..0b69d754ab7 100644
--- ping.c
+++ ping.c
@@ -251,7 +251,7 @@ main(int argc, char *argv[])
struct passwd *pw;
socklen_t maxsizelen;
int64_t preload;
-   int ch, i, optval = 1, packlen, maxsize, error, s;
+   int ch, i, optval = 1, packlen, maxsize, error, s, flooddone = 0;
int df = 0, tos = 0, bufspace = IP_MAXPACKET, hoplimit = -1, mflag = 0;
u_char *datap, *packet;
u_char ttl = MAXTTL;
@@ -870,6 +870,8 @@ main(int argc, char *argv[])
if (seenint)
break;
if (seenalrm) {
+   if (flooddone)
+   break;
retransmit(s);
seenalrm = 0;
if (ntransmitted - nreceived - 1 > nmissedmax) {
@@ -886,7 +888,7 @@ main(int argc, char *argv[])
continue;
}
 
-   if (options & F_FLOOD) {
+   if ((options & F_FLOOD && !flooddone)) {
if (pinger(s) != 0) {
(void)signal(SIGALRM, onsignal);
timeout = INFTIM;
@@ -901,7 +903,7 @@ main(int argc, char *argv[])
(void)setitimer(ITIMER_REAL, , NULL);
 
/* When the alarm goes off we are done. */
-   seenint = 1;
+   flooddone = 1;
} else
timeout = 10;
} else


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



acme-client(1): dns-01

2020-12-24 Thread Florian Obser
'tis the season to be jolly...

I think it's time to kick the tires on this one.

I don't like the "exec" keyword, we should find something better.
Also, should the user be optional?
Oh, and it's not enforcing that exec is present in the config.

sthen pointed me in the direction of dehydrated
https://github.com/dehydrated-io/dehydrated/blob/master/docs/dns-verification.md
and uacme
https://github.com/ndilieto/uacme
as examples for acme clients that implement hooks for (dns) challenges

I implemented the uacme api since I find that less ugly. It should be
trivial to transmogrify it with a shell one-liner to support
dehydrated.

Comments, tests?

diff --git acme-client.conf.5 acme-client.conf.5
index 3c5fd1c2362..e580a365c51 100644
--- acme-client.conf.5
+++ acme-client.conf.5
@@ -170,11 +170,55 @@ in one file, and is required by most browsers.
 This is optional if
 .Ar domain certificate
 is specified.
-.It Ic sign with Ar authority
+.It Ic sign with Ar authority Op Ic challenge Ar type
 The certificate authority (as declared above in the
 .Sx AUTHORITIES
 section) to use.
 If this setting is absent, the first authority specified is used.
+.Ar type
+can be
+.Cm http
+or
+.Cm dns .
+It defaults to
+.Cm http .
+.It Ic exec Ar script Ic as Ar user
+Run
+.Ar script
+as user
+.Ar user
+for each
+.Cm dns
+challenge.
+This is required when using the
+.Cm dns
+challenge type.
+The script is called with five arguments:
+.Bl -tag -width Ds
+.It Ar method
+.Cm begin ,
+.Cm done ,
+or
+.Cm failed .
+.Cm begin
+indicates that a DNS record should be created and
+.Cm done
+or
+.Cm failed
+indicate that a DNS record should be removed.
+.It Ar type
+.Cm dns-01 .
+.It Ar ident
+The domain.
+.It Ar token
+Unused, for compatibility with existing hook scripts.
+.It Ar auth
+The challenge response.
+.El
+.Pp
+The script needs to create a DNS record of the form
+.Dl _acme-challenge.ident 30 IN TXT auth
+and exit once it has propagated to all name servers.
 .It Ic challengedir Ar path
 The directory in which the challenge file will be stored.
 If it is not specified, a default of
diff --git chngproc.c chngproc.c
index 476daed3416..deef61fccc6 100644
--- chngproc.c
+++ chngproc.c
@@ -15,10 +15,13 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include 
+
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -26,8 +29,38 @@
 
 #include "extern.h"
 
-int
-chngproc(int netsock, const char *root)
+static int
+do_exec(const char *exec, const char *method, const char *type,
+const char *ident, const char *token, const char *auth)
+{
+   pid_tpid;
+   int  status;
+
+   switch (pid = fork()) {
+   case -1:
+   warn("fork");
+   return -1;
+   case 0: /* child */
+   /* XXX close netproc fd */
+   if (pledge("stdio rpath exec", NULL) == -1) {
+   warn("pledge");
+   return -1;
+   }
+   execl(exec, exec, method, type, ident, token, auth, NULL);
+   warn("execl %s", exec);
+   _exit(1);
+   }
+   if (waitpid(pid, , 0) == -1) {
+   warn("waitpid");
+   return -1;
+   }
+   if (WEXITSTATUS(status) == 0)
+   return 0;
+   return -1;
+}
+
+static int
+chngproc_http(int netsock, const char *root)
 {
char *tok = NULL, *th = NULL, *fmt = NULL, **fs = NULL;
size_ti, fsz = 0;
@@ -153,3 +186,114 @@ out:
free(tok);
return rc;
 }
+
+static int
+chngproc_dns(int netsock, const char* exec, const char *exec_as)
+{
+   struct passwd   *pw;
+   size_t   i, identsz = 0;
+   long lval;
+   int  rc = 0, cc;
+   enum chngop  op;
+   char*ident = NULL, *tok = NULL, *auth = NULL;
+   char**idents = NULL;
+   void*pp;
+
+   if ((pw = getpwnam(exec_as)) == NULL) {
+   warn("getpwnam");
+   goto out;
+   }
+   if (setgroups(1, >pw_gid) ||
+   setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) ||
+   setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid)) {
+   warnx("can't drop privileges");
+   goto out;
+   }
+
+   if (unveil(exec, "x") == -1) {
+   warn("unveil");
+   goto out;
+   }
+
+   if (pledge("stdio rpath proc exec", NULL) == -1) {
+   warn("pledge");
+   goto out;
+   }
+
+   for (;;) {
+   op = CHNG__MAX;
+   if ((lval = readop(netsock, COMM_CHNG_OP)) == 0)
+   op = CHNG_STOP;
+   else if (lval == CHNG_SYN)
+   op = lval;
+
+   if (op == CHNG__MAX) {
+   warnx("unknown operation from netproc");
+   goto out;
+   } else if 

Re: acme-client(1): fulfil all challenges, then tell the the CA

2020-12-24 Thread Florian Obser
On Wed, Dec 23, 2020 at 09:58:41PM +, Stuart Henderson wrote:
> On 2020/12/23 18:09, Florian Obser wrote:
> > First fulfil all challenges then tell the CA that it should check.
> > 
> > With a CSR with multiple SANs acme-client would write one challenge,
> > tell the CA, write the next challenge and so on.
> > 
> > For http-01 this doesn't matter but I think this will be nicer for dns-01
> > because there are propagation delays to consider.
> > 
> > Please be extra careful checking this. If I mess this up people might
> > run into renewal issues months from now. And when that happens people
> > tend to comment... (Which I also pull this out of the big diff I'm
> > currently working on for dns-01.)
> > 
> > OK?
> 
> I tested by forcibly renewing some multi-name certificates. I saw that
> letsencrypt didn't bother re-challenging because they already had a
> recent auth so I moved them to buypass, all looks good. (FWIW I did

Yes, it's a bit annoying that you can't force a revalidation.

Say you fiddle around with your webserver config and want to test if
acme-client still works. Chances are that you won't notice if there is
a problem because let's encrypt just hands you a renewed cert.

Another way to force a revalidation is deleting the account key...

> some ecdsa as well as rsa, not that it matters for this test).

Thanks, one thing I'm worried about is some challenges being valid and
others are pending. (I.e. is the "if() continue" correct, pretty sure
it is since I just pull it down...) But I have no idea how to test
that.

> 
> Reads good and works for me, OK.
> 
> 
> > diff --git netproc.c netproc.c
> > index 38732a4dd01..7c502643acc 100644
> > --- netproc.c
> > +++ netproc.c
> > @@ -840,7 +840,12 @@ netproc(int kfd, int afd, int Cfd, int cfd, int dfd, 
> > int rfd,
> > if (readop(Cfd, COMM_CHNG_ACK) != CHNG_ACK)
> > goto out;
> >  
> > -   /* Write to the CA that it's ready. */
> > +   }
> > +   /* Write to the CA that it's ready. */
> > +   for (i = 0; i < order.authsz; i++) {
> > +   if (chngs[i].status == CHNG_VALID ||
> > +   chngs[i].status == CHNG_INVALID)
> > +   continue;
> > if (!dochngresp(, [i]))
> > goto out;
> > }
> > 
> > 
> > -- 
> > I'm not entirely sure you are real.
> > 
> 

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



acme-client(1): fulfil all challenges, then tell the the CA

2020-12-23 Thread Florian Obser
First fulfil all challenges then tell the CA that it should check.

With a CSR with multiple SANs acme-client would write one challenge,
tell the CA, write the next challenge and so on.

For http-01 this doesn't matter but I think this will be nicer for dns-01
because there are propagation delays to consider.

Please be extra careful checking this. If I mess this up people might
run into renewal issues months from now. And when that happens people
tend to comment... (Which I also pull this out of the big diff I'm
currently working on for dns-01.)

OK?

diff --git netproc.c netproc.c
index 38732a4dd01..7c502643acc 100644
--- netproc.c
+++ netproc.c
@@ -840,7 +840,12 @@ netproc(int kfd, int afd, int Cfd, int cfd, int dfd, int 
rfd,
if (readop(Cfd, COMM_CHNG_ACK) != CHNG_ACK)
goto out;
 
-   /* Write to the CA that it's ready. */
+   }
+   /* Write to the CA that it's ready. */
+   for (i = 0; i < order.authsz; i++) {
+   if (chngs[i].status == CHNG_VALID ||
+   chngs[i].status == CHNG_INVALID)
+   continue;
if (!dochngresp(, [i]))
goto out;
}


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



Re: IPv6 pf_test EACCES

2020-12-22 Thread Florian Obser
Yes please.
OK florian

On 21 December 2020 23:34:04 CET, Alexander Bluhm  
wrote:
>Hi,
>
>A while ago we decided to pass EACCES to uerland if pf blocks a
>packet.  IPv6 still has the old EHOSTUNREACH code.
>
>Use the same errno for dropped IPv6 packets as in IPv4.
>
>ok?
>
>bluhm
>
>Index: netinet6/ip6_output.c
>===
>RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_output.c,v
>retrieving revision 1.247
>diff -u -p -r1.247 ip6_output.c
>--- netinet6/ip6_output.c  17 Jul 2020 15:21:36 -  1.247
>+++ netinet6/ip6_output.c  21 Dec 2020 22:27:24 -
>@@ -616,7 +616,7 @@ reroute:
> 
> #if NPF > 0
>   if (pf_test(AF_INET6, PF_OUT, ifp, ) != PF_PASS) {
>-  error = EHOSTUNREACH;
>+  error = EACCES;
>   m_freem(m);
>   goto done;
>   }
>@@ -2773,7 +2773,7 @@ ip6_output_ipsec_send(struct tdb *tdb, s
>   if ((encif = enc_getif(tdb->tdb_rdomain, tdb->tdb_tap)) == NULL ||
>   pf_test(AF_INET6, fwd ? PF_FWD : PF_OUT, encif, ) != PF_PASS) {
>   m_freem(m);
>-  return EHOSTUNREACH;
>+  return EACCES;
>   }
>   if (m == NULL)
>   return 0;

-- 
Sent from a mobile device. Please excuse poor formating.



Re: kdump: show scope for v6 addresses if set

2020-12-20 Thread Florian Obser
On Sun, Dec 20, 2020 at 02:34:09PM +0100, Claudio Jeker wrote:
> On Sun, Dec 20, 2020 at 01:39:57PM +0100, Otto Moerbeek wrote:
> > Hi,
> > 
> > scope is there, just not shown. While there, use proper constants for
> > two sizes.
> > 
> > -Otto
> > 
> > 
> > Index: ktrstruct.c
> > ===
> > RCS file: /cvs/src/usr.bin/kdump/ktrstruct.c,v
> > retrieving revision 1.28
> > diff -u -p -r1.28 ktrstruct.c
> > --- ktrstruct.c 17 Nov 2018 20:46:12 -  1.28
> > +++ ktrstruct.c 20 Dec 2020 12:34:34 -
> > @@ -90,7 +90,7 @@ ktrsockaddr(struct sockaddr *sa)
> > switch(sa->sa_family) {
> > case AF_INET: {
> > struct sockaddr_in  *sa_in;
> > -   char addr[64];
> > +   char addr[INET_ADDRSTRLEN];
> >  
> > sa_in = (struct sockaddr_in *)sa;
> > check_sockaddr_len(in);
> > @@ -100,12 +100,15 @@ ktrsockaddr(struct sockaddr *sa)
> > }
> > case AF_INET6: {
> > struct sockaddr_in6 *sa_in6;
> > -   char addr[64];
> > +   char addr[INET6_ADDRSTRLEN], scope[12] = { 0 };
> >  
> > sa_in6 = (struct sockaddr_in6 *)sa;
> > check_sockaddr_len(in6);
> > inet_ntop(AF_INET6, _in6->sin6_addr, addr, sizeof addr);
> > -   printf("[%s]:%u", addr, htons(sa_in6->sin6_port));
> > +   if (sa_in6->sin6_scope_id)
> > +   snprintf(scope, sizeof(scope), "%%%u",
> > +   sa_in6->sin6_scope_id);
> 
> Would it make sense to use if_indextoname() here to translate the string
> into an interface name? The snprintf would still be needed for the case
> where NULL is returned by if_indextoname().

(I had already OK'ed this off-list.)

I had thought about this as well, at that point we might as well use
getnameinfo(3).

However, when we run kdump the interface might no longer exist or
maybe we run kdump on a different host with different interfaces.

> 
> > +   printf("[%s%s]:%u", addr, scope, htons(sa_in6->sin6_port));
> > break;
> > }
> > case AF_UNIX: {
> > 
> 
> -- 
> :wq Claudio
> 

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



dig(1): nuke isc_sockaddr_fromin{,6}

2020-12-20 Thread Florian Obser
Rewrite parse_netprefix to no longer use isc_sockaddr_fromin{,6}.
Since this was the last user of those functions we can delete them.

That turd seems to be reasonably shiny with this...

OK?

diff --git dighost.c dighost.c
index 116de3a1c6d..b822a92c756 100644
--- dighost.c
+++ dighost.c
@@ -933,68 +933,34 @@ parse_bits(char *arg, uint32_t max) {
 isc_result_t
 parse_netprefix(struct sockaddr_storage **sap, int *plen, const char *value) {
struct sockaddr_storage *sa = NULL;
-   struct in_addr in4;
-   struct in6_addr in6;
-   uint32_t prefix_length = 0x;
-   char *slash = NULL;
-   int parsed = 0;
-   int prefix_parsed = 0;
-   char buf[sizeof("::::::XXX.XXX.XXX.XXX/128")];
-   const char *errstr;
+   struct in_addr *in4;
+   struct in6_addr *in6;
+   int prefix_length;
 
REQUIRE(sap != NULL && *sap == NULL);
 
-   if (strlcpy(buf, value, sizeof(buf)) >= sizeof(buf))
-   fatal("invalid prefix '%s'\n", value);
-
-   sa = malloc(sizeof(*sa));
+   sa = calloc(1, sizeof(*sa));
if (sa == NULL)
fatal("out of memory");
-   memset(sa, 0, sizeof(*sa));
 
-   if (strcmp(buf, "0") == 0) {
+   in4 = &((struct sockaddr_in *)sa)->sin_addr;
+   in6 = &((struct sockaddr_in6 *)sa)->sin6_addr;
+
+   if (strcmp(value, "0") == 0) {
sa->ss_family = AF_UNSPEC;
prefix_length = 0;
goto done;
}
 
-   slash = strchr(buf, '/');
-   if (slash != NULL) {
-   *slash = '\0';
-   prefix_length = strtonum(slash + 1, 0, 128, );
-   if (errstr != NULL) {
-   fatal("prefix length is %s: '%s'", errstr, value);
-   }
-   prefix_parsed = 1;
-   }
-
-   if (inet_pton(AF_INET6, buf, ) == 1) {
-   parsed = 1;
-   isc_sockaddr_fromin6(sa, , 0);
-   if (prefix_length > 128)
-   prefix_length = 128;
-   } else if (inet_pton(AF_INET, buf, ) == 1) {
-   parsed = 1;
-   isc_sockaddr_fromin(sa, , 0);
-   if (prefix_length > 32)
-   prefix_length = 32;
-   } else if (prefix_parsed) {
-   int i;
-
-   for (i = 0; i < 3 && strlen(buf) < sizeof(buf) - 2; i++) {
-   strlcat(buf, ".0", sizeof(buf));
-   if (inet_pton(AF_INET, buf, ) == 1) {
-   parsed = 1;
-   isc_sockaddr_fromin(sa, , 0);
-   break;
-   }
-   }
-
-   if (prefix_length > 32)
-   prefix_length = 32;
-   }
-
-   if (!parsed)
+   if ((prefix_length = inet_net_pton(AF_INET6, value, in6, sizeof(*in6)))
+   != -1) {
+   sa->ss_len = sizeof(struct sockaddr_in6);
+   sa->ss_family = AF_INET6;
+   } else if ((prefix_length = inet_net_pton(AF_INET, value, in4,
+   sizeof(*in4))) != -1) {
+   sa->ss_len = sizeof(struct sockaddr_in);
+   sa->ss_family = AF_INET;
+   } else
fatal("invalid address '%s'", value);
 
 done:
diff --git lib/isc/include/isc/sockaddr.h lib/isc/include/isc/sockaddr.h
index c29bbd30760..365e9528e84 100644
--- lib/isc/include/isc/sockaddr.h
+++ lib/isc/include/isc/sockaddr.h
@@ -82,20 +82,6 @@ isc_sockaddr_anyofpf(struct sockaddr_storage *sockaddr, int 
family);
  * \li 'family' is AF_INET or AF_INET6.
  */
 
-void
-isc_sockaddr_fromin(struct sockaddr_storage *sockaddr, const struct in_addr 
*ina,
-   in_port_t port);
-/*%<
- * Construct an struct sockaddr_storage from an IPv4 address and port.
- */
-
-void
-isc_sockaddr_fromin6(struct sockaddr_storage *sockaddr, const struct in6_addr 
*ina6,
-in_port_t port);
-/*%<
- * Construct an struct sockaddr_storage from an IPv6 address and port.
- */
-
 int
 isc_sockaddr_pf(const struct sockaddr_storage *sockaddr);
 /*%<
diff --git lib/isc/sockaddr.c lib/isc/sockaddr.c
index 42ee201fb35..56b79d7c881 100644
--- lib/isc/sockaddr.c
+++ lib/isc/sockaddr.c
@@ -195,18 +195,6 @@ isc_sockaddr_any6(struct sockaddr_storage *sockaddr)
sin6->sin6_port = 0;
 }
 
-void
-isc_sockaddr_fromin(struct sockaddr_storage *sockaddr, const struct in_addr 
*ina,
-   in_port_t port)
-{
-   struct sockaddr_in *sin = (struct sockaddr_in *) sockaddr;
-   memset(sockaddr, 0, sizeof(*sockaddr));
-   sin->sin_family = AF_INET;
-   sin->sin_len = sizeof(*sin);
-   sin->sin_addr = *ina;
-   sin->sin_port = htons(port);
-}
-
 void
 isc_sockaddr_anyofpf(struct sockaddr_storage *sockaddr, int pf) {
  switch (pf) {
@@ -221,18 +209,6 @@ isc_sockaddr_anyofpf(struct sockaddr_storage *sockaddr, 
int pf) {
  }
 }
 
-void
-isc_sockaddr_fromin6(struct 

Re: dig vs ipv6 (scoped) addresses

2020-12-20 Thread Florian Obser
On Thu, Dec 17, 2020 at 12:15:16PM +0100, Otto Moerbeek wrote:
> Hi,

> 
> as noted on misc dig does not like to talk to local link addresses.
> This fixes that case. While investigating I also found another bug:

Thanks for looking into this. Looks like I got distracted while
ripping out isc_sockaddr and did not fully clean it up. Probably
because I found another isc_indirection to delete :/

I'd rather like to get rid of isc_sockaddr_fromin* completely, see
diff at the end.

> selecting v6 or v4 addresses only from resolv.conf via the -4 or -6
> command line argument does not work as expected.

Nice catch.

> 
> This fixes both. I did not test binding to a src address with this.
> This is likely as broken as it was before.

My diff fixes that, too.

I still need to keep isc_sockaddr_fromin* because it's used for
+subnet i.e. ecs. Which is broken, too. I'm having a look now.

> 
>   -Otto
> 

> Index: lib/lwres/lwconfig.c
> ===
> RCS file: /cvs/src/usr.bin/dig/lib/lwres/lwconfig.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 lwconfig.c
> --- lib/lwres/lwconfig.c  25 Feb 2020 05:00:43 -  1.6
> +++ lib/lwres/lwconfig.c  17 Dec 2020 11:06:49 -
> @@ -231,7 +231,7 @@ lwres_conf_parsenameserver(lwres_conf_t 
>  
>   res = lwres_create_addr(word, , 1);
>   use_ipv4 = confdata->flags & LWRES_USEIPV4;
> - use_ipv6 = confdata->flags & LWRES_USEIPV4;
> + use_ipv6 = confdata->flags & LWRES_USEIPV6;
>   if (res == LWRES_R_SUCCESS &&
>   ((address.family == LWRES_ADDRTYPE_V4 && use_ipv4) ||
>   (address.family == LWRES_ADDRTYPE_V6 && use_ipv6))) {
> 

OK florian for this

OK for this version for the rest?


diff --git dig.c dig.c
index a0988a0567b..6b142a03239 100644
--- dig.c
+++ dig.c
@@ -17,7 +17,10 @@
 /* $Id: dig.c,v 1.18 2020/09/15 11:47:42 florian Exp $ */
 
 /*! \file */
-#include 
+#include 
+#include 
+
+#include 
 
 #include 
 #include 
@@ -1275,10 +1278,7 @@ dash_option(char *option, char *next, dig_lookup_t 
**lookup,
dns_rdatatype_t rdtype;
dns_rdataclass_t rdclass;
char textname[MXNAME];
-   struct in_addr in4;
-   struct in6_addr in6;
-   in_port_t srcport;
-   char *hash, *cmd;
+   char *cmd;
uint32_t num;
const char *errstr;
 
@@ -1346,28 +1346,39 @@ dash_option(char *option, char *next, dig_lookup_t 
**lookup,
if (value == NULL)
goto invalid_option;
switch (opt) {
-   case 'b':
+   case 'b': {
+   struct addrinfo *ai = NULL, hints;
+   int error;
+   char *hash;
+
+   memset(, 0, sizeof(hints));
+   hints.ai_flags = AI_NUMERICHOST;
+   hints.ai_socktype = SOCK_STREAM;
+
hash = strchr(value, '#');
if (hash != NULL) {
-   num = strtonum(hash + 1, 0, MAXPORT, );
-   if (errstr != NULL)
-   fatal("port number is %s: '%s'", errstr,
-   hash + 1);
-   srcport = num;
*hash = '\0';
+   error = getaddrinfo(value, hash + 1, , );
+   *hash = '#';
} else
-   srcport = 0;
-   if (have_ipv6 && inet_pton(AF_INET6, value, ) == 1)
-   isc_sockaddr_fromin6(_address, , srcport);
-   else if (have_ipv4 && inet_pton(AF_INET, value, ) == 1)
-   isc_sockaddr_fromin(_address, , srcport);
-   else
+   error = getaddrinfo(value, NULL, , );
+
+   if (error)
+   fatal("invalid address %s: %s", value,
+   gai_strerror(error));
+   if (ai == NULL || ai->ai_addrlen > sizeof(bind_address))
+   fatal("invalid address %s", value);
+   if (!have_ipv4 && ai->ai_family == AF_INET)
+   fatal("invalid address %s", value);
+   if (!have_ipv6 && ai->ai_family == AF_INET6)
fatal("invalid address %s", value);
 
-   if (hash != NULL)
-   *hash = '#';
+   memset(_address, 0, sizeof(bind_address));
+   memcpy(_address, ai->ai_addr, ai->ai_addrlen);
+
specified_source = 1;
return (value_from_next);
+   }
case 'c':
if ((*lookup)->rdclassset) {
fprintf(stderr, ";; Warning, extra class option\n");
diff --git dighost.c dighost.c
index 52afde3d7d3..3974843600d 100644
--- dighost.c
+++ dighost.c
@@ -498,6 +498,7 @@ get_addresses(const char *hostname, in_port_t dstport,
 {
struct addrinfo *ai = NULL, *tmpai, hints;
int result, i;
+   char dport[sizeof("65535")];
 
REQUIRE(hostname != NULL);

Re: acme-client(1) make -F flag use more obvious

2020-12-15 Thread Florian Obser



On 15 December 2020 14:56:41 CET, Stuart Henderson  wrote:
>On 2020/12/15 10:18, Solene Rapenne wrote:
>> This is a small change to acme-client(1) because I find
>> the explanation of -F flag not being obvious that you
>> need it when you add/remove an alternative name in your
>> domain config.
>
>This only works directly for adding. For removal you need to rm
>the existing certificate.

-F only handles forced renewals correctly.
That it can be (ab)used to add subject alt names to a cert is an implementation 
detail.

It would be nice if someone™ would fix this properly by acme-client detecting 
that cert and config do not agree anymore.

-- 
Sent from a mobile device. Please excuse poor formating.



Re: unwind(8): handle large answers differently

2020-12-03 Thread Florian Obser
Anyone?

While I could volunteer all y'all for testing that seems a bit silly.
I'd rather have someone to read the diff because
- The tricky cases will be very rare to hit in normal operations,
  DNS answers are just not that big.
- I'd like to hear that this is actually a good pattern to do this and
  I'm not completely off the trail

Eventually I will put it in though, to make progress.

Thanks.

On Thu, Nov 19, 2020 at 03:59:03PM +0100, Florian Obser wrote:
> On Fri, Nov 13, 2020 at 05:53:53PM +0100, Florian Obser wrote:
> > The recent fix for handling large (about 16k) answers in unwind has
> > the downside that we are now always copying at least 16k per answer
> > between the resolver and frontend process.
> > That seems wasteful.
> > 
> > This re-arranges things to only copy what its needed.
> > 
> > Tests, OKs?
> > 
> 
> Anyone? So far I had one (1) test report.
> 
> diff --git frontend.c frontend.c
> index 8adbb07219b..74715087c5f 100644
> --- frontend.c
> +++ frontend.c
> @@ -86,7 +86,8 @@ struct pending_query {
>   int  fd;
>   int  bogus;
>   int  rcode_override;
> - ssize_t  answer_len;
> + int  answer_len;
> + int  received;
>   uint8_t *answer;
>  };
>  
> @@ -424,10 +425,7 @@ frontend_dispatch_resolver(int fd, short event, void 
> *bula)
>   struct imsgev   *iev = bula;
>   struct imsgbuf  *ibuf = >ibuf;
>   struct imsg  imsg;
> - struct query_imsg   *query_imsg;
> - struct answer_imsg  *answer_imsg;
>   int  n, shut = 0, chg;
> - uint8_t *p;
>  
>   if (event & EV_READ) {
>   if ((n = imsg_read(ibuf)) == -1 && errno != EAGAIN)
> @@ -449,45 +447,30 @@ frontend_dispatch_resolver(int fd, short event, void 
> *bula)
>   break;
>  
>   switch (imsg.hdr.type) {
> - case IMSG_ANSWER_HEADER:
> - if (IMSG_DATA_SIZE(imsg) != sizeof(*query_imsg))
> - fatalx("%s: IMSG_ANSWER_HEADER wrong length: "
> - "%lu", __func__, IMSG_DATA_SIZE(imsg));
> - query_imsg = (struct query_imsg *)imsg.data;
> - if ((pq = find_pending_query(query_imsg->id)) ==
> - NULL) {
> - log_warnx("cannot find pending query %llu",
> - query_imsg->id);
> - break;
> - }
> - if (query_imsg->err) {
> - send_answer(pq);
> - pq = NULL;
> - break;
> - }
> - pq->bogus = query_imsg->bogus;
> - break;
> - case IMSG_ANSWER:
> - if (IMSG_DATA_SIZE(imsg) != sizeof(*answer_imsg))
> + case IMSG_ANSWER: {
> + struct answer_header*answer_header;
> + int  data_len;
> + uint8_t *data;
> +
> + if (IMSG_DATA_SIZE(imsg) < sizeof(*answer_header))
>   fatalx("%s: IMSG_ANSWER wrong length: "
>   "%lu", __func__, IMSG_DATA_SIZE(imsg));
> - answer_imsg = (struct answer_imsg *)imsg.data;
> - if ((pq = find_pending_query(answer_imsg->id)) ==
> + answer_header = (struct answer_header *)imsg.data;
> + data = (uint8_t *)imsg.data + sizeof(*answer_header);
> + if (answer_header->answer_len > 65535)
> + fatalx("%s: IMSG_ANSWER answer too big: %d",
> + __func__, answer_header->answer_len);
> + data_len = IMSG_DATA_SIZE(imsg) -
> + sizeof(*answer_header);
> +
> + if ((pq = find_pending_query(answer_header->id)) ==
>   NULL) {
> - log_warnx("cannot find pending query %llu",
> - answer_imsg->id);
> + log_warnx("%s: cannot find pending query

nsd 4.3.4

2020-12-02 Thread Florian Obser


Tests, OKs?

diff --git doc/ChangeLog doc/ChangeLog
index 9bcf7de6ab6..018c484aac3 100644
--- doc/ChangeLog
+++ doc/ChangeLog
@@ -1,3 +1,45 @@
+24 November 2020: Wouter
+   - Merge PR #141: ZONEMD RR type.
+   - tag for 4.3.4rc1.
+
+23 November 2020: Wouter
+   - Fix #142: NODATA answers missin SOA in authority section after
+ CNAME chain.
+   - Fix for CVE-2020-28935 : Fix that symlink does not interfere
+ with chown of pidfile.
+   - fix writepid for retvalue 0.
+
+9 November 2020: Wouter
+   - Fix #138: NSD returns non-EDNS answer when QUESTION is empty.
+   - Fix to check nscount in previous fix for EDNS in formerr response
+ when there is no question.
+
+28 October 2020: Wouter
+   - Remove unused init_cfg_parse routine from configlexer.
+
+20 October 2020: Wouter
+   - Fix to add missing closest encloser NSEC3 for wildcard nodata type
+ DS answer.
+
+14 October 2020: Wouter
+   - Fix #134: IPV4_MINIMAL_RESPONSE_SIZE vs EDNS_MAX_MESSAGE_LEN.
+
+13 October 2020: Wouter
+   - Fix missing parenthesis on size of fix to init buffer.
+
+12 October 2020: Wouter
+   - Fix #127: two minor `-Wcast-qual` cleanups
+   - Fix #126: minor header hygiene
+   - Fix #125: include config.h in compat/setproctitle.c and fix prototype 
of `setproctitle`
+   - Fix #133: fix 0-init of local ( stack ) buffer.
+
+8 October 2020: Wouter
+   - tag for 4.3.3 release
+   - current repository contains 4.3.4 in development.
+   - Fix #129: ambiguous use of errno, in log message if sendmmsg fails.
+   - Fix #128: Fix that the invalid port number is logged for sendmmsg
+ failed: Invalid argument.
+
 1 October 2020: Wouter
- tag for 4.3.3rc1 release.
 
diff --git config.h.in config.h.in
index 4940c8e28ec..c528729cc75 100644
--- config.h.in
+++ config.h.in
@@ -946,7 +946,7 @@ char *nsd_strptime(const char *s, const char *format, 
struct tm *tm);
 #ifdef __linux__
 #define HAVE_SETPROCTITLE 1
 #include 
-void setproctitle(char *fmt, ...);
+void setproctitle(const char *fmt, ...);
 #endif
 #endif
 
diff --git configlexer.lex configlexer.lex
index bb4dd3499c4..7ed3deb2f1f 100644
--- configlexer.lex
+++ configlexer.lex
@@ -36,16 +36,6 @@ struct inc_state {
 };
 static struct inc_state* config_include_stack = NULL;
 static int inc_depth = 0;
-static int inc_prev = 0;
-static int num_args = 0;
-
-void init_cfg_parse(void)
-{
-   config_include_stack = NULL;
-   inc_depth = 0;
-   inc_prev = 0;
-   num_args = 0;
-}
 
 static void config_start_include(const char* filename)
 {
diff --git configure configure
index 1e8b73a0cd6..349068f5d0c 100644
--- configure
+++ configure
@@ -1,6 +1,6 @@
 #! /bin/sh
 # Guess values for system-dependent variables and create Makefiles.
-# Generated by GNU Autoconf 2.69 for NSD 4.3.3.
+# Generated by GNU Autoconf 2.69 for NSD 4.3.4.
 #
 # Report bugs to .
 #
@@ -580,8 +580,8 @@ MAKEFLAGS=
 # Identity of this package.
 PACKAGE_NAME='NSD'
 PACKAGE_TARNAME='nsd'
-PACKAGE_VERSION='4.3.3'
-PACKAGE_STRING='NSD 4.3.3'
+PACKAGE_VERSION='4.3.4'
+PACKAGE_STRING='NSD 4.3.4'
 PACKAGE_BUGREPORT='nsd-b...@nlnetlabs.nl'
 PACKAGE_URL=''
 
@@ -1314,7 +1314,7 @@ if test "$ac_init_help" = "long"; then
   # Omit some internal or obsolete options to make the list less imposing.
   # This message is too long to be a string in the A/UX 3.1 sh.
   cat <<_ACEOF
-\`configure' configures NSD 4.3.3 to adapt to many kinds of systems.
+\`configure' configures NSD 4.3.4 to adapt to many kinds of systems.
 
 Usage: $0 [OPTION]... [VAR=VALUE]...
 
@@ -1376,7 +1376,7 @@ fi
 
 if test -n "$ac_init_help"; then
   case $ac_init_help in
- short | recursive ) echo "Configuration of NSD 4.3.3:";;
+ short | recursive ) echo "Configuration of NSD 4.3.4:";;
esac
   cat <<\_ACEOF
 
@@ -1536,7 +1536,7 @@ fi
 test -n "$ac_init_help" && exit $ac_status
 if $ac_init_version; then
   cat <<\_ACEOF
-NSD configure 4.3.3
+NSD configure 4.3.4
 generated by GNU Autoconf 2.69
 
 Copyright (C) 2012 Free Software Foundation, Inc.
@@ -2245,7 +2245,7 @@ cat >config.log <<_ACEOF
 This file contains any messages produced by compilers while
 running configure, to aid debugging if configure makes a mistake.
 
-It was created by NSD $as_me 4.3.3, which was
+It was created by NSD $as_me 4.3.4, which was
 generated by GNU Autoconf 2.69.  Invocation command line was
 
   $ $0 $@
@@ -10835,7 +10835,7 @@ cat >>$CONFIG_STATUS <<\_ACEOF || ac_write_fail=1
 # report actual input values of CONFIG_FILES etc. instead of their
 # values after options handling.
 ac_log="
-This file was extended by NSD $as_me 4.3.3, which was
+This file was extended by NSD $as_me 4.3.4, which was
 generated by GNU Autoconf 2.69.  Invocation command line was
 
   CONFIG_FILES= $CONFIG_FILES
@@ -10897,7 +10897,7 @@ _ACEOF
 cat >>$CONFIG_STATUS <<_ACEOF || ac_write_fail=1
 ac_cs_config="`$as_echo "$ac_configure_args" | sed 's/^ //; 

Re: ACPI diff that needs wide testing

2020-12-01 Thread Florian Obser
On Tue, Dec 01, 2020 at 10:40:30PM +0100, Mark Kettenis wrote:
> > From: Greg Steuck 
> > Date: Mon, 30 Nov 2020 20:54:59 -0800
> > 
> > Mark Kettenis  writes:
> > 
> > > The diff below fixes the way we handle named references in AML
> > > packages.  This fixes some bugs but I'd like to make sure that this
> > > doesn't inadvertedly break things.  So tests on a wide variety of
> > > machines are welcome.
> > 
> > I see a prompt crash:
> 
> Does this one work better?

Indeed it does, no panic, suspend resume still works, no change in dmesg.
X1 Gen 2.

OpenBSD 6.8-current (GENERIC.MP) #2: Wed Dec  2 08:09:03 CET 2020

flor...@openbsd-build.home.narrans.de:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 8233394176 (7851MB)
avail mem = 7968555008 (7599MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xdcd3d000 (61 entries)
bios0: vendor LENOVO version "GRET40WW (1.17 )" date 09/02/2014
bios0: LENOVO 20A7006VUS
acpi0 at bios0: ACPI 5.0
acpi0: sleep states S0 S3 S4 S5
acpi0: tables DSDT FACP SLIC DBGP ECDT HPET APIC MCFG SSDT SSDT SSDT SSDT SSDT 
SSDT SSDT PCCT SSDT UEFI MSDM ASF! BATB FPDT UEFI DMAR
acpi0: wakeup devices LID_(S4) SLPB(S3) IGBE(S4) EXP2(S4) XHCI(S3) EHC1(S3)
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpiec0 at acpi0
acpihpet0 at acpi0: 14318179 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Core(TM) i7-4600U CPU @ 2.10GHz, 798.30 MHz, 06-45-01
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,SRBDS_CTRL,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
cpu0: 256KB 64b/line 8-way L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 99MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4.1.1.1, IBE
cpu1 at mainbus0: apid 1 (application processor)
cpu1: Intel(R) Core(TM) i7-4600U CPU @ 2.10GHz, 798.16 MHz, 06-45-01
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,SRBDS_CTRL,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
cpu1: 256KB 64b/line 8-way L2 cache
cpu1: smt 1, core 0, package 0
cpu2 at mainbus0: apid 2 (application processor)
cpu2: Intel(R) Core(TM) i7-4600U CPU @ 2.10GHz, 798.16 MHz, 06-45-01
cpu2: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,SRBDS_CTRL,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
cpu2: 256KB 64b/line 8-way L2 cache
cpu2: smt 0, core 1, package 0
cpu3 at mainbus0: apid 3 (application processor)
cpu3: Intel(R) Core(TM) i7-4600U CPU @ 2.10GHz, 798.16 MHz, 06-45-01
cpu3: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,SRBDS_CTRL,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
cpu3: 256KB 64b/line 8-way L2 cache
cpu3: smt 1, core 1, package 0
ioapic0 at mainbus0: apid 2 pa 0xfec0, version 20, 40 pins
acpimcfg0 at acpi0
acpimcfg0: addr 0xf800, bus 0-63
acpiprt0 at acpi0: bus 0 (PCI0)
acpiprt1 at acpi0: bus -1 (PEG_)
acpiprt2 at acpi0: bus 2 (EXP1)
acpiprt3 at acpi0: bus 3 (EXP2)
acpiprt4 at acpi0: bus -1 (EXP3)
dwiic0 at acpi0 I2C1 addr 0xfe105000/0x1000 irq 7
iic0 at dwiic0
"CPLM3218" at iic0 addr 0x48 not configured
acpibtn0 at acpi0: LID_
acpibtn1 at acpi0: SLPB
acpipci0 at acpi0 PCI0: 0x 0x0011 0x0001
acpicmos0 at acpi0
acpibat0 at acpi0: BAT0 model "45N1703" serial  3191 type LiP oem "SMP"
acpiac0 at acpi0: AC unit offline
acpithinkpad0 at acpi0: version 2.0
"PNP0C14" at acpi0 not configured
"PNP0C14" at acpi0 not configured
"PNP0C14" at acpi0 not configured
"INT340F" at acpi0 not configured
acpicpu0 at acpi0: C3(200@506 mwait.1@0x60), C2(200@148 mwait.1@0x33), 
C1(1000@1 

Re: rad(8): rdomain support

2020-12-01 Thread Florian Obser
I copied the (void)strlcpy from somewhere, likely ifconfig originally.
Thanks for pointing it out, I shall remove it before commit.
I don't like that style and don't think it helps anything during review since 
one needs to check anyway that truncation either can't happen or doesn't matter.

On 1 December 2020 12:02:14 CET, Theo Buehler  wrote:
>On Mon, Nov 30, 2020 at 05:46:57PM +0100, Florian Obser wrote:
>> On Sun, Nov 29, 2020 at 12:20:31PM +0100, Florian Obser wrote:
>> > 
>> > Let rad(8) handle all rdomains in a single daemon, similar to
>previous
>> > work in slaacd.
>> > 
>> > Suggested / requested by tb who showed me previous work by reyk
>which
>> > unfortunately predated my work in slaacd and followed a different
>> > pattern.
>> > 
>> > There has already been a fair bit of testing and input from tb on
>> > previous iterations.
>> > 
>> > More tests, OKs?
>> > 
>> 
>> Updated diff after some testing by tb@
>> 
>> - our AF_ROUTE socket needs a RTABLE_ANY ROUTE_TABLEFILTER otherwise
>>   we don't see link-local addresses arriving in rdomains != 0.
>> - check if the arriving link-local address is tentative (or a
>>   douplicate) and ignore it because we can't use it and sendmsg
>failes.
>>   Monitor RTM_CHGADDRATTR messages to see when the link-local address
>>   becomes valid
>
>Thank you so much for this!
>
>I think I have done all the testing I can for my use case and this now
>looks all good. I think this covers most, if not all, code paths added
>by this diff. I did as thorough a review as I can, being foreign to
>this
>part of the tree.
>
>ok tb
>
>I only have one tiny comment:
>
>> +++ frontend.c
>
>[...]
>
>> @@ -746,15 +743,29 @@ get_link_state(char *if_name)
>>  return ls;
>>  }
>>  
>> +int
>> +get_ifrdomain(char *if_name)
>> +{
>> +struct ifreq ifr;
>> +
>> +(void) strlcpy(ifr.ifr_name, if_name, sizeof(ifr.ifr_name));
>
>I'm not sure why you explicitly indicate that you don't want to check,
>while you don't so in interface_has_linklocal_address() (both copy the
>same name to an array of length IFNAMSIZ).

-- 
Sent from a mobile device. Please excuse poor formating.



Re: rad(8): rdomain support

2020-11-30 Thread Florian Obser
On Sun, Nov 29, 2020 at 12:20:31PM +0100, Florian Obser wrote:
> 
> Let rad(8) handle all rdomains in a single daemon, similar to previous
> work in slaacd.
> 
> Suggested / requested by tb who showed me previous work by reyk which
> unfortunately predated my work in slaacd and followed a different
> pattern.
> 
> There has already been a fair bit of testing and input from tb on
> previous iterations.
> 
> More tests, OKs?
> 

Updated diff after some testing by tb@

- our AF_ROUTE socket needs a RTABLE_ANY ROUTE_TABLEFILTER otherwise
  we don't see link-local addresses arriving in rdomains != 0.
- check if the arriving link-local address is tentative (or a
  douplicate) and ignore it because we can't use it and sendmsg failes.
  Monitor RTM_CHGADDRATTR messages to see when the link-local address
  becomes valid

diff --git frontend.c frontend.c
index 8947f616e73..c9c63b0cb6b 100644
--- frontend.c
+++ frontend.c
@@ -95,19 +95,22 @@ struct icmp6_ev {
struct msghdrrcvmhdr;
struct iovec rcviov[1];
struct sockaddr_in6  from;
-} icmp6ev;
+   int  refcnt;
+};
 
 struct ra_iface {
-   TAILQ_ENTRY(ra_iface)   entry;
-   struct ra_prefix_conf_head  prefixes;
-   charname[IF_NAMESIZE];
-   charconf_name[IF_NAMESIZE];
-   uint32_tif_index;
-   int removed;
-   int link_state;
-   int prefix_count;
-   size_t  datalen;
-   uint8_t data[RA_MAX_SIZE];
+   TAILQ_ENTRY(ra_iface)entry;
+   struct icmp6_ev *icmp6ev;
+   struct ra_prefix_conf_head   prefixes;
+   char name[IF_NAMESIZE];
+   char conf_name[IF_NAMESIZE];
+   uint32_t if_index;
+   int  rdomain;
+   int  removed;
+   int  link_state;
+   int  prefix_count;
+   size_t   datalen;
+   uint8_t  data[RA_MAX_SIZE];
 };
 
 TAILQ_HEAD(, ra_iface) ra_interfaces;
@@ -119,6 +122,7 @@ void icmp6_receive(int, short, void 
*);
 voidjoin_all_routers_mcast_group(struct ra_iface *);
 voidleave_all_routers_mcast_group(struct ra_iface *);
 int get_link_state(char *);
+int get_ifrdomain(char *);
 voidmerge_ra_interface(char *, char *);
 voidmerge_ra_interfaces(void);
 struct ra_iface*find_ra_iface_by_id(uint32_t);
@@ -127,6 +131,9 @@ struct ra_iface_conf*find_ra_iface_conf(struct 
ra_iface_conf_head *,
char *);
 struct ra_prefix_conf  *find_ra_prefix_conf(struct ra_prefix_conf_head*,
struct in6_addr *, int);
+struct icmp6_ev*get_icmp6ev_by_rdomain(int);
+voidunref_icmp6ev(struct ra_iface *);
+voidset_icmp6sock(int, int);
 voidadd_new_prefix_to_ra_iface(struct ra_iface *r,
struct in6_addr *, int, struct ra_prefix_conf *);
 voidfree_ra_iface(struct ra_iface *);
@@ -147,7 +154,7 @@ struct rad_conf *frontend_conf;
 struct imsgev  *iev_main;
 struct imsgev  *iev_engine;
 struct eventev_route;
-int icmp6sock = -1, ioctlsock = -1, routesock = -1;
+int ioctlsock = -1, routesock = -1;
 struct ipv6_mreqall_routers;
 struct sockaddr_in6 all_nodes;
 struct msghdr   sndmhdr;
@@ -175,8 +182,7 @@ frontend(int debug, int verbose)
 {
struct event ev_sigint, ev_sigterm;
struct passwd   *pw;
-   size_t   rcvcmsglen, sndcmsgbuflen;
-   uint8_t *rcvcmsgbuf;
+   size_t   sndcmsgbuflen;
uint8_t *sndcmsgbuf = NULL;
 
frontend_conf = config_new_empty();
@@ -229,20 +235,6 @@ frontend(int debug, int verbose)
iev_main->handler, iev_main);
event_add(_main->ev, NULL);
 
-   rcvcmsglen = CMSG_SPACE(sizeof(struct in6_pktinfo)) +
-   CMSG_SPACE(sizeof(int));
-   if((rcvcmsgbuf = malloc(rcvcmsglen)) == NULL)
-   fatal("malloc");
-
-   icmp6ev.rcviov[0].iov_base = (caddr_t)icmp6ev.answer;
-   icmp6ev.rcviov[0].iov_len = sizeof(icmp6ev.answer);
-   icmp6ev.rcvmhdr.msg_name = (caddr_t)
-   icmp6ev.rcvmhdr.msg_namelen = sizeof(icmp6ev.from);
-   icmp6ev.rcvmhdr.msg_iov =

Re: ldapd warning

2020-11-29 Thread Florian Obser
On Sun, Nov 29, 2020 at 08:41:50AM +0100, Theo Buehler wrote:
> On Sun, Nov 29, 2020 at 08:02:45AM +0100, Emil Engler wrote:
> > It can overflow! Please check for the positivity and width of size_t before!
> 
> What can overflow? ret is guaranteed to be non-negative before the cast.
> 
> As for the width (which would be about truncation, not overflow): while
> the standard allows for size_t to be an unsigned integer type as small
> as 16 bits, we generally assume that sizeof(size_t) >= sizeof(int).
> I don't think I've ever seen a width check ensuring this in our sources.

Maybe rummage arround in the openssl attic? There migth be code that
checks for sizeof(size_t) changing during runtime :P

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



rad(8): rdomain support

2020-11-29 Thread Florian Obser


Let rad(8) handle all rdomains in a single daemon, similar to previous
work in slaacd.

Suggested / requested by tb who showed me previous work by reyk which
unfortunately predated my work in slaacd and followed a different
pattern.

There has already been a fair bit of testing and input from tb on
previous iterations.

More tests, OKs?

diff --git frontend.c frontend.c
index 8947f616e73..014e5700588 100644
--- frontend.c
+++ frontend.c
@@ -95,19 +95,22 @@ struct icmp6_ev {
struct msghdrrcvmhdr;
struct iovec rcviov[1];
struct sockaddr_in6  from;
-} icmp6ev;
+   int  refcnt;
+};
 
 struct ra_iface {
-   TAILQ_ENTRY(ra_iface)   entry;
-   struct ra_prefix_conf_head  prefixes;
-   charname[IF_NAMESIZE];
-   charconf_name[IF_NAMESIZE];
-   uint32_tif_index;
-   int removed;
-   int link_state;
-   int prefix_count;
-   size_t  datalen;
-   uint8_t data[RA_MAX_SIZE];
+   TAILQ_ENTRY(ra_iface)entry;
+   struct icmp6_ev *icmp6ev;
+   struct ra_prefix_conf_head   prefixes;
+   char name[IF_NAMESIZE];
+   char conf_name[IF_NAMESIZE];
+   uint32_t if_index;
+   int  rdomain;
+   int  removed;
+   int  link_state;
+   int  prefix_count;
+   size_t   datalen;
+   uint8_t  data[RA_MAX_SIZE];
 };
 
 TAILQ_HEAD(, ra_iface) ra_interfaces;
@@ -119,6 +122,7 @@ void icmp6_receive(int, short, void 
*);
 voidjoin_all_routers_mcast_group(struct ra_iface *);
 voidleave_all_routers_mcast_group(struct ra_iface *);
 int get_link_state(char *);
+int get_ifrdomain(char *);
 voidmerge_ra_interface(char *, char *);
 voidmerge_ra_interfaces(void);
 struct ra_iface*find_ra_iface_by_id(uint32_t);
@@ -127,6 +131,9 @@ struct ra_iface_conf*find_ra_iface_conf(struct 
ra_iface_conf_head *,
char *);
 struct ra_prefix_conf  *find_ra_prefix_conf(struct ra_prefix_conf_head*,
struct in6_addr *, int);
+struct icmp6_ev*get_icmp6ev_by_rdomain(int);
+voidunref_icmp6ev(struct ra_iface *);
+voidset_icmp6sock(int, int);
 voidadd_new_prefix_to_ra_iface(struct ra_iface *r,
struct in6_addr *, int, struct ra_prefix_conf *);
 voidfree_ra_iface(struct ra_iface *);
@@ -147,7 +154,7 @@ struct rad_conf *frontend_conf;
 struct imsgev  *iev_main;
 struct imsgev  *iev_engine;
 struct eventev_route;
-int icmp6sock = -1, ioctlsock = -1, routesock = -1;
+int ioctlsock = -1, routesock = -1;
 struct ipv6_mreqall_routers;
 struct sockaddr_in6 all_nodes;
 struct msghdr   sndmhdr;
@@ -175,8 +182,7 @@ frontend(int debug, int verbose)
 {
struct event ev_sigint, ev_sigterm;
struct passwd   *pw;
-   size_t   rcvcmsglen, sndcmsgbuflen;
-   uint8_t *rcvcmsgbuf;
+   size_t   sndcmsgbuflen;
uint8_t *sndcmsgbuf = NULL;
 
frontend_conf = config_new_empty();
@@ -229,20 +235,6 @@ frontend(int debug, int verbose)
iev_main->handler, iev_main);
event_add(_main->ev, NULL);
 
-   rcvcmsglen = CMSG_SPACE(sizeof(struct in6_pktinfo)) +
-   CMSG_SPACE(sizeof(int));
-   if((rcvcmsgbuf = malloc(rcvcmsglen)) == NULL)
-   fatal("malloc");
-
-   icmp6ev.rcviov[0].iov_base = (caddr_t)icmp6ev.answer;
-   icmp6ev.rcviov[0].iov_len = sizeof(icmp6ev.answer);
-   icmp6ev.rcvmhdr.msg_name = (caddr_t)
-   icmp6ev.rcvmhdr.msg_namelen = sizeof(icmp6ev.from);
-   icmp6ev.rcvmhdr.msg_iov = icmp6ev.rcviov;
-   icmp6ev.rcvmhdr.msg_iovlen = 1;
-   icmp6ev.rcvmhdr.msg_control = (caddr_t) rcvcmsgbuf;
-   icmp6ev.rcvmhdr.msg_controllen = rcvcmsglen;
-
if (inet_pton(AF_INET6, "ff02::2",
_routers.ipv6mr_multiaddr.s6_addr) == -1)
fatal("inet_pton");
@@ -316,7 +308,7 @@ frontend_dispatch_main(int fd, short event, void *bula)
struct ra_prefix_conf   *ra_prefix_conf;
struct ra_rdnss_conf*ra_rdnss_conf;
struct ra_dnssl_conf*ra_dnssl_conf;
-  

Re: ldapd warning

2020-11-29 Thread Florian Obser
On Sun, Nov 29, 2020 at 06:42:47PM +1000, Jonathan Matthew wrote:
> On Sat, Nov 28, 2020 at 11:20:30PM +0100, Theo Buehler wrote:
> > /usr/src/usr.sbin/ldapd/util.c:46:21: warning: comparison of integers of 
> > different signs:
> >   'int' and 'size_t' (aka 'unsigned long') [-Wsign-compare]
> > if (ret < 0 || ret >= size)
> >~~~ ^  
> > 
> > This has been around for a while. I forgot that I had this patch in my
> > tree.
> 
> 'size' was cast to int before r1.11 of util.c, I'm not sure why the cast was
> removed.  smtpd also has a copy of this function that still has the cast.

because it's the wrong cast. Presumably that cast was put in to
silence a compiler warning, but it's the wrong way around. We need to
cast the smaller type to the larger type.

OK florian

tb, could you also fix smtpd?

> 
> 
> > 
> > Index: util.c
> > ===
> > RCS file: /cvs/src/usr.sbin/ldapd/util.c,v
> > retrieving revision 1.12
> > diff -u -p -r1.12 util.c
> > --- util.c  24 Oct 2019 12:39:26 -  1.12
> > +++ util.c  4 Aug 2020 07:14:33 -
> > @@ -43,7 +43,7 @@ bsnprintf(char *str, size_t size, const 
> > va_start(ap, format);
> > ret = vsnprintf(str, size, format, ap);
> > va_end(ap);
> > -   if (ret < 0 || ret >= size)
> > +   if (ret < 0 || (size_t)ret >= size)
> > return 0;
> >  
> > return 1;
> > 
> 

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



slaacd(8): unexpected FD

2020-11-27 Thread Florian Obser
An interface might have disappeared or switched rdomains while we
waited for a FD. It's not a fatal condition if it arrives late.

I can only get it to lose the race by introducing a sleep in the
parent process, but the race is still there. Found while implementing
rdomain support in rad(8) using the same pattern.

I'm now wondering if it would be better to listen on the route socket
for interface arrivals / departure in the parent process instead of
the frontend asking for a raw socket to be opened. But that's a diff
for another time.

OK?

diff --git frontend.c frontend.c
index 4b3f575611e..7efbe50df20 100644
--- frontend.c
+++ frontend.c
@@ -1164,9 +1164,14 @@ set_icmp6sock(int icmp6sock, int rdomain)
}
}
 
-   if (icmp6sock != -1)
-   fatalx("received unneeded ICMPv6 socket for rdomain %d",
-   rdomain);
+   if (icmp6sock != -1) {
+   /*
+* The interface disappeared or changed rdomain while we were
+* waiting for the parent process to open the raw socket.
+*/
+   close(icmp6sock);
+   return;
+   }
 
LIST_FOREACH (iface, , entries) {
if (event_initialized(>icmp6ev->ev) &&

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



slaacd(8): unref_icmp6ev()

2020-11-27 Thread Florian Obser
Reduce code duplication by introducing unref_icmp6ev()

OK?

diff --git frontend.c frontend.c
index 624ff5562f3..4b3f575611e 100644
--- frontend.c
+++ frontend.c
@@ -92,6 +92,7 @@ void   get_lladdr(char *, struct ether_addr *, struct 
sockaddr_in6 *);
 struct iface   *get_iface_by_id(uint32_t);
 voidremove_iface(uint32_t);
 struct icmp6_ev*get_icmp6ev_by_rdomain(int);
+voidunref_icmp6ev(struct iface *);
 voidset_icmp6sock(int, int);
 voidsend_solicitation(uint32_t);
 #ifndefSMALL
@@ -519,15 +520,7 @@ update_iface(uint32_t if_index, char* if_name)
 
if (iface != NULL) {
if (iface->rdomain != ifrdomain) {
-   if (iface->icmp6ev != NULL) {
-   iface->icmp6ev->refcnt--;
-   if (iface->icmp6ev->refcnt == 0) {
-   event_del(>icmp6ev->ev);
-   close(EVENT_FD(>icmp6ev->ev));
-   free(iface->icmp6ev);
-   }
-   iface->icmp6ev = NULL;
-   }
+   unref_icmp6ev(iface);
iface->rdomain = ifrdomain;
iface->icmp6ev = get_icmp6ev_by_rdomain(ifrdomain);
}
@@ -1102,14 +1095,7 @@ remove_iface(uint32_t if_index)
 
LIST_REMOVE(iface, entries);
 
-   if (iface->icmp6ev != NULL) {
-   iface->icmp6ev->refcnt--;
-   if (iface->icmp6ev->refcnt == 0) {
-   event_del(>icmp6ev->ev);
-   close(EVENT_FD(>icmp6ev->ev));
-   free(iface->icmp6ev);
-   }
-   }
+   unref_icmp6ev(iface);
free(iface);
 }
 
@@ -1148,6 +1134,20 @@ get_icmp6ev_by_rdomain(int rdomain)
return (icmp6ev);
 }
 
+void
+unref_icmp6ev(struct iface *iface)
+{
+   if (iface->icmp6ev != NULL) {
+   iface->icmp6ev->refcnt--;
+   if (iface->icmp6ev->refcnt == 0) {
+   event_del(>icmp6ev->ev);
+   close(EVENT_FD(>icmp6ev->ev));
+   free(iface->icmp6ev);
+   }
+   iface->icmp6ev = NULL;
+   }
+}
+
 void
 set_icmp6sock(int icmp6sock, int rdomain)
 {
-- 
2.29.2



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



slaacd(8): changing rdomain

2020-11-27 Thread Florian Obser


Handle the case of an autoconf interface changing its rdomain.

To avoide code duplication have get_icmp6ev_by_rdomain() either return
an existing icmp6ev in the correct rdomain or allocate one.

OK?

diff --git frontend.c frontend.c
index 3bf418ba31e..624ff5562f3 100644
--- frontend.c
+++ frontend.c
@@ -502,7 +502,6 @@ void
 update_iface(uint32_t if_index, char* if_name)
 {
struct iface*iface;
-   struct icmp6_ev *icmp6ev;
struct imsg_ifinfo   imsg_ifinfo;
int  flags, xflags, ifrdomain;
 
@@ -516,32 +515,29 @@ update_iface(uint32_t if_index, char* if_name)
if((ifrdomain = get_ifrdomain(if_name)) == -1)
return;
 
-   if ((iface = get_iface_by_id(if_index)) == NULL) {
+   iface = get_iface_by_id(if_index);
+
+   if (iface != NULL) {
+   if (iface->rdomain != ifrdomain) {
+   if (iface->icmp6ev != NULL) {
+   iface->icmp6ev->refcnt--;
+   if (iface->icmp6ev->refcnt == 0) {
+   event_del(>icmp6ev->ev);
+   close(EVENT_FD(>icmp6ev->ev));
+   free(iface->icmp6ev);
+   }
+   iface->icmp6ev = NULL;
+   }
+   iface->rdomain = ifrdomain;
+   iface->icmp6ev = get_icmp6ev_by_rdomain(ifrdomain);
+   }
+   } else {
if ((iface = calloc(1, sizeof(*iface))) == NULL)
fatal("calloc");
iface->if_index = if_index;
iface->rdomain = ifrdomain;
+   iface->icmp6ev = get_icmp6ev_by_rdomain(ifrdomain);
 
-   if ((icmp6ev = get_icmp6ev_by_rdomain(ifrdomain)) == NULL) {
-   if ((icmp6ev = calloc(1, sizeof(*icmp6ev))) == NULL)
-   fatal("calloc");
-   icmp6ev->rcviov[0].iov_base = (caddr_t)icmp6ev->answer;
-   icmp6ev->rcviov[0].iov_len = sizeof(icmp6ev->answer);
-   icmp6ev->rcvmhdr.msg_name = (caddr_t)>from;
-   icmp6ev->rcvmhdr.msg_namelen = sizeof(icmp6ev->from);
-   icmp6ev->rcvmhdr.msg_iov = icmp6ev->rcviov;
-   icmp6ev->rcvmhdr.msg_iovlen = 1;
-   icmp6ev->rcvmhdr.msg_controllen =
-   CMSG_SPACE(sizeof(struct in6_pktinfo)) +
-   CMSG_SPACE(sizeof(int));
-   if ((icmp6ev->rcvmhdr.msg_control = malloc(icmp6ev->
-   rcvmhdr.msg_controllen)) == NULL)
-   fatal("malloc");
-   frontend_imsg_compose_main(IMSG_OPEN_ICMP6SOCK, 0,
-   , sizeof(ifrdomain));
-   }
-   iface->icmp6ev = icmp6ev;
-   iface->icmp6ev->refcnt++;
LIST_INSERT_HEAD(, iface, entries);
}
 
@@ -1121,13 +1117,35 @@ struct icmp6_ev*
 get_icmp6ev_by_rdomain(int rdomain)
 {
struct iface*iface;
+   struct icmp6_ev *icmp6ev = NULL;
 
LIST_FOREACH (iface, , entries) {
-   if (iface->rdomain == rdomain)
-   return (iface->icmp6ev);
+   if (iface->rdomain == rdomain) {
+   icmp6ev = iface->icmp6ev;
+   break;
+   }
}
 
-   return (NULL);
+   if (icmp6ev == NULL) {
+   if ((icmp6ev = calloc(1, sizeof(*icmp6ev))) == NULL)
+   fatal("calloc");
+   icmp6ev->rcviov[0].iov_base = (caddr_t)icmp6ev->answer;
+   icmp6ev->rcviov[0].iov_len = sizeof(icmp6ev->answer);
+   icmp6ev->rcvmhdr.msg_name = (caddr_t)>from;
+   icmp6ev->rcvmhdr.msg_namelen = sizeof(icmp6ev->from);
+   icmp6ev->rcvmhdr.msg_iov = icmp6ev->rcviov;
+   icmp6ev->rcvmhdr.msg_iovlen = 1;
+   icmp6ev->rcvmhdr.msg_controllen =
+   CMSG_SPACE(sizeof(struct in6_pktinfo)) +
+   CMSG_SPACE(sizeof(int));
+   if ((icmp6ev->rcvmhdr.msg_control = malloc(icmp6ev->
+   rcvmhdr.msg_controllen)) == NULL)
+   fatal("malloc");
+   frontend_imsg_compose_main(IMSG_OPEN_ICMP6SOCK, 0,
+   , sizeof(rdomain));
+   }
+   icmp6ev->refcnt++;
+   return (icmp6ev);
 }
 
 void
-- 
2.29.2



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



Re: Typo fix in nsd.conf.5.in

2020-11-27 Thread Florian Obser
Could you please submit this upstream at
https://github.com/NLnetLabs/nsd

Thanks,
Florian

On Fri, Nov 27, 2020 at 05:44:11AM -0500, Eddie Thieda wrote:
> Hello,
> 
> Here's a small typo fix, url included if text gets mangled.
> 
> http://ix.io/2FEF
> 
> --- nsd.conf.5.in Tue Oct 13 06:06:08 2020
> +++ nsd.conf.5.in2 Fri Nov 27 05:35:17 2020
> @@ -161,7 +161,7 @@ anycast instances.  Use ip-transparent to be able to l
>  turn on later (typical for certain load-balancing).
>  .TP
>  .B interface:\fR [@port] [servers] [bindtodevice] [setfib]
> -Same as ip\-address (for easy of compatibility with unbound.conf).
> +Same as ip\-address (for easy compatibility with unbound.conf).
>  .TP
>  .B ip\-transparent:\fR 
>  Allows NSD to bind to non local addresses. This is useful to have NSD
> 

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



rad(8): check if IPv6 is enabled on an interface

2020-11-26 Thread Florian Obser
The way rad(8) is normaly used is netstart(8) setup a bunch of
interfaces with IPv6 prefixes and one or more are listed in rad.conf.
Everything just works.

But rad can also be configured to work on interfaces that are
dynamically added (tap, pair, vether...) It would then spam the log
with:
sendmsg on vether1: Can't assign requested address
as noted by tb.

It's also not sensible to use such an interface, it can't possibly
work. So we are going to ignore interfaces without link local address.

OK?

diff --git frontend.c frontend.c
index c932c3dfca3..77bbedb2a3e 100644
--- frontend.c
+++ frontend.c
@@ -133,6 +133,7 @@ void free_ra_iface(struct ra_iface 
*);
 int in6_mask2prefixlen(struct in6_addr *);
 voidget_interface_prefixes(struct ra_iface *,
 struct ra_prefix_conf *);
+int interface_has_linklocal_address(char *);
 voidbuild_packet(struct ra_iface *);
 voidbuild_leaving_packet(struct ra_iface *);
 voidra_output(struct ra_iface *, struct sockaddr_in6 *);
@@ -750,14 +751,19 @@ merge_ra_interface(char *name, char *conf_name)
 {
struct ra_iface *ra_iface;
uint32_t if_index;
-   int  link_state;
+   int  link_state, has_linklocal;
 
link_state = get_link_state(name);
+   has_linklocal = interface_has_linklocal_address(name);
 
if ((ra_iface = find_ra_iface_by_name(name)) != NULL) {
ra_iface->link_state = link_state;
if (!LINK_STATE_IS_UP(link_state)) {
-   log_debug("%s down, ignoring", name);
+   log_debug("%s down, removing", name);
+   ra_iface->removed = 1;
+   } else if (!has_linklocal) {
+   log_debug("%s has no IPv6 link-local address, "
+   "removing", name);
ra_iface->removed = 1;
} else {
log_debug("keeping interface %s", name);
@@ -771,6 +777,11 @@ merge_ra_interface(char *name, char *conf_name)
return;
}
 
+   if (!has_linklocal) {
+   log_debug("%s has no IPv6 link-local address, ignoring", name);
+   return;
+   }
+
log_debug("new interface %s", name);
if ((if_index = if_nametoindex(name)) == 0)
return;
@@ -918,6 +929,32 @@ in6_mask2prefixlen(struct in6_addr *in6)
return (plen);
 }
 
+int
+interface_has_linklocal_address(char *name)
+{
+   struct ifaddrs  *ifap, *ifa;
+   struct sockaddr_in6 *sin6;
+   int  ret = 0;
+
+   if (getifaddrs() != 0)
+   fatal("getifaddrs");
+
+   for (ifa = ifap; ifa != NULL; ifa = ifa->ifa_next) {
+   if (strcmp(name, ifa->ifa_name) != 0)
+   continue;
+   if (ifa->ifa_addr->sa_family != AF_INET6)
+   continue;
+
+   sin6 = (struct sockaddr_in6 *)ifa->ifa_addr;
+
+   if (IN6_IS_ADDR_LINKLOCAL(>sin6_addr)) {
+   ret = 1;
+   break;
+   }
+   }
+   freeifaddrs(ifap);
+   return (ret);
+}
 void
 get_interface_prefixes(struct ra_iface *ra_iface, struct ra_prefix_conf
 *autoprefix)


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



Re: unwind(8): handle large answers differently

2020-11-19 Thread Florian Obser
On Fri, Nov 13, 2020 at 05:53:53PM +0100, Florian Obser wrote:
> The recent fix for handling large (about 16k) answers in unwind has
> the downside that we are now always copying at least 16k per answer
> between the resolver and frontend process.
> That seems wasteful.
> 
> This re-arranges things to only copy what its needed.
> 
> Tests, OKs?
> 

Anyone? So far I had one (1) test report.

diff --git frontend.c frontend.c
index 8adbb07219b..74715087c5f 100644
--- frontend.c
+++ frontend.c
@@ -86,7 +86,8 @@ struct pending_query {
int  fd;
int  bogus;
int  rcode_override;
-   ssize_t  answer_len;
+   int  answer_len;
+   int  received;
uint8_t *answer;
 };
 
@@ -424,10 +425,7 @@ frontend_dispatch_resolver(int fd, short event, void *bula)
struct imsgev   *iev = bula;
struct imsgbuf  *ibuf = >ibuf;
struct imsg  imsg;
-   struct query_imsg   *query_imsg;
-   struct answer_imsg  *answer_imsg;
int  n, shut = 0, chg;
-   uint8_t *p;
 
if (event & EV_READ) {
if ((n = imsg_read(ibuf)) == -1 && errno != EAGAIN)
@@ -449,45 +447,30 @@ frontend_dispatch_resolver(int fd, short event, void 
*bula)
break;
 
switch (imsg.hdr.type) {
-   case IMSG_ANSWER_HEADER:
-   if (IMSG_DATA_SIZE(imsg) != sizeof(*query_imsg))
-   fatalx("%s: IMSG_ANSWER_HEADER wrong length: "
-   "%lu", __func__, IMSG_DATA_SIZE(imsg));
-   query_imsg = (struct query_imsg *)imsg.data;
-   if ((pq = find_pending_query(query_imsg->id)) ==
-   NULL) {
-   log_warnx("cannot find pending query %llu",
-   query_imsg->id);
-   break;
-   }
-   if (query_imsg->err) {
-   send_answer(pq);
-   pq = NULL;
-   break;
-   }
-   pq->bogus = query_imsg->bogus;
-   break;
-   case IMSG_ANSWER:
-   if (IMSG_DATA_SIZE(imsg) != sizeof(*answer_imsg))
+   case IMSG_ANSWER: {
+   struct answer_header*answer_header;
+   int  data_len;
+   uint8_t *data;
+
+   if (IMSG_DATA_SIZE(imsg) < sizeof(*answer_header))
fatalx("%s: IMSG_ANSWER wrong length: "
"%lu", __func__, IMSG_DATA_SIZE(imsg));
-   answer_imsg = (struct answer_imsg *)imsg.data;
-   if ((pq = find_pending_query(answer_imsg->id)) ==
+   answer_header = (struct answer_header *)imsg.data;
+   data = (uint8_t *)imsg.data + sizeof(*answer_header);
+   if (answer_header->answer_len > 65535)
+   fatalx("%s: IMSG_ANSWER answer too big: %d",
+   __func__, answer_header->answer_len);
+   data_len = IMSG_DATA_SIZE(imsg) -
+   sizeof(*answer_header);
+
+   if ((pq = find_pending_query(answer_header->id)) ==
NULL) {
-   log_warnx("cannot find pending query %llu",
-   answer_imsg->id);
+   log_warnx("%s: cannot find pending query %llu",
+   __func__, answer_header->id);
break;
}
 
-   p = realloc(pq->answer, pq->answer_len +
-   answer_imsg->len);
-
-   if (p != NULL) {
-   pq->answer = p;
-   memcpy(pq->answer + pq->answer_len,
-   answer_imsg->answer, answer_imsg->len);
-   pq->answer_len += answer_imsg->len;
-   } else {
+   if (answer_header->srvfail) {
free(pq->answer);
pq->answer_len = 0;
  

unwind(8): handle large answers differently

2020-11-13 Thread Florian Obser
The recent fix for handling large (about 16k) answers in unwind has
the downside that we are now always copying at least 16k per answer
between the resolver and frontend process.
That seems wasteful.

This re-arranges things to only copy what its needed.

Tests, OKs?

diff --git frontend.c frontend.c
index 8adbb07219b..74715087c5f 100644
--- frontend.c
+++ frontend.c
@@ -86,7 +86,8 @@ struct pending_query {
int  fd;
int  bogus;
int  rcode_override;
-   ssize_t  answer_len;
+   int  answer_len;
+   int  received;
uint8_t *answer;
 };
 
@@ -424,10 +425,7 @@ frontend_dispatch_resolver(int fd, short event, void *bula)
struct imsgev   *iev = bula;
struct imsgbuf  *ibuf = >ibuf;
struct imsg  imsg;
-   struct query_imsg   *query_imsg;
-   struct answer_imsg  *answer_imsg;
int  n, shut = 0, chg;
-   uint8_t *p;
 
if (event & EV_READ) {
if ((n = imsg_read(ibuf)) == -1 && errno != EAGAIN)
@@ -449,45 +447,30 @@ frontend_dispatch_resolver(int fd, short event, void 
*bula)
break;
 
switch (imsg.hdr.type) {
-   case IMSG_ANSWER_HEADER:
-   if (IMSG_DATA_SIZE(imsg) != sizeof(*query_imsg))
-   fatalx("%s: IMSG_ANSWER_HEADER wrong length: "
-   "%lu", __func__, IMSG_DATA_SIZE(imsg));
-   query_imsg = (struct query_imsg *)imsg.data;
-   if ((pq = find_pending_query(query_imsg->id)) ==
-   NULL) {
-   log_warnx("cannot find pending query %llu",
-   query_imsg->id);
-   break;
-   }
-   if (query_imsg->err) {
-   send_answer(pq);
-   pq = NULL;
-   break;
-   }
-   pq->bogus = query_imsg->bogus;
-   break;
-   case IMSG_ANSWER:
-   if (IMSG_DATA_SIZE(imsg) != sizeof(*answer_imsg))
+   case IMSG_ANSWER: {
+   struct answer_header*answer_header;
+   int  data_len;
+   uint8_t *data;
+
+   if (IMSG_DATA_SIZE(imsg) < sizeof(*answer_header))
fatalx("%s: IMSG_ANSWER wrong length: "
"%lu", __func__, IMSG_DATA_SIZE(imsg));
-   answer_imsg = (struct answer_imsg *)imsg.data;
-   if ((pq = find_pending_query(answer_imsg->id)) ==
+   answer_header = (struct answer_header *)imsg.data;
+   data = (uint8_t *)imsg.data + sizeof(*answer_header);
+   if (answer_header->answer_len > 65535)
+   fatalx("%s: IMSG_ANSWER answer too big: %d",
+   __func__, answer_header->answer_len);
+   data_len = IMSG_DATA_SIZE(imsg) -
+   sizeof(*answer_header);
+
+   if ((pq = find_pending_query(answer_header->id)) ==
NULL) {
-   log_warnx("cannot find pending query %llu",
-   answer_imsg->id);
+   log_warnx("%s: cannot find pending query %llu",
+   __func__, answer_header->id);
break;
}
 
-   p = realloc(pq->answer, pq->answer_len +
-   answer_imsg->len);
-
-   if (p != NULL) {
-   pq->answer = p;
-   memcpy(pq->answer + pq->answer_len,
-   answer_imsg->answer, answer_imsg->len);
-   pq->answer_len += answer_imsg->len;
-   } else {
+   if (answer_header->srvfail) {
free(pq->answer);
pq->answer_len = 0;
pq->answer = NULL;
@@ -495,9 +478,32 @@ frontend_dispatch_resolver(int fd, short event, void *bula)
send_answer(pq);
break;
}
-   if (!answer_imsg->truncated)
+
+   if (pq->answer == NULL) {
+ 

unwind(8): handle large answers

2020-11-04 Thread Florian Obser
Handle DNS answers that are larger than the maximum imsg size by
splitting them up.

This might fix an issue Leo reported to me in private where unwind
would exit with this in syslog:
unwind[13752]: fatal in frontend: expected IMSG_ANSWER but got HEADER
unwind[45337]: resolver exiting
unwind[43711]: terminating 

The maximum imsg size is about 16k and it kinda seems unlikely to see
answers in the wild that are this big but that's currently the only
theory I have. This also improves error logging, so if it's not this
we might ave more luck once it triggers again.

This is on top of the query_imsg2str diff I just send to tech.

OK?

diff --git frontend.c frontend.c
index f8558d60ab4..90fdd805257 100644
--- frontend.c
+++ frontend.c
@@ -420,12 +420,14 @@ frontend_dispatch_main(int fd, short event, void *bula)
 void
 frontend_dispatch_resolver(int fd, short event, void *bula)
 {
-   static struct pending_query *pq;
+   struct pending_query*pq;
struct imsgev   *iev = bula;
struct imsgbuf  *ibuf = >ibuf;
struct imsg  imsg;
struct query_imsg   *query_imsg;
+   struct answer_imsg  *answer_imsg;
int  n, shut = 0, chg;
+   uint8_t *p;
 
if (event & EV_READ) {
if ((n = imsg_read(ibuf)) == -1 && errno != EAGAIN)
@@ -448,8 +450,6 @@ frontend_dispatch_resolver(int fd, short event, void *bula)
 
switch (imsg.hdr.type) {
case IMSG_ANSWER_HEADER:
-   if (pq != NULL)
-   fatalx("expected IMSG_ANSWER but got HEADER");
if (IMSG_DATA_SIZE(imsg) != sizeof(*query_imsg))
fatalx("%s: IMSG_ANSWER_HEADER wrong length: "
"%lu", __func__, IMSG_DATA_SIZE(imsg));
@@ -468,19 +468,32 @@ frontend_dispatch_resolver(int fd, short event, void 
*bula)
pq->bogus = query_imsg->bogus;
break;
case IMSG_ANSWER:
-   if (pq == NULL)
-   fatalx("IMSG_ANSWER without HEADER");
-
-   if (pq->answer)
-   fatal("pq->answer");
-   if ((pq->answer = malloc(IMSG_DATA_SIZE(imsg))) !=
+   if (IMSG_DATA_SIZE(imsg) != sizeof(*answer_imsg))
+   fatalx("%s: IMSG_ANSWER wrong length: "
+   "%lu", __func__, IMSG_DATA_SIZE(imsg));
+   answer_imsg = (struct answer_imsg *)imsg.data;
+   if ((pq = find_pending_query(answer_imsg->id)) ==
NULL) {
-   pq->answer_len = IMSG_DATA_SIZE(imsg);
-   memcpy(pq->answer, imsg.data, pq->answer_len);
-   } else
+   log_warnx("cannot find pending query %llu",
+   answer_imsg->id);
+   break;
+   }
+
+   p = realloc(pq->answer, pq->answer_len +
+   answer_imsg->len);
+
+   if (p != NULL) {
+   pq->answer = p;
+   memcpy(pq->answer + pq->answer_len,
+   answer_imsg->answer, answer_imsg->len);
+   pq->answer_len += answer_imsg->len;
+   } else {
+   pq->answer_len = 0;
+   pq->answer = NULL;
pq->rcode_override = LDNS_RCODE_SERVFAIL;
-   send_answer(pq);
-   pq = NULL;
+   }
+   if (!answer_imsg->truncated)
+   send_answer(pq);
break;
case IMSG_CTL_RESOLVER_INFO:
case IMSG_CTL_AUTOCONF_RESOLVER_INFO:
diff --git resolver.c resolver.c
index a1e18f9d130..4aec42ac5cb 100644
--- resolver.c
+++ resolver.c
@@ -879,6 +879,7 @@ resolve_done(struct uw_resolver *res, void *arg, int rcode,
sldns_buffer*buf = NULL;
struct regional *region = NULL;
struct query_imsg   *query_imsg;
+   struct answer_imsg   answer_imsg;
struct running_query*rq;
struct timespec  tp, elapsed;
int64_t  ms;
@@ -886,6 +887,7 @@ resolve_done(struct uw_resolver *res, void *arg, int rcode,
int  running_res, asr_pref_pos, force_acceptbogus;
char*str;
char rcode_buf[16];
+   uint8_t *p;
 
clock_gettime(CLOCK_MONOTONIC, );
 
@@ 

unwind(8): query_imsg2str

2020-11-04 Thread Florian Obser
Introduce query_imsg2str() for the printing "qname class type".

OK?

diff --git resolver.c resolver.c
index dea78ca2fb3..a1e18f9d130 100644
--- resolver.c
+++ resolver.c
@@ -194,6 +194,7 @@ void decay_latest_histograms(int, 
short, void *);
 int running_query_cnt(void);
 int*resolvers_to_restart(struct uw_conf *,
 struct uw_conf *);
+const char *query_imsg2str(struct query_imsg *);
 
 struct uw_conf *resolver_conf;
 struct imsgev  *iev_frontend;
@@ -752,8 +753,6 @@ try_next_resolver(struct running_query *rq)
struct timespec  tp, elapsed;
struct timeval   tv = {0, 0};
int64_t  ms;
-   char qclass_buf[16];
-   char qtype_buf[16];
 
while(rq->next_resolver < rq->res_pref.len &&
((res = resolvers[rq->res_pref.types[rq->next_resolver]]) == NULL ||
@@ -772,13 +771,9 @@ try_next_resolver(struct running_query *rq)
timespecsub(, >tp, );
ms = elapsed.tv_sec * 1000 + elapsed.tv_nsec / 100;
 
-   sldns_wire2str_class_buf(rq->query_imsg->c, qclass_buf,
-   sizeof(qclass_buf));
-   sldns_wire2str_type_buf(rq->query_imsg->t, qtype_buf,
-   sizeof(qtype_buf));
-   log_debug("%s[+%lldms]: %s[%s] %s %s %s", __func__, ms,
+   log_debug("%s[+%lldms]: %s[%s] %s", __func__, ms,
uw_resolver_type_str[res->type], uw_resolver_state_str[res->state],
-   rq->query_imsg->qname, qclass_buf, qtype_buf);
+   query_imsg2str(rq->query_imsg));
 
if ((query_imsg = malloc(sizeof(*query_imsg))) == NULL) {
log_warnx("%s", __func__);
@@ -891,8 +886,6 @@ resolve_done(struct uw_resolver *res, void *arg, int rcode,
int  running_res, asr_pref_pos, force_acceptbogus;
char*str;
char rcode_buf[16];
-   char qclass_buf[16];
-   char qtype_buf[16];
 
clock_gettime(CLOCK_MONOTONIC, );
 
@@ -949,11 +942,9 @@ resolve_done(struct uw_resolver *res, void *arg, int rcode,
result->answer_len = 0;
 
sldns_wire2str_rcode_buf(result->rcode, rcode_buf, sizeof(rcode_buf));
-   sldns_wire2str_class_buf(query_imsg->c, qclass_buf, sizeof(qclass_buf));
-   sldns_wire2str_type_buf(query_imsg->t, qtype_buf, sizeof(qtype_buf));
-   log_debug("%s[%s]: %s %s %s rcode: %s[%d], elapsed: %lldms, running: 
%d",
-   __func__, uw_resolver_type_str[res->type], query_imsg->qname,
-   qclass_buf, qtype_buf, rcode_buf, result->rcode, ms,
+   log_debug("%s[%s]: %s rcode: %s[%d], elapsed: %lldms, running: %d",
+   __func__, uw_resolver_type_str[res->type],
+   query_imsg2str(query_imsg), rcode_buf, result->rcode, ms,
running_query_cnt());
 
force_acceptbogus = find_force(_conf->force, query_imsg->qname,
@@ -2116,3 +2107,18 @@ resolvers_to_restart(struct uw_conf *oconf, struct 
uw_conf *nconf)
}
return restart;
 }
+
+const char*
+query_imsg2str(struct query_imsg *query_imsg)
+{
+   static char  buf[sizeof(query_imsg->qname) + 1 + 16 + 1 + 16];
+   char qclass_buf[16];
+   char qtype_buf[16];
+
+   sldns_wire2str_class_buf(query_imsg->c, qclass_buf, sizeof(qclass_buf));
+   sldns_wire2str_type_buf(query_imsg->t, qtype_buf, sizeof(qtype_buf));
+
+   snprintf(buf, sizeof(buf), "%s %s %s", query_imsg->qname, qclass_buf,
+   qtype_buf);
+   return buf;
+}


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



Re: acme-client(1): replace httpd(8) location block in manpage by better match

2020-11-03 Thread Florian Obser
On Tue, Nov 03, 2020 at 10:37:04AM +0100, Matthias Pressfreund wrote:
> 
> On 2020-11-03 09:56, Florian Obser wrote:
> > On Mon, Nov 02, 2020 at 02:35:48PM +0100, Matthias Pressfreund wrote:
> >> The patch below updates the acme-client(1) manpage by providing a
> >> closer match for the httpd(8) location block accepting acme challenge
> >> responses.
> > 
> > How is this better?
> > 
> > When the requested file exits in /var/www/acme/ I get a 200 in both cases.
> > When the file does not exists I get a 404 in both cases.
> > 
> 
> It is better because I may not want the server to return 404 if the file
> does not exist. Instead, I may want to let the server fall back to its
> default behavior as shown in the example below where it would simply drop
> the connection.
> 
> server "example.com" {
>   ...
>   block drop
>   location found "/.well-known/acme-challenge/*" { ... }
>   ...
> }

I don't know, I'm not buying it, this doesn't feel neccesary for acme.
We wanted to have a minimal example that gets people going with
acme-client and httpd, now we have more fluff.

But I guess it's just me, so meh.

> 
> > If /var/www/acme itself is missing I get 404 without this and 500 with
> > this patch. Why is 500 better?
> > 
> 
> Even in this case I like the 500 better as it reflects the state of my
> server, like if I point a location's root to a directory that does not
> exist, my server truly suffers from an 'internal server error'.
> 
> 
> > Thanks,
> > Florian
> >>
> >>
> >> Index: usr.sbin/acme-client/acme-client.1
> >> ===
> >> RCS file: /cvs/src/usr.sbin/acme-client/acme-client.1,v
> >> retrieving revision 1.34
> >> diff -u -p -u -p -r1.34 acme-client.1
> >> --- usr.sbin/acme-client/acme-client.1 10 May 2020 12:06:18 -  
> >> 1.34
> >> +++ usr.sbin/acme-client/acme-client.1 2 Nov 2020 13:18:12 -
> >> @@ -14,7 +14,7 @@
> >>  .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT 
> >> OF
> >>  .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> >>  .\"
> >> -.Dd $Mdocdate: May 10 2020 $
> >> +.Dd $Mdocdate: November 2 2020 $
> >>  .Dt ACME-CLIENT 1
> >>  .Os
> >>  .Sh NAME
> >> @@ -58,7 +58,7 @@ can be served by
> >>  with this location block,
> >>  which will properly map response challenges:
> >>  .Bd -literal -offset indent
> >> -location "/.well-known/acme-challenge/*" {
> >> +location found "/.well-known/acme-challenge/*" {
> >>root "/acme"
> >>request strip 2
> >>  }
> >>
> > 
> 

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



Re: acme-client(1): replace httpd(8) location block in manpage by better match

2020-11-03 Thread Florian Obser
On Mon, Nov 02, 2020 at 02:35:48PM +0100, Matthias Pressfreund wrote:
> The patch below updates the acme-client(1) manpage by providing a
> closer match for the httpd(8) location block accepting acme challenge
> responses.

How is this better?

When the requested file exits in /var/www/acme/ I get a 200 in both cases.
When the file does not exists I get a 404 in both cases.

If /var/www/acme itself is missing I get 404 without this and 500 with
this patch. Why is 500 better?

Thanks,
Florian
> 
> 
> Index: usr.sbin/acme-client/acme-client.1
> ===
> RCS file: /cvs/src/usr.sbin/acme-client/acme-client.1,v
> retrieving revision 1.34
> diff -u -p -u -p -r1.34 acme-client.1
> --- usr.sbin/acme-client/acme-client.110 May 2020 12:06:18 -  
> 1.34
> +++ usr.sbin/acme-client/acme-client.12 Nov 2020 13:18:12 -
> @@ -14,7 +14,7 @@
>  .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
>  .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>  .\"
> -.Dd $Mdocdate: May 10 2020 $
> +.Dd $Mdocdate: November 2 2020 $
>  .Dt ACME-CLIENT 1
>  .Os
>  .Sh NAME
> @@ -58,7 +58,7 @@ can be served by
>  with this location block,
>  which will properly map response challenges:
>  .Bd -literal -offset indent
> -location "/.well-known/acme-challenge/*" {
> +location found "/.well-known/acme-challenge/*" {
>   root "/acme"
>   request strip 2
>  }
> 

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



dig(1): Extended DNS Error (RFC 8914)

2020-10-30 Thread Florian Obser
$ obj/dig @1.1.1.1 dnssec-failed.org

; <<>> dig 9.10.8-P1 <<>> @1.1.1.1 dnssec-failed.org
; (1 server found)
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: SERVFAIL, id: 26772
;; flags: qr rd ra; QUERY: 1, ANSWER: 0, AUTHORITY: 0, ADDITIONAL: 1

;; OPT PSEUDOSECTION:
; EDNS: version: 0, flags:; udp: 1232
; EDE: 6 (DNSSEC Bogus)
;; QUESTION SECTION:
;dnssec-failed.org. IN  A

;; Query time: 244 msec
;; SERVER: 1.1.1.1#53(1.1.1.1)
;; WHEN: Fri Oct 30 14:59:09 CET 2020
;; MSG SIZE  rcvd: 52

Since I'm not aware of a server/query combination that responds with
UTF-8 encoded EXTENDED-TEXT I didn't implement anything special for
this so it will use the default renderer that's also used for NSIDs,
printing a hexdump + printable ascii, e.g.:

$ dig @k.root-servers.net +nsid . soa
[...]
;; OPT PSEUDOSECTION:
; EDNS: version: 0, flags:; udp: 1232
; NSID: 6e 73 33 2e 6e 6c 2d 61 6d 73 2e 6b 2e 72 69 70 65 2e 6e 65 74 
("ns3.nl-ams.k.ripe.net")

OK?

diff --git lib/dns/include/dns/message.h lib/dns/include/dns/message.h
index 65ffcfd4c3f..a70720eee39 100644
--- lib/dns/include/dns/message.h
+++ lib/dns/include/dns/message.h
@@ -104,6 +104,7 @@
 #define DNS_OPT_COOKIE 10  /*%< COOKIE opt code */
 #define DNS_OPT_PAD12  /*%< PAD opt code */
 #define DNS_OPT_KEY_TAG14  /*%< Key tag opt code */
+#define DNS_OPT_EDE15  /* RFC 8914 */
 
 /*%< The number of EDNS options we know about. */
 #define DNS_EDNSOPTIONS4
diff --git lib/dns/message.c lib/dns/message.c
index 5e0fb167382..9721f9c0ef4 100644
--- lib/dns/message.c
+++ lib/dns/message.c
@@ -2434,6 +2434,68 @@ render_ecs(isc_buffer_t *ecsbuf, isc_buffer_t *target) {
return (ISC_R_SUCCESS);
 }
 
+static const char *
+ede_info_code2str(uint16_t info_code)
+{
+   if (info_code > 49151)
+   return "Private Use";
+
+   switch (info_code) {
+   case 0:
+   return "Other Error";
+   case 1:
+   return "Unsupported DNSKEY Algorithm";
+   case 2:
+   return "Unsupported DS Digest Type";
+   case 3:
+   return "Stale Answer";
+   case 4:
+   return "Forged Answer";
+   case 5:
+   return "DNSSEC Indeterminate";
+   case 6:
+   return "DNSSEC Bogus";
+   case 7:
+   return "Signature Expired";
+   case 8:
+   return "Signature Not Yet Valid";
+   case 9:
+   return "DNSKEY Missing";
+   case 10:
+   return "RRSIGs Missing";
+   case 11:
+   return "No Zone Key Bit Set";
+   case 12:
+   return "NSEC Missing";
+   case 13:
+   return "Cached Error";
+   case 14:
+   return "Not Ready";
+   case 15:
+   return "Blocked";
+   case 16:
+   return "Censored";
+   case 17:
+   return "Filtered";
+   case 18:
+   return "Prohibited";
+   case 19:
+   return "Stale NXDomain Answer";
+   case 20:
+   return "Not Authoritative";
+   case 21:
+   return "Not Supported";
+   case 22:
+   return "No Reachable Authority";
+   case 23:
+   return "Network Error";
+   case 24:
+   return "Invalid Data";
+   default:
+   return "Unassigned";
+   }
+}
+
 isc_result_t
 dns_message_pseudosectiontotext(dns_message_t *msg,
dns_pseudosection_t section,
@@ -2557,6 +2619,20 @@ dns_message_pseudosectiontotext(dns_message_t *msg,
ADD_STRING(target, "\n");
continue;
}
+   } else if (optcode == DNS_OPT_EDE) {
+   uint16_t info_code;
+   ADD_STRING(target, "; EDE");
+   if (optlen >= 2) {
+   info_code =
+   isc_buffer_getuint16();
+   optlen -= 2;
+   snprintf(buf, sizeof(buf), ": %u (",
+   info_code);
+   ADD_STRING(target, buf);
+   ADD_STRING(target,
+   ede_info_code2str(info_code));
+   ADD_STRING(target, ")");
+   }
} else {
ADD_STRING(target, "; OPT=");
snprintf(buf, sizeof(buf), "%u", optcode);


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



Re: ping(8) put hop_limit warning in verbose mode

2020-10-20 Thread Florian Obser
On Tue, Oct 20, 2020 at 01:11:09PM -0600, Theo de Raadt wrote:
> I believe most of the new local variables you added in main,
> can instead be added in the flush loop you wrote, even if you
> have to instantiate them.  Or move it into a new function,
> then it is even easier.  their scope is just too large...

Sure, I'd like to go with this for now and then refactor main later
and split it into functions. It's always a pain to find the spot where
it does anything...

diff --git ping.c ping.c
index d198d233921..7cca8c70342 100644
--- ping.c
+++ ping.c
@@ -786,6 +786,55 @@ main(int argc, char *argv[])
smsghdr.msg_iov = 
smsghdr.msg_iovlen = 1;
 
+   /* Drain our socket. */
+   (void)signal(SIGALRM, onsignal);
+   memset(, 0, sizeof(itimer));
+   itimer.it_value.tv_sec = 1; /* make sure we don't get stuck */
+   (void)setitimer(ITIMER_REAL, , NULL);
+   for (;;) {
+   struct msghdr   m;
+   union {
+   struct cmsghdr hdr;
+   u_char buf[CMSG_SPACE(1024)];
+   }   cmsgbuf;
+   struct ioveciov[1];
+   struct pollfd   pfd;
+   struct sockaddr_in  peer4;
+   struct sockaddr_in6 peer6;
+   ssize_t cc;
+
+   if (seenalrm)
+   break;
+
+   pfd.fd = s;
+   pfd.events = POLLIN;
+
+   if (poll(, 1, 0) <= 0)
+   break;
+
+   if (v6flag) {
+   m.msg_name = 
+   m.msg_namelen = sizeof(peer6);
+   } else {
+   m.msg_name = 
+   m.msg_namelen = sizeof(peer4);
+   }
+   memset(, 0, sizeof(iov));
+   iov[0].iov_base = (caddr_t)packet;
+   iov[0].iov_len = packlen;
+
+   m.msg_iov = iov;
+   m.msg_iovlen = 1;
+   m.msg_control = (caddr_t)
+   m.msg_controllen = sizeof(cmsgbuf.buf);
+
+   cc = recvmsg(s, , 0);
+   if (cc == -1 && errno != EINTR)
+   break;
+   }
+   memset(, 0, sizeof(itimer));
+   (void)setitimer(ITIMER_REAL, , NULL);
+
while (preload--)   /* Fire off them quickies. */
pinger(s);
 


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



Re: ping(8) put hop_limit warning in verbose mode

2020-10-20 Thread Florian Obser
On Tue, Oct 20, 2020 at 04:07:41PM +0200, Martijn van Duren wrote:
> When running:
> /usr/local/libexec/nagios/check_ping -vv -6 -H  -c 500,75% -w 250,50%
> in a tight loop at some point I get the following output:
> 
> CMD: /sbin/ping6 -n -c 5 
> Output: PING  (): 56 data bytes
> Output: 64 bytes from : icmp_seq=0 hlim=57 time=1.724 ms
> Output: 64 bytes from : icmp_seq=1 hlim=57 time=1.612 ms
> Output: 64 bytes from : icmp_seq=2 hlim=57 time=1.785 ms
> Output: 64 bytes from : icmp_seq=3 hlim=57 time=1.762 ms
> Output: 64 bytes from : icmp_seq=4 hlim=57 time=1.708 ms
> Output:
> Output: ---  ping statistics ---
> Output: 5 packets transmitted, 5 packets received, 0.0% packet loss
> Output: round-trip min/avg/max/std-dev = 1.612/1.718/1.785/0.060 ms
> Got stderr: ping6: failed to get receiving hop limit
> PING WARNING - System call sent warnings to stderr Packet loss = 0%, RTA = 
> 1.72 ms|rta=1.718000ms;3000.00;5000.00;0.00 pl=0%;80;100;0
> 3000.00:80% 5000.00:100%
> 
> So I have 5 echo requests, 5 echo replies and one unrelated packet not
> related to my ping command. Yet this notice to stderr forces check_ping
> into a warning state.
> 

Oh, got it know. The problem is packets arriving between socket(2) and
whenever we are finished calling all setsockopt(2)s. In this case
IPV6_RECVHOPLIMIT.

We need to drain the socket after we are fully setup.

Can you give this a spin? OK?

diff --git ping.c ping.c
index d198d233921..acfe2ed8e18 100644
--- ping.c
+++ ping.c
@@ -240,6 +240,17 @@ voidpr_retip6(struct ip6_hdr *, 
u_char *);
 int
 main(int argc, char *argv[])
 {
+   struct msghdr   m;
+   union {
+   struct cmsghdr hdr;
+   u_char buf[CMSG_SPACE(1024)];
+   }   cmsgbuf;
+   struct ioveciov[1];
+   struct pollfd   pfd;
+   struct sockaddr_in  peer4;
+   struct sockaddr_in6 peer6;
+   ssize_t cc;
+
struct addrinfo hints, *res;
struct itimerval itimer;
struct sockaddr *from, *dst;
@@ -786,6 +797,44 @@ main(int argc, char *argv[])
smsghdr.msg_iov = 
smsghdr.msg_iovlen = 1;
 
+   if (v6flag) {
+   m.msg_name = 
+   m.msg_namelen = sizeof(peer6);
+   } else {
+   m.msg_name = 
+   m.msg_namelen = sizeof(peer4);
+   }
+   memset(, 0, sizeof(iov));
+   iov[0].iov_base = (caddr_t)packet;
+   iov[0].iov_len = packlen;
+
+   /* Drain our socket. */
+   (void)signal(SIGALRM, onsignal);
+   memset(, 0, sizeof(itimer));
+   itimer.it_value.tv_sec = 1; /* make sure we don't get stuck */
+   (void)setitimer(ITIMER_REAL, , NULL);
+   for (;;) {
+   if (seenalrm)
+   break;
+
+   pfd.fd = s;
+   pfd.events = POLLIN;
+
+   if (poll(, 1, 0) <= 0)
+   break;
+
+   m.msg_iov = iov;
+   m.msg_iovlen = 1;
+   m.msg_control = (caddr_t)
+   m.msg_controllen = sizeof(cmsgbuf.buf);
+
+   cc = recvmsg(s, , 0);
+   if (cc == -1 && errno != EINTR)
+   break;
+   }
+   memset(, 0, sizeof(itimer));
+   (void)setitimer(ITIMER_REAL, , NULL);
+
while (preload--)   /* Fire off them quickies. */
pinger(s);
 
@@ -805,17 +854,7 @@ main(int argc, char *argv[])
seeninfo = 0;
 
for (;;) {
-   struct msghdr   m;
-   union {
-   struct cmsghdr hdr;
-   u_char buf[CMSG_SPACE(1024)];
-   }   cmsgbuf;
-   struct ioveciov[1];
-   struct pollfd   pfd;
-   struct sockaddr_in  peer4;
-   struct sockaddr_in6 peer6;
-   ssize_t cc;
-   int timeout;
+   int timeout;
 
/* signal handling */
if (seenint)
@@ -864,16 +903,6 @@ main(int argc, char *argv[])
if (poll(, 1, timeout) <= 0)
continue;
 
-   if (v6flag) {
-   m.msg_name = 
-   m.msg_namelen = sizeof(peer6);
-   } else {
-   m.msg_name = 
-   m.msg_namelen = sizeof(peer4);
-   }
-   memset(, 0, sizeof(iov));
-   iov[0].iov_base = (caddr_t)packet;
-   iov[0].iov_len = packlen;
m.msg_iov = iov;
m.msg_iovlen = 1;
m.msg_control = (caddr_t)


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



Re: ping(8) put hop_limit warning in verbose mode

2020-10-20 Thread Florian Obser
On Tue, Oct 20, 2020 at 03:46:19PM +0200, Martijn van Duren wrote:
> On Tue, 2020-10-20 at 15:19 +0200, Florian Obser wrote:
> > On Tue, Oct 20, 2020 at 09:20:32AM +0200, Martijn van Duren wrote:
> > > I have an icinga-instance running on openbsd.amsterdam. Here I found
> > > that sometimes check_ping from the monitoring-plugins package fails,
> > > because ping(8) sends "failed to get receiving hop limit", but still
> > > receives all the ping replies. These packets/annomalies are clearly
> > > not meant for us.
> > 
> > But it is. This is comming from the local machine,
> > see L949ff of netinet6/ip6_input.c.
> 
> What I meant to say is meant for ping(8). All the icmp echo replies
> still arrive and are accounted for.
> 

I don't understand.

I patched the kernel to make sbcreatecontrol() fail 50% of the time:

$ ping6 -qc10 fe80::5667:51ff:fede:e7ce%vio0
PING fe80::5667:51ff:fede:e7ce%vio0 (fe80::5667:51ff:fede:e7ce%vio0): 56 data 
bytes
ping6: failed to get receiving hop limit
ping6: failed to get receiving hop limit
ping6: failed to get receiving hop limit
ping6: failed to get receiving hop limit

--- fe80::5667:51ff:fede:e7ce%vio0 ping statistics ---
10 packets transmitted, 6 packets received, 40.0% packet loss
round-trip min/avg/max/std-dev = 0.980/1.196/1.955/0.342 ms

I assure you, I do not have 40% packet loss towards my gateway.
The packets are not accounted for.

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



Re: ping(8) put hop_limit warning in verbose mode

2020-10-20 Thread Florian Obser
On Tue, Oct 20, 2020 at 09:20:32AM +0200, Martijn van Duren wrote:
> I have an icinga-instance running on openbsd.amsterdam. Here I found
> that sometimes check_ping from the monitoring-plugins package fails,
> because ping(8) sends "failed to get receiving hop limit", but still
> receives all the ping replies. These packets/annomalies are clearly
> not meant for us.

But it is. This is comming from the local machine,
see L949ff of netinet6/ip6_input.c.
sbcreatecontrol() fails because it tries to allocate space with
M_DONTWAIT. Looks like you drove your system out of resources.

While the local system receives all replies and passes them on to
ping6(8) they are not counted as received. So ping6(8) will report
packet loss while there is none.
I think it's a mistake to hide the warning because it will lead people
on a wild goose chase.
Also since ping6(8) will report packet loss I'm not conviced this will
solve your problem.

> 
> Since this check is done before the ICMP6_ECHO_REPLY check and our
> manpage states:
> -v  Verbose output.  ICMP packets other than ECHO_REPLY that are
> received are listed.
> I think the following diff is warranted and solves my false warning
> states.
> 
> OK?
> 
> martijn@
> 
> Index: ping.c
> ===
> RCS file: /cvs/src/sbin/ping/ping.c,v
> retrieving revision 1.240
> diff -u -p -r1.240 ping.c
> --- ping.c11 Feb 2020 18:41:39 -  1.240
> +++ ping.c20 Oct 2020 07:19:49 -
> @@ -1180,7 +1180,8 @@ pr_pack(u_char *buf, int cc, struct msgh
>   icp6 = (struct icmp6_hdr *)buf;
>  
>   if ((hoplim = get_hoplim(mhdr)) == -1) {
> - warnx("failed to get receiving hop limit");
> + if (options & F_VERBOSE)
> + warnx("failed to get receiving hop limit");
>   return;
>   }
>  
> 

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



Re: acme-client: improve account creation error message

2020-09-15 Thread Florian Obser
On Mon, Sep 14, 2020 at 04:26:20PM -0500, Rafael Possamai wrote:
> >please dont drop the all buffer , or keep it with -vv ?
> >example : warnx("%s: bad JSON object:%s", p->newaccount, c->buf.buf);
> >
> >i don't want to ktrace it to see why the new certbot version is not working
> 
> Yeah, I think it's good to have the choice to inspect the vomit, maybe you 
> stumble upon some useful nuggets for troubleshooting an obscure issue.
> 
> I believe the JSON output is valid, just not pretty. Maybe we could add 
> syntax highlighting? (just kidding)
> 

I don't understand what you people are going on about, I didn't touch
the -v output. You can still roll around in json.

It's not about if it's valid json or not, it's untrusted input that we
spit on your console when used with -v.

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



Re: acme-client(1) and Buypass Go SSL

2020-09-14 Thread Florian Obser


This fell through the cracks back in April.

We need to be able to provide contact information to use the
buypass.com acme api.

OK?

diff --git etc/examples/acme-client.conf etc/examples/acme-client.conf
index 32ecd8e8655..40d231725ac 100644
--- etc/examples/acme-client.conf
+++ etc/examples/acme-client.conf
@@ -11,6 +11,18 @@ authority letsencrypt-staging {
account key "/etc/acme/letsencrypt-staging-privkey.pem"
 }
 
+authority buypass {
+api url "https://api.buypass.com/acme/directory;
+account key "/etc/acme/buypass-privkey.pem"
+contact "mailto:m...@example.com;
+}
+
+authority buypass-test {
+api url "https://api.test4.buypass.no/acme/directory;
+account key "/etc/acme/buypass-test-privkey.pem"
+contact "mailto:m...@example.com;
+}
+
 domain example.com {
alternative names { secure.example.com }
domain key "/etc/ssl/private/example.com.key"
diff --git usr.sbin/acme-client/acme-client.conf.5 
usr.sbin/acme-client/acme-client.conf.5
index 08a47a76ab7..41994d13676 100644
--- usr.sbin/acme-client/acme-client.conf.5
+++ usr.sbin/acme-client/acme-client.conf.5
@@ -98,6 +98,11 @@ It defaults to
 Specify the
 .Ar url
 under which the ACME API is reachable.
+.It Ic contact Ar contact
+Optional
+.Ar contact
+URLs that the authority can use to contact the client for issues related to
+this account.
 .El
 .Sh DOMAINS
 The certificates to be obtained through ACME.
diff --git usr.sbin/acme-client/extern.h usr.sbin/acme-client/extern.h
index 364425b0500..ee341e0950f 100644
--- usr.sbin/acme-client/extern.h
+++ usr.sbin/acme-client/extern.h
@@ -263,7 +263,7 @@ char*json_getstr(struct jsmnn *, const char 
*);
 
 char   *json_fmt_newcert(const char *);
 char   *json_fmt_chkacc(void);
-char   *json_fmt_newacc(void);
+char   *json_fmt_newacc(const char *);
 char   *json_fmt_neworder(const char *const *, size_t);
 char   *json_fmt_protected_rsa(const char *,
const char *, const char *, const char *);
diff --git usr.sbin/acme-client/json.c usr.sbin/acme-client/json.c
index a6762eeb258..9201f8d2fc3 100644
--- usr.sbin/acme-client/json.c
+++ usr.sbin/acme-client/json.c
@@ -618,14 +618,24 @@ json_fmt_chkacc(void)
  * Format the "newAccount" resource request.
  */
 char *
-json_fmt_newacc(void)
+json_fmt_newacc(const char *contact)
 {
int  c;
-   char*p;
+   char*p, *cnt = NULL;
+
+   if (contact != NULL) {
+   c = asprintf(, "\"contact\": [ \"%s\" ], ", contact);
+   if (c == -1) {
+   warn("asprintf");
+   return NULL;
+   }
+   }
 
c = asprintf(, "{"
+   "%s"
"\"termsOfServiceAgreed\": true"
-   "}");
+   "}", cnt == NULL ? "" : cnt);
+   free(cnt);
if (c == -1) {
warn("asprintf");
p = NULL;
diff --git usr.sbin/acme-client/netproc.c usr.sbin/acme-client/netproc.c
index 05e36897c38..4490450003e 100644
--- usr.sbin/acme-client/netproc.c
+++ usr.sbin/acme-client/netproc.c
@@ -369,14 +369,14 @@ sreq(struct conn *c, const char *addr, int kid, const 
char *req, char **loc)
  * Returns non-zero on success.
  */
 static int
-donewacc(struct conn *c, const struct capaths *p)
+donewacc(struct conn *c, const struct capaths *p, const char *contact)
 {
struct jsmnn*j = NULL;
int  rc = 0;
char*req, *detail, *error = NULL;
long lc;
 
-   if ((req = json_fmt_newacc()) == NULL)
+   if ((req = json_fmt_newacc(contact)) == NULL)
warnx("json_fmt_newacc");
else if ((lc = sreq(c, p->newaccount, 0, req, >kid)) < 0)
warnx("%s: bad comm", p->newaccount);
@@ -410,7 +410,7 @@ donewacc(struct conn *c, const struct capaths *p)
  * Returns non-zero on success.
  */
 static int
-dochkacc(struct conn *c, const struct capaths *p)
+dochkacc(struct conn *c, const struct capaths *p, const char *contact)
 {
int  rc = 0;
char*req;
@@ -425,7 +425,7 @@ dochkacc(struct conn *c, const struct capaths *p)
else if (c->buf.buf == NULL || c->buf.sz == 0)
warnx("%s: empty response", p->newaccount);
else if (lc == 400)
-   rc = donewacc(c, p);
+   rc = donewacc(c, p, contact);
else
rc = 1;
 
@@ -755,7 +755,7 @@ netproc(int kfd, int afd, int Cfd, int cfd, int dfd, int 
rfd,
c.newnonce = paths.newnonce;
 
/* Check if our account already exists or create it. */
-   if (!dochkacc(, ))
+   if (!dochkacc(, , authority->contact))
goto out;
 
/*
diff --git usr.sbin/acme-client/parse.h usr.sbin/acme-client/parse.h
index 9de5a490f69..c928a9de7da 100644
--- usr.sbin/acme-client/parse.h
+++ usr.sbin/acme-client/parse.h
@@ -38,6 +38,7 @@ struct authority_c {

acme-client: relax certificate parsing

2020-09-14 Thread Florian Obser
Relax parsing of pem files a bit. Apparently there are CAs that use
\r\n line endings.
>From Bartosz Kuzma as part of a larger diff.

OK?

diff --git certproc.c certproc.c
index 7fde96e970e..975e12afaaa 100644
--- certproc.c
+++ certproc.c
@@ -28,7 +28,8 @@
 
 #include "extern.h"
 
-#define MARKER "-END CERTIFICATE-\n"
+#define BEGIN_MARKER "-BEGIN CERTIFICATE-"
+#define END_MARKER "-END CERTIFICATE-"
 
 int
 certproc(int netsock, int filesock)
@@ -81,19 +82,25 @@ certproc(int netsock, int filesock)
if ((csr = readbuf(netsock, COMM_CSR, )) == NULL)
goto out;
 
-   if (csrsz < strlen(MARKER)) {
+   if (csrsz < strlen(END_MARKER)) {
warnx("invalid cert");
goto out;
}
 
-   chaincp = strstr(csr, MARKER);
+   chaincp = strstr(csr, END_MARKER);
 
if (chaincp == NULL) {
warnx("invalid cert");
goto out;
}
 
-   chaincp += strlen(MARKER);
+   chaincp += strlen(END_MARKER);
+
+   if ((chaincp = strstr(chaincp, BEGIN_MARKER)) == NULL) {
+   warnx("invalid certificate chain");
+   goto out;
+   }
+
if ((chain = strdup(chaincp)) == NULL) {
warn("strdup");
goto out;


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



acme-client: improve account creation error message

2020-09-14 Thread Florian Obser
not helpful:
$ doas acme-client $(hostname)
acme-client: https://api.test4.buypass.no/acme-v02/new-acct: bad HTTP: 400

vomitting unformated json is not better:
$ doas acme-client -v $(hostname)
acme-client: transfer buffer: 
[{"type":"urn:ietf:params:acme:error:malformed","detail":"Email is a required 
contact","code":400,"message":"MALFORMED_BAD_REQUEST","details":"HTTP 400 Bad 
Request"}] (164 bytes)

let's do this:
$ doas obj/acme-client -v $(hostname)
acme-client: Email is a required contact

OK?

diff --git extern.h extern.h
index 529d3350205..364425b0500 100644
--- extern.h
+++ extern.h
@@ -259,6 +259,7 @@ int  json_parse_order(struct jsmnn *, struct order 
*);
 int json_parse_upd_order(struct jsmnn *, struct order *);
 voidjson_free_capaths(struct capaths *);
 int json_parse_capaths(struct jsmnn *, struct capaths *);
+char   *json_getstr(struct jsmnn *, const char *);
 
 char   *json_fmt_newcert(const char *);
 char   *json_fmt_chkacc(void);
diff --git json.c json.c
index 61d2631359f..a6762eeb258 100644
--- json.c
+++ json.c
@@ -297,7 +297,7 @@ json_getobj(struct jsmnn *n, const char *name)
  * that it's the correct type.
  * Returns NULL on failure.
  */
-static char *
+char *
 json_getstr(struct jsmnn *n, const char *name)
 {
size_t   i;
diff --git netproc.c netproc.c
index 7b8152196d1..05e36897c38 100644
--- netproc.c
+++ netproc.c
@@ -371,15 +371,27 @@ sreq(struct conn *c, const char *addr, int kid, const 
char *req, char **loc)
 static int
 donewacc(struct conn *c, const struct capaths *p)
 {
+   struct jsmnn*j = NULL;
int  rc = 0;
-   char*req;
+   char*req, *detail, *error = NULL;
long lc;
 
if ((req = json_fmt_newacc()) == NULL)
warnx("json_fmt_newacc");
else if ((lc = sreq(c, p->newaccount, 0, req, >kid)) < 0)
warnx("%s: bad comm", p->newaccount);
-   else if (lc != 200 && lc != 201)
+   else if (lc == 400) {
+   if ((j = json_parse(c->buf.buf, c->buf.sz)) == NULL)
+   warnx("%s: bad JSON object", p->newaccount);
+   else {
+   detail = json_getstr(j, "detail");
+   if (detail != NULL && stravis(, detail, VIS_SAFE)
+   != -1) {
+   warnx("%s", error);
+   free(error);
+   }
+   }
+   } else if (lc != 200 && lc != 201)
warnx("%s: bad HTTP: %ld", p->newaccount, lc);
else if (c->buf.buf == NULL || c->buf.sz == 0)
warnx("%s: empty response", p->newaccount);


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



Re: unwind(8): forget learned forwarders when interface disappears

2020-09-12 Thread Florian Obser
On Sat, Sep 12, 2020 at 08:13:43PM +0300, Peter Pentchev wrote:
> On Sat, Sep 12, 2020 at 06:58:30PM +0200, Florian Obser wrote:
> > On Sat, Sep 12, 2020 at 05:15:08PM +0200, Klemens Nanni wrote:
> > > On Sat, Sep 12, 2020 at 05:11:00PM +0200, Klemens Nanni wrote:
> > > > Bit hard to read, what about aligning like this?
> > > > 
> > > > +   if ((rdns_proposal->src == 0 ||
> > > > +rdns_proposal->src == tmp->src) &&
> > > > +   (rdns_proposal->if_index == 0 ||
> > > > +   rdns_proposal->if_index == tmp->if_index))
> > > With a space here   ^
> > > 
> > 
> > I'd rather not, style(9):
> > Indentation is an 8 character tab.  Second level indents are four 
> > spaces.
> > 
> > Also, my editor would fix it up the next time I touch those lines.
> 
> Er... so what's the difference with the second line, two lines above
> that? "rdns_proposal->src == tmp->src" seems to have that space.

sure, because kn added it, have a look at the original diff, it's
formated completely different.

> 
> G'luck,
> Peter
> 
> -- 
> Peter Pentchev  r...@ringlet.net r...@debian.org p...@storpool.com
> PGP key:http://people.FreeBSD.org/~roam/roam.key.asc
> Key fingerprint 2EE7 A7A5 17FC 124C F115  C354 651E EFB0 2527 DF13



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



Re: trunk: keep interface up on port removal

2020-09-12 Thread Florian Obser
Seems reasonable. OK

On Sat, Sep 12, 2020 at 05:49:52PM +0200, Klemens Nanni wrote:
> Unconfiguring a member interface from trunk(4) or simply destroying the
> trunk pulls the member down for no reason, both comment and code are
> there since import, but I see no justification for doing so.
> 
> aggr(4) does not pull its member down upon removal either.
> 
> I came across this after
> 
>   $ doas ifconfig trunk0 destroy
>   $ doas sh /etc/netstart trunk0
> 
> yielded no network and I had to manually pull up members.
> 
> Feedback? OK?
> 
> 
> Index: if_trunk.c
> ===
> RCS file: /cvs/src/sys/net/if_trunk.c,v
> retrieving revision 1.149
> diff -u -p -r1.149 if_trunk.c
> --- if_trunk.c28 Jul 2020 09:52:32 -  1.149
> +++ if_trunk.c12 Sep 2020 15:41:14 -
> @@ -423,10 +423,6 @@ trunk_port_destroy(struct trunk_port *tp
>   /* Remove multicast addresses from this port */
>   trunk_ether_cmdmulti(tp, SIOCDELMULTI);
>  
> - /* Port has to be down */
> - if (ifp->if_flags & IFF_UP)
> - if_down(ifp);
> -
>   ifpromisc(ifp, 0);
>  
>   if (tr->tr_port_destroy != NULL)
> 

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



Re: unwind(8): forget learned forwarders when interface disappears

2020-09-12 Thread Florian Obser
On Sat, Sep 12, 2020 at 05:15:08PM +0200, Klemens Nanni wrote:
> On Sat, Sep 12, 2020 at 05:11:00PM +0200, Klemens Nanni wrote:
> > Bit hard to read, what about aligning like this?
> > 
> > +   if ((rdns_proposal->src == 0 ||
> > +rdns_proposal->src == tmp->src) &&
> > +   (rdns_proposal->if_index == 0 ||
> > +   rdns_proposal->if_index == tmp->if_index))
> With a space here   ^
> 

I'd rather not, style(9):
Indentation is an 8 character tab.  Second level indents are four 
spaces.

Also, my editor would fix it up the next time I touch those lines.

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



unwind(8): forget learned forwarders when interface disappears

2020-09-12 Thread Florian Obser
... say if you pull a usb stick.

OK?

diff --git frontend.c frontend.c
index 87141f81e8c..b92cde8226c 100644
--- frontend.c
+++ frontend.c
@@ -913,8 +913,21 @@ handle_route_message(struct rt_msghdr *rtm, struct 
sockaddr **rti_info)
 {
struct imsg_rdns_proposalrdns_proposal;
struct sockaddr_rtdns   *rtdns;
+   struct if_announcemsghdr*ifan;
 
switch (rtm->rtm_type) {
+   case RTM_IFANNOUNCE:
+   ifan = (struct if_announcemsghdr *)rtm;
+   if (ifan->ifan_what == IFAN_ARRIVAL)
+   break;
+   rdns_proposal.if_index = ifan->ifan_index;
+   rdns_proposal.src = 0;
+   rdns_proposal.rtdns.sr_family = AF_INET;
+   rdns_proposal.rtdns.sr_len = offsetof(struct sockaddr_rtdns,
+   sr_dns);
+   frontend_imsg_compose_resolver(IMSG_REPLACE_DNS, 0,
+   _proposal, sizeof(rdns_proposal));
+   break;
case RTM_IFINFO:
frontend_imsg_compose_resolver(IMSG_NETWORK_CHANGED, 0, NULL,
0);
diff --git resolver.c resolver.c
index 874ad5e76b3..8cf72db7250 100644
--- resolver.c
+++ resolver.c
@@ -1952,10 +1952,13 @@ replace_autoconf_forwarders(struct imsg_rdns_proposal 
*rdns_proposal)
}
 
TAILQ_FOREACH(tmp, _forwarder_list, entry) {
-   /* if_index of zero signals to clear all proposals */
-   if (rdns_proposal->src == tmp->src &&
-   (rdns_proposal->if_index == 0 || rdns_proposal->if_index ==
-   tmp->if_index))
+   /*
+* if_index of zero signals to clear all proposals
+* src of zero signals interface gone
+*/
+   if ((rdns_proposal->src == 0 || rdns_proposal->src ==
+   tmp->src) && (rdns_proposal->if_index == 0 ||
+   rdns_proposal->if_index == tmp->if_index))
continue;
if ((uw_forwarder = calloc(1, sizeof(struct uw_forwarder))) ==
NULL)
diff --git unwind.c unwind.c
index b17bf7e413c..d7ae76d4274 100644
--- unwind.c
+++ unwind.c
@@ -266,7 +266,8 @@ main(int argc, char *argv[])
AF_INET)) == -1)
fatal("route socket");
 
-   rtfilter = ROUTE_FILTER(RTM_IFINFO) | ROUTE_FILTER(RTM_PROPOSAL);
+   rtfilter = ROUTE_FILTER(RTM_IFINFO) | ROUTE_FILTER(RTM_PROPOSAL)
+   | ROUTE_FILTER(RTM_IFANNOUNCE);
if (setsockopt(frontend_routesock, AF_ROUTE, ROUTE_MSGFILTER,
, sizeof(rtfilter)) == -1)
fatal("setsockopt(ROUTE_MSGFILTER)");


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



Re: undwind(8): request for data

2020-09-12 Thread Florian Obser
On Thu, Sep 03, 2020 at 06:13:41PM +0200, Florian Obser wrote:
> The point of this excercise is to work out if it's worthwhile to
> implement RFC 8806 "Running a Root Server Local to a Resolver" in
> unwind(8).
> 
> We are trading latency for bandwidth. Lower latency is almost always a
> win for unwind(8) usecases. But the work if we fetch the root zone is
> not trivial either.
> 
> The zone usually gets updated twice a day and is about 1.2MB. Due to the
> timing parameters unwind(8) would check every 30 minutes with a SOA
> query if a new zone is available:
> $ dig @k.root-servers.net +multiline +noall +answer . soa
> . 86400 IN SOA a.root-servers.net. 
> nstld.verisign-grs.com. (
>   2020090300 ; serial
>   1800   ; refresh (30 minutes)
>   900; retry (15 minutes)
>   604800 ; expire (1 week)
>   86400  ; minimum (1 day)
>   )
> 
> The code complexity should be managable, all the bits and pieces are
> there in libunbound.

Curiously nobody reported unwind(8) doing tcp queries.

The histogram for udp looks like this:

050 3
   50   100 3
  100   150 1
  150   200 1
  200   250 1
  300   350 1
  700   750 1
 1000  1050 1
 1700  1750 1
 1800  1850 1
 2050  2100 1
 5200  5250 1
 9900  9950 1
35050 35100 1

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



undwind(8): request for data

2020-09-03 Thread Florian Obser
If you are running unwind(8) on your laptop / workstation or
unbound(8) for a small home network I'd be interested to know how
often they talk to the root zone during a typical day.

Could you please add the following table and match rules to
/etc/pf.conf:

table  const { \
198.41.0.4, 2001:503:ba3e::2:30, \
199.9.14.201, 2001:500:200::b, \
192.33.4.12, 2001:500:2::c, \
199.7.91.13, 2001:500:2d::d, \
192.203.230.10, 2001:500:a8::e, \
192.5.5.241, 2001:500:2f::f, \
192.112.36.4, 2001:500:12::d0d, \
198.97.190.53, 2001:500:1::53, \
192.36.148.17, 2001:7fe::53, \
192.58.128.30, 2001:503:c27::2:30, \
193.0.14.129, 2001:7fd::1, \
199.7.83.42, 2001:500:9f::42, \
202.12.27.33, 2001:dc3::35 }

match out proto {tcp} to  port 53 label rootdns_tcp
match out proto {udp} to  port 53 label rootdns_udp

Alternatively, this diff should apply to to a default /etc/pf.conf:

diff --git pf.conf pf.conf
index ecf2183c210..9cb8d752f6f 100644
--- pf.conf
+++ pf.conf
@@ -2,8 +2,26 @@
 #
 # See pf.conf(5) and /etc/examples/pf.conf
 
+table  const { \
+   198.41.0.4, 2001:503:ba3e::2:30, \
+   199.9.14.201, 2001:500:200::b, \
+   192.33.4.12, 2001:500:2::c, \
+   199.7.91.13, 2001:500:2d::d, \
+   192.203.230.10, 2001:500:a8::e, \
+   192.5.5.241, 2001:500:2f::f, \
+   192.112.36.4, 2001:500:12::d0d, \
+   198.97.190.53, 2001:500:1::53, \
+   192.36.148.17, 2001:7fe::53, \
+   192.58.128.30, 2001:503:c27::2:30, \
+   193.0.14.129, 2001:7fd::1, \
+   199.7.83.42, 2001:500:9f::42, \
+   202.12.27.33, 2001:dc3::35 }
+
 set skip on lo
 
+match out proto {tcp} to  port 53 label rootdns_tcp
+match out proto {udp} to  port 53 label rootdns_udp
+
 block return   # block stateless traffic
 pass   # establish keep-state
 

Then reload the ruleset and restart unwind:

# pfctl -f /etc/pf.conf
# rcctl restart unwind

You can now get stats on how often your machine talks to the root name servers:

# pfctl -s label
rootdns_tcp 2730 0 0 0 0 0 0 0
rootdns_udp 266 2 187 1 56 1 131 0

The columns are: label, evaluations, packets total, bytes total,
packets in, bytes in, packets out, bytes out, state creations

Please report the stats after a day of normal use, thanks.



The point of this excercise is to work out if it's worthwhile to
implement RFC 8806 "Running a Root Server Local to a Resolver" in
unwind(8).

We are trading latency for bandwidth. Lower latency is almost always a
win for unwind(8) usecases. But the work if we fetch the root zone is
not trivial either.

The zone usually gets updated twice a day and is about 1.2MB. Due to the
timing parameters unwind(8) would check every 30 minutes with a SOA
query if a new zone is available:
$ dig @k.root-servers.net +multiline +noall +answer . soa
.   86400 IN SOA a.root-servers.net. 
nstld.verisign-grs.com. (
2020090300 ; serial
1800   ; refresh (30 minutes)
900; retry (15 minutes)
604800 ; expire (1 week)
86400  ; minimum (1 day)
)

The code complexity should be managable, all the bits and pieces are
there in libunbound.

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



Re: unwind(8): use SO_REUSEADDR

2020-08-29 Thread Florian Obser
I can't think of a downside, OK.
(Not sure of a use case either though.)

On 29 August 2020 19:13:40 CEST, Jeremie Courreges-Anglas  
wrote:
>On Sat, Aug 29 2020, Stuart Henderson  wrote:
>> On 2020/08/27 15:28, Florian Obser wrote:
>>> all heavy lifting done by sthen in unbound
>>> 
>>> tests?
>>
>> ok with me. only tested lightly (the machine I normally use does DNS
>for
>> other machines too so runs unbound).
>>
>> related, any idea what's happening here?
>>
>> unwind[51500]: fatal in main: could not bind to 127.0.0.1 or ::1 on
>port 53: Address already in use
>>
>> unbound is listening to *:53, but shouldn't other software be able to
>> listen if bound to a specific address like 127.0.0.1:53? is this a
>bug
>> somewhere or am I just missing something about UDP?
>
>Once *:53 is bound you need SO_REUSEADDR to loosen the checks.  ktrace
>says unbound(8) uses this unconditionally.  ChangeLog entry:
>
>   - Fix #531: Set SO_REUSEADDR so that the wildcard interface and a 
> more specific interface port 53 can be used at the same time, and
> one of the daemons is unbound.
>
>I think unwind could use the same treatment.
>
>Note that using SO_REUSEADDR still prevents two unwind copies from
>binding to 127.0.0.1:53 / [::1]:53 (for that to work you'd need
>SO_REUSEPORT instead).
>
>Thoughts?  oks?
>
>
>Index: unwind.c
>===
>RCS file: /d/cvs/src/sbin/unwind/unwind.c,v
>retrieving revision 1.47
>diff -u -p -p -u -r1.47 unwind.c
>--- unwind.c   25 May 2020 16:52:15 -  1.47
>+++ unwind.c   29 Aug 2020 17:07:49 -
>@@ -726,6 +726,7 @@ open_ports(void)
> {
>   struct addrinfo  hints, *res0;
>   int  udp4sock = -1, udp6sock = -1, error;
>+  int  opt = 1;
> 
>   memset(, 0, sizeof(hints));
>   hints.ai_family = AF_INET;
>@@ -736,6 +737,9 @@ open_ports(void)
>   if (!error && res0) {
>   if ((udp4sock = socket(res0->ai_family, res0->ai_socktype,
>   res0->ai_protocol)) != -1) {
>+  if (setsockopt(udp4sock, SOL_SOCKET, SO_REUSEADDR,
>+  , sizeof(opt)) == -1)
>+  log_warn("setting SO_REUSEADDR on socket");
>   if (bind(udp4sock, res0->ai_addr, res0->ai_addrlen)
>   == -1) {
>   close(udp4sock);
>@@ -751,6 +755,9 @@ open_ports(void)
>   if (!error && res0) {
>   if ((udp6sock = socket(res0->ai_family, res0->ai_socktype,
>   res0->ai_protocol)) != -1) {
>+  if (setsockopt(udp6sock, SOL_SOCKET, SO_REUSEADDR,
>+  , sizeof(opt)) == -1)
>+  log_warn("setting SO_REUSEADDR on socket");
>   if (bind(udp6sock, res0->ai_addr, res0->ai_addrlen)
>   == -1) {
>   close(udp6sock);

-- 
Sent from a mobile device. Please excuse poor formating.



Re: Refine IPv6 source address selection

2020-08-24 Thread Florian Obser
On Mon, Aug 24, 2020 at 07:06:26PM +0200, Denis Fondras wrote:
> On Mon, Aug 24, 2020 at 06:42:02PM +0200, Florian Obser wrote:
> > To clarify, this is independent of my recent work in
> > in6_ifawithscope(), -ifa did not work with the old code, either.
> > 
> 
> Of course ! Sorry if my message led to think you were responsible for it.
> 

No, not me :)
This was for the people on tech@ who were not part of our private
discussion.

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



Re: Refine IPv6 source address selection

2020-08-24 Thread Florian Obser
On Mon, Aug 24, 2020 at 05:34:09PM +0200, Denis Fondras wrote:
> While working on source selection, I noticed the IPv6 source was not honored
> when set from route(8) with -ifa.
> 
> After discussing with florian@, here is a proposed change. It chooses the 
> source
> address associated with the route (hence honoring -ifa) instead of the first
> address of the output interface which becomes the source address of last
> resort.

To clarify, this is independent of my recent work in
in6_ifawithscope(), -ifa did not work with the old code, either.

Turns out this is not correct. I seems to always bypass
in6_ifawithscope(), so on a system with slaac where default is on a
link local address this will pick a link local address as the source.
That's not going to work.

> 
> Index: netinet6/in6_src.c
> ===
> RCS file: /cvs/src/sys/netinet6/in6_src.c,v
> retrieving revision 1.81
> diff -u -p -r1.81 in6_src.c
> --- netinet6/in6_src.c2 Dec 2016 11:16:04 -   1.81
> +++ netinet6/in6_src.c24 Aug 2020 15:14:53 -
> @@ -207,13 +207,14 @@ in6_pcbselsrc(struct in6_addr **in6src, 
>*/
>  
>   if (ro->ro_rt) {
> - ifp = if_get(ro->ro_rt->rt_ifidx);
> - if (ifp != NULL) {
> - ia6 = in6_ifawithscope(ifp, dst, rtableid);
> - if_put(ifp);
> + ia6 = ifatoia6(ro->ro_rt->rt_ifa);
> + if (ia6 == NULL) {
> + ifp = if_get(ro->ro_rt->rt_ifidx);
> + if (ifp != NULL) {
> + ia6 = in6_ifawithscope(ifp, dst, rtableid);
> + if_put(ifp);
> + }
>   }
> - if (ia6 == NULL) /* xxx scope error ?*/
> - ia6 = ifatoia6(ro->ro_rt->rt_ifa);
>   }
>   if (ia6 == NULL)
>   return (EHOSTUNREACH);  /* no route */
> 

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



unbound(8): disable explicit port randomisation

2020-08-24 Thread Florian Obser
With the update sthen@ just put in we can enable this:

  --disable-explicit-port-randomisation
  disable explicit source port randomisation and rely
  on the kernel to provide random source ports

OK?

diff --git Makefile.bsd-wrapper Makefile.bsd-wrapper
index ff9bc927592..c4abf8dbb97 100644
--- Makefile.bsd-wrapper
+++ Makefile.bsd-wrapper
@@ -17,7 +17,8 @@ CONFIGURE_OPTS_UNBOUND=   --enable-allsymbols \
--with-rootkey-file=/var/unbound/db/root.key \
--with-conf-file=${CHROOTDIR}/etc/unbound.conf \
--with-username=_unbound \
-   --disable-shared
+   --disable-shared \
+   --disable-explicit-port-randomisation
 
 # do not remove, breaks unwind(8)
 CONFIGURE_OPTS_UNBOUND+= --without-pthreads



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



Re: openrsync(1): add support for IPv6-only hosts

2020-08-19 Thread Florian Obser
On Wed, Aug 19, 2020 at 12:20:19AM +0200, Klemens Nanni wrote:
> On Tue, Aug 18, 2020 at 09:58:56AM +0200, Sasha Romijn wrote:
> > The current openrsync client is not able to connect to dual-stack remote 
> > hosts, when the local host does not have any IPv4 connectivity. This is 
> > because connect() fails with EADDRNOTAVAIL when trying to connect to the 
> > remote IPv4 address on an IPv6-only local host - and IPv4 seems to be 
> > attempted first. The current client does not try other remote host 
> > addresses after EADDRNOTAVAIL, and therefore never tries the remote IPv6 
> > address.
> For openrsync(1) this won't happen with `family inet6 inet4' in
> resolv.conf(5).
> 
> > The included patch allows the client to continue after EADDRNOTAVAIL, 
> > making it to try other addresses, until it reaches a working IPv6 address. 
> > If the local host is IPv6-only and the remote host IPv4-only, the 
> > connection still fails with an error produced from rsync_connect().
> Your diff reads correct to me, thanks; regular rsync(1) connects fine
> regardless of `family'.
> 
> > Perhaps a warning should be issued for the EADDRNOTAVAIL case, like it does 
> > for EHOSTUNREACH - but I thought it would be a bit much, as an IPv6-only 
> > host would then emit warnings on pretty much every single use of the 
> > openrsync client.
> Yes, I don't think a warning is warrented here - afterall it can connect
> without problems.
> 
> OK to commit anyone?
> 

Sorry for being late to the party.
Should we maybe inline inet_connect into rsync_connect and ignore all
connect(3) errors and try the host?
That's what ssh is doing in ssh_connect_direct()

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



slaacd(8): use correct source link-layer address

2020-08-18 Thread Florian Obser
When sending a router solicitation use the link-layer (mac) address of
the outgoing interface in the source link-layer address ICMPv6 option
instead of the address of the last configured autoconf interface.

It is not the most efficient way to first transform an if_index into
and interface name and then iterate over all addresses but this is
also not in the hot path. Under normal operations slaacd will send
one solicitation when an interface is set to autoconf and then
never again because it will see unsolicitated router advertisements
before addresses expire.

Tests, OKs?

diff --git frontend.c frontend.c
index 2adbdb0f97a..4b94af5ec8c 100644
--- frontend.c
+++ frontend.c
@@ -520,9 +520,6 @@ update_iface(uint32_t if_index, char* if_name)
imsg_ifinfo.soii = !(xflags & IFXF_INET6_NOSOII);
get_lladdr(if_name, _ifinfo.hw_address, _ifinfo.ll_address);
 
-   memcpy(_opt_source_link_addr, _ifinfo.hw_address,
-   sizeof(nd_opt_source_link_addr));
-
frontend_imsg_compose_main(IMSG_UPDATE_IF, 0, _ifinfo,
sizeof(imsg_ifinfo));
 }
@@ -1023,9 +1020,20 @@ send_solicitation(uint32_t if_index)
 {
struct in6_pktinfo  *pi;
struct cmsghdr  *cm;
+   struct ether_addrhw_address;
+   struct sockaddr_in6  ll_address;
+   char if_name[IF_NAMESIZE];
 
log_debug("%s(%u)", __func__, if_index);
 
+   if (if_indextoname(if_index, if_name) == NULL)
+   return;
+
+   get_lladdr(if_name, _address, _address);
+
+   memcpy(_opt_source_link_addr, _address,
+   sizeof(nd_opt_source_link_addr));
+
dst.sin6_scope_id = if_index;
 
cm = CMSG_FIRSTHDR();


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



Re: allow TCP connections to IPv6 anycast addresses

2020-08-08 Thread Florian Obser
On Fri, Aug 07, 2020 at 11:52:46PM +0200, Jeremie Courreges-Anglas wrote:
> If you don't want to remove M_ACAST from sys/mbuf.h, can you please at
> least change the comment?  /* obsolete */ or something.

Good point, I forgot to ask about what to do with the flag.
I think we can remove it, from what I understand %b in printf(9) works
fine with a sparse decoding string.

It compiles but I have no idea how to test it in ddb.

OK? Better to leave out the comment?

diff --git sys/mbuf.h sys/mbuf.h
index d52896d3be8..3ddd1b89d66 100644
--- sys/mbuf.h
+++ sys/mbuf.h
@@ -190,7 +190,7 @@ struct mbuf {
 /* mbuf pkthdr flags, also in m_flags */
 #define M_VLANTAG  0x0020  /* ether_vtag is valid */
 #define M_LOOP 0x0040  /* packet has been sent from local machine */
-#define M_ACAST0x0080  /* received as IPv6 anycast */
+   /* 0x0080 used to be M_ACAST */
 #define M_BCAST0x0100  /* sent/received as link-level 
broadcast */
 #define M_MCAST0x0200  /* sent/received as link-level 
multicast */
 #define M_CONF 0x0400  /* payload was encrypted (ESP-transport) */
@@ -203,14 +203,13 @@ struct mbuf {
 #ifdef _KERNEL
 #define M_BITS \
 ("\20\1M_EXT\2M_PKTHDR\3M_EOR\4M_EXTWR\5M_PROTO1\6M_VLANTAG\7M_LOOP" \
-"\10M_ACAST\11M_BCAST\12M_MCAST\13M_CONF\14M_AUTH\15M_TUNNEL" \
+"\11M_BCAST\12M_MCAST\13M_CONF\14M_AUTH\15M_TUNNEL" \
 "\16M_ZEROIZE\17M_COMP\20M_LINK0")
 #endif
 
 /* flags copied when copying m_pkthdr */
 #defineM_COPYFLAGS 
(M_PKTHDR|M_EOR|M_PROTO1|M_BCAST|M_MCAST|M_CONF|M_COMP|\
-M_AUTH|M_LOOP|M_TUNNEL|M_LINK0|M_VLANTAG|M_ACAST|\
-M_ZEROIZE)
+M_AUTH|M_LOOP|M_TUNNEL|M_LINK0|M_VLANTAG|M_ZEROIZE)
 
 /* Checksumming flags */
 #defineM_IPV4_CSUM_OUT 0x0001  /* IPv4 checksum needed */


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



allow TCP connections to IPv6 anycast addresses

2020-08-07 Thread Florian Obser


No longer prevent TCP connections to IPv6 anycast addresses.

RFC 4291 dropped this requirement from RFC 3513:
   o  An anycast address must not be used as the source address of an
  IPv6 packet.

And from that requirement draft-itojun-ipv6-tcp-to-anycast rightly
concluded that TCP connections must be prevented.

The draft also states:

The proposed method MUST be removed when one of the following events
happens in the future:

o  Restriction imposed on IPv6 anycast address is loosened, so that
   anycast address can be placed into source address field of the IPv6
   header[...]

OK?

diff --git share/man/man9/mbuf.9 share/man/man9/mbuf.9
index 6f798945437..ab02c36798f 100644
--- share/man/man9/mbuf.9
+++ share/man/man9/mbuf.9
@@ -306,8 +306,6 @@ protocol-specific.
 variable is valid.
 .It Dv M_LOOP
 packet has been sent from local machine.
-.It Dv M_ACAST
-received as IPv6 anycast.
 .It Dv M_BCAST
 packet sent/received as link-level broadcast.
 .It Dv M_MCAST
diff --git sys/netinet/ip_input.c sys/netinet/ip_input.c
index 1b511d14a4b..40c2f675959 100644
--- sys/netinet/ip_input.c
+++ sys/netinet/ip_input.c
@@ -619,20 +619,6 @@ ip_deliver(struct mbuf **mp, int *offp, int nxt, int af)
goto bad;
}
 
-#ifdef INET6
-   /* draft-itojun-ipv6-tcp-to-anycast */
-   if (af == AF_INET6 &&
-   ISSET((*mp)->m_flags, M_ACAST) && (nxt == IPPROTO_TCP)) {
-   if ((*mp)->m_len >= sizeof(struct ip6_hdr)) {
-   icmp6_error(*mp, ICMP6_DST_UNREACH,
-   ICMP6_DST_UNREACH_ADDR,
-   offsetof(struct ip6_hdr, ip6_dst));
-   *mp = NULL;
-   }
-   goto bad;
-   }
-#endif /* INET6 */
-
 #ifdef IPSEC
if (ipsec_in_use) {
if (ipsec_local_check(*mp, *offp, nxt, af) != 0) {
diff --git sys/netinet6/ip6_input.c sys/netinet6/ip6_input.c
index 64489f53b48..6c1beb6866c 100644
--- sys/netinet6/ip6_input.c
+++ sys/netinet6/ip6_input.c
@@ -424,8 +424,6 @@ ip6_input_if(struct mbuf **mp, int *offp, int nxt, int af, 
struct ifnet *ifp)
 */
if (rtisvalid(rt) && ISSET(rt->rt_flags, RTF_LOCAL)) {
struct in6_ifaddr *ia6 = ifatoia6(rt->rt_ifa);
-   if (ia6->ia6_flags & IN6_IFF_ANYCAST)
-   m->m_flags |= M_ACAST;
 
if (ip6_forwarding == 0 && rt->rt_ifidx != ifp->if_index &&
!((ifp->if_flags & IFF_LOOPBACK) ||


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



Re: current.html: i586 requirement for i386 architecture

2020-08-07 Thread Florian Obser
On Fri, Aug 07, 2020 at 09:57:37AM -0400, Bryan Steele wrote:
> On Fri, Aug 07, 2020 at 03:49:32PM +0200, Solene Rapenne wrote:
> > Now that i386 platform requires i586 CPU, I guess we should mention
> > it in current.html (the page i386.html should be updated accordingly
> > at 6.8 release)
> > 
> > Index: current.html
> > ===
> > RCS file: /home/reposync/www/faq/current.html,v
> > retrieving revision 1.1048
> > diff -u -p -r1.1048 current.html
> > --- current.html11 Jul 2020 16:53:47 -  1.1048
> > +++ current.html7 Aug 2020 13:43:25 -
> > @@ -57,6 +57,16 @@ use a snapshot to recover.
> >  Most of these changes will have to be performed as root.
> >  
> >  
> > +2020/08/03 - i386 requirements raised to i586
> > +
> > +Due to llvm upgrade to version 10 in base system, the minimal
> > +CPU instruction sets requirements for i386 architecture has been
> > +raised from i386 to i586.
> 
> I can't speak as to whether or not this is warrented or not, but this

I think it's worthwhile to have this since it will likely be picked up
for the upgrade notes from here as well.

Please add new entries to the end of the file, not the beginning.

> part is at least wrong. i386 CPU support was dropped years ago in the
> kernel, only 486-class processors and higher (with a math-coprocessor)
> are supported.
> 
> The real hint here might be that cpu0: must contain "CX8" to reliably
> work from now on, which indicates support for the CMPXCHG8 instruction.
> 
> > +In concrete terms for Intel and AMD CPUs, Intel Pentium and AMD K5
> > +are now the minimal requirements for i386 architecture.
> > +
> > +
> >  2020/05/29 - [ports] net/isc-bind example files 
> > dropped
> >  
> >  Outdated example configuration (including some "standard" zone files and
> > 
> > 
> 

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



additions to unit(1)

2020-08-04 Thread Florian Obser
Because of reasons I recently had to carry a lot of garbage around for
the municipality to pick up. They would only pickup 2 cubic meters in
one sitting so I had to check how much I had. Turned out to be 0.9 m^3.

Of course inquisitive minds wanted to know how much that is in
buttloads, to my great dismay units(1) did not know!

So I had to do it by hand like some savage and of course I got the
conversion wrong hence the following diff:

You have: 0.9 m3
You want: buttloads
* 0.53207987
/ 1.8794171

About half a buttload, good to know.

Extensive research on the subject[1] found definitions for the
imperial buttload as well as the related metric assload. Curiously the
assload is a measurement of mass while the buttload is a measurement
of volume.

Comments, OKs?

diff --git units.lib units.lib
index 0f811786bf3..df792a0a917 100644
--- units.lib
+++ units.lib
@@ -452,6 +452,7 @@ asb apostilb
 are1e+2 m2
 arpentcan  27.52 mi
 arpentlin  191.835 ft
+assload50 kg
 astronomicalunit   AU
 atmosphere 1.01325e+5 nt/m2
 atmatmosphere
@@ -477,6 +478,8 @@ bolt40 yd
 bottommeasure  1|40 in
 Bq becquerel
 britishthermalunit 1.05506e+3 joule fuzz
+butt   2 hogsheads
+buttload   6 seams
 btubritishthermalunit
 refrigeration  12000 btu/ton-hour
 buck   dollar

1) I tossed it into google and arrived at
https://en.wiktionary.org/wiki/buttload
as well as
https://www.ed.ac.uk/files/imports/fileManager/donkey%20fact%20sheet.pdf

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



Re: ssh(1), getrrsetbyname(3), SSHFP and DNSSEC

2020-07-29 Thread Florian Obser
On Wed, Jul 29, 2020 at 03:51:55PM +0200, Jeremie Courreges-Anglas wrote:
> So I did a few tests and read some unwind(8) code, and it *appears* safe
> to use unwind(8) along with "options trust-ad".

Yes.

> 
> First you mention fallback to DHCP-learned resolvers.  Those you should
> probably not trust indeed, but it looks like unwind(8) attempts to use
> them to perform its own validation.  So the value of the AD flag in
> unwind(8)'s response should be trustworthy.  At least that's what I see
> when I test unwind with "preference { dhcp }".
>

unwind always does its own validation or decides it can't do
validation. In both cases you can trust the AD flag.
The AD flag is set if and only if unwind did a successful validation.

You cannot trust the RCODE (and answer) when AD is *NOT* set. That
seems like a strange and obvious statement but it is the main
difference to validating unbound. When you enable validation in
unbound and someone messes with your packets you will get a SERVFAIL.

You can trick unwind into believing that DNSSEC validation is not
possible in your current network location. It will still resolve but
it will NOT set AD.

In the context of the current discussion this means that you can't
validate SSHFPs, but you can still try to connect to your ssh server
if you have the fingerprint in known_hosts. The later is still save
even if people play tricks with DNS in your current location.
 
> And then there's the asr fallback, tested with "preference { stub }".
> unwind(8) allocates its own asr context, ignoring the global
> /etc/resolv.conf and thus "options trust-ad".  IIUC it then hands off

Yes. But even if it did not it would not matter since it will clear
AD.

> the answer to libunbound which cooks its own soup.  So it looks like

No, the asr fallback is there for when your local network is truly
fucked - think last century equipment that knows DNS is udp/53 only and
max 512 bytes. While we already lower standards considerably in unwind
when we use libunbound compared to unbound the network still needs to
provide some things. Working edns0 is one of them, it's near
impossible to turn of in libunbound.

> unwind needs no special code to disable/ignore "options trust-ad".
> 
> I have no idea how unwind/libunbound behaves when using forwarders, the
> "accept bogus" feature, (opportunistic) DoT or other stuff I'm probably

Nope, as stated above, you can trust AD.

> overlooking.  But AFAICS using unwind + "options trust-ad" brings no
> problem.  cc'ing Florian, input welcome.
> 
> I think the issue with trust-ad is the list of resolvers that end up in
> resolv.conf.  I would expect people who use a resolver on localhost to
> also have other resolvers listed in their /etc/resolv.conf, because of
> a conscious choice (bsd.rd) or just because it should be more reliable.
> It seems to me that the resolv.conf(5) manpage addition should cover
> that.  Or should I make it more explicit?
> 
> Index: share/man/man5/resolv.conf.5
> ===
> RCS file: /d/cvs/src/share/man/man5/resolv.conf.5,v
> retrieving revision 1.60
> diff -u -p -r1.60 resolv.conf.5
> --- share/man/man5/resolv.conf.5  25 Apr 2020 14:22:04 -  1.60
> +++ share/man/man5/resolv.conf.5  25 Jul 2020 02:08:17 -
> @@ -285,6 +285,15 @@ first as an absolute name before any sea
>  .It Cm tcp
>  Forces the use of TCP for queries.
>  Normal behaviour is to query via UDP but fall back to TCP on failure.
> +.It Cm trust-ad
> +Sets the AD flag in DNS queries, and preserve the value of the AD flag
> +in DNS replies (this flag is stripped by default).
> +Useful when applications want to ensure that the DNS resources they
> +look up have been signed with DNSSEC and validated by the
> +.Ic nameserver .
> +This option should be used only if the transport between the
> +.Ic nameserver
> +and the resolver is secure.

The last sentence is the problem in my opinion. trust-ad is a button
mere mortals cannot push.

The typical answer is: Oh, I use a vpn tunel and my nameserver is
behind the tunnel. Does the tunel terminate on the nameserver?
My enterprise vpn hands me 4 resolvers. They are in a different
subnet than the vpn concentrator.

>  .El
>  .El
>  .Pp
> 
> Lazy me would be tempted to just let asr trust 127.0.0.1 / ::1 by

probably only save if you know that you are talking to certain
software that does it's own validation (unwind, unbound). Not save if
you are talking to say dnscrypt-proxy.

> default and let others deal with the edge cases, but where is the fun in
> that... :)
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
> 

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



Re: ssh(1), getrrsetbyname(3), SSHFP and DNSSEC

2020-07-29 Thread Florian Obser
On Wed, Jul 29, 2020 at 03:51:17PM +0200, Sebastian Benoit wrote:
> If i remember correctly, the fallout was caused by EDNS but i might be
> wrong. The unbound commit caused a developer some headscratching, because
> his upstream internet did not work with such packets, which led to immediate
> backout of the change, because a default config that does not work is not
> good.

It was time. Running DNSSEC validation on a system without an RTC is
not a good idea. NTP could not fix this because it depends on working
DNS. This has since been addressed by Otto.

The edns problem is well understood and has nothing to do with turning
DNSSEC validation on in unbound since unbound always sends an edns0
option. So if your network sucks so badly that you can't edns0 you
can't use unbound, period.

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



Re: acme-client config grammar improvements

2020-07-24 Thread Florian Obser
On Thu, Jul 16, 2020 at 07:40:35AM +0200, Daniel Eisele wrote:
> Also it would be nice to have a feature to update all domains of the
> config file. I currently do that in a shell script by parsing the output
> of acme-client -nv with sed and then calling acme-client multiple times.
> 
> Maybe an easy solution would be an option that prints the list of all
> domains, so I can avoid the sed parsing, as this is prone to breaking.

I'm not opposed to that. You will probably need to output some form of
csv.

Consider this:

domain handle1-example.com {
domain name example.com
alternative names { www.example.com secure.example.com }
domain key "/etc/ssl..." rsa
}
domain handle2-example.com {
domain name example.com
alternative names { mail.example.com }
domain key "/etc/ssl..." ecdsa
}

Should it be output like this?

handle1-example.com; example.com; www.example.com, secure.example.com
handle2-example.com; example.com; mail.example.com

Or this?

handle1-example.com; example.com; www.example.com
handle1-example.com; example.com; secure.example.com
handle2-example.com; example.com; mail.example.com


> 
> Another solution is obviously to just add an "update all" command line
> option (or maybe even in the config?), but that is probably more
> complicated to implement.

I'm more worried that you will very soon end up with some form of exec
plugin mechanism. Typically you need to do something when a cert is
renewed (restart daemon).

My acme-client.conf is generate by a config management system which
also creates individual cronjobs for each renew job and knows how to
handle a cert renew.

> 
> What do you think about that?
> 

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



Re: acme-client config grammar improvements

2020-07-24 Thread Florian Obser
On Tue, Jul 21, 2020 at 10:24:25PM +0200, Sebastian Benoit wrote:
> Daniel Eisele(daniel_eis...@gmx.de) on 2020.07.16 07:40:35 +0200:
> > Am 15.07.2020 um 23:51 schrieb Sebastian Benoit:
> > >> src/usr.sbin/acme-client/parse.y:
> > >> * The grammar allows the user to omit the newline after the first line
> > >>   in a domain or authority block.
> > >
> > > Yes. I'm still pnodering this. What are the chances that someone does 
> > > that?
> > >
> > > Probably no one does, but it worthwhile to break someones config for such 
> > > a
> > > change?
> > 
> > I can't imagine that anyone uses this, I only noticed it by reading the
> > grammar. But still not an important fix, I just noticed it and wanted to
> > point it out...
> > 
> > >> src/usr.sbin/acme-client/dbg.c doesn't build because in the included
> > >> header file extern.h the type pid_t is missing (unistd.h).
> > >
> > > extern.h should #include  for that, no?
> > 
> > Yes, sys/types.h is better, sorry about that.
> 
> So here is a patch i would like to commit.
> 
> ok?
> 
> diff --git usr.sbin/acme-client/dnsproc.c usr.sbin/acme-client/dnsproc.c
> index 664ef8d9b8b..72a0ea6b30e 100644
> --- usr.sbin/acme-client/dnsproc.c
> +++ usr.sbin/acme-client/dnsproc.c
> @@ -16,6 +16,7 @@
>   */
>  
>  #include 
> +#include 
>  #include 
>  
>  #include 
> diff --git usr.sbin/acme-client/extern.h usr.sbin/acme-client/extern.h
> index 529d3350205..76c86b5cce0 100644
> --- usr.sbin/acme-client/extern.h
> +++ usr.sbin/acme-client/extern.h
> @@ -17,6 +17,8 @@
>  #ifndef EXTERN_H
>  #define EXTERN_H
>  
> +#include 
> +
>  #include "parse.h"
> 

these two chunks are OK florian
 
>  #define MAX_SERVERS_DNS 8
> diff --git usr.sbin/acme-client/parse.y usr.sbin/acme-client/parse.y
> index 120f253a63f..0b96794f8da 100644
> --- usr.sbin/acme-client/parse.y
> +++ usr.sbin/acme-client/parse.y
> @@ -170,11 +170,11 @@ varset  : STRING '=' string {
>   }
>   ;
>  
> -optnl: '\n' optnl
> +optnl: nl
>   |
>   ;
>  
> -nl   : '\n' optnl/* one newline or more */
> +nl   : optnl '\n'/* one newline or more */
>   ;
> 

I'm not a fan of this.

We currently have two different optnl implementations in 24 parsers
(hostappd and radiusd are weird):

optnl   : '\n' optnl
|
;

and

optnl   : /* empty */
| '\n' optnl
;

I don't think we need a third one. Also it shouldn't depend on nl
because nl is not implemented in all parsers.

Can we go with this and leave nl alone because nl is not recursive
itself so there is no problem there.

optnl   : /* empty */
| optnl '\n'
;

I think we can copy that to all other parse.y.

The rest is OK florian
 
>  comma: ','
> @@ -190,7 +190,7 @@ authority : AUTHORITY STRING {
>   yyerror("authority already defined");
>   YYERROR;
>   }
> - } '{' optnl authorityopts_l '}' {
> + } '{' optnl authorityopts_l optnl '}' {
>   if (auth->api == NULL) {
>   yyerror("authority %s: no api URL specified",
>   auth->name);
> @@ -205,8 +205,8 @@ authority : AUTHORITY STRING {
>   }
>   ;
>  
> -authorityopts_l  : authorityopts_l authorityoptsl nl
> - | authorityoptsl optnl
> +authorityopts_l  : authorityopts_l nl authorityoptsl
> + | authorityoptsl
>   ;
>  
>  authorityoptsl   : API URL STRING {
> @@ -246,7 +246,7 @@ domain: DOMAIN STRING {
>   yyerror("domain already defined");
>   YYERROR;
>   }
> - } '{' optnl domainopts_l '}' {
> + } '{' optnl domainopts_l optnl '}' {
>   if (domain->domain == NULL) {
>   if ((domain->domain = strdup(domain->handle))
>   == NULL)
> @@ -273,8 +273,8 @@ keytype   : RSA   { $$ = KT_RSA; }
>   |   { $$ = KT_RSA; }
>   ;
>  
> -domainopts_l : domainopts_l domainoptsl nl
> - | domainoptsl optnl
> +domainopts_l : domainopts_l nl domainoptsl
> + | domainoptsl
>   ;
>  
>  domainoptsl  : ALTERNATIVE NAMES '{' altname_l '}'
> @@ -385,7 +385,7 @@ domainoptsl   : ALTERNATIVE NAMES '{' altname_l '}'
>   }
>   ;
>  
> -altname_l: altname comma altname_l
> +altname_l: altname_l comma altname
>   | altname
>   ;
>  

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



nsd 4.3.2

2020-07-20 Thread Florian Obser
Tests, OKs?

diff --git doc/ChangeLog doc/ChangeLog
index 09ea79bafd3..ba80174afdf 100644
--- doc/ChangeLog
+++ doc/ChangeLog
@@ -1,3 +1,69 @@
+7 July 2020: Wouter
+   - Tag for 4.3.2rc1.
+
+6 July 2020: Wouter
+   - Fix compile includes for xfr-inspect tool on FreeBSD.
+   - Add tpkg/run_vm.sh that runs test when in a virtual machine.
+   - Merge #112 from jaredmauch: log old and new serials when NSD
+ rejects an IXFR due to an old serial number.
+   - Fix bug034 test for vm test changes.
+
+22 June 2020: Wouter
+   - Remove errno reset behaviour from sendmmsg and recvmmsg
+ replacement functions.
+   - Fix unit test for different nsd-control-setup -h exit code.
+
+19 June 2020: Wouter
+   - Merge #108 from Nomis: Make the max-retry-time description clearer.
+   - Retry when udp send buffer is full to wait until buffer space is
+ available.
+
+18 June 2020: Wouter
+   - Do not log EAGAIN errors for sendmmsg, to stop log spam on OpenBSD.
+
+17 June 2020: Wouter
+   - Fix #107: nsd -v shows configure line, openssl version and libevent 
version.
+
+27 May 2020: Wouter
+   - Fix unlink of pidfile warning if not possible due to permissions,
+ nsd can display the message at high verbosity levels.
+   - Update contrib/nsd.service for chown of nsd.log and /var/log in
+ ReadWritePaths.
+   - Removed contrib/nsd.service, example is too complicated and not
+ useful.
+
+15 May 2020: Wouter
+   - Merge PR#102 from and0x000: add missing default in documentation
+ for drop-updates.
+   - Fix checkconf test for log-only-syslog option.
+
+14 May 2020: Wouter
+   - Document default value for tcp-timeout.
+
+13 May 2020: Jeroen
+   - Fix #99: Fix copying of socket properties with reuseport enabled.
+
+24 April 2020: Wouter
+   - Fix #97: EDNS unknown version: query not in response.
+
+21 April 2020: Wouter
+   - Fix #96: log-only-syslog: yes sets to only use syslog, fixes
+ that the default configuration and systemd results in duplicate
+ log messages.
+
+20 April 2020: Wouter
+   - Fix #95: Removed make test check because tpkg not included in
+ release tarballs.
+   - Fix unused parameter compile warnings.
+
+16 April 2020: Wouter
+   - Tag for 4.3.1 release and track 4.3.2 release in code repository.
+   - note sha256 digest algo use in makedist.sh.
+   - Fix for posix shell syntax for trap in nsd-control-setup.
+   - Fix to omit the listen-on lines from log at startup, unless verbose.
+   - Fix uninitialised values for bindtodevice option at startup with
+ reuseport and multiple interfaces.
+
 8 April 2020: Wouter
- Tag for 4.3.1rc2.
 
diff --git Makefile.in Makefile.in
index 4e6915d4461..39076034e6c 100644
--- Makefile.in
+++ Makefile.in
@@ -81,7 +81,7 @@ MANUALS=nsd.8 nsd-checkconf.8 nsd-checkzone.8 nsd-control.8 
nsd.conf.5
 COMMON_OBJ=answer.o axfr.o buffer.o configlexer.o configparser.o dname.o dns.o 
edns.o iterated_hash.o lookup3.o namedb.o nsec3.o options.o packet.o query.o 
rbtree.o radtree.o rdata.o region-allocator.o rrl.o tsig.o tsig-openssl.o udb.o 
udbradtree.o udbzone.o util.o bitset.o popen3.o
 XFRD_OBJ=xfrd-disk.o xfrd-notify.o xfrd-tcp.o xfrd.o remote.o $(DNSTAP_OBJ)
 NSD_OBJ=$(COMMON_OBJ) $(XFRD_OBJ) difffile.o ipc.o mini_event.o netio.o nsd.o 
server.o dbaccess.o dbcreate.o zlexer.o zonec.o zparser.o
-ALL_OBJ=$(NSD_OBJ) nsd-checkconf.o nsd-checkzone.o nsd-control.o nsd-mem.o
+ALL_OBJ=$(NSD_OBJ) nsd-checkconf.o nsd-checkzone.o nsd-control.o nsd-mem.o 
xfr-inspect.o
 NSD_CHECKCONF_OBJ=$(COMMON_OBJ) nsd-checkconf.o
 NSD_CHECKZONE_OBJ=$(COMMON_OBJ) $(XFRD_OBJ) dbaccess.o dbcreate.o difffile.o 
ipc.o mini_event.o netio.o server.o zonec.o zparser.o zlexer.o nsd-checkzone.o
 NSD_CONTROL_OBJ=$(COMMON_OBJ) nsd-control.o
@@ -193,9 +193,6 @@ audit: nsd nsd-checkconf nsd-checkzone nsd-control nsd-mem 
checksec
./checksec --file=nsd-control
./checksec --file=nsd-mem
 
-test check: cutest
-   ./cutest
-
 clean:
rm -f *.o $(TARGETS) $(MANUALS) cutest popen3_echo udb-inspect 
xfr-inspect nsd-mem
 
diff --git config.h.in config.h.in
index 67cd876e427..35cd47f2e82 100644
--- config.h.in
+++ config.h.in
@@ -9,6 +9,9 @@
 /* NSD default chroot directory */
 #undef CHROOTDIR
 
+/* Command line arguments used with configure */
+#undef CONFCMDLINE
+
 /* NSD config dir */
 #undef CONFIGDIR
 
diff --git configlexer.lex configlexer.lex
index 8469bbce35d..bb4dd3499c4 100644
--- configlexer.lex
+++ configlexer.lex
@@ -215,6 +215,7 @@ identity{COLON} { LEXOUT(("v(%s) ", yytext)); 
return VAR_IDENTITY;}
 version{COLON} { LEXOUT(("v(%s) ", yytext)); return VAR_VERSION;}
 nsid{COLON}{ LEXOUT(("v(%s) ", yytext)); return VAR_NSID;}
 logfile{COLON} { LEXOUT(("v(%s) ", yytext)); return VAR_LOGFILE;}
+log-only-syslog{COLON} { LEXOUT(("v(%s) ", yytext)); return 

Re: empty rc.firsttime when installing

2020-07-14 Thread Florian Obser
On Tue, Jul 14, 2020 at 03:03:33PM +0200, Denis Fondras wrote:
> I was upgrading an EdgeRouter and it restarted multiple times instead of 
> booting
> /bsd
> 
> When I had a chance to boot it correctly, I noticed that sysmerge and 
> fw_update
> were run multiple times.
> 
> This diff avoids filling rc.firsttime and rc.sysmerge.

This interfers with sysupgrade(8) cleanup.

> 
> 
> Index: distrib/miniroot/install.sub
> ===
> RCS file: /cvs/src/distrib/miniroot/install.sub,v
> retrieving revision 1.1154
> diff -u -p -r1.1154 install.sub
> --- distrib/miniroot/install.sub  26 May 2020 16:21:00 -  1.1154
> +++ distrib/miniroot/install.sub  14 Jul 2020 12:54:27 -
> @@ -2734,6 +2734,9 @@ finish_up() {
>   local _kernel_dir=/mnt/usr/share/relink/kernel
>   local _kernel=${MDKERNEL:-GENERIC} _syspatch_archs="amd64 arm64 i386"
>  
> + # Empty rc.firsttime
> + echo "" >/mnt/etc/rc.firsttime
> +
>   # Mount all known swap partitions.  This gives systems with little
>   # memory a better chance at running 'MAKEDEV all'.
>   if [[ -x /mnt/sbin/swapctl ]]; then
> @@ -2812,7 +2815,7 @@ finish_up() {
>  
>   # Ensure that sysmerge in batch mode is run on reboot.
>   [[ $MODE == upgrade ]] &&
> - echo "/usr/sbin/sysmerge -b" >>/mnt/etc/rc.sysmerge
> + echo "/usr/sbin/sysmerge -b" >/mnt/etc/rc.sysmerge
>  
>   # If a proxy was needed to fetch the sets, use it for fw_update and 
> syspatch
>   [[ -n $http_proxy ]] &&
> 

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



Re: Undefined Behavior at jsmn.c

2020-07-12 Thread Florian Obser
On Sun, Jul 12, 2020 at 09:10:57AM +0200, Otto Moerbeek wrote:
> On Sun, Jul 12, 2020 at 09:57:02AM +0430, Ali Farzanrad wrote:
> 
> > Hi @tech,
> > 
> > I was comparing jsmn.c in acme-client with jsmn.c in FreeBSD [1].
> > I found a switch without a default case which is an undefined behavior:
> > 
> > @@ -69,6 +69,8 @@
> > case '\t' : case '\r' : case '\n' : case ' ' :
> > case ','  : case ']'  : case '}' :
> > goto found;
> > +   default:
> > +   break;
> > }
> > if (js[parser->pos] < 32 || js[parser->pos] >= 127) {
> > parser->pos = start;
> > 
> > I have patched that undefined behavior + some style fix.
> 
> It is bad practise to intermix style changes with bug fixes. 
> Please post the fix seperately.

It's also not undefined behaviour.
ISO/IEC 9899:1999 6.8.4.2:

If no converted case constant expression matches and there is no
default label, no part of the switch body is executed.

Not having a default case might be considered bad style in itself
though.

> 
>   -Otto
> 

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



Re: 11n Tx aggregation for iwm(4)

2020-06-26 Thread Florian Obser
Seems to be working on a X1 gen2 using
iwm0 at pci2 dev 0 function 0 "Intel Dual Band Wireless AC 7260" rev 0x83, msi
against a Unifi AP-SHD.

Before:

bandwidth min/avg/max/std-dev = 7.344/9.077/11.514/0.803 Mbps

after:

bandwidth min/avg/max/std-dev = 12.551/65.407/82.835/14.169 Mbps

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



  1   2   3   4   5   6   7   >