Re: patch unveil fail

[Florian Obser]

reads correct, OK florian

On 2023-10-25 13:38 +02, Alexander Bluhm  wrote:
> Hi,
>
> Since 7.4 patch(1) does not work if an explicit patchfile is given on
> command line.
>
> https://marc.info/?l=openbsd-cvs&m=168941770509379&w=2
>
> root@ot14:.../~# patch /usr/src/usr.bin/patch/patch.c patch-unveil.diff
> Hmm...  Looks like a unified diff to me...
> The text leading up to this was:
> --
> |Index: patch.c
> |===
> |RCS file: /data/mirror/openbsd/cvs/src/usr.bin/patch/patch.c,v
> |diff -u -p -r1.74 patch.c
> |--- patch.c19 Jul 2023 13:26:20 -  1.74
> |+++ patch.c24 Oct 2023 17:13:28 -
> --
> Patching file /usr/src/usr.bin/patch/patch.c using Plan A...
> Hunk #1 succeeded at 32.
> Hunk #2 succeeded at 214.
> Hunk #3 succeeded at 245.
> Can't backup /usr/src/usr.bin/patch/patch.c, output is in 
> /tmp/patchoorjYymLKcM: No such file or directory
> done
>
> A backup file should be created in the directory of the original
> file, but only the current directory is unveiled.  Then the patched
> file is created in /tmp and does not replace the original patchfile
> in place.
>
> Diff below fixes it.
>
> ok?
>
> bluhm
>
> Index: patch.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.bin/patch/patch.c,v
> diff -u -p -r1.74 patch.c
> --- patch.c   19 Jul 2023 13:26:20 -  1.74
> +++ patch.c   24 Oct 2023 17:13:28 -
> @@ -32,6 +32,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -213,11 +214,27 @@ main(int argc, char *argv[])
>   perror("unveil");
>   my_exit(2);
>   }
> - if (filearg[0] != NULL)
> + if (filearg[0] != NULL) {
> + char *origdir;
> +
>   if (unveil(filearg[0], "rwc") == -1) {
>   perror("unveil");
>   my_exit(2);
>   }
> + if ((origdir = dirname(filearg[0])) == NULL) {
> + perror("dirname");
> + my_exit(2);
> + }
> + if (unveil(origdir, "rwc") == -1) {
> + perror("unveil");
> + my_exit(2);
> + }
> + } else {
> + if (unveil(".", "rwc") == -1) {
> + perror("unveil");
> + my_exit(2);
> + }
> + }
>   if (filearg[1] != NULL)
>   if (unveil(filearg[1], "r") == -1) {
>   perror("unveil");
> @@ -228,10 +245,6 @@ main(int argc, char *argv[])
>   perror("unveil");
>   my_exit(2);
>   }
> - if (unveil(".", "rwc") == -1) {
> - perror("unveil");
> - my_exit(2);
> - }
>   if (*rejname != '\0')
>   if (unveil(rejname, "rwc") == -1) {
>   perror("unveil");
>

-- 
In my defence, I have been left unsupervised.

Re: relayd does not delete control socket on shutdown

[Florian Obser]

On 2023-10-21 14:49 +03, Kapetanakis Giannis  wrote:
> Rev 1.140 by florian@ seems to have changed that.
>
> Do not try to unlink the control socket in an unprivileged child
> process on shutdown.
> Found while working ontame(2)  .
> OK benno@
>

Which was 8 years ago. I don't understand why you see a change in 7.4.

Anyway, we decided to not clean up control sockets in any of our
privsep daemons because leaving them behind does not cause any issues.

> G
>
>
> On 21/10/2023 14:41, Kapetanakis Giannis wrote:
>> After 7.4 relayd does not unlink it's socket
>>
>> I've added the following but it's probably not enough. unveil?
>>
>> G
>>
>> Index: relayd.c
>> ===
>> RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
>> retrieving revision 1.191
>> diff -u -p -r1.191 relayd.c
>> --- relayd.c    25 Jun 2023 08:07:38 -    1.191
>> +++ relayd.c    21 Oct 2023 11:39:44 -
>> @@ -382,6 +382,8 @@ parent_shutdown(struct relayd *env)
>>  free(env->sc_ps);
>>  free(env);
>>
>> +    unlink(env->sc_ps->ps_csock.cs_name);
>> +
>>  log_info("parent terminating, pid %d", getpid());
>>
>>  exit(0);
>>
>

-- 
In my defence, I have been left unsupervised.

Re: sysupgrade: omit default sets answer

[Florian Obser]

On 2023-09-29 14:41 UTC, Klemens Nanni  wrote:
> The response file contains only to non-defaults, except for
>   Set name(s)? (or 'abort' or 'done') [done] done
>
> which is the hardcoded default since 2009:
>   ask "Set name(s)? (or 'abort' or 'done')" done
>
> We pass it since r1.23 in 2019
> Let sysupgrade(8) create auto_upgrade.conf file in preparation of
> moving the functionality out of install.sub.
> OK deraadt
>
> I see no need for explicit defaults.
> sysupgrade(8) and installer work the same with this diff.

I have a very vague recollection that there was a reason to have this
but I can't put my finger on it. Something like the installer would spin
on selecting the sets while we were developing this inside of
install.sub. It was probably some other mistake that got fixed and this
is a left-over.

Anyway, soon a lot of people are going to try sysupgrade in all kinds of
weird setups, now is a good time to put this in, IFF you have the time
to keep an eye on that it doesn't break. You also want deraadt@ on board
for this.

With all those constraints met, OK florian (I *don't* have time to keep an
eye on it.)


>
> Index: sysupgrade.sh
> ===jk
> RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
> retrieving revision 1.48
> diff -u -p -r1.48 sysupgrade.sh
> --- sysupgrade.sh 8 Jun 2022 09:03:11 -   1.48
> +++ sysupgrade.sh 29 Sep 2023 13:48:18 -
> @@ -185,7 +185,6 @@ fi
>  cat <<__EOT >/auto_upgrade.conf
>  Location of sets = disk
>  Pathname to the sets = ${SETSDIR}/
> -Set name(s) = done
>  Directory does not contain SHA256.sig. Continue without verification = yes
>  __EOT
>  
>

-- 
In my defence, I have been left unsupervised.

Re: patch: partially fix interactive mode

[Florian Obser]

OK florian

On 2023-07-19 13:17 +02, Theo Buehler  wrote:
> The addition of unveil broke interactive mode since ask() assumes the
> default answer if it fails to open _PATH_TTY. Questions are only asked
> if neither force nor batch mode is activated, so condition on those.
>
> It seams cleaner to do unveil _PATH_TTY than to add a tty pledge since
> as far as I can see no ioctls on /dev/tty are emitted.
>
> Without this patch:
>
> $ cd usr.bin
> $ patch -Rs < /path/to/patch.below
> File to patch:
> No file found--skip this patch? [y]
> 1 out of 1 hunks ignored--saving rejects to Oops.rej
>
> With this patch:
>
> $ cd usr.bin
> $ patch -Rs < /path/to/patch.below
> File to patch: patch/patch.c
> Unreversed (or previously applied) patch detected!  Ignore -R? [y] y
>
> Obviously the unveil(".", rwc) places restrictions on the path that can
> be given, but that's expected.
>
> Index: patch.c
> ===
> RCS file: /cvs/src/usr.bin/patch/patch.c,v
> retrieving revision 1.73
> diff -u -p -4 -r1.73 patch.c
> --- patch.c   15 Jul 2023 10:42:54 -  1.73
> +++ patch.c   19 Jul 2023 11:09:37 -
> @@ -222,8 +222,13 @@ main(int argc, char *argv[])
>   if (unveil(filearg[1], "r") == -1) {
>   perror("unveil");
>   my_exit(2);
>   }
> + if (!force && !batch)
> + if (unveil(_PATH_TTY, "rw") == -1) {
> + perror("unveil");
> + my_exit(2);
> + }
>   if (unveil(".", "rwc") == -1) {
>   perror("unveil");
>   my_exit(2);
>   }
>

-- 
In my defence, I have been left unsupervised.

Re: Remove ENGINE use from relayd

[Florian Obser]

I for one welcome our new relayd maintainer!

patch(1): don't run off the end in num_components.

[Florian Obser]

Found with afl, if path ends in '/', num_components will run off the end
of the string.

OK?

(this is on top of tb's fix on bugs but should be independent and not
cause conflicts.)

diff --git pch.c pch.c
index 63543a609fb..8c58dc9ffe5 100644
--- pch.c
+++ pch.c
@@ -1484,7 +1484,8 @@ num_components(const char *path)
size_t n;
const char *cp;
 
-   for (n = 0, cp = path; (cp = strchr(cp, '/')) != NULL; n++, cp++) {
+   for (n = 0, cp = path; (cp = strchr(cp, '/')) != NULL; n++) {
+   cp++;
while (*cp == '/')
cp++;   /* skip consecutive slashes */
}

-- 
In my defence, I have been left unsupervised.

patch(1): basename(3) can fail

[Florian Obser]

So I was sufficiently bored during breakfast and decided to run afl
against patch...

basename(3) can fail thusly:
ERRORS
 The following error codes may be set in errno:

 [ENAMETOOLONG] The path component to be returned was larger than
PATH_MAX.

and then strlen(3) segfaults.

OK?

(this is on top of tb's fix on bugs but should be independent and not
cause conflicts.)

diff --git pch.c pch.c
index 4ae5f363393..63543a609fb 100644
--- pch.c
+++ pch.c
@@ -1422,7 +1422,7 @@ compare_names(const struct file_name *names, bool 
assume_exists)
 {
size_t min_components, min_baselen, min_len, tmp;
char *best = NULL;
-   char *path;
+   char *path, *bn;
int i;
 
/*
@@ -1443,7 +1443,10 @@ compare_names(const struct file_name *names, bool 
assume_exists)
min_components = tmp;
best = path;
}
-   if ((tmp = strlen(basename(path))) > min_baselen)
+   bn = basename(path);
+   if (bn == NULL)
+   continue;
+   if ((tmp = strlen(bn)) > min_baselen)
continue;
if (tmp < min_baselen) {
min_baselen = tmp;

-- 
In my defence, I have been left unsupervised.

Re: httpd: use the host name in SERVER_NAME

[Florian Obser]

On 2023-06-30 10:46 +02, Omar Polo  wrote:
> On 2023/06/29 23:43:25 +0200, Omar Polo  wrote:
>> On 2023/06/29 19:55:52 +0200, Florian Obser  wrote:
>> > I'm worried that we pass un-sanitized input through to fcgi.
>> > Of course we are passing *a lot* of un-sanitized input through to fcgi,
>> > so does this matter in the grand scheme of things?
>> > But I'd like if server_http_parsehost() enforces syntactically valid
>> > hostnames first.
>> > 
>> > There is valid_domain() in usr.bin/ssh/misc.c or res_hnok in
>> > resolv.h. Which is probably fine to use since we don't care too much
>> > about portable.
>> 
>> right, httpd is happily accepting any nonsense as Host.  Here's an
>> adaptation of millert' valid_domain() from usr.bin/ssh/misc.c.
>> Hope noone is relying on using an empty/malformed Host :>
>
> actually this is wrong.  it breaks when an ipv6 address is used as
> Host.  this seems to be an issue in ssh too, although I haven't

IIRC ssh only uses valid_domain() when it already knows that it's not an
IPv6 address.

> checked if the ssh URI specification inherits the authority from
> rfc3986 as-is.
>
> here's a slightly revised version, but just for the sake of the
> discussion since its ipv6 parsing is too permissive.
>
> should I make it strictier (not done just because it's a bit long and
> definitely not fun to validate ipv6 addresses), or there's something
> else in base that I can steal fom? :)

getaddrinfo(3) with AI_NUMERICHOST.

>
> (while here I've tweaked slightly the nonsense in
> server_http_parsehost wrt ipv6 addresses and port numbers.)
>
>
> Thanks!
>
>
> diff /usr/src
> commit - 9832f5035d799ae12e9f848cff5650481b673260
> path + /usr/src
> blob - 091cc86fec946a4a9d422c04a48f5cbfae015ce0
> file + usr.sbin/httpd/server_http.c
> --- usr.sbin/httpd/server_http.c
> +++ usr.sbin/httpd/server_http.c
> @@ -831,6 +831,60 @@ char *
>   return (buf);
>  }
>  
> +static int
> +valid_domain(char *name, const char **errstr)
> +{
> + size_t i;
> + unsigned char c, last = '\0';
> +
> + *errstr = NULL;
> +
> + if (*name == '\0') {
> + *errstr = "empty domain name";
> + return 0;
> + }
> +
> + if (name[0] == '[') {
> + for (i = 1; name[i] != '\0'; ++i) {
> + if (name[i] == ']')
> + break;
> + if (!isxdigit((unsigned char)name[i]) &&
> + name[i] != ':') {
> + *errstr = "invalid IPv6 address";
> + return 0;
> + }
> + }
> + if (name[i] != ']' || name[i + 1] != '\0') {
> + *errstr = "invalid IPv6 address";
> + return 0;
> + }
> + return 1;
> + }
> +
> + if (!isalpha((unsigned char)name[0]) &&
> + !isdigit((unsigned char)name[0])) {
> + *errstr = "domain name starts with an invalid character";
> + return 0;
> + }
> + for (i = 0; name[i] != '\0'; ++i) {
> + c = tolower((unsigned char)name[i]);
> + name[i] = c;
> + if (last == '.' && c == '.') {
> + *errstr = "domain name contains consecutive separators";
> + return 0;
> + }
> + if (c != '.' && c != '-' && !isalnum(c) &&
> + c != '_') /* technically invalid, but common */ {
> + *errstr = "domain name contains invalid characters";
> + return 0;
> + }
> + last = c;
> + }
> + if (name[i - 1] == '.')
> + name[i - 1] = '\0';
> + return 1;
> +}
> +
>  char *
>  server_http_parsehost(char *host, char *buf, size_t len, int *portval)
>  {
> @@ -847,13 +901,11 @@ server_http_parsehost(char *host, char *buf, size_t le
>  
>   if (*start == '[' && (end = strchr(start, ']')) != NULL) {
>   /* Address enclosed in [] with port, eg. [2001:db8::1]:80 */
> - start++;

are you forgetting to skip over the opening '['?

> - *end++ = '\0';
> - if ((port = strchr(end, ':')) == NULL || *port == '\

Re: httpd: use the host name in SERVER_NAME

[Florian Obser]

On 2023-06-19 18:27 +02, Omar Polo  wrote:
> currently httpd uses the name specified in the config `server' block
> which is not guaranteed to be a valid hostname.
>
> quoting rfc3875:
>
>The SERVER_NAME variable MUST be set to the name of the server host
>to which the client request is directed.  It is a case-insensitive
>hostname or network address.  It forms the host part of the
>Script-URI.
>
>[...]
>
>A deployed server can have more than one possible value for this
>variable, where several HTTP virtual hosts share the same IP
>address.  In that case, the server would use the contents of the
>request's Host header field to select the correct virtual host.
>
> This is important since `block return' with $SERVER_NAME may be
> expanded to something invalid, or a fastcgi application that looks at
> SERVER_NAME (e.g. gotwebd or php) may receive stuff like
> "*.example.com" instead of a proper host name depending on the
> configuration.
>
> This introduces some redundancy with HTTP_HOST (which we have since
> all the HTTP headers become a fastcgi param in the form of HTTP_*).
>
> I'm relying on the fact that httpd disallows requests without a Host
> header, so that in the fastcgi path desc->http_host is not NULL.
>
> thoughts/ok?

I'm worried that we pass un-sanitized input through to fcgi.
Of course we are passing *a lot* of un-sanitized input through to fcgi,
so does this matter in the grand scheme of things?
But I'd like if server_http_parsehost() enforces syntactically valid
hostnames first.

There is valid_domain() in usr.bin/ssh/misc.c or res_hnok in
resolv.h. Which is probably fine to use since we don't care too much
about portable.

Ignoring those musings, diff is OK

>
>
> diff /usr/src
> commit - 0cb70ced01a203d011a17a85e0c08bbb95ac164d
> path + /usr/src
> blob - 073ab344079c6ee4ae422c6e7ad01c70a7002055
> file + usr.sbin/httpd/server_fcgi.c
> --- usr.sbin/httpd/server_fcgi.c
> +++ usr.sbin/httpd/server_fcgi.c
> @@ -333,7 +333,7 @@ server_fcgi(struct httpd *env, struct client *clt)
>   goto fail;
>   }
>  
> - if (fcgi_add_param(¶m, "SERVER_NAME", srv_conf->name,
> + if (fcgi_add_param(¶m, "SERVER_NAME", desc->http_host,
>   clt) == -1) {
>   errstr = "failed to encode param";
>   goto fail;
> blob - 08deda7d5f097683571264da0ff434553801f8f7
> file + usr.sbin/httpd/server_http.c
> --- usr.sbin/httpd/server_http.c
> +++ usr.sbin/httpd/server_http.c
> @@ -1245,9 +1245,10 @@ server_expand_http(struct client *clt, const char *val
>   return (NULL);
>   }
>   if (strstr(val, "$SERVER_NAME") != NULL) {
> - if ((str = url_encode(srv_conf->name))
> -  == NULL)
> + if (desc->http_host == NULL)
>   return (NULL);
> + if ((str = url_encode(desc->http_host)) == NULL)
> + return (NULL);
>   ret = expand_string(buf, len, "$SERVER_NAME", str);
>   free(str);
>   if (ret != 0)
>

-- 
In my defence, I have been left unsupervised.

Re: relayd: fix route handling for IPv6

[Florian Obser]

On 2023-06-29 15:03 +02, Claudio Jeker  wrote:
> Once again struct sockaddr_in6 causes 64bit systems to cry. This time in
> relayd. You can not statically setup a route message and think it will
> work. All our routing daemons switched to iov for building the route
> message out of various components. This diff does the same for relayd.
> With this it is possible to use router blocks with IPv6 addrs.
>
> Btw. this does not work with link local addressing but I do not care
> about that dumpster fire.

I had hoped for considerably more -
Oh well, one can dream.
One nit inline, diff reads fine, OK florian

> -- 
> :wq Claudio
>
> Index: pfe_route.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/pfe_route.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 pfe_route.c
> --- pfe_route.c   28 May 2017 10:39:15 -  1.12
> +++ pfe_route.c   29 Jun 2023 12:55:59 -
> @@ -19,12 +19,14 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
>  #include 
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -32,24 +34,6 @@
>  
>  #include "relayd.h"
>  
> -struct relay_rtmsg {
> - struct rt_msghdrrm_hdr;
> - union {
> - struct {
> - struct sockaddr_in  rm_dst;
> - struct sockaddr_in  rm_gateway;
> - struct sockaddr_in  rm_netmask;
> - struct sockaddr_rtlabel rm_label;
> - }u4;
> - struct {
> - struct sockaddr_in6 rm_dst;
> - struct sockaddr_in6 rm_gateway;
> - struct sockaddr_in6 rm_netmask;
> - struct sockaddr_rtlabel rm_label;
> - }u6;
> - }rm_u;
> -};
> -
>  void
>  init_routes(struct relayd *env)
>  {
> @@ -103,110 +87,97 @@ sync_routes(struct relayd *env, struct r
>   }
>  }
>  
> +static void
> +pfe_apply_prefixlen(struct sockaddr_storage *ss, int af, int len)
> +{
> + int q, r, off;
> + uint8_t *b = (uint8_t *)ss;
> +
> +q = len >> 3;

^ spaces vs. tab

> + r = len & 7;
> +
> + bzero(ss, sizeof(*ss));
> + ss->ss_family = af;
> + switch (af) {
> + case AF_INET:
> + ss->ss_len = sizeof(struct sockaddr_in);
> + off = offsetof(struct sockaddr_in, sin_addr);
> + break;
> + case AF_INET6:
> + ss->ss_len = sizeof(struct sockaddr_in6);
> + off = offsetof(struct sockaddr_in6, sin6_addr);
> + break;
> + default:
> + fatal("%s: invalid address family", __func__);
> + }
> + if (q > 0)
> + memset(b + off, 0xff, q);
> + if (r > 0)
> + b[off + q] = (0xff00 >> r) & 0xff;
> +}
> +
> +#define ROUNDUP(a) \
> + ((a) > 0 ? (1 + (((a) - 1) | (sizeof(long) - 1))) : sizeof(long))
> +
>  int
>  pfe_route(struct relayd *env, struct ctl_netroute *crt)
>  {
> - struct relay_rtmsg   rm;
> - struct sockaddr_rtlabel  sr;
> - struct sockaddr_storage *gw;
> - struct sockaddr_in  *s4;
> - struct sockaddr_in6 *s6;
> - size_t   len = 0;
> + struct iovec iov[5];
> + struct rt_msghdr hdr;
> + struct sockaddr_storage  dst, gw, mask, label;
> + struct sockaddr_rtlabel *sr = (struct sockaddr_rtlabel *)&label;
> + int  iovcnt = 0;
>   char*gwname;
> - int  i = 0;
>  
> - gw = &crt->host.ss;
> - gwname = crt->host.name;
> + bzero(&hdr, sizeof(hdr));
> + hdr.rtm_msglen = sizeof(hdr);
> + hdr.rtm_version = RTM_VERSION;
> + hdr.rtm_type = HOST_ISUP(crt->up) ? RTM_ADD : RTM_DELETE;
> + hdr.rtm_flags = RTF_STATIC | RTF_GATEWAY | RTF_MPATH;
> + hdr.rtm_seq = env->sc_rtseq++;
> + hdr.rtm_addrs = RTA_DST | RTA_GATEWAY | RTA_NETMASK;
> + hdr.rtm_tableid = crt->rt.rtable;
> + hdr.rtm_priority = crt->host.priority;
>  
> - bzero(&rm, sizeof(rm));
> - bzero(&sr, sizeof(sr));
> + iov[iovcnt].iov_base = &hdr;
> + iov[iovcnt++].iov_len = sizeof(hdr);
> +
> + dst = crt->nr.ss;
> + gw = crt->host.ss;
> + gwname = crt->host.name;
> + pfe_apply_prefixlen(&mask, dst.ss_family, crt->nr.prefixlen);
>  
> - rm.rm_hdr.rtm_msglen = len;
> - rm.rm_hdr.rtm_version = RTM_VERSION;
> - rm.rm_hdr.rtm_type = HOST_ISUP(crt->up) ? RTM_ADD : RTM_DELETE;
> - rm.rm_hdr.rtm_flags = RTF_STATIC | RTF_GATEWAY | RTF_MPATH;
> - rm.rm_hdr.rtm_seq = env->sc_rtseq++;
> - rm.rm_hdr.rtm_addrs = RTA_DST | RTA_GATEWAY;
> - rm.rm_hdr.rtm_tableid = crt->rt.rtable;
> - rm.rm_hdr.rtm_priority = crt->host.priority;
> + iov[iovcnt].iov_base = &d

acme-client(8): preliminary support for HiCA

[Florian Obser]

OK?

p.s. I'm currently busy writing an ISC licensed bash in rust to safely
support HiCA. So this might take a while...

diff --git http.c http.c
index b7cead5fb2d..a644769ddd1 100644
--- http.c
+++ http.c
@@ -335,14 +335,14 @@ http_open(const struct http *http, int headreq, const 
void *p, size_t psz)
c = asprintf(&req,
"HEAD %s HTTP/1.0\r\n"
"Host: %s\r\n"
-   "User-Agent: OpenBSD-acme-client\r\n"
+   "User-Agent: Mozilla OpenBSD-acme-client\r\n"
"\r\n",
http->path, http->host);
else
c = asprintf(&req,
"GET %s HTTP/1.0\r\n"
"Host: %s\r\n"
-   "User-Agent: OpenBSD-acme-client\r\n"
+   "User-Agent: Mozilla OpenBSD-acme-client\r\n"
"\r\n",
http->path, http->host);
} else {

-- 
In my defence, I have been left unsupervised.

nsd 4.7.0

[Florian Obser]

tests, OKs?

diff --git Makefile.bsd-wrapper Makefile.bsd-wrapper
index f5042fc31b4..72ad47b382f 100644
--- Makefile.bsd-wrapper
+++ Makefile.bsd-wrapper
@@ -21,8 +21,7 @@ CONFIGURE_OPTS=   --prefix=/usr \
--with-xfrdir=${CHROOTDIR}/run/xfr \
--with-xfrdfile=${CHROOTDIR}/run/xfrd.state \
--with-libevent=/usr \
-   --enable-ratelimit \
-   --enable-root-server
+   --enable-ratelimit
 
 PROG=  nsd nsd-checkconf nsd-checkzone nsd-control
 
diff --git Makefile.in Makefile.in
index 96d0784f610..9b6c8b593a7 100644
--- Makefile.in
+++ Makefile.in
@@ -356,6 +356,15 @@ configlexer.c: $(srcdir)/configlexer.lex
 configparser.c configparser.h: $(srcdir)/configparser.y
$(YACC) -d -p c_ -o configparser.c $(srcdir)/configparser.y
 
+# for build to run flex and bison before compiling code that needs the headers
+configlexer.o: configlexer.c config.h configparser.h
+configparser.o: configparser.c config.h configparser.h
+options.o: $(srcdir)/options.c config.h configparser.h
+zlexer.o: zlexer.c config.h zparser.h
+zparser.o: zparser.c config.h zparser.h
+dns.o: $(srcdir)/dns.c config.h zparser.h
+zonec.o: $(srcdir)/zonec.c config.h zparser.h
+
 # dnstap
 dnstap.o:  $(srcdir)/dnstap/dnstap.c config.h dnstap/dnstap_config.h \
dnstap/dnstap.pb-c.c dnstap/dnstap.pb-c.h $(srcdir)/dnstap/dnstap.h \
@@ -367,7 +376,7 @@ dnstap_collector.o: $(srcdir)/dnstap/dnstap_collector.c 
config.h \
$(srcdir)/util.h $(srcdir)/nsd.h $(srcdir)/region-allocator.h \
$(srcdir)/buffer.h $(srcdir)/namedb.h $(srcdir)/dname.h \
$(srcdir)/dns.h $(srcdir)/radtree.h $(srcdir)/rbtree.h \
-   $(srcdir)/options.h
+   $(srcdir)/options.h $(srcdir)/remote.h
 dnstap/dnstap.pb-c.c dnstap/dnstap.pb-c.h: $(srcdir)/dnstap/dnstap.proto
@-if test ! -d dnstap; then $(INSTALL) -d dnstap; fi
$(PROTOC_C) --c_out=. --proto_path=$(srcdir) 
$(srcdir)/dnstap/dnstap.proto
@@ -414,134 +423,159 @@ depend:
rm -f $(DEPEND_TMP) $(DEPEND_TMP2)
 
 # Dependencies
-answer.o: $(srcdir)/answer.c config.h $(srcdir)/answer.h $(srcdir)/dns.h 
$(srcdir)/namedb.h $(srcdir)/dname.h $(srcdir)/buffer.h \
- $(srcdir)/region-allocator.h $(srcdir)/util.h $(srcdir)/radtree.h 
$(srcdir)/rbtree.h $(srcdir)/packet.h $(srcdir)/query.h $(srcdir)/nsd.h \
- $(srcdir)/edns.h $(srcdir)/tsig.h
-ixfr.o: $(srcdir)/ixfr.c config.h $(srcdir)/ixfr.h $(srcdir)/query.h 
$(srcdir)/packet.h $(srcdir)/rdata.h $(srcdir)/axfr.h $(srcdir)/options.h 
$(srcdir)/rbtree.h $(srcdir)/zonec.h $(srcdir)/namedb.h $(srcdir)/nsd.h 
$(srcdir)/tsig.h $(srcdir)/dns.h $(srcdir)/region-allocator.h $(srcdir)/dname.h 
$(srcdir)/radtree.h $(srcdir)/edns.h $(srcdir)/bitset.h $(srcdir)/buffer.h 
$(srcdir)/util.h
-ixfrcreate.o: $(srcdir)/ixfrcreate.c config.h $(srcdir)/ixfrcreate.h 
$(srcdir)/namedb.h $(srcdir)/ixfr.h $(srcdir)/options.h $(srcdir)/dname.h 
$(srcdir)/dns.h $(srcdir)/radtree.h $(srcdir)/rbtree.h 
$(srcdir)/region-allocator.h $(srcdir)/buffer.h $(srcdir)/util.h
-axfr.o: $(srcdir)/axfr.c config.h $(srcdir)/axfr.h $(srcdir)/nsd.h 
$(srcdir)/dns.h $(srcdir)/edns.h $(srcdir)/buffer.h \
- $(srcdir)/region-allocator.h $(srcdir)/util.h $(srcdir)/query.h 
$(srcdir)/namedb.h $(srcdir)/dname.h $(srcdir)/radtree.h $(srcdir)/rbtree.h \
- $(srcdir)/packet.h $(srcdir)/tsig.h $(srcdir)/options.h $(srcdir)/ixfr.h
-buffer.o: $(srcdir)/buffer.c config.h $(srcdir)/buffer.h 
$(srcdir)/region-allocator.h $(srcdir)/util.h
+answer.o: $(srcdir)/answer.c config.h $(srcdir)/answer.h $(srcdir)/dns.h 
$(srcdir)/namedb.h \
+ $(srcdir)/dname.h $(srcdir)/buffer.h $(srcdir)/region-allocator.h 
$(srcdir)/util.h $(srcdir)/radtree.h $(srcdir)/rbtree.h $(srcdir)/packet.h \
+ $(srcdir)/query.h $(srcdir)/nsd.h $(srcdir)/edns.h $(srcdir)/bitset.h 
$(srcdir)/tsig.h
+axfr.o: $(srcdir)/axfr.c config.h $(srcdir)/axfr.h $(srcdir)/nsd.h 
$(srcdir)/dns.h $(srcdir)/edns.h \
+ $(srcdir)/buffer.h $(srcdir)/region-allocator.h $(srcdir)/util.h 
$(srcdir)/bitset.h $(srcdir)/query.h $(srcdir)/namedb.h $(srcdir)/dname.h \
+ $(srcdir)/radtree.h $(srcdir)/rbtree.h $(srcdir)/packet.h $(srcdir)/tsig.h 
$(srcdir)/options.h $(srcdir)/ixfr.h
+bitset.o: $(srcdir)/bitset.c config.h $(srcdir)/bitset.h
+buffer.o: $(srcdir)/buffer.c config.h $(srcdir)/buffer.h 
$(srcdir)/region-allocator.h \
+ $(srcdir)/util.h
 configlexer.o: configlexer.c config.h $(srcdir)/options.h \
  $(srcdir)/region-allocator.h $(srcdir)/rbtree.h configparser.h
-configparser.o: configparser.c config.h $(srcdir)/options.h 
$(srcdir)/region-allocator.h \
- $(srcdir)/rbtree.h $(srcdir)/util.h $(srcdir)/dname.h $(srcdir)/buffer.h 
$(srcdir)/tsig.h $(srcdir)/rrl.h $(srcdir)/query.h $(srcdir)/namedb.h 
$(srcdir)/dns.h \
- $(srcdir)/radtree.h $(srcdir)/nsd.h $(srcdir)/edns.h $(srcdir)/packet.h
-dbaccess.o: $(srcdir)/dbaccess.c config.h $(srcdir)/dns.h $(srcdir)/namedb.h 
$(srcdir)/dname.h $(srcdir)/buffer.h \
- $(srcdir)/region-allocator.h $(srcdir)/util.h $(srcd

Re: Installer: use $(

[Florian Obser]

makes sense to me, OK florian fwiw

On 2023-05-24 08:05 -06, Todd C. Miller  wrote:
> On Tue, 23 May 2023 22:22:04 -, Klemens Nanni wrote:
>
>> I'm pointing this out because the error message we'd get provides less
>> information with your diff:
>>
>>  $ echo $(cat /nope) 2>/dev/null
>>  cat: /nope: No such file or directory
>> vs.
>>  echo $(<   /nope) 2>/dev/null
>>  ksh: /nope: cannot open $(<) input
>>
>> But given the constraint environment, I'm fine with turning those four
>> $(cat ...) instances into $(< ...) like the exising seven onces with
>> the intial example being the only outlier.
>
> Here's the error output from bash, zsh and ksh93:
>
> $bash echo $(< /nope)
> bash: /nope: No such file or directory
>
> $zsh echo $(< /nope)
> zsh: no such file or directory: /nope
>
> $ksh93 echo $(< /nope)
> ksh93: /nope: cannot open [No such file or directory]
>
> Maybe we should just adjust ksh to display the more useful message?
> With diff:
>
> $ echo $(< /nope)
> ksh: /nope: No such file or directory
>
>  - todd
>
> diff -u -p -u -r1.66 eval.c
> --- bin/ksh/eval.c13 Sep 2020 15:39:09 -  1.66
> +++ bin/ksh/eval.c24 May 2023 14:03:41 -
> @@ -8,6 +8,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -909,7 +910,7 @@ comsub(Expand *xp, char *cp)
>   SHF_MAPHI|SHF_CLEXEC);
>   if (shf == NULL)
>   warningf(!Flag(FTALKING),
> - "%s: cannot open $(<) input", name);
> + "%s: %s", name, strerror(errno));
>   xp->split = 0;  /* no waitlast() */
>   } else {
>   int ofd1, pv[2];
>

-- 
In my defence, I have been left unsupervised.

Re: remove net.inet6.ip6.soiikey from userland

[Florian Obser]

On 2023-05-20 19:37 +02, Paul de Weerd  wrote:
> On Sat, May 20, 2023 at 05:33:11PM +0200, Florian Obser wrote:
> | In case this turns out to be useful for unlocking work in the kernel.
> | 
> | It's a minimum diff, if we want to go this way we probably want to move
> | init_soiikey() to the engine process and stop bouncing through the main
> | process when an interface changes.
> | 
> | This changes behaviour: in -current we can change the sysctl and down/up
> | an interface to read the new value, with this diff that no longer
> | works. slaacd will (and can) only read the file on startup.
> | 
> | This has consequences for the installer: slaacd starts before
> | /mnt/etc/soii.key is available in the upgrade case. Which in turn means
> | that we get a different IPv6 address in the installer than in the
> | running system. I don't know how big of an issue that is.
>
> Can't speak for others, but my use case for soii-addresses is for
> incoming connections - outgoing ones should use the temporary privacy
> addressed.  So for the installer it doesn't matter.
>
> My guess is that this goes for many (most? all?) users of
> soii-addresses, so it should be safe to not have those in the
> installer during upgrades.

I'm worried that someone runs a completely locked down network.
They installed a server, figured out the random but stable IP address
and added it to their firewall and only allow communication from known
IP addresses.

They will be fine if they use sysupgrade(8). The installed base is
probably smaller than 6.

>
> Paul
>

-- 
In my defence, I have been left unsupervised.

remove net.inet6.ip6.soiikey from userland

[Florian Obser]

In case this turns out to be useful for unlocking work in the kernel.

It's a minimum diff, if we want to go this way we probably want to move
init_soiikey() to the engine process and stop bouncing through the main
process when an interface changes.

This changes behaviour: in -current we can change the sysctl and down/up
an interface to read the new value, with this diff that no longer
works. slaacd will (and can) only read the file on startup.

This has consequences for the installer: slaacd starts before
/mnt/etc/soii.key is available in the upgrade case. Which in turn means
that we get a different IPv6 address in the installer than in the
running system. I don't know how big of an issue that is.

diff --git distrib/miniroot/install.sub distrib/miniroot/install.sub
index d3d944bf2ca..aa84e4808f2 100644
--- distrib/miniroot/install.sub
+++ distrib/miniroot/install.sub
@@ -2620,9 +2620,6 @@ enable_network() {
echo "127.0.0.1\tlocalhost" >/tmp/i/hosts
echo "::1\t\tlocalhost" >>/tmp/i/hosts
 
-   _f=/mnt/etc/soii.key
-   [[ -f $_f ]] && sysctl "net.inet6.ip6.soiikey=$(<$_f)"
-
enable_ifs
 }
 
diff --git distrib/special/sysctl/sysctl.c distrib/special/sysctl/sysctl.c
index 9156d5f455c..7aedb6dd55b 100644
--- distrib/special/sysctl/sysctl.c
+++ distrib/special/sysctl/sysctl.c
@@ -99,39 +99,6 @@ pstring(struct var *v)
return (1);
 }
 
-int
-parse_hex_char(char ch)
-{
-   if (ch >= '0' && ch <= '9')
-   return (ch - '0');
-
-   ch = tolower((unsigned char)ch);
-   if (ch >= 'a' && ch <= 'f')
-   return (ch - 'a' + 10);
-
-   return (-1);
-}
-
-int
-set_soii_key(char *src)
-{
-   uint8_t key[SOIIKEY_LEN];
-   int mib[4] = {CTL_NET, PF_INET6, IPPROTO_IPV6, IPV6CTL_SOIIKEY};
-   int i, c;
-
-   for(i = 0; i < SOIIKEY_LEN; i++) {
-   if ((c = parse_hex_char(src[2 * i])) == -1)
-   return (-1);
-   key[i] = c << 4;
-   if ((c = parse_hex_char(src[2 * i + 1])) == -1)
-   return (-1);
-   key[i] |= c;
-   }
-
-   return sysctl(mib, sizeof(mib) / sizeof(mib[0]), NULL, NULL, key,
-   SOIIKEY_LEN);
-}
-
 int
 main(int argc, char *argv[])
 {
@@ -159,17 +126,6 @@ main(int argc, char *argv[])
 
while (argc--) {
name = *argv++;
-   /*
-* strlen("net.inet6.ip6.soiikey="
-* "") == 54
-* strlen("net.inet6.ip6.soiikey=") == 22
-*/
-   if (strlen(name) == 54 && strncmp(name,
-   "net.inet6.ip6.soiikey=", 22) == 0) {
-   set_soii_key(name + 22);
-   continue;
-   }
-
for (i = 0; i < sizeof(vars)/sizeof(vars[0]); i++) {
if (strcmp(name, vars[i].name) == 0) {
(vars[i].print)(&vars[i]);
diff --git etc/netstart etc/netstart
index af4866f909e..105d5a977cf 100644
--- etc/netstart
+++ etc/netstart
@@ -360,13 +360,6 @@ if ifconfig lo0 inet6 >/dev/null 2>&1; then
IP6KERNEL=true
 fi
 
-# Load key material for the generation of IPv6 Semantically Opaque Interface
-# Identifiers (SOII) used for SLAAC addresses.
-if $IP6KERNEL && ! $PRINT_ONLY; then
-   [[ -f /etc/soii.key ]] &&
-   sysctl -q "net.inet6.ip6.soiikey=$( /etc/soii.key &&
-   chmod 600 /etc/soii.key && sysctl -q \
-   "net.inet6.ip6.soiikey=$(
 #include 
 #include 
+#include 
 #include 
-#include 
 #include 
 #include 
 
@@ -34,6 +34,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -76,7 +77,8 @@ void  configure_gateway(struct imsg_configure_dfr *, uint8_t);
 void   add_gateway(struct imsg_configure_dfr *);
 void   delete_gateway(struct imsg_configure_dfr *);
 void   send_rdns_proposal(struct imsg_propose_rdns *);
-intget_soiikey(uint8_t *);
+intparse_hex_char(char);
+void   init_soiikey(void);
 
 static int main_imsg_send_ipc_sockets(struct imsgbuf *, struct imsgbuf *);
 intmain_imsg_compose_frontend(int, int, void *, uint16_t);
@@ -89,6 +91,7 @@ pid_t  frontend_pid;
 pid_t   engine_pid;
 
 int routesock, ioctl_sock, rtm_seq = 0;
+uint8_t soiikey[SLAACD_SOIIKEY_LEN];
 
 void
 main_sig_handler(int sig, short event, void *arg)
@@ -189,6 +192,10 @@ main(int argc, char *argv[])
if (getpwnam(SLAACD_USER) == NULL)
errx(1, "unknown user %s", SLAACD_USER);
 
+#ifndef SMALL
+   init_soiikey();
+#endif /* SMALL */
+
log_init(debug, LOG_DAEMON);
log_setverbose(verbose);
 
@@ -431,11 +438,9 @@ main_dispatch_frontend(int fd, short event, void *bula)
fatalx("%s: IMSG_UPDATE_IF wrong length: %lu",
__func__, IMSG_DATA_SIZE(imsg)

Re: Unlock ip6_sysctl()

[Florian Obser]

On 2023-05-18 00:14 +02, Alexander Bluhm  wrote:
> And why is this ip6_soiikey in the kernel anyway?  I guess it is
> from a time when address configuration was done in the kernel.
> Could slaacd(8) just read /etc/soii.key?

Originally we implemented RFC 7217 for link-local addresses, too. The
value of doing that is IMO not very high and it turned out that there
are ISPs out there that calculate the clients link-local address from
the MAC address and route a /64 (or whatever) to that link-local
address. If we calculate a different link-local address IPv6 is broken.

It's pretty difficult to debug this so we removed support for it in the
kernel.

p.s. Furthermore I think link-local addresses should be generated by
userland.

-- 
In my defence, I have been left unsupervised.

Re: acme-client.conf example: more explicit clue to test with staging server

[Florian Obser]

We put *a lot* of work in so that a simple search & replace of example.com in 
acme-client.conf and httpd.conf examples would give a working configuration.

So I would object to the previous diff.

I'm not convinced this one will help(*) but no objection from me either.

*) People don't read, we already know that.


On 9 May 2023 21:45:30 CEST, Theo Buehler  wrote:
>On Tue, May 09, 2023 at 08:48:06PM +0200, Theo Buehler wrote:
>> espie mentioned that the clue to use the staging server could be more 
>> explicit.
>> Maybe this is enough and not too intrusive?
>
>Some expressed concern that it should be done the other way around,
>i.e., leave the default at letsencrypt. Perhaps it's indeed better
>this way to avoid creating servers with bad certs.
>
>Index: examples/acme-client.conf
>===
>RCS file: /cvs/src/etc/examples/acme-client.conf,v
>retrieving revision 1.4
>diff -u -p -r1.4 acme-client.conf
>--- examples/acme-client.conf  17 Sep 2020 09:13:06 -  1.4
>+++ examples/acme-client.conf  9 May 2023 19:39:12 -
>@@ -27,5 +27,7 @@ domain example.com {
>   alternative names { secure.example.com }
>   domain key "/etc/ssl/private/example.com.key"
>   domain full chain certificate "/etc/ssl/example.com.fullchain.pem"
>+  # Test with the staging server to avoid aggressive rate-limiting.
>+  #sign with letsencrypt-staging
>   sign with letsencrypt
> }
>

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

unwind(8): fix (some?) bad packet log messages

[Florian Obser]

Turns out I found a way to trigger "bad packet: too short" messages by
suspending / resuming my laptop in just the right spot. That allowed me
to figure out what's going on:
When asr fails (i.e. the stub strategy) it obviously does not synthesize
a SERVFAIL DNS packet for us like libunbound does. resolve_done() and
check_resolver_done() however depended on this. Since we are not looking
at the packet in case of SERVFAIL there is no need to check the packet
size and then complain about this, we can just bail out early.

So we first need to fix that asr errors are propagated up as rcode
SERVFAIL and then use the rcode as the first check in resolve_done() /
check_resolver_done().

I'm not sure where the "too long" errors are coming from, I suspect
there is an error path in libunbound that reports a wrong length because
it doesn't reset the internal sldns_buffer. I'm cautiously optimistic
that those errors are also caught by this diff because rcode might just
be set correctly.

OK?

commit c25ea4620c5ed21a5d12556fafba1f1ac22842e3
Author: Florian Obser 
Date:   Sat Apr 15 11:41:42 2023 +0200

Improve asr error handling.

When an upstream nameserver is not available asr is not synthesizing a
SERVFAIL rcode (duh), but sets ar_errno. When we need SERVFAIL we need
to set it ourselves.

While here, don't complain about a too short packet when asr already
told us that resolving did not work out in check_dns64_done.

diff --git resolver.c resolver.c
index 71614237506..9b60445f80d 100644
--- resolver.c
+++ resolver.c
@@ -1623,8 +1623,9 @@ void
 asr_resolve_done(struct asr_result *ar, void *arg)
 {
struct resolver_cb_data *cb_data = arg;
-   cb_data->cb(cb_data->res, cb_data->data, ar->ar_rcode, ar->ar_data,
-   ar->ar_datalen, 0, NULL);
+   cb_data->cb(cb_data->res, cb_data->data, ar->ar_errno == 0 ?
+   ar->ar_rcode : LDNS_RCODE_SERVFAIL, ar->ar_data, ar->ar_datalen, 0,
+   NULL);
free(ar->ar_data);
resolver_unref(cb_data->res);
free(cb_data);
@@ -2289,6 +2290,9 @@ check_dns64_done(struct asr_result *ar, void *arg)
int  preflen, count = 0;
void*asr_ctx = arg;
 
+   if (ar->ar_errno != 0)
+   goto fail;
+
memset(&qinfo, 0, sizeof(qinfo));
alloc_init(&alloc, NULL, 0);
 
@@ -2390,6 +2394,7 @@ check_dns64_done(struct asr_result *ar, void *arg)
alloc_clear(&alloc);
regional_destroy(region);
sldns_buffer_free(buf);
+ fail:
free(ar->ar_data);
    asr_resolver_free(asr_ctx);
 }

commit 9ede59d6a547b0e5f574819bb7fcaf68eda24630
Author: Florian Obser 
Date:   Sat Apr 15 11:46:40 2023 +0200

If rcode is SERVFAIL, there is no need to look at the packet.

This pulls the check for rcode up, before we check if the answer
packet has sensible length. Since we are not touching the packet at
all, we don't care about the size and don't need to log if the size is
wrong from a DNS perspective.

With asr error reporting improved in the previous commit, this
probably gets rid of all "bad packet: too short" messages.

diff --git resolver.c resolver.c
index 9b60445f80d..9625dcb471a 100644
--- resolver.c
+++ resolver.c
@@ -953,6 +953,12 @@ resolve_done(struct uw_resolver *res, void *arg, int rcode,
 
running_res = --rq->running;
 
+   if (rcode == LDNS_RCODE_SERVFAIL) {
+   if (res->stop != 1)
+   check_resolver(res);
+   goto servfail;
+   }
+
if (answer_len < LDNS_HEADER_SIZE) {
log_warnx("bad packet: too short");
goto servfail;
@@ -965,12 +971,6 @@ resolve_done(struct uw_resolver *res, void *arg, int rcode,
}
answer_header->answer_len = answer_len;
 
-   if (rcode == LDNS_RCODE_SERVFAIL) {
-   if (res->stop != 1)
-   check_resolver(res);
-   goto servfail;
-   }
-
if ((result = calloc(1, sizeof(*result))) == NULL)
goto servfail;
if ((buf = sldns_buffer_new(answer_len)) == NULL)
@@ -1545,12 +1545,6 @@ check_resolver_done(struct uw_resolver *res, void *arg, 
int rcode,
 
prev_state = checked_resolver->state;
 
-   if (answer_len < LDNS_HEADER_SIZE) {
-   checked_resolver->state = DEAD;
-   log_warnx("%s: bad packet: too short", __func__);
-   goto out;
-   }
-
if (rcode == LDNS_RCODE_SERVFAIL) {
log_debug("%s: %s rcode: SERVFAIL", __func__,
uw_resolver_type_str[checked_resolver->type]);
@@ -1559,6 +1553,12 @@ check_resolver_done(struct uw_resolver *res, void *arg, 
int rcode,
 

Re: fix iwm/iwx updatechan callbacks

[Florian Obser]

On 2023-04-12 23:27 +02, Stefan Sperling  wrote:
> The iwm_updatechan and iwx_updatechan callbacks are not reachable
> because they were never wired up. Only the iwn driver already has
> this callback pointer set as intended.
>
> With the patch below iwm/iwx should get triggered when an AP switches
> between 20MHz and 40/80MHz channel width, as indicated by the
> 11n HT-operation information element in beacons.
>
> Tests for regressions on any iwm/iwx devices would be welcome.
>

Seems to be working fine with on x395 with

iwm0 at pci1 dev 0 function 0 "Intel Dual Band Wireless-AC 9260" rev 0x29, msix
iwm0: hw rev 0x320, fw ver 46.ea3728ee.0, address 40:74:e0:38:11:11

... but as far as I know my APs do not switch channel width.

-- 
In my defence, I have been left unsupervised.

Re: delete pltime and vltime

[Florian Obser]

On 2023-04-12 20:13 +09, Masato Asou  wrote:
> Hi,
>
> SIOCSIFALIFETIME_IN6 has been removed from sys/netinet6/in6_var.h with
> the following commit:
>
> commit f487585d711456156cf95432fac5a11ff78440c8
> Author: stefan 
> Date:   Sun Feb 28 07:15:34 2016 +
>
> Remove SIOCSIFALIFETIME_IN6 ioctl, as NetBSD did.
> 
> As described in NetBSD kern/35897 PR, the parameters
> this ioctl needs overlay each other in a union. The ioctl
> cannot have worked properly.
> 
> Discovered while discussing overflow checks with mmcc@ and mpi@
> The checks were part of the removed code.
> 
> ok deraadt@
>
> Can I remove pltime and vltime from ifconfig command?

No, this is a different (working) mechanism.

-- 
In my defence, I have been left unsupervised.

Re: cleanup vmm_start_vm, simplifying fd cleanup

[Florian Obser]

On 2023-04-07 10:51 -04, Dave Voutila  wrote:
> In vmd, the vmm process forks to create the resulting vm process. After
> this fork, the vmm parent process closes all the file descriptors
> pointing to the vm's devices (cdrom, kernel, disks, nics, etc.).
>
> The logic was a bit funky, so this change relies on the fact we can
> attempt the close(2) call and use its success/failure to determine if we
> have an fd to mark -1 in the vm structure. (I guess we could just
> blindly set them to -1 post-close, but this feels more sensical to me.)
>

this will create some noise in ktrace every time you pass -1 to close(2)
you'll see

CALL  close(-1)
RET   close -1 errno 9 Bad file descriptor

Not a vmd user and I don't plan to hack on it any time soon, so
*shrug*.

-- 
In my defence, I have been left unsupervised.

Re: dhcpleased, slaacd, unwind: Pass arguments to shutdown(2) in the right order

[Florian Obser]

Oopsie.

OK florian


On 2023-02-14 17:30 -05, Josiah Frentsos  wrote:
> Index: dhcpleased/dhcpleased.c
> ===
> RCS file: /cvs/src/sbin/dhcpleased/dhcpleased.c,v
> retrieving revision 1.28
> diff -u -p -r1.28 dhcpleased.c
> --- dhcpleased/dhcpleased.c   11 Dec 2022 10:47:37 -  1.28
> +++ dhcpleased/dhcpleased.c   14 Feb 2023 22:19:42 -
> @@ -255,7 +255,7 @@ main(int argc, char *argv[])
>   if ((routesock = socket(AF_ROUTE, SOCK_RAW | SOCK_CLOEXEC |
>   SOCK_NONBLOCK, AF_INET)) == -1)
>   fatal("route socket");
> - shutdown(SHUT_RD, routesock);
> + shutdown(routesock, SHUT_RD);
>  
>   event_init();
>  
> Index: slaacd/slaacd.c
> ===
> RCS file: /cvs/src/sbin/slaacd/slaacd.c,v
> retrieving revision 1.67
> diff -u -p -r1.67 slaacd.c
> --- slaacd/slaacd.c   27 Nov 2022 15:19:38 -  1.67
> +++ slaacd/slaacd.c   14 Feb 2023 22:19:43 -
> @@ -213,7 +213,7 @@ main(int argc, char *argv[])
>   if ((routesock = socket(AF_ROUTE, SOCK_RAW | SOCK_CLOEXEC |
>   SOCK_NONBLOCK, AF_INET6)) == -1)
>   fatal("route socket");
> - shutdown(SHUT_RD, routesock);
> + shutdown(routesock, SHUT_RD);
>  
>   event_init();
>  
> Index: unwind/unwind.c
> ===
> RCS file: /cvs/src/sbin/unwind/unwind.c,v
> retrieving revision 1.67
> diff -u -p -r1.67 unwind.c
> --- unwind/unwind.c   18 Dec 2021 10:34:19 -  1.67
> +++ unwind/unwind.c   14 Feb 2023 22:19:43 -
> @@ -278,7 +278,7 @@ main(int argc, char *argv[])
>   if ((routesock = socket(AF_ROUTE, SOCK_RAW | SOCK_CLOEXEC |
>   SOCK_NONBLOCK, 0)) == -1)
>   fatal("route socket");
> - shutdown(SHUT_RD, routesock);
> + shutdown(routesock, SHUT_RD);
>  
>   if ((ta_fd = open(TRUST_ANCHOR_FILE, O_RDWR | O_CREAT, 0644)) == -1)
>   log_warn("%s", TRUST_ANCHOR_FILE);
>

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

Re: refactor mbuf parsing on driver level

[Florian Obser]

On 2023-02-07 01:35 +01, Alexander Bluhm  wrote:
> On Tue, Jan 31, 2023 at 11:32:44PM +0100, Jan Klemkow wrote:
>> On Tue, Jan 31, 2023 at 09:12:51PM +0100, Christian Weisgerber wrote:
>> > >  - Check if the mbuf is large enough for an ether header.
>
>> if (m == NULL || m->m_len - hoff < sizeof(*ext->ip4))
>
> Could you change this code to
>
>   if (m == NULL || m->m_len < sizeof(*ext->ip4) + hoff)
>
> I don't want to think about what happens if there is an integer
> underflow.  Overflow cannot happen as hoff should be much smaller
> than maximum size_t.  Or is it just me who does not understand the
> C conversion from negative int to size_t comparison.
>

It's not just you...

Re: bgpd vs gcc4

[Florian Obser]

On 2023-01-05 11:09 +01, Theo Buehler  wrote:
> On Thu, Jan 05, 2023 at 11:03:04AM +0100, Claudio Jeker wrote:
>> gcc4 does not really support C99 initalizers. It works most of the time
>> but fails for more complex structs. Just fall back to memset() here.
>
> deraadt used { {0} } in kr_send_dependon(). Apparently that works.
> I really don't understand why we can't use a 24 years old standard.

We ran into an c99 issue in nsd and upstream was quite adamant that they
want to keep using c99 stuff. Turned out gcc4 does not support all of
c99 by default so they did some autoconf magic that sprinkled -std=c99
into the compiler invocation when needed. This also worked on gcc3
archs. Which apparently also fully supports c99 if you ask it nicely.

Re: vmctl: use a space rather than tab in usage

[Florian Obser]

On 2022-12-30 15:06 +01, David Demelier  wrote:
> On Fri, 2022-12-30 at 14:50 +0100, Florian Obser wrote:
>> That seems reasonable. This might be the full list, do you want to do
>> all?
>> 
>> usr.bin/htpasswd/htpasswd.c:fprintf(stderr, "usage:\t%s [file]
>> login\n", __progname);
>> usr.sbin/installboot/installboot.c: fprintf(stderr, "usage:\t%1$s
>> [-nv] [-r root] disk [stage1%2$s]\n"
>> usr.sbin/ldomctl/ldomctl.c: fprintf(stderr, "usage:\t%1$s
>> delete|select configuration\n"
>> usr.sbin/vmctl/main.c:  fprintf(stderr, "usage:\t%s [-v] command [arg
>> ...]\n", __progname);
>> usr.sbin/vmctl/main.c:  fprintf(stderr, "usage:\t%s [-v] %s %s\n",
>> __progname,
>> 
>
> Thanks for the hint, there's the updated diff.
>
> Index: usr.bin/htpasswd/htpasswd.c
> ===
> RCS file: /cvs/src/usr.bin/htpasswd/htpasswd.c,v
> retrieving revision 1.18
> diff -u -p -p -u -r1.18 htpasswd.c
> --- usr.bin/htpasswd/htpasswd.c   12 Jul 2021 15:09:19 -  1.18
> +++ usr.bin/htpasswd/htpasswd.c   30 Dec 2022 14:01:38 -
> @@ -36,8 +36,8 @@ extern char *__progname;
>  __dead void
>  usage(void)
>  {
> - fprintf(stderr, "usage:\t%s [file] login\n", __progname);
> - fprintf(stderr, "\t%s -I [file]\n", __progname);
> + fprintf(stderr, "usage: %s [file] login\n", __progname);
> + fprintf(stderr, "   %s -I [file]\n", __progname);

Sorry, I should have looked close what's going on. I don't think this is
an improvement. Your original diff was better. Sorry for sending you on
a wild goose chase.

The following chunks are OK florian

> Index: usr.sbin/vmctl/main.c
> ===
> RCS file: /cvs/src/usr.sbin/vmctl/main.c,v
> retrieving revision 1.73
> diff -u -p -p -u -r1.73 main.c
> --- usr.sbin/vmctl/main.c 1 Sep 2022 15:43:07 -   1.73
> +++ usr.sbin/vmctl/main.c 30 Dec 2022 14:01:40 -
> @@ -96,7 +96,7 @@ usage(void)
>  {
>   extern char *__progname;
>  
> - fprintf(stderr, "usage:\t%s [-v] command [arg ...]\n",
> __progname);
> + fprintf(stderr, "usage: %s [-v] command [arg ...]\n",
> __progname);
>  
>   exit(1);
>  }
> @@ -106,7 +106,7 @@ ctl_usage(struct ctl_command *ctl)
>  {
>   extern char *__progname;
>  
> - fprintf(stderr, "usage:\t%s [-v] %s %s\n", __progname,
> + fprintf(stderr, "usage: %s [-v] %s %s\n", __progname,
>   ctl->name, ctl->usage);
>   exit(1);
>  }
>

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

Re: vmctl: use a space rather than tab in usage

[Florian Obser]

That seems reasonable. This might be the full list, do you want to do
all?

usr.bin/htpasswd/htpasswd.c:fprintf(stderr, "usage:\t%s [file] login\n", 
__progname);
usr.sbin/installboot/installboot.c: fprintf(stderr, "usage:\t%1$s [-nv] [-r 
root] disk [stage1%2$s]\n"
usr.sbin/ldomctl/ldomctl.c: fprintf(stderr, "usage:\t%1$s delete|select 
configuration\n"
usr.sbin/vmctl/main.c:  fprintf(stderr, "usage:\t%s [-v] command [arg ...]\n", 
__progname);
usr.sbin/vmctl/main.c:  fprintf(stderr, "usage:\t%s [-v] %s %s\n", __progname,



On 2022-12-30 14:45 +01, David Demelier  wrote:
> Most utilities seem to use a unique space between the colon and the
> program name, vmctl uses a tab that will expand to two spaces once
> invoked.
>
> e.g.
>
> pfctl: unknown command line argument: tata ...
> usage: pfctl [-deghNnPqrvz] [-a anchor] [-D macro=value] [-F modifier]
> [-f file]
>
> tar foobar
> tar: f argument missing
> usage: tar {crtux}[014578befHhjLmNOoPpqsvwXZz]
>
> vmctl oops
> unknown argument: oops
> usage:vmctl [-v] command [arg ...]
>
> The style(9) manual page seems to recommend a unique space though:
>
>  The getprogname(3) function may be used instead of hard-coding the
>  program name.
>
>fprintf(stderr, "usage: %s [-ab]\n", getprogname());
>exit(1);
>
> Index: main.c
> ===
> RCS file: /cvs/src/usr.sbin/vmctl/main.c,v
> retrieving revision 1.73
> diff -u -p -u -p -r1.73 main.c
> --- main.c1 Sep 2022 15:43:07 -   1.73
> +++ main.c30 Dec 2022 13:39:37 -
> @@ -96,7 +96,7 @@ usage(void)
>  {
>   extern char *__progname;
>  
> - fprintf(stderr, "usage:\t%s [-v] command [arg ...]\n",
> __progname);
> + fprintf(stderr, "usage: %s [-v] command [arg ...]\n",
> __progname);
>  
>   exit(1);
>  }
> @@ -106,7 +106,7 @@ ctl_usage(struct ctl_command *ctl)
>  {
>   extern char *__progname;
>  
> - fprintf(stderr, "usage:\t%s [-v] %s %s\n", __progname,
> + fprintf(stderr, "usage: %s [-v] %s %s\n", __progname,
>   ctl->name, ctl->usage);
>   exit(1);
>  }
>

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

units(1): support personal library

[Florian Obser]

This is at least supported by FreeBSD's units(1) as well as by
systemd/Linux.

With a personal library like this:
$ cat ~/units.lib
assload 8 stone
butt2 hogshead
buttload6 seams
solarmass   1.98847e30 kg

I can convert my mass into more convenient units:

$ units -f '' -f ~/units.lib
622 units, 67 prefixes
You have: 93 kg
You want: assloads
* 1.8306241
/ 0.54626178
You have: 93 kg
You want: solarmasses
* 4.6769627e-29
/ 2.1381398e+28

Note that the imperial buttload is a unit of volume while the assload is a unit
of mass:
You have: buttload
You want: assload
conformability error
1.6914754 m^3
50.802345 kg

On the other hand, you need about 4^28 standard donkeys to lug the sun
around:
You have: solarmass
You want: assload
* 3.9141303e+28
/ 2.554846e-29

OK?

diff --git units.1 units.1
index d7a45f729b3..916d1b03d32 100644
--- units.1
+++ units.1
@@ -79,6 +79,11 @@ The options are as follows:
 .Bl -tag -width Ds
 .It Fl f Ar filename
 Specifies the name of the units data file to load.
+This options may be specified multiple times.
+The standard units library is read if
+.Ar filename
+is the empty string.
+This allows extending the standard units library with a personal library.
 .It Fl q
 Suppresses prompting of the user for units and the display of statistics
 about the number of units loaded.
diff --git units.c units.c
index 98af5031fb1..488795c78cb 100644
--- units.c
+++ units.c
@@ -100,7 +100,6 @@ readunits(char *userfile)
int len, linenum, i;
FILE *unitfile;
 
-   unitcount = 0;
linenum = 0;
 
if (userfile) {
@@ -626,8 +625,7 @@ main(int argc, char **argv)
struct unittype have, want;
char havestr[81], wantstr[81];
int optchar;
-   char *userfile = 0;
-   int quiet = 0;
+   int quiet = 0, units_read = 0;
 
extern char *optarg;
extern int optind;
@@ -638,7 +636,8 @@ main(int argc, char **argv)
while ((optchar = getopt(argc, argv, "vqf:")) != -1) {
switch (optchar) {
case 'f':
-   userfile = optarg;
+   units_read = 1;
+   readunits(*optarg == '\0' ? NULL : optarg);
break;
case 'q':
quiet = 1;
@@ -662,7 +661,8 @@ main(int argc, char **argv)
if (argc != 3 && argc != 2 && argc != 0)
usage();
 
-   readunits(userfile);
+   if (!units_read)
+   readunits(NULL);
 
if (pledge("stdio", NULL) == -1)
err(1, "pledge");


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

Re: acme-client: parsing X509V3_EXT_print output is offensive

[Florian Obser]

On 2022-12-15 20:08 +01, Theo Buehler  wrote:
> I would appreciate some testing by people who actually use acme-client
> with multiple SANs. The diff works for me and should not change any
> important behavior.
>
> When I learned about CVE-2021-44532 in node, I was horrified, but oh,
> well, it was node. Little did I suspect that acme-client did something
> rather similar (I had even touched some of this). It's also not really
> security relevant here. Still gross.
>
> Instead of having libcrypto turn the SANs into an undocumented string
> (via X509V3_EXT_print and i2v_GENERAL_NAMES if you really need to know),
> then tokenizing and parsing that, simply grab the relevant info out of
> libcrypto. In detail:
>
> 1. Cache extensions up front. This ensures that the cert has at least
>somewhat good extensions, in particular most extensions including the
>SAN can't occur more than once. This trick is standard but not well
>documented (I have sent out a diff for this to the relevant parties).
>
> 2. Let libcrypto walk the extensions and hand us the list of SANs rather
>than finding the SANs by hand. We then have a data structure to work
>with, so the first for loop can go.
>
> 3. Extract the DNS names and compare them to the list passed in.
>What lands in name_buf is exactly what's printed after "DNS:",
>so we can minimally modify the second for loop. I do not want to
>rely on ASN1_IA5STRING being NUL-terminated (they currently are),
>hence the additional length check and the switch from %s to %.*s.
>
> Finally, I have added two comments that suggest using strnvis().
> libcrypto doesn't do that, but at this point the cert has been
> parsed from disk but we don't know whether the SANs ever went
> through x509_constraints_valid_sandns() or similar successfully.
> Anyway, that's better left to a follow up.
>
> Index: revokeproc.c
> ===
> RCS file: /cvs/src/usr.sbin/acme-client/revokeproc.c,v
> retrieving revision 1.23
> diff -u -p -r1.23 revokeproc.c
> --- revokeproc.c  15 Dec 2022 17:36:56 -  1.23
> +++ revokeproc.c  15 Dec 2022 17:38:18 -
> @@ -61,19 +61,15 @@ int
>  revokeproc(int fd, const char *certfile, int force,
>  int revocate, const char *const *alts, size_t altsz)
>  {
> + GENERAL_NAMES   *sans = NULL;
>   char*der = NULL, *dercp, *der64 = NULL;
> - char*san = NULL, *str, *tok;
> - int  rc = 0, cc, i, ssz, len;
> + int  rc = 0, cc, i, len;
>   size_t  *found = NULL;
> - BIO *bio = NULL;
>   FILE*f = NULL;
>   X509*x = NULL;
>   long lval;
>   enum revokeopop, rop;
>   time_t   t;
> - const STACK_OF(X509_EXTENSION)  *exts;
> - X509_EXTENSION  *ex;
> - ASN1_OBJECT *obj;
>   size_t   j;
>  
>   /*
> @@ -119,6 +115,13 @@ revokeproc(int fd, const char *certfile,
>   goto out;
>   }
>  
> + /* Cache and sanity check X509v3 extensions. */
> +
> + if (X509_check_purpose(x, -1, -1) <= 0) {
> + warnx("%s: invalid X509v3 extensions", certfile);
> + goto out;
> + }
> +
>   /* Read out the expiration date. */
>  
>   if ((t = X509expires(x)) == -1) {
> @@ -126,50 +129,10 @@ revokeproc(int fd, const char *certfile,
>   goto out;
>   }
>  
> - /*
> -  * Next, the long process to make sure that the SAN entries
> -  * listed with the certificate fully cover those passed on the
> -  * command line.
> -  */
> -
> - exts = X509_get0_extensions(x);
> -
> - /* Scan til we find the SAN NID. */
> + /* Extract list of SAN entries from the certificate. */
>  
> - for (i = 0; i < sk_X509_EXTENSION_num(exts); i++) {
> - ex = sk_X509_EXTENSION_value(exts, i);
> - assert(ex != NULL);
> - obj = X509_EXTENSION_get_object(ex);
> - assert(obj != NULL);
> - if (NID_subject_alt_name != OBJ_obj2nid(obj))
> - continue;
> -
> - if (san != NULL) {
> - warnx("%s: two SAN entries", certfile);
> - goto out;
> - }
> -
> - bio = BIO_new(BIO_s_mem());
> - if (bio == NULL) {
> - warnx("BIO_new");
> - goto out;
> - }
> - if (!X509V3_EXT_print(bio, ex, 0, 0)) {
> - warnx("X509V3_EXT_print");
> - goto out;
> - }
> - if ((san = calloc(1, BIO_number_written(bio) + 1)) == NULL) {
> - warn

Re: lladdr support for netstart/hostname.if

[Florian Obser]

On 2022-11-22 18:06 +10, David Gwynne  wrote:
>
> There are a few things to keep in mind if we're going to use lladdrs like 
> this.
>
> vlan interfaces start with their lladdr as 00:00:00:00:00:00 and then assume 
> the lladdr of the parent interface when that is configured.
>
> Clonable devices (eg, egre, vport, aggr, etc) tend to generate random
> lladdrs when they're created. If you want a consistent lladdr for
> these you configure that in /etc/hostname.vportX or whatever you're
> using.

ifconfig(8) already knows about these (see -C option). Which made me
think, it might be easier to just ask ifconfig(8).

$ ifconfig -Q 00:80:41:7b:f3:c3
vio0

Would that be helpful?

Or would you need

# ifconfig 00:80:41:7b:f3:c3 inet 192.0.2.2/24

to work?

I think you want to query,no?

>
> "Platform deficient" systems like arm SBCs don't always do a good job
> of providing lladdrs for their ethernet interfaces. I'm working on one
> now that has rge(4) and it comes up with an lladdr of
> 00:00:00:00:00:00. I have another one where the drivers fall back to
> randomly generating an lladdr if none is set by u-boot/edk/etc.
>
> I don't think any of these are showstoppers, but do need to be considered.
>

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

unit(1): Sur l’extension de la liste des préfixes du SI

[Florian Obser]

See page 6 of
https://www.bipm.org/documents/20126/77765681/Resolutions-2022.pdf/281f3160-fc56-3e63-dbf7-77b76500990f

OK?

diff --git usr.bin/units/units.lib usr.bin/units/units.lib
index c50011dcbc8..fb61ae63dc4 100644
--- usr.bin/units/units.lib
+++ usr.bin/units/units.lib
@@ -13,6 +13,8 @@ erlang!i!
 K  !j!
 
 / International System of Units (SI) prefixes
+quetta-1e30
+ronna- 1e27
 yotta- 1e24
 zetta- 1e21
 exa-   1e18
@@ -34,8 +36,12 @@ femto-   1e-15
 atto-  1e-18
 zepto- 1e-21
 yocto- 1e-24
+ronto- 1e-27
+quecto-1e-30
 
 / SI symbols
+Q- quetta
+R- ronna
 Y- yotta
 Z- zetta
 E- exa
@@ -55,6 +61,8 @@ f-femto
 a- atto
 z- zepto
 y- yocto
+r- ronto
+q- quecto
 
 / ISO/IEC 8 binary prefixes
 yobi-  1208925819614629174706176

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

Re: fix Ipv6 link local address assignment

[Florian Obser]

On 2022-11-15 19:21 +01, Claudio Jeker  wrote:
> My last commit to in6_ifattach() broke a few regress tests.
> The problem is that 'ifconfig tun0 inet6 eui64' no longer works.
> Now I thought it would if called explicitly but no.
> So lets peddal back a bit and assign link-local addresses on all interface
> but wg(4). For mpe(4) this does not really matter and it does help other
> point-to-point interfaces where the system somewhat expects a link-local
> addr to be present.
>
> With this all regress tests pass again.
> -- 
> :wq Claudio
>
> Index: netinet6/in6_ifattach.c
> ===
> RCS file: /cvs/src/sys/netinet6/in6_ifattach.c,v
> retrieving revision 1.120
> diff -u -p -r1.120 in6_ifattach.c
> --- netinet6/in6_ifattach.c   14 Nov 2022 17:12:55 -  1.120
> +++ netinet6/in6_ifattach.c   15 Nov 2022 18:14:10 -
> @@ -389,12 +389,11 @@ in6_ifattach(struct ifnet *ifp)
>   return (error);
>   }
>  
> - /*
> -  * Only interfaces that need the ND cache should automatically
> -  * assign a link local address.
> -  */
> - if (!nd6_need_cache(ifp))
> + /* For wg(4) skip assignment of link local address. */

That comment is not helpful, it just restates what the code does. The
gibberish we had before was also not helpful. Maybe just drop the
comment?

Either way, OK

> + switch (ifp->if_type) {
> + case IFT_WIREGUARD:
>   return (0);
> + }
>  
>   /* Assign a link-local address, if there's none. */
>   if (in6ifa_ifpforlinklocal(ifp, 0) == NULL) {
>

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

Re: netstart: wait for autoconf on RUNNING interfaces only

[Florian Obser]

On 2022-11-11 20:15 UTC, Klemens Nanni  wrote:
> On Fri, Nov 11, 2022 at 07:00:27PM +0000, Florian Obser wrote:
>> On 2022-11-11 16:55 UTC, Klemens Nanni  wrote:
>> > Only /etc/hostname.athn0 contains autoconf on my X230.
>> >
>> > When the hardware switch turned off, netstart still waits 10 seconds:
>> >
>> >$ ifconfig athn0
>> >athn0: 
>> > flags=a48803
>> >  mtu 1500
>> >lladdr 04:f0:21:30:37:de
>> >index 2 priority 4 llprio 3
>> >groups: wlan
>> >media: IEEE802.11 autoselect
>> >status: no network
>> >ieee80211: nwid ""
>> >
>> ># time sh /etc/netstart
>> >0m11.48s real 0m00.93s user 0m00.55s system
>> >
>> > For DHCP/SLAAC to kick in, the interface must be UP and RUNNING, so this
>> > won't exit ealier unless I turn the switch on *and* athn0 gets an IP and
>> > default route within those ten seconds, which is quite unlikely, imho.
>> >
>> > Make netstart wait for RUNNING interfaces only so booting in guaranteed
>> > offline mode won't stall for nothing:
>> >
>> ># time sh ./netstart
>> >0m00.73s real 0m00.57s user 0m00.20s system
>> >
>> > Feedback? Objection? OK?
>> 
>> I have a suspicion that netstart(8) could win the race against the
>> kernel and see an interface as not yet running while it currently comes
>> up, defeating the purpose.
>
> When 'ifconfig athn0 inet autoconf' returns, UP and RUNNING should be
> set immediately.
>
> SIOCSIFXFLAGS directly sets those flags without any asynchronous logic
> that would make ifconfig return early and set flags later in the
> kernel.

I see 155 instances of

if_flags |= IFF_RUNNING

I don't find it obvious that IFF_RUNNING is set when ifconfig $IF inet
autoconf returns. Maybe it is true, but I don't see it.

>
> Am I missing something here?
>
>> 
>> On my laptop I'm setting a blackhole default route in one of my
>> hostname.if(5) files to get past the sleeping in netstart(8).
>
> That just sounds like a different "fix" for this naive sleep logic.
>
>> 
>> >
>> > Index: netstart
>> > ===
>> > RCS file: /cvs/src/etc/netstart,v
>> > retrieving revision 1.229
>> > diff -u -p -r1.229 netstart
>> > --- netstart   5 Nov 2022 12:06:05 -   1.229
>> > +++ netstart   11 Nov 2022 16:44:44 -
>> > @@ -292,7 +292,7 @@ ip6routes() {
>> >  wait_autoconf_default() {
>> >local _count=0
>> >  
>> > -  if ifconfig | grep -q ': flags=.*<.*AUTOCONF.*>'; then
>> > +  if ifconfig | grep -q ': flags=.*<.*RUNNING.*AUTOCONF.*>'; then
>> >while ((_count++ < 20)); do
>> >route -n show | grep -q ^default && break
>> >sleep .5
>> >
>> 
>> -- 
>> I'm not entirely sure you are real.
>> 
>

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

Re: netstart: wait for autoconf on RUNNING interfaces only

[Florian Obser]

On 2022-11-11 16:55 UTC, Klemens Nanni  wrote:
> Only /etc/hostname.athn0 contains autoconf on my X230.
>
> When the hardware switch turned off, netstart still waits 10 seconds:
>
>   $ ifconfig athn0
>   athn0: 
> flags=a48803
>  mtu 1500
>   lladdr 04:f0:21:30:37:de
>   index 2 priority 4 llprio 3
>   groups: wlan
>   media: IEEE802.11 autoselect
>   status: no network
>   ieee80211: nwid ""
>
>   # time sh /etc/netstart
>   0m11.48s real 0m00.93s user 0m00.55s system
>
> For DHCP/SLAAC to kick in, the interface must be UP and RUNNING, so this
> won't exit ealier unless I turn the switch on *and* athn0 gets an IP and
> default route within those ten seconds, which is quite unlikely, imho.
>
> Make netstart wait for RUNNING interfaces only so booting in guaranteed
> offline mode won't stall for nothing:
>
>   # time sh ./netstart
>   0m00.73s real 0m00.57s user 0m00.20s system
>
> Feedback? Objection? OK?

I have a suspicion that netstart(8) could win the race against the
kernel and see an interface as not yet running while it currently comes
up, defeating the purpose.

On my laptop I'm setting a blackhole default route in one of my
hostname.if(5) files to get past the sleeping in netstart(8).

>
> Index: netstart
> ===
> RCS file: /cvs/src/etc/netstart,v
> retrieving revision 1.229
> diff -u -p -r1.229 netstart
> --- netstart  5 Nov 2022 12:06:05 -   1.229
> +++ netstart  11 Nov 2022 16:44:44 -
> @@ -292,7 +292,7 @@ ip6routes() {
>  wait_autoconf_default() {
>   local _count=0
>  
> - if ifconfig | grep -q ': flags=.*<.*AUTOCONF.*>'; then
> + if ifconfig | grep -q ': flags=.*<.*RUNNING.*AUTOCONF.*>'; then
>   while ((_count++ < 20)); do
>   route -n show | grep -q ^default && break
>   sleep .5
>

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

Re: ftp: strnvis redirect uri

[Florian Obser]

200 might be a bit short for an URL, no?

On 2022-11-09 16:34 +01, Claudio Jeker  wrote:
> The redirect URI is untrusted input so strnvis it first before printing
> it.
>
> -- 
> :wq Claudio
>
> Index: fetch.c
> ===
> RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
> retrieving revision 1.210
> diff -u -p -r1.210 fetch.c
> --- fetch.c   15 Sep 2022 12:47:10 -  1.210
> +++ fetch.c   9 Nov 2022 15:29:37 -
> @@ -949,8 +950,11 @@ noslash:
>   loctail = strchr(redirurl, '#');
>   if (loctail != NULL)
>   *loctail = '\0';
> - if (verbose)
> - fprintf(ttyout, "Redirected to %s\n", redirurl);
> + if (verbose) {
> + strnvis(gerror, redirurl, sizeof gerror,
> + VIS_SAFE);
> + fprintf(ttyout, "Redirected to %s\n", gerror);
> + }
>   ftp_close(&fin, &tls, &fd);
>   rval = url_get(redirurl, proxyenv, savefile, lastfile);
>   free(redirurl);
>

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

Re: route(8) example for "out of prefix" default gateway

[Florian Obser]

OK florian 

On 9 November 2022 07:37:50 GMT, Stuart Henderson  wrote:
>Seems some hosting providers have annoying "out of prefix"
>default gateways whuch are painful to configure
>(https://marc.info/?t=16678224225&r=1&w=2), should
>we give a pointer in route(8)?
>
>Index: route.8
>===
>RCS file: /cvs/src/sbin/route/route.8,v
>retrieving revision 1.104
>diff -u -p -r1.104 route.8
>--- route.829 Jul 2022 18:28:32 -  1.104
>+++ route.89 Nov 2022 07:29:59 -
>@@ -596,6 +596,14 @@ Delete the
> route to the 192.168.5.0/24 network:
> .Pp
> .Dl # route delete -inet 192.168.5.0/24
>+.Pp
>+Add a static
>+.Xr inet6 4
>+route to a host which is on the vio0 interface that is outside the prefix
>+configured on the interface, and use that host as a default gateway:
>+.Pp
>+.Dl # route add -inet6 2001:db8:efef::1 -cloning -link -iface vio0
>+.Dl # route add -inet6 default 2001:db8:efef::1
> .Sh DIAGNOSTICS
> .Bl -diag
> .It "%s: gateway %s flags %x"
>

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

Re: rad(8): Implement RFC 8781 PREF64 router advertisement option.

[Florian Obser]

On 2022-10-15 13:00 +02, Florian Obser  wrote:
> With this clients can learn the presence and used prefix for Network
> Address and Protocol Translation between IPv6 and IPv4 (NAT64).
>
> Apparently there is support in mobile devices as well as in macOS.
>
> This option, together with the the dhcp "IPv6-only preferred"
> option (108) enables the Customer-side transLATor (CLAT) on macOS so
> IPv4 literals can be used in IPv6-only networks.
>

Better diff, I messed up the default and maximum lifetime seconds.

OK?

diff --git engine.c engine.c
index dddff2d4579..543abffe388 100644
--- engine.c
+++ engine.c
@@ -269,6 +269,7 @@ engine_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;
+   struct ra_pref64_conf   *pref64;
ssize_t  n;
int  shut = 0;
 
@@ -333,6 +334,7 @@ engine_dispatch_main(int fd, short event, void *bula)
SIMPLEQ_INIT(&nconf->ra_iface_list);
SIMPLEQ_INIT(&nconf->ra_options.ra_rdnss_list);
SIMPLEQ_INIT(&nconf->ra_options.ra_dnssl_list);
+   SIMPLEQ_INIT(&nconf->ra_options.ra_pref64_list);
ra_options = &nconf->ra_options;
break;
case IMSG_RECONF_RA_IFACE:
@@ -349,6 +351,7 @@ engine_dispatch_main(int fd, short event, void *bula)
SIMPLEQ_INIT(&ra_iface_conf->ra_prefix_list);
SIMPLEQ_INIT(&ra_iface_conf->ra_options.ra_rdnss_list);
SIMPLEQ_INIT(&ra_iface_conf->ra_options.ra_dnssl_list);
+   SIMPLEQ_INIT(&ra_iface_conf->ra_options.ra_pref64_list);
SIMPLEQ_INSERT_TAIL(&nconf->ra_iface_list,
ra_iface_conf, entry);
ra_options = &ra_iface_conf->ra_options;
@@ -405,6 +408,18 @@ engine_dispatch_main(int fd, short event, void *bula)
SIMPLEQ_INSERT_TAIL(&ra_options->ra_dnssl_list,
ra_dnssl_conf, entry);
break;
+   case IMSG_RECONF_RA_PREF64:
+   if(IMSG_DATA_SIZE(imsg) != sizeof(struct
+   ra_pref64_conf))
+   fatalx("%s: IMSG_RECONF_RA_PREF64 wrong length: 
"
+   "%lu", __func__, IMSG_DATA_SIZE(imsg));
+   if ((pref64 = malloc(sizeof(struct ra_pref64_conf))) ==
+   NULL)
+   fatal(NULL);
+   memcpy(pref64, imsg.data, sizeof(struct 
ra_pref64_conf));
+   SIMPLEQ_INSERT_TAIL(&ra_options->ra_pref64_list, pref64,
+   entry);
+   break;
case IMSG_RECONF_END:
if (nconf == NULL)
fatalx("%s: IMSG_RECONF_END without "
diff --git frontend.c frontend.c
index 30bcde218c4..435d6aed8a8 100644
--- frontend.c
+++ frontend.c
@@ -113,6 +113,14 @@ struct ra_iface {
uint8_t  data[RA_MAX_SIZE];
 };
 
+#define ND_OPT_PREF64  38
+struct nd_opt_pref64 {
+   u_int8_tnd_opt_pref64_type;
+   u_int8_tnd_opt_pref64_len;
+   u_int16_t   nd_opt_pref64_sltime_plc;
+   u_int8_tnd_opt_pref64[12];
+};
+
 TAILQ_HEAD(, ra_iface) ra_interfaces;
 
 __dead void frontend_shutdown(void);
@@ -300,6 +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;
+   struct ra_pref64_conf   *pref64;
int  n, shut = 0, icmp6sock, rdomain;
 
if (event & EV_READ) {
@@ -361,6 +370,7 @@ frontend_dispatch_main(int fd, short event, void *bula)
SIMPLEQ_INIT(&nconf->ra_iface_list);
SIMPLEQ_INIT(&nconf->ra_options.ra_rdnss_list);
SIMPLEQ_INIT(&nconf->ra_options.ra_dnssl_list);
+   SIMPLEQ_INIT(&nconf->ra_options.ra_pref64_list);
ra_options = &nconf->ra_options;
break;
case IMSG_RECONF_RA_IFACE:
@@ -377,6 +387,7 @@ frontend_dispatch_main(int fd, short event, void *bula)
SIMPLEQ_INIT(&ra_iface_conf->ra_prefix_list);
SIMPLEQ_INIT(&ra_iface_conf->ra_options.ra_rdnss_list);

rad(8): Implement RFC 8781 PREF64 router advertisement option.

[Florian Obser]

With this clients can learn the presence and used prefix for Network
Address and Protocol Translation between IPv6 and IPv4 (NAT64).

Apparently there is support in mobile devices as well as in macOS.

This option, together with the the dhcp "IPv6-only preferred"
option (108) enables the Customer-side transLATor (CLAT) on macOS so
IPv4 literals can be used in IPv6-only networks.

OK?

diff --git engine.c engine.c
index dddff2d4579..543abffe388 100644
--- engine.c
+++ engine.c
@@ -269,6 +269,7 @@ engine_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;
+   struct ra_pref64_conf   *pref64;
ssize_t  n;
int  shut = 0;
 
@@ -333,6 +334,7 @@ engine_dispatch_main(int fd, short event, void *bula)
SIMPLEQ_INIT(&nconf->ra_iface_list);
SIMPLEQ_INIT(&nconf->ra_options.ra_rdnss_list);
SIMPLEQ_INIT(&nconf->ra_options.ra_dnssl_list);
+   SIMPLEQ_INIT(&nconf->ra_options.ra_pref64_list);
ra_options = &nconf->ra_options;
break;
case IMSG_RECONF_RA_IFACE:
@@ -349,6 +351,7 @@ engine_dispatch_main(int fd, short event, void *bula)
SIMPLEQ_INIT(&ra_iface_conf->ra_prefix_list);
SIMPLEQ_INIT(&ra_iface_conf->ra_options.ra_rdnss_list);
SIMPLEQ_INIT(&ra_iface_conf->ra_options.ra_dnssl_list);
+   SIMPLEQ_INIT(&ra_iface_conf->ra_options.ra_pref64_list);
SIMPLEQ_INSERT_TAIL(&nconf->ra_iface_list,
ra_iface_conf, entry);
ra_options = &ra_iface_conf->ra_options;
@@ -405,6 +408,18 @@ engine_dispatch_main(int fd, short event, void *bula)
SIMPLEQ_INSERT_TAIL(&ra_options->ra_dnssl_list,
ra_dnssl_conf, entry);
break;
+   case IMSG_RECONF_RA_PREF64:
+   if(IMSG_DATA_SIZE(imsg) != sizeof(struct
+   ra_pref64_conf))
+   fatalx("%s: IMSG_RECONF_RA_PREF64 wrong length: 
"
+   "%lu", __func__, IMSG_DATA_SIZE(imsg));
+   if ((pref64 = malloc(sizeof(struct ra_pref64_conf))) ==
+   NULL)
+   fatal(NULL);
+   memcpy(pref64, imsg.data, sizeof(struct 
ra_pref64_conf));
+   SIMPLEQ_INSERT_TAIL(&ra_options->ra_pref64_list, pref64,
+   entry);
+   break;
case IMSG_RECONF_END:
if (nconf == NULL)
fatalx("%s: IMSG_RECONF_END without "
diff --git frontend.c frontend.c
index 30bcde218c4..435d6aed8a8 100644
--- frontend.c
+++ frontend.c
@@ -113,6 +113,14 @@ struct ra_iface {
uint8_t  data[RA_MAX_SIZE];
 };
 
+#define ND_OPT_PREF64  38
+struct nd_opt_pref64 {
+   u_int8_tnd_opt_pref64_type;
+   u_int8_tnd_opt_pref64_len;
+   u_int16_t   nd_opt_pref64_sltime_plc;
+   u_int8_tnd_opt_pref64[12];
+};
+
 TAILQ_HEAD(, ra_iface) ra_interfaces;
 
 __dead void frontend_shutdown(void);
@@ -300,6 +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;
+   struct ra_pref64_conf   *pref64;
int  n, shut = 0, icmp6sock, rdomain;
 
if (event & EV_READ) {
@@ -361,6 +370,7 @@ frontend_dispatch_main(int fd, short event, void *bula)
SIMPLEQ_INIT(&nconf->ra_iface_list);
SIMPLEQ_INIT(&nconf->ra_options.ra_rdnss_list);
SIMPLEQ_INIT(&nconf->ra_options.ra_dnssl_list);
+   SIMPLEQ_INIT(&nconf->ra_options.ra_pref64_list);
ra_options = &nconf->ra_options;
break;
case IMSG_RECONF_RA_IFACE:
@@ -377,6 +387,7 @@ frontend_dispatch_main(int fd, short event, void *bula)
SIMPLEQ_INIT(&ra_iface_conf->ra_prefix_list);
SIMPLEQ_INIT(&ra_iface_conf->ra_options.ra_rdnss_list);
SIMPLEQ_INIT(&ra_iface_conf->ra_options.ra_dnssl_list);
+   SIMPLEQ_INIT(&ra_iface_conf->ra_options.ra_pref64_list);
SIMPLEQ_INSERT_TAIL(&nconf->ra_iface_list,
ra_iface_conf, entry);
ra_options = &ra_iface_conf->ra_options;
@@ -4

Re: mg: add zap-to-char and zap-up-to-char

[Florian Obser]

On 2022-10-13 12:16 +02, Omar Polo  wrote:
> small quality-of-life addition.  GNU Emacs has zap-to-char bound by to
> M-z and zap-up-to-char unbound; i'm unsure how closely we want to
> follow emacs here, IMHO zap-up-to-char is way more useful than
> zap-to-char and so i opted to bound M-z to zap-up-to-char by default.

I always tried to stay as close to emacs as possible. I'd consider it a
bug otherwise. So I think you should bind zap-to-char per default and
override it in your startup file.

emacs also has this: "Goes backward if ARG is negative"

Could you implement that, or at least check that n is not negative?


>
> ok?
>
> diff 40c942665e2a6e73653062386b61f09373477e3d 
> b164b420ae16a7b218f9df51fb8c8a7ce8d5b054
> commit - 40c942665e2a6e73653062386b61f09373477e3d
> commit + b164b420ae16a7b218f9df51fb8c8a7ce8d5b054
> blob - d546dba15dd5b45503679454fbe55673640ea74e
> blob + 87704c129ee172c2f504741d534df826cf77131d
> --- usr.bin/mg/def.h
> +++ usr.bin/mg/def.h
> @@ -651,6 +651,9 @@ intreadpattern(char *);
>  int   forwsrch(void);
>  int   backsrch(void);
>  int   readpattern(char *);
> +int   zapuptochar(int, int);
> +int   zaptochar(int, int);
> +int   zap(int, int);
>  
>  /* spawn.c X */
>  int   spawncli(int, int);
> blob - 36a88e4573d47eae4d321ee899f71e52ef77a8c9
> blob + d9cb10a264955db8e6e77b8b8aa6352ed8db9bfd
> --- usr.bin/mg/funmap.c
> +++ usr.bin/mg/funmap.c
> @@ -138,6 +138,8 @@ static struct funmap functnames[] = {
>   {killbuffer_cmd, "kill-buffer", 1},
>   {killline, "kill-line", 1},
>   {killpara, "kill-paragraph", 1},
> + {zaptochar, "zap-to-char", 1},
> + {zapuptochar, "zap-up-to-char", 1},
>   {killregion, "kill-region", 0},
>   {delfword, "kill-word", 1},
>   {toggleleavetmp, "leave-tmpdir-backups", 0},
> blob - 32a9267d30fbbbc2f4fcc126133313cbc6799201
> blob + 6e6b649f756b879127e1f22b8178fe529b7ced03
> --- usr.bin/mg/keymap.c
> +++ usr.bin/mg/keymap.c
> @@ -290,7 +290,7 @@ static PF metal[] = {
>   copyregion, /* w */
>   extend, /* x */
>   rescan, /* y */
> - rescan, /* z */
> + zapuptochar,/* z */
>   gotobop,/* { */
>   piperegion, /* | */
>   gotoeop /* } */
> blob - a335c89e4d5329776a85c473c716b5e9e2597f19
> blob + fd5a2df0f48d7966ca1d4467ccf16bd68caf4c5b
> --- usr.bin/mg/mg.1
> +++ usr.bin/mg/mg.1
> @@ -331,6 +331,8 @@ execute-extended-command
>  copy-region-as-kill
>  .It M-x
>  execute-extended-command
> +.It M-z
> +zap-up-to-char
>  .It M-{
>  backward-paragraph
>  .It M-|
> @@ -986,6 +988,11 @@ It is not a ring.
>  kill buffer consists only
>  of the most recent kill.
>  It is not a ring.
> +.It zap-to-char
> +Ask for a character and delete text from the current cursor position
> +until the next instance of that character, including it.
> +.It zap-up-to-char
> +Like zap-to-char but doesn't delete the target character.
>  .El
>  .Sh MG DIRED KEY BINDINGS
>  Specific key bindings are available in dired mode.
> blob - fa12b4094aae01385711ca7b01c7eba964571e65
> blob + e9184de33a7cd149fd521ba7d1c57310f3b16953
> --- usr.bin/mg/search.c
> +++ usr.bin/mg/search.c
> @@ -853,3 +853,61 @@ readpattern(char *r_prompt)
>   retval = FALSE;
>   return (retval);
>  }
> +
> +/*
> + * Prompt for a character and kill until a it (including).  Mark is
> + * cleared afterwards.
> + */
> +int
> +zaptochar(int f, int n)
> +{
> + return (zap(TRUE, n));
> +}
> +
> +/* Like zaptochar but stops before the character. */
> +int
> +zapuptochar(int f, int n)
> +{
> + return (zap(FALSE, n));
> +}
> +
> +/*
> + * Prompt for a charcter and deletes from the point up to, optionally
> + * including, the first instance of that charcters.  Marks is cleared
> + * afterwards.
> + */
> +int
> +zap(int including, int n)
> +{
> + int s;
> +
> + if (including)
> + ewprintf("Zap to char: ");
> + else
> + ewprintf("Zap up to char: ");
> +
> + s = getkey(FALSE);
> + eerase();
> + if (s == ABORT || s == CCHR('G'))
> + return (FALSE);
> +
> + pat[0] = (char)s;
> + pat[1] = '\0';
> +
> + isetmark();
> + while (n--) {
> + if (forwsrch() == FALSE) {
> + dobeep();
> + ewprintf("Search failed: \"%s\"", pat);
> + swapmark(FFARG, 0);
> + clearmark(FFARG, 0);
> + return (FALSE);
> + }
> + }
> +
> + if (!including)
> + (void) backchar(FFARG, 1);
> + killregion(FFARG, 0);
> + clearmark(FFARG, 0);
> + return (TRUE);
> +}
>
>

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

Re: sysupgrade: exit 1 instead of exit 0 when ending early

[Florian Obser]

On 2022-10-07 14:39 -04, Josh Grosse  wrote:
> For ease of running sysupgrade from within a script.
>
> diff --git a/usr.sbin/sysupgrade/sysupgrade.sh 
> b/usr.sbin/sysupgrade/sysupgrade.sh
> index d80ff127ffa..ce5800093c9 100644
> --- a/usr.sbin/sysupgrade/sysupgrade.sh
> +++ b/usr.sbin/sysupgrade/sysupgrade.sh
> @@ -153,7 +153,7 @@ rm SHA256.sig
>  
>  if cmp -s /var/db/installed.SHA256 SHA256 && ! $FORCE; then
>   echo "Already on latest snapshot."
> - exit 0
> + exit 1
>  fi
>  
>  # INSTALL.*, bsd*, *.tgz
>

Being up2date doesn't feel like an error to me, what am I missing?

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

Re: acme-client: allow newlines in alternative names

[Florian Obser]

Makes sense to me, OK florian

Please wait a day or two in case there are objections.

On 2022-10-05 09:28 +02, Omar Polo  wrote:
> just a small scratch to itch; i'd prefer if i could split the
> alternative names in multiple lines without using \
>
> so, now one should be able to write
>
> domain example.com {
>   alternative names {
>   some-subdomain.example.com
>   another-subdomain.example.com
>   }
> }
>
> not sure if the manpage needs an update then, now it reads:
>
>  alternative names {...}
>  A list of alternative names, comma or space separated, for which
>  the certificate will be valid.
>
> but in other places we don't specify that newlines are accepted within
> curly braces blocks.
>
> diff /home/op/w/acme-client
> commit - 0b11e45035f5839d5e3ce9e86d234a5c7eb8f919
> path + /home/op/w/acme-client
> blob - e09c2283ad4b05452a6305be2d296cbb48496f2f
> file + parse.y
> --- parse.y
> +++ parse.y
> @@ -177,8 +177,8 @@ comma : ','
>  nl   : '\n' optnl/* one newline or more */
>   ;
>  
> -comma: ','
> - | /*empty*/
> +optcommanl   : ',' optnl
> + | optnl
>   ;
>  
>  authority: AUTHORITY STRING {
> @@ -287,7 +287,7 @@ domainoptsl   : ALTERNATIVE NAMES '{' altname_l '}'
>   | domainoptsl optnl
>   ;
>  
> -domainoptsl  : ALTERNATIVE NAMES '{' altname_l '}'
> +domainoptsl  : ALTERNATIVE NAMES '{' optnl altname_l '}'
>   | DOMAIN NAME STRING {
>   char *s;
>   if (domain->domain != NULL) {
> @@ -395,8 +395,8 @@ altname_l : altname comma altname_l
>   }
>   ;
>  
> -altname_l: altname comma altname_l
> - | altname
> +altname_l: altname optcommanl altname_l
> + | altname optnl
>   ;
>  
>  altname  : STRING {
>

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

Re: Remove some unnecessary setproctitle(3) format strings

[Florian Obser]

On 2022-09-27 09:26 +02, Martijn van Duren  wrote:
> The caveats section talks about "user-supplied data". These string are
> constant and don't contain any '%'. Most other daemons in base use the
> setproctitle("title"); format as well.

It's not that clear cut, snmpd(8), a daemon you might be familiar with,
uses
setproctitle("%s", p->p_title);

while p->p_title is not user supplied ;)

The diff is probably not wrong (I only glanced at it), but I don't think
it solves a problem. It then comes down to style and I prefer things the
way they are.

>
> On Tue, 2022-09-27 at 08:07 +0100, Stuart Henderson wrote:
>> These programs seem OK as-is, they are following the advice in
>> https://man.openbsd.org/setproctitle.3#CAVEATS

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

Re: grdc: show timezone when TZ is set

[Florian Obser]

deraadt objected to the time zone validation. I don't care about the
feature and I agree with the point that I shouldn't do it because there
is no API for it. I don't even know where the time zone files are.

To make this all more symmetric always print tm_zone, even if TZ is not
set.

OK?

diff --git grdc.6 grdc.6
index 16e1758c6d2..5aa6e84a2d2 100644
--- grdc.6
+++ grdc.6
@@ -34,8 +34,11 @@ key exits the program.
 .Bl -tag -width Ds
 .It Ev TZ
 The time zone to use for displaying the time.
-It is specified as a pathname relative to
-.Pa /usr/share/zoneinfo .
+It is normally specified as a pathname relative to
+.Pa /usr/share/zoneinfo ,
+though see
+.Xr tzset 3
+for more information.
 If this variable is not set, the time zone is determined based on
 .Pa /etc/localtime .
 .El
diff --git grdc.c grdc.c
index 66e5eee79e6..01d29b08d64 100644
--- grdc.c
+++ grdc.c
@@ -12,6 +12,7 @@
  */
 
 #include 
+#include 
 
 #include 
 #include 
@@ -20,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -78,9 +80,14 @@ main(int argc, char *argv[])
int xbase;
int ybase;
int wintoosmall;
+   int tz_len = 0;
+   int h, m;
+   int prev_tm_gmtoff;
char *tz;
 
tz = getenv("TZ");
+   if (tz != NULL)
+   tz_len = strlen(tz);
 
scrol = wintoosmall = 0;
while ((i = getopt(argc, argv, "sh")) != -1) {
@@ -135,6 +142,7 @@ main(int argc, char *argv[])
 
curs_set(0);
sigwinched = 1; /* force initial sizing */
+   prev_tm_gmtoff = 24 * 3600; /* force initial header printing */
 
clock_gettime(CLOCK_REALTIME, &now);
if (n) {
@@ -152,9 +160,11 @@ main(int argc, char *argv[])
set(tm->tm_hour / 10, 24);
set(10, 7);
set(10, 17);
-   if (sigwinched) {
+   /* force repaint if window size changed or DST changed */
+   if (sigwinched || prev_tm_gmtoff != tm->tm_gmtoff) {
sigwinched = 0;
wintoosmall = 0;
+   prev_tm_gmtoff = tm->tm_gmtoff;
getwinsize(&i, &j);
if (i >= XLENGTH + 2)
xbase = (i - XLENGTH) / 2;
@@ -184,11 +194,19 @@ main(int argc, char *argv[])
move(ybase, xbase + XLENGTH);
vline(ACS_VLINE, YDEPTH);
 
-   if (tz != NULL) {
-   move(ybase - 1, xbase);
-   printw("[ %s %+d ]", tz,
-   tm->tm_gmtoff / 60 / 60 );
-   }
+   move(ybase - 1, xbase);
+
+   h = tm->tm_gmtoff / 3600;
+   m = abs((int)tm->tm_gmtoff % 3600 / 60);
+
+   if (tz_len > 0 && tz_len <= XLENGTH -
+   strlen("[  () + ]") -
+   strlen(tm->tm_zone))
+   printw("[ %s (%s) %+2.2d%02d ]", tz,
+   tm->tm_zone, h, m);
+   else
+   printw("[ %s %+2.2d%02d ]",
+   tm->tm_zone, h, m);
 
attrset(COLOR_PAIR(2));
}

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

Re: grdc: show timezone when TZ is set

[Florian Obser]

So, with the tzset(3) restriction in place I'd like to fix grdc, because
what we currently have is wrong:

There are time zones that have minute offsets, display those
correctly. Pointed out by pjanzen@.
To display the offset, use ISO 8601, as suggested by David Goerger.

Take a guess if tzset(3) will accept the time zone specified in TZ as
a relative path and only display it if that is true and it's not too
long. All time zones currently in /usr/share/zoneinfo fit.

Lastly check if tm->tm_gmtoff changed which probably means that we
moved in or out of daylight savings time.

OK?

p.s. I don't know what to do about wintoosmall at this time, the diff is
big enough as it is. And quite frankly I don't care about that.

diff --git grdc.6 grdc.6
index 16e1758c6d2..5aa6e84a2d2 100644
--- grdc.6
+++ grdc.6
@@ -34,8 +34,11 @@ key exits the program.
 .Bl -tag -width Ds
 .It Ev TZ
 The time zone to use for displaying the time.
-It is specified as a pathname relative to
-.Pa /usr/share/zoneinfo .
+It is normally specified as a pathname relative to
+.Pa /usr/share/zoneinfo ,
+though see
+.Xr tzset 3
+for more information.
 If this variable is not set, the time zone is determined based on
 .Pa /etc/localtime .
 .El
diff --git grdc.c grdc.c
index 66e5eee79e6..4bb7fa1b1af 100644
--- grdc.c
+++ grdc.c
@@ -12,6 +12,7 @@
  */
 
 #include 
+#include 
 
 #include 
 #include 
@@ -20,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -45,6 +47,7 @@ void getwinsize(int *, int *);
 void set(int, int);
 void standt(int);
 void __dead usage(void);
+int check_tz(const char *);
 
 void
 sigalrm(int signo)
@@ -64,6 +67,36 @@ sigresize(int signo)
sigwinched = signo;
 }
 
+/* Take a guess if tzset(3) will accept TZ as a relative path */
+int
+check_tz(const char *tz)
+{
+   struct stat  sb;
+   char fullname[PATH_MAX];
+   int  i;
+
+   if (tz == NULL)
+   return 0;
+
+   if (tz[0] == ':')
+   tz++;
+
+   if (tz[0] == '/' || strstr(tz, "../") != NULL)
+   return 0;
+
+   i = snprintf(fullname, sizeof(fullname), "/usr/share/zoneinfo/%s", tz);
+   if (i < 0 || i >= sizeof(fullname))
+   return 0;
+
+   if (stat(fullname, &sb) == -1)
+   return 0;
+
+   if (!S_ISREG(sb.st_mode))
+   return 0;
+
+   return 1;
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -78,9 +111,19 @@ main(int argc, char *argv[])
int xbase;
int ybase;
int wintoosmall;
+   int tz_valid;
+   int tz_len = 0;
+   int prev_tm_gmtoff;
char *tz;
 
tz = getenv("TZ");
+   tz_valid = check_tz(tz);
+
+   if (tz_valid) {
+   if (tz[0] == ':')
+   tz++;
+   tz_len = strlen(tz);
+   }
 
scrol = wintoosmall = 0;
while ((i = getopt(argc, argv, "sh")) != -1) {
@@ -135,6 +178,7 @@ main(int argc, char *argv[])
 
curs_set(0);
sigwinched = 1; /* force initial sizing */
+   prev_tm_gmtoff = 24 * 3600; /* force initial header printing */
 
clock_gettime(CLOCK_REALTIME, &now);
if (n) {
@@ -152,9 +196,11 @@ main(int argc, char *argv[])
set(tm->tm_hour / 10, 24);
set(10, 7);
set(10, 17);
-   if (sigwinched) {
+   /* force repaint if window size changed or DST changed */
+   if (sigwinched || prev_tm_gmtoff != tm->tm_gmtoff) {
sigwinched = 0;
wintoosmall = 0;
+   prev_tm_gmtoff = tm->tm_gmtoff;
getwinsize(&i, &j);
if (i >= XLENGTH + 2)
xbase = (i - XLENGTH) / 2;
@@ -185,9 +231,20 @@ main(int argc, char *argv[])
vline(ACS_VLINE, YDEPTH);
 
if (tz != NULL) {
+   int h, m;
+   h = tm->tm_gmtoff / 3600;
+   m = abs((int)tm->tm_gmtoff % 3600 / 60);
+   if (tz_valid && tz_len > XLENGTH -
+   strlen("[  () + ]") -
+   strlen(tm->tm_zone))
+   tz_valid = 0;
move(ybase - 1, xbase);
-   printw("[ %s %+d ]", tz,
-   tm->tm_gmtoff / 60 / 60 );
+   if (tz_valid)
+   printw("[ %s (%s) %+2.2d%02d ]",
+   tz, tm->tm_zone, h, m);
+   else
+   printw("[ %s %+2.2d%02d ]",
+ 

Re: sysupgrade - Reading from socket: Undefined error: 0

[Florian Obser]

On 2022-09-19 22:27 +02, Hrvoje Popovski  wrote:
> Hi all,
>
> when doing sysupgrade few minutes ago on multiple machines i'm getting
> error in subject
>
> smc24# sysupgrade -s
> Fetching from https://cdn.openbsd.org/pub/OpenBSD/snapshots/amd64/
> SHA256.sig   100% |*|
> 2144   00:00
> Signature Verified
> INSTALL.amd64 100% ||
> 43554   00:00
> base72.tgz   100% |*|
> 331 MB00:16
> bsd  100% |*|
> 22449 KB00:05
> bsd.mp   100% |*|
> 22562 KB00:04
> bsd.rd   100% |*|
> 4533 KB00:01
> comp72.tgz   100% |*|
> 74598 KB00:09
> game72.tgz   100% |*|
> 2745 KB00:01
> man72.tgz100% |*|
> 7610 KB00:02
> xbase72.tgz   29% |**   |
> 15744 KB00:14 ETAsysupgrade: Reading from socket: Undefined error: 0
> smc24#
>

Is this somehow coming from the non-blocking connect diff? I can't spot
it though...

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

Re: grdc: show timezone when TZ is set

[Florian Obser]

I'm happy with that, let's do this then
- fix the offset calculation
- output tm->tm_zone in addition to TZ to be able to spot typos.

OK?

diff --git grdc.c grdc.c
index 66e5eee79e6..05b1ff1ea87 100644
--- grdc.c
+++ grdc.c
@@ -185,9 +185,12 @@ main(int argc, char *argv[])
vline(ACS_VLINE, YDEPTH);
 
if (tz != NULL) {
+   int h, m;
+   h = tm->tm_gmtoff / 3600;
+   m = abs((int)tm->tm_gmtoff % 3600 / 60);
move(ybase - 1, xbase);
-   printw("[ %s %+d ]", tz,
-   tm->tm_gmtoff / 60 / 60 );
+   printw("[ %s (%s) %+dh%02d ]", tz,
+   tm->tm_zone, h, m);
}
 
attrset(COLOR_PAIR(2));



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

Re: grdc: show timezone when TZ is set

[Florian Obser]

On 2022-09-18 01:55 -04, Paul Janzen  wrote:
> The recent change to grdc(6), to display additional information if TZ is
> set, has a few issues.
>
> 1.  Time zone offset incorrectly reported in Newfoundland.
>
> Some time zones have offsets of 30 or 45 minutes.  The displayed time
> offset is currently truncated to the closest hour.  (Australia/Eucla
> is a fun time zone too.)

Very good point. Let's shine this turd a bit more!

>
> 2.  The new, additional information disappears if the window is sized too
> small (wintoosmall).  There's basically room for it though...  except

I don't care too much about the wintoosmall case, so I left your version
in. I guess we could do better with the TZ validation (see below).

>
> 3.  The TZ information is a string of unknown length, and so it doesn't
> necessarily display correctly.
>
> Yeah, I know with pledge() you can't do testing like
> TZ=:/home/pjanzen/dev/usr/share/zoneinfo/testing/2022/America/Kentucky/Monticello
> any more, even though that's a perfectly cromulent and functional TZ on my
> system otherwise. So for the time being, probably any TZ that exists and
> doesn't abort grdc is 38 bytes or shorter.  Which works.

grdc got aborted even before this last change. That's not optimal. So we
have to hoist the (implicit) call to tzset before pledge(2). I went
further and did:
pledge("stdio rpath tty")
tzset()
pledge("stdio tty")

>
> But, if your timezone exists, it already has its name in short form
> included in tm->tm_zone.  That's what I think should be
> printed, even if "EDT" is way less cool than "America/Pangnirtung".  I
> mean, if the point is just to be able to label clocks with nice places,
> instead of the time zone it's showing, maybe it could be a different
> option.

For me, that would completely miss the point. I have no idea what EDT
means.

>
> Counterpoint:  some of the timezones have short names that are just the UTC
> offset, which is really boring and duplicated given that we print out the
> offset already.  Hey, maybe we could just print the offset...
>
> 4.  There's no indication if you type an invalid TZ--you get UTC and a
> misleading label onscreen.
>
> Timezone handling defaults to UTC if anything breaks along the chain, as
> the tzset(3) man page makes clear.  That means if you misspell Antarctica
> while setting your TZ=Antartica/McMurdo, you'll end up half a day off from
> all your pals at McMurdo as your screen happily tells you that you're
> seeing McMurdo +0 time.
>
> There's no simple way to tell if your $TZ is valid or not (if you get back
> UTC, maybe that's really what it should be!).  At least if we display
> tm->tm_zone rather than $TZ, we're not misleading.
>

Right, so I had a stab at validating TZ:
If TZ is a relative path and exists as a file in /usr/share/zoneinfo
display it and also print tm->tm_zone:

  ┌[ Antarctica/McMurdo (NZST) +12h00 ]──┐

  ┌[ Australia/Eucla (+0845) +8h45 ]─┐

There is a TOCTU issue (doesn't matter I think) and an issue where what
we find in /usr/share/zoneinfo is not syntactically correct, i.e.:
$ TZ=zone.tab grdc
  ┌[ zone.tab (GMT) +0h00 ]──┐

I don't think that matters either. Both are cases of: don't do that.

>
>
> I was going to be upset that the man page for grdc(6) is way pickier on TZ
> than tzset(3), but it's quite accurate given the pledge() call.  Speaking
> of which, I don't expect any one else plays with timezone files, and surely
> one doesn't want one's general utilities to be pwned by possible bugs in
> the time-handling code exploited with custom files created outside
> /usr/share/zoneinfo.  But it's still a touch irritating-should-be-fixable
> that, because the pledge() has to be after initscr(), grdc has the
> possibility of leaving the terminal in the wrong state when it aborts on
> test TZ files.
>

This works now, too:

$ TZ=~/Newfoundland grdc
  ┌[ NDT -2h30 ]─┐

>
>
> Paul Janzen.
>
> Index: grdc.c
> ===
> RCS file: /cvs/src/games/grdc/grdc.c,v
> retrieving revision 1.35
> +void
> +print_tz(int y, int x, int sml)
> +{
> + int i, j;
> +
> + move(y, x);
> + i = tm->tm_gmtoff / 60 / 60;
> + j = tm->tm_gmtoff / 60 - i * 60;

Isn't this just a weird spelling for mod (%)?

> + if (i < 0)
> + j = -j;
> + if (!sml)
> + printw("[ %s %+dh%02d ]", tm->tm_zone, i, j);
> + else
> + printw("[%+dh%02d]", i, j);
>  }
>  
>  void
>

diff --git grdc.c grdc.c
index 66e5eee79e6..5d2ea19a532 100644
--- grdc.c
+++ grdc.c
@@ -12,6 +12,7 @@
  */
 
 #include 
+#include 
 
 #include 
 #include 
@@ -26,9 +27,6 @@
 #define XLENGTH 58
 #define YDEPTH  7
 
-struct timespec now;
-struct tm *tm;
-
 short disp[11] = {
075557, 01, 071747, 071717, 055711,
074717, 074757, 07, 0

Re: grdc: show timezone when TZ is set

[Florian Obser]

On 2022-09-17 08:42 UTC, Klemens Nanni  wrote:
> On Sat, Sep 17, 2022 at 09:40:27AM +0200, Florian Obser wrote:
>> On 2021-10-24 03:06 +02, James Russell Stickney  wrote:
>> > I recently found myself wanting to moniter local time from a number of 
>> > locations around the world.
>> > Setting the TZ environment variable on grdc did a wonderfull job at this.
>> > At which point, I wanted to know which clock was showing what time.
>> > This lead to the following patch.
>> >
>> > If curious as to how this looked, it was sort of like the image below.
>> > https://nl1.outband.net/image/grdc_desktop_example.png
>> >
>> 
>> I recently found a use for this as well and remembered this diff. One
>> long line re-wrapped.
>> 
>> OK?
>
> Reads fine, but I don't use this "game".
>
>> 
>> (Or commit it with OK florian)
>> 
>> diff --git grdc.6 grdc.6
>> index 16c1fb5cd3d..69febddc9a2 100644
>> --- grdc.6
>> +++ grdc.6
>> @@ -30,6 +30,13 @@ skips seconds.
>>  Pressing the
>>  .Sq q
>>  key exits the program.
>> +.Sh ENVIRONMENT
>> +.Bl -tag -width "daemon_timeout"
>> +.It Ev TZ
>> +Time shown will be from zone as found under /usr/share/zonefile.
>
> This should be
> .Pa /usr/share/zonefile .

Good catch, I have adapted the text from date(1) for this, I think it
flows nicer and dropped the example.

Update diff with a local variable for getenv(3) and pulled the call up,
it's not going to change during runtime.

OK?

diff --git grdc.6 grdc.6
index 16c1fb5cd3d..a7258488e5a 100644
--- grdc.6
+++ grdc.6
@@ -30,6 +30,15 @@ skips seconds.
 Pressing the
 .Sq q
 key exits the program.
+.Sh ENVIRONMENT
+.Bl -tag -width Ds
+.It Ev TZ
+The time zone to use for displaying the time.
+It is specified as a pathname relative to
+.Pa /usr/share/zoneinfo .
+If this variable is not set, the time zone is determined based on
+.Pa /etc/localtime .
+.El
 .Sh AUTHORS
 .An -nosplit
 .An Amos Shapir ,
diff --git grdc.c grdc.c
index 287fb14e95f..94f80a865c0 100644
--- grdc.c
+++ grdc.c
@@ -78,6 +78,9 @@ main(int argc, char *argv[])
int xbase;
int ybase;
int wintoosmall;
+   char *tz;
+
+   tz = getenv("TZ");
 
scrol = wintoosmall = 0;
while ((i = getopt(argc, argv, "sh")) != -1) {
@@ -139,6 +142,16 @@ main(int argc, char *argv[])
alarm(n);
}
do {
+   mask = 0;
+   tm = localtime(&now.tv_sec);
+   set(tm->tm_sec % 10, 0);
+   set(tm->tm_sec / 10, 4);
+   set(tm->tm_min % 10, 10);
+   set(tm->tm_min / 10, 14);
+   set(tm->tm_hour % 10, 20);
+   set(tm->tm_hour / 10, 24);
+   set(10, 7);
+   set(10, 17);
if (sigwinched) {
sigwinched = 0;
wintoosmall = 0;
@@ -171,21 +184,17 @@ main(int argc, char *argv[])
move(ybase, xbase + XLENGTH);
vline(ACS_VLINE, YDEPTH);
 
+   if (tz != NULL) {
+   move(ybase - 1, xbase);
+   printw("[ %s %+d ]", tz,
+   tm->tm_gmtoff / 60 / 60 );
+   }
+
attrset(COLOR_PAIR(2));
}
for (k = 0; k < 6; k++)
old[k] = 0;
}
-   mask = 0;
-   tm = localtime(&now.tv_sec);
-   set(tm->tm_sec % 10, 0);
-   set(tm->tm_sec / 10, 4);
-   set(tm->tm_min % 10, 10);
-   set(tm->tm_min / 10, 14);
-   set(tm->tm_hour % 10, 20);
-   set(tm->tm_hour / 10, 24);
-   set(10, 7);
-   set(10, 17);
if (wintoosmall) {
move(0, 0);
printw("%02d:%02d:%02d", tm->tm_hour, tm->tm_min,





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

Re: grdc: show timezone when TZ is set

[Florian Obser]

On 2021-10-24 03:06 +02, James Russell Stickney  wrote:
> I recently found myself wanting to moniter local time from a number of 
> locations around the world.
> Setting the TZ environment variable on grdc did a wonderfull job at this.
> At which point, I wanted to know which clock was showing what time.
> This lead to the following patch.
>
> If curious as to how this looked, it was sort of like the image below.
> https://nl1.outband.net/image/grdc_desktop_example.png
>

I recently found a use for this as well and remembered this diff. One
long line re-wrapped.

OK?

(Or commit it with OK florian)

diff --git grdc.6 grdc.6
index 16c1fb5cd3d..69febddc9a2 100644
--- grdc.6
+++ grdc.6
@@ -30,6 +30,13 @@ skips seconds.
 Pressing the
 .Sq q
 key exits the program.
+.Sh ENVIRONMENT
+.Bl -tag -width "daemon_timeout"
+.It Ev TZ
+Time shown will be from zone as found under /usr/share/zonefile.
+For Example:
+TZ=Asia/Tokyo grdc
+.El
 .Sh AUTHORS
 .An -nosplit
 .An Amos Shapir ,
diff --git grdc.c grdc.c
index 287fb14e95f..f8521a5029d 100644
--- grdc.c
+++ grdc.c
@@ -139,6 +139,16 @@ main(int argc, char *argv[])
alarm(n);
}
do {
+   mask = 0;
+   tm = localtime(&now.tv_sec);
+   set(tm->tm_sec % 10, 0);
+   set(tm->tm_sec / 10, 4);
+   set(tm->tm_min % 10, 10);
+   set(tm->tm_min / 10, 14);
+   set(tm->tm_hour % 10, 20);
+   set(tm->tm_hour / 10, 24);
+   set(10, 7);
+   set(10, 17);
if (sigwinched) {
sigwinched = 0;
wintoosmall = 0;
@@ -171,21 +181,17 @@ main(int argc, char *argv[])
move(ybase, xbase + XLENGTH);
vline(ACS_VLINE, YDEPTH);
 
+   if (getenv("TZ") ) {
+   move(ybase - 1, xbase);
+   printw("[ %s %+d ]", getenv("TZ"),
+   tm->tm_gmtoff / 60 / 60 );
+   }
+
attrset(COLOR_PAIR(2));
}
for (k = 0; k < 6; k++)
old[k] = 0;
}
-   mask = 0;
-   tm = localtime(&now.tv_sec);
-   set(tm->tm_sec % 10, 0);
-   set(tm->tm_sec / 10, 4);
-   set(tm->tm_min % 10, 10);
-   set(tm->tm_min / 10, 14);
-   set(tm->tm_hour % 10, 20);
-   set(tm->tm_hour / 10, 24);
-   set(10, 7);
-   set(10, 17);
if (wintoosmall) {
move(0, 0);
printw("%02d:%02d:%02d", tm->tm_hour, tm->tm_min,


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

Re: httpd: overwrite rather than error for duplicate type entries

[Florian Obser]

This diff is correct and the use-case makes sense to me.
OK florian


On 2022-09-01 21:30 +01, Ben Fuller  wrote:
> On Thu, Sep 01, 2022 at 21:22:13 +0100, Ben Fuller wrote:
>> On Thu, Sep 01, 2022 at 21:44:34 +0200, Florian Obser wrote:
>> > Pretty sure this doesn't compile.
>> > If it were to compile it would leak memory.
>> 
>> It did compile, but you're right. This version should free everything:
>
> Makes more sense to use the existing function (sorry for the spam):
>
> ---
>  usr.sbin/httpd/httpd.c  | 4 ++--
>  usr.sbin/httpd/httpd.conf.5 | 1 +
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git usr.sbin/httpd/httpd.c usr.sbin/httpd/httpd.c
> index 2acecd1732f..efe199c20a9 100644
> --- usr.sbin/httpd/httpd.c
> +++ usr.sbin/httpd/httpd.c
> @@ -1080,9 +1080,9 @@ media_add(struct mediatypes *types, struct media_type 
> *media)
>   struct media_type   *entry;
>  
>   if ((entry = RB_FIND(mediatypes, types, media)) != NULL) {
> - log_debug("%s: duplicated entry for \"%s\"", __func__,
> + log_debug("%s: entry overwritten for \"%s\"", __func__,
>   media->media_name);
> - return (NULL);
> + media_delete(types, entry);
>   }
>  
>   if ((entry = malloc(sizeof(*media))) == NULL)
> diff --git usr.sbin/httpd/httpd.conf.5 usr.sbin/httpd/httpd.conf.5
> index b5f0be465a0..02f240091b0 100644
> --- usr.sbin/httpd/httpd.conf.5
> +++ usr.sbin/httpd/httpd.conf.5
> @@ -753,6 +753,7 @@ to the specified extension
>  .Ar name .
>  One or more names can be specified per line.
>  Each line may end with an optional semicolon.
> +Later lines overwrite earlier lines.
>  .It Ic include Ar file
>  Include types definitions from an external file, for example
>  .Pa /usr/share/misc/mime.types .
>

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

Re: httpd: overwrite rather than error for duplicate type entries

[Florian Obser]

Pretty sure this doesn't compile.
If it were to compile it would leak memory.

On 1 September 2022 20:32:55 CEST, Ben Fuller  wrote:
>Hi,
>
>In my httpd.conf, I include /usr/share/misc/mime.types but also want to
>define a few of my own type rules: in particular, I wanted to use
>
>text/"plain;charset=UTF-8" txt
>
>to get UTF-8 plain text to be displayed correctly. However, this txt
>rule collides with the text/plain rule in /usr/share/misc/mime.types.
>
>This patch allows duplicate type entries in httpd.conf, with rules
>further down the file overwriting earlier ones. So now I can include
>mime.types and write changed rules underneath.
>
>Ben
>
>---
> usr.sbin/httpd/httpd.c  | 4 ++--
> usr.sbin/httpd/httpd.conf.5 | 1 +
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
>diff --git usr.sbin/httpd/httpd.c usr.sbin/httpd/httpd.c
>index 2acecd1732f..abc2991aaf0 100644
>--- usr.sbin/httpd/httpd.c
>+++ usr.sbin/httpd/httpd.c
>@@ -1080,9 +1080,9 @@ media_add(struct mediatypes *types, struct media_type 
>*media)
>   struct media_type   *entry;
> 
>   if ((entry = RB_FIND(mediatypes, types, media)) != NULL) {
>-  log_debug("%s: duplicated entry for \"%s\"", __func__,
>+  log_debug("%s: entry overwritten for \"%s\"", __func__,
>   media->media_name);
>-  return (NULL);
>+  RB_REMOVE(mediatypes, types, media);
>   }
> 
>   if ((entry = malloc(sizeof(*media))) == NULL)
>diff --git usr.sbin/httpd/httpd.conf.5 usr.sbin/httpd/httpd.conf.5
>index b5f0be465a0..02f240091b0 100644
>--- usr.sbin/httpd/httpd.conf.5
>+++ usr.sbin/httpd/httpd.conf.5
>@@ -753,6 +753,7 @@ to the specified extension
> .Ar name .
> One or more names can be specified per line.
> Each line may end with an optional semicolon.
>+Later lines overwrite earlier lines.
> .It Ic include Ar file
> Include types definitions from an external file, for example
> .Pa /usr/share/misc/mime.types .
>

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

Re: ps(1): add -d (descendancy) option to display parent/child process relationships

[Florian Obser]

On 2022-09-01 09:55 -06, "Theo de Raadt"  wrote:
> Job Snijders  wrote:
>
>> On Thu, Sep 01, 2022 at 03:14:40PM +0200, Martin Schröder wrote:
>> > Am Do., 1. Sept. 2022 um 05:38 Uhr schrieb Job Snijders :
>> > > Some ps(1) implementations have an '-d' ('descendancy') option. Through
>> > > ASCII art parent/child process relationships are grouped and displayed.
>> > >
>> > > Thoughts?
>> > 
>> > gnu ps has
>> > 
>> > -d Select all processes except session leaders.
>> > 
>> > and
>> > 
>> >f  ASCII art process hierarchy (forest).
>> > 
>> >--forest
>> >   ASCII art process tree.
>> 
>> GNU ps uses both '-f', '--forest', and '-H' to display process
>> hierarchy. The '-H' option uses indenting (no ASCII art).
>> 
>> NetBSD's and FreeBSD's ps(1) use '-d' to display process hierarchy.
>
> using -f would follow the path of least resistance.  Is there really a common
> user commnity between freebsd netbsd and openbsd?  I doubt it.
>

Curious, my Jesus Laptop (macOS 12.5) has
 -A  Display information about other users' processes, including
 those without controlling terminals.
[...]
 -d  Like -A, but excludes session leaders.

It does not have this feature at all. Is this a new thing in FreeBSD?

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

Re: dhcpleased.8: add lease files to FILES

[Florian Obser]

On 2022-08-18 20:34 UTC, Klemens Nanni  wrote:
> On Thu, Aug 18, 2022 at 08:53:51PM +0100, Jason McIntyre wrote:
>> On Thu, Aug 18, 2022 at 07:29:42PM +, Klemens Nanni wrote:
>> > There is dhcpleasectl(8) -l but that only works for currently
>> > configured leases/interfaces and does not print all information
>> > contained in the lease file (mostly tailored to fit the installer's
>> > needs).
>> > 
>> > Feedback? OK?
>> > 
>> 
>> hi.
>> 
>> - in general i like this. i didn;t know about it, and am currently
>>   having to run dhcpleasectl to get the info (and generating a temp
>>   file so a script can read it!)
>> 
>> - when you say dhcpleasectl only works "for currently configured
>>   leases/interfaces" how does this differ? you mean it shows you the
>>   last lease, even if one is not currently in use?
>
> Lease file stay behind until manually removed or overwritten with fresh
> leases, so you need to know what's what when parsing them:
>   $ ls /var/db/dhcpleased/
>   athn0cdce0em0  trunk0   urndis0
>
> For example, there is no cdce0 or urndis0 right now on my box, those are
> from past tethering situations and those lease files are useless now.

They are used when cdce0 or urndis0 come back and made autoconf
interfaces. dhcpleased then goes throught the REBOOTING state, which is
faster and gives you your old IP back, if available.
So I don't think "useless" is the right word in the general case.

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

Re: slowcgi, httpd and fastcgi abnormal termination

[Florian Obser]

On 2022-08-11 11:39 +02, Claudio Jeker  wrote:
> On Wed, Aug 10, 2022 at 09:45:44PM +0200, Omar Polo wrote:
>> On 2022/08/10 15:07:15 +0200, Claudio Jeker wrote:
>> > On Sun, Aug 07, 2022 at 11:10:22AM +0200, Omar Polo wrote:
>> > Should slowcgi kill the command if SCRIPT_DONE is not set? 
>> 
>> RFC 3875 says that a script should be prepared to handle abnormal
>> termination and tha the server acn interrupt or terminate it at any
>> time and without warning, so killing the script is a possibility.
>> 
>> However, I'm a bit worried.  We could hit the timer not because of a
>> faulty script but because an HTTP client is purposefully reading data
>> slowly.  This could even allow to kill the scripts at specific points.
>> Maybe I'm overthinking and it's not an issue.
>
> The question I have is if scripts properly notice the IO error and
> terminate. If a script got stuck somehow and just sits there it will not
> realize it got killed. If that process exits at a later point slowcgi will
> be confused about a spurious SIGCHLD.
>
> What would happen if this instead sends a SIGTERM and after 5sec timeout a
> SIGKILL to the process? It should result in a proper close of the session
> via the usual channels (signal handler, io handler). Isn't that a better
> way of handling a timeout?

It was a design decision why back when to ignore the RFC and not kill
the script on timeout because it was unclear how many badly behaving
scripts there are.

I do hope that most people moved to fastcgi by now and maybe it's time
to shake out scripts that don't handle termination correctly?

This has come up so many times over the years that I suspect I'm the
only one worried about misbehaving scripts.
So ok, fine, go ahead, terminate & kill after timeout.

Btw. it's not getting too confused about a spurious SIGCHLD, it just logs
it.

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

Re: nd6: Rename is_newentry to newentry

[Florian Obser]

On 2022-08-04 14:21 UTC, Klemens Nanni  wrote:
> This matches the extensive comments and schema for related variables.
> No functional change.

are you planning to work on ND, or is this just shuffing of deck chairs?

When I rewrote source address selection it was worthwhile that blame
worked to figure out when stuff was added. You are adding noise to
that.

(I do not plan to work on this any time soon.)

>
> OK?
>
> diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c
> index c0b92f059c9..d793a0b07f4 100644
> --- a/sys/netinet6/nd6.c
> +++ b/sys/netinet6/nd6.c
> @@ -1084,7 +1084,7 @@ nd6_cache_lladdr(struct ifnet *ifp, struct in6_addr 
> *from, char *lladdr,
>  {
>   struct rtentry *rt = NULL;
>   struct llinfo_nd6 *ln = NULL;
> - int is_newentry;
> + int newentry;
>   struct sockaddr_dl *sdl = NULL;
>   int do_update;
>   int olladdr;
> @@ -1113,14 +1113,14 @@ nd6_cache_lladdr(struct ifnet *ifp, struct in6_addr 
> *from, char *lladdr,
>   rt = nd6_lookup(from, 0, ifp, ifp->if_rdomain);
>   if (rt == NULL) {
>   rt = nd6_lookup(from, 1, ifp, ifp->if_rdomain);
> - is_newentry = 1;
> + newentry = 1;
>   } else {
>   /* do not overwrite local or static entry */
>   if (ISSET(rt->rt_flags, RTF_STATIC|RTF_LOCAL)) {
>   rtfree(rt);
>   return;
>   }
> - is_newentry = 0;
> + newentry = 0;
>   }
>  
>   if (!rt)
> @@ -1175,7 +1175,7 @@ fail:
>   bcopy(lladdr, LLADDR(sdl), ifp->if_addrlen);
>   }
>  
> - if (!is_newentry) {
> + if (!newentry) {
>   if ((!olladdr && lladdr) || /* (3) */
>   (olladdr && lladdr && llchange)) {  /* (5) */
>   do_update = 1;
> @@ -1259,7 +1259,7 @@ fail:
>   /*
>* New entry must have is_router flag cleared.
>*/
> - if (is_newentry)/* (6-7) */
> + if (newentry)   /* (6-7) */
>   ln->ln_router = 0;
>   break;
>   case ND_REDIRECT:
> @@ -1270,7 +1270,7 @@ fail:
>*/
>   if (code == ND_REDIRECT_ROUTER)
>   ln->ln_router = 1;
> - else if (is_newentry) /* (6-7) */
> + else if (newentry) /* (6-7) */
>   ln->ln_router = 0;
>   break;
>   case ND_ROUTER_SOLICIT:
> @@ -1283,8 +1283,8 @@ fail:
>   /*
>* Mark an entry with lladdr as a router.
>*/
> - if ((!is_newentry && (olladdr || lladdr)) ||/* (2-5) */
> - (is_newentry && lladdr)) {  /* (7) */
> + if ((!newentry && (olladdr || lladdr)) ||   /* (2-5) */
> + (newentry && lladdr)) { /* (7) */
>   ln->ln_router = 1;
>   }
>   break;
>

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

slaacd(8): delete autoconf or temporary address on interface flag removal

[Florian Obser]

I just fixed the case where autoconf and temporary addresses stayed
behind when the interface no longer has inet6 autoconf and inet6
temporary.
This deletes addresses when one removes the temporary or autoconf flag
but the other one is still set.

OK?

(This needs rev 1.82 of engine.c to work correctly)

diff --git engine.c engine.c
index 226342364e6..1e67ab11a45 100644
--- engine.c
+++ engine.c
@@ -1930,6 +1930,20 @@ update_iface_ra_prefix(struct slaacd_iface *iface, 
struct radv *ra,
 
found = found_temporary = duplicate_found = 0;
 
+   if (!!iface->autoconf != !!iface->temporary) {
+   struct address_proposal *tmp;
+   /*
+* if only the autoconf or temporary flag is set check if we
+* have the "other kind" of address configured at delete it
+*/
+   LIST_FOREACH_SAFE (addr_proposal, &iface->addr_proposals,
+   entries, tmp) {
+   if ((!addr_proposal->temporary && !iface->autoconf) ||
+   (addr_proposal->temporary && !iface->temporary))
+   free_address_proposal(addr_proposal);
+   }
+   }
+
LIST_FOREACH(addr_proposal, &iface->addr_proposals, entries) {
if (prefix->prefix_len == addr_proposal-> prefix_len &&
memcmp(&prefix->prefix, &addr_proposal->prefix,



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

Re: nd6: Zap nd6_recalc_reachtm_interval indirection

[Florian Obser]

On 2022-07-22 14:27 +02, Claudio Jeker  wrote:
> On Fri, Jul 22, 2022 at 12:18:34PM +, Klemens Nanni wrote:
>> Only used once, so use the macro directly like ND6_SLOWTIMER_INTERVAL
>> is used in many places.
>> 
>> OK?
>
> Is that a value that should be adjustable?

I don't think so, this is the amount of time that has to elapse until a
new random time for the reachability timer is calculated. This is like
three layers deep in nd6, this is not a knob that needs twiddling.

It's also not a knob that RFC 4861 specifies, let alone specifies as
twiddleable.

OK florian

>  
>> diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c
>> index ff679bcb151..3decec947c4 100644
>> --- a/sys/netinet6/nd6.c
>> +++ b/sys/netinet6/nd6.c
>> @@ -88,8 +88,6 @@ TAILQ_HEAD(llinfo_nd6_head, llinfo_nd6) nd6_list;
>>  struct  pool nd6_pool;  /* pool for llinfo_nd6 structures */
>>  int nd6_inuse;
>>  
>> -int nd6_recalc_reachtm_interval = ND6_RECALC_REACHTM_INTERVAL;
>> -
>>  void nd6_timer(void *);
>>  void nd6_slowtimo(void *);
>>  void nd6_expire(void *);
>> @@ -1318,7 +1316,7 @@ nd6_slowtimo(void *ignored_arg)
>>   * value gets recomputed at least once every few hours.
>>   * (RFC 2461, 6.3.4)
>>   */
>> -nd6if->recalctm = nd6_recalc_reachtm_interval;
>> +nd6if->recalctm = ND6_RECALC_REACHTM_INTERVAL;
>>  nd6if->reachable = 
>> ND_COMPUTE_RTIME(nd6if->basereachable);
>>  }
>>  }
>> 
>
> -- 
> :wq Claudio
>

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

Re: Remove support for CH and HS classes from dig(1)

[Florian Obser]

On 2022-07-18 14:52 +01, Ricardo Mestre  wrote:
> Hi,
>
> I'm too young to ever know there were other types of networks still supported 
> by
> dig(1), but it seems it's a thing. Found while reading [0].
>
> Realistically speaking do we want to keep supporting these kind of ancient
> networks on our version? Is there still someone out there using them?

yes, I use it daily.

$ dig @k.root-servers.net +norec +noall +answer hostname.bind ch txt
hostname.bind.  0   CH  TXT "ns1.ch-gva.k.ripe.net"
$ dig @k.root-servers.net +norec +noall +answer version.bind ch txt
version.bind.   0   CH  TXT "NSD"
$ dig @k.root-servers.net +norec +noall +answer id.server ch txt
id.server.  0   CH  TXT "ns1.ch-gva.k.ripe.net"
$ dig @k.root-servers.net +norec +noall +answer version.server ch txt
version.server. 0   CH  TXT "NSD"

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

Re: dhcpleased(8): close unneeded bpf FDs

[Florian Obser]

On 2022-07-12 14:35 +02, Florian Obser  wrote:
> When the autoconf flag flaps around we might end up with multiple bpf
> FDs in flight. Things then get confusing. The kernel tells us we can
> read from the bpf FD but the data is actually "on the other FD", so
> read(2) returns 0.
>
> Found the hard way by, and patiently debugged with weerd@
>
> One way to trigger this is booting a vmm VM where dhcpleased(8)'s
> init_ifaces() loses a race against netstart(8). init_ifaces() would
> already see the autoconf flag and request a bpf FD.
> But then it would receive a RTM_IFINFO message without the autoconf flag
> set from when the interface came up. Then it will see another RTM_IFINFO
> message with the autoconf flag set and request yet another bpf FD. If
> the first bpf FD had not arrived yet we ended up with two in the frontend
> process.
>
> While here make sure a bpf FD has been configured before calling
> close(2) on it.
>
> OK?
>

Anyone? This is a slightly annoying bug... Difficult to hit though.


diff --git frontend.c frontend.c
index 2959a6c0d0f..275e5352e0b 100644
--- frontend.c
+++ frontend.c
@@ -1170,8 +1170,10 @@ remove_iface(uint32_t if_index)
return;
 
LIST_REMOVE(iface, entries);
-   event_del(&iface->bpfev.ev);
-   close(EVENT_FD(&iface->bpfev.ev));
+   if (event_initialized(&iface->bpfev.ev)) {
+   event_del(&iface->bpfev.ev);
+   close(EVENT_FD(&iface->bpfev.ev));
+   }
if (iface->udpsock != -1)
close(iface->udpsock);
free(iface);
@@ -1185,7 +1187,13 @@ set_bpfsock(int bpfsock, uint32_t if_index)
if ((iface = get_iface_by_id(if_index)) == NULL) {
/*
 * The interface disappeared while we were waiting for the
-* parent process to open the raw socket.
+* parent process to open the bpf socket.
+*/
+   close(bpfsock);
+   } else if (event_initialized(&iface->bpfev.ev)) {
+   /*
+* The autoconf flag is flapping and we have multiple bpf 
sockets in
+* flight. We don't need this one because we already got one.
 */
close(bpfsock);
} else {


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

dhcpleased(8): close unneeded bpf FDs

[Florian Obser]

When the autoconf flag flaps around we might end up with multiple bpf
FDs in flight. Things then get confusing. The kernel tells us we can
read from the bpf FD but the data is actually "on the other FD", so
read(2) returns 0.

Found the hard way by, and patiently debugged with weerd@

One way to trigger this is booting a vmm VM where dhcpleased(8)'s
init_ifaces() loses a race against netstart(8). init_ifaces() would
already see the autoconf flag and request a bpf FD.
But then it would receive a RTM_IFINFO message without the autoconf flag
set from when the interface came up. Then it will see another RTM_IFINFO
message with the autoconf flag set and request yet another bpf FD. If
the first bpf FD had not arrived yet we ended up with two in the frontend
process.

While here make sure a bpf FD has been configured before calling
close(2) on it.

OK?

diff --git frontend.c frontend.c
index 2959a6c0d0f..275e5352e0b 100644
--- frontend.c
+++ frontend.c
@@ -1170,8 +1170,10 @@ remove_iface(uint32_t if_index)
return;
 
LIST_REMOVE(iface, entries);
-   event_del(&iface->bpfev.ev);
-   close(EVENT_FD(&iface->bpfev.ev));
+   if (event_initialized(&iface->bpfev.ev)) {
+   event_del(&iface->bpfev.ev);
+   close(EVENT_FD(&iface->bpfev.ev));
+   }
if (iface->udpsock != -1)
close(iface->udpsock);
free(iface);
@@ -1185,7 +1187,13 @@ set_bpfsock(int bpfsock, uint32_t if_index)
if ((iface = get_iface_by_id(if_index)) == NULL) {
/*
 * The interface disappeared while we were waiting for the
-* parent process to open the raw socket.
+* parent process to open the bpf socket.
+*/
+   close(bpfsock);
+   } else if (event_initialized(&iface->bpfev.ev)) {
+   /*
+* The autoconf flag is flapping and we have multiple bpf 
sockets in
+* flight. We don't need this one because we already got one.
 */
close(bpfsock);
} else {

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

Re: dig(1): SVCB and HTTPS RR types

[Florian Obser]

anyone?

On 2022-06-25 13:15 +02, Florian Obser  wrote:
> See https://datatracker.ietf.org/doc/draft-ietf-dnsop-svcb-https/
>
> $ ./obj/dig @8.8.8.8 +norec _dns.resolver.arpa svcb
>
> ; <<>> dig 9.10.8-P1 <<>> @8.8.8.8 +norec _dns.resolver.arpa svcb
> ; (1 server found)
> ;; global options: +cmd
> ;; Got answer:
> ;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 21245
> ;; flags: qr aa ra; QUERY: 1, ANSWER: 2, AUTHORITY: 0, ADDITIONAL: 4
>
> ;; QUESTION SECTION:
> ;_dns.resolver.arpa.IN  SVCB
>
> ;; ANSWER SECTION:
> _dns.resolver.arpa. 86400   IN  SVCB1 dns.google. alpn="dot"
> _dns.resolver.arpa.  86400 IN SVCB 2 dns.google. alpn="h2,h3"
> dohpath="/dns-query{?dns}"
>
> ;; ADDITIONAL SECTION:
> dns.google. 86400   IN  A   8.8.8.8
> dns.google. 86400   IN  A   8.8.4.4
> dns.google. 86400   IN  2001:4860:4860::
> dns.google. 86400   IN  2001:4860:4860::8844
>
> ;; Query time: 11 msec
> ;; SERVER: 8.8.8.8#53(8.8.8.8)
> ;; WHEN: Sat Jun 25 13:08:21 CEST 2022
> ;; MSG SIZE  rcvd: 224
>
> $ ./obj/dig +dnssec cloudflare.com https
>
> ; <<>> dig 9.10.8-P1 <<>> +dnssec cloudflare.com https
> ;; global options: +cmd
> ;; Got answer:
> ;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 22508
> ;; flags: qr rd ra ad; QUERY: 1, ANSWER: 2, AUTHORITY: 0, ADDITIONAL: 0
>
> ;; QUESTION SECTION:
> ;cloudflare.com.IN  HTTPS
>
> ;; ANSWER SECTION:
> cloudflare.com.  217 IN HTTPS 1 . alpn="h3,h3-29,h2"
> ipv4hint=104.16.132.229,104.16.133.229
> ipv6hint=2606:4700::6810:84e5,2606:4700::6810:85e5
> cloudflare.com.  217 IN RRSIG HTTPS 13 2 300 20220626120906
> 20220624100906 34505
> cloudflare.com. PbQwTGVBW2MIXubouK2vUo92UNvlJ874KCrqah/Or21Jo2oDxfgI15jA
> 8z/Q6mseLPWIlTxex+KoIqv9y+FNjg==
>
> ;; Query time: 0 msec
> ;; SERVER: 127.0.0.1#53(127.0.0.1)
> ;; WHEN: Sat Jun 25 13:10:29 CEST 2022
> ;; MSG SIZE  rcvd: 221
>
> OK?

diff --git lib/dns/include/dns/types.h lib/dns/include/dns/types.h
index 63ea8d67f51..7085ce29f2e 100644
--- lib/dns/include/dns/types.h
+++ lib/dns/include/dns/types.h
@@ -139,6 +139,8 @@ enum {
dns_rdatatype_openpgpkey = 61,
dns_rdatatype_csync = 62,
dns_rdatatype_zonemd = 63,
+   dns_rdatatype_svcb = 64,
+   dns_rdatatype_https = 65,
dns_rdatatype_spf = 99,
dns_rdatatype_unspec = 103,
dns_rdatatype_nid = 104,
diff --git lib/dns/rdata.c lib/dns/rdata.c
index c27409efc3c..d731eb3a846 100644
--- lib/dns/rdata.c
+++ lib/dns/rdata.c
@@ -775,6 +775,7 @@ dns_rdatatype_fromtext(dns_rdatatype_t *typep, 
isc_textregion_t *source) {
{"gpos",27},
{"hinfo",   13},
{"hip", 55},
+   {"https",   65},
{"ipseckey",45},
{"isdn",20},
{"ixfr",251},
@@ -822,6 +823,7 @@ dns_rdatatype_fromtext(dns_rdatatype_t *typep, 
isc_textregion_t *source) {
{"spf", 99},
{"srv", 33},
{"sshfp",   44},
+   {"svcb",64},
{"ta",  32768},
{"talink",  58},
{"tkey",249},
@@ -1006,6 +1008,10 @@ dns_rdatatype_totext(dns_rdatatype_t type, isc_buffer_t 
*target) {
return (isc_str_tobuffer("CSYNC", target));
case 63:
return (isc_str_tobuffer("ZONEMD", target));
+   case 64:
+       return (isc_str_tobuffer("SVCB", target));
+   case 65:
+   return (isc_str_tobuffer("HTTPS", target));
case 99:
return (isc_str_tobuffer("SPF", target));
case 100:
diff --git lib/dns/rdata/in_1/https_65.c lib/dns/rdata/in_1/https_65.c
new file mode 100644
index 000..23d80f8d352
--- /dev/null
+++ lib/dns/rdata/in_1/https_65.c
@@ -0,0 +1,48 @@
+/*
+ * Copyright (C) 2022 Florian Obser 
+ *
+ * Permission to use, copy, modify, and/or 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 ISC DISCLAIMS ALL WARRANTIES WITH
+ * REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+ * INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY

Re: slaacd(8): state machine rewrite, improved roaming support

[Florian Obser]

this one works better on real wifi, which can transition down -> down

diff --git engine.c engine.c
index db6d619abf1..4703c3ec7c7 100644
--- engine.c
+++ engine.c
@@ -100,23 +100,13 @@
 
 enum if_state {
IF_DOWN,
-   IF_DELAY,
-   IF_PROBE,
-   IF_IDLE,
-   IF_DEAD,
-};
-
-const char* if_state_name[] = {
-   "IF_DOWN",
-   "IF_DELAY",
-   "IF_PROBE",
-   "IF_IDLE",
-   "IF_DEAD",
+   IF_INIT,
+   IF_BOUND,
 };
 
 enum proposal_state {
+   PROPOSAL_IF_DOWN,
PROPOSAL_NOT_CONFIGURED,
-   PROPOSAL_SENT,
PROPOSAL_CONFIGURED,
PROPOSAL_NEARLY_EXPIRED,
PROPOSAL_WITHDRAWN,
@@ -124,16 +114,6 @@ enum proposal_state {
PROPOSAL_STALE,
 };
 
-const char* proposal_state_name[] = {
-   "NOT_CONFIGURED",
-   "SENT",
-   "CONFIGURED",
-   "NEARLY_EXPIRED",
-   "WITHDRAWN",
-   "DUPLICATED",
-   "STALE",
-};
-
 const char* rpref_name[] = {
"Low",
"Medium",
@@ -181,12 +161,13 @@ struct address_proposal {
struct event timer;
int64_t  id;
enum proposal_state  state;
-   time_t   next_timeout;
+   struct timeval   timo;
struct timespec  created;
struct timespec  when;
struct timespec  uptime;
uint32_t if_index;
struct ether_addrhw_address;
+   struct sockaddr_in6  from;
struct sockaddr_in6  addr;
struct in6_addr  mask;
struct in6_addr  prefix;
@@ -204,7 +185,7 @@ struct dfr_proposal {
struct event timer;
int64_t  id;
enum proposal_state  state;
-   time_t   next_timeout;
+   struct timeval   timo;
struct timespec  when;
struct timespec  uptime;
uint32_t if_index;
@@ -219,7 +200,7 @@ struct rdns_proposal {
struct event timer;
int64_t  id;
enum proposal_state  state;
-   time_t   next_timeout;
+   struct timeval   timo;
struct timespec  when;
struct timespec  uptime;
uint32_t if_index;
@@ -234,6 +215,8 @@ struct slaacd_iface {
LIST_ENTRY(slaacd_iface) entries;
enum if_statestate;
struct event timer;
+   struct timeval   timo;
+   struct timespec  last_sol;
int  probes;
uint32_t if_index;
uint32_t rdomain;
@@ -264,12 +247,19 @@ void   engine_showinfo_ctl(struct 
imsg *, uint32_t);
 voiddebug_log_ra(struct imsg_ra *);
 int in6_mask2prefixlen(struct in6_addr *);
 #endif /* SMALL */
-voiddeprecate_all_proposals(struct slaacd_iface *);
-voidsend_rdns_withdraw(struct slaacd_iface *);
 struct slaacd_iface*get_slaacd_iface_by_id(uint32_t);
 voidremove_slaacd_iface(uint32_t);
 voidfree_ra(struct radv *);
+voidiface_state_transition(struct slaacd_iface *, enum
+if_state);
+voidaddr_proposal_state_transition(struct
+address_proposal *, enum proposal_state);
+voiddfr_proposal_state_transition(struct dfr_proposal *,
+enum proposal_state);
+voidrdns_proposal_state_transition(struct rdns_proposal *,
+enum proposal_state);
 voidengine_update_iface(struct imsg_ifinfo *);
+voidrequest_solicitation(struct slaacd_iface *);
 voidparse_ra(struct slaacd_iface *, struct imsg_ra *);
 voidgen_addr(struct slaacd_iface *, struct radv_prefix *,
 struct address_proposal *, int);
@@ -277,7 +267,6 @@ void gen_address_proposal(struct 
slaacd_iface *, struct
 radv *, struct radv_prefix *, int);
 voidfree_address_proposal(struct address_proposal *);
 voidwithdraw_addr(struct address_proposal *);
-voidtimeout_from_lifetime(struct address_proposal *);
 voidconfigure_address(struct address_proposal *);
 voidin6_prefixlen2mask(struct in6_addr *, int len);
 voidgen_dfr_propo

nsd 4.6.0

[Florian Obser]

OK?

diff --git Makefile.in Makefile.in
index b6b7eb37570..96d0784f610 100644
--- Makefile.in
+++ Makefile.in
@@ -81,13 +81,13 @@ MANUALS=nsd.8 nsd-checkconf.8 nsd-checkzone.8 nsd-control.8 
nsd.conf.5
 
 COMMON_OBJ=answer.o axfr.o ixfr.o ixfrcreate.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 
siphash.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
+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 verify.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_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 
verify.o
 NSD_CONTROL_OBJ=$(COMMON_OBJ) nsd-control.o
-CUTEST_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 cutest_dname.o 
cutest_dns.o cutest_iterated_hash.o cutest_run.o cutest_radtree.o 
cutest_rbtree.o cutest_namedb.o cutest_options.o cutest_region.o cutest_rrl.o 
cutest_udb.o cutest_udbrad.o cutest_util.o cutest_bitset.o cutest_popen3.o 
cutest_iter.o cutest_event.o cutest.o qtest.o
-NSD_MEM_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-mem.o
+CUTEST_OBJ=$(COMMON_OBJ) $(XFRD_OBJ) dbaccess.o dbcreate.o difffile.o ipc.o 
mini_event.o netio.o server.o verify.o zonec.o zparser.o zlexer.o 
cutest_dname.o cutest_dns.o cutest_iterated_hash.o cutest_run.o 
cutest_radtree.o cutest_rbtree.o cutest_namedb.o cutest_options.o 
cutest_region.o cutest_rrl.o cutest_udb.o cutest_udbrad.o cutest_util.o 
cutest_bitset.o cutest_popen3.o cutest_iter.o cutest_event.o cutest.o qtest.o
+NSD_MEM_OBJ=$(COMMON_OBJ) $(XFRD_OBJ) dbaccess.o dbcreate.o difffile.o ipc.o 
mini_event.o netio.o verify.o server.o zonec.o zparser.o zlexer.o nsd-mem.o
 all:   $(TARGETS) $(MANUALS)
 
 $(ALL_OBJ):
@@ -496,7 +496,7 @@ rrl.o: $(srcdir)/rrl.c config.h $(srcdir)/rrl.h 
$(srcdir)/query.h $(srcdir)/name
 server.o: $(srcdir)/server.c config.h $(srcdir)/axfr.h $(srcdir)/nsd.h 
$(srcdir)/dns.h $(srcdir)/edns.h $(srcdir)/buffer.h \
  $(srcdir)/region-allocator.h $(srcdir)/util.h $(srcdir)/query.h 
$(srcdir)/namedb.h $(srcdir)/dname.h $(srcdir)/radtree.h $(srcdir)/rbtree.h \
  $(srcdir)/packet.h $(srcdir)/tsig.h $(srcdir)/netio.h $(srcdir)/xfrd.h 
$(srcdir)/options.h $(srcdir)/xfrd-tcp.h $(srcdir)/xfrd-disk.h \
- $(srcdir)/difffile.h $(srcdir)/udb.h $(srcdir)/nsec3.h $(srcdir)/ipc.h 
$(srcdir)/remote.h $(srcdir)/lookup3.h $(srcdir)/dnstap/dnstap_collector.h 
$(srcdir)/rrl.h $(srcdir)/ixfr.h
+ $(srcdir)/difffile.h $(srcdir)/udb.h $(srcdir)/nsec3.h $(srcdir)/ipc.h 
$(srcdir)/remote.h $(srcdir)/lookup3.h $(srcdir)/dnstap/dnstap_collector.h 
$(srcdir)/rrl.h $(srcdir)/ixfr.h $(srcdir)/verify.h
 siphash.o: $(srcdir)/siphash.c
 tsig.o: $(srcdir)/tsig.c config.h $(srcdir)/tsig.h $(srcdir)/buffer.h 
$(srcdir)/region-allocator.h $(srcdir)/util.h $(srcdir)/dname.h \
  $(srcdir)/tsig-openssl.h $(srcdir)/dns.h $(srcdir)/packet.h 
$(srcdir)/namedb.h $(srcdir)/radtree.h $(srcdir)/rbtree.h $(srcdir)/query.h 
$(srcdir)/nsd.h \
@@ -511,6 +511,9 @@ udbzone.o: $(srcdir)/udbzone.c config.h $(srcdir)/udbzone.h 
$(srcdir)/udb.h $(sr
 util.o: $(srcdir)/util.c config.h $(srcdir)/util.h 
$(srcdir)/region-allocator.h $(srcdir)/dname.h $(srcdir)/buffer.h \
  $(srcdir)/namedb.h $(srcdir)/dns.h $(srcdir)/radtree.h $(srcdir)/rbtree.h 
$(srcdir)/rdata.h $(srcdir)/zonec.h
 bitset.o: $(srcdir)/bitset.c $(srcdir)/bitset.h
+verify.o: $(srcdir)/verify.c config.h $(srcdir)/region-allocator.h 
$(srcdir)/namedb.h $(srcdir)/dname.h $(srcdir)/buffer.h \
+ $(srcdir)/util.h config.h $(srcdir)/dns.h $(srcdir)/rbtree.h $(srcdir)/nsd.h 
$(srcdir)/edns.h $(srcdir)/options.h $(srcdir)/difffile.h \
+ $(srcdir)/netio.h $(srcdir)/verify.h
 xfrd.o: $(srcdir)/xfrd.c config.h $(srcdir)/xfrd.h $(srcdir)/rbtree.h 
$(srcdir)/region-allocator.h $(srcdir)/namedb.h \
  $(srcdir)/dname.h $(srcdir)/buffer.h $(srcdir)/util.h $(srcdir)/dns.h 
$(srcdir)/radtree.h $(srcdir)/options.h $(srcdir)/tsig.h $(srcdir)/xfrd-tcp.h \
  $(srcdir)/xfrd-disk.h $(srcdir)/xfrd-notify.h $(srcdir)/netio.h 
$(srcdir)/nsd.h $(srcdir)/edns.h $(srcdir)/packet.h $(srcdir)/rdata.h \
diff --git config.h.in config.h.in
index 34a89602063..741669c83fe 100644
--- co

slaacd(8): state machine rewrite, improved roaming support

[Florian Obser]

This rewrite was inspired by what we learned in dhcpleased.
I find state_transition / timeout split easier to reason about.

This also fixes a bunch of bugs, like remove stale IPs / routes / DNS
servers when moving from one IPv6 enabled network to another.

Tests, comments, OKs?

diff --git engine.c engine.c
index db6d619abf1..a6a29881600 100644
--- engine.c
+++ engine.c
@@ -100,21 +100,16 @@
 
 enum if_state {
IF_DOWN,
+   IF_INIT,
+   IF_BOUND,
IF_DELAY,
IF_PROBE,
IF_IDLE,
IF_DEAD,
 };
 
-const char* if_state_name[] = {
-   "IF_DOWN",
-   "IF_DELAY",
-   "IF_PROBE",
-   "IF_IDLE",
-   "IF_DEAD",
-};
-
 enum proposal_state {
+   PROPOSAL_IF_DOWN,
PROPOSAL_NOT_CONFIGURED,
PROPOSAL_SENT,
PROPOSAL_CONFIGURED,
@@ -124,16 +119,6 @@ enum proposal_state {
PROPOSAL_STALE,
 };
 
-const char* proposal_state_name[] = {
-   "NOT_CONFIGURED",
-   "SENT",
-   "CONFIGURED",
-   "NEARLY_EXPIRED",
-   "WITHDRAWN",
-   "DUPLICATED",
-   "STALE",
-};
-
 const char* rpref_name[] = {
"Low",
"Medium",
@@ -181,12 +166,13 @@ struct address_proposal {
struct event timer;
int64_t  id;
enum proposal_state  state;
-   time_t   next_timeout;
+   struct timeval   timo;
struct timespec  created;
struct timespec  when;
struct timespec  uptime;
uint32_t if_index;
struct ether_addrhw_address;
+   struct sockaddr_in6  from;
struct sockaddr_in6  addr;
struct in6_addr  mask;
struct in6_addr  prefix;
@@ -204,7 +190,7 @@ struct dfr_proposal {
struct event timer;
int64_t  id;
enum proposal_state  state;
-   time_t   next_timeout;
+   struct timeval   timo;
struct timespec  when;
struct timespec  uptime;
uint32_t if_index;
@@ -219,7 +205,7 @@ struct rdns_proposal {
struct event timer;
int64_t  id;
enum proposal_state  state;
-   time_t   next_timeout;
+   struct timeval   timo;
struct timespec  when;
struct timespec  uptime;
uint32_t if_index;
@@ -234,6 +220,8 @@ struct slaacd_iface {
LIST_ENTRY(slaacd_iface) entries;
enum if_statestate;
struct event timer;
+   struct timeval   timo;
+   struct timespec  last_sol;
int  probes;
uint32_t if_index;
uint32_t rdomain;
@@ -265,11 +253,19 @@ void   debug_log_ra(struct imsg_ra *);
 int in6_mask2prefixlen(struct in6_addr *);
 #endif /* SMALL */
 voiddeprecate_all_proposals(struct slaacd_iface *);
-voidsend_rdns_withdraw(struct slaacd_iface *);
 struct slaacd_iface*get_slaacd_iface_by_id(uint32_t);
 voidremove_slaacd_iface(uint32_t);
 voidfree_ra(struct radv *);
+voidiface_state_transition(struct slaacd_iface *, enum
+if_state);
+voidaddr_proposal_state_transition(struct
+address_proposal *, enum proposal_state);
+voiddfr_proposal_state_transition(struct dfr_proposal *,
+enum proposal_state);
+voidrdns_proposal_state_transition(struct rdns_proposal *,
+enum proposal_state);
 voidengine_update_iface(struct imsg_ifinfo *);
+voidrequest_solicitation(struct slaacd_iface *);
 voidparse_ra(struct slaacd_iface *, struct imsg_ra *);
 voidgen_addr(struct slaacd_iface *, struct radv_prefix *,
 struct address_proposal *, int);
@@ -277,7 +273,6 @@ void gen_address_proposal(struct 
slaacd_iface *, struct
 radv *, struct radv_prefix *, int);
 voidfree_address_proposal(struct address_proposal *);
 voidwithdraw_addr(struct address_proposal *);
-voidtimeout_from_lifetime(struct address_proposal *);
 voidconfigure_address(struct address_proposal *);
 voidin6_prefi

Re: snmpd(8): Add rudimentary AgentX support

[Florian Obser]

On 2022-06-27 13:32 +02, Martijn van Duren  
wrote:
> For the group-id I went with 92, which was used by _rtadvd. It's one up
> from _snmpd and has been used previously by _rtadvd, which should make
> it the perfect candidate. According to florian rtadvd never stored
> anything on disk and chances of any lingering rtadvd binary still around
> on a modern install still running approach 0. So I don't see any risks
> there. If someone kept the old user/group around sysmerge should change
> _rtadvd to _agentx, which florian, sthen, and I also don't see an issue
> with.

What I mean was, if they didn't clean up they still have _rtadvd:_rtadvd
which will become _rtadvd:_agentx. Operationally this should not a problem,
nothing is supposed to use that user.

dig(1): no trust

[Florian Obser]

A day without a removal diff for dig is a sad day, let's have a happy
day!

OK?

diff --git lib/dns/include/dns/rdataset.h lib/dns/include/dns/rdataset.h
index 785821dabf2..26003cfaad4 100644
--- lib/dns/include/dns/rdataset.h
+++ lib/dns/include/dns/rdataset.h
@@ -86,7 +86,6 @@ struct dns_rdataset {
dns_rdataclass_trdclass;
dns_rdatatype_t type;
dns_ttl_t   ttl;
-   dns_trust_t trust;
dns_rdatatype_t covers;
/*
 * attributes
diff --git lib/dns/include/dns/types.h lib/dns/include/dns/types.h
index 63ea8d67f51..a3b2f7e31f2 100644
--- lib/dns/include/dns/types.h
+++ lib/dns/include/dns/types.h
@@ -54,7 +54,6 @@ typedef struct dns_rdatalist  dns_rdatalist_t;
 typedef struct dns_rdatasetdns_rdataset_t;
 typedef uint16_t   dns_rdatatype_t;
 typedef uint8_tdns_secalg_t;
-typedef uint16_t   dns_trust_t;
 typedef struct dns_tsigkey dns_tsigkey_t;
 typedef uint32_t   dns_ttl_t;
 typedef struct dns_viewdns_view_t;
@@ -231,55 +230,6 @@ enum {
 #define dns_opcode_update  ((dns_opcode_t)dns_opcode_update)
 };
 
-/*%
- * Trust levels.  Must be kept in sync with trustnames[] in masterdump.c.
- */
-enum {
-   /* Sentinel value; no data should have this trust level. */
-   dns_trust_none = 0,
-#define dns_trust_none ((dns_trust_t)dns_trust_none)
-
-   /*%
-* Subject to DNSSEC validation but has not yet been validated
-* dns_trust_pending_additional (from the additional section).
-*/
-   dns_trust_pending_additional = 1,
-#define dns_trust_pending_additional \
-((dns_trust_t)dns_trust_pending_additional)
-
-   dns_trust_pending_answer = 2,
-#define dns_trust_pending_answer   ((dns_trust_t)dns_trust_pending_answer)
-
-   /*% Received in the additional section of a response. */
-   dns_trust_additional = 3,
-#define dns_trust_additional   ((dns_trust_t)dns_trust_additional)
-
-   /* Received in a referral response. */
-   dns_trust_glue = 4,
-#define dns_trust_glue ((dns_trust_t)dns_trust_glue)
-
-   /* Answer from a non-authoritative server */
-   dns_trust_answer = 5,
-#define dns_trust_answer   ((dns_trust_t)dns_trust_answer)
-
-   /*  Received in the authority section as part of an
-   authoritative response */
-   dns_trust_authauthority = 6,
-#define dns_trust_authauthority
((dns_trust_t)dns_trust_authauthority)
-
-   /* Answer from an authoritative server */
-   dns_trust_authanswer = 7,
-#define dns_trust_authanswer   ((dns_trust_t)dns_trust_authanswer)
-
-   /* Successfully DNSSEC validated */
-   dns_trust_secure = 8,
-#define dns_trust_secure   ((dns_trust_t)dns_trust_secure)
-
-   /* This server is authoritative */
-   dns_trust_ultimate = 9
-#define dns_trust_ultimate ((dns_trust_t)dns_trust_ultimate)
-};
-
 /*
  * Functions.
  */
diff --git lib/dns/message.c lib/dns/message.c
index 64053ead3e7..77a77b1cb9c 100644
--- lib/dns/message.c
+++ lib/dns/message.c
@@ -1674,8 +1674,7 @@ dns_message_rendersection(dns_message_t *msg, 
dns_section_t sectionid)
 * If we have rendered non-validated data,
 * ensure that the AD bit is not set.
 */
-   if (rdataset->trust != dns_trust_secure &&
-   (sectionid == DNS_SECTION_ANSWER ||
+   if ((sectionid == DNS_SECTION_ANSWER ||
 sectionid == DNS_SECTION_AUTHORITY))
msg->flags &= ~DNS_MESSAGEFLAG_AD;
 
diff --git lib/dns/rdatalist.c lib/dns/rdatalist.c
index a715a89c762..a6ffdc346a8 100644
--- lib/dns/rdatalist.c
+++ lib/dns/rdatalist.c
@@ -70,7 +70,6 @@ dns_rdatalist_tordataset(dns_rdatalist_t *rdatalist,
rdataset->type = rdatalist->type;
rdataset->covers = rdatalist->covers;
rdataset->ttl = rdatalist->ttl;
-   rdataset->trust = 0;
rdataset->private1 = rdatalist;
rdataset->private2 = NULL;
 
diff --git lib/dns/rdataset.c lib/dns/rdataset.c
index 18e7854a144..6e6390aa66a 100644
--- lib/dns/rdataset.c
+++ lib/dns/rdataset.c
@@ -41,7 +41,6 @@ dns_rdataset_init(dns_rdataset_t *rdataset) {
rdataset->rdclass = 0;
rdataset->type = 0;
rdataset->ttl = 0;
-   rdataset->trust = 0;
rdataset->covers = 0;
rdataset->attributes = 0;
rdataset->count = UINT32_MAX;
@@ -64,7 +63,6 @@ dns_rdataset_disassociate(dns_rdataset_t *rdataset) {
rdataset->rdclass = 0;
rd

dig(1): SVCB and HTTPS RR types

[Florian Obser]

See https://datatracker.ietf.org/doc/draft-ietf-dnsop-svcb-https/

$ ./obj/dig @8.8.8.8 +norec _dns.resolver.arpa svcb

; <<>> dig 9.10.8-P1 <<>> @8.8.8.8 +norec _dns.resolver.arpa svcb
; (1 server found)
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 21245
;; flags: qr aa ra; QUERY: 1, ANSWER: 2, AUTHORITY: 0, ADDITIONAL: 4

;; QUESTION SECTION:
;_dns.resolver.arpa.IN  SVCB

;; ANSWER SECTION:
_dns.resolver.arpa. 86400   IN  SVCB1 dns.google. alpn="dot"
_dns.resolver.arpa. 86400   IN  SVCB2 dns.google. alpn="h2,h3" 
dohpath="/dns-query{?dns}"

;; ADDITIONAL SECTION:
dns.google. 86400   IN  A   8.8.8.8
dns.google. 86400   IN  A   8.8.4.4
dns.google. 86400   IN  2001:4860:4860::
dns.google. 86400   IN  2001:4860:4860::8844

;; Query time: 11 msec
;; SERVER: 8.8.8.8#53(8.8.8.8)
;; WHEN: Sat Jun 25 13:08:21 CEST 2022
;; MSG SIZE  rcvd: 224

$ ./obj/dig +dnssec cloudflare.com https

; <<>> dig 9.10.8-P1 <<>> +dnssec cloudflare.com https
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 22508
;; flags: qr rd ra ad; QUERY: 1, ANSWER: 2, AUTHORITY: 0, ADDITIONAL: 0

;; QUESTION SECTION:
;cloudflare.com.IN  HTTPS

;; ANSWER SECTION:
cloudflare.com. 217 IN  HTTPS   1 . alpn="h3,h3-29,h2" 
ipv4hint=104.16.132.229,104.16.133.229 
ipv6hint=2606:4700::6810:84e5,2606:4700::6810:85e5
cloudflare.com. 217 IN  RRSIG   HTTPS 13 2 300 20220626120906 
20220624100906 34505 cloudflare.com. 
PbQwTGVBW2MIXubouK2vUo92UNvlJ874KCrqah/Or21Jo2oDxfgI15jA 
8z/Q6mseLPWIlTxex+KoIqv9y+FNjg==

;; Query time: 0 msec
;; SERVER: 127.0.0.1#53(127.0.0.1)
;; WHEN: Sat Jun 25 13:10:29 CEST 2022
;; MSG SIZE  rcvd: 221

OK?

diff --git lib/dns/include/dns/types.h lib/dns/include/dns/types.h
index 63ea8d67f51..7085ce29f2e 100644
--- lib/dns/include/dns/types.h
+++ lib/dns/include/dns/types.h
@@ -139,6 +139,8 @@ enum {
dns_rdatatype_openpgpkey = 61,
dns_rdatatype_csync = 62,
dns_rdatatype_zonemd = 63,
+   dns_rdatatype_svcb = 64,
+   dns_rdatatype_https = 65,
dns_rdatatype_spf = 99,
dns_rdatatype_unspec = 103,
dns_rdatatype_nid = 104,
diff --git lib/dns/rdata.c lib/dns/rdata.c
index c27409efc3c..d731eb3a846 100644
--- lib/dns/rdata.c
+++ lib/dns/rdata.c
@@ -775,6 +775,7 @@ dns_rdatatype_fromtext(dns_rdatatype_t *typep, 
isc_textregion_t *source) {
{"gpos",27},
{"hinfo",   13},
{"hip", 55},
+   {"https",   65},
{"ipseckey",45},
{"isdn",20},
{"ixfr",251},
@@ -822,6 +823,7 @@ dns_rdatatype_fromtext(dns_rdatatype_t *typep, 
isc_textregion_t *source) {
{"spf", 99},
{"srv", 33},
{"sshfp",   44},
+   {"svcb",64},
{"ta",  32768},
{"talink",  58},
{"tkey",249},
@@ -1006,6 +1008,10 @@ dns_rdatatype_totext(dns_rdatatype_t type, isc_buffer_t 
*target) {
return (isc_str_tobuffer("CSYNC", target));
case 63:
return (isc_str_tobuffer("ZONEMD", target));
+   case 64:
+   return (isc_str_tobuffer("SVCB", target));
+   case 65:
+   return (isc_str_tobuffer("HTTPS", target));
case 99:
    return (isc_str_tobuffer("SPF", target));
case 100:
diff --git lib/dns/rdata/in_1/https_65.c lib/dns/rdata/in_1/https_65.c
new file mode 100644
index 000..23d80f8d352
--- /dev/null
+++ lib/dns/rdata/in_1/https_65.c
@@ -0,0 +1,48 @@
+/*
+ * Copyright (C) 2022 Florian Obser 
+ *
+ * Permission to use, copy, modify, and/or 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 ISC DISCLAIMS ALL WARRANTIES WITH
+ * REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS.  IN NO EVENT SHALL ISC 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.
+ */
+
+/* draft-ietf-dnsop-svcb-https-10 */
+
+#ifnd

dig(1): remove unused DNS_RDATASETATTR defines

[Florian Obser]

OK?

diff --git lib/dns/include/dns/rdataset.h lib/dns/include/dns/rdataset.h
index a2e86b62055..e2c453adc81 100644
--- lib/dns/include/dns/rdataset.h
+++ lib/dns/include/dns/rdataset.h
@@ -67,46 +67,6 @@ typedef struct dns_rdatasetmethods {
void(*clone)(dns_rdataset_t *source,
 dns_rdataset_t *target);
unsigned int(*count)(dns_rdataset_t *rdataset);
-   isc_result_t(*addnoqname)(dns_rdataset_t *rdataset,
- dns_name_t *name);
-   isc_result_t(*getnoqname)(dns_rdataset_t *rdataset,
- dns_name_t *name,
- dns_rdataset_t *neg,
- dns_rdataset_t *negsig);
-   isc_result_t(*addclosest)(dns_rdataset_t *rdataset,
- dns_name_t *name);
-   isc_result_t(*getclosest)(dns_rdataset_t *rdataset,
- dns_name_t *name,
- dns_rdataset_t *neg,
- dns_rdataset_t *negsig);
-   isc_result_t(*getadditional)(dns_rdataset_t *rdataset,
-dns_rdatasetadditional_t type,
-dns_rdatatype_t qtype,
-dns_acache_t *acache,
-dns_zone_t **zonep,
-dns_db_t **dbp,
-dns_dbversion_t **versionp,
-dns_dbnode_t **nodep,
-dns_name_t *fname,
-dns_message_t *msg,
-time_t now);
-   isc_result_t(*setadditional)(dns_rdataset_t *rdataset,
-dns_rdatasetadditional_t type,
-dns_rdatatype_t qtype,
-dns_acache_t *acache,
-dns_zone_t *zone,
-dns_db_t *db,
-dns_dbversion_t *version,
-dns_dbnode_t *node,
-dns_name_t *fname);
-   isc_result_t(*putadditional)(dns_acache_t *acache,
-dns_rdataset_t *rdataset,
-dns_rdatasetadditional_t type,
-dns_rdatatype_t qtype);
-   void(*settrust)(dns_rdataset_t *rdataset,
-   dns_trust_t trust);
-   void(*expire)(dns_rdataset_t *rdataset);
-   void(*clearprefetch)(dns_rdataset_t *rdataset);
 } dns_rdatasetmethods_t;
 
 /*%
@@ -139,11 +99,6 @@ struct dns_rdataset {
 * increment the counter.
 */
uint32_tcount;
-   /*
-* This RRSIG RRset should be re-generated around this time.
-* Only valid if DNS_RDATASETATTR_RESIGN is set in attributes.
-*/
-   time_t  resign;
/*@{*/
/*%
 * These are for use by the rdataset implementation, and MUST NOT
@@ -163,40 +118,10 @@ struct dns_rdataset {
 /*!
  * \def DNS_RDATASETATTR_RENDERED
  * Used by message.c to indicate that the rdataset was rendered.
- *
- * \def DNS_RDATASETATTR_TTLADJUSTED
- * Used by message.c to indicate that the rdataset's rdata had differing
- * TTL values, and the rdataset->ttl holds the smallest.
- *
- * \def DNS_RDATASETATTR_LOADORDER
- * Output the RRset in load order.
  */
 
 #define DNS_RDATASETATTR_QUESTION  0x0001
 #define DNS_RDATASETATTR_RENDERED  0x0002  /*%< Used by message.c 
*/
-#define DNS_RDATASETATTR_ANSWERED  0x0004  /*%< Used by server. */
-#define DNS_RDATASETATTR_CACHE 0x0008  /*%< Used by resolver. 
*/
-#define DNS_RDATASETATTR_ANSWER0x0010  /*%< Used by 
resolver. */
-#define DNS_RDATASETATTR_ANSWERSIG 0x0020  /*%< Used by resolver. 
*/
-#define DNS_RDATASETATTR_EXTERNAL  0x0040  /*%< Used by resolver. 
*/
-#define DNS_RDATASETATTR_NCACHE0x0080  /*%< Used by 
resolver. */
-#define DNS_RDATASETATTR_CHAINING  0x0100  /*%< Used by resolver. 
*/
-#define DNS_RDATASETATTR_TTLADJUSTED   0x0200  /*%< Used by message.c 
*/
-#define DNS_RDATASETATTR_CHASE 

dig(1): remove order and order_args

[Florian Obser]

We do not order RRsets and we are not interested in partial RRsets.

OK?

diff --git lib/dns/include/dns/message.h lib/dns/include/dns/message.h
index a70720eee39..7f547411bf0 100644
--- lib/dns/include/dns/message.h
+++ lib/dns/include/dns/message.h
@@ -226,9 +226,6 @@ struct dns_message {
dns_rcode_t sig0status;
isc_region_tquery;
isc_region_tsaved;
-
-   dns_rdatasetorderfunc_t order;
-   const void *order_arg;
 };
 
 struct dns_ednsopt {
diff --git lib/dns/include/dns/rdataset.h lib/dns/include/dns/rdataset.h
index 418c9b5284f..a2e86b62055 100644
--- lib/dns/include/dns/rdataset.h
+++ lib/dns/include/dns/rdataset.h
@@ -182,8 +182,6 @@ struct dns_rdataset {
 #define DNS_RDATASETATTR_NCACHE0x0080  /*%< Used by 
resolver. */
 #define DNS_RDATASETATTR_CHAINING  0x0100  /*%< Used by resolver. 
*/
 #define DNS_RDATASETATTR_TTLADJUSTED   0x0200  /*%< Used by message.c 
*/
-#define DNS_RDATASETATTR_FIXEDORDER0x0400
-#define DNS_RDATASETATTR_RANDOMIZE 0x0800
 #define DNS_RDATASETATTR_CHASE 0x1000  /*%< Used by resolver. 
*/
 #define DNS_RDATASETATTR_NXDOMAIN  0x2000
 #define DNS_RDATASETATTR_NOQNAME   0x4000
@@ -388,8 +386,6 @@ dns_rdataset_towiresorted(dns_rdataset_t *rdataset,
  const dns_name_t *owner_name,
  dns_compress_t *cctx,
  isc_buffer_t *target,
- dns_rdatasetorderfunc_t order,
- const void *order_arg,
  unsigned int *countp);
 /*%<
  * Like dns_rdataset_towire(), but sorting the rdatasets according to
diff --git lib/dns/message.c lib/dns/message.c
index 640ef497061..9fa8671d718 100644
--- lib/dns/message.c
+++ lib/dns/message.c
@@ -333,8 +333,6 @@ msginit(dns_message_t *m) {
m->tcp_continuation = 0;
m->verified_sig = 0;
m->verify_attempted = 0;
-   m->order = NULL;
-   m->order_arg = NULL;
m->query.base = NULL;
m->query.length = 0;
m->free_query = 0;
@@ -1657,8 +1655,6 @@ dns_message_rendersection(dns_message_t *msg, 
dns_section_t sectionid)
  name,
  msg->cctx,
  msg->buffer,
- msg->order,
- msg->order_arg,
  &count);
 
total += count;
diff --git lib/dns/rdataset.c lib/dns/rdataset.c
index 610b3c42229..fbc59352fa8 100644
--- lib/dns/rdataset.c
+++ lib/dns/rdataset.c
@@ -220,32 +220,21 @@ dns_rdataset_current(dns_rdataset_t *rdataset, 
dns_rdata_t *rdata) {
 }
 
 #define MAX_SHUFFLE32
-#define WANT_FIXED(r)  (((r)->attributes & DNS_RDATASETATTR_FIXEDORDER) != 0)
-#define WANT_RANDOM(r) (((r)->attributes & DNS_RDATASETATTR_RANDOMIZE) != 0)
 
 struct towire_sort {
int key;
dns_rdata_t *rdata;
 };
 
-static int
-towire_compare(const void *av, const void *bv) {
-   const struct towire_sort *a = (const struct towire_sort *) av;
-   const struct towire_sort *b = (const struct towire_sort *) bv;
-   return (a->key - b->key);
-}
-
 static isc_result_t
 towiresorted(dns_rdataset_t *rdataset, const dns_name_t *owner_name,
-dns_compress_t *cctx, isc_buffer_t *target,
-dns_rdatasetorderfunc_t order, const void *order_arg,
-int partial, unsigned int *countp)
+dns_compress_t *cctx, isc_buffer_t *target, unsigned int *countp)
 {
dns_rdata_t rdata = DNS_RDATA_INIT;
isc_region_t r;
isc_result_t result;
-   unsigned int i, count = 0, added, choice;
-   isc_buffer_t savedbuffer, rdlen, rrbuffer;
+   unsigned int i, count = 0, added;
+   isc_buffer_t savedbuffer, rdlen;
unsigned int headlen;
int question = 0;
int shuffle = 0;
@@ -259,7 +248,6 @@ towiresorted(dns_rdataset_t *rdataset, const dns_name_t 
*owner_name,
 
REQUIRE(rdataset->methods != NULL);
REQUIRE(countp != NULL);
-   REQUIRE((order == NULL) == (order_arg == NULL));
REQUIRE(cctx != NULL);
 
if ((rdataset->attributes & DNS_RDATASETATTR_QUESTION) != 0) {
@@ -279,9 +267,7 @@ towiresorted(dns_rdataset_t *rdataset, const dns_name_t 
*owner_name,
/*
 * Do we want to shuffle this answer?
 */
-   if (!question && count > 1 &&
-   (!WANT_FIXED(rdataset) || order != NULL) &&
-   rdataset->type != dns_rdatatype_rrsig)
+   if (!question && count > 1 && rdataset->type != dns_rdatatype_rrsig)
shuffle = 1;
 
if (shuffle && count > MAX_SHUFFLE) {
@@ -295,6 +281,9 @@ towiresorted(dns_rdataset_t *rdataset, c

improve unwind memory usage

[Florian Obser]

Some time ago (it has been years actually), Otto instrumented malloc(3)
to see where unwind is using a lot of memory when it's just sitting
there.
One of the remaining areas is struct config_file with its member
outgoing_avail_ports:

if(!(cfg->outgoing_avail_ports = (int*)calloc(65536, sizeof(int
goto error_exit;

That's a 256k just wasted on OpenBSD.

Some time ago I added and upstreamed
--disable-explicit-port-randomisation to unbound(8) so that it doesn't
need to try to find a random outgoing port on its own. On OpenBSD we can
just trust the kernel to do this for us. Apparently I left that struct
behind which is part of the userland machinery in unbound.

Especially in unwind this burns a lot of memory because this is
allocated per libunbound context. Without a config file memory usage
goes down from 10M to 6.7M.

This is a diff to get rid of outgoing_avail_ports in unwind and
unbound. I'll try to upstream it. unbound tests would be very much
appreciated.

Comments, OKs?

diff --git sbin/unwind/libunbound/libunbound/libworker.c 
sbin/unwind/libunbound/libunbound/libworker.c
index 11bf5f9db55..941a3d660c8 100644
--- sbin/unwind/libunbound/libunbound/libworker.c
+++ sbin/unwind/libunbound/libunbound/libworker.c
@@ -225,6 +225,7 @@ libworker_setup(struct ub_ctx* ctx, int is_bg, struct 
ub_event_base* eb)
if(!w->is_bg || w->is_bg_thread) {
lock_basic_lock(&ctx->cfglock);
}
+#ifndef DISABLE_EXPLICIT_PORT_RANDOMISATION
numports = cfg_condense_ports(cfg, &ports);
if(numports == 0) {
if(!w->is_bg || w->is_bg_thread) {
@@ -233,6 +234,10 @@ libworker_setup(struct ub_ctx* ctx, int is_bg, struct 
ub_event_base* eb)
libworker_delete(w);
return NULL;
}
+#else
+   numports = 1;
+   ports = NULL;
+#endif
w->back = outside_network_create(w->base, cfg->msg_buffer_size,
(size_t)cfg->outgoing_num_ports, cfg->out_ifs,
cfg->num_out_ifs, cfg->do_ip4, cfg->do_ip6, 
diff --git sbin/unwind/libunbound/util/config_file.c 
sbin/unwind/libunbound/util/config_file.c
index d7bd37a8890..f3713792a25 100644
--- sbin/unwind/libunbound/util/config_file.c
+++ sbin/unwind/libunbound/util/config_file.c
@@ -84,8 +84,10 @@ size_t http2_response_buffer_max = 4 * 1024 * 1024;
 /** global config during parsing */
 struct config_parser_state* cfg_parser = 0;
 
+#ifndef DISABLE_EXPLICIT_PORT_RANDOMISATION
 /** init ports possible for use */
 static void init_outgoing_availports(int* array, int num);
+#endif
 
 struct config_file* 
 config_create(void)
@@ -176,9 +178,11 @@ config_create(void)
cfg->infra_keep_probing = 0;
cfg->delay_close = 0;
cfg->udp_connect = 1;
+#ifndef DISABLE_EXPLICIT_PORT_RANDOMISATION
if(!(cfg->outgoing_avail_ports = (int*)calloc(65536, sizeof(int
goto error_exit;
init_outgoing_availports(cfg->outgoing_avail_ports, 65536);
+#endif
if(!(cfg->username = strdup(UB_USERNAME))) goto error_exit;
 #ifdef HAVE_CHROOT
if(!(cfg->chrootdir = strdup(CHROOT_DIR))) goto error_exit;
@@ -482,12 +486,14 @@ int config_set_option(struct config_file* cfg, const 
char* opt,
} else if(strcmp(opt, "num-threads:") == 0) {
/* not supported, library must have 1 thread in bgworker */
return 0;
+#ifndef DISABLE_EXPLICIT_PORT_RANDOMISATION
} else if(strcmp(opt, "outgoing-port-permit:") == 0) {
return cfg_mark_ports(val, 1, 
cfg->outgoing_avail_ports, 65536);
} else if(strcmp(opt, "outgoing-port-avoid:") == 0) {
return cfg_mark_ports(val, 0, 
cfg->outgoing_avail_ports, 65536);
+#endif
} else if(strcmp(opt, "local-zone:") == 0) {
return cfg_parse_local_zone(cfg, val);
} else if(strcmp(opt, "val-override-date:") == 0) {
@@ -1577,7 +1583,9 @@ config_delete(struct config_file* cfg)
free(cfg->nsid_cfg_str);
free(cfg->nsid);
free(cfg->module_conf);
+#ifndef DISABLE_EXPLICIT_PORT_RANDOMISATION
free(cfg->outgoing_avail_ports);
+#endif
config_delstrlist(cfg->caps_whitelist);
config_delstrlist(cfg->private_address);
config_delstrlist(cfg->private_domain);
@@ -1640,6 +1648,7 @@ config_delete(struct config_file* cfg)
free(cfg);
 }
 
+#ifndef DISABLE_EXPLICIT_PORT_RANDOMISATION
 static void 
 init_outgoing_availports(int* a, int num)
 {
@@ -1668,11 +1677,7 @@ int
 cfg_mark_ports(const char* str, int allow, int* avail, int num)
 {
char* mid = strchr(str, '-');
-#ifdef DISABLE_EXPLICIT_PORT_RANDOMISATION
-   log_warn("Explicit port randomisation disabled, ignoring "
-   "outgoing-port-permit and outgoing-port-avoid configuration "
-   "options");
-#endif
+
if(!mid) {
int port = atoi(str);
if(port == 0 && strcmp(str, 

Re: [PATCH] adds -t timeout to slowcgi

[Florian Obser]

On 2022-06-10 04:27 -07, Alfred Morgan  wrote:
>> The connection to upstream (e.g. httpd) is closed so the client gets a 500 
>> error.
>
> Hmm, this isn't my experience. Possibly a slowcgi bug? My clients were
> getting no response, e.g.:
> curl: (52) Empty reply from server
>
>> But the script keeps running.
>
> Ah, I see now that the process is left to run now. My CGI read/writes
> several MB/s lasting longer than the hard coded 120 sec timeout which
> means my process would immediately get a broken pipe which led me to
> erroneously believe my process was being terminated by slowcgi.
>
>> We were worried that scripts are not prepared when they suddenly get 
>> terminated.
>
> To put worry at rest, in the rfc 3875 CGI Version 1.1 section 3.4 Execution:
> ...
> "In the event of an error condition, the server can interrupt or
>terminate script execution at any time and without warning.  That
>could occur, for example, in the event of a transport failure between
>the server and the client; so the script SHOULD be prepared to handle
>abnormal termination."
>
>> Also as sthen pointed out your MUA mangled the diff.
>
> I believe I mangled the diff by copying from my terminal window.
>
>> the man page needs better wording.
>> please use strtonum(3) and use the idiom from the man page example.
>
> I will attach the patch.
>
> -alfred
>
> Index: slowcgi.8
> ===
> RCS file: /cvs/src/usr.sbin/slowcgi/slowcgi.8,v
> retrieving revision 1.16
> diff -u -p -r1.16 slowcgi.8
> --- slowcgi.8 2 Sep 2021 14:14:44 -   1.16
> +++ slowcgi.8 10 Jun 2022 11:20:04 -
> @@ -76,6 +76,10 @@ effectively disables the chroot.
>  .It Fl s Ar socket
>  Create and bind to alternative local socket at
>  .Ar socket .
> +.It Fl t Ar timeout
> +Closes the file descriptors after
> +.Ar timeout
> +seconds instead of the default 120 seconds. The CGI is left to run.
>  .It Fl U Ar user
>  Change the owner of
>  .Pa /var/www/run/slowcgi.sock
> Index: slowcgi.c
> ===
> RCS file: /cvs/src/usr.sbin/slowcgi/slowcgi.c,v
> retrieving revision 1.62
> diff -u -p -r1.62 slowcgi.c
> --- slowcgi.c 2 Sep 2021 14:14:44 -   1.62
> +++ slowcgi.c 10 Jun 2022 11:20:04 -
> @@ -40,6 +40,7 @@
>  #include 
>  
>  #define TIMEOUT_DEFAULT   120
> +#define TIMEOUT_MAX   86400 * 365
>  #define SLOWCGI_USER  "www"
>  
>  #define FCGI_CONTENT_SIZE 65535
> @@ -252,7 +253,7 @@ usage(void)
>  {
>   extern char *__progname;
>   fprintf(stderr,
> - "usage: %s [-dv] [-p path] [-s socket] [-U user] [-u user]\n",
> + "usage: %s [-dv] [-p path] [-s socket] [-t timeout] [-U user] [-u 
> user]\n",
>   __progname);
>   exit(1);
>  }
> @@ -275,6 +276,7 @@ main(int argc, char *argv[])
>   const char  *chrootpath = NULL;
>   const char  *sock_user = SLOWCGI_USER;
>   const char  *slowcgi_user = SLOWCGI_USER;
> + const char *errstr;
>
  ^ space vs. tab

With that fixed this is OK florian

No need to resend the diff, I trust whoever ends up commiting this can
fix that up before commit.

>   /*
>* Ensure we have fds 0-2 open so that we have no fd overlaps
> @@ -293,7 +295,7 @@ main(int argc, char *argv[])
>   }
>   }
>  
> - while ((c = getopt(argc, argv, "dp:s:U:u:v")) != -1) {
> + while ((c = getopt(argc, argv, "dp:s:t:U:u:v")) != -1) {
>   switch (c) {
>   case 'd':
>   debug++;
> @@ -304,6 +306,11 @@ main(int argc, char *argv[])
>   case 's':
>   fcgi_socket = optarg;
>   break;
> + case 't':
> + timeout.tv_sec = strtonum(optarg, 1, TIMEOUT_MAX, 
> &errstr);
> + if (errstr != NULL)
> + errx(1, "timeout is %s: %s", errstr, optarg);
> + break;
>   case 'U':
>   sock_user = optarg;
>   break;
> @@ -507,7 +514,12 @@ slowcgi_accept(int fd, short events, voi
>  void
>  slowcgi_timeout(int fd, short events, void *arg)
>  {
> - cleanup_request((struct request*) arg);
> + struct request  *req;
> +
> + req = arg;
> +
> + lwarnx("timeout child %i", req->script_pid);
> + cleanup_request(req);
>  }
>  
>  void
>

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

Re: [PATCH] adds -t timeout to slowcgi

[Florian Obser]

On 2022-06-09 01:36 -07, Alfred Morgan  wrote:
> I think this got missed on misc@ when I posted on 5/24. I'm now
> reposting here in tech@ with the [PATCH] subject tag.
>
> Index: usr.sbin/slowcgi/slowcgi.8
> ===
> RCS file: /cvs/src/usr.sbin/slowcgi/slowcgi.8,v
> retrieving revision 1.16
> diff -u -p -r1.16 slowcgi.8
> --- usr.sbin/slowcgi/slowcgi.8 2 Sep 2021 14:14:44 - 1.16
> +++ usr.sbin/slowcgi/slowcgi.8 25 May 2022 04:30:18 -
> @@ -76,6 +76,10 @@ effectively disables the chroot.
>  .It Fl s Ar socket
>  Create and bind to alternative local socket at
>  .Ar socket .
> +.It Fl t Ar timeout
> +Terminate the CGI script after
> +.Ar timeout
> +seconds instead of the default 120 seconds.

Scripts are not terminated, we intentionally let them running. The
connection to upstream (e.g. httpd) is closed so the client gets a 500
error. But the script keeps running. We were worried that scripts are
not prepared when they suddenly get terminated. Maybe that worry is
unfounded. Anyway, the man page needs better wording.

>  .It Fl U Ar user
>  Change the owner of
>  .Pa /var/www/run/slowcgi.sock
> Index: usr.sbin/slowcgi/slowcgi.c
> ===
> RCS file: /cvs/src/usr.sbin/slowcgi/slowcgi.c,v
> retrieving revision 1.62
> diff -u -p -r1.62 slowcgi.c
> --- usr.sbin/slowcgi/slowcgi.c 2 Sep 2021 14:14:44 - 1.62
> +++ usr.sbin/slowcgi/slowcgi.c 25 May 2022 04:30:18 -
> @@ -252,7 +252,7 @@ usage(void)
>  {
>   extern char *__progname;
>   fprintf(stderr,
> -"usage: %s [-dv] [-p path] [-s socket] [-U user] [-u user]\n",
> +"usage: %s [-dv] [-p path] [-s socket] [-t timeout] [-U user] [-u
> user]\n",
>  __progname);
>   exit(1);
>  }
> @@ -275,6 +275,7 @@ main(int argc, char *argv[])
>   const char *chrootpath = NULL;
>   const char *sock_user = SLOWCGI_USER;
>   const char *slowcgi_user = SLOWCGI_USER;
> + char *p;
>
>   /*
>   * Ensure we have fds 0-2 open so that we have no fd overlaps
> @@ -293,7 +294,7 @@ main(int argc, char *argv[])
>   }
>   }
>
> - while ((c = getopt(argc, argv, "dp:s:U:u:v")) != -1) {
> + while ((c = getopt(argc, argv, "dp:s:t:U:u:v")) != -1) {
>   switch (c) {
>   case 'd':
>   debug++;
> @@ -304,6 +305,11 @@ main(int argc, char *argv[])
>   case 's':
>   fcgi_socket = optarg;
>   break;
> +case 't':
> + timeout.tv_sec = strtoll(optarg, &p, 10);
> + if (*p)
> + errx(1, "illegal timeout -- %s", optarg);
> +  break;

please use strtonum(3) and use the idiom from the man page example.
Also as sthen pointed out your MUA mangled the diff.

>   case 'U':
>   sock_user = optarg;
>   break;
>

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

Re: httpd: add include_dir keyword

[Florian Obser]

On 2022-06-02 11:04 +02, qorg11  wrote:
> This patch addes the "inlcude_dir" keyword for httpd.conf. Which works
> just like "include" but it includes all the files in a directory, for
> example: include "/etc/httpd.d"
>
> The diff file is attatched.

I don't think we want this functionality.

More inline.

>
> Index: httpd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
> retrieving revision 1.121
> diff -u -p -u -p -r1.121 httpd.conf.5
> --- httpd.conf.5  9 Mar 2022 13:50:41 -   1.121
> +++ httpd.conf.5  2 Jun 2022 09:02:22 -
> @@ -84,6 +84,12 @@ keyword, for example:
>  .Bd -literal -offset indent
>  include "/etc/httpd.conf.local"
>  .Ed
> +A directory with configuration files can be included with the
> +.Ic include_dir
> +keyword, for example:
> +.Bd -literal -offset indent
> +include_dir "/etc/httpd.conf.d"

this should be
"include directory"
or
"include /etc/httpd.d/"
should be made to work.

> +.Ed
>  .Sh MACROS
>  Macros can be defined that will later be expanded in context.
>  Macro names must start with a letter, digit, or underscore,
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> retrieving revision 1.128
> diff -u -p -u -p -r1.128 parse.y
> --- parse.y   27 Feb 2022 20:30:30 -  1.128
> +++ parse.y   2 Jun 2022 09:02:22 -
> @@ -52,6 +52,7 @@
>  #include 
>  #include 
>  #include 
> +#include 

trailing whitespace

>  
>  #include "httpd.h"
>  #include "http.h"
> @@ -139,7 +140,7 @@ typedef struct {
>  %token   LISTEN LOCATION LOG LOGDIR MATCH MAXIMUM NO NODELAY OCSP ON 
> PORT PREFORK
>  %token   PROTOCOLS REQUESTS ROOT SACK SERVER SOCKET STRIP STYLE SYSLOG 
> TCP TICKET
>  %token   TIMEOUT TLS TYPE TYPES HSTS MAXAGE SUBDOMAINS DEFAULT PRELOAD 
> REQUEST
> -%token   ERROR INCLUDE AUTHENTICATE WITH BLOCK DROP RETURN PASS REWRITE
> +%token   ERROR INCLUDE INCLUDE_DIR AUTHENTICATE WITH BLOCK DROP RETURN 
> PASS REWRITE
>  %token   CA CLIENT CRL OPTIONAL PARAM FORWARDED FOUND NOT
>  %token   ERRDOCS GZIPSTATIC
>  %token STRING
> @@ -155,6 +156,7 @@ typedef struct {
>  
>  grammar  : /* empty */
>   | grammar include '\n'
> + | grammar include_dir '\n'
>   | grammar '\n'
>   | grammar varset '\n'
>   | grammar main '\n'
> @@ -165,7 +167,6 @@ grammar   : /* empty */
>  
>  include  : INCLUDE STRING{
>   struct file *nfile;
> -
>   if ((nfile = pushfile($2, 0)) == NULL) {
>   yyerror("failed to include file %s", $2);
>   free($2);
> @@ -178,6 +179,46 @@ include  : INCLUDE STRING{
>   }
>   ;
>

the following block has some weird tab space tab indent

> +include_dir : INCLUDE_DIR STRING {
> + char absolute_path[PATH_MAX];
> + char dir[PATH_MAX];
> + struct file *nfile;
> + DIR *opened_dir = opendir($2);

do not initialize a variable with a function call when declaring it

> + struct dirent *entry;
> +
> + if(opened_dir == NULL) {
> + free($2);
> + yyerror("Failed to open directory %s", $2);
> + YYERROR;
> + }
> +
> + size_t len = strlcpy(dir, $2, PATH_MAX);

no declaration in a middle of a block

> +
> + if(len >= sizeof(dir)) {
> + free($2);
> + yyerror("too long");
> + YYERROR;
> + }
> +
> + while((entry = readdir(opened_dir))) {
> + if(entry->d_name[0] == '.')
> + continue;

wrong indent

> + len = 
> snprintf(absolute_path,PATH_MAX,"%s%s",dir, entry->d_name);

missing spaces after ","

> + if(len < 0|| len >= sizeof(absolute_path)) {

missing space ^

> + yyerror("too long");
> + YYERROR;
> + }
> + if((nfile = pushfile(absolute_path, 0)) == 
> NULL) {
> + yyerror("failed to include file %s", 
> $2);
> + YYERROR;
> + }
> + }
> +
> + file = nfile;
> + lungetc('\n');
> +}
> +;
> +
>  varset   : STRING '=' STRING {
>   char *s = $1;
>   

Re: acme-client: check token names

[Florian Obser]

On 2022-05-04 13:21 +0430, Ali Farzanrad  wrote:
> OK, I've tested following diff on my own domain and it works.
> I did 2 modifications:
>
>  1. I explicitly call setlocate with "C" to ensure C locale,

I came to the conclusion that it's best to call setlocale in first thing
in main, that's what other code does, too and seems less surprising.

>
>  2. I did a string length check.  According to RFC it must have 128 bit
> of random entropy, so it should have at least 22 base64 characters,
> but I was unsure.  So I only check for empty strings.

Checking for an empty string gives us a better error message, we would
error out with EISDIR in open(2) later, so that's good I guess.
Trying to enforce entropy seems a bit silly though, it's there to
protect the CA, if they mess this up it's their problem.

The following diff just moves setlocale to main and is OK florian

Or I can commit it myself is someone else OKs it.

diff --git chngproc.c chngproc.c
index 0cbfaf27c31..f9cff65311d 100644
--- chngproc.c
+++ chngproc.c
@@ -16,6 +16,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -77,6 +78,18 @@ chngproc(int netsock, const char *root)
goto out;
else if ((tok = readstr(netsock, COMM_TOK)) == NULL)
goto out;
+   else if (strlen(tok) < 1) {
+   warnx("token is too short");
+   goto out;
+   }
+
+   for (i = 0; tok[i]; ++i) {
+   int ch = (unsigned char)tok[i];
+   if (!isalnum(ch) && ch != '-' && ch != '_') {
+   warnx("token is not a valid base64url");
+   goto out;
+   }
+   }
 
if (asprintf(&fmt, "%s.%s", tok, th) == -1) {
warn("asprintf");
diff --git main.c main.c
index 65ea2cf3ac3..a3006ca1483 100644
--- main.c
+++ main.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -56,6 +57,9 @@ main(int argc, char *argv[])
struct domain_c *domain = NULL;
struct altname_c*ac;
 
+   if (setlocale(LC_CTYPE, "C") == NULL)
+   errx(1, "setlocale");
+
while ((c = getopt(argc, argv, "Fnrvf:")) != -1)
switch (c) {
case 'F':


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

Re: acme-client: check token names

[Florian Obser]

On 2022-05-03 17:41 +0430, Ali Farzanrad  wrote:
>
> Hi Florian,
>
> Yes, I read the RFC, it should work, but I couldn't test it yet, because
> my domain manager is a little lazy (I've registeret 2 subdomains for my
> domain, but it is not listed in name servers yet).  I'll probably test
> it tomorrow.
>
> I read the isalnum man page, it said that isalnum might have different
> behavior based on locale, so I decide to not use that.  Is it OK?

No, please do a
setlocale(LC_CTYPE, "");
at the beginning of chngproc, before the unveil and then use isalnum
instead of handrolling it. It will fit into chngproc, so no need for
another function

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

Re: acme-client: check token names

[Florian Obser]

On 2022-05-02 03:04 +0430, Ali Farzanrad  wrote:
> Hi tech@,
>
> I know that acme-client is unveiled properly, but isn't it better to
> check token names?

Nice catch, the token is untrusted input.
We should validate this differently though.

RFC 8555, 8.5 HTTP Challenge:

   token (required, string):  A random value that uniquely identifies
  the challenge.  This value MUST have at least 128 bits of entropy.
  It MUST NOT contain any characters outside the base64url alphabet
  and MUST NOT include base64 padding characters ("=").

base64url is defined in
RFC 4648, 5. Base 64 Encoding with URL and Filename Safe Alphabet

It's basically isalpha || '-' || '_'.

Are you up to implementing that check instead?

>
> ===
> RCS file: /cvs/src/usr.sbin/acme-client/chngproc.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 chngproc.c
> --- chngproc.c12 Jul 2021 15:09:20 -  1.16
> +++ chngproc.c1 May 2022 22:28:43 -
> @@ -77,6 +77,11 @@ chngproc(int netsock, const char *root)
>   goto out;
>   else if ((tok = readstr(netsock, COMM_TOK)) == NULL)
>   goto out;
> + else if (strstr(tok, "../") == tok ||
> + strstr(tok, "/../") != NULL) {
> + warnx("bad file path");
> + goto out;
> + }
>  
>   if (asprintf(&fmt, "%s.%s", tok, th) == -1) {
>   warn("asprintf");
>

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

Re: ssh-keygen(1): resident fido2 keys

[Florian Obser]

On 2022-05-01 14:43 +02, Christian Weisgerber  wrote:
> Florian Obser:
>
>> Sounds reasonable, this adds the FIDO section and moves the -O bits in.
>> The wording is inspired by / copied from the 8.2 release notes and the
>> CERTIFICATES section.
>
> I think that makes sense.
>
> s/token/authenticator/g
>
> We standardized on "FIDO authenticator" some time ago, because
> "token" is already used for % expansion, and "security key" was
> confusing.  "Authenticator" is also the official term used in the
> FIDO standard.
>
> ... I see some instances of "FIDO token" already slipped in over
> the last months, but that needs to be fixed.

fixed

I'm also using

.Sh FIDO AUTHENTICATOR

>
>> Maybe you have text for DESCRIPTION?
>> Not sure what to do about -K, maybe
>>  see
>>  .Sx FIDO
>>  for details.
>
> "... for more information."?

I went with

See the
.Sx FIDO AUTHENTICATOR
section for more information.

because I noticed that's used already in the document for other
sections.

diff --git ssh-keygen.1 ssh-keygen.1
index 59b7f23a1fa..4d83e8f085e 100644
--- ssh-keygen.1
+++ ssh-keygen.1
@@ -396,6 +396,9 @@ Public and private key files will be written to the current 
directory for
 each downloaded key.
 If multiple FIDO authenticators are attached, keys will be downloaded from
 the first touched authenticator.
+See the
+.Sx FIDO AUTHENTICATOR
+section for more information.
 .It Fl k
 Generate a KRL file.
 In this mode,
@@ -487,56 +490,9 @@ listed in the
 .Sx MODULI GENERATION
 section may be specified.
 .Pp
-When generating a key that will be hosted on a FIDO authenticator,
-this flag may be used to specify key-specific options.
-Those supported at present are:
-.Bl -tag -width Ds
-.It Cm application
-Override the default FIDO application/origin string of
-.Dq ssh: .
-This may be useful when generating host or domain-specific resident keys.
-The specified application string must begin with
-.Dq ssh: .
-.It Cm challenge Ns = Ns Ar path
-Specifies a path to a challenge string that will be passed to the
-FIDO token during key generation.
-The challenge string may be used as part of an out-of-band
-protocol for key enrollment
-(a random challenge is used by default).
-.It Cm device
-Explicitly specify a
-.Xr fido 4
-device to use, rather than letting the token middleware select one.
-.It Cm no-touch-required
-Indicate that the generated private key should not require touch
-events (user presence) when making signatures.
-Note that
-.Xr sshd 8
-will refuse such signatures by default, unless overridden via
-an authorized_keys option.
-.It Cm resident
-Indicate that the key should be stored on the FIDO authenticator itself.
-Resident keys may be supported on FIDO2 tokens and typically require that
-a PIN be set on the token prior to generation.
-Resident keys may be loaded off the token using
-.Xr ssh-add 1 .
-.It Cm user
-A username to be associated with a resident key,
-overriding the empty default username.
-Specifying a username may be useful when generating multiple resident keys
-for the same application name.
-.It Cm verify-required
-Indicate that this private key should require user verification for
-each signature.
-Not all FIDO tokens support this option.
-Currently PIN authentication is the only supported verification method,
-but other methods may be supported in the future.
-.It Cm write-attestation Ns = Ns Ar path
-May be used at key generation time to record the attestation data
-returned from FIDO tokens during key generation.
-This information is potentially sensitive.
-By default, this information is discarded.
-.El
+When generating FIDO authenticator-backed keys, the options listed in the
+.Sx FIDO AUTHENTICATOR
+section may be specified.
 .Pp
 When performing signature-related options using the
 .Fl Y
@@ -1060,6 +1016,76 @@ public key must be trusted by
 or
 .Xr ssh 1 .
 Refer to those manual pages for details.
+.Sh FIDO AUTHENTICATOR
+.Nm
+is able to to generate FIDO authenticator-backed keys, after which
+they may be used much like any other key type supported by OpenSSH, so
+long as the hardware authenticator is attached when the keys are used.
+FIDO authenticators generally require the user to explicitly authorise
+operations by touching or tapping them.
+FIDO keys consist of two parts: a key handle part stored in the
+private key file on disk, and a per-device private key that is unique
+to each FIDO authenticator and that cannot be exported from the
+authenticator hardware.
+These are combined by the hardware at authentication time to derive
+the real key that is used to sign authentication challenges.
+Supported key types are
+.Cm ecdsa-sk
+and
+.Cm ed25519-sk .
+.Pp
+The options that are valid for FIDO keys are:
+.Bl -tag -width Ds
+.It Cm application
+Override the default FIDO application/origin string of
+.Dq 

Re: ssh-keygen(1): resident fido2 keys

[Florian Obser]

On 2022-04-29 19:24 +01, Jason McIntyre  wrote:
> what we probably want is a simple overview of these devices in
> DESCRIPTION. but that's not simple. the page is already a bruiser. i
> mean, it discusses what constitues a good password/phrase! where to go
> from there?
>
> i  note that the other components using -O have their own sections
> (CERTIFICATES/MODULE GENERATION). so what about this:
>
> - add a very small note to DESCRIPTION saying it can handle fido keys.
> that seems warranted anyway because they seem now fairly common and are
> somewhat different to traditional (software) keys.
>
> - move the -O stuff pertaining to fido keys to its own section.
>
> - tweak the new fido section to give a simple overview of these devices.
>
> i'm not able to offer a diff at this point, but maybe we could piece
> something together if you agree?

Sounds reasonable, this adds the FIDO section and moves the -O bits in.
The wording is inspired by / copied from the 8.2 release notes and the
CERTIFICATES section.

Maybe you have text for DESCRIPTION?
Not sure what to do about -K, maybe
see
.Sx FIDO
for details.

diff --git ssh-keygen.1 ssh-keygen.1
index 59b7f23a1fa..6dc71aa32bd 100644
--- ssh-keygen.1
+++ ssh-keygen.1
@@ -487,56 +487,9 @@ listed in the
 .Sx MODULI GENERATION
 section may be specified.
 .Pp
-When generating a key that will be hosted on a FIDO authenticator,
-this flag may be used to specify key-specific options.
-Those supported at present are:
-.Bl -tag -width Ds
-.It Cm application
-Override the default FIDO application/origin string of
-.Dq ssh: .
-This may be useful when generating host or domain-specific resident keys.
-The specified application string must begin with
-.Dq ssh: .
-.It Cm challenge Ns = Ns Ar path
-Specifies a path to a challenge string that will be passed to the
-FIDO token during key generation.
-The challenge string may be used as part of an out-of-band
-protocol for key enrollment
-(a random challenge is used by default).
-.It Cm device
-Explicitly specify a
-.Xr fido 4
-device to use, rather than letting the token middleware select one.
-.It Cm no-touch-required
-Indicate that the generated private key should not require touch
-events (user presence) when making signatures.
-Note that
-.Xr sshd 8
-will refuse such signatures by default, unless overridden via
-an authorized_keys option.
-.It Cm resident
-Indicate that the key should be stored on the FIDO authenticator itself.
-Resident keys may be supported on FIDO2 tokens and typically require that
-a PIN be set on the token prior to generation.
-Resident keys may be loaded off the token using
-.Xr ssh-add 1 .
-.It Cm user
-A username to be associated with a resident key,
-overriding the empty default username.
-Specifying a username may be useful when generating multiple resident keys
-for the same application name.
-.It Cm verify-required
-Indicate that this private key should require user verification for
-each signature.
-Not all FIDO tokens support this option.
-Currently PIN authentication is the only supported verification method,
-but other methods may be supported in the future.
-.It Cm write-attestation Ns = Ns Ar path
-May be used at key generation time to record the attestation data
-returned from FIDO tokens during key generation.
-This information is potentially sensitive.
-By default, this information is discarded.
-.El
+When generating FIDO token-backed keys, the options listed in the
+.Sx FIDO
+section may be specified.
 .Pp
 When performing signature-related options using the
 .Fl Y
@@ -1060,6 +1013,75 @@ public key must be trusted by
 or
 .Xr ssh 1 .
 Refer to those manual pages for details.
+.Sh FIDO
+.Nm
+is able to to generate FIDO token-backed keys, after which they may
+be used much like any other key type supported by OpenSSH, so long as
+the hardware token is attached when the keys are used.
+FIDO tokens generally require the user to explicitly authorise
+operations by touching or tapping them.
+FIDO keys consist of two parts: a key handle part stored in the
+private key file on disk, and a per-device private key that is unique
+to each FIDO token and that cannot be exported from the token
+hardware.
+These are combined by the hardware at authentication time to derive
+the real key that is used to sign authentication challenges.
+Supported key types are
+.Cm ecdsa-sk
+and
+.Cm ed25519-sk .
+.Pp
+The options that are valid for FIDO keys are:
+.Bl -tag -width Ds
+.It Cm application
+Override the default FIDO application/origin string of
+.Dq ssh: .
+This may be useful when generating host or domain-specific resident keys.
+The specified application string must begin with
+.Dq ssh: .
+.It Cm challenge Ns = Ns Ar path
+Specifies a path to a challenge string that will be passed to the
+FIDO token during key generation.
+The challenge string may be used as part of an out-of-band
+protocol for key enrollment
+(a random challenge is used by default).
+.It Cm device
+Explicitly speci

dhcpleased(8): be more lenient with host name and domain name options

[Florian Obser]

As found by n18fuhtm AT tutanota.com there are dhcp servers that send a
domain name option with length 1 and a single \0.
We strip trailing \0 and then end up with length 0.
This is a protocol violation, the minimum length for domain name option
is 1, and we ignore the lease.

So we are not going to fix this server side, we might as well just
pretend that we didn't receive a domain name (or host name). We only
ever care about them in the installer anyway. Not getting a lease
because of this corner case is not nice.

OK?

diff --git engine.c engine.c
index ae3f467ac3a..64770567198 100644
--- engine.c
+++ engine.c
@@ -882,6 +882,8 @@ parse_dhcp(struct dhcpleased_iface *iface, struct imsg_dhcp 
*dhcp)
memset(&subnet_mask, 0, sizeof(subnet_mask));
memset(&routes, 0, sizeof(routes));
memset(&nameservers, 0, sizeof(nameservers));
+   memset(hostname, 0, sizeof(hostname));
+   memset(domainname, 0, sizeof(domainname));
 
while (rem > 0 && dho != DHO_END) {
dho = *p;
@@ -1014,14 +1016,18 @@ parse_dhcp(struct dhcpleased_iface *iface, struct 
imsg_dhcp *dhcp)
rem -= dho_len;
break;
case DHO_HOST_NAME:
-   if (dho_len < 1)
-   goto wrong_length;
+   if (dho_len < 1) {
+   /*
+* Protocol violation: minimum length is 1;
+* pretend the option is not there
+*/
+   break;
+   }
/* MUST delete trailing NUL, per RFC 2132 */
slen = dho_len;
while (slen > 0 && p[slen - 1] == '\0')
slen--;
-   if (slen < 1)
-   goto wrong_length;
+   /* slen might be 0 here, pretend option is not there. */
strvisx(hostname, p, slen, VIS_SAFE);
if (log_getverbose() > 1)
log_debug("DHO_HOST_NAME: %s", hostname);
@@ -1029,14 +1035,18 @@ parse_dhcp(struct dhcpleased_iface *iface, struct 
imsg_dhcp *dhcp)
rem -= dho_len;
break;
case DHO_DOMAIN_NAME:
-   if (dho_len < 1)
-   goto wrong_length;
+   if (dho_len < 1) {
+   /*
+* Protocol violation: minimum length is 1;
+* pretend the option is not there
+*/
+   break;
+   }
/* MUST delete trailing NUL, per RFC 2132 */
slen = dho_len;
while (slen > 0 && p[slen - 1] == '\0')
slen--;
-   if (slen < 1)
-   goto wrong_length;
+   /* slen might be 0 here, pretend option is not there. */
strvisx(domainname, p, slen, VIS_SAFE);
if (log_getverbose() > 1)
log_debug("DHO_DOMAIN_NAME: %s", domainname);

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

ssh-keygen(1): resident fido2 keys

[Florian Obser]

So I got a yubikey and I wanted to try fido2 with ssh. I was a bit
unsure on how to generate a key (hint: it's just ssh-keygen -t
ed25519-sk). So I went and ask the Internet. I ran into some page that
suggested that you need to give a ton of options to ssh-keygen,
including -o resident without explaining why. Our man page confused me
even more:

 resident
 Indicate that the key should be stored on the FIDO
 authenticator itself.

Well, that sounds reasonable, I mean, I got this thing to store my ssh
key. But hang on, why would I want a non-resident key?

The 8.2 release notes provided the answer under the heading "FIDO2
resident keys" (https://www.openssh.com/txt/release-8.2)

I have used parts of the explanation and adjusted it a bit to make it
better fit the man page.

It's a bit weird to explain how fido keys work in the middle of options
discussion, but I couldn't find a better spot.

Thoughts?

diff --git ssh-keygen.1 ssh-keygen.1
index 59b7f23a1fa..b333f740936 100644
--- ssh-keygen.1
+++ ssh-keygen.1
@@ -516,6 +516,11 @@ will refuse such signatures by default, unless overridden 
via
 an authorized_keys option.
 .It Cm resident
 Indicate that the key should be stored on the FIDO authenticator itself.
+FIDO keys consist of two parts, a key handle part stored in the private key
+file on disk, and a per-token private key that cannot be exported from the
+token hardware.
+This stores the key handle on the token itself but increases the likelihood of
+an attacker being able to use a stolen token device.
 Resident keys may be supported on FIDO2 tokens and typically require that
 a PIN be set on the token prior to generation.
 Resident keys may be loaded off the token using


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

Re: dhcpleased: Don’t set option 12 if host name is empty

[Florian Obser]

On 2022-04-24 20:01 +02, Ibrahim Khalifa  wrote:
> Hi,
>
> I ran into an issue with dhcpleased when trying to do pxeboot and automatic 
> installation when using DHCP Relay on Cisco ASA.
>
> The problem is when dhcpleased starts for the first time after bsd.rd
> is loaded there is no hostname set for the server yet. Dhcpleased will
> set option 12 in it’s discover request but set the length to 0. As per
> the RFC the minimum length for the host name is 1. The assumption here
> is that sending option 12 with a zero length would be treated the same
> as it not being set. However Cisco ASA will block such package with
> error ”option 12 is malformed.”.
>
> It can be argued on which view on this is the most correct, but as it
> is now at least Cisco ASA blocks it as malformed. The attached patch
> will not set option 12 if the host name is empty. It works for me,
> both in the above scenario but also when host name is set. Someone
> with better insight in the depth of dhcpleased might know if this
> could be done in a more elegant way.
>
> Best regards,
>
> //Ibo
>
>

very good catch.

There is no need for strlen(3), we just want to know if hostname is the
empty string, i.e. starts with '\0'.

Btw. fun thing, I tested this by moving /etc/myname out of the way and
the dhcp server on my CPE just echos the empty hostname back. dhcpleased
refuses the lease because the hostname option is invalid :D

OK?

diff --git frontend.c frontend.c
index 58c6153793f..cb22bcad32e 100644
--- frontend.c
+++ frontend.c
@@ -971,7 +971,8 @@ build_packet(uint8_t message_type, char *if_name, uint32_t 
xid,
 #endif /* SMALL */
{
if (gethostname(dhcp_hostname + 2,
-   sizeof(dhcp_hostname) - 2) == 0) {
+   sizeof(dhcp_hostname) - 2) == 0 &&
+   dhcp_hostname[2] != '\0') {
if ((c = strchr(dhcp_hostname + 2, '.')) != NULL)
*c = '\0';
dhcp_hostname[1] = strlen(dhcp_hostname + 2);


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

Re: pf igmp icmp6 multicast router alert

[Florian Obser]

On 2022-04-21 21:10 +02, Alexander Bluhm  wrote:
> On Thu, Apr 21, 2022 at 08:56:07PM +0200, Otto Moerbeek wrote:
>> > Currently it allows all options.  Should I make it specific to
>> > router alert with IGMP or ICMP6?
>> 
>> To me it looks like the icmp6 case already is limited to MLD?
>
> The question is the other way around.  My current diff allows any
> option with ICMP6 MLD.  Do we want to restict the option to router
> alert?
>
> In our ip6.h we have:
> #define IP6OPT_JUMBO0xC2/* 11 0 00010 = 194 */
> #define IP6OPT_NSAP_ADDR0xC3/* 11 0 00011 */
> #define IP6OPT_TUNNEL_LIMIT 0x04/* 00 0 00100 */
> #define IP6OPT_ROUTER_ALERT 0x05/* 00 0 00101 (RFC3542, recommended) 
> */
>
> And who knows what other options have been designed.
>
> In ip.h I see these:
> #define IPOPT_RR7   /* record packet route */
> #define IPOPT_TS68  /* timestamp */
> #define IPOPT_SECURITY  130 /* provide s,c,h,tcc */
> #define IPOPT_LSRR  131 /* loose source route */
> #define IPOPT_SATID 136 /* satnet id */
> #define IPOPT_SSRR  137 /* strict source route */
> #define IPOPT_RA148 /* router alert */
>
> The option I have ever seen in the wild is router alert.  So it may
> be better to allow IGMP and ICMP6 multicast if router alert is the
> only option in the packet.

That's my gut feeling as well. But I don't have any hard evidence that
that is actually correct. However, since we learned that router-alert is
an actual problem it is also a good incremental step.

>
> bluhm
>

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

rad(8): rate limit solicited router advertisements

[Florian Obser]

Rate limit router advertisements according to RFC 4861 6.2.6.

   In all cases, Router Advertisements sent in response to a Router
   Solicitation MUST be delayed by a random time between 0 and
   MAX_RA_DELAY_TIME seconds. (If a single advertisement is sent in
   response to multiple solicitations, the delay is relative to the
   first solicitation.)  In addition, consecutive Router Advertisements
   sent to the all-nodes multicast address MUST be rate limited to no
   more than one advertisement every MIN_DELAY_BETWEEN_RAS seconds.

We are kinda not delaying unicast router advertisements "by a random
time between 0 and MAX_RA_DELAY_TIME seconds." But I'll just argue that
our network stack will introduce a delay between 0 and 500ms. Of course
the delay his highly skewed towards 0, but the rfc doesn't call for a
uniform distribution ;)

OK?

diff --git engine.c engine.c
index 7c999bc5447..04bf6ca8a30 100644
--- engine.c
+++ engine.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -33,10 +34,11 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 
 #include "log.h"
@@ -46,7 +48,9 @@
 struct engine_iface {
TAILQ_ENTRY(engine_iface)   entry;
struct eventtimer;
+   struct timespec last_ra;
uint32_tif_index;
+   int ras_delayed;
 };
 
 TAILQ_HEAD(, engine_iface) engine_interfaces;
@@ -464,20 +468,21 @@ void
 parse_rs(struct imsg_ra_rs *rs)
 {
struct nd_router_solicit*nd_rs;
-   struct imsg_send_ra  send_ra;
+   struct engine_iface *engine_iface;
ssize_t  len;
+   int  unicast_ra = 0;
const char  *hbuf;
char ifnamebuf[IFNAMSIZ];
uint8_t *p;
 
hbuf = sin6_to_str(&rs->from);
 
-   send_ra.if_index = rs->if_index;
-   memcpy(&send_ra.to, &all_nodes, sizeof(send_ra.to));
-
log_debug("got RS from %s on %s", hbuf, if_indextoname(rs->if_index,
ifnamebuf));
 
+   if ((engine_iface = find_engine_iface_by_id(rs->if_index)) == NULL)
+   return;
+
len = rs->len;
 
if (!(IN6_IS_ADDR_LINKLOCAL(&rs->from.sin6_addr) ||
@@ -517,7 +522,7 @@ parse_rs(struct imsg_ra_rs *rs)
switch (nd_opt_hdr->nd_opt_type) {
case ND_OPT_SOURCE_LINKADDR:
log_debug("got RS with source linkaddr option");
-   memcpy(&send_ra.to, &rs->from, sizeof(send_ra.to));
+   unicast_ra = 1;
break;
default:
log_debug("\t\tUNKNOWN: %d", nd_opt_hdr->nd_opt_type);
@@ -526,8 +531,33 @@ parse_rs(struct imsg_ra_rs *rs)
len -= nd_opt_hdr->nd_opt_len * 8 - 2;
p += nd_opt_hdr->nd_opt_len * 8 - 2;
}
-   engine_imsg_compose_frontend(IMSG_SEND_RA, 0, &send_ra,
-   sizeof(send_ra));
+
+   if (unicast_ra) {
+   struct imsg_send_ra  send_ra;
+   send_ra.if_index = rs->if_index;
+   memcpy(&send_ra.to, &rs->from, sizeof(send_ra.to));
+   engine_imsg_compose_frontend(IMSG_SEND_RA, 0, &send_ra,
+   sizeof(send_ra));
+   } else {
+   struct timespec  now, diff, ra_delay = {MIN_DELAY_BETWEEN_RAS, 
0};
+   struct timeval   tv = {0, 0};
+
+   /* a multicast RA is already scheduled within the next 3 
seconds */
+   if (engine_iface->ras_delayed)
+   return;
+
+   engine_iface->ras_delayed = 1;
+   clock_gettime(CLOCK_MONOTONIC, &now);
+   timespecsub(&now, &engine_iface->last_ra, &diff);
+
+   if (timespeccmp(&diff, &ra_delay, <)) {
+   timespecsub(&ra_delay, &diff, &ra_delay);
+   TIMESPEC_TO_TIMEVAL(&tv, &ra_delay);
+   }
+
+   tv.tv_usec = arc4random_uniform(MAX_RA_DELAY_TIME * 1000);
+   evtimer_add(&engine_iface->timer, &tv);
+   }
 }
 
 struct engine_iface*
@@ -607,4 +637,6 @@ iface_timeout(int fd, short events, void *arg)
memcpy(&send_ra.to, &all_nodes, sizeof(send_ra.to));
engine_imsg_compose_frontend(IMSG_SEND_RA, 0, &send_ra,
sizeof(send_ra));
+   clock_gettime(CLOCK_MONOTONIC, &engine_iface->last_ra);
+   engine_iface->ras_delayed = 0;
 }
diff --git rad.h rad.h
index d6fdaf5b325..2dde4bef063 100644
--- rad.h
+++ rad.h
@@ -30,7 +30,9 @@
 #defineMIN_RTR_ADV_INTERVAL200
 #defineADV_DEFAULT_LIFETIME3 * MAX_RTR_ADV_INTERVAL
 #defineADV_PREFERRED_LIFETIME  604800  /* 7 days */
-#define ADV_VALID_LIFETIME 2592000 /* 30 days */
+#define  

Re: initial 11ac support for iwm(4)

[Florian Obser]

Still works fine on 9260.

While playing around with this I noticed something else which is
probably not a regression:

I have two SSIDs, "normal" and NAT64, they are on the same AP and just
come out on different vlans, they use the same channel. They are also on
2.4GHz.

Switching between them with
ifconfig iwm0 nwid MYSSID wpakey...
ifconfig iwm0 nwid MYSSID64 wpakey...
ifconfig iwm0 nwid MYSSID wpakey...
and so on sometimes makes iwm0 chose channel 1, i.e. 2.4GHz which is a
20MHz wide channel. And it really likes it there, it's not leaving even
when I switch the SSID around. ifconfig iwm0 mode 11ac makes it switch
over and then it stays there again, even if I do ifconfig iwm0 -mode.

It's a bit hard to trigger, it happens maybe 1 in 50 tries or so? Or
maybe it's a timing thing?

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

Re: initial 11ac support for iwm(4)

[Florian Obser]

This works fine on

iwm0 at pci1 dev 0 function 0 "Intel Dual Band Wireless-AC 9260" rev 0x29, msix
iwm0: hw rev 0x320, fw ver 46.4e1ceb39.0

and

iwm0 at pci2 dev 0 function 0 "Intel AC 7260" rev 0x83, msi
iwm0: hw rev 0x140, fw ver 17.3216344376.0

against UniFi UAP-AC-SHD and UAP-AC-Pro. I have also tested roaming
between the APs. There is no intereference on the channels by other APs
but other devices get air time.

server means tcpbench -s on the laptop, target is a VM that can handle
gbit/s speeds.



9260


$ tail -1 tcpbench_11{n,ac}.log tcpbench_server_11{n,ac}.log
==> tcpbench_11n.log <==
bandwidth min/avg/max/std-dev = 50.917/141.416/175.932/24.805 Mbps

==> tcpbench_11ac.log <==
bandwidth min/avg/max/std-dev = 80.264/252.530/301.530/51.497 Mbps

==> tcpbench_server_11n.log <==
bandwidth min/avg/max/std-dev = 25.797/79.505/146.893/26.857 Mbps

==> tcpbench_server_11ac.log <==
bandwidth min/avg/max/std-dev = 70.532/235.260/316.520/61.451 Mbps



7260


$ tail -1 tcpbench_11{n,ac}.log tcpbench_server_11{n,ac}.log
==> tcpbench_11n.log <==
bandwidth min/avg/max/std-dev = 42.412/179.054/201.801/32.961 Mbps

==> tcpbench_11ac.log <==
bandwidth min/avg/max/std-dev = 90.708/237.702/311.002/50.290 Mbps

==> tcpbench_server_11n.log <==
bandwidth min/avg/max/std-dev = 61.289/153.094/185.233/29.192 Mbps

==> tcpbench_server_11ac.log <==
bandwidth min/avg/max/std-dev = 29.161/170.507/234.489/46.218 Mbps

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

nsd 4.4.0

[Florian Obser]

Tests, OKs?

diff --git usr.sbin/nsd/Makefile.in usr.sbin/nsd/Makefile.in
index 8aa40269f2a..e28fc47cd32 100644
--- usr.sbin/nsd/Makefile.in
+++ usr.sbin/nsd/Makefile.in
@@ -586,7 +586,7 @@ cutest_udb.o: $(srcdir)/tpkg/cutest/cutest_udb.c config.h 
$(srcdir)/tpkg/cutest/
 cutest_udbrad.o: $(srcdir)/tpkg/cutest/cutest_udbrad.c config.h \
  $(srcdir)/tpkg/cutest/cutest.h $(srcdir)/udbradtree.h $(srcdir)/udb.h
 cutest_util.o: $(srcdir)/tpkg/cutest/cutest_util.c config.h 
$(srcdir)/tpkg/cutest/cutest.h \
- $(srcdir)/region-allocator.h $(srcdir)/util.h
+ $(srcdir)/region-allocator.h $(srcdir)/util.h $(srcdir)/xfrd-tcp.h
 qtest.o: $(srcdir)/tpkg/cutest/qtest.c config.h $(srcdir)/tpkg/cutest/qtest.h 
$(srcdir)/buffer.h \
  $(srcdir)/region-allocator.h $(srcdir)/util.h $(srcdir)/query.h 
$(srcdir)/namedb.h $(srcdir)/dname.h $(srcdir)/buffer.h $(srcdir)/dns.h \
  $(srcdir)/radtree.h $(srcdir)/rbtree.h $(srcdir)/nsd.h $(srcdir)/edns.h 
$(srcdir)/packet.h $(srcdir)/tsig.h $(srcdir)/namedb.h $(srcdir)/util.h 
$(srcdir)/nsec3.h \
diff --git usr.sbin/nsd/configlexer.lex usr.sbin/nsd/configlexer.lex
index d5fcd58b7f6..a168834067a 100644
--- usr.sbin/nsd/configlexer.lex
+++ usr.sbin/nsd/configlexer.lex
@@ -297,6 +297,8 @@ tls-cert-bundle{COLON}  { LEXOUT(("v(%s) ", yytext)); 
return VAR_TLS_CERT_BUNDLE;
 answer-cookie{COLON}   { LEXOUT(("v(%s) ", yytext)); return VAR_ANSWER_COOKIE;}
 cookie-secret{COLON}   { LEXOUT(("v(%s) ", yytext)); return VAR_COOKIE_SECRET;}
 cookie-secret-file{COLON}  { LEXOUT(("v(%s) ", yytext)); return 
VAR_COOKIE_SECRET_FILE;}
+xfrd-tcp-max{COLON}{ LEXOUT(("v(%s) ", yytext)); return VAR_XFRD_TCP_MAX;}
+xfrd-tcp-pipeline{COLON}   { LEXOUT(("v(%s) ", yytext)); return 
VAR_XFRD_TCP_PIPELINE;}
 {NEWLINE}  { LEXOUT(("NL\n")); cfg_parser->line++;}
 
 servers={UNQUOTEDLETTER}*  {
diff --git usr.sbin/nsd/configparser.y usr.sbin/nsd/configparser.y
index 70e54cf21f3..0815a0a16af 100644
--- usr.sbin/nsd/configparser.y
+++ usr.sbin/nsd/configparser.y
@@ -119,6 +119,8 @@ static int parse_range(const char *str, long long *low, 
long long *high);
 %token VAR_XFRD_CPU_AFFINITY
 %token  VAR_SERVER_CPU_AFFINITY
 %token VAR_DROP_UPDATES
+%token VAR_XFRD_TCP_MAX
+%token VAR_XFRD_TCP_PIPELINE
 
 /* dnstap */
 %token VAR_DNSTAP
@@ -454,6 +456,10 @@ server_option:
 { cfg_parser->opt->cookie_secret = region_strdup(cfg_parser->opt->region, 
$2); }
   | VAR_COOKIE_SECRET_FILE STRING
 { cfg_parser->opt->cookie_secret_file = 
region_strdup(cfg_parser->opt->region, $2); }
+  | VAR_XFRD_TCP_MAX number
+{ cfg_parser->opt->xfrd_tcp_max = (int)$2; }
+  | VAR_XFRD_TCP_PIPELINE number
+{ cfg_parser->opt->xfrd_tcp_pipeline = (int)$2; }
   | VAR_CPU_AFFINITY cpus
 {
   cfg_parser->opt->cpu_affinity = $2;
diff --git usr.sbin/nsd/configure usr.sbin/nsd/configure
index ca313c85794..0a5938d017b 100644
--- usr.sbin/nsd/configure
+++ usr.sbin/nsd/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.9.
+# Generated by GNU Autoconf 2.69 for NSD 4.4.0.
 #
 # Report bugs to .
 #
@@ -580,8 +580,8 @@ MAKEFLAGS=
 # Identity of this package.
 PACKAGE_NAME='NSD'
 PACKAGE_TARNAME='nsd'
-PACKAGE_VERSION='4.3.9'
-PACKAGE_STRING='NSD 4.3.9'
+PACKAGE_VERSION='4.4.0'
+PACKAGE_STRING='NSD 4.4.0'
 PACKAGE_BUGREPORT='nsd-b...@nlnetlabs.nl'
 PACKAGE_URL=''
 
@@ -1328,7 +1328,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.9 to adapt to many kinds of systems.
+\`configure' configures NSD 4.4.0 to adapt to many kinds of systems.
 
 Usage: $0 [OPTION]... [VAR=VALUE]...
 
@@ -1390,7 +1390,7 @@ fi
 
 if test -n "$ac_init_help"; then
   case $ac_init_help in
- short | recursive ) echo "Configuration of NSD 4.3.9:";;
+ short | recursive ) echo "Configuration of NSD 4.4.0:";;
esac
   cat <<\_ACEOF
 
@@ -1563,7 +1563,7 @@ fi
 test -n "$ac_init_help" && exit $ac_status
 if $ac_init_version; then
   cat <<\_ACEOF
-NSD configure 4.3.9
+NSD configure 4.4.0
 generated by GNU Autoconf 2.69
 
 Copyright (C) 2012 Free Software Foundation, Inc.
@@ -2272,7 +2272,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.9, which was
+It was created by NSD $as_me 4.4.0, which was
 generated by GNU Autoconf 2.69.  Invocation command line was
 
   $ $0 $@
@@ -11155,7 +11155,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.9, which was
+This file was extended by NSD $as_me 4.4.0, which was
 generated by GNU Autoconf 2.69.  Invocation command li

Re: unwind(8): simplify query parsing

[Florian Obser]

anyone?

On 2022-03-03 19:57 +01, Florian Obser  wrote:
> parse_packet() is used by unbound(8) to parse response packets, not
> queries. There is no need to do all this work just to get access to
> the query id and flags. This is what unbound(8) is doing.
>
> OK?
>
> diff --git frontend.c frontend.c
> index 6316231f4bf..ac53fc01ef1 100644
> --- frontend.c
> +++ frontend.c
> @@ -100,12 +100,13 @@ struct pending_query {
>   struct sldns_buffer *abuf;
>   struct regional *region;
>   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;
> + uint16_t id;
> + uint16_t flags;
>   int  fd;
>   int  tcp;
>   int  dns64_synthesize;
> @@ -536,8 +537,7 @@ frontend_dispatch_resolver(int fd, short event, void 
> *bula)
>   break;
>   }
>  
> - if (answer_header->bogus && !(pq->qmsg->flags &
> - BIT_CD)) {
> + if (answer_header->bogus && !(pq->flags & BIT_CD)) {
>   error_answer(pq, LDNS_RCODE_SERVFAIL);
>   send_answer(pq);
>   break;
> @@ -716,15 +716,13 @@ udp_receive(int fd, short events, void *arg)
>   pq->qbuf = sldns_buffer_new(len);
>   pq->abuf = sldns_buffer_new(len); /* make sure we can send errors */
>   pq->region = regional_create();
> - pq->qmsg = regional_alloc(pq->region, sizeof(*pq->qmsg));
>  
> - if (!pq->qbuf || !pq->abuf || !pq->region || !pq->qmsg) {
> + if (!pq->qbuf || !pq->abuf || !pq->region) {
>   log_warnx("out of memory");
>   free_pending_query(pq);
>   return;
>   }
>  
> - memset(pq->qmsg, 0, sizeof(*pq->qmsg));
>   sldns_buffer_write(pq->qbuf, udpev->query, len);
>   sldns_buffer_flip(pq->qbuf);
>   handle_query(pq);
> @@ -734,7 +732,6 @@ void
>  handle_query(struct pending_query *pq)
>  {
>   struct query_imsgquery_imsg;
> - struct query_infoskip;
>   struct bl_node   find;
>   int  rcode;
>   char*str;
> @@ -750,16 +747,16 @@ handle_query(struct pending_query *pq)
>   free(str);
>   }
>  
> - if (!query_info_parse(&pq->qinfo, pq->qbuf)) {
> - log_warnx("query_info_parse failed");
> + if (sldns_buffer_remaining(pq->qbuf) < LDNS_HEADER_SIZE) {
> + log_warnx("bad query: too short, dropped");
>   goto drop;
>   }
>  
> - sldns_buffer_rewind(pq->qbuf);
> + pq->id = sldns_buffer_read_u16_at(pq->qbuf, 0);
> + pq->flags = sldns_buffer_read_u16_at(pq->qbuf, 2);
>  
> - if (parse_packet(pq->qbuf, pq->qmsg, pq->region) !=
> - LDNS_RCODE_NOERROR) {
> - log_warnx("parse_packet failed");
> + if (!query_info_parse(&pq->qinfo, pq->qbuf)) {
> + log_warnx("query_info_parse failed");
>   goto drop;
>   }
>  
> @@ -774,11 +771,6 @@ handle_query(struct pending_query *pq)
>   goto send_answer;
>   }
>  
> - sldns_buffer_rewind(pq->qbuf);
> - if (!query_info_parse(&skip, pq->qbuf)) {
> - error_answer(pq, LDNS_RCODE_SERVFAIL);
> - goto send_answer;
> - }
>   rcode = parse_edns_from_query_pkt(pq->qbuf, &pq->edns, NULL, NULL,
>   pq->region);
>   if (rcode != LDNS_RCODE_NOERROR) {
> @@ -891,7 +883,7 @@ noerror_answer(struct pending_query *pq)
>   }
>  
>   sldns_buffer_clear(pq->abuf);
> - if (reply_info_encode(&pq->qinfo, rinfo, pq->qmsg->id, rinfo->flags,
> + if (reply_info_encode(&pq->qinfo, rinfo, htons(pq->id), rinfo->flags,
>   pq->abuf, 0, pq->region, pq->tcp ? UINT16_MAX : pq->edns.udp_size,
>   pq->edns.bits & EDNS_DO, MINIMIZE_ANSWER) == 0)
>   goto srvfail;
> @@ -986,7 +978,7 @@ synthesize_dns64_answer(struct p

unwind(8): simplify query parsing

[Florian Obser]

parse_packet() is used by unbound(8) to parse response packets, not
queries. There is no need to do all this work just to get access to
the query id and flags. This is what unbound(8) is doing.

OK?

diff --git frontend.c frontend.c
index 6316231f4bf..ac53fc01ef1 100644
--- frontend.c
+++ frontend.c
@@ -100,12 +100,13 @@ struct pending_query {
struct sldns_buffer *abuf;
struct regional *region;
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;
+   uint16_t id;
+   uint16_t flags;
int  fd;
int  tcp;
int  dns64_synthesize;
@@ -536,8 +537,7 @@ frontend_dispatch_resolver(int fd, short event, void *bula)
break;
}
 
-   if (answer_header->bogus && !(pq->qmsg->flags &
-   BIT_CD)) {
+   if (answer_header->bogus && !(pq->flags & BIT_CD)) {
error_answer(pq, LDNS_RCODE_SERVFAIL);
send_answer(pq);
break;
@@ -716,15 +716,13 @@ udp_receive(int fd, short events, void *arg)
pq->qbuf = sldns_buffer_new(len);
pq->abuf = sldns_buffer_new(len); /* make sure we can send errors */
pq->region = regional_create();
-   pq->qmsg = regional_alloc(pq->region, sizeof(*pq->qmsg));
 
-   if (!pq->qbuf || !pq->abuf || !pq->region || !pq->qmsg) {
+   if (!pq->qbuf || !pq->abuf || !pq->region) {
log_warnx("out of memory");
free_pending_query(pq);
return;
}
 
-   memset(pq->qmsg, 0, sizeof(*pq->qmsg));
sldns_buffer_write(pq->qbuf, udpev->query, len);
sldns_buffer_flip(pq->qbuf);
handle_query(pq);
@@ -734,7 +732,6 @@ void
 handle_query(struct pending_query *pq)
 {
struct query_imsgquery_imsg;
-   struct query_infoskip;
struct bl_node   find;
int  rcode;
char*str;
@@ -750,16 +747,16 @@ handle_query(struct pending_query *pq)
free(str);
}
 
-   if (!query_info_parse(&pq->qinfo, pq->qbuf)) {
-   log_warnx("query_info_parse failed");
+   if (sldns_buffer_remaining(pq->qbuf) < LDNS_HEADER_SIZE) {
+   log_warnx("bad query: too short, dropped");
goto drop;
}
 
-   sldns_buffer_rewind(pq->qbuf);
+   pq->id = sldns_buffer_read_u16_at(pq->qbuf, 0);
+   pq->flags = sldns_buffer_read_u16_at(pq->qbuf, 2);
 
-   if (parse_packet(pq->qbuf, pq->qmsg, pq->region) !=
-   LDNS_RCODE_NOERROR) {
-   log_warnx("parse_packet failed");
+   if (!query_info_parse(&pq->qinfo, pq->qbuf)) {
+   log_warnx("query_info_parse failed");
goto drop;
}
 
@@ -774,11 +771,6 @@ handle_query(struct pending_query *pq)
goto send_answer;
}
 
-   sldns_buffer_rewind(pq->qbuf);
-   if (!query_info_parse(&skip, pq->qbuf)) {
-   error_answer(pq, LDNS_RCODE_SERVFAIL);
-   goto send_answer;
-   }
rcode = parse_edns_from_query_pkt(pq->qbuf, &pq->edns, NULL, NULL,
pq->region);
if (rcode != LDNS_RCODE_NOERROR) {
@@ -891,7 +883,7 @@ noerror_answer(struct pending_query *pq)
}
 
sldns_buffer_clear(pq->abuf);
-   if (reply_info_encode(&pq->qinfo, rinfo, pq->qmsg->id, rinfo->flags,
+   if (reply_info_encode(&pq->qinfo, rinfo, htons(pq->id), rinfo->flags,
pq->abuf, 0, pq->region, pq->tcp ? UINT16_MAX : pq->edns.udp_size,
pq->edns.bits & EDNS_DO, MINIMIZE_ANSWER) == 0)
goto srvfail;
@@ -986,7 +978,7 @@ synthesize_dns64_answer(struct pending_query *pq)
 
sldns_buffer_clear(pq->abuf);
 
-   if (reply_info_encode(&pq->qinfo, synth_rinfo, pq->qmsg->id,
+   if (reply_info_encode(&pq->qinfo, synth_rinfo, htons(pq->id),
synth_rinfo->flags, pq->abuf, 0, pq->region,
pq->tcp ? UINT16_MAX : pq->edns.udp_size,
pq->edns.bits & EDNS_DO, MINIMIZE_ANSWER) == 0)
@@ -1006,7 +998,6 @@ void
 resend_dns64_query(struct pending_query *opq) {
struct pending_query*pq;
struct query_imsgquery_imsg;
-   struct query_infoskip;
int  rcode;
char dname[LDNS_MAX_DOMAINLEN + 1];
 
@@ -1028,9 +1019,8 @@ resend_

Re: [PATCH] httpd initialize kv structs on stack

[Florian Obser]

On 2022-03-01 10:22 -08, j...@bitminer.ca wrote:
> Looking at the gz option, I noticed some kv structs allocated on
> stack but not fully initialized.

Nice catch.

>
> This patches initializes the kv struct to avoid randomly getting
> KV_GLAG_GLOBBING in kv_find depending on stack contents, whenever
> a kv struct is allocated.
>
> Only kv structs seem to be affected.

The diff is correct, but a bit inconsistent. in server_fcgi.c you set
kv_flags every time kv_key is set, but you could just initialize it at
the beginning of the function. In server_http.c server_log_http() you
depend on kv_flags being initialized once.

TBH, I'd like my bikeshed green, how about we remove kv_flags, it's
unused and alreayd diverged from relayd(8).

OK?

If people want to keep the functionality, OK florian for John's diff.

diff --git httpd.c httpd.c
index 99687a18939..86d5ea9f96f 100644
--- httpd.c
+++ httpd.c
@@ -1063,22 +1063,7 @@ kv_free(struct kv *kv)
 struct kv *
 kv_find(struct kvtree *keys, struct kv *kv)
 {
-   struct kv   *match;
-   const char  *key;
-
-   if (kv->kv_flags & KV_FLAG_GLOBBING) {
-   /* Test header key using shell globbing rules */
-   key = kv->kv_key == NULL ? "" : kv->kv_key;
-   RB_FOREACH(match, kvtree, keys) {
-   if (fnmatch(key, match->kv_key, FNM_CASEFOLD) == 0)
-   break;
-   }
-   } else {
-   /* Fast tree-based lookup only works without globbing */
-   match = RB_FIND(kvtree, keys, kv);
-   }
-
-   return (match);
+   return (RB_FIND(kvtree, keys, kv));
 }
 
 int
diff --git httpd.h httpd.h
index 692c5611bb5..1b37d87c6e6 100644
--- httpd.h
+++ httpd.h
@@ -131,10 +131,6 @@ struct kv {
char*kv_key;
char*kv_value;
 
-#define KV_FLAG_INVALID 0x01
-#define KV_FLAG_GLOBBING0x02
-   uint8_t  kv_flags;
-
struct kvlistkv_children;
struct kv   *kv_parent;
TAILQ_ENTRY(kv)  kv_entry;
diff --git server_fcgi.c server_fcgi.c
index 6542b1f1739..810b217ebe0 100644
--- server_fcgi.c
+++ server_fcgi.c
@@ -702,9 +702,6 @@ server_fcgi_writeheader(struct client *clt, struct kv *hdr, 
void *arg)
const char  *key;
int  ret;
 
-   if (hdr->kv_flags & KV_FLAG_INVALID)
-   return (0);
-
/* The key might have been updated in the parent */
if (hdr->kv_parent != NULL && hdr->kv_parent->kv_key != NULL)
key = hdr->kv_parent->kv_key;
diff --git server_http.c server_http.c
index d5d31fa03ef..40e202665b5 100644
--- server_http.c
+++ server_http.c
@@ -1647,9 +1647,6 @@ server_writeheader_http(struct client *clt, struct kv 
*hdr, void *arg)
char*ptr;
const char  *key;
 
-   if (hdr->kv_flags & KV_FLAG_INVALID)
-   return (0);
-
/* The key might have been updated in the parent */
if (hdr->kv_parent != NULL && hdr->kv_parent->kv_key != NULL)
key = hdr->kv_parent->kv_key;


>
>
>
> John
>
> Index: server_fcgi.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
> retrieving revision 1.89
> diff -u -p -u -p -r1.89 server_fcgi.c
> --- server_fcgi.c 23 Oct 2021 15:52:44 -  1.89
> +++ server_fcgi.c 1 Mar 2022 15:35:41 -
> @@ -629,6 +629,7 @@ server_fcgi_header(struct client *clt, u
>  
>   /* But then we need a Content-Length unless method is HEAD... */
>   if (desc->http_method != HTTP_METHOD_HEAD) {
> + key.kv_flags = 0;
>   key.kv_key = "Content-Length";
>   if ((kv = kv_find(&resp->http_headers, &key)) == NULL) {
>   if (kv_add(&resp->http_headers,
> @@ -641,6 +642,7 @@ server_fcgi_header(struct client *clt, u
>   /* Send chunked encoding header */
>   if (clt->clt_fcgi.chunked) {
>   /* but only if no Content-Length header is supplied */
> + key.kv_flags = 0;
>   key.kv_key = "Content-Length";
>   if ((kv = kv_find(&resp->http_headers, &key)) != NULL) {
>   clt->clt_fcgi.chunked = 0;
> @@ -679,6 +681,7 @@ server_fcgi_header(struct client *clt, u
>   }
>  
>   /* Date header is mandatory and should be added as late as possible */
> + key.kv_flags = 0;
>   key.kv_key = "Date";
>   if (kv_find(&resp->http_headers, &key) == NULL &&
>   (server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
> Index: server_file.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/server_file.c,v
> retrieving revision 1.71
> diff -u -p -u -p -r1.71 server_file.c
> --- server_file.

Re: unwind(8): use parse_edns_from_pkt

[Florian Obser]

anyone had a chance to test this?

On 2022-02-24 18:38 +01, Florian Obser  wrote:
> Upstream renamed parse_extract_edns to
> parse_extract_edns_from_response_msg and parse_edns_from_pkt to
> parse_edns_from_query_pkt in the upcomming libunbound 1.15.0
> update. Both funktions work equally well for us but it would look weird
> to use the "from_response_msg" function on the query so switch to
> parse_edns_from_pkt in preparation for the libunbound update.
>
> OK?
>
> diff --git frontend.c frontend.c
> index 1b6333da22c..3a2ee87dea8 100644
> --- frontend.c
> +++ frontend.c
> @@ -734,6 +734,7 @@ void
>  handle_query(struct pending_query *pq)
>  {
>   struct query_imsgquery_imsg;
> + struct query_infoskip;
>   struct bl_node   find;
>   int  rcode;
>   char*str;
> @@ -773,7 +774,12 @@ handle_query(struct pending_query *pq)
>   goto send_answer;
>   }
>  
> - rcode = parse_extract_edns(pq->qmsg, &pq->edns, pq->region);
> + sldns_buffer_rewind(pq->qbuf);
> + if (!query_info_parse(&skip, pq->qbuf)) {
> + error_answer(pq, LDNS_RCODE_SERVFAIL);
> + goto send_answer;
> + }
> + rcode = parse_edns_from_pkt(pq->qbuf, &pq->edns, pq->region);
>   if (rcode != LDNS_RCODE_NOERROR) {
>   error_answer(pq, rcode);
>   goto send_answer;
> @@ -999,6 +1005,7 @@ void
>  resend_dns64_query(struct pending_query *opq) {
>   struct pending_query*pq;
>   struct query_imsgquery_imsg;
> + struct query_infoskip;
>   int  rcode;
>   char dname[LDNS_MAX_DOMAINLEN + 1];
>  
> @@ -1059,7 +1066,12 @@ resend_dns64_query(struct pending_query *opq) {
>   goto drop;
>   }
>  
> - rcode = parse_extract_edns(pq->qmsg, &pq->edns, pq->region);
> + sldns_buffer_rewind(pq->qbuf);
> + if (!query_info_parse(&skip, pq->qbuf)) {
> + error_answer(pq, LDNS_RCODE_SERVFAIL);
> + goto send_answer;
> + }
> + rcode = parse_edns_from_pkt(pq->qbuf, &pq->edns, pq->region);
>   if (rcode != LDNS_RCODE_NOERROR) {
>   error_answer(pq, rcode);
>   goto send_answer;
>
> -- 
>
> I'm not entirely sure you are real.
>

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

Re: sysupgrade(8): Pick correct firmware directory

[Florian Obser]

On 2022-02-28 06:30 -08, Andrew Hewus Fresh  wrote:
> On Mon, Feb 28, 2022 at 08:27:13AM +0100, Florian Obser wrote:
>> On 2022-02-27 21:33 -08, Andrew Hewus Fresh 
>>  wrote:
>> > So, sdk@ noticed that sysupgrade didn't get updated for the new logic in
>> > the firmware directories.  Specifically that the only time we use
>> > "snapshots" directory is with -current.  The rest of the time, including
>> > during -beta we use the version directory.  This diff should handle that
>> > case, installing the correct firmware for the system we are about to
>> > install.
>> >
>> > This also uses a secret feature of fw_update(8) where if you set "VNAME"
>> > in the environment, it uses that instead of asking sysctl to calculate
>> > the name of the signify key to use.   Probably that could use an
>> > improvement, maybe trusting the untrusted comment at the top of the
>> > file..
>> >
>> > I'm not sure if there's a better way to find the version string from a
>> > bsd kernel, so I used the one I knew about.
>> 
>> I think you can just look at _KERNV which is populated thusly:
>> 
>> 
>> 98   set -A _KERNV -- $(sysctl -n kern.version |
>> 99   sed 's/^OpenBSD \([1-9][0-9]*\.[0-9]\)\([^ ]*\).*/\1 
>> \2/;q')
>> 
>> $ sysctl kern.version
>> kern.version=OpenBSD 7.0-current (GENERIC.MP) #370: Sat Feb 19 10:36:59 MST 
>> 2022
>> dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
>> 
>> $ set -A _KERNV -- $(sysctl -n kern.version |
>> > sed 's/^OpenBSD \([1-9][0-9]*\.[0-9]\)\([^ ]*\).*/\1 \2/;q')
>> 
>> $ echo ${_KERNV[1]}
>> -current
>
>
> I don't think so, that is the running kernel, I need the version of the
> _new_ kernel.  Imagine this situation:

Oh, I missed that bit. There is what(1):

$ what bsd
bsd:
OpenBSD 7.1-beta (GENERIC) #368: Sun Feb 27 20:02:50 MST 2022


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

Re: sysupgrade(8): Pick correct firmware directory

[Florian Obser]

On 2022-02-27 21:33 -08, Andrew Hewus Fresh  
wrote:
> So, sdk@ noticed that sysupgrade didn't get updated for the new logic in
> the firmware directories.  Specifically that the only time we use
> "snapshots" directory is with -current.  The rest of the time, including
> during -beta we use the version directory.  This diff should handle that
> case, installing the correct firmware for the system we are about to
> install.
>
> This also uses a secret feature of fw_update(8) where if you set "VNAME"
> in the environment, it uses that instead of asking sysctl to calculate
> the name of the signify key to use.   Probably that could use an
> improvement, maybe trusting the untrusted comment at the top of the
> file..
>
> I'm not sure if there's a better way to find the version string from a
> bsd kernel, so I used the one I knew about.

I think you can just look at _KERNV which is populated thusly:


98  set -A _KERNV -- $(sysctl -n kern.version |
99  sed 's/^OpenBSD \([1-9][0-9]*\.[0-9]\)\([^ ]*\).*/\1 \2/;q')

$ sysctl kern.version
kern.version=OpenBSD 7.0-current (GENERIC.MP) #370: Sat Feb 19 10:36:59 MST 2022
dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP

$ set -A _KERNV -- $(sysctl -n kern.version |
> sed 's/^OpenBSD \([1-9][0-9]*\.[0-9]\)\([^ ]*\).*/\1 \2/;q')

$ echo ${_KERNV[1]}
-current

>
> Comments, OK?
>
> Index: sysupgrade.sh
> ===
> RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
> retrieving revision 1.45
> diff -u -p -r1.45 sysupgrade.sh
> --- sysupgrade.sh 11 Feb 2022 12:58:18 -  1.45
> +++ sysupgrade.sh 28 Feb 2022 05:24:48 -
> @@ -123,10 +123,8 @@ fi
>  
>  if $SNAP; then
>   URL=${MIRROR}/snapshots/${ARCH}/
> - FW_URL=http://firmware.openbsd.org/firmware/snapshots/
>  else
>   URL=${MIRROR}/${NEXT_VERSION}/${ARCH}/
> - FW_URL=http://firmware.openbsd.org/firmware/${NEXT_VERSION}/
>  fi
>  
>  install -d -o 0 -g 0 -m 0755 ${SETSDIR}
> @@ -196,7 +194,15 @@ __EOT
>  fi
>  
>  echo Fetching updated firmware.
> -fw_update -p ${FW_URL} || true
> +VTYPE="$( echo exit | config -eo /dev/null bsd |
> +sed -n "/^OpenBSD $NEXT_VERSION\([^ ]*\).*$/s//\1/p" )"
> +
> +if [ "$VTYPE" = "-current" ]; then
> + FW_URL=http://firmware.openbsd.org/firmware/snapshots/
> +else
> + FW_URL=http://firmware.openbsd.org/firmware/${NEXT_VERSION}/
> +fi
> +VNAME="${NEXT_VERSION}" fw_update -p ${FW_URL} || true
>  
>  install -F -m 700 bsd.rd /bsd.upgrade
>  logger -t sysupgrade -p kern.info "installed new /bsd.upgrade. Old kernel 
> version: $(sysctl -n kern.version)"
>

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

unwind(8): use parse_edns_from_pkt

[Florian Obser]

Upstream renamed parse_extract_edns to
parse_extract_edns_from_response_msg and parse_edns_from_pkt to
parse_edns_from_query_pkt in the upcomming libunbound 1.15.0
update. Both funktions work equally well for us but it would look weird
to use the "from_response_msg" function on the query so switch to
parse_edns_from_pkt in preparation for the libunbound update.

OK?

diff --git frontend.c frontend.c
index 1b6333da22c..3a2ee87dea8 100644
--- frontend.c
+++ frontend.c
@@ -734,6 +734,7 @@ void
 handle_query(struct pending_query *pq)
 {
struct query_imsgquery_imsg;
+   struct query_infoskip;
struct bl_node   find;
int  rcode;
char*str;
@@ -773,7 +774,12 @@ handle_query(struct pending_query *pq)
goto send_answer;
}
 
-   rcode = parse_extract_edns(pq->qmsg, &pq->edns, pq->region);
+   sldns_buffer_rewind(pq->qbuf);
+   if (!query_info_parse(&skip, pq->qbuf)) {
+   error_answer(pq, LDNS_RCODE_SERVFAIL);
+   goto send_answer;
+   }
+   rcode = parse_edns_from_pkt(pq->qbuf, &pq->edns, pq->region);
if (rcode != LDNS_RCODE_NOERROR) {
error_answer(pq, rcode);
goto send_answer;
@@ -999,6 +1005,7 @@ void
 resend_dns64_query(struct pending_query *opq) {
struct pending_query*pq;
struct query_imsgquery_imsg;
+   struct query_infoskip;
int  rcode;
char dname[LDNS_MAX_DOMAINLEN + 1];
 
@@ -1059,7 +1066,12 @@ resend_dns64_query(struct pending_query *opq) {
goto drop;
}
 
-   rcode = parse_extract_edns(pq->qmsg, &pq->edns, pq->region);
+   sldns_buffer_rewind(pq->qbuf);
+   if (!query_info_parse(&skip, pq->qbuf)) {
+   error_answer(pq, LDNS_RCODE_SERVFAIL);
+   goto send_answer;
+   }
+   rcode = parse_edns_from_pkt(pq->qbuf, &pq->edns, pq->region);
if (rcode != LDNS_RCODE_NOERROR) {
error_answer(pq, rcode);
goto send_answer;

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

[Wolf] [PATCH] Move warnx into correct place

[Florian Obser]

OK florian

 Start of forwarded message 
From: Wolf 
To: m...@openbsd.org
Cc: Wolf 
Subject: [PATCH] Move warnx into correct place
Date: Sun, 20 Feb 2022 15:10:16 +0100

Original location caused the line to be printed every time for ec keys.
I suspect copy&paste error from rsa_key_create.
---
 usr.sbin/acme-client/key.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/usr.sbin/acme-client/key.c b/usr.sbin/acme-client/key.c
index 6604751caef..64b00a36c2f 100644
--- a/usr.sbin/acme-client/key.c
+++ b/usr.sbin/acme-client/key.c
@@ -98,7 +98,7 @@ ec_key_create(FILE *f, const char *fname)
/* Serialise the key to the disc in EC format */
 
if (!PEM_write_ECPrivateKey(f, eckey, NULL, NULL, 0, NULL, NULL)) {
-   warnx("PEM_write_ECPrivateKey");
+   warnx("%s: PEM_write_ECPrivateKey", fname);
goto err;
}
 
@@ -113,8 +113,6 @@ ec_key_create(FILE *f, const char *fname)
goto err;
}
 
-   warnx("%s: PEM_write_ECPrivateKey", fname);
-
goto out;
 
 err:
-- 
2.34.1

 End of forwarded message 

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

Re: ping icmp ident collisions

[Florian Obser]

On 2022-02-18 12:17 +10, Jonathan Matthew  wrote:
> The only thing ping uses to determine whether a received icmp echo reply 
> packet is a
> response to one of its requests is the 16 bit icmp ident field.  If you ping 
> enough
> stuff at the same time, eventually you'll have two concurrent pings using the 
> same ident,
> and they will both see each other's replies.  Since we do tricky MAC stuff on 
> the ping
> payload, this results in signature mismatches that look like this:
>
> PING 172.23.94.210 (172.23.94.210): 56 data bytes
> 64 bytes from 172.23.94.210: icmp_seq=0 ttl=253 time=0.820 ms
> 64 bytes from 172.23.94.210: icmp_seq=1 ttl=253 time=0.419 ms
> 64 bytes from 172.23.94.210: icmp_seq=2 ttl=253 time=0.369 ms
> signature mismatch!
> 64 bytes from 172.23.94.210: icmp_seq=3 ttl=253 time=0.273 ms
>
> --- 172.23.94.210 ping statistics ---
> 4 packets transmitted, 5 packets received, -- somebody's duplicating packets!
> round-trip min/avg/max/std-dev = 0.273/0.376/0.820/0.265 ms
>
> ping is counting the packet with the signature mismatch as a reply it 
> received, and it
> prints a misleading message about duplicated packets because it got more 
> replies than
> the number of requests it sent.
>
> I think it would be more helpful not to count signature mismatch packets as 
> replies.
> If you're actually getting corrupted replies, I'd say that's more like packet 
> loss
> than normal operation.  If you're getting extra replies due to ident 
> collisions, this
> will result in ping sending and receiving the expected number of packets.
>
> Printing the source address and sequence number on signature mismatches would 
> also help.
> I would have figured this out much quicker had ping told me the mismatch 
> packets were
> from a completely different source.  For example:
>
> PING 172.23.94.210 (172.23.94.210): 56 data bytes
> 64 bytes from 172.23.94.210: icmp_seq=0 ttl=253 time=2.645 ms
> 64 bytes from 172.23.94.210: icmp_seq=1 ttl=253 time=1.360 ms
> 64 bytes from 172.23.94.210: icmp_seq=2 ttl=253 time=0.506 ms
> 64 bytes from 172.23.94.210: icmp_seq=3 ttl=253 time=0.615 ms
> signature mismatch from 10.138.79.45: icmp_seq=0
> 64 bytes from 172.23.94.210: icmp_seq=4 ttl=253 time=0.431 ms
>
> --- 172.23.94.210 ping statistics ---
> 5 packets transmitted, 5 packets received, 0.0% packet loss
> round-trip min/avg/max/std-dev = 0.431/1.111/2.645/0.835 ms
>
> ok?

OK florian

I think we can go further and also check the from address in the echo
reply case, like this.

If something on the path is so confused as to answer to our pings with
the wrong source address I think it's tcpdump time...

Feel free to put this in at the same time if you agree.

diff --git sbin/ping/ping.c sbin/ping/ping.c
index 6fa634bca3e..e47baa8912c 100644
--- sbin/ping/ping.c
+++ sbin/ping/ping.c
@@ -181,6 +181,9 @@ char *hostname;
 int ident; /* random number to identify our packets */
 int v6flag;/* are we ping6? */
 
+struct sockaddr_in dst4;
+struct sockaddr_in6 dst6;
+
 /* counters */
 int64_t npackets;  /* max packets to transmit */
 int64_t nreceived; /* # of packets we got back */
@@ -243,8 +246,8 @@ main(int argc, char *argv[])
struct addrinfo hints, *res;
struct itimerval itimer;
struct sockaddr *from, *dst;
-   struct sockaddr_in from4, dst4;
-   struct sockaddr_in6 from6, dst6;
+   struct sockaddr_in from4;
+   struct sockaddr_in6 from6;
struct cmsghdr *scmsg = NULL;
struct in6_pktinfo *pktinfo = NULL;
struct icmp6_filter filt;
@@ -1285,6 +1288,13 @@ pr_pack(u_char *buf, int cc, struct msghdr *mhdr)
}
 
if (echo_reply) {
+   if (v6flag) {
+   if (memcmp(&dst6, from, sizeof(dst6)) != 0)
+   return; /* 'Twas not our ECHO */
+   } else {
+   if (memcmp(&dst4, from, sizeof(dst4)) != 0)
+   return; /* 'Twas not our ECHO */
+   }
++nreceived;
if (cc >= ECHOLEN + ECHOTMLEN) {
SIPHASH_CTX ctx;
@@ -1302,7 +1312,10 @@ pr_pack(u_char *buf, int cc, struct msghdr *mhdr)
 
if (timingsafe_memcmp(mac, &payload.mac,
sizeof(mac)) != 0) {
-   printf("signature mismatch!\n");
+   printf("signature mismatch from %s: "
+   "icmp_seq=%u\n", pr_addr(from, fromlen),
+   ntohs(seq));
+   --nreceived;
return;
}
timinginfo=1;


>
> Index: ping.c
> ===
> RCS file: /cvs/src/sbin/ping/ping.c,v
> retrieving revision 1.245
> diff -u -p -r1.245 ping.c
> --- pin

Re: dhcpleased(8) vs. microsoft dhcp server

[Florian Obser]

On 2022-02-15 12:07 -07, "Todd C. Miller"  wrote:
> On Tue, 15 Feb 2022 20:01:52 +0100, Florian Obser wrote:
>
> I think you need that to be:
>
>   /* MUST delete trailing NUL, per RFC 2132 */
>   slen = dho_len;
>   while (slen > 0 && p[slen - 1] == '\0')
>   slen--;
>
> to avoid underflow if the string happens to consist entirely of NULs.
> I'd also add:
>
>   if (slen < 1)
>   goto wrong_length;
>
>  - todd

ugh, of course. Teaches me right to rewrite the diff one last time
before sending :/
Also fixes a whitespace issue while here.

if (dho_len < 1)
goto wrong_length;

is redundant now, but I want to keep the pattern of checking the length
right after identifying the option.


diff --git engine.c engine.c
index fa25fbbf0b9..b7cfbdca4c9 100644
--- engine.c
+++ engine.c
@@ -727,7 +727,7 @@ parse_dhcp(struct dhcpleased_iface *iface, struct imsg_dhcp 
*dhcp)
size_t   rem, i;
uint32_t sum, usum, lease_time = 0, renewal_time = 0;
uint32_t rebinding_time = 0;
-   uint8_t *p, dho = DHO_PAD, dho_len;
+   uint8_t *p, dho = DHO_PAD, dho_len, slen;
uint8_t  dhcp_message_type = 0;
int  routes_len = 0, routers = 0, csr = 0;
char from[sizeof("xx:xx:xx:xx:xx:xx")];
@@ -1014,18 +1014,30 @@ parse_dhcp(struct dhcpleased_iface *iface, struct 
imsg_dhcp *dhcp)
rem -= dho_len;
break;
case DHO_HOST_NAME:
-   if ( dho_len < 1)
+   if (dho_len < 1)
goto wrong_length;
-   strvisx(hostname, p, dho_len, VIS_SAFE);
+   /* MUST delete trailing NUL, per RFC 2132 */
+   slen = dho_len;
+   while (slen > 0 && p[slen - 1] == '\0')
+   slen--;
+   if (slen < 1)
+   goto wrong_length;
+   strvisx(hostname, p, slen, VIS_SAFE);
if (log_getverbose() > 1)
log_debug("DHO_HOST_NAME: %s", hostname);
p += dho_len;
rem -= dho_len;
break;
case DHO_DOMAIN_NAME:
-   if ( dho_len < 1)
+   if (dho_len < 1)
+   goto wrong_length;
+   /* MUST delete trailing NUL, per RFC 2132 */
+   slen = dho_len;
+   while (slen > 0 && p[slen - 1] == '\0')
+   slen--;
+   if (slen < 1)
goto wrong_length;
-   strvisx(domainname, p, dho_len, VIS_SAFE);
+   strvisx(domainname, p, slen, VIS_SAFE);
if (log_getverbose() > 1)
log_debug("DHO_DOMAIN_NAME: %s", domainname);
p += dho_len;


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

dhcpleased(8) vs. microsoft dhcp server

[Florian Obser]

Jan reported that the microsoft dhcp server sends the domain name option
as a C string (i.e. NUL terminated) on-wire.
This then ends up in /var/db/dhcpleased/$IF as e.g.

domain-name: example.com\^@

which the installer uses to form /etc/myname which then later on smtpd
complains about.

I'm fresh out of microsoft dhcp servers, but this scapy script
demonstrates the problem:

from scapy.all import DHCP_am
from scapy.base_classes import Net

dhcp_server = DHCP_am(iface='carp0', domain='example.com\0',
  pool=Net('192.168.10.0/24'),
  network='192.168.10.0/24',
  gw='192.168.10.254',
  renewal_time=600, lease_time=3600)
dhcp_server()


RFC 2132, 2. BOOTP Extension/DHCP Option Field Format

   Options containing NVT ASCII data SHOULD
   NOT include a trailing NULL; however, the receiver of such options
   MUST be prepared to delete trailing nulls if they exist.  The
   receiver MUST NOT require that a trailing null be included in the
   data.

OK?

p.s. I'm happy to entertain diffs that ensure that the hostname and
domainname follow the rules of RFC 1034, 3.5. Preferred name syntax.

diff --git engine.c engine.c
index fa25fbbf0b9..dad299a0a6e 100644
--- engine.c
+++ engine.c
@@ -727,7 +727,7 @@ parse_dhcp(struct dhcpleased_iface *iface, struct imsg_dhcp 
*dhcp)
size_t   rem, i;
uint32_t sum, usum, lease_time = 0, renewal_time = 0;
uint32_t rebinding_time = 0;
-   uint8_t *p, dho = DHO_PAD, dho_len;
+   uint8_t *p, dho = DHO_PAD, dho_len, slen;
uint8_t  dhcp_message_type = 0;
int  routes_len = 0, routers = 0, csr = 0;
char from[sizeof("xx:xx:xx:xx:xx:xx")];
@@ -1016,7 +1016,11 @@ parse_dhcp(struct dhcpleased_iface *iface, struct 
imsg_dhcp *dhcp)
case DHO_HOST_NAME:
if ( dho_len < 1)
goto wrong_length;
-   strvisx(hostname, p, dho_len, VIS_SAFE);
+   /* MUST delete trailing NUL, per RFC 2132 */
+   slen = dho_len;
+   while (p[slen - 1] == '\0')
+   slen--;
+   strvisx(hostname, p, slen, VIS_SAFE);
if (log_getverbose() > 1)
log_debug("DHO_HOST_NAME: %s", hostname);
p += dho_len;
@@ -1025,7 +1029,11 @@ parse_dhcp(struct dhcpleased_iface *iface, struct 
imsg_dhcp *dhcp)
case DHO_DOMAIN_NAME:
if ( dho_len < 1)
goto wrong_length;
-   strvisx(domainname, p, dho_len, VIS_SAFE);
+   /* MUST delete trailing NUL, per RFC 2132 */
+   slen = dho_len;
+   while (p[slen - 1] == '\0')
+   slen--;
+   strvisx(domainname, p, slen, VIS_SAFE);
if (log_getverbose() > 1)
log_debug("DHO_DOMAIN_NAME: %s", domainname);
p += dho_len;


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

Re: adding MIME type for XSLT

[Florian Obser]

On 2022-02-11 21:51 UTC, Stuart Henderson  wrote:
> On 2022/02/11 11:19, Florian Obser wrote:
>> I'm wondering if we need to sync, unfortunately the two files are
>> not diffable :/
>
> easy enough to transform, and the extensions and mimetypes are basically
> in sync. here are the differences:
>
> --- ours
> +++ nginx
> @@ -2 +1,0 @@ application/atom+xml atom
> -application/font-woff woff
> @@ -11 +10 @@ application/octet-stream dmg
> -application/octet-stream fs iso img
> +application/octet-stream iso img
> @@ -64,0 +64,3 @@ audio/x-realaudio ra
> +font/woff woff
> +font/woff2 woff2
> +image/avif avif
>
> we want to keep fs in octet-stream, it would make sense to change woff,
> and add avif.
>

That makes sense I suppose.

In general I don't have a clue about mime types. I was only remembering
how we came up with that list in the first place...

No idea what to do about xslt...

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

Re: adding MIME type for XSLT

[Florian Obser]

On 2022-02-11 02:29 -07, "Anthony J. Bentley"  wrote:
> Jesse Alama writes:
>> XSLT is a well-established XML-based language for stylesheets. It has been ar
>> ound since the late 90s; the most recent version was finalized in 2017 (see  
>> https://www.w3.org/TR/xslt-30/). The mime.types file bundled with OpenBSD 7.0
>>  -- typically used with httpd -- doesn't include this common MIME type. May w
>> e add it? Conventionally, XSLT files use the .xsl file extension and the stan
>> dard MIME type is "application/xslt+xml" (see https://datatracker.ietf.org/do
>> c/html/rfc3023#section-8.17). A diff looks like this:
>>
>> diff -Naur /usr/share/misc/mime.types /usr/src/share/misc/mime.types
>> --- /usr/share/misc/mime.types   Thu Sep 30 20:01:17 2021
>> +++ /usr/src/share/misc/mime.types   Fri Feb 11 07:36:11 2022
>> @@ -56,6 +56,7 @@
>>  application/x-tcl   tcl tk
>>  application/x-x509-ca-cert  der pem crt
>>  application/x-xpinstall xpi
>> +application/xslt+xmlxsl
>>  application/xhtml+xml   xhtml
>>  application/zip zip
>
> The list is sorted alphabetically, so xslt needs to come after xhtml.
>
> I like the idea. From some basic searches it looks like Chrome might be
> unable to handle XSLT with the registered MIME type, only supporting
> text/xml. Is that still the case, and if so, do we care?
>

IIRC we got the list of mime types originally from nginx.
I just had a look, it does not have a mime type for xsl.

I'm wondering if we need to sync, unfortunately the two files are
not diffable :/

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

Re: IPv6 privacy extensions

[Florian Obser]

On 2022-01-24 00:17 +01, Marcel Logen <33327110-0...@ybtra.de> wrote:
> Hello,
>
> since ca. April 2021 I see, that (after boot) no new IPv6
> temporary adresses are created after 900 seconds (15 min).
>
> The pltime decreases to 900 and then gets a value of 1800.
> No new temporary address is generated.
>
> Is this behaviour correct?

Yes.

slaacd(8) used to never renew temporary addresses but would form a new
one once the pltime reached 0. This was because I misunderstood what the
RFC was saying and did not appreciate what would hapen.
If there is a huge difference between pltime and vltime in router
advertisements slaacd would accumulate deprecated temporary IPv6
addresses and wouldn't be able to get rid of them since the vltime did
not expire. Say you have a pltime of 5 minutes and a vltime of a
day. You'd get a new temporary address every 5 minutes and they would
stick around for a day. You'd end up with 288 temporary IPv6 addresses.

Long story short, this was fixed in rev 1.57 in engine.c

See RFC 8981 3.4. step 1.

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

Re: slaacd(8): router lifetime zero vs. prefixes

[Florian Obser]

ping
On 2021-12-27 17:01 +01, Florian Obser  wrote:
> Prefix life time is independent from router life time.
> Form an IPv6 address even if the router announcing the prefix isn't a
> default router.
> Problem reported by mgraves AT brainfat.net on misc
>
> OK?
>
> diff --git engine.c engine.c
> index 81a06cc5528..7a2c11e1bc2 100644
> --- engine.c
> +++ engine.c
> @@ -1749,14 +1749,13 @@ void update_iface_ra(struct slaacd_iface *iface, 
> struct radv *ra)
>  
>   update_iface_ra_dfr(iface, ra);
>  
> - if (ra->router_lifetime != 0)
> - LIST_FOREACH(prefix, &ra->prefixes, entries) {
> - if (!prefix->autonomous || prefix->vltime == 0 ||
> - prefix->pltime > prefix->vltime ||
> - IN6_IS_ADDR_LINKLOCAL(&prefix->prefix))
> - continue;
> - update_iface_ra_prefix(iface, ra, prefix);
> - }
> + LIST_FOREACH(prefix, &ra->prefixes, entries) {
> + if (!prefix->autonomous || prefix->vltime == 0 ||
> + prefix->pltime > prefix->vltime ||
> + IN6_IS_ADDR_LINKLOCAL(&prefix->prefix))
> + continue;
> + update_iface_ra_prefix(iface, ra, prefix);
> + }
>  
>   update_iface_ra_rdns(iface, ra);
>  }
>
> -- 
>
> I'm not entirely sure you are real.
>

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