Re: [PATCH] remove variable names from prototypes in sys_pipe.c

2020-11-14 Thread Sebastian Benoit
henkjan gersen(h.ger...@gmail.com) on 2020.11.14 19:00:15 +:
> OK, no problem it just looked strange when reading the code as most
> definitions in that file don't have them.
> 
> btw) I'm by now way more puzzled why this file has a function to create a pipe
> 
> struct pipe_pair *pipe_pair_create(void);
> 
> but lacks a corresponding destroy function. This makes it look like
> the pipes created as a pipe_pair don't get properly destroyed, but my C
> is rusty.

pipe(2) explains

  The pipe itself persists until all its associated descriptors are closed.



rpki-client 6.8p1 released

2020-11-12 Thread Sebastian Benoit
rpki-client 6.8p1 has just been released and will be available in the
rpki-client directory of any OpenBSD mirror soon.

rpki-client is a FREE, easy-to-use implementation of the Resource
Public Key Infrastructure (RPKI) for Relying Parties (RP) to
facilitate validation of the Route Origin of a BGP announcement. The
program queries the RPKI repository system and outputs Validated ROA
Payloads in the configuration format of OpenBGPD, BIRD, and also as
CSV or JSON objects for consumption by other routing stacks.

See RFC 6811 for a description of how BGP Prefix Origin Validation
secures the Internet's global routing system.

rpki-client was primarily developed by Kristaps Dzonsons, Claudio
Jeker, Job Snijders, and Sebastian Benoit as part of the OpenBSD
Project and gets released as a base component of OpenBSD every six
months, and follows the OpenBSD release numbering scheme.

This is the second release based on OpenBSD 6.8. It includes the following
changes to the previous release:

* incorporate OpenBSD 6.8 errata 006 of November 10, 2020:
  rpki-client incorrectly checks the manifest validity interval.

In the portable version,

* Add compat code for the LibreSSL ASN1_time_parse() and ASN1_time_tm_cmp()
  functions. Those are needed to properly check the validity of MFT files.

rpki-client is known to compile and run on at least the following
Linux distributions: Alpine 3.12, Debian 9, Debian 10, Fedora 31,
Fedora 32, Fedora 33, RHEL/CentOS 7, RHEL/CentOS 8.
It is our hope that packagers take interest and help adapt
OpenBGPD-portable to more distributions.

The mirrors where rpki-client can be found are on
https://www.rpki-client.org/portable.html

Reporting Bugs:
===

General bugs may be reported to tech@openbsd.org

Portable bugs may be filed at 
https://github.com/rpki-client/rpki-client-portable

We welcome feedback and improvements from the broader community.
Thanks to all of the contributors who helped make this release
possible.



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

2020-11-03 Thread Sebastian Benoit
Florian Obser(flor...@openbsd.org) on 2020.11.03 14:52:55 +0100:
> On Tue, Nov 03, 2020 at 10:37:04AM +0100, Matthias Pressfreund wrote:
> > 
> > On 2020-11-03 09:56, Florian Obser wrote:
> > > On Mon, Nov 02, 2020 at 02:35:48PM +0100, Matthias Pressfreund wrote:
> > >> The patch below updates the acme-client(1) manpage by providing a
> > >> closer match for the httpd(8) location block accepting acme challenge
> > >> responses.
> > > 
> > > How is this better?
> > > 
> > > When the requested file exits in /var/www/acme/ I get a 200 in both cases.
> > > When the file does not exists I get a 404 in both cases.
> > > 
> > 
> > It is better because I may not want the server to return 404 if the file
> > does not exist. Instead, I may want to let the server fall back to its
> > default behavior as shown in the example below where it would simply drop
> > the connection.
> > 
> > server "example.com" {
> > ...
> > block drop
> > location found "/.well-known/acme-challenge/*" { ... }
> > ...
> > }
> 
> I don't know, I'm not buying it, this doesn't feel neccesary for acme.
> We wanted to have a minimal example that gets people going with
> acme-client and httpd, now we have more fluff.
> 
> But I guess it's just me, so meh.

Its not just you.
I think the example is fine.



Re: Refactor bgpd control code

2020-11-03 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2020.11.03 09:09:47 +0100:
> On Wed, Oct 21, 2020 at 07:16:07PM +0200, Claudio Jeker wrote:
> > This refactors the control code a bit and removes the common var from the
> > session.h header. The session engine no longer walks the control
> > connection list. Additionally cleanup the control.c code around
> > control_dispatch_msg(). E.g. don't do double lookups of control sessions
> > by fd to close them.
> > 
> > OK?

ok


> 
> Ping
> 
> -- 
> :wq Claudio
> 
> Index: control.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/control.c,v
> retrieving revision 1.100
> diff -u -p -r1.100 control.c
> --- control.c 10 May 2020 13:38:46 -  1.100
> +++ control.c 21 Oct 2020 16:57:53 -
> @@ -29,11 +29,13 @@
>  #include "session.h"
>  #include "log.h"
>  
> +TAILQ_HEAD(ctl_conns, ctl_conn) ctl_conns = 
> TAILQ_HEAD_INITIALIZER(ctl_conns);
> +
>  #define  CONTROL_BACKLOG 5
>  
>  struct ctl_conn  *control_connbyfd(int);
>  struct ctl_conn  *control_connbypid(pid_t);
> -int   control_close(int);
> +int   control_close(struct ctl_conn *);
>  void  control_result(struct ctl_conn *, u_int);
>  ssize_t   imsg_read_nofd(struct imsgbuf *);
>  
> @@ -136,6 +138,22 @@ control_shutdown(int fd)
>   close(fd);
>  }
>  
> +size_t
> +control_fill_pfds(struct pollfd *pfd, size_t size)
> +{
> + struct ctl_conn *ctl_conn;
> + size_t i = 0;
> +
> + TAILQ_FOREACH(ctl_conn, _conns, entry) {
> + pfd[i].fd = ctl_conn->ibuf.fd;
> + pfd[i].events = POLLIN;
> + if (ctl_conn->ibuf.w.queued > 0)
> + pfd[i].events |= POLLOUT;
> + i++;
> + }
> + return i;
> +}
> +
>  unsigned int
>  control_accept(int listenfd, int restricted)
>  {
> @@ -198,15 +216,8 @@ control_connbypid(pid_t pid)
>  }
>  
>  int
> -control_close(int fd)
> +control_close(struct ctl_conn *c)
>  {
> - struct ctl_conn *c;
> -
> - if ((c = control_connbyfd(fd)) == NULL) {
> - log_warn("control_close: fd %d: not found", fd);
> - return (0);
> - }
> -
>   if (c->terminate && c->ibuf.pid)
>   imsg_ctl_rde(IMSG_CTL_TERMINATE, c->ibuf.pid, NULL, 0);
>  
> @@ -220,8 +231,7 @@ control_close(int fd)
>  }
>  
>  int
> -control_dispatch_msg(struct pollfd *pfd, u_int *ctl_cnt,
> -struct peer_head *peers)
> +control_dispatch_msg(struct pollfd *pfd, struct peer_head *peers)
>  {
>   struct imsg  imsg;
>   struct ctl_conn *c;
> @@ -237,10 +247,8 @@ control_dispatch_msg(struct pollfd *pfd,
>   }
>  
>   if (pfd->revents & POLLOUT) {
> - if (msgbuf_write(>ibuf.w) <= 0 && errno != EAGAIN) {
> - *ctl_cnt -= control_close(pfd->fd);
> - return (1);
> - }
> + if (msgbuf_write(>ibuf.w) <= 0 && errno != EAGAIN)
> + return control_close(c);
>   if (c->throttled && c->ibuf.w.queued < CTL_MSG_LOW_MARK) {
>   if (imsg_ctl_rde(IMSG_XON, c->ibuf.pid, NULL, 0) != -1)
>   c->throttled = 0;
> @@ -251,16 +259,12 @@ control_dispatch_msg(struct pollfd *pfd,
>   return (0);
>  
>   if (((n = imsg_read_nofd(>ibuf)) == -1 && errno != EAGAIN) ||
> - n == 0) {
> - *ctl_cnt -= control_close(pfd->fd);
> - return (1);
> - }
> + n == 0)
> + return control_close(c);
>  
>   for (;;) {
> - if ((n = imsg_get(>ibuf, )) == -1) {
> - *ctl_cnt -= control_close(pfd->fd);
> - return (1);
> - }
> + if ((n = imsg_get(>ibuf, )) == -1)
> + return control_close(c);
>  
>   if (n == 0)
>   break;
> Index: session.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
> retrieving revision 1.402
> diff -u -p -r1.402 session.c
> --- session.c 27 Jun 2020 07:24:42 -  1.402
> +++ session.c 21 Oct 2020 16:49:10 -
> @@ -196,7 +196,6 @@ session_main(int debug, int verbose)
>   struct peer *p, **peer_l = NULL, *next;
>   struct mrt  *m, *xm, **mrt_l = NULL;
>   struct pollfd   *pfd = NULL;
> - struct ctl_conn *ctl_conn;
>   struct listen_addr  *la;
>   void*newp;
>   time_t   now;
> @@ -237,7 +236,6 @@ session_main(int debug, int verbose)
>   fatal(NULL);
>   imsg_init(ibuf_main, 3);
>  
> - TAILQ_INIT(_conns);
>   LIST_INIT();
>   listener_cnt = 0;
>   peer_cnt = 0;
> @@ -438,13 +436,10 @@ session_main(int debug, int verbose)
>  
>   idx_mrts = i;
>  
> - TAILQ_FOREACH(ctl_conn, _conns, entry) {
> -

Re: cleanup bgpd commons first step

2020-11-03 Thread Sebastian Benoit
ok

Claudio Jeker(cje...@diehard.n-r-g.com) on 2020.11.03 09:07:31 +0100:
> On Wed, Oct 21, 2020 at 06:08:05PM +0200, Claudio Jeker wrote:
> > Bgpd uses many common symbols and the latest compilers are being picky
> > about these common symbols.
> > This removes the global bgpd_process variable and cleans up the filter_set
> > code to not depend on process knowledge (instead use a new type and don't
> > overload another one).
> 
> Ping
>  
> -- 
> :wq Claudio
> 
> Index: bgpd.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.c,v
> retrieving revision 1.229
> diff -u -p -r1.229 bgpd.c
> --- bgpd.c11 May 2020 16:59:19 -  1.229
> +++ bgpd.c21 Oct 2020 07:18:00 -
> @@ -117,10 +117,9 @@ main(int argc, char *argv[])
>   int  pipe_m2r[2];
>  
>   conffile = CONFFILE;
> - bgpd_process = PROC_MAIN;
>  
>   log_init(1, LOG_DAEMON);/* log to stderr until daemonized */
> - log_procinit(log_procnames[bgpd_process]);
> + log_procinit(log_procnames[PROC_MAIN]);
>   log_setverbose(1);
>  
>   saved_argv0 = argv[0];
> Index: bgpd.h
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.403
> diff -u -p -r1.403 bgpd.h
> --- bgpd.h10 May 2020 13:38:46 -  1.403
> +++ bgpd.h21 Oct 2020 07:29:53 -
> @@ -120,7 +120,7 @@ enum bgpd_process {
>   PROC_MAIN,
>   PROC_SE,
>   PROC_RDE
> -} bgpd_process;
> +};
>  
>  enum reconf_action {
>   RECONF_NONE,
> @@ -995,6 +995,7 @@ enum action_types {
>   ACTION_SET_PREPEND_PEER,
>   ACTION_SET_AS_OVERRIDE,
>   ACTION_SET_NEXTHOP,
> + ACTION_SET_NEXTHOP_REF,
>   ACTION_SET_NEXTHOP_REJECT,
>   ACTION_SET_NEXTHOP_BLACKHOLE,
>   ACTION_SET_NEXTHOP_NOMODIFY,
> @@ -1017,7 +1018,7 @@ struct filter_set {
>   u_int32_tmetric;
>   int32_t  relative;
>   struct bgpd_addr nexthop;
> - struct nexthop  *nh;
> + struct nexthop  *nh_ref;
>   struct community community;
>   char pftable[PFTABLE_LEN];
>   char rtlabel[RTLABEL_LEN];
> Index: printconf.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/printconf.c,v
> retrieving revision 1.142
> diff -u -p -r1.142 printconf.c
> --- printconf.c   23 Apr 2020 16:13:11 -  1.142
> +++ printconf.c   21 Oct 2020 07:32:06 -
> @@ -356,6 +356,7 @@ print_set(struct filter_set_head *set)
>   break;
>   case ACTION_RTLABEL_ID:
>   case ACTION_PFTABLE_ID:
> + case ACTION_SET_NEXTHOP_REF:
>   /* not possible */
>   printf("king bula saiz: config broken");
>   break;
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.503
> diff -u -p -r1.503 rde.c
> --- rde.c 21 Oct 2020 06:56:32 -  1.503
> +++ rde.c 21 Oct 2020 09:39:49 -
> @@ -157,8 +157,7 @@ rde_main(int debug, int verbose)
>   log_init(debug, LOG_DAEMON);
>   log_setverbose(verbose);
>  
> - bgpd_process = PROC_RDE;
> - log_procinit(log_procnames[bgpd_process]);
> + log_procinit(log_procnames[PROC_RDE]);
>  
>   if ((pw = getpwnam(BGPD_USER)) == NULL)
>   fatal("getpwnam");
> @@ -509,8 +508,11 @@ badnetdel:
>   if ((s = malloc(sizeof(struct filter_set))) == NULL)
>   fatal(NULL);
>   memcpy(s, imsg.data, sizeof(struct filter_set));
> - if (s->type == ACTION_SET_NEXTHOP)
> - s->action.nh = nexthop_get(>action.nexthop);
> + if (s->type == ACTION_SET_NEXTHOP) {
> + s->action.nh_ref =
> + nexthop_get(>action.nexthop);
> + s->type = ACTION_SET_NEXTHOP_REF;
> + }
>   TAILQ_INSERT_TAIL(_set, s, entry);
>   break;
>   case IMSG_CTL_SHOW_NETWORK:
> @@ -922,8 +924,11 @@ rde_dispatch_imsg_parent(struct imsgbuf 
>   if ((s = malloc(sizeof(struct filter_set))) == NULL)
>   fatal(NULL);
>   memcpy(s, imsg.data, sizeof(struct filter_set));
> - if (s->type == ACTION_SET_NEXTHOP)
> - s->action.nh = nexthop_get(>action.nexthop);
> + if (s->type == ACTION_SET_NEXTHOP) {
> + s->action.nh_ref =
> +   

Re: Minor tweak relayd agentx manpage

2020-10-30 Thread Sebastian Benoit
sure

Martijn van Duren(openbsd+t...@list.imperialat.at) on 2020.10.30 09:53:08 +0100:
> I think metrics is a better word than statistics and it might help
> people if they knew where to query for these metrics.
> 
> OK?
> martijn@
> 
> Index: relayd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relayd.conf.5,v
> retrieving revision 1.201
> diff -u -p -r1.201 relayd.conf.5
> --- relayd.conf.5 22 Oct 2020 08:00:24 -  1.201
> +++ relayd.conf.5 30 Oct 2020 08:48:23 -
> @@ -121,10 +121,12 @@ Here are the settings that can be set gl
>  .It Ic agentx Oo Ic context Ar context Oc Oo Ic path Ar path Oc
>  Export
>  .Xr relayd 8
> -statistics via an agentx compatible
> +metrics via an agentx compatible
>  .Pq snmp
>  daemon by connecting to
>  .Ar path .
> +Metrics can be found under the relaydMIBObjects subtree
> +.Pq enterprises.30155.3 .
>  If
>  .Ar path
>  is omitted it will default to
> 



Re: relayd(8) remove snmp keyword

2020-10-30 Thread Sebastian Benoit
ok benno@

and yes, add a line to current.html.

Denis Fondras(open...@ledeuns.net) on 2020.10.30 10:13:56 +0100:
> On Thu, Oct 29, 2020 at 03:51:24PM +0100, Martijn van Duren wrote:
> > 6.8 is out in the wild. I guess this is as good a time as any to remove
> > the old snmp keyword.
> > 
> > OK?
> > 
> 
> OK denis@
> 
> And while it is fresh, is this the right time to update plus.html and
> current.html ?
> 



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

2020-10-20 Thread Sebastian Benoit
still ok

Florian Obser(flor...@openbsd.org) on 2020.10.20 21:36:06 +0200:
> On Tue, Oct 20, 2020 at 01:11:09PM -0600, Theo de Raadt wrote:
> > I believe most of the new local variables you added in main,
> > can instead be added in the flush loop you wrote, even if you
> > have to instantiate them.  Or move it into a new function,
> > then it is even easier.  their scope is just too large...
> 
> Sure, I'd like to go with this for now and then refactor main later
> and split it into functions. It's always a pain to find the spot where
> it does anything...
> 
> diff --git ping.c ping.c
> index d198d233921..7cca8c70342 100644
> --- ping.c
> +++ ping.c
> @@ -786,6 +786,55 @@ main(int argc, char *argv[])
>   smsghdr.msg_iov = 
>   smsghdr.msg_iovlen = 1;
>  
> + /* Drain our socket. */
> + (void)signal(SIGALRM, onsignal);
> + memset(, 0, sizeof(itimer));
> + itimer.it_value.tv_sec = 1; /* make sure we don't get stuck */
> + (void)setitimer(ITIMER_REAL, , NULL);
> + for (;;) {
> + struct msghdr   m;
> + union {
> + struct cmsghdr hdr;
> + u_char buf[CMSG_SPACE(1024)];
> + }   cmsgbuf;
> + struct ioveciov[1];
> + struct pollfd   pfd;
> + struct sockaddr_in  peer4;
> + struct sockaddr_in6 peer6;
> + ssize_t cc;
> +
> + if (seenalrm)
> + break;
> +
> + pfd.fd = s;
> + pfd.events = POLLIN;
> +
> + if (poll(, 1, 0) <= 0)
> + break;
> +
> + if (v6flag) {
> + m.msg_name = 
> + m.msg_namelen = sizeof(peer6);
> + } else {
> + m.msg_name = 
> + m.msg_namelen = sizeof(peer4);
> + }
> + memset(, 0, sizeof(iov));
> + iov[0].iov_base = (caddr_t)packet;
> + iov[0].iov_len = packlen;
> +
> + m.msg_iov = iov;
> + m.msg_iovlen = 1;
> + m.msg_control = (caddr_t)
> + m.msg_controllen = sizeof(cmsgbuf.buf);
> +
> + cc = recvmsg(s, , 0);
> + if (cc == -1 && errno != EINTR)
> + break;
> + }
> + memset(, 0, sizeof(itimer));
> + (void)setitimer(ITIMER_REAL, , NULL);
> +
>   while (preload--)   /* Fire off them quickies. */
>   pinger(s);
>  
> 
> 
> -- 
> I'm not entirely sure you are real.
> 



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

2020-10-20 Thread Sebastian Benoit
i had observed this before with
for i in `jot 5`;do ping6 -c 1 2001:4860:4860:: 1>/dev/null 

diff fixes the problem and looks correct to me.

ok benno@


Florian Obser(flor...@openbsd.org) on 2020.10.20 18:39:58 +0200:
> On Tue, Oct 20, 2020 at 04:07:41PM +0200, Martijn van Duren wrote:
> > When running:
> > /usr/local/libexec/nagios/check_ping -vv -6 -H  -c 500,75% -w 250,50%
> > in a tight loop at some point I get the following output:
> > 
> > CMD: /sbin/ping6 -n -c 5 
> > Output: PING  (): 56 data bytes
> > Output: 64 bytes from : icmp_seq=0 hlim=57 time=1.724 ms
> > Output: 64 bytes from : icmp_seq=1 hlim=57 time=1.612 ms
> > Output: 64 bytes from : icmp_seq=2 hlim=57 time=1.785 ms
> > Output: 64 bytes from : icmp_seq=3 hlim=57 time=1.762 ms
> > Output: 64 bytes from : icmp_seq=4 hlim=57 time=1.708 ms
> > Output:
> > Output: ---  ping statistics ---
> > Output: 5 packets transmitted, 5 packets received, 0.0% packet loss
> > Output: round-trip min/avg/max/std-dev = 1.612/1.718/1.785/0.060 ms
> > Got stderr: ping6: failed to get receiving hop limit
> > PING WARNING - System call sent warnings to stderr Packet loss = 0%, RTA = 
> > 1.72 ms|rta=1.718000ms;3000.00;5000.00;0.00 pl=0%;80;100;0
> > 3000.00:80% 5000.00:100%
> > 
> > So I have 5 echo requests, 5 echo replies and one unrelated packet not
> > related to my ping command. Yet this notice to stderr forces check_ping
> > into a warning state.
> > 
> 
> Oh, got it know. The problem is packets arriving between socket(2) and
> whenever we are finished calling all setsockopt(2)s. In this case
> IPV6_RECVHOPLIMIT.
> 
> We need to drain the socket after we are fully setup.
> 
> Can you give this a spin? OK?
> 
> diff --git ping.c ping.c
> index d198d233921..acfe2ed8e18 100644
> --- ping.c
> +++ ping.c
> @@ -240,6 +240,17 @@ void  pr_retip6(struct ip6_hdr *, 
> u_char *);
>  int
>  main(int argc, char *argv[])
>  {
> + struct msghdr   m;
> + union {
> + struct cmsghdr hdr;
> + u_char buf[CMSG_SPACE(1024)];
> + }   cmsgbuf;
> + struct ioveciov[1];
> + struct pollfd   pfd;
> + struct sockaddr_in  peer4;
> + struct sockaddr_in6 peer6;
> + ssize_t cc;
> +
>   struct addrinfo hints, *res;
>   struct itimerval itimer;
>   struct sockaddr *from, *dst;
> @@ -786,6 +797,44 @@ main(int argc, char *argv[])
>   smsghdr.msg_iov = 
>   smsghdr.msg_iovlen = 1;
>  
> + if (v6flag) {
> + m.msg_name = 
> + m.msg_namelen = sizeof(peer6);
> + } else {
> + m.msg_name = 
> + m.msg_namelen = sizeof(peer4);
> + }
> + memset(, 0, sizeof(iov));
> + iov[0].iov_base = (caddr_t)packet;
> + iov[0].iov_len = packlen;
> +
> + /* Drain our socket. */
> + (void)signal(SIGALRM, onsignal);
> + memset(, 0, sizeof(itimer));
> + itimer.it_value.tv_sec = 1; /* make sure we don't get stuck */
> + (void)setitimer(ITIMER_REAL, , NULL);
> + for (;;) {
> + if (seenalrm)
> + break;
> +
> + pfd.fd = s;
> + pfd.events = POLLIN;
> +
> + if (poll(, 1, 0) <= 0)
> + break;
> +
> + m.msg_iov = iov;
> + m.msg_iovlen = 1;
> + m.msg_control = (caddr_t)
> + m.msg_controllen = sizeof(cmsgbuf.buf);
> +
> + cc = recvmsg(s, , 0);
> + if (cc == -1 && errno != EINTR)
> + break;
> + }
> + memset(, 0, sizeof(itimer));
> + (void)setitimer(ITIMER_REAL, , NULL);
> +
>   while (preload--)   /* Fire off them quickies. */
>   pinger(s);
>  
> @@ -805,17 +854,7 @@ main(int argc, char *argv[])
>   seeninfo = 0;
>  
>   for (;;) {
> - struct msghdr   m;
> - union {
> - struct cmsghdr hdr;
> - u_char buf[CMSG_SPACE(1024)];
> - }   cmsgbuf;
> - struct ioveciov[1];
> - struct pollfd   pfd;
> - struct sockaddr_in  peer4;
> - struct sockaddr_in6 peer6;
> - ssize_t cc;
> - int timeout;
> + int timeout;
>  
>   /* signal handling */
>   if (seenint)
> @@ -864,16 +903,6 @@ main(int argc, char *argv[])
>   if (poll(, 1, timeout) <= 0)
>   continue;
>  
> - if (v6flag) {
> - m.msg_name = 
> - m.msg_namelen = sizeof(peer6);
> - } else {
> - m.msg_name = 
> - m.msg_namelen = sizeof(peer4);
> - }
> - memset(, 0, sizeof(iov));
> - iov[0].iov_base = (caddr_t)packet;
> - 

rpki-client 6.8p0 released

2020-10-20 Thread Sebastian Benoit
rpki-client 6.8p0 has just been released and will be available in the
rpki-client directory of any OpenBSD mirror soon.

rpki-client is a FREE, easy-to-use implementation of the Resource
Public Key Infrastructure (RPKI) for Relying Parties (RP) to
facilitate validation of the Route Origin of a BGP announcement. The
program queries the RPKI repository system and outputs Validated ROA
Payloads in the configuration format of OpenBGPD, BIRD, and also as
CSV or JSON objects for consumption by other routing stacks.

See RFC 6811 for a description of how BGP Prefix Origin Validation
secures the Internet's global routing system.

rpki-client was primarily developed by Kristaps Dzonsons, Claudio
Jeker, Job Snijders, and Sebastian Benoit as part of the OpenBSD
Project and gets released as a base component of OpenBSD every six
months, and follows the OpenBSD release numbering scheme.

This is the first release based on OpenBSD 6.8. It includes the following
changes to the previous release:

* Improve how repositories are downloaded: do not fetch symlinks and
  clean extraneous files in the repositories after download using the
  cryptographically signed RPKI manifest listings.

* Fix a bug where rpki-client could hang after calling rsync.

* Remove the -f option, no longer needed.

* Improved validation of the trust anchors.

* Add new option '-s timeout' to make rpki-client automatically
  terminate after a timeout (default 1 hour). This helps when
  rpki-client is run via cron to prevent a hanging process to cause
  problems.

Portability improvements:

* Replace warnc() with warnx() + strerror()

* Replace b64_pton() with code using the libcrypto EVP_Decode*
  functionality.

* Adjust for OpenSSL 1.1.x compatible use of the EVP_ENCODE_CTX
  struct.

rpki-client is known to compile and run on at least the following
Linux distributions: Alpine 3.12, Debian 9, Debian 10, Fedora 31,
Fedora 32, Fedora 33, RHEL/CentOS 7, RHEL/CentOS 8.
It is our hope that packagers take interest and help adapt
OpenBGPD-portable to more distributions.

The mirrors where rpki-client can be found are on
https://www.rpki-client.org/portable.html

Reporting Bugs:
===

General bugs may be reported to tech@openbsd.org

Portable bugs may be filed at 
https://github.com/rpki-client/rpki-client-portable

We welcome feedback and improvements from the broader community.
Thanks to all of the contributors who helped make this release
possible.



Re: acme-client: relax certificate parsing

2020-09-15 Thread Sebastian Benoit
ok

Florian Obser(flor...@openbsd.org) on 2020.09.14 17:12:01 +0200:
> Relax parsing of pem files a bit. Apparently there are CAs that use
> \r\n line endings.
> From Bartosz Kuzma as part of a larger diff.
> 
> OK?
> 
> diff --git certproc.c certproc.c
> index 7fde96e970e..975e12afaaa 100644
> --- certproc.c
> +++ certproc.c
> @@ -28,7 +28,8 @@
>  
>  #include "extern.h"
>  
> -#define MARKER "-END CERTIFICATE-\n"
> +#define BEGIN_MARKER "-BEGIN CERTIFICATE-"
> +#define END_MARKER "-END CERTIFICATE-"
>  
>  int
>  certproc(int netsock, int filesock)
> @@ -81,19 +82,25 @@ certproc(int netsock, int filesock)
>   if ((csr = readbuf(netsock, COMM_CSR, )) == NULL)
>   goto out;
>  
> - if (csrsz < strlen(MARKER)) {
> + if (csrsz < strlen(END_MARKER)) {
>   warnx("invalid cert");
>   goto out;
>   }
>  
> - chaincp = strstr(csr, MARKER);
> + chaincp = strstr(csr, END_MARKER);
>  
>   if (chaincp == NULL) {
>   warnx("invalid cert");
>   goto out;
>   }
>  
> - chaincp += strlen(MARKER);
> + chaincp += strlen(END_MARKER);
> +
> + if ((chaincp = strstr(chaincp, BEGIN_MARKER)) == NULL) {
> + warnx("invalid certificate chain");
> + goto out;
> + }
> +
>   if ((chain = strdup(chaincp)) == NULL) {
>   warn("strdup");
>   goto out;
> 
> 
> -- 
> I'm not entirely sure you are real.
> 



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

2020-09-15 Thread Sebastian Benoit
ok!

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

Re: [Patch] Change httpd's handling of request "Host:" headers

2020-08-10 Thread Sebastian Benoit
Ross L Richardson(open...@rlr.id.au) on 2020.08.09 20:07:11 +1000:
> 
> At present, if a request contains no "Host:" header [HTTP pre-1.1] or
> if the supplied header does not match any of the servers configured
> in httpd.conf, the request is directed to the first server.  This
> isn't documented, AFAICT.
> 
> For example, if httpd.conf has just one server
>   server "www.example.com"
> then we currently get
>   $ printf "HEAD / HTTP/1.0\r\nHost: www.openbsd.org\r\n\r\n" \
>   | nc www.example.com www | sed 1q 
>   HTTP/1.0 200 OK
> 
> This behaviour strikes me as wrong (or at least sub-optimal) in the
> case of non-matching "Host:" headers.  The simplistic patch below
> changes things to return a 404 status if no matching server is found.
> 
> [If status code 400 (bad request) is preferred, "goto fail;"
> could be used.]
> 
> Justification:
> - This seems more correct, and is consistent with the "fail closed"
>   approach.

In which way can the current behaviour cause problems?

I dont think we should treat Host: headers as secrets, so there is no
information leakage or such a thing.

The downside of changing this is possible breakage in existing configs,
that should be avoided.

> - There is a net gain in functionality, as use of glob/patterns
>   wildcards can easily re-establish the current behaviour.  In
>   contrast, there's no way at present to disable the implicit
>   match-anything behaviour.

As jca@ shows the first host can be a dummy.

I kind of think that this is a documentation problem, we should docuemnt
this in the manpage and maybe example config:

diff --git etc/examples/httpd.conf etc/examples/httpd.conf
index fee8d607e90..67eb075eb3e 100644
--- etc/examples/httpd.conf
+++ etc/examples/httpd.conf
@@ -1,5 +1,11 @@
 # $OpenBSD: httpd.conf,v 1.20 2018/06/13 15:08:24 reyk Exp $
 
+# define a default server, to produce 404 responses for unknown hosts.
+server "default" {
+   listen on * port 80
+   root "/nonexistant"
+}
+
 server "example.com" {
listen on * port 80
location "/.well-known/acme-challenge/*" {
diff --git usr.sbin/httpd/httpd.conf.5 usr.sbin/httpd/httpd.conf.5
index 02b4442693b..45780cab78b 100644
--- usr.sbin/httpd/httpd.conf.5
+++ usr.sbin/httpd/httpd.conf.5
@@ -660,6 +660,12 @@ It is possible to set
 to default to use the httpd default timeout of 2 hours.
 .El
 .El
+.Pp
+The first
+.Ic server
+section defines an implicit default for all requests that are not served by 
other
+.Ic server
+declarations.
 .Sh TYPES
 Configure the supported media types.
 .Xr httpd 8



Re: PATCH: iostat spacing

2020-08-09 Thread Sebastian Benoit
Klemens Nanni(k...@openbsd.org) on 2020.08.08 04:12:31 +0200:
> On Fri, Aug 07, 2020 at 12:04:59PM -0700, jo...@armadilloaerospace.com wrote:
> > IO rates above 100 MB/s are common with SSD; this patch expands the
> > column so it stays neatly printed.
> This is OK with me as it fixes the default view, but I think other views

ok benno@

> need fixing as well, e.g.
> 
>   $ iostat -I
>   ttysd0 sd1
> cpu
>  tin tout  KB/t   xfr MB   KB/t   xfr MB  us ni sy sp in 
> id
>   178524 22139320 26.99 10698431 281987.61  11.78 7582117 87218.82   7  0 
>  3  1  0 88
> 
> I'm testing with `dd if=/dev/rsd0c of=/dev/null bs=1m' on a X230 with
> sd0 at scsibus1 targ 0 lun 0:  
> naa.5002538d410a7bce
> 
> > An argument can be made for expanding it one more for fast M.2 drives.
> I don't have that fast disks at hand, but expanding it by another column
> makes sense to me and could further simplify some of the code.
> 



Re: [Patch] Remove unused functions in httpd's proc(?)

2020-08-03 Thread Sebastian Benoit
Ross L Richardson(open...@rlr.id.au) on 2020.08.03 15:25:05 +1000:
> cppcheck reports that proc_iev and proc_ispeer are unused.
> 
> Unless they are wanted for consistency with other versions of proc.c,
> tbey can be removed.

Yes, proc.c should stay the same across 

./sbin/iked/proc.c
./usr.sbin/httpd/proc.c
./usr.sbin/relayd/proc.c
./usr.sbin/snmpd/proc.c
./usr.sbin/switchd/proc.c
./usr.sbin/vmd/proc.c

and i would have taken care of doing that with your other diff touching it.

These functions are used elsewhere, so lets keep them.

/Benno

> 
> Ross
> 
> 
> Index: httpd.h
> ===
> RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
> retrieving revision 1.148
> diff -u -p -r1.148 httpd.h
> --- httpd.h   30 Jul 2020 21:06:19 -  1.148
> +++ httpd.h   3 Aug 2020 05:21:39 -
> @@ -803,8 +803,6 @@ intproc_forward_imsg(struct privsep *,
>   enum privsep_procid, int);
>  struct imsgbuf *
>proc_ibuf(struct privsep *, enum privsep_procid, int);
> -struct imsgev *
> -  proc_iev(struct privsep *, enum privsep_procid, int);
>  int   proc_flush_imsg(struct privsep *, enum privsep_procid, int);
>  void  imsg_event_add(struct imsgev *);
>  int   imsg_compose_event(struct imsgev *, uint16_t, uint32_t,
> Index: proc.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/proc.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 proc.c
> --- proc.c9 Sep 2018 21:06:51 -   1.38
> +++ proc.c3 Aug 2020 05:21:39 -
> @@ -43,24 +43,11 @@ void   proc_open(struct privsep *, int, i
>  void  proc_accept(struct privsep *, int, enum privsep_procid,
>   unsigned int);
>  void  proc_close(struct privsep *);
> -int   proc_ispeer(struct privsep_proc *, unsigned int, enum privsep_procid);
>  void  proc_shutdown(struct privsep_proc *);
>  void  proc_sig_handler(int, short, void *);
>  void  proc_range(struct privsep *, enum privsep_procid, int *, int *);
>  int   proc_dispatch_null(int, struct privsep_proc *, struct imsg *);
>  
> -int
> -proc_ispeer(struct privsep_proc *procs, unsigned int nproc,
> -enum privsep_procid type)
> -{
> - unsigned inti;
> -
> - for (i = 0; i < nproc; i++)
> - if (procs[i].p_id == type)
> - return (1);
> - return (0);
> -}
> -
>  enum privsep_procid
>  proc_getid(struct privsep_proc *procs, unsigned int nproc,
>  const char *proc_name)
> @@ -819,15 +806,6 @@ proc_ibuf(struct privsep *ps, enum privs
>  
>   proc_range(ps, id, , );
>   return (>ps_ievs[id][n].ibuf);
> -}
> -
> -struct imsgev *
> -proc_iev(struct privsep *ps, enum privsep_procid id, int n)
> -{
> - int  m;
> -
> - proc_range(ps, id, , );
> - return (>ps_ievs[id][n]);
>  }
>  
>  /* This function should only be called with care as it breaks async I/O */
> 



Re: [Patch] Remove unused functions in httpd[.ch]

2020-08-03 Thread Sebastian Benoit


ok

Ross L Richardson(open...@rlr.id.au) on 2020.08.03 15:37:50 +1000:
> cppcheck reports that kv_inherit(), kv_log(), and print_time() are unused.
> 
> The patch below deletes them.
> 
> Ross
> --
> 
> Index: httpd.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/httpd.c,v
> retrieving revision 1.69
> diff -u -p -r1.69 httpd.c
> --- httpd.c   30 Jul 2020 21:06:19 -  1.69
> +++ httpd.c   3 Aug 2020 05:35:09 -
> @@ -1059,50 +1059,6 @@ kv_free(struct kv *kv)
>  }
>  
>  struct kv *
> -kv_inherit(struct kv *dst, struct kv *src)
> -{
> - memset(dst, 0, sizeof(*dst));
> - memcpy(dst, src, sizeof(*dst));
> - TAILQ_INIT(>kv_children);
> -
> - if (src->kv_key != NULL) {
> - if ((dst->kv_key = strdup(src->kv_key)) == NULL) {
> - kv_free(dst);
> - return (NULL);
> - }
> - }
> - if (src->kv_value != NULL) {
> - if ((dst->kv_value = strdup(src->kv_value)) == NULL) {
> - kv_free(dst);
> - return (NULL);
> - }
> - }
> -
> - return (dst);
> -}
> -
> -int
> -kv_log(struct evbuffer *log, struct kv *kv)
> -{
> - char*msg;
> -
> - if (log == NULL)
> - return (0);
> - if (asprintf(, " [%s%s%s]",
> - kv->kv_key == NULL ? "(unknown)" : kv->kv_key,
> - kv->kv_value == NULL ? "" : ": ",
> - kv->kv_value == NULL ? "" : kv->kv_value) == -1)
> - return (-1);
> - if (evbuffer_add(log, msg, strlen(msg)) == -1) {
> - free(msg);
> - return (-1);
> - }
> - free(msg);
> -
> - return (0);
> -}
> -
> -struct kv *
>  kv_find(struct kvtree *keys, struct kv *kv)
>  {
>   struct kv   *match;
> @@ -1270,22 +1226,6 @@ print_host(struct sockaddr_storage *ss, 
>   buf[0] = '\0';
>   return (NULL);
>   }
> - return (buf);
> -}
> -
> -const char *
> -print_time(struct timeval *a, struct timeval *b, char *buf, size_t len)
> -{
> - struct timeval  tv;
> - unsigned long   h, sec, min;
> -
> - timerclear();
> - timersub(a, b, );
> - sec = tv.tv_sec % 60;
> - min = tv.tv_sec / 60 % 60;
> - h = tv.tv_sec / 60 / 60;
> -
> - snprintf(buf, len, "%.2lu:%.2lu:%.2lu", h, min, sec);
>   return (buf);
>  }
>  
> Index: httpd.h
> ===
> RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
> retrieving revision 1.148
> diff -u -p -r1.148 httpd.h
> --- httpd.h   30 Jul 2020 21:06:19 -  1.148
> +++ httpd.h   3 Aug 2020 05:35:09 -
> @@ -731,8 +731,6 @@ void   kv_delete(struct kvtree *, struct
>  struct kv*kv_extend(struct kvtree *, struct kv *, char *);
>  void  kv_purge(struct kvtree *);
>  void  kv_free(struct kv *);
> -struct kv*kv_inherit(struct kv *, struct kv *);
> -int   kv_log(struct evbuffer *, struct kv *);
>  struct kv*kv_find(struct kvtree *, struct kv *);
>  int   kv_cmp(struct kv *, struct kv *);
>  struct media_type
> @@ -751,7 +749,6 @@ struct auth   *auth_add(struct serverauth 
>  struct auth  *auth_byid(struct serverauth *, uint32_t);
>  void  auth_free(struct serverauth *, struct auth *);
>  const char   *print_host(struct sockaddr_storage *, char *, size_t);
> -const char   *print_time(struct timeval *, struct timeval *, char *, size_t);
>  const char   *printb_flags(const uint32_t, const char *);
>  void  getmonotime(struct timeval *);
>  
> 



Re: [Patch] Remove redundant condition in httpd's server_http.c

2020-08-03 Thread Sebastian Benoit
ok

Ross L Richardson(open...@rlr.id.au) on 2020.08.03 15:06:31 +1000:
> cppcheck reports this [and other less simple things].
> 
> Ross
> 
> Index: server_http.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
> retrieving revision 1.139
> diff -u -p -r1.139 server_http.c
> --- server_http.c 22 May 2020 07:18:17 -  1.139
> +++ server_http.c 3 Aug 2020 05:01:26 -
> @@ -1273,8 +1273,7 @@ server_response(struct httpd *httpd, str
>   hostname, FNM_CASEFOLD);
>   }
>   if (ret == 0 &&
> - (portval == -1 ||
> - (portval != -1 && portval == srv_conf->port))) {
> + (portval == -1 || portval == srv_conf->port)) {
>   /* Replace host configuration */
>   clt->clt_srv_conf = srv_conf;
>   srv_conf = NULL;
> 



Re: [Patch] Remove unnecessary assignment in httpd's server_fcgi.c

2020-08-02 Thread Sebastian Benoit
ok

Ross L Richardson(open...@rlr.id.au) on 2020.08.02 21:52:50 +1000:
> cppcheck finds an unnecessary assignment.
> 
> The patch below deletes it.
> 
> Ross
> 
> Index: server_fcgi.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
> retrieving revision 1.81
> diff -u -p -r1.81 server_fcgi.c
> --- server_fcgi.c 9 Feb 2020 09:44:04 -   1.81
> +++ server_fcgi.c 2 Aug 2020 11:49:44 -
> @@ -684,7 +684,7 @@ server_fcgi_header(struct client *clt, u
>  
>   /* Date header is mandatory and should be added as late as possible */
>   key.kv_key = "Date";
> - if ((kv = kv_find(>http_headers, )) == NULL &&
> + if (kv_find(>http_headers, ) == NULL &&
>   (server_http_time(time(NULL), tmbuf, sizeof(tmbuf)) <= 0 ||
>   kv_add(>http_headers, "Date", tmbuf) == NULL))
>   return (-1);
> 



Re: [Patch] Delete redundant condition in httpd's proc.c

2020-08-02 Thread Sebastian Benoit
ok

relayd has the same btw.

Ross L Richardson(open...@rlr.id.au) on 2020.08.02 21:41:33 +1000:
> cppcheck finds a redundant condition.
> 
> The patch below deletes it.
> 
> Ross
> 
> Index: proc.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/proc.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 proc.c
> --- proc.c9 Sep 2018 21:06:51 -   1.38
> +++ proc.c2 Aug 2020 11:38:50 -
> @@ -401,7 +401,7 @@ proc_kill(struct privsep *ps)
>   free(cause);
>   } else
>   log_warnx("lost child: pid %u", pid);
> - } while (pid != -1 || (pid == -1 && errno == EINTR));
> + } while (pid != -1 || errno == EINTR);
>  }
>  
>  void
> 



Re: [Patch] Delete unread assignments in httpd's config.c

2020-08-02 Thread Sebastian Benoit
ok

Ross L Richardson(open...@rlr.id.au) on 2020.08.02 21:32:44 +1000:
> cppcheck finds some unread assignments in httpd's config.c
> 
> The patch below deletes them and the resulting unused variables.  By way of
> [a bit more] context, the last of these is:
> 
>682if (srv->srv_conf.return_uri_len != 0) {
>683if ((srv->srv_conf.return_uri = get_data(p + s,
>684srv->srv_conf.return_uri_len)) == NULL)
>685goto fail;
>686s += srv->srv_conf.return_uri_len;
>687}
>688
>689return (0);
>690
>691 fail:
> 
> Ross
> 
> ===
> RCS file: /cvs/src/usr.sbin/httpd/config.c,v
> retrieving revision 1.57
> diff -u -p -r1.57 config.c
> --- config.c  8 May 2019 19:57:45 -   1.57
> +++ config.c  2 Aug 2020 11:12:28 -
> @@ -135,9 +135,7 @@ config_getreset(struct httpd *env, struc
>  int
>  config_getcfg(struct httpd *env, struct imsg *imsg)
>  {
> - struct privsep  *ps = env->sc_ps;
>   struct ctl_flags cf;
> - unsigned int what;
>  
>   if (IMSG_DATA_SIZE(imsg) != sizeof(cf))
>   return (0); /* ignore */
> @@ -148,8 +146,6 @@ config_getcfg(struct httpd *env, struct 
>   env->sc_flags = cf.cf_flags;
>   memcpy(env->sc_tls_sid, cf.cf_tls_sid, sizeof(env->sc_tls_sid));
>  
> - what = ps->ps_what[privsep_process];
> -
>   if (privsep_process != PROC_PARENT)
>   proc_compose(env->sc_ps, PROC_PARENT,
>   IMSG_CFG_DONE, NULL, 0);
> @@ -683,7 +679,6 @@ config_getserver(struct httpd *env, stru
>   if ((srv->srv_conf.return_uri = get_data(p + s,
>   srv->srv_conf.return_uri_len)) == NULL)
>   goto fail;
> - s += srv->srv_conf.return_uri_len;
>   }
>  
>   return (0);
> 



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

2020-07-29 Thread Sebastian Benoit
Jeremie Courreges-Anglas(j...@wxcvbn.org) on 2020.07.29 15:51:55 +0200:
> On Sun, Jul 26 2020, Jeremie Courreges-Anglas  wrote:
> > On Sat, Jul 25 2020, Sebastian Benoit  wrote:
> 
> [...]
> 
> >> If you enable trust-ad on a system that moves around, e.g. your laptop, you
> >> will experience failures, which is why unwind tests for this and falls back
> >> to non-trusting dhcp learned resolvers in such cases. In fact it also falls
> >> back to the stub resolver, i.e. our libc resolver. Which makes me think 
> >> that
> >> trust-ad should not be used when you run unwind (because the whole point of
> >> the fallback is that dnssec does not work). But thats a documentation 
> >> issue.
> 
> One important thing to keep in mind: trust-ad is not so much about being
> able to do DNSSEC validation.  It's about trusting the validation done
> by the resolver(s).  So if the resolver can't do validation and ensures
> that AD flag is clear in the reply there is no breach of contract.
> 
> > Thanks for pointing this out.  I would expect from unwind(8) that it
> > always clears the AD bit from its responses if it could not validate
> > them.  Inclusing when it falls back to dynamically learned resolvers or
> > the libc resolver.
> 
> So I did a few tests and read some unwind(8) code, and it *appears* safe
> to use unwind(8) along with "options trust-ad".
> 
> First you mention fallback to DHCP-learned resolvers.  Those you should
> probably not trust indeed, but it looks like unwind(8) attempts to use
> them to perform its own validation.  So the value of the AD flag in
> unwind(8)'s response should be trustworthy.  At least that's what I see
> when I test unwind with "preference { dhcp }".
> 
> And then there's the asr fallback, tested with "preference { stub }".
> unwind(8) allocates its own asr context, ignoring the global
> /etc/resolv.conf and thus "options trust-ad".  IIUC it then hands off
> the answer to libunbound which cooks its own soup.  So it looks like
> unwind needs no special code to disable/ignore "options trust-ad".

you are right, i forgot about that.
 
> I have no idea how unwind/libunbound behaves when using forwarders, the
> "accept bogus" feature, (opportunistic) DoT or other stuff I'm probably
> overlooking.  But AFAICS using unwind + "options trust-ad" brings no
> problem.  cc'ing Florian, input welcome.
> 
> I think the issue with trust-ad is the list of resolvers that end up in
> resolv.conf.  I would expect people who use a resolver on localhost to
> also have other resolvers listed in their /etc/resolv.conf, because of
> a conscious choice (bsd.rd) or just because it should be more reliable.
> It seems to me that the resolv.conf(5) manpage addition should cover
> that.  Or should I make it more explicit?

I think its a good start.

Fwiw i dont have other resolvers except localhost in resolv.conf, on laptops
(all running unwind) as well as my gateway (running unbound). (Server type
systems are a different thing of course). If the local resolver does not
work, i want to notice it right away.

> 
> Index: share/man/man5/resolv.conf.5
> ===
> RCS file: /d/cvs/src/share/man/man5/resolv.conf.5,v
> retrieving revision 1.60
> diff -u -p -r1.60 resolv.conf.5
> --- share/man/man5/resolv.conf.5  25 Apr 2020 14:22:04 -  1.60
> +++ share/man/man5/resolv.conf.5  25 Jul 2020 02:08:17 -
> @@ -285,6 +285,15 @@ first as an absolute name before any sea
>  .It Cm tcp
>  Forces the use of TCP for queries.
>  Normal behaviour is to query via UDP but fall back to TCP on failure.
> +.It Cm trust-ad
> +Sets the AD flag in DNS queries, and preserve the value of the AD flag
> +in DNS replies (this flag is stripped by default).
> +Useful when applications want to ensure that the DNS resources they
> +look up have been signed with DNSSEC and validated by the
> +.Ic nameserver .
> +This option should be used only if the transport between the
> +.Ic nameserver
> +and the resolver is secure.
>  .El
>  .El
>  .Pp
> 
> Lazy me would be tempted to just let asr trust 127.0.0.1 / ::1 by
> default and let others deal with the edge cases, but where is the fun in
> that... :)
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
> 



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

2020-07-29 Thread Sebastian Benoit
Jeremie Courreges-Anglas(j...@wxcvbn.org) on 2020.07.26 22:52:47 +0200:
> On Sat, Jul 25 2020, Sebastian Benoit  wrote:
> > Jeremie Courreges-Anglas(j...@wxcvbn.org) on 2020.07.25 14:01:06 +0200:
> >> On Fri, Jul 17 2020, Jesper Wallin  wrote:
> >> > Hi all,
> >> >
> >> > I recently decided to add SSHFP records for my servers, since I never
> >> > memorize or write down my key fingerprints.  I learned that if I
> >> > want ssh(1) to trust these records, DNSSEC needs to be enabled for my
> >> > zone.  To validate these records, ssh(1) is using getrrsetbyname(3),
> >> > which checks if the AD (Authentic Data) flag is set in the response.
> >> >
> >> > To get a response with the AD flag set, the request itself also needs
> >> > to have the AD flag set.  It turns out that getrrsetbyname(3) doesn't
> >> > set this and will therefore never get a validated response, unless the
> >> > resolver is configured to always send it, no matter what the client
> >> > requested.
> >> >
> >> > It seems like unwind(8) behaves this way but it also responds with the
> >> > RRSIG records, which is extra overhead and ignored by getrrsetbyname(3).
> >> >
> >> > This was mentioned a few years ago [0] and the solution suggested was
> >> > to add the RES_USE_DNSSEC to _res.options, which also makes the resolver
> >> > respond with the extra RRSIG records.
> >> >
> >> > Instead, by only setting the AD flag, both the request and the response
> >> > has the same size as without the flag set.  The patch below will add
> >> > RES_USE_AD as an option to _res.options and set it by default.
> >> > This is also the default behaviour in dig(1), which I understand is a
> >> > bit different, but that sure added some confusion while debugging this.
> >> >
> >> > This let you run unbound(8) or any other validating resolver on your
> >> > local network and getrrsetbyname(3) will trust it.  Do read the CAVEATS
> >> > in the manual of getrrsetbyname(3) though.
> >> >
> >> > As a side note, I noticed that the default value of _res.options was the
> >> > same value as RES_DEFAULT, so I changed it to RES_DEFAULT instead, for
> >> > the sake of consistency.
> >> >
> >> > Thoughts?
> >> 
> >> Thanks for addressing this longstanding issue.
> >> 
> >> I think using the AD bit in queries is a good idea. IIRC Peter J.
> >> Philipp (cc'd) suggested using it but I was not thrilled because:
> >> 
> >> 1. the meaning of the AD bit in queries is relatively recent (2013
> >>   I think[0])
> >> 2. getrrsetbyname also collects signature records, and for this you need
> >>   EDNS + the DO bit set.  Implementing this in was not 100% trivial,
> >>   I think we had something working but Eric or I were not 100% happy
> >>   with it.
> >> 
> >> 1. is probably not a concern, after all you're supposed to use
> >> a trustworthy resolver, which should be modern and understand the
> >> purpose of the AD bit in queries.
> >> 
> >> 2. is probably not a concern either.  I guess that all getrrsetbyname(3)
> >> callers only care about the target records and the AD bit, not about the
> >> signature records.  (Why would they use it for anyway?).  In the base
> >> system, only ssh(1) and traceroute(8) use getrrsetbyname(3).
> >> AFAIK no other system provides getrrsetbyname(3), and ISC has removed
> >> getrrsetbyname and the whole lwres API in 2017[1].  So I'd say we're
> >> free to improve our version of getrrsetbyname(3) as we see fit.
> >
> > This is a concern for the stub resolver, because edns and AD does not work
> > everywhere.
> > Indeed when we switched unbound to validate by default we
> > learned that this is not a good idea for everyone - which lead to the
> > development of unwind(8).
> 
> Do you by chance have any data regarding fallout because of the AD bit
> set in queries?  I would expect it to be ignored when not supported.
> EDNS and the DNSSEC DO bit is a different story indeed.

If i remember correctly, the fallout was caused by EDNS but i might be
wrong. The unbound commit caused a developer some headscratching, because
his upstream internet did not work with such packets, which led to immediate
backout of the change, because a default config that does not work is not
good.

In that regard i worry about the resolv.conf option: it has the potential to
break peoples DNS in non obvious ways. At least dont advertise it as a way
to "enable dnssec" for the whole system, instead point people to unwind.



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

2020-07-25 Thread Sebastian Benoit
Jeremie Courreges-Anglas(j...@wxcvbn.org) on 2020.07.25 14:01:06 +0200:
> On Fri, Jul 17 2020, Jesper Wallin  wrote:
> > Hi all,
> >
> > I recently decided to add SSHFP records for my servers, since I never
> > memorize or write down my key fingerprints.  I learned that if I
> > want ssh(1) to trust these records, DNSSEC needs to be enabled for my
> > zone.  To validate these records, ssh(1) is using getrrsetbyname(3),
> > which checks if the AD (Authentic Data) flag is set in the response.
> >
> > To get a response with the AD flag set, the request itself also needs
> > to have the AD flag set.  It turns out that getrrsetbyname(3) doesn't
> > set this and will therefore never get a validated response, unless the
> > resolver is configured to always send it, no matter what the client
> > requested.
> >
> > It seems like unwind(8) behaves this way but it also responds with the
> > RRSIG records, which is extra overhead and ignored by getrrsetbyname(3).
> >
> > This was mentioned a few years ago [0] and the solution suggested was
> > to add the RES_USE_DNSSEC to _res.options, which also makes the resolver
> > respond with the extra RRSIG records.
> >
> > Instead, by only setting the AD flag, both the request and the response
> > has the same size as without the flag set.  The patch below will add
> > RES_USE_AD as an option to _res.options and set it by default.
> > This is also the default behaviour in dig(1), which I understand is a
> > bit different, but that sure added some confusion while debugging this.
> >
> > This let you run unbound(8) or any other validating resolver on your
> > local network and getrrsetbyname(3) will trust it.  Do read the CAVEATS
> > in the manual of getrrsetbyname(3) though.
> >
> > As a side note, I noticed that the default value of _res.options was the
> > same value as RES_DEFAULT, so I changed it to RES_DEFAULT instead, for
> > the sake of consistency.
> >
> > Thoughts?
> 
> Thanks for addressing this longstanding issue.
> 
> I think using the AD bit in queries is a good idea. IIRC Peter J.
> Philipp (cc'd) suggested using it but I was not thrilled because:
> 
> 1. the meaning of the AD bit in queries is relatively recent (2013
>   I think[0])
> 2. getrrsetbyname also collects signature records, and for this you need
>   EDNS + the DO bit set.  Implementing this in was not 100% trivial,
>   I think we had something working but Eric or I were not 100% happy
>   with it.
> 
> 1. is probably not a concern, after all you're supposed to use
> a trustworthy resolver, which should be modern and understand the
> purpose of the AD bit in queries.
> 
> 2. is probably not a concern either.  I guess that all getrrsetbyname(3)
> callers only care about the target records and the AD bit, not about the
> signature records.  (Why would they use it for anyway?).  In the base
> system, only ssh(1) and traceroute(8) use getrrsetbyname(3).
> AFAIK no other system provides getrrsetbyname(3), and ISC has removed
> getrrsetbyname and the whole lwres API in 2017[1].  So I'd say we're
> free to improve our version of getrrsetbyname(3) as we see fit.

This is a concern for the stub resolver, because edns and AD does not work
everywhere. Indeed when we switched unbound to validate by default we
learned that this is not a good idea for everyone - which lead to the
development of unwind(8).
 
> Now let's discuss the implementation.  There are two main things
> I dislike with your diff:
> - it affects all DNS queries done through libc, even if the user doesn't
>   care at all about DNSSEC.  I suspect there's potential for fallout
>   here, but I have no data to back this.
> - trusting the AD bit by default looks a bit too easy.  The
>   documentation says that people should only trust the AD bit /
>   RRSET_VALIDATED if the communication between libc and the resolver is
>   secure, but who actually reads the documentation?
> 
> For those two reasons I think the feature should be opt-in.  That's what
> Florian Weimer did in glibc, with the implementation of RES_TRUSTAD[2][3].
> The diff below implements the same semantics:
> - by default, don't send queries the AD flag, and don't trust its value
>   in replies
> - if "options trust-ad" is set in resolv.conf(5), set the AD flag in
>   queries and allow applications to look at the AD flag in replies.
> 
> There's one more thing I'd like to check in the res_* API, but I'm
> sitting on this idea since too long already, and I'm happy with my test
> results so far.
> 
> Thoughts?  Comments?

Your observation that this should not be on by default is probably correct.

If you enable trust-ad on a system that moves around, e.g. your laptop, you
will experience failures, which is why unwind tests for this and falls back
to non-trusting dhcp learned resolvers in such cases. In fact it also falls
back to the stub resolver, i.e. our libc resolver. Which makes me think that
trust-ad should not be used when you run unwind (because the whole point of
the fallback is 

Re: acme-client config grammar improvements

2020-07-21 Thread Sebastian Benoit
Daniel Eisele(daniel_eis...@gmx.de) on 2020.07.16 07:40:35 +0200:
> Am 15.07.2020 um 23:51 schrieb Sebastian Benoit:
> >> src/usr.sbin/acme-client/parse.y:
> >> * The grammar allows the user to omit the newline after the first line
> >>   in a domain or authority block.
> >
> > Yes. I'm still pnodering this. What are the chances that someone does that?
> >
> > Probably no one does, but it worthwhile to break someones config for such a
> > change?
> 
> I can't imagine that anyone uses this, I only noticed it by reading the
> grammar. But still not an important fix, I just noticed it and wanted to
> point it out...
> 
> >> src/usr.sbin/acme-client/dbg.c doesn't build because in the included
> >> header file extern.h the type pid_t is missing (unistd.h).
> >
> > extern.h should #include  for that, no?
> 
> Yes, sys/types.h is better, sorry about that.

So here is a patch i would like to commit.

ok?

diff --git usr.sbin/acme-client/dnsproc.c usr.sbin/acme-client/dnsproc.c
index 664ef8d9b8b..72a0ea6b30e 100644
--- usr.sbin/acme-client/dnsproc.c
+++ usr.sbin/acme-client/dnsproc.c
@@ -16,6 +16,7 @@
  */
 
 #include 
+#include 
 #include 
 
 #include 
diff --git usr.sbin/acme-client/extern.h usr.sbin/acme-client/extern.h
index 529d3350205..76c86b5cce0 100644
--- usr.sbin/acme-client/extern.h
+++ usr.sbin/acme-client/extern.h
@@ -17,6 +17,8 @@
 #ifndef EXTERN_H
 #define EXTERN_H
 
+#include 
+
 #include "parse.h"
 
 #define MAX_SERVERS_DNS 8
diff --git usr.sbin/acme-client/parse.y usr.sbin/acme-client/parse.y
index 120f253a63f..0b96794f8da 100644
--- usr.sbin/acme-client/parse.y
+++ usr.sbin/acme-client/parse.y
@@ -170,11 +170,11 @@ varset: STRING '=' string {
}
;
 
-optnl  : '\n' optnl
+optnl  : nl
|
;
 
-nl : '\n' optnl/* one newline or more */
+nl : optnl '\n'/* one newline or more */
;
 
 comma  : ','
@@ -190,7 +190,7 @@ authority   : AUTHORITY STRING {
yyerror("authority already defined");
YYERROR;
}
-   } '{' optnl authorityopts_l '}' {
+   } '{' optnl authorityopts_l optnl '}' {
if (auth->api == NULL) {
yyerror("authority %s: no api URL specified",
auth->name);
@@ -205,8 +205,8 @@ authority   : AUTHORITY STRING {
}
;
 
-authorityopts_l: authorityopts_l authorityoptsl nl
-   | authorityoptsl optnl
+authorityopts_l: authorityopts_l nl authorityoptsl
+   | authorityoptsl
;
 
 authorityoptsl : API URL STRING {
@@ -246,7 +246,7 @@ domain  : DOMAIN STRING {
yyerror("domain already defined");
YYERROR;
}
-   } '{' optnl domainopts_l '}' {
+   } '{' optnl domainopts_l optnl '}' {
if (domain->domain == NULL) {
if ((domain->domain = strdup(domain->handle))
== NULL)
@@ -273,8 +273,8 @@ keytype : RSA   { $$ = KT_RSA; }
|   { $$ = KT_RSA; }
;
 
-domainopts_l   : domainopts_l domainoptsl nl
-   | domainoptsl optnl
+domainopts_l   : domainopts_l nl domainoptsl
+   | domainoptsl
;
 
 domainoptsl: ALTERNATIVE NAMES '{' altname_l '}'
@@ -385,7 +385,7 @@ domainoptsl : ALTERNATIVE NAMES '{' altname_l '}'
}
;
 
-altname_l  : altname comma altname_l
+altname_l  : altname_l comma altname
| altname
;
 



Re: [Patch] httpd.h - delete unused field and enum

2020-07-21 Thread Sebastian Benoit
Ross L Richardson(open...@rlr.id.au) on 2020.07.15 14:49:23 +1000:
> Field kv_type in struct kv is not used.  As that's the only use of
> enum key_type, delete them both.

This is probably because its a copy from relayd.
I dont think there is a reason to keep it.

i'd like to commit, anyone else to ok it?


> 
> Ross
> 
> 
> Index: httpd.h
> ===
> RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
> retrieving revision 1.146
> diff -u -p -r1.146 httpd.h
> --- httpd.h   9 Feb 2020 09:44:04 -   1.146
> +++ httpd.h   15 Jul 2020 04:45:16 -
> @@ -121,24 +121,12 @@ struct ctl_flags {
>   uint8_t  cf_tls_sid[TLS_MAX_SESSION_ID_LENGTH];
>  };
>  
> -enum key_type {
> - KEY_TYPE_NONE   = 0,
> - KEY_TYPE_COOKIE,
> - KEY_TYPE_HEADER,
> - KEY_TYPE_PATH,
> - KEY_TYPE_QUERY,
> - KEY_TYPE_URL,
> - KEY_TYPE_MAX
> -};
> -
>  TAILQ_HEAD(kvlist, kv);
>  RB_HEAD(kvtree, kv);
>  
>  struct kv {
>   char*kv_key;
>   char*kv_value;
> -
> - enum key_typekv_type;
>  
>  #define KV_FLAG_INVALID   0x01
>  #define KV_FLAG_GLOBBING  0x02
> 



Re: acme-client config grammar improvements

2020-07-15 Thread Sebastian Benoit
Daniel Eisele(daniel_eis...@gmx.de) on 2020.07.15 09:32:37 +0200:
> Hi,
> 
> I'm currently porting acme-client to FreeBSD and while doing that I've
> noticed a few small issues.
> 
> src/usr.sbin/acme-client/parse.y:
> * The grammar allows the user to omit the newline after the first line
>   in a domain or authority block.

Yes. I'm still pnodering this. What are the chances that someone does that?

Probably no one does, but it worthwhile to break someones config for such a
change?

> * The grammar uses right recursion for lists, left recursion is
>   probably the better option here
>   (https://docs.oracle.com/cd/E19504-01/802-5880/yacc-13/index.html)
> 
> src/usr.sbin/acme-client/dbg.c doesn't build because in the included
> header file extern.h the type pid_t is missing (unistd.h).

extern.h should #include  for that, no?

> src/usr.sbin/acme-client/dnsproc.c also fails to build because
> struct sockaddr_in and struct sockaddr_in6 are missing (netinet/in.h).

correct.

> The missing header files only cause a build failure on FreeBSD, but I
> think it would still be a good idea to fix them in OpenBSD...
> 
> Regards,
> Daniel Eisele
> 
> --- parse.y.orig  2020-07-10 12:36:30 UTC
> +++ parse.y
> @@ -170,11 +170,11 @@ varset  : STRING '=' string {
>   }
>   ;
> 
> -optnl: '\n' optnl
> +optnl: nl
>   |
>   ;
> 
> -nl   : '\n' optnl/* one newline or more */
> +nl   : optnl '\n'/* one newline or more */
>   ;
> 
>  comma: ','
> @@ -190,7 +190,7 @@ authority : AUTHORITY STRING {
>   yyerror("authority already defined");
>   YYERROR;
>   }
> - } '{' optnl authorityopts_l '}' {
> + } '{' optnl authorityopts_l optnl '}' {
>   if (auth->api == NULL) {
>   yyerror("authority %s: no api URL specified",
>   auth->name);
> @@ -205,8 +205,8 @@ authority : AUTHORITY STRING {
>   }
>   ;
> 
> -authorityopts_l  : authorityopts_l authorityoptsl nl
> - | authorityoptsl optnl
> +authorityopts_l  : authorityopts_l nl authorityoptsl
> + | authorityoptsl
>   ;
> 
>  authorityoptsl   : API URL STRING {
> @@ -246,7 +246,7 @@ domain: DOMAIN STRING {
>   yyerror("domain already defined");
>   YYERROR;
>   }
> - } '{' optnl domainopts_l '}' {
> + } '{' optnl domainopts_l optnl '}' {
>   if (domain->domain == NULL) {
>   if ((domain->domain = strdup(domain->handle))
>   == NULL)
> @@ -273,8 +273,8 @@ keytype   : RSA   { $$ = KT_RSA; }
>   |   { $$ = KT_RSA; }
>   ;
> 
> -domainopts_l : domainopts_l domainoptsl nl
> - | domainoptsl optnl
> +domainopts_l : domainopts_l nl domainoptsl
> + | domainoptsl
>   ;
> 
>  domainoptsl  : ALTERNATIVE NAMES '{' altname_l '}'
> @@ -385,7 +385,7 @@ domainoptsl   : ALTERNATIVE NAMES '{' altname_l '}'
>   }
>   ;
> 
> -altname_l: altname comma altname_l
> +altname_l: altname_l comma altname
>   | altname
>   ;
> 



Re: 11n Tx aggregation for iwm(4)

2020-06-29 Thread Sebastian Benoit
Sebastian Benoit(be...@openbsd.org) on 2020.06.29 16:18:03 +0200:
> Stefan Sperling(s...@stsp.name) on 2020.06.26 14:45:53 +0200:
> > This patch adds support for 11n Tx aggregation to iwm(4).
> > 
> > Please help with testing if you can by running the patch and using wifi
> > as usual. Nothing should change, except that Tx speed may potentially
> > improve. If you have time to run before/after performance measurements with
> > tcpbench or such, that would be nice. But it's not required for testing.
> > 
> > If Tx aggregation is active then netstat will show a non-zero output block 
> > ack
> > agreement counter:
> > 
> > $ netstat -W iwm0 | grep 'output block'
> > 3 new output block ack agreements
> > 0 output block ack agreements timed out
> 
> iwm0 at pci2 dev 0 function 0 "Intel Dual Band Wireless-AC 8265" rev 0x78, msi
> iwm0: hw rev 0x230, fw ver 34.0.1, address 9c:da:3e:6f:51:a4
> 
> With intel firmware updated: intel-firmware-20200520v0->20200609v0: ok

He, i meant to say with "iwm-firmware-20191022p1" as before, but copied the
wrong line and then wrote the correct thing for it :)



Re: 11n Tx aggregation for iwm(4)

2020-06-29 Thread Sebastian Benoit
Stefan Sperling(s...@stsp.name) on 2020.06.26 14:45:53 +0200:
> This patch adds support for 11n Tx aggregation to iwm(4).
> 
> Please help with testing if you can by running the patch and using wifi
> as usual. Nothing should change, except that Tx speed may potentially
> improve. If you have time to run before/after performance measurements with
> tcpbench or such, that would be nice. But it's not required for testing.
> 
> If Tx aggregation is active then netstat will show a non-zero output block ack
> agreement counter:
> 
> $ netstat -W iwm0 | grep 'output block'
> 3 new output block ack agreements
>   0 output block ack agreements timed out

iwm0 at pci2 dev 0 function 0 "Intel Dual Band Wireless-AC 8265" rev 0x78, msi
iwm0: hw rev 0x230, fw ver 34.0.1, address 9c:da:3e:6f:51:a4

With intel firmware updated: intel-firmware-20200520v0->20200609v0: ok

With the patch i get a speedup from ca 25 MBit/s to ca. 35 MBit/s.

However i do not see any 'new output block ack agreements'.

0 new output block ack agreements
0 output block ack agreements timed out

This is with two TP-Link APs (Archer A7 or something like that).

/Benno



Re: unveil(2) relayd(8)'s main proc, now for real

2020-06-19 Thread Sebastian Benoit
Ricardo Mestre(ser...@helheim.mooo.com) on 2020.06.18 23:40:54 +0100:
> Hi,
> 
> Yes, this is a really broad permission to give but it's needed in order to 
> read
> the config file (and those ones included from it) and also to exec the "check
> script(s)" which I missed in my last attempt to unveil(2) relayd(8).
> 
> The reason it cannot be pledge(2)d is due to forbidden ioctls(2)s related to
> carp(4).
> 
> This permits reading or execing anything from the filesystem but at least
> prevents create/write/delete files and regress tests still pass.
> 
> Comments? OK?

ok benno@

> Index: relayd.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
> retrieving revision 1.182
> diff -u -p -u -r1.182 relayd.c
> --- relayd.c  15 Sep 2019 19:23:29 -  1.182
> +++ relayd.c  18 Jun 2020 22:19:50 -
> @@ -223,6 +223,11 @@ main(int argc, char *argv[])
>   if (ps->ps_noaction == 0)
>   log_info("startup");
>  
> + if (unveil("/", "rx") == -1)
> + err(1, "unveil");
> + if (unveil(NULL, NULL) == -1)
> + err(1, "unveil");
> +
>   event_init();
>  
>   signal_set(>ps_evsigint, SIGINT, parent_sig_handler, ps);
> 



Re: netstat -R: list rdomains with associated ifs and tables

2020-06-13 Thread Sebastian Benoit
Remi Locherer(remi.loche...@relo.ch) on 2020.06.11 21:46:45 +0200:
> > This would be clearer if it used table/tables as appropriate e.g.
> > 
> >   Routing table: 0
> >   Routing table: 100
> >   Routing tables: 0 6 7 77
> > 
> > the code to handle this gets messy though, maybe someone can think
> > of alternative wording that gets around this another way..
> > 
> 
> It's not too messy I think.
> 
> twister ..in/netstat$ obj/netstat -R 
> Rdomain 0
>   Interfaces: lo0 iwm0 re0 enc0 pflog0
>   Routing tables: 0 5
> 
> Rdomain 255
>   Interface: lo255
>   Routing table: 255

Of course that makes parsing the output more difficult.
Maybe we can change this in tree when we fins a better solution?

/B.



Re: netstat -R: list rdomains with associated ifs and tables

2020-06-10 Thread Sebastian Benoit
Remi Locherer(remi.loche...@relo.ch) on 2020.06.10 22:16:36 +0200:
> On Tue, Jun 09, 2020 at 10:02:06AM +0200, Remi Locherer wrote:
> > On Tue, Jun 09, 2020 at 09:17:31AM +0200, Claudio Jeker wrote:
> > > On Tue, Jun 09, 2020 at 08:44:42AM +0200, Remi Locherer wrote:
> > > > On Mon, Jun 08, 2020 at 10:10:17PM +0200, Remi Locherer wrote:
> > > > > Hi,
> > > > > 
> > > > > to my knowledge there is no easy way to list all active rdomains or
> > > > > routing tables. Other platforms have "show vrf" or similar commands
> > > > > for an overview.
> > > > > 
> > > > > Here is my attempt at such a view for OpenBSD:
> > > > 
> > > > Updated diff with small changes:
> > > > - Print inet instead of Internet (input deraadt)
> > > > - Removed padding before rdomain id.
> > > > - Changed man page wording.
> > > > 
> > > > twister ..in/netstat$ obj/netstat -R
> > > > Rdomain 0
> > > >   Interfaces: lo0 iwm0 re0 enc0 pflog0 mpe0
> > > >   Routing tables:
> > > >   0: inet   8, inet6  45, mpls   1
> > > >   3: inet   1, inet6   0, mpls   0
> > > >   7: inet  130309, inet6   1, mpls   0
> > > > 
> > > > Rdomain 77
> > > >   Interfaces: vether77 lo77
> > > >   Routing tables:
> > > >  77: inet   0, inet6   0, mpls   0
> > > > 
> > > > Rdomain 122
> > > >   Interfaces: vether122 lo122 pair122 vether1122 vether1123 vether1124 
> > > > vether1125 vether1126 vether1127
> > > >   Routing tables:
> > > > 122: inet  24, inet6   0, mpls   0
> > > > 
> > > > Rdomain 255
> > > >   Interfaces: vether255 lo255
> > > >   Routing tables:
> > > > 255: inet   3, inet6   0, mpls   0
> > > > 
> > > > twister ..in/netstat$
> > > > 
> > > > OK?
> > > 
> > > Why do you think the route counts are needed? You fetch all routing tables
> > > to count them in userland. The sysctl for doing that is expensive and
> > > especially on systems with full tables will make this command slow.
> > > If this is something we really want then the kernel should track and
> > > provide the count.
> > 
> > These counters are of interest for operators. But I agree that counting
> > the routes in userland is unfortunate. But I don't know how bad it is.
> > Is a lock involved in the kernel when dumping the full table?
> 
> I did some homework and figured out, that dumping a routing table takes the
> NET_LOCK. So it's not just inefficient counting all routes in userland it
> might have a negative impact on the system.
> 
> Below my new proposal without the counters. I still think it would be good
> to have those counters. Maybe I'll try to find a solution for that.

Maybe sysctl NET_RT_STATS and struct rtstat could be expanded to cover this?

As for the diff, ok benno@. The number of routes is interesting but can be
added later.

/B.



 
> twister ..in/netstat$ obj/netstat -R 
> Rdomain 0
>   Interfaces: lo0 iwm0 re0 enc0 pflog0
>   Routing tables: 0 6 7 77
> 
> Rdomain 100
>   Interfaces: vether100 lo100 vether101 vether102 vether103 vether104 
> vether105 vether106 vether107 vether108 vether109
>   Routing tables: 100
> 
> Rdomain 255
>   Interfaces: vether255 lo255
>   Routing tables: 255
> 
> twister ..in/netstat$
> 
> 
> 
> Index: main.c
> ===
> RCS file: /cvs/src/usr.bin/netstat/main.c,v
> retrieving revision 1.116
> diff -u -p -r1.116 main.c
> --- main.c28 Apr 2019 17:59:51 -  1.116
> +++ main.c30 May 2020 17:59:33 -
> @@ -127,7 +127,7 @@ main(int argc, char *argv[])
>   tableid = getrtable();
>  
>   while ((ch = getopt(argc, argv,
> - "AaBbc:deFf:ghI:iLlM:mN:np:P:qrsT:tuvW:w:")) != -1)
> + "AaBbc:deFf:ghI:iLlM:mN:np:P:qRrsT:tuvW:w:")) != -1)
>   switch (ch) {
>   case 'A':
>   Aflag = 1;
> @@ -225,6 +225,9 @@ main(int argc, char *argv[])
>   case 'q':
>   qflag = 1;
>   break;
> + case 'R':
> + Rflag = 1;
> + break;
>   case 'r':
>   rflag = 1;
>   break;
> @@ -318,6 +321,11 @@ main(int argc, char *argv[])
>   mroutepr();
>   if (af == AF_INET6 || af == AF_UNSPEC)
>   mroute6pr();
> + exit(0);
> + }
> +
> + if (Rflag) {
> + rdomainpr();
>   exit(0);
>   }
>  
> Index: netstat.1
> ===
> RCS file: /cvs/src/usr.bin/netstat/netstat.1,v
> retrieving revision 1.86
> diff -u -p -r1.86 netstat.1
> --- netstat.1 17 Apr 2019 20:34:21 -  1.86
> +++ netstat.1 8 Jun 2020 20:42:46 -
> @@ -74,6 +74,8 @@
>  .Op Fl i | I Ar interface
>  .Nm
>  .Op Fl W Ar interface
> +.Nm
> +.Op Fl R
>  .Sh DESCRIPTION
>  The
>  .Nm
> @@ -267,6 +269,8 @@ Otherwise the states of the matching soc
>  Only show interfaces that have 

Re: urtwn(4) hardware crypto

2020-06-08 Thread Sebastian Benoit
Jonathan Matthew(jonat...@d14n.org) on 2020.06.05 21:54:30 +1000:
> This enables use of hardware crypto for CCMP in urtwn(4). As with other
> drivers, this reduces cpu usage significantly when moving lots of data.
> I've tested this on an assortment of hardware (RTL8188CUS, RTL8188EU,
> RTL8192EU) with no problems, and this is one of the few things that
> remains constant across a lot of Realtek wifi chips, but some wider
> testing couldn't hurt. Since this touches the code shared with rtwn(4),
> I've also tested that that still works.

Works on 

urtwn0 at uhub2 port 2 configuration 1 interface 0 "Realtek 802.11n NIC" rev 
2.00/0.00 addr 3
urtwn0: MAC/BB RTL8188EU, RF 6052 1T1R, address d4:6e:0e:29:e2:84

usb stick, on armv7.

In a quick test, download of a 10MB large file is faster with your diff, up
from 1MB/s to now 1.6MB/s.

/Benno

> 
> 
> Index: ic/r92creg.h
> ===
> RCS file: /cvs/src/sys/dev/ic/r92creg.h,v
> retrieving revision 1.24
> diff -u -p -r1.24 r92creg.h
> --- ic/r92creg.h  11 Mar 2019 06:19:33 -  1.24
> +++ ic/r92creg.h  5 Jun 2020 11:52:21 -
> @@ -688,6 +688,16 @@
>  #define R92C_CAMCMD_CLR  0x4000
>  #define R92C_CAMCMD_POLLING  0x8000
>  
> +/* Bits for R92C_SECCFG. */
> +#define R92C_SECCFG_TXUCKEY_DEF 0x0001
> +#define R92C_SECCFG_RXUCKEY_DEF  0x0002
> +#define R92C_SECCFG_TXENC_ENA0x0004
> +#define R92C_SECCFG_RXENC_ENA0x0008
> +#define R92C_SECCFG_CMP_A2   0x0010
> +#define R92C_SECCFG_MC_SRCH_DIS  0x0020
> +#define R92C_SECCFG_TXBCKEY_DEF 0x0040
> +#define R92C_SECCFG_RXBCKEY_DEF 0x0080
> +
>  /* IMR */
>   
>  /*Beacon DMA interrupt 6 */
> Index: ic/rtwn.c
> ===
> RCS file: /cvs/src/sys/dev/ic/rtwn.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 rtwn.c
> --- ic/rtwn.c 9 Jan 2020 14:35:19 -   1.49
> +++ ic/rtwn.c 5 Jun 2020 11:52:22 -
> @@ -3154,6 +3154,14 @@ rtwn_init(struct ifnet *ifp)
>   /* Clear per-station keys table. */
>   rtwn_cam_init(sc);
>  
> + /* Enable decryption / encryption. */
> + if (sc->chip & RTWN_CHIP_USB) {
> + rtwn_write_2(sc, R92C_SECCFG,
> + R92C_SECCFG_TXUCKEY_DEF | R92C_SECCFG_RXUCKEY_DEF |
> + R92C_SECCFG_TXENC_ENA | R92C_SECCFG_RXENC_ENA |
> + R92C_SECCFG_TXBCKEY_DEF | R92C_SECCFG_RXBCKEY_DEF);
> + }
> +
>   /* Enable hardware sequence numbering. */
>   rtwn_write_1(sc, R92C_HWSEQ_CTRL, 0xff);
>  
> @@ -3204,14 +3212,14 @@ rtwn_init(struct ifnet *ifp)
>   ifq_clr_oactive(>if_snd);
>   ifp->if_flags |= IFF_RUNNING;
>  
> -#ifdef notyet
> - if (ic->ic_flags & IEEE80211_F_WEPON) {
> + if ((ic->ic_flags & IEEE80211_F_WEPON) &&
> + (sc->chip & RTWN_CHIP_USB)) {
>   /* Install WEP keys. */
>   for (i = 0; i < IEEE80211_WEP_NKID; i++)
>   ic->ic_set_key(ic, NULL, >ic_nw_keys[i]);
>   sc->sc_ops.wait_async(sc->sc_ops.cookie);
>   }
> -#endif
> +
>   if (ic->ic_opmode == IEEE80211_M_MONITOR)
>   ieee80211_new_state(ic, IEEE80211_S_RUN, -1);
>   else
> Index: usb/if_urtwn.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_urtwn.c,v
> retrieving revision 1.89
> diff -u -p -r1.89 if_urtwn.c
> --- usb/if_urtwn.c26 May 2020 06:04:30 -  1.89
> +++ usb/if_urtwn.c5 Jun 2020 11:52:22 -
> @@ -490,10 +490,8 @@ urtwn_attach(struct device *parent, stru
>  
>   ic->ic_updateslot = urtwn_updateslot;
>   ic->ic_updateedca = urtwn_updateedca;
> -#ifdef notyet
>   ic->ic_set_key = urtwn_set_key;
>   ic->ic_delete_key = urtwn_delete_key;
> -#endif
>   /* Override state transition machine. */
>   ic->ic_newstate = urtwn_newstate;
>  
> @@ -1035,6 +1033,10 @@ urtwn_set_key(struct ieee80211com *ic, s
>   struct urtwn_softc *sc = (struct urtwn_softc *)self;
>   struct urtwn_cmd_key cmd;
>  
> + /* Only handle keys for CCMP */
> + if (k->k_cipher != IEEE80211_CIPHER_CCMP)
> + return ieee80211_set_key(ic, ni, k);
> +
>   /* Defer setting of WEP keys until interface is brought up. */
>   if ((ic->ic_if.if_flags & (IFF_UP | IFF_RUNNING)) !=
>   (IFF_UP | IFF_RUNNING))
> @@ -1065,6 +1067,12 @@ urtwn_delete_key(struct ieee80211com *ic
>   struct urtwn_softc *sc = (struct urtwn_softc *)self;
>   struct urtwn_cmd_key cmd;
>  
> + /* Only handle keys for CCMP */
> + if (k->k_cipher != IEEE80211_CIPHER_CCMP) {
> + ieee80211_delete_key(ic, ni, k);
> + return;
> + }
> +
>   if (!(ic->ic_if.if_flags & IFF_RUNNING) ||
>   ic->ic_state != IEEE80211_S_RUN)
>   return; /* Nothing to do. */
> @@ -1084,6 +1092,52 @@ urtwn_delete_key_cb(struct urtwn_softc *
>   rtwn_delete_key(ic, cmd->ni, >key);
> 

Re: ospf6d: change the way interfaces are handled

2020-06-03 Thread Sebastian Benoit
Remi Locherer(remi.loche...@relo.ch) on 2020.06.03 15:36:17 +0200:
> On Sat, May 30, 2020 at 04:37:43PM +0200, Denis Fondras wrote:
> > This diff updates how ospf6d(8) handles interfaces.
> > It is now in line with what ospfd(8) does.
> > 
> > Last step before enabling reload.
> > 
> > Tested against Mikrotik and Zebra implementations.
> > 
> > Warning: it changes the default behaviour. No prefix is announced if no
> > "redistribute" statement is present in config file. Is this a showstopper ?
> 
> The diff reads good and works. I mostly agree with it.
> 
> But we should not change the behaviour. That prefixes configured on an
> interface need a redistribute statement is counter intuitive. The "passive"
> statement would be useless.

There is also a behaviour difference: the "interface { passive }" way only
announces the prefix on active/master interfaces.

I think it needs to stay.

Great work btw!

/Benno

> 
> > 
> > Index: hello.c
> > ===
> > RCS file: /cvs/src/usr.sbin/ospf6d/hello.c,v
> > retrieving revision 1.22
> > diff -u -p -r1.22 hello.c
> > --- hello.c 3 Jan 2020 17:25:48 -   1.22
> > +++ hello.c 30 May 2020 14:19:09 -
> > @@ -175,12 +175,16 @@ recv_hello(struct iface *iface, struct i
> > nbr->priority = LSA_24_GETHI(ntohl(hello.opts));
> > /* XXX neighbor address shouldn't be stored on virtual links */
> > nbr->addr = *src;
> > +   ospfe_imsg_compose_rde(IMSG_NEIGHBOR_ADDR, nbr->peerid, 0,
> > +   src, sizeof(struct in6_addr));
> > }
> >  
> > if (!IN6_ARE_ADDR_EQUAL(>addr, src)) {
> > log_warnx("%s: neighbor ID %s changed its address to %s",
> > __func__, inet_ntoa(nbr->id), log_in6addr(src));
> > nbr->addr = *src;
> > +   ospfe_imsg_compose_rde(IMSG_NEIGHBOR_ADDR, nbr->peerid, 0,
> > +   src, sizeof(struct in6_addr));
> > }
> >  
> > nbr->options = opts;
> > Index: interface.c
> > ===
> > RCS file: /cvs/src/usr.sbin/ospf6d/interface.c,v
> > retrieving revision 1.29
> > diff -u -p -r1.29 interface.c
> > --- interface.c 27 May 2020 09:03:56 -  1.29
> > +++ interface.c 30 May 2020 14:19:09 -
> > @@ -72,8 +72,6 @@ struct {
> >  static int vlink_cnt = 0;
> >  #endif
> >  
> > -TAILQ_HEAD(, iface)iflist;
> > -
> >  const char * const if_event_names[] = {
> > "NOTHING",
> > "UP",
> > @@ -145,10 +143,6 @@ if_fsm(struct iface *iface, enum iface_e
> > area_track(iface->area);
> > orig_rtr_lsa(iface->area);
> > orig_link_lsa(iface);
> > -
> > -   /* state change inform RDE */
> > -   ospfe_imsg_compose_rde(IMSG_IFINFO, iface->self->peerid, 0,
> > -   >state, sizeof(iface->state));
> > }
> >  
> > if (old_state & (IF_STA_MULTI | IF_STA_POINTTOPOINT) &&
> > @@ -166,41 +160,8 @@ if_fsm(struct iface *iface, enum iface_e
> > return (ret);
> >  }
> >  
> > -int
> > -if_init(void)
> > -{
> > -   TAILQ_INIT();
> > -
> > -   return (fetchifs(0));
> > -}
> > -
> > -/* XXX using a linked list should be OK for now */
> >  struct iface *
> > -if_find(unsigned int ifindex)
> > -{
> > -   struct iface*iface;
> > -
> > -   TAILQ_FOREACH(iface, , list) {
> > -   if (ifindex == iface->ifindex)
> > -   return (iface);
> > -   }
> > -   return (NULL);
> > -}
> > -
> > -struct iface *
> > -if_findname(char *name)
> > -{
> > -   struct iface*iface;
> > -
> > -   TAILQ_FOREACH(iface, , list) {
> > -   if (!strcmp(name, iface->name))
> > -   return (iface);
> > -   }
> > -   return (NULL);
> > -}
> > -
> > -struct iface *
> > -if_new(u_short ifindex, char *ifname)
> > +if_new(struct kif *kif, struct kif_addr *ka)
> >  {
> > struct iface*iface;
> >  
> > @@ -210,7 +171,6 @@ if_new(u_short ifindex, char *ifname)
> > iface->state = IF_STA_DOWN;
> >  
> > LIST_INIT(>nbr_list);
> > -   TAILQ_INIT(>ifa_list);
> > TAILQ_INIT(>ls_ack_list);
> > RB_INIT(>lsa_tree);
> >  
> > @@ -225,34 +185,36 @@ if_new(u_short ifindex, char *ifname)
> > return (iface);
> > }
> >  #endif
> > -   strlcpy(iface->name, ifname, sizeof(iface->name));
> > -   iface->ifindex = ifindex;
> > -
> > -   TAILQ_INSERT_TAIL(, iface, list);
> > -
> > -   return (iface);
> > -}
> >  
> > -void
> > -if_update(struct iface *iface, int mtu, int flags, u_int8_t type,
> > -u_int8_t state, u_int64_t rate, u_int32_t rdomain)
> > -{
> > -   iface->mtu = mtu;
> > -   iface->flags = flags;
> > -   iface->if_type = type;
> > -   iface->linkstate = state;
> > -   iface->baudrate = rate;
> > -   iface->rdomain = rdomain;
> > +   strlcpy(iface->name, kif->ifname, sizeof(iface->name));
> >  
> > -   /* set type */
> > -   if (flags & IFF_POINTOPOINT)
> > +   /* get type */
> > +   if (kif->flags & 

Re: sysupgrade change to allow installing from url

2020-05-25 Thread Sebastian Benoit
Solene Rapenne(sol...@perso.pw) on 2020.05.25 15:25:40 +0200:
> Hi,
> 
> I don't know if this will be accepted but I propose to add a -u [url]
> parameter to use older snapshots from an archive server for example.
> 
> I wanted to add an optional parameter to -s at first but in case of
> sysupgrade -s [installurl] the install url would have been mistaken by
> this.
> 
> If you specify a -u url and installurl at the same time, -u url
> superseed installurl.
> 
> This would allow easier bisecting using older snapshots.

I have fixed the way the directories are laid out, so now you can do this

 # echo "http://ftp.hostserver.de/archive/2020-05-14-0105; > /etc/installurl
 # sysupgrade -n
 # strings /home/_sysupgrade/bsd | grep 6.7-current 
@(#)OpenBSD 6.7-current (GENERIC) #181: Wed May 13 12:43:30 MDT 2020


> Index: sysupgrade.8
> ===
> RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.8,v
> retrieving revision 1.10
> diff -u -p -r1.10 sysupgrade.8
> --- sysupgrade.8  3 Oct 2019 12:43:58 -   1.10
> +++ sysupgrade.8  25 May 2020 13:23:06 -
> @@ -24,6 +24,7 @@
>  .Nm
>  .Op Fl fkn
>  .Op Fl r | s
> +.Op Fl u Pa url
>  .Op Ar installurl
>  .Sh DESCRIPTION
>  .Nm
> @@ -66,6 +67,13 @@ This is the default if the system is cur
>  .It Fl s
>  Upgrade to a snapshot.
>  This is the default if the system is currently running a snapshot.
> +.It Fl u Pa url
> +Fetch and verify the files downloaded from 
> +.Pa url .
> +This is superseeding
> +.Op Pa installurl ,
> +its usage is intended for installing older snapshots available under url not 
> following
> +the usual installurl format.
>  .El
>  .Sh FILES
>  .Bl -tag -width "/auto_upgrade.conf" -compact
> Index: sysupgrade.sh
> ===
> RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
> retrieving revision 1.37
> diff -u -p -r1.37 sysupgrade.sh
> --- sysupgrade.sh 26 Jan 2020 22:08:36 -  1.37
> +++ sysupgrade.sh 25 May 2020 13:23:06 -
> @@ -34,7 +34,7 @@ ug_err()
>  
>  usage()
>  {
> - ug_err "usage: ${0##*/} [-fkn] [-r | -s] [installurl]"
> + ug_err "usage: ${0##*/} [-fkn] [-r | -s] [-u url] [installurl]"
>  }
>  
>  unpriv()
> @@ -79,13 +79,14 @@ FORCE=false
>  KEEP=false
>  REBOOT=true
>  
> -while getopts fknrs arg; do
> +while getopts fknrsu: arg; do
>   case ${arg} in
>   f)  FORCE=true;;
>   k)  KEEP=true;;
>   n)  REBOOT=false;;
>   r)  RELEASE=true;;
>   s)  SNAP=true;;
> + u)  SNAPURL=${OPTARG};;
>   *)  usage;;
>   esac
>  done
> @@ -121,7 +122,11 @@ else
>  fi
>  
>  if $SNAP; then
> - URL=${MIRROR}/snapshots/${ARCH}/
> + if [[ -n "$SNAPURL" ]]; then
> + URL=$SNAPURL
> + else
> + URL=${MIRROR}/snapshots/${ARCH}/
> + fi
>  else
>   URL=${MIRROR}/${NEXT_VERSION}/${ARCH}/
>  fi
> 



Re: {plus,}67.html: fix link crontab(5)

2020-05-21 Thread Sebastian Benoit
Martin Vahlensieck(open...@academicsolutions.ch) on 2020.05.20 19:15:48 +0200:
> Hey there!
> 
> Otherwise it's going to crontab(1).

Thanks, commited.

Benno



Re: include interface name in rad error message

2020-05-20 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2020.05.20 09:12:23 +0200:
> In my syslog I have this:
> rad[83563]: RA from non link local address ::
> now it would be splendid to know on which of the 4 interfaces rad is
> operating on this happened. So here is a diff doing that.

ok

> 
> -- 
> :wq Claudio
> 
> Index: engine.c
> ===
> RCS file: /cvs/src/usr.sbin/rad/engine.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 engine.c
> --- engine.c  15 Mar 2019 16:47:19 -  1.15
> +++ engine.c  20 May 2020 07:09:32 -
> @@ -482,7 +482,8 @@ parse_rs(struct imsg_ra_rs *rs)
>   len = rs->len;
>  
>   if (!IN6_IS_ADDR_LINKLOCAL(>from.sin6_addr)) {
> - log_warnx("RA from non link local address %s", hbuf);
> + log_warnx("RA from non link local address %s on %s", hbuf,
> + if_indextoname(rs->if_index, ifnamebuf));
>   return;
>   }
>  
> 



rpki-client 6.7p0 released

2020-05-18 Thread Sebastian Benoit
rpki-client 6.7 has just been released and will be available in the
rpki-client directory of any OpenBSD mirror soon.

rpki-client is a FREE, easy-to-use implementation of the Resource
Public Key Infrastructure (RPKI) for Relying Parties (RP) to
facilitate validation of the Route Origin of a BGP announcement. The
program queries the RPKI repository system and outputs Validated ROA
Payloads in the configuration format of OpenBGPD, BIRD, and also as
CSV or JSON objects for consumption by other routing stacks.

See RFC 6811 for a description of how BGP Prefix Origin Validation
secures the Internet's global routing system.

rpki-client was primarily developed by Kristaps Dzonsons, Claudio
Jeker, Job Snijders, and Sebastian Benoit as part of the OpenBSD
Project and gets released as a base component of OpenBSD every six
months, and follows the OpenBSD release numbering scheme.

This is the first release of the 6.7 version. It includes the follwoing
changes to the previous release:

 * Document the suggested interval for running rpki-client in man page.

 * Always initialize cachedir and outputdir.

 * Print statistics as comments at the top of the output files which
   can take comments, including the date and time when the files were
   produced, and runtime statistics when producing them.

 * Improve log messages to clarify what's happening.

 * Fix a bug where rpki-client would not properly wait for exiting
   rsync processes, causing rpki-client to hang.

The mirrors where rpki-client can be found are on
https://www.rpki-client.org/portable.html

Reporting Bugs:
===

General bugs may be reported to tech@openbsd.org

Portable bugs may be filed at 
https://github.com/rpki-client/rpki-client-portable

We welcome feedback and improvements from the broader community.
Thanks to all of the contributors who helped make this release
possible.



Re: bgpctl parser cleanup

2020-05-12 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2020.05.12 12:42:36 +0200:
> Minimal cleanup of things not used in the bgpctl parser.
> Bulk is not used and the ADDRESS / PREFIX tokens no longer overwrite the
> action since a while.

ok benno@

> 
> -- 
> :wq Claudio
> 
> Index: parser.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/parser.c,v
> retrieving revision 1.103
> diff -u -p -r1.103 parser.c
> --- parser.c  11 May 2020 07:55:18 -  1.103
> +++ parser.c  12 May 2020 10:25:37 -
> @@ -61,8 +61,7 @@ enum token_type {
>   RD,
>   FAMILY,
>   RTABLE,
> - FILENAME,
> - BULK
> + FILENAME
>  };
>  
>  struct token {
> @@ -592,24 +591,18 @@ match_token(int *argc, char **argv[], co
>   if (parse_addr(word, )) {
>   match++;
>   t = [i];
> - if (t->value)
> - res.action = t->value;
>   }
>   break;
>   case PEERADDRESS:
>   if (parse_addr(word, )) {
>   match++;
>   t = [i];
> - if (t->value)
> - res.action = t->value;
>   }
>   break;
>   case PREFIX:
>   if (parse_prefix(word, wordlen, , 
> )) {
>   match++;
>   t = [i];
> - if (t->value)
> - res.action = t->value;
>   }
>   break;
>   case ASTYPE:
> @@ -797,10 +790,6 @@ match_token(int *argc, char **argv[], co
>   t = [i];
>   }
>   break;
> - case BULK:
> - match++;
> - t = [i];
> - break;
>   case ENDTOKEN:
>   break;
>   }
> @@ -890,7 +879,6 @@ show_valid_args(const struct token table
>   case FILENAME:
>   fprintf(stderr, "  \n");
>   break;
> - case BULK:
>   case ENDTOKEN:
>   break;
>   }
> 



Re: teach bgpctl about IPv6 MPLS VPN

2020-05-10 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2020.05.08 09:40:38 +0200:
> Bgpctl has a way to specify the address family to show in 'show rib'
> commands. Teach it to also support IPv6 MPLS VPNs (aka VPNv6).
> 
> OK?



ok

> -- 
> :wq Claudio
> 
> Index: parser.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/parser.c,v
> retrieving revision 1.101
> diff -u -p -r1.101 parser.c
> --- parser.c  22 Jan 2020 07:52:38 -  1.101
> +++ parser.c  7 May 2020 17:11:38 -
> @@ -581,6 +581,11 @@ match_token(int *argc, char **argv[], co
>   t = [i];
>   res.aid = AID_VPN_IPv4;
>   }
> + if (!strcasecmp(word, "VPNv6")) {
> + match++;
> + t = [i];
> + res.aid = AID_VPN_IPv6;
> + }
>   break;
>   case ADDRESS:
>   if (parse_addr(word, )) {
> @@ -878,7 +883,8 @@ show_valid_args(const struct token table
>   fprintf(stderr, "  \n");
>   break;
>   case FAMILY:
> - fprintf(stderr, "  [ inet | inet6 | IPv4 | IPv6 | VPNv4 
> ]\n");
> + fprintf(stderr, "  [ inet | inet6 | IPv4 | IPv6 | "
> + "VPNv4 | VPNv6 ]\n");
>   break;
>   case FILENAME:
>   fprintf(stderr, "  \n");
> 



Re: [patch] relayd.conf.5, DHE params seems incorrect/outdated.

2020-05-02 Thread Sebastian Benoit
Jesper Wallin(jes...@ifconfig.se) on 2020.05.01 12:15:06 +0200:
> Hi all,
> 
> I was trying to score 100 on all the tests over at ssllabs.com, but seem
> to only reach 90 on "Key Exchange".  Not sure if it's related, but I was
> playing with the "dhe" option in relayd.conf(5) in order to increase the
> number of bits used for the ephemeral key.  No matter how I specified
> the option, nothing really changed and I started reading the code in
> order to understand what the option actually do.  I might be completely
> wrong, but from my understanding, it feeds the params of "dhe" as the
> second argument to tls_config_set_dheparams(), which expects "none",
> "legacy" or "auto".
> 
> My guess is that tls_config_set_dheparams() has been updated and the
> manual for relayd.conf(5) has not.  Here's a diff that hopefully solves
> that.
> 
> 
> Jesper Wallin

Thanks for noticing this, i fixed the manpage.

/Benno



Re: iked(8): Removing SHA1 from default transforms

2020-05-01 Thread Sebastian Benoit
Stuart Henderson(s...@spacehopper.org) on 2020.05.01 23:46:49 +0100:
> On 2020/05/02 00:43, Stephan Mending wrote:
> > On 02/05/2020 00:40, Stuart Henderson wrote:
> > > On 2020/05/02 00:23, Stephan Mending wrote:
> > > > Hi,
> > > > 
> > > > I actually read your thread. By what I understood you're at the moment
> > > > trying to change a few defaults.
> > > > 
> > > > That was the reason I wanted to add SHA1 for removal. I just thought it
> > > > deserved a seperate thread.
> > > > 
> > > > I do understand that you're trying to be careful with removing or 
> > > > changing
> > > > defaults. From my point of view everybody that is (maybe implicitly) 
> > > > using
> > > > SHA1 right now is better off to be get this wakeup call the earlier the
> > > > better.
> > > > 
> > > > We aren't even removing SHA1 we're just not offering it as default. And 
> > > > for
> > > > those Windows boxes who require it, those people just have to add a 
> > > > line to
> > > > explicitly enable it. I would not see such big of a problem.
> > > The things removed recently have a very low risk of affecting anyone.
> > > sha1 (and modp1024) are high risk.
> > > 
> > > Removing from the default list may cause some people to be unable
> > > to connect to their network after updating. This may mean that they
> > > are then unable to connect back in to fix it.
> > > 
> > > If this change is made it needs to be done fairly early in the release
> > > cycle, and preferably at a time when slightly fewer people are relying
> > > on working remote access to get at their networks.
> > > 
> > 
> > I dont't have much experience with such a big projekt like OpenBSD. How do
> > you normally carry through with such significant changes ? Just the release
> > notes and hoping somebody in snaps will complain ? Or is there more to it,
> > which I didn't notice ?
> > 
> 
> Testing where we can, but allowing for the fact that we can't test
> everything riskier changes need to be done at a point where we have a
> good chance to get feedback from -current users so we can come up with
> good advice for release notes.

For flag days like this, we try to have a transition period if possible.

For example here we could announce (in the release notes and/or the upgrade
guide for 6.7) that sha1/modp1024 is deprecated, including a command for
users to check if they have it in use. Some time after 6.7 is released, in
the -current branch, we remove them.

Then users of releases/stable-branch have 6 months to change their
configuration.

Alternatively we can remove them after 6.8 has been released, giving even
more time.



Re: JSON support for bgpctl(8)

2020-05-01 Thread Sebastian Benoit
Hiltjo Posthuma(hil...@codemadness.org) on 2020.05.01 16:31:33 +0200:
> On Fri, May 01, 2020 at 01:18:03PM +0200, Claudio Jeker wrote:
> > This diff add JSON output support for bgpctl.
> > Most commands should produce now a resonable JSON object.
> > The individual objects can probably be improved and extended.
> > I'm at a point where I'm happy with the result and looking for feedback.
> > It is possible to commit the non-JSON bits first and add output-json.c
> > once the output is enough stable.
> > 
> 
> Hi,
> 
> Maybe in the function do_name() and in functions where strings are printed the
> text should be escaped just in case? For example the characters ", \ and
> encoding of control-characters.

I think the only string thats not generated from trusted data (i.e.
hardcoded strings, strings fromthe configuration file or numbers) is escaped
with strnvis() in log_shutcomm().



Re: [patch] relayd.conf.5, DHE params seems incorrect/outdated.

2020-05-01 Thread Sebastian Benoit
Jesper Wallin(jes...@ifconfig.se) on 2020.05.01 12:15:06 +0200:
> Hi all,
> 
> I was trying to score 100 on all the tests over at ssllabs.com, but seem
> to only reach 90 on "Key Exchange".  Not sure if it's related, but I was
> playing with the "dhe" option in relayd.conf(5) in order to increase the
> number of bits used for the ephemeral key.  No matter how I specified
> the option, nothing really changed and I started reading the code in
> order to understand what the option actually do.  I might be completely
> wrong, but from my understanding, it feeds the params of "dhe" as the
> second argument to tls_config_set_dheparams(), which expects "none",
> "legacy" or "auto".
> 
> My guess is that tls_config_set_dheparams() has been updated and the
> manual for relayd.conf(5) has not.  Here's a diff that hopefully solves
> that.
> 
> 
> Jesper Wallin

Thanks for noticing this.

Indeed it has to specified as auto, none or legacy.

I cant see how this could ever have worked, i believe the documentation was
always wrong.

I changed the description in the manpage a bit.

ok?

Index: relayd.conf.5
===
RCS file: /cvs/src/usr.sbin/relayd/relayd.conf.5,v
retrieving revision 1.195
diff -u -p -r1.195 relayd.conf.5
--- relayd.conf.5   23 Apr 2020 21:28:10 -  1.195
+++ relayd.conf.5   1 May 2020 12:30:30 -
@@ -960,17 +960,22 @@ suites, in order of preference.
 The special value of "default" will use the default curves; see
 .Xr tls_config_set_ecdhecurves 3
 for further details.
-.It Ic edh Op Ic params Ar maximum
+.It Ic edh Op Ic params Pq Ic none Ns | Ns Ic auto Ns | Ns Ic legacy
 Enable EDH-based cipher suites with Perfect Forward Secrecy (PFS) for
 older clients that do not support ECDHE.
-If the
-.Ar maximum
-length of the DH params for EDH is not specified, the default value of
-1024 bits will be used.
-Other possible values are numbers between 1024 and 8192, including
-1024, 1536, 2048, 4096, or 8192.
-Values higher than 1024 bits can cause incompatibilities with older
-TLS clients.
+In
+.Ic auto
+mode, the key size of the ephemeral key is automatically selected
+based on the size of the private key used for signing.
+In
+.Ic legacy
+mode, a 1024 bit ephemeral key is used.
+If
+.Ic edh
+is enabled without specifiying the
+.Ic params ,
+.Ic auto
+is used.
 The default is
 .Ic no edh .
 .It Ic keypair Ar name



Re: [PATCH] sysupgrade

2020-04-30 Thread Sebastian Benoit
James Jerkins(j...@jamesjerkinscomputer.com) on 2020.04.29 22:28:12 -0500:
> Hello,
> 

> This patch adds two new options to sysupgrade. The first option is for
> small box systems like an APU system that only has the base and manual

We wont add tons of options to this tool for every use case.

Just make a copy of the sysupgrade script and add your own mondifications
there. Or a wrapper around it.



alpha installation notes INSTALL.alpha

2020-04-27 Thread Sebastian Benoit
Hi,

there have been no floppy images since the 6.2 release. This removes mention
of boot floppies from the INSTALL.alpha notes. Maybe someone who knows
something about alpha machines can do a check?

comments or oks?

diff --git distrib/notes/alpha/contents distrib/notes/alpha/contents
index eccbc79af6c..bb3867a540b 100644
--- distrib/notes/alpha/contents
+++ distrib/notes/alpha/contents
@@ -5,35 +5,6 @@ OpenBSDminiroot
It can be copied to the swap partition of an existing
disk to allow installing or upgrading to OpenBSD OSREV.
 
-OpenBSDfloppy
-   This floppy image will boot on the following MACHINE
-   models:
-   - AlphaStation 200, 250, 255, 400
-   - AlphaServer 300 and 400
-   - AlphaStation 500, 600
-   - AlphaStation 600A, 1200
-   - AlphaServer 800, 1000, 1000A, 1200, 4000 and 4100
-   - AXPpci33 based machines, including ``Noname'',
- UDB, Multia
-   - EB164 based machines, including PC164, 164SX,
- and 164LX
-   - Personal Workstation (Miata)
-
-   floppyB{:--:}OSrev.fs   Another MACHINE boot and installation floppy; 
see
-   below.
-   This floppy image will boot on the following MACHINE
-   models:
-   - Alpha Processor, Inc. UP1000, UP1100, UP2000 and
- UP2000+
-   - XP900, XP1000, CS20, DS10, DS20, DS20L, ES40 and
- 264DP
-
-   floppyC{:--:}OSrev.fs   Another MACHINE boot and installation floppy; 
see
-   below.
-   This floppy image will boot on the following MACHINE
-   models:
-   - Tadpole AlphaBook
-
 OpenBSDdistsets
 
 OpenBSDbsd
@@ -57,8 +28,6 @@ OpenBSDcd
netboot.mop The OpenBSD/MACHINE network boot loader, for MOP
protocol.
 
-OpenBSDfloppydesc(three,Each,s)
-
 DistributionDescription(eight)
 
 OpenBSDbase(101660534,244170752)
diff --git distrib/notes/alpha/install distrib/notes/alpha/install
index 1c0033aafa9..3ab01311789 100644
--- distrib/notes/alpha/install
+++ distrib/notes/alpha/install
@@ -3,23 +3,9 @@ OpenBSDInstallPrelude
 
 There are several ways to install OpenBSD onto a disk.  The easiest way is
 to boot from the bootable CD-ROM mini image, then install from your favorite
-source. You can also use one of the OpenBSD installation floppies, if your
-machine has a floppy drive. Network booting is supported through means of
+source. Network booting is supported through means of
 dhcpd(8) and tftpd(8).
 
-Booting from Floppy Disk installation media:
-
-   At the SRM console prompt, enter
-   boot dva0
-   You should see info about the primary and secondary boot
-   and then the kernel should start to load.  It will take a
-   while to load the kernel from the floppy, most likely more
-   than a minute.  If some action doesn't eventually happen,
-   or the spinning cursor has stopped and nothing further has
-   happened, or the machine spontaneously reboots, then either
-   you have a bad boot floppy (in which case you should try
-   another) or your alpha is not currently supported by OpenBSD.
-
 Booting from CD-ROM installation media:
 
At the SRM console prompt, enter
@@ -36,12 +22,12 @@ Booting from CD-ROM installation media:
boot DEVICE
where DEVICE is the dka device name.
 
-   You should see info about the primary and secondary boot
-   and then the kernel should start to load.  If the kernel
-   fails to load or the spinning cursor has stopped and nothing
-   further has happened, you either have a hardware problem or
-   your MACHINE is not currently supported by OpenBSD; try booting
-   from a floppy instead if possible.
+   You should see info about the primary and secondary boot and then the
+   kernel should start to load.  If the kernel fails to load or the
+   spinning cursor has stopped and nothing further has happened, you
+   either have a hardware problem or your MACHINE is not currently
+   supported by OpenBSD; try booting from the network instead if
+   possible.
 
 Booting from Network:
 
@@ -124,7 +110,7 @@ Booting from Network:
Once loaded, the boot loader will mount /alpha over NFS and load
the kernel from there.
 
-Installing using the Floppy, CD-ROM or Network procedure:
+Installing using the CD-ROM or Network procedure:
 
 OpenBSDInstallPart2
 
diff --git distrib/notes/alpha/prep distrib/notes/alpha/prep
index 29fda76e463..4aadfc21f83 100644
--- distrib/notes/alpha/prep
+++ distrib/notes/alpha/prep
@@ -63,8 +63,7 @@ Using the SRM console:
 

Re: Make Rockchip RK3399 eMMC faster

2020-04-24 Thread Sebastian Benoit
Mark Kettenis(mark.kette...@xs4all.nl) on 2020.04.23 22:56:17 +0200:
> I put this in at some point since I couldn't get the eMMC on my
> firefly-rk3399 working otherwise.  But its eMMC died and on my
> rockpro64 and rk3399-q7 boards things work very well without it.  On
> the latter board it even makes things a bit speedier: the raw read
> performance goes up from 35 MB/s to 43 MB/s.
> 
> Probably good if this was tested on the pinebook pro.

On the pinebook pro with the diff i get 38 MB/s from the eMMC, reading from
rsd1c. Thats average over 10 reads.

Without the diff i get only about 28 MB/s.

/Benno

> 
> ok?
> 
> 
> Index: dev/fdt/sdhc_fdt.c
> ===
> RCS file: /cvs/src/sys/dev/fdt/sdhc_fdt.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 sdhc_fdt.c
> --- dev/fdt/sdhc_fdt.c21 Apr 2020 07:58:57 -  1.7
> +++ dev/fdt/sdhc_fdt.c23 Apr 2020 20:51:48 -
> @@ -154,9 +154,6 @@ sdhc_fdt_attach(struct device *parent, s
>*/
>   phy_enable(faa->fa_node, "phy_arasan");
>   sc->sc.sc_flags |= SDHC_F_NOPWR0;
> -
> - /* XXX Doesn't work on Rockchip RK3399. */
> - sc->sc.sc_flags |= SDHC_F_NODDR50;
>   }
>  
>   if (OF_is_compatible(faa->fa_node, "brcm,bcm2711-emmc2"))
> 



Re: bgpd local-address improvement

2020-04-23 Thread Sebastian Benoit
reads ok

Claudio Jeker(cje...@diehard.n-r-g.com) on 2020.04.23 10:04:15 +0200:
> local-address is one of those values that need to be set in some cases but
> is not very flexible to use. This diff tries to change this a bit.
> 
> It allows to set the local-address for both IPv4 and IPv6 at the same time
> and also allows to unset a previously set local-address. For example:
> 
> group IBGP {
> local-address 192.0.2.1
> local-address 2001:db8:abcd::1
> 
>   neighbor 192.0.2.2 { remote-as $AS }
>   neighbor 2001:db8:abcd::2 { remote-as $AS }
> 
>   # reset the local-address for whatever reason
>   neighbor 192.0.2.3 {
>   no local-address
>   remote-as $AS
>   }
> }
> 
> As usual setting a local-address on the neighbor will override the group
> config. I think for IBGP and multihop sessions this can simplify the
> config a fair bit. In my case this will collaps IPv4 and IPv6 specific
> groups back together since the only reason they are split is because of
> local-address.
> 
> What do other bgpd user think?
> -- 
> :wq Claudio
> 
> Index: bgpd.h
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.401
> diff -u -p -r1.401 bgpd.h
> --- bgpd.h14 Feb 2020 13:54:31 -  1.401
> +++ bgpd.h22 Apr 2020 15:50:46 -
> @@ -365,7 +365,8 @@ struct capabilities {
>  
>  struct peer_config {
>   struct bgpd_addr remote_addr;
> - struct bgpd_addr local_addr;
> + struct bgpd_addr local_addr_v4;
> + struct bgpd_addr local_addr_v6;
>   struct peer_auth auth;
>   struct capabilities  capabilities;
>   char group[PEER_DESCR_LEN];
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.405
> diff -u -p -r1.405 parse.y
> --- parse.y   16 Mar 2020 14:47:30 -  1.405
> +++ parse.y   23 Apr 2020 07:51:25 -
> @@ -1260,8 +1260,27 @@ peeropts   : REMOTEAS as4number{
>   free($2);
>   }
>   | LOCALADDR address {
> - memcpy(>conf.local_addr, &$2,
> - sizeof(curpeer->conf.local_addr));
> + if ($2.aid == AID_INET)
> + memcpy(>conf.local_addr_v4, &$2,
> + sizeof(curpeer->conf.local_addr_v4));
> + else if ($2.aid == AID_INET6)
> + memcpy(>conf.local_addr_v6, &$2,
> + sizeof(curpeer->conf.local_addr_v6));
> + else {
> + yyerror("Unsupported address family %s for "
> + "local-addr", aid2str($2.aid));
> + YYERROR;
> + }
> + }
> + | yesno LOCALADDR   {
> + if ($1) {
> + yyerror("bad local-address definition");
> + YYERROR;
> + }
> + memset(>conf.local_addr_v4, 0,
> + sizeof(curpeer->conf.local_addr_v4));
> + memset(>conf.local_addr_v6, 0,
> + sizeof(curpeer->conf.local_addr_v6));
>   }
>   | MULTIHOP NUMBER   {
>   if ($2 < 2 || $2 > 255) {
> @@ -4176,11 +4195,17 @@ str2key(char *s, char *dest, size_t max_
>  int
>  neighbor_consistent(struct peer *p)
>  {
> - /* local-address and peer's address: same address family */
> - if (p->conf.local_addr.aid &&
> - p->conf.local_addr.aid != p->conf.remote_addr.aid) {
> - yyerror("local-address and neighbor address "
> - "must be of the same address family");
> + struct bgpd_addr *local_addr;
> +
> + switch (p->conf.remote_addr.aid) {
> + case AID_INET:
> + local_addr = >conf.local_addr_v4;
> + break;
> + case AID_INET6:
> + local_addr = >conf.local_addr_v6;
> + break;
> + default:
> + yyerror("Bad address family for remote-addr");
>   return (-1);
>   }
>  
> @@ -4189,7 +4214,7 @@ neighbor_consistent(struct peer *p)
>   p->conf.auth.method == AUTH_IPSEC_IKE_AH ||
>   p->conf.auth.method == AUTH_IPSEC_MANUAL_ESP ||
>   p->conf.auth.method == AUTH_IPSEC_MANUAL_AH) &&
> - !p->conf.local_addr.aid) {
> + local_addr->aid == AID_UNSPEC) {
>   yyerror("neighbors with any form of IPsec configured "
>   "need local-address to be specified");
>   return (-1);
> Index: pfkey.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/pfkey.c,v
> retrieving 

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

2020-04-21 Thread Sebastian Benoit
Bartosz Kuzma(bartosz.ku...@release11.com) on 2020.04.21 20:59:54 +0200:
> Hello,
> 
> thanks for looking at this!
> 
> On 21/04/2020 17:43, Florian Obser wrote:
> >Hi,
> >
> >thanks for working on this and finding another acme implementor!
> >
> >On Mon, Apr 20, 2020 at 06:51:17PM +0200, Bartosz Kuzma wrote:
> >>Hello,
> >>
> >>I've tried to get a certificate from Buypass Go SSL provider using
> >>acme-client(1) but it ends with the following error:
> >>
> >>acme-client: https://api.buypass.com/acme-v02/new-acct: bad HTTP: 400
> >>acme-client: transfer buffer:
> >>[{"type":"urn:ietf:params:acme:error:malformed","d
> >>etail":"Email is a required
> >>contact","code":400,"message":"MALFORMED_BAD_REQUEST
> >>","details":"HTTP 400 Bad Request"}] (164 bytes)
> >>
> >>Buypass Go SSL requires that optional field contact in Account Objects
> >>will contain email address.
> >>
> >>I've added new option "contact" to acme-client.conf to fullfil this
> >>requirement. It is also required to change MARKER in certproc.c to fix 
> >>line
> >>endings handling in pem files.
> >
> >they are leaving out the last newline?
> 
> It seems that chain can be provided with "\n" or "\r\n" line separator. 
> I removed it from MARKER to handle both cases.
> 
> >
> >>
> >>-- 
> >>Kind regards, Bartosz Kuzma.
> >
> >>Index: usr.sbin/acme-client/acme-client.conf.5
> >>===
> >>RCS file: /cvs/src/usr.sbin/acme-client/acme-client.conf.5,v
> >>retrieving revision 1.22
> >>diff -u -p -r1.22 acme-client.conf.5
> >>--- usr.sbin/acme-client/acme-client.conf.5 10 Feb 2020 13:18:21 - 
> >>1.22
> >>+++ usr.sbin/acme-client/acme-client.conf.5 20 Apr 2020 16:24:46 -
> >>@@ -98,6 +98,10 @@ It defaults to
> >>  Specify the
> >>  .Ar url
> >>  under which the ACME API is reachable.
> >>+.It Ic contact Ar contact
> >>+Optional
> >>+.Ar contact
> >>+URLs that the authority can use to contact the client for issues related 
> >>to this account.
> >
> >I think we should call it emails. That's what people actually have.
> >According to the RFC it's a list of contacts, but I'm fine with having 
> >only one.
> 
> For me one contact is also enough.
> 
> >
> >>  .El
> >>  .Sh DOMAINS
> >>  The certificates to be obtained through ACME.
> >>Index: usr.sbin/acme-client/certproc.c
> >>===
> >>RCS file: /cvs/src/usr.sbin/acme-client/certproc.c,v
> >>retrieving revision 1.12
> >>diff -u -p -r1.12 certproc.c
> >>--- usr.sbin/acme-client/certproc.c 7 Jun 2019 08:07:52 -   1.12
> >>+++ usr.sbin/acme-client/certproc.c 20 Apr 2020 16:24:46 -
> >>@@ -28,7 +28,7 @@
> >>  
> >>  #include "extern.h"
> >>  
> >>-#define MARKER "-END CERTIFICATE-\n"
> >>+#define MARKER "-END CERTIFICATE-"
> >>  
> >>  int
> >>  certproc(int netsock, int filesock)
> >>@@ -94,6 +94,12 @@ certproc(int netsock, int filesock)
> >>}
> >>  
> >>chaincp += strlen(MARKER);
> >>+
> >>+   if ((chaincp = strchr(chaincp, '-')) == NULL) {
> >>+   warn("strchr");
> >>+   goto out;
> >>+   }
> >>+
> >
> >I don't quite understand what this is doing.
> >
> Because I removed new line character from MARKER strchr() just move 
> pointer to -BEGIN CERTIFICATE to skip any new lines character 
> from chain. It can be replaced by strstr("-BEGIN CERTIFICATE-") 
> if this improve readability.

The warn() should be a warnx() then, because strchr() does not set errno.
When it returns NULL it just means that it did not find the string. So

   warnx("invalid certificate chain");

would be a good error message here i think.



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

2020-04-21 Thread Sebastian Benoit
Bartosz Kuzma(bartosz.ku...@release11.com) on 2020.04.20 18:51:17 +0200:
> Hello,
> 
> I've tried to get a certificate from Buypass Go SSL provider using
> acme-client(1) but it ends with the following error:
> 
> acme-client: https://api.buypass.com/acme-v02/new-acct: bad HTTP: 400
> acme-client: transfer buffer: 
> [{"type":"urn:ietf:params:acme:error:malformed","d
> etail":"Email is a required 
> contact","code":400,"message":"MALFORMED_BAD_REQUEST
> ","details":"HTTP 400 Bad Request"}] (164 bytes)
> 
> Buypass Go SSL requires that optional field contact in Account Objects
> will contain email address.
> 
> I've added new option "contact" to acme-client.conf to fullfil this 
> requirement.

Thanks, that looks ok.

> It is also required to change MARKER in certproc.c to fix 
> line endings handling in pem files.

Can you explain what happens there?

/Benno

> -- 
> Kind regards, Bartosz Kuzma.

> Index: usr.sbin/acme-client/acme-client.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/acme-client/acme-client.conf.5,v
> retrieving revision 1.22
> diff -u -p -r1.22 acme-client.conf.5
> --- usr.sbin/acme-client/acme-client.conf.5   10 Feb 2020 13:18:21 -  
> 1.22
> +++ usr.sbin/acme-client/acme-client.conf.5   20 Apr 2020 16:24:46 -
> @@ -98,6 +98,10 @@ It defaults to
>  Specify the
>  .Ar url
>  under which the ACME API is reachable.
> +.It Ic contact Ar contact
> +Optional
> +.Ar contact
> +URLs that the authority can use to contact the client for issues related to 
> this account.
>  .El
>  .Sh DOMAINS
>  The certificates to be obtained through ACME.
> Index: usr.sbin/acme-client/certproc.c
> ===
> RCS file: /cvs/src/usr.sbin/acme-client/certproc.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 certproc.c
> --- usr.sbin/acme-client/certproc.c   7 Jun 2019 08:07:52 -   1.12
> +++ usr.sbin/acme-client/certproc.c   20 Apr 2020 16:24:46 -
> @@ -28,7 +28,7 @@
>  
>  #include "extern.h"
>  
> -#define MARKER "-END CERTIFICATE-\n"
> +#define MARKER "-END CERTIFICATE-"
>  
>  int
>  certproc(int netsock, int filesock)
> @@ -94,6 +94,12 @@ certproc(int netsock, int filesock)
>   }
>  
>   chaincp += strlen(MARKER);
> +
> + if ((chaincp = strchr(chaincp, '-')) == NULL) {
> + warn("strchr");
> + goto out;
> + }
> +
>   if ((chain = strdup(chaincp)) == NULL) {
>   warn("strdup");
>   goto out;
> Index: usr.sbin/acme-client/extern.h
> ===
> RCS file: /cvs/src/usr.sbin/acme-client/extern.h,v
> retrieving revision 1.17
> diff -u -p -r1.17 extern.h
> --- usr.sbin/acme-client/extern.h 7 Feb 2020 14:34:15 -   1.17
> +++ usr.sbin/acme-client/extern.h 20 Apr 2020 16:24:46 -
> @@ -261,13 +261,13 @@ int  json_parse_capaths(struct jsmnn *,
>  
>  char *json_fmt_newcert(const char *);
>  char *json_fmt_chkacc(void);
> -char *json_fmt_newacc(void);
> +char *json_fmt_newacc(const char *);
>  char *json_fmt_neworder(const char *const *, size_t);
>  char *json_fmt_protected_rsa(const char *,
>   const char *, const char *, const char *);
>  char *json_fmt_protected_ec(const char *, const char *, const char *,
>   const char *);
> -char *json_fmt_protected_kid(const char*, const char *, const char *,
> +char *json_fmt_protected_kid(const char *, const char *, const char 
> *,
>   const char *);
>  char *json_fmt_revokecert(const char *);
>  char *json_fmt_thumb_rsa(const char *, const char *);
> Index: usr.sbin/acme-client/json.c
> ===
> RCS file: /cvs/src/usr.sbin/acme-client/json.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 json.c
> --- usr.sbin/acme-client/json.c   22 Jan 2020 22:25:22 -  1.16
> +++ usr.sbin/acme-client/json.c   20 Apr 2020 16:24:46 -
> @@ -616,19 +616,30 @@ json_fmt_chkacc(void)
>   * Format the "newAccount" resource request.
>   */
>  char *
> -json_fmt_newacc(void)
> +json_fmt_newacc(const char *contact)
>  {
>   int  c;
> - char*p;
> + char*p, *cnt;
> +
> + c = asprintf(, "\"contact\": [ \"%s\" ], ", contact);
> + if (c == -1) {
> + warn("asprintf");
> + goto err;
> + }
>  
>   c = asprintf(, "{"
> + "%s"
>   "\"termsOfServiceAgreed\": true"
> - "}");
> + "}", contact == NULL ? "" : cnt);
> + free(cnt);
>   if (c == -1) {
>   warn("asprintf");
>   p = NULL;
>   }
>   return p;
> +
> +err:
> + return NULL;
>  }
>  
>  /*
> Index: usr.sbin/acme-client/netproc.c
> 

Re: unwind(8): recommend supersede in dhclient.conf

2020-04-21 Thread Sebastian Benoit
Florian Obser(flor...@openbsd.org) on 2020.04.21 06:57:49 +0200:
> We didn't get around to run unwind per default and integrate it
> tighter with dhclient this release cycle.
> But there is also no need anymore to recomend prepend in
> dhclient.conf, unwind(8) is no longer closing it's service port when
> it's running so it should always be available.
> 
> (I checked the installer there supersede does not have any effect,
> probably because the small dhclient doesn't parse /etc/dhclient.conf.)
> 
> Comments, OK?

I've actually wondered about this myself and been using it with supersede
for a few weeks now.

ok benno@

> 
> diff --git unwind.8 unwind.8
> index fe08a9b4c4c..7014db070f9 100644
> --- unwind.8
> +++ unwind.8
> @@ -52,7 +52,7 @@ in
>  .Pp
>  Adding
>  .Pp
> -.Dl prepend domain-name-servers 127.0.0.1;
> +.Dl supersede domain-name-servers 127.0.0.1;
>  .Pp
>  to
>  .Pa /etc/dhclient.conf
> 
> 
> -- 
> I'm not entirely sure you are real.
> 



rpki-client 6.6p2 (portable) has been released

2020-04-19 Thread Sebastian Benoit
rpki-client 6.6p2 has just been released. It is the first public
portable release of the rpki-client code, available in OpenBSD.

It will be available from the mirrors listed at
http://www.rpki-client.org/ shortly.

rpki-client is a FREE, easy-to-use implementation of the Resource
Public Key Infrastructure (RPKI) for Relying Parties (RP) to
facilitate validation of the Route Origin of a BGP announcement. The
program queries the RPKI repository system and outputs Validated ROA
Payloads in the configuration format of OpenBGPD, BIRD, and also as
CSV or JSON objects for consumption by other routing stacks.

See RFC 6811 for a description of how BGP Prefix Origin Validation
secures the Internet's global routing system.

rpki-client was primarily developed by Kristaps Dzonsons, Claudio
Jeker, Job Snijders, and Sebastian Benoit as part of the OpenBSD
Project and gets released as a base component of OpenBSD every six
months.

This first release is based on the OpenBSD source code available in the
OpenBSD CVS repository as of 2020-04-19.

Reporting Bugs:
===

General bugs may be reported to tech@openbsd.org

Portable bugs may be filed at 
https://github.com/rpki-client/rpki-client-portable



Re: cpu utilisation bars for top(1)

2020-04-13 Thread Sebastian Benoit
Edd Barrett(e...@theunixzoo.co.uk) on 2020.04.13 15:47:03 +0100:
> Hi,
> 
> One thing I miss from our top(1) is the ability to see overall CPU
> utilisation at a glance (I usually scan for the idle percentage and
> invert it in my head).
> 
> This diff adds a way to toggle (using `B`) CPU utilisation bars, like this:
> 
> ```
> load averages:  0.92,  0.52,  1.26  arrakis.home 
> 15:31:14
> 110 processes: 108 idle, 1 stopped, 1 on processorup 1 day,  
> 2:15
> []  
> 37.1%
> [#   ]  
> 45.0%
> []  
> 42.8%
> [### ]  
> 47.2%
> Memory: Real: 4399M/12G act/tot Free: 11G Cache: 6204M Swap: 0K/1028M
> ```
> 
> The value reported is the inverse of the idle percentage and the bars
> replace the 'user, nice, sys, ...' lines.
> 
> Can also be enabled from the command line with `-B`.
> 
> What do people think?

If you make them look like the display in systat, you still get the user,
... nice, sys information. Also i like my bikeshed green.



Re: slaacd(8): honour rdomain we are running in

2020-04-12 Thread Sebastian Benoit
Florian Obser(flor...@openbsd.org) on 2020.04.12 19:53:23 +0200:
> OK?

yes, this is probably better than having it configurable via option.

> diff --git slaacd.c slaacd.c
> index 58f15bcda37..dae2eab3434 100644
> --- slaacd.c
> +++ slaacd.c
> @@ -755,7 +755,7 @@ configure_gateway(struct imsg_configure_dfr *dfr, uint8_t 
> rtm_type)
>   rtm.rtm_version = RTM_VERSION;
>   rtm.rtm_type = rtm_type;
>   rtm.rtm_msglen = sizeof(rtm);
> - rtm.rtm_tableid = 0; /* XXX imsg->rdomain; */
> + rtm.rtm_tableid = getrtable();
>   rtm.rtm_index = dfr->if_index;
>   rtm.rtm_seq = ++rtm_seq;
>   rtm.rtm_priority = RTP_NONE;
> @@ -852,7 +852,7 @@ send_rdns_proposal(struct imsg_propose_rdns *rdns)
>   rtm.rtm_version = RTM_VERSION;
>   rtm.rtm_type = RTM_PROPOSAL;
>   rtm.rtm_msglen = sizeof(rtm);
> - rtm.rtm_tableid = 0; /* XXX imsg->rdomain; */
> + rtm.rtm_tableid = getrtable();
>   rtm.rtm_index = rdns->if_index;
>   rtm.rtm_seq = ++rtm_seq;
>   rtm.rtm_priority = RTP_PROPOSAL_SLAAC;
> 
> 
> -- 
> I'm not entirely sure you are real.
> 



Re: rpki-client and non-existing files

2020-04-01 Thread Sebastian Benoit
ok

you remove the "if (verbose > 0)" in the cms_parse_validate() case on
purpose?

Claudio Jeker(cje...@diehard.n-r-g.com) on 2020.04.01 16:33:44 +0200:
> On Wed, Apr 01, 2020 at 01:06:21PM +0200, Claudio Jeker wrote:
> > Currently rpki-client logs missing files like this:
> > 
> > rpki-client:  ...trace: error:02FFF002:system library:func(4095):No such 
> > file or directory
> > rpki-client:  ...trace: error:20FFF080:BIO routines:CRYPTO_internal:no such 
> > file
> > rpki-client: 
> > rpki.cnnic.cn/rpki/A9162E3D/515/FE-4PMY9qqTI2aJ0xLDm7cD-fvw.mft: 
> > BIO_new_file
> > 
> > Yes, you need to read the errors in reverse and even then the errors are
> > just hard to read.
> > 
> > This ugly format is mostly to blame on the error stack of OpenSSL.
> > As a workaround I switched to using fopen() and then BIO_new_fd()
> > which does the same thing but allows me to get a nice error from fopen():
> > 
> > rpki-client: 
> > rpki.cnnic.cn/rpki/A9162E3D/515/FE-4PMY9qqTI2aJ0xLDm7cD-fvw.mft: fopen: 
> > No such file or directory
> > 
> > Any opinions?
> 
> This diff removes the fopen: from the warn string:
> 
> rpki-client: 
> rpki.cnnic.cn/rpki/A9162E3D/515/FE-4PMY9qqTI2aJ0xLDm7cD-fvw.mft: No such 
> file or directory
> 
> This is more in form with e.g.
> 
> rpki-client: 
> rpki-repo.registro.br/repo/D81aiXpDAv5WBmgE8oEpfordjGP62otn2fHrhaL4cgby/0/3137372e3133302e302e302f32302d3234203d3e203238323630.roa:
>  CRL has expired
> 
> -- 
> :wq Claudio
> 
> Index: cert.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 cert.c
> --- cert.c26 Feb 2020 02:35:08 -  1.14
> +++ cert.c1 Apr 2020 14:28:29 -
> @@ -930,12 +930,18 @@ cert_parse_inner(X509 **xp, const char *
>   ASN1_OBJECT *obj;
>   struct parse p;
>   BIO *bio = NULL, *shamd;
> + FILE*f;
>   EVP_MD  *md;
>   char mdbuf[EVP_MAX_MD_SIZE];
>  
>   *xp = NULL;
>  
> - if ((bio = BIO_new_file(fn, "rb")) == NULL) {
> + if ((f = fopen(fn, "rb")) == NULL) {
> + warn("%s", fn);
> + return NULL;
> + }
> +
> + if ((bio = BIO_new_fp(f, BIO_CLOSE)) == NULL) {
>   if (verbose > 0)
>   cryptowarnx("%s: BIO_new_file", fn);
>   return NULL;
> Index: cms.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cms.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 cms.c
> --- cms.c 29 Nov 2019 05:14:11 -  1.6
> +++ cms.c 1 Apr 2020 14:28:34 -
> @@ -42,6 +42,7 @@ cms_parse_validate(X509 **xp, const char
>   ASN1_OCTET_STRING   **os = NULL;
>   BIO *bio = NULL, *shamd;
>   CMS_ContentInfo *cms;
> + FILE*f;
>   char buf[128], mdbuf[EVP_MAX_MD_SIZE];
>   int  rc = 0, sz;
>   STACK_OF(X509)  *certs = NULL;
> @@ -55,10 +56,13 @@ cms_parse_validate(X509 **xp, const char
>* This is usually fopen() failure, so let it pass through to
>* the handler, which will in turn ignore the entity.
>*/
> + if ((f = fopen(fn, "rb")) == NULL) {
> + warn("%s", fn);
> + return NULL;
> + }
>  
> - if ((bio = BIO_new_file(fn, "rb")) == NULL) {
> - if (verbose > 0)
> - cryptowarnx("%s: BIO_new_file", fn);
> + if ((bio = BIO_new_fp(f, BIO_CLOSE)) == NULL) {
> + cryptowarnx("%s: BIO_new_fp", fn);
>   return NULL;
>   }
>  
> Index: crl.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/crl.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 crl.c
> --- crl.c 29 Nov 2019 04:40:04 -  1.7
> +++ crl.c 1 Apr 2020 14:28:41 -
> @@ -36,10 +36,16 @@ crl_parse(const char *fn, const unsigned
>   int  rc = 0, sz;
>   X509_CRL*x = NULL;
>   BIO *bio = NULL, *shamd;
> + FILE*f;
>   EVP_MD  *md;
>   char mdbuf[EVP_MAX_MD_SIZE];
>  
> - if ((bio = BIO_new_file(fn, "rb")) == NULL) {
> + if ((f = fopen(fn, "rb")) == NULL) {
> + warn("%s", fn);
> + return NULL;
> + }
> +
> + if ((bio = BIO_new_fp(f, BIO_CLOSE)) == NULL) {
>   if (verbose > 0)
>   cryptowarnx("%s: BIO_new_file", fn);
>   return NULL;
> 



Re: bgpctl code reshuffle

2020-03-19 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2020.03.19 18:42:28 +0100:
> Move some more output functions to output.c and convert some other
> functions to a fmt_xyz() function that returns a string with the value
> instead of doing a printf(). This is mostly mechanical but please test.
> 

ok

> -- 
> :wq Claudio
> 
> Index: bgpctl.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
> retrieving revision 1.258
> diff -u -p -r1.258 bgpctl.c
> --- bgpctl.c  24 Jan 2020 05:46:00 -  1.258
> +++ bgpctl.c  19 Mar 2020 17:36:18 -
> @@ -46,11 +46,6 @@
>  
>  int   main(int, char *[]);
>  int   show(struct imsg *, struct parse_result *);
> -void  show_attr(void *, u_int16_t, int);
> -void  show_communities(u_char *, size_t, int);
> -void  show_community(u_char *, u_int16_t);
> -void  show_large_community(u_char *, u_int16_t);
> -void  show_ext_community(u_char *, u_int16_t);
>  void  send_filterset(struct imsgbuf *, struct filter_set_head *);
>  void  show_mrt_dump_neighbors(struct mrt_rib *, struct mrt_peer *,
>   void *);
> @@ -425,7 +420,6 @@ show(struct imsg *imsg, struct parse_res
>   if (imsg->hdr.len < IMSG_HEADER_SIZE + sizeof(*kt))
>   errx(1, "wrong imsg len");
>   kt = imsg->data;
> -
>   show_fib_table(kt);
>   break;
>   case IMSG_CTL_SHOW_RIB:
> @@ -443,7 +437,7 @@ show(struct imsg *imsg, struct parse_res
>   warnx("bad IMSG_CTL_SHOW_RIB_COMMUNITIES received");
>   break;
>   }
> - show_communities(imsg->data, ilen, res->flags);
> + show_communities(imsg->data, ilen, res);
>   break;
>   case IMSG_CTL_SHOW_RIB_ATTR:
>   ilen = imsg->hdr.len - IMSG_HEADER_SIZE;
> @@ -451,7 +445,7 @@ show(struct imsg *imsg, struct parse_res
>   warnx("bad IMSG_CTL_SHOW_RIB_ATTR received");
>   break;
>   }
> - show_attr(imsg->data, ilen, res->flags);
> + show_attr(imsg->data, ilen, res);
>   break;
>   case IMSG_CTL_SHOW_RIB_MEM:
>   memcpy(, imsg->data, sizeof(stats));
> @@ -506,7 +500,7 @@ fmt_peer(const char *descr, const struct
>  }
>  
>  const char *
> -print_auth_method(enum auth_method method)
> +fmt_auth_method(enum auth_method method)
>  {
>   switch (method) {
>   case AUTH_MD5SIG:
> @@ -525,75 +519,6 @@ print_auth_method(enum auth_method metho
>   }
>  }
>  
> -void
> -print_neighbor_capa_mp(struct peer *p)
> -{
> - int comma;
> - u_int8_ti;
> -
> - for (i = 0, comma = 0; i < AID_MAX; i++)
> - if (p->capa.peer.mp[i]) {
> - printf("%s%s", comma ? ", " : "", aid2str(i));
> - comma = 1;
> - }
> -}
> -
> -void
> -print_neighbor_capa_restart(struct peer *p)
> -{
> - int comma;
> - u_int8_ti;
> -
> - if (p->capa.peer.grestart.timeout)
> - printf(": Timeout: %d, ", p->capa.peer.grestart.timeout);
> - for (i = 0, comma = 0; i < AID_MAX; i++)
> - if (p->capa.peer.grestart.flags[i] & CAPA_GR_PRESENT) {
> - if (!comma &&
> - p->capa.peer.grestart.flags[i] & CAPA_GR_RESTART)
> - printf("restarted, ");
> - if (comma)
> - printf(", ");
> - printf("%s", aid2str(i));
> - if (p->capa.peer.grestart.flags[i] & CAPA_GR_FORWARD)
> - printf(" (preserved)");
> - comma = 1;
> - }
> -}
> -
> -void
> -print_neighbor_msgstats(struct peer *p)
> -{
> - printf("  Message statistics:\n");
> - printf("  %-15s %-10s %-10s\n", "", "Sent", "Received");
> - printf("  %-15s %10llu %10llu\n", "Opens",
> - p->stats.msg_sent_open, p->stats.msg_rcvd_open);
> - printf("  %-15s %10llu %10llu\n", "Notifications",
> - p->stats.msg_sent_notification, p->stats.msg_rcvd_notification);
> - printf("  %-15s %10llu %10llu\n", "Updates",
> - p->stats.msg_sent_update, p->stats.msg_rcvd_update);
> - printf("  %-15s %10llu %10llu\n", "Keepalives",
> - p->stats.msg_sent_keepalive, p->stats.msg_rcvd_keepalive);
> - printf("  %-15s %10llu %10llu\n", "Route Refresh",
> - p->stats.msg_sent_rrefresh, p->stats.msg_rcvd_rrefresh);
> - printf("  %-15s %10llu %10llu\n\n", "Total",
> - p->stats.msg_sent_open + p->stats.msg_sent_notification +
> - p->stats.msg_sent_update + p->stats.msg_sent_keepalive +
> - p->stats.msg_sent_rrefresh,
> - p->stats.msg_rcvd_open + p->stats.msg_rcvd_notification +
> - p->stats.msg_rcvd_update + 

Re: regress: bgpd: config: Fix attribute ordering

2020-03-06 Thread Sebastian Benoit
I dont see that here.
Sure that you have an up-to-date tree?
And no diff in there?


Klemens Nanni(k...@openbsd.org) on 2020.03.05 23:39:20 +0100:
> 
> I ran bgpd to test diffs and stumbled across what looks like simple
> disorder in the config checks.
> 
> bgpd must have changed in how it orders attributes within `set { ... }'
> blocks;  breaking the sets into multiple lines and diffing line-wise
> instead of word-wise shows that the printed config indeed only differs
> in the way set attributes from the config appear in bgpd output.
> 
> Still, I'd like any of the bgpd hackers to verify before I "fix" regress.
> 
> OK?
> 
> 
> Index: regress/usr.sbin/bgpd/config/bgpd.conf.10.ok
> ===
> RCS file: /cvs/src/regress/usr.sbin/bgpd/config/bgpd.conf.10.ok,v
> retrieving revision 1.6
> diff -u -p -r1.6 bgpd.conf.10.ok
> --- regress/usr.sbin/bgpd/config/bgpd.conf.10.ok  17 Jul 2019 10:27:50 
> -  1.6
> +++ regress/usr.sbin/bgpd/config/bgpd.conf.10.ok  5 Mar 2020 22:32:53 
> -
> @@ -40,4 +40,4 @@ match from any large-community 1234:5678
>  match from any large-community 1234:5678:1 large-community 1234:5678:2 
> large-community 1234:5678:3 
>  match from any community 1234:1 large-community 1234:5678:1 
>  match from any large-community 1234:5678:1 community 1234:1 
> -match from any set { community delete 1234:5678 community delete 1234:* 
> community delete *:5678 community delete local-as:5678 community delete 
> local-as:neighbor-as large-community delete 1234:15:5678 large-community 
> delete *:15:5678 large-community delete local-as:15:5678 large-community 
> delete local-as:15:* large-community delete local-as:15:neighbor-as 
> large-community delete local-as:*:* community 1234:5678 community 
> local-as:5678 community local-as:neighbor-as large-community 1234:15:5678 
> large-community local-as:15:5678 large-community local-as:15:neighbor-as }
> +match from any set { community delete 1234:5678 large-community delete 
> 1234:15:5678 community delete *:5678 large-community delete *:15:5678 
> community delete local-as:5678 large-community delete local-as:15:5678 
> community delete 1234:* community delete local-as:neighbor-as large-community 
> delete local-as:15:* large-community delete local-as:*:* large-community 
> delete local-as:15:neighbor-as community 1234:5678 large-community 
> 1234:15:5678 community local-as:5678 large-community local-as:15:5678 
> community local-as:neighbor-as large-community local-as:15:neighbor-as }
> Index: regress/usr.sbin/bgpd/config/bgpd.conf.11.ok
> ===
> RCS file: /cvs/src/regress/usr.sbin/bgpd/config/bgpd.conf.11.ok,v
> retrieving revision 1.5
> diff -u -p -r1.5 bgpd.conf.11.ok
> --- regress/usr.sbin/bgpd/config/bgpd.conf.11.ok  17 Jul 2019 10:27:50 
> -  1.5
> +++ regress/usr.sbin/bgpd/config/bgpd.conf.11.ok  5 Mar 2020 22:35:04 
> -
> @@ -33,7 +33,7 @@ match from any ext-community ovs invalid
>  match from any ext-community ovs not-found 
>  match from any ext-community rt 64496:201 ext-community soo 64496:202 
>  match from any ext-community rt 64496:301 ext-community soo 420001:302 
> ext-community odi 127.0.0.1:303 
> -match from any set { ext-community delete ovs valid ext-community delete odi 
> 127.0.0.1:6003 ext-community delete soo 420001:6002 ext-community delete 
> ort 0x123456789abf ext-community delete rt 64496:6001 ext-community ovs valid 
> ext-community odi 127.0.0.1:5003 ext-community soo 420001:5002 
> ext-community ort 0x123456789abc ext-community rt 64496:5001 }
> +match from any set { ext-community delete ovs valid ext-community delete ort 
> 0x123456789abf ext-community delete rt 64496:6001 ext-community delete odi 
> 127.0.0.1:6003 ext-community delete soo 420001:6002 ext-community ovs 
> valid ext-community ort 0x123456789abc ext-community rt 64496:5001 
> ext-community odi 127.0.0.1:5003 ext-community soo 420001:5002 }
>  match from any ext-community * * 
>  match from any ext-community rt * 
>  match from any ext-community soo * 
> @@ -47,7 +47,7 @@ match from any ext-community rt 127.0.0.
>  match from any ext-community soo 64496:* 
>  match from any ext-community soo 420001:* 
>  match from any ext-community soo 127.0.0.1:* 
> -match from any set { ext-community delete odi 127.0.0.1:* ext-community 
> delete soo 420001:* ext-community delete rt 64496:* ext-community delete 
> mac-mob * ext-community delete ovs * ext-community delete rt * ext-community 
> delete soo * ext-community delete odi * ext-community delete ort * }
> +match from any set { ext-community delete ort * ext-community delete mac-mob 
> * ext-community delete ovs * ext-community delete rt * ext-community delete 
> soo * ext-community delete odi * ext-community delete rt 64496:* 
> ext-community delete odi 127.0.0.1:* ext-community delete soo 420001:* }
>  match from any 

Re: BIRD 1.x/2.x support at rpki-client

2020-03-06 Thread Sebastian Benoit
Robert Scheck(rob...@fedoraproject.org) on 2020.03.06 14:02:26 +0100:
> On Fri, 06 Mar 2020, Job Snijders wrote:
> > I believe Robert is referring to this snippet of code:
> > 
> > 
> > https://patch-diff.githubusercontent.com/raw/kristapsdz/rpki-client/pull/21.patch

Thanks for the patch.
I commited it with the function moved into output-bird.c

If you have more (feature) diffs, send them to tech@ too.

/Benno



Re: BIRD 1.x/2.x support at rpki-client

2020-03-06 Thread Sebastian Benoit
Job Snijders(j...@openbsd.org) on 2020.03.06 17:31:13 +:
> I have a small suggestion, in some deployments I saw the convention to
> name it as following so it is clear the data came from user provided
> data rather than internal bird structures 
> 
> I tested Benno's patch against BIRD 1.6.6 - wfm.

thanks.

I did not commit this bit, and i have not substantiated opinion on it.
If thats how bird users do it, commit it ;)

> 
> Index: main.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
> retrieving revision 1.58
> diff -u -p -r1.58 main.c
> --- main.c11 Feb 2020 18:41:39 -  1.58
> +++ main.c6 Mar 2020 17:25:56 -
> @@ -156,7 +156,7 @@ static void   build_chain(const struct aut
>  static void  build_crls(const struct auth *, struct crl_tree *,
>   STACK_OF(X509_CRL) **);
>  
> -const char   *bird_tablename = "roa";
> +const char   *bird_tablename = "ROAS";
>  
>  int   verbose;
>  
> 



Re: BIRD 1.x/2.x support at rpki-client

2020-03-06 Thread Sebastian Benoit
Hi,

generate 3 different outputs for BIRD:
- bird v1 with IPv4 routes
- bird v1 with IPv6 routes
- bird v2
when using command line option -B.
BIRD v2 output from Robert Scheck, robert AT fedoraproject DOT org


Note that I haven't tried this with bird 1 or 2 yet ;)
comments, oks?


(benno_rpki_bird.diff)

diff --git usr.sbin/rpki-client/extern.h usr.sbin/rpki-client/extern.h
index 127db60fa37..6592ea83b5e 100644
--- usr.sbin/rpki-client/extern.h
+++ usr.sbin/rpki-client/extern.h
@@ -374,7 +374,9 @@ FILE*output_createtmp(char *);
 voidoutput_cleantmp(void);
 voidoutput_finish(FILE *);
 int output_bgpd(FILE *, struct vrp_tree *);
-int output_bird(FILE *, struct vrp_tree *);
+int output_bird1v4(FILE *, struct vrp_tree *);
+int output_bird1v6(FILE *, struct vrp_tree *);
+int output_bird2(FILE *, struct vrp_tree *);
 int output_csv(FILE *, struct vrp_tree *);
 int output_json(FILE *, struct vrp_tree *);
 
diff --git usr.sbin/rpki-client/output-bird.c usr.sbin/rpki-client/output-bird.c
index a15faa69164..0299018f8af 100644
--- usr.sbin/rpki-client/output-bird.c
+++ usr.sbin/rpki-client/output-bird.c
@@ -1,6 +1,7 @@
 /* $OpenBSD: output-bird.c,v 1.6 2019/12/04 23:03:05 benno Exp $ */
 /*
  * Copyright (c) 2019 Claudio Jeker 
+ * Copyright (c) 2020 Robert Scheck 
  *
  * Permission to use, copy, modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -21,7 +22,7 @@
 #include "extern.h"
 
 int
-output_bird(FILE *out, struct vrp_tree *vrps)
+output_bird1v4(FILE *out, struct vrp_tree *vrps)
 {
extern  const char *bird_tablename;
char buf[64];
@@ -31,10 +32,78 @@ output_bird(FILE *out, struct vrp_tree *vrps)
return -1;
 
RB_FOREACH(v, vrp_tree, vrps) {
-   ip_addr_print(>addr, v->afi, buf, sizeof(buf));
-   if (fprintf(out, "\troa %s max %u as %u;\n", buf, v->maxlength,
-   v->asid) < 0)
+   if (v->afi == AFI_IPV4) {
+   ip_addr_print(>addr, v->afi, buf, sizeof(buf));
+   if (fprintf(out, "\troa %s max %u as %u;\n", buf,
+   v->maxlength, v->asid) < 0)
+   return -1;
+   }
+   }
+
+   if (fprintf(out, "}\n") < 0)
return -1;
+   return 0;
+}
+
+int
+output_bird1v6(FILE *out, struct vrp_tree *vrps)
+{
+   extern  const char *bird_tablename;
+   char buf[64];
+   struct vrp  *v;
+
+   if (fprintf(out, "roa table %s {\n", bird_tablename) < 0)
+   return -1;
+
+   RB_FOREACH(v, vrp_tree, vrps) {
+   if (v->afi == AFI_IPV6) {
+   ip_addr_print(>addr, v->afi, buf, sizeof(buf));
+   if (fprintf(out, "\troa %s max %u as %u;\n", buf,
+   v->maxlength, v->asid) < 0)
+   return -1;
+   }
+   }
+
+   if (fprintf(out, "}\n") < 0)
+   return -1;
+   return 0;
+}
+
+int
+output_bird2(FILE *out, struct vrp_tree *vrps)
+{
+   extern  const char *bird_tablename;
+   char buf[64];
+   struct vrp  *v;
+   time_t   now = time(NULL);
+
+   if (fprintf(out, "define force_roa_table_update = %lld;\n\n"
+   "roa4 table %s4;\nroa6 table %s6;\n\n"
+   "protocol static {\n\troa4 { table %s4; };\n\n",
+   (long long) now, bird_tablename, bird_tablename,
+   bird_tablename) < 0)
+   return -1;
+
+   RB_FOREACH(v, vrp_tree, vrps) {
+   if (v->afi == AFI_IPV4) {
+   ip_addr_print(>addr, v->afi, buf, sizeof(buf));
+   if (fprintf(out, "\troute %s max %u as %u;\n", buf,
+   v->maxlength, v->asid) < 0)
+   return -1;
+   }
+   }
+
+   if (fprintf(out, "}\n\nprotocol static {\n\troa6 { table %s6; };\n\n",
+   bird_tablename) < 0)
+   return -1;
+
+   RB_FOREACH(v, vrp_tree, vrps) {
+   if (v->afi == AFI_IPV6) {
+   ip_addr_print(>addr, v->afi, buf, sizeof(buf));
+   if (fprintf(out, "\troute %s max %u as %u;\n", buf,
+   v->maxlength, v->asid) < 0)
+   return -1;
+   }
}
 
if (fprintf(out, "}\n") < 0)
diff --git usr.sbin/rpki-client/output.c usr.sbin/rpki-client/output.c
index adafc5c0b53..b26b964eeaa 100644
--- usr.sbin/rpki-client/output.c
+++ usr.sbin/rpki-client/output.c
@@ -39,10 +39,12 @@ struct outputs {
char*name;
int (*fn)(FILE *, struct vrp_tree *);
 } outputs[] = {
-   { FORMAT_OPENBGPD, "openbgpd", output_bgpd },
- 

Re: BIRD 1.x/2.x support at rpki-client

2020-03-06 Thread Sebastian Benoit
Robert Scheck(rob...@fedoraproject.org) on 2020.03.06 14:02:26 +0100:
> On Fri, 06 Mar 2020, Job Snijders wrote:
> > I believe Robert is referring to this snippet of code:
> > 
> > 
> > https://patch-diff.githubusercontent.com/raw/kristapsdz/rpki-client/pull/21.patch
> 
> Exactly.

Ah, i thought it was some diff to bird! Thanks, i'll tkae care of it.

/Benno



Re: BIRD 1.x/2.x support at rpki-client

2020-03-06 Thread Sebastian Benoit
Robert Scheck(rob...@fedoraproject.org) on 2020.03.03 01:20:24 +0100:
> Hi,
> 
> job@ suggested to move this from GitHub to tech@ list (as upstream):
> 
> 1. Currently, BIRD 1.x support in rpki-client seems to be broken: As per
>BIRD upstream the "combined format" produced by rpki-client can't be
>used as-is with BIRD 1.x due to separated daemons (and configuration
>files) for IPv4 and IPv6.
> 2. Lack of BIRD 2.x support in rpki-client, which requires a different
>output/configuration format (semi-finished pull request at GitHub).

Hi, can you point me to that pull request, i could not find it.

/Benno

> 
> To cover this, job@ suggested to maybe generate bird1-ipv6, bird1-ipv4 and
> bird2 when using -B option. The option currently leads to "bird" file with
> BIRD 1.x support only.
> 
> However, I'm not sure if the current options -B, -c, -j and -o are that
> great. Maybe something like "-o " would be
> more powerful and more flexible?
> 
> Opinions?
> 
> 
> Regards,
>   Robert
> 



Re: minor bgpd cleanup

2020-02-14 Thread Sebastian Benoit
ok

Claudio Jeker(cje...@diehard.n-r-g.com) on 2020.02.14 14:06:37 +0100:
> Move and rename copy_filterset to rde_filter.c as filterset_copy.
> This way it matches the other filterset_* functions.
> 
> OK?
> -- 
> :wq Claudio
> 
> Index: bgpd.h
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.400
> diff -u -p -r1.400 bgpd.h
> --- bgpd.h12 Feb 2020 10:33:56 -  1.400
> +++ bgpd.h14 Feb 2020 12:16:43 -
> @@ -1182,8 +1182,6 @@ voidfree_prefixtree(struct prefixset_t
>  void filterlist_free(struct filter_head *);
>  int  host(const char *, struct bgpd_addr *, u_int8_t *);
>  u_int32_tget_bgpid(void);
> -void copy_filterset(struct filter_set_head *,
> - struct filter_set_head *);
>  void expand_networks(struct bgpd_config *);
>  int  prefixset_cmp(struct prefixset_item *, struct prefixset_item *);
>  RB_PROTOTYPE(prefixset_tree, prefixset_item, entry, prefixset_cmp);
> @@ -1261,10 +1259,10 @@ int   pftable_addr_remove(struct pftable_m
>  int  pftable_commit(void);
>  
>  /* rde_filter.c */
> -void  filterset_free(struct filter_set_head *);
> -int   filterset_cmp(struct filter_set *, struct filter_set *);
> -void  filterset_move(struct filter_set_head *,
> - struct filter_set_head *);
> +void filterset_free(struct filter_set_head *);
> +int  filterset_cmp(struct filter_set *, struct filter_set *);
> +void filterset_move(struct filter_set_head *, struct filter_set_head *);
> +void filterset_copy(struct filter_set_head *, struct filter_set_head *);
>  const char   *filterset_name(enum action_types);
>  
>  /* rde_sets.c */
> Index: config.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/config.c,v
> retrieving revision 1.94
> diff -u -p -r1.94 config.c
> --- config.c  28 Jan 2020 15:45:46 -  1.94
> +++ config.c  14 Feb 2020 12:21:44 -
> @@ -496,22 +496,6 @@ prepare_listeners(struct bgpd_config *co
>  }
>  
>  void
> -copy_filterset(struct filter_set_head *source, struct filter_set_head *dest)
> -{
> - struct filter_set   *s, *t;
> -
> - if (source == NULL)
> - return;
> -
> - TAILQ_FOREACH(s, source, entry) {
> - if ((t = malloc(sizeof(struct filter_set))) == NULL)
> - fatal(NULL);
> - memcpy(t, s, sizeof(struct filter_set));
> - TAILQ_INSERT_TAIL(dest, t, entry);
> - }
> -}
> -
> -void
>  expand_networks(struct bgpd_config *c)
>  {
>   struct network  *n, *m, *tmp;
> @@ -533,8 +517,7 @@ expand_networks(struct bgpd_config *c)
>   memcpy(>net.prefix, >p.addr,
>   sizeof(m->net.prefix));
>   m->net.prefixlen = psi->p.len;
> - TAILQ_INIT(>net.attrset);
> - copy_filterset(>net.attrset,
> + filterset_copy(>net.attrset,
>   >net.attrset);
>   TAILQ_INSERT_TAIL(nw, m, entry);
>   }
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.403
> diff -u -p -r1.403 parse.y
> --- parse.y   24 Jan 2020 05:44:05 -  1.403
> +++ parse.y   14 Feb 2020 12:21:59 -
> @@ -4076,8 +4076,7 @@ expand_rule(struct filter_rule *rule, st
>   memcpy(r, rule, sizeof(struct 
> filter_rule));
>   memcpy(>match, match,
>   sizeof(struct filter_match));
> - TAILQ_INIT(>set);
> - copy_filterset(set, >set);
> + filterset_copy(set, >set);
>  
>   if (rb != NULL)
>   strlcpy(r->rib, rb->name,
> Index: rde_filter.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde_filter.c,v
> retrieving revision 1.122
> diff -u -p -r1.122 rde_filter.c
> --- rde_filter.c  13 Aug 2019 12:16:20 -  1.122
> +++ rde_filter.c  14 Feb 2020 12:21:28 -
> @@ -502,6 +502,10 @@ filterset_cmp(struct filter_set *a, stru
>   return (0);
>  }
>  
> +/*
> + * move filterset from source to dest. dest will be initialized first.
> + * After the move source is an empty list.
> + */
>  void
>  filterset_move(struct filter_set_head *source, struct filter_set_head *dest)
>  {
> @@ -509,6 +513,26 @@ filterset_move(struct filter_set_head *s
>   if (source == NULL)
>   return;
>   TAILQ_CONCAT(dest, source, entry);
> +}
> +
> +/*
> + * copy filterset from source 

Re: mg: fix problems found by gcc 10

2020-02-09 Thread Sebastian Benoit
read ok

Florian Obser(flor...@openbsd.org) on 2020.02.09 10:46:34 +0100:
> Anyone? I'll commit this soon if I don't hear back, I don't think this
> is contentious.
> 
> On Fri, Feb 07, 2020 at 03:59:50PM +0100, Florian Obser wrote:
> > Moving from misc to tech.
> > 
> > This is effectively Ulrich's diff from github with a bit of whitespace 
> > shuffling.
> > 
> > OK?
> > 
> > diff --git def.h def.h
> > index d4f00e84e59..0db023973e0 100644
> > --- def.h
> > +++ def.h
> > @@ -337,7 +337,7 @@ void ttnowindow(void);
> >  voidttcolor(int);
> >  voidttresize(void);
> >  
> > -volatile sig_atomic_t winch_flag;
> > +extern volatile sig_atomic_t winch_flag;
> >  
> >  /* ttyio.c */
> >  voidttopen(void);
> > @@ -752,11 +752,7 @@ extern char cinfo[];
> >  extern char*keystrings[];
> >  extern char pat[NPAT];
> >  extern char prompt[];
> > -
> > -/*
> > - * Globals.
> > - */
> > -int tceeol;
> > -int tcinsl;
> > -int tcdell;
> > -int rptcount;  /* successive invocation count */
> > +extern int  tceeol;
> > +extern int  tcinsl;
> > +extern int  tcdell;
> > +extern int  rptcount;  /* successive invocation count */
> > diff --git kbd.c kbd.c
> > index 06d6c9fcf48..5f9b0a9efa6 100644
> > --- kbd.c
> > +++ kbd.c
> > @@ -26,13 +26,13 @@ char prompt[PROMPTL] = "", *promptp = prompt;
> >  
> >  static int mgwrap(PF, int, int);
> >  
> > -static int  use_metakey = TRUE;
> > -static int  pushed = FALSE;
> > -static int  pushedc;
> > +static int  use_metakey = TRUE;
> > +static int  pushed = FALSE;
> > +static int  pushedc;
> >  
> >  struct map_element *ele;
> > -
> > -struct key key;
> > +struct key  key;
> > +int rptcount;
> >  
> >  /*
> >   * Toggle the value of use_metakey
> > diff --git tty.c tty.c
> > index 0b64c4b5453..c378cb240dd 100644
> > --- tty.c
> > +++ tty.c
> > @@ -45,6 +45,11 @@ static const char*scroll_fwd;/* How to 
> > scroll forward. */
> >  
> >  static void winchhandler(int);
> >  
> > +volatile sig_atomic_t   winch_flag;
> > +int tceeol;
> > +int tcinsl;
> > +int tcdell;
> > +
> >  /* ARGSUSED */
> >  static void
> >  winchhandler(int sig)
> > 
> > 
> > On Tue, Feb 04, 2020 at 12:51:46AM +0100, Han Boetes wrote:
> > > The latest version of gcc is more picky about global variables resulting 
> > > in
> > > this bug report for my portable version of mg:
> > >   https://github.com/hboetes/mg/issues/12
> > > 
> > > To which Ulrich M?ller created a pull request which fixed the problem:
> > >   https://github.com/hboetes/mg/pull/13/files
> > > 
> > > Is this worth applying to the upstream branch?
> > 
> > -- 
> > I'm not entirely sure you are real.
> > 
> 
> -- 
> I'm not entirely sure you are real.
> 



Re: mention /etc/examples/ in bgpf.conf(5)/bgpd(8)

2020-02-08 Thread Sebastian Benoit
Ingo Schwarze(schwa...@usta.de) on 2020.02.09 00:33:06 +0100:
> Hi,
> 
> Jason McIntyre wrote on Sat, Feb 08, 2020 at 10:15:08PM +:
> 
> > - i'm ok with adding the path to these files to a FILES section
> 
> So, here is a specific patch for bgpf.conf(5) and bgpd(8) such
> that we can agree on a general direction for one case where
> the example file is particularly important.
> 
> Once the direction is agreed, applying it to the relatively
> small number of other cases is likely not difficult.
> 
> OK?
>   Ingo

I'm not against this as it only adds to the manpage, but an
alternative could be to just tell users (in afterboot(8)?) that there is
/etc/examples for them to look at and see whats there?

Otherwise ok.


> Index: bgpd.8
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.8,v
> retrieving revision 1.62
> diff -u -p -r1.62 bgpd.8
> --- bgpd.810 Nov 2019 20:51:53 -  1.62
> +++ bgpd.88 Feb 2020 23:24:28 -
> @@ -205,11 +205,13 @@ Only check the configuration file for va
>  Produce more verbose output.
>  .El
>  .Sh FILES
> -.Bl -tag -width "/var/run/bgpd.sockXXX" -compact
> +.Bl -tag -width "/etc/examples/bgpd.conf" -compact
>  .It Pa /etc/bgpd.conf
>  default
>  .Nm
>  configuration file
> +.It Pa /etc/examples/bgpd.conf
> +example file, do not edit
>  .It Pa /var/run/bgpd.sock
>  default
>  .Nm
> Index: bgpd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.conf.5,v
> retrieving revision 1.199
> diff -u -p -r1.199 bgpd.conf.5
> --- bgpd.conf.5   25 Jan 2020 12:07:28 -  1.199
> +++ bgpd.conf.5   8 Feb 2020 23:24:28 -
> @@ -1883,10 +1883,12 @@ For prefixes with equally long paths, th
>  is selected.
>  .El
>  .Sh FILES
> -.Bl -tag -width "/etc/bgpd.conf" -compact
> +.Bl -tag -width "/etc/examples/bgpd.conf" -compact
>  .It Pa /etc/bgpd.conf
>  .Xr bgpd 8
>  configuration file
> +.It Pa /etc/examples/bgpd.conf
> +example file, do not edit
>  .El
>  .Sh SEE ALSO
>  .Xr strftime 3 ,
> 



Re: httpd(8): patch to allow FastCGI chroots in sub-directories

2020-02-08 Thread Sebastian Benoit
ok

Florian Obser(flor...@openbsd.org) on 2020.02.07 16:49:08 +0100:
> Slightly tweaked diff by me, fixing "new sentence new line" in the man
> page.
> 
> This is OK florian@ if someone wants to commit it or I can commit it
> if someone OKs it.
> 
> diff --git httpd.conf.5 httpd.conf.5
> index f4ea2e55766..494271672ea 100644
> --- httpd.conf.5
> +++ httpd.conf.5
> @@ -300,6 +300,12 @@ Alternatively if
>  the FastCGI handler is listening on a TCP socket,
>  .Ar socket
>  starts with a colon followed by the TCP port number.
> +.It Ic strip Ar number
> +Strip
> +.Ar number
> +path components from the beginning of DOCUMENT_ROOT and
> +SCRIPT_FILENAME before sending them to the FastCGI server.
> +This allows FastCGI server chroot to be a directory under httpd chroot.
>  .It Ic param Ar variable value
>  Sets a variable that will be sent to the FastCGI server.
>  Each statement defines one variable.
> diff --git httpd.h httpd.h
> index b1f17af8cd7..b22586974a5 100644
> --- httpd.h
> +++ httpd.h
> @@ -547,6 +547,7 @@ struct server_config {
>   uint8_t  hsts_flags;
>  
>   struct server_fcgiparams fcgiparams;
> + int  fcgistrip;
>  
>   TAILQ_ENTRY(server_config) entry;
>  };
> diff --git parse.y parse.y
> index 054302269f4..109efd36a9f 100644
> --- parse.y
> +++ parse.y
> @@ -689,6 +689,13 @@ fcgiflags: SOCKET STRING {
>   param->name, param->value);
>   TAILQ_INSERT_HEAD(_conf->fcgiparams, param, entry);
>   }
> + | STRIP NUMBER  {
> + if ($2 < 0 || $2 > INT_MAX) {
> + yyerror("invalid fastcgi strip number");
> + YYERROR;
> + }
> + srv_conf->fcgistrip = $2;
> + }
>   ;
>  
>  connection   : CONNECTION '{' optnl conflags_l '}'
> diff --git server_fcgi.c server_fcgi.c
> index 864ce6b16d5..a85b5b44804 100644
> --- server_fcgi.c
> +++ server_fcgi.c
> @@ -241,7 +241,8 @@ server_fcgi(struct httpd *env, struct client *clt)
>   errstr = "failed to encode param";
>   goto fail;
>   }
> - if (fcgi_add_param(, "SCRIPT_FILENAME", script, clt) == -1) {
> + if (fcgi_add_param(, "SCRIPT_FILENAME", server_root_strip(script,
> + srv_conf->fcgistrip), clt) == -1) {
>   errstr = "failed to encode param";
>   goto fail;
>   }
> @@ -257,8 +258,8 @@ server_fcgi(struct httpd *env, struct client *clt)
>   goto fail;
>   }
>  
> - if (fcgi_add_param(, "DOCUMENT_ROOT", srv_conf->root,
> - clt) == -1) {
> + if (fcgi_add_param(, "DOCUMENT_ROOT", server_root_strip(
> + srv_conf->root, srv_conf->fcgistrip), clt) == -1) {
>   errstr = "failed to encode param";
>   goto fail;
>   }
> 
> 
> On Sat, Jan 18, 2020 at 07:19:33AM +0100, Nazar Zhuk wrote:
> > On Tue, Jan 14, 2020 at 03:07:05PM +0100, Florian Obser wrote:
> > > I like the idea. Unfortunately the diff does not apply.
> > Looks like I had formatting issues there. This should apply cleanly now.
> > 
> > 
> > Index: usr.sbin/httpd/httpd.conf.5
> > ===
> > RCS file: /cvs/src/usr.sbin/httpd/httpd.conf.5,v
> > retrieving revision 1.107
> > diff -u -p -u -r1.107 httpd.conf.5
> > --- usr.sbin/httpd/httpd.conf.5 8 May 2019 21:46:56 -   1.107
> > +++ usr.sbin/httpd/httpd.conf.5 17 Jan 2020 06:20:14 -
> > @@ -300,6 +300,10 @@ Alternatively if
> >  the FastCGI handler is listening on a TCP socket,
> >  .Ar socket
> >  starts with a colon followed by the TCP port number.
> > +.It Ic strip Ar number
> > +Strip
> > +.Ar number
> > +path components from the beginning of DOCUMENT_ROOT and SCRIPT_FILENAME 
> > before sending them to the FastCGI server. This allows FastCGI server 
> > chroot to be a directory under httpd chroot.
> >  .It Ic param Ar variable value
> >  Sets a variable that will be sent to the FastCGI server.
> >  Each statement defines one variable.
> > Index: usr.sbin/httpd/httpd.h
> > ===
> > RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
> > retrieving revision 1.145
> > diff -u -p -u -r1.145 httpd.h
> > --- usr.sbin/httpd/httpd.h  8 May 2019 19:57:45 -   1.145
> > +++ usr.sbin/httpd/httpd.h  17 Jan 2020 06:20:14 -
> > @@ -547,6 +547,7 @@ struct server_config {
> > uint8_t  hsts_flags;
> >  
> > struct server_fcgiparams fcgiparams;
> > +   int  fcgistrip;
> >  
> > TAILQ_ENTRY(server_config) entry;
> >  };
> > Index: usr.sbin/httpd/parse.y
> > ===
> > RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> > retrieving revision 1.113
> > diff -u -p -u -r1.113 parse.y
> > --- usr.sbin/httpd/parse.y  28 Jun 

Re: Teach du(1) the -m flag, disk usage in megabytes

2020-01-29 Thread Sebastian Benoit
Lauri Tirkkonen(la...@hacktheplanet.fi) on 2020.01.29 01:31:56 +0200:
> On Tue, Jan 28 2020 18:03:19 +0100, Florian Obser wrote:
> > On Tue, Jan 28, 2020 at 09:58:40AM -0700, Todd C. Miller wrote:
> > > On Mon, 27 Jan 2020 18:29:39 -0500, Daniel Jakots wrote:
> > > 
> > > > Can't you achieve what you want with `du -sh * | sort -h`? du(1)'s -h
> > > > options will automatically select the best suffix and sort(1)'s -h
> > > > will sort first using the suffix then the numerical value.
> > > 
> > > Yes, I forgot about "sort -h".  Old habits die hard :-)
> > 
> > ... which is not in posix, netbsd nor illumos.
> 
> So, do you think that 'du -m' will be in all those then? POSIX doesn't
> have it [0].
> 
> The way I see it, the entire conversation in this thread is about doing
> things that might be useful to people. IMO, arguing about where
> extensions are or aren't implemented isn't productive.

Yes it is. We try to avoid adding options whereever we can, because

* every option letter we grab thats not used for the same purpose elsewhere
  can crate problems down the road
* more options means more bugs

B

> 
> [0]: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/du.html
> 
> -- 
> Lauri Tirkkonen | lotheac @ IRCnet
> 



Re: usr.sbin/snmpd: use TAILQ_CONCAT(3)

2020-01-27 Thread Sebastian Benoit
Bj??rn Ketelaars(bjorn.ketela...@hydroxide.nl) on 2020.01.27 20:53:52 +0100:
> Replace custom TAILQ concatenation loop by TAILQ_CONCAT(3).
> 
> Comments/OK?

reads ok benno@

> diff --git usr.sbin/snmpd/control.c usr.sbin/snmpd/control.c
> index 54b58bbb7b6..dda18c1bad5 100644
> --- usr.sbin/snmpd/control.c
> +++ usr.sbin/snmpd/control.c
> @@ -487,10 +487,7 @@ control_dispatch_agentx(int fd, short event, void *arg)
>   TAILQ_INSERT_TAIL(, miboid, o_list);
>   } while (++oid.bo_id[rhdr.subrange] <= ubound);
>  
> - while ((miboid = TAILQ_FIRST()) != NULL) {
> - TAILQ_REMOVE(, miboid, o_list);
> - TAILQ_INSERT_TAIL(>oids, miboid, o_list);
> - }
> + TAILQ_CONCAT(>oids, , o_list);
>   dodone:
>   break;
>   }
> 



Re: sbin/unwind: use TAILQ_CONCAT(3)

2020-01-27 Thread Sebastian Benoit
Bj??rn Ketelaars(bjorn.ketela...@hydroxide.nl) on 2020.01.27 20:52:36 +0100:
> Replace custom TAILQ concatenation loop by TAILQ_CONCAT(3).
> 
> Comments/OK?

reads ok benno@

> diff --git sbin/unwind/frontend.c sbin/unwind/frontend.c
> index b64036c4332..d2b69084db7 100644
> --- sbin/unwind/frontend.c
> +++ sbin/unwind/frontend.c
> @@ -1011,10 +1011,7 @@ merge_tas(struct trust_anchor_head *newh, struct 
> trust_anchor_head *oldh)
>  
>   if (chg) {
>   free_tas(oldh);
> - while((i = TAILQ_FIRST(newh)) != NULL) {
> - TAILQ_REMOVE(newh, i, entry);
> - TAILQ_INSERT_TAIL(oldh, i, entry);
> - }
> + TAILQ_CONCAT(oldh, newh, entry);
>   } else {
>   free_tas(newh);
>   }
> diff --git sbin/unwind/resolver.c sbin/unwind/resolver.c
> index 15d2c90b1e8..c12bdbdab26 100644
> --- sbin/unwind/resolver.c
> +++ sbin/unwind/resolver.c
> @@ -1650,10 +1650,7 @@ replace_forwarders(struct uw_forwarder_head *new_list, 
> struct
>   free(uw_forwarder);
>   }
>  
> - while ((uw_forwarder = TAILQ_FIRST(new_list)) != NULL) {
> - TAILQ_REMOVE(new_list, uw_forwarder, entry);
> - TAILQ_INSERT_TAIL(old_list, uw_forwarder, entry);
> - }
> + TAILQ_CONCAT(old_list, new_list, entry);
>  }
>  
>  int
> 



Re: usr.sbin/bgpd: use TAILQ_CONCAT(3)

2020-01-27 Thread Sebastian Benoit
Bj??rn Ketelaars(bjorn.ketela...@hydroxide.nl) on 2020.01.27 20:53:06 +0100:
> Replace custom TAILQ concatenation loop by TAILQ_CONCAT(3).
> 
> Comments/OK?

ok benno@

> diff --git usr.sbin/bgpd/config.c usr.sbin/bgpd/config.c
> index cb43afb81fe..fc81a3efd3b 100644
> --- usr.sbin/bgpd/config.c
> +++ usr.sbin/bgpd/config.c
> @@ -195,7 +195,6 @@ void
>  merge_config(struct bgpd_config *xconf, struct bgpd_config *conf)
>  {
>   struct listen_addr  *nla, *ola, *next;
> - struct network  *n;
>   struct peer *p, *np, *nextp;
>  
>   /*
> @@ -250,10 +249,7 @@ merge_config(struct bgpd_config *xconf, struct 
> bgpd_config *conf)
>  
>   /* switch the network statements, but first remove the old ones */
>   free_networks(>networks);
> - while ((n = TAILQ_FIRST(>networks)) != NULL) {
> - TAILQ_REMOVE(>networks, n, entry);
> - TAILQ_INSERT_TAIL(>networks, n, entry);
> - }
> + TAILQ_CONCAT(>networks, >networks, entry);
>  
>   /* switch the l3vpn configs, first remove the old ones */
>   free_l3vpns(>l3vpns);
> 



Re: Teach du(1) the -m flag, disk usage in megabytes

2020-01-27 Thread Sebastian Benoit
Florian Obser(flor...@openbsd.org) on 2020.01.27 19:57:41 +0100:
> On Mon, Jan 27, 2020 at 10:33:49AM -0700, Todd C. Miller wrote:
> > On Mon, 27 Jan 2020 10:05:41 +1100, Jonathan Gray wrote:
> > 
> > > On Sun, Jan 26, 2020 at 11:59:33AM -0500, David Goerger wrote:
> > > > This diff teaches du(1) the -m flag, report disk usage in megabytes. 
> > > > This brings us in line with implementations in the other BSDs, Linux, 
> > > > and Illumos.
> > >
> > > Why is it needed?  -k is required by POSIX, adding arguments for
> > > megabytes, gigabytes, terabytes, petabytes etc seems silly when
> > > there is already 512 byte blocks, kilobytes and -h output.
> > 
> > It is useful in conjunction with sort.  For example, I often do:
> > 
> > du -sk * | sort -rn | head
> > 
> > to see the largest disk users.
> 
> Me, too. Given its wide spread adoption I think we should implement it.
> OK florian@
> 
> > 
> > However, output in kilobytes is less useful than it used to be due
> > to larger files now being common.  Using the BLOCKSIZE env var is
> > more flexible but is cumbersome to use and not portable.
> > 
> >  - todd

Every time my laptop disk gets full i use a horrible awk construct
to make things readable. I would certainly like to see this.

Do we need -B for that? I dont think so...

I'm happy to commit this if nobody complains ;)



Re: Teach du(1) the -m flag, disk usage in megabytes

2020-01-26 Thread Sebastian Benoit
Maybe the manpage text could be better, but i'll leave that to jmc@

ok benno@

David Goerger(da...@goerger.info) on 2020.01.26 11:59:33 -0500:
> This diff teaches du(1) the -m flag, report disk usage in megabytes. 
> This brings us in line with implementations in the other BSDs, Linux, 
> and Illumos.
> 
> Other base utilities where this flag might be useful include df(1)
> and quot(8), although it doesn't appear to be universally adopted
> among the other implementations. That said I can definitely cook
> up a diff if others would find the flag useful elsewhere. In
> particular I think the flag would be useful in quot(8), but that
> tool has an unfortunate legacy, discouraged option "-h" which
> conflicts with -k/-m/-h semantics elsewhere in base, such that
> adding "-m" to quot(8) might only invite confusion.
> 
> Many thanks to florian@ for a first-pass review on bsd.network, and
> for encouraging me to check out what the other BSDs and utilities in
> base do, so that we maintain consistency across the ecosystem.
> 
> This is my first patch submission---any pointers for improvement would 
> be greatly appreciated! Thanks!
> 
> (diff below and also attached as "du-megabytes.diff" in case my mail
> client mangles formatting; hopefully I got this right!)
> 
> ---
> 
> Index: du.1
> ===
> RCS file: /cvs/src/usr.bin/du/du.1,v
> retrieving revision 1.35
> diff -u -p -r1.35 du.1
> --- du.1  2 Sep 2019 21:18:41 -   1.35
> +++ du.1  25 Jan 2020 20:52:11 -
> @@ -38,7 +38,7 @@
>  .Nd display disk usage statistics
>  .Sh SYNOPSIS
>  .Nm du
> -.Op Fl achkrsx
> +.Op Fl achkmrsx
>  .Op Fl H | L | P
>  .Op Fl d Ar depth
>  .Op Ar
> @@ -86,6 +86,10 @@ By default, all sizes are reported in 51
>  The
>  .Fl k
>  option causes the numbers to be reported in kilobyte counts.
> +.It Fl m
> +Similar to
> +.Fl k ,
> +but report disk usage in megabytes.
>  .It Fl L
>  All symbolic links are followed.
>  .It Fl P
> Index: du.c
> ===
> RCS file: /cvs/src/usr.bin/du/du.c,v
> retrieving revision 1.32
> diff -u -p -r1.32 du.c
> --- du.c  24 Aug 2016 03:13:45 -  1.32
> +++ du.c  25 Jan 2020 20:52:31 -
> @@ -61,7 +61,7 @@ main(int argc, char *argv[])
>   long blocksize;
>   int64_t totalblocks;
>   int ftsoptions, listfiles, maxdepth;
> - int Hflag, Lflag, cflag, hflag, kflag;
> + int Hflag, Lflag, cflag, hflag, kflag, mflag;
>   int ch, notused, rval;
>   char **save;
>   const char *errstr;
> @@ -70,11 +70,11 @@ main(int argc, char *argv[])
>   err(1, "pledge");
> 
>   save = argv;
> - Hflag = Lflag = cflag = hflag = kflag = listfiles = 0;
> + Hflag = Lflag = cflag = hflag = kflag = listfiles = mflag = 0;
>   totalblocks = 0;
>   ftsoptions = FTS_PHYSICAL;
>   maxdepth = -1;
> - while ((ch = getopt(argc, argv, "HLPacd:hkrsx")) != -1)
> + while ((ch = getopt(argc, argv, "HLPacd:hkmrsx")) != -1)
>   switch (ch) {
>   case 'H':
>   Hflag = 1;
> @@ -103,10 +103,17 @@ main(int argc, char *argv[])
>   case 'h':
>   hflag = 1;
>   kflag = 0;
> + mflag = 0;
>   break;
>   case 'k':
>   kflag = 1;
>   hflag = 0;
> + mflag = 0;
> + break;
> + case 'm':
> + kflag = 0;
> + hflag = 0;
> + mflag = 1;
>   break;
>   case 's':
>   maxdepth = 0;
> @@ -155,6 +162,8 @@ main(int argc, char *argv[])
>   blocksize = 512;
>   else if (kflag)
>   blocksize = 1024;
> + else if (mflag)
> + blocksize = 1048576;
>   else
>   (void)getbsize(, );
>   blocksize /= 512;
> @@ -320,6 +329,6 @@ usage(void)
>  {
> 
>   (void)fprintf(stderr,
> - "usage: du [-achkrsx] [-H | -L | -P] [-d depth] [file ...]\n");
> + "usage: du [-achkmrsx] [-H | -L | -P] [-d depth] [file ...]\n");
>   exit(1);
>  }

> Index: du.1
> ===
> RCS file: /cvs/src/usr.bin/du/du.1,v
> retrieving revision 1.35
> diff -u -p -r1.35 du.1
> --- du.1  2 Sep 2019 21:18:41 -   1.35
> +++ du.1  25 Jan 2020 20:52:11 -
> @@ -38,7 +38,7 @@
>  .Nd display disk usage statistics
>  .Sh SYNOPSIS
>  .Nm du
> -.Op Fl achkrsx
> +.Op Fl achkmrsx
>  .Op Fl H | L | P
>  .Op Fl d Ar depth
>  .Op Ar
> @@ -86,6 +86,10 @@ By default, all sizes are reported in 51
>  The
>  .Fl k
>  option causes the numbers to be reported in kilobyte counts.
> +.It Fl m
> +Similar to
> +.Fl k ,
> +but report disk usage in megabytes.
>  .It Fl L
>  All symbolic links are followed.
>  .It 

Re: ospf6d: simplify lsa_snap()

2020-01-23 Thread Sebastian Benoit
Remi Locherer(remi.loche...@relo.ch) on 2020.01.22 06:49:53 +0100:
> On Wed, Jan 22, 2020 at 12:56:00AM +0100, Claudio Jeker wrote:
> > On Tue, Jan 21, 2020 at 03:58:58PM +0100, Remi Locherer wrote:
> > > On Tue, Jan 21, 2020 at 01:09:30PM +0100, Denis Fondras wrote:
> > > > On Tue, Jan 21, 2020 at 09:35:06AM +0100, Remi Locherer wrote:
> > > > > > @@ -235,6 +233,7 @@ lsa_check(struct rde_nbr *nbr, struct ls
> > > > > > case LSA_TYPE_NETWORK:
> > > > > > if ((len % sizeof(u_int32_t)) ||
> > > > > > len < sizeof(lsa->hdr) + sizeof(u_int32_t)) {
> > > > > > +   log_warnx("lsa_check: bad LSA network packet");
> > > > > 
> > > > > please use __func__
> > > > > 
> > > > 
> > > > None use __func__ currently.
> > > > 
> > > 
> > > Right, it's not often used in ospf6d.
> > > 
> > > I think we should use it more in such cases.
> > > 
> > > But you have my OK with or without that.
> > > 
> > 
> > I think the log_warnx should use __func__ less and instead use better
> > messages that an operator can understand without having to check the code.
> > As a developer 'lsa_check: bad LSA network packet' sounds great since I
> > can find the code but as an operator 'dropped LSA network packet with bad
> > size from neighbor XY' would be more effective. I'm probably the source of
> > most of those messages that's why I think they could be better :)
> > But changing those can happen some other time.
> 
> I agree with that point. But when the function name is used in a message
> I prefer if __func__ is used.

Another consideration: if you have the same message in multiple functions,
please use it so we know which one it was.



Re: carp: send only IPv4 carp packets on dual stack interface

2020-01-18 Thread Sebastian Benoit
chr...@openbsd.org(chr...@openbsd.org) on 2020.01.18 06:18:21 +0100:
> On Wed, Jan 15, 2020 at 12:47:28PM +0100, Sebastian Benoit wrote:
> >Christopher Zimmermann(chr...@openbsd.org) on 2020.01.15 11:55:43 +0100:
> >>Hi,
> >>
> >>as far as I can see a dual stack carp interface does not care whether it
> >>receives advertisements addressed to IPv4 or IPv6. Any one will do.
> >>So I propose to send IPv6 advertisements only when IPv4 is not possible.
> >>
> >>Why?
> >>
> >>- Noise can be reduced by using unicast advertisements.
> >>  This is only possible for IPv4 by ``ifconfig carppeer``.
> >>  I don't like flooding the whole network with carp advertisements when
> >>  I may also unicast them.
> >
> >Maybe i'm getting confused, but in the problem description you were talking
> >about v6 vs v4, and here you argue about unicast (vs multicast?) being
> >better. Thats orthogonal, isnt it?
> 
> Yes, kind of. The point is we support ``carppeer`` for IPv4, but not for 
> IPv6.
> 
> >>- breaking IPv6 connectivity (for example by running iked without -6)
> >>  will start a preempt-war, because failing ip6_output will cause the
> >>  demote counter to be increased. That's what hit me.
> >
> >But the whole point of carp is to notice broken connectivity. If you run v6
> >on an interface, you want to know if its working, no?
> 
> I grant you that much. But what kind of failures do you hope to detect 
> on the _sending_ carp master, that would not also affect the backup?

sure: misconfigured pf. Missing routes. Buggy switch. 
 
> >At the very least, this needs some more thought and testing in all the ways
> >carp can be configured.
> 
> Anyway, my main concern indeed is the broadcast noise generated by carp 
> and I would be equally happy if we had a ``carppeer6`` option. Would 
> that be considered?

of course carppeer should work with v6, and as claudio says without an extra
keyword in ifconfig, but thats a trivial detail.



Re: carp: send only IPv4 carp packets on dual stack interface

2020-01-15 Thread Sebastian Benoit
Christopher Zimmermann(chr...@openbsd.org) on 2020.01.15 11:55:43 +0100:
> Hi,
> 
> as far as I can see a dual stack carp interface does not care whether it 
> receives advertisements addressed to IPv4 or IPv6. Any one will do.
> So I propose to send IPv6 advertisements only when IPv4 is not possible.
> 
> Why?
> 
> - Noise can be reduced by using unicast advertisements.
>   This is only possible for IPv4 by ``ifconfig carppeer``.
>   I don't like flooding the whole network with carp advertisements when 
>   I may also unicast them.

Maybe i'm getting confused, but in the problem description you were talking
about v6 vs v4, and here you argue about unicast (vs multicast?) being
better. Thats orthogonal, isnt it?

> - breaking IPv6 connectivity (for example by running iked without -6) 
>   will start a preempt-war, because failing ip6_output will cause the 
>   demote counter to be increased. That's what hit me.

But the whole point of carp is to notice broken connectivity. If you run v6
on an interface, you want to know if its working, no?

At the very least, this needs some more thought and testing in all the ways
carp can be configured.

> I would suggest a change like this:
> 
> Index: ip_carp.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_carp.c,v
> retrieving revision 1.342
> diff -u -p -r1.342 ip_carp.c
> --- ip_carp.c 8 Nov 2019 07:51:41 -   1.342
> +++ ip_carp.c 15 Jan 2020 10:45:56 -
> @@ -1175,7 +1175,7 @@ carp_send_ad(struct carp_vhost_entry *vh
>   }
>   }
>  #ifdef INET6
> - if (sc->sc_naddrs6) {
> + else if (sc->sc_naddrs6) {
>   struct ip6_hdr *ip6;
>  
>   MGETHDR(m, M_DONTWAIT, MT_HEADER);
> 
> 
> one could also use a slightly smaller hammer and only avoid sending IPv6 
> if the user requested an IPv4 unicast address:
> 
> - if (sc->sc_naddrs6) {
> + if (sc->sc_naddrs6 &&
> + (! sc->sc_naddrs ||
> + sc->sc_peer.s_addr == INADDR_CARP_GROUP) ) {
> 
> 
> Christopher
> 
> 
> -- 
> http://gmerlin.de
> OpenPGP: http://gmerlin.de/christopher.pub
> CB07 DA40 B0B6 571D 35E2  0DEF 87E2 92A7 13E5 DEE1
> 



Re: in httpd, use the correct configured server config

2020-01-14 Thread Sebastian Benoit
Thanks for the diff, commited.

Sebastian Benoit(be...@openbsd.org) on 2020.01.14 21:14:44 +0100:
> seems sensible.
> 
> ok benno@
> 
> 
> Tracey Emery(tra...@traceyemery.net) on 2020.01.14 13:08:03 -0700:
> > Hello,
> > 
> > In the server_response function of httpd, the if comparison to
> > srv_conf->maxrequests is using the wrong value. The value is derived from 
> > the
> > first server configuration in httpd.conf, since we still don't know which 
> > server
> > name the client is requesting.
> > 
> > This small diff moves srv_conf->maxrequests usage in server_response to
> > where we finally know which configured server config to actually use.
> > This ensures we are not using the first configured server config in the
> > queue.
> > 
> > Thanks,
> > 
> > Tracey
> > 
> > -- 
> > 
> > Tracey Emery
> > 
> > diff e80014c68b2561493718bbcef6e7fcb172d7f885 /usr/src
> > blob - 326daa6a687ff7159adc744f66b38e6ea9e266eb
> > file + usr.sbin/httpd/server_http.c
> > --- usr.sbin/httpd/server_http.c
> > +++ usr.sbin/httpd/server_http.c
> > @@ -1232,13 +1232,6 @@ server_response(struct httpd *httpd, struct client 
> > *cl
> > clt->clt_persist = 0;
> > }
> >  
> > -   if (clt->clt_persist >= srv_conf->maxrequests)
> > -   clt->clt_persist = 0;
> > -
> > -   /* pipelining should end after the first "idempotent" method */
> > -   if (clt->clt_pipelining && clt->clt_toread > 0)
> > -   clt->clt_persist = 0;
> > -
> > /*
> >  * Do we have a Host header and matching configuration?
> >  * XXX the Host can also appear in the URL path.
> > @@ -1291,6 +1284,13 @@ server_response(struct httpd *httpd, struct client 
> > *cl
> > goto fail;
> > srv_conf = clt->clt_srv_conf;
> > }
> > +
> > +   if (clt->clt_persist >= srv_conf->maxrequests)
> > +   clt->clt_persist = 0;
> > +
> > +   /* pipelining should end after the first "idempotent" method */
> > +   if (clt->clt_pipelining && clt->clt_toread > 0)
> > +   clt->clt_persist = 0;
> >  
> > if ((desc->http_host = strdup(hostname)) == NULL)
> > goto fail;
> > 
> 



Re: in httpd, use the correct configured server config

2020-01-14 Thread Sebastian Benoit
seems sensible.

ok benno@


Tracey Emery(tra...@traceyemery.net) on 2020.01.14 13:08:03 -0700:
> Hello,
> 
> In the server_response function of httpd, the if comparison to
> srv_conf->maxrequests is using the wrong value. The value is derived from the
> first server configuration in httpd.conf, since we still don't know which 
> server
> name the client is requesting.
> 
> This small diff moves srv_conf->maxrequests usage in server_response to
> where we finally know which configured server config to actually use.
> This ensures we are not using the first configured server config in the
> queue.
> 
> Thanks,
> 
> Tracey
> 
> -- 
> 
> Tracey Emery
> 
> diff e80014c68b2561493718bbcef6e7fcb172d7f885 /usr/src
> blob - 326daa6a687ff7159adc744f66b38e6ea9e266eb
> file + usr.sbin/httpd/server_http.c
> --- usr.sbin/httpd/server_http.c
> +++ usr.sbin/httpd/server_http.c
> @@ -1232,13 +1232,6 @@ server_response(struct httpd *httpd, struct client *cl
>   clt->clt_persist = 0;
>   }
>  
> - if (clt->clt_persist >= srv_conf->maxrequests)
> - clt->clt_persist = 0;
> -
> - /* pipelining should end after the first "idempotent" method */
> - if (clt->clt_pipelining && clt->clt_toread > 0)
> - clt->clt_persist = 0;
> -
>   /*
>* Do we have a Host header and matching configuration?
>* XXX the Host can also appear in the URL path.
> @@ -1291,6 +1284,13 @@ server_response(struct httpd *httpd, struct client *cl
>   goto fail;
>   srv_conf = clt->clt_srv_conf;
>   }
> +
> + if (clt->clt_persist >= srv_conf->maxrequests)
> + clt->clt_persist = 0;
> +
> + /* pipelining should end after the first "idempotent" method */
> + if (clt->clt_pipelining && clt->clt_toread > 0)
> + clt->clt_persist = 0;
>  
>   if ((desc->http_host = strdup(hostname)) == NULL)
>   goto fail;
> 



Re: iked(8): get rid of IPv6 flow and -6 flag?

2020-01-14 Thread Sebastian Benoit
Alexander Bluhm(alexander.bl...@gmx.net) on 2020.01.13 18:19:31 +0100:
> On Mon, Jan 13, 2020 at 05:55:06PM +0100, Tobias Heider wrote:
> > I think we should discuss whether we can remove the flow
> > (and the -6 flag) as I constantly hear people complaining
> > that it broke their setups and I don't think anyone
> > expects some seemingly unrelated program breaking IPv6.
> 
> A missing -6 flag on the iked command line, is a very unexpected
> way to break your IPv6 setup.  So we should remove that.
> 
> OK bluhm@
> 
> If there is demand for such a feature, we could create an option
> in the example/iked.conf that shows how to disable IPv6.
> And perhaps one to disable IPv4 for the IPv6 hipser :-)

I'm ok with getting rid of it, but as kn@ suggests please with disable and
warning. Please add comment /* XXX remove during OpenBSD 6.7-current */
A current.html note is needed.



Re: ospf(6)d.conf: define interface parameters per area or globally

2020-01-12 Thread Sebastian Benoit
Theo de Raadt(dera...@openbsd.org) on 2020.01.12 12:03:40 -0700:
> Remi Locherer  wrote:
> 
> > On Sun, Jan 12, 2020 at 04:18:26PM +0100, Claudio Jeker wrote:
> > > On Sun, Jan 12, 2020 at 03:46:15PM +0100, Remi Locherer wrote:
> > > > On Wed, Jan 08, 2020 at 01:13:45PM +0100, Denis Fondras wrote:
> > > > > On Wed, Jan 08, 2020 at 09:14:48AM +0100, Remi Locherer wrote:
> > > > > > > I have a diff to allow parameters after interface or area 
> > > > > > > definition.
> > > > > > > Not sure if we want to do that though.
> > > > > > 
> > > > > > I would appreciate that! ;-)
> > > > > > 
> > > > > 
> > > > > The ospfd diff needs some more work. Crypt authentication handling is 
> > > > > not
> > > > > perfect.
> > > > 
> > > > This works fine for me and the diff reads good. I tested ospfd and 
> > > > ospf6d.
> > > > Also the crypt options for ospfd worked fine.
> > > > 
> > > > ok remi@
> > > 
> > > Currently all daemons behave the same way and inherit at the moment of
> > > creation. Having this behave different between daemons is confusing.
> > > At least ospfd and bgpd should behave the same. Not saying that the
> > > current behaviour is great.
> > > I think in the case of ospfd the way auth-md is handled by this diff is
> > > not comparable with the behaviour of the other settings.
> > 
> > I agree. But that should not stop us improving one program before the
> > other ones.
> > 
> > > 
> > > area 0.0.0.0 {
> > >   hello-interval 10
> > >   auth-md 1 foo
> > > 
> > >   interface em0
> > > 
> > >   hello-interval 20
> > >   auth-md 1 bar
> > >   auth-md 2 foofoo
> > > 
> > >   interface em1 {
> > >   auth-md 3 barbar
> > >   }
> > > 
> > >   hello-interval 30
> > >   auth-md 1 bay
> > >   auth-md 2 foobar
> > > }
> > > 
> > > What values for hello-interval and auth-md should be set on em0 and em1?
> > >  
> > 
> > To me it looks natural if the latest value per level is used. With your
> > example that would be:
> > 
> > em0:
> > - auth-md 1 "bay"
> > - auth-md 2 "foobar"
> > - hello-interval 30
> > 
> > em1:
> > - auth-md 1 "bay"
> > - auth-md 2 "foobar"
> > - auth-md 3 "barbar"
> > - hello-interval 30
> > 
> > In my testing this is the result of the diff from Denis. (I modified
> > printconf.c to print the keys to see the results).
> 
> I think that is very dangerous.  In some other daemons it could be
> disastrous.
>
> 
> > Another option would be to make it an error specifying the same option
> > more than once at the same level.
> 
> I think that will be the easier solution.

I too think that should be an error.
Why should you specify it twice?
 
> The approach of "collect all the root info first, then apply to the
> children aftwards" will be difficult to apply to all our
> domain-specific-grammer-daemons.

We should keep it the same in the routing daemons. I think the rest is
different here and there already.

/Benno



Re: small bgpd performance improvement

2020-01-09 Thread Sebastian Benoit
nice, ok

Claudio Jeker(cje...@diehard.n-r-g.com) on 2020.01.09 16:25:58 +0100:
> The path_hash function is called reasonably often. Calling
> SipHash24_Update() over and over for small data is not optimal.
> Inspired by /sys/sys/proc.h add a aspath_hashstart and aspath_hashend to
> the struct rde_aspath and use that for the hash.
> 
> -- 
> :wq Claudio
> 
> Index: rde.h
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
> retrieving revision 1.230
> diff -u -p -r1.230 rde.h
> --- rde.h 9 Jan 2020 13:31:52 -   1.230
> +++ rde.h 9 Jan 2020 15:12:47 -
> @@ -215,14 +215,16 @@ struct rde_aspath {
>   struct attr **others;
>   struct aspath   *aspath;
>   u_int64_thash;
> + int  refcnt;
>   u_int32_tflags; /* internally used */
> +#define  aspath_hashstartmed
>   u_int32_tmed;   /* multi exit disc */
>   u_int32_tlpref; /* local pref */
>   u_int32_tweight;/* low prio lpref */
> - int  refcnt;
>   u_int16_trtlabelid; /* route label id */
>   u_int16_tpftableid; /* pf table id */
>   u_int8_t origin;
> +#define  aspath_hashend  others_len
>   u_int8_t others_len;
>  };
>  
> Index: rde_rib.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
> retrieving revision 1.212
> diff -u -p -r1.212 rde_rib.c
> --- rde_rib.c 9 Jan 2020 11:55:25 -   1.212
> +++ rde_rib.c 9 Jan 2020 15:14:27 -
> @@ -701,12 +701,8 @@ path_hash(struct rde_aspath *asp)
>   u_int64_t   hash;
>  
>   SipHash24_Init(, );
> - SipHash24_Update(, >origin, sizeof(asp->origin));
> - SipHash24_Update(, >med, sizeof(asp->med));
> - SipHash24_Update(, >lpref, sizeof(asp->lpref));
> - SipHash24_Update(, >weight, sizeof(asp->weight));
> - SipHash24_Update(, >rtlabelid, sizeof(asp->rtlabelid));
> - SipHash24_Update(, >pftableid, sizeof(asp->pftableid));
> + SipHash24_Update(, >aspath_hashstart,
> + (char *)>aspath_hashend - (char *)>aspath_hashstart);
>  
>   if (asp->aspath)
>   SipHash24_Update(, asp->aspath->data, asp->aspath->len);
> 



Re: bgpd sofreconfigure and export default

2020-01-09 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2020.01.09 16:05:06 +0100:
> When using 'export default-route' you still need an output filter to allow
> the default route out. I'm probably not the only one forgetting this fact
> from time to time. Now to make things worse adding the filter rule to
> allow the route plus config reload does not work since the softreconfigure
> code does not handle the export default-route case correctly.

ok
 
> The following diff fixes this. There is still the problem that changing
> the 'export default-route' setting itself needs a session reset but that
> one is less easy to tackle :(
>
> -- 
> :wq Claudio
> 
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.498
> diff -u -p -r1.498 rde.c
> --- rde.c 9 Jan 2020 13:31:52 -   1.498
> +++ rde.c 9 Jan 2020 14:42:37 -
> @@ -3221,11 +3221,25 @@ rde_softreconfig_in_done(void *arg, u_in
>   }
>  
>   LIST_FOREACH(peer, , peer_l) {
> - if (peer->reconf_out)
> - rib_byid(peer->loc_rib_id)->state = RECONF_RELOAD;
> - else if (peer->reconf_rib) {
> - u_int8_t aid;
> + u_int8_t aid;
>  
> + if (peer->reconf_out) {
> + if (peer->conf.export_type == EXPORT_NONE) {
> + /* nothing to do here */
> + peer->reconf_out = 0;
> + } else if (peer->conf.export_type ==
> + EXPORT_DEFAULT_ROUTE) {
> + /* just resend the default route */
> + for (aid = 0; aid < AID_MAX; aid++) {
> + if (peer->capa.mp[aid])
> + up_generate_default(out_rules,
> + peer, aid);
> + }
> + peer->reconf_out = 0;
> + } else
> + rib_byid(peer->loc_rib_id)->state =
> + RECONF_RELOAD;
> + } else if (peer->reconf_rib) {
>   /* dump the full table to neighbors that changed rib */
>   for (aid = 0; aid < AID_MAX; aid++) {
>   if (peer->capa.mp[aid])
> 



Re: bgpd, move peer related functions to rde_peer.c

2020-01-09 Thread Sebastian Benoit


ok

Claudio Jeker(cje...@diehard.n-r-g.com) on 2020.01.09 13:03:13 +0100:
> On Thu, Jan 09, 2020 at 09:42:39AM +0100, Claudio Jeker wrote:
> > This diff just moves some of the code from rde.c to rde_peer.c where it
> > should be. Apart from moving the code peer_down() was modified so that it
> > can be used as callback of peer_foreach(). Also peer_foreach() was changed
> > to just walk the peer list instead of traversing the peer hash table.
> > 
> 
> Updated version after last commit.
> 
> -- 
> :wq Claudio
> 
> ? obj
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.497
> diff -u -p -r1.497 rde.c
> --- rde.c 9 Jan 2020 11:55:25 -   1.497
> +++ rde.c 9 Jan 2020 12:01:51 -
> @@ -20,12 +20,10 @@
>   */
>  
>  #include 
> -#include 
>  #include 
>  #include 
>  
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -63,9 +61,6 @@ int  rde_get_mp_nexthop(u_char *, u_int
>struct filterstate *);
>  void  rde_update_err(struct rde_peer *, u_int8_t , u_int8_t,
>void *, u_int16_t);
> -void  rde_update_log(const char *, u_int16_t,
> -  const struct rde_peer *, const struct bgpd_addr *,
> -  const struct bgpd_addr *, u_int8_t);
>  void  rde_as4byte_fixup(struct rde_peer *, struct rde_aspath *);
>  void  rde_reflector(struct rde_peer *, struct rde_aspath *);
>  
> @@ -94,20 +89,8 @@ voidrde_mark_prefixsets_dirty(struct 
>  u_int8_t  rde_roa_validity(struct rde_prefixset *,
>struct bgpd_addr *, u_int8_t, u_int32_t);
>  
> -void  peer_init(u_int32_t);
> -void  peer_shutdown(void);
> -void  peer_foreach(void (*)(struct rde_peer *, void *), void *);
> -int   peer_imsg_pending(void);
> -int   peer_localaddrs(struct rde_peer *, struct bgpd_addr *);
> -struct rde_peer *peer_match(struct ctl_neighbor *, u_int32_t);
> -struct rde_peer  *peer_add(u_int32_t, struct peer_config *);
> -void  peer_up(struct rde_peer *, struct session_up *);
> -void  peer_down(struct rde_peer *);
> -void  peer_flush(struct rde_peer *, u_int8_t, time_t);
> -void  peer_stale(struct rde_peer *, u_int8_t);
> -void  peer_dump(struct rde_peer *, u_int8_t);
> -static void   peer_recv_eor(struct rde_peer *, u_int8_t);
> -static void   peer_send_eor(struct rde_peer *, u_int8_t);
> +static void   rde_peer_recv_eor(struct rde_peer *, u_int8_t);
> +static void   rde_peer_send_eor(struct rde_peer *, u_int8_t);
>  
>  void  network_add(struct network_config *, struct filterstate *);
>  void  network_delete(struct network_config *);
> @@ -115,13 +98,10 @@ static void   network_dump_upcall(struct 
>  static void   network_flush_upcall(struct rib_entry *, void *);
>  
>  void  rde_shutdown(void);
> -int   sa_cmp(struct bgpd_addr *, struct sockaddr *);
>  int   ovs_match(struct prefix *, u_int32_t);
>  
>  volatile sig_atomic_t rde_quit = 0;
>  struct bgpd_config   *conf, *nconf;
> -struct rde_peer_head  peerlist;
> -struct rde_peer  *peerself;
>  struct filter_head   *out_rules, *out_rules_tmp;
>  struct imsgbuf   *ibuf_se;
>  struct imsgbuf   *ibuf_se_ctl;
> @@ -129,6 +109,9 @@ struct imsgbuf*ibuf_main;
>  struct rde_memstats   rdemem;
>  int   softreconfig;
>  
> +extern struct rde_peer_head   peerlist;
> +extern struct rde_peer   *peerself;
> +
>  struct rde_dump_ctx {
>   LIST_ENTRY(rde_dump_ctx)entry;
>   struct ctl_show_rib_request req;
> @@ -990,10 +973,14 @@ rde_dispatch_imsg_peer(struct rde_peer *
>   if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(sup))
>   fatalx("incorrect size of session request");
>   memcpy(, imsg.data, sizeof(sup));
> - peer_up(peer, );
> + if (peer_up(peer, ) == -1) {
> + peer->state = PEER_DOWN;
> + imsg_compose(ibuf_se, IMSG_SESSION_DOWN, peer->conf.id,
> + 0, -1, NULL, 0);
> + }
>   break;
>   case IMSG_SESSION_DOWN:
> - peer_down(peer);
> + peer_down(peer, NULL);
>   break;
>   case IMSG_SESSION_STALE:
>   case IMSG_SESSION_FLUSH:
> @@ -1088,7 +1075,7 @@ rde_update_dispatch(struct rde_peer *pee
>   }
>   if (withdrawn_len == 0) {
>   /* EoR marker */
> - peer_recv_eor(peer, AID_INET);
> + rde_peer_recv_eor(peer, AID_INET);
>   return;
>   }
>   }
> @@ -1200,7 +1187,7 @@ rde_update_dispatch(struct rde_peer *pee
>   if ((state.aspath.flags & ~F_ATTR_MP_UNREACH) == 0 &&
>  

Re: snmpd(8): filter pf table addresses

2020-01-02 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2019.12.31 11:44:07 +0100:
> On Tue, Dec 31, 2019 at 11:16:37AM +0100, Martijn van Duren wrote:
> > I'm on the fence about this. So if you feel strongly about this go
> > ahead if it works.
> 
> In some regard I agree but in this case I think it makes sense.

I think it even is useful if snmpd were more efficient in retrieving the
data from pf. It just is a waste of cpu and bandwith to transfer these is
you know in advance that you dont need them.

> > I am however somewhat confused about your description. You say it
> > times out, but considering it (by default) only retrieves 10 entries
> > per packet it should return somewhat quick and not cause a timeout.
> > If you want to move through it quicker you can increase the amount
> > retrieved per request by setting -CrX where X can be any value.
> > I reckon -Cr40 is a decent number.
> 
> I doubt this will help. Looking at the code it seems that snmpd will fetch
> the full pftable from the kernel for each request which is probably where
> the inefficency and timeout happen. Also it does a linear search over
> Florian's 300'000 addresses to pick up the next address.
> 
> In short the pftable code and probably also the rest of the pf code in
> snmpd is not built to scale. There is no caching, no fast lookups, no
> smart paging implemented. This code needs major work to be usable.
> 
> I think the diff by Florian is OK and should go in.

+1

ok benno@



Re: bgpd, fairer imsg processing

2019-12-31 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2019.12.30 18:35:28 +0100:
> The imsg processing in the RDE is sometimes a bit unfair. The problem is
> that peers sending many UPDATES starve out the others especially on
> intial table dumps. This comes from the fact that imsg are processed to
> completion and so the system sending more just gets more CPU.
> 
> This diff changes this and moves peer specific imsgs to a per peer imsg
> queue where messages are enqueued and later dequeued one at a time.
> This results in a quicker main poll loop since less work is done per read
> operation. Also on startup peers finish their initial sync relative to
> their table size. Additionally a big peer flapping will have less
> influence at the processing speed of other updates or withdraws.

ok benno@
 
> This can be further optimized but lets keep the diffs small for a change

thanks ;)

> -- 
> :wq Claudio
> 
> Index: Makefile
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/Makefile,v
> retrieving revision 1.35
> diff -u -p -r1.35 Makefile
> --- Makefile  17 Jun 2019 11:02:19 -  1.35
> +++ Makefile  30 Dec 2019 12:30:18 -
> @@ -5,7 +5,7 @@ SRCS= bgpd.c session.c log.c logmsg.c pa
>   rde.c rde_rib.c rde_decide.c rde_prefix.c mrt.c kroute.c control.c \
>   pfkey.c rde_update.c rde_attr.c rde_community.c printconf.c \
>   rde_filter.c rde_sets.c rde_trie.c pftable.c name2id.c \
> - util.c carp.c timer.c
> + util.c carp.c timer.c rde_peer.c
>  CFLAGS+= -Wall -I${.CURDIR}
>  CFLAGS+= -Wstrict-prototypes -Wmissing-prototypes
>  CFLAGS+= -Wmissing-declarations
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.493
> diff -u -p -r1.493 rde.c
> --- rde.c 16 Dec 2019 10:35:02 -  1.493
> +++ rde.c 30 Dec 2019 14:01:58 -
> @@ -49,6 +49,7 @@
>  void  rde_sighdlr(int);
>  void  rde_dispatch_imsg_session(struct imsgbuf *);
>  void  rde_dispatch_imsg_parent(struct imsgbuf *);
> +void  rde_dispatch_imsg_peer(struct rde_peer *, void *);
>  void  rde_update_dispatch(struct rde_peer *, struct imsg *);
>  int   rde_update_update(struct rde_peer *, struct filterstate *,
>struct bgpd_addr *, u_int8_t);
> @@ -95,6 +96,8 @@ u_int8_t rde_roa_validity(struct rde_pr
>  
>  void  peer_init(u_int32_t);
>  void  peer_shutdown(void);
> +void  peer_foreach(void (*)(struct rde_peer *, void *), void *);
> +int   peer_imsg_pending(void);
>  int   peer_localaddrs(struct rde_peer *, struct bgpd_addr *);
>  struct rde_peer *peer_match(struct ctl_neighbor *, u_int32_t);
>  struct rde_peer  *peer_add(u_int32_t, struct peer_config *);
> @@ -266,7 +269,7 @@ rde_main(int debug, int verbose)
>   }
>  
>   if (rib_dump_pending() || rde_update_queue_pending() ||
> - nexthop_pending())
> + nexthop_pending() || peer_imsg_pending())
>   timeout = 0;
>  
>   if (poll(pfd, i, timeout) == -1) {
> @@ -305,6 +308,7 @@ rde_main(int debug, int verbose)
>   mctx = LIST_NEXT(mctx, entry);
>   }
>  
> + peer_foreach(rde_dispatch_imsg_peer, NULL);
>   rib_dump_runner();
>   nexthop_runner();
>   if (ibuf_se && ibuf_se->w.queued < SESS_MSG_HIGH_MARK) {
> @@ -358,7 +362,6 @@ rde_dispatch_imsg_session(struct imsgbuf
>   struct imsg  imsg;
>   struct peer  p;
>   struct peer_config   pconf;
> - struct session_upsup;
>   struct ctl_show_rib  csr;
>   struct ctl_show_rib_request req;
>   struct rde_peer *peer;
> @@ -370,7 +373,6 @@ rde_dispatch_imsg_session(struct imsgbuf
>   size_t   aslen;
>   int  verbose;
>   u_int16_tlen;
> - u_int8_t aid;
>  
>   while (ibuf) {
>   if ((n = imsg_get(ibuf, )) == -1)
> @@ -380,106 +382,24 @@ rde_dispatch_imsg_session(struct imsgbuf
>  
>   switch (imsg.hdr.type) {
>   case IMSG_UPDATE:
> - if ((peer = peer_get(imsg.hdr.peerid)) == NULL) {
> - log_warnx("rde_dispatch: unknown peer id %d",
> - imsg.hdr.peerid);
> - break;
> - }
> - rde_update_dispatch(peer, );
> - break;
> - case IMSG_SESSION_ADD:
> - if (imsg.hdr.len - IMSG_HEADER_SIZE != sizeof(pconf))
> - fatalx("incorrect size of session request");
> - memcpy(, imsg.data, sizeof(pconf));
> - peer_add(imsg.hdr.peerid, );
> - 

Re: LIST_FOREACH_SAFE macro for mnt_vnodelist

2019-12-25 Thread Sebastian Benoit
Alexander Bluhm(alexander.bl...@gmx.net) on 2019.12.24 23:11:12 +0100:
> Hi,
> 
> Use FOREACH macro for mnt_vnodelist.
> 
> ok?

ok benno@

> 
> bluhm
> 
> Index: nfs/nfs_subs.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/nfs/nfs_subs.c,v
> retrieving revision 1.139
> diff -u -p -r1.139 nfs_subs.c
> --- nfs/nfs_subs.c30 Nov 2018 09:24:57 -  1.139
> +++ nfs/nfs_subs.c24 Dec 2019 21:54:49 -
> @@ -1515,10 +1515,9 @@ nfs_clearcommit(struct mount *mp)
> 
>   s = splbio();
>  loop:
> - for (vp = LIST_FIRST(>mnt_vnodelist); vp != NULL; vp = nvp) {
> + LIST_FOREACH_SAFE(vp, >mnt_vnodelist, v_mntvnodes, nvp) {
>   if (vp->v_mount != mp)  /* Paranoia */
>   goto loop;
> - nvp = LIST_NEXT(vp, v_mntvnodes);
>   LIST_FOREACH_SAFE(bp, >v_dirtyblkhd, b_vnbufs, nbp) {
>   if ((bp->b_flags & (B_BUSY | B_DELWRI | B_NEEDCOMMIT))
>   == (B_DELWRI | B_NEEDCOMMIT))
> 



Re: ospf6d: rename & move function

2019-12-22 Thread Sebastian Benoit
Denis Fondras(open...@ledeuns.net) on 2019.12.22 10:55:39 +0100:
> Rename and move calc_nexthop_clear()/calc_nexthop_add() to
> vertex_nexthop_clear()/vertex_nexthop_add()
> 
> It brings ospf6d closer to ospfd.

ok

> 
> 
> Index: rde.h
> ===
> RCS file: /cvs/src/usr.sbin/ospf6d/rde.h,v
> retrieving revision 1.22
> diff -u -p -r1.22 rde.h
> --- rde.h 1 Jul 2010 19:47:04 -   1.22
> +++ rde.h 18 Dec 2019 10:03:27 -
> @@ -140,6 +140,9 @@ void   orig_intra_area_prefix_lsas(struc
>  void  lsa_init(struct lsa_tree *);
>  int   lsa_compare(struct vertex *, struct vertex *);
>  void  vertex_free(struct vertex *);
> +void  vertex_nexthop_clear(struct vertex *);
> +void  vertex_nexthop_add(struct vertex *, struct vertex *,
> + const struct in6_addr *, u_int32_t);
>  int   lsa_newer(struct lsa_hdr *, struct lsa_hdr *);
>  int   lsa_check(struct rde_nbr *, struct lsa *, u_int16_t);
>  int   lsa_self(struct lsa *);
> Index: rde_lsdb.c
> ===
> RCS file: /cvs/src/usr.sbin/ospf6d/rde_lsdb.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 rde_lsdb.c
> --- rde_lsdb.c18 Oct 2013 11:16:52 -  1.38
> +++ rde_lsdb.c18 Dec 2019 10:03:27 -
> @@ -99,9 +99,39 @@ void
>  vertex_free(struct vertex *v)
>  {
>   RB_REMOVE(lsa_tree, v->lsa_tree, v);
> +
>   (void)evtimer_del(>ev);
> + vertex_nexthop_clear(v);
>   free(v->lsa);
>   free(v);
> +}
> +
> +void
> +vertex_nexthop_clear(struct vertex *v)
> +{
> + struct v_nexthop*vn;
> +
> + while ((vn = TAILQ_FIRST(>nexthop))) {
> + TAILQ_REMOVE(>nexthop, vn, entry);
> + free(vn);
> + }
> +}
> +
> +void
> +vertex_nexthop_add(struct vertex *dst, struct vertex *parent,
> +const struct in6_addr *nexthop, u_int32_t ifindex)
> +{
> + struct v_nexthop*vn;
> +
> + if ((vn = calloc(1, sizeof(*vn))) == NULL)
> + fatal("vertex_nexthop_add");
> +
> + vn->prev = parent;
> + if (nexthop)
> + vn->nexthop = *nexthop;
> + vn->ifindex = ifindex;
> +
> + TAILQ_INSERT_TAIL(>nexthop, vn, entry);
>  }
>  
>  /* returns -1 if a is older, 1 if newer and 0 if equal to b */
> Index: rde_spf.c
> ===
> RCS file: /cvs/src/usr.sbin/ospf6d/rde_spf.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 rde_spf.c
> --- rde_spf.c 5 Dec 2015 06:45:19 -   1.25
> +++ rde_spf.c 18 Dec 2019 10:03:27 -
> @@ -36,9 +36,6 @@ RB_PROTOTYPE(rt_tree, rt_node, entry, rt
>  RB_GENERATE(rt_tree, rt_node, entry, rt_compare)
>  struct vertex*spf_root = NULL;
>  
> -void  calc_nexthop_clear(struct vertex *);
> -void  calc_nexthop_add(struct vertex *, struct vertex *,
> -  const struct in6_addr *, u_int32_t);
>  struct in6_addr  *calc_nexthop_lladdr(struct vertex *, struct 
> lsa_rtr_link *,
>unsigned int);
>  void  calc_nexthop_transit_nbr(struct vertex *, struct vertex *,
> @@ -142,7 +139,7 @@ spf_calc(struct area *area)
>   continue;
>   if (d < w->cost) {
>   w->cost = d;
> - calc_nexthop_clear(w);
> + vertex_nexthop_clear(w);
>   calc_nexthop(w, v, area, rtr_link);
>   /*
>* need to readd to candidate list
> @@ -156,7 +153,7 @@ spf_calc(struct area *area)
>   } else if (w->cost == LS_INFINITY && d < LS_INFINITY) {
>   w->cost = d;
>  
> - calc_nexthop_clear(w);
> + vertex_nexthop_clear(w);
>   calc_nexthop(w, v, area, rtr_link);
>   cand_list_add(w);
>   }
> @@ -420,26 +417,25 @@ asext_calc(struct vertex *v)
>   }
>  
>   area.s_addr = 0;
> - calc_nexthop_clear(v);
> + vertex_nexthop_clear(v);
>   TAILQ_FOREACH(rn, >nexthop, entry) {
>   if (rn->invalid)
>   continue;
>  
>   if (rn->connected && r->d_type == DT_NET) {
>   if (metric & LSA_ASEXT_F_FLAG)
> - calc_nexthop_add(v, NULL, _addr,
> + vertex_nexthop_add(v, NULL, _addr,
>   rn->ifindex);
>   else
>   fatalx("asext_calc: I'm sorry Dave, "
> 

Re: bgpctl: split out show functions into own file

2019-12-20 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2019.12.20 08:24:34 +0100:
> This diff just moves most show related functions into a new file.
> It is mostly mechanical (remove function from bgpctl.c and add it to
> output.c).
> 
> OK?

ok

did you check that bgplg etc still build (i dont see why they should not).

> -- 
> :wq Claudio
> 
> ? obj
> Index: Makefile
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/Makefile,v
> retrieving revision 1.15
> diff -u -p -r1.15 Makefile
> --- Makefile  25 Jun 2019 07:44:20 -  1.15
> +++ Makefile  20 Dec 2019 07:20:26 -
> @@ -3,7 +3,7 @@
>  .PATH:   ${.CURDIR}/../bgpd
>  
>  PROG=bgpctl
> -SRCS=bgpctl.c parser.c mrtparser.c util.c
> +SRCS=bgpctl.c output.c parser.c mrtparser.c util.c
>  CFLAGS+= -Wall
>  CFLAGS+= -Wstrict-prototypes -Wmissing-prototypes
>  CFLAGS+= -Wmissing-declarations
> Index: bgpctl.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
> retrieving revision 1.253
> diff -u -p -r1.253 bgpctl.c
> --- bgpctl.c  20 Dec 2019 07:18:51 -  1.253
> +++ bgpctl.c  20 Dec 2019 07:20:27 -
> @@ -2,6 +2,7 @@
>  
>  /*
>   * Copyright (c) 2003 Henning Brauer 
> + * Copyright (c) 2004-2019 Claudio Jeker 
>   * Copyright (c) 2016 Job Snijders 
>   * Copyright (c) 2016 Peter Hessler 
>   *
> @@ -38,49 +39,20 @@
>  #include "bgpd.h"
>  #include "session.h"
>  #include "rde.h"
> +
> +#include "bgpctl.h"
>  #include "parser.h"
>  #include "mrtparser.h"
>  
> -enum neighbor_views {
> - NV_DEFAULT,
> - NV_TIMERS
> -};
> -
> -#define EOL0(flag)   ((flag & F_CTL_SSV) ? ';' : '\n')
> -
>  int   main(int, char *[]);
>  int   show(struct imsg *, struct parse_result *);
> -char *fmt_peer(const char *, const struct bgpd_addr *, int);
> -void  show_summary(struct peer *);
> -void  show_neighbor_full(struct peer *, struct parse_result *);
> -void  show_neighbor(struct peer *, struct parse_result *res);
> -void  print_neighbor_capa_mp(struct peer *);
> -void  print_neighbor_capa_restart(struct peer *);
> -void  print_neighbor_msgstats(struct peer *);
>  void  print_timer(const char *, time_t);
> -const char   *fmt_timeframe(time_t t);
> -void  show_fib_flags(u_int16_t);
> -void  show_fib(struct kroute_full *);
> -void  show_fib_table(struct ktable *);
> -void  show_nexthop(struct ctl_show_nexthop *);
> -void  show_interface(struct ctl_show_interface *);
> -void  print_prefix(struct bgpd_addr *, u_int8_t, u_int8_t, u_int8_t);
> -const char *  print_origin(u_int8_t, int);
> -const char *  print_ovs(u_int8_t, int);
> -void  print_flags(u_int8_t, int);
> -void  show_rib(struct ctl_show_rib *, u_char *, size_t,
> - struct parse_result *);
> -void  show_rib_brief(struct ctl_show_rib *, u_char *, size_t);
> -void  show_rib_detail(struct ctl_show_rib *, u_char *, size_t, int);
>  void  show_attr(void *, u_int16_t, int);
>  void  show_communities(u_char *, size_t, int);
>  void  show_community(u_char *, u_int16_t);
>  void  show_large_community(u_char *, u_int16_t);
>  void  show_ext_community(u_char *, u_int16_t);
> -void  show_rib_mem(struct rde_memstats *);
> -void  show_rib_hash(struct rde_hashstats *);
>  void  send_filterset(struct imsgbuf *, struct filter_set_head *);
> -const char   *get_errstr(u_int8_t, u_int8_t);
>  void  show_mrt_dump_neighbors(struct mrt_rib *, struct mrt_peer *,
>   void *);
>  void  show_mrt_dump(struct mrt_rib *, struct mrt_peer *, void *);
> @@ -89,10 +61,7 @@ voidshow_mrt_state(struct mrt_bgp_sta
>  void  show_mrt_msg(struct mrt_bgp_msg *, void *);
>  const char   *msg_type(u_int8_t);
>  void  network_bulk(struct parse_result *);
> -const char   *print_auth_method(enum auth_method);
>  int   match_aspath(void *, u_int16_t, struct filter_as *);
> -void  show_head(struct parse_result *);
> -void  show_result(u_int);
>  
>  struct imsgbuf   *ibuf;
>  struct mrt_parser show_mrt = { show_mrt_dump, show_mrt_state, show_mrt_msg };
> @@ -537,46 +506,6 @@ fmt_peer(const char *descr, const struct
>   return (p);
>  }
>  
> -void
> -show_summary(struct peer *p)
> -{
> - char*s;
> - const char  *a;
> - size_t  alen;
> -
> - s = fmt_peer(p->conf.descr, >conf.remote_addr,
> - p->conf.remote_masklen);
> -
> - a = log_as(p->conf.remote_as);
> - alen = strlen(a);
> - /* max displayed length of the peers name is 28 */
> - if (alen < 28) {
> - if (strlen(s) > 28 - alen)
> - s[28 - alen] = '\0';
> - } else
> - alen = 0;

Re: ospf6d: rework priority handling

2019-12-15 Thread Sebastian Benoit
reads ok.

unrelated to this diff: I wonder if the manpage (of both ospfd and pspf6d)
should mention that changing fib-priority with a reload is equivalent toa
uncouple/couple?


Denis Fondras(open...@ledeuns.net) on 2019.12.15 09:56:15 +0100:
> 
> Index: kroute.c
> ===
> RCS file: /cvs/src/usr.sbin/ospf6d/kroute.c,v
> retrieving revision 1.61
> diff -u -p -r1.61 kroute.c
> --- kroute.c  12 Dec 2019 08:21:34 -  1.61
> +++ kroute.c  15 Dec 2019 08:42:10 -
> @@ -97,10 +97,11 @@ RB_PROTOTYPE(kroute_tree, kroute_node, e
>  RB_GENERATE(kroute_tree, kroute_node, entry, kroute_compare)
>  
>  int
> -kr_init(int fs, u_int rdomain, u_int8_t fib_prio)
> +kr_init(int fs, u_int rdomain, int redis_label_or_prefix, u_int8_t fib_prio)
>  {
>   int opt = 0, rcvbuf, default_rcvbuf;
>   socklen_t   optlen;
> + int filter_prio = fib_prio;
>  
>   kr_state.fib_sync = fs;
>   kr_state.rdomain = rdomain;
> @@ -117,6 +118,18 @@ kr_init(int fs, u_int rdomain, u_int8_t 
>   , sizeof(opt)) == -1)
>   log_warn("kr_init: setsockopt");/* not fatal */
>  
> + if (redis_label_or_prefix) {
> + filter_prio = 0;
> + log_info("%s: priority filter disabled", __func__);
> + } else
> + log_debug("%s: priority filter enabled", __func__);
> +
> + if (setsockopt(kr_state.fd, AF_ROUTE, ROUTE_PRIOFILTER, _prio,
> + sizeof(filter_prio)) == -1) {
> + log_warn("%s: setsockopt AF_ROUTE ROUTE_PRIOFILTER", __func__);
> + /* not fatal */
> + }
> +
>   /* grow receive buffer, don't wanna miss messages */
>   optlen = sizeof(default_rcvbuf);
>   if (getsockopt(kr_state.fd, SOL_SOCKET, SO_RCVBUF,
> @@ -353,6 +366,21 @@ kr_fib_decouple(void)
>   log_info("kernel routing table decoupled");
>  }
>  
> +void
> +kr_fib_update_prio(u_int8_t fib_prio)
> +{
> + struct kroute_node  *kr;
> +
> + RB_FOREACH(kr, kroute_tree, )
> + if ((kr->r.flags & F_OSPFD_INSERTED))
> + kr->r.priority = fib_prio;
> +
> + log_info("fib priority changed from %hhu to %hhu", kr_state.fib_prio,
> + fib_prio);
> +
> + kr_state.fib_prio = fib_prio;
> +}
> +
>  /* ARGSUSED */
>  void
>  kr_dispatch_msg(int fd, short event, void *bula)
> @@ -522,11 +550,25 @@ kr_redistribute(struct kroute_node *kh)
>  }
>  
>  void
> -kr_reload(void)
> +kr_reload(int redis_label_or_prefix)
>  {
>   struct kroute_node  *kr, *kn;
>   u_int32_tdummy;
>   int  r;
> + int  filter_prio = kr_state.fib_prio;
> +
> + /* update the priority filter */
> + if (redis_label_or_prefix) {
> + filter_prio = 0;
> + log_info("%s: priority filter disabled", __func__);
> + } else
> + log_debug("%s: priority filter enabled", __func__);
> +
> + if (setsockopt(kr_state.fd, AF_ROUTE, ROUTE_PRIOFILTER, _prio,
> + sizeof(filter_prio)) == -1) {
> + log_warn("%s: setsockopt AF_ROUTE ROUTE_PRIOFILTER", __func__);
> + /* not fatal */
> + }
>  
>   RB_FOREACH(kr, kroute_tree, ) {
>   for (kn = kr; kn; kn = kn->next) {
> Index: ospf6d.c
> ===
> RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.c,v
> retrieving revision 1.44
> diff -u -p -r1.44 ospf6d.c
> --- ospf6d.c  25 Mar 2019 20:53:33 -  1.44
> +++ ospf6d.c  15 Dec 2019 08:42:10 -
> @@ -280,7 +280,8 @@ main(int argc, char *argv[])
>   fatal("unveil");
>  
>   if (kr_init(!(ospfd_conf->flags & OSPFD_FLAG_NO_FIB_UPDATE),
> - ospfd_conf->rdomain, ospfd_conf->fib_priority) == -1)
> + ospfd_conf->rdomain, ospfd_conf->redist_label_or_prefix,
> + ospfd_conf->fib_priority) == -1)
>   fatalx("kr_init failed");
>  
>   event_dispatch();
> @@ -631,7 +632,7 @@ ospf_reload(void)
>  
>   merge_config(ospfd_conf, xconf);
>   /* update redistribute lists */
> - kr_reload();
> + kr_reload(ospfd_conf->redist_label_or_prefix);
>   return (0);
>  #else
>   return (-1);
> @@ -654,12 +655,16 @@ merge_config(struct ospfd_conf *conf, st
>   struct area *a, *xa, *na;
>   struct iface*iface;
>   struct redistribute *r;
> + int  rchange = 0;
>  
>   /* change of rtr_id needs a restart */
>   conf->flags = xconf->flags;
>   conf->spf_delay = xconf->spf_delay;
>   conf->spf_hold_time = xconf->spf_hold_time;
> - conf->redistribute = xconf->redistribute;
> + if (SIMPLEQ_EMPTY(>redist_list) !=
> + SIMPLEQ_EMPTY(>redist_list))
> + rchange = 1;
> + conf->redist_label_or_prefix = xconf->redist_label_or_prefix;
>  
>   if (ospfd_process == PROC_MAIN) {
>   /* main 

Re: ripd: memory leak and double free/use-after-free

2019-12-11 Thread Sebastian Benoit
Remi Locherer(remi.loche...@relo.ch) on 2019.12.10 22:39:32 +0100:
> On Tue, Dec 10, 2019 at 07:05:27PM +0100, Hiltjo Posthuma wrote:
> > Hi,
> > 
> > While looking at the code of ripd:
> > 
> > I think there are (also) 2 small memleaks in a debug/error path
> > (IMSG_REQUEST_ADD and IMSG_RESPONSE_ADD). It breaks out before adding the
> > struct rip_route as an entry by the add_entry function (which adds it and 
> > adds
> > a reference count) in the log_debug block.
> > 
> > clang-analyzer also pointed at a double-free and use of free'd data: the
> > function kroute_insert frees kr and returns -1 when struct kroute is 
> > duplicate.
> > 
> > Patch below (untested):
> > 
> 
> OK remi@

go ahead and commit it, ok benno@

> 
> > 
> > diff --git a/usr.sbin/ripd/kroute.c b/usr.sbin/ripd/kroute.c
> > index 6e7449e0909..71758a75e44 100644
> > --- a/usr.sbin/ripd/kroute.c
> > +++ b/usr.sbin/ripd/kroute.c
> > @@ -183,8 +183,7 @@ kr_change_fib(struct kroute_node *kr, struct kroute 
> > *kroute, int action)
> >  
> > if (kroute_insert(kr) == -1) {
> > log_debug("kr_update_fib: cannot insert %s",
> > -   inet_ntoa(kr->r.nexthop));
> > -   free(kr);
> > +   inet_ntoa(kroute->nexthop));
> > }
> > } else
> > kr->r.nexthop.s_addr = kroute->nexthop.s_addr;
> > diff --git a/usr.sbin/ripd/ripe.c b/usr.sbin/ripd/ripe.c
> > index d83901e245f..1f6f9b6583f 100644
> > --- a/usr.sbin/ripd/ripe.c
> > +++ b/usr.sbin/ripd/ripe.c
> > @@ -351,6 +351,7 @@ ripe_dispatch_rde(int fd, short event, void *bula)
> > NULL) {
> > log_debug("unknown neighbor id %u",
> > imsg.hdr.peerid);
> > +   free(rr);
> > break;
> > }
> > add_entry(>rq_list, rr);
> > @@ -396,6 +397,7 @@ ripe_dispatch_rde(int fd, short event, void *bula)
> > if ((nbr = nbr_find_peerid(imsg.hdr.peerid)) == NULL) {
> > log_debug("unknown neighbor id %u",
> > imsg.hdr.peerid);
> > +   free(rr);
> > break;
> > }
> > iface = nbr->iface;
> > 
> > -- 
> > Kind regards,
> > Hiltjo
> > 
> 



Re: massage tcpdump ip and encapsulation output

2019-12-06 Thread Sebastian Benoit
David Gwynne(da...@gwynne.id.au) on 2019.12.06 15:14:42 +1000:
> 
> 
> > On 5 Dec 2019, at 21:14, Sebastian Benoit  wrote:
> > 
> > Claudio Jeker(cje...@diehard.n-r-g.com) on 2019.12.05 09:53:49 +0100:
> >> I would suggest to just pack most of the headers into one group of ().
> >> 
> >> IPv4 ttl 1 [tos 0x20] 10.0.127.15 > 10.0.127.1
> >> would become
> >> IPv4 (ttl 1 tos 0x20) 10.0.127.15 > 10.0.127.1
> >> and
> >> IPv4 ttl 1 [tos 0x20] (id 39958, len 84) 10.0.127.15 > 10.0.127.1
> >> would become
> >> IPv4 (ttl 1 tos 0x20 id 39958 len 84) 10.0.127.15 > 10.0.127.1
> >> 
> >> Maybe add the commas if that is easy to do.
> > 
> > its more readable with commas, i think
> 
> do you want me to come up with something in this space as part of the
> large diff, or is the large change generally ok and we can tinker with
> this stuff afterward?

It was just a comment on the readability of lists like that.
I like your idea, please proceed whichever way you like.

> 
> there's some concern that what i'm proposing is too radical and will break
> peoples muscle memory.



Re: Does rpki-client need to unveil(NULL, NULL)?

2019-12-05 Thread Sebastian Benoit
ok

George Brown(321.geo...@gmail.com) on 2019.12.04 18:57:17 +:
> After pledge is immediately called without the unveil promise so this
> seems redundant.
> 
> diff --git a/usr.sbin/rpki-client/main.c b/usr.sbin/rpki-client/main.c
> index f05ec1c5837..53ee4223371 100644
> --- a/usr.sbin/rpki-client/main.c
> +++ b/usr.sbin/rpki-client/main.c
> @@ -1498,8 +1498,6 @@ main(int argc, char *argv[])
> /* Only allow access to BASE_DIR. */
> if (unveil(BASE_DIR, "r") == -1)
> err(1, "%s: unveil", BASE_DIR);
> -   if (unveil(NULL, NULL) == -1)
> -   err(1, "unveil");
> if (pledge("stdio rpath", NULL) == -1)
> err(1, "pledge");
> proc_parser(fd[0], force);
> 



Re: massage tcpdump ip and encapsulation output

2019-12-05 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2019.12.05 09:53:49 +0100:
> I would suggest to just pack most of the headers into one group of ().
> 
> IPv4 ttl 1 [tos 0x20] 10.0.127.15 > 10.0.127.1
> would become
> IPv4 (ttl 1 tos 0x20) 10.0.127.15 > 10.0.127.1
> and
> IPv4 ttl 1 [tos 0x20] (id 39958, len 84) 10.0.127.15 > 10.0.127.1
> would become
> IPv4 (ttl 1 tos 0x20 id 39958 len 84) 10.0.127.15 > 10.0.127.1
> 
> Maybe add the commas if that is easy to do.

its more readable with commas, i think



Re: ldomctl: Add create-vdisk command

2019-12-05 Thread Sebastian Benoit
Klemens Nanni(k...@openbsd.org) on 2019.11.30 01:44:48 +0100:
> Just like on amd64 with vmctl(8), I want to be able to easily create
> disk images.
> 
> ldomctl(8) currently advises to use dd(1), those files are not sparse
> either so creating big images may take a lot of time and the process
> tends to be error prone.

you can create sparse files with dd using seek=

  dd if=/dev/zero of=blarf bs=1 count=0 seek=512M

no opinion on the diff.


> `ldomctl create-vdisk -s size file' behaves like
> `vmctl create -s size file' except that size is parsed by scan_scaled(3)
> and not assumed to be megabytes, e.g.
> 
>   $ ldomctl create-vdisk -s 8G ~/guest01.img
>   t4-2$ ./obj/ldomctl create-vdisk -s 8G ~/guest01.img
>   t4-2$ ls -l ~/guest01.img
>   -rw---  1 kn  kn  8589934592 Nov 30 01:42 /home/kn/guest01.img
>   t4-2$ du ~/guest01.img
>   96   /home/kn/guest01.img  
> 
> Feedback? OK?
> 
> 
> Index: ldom.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/ldomctl/ldom.conf.5,v
> retrieving revision 1.7
> diff -u -p -r1.7 ldom.conf.5
> --- ldom.conf.5   28 Nov 2019 18:40:42 -  1.7
> +++ ldom.conf.5   29 Nov 2019 23:56:07 -
> @@ -59,8 +59,9 @@ for a list of OpenPROM variables.
>  The specified file is used to back a virtual disk of the guest
>  domain.
>  .Ar file
> -can be a block device node or a disk image file created with
> -.Xr dd 1 .
> +can be a block device node or a disk image file created with the
> +.Cm create-vdisk
> +command.
>  This keyword can be used multiple times.
>  .It Ic vnet Op Brq Ar keyword Ns = Ns Ar value ...
>  Assign a
> Index: ldomctl.8
> ===
> RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.8,v
> retrieving revision 1.18
> diff -u -p -r1.18 ldomctl.8
> --- ldomctl.8 28 Nov 2019 18:03:33 -  1.18
> +++ ldomctl.8 29 Nov 2019 23:47:01 -
> @@ -34,6 +34,16 @@ information about domains running on the
>  .Pp
>  The following commands are available:
>  .Bl -tag -width Ds
> +.It Cm create-vdisk Fl s Ar size Ar file
> +Create a virtual disk image with the specified
> +.Ar file
> +path and
> +.Ar size ,
> +in bytes.
> +.Ar size
> +can be specified with a human-readable scale, using the format described in
> +.Xr scan_scaled 3 ,
> +e.g. 512M.
>  .It Cm console Ar domain
>  Using
>  .Xr cu 1
> @@ -119,8 +129,8 @@ openbsd [next]
>  .Pp
>  Create a virtual disk image for each guest domain:
>  .Bd -literal -offset indent
> -# dd if=/dev/zero of=/home/puffy/vdisk0 bs=1m count=8192
> -# dd if=/dev/zero of=/home/salmah/vdisk0 bs=1m count=8192
> +# ldomctl create-vdisk -s 8G /home/puffy/vdisk0
> +# ldomctl create-vdisk -s 8G /home/salmah/vdisk0
>  .Ed
>  .Pp
>  The minirootfs install media can be used to boot guest domains:
> Index: ldomctl.c
> ===
> RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 ldomctl.c
> --- ldomctl.c 28 Nov 2019 18:40:42 -  1.27
> +++ ldomctl.c 30 Nov 2019 00:40:49 -
> @@ -18,12 +18,15 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "ds.h"
>  #include "hvctl.h"
> @@ -53,6 +56,7 @@ void list(int argc, char **argv);
>  void list_io(int argc, char **argv);
>  void xselect(int argc, char **argv);
>  void delete(int argc, char **argv);
> +void create_vdisk(int argc, char **argv);
>  void guest_start(int argc, char **argv);
>  void guest_stop(int argc, char **argv);
>  void guest_panic(int argc, char **argv);
> @@ -67,6 +71,7 @@ struct command commands[] = {
>   { "list-io",list_io },
>   { "select", xselect },
>   { "delete", delete },
> + { "create-vdisk", create_vdisk },
>   { "start",  guest_start },
>   { "stop",   guest_stop },
>   { "panic",  guest_panic },
> @@ -117,6 +122,9 @@ main(int argc, char **argv)
>   if (cmdp->cmd_name == NULL)
>   usage();
>  
> + if (strcmp(argv[0], "create-vdisk") == 0)
> + goto skip_hv;
> +
>   hv_open();
>  
>   /*
> @@ -153,6 +161,7 @@ main(int argc, char **argv)
>   add_guest(prop->d.arc.node);
>   }
>  
> +skip_hv:
>   (cmdp->cmd_func)(argc, argv);
>  
>   exit(EXIT_SUCCESS);
> @@ -165,6 +174,7 @@ usage(void)
>   "\t%1$s download directory\n"
>   "\t%1$s dump|list|list-io\n"
>   "\t%1$s init-system file\n"
> + "\t%1$s create-vdisk -s size file\n"
>   "\t%1$s console|panic|start|status|stop [domain]\n", getprogname());
>   exit(EXIT_FAILURE);
>  }
> @@ -347,6 +357,48 @@ delete(int argc, char **argv)
>   ds_conn_handle(dc);
>  
>   mdstore_delete(dc, argv[1]);
> +}
> +
> +void
> +create_vdisk(int argc, char **argv)
> +{
> + int ch, fd, save_errno;
> +  

Re: rad unveil

2019-11-26 Thread Sebastian Benoit
Sebastian Benoit(be...@openbsd.org) on 2019.11.26 18:46:11 +0100:
> 
> remove include statement and unveil() rad.
> 
> ok?


diff --git usr.sbin/rad/parse.y usr.sbin/rad/parse.y
index bb18c3d9c9c..443cff66065 100644
--- usr.sbin/rad/parse.y
+++ usr.sbin/rad/parse.y
@@ -112,7 +112,7 @@ typedef struct {
 
 %}
 
-%token RA_IFACE YES NO INCLUDE ERROR
+%token RA_IFACE YES NO ERROR
 %token DEFAULT ROUTER HOP LIMIT MANAGED ADDRESS
 %token CONFIGURATION OTHER LIFETIME REACHABLE TIME RETRANS TIMER
 %token AUTO PREFIX VALID PREFERRED LIFETIME ONLINK AUTONOMOUS
@@ -134,21 +134,6 @@ grammar: /* empty */
| grammar error '\n'{ file->errors++; }
;
 
-include: INCLUDE STRING{
-   struct file *nfile;
-
-   if ((nfile = pushfile($2, 0)) == NULL) {
-   yyerror("failed to include file %s", $2);
-   free($2);
-   YYERROR;
-   }
-   free($2);
-
-   file = nfile;
-   lungetc('\n');
-   }
-   ;
-
 string : string STRING {
if (asprintf(&$$, "%s %s", $1, $2) == -1) {
free($1);
@@ -428,7 +413,6 @@ lookup(char *s)
{"default", DEFAULT},
{"dns", DNS},
{"hop", HOP},
-   {"include", INCLUDE},
{"interface",   RA_IFACE},
{"lifetime",LIFETIME},
{"limit",   LIMIT},
diff --git usr.sbin/rad/rad.c usr.sbin/rad/rad.c
index 93675167b6b..3a79a08d3db 100644
--- usr.sbin/rad/rad.c
+++ usr.sbin/rad/rad.c
@@ -296,6 +296,11 @@ main(int argc, char *argv[])
if ((control_fd = control_init(csock)) == -1)
fatalx("control socket setup failed");
 
+if (unveil(conffile, "r") == -1)
+err(1, "unveil");
+if (unveil(NULL, NULL) == -1)
+err(1, "unveil");
+
main_imsg_compose_frontend_fd(IMSG_ICMP6SOCK, 0, icmp6sock);
main_imsg_compose_frontend_fd(IMSG_ROUTESOCK, 0, frontend_routesock);
main_imsg_compose_frontend_fd(IMSG_CONTROLFD, 0, control_fd);
diff --git usr.sbin/rad/rad.conf.5 usr.sbin/rad/rad.conf.5
index d4ca72f4639..ea0cb95d77e 100644
--- usr.sbin/rad/rad.conf.5
+++ usr.sbin/rad/rad.conf.5
@@ -50,10 +50,6 @@ sends IPv6 router advertisement messages.
 This section defines on which interfaces to advertise prefix information
 and their associated parameters.
 .El
-.Pp
-Additional configuration files can be included with the
-.Ic include
-keyword.
 .Sh MACROS
 Macros can be defined that will later be expanded in context.
 Macro names must start with a letter, digit, or underscore,



unveil slaacd

2019-11-26 Thread Sebastian Benoit
slaacd does not have include statements

open the control socket earlier, then unveil().

diff --git sbin/slaacd/slaacd.c sbin/slaacd/slaacd.c
index 9c3f64f407d..990614df734 100644
--- sbin/slaacd/slaacd.c
+++ sbin/slaacd/slaacd.c
@@ -179,6 +179,16 @@ main(int argc, char *argv[])
if (getpwnam(SLAACD_USER) == NULL)
errx(1, "unknown user %s", SLAACD_USER);
 
+#ifndef SMALL
+   if ((control_fd = control_init(csock)) == -1)
+   fatalx("control socket setup failed");
+#endif /* SMALL */
+
+   if (unveil(saved_argv0, "x") == -1)
+   err(1, "unveil");
+   if (unveil(NULL, NULL) == -1)
+   err(1, "unveil");
+
log_init(debug, LOG_DAEMON);
log_setverbose(verbose);
 
@@ -276,11 +286,6 @@ main(int argc, char *argv[])
, sizeof(rtfilter)) == -1)
fatal("setsockopt(ROUTE_MSGFILTER)");
 
-#ifndef SMALL
-   if ((control_fd = control_init(csock)) == -1)
-   fatalx("control socket setup failed");
-#endif /* SMALL */
-
if (pledge("stdio sendfd wroute", NULL) == -1)
fatal("pledge");
 



rad unveil

2019-11-26 Thread Sebastian Benoit


remove include statement and unveil() rad.

ok?

diff --git usr.sbin/rad/parse.y usr.sbin/rad/parse.y
index bb18c3d9c9c..443cff66065 100644
--- usr.sbin/rad/parse.y
+++ usr.sbin/rad/parse.y
@@ -112,7 +112,7 @@ typedef struct {
 
 %}
 
-%token RA_IFACE YES NO INCLUDE ERROR
+%token RA_IFACE YES NO ERROR
 %token DEFAULT ROUTER HOP LIMIT MANAGED ADDRESS
 %token CONFIGURATION OTHER LIFETIME REACHABLE TIME RETRANS TIMER
 %token AUTO PREFIX VALID PREFERRED LIFETIME ONLINK AUTONOMOUS
@@ -134,21 +134,6 @@ grammar: /* empty */
| grammar error '\n'{ file->errors++; }
;
 
-include: INCLUDE STRING{
-   struct file *nfile;
-
-   if ((nfile = pushfile($2, 0)) == NULL) {
-   yyerror("failed to include file %s", $2);
-   free($2);
-   YYERROR;
-   }
-   free($2);
-
-   file = nfile;
-   lungetc('\n');
-   }
-   ;
-
 string : string STRING {
if (asprintf(&$$, "%s %s", $1, $2) == -1) {
free($1);
@@ -428,7 +413,6 @@ lookup(char *s)
{"default", DEFAULT},
{"dns", DNS},
{"hop", HOP},
-   {"include", INCLUDE},
{"interface",   RA_IFACE},
{"lifetime",LIFETIME},
{"limit",   LIMIT},
diff --git usr.sbin/rad/rad.c usr.sbin/rad/rad.c
index 93675167b6b..986f0f2b186 100644
--- usr.sbin/rad/rad.c
+++ usr.sbin/rad/rad.c
@@ -200,6 +200,15 @@ main(int argc, char *argv[])
if (getpwnam(RAD_USER) == NULL)
errx(1, "unknown user %s", RAD_USER);
 
+if (unveil(conffile, "r") == -1)
+err(1, "unveil");
+if (unveil(csock, "rwc") == -1)
+err(1, "unveil");
+if (unveil(saved_argv0, "x") == -1)
+err(1, "unveil");
+if (unveil(NULL, NULL) == -1)
+err(1, "unveil");
+
log_init(debug, LOG_DAEMON);
log_setverbose(cmd_opts & OPT_VERBOSE);
 
diff --git usr.sbin/rad/rad.conf.5 usr.sbin/rad/rad.conf.5
index d4ca72f4639..ea0cb95d77e 100644
--- usr.sbin/rad/rad.conf.5
+++ usr.sbin/rad/rad.conf.5
@@ -50,10 +50,6 @@ sends IPv6 router advertisement messages.
 This section defines on which interfaces to advertise prefix information
 and their associated parameters.
 .El
-.Pp
-Additional configuration files can be included with the
-.Ic include
-keyword.
 .Sh MACROS
 Macros can be defined that will later be expanded in context.
 Macro names must start with a letter, digit, or underscore,



Re: ifconfig inet6 netmask

2019-11-18 Thread Sebastian Benoit
Alexander Bluhm(alexander.bl...@gmx.net) on 2019.11.18 16:31:05 +0100:
> On Thu, Nov 14, 2019 at 11:14:45PM +0100, Sebastian Benoit wrote:
> > The alternative is to not allow netmask for ipv6 and only / and 
> > prefixlen
> > . Why support such a crazy way of specifying the mask?
> 
> We can also do it the other way around and forbid ifconfig inet6
> netmask.

sorry, thats what i meant.
 
> > In route we removed a few quirks like that with notice in current.html , 
> > why not here?
> 
> route(8) still accepts netmask.

Yes, i  wrote too fast. The quirks were problematic ordering of arguments.

> route add -inet6 1234:: -netmask :: ::1
> 
> Ignoring options silently is bad.  So either use or reject netmask.
> If ifconfig(8) uses it, it would be consistent to route(8).

Right. Probably better to be consistent and not break peoples configs then.

ok for both diffs with a preference for the first ;)

> 
> bluhm
> 
> Index: sbin/ifconfig/ifconfig.8
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sbin/ifconfig/ifconfig.8,v
> retrieving revision 1.343
> diff -u -p -r1.343 ifconfig.8
> --- sbin/ifconfig/ifconfig.8  10 Nov 2019 09:10:44 -  1.343
> +++ sbin/ifconfig/ifconfig.8  18 Nov 2019 13:57:18 -
> @@ -429,7 +429,7 @@ output from
>  .Cm hwfeatures
>  shows the maximum supported MTU.
>  .It Cm netmask Ar mask
> -(inet and inet6 only)
> +(inet only)
>  Specify how much of the address to reserve for subdividing
>  networks into subnetworks.
>  The mask includes the network part of the local address
> Index: sbin/ifconfig/ifconfig.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.414
> diff -u -p -r1.414 ifconfig.c
> --- sbin/ifconfig/ifconfig.c  24 Oct 2019 18:54:10 -  1.414
> +++ sbin/ifconfig/ifconfig.c  18 Nov 2019 13:56:31 -
> @@ -6151,6 +6151,9 @@ in6_getaddr(const char *s, int which)
>   char buf[HOST_NAME_MAX+1 + sizeof("/128")], *pfxlen;
>   int error;
> 
> + if (which == MASK)
> + errx(1, "inet6 needs prefixlen, not netmask");
> +
>   memset(, 0, sizeof(hints));
>   hints.ai_family = AF_INET6;
>   hints.ai_socktype = SOCK_DGRAM; /*dummy*/
> 



Re: ifconfig inet6 netmask

2019-11-14 Thread Sebastian Benoit
Alexander Bluhm(alexander.bl...@gmx.net) on 2019.11.14 22:50:50 +0100:
> Hi,
> 
> While writing my ifconfig regress test I realized that IPv6 netmasks
> are parsed, but silently ignored.  Ignoring commandline parameters
> feels wrong and is inconsistent to IPv4.
> 
> Of course I don't expect anyone to use something like this:
> 
> ifconfig vether0 inet6 fdd7:e83e:66bc:::17 netmask 
> :::::::

reminds me of the problems in route.

> But current implementation sets a prefixlen 64 in this case.
> 
> ok?

diff is ok, but ...

The alternative is to not allow netmask for ipv6 and only / and 
prefixlen
. Why support such a crazy way of specifying the mask?

In route we removed a few quirks like that with notice in current.html , why 
not here?

Con: Unfortunatly most netmasks configured will be /64, so probability is high
that if someone actually uses netmask, the problem wont be noticed.

> bluhm
> 
> Index: sbin/ifconfig/ifconfig.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.414
> diff -u -p -r1.414 ifconfig.c
> --- sbin/ifconfig/ifconfig.c  24 Oct 2019 18:54:10 -  1.414
> +++ sbin/ifconfig/ifconfig.c  14 Nov 2019 21:27:47 -
> @@ -1301,6 +1301,7 @@ void
>  setifnetmask(const char *addr, int ignored)
>  {
>   afp->af_getaddr(addr, MASK);
> + explicit_prefix = 1;
>  }
> 
>  /* ARGSUSED */
> 



Re: ospfd: correct function name in error message

2019-11-09 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2019.11.09 15:32:39 +0100:
> On Sat, Nov 09, 2019 at 03:27:31PM +0100, Denis Fondras wrote:
> > Fix function name in error message.
> > 
> > Index: kroute.c
> > ===
> > RCS file: /cvs/src/usr.sbin/ospfd/kroute.c,v
> > retrieving revision 1.112
> > diff -u -p -r1.112 kroute.c
> > --- kroute.c28 Dec 2018 19:25:10 -  1.112
> > +++ kroute.c9 Nov 2019 14:11:03 -
> > @@ -1501,7 +1501,7 @@ rtmsg_process(char *buf, size_t len)
> > if ((mpath || prio == kr_state.fib_prio) &&
> > (kr = kroute_matchgw(okr, nexthop)) ==
> > NULL) {
> > -   log_warnx("dispatch_rtmsg "
> > +   log_warnx("rtmsg_process "
> > "mpath route not found");
> > /* add routes we missed out earlier */
> > goto add;
> > @@ -1536,7 +1536,7 @@ rtmsg_process(char *buf, size_t len)
> >  add:
> > if ((kr = calloc(1,
> > sizeof(struct kroute_node))) == NULL) {
> > -   log_warn("dispatch calloc");
> > +   log_warn("rtmsg_process calloc");
> > return (-1);
> > }
> >  
> > @@ -1580,7 +1580,7 @@ add:
> > okr = kr;
> > if (mpath &&
> > (kr = kroute_matchgw(kr, nexthop)) == NULL) {
> > -   log_warnx("dispatch_rtmsg "
> > +   log_warnx("rtmsg_process "
> > "mpath route not found");
> > return (-1);
> > }
> > 
> 
> OK. Another option is to use __func__ which is always correct.

Please definatly use __func__. With that ok benno too.



Re: mg(1) tell make-backup-files is on by default

2019-11-08 Thread Sebastian Benoit
Solene Rapenne(sol...@perso.pw) on 2019.11.08 18:39:20 +0100:
> Someone on reddit had issue with this config file, there was no backup
> file, in file directory or in ~/.mg.d
> 
> make-backup-files
> backup-to-home-directory
> 
> in fact, having "make-backup-files" disables backups.
> 
> 
> I've looked at the mg logic for backup files and I could sort that the
> default make-backup-files value is 0
> 
> /funmap.c: {makebkfile, "make-backup-files", 0},

No, that 0 is not the default. The line defines a command
'make-backup-files' (that can be given in the config file, but also with
"Meta-x make-backup-files"). The 0 indicates to the command parser that
this command does not take a argument, i.e. is a "toggle". When the command
is given, the makebkfile() is called.

> but in file.c there is a statement with a default to TRUE
> 
>  * Save the contents of the current buffer back into its associated
>  * file.
>   */
>   static int  makebackup = TRUE;
> 
> 
> I don't really get the logic here, nor when the configuration file and
> that TRUE variable get in touch.

in the file.c function makebkfile() which modifies the static variable
makebackup.

So yes, the default is TRUE and the command make-backup-files changes the
variable makebackup in makebkfile():

makebackup = !makebackup;

> What I propose is to update the manual that make-backup-files is true by
> default, so toggling it disable backups.

I think that is the correct sollution. ok benno@
 
> Index: mg.1
> ===
> RCS file: /data/cvs/src/usr.bin/mg/mg.1,v
> retrieving revision 1.117
> diff -u -p -r1.117 mg.1
> --- mg.1  2 Jul 2019 16:25:39 -   1.117
> +++ mg.1  8 Nov 2019 17:16:47 -
> @@ -688,6 +688,7 @@ Bind a key mapping in the local (topmost
>  Unbind a key mapping in the local (topmost) mode.
>  .It make-backup-files
>  Toggle generation of backup files.
> +Enabled by default.
>  .It make-directory
>  Prompt the user for a path or directory name which is then created.
>  .It mark-paragraph
> 



Re: upgrade66.html missing acme-client.conf staging api url change

2019-11-08 Thread Sebastian Benoit
Solene Rapenne(sol...@perso.pw) on 2019.11.08 18:13:40 +0100:
> The staging api changed too. I know people who did not update that url,
> it's not especially obvious because the old url doesn't contain v01.
> 
> ok ?

Nice! I actually wondered about that and my google foo was too low.
Thanks for fixing it. ok benno@

> 
> Index: upgrade66.html
> ===
> RCS file: /data/cvs/www/faq/upgrade66.html,v
> retrieving revision 1.15
> diff -u -p -r1.15 upgrade66.html
> --- upgrade66.html30 Oct 2019 04:11:00 -  1.15
> +++ upgrade66.html8 Nov 2019 17:12:01 -
> @@ -147,6 +147,14 @@ any post-release fixes.
>
>https://acme-v02.api.letsencrypt.org/directory
> +  The staging api url should also be changed from
> +  
> +  https://acme-staging.api.letsencrypt.org/directory
> +  to
> +  
> +  https://acme-staging-v02.api.letsencrypt.org/directory
>
>The -A and -D flags have been removed from
>https://man.openbsd.org/OpenBSD-6.6/acme-client.1;>
> 



  1   2   3   4   5   6   >