Syspatch retracted (OpenBSD Errata: April 22, 2022 (wifi))

2022-04-23 Thread Sebastian Benoit
Syspatch syspatch71-001_wifi has been retracted.

A mistake was made in generating the syspatch(8) binary update
syspatch71-001_wifi for this errata. This causes problems installing future
binary updates and reverting the syspatch. Because of this, the syspatch has
been retracted until the issue is resolved. The actual source patch is not
affected from this (and can be applied as described in the patch
instructions) and the syspatch itself works fine as well.

If you already have installed the syspatch71-001_wifi update, you can
continue running with it until we publish instructions on how to recover on
http://www.openbsd.org/errata71.html.

Alexander Bluhm(bl...@openbsd.org) on 2022.04.22 16:44:06 +0200:
> Errata patch for wireless drivers in the kernel has been released
> for OpenBSD 7.1.
> 
> Binary updates for the amd64, i386 and arm64 platform are available
> via the syspatch utility.  Source code patches can be found on the
> respective errata page:
> 
>   https://www.openbsd.org/errata71.html
> 



Re: rpki-client: TZ=UTC + localtime -> gmtime?

2022-04-21 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2022.04.20 15:12:57 +0200:
> On Wed, Apr 20, 2022 at 03:00:15PM +0200, Theo Buehler wrote:
> > Found this when looking at the timezone issue a couple of weeks back and
> > then forgot about it:
> > 
> > This setenv() + localtime() looks like a hack to me and I don't really
> > understand why it should be preferable over using gmtime() directly. The
> > only change in output that I can see is that it causes %Z to print GMT
> > instead of UTC, so I modified the format string accordingly (but I have
> > no strong opinion on this).
> 
> UTC is the proper timezone here. Not sure why gmtime results in %Z to
> return GMT since gmtime converts into UTC. On the other hand timezones
> in struct tm are strange so it probably just defaults to GMT for a 0
> offset time.

well, no.
UTC is not a timezone. UTC is a time standard.
GMT is the correct timezone.



rpki-client 7.8 has just been released

2022-04-09 Thread Sebastian Benoit
rpki-client 7.8 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 a BGP announcement. The program queries the
global RPKI repository system and outputs validated ROA payloads and
BGPsec Router keys in the configuration format of OpenBGPD, BIRD, and
also as CSV or JSON objects for consumption by other routing stacks.

See RFC 6480 and RFC 6811 for a description of how RPKI and BGP Prefix
Origin Validation help secure the Internet's global routing system.

rpki-client was primarily developed by Kristaps Dzonsons, Claudio
Jeker, Job Snijders, Theo Buehler, Theo de Raadt and Sebastian Benoit
as part of the OpenBSD Project.

This release includes the following changes to the previous release:

- Do not apply timezone offsets when converting X509 times.  X509
  times are in UTC and comparing them to times in different timezones
  would cause validity problems.

rpki-client works on all operating systems with a libcrypto library
based on OpenSSL 1.1 or LibreSSL 3.4, and a libtls library compatible
with LibreSSL 3.4 or later.

rpki-client is known to compile and run on at least the following
operating systems: Alpine, CentOS, Debian, Fedora, FreeBSD, Red Hat,
Rocky, Ubuntu, macOS, and of course OpenBSD!

It is our hope that packagers take interest and help adapt
rpki-client-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.

Assistance to coordinate security issues is available via
secur...@openbsd.org.



rpki-client 7.7 has just been released

2022-04-07 Thread Sebastian Benoit
rpki-client 7.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 a BGP announcement. The program queries the
global RPKI repository system and outputs validated ROA payloads and
BGPsec Router keys in the configuration format of OpenBGPD, BIRD, and
also as CSV or JSON objects for consumption by other routing stacks.

See RFC 6480 and RFC 6811 for a description of how RPKI and BGP Prefix
Origin Validation help secure the Internet's global routing system.

rpki-client was primarily developed by Kristaps Dzonsons, Claudio
Jeker, Job Snijders, Theo Buehler, Theo de Raadt and Sebastian Benoit
as part of the OpenBSD Project.

This release includes the following changes to the previous release:

  - Add various RFC 6488 compliance checks to improve the CMS parser.
  - Improve RRDP replication through less aggressive cache cleanup.
  - Add a check whether a given Manifest EE certificate is listed on the
applicable CRL.
  - For forward compatibility permit ASPA object to appear on Manifests.
  - Various improvements to the '-f ' diagnostic option to
now also validate files containing Trust Anchor certs and CRLs.

rpki-client works on all operating systems with a libcrypto library
based on OpenSSL 1.1 or LibreSSL 3.3, and a libtls library compatible
with LibreSSL 3.4 or later.

rpki-client is known to compile and run on at least the following
operating systems: Alpine, CentOS, Debian, Fedora, FreeBSD, Red Hat,
Rocky, Ubuntu, macOS, and of course OpenBSD!

It is our hope that packagers take interest and help adapt
rpki-client-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.

Assistance to coordinate security issues is available via
secur...@openbsd.org.



rpki-client 7.6 released

2022-02-07 Thread Sebastian Benoit
rpki-client 7.6 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 a BGP announcement. The program queries the
global 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, Theo Buehler, Theo de Raadt and Sebastian Benoit
as part of the OpenBSD Project.

This release includes the following changes to the previous release:

  - Enforce the correct namespace of rrdp files.
  - Fail certificate verification if a certificate contains unknown
critical extensions.
  - Improve cleanup of rrdp directory contents.
  - Introduce a validated cache which holds all the files that have
successfully been verified by rpki-client.
  - Add a new option '-f ' to validate a signed object in a file
against the RPKI cache.

rpki-client works on all operating systems with a libcrypto library
based on OpenSSL 1.1 or LibreSSL 3.3, and a libtls library compatible
with LibreSSL 3.3 or later.

rpki-client is known to compile and run on at least the following
operating systems: Alpine, CentOS, Debian, Fedora, FreeBSD, Red Hat,
Rocky, Ubuntu, macOS, and of course OpenBSD!

It is our hope that packagers take interest and help adapt
rpki-client-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.

Assistance to coordinate security issues is available via
secur...@openbsd.org.



Re: rpki-client repo layout change

2022-01-26 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2022.01.26 11:54:41 +0100:
> On Wed, Jan 26, 2022 at 11:43:25AM +0100, Theo Buehler wrote:
> > On Wed, Jan 26, 2022 at 10:06:37AM +0100, Claudio Jeker wrote:
> > > This diff removes the valid/ subdir in favor of a more direct directory
> > > layout for all valid CA repository files.
> > > It moves rrdp and rsync to .rsync and .rrdp but keeps ta/ because trust
> > > anchors are special.
> > > 
> > > The biggest change is probably in the FTS code to cleanup the repo since
> > > the traversing now starts at ".". Apart from that removing valid results
> > > in some simplified code (esp. in -f mode).
> > > 
> > > This diff is mostly from job@, I just did fixed the FTS bits so the
> > > cleanup works again.
> > 
> > If I do a full run with this diff, I run into
> > 
> > rpki-client: both possibilities of file present
> > 
> > Instrumenting this error (should it include fn?) it turns out to be
> 
> Yes, it should show at least one fn.
> 
> > pki-repo.registro.br/repo/A2x6icaKpVWP5uUBzJQzRvEpjvS7PV2qt1Ct/1/3230302e3138392e34302e302f32332d3234203d3e20313roa
> > 
> > I have not yet figured out why.
> 
> Gr. I once hit this but thought it was something that was wrong with
> some of my other diffs I have around. Need to look into this. It it for
> sure something that should not happen.

interestingly, i did not get that.

diff reads ok to me otherwise.

>  
> > One tiny nit inline.
> > 
> 
> 
> > > @@ -1270,40 +1279,38 @@ static void
> > >  repo_move_valid(struct filepath_tree *tree)
> > >  {
> > >   struct filepath *fp, *nfp;
> > > - size_t rsyncsz = strlen("rsync/");
> > > - size_t rrdpsz = strlen("rrdp/");
> > > + size_t rsyncsz = strlen(".rsync/");
> > > + size_t rrdpsz = strlen(".rrdp/");
> > >   char *fn, *base;
> > >  
> > >   RB_FOREACH_SAFE(fp, filepath_tree, tree, nfp) {
> > > - if (strncmp(fp->file, "rsync/", rsyncsz) != 0 &&
> > > - strncmp(fp->file, "rrdp/", rrdpsz) != 0)
> > > + if (strncmp(fp->file, ".rsync/", rsyncsz) != 0 &&
> > > + strncmp(fp->file, ".rrdp/", rrdpsz) != 0)
> > >   continue; /* not a temporary file path */
> > >  
> > > - if (strncmp(fp->file, "rsync/", rsyncsz) == 0) {
> > > - if (asprintf(, "valid/%s", fp->file + rsyncsz) == -1)
> > > - err(1, NULL);
> > > + if (strncmp(fp->file, ".rsync/", rsyncsz) == 0) {
> > > + fn = fp->file + rsyncsz;
> > >   } else {
> > >   base = strchr(fp->file + rrdpsz, '/');
> > >   assert(base != NULL);
> > > - if (asprintf(, "valid/%s", base + 1) == -1)
> > > - err(1, NULL);
> > > + fn = base + 1;
> > >   }
> > >  
> > >   if (repo_mkpath(fn) == -1) {
> > > - free(fn);
> > >   continue;
> > >   }
> > >  
> > >   if (rename(fp->file, fn) == -1) {
> > >   warn("rename %s", fp->file);
> > > - free(fn);
> > >   continue;
> > >   }
> > >  
> > >   /* switch filepath node to new path */
> > >   RB_REMOVE(filepath_tree, tree, fp);
> > > - free(fp->file);
> > > - fp->file = fn;
> > > + base = fp->file;
> > > + if ((fp->file = strdup(fn)) == NULL)
> > > + err(1, NULL);
> > > + free(base);
> > 
> > The dance with base is not necessary:
> > 
> > free(fp->file)
> > if ((fp->file = strdup(fn)) == NULL)
> > err(1, NULL);
> > 
> 
> fn points into fp->file (see further up e.g. fn = fp->file + rsyncsz;
> So free() before strdup will result in a use after free.
>  
> -- 
> :wq Claudio
> 



Re: application.c be more paranoid for misbehaving backends

2022-01-21 Thread Sebastian Benoit
Martijn van Duren(openbsd+t...@list.imperialat.at) on 2022.01.20 22:53:06 +0100:
> There's a missing NULL check in appl_response(). This should only happenwhen 
> a backend is misbehaving, so I only managed to find this because
> I'm actively bashing it right now. This should make us a little more
> future-proof. Code further down the path already has similar NULL checks
> against this variable.
> 
> OK?

ok

> 
> martijn@
> 
> Index: application.c
> ===
> RCS file: /cvs/src/usr.sbin/snmpd/application.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 application.c
> --- application.c 19 Jan 2022 10:59:35 -  1.1
> +++ application.c 20 Jan 2022 21:52:41 -
> @@ -1056,7 +1056,8 @@ appl_response(struct appl_backend *backe
>   appl_varbind_error(origvb, error);
>   origvb->avi_state = APPL_VBSTATE_DONE;
>   origvb->avi_varbind.av_oid = vb->av_oid;
> - if (vb->av_value->be_class == BER_CLASS_CONTEXT &&
> + if (vb->av_value != NULL &&
> + vb->av_value->be_class == BER_CLASS_CONTEXT &&
>   vb->av_value->be_type == APPL_EXC_ENDOFMIBVIEW) {
>   nregion = appl_region_next(ureq->aru_ctx,
>   &(vb->av_oid), origvb->avi_region);
> 



Re: rpki-client pass real filename from parser back to parent

2022-01-04 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2022.01.04 16:15:56 +0100:
> This is another diff on the way to having a validated repo.
> Pass the filename of the entity which was parsed back to the parent.
> With this we can move the filepath_add() call from entity_write_req()
> to entity_process(). As a side-effect the "Already visited" check is moved
> after parsing so a file may be reparsed before being ignored. I doubt this
> causes an issue.
> 
> On top of this change how entp->file is passed to the individual parser
> functions. Just pass the filename to those functions. Only exception for
> now is proc_parser_root_cert() since it accesses more of the entp.
> 
> Again this is done to make it possible to have the parser decide which
> path to use for accessing a file. For now this is just shuffling code but
> once the code has two places to look for a file this will be all needed.

ok

> 
> -- 
> :wq Claudio
> 
> Index: main.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
> retrieving revision 1.170
> diff -u -p -r1.170 main.c
> --- main.c29 Dec 2021 11:37:57 -  1.170
> +++ main.c4 Jan 2022 13:39:31 -
> @@ -132,12 +132,6 @@ entity_write_req(const struct entity *en
>  {
>   struct ibuf *b;
>  
> - if (filepath_add(, ent->file) == 0) {
> - warnx("%s: File already visited", ent->file);
> - entity_queue--;
> - return;
> - }
> -
>   b = io_new_buffer();
>   io_simple_buffer(b, >type, sizeof(ent->type));
>   io_simple_buffer(b, >talid, sizeof(ent->talid));
> @@ -467,6 +461,7 @@ entity_process(struct ibuf *b, struct st
>   struct cert *cert;
>   struct mft  *mft;
>   struct roa  *roa;
> + char*file;
>   int  c;
>  
>   /*
> @@ -476,6 +471,14 @@ entity_process(struct ibuf *b, struct st
>* We follow that up with whether the resources didn't parse.
>*/
>   io_read_buf(b, , sizeof(type));
> + io_read_str(b, );
> +
> + if (filepath_add(, file) == 0) {
> + warnx("%s: File already visited", file);
> + free(file);
> + entity_queue--;
> + return;
> + }
>  
>   switch (type) {
>   case RTYPE_TAL:
> @@ -544,6 +547,7 @@ entity_process(struct ibuf *b, struct st
>   errx(1, "unknown entity type %d", type);
>   }
>  
> + free(file);
>   entity_queue--;
>  }
>  
> Index: parser.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 parser.c
> --- parser.c  29 Dec 2021 11:37:57 -  1.29
> +++ parser.c  4 Jan 2022 13:19:06 -
> @@ -51,7 +51,7 @@ static struct crl_tree   crlt = RB_INITIA
>   * Returns the roa on success, NULL on failure.
>   */
>  static struct roa *
> -proc_parser_roa(struct entity *entp, const unsigned char *der, size_t len)
> +proc_parser_roa(const char *file, const unsigned char *der, size_t len)
>  {
>   struct roa  *roa;
>   X509*x509;
> @@ -61,10 +61,10 @@ proc_parser_roa(struct entity *entp, con
>   STACK_OF(X509_CRL)  *crls;
>   struct crl  *crl;
>  
> - if ((roa = roa_parse(, entp->file, der, len)) == NULL)
> + if ((roa = roa_parse(, file, der, len)) == NULL)
>   return NULL;
>  
> - a = valid_ski_aki(entp->file, , roa->ski, roa->aki);
> + a = valid_ski_aki(file, , roa->ski, roa->aki);
>   build_chain(a, );
>   crl = get_crl(a);
>   build_crls(crl, );
> @@ -82,8 +82,7 @@ proc_parser_roa(struct entity *entp, con
>   c = X509_STORE_CTX_get_error(ctx);
>   X509_STORE_CTX_cleanup(ctx);
>   if (verbose > 0 || c != X509_V_ERR_UNABLE_TO_GET_CRL)
> - warnx("%s: %s", entp->file,
> - X509_verify_cert_error_string(c));
> + warnx("%s: %s", file, X509_verify_cert_error_string(c));
>   X509_free(x509);
>   roa_free(roa);
>   sk_X509_free(chain);
> @@ -112,7 +111,7 @@ proc_parser_roa(struct entity *entp, con
>* the code around roa_read() to check the "valid" field itself.
>*/
>  
> - if (valid_roa(entp->file, , roa))
> + if (valid_roa(file, , roa))
>   roa->valid = 1;
>  
>   sk_X509_free(chain);
> @@ -133,7 +132,7 @@ proc_parser_roa(struct entity *entp, con
>   * Return the mft on success or NULL on failure.
>   */
>  static struct mft *
> -proc_parser_mft(struct entity *entp, const unsigned char *der, size_t len)
> +proc_parser_mft(const char *file, const unsigned char *der, size_t len)
>  {
>   struct mft  *mft;
>   X509*x509;
> @@ -141,10 +140,10 @@ proc_parser_mft(struct entity *entp, con
>   struct auth *a;
>   STACK_OF(X509)

Re: unbreak rpki-client -n mode

2022-01-04 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2022.01.04 16:05:57 +0100:
> Currently running rpki-client -n with an up to date repo results in  the
> loss of around 25% of ROAs. The reason is that most of apnic fails since
> they decided it is a glorious idea to put two rsync repos into one rrdp
> repo.
> 
> When changing the repo state for the noop case from REPO_DONE to
> REPO_FAILED it resulted in a unwanted side-effect. The problem is that a
> second rrdp_get() call for such a failed repo fails and so the code falls
> back to rsync but there is nothing in the rsync dir.
> 
> Just change back the status to REPO_DONE and -n works like before.

ok

> -- 
> :wq Claudio
> 
> Index: repo.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/repo.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 repo.c
> --- repo.c29 Dec 2021 11:35:23 -  1.18
> +++ repo.c4 Jan 2022 14:28:41 -
> @@ -447,7 +447,7 @@ ta_get(struct tal *tal)
>   tal->uri = NULL;
>  
>   if (noop) {
> - tr->state = REPO_FAILED;
> + tr->state = REPO_DONE;
>   logx("ta/%s: using cache", tr->descr);
>   repo_done(tr, 0);
>   } else {
> @@ -512,7 +512,7 @@ rsync_get(const char *uri, int nofetch)
>   rr->basedir = repo_dir(repo, "rsync", 0);
>  
>   if (noop || nofetch) {
> - rr->state = REPO_FAILED;
> + rr->state = REPO_DONE;
>   logx("%s: using cache", rr->basedir);
>   repo_done(rr, 0);
>   } else {
> @@ -582,7 +582,7 @@ rrdp_get(const char *uri, int nofetch)
>   RB_INIT(>deleted);
>  
>   if (noop || nofetch) {
> - rr->state = REPO_FAILED;
> + rr->state = REPO_DONE;
>   logx("%s: using cache", rr->notifyuri);
>   repo_done(rr, 0);
>   } else {
> 



Re: fix some -Wunused-but-set-variable warnings in vmd

2022-01-04 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2022.01.04 14:12:02 +0100:
> On Tue, Jan 04, 2022 at 10:58:41AM +0100, Claudio Jeker wrote:
> > This are obvious and easy to fix unused but set variables.
> > There are more in vioscsi.c but those are actually used if compiled with
> > DEBUG set.
> 
> The changes in loadfile_elf.c, vioqcow2.c and vmd.c are trivial and can be
> committed one by one. 

ok on those

> The change in vmm.c actually uncovered a possible
> issue. If vm_register() fails the vm pointer will most probably be NULL
> and so the next line will access a NULL pointer.
> 
> I think this diff is better. It cleans up also a totally unused
> IMSG_VMDOP_RECEIVE_VM_RESPONSE imsg type.

looks ok too, i think.
  
> It is hard to fail the vm_register() call so it is not trivial to really
> test the error case but I did test vmctl receive and that still works.
> -- 
> :wq Claudio
> 
> Index: control.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/control.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 control.c
> --- control.c 29 Nov 2021 05:17:35 -  1.38
> +++ control.c 4 Jan 2022 12:05:35 -
> @@ -94,7 +94,6 @@ control_dispatch_vmd(int fd, struct priv
>   case IMSG_VMDOP_START_VM_RESPONSE:
>   case IMSG_VMDOP_PAUSE_VM_RESPONSE:
>   case IMSG_VMDOP_SEND_VM_RESPONSE:
> - case IMSG_VMDOP_RECEIVE_VM_RESPONSE:
>   case IMSG_VMDOP_UNPAUSE_VM_RESPONSE:
>   case IMSG_VMDOP_GET_INFO_VM_DATA:
>   case IMSG_VMDOP_GET_INFO_VM_END_DATA:
> Index: vmd.h
> ===
> RCS file: /cvs/src/usr.sbin/vmd/vmd.h,v
> retrieving revision 1.107
> diff -u -p -r1.107 vmd.h
> --- vmd.h 29 Nov 2021 05:17:35 -  1.107
> +++ vmd.h 4 Jan 2022 12:05:24 -
> @@ -101,7 +101,6 @@ enum imsg_type {
>   IMSG_VMDOP_SEND_VM_REQUEST,
>   IMSG_VMDOP_SEND_VM_RESPONSE,
>   IMSG_VMDOP_RECEIVE_VM_REQUEST,
> - IMSG_VMDOP_RECEIVE_VM_RESPONSE,
>   IMSG_VMDOP_RECEIVE_VM_END,
>   IMSG_VMDOP_WAIT_VM_REQUEST,
>   IMSG_VMDOP_TERMINATE_VM_REQUEST,
> Index: vmm.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/vmm.c,v
> retrieving revision 1.102
> diff -u -p -r1.102 vmm.c
> --- vmm.c 29 Nov 2021 05:17:35 -  1.102
> +++ vmm.c 4 Jan 2022 12:05:04 -
> @@ -102,7 +102,7 @@ int
>  vmm_dispatch_parent(int fd, struct privsep_proc *p, struct imsg *imsg)
>  {
>   struct privsep  *ps = p->p_ps;
> - int  res = 0, cmd = 0, verbose, ret;
> + int  res = 0, cmd = 0, verbose;
>   struct vmd_vm   *vm = NULL;
>   struct vm_terminate_params vtp;
>   struct vmop_id   vid;
> @@ -278,8 +278,12 @@ vmm_dispatch_parent(int fd, struct privs
>   case IMSG_VMDOP_RECEIVE_VM_REQUEST:
>   IMSG_SIZE_CHECK(imsg, );
>   memcpy(, imsg->data, sizeof(vmc));
> - ret = vm_register(ps, , ,
> - imsg->hdr.peerid, vmc.vmc_owner.uid);
> + if (vm_register(ps, , ,
> + imsg->hdr.peerid, vmc.vmc_owner.uid) != 0) {
> + res = errno;
> + cmd = IMSG_VMDOP_START_VM_RESPONSE;
> + break;
> + }
>   vm->vm_tty = imsg->fd;
>   vm->vm_state |= VM_STATE_RECEIVED;
>   vm->vm_state |= VM_STATE_PAUSED;
> @@ -328,6 +332,7 @@ vmm_dispatch_parent(int fd, struct privs
>   }
>   if (id == 0)
>   id = imsg->hdr.peerid;
> + /* FALLTHROUGH */
>   case IMSG_VMDOP_PAUSE_VM_RESPONSE:
>   case IMSG_VMDOP_UNPAUSE_VM_RESPONSE:
>   case IMSG_VMDOP_TERMINATE_VM_RESPONSE:
> 



Re: simplify rpki-client entity marshal

2021-12-28 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.12.28 16:57:48 +0100:
> This re-shuffles struct entity a bit and removes the unneeded has_data
> indicator. Both data and datasz are not null when data is present and null
> when there is no data. With this in mind the code becomes simpler.
> 

ok benno@

> -- 
> :wq Claudio
> 
> Index: extern.h
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.99
> diff -u -p -r1.99 extern.h
> --- extern.h  22 Dec 2021 09:35:14 -  1.99
> +++ extern.h  28 Dec 2021 15:40:55 -
> @@ -336,13 +336,12 @@ enum publish_type {
>   * and parsed.
>   */
>  struct entity {
> - enum rtype   type;  /* type of entity (not RTYPE_EOF) */
> + TAILQ_ENTRY(entity) entries;
>   char*file;  /* local path to file */
> - int  has_data;  /* whether data blob is specified */
>   unsigned char   *data;  /* optional data blob */
>   size_t   datasz;/* length of optional data blob */
>   int  talid; /* tal identifier */
> - TAILQ_ENTRY(entity) entries;
> + enum rtype   type;  /* type of entity (not RTYPE_EOF) */
>  };
>  TAILQ_HEAD(entityq, entity);
>  
> Index: main.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
> retrieving revision 1.169
> diff -u -p -r1.169 main.c
> --- main.c22 Dec 2021 09:35:14 -  1.169
> +++ main.c28 Dec 2021 15:39:11 -
> @@ -120,9 +120,7 @@ entity_read_req(struct ibuf *b, struct e
>   io_read_buf(b, >type, sizeof(ent->type));
>   io_read_buf(b, >talid, sizeof(ent->talid));
>   io_read_str(b, >file);
> - io_read_buf(b, >has_data, sizeof(ent->has_data));
> - if (ent->has_data)
> - io_read_buf_alloc(b, (void **)>data, >datasz);
> + io_read_buf_alloc(b, (void **)>data, >datasz);
>  }
>  
>  /*
> @@ -144,9 +142,7 @@ entity_write_req(const struct entity *en
>   io_simple_buffer(b, >type, sizeof(ent->type));
>   io_simple_buffer(b, >talid, sizeof(ent->talid));
>   io_str_buffer(b, ent->file);
> - io_simple_buffer(b, >has_data, sizeof(int));
> - if (ent->has_data)
> - io_buf_buffer(b, ent->data, ent->datasz);
> + io_buf_buffer(b, ent->data, ent->datasz);
>   io_close_buffer(, b);
>  }
>  
> @@ -194,11 +190,8 @@ entityq_add(char *file, enum rtype type,
>   p->type = type;
>   p->talid = talid;
>   p->file = file;
> - p->has_data = data != NULL;
> - if (p->has_data) {
> - p->data = data;
> - p->datasz = datasz;
> - }
> + p->data = data;
> + p->datasz = (data != NULL) ? datasz : 0;
>  
>   entity_queue++;
>  
> Index: parser.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
> retrieving revision 1.28
> diff -u -p -r1.28 parser.c
> --- parser.c  4 Nov 2021 18:26:48 -   1.28
> +++ parser.c  28 Dec 2021 15:40:04 -
> @@ -195,7 +195,7 @@ proc_parser_cert(const struct entity *en
>   STACK_OF(X509)  *chain;
>   STACK_OF(X509_CRL)  *crls;
>  
> - assert(!entp->has_data);
> + assert(entp->data == NULL);
>  
>   /* Extract certificate data and X509. */
>  
> @@ -274,7 +274,7 @@ proc_parser_root_cert(const struct entit
>   struct cert *cert;
>   X509*x509;
>  
> - assert(entp->has_data);
> + assert(entp->data != NULL);
>  
>   /* Extract certificate data and X509. */
>  
> @@ -525,7 +525,7 @@ parse_entity(struct entityq *q, struct m
>   tal_free(tal);
>   break;
>   case RTYPE_CER:
> - if (entp->has_data)
> + if (entp->data != NULL)
>   cert = proc_parser_root_cert(entp, f, flen);
>   else
>   cert = proc_parser_cert(entp, f, flen);
> 



Re: rpki-client: use single function to build basedir

2021-12-04 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.12.03 16:45:48 +0100:
> Currently ta, rrdp and rsync repositories use different functions to build
> their base path. This diff changes this so that all can use the same
> function.
> 
> This is a first step to introduce a common validated repository.

i think this is ok, benno@

> -- 
> :wq Claudio
> 
> Index: repo.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/repo.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 repo.c
> --- repo.c25 Nov 2021 14:03:40 -  1.14
> +++ repo.c3 Dec 2021 15:20:39 -
> @@ -212,26 +212,15 @@ RB_GENERATE(filepath_tree, filepath, ent
>  
>  /*
>   * Function to hash a string into a unique directory name.
> - * prefixed with dir.
> + * Returned hash needs to be freed.
>   */
>  static char *
> -hash_dir(const char *uri, const char *dir)
> +hash_dir(const char *uri)
>  {
> - const char hex[] = "0123456789abcdef";
>   unsigned char m[SHA256_DIGEST_LENGTH];
> - char hash[SHA256_DIGEST_LENGTH * 2 + 1];
> - char *out;
> - size_t i;
>  
>   SHA256(uri, strlen(uri), m);
> - for (i = 0; i < SHA256_DIGEST_LENGTH; i++) {
> - hash[i * 2] = hex[m[i] >> 4];
> - hash[i * 2 + 1] = hex[m[i] & 0xf];
> - }
> - hash[SHA256_DIGEST_LENGTH * 2] = '\0';
> -
> - asprintf(, "%s/%s", dir, hash);
> - return out;
> + return hex_encode(m, sizeof(m));
>  }
>  
>  /*
> @@ -239,13 +228,24 @@ hash_dir(const char *uri, const char *di
>   * as prefix. Skip the proto:// in URI but keep everything else.
>   */
>  static char *
> -rsync_dir(const char *uri, const char *dir)
> +repo_dir(const char *uri, const char *dir, int hash)
>  {
> - char *local, *out;
> + const char *local;
> + char *out, *hdir = NULL;
>  
> - local = strchr(uri, ':') + strlen("://");
> + if (hash) {
> + local = hdir = hash_dir(uri);
> + } else {
> + local = strchr(uri, ':');
> + if (local != NULL)
> + local += strlen("://");
> + else
> + local = uri;
> + }
>  
> - asprintf(, "%s/%s", dir, local);
> + if (asprintf(, "%s/%s", dir, local) == -1)
> + err(1, NULL);
> + free(hdir);
>   return out;
>  }
>  
> @@ -397,8 +397,7 @@ ta_get(struct tal *tal, int nofetch)
>  
>   if ((tr->descr = strdup(tal->descr)) == NULL)
>   err(1, NULL);
> - if (asprintf(>basedir, "ta/%s", tal->descr) == -1)
> - err(1, NULL);
> + tr->basedir = repo_dir(tal->descr, "ta", 0);
>  
>   /* steal URI infromation from TAL */
>   tr->urisz = tal->urisz;
> @@ -469,7 +468,7 @@ rsync_get(const char *uri, int nofetch)
>   SLIST_INSERT_HEAD(, rr, entry);
>  
>   rr->repouri = repo;
> - rr->basedir = rsync_dir(repo, "rsync");
> + rr->basedir = repo_dir(repo, "rsync", 0);
>  
>   if (noop || nofetch) {
>   rr->state = REPO_DONE;
> @@ -536,7 +535,7 @@ rrdp_get(const char *uri, int nofetch)
>  
>   if ((rr->notifyuri = strdup(uri)) == NULL)
>   err(1, NULL);
> - rr->basedir = hash_dir(uri, "rrdp");
> + rr->basedir = repo_dir(uri, "rrdp", 1);
>  
>   RB_INIT(>added);
>   RB_INIT(>deleted);
> 



Re: rpki-client: make maximum number of publication points to sync operator configurable

2021-11-25 Thread Sebastian Benoit
Job Snijders(j...@openbsd.org) on 2021.11.25 16:13:51 +:
> It might be advantageous to permit operators to optionally specify the
> maximum number of publication points with which rpki-client will
> synchronize.
> 
> For example: "doas rpki-client -m 1 -t /etc/rpki/ripe.tal" has as effect
> that only RIPE NCC's repository is contacted, but none of the delegated
> repositories.
> 
> This flag perhaps permits us to start shipping with a more conservative
> default than 1000, like 100 or 200. It is clear that encouraging the
> ecosystem to embrace 'Publish in Parent' is a saner model than everyone
> running their own delegation.
> 
> Thoughts?

Yes, if we dont have these configurable, we will have users running into
them eventually and be sad.

maxpubpoints sounds a bit funny, but i have no other idea.

code reads ok.

> 
> Kind regards,
> 
> Job
> 
> Index: extern.h
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.98
> diff -u -p -r1.98 extern.h
> --- extern.h  25 Nov 2021 14:03:40 -  1.98
> +++ extern.h  25 Nov 2021 16:08:04 -
> @@ -624,7 +624,4 @@ int   mkpath(const char *);
>  /* Maximum number of concurrent rsync processes. */
>  #define MAX_RSYNC_PROCESSES  16
>  
> -/* Maximum allowd repositories per tal */
> -#define MAX_REPO_PER_TAL 1000
> -
>  #endif /* ! EXTERN_H */
> Index: main.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
> retrieving revision 1.166
> diff -u -p -r1.166 main.c
> --- main.c25 Nov 2021 14:03:40 -  1.166
> +++ main.c25 Nov 2021 16:08:10 -
> @@ -69,6 +69,7 @@ int verbose;
>  int  noop;
>  int  rrdpon = 1;
>  int  repo_timeout = 15*60;
> +unsigned int maxpubpoints = 1000;
>  
>  struct stats  stats;
>  
> @@ -714,7 +715,7 @@ main(int argc, char *argv[])
>   "proc exec unveil", NULL) == -1)
>   err(1, "pledge");
>  
> - while ((c = getopt(argc, argv, "b:Bcd:e:f:jnorRs:t:T:vV")) != -1)
> + while ((c = getopt(argc, argv, "b:Bcd:e:f:jm:norRs:t:T:vV")) != -1)
>   switch (c) {
>   case 'b':
>   bind_addr = optarg;
> @@ -738,6 +739,11 @@ main(int argc, char *argv[])
>   case 'j':
>   outformats |= FORMAT_JSON;
>   break;
> + case 'm':
> + maxpubpoints = strtonum(optarg, 0, 10, );
> + if (errs)
> + errx(1, "-m: %s", errs);
> + break;
>   case 'n':
>   noop = 1;
>   break;
> @@ -1220,7 +1226,7 @@ usage:
>   fprintf(stderr,
>   "usage: rpki-client [-BcjnoRrVv] [-b sourceaddr] [-d cachedir]"
>   " [-e rsync_prog]\n"
> - "   [-s timeout] [-T table] [-t tal]"
> - " [outputdir]\n");
> + "   [-m maxpubpoints] [-s timeout] [-T table] "
> + "[-t tal] [outputdir]\n");
>   return 1;
>  }
> Index: repo.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/repo.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 repo.c
> --- repo.c25 Nov 2021 14:03:40 -  1.14
> +++ repo.c25 Nov 2021 16:08:17 -
> @@ -41,6 +41,7 @@ extern struct stats stats;
>  extern int   noop;
>  extern int   rrdpon;
>  extern int   repo_timeout;
> +extern unsigned int  maxpubpoints;
>  
>  enum repo_state {
>   REPO_LOADING = 0,
> @@ -1100,12 +1101,14 @@ ta_lookup(int id, struct tal *tal)
>   if ((rp->repouri = strdup(tal->descr)) == NULL)
>   err(1, NULL);
>  
> - if (++talrepocnt[id] >= MAX_REPO_PER_TAL) {
> - if (talrepocnt[id] == MAX_REPO_PER_TAL)
> - warnx("too many repositories under %s", tals[id]);
> + if (talrepocnt[id] >= maxpubpoints + 1) {
> + if (talrepocnt[id] == maxpubpoints)
> + warnx("too many publication points under %s", tals[id]);
>   nofetch = 1;
>   }
>  
> + talrepocnt[id]++;
> +
>   rp->ta = ta_get(tal, nofetch);
>  
>   return rp;
> @@ -1146,11 +1149,12 @@ repo_lookup(int id, const char *uri, con
>   if ((rp->notifyuri = strdup(notify)) == NULL)
>   err(1, NULL);
>  
> - if (++talrepocnt[id] >= MAX_REPO_PER_TAL) {
> - if (talrepocnt[id] == MAX_REPO_PER_TAL)
> - warnx("too many repositories under %s", tals[id]);
> + if (talrepocnt[id] >= maxpubpoints + 1) {
> + if (talrepocnt[id] == maxpubpoints)
> + warnx("too many publication points under %s", tals[id]);
>   nofetch = 1;
>   }
> + talrepocnt[id]++;
>  
>   /* try RRDP first if available */
>   if (notify != NULL)
> Index: rpki-client.8

Re: rpki-client rrdp regress test

2021-11-25 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.11.25 12:30:31 +0100:
> This add an RRDP regress test that checks basic operation.
> It checks some valid notification, snapshot and delta XML.
> There are also two XML attacks included (billion laughs and XXE).
> More bad XML files should be added.
> 
> Comments?

Thanks for implementing all that.

Looks good, put it in and see how it goes?

> -- 
> :wq Claudio
> 
> Index: Makefile.inc
> ===
> RCS file: /cvs/src/regress/usr.sbin/rpki-client/Makefile.inc,v
> retrieving revision 1.15
> diff -u -p -r1.15 Makefile.inc
> --- Makefile.inc  24 Oct 2021 17:54:28 -  1.15
> +++ Makefile.inc  24 Nov 2021 14:12:39 -
> @@ -8,6 +8,7 @@ PROGS += test-gbr
>  PROGS += test-mft
>  PROGS += test-roa
>  PROGS += test-tal
> +PROGS += test-rrdp
>  
>  .for p in ${PROGS}
>  REGRESS_TARGETS += run-regress-$p
> @@ -50,3 +51,35 @@ SRCS_test-tal+=test-tal.c tal.c ip.c io
>   encoding.c print.c dummy.c
>  run-regress-test-tal: test-tal
>   ./test-tal -v ${.CURDIR}/../tal/*.tal
> +
> +SRCS_test-rrdp+= test-rrdp.c rrdp_delta.c rrdp_notification.c \
> + rrdp_snapshot.c rrdp_util.c \
> + log.c encoding.c ip.c validate.c dummy.c
> +LDADD_test-rrdp+=-lexpat ${LDADD}
> +DPADD_test-rrdp+=${LIBEXPAT} ${DPADD}
> +run-regress-test-rrdp: test-rrdp
> + ./test-rrdp \
> + -n < ${.CURDIR}/../rrdp/notification.xml 2>&1 | tee rrdp-r1.out
> + cmp ${.CURDIR}/../rrdp/rrdp-r1.out rrdp-r1.out
> +
> + ./test-rrdp -S 8fe05c2e-047d-49e7-8398-cd4250a572b1 -N 50500 \
> + -n < ${.CURDIR}/../rrdp/notification.xml 2>&1 | tee rrdp-r2.out
> + cmp ${.CURDIR}/../rrdp/rrdp-r2.out rrdp-r2.out
> + 
> + ./test-rrdp -S 9b3f7e31-4979-ef8c-d818-73e4dadc3e6b -N 13755 \
> + -H 27571e365a4c87b51a03731415ce2118cc268d686db3209ae752f01ba2d74bc5 \
> + -d < ${.CURDIR}/../rrdp/delta.xml 2>&1 | tee rrdp-r3.out
> + cmp ${.CURDIR}/../rrdp/rrdp-r3.out rrdp-r3.out
> +
> + ./test-rrdp -S 7e7d2563-5bbb-40b0-8723-6a2e90c85d9e -N 28483 \
> + -H 2a051bfd199150fe6bcdc777d09e70fe1acdf239fbf98ba378a793685e5adb21 \
> + -s < ${.CURDIR}/../rrdp/snapshot.xml 2>&1 | tee rrdp-r4.out
> + cmp ${.CURDIR}/../rrdp/rrdp-r4.out rrdp-r4.out
> +
> + ./test-rrdp \
> + -n < ${.CURDIR}/../rrdp/xxe.xml 2>&1 | tee rrdp-r5.out
> + cmp ${.CURDIR}/../rrdp/rrdp-r5.out rrdp-r5.out
> +
> + ./test-rrdp \
> + -n < ${.CURDIR}/../rrdp/billion_lol.xml 2>&1 | tee rrdp-r6.out
> + cmp ${.CURDIR}/../rrdp/rrdp-r6.out rrdp-r6.out
> Index: test-rrdp.c
> ===
> RCS file: test-rrdp.c
> diff -N test-rrdp.c
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ test-rrdp.c   24 Nov 2021 17:34:37 -
> @@ -0,0 +1,338 @@
> +/*   $OpenBSD: rrdp.c,v 1.17 2021/10/29 09:27:36 claudio Exp $ */
> +/*
> + * Copyright (c) 2020 Nils Fisher 
> + * Copyright (c) 2021 Claudio Jeker 
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +#include "extern.h"
> +#include "rrdp.h"
> +
> +#define REGRESS_NOTIFY_URI   "https://rpki.example.com/notify.xml;
> +
> +#define MAX_SESSIONS 12
> +#define  READ_BUF_SIZE   (32 * 1024)
> +
> +#define RRDP_STATE_REQ   0x01
> +#define RRDP_STATE_WAIT  0x02
> +#define RRDP_STATE_PARSE 0x04
> +#define RRDP_STATE_PARSE_ERROR   0x08
> +#define RRDP_STATE_PARSE_DONE0x10
> +#define RRDP_STATE_HTTP_DONE 0x20
> +#define RRDP_STATE_DONE  (RRDP_STATE_PARSE_DONE | 
> RRDP_STATE_HTTP_DONE)
> +
> +struct rrdp {
> + TAILQ_ENTRY(rrdp)entry;
> + size_t   id;
> + char*notifyuri;
> + char*local;
> + char*last_mod;
> +
> + struct pollfd   *pfd;
> + int  infd;
> + int  state;
> + unsigned int file_pending;
> + unsigned int file_failed;
> + enum 

Re: snmpd: tweak listen on

2021-11-14 Thread Sebastian Benoit
If there is no obvious reason (i.e. be different because you need it for a
specific feature) why not to use the same host*() function as other parse.y?
it would be better to stay in sync with otehrr daemons. That way if there is
an issue in one daemon, we can fix it in all of them.

Or, to turn the argument around: if you have found a way to improve those
functions in parse.y, you could push for your changes to be applied to all
parse.y that use the same function.

This applies to other parse.y functions too. The more they deviate, the
harder maintanance becomes.

Martijn van Duren(openbsd+t...@list.imperialat.at) on 2021.11.14 00:23:59 +0100:
> On Sat, 2021-11-13 at 13:23 +, Stuart Henderson wrote:
> > On 2021/08/09 20:55, Martijn van Duren wrote:
> > > On Mon, 2021-08-09 at 11:57 +0200, Martijn van Duren wrote:
> > > > 
> > > > This diff fixes all of the above:
> > > > - Allow any to be used resolving to 0.0.0.0 and ::
> > > > - Set SO_REUSEADDR on sockets, so we can listen on both any and
> > > > ?? localhost
> > > > - Document that we listen on any by default
> > 
> > I've discovered a problem with this, if you have "family inet4" or
> > "family inet6" in resolv.conf then startup fails, either with the
> > implicit listen:
> > 
> > snmpd: Unexpected resolving of ::
> > 
> > or with explicit e.g. "listen on any snmpv3":
> > 
> > /etc/snmpd.conf:3: invalid address: any
> > 
> > Config-based workaround is e.g. "listen on 0.0.0.0 snmpv3"
> > 
> > Should host() use a specific ai_family instead of PF_UNSPEC where we
> > already know that it's a v4 or v6 address? Or just do like httpd/parse.y
> > where host() tries v4, then v6 if that fails, then dns?
> > 
> Actually, this diff isn't the cause of the regression, it's r1.60 of
> parse.y. Prior to that snmpd also used the v4->v6->dns method, similar
> to httpd. I removed these steps to simplify the code and because
> getaddrinfo(3) is supposed to do the same thing as host_v4 and host_v6
> do/did. To be more precise: host_v6 uses getaddrinfo(3), but with the
> AI_NUMERICHOST flag.
> 
> So this brings to light a discrepancy's between calling getaddrinfo(3)
> with and without AI_NUMERICHOST:
> When called without AI_NUMERICHOST the family keyword in resolv.conf(5)
> is "honoured", but when called with AI_NUMERICHOST the family keyword is
> ignored.
> 
> When looking at POSIX[0] it states:
> If the nodename argument is not null, it can be a descriptive name or
> can be an address string. If the specified address family is AF_INET,
> [IP6] [Option Start]  AF_INET6, [Option End] or AF_UNSPEC, valid
> descriptive names include host names. If the specified address family is
> AF_INET or AF_UNSPEC, address strings using Internet standard dot
> notation as specified in inet_addr are valid.
> 
> [IP6] [Option Start] If the specified address family is AF_INET6 or
> AF_UNSPEC, standard IPv6 text forms described in inet_ntop are valid.
> [Option End]
> 
> The way I read this text is that we should always test for numeric
> hostnames (within the requested family of ai_family) regardless of what
> is in resolv.conf and would make us consistent with the AI_NUMERICHOST
> case. This is also consistent with the phrasing in resolv.conf(5), which
> states "inet4 IPv4 queries.", since I don't consider calling
> inet_pton(3) (which is what is called internally) a query.
> Diff below does this and fixes the bug in snmpd(8).
> 
> As for the risks: I think these are nil: It only affects the few
> installs that have set family to a single value. For the people that
> do have it, it will change an IPv{4,6} literal to always be "resolved",
> but if you enter e.g. a v6 numeric address on a v4 only machines it will
> almost always error out on the next step (most probably EHOSTUNREACH or
> EADDRNOTAVAIL).
> 
> regress/lib/libc/asr is currently broken, but with some minimal tweaking
> all test-cases pass.
> 
> martijn@
> 
> [0] 
> https://pubs.opengroup.org/onlinepubs/9699919799/functions/getaddrinfo.html
> 
> Index: asr/getaddrinfo_async.c
> ===
> RCS file: /cvs/src/lib/libc/asr/getaddrinfo_async.c,v
> retrieving revision 1.57
> diff -u -p -r1.57 getaddrinfo_async.c
> --- asr/getaddrinfo_async.c   26 Jan 2021 12:27:28 -  1.57
> +++ asr/getaddrinfo_async.c   13 Nov 2021 23:17:43 -
> @@ -492,8 +492,8 @@ get_port(const char *servname, const cha
>  }
>  
>  /*
> - * Iterate over the address families that are to be queried. Use the
> - * list on the async context, unless a specific family was given in hints.
> + * Iterate over PF_INET and PF_INET6 unless a specific family was given in
> + * hints. Only to be used when resolving numeric address.
>   */
>  static int
>  iter_family(struct asr_query *as, int first)
> @@ -502,7 +502,7 @@ iter_family(struct asr_query *as, int fi
>   as->as_family_idx = 0;
>   if (as->as.ai.hints.ai_family != PF_UNSPEC)
>   return 

Re: give sppp(4) its own RTM_PROPOSAL priority

2021-11-10 Thread Sebastian Benoit
Theo de Raadt(dera...@openbsd.org) on 2021.11.10 09:46:32 -0700:
> Sebastien Marie  wrote:
> 
> > I just wonder about the system behaviour after building a new kernel
> > and rebooting to build userland: RTP_PROPOSAL_SOLICIT is changed and
> > kernel/userland will mismatch.
> > 
> > But UMB proposal was done this way too (moving RTP_PROPOSAL_SOLICIT to
> > next id). So disturb shouldn't be big.
> 
> This is a messy part of the route proposal mechanism. The messaging is a
> bit weird. We discussed eventually fixing, but never got around to it.
> 
> The route priority code rt_priority is abused to signal the DNS resolver
> choice priority.  resolvd sees proposals in the range RTP_PROPOSAL_STATIC
> to RTP_PROPOSAL_UMB (57 to 60), and uses this to sort output in the
> resolv.conf file.  RTP_PROPOSAL_SOLICIT is a totally different kind of
> message in the opposite direction.
> 
> Something bad might happens if a new prio code is placed on the other
> side of RTP_PROPOSAL_SOLICIT, so would not encourage that.
> 
> rtm_type indicates RTM_PROPOSAL for all DNS resolver messages, so the
> priority code can be totally unique.  Maybe we should move
> RTP_PROPOSAL_SOLICIT out of the sorted range.  like make it 1?.  The
> only goal being to make the range-to-be-sorted not contain it, even
> as we expand the priorities in the future...
> 
> This would break compatibility only one more time.

Yes.

RTP_PROPOSAL* is only used in RTM_PROPOSAL messages and thus
the range of values in rt_priority can overlap with the other RTP_ values.

If we move the RTP_PROPOSAL_SOLICIT we might as well at the same time move
the other RTP_PROPOSAL values to give us more room for additional devices.

It would be a "upgrade with a snapshot or your dns might not work"
situation.



rpki-client 7.5 has just been released

2021-11-09 Thread Sebastian Benoit
rpki-client 7.5 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 a BGP announcement. The program queries the
global 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, Theo Buehler, Theo de Raadt and Sebastian Benoit
as part of the OpenBSD Project.

This release includes the following changes to the previous release:

* Make rpki-client more resilient regarding untrusted input:
  - fail repository synchronisation after 15min runtime
  - limit the number of repositories per TAL
  - don't allow DOCTYPE definitions in RRDP XML files
  - fix detection of HTTP redirect loops.
* limit the number of concurrent rsync processes.
* fix CRLF in tal files.

rpki-client works on all operating systems with a libcrypto library
based on OpenSSL 1.1 or LibreSSL 3.3, and a libtls library compatible
with LibreSSL 3.3 or later.

rpki-client is known to compile and run on at least the following
operating systems: Alpine, CentOS, Debian, Fedora, FreeBSD, Red Hat,
Rocky, Ubuntu, macOS, and of course OpenBSD!

It is our hope that packagers take interest and help adapt
rpki-client-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.

Assistance to coordinate security issues is available via
secur...@openbsd.org.



OpenBSD Errata: November 9, 2021 (rpki-client)

2021-11-09 Thread Sebastian Benoit
An errata patch for rpki-client has been released for OpenBSD 6.9 and
OpenBSD 7.0.

rpki-client(8) should handle CA misbehaviours as soft-errors.

Binary updates for the amd64, i386 and arm64 platform are available
via the syspatch utility.  Source code patches can be found on the
respective errata page:

  https://www.openbsd.org/errata69.html
  https://www.openbsd.org/errata70.html



Re: [patch] httpd static gzip compression

2021-11-05 Thread Sebastian Benoit
Theo de Raadt(dera...@openbsd.org) on 2021.11.05 08:24:21 -0600:
> prx  wrote:
> 
> > I think this remark should be placed into perspective.
> > 
> > When a file is requested, its gzipped version is send if : 
> > * The client ask for it with appropriate header.
> > * The server admin configured httpd to do so **and** compressed files.
> 
> and if the gzipd file does not contain the contents as the ungzip'd
> file, then two different clients may get different results.

Yes, but it's the responsibility of whoever puts the content there to do the
correct thing.

If i put a html file into a web directory and call it foo.jpg i would not
expect a sensible result either.




Re: rpki-client show attr name in rrdp parse errors

2021-11-05 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.11.05 15:26:57 +0100:
> On Wed, Nov 03, 2021 at 12:58:17PM +0100, Claudio Jeker wrote:
> > In one place this is already done but this makes sure we show the bad
> > attribute in all cases where a non conforming attribute is found.
> 
> Found another bunch of those non conforming attribute errors. Adjust them
> as well.
> 
> OK?

ok

> -- 
> :wq Claudio
> 
> Index: rrdp_notification.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/rrdp_notification.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 rrdp_notification.c
> --- rrdp_notification.c   29 Oct 2021 09:27:36 -  1.9
> +++ rrdp_notification.c   5 Nov 2021 14:06:18 -
> @@ -141,7 +141,7 @@ start_notification_elem(struct notificat
>   continue;
>   }
>   PARSE_FAIL(p, "parse failed - non conforming "
> - "attribute found in notification elem");
> + "attribute '%s' found in notification elem", attr[i]);
>   }
>   if (!(has_xmlns && nxml->version && nxml->session_id && nxml->serial))
>   PARSE_FAIL(p, "parse failed - incomplete "
> @@ -185,7 +185,7 @@ start_snapshot_elem(struct notification_
>   continue;
>   }
>   PARSE_FAIL(p, "parse failed - non conforming "
> - "attribute found in snapshot elem");
> + "attribute '%s' found in snapshot elem", attr[i]);
>   }
>   if (hasUri != 1 || hasHash != 1)
>   PARSE_FAIL(p, "parse failed - incomplete snapshot attributes");
> @@ -239,7 +239,7 @@ start_delta_elem(struct notification_xml
>   continue;
>   }
>   PARSE_FAIL(p, "parse failed - non conforming "
> - "attribute found in snapshot elem");
> + "attribute '%s' found in snapshot elem", attr[i]);
>   }
>   /* Only add to the list if we are relevant */
>   if (hasUri != 1 || hasHash != 1 || delta_serial == 0)
> 



Re: [patch] httpd static gzip compression

2021-11-05 Thread Sebastian Benoit
Ingo Schwarze(schwa...@usta.de) on 2021.11.05 14:37:15 +0100:
> Hi Theo,
> 
> Theo de Raadt wrote on Thu, Nov 04, 2021 at 08:27:47AM -0600:
> > prx  wrote:
> >> On 2021/11/04 14:21, prx wrote:
> 
> >>> The attached patch add support for static gzip compression.
> >>> 
> >>> In other words, if a client support gzip compression, when "file" is
> >>> requested, httpd will check if "file.gz" is avaiable to serve.
> 
> >> This diff doesn't compress "on the fly".
> >> It's up to the webmaster to compress files **before** serving them.
> 
> > Does any other program work this way?
> 
> Yes.  The man(1) program does.  At least on the vast majority of
> Linux systems (at least those using the man-db implementation
> of man(1)), on FreeBSD, and on DragonFly BSD.
> 
> Those systems store most manual pages as gzipped man(7) and mdoc(7)
> files, and man(1) decompresses them every time a user wants to look
> at one of them.  You say "man ls", and what you get is actually
> /usr/share/man/man1/ls.1.gz or something like that.
> 
> For man(1), that is next to useless because du -sh /usr/share/man =
> 42.6M uncompressed.  But it has repeatedly caused bugs in the past.
> I would love to remove the feature from mandoc, but even though it is
> rarely used in OpenBSD (some ports installed gzipped manuals in the
> past, but i think the ports tree has been clean now for quite some
> time; you might still need the feature when installing software
> or unpacking foreign manual page packages without using ports)
> it would be a bad idea to remove it because it is too widely used
> elsewhere.  Note that even the old BSD man(1) supported it.
> 
> > Where you request one filename, and it gives you another?
> 
> You did not ask what web servers do, but we are discussing a patch to
> a webserver.  For this reason, let me note in passing that even some
> otherwise extremely useful sites get it very wrong the other way round:
> 
>  $ ftp https://manpages.debian.org/bullseye/coreutils/ls.1.en.gz
> Trying 130.89.148.77...
> Requesting https://manpages.debian.org/bullseye/coreutils/ls.1.en.gz
> 100% |**|  8050   00:00   
>  
> 8050 bytes received in 0.00 seconds (11.74 MB/s)
>  $ file ls.1.en.gz
> ls.1.en.gz: troff or preprocessor input text
>  $ grep -A 1 '^.SH NAME' ls.1.en.gz  
> .SH NAME
> ls \- list directory contents
>  $ gunzip ls.1.en.gz
> gunzip: ls.1.en.gz: unrecognized file format

But with this patch, you are not asking the webserver for 
https://manpages.debian.org/bullseye/coreutils/ls.1.en.gz

You would be asking for

https://exmaple.com/whatever/ls.1

and with Accept-Encoding: gzip in the http header, and the
webserver would then look if it has a file

/whatever/ls.1.gz

(instead of without .gz) in its document tree and send you that, with 
"Content-Encoding: gzip" http header.
And because of that header, your client will know that the data is gzipped
and will unzip it before writing the file to the output.

I.e. there is no such problem (unless the patch has a bug).

/Benno

> 
> > I have a difficult time understanding why gzip has to sneak it's way
> > into everything.
> > 
> > I always prefer software that does precisely what I expect it to do.
> 
> Certainly.
> 
> I have no strong opinion whether a webserver qualifies as "everything",
> though, nor did i look at the diff.  While all manpages are small in the
> real world, some web servers may have to store huge amounts of data that
> clients might request, so disk space might occasionally be an issue on
> a web server even in 2021.  Also, some websites deliver huge amounts of
> data to the client even when the user merely asked for some text (not sure
> such sites would consider running OpenBSD httpd(8), but whatever :) - when
> browsing the web, bandwidth is still occasionally an issue even in 2021,
> even though that is a rather absurd fact.
> 
> Yours,
>   Ingo
> 



Re: speedup io marshal in rpki-client

2021-11-05 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.11.05 09:18:15 +0100:
> Noticed the other day. The ip addr arrays and as number array are
> marshalled element by element which is not very efficent.
> All the data is in one big blob of memory so just use the basic io
> operations for a memory blob and ship the full array at once.
> 
> This seems to reduce runtime by 5-10% (in my unscientific testing).
> Also it makes the code a fair bit simpler.

can't test right now, but i think all the multiplications are correct.
You dropped some asserts(), but i guess thats fine.

ok benno@

> -- 
> :wq Claudio
> 
> Index: cert.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> retrieving revision 1.46
> diff -u -p -r1.46 cert.c
> --- cert.c4 Nov 2021 11:32:55 -   1.46
> +++ cert.c5 Nov 2021 07:53:50 -
> @@ -1225,34 +1225,6 @@ cert_free(struct cert *p)
>   free(p);
>  }
>  
> -static void
> -cert_ip_buffer(struct ibuf *b, const struct cert_ip *p)
> -{
> - io_simple_buffer(b, >afi, sizeof(enum afi));
> - io_simple_buffer(b, >type, sizeof(enum cert_ip_type));
> -
> - if (p->type != CERT_IP_INHERIT) {
> - io_simple_buffer(b, >min, sizeof(p->min));
> - io_simple_buffer(b, >max, sizeof(p->max));
> - }
> -
> - if (p->type == CERT_IP_RANGE)
> - ip_addr_range_buffer(b, >range);
> - else if (p->type == CERT_IP_ADDR)
> - ip_addr_buffer(b, >ip);
> -}
> -
> -static void
> -cert_as_buffer(struct ibuf *b, const struct cert_as *p)
> -{
> - io_simple_buffer(b, >type, sizeof(enum cert_as_type));
> - if (p->type == CERT_AS_RANGE) {
> - io_simple_buffer(b, >range.min, sizeof(uint32_t));
> - io_simple_buffer(b, >range.max, sizeof(uint32_t));
> - } else if (p->type == CERT_AS_ID)
> - io_simple_buffer(b, >id, sizeof(uint32_t));
> -}
> -
>  /*
>   * Write certificate parsed content into buffer.
>   * See cert_read() for the other side of the pipe.
> @@ -1260,18 +1232,15 @@ cert_as_buffer(struct ibuf *b, const str
>  void
>  cert_buffer(struct ibuf *b, const struct cert *p)
>  {
> - size_t   i;
> -
>   io_simple_buffer(b, >expires, sizeof(p->expires));
>   io_simple_buffer(b, >purpose, sizeof(p->purpose));
>   io_simple_buffer(b, >talid, sizeof(p->talid));
>   io_simple_buffer(b, >ipsz, sizeof(p->ipsz));
> - for (i = 0; i < p->ipsz; i++)
> - cert_ip_buffer(b, >ips[i]);
> -
>   io_simple_buffer(b, >asz, sizeof(p->asz));
> - for (i = 0; i < p->asz; i++)
> - cert_as_buffer(b, >as[i]);
> +
> + io_simple_buffer(b, p->ips, p->ipsz * sizeof(p->ips[0]));
> + io_simple_buffer(b, p->as, p->asz * sizeof(p->as[0]));
> +
>   io_str_buffer(b, p->mft);
>   io_str_buffer(b, p->notify);
>   io_str_buffer(b, p->repo);
> @@ -1282,34 +1251,6 @@ cert_buffer(struct ibuf *b, const struct
>   io_str_buffer(b, p->pubkey);
>  }
>  
> -static void
> -cert_ip_read(struct ibuf *b, struct cert_ip *p)
> -{
> - io_read_buf(b, >afi, sizeof(enum afi));
> - io_read_buf(b, >type, sizeof(enum cert_ip_type));
> -
> - if (p->type != CERT_IP_INHERIT) {
> - io_read_buf(b, >min, sizeof(p->min));
> - io_read_buf(b, >max, sizeof(p->max));
> - }
> -
> - if (p->type == CERT_IP_RANGE)
> - ip_addr_range_read(b, >range);
> - else if (p->type == CERT_IP_ADDR)
> - ip_addr_read(b, >ip);
> -}
> -
> -static void
> -cert_as_read(struct ibuf *b, struct cert_as *p)
> -{
> - io_read_buf(b, >type, sizeof(enum cert_as_type));
> - if (p->type == CERT_AS_RANGE) {
> - io_read_buf(b, >range.min, sizeof(uint32_t));
> - io_read_buf(b, >range.max, sizeof(uint32_t));
> - } else if (p->type == CERT_AS_ID)
> - io_read_buf(b, >id, sizeof(uint32_t));
> -}
> -
>  /*
>   * Allocate and read parsed certificate content from descriptor.
>   * The pointer must be freed with cert_free().
> @@ -1319,7 +1260,6 @@ struct cert *
>  cert_read(struct ibuf *b)
>  {
>   struct cert *p;
> - size_t   i;
>  
>   if ((p = calloc(1, sizeof(struct cert))) == NULL)
>   err(1, NULL);
> @@ -1328,19 +1268,17 @@ cert_read(struct ibuf *b)
>   io_read_buf(b, >purpose, sizeof(p->purpose));
>   io_read_buf(b, >talid, sizeof(p->talid));
>   io_read_buf(b, >ipsz, sizeof(p->ipsz));
> + io_read_buf(b, >asz, sizeof(p->asz));
>  
>   p->ips = calloc(p->ipsz, sizeof(struct cert_ip));
>   if (p->ips == NULL)
>   err(1, NULL);
> - for (i = 0; i < p->ipsz; i++)
> - cert_ip_read(b, >ips[i]);
> + io_read_buf(b, p->ips, p->ipsz * sizeof(p->ips[0]));
>  
> - io_read_buf(b, >asz, sizeof(p->asz));
>   p->as = calloc(p->asz, sizeof(struct cert_as));
>   if (p->as == NULL)
>   err(1, NULL);
> - for (i = 0; i < p->asz; i++)
> -   

Re: rpki-client X509_free XXX fix

2021-11-04 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.11.04 18:31:54 +0100:
> There is this bit in parser.c
>   X509_free(x509); // needed? XXX
> 
> As tb@ properly noted this X509_free() is needed because the cert_parse()
> returns an up referenced x509 pointer back.
> 
> I moved the X509_free() so the error cases become simpler and we no longer
> leak a reference on success. At least this is how I read the code.
> 
> OK?

ok

> -- 
> :wq Claudio
> 
> Index: parser.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 parser.c
> --- parser.c  4 Nov 2021 11:32:55 -   1.27
> +++ parser.c  4 Nov 2021 17:25:18 -
> @@ -232,12 +232,12 @@ proc_parser_cert(const struct entity *en
>   X509_STORE_CTX_cleanup(ctx);
>   sk_X509_free(chain);
>   sk_X509_CRL_free(crls);
> + X509_free(x509);
>  
>   cert->talid = a->cert->talid;
>  
>   /* Validate the cert to get the parent */
>   if (!valid_cert(entp->file, , cert)) {
> - X509_free(x509); // needed? XXX
>   cert_free(cert);
>   return NULL;
>   }
> @@ -247,7 +247,6 @@ proc_parser_cert(const struct entity *en
>*/
>   if (cert->purpose == CERT_PURPOSE_CA) {
>   if (!auth_insert(, cert, a)) {
> - X509_free(x509); // needed? XXX
>   cert_free(cert);
>   return NULL;
>   }
> @@ -318,20 +317,22 @@ proc_parser_root_cert(const struct entit
>   goto badcert;
>   }
>  
> + X509_free(x509);
> +
>   cert->talid = entp->talid;
>  
>   /*
>* Add valid roots to the RPKI auth tree.
>*/
>   if (!auth_insert(, cert, NULL)) {
> - X509_free(x509); // needed? XXX
>   cert_free(cert);
>   return NULL;
>   }
>  
>   return cert;
> +
>   badcert:
> - X509_free(x509); // needed? XXX
> + X509_free(x509);
>   cert_free(cert);
>   return NULL;
>  }
> 



Re: rpki-client better exit behaviour when something goes wrong

2021-11-04 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.11.04 18:43:10 +0100:
> On Thu, Nov 04, 2021 at 11:27:46AM -0600, Theo de Raadt wrote:
> > Claudio Jeker  wrote:
> > 
> > > This diff replaces the errx() call in the poll fd check with warnings plus
> > > an exit of the main event loop. It also prints an error in case not all
> > > files have been processed.
> > > 
> > > An example after kill -9 of the rsync process is:
> > > rpki-client: https://rrdp.lacnic.net/rrdp/notification.xml: loaded from 
> > > network
> > > rpki-client: poll[1]: hangup
> > > rpki-client: rsync terminated signal 9
> > 
> > I am not thrilled with giving people error messages about a system call 
> > (poll).
> > 
> > In this specific case, the actual issue is on the next like (it is a rsync
> > failure, which you caused I suspect)
> > 
> > Is that 2nd message not enough?
> > 
> > Could you set hangup = 1, but skip the warnx, so that the code just reacts
> > correctly?
> > 
> > Are there other circumstances (different types of fd), which do not make
> > it to a wait (rrdp?).  Can those report a nice termination message?
> > 
> 
> I agree that the hangup message is not very helpful. It is actually 
> the one message I did not alter. I'm happy to remove that message.
> For the errors from msgbuf_write() I think those are valid error messages
> and I feel the POLLERR|POLLNVAL fall into the same boat.
> 
> Updated diff below

i'm fine with that
ok benno@

> -- 
> :wq Claudio
> 
> Index: http.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/http.c,v
> retrieving revision 1.48
> diff -u -p -r1.48 http.c
> --- http.c4 Nov 2021 14:24:41 -   1.48
> +++ http.c4 Nov 2021 14:25:03 -
> @@ -159,7 +159,7 @@ static uint8_t *tls_ca_mem;
>  static size_t tls_ca_size;
>  
>  /* HTTP request API */
> -static void  http_req_new(size_t, char *, char *, int);
> +static void  http_req_new(size_t, char *, char *, int, int);
>  static void  http_req_free(struct http_request *);
>  static void  http_req_done(size_t, enum http_result, const char *);
>  static void  http_req_fail(size_t);
> @@ -507,7 +507,7 @@ http_resolv(struct addrinfo **res, const
>   * Create and queue a new request.
>   */
>  static void
> -http_req_new(size_t id, char *uri, char *modified_since, int outfd)
> +http_req_new(size_t id, char *uri, char *modified_since, int count, int 
> outfd)
>  {
>   struct http_request *req;
>   char *host, *port, *path;
> @@ -530,6 +530,7 @@ http_req_new(size_t id, char *uri, char 
>   req->path = path;
>   req->uri = uri;
>   req->modified_since = modified_since;
> + req->redirect_loop = count;
>  
>   TAILQ_INSERT_TAIL(, req, entry);
>  }
> @@ -1135,7 +1136,8 @@ http_redirect(struct http_connection *co
>   err(1, NULL);
>  
>   logx("redirect to %s", http_info(uri));
> - http_req_new(conn->req->id, uri, mod_since, outfd); 
> + http_req_new(conn->req->id, uri, mod_since, conn->req->redirect_loop,
> + outfd); 
>  
>   /* clear request before moving connection to idle */
>   http_req_free(conn->req);
> @@ -1867,7 +1869,7 @@ proc_http(char *bind_addr, int fd)
>   io_read_str(b, );
>  
>   /* queue up new requests */
> - http_req_new(id, uri, mod, b->fd);
> + http_req_new(id, uri, mod, 0, b->fd);
>   ibuf_free(b);
>   }
>   }
> Index: main.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
> retrieving revision 1.162
> diff -u -p -r1.162 main.c
> --- main.c4 Nov 2021 14:24:41 -   1.162
> +++ main.c4 Nov 2021 17:42:42 -
> @@ -1020,19 +1020,23 @@ main(int argc, char *argv[])
>   }
>  
>   for (i = 0; i < NPFD; i++) {
> - if (pfd[i].revents & (POLLERR|POLLNVAL))
> - errx(1, "poll[%zu]: bad fd", i);
> - if (pfd[i].revents & POLLHUP) {
> - warnx("poll[%zu]: hangup", i);
> + if (pfd[i].revents & (POLLERR|POLLNVAL)) {
> + warnx("poll[%zu]: bad fd", i);
>   hangup = 1;
>   }
> + if (pfd[i].revents & POLLHUP)
> + hangup = 1;
>   if (pfd[i].revents & POLLOUT) {
>   switch (msgbuf_write(queues[i])) {
>   case 0:
> - errx(1, "write[%zu]: "
> + warnx("write[%zu]: "
>   "connection closed", i);
> + hangup = 1;
> + break;
>   

Re: [patch] httpd static gzip compression

2021-11-04 Thread Sebastian Benoit
Theo de Raadt(dera...@openbsd.org) on 2021.11.04 08:53:13 -0600:
> Stuart Henderson  wrote:
> 
> > In some ways it would be better if it *did* compress on the fly, as then
> > you don't have so much to consider with the effect on block/match rules,
> > whether a request is passed to a fastcgi handler, etc. (But of course
> > then you have CPU use issues).
> 
> I don't want my webservers to perform unexpected compute.  Sending extra
> packets is cheaper than doing the compute. 

Exactly.

Other webservers can do this too. In apache you do it with rewrite rules, in
nginx there is the gzip_static option.

As to your question:

> Where you request one filename, and it gives you another?

With most web applications it is common that paths are rewritten. You dont
get the file at the path you request.
 
> > Not sure if it's still actually needed, but most web servers with gzip
> > support usually have a way to disable it per user-agent due to problems
> > that have occurred.

This was needed for old internet explorer version and when i was still in
this business, we stopped using such configuration about 6 years ago. I
don't think we have to care about that anymore.

All currently used browsers support compression _when they ask for it_

> I was not talking about other webservers.  I was talking about any other
> program going, "OH i see you have a .gz file, I cannot actually confirm it
> is a gzip of the non-gzip file, but here you go, here is the thing you
> didn't ask for".

Changing the content of what is served by a webserver depending not only on
the path but also on other headers (such as Accept-Language etc) has been a
feature of HTTP for ages.

Its the job of the administrator setting things up to make sure that the
content served is correct. I don't see a problem with that: the admin needs
to make sure that the correct files are in a directory, independent of what
type of file they are.

However, i think the feature needs to be optional, on a per directory basis.

If the patch is extended to be setable per directory, i'm willing to review
it further.



Re: ospfd/ospf6d, interfaces in log messages

2021-11-03 Thread Sebastian Benoit
Remi Locherer(remi.loche...@relo.ch) on 2021.11.03 22:23:44 +0100:
> On Tue, Nov 02, 2021 at 05:27:11PM +, Stuart Henderson wrote:
> > I've recently started seeing a number of flaps with ospfd/ospf6d
> > with invalid seq nums / "seq num mismatch, bad flags" logged.
> > Not quite sure what's going yet as they must be occurring on
> > various local switched segments on one nic and also on ethernet
> > wan circuits direct to router on a separate pcie nic, anyway
> > it's made it clear that very few of the log messages relating
> > to neighbours identify which interface is involved.
> > 
> > I don't know if it makes sense to commit or not, but there's a
> > diff below adding the interface wherever the neighbour ID is logged
> > if anyone's interested (same changes to both ospfd and ospf6d).
> > 
> > 
> > Nov  2 11:29:30  ospfd[78532]: recv_db_description: neighbor ID 
> > xx.2: invalid seq num, mine 20d22487 his 20d22485
> > Nov  2 11:29:30  ospf6d[89545]: recv_db_description: neighbor ID 
> > xx.2: invalid seq num, mine 4cabc5c1 his 4cabc5c0
> > Nov  2 11:29:34  ospf6d[89545]: recv_db_description: neighbor ID 
> > xx.1: invalid seq num, mine 98360a5 his 98360a4
> > Nov  2 11:29:34  ospfd[78532]: recv_db_description: neighbor ID 
> > xx.1: invalid seq num, mine f708c646 his f708c645
> > Nov  2 11:29:38  ospfd[78532]: recv_db_description: neighbor ID 
> > xx.11: invalid seq num, mine e4068bcc his e4068bcb
> > Nov  2 11:30:06  ospf6d[89545]: recv_db_description: neighbor ID 
> > xx.3: seq num mismatch, bad flags
> > Nov  2 11:30:14  ospf6d[89545]: recv_db_description: neighbor ID 
> > xx.1: invalid seq num, mine 98360ae his 98360ad
> > Nov  2 11:30:14  ospfd[78532]: recv_db_description: neighbor ID 
> > xx.1: invalid seq num, mine f708c64f his f708c64e
> > Nov  2 11:30:22  ospfd[78532]: recv_db_description: neighbor ID 
> > xx.2: invalid seq num, mine 20d22493 his 20d22490
> > Nov  2 11:30:22  ospfd[78532]: recv_db_description: neighbor ID 
> > xx.2: invalid seq num, mine 20d22493 his 20d22492
> > Nov  2 11:30:39  ospfd[78532]: recv_db_description: neighbor ID 
> > xx.2: invalid seq num, mine 20d2249c his 20d2249b
> > Nov  2 11:30:59  ospf6d[89545]: recv_db_description: neighbor ID 
> > xx.11: seq num mismatch, bad flags
> > Nov  2 11:30:59  ospfd[78532]: recv_db_description: neighbor ID 
> > xx.11: seq num mismatch, bad flags
> > Nov  2 11:31:09  ospfd[78532]: recv_db_description: neighbor ID 
> > xx.1: invalid seq num, mine f708c65c his f708c65b
> > 
> 
> I think this addition makes sense. Over which link a neighbor is connected
> can only be looked up via ospfctl. It's valuable having this info in the
> logs when analysing past events.

I thought i had okayed this yesterday already ;)

Makes complete sense.
 
> Diff reads fine, applies and compiles.

same.

> OK remi

ok benno

> 
> > 
> > Index: ospf6d/database.c
> > ===
> > RCS file: /cvs/src/usr.sbin/ospf6d/database.c,v
> > retrieving revision 1.20
> > diff -u -p -r1.20 database.c
> > --- ospf6d/database.c   15 Jul 2020 14:47:41 -  1.20
> > +++ ospf6d/database.c   2 Nov 2021 17:11:38 -
> > @@ -60,9 +60,9 @@ send_db_description(struct nbr *nbr)
> > case NBR_STA_INIT:
> > case NBR_STA_2_WAY:
> > case NBR_STA_SNAP:
> > -   log_debug("send_db_description: neighbor ID %s: "
> > +   log_debug("send_db_description: neighbor ID %s (%s): "
> > "cannot send packet in state %s", inet_ntoa(nbr->id),
> > -   nbr_state_name(nbr->state));
> > +   nbr->iface->name, nbr_state_name(nbr->state));
> > goto fail;
> > case NBR_STA_XSTRT:
> > bits |= OSPF_DBD_MS | OSPF_DBD_M | OSPF_DBD_I;
> > @@ -160,8 +160,8 @@ recv_db_description(struct nbr *nbr, cha
> > int  dupe = 0;
> >  
> > if (len < sizeof(dd_hdr)) {
> > -   log_warnx("recv_db_description: neighbor ID %s: "
> > -   "bad packet size", inet_ntoa(nbr->id));
> > +   log_warnx("recv_db_description: neighbor ID %s (%s): "
> > +   "bad packet size", inet_ntoa(nbr->id), nbr->iface->name);
> > return;
> > }
> > memcpy(_hdr, buf, sizeof(dd_hdr));
> > @@ -170,9 +170,10 @@ recv_db_description(struct nbr *nbr, cha
> >  
> > /* db description packet sanity checks */
> > if (ntohs(dd_hdr.iface_mtu) > nbr->iface->mtu) {
> > -   log_warnx("recv_db_description: neighbor ID %s: "
> > +   log_warnx("recv_db_description: neighbor ID %s (%s): "
> > "invalid MTU %d expected %d", inet_ntoa(nbr->id),
> > -   ntohs(dd_hdr.iface_mtu), nbr->iface->mtu);
> > +   nbr->iface->name, ntohs(dd_hdr.iface_mtu),
> > +   nbr->iface->mtu);
> > return;
> > }
> >  
> > @@ -180,8 +181,9 @@ 

OpenBSD Errata: October 31, 2021 (uipc)

2021-10-30 Thread Sebastian Benoit
An errata patch for the kernel has been released for OpenBSD 6.9 and
OpenBSD 7.0.

The kernel could leak memory when closing unix sockets.

Binary updates for the amd64, i386 and arm64 platform are available
via the syspatch utility.  Source code patches can be found on the
respective errata page:

  https://www.openbsd.org/errata69.html
  https://www.openbsd.org/errata70.html



rpki-client-7.4 released

2021-10-30 Thread Sebastian Benoit
rpki-client 7.4 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 a BGP announcement. The program queries the
global 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, Theo Buehler, Theo de Raadt and Sebastian Benoit
as part of the OpenBSD Project.

This release includes the following changes to the previous release:

* Added support for validating BGPsec Router Public Keys.
* Fix issues with chunked transfer encoding in the RRDP HTTP client.
* Cleanup and improvement of how IO is handled.
* Improvements in the way X509 certificates are verified.
* Make rpki-client more resilient regarding untrusted input:
  - Limit the allowed character set for filename listings on
Manifests.
  - Limit the length of SIA URIs.
  - Limit the size of certain untrusted inputs.
  - Don't exit on failures to parse x509 objects.
  - Limit the size of objects retreived via RRDP or RSYNC.
  - Limit the number of FileAndHash entries on a manifest.
  - Constrain RRDP such that the delta/snapshot files must be hosted
at the same host as the notification file.

rpki-client works on all operating systems with a libcrypto library
based on OpenSSL 1.1 or LibreSSL 3.3, and a libtls library compatible
with LibreSSL 3.3 or later.

rpki-client is known to compile and run on at least the following
operating systems: Alpine, CentOS, Debian, Fedora, FreeBSD, Red Hat,
Rocky, Ubuntu, macOS, and of course OpenBSD!

It is our hope that packagers take interest and help adapt
rpki-client-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.

Assistance to coordinate security issues is available via
secur...@openbsd.org.



OpenBSD Errata: October 31, 2021 (bpf)

2021-10-30 Thread Sebastian Benoit
An errata patch for the kernel has been released for OpenBSD 6.9 and
OpenBSD 7.0.

Opening /dev/bpf too quickly too often could lead to a kernel crash.

Binary updates for the amd64, i386 and arm64 platform are available
via the syspatch utility.  Source code patches can be found on the
respective errata page:

  https://www.openbsd.org/errata69.html
  https://www.openbsd.org/errata70.html



OpenBSD Errata: October 31, 2021 (nsd)

2021-10-30 Thread Sebastian Benoit
An errata patch for nsd(8) has been released for OpenBSD 7.0.

In certain configurations, nsd can be crashed remotely.

Binary updates for the amd64, i386 and arm64 platform are available
via the syspatch utility.  Source code patches can be found on the
respective errata page:

  https://www.openbsd.org/errata70.html



Re: demystify vport(4) in vport(4) and ifconfig(8)

2021-10-28 Thread Sebastian Benoit
David Gwynne(da...@gwynne.id.au) on 2021.10.29 07:02:14 +1000:
> On Thu, Oct 28, 2021 at 03:43:11PM +0100, Jason McIntyre wrote:
> > On Thu, Oct 28, 2021 at 04:53:39PM +1000, David Gwynne wrote:
> > > 
> > > 
> > > > On 28 Oct 2021, at 15:35, Jason McIntyre  wrote:
> > > > 
> > > > On Thu, Oct 28, 2021 at 01:27:17PM +1000, David Gwynne wrote:
> > > >> 
> > > >>> that strategy does rely on individual driver docs saying upfront that
> > > >>> they can be created though, even if using create is not common. i 
> > > >>> wonder if
> > > >>> ifconfig already knows what it can create, and could maybe be more
> > > >>> helpful if "create" without an ifname gave a hint.
> > > >> 
> > > >> dlg@rpi3b trees$ ifconfig -C
> > > >> aggr bpe bridge carp egre enc eoip etherip gif gre lo mgre mpe mpip 
> > > >> mpw nvgre pair pflog pflow pfsync ppp pppoe svlan switch tap tpmr 
> > > >> trunk tun veb vether vlan vport vxlan wg
> > > >> 
> > > >> the "interface can be created paragraph" is in most of the manpages for
> > > >> these drivers, except for pair, pfsync, pppoe, and vether (and
> > > >> veb/vport). some of them could be improved, eg, bpe and switch.
> > > >> 
> > > > 
> > > > oops, missed that flag!
> > > 
> > > maybe the doco for "create" in ifconfig.8 should refer back to it too.
> > > 
> > 
> > good idea - this fits in nicely with stuart's proposal to not list every
> > device.
> > 
> > > 
> > > > i had thought maybe if there was such an option, we wouldn;t need the
> > > > "if can be created" blurb in every page. but i suppose we do need to say
> > > > it somewhere.
> > > 
> > > the driver manpages are in a weird place because they're supposed to be 
> > > for programmers, and the ifconfig manpage is for "operators". however, 
> > > the driver pages have the "theory of operation" for their interfaces. so 
> > > you have the high level theory in the driver manpage, the way 99% of use 
> > > actually interact with interfaces in ifconfig.8, and then you go back to 
> > > the driver doco for the low level programming detail. it's not the best 
> > > sandwich.
> > > 
> > 
> > yep.
> > 
> > > > another issue is that the text is inconsistent across pages.
> > > 
> > > yeah, but that can be fixed easily.
> > > 
> > 
> > hopefully!
> > 
> > anyway, here' my proposal following your and sthen's advice.
> > ok?
> > 
> > jmc
> > 
> > Index: ifconfig.8
> > ===
> > RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
> > retrieving revision 1.377
> > diff -u -p -r1.377 ifconfig.8
> > --- ifconfig.8  27 Oct 2021 06:36:51 -  1.377
> > +++ ifconfig.8  28 Oct 2021 14:41:06 -
> > @@ -177,42 +177,9 @@ network.
> >  The default broadcast address is the address with a host part of all 1's.
> >  .It Cm create
> >  Create the specified network pseudo-device.
> > -At least the following devices can be created on demand:
> > -.Pp
> > -.Xr aggr 4 ,
> > -.Xr bpe 4 ,
> > -.Xr bridge 4 ,
> > -.Xr carp 4 ,
> > -.Xr egre 4 ,
> > -.Xr enc 4 ,
> > -.Xr eoip 4 ,
> > -.Xr etherip 4 ,
> > -.Xr gif 4 ,
> > -.Xr gre 4 ,
> > -.Xr lo 4 ,
> > -.Xr mgre 4 ,
> > -.Xr mpe 4 ,
> > -.Xr mpip 4 ,
> > -.Xr mpw 4 ,
> > -.Xr nvgre 4 ,
> > -.Xr pair 4 ,
> > -.Xr pflog 4 ,
> > -.Xr pflow 4 ,
> > -.Xr pfsync 4 ,
> > -.Xr ppp 4 ,
> > -.Xr pppoe 4 ,
> > -.Xr svlan 4 ,
> > -.Xr switch 4 ,
> > -.Xr tap 4 ,
> > -.Xr tpmr 4 ,
> > -.Xr trunk 4 ,
> > -.Xr tun 4 ,
> > -.Xr veb 4 ,
> > -.Xr vether 4 ,
> > -.Xr vlan 4 ,
> > -.Xr vport 4 ,
> > -.Xr vxlan 4 ,
> > -.Xr wg 4
> > +A list of devices which can be dynamically created may be shown with the
> > +.Fl C
> > +option.
> >  .It Cm debug
> >  Enable driver-dependent debugging code; usually, this turns on
> >  extra console error logging.
> 
> if solene@ doesnt think it hurts then i'm ok with it.

We tend to forget that there are other output formats for manpages as well.
right now, i can got to https://man.openbsd.org/ifconfig and jump from there
to all of these manpages. With them removed, i can no longer do that.

Not sure if thats a valid consideration though.



Re: openrsync add --max-size and --min-size support

2021-10-28 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.10.28 17:36:27 +0200:
> This diff should implement --max-size and --min-size almost equivalent to
> GNU rsync. I decided to use scan_scaled() instead of building something
> new that handles all the extra bits GNU rsync has.
> The remote rsync process gets the sizes in bytes so scaling is just a
> local issue.
> 
> Manpage probably needs more love.

rsync does not print a message about skipping a file, should we.

ok benno@

> -- 
> :wq Claudio
> 
> Index: Makefile
> ===
> RCS file: /cvs/src/usr.bin/rsync/Makefile,v
> retrieving revision 1.12
> diff -u -p -r1.12 Makefile
> --- Makefile  22 Oct 2021 11:10:34 -  1.12
> +++ Makefile  28 Oct 2021 14:06:07 -
> @@ -4,8 +4,8 @@ PROG= openrsync
>  SRCS=blocks.c client.c copy.c downloader.c fargs.c flist.c hash.c 
> ids.c \
>   io.c log.c main.c misc.c mkpath.c mktemp.c receiver.c rmatch.c \
>   rules.c sender.c server.c session.c socket.c symlinks.c uploader.c
> -LDADD+= -lcrypto -lm
> -DPADD+= ${LIBCRYPTO} ${LIBM}
> +LDADD+= -lcrypto -lm -lutil
> +DPADD+= ${LIBCRYPTO} ${LIBM} ${LIBUTIL}
>  MAN= openrsync.1
>  
>  CFLAGS+= -Wall -Wextra
> Index: extern.h
> ===
> RCS file: /cvs/src/usr.bin/rsync/extern.h,v
> retrieving revision 1.42
> diff -u -p -r1.42 extern.h
> --- extern.h  22 Oct 2021 11:10:34 -  1.42
> +++ extern.h  28 Oct 2021 13:59:20 -
> @@ -141,6 +141,8 @@ structopts {
>   int  numeric_ids;   /* --numeric-ids */
>   int  one_file_system;   /* -x */
>   int  alt_base_mode;
> + off_tmax_size;  /* --max-size */
> + off_tmin_size;  /* --min-size */
>   char*rsync_path;/* --rsync-path */
>   char*ssh_prog;  /* --rsh or -e */
>   char*port;  /* --port */
> Index: fargs.c
> ===
> RCS file: /cvs/src/usr.bin/rsync/fargs.c,v
> retrieving revision 1.20
> diff -u -p -r1.20 fargs.c
> --- fargs.c   22 Oct 2021 11:10:34 -  1.20
> +++ fargs.c   28 Oct 2021 14:09:23 -
> @@ -131,6 +131,10 @@ fargs_cmdline(struct sess *sess, const s
>   if (!sess->opts->specials && sess->opts->devices)
>   /* --devices is sent as -D --no-specials */
>   addargs(, "--no-specials");
> + if (sess->opts->max_size >= 0)
> + addargs(, "--max-size=%lld", sess->opts->max_size);
> + if (sess->opts->min_size >= 0)
> + addargs(, "--min-size=%lld", sess->opts->min_size);
>  
>   /* only add --compare-dest, etc if this is the sender */
>   if (sess->opts->alt_base_mode != 0 && 
> Index: main.c
> ===
> RCS file: /cvs/src/usr.bin/rsync/main.c,v
> retrieving revision 1.61
> diff -u -p -r1.61 main.c
> --- main.c28 Oct 2021 13:07:43 -  1.61
> +++ main.c28 Oct 2021 15:17:39 -
> @@ -26,6 +26,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "extern.h"
>  
> @@ -341,7 +342,7 @@ main(int argc, char *argv[])
>   pid_tchild;
>   int  fds[2], sd = -1, rc, c, st, i, lidx;
>   size_t   basedir_cnt = 0;
> - struct sess   sess;
> + struct sess  sess;
>   struct fargs*fargs;
>   char**args;
>   const char  *errstr;
> @@ -352,6 +353,8 @@ main(int argc, char *argv[])
>   NULL) == -1)
>   err(ERR_IPC, "pledge");
>  
> + opts.max_size = opts.min_size = -1;
> +
>   while ((c = getopt_long(argc, argv, "Dae:ghlnoprtvxz", lopts, ))
>   != -1) {
>   switch (c) {
> @@ -472,8 +475,12 @@ basedir:
>   opts.basedir[basedir_cnt++] = optarg;
>   break;
>   case OP_MAX_SIZE:
> + if (scan_scaled(optarg, _size) == -1)
> + err(1, "bad max-size");
> + break;
>   case OP_MIN_SIZE:
> - /* for now simply ignore */
> + if (scan_scaled(optarg, _size) == -1)
> + err(1, "bad min-size");
>   break;
>   case OP_VERSION:
>   fprintf(stderr, "openrsync: protocol version %u\n",
> Index: rsync.1
> ===
> RCS file: /cvs/src/usr.bin/rsync/rsync.1,v
> retrieving revision 1.27
> diff -u -p -r1.27 rsync.1
> --- rsync.1   22 Oct 2021 16:42:28 -  1.27
> +++ rsync.1   28 Oct 2021 15:29:42 -
> @@ -31,6 +31,8 @@
>  .Op Fl -exclude-from Ns = Ns Ar file
>  .Op Fl -include Ar pattern
>  .Op Fl -include-from Ns = Ns Ar file
> +.Op Fl -max-size Ns 

Re: demystify vport(4) in vport(4) and ifconfigt(8)

2021-10-26 Thread Sebastian Benoit
Solene Rapenne(sol...@perso.pw) on 2021.10.26 21:18:30 +0200:
> I tried to figure out how to use veb interfaces but the man page
> wasn't obvious in regards to the "vport" thing. It turns out it's
> a kind of interface that can be created with ifconfig.
> 
> I think we should make this clearer.
> 
> Because ifconfig(8) mentions many type of interfaces I've searched
> for "vport" without success while "most" types are referenced in
> the man page. Like I added veb(4) recently, the diff adds vport(4)
> and missing mpip(4) so a search would give a clue it's related to
> ifconfig.
> 
> in veb(4), I think we should add vport in the synposis because the
> man page is shared for veb and vport interfaces but at first look
> it seems only veb is a type of interface.
> 
> And finally, I added a mention that vport can be created with
> ifconfig(8) so it's really obvious. Maybe it's too much and can be
> removed.
> 
> comments? ok?

ok

i think this can be expanded more.

vether(4) has a similar function for bridge(4). Maybe some text of vether(4)
can be adapted and added to veb(4).

/Benno

> Index: share/man/man4//veb.4
> ===
> RCS file: /home/reposync/src/share/man/man4/veb.4,v
> retrieving revision 1.2
> diff -u -p -r1.2 veb.4
> --- share/man/man4//veb.4 23 Feb 2021 11:43:41 -  1.2
> +++ share/man/man4//veb.4 26 Oct 2021 19:10:17 -
> @@ -23,6 +23,7 @@
>  .Nd Virtual Ethernet Bridge network device
>  .Sh SYNOPSIS
>  .Cd "pseudo-device veb"
> +.Cd "pseudo-device vport"
>  .Sh DESCRIPTION
>  The
>  .Nm veb
> @@ -37,7 +38,9 @@ by
>  .Nm veb
>  by creating a
>  .Nm vport
> -interface and attaching it as a port to the bridge.
> +interface using
> +.Xr ifconfig 8
> +and attaching it as a port to the bridge.
>  From the perspective of the host network stack, a
>  .Nm vport
>  interface acts as a normal interface connected to an Ethernet
> Index: sbin/ifconfig//ifconfig.8
> ===
> RCS file: /home/reposync/src/sbin/ifconfig/ifconfig.8,v
> retrieving revision 1.375
> diff -u -p -r1.375 ifconfig.8
> --- sbin/ifconfig//ifconfig.8 18 Aug 2021 18:10:33 -  1.375
> +++ sbin/ifconfig//ifconfig.8 26 Oct 2021 19:13:27 -
> @@ -192,6 +192,7 @@ At least the following devices can be cr
>  .Xr lo 4 ,
>  .Xr mgre 4 ,
>  .Xr mpe 4 ,
> +.Xr mpip 4 ,
>  .Xr mpw 4 ,
>  .Xr nvgre 4 ,
>  .Xr pair 4 ,
> @@ -209,6 +210,7 @@ At least the following devices can be cr
>  .Xr veb 4 ,
>  .Xr vether 4 ,
>  .Xr vlan 4 ,
> +.Xr vport 4 ,
>  .Xr vxlan 4 ,
>  .Xr wg 4
>  .It Cm debug
> 



Re: route.8: nameserver command is not resolvd(8) specific

2021-10-26 Thread Sebastian Benoit
Klemens Nanni(k...@openbsd.org) on 2021.10.26 15:30:17 +:
> On Tue, Oct 26, 2021 at 04:06:20PM +0100, Jason McIntyre wrote:
> > On Tue, Oct 26, 2021 at 08:57:40AM -0600, Theo de Raadt wrote:
> > > Jason McIntyre  wrote:
> > > 
> > > > On Tue, Oct 26, 2021 at 12:21:52PM +, Klemens Nanni wrote:
> > > > > While designed to populate resolv.conf(5), there is nothing resolvd(8)
> > > > > specific about the route message route(8) sends.
> > > > > 
> > > > > At least unwind(8) consumes and acts upon it;  there might as well be
> > > > > other programs in ports that do such things, so describe it 
> > > > > generically
> > > > > while going into required detail for our selected base daemons.
> > > > > 
> > > > > OK?
> > > > > 
> > > > > NB:  I'll fix unwind.conf(5) wrt. `route nameserver' separately.
> > > > > 
> > > > 
> > > > hi.
> > > > 
> > > > i think it sounds weird to talk about sending things without saying
> > > > where they're going. maybe there is a better word than send? maybe this
> > > > command is just recording, or specifying, or ...? i don;t have a good
> > > > grasp of the process here.
> > > 
> > > "to the route socket"
> > > 
> > > Maybe rather than 'send', does it explain things better to say 
> > > 'broadcast',
> > > 
> > 
> > i think either saying to the route socket or broadcast are clear and
> > would read better.
> 
> The latter is great, thanks.
> 
> OK?

yes, this is good.

I don't think we should hide how all these tools work together. After all we
want users to understand how to use it. And while it is build to work out of
the box, there are certainly situations where one needs to know more.

Thanks.

> 
> 
> Index: route.8
> ===
> RCS file: /cvs/src/sbin/route/route.8,v
> retrieving revision 1.101
> diff -u -p -r1.101 route.8
> --- route.8   22 Oct 2021 18:31:12 -  1.101
> +++ route.8   26 Oct 2021 15:30:04 -
> @@ -169,11 +169,16 @@ only changes in that routing table will 
>  .Ar interface
>  .Op Ar address ...
>  .Xc
> -Send a list of up to five nameserver address proposals to
> -.Xr resolvd 8 .
> +Broadcast a list of up to five nameserver address proposals.
> +.Pp
> +.Xr unwind 8
> +will learn them and act according to
> +.Xr unwind.conf 5 .
> +.Pp
>  .Xr resolvd 8
>  will replace all existing nameservers for the given interface in
>  .Xr resolv.conf 5 .
> +.Pp
>  If no
>  .Ar address
>  argument is given, a request to remove the nameservers previously entered for
> 



Re: snmpd(8): Log correct engineid

2021-10-25 Thread Sebastian Benoit
ok

Martijn van Duren(openbsd+t...@list.imperialat.at) on 2021.10.21 08:45:51 +0100:
> ping
> 
> On Sun, 2021-09-26 at 10:22 +0200, Martijn van Duren wrote:
> > ober_get_nstring writes a pointer to buf and does not overwrite the
> > content of buf itself. So pushing an array in there will result in it
> > only writing the pointer address to the array, which is not exactly what
> > we want to show.
> > 
> > I choose to go for sizeof, instead of using the define to be a little
> > more on the safe side, but I didn't change SNMPD_MAXCONTEXNAMELEN to
> > keep the diff small.
> > 
> > OK?
> > 
> > martijn@
> > 
> > Index: snmpe.c
> > ===
> > RCS file: /cvs/src/usr.sbin/snmpd/snmpe.c,v
> > retrieving revision 1.76
> > diff -u -p -r1.76 snmpe.c
> > --- snmpe.c 6 Sep 2021 13:32:18 -   1.76
> > +++ snmpe.c 26 Sep 2021 08:19:59 -
> > @@ -237,7 +237,7 @@ snmpe_parse(struct snmp_message *msg)
> > long longerrval, erridx;
> > u_intclass;
> > char*comn;
> > -   char*flagstr, *ctxname;
> > +   char*flagstr, *ctxname, *engineid;
> > size_t   len;
> > struct sockaddr_storage *ss = >sm_ss;
> > struct ber_element  *root = msg->sm_req;
> > @@ -300,9 +300,12 @@ snmpe_parse(struct snmp_message *msg)
> > }
> >  
> > if (ober_scanf_elements(a, "{xxeS$}$",
> > -   >sm_ctxengineid, >sm_ctxengineid_len,
> > -   , , >sm_pdu) != 0)
> > +   , >sm_ctxengineid_len, , ,
> > +   >sm_pdu) != 0)
> > goto parsefail;
> > +   if (msg->sm_ctxengineid_len > sizeof(msg->sm_ctxengineid))
> > +   goto parsefail;
> > +   memcpy(msg->sm_ctxengineid, engineid, msg->sm_ctxengineid_len);
> > if (len > SNMPD_MAXCONTEXNAMELEN)
> > goto parsefail;
> > memcpy(msg->sm_ctxname, ctxname, len);
> > 
> 
> 



Re: relayd patch for websocket upgrade

2021-10-23 Thread Sebastian Benoit
commited,

Thanks for reporting and this and the patches, and sorry for the delay.

/Benno

Sebastian Benoit(be...@openbsd.org) on 2021.10.23 22:22:10 +0200:
> Jonathon Fletcher(jonathon.fletc...@gmail.com) on 2021.10.19 14:26:51 -0700:
> > On Sun, May 02, 2021 at 11:05:16AM -0700, Jonathon Fletcher wrote:
> > > On Sun, Mar 07, 2021 at 06:22:04PM -0800, Jonathon Fletcher wrote:
> > > > On Sun, Mar 07, 2021 at 06:46:33PM +0100, Marcus MERIGHI wrote:
> > > > > Hello Jonathon!
> > > > > 
> > > > > welcome to the party:
> > > > > 
> > > > > https://marc.info/?t=15833439123
> > > > > 
> > > > > especially the two comments by sthen@:
> > > > > 
> > > > > https://marc.info/?m=161349608614743
> > > > > https://marc.info/?m=16135019371
> > > > > 
> > > > > reyk@ removed from CC: on purpose: 
> > > > > https://twitter.com/reykfloeter/status/1284868070901776384
> > > > > 
> > > > > Marcus
> > > > > 
> > > > > jonathon.fletc...@gmail.com (Jonathon Fletcher), 2021.03.06 (Sat) 
> > > > > 21:02 (CET):
> > > > > > When relayd relays a connection upgrade to a websocket, it relays
> > > > > > the outbound "Connection: Upgrade" header from the interal server.
> > > > > > 
> > > > > > It also tags on a "Connection: close" header to the outbound
> > > > > > response - ie the response goes out with two "Connection"
> > > > > > header lines.
> > > > > > 
> > > > > > Chrome and Netscape work despite the double upgrade/close 
> > > > > > connection 
> > > > > > headers. Safari fails.
> > > > > > 
> > > > > > Small patch below against 6.8 to only send the "Connection: close"
> > > > > > header if we are not handling a http_status 101.
> > > > > > 
> > > > > > Thanks,
> > > > > > Jonathon
> > > > > > 
> > > > > > 
> > > > > > cvs -q -d /cvs diff -ub -rOPENBSD_6_8 usr.sbin/relayd/relay_http.c
> > > > > > 
> > > > > > 
> > > 
> > > snip
> > > 
> > > > Marcus,
> > > > 
> > > > I did not realize that there was already a party. Apologies for my
> > > > previous, duplicate, patch.
> > > > 
> > > > Reading through the thread, I came to the conclusion that the comments
> > > > worried that the previous patch(es) were not specific enough.
> > > > 
> > > > The current relayd behaviour is that outbound http responses have a
> > > > "Connection: close" header/value attached to them by relayd.
> > > > This can result in multiple "Connection" headers in the response
> > > > which is .. not good.
> > > > 
> > > > The current behaviour is because relayd does not handle repeated http
> > > > request/response sequences after the first one and prefers to force the
> > > > http session to close. Fortunately for websockets, the protocol after
> > > > the websocket upgrade is not http and so there is no need for relayd
> > > > to look for or process http headers after the upgrade.
> > > > 
> > > > Here is an updated patch. This avoids the incorrect current in-tree
> > > > behaviour in the following specific sitations:
> > > > 
> > > > 1: The headers for an outbound (internal -> external) response already
> > > >include "Connection: Upgrade", "Upgrade: websocket" and the config
> > > >permits websocket upgrades, or
> > > > 
> > > > 2: The headers for an outbound response already include a Connection
> > > >header with the value "close" - so do not send a duplicate as the
> > > >in-tree code currently does.
> > > > 
> > > > I think this is specfic enough for now. In order for a websocket upgrade
> > > > to work the external client has to request it and the internal server 
> > > > has to respond in agreement.
> > > > 
> > > > I am explicit about websocket upgrades in my configs: I require the
> > > > "Connection" and "Upgrade" headers in the rule that directs traffic
> 

Re: relayd patch for websocket upgrade

2021-10-23 Thread Sebastian Benoit
Jonathon Fletcher(jonathon.fletc...@gmail.com) on 2021.10.19 14:26:51 -0700:
> On Sun, May 02, 2021 at 11:05:16AM -0700, Jonathon Fletcher wrote:
> > On Sun, Mar 07, 2021 at 06:22:04PM -0800, Jonathon Fletcher wrote:
> > > On Sun, Mar 07, 2021 at 06:46:33PM +0100, Marcus MERIGHI wrote:
> > > > Hello Jonathon!
> > > > 
> > > > welcome to the party:
> > > > 
> > > > https://marc.info/?t=15833439123
> > > > 
> > > > especially the two comments by sthen@:
> > > > 
> > > > https://marc.info/?m=161349608614743
> > > > https://marc.info/?m=16135019371
> > > > 
> > > > reyk@ removed from CC: on purpose: 
> > > > https://twitter.com/reykfloeter/status/1284868070901776384
> > > > 
> > > > Marcus
> > > > 
> > > > jonathon.fletc...@gmail.com (Jonathon Fletcher), 2021.03.06 (Sat) 21:02 
> > > > (CET):
> > > > > When relayd relays a connection upgrade to a websocket, it relays
> > > > > the outbound "Connection: Upgrade" header from the interal server.
> > > > > 
> > > > > It also tags on a "Connection: close" header to the outbound
> > > > > response - ie the response goes out with two "Connection"
> > > > > header lines.
> > > > > 
> > > > > Chrome and Netscape work despite the double upgrade/close connection 
> > > > > headers. Safari fails.
> > > > > 
> > > > > Small patch below against 6.8 to only send the "Connection: close"
> > > > > header if we are not handling a http_status 101.
> > > > > 
> > > > > Thanks,
> > > > > Jonathon
> > > > > 
> > > > > 
> > > > > cvs -q -d /cvs diff -ub -rOPENBSD_6_8 usr.sbin/relayd/relay_http.c
> > > > > 
> > > > > 
> > 
> > snip
> > 
> > > Marcus,
> > > 
> > > I did not realize that there was already a party. Apologies for my
> > > previous, duplicate, patch.
> > > 
> > > Reading through the thread, I came to the conclusion that the comments
> > > worried that the previous patch(es) were not specific enough.
> > > 
> > > The current relayd behaviour is that outbound http responses have a
> > > "Connection: close" header/value attached to them by relayd.
> > > This can result in multiple "Connection" headers in the response
> > > which is .. not good.
> > > 
> > > The current behaviour is because relayd does not handle repeated http
> > > request/response sequences after the first one and prefers to force the
> > > http session to close. Fortunately for websockets, the protocol after
> > > the websocket upgrade is not http and so there is no need for relayd
> > > to look for or process http headers after the upgrade.
> > > 
> > > Here is an updated patch. This avoids the incorrect current in-tree
> > > behaviour in the following specific sitations:
> > > 
> > > 1: The headers for an outbound (internal -> external) response already
> > >include "Connection: Upgrade", "Upgrade: websocket" and the config
> > >permits websocket upgrades, or
> > > 
> > > 2: The headers for an outbound response already include a Connection
> > >header with the value "close" - so do not send a duplicate as the
> > >in-tree code currently does.
> > > 
> > > I think this is specfic enough for now. In order for a websocket upgrade
> > > to work the external client has to request it and the internal server 
> > > has to respond in agreement.
> > > 
> > > I am explicit about websocket upgrades in my configs: I require the
> > > "Connection" and "Upgrade" headers in the rule that directs traffic
> > > to the websocket pool:
> > > 
> > > 
> > > pass request quick \
> > > header "Host" value "ws.example.com" \
> > > header "Connection" value "Upgrade" \
> > > header "Upgrade" value "websocket" \
> > > forward to 
> > > 
> > > 
> > > This is for my use cases (tls accelerator) and relayd is adept at
> > > handling them. I really appreciate the functionality of relayd in base.
> > > 
> > > Let me know if there are specific concerns about the patch below or
> > > if there are specific preferred ways to accomplish better compliance
> > > with the RFC within the context of relayd.
> > > 
> > > Thanks,
> > > Jonathon
> > > 
> > > The Connection header is covered in:
> > > 
> > > https://tools.ietf.org/html/rfc7230#section-6
> > > 
> > 
> > Here is the same relayd websocket upgrade patch as above, but
> > against OPENBSD_6_9.
> > 
> > Thanks,
> > Jonathon
> 
> Hi, here is the same relayd websocket upgrade patch as above, but
> against OPENBSD_7_0. The OPENBSD_6_9 patch is almost the same, there
> were some minor changes (printing strings with "%s") that move one
> of the patch hunks a little.

Hi Jonathon,

your diff is ok benno@, i'd like to have an ok from claudio@ too.

rediffed with one whitespace change:

diff --git usr.sbin/relayd/relay_http.c usr.sbin/relayd/relay_http.c
index b1cfafd9718..3d6d65f18e7 100644
--- usr.sbin/relayd/relay_http.c
+++ usr.sbin/relayd/relay_http.c
@@ -177,6 +177,8 @@ relay_read_http(struct bufferevent *bev, void *arg)
size_t   size, linelen;
struct kv   *hdr 

Re: fix IO handling in rpki-client

2021-10-23 Thread Sebastian Benoit
ok benno@

Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.10.23 14:20:19 +0200:
> This diff changes the io read functions to work on ibufs.
> With this the poll loops will consume data with io_buf_read() until a full
> message is received and then that message is processed. Thanks to this
> the processes no longer block while waiting for more data in the io read
> functions.
> 
> With this also the blocking/nonblocking dance done in a few places is no
> longer needed. Everything is now nonblocking.
> 
> Further cleanup of the various marshaling functions can follow in a later
> step.
> -- 
> :wq Claudio
> 
> Index: cert.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 cert.c
> --- cert.c15 Oct 2021 22:30:33 -  1.39
> +++ cert.c23 Oct 2021 11:58:43 -
> @@ -1281,33 +1281,31 @@ cert_buffer(struct ibuf *b, const struct
>  }
>  
>  static void
> -cert_ip_read(int fd, struct cert_ip *p)
> +cert_ip_read(struct ibuf *b, struct cert_ip *p)
>  {
> -
> - io_simple_read(fd, >afi, sizeof(enum afi));
> - io_simple_read(fd, >type, sizeof(enum cert_ip_type));
> + io_read_buf(b, >afi, sizeof(enum afi));
> + io_read_buf(b, >type, sizeof(enum cert_ip_type));
>  
>   if (p->type != CERT_IP_INHERIT) {
> - io_simple_read(fd, >min, sizeof(p->min));
> - io_simple_read(fd, >max, sizeof(p->max));
> + io_read_buf(b, >min, sizeof(p->min));
> + io_read_buf(b, >max, sizeof(p->max));
>   }
>  
>   if (p->type == CERT_IP_RANGE)
> - ip_addr_range_read(fd, >range);
> + ip_addr_range_read(b, >range);
>   else if (p->type == CERT_IP_ADDR)
> - ip_addr_read(fd, >ip);
> + ip_addr_read(b, >ip);
>  }
>  
>  static void
> -cert_as_read(int fd, struct cert_as *p)
> +cert_as_read(struct ibuf *b, struct cert_as *p)
>  {
> -
> - io_simple_read(fd, >type, sizeof(enum cert_as_type));
> + io_read_buf(b, >type, sizeof(enum cert_as_type));
>   if (p->type == CERT_AS_RANGE) {
> - io_simple_read(fd, >range.min, sizeof(uint32_t));
> - io_simple_read(fd, >range.max, sizeof(uint32_t));
> + io_read_buf(b, >range.min, sizeof(uint32_t));
> + io_read_buf(b, >range.max, sizeof(uint32_t));
>   } else if (p->type == CERT_AS_ID)
> - io_simple_read(fd, >id, sizeof(uint32_t));
> + io_read_buf(b, >id, sizeof(uint32_t));
>  }
>  
>  /*
> @@ -1316,7 +1314,7 @@ cert_as_read(int fd, struct cert_as *p)
>   * Always returns a valid pointer.
>   */
>  struct cert *
> -cert_read(int fd)
> +cert_read(struct ibuf *b)
>  {
>   struct cert *p;
>   size_t   i;
> @@ -1324,35 +1322,36 @@ cert_read(int fd)
>   if ((p = calloc(1, sizeof(struct cert))) == NULL)
>   err(1, NULL);
>  
> - io_simple_read(fd, >valid, sizeof(int));
> - io_simple_read(fd, >expires, sizeof(time_t));
> - io_simple_read(fd, >purpose, sizeof(enum cert_purpose));
> - io_simple_read(fd, >ipsz, sizeof(size_t));
> + io_read_buf(b, >valid, sizeof(int));
> + io_read_buf(b, >expires, sizeof(time_t));
> + io_read_buf(b, >purpose, sizeof(enum cert_purpose));
> + io_read_buf(b, >ipsz, sizeof(size_t));
> +
>   p->ips = calloc(p->ipsz, sizeof(struct cert_ip));
>   if (p->ips == NULL)
>   err(1, NULL);
>   for (i = 0; i < p->ipsz; i++)
> - cert_ip_read(fd, >ips[i]);
> + cert_ip_read(b, >ips[i]);
>  
> - io_simple_read(fd, >asz, sizeof(size_t));
> + io_read_buf(b, >asz, sizeof(size_t));
>   p->as = calloc(p->asz, sizeof(struct cert_as));
>   if (p->as == NULL)
>   err(1, NULL);
>   for (i = 0; i < p->asz; i++)
> - cert_as_read(fd, >as[i]);
> + cert_as_read(b, >as[i]);
> +
> + io_read_str(b, >mft);
> + io_read_str(b, >notify);
> + io_read_str(b, >repo);
> + io_read_str(b, >crl);
> + io_read_str(b, >aia);
> + io_read_str(b, >aki);
> + io_read_str(b, >ski);
> + io_read_str(b, >tal);
> + io_read_str(b, >pubkey);
>  
> - io_str_read(fd, >mft);
>   assert(p->mft != NULL || p->purpose == CERT_PURPOSE_BGPSEC_ROUTER);
> - io_str_read(fd, >notify);
> - io_str_read(fd, >repo);
> - io_str_read(fd, >crl);
> - io_str_read(fd, >aia);
> - io_str_read(fd, >aki);
> - io_str_read(fd, >ski);
>   assert(p->ski);
> - io_str_read(fd, >tal);
> - io_str_read(fd, >pubkey);
> -
>   return p;
>  }
>  
> Index: extern.h
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.73
> diff -u -p -r1.73 extern.h
> --- extern.h  22 Oct 2021 11:13:06 -  1.73
> +++ extern.h  23 Oct 2021 12:03:19 -
> @@ -399,25 +399,25 @@ void tal_buffer(struct ibuf *, 

httpd request body too large in log

2021-10-23 Thread Sebastian Benoit
differentiate the third 413 from the other two in httpd.

ok?

diff --git usr.sbin/httpd/server_http.c usr.sbin/httpd/server_http.c
index 153829f4201..bf3fae05414 100644
--- usr.sbin/httpd/server_http.c
+++ usr.sbin/httpd/server_http.c
@@ -1406,7 +1406,7 @@ server_response(struct httpd *httpd, struct client *clt)
 
if (clt->clt_toread > 0 && (size_t)clt->clt_toread >
srv_conf->maxrequestbody) {
-   server_abort_http(clt, 413, NULL);
+   server_abort_http(clt, 413, "request body too large");
return (-1);
}
 



Re: [Possible patch] httpd and HEAD requests to CGI scripts

2021-10-23 Thread Sebastian Benoit
Ross L Richardson(open...@rlr.id.au) on 2021.10.09 21:40:50 +1100:
> This relates to the earlier messages I sent to bugs@ in:
>   https://marc.info/?t=16330937691=1=2
> 
> RFC 7231 [HTTP/1.1] section 4.3.2. "HEAD" states:
>   The HEAD method is identical to GET except that the server MUST NOT
>   send a message body in the response (i.e., the response terminates at
>   the end of the header section).
> 
> RFC 3875 [The Common Gateway Interface (CGI) Version 1.1] in
> section 4.3.2 HEAD states:
>   The HEAD method requests the script to do sufficient processing to
>   return the response header fields, without providing a response
>   message-body.  The script MUST NOT provide a response message-body
>   for a HEAD request.  If it does, then the server MUST discard the
>   message-body when reading the response from the script.
> 
> Therefore, a CGI script which sends a message body is violation of the CGI
> specification, but so is the server if it fails to elide the body.
> 
> 
> With httpd, we see (for example):
> 
> $ printf "HEAD /cgi-bin/ftplist.cgi?dbversion=1 
> HTTP/1.0\r\nHost:ftp.openbsd.org\r\n\r\n" \
> | nc -c ftp.openbsd.org https
> HTTP/1.0 200 OK
> Connection: close
> Content-type: text/plain
> Date: Fri, 01 Oct 2021 12:50:59 GMT
> Server: OpenBSD httpd
> 
> https://mirror.aarnet.edu.au/pub/OpenBSD  Canberra, Australia
> https://cdn.openbsd.org/pub/OpenBSD  Fastly (CDN)
> https://cloudflare.cdn.openbsd.org/pub/OpenBSD   Cloudflare (CDN)
> ...
> RND_BYTES=0xfe9832a3...
> 
> 
> So httpd isn't behaving correctly.
> 
> The patch below is offered in the hope that it is a starting point for
> a proper solution.  Whilst it solves the problem in a simple test case,
> I'm insufficiently familiar with the httpd code to know whether this is
> correct or sufficient!

Hi Ross,

your two diffs are a good start.

So here is a combined diff that does the following from your two patches:

* stop sending the content for head requests, even when its supplied by the
  fcgi.
* If the client sends an empty body without a Content-Lenght:
  do not add the Content-Lenght if it's a HEAD request.
  If it's a HEAD request, the Content-Lenght should show the size of the
  equivalent GET request, but we don't know how much that will be so
  don't lie.

I think your interpretation of the RFCs is correct.

Additionally, when the fcgi supplies a Content-Length header, do not remove
it and set Transfer-Encoding: chunked. Instead, leave the Content-Lenght
header in place, as obviously the fcgi knows how much data will come.

I tested this with some handwritten cgi scripts and slowcgi and with
nextcloud/php-fcgi.

comments, oks?

diff --git usr.sbin/httpd/server_fcgi.c usr.sbin/httpd/server_fcgi.c
index 0f06f001a33..29a016eb456 100644
--- usr.sbin/httpd/server_fcgi.c
+++ usr.sbin/httpd/server_fcgi.c
@@ -559,6 +559,12 @@ server_fcgi_read(struct bufferevent *bev, void *arg)
return;
}
}
+   /* Don't send content for HEAD requests */
+   if (clt->clt_fcgi.headerssent &&
+   ((struct http_descriptor *)
+   clt->clt_descreq)->http_method
+   == HTTP_METHOD_HEAD)
+   return;
if (server_fcgi_writechunk(clt) == -1) {
server_abort_http(clt, 500,
"encoding error");
@@ -621,29 +627,32 @@ server_fcgi_header(struct client *clt, unsigned int code)
/* Can't chunk encode an empty body. */
clt->clt_fcgi.chunked = 0;
 
-   /* But then we need a Content-Length... */
-   key.kv_key = "Content-Length";
-   if ((kv = kv_find(>http_headers, )) == NULL) {
-   if (kv_add(>http_headers,
-   "Content-Length", "0") == NULL)
-   return (-1);
+   /* But then we need a Content-Length unless method is HEAD... */
+   if (desc->http_method != HTTP_METHOD_HEAD) {
+   key.kv_key = "Content-Length";
+   if ((kv = kv_find(>http_headers, )) == NULL) {
+   if (kv_add(>http_headers,
+   "Content-Length", "0") == NULL)
+   return (-1);
+   }
}
}
 
-   /* Set chunked encoding */
+   /* Send chunked encoding header */
if (clt->clt_fcgi.chunked) {
-   /* XXX Should we keep and handle Content-Length instead? */
+   /* but only if no Content-Length header is supplied */
 

Re: ixl(4): add checksum receive offloading

2021-10-22 Thread Sebastian Benoit
Stuart Henderson(s...@spacehopper.org) on 2021.10.22 12:55:20 +0100:
> On 2021/10/22 11:25, Jan Klemkow wrote:
> > this diff add hardware checksum offloading for the receive path of
> > ixl(4) interfaces.
> 
> Would be good to have this tested with NFS if anyone has a way to do so.
> nics are probably better now but I'm pretty sure we have had problems
> with NFS and offloading in the past.

And ospf(6)d... mikeb hit that i believe?
 
> (there's a chance I might be able to test if my c27xx atom box still
> lives but you might be able to do it more easily)
> 



Re: isakmpd: prepare for opaque X509_EXTENSION

2021-10-21 Thread Sebastian Benoit
see the "if (csc == NULL)" error case below.

otherwise ok

Theo Buehler(t...@theobuehler.org) on 2021.10.21 13:45:43 +0200:
> On Thu, Oct 21, 2021 at 01:05:18PM +0200, Theo Buehler wrote:
> > This is the first of two diffs to prepare isakmpd for upcoming libcrypto
> > changes.  X509_EXTENSION will become opaque so we need to use an accessor.
> > I decided to leave accesses into ASN1_OCTET_STRING as they are for
> > readability (asn1_string_st is still exposed in OpenSSL's asn1.h).
> 
> Here's a second diff that deals with opaque X509_STORE_CTX.
> 
> There is a minor piece left that needs X509_OBJECT_{new,free}() to land
> in libcrypto.
> 
> Index: x509.c
> ===
> RCS file: /cvs/src/sbin/isakmpd/x509.c,v
> retrieving revision 1.120
> diff -u -p -r1.120 x509.c
> --- x509.c13 Oct 2021 16:57:43 -  1.120
> +++ x509.c21 Oct 2021 11:19:14 -
> @@ -109,7 +109,7 @@ x509_generate_kn(int id, X509 *cert)
>   "Conditions: %s >= \"%s\" && %s <= \"%s\";\n";
>   X509_NAME *issuer, *subject;
>   struct keynote_deckey dc;
> - X509_STORE_CTX csc;
> + X509_STORE_CTX *csc = NULL;
>   X509_OBJECT obj;
>   X509*icert;
>   RSA *key = NULL;
> @@ -154,24 +154,32 @@ x509_generate_kn(int id, X509 *cert)
>   RSA_free(key);
>   key = NULL;
>  
> + csc = X509_STORE_CTX_new();
> + if (csc == NULL) {
> + log_print("x509_generate_kn: failed to get memory for "
> + "certificate store");
> + goto fail;
> + }
> +
>   /* Now find issuer's certificate so we can get the public key.  */
> - X509_STORE_CTX_init(, x509_cas, cert, NULL);
> - if (X509_STORE_get_by_subject(, X509_LU_X509, issuer, ) !=
> + X509_STORE_CTX_init(csc, x509_cas, cert, NULL);
> + if (X509_STORE_get_by_subject(csc, X509_LU_X509, issuer, ) !=
>   X509_LU_X509) {
> - X509_STORE_CTX_cleanup();
> - X509_STORE_CTX_init(, x509_certs, cert, NULL);
> - if (X509_STORE_get_by_subject(, X509_LU_X509, issuer, )
> + X509_STORE_CTX_cleanup(csc);
> + X509_STORE_CTX_init(csc, x509_certs, cert, NULL);
> + if (X509_STORE_get_by_subject(csc, X509_LU_X509, issuer, )
>   != X509_LU_X509) {
> - X509_STORE_CTX_cleanup();
> + X509_STORE_CTX_cleanup(csc);
>   LOG_DBG((LOG_POLICY, 30,
>   "x509_generate_kn: no certificate found for "
>   "issuer"));
>   goto fail;
>   }
>   }
> - X509_STORE_CTX_cleanup();
> - icert = obj.data.x509;
> + X509_STORE_CTX_free(csc);
> + csc = NULL;
>  
> + icert = X509_OBJECT_get0_X509();
>   if (icert == NULL) {
>   LOG_DBG((LOG_POLICY, 30, "x509_generate_kn: "
>   "missing certificates, cannot construct X509 chain"));
> @@ -435,6 +443,7 @@ x509_generate_kn(int id, X509 *cert)
>   return 1;
>  
>  fail:
> + X509_STORE_CTX_free(csc);
>   free(buf);
>   free(skey);
>   free(ikey);
> @@ -812,25 +821,31 @@ x509_cert_get(u_int8_t *asn, u_int32_t l
>  int
>  x509_cert_validate(void *scert)
>  {
> - X509_STORE_CTX  csc;
> + X509_STORE_CTX  *csc;
>   X509_NAME   *issuer, *subject;
>   X509*cert = (X509 *) scert;
>   EVP_PKEY*key;
> - int res, err;
> + int res, err, flags;
>  
>   /*
>* Validate the peer certificate by checking with the CA certificates
>* we trust.
>*/
> - X509_STORE_CTX_init(, x509_cas, cert, NULL);
> + csc = X509_STORE_CTX_new();
> + if (csc == NULL) {
> + log_print("x509_cert_validate: failed to get memory for "
> + "certificate store");

return 0 ?

> + }
> + X509_STORE_CTX_init(csc, x509_cas, cert, NULL);
>   /* XXX See comment in x509_read_crls_from_dir.  */
> - if (x509_cas->param->flags & X509_V_FLAG_CRL_CHECK) {
> - X509_STORE_CTX_set_flags(, X509_V_FLAG_CRL_CHECK);
> - X509_STORE_CTX_set_flags(, X509_V_FLAG_CRL_CHECK_ALL);
> - }
> - res = X509_verify_cert();
> - err = csc.error;
> - X509_STORE_CTX_cleanup();
> + flags = X509_VERIFY_PARAM_get_flags(X509_STORE_get0_param(x509_cas));
> + if (flags & X509_V_FLAG_CRL_CHECK) {
> + X509_STORE_CTX_set_flags(csc, X509_V_FLAG_CRL_CHECK);
> + X509_STORE_CTX_set_flags(csc, X509_V_FLAG_CRL_CHECK_ALL);
> + }
> + res = X509_verify_cert(csc);
> + err = X509_STORE_CTX_get_error(csc);
> + X509_STORE_CTX_free(csc);
>  
>   /*
>* Return if validation succeeded or self-signed certs are not
> 



Re: isakmpd: prepare for opaque X509_EXTENSION

2021-10-21 Thread Sebastian Benoit
Theo Buehler(t...@theobuehler.org) on 2021.10.21 13:05:18 +0200:
> This is the first of two diffs to prepare isakmpd for upcoming libcrypto
> changes.  X509_EXTENSION will become opaque so we need to use an accessor.
> I decided to leave accesses into ASN1_OCTET_STRING as they are for
> readability (asn1_string_st is still exposed in OpenSSL's asn1.h).

reads ok
 
> Index: x509.c
> ===
> RCS file: /cvs/src/sbin/isakmpd/x509.c,v
> retrieving revision 1.120
> diff -u -p -r1.120 x509.c
> --- x509.c13 Oct 2021 16:57:43 -  1.120
> +++ x509.c21 Oct 2021 10:14:03 -
> @@ -1064,9 +1064,10 @@ x509_cert_obtain(u_int8_t *id, size_t id
>  int
>  x509_cert_subjectaltname(X509 *scert, u_int8_t **altname, u_int32_t *len)
>  {
> - X509_EXTENSION  *subjectaltname;
> - u_int8_t*sandata;
> - int extpos, santype, sanlen;
> + X509_EXTENSION  *subjectaltname;
> + ASN1_OCTET_STRING   *sanasn1data;
> + u_int8_t*sandata;
> + int  extpos, santype, sanlen;
>  
>   extpos = X509_get_ext_by_NID(scert, NID_subject_alt_name, -1);
>   if (extpos == -1) {
> @@ -1075,16 +1076,16 @@ x509_cert_subjectaltname(X509 *scert, u_
>   return 0;
>   }
>   subjectaltname = X509_get_ext(scert, extpos);
> + sanasn1data = X509_EXTENSION_get_data(subjectaltname);
>  
> - if (!subjectaltname || !subjectaltname->value ||
> - !subjectaltname->value->data ||
> - subjectaltname->value->length < 4) {
> + if (!subjectaltname || !sanasn1data || !sanasn1data->data ||
> + sanasn1data->length < 4) {
>   log_print("x509_cert_subjectaltname: invalid "
>   "subjectaltname extension");
>   return 0;
>   }
>   /* SSL does not handle unknown ASN stuff well, do it by hand.  */
> - sandata = subjectaltname->value->data;
> + sandata = sanasn1data->data;
>   santype = sandata[2] & 0x3f;
>   sanlen = sandata[3];
>   sandata += 4;
> @@ -1094,7 +1095,7 @@ x509_cert_subjectaltname(X509 *scert, u_
>* extra stuff in subjectAltName, so we will just take the first
>* salen bytes, and not worry about what follows.
>*/
> - if (sanlen + 4 > subjectaltname->value->length) {
> + if (sanlen + 4 > sanasn1data->length) {
>   log_print("x509_cert_subjectaltname: subjectaltname invalid "
>   "length");
>   return 0;
> 



Re: acme-client: don't reach into X509

2021-10-13 Thread Sebastian Benoit
Theo Buehler(t...@theobuehler.org) on 2021.10.13 13:55:14 +0200:
> In an upcoming libcrypto bump, we will make a few structs in libcrypto
> opaque. This needs a small change in acme-client.  Fetch the extension
> stack using X509_get0_extensions() and iterate using the stack API.
> Note that sk_*_num() returns -1 on NULL, so we won't enter the for loop
> and the extsz dance is unnecessary.
> 
> The first hunk is mostly whitespace. It only drops extsz and adds exts.

ok benno@


> 
> Index: revokeproc.c
> ===
> RCS file: /cvs/src/usr.sbin/acme-client/revokeproc.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 revokeproc.c
> --- revokeproc.c  2 Jan 2021 19:04:21 -   1.17
> +++ revokeproc.c  13 Oct 2021 10:44:57 -
> @@ -94,19 +94,20 @@ int
>  revokeproc(int fd, const char *certfile, int force,
>  int revocate, const char *const *alts, size_t altsz)
>  {
> - char*der = NULL, *dercp, *der64 = NULL;
> - char*san = NULL, *str, *tok;
> - int  rc = 0, cc, i, extsz, ssz, len;
> - size_t  *found = NULL;
> - BIO *bio = NULL;
> - FILE*f = NULL;
> - X509*x = NULL;
> - long lval;
> - enum revokeopop, rop;
> - time_t   t;
> - X509_EXTENSION  *ex;
> - ASN1_OBJECT *obj;
> - size_t   j;
> + char*der = NULL, *dercp, *der64 = NULL;
> + char*san = NULL, *str, *tok;
> + int  rc = 0, cc, i, ssz, len;
> + size_t  *found = NULL;
> + BIO *bio = NULL;
> + FILE*f = NULL;
> + X509*x = NULL;
> + long lval;
> + enum revokeopop, rop;
> + time_t   t;
> + const STACK_OF(X509_EXTENSION)  *exts;
> + X509_EXTENSION  *ex;
> + ASN1_OBJECT *obj;
> + size_t   j;
>  
>   /*
>* First try to open the certificate before we drop privileges
> @@ -164,13 +165,12 @@ revokeproc(int fd, const char *certfile,
>* command line.
>*/
>  
> - extsz = x->cert_info->extensions != NULL ?
> - sk_X509_EXTENSION_num(x->cert_info->extensions) : 0;
> + exts = X509_get0_extensions(x);
>  
>   /* Scan til we find the SAN NID. */
>  
> - for (i = 0; i < extsz; i++) {
> - ex = sk_X509_EXTENSION_value(x->cert_info->extensions, i);
> + for (i = 0; i < sk_X509_EXTENSION_num(exts); i++) {
> + ex = sk_X509_EXTENSION_value(exts, i);
>   assert(ex != NULL);
>   obj = X509_EXTENSION_get_object(ex);
>   assert(obj != NULL);
> 



Re: Variable type fix in parse.y (all of them)

2021-10-12 Thread Sebastian Benoit
Christian Weisgerber(na...@mips.inka.de) on 2021.10.12 12:49:24 +0200:
> Christian Weisgerber:
> 
> > Here's another attempt, incorporating millert's feedback and adding
> > a few more casts:
> 
> Any interest in this or not worth the churn and I should drop it?

i think it should go in.

ok benno@

btw, i'm not sure all of these need the pushback.
 
> > Index: bin/chio/parse.y
> > ===
> > RCS file: /cvs/src/bin/chio/parse.y,v
> > retrieving revision 1.23
> > diff -u -p -r1.23 parse.y
> > --- bin/chio/parse.y15 Oct 2020 19:42:56 -  1.23
> > +++ bin/chio/parse.y2 Oct 2021 19:42:06 -
> > @@ -179,9 +179,9 @@ lookup(char *s)
> >  
> >  #define MAXPUSHBACK128
> >  
> > -u_char *parsebuf;
> > +char   *parsebuf;
> >  int parseindex;
> > -u_char  pushback_buffer[MAXPUSHBACK];
> > +charpushback_buffer[MAXPUSHBACK];
> >  int pushback_index = 0;
> >  
> >  int
> > @@ -192,7 +192,7 @@ lgetc(int quotec)
> > if (parsebuf) {
> > /* Read character from the parsebuffer instead of input. */
> > if (parseindex >= 0) {
> > -   c = parsebuf[parseindex++];
> > +   c = (unsigned char)parsebuf[parseindex++];
> > if (c != '\0')
> > return (c);
> > parsebuf = NULL;
> > @@ -201,7 +201,7 @@ lgetc(int quotec)
> > }
> >  
> > if (pushback_index)
> > -   return (pushback_buffer[--pushback_index]);
> > +   return ((unsigned char)pushback_buffer[--pushback_index]);
> >  
> > if (quotec) {
> > if ((c = getc(file->stream)) == EOF) {
> > @@ -242,10 +242,10 @@ lungetc(int c)
> > if (parseindex >= 0)
> > return (c);
> > }
> > -   if (pushback_index < MAXPUSHBACK-1)
> > -   return (pushback_buffer[pushback_index++] = c);
> > -   else
> > +   if (pushback_index + 1 >= MAXPUSHBACK)
> > return (EOF);
> > +   pushback_buffer[pushback_index++] = c;
> > +   return (c);
> >  }
> >  
> >  int
> > @@ -272,8 +272,8 @@ findeol(void)
> >  int
> >  yylex(void)
> >  {
> > -   u_char   buf[8096];
> > -   u_char  *p;
> > +   char buf[8096];
> > +   char*p;
> > int  quotec, next, c;
> > int  token;
> >  
> > @@ -353,8 +353,8 @@ yylex(void)
> > } else {
> >  nodigits:
> > while (p > buf + 1)
> > -   lungetc(*--p);
> > -   c = *--p;
> > +   lungetc((unsigned char)*--p);
> > +   c = (unsigned char)*--p;
> > if (c == '-')
> > return (c);
> > }
> > Index: sbin/dhcpleased/parse.y
> > ===
> > RCS file: /cvs/src/sbin/dhcpleased/parse.y,v
> > retrieving revision 1.4
> > diff -u -p -r1.4 parse.y
> > --- sbin/dhcpleased/parse.y 20 Sep 2021 11:46:22 -  1.4
> > +++ sbin/dhcpleased/parse.y 2 Oct 2021 19:17:33 -
> > @@ -463,10 +463,10 @@ findeol(void)
> >  int
> >  yylex(void)
> >  {
> > -   unsigned charbuf[8096];
> > -   unsigned char   *p, *val;
> > -   int  quotec, next, c;
> > -   int  token;
> > +   char buf[8096];
> > +   char*p, *val;
> > +   int  quotec, next, c;
> > +   int  token;
> >  
> >  top:
> > p = buf;
> > @@ -502,7 +502,7 @@ top:
> > p = val + strlen(val) - 1;
> > lungetc(DONE_EXPAND);
> > while (p >= val) {
> > -   lungetc(*p);
> > +   lungetc((unsigned char)*p);
> > p--;
> > }
> > lungetc(START_EXPAND);
> > @@ -578,8 +578,8 @@ top:
> > } else {
> >  nodigits:
> > while (p > buf + 1)
> > -   lungetc(*--p);
> > -   c = *--p;
> > +   lungetc((unsigned char)*--p);
> > +   c = (unsigned char)*--p;
> > if (c == '-')
> > return (c);
> > }
> > Index: sbin/iked/parse.y
> > ===
> > RCS file: /cvs/src/sbin/iked/parse.y,v
> > retrieving revision 1.132
> > diff -u -p -r1.132 parse.y
> > --- sbin/iked/parse.y   18 Sep 2021 16:45:52 -  1.132
> > +++ sbin/iked/parse.y   2 Oct 2021 19:07:12 -
> > @@ -1510,10 +1510,10 @@ findeol(void)
> >  int
> >  yylex(void)
> >  {
> > -   unsigned charbuf[8096];
> > -   unsigned char   *p, *val;
> > -   int  quotec, next, c;
> > -   int  token;
> > +   char buf[8096];
> > +   char*p, *val;
> > +   int  quotec, next, c;
> > +   int  token;
> >  
> >  top:
> > p = buf;
> > @@ -1549,7 +1549,7 @@ top:
> > p = val + strlen(val) - 1;
> > 

Re: Relayd daily crash ca_dispatch_relay invalid

2021-10-02 Thread Sebastian Benoit
abyx...@mnetic.ch(abyx...@mnetic.ch) on 2021.10.01 09:56:32 -0400:
> On Fri, Oct 1, 2021, at 09:44, Stuart Henderson wrote:
> > On 2021/10/01 14:43, Stuart Henderson wrote:
> >> On 2021/10/01 09:29, abyx...@mnetic.ch wrote:
> >> > I'm getting a daily crash (call to fatalx). No clue what triggers it and 
> >> > the logging is really sparse. I couldn't follow what the code in ca.c is 
> >> > actually doing (what the hash belongs to that is triggering the crash). 
> >> > A snip from /var/log/daemon is reproduced below. There are no other log 
> >> > messages in any logs around the same time frame as the relayd shutdown. 
> >> > Also, that fd passing failed for https is concerning. Any suggestions in 
> >> > debugging this? OpenBSD 6.9, dmesg at bottom. 
> >> > 
> >> > 
> >> > grep relayd /var/log/daemon
> >> > 876:Sep 30 15:07:39 mnetic relayd[222]: adding 1 hosts from table 
> >> > axolotlfeeds:7053 (no check)
> >> > 877:Sep 30 15:07:39 mnetic relayd[44589]: adding 1 hosts from table 
> >> > axolotlfeeds:7053 (no check)
> >> > 878:Sep 30 15:07:39 mnetic relayd[57595]: adding 1 hosts from table 
> >> > axolotlfeeds:7053 (no check)
> >> > 909:Oct  1 00:42:20 mnetic relayd[95946]: ca: ca_dispatch_relay: invalid 
> >> > relay hash 
> >> > 'SHA256:f545fa75bd01d82c05359e21ae769d525c2971b8221a8e50e415fe3e62ea6553'
> >> > 910:Oct  1 00:42:20 mnetic relayd[44589]: relay: pipe closed
> >> > 911:Oct  1 00:42:20 mnetic relayd[8473]: hce exiting, pid 8473
> >> > 912:Oct  1 00:42:20 mnetic relayd[15293]: pfe exiting, pid 15293
> >> > 913:Oct  1 00:42:20 mnetic relayd[29437]: ca exiting, pid 29437
> >> > 914:Oct  1 00:42:20 mnetic relayd[52845]: ca exiting, pid 52845
> >> > 915:Oct  1 00:42:20 mnetic relayd[99285]: parent terminating, pid 99285
> >> > 916:Oct  1 00:42:20 mnetic relayd[222]: relay exiting, pid 222
> >> > 917:Oct  1 00:42:20 mnetic relayd[57595]: relay exiting, pid 57595
> >> > 921:Oct  1 09:12:59 mnetic relayd[2129]: startup
> >> > 922:Oct  1 09:12:59 mnetic relayd[2129]: config_setrelay: fd passing 
> >> > failed for `https': Too many open files
> >>
> >> ^^^
> >> 
> >> By default the limit for relayd is controlled by the openfiles settings
> >> for the "daemon" class in /etc/login.conf. You can check current use
> >> with fstat | grep -c relayd. If you need to raise it you can add
> >> something like this to the file, replacing XXX with "what you normally
> >> see when it's busy plus some extra leeway" and then "rcctl restart
> >> relayd" so it takes effect.
> >> 
> >> relayd:\
> >> :openfiles-max=XXX:\
> >> :openfiles-cur=XXX:\
> >> :tc=default:
> >
> > Hmm. Rereading your mail it might not be that..but if the limit is too
> > low you could have other things failing as a result..
> 
> Looks like I was using 154 (of 128) files, so I bumped openfiles-cur slightly 
> to accommodate that. 
> Unfortunately it's now a day or two wait to see if that was contributing to 
> the failure. 

relayd should not fatal in such a case like this, but handle this a bit more
graceful. Please let me know if bumping the limit fixes it.




OpenBSD Errata: September 30, 2021 (libressl)

2021-09-30 Thread Sebastian Benoit
An errata patch for LibreSSL has been released for OpenBSD 6.8 and
OpenBSD 6.9.

Compensate for the expiry of the DST Root X3 certificate.  The use of an
unnecessary expired certificate in certificate chains can cause validation
errors.

Binary updates for the amd64, i386 and arm64 platform are available
via the syspatch utility.  Source code patches can be found on the
respective errata page:

  https://www.openbsd.org/errata68.html
  https://www.openbsd.org/errata69.html



OpenBSD Errata: September 27, 2021 (libressl)

2021-09-26 Thread Sebastian Benoit
An errata patch for LibreSSL has been released for OpenBSD 6.8 and
OpenBSD 6.9.

A stack overread could occur when checking X.509 name constraints.

Binary updates for the amd64, i386 and arm64 platform are available
via the syspatch utility.  Source code patches can be found on the
respective errata page:

  https://www.openbsd.org/errata68.html
  https://www.openbsd.org/errata69.html



OpenBSD Errata: September 27, 2021 (sshd)

2021-09-26 Thread Sebastian Benoit
An errata patch for sshd(8) has been released for OpenBSD 6.8 and
OpenBSD 6.9.

  sshd(8) from OpenSSH 6.2 (OpenBSD 5.3) through 8.7 (OpenBSD 6.9) failed to
  correctly initialise supplemental groups when executing an
  AuthorizedKeysCommand or AuthorizedPrincipalsCommand, where a
  AuthorizedKeysCommandUser or AuthorizedPrincipalsCommandUser directive has
  been set to run the command as a different user. Instead these commands
  would inherit the groups that sshd(8) was started with.

  Depending on system configuration, inherited groups may allow
  AuthorizedKeysCommand/AuthorizedPrincipalsCommand helper programs to gain
  unintended privilege.

  Neither AuthorizedKeysCommand nor AuthorizedPrincipalsCommand are enabled
  by default in sshd_config(5).

Binary updates for the amd64, i386 and arm64 platform are available
via the syspatch utility.  Source code patches can be found on the
respective errata page:

  https://www.openbsd.org/errata68.html
  https://www.openbsd.org/errata69.html



rpki-client-7.3 released

2021-09-23 Thread Sebastian Benoit
rpki-client 7.3 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, Theo Buehler, Theo de Raadt and Sebastian Benoit
as part of the OpenBSD Project.

This release includes the following changes to the previous release:

 * Improve the HTTP client code (status code handling, http proxy
   support, keep-alive).
 * In RRDP, do not access URI with userinfo (@-sign)
 * Improve RRDP syncing by considering a notification file serial
   jumping backwards as synced repository.
 * Make -R (rsync only) also apply to the fetching of TA files.
 * Only sync *.{cer,crl,gbr,mft,roa} files via rsync and exclude all others.
 * When producing output for OpenBGPd, make use of the 'roa-set
   expires' attribute to prevent machines from loading outdated roa-sets.
 * In RRDP, limit the number of deltas to 300 per repo. If more deltas
   exist, downloading a full snapshot is faster.
 * Limit the validation depth of X509 certificate chains to 12, double
   the current depth seen in RPKI.

rpki-client works on all operating systems with a libcrypto library
based on OpenSSL 1.1 or LibreSSL 3.3, and a libtls library compatible
with LibreSSL 3.3 or later.

rpki-client is known to compile and run on at least the following
operating systems: Alpine, CentOS, Debian, Fedora, FreeBSD, Red Hat,
Rocky, Ubuntu, macOS, and of course OpenBSD!

It is our hope that packagers take interest and help adapt
rpki-client-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: less: tighten pledge in secure mode

2021-09-22 Thread Sebastian Benoit
Tobias Stoeckmann(tob...@stoeckmann.org) on 2021.09.21 22:23:55 +0200:
> Hi,
> 
> upstream (greenwood) less has disabled history file support for secure
> mode, i.e. LESSSECURE=1: https://github.com/gwsw/less/pull/201
> 
> The problem was about permanent marks for which we do not have support
> anyway. Users could possibly access files they should not be able to.
> 
> Since upstream does not allow history file in secure mode anymore we
> could do the same and remove wpath from secure mode pledge.
> 
> I have added a note about history file to our manual page.
> 
> Comments? Okays?

seems reasonable.

ok.


> Tobias
> 
> Index: cmdbuf.c
> ===
> RCS file: /cvs/src/usr.bin/less/cmdbuf.c,v
> retrieving revision 1.20
> diff -u -p -u -p -r1.20 cmdbuf.c
> --- cmdbuf.c  2 Sep 2019 14:07:45 -   1.20
> +++ cmdbuf.c  21 Sep 2021 20:16:08 -
> @@ -20,6 +20,7 @@
>  #include "cmd.h"
>  #include "less.h"
>  
> +extern int secure;
>  extern int sc_width;
>  extern int utf_mode;
>  
> @@ -1203,6 +1204,8 @@ init_cmdhist(void)
>   FILE *f;
>   char *p;
>  
> + if (secure)
> + return;
>   filename = histfile_name();
>   if (filename == NULL)
>   return;
> @@ -1274,6 +1277,8 @@ save_cmdhist(void)
>   struct stat statbuf;
>   int r;
>  
> + if (secure)
> + return;
>   if (mlist_search.modified)
>   modified = 1;
>   if (mlist_shell.modified)
> Index: less.1
> ===
> RCS file: /cvs/src/usr.bin/less/less.1,v
> retrieving revision 1.57
> diff -u -p -u -p -r1.57 less.1
> --- less.12 Sep 2019 14:07:45 -   1.57
> +++ less.121 Sep 2021 20:16:09 -
> @@ -1697,6 +1697,8 @@ Use of lesskey files.
>  .It Fl t
>  Use of tags files.
>  .It " "
> +Use of history file.
> +.It " "
>  Metacharacters in filenames, such as "*".
>  .It " "
>  Filename completion (TAB, ^L).
> Index: main.c
> ===
> RCS file: /cvs/src/usr.bin/less/main.c,v
> retrieving revision 1.37
> diff -u -p -u -p -r1.37 main.c
> --- main.c28 Jun 2019 05:44:09 -  1.37
> +++ main.c21 Sep 2021 20:16:09 -
> @@ -91,7 +91,7 @@ main(int argc, char *argv[])
>   secure = 1;
>  
>   if (secure) {
> - if (pledge("stdio rpath wpath tty", NULL) == -1) {
> + if (pledge("stdio rpath tty", NULL) == -1) {
>   perror("pledge");
>   exit(1);
>   }
> 



Re: pf.conf(5) & reply-to

2021-09-21 Thread Sebastian Benoit
Alexander Bluhm(alexander.bl...@gmx.net) on 2021.09.21 22:34:09 +0200:
> On Mon, Sep 20, 2021 at 03:54:58PM +0200, Landry Breuil wrote:
> > did i screwup something somewhere in my config and there's a better way
> > for that ?
> 
> This was changed in February.  No more interface, but gateway
> addresses.  It seems that some parts of the documentation were
> missed.
> 
> > should the manpage be improved for reply-to and talk about 'destination
> > address' instead of 'interface' like route-to does ?
> 
> Yes.
> 
> It looks like most information is in the commit message.
> https://marc.info/?l=openbsd-cvs=161213948819452=2

It's also on http://www.openbsd.org/faq/upgrade69.html



Re: rpki-client add back keep-alive to http requests

2021-09-12 Thread Sebastian Benoit


ok

Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.09.10 12:09:47 +0200:
> On Thu, Sep 09, 2021 at 09:18:04AM -0600, Bob Beck wrote:
> > 
> > ok beck@
> > 
> > On Thu, Sep 09, 2021 at 09:35:51AM +0200, Claudio Jeker wrote:
> > > While Connection: keep-alive should be the default it seems that at least
> > > some of the CA repositories fail to behave like that. Adding back the
> > > Connection header seems to fix this and delta downloads go faster again.
> > > 
> 
> After further inspection of the code and the HTTP/1.1 RFC I came to the
> conclusion that the code is wrong. For HTTP/1.1 the default should be
> keep-alive being on. For non-HTTP/1.1 (aka HTTP/1.0 there is no other 1.X
> spec) keep-alive should be off by default.
> 
> So look at the HTTP protocol header and set keep-alive then. This seems to
> work. Also check for Connection: close. At least one server sends a
> Connection: close header after a bunch of requests.
> 
> So this is a better fix for the keep-alive issue.
> -- 
> :wq Claudio
> 
> Index: http.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/http.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 http.c
> --- http.c1 Sep 2021 09:39:14 -   1.38
> +++ http.c10 Sep 2021 10:02:12 -
> @@ -1053,11 +1053,16 @@ http_request(struct http_connection *con
>  static int
>  http_parse_status(struct http_connection *conn, char *buf)
>  {
> +#define HTTP_11  "HTTP/1.1 "
>   const char *errstr;
>   char *cp, ststr[4];
>   char gerror[200];
>   int status;
>  
> + /* Check if the protocol is 1.1 and enable keep-alive in that case */
> + if (strncmp(buf, HTTP_11, strlen(HTTP_11)) == 0)
> + conn->keep_alive = 1;
> +
>   cp = strchr(buf, ' ');
>   if (cp == NULL) {
>   warnx("Improper response from %s", http_info(conn->host));
> @@ -1226,7 +1231,9 @@ http_parse_header(struct http_connection
>   } else if (strncasecmp(cp, CONNECTION, sizeof(CONNECTION) - 1) == 0) {
>   cp += sizeof(CONNECTION) - 1;
>   cp[strcspn(cp, " \t")] = '\0';
> - if (strcasecmp(cp, "keep-alive") == 0)
> + if (strcasecmp(cp, "close") == 0)
> + conn->keep_alive = 0;
> + else if (strcasecmp(cp, "keep-alive") == 0)
>   conn->keep_alive = 1;
>   } else if (strncasecmp(cp, LAST_MODIFIED,
>   sizeof(LAST_MODIFIED) - 1) == 0) {
> 



Re: iked(8): make proto option accept lists

2021-09-04 Thread Sebastian Benoit
Tobias Heider(tobias.hei...@stusta.de) on 2021.09.04 12:39:26 +0200:
> Here's an updated diff including the man page bits.

I don't want to bikeshed the manpage. The code is ok benno@ :)

> Looking at pf.conf(5)
> and ipsec.conf(5), there does not really seem to be a standard way to document
> which parameters accept lists.

This is because the both parsers define "lists" as "duplicate the rules for
all combinations".

The parsers of other programs (like bgpd, ospfd or relayd) have more
elaborate language constructs where simple expansion by duplicating the
line/rule is not sensible. This also applies to iked.

So i think one should look at those daemons and document the fact
that a specific keyword takes a { ... } list as argument, like this:

diff --git sbin/iked/iked.conf.5 sbin/iked/iked.conf.5
index df1a0f09442..6d8cb47cdbe 100644
--- sbin/iked/iked.conf.5
+++ sbin/iked/iked.conf.5
@@ -354,7 +354,13 @@ Note that this only matters for IKEv2 endpoints and does 
not
 restrict the traffic selectors to negotiate flows with different
 address families, e.g. IPv6 flows negotiated by IPv4 endpoints.
 .Pp
-.It Ic proto Ar protocol
+.It Xo
+.Ic proto Ar protocol
+.Xc
+.It Xo
+.Ic proto
+.Ic { Ar protocol ... Ic }
+.Xc
 The optional
 .Ic proto
 parameter restricts the flow to a specific IP protocol.
@@ -368,6 +374,14 @@ For a list of all the protocol name to number mappings 
used by
 see the file
 .Pa /etc/protocols .
 .Pp
+Multiple
+.Ar protocol
+entries can be specified, separated by commas or whitespace,
+if enclosed in curly brackets:
+.Bd -literal -offset indent
+proto { tcp, udp }
+.Ed
+.Pp
 .It Ic rdomain Ar number
 Specify a different routing domain for unencrypted traffic.
 The resulting IPsec SAs will match outgoing packets in the specified

Search for "Multiple" in bgpd.conf for more examples.

I CC jmc@, maybe he wants to help with the manpage.

> 
> Index: iked.conf.5
> ===
> RCS file: /cvs/src/sbin/iked/iked.conf.5,v
> retrieving revision 1.86
> diff -u -p -r1.86 iked.conf.5
> --- iked.conf.5   3 Aug 2021 12:46:30 -   1.86
> +++ iked.conf.5   4 Sep 2021 09:54:55 -
> @@ -93,6 +93,16 @@ keyword, for example:
>  .Bd -literal -offset indent
>  include "/etc/macros.conf"
>  .Ed
> +.Pp
> +Certain parameters can be expressed as lists, in which case
> +.Xr iked 8
> +generates all the necessary flow combinations.
> +For example:
> +.Bd -literal -offset indent
> +ikev2 esp proto { tcp, udp } \e
> + from 192.168.1.1 to 10.0.0.18 \e
> + peer 192.168.10.1
> +.Ed
>  .Sh MACROS
>  Macros can be defined that will later be expanded in context.
>  Macro names must start with a letter, digit, or underscore,
> Index: iked.h
> ===
> RCS file: /cvs/src/sbin/iked/iked.h,v
> retrieving revision 1.193
> diff -u -p -r1.193 iked.h
> --- iked.h1 Sep 2021 15:30:06 -   1.193
> +++ iked.h4 Sep 2021 09:54:55 -
> @@ -242,10 +242,9 @@ struct iked_policy {
>  
>  #define IKED_SKIP_FLAGS   0
>  #define IKED_SKIP_AF  1
> -#define IKED_SKIP_PROTO   2
> -#define IKED_SKIP_SRC_ADDR3
> -#define IKED_SKIP_DST_ADDR4
> -#define IKED_SKIP_COUNT   5
> +#define IKED_SKIP_SRC_ADDR2
> +#define IKED_SKIP_DST_ADDR3
> +#define IKED_SKIP_COUNT   4
>   struct iked_policy  *pol_skip[IKED_SKIP_COUNT];
>  
>   uint8_t  pol_flags;
> @@ -265,7 +264,8 @@ struct iked_policy {
>   int  pol_af;
>   int  pol_rdomain;
>   uint8_t  pol_saproto;
> - unsigned int pol_ipproto;
> + unsigned int pol_ipproto[IKED_IPPROTO_MAX];
> + unsigned int pol_nipproto;
>  
>   struct iked_addr pol_peer;
>   struct iked_static_idpol_peerid;
> Index: parse.y
> ===
> RCS file: /cvs/src/sbin/iked/parse.y,v
> retrieving revision 1.131
> diff -u -p -r1.131 parse.y
> --- parse.y   28 May 2021 18:01:39 -  1.131
> +++ parse.y   4 Sep 2021 09:54:55 -
> @@ -374,7 +374,7 @@ void   copy_transforms(unsigned int,
>   const struct ipsec_xf **, unsigned int,
>   struct iked_transform **, unsigned int *,
>   struct iked_transform *, size_t);
> -int   create_ike(char *, int, uint8_t,
> +int   create_ike(char *, int, struct ipsec_addr_wrap *,
>   int, struct ipsec_hosts *,
>   struct ipsec_hosts *, struct ipsec_mode *,
>   struct ipsec_mode *, uint8_t,
> @@ -388,9 +388,9 @@ 

Re: iked(8): make proto option accept lists

2021-09-03 Thread Sebastian Benoit
Florian Obser(flor...@openbsd.org) on 2021.09.03 11:32:42 +0200:
> On 2021-09-03 10:38 +02, Claudio Jeker  wrote:
> > On Fri, Sep 03, 2021 at 10:12:57AM +0200, Sebastian Benoit wrote:
> >> Tobias Heider(tobias.hei...@stusta.de) on 2021.09.02 15:39:46 +0200:
> >> > +;
> >> > +
> >> > +proto_list  : protoval  { $$ = $1; }
> >> > +| proto_list comma protoval {
> >> > +if ($3 == NULL)
> >> > +$$ = $1;
> >> > +else if ($1 == NULL)
> >> > +$$ = $3;
> >> > +else {
> >> > +$1->tail->next = $3;
> >> > +$1->tail = $3->tail;
> >> > +$$ = $1;
> >> > +}
> >> > +}
> >> 
> >> why dont you make it 
> >> 
> >>| protoval comma proto_list {
> >> 
> >> then you dont need the conditional.
> >
> > AFAIK yacc rules should be left expand. I don't fully remember the reason
> > but your example is not the common way.
> 
> because the parser itself is left recursive. If you make your grammar
> right recursive the parser needs to build a stack. Which is slower and
> possibly can overflow.

yes, but that does not really matter in such a short case. If the parser is
able to help you put your data structure together, why not let it help.



Re: iked(8): make proto option accept lists

2021-09-03 Thread Sebastian Benoit
Tobias Heider(tobias.hei...@stusta.de) on 2021.09.02 15:39:46 +0200:
> The diff below makes iked accept a list of protocols for the "proto" config
> option in iked.conf(5).
> This would allow us to have a single policy with "proto { ipencap, ipv6 }"
> to secure a gif(4) tunnel, instead of requiring one policy for each protocol.
> 
> ok?

I only gave the parser bits a quick read.

The manpage change would be nice to compare the parser with what you
document.

A bit more below.
 
> Index: iked.h
> ===
> RCS file: /cvs/src/sbin/iked/iked.h,v
> retrieving revision 1.193
> diff -u -p -r1.193 iked.h
> --- iked.h1 Sep 2021 15:30:06 -   1.193
> +++ iked.h2 Sep 2021 13:37:21 -
> @@ -242,10 +242,9 @@ struct iked_policy {
>  
>  #define IKED_SKIP_FLAGS   0
>  #define IKED_SKIP_AF  1
> -#define IKED_SKIP_PROTO   2
> -#define IKED_SKIP_SRC_ADDR3
> -#define IKED_SKIP_DST_ADDR4
> -#define IKED_SKIP_COUNT   5
> +#define IKED_SKIP_SRC_ADDR2
> +#define IKED_SKIP_DST_ADDR3
> +#define IKED_SKIP_COUNT   4
>   struct iked_policy  *pol_skip[IKED_SKIP_COUNT];
>  
>   uint8_t  pol_flags;
> @@ -265,7 +264,8 @@ struct iked_policy {
>   int  pol_af;
>   int  pol_rdomain;
>   uint8_t  pol_saproto;
> - unsigned int pol_ipproto;
> + unsigned int pol_ipproto[IKED_IPPROTO_MAX];
> + unsigned int pol_nipproto;
>  
>   struct iked_addr pol_peer;
>   struct iked_static_idpol_peerid;
> Index: parse.y
> ===
> RCS file: /cvs/src/sbin/iked/parse.y,v
> retrieving revision 1.131
> diff -u -p -r1.131 parse.y
> --- parse.y   28 May 2021 18:01:39 -  1.131
> +++ parse.y   2 Sep 2021 13:37:21 -
> @@ -374,7 +374,7 @@ void   copy_transforms(unsigned int,
>   const struct ipsec_xf **, unsigned int,
>   struct iked_transform **, unsigned int *,
>   struct iked_transform *, size_t);
> -int   create_ike(char *, int, uint8_t,
> +int   create_ike(char *, int, struct ipsec_addr_wrap *,
>   int, struct ipsec_hosts *,
>   struct ipsec_hosts *, struct ipsec_mode *,
>   struct ipsec_mode *, uint8_t,
> @@ -388,9 +388,9 @@ uint8_tx2i(unsigned char *);
>  int   parsekey(unsigned char *, size_t, struct iked_auth *);
>  int   parsekeyfile(char *, struct iked_auth *);
>  void  iaw_free(struct ipsec_addr_wrap *);
> -static intcreate_flow(struct iked_policy *pol, struct 
> ipsec_addr_wrap *ipa,
> +static intcreate_flow(struct iked_policy *pol, int, struct 
> ipsec_addr_wrap *ipa,
>   struct ipsec_addr_wrap *ipb);
> -static intexpand_flows(struct iked_policy *, struct 
> ipsec_addr_wrap *,
> +static intexpand_flows(struct iked_policy *, int, struct 
> ipsec_addr_wrap *,
>   struct ipsec_addr_wrap *);
>  static struct ipsec_addr_wrap *
>expand_keyword(struct ipsec_addr_wrap *);
> @@ -407,7 +407,6 @@ typedef struct {
>   uint8_t  ikemode;
>   uint8_t  dir;
>   uint8_t  satype;
> - uint8_t  proto;
>   char*string;
>   uint16_t port;
>   struct ipsec_hosts  *hosts;
> @@ -415,6 +414,7 @@ typedef struct {
>   struct ipsec_addr_wrap  *anyhost;
>   struct ipsec_addr_wrap  *host;
>   struct ipsec_addr_wrap  *cfg;
> + struct ipsec_addr_wrap  *proto;
>   struct {
>   char*srcid;
>   char*dstid;
> @@ -449,8 +449,7 @@ typedef struct {
>  %token NUMBER
>  %type  string
>  %type  satype
> -%type   proto
> -%type  protoval
> +%type   proto proto_list protoval
>  %type   hosts hosts_list
>  %typeport
>  %type  portval af rdomain
> @@ -632,8 +631,21 @@ af   : /* empty */   { $$ = 
> AF_UNSPEC; }
>  
>  proto: /* empty */   { $$ = 0; }
>   | PROTO protoval{ $$ = $2; }
> - | PROTO ESP

Re: Removal of old users and groups in the upgrade notes

2021-09-02 Thread Sebastian Benoit
Sebastian Benoit(be...@openbsd.org) on 2021.09.02 21:41:15 +0200:
> Florian Obser(flor...@openbsd.org) on 2021.09.02 14:04:22 +0200:
> > On 2021-09-02 12:26 +02, Sebastian Benoit  wrote:
> > > Raf Czlonka(rczlo...@gmail.com) on 2021.09.02 10:51:19 +0100:
> > >> Ping.
> > >> 
> > >> On Mon, May 24, 2021 at 05:06:08PM BST, Raf Czlonka wrote:
> > >> > Ping.
> > >> > 
> > >> > On Sun, May 09, 2021 at 01:07:15PM BST, Raf Czlonka wrote:
> > >> > > Hello,
> > >> > > 
> > >> > > This is both a general question and specific example of removal of
> > >> > > old users and groups.
> > >> > > 
> > >> > > With the release of 6.7, rebound(8) got tedued[0] ;^)
> > >> > > However, there were no specific instructions regarding removal of
> > >> > > _rebound user and group, or the /etc/rebound.conf file, in the
> > >> > > upgrade notes - I had the latter added to my /etc/sysclean.ignore
> > >> > > file and didn't notice it until today.
> > >> > > 
> > >> > > Grepping for '{user,group}del' under 'faq/upgrade*', shows a handful
> > >> > > of examples - some are straight after a user or a group has been
> > >> > > retired[1], others when the UID/GID got recycled[2].
> > >> > > 
> > >> > > Probably too little too late but, should the 6.7 upgrade page get:
> > >> > > 
> > >> > >  # rm /etc/rebound.conf
> > >> > >  # userdel _rebound
> > >> > >  # groupdel _rebound
> > >> > > 
> > >> > > instructions added, i.e. need I bother you with a diff, or will you
> > >> > > simply add it once the UID/GID gets recycled?
> > >
> > > You cannot change the 6.7 upgrade notes now and expect anyone to apply 
> > > that.
> > >
> > > That is, the remove instructions will need to be given when the UID/GID is
> > > recycled. That is completly fine, we have done it like this in the past.
> > >
> > 
> > Not even that, experience with iirc slaacd showed that sysmerge is
> > complaining loudly when a uid is recycled.

So, if anyone thinks this is useful to do now, let's add it to current.html
so that it ends up in upgrade70.html :)

Index: current.html
===
RCS file: /cvs/www/faq/current.html,v
retrieving revision 1.1075
diff -u -p -r1.1075 current.html
--- current.html11 Aug 2021 19:20:18 -  1.1075
+++ current.html2 Sep 2021 20:19:02 -
@@ -107,6 +107,15 @@ The default authentication has changed t
 
 
 
+2021/09/02 - garbage-collect _rebound user and group
+The rebound daemon was removed in OpenBSD 6.7, however its user and
+group _rebound were not deleted at the time. If you have kept
+upgrading your system from that time and never deleted the user and
+group, delete them with
+  
+  # userdel _rebound
+  # groupdel _rebound
+
 

Re: Removal of old users and groups in the upgrade notes

2021-09-02 Thread Sebastian Benoit
Florian Obser(flor...@openbsd.org) on 2021.09.02 14:04:22 +0200:
> On 2021-09-02 12:26 +02, Sebastian Benoit  wrote:
> > Raf Czlonka(rczlo...@gmail.com) on 2021.09.02 10:51:19 +0100:
> >> Ping.
> >> 
> >> On Mon, May 24, 2021 at 05:06:08PM BST, Raf Czlonka wrote:
> >> > Ping.
> >> > 
> >> > On Sun, May 09, 2021 at 01:07:15PM BST, Raf Czlonka wrote:
> >> > > Hello,
> >> > > 
> >> > > This is both a general question and specific example of removal of
> >> > > old users and groups.
> >> > > 
> >> > > With the release of 6.7, rebound(8) got tedued[0] ;^)
> >> > > However, there were no specific instructions regarding removal of
> >> > > _rebound user and group, or the /etc/rebound.conf file, in the
> >> > > upgrade notes - I had the latter added to my /etc/sysclean.ignore
> >> > > file and didn't notice it until today.
> >> > > 
> >> > > Grepping for '{user,group}del' under 'faq/upgrade*', shows a handful
> >> > > of examples - some are straight after a user or a group has been
> >> > > retired[1], others when the UID/GID got recycled[2].
> >> > > 
> >> > > Probably too little too late but, should the 6.7 upgrade page get:
> >> > > 
> >> > ># rm /etc/rebound.conf
> >> > ># userdel _rebound
> >> > ># groupdel _rebound
> >> > > 
> >> > > instructions added, i.e. need I bother you with a diff, or will you
> >> > > simply add it once the UID/GID gets recycled?
> >
> > You cannot change the 6.7 upgrade notes now and expect anyone to apply that.
> >
> > That is, the remove instructions will need to be given when the UID/GID is
> > recycled. That is completly fine, we have done it like this in the past.
> >
> 
> Not even that, experience with iirc slaacd showed that sysmerge is
> complaining loudly when a uid is recycled.

Well, yes. But that cant be a reason not to garbage collect and recycle
eventually. Just means it needs more thought.



Re: Removal of old users and groups in the upgrade notes

2021-09-02 Thread Sebastian Benoit
Raf Czlonka(rczlo...@gmail.com) on 2021.09.02 10:51:19 +0100:
> Ping.
> 
> On Mon, May 24, 2021 at 05:06:08PM BST, Raf Czlonka wrote:
> > Ping.
> > 
> > On Sun, May 09, 2021 at 01:07:15PM BST, Raf Czlonka wrote:
> > > Hello,
> > > 
> > > This is both a general question and specific example of removal of
> > > old users and groups.
> > > 
> > > With the release of 6.7, rebound(8) got tedued[0] ;^)
> > > However, there were no specific instructions regarding removal of
> > > _rebound user and group, or the /etc/rebound.conf file, in the
> > > upgrade notes - I had the latter added to my /etc/sysclean.ignore
> > > file and didn't notice it until today.
> > > 
> > > Grepping for '{user,group}del' under 'faq/upgrade*', shows a handful
> > > of examples - some are straight after a user or a group has been
> > > retired[1], others when the UID/GID got recycled[2].
> > > 
> > > Probably too little too late but, should the 6.7 upgrade page get:
> > > 
> > >   # rm /etc/rebound.conf
> > >   # userdel _rebound
> > >   # groupdel _rebound
> > > 
> > > instructions added, i.e. need I bother you with a diff, or will you
> > > simply add it once the UID/GID gets recycled?

You cannot change the 6.7 upgrade notes now and expect anyone to apply that.

That is, the remove instructions will need to be given when the UID/GID is
recycled. That is completly fine, we have done it like this in the past.



Re: timeout: Prettify man page and usage

2021-09-02 Thread Sebastian Benoit


ok

Martijn van Duren(openbsd+t...@list.imperialat.at) on 2021.09.02 11:05:24 +0200:
> On Thu, 2021-09-02 at 08:56 +, Job Snijders wrote:
> > On Thu, Sep 02, 2021 at 07:23:26AM +0100, Jason McIntyre wrote:
> > > >  .Ar time
> > > > -can be integer or decimal numbers.
> > > > +are positive integer or real (decimal) numbers, with an optional
> > > 
> > > can you have a negative timeout?
> > 
> > Negative values are not permitted
> > 
> > $ timeout -- -1 /bin/ls
> > timeout: invalid duration: Undefined error: 0
> > 
> > Kind regards,
> > 
> > Job
> > 
> There are a few cases where we don't set errno, but do use err(3).
> 
> Index: timeout.c
> ===
> RCS file: /cvs/src/usr.bin/timeout/timeout.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 timeout.c
> --- timeout.c 2 Sep 2021 06:23:32 -   1.13
> +++ timeout.c 2 Sep 2021 09:05:08 -
> @@ -68,15 +68,15 @@ parse_duration(const char *duration)
>  
>   ret = strtod(duration, );
>   if (ret == 0 && suffix == duration)
> - err(1, "invalid duration");
> + errx(1, "invalid duration");
>   if (ret < 0 || ret >= 1UL)
> - err(1, "invalid duration");
> + errx(1, "invalid duration");
>  
>   if (suffix == NULL || *suffix == '\0')
>   return (ret);
>  
>   if (suffix != NULL && *(suffix + 1) != '\0')
> - err(1, "invalid duration");
> + errx(1, "invalid duration");
>  
>   switch (*suffix) {
>   case 's':
> @@ -91,7 +91,7 @@ parse_duration(const char *duration)
>   ret *= 60 * 60 * 24;
>   break;
>   default:
> - err(1, "invalid duration");
> + errx(1, "invalid duration");
>   }
>  
>   return (ret);
> @@ -111,12 +111,12 @@ parse_signal(const char *str)
>   if (strcasecmp(str, sys_signame[i]) == 0)
>   return (i);
>   }
> - err(1, "invalid signal name");
> + errx(1, "invalid signal name");
>   }
>  
>   sig = strtonum(str, 1, NSIG, );
>   if (errstr != NULL)
> - err(1, "signal %s %s", str, errstr);
> + errx(1, "signal %s %s", str, errstr);
>  
>   return (int)sig;
>  }
> 
> 



Re: rpki-client exclude files from rsync fetch

2021-08-31 Thread Sebastian Benoit
Theo de Raadt(dera...@openbsd.org) on 2021.08.31 11:09:22 -0600:
> I don't understand -- why would people edit this file?
> 
> If this list is in argv, it will be difficult to identify targets using
> ps, because the hostname is way at the end.

Yes.

If we worry about people touching it, rpki-client could write it out to a
tmp file just before running rsync. But i think that can be done when
someone actually shot themself in the foot.

ok for the diff.
 
> Job Snijders  wrote:
> 
> > Hi,
> > 
> > I don't think this should be user configurable.
> > 
> > If folks remove entries like "+ *.crl" it breaks things.
> > If folks add entries like "+ *.mp3" it wastes network bandwidth. :-)
> > 
> > Let's use "--include" and "--exclude" instead.
> > 
> > kind regards,
> > 
> > Job
> > 
> > On Tue, Aug 31, 2021 at 02:23:57PM +0200, Claudio Jeker wrote:
> > > RPKI repository can only include a few specific files, everything else is
> > > just ignored and deleted after every fetch.  Since openrsync supports
> > > --exclude-file now we can use this to limit what is actually accepted by
> > > the client.
> > > 
> > > I used a config file in /etc/rpki instead of using multiple --exclude /
> > > --include arguments. Mostly to keep the execvp argv short.
> > > 
> > > What you think?
> > > -- 
> > > :wq Claudio
> > > 
> > > Index: etc/Makefile
> > > ===
> > > RCS file: /cvs/src/etc/Makefile,v
> > > retrieving revision 1.484
> > > diff -u -p -r1.484 Makefile
> > > --- etc/Makefile  1 May 2021 16:11:07 -   1.484
> > > +++ etc/Makefile  31 Aug 2021 12:17:40 -
> > > @@ -156,7 +156,7 @@ distribution-etc-root-var: distrib-dirs
> > >   ${DESTDIR}/etc/ppp
> > >   cd rpki; \
> > >   ${INSTALL} -c -o root -g wheel -m 644 \
> > > - afrinic.tal apnic.tal lacnic.tal ripe.tal \
> > > + afrinic.tal apnic.tal lacnic.tal ripe.tal rsync.filter \
> > >   ${DESTDIR}/etc/rpki
> > >   cd examples; \
> > >   ${INSTALL} -c -o root -g wheel -m 644 ${EXAMPLES} \
> > > Index: etc/rpki/rsync.filter
> > > ===
> > > RCS file: etc/rpki/rsync.filter
> > > diff -N etc/rpki/rsync.filter
> > > --- /dev/null 1 Jan 1970 00:00:00 -
> > > +++ etc/rpki/rsync.filter 31 Aug 2021 12:09:24 -
> > > @@ -0,0 +1,7 @@
> > > ++ */
> > > ++ *.cer
> > > ++ *.crl
> > > ++ *.gbr
> > > ++ *.mft
> > > ++ *.roa
> > > +- *
> > > Index: usr.sbin/rpki-client/rsync.c
> > > ===
> > > RCS file: /cvs/src/usr.sbin/rpki-client/rsync.c,v
> > > retrieving revision 1.24
> > > diff -u -p -r1.24 rsync.c
> > > --- usr.sbin/rpki-client/rsync.c  19 Apr 2021 17:04:35 -  1.24
> > > +++ usr.sbin/rpki-client/rsync.c  31 Aug 2021 12:17:11 -
> > > @@ -279,6 +279,8 @@ proc_rsync(char *prog, char *bind_addr, 
> > >   args[i++] = "--no-motd";
> > >   args[i++] = "--timeout";
> > >   args[i++] = "180";
> > > + args[i++] = "--exclude-from";
> > > + args[i++] = "/etc/rpki/rsync.filter";
> > >   if (bind_addr != NULL) {
> > >   args[i++] = "--address";
> > >   args[i++] = (char *)bind_addr;
> > > 
> > 
> 



Re: relayd(8): agentx allow re-enabling

2021-08-30 Thread Sebastian Benoit
Martijn van Duren(openbsd+t...@list.imperialat.at) on 2021.08.30 12:50:23 +0200:
> Via "relayctl reload" agentx can be enabled, disabled, but if it's
> enabled->disabled->enabled the final enable won't work because we
> never reset the sa.
> 
> Also add an extra guard so that we don't accidentally free it
> twice.

maybe thats not needed, agentx_free() just returns if the argument is NULL.
 
> OK?

ok

> 
> martijn@
> 
> Index: agentx_control.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/agentx_control.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 agentx_control.c
> --- agentx_control.c  27 Oct 2020 18:48:07 -  1.4
> +++ agentx_control.c  30 Aug 2021 10:49:49 -
> @@ -124,7 +124,7 @@ static struct snmp_oidhosttrapoid = {
>  #define RELAYDTABLENAME  RELAYDTABLEENTRY, 2
>  #define RELAYDTABLESTATUSRELAYDTABLEENTRY, 3
>  
> -void agentx_needsock(struct agentx *, void *, int);
> +void agentx_nofd(struct agentx *, void *, int);
>  
>  struct relayd *env;
>  
> @@ -202,6 +202,7 @@ agentx_init(struct relayd *nenv)
>   struct agentx_context *sac;
>   struct agentx_region *sar;
>   struct agentx_index *session_idxs[2];
> + static int freed;
>  
>   agentx_log_fatal = fatalx;
>   agentx_log_warn = log_warnx;
> @@ -211,14 +212,17 @@ agentx_init(struct relayd *nenv)
>   env = nenv;
>  
>   if ((env->sc_conf.flags & F_AGENTX) == 0) {
> - if (sa != NULL)
> + if (sa != NULL && !freed) {
>   agentx_free(sa);
> + freed = 1;
> + }
>   return;
>   }
>   if (sa != NULL)
>   return;
>  
> - if ((sa = agentx(agentx_needsock, NULL)) == NULL)
> + freed = 0;
> + if ((sa = agentx(agentx_nofd, NULL)) == NULL)
>   fatal("%s: agentx alloc", __func__);
>   if ((sas = agentx_session(sa, NULL, 0, "relayd", 0)) == NULL)
>   fatal("%s: agentx session alloc", __func__);
> @@ -420,9 +424,15 @@ agentx_init(struct relayd *nenv)
>  }
>  
>  void
> -agentx_needsock(struct agentx *usa, void *cookie, int fd)
> +agentx_nofd(struct agentx *usa, void *cookie, int close)
>  {
> - proc_compose(env->sc_ps, PROC_PARENT, IMSG_AGENTXSOCK, NULL, 0);
> + if (!close)
> + proc_compose(env->sc_ps, PROC_PARENT, IMSG_AGENTXSOCK, NULL, 0);
> + else {
> + sa = NULL;
> + agentx_init(env);
> + event_del(&(env->sc_agentxev));
> + }
>  }
>  
>  void
> 
> 



Re: wg(4) ipv6 ospf6d

2021-08-25 Thread Sebastian Benoit
Stefan Sperling(s...@stsp.name) on 2021.08.25 22:02:02 +0200:
> On Wed, Aug 25, 2021 at 08:13:26PM +0200, Florian Obser wrote:
> > On 2021-08-25 18:02 +01, Stuart Henderson  wrote:
> > > Trying to announce a network on a wg(4) interface via ospf6d, just
> > > using passive to pick up the prefix, i.e.
> > >
> > > interface wg0 { passive }
> > >
> > > It's failing with "/etc/ospf6d.conf:10: unnumbered interface wg0".
> > >
> > > With -v I get 'interface with index 27 not found' (this is "normal"
> > > with ospf6d) and the routable address does show up e.g. "if_newaddr:
> > > ifindex 27, addr 2a03::xx:xx::/64" before giving the
> > > unnumbered interface error. There is normally no link-local address
> > > for wg.
> > >
> > > If I manually configure a link-local the interface is successfully
> > > added.
> > >
> > > Anyone have an idea what the behaviour should be here? For passive
> > > would it make sense to accept an interface without link-local?
> > >
> > 
> > RFC 4291 2.1:
> >All interfaces are required to have at least one Link-Local unicast
> >address.
>  
> If you're not using the interface to send or receive OSPF messages this
> should not matter. I doubt the RFC authors considered the possibility
> of an IPv6-capable interface that doesn't support link-local.

Thats because by definition it's not IPv6 capable :-P

In this case, it should be possible to distribute a route that points to the
wg peer using

 redistribute _prefix_ depend on wg0

instead of using passive.

If that does not work i would like to know why.



Re: acme-client(1): Fix misleading comment

2021-08-24 Thread Sebastian Benoit
commited, thanks

Emil Engler(m...@emilengler.com) on 2021.08.24 08:52:57 +0200:
> While auditing acme-client(1) I have noticed that the source code still
> makes references to curl.
> 
> Apparently acme-client(1) used curl for HTTP transfers up until this
> commit:
> https://github.com/kristapsdz/acme-client/commit/d9d2382d5ebfa9dc6c3c086c1acf0e905d389fbc
> 
> The following diff should solve it:
> Index: usr.sbin/acme-client/netproc.c
> ===
> RCS file: /cvs/src/usr.sbin/acme-client/netproc.c,v
> retrieving revision 1.30
> diff -u -p -u -p -r1.30 netproc.c
> --- usr.sbin/acme-client/netproc.c  12 Jul 2021 15:09:20 -  1.30
> +++ usr.sbin/acme-client/netproc.c  24 Aug 2021 06:47:42 -
> @@ -33,7 +33,7 @@
>  #define RETRY_MAX 10
>  
>  /*
> - * Buffer used when collecting the results of a CURL transfer.
> + * Buffer used when collecting the results of an http transfer.
>   */
>  struct buf {
> char*buf; /* binary buffer */
> @@ -41,7 +41,7 @@ structbuf {
>  };
>  
>  /*
> - * Used for CURL communications.
> + * Used for communication with other processes.
>   */
>  struct conn {
> const char*newnonce; /* nonce authority */
> 



Re: handle RTM_IFANNOUNCE in dhcpleased & slaacd

2021-08-24 Thread Sebastian Benoit
Florian Obser(flor...@openbsd.org) on 2021.08.23 20:30:07 +0200:
> So I was playing with a usb network adapter and noticed that dhcpleased
> and slaacd would hold on to them when I unplugged them.

don't do that :P

> They would be listed as "unknown" because we can't find the if_name for
> the if_index anymore.
> 
> Turns out we are not getting a RTM_IFINFO when an interface disappears
> but instead we have to handle the RTM_IFANNOUNCE message.
> While here fix a bug in slaacd where it would remove the interface only
> in the engine process but not in the frontend when we lose an interface
> while handing RTM_IFINFO.
> 
> OK?

ok benno@

> 
> 
> diff --git sbin/dhcpleased/dhcpleased.c sbin/dhcpleased/dhcpleased.c
> index 5ff5dcc9480..55c51d53b18 100644
> --- sbin/dhcpleased/dhcpleased.c
> +++ sbin/dhcpleased/dhcpleased.c
> @@ -294,7 +294,8 @@ main(int argc, char *argv[])
>   AF_INET)) == -1)
>   fatal("route socket");
>  
> - rtfilter = ROUTE_FILTER(RTM_IFINFO) | ROUTE_FILTER(RTM_PROPOSAL);
> + rtfilter = ROUTE_FILTER(RTM_IFINFO) | ROUTE_FILTER(RTM_PROPOSAL) |
> + ROUTE_FILTER(RTM_IFANNOUNCE);
>   if (setsockopt(frontend_routesock, AF_ROUTE, ROUTE_MSGFILTER,
>   , sizeof(rtfilter)) == -1)
>   fatal("setsockopt(ROUTE_MSGFILTER)");
> diff --git sbin/dhcpleased/frontend.c sbin/dhcpleased/frontend.c
> index 84ae1cda9a4..44a217cb51b 100644
> --- sbin/dhcpleased/frontend.c
> +++ sbin/dhcpleased/frontend.c
> @@ -768,7 +768,10 @@ route_receive(int fd, short events, void *arg)
>  void
>  handle_route_message(struct rt_msghdr *rtm, struct sockaddr **rti_info)
>  {
> - struct sockaddr_dl  *sdl = NULL;
> + struct sockaddr_dl  *sdl = NULL;
> + struct if_announcemsghdr*ifan;
> + uint32_t if_index;
> +
>   switch (rtm->rtm_type) {
>   case RTM_IFINFO:
>   if (rtm->rtm_addrs & RTA_IFP && rti_info[RTAX_IFP]->sa_family
> @@ -776,6 +779,15 @@ handle_route_message(struct rt_msghdr *rtm, struct 
> sockaddr **rti_info)
>   sdl = (struct sockaddr_dl *)rti_info[RTAX_IFP];
>   update_iface((struct if_msghdr *)rtm, sdl);
>   break;
> + case RTM_IFANNOUNCE:
> + ifan = (struct if_announcemsghdr *)rtm;
> + if_index = ifan->ifan_index;
> +if (ifan->ifan_what == IFAN_DEPARTURE) {
> + frontend_imsg_compose_engine(IMSG_REMOVE_IF, 0, 0,
> + _index, sizeof(if_index));
> + remove_iface(if_index);
> + }
> + break;
>   case RTM_PROPOSAL:
>   if (rtm->rtm_priority == RTP_PROPOSAL_SOLICIT) {
>   log_debug("RTP_PROPOSAL_SOLICIT");
> @@ -784,7 +796,6 @@ handle_route_message(struct rt_msghdr *rtm, struct 
> sockaddr **rti_info)
>   }
>  #ifndef SMALL
>   else if (rtm->rtm_flags & RTF_PROTO3) {
> - uint32_t if_index;
>   char ifnamebuf[IF_NAMESIZE], *if_name;
>  
>   if_index = rtm->rtm_index;
> diff --git sbin/slaacd/frontend.c sbin/slaacd/frontend.c
> index d3cb23a2925..27fbd212a66 100644
> --- sbin/slaacd/frontend.c
> +++ sbin/slaacd/frontend.c
> @@ -778,6 +778,7 @@ void
>  handle_route_message(struct rt_msghdr *rtm, struct sockaddr **rti_info)
>  {
>   struct if_msghdr*ifm;
> + struct if_announcemsghdr*ifan;
>   struct imsg_del_addr del_addr;
>   struct imsg_del_routedel_route;
>   struct imsg_dup_addr dup_addr;
> @@ -798,6 +799,7 @@ handle_route_message(struct rt_msghdr *rtm, struct 
> sockaddr **rti_info)
>   if_index = ifm->ifm_index;
>   frontend_imsg_compose_engine(IMSG_REMOVE_IF, 0, 0,
>   _index, sizeof(if_index));
> + remove_iface(if_index);
>   } else {
>   xflags = get_xflags(if_name);
>   if (xflags == -1 || !(xflags & (IFXF_AUTOCONF6 |
> @@ -817,6 +819,15 @@ handle_route_message(struct rt_msghdr *rtm, struct 
> sockaddr **rti_info)
>   }
>   }
>   break;
> + case RTM_IFANNOUNCE:
> + ifan = (struct if_announcemsghdr *)rtm;
> + if_index = ifan->ifan_index;
> +if (ifan->ifan_what == IFAN_DEPARTURE) {
> + frontend_imsg_compose_engine(IMSG_REMOVE_IF, 0, 0,
> + _index, sizeof(if_index));
> + remove_iface(if_index);
> + }
> + break;
>   case RTM_NEWADDR:
>   ifm = (struct if_msghdr *)rtm;
>   if_name = if_indextoname(ifm->ifm_index, ifnamebuf);
> diff --git sbin/slaacd/slaacd.c sbin/slaacd/slaacd.c
> index 0b31934c211..ca5e942af2f 100644
> --- sbin/slaacd/slaacd.c
> +++ 

OpenBSD Errata: August 20, 2021 (libressl)

2021-08-20 Thread Sebastian Benoit
An errata patch for LibreSSL has been released for OpenBSD 6.8 and
OpenBSD 6.9.

Printing a certificate can result in a crash in X509_CERT_AUX_print().

Binary updates for the amd64, i386 and arm64 platform are available
via the syspatch utility.  Source code patches can be found on the
respective errata page:

  https://www.openbsd.org/errata68.html
  https://www.openbsd.org/errata69.html



Re: cal(1): Clean up mutually exclusive options

2021-08-16 Thread Sebastian Benoit
Jason McIntyre(j...@kerhand.co.uk) on 2021.08.16 12:02:13 +0100:
> when i wrote my mail, i failed to understand that "overrides earlier"
> was really just another way of saying "mutually exclusive". i don;t find
> it as clear, and i don;t hugely like it, but i guess it's just my
> preference.

Not really. I'm not a native speaker, but i would understand "mutually
exclusive" to mean "program probably terminates with an error", whereas
"overrides earlier" is "program uses later option and continues to do
whatever its suposed to".



OpenBSD Errata: August 11, 2021 (perl)

2021-08-10 Thread Sebastian Benoit
An errata patch for perl has been released for OpenBSD 6.9.

perl(1) Encode (3p) loads a module from an incorrect relative path.

Binary updates for the amd64, i386 and arm64 platform are available
via the syspatch utility.  Source code patches can be found on the
respective errata page:

  https://www.openbsd.org/errata69.html



OpenBSD Errata: August 11, 2021 (kernel)

2021-08-10 Thread Sebastian Benoit
An errata patch for the kernel has been released for OpenBSD 6.8 and
OpenBSD 6.9.

In a specific configuration, wg(4) leaked mbufs.

Binary updates for the amd64, i386 and arm64 platform are available
via the syspatch utility.  Source code patches can be found on the
respective errata page:

  https://www.openbsd.org/errata68.html
  https://www.openbsd.org/errata69.html



Re: bgpd add add-path receive support

2021-08-06 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.08.04 17:55:45 +0200:
> On Fri, Jul 30, 2021 at 12:02:12PM +0200, Claudio Jeker wrote:
> > This diff implements the bit to support the receive side of
> > RFC7911 - Advertisement of Multiple Paths in BGP.
> > 
> > I did some basic tests and it works for me. People running route
> > collectors should give this a try. The interaction of Add-Path and bgpctl
> > probably needs some work. Also the MRT dumper needs to be updated to
> > support RFC8050. I have a partial diff for that ready as well.
> > 
> > Sending out multiple paths will follow in a later step since that is a
> > bit more complex. I still need to decide how stable I want to make the
> > assigned path_ids for the multiple paths and then changes to the decision
> > process and adjrib-out are required to allow multipe paths there.
> 
> Updated diff that includes some minimal support for bgpctl.
> This add 'bgpctl show rib nei foo path-id 42' as a way to limit which
> paths to show. Now the RFC itself is very flexible in how path-ids are
> assigned (it is possible that different prefixes have different path-ids)
> but the assumtion is that most systems assign path-id in a stable way and
> so it makes sense to allow to filter on path-id.
> Apart from that not much changed.

ok benno@

Only one thing, I worry that using this while the sending side is not working 
can lead to
blackholing of prefixes.

> 
> -- 
> :wq Claudio
> 
> Index: bgpctl/bgpctl.8
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.8,v
> retrieving revision 1.99
> diff -u -p -r1.99 bgpctl.8
> --- bgpctl/bgpctl.8   16 Jun 2021 16:24:12 -  1.99
> +++ bgpctl/bgpctl.8   4 Aug 2021 13:15:53 -
> @@ -348,6 +348,13 @@ Show RIB memory statistics.
>  Show only entries from the specified peer.
>  .It Cm neighbor group Ar description
>  Show only entries from the specified peer group.
> +.It Cm path-id Ar pathid
> +Show only entries which match the specified
> +.Ar pathid .
> +Must be used together with either
> +.Cm neighbor
> +or
> +.Cm out .
>  .It Cm peer-as Ar as
>  Show all entries with
>  .Ar as
> Index: bgpctl/bgpctl.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
> retrieving revision 1.272
> diff -u -p -r1.272 bgpctl.c
> --- bgpctl/bgpctl.c   2 Aug 2021 16:51:39 -   1.272
> +++ bgpctl/bgpctl.c   4 Aug 2021 15:54:25 -
> @@ -249,6 +249,7 @@ main(int argc, char *argv[])
>   ribreq.neighbor = neighbor;
>   strlcpy(ribreq.rib, res->rib, sizeof(ribreq.rib));
>   ribreq.aid = res->aid;
> + ribreq.path_id = res->pathid;
>   ribreq.flags = res->flags;
>   imsg_compose(ibuf, type, 0, 0, -1, , sizeof(ribreq));
>   break;
> Index: bgpctl/parser.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/parser.c,v
> retrieving revision 1.106
> diff -u -p -r1.106 parser.c
> --- bgpctl/parser.c   16 Feb 2021 08:30:21 -  1.106
> +++ bgpctl/parser.c   4 Aug 2021 13:08:31 -
> @@ -61,7 +61,8 @@ enum token_type {
>   RD,
>   FAMILY,
>   RTABLE,
> - FILENAME
> + FILENAME,
> + PATHID,
>  };
>  
>  struct token {
> @@ -114,6 +115,7 @@ static const struct token t_log[];
>  static const struct token t_fib_table[];
>  static const struct token t_show_fib_table[];
>  static const struct token t_communication[];
> +static const struct token t_show_rib_path[];
>  
>  static const struct token t_main[] = {
>   { KEYWORD,  "reload",   RELOAD, t_communication},
> @@ -178,10 +180,11 @@ static const struct token t_show_rib[] =
>   { FLAG, "in",   F_CTL_ADJ_IN,   t_show_rib},
>   { FLAG, "out",  F_CTL_ADJ_OUT,  t_show_rib},
>   { KEYWORD,  "neighbor", NONE,   t_show_rib_neigh},
> + { KEYWORD,  "ovs",  NONE,   t_show_ovs},
> + { KEYWORD,  "path-id",  NONE,   t_show_rib_path},
>   { KEYWORD,  "table",NONE,   t_show_rib_rib},
>   { KEYWORD,  "summary",  SHOW_SUMMARY,   t_show_summary},
>   { KEYWORD,  "memory",   SHOW_RIB_MEM,   NULL},
> - { KEYWORD,  "ovs",  NONE,   t_show_ovs},
>   { FAMILY,   "", NONE,   t_show_rib},
>   { PREFIX,   "", NONE,   t_show_prefix},
>   { ENDTOKEN, "", NONE,   NULL}
> @@ -479,6 +482,11 @@ static const struct token t_show_fib_tab
>   { ENDTOKEN, "", NONE,   NULL}
>  };
>  
> +static const struct token t_show_rib_path[] = {
> + { PATHID,   "", NONE,   t_show_rib},
> + { ENDTOKEN, "", NONE,   NULL}
> +};
> +
>  static struct parse_result   res;
>  
>  const struct token   

OpenBSD Errata: August 4, 2021 (kernel, sparc64)

2021-08-04 Thread Sebastian Benoit
An errata patch for the kernel on the sparc64 architecture has been
released for OpenBSD 6.8 and OpenBSD 6.9.

On sparc64, a missaligned address could trigger a kernel assert and
panic the kernel.

Source code patches can be found on the respective errata pages:

  https://www.openbsd.org/errata68.html
  https://www.openbsd.org/errata69.html



Re: rpki-client support more http status codes

2021-08-04 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.08.04 17:45:14 +0200:
> On Wed, Aug 04, 2021 at 10:53:39AM +0200, Claudio Jeker wrote:
> > This adds a few more HTTP Status codes to the mix of the accepted ones.
> > Mainly 100, 103 and 203 are now also accepted. All other codes in the 1xx
> > and 2xx are still considered an error since they are not expected from the
> > GET request made by the http client. This is a minimal HTTP client and it
> > should remain minimal. If a server is sending back something unexpected
> > just fail and fall back to rsync.
> 
> 
> Update with additional comments for the various status codes.

just as i had looked them up :P

ok benno@

> -- 
> :wq Claudio
> 
> Index: http.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/http.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 http.c
> --- http.c23 Jul 2021 16:03:47 -  1.34
> +++ http.c4 Aug 2021 15:22:22 -
> @@ -865,7 +865,9 @@ http_request(struct http_connection *con
>  
>  /*
>   * Parse the HTTP status line.
> - * Return 0 for status codes 200, 301-304, 307-308.
> + * Return 0 for status codes 100, 103, 200, 203, 301-304, 307-308.
> + * The other 1xx and 2xx status codes are explicitly not handled and are
> + * considered an error.
>   * Failure codes and other errors return -1.
>   * The redirect loop limit is enforced here.
>   */
> @@ -885,7 +887,7 @@ http_parse_status(struct http_connection
>   cp++;
>  
>   strlcpy(ststr, cp, sizeof(ststr));
> - status = strtonum(ststr, 200, 599, );
> + status = strtonum(ststr, 100, 599, );
>   if (errstr != NULL) {
>   strnvis(gerror, cp, sizeof gerror, VIS_SAFE);
>   warnx("Error retrieving %s: %s", http_info(conn->host),
> @@ -894,19 +896,23 @@ http_parse_status(struct http_connection
>   }
>  
>   switch (status) {
> - case 301:
> - case 302:
> - case 303:
> - case 307:
> - case 308:
> + case 301:   /* Redirect: moved permanently */
> + case 302:   /* Redirect: found / moved temporarily */
> + case 303:   /* Redirect: see other */
> + case 307:   /* Redirect: temporary redirect */
> + case 308:   /* Redirect: permanent redirect */
>   if (conn->req->redirect_loop++ > 10) {
>   warnx("%s: Too many redirections requested",
>   http_info(conn->host));
>   return -1;
>   }
>   /* FALLTHROUGH */
> - case 200:
> - case 304:
> + case 100:   /* Informational: continue (ignored) */
> + case 103:   /* Informational: early hints (ignored) */
> + /* FALLTHROUGH */
> + case 200:   /* Success: OK */
> + case 203:   /* Success: non-authoritative information (proxy) */
> + case 304:   /* Redirect: not modified */
>   conn->status = status;
>   break;
>   default:
> @@ -931,6 +937,14 @@ http_isredirect(struct http_connection *
>   return 0;
>  }
>  
> +static inline int
> +http_isok(struct http_connection *conn)
> +{
> + if (conn->status >= 200 && conn->status < 300)
> + return 1;
> + return 0;
> +}
> +
>  static void
>  http_redirect(struct http_connection *conn)
>  {
> @@ -1165,7 +1179,7 @@ again:
>   }
>  
>   /* Check status header and decide what to do next */
> - if (conn->status == 200 || http_isredirect(conn)) {
> + if (http_isok(conn) || http_isredirect(conn)) {
>   if (http_isredirect(conn))
>   http_redirect(conn);
>  
> @@ -1174,6 +1188,8 @@ again:
>   else
>   conn->state = STATE_RESPONSE_DATA;
>   goto again;
> + } else if (conn->status == 100 || conn->status == 103) {
> + conn->state = STATE_RESPONSE_STATUS;
>   } else if (conn->status == 304) {
>   return http_done(conn, HTTP_NOT_MOD);
>   }
> 



Re: Rationale behind exec clearing out unveil paths

2021-08-03 Thread Sebastian Benoit
dz...@disroot.org(dz...@disroot.org) on 2021.06.15 14:12:22 +:
> > Seems to be working as intended. You are letting someone run all binaries.
> And I am not letting someone write to the filesystem. Yet, they can
> bypass that easily. `unveil("/", "rx")` gives a false illusion of
> security, which can even trip up OpenBSD maintainers (more below).
> 
> > Or is it your expectation is that all binaries should crash when they
> > cannot start ld.so or load libc?
> "/" is mounted for reads, why would a program crash while loading
> libc? You don't need write access to execute a program.
> 
> > I'd say the problem is whoever wrote this code unrealistic 2-liner code
> > example, oh wait, that is you.
>   (and)
> > The expected uses of unveil and pledge aren't some weird fiction
> > of "oh look I can use them wrong".
> https://github.com/openbsd/src/commit/15e2c6823410e554b348cd3fb137566da656e866
> 
> 
> Also to be clear - I'm not throwing blame to the author of the commit
> here, it's not their fault. This behaviour isn't documented, so unless
> you have seen the exec() source, you wouldn't know about it.

If anything, that example shows that relayd needs to be redesigned to make
good use of pledge() and unveil(). Because of the capability to reload the
configuration (making it possible to change the patch to external check
programms), with the current structure of relayd it is impossible to do this
better.

The way relayd runs external check scripts need to be changed such
that a tighter unveil() becomes possible.

Maybe you can come up with a patch?



rpki-client 7.2 released

2021-07-28 Thread Sebastian Benoit
rpki-client 7.2 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, Theo Buehler, Theo de Raadt and Sebastian Benoit
as part of the OpenBSD Project.

This release includes the following changes to the previous release:

 * Use RRDP as default protocol for syncronizing the RPKI repository
   data, with rsync used as secondary.
 * At startup, warn if the filesystem containing the cache directory
   is probably too small. 500 MB is the suggested minimum size.
 * Handle running out of disk space more gracefully, including cleanup
   of temporary and old files before exiting.
 * Improve the HTTP/1.1 request headers being sent.
 * Improved validation checks for ROA and MFT objects.

rpki-client is known to compile and run on at least the following
operating systems: Alpine 3.12, CentOS/RHEL/Rocky 7, 8, Debian 9 and
10, Fedora 32, 33 and 34, Ubuntu 20.04 LTS, FreeBSD 12 and 13, macOS,
and of course OpenBSD.

It is our hope that packagers take interest and help adapt
rpki-client-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.



tpmr manpage add veb reference

2021-07-27 Thread Sebastian Benoit
tpmr(4) connects only two ethernet ports with not much functionality, so the
manpage is helpful by telling us bridge(4) as a more complete alternative.
We now also have veb(4), so mention that as well.

ok?

diff --git share/man/man4/tpmr.4 share/man/man4/tpmr.4
index ab9eba4cee3..de0b200d429 100644
--- share/man/man4/tpmr.4
+++ share/man/man4/tpmr.4
@@ -43,7 +43,9 @@ configuration file for
 .Pp
 Other forms of Ethernet bridging are available using the
 .Xr bridge 4
-driver.
+Ethernet bridge driver and the
+.Xr veb 4
+Virtual Ethernet Bridge device.
 Link aggregation of Ethernet interfaces can be achieved
 using the
 .Xr aggr 4
@@ -145,6 +147,7 @@ interfaces.
 .Xr bridge 4 ,
 .Xr pf 4 ,
 .Xr trunk 4 ,
+.Xr veb 4 ,
 .Xr hostname.if 5 ,
 .Xr ifconfig 8 ,
 .Xr netstart 8



Re: bgpd refactor struct prefix

2021-07-26 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.07.14 09:33:19 +0200:
> On Tue, Jun 29, 2021 at 12:00:24PM +0200, Claudio Jeker wrote:
> > This diff moves the rib_entry pointer re into the union to safe some
> > space. For add-path I need to add a few more u_int32_t and that would
> > blow the size of struct prefix from 128 to 132 bytes. malloc would round
> > that up to 256bytes and that is bad for the struct that is allocted in
> > millions in bgpd.
> > 
> > To make this somewhat save introduce PREFIX_FLAG_ADJOUT to mark prefixes
> > that live in the adj-rib-out. Those prefixes can not access the re pointer
> > also use a wrapper prefix_re() which returns the re pointer or NULL.
> > Also add some assertions to make sure that prefixes don't end up in the
> > wrong tree.
> > 
> > This change shrinks the struct back to 120bytes and gives me the space
> > needed for add-path.
> > 
> > Please test
> 

reads ok, and works for me.

I don't like all those fatals. We are bound to overlook something and hit.
But of course those are hard to work around.

> -- 
> :wq Claudio
> 
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.530
> diff -u -p -r1.530 rde.c
> --- rde.c 25 Jun 2021 09:25:48 -  1.530
> +++ rde.c 29 Jun 2021 08:28:33 -
> @@ -2298,6 +2298,7 @@ rde_dump_rib_as(struct prefix *p, struct
>   struct ibuf *wbuf;
>   struct attr *a;
>   struct nexthop  *nexthop;
> + struct rib_entry*re;
>   void*bp;
>   time_t   staletime;
>   size_t   aslen;
> @@ -2330,7 +2331,8 @@ rde_dump_rib_as(struct prefix *p, struct
>   rib.origin = asp->origin;
>   rib.validation_state = p->validation_state;
>   rib.flags = 0;
> - if (p->re != NULL && p->re->active == p)
> + re = prefix_re(p);
> + if (re != NULL && re->active == p)
>   rib.flags |= F_PREF_ACTIVE;
>   if (!prefix_peer(p)->conf.ebgp)
>   rib.flags |= F_PREF_INTERNAL;
> @@ -2412,14 +2414,16 @@ static void
>  rde_dump_filter(struct prefix *p, struct ctl_show_rib_request *req)
>  {
>   struct rde_aspath   *asp;
> + struct rib_entry*re;
>  
>   if (!rde_match_peer(prefix_peer(p), >neighbor))
>   return;
>  
>   asp = prefix_aspath(p);
> + re = prefix_re(p);
>   if (asp == NULL)/* skip pending withdraw in Adj-RIB-Out */
>   return;
> - if ((req->flags & F_CTL_ACTIVE) && p->re->active != p)
> + if ((req->flags & F_CTL_ACTIVE) && re != NULL && re->active != p)
>   return;
>   if ((req->flags & F_CTL_INVALID) &&
>   (asp->flags & F_ATTR_PARSE_ERR) == 0)
> Index: rde.h
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
> retrieving revision 1.240
> diff -u -p -r1.240 rde.h
> --- rde.h 17 Jun 2021 16:05:26 -  1.240
> +++ rde.h 29 Jun 2021 08:33:18 -v
> @@ -56,10 +56,10 @@ struct rib {
>   struct filter_head  *in_rules_tmp;
>   u_int   rtableid;
>   u_int   rtableid_tmp;
> + enum reconf_action  state, fibstate;
> + u_int16_t   id;
>   u_int16_t   flags;
>   u_int16_t   flags_tmp;
> - u_int16_t   id;
> - enum reconf_action  state, fibstate;
>  };
>  
>  #define RIB_ADJ_IN   0
> @@ -317,13 +317,13 @@ struct prefix {
>   union {
>   struct {
>   LIST_ENTRY(prefix)   rib, nexthop;
> + struct rib_entry*re;
>   } list;
>   struct {
>   RB_ENTRY(prefix) index, update;
>   } tree;
>   }entry;
>   struct pt_entry *pt;
> - struct rib_entry*re;
>   struct rde_aspath   *aspath;
>   struct rde_community*communities;
>   struct rde_peer *peer;
> @@ -338,6 +338,7 @@ struct prefix {
>  #define  PREFIX_FLAG_DEAD0x04/* locked but removed */
>  #define  PREFIX_FLAG_STALE   0x08/* stale entry (graceful 
> reload) */
>  #define  PREFIX_FLAG_MASK0x0f/* mask for the prefix types */
> +#define  PREFIX_FLAG_ADJOUT  0x10/* prefix is in the adj-out rib 
> */
>  #define  PREFIX_NEXTHOP_LINKED   0x40/* prefix is linked onto 
> nexthop list */
>  #define  PREFIX_FLAG_LOCKED  0x80/* locked by rib walker */
>  };
> @@ -637,6 +638,14 @@ static inline u_int8_t
>  prefix_vstate(struct prefix *p)
>  {
>   return (p->validation_state & ROA_MASK);
> +}
> +
> +static inline struct rib_entry *
> +prefix_re(struct prefix *p)
> +{
> + if (p->flags & PREFIX_FLAG_ADJOUT)
> + return 

Re: bgpctl add support for RFC8050 (add-path support for MRT parser)

2021-07-26 Thread Sebastian Benoit
ok

Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.07.13 15:37:36 +0200:
> This diff adds support to read MRT files using the new introduced _ADDPATH
> types as defined in RFC8050. I also started adding MRT support to bgpd but
> that depends on ADD-PATH itself.
> 
> There are a few gotchas, especially the MRT_DUMP_V2 RIB_GENERIC_ADDPATH
> handling is different from all other RIB entry handling. This is a major
> pain point for bgpd less so for the bgpctl parser.
> 
> Some MRT update dumps that can be downloaded and use ADDPATH do actually
> use the BGP4MP_MESSAGE _ADDPATH variant for non-addpath enabled sessions.
> The update messages can not be parsed because the NLRI encoding is incorrect.
> 
> I tested with a few RIB and UPDATE dumps from RIS, route-views and other
> open collectors and it works for me.
> -- 
> :wq Claudio
> 
> Index: usr.sbin/bgpctl/bgpctl.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
> retrieving revision 1.269
> diff -u -p -r1.269 bgpctl.c
> --- usr.sbin/bgpctl/bgpctl.c  16 Jun 2021 16:24:11 -  1.269
> +++ usr.sbin/bgpctl/bgpctl.c  13 Jul 2021 13:20:51 -
> @@ -470,7 +470,7 @@ show(struct imsg *imsg, struct parse_res
>   warnx("bad IMSG_CTL_SHOW_RIB_ATTR received");
>   break;
>   }
> - output->attr(imsg->data, ilen, res->flags);
> + output->attr(imsg->data, ilen, res->flags, 0);
>   break;
>   case IMSG_CTL_SHOW_RIB_MEM:
>   if (imsg->hdr.len < IMSG_HEADER_SIZE + sizeof(stats))
> @@ -1150,6 +1150,10 @@ show_mrt_dump(struct mrt_rib *mr, struct
>   ctl.local_pref = mre->local_pref;
>   ctl.med = mre->med;
>   /* weight is not part of the mrt dump so it can't be set */
> + if (mr->add_path) {
> + ctl.flags |= F_PREF_PATH_ID;
> + ctl.path_id = mre->path_id;
> + }
>  
>   if (mre->peer_idx < mp->npeers) {
>   ctl.remote_addr = mp->peers[mre->peer_idx].addr;
> @@ -1195,7 +1199,7 @@ show_mrt_dump(struct mrt_rib *mr, struct
>   if (req->flags & F_CTL_DETAIL) {
>   for (j = 0; j < mre->nattrs; j++)
>   output->attr(mre->attrs[j].attr,
> - mre->attrs[j].attr_len, req->flags);
> + mre->attrs[j].attr_len, req->flags, 0);
>   }
>   }
>  }
> @@ -1211,6 +1215,10 @@ network_mrt_dump(struct mrt_rib *mr, str
>   time_t   now;
>   u_int16_ti, j;
>  
> + /* can't announce more than one path so ignore add-path */
> + if (mr->add_path)
> + return;
> +
>   now = time(NULL);
>   for (i = 0; i < mr->nentries; i++) {
>   mre = >entries[i];
> @@ -1586,10 +1594,11 @@ show_mrt_notification(u_char *p, u_int16
>  
>  /* XXX this function does not handle JSON output */
>  static void
> -show_mrt_update(u_char *p, u_int16_t len, int reqflags)
> +show_mrt_update(u_char *p, u_int16_t len, int reqflags, int addpath)
>  {
>   struct bgpd_addr prefix;
>   int pos;
> + u_int32_t pathid;
>   u_int16_t wlen, alen;
>   u_int8_t prefixlen;
>  
> @@ -1609,12 +1618,25 @@ show_mrt_update(u_char *p, u_int16_t len
>   if (wlen > 0) {
>   printf("\n Withdrawn prefixes:");
>   while (wlen > 0) {
> + if (addpath) {
> + if (wlen <= sizeof(pathid)) {
> + printf("bad withdraw prefix");
> + return;
> + }
> + memcpy(, p, sizeof(pathid));
> + pathid = ntohl(pathid);
> + p += sizeof(pathid);
> + len -= sizeof(pathid);
> + wlen -= sizeof(pathid);
> + }
>   if ((pos = nlri_get_prefix(p, wlen, ,
>   )) == -1) {
>   printf("bad withdraw prefix");
>   return;
>   }
>   printf(" %s/%u", log_addr(), prefixlen);
> + if (addpath)
> + printf(" path-id %u", pathid);
>   p += pos;
>   len -= pos;
>   wlen -= pos;
> @@ -1655,7 +1677,7 @@ show_mrt_update(u_char *p, u_int16_t len
>   attrlen += 1 + 2;
>   }
>  
> - output->attr(p, attrlen, reqflags);
> + output->attr(p, attrlen, reqflags, addpath);
>   p += attrlen;
>   alen -= attrlen;
>   len -= attrlen;
> @@ -1664,12 +1686,24 @@ show_mrt_update(u_char *p, u_int16_t 

OpenBSD Errata: July 25, 2021 (libc, mips64)

2021-07-26 Thread Sebastian Benoit
An errata patch for the libc library on the mips64 architecture has  
been released for OpenBSD 6.8 and OpenBSD 6.9.

On mips64, the strchr/index/strrchr/rindex functions in libc handled
signed characters incorrectly.

Source code patches can be found on the respective errata pages:

  https://www.openbsd.org/errata68.html
  https://www.openbsd.org/errata69.html



OpenBSD Errata: July 25, 2021 (relayd)

2021-07-25 Thread Sebastian Benoit
An errata patch for the relayd application layer gateway daemon has
been released for OpenBSD 6.9.

relayd(8), when using the the http protocol strip filter directive or http
protocol macro expansion, processes format strings.

Binary updates for the amd64, i386, and arm64 platform are available
via the syspatch utility. Source code patches can be found on the
respective errata page:

  https://www.openbsd.org/errata69.html




Re: unwind(8): store enabled resolvers lookup table in config

2021-07-24 Thread Sebastian Benoit
Florian Obser(flor...@openbsd.org) on 2021.07.23 20:27:40 +0200:
> We store a list of resolver strategies in order of their preference in
> the configuration struct. This is also an implicit list of enabled
> resolver strategies. We have also stored an explict lookup array of
> enabled strategies outside of the configuration to be able to
> quickly answer "is this strategy enabled" without traversing the
> preferences list.
> 
> Move this table into the configuration so that we don't need to
> "repair" it on config reload.
> 
> This fixes a bug where on startup the preferences list and enabled
> lookup table were not in sync.
> 
> OK?

ok

> 
> diff --git parse.y parse.y
> index 5fb796655b8..65e8af2deff 100644
> --- parse.y
> +++ parse.y
> @@ -192,7 +192,11 @@ block_list   : BLOCK LIST STRING log {
>   }
>   ;
>  
> -uw_pref  : PREFERENCE { conf->res_pref.len = 0; } 
> pref_block
> +uw_pref  : PREFERENCE {
> + conf->res_pref.len = 0;
> + memset(conf->enabled_resolvers, 0,
> + sizeof(conf->enabled_resolvers));
> + } pref_block
>   ;
>  
>  pref_block   : '{' optnl prefopts_l '}'
> @@ -211,6 +215,7 @@ prefoptsl : prefopt {
>   YYERROR;
>   }
>   conf->res_pref.types[conf->res_pref.len++] = $1;
> + conf->enabled_resolvers[$1] = 1;
>   }
>   ;
>  
> diff --git resolver.c resolver.c
> index 81610f68b3e..06f09604f6e 100644
> --- resolver.c
> +++ resolver.c
> @@ -211,7 +211,6 @@ static struct imsgev  *iev_frontend;
>  static struct imsgev *iev_main;
>  struct uw_forwarder_head  autoconf_forwarder_list;
>  struct uw_resolver   *resolvers[UW_RES_NONE];
> -int   enabled_resolvers[UW_RES_NONE];
>  struct timespec   last_network_change;
>  
>  struct event  trust_anchor_timer;
> @@ -672,10 +671,6 @@ resolver_dispatch_main(int fd, short event, void *bula)
>   "IMSG_RECONF_CONF", __func__);
>   restart = resolvers_to_restart(resolver_conf, nconf);
>   merge_config(resolver_conf, nconf);
> - memset(enabled_resolvers, 0, sizeof(enabled_resolvers));
> - for (i = 0; i < resolver_conf->res_pref.len; i++)
> - enabled_resolvers[
> - resolver_conf->res_pref.types[i]] = 1;
>   nconf = NULL;
>   for (i = 0; i < UW_RES_NONE; i++)
>   if (restart[i])
> @@ -1088,7 +1083,7 @@ new_resolver(enum uw_resolver_type type, enum 
> uw_resolver_state state)
>   free_resolver(resolvers[type]);
>   resolvers[type] = NULL;
>  
> - if (!enabled_resolvers[type])
> + if (!resolver_conf->enabled_resolvers[type])
>   return;
>  
>   switch (type) {
> @@ -2162,8 +2157,6 @@ int *
>  resolvers_to_restart(struct uw_conf *oconf, struct uw_conf *nconf)
>  {
>   static int   restart[UW_RES_NONE];
> - int  o_enabled[UW_RES_NONE];
> - int  n_enabled[UW_RES_NONE];
>   int  i;
>  
>   memset(, 0, sizeof(restart));
> @@ -2176,16 +2169,9 @@ resolvers_to_restart(struct uw_conf *oconf, struct 
> uw_conf *nconf)
>   >uw_dot_forwarder_list)) {
>   restart[UW_RES_DOT] = 1;
>   }
> - memset(o_enabled, 0, sizeof(o_enabled));
> - memset(n_enabled, 0, sizeof(n_enabled));
> - for (i = 0; i < oconf->res_pref.len; i++)
> - o_enabled[oconf->res_pref.types[i]] = 1;
> -
> - for (i = 0; i < nconf->res_pref.len; i++)
> - n_enabled[nconf->res_pref.types[i]] = 1;
>  
>   for (i = 0; i < UW_RES_NONE; i++) {
> - if (n_enabled[i] != o_enabled[i])
> + if (oconf->enabled_resolvers[i] != nconf->enabled_resolvers[i])
>   restart[i] = 1;
>   }
>   return restart;
> diff --git unwind.c unwind.c
> index 4697767ac34..9531cf18a08 100644
> --- unwind.c
> +++ unwind.c
> @@ -694,6 +694,7 @@ config_new_empty(void)
>   UW_RES_DHCP,
>   UW_RES_ASR};
>   struct uw_conf  *xconf;
> + int  i;
>  
>   xconf = calloc(1, sizeof(*xconf));
>   if (xconf == NULL)
> @@ -702,6 +703,8 @@ config_new_empty(void)
>   memcpy(>res_pref.types, _res_pref,
>   sizeof(default_res_pref));
>   xconf->res_pref.len = nitems(default_res_pref);
> + for (i = 0; i < xconf->res_pref.len; i++)
> + xconf->enabled_resolvers[xconf->res_pref.types[i]] = 1;
>  
>   TAILQ_INIT(>uw_forwarder_list);
>  

Re: unwind(8): don't doubt secure answers on network change

2021-07-24 Thread Sebastian Benoit
Florian Obser(flor...@narrans.de) on 2021.07.23 20:28:33 +0200:
> Do not doubt a secure (i.e. validated) NXDOMAIN response when we just
> switched networks. We just validated it!
> 
> While here reorder the long list of conditions to make it easier to
> understand when we doubt a response because we might be behind a
> captive portal. First list all conditions when we do not doubt the
> response and then the two conditions when we do doubt the response.
> 
> OK?

ok

> 
> diff --git resolver.c resolver.c
> index 06f09604f6e..7e18fc3449a 100644
> --- resolver.c
> +++ resolver.c
> @@ -988,9 +988,9 @@ resolve_done(struct uw_resolver *res, void *arg, int 
> rcode,
>   force_acceptbogus = 0;
>  
>   timespecsub(, _network_change, );
> - if ((result->rcode == LDNS_RCODE_NXDOMAIN || sec == BOGUS) &&
> - !force_acceptbogus && res->type != UW_RES_ASR && elapsed.tv_sec <
> - DOUBT_NXDOMAIN_SEC) {
> + if (sec != SECURE && elapsed.tv_sec < DOUBT_NXDOMAIN_SEC &&
> + !force_acceptbogus && res->type != UW_RES_ASR &&
> + (result->rcode == LDNS_RCODE_NXDOMAIN || sec == BOGUS)) {
>   /*
>* Doubt NXDOMAIN or BOGUS if we just switched networks, we
>* might be behind a captive portal.
> 
> -- 
> I'm not entirely sure you are real.
> 



Re: rpki-client: adjust HTTP/1.1 request string

2021-07-23 Thread Sebastian Benoit
Job Snijders(j...@openbsd.org) on 2021.07.23 15:23:49 +:
> Hi all,
> 
> Based on suggestions from Julian Reschke.
> 
> * "Connection: keep-alive" isn't needed, as the HTTP 1.1 default is to
>   use persistent connections (RFC 7230, section 6.3).
> 
> * "Host" is recommended to be in the front.
> 
> * "Accept-Encoding: identity" makes it clear to the server compression
>   encodings are not supported.
>   https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept-Encoding
> 
> OK?

reasonable.
ok benno@

> 
> Kind regards,
> 
> Job
> 
> Index: http.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/http.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 http.c
> --- http.c10 May 2021 15:12:33 -  1.33
> +++ http.c23 Jul 2021 15:13:35 -
> @@ -847,9 +847,10 @@ http_request(struct http_connection *con
>   conn->bufpos = 0;
>   if ((r = asprintf(>buf,
>   "GET /%s HTTP/1.1\r\n"
> - "Connection: keep-alive\r\n"
> + "Host: %s\r\n"
> + "Accept-Encoding: identity\r\n"
>   "User-Agent: " HTTP_USER_AGENT "\r\n"
> - "Host: %s\r\n%s\r\n",
> + "%s\r\n",
>   epath, host,
>   modified_since ? modified_since : "")) == -1)
>   err(1, NULL);
> 



Re: rsync getopt_long cleanup

2021-07-14 Thread Sebastian Benoit
ok benno@

much better as the list grows

Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.07.13 20:06:39 +0200:
> I never really liked the getopt_long definitions in rsync. Too much magic
> and chaos.
> 
> This moves the table out of main to gain some more space and to make it a
> proper read-only object. Because of this struct opts also needs to become
> a global but that is OK.
> 
> Clean up the required_argument options that have no short from. Instead of
> small numbers use some defines and make the values larger than any char
> value (I chose 1000 and up).
> 
> Fix --no-motd, it is just a flag setting a value. So just use the
> getopt_long() method for doing that.
> 
> Sort the options alphabetically with the exception of no-XYZ options which
> I added below the XYZ option itself.
> 
> IMO the result is better than what was there before.
> -- 
> :wq Claudio
> 
> Index: main.c
> ===
> RCS file: /cvs/src/usr.bin/rsync/main.c,v
> retrieving revision 1.55
> diff -u -p -r1.55 main.c
> --- main.c30 Jun 2021 13:10:04 -  1.55
> +++ main.c13 Jul 2021 17:54:13 -
> @@ -269,61 +269,67 @@ fargs_parse(size_t argc, char *argv[], s
>   return f;
>  }
>  
> +static struct optsopts;
> +
> +#define OP_ADDRESS   1000
> +#define OP_PORT  1001
> +#define OP_RSYNCPATH 1002
> +#define OP_TIMEOUT   1003
> +#define OP_VERSION   1004
> +
> +const struct option   lopts[] = {
> +{ "address", required_argument, NULL,OP_ADDRESS },
> +{ "archive", no_argument,NULL,   'a' },
> +{ "compress",no_argument,NULL,   'z' },
> +{ "del", no_argument,,  1 },
> +{ "delete",  no_argument,,  1 },
> +{ "devices", no_argument,,  1 },
> +{ "no-devices",  no_argument,,  0 },
> +{ "dry-run", no_argument,_run,  1 },
> +{ "group",   no_argument,_gids,1 },
> +{ "no-group",no_argument,_gids,0 },
> +{ "help",no_argument,NULL,   'h' },
> +{ "links",   no_argument,_links,   1 },
> +{ "no-links",no_argument,_links,   0 },
> +{ "no-motd", no_argument,_motd,  1 },
> +{ "numeric-ids", no_argument,_ids,  1 },
> +{ "owner",   no_argument,_uids,1 },
> +{ "no-owner",no_argument,_uids,0 },
> +{ "perms",   no_argument,_perms,   1 },
> +{ "no-perms",no_argument,_perms,   0 },
> +{ "port",required_argument, NULL,OP_PORT 
> },
> +{ "recursive",   no_argument,,1 },
> +{ "no-recursive",no_argument,,0 },
> +{ "rsh", required_argument, NULL,'e' },
> +{ "rsync-path",  required_argument, NULL,OP_RSYNCPATH },
> +{ "sender",  no_argument,,   1 },
> +{ "server",  no_argument,,   1 },
> +{ "specials",no_argument,, 1 },
> +{ "no-specials", no_argument,, 0 },
> +{ "timeout", required_argument, NULL,OP_TIMEOUT },
> +{ "times",   no_argument,_times,   1 },
> +{ "no-times",no_argument,_times,   0 },
> +{ "verbose", no_argument,,   1 },
> +{ "no-verbose",  no_argument,,   0 },
> +{ "version", no_argument,NULL,   OP_VERSION },
> +{ NULL,  0,  NULL,   0 }
> +};
> +
>  int
>  main(int argc, char *argv[])
>  {
> - struct opts  opts;
>   pid_tchild;
>   int  fds[2], sd = -1, rc, c, st, i;
>   struct sess   sess;
>   struct fargs*fargs;
>   char**args;
>   const char  *errstr;
> - const struct option  lopts[] = {
> - { "port",   required_argument, NULL,3 },
> - { "rsh",required_argument, NULL,'e' },
> - { "rsync-path", required_argument, NULL,1 },
> - { "sender", no_argument,,   1 },
> - { "server", no_argument,,   1 },
> - { "dry-run",no_argument,_run,  1 },
> - { "version",no_argument,NULL,   2 },
> - { "archive",no_argument,NULL,   'a' },
> - { "help",   no_argument,NULL,   'h' },
> - { "compress",   no_argument,NULL,   'z' },
> - { "del",no_argument,,  1 },
> - { "delete", no_argument,,  1 },
> - { "devices",no_argument,   

Re: bgpd refactor network flush code a bit

2021-06-24 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.06.24 17:06:36 +0200:
> The network flush code only operates on peerself (like all the other
> network commands). Instead of passing a peer to the tree walker just
> default to peerself in network_flush_upcall().
> This makes the code more obivous that it operates on peerself.
> 

ok


> -- 
> :wq Claudio
> 
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.528
> diff -u -p -r1.528 rde.c
> --- rde.c 24 Jun 2021 13:03:31 -  1.528
> +++ rde.c 24 Jun 2021 15:00:32 -
> @@ -517,7 +517,7 @@ badnetdel:
>   break;
>   }
>   if (rib_dump_new(RIB_ADJ_IN, AID_UNSPEC,
> - RDE_RUNNER_ROUNDS, peerself, network_flush_upcall,
> + RDE_RUNNER_ROUNDS, NULL, network_flush_upcall,
>   NULL, NULL) == -1)
>   log_warn("rde_dispatch: IMSG_NETWORK_FLUSH");
>   break;
> @@ -4065,13 +4065,12 @@ network_dump_upcall(struct rib_entry *re
>  static void
>  network_flush_upcall(struct rib_entry *re, void *ptr)
>  {
> - struct rde_peer *peer = ptr;
>   struct bgpd_addr addr;
>   struct prefix *p;
>   u_int32_t i;
>   u_int8_t prefixlen;
>  
> - p = prefix_bypeer(re, peer);
> + p = prefix_bypeer(re, peerself);
>   if (p == NULL)
>   return;
>   if ((prefix_aspath(p)->flags & F_ANN_DYNAMIC) != F_ANN_DYNAMIC)
> @@ -4084,14 +4083,14 @@ network_flush_upcall(struct rib_entry *r
>   struct rib *rib = rib_byid(i);
>   if (rib == NULL)
>   continue;
> - if (prefix_withdraw(rib, peer, , prefixlen) == 1)
> - rde_update_log("flush announce", i, peer,
> + if (prefix_withdraw(rib, peerself, , prefixlen) == 1)
> + rde_update_log("flush announce", i, peerself,
>   NULL, , prefixlen);
>   }
>  
> - if (prefix_withdraw(rib_byid(RIB_ADJ_IN), peer, ,
> + if (prefix_withdraw(rib_byid(RIB_ADJ_IN), peerself, ,
>   prefixlen) == 1)
> - peer->prefix_cnt--;
> + peerself->prefix_cnt--;
>  }
>  
>  /* clean up */
> 



Re: bgpd shuffle some code around

2021-06-24 Thread Sebastian Benoit
ok

Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.06.24 17:03:58 +0200:
> In rde_update_dispatch() do the AFI check for IPv4 prefixes before
> extracting the prefix. This is similar to what the MP code does and
> is also more logical.
> 
> OK?
> -- 
> :wq Claudio
> 
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.528
> diff -u -p -r1.528 rde.c
> --- rde.c 24 Jun 2021 13:03:31 -  1.528
> +++ rde.c 24 Jun 2021 15:00:10 -
> @@ -1280,6 +1280,14 @@ rde_update_dispatch(struct rde_peer *pee
>  
>   /* withdraw prefix */
>   while (len > 0) {
> + if (peer->capa.mp[AID_INET] == 0) {
> + log_peer_warnx(>conf,
> + "bad withdraw, %s disabled", aid2str(AID_INET));
> + rde_update_err(peer, ERR_UPDATE, ERR_UPD_OPTATTR,
> + NULL, 0);
> + goto done;
> + }
> +
>   if ((pos = nlri_get_prefix(p, len, ,
>   )) == -1) {
>   /*
> @@ -1294,14 +1302,6 @@ rde_update_dispatch(struct rde_peer *pee
>   p += pos;
>   len -= pos;
>  
> - if (peer->capa.mp[AID_INET] == 0) {
> - log_peer_warnx(>conf,
> - "bad withdraw, %s disabled", aid2str(AID_INET));
> - rde_update_err(peer, ERR_UPDATE, ERR_UPD_OPTATTR,
> - NULL, 0);
> - goto done;
> - }
> -
>   rde_update_withdraw(peer, , prefixlen);
>   }
>  
> @@ -1393,6 +1393,14 @@ rde_update_dispatch(struct rde_peer *pee
>  
>   /* parse nlri prefix */
>   while (nlri_len > 0) {
> + if (peer->capa.mp[AID_INET] == 0) {
> + log_peer_warnx(>conf,
> + "bad update, %s disabled", aid2str(AID_INET));
> + rde_update_err(peer, ERR_UPDATE, ERR_UPD_OPTATTR,
> + NULL, 0);
> + goto done;
> + }
> +
>   if ((pos = nlri_get_prefix(p, nlri_len, ,
>   )) == -1) {
>   log_peer_warnx(>conf, "bad nlri prefix");
> @@ -1402,14 +1410,6 @@ rde_update_dispatch(struct rde_peer *pee
>   }
>   p += pos;
>   nlri_len -= pos;
> -
> - if (peer->capa.mp[AID_INET] == 0) {
> - log_peer_warnx(>conf,
> - "bad update, %s disabled", aid2str(AID_INET));
> - rde_update_err(peer, ERR_UPDATE, ERR_UPD_OPTATTR,
> - NULL, 0);
> - goto done;
> - }
>  
>   if (rde_update_update(peer, , , prefixlen) == -1)
>   goto done;
> 



Re: bgpd fix add-path capability encoding

2021-06-24 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.06.22 19:47:04 +0200:
> Dumb copy paste error. The add-path capability is 4byte per AFI/SAFI
> the 2 + is from graceful restart where two extra bytes are at the front of
> the AFI/SAFI list.

ok.

> 
> -- 
> :wq Claudio
> 
> Index: session.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
> retrieving revision 1.421
> diff -u -p -r1.421 session.c
> --- session.c 17 Jun 2021 16:05:26 -  1.421
> +++ session.c 22 Jun 2021 11:50:08 -
> @@ -1470,9 +1470,9 @@ session_open(struct peer *p)
>   u_int8_taplen;
>  
>   if (mpcapa)
> - aplen = 2 + 4 * mpcapa;
> + aplen = 4 * mpcapa;
>   else/* AID_INET */
> - aplen = 2 + 4;
> + aplen = 4;
>   errs += session_capa_add(opb, CAPA_ADD_PATH, aplen);
>   if (mpcapa) {
>   for (i = AID_MIN; i < AID_MAX; i++) {
> 



OpenBSD Errata: June 25, 2021 (bgpd)

2021-06-24 Thread Sebastian Benoit
An errata patch for the bgpd routing daemon has been released for
OpenBSD 6.9.

During bgpd(8) config reloads prefixes of the wrong address family could
leak to peers resulting in session resets.

Binary updates for the amd64, i386, and arm64 platform are available
via the syspatch utility. Source code patches can be found on the
respective errata page:

  https://www.openbsd.org/errata69.html
  



rpki-client 7.1 released

2021-05-18 Thread Sebastian Benoit
rpki-client 7.1 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.

This release includes the following changes to the previous release:

 * Add keep-alive support to the HTTP client code for RRDP,
 * Reference-count and delete unused files synced via RRDP, as far as
   possible,
 * In the JSON output, change the AS Number from a string ("AS123") to
   an integer ("123") to make processing of the output easier,
 * Add an 'expires' column to CSV & JSON output, based on certificate
   and CRL validity times. The 'expires' value can be used to avoid route
   selection based on stale data when generating VRP sets, when faced
   with loss of communication between consumer and valdiator, or
   validator and CA repository,
 * Make the runtime timeout (-s option) also triggers in
   child proecesses.
 * Improved RRDP support, we encourage testing of RRDP with the -r
   option so that RRDP can be enabled by default in a future release.
   Please report any issues found.

In the portable version,

 * Improve support for older libressl versions (altough the latest
   stable release is recommended),
 * Add missing compat headers in release packages so they build on
   Alpine Linux and macOS.

rpki-client is known to compile and run on at least the following
operating systems: Alpine 3.12, Debian 9, 10, Fedora 31, 32, 33, macOS,
RHEL/CentOS 7, 8, Windows Subsystem for Linux 2, and OpenBSD.

It is our hope that packagers take interest and help adapt
rpki-client-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: httpd(8): don't try to chunk-encode an empty body

2021-05-17 Thread Sebastian Benoit
Florian Obser(flor...@openbsd.org) on 2021.05.15 17:14:38 +0200:
> Turns out it's not that difficult to do this correctly since we already
> wait until we read all http headers from the fcgi upstream. We just need
> to delay writing of the http header until we know if the body is empty
> or not.
> 
> OK?

seems to work, ok benno@

The comments in server_fcgi_header seem to suggest more dragons lurk in this
area.

> 
> 
> diff --git httpd.h httpd.h
> index b3a40b3af68..c4adfba232d 100644
> --- httpd.h
> +++ httpd.h
> @@ -300,6 +300,7 @@ struct fcgi_data {
>   int  end;
>   int  status;
>   int  headersdone;
> + int  headerssent;
>  };
>  
>  struct range {
> diff --git server_fcgi.c server_fcgi.c
> index 64a0e9d2abb..e1e9704c920 100644
> --- server_fcgi.c
> +++ server_fcgi.c
> @@ -114,6 +114,7 @@ server_fcgi(struct httpd *env, struct client *clt)
>   clt->clt_fcgi.toread = sizeof(struct fcgi_record_header);
>   clt->clt_fcgi.status = 200;
>   clt->clt_fcgi.headersdone = 0;
> + clt->clt_fcgi.headerssent = 0;
>  
>   if (clt->clt_srvevb != NULL)
>   evbuffer_free(clt->clt_srvevb);
> @@ -544,22 +545,20 @@ server_fcgi_read(struct bufferevent *bev, void *arg)
>   if (!clt->clt_fcgi.headersdone) {
>   clt->clt_fcgi.headersdone =
>   server_fcgi_getheaders(clt);
> - if (clt->clt_fcgi.headersdone) {
> - if (server_fcgi_header(clt,
> - clt->clt_fcgi.status)
> - == -1) {
> - server_abort_http(clt,
> - 500,
> - "malformed fcgi "
> - "headers");
> - return;
> - }
> - }
>   if (!EVBUFFER_LENGTH(clt->clt_srvevb))
>   break;
>   }
>   /* FALLTHROUGH */
>   case FCGI_END_REQUEST:
> + if (clt->clt_fcgi.headersdone &&
> + !clt->clt_fcgi.headerssent) {
> + if (server_fcgi_header(clt,
> + clt->clt_fcgi.status) == -1) {
> + server_abort_http(clt, 500,
> + "malformed fcgi headers");
> + return;
> + }
> + }
>   if (server_fcgi_writechunk(clt) == -1) {
>   server_abort_http(clt, 500,
>   "encoding error");
> @@ -600,6 +599,8 @@ server_fcgi_header(struct client *clt, unsigned int code)
>   char tmbuf[32];
>   struct kv   *kv, *cl, key;
>  
> + clt->clt_fcgi.headerssent = 1;
> +
>   if (desc == NULL || (error = server_httperror_byid(code)) == NULL)
>   return (-1);
>  
> @@ -615,6 +616,12 @@ server_fcgi_header(struct client *clt, unsigned int code)
>   if (kv_add(>http_headers, "Server", HTTPD_SERVERNAME) == NULL)
>   return (-1);
>  
> + if (clt->clt_fcgi.type == FCGI_END_REQUEST ||
> + EVBUFFER_LENGTH(clt->clt_srvevb) == 0) {
> + /* Can't chunk encode an empty body. */
> + clt->clt_fcgi.chunked = 0;
> + }
> +
>   /* Set chunked encoding */
>   if (clt->clt_fcgi.chunked) {
>   /* XXX Should we keep and handle Content-Length instead? */
> 
> 
> -- 
> I'm not entirely sure you are real.
> 



Re: bgpd strict community negotiation

2021-05-14 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.05.12 19:49:08 +0200:
> RFC5492 is fairly explicit when a capability should be enabled on a
> session:
> 
>A BGP speaker that supports a particular capability may use this
>capability with its peer after the speaker determines (as described
>above) that the peer supports this capability.  Simply put, a given
>capability can be used on a peering if that capability has been
>advertised by both peers.  If either peer has not advertised it, the
>capability cannot be used.
> 
> Adjust capa_neg_calc() to follow this strict model.
> This affects route refersh and graceful restart. For graceful restart this
> requires to flush the RIBs immediatly.
> 
> Also ignore and warn about RREFRESH messages that are received on a
> session where route refesh is disabled.

ok.

This might actually fix some strange behaviour re graceful restart i've seen in 
the
past.

/Benno


> 
> -- 
> :wq Claudio
> 
> Index: session.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
> retrieving revision 1.414
> diff -u -p -r1.414 session.c
> --- session.c 6 May 2021 09:18:54 -   1.414
> +++ session.c 12 May 2021 16:33:42 -
> @@ -1636,7 +1636,7 @@ session_neighbor_rrefresh(struct peer *p
>  {
>   u_int8_ti;
>  
> - if (!p->capa.peer.refresh)
> + if (!p->capa.neg.refresh)
>   return (-1);
>  
>   for (i = 0; i < AID_MAX; i++) {
> @@ -2257,6 +2257,11 @@ parse_refresh(struct peer *peer)
>   return (0);
>   }
>  
> + if (!peer->capa.neg.refresh) {
> + log_peer_warnx(>conf, "peer sent unexpected refresh");
> + return (0);
> + }
> +
>   if (imsg_rde(IMSG_REFRESH, peer->conf.id, , sizeof(aid)) == -1)
>   return (-1);
>  
> @@ -2546,16 +2551,15 @@ capa_neg_calc(struct peer *p)
>  {
>   u_int8_ti, hasmp = 0;
>  
> - /* refresh: does not realy matter here, use peer setting */
> - p->capa.neg.refresh = p->capa.peer.refresh;
> + /* a capability is accepted only if both sides announced it */
>  
> - /* as4byte: both side must announce capability */
> - if (p->capa.ann.as4byte && p->capa.peer.as4byte)
> - p->capa.neg.as4byte = 1;
> - else
> - p->capa.neg.as4byte = 0;
> + p->capa.neg.refresh =
> + (p->capa.ann.refresh && p->capa.peer.refresh) != 0;
> +
> + p->capa.neg.as4byte =
> + (p->capa.ann.as4byte && p->capa.peer.as4byte) != 0;
>  
> - /* MP: both side must announce capability */
> + /* MP: both side must agree on the AFI,SAFI pair */
>   for (i = 0; i < AID_MAX; i++) {
>   if (p->capa.ann.mp[i] && p->capa.peer.mp[i])
>   p->capa.neg.mp[i] = 1;
> @@ -2569,18 +2573,21 @@ capa_neg_calc(struct peer *p)
>   p->capa.neg.mp[AID_INET] = 1;
>  
>   /*
> -  * graceful restart: only the peer capabilities are of interest here.
> +  * graceful restart: the peer capabilities are of interest here.
>* It is necessary to compare the new values with the previous ones
>* and act acordingly. AFI/SAFI that are not part in the MP capability
>* are treated as not being present.
> +  * Also make sure that a flush happens if the session stopped
> +  * supporting graceful restart.
>*/
>  
>   for (i = 0; i < AID_MAX; i++) {
>   int8_t  negflags;
>  
>   /* disable GR if the AFI/SAFI is not present */
> - if (p->capa.peer.grestart.flags[i] & CAPA_GR_PRESENT &&
> - p->capa.neg.mp[i] == 0)
> + if (p->capa.ann.grestart.restart == 0 ||
> + (p->capa.peer.grestart.flags[i] & CAPA_GR_PRESENT &&
> + p->capa.neg.mp[i] == 0))
>   p->capa.peer.grestart.flags[i] = 0; /* disable */
>   /* look at current GR state and decide what to do */
>   negflags = p->capa.neg.grestart.flags[i];
> @@ -2600,6 +2607,8 @@ capa_neg_calc(struct peer *p)
>   }
>   p->capa.neg.grestart.timeout = p->capa.peer.grestart.timeout;
>   p->capa.neg.grestart.restart = p->capa.peer.grestart.restart;
> + if (p->capa.ann.grestart.restart == 0)
> + p->capa.neg.grestart.restart = 0;
>  
>   return (0);
>  }
> 



Re: rsync fix file handling in uploader

2021-05-14 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.05.07 17:12:16 +0200:
> So yesterday I committed a change to simplify file handling. This removed
> the O_NONBLOCK flag from openat() but today I realized that this was a bit
> premature. The code at that point does not know if the file is actually a
> regular file and so if you put a fifo in place of a regular file it will
> hang rsync.

right :/

> 
> Anyway doing a blind open of any file here is far from ideal. There may be
> involuntary side-effects especially on device nodes. So better change the
> code to do the fstatat() first and only open the file if it is indeed a
> regular file.
> 
> While there change log_link to log_symlink which is more precise and also
> fix a comment that missed some context.
> 
> OK?

yes.

> -- 
> :wq Claudio
> 
> Index: uploader.c
> ===
> RCS file: /cvs/src/usr.bin/rsync/uploader.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 uploader.c
> --- uploader.c6 May 2021 07:35:22 -   1.25
> +++ uploader.c7 May 2021 15:03:03 -
> @@ -79,7 +79,7 @@ log_dir(struct sess *sess, const struct 
>   * operator that we're a link.
>   */
>  static void
> -log_link(struct sess *sess, const struct flist *f)
> +log_symlink(struct sess *sess, const struct flist *f)
>  {
>  
>   if (!sess->opts->server)
> @@ -181,7 +181,7 @@ pre_link(struct upload *p, struct sess *
>   return 0;
>   }
>   if (sess->opts->dry_run) {
> - log_link(sess, f);
> + log_symlink(sess, f);
>   return 0;
>   }
>  
> @@ -259,7 +259,7 @@ pre_link(struct upload *p, struct sess *
>   free(temp);
>   }
>  
> - log_link(sess, f);
> + log_symlink(sess, f);
>   return 0;
>  }
>  
> @@ -645,6 +645,7 @@ pre_file(const struct upload *p, int *fi
>  struct sess *sess)
>  {
>   const struct flist *f;
> + int rc;
>  
>   f = >fl[p->idx];
>   assert(S_ISREG(f->st.mode));
> @@ -661,51 +662,43 @@ pre_file(const struct upload *p, int *fi
>   /*
>* For non dry-run cases, we'll write the acknowledgement later
>* in the rsync_uploader() function.
> -  * If the call to openat() fails with ENOENT, there's a
> -  * fast-path between here and the write function.
>*/
>  
> - *filefd = openat(p->rootfd, f->path,
> - O_RDONLY | O_NOFOLLOW, 0);
> + *filefd = -1;
> + rc = fstatat(p->rootfd, f->path, st, AT_SYMLINK_NOFOLLOW);
>  
> - if (*filefd == -1 && errno != ENOENT) {
> - ERR("%s: openat", f->path);
> + if (rc == -1) {
> + if (errno == ENOENT)
> + return 1;
> +
> + ERR("%s: fstatat", f->path);
>   return -1;
>   }
> - if (*filefd == -1)
> + if (rc != -1 && !S_ISREG(st->st_mode)) {
> + if (S_ISDIR(st->st_mode) &&
> + unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
> + ERR("%s: unlinkat", f->path);
> + return -1;
> + }
>   return 1;
> -
> - /*
> -  * Check if the file is already up to date, in which case close it
> -  * and go to the next one.
> -  */
> -
> - if (fstat(*filefd, st) == -1) {
> - ERR("%s: fstat", f->path);
> - close(*filefd);
> - *filefd = -1;
> - return -1;
> - } else if (!S_ISREG(st->st_mode)) {
> - ERRX("%s: not regular", f->path);
> - close(*filefd);
> - *filefd = -1;
> - return -1;
>   }
>  
>   if (st->st_size == f->st.size &&
>   st->st_mtime == f->st.mtime) {
>   LOG3("%s: skipping: up to date", f->path);
> - if (!rsync_set_metadata(sess, 0, *filefd, f, f->path)) {
> + if (!rsync_set_metadata_at(sess, 0, p->rootfd, f, f->path)) {
>   ERRX1("rsync_set_metadata");
> - close(*filefd);
> - *filefd = -1;
>   return -1;
>   }
> - close(*filefd);
> - *filefd = -1;
>   return 0;
>   }
>  
> + *filefd = openat(p->rootfd, f->path, O_RDONLY | O_NOFOLLOW, 0);
> + if (*filefd == -1 && errno != ENOENT) {
> + ERR("%s: openat", f->path);
> + return -1;
> + }
> +
>   /* file needs attention */
>   return 1;
>  }
> @@ -785,8 +778,7 @@ rsync_uploader(struct upload *u, int *fi
>   off_t   offs;
>   int c;
>  
> - /* This should never get called. */
> -
> + /* Once finished this should never get called again. */
>   assert(u->state != UPLOAD_FINISHED);
>  
>   /*
> 



Re: rsync exit code and error cleanup

2021-05-14 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.05.07 12:16:26 +0200:
> Currently our rsync does not follow the exit codes from rsync. Also the
> error handling is complex because ERR() and ERRX() are not terminating the
> process.
> 
> This diff tries to start cleaning up the mess a bit. Introduce some exit
> codes to use and apply them in places where it is obvious.
> 
> The rsync server process should normally communicate errors back via the
> rsync socket but there are some errors where this will most probably fail
> as well. A good example are memory failures.
> 
> I used ERR_IPC as a kind of catchall for any system error that pops up
> (fork, socketpair but also pledge, unveil).
> 
> The goal is to reduce the amount of ERR(), ERRX() and ERRX1() from rsync.
> This should simplify the code base.
> 
> I also synced the mkpath() call with the one from mkdir and handle the
> error now outside of the call. Again I think the result is cleaner.
> 
> OK?

ok benno

> -- 
> :wq Claudio
> 
> Index: client.c
> ===
> RCS file: /cvs/src/usr.bin/rsync/client.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 client.c
> --- client.c  8 May 2019 20:00:25 -   1.15
> +++ client.c  7 May 2021 08:29:10 -
> @@ -43,7 +43,7 @@ rsync_client(const struct opts *opts, in
>  
>   if (pledge("stdio unix rpath wpath cpath dpath fattr chown getpw 
> unveil",
>   NULL) == -1)
> - err(1, "pledge");
> + err(ERR_IPC, "pledge");
>  
>   memset(, 0, sizeof(struct sess));
>   sess.opts = opts;
> Index: extern.h
> ===
> RCS file: /cvs/src/usr.bin/rsync/extern.h,v
> retrieving revision 1.36
> diff -u -p -r1.36 extern.h
> --- extern.h  31 Mar 2021 19:45:16 -  1.36
> +++ extern.h  7 May 2021 08:22:32 -
> @@ -42,6 +42,19 @@
>  #define  CSUM_LENGTH_PHASE2 (16)
>  
>  /*
> + * Rsync error codes.
> + */
> +#define ERR_SYNTAX   1
> +#define ERR_PROTOCOL 2
> +#define ERR_SOCK_IO  10
> +#define ERR_FILE_IO  11
> +#define ERR_WIREPROTO12
> +#define ERR_IPC  14  /* catchall for any kind of syscall 
> error */
> +#define ERR_TERMIMATED   16
> +#define ERR_WAITPID  21
> +#define ERR_NOMEM22
> +
> +/*
>   * Use this for --timeout.
>   * All poll events will use it and catch time-outs.
>   */
> Index: fargs.c
> ===
> RCS file: /cvs/src/usr.bin/rsync/fargs.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 fargs.c
> --- fargs.c   8 May 2019 20:00:25 -   1.17
> +++ fargs.c   7 May 2021 08:19:29 -
> @@ -17,6 +17,7 @@
>  #include 
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -51,7 +52,7 @@ fargs_cmdline(struct sess *sess, const s
>   if (sess->opts->ssh_prog) {
>   ap = strdup(sess->opts->ssh_prog);
>   if (ap == NULL)
> - goto out;
> + err(ERR_NOMEM, NULL);
>  
>   while ((arg = strsep(, " \t")) != NULL) {
>   if (arg[0] == '\0') {
> @@ -127,8 +128,4 @@ fargs_cmdline(struct sess *sess, const s
>   addargs(, "%s", f->sink);
>  
>   return args.list;
> -out:
> - freeargs();
> - ERR("calloc");
> - return NULL;
>  }
> Index: main.c
> ===
> RCS file: /cvs/src/usr.bin/rsync/main.c,v
> retrieving revision 1.53
> diff -u -p -r1.53 main.c
> --- main.c31 Mar 2021 19:45:16 -  1.53
> +++ main.c7 May 2021 08:30:18 -
> @@ -83,18 +83,18 @@ fargs_parse(size_t argc, char *argv[], s
>   /* Allocations. */
>  
>   if ((f = calloc(1, sizeof(struct fargs))) == NULL)
> - err(1, "calloc");
> + err(ERR_NOMEM, NULL);
>  
>   f->sourcesz = argc - 1;
>   if ((f->sources = calloc(f->sourcesz, sizeof(char *))) == NULL)
> - err(1, "calloc");
> + err(ERR_NOMEM, NULL);
>  
>   for (i = 0; i < argc - 1; i++)
>   if ((f->sources[i] = strdup(argv[i])) == NULL)
> - err(1, "strdup");
> + err(ERR_NOMEM, NULL);
>  
>   if ((f->sink = strdup(argv[i])) == NULL)
> - err(1, "strdup");
> + err(ERR_NOMEM, NULL);
>  
>   /*
>* Test files for its locality.
> @@ -109,15 +109,16 @@ fargs_parse(size_t argc, char *argv[], s
>   if (fargs_is_remote(f->sink)) {
>   f->mode = FARGS_SENDER;
>   if ((f->host = strdup(f->sink)) == NULL)
> - err(1, "strdup");
> + err(ERR_NOMEM, NULL);
>   }
>  
>   if (fargs_is_remote(f->sources[0])) {
>   if (f->host != NULL)
> - errx(1, "both source and destination cannot be remote 
> files");
> + 

Re: limit concurrent RTR connects in bgpd

2021-05-14 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.05.14 11:21:11 +0200:
> I think it is a good idea to limit the number of concurrent connects in
> bgpd. I used 32 as the limit since that is way enough for the number of
> RTR sessions people will configure.
> 
> If the limit is hit the request will be dropped and the rtr process will
> retry the connect after the retry timeout. Hopefully by then the number of
> connections is down again.

ok benno

but i suggest to s/inflight/concurrent/, and keep the word inflight for log
messages about tracking connections for purpose of not running out of file
descriptors?

> -- 
> :wq Claudio
> 
> Index: bgpd.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.c,v
> retrieving revision 1.236
> diff -u -p -r1.236 bgpd.c
> --- bgpd.c11 May 2021 07:57:24 -  1.236
> +++ bgpd.c11 May 2021 08:00:25 -
> @@ -74,6 +74,7 @@ struct connect_elm {
>  TAILQ_HEAD( ,connect_elm)connect_queue = \
>   TAILQ_HEAD_INITIALIZER(connect_queue);
>  u_intconnect_cnt;
> +#define MAX_CONNECT_CNT  32
>  
>  void
>  sighdlr(int sig)
> @@ -1303,6 +1304,12 @@ bgpd_rtr_connect(struct rtr_config *r)
>   struct connect_elm *ce;
>   struct sockaddr *sa;
>   socklen_t len;
> +
> + if (connect_cnt >= MAX_CONNECT_CNT) {
> + log_warnx("rtr %s: too many inflight connection requests",
> + r->descr);
> + return;
> + }
>  
>   if ((ce = calloc(1, sizeof(*ce))) == NULL) {
>   log_warn("rtr %s", r->descr);
> 



Re: httpd(8): don't try to chunk-encode an empty body

2021-05-14 Thread Sebastian Benoit
Florian Obser(flor...@openbsd.org) on 2021.05.14 19:13:49 +0200:
> As found out by Chris Narkiewicz the hard way, trying to chunk encode an
> empty body makes the nextclown app stop working. (see "Nextcloud stopped
> working after upgrade to 6.9" on ports@).
> 
> I don't think there is a valid way to do this, so don't try to.
> 
> This is kinda maybe a hack since there might be other stuff that isn't
> sending a body. So if anyone has a long list of status codes that should
> be in the same list, let me know.

https://datatracker.ietf.org/doc/html/rfc7230#page-32
gives the list as 

   ... any response with a 1xx
   (Informational), 204 (No Content), or 304 (Not Modified) status
   code is always terminated by the first empty line after the
   header fields, regardless of the header fields present in the
   message, and thus cannot contain a message body.

see the text for other responses that must have an empty body (for other
reasons than the status code).

/Benno

> 
> We can't easily find out if the fcgi server is going to send us an
> empty body without reading everything from the server until we hit the
> body. I'm happy to entertain a diff that does that.
> 
> In the meantime, this scratches my itch.
> OK?

ok benno@

> 
> diff --git server_fcgi.c server_fcgi.c
> index 8d3b581568f..0ac80c27d11 100644
> --- server_fcgi.c
> +++ server_fcgi.c
> @@ -615,6 +615,10 @@ server_fcgi_header(struct client *clt, unsigned int code)
>   if (kv_add(>http_headers, "Server", HTTPD_SERVERNAME) == NULL)
>   return (-1);
>  
> + /* we cannot chunk-encode no-content */
> + if (code == 204)
> + clt->clt_fcgi.chunked = 0;
> +
>   /* Set chunked encoding */
>   if (clt->clt_fcgi.chunked) {
>   /* XXX Should we keep and handle Content-Length instead? */
> 
> 
> -- 
> I'm not entirely sure you are real.
> 



Re: bgpd, non-blocking rtr connect

2021-05-10 Thread Sebastian Benoit
ok benno@

Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.05.03 17:37:36 +0200:
> The RTR session was opened with a blocking connect() call. This is rather
> bad if the RTR peer does not exist since then bgpd will block until the
> connect timed out. This diff makes the connect() call non-blocking.
> With this connecting to non-existing RTR servers no longer blocks the main
> process.
> 
> -- 
> :wq Claudio
> 
> Index: bgpd.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.c,v
> retrieving revision 1.235
> diff -u -p -r1.235 bgpd.c
> --- bgpd.c3 May 2021 13:18:06 -   1.235
> +++ bgpd.c3 May 2021 15:33:20 -
> @@ -50,6 +50,7 @@ static void getsockpair(int [2]);
>  int  imsg_send_sockets(struct imsgbuf *, struct imsgbuf *,
>   struct imsgbuf *);
>  void bgpd_rtr_connect(struct rtr_config *);
> +void bgpd_rtr_connect_done(int, struct bgpd_config *);
>  
>  int   cflags;
>  volatile sig_atomic_t mrtdump;
> @@ -64,6 +65,16 @@ struct rib_namesribnames = SIMPLEQ_HEA
>  char *cname;
>  char *rcname;
>  
> +struct connect_elm {
> + TAILQ_ENTRY(connect_elm)entry;
> + u_int32_t   id;
> + int fd;
> +};
> +
> +TAILQ_HEAD( ,connect_elm)connect_queue = \
> + TAILQ_HEAD_INITIALIZER(connect_queue);
> +u_intconnect_cnt;
> +
>  void
>  sighdlr(int sig)
>  {
> @@ -97,7 +108,7 @@ usage(void)
>  #define PFD_PIPE_RTR 2
>  #define PFD_SOCK_ROUTE   3
>  #define PFD_SOCK_PFKEY   4
> -#define POLL_MAX 5
> +#define PFD_CONNECT_START5
>  #define MAX_TIMEOUT  3600
>  
>  int   cmd_opts;
> @@ -109,11 +120,13 @@ main(int argc, char *argv[])
>   enum bgpd_processproc = PROC_MAIN;
>   struct rde_rib  *rr;
>   struct peer *p;
> - struct pollfdpfd[POLL_MAX];
> + struct pollfd   *pfd = NULL;
> + struct connect_elm  *ce;
>   time_t   timeout;
>   pid_tse_pid = 0, rde_pid = 0, rtr_pid = 0, pid;
>   char*conffile;
>   char*saved_argv0;
> + u_intpfd_elms = 0, npfd, i;
>   int  debug = 0;
>   int  rfd, keyfd;
>   int  ch, status;
> @@ -289,7 +302,21 @@ BROKEN   if (pledge("stdio rpath wpath cpa
>   quit = 1;
>  
>   while (quit == 0) {
> - bzero(pfd, sizeof(pfd));
> + if (pfd_elms < PFD_CONNECT_START + connect_cnt) {
> + struct pollfd *newp;
> +
> + if ((newp = reallocarray(pfd,
> + PFD_CONNECT_START + connect_cnt,
> + sizeof(struct pollfd))) == NULL) {
> + log_warn("could not resize pfd from %u -> %u"
> + " entries", pfd_elms, PFD_CONNECT_START +
> + connect_cnt);
> + fatalx("exiting");
> + }
> + pfd = newp;
> + pfd_elms = PFD_CONNECT_START + connect_cnt;
> + }
> + bzero(pfd, sizeof(struct pollfd) * pfd_elms);
>  
>   timeout = mrt_timeout(conf->mrt);
>  
> @@ -303,9 +330,17 @@ BROKEN   if (pledge("stdio rpath wpath cpa
>   set_pollfd([PFD_PIPE_RDE], ibuf_rde);
>   set_pollfd([PFD_PIPE_RTR], ibuf_rtr);
>  
> + npfd = PFD_CONNECT_START;
> + TAILQ_FOREACH(ce, _queue, entry) {
> + pfd[npfd].fd = ce->fd;
> + pfd[npfd++].events = POLLOUT;
> + if (npfd > pfd_elms)
> + fatalx("polli pfd overflow");
> + }
> +
>   if (timeout < 0 || timeout > MAX_TIMEOUT)
>   timeout = MAX_TIMEOUT;
> - if (poll(pfd, POLL_MAX, timeout * 1000) == -1)
> + if (poll(pfd, npfd, timeout * 1000) == -1)
>   if (errno != EINTR) {
>   log_warn("poll error");
>   quit = 1;
> @@ -357,6 +392,10 @@ BROKEN   if (pledge("stdio rpath wpath cpa
>   }
>   }
>  
> + for (i = PFD_CONNECT_START; i < npfd; i++)
> + if (pfd[i].revents != 0)
> + bgpd_rtr_connect_done(pfd[i].fd, conf);
> +
>   if (reconfig) {
>   u_int   error;
>  
> @@ -1261,32 +1300,97 @@ imsg_send_sockets(struct imsgbuf *se, st
>  void
>  bgpd_rtr_connect(struct rtr_config *r)
>  {
> + struct connect_elm *ce;
>   struct sockaddr *sa;
>   socklen_t len;
> -  

Re: more rsync cleanup

2021-05-07 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.05.06 17:59:32 +0200:
> As noticed by benno@ the blk.blks buffer is leaked in some cases.
> Fix those and cleanup up the pre_* functions a bit more.
> I increased the diff context a bit to make the diff easier to read.

reads ok

> 
> -- 
> :wq Claudio
> 
> Index: uploader.c
> ===
> RCS file: /cvs/src/usr.bin/rsync/uploader.c,v
> retrieving revision 1.25
> diff -u -p -U6 -r1.25 uploader.c
> --- uploader.c6 May 2021 07:35:22 -   1.25
> +++ uploader.c6 May 2021 15:34:36 -
> @@ -191,22 +191,24 @@ pre_link(struct upload *p, struct sess *
>* to overwriting with a symbolic link.
>* If it's a non-directory, we just overwrite it.
>*/
>  
>   assert(p->rootfd != -1);
>   rc = fstatat(p->rootfd, f->path, , AT_SYMLINK_NOFOLLOW);
> +
> + if (rc == -1 && errno != ENOENT) {
> + ERR("%s: fstatat", f->path);
> + return -1;
> + }
>   if (rc != -1 && !S_ISLNK(st.st_mode)) {
>   if (S_ISDIR(st.st_mode) &&
>   unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
>   ERR("%s: unlinkat", f->path);
>   return -1;
>   }
>   rc = -1;
> - } else if (rc == -1 && errno != ENOENT) {
> - ERR("%s: fstatat", f->path);
> - return -1;
>   }
>  
>   /*
>* If the symbolic link already exists, then make sure that it
>* points to the correct place.
>*/
> @@ -294,22 +296,23 @@ pre_dev(struct upload *p, struct sess *s
>* If it replaces a directory, remove the directory first.
>*/
>  
>   assert(p->rootfd != -1);
>   rc = fstatat(p->rootfd, f->path, , AT_SYMLINK_NOFOLLOW);
>  
> + if (rc == -1 && errno != ENOENT) {
> + ERR("%s: fstatat", f->path);
> + return -1;
> + }
>   if (rc != -1 && !(S_ISBLK(st.st_mode) || S_ISCHR(st.st_mode))) {
>   if (S_ISDIR(st.st_mode) &&
>   unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
>   ERR("%s: unlinkat", f->path);
>   return -1;
>   }
>   rc = -1;
> - } else if (rc == -1 && errno != ENOENT) {
> - ERR("%s: fstatat", f->path);
> - return -1;
>   }
>  
>   /* Make sure existing device is of the correct type. */
>  
>   if (rc != -1) {
>   if ((f->st.mode & (S_IFCHR|S_IFBLK)) !=
> @@ -382,22 +385,23 @@ pre_fifo(struct upload *p, struct sess *
>* mark it from replacement.
>*/
>  
>   assert(p->rootfd != -1);
>   rc = fstatat(p->rootfd, f->path, , AT_SYMLINK_NOFOLLOW);
>  
> + if (rc == -1 && errno != ENOENT) {
> + ERR("%s: fstatat", f->path);
> + return -1;
> + }
>   if (rc != -1 && !S_ISFIFO(st.st_mode)) {
>   if (S_ISDIR(st.st_mode) &&
>   unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
>   ERR("%s: unlinkat", f->path);
>   return -1;
>   }
>   rc = -1;
> - } else if (rc == -1 && errno != ENOENT) {
> - ERR("%s: fstatat", f->path);
> - return -1;
>   }
>  
>   if (rc == -1) {
>   newfifo = 1;
>   if (mktemplate(, f->path, sess->opts->recursive) == -1) {
>   ERRX1("mktemplate");
> @@ -458,22 +462,23 @@ pre_sock(struct upload *p, struct sess *
>* mark it from replacement.
>*/
>  
>   assert(p->rootfd != -1);
>   rc = fstatat(p->rootfd, f->path, , AT_SYMLINK_NOFOLLOW);
>  
> + if (rc == -1 && errno != ENOENT) {
> + ERR("%s: fstatat", f->path);
> + return -1;
> + }
>   if (rc != -1 && !S_ISSOCK(st.st_mode)) {
>   if (S_ISDIR(st.st_mode) &&
>   unlinkat(p->rootfd, f->path, AT_REMOVEDIR) == -1) {
>   ERR("%s: unlinkat", f->path);
>   return -1;
>   }
>   rc = -1;
> - } else if (rc == -1 && errno != ENOENT) {
> - ERR("%s: fstatat", f->path);
> - return -1;
>   }
>  
>   if (rc == -1) {
>   newsock = 1;
>   if (mktemplate(, f->path, sess->opts->recursive) == -1) {
>   ERRX1("mktemplate");
> @@ -530,13 +535,14 @@ pre_dir(const struct upload *p, struct s
>   assert(p->rootfd != -1);
>   rc = fstatat(p->rootfd, f->path, , AT_SYMLINK_NOFOLLOW);
>  
>   if (rc == -1 && errno != ENOENT) {
>   ERR("%s: fstatat", f->path);
>   return -1;
> - } else if (rc != -1 && !S_ISDIR(st.st_mode)) {
> + }
> + if (rc != -1 && !S_ISDIR(st.st_mode)) {
>   ERRX("%s: not a directory", f->path);
>   return -1;
>   } else if (rc != -1) {
>   /*
> 

Re: make rsync -v less verbose

2021-05-05 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.05.05 20:03:19 +0200:
> I like rsync -v but hell it is noisy with openrsync.
> Just shut up about all the files that have not changed unless you go -vv.

Before we do this, are there reasons to keep this like it is in the original?

I think i actually made it do this.

If you really think it helps, ok benno@

> 
> -- 
> :wq Claudio
> 
> Index: downloader.c
> ===
> RCS file: /cvs/src/usr.bin/rsync/downloader.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 downloader.c
> --- downloader.c  8 May 2019 21:30:11 -   1.21
> +++ downloader.c  5 May 2021 17:56:20 -
> @@ -85,7 +85,7 @@ log_file(struct sess *sess,
>   if (sess->opts->server)
>   return;
>  
> - frac = (dl->total == 0) ? 100.0 :
> + frac = (dl->total == 0) ? 0.0 :
>   100.0 * dl->downloaded / dl->total;
>  
>   if (dl->total > 1024 * 1024 * 1024) {
> @@ -102,8 +102,12 @@ log_file(struct sess *sess,
>   unit = "KB";
>   }
>  
> - LOG1("%s (%.*f %s, %.1f%% downloaded)",
> - f->path, prec, tot, unit, frac);
> + if (dl->downloaded > 0)
> + LOG1("%s (%.*f %s, %.1f%% downloaded)",
> + f->path, prec, tot, unit, frac);
> + else
> + LOG2("%s (%.*f %s, %.1f%% downloaded)",
> + f->path, prec, tot, unit, frac);
>  }
>  
>  /*
> 



Re: rpki-client: change "asn" from string to integer in JSON output

2021-05-05 Thread Sebastian Benoit
Job Snijders(j...@openbsd.org) on 2021.05.05 16:35:46 +:
> I'd like to modify our JSON format, many people in the community have
> voiced complaints that transforming the string to an integer is
> annoying.
> 
> This won't break existing deployments coupled with GoRTR.
> 
> OK?

ok benno@

> 
> Index: output-json.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/output-json.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 output-json.c
> --- output-json.c 8 Apr 2021 19:49:27 -   1.15
> +++ output-json.c 5 May 2021 15:29:15 -
> @@ -100,7 +100,7 @@ output_json(FILE *out, struct vrp_tree *
>  
>   ip_addr_print(>addr, v->afi, buf, sizeof(buf));
>  
> - if (fprintf(out, "\t\t{ \"asn\": \"AS%u\", \"prefix\": \"%s\", "
> + if (fprintf(out, "\t\t{ \"asn\": %u, \"prefix\": \"%s\", "
>   "\"maxLength\": %u, \"ta\": \"%s\" }",
>   v->asid, buf, v->maxlength, v->tal) < 0)
>   return -1;
> 



Re: simplify the openrsync uploader

2021-05-05 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.05.05 17:53:20 +0200:
> The rsync uploader (what is the generator in rsync) can be simplified and
> cleaned up a fair bit.
> 
> There is some confusion of non-blocking IO on regular files and the idea
> to poll() between openat() and fstat(). This is all not needed and
> therefor a lot of the code handling files can be moved into pre_file.
> This also removes the UPLOAD_READ_LOCAL state since it is no longer
> needed.
> 
> As a little extra gift this diff also plugs a mem leak in rsync_uploader.
> The mbuf buffer was not freed and so for every file being checksummed it
> would leak one of those.

see below about that.

Other than that its ok.

> 
> -- 
> :wq Claudio
> 
> Index: uploader.c
> ===
> RCS file: /cvs/src/usr.bin/rsync/uploader.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 uploader.c
> --- uploader.c22 Mar 2021 11:20:04 -  1.24
> +++ uploader.c5 May 2021 15:39:10 -
> @@ -33,8 +33,7 @@
>  
>  enum uploadst {
>   UPLOAD_FIND_NEXT = 0, /* find next to upload to sender */
> - UPLOAD_WRITE_LOCAL, /* wait to write to sender */
> - UPLOAD_READ_LOCAL, /* wait to read from local file */
> + UPLOAD_WRITE, /* wait to write to sender */
>   UPLOAD_FINISHED /* nothing more to do in phase */
>  };
>  
> @@ -180,7 +179,8 @@ pre_link(struct upload *p, struct sess *
>   if (!sess->opts->preserve_links) {
>   WARNX("%s: ignoring symlink", f->path);
>   return 0;
> - } else if (sess->opts->dry_run) {
> + }
> + if (sess->opts->dry_run) {
>   log_link(sess, f);
>   return 0;
>   }
> @@ -282,7 +282,8 @@ pre_dev(struct upload *p, struct sess *s
>   if (!sess->opts->devices || getuid() != 0) {
>   WARNX("skipping non-regular file %s", f->path);
>   return 0;
> - } else if (sess->opts->dry_run) {
> + }
> + if (sess->opts->dry_run) {
>   log_file(sess, f);
>   return 0;
>   }
> @@ -369,7 +370,8 @@ pre_fifo(struct upload *p, struct sess *
>   if (!sess->opts->specials) {
>   WARNX("skipping non-regular file %s", f->path);
>   return 0;
> - } else if (sess->opts->dry_run) {
> + }
> + if (sess->opts->dry_run) {
>   log_file(sess, f);
>   return 0;
>   }
> @@ -444,7 +446,8 @@ pre_sock(struct upload *p, struct sess *
>   if (!sess->opts->specials) {
>   WARNX("skipping non-regular file %s", f->path);
>   return 0;
> - } else if (sess->opts->dry_run) {
> + }
> + if (sess->opts->dry_run) {
>   log_file(sess, f);
>   return 0;
>   }
> @@ -518,7 +521,8 @@ pre_dir(const struct upload *p, struct s
>   if (!sess->opts->recursive) {
>   WARNX("%s: ignoring directory", f->path);
>   return 0;
> - } else if (sess->opts->dry_run) {
> + }
> + if (sess->opts->dry_run) {
>   log_dir(sess, f);
>   return 0;
>   }
> @@ -579,13 +583,14 @@ post_dir(struct sess *sess, const struct
>  
>   if (!sess->opts->recursive)
>   return 1;
> - else if (sess->opts->dry_run)
> + if (sess->opts->dry_run)
>   return 1;
>  
>   if (fstatat(u->rootfd, f->path, , AT_SYMLINK_NOFOLLOW) == -1) {
>   ERR("%s: fstatat", f->path);
>   return 0;
> - } else if (!S_ISDIR(st.st_mode)) {
> + }
> + if (!S_ISDIR(st.st_mode)) {
>   WARNX("%s: not a directory", f->path);
>   return 0;
>   }
> @@ -631,12 +636,13 @@ post_dir(struct sess *sess, const struct
>  
>  /*
>   * Try to open the file at the current index.
> - * If the file does not exist, returns with success.
> + * If the file does not exist, returns with >0.
>   * Return <0 on failure, 0 on success w/nothing to be done, >0 on
>   * success and the file needs attention.
>   */
>  static int
> -pre_file(const struct upload *p, int *filefd, struct sess *sess)
> +pre_file(const struct upload *p, int *filefd, struct stat *st,
> +struct sess *sess)
>  {
>   const struct flist *f;
>  
> @@ -654,19 +660,54 @@ pre_file(const struct upload *p, int *fi
>  
>   /*
>* For non dry-run cases, we'll write the acknowledgement later
> -  * in the rsync_uploader() function because we need to wait for
> -  * the open() call to complete.
> +  * in the rsync_uploader() function.
>* If the call to openat() fails with ENOENT, there's a
> -  * fast-path between here and the write function, so we won't do
> -  * any blocking between now and then.
> +  * fast-path between here and the write function.
>*/
>  
>   *filefd = openat(p->rootfd, f->path,
> - O_RDONLY | O_NOFOLLOW | O_NONBLOCK, 0);
> - if (*filefd != -1 || errno == ENOENT)
> + O_RDONLY | 

Re: openrsync mini cleanup

2021-05-05 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.05.05 17:13:03 +0200:
> Normalize some code.

ok

> 
> -- 
> :wq Claudio
> 
> Index: receiver.c
> ===
> RCS file: /cvs/src/usr.bin/rsync/receiver.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 receiver.c
> --- receiver.c24 Nov 2020 16:54:44 -  1.25
> +++ receiver.c5 May 2021 15:11:38 -
> @@ -356,7 +356,7 @@ rsync_receiver(struct sess *sess, int fd
>*/
>  
>   if (sess->mplex_reads &&
> - (POLLIN & pfd[PFD_SENDER_IN].revents)) {
> + (pfd[PFD_SENDER_IN].revents & POLLIN)) {
>   if (!io_read_flush(sess, fdin)) {
>   ERRX1("io_read_flush");
>   goto out;
> @@ -371,8 +371,8 @@ rsync_receiver(struct sess *sess, int fd
>* is read to mmap.
>*/
>  
> - if ((POLLIN & pfd[PFD_UPLOADER_IN].revents) ||
> - (POLLOUT & pfd[PFD_SENDER_OUT].revents)) {
> + if ((pfd[PFD_UPLOADER_IN].revents & POLLIN) ||
> + (pfd[PFD_SENDER_OUT].revents & POLLOUT)) {
>   c = rsync_uploader(ul,
>   [PFD_UPLOADER_IN].fd,
>   sess, [PFD_SENDER_OUT].fd);
> @@ -391,8 +391,8 @@ rsync_receiver(struct sess *sess, int fd
>* messages, which will otherwise clog up the pipes.
>*/
>  
> - if ((POLLIN & pfd[PFD_SENDER_IN].revents) ||
> - (POLLIN & pfd[PFD_DOWNLOADER_IN].revents)) {
> + if ((pfd[PFD_SENDER_IN].revents & POLLIN) ||
> + (pfd[PFD_DOWNLOADER_IN].revents & POLLIN)) {
>   c = rsync_downloader(dl, sess,
>   [PFD_DOWNLOADER_IN].fd);
>   if (c < 0) {
> @@ -421,10 +421,12 @@ rsync_receiver(struct sess *sess, int fd
>   if (!io_write_int(sess, fdout, -1)) {
>   ERRX1("io_write_int");
>   goto out;
> - } else if (!io_read_int(sess, fdin, )) {
> + }
> + if (!io_read_int(sess, fdin, )) {
>   ERRX1("io_read_int");
>   goto out;
> - } else if (ioerror != -1) {
> + }
> + if (ioerror != -1) {
>   ERRX("expected phase ack");
>   goto out;
>   }
> @@ -445,7 +447,8 @@ rsync_receiver(struct sess *sess, int fd
>   if (!sess_stats_recv(sess, fdin)) {
>   ERRX1("sess_stats_recv");
>   goto out;
> - } else if (!io_write_int(sess, fdout, -1)) {
> + }
> + if (!io_write_int(sess, fdout, -1)) {
>   ERRX1("io_write_int");
>   goto out;
>   }
> 



  1   2   3   4   5   6   7   8   >