Re: sshd_config(5): Use correct path for system-wide known_hosts

2022-04-11 Thread Darren Tucker
On Mon, 11 Apr 2022 at 16:12, Martin Vahlensieck
 wrote:
> The path to the system-wide known_hosts file is /etc/ssh/ssh_known_hosts
> and not /etc/ssh/known_hosts.  See auth2-hostbased.c line 221-223.

Applied, thanks.

-- 
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.



Re: ddb: simplify "machine" command handling

2022-04-11 Thread Jeremie Courreges-Anglas
On Mon, Apr 11 2022, Christian Weisgerber  wrote:
> Christian Weisgerber:
>
>> This will allow constifying the ddb command tables in a subsequent
>> step.
>
> And here's that boring follow-up diff.

sparc64 and riscv64 GENERIC and GENERIC.MP build successfully with this
diff and the previous one, "machine" behaves as expected in ddb(4).

ok jca@

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



Re: rpki-client: extend -f to print TAL details

2022-04-11 Thread Claudio Jeker
On Mon, Apr 11, 2022 at 07:37:51PM +0200, Theo Buehler wrote:
> On Mon, Apr 11, 2022 at 05:11:30PM +, Job Snijders wrote:
> > On Mon, Apr 11, 2022 at 06:46:20PM +0200, Theo Buehler wrote:
> > > Is this base64 blob really useful? The exact same thing is contained in
> > > a more readable fashion (i.e. with line breaks) in the .tal file itself.
> > 
> > OK, cat(1) can also be used indeed :-)
> > 
> > > Apart from that, I'm fine with having something like this. Couple
> > > comments inline
> > 
> > $ rpki-client -f /etc/rpki/ripe.tal
> > File: /etc/rpki/ripe.tal
> > Trust anchor name: ripe
> > Subject key identifier: 
> > E8:55:2B:1F:D6:D1:A4:F7:E4:04:C6:D8:E5:68:0D:1E:BC:16:3F:C3
> 
> I should have pointed this out before... This is 83 characters wide, so
> maybe we should use a line break. Perhaps
> 
>   printf("Subject key identifier:\n\*s%s\n", 7, "", pretty_key_id(ski));
> 
> to align it with the URIs would look nicer.
> 
> I'm ok with either variant but please give claudio a chance to comment.
> 

I'm fine with either version. I think keeping it on one line has benefits
if the output is processed by something afterwards. A third option is to
shorten "Subject key identifier"

-- 
:wq Claudio



Re: rpki-client: extend -f to print TAL details

2022-04-11 Thread Theo Buehler
On Mon, Apr 11, 2022 at 05:11:30PM +, Job Snijders wrote:
> On Mon, Apr 11, 2022 at 06:46:20PM +0200, Theo Buehler wrote:
> > Is this base64 blob really useful? The exact same thing is contained in
> > a more readable fashion (i.e. with line breaks) in the .tal file itself.
> 
> OK, cat(1) can also be used indeed :-)
> 
> > Apart from that, I'm fine with having something like this. Couple
> > comments inline
> 
> $ rpki-client -f /etc/rpki/ripe.tal
> File: /etc/rpki/ripe.tal
> Trust anchor name: ripe
> Subject key identifier: 
> E8:55:2B:1F:D6:D1:A4:F7:E4:04:C6:D8:E5:68:0D:1E:BC:16:3F:C3

I should have pointed this out before... This is 83 characters wide, so
maybe we should use a line break. Perhaps

printf("Subject key identifier:\n\*s%s\n", 7, "", pretty_key_id(ski));

to align it with the URIs would look nicer.

I'm ok with either variant but please give claudio a chance to comment.



Re: refcount btrace

2022-04-11 Thread Martin Pieuchot
On 08/04/22(Fri) 12:16, Alexander Bluhm wrote:
> On Fri, Apr 08, 2022 at 02:39:34AM +, Visa Hankala wrote:
> > On Thu, Apr 07, 2022 at 07:55:11PM +0200, Alexander Bluhm wrote:
> > > On Wed, Mar 23, 2022 at 06:13:27PM +0100, Alexander Bluhm wrote:
> > > > In my opinion tracepoints give insight at minimal cost.  It is worth
> > > > it to have it in GENERIC to make it easy to use.
> > > 
> > > After release I want to revive the btrace of refcounts discussion.
> > > 
> > > As mpi@ mentioned the idea of dt(4) is to have these trace points
> > > in GENERIC kernel.  If you want to hunt a bug, just turn it on.
> > > Refounting is a common place for bugs, leaks can be detected easily.
> > > 
> > > The alternative are some defines that you can compile in and access
> > > from ddb.  This is more work and you would have to implement it for
> > > every recount.
> > > https://marc.info/?l=openbsd-tech=163786435916039=2
> > > 
> > > There is no measuarable performance difference.  dt(4) is written
> > > in a way that is is only one additional branch.  At least my goal
> > > is to add trace points to useful places when we identify them.
> > 
> > DT_INDEX_ENTER() still checks the index first, so it has two branches
> > in practice.
> > 
> > I think dt_tracing should be checked first so that it serves as
> > a gateway to the trace code. Under normal operation, the check's
> > outcome is always the same, which is easy even for simple branch
> > predictors.
> 
> Reordering the check is easy.  Now dt_tracing is first.
> 
> > I have a slight suspicion that dt(4) is now becoming a way to add code
> > that would be otherwise unacceptable. Also, how "durable" are trace
> > points perceived? Is an added trace point an achieved advantage that
> > is difficult to remove even when its utility has diminished? There is
> > a similarity to (ad hoc) debug printfs too.
> 
> As I understand dt(4) it is a replacement for debug printfs.  But
> it has advantages.  I can be turnd on selectively from userland.
> It does not spam the console, but can be processed in userland.  It
> is always there, you don't have to recompile.
> 
> Of course you always have the printf or tracepoint at the worng
> place.  I think people debugging the code should move them to
> the useful places.  Then we may end with generally useful tool.
> A least that is my hope.
> 
> There are obvious places to debug.  We have syscall entry and return.
> And I think reference counting is also generally interesting.

I'm happy if this can help debugging real reference counting issues.  Do
you have a script that could be committed to /usr/share/btrace to show
how to track reference counting using these probes?

> Index: dev/dt/dt_prov_static.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/dev/dt/dt_prov_static.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 dt_prov_static.c
> --- dev/dt/dt_prov_static.c   17 Mar 2022 14:53:59 -  1.13
> +++ dev/dt/dt_prov_static.c   8 Apr 2022 09:40:29 -
> @@ -87,6 +87,12 @@ DT_STATIC_PROBE1(smr, barrier_exit, "int
>  DT_STATIC_PROBE0(smr, wakeup);
>  DT_STATIC_PROBE2(smr, thread, "uint64_t", "uint64_t");
>  
> +/*
> + * reference counting
> + */
> +DT_STATIC_PROBE0(refcnt, none);
> +DT_STATIC_PROBE3(refcnt, inpcb, "void *", "int", "int");
> +DT_STATIC_PROBE3(refcnt, tdb, "void *", "int", "int");
>  
>  /*
>   * List of all static probes
> @@ -127,15 +133,24 @@ struct dt_probe *const dtps_static[] = {
>   &_DT_STATIC_P(smr, barrier_exit),
>   &_DT_STATIC_P(smr, wakeup),
>   &_DT_STATIC_P(smr, thread),
> + /* refcnt */
> + &_DT_STATIC_P(refcnt, none),
> + &_DT_STATIC_P(refcnt, inpcb),
> + &_DT_STATIC_P(refcnt, tdb),
>  };
>  
> +struct dt_probe *const *dtps_index_refcnt;
> +
>  int
>  dt_prov_static_init(void)
>  {
>   int i;
>  
> - for (i = 0; i < nitems(dtps_static); i++)
> + for (i = 0; i < nitems(dtps_static); i++) {
> + if (dtps_static[i] == &_DT_STATIC_P(refcnt, none))
> + dtps_index_refcnt = _static[i];
>   dt_dev_register_probe(dtps_static[i]);
> + }
>  
>   return i;
>  }
> Index: dev/dt/dtvar.h
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/dev/dt/dtvar.h,v
> retrieving revision 1.13
> diff -u -p -r1.13 dtvar.h
> --- dev/dt/dtvar.h27 Feb 2022 10:14:01 -  1.13
> +++ dev/dt/dtvar.h8 Apr 2022 09:42:19 -
> @@ -313,11 +313,30 @@ extern volatile uint32_tdt_tracing; /* 
>  #define  DT_STATIC_ENTER(func, name, args...) do {   
> \
>   extern struct dt_probe _DT_STATIC_P(func, name);\
>   struct dt_probe *dtp = &_DT_STATIC_P(func, name);   \
> - struct dt_provider *dtpv = dtp->dtp_prov;   \
>   \
>   if 

Kill selrecord()

2022-04-11 Thread Martin Pieuchot
Now that poll(2) & select(2) use the kqueue backend under the hood we
can start retiring the old machinery. 

The diff below does not touch driver definitions, however it :

- kills selrecord() & doselwakeup()

- make it obvious that `kern.nselcoll' is now always 0 

- Change all poll/select hooks to return 0

- Kill a seltid check in wsdisplaystart() which is now always true

In a later step we could remove the *_poll() requirement from device
drivers and kill seltrue() & selfalse().

ok?

Index: arch/sparc64/dev/vldcp.c
===
RCS file: /cvs/src/sys/arch/sparc64/dev/vldcp.c,v
retrieving revision 1.22
diff -u -p -r1.22 vldcp.c
--- arch/sparc64/dev/vldcp.c24 Oct 2021 17:05:04 -  1.22
+++ arch/sparc64/dev/vldcp.c11 Apr 2022 16:38:32 -
@@ -581,44 +581,7 @@ vldcpioctl(dev_t dev, u_long cmd, caddr_
 int
 vldcppoll(dev_t dev, int events, struct proc *p)
 {
-   struct vldcp_softc *sc;
-   struct ldc_conn *lc;
-   uint64_t head, tail, state;
-   int revents = 0;
-   int s, err;
-
-   sc = vldcp_lookup(dev);
-   if (sc == NULL)
-   return (POLLERR);
-   lc = >sc_lc;
-
-   s = spltty();
-   if (events & (POLLIN | POLLRDNORM)) {
-   err = hv_ldc_rx_get_state(lc->lc_id, , , );
-
-   if (err == 0 && state == LDC_CHANNEL_UP && head != tail)
-   revents |= events & (POLLIN | POLLRDNORM);
-   }
-   if (events & (POLLOUT | POLLWRNORM)) {
-   err = hv_ldc_tx_get_state(lc->lc_id, , , );
-
-   if (err == 0 && state == LDC_CHANNEL_UP && head != tail)
-   revents |= events & (POLLOUT | POLLWRNORM);
-   }
-   if (revents == 0) {
-   if (events & (POLLIN | POLLRDNORM)) {
-   cbus_intr_setenabled(sc->sc_bustag, sc->sc_rx_ino,
-   INTR_ENABLED);
-   selrecord(p, >sc_rsel);
-   }
-   if (events & (POLLOUT | POLLWRNORM)) {
-   cbus_intr_setenabled(sc->sc_bustag, sc->sc_tx_ino,
-   INTR_ENABLED);
-   selrecord(p, >sc_wsel);
-   }
-   }
-   splx(s);
-   return revents;
+   return 0;
 }
 
 void
Index: dev/audio.c
===
RCS file: /cvs/src/sys/dev/audio.c,v
retrieving revision 1.198
diff -u -p -r1.198 audio.c
--- dev/audio.c 21 Mar 2022 19:22:39 -  1.198
+++ dev/audio.c 11 Apr 2022 16:38:52 -
@@ -2053,17 +2053,7 @@ audio_mixer_read(struct audio_softc *sc,
 int
 audio_mixer_poll(struct audio_softc *sc, int events, struct proc *p)
 {
-   int revents = 0;
-
-   mtx_enter(_lock);
-   if (sc->mix_isopen && sc->mix_pending)
-   revents |= events & (POLLIN | POLLRDNORM);
-   if (revents == 0) {
-   if (events & (POLLIN | POLLRDNORM))
-   selrecord(p, >mix_sel);
-   }
-   mtx_leave(_lock);
-   return revents;
+   return 0;
 }
 
 int
@@ -2101,21 +2091,7 @@ audio_mixer_close(struct audio_softc *sc
 int
 audio_poll(struct audio_softc *sc, int events, struct proc *p)
 {
-   int revents = 0;
-
-   mtx_enter(_lock);
-   if ((sc->mode & AUMODE_RECORD) && sc->rec.used > 0)
-   revents |= events & (POLLIN | POLLRDNORM);
-   if ((sc->mode & AUMODE_PLAY) && sc->play.used < sc->play.len)
-   revents |= events & (POLLOUT | POLLWRNORM);
-   if (revents == 0) {
-   if (events & (POLLIN | POLLRDNORM))
-   selrecord(p, >rec.sel);
-   if (events & (POLLOUT | POLLWRNORM))
-   selrecord(p, >play.sel);
-   }
-   mtx_leave(_lock);
-   return revents;
+   return 0;
 }
 
 int
Index: dev/hotplug.c
===
RCS file: /cvs/src/sys/dev/hotplug.c,v
retrieving revision 1.21
diff -u -p -r1.21 hotplug.c
--- dev/hotplug.c   25 Dec 2020 12:59:52 -  1.21
+++ dev/hotplug.c   11 Apr 2022 16:39:24 -
@@ -183,16 +183,7 @@ hotplugioctl(dev_t dev, u_long cmd, cadd
 int
 hotplugpoll(dev_t dev, int events, struct proc *p)
 {
-   int revents = 0;
-
-   if (events & (POLLIN | POLLRDNORM)) {
-   if (evqueue_count > 0)
-   revents |= events & (POLLIN | POLLRDNORM);
-   else
-   selrecord(p, _sel);
-   }
-
-   return (revents);
+   return (0);
 }
 
 int
Index: dev/midi.c
===
RCS file: /cvs/src/sys/dev/midi.c,v
retrieving revision 1.54
diff -u -p -r1.54 midi.c
--- dev/midi.c  6 Apr 2022 18:59:27 -   1.54
+++ dev/midi.c  11 Apr 2022 16:39:31 -
@@ -334,31 +334,7 @@ done:
 int
 midipoll(dev_t dev, int events, struct proc *p)
 {
-   struct midi_softc *sc;
-   int 

Re: rpki-client: extend -f to print TAL details

2022-04-11 Thread Job Snijders
On Mon, Apr 11, 2022 at 06:46:20PM +0200, Theo Buehler wrote:
> Is this base64 blob really useful? The exact same thing is contained in
> a more readable fashion (i.e. with line breaks) in the .tal file itself.

OK, cat(1) can also be used indeed :-)

> Apart from that, I'm fine with having something like this. Couple
> comments inline

$ rpki-client -f /etc/rpki/ripe.tal
File: /etc/rpki/ripe.tal
Trust anchor name: ripe
Subject key identifier: 
E8:55:2B:1F:D6:D1:A4:F7:E4:04:C6:D8:E5:68:0D:1E:BC:16:3F:C3
Trust anchor locations:
1: https://rpki.ripe.net/ta/ripe-ncc-ta.cer
2: rsync://rpki.ripe.net/ta/ripe-ncc-ta.cer

How about the below?

Index: print.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/print.c,v
retrieving revision 1.6
diff -u -p -r1.6 print.c
--- print.c 21 Mar 2022 10:39:51 -  1.6
+++ print.c 11 Apr 2022 17:08:14 -
@@ -25,6 +25,8 @@
 #include 
 #include 
 
+#include 
+
 #include "extern.h"
 
 static const char *
@@ -62,10 +64,41 @@ time2str(time_t t)
 void
 tal_print(const struct tal *p)
 {
-   size_t   i;
+   char*ski;
+   EVP_PKEY*pk;
+   RSA *r;
+   const unsigned char *der;
+   unsigned char   *rder = NULL;
+   unsigned charmd[SHA_DIGEST_LENGTH];
+   int  rder_len;
+   size_t   i;
+
+   printf("Trust anchor name: %s\n", p->descr);
+
+   der = p->pkey;
+   pk = d2i_PUBKEY(NULL, , p->pkeysz);
+   if (pk == NULL)
+   errx(1, "d2i_PUBKEY failed in %s", __func__);
+
+   r = EVP_PKEY_get0_RSA(pk);
+   if (r == NULL)
+   errx(1, "EVP_PKEY_get0_RSA failed in %s", __func__);
+   if ((rder_len = i2d_RSAPublicKey(r, )) <= 0)
+   errx(1, "i2d_RSAPublicKey failed in %s", __func__);
+
+   if (!EVP_Digest(rder, rder_len, md, NULL, EVP_sha1(), NULL))
+   errx(1, "EVP_Digest failed in %s", __func__);
 
+   ski = hex_encode(md, SHA_DIGEST_LENGTH);
+   printf("Subject key identifier: %s\n", pretty_key_id(ski));
+
+   printf("Trust anchor locations:\n");
for (i = 0; i < p->urisz; i++)
-   printf("%5zu: URI: %s\n", i + 1, p->uri[i]);
+   printf("%5zu: %s\n", i + 1, p->uri[i]);
+
+   EVP_PKEY_free(pk);
+   free(rder);
+   free(ski);
 }
 
 void
Index: rpki-client.8
===
RCS file: /cvs/src/usr.sbin/rpki-client/rpki-client.8,v
retrieving revision 1.57
diff -u -p -r1.57 rpki-client.8
--- rpki-client.8   31 Mar 2022 17:27:31 -  1.57
+++ rpki-client.8   11 Apr 2022 17:08:14 -
@@ -99,7 +99,9 @@ and
 .Fl -address
 flags and connect with rsync-protocol locations.
 .It Fl f Ar
-Validate the
+Decode the
+. Em TAL
+or validate the
 .Em Signed Object
 in
 .Ar file



Re: rpki-client: extend -f to print TAL details

2022-04-11 Thread Theo Buehler
On Mon, Apr 11, 2022 at 04:05:27PM +, Job Snijders wrote:
> Hi,
> 
> This changeset extends rpki-client to print more detail encapsulated
> inside TAL files, of specific interest is printing the Subject Key
> Identifier (SKI) of the Trust Anchor you'd find if you download the
> referenced .cer file. The SPKI is printed as base64 encoded DER.
> 
> Example:
> 
> $ rpki-client -f /etc/rpki/ripe.tal
> File: /etc/rpki/ripe.tal
> Trust anchor name: ripe
> Subject key identifier: 
> E8:55:2B:1F:D6:D1:A4:F7:E4:04:C6:D8:E5:68:0D:1E:BC:16:3F:C3
> Trust anchor locations:
> 1: https://rpki.ripe.net/ta/ripe-ncc-ta.cer
> 2: rsync://rpki.ripe.net/ta/ripe-ncc-ta.cer
> Subject public key information: 
> MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA0URYSGqUz2myBsOzeW1jQ6NsxNvlLMyhWknvnl8NiBCs/T/S2XuNKQNZ+wBZxIgPPV2pFBFeQAvoH/WK83HwA26V2siwm/MY2nKZ+Olw+wlpzlZ1p3Ipj2eNcKrmit8BwBC8xImzuCGaV0jkRB0GZ0hoH6Ml03umLprRsn6v0xOP0+l6Qc1ZHMFVFb385IQ7FQQTcVIxrdeMsoyJq9eMkE6DoclHhF/NlSllXubASQ9KUWqJ0+Ot3QCXr4LXECMfkpkVR2TZT+v5v658bHVs6ZxRD1b6Uk1uQKAyHUbn/tXvP8lrjAibGzVsXDT2L0x4Edx+QdixPgOji3gBMyL2VwIDAQAB

Is this base64 blob really useful? The exact same thing is contained in
a more readable fashion (i.e. with line breaks) in the .tal file itself.

Apart from that, I'm fine with having something like this. Couple
comments inline

> 
> OK?
> 
> Kind regards,
> 
> Job
> 
> Index: print.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/print.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 print.c
> --- print.c   21 Mar 2022 10:39:51 -  1.6
> +++ print.c   11 Apr 2022 16:03:39 -
> @@ -25,6 +25,8 @@
>  #include 
>  #include 
>  
> +#include 
> +
>  #include "extern.h"
>  
>  static const char *
> @@ -62,10 +64,46 @@ time2str(time_t t)
>  void
>  tal_print(const struct tal *p)
>  {
> - size_t   i;
> + char*talpkey, *ski;
> + EVP_PKEY*pk;
> + RSA *r;
> + unsigned char   *der, *rder = NULL;

I'd make der a 'const unsigned char *' and drop the cast in d2i_PUBKEY().

> + unsigned charmd[SHA_DIGEST_LENGTH];
> + int  rder_len;
> + size_t   i;
> +
> + printf("Trust anchor name: %s\n", p->descr);
> +
> + der = p->pkey;
> + pk = d2i_PUBKEY(NULL, (const unsigned char **), p->pkeysz);
> + if (pk == NULL)
> + errx(1, "d2i_PUBKEY failed in %s", __func__);
> +
> + r = EVP_PKEY_get1_RSA(pk);

No need to use get1. Use get0, then it also matches the error message
below. If you change this, drop the RSA_free() at the end.

> + if (r == NULL)
> + errx(1, "EVP_PKEY_get0_RSA failed in %s", __func__);
> + if ((rder_len = i2d_RSAPublicKey(r, )) <= 0)
> + errx(1, "i2d_RSAPublicKey failed in %s", __func__);
> +
> + if (!EVP_Digest(rder, rder_len, md, NULL, EVP_sha1(), NULL))
> + errx(1, "EVP_Digest failed in %s", __func__);
>  
> + ski = hex_encode(md, SHA_DIGEST_LENGTH);
> + printf("Subject key identifier: %s\n", pretty_key_id(ski));
> +
> + printf("Trust anchor locations:\n");
>   for (i = 0; i < p->urisz; i++)
> - printf("%5zu: URI: %s\n", i + 1, p->uri[i]);
> + printf("%5zu: %s\n", i + 1, p->uri[i]);
> +
> + if (base64_encode(p->pkey, p->pkeysz, ) == -1)
> + errx(1, "base64_encode failed in %s", __func__);
> + printf("Subject public key information: %s\n", talpkey);
> +
> + EVP_PKEY_free(pk);
> + RSA_free(r);
> + free(rder);
> + free(ski);
> + free(talpkey);
>  }
>  
>  void
> Index: rpki-client.8
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/rpki-client.8,v
> retrieving revision 1.57
> diff -u -p -r1.57 rpki-client.8
> --- rpki-client.8 31 Mar 2022 17:27:31 -  1.57
> +++ rpki-client.8 11 Apr 2022 16:03:39 -
> @@ -99,7 +99,9 @@ and
>  .Fl -address
>  flags and connect with rsync-protocol locations.
>  .It Fl f Ar
> -Validate the
> +Decode the
> +. Em TAL
> +or validate the
>  .Em Signed Object
>  in
>  .Ar file
> 



rpki-client: extend -f to print TAL details

2022-04-11 Thread Job Snijders
Hi,

This changeset extends rpki-client to print more detail encapsulated
inside TAL files, of specific interest is printing the Subject Key
Identifier (SKI) of the Trust Anchor you'd find if you download the
referenced .cer file. The SPKI is printed as base64 encoded DER.

Example:

$ rpki-client -f /etc/rpki/ripe.tal
File: /etc/rpki/ripe.tal
Trust anchor name: ripe
Subject key identifier: 
E8:55:2B:1F:D6:D1:A4:F7:E4:04:C6:D8:E5:68:0D:1E:BC:16:3F:C3
Trust anchor locations:
1: https://rpki.ripe.net/ta/ripe-ncc-ta.cer
2: rsync://rpki.ripe.net/ta/ripe-ncc-ta.cer
Subject public key information: 
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA0URYSGqUz2myBsOzeW1jQ6NsxNvlLMyhWknvnl8NiBCs/T/S2XuNKQNZ+wBZxIgPPV2pFBFeQAvoH/WK83HwA26V2siwm/MY2nKZ+Olw+wlpzlZ1p3Ipj2eNcKrmit8BwBC8xImzuCGaV0jkRB0GZ0hoH6Ml03umLprRsn6v0xOP0+l6Qc1ZHMFVFb385IQ7FQQTcVIxrdeMsoyJq9eMkE6DoclHhF/NlSllXubASQ9KUWqJ0+Ot3QCXr4LXECMfkpkVR2TZT+v5v658bHVs6ZxRD1b6Uk1uQKAyHUbn/tXvP8lrjAibGzVsXDT2L0x4Edx+QdixPgOji3gBMyL2VwIDAQAB

OK?

Kind regards,

Job

Index: print.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/print.c,v
retrieving revision 1.6
diff -u -p -r1.6 print.c
--- print.c 21 Mar 2022 10:39:51 -  1.6
+++ print.c 11 Apr 2022 16:03:39 -
@@ -25,6 +25,8 @@
 #include 
 #include 
 
+#include 
+
 #include "extern.h"
 
 static const char *
@@ -62,10 +64,46 @@ time2str(time_t t)
 void
 tal_print(const struct tal *p)
 {
-   size_t   i;
+   char*talpkey, *ski;
+   EVP_PKEY*pk;
+   RSA *r;
+   unsigned char   *der, *rder = NULL;
+   unsigned charmd[SHA_DIGEST_LENGTH];
+   int  rder_len;
+   size_t   i;
+
+   printf("Trust anchor name: %s\n", p->descr);
+
+   der = p->pkey;
+   pk = d2i_PUBKEY(NULL, (const unsigned char **), p->pkeysz);
+   if (pk == NULL)
+   errx(1, "d2i_PUBKEY failed in %s", __func__);
+
+   r = EVP_PKEY_get1_RSA(pk);
+   if (r == NULL)
+   errx(1, "EVP_PKEY_get0_RSA failed in %s", __func__);
+   if ((rder_len = i2d_RSAPublicKey(r, )) <= 0)
+   errx(1, "i2d_RSAPublicKey failed in %s", __func__);
+
+   if (!EVP_Digest(rder, rder_len, md, NULL, EVP_sha1(), NULL))
+   errx(1, "EVP_Digest failed in %s", __func__);
 
+   ski = hex_encode(md, SHA_DIGEST_LENGTH);
+   printf("Subject key identifier: %s\n", pretty_key_id(ski));
+
+   printf("Trust anchor locations:\n");
for (i = 0; i < p->urisz; i++)
-   printf("%5zu: URI: %s\n", i + 1, p->uri[i]);
+   printf("%5zu: %s\n", i + 1, p->uri[i]);
+
+   if (base64_encode(p->pkey, p->pkeysz, ) == -1)
+   errx(1, "base64_encode failed in %s", __func__);
+   printf("Subject public key information: %s\n", talpkey);
+
+   EVP_PKEY_free(pk);
+   RSA_free(r);
+   free(rder);
+   free(ski);
+   free(talpkey);
 }
 
 void
Index: rpki-client.8
===
RCS file: /cvs/src/usr.sbin/rpki-client/rpki-client.8,v
retrieving revision 1.57
diff -u -p -r1.57 rpki-client.8
--- rpki-client.8   31 Mar 2022 17:27:31 -  1.57
+++ rpki-client.8   11 Apr 2022 16:03:39 -
@@ -99,7 +99,9 @@ and
 .Fl -address
 flags and connect with rsync-protocol locations.
 .It Fl f Ar
-Validate the
+Decode the
+. Em TAL
+or validate the
 .Em Signed Object
 in
 .Ar file



Re: rpki-client refactor process startup

2022-04-11 Thread Theo Buehler
On Mon, Apr 11, 2022 at 04:43:33PM +0200, Claudio Jeker wrote:
> rpki-client starts a few processes and it can do this a bit more elegant
> by factoring the common code out into process_start(). This makes the code
> in main a fair bit shorter.
> 
> I decided to move all pledge calles into the individual processes. In my
> opinion there is little benefit in keeping them in main.

ok

It is not really necessary to introduce the additional 'fd' variable in
main(). You could pass in the appropriate fd variable directly

procpid = process_start("parser", );
if (procpid == 0) {
proc_parser(proc);
errx(...)
}

etc and save a few more lines. Not sure if that's clearer or worth it.



rpki-client refactor process startup

2022-04-11 Thread Claudio Jeker
rpki-client starts a few processes and it can do this a bit more elegant
by factoring the common code out into process_start(). This makes the code
in main a fair bit shorter.

I decided to move all pledge calles into the individual processes. In my
opinion there is little benefit in keeping them in main.
-- 
:wq Claudio

Index: http.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/http.c,v
retrieving revision 1.54
diff -u -p -r1.54 http.c
--- http.c  11 Mar 2022 09:57:54 -  1.54
+++ http.c  11 Apr 2022 14:04:57 -
@@ -1773,6 +1773,9 @@ proc_http(char *bind_addr, int fd)
struct http_request *req, *nr;
struct ibuf *b, *inbuf = NULL;
 
+   if (pledge("stdio rpath inet dns recvfd", NULL) == -1)
+   err(1, "pledge");
+
if (bind_addr != NULL) {
struct addrinfo hints, *res;
 
Index: main.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
retrieving revision 1.192
diff -u -p -r1.192 main.c
--- main.c  4 Apr 2022 16:02:54 -   1.192
+++ main.c  11 Apr 2022 14:28:51 -
@@ -703,6 +703,34 @@ check_fs_size(int fd, const char *cached
}
 }
 
+static pid_t
+process_start(const char *title, int *fd)
+{
+   int  fl = SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK;
+   pid_tpid;
+   int  pair[2];
+
+   if (socketpair(AF_UNIX, fl, 0, pair) == -1)
+   err(1, "socketpair");
+   if ((pid = fork()) == -1)
+   err(1, "fork");
+
+   if (pid == 0) {
+   setproctitle("%s", title);
+   /* change working directory to the cache directory */
+   if (fchdir(cachefd) == -1)
+   err(1, "fchdir");
+   if (timeout)
+   alarm(timeout);
+   close(pair[1]);
+   *fd = pair[0];
+   } else {
+   close(pair[0]);
+   *fd = pair[1];
+   }
+   return pid;
+}
+
 void
 suicide(int sig __attribute__((unused)))
 {
@@ -714,11 +742,9 @@ suicide(int sig __attribute__((unused)))
 int
 main(int argc, char *argv[])
 {
-   int  rc, c, st, proc, rsync, http, rrdp, hangup = 0;
-   int  fl = SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK;
+   int  fd, rc, c, st, proc, rsync, http, rrdp, hangup = 0;
size_t   i;
pid_tpid, procpid, rsyncpid, httppid, rrdppid;
-   int  fd[2];
struct pollfdpfd[NPFD];
struct msgbuf   *queues[NPFD];
struct ibuf *b, *httpbuf = NULL, *procbuf = NULL;
@@ -869,33 +895,12 @@ main(int argc, char *argv[])
 * manifests, certificates, etc.) and returning contents.
 */
 
-   if (socketpair(AF_UNIX, fl, 0, fd) == -1)
-   err(1, "socketpair");
-   if ((procpid = fork()) == -1)
-   err(1, "fork");
-
+   procpid = process_start("parser", );
if (procpid == 0) {
-   close(fd[1]);
-
-   setproctitle("parser");
-   /* change working directory to the cache directory */
-   if (fchdir(cachefd) == -1)
-   err(1, "fchdir");
-
-   if (timeout)
-   alarm(timeout);
-
-   /* Only allow access to the cache directory. */
-   if (unveil(".", "r") == -1)
-   err(1, "%s: unveil", cachedir);
-   if (pledge("stdio rpath", NULL) == -1)
-   err(1, "pledge");
-   proc_parser(fd[0]);
+   proc_parser(fd);
errx(1, "parser process returned");
}
-
-   close(fd[0]);
-   proc = fd[1];
+   proc = fd;
 
/*
 * Create a process that will do the rsync'ing.
@@ -905,32 +910,13 @@ main(int argc, char *argv[])
 */
 
if (!noop) {
-   if (socketpair(AF_UNIX, fl, 0, fd) == -1)
-   err(1, "socketpair");
-   if ((rsyncpid = fork()) == -1)
-   err(1, "fork");
-
+   rsyncpid = process_start("rsync", );
if (rsyncpid == 0) {
close(proc);
-   close(fd[1]);
-
-   setproctitle("rsync");
-   /* change working directory to the cache directory */
-   if (fchdir(cachefd) == -1)
-   err(1, "fchdir");
-
-   if (timeout)
-   alarm(timeout);
-
-   if (pledge("stdio rpath proc exec unveil", NULL) == -1)
-   err(1, "pledge");
-
-   proc_rsync(rsync_prog, bind_addr, fd[0]);
+   proc_rsync(rsync_prog, bind_addr, fd);
errx(1, "rsync process 

Re: ddb: simplify "machine" command handling

2022-04-11 Thread Christian Weisgerber
Christian Weisgerber:

> This will allow constifying the ddb command tables in a subsequent
> step.

And here's that boring follow-up diff.


M  sys/arch/alpha/alpha/db_interface.c
M  sys/arch/amd64/amd64/db_interface.c
M  sys/arch/arm/arm/db_interface.c
M  sys/arch/arm64/arm64/db_interface.c
M  sys/arch/i386/i386/db_interface.c
M  sys/arch/m88k/m88k/db_interface.c
M  sys/arch/mips64/mips64/db_machdep.c
M  sys/arch/powerpc/ddb/db_interface.c
M  sys/arch/powerpc64/powerpc64/db_interface.c
M  sys/arch/riscv64/riscv64/db_interface.c
M  sys/arch/sh/sh/db_interface.c
M  sys/arch/sparc64/sparc64/db_interface.c
M  sys/ddb/db_command.c
M  sys/ddb/db_command.h

diff c5fcb00219e59f5bbd0cb5171af7fe5d01f0a0c3 
6e95eec92a4f984cd0aff5dd99dd725920c1f1b6
blob - 7e9ce6cc9803586260886d5c16ff2e1219cc2c77
blob + 3f5056966aa3b44d3d4cdc1113256fc05c528026
--- sys/arch/alpha/alpha/db_interface.c
+++ sys/arch/alpha/alpha/db_interface.c
@@ -85,7 +85,7 @@ db_regs_t ddb_regs;
 void   db_mach_cpu(db_expr_t, int, db_expr_t, char *);
 #endif
 
-struct db_command db_machine_command_table[] = {
+const struct db_command db_machine_command_table[] = {
 #if defined(MULTIPROCESSOR)
{ "ddbcpu", db_mach_cpu,0,  NULL },
 #endif
blob - b12b9c48cf147c1525c19e11fac84b35b02bc19f
blob + bd57af87e871b65736c5f4afa8c4f3f98b9bef5c
--- sys/arch/amd64/amd64/db_interface.c
+++ sys/arch/amd64/amd64/db_interface.c
@@ -394,7 +394,7 @@ x86_ipi_db(struct cpu_info *ci)
 #endif /* MULTIPROCESSOR */
 
 #if NACPI > 0
-struct db_command db_acpi_cmds[] = {
+const struct db_command db_acpi_cmds[] = {
{ "disasm", db_acpi_disasm, CS_OWN, NULL },
{ "showval",db_acpi_showval,CS_OWN, NULL },
{ "tree",   db_acpi_tree,   0,  NULL },
@@ -403,7 +403,7 @@ struct db_command db_acpi_cmds[] = {
 };
 #endif /* NACPI > 0 */
 
-struct db_command db_machine_command_table[] = {
+const struct db_command db_machine_command_table[] = {
 #ifdef MULTIPROCESSOR
{ "cpuinfo",db_cpuinfo_cmd, 0,  0 },
{ "startcpu",   db_startproc_cmd,   0,  0 },
blob - 719da88249590509dabb734794daedc7c2a3dd61
blob + bcc0cad7ce3f7c7c180b99d7aaa43b9d6c554738
--- sys/arch/arm/arm/db_interface.c
+++ sys/arch/arm/arm/db_interface.c
@@ -340,7 +340,7 @@ db_enter(void)
asm(".word  0xe7ff");
 }
 
-struct db_command db_machine_command_table[] = {
+const struct db_command db_machine_command_table[] = {
{ "frame",  db_show_frame_cmd,  0, NULL },
 #ifdef ARM32_DB_COMMANDS
ARM32_DB_COMMANDS,
blob - 1645b49186bed36130ffd73a933e45f8f9fc5e18
blob + cb7ecd25005541d590f94cf07152154d3340ade5
--- sys/arch/arm64/arm64/db_interface.c
+++ sys/arch/arm64/arm64/db_interface.c
@@ -469,7 +469,7 @@ db_stopcpu(int cpu)
 }
 #endif
 
-struct db_command db_machine_command_table[] = {
+const struct db_command db_machine_command_table[] = {
 #ifdef MULTIPROCESSOR
{ "cpuinfo",db_cpuinfo_cmd, 0,  NULL },
{ "startcpu",   db_startproc_cmd,   0,  NULL },
blob - 1807bc890744e4f32ef839e73e5647fd5488b901
blob + be2562e3a816dc0d8579275de6fc373458f5fb3e
--- sys/arch/i386/i386/db_interface.c
+++ sys/arch/i386/i386/db_interface.c
@@ -314,7 +314,7 @@ db_ddbproc_cmd(db_expr_t addr, int have_addr, db_expr_
 #endif /* MULTIPROCESSOR */
 
 #if NACPI > 0
-struct db_command db_acpi_cmds[] = {
+const struct db_command db_acpi_cmds[] = {
{ "disasm", db_acpi_disasm, CS_OWN, NULL },
{ "showval",db_acpi_showval,CS_OWN, NULL },
{ "tree",   db_acpi_tree,   0,  NULL },
@@ -323,7 +323,7 @@ struct db_command db_acpi_cmds[] = {
 };
 #endif /* NACPI > 0 */
 
-struct db_command db_machine_command_table[] = {
+const struct db_command db_machine_command_table[] = {
{ "sysregs",db_sysregs_cmd, 0,  0 },
 #ifdef MULTIPROCESSOR
{ "cpuinfo",db_cpuinfo_cmd, 0,  0 },
blob - 55d5e5ba78719c954c4d6d1f5ffa9083e941a932
blob + eab0352da587873642a701debbbc0c00e8f19052
--- sys/arch/m88k/m88k/db_interface.c
+++ sys/arch/m88k/m88k/db_interface.c
@@ -654,7 +654,7 @@ m88k_db_cpu_cmd(db_expr_t addr, int have_addr, db_expr
 /* COMMAND TABLE / INIT */
 //
 
-struct db_command db_machine_command_table[] = {
+const struct db_command db_machine_command_table[] = {
 #ifdef MULTIPROCESSOR
{ "ddbcpu", m88k_db_cpu_cmd,0,  NULL },
 #endif
blob - 2c7d67c597c2a237420af198985aaa20781f124e
blob + b5f2651c9dccdc3281ea43496bc68a8a86a73d26
--- sys/arch/mips64/mips64/db_machdep.c
+++ sys/arch/mips64/mips64/db_machdep.c
@@ -492,7 +492,7 @@ db_dump_tlb_cmd(db_expr_t addr, int have_addr, db_expr
 }
 
 
-struct db_command db_machine_command_table[] = {
+const struct db_command db_machine_command_table[] = {
{ "tlb",db_dump_tlb_cmd,0,  NULL },
{ "trap",   db_trap_trace_cmd,  0,  NULL },
 #ifdef MULTIPROCESSOR
blob - 

Re: rpki-client: simplify SIA parsing

2022-04-11 Thread Claudio Jeker
On Mon, Apr 11, 2022 at 11:37:11AM +0200, Theo Buehler wrote:
> This should be the last step. It inlines sbgp_sia_resource_entry() into
> sbgp_sia() and dedups the sbgp_sia_resource_{notify,mft,carepo}() using
> a new sbgp_sia_location(). Move the GEN_URI check to sbgp_sia_location()
> since that seems cleaner.
> 
> Index: cert.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> retrieving revision 1.65
> diff -u -p -r1.65 cert.c
> --- cert.c11 Apr 2022 08:28:54 -  1.65
> +++ cert.c11 Apr 2022 09:27:53 -
> @@ -125,132 +125,40 @@ sbgp_addr(struct parse *p,
>  }
>  
>  /*
> - * Parse the SIA notify URL, 4.8.8.1.
> - * Returns zero on failure, non-zero on success.
> + * Extract and validate a SIA accessLocation, RFC 6487, 4.8.8 and RFC 8192, 
> 3.2.
> + * Returns 0 on failure and 1 on success.
>   */
>  static int
> -sbgp_sia_resource_notify(struct parse *p, const char *d, size_t dsz)
> +sbgp_sia_location(const char *fn, const char *descr, const char *proto,
> +GENERAL_NAME *location, char **out)
>  {
> - if (p->res->notify != NULL) {
> - warnx("%s: RFC 6487 section 4.8.8: SIA: "
> - "Notify location already specified", p->fn);
> - return 0;
> - }
> -
> - /* Make sure it's a https:// address. */
> - if (!valid_uri(d, dsz, "https://;)) {
> - warnx("%s: RFC 8182 section 3.2: bad Notify URI", p->fn);
> - return 0;
> - }
> + ASN1_IA5STRING  *uri;
>  
> - if ((p->res->notify = strndup(d, dsz)) == NULL)
> - err(1, NULL);
> -
> - return 1;
> -}
> -
> -/*
> - * Parse the SIA manifest, 4.8.8.1.
> - * Returns zero on failure, non-zero on success.
> - */
> -static int
> -sbgp_sia_resource_mft(struct parse *p, const char *d, size_t dsz)
> -{
> - if (p->res->mft != NULL) {
> - warnx("%s: RFC 6487 section 4.8.8: SIA: "
> - "MFT location already specified", p->fn);
> + if (*out != NULL) {
> + warnx("%s: RFC 6487 section 4.8.8: SIA: %s already specified",
> + fn, descr);
>   return 0;
>   }
>  
> - /* Make sure it's an rsync address. */
> - if (!valid_uri(d, dsz, "rsync://")) {
> - warnx("%s: RFC 6487 section 4.8.8: bad MFT location", p->fn);
> + if (location->type != GEN_URI) {
> + warnx("%s: RFC 6487 section 4.8.8: SIA: %s not URI", fn, descr);
>   return 0;
>   }
>  
> - if ((p->res->mft = strndup(d, dsz)) == NULL)
> - err(1, NULL);
> + uri = location->d.uniformResourceIdentifier;
>  
> - return 1;
> -}
> -
> -/*
> - * Parse the SIA manifest, 4.8.8.1.
> - * Returns zero on failure, non-zero on success.
> - */
> -static int
> -sbgp_sia_resource_carepo(struct parse *p, const char *d, size_t dsz)
> -{
> - if (p->res->repo != NULL) {
> - warnx("%s: RFC 6487 section 4.8.8: SIA: "
> - "CA repository already specified", p->fn);
> + if (!valid_uri(uri->data, uri->length, proto)) {
> + warnx("%s: RFC 6487 section 4.8.8: bad %s location", fn, descr);
>   return 0;
>   }
>  
> - /* Make sure it's an rsync:// address. */
> - if (!valid_uri(d, dsz, "rsync://")) {
> - warnx("%s: RFC 6487 section 4.8.8: bad CA repository URI",
> - p->fn);
> - return 0;
> - }
> -
> - if ((p->res->repo = strndup(d, dsz)) == NULL)
> + if ((*out = strndup(uri->data, uri->length)) == NULL)
>   err(1, NULL);
>  
>   return 1;
>  }
>  
>  /*
> - * Parse the SIA entries, 4.8.8.1.
> - * There may be multiple different resources at this location, so throw
> - * out all but the matching resource type. Currently only two entries
> - * are of interest: rpkiManifest and rpkiNotify.
> - * Returns zero on failure, non-zero on success.
> - */
> -static int
> -sbgp_sia_resource_entry(struct parse *p, ACCESS_DESCRIPTION *ad)
> -{
> - ASN1_OBJECT *oid;
> - ASN1_IA5STRING  *uri = NULL;
> - int  rc = 0;
> -
> - if (ad->location->type == GEN_URI)
> - uri = ad->location->d.uniformResourceIdentifier;
> -
> - oid = ad->method;
> -
> - if (OBJ_cmp(oid, carepo_oid) == 0) {
> - if (uri == NULL) {
> - warnx("%s: RFC 6487: 4.8.8.1 caRepository without URI",
> - p->fn);
> - goto out;
> - }
> - if (!sbgp_sia_resource_carepo(p, uri->data, uri->length))
> - goto out;
> - } else if (OBJ_cmp(oid, manifest_oid) == 0) {
> - if (uri == NULL) {
> - warnx("%s: RFC 6487: 4.8.8 SIA manifest without URI",
> - p->fn);
> - goto out;
> - }
> - if (!sbgp_sia_resource_mft(p, uri->data, uri->length))
> -  

Re: rpki-client: simplify SIA parsing

2022-04-11 Thread Theo Buehler
This should be the last step. It inlines sbgp_sia_resource_entry() into
sbgp_sia() and dedups the sbgp_sia_resource_{notify,mft,carepo}() using
a new sbgp_sia_location(). Move the GEN_URI check to sbgp_sia_location()
since that seems cleaner.

Index: cert.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.65
diff -u -p -r1.65 cert.c
--- cert.c  11 Apr 2022 08:28:54 -  1.65
+++ cert.c  11 Apr 2022 09:27:53 -
@@ -125,132 +125,40 @@ sbgp_addr(struct parse *p,
 }
 
 /*
- * Parse the SIA notify URL, 4.8.8.1.
- * Returns zero on failure, non-zero on success.
+ * Extract and validate a SIA accessLocation, RFC 6487, 4.8.8 and RFC 8192, 
3.2.
+ * Returns 0 on failure and 1 on success.
  */
 static int
-sbgp_sia_resource_notify(struct parse *p, const char *d, size_t dsz)
+sbgp_sia_location(const char *fn, const char *descr, const char *proto,
+GENERAL_NAME *location, char **out)
 {
-   if (p->res->notify != NULL) {
-   warnx("%s: RFC 6487 section 4.8.8: SIA: "
-   "Notify location already specified", p->fn);
-   return 0;
-   }
-
-   /* Make sure it's a https:// address. */
-   if (!valid_uri(d, dsz, "https://;)) {
-   warnx("%s: RFC 8182 section 3.2: bad Notify URI", p->fn);
-   return 0;
-   }
+   ASN1_IA5STRING  *uri;
 
-   if ((p->res->notify = strndup(d, dsz)) == NULL)
-   err(1, NULL);
-
-   return 1;
-}
-
-/*
- * Parse the SIA manifest, 4.8.8.1.
- * Returns zero on failure, non-zero on success.
- */
-static int
-sbgp_sia_resource_mft(struct parse *p, const char *d, size_t dsz)
-{
-   if (p->res->mft != NULL) {
-   warnx("%s: RFC 6487 section 4.8.8: SIA: "
-   "MFT location already specified", p->fn);
+   if (*out != NULL) {
+   warnx("%s: RFC 6487 section 4.8.8: SIA: %s already specified",
+   fn, descr);
return 0;
}
 
-   /* Make sure it's an rsync address. */
-   if (!valid_uri(d, dsz, "rsync://")) {
-   warnx("%s: RFC 6487 section 4.8.8: bad MFT location", p->fn);
+   if (location->type != GEN_URI) {
+   warnx("%s: RFC 6487 section 4.8.8: SIA: %s not URI", fn, descr);
return 0;
}
 
-   if ((p->res->mft = strndup(d, dsz)) == NULL)
-   err(1, NULL);
+   uri = location->d.uniformResourceIdentifier;
 
-   return 1;
-}
-
-/*
- * Parse the SIA manifest, 4.8.8.1.
- * Returns zero on failure, non-zero on success.
- */
-static int
-sbgp_sia_resource_carepo(struct parse *p, const char *d, size_t dsz)
-{
-   if (p->res->repo != NULL) {
-   warnx("%s: RFC 6487 section 4.8.8: SIA: "
-   "CA repository already specified", p->fn);
+   if (!valid_uri(uri->data, uri->length, proto)) {
+   warnx("%s: RFC 6487 section 4.8.8: bad %s location", fn, descr);
return 0;
}
 
-   /* Make sure it's an rsync:// address. */
-   if (!valid_uri(d, dsz, "rsync://")) {
-   warnx("%s: RFC 6487 section 4.8.8: bad CA repository URI",
-   p->fn);
-   return 0;
-   }
-
-   if ((p->res->repo = strndup(d, dsz)) == NULL)
+   if ((*out = strndup(uri->data, uri->length)) == NULL)
err(1, NULL);
 
return 1;
 }
 
 /*
- * Parse the SIA entries, 4.8.8.1.
- * There may be multiple different resources at this location, so throw
- * out all but the matching resource type. Currently only two entries
- * are of interest: rpkiManifest and rpkiNotify.
- * Returns zero on failure, non-zero on success.
- */
-static int
-sbgp_sia_resource_entry(struct parse *p, ACCESS_DESCRIPTION *ad)
-{
-   ASN1_OBJECT *oid;
-   ASN1_IA5STRING  *uri = NULL;
-   int  rc = 0;
-
-   if (ad->location->type == GEN_URI)
-   uri = ad->location->d.uniformResourceIdentifier;
-
-   oid = ad->method;
-
-   if (OBJ_cmp(oid, carepo_oid) == 0) {
-   if (uri == NULL) {
-   warnx("%s: RFC 6487: 4.8.8.1 caRepository without URI",
-   p->fn);
-   goto out;
-   }
-   if (!sbgp_sia_resource_carepo(p, uri->data, uri->length))
-   goto out;
-   } else if (OBJ_cmp(oid, manifest_oid) == 0) {
-   if (uri == NULL) {
-   warnx("%s: RFC 6487: 4.8.8 SIA manifest without URI",
-   p->fn);
-   goto out;
-   }
-   if (!sbgp_sia_resource_mft(p, uri->data, uri->length))
-   goto out;
-   } else if (OBJ_cmp(oid, notify_oid) == 0) {
-   if (uri == NULL) {
-   warnx("%s: RFC 6487: 4.8.8 SIA resourceNotify "
-   

Re: rpki-client: simplify SIA parsing

2022-04-11 Thread Claudio Jeker
On Mon, Apr 11, 2022 at 09:41:05AM +0200, Theo Buehler wrote:
> On Sun, Apr 10, 2022 at 12:40:08PM +0200, Claudio Jeker wrote:
> > This is a lot cleaner and indeed an improvement. I think some of the rc
> > handling can also be simplified. The code in sbgp_sia_resource_entry()
> > and sbgp_sia_resource() no longer require cleanup on error so we can just
> > return 0 instead of goto out. It is OK to do this cleanup in a 2nd step
> > (which you probably already planned).
> 
> Yes, I kept the rc dance since it avoids noise in the following steps.
> The ones without cleanup will go away eventually.
> 
> Here's the next step: merge sbgp_sia() and sbgp_sia_resource().  Now
> that both are short and easy, there's no need for a split.
> 
> Also: move the .mft extension check out of sbgp_sia_resource_mft() and
> use rtype_from_file_extension() instead. The next step will dedup
> sbgp_sia_resource_*().
> 
> Index: cert.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> retrieving revision 1.63
> diff -u -p -r1.63 cert.c
> --- cert.c11 Apr 2022 06:47:38 -  1.63
> +++ cert.c11 Apr 2022 07:11:27 -
> @@ -162,18 +162,12 @@ sbgp_sia_resource_mft(struct parse *p, c
>   return 0;
>   }
>  
> - /* Make sure it's an MFT rsync address. */
> + /* Make sure it's an rsync address. */
>   if (!valid_uri(d, dsz, "rsync://")) {
>   warnx("%s: RFC 6487 section 4.8.8: bad MFT location", p->fn);
>   return 0;
>   }
>  
> - if (dsz < 4 || strcasecmp(d + dsz - 4, ".mft") != 0) {
> - warnx("%s: RFC 6487 section 4.8.8: SIA: "
> - "not an MFT file", p->fn);
> - return 0;
> - }
> -
>   if ((p->res->mft = strndup(d, dsz)) == NULL)
>   err(1, NULL);
>  
> @@ -257,15 +251,28 @@ sbgp_sia_resource_entry(struct parse *p,
>  }
>  
>  /*
> - * Multiple locations as defined in RFC 6487, 4.8.8.1.
> + * Parse "Subject Information Access" extension, RFC 6487 4.8.8.
>   * Returns zero on failure, non-zero on success.
>   */
>  static int
> -sbgp_sia_resource(struct parse *p, AUTHORITY_INFO_ACCESS *sia)
> +sbgp_sia(struct parse *p, X509_EXTENSION *ext)
>  {
> + AUTHORITY_INFO_ACCESS   *sia = NULL;
>   ACCESS_DESCRIPTION  *ad;
>   int  i, rc = 0;
>  
> + if (X509_EXTENSION_get_critical(ext)) {
> + warnx("%s: RFC 6487 section 4.8.8: SIA: "
> + "extension not non-critical", p->fn);
> + goto out;
> + }
> +
> + if ((sia = X509V3_EXT_d2i(ext)) == NULL) {
> + cryptowarnx("%s: RFC 6487 section 4.8.8: SIA: "
> + "failed extension parse", p->fn);
> + goto out;
> + }
> +
>   for (i = 0; i < sk_ACCESS_DESCRIPTION_num(sia); i++) {
>   ad = sk_ACCESS_DESCRIPTION_value(sia, i);
>   if (!sbgp_sia_resource_entry(p, ad))
> @@ -285,34 +292,11 @@ sbgp_sia_resource(struct parse *p, AUTHO
>   goto out;
>   }
>  
> - rc = 1;
> - out:
> - return rc;
> -}
> -
> -/*
> - * Parse "Subject Information Access" extension, RFC 6487 4.8.8.
> - * Returns zero on failure, non-zero on success.
> - */
> -static int
> -sbgp_sia(struct parse *p, X509_EXTENSION *ext)
> -{
> - AUTHORITY_INFO_ACCESS   *sia = NULL;
> - int  rc = 0;
> -
> - if (X509_EXTENSION_get_critical(ext)) {
> + if (rtype_from_file_extension(p->res->mft) != RTYPE_MFT) {
>   warnx("%s: RFC 6487 section 4.8.8: SIA: "
> - "extension not non-critical", p->fn);
> - goto out;
> - }
> -
> - if ((sia = X509V3_EXT_d2i(ext)) == NULL) {
> - cryptowarnx("%s: RFC 6487 section 4.8.8: SIA: "
> - "failed extension parse", p->fn);
> + "not an MFT file", p->fn);
>   goto out;
>   }
> - if (!sbgp_sia_resource(p, sia))
> - goto out;
>  
>   rc = 1;
>   out:
> 

OK claudio@

-- 
:wq Claudio



Re: rpki-client: simplify SIA parsing

2022-04-11 Thread Theo Buehler
On Sun, Apr 10, 2022 at 12:40:08PM +0200, Claudio Jeker wrote:
> This is a lot cleaner and indeed an improvement. I think some of the rc
> handling can also be simplified. The code in sbgp_sia_resource_entry()
> and sbgp_sia_resource() no longer require cleanup on error so we can just
> return 0 instead of goto out. It is OK to do this cleanup in a 2nd step
> (which you probably already planned).

Yes, I kept the rc dance since it avoids noise in the following steps.
The ones without cleanup will go away eventually.

Here's the next step: merge sbgp_sia() and sbgp_sia_resource().  Now
that both are short and easy, there's no need for a split.

Also: move the .mft extension check out of sbgp_sia_resource_mft() and
use rtype_from_file_extension() instead. The next step will dedup
sbgp_sia_resource_*().

Index: cert.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.63
diff -u -p -r1.63 cert.c
--- cert.c  11 Apr 2022 06:47:38 -  1.63
+++ cert.c  11 Apr 2022 07:11:27 -
@@ -162,18 +162,12 @@ sbgp_sia_resource_mft(struct parse *p, c
return 0;
}
 
-   /* Make sure it's an MFT rsync address. */
+   /* Make sure it's an rsync address. */
if (!valid_uri(d, dsz, "rsync://")) {
warnx("%s: RFC 6487 section 4.8.8: bad MFT location", p->fn);
return 0;
}
 
-   if (dsz < 4 || strcasecmp(d + dsz - 4, ".mft") != 0) {
-   warnx("%s: RFC 6487 section 4.8.8: SIA: "
-   "not an MFT file", p->fn);
-   return 0;
-   }
-
if ((p->res->mft = strndup(d, dsz)) == NULL)
err(1, NULL);
 
@@ -257,15 +251,28 @@ sbgp_sia_resource_entry(struct parse *p,
 }
 
 /*
- * Multiple locations as defined in RFC 6487, 4.8.8.1.
+ * Parse "Subject Information Access" extension, RFC 6487 4.8.8.
  * Returns zero on failure, non-zero on success.
  */
 static int
-sbgp_sia_resource(struct parse *p, AUTHORITY_INFO_ACCESS *sia)
+sbgp_sia(struct parse *p, X509_EXTENSION *ext)
 {
+   AUTHORITY_INFO_ACCESS   *sia = NULL;
ACCESS_DESCRIPTION  *ad;
int  i, rc = 0;
 
+   if (X509_EXTENSION_get_critical(ext)) {
+   warnx("%s: RFC 6487 section 4.8.8: SIA: "
+   "extension not non-critical", p->fn);
+   goto out;
+   }
+
+   if ((sia = X509V3_EXT_d2i(ext)) == NULL) {
+   cryptowarnx("%s: RFC 6487 section 4.8.8: SIA: "
+   "failed extension parse", p->fn);
+   goto out;
+   }
+
for (i = 0; i < sk_ACCESS_DESCRIPTION_num(sia); i++) {
ad = sk_ACCESS_DESCRIPTION_value(sia, i);
if (!sbgp_sia_resource_entry(p, ad))
@@ -285,34 +292,11 @@ sbgp_sia_resource(struct parse *p, AUTHO
goto out;
}
 
-   rc = 1;
- out:
-   return rc;
-}
-
-/*
- * Parse "Subject Information Access" extension, RFC 6487 4.8.8.
- * Returns zero on failure, non-zero on success.
- */
-static int
-sbgp_sia(struct parse *p, X509_EXTENSION *ext)
-{
-   AUTHORITY_INFO_ACCESS   *sia = NULL;
-   int  rc = 0;
-
-   if (X509_EXTENSION_get_critical(ext)) {
+   if (rtype_from_file_extension(p->res->mft) != RTYPE_MFT) {
warnx("%s: RFC 6487 section 4.8.8: SIA: "
-   "extension not non-critical", p->fn);
-   goto out;
-   }
-
-   if ((sia = X509V3_EXT_d2i(ext)) == NULL) {
-   cryptowarnx("%s: RFC 6487 section 4.8.8: SIA: "
-   "failed extension parse", p->fn);
+   "not an MFT file", p->fn);
goto out;
}
-   if (!sbgp_sia_resource(p, sia))
-   goto out;
 
rc = 1;
  out:



sshd_config(5): Use correct path for system-wide known_hosts

2022-04-11 Thread Martin Vahlensieck
Hi

The path to the system-wide known_hosts file is /etc/ssh/ssh_known_hosts
and not /etc/ssh/known_hosts.  See auth2-hostbased.c line 221-223.

Best,

Martin

Index: sshd_config.5
===
RCS file: /cvs/src/usr.bin/ssh/sshd_config.5,v
retrieving revision 1.340
diff -u -p -r1.340 sshd_config.5
--- sshd_config.5   31 Mar 2022 17:58:44 -  1.340
+++ sshd_config.5   10 Apr 2022 20:35:39 -
@@ -818,7 +818,7 @@ should ignore the user's
 during
 .Cm HostbasedAuthentication
 and use only the system-wide known hosts file
-.Pa /etc/ssh/known_hosts .
+.Pa /etc/ssh/ssh_known_hosts .
 The default is
 .Dq no .
 .It Cm Include