Re: Correctly set SSL error if x509_verify fails

2020-10-25 Thread Bob Beck
On Sun, Oct 25, 2020 at 01:43:10PM -0600, Bob Beck wrote:
> 
> 
> 
> On Fri, Oct 23, 2020 at 09:13:23AM +0200, Theo Buehler wrote:
> > On Thu, Oct 22, 2020 at 08:44:29PM -0700, Jeremy Evans wrote:
> > > I was trying to diagnose a certificate validation failure in Ruby's
> > > openssl extension tests with LibreSSL 3.2.2, and it was made more
> > > difficult because the verification error type was dropped, resulting
> > > in a SSL_R_CERTIFICATE_VERIFY_FAILED error where
> > > SSL_get_verify_result returned X509_V_OK.
> > 
> > Could you perhaps isolate a test case or explain the reproducer in a bit
> > more detail? I tried running ruby 2.7's regress from ports but that
> > always resulted in a SIGABRT (usually while running test_ftp.rb with or
> > without your diff).
> > 
> > With my diff below I once got past this abort and saw this:
> > 
> > OpenSSL::TestSSL#test_verify_result
> > [/usr/ports/pobj/ruby-2.7.2/ruby-2.7.2/test/openssl/utils.rb:279]:
> > exceptions on 1 threads:
> > <19> expected but was
> > <20>.
> > 
> > Did you see <0> here instead?
> > 
> > > I think this patch will fix it.  Compile tested only.  OKs, or is there
> > > a better way to fix it?
> > 
> > This will probably also address the issue with the haproxy test reported
> > on the libressl list:
> > 
> > https://marc.info/?l=libressl=160339671313132=2
> > https://github.com/haproxy/haproxy/issues/916
> > 
> > Regarding your diff, I think setting the error on the store context
> > should not be the responsibility of x509_verify()'s caller. After all,
> > x509_verify() will likely end up being public API.
> > 
> > The below diff should have the same effect as yours.
> 
> This looks ok to me tb@.. and appears to be the correct fix. 
> 

Actually I rescind the ok -> your diff introduces a new problem.

I added regress to catch this problem and it found an issue:
the call into the x509_vfy_check_id can set the xsc->error but
then we don't set the ctx->error and it's sitting still as V_OK
when your diff sets it. and that's bad. 

So this correctly sets the ctx->error coming back from
x509_vfy_check_id so your diff doesn't introduce another regression

you have my ok on this modified version :)  I'll commit
the change to the bettertls regress to catch it once you commit.

Index: x509/x509_verify.c
===
RCS file: /cvs/src/lib/libcrypto/x509/x509_verify.c,v
retrieving revision 1.13
diff -u -p -u -p -r1.13 x509_verify.c
--- x509/x509_verify.c  26 Sep 2020 15:44:06 -  1.13
+++ x509/x509_verify.c  25 Oct 2020 23:58:08 -
@@ -458,8 +458,12 @@ x509_verify_cert_hostname(struct x509_ve
size_t len;
 
if (name == NULL) {
-   if (ctx->xsc != NULL)
-   return x509_vfy_check_id(ctx->xsc);
+   if (ctx->xsc != NULL) {
+   int ret;
+   if ((ret = x509_vfy_check_id(ctx->xsc)) == 0)
+   ctx->error = ctx->xsc->error;
+   return ret;
+   }
return 1;
}
if ((candidate = strdup(name)) == NULL) {
@@ -853,13 +857,13 @@ x509_verify(struct x509_verify_ctx *ctx,
 
if (ctx->roots == NULL || ctx->max_depth == 0) {
ctx->error = X509_V_ERR_INVALID_CALL;
-   return 0;
+   goto err;
}
 
if (ctx->xsc != NULL) {
if (leaf != NULL || name != NULL) {
ctx->error = X509_V_ERR_INVALID_CALL;
-   return 0;
+   goto err;
}
leaf = ctx->xsc->cert;
 
@@ -872,34 +876,34 @@ x509_verify(struct x509_verify_ctx *ctx,
 */
if ((ctx->xsc->chain = sk_X509_new_null()) == NULL) {
ctx->error = X509_V_ERR_OUT_OF_MEM;
-   return 0;
+   goto err;
}
if (!X509_up_ref(leaf)) {
ctx->error = X509_V_ERR_OUT_OF_MEM;
-   return 0;
+   goto err;
}
if (!sk_X509_push(ctx->xsc->chain, leaf)) {
X509_free(leaf);
ctx->error = X509_V_ERR_OUT_OF_MEM;
-   return 0;
+   goto err;
}
ctx->xsc->error_depth = 0;
ctx->xsc->current_cert = leaf;
}
 
if (!x509_verify_cert_valid(ctx, leaf, NULL))
-   return 0;
+   goto

Re: Correctly set SSL error if x509_verify fails

2020-10-25 Thread Bob Beck




On Fri, Oct 23, 2020 at 09:13:23AM +0200, Theo Buehler wrote:
> On Thu, Oct 22, 2020 at 08:44:29PM -0700, Jeremy Evans wrote:
> > I was trying to diagnose a certificate validation failure in Ruby's
> > openssl extension tests with LibreSSL 3.2.2, and it was made more
> > difficult because the verification error type was dropped, resulting
> > in a SSL_R_CERTIFICATE_VERIFY_FAILED error where
> > SSL_get_verify_result returned X509_V_OK.
> 
> Could you perhaps isolate a test case or explain the reproducer in a bit
> more detail? I tried running ruby 2.7's regress from ports but that
> always resulted in a SIGABRT (usually while running test_ftp.rb with or
> without your diff).
> 
> With my diff below I once got past this abort and saw this:
> 
> OpenSSL::TestSSL#test_verify_result
> [/usr/ports/pobj/ruby-2.7.2/ruby-2.7.2/test/openssl/utils.rb:279]:
> exceptions on 1 threads:
> <19> expected but was
> <20>.
> 
> Did you see <0> here instead?
> 
> > I think this patch will fix it.  Compile tested only.  OKs, or is there
> > a better way to fix it?
> 
> This will probably also address the issue with the haproxy test reported
> on the libressl list:
> 
> https://marc.info/?l=libressl=160339671313132=2
> https://github.com/haproxy/haproxy/issues/916
> 
> Regarding your diff, I think setting the error on the store context
> should not be the responsibility of x509_verify()'s caller. After all,
> x509_verify() will likely end up being public API.
> 
> The below diff should have the same effect as yours.

This looks ok to me tb@.. and appears to be the correct fix. 

> 
> > Thanks,
> > Jeremy
> > 
> > Index: x509_vfy.c
> > ===
> > RCS file: /cvs/src/lib/libcrypto/x509/x509_vfy.c,v
> > retrieving revision 1.81
> > diff -u -p -r1.81 x509_vfy.c
> > --- x509_vfy.c  26 Sep 2020 02:06:28 -  1.81
> > +++ x509_vfy.c  23 Oct 2020 03:34:10 -
> > @@ -680,6 +680,9 @@ X509_verify_cert(X509_STORE_CTX *ctx)
> > if ((vctx = x509_verify_ctx_new_from_xsc(ctx, roots)) != NULL) {
> > ctx->error = X509_V_OK; /* Initialize to OK */
> > chain_count = x509_verify(vctx, NULL, NULL);
> > +   if (vctx->error) {
> > +   ctx->error = vctx->error;
> > +   }
> > }
> >  
> > sk_X509_pop_free(roots, X509_free);
> > 
> 
> 
> Index: x509/x509_verify.c
> ===
> RCS file: /var/cvs/src/lib/libcrypto/x509/x509_verify.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 x509_verify.c
> --- x509/x509_verify.c26 Sep 2020 15:44:06 -  1.13
> +++ x509/x509_verify.c23 Oct 2020 05:04:05 -
> @@ -853,13 +853,13 @@ x509_verify(struct x509_verify_ctx *ctx,
>  
>   if (ctx->roots == NULL || ctx->max_depth == 0) {
>   ctx->error = X509_V_ERR_INVALID_CALL;
> - return 0;
> + goto err;
>   }
>  
>   if (ctx->xsc != NULL) {
>   if (leaf != NULL || name != NULL) {
>   ctx->error = X509_V_ERR_INVALID_CALL;
> - return 0;
> + goto err;
>   }
>   leaf = ctx->xsc->cert;
>  
> @@ -872,34 +872,34 @@ x509_verify(struct x509_verify_ctx *ctx,
>*/
>   if ((ctx->xsc->chain = sk_X509_new_null()) == NULL) {
>   ctx->error = X509_V_ERR_OUT_OF_MEM;
> - return 0;
> + goto err;
>   }
>   if (!X509_up_ref(leaf)) {
>   ctx->error = X509_V_ERR_OUT_OF_MEM;
> - return 0;
> + goto err;
>   }
>   if (!sk_X509_push(ctx->xsc->chain, leaf)) {
>   X509_free(leaf);
>   ctx->error = X509_V_ERR_OUT_OF_MEM;
> - return 0;
> + goto err;
>   }
>   ctx->xsc->error_depth = 0;
>   ctx->xsc->current_cert = leaf;
>   }
>  
>   if (!x509_verify_cert_valid(ctx, leaf, NULL))
> - return 0;
> + goto err;
>  
>   if (!x509_verify_cert_hostname(ctx, leaf, name))
> - return 0;
> + goto err;
>  
>   if ((current_chain = x509_verify_chain_new()) == NULL) {
>   ctx->error = X509_V_ERR_OUT_OF_MEM;
> - return 0;
> + goto err;
>   }
>   if (!x509_verify_chain_append(current_chain, leaf, >error)) {
>   x509_verify_chain_free(current_chain);
> - return 0;
> + goto err;
>   }
>   if (x509_verify_ctx_cert_is_root(ctx, leaf))
>   x509_verify_ctx_add_chain(ctx, current_chain);
> @@ -925,4 +925,9 @@ x509_verify(struct x509_verify_ctx *ctx,
>   return ctx->xsc->verify_cb(ctx->chains_count, ctx->xsc);
>   }
>   return (ctx->chains_count);
> +
> + err:
> + if 

Happy 25th Birthday OpenBSD!

2020-10-18 Thread Bob Beck


Yeah, it's just a number. 

But it's been a pretty wild ride. Thanks everyone for 25 years.

-Bob







Re: [PATCH netcat] Only force fd's to -1 once

2020-09-27 Thread Bob Beck
On Sun, Sep 27, 2020 at 02:46:39PM +1000, Duncan Roe wrote:
> The motivation for this is to make debug logs less confusing.

What is this fixing and what behavior are you changing?

> 
> All changed lines have previously demonstrated the problem.
> 
> Signed-off-by: Duncan Roe 
> ---
>  usr.bin/nc/netcat.c | 27 ++-
>  1 file changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/usr.bin/nc/netcat.c b/usr.bin/nc/netcat.c
> index 528dbeea678..b152cfaf635 100644
> --- a/usr.bin/nc/netcat.c
> +++ b/usr.bin/nc/netcat.c
> @@ -1196,7 +1196,7 @@ readwrite(int net_fd, struct tls *tls_ctx)
>   !(pfd[POLL_NETIN].revents & POLLIN))
>   pfd[POLL_NETIN].fd = -1;
>  
> - if (pfd[POLL_NETOUT].revents & POLLHUP) {
> + if ((pfd[POLL_NETOUT].revents & POLLHUP) && pfd[POLL_NETOUT].fd 
> != -1) {
>   if (Nflag)
>   shutdown(pfd[POLL_NETOUT].fd, SHUT_WR);
>   pfd[POLL_NETOUT].fd = -1;
> @@ -1205,14 +1205,14 @@ readwrite(int net_fd, struct tls *tls_ctx)
>   if (pfd[POLL_STDOUT].revents & POLLHUP)
>   pfd[POLL_STDOUT].fd = -1;
>   /* if no net out, stop watching stdin */
> - if (pfd[POLL_NETOUT].fd == -1)
> + if (pfd[POLL_NETOUT].fd == -1 && pfd[POLL_STDIN].fd != -1)
>   pfd[POLL_STDIN].fd = -1;
>   /* if no stdout, stop watching net in */
> - if (pfd[POLL_STDOUT].fd == -1) {
> - if (pfd[POLL_NETIN].fd != -1)
> - shutdown(pfd[POLL_NETIN].fd, SHUT_RD);
> - pfd[POLL_NETIN].fd = -1;
> - }
> + if (pfd[POLL_STDOUT].fd == -1 &&
> + pfd[POLL_NETIN].fd != -1) {
> + shutdown(pfd[POLL_NETIN].fd, SHUT_RD);
> + pfd[POLL_NETIN].fd = -1;
> + }
>  
>   /* try to read from stdin */
>   if (pfd[POLL_STDIN].revents & POLLIN && stdinbufpos < BUFSIZE) {
> @@ -1299,15 +1299,16 @@ readwrite(int net_fd, struct tls *tls_ctx)
>   }
>  
>   /* stdin gone and queue empty? */
> - if (pfd[POLL_STDIN].fd == -1 && stdinbufpos == 0) {
> - if (pfd[POLL_NETOUT].fd != -1 && Nflag)
> - shutdown(pfd[POLL_NETOUT].fd, SHUT_WR);
> + if (pfd[POLL_STDIN].fd == -1 && stdinbufpos == 0 &&
> + pfd[POLL_NETOUT].fd != -1) {
> + if (Nflag)
> + shutdown(pfd[POLL_NETOUT].fd, SHUT_WR);
>   pfd[POLL_NETOUT].fd = -1;
>   }
>   /* net in gone and queue empty? */
> - if (pfd[POLL_NETIN].fd == -1 && netinbufpos == 0) {
> - pfd[POLL_STDOUT].fd = -1;
> - }
> + if (pfd[POLL_NETIN].fd == -1 && netinbufpos == 0 &&
> + pfd[POLL_STDOUT].fd != -1)
> + pfd[POLL_STDOUT].fd = -1;
>   }
>  }
>  
> -- 
> 2.17.5
> 



Re: agentx and clang static analyzer

2020-09-15 Thread Bob Beck
On Tue, Sep 15, 2020 at 11:08:04AM +0200, Martijn van Duren wrote:
> There are 3 things that actually look like valid complaints when running
> clang's static analyzer.
> 
> 1) A dead store in agentx_recv.
> 2) sizeof(ipaddress) intead of sizeof(*ipaddress). Since this is ipv4,
>this is only a problem if sizeof(pointer) is smaller then 4 bytes,
>which can't happen afaik.
> 3) dst does indeed need to be dereffed, but since dstlen and buflen are
>initialized to 0 and srclen is practically always larger then 0
>we're fine. I'm keeping the *dst here as an additional safeguard.
> 
> The rest seem like false positives to me.
> 
> OK?
> 
> martijn@
> 
> Index: agentx.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/agentx.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 agentx.c
> --- agentx.c  14 Sep 2020 11:30:25 -  1.16
> +++ agentx.c  15 Sep 2020 09:05:57 -
> @@ -135,7 +135,6 @@ agentx_recv(struct agentx *ax)
>   header.aph_packetid = agentx_pdutoh32(, u8);
>   u8 += 4;
>   header.aph_plength = agentx_pdutoh32(, u8);
> - u8 += 4;
>  
>   if (header.aph_version != 1) {
>   errno = EPROTO;

ok for this piece above


> Index: subagentx.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/subagentx.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 subagentx.c
> --- subagentx.c   14 Sep 2020 11:30:25 -  1.1
> +++ subagentx.c   15 Sep 2020 09:05:58 -
> @@ -2929,7 +2929,7 @@ getnext:
>   index->sav_idatacomplete = 1;
>   break;
>   case AGENTX_DATA_TYPE_IPADDRESS:
> - ipaddress = calloc(1, sizeof(ipaddress));
> + ipaddress = calloc(1, sizeof(*ipaddress));
>   if (ipaddress == NULL) {
>   subagentx_log_sag_warn(sag,
>   "Failed to bind ipaddress index");

Not ok for this, it's probably correct, but there are other instances of this
in this code and so you need to engage brain, not static analyzer, go fix or
don't fix them all in a separate commit. 


> @@ -3833,7 +3833,7 @@ subagentx_strcat(char **dst, const char 
>   }
>  
>   srclen = strlen(src);
> - if (dst == NULL || dstlen + srclen > buflen) {
> + if (*dst == NULL || dstlen + srclen > buflen) {
>   nbuflen = (((dstlen + srclen) / 512) + 1) * 512;
>   tmp = recallocarray(*dst, buflen, nbuflen, sizeof(*tmp));
>   if (tmp == NULL)
> 
ok for this piece above



Re: acme-client: improve account creation error message

2020-09-14 Thread Bob Beck


But what if I like json and I am already set up to be a hipster and
feed all the untrusted inputs through jq..

(ok beck@)

On Mon, Sep 14, 2020 at 03:37:25PM +0200, Florian Obser wrote:
> not helpful:
> $ doas acme-client $(hostname)
> acme-client: https://api.test4.buypass.no/acme-v02/new-acct: bad HTTP: 400
> 
> vomitting unformated json is not better:
> $ doas acme-client -v $(hostname)
> acme-client: transfer buffer: 
> [{"type":"urn:ietf:params:acme:error:malformed","detail":"Email is a required 
> contact","code":400,"message":"MALFORMED_BAD_REQUEST","details":"HTTP 400 Bad 
> Request"}] (164 bytes)
> 
> let's do this:
> $ doas obj/acme-client -v $(hostname)
> acme-client: Email is a required contact
> 
> OK?
> 
> diff --git extern.h extern.h
> index 529d3350205..364425b0500 100644
> --- extern.h
> +++ extern.h
> @@ -259,6 +259,7 @@ intjson_parse_order(struct jsmnn *, 
> struct order *);
>  int   json_parse_upd_order(struct jsmnn *, struct order *);
>  void  json_free_capaths(struct capaths *);
>  int   json_parse_capaths(struct jsmnn *, struct capaths *);
> +char *json_getstr(struct jsmnn *, const char *);
>  
>  char *json_fmt_newcert(const char *);
>  char *json_fmt_chkacc(void);
> diff --git json.c json.c
> index 61d2631359f..a6762eeb258 100644
> --- json.c
> +++ json.c
> @@ -297,7 +297,7 @@ json_getobj(struct jsmnn *n, const char *name)
>   * that it's the correct type.
>   * Returns NULL on failure.
>   */
> -static char *
> +char *
>  json_getstr(struct jsmnn *n, const char *name)
>  {
>   size_t   i;
> diff --git netproc.c netproc.c
> index 7b8152196d1..05e36897c38 100644
> --- netproc.c
> +++ netproc.c
> @@ -371,15 +371,27 @@ sreq(struct conn *c, const char *addr, int kid, const 
> char *req, char **loc)
>  static int
>  donewacc(struct conn *c, const struct capaths *p)
>  {
> + struct jsmnn*j = NULL;
>   int  rc = 0;
> - char*req;
> + char*req, *detail, *error = NULL;
>   long lc;
>  
>   if ((req = json_fmt_newacc()) == NULL)
>   warnx("json_fmt_newacc");
>   else if ((lc = sreq(c, p->newaccount, 0, req, >kid)) < 0)
>   warnx("%s: bad comm", p->newaccount);
> - else if (lc != 200 && lc != 201)
> + else if (lc == 400) {
> + if ((j = json_parse(c->buf.buf, c->buf.sz)) == NULL)
> + warnx("%s: bad JSON object", p->newaccount);
> + else {
> + detail = json_getstr(j, "detail");
> + if (detail != NULL && stravis(, detail, VIS_SAFE)
> + != -1) {
> + warnx("%s", error);
> + free(error);
> + }
> + }
> + } else if (lc != 200 && lc != 201)
>   warnx("%s: bad HTTP: %ld", p->newaccount, lc);
>   else if (c->buf.buf == NULL || c->buf.sz == 0)
>   warnx("%s: empty response", p->newaccount);
> 
> 
> -- 
> I'm not entirely sure you are real.
> 



Re: dt: add static vfs probes

2020-09-14 Thread Bob Beck


ok beck@

On Mon, Sep 14, 2020 at 12:45:55PM +0200, Jasper Lievisse Adriaanse wrote:
> Hi,
> 
> Whilst analyzing the cleaner I added tracepoints called 'cleaner' and 
> 'bufcache_take' to
> track its behaviour.
> 
> For the sake of symmetry I've added one in bufcache_release() too and moved 
> the assignment
> of 'pages' until after the KASSERT(), following the flow of bufcache_take().
> 
> Sample usage of these probes:
> 
> tracepoint:vfs:bufcache_take {
> printf("bcache_take:%d(%s) flags: 0x%x cache: %d pages: %d\n",
> tid, comm, arg0 , arg1, arg2);
> }
> 
> tracepoint:vfs:bufcache_rel{
> printf("bcache_rel:%d(%s) flags: 0x%x cache: %d pages: %d\n",
> tid, comm, arg0, arg1, arg2);
> }
> 
> tracepoint:vfs:cleaner{
> printf("cleaner:%d(%s) flags: 0x%x pushed: %d lodirtypages: %d, 
> hidirtypages: %d\n",
> tid, comm, arg0, arg1, arg2, arg3);
> }
> 
> OK to commit this?
> 
> Index: dev/dt/dt_prov_static.c
> ===
> RCS file: /cvs/src/sys/dev/dt/dt_prov_static.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 dt_prov_static.c
> --- dev/dt/dt_prov_static.c   13 Sep 2020 14:55:08 -  1.4
> +++ dev/dt/dt_prov_static.c   14 Sep 2020 10:43:43 -
> @@ -58,6 +58,13 @@ DT_STATIC_PROBE3(uvm, map_insert, "vaddr
>  DT_STATIC_PROBE3(uvm, map_remove, "vaddr_t", "vaddr_t", "vm_prot_t");
>  
>  /*
> + * VFS
> + */
> +DT_STATIC_PROBE3(vfs, bufcache_rel, "long", "int", "int64_t");
> +DT_STATIC_PROBE3(vfs, bufcache_take, "long", "int", "int64_t");
> +DT_STATIC_PROBE4(vfs, cleaner, "long", "int", "long", "long");
> +
> +/*
>   * List of all static probes
>   */
>  struct dt_probe *dtps_static[] = {
> @@ -76,6 +83,10 @@ struct dt_probe *dtps_static[] = {
>   &_DT_STATIC_P(uvm, fault),
>   &_DT_STATIC_P(uvm, map_insert),
>   &_DT_STATIC_P(uvm, map_remove),
> + /* VFS */
> + &_DT_STATIC_P(vfs, bufcache_rel),
> + &_DT_STATIC_P(vfs, bufcache_take),
> + &_DT_STATIC_P(vfs, cleaner),
>  };
>  
>  int
> Index: kern/vfs_bio.c
> ===
> RCS file: /cvs/src/sys/kern/vfs_bio.c,v
> retrieving revision 1.202
> diff -u -p -r1.202 vfs_bio.c
> --- kern/vfs_bio.c12 Sep 2020 11:57:24 -  1.202
> +++ kern/vfs_bio.c14 Sep 2020 10:43:43 -
> @@ -57,6 +57,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  /* XXX Should really be in buf.h, but for uvm_constraint_range.. */
> @@ -1209,6 +1210,9 @@ buf_daemon(void *arg)
>   }
>  
>   while ((bp = bufcache_getdirtybuf())) {
> + TRACEPOINT(vfs, cleaner, bp->b_flags, pushed,
> + lodirtypages, hidirtypages);
> +
>   if (UNCLEAN_PAGES < lodirtypages &&
>   bcstats.kvaslots_avail > 2 * RESERVE_SLOTS &&
>   pushed >= 16)
> @@ -1693,6 +1697,9 @@ bufcache_take(struct buf *bp)
>   KASSERT((bp->cache < NUM_CACHES));
>  
>   pages = atop(bp->b_bufsize);
> +
> + TRACEPOINT(vfs, bufcache_take, bp->b_flags, bp->cache, pages);
> +
>   struct bufcache *cache = [bp->cache];
>   if (!ISSET(bp->b_flags, B_DELWRI)) {
>  if (ISSET(bp->b_flags, B_COLD)) {
> @@ -1756,8 +1763,11 @@ bufcache_release(struct buf *bp)
>   int64_t pages;
>   struct bufcache *cache = [bp->cache];
>  
> - pages = atop(bp->b_bufsize);
>   KASSERT(ISSET(bp->b_flags, B_BC));
> + pages = atop(bp->b_bufsize);
> +
> + TRACEPOINT(vfs, bufcache_rel, bp->b_flags, bp->cache, pages);
> +
>   if (fliphigh) {
>   if (ISSET(bp->b_flags, B_DMA) && bp->cache > 0)
>   panic("B_DMA buffer release from cache %d",
> -- 
> jasper
> 



Re: rpki-client cleanup includes

2020-09-12 Thread Bob Beck


ok beck@

On Sat, Sep 12, 2020 at 05:42:39PM +0200, Claudio Jeker wrote:
> extern.h uses stuff from openssl/x509.h so put that include in there
> and remove all the various other openssl includes in other files that
> actually don't need x509 functions.
> 
> -- 
> :wq Claudio
> 
> Index: as.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/as.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 as.c
> --- as.c  27 Nov 2019 17:18:24 -  1.5
> +++ as.c  12 Sep 2020 15:02:20 -
> @@ -25,8 +25,6 @@
>  #include 
>  #include 
>  
> -#include 
> -
>  #include "extern.h"
>  
>  /*
> Index: cert.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 cert.c
> --- cert.c28 Jul 2020 07:35:04 -  1.17
> +++ cert.c12 Sep 2020 15:02:20 -
> @@ -26,7 +26,6 @@
>  #include 
>  #include 
>  
> -#include 
>  #include  /* DIST_POINT */
>  
>  #include "extern.h"
> Index: crl.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/crl.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 crl.c
> --- crl.c 2 Apr 2020 09:16:43 -   1.8
> +++ crl.c 12 Sep 2020 15:02:20 -
> @@ -26,8 +26,6 @@
>  #include 
>  #include 
>  
> -#include 
> -
>  #include "extern.h"
>  
>  X509_CRL *
> Index: extern.h
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.33
> diff -u -p -r1.33 extern.h
> --- extern.h  12 Sep 2020 10:02:01 -  1.33
> +++ extern.h  12 Sep 2020 15:02:20 -
> @@ -20,6 +20,8 @@
>  #include 
>  #include 
>  
> +#include 
> +
>  enum cert_as_type {
>   CERT_AS_ID, /* single identifier */
>   CERT_AS_INHERIT, /* inherit from parent */
> Index: io.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/io.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 io.c
> --- io.c  29 Nov 2019 05:09:50 -  1.8
> +++ io.c  12 Sep 2020 15:02:20 -
> @@ -25,8 +25,6 @@
>  #include 
>  #include 
>  
> -#include 
> -
>  #include "extern.h"
>  
>  void
> Index: ip.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/ip.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 ip.c
> --- ip.c  16 Apr 2020 14:39:44 -  1.12
> +++ ip.c  12 Sep 2020 15:02:20 -
> @@ -25,8 +25,6 @@
>  #include 
>  #include 
>  
> -#include 
> -
>  #include "extern.h"
>  
>  #define   PREFIX_SIZE(x)  (((x) + 7) / 8)
> Index: log.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/log.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 log.c
> --- log.c 29 Nov 2019 05:14:11 -  1.5
> +++ log.c 12 Sep 2020 15:02:20 -
> @@ -21,7 +21,6 @@
>  #include 
>  
>  #include 
> -#include 
>  
>  #include "extern.h"
>  
> Index: mft.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/mft.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 mft.c
> --- mft.c 30 Jun 2020 12:52:44 -  1.15
> +++ mft.c 12 Sep 2020 15:02:20 -
> @@ -24,7 +24,6 @@
>  #include 
>  #include 
>  
> -#include 
>  #include 
>  
>  #include "extern.h"
> Index: output-bgpd.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/output-bgpd.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 output-bgpd.c
> --- output-bgpd.c 28 Apr 2020 13:41:35 -  1.17
> +++ output-bgpd.c 12 Sep 2020 15:02:20 -
> @@ -16,7 +16,6 @@
>   */
>  
>  #include 
> -#include 
>  
>  #include "extern.h"
>  
> Index: output-bird.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/output-bird.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 output-bird.c
> --- output-bird.c 28 Apr 2020 15:03:39 -  1.9
> +++ output-bird.c 12 Sep 2020 15:02:20 -
> @@ -17,7 +17,6 @@
>   */
>  
>  #include 
> -#include 
>  
>  #include "extern.h"
>  
> Index: output-csv.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/output-csv.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 output-csv.c
> --- output-csv.c  28 Apr 2020 13:41:35 -  1.7
> +++ output-csv.c  12 Sep 2020 15:02:20 -
> @@ -16,7 +16,6 @@
>   */
>  
>  #include 
> -#include 
>  
>  #include "extern.h"
>  
> Index: output-json.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/output-json.c,v
> retrieving revision 1.12
> diff -u -p -r1.12 output-json.c
> --- output-json.c 3 May 2020 20:24:02 -   1.12
> 

Re: tmpfs bug in reclaim

2020-07-14 Thread Bob Beck


In the spirit of be careful what sticks to you, 

this has ok beck@


On Mon, Jul 13, 2020 at 11:56:18AM +0200, Gerhard Roth wrote:
> tmpfs_reclaim() has to make sure that the VFS cache has no more
> locks held for the vnode. Else vclean() could panic because v_holdcnt
> is non-zero.
> 
> I know that tmpfs is disabled by default, but it would be nice
> to have this fix in the code base anyway.
> 
> Gerhard
> 
> 
> Index: sys/tmpfs/tmpfs_vnops.c
> ===
> RCS file: /cvs/src/sys/tmpfs/tmpfs_vnops.c,v
> retrieving revision 1.42
> diff -u -p -u -p -r1.42 tmpfs_vnops.c
> --- sys/tmpfs/tmpfs_vnops.c   11 Jun 2020 09:18:43 -  1.42
> +++ sys/tmpfs/tmpfs_vnops.c   13 Jul 2020 09:48:37 -
> @@ -1080,6 +1080,8 @@ tmpfs_reclaim(void *v)
>   racing = TMPFS_NODE_RECLAIMING(node);
>   rw_exit_write(>tn_nlock);
>  
> + cache_purge(vp);
> +
>   /*
>* If inode is not referenced, i.e. no links, then destroy it.
>* Note: if racing - inode is about to get a new vnode, leave it.
> 



Re: Stuck in Needbuf state, trying to understand (6.7)

2020-07-14 Thread Bob Beck
On Mon, Jun 29, 2020 at 03:56:43PM -0400, sven falempin wrote:
> On Mon, Jun 29, 2020 at 12:58 PM sven falempin 
> wrote:
>
> It works in the original problematic setup.
> 
> Will it go to base ?
> 

Yes.

revision 1.201
date: 2020/07/14 06:02:50;  author: beck;  state: Exp;  lines: +9 -3;  
commitid: G6yRUUYskLjLY0oH;
Do not convert the NOCACHE buffers that come from a vnd strategy routine
into more delayed writes if the vnd is mounted from a file on an MNT_ASYNC
filesystem. This prevents a situaiton where the cleaner can not clean
delayed writes out without making more delayed writes, and we end up
waiting for the syncer to spit things occasionaly when it runs.

noticed and reported by sven falempin  on tech,
who validated this fixes his issue.

ok krw@



Re: Stuck in Needbuf state, trying to understand (6.7)

2020-06-29 Thread Bob Beck


> Awesome, thanks!
> 
> I will test that, ASAP,
> do not hesitate to slay dragon,
> i heard the bathing in the blood pool is good for the skin
> 
> Little concern, I did the test without the MFS and ran into issues ,
> anyway i get back to you (or list ?) when i have test report with patched
> kernel

Yes, howver, you didn't tell my what options you had on the filesystem mounted
when you did the test without MFS, because it matters. If you had your 
filesystem
mounted ASYNC it would have exhibited the same behavoir.  the issue is due to 
the
async mount, which MFS does by default, not strictly to do with MFS. 


> 
> Again thanks for helping.
> 
> -- 
> --
> -
> Knowing is not enough; we must apply. Willing is not enough; we must do



Re: Stuck in Needbuf state, trying to understand (6.7)

2020-06-29 Thread Bob Beck
On Sun, Jun 28, 2020 at 12:18:06PM -0400, sven falempin wrote:
> On Sun, Jun 28, 2020 at 2:40 AM Bryan Linton  wrote:
> 
> > On 2020-06-27 19:29:31, Bob Beck  wrote:
> > >
> > > No.
> > >
> > > I know *exactly* what needbuf is but to attempt to diagnose what your
> > > problem is we need exact details. especially:
> > >
> > > 1) The configuration of your system including all the details of the
> > filesystems
> > > you have mounted, all options used, etc.
> > >
> > > 2) The script you are using to generate the problem (Not a paraphrasing
> > of what
> > > you think the script does) What filesystems it is using.
> > >
> >
> > Not the OP, but this problem sounds almost exactly like the bug I
> > reported last year.
> >
> > There is a detailed list of steps I used to reproduce the bug in
> > the following bug report.
> >
> > https://marc.info/?l=openbsd-bugs=156412299418191
> >
> > I was even able to bisect and identify the commit which first
> > caused the breakage for me.
> >
> >
> > ---8<---
> >
> > CVSROOT:/cvs
> > Module name:src
> > Changes by: b...@cvs.openbsd.org2019/05/08 06:40:57
> >
> > Modified files:
> > sys/kern   : vfs_bio.c vfs_biomem.c
> >
> > Log message:
> > Modify the buffer cache to always flip recovered DMA buffers high.
> >
> > This also modifies the backoff logic to only back off what is requested
> > and not a "mimimum" amount. Tested by me, benno@, tedu@ anda ports build
> > by naddy@.
> >
> > ok tedu@
> >
> > ---8<---
> >
> > However, I have since migrated away from using vnd(4)s since I was
> > able to find other solutions that worked for my use cases.  So I
> > may not be able to provide much additional information other than
> > what is contained in the above bug report.
> >
> > --
> > Bryan
> >
> > >
> > >
> >
> >
> Reproduction of BUG.
> 
> 
> # optional
> mkdir /tmpfs
> mount_mfs -o rw -s 2500M swap /tmpfs # i mounted through fstab so this line
> is not tested
> #the bug
> /bin/dd if=/dev/zero of=/tmpfs/img.dd count=0 bs=1 seek=25
> vnconfig vnd3 /tmpfs/img.dd
> printf "a a\n\n\n\nw\nq\n" | disklabel -E vnd3
> newfs vnd3a
> mount /dev/vnd3a /mnt
> cd /tmp && ftp https://cdn.openbsd.org/pub/OpenBSD/6.7/amd64/base67.tgz
> cd /mnt
> #will occur here (the mkdir was ub beedbuf state for a while ...
> for v in 1 2 3 4 5 6 7 8 9; do mkdir /tmp/$v; tar xzvf /tmp/base67.tgz -C
> /mnt/$v; done
> 
> Ready to test patches.
> 
> 

So, your problem is that you have your vnd created in an mfs
filesystem, when I run your test with the vnd backed by a regular
filesystem (withe softdep even) it works fine. 

The trouble happens when your VND has buffers cached in it's
"filesystem" but then is not flushing them out to the underlyin file
(vnode) that you have your vnd backed by.  On normal filesystems this
works fine, since vnd tells the lower layer to not cache the writes
and to do them syncrhonously, to avoid an explosion of delayed writes
and dependencies of buffers. 

The problem happens when we convert syncryonous bwrites to
asynchronous bdwrites if the fileystem is mounted ASYNC, which,
curiously, MFS always is (I don't know why, it doesn't really make any
sense, and I might even look at changing that) All the writes you do
end up being delayed anc chewing up more buffer space. And they are
all tied to one vnode (your image).  once you exhaust the buffer
space, the cleaner runs, but as you have noticed can't clean out your
vnode until the syncer runs (every 60 seconds).  This is why your
thing "takes a long time", and things stall in need buffer. softdep
has deep dark voodoo in it to avoid this problem and therefore when I
use a softdep filesystem instead of an ASYNC filesystem it works. 

Anyway, what's below fixes your issue on my machine. I'm not sure I'm
happy that it's the final fix but it does fix it. there are many other
dragons lurking in here.

Index: sys/kern/vfs_bio.c
===
RCS file: /cvs/src/sys/kern/vfs_bio.c,v
retrieving revision 1.200
diff -u -p -u -p -r1.200 vfs_bio.c
--- sys/kern/vfs_bio.c  29 Apr 2020 02:25:48 -  1.200
+++ sys/kern/vfs_bio.c  29 Jun 2020 15:18:21 -
@@ -706,8 +706,14 @@ bwrite(struct buf *bp)
 */
async = ISSET(bp->b_flags, B_ASYNC);
if (!async && mp && ISSET(mp->mnt_flag, MNT_ASYNC)) {
-   bdwrite(bp);
-   return (0);
+   /*
+* Don't convert writes from VND on async filesystems
+* that already have delayed writes in the upper layer.
+*/
+   if (!ISSET(bp->b_flags, B_NOCACHE)) {
+   bdwrite(bp);
+   return (0);
+   }
}
 
/*



Re: Stuck in Needbuf state, trying to understand (6.7)

2020-06-27 Thread Bob Beck


No. 

I know *exactly* what needbuf is but to attempt to diagnose what your
problem is we need exact details. especially:

1) The configuration of your system including all the details of the filesystems
you have mounted, all options used, etc. 

2) The script you are using to generate the problem (Not a paraphrasing of what
you think the script does) What filesystems it is using. 



On Sat, Jun 27, 2020 at 08:09:18PM -0400, sven falempin wrote:
> On Fri, Jun 26, 2020 at 7:35 PM sven falempin 
> wrote:
> 
> >
> >
> > On Fri, Jun 26, 2020 at 5:22 PM Stuart Henderson 
> > wrote:
> >
> >> On 2020/06/26 15:30, sven falempin wrote:
> >> > behavior confirmed on current.
> >> >
> >> > Once the process stalls,  ( could be anything writing to the vnconfig
> >> disk,
> >> > cp , umount )
> >> > a few other calls like df , or ps, etc may hang, never the same
> >> > sp or mp kernel, reproduced on today's snapshots.
> >>
> >> vnconfig is used as part of "make release", many builds are done every
> >> week using this so it's not a general problem with vnconfig.
> >>
> >> Can you show some commands or a script to trigger the behaviour?
> >>
> >
> > the perl script use the system to call :
> >
> > vnconfig.
> > mount.
> > umount. <- saw hanged
> > cp.<- saw hanged
> > tar.<- saw hanged
> > svn up.<- saw hanged
> > and dd.
> > newfs.
> >
> > really nothing fancy, only stuff writing to disk got stuck.
> >
> > At one point it does a chroot but it never hangs near that , most of the
> > time it hangs before.
> >
> > The script has been used like 1000 times on 6.0 and maybe twice more on
> > 6.4.
> >
> > I have absolutely no idea what the 'needbuf' of top is .
> >
> > the script hangs at random position , always writing into vnconfig.
> >
> > I have no idea how to reproduce outside the perl script , so maybe it is
> > related
> > to some devious perl stdin/stdout buffer .
> >
> > Nevertheless there's like a 5% chance that's the script will work( slowly )
> >
> > Most of the system call are inside a routine to log
> >
> > sub debug_system {
> >   $logger->debug('running: '.join(' ', @_));
> >   return system(@_);
> > }
> >
> > so i can easily put things inside to try to understand the issue.
> >
> > It is really a strange behavior, and the device must be shut down
> > electrically.
> > Something really odd, i run syslogd on a buffer, and syslogc buffer is
> > stuck too
> > when the device stuck (but it supposed to be mostly already allocated
> > memory ).
> >
> > It's really like the vm does not want to give anymore bucket (<- i
> > don't know what i m talking about here,
> > but i looks like that anything that doesn't malloc is ok , computer reply
> > to ping , can do a few things for a while , and then complete
> > hang )
> >
> > I ran the 6.7 release on a VM somewhere and another device with many perl
> > script and they work.
> >
> > Only this fails 95% of the time and is VERY VERY slow when ok.
> > compared to what i saw in /usr/src the vnconfig is big ,  ( forgot to copy
> > df -h  ),
> > like 2GB
> >
> 
> 
> i put ktrace in front of the perl system call
> 
> An di was able to recover a 800MB trace
> 
> $ kdump -f ./trace.out | tail -20
> kdump: realloc: Cannot allocate memory
>  25955 UNKNOWN(1634890859)
>  72466 ? CALL  syscall()
> 
> 
> could that be of some use ?
> 
> 
> -- 
> --
> -
> Knowing is not enough; we must apply. Willing is not enough; we must do



Re: drop addtrust from cert.pem?

2020-06-02 Thread Bob Beck
On Mon, Jun 01, 2020 at 06:04:17PM +0100, Stuart Henderson wrote:
> OK to drop the expired AddTrust cert from cert.pem?

yes, thanks.

> 
> I checked against the firefox set, there are no new/removed certs that
> work with libressl there. There are now two with GENERALIZEDTIME notAfter
> dates from before 2050 that don't work though (I only remember seeing one
> of those when I last looked).. but that is a separate issue.
> 
> /C=EE/O=AS Sertifitseerimiskeskus/CN=EE Certification Centre Root 
> CA/emailAddress=p...@sk.ee
> /C=PL/O=Unizeto Technologies S.A./OU=Certum Certification Authority/CN=Certum 
> Trusted Network CA 2

I suspect these can safely be dropped too.

> 
> 
> Index: cert.pem
> ===
> RCS file: /cvs/src/lib/libcrypto/cert.pem,v
> retrieving revision 1.20
> diff -u -p -r1.20 cert.pem
> --- cert.pem  10 Apr 2020 12:13:17 -  1.20
> +++ cert.pem  1 Jun 2020 16:59:23 -
> @@ -350,58 +350,6 @@ LysRJyU3eExRarDzzFhdFPFqSBX/wge2sY0PjlxQ
>  LnPqZih4zR0Uv6CPLy64Lo7yFIrM6bV8+2ydDKXhlg==
>  -END CERTIFICATE-
>  
> -### AddTrust AB
> -
> -=== /C=SE/O=AddTrust AB/OU=AddTrust External TTP Network/CN=AddTrust 
> External CA Root
> -Certificate:
> -Data:
> -Version: 3 (0x2)
> -Serial Number: 1 (0x1)
> -Signature Algorithm: sha1WithRSAEncryption
> -Validity
> -Not Before: May 30 10:48:38 2000 GMT
> -Not After : May 30 10:48:38 2020 GMT
> -Subject: C=SE, O=AddTrust AB, OU=AddTrust External TTP Network, 
> CN=AddTrust External CA Root
> -X509v3 extensions:
> -X509v3 Subject Key Identifier: 
> -AD:BD:98:7A:34:B4:26:F7:FA:C4:26:54:EF:03:BD:E0:24:CB:54:1A
> -X509v3 Key Usage: 
> -Certificate Sign, CRL Sign
> -X509v3 Basic Constraints: critical
> -CA:TRUE
> -X509v3 Authority Key Identifier: 
> -
> keyid:AD:BD:98:7A:34:B4:26:F7:FA:C4:26:54:EF:03:BD:E0:24:CB:54:1A
> -DirName:/C=SE/O=AddTrust AB/OU=AddTrust External TTP 
> Network/CN=AddTrust External CA Root
> -serial:01
> -
> -SHA1 Fingerprint=02:FA:F3:E2:91:43:54:68:60:78:57:69:4D:F5:E4:5B:68:85:18:68
> -SHA256 
> Fingerprint=68:7F:A4:51:38:22:78:FF:F0:C8:B1:1F:8D:43:D5:76:67:1C:6E:B2:BC:EA:B4:13:FB:83:D9:65:D0:6D:2F:F2
> --BEGIN CERTIFICATE-
> -MIIENjCCAx6gAwIBAgIBATANBgkqhkiG9w0BAQUFADBvMQswCQYDVQQGEwJTRTEU
> -MBIGA1UEChMLQWRkVHJ1c3QgQUIxJjAkBgNVBAsTHUFkZFRydXN0IEV4dGVybmFs
> -IFRUUCBOZXR3b3JrMSIwIAYDVQQDExlBZGRUcnVzdCBFeHRlcm5hbCBDQSBSb290
> -MB4XDTAwMDUzMDEwNDgzOFoXDTIwMDUzMDEwNDgzOFowbzELMAkGA1UEBhMCU0Ux
> -FDASBgNVBAoTC0FkZFRydXN0IEFCMSYwJAYDVQQLEx1BZGRUcnVzdCBFeHRlcm5h
> -bCBUVFAgTmV0d29yazEiMCAGA1UEAxMZQWRkVHJ1c3QgRXh0ZXJuYWwgQ0EgUm9v
> -dDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALf3GjPm8gAELTngTlvt
> -H7xsD821+iO2zt6bETOXpClMfZOfvUq8k+0DGuOPz+VtUFrWlymUWoCwSXrbLpX9
> -uMq/NzgtHj6RQa1wVsfwTz/oMp50ysiQVOnGXw94nZpAPA6sYapeFI+eh6FqUNzX
> -mk6vBbOmcZSccbNQYArHE504B4YCqOmoaSYYkKtMsE8jqzpPhNjfzp/haW+710LX
> -a0Tkx63ubUFfclpxCDezeWWkWaCUN/cALw3CknLa0Dhy2xSoRcRdKn23tNbE7qzN
> -E0S3ySvdQwAl+mG5aWpYIxG3pzOPVnVZ9c0p10a3CitlttNCbxWyuHv77+ldU9U0
> -WicCAwEAAaOB3DCB2TAdBgNVHQ4EFgQUrb2YejS0Jvf6xCZU7wO94CTLVBowCwYD
> -VR0PBAQDAgEGMA8GA1UdEwEB/wQFMAMBAf8wgZkGA1UdIwSBkTCBjoAUrb2YejS0
> -Jvf6xCZU7wO94CTLVBqhc6RxMG8xCzAJBgNVBAYTAlNFMRQwEgYDVQQKEwtBZGRU
> -cnVzdCBBQjEmMCQGA1UECxMdQWRkVHJ1c3QgRXh0ZXJuYWwgVFRQIE5ldHdvcmsx
> -IjAgBgNVBAMTGUFkZFRydXN0IEV4dGVybmFsIENBIFJvb3SCAQEwDQYJKoZIhvcN
> -AQEFBQADggEBALCb4IUlwtYj4g+WBpKdQZic2YR5gdkeWxQHIzZlj7DYd7usQWxH
> -YINRsPkyPef89iYTx4AWpb9a/IfPeHmJIZriTAcKhjW88t5RxNKWt9x+Tu5w/Rw5
> -6wwCURQtjr0W4MHfRnXnJK3s9EK0hZNwEGe6nQY1ShjTK3rMUUKhemPR5ruhxSvC
> -Nr4TDea9Y355e6cJDUCrat2PisP29owaQgVR1EX1n6diIWgVIEM8med8vSTYqZEX
> -c4g/VhsxOBi0cQ+azcgOno4uG+GMmIPLHzHxREzGBHNJdmAPx/i9F4BrLunMTA5a
> -mnkPIAou1Z5jJh5VkpTYghdae9C8x49OhgQ=
> --END CERTIFICATE-
> -
>  ### AffirmTrust
>  
>  === /C=US/O=AffirmTrust/CN=AffirmTrust Commercial
> 



Re: drop addtrust from cert.pem?

2020-06-02 Thread Bob Beck
On Mon, Jun 01, 2020 at 07:17:28PM +0200, Theo Buehler wrote:
> On Mon, Jun 01, 2020 at 06:04:17PM +0100, Stuart Henderson wrote:
> > OK to drop the expired AddTrust cert from cert.pem?
> 
> Thanks for taking care of this (and for checking the firefox set). I see
> no reason to keep it.
> 
> ok
> 
> > I checked against the firefox set, there are no new/removed certs that
> > work with libressl there. There are now two with GENERALIZEDTIME notAfter
> > dates from before 2050 that don't work though (I only remember seeing one
> > of those when I last looked).. but that is a separate issue.
> > 
> > /C=EE/O=AS Sertifitseerimiskeskus/CN=EE Certification Centre Root 
> > CA/emailAddress=p...@sk.ee
> > /C=PL/O=Unizeto Technologies S.A./OU=Certum Certification 
> > Authority/CN=Certum Trusted Network CA 2
> 
> Ugh...




Re: smtpd: make smarthost to use SNI when relaying

2020-05-31 Thread Bob Beck
looks good to me

ok beck@

On Sun, May 31, 2020 at 03:38:00PM +0200, Sebastien Marie wrote:
> Hi,
> 
> updated diff after millert@ and beck@ remarks:
> - use union to collapse in_addr + in6_addr
> - doesn't allocate buffer and directly use s->relay->domain->name
> 
> Thanks.
> -- 
> Sebastien Marie
> 
> 
> diff 73b535ef4537e8454483912fc3420bc304759e96 /home/semarie/repos/openbsd/src
> blob - d384692a0e43de47d645142a6b99e72b7d83b687
> file + usr.sbin/smtpd/mta_session.c
> --- usr.sbin/smtpd/mta_session.c
> +++ usr.sbin/smtpd/mta_session.c
> @@ -26,6 +26,7 @@
>  #include 
>  #include 
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1604,6 +1605,10 @@ mta_cert_init_cb(void *arg, int status, const char *na
>   struct mta_session *s = arg;
>   void *ssl;
>   char *xname = NULL, *xcert = NULL;
> + union {
> + struct in_addr in4;
> + struct in6_addr in6;
> + } addrbuf;
>  
>   if (s->flags & MTA_WAIT)
>   mta_tree_pop(_tls_init, s->id);
> @@ -1623,6 +1628,22 @@ mta_cert_init_cb(void *arg, int status, const char *na
>   free(xcert);
>   if (ssl == NULL)
>   fatal("mta: ssl_mta_init");
> +
> + /*
> +  * RFC4366 (SNI): Literal IPv4 and IPv6 addresses are not
> +  * permitted in "HostName".
> +  */
> + if (s->relay->domain->as_host == 1) {
> + if (inet_pton(AF_INET, s->relay->domain->name, ) != 1 &&
> + inet_pton(AF_INET6, s->relay->domain->name, ) != 1) 
> {
> + log_debug("%016"PRIx64" mta tls setting SNI name=%s",
> + s->id, s->relay->domain->name);
> + if (SSL_set_tlsext_host_name(ssl, 
> s->relay->domain->name) == 0)
> + log_warnx("%016"PRIx64" mta tls setting SNI 
> failed",
> +s->id);
> + }
> + }
> +
>   io_start_tls(s->io, ssl);
>  }
>  
> 



Re: smtpd: make smarthost to use SNI when relaying

2020-05-30 Thread Bob Beck
On Sat, May 30, 2020 at 05:40:43PM +0200, Sebastien Marie wrote:
> Hi,
> 
> I am looking to make smtpd to set SNI (SSL_set_tlsext_host_name) when 
> connecting
> to smarthost when relaying mail.
> 
> After digging a bit in libtls (to stole the right code) and smtpd (to see 
> where
> to put the stolen code), I have the following diff:
> 
> 
> diff 73b535ef4537e8454483912fc3420bc304759e96 /home/semarie/repos/openbsd/src
> blob - d384692a0e43de47d645142a6b99e72b7d83b687
> file + usr.sbin/smtpd/mta_session.c
> --- usr.sbin/smtpd/mta_session.c
> +++ usr.sbin/smtpd/mta_session.c
> @@ -26,6 +26,7 @@
>  #include 
>  #include 
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1604,6 +1605,8 @@ mta_cert_init_cb(void *arg, int status, const char *na
>   struct mta_session *s = arg;
>   void *ssl;
>   char *xname = NULL, *xcert = NULL;
> + struct in_addr addrbuf4;
> + struct in6_addr addrbuf6;
>  
>   if (s->flags & MTA_WAIT)
>   mta_tree_pop(_tls_init, s->id);
> @@ -1623,6 +1626,24 @@ mta_cert_init_cb(void *arg, int status, const char *na
>   free(xcert);
>   if (ssl == NULL)
>   fatal("mta: ssl_mta_init");
> +
> + /*
> +  * RFC4366 (SNI): Literal IPv4 and IPv6 addresses are not
> +  * permitted in "HostName".
> +  */
> + if (s->relay->domain->as_host == 1) {
> + xname = xstrdup(s->relay->domain->name);

This allocation appears to be unnecessary.  I believe you should be
able to simply check with inet_pton, and then use
s->relay->domain->name

On a strictly smtpd front, it seems odd to have somthing called
domain->name possibly being an ip address in text form. It would seem
prudent to keep these things separate as we resolve things. (domain->ip, or
domain->mxip if we have resolved down to that might be better) but
that's a separate issue and larger change.

> + if (inet_pton(AF_INET, xname, ) != 1 &&
> + inet_pton(AF_INET6, xname, ) != 1) {
> + log_info("%016"PRIx64" mta setting SNI name=%s",
> + s->id, xname);
> + if (SSL_set_tlsext_host_name(ssl, xname) == 0)
> + log_warnx("%016"PRIx64" mta setting SNI failed",
> +s->id);
> + }
> + free(xname);
> + }
> +
>   io_start_tls(s->io, ssl);
>  }
>  
> 
> 
> For what I understood:
> 
> mta_cert_init_cb() function is responsable to prepare a connection. the SSL
> initialization (SSL_new() call) occured in ssl_mta_init() which was just 
> called,
> so it seems it is the right place to call SSL_set_tlsext_host_name().
> 
> We just need the hostname to configure it.
> 
> Regarding mta_session structure, relay->domain->as_host is set to 1 when the
> domain is linked to smarthost configuration (or when the mx is ip address I
> think). And in smarthost case, the domain->name is the hostname. For SNI, we 
> are
> excluding ip, so I assume it should copte with domain->name as ip.
> 
> Does someone with better understanding of smtpd code source could confirm the
> approch is right and comment ?
> 
> Please note I have only tested it on simple configuration.
> 
> Thanks.
> -- 
> Sebastien Marie
> 



Re: official ports vs DEBUG_PACKAGES

2020-05-29 Thread Bob Beck
> (iirc python does something strange)

Inconcievable!



Re: official ports vs DEBUG_PACKAGES

2020-05-29 Thread Bob Beck
On Fri, May 29, 2020 at 06:14:44PM +0200, Marc Espie wrote:
> In a trace:
> 
> > > > #3  0x15e48c95459e in WebVfx::shutdown ()
> > > >  at /usr/obj/ports/webvfx-1.2.0/webvfx-1.2.0/webvfx/webvfx.cpp:193
> 
> Now, this is NOT the default location for WRKOBJDIR, but we are shipping
> packages with debug information...
> 
> This could be a bit cumbersome, if in order to debug locally, you have
> to make patch NO_DEPENDS=Yes, and then you figure out you have to mimic the
> "official" setup for ports, which does not even match the default.
> 
> There are about 3 solutions to that:
> - change the bulk machines to /usr/ports/pobj
> - change the ports default to /usr/obj/ports

It's not realpathing is it? correct me if I am wrong but would
making /usr/ports/pobj a symlink to /usr/obj/ports on the bulk 
machines and running them with the default config just "fix" this? 

> - have some magic I don't know in ELF handling that would allow to either
> tweak the default location/introduce ${WRKOBJDIR} in that debug info.
> 



Re: nsd 4.3.1

2020-05-08 Thread Bob Beck



> On May 8, 2020, at 03:00, Stuart Henderson  wrote:
> 
> On 2020/05/08 06:58, Florian Obser wrote:
>> I'm running this for about 2 weeks or so.
>> Tests, OKs?
> 
> Just off to look at a radio link in a church tower that I suspect a pigeon
> may have knocked out of alignment,


This is possibly one of the most English outages I have ever heard of




> I'll put this on some machines when I get
> back, just wanted to comment:
> 
>> - I'm adding RELNOTES, it helps my process of merging upstream
>> - I forgot to update ChangeLog previously, so there are unrelated
>> changes in there as well. I'll first bring in the ChangeLog and
>> RELNOTES changes up to 4.2.4 and then update to 4.3.1 on top.
> 
> Yes please, I found this very helpful when doing Unbound updates.
> 
> I wonder if testers will notice what's different about responses (I think
> it's a good thing).
> 



Recent 'ftplist' changes visible in the installer

2020-04-28 Thread Bob Beck


So, as some of you know the installer hits ftp.openbsd.org during the
install process to query a CGI to provide you with a list of nearby mirrors
and some other useful things. 

I've recently made some changes to modernize and improve this after
the retirement of the GEO:IP modules and increasing fun with using maxmind
without hitting an API server.  While it's true that if you install from
a mirror or hit the cgi, we know where you came from, I'm not super comfortable
handing that information to a third party for no good reason. 

Fortunately with the help of fcambus@, I managed to switch over to
the free dbip databases, which can be installed from ports and kept local
to the machine.  With the help of afresh1@ we managed to get self contained
timezone information as well, based on some earlier work by andy hook.

Which is long way to say, there should be improvements in what
mirrors you are offered in the installer, especially if you are in some of
the more isolated places lacking an openbsd mirror in your country. 

If you can try it out, and you see anything problematic, please let
me know. 

Thanks

-Bob



Re: suggest to run rpki-client hourly

2020-04-13 Thread Bob Beck
On Mon, Apr 13, 2020 at 09:23:23PM -0600, Todd C. Miller wrote:
> On Mon, 13 Apr 2020 20:27:30 -0600, Bob Beck wrote:
> 
> > In my hearts desire I'd love for "R" to be chosen for each line once at 
> > start
> > up. (so in
> > the above example the things are randomly distributed).  but not sure how 
> > har
> > d that is..
> >
> > If it saves code and effort I really think this is only useful for hours 
> > and 
> > minutes
> 
> Here's a version that uses a suggestion from Theo to support ranges
> like "0~30" to mean a random number between 0 and 30 and just a
> bare "~" to mean a random value in the valid range for that field.
> 
> The random values are chosen when the file is parsed which means
> that on reload due to crontab edit they will change.  I was trying
> to avoid that initially but now I don't think it is a big deal.

Like this one plenty.  I think it's ok the values change on reload. 

> 
>  - todd
> 
> Index: usr.sbin/cron/entry.c
> ===
> RCS file: /cvs/src/usr.sbin/cron/entry.c,v
> retrieving revision 1.49
> diff -u -p -u -r1.49 entry.c
> --- usr.sbin/cron/entry.c 13 Jun 2018 11:27:30 -  1.49
> +++ usr.sbin/cron/entry.c 14 Apr 2020 01:52:13 -
> @@ -450,13 +450,30 @@ static int
>  get_range(bitstr_t *bits, int low, int high, const char *names[],
> int ch, FILE *file)
>  {
> - /* range = number | number "-" number [ "/" number ]
> + /* range = number | number* "~" number* | number "-" number ["/" number]
>*/
>  
>   int i, num1, num2, num3;
>  
> + /* XXX - parse ~, X~Y, X~ and ~Y */
> +
> + if (ch == '~') {
> + /* '~' means a random number [high, low]
> +  */
> + num1 = arc4random_uniform(high - low + 1) + low;
> + ch = get_char(file);
> + if (ch == EOF)
> + return (EOF);
> +
> + if (EOF == set_element(bits, low, high, num1)) {
> + unget_char(ch, file);
> + return (EOF);
> + }
> + return (ch);
> + }
> +
>   if (ch == '*') {
> - /* '*' means "first-last" but can still be modified by /step
> + /* '*' means "high-low" but can still be modified by /step
>*/
>   num1 = low;
>   num2 = high;
> @@ -464,30 +481,45 @@ get_range(bitstr_t *bits, int low, int h
>   if (ch == EOF)
>   return (EOF);
>   } else {
> - ch = get_number(, low, names, ch, file, ",- \t\n");
> + ch = get_number(, low, names, ch, file, ",-~ \t\n");
>   if (ch == EOF)
>   return (EOF);
>  
> - if (ch != '-') {
> - /* not a range, it's a single number.
> -  */
> - if (EOF == set_element(bits, low, high, num1)) {
> - unget_char(ch, file);
> - return (EOF);
> - }
> - return (ch);
> - } else {
> - /* eat the dash
> + switch (ch) {
> + case '-':
> + case '~':
> + num3 = ch;
> +
> + /* eat the dash/tilde
>*/
>   ch = get_char(file);
>   if (ch == EOF)
>   return (EOF);
>  
> - /* get the number following the dash
> + /* get the number following the dash/tilde
>*/
>   ch = get_number(, low, names, ch, file, "/, \t\n");
>   if (ch == EOF || num1 > num2)
>   return (EOF);
> +
> + /* if it's a standard range, we're done here.
> +  */
> + if (num3 == '-')
> + break;
> +
> + /* get a random number in the range [num1, num2]
> +  */
> + num3 = num1;
> + num1 = arc4random_uniform(num2 - num3 + 1) + num3;
> + /* FALLTHROUGH */
> + default:
> + /* not a range, it's a single number.
> +  */
> + if (EOF == set_element(bits, low, high, num1)) {
> + unget_char(ch, file);
> + return (EOF);
> + }
> + return (ch);
>   }
>   }
>  
> 



Re: suggest to run rpki-client hourly

2020-04-13 Thread Bob Beck


A couple thoughts: 

1) I'm not sure how useful random months or days of the week are, but for 
consistency maybe?
2) if I do something like

R * * * * /usr/local/bin/thing1
R * * * * /usr/local/bin/thing2
R * * * * /usr/local/bin/thing3
...
R * * * * /usr/local/bin/thing1000

I still have the thundering herd problem a bit, in that all my things fire at 
the same R. 

In my hearts desire I'd love for "R" to be chosen for each line once at 
startup. (so in
the above example the things are randomly distributed).  but not sure how hard 
that is..

If it saves code and effort I really think this is only useful for hours and 
minutes


On Mon, Apr 13, 2020 at 12:54:34PM -0600, Todd C. Miller wrote:
> On Mon, 13 Apr 2020 10:00:52 -0600, Bob Beck wrote:
> 
> > +1000. a new random time chosen at cron start. 
> >
> > We see this all the time, and it would be a lot better than the hacks for 
> > all
> >  the things
> >
> > "R" for random sounds good to me
> 
> How about this?  If you guys like it I'll update the man page too.
> So far only tested with a crontab entry like:
> 
> R * * * * date
> 
>  - todd
> 
> Index: usr.sbin/cron/cron.c
> ===
> RCS file: /cvs/src/usr.sbin/cron/cron.c,v
> retrieving revision 1.78
> diff -u -p -u -r1.78 cron.c
> --- usr.sbin/cron/cron.c  11 Feb 2020 12:42:01 -  1.78
> +++ usr.sbin/cron/cron.c  13 Apr 2020 16:25:44 -
> @@ -129,6 +129,7 @@ main(int argc, char *argv[])
>   syslog(LOG_INFO, "(CRON) STARTUP (%s)", CRON_VERSION);
>   }
>  
> + init_random();
>   load_database();
>   scan_atjobs(_database, NULL);
>   set_time(TRUE);
> Index: usr.sbin/cron/entry.c
> ===
> RCS file: /cvs/src/usr.sbin/cron/entry.c,v
> retrieving revision 1.49
> diff -u -p -u -r1.49 entry.c
> --- usr.sbin/cron/entry.c 13 Jun 2018 11:27:30 -  1.49
> +++ usr.sbin/cron/entry.c 13 Apr 2020 18:49:56 -
> @@ -54,6 +54,12 @@ static const char *ecodes[] = {
>   "out of memory"
>  };
>  
> +typedef  enum r_type {
> + r_minute, r_hour, r_dom, r_month, r_dow
> +} rtype_e;
> +
> +static int rand_vals[5];
> +
>  static const char *MonthNames[] = {
>   "Jan", "Feb", "Mar", "Apr", "May", "Jun",
>   "Jul", "Aug", "Sep", "Oct", "Nov", "Dec",
> @@ -70,6 +76,8 @@ static int  get_list(bitstr_t *, int, int
>   get_number(int *, int, const char *[], int, FILE *, const char 
> *),
>   set_element(bitstr_t *, int, int, int);
>  
> +static int   set_random(bitstr_t *, int, int, FILE *);
> +
>  void
>  free_entry(entry *e)
>  {
> @@ -187,10 +195,15 @@ load_entry(FILE *file, void (*error_func
>   goto eof;
>   }
>   } else {
> - if (ch == '*')
> - e->flags |= MIN_STAR;
> - ch = get_list(e->minute, FIRST_MINUTE, LAST_MINUTE,
> -   NULL, ch, file);
> + if (ch == 'R') {
> + ch = set_random(e->minute, rand_vals[r_minute], ch,
> + file);
> + } else {
> + if (ch == '*')
> + e->flags |= MIN_STAR;
> + ch = get_list(e->minute, FIRST_MINUTE, LAST_MINUTE,
> +   NULL, ch, file);
> + }
>   if (ch == EOF) {
>   ecode = e_minute;
>   goto eof;
> @@ -199,10 +212,14 @@ load_entry(FILE *file, void (*error_func
>   /* hours
>*/
>  
> - if (ch == '*')
> - e->flags |= HR_STAR;
> - ch = get_list(e->hour, FIRST_HOUR, LAST_HOUR,
> -   NULL, ch, file);
> + if (ch == 'R') {
> + ch = set_random(e->hour, rand_vals[r_hour], ch, file);
> + } else {
> + if (ch == '*')
> + e->flags |= HR_STAR;
> + ch = get_list(e->hour, FIRST_HOUR, LAST_HOUR,
> +   NULL, ch, file);
> + }
>   if (ch == EOF) {
>   ecode = e_hour;
>   goto eof;
> @@ -211,10 +228,15 @@ load_entry(FILE *file, void (*error_func
>   /* DOM (days of month)
>*/
>  
> - 

Re: suggest to run rpki-client hourly

2020-04-13 Thread Bob Beck
On Mon, Apr 13, 2020 at 09:56:52AM -0600, Todd C. Miller wrote:
> On Mon, 13 Apr 2020 09:37:14 -0600, "Theo de Raadt" wrote:
> 
> > While I understand what RANDOM is trying to do, I am not a fan.  I've
> > thought often of an improvement, where the minute marker in a crontab
> > file could be a letter 'R', which indicates the specific random value
> > for this cron start.  That value would be selected at cron start, and
> > remain constant for the runtime of cron.
> 
> I was thinking the same thing, but using '?' as the character.  It
> doesn't really matter what we choose, the implementation should be
> straight-forward.

+1000. a new random time chosen at cron start. 

We see this all the time, and it would be a lot better than the hacks for all 
the things

"R" for random sounds good to me

> 
>  - todd
> 



Re: fts and unveil issue

2019-02-03 Thread Bob Beck
yes you are seeing the limitation of 6.4 unveil as mentioned at the bottom
of the man page.   this should be fixed in current

On Sun, Feb 3, 2019 at 03:29 Kristaps Dzonsons  wrote:

> When I unveil(2), fts doesn't behave well.  But only in a subtle way.
> Enclosed is a demonstration.  I found this with openrsync, which unveils
> before using fts_open to scan for files.
>
> When run with a directory with only empty subdirectories or just files,
> this works fine.  But when run with a directory that contains other
> non-empty directories, the fts_read fails in the nested directories.
>
> This is on stock OpenBSD 6.4, syspatched, amd64.
>
> For example, consider the following abridged output (to fit into this
> e-mail window):
>
> % find ~/tmp/test -ls
> drwxr-xr-x3  /home/kristaps/tmp/test
> drwxr-xr-x3  /home/kristaps/tmp/test/test2
> -rw-r--r--1  /home/kristaps/tmp/test/test2/test2
> drwxr-xr-x2  /home/kristaps/tmp/test/test2/test3
> -rw-r--r--1  /home/kristaps/tmp/test/test1
> % gcc -W -Wall -Wextra -g foo.c
> % ./a.out /home/kristaps/tmp/test/
> a.out: /home/kristaps/tmp/test/
> a.out: /home/kristaps/tmp/test/test2
> a.out: /home/kristaps/tmp/test/test2/test2
> a.out: /home/kristaps/tmp/test/test2/test3
> a.out: /home/kristaps/tmp/test/test2/test3
> a.out: /home/kristaps/tmp/test/test2
> a.out: /home/kristaps/tmp/test/test1
> a.out: /home/kristaps/tmp/test/
> a.out: TRYING AGAIN.
> a.out: /home/kristaps/tmp/test/
> a.out: /home/kristaps/tmp/test/test2
> a.out: /home/kristaps/tmp/test/test2/test2: no stat
> a.out: ...but regular stat works
>
> So the first nested child fails (regardless of whether it's a file or
> directory, by the way).  But a regular stat still works.
>
> The same happens if I use unveil("/", "r").
>


Re: unveil spamlogd

2018-10-24 Thread Bob Beck
ok beck@ as well

On Wed, Oct 24, 2018 at 06:13 Todd C. Miller  wrote:

> On Wed, 24 Oct 2018 08:05:11 +0100, Ricardo Mestre wrote:
>
> > The only file that spamlogd needs to access after calling pledge is
> > PATH_SPAMD_DB, so unveil it with O_RDWR permissions.
>
> Looks good.  OK millert@
>
>  - todd
>


Re: Reuse VM ids.

2018-10-08 Thread Bob Beck
works here and I like it.  but probably for after unlock

On Sun, Oct 7, 2018 at 22:11 Mischa Peters  wrote:

> No idea if the code works yet.
> Hopefully I can try later. But love the idea.
>
> Mischa
>
> > On 8 Oct 2018, at 04:31, Ori Bernstein  wrote:
> >
> > Keep a list of known vms, and reuse the VM IDs. This means that when
> using
> > '-L', the IP addresses of the VMs are stable.
> >
> > diff --git usr.sbin/vmd/config.c usr.sbin/vmd/config.c
> > index af12b790002..522bae32501 100644
> > --- usr.sbin/vmd/config.c
> > +++ usr.sbin/vmd/config.c
> > @@ -61,7 +61,10 @@ config_init(struct vmd *env)
> >if (what & CONFIG_VMS) {
> >if ((env->vmd_vms = calloc(1, sizeof(*env->vmd_vms))) == NULL)
> >return (-1);
> > +if ((env->vmd_known = calloc(1, sizeof(*env->vmd_known))) ==
> NULL)
> > +return (-1);
> >TAILQ_INIT(env->vmd_vms);
> > +TAILQ_INIT(env->vmd_known);
> >}
> >if (what & CONFIG_SWITCHES) {
> >if ((env->vmd_switches = calloc(1,
> > diff --git usr.sbin/vmd/vmd.c usr.sbin/vmd/vmd.c
> > index 18a5e0d3d5d..732691b4381 100644
> > --- usr.sbin/vmd/vmd.c
> > +++ usr.sbin/vmd/vmd.c
> > @@ -1169,6 +1169,27 @@ vm_remove(struct vmd_vm *vm, const char *caller)
> >free(vm);
> > }
> >
> > +static uint32_t
> > +claim_vmid(const char *name)
> > +{
> > +struct name2id *n2i = NULL;
> > +
> > +TAILQ_FOREACH(n2i, env->vmd_known, entry)
> > +if (strcmp(n2i->name, name) == 0)
> > +return n2i->id;
> > +
> > +if (++env->vmd_nvm == 0)
> > +fatalx("too many vms");
> > +if ((n2i = calloc(1, sizeof(struct name2id))) == NULL)
> > +fatalx("could not alloc vm name");
> > +n2i->id = env->vmd_nvm;
> > +if (strlcpy(n2i->name, name, sizeof(n2i->name)) >=
> sizeof(n2i->name))
> > +fatalx("overlong vm name");
> > +TAILQ_INSERT_TAIL(env->vmd_known, n2i, entry);
> > +
> > +return n2i->id;
> > +}
> > +
> > int
> > vm_register(struct privsep *ps, struct vmop_create_params *vmc,
> > struct vmd_vm **ret_vm, uint32_t id, uid_t uid)
> > @@ -1300,11 +1321,8 @@ vm_register(struct privsep *ps, struct
> vmop_create_params *vmc,
> >vm->vm_cdrom = -1;
> >vm->vm_iev.ibuf.fd = -1;
> >
> > -if (++env->vmd_nvm == 0)
> > -fatalx("too many vms");
> > -
> >/* Assign a new internal Id if not specified */
> > -vm->vm_vmid = id == 0 ? env->vmd_nvm : id;
> > +vm->vm_vmid = (id == 0) ? claim_vmid(vcp->vcp_name) : id;
> >
> >log_debug("%s: registering vm %d", __func__, vm->vm_vmid);
> >TAILQ_INSERT_TAIL(env->vmd_vms, vm, vm_entry);
> > diff --git usr.sbin/vmd/vmd.h usr.sbin/vmd/vmd.h
> > index b7c012854e8..86fad536e59 100644
> > --- usr.sbin/vmd/vmd.h
> > +++ usr.sbin/vmd/vmd.h
> > @@ -276,6 +276,13 @@ struct vmd_user {
> > };
> > TAILQ_HEAD(userlist, vmd_user);
> >
> > +struct name2id {
> > +charname[VMM_MAX_NAME_LEN];
> > +int32_tid;
> > +TAILQ_ENTRY(name2id)entry;
> > +};
> > +TAILQ_HEAD(name2idlist, name2id);
> > +
> > struct address {
> >struct sockaddr_storage ss;
> >int prefixlen;
> > @@ -300,6 +307,7 @@ struct vmd {
> >
> >uint32_t vmd_nvm;
> >struct vmlist*vmd_vms;
> > +struct name2idlist*vmd_known;
> >uint32_t vmd_nswitches;
> >struct switchlist*vmd_switches;
> >struct userlist*vmd_users;
> >
> > --
> >Ori Bernstein
> >
>
>


Re: openssl s_time: different tally marks for different TLS versions

2018-09-15 Thread Bob Beck
I'm generally opposed to breaking stdout compatibility with the
"openssl" command tools because we have no clue what shell scripts and
other applications this will break.

with a *very good reason* I think it's ok, but this (I think this
looks better) isn't one of them.  the "openssl" command is kept the
way it is *for compatibilityt with crap that wants it*.

If you truly dislike the output - WRITE A NEW TOOL THAT DOESN'T SUCK  ;)


On Sat, Sep 15, 2018 at 1:21 PM Scott Cheloha  wrote:
>
> Bump.
>
> On Tue, Aug 28, 2018 at 10:33:34AM -0500, Scott Cheloha wrote:
> > Two diffs here.
> >
> > First, move the tally mark printing out of the benchmark loop.
> >
> > Second, print '0' for TLS 1.0, '1' for TLS 1.1, etc.
> >
> > This breaks stdout compatibility with OpenSSL s_time, and prior
> > versions of s_time in general, because 't' was used for TLS 1.0
> > (behavior change) and '2' was used for SSLv2 (marker collision).
> >
> > (The choice of a single character as the mark predated any plans
> > for a successor to SSL.  The choice of 't' predated any plans for
> > a revision to TLS.)
> >
> > I think the utility of distinguishing between the various TLS
> > versions at a glance outweighs the value of compatibility with
> > older versions of the software.  Especially given how haphazard
> > the stdout behavior of this code is anyway, I don't think we're
> > going to break a zillion scripts.  The primary utility of this
> > app is interactive testing and eyeballing your performance.
> >
> > But... if this is unacceptable the alternative is to just print
> > 't' for any and all TLS versions.  I think this is less useful,
> > but one can always use s_client, so it isn't the end of the world.
> >
> > Thoughts?  ok?
> >
> > PS. Using DTLS to encrypt HTTP isn't a thing, right?  It isn't
> > useful to check for DTLS1_VERSION from SSL_version(3)?
> >
> > Diff 1:
> >
> > Index: s_time.c
> > ===
> > RCS file: /cvs/src/usr.bin/openssl/s_time.c,v
> > retrieving revision 1.31
> > diff -u -p -r1.31 s_time.c
> > --- s_time.c  28 Aug 2018 14:30:48 -  1.31
> > +++ s_time.c  28 Aug 2018 15:13:18 -
> > @@ -92,6 +92,7 @@ extern int verify_depth;
> >  static void s_time_usage(void);
> >  static int run_test(SSL *);
> >  static int benchmark(int);
> > +static void print_tally_mark(SSL *);
> >
> >  static SSL_CTX *tm_ctx = NULL;
> >  static const SSL_METHOD *s_time_meth = NULL;
> > @@ -393,6 +394,24 @@ run_test(SSL *scon)
> >   return 1;
> >  }
> >
> > +static void
> > +print_tally_mark(SSL *scon)
> > +{
> > + int ver;
> > +
> > + if (SSL_session_reused(scon))
> > + ver = 'r';
> > + else {
> > + ver = SSL_version(scon);
> > + if (ver == TLS1_VERSION)
> > + ver = 't';
> > + else
> > + ver = '*';
> > + }
> > + fputc(ver, stdout);
> > + fflush(stdout);
> > +}
> > +
> >  static int
> >  benchmark(int reuse_session)
> >  {
> > @@ -400,7 +419,6 @@ benchmark(int reuse_session)
> >   int nConn = 0;
> >   SSL *scon = NULL;
> >   int ret = 1;
> > - int ver;
> >
> >   if (reuse_session) {
> >   /* Get an SSL object so we can reuse the session id */
> > @@ -429,18 +447,7 @@ benchmark(int reuse_session)
> >   if (!run_test(scon))
> >   goto end;
> >   nConn += 1;
> > - if (SSL_session_reused(scon))
> > - ver = 'r';
> > - else {
> > - ver = SSL_version(scon);
> > - if (ver == TLS1_VERSION)
> > - ver = 't';
> > - else
> > - ver = '*';
> > - }
> > - fputc(ver, stdout);
> > - fflush(stdout);
> > -
> > + print_tally_mark(scon);
> >   if (!reuse_session) {
> >   SSL_free(scon);
> >   scon = NULL;
> >
> > Diff 1+2:
> >
> > Index: s_time.c
> > ===
> > RCS file: /cvs/src/usr.bin/openssl/s_time.c,v
> > retrieving revision 1.31
> > diff -u -p -r1.31 s_time.c
> > --- s_time.c  28 Aug 2018 14:30:48 -  1.31
> > +++ s_time.c  28 Aug 2018 15:15:27 -
> > @@ -92,6 +92,7 @@ extern int verify_depth;
> >  static void s_time_usage(void);
> >  static int run_test(SSL *);
> >  static int benchmark(int);
> > +static void print_tally_mark(SSL *);
> >
> >  static SSL_CTX *tm_ctx = NULL;
> >  static const SSL_METHOD *s_time_meth = NULL;
> > @@ -393,6 +394,33 @@ run_test(SSL *scon)
> >   return 1;
> >  }
> >
> > +static void
> > +print_tally_mark(SSL *scon)
> > +{
> > + int mark;
> > +
> > + if (SSL_session_reused(scon)) {
> > + mark = 'r';
> > + goto print;
> > + }
> > + switch (SSL_version(scon)) {
> > + case TLS1_VERSION:
> > +  

Nuke PLEDGE_STAT for further pledge/unveil disentaglement.

2018-08-05 Thread Bob Beck
So this gets rid of unveil's PLEDGE_STAT.

Instead we use UNVEIL_INSPECT which is set by the stat and access opeerations
that are needed for realpath() type traversals that effectively call stat/access
for each component of a pathname before doing a final operation on the end. 

The intended semantic of UNVEIL_INSPECT (which is only used in the kernel) 
is to allow inspection of vnodes that are traversed on the way to an 
unveil'ed component - just like what PLEDGE_STAT did. 

This also removes the use of PLEDGE_STATLIE in unveil - theo and I had
discussed that this was probably fine in lubljana, but I never did it
then. I'll remove STATLIE later if we decide that's the way we
are going. 

Passes regress - realpath still works, etc. etc.

ok?

Index: kern/kern_pledge.c
===
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.239
diff -u -p -u -p -r1.239 kern_pledge.c
--- kern/kern_pledge.c  2 Aug 2018 15:34:07 -   1.239
+++ kern/kern_pledge.c  5 Aug 2018 17:45:52 -
@@ -608,14 +608,14 @@ pledge_namei(struct proc *p, struct name
switch (p->p_pledge_syscall) {
case SYS_access:
/* tzset() needs this. */
-   if ((ni->ni_pledge == (PLEDGE_RPATH | PLEDGE_STAT)) &&
+   if (ni->ni_pledge == PLEDGE_RPATH &&
strcmp(path, "/etc/localtime") == 0) {
ni->ni_cnd.cn_flags |= BYPASSUNVEIL;
return (0);
}
 
/* when avoiding YP mode, getpw* functions touch this */
-   if (ni->ni_pledge == (PLEDGE_RPATH | PLEDGE_STAT) &&
+   if (ni->ni_pledge == PLEDGE_RPATH &&
strcmp(path, "/var/run/ypbind.lock") == 0) {
if (p->p_p->ps_pledge & PLEDGE_GETPW) {
ni->ni_cnd.cn_flags |= BYPASSUNVEIL;
@@ -713,7 +713,7 @@ pledge_namei(struct proc *p, struct name
break;
case SYS_readlink:
/* Allow /etc/malloc.conf for malloc(3). */
-   if ((ni->ni_pledge == (PLEDGE_RPATH | PLEDGE_STAT)) &&
+   if ((ni->ni_pledge == PLEDGE_RPATH) &&
strcmp(path, "/etc/malloc.conf") == 0) {
ni->ni_cnd.cn_flags |= BYPASSUNVEIL;
return (0);
@@ -721,7 +721,7 @@ pledge_namei(struct proc *p, struct name
break;
case SYS_stat:
/* DNS needs /etc/resolv.conf. */
-   if ((ni->ni_pledge == (PLEDGE_RPATH | PLEDGE_STAT)) &&
+   if ((ni->ni_pledge == PLEDGE_RPATH) &&
(p->p_p->ps_pledge & PLEDGE_DNS) &&
strcmp(path, "/etc/resolv.conf") == 0) {
ni->ni_cnd.cn_flags |= BYPASSUNVEIL;
@@ -732,9 +732,9 @@ pledge_namei(struct proc *p, struct name
 
/*
 * Ensure each flag of ni_pledge has counterpart allowing it in
-* ps_pledge. discard PLEDGE_STAT as it is unveil(2) stuff.
+* ps_pledge.
 */
-   if ((ni->ni_pledge & ~PLEDGE_STAT) & ~p->p_p->ps_pledge)
+   if (ni->ni_pledge & ~p->p_p->ps_pledge)
return (pledge_fail(p, EPERM, (ni->ni_pledge & 
~p->p_p->ps_pledge)));
 
/* continue, and check unveil if present */
Index: kern/kern_unveil.c
===
RCS file: /cvs/src/sys/kern/kern_unveil.c,v
retrieving revision 1.11
diff -u -p -u -p -r1.11 kern_unveil.c
--- kern/kern_unveil.c  5 Aug 2018 14:23:57 -   1.11
+++ kern/kern_unveil.c  5 Aug 2018 17:45:52 -
@@ -379,13 +379,20 @@ unveil_add_vnode(struct process *pr, str
return (uv);
 }
 
-void
+int
 unveil_add_traversed_vnodes(struct proc *p, struct nameidata *ndp)
 {
/*
-* add the traversed vnodes with 0 flags if they
-* are not already present.
+* Add the traversed vnodes with the UNVEIL_INSPECT flag
+* if they are not already present to allow traversal
+* operations such as access and stat. This lets
+* TOCTOU fans that call access on all components of
+* an unveil'ed path before the final operation
+* work.
 */
+   int ret = 0;
+   struct unveil *uv;
+
if (ndp->ni_tvpsize) {
size_t i;
 
@@ -394,10 +401,15 @@ unveil_add_traversed_vnodes(struct proc 
if (unveil_lookup(vp, p) == NULL) {
vref(vp);
vp->v_uvcount++;
-   unveil_add_vnode(p->p_p, vp);
+   uv = unveil_add_vnode(p->p_p, vp);
+   if (uv != NULL)
+   uv->uv_flags = UNVEIL_INSPECT;
+   else
+   ret = E2BIG;
}
}
}
+   return 

Re: unveil: incomplete unveil_flagmatch semantic

2018-08-04 Thread Bob Beck


> Some examples that will need consideration for unveil(2):
> - mount(2)
> - unmount(2)
> - quotactl(2)
> - chroot(2)
> - getfh(2)
> - acct(2)
> - coredump()
> - loadfirmware() - I think ifconfig(1) could make the kernel loading a
>   firmware for some network card
> 
> so having ni_unveil separated from ni_pledge could be good to be able to
> manage these namei() calls in unveiled programs.
> 

And yes, I am in violent agreement with this :)

this lets us have a cleaner separation and unveil things that aren't 
pledgeable. 



Re: unveil: incomplete unveil_flagmatch semantic

2018-08-04 Thread Bob Beck



> On Sat, Aug 04, 2018 at 10:40:11AM -0600, Bob Beck wrote:
> > On Fri, Aug 03, 2018 at 06:31:00AM +0200, Sebastien Marie wrote:
> > > On Thu, Aug 02, 2018 at 03:42:03PM +0200, Sebastien Marie wrote:
> > > > On Mon, Jul 30, 2018 at 07:55:35AM -0600, Bob Beck wrote:
> > > > > yeah the latter will be the way to go
> > > > > 
> > > > 
> > 
> > > > new diff with direct lookup using an indirection table.
> > > > 
> > > 
> > > new (emergency) version with PLEDGE_CHOWN consideration for unveil(2).
> > > 
> > > sorry for having missed it.
> > >  
> > 
> > All good because you gave me inspiration, after I ran your diff. 
> > 
> > I tied unveil to the pledge flags when I first did it because it was
> > convenient - I think this thig with chmod (and the awkwardness of
> > PLEDGE_STAT, etc. etc.) just shows that this was a decision of
> > convienience in the short term that is going to bite us in the long
> > term. 
> > 
> > The lookup table is clever, but is frankly, voodoo :) I don't like
> > trying to follow the logic of what maps to what and be concerned
> > about what flags are where just for the sake of this, and it
> > makes things ugly to read.
> > 
> > I think I would rather add my own char to the namei structure, 
> > and set it appropriately in the same places that pledge does. IMO
> > this makes looking at the source code for system calls much clearer
> > int the kernel - rather than trying to fathom in your head how a
> > combination of pledge flags will turn into unveil. 
> 
> it seems to me it is a bit duplicating annotations: annotating syscalls
> for pledge, and annotating syscalls for unveil, while pledge has just
> more granuality. but I can understand the problem with an additional
> level for translating pledge-flags to unveil-flags.
> 
> in consequence, you will have to change some bits in man page, as "r"
> will not be anymore "corresponding to the pledge(2) promise rpath" :)
> 
> > So this is a somewhat "minimal" diff tha puts the flags in 
> > namei.h, and checks them as per your change, but rather
> > than using a lookup table just expressly sets them
> > for each system call appropriately.. it passes regress
> > as is. 
> > 
> > I think after doing this I can probably go in an get rid of
> > the awkward PLEDGE_STAT and simplify BYPASS considerably
> > as well, but I will do that separately. 
> > 
> > ok?
> 
> some comments inlined.
> 
> and one important for starting: you should consider (ni_unveil==0) as
> deny by default, instead of allow by default. eventually having an
> panic(9) or an assert(9) (but it should be too early for now in case we
> want it).
> 
> it is important because else if we miss an explicit initialisation (the
> struct is zeroed, ni_unveil=0 by default), it will be an allowed
> operation by default and we will *not* see the problem. with deny or a
> panic(9) we will not miss it: someone will complain.
> 
> for a panic(9), it could be done in namei(), eventually just after
> pledge_namei() call, to avoid checking the variable too often.
> 
> 
> another important thing I just realized: not all filesystem operations
> were pledeable, and some haven't ni_pledge because the function was
> unreachable while pledged (but pledge_namei() has a panic for such
> cases).
> 
> Some examples that will need consideration for unveil(2):
> - mount(2)
> - unmount(2)
> - quotactl(2)
> - chroot(2)
> - getfh(2)
> - acct(2)
> - coredump()
> - loadfirmware() - I think ifconfig(1) could make the kernel loading a
>   firmware for some network card
> 
> so having ni_unveil separated from ni_pledge could be good to be able to
> manage these namei() calls in unveiled programs.
> 
> 
> > Index: kern/kern_unveil.c
> > ===
> > RCS file: /cvs/src/sys/kern/kern_unveil.c,v
> > retrieving revision 1.9
> > diff -u -p -u -p -r1.9 kern_unveil.c
> > --- kern/kern_unveil.c  30 Jul 2018 15:16:27 -  1.9
> > +++ kern/kern_unveil.c  4 Aug 2018 16:13:07 -
> > @@ -40,6 +40,11 @@
> >  #define UNVEIL_MAX_VNODES  128
> >  #define UNVEIL_MAX_NAMES   128
> >  
> > +#defineUNVEIL_READ 0x01
> > +#defineUNVEIL_WRITE0x02
> > +#defineUNVEIL_CREATE   0x04
> > +#defineUNVEIL_EXEC 0x08
> > +
> 
> the flags are duplicated with namei.h. I think namei.h is the right
> place, and there could be removed from here.
> 
> 

Re: unveil: incomplete unveil_flagmatch semantic

2018-08-04 Thread Bob Beck


> > +   nd.ni_unveil = 0; /* XXX No flags == allow it */
> 
> see my comment about ni_unveil != 0.
> 
> as you still have check on (ni_pledge & PLEDGE_STAT), it should be still
> ok.
> 

It doesn't actually do this yt.. this comment was a reminder for me
and should have had allow it? for my dealig with PLEDGE_STAT afterwards

I'm intend on making another flag for that the "you have to
be able to access it" a-la PLEDGE_STAT which was a hack - and
clean that up in a separate diff. so no, 0 flags won't be "allow it"



Re: unveil: incomplete unveil_flagmatch semantic

2018-08-04 Thread Bob Beck
On Fri, Aug 03, 2018 at 06:31:00AM +0200, Sebastien Marie wrote:
> On Thu, Aug 02, 2018 at 03:42:03PM +0200, Sebastien Marie wrote:
> > On Mon, Jul 30, 2018 at 07:55:35AM -0600, Bob Beck wrote:
> > > yeah the latter will be the way to go
> > > 
> > 

> > new diff with direct lookup using an indirection table.
> > 
> 
> new (emergency) version with PLEDGE_CHOWN consideration for unveil(2).
> 
> sorry for having missed it.
>  

All good because you gave me inspiration, after I ran your diff. 

I tied unveil to the pledge flags when I first did it because it was
convenient - I think this thig with chmod (and the awkwardness of
PLEDGE_STAT, etc. etc.) just shows that this was a decision of
convienience in the short term that is going to bite us in the long
term. 

The lookup table is clever, but is frankly, voodoo :) I don't like
trying to follow the logic of what maps to what and be concerned
about what flags are where just for the sake of this, and it
makes things ugly to read.

I think I would rather add my own char to the namei structure, 
and set it appropriately in the same places that pledge does. IMO
this makes looking at the source code for system calls much clearer
int the kernel - rather than trying to fathom in your head how a
combination of pledge flags will turn into unveil. 

So this is a somewhat "minimal" diff tha puts the flags in 
namei.h, and checks them as per your change, but rather
than using a lookup table just expressly sets them
for each system call appropriately.. it passes regress
as is. 

I think after doing this I can probably go in an get rid of
the awkward PLEDGE_STAT and simplify BYPASS considerably
as well, but I will do that separately. 

ok?


Index: dev/diskmap.c
===
RCS file: /cvs/src/sys/dev/diskmap.c,v
retrieving revision 1.22
diff -u -p -u -p -r1.22 diskmap.c
--- dev/diskmap.c   4 Jul 2018 12:42:30 -   1.22
+++ dev/diskmap.c   3 Aug 2018 02:38:26 -
@@ -85,6 +85,7 @@ diskmapioctl(dev_t dev, u_long cmd, cadd
 
NDINIT(, 0, 0, UIO_SYSSPACE, devname, p);
ndp.ni_pledge = PLEDGE_RPATH;
+   ndp.ni_unveil = UNVEIL_READ;
if ((error = vn_open(, fp0->f_flag, 0)) != 0)
goto invalid;
 
Index: kern/exec_elf.c
===
RCS file: /cvs/src/sys/kern/exec_elf.c,v
retrieving revision 1.145
diff -u -p -u -p -r1.145 exec_elf.c
--- kern/exec_elf.c 20 Jul 2018 21:57:26 -  1.145
+++ kern/exec_elf.c 3 Aug 2018 02:38:26 -
@@ -332,6 +332,7 @@ elf_load_file(struct proc *p, char *path
 
NDINIT(, LOOKUP, FOLLOW | LOCKLEAF, UIO_SYSSPACE, path, p);
nd.ni_pledge = PLEDGE_RPATH;
+   nd.ni_unveil = UNVEIL_READ;
if ((error = namei()) != 0) {
return (error);
}
Index: kern/kern_exec.c
===
RCS file: /cvs/src/sys/kern/kern_exec.c,v
retrieving revision 1.200
diff -u -p -u -p -r1.200 kern_exec.c
--- kern/kern_exec.c20 Jul 2018 21:57:26 -  1.200
+++ kern/kern_exec.c3 Aug 2018 02:38:26 -
@@ -275,6 +275,7 @@ sys_execve(struct proc *p, void *v, regi
 
NDINIT(, LOOKUP, NOFOLLOW, UIO_USERSPACE, SCARG(uap, path), p);
nid.ni_pledge = PLEDGE_EXEC;
+   nid.ni_unveil = UNVEIL_EXEC;
 
/*
 * initialize the fields of the exec package.
Index: kern/kern_ktrace.c
===
RCS file: /cvs/src/sys/kern/kern_ktrace.c,v
retrieving revision 1.98
diff -u -p -u -p -r1.98 kern_ktrace.c
--- kern/kern_ktrace.c  20 Jun 2018 10:48:55 -  1.98
+++ kern/kern_ktrace.c  3 Aug 2018 02:38:26 -
@@ -513,6 +513,7 @@ sys_ktrace(struct proc *p, void *v, regi
cred = p->p_ucred;
NDINIT(, LOOKUP, FOLLOW, UIO_USERSPACE, fname, p);
nd.ni_pledge = PLEDGE_CPATH | PLEDGE_WPATH;
+   nd.ni_unveil = UNVEIL_CREATE | UNVEIL_WRITE;
if ((error = vn_open(, FWRITE|O_NOFOLLOW, 0)) != 0)
return error;
vp = nd.ni_vp;
Index: kern/kern_unveil.c
===
RCS file: /cvs/src/sys/kern/kern_unveil.c,v
retrieving revision 1.9
diff -u -p -u -p -r1.9 kern_unveil.c
--- kern/kern_unveil.c  30 Jul 2018 15:16:27 -  1.9
+++ kern/kern_unveil.c  4 Aug 2018 16:13:07 -
@@ -40,6 +40,11 @@
 #define UNVEIL_MAX_VNODES  128
 #define UNVEIL_MAX_NAMES   128
 
+#defineUNVEIL_READ 0x01
+#defineUNVEIL_WRITE0x02
+#defineUNVEIL_CREATE   0x04
+#defineUNVEIL_EXEC 0x08
+
 static inline int
 unvname_compare(const struct unvname *n1, const struct unvname *n2)
 {
@@ -50,7 +55,7 @@ unvname_compare(const struct unvname *n1
 }
 
 struct unvname *
-unvname_new(

Re: unveil: incomplete unveil_flagmatch semantic

2018-07-30 Thread Bob Beck
yeah the latter will be the way to go

On Mon, Jul 30, 2018 at 06:02 Sebastien Marie  wrote:

> Hi,
>
> I think unveil_flagmatch() isn't complete and/or has not the right
> semantic.
>
> A bit of internals for starting (I will speak about ni_pledge, people
> that know what it is and how it works with pledge/unveil could go to
> "what is the problem" part).
>
> unveil(2) works with the syscall annotation introduced for pledge(2).
>
> When kernel needs to resolv a name to vnode, it used namei() function.
> For that it initializes a special structure used as namei() argument:
> `struct nameidata`. This struct has a field `ni_pledge` used to let vfs
> subsystem know what kind of syscalls called it. This way, underline
> subsystem doesn't have to know all syscalls, and could work on them by
> "group".
>
> For example, when you call open(2), depending the flags argument used,
> ni_pledge will contain PLEDGE_RPATH, PLEDGE_WPATH, or PLEDGE_CPATH. The
> subsystem has a quick and accurate view of the intented usage of the
> vnode.
>
> pledge(2) uses it to check that you have the promise of intent to use,
> and unveil(2) uses it too in a similar way, in particular in
> unveil_flagmatch() to check if the process has the accurate permission
> for this particular vnode.
>
>
>
> Now, what is the problem with unveil_flagmatch() ?
>
> If I exclude the PLEDGE_STAT stuff from the equation (I am not still
> convinced it is the right way to do it - but it isn't the question for
> now), unveil_flagmatch() has a default policy to allow the operation,
> and only check for some flags in ni_pledge to deny it.
>
> For simple syscall like open(2) there is no problem. The possible
> value of ni_pledge are composed with only PLEDGE_RPATH, PLEDGE_WPATH, or
> PLEDGE_CPATH.
>
> But for some others syscalls, it isn't the case.
>
> For example, chmod(2) will set ni_pledge to "PLEDGE_FATTR |
> PLEDGE_RPATH". For pledge(2), it means you must have "fattr" (capability
> to change attribute on the node) and "rpath" (capability to read the
> node) promise to use such syscall.
>
> As unveil(2) doesn't have the "fattr" concept, and unveil_flagmatch()
> will check only for PLEDGE_RPATH, PLEDGE_WPATH, PLEDGE_CPATH and
> PLEDGE_EXEC, we end with unsound policy: you could use chmod(2) with just
> "r" on unveiled part of the filesystem.
>
> Some others flags could occurs in ni_pledge:
> - PLEDGE_CHOWN: chown(2) family
> - PLEDGE_DPATH: mknod(2), mkfifo(2)
> - PLEDGE_FATTR: utimes(2) family, chmod(2) family, truncate(2), chflags(2)
> - PLEDGE_TTY:   revoke(2)
> - PLEDGE_UNIX:  bind(2), connect(2)
>
> unveil_flagmatch() has a special case for "" flag: it means deny.
> but as soon as you have "r", "w", "x" or "c", you have an allow policy
> by default. Check will be done only for a part of ni_pledge.
>
>
> I see basically two possibilities:
> - extending unveil(2) permissions to match pledge(2) flags - but I don't
>   like it, unveil(2) should be kept simple.
>
> - having a separate namespace for unveil and pledge flags (to avoid
>   confusion), and translating all pledge flags to unveil flags.
>
>   PLEDGE_RPATH => UNVEIL_READ
>   PLEDGE_WPATH => UNVEIL_WRITE
>   PLEDGE_CPATH => UNVEIL_CREATE
>   PLEDGE_EXEC  => UNVEIL_EXEC
>   PLEDGE_CHOWN => UNVEIL_WRITE
>   PLEDGE_DPATH => UNVEIL_CREATE
>   PLEDGE_FATTR => UNVEIL_WRITE
>   PLEDGE_TTY   => UNVEIL_WRITE
>   PLEDGE_UNIX  => UNVEIL_READ|UNVEIL_WRITE (requiring both)
>   any others   => panic(9)
>
> This way, we could be really exhaustive in unveil_flagmatch() without
> having to bother for future PLEDGE flag addition (as we will panic(9) if
> some developer doesn't add it where intented).
>
> Thanks.
> --
> Sebastien Marie
>


Re: unveil: incorrect type flags on unvname_new()

2018-07-16 Thread Bob Beck
ok beck@

On Mon, Jul 16, 2018 at 15:53 Sebastien Marie  wrote:

> Hi,
>
> While reviewing unveil(2) code, I found an incorrect type on
> unvname_new() function: flags argument should be uint64_t.
>
> It is called by unveil_add_name() which uses uint64_t for flags, and
> store the value in struct unvname un_flags member which is uint64_t.
>
> Thanks.
> --
> Sebastien Marie
>
>
> Index: kern/kern_unveil.c
> ===
> RCS file: /cvs/src/sys/kern/kern_unveil.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 kern_unveil.c
> --- kern/kern_unveil.c  13 Jul 2018 13:47:41 -  1.2
> +++ kern/kern_unveil.c  16 Jul 2018 13:08:40 -
> @@ -50,7 +50,7 @@ unvname_compare(const struct unvname *n1
>  }
>
>  struct unvname *
> -unvname_new(const char *name, size_t size, int flags)
> +unvname_new(const char *name, size_t size, uint64_t flags)
>  {
> struct unvname *ret = malloc(sizeof(struct unvname), M_PROC,
> M_WAITOK);
> ret->un_name = malloc(size, M_PROC, M_WAITOK);
>


Re: const qualifiers for EVP_*

2018-05-12 Thread Bob Beck
ok

On Sat, May 12, 2018 at 13:14 Theo Buehler  wrote:

> Here's another straightforward batch. As usual, it's been tested in a
> bulk by sthen and there was no fallout.
>
> Index: lib/libcrypto/asn1/ameth_lib.c
> ===
> RCS file: /var/cvs/src/lib/libcrypto/asn1/ameth_lib.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 ameth_lib.c
> --- lib/libcrypto/asn1/ameth_lib.c  21 Jan 2017 04:31:25 -
> 1.16
> +++ lib/libcrypto/asn1/ameth_lib.c  12 May 2018 18:42:51 -
> @@ -299,7 +299,7 @@ EVP_PKEY_asn1_get0_info(int *ppkey_id, i
>  }
>
>  const EVP_PKEY_ASN1_METHOD*
> -EVP_PKEY_get0_asn1(EVP_PKEY *pkey)
> +EVP_PKEY_get0_asn1(const EVP_PKEY *pkey)
>  {
> return pkey->ameth;
>  }
> Index: lib/libcrypto/evp/evp.h
> ===
> RCS file: /var/cvs/src/lib/libcrypto/evp/evp.h,v
> retrieving revision 1.59
> diff -u -p -r1.59 evp.h
> --- lib/libcrypto/evp/evp.h 2 May 2018 15:51:41 -   1.59
> +++ lib/libcrypto/evp/evp.h 12 May 2018 18:42:51 -
> @@ -617,7 +617,8 @@ int EVP_DigestSignFinal(EVP_MD_CTX *ctx,
>
>  int EVP_DigestVerifyInit(EVP_MD_CTX *ctx, EVP_PKEY_CTX **pctx,
>  const EVP_MD *type, ENGINE *e, EVP_PKEY *pkey);
> -int EVP_DigestVerifyFinal(EVP_MD_CTX *ctx, unsigned char *sig, size_t
> siglen);
> +int EVP_DigestVerifyFinal(EVP_MD_CTX *ctx, const unsigned char *sig,
> +size_t siglen);
>
>  int EVP_OpenInit(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *type,
>  const unsigned char *ek, int ekl, const unsigned char *iv, EVP_PKEY
> *priv);
> @@ -866,12 +867,12 @@ int EVP_PKEY_encrypt_old(unsigned char *
>  int EVP_PKEY_type(int type);
>  int EVP_PKEY_id(const EVP_PKEY *pkey);
>  int EVP_PKEY_base_id(const EVP_PKEY *pkey);
> -int EVP_PKEY_bits(EVP_PKEY *pkey);
> +int EVP_PKEY_bits(const EVP_PKEY *pkey);
>  int EVP_PKEY_size(EVP_PKEY *pkey);
>  int EVP_PKEY_set_type(EVP_PKEY *pkey, int type);
>  int EVP_PKEY_set_type_str(EVP_PKEY *pkey, const char *str, int len);
>  int EVP_PKEY_assign(EVP_PKEY *pkey, int type, void *key);
> -void *EVP_PKEY_get0(EVP_PKEY *pkey);
> +void *EVP_PKEY_get0(const EVP_PKEY *pkey);
>
>  #ifndef OPENSSL_NO_RSA
>  struct rsa_st;
> @@ -995,7 +996,7 @@ int EVP_PKEY_asn1_get0_info(int *ppkey_i
>  const char **pinfo, const char **ppem_str,
>  const EVP_PKEY_ASN1_METHOD *ameth);
>
> -const EVP_PKEY_ASN1_METHOD* EVP_PKEY_get0_asn1(EVP_PKEY *pkey);
> +const EVP_PKEY_ASN1_METHOD* EVP_PKEY_get0_asn1(const EVP_PKEY *pkey);
>  EVP_PKEY_ASN1_METHOD* EVP_PKEY_asn1_new(int id, int flags, const char
> *pem_str,
>  const char *info);
>  void EVP_PKEY_asn1_copy(EVP_PKEY_ASN1_METHOD *dst,
> Index: lib/libcrypto/evp/evp_pkey.c
> ===
> RCS file: /var/cvs/src/lib/libcrypto/evp/evp_pkey.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 evp_pkey.c
> --- lib/libcrypto/evp/evp_pkey.c29 Jan 2017 17:49:23 -
> 1.19
> +++ lib/libcrypto/evp/evp_pkey.c12 May 2018 18:42:51 -
> @@ -181,7 +181,8 @@ EVP_PKEY_get_attr_by_NID(const EVP_PKEY
>  }
>
>  int
> -EVP_PKEY_get_attr_by_OBJ(const EVP_PKEY *key, ASN1_OBJECT *obj, int
> lastpos)
> +EVP_PKEY_get_attr_by_OBJ(const EVP_PKEY *key, const ASN1_OBJECT *obj,
> +int lastpos)
>  {
> return X509at_get_attr_by_OBJ(key->attributes, obj, lastpos);
>  }
> Index: lib/libcrypto/evp/m_sigver.c
> ===
> RCS file: /var/cvs/src/lib/libcrypto/evp/m_sigver.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 m_sigver.c
> --- lib/libcrypto/evp/m_sigver.c29 Jan 2017 17:49:23 -  1.6
> +++ lib/libcrypto/evp/m_sigver.c12 May 2018 18:42:51 -
> @@ -166,7 +166,7 @@ EVP_DigestSignFinal(EVP_MD_CTX *ctx, uns
>  }
>
>  int
> -EVP_DigestVerifyFinal(EVP_MD_CTX *ctx, unsigned char *sig, size_t siglen)
> +EVP_DigestVerifyFinal(EVP_MD_CTX *ctx, const unsigned char *sig, size_t
> siglen)
>  {
> EVP_MD_CTX tmp_ctx;
> unsigned char md[EVP_MAX_MD_SIZE];
> Index: lib/libcrypto/evp/p_lib.c
> ===
> RCS file: /var/cvs/src/lib/libcrypto/evp/p_lib.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 p_lib.c
> --- lib/libcrypto/evp/p_lib.c   14 Apr 2018 07:09:21 -  1.21
> +++ lib/libcrypto/evp/p_lib.c   12 May 2018 18:42:51 -
> @@ -85,7 +85,7 @@
>  static void EVP_PKEY_free_it(EVP_PKEY *x);
>
>  int
> -EVP_PKEY_bits(EVP_PKEY *pkey)
> +EVP_PKEY_bits(const EVP_PKEY *pkey)
>  {
> if (pkey && pkey->ameth && pkey->ameth->pkey_bits)
> return pkey->ameth->pkey_bits(pkey);
> @@ -277,7 +277,7 @@ EVP_PKEY_assign(EVP_PKEY *pkey, int type
>  }
>
>  void *
> -EVP_PKEY_get0(EVP_PKEY *pkey)
> +EVP_PKEY_get0(const EVP_PKEY *pkey)
>  {
> return pkey->pkey.ptr;
>  }
> Index: lib/libcrypto/x509/x509.h
> 

Re: Anyone can suggest a BitCoin processor to the OpenBSD Foundation? BitPay has become terrible

2018-03-28 Thread Bob Beck
So, related to this topic, Apparently BitPay has now fixed us up again.

I have put the button back on the web site, if anyone wants to try a
bitcoin donation is is supposed to be possible again



Anyone can suggest a BitCoin processor to the OpenBSD Foundation? BitPay has become terrible

2018-02-16 Thread Bob Beck
So, as some of you may know, the OpenBSD Foundation has accepted BitCoin
donations
for some time via BitPay.com

BitPay was convenient for us since they will sell the BTC donations
immediately, and
convert to Canadian Dollars.  We then periodically get bank transfers of
the balance,
and this works great.

Obviously, for money laundering rules, we do need to provide them with some
documentation to prove the foundation's legitimacy, which we have
repatedly, and
this used to be not a problem.

Lately they seem to have decided in spite of being in posession of
1) A copy of two of our personal passports
2) A copy of the Canadian Federal Incorporation documents for a not for
profit
3) A copy of the Electric Bill for the address of record
4) A copy of an invoice sent to the foundation at the address of record
5) A copy of a bank statement from the foundation

That they seem to be suspending our access to BitPay - this in spite of this
being the only documents they request from us and then they seem to say
they won't work - because apparently they don't understand Canada our
countries outside of the USA.

So in short, bitPay has always been a bit of a pain for us, but now seems
to be
unable to be used by us.

So, what I'm looking for is opinions on an online processor.  Yes I am
aware they
charge fees, I am looking for convenience, basically, the ability to put a
button
on the foundation donation page that links to them. which will take a
donation
in BTC, convert it for us, and get it to us at a Canadian bank.  What are
all you people using?

For a number of reasons, we do *not* want to hold a bitCoin wallet
ourselves, and
be in the business of selling bitcoins, so please don't suggest this, and
No, we can't
have a bank account outside of Canada, so please don't suggest that either
:)


Re: libressl: crash in DES_fcrypt

2017-12-13 Thread Bob Beck
why AA?  why not just choose two random ascii salt chars at that point?  or
since this is effectively a failure case encrypt a random ascii salt and
random string?

using AA will produce a usable result based on the original string.
 encrypting a random string with a random salt means the failure return
wont be accidentally used

On Wed, Dec 13, 2017 at 06:51 Theo de Raadt  wrote:

> I still don't understand.
>
> This feels like fail-open.
>
> > I would like to commit this fix.  I tried to avoid the crash in
> > libcrypto with least possible impact for the DES_fcrypt() API.
> >
> > ok?
> >
> > bluhm
> >
> > On Tue, Dec 05, 2017 at 05:20:49PM +0100, Alexander Bluhm wrote:
> > > On Fri, Oct 27, 2017 at 01:50:26AM +0200, Jan Engelhardt wrote:
> > > > #include 
> > > > int main(void) {
> > > > char salt[3] = {0xf8, 0xd0, 0x00};
> > > > char out[32];
> > > > DES_fcrypt("foo", salt, out);
> > > > }
> > >
> > > This program produces a Segmentation fault in OpenBSD current.
> > >
> > > > openssl 1.1.x has it fixed (but 1.0.2l does not!) - their commit
> > > > seems to be 6493e4801e9edbe1ad1e256d4ce9cd55c8aa2242 in
> > > > https://github.com/openssl/openssl .
> > >
> > > Their fix changes the semantics of DES_crypt() and DES_fcrypt().
> > > They may fail and return NULL now.  This was never the case before
> > > so we may expect that programs do not check it.  With DES_fcrypt()
> > > it is very likely that some uninitilaized content of the return
> > > string is used.
> > >
> > > So I have extended the workaround that was there already.  Read the
> > > comment above the fix.  If the salt does not consist of two ascii
> > > characters, replace it with "AA".  This gives a safe result in all
> > > cases.
> > >
> > > ok?
> > >
> > > bluhm
> > >
> > > Index: lib/libcrypto/des/fcrypt.c
> > > ===
> > > RCS file: /data/mirror/openbsd/cvs/src/lib/libcrypto/des/fcrypt.c,v
> > > retrieving revision 1.12
> > > diff -u -p -r1.12 fcrypt.c
> > > --- lib/libcrypto/des/fcrypt.c  26 Dec 2016 21:30:10 -
> 1.12
> > > +++ lib/libcrypto/des/fcrypt.c  5 Dec 2017 16:03:57 -
> > > @@ -78,9 +78,15 @@ char *DES_fcrypt(const char *buf, const
> > >  * crypt to "*".  This was found when replacing the crypt in
> > >  * our shared libraries.  People found that the disabled
> > >  * accounts effectively had no passwd :-(. */
> > > -   x=ret[0]=((salt[0] == '\0')?'A':salt[0]);
> > > +   x = salt[0];
> > > +   if (x == 0 || x >= sizeof(con_salt))
> > > +   x = 'A';
> > > +   ret[0] = x;
> > > Eswap0=con_salt[x]<<2;
> > > -   x=ret[1]=((salt[1] == '\0')?'A':salt[1]);
> > > +   x = (salt[0] == '\0') ? 'A' : salt[1];
> > > +   if (x == 0 || x >= sizeof(con_salt))
> > > +   x = 'A';
> > > +   ret[1] = x;
> > > Eswap1=con_salt[x]<<6;
> > >  /* EAY
> > >  r=strlen(buf);
> > > Index: regress/lib/libcrypto/des/destest.c
> > > ===
> > > RCS file:
> /data/mirror/openbsd/cvs/src/regress/lib/libcrypto/des/destest.c,v
> > > retrieving revision 1.3
> > > diff -u -p -r1.3 destest.c
> > > --- regress/lib/libcrypto/des/destest.c 30 Oct 2015 15:58:40
> -  1.3
> > > +++ regress/lib/libcrypto/des/destest.c 5 Dec 2017 16:02:18 -
> > > @@ -749,18 +749,38 @@ plain[8+4], plain[8+5], plain[8+6], plai
> > > }
> > > printf("\n");
> > > printf("fast crypt test ");
> > > -   str=crypt("testing","ef");
> > > +   str=DES_crypt("testing","ef");
> > > if (strcmp("efGnQx2725bI2",str) != 0)
> > > {
> > > printf("fast crypt error, %s should be
> efGnQx2725bI2\n",str);
> > > err=1;
> > > }
> > > -   str=crypt("bca76;23","yA");
> > > +   str=DES_crypt("bca76;23","yA");
> > > if (strcmp("yA1Rp/1hZXIJk",str) != 0)
> > > {
> > > printf("fast crypt error, %s should be
> yA1Rp/1hZXIJk\n",str);
> > > err=1;
> > > }
> > > +   str = DES_crypt("testing", "\202B");
> > > +   if (strncmp("AB",str,2) != 0) {
> > > +   printf("salt %s first non ascii not replaced with A\n",
> str);
> > > +   err = 1;
> > > +   }
> > > +   str = DES_crypt("testing", "B\202");
> > > +   if (strncmp("BA",str,2) != 0) {
> > > +   printf("salt %s second non ascii not replaced with A\n",
> str);
> > > +   err = 1;
> > > +   }
> > > +   str = DES_crypt("testing", "\0B");
> > > +   if (strncmp("AA",str,2) != 0) {
> > > +   printf("salt %s first NUL not replaced with AA\n", str);
> > > +   err = 1;
> > > +   }
> > > +   str = DES_crypt("testing", "B");
> > > +   if (strncmp("BA",str,2) != 0) {
> > > +   printf("salt %s second NUL not replaced with A\n", str);
> > > +   err = 1;
> > > +   }
> > > printf("\n");
> > > return(err);
> > > }
> >
>
>


Re: iked, don't return NULL in print_host

2017-11-28 Thread Bob Beck
ok beck@

On Wed, Nov 29, 2017 at 02:17:21AM +0100, Claudio Jeker wrote:
> On Wed, Nov 29, 2017 at 01:59:06AM +0100, Claudio Jeker wrote:
> > Seen in my log file:
> > Nov 28 17:47:22 dramaqueen iked: vfprintf %s NULL in "%s: %s %s from %s to
> > %s ms gid %u, %ld bytes%s"
> > 
> > and
> > 
> > Nov 29 01:02:39 dramaqueen iked[49967]: ikev2_msg_send: IKE_SA_INIT
> > request from (null) to 62.48.30.5:500 msgid 0, 438 bytes
> > 
> > The problem seems to be in print_host so try to not return NULL in there.
> > Maybe we could return something else but this is a start IMO.
> 
> beck@ prefers to just print unknown instead  of the gai_strerror -- guess
> that is more sensible.
> 
> -- 
> :wq Claudio
> 
> Index: util.c
> ===
> RCS file: /cvs/src/sbin/iked/util.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 util.c
> --- util.c9 Jan 2017 14:49:21 -   1.33
> +++ util.c29 Nov 2017 01:16:21 -
> @@ -654,8 +654,8 @@ print_host(struct sockaddr *sa, char *bu
>  
>   if (getnameinfo(sa, sa->sa_len,
>   buf, len, NULL, 0, NI_NUMERICHOST) != 0) {
> - buf[0] = '\0';
> - return (NULL);
> + strlcpy(buf, "unknown", len);
> + return (buf);
>   }
>  
>   if ((port = socket_getport(sa)) != 0) {
> 



Official OpenBSD 6.2 CD set up for auction on Ebay

2017-11-18 Thread Bob Beck
So, the only 6.2 set to be produced is up for auction, featuring hand-drawn
artwork by Theo.

Artisanally Made in Canada!

All proceeds of the sale to fund OpenBSD development.

Go have a look at
http://www.ebay.ca/itm/Official-OpenBSD-6-2-CD-Set/253265944606


Re: [patch] ocspcheck: nextUpdate is optional according to RFC 6960

2017-09-06 Thread Bob Beck
effectivelyu providing a limitless OCSP staple is kind of stupid - you may
as well simply *not staple*

On Wed, Sep 6, 2017 at 8:23 AM, Bob Beck <b...@obtuse.com> wrote:

> I'm not super inclined to make this "flexible" unless we see this used int
> the wild, which I have not. We are more restrictive than
> OpenSSL in many areas.
>
> On Wed, Sep 6, 2017 at 1:31 AM, Andreas Bartelt <o...@bartula.de> wrote:
>
>> On 09/06/17 04:40, Bob Beck wrote:
>>
>>> Andreas where are you seeing this as being a real issue - who is shipping
>>> out OCSP responses without a next update field?
>>>
>>>
>> I've noticed this while playing with a local CA and a corresponding OCSP
>> responder on my LAN. For openssl ocsp, the -nmin or -ndays argument is
>> optional. If these arguments are not explicitly provided, the next update
>> field will not be set.
>>
>>
>>
>>>
>>> On Sat, Sep 2, 2017 at 11:28 AM, Andreas Bartelt <o...@bartula.de>
>>> wrote:
>>>
>>> ocspcheck effectively treats a missing nextUpdate like an error, i.e., it
>>>> always provides a warning and no staplefile is written out. According to
>>>> RFC 6960, the nextUpdate field is optional. The following patch should
>>>> handle this case more gracefully and include a suitable debug message
>>>> only
>>>> in case -vv is specified.
>>>>
>>>> OK?
>>>>
>>>> Index: src/usr.sbin/ocspcheck/ocspcheck.c
>>>> ===
>>>> RCS file: /cvs/src/usr.sbin/ocspcheck/ocspcheck.c,v
>>>> retrieving revision 1.21
>>>> diff -u -p -u -r1.21 ocspcheck.c
>>>> --- src/usr.sbin/ocspcheck/ocspcheck.c  8 May 2017 20:15:34 -
>>>>   1.21
>>>> +++ src/usr.sbin/ocspcheck/ocspcheck.c  2 Sep 2017 17:09:00 -
>>>> @@ -368,7 +368,7 @@ validate_response(char *buf, size_t size
>>>>   {
>>>>  ASN1_GENERALIZEDTIME *revtime = NULL, *thisupd = NULL,
>>>> *nextupd =
>>>> NULL;
>>>>  const unsigned char **p = (const unsigned char **)
>>>> -   int status, cert_status=0, crl_reason=0;
>>>> +   int status, cert_status=0, crl_reason=0, next_update=0;
>>>>  time_t now, rev_t = -1, this_t, next_t;
>>>>  OCSP_RESPONSE *resp;
>>>>  OCSP_BASICRESP *bresp;
>>>> @@ -447,12 +447,14 @@ validate_response(char *buf, size_t size
>>>>  return 0;
>>>>  }
>>>>  if ((next_t = parse_ocsp_time(nextupd)) == -1) {
>>>> -   warnx("unable to parse next update time in OCSP reply");
>>>> -   return 0;
>>>> +   if (verbose >= 2)
>>>> +   fprintf(stderr, "Optional timestamp for next
>>>> update not included in OCSP reply\n");
>>>>  }
>>>> +   else
>>>> +   next_update = 1;
>>>>
>>>>  /* Don't allow this update to precede next update */
>>>> -   if (this_t >= next_t) {
>>>> +   if (next_update == 1 && this_t >= next_t) {
>>>>  warnx("Invalid OCSP reply: this update >= next
>>>> update");
>>>>  return 0;
>>>>  }
>>>> @@ -481,7 +483,7 @@ validate_response(char *buf, size_t size
>>>>  /*
>>>>   * Check that next update is still valid
>>>>   */
>>>> -   if (next_t < now - JITTER_SEC) {
>>>> +   if (next_update == 1 && next_t < now - JITTER_SEC) {
>>>>  warnx("Invalid OCSP reply: reply has expired (%s)",
>>>>  ctime(_t));
>>>>  return 0;
>>>> @@ -489,7 +491,8 @@ validate_response(char *buf, size_t size
>>>>
>>>>  vspew("OCSP response validated from %s\n", host);
>>>>  vspew("This Update: %s", ctime(_t));
>>>> -   vspew("Next Update: %s", ctime(_t));
>>>> +   if (next_update == 1)
>>>> +   vspew("Next Update: %s", ctime(_t));
>>>>  return 1;
>>>>   }
>>>>
>>>>
>>>>
>>>
>>
>


Re: [patch] ocspcheck: nextUpdate is optional according to RFC 6960

2017-09-06 Thread Bob Beck
I'm not super inclined to make this "flexible" unless we see this used int
the wild, which I have not. We are more restrictive than
OpenSSL in many areas.

On Wed, Sep 6, 2017 at 1:31 AM, Andreas Bartelt <o...@bartula.de> wrote:

> On 09/06/17 04:40, Bob Beck wrote:
>
>> Andreas where are you seeing this as being a real issue - who is shipping
>> out OCSP responses without a next update field?
>>
>>
> I've noticed this while playing with a local CA and a corresponding OCSP
> responder on my LAN. For openssl ocsp, the -nmin or -ndays argument is
> optional. If these arguments are not explicitly provided, the next update
> field will not be set.
>
>
>
>>
>> On Sat, Sep 2, 2017 at 11:28 AM, Andreas Bartelt <o...@bartula.de> wrote:
>>
>> ocspcheck effectively treats a missing nextUpdate like an error, i.e., it
>>> always provides a warning and no staplefile is written out. According to
>>> RFC 6960, the nextUpdate field is optional. The following patch should
>>> handle this case more gracefully and include a suitable debug message
>>> only
>>> in case -vv is specified.
>>>
>>> OK?
>>>
>>> Index: src/usr.sbin/ocspcheck/ocspcheck.c
>>> ===
>>> RCS file: /cvs/src/usr.sbin/ocspcheck/ocspcheck.c,v
>>> retrieving revision 1.21
>>> diff -u -p -u -r1.21 ocspcheck.c
>>> --- src/usr.sbin/ocspcheck/ocspcheck.c  8 May 2017 20:15:34 -
>>>   1.21
>>> +++ src/usr.sbin/ocspcheck/ocspcheck.c  2 Sep 2017 17:09:00 -
>>> @@ -368,7 +368,7 @@ validate_response(char *buf, size_t size
>>>   {
>>>  ASN1_GENERALIZEDTIME *revtime = NULL, *thisupd = NULL, *nextupd
>>> =
>>> NULL;
>>>  const unsigned char **p = (const unsigned char **)
>>> -   int status, cert_status=0, crl_reason=0;
>>> +   int status, cert_status=0, crl_reason=0, next_update=0;
>>>  time_t now, rev_t = -1, this_t, next_t;
>>>  OCSP_RESPONSE *resp;
>>>  OCSP_BASICRESP *bresp;
>>> @@ -447,12 +447,14 @@ validate_response(char *buf, size_t size
>>>  return 0;
>>>  }
>>>  if ((next_t = parse_ocsp_time(nextupd)) == -1) {
>>> -   warnx("unable to parse next update time in OCSP reply");
>>> -   return 0;
>>> +   if (verbose >= 2)
>>> +   fprintf(stderr, "Optional timestamp for next
>>> update not included in OCSP reply\n");
>>>  }
>>> +   else
>>> +   next_update = 1;
>>>
>>>  /* Don't allow this update to precede next update */
>>> -   if (this_t >= next_t) {
>>> +   if (next_update == 1 && this_t >= next_t) {
>>>  warnx("Invalid OCSP reply: this update >= next update");
>>>  return 0;
>>>  }
>>> @@ -481,7 +483,7 @@ validate_response(char *buf, size_t size
>>>  /*
>>>   * Check that next update is still valid
>>>   */
>>> -   if (next_t < now - JITTER_SEC) {
>>> +   if (next_update == 1 && next_t < now - JITTER_SEC) {
>>>  warnx("Invalid OCSP reply: reply has expired (%s)",
>>>  ctime(_t));
>>>  return 0;
>>> @@ -489,7 +491,8 @@ validate_response(char *buf, size_t size
>>>
>>>  vspew("OCSP response validated from %s\n", host);
>>>  vspew("This Update: %s", ctime(_t));
>>> -   vspew("Next Update: %s", ctime(_t));
>>> +   if (next_update == 1)
>>> +   vspew("Next Update: %s", ctime(_t));
>>>  return 1;
>>>   }
>>>
>>>
>>>
>>
>


Re: [patch] ocspcheck: nextUpdate is optional according to RFC 6960

2017-09-05 Thread Bob Beck
Andreas where are you seeing this as being a real issue - who is shipping
out OCSP responses without a next update field?



On Sat, Sep 2, 2017 at 11:28 AM, Andreas Bartelt  wrote:

> ocspcheck effectively treats a missing nextUpdate like an error, i.e., it
> always provides a warning and no staplefile is written out. According to
> RFC 6960, the nextUpdate field is optional. The following patch should
> handle this case more gracefully and include a suitable debug message only
> in case -vv is specified.
>
> OK?
>
> Index: src/usr.sbin/ocspcheck/ocspcheck.c
> ===
> RCS file: /cvs/src/usr.sbin/ocspcheck/ocspcheck.c,v
> retrieving revision 1.21
> diff -u -p -u -r1.21 ocspcheck.c
> --- src/usr.sbin/ocspcheck/ocspcheck.c  8 May 2017 20:15:34 -
>  1.21
> +++ src/usr.sbin/ocspcheck/ocspcheck.c  2 Sep 2017 17:09:00 -
> @@ -368,7 +368,7 @@ validate_response(char *buf, size_t size
>  {
> ASN1_GENERALIZEDTIME *revtime = NULL, *thisupd = NULL, *nextupd =
> NULL;
> const unsigned char **p = (const unsigned char **)
> -   int status, cert_status=0, crl_reason=0;
> +   int status, cert_status=0, crl_reason=0, next_update=0;
> time_t now, rev_t = -1, this_t, next_t;
> OCSP_RESPONSE *resp;
> OCSP_BASICRESP *bresp;
> @@ -447,12 +447,14 @@ validate_response(char *buf, size_t size
> return 0;
> }
> if ((next_t = parse_ocsp_time(nextupd)) == -1) {
> -   warnx("unable to parse next update time in OCSP reply");
> -   return 0;
> +   if (verbose >= 2)
> +   fprintf(stderr, "Optional timestamp for next
> update not included in OCSP reply\n");
> }
> +   else
> +   next_update = 1;
>
> /* Don't allow this update to precede next update */
> -   if (this_t >= next_t) {
> +   if (next_update == 1 && this_t >= next_t) {
> warnx("Invalid OCSP reply: this update >= next update");
> return 0;
> }
> @@ -481,7 +483,7 @@ validate_response(char *buf, size_t size
> /*
>  * Check that next update is still valid
>  */
> -   if (next_t < now - JITTER_SEC) {
> +   if (next_update == 1 && next_t < now - JITTER_SEC) {
> warnx("Invalid OCSP reply: reply has expired (%s)",
> ctime(_t));
> return 0;
> @@ -489,7 +491,8 @@ validate_response(char *buf, size_t size
>
> vspew("OCSP response validated from %s\n", host);
> vspew("This Update: %s", ctime(_t));
> -   vspew("Next Update: %s", ctime(_t));
> +   if (next_update == 1)
> +   vspew("Next Update: %s", ctime(_t));
> return 1;
>  }
>
>


Re: [PATCH 0/2] SMALL_TIME_T follow-ups (was Re: [PATCH] allow notAfter after 2038 with 32-bit time_t)

2017-08-26 Thread Bob Beck
> 
> With the new define (SMALL_TIME_T) enabled, a 32-bit time_t build  
> using "openssl s_client -connect" can successfully connect to a server  
> and verify its certificate chain when one or more notAfter dates after  
> 2038 are present.
> 
> However, using "nc -c" fails to connect to the same server.
> 
> The reason being that libtls also needs to clamp the notAfter date.

I've just committed a change that enables libtls to use it. so
nc and libtls should work on broken computers now. 

I still think karma for me doing this is I will be killed by an
embedded 32 bit linux device in 2038...

 -Bob




Re: [PATCH] allow notAfter after 2038 with 32-bit time_t

2017-08-13 Thread Bob Beck
https://github.com/openbsd/src/commit/b943944faeecf3a978bf3f57df1b35335ffecbec

On Tue, Jul 11, 2017 at 4:23 AM, Stuart Henderson 
wrote:

> On 2017/07/11 01:55, Kyle J. McKay wrote:
> > 2) 32-bit systems are going to be around for many years still; 32-bit ARM
> > platforms are everywhere
> ..
> > 4) 32-bit time_t has potentially still got over 20 years of life left in
> it
>
> Yes. The gamble is whether 32-bit systems will still be around then.
> I don't see why they _wouldn't_ be. The sooner that OS adapt, the less
> likely there are to still be operational systems when 2038 comes around.
>
> > 3) 32-bit systems typically have 32-bit time_t values (I'm not aware of
> > anything in the standard preventing use of a 64-bit time_t on a 32-bit
> > system but that doesn't seem to be happening, at least not yet)
>
> This depends on the OS. Linux's general avoidance of ABI breaks is
> certainly a problem in this area. NetBSD has used 64-bit time_t on all
> architectures since 6.0 (2012) and OpenBSD since 5.5 (2014). I haven't
> looked recently but IIRC FreeBSD has 64-bit time_t on 32-bit ARM
> (and of course on 64-bit systems).
>
> Plenty of software still needs patching to fix operation on a 32-bit
> arch with 64-bit time_t - we have a lot of these in the ports tree
> (with mixed success feeding such changes upstream - "Linux doesn't
> need it"...).
>
>


Re: [PATCH] allow notAfter after 2038 with 32-bit time_t

2017-07-05 Thread Bob Beck
On Thu, May 18, 2017 at 7:31 AM, Kyle J. McKay  wrote:

> RFC 5280 section 4.1.2.5 states:
>
> To indicate that a certificate has no well-defined expiration date,
> the notAfter SHOULD be assigned the GeneralizedTime value of
> 1231235959Z.
>
>
True enough.



> Unfortunately, if sizeof(time_t) == 4, -12-31T23:59:59Z cannot be
> represented as a time_t value causing valid certificates to be rejected
> just because the notAfter value is after 2038-01-19T03:14:07Z.
>

Correct

So, I'll ask - what is the platform you are using that needs this? OpenBSD
does not, nor do most modern unix systems - So what platform are we doing
this for?

I'm not asking it to be nasty, but to put it in a slightly different
context "OMG, windows 98 does not support this, but if you add this code I
can support windows 98". You and I both know what your answer would
probably be for the person with Windows 98, which is "Here's a nickel kid,
get a modern operating system".

We ripped out support for a lot of moribund platforms for a reason.  What
platform are you using with 32 bit time where this is an issue?  I'm not
saying no, I'm saying "convince me this matters for anything relevant
please", and I am open to being convinced.


Fix this problem by disabling the restriction in the X509_cmp_time function
> and "wrap" far in the future notAfter values to 2038-01-19T03:14:07Z in the
> tls_get_peer_cert_times function.
>
> With both of these changes certificates with "no well-defined expiration
> date" as specified by RFC 5280 are again accepted on platforms where the
> sizeof(time_t) == 4.
>
> In general, there's no reason that a notAfter value should not be wrapped
> to 2038-01-19T03:14:07Z on a system with a 32-bit time_t.  The system
> itself
> can never have a time after 2038-01-19T03:14:07Z because of the size of the
> time_t type and so wrapping a notAfter date that is after
> 2038-01-19T03:14:07Z
> to 2038-01-19T03:14:07Z can never result in any additional certificates
> being
> accepted on such a system.
>
> Signed-off-by: Kyle J. McKay 
> ---
>
> For those using the libressl-2.5.4.tar.gz distribution, an equivalent
> patch that updates the tarball files instead can be found here:
>
>   https://gist.github.com/7d4d59bbae9e4d18444b86aa79d6f350
>
> Without this patch (or an equivalent), libressl-portable is not a viable
> alternative to OpenSSL on systems with a 32-bit time_t.
>
> It rejects valid TLS connections to any site that contains a notAfter date
> after 2038-01-19T03:14:07Z in any certificate in the chain.
>
> Besides the special case date mentioned above, there are many root
> certificates
> already in use today that have notAfter dates beyond 2038-01-19.
>
> These patches have been tested with libressl-portable 2.5.4 on a system
> with
> a 32-bit time_t.  With both of these patches the "nc -c" command
> successfully
> connects to sites with certificates that include notAfter dates after
> 2038-01-19.
>
> If either patch is omitted, "nc -c" fails to connect.
>
>  src/lib/libcrypto/x509/x509_vfy.c | 3 ++-
>  src/lib/libtls/tls_conninfo.c | 8 ++--
>  2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/src/lib/libcrypto/x509/x509_vfy.c
> b/src/lib/libcrypto/x509/x509_vfy.c
> index d8c09a12..c59bd258 100644
> --- a/src/lib/libcrypto/x509/x509_vfy.c
> +++ b/src/lib/libcrypto/x509/x509_vfy.c
> @@ -1882,7 +1882,8 @@ X509_cmp_time(const ASN1_TIME *ctm, time_t *cmp_time)
>  * a time_t. A time_t must be sane if you care about times after
>  * Jan 19 2038.
>  */
> -   if ((time1 = timegm()) == -1)
> +   if (((time1 = timegm()) == -1) &&
> +   ((sizeof(time_t) != 4) || tm1.tm_year < 138))
> goto out;
>
> if (gmtime_r(, ) == NULL)
> diff --git a/src/lib/libtls/tls_conninfo.c b/src/lib/libtls/tls_conninfo.c
> index 5cdd0f77..a59b4ba2 100644
> --- a/src/lib/libtls/tls_conninfo.c
> +++ b/src/lib/libtls/tls_conninfo.c
> @@ -142,8 +142,12 @@ tls_get_peer_cert_times(struct tls *ctx, time_t
> *notbefore,
> goto err;
> if ((*notbefore = timegm(_tm)) == -1)
> goto err;
> -   if ((*notafter = timegm(_tm)) == -1)
> -   goto err;
> +   if ((*notafter = timegm(_tm)) == -1) {
> +   if (sizeof(time_t) == 4 && after_tm.tm_year >= 138)
> +   *notafter = 2147483647;
> +   else
> +   goto err;
> +   }
>
> return (0);
>
> ---
>
>


Re: Better handling of short reads

2017-06-14 Thread Bob Beck

> As you all might have gathered by now Amit has jumped the gun
> but was wrong to do so.  His setup is not affected by this change.
> That was expected so please don't get distracted by this as I'm
> still looking forward to replies to the original set of changes.
> beck@?
> 
> > diff --git sys/kern/vfs_bio.c sys/kern/vfs_bio.c
> > index 95bc80bc0e6..9316e6e0eb2 100644
> > --- sys/kern/vfs_bio.c
> > +++ sys/kern/vfs_bio.c
> > @@ -534,10 +534,27 @@ bread_cluster_callback(struct buf *bp)
> >  */
> > buf_fix_mapping(bp, newsize);
> > bp->b_bcount = newsize;
> > }
> >  
> > +   /* Invalidate read-ahead buffers if read short */
> > +   if (bp->b_resid > 0) {
> > +   printf("read %ld resid %ld\n", bp->b_bcount, bp->b_resid);

Should the printf actually be here?  I'm not thinking this thing 
spewing dmesg like a banshee if we get short reads is really going
to help anything

> > +   for (i = 0; xbpp[i] != NULL; i++)
> > +   continue;
> > +   for (i = i - 1; i != 0; i--) {
> > +   if (xbpp[i]->b_bufsize <= bp->b_resid) {
> > +   bp->b_resid -= xbpp[i]->b_bufsize;
> > +   SET(xbpp[i]->b_flags, B_INVAL);
> > +   } else if (bp->b_resid > 0) {
> > +   bp->b_resid = 0;
> > +   SET(xbpp[i]->b_flags, B_INVAL);
> > +   } else
> > +   break;
> > +   }
> > +   }
> > +
> > for (i = 1; xbpp[i] != 0; i++) {
> > if (ISSET(bp->b_flags, B_ERROR))
> > SET(xbpp[i]->b_flags, B_INVAL | B_ERROR);
> > biodone(xbpp[i]);
> > }
> 



Re: Better handling of short reads

2017-06-14 Thread Bob Beck
 - ok mike, I'm looking at it..   Allow me a short while to beat my
head against a wall for a bit to get it into readahead mode...


On Wed, Jun 14, 2017 at 3:56 AM, Mike Belopuhov  wrote:

> On Thu, Jun 08, 2017 at 11:55 +0200, Mike Belopuhov wrote:
> > On Wed, Jun 07, 2017 at 23:04 -0500, Amit Kulkarni wrote:
> > > On Wed, 7 Jun 2017 21:27:27 -0500
> > > Amit Kulkarni  wrote:
> > >
> > > > On Thu, 8 Jun 2017 01:57:25 +0200
> > > > Mike Belopuhov  wrote:
> > > >
> > > > > On Wed, Jun 07, 2017 at 18:35 -0500, Amit Kulkarni wrote:
> > > > > > Wow, please get this in!!!
> > > > > >
> > > > > > This fixes cvs update on hard disks, to go much much faster.
> When I am
> > > > > > updating the entire set of cvs trees: www, src, xenocara, ports,
> I can
> > > > > > still use firefox and have it perfectly usable. There's a night
> and
> > > > > > day improvement, before and after. Thanks for debugging and
> fixing
> > > > > > this.
> > > > > >
> > > > >
> > > > > What kind of broken hardware do you have that this diff helps you?
> > > > > Can you show us your dmesg?
> > > > >
> > >
> > > Please ignore previous dmesg, it was incomplete.
> > >
> >
> > Are you 100% sure this diff changes anything for you?
> > Can you please try the one below.  It adds a printf.
> >
>
> As you all might have gathered by now Amit has jumped the gun
> but was wrong to do so.  His setup is not affected by this change.
> That was expected so please don't get distracted by this as I'm
> still looking forward to replies to the original set of changes.
> beck@?
>
> > diff --git sys/kern/vfs_bio.c sys/kern/vfs_bio.c
> > index 95bc80bc0e6..9316e6e0eb2 100644
> > --- sys/kern/vfs_bio.c
> > +++ sys/kern/vfs_bio.c
> > @@ -534,10 +534,27 @@ bread_cluster_callback(struct buf *bp)
> >*/
> >   buf_fix_mapping(bp, newsize);
> >   bp->b_bcount = newsize;
> >   }
> >
> > + /* Invalidate read-ahead buffers if read short */
> > + if (bp->b_resid > 0) {
> > + printf("read %ld resid %ld\n", bp->b_bcount, bp->b_resid);
> > + for (i = 0; xbpp[i] != NULL; i++)
> > + continue;
> > + for (i = i - 1; i != 0; i--) {
> > + if (xbpp[i]->b_bufsize <= bp->b_resid) {
> > + bp->b_resid -= xbpp[i]->b_bufsize;
> > + SET(xbpp[i]->b_flags, B_INVAL);
> > + } else if (bp->b_resid > 0) {
> > + bp->b_resid = 0;
> > + SET(xbpp[i]->b_flags, B_INVAL);
> > + } else
> > + break;
> > + }
> > + }
> > +
> >   for (i = 1; xbpp[i] != 0; i++) {
> >   if (ISSET(bp->b_flags, B_ERROR))
> >   SET(xbpp[i]->b_flags, B_INVAL | B_ERROR);
> >   biodone(xbpp[i]);
> >   }
>
>


Re: ocspcheck size_t printing

2017-05-08 Thread Bob Beck
You are correct. 

Patch committed.  Thanks!

-Bob


On Mon, May 08, 2017 at 08:20:57PM +0200, Jonas 'Sortie' Termansen wrote:
> Hi,
> 
> When upgrading to libressl-2.5.4 I noticed a couple -Wformat errors due
> to this code assuming size_t is of type long when it was actually int on
> this 32-bit system. Here's a patch against cvs that fixes the issue and
> also prints the variableas unsigned type.
> 
> Jonas
> 
> Index: ocspcheck.c
> ===
> RCS file: /cvs/src/usr.sbin/ocspcheck/ocspcheck.c,v
> retrieving revision 1.20
> diff -u -r1.20 ocspcheck.c
> --- ocspcheck.c   27 Mar 2017 23:59:08 -  1.20
> +++ ocspcheck.c   8 May 2017 18:15:51 -
> @@ -564,7 +564,7 @@
>   if ((request = ocsp_request_new_from_cert(certfile, nonce)) == NULL)
>   exit(1);
>  
> - dspew("Built an %ld byte ocsp request\n", request->size);
> + dspew("Built an %zu byte ocsp request\n", request->size);
>  
>   if ((host = url2host(request->url, , )) == NULL)
>   errx(1, "Invalid OCSP url %s from %s", request->url,
> @@ -601,7 +601,7 @@
>   dspew("Server at %s returns:\n", host);
>   for (i = 0; i < httphsz; i++)
>   dspew("   [%s]=[%s]\n", httph[i].key, httph[i].val);
> - dspew("   [Body]=[%ld bytes]\n", hget->bodypartsz);
> + dspew("   [Body]=[%zu bytes]\n", hget->bodypartsz);
>   if (hget->bodypartsz <= 0)
>   errx(1, "No body in reply from %s", host);
>  
> 



Official OpenBSD 6.1 CD !

2017-05-03 Thread Bob Beck
So.  There *Is* an official OpenBSD 6.1 CD

Just One.

If you are interested, please bid on ebay :

http://www.ebay.com/itm/The-only-Official-OpenBSD-6-1-CD-set-to-be-made-For-auction-for-the-project-/252910718452?hash=item3ae2a74df4:g:SJQAAOSwrhBZBqkd

(It's a pretty cool little CD set!)


Re: explicit_bzero after readpassphrase

2017-05-01 Thread Bob Beck
On Mon, May 01, 2017 at 04:07:27PM -0600, Theo de Raadt wrote:
> 
> Let me stop here and ask if the pattern is: "always explicit_bzero
> a password field once it is used"?  It might make sense, but some
> of these are heading straight to exit immediately.  Is it too much
> to do it then, or is the worry these code patterns might get copied
> elsewhere?
> 

I would fall on the side of "It could get copied elsewhere or hoisted 
for other reasons (like pledge)" so do it anyway. 



Re: patch: mv(1): Add -p flag to preserve time stamps for moved directories

2017-04-11 Thread Bob Beck

> Note that I have noatime on this FS.

then turn that off, or understand that things will not behave as you expect 
them to with it on. 




Re: httpd/libtls: TLS client certificate revocation checking

2017-04-01 Thread Bob Beck
There will be some libtls api additions post 6.1 to get the peer cert in
PEM format

In the meantime, testing snaps prior to 6.1 should be the priority. not a
talkathon.

On Sat, Apr 1, 2017 at 10:49 Joerg Sonnenberger  wrote:

> On Sat, Apr 01, 2017 at 07:53:05PM +1030, Jack Burton wrote:
> > One common example of that happening is when a cert gets revoked because
> > its private key has been lost/stolen and the user needs a new cert
> > associated with the same identity. An even more common example is when
> > a cert expires & gets renewed.
>
> If you are using certificate revocation, I think you should do the check
> as early as possible. That means in httpd in this case. Nothing later in
> the stack should have to care about expired or revoked certificates --
> it just adds complexity and the danger of someone forgetting about it.
>
> Which mechanisms to support (i.e. CRLs or OCSP) is a completely
> different topic.
>
> Joerg
>
>


Re: regarding OpenSSL License change

2017-03-23 Thread Bob Beck
On Thu, Mar 23, 2017 at 17:48 Bob Beck <b...@obtuse.com> wrote:

> Honestly, anyone who gets one of these should say no
>
> what would you all think if people quietly took derived works of software
> licensed under one license and took silence as assent to relicense
>
> Does this mean that with an unanswered email i can now release my re
> licensed as ISC version of gcc?  or the linux kernel?
>
> This sort of action just means that any software you write can be
> plagiarized against your will if you did not find out about it in time.
>
> thats really not cool
>
> If you write software this is not a world you want to live in.   Even if
> it does mean a anyone who can fork a github repo could get rid of the GPL
> after a period of non response from an author (dont go on vacation). As
> much as I might not agree with the GPL personally, I respect someones right
> to release thier work under a license and have it respected. without having
> to constantly answer emails and click web links telling people no
>
>
>
> On Thu, Mar 23, 2017 at 10:58 Theo de Raadt <dera...@openbsd.org> wrote:
>
> > > The start suggests they want to privately collect sufficient consensus
> > > to pass their agenda.  They appear to be considering all actions in
> > > the tree (including mine) on equal grounds.
> >
> > I already sent them a clear "NO, i explicitly object to relicensing
> > any of my contributions."
> >
> > If any of you care about the possibility of merging future OpenSSL
> > improvements to LibreSSL and OpenBSD, i suggest you do the same.
> >
> > Similarly, if any of you dislike publishing their own code under Apache
> 2.
>
> There has been no discussion amongst the greater community of
> developers as to which license to take.  Apache 2 has come as an edict
> from Rich Salz.
>
> There has also been no statement from the original authorship that this
> is the way to go.
>
> I suspect there is a lack of approval from some, and manufacturing
> consent in volume is the approach being taken.
>
>
> Apparently lawyers are being paid to help them push this through.  Is
> that being paid for by donations people gave after Heartbleed?  Is
> this why people donated?
>
>


Re: regarding OpenSSL License change

2017-03-23 Thread Bob Beck
Honestly, anyone who gets one of these should say no

what would you all think if people quietly took derived works of software
licensed under one license and took silence as assent to relicense

Does this mean that with an unanswered email i can now release my re
licensed as ISC version of gcc?  or the linux kernel?

This sort of action just means that any software you write can be
plagiarized against your will if you did not find out about it in time.

thats really not cool

If you write software this is not a world you want to live in.   Even if it
does mean a anyone who can fork a github repo could get rid of the GPL
after a period of non response from an author (dont go on vacation). As
much as I might not agree with the GPL personally, I respect someones right
to release thier work under a license and have it respected. without having
to constantly answer emails and click web links telling people no



On Thu, Mar 23, 2017 at 10:58 Theo de Raadt  wrote:

> > > The start suggests they want to privately collect sufficient consensus
> > > to pass their agenda.  They appear to be considering all actions in
> > > the tree (including mine) on equal grounds.
> >
> > I already sent them a clear "NO, i explicitly object to relicensing
> > any of my contributions."
> >
> > If any of you care about the possibility of merging future OpenSSL
> > improvements to LibreSSL and OpenBSD, i suggest you do the same.
> >
> > Similarly, if any of you dislike publishing their own code under Apache
> 2.
>
> There has been no discussion amongst the greater community of
> developers as to which license to take.  Apache 2 has come as an edict
> from Rich Salz.
>
> There has also been no statement from the original authorship that this
> is the way to go.
>
> I suspect there is a lack of approval from some, and manufacturing
> consent in volume is the approach being taken.
>
>
> Apparently lawyers are being paid to help them push this through.  Is
> that being paid for by donations people gave after Heartbleed?  Is
> this why people donated?
>
>


Re: tlsv1 alert decrypt error

2017-03-06 Thread Bob Beck
And as joel mentioned, a fix is already arriving for this - there was a bug
in SSLv2 compatible handshake initiation,
and Paypal still has it enabled... (yeeuch)

On Mon, Mar 6, 2017 at 3:48 PM, Bob Beck <b...@obtuse.com> wrote:

>
> Move it to tech@ from misc.. not libressl.. libressl is not special ;)
>
> On Mon, Mar 6, 2017 at 3:21 PM, Kirill Miazine <k...@krot.org> wrote:
>
>> Moving to libressl@ from misc@, as it's a LibreSSL issue.
>>
>> * Joel Sing [2017-03-05 23:01]:
>>
>> On Thursday 02 March 2017 13:28:08 Kirill Miazine wrote:
>>>
>>>> Recently I've noticed a number of error messages in my Exim mail log:
>>>>
>>>> TLS error on connection from mx1.slc.paypal.com (mx0.slc.paypal.com
>>>> )
>>>> [173.0.84.226] \ (SSL_accept): error:1403741B:SSL
>>>> routines:ACCEPT_SR_KEY_EXCH:tlsv1 alert decrypt error TLS client
>>>> disconnected cleanly (rejected our certificate?)
>>>>
>>>
>>> This is most likely the same issue as that reported on the libressl@
>>> mailing
>>> list a day or so ago - expect a fix to arrive shortly.
>>>
>>
>> I rebuilt exim on latest snapshot (OpenBSD 6.1-beta (GENERIC.MP) #213:
>> Mon Mar  6 12:31:59 MST 2017) and the error looks different now:
>>
>> TLS error on connection from mx0.phx.paypal.com [66.211.168.230] \
>>(SSL_accept): error:14039119:SSL routines:ACCEPT_SR_CERT_VRFY:decryption
>> \
>>failed or bad record mac
>>
>>
>> --
>>-- Kirill Miazine <k...@krot.org>
>>
>>
>


Re: tlsv1 alert decrypt error

2017-03-06 Thread Bob Beck
Move it to tech@ from misc.. not libressl.. libressl is not special ;)

On Mon, Mar 6, 2017 at 3:21 PM, Kirill Miazine  wrote:

> Moving to libressl@ from misc@, as it's a LibreSSL issue.
>
> * Joel Sing [2017-03-05 23:01]:
>
> On Thursday 02 March 2017 13:28:08 Kirill Miazine wrote:
>>
>>> Recently I've noticed a number of error messages in my Exim mail log:
>>>
>>> TLS error on connection from mx1.slc.paypal.com (mx0.slc.paypal.com)
>>> [173.0.84.226] \ (SSL_accept): error:1403741B:SSL
>>> routines:ACCEPT_SR_KEY_EXCH:tlsv1 alert decrypt error TLS client
>>> disconnected cleanly (rejected our certificate?)
>>>
>>
>> This is most likely the same issue as that reported on the libressl@
>> mailing
>> list a day or so ago - expect a fix to arrive shortly.
>>
>
> I rebuilt exim on latest snapshot (OpenBSD 6.1-beta (GENERIC.MP) #213:
> Mon Mar  6 12:31:59 MST 2017) and the error looks different now:
>
> TLS error on connection from mx0.phx.paypal.com [66.211.168.230] \
>(SSL_accept): error:14039119:SSL routines:ACCEPT_SR_CERT_VRFY:decryption
> \
>failed or bad record mac
>
>
> --
>-- Kirill Miazine 
>
>


Re: Scheduler ping-pong with preempt()

2017-02-06 Thread Bob Beck
Go for it mpi.. move forward.

ok beck@

On Mon, Feb 6, 2017 at 7:48 AM, Martin Pieuchot  wrote:

> On 24/01/17(Tue) 13:35, Martin Pieuchot wrote:
> > Userland threads are preempt()'d when hogging a CPU or when processing
> > an AST.  Currently when such a thread is preempted the scheduler looks
> > for an idle CPU and puts it on its run queue.  That means the number of
> > involuntary context switch often result in a migration.
> >
> > This is not a problem per se and one could argue that if another CPU
> > is idle it makes sense to move.  However with the KERNEL_LOCK() moving
> > to another CPU won't necessarily allows the preempt()'d thread to run.
> > It's even worse, it increases contention.
> >
> > If you add to this behavior the fact that sched_choosecpu() prefers idle
> > CPUs in a linear order, meaning CPU0 > CPU1 > .. > CPUN, you'll
> > understand that the set of idle CPUs will change every time preempt() is
> > called.
> >
> > I believe this behavior affects kernel threads by side effect, since
> > the set of idle CPU changes every time a thread is preempted.  With this
> > diff the 'softnet' thread didn't move on a 2 CPUs machine during simple
> > benchmarks.  Without, it plays ping-pong between CPU.
> >
> > The goal of this diff is to reduce the number of migrations.  You
> > can compare the value of 'sched_nomigrations' and 'sched_nmigrations'
> > with and without it.
> >
> > As usual, I'd like to know what's the impact of this diff on your
> > favorite benchmark.  Please test and report back.
>
> I only got positive test results so I'd like to commit the diff below.
>
> ok?
>
> Index: kern/sched_bsd.c
> ===
> RCS file: /cvs/src/sys/kern/sched_bsd.c,v
> retrieving revision 1.44
> diff -u -p -r1.44 sched_bsd.c
> --- kern/sched_bsd.c25 Jan 2017 06:15:50 -  1.44
> +++ kern/sched_bsd.c6 Feb 2017 14:47:28 -
> @@ -329,7 +329,6 @@ preempt(struct proc *newp)
> SCHED_LOCK(s);
> p->p_priority = p->p_usrpri;
> p->p_stat = SRUN;
> -   p->p_cpu = sched_choosecpu(p);
> setrunqueue(p);
> p->p_ru.ru_nivcsw++;
> mi_switch();
>
>


Re: Password corruption in adduser

2017-02-05 Thread Bob Beck
ok beck@
On Sun, Feb 5, 2017 at 22:53 Theo Buehler  wrote:

> On Sun, Feb 05, 2017 at 09:47:35PM -0800, Philip Guenther wrote:
> > On Sun, 5 Feb 2017, John McGuigan wrote:
> > > I've noticed something strange in adduser -- when attempting to add a
> > > user completely though command line argument it seems to corrupt the
> > > entry in /etc/master.passwd.
> > >
> > > Example:
> > >
> > > $ echo "HorseBatteryStaple" | encrypt
> > > $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2
> > >
> > > # adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh \
> > > -message no -batch some.user "" "Some User" \
> > > $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2
> > > Added user ``some.user''
> > ...
> > > some.user:b/bin/ksh9/9uoOrbTRaf//3ZprAb9k.hOpfe9vYVqjf1a:5000:5000:: \
> > > 0:0:Some User:/home/some.user:/bin/ksh
> > >
> > > As you can see the password entry gets corrupted with a 'b/bin/ksh...'
> >
> > Let's see what the adduser command is seeing by passing that all to
> 'echo'
> > instead:
> >
> > # echo \
> > > adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh \
> > > -message no -batch some.user "" "Some User" \
> > > $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2
> > adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh
> -message no -batch some.user  Some User b/bin/ksh9/FGXw.9oNjr3BLTS7DJp5n4M2
> > #
> >
> > Ah, so the expansion is happening *outside* of adduser...in the shell.
> > Yes, the shell does variable expansion even if the dollar-sign is in the
> > middle of a word, so it's expanding the variables
> >   $2  --> ""
> >   $0  --> "/bin/ksh"
> >   $ssZSLC6laHsTS7O2FwJ4Mufw6mSS   --> ""
> >
> >
> > > Behavior *is* present when hash is wrapped in "
> >
> > Sure, because double-quotes only stop file-globbing and field splitting
> > and not variable expansion.  You need single quotes for that:
> >
> > # echo \
> > > adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh \
> > > -message no -batch some.user "" "Some User" \
> > > '$2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2'
> > adduser -silent -noconfig -uid_start 5000 -group USER -shell ksh
> -message no -batch some.user  Some User
> $2b$09$ssZSLC6laHsTS7O2FwJ4Mufw6mSS/FGXw.9oNjr3BLTS7DJp5n4M2
> > #
>
> The adduser.8 manual page has an example with no quotes in it, so we
> should fix that.  Also, let's use a new hash using $2b$ instead of $2a$.
>
> Index: adduser.8
> ===
> RCS file: /var/cvs/src/usr.sbin/adduser/adduser.8,v
> retrieving revision 1.44
> diff -u -p -r1.44 adduser.8
> --- adduser.8   24 Dec 2015 16:54:37 -  1.44
> +++ adduser.8   6 Feb 2017 05:49:00 -
> @@ -373,7 +373,7 @@ The password has been created using
>  .Xr encrypt 1 :
>  .Bd -literal -offset indent
>  # adduser -batch falken guest,staff,beer 'Prof. Falken' \e
> -$2a$06$1Sdjxjoxg4cNmT6zAxriGOLgdLXQ3HdJ2dKBbzEk68jSrO1EtLJ3C
> +'$2b$10$aOadQNznQ1YJFnqNaRRneOvYvZAEO7atYiTND3EsLf6afHT5t1UIK'
>  .Ed
>  .Pp
>  Create user
>
>


Re: netcat: IPv6 address support for proxy

2017-02-04 Thread Bob Beck
ok beck@

On Sun, Feb 05, 2017 at 12:27:19AM +0100, Jeremie Courreges-Anglas wrote:
> 
> The colons used in IPv6 addresses conflicts with the proxy port
> specification.  Do the right thing for -x ::1:8080, [::1] and
> [::1]:8080.
> 
> ok?
> 
> 
> Index: netcat.c
> ===
> RCS file: /d/cvs/src/usr.bin/nc/netcat.c,v
> retrieving revision 1.171
> diff -u -p -p -u -r1.171 netcat.c
> --- netcat.c  30 Nov 2016 07:56:23 -  1.171
> +++ netcat.c  4 Feb 2017 23:24:42 -
> @@ -148,8 +148,8 @@ main(int argc, char *argv[])
>   struct servent *sv;
>   socklen_t len;
>   struct sockaddr_storage cliaddr;
> - char *proxy;
> - const char *errstr, *proxyhost = "", *proxyport = NULL;
> + char *proxy, *proxyport = NULL;
> + const char *errstr;
>   struct addrinfo proxyhints;
>   char unix_dg_tmp_socket_buf[UNIX_DG_TMP_SOCKET_SIZE];
>   struct tls_config *tls_cfg = NULL;
> @@ -426,15 +426,29 @@ main(int argc, char *argv[])
>   if (family == AF_UNIX)
>   errx(1, "no proxy support for unix sockets");
>  
> - /* XXX IPv6 transport to proxy would probably work */
> - if (family == AF_INET6)
> - errx(1, "no proxy support for IPv6");
> -
>   if (sflag)
>   errx(1, "no proxy support for local source address");
>  
> - proxyhost = strsep(, ":");
> - proxyport = proxy;
> + if (*proxy == '[') {
> + ++proxy;
> + proxyport = strchr(proxy, ']');
> + if (proxyport == NULL)
> + errx(1, "missing closing bracket in proxy");
> + *proxyport++ = '\0';
> + if (*proxyport == '\0')
> + /* Use default proxy port. */
> + proxyport = NULL;
> + else {
> + if (*proxyport == ':')
> + ++proxyport;
> + else
> + errx(1, "garbage proxy port delimiter");
> + }
> + } else {
> + proxyport = strrchr(proxy, ':');
> + if (proxyport != NULL)
> + *proxyport++ = '\0';
> + }
>  
>   memset(, 0, sizeof(struct addrinfo));
>   proxyhints.ai_family = family;
> @@ -617,7 +631,7 @@ main(int argc, char *argv[])
>   }
>   if (xflag)
>   s = socks_connect(host, portlist[i], hints,
> - proxyhost, proxyport, proxyhints, socksv,
> + proxy, proxyport, proxyhints, socksv,
>   Pflag);
>   else
>   s = remote_connect(host, portlist[i], hints);
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
> 



Re: Update for US Holidays.

2017-02-04 Thread Bob Beck

On Sat, Feb 04, 2017 at 01:52:14PM -0700, Bob Beck wrote:
> 
> Presented without further comment. 
> 
> ok? 
> 

Or maybe this is more appropriate:

Index: calendar.history
===
RCS file: /cvs/src/usr.bin/calendar/calendars/calendar.history,v
retrieving revision 1.80
diff -u -p -u -p -r1.80 calendar.history
--- calendar.history19 Nov 2016 12:41:22 -  1.80
+++ calendar.history4 Feb 2017 21:28:55 -
@@ -475,6 +475,7 @@
 12/12  First wireless message sent across Atlantic by Marconi, 1901
 12/13  Dartmouth College chartered, 1769
 12/14  Portugal joins the United Nations, 1955
+12/14  World totals the cost of running embedded 32 bit Linux, 1901
 12/15  Argo Merchant oil spill, 1976
 12/15  James Naismith invents basketball, Canada, 1891
 12/16  Pokemon episode (Electric Soldier Porygon) triggers attacks of



Re: Update for US Holidays.

2017-02-04 Thread Bob Beck
On Sat, Feb 04, 2017 at 12:59:53PM -0800, Philip Guenther wrote:
> On Sat, Feb 4, 2017 at 12:52 PM, Bob Beck <b...@obtuse.com> wrote:
> >
> > Presented without further comment.
> >
> > ok?
> 
> NACK.  Obsolete 32bit time_t OSes can track their own damn holidays.

But how will I remember to be appropriately devotional if my embedded
linux pacemaker didn't kill me the previous day?  :)



Update for US Holidays.

2017-02-04 Thread Bob Beck

Presented without further comment. 

ok? 

Index: calendar.usholiday
===
RCS file: /cvs/src/usr.bin/calendar/calendars/calendar.usholiday,v
retrieving revision 1.9
diff -u -p -u -p -r1.9 calendar.usholiday
--- calendar.usholiday  19 Jan 2015 18:07:47 -  1.9
+++ calendar.usholiday  4 Feb 2017 20:50:41 -
@@ -33,6 +33,7 @@
 11/SunFirstDaylight Saving Time ends; clocks move back (1st Sunday in 
November)
 11/11  Veterans' Day
 11/ThuFourth   Thanksgiving Day (4th Thursday in November)
+12/14  "National Day of Patriotic Devotion" (After 1901, for 32 bit time_t 
systems)
 12/21* Winter Solstice
 12/24  Christmas Eve
 12/25  Christmas



Re: specify curves via ecdhe statement in httpd.conf

2017-02-04 Thread Bob Beck
try connecting with openbsd nc rather than s-client

On Sat, Feb 4, 2017 at 09:13 Bob Beck <b...@obtuse.com> wrote:

>
> On Sat, Feb 4, 2017 at 07:51 Andreas Bartelt <o...@bartula.de> wrote:
>
> On 02/04/17 05:26, Joel Sing wrote:
> > On Wednesday 01 February 2017 15:41:29 Andreas Bartelt wrote:
> >> Hello,
> >>
> >> after reading the LibreSSL accouncement from today, I assumed that
> >> specifying ecdhe "auto" in /etc/httpd.conf would enable X25519, P-256
> >> and P-384 on current.
> >
> > This is correct.
> >
> >> I've noticed that "auto" enables only curves x25519 and P-256 (which is
> >> what I'd want to use - but somehow unexpected with regard to the
> >> announcement).
> >
> > Why do you believe this is the case?
> >
>
> Tested with a build of today's current:
> - httpd started with ecdhe "auto" in /etc/httpd.conf
> - then trying to connect via openssl s_client with -groups P-384 option
> doesn't negotiate a cipher suite.
>
> However, specifying -groups P-256 works. I don't know how to specify
> x25519 with OpenBSD's openssl s_client (it's not yet listed in openssl
> ecparam -list_curves output) but SSL Labs successfully negotiates via
> x25519 and P-256 (but not P-384). P-384 doesn't seem to be enabled with
> "auto".
>
> Another confusing test result:
> - httpd started with ecdhe "secp384r1" (P-384)
> - then trying to connect via openssl s_client with -groups P-384 option
> also doesn't negotiate a cipher suite!
>
> However, SSL Labs successfully connects to httpd and confirms support
> for secp384r1.
>
> Can you reproduce this?
>
> >> Diff is attached which clarifies the meaning of "auto" in httpd.conf.5.
> >
> > There are some documentation improvements that could be used here,
> however the
> > meaning of auto for httpd.conf.5 needs to refer to the meaning of "auto"
> for
> > libtls (currently tls_config_set_ecdhecurve()). Otherwise libtls changes
> and
> > httpd becomes out of date.
> >
> >> There currently seems to be no way to explicitly specify x25519, or to
> >> specify multiple colon separated curves with the ecdhe statement. Would
> >> it make sense to change semantics and make the ecdhe statement in
> >> httpd.conf consistent with the recent changes to openssl s_client
> >> -groups (e.g., to also allow more common names like P-256 instead of
> >> prime256v1)?
> >
> > Yes - tls_config_set_ecdhecurve() needs to change to accept the same
> colon
> > separate list of priority ordered curve names, that
> SSL_set1_curves_list()
> > accepts.
> >
> >
>
>


OpenBSD errata, Jan 31, 2017

2017-02-01 Thread Bob Beck
An issue has been identified whereby httpd(8) could be subject to a denial
of service attack. Repeated crafted requests could be made from a client
using file-range requests, making the server consume excessive amounts of
memory.

This issue has been fixed in current. For 5.9 and 6.0 the following errata
will disable range header processing in httpd(8) to prevent the problem.

Thanks to Pierre Kim  for reporting
the issue.

https://ftp.openbsd.org/pub/OpenBSD/patches/6.0/common/017_httpd.patch.sig

https://ftp.openbsd.org/pub/OpenBSD/patches/5.9/common/034_httpd.patch.sig


err with multiple TLS sites but one OCSP?

2017-01-28 Thread Bob Beck

Sooo.. 

Pretty sure mlucas has uncovered a problem with the ocsp interface. 

Basically I didn't attach it to the keypair, (yes Joel, I think you
told me so) so it only works with the master keypair.. OK, but the
problem is that it also returns the staple for other keypairs which is
wrong. 

This attaches the ocsp staple to the keypair, rather than the config. 

It does not yet add a way to change it for keypairs other than the
master - that will require an API change - but with this change
it should not return an incorrect ocsp staple for the non primary
keypair. I'll deal with the API change separately after pestering
joel about it a bit.

ok?

Index: tls_config.c
===
RCS file: /cvs/src/lib/libtls/tls_config.c,v
retrieving revision 1.34
diff -u -p -u -p -r1.34 tls_config.c
--- tls_config.c24 Jan 2017 01:48:05 -  1.34
+++ tls_config.c28 Jan 2017 21:40:14 -
@@ -101,6 +101,26 @@ tls_keypair_set_key_mem(struct tls_keypa
return set_mem(>key_mem, >key_len, key, len);
 }
 
+static int
+tls_keypair_set_ocsp_staple_file(struct tls_keypair *keypair,
+struct tls_error *error, const char *ocsp_file)
+{
+   if (keypair->ocsp_staple != NULL)
+   explicit_bzero(keypair->ocsp_staple, keypair->ocsp_staple_len);
+   return tls_config_load_file(error, "ocsp", ocsp_file,
+   >ocsp_staple, >ocsp_staple_len);
+}
+
+static int
+tls_keypair_set_ocsp_staple_mem(struct tls_keypair *keypair,
+const uint8_t *staple, size_t len)
+{
+   if (keypair->ocsp_staple != NULL)
+   explicit_bzero(keypair->ocsp_staple, keypair->ocsp_staple_len);
+   return set_mem(>ocsp_staple, >ocsp_staple_len, staple,
+   len);
+}
+
 static void
 tls_keypair_clear(struct tls_keypair *keypair)
 {
@@ -241,7 +261,6 @@ tls_config_free(struct tls_config *confi
free((char *)config->ca_mem);
free((char *)config->ca_path);
free((char *)config->ciphers);
-   free(config->ocsp_staple);
 
free(config);
 }
@@ -664,14 +683,14 @@ tls_config_verify_client_optional(struct
 int
 tls_config_set_ocsp_staple_file(struct tls_config *config, const char 
*staple_file)
 {
-   return tls_config_load_file(>error, "OCSP", staple_file,
-   >ocsp_staple, >ocsp_staple_len);
+   return tls_keypair_set_ocsp_staple_file(config->keypair, >error,
+   staple_file);
 }
 
 int
 tls_config_set_ocsp_staple_mem(struct tls_config *config, char *staple, size_t 
len)
 {
-   return set_mem(>ocsp_staple, >ocsp_staple_len, staple, 
len);
+   return tls_keypair_set_ocsp_staple_mem(config->keypair, staple, len);
 }
 
 int
Index: tls_internal.h
===
RCS file: /cvs/src/lib/libtls/tls_internal.h,v
retrieving revision 1.52
diff -u -p -u -p -r1.52 tls_internal.h
--- tls_internal.h  26 Jan 2017 12:56:37 -  1.52
+++ tls_internal.h  28 Jan 2017 21:07:25 -
@@ -51,6 +51,8 @@ struct tls_keypair {
size_t cert_len;
char *key_mem;
size_t key_len;
+   char *ocsp_staple;
+   size_t ocsp_staple_len;
 };
 
 #define TLS_MIN_SESSION_TIMEOUT (4)
@@ -83,8 +85,6 @@ struct tls_config {
int ecdhecurve;
struct tls_keypair *keypair;
int ocsp_require_stapling;
-   char *ocsp_staple;
-   size_t ocsp_staple_len;
uint32_t protocols;
unsigned char session_id[TLS_MAX_SESSION_ID_LENGTH];
int session_lifetime;
Index: tls_ocsp.c
===
RCS file: /cvs/src/lib/libtls/tls_ocsp.c,v
retrieving revision 1.10
diff -u -p -u -p -r1.10 tls_ocsp.c
--- tls_ocsp.c  27 Jan 2017 07:03:27 -  1.10
+++ tls_ocsp.c  28 Jan 2017 21:42:22 -
@@ -332,17 +332,19 @@ tls_ocsp_stapling_cb(SSL *ssl, void *arg
if ((ctx = SSL_get_app_data(ssl)) == NULL)
goto err;
 
-   if (ctx->config->ocsp_staple == NULL ||
-   ctx->config->ocsp_staple_len == 0)
+   if (ctx->config->keypair->ocsp_staple == NULL ||
+   ctx->config->keypair->ocsp_staple == NULL ||
+   ctx->config->keypair->ocsp_staple_len == 0)
return SSL_TLSEXT_ERR_NOACK;
 
-   if ((ocsp_staple = malloc(ctx->config->ocsp_staple_len)) == NULL)
+   if ((ocsp_staple = malloc(ctx->config->keypair->ocsp_staple_len)) ==
+   NULL)
goto err;
 
-   memcpy(ocsp_staple, ctx->config->ocsp_staple,
-   ctx->config->ocsp_staple_len);
+   memcpy(ocsp_staple, ctx->config->keypair->ocsp_staple,
+   ctx->config->keypair->ocsp_staple_len);
if (SSL_set_tlsext_status_ocsp_resp(ctx->ssl_conn, ocsp_staple,
-   ctx->config->ocsp_staple_len) != 1)
+   ctx->config->keypair->ocsp_staple_len) != 1)
goto err;
 
ret = SSL_TLSEXT_ERR_OK;



Re: err with multiple TLS sites but one OCSP?

2017-01-27 Thread Bob Beck
On Fri, Jan 27, 2017 at 15:23 Stuart Henderson <s...@spacehopper.org> wrote:

> On 2017/01/27 22:09, Bob Beck wrote:
>
> > I think you have more issues than ocsp. if thats the same host you can't
>
> > have two different tls certs on the same ip.   and you have them both on
>
> > *443
>
> >
>
> > try using a separate ip for each
>
>
>
> Wasn't SNI support added to httpd already?
>
> hmmm. right. but i bet itll work with explicit separate ip's.  stapling on
> the other hand will be per config. so thats probably whats fighting.
> separate ip would confirm that.


> im tired. ill look at it tomorrow unless someone else does
>
>
>


Re: err with multiple TLS sites but one OCSP?

2017-01-27 Thread Bob Beck
I think you have more issues than ocsp. if thats the same host you can't
have two different tls certs on the same ip.   and you have them both on
*443

try using a separate ip for each



On Fri, Jan 27, 2017 at 15:03 Michael W. Lucas <mwlu...@michaelwlucas.com>
wrote:

> On Fri, Jan 27, 2017 at 09:53:25PM +0000, Bob Beck wrote:
>
> >On Fri, Jan 27, 2017 at 14:12 Michael W. Lucas
>
> >  Or a misconfiguration. Â show configs
>
>
>
>
>
> Configs follow.
>
>
>
> # cat /etc/httpd.conf
>
> include "/etc/sites/www3.conf"
>
> include "/etc/sites/www4.conf"
>
>
>
> www3.conf:
>
>
>
> server "www3.mwlucas.org" {
>
>listen on * port 80
>
>block return 302 "https://$SERVER_NAME$REQUEST_URI;
>
> }
>
>
>
>
>
> server "www3.mwlucas.org" {
>
> alias tarpit.mwlucas.org
>
> listen on * tls port 443
>
> hsts
>
> # TLS certificate and key files created with acme-client(1)
>
> tls certificate "/etc/ssl/acme/www3/www3.fullchain.pem"
>
> tls key "/etc/ssl/acme/www3/www3.key"
>
> tls ocsp "/etc/ssl/acme/www3/www3.der"
>
> tcp nodelay
>
>
>
>location "/.well-known/acme-challenge/*" {
>
>root "/acme"
>
>root strip 2
>
>}
>
> }
>
>
>
>
>
> www4:
>
>
>
> server "www4.mwlucas.org" {
>
> alias bill.mwlucas.org
>
> alias auction.mwlucas.org
>
> listen on * port 80
>
>
>
>location "/.well-known/acme-challenge/*" {
>
>root "/acme"
>
>root strip 2
>
>}
>
>
>
>
>
> block return 301 "https://$DOCUMENT_URI;
>
> }
>
>
>
> server "www4.mwlucas.org" {
>
> alias bill.mwlucas.org
>
> alias auction.mwlucas.org
>
> root "/www4"
>
> listen on * tls port 443
>
> hsts
>
> # TLS certificate and key files created with acme-client(1)
>
> tls certificate "/etc/ssl/acme/www4/www4.fullchain.pem"
>
> tls key "/etc/ssl/acme/www4/www4.key"
>
> #   tls ocsp "/etc/ssl/acme/www4/www4.der"
>
> tcp nodelay
>
>location "/.well-known/acme-challenge/*" {
>
>root "/acme"
>
>root strip 2
>
>}
>
>
>
> }
>
>
>
>
>
>
>
>
>
> --
>
> Michael W. LucasTwitter @mwlauthor
>
> nonfiction: https://www.michaelwlucas.com/
>
> fiction: https://www.michaelwarrenlucas.com/
>
> blog: http://blather.michaelwlucas.com/
>
>


Re: err with multiple TLS sites but one OCSP?

2017-01-27 Thread Bob Beck
On Fri, Jan 27, 2017 at 14:12 Michael W. Lucas 
wrote:

> On Fri, Jan 27, 2017 at 02:50:29PM -0500, Michael W. Lucas wrote:
>
> > On Fri, Jan 27, 2017 at 06:49:06PM +, Stuart Henderson wrote:
>
> > > That looks like a web server bug, it shouldn't return a staple
>
>
> Or a misconfiguration.  show configs
>
>
> > > in that case.  What software are you using for that?
>
> >
>
> > 
>
> >
>
> > OpenBSD httpd, of course. amd64 snapshot downloaded yesterday from
>
> > ftp3.usa.openbsd.org.
>
>
>
> To be clear, that's a "How the hell could I forget to include that?"
>
> facepalm, not anything about Stuart asking the question...
>
>
>
> --
>
> Michael W. LucasTwitter @mwlauthor
>
> nonfiction: https://www.michaelwlucas.com/
>
> fiction: https://www.michaelwarrenlucas.com/
>
> blog: http://blather.michaelwlucas.com/
>
>
>
>


Re: Allow install from https server w/ self signed cert

2017-01-07 Thread Bob Beck

On Sat, Jan 07, 2017 at 03:52:04PM -0700, Theo de Raadt wrote:
> > What workarounds would be reasonable and approriate? and does it
> > make sense for OpenBSD to support such scenarios out-of-the-box to
> > promote wider adoption of better software?
> 
> If you want buy the OpenBSD-installer-for-drones, contact me offline.
> That featureset didn't make it into the free software version.


But out GeoLocation shit in the installer will just find that it's
located on a netblock that is registered 5000 feet over Pakistan and
direct it to a local public mirror!  Definately have to disable
that feature from the free software version.







Re: Allow install from https server w/ self signed cert

2017-01-07 Thread Bob Beck



On Sat, Jan 07, 2017 at 05:42:24PM -0500, Jacob L. Leifman wrote:

> Most of the time I agree with this particular attitude and it is indeed 
> appropriate for the OP case. However, there some major networks such as 
> various governments (or for example .mil) that do not participate in 
> the public PKI but run their own PKI infrastructure. What effect will 
> the installer's checks have in that environment? What workarounds would 
> be reasonable and approriate? and does it make sense for OpenBSD to 
> support such scenarios out-of-the-box to promote wider adoption of 
> better software?

That's not a good reason, since in the case of what I am describing they
would *still be depending on the public PKI infrastrucure* to publish
their own list of signers..  No.. they aren't going to do that.. Sorry, 
Unless you're mailing me from a .mil address I think you might
be imagining this usage case. 

they're probably building from source, or have the wherewithall to roll
their own bsd.rd if they care about doing this. 





Re: Allow install from https server w/ self signed cert

2017-01-07 Thread Bob Beck

On Fri, Jan 06, 2017 at 10:48:37AM -0500, RD Thrush wrote:
> On 01/06/17 06:28, Stuart Henderson wrote:
> > Related to this (and particularly thinking about autoinstalls),
> > would it make sense to allow explicit protocols in the hostname?
> > 
> > some.host -> https with http fallback
> > http://some.host/ -> http only
> > https://some.host/ -> https only, no fallback
> 
> That would totally work for my install problem.
> 
> FWIW, instead of running a patched install.sub, "rm /etc/ssl/cert.pem" makes 
> the install bypass the https attempt.
> 

Note, if you're upgrading or otherwise have a way to et a cert.pem bundle onto 
there to *replace*
the default, you could always drop the signer for your private self-signed 
server into the cert.pem
bundle, at which point it would be accepted as trusted. 

of course if you're just installing you have an interesting chicken and egg 
problem, unless
you put it somewhere on an https site that does have a real certificate, drop 
out of the
installer and do

ftp -o /tmp/mysigner.pem https://my.secure.site/mysigner.pem
cat /tmp/mysigner.pem >> /etc/ssl/cert.pem

then continue the install, and you're good. 

Almost wonder if it's worth an extra question in the installer to ask
for an https address to retrieve a certficiate bundle to be appended to cert.pem
for the install...







Re: acme-client use configuration file [1 of 5]

2017-01-02 Thread Bob Beck
No objection in principle.. although since some of us depend on this we
might either need warning and/or a small period of overlap where the old
stuff works and then we can move to the new stuff without things blowing
up.

On Sun, Jan 1, 2017 at 1:59 PM, Sebastian Benoit  wrote:

> start using the configuration file and delete command line arguments:
>
> -a agreement-> agreement url ...
> -c certdir  -> domain certificate "path"
> -f accountkey   -> account key "path"
> -k domainkey-> domain key "path"
> -s authority-> sign with "name"
>
> new argument:
> -f configfile
>
> the changes needed to use the new configuration are local to main.c for
> now.
> While the configuration could be passed directly to netproc(), keyproc()
> etc,
> the diff is smaller this way.
>
> This also removes the multidir (-m) mode for now - specify different paths
> in
> each domain {} block instead.
>
> diff --git usr.sbin/acme-client/Makefile usr.sbin/acme-client/Makefile
> index 55e0b0e..eae13ed 100644
> --- usr.sbin/acme-client/Makefile
> +++ usr.sbin/acme-client/Makefile
> @@ -13,6 +13,6 @@ CFLAGS+=  -W -Wall -I${.CURDIR}
>  CFLAGS+=   -Wstrict-prototypes -Wmissing-prototypes
>  CFLAGS+=   -Wmissing-declarations
>  CFLAGS+=   -Wshadow -Wpointer-arith
> -CFLAGS+=   -Wsign-compare
> +CFLAGS+=   -Wsign-compare -Wunused
>
>  .include 
> diff --git usr.sbin/acme-client/acme-client.1 usr.sbin/acme-client/acme-
> client.1
> index 526c11f..6f38573 100644
> --- usr.sbin/acme-client/acme-client.1
> +++ usr.sbin/acme-client/acme-client.1
> @@ -22,15 +22,10 @@
>  .Nd ACME client
>  .Sh SYNOPSIS
>  .Nm acme-client
> -.Op Fl bFmNnrv
> -.Op Fl a Ar agreement
> +.Op Fl bFNnrv
>  .Op Fl C Ar challengedir
> -.Op Fl c Ar certdir
> -.Op Fl f Ar accountkey
> -.Op Fl k Ar domainkey
> -.Op Fl s Ar authority
> +.Op Fl f Ar configfile
>  .Ar domain
> -.Op Ar altnames
>  .Sh DESCRIPTION
>  The
>  .Nm
> @@ -39,8 +34,6 @@ Automatic Certificate Management Environment (ACME)
> client.
>  .Pp
>  The options are as follows:
>  .Bl -tag -width Ds
> -.It Fl a Ar agreement
> -Use an alternative user agreement URL.
>  .It Fl b
>  Back up all certificates in the certificate directory.
>  This only happens if a remove or replace operation is possible.
> @@ -58,67 +51,21 @@ Any given backup uses the same Epoch time for all
> three certificates.
>  If there are no certificates in place, this option does nothing.
>  .It Fl C Ar challengedir
>  The directory to register challenges.
> -.It Fl c Ar certdir
> -The directory to store public certificates.
>  .It Fl F
>  Force updating the certificate signature even if it's too soon.
> -.It Fl f Ar accountkey
> -The account private key.
> -This was either made with a previous client or with
> -.Fl n .
> -.It Fl k Ar domainkey
> -The private key for the domain.
> -This may also be created with
> -.Fl N .
> -.It Fl m
> -Append
> -.Ar domain
> -to all default paths except the challenge path
> -.Pq i.e. those that are overridden by Fl c , k , f .
> -Thus,
> -.Ar foo.com
> -as the initial domain would make the default domain private key into
> -.Pa /etc/ssl/acme/private/foo.com/privkey.pem .
> -This is useful in setups with multiple domain sets.
> +.It Fl f Ar configfile
> +Specify an alternative configuration file.
>  .It Fl N
>  Create a new RSA domain key if one does not already exist.
>  .It Fl n
>  Create a new RSA account key if one does not already exist.
>  .It Fl r
>  Revoke the X509 certificate found in the certificates.
> -.It Fl s Ar authority
> -ACME
> -.Ar authority
> -to talk to.
> -Currently the following authorities are available:
> -.Pp
> -.Bl -tag -width "letsencrypt-staging" -compact
> -.It Cm letsencrypt
> -Let's Encrypt authority
> -.It Cm letsencrypt-staging
> -Let's Encrypt staging authority
> -.El
> -.Pp
> -The default is
> -.Cm letsencrypt .
>  .It Fl v
>  Verbose operation.
>  Specify twice to also trace communication and data transfers.
>  .It Ar domain
>  The domain name.
> -The only difference between this and
> -.Ar altnames
> -is that it's put into the certificate's
> -.Li CN
> -field and it uses the
> -.Qq main
> -domain when specifying
> -.Fl m .
> -.It Ar altnames
> -Alternative names
> -.Pq Dq SAN
> -for the domain name.
> -The number of SAN entries is limited to 100 or so.
>  .El
>  .Pp
>  Public certificates are by default placed in
> @@ -175,7 +122,7 @@ as in the
>  .Sx Challenges
>  section:
>  .Pp
> -.Dl # acme-client -vNn foo.com www.foo.com smtp.foo.com
> +.Dl # acme-client -vNn www.foo.com
>  .Pp
>  A daily
>  .Xr cron 8
> @@ -183,7 +130,7 @@ job can renew the certificates:
>  .Bd -literal -offset indent
>  #! /bin/sh
>
> -acme-client foo.com www.foo.com smtp.foo.com
> +acme-client www.foo.com
>
>  if [ $? -eq 0 ]
>  then
> diff --git usr.sbin/acme-client/chngproc.c usr.sbin/acme-client/chngproc.c
> index 4cb7f33..3e931da 100644
> --- usr.sbin/acme-client/chngproc.c
> +++ usr.sbin/acme-client/chngproc.c
> @@ -27,7 +27,7 @@
>  

Re: libtls syslogd pledge abort

2016-12-29 Thread Bob Beck



> Or do not call tls_configure_ssl_verify() if verification is turned
> off.

This makes sense to me. 

> 
> Index: lib/libtls/tls_client.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/lib/libtls/tls_client.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 tls_client.c
> --- lib/libtls/tls_client.c   26 Dec 2016 16:20:58 -  1.38
> +++ lib/libtls/tls_client.c   29 Dec 2016 22:56:23 -
> @@ -195,7 +195,9 @@ tls_connect_common(struct tls *ctx, cons
>   }
>   }
>  
> - if (tls_configure_ssl_verify(ctx, ctx->ssl_ctx, SSL_VERIFY_PEER) == -1)
> + if (ctx->config->verify_cert &&
> + (tls_configure_ssl_verify(ctx, ctx->ssl_ctx,
> +  SSL_VERIFY_PEER) == -1))
>   goto err;
>  
>   if (SSL_CTX_set_tlsext_status_cb(ctx->ssl_ctx, tls_ocsp_verify_cb) != 
> 1) {
> 

ok beck@

> I would prefer the fix in libtls as
> - this problem may also affect other daemons
> - avoid to do unnecsessary stuff
> - syslogd could run on a system without cert.pem
> 
> comments? ok?
> 
> bluhm



Re: httpd(8)/proc.c: use less fds on startup

2016-10-07 Thread Bob Beck

This is now working on www.openbsd.org.  I upgraded my 
6.0 system to current today off the latest snap and httpd would
not start, same problem. 

This diff lets current httpd start again. 

ok beck@


On Tue, Oct 04, 2016 at 11:54:37PM +0200, Rafael Zalamena wrote:
> On Tue, Oct 04, 2016 at 07:46:52PM +0200, Rafael Zalamena wrote:
> > This diff makes proc.c daemons to use less file descriptors on startup,
> > this way we increase the number of child we can have considerably. This
> > also improves the solution on a bug reported in bugs@
> > "httpd errors out with 'too many open files'". 
> > 
> > To achieve that I delayed the socket distribution and made a minimal
> > socket allocation in proc_init(), so only the necessary children socket
> > are allocated and passed with proc_exec(). After the event_init() is called
> > we call proc_connect() which creates the socketpair() and immediatly after
> > each call we already sends them without accumulating.
> > 
> > Note: We still have to calculate how many fds we will want to have and
> >   then limit the daemon prefork configuration.
> > 
> > ok?
> > 
> 
> Paul de Weerd still found problems with the diff, because the httpd(8)
> would not exit successfully with '-n' flag. It happened because the
> new proc_connect() code tried to write fds to children process that
> did not start caused by the ps_noaction flag. (thanks Paul!)
> 
> This new diff just adds a check for ps_noaction in proc_init() and
> proc_connect() so it doesn't try to do anything if we are not really
> going to start the daemon. Also I removed the ps_noaction from proc_run()
> since we are not going to get to this point anymore.
> 
> ok?
> 
> 
> Index: proc.c
> ===
> RCS file: /home/obsdcvs/src/usr.sbin/httpd/proc.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 proc.c
> --- proc.c28 Sep 2016 12:01:04 -  1.27
> +++ proc.c4 Oct 2016 21:50:43 -
> @@ -36,8 +36,6 @@
>  
>  void  proc_exec(struct privsep *, struct privsep_proc *, unsigned int,
>   int, char **);
> -void  proc_connectpeer(struct privsep *, enum privsep_procid, int,
> - struct privsep_pipes *);
>  void  proc_setup(struct privsep *, struct privsep_proc *, unsigned int);
>  void  proc_open(struct privsep *, int, int);
>  void  proc_accept(struct privsep *, int, enum privsep_procid,
> @@ -147,72 +145,18 @@ proc_exec(struct privsep *ps, struct pri
>  }
>  
>  void
> -proc_connectpeer(struct privsep *ps, enum privsep_procid id, int inst,
> -struct privsep_pipes *pp)
> -{
> - unsigned int i, j;
> - struct privsep_fdpf;
> -
> - for (i = 0; i < PROC_MAX; i++) {
> - /* Parent is already connected with everyone. */
> - if (i == PROC_PARENT)
> - continue;
> -
> - for (j = 0; j < ps->ps_instances[i]; j++) {
> - /* Don't send socket to child itself. */
> - if (i == (unsigned int)id &&
> - j == (unsigned int)inst)
> - continue;
> - if (pp->pp_pipes[i][j] == -1)
> - continue;
> -
> - pf.pf_procid = i;
> - pf.pf_instance = j;
> - proc_compose_imsg(ps, id, inst, IMSG_CTL_PROCFD,
> - -1, pp->pp_pipes[i][j], , sizeof(pf));
> - pp->pp_pipes[i][j] = -1;
> - }
> - }
> -}
> -
> -/* Inter-connect all process except with ourself. */
> -void
>  proc_connect(struct privsep *ps)
>  {
> - unsigned int src, i, j;
> - struct privsep_pipes*pp;
> - struct imsgev   *iev;
> -
> - /* Listen on appropriate pipes. */
> - src = privsep_process;
> - pp = >ps_pipes[src][ps->ps_instance];
> -
> - for (i = 0; i < PROC_MAX; i++) {
> - /* Don't listen to ourself. */
> - if (i == src)
> - continue;
> -
> - for (j = 0; j < ps->ps_instances[i]; j++) {
> - if (pp->pp_pipes[i][j] == -1)
> - continue;
> -
> - iev = >ps_ievs[i][j];
> - imsg_init(>ibuf, pp->pp_pipes[i][j]);
> - event_set(>ev, iev->ibuf.fd, iev->events,
> - iev->handler, iev->data);
> - event_add(>ev, NULL);
> - }
> - }
> + unsigned int src, dst;
>  
> - /* Exchange pipes between process. */
> - for (i = 0; i < PROC_MAX; i++) {
> - /* Parent is already connected with everyone. */
> - if (i == PROC_PARENT)
> - continue;
> + /* Don't distribute any sockets if we are not really going to run. */
> + if (ps->ps_noaction)
> + return;
>  
> - for (j = 0; j < ps->ps_instances[i]; j++)
> - 

Re: rebound quantum entanglement

2016-09-14 Thread Bob Beck
BTW I'm not picking on you.. my DNS setup blew up this week for local
resolution and I've been dealing with the fallout - so the topic
is relatively near and dear to my heart.

On Wed, Sep 14, 2016 at 10:07 PM, Bob Beck <b...@obtuse.com> wrote:

>
> Yep.  and now you need to solve the problem that when prepending
> 127.0.0.1, and hitting rebound, which in turn is going to only grab the
> first dns server from my resolv.conf instead of all of them, that it now
> doubles my failure time when the first dns server doesn't respond (once for
> libc asking rebound, which asks only the first server it found, which
> fails) then falls back to libc asking resolv.conf which again... asks the
> first server, and fails again.
>
> So the problem here is that it's going to be great when things are working
> but become a failure multiplier when something breaks.
>
> Of course - perhaps you could query them all in parallel to mitigate this?
>   Rebound might need to become a real boy if we're going to do something
> like this.  If rebound were a "real boy" it would be it easier to say "just
> use rebound or if it's not there just use resolv.conf normally
>
> then nothing changes at *all* when it's not there.
>
>
> On Wed, Sep 14, 2016 at 8:39 PM, Ted Unangst <t...@tedunangst.com> wrote:
>
>> Ted Unangst wrote:
>> > Bob Beck wrote:
>> > > how is rebound going to handle a change in resolv.conf? thats still a
>> > > problem here
>> >
>> > oh, that's easy. it watches the file for changes. i never quite got
>> around to
>> > that, but it's another five lines.
>>
>> ok, so it's a net +15 lines, including blanks.
>>
>> Index: rebound.8
>> ===
>> RCS file: /cvs/src/usr.sbin/rebound/rebound.8,v
>> retrieving revision 1.4
>> diff -u -p -r1.4 rebound.8
>> --- rebound.8   4 Dec 2015 04:50:43 -   1.4
>> +++ rebound.8   15 Sep 2016 00:57:21 -
>> @@ -33,9 +33,7 @@ The options are as follows:
>>  .Bl -tag -width Ds
>>  .It Fl c Ar config
>>  Specify an alternative configuration file, instead of the default
>> -.Pa /etc/rebound.conf .
>> -At present, the config file consists of a single line containing the next
>> -hop DNS server.
>> +.Pa /etc/resolv.conf .
>>  .Nm
>>  will reload the configuration file when sent a SIGHUP signal.
>>  .It Fl d
>> @@ -46,8 +44,8 @@ does not
>>  into the background.
>>  .El
>>  .Sh FILES
>> -.Bl -tag -width "/etc/rebound.confXX" -compact
>> -.It Pa /etc/rebound.conf
>> +.Bl -tag -width "/etc/resolv.confXX" -compact
>> +.It Pa /etc/resolv.conf
>>  Default
>>  .Nm
>>  configuration file.
>> Index: rebound.c
>> ===
>> RCS file: /cvs/src/usr.sbin/rebound/rebound.c,v
>> retrieving revision 1.70
>> diff -u -p -r1.70 rebound.c
>> --- rebound.c   1 Sep 2016 10:57:24 -   1.70
>> +++ rebound.c   15 Sep 2016 02:30:46 -
>> @@ -33,10 +33,12 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>
>>  #define MINIMUM(a,b) (((a)<(b))?(a):(b))
>>
>> @@ -455,34 +457,51 @@ fail:
>>  }
>>
>>  static int
>> -readconfig(FILE *conf, union sockun *remoteaddr)
>> +readconfig(int conffd, union sockun *remoteaddr)
>>  {
>> +   const char ns[] = "nameserver";
>> char buf[1024];
>> +   char *p;
>> struct sockaddr_in *sin = >i;
>> struct sockaddr_in6 *sin6 = >i6;
>> +   FILE *conf;
>> +   int rv = -1;
>>
>> -   if (fgets(buf, sizeof(buf), conf) == NULL)
>> -   return -1;
>> -   buf[strcspn(buf, "\n")] = '\0';
>> +   conf = fdopen(conffd, "r");
>>
>> -   memset(remoteaddr, 0, sizeof(*remoteaddr));
>> -   if (inet_pton(AF_INET, buf, >sin_addr) == 1) {
>> -   sin->sin_len = sizeof(*sin);
>> -   sin->sin_family = AF_INET;
>> -   sin->sin_port = htons(53);
>> -   return AF_INET;
>> -   } else if (inet_pton(AF_INET6, buf, >sin6_addr) == 1) {
>> -   sin6->sin6_len = sizeof(*sin6);
>> -   sin6->sin6_family = AF_INET6;
>> -   sin6->sin6_port = htons(53);
>> -   return AF_INET6;
>> -   } else {
&

Re: rebound quantum entanglement

2016-09-14 Thread Bob Beck
Yep.  and now you need to solve the problem that when prepending 127.0.0.1,
and hitting rebound, which in turn is going to only grab the first dns
server from my resolv.conf instead of all of them, that it now doubles my
failure time when the first dns server doesn't respond (once for libc
asking rebound, which asks only the first server it found, which fails)
then falls back to libc asking resolv.conf which again... asks the first
server, and fails again.

So the problem here is that it's going to be great when things are working
but become a failure multiplier when something breaks.

Of course - perhaps you could query them all in parallel to mitigate this?
  Rebound might need to become a real boy if we're going to do something
like this.  If rebound were a "real boy" it would be it easier to say "just
use rebound or if it's not there just use resolv.conf normally

then nothing changes at *all* when it's not there.


On Wed, Sep 14, 2016 at 8:39 PM, Ted Unangst <t...@tedunangst.com> wrote:

> Ted Unangst wrote:
> > Bob Beck wrote:
> > > how is rebound going to handle a change in resolv.conf? thats still a
> > > problem here
> >
> > oh, that's easy. it watches the file for changes. i never quite got
> around to
> > that, but it's another five lines.
>
> ok, so it's a net +15 lines, including blanks.
>
> Index: rebound.8
> ===
> RCS file: /cvs/src/usr.sbin/rebound/rebound.8,v
> retrieving revision 1.4
> diff -u -p -r1.4 rebound.8
> --- rebound.8   4 Dec 2015 04:50:43 -   1.4
> +++ rebound.8   15 Sep 2016 00:57:21 -
> @@ -33,9 +33,7 @@ The options are as follows:
>  .Bl -tag -width Ds
>  .It Fl c Ar config
>  Specify an alternative configuration file, instead of the default
> -.Pa /etc/rebound.conf .
> -At present, the config file consists of a single line containing the next
> -hop DNS server.
> +.Pa /etc/resolv.conf .
>  .Nm
>  will reload the configuration file when sent a SIGHUP signal.
>  .It Fl d
> @@ -46,8 +44,8 @@ does not
>  into the background.
>  .El
>  .Sh FILES
> -.Bl -tag -width "/etc/rebound.confXX" -compact
> -.It Pa /etc/rebound.conf
> +.Bl -tag -width "/etc/resolv.confXX" -compact
> +.It Pa /etc/resolv.conf
>  Default
>  .Nm
>  configuration file.
> Index: rebound.c
> ===
> RCS file: /cvs/src/usr.sbin/rebound/rebound.c,v
> retrieving revision 1.70
> diff -u -p -r1.70 rebound.c
> --- rebound.c   1 Sep 2016 10:57:24 -   1.70
> +++ rebound.c   15 Sep 2016 02:30:46 -
> @@ -33,10 +33,12 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #define MINIMUM(a,b) (((a)<(b))?(a):(b))
>
> @@ -455,34 +457,51 @@ fail:
>  }
>
>  static int
> -readconfig(FILE *conf, union sockun *remoteaddr)
> +readconfig(int conffd, union sockun *remoteaddr)
>  {
> +   const char ns[] = "nameserver";
> char buf[1024];
> +   char *p;
> struct sockaddr_in *sin = >i;
> struct sockaddr_in6 *sin6 = >i6;
> +   FILE *conf;
> +   int rv = -1;
>
> -   if (fgets(buf, sizeof(buf), conf) == NULL)
> -   return -1;
> -   buf[strcspn(buf, "\n")] = '\0';
> +   conf = fdopen(conffd, "r");
>
> -   memset(remoteaddr, 0, sizeof(*remoteaddr));
> -   if (inet_pton(AF_INET, buf, >sin_addr) == 1) {
> -   sin->sin_len = sizeof(*sin);
> -   sin->sin_family = AF_INET;
> -   sin->sin_port = htons(53);
> -   return AF_INET;
> -   } else if (inet_pton(AF_INET6, buf, >sin6_addr) == 1) {
> -   sin6->sin6_len = sizeof(*sin6);
> -   sin6->sin6_family = AF_INET6;
> -   sin6->sin6_port = htons(53);
> -   return AF_INET6;
> -   } else {
> -   return -1;
> +   while (fgets(buf, sizeof(buf), conf) != NULL) {
> +   buf[strcspn(buf, "\n")] = '\0';
> +
> +   if (strncmp(buf, ns, strlen(ns)) != 0)
> +   continue;
> +   p = buf + strlen(ns) + 1;
> +   while (isspace((unsigned char)*p))
> +   p++;
> +
> +   /* this will not end well */
> +   if (strcmp(p, "127.0.0.1") == 0)
> +   continue;
> +
> +   memset(remoteaddr, 0, sizeof(*remoteaddr));
> +   if (inet_pton(AF_INET, p, >sin_addr) == 1) {
> +

Re: rebound quantum entanglement

2016-09-14 Thread Bob Beck
wont this also mean if it is not running i have to wait for the localhost
attempt to fail before the resolver moves on? (ASR_STATE_NEXT_NS, etc) so i
slow everything down for a timeout?

dont get me wrong, it is an interesting direction, but I think maybe get
the rest of the five line changes into rebound to make it useful and then
look at libc which might need slightly more cleverness than just adding
localhost unconditionally.

On Wednesday, 14 September 2016, Ted Unangst <t...@tedunangst.com> wrote:

> Bob Beck wrote:
> > how is rebound going to handle a change in resolv.conf? thats still a
> > problem here
>
> oh, that's easy. it watches the file for changes. i never quite got around
> to
> that, but it's another five lines.
>


Re: rebound quantum entanglement

2016-09-14 Thread Bob Beck
how is rebound going to handle a change in resolv.conf? thats still a
problem here

On Wednesday, 14 September 2016, Ted Unangst  wrote:

> So the plan is for rebound to be the 'system' resolver, with libc talking
> to
> rbeound and rebound talking to the cloud. The main wrinkle is how does
> rebound
> find the cloud? rebound.conf, but dhclient doesn't know anything about
> rebound.conf, preferring to edit resolv.conf. But if rebound reads
> resolv.conf, what does libc read? This has been a bit of a tangle until
> now,
> especially in scenarios like upgrades where rebound may not even be
> running.
>
> And so I present the following diff to enable a smooth transition. It's
> 'quantum' because it works whether or not rebound is running. No need to
> open
> the box.
>
> 1. rebound reads resolv.conf. This remains the config file for upstream
> DNS.
>
> 2. libc now prepends its nameserver list with localhost, thus always
> searching
> for rebound. If it's not running, we just continue down the list.
>
> This covers the basic use case, where enabling rebound now requires no
> additional work. No need to edit dhclient.conf, etc. It also works on
> ramdisks. It also works with a mix of old and new binaries. Once you flip
> resolv.conf back to upstream, old binaries will bypass rebound, but that's
> ok.
> The new rebound checks to make sure it's not stuck in a time loop, which is
> never good.
>
> I also note this improves the situation for people who have been using
> unbound
> as a local cache, too. Just enable unbound and libc will use it
> automatically.
>
> Particular edge case: if resolv.conf has no nameservers, then the localhost
> default is not prepended. So libc won't try talking to rebound if it's
> specifically configured not to (chroot).
>
>
> Index: lib/libc/asr/asr.c
> ===
> RCS file: /cvs/src/lib/libc/asr/asr.c,v
> retrieving revision 1.54
> diff -u -p -r1.54 asr.c
> --- lib/libc/asr/asr.c  18 Jun 2016 15:25:28 -  1.54
> +++ lib/libc/asr/asr.c  15 Sep 2016 00:42:30 -
> @@ -549,6 +549,15 @@ pass0(char **tok, int n, struct asr_ctx
> return;
> if (n != 2)
> return;
> +   /* prepend localhost to list */
> +   if (ac->ac_nscount == 0) {
> +   if (asr_parse_nameserver((struct sockaddr *),
> "127.0.0.1"))
> +   return;
> +   if ((ac->ac_ns[ac->ac_nscount] = calloc(1,
> ss.ss_len)) == NULL)
> +   return;
> +   memmove(ac->ac_ns[ac->ac_nscount], ,
> ss.ss_len);
> +   ac->ac_nscount += 1;
> +   }
> if (asr_parse_nameserver((struct sockaddr *), tok[1]))
> return;
> if ((ac->ac_ns[ac->ac_nscount] = calloc(1, ss.ss_len)) ==
> NULL)
> Index: usr.sbin/rebound/rebound.8
> ===
> RCS file: /cvs/src/usr.sbin/rebound/rebound.8,v
> retrieving revision 1.4
> diff -u -p -r1.4 rebound.8
> --- usr.sbin/rebound/rebound.8  4 Dec 2015 04:50:43 -   1.4
> +++ usr.sbin/rebound/rebound.8  15 Sep 2016 00:57:21 -
> @@ -33,9 +33,7 @@ The options are as follows:
>  .Bl -tag -width Ds
>  .It Fl c Ar config
>  Specify an alternative configuration file, instead of the default
> -.Pa /etc/rebound.conf .
> -At present, the config file consists of a single line containing the next
> -hop DNS server.
> +.Pa /etc/resolv.conf .
>  .Nm
>  will reload the configuration file when sent a SIGHUP signal.
>  .It Fl d
> @@ -46,8 +44,8 @@ does not
>  into the background.
>  .El
>  .Sh FILES
> -.Bl -tag -width "/etc/rebound.confXX" -compact
> -.It Pa /etc/rebound.conf
> +.Bl -tag -width "/etc/resolv.confXX" -compact
> +.It Pa /etc/resolv.conf
>  Default
>  .Nm
>  configuration file.
> Index: usr.sbin/rebound/rebound.c
> ===
> RCS file: /cvs/src/usr.sbin/rebound/rebound.c,v
> retrieving revision 1.70
> diff -u -p -r1.70 rebound.c
> --- usr.sbin/rebound/rebound.c  1 Sep 2016 10:57:24 -   1.70
> +++ usr.sbin/rebound/rebound.c  15 Sep 2016 00:53:26 -
> @@ -37,6 +37,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #define MINIMUM(a,b) (((a)<(b))?(a):(b))
>
> @@ -457,28 +458,41 @@ fail:
>  static int
>  readconfig(FILE *conf, union sockun *remoteaddr)
>  {
> +   const char ns[] = "nameserver";
> char buf[1024];
> +   char *p;
> struct sockaddr_in *sin = >i;
> struct sockaddr_in6 *sin6 = >i6;
>
> -   if (fgets(buf, sizeof(buf), conf) == NULL)
> -   return -1;
> -   buf[strcspn(buf, "\n")] = '\0';
> +   while (fgets(buf, sizeof(buf), conf) != NULL) {
> +   buf[strcspn(buf, "\n")] = '\0';
>
> -   memset(remoteaddr, 0, sizeof(*remoteaddr));
> -

Re: reduce double caching in mfs

2016-09-09 Thread Bob Beck
I really dislike "CHEAP".

and it almost seems like these should actually be NOCACHE.. why the heck
can't they be?


On Thu, Sep 8, 2016 at 7:49 PM, Ted Unangst  wrote:

> Currently, the bufcache doesn't know that mfs is backed by memory. All i/o
> to
> mfs ends up being double cached, once in the userland process and again in
> the
> kernel bufcache. This is wasteful. In particular, it means one can't use
> mfs
> to increase the effective size of the buffer cache. Reading or writing to
> mfs
> will push out buffers used for disk caching. (I think you can even end up
> with
> triple buffering when mfs starts swapping...)
>
> This is mostly inherent to the design of mfs. But with a small tweak to the
> buffer cache, we can improve the situation. This introduces the concept of
> 'cheap' buffers, a hint to the buffer cache that they are easily replaced.
> (There's a 'nocache' flag already, but it's not suitable here.) When mfs
> finishes with a buf, it marks it cheap. Then it goes onto a special queue
> that
> gets chewed up before we start looking at the regular caches. We still
> cache
> some number of cheap buffers to prevent constant memory copying.
>
> With this diff, I've confirmed that reading/writing large files to mfs
> doesn't
> flush the cache, but performance appears about the same. (Of particular
> note,
> my bufcache is big enough to cache all of src/, but not src/ and obj/. With
> obj/ on mfs, src never gets flushed.)
>
>
> Index: kern/vfs_bio.c
> ===
> RCS file: /cvs/src/sys/kern/vfs_bio.c,v
> retrieving revision 1.176
> diff -u -p -r1.176 vfs_bio.c
> --- kern/vfs_bio.c  4 Sep 2016 10:51:24 -   1.176
> +++ kern/vfs_bio.c  8 Sep 2016 18:31:52 -
> @@ -93,7 +93,10 @@ int bd_req;  /* Sleep point for cleaner
>
>  #define NUM_CACHES 2
>  #define DMA_CACHE 0
> +#define CHEAP_LIMIT 256
>  struct bufcache cleancache[NUM_CACHES];
> +struct bufqueue cheapqueue;
> +u_int cheapqueuelen;
>  struct bufqueue dirtyqueue;
>
>  void
> @@ -1297,6 +1300,7 @@ bufcache_init(void)
> TAILQ_INIT([i].coldqueue);
> TAILQ_INIT([i].warmqueue);
> }
> +   TAILQ_INIT();
> TAILQ_INIT();
>  }
>
> @@ -1329,6 +1333,12 @@ bufcache_getcleanbuf(int cachenum, int d
>
> splassert(IPL_BIO);
>
> +   /* try cheap queue if over limit */
> +   if (discard || cheapqueuelen > CHEAP_LIMIT) {
> +   if ((bp = TAILQ_FIRST()))
> +   return bp;
> +   }
> +
> /* try  cold queue */
> while ((bp = TAILQ_FIRST(>coldqueue))) {
> if ((!discard) &&
> @@ -1356,6 +1366,8 @@ bufcache_getcleanbuf(int cachenum, int d
> /* buffer is cold - give it up */
> return bp;
> }
> +   if ((bp = TAILQ_FIRST()))
> +   return bp;
> if ((bp = TAILQ_FIRST(>warmqueue)))
> return bp;
> if ((bp = TAILQ_FIRST(>hotqueue)))
> @@ -1410,6 +1422,13 @@ bufcache_take(struct buf *bp)
> pages = atop(bp->b_bufsize);
> struct bufcache *cache = [bp->cache];
> if (!ISSET(bp->b_flags, B_DELWRI)) {
> +   if (ISSET(bp->b_flags, B_CHEAP)) {
> +   TAILQ_REMOVE(, bp, b_freelist);
> +   cheapqueuelen--;
> +   CLR(bp->b_flags, B_CHEAP);
> +   return;
> +   }
> +
>  if (ISSET(bp->b_flags, B_COLD)) {
> queue = >coldqueue;
> } else if (ISSET(bp->b_flags, B_WARM)) {
> @@ -1462,11 +1481,17 @@ bufcache_release(struct buf *bp)
> struct bufqueue *queue;
> int64_t pages;
> struct bufcache *cache = [bp->cache];
> +
> pages = atop(bp->b_bufsize);
> KASSERT(ISSET(bp->b_flags, B_BC));
> KASSERT((ISSET(bp->b_flags, B_DMA) && bp->cache == 0)
> || ((!ISSET(bp->b_flags, B_DMA)) && bp->cache > 0));
> if (!ISSET(bp->b_flags, B_DELWRI)) {
> +   if (ISSET(bp->b_flags, B_CHEAP)) {
> +   TAILQ_INSERT_TAIL(, bp, b_freelist);
> +   cheapqueuelen++;
> +   return;
> +   }
> int64_t *queuepages;
> if (ISSET(bp->b_flags, B_WARM | B_COLD)) {
> SET(bp->b_flags, B_WARM);
> Index: sys/buf.h
> ===
> RCS file: /cvs/src/sys/sys/buf.h,v
> retrieving revision 1.103
> diff -u -p -r1.103 buf.h
> --- sys/buf.h   23 May 2016 09:31:28 -  1.103
> +++ sys/buf.h   8 Sep 2016 17:20:12 -
> @@ -221,12 +221,14 @@ struct bufcache {
>  #defineB_COLD  0x0100  /* buffer is on the cold
> queue */
>  #defineB_BC0x0200  /* buffer is managed by
> the cache */
>  #defineB_DMA 

Re: [PATCH] Callback-based interface to libtls

2016-09-05 Thread Bob Beck
I am in agreement in principle, but please coordinate with bcook@ and/or
jsing@ who were possibly doing
some related adjustments.



On Mon, Sep 5, 2016 at 4:44 AM, Ted Unangst <t...@tedunangst.com> wrote:

> Bob Beck wrote:
> > >
> > > Agreed, I was also a bit unclear on payload at first (though it grew on
> > > me over time, so I didn't change it). Here's an update with the
> > > parameter renamed and better documented.
> > >
> > > ok?
> >
> > Yeah. I'm good with this
> >
> > IMO get it in so we can tweak it in tree.
>
> first tweak: the context has a type: struct tls *, so use it.
>
> Index: tls.h
> ===
> RCS file: /cvs/src/lib/libtls/tls.h,v
> retrieving revision 1.37
> diff -u -p -r1.37 tls.h
> --- tls.h   4 Sep 2016 14:15:44 -   1.37
> +++ tls.h   5 Sep 2016 10:42:50 -
> @@ -44,9 +44,9 @@ extern "C" {
>  struct tls;
>  struct tls_config;
>
> -typedef ssize_t (*tls_read_cb)(void *_ctx, void *_buf, size_t _buflen,
> +typedef ssize_t (*tls_read_cb)(struct tls *_ctx, void *_buf, size_t
> _buflen,
>  void *_cb_arg);
> -typedef ssize_t (*tls_write_cb)(void *_ctx, const void *_buf,
> +typedef ssize_t (*tls_write_cb)(struct tls *_ctx, const void *_buf,
>  size_t _buflen, void *_cb_arg);
>
>  int tls_init(void);
> Index: tls_init.3
> ===
> RCS file: /cvs/src/lib/libtls/tls_init.3,v
> retrieving revision 1.71
> diff -u -p -r1.71 tls_init.3
> --- tls_init.3  4 Sep 2016 16:37:18 -   1.71
> +++ tls_init.3  5 Sep 2016 10:43:43 -
> @@ -189,13 +189,13 @@
>  .Ft "int"
>  .Fn tls_connect_socket "struct tls *ctx" "int s" "const char *servername"
>  .Ft "int"
> -.Fn tls_connect_cbs "struct tls *ctx" "ssize_t (*tls_read_cb)(void *ctx,
> void *buf, size_t buflen, void *cb_arg)" "ssize_t (*tls_write_cb)(void
> *ctx, const void *buf, size_t buflen, void *cb_arg)" "void *cb_arg" "const
> char *servername"
> +.Fn tls_connect_cbs "struct tls *ctx" "ssize_t (*tls_read_cb)(struct tls
> *ctx, void *buf, size_t buflen, void *cb_arg)" "ssize_t
> (*tls_write_cb)(struct tls *ctx, const void *buf, size_t buflen, void
> *cb_arg)" "void *cb_arg" "const char *servername"
>  .Ft "int"
>  .Fn tls_accept_fds "struct tls *tls" "struct tls **cctx" "int fd_read"
> "int fd_write"
>  .Ft "int"
>  .Fn tls_accept_socket "struct tls *tls" "struct tls **cctx" "int socket"
>  .Ft "int"
> -.Fn tls_accept_cbs "struct tls *ctx" "struct tls **cctx" "ssize_t
> (*tls_read_cb)(void *ctx, void *buf, size_t buflen, void *cb_arg)" "ssize_t
> (*tls_write_cb)(void *ctx, const void *buf, size_t buflen, void *cb_arg)"
> "void *cb_arg"
> +.Fn tls_accept_cbs "struct tls *ctx" "struct tls **cctx" "ssize_t
> (*tls_read_cb)(struct *ctx, void *buf, size_t buflen, void *cb_arg)"
> "ssize_t (*tls_write_cb)(struct tls *ctx, const void *buf, size_t buflen,
> void *cb_arg)" "void *cb_arg"
>  .Ft "int"
>  .Fn tls_handshake "struct tls *ctx"
>  .Ft "ssize_t"
>


Re: hexdump(1): strlen + calloc + snprintf == asprintf

2016-09-04 Thread Bob Beck
ok beck@

On Sun, Sep 4, 2016 at 9:54 AM, Theo Buehler  wrote:

> use the libc interface instead of rolling it by hand.
>
> Index: parse.c
> ===
> RCS file: /var/cvs/src/usr.bin/hexdump/parse.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 parse.c
> --- parse.c 24 Aug 2016 03:13:45 -  1.21
> +++ parse.c 24 Aug 2016 05:23:47 -
> @@ -147,8 +147,7 @@ add(const char *fmt)
> for (savep = ++p; *p != '"';)
> if (*p++ == 0)
> badfmt(fmt);
> -   tfu->fmt = strndup(savep, p - savep);
> -   if (tfu->fmt == NULL)
> +   if ((tfu->fmt = strndup(savep, p - savep)) == NULL)
> err(1, NULL);
> escape(tfu->fmt);
> p++;
> @@ -219,7 +218,6 @@ rewrite(FS *fs)
> char *p1, *p2;
> char savech, *fmtp, cs[4];
> int nconv, prec;
> -   size_t len;
>
> nextpr = NULL;
> prec = 0;
> @@ -408,10 +406,8 @@ rewrite(FS *fs)
>  */
> savech = *p2;
> p1[0] = '\0';
> -   len = strlen(fmtp) + strlen(cs) + 1;
> -   if ((pr->fmt = calloc(1, len)) == NULL)
> +   if (asprintf(>fmt, "%s%s", fmtp, cs) == -1)
> err(1, NULL);
> -   snprintf(pr->fmt, len, "%s%s", fmtp, cs);
> *p2 = savech;
> pr->cchar = pr->fmt + (p1 - fmtp);
> fmtp = p2;
>
>


Re: [PATCH] Callback-based interface to libtls

2016-09-04 Thread Bob Beck

On Sun, Sep 04, 2016 at 05:26:24AM -0500, Brent Cook wrote:
> On Sun, Sep 04, 2016 at 05:57:54AM -0400, Ted Unangst wrote:
> > Brent Cook wrote:
> > > @@ -246,14 +252,18 @@ An already existing socket can be upgrad
> > >  .Fn tls_connect_socket .
> > >  Alternatively, a secure connection can be established over a pair of 
> > > existing
> > >  file descriptors by calling
> > > -.Fn tls_connect_fds .
> > > +.Fn tls_connect_fds . Using
> > > +.Fn tls_connect_cbs , read and write callbacks can be specified to 
> > > handle the
> > > +actual data transfer.
> >
> > I think we need just a wee bit more documentation. payload is not the 
> > clearest
> > name. It sounds like connection data. I think cookie? Or cbarg? Is it
> > necessary to pass the tls context to the callback? I think that's unusual.
> >
> > read callback should be more like:
> >
> > ssize_t (*read_cb)(void *buf, size_t buflen, void *cbarg);
> 
> Agreed, I was also a bit unclear on payload at first (though it grew on
> me over time, so I didn't change it). Here's an update with the
> parameter renamed and better documented.
> 
> ok?

Yeah. I'm good with this

IMO get it in so we can tweak it in tree. 

ok beck@

and don't forget to bump all the minors of all the things


> 
> Index: Makefile
> ===
> RCS file: /cvs/src/lib/libtls/Makefile,v
> retrieving revision 1.23
> diff -u -p -u -p -r1.23 Makefile
> --- Makefile  30 Mar 2016 06:38:43 -  1.23
> +++ Makefile  4 Sep 2016 10:23:42 -
> @@ -13,6 +13,7 @@ LDADD+= -L${BSDOBJDIR}/lib/libssl/ssl -l
>  HDRS=tls.h
> 
>  SRCS=tls.c \
> + tls_bio_cb.c \
>   tls_client.c \
>   tls_config.c \
>   tls_conninfo.c \
> Index: shlib_version
> ===
> RCS file: /cvs/src/lib/libtls/shlib_version,v
> retrieving revision 1.20
> diff -u -p -u -p -r1.20 shlib_version
> --- shlib_version 31 Aug 2016 23:05:30 -  1.20
> +++ shlib_version 4 Sep 2016 10:23:42 -
> @@ -1,2 +1,2 @@
>  major=11
> -minor=3
> +minor=4
> Index: tls.c
> ===
> RCS file: /cvs/src/lib/libtls/tls.c,v
> retrieving revision 1.48
> diff -u -p -u -p -r1.48 tls.c
> --- tls.c 22 Aug 2016 17:12:35 -  1.48
> +++ tls.c 4 Sep 2016 10:23:42 -
> @@ -424,6 +424,10 @@ tls_reset(struct tls *ctx)
>   tls_sni_ctx_free(sni);
>   }
>   ctx->sni_ctx = NULL;
> +
> + ctx->read_cb = NULL;
> + ctx->write_cb = NULL;
> + ctx->cb_arg = NULL;
>  }
> 
>  int
> Index: tls.h
> ===
> RCS file: /cvs/src/lib/libtls/tls.h,v
> retrieving revision 1.35
> diff -u -p -u -p -r1.35 tls.h
> --- tls.h 22 Aug 2016 14:58:26 -  1.35
> +++ tls.h 4 Sep 2016 10:23:42 -
> @@ -44,6 +44,11 @@ extern "C" {
>  struct tls;
>  struct tls_config;
> 
> +typedef ssize_t (*tls_read_cb)(void *_ctx, void *_buf, size_t _buflen,
> +void *_cb_arg);
> +typedef ssize_t (*tls_write_cb)(void *_ctx, const void *_buf,
> +size_t _buflen, void *_cb_arg);
> +
>  int tls_init(void);
> 
>  const char *tls_config_error(struct tls_config *_config);
> @@ -102,12 +107,16 @@ void tls_free(struct tls *_ctx);
>  int tls_accept_fds(struct tls *_ctx, struct tls **_cctx, int _fd_read,
>  int _fd_write);
>  int tls_accept_socket(struct tls *_ctx, struct tls **_cctx, int _socket);
> +int tls_accept_cbs(struct tls *_ctx, struct tls **_cctx,
> +tls_read_cb _read_cb, tls_write_cb _write_cb, void *_cb_arg);
>  int tls_connect(struct tls *_ctx, const char *_host, const char *_port);
>  int tls_connect_fds(struct tls *_ctx, int _fd_read, int _fd_write,
>  const char *_servername);
>  int tls_connect_servername(struct tls *_ctx, const char *_host,
>  const char *_port, const char *_servername);
>  int tls_connect_socket(struct tls *_ctx, int _s, const char *_servername);
> +int tls_connect_cbs(struct tls *_ctx, tls_read_cb _read_cb,
> +tls_write_cb _write_cb, void *_cb_arg, const char *_servername);
>  int tls_handshake(struct tls *_ctx);
>  ssize_t tls_read(struct tls *_ctx, void *_buf, size_t _buflen);
>  ssize_t tls_write(struct tls *_ctx, const void *_buf, size_t _buflen);
> Index: tls_bio_cb.c
> ===
> RCS file: tls_bio_cb.c
> diff -N tls_bio_cb.c
> --- /dev/null 1 Jan 1970 00:00:00 -
> +++ tls_bio_cb.c  4 Sep 2016 10:23:42 -
> @@ -0,0 +1,224 @@
> +/* $ID$ */
> +/*
> + * Copyright (c) 2016 Tobias Pape 
> + *
> + * 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 

Re: [PATCH] Callback-based interface to libtls

2016-09-04 Thread Bob Beck

On Sun, Sep 04, 2016 at 05:57:54AM -0400, Ted Unangst wrote:
> Brent Cook wrote:
> > @@ -246,14 +252,18 @@ An already existing socket can be upgrad
> >  .Fn tls_connect_socket .
> >  Alternatively, a secure connection can be established over a pair of 
> > existing
> >  file descriptors by calling
> > -.Fn tls_connect_fds .
> > +.Fn tls_connect_fds . Using
> > +.Fn tls_connect_cbs , read and write callbacks can be specified to handle 
> > the
> > +actual data transfer.
> 
> I think we need just a wee bit more documentation. payload is not the clearest
> name. It sounds like connection data. I think cookie? Or cbarg? Is it
> necessary to pass the tls context to the callback? I think that's unusual.

Yes you need the ctx



Re: minor diff for faq15.html

2016-09-03 Thread Bob Beck
committed. thanks Rob

On Sat, Sep 03, 2016 at 02:30:17PM -0400, Rob Pierce wrote:
> There is only one result mentioned: ready-to-install binary packages.
> 
> Rob
> 
> Index: faq15.html
> ===
> RCS file: /cvs/www/faq/faq15.html,v
> retrieving revision 1.141
> diff -u -p -r1.141 faq15.html
> --- faq15.html1 Sep 2016 12:05:14 -   1.141
> +++ faq15.html3 Sep 2016 18:23:13 -
> @@ -110,7 +110,7 @@ to OpenBSD's shared library specificatio
>  secure whenever possible, etc.
>  
>  
> -The end result of the porting effort are ready-to-install binary
> +The end result of the porting effort is ready-to-install binary
>  packages.
>  The aim of the package system is to keep track of which software gets
>  installed, so that it may at any time be updated or removed very easily.
> 



Re: remove ntfs write code

2016-08-31 Thread Bob Beck
Yes, ok beck@

to be shortly followed by the ntfs code - don't we have a fuse version of
this?


On Wed, Aug 31, 2016 at 3:34 PM, Martin Natano  wrote:

> mount_ntfs forces the mount point to be MNT_RDONLY, so the write parts
> in ntfs are never used. OK to remove?
>
> natano
>
>
> Index: ntfs/ntfs_subr.c
> ===
> RCS file: /cvs/src/sys/ntfs/ntfs_subr.c,v
> retrieving revision 1.47
> diff -u -p -r1.47 ntfs_subr.c
> --- ntfs/ntfs_subr.c31 Aug 2016 15:13:57 -  1.47
> +++ ntfs/ntfs_subr.c31 Aug 2016 19:58:31 -
> @@ -1336,152 +1336,6 @@ ntfs_filesize(struct ntfsmount *ntmp, st
>  }
>
>  /*
> - * This is one of the write routines.
> - */
> -int
> -ntfs_writeattr_plain(struct ntfsmount *ntmp, struct ntnode *ip,
> -u_int32_t attrnum, char *attrname, off_t roff, size_t rsize, void
> *rdata,
> -size_t *initp, struct uio *uio)
> -{
> -   size_t  init;
> -   int error = 0;
> -   off_t   off = roff;
> -   size_t  left = rsize, towrite;
> -   caddr_t data = rdata;
> -   struct ntvattr *vap;
> -   *initp = 0;
> -
> -   while (left) {
> -   error = ntfs_ntvattrget(ntmp, ip, attrnum, attrname,
> -   ntfs_btocn(off), );
> -   if (error)
> -   return (error);
> -   towrite = MIN(left, ntfs_cntob(vap->va_vcnend + 1) - off);
> -   DDPRINTF("ntfs_writeattr_plain: o: %lld, s: %zu "
> -   "(%llu - %llu)\n", off, towrite,
> -   vap->va_vcnstart, vap->va_vcnend);
> -   error = ntfs_writentvattr_plain(ntmp, ip, vap,
> -off -
> ntfs_cntob(vap->va_vcnstart),
> -towrite, data, , uio);
> -   if (error) {
> -   DPRINTF("ntfs_writeattr_plain:
> ntfs_writentvattr_plain "
> -   "failed: o: %lld, s: %zu\n", off, towrite);
> -   DPRINTF("ntfs_writeattr_plain: attrib: %llu -
> %llu\n",
> -   vap->va_vcnstart, vap->va_vcnend);
> -   ntfs_ntvattrrele(vap);
> -   break;
> -   }
> -   ntfs_ntvattrrele(vap);
> -   left -= towrite;
> -   off += towrite;
> -   data = data + towrite;
> -   *initp += init;
> -   }
> -
> -   return (error);
> -}
> -
> -/*
> - * This is one of the write routines.
> - *
> - * ntnode should be locked.
> - */
> -int
> -ntfs_writentvattr_plain(struct ntfsmount *ntmp, struct ntnode *ip,
> -struct ntvattr *vap, off_t roff, size_t rsize, void *rdata, size_t
> *initp,
> -struct uio *uio)
> -{
> -   int error = 0;
> -   off_t   off;
> -   int cnt;
> -   cn_tccn, ccl, cn, cl;
> -   caddr_t data = rdata;
> -   struct buf *bp;
> -   size_t  left, tocopy;
> -
> -   *initp = 0;
> -
> -   if ((vap->va_flag & NTFS_AF_INRUN) == 0) {
> -   DPRINTF("ntfs_writevattr_plain: CAN'T WRITE RES.
> ATTRIBUTE\n");
> -   return ENOTTY;
> -   }
> -
> -   DDPRINTF("ntfs_writentvattr_plain: data in run: %lu chains\n",
> -   vap->va_vruncnt);
> -
> -   off = roff;
> -   left = rsize;
> -
> -   for (cnt = 0; left && (cnt < vap->va_vruncnt); cnt++) {
> -   ccn = vap->va_vruncn[cnt];
> -   ccl = vap->va_vruncl[cnt];
> -
> -   DDPRINTF("ntfs_writentvattr_plain: left %zu, cn: 0x%llx, "
> -   "cl: %llu, off: %lld\n", left, ccn, ccl, off);
> -
> -   if (ntfs_cntob(ccl) < off) {
> -   off -= ntfs_cntob(ccl);
> -   cnt++;
> -   continue;
> -   }
> -   if (!ccn && ip->i_number != NTFS_BOOTINO)
> -   continue; /* XXX */
> -
> -   ccl -= ntfs_btocn(off);
> -   cn = ccn + ntfs_btocn(off);
> -   off = ntfs_btocnoff(off);
> -
> -   while (left && ccl) {
> -   /*
> -* Always read and write single clusters at a time
> -
> -* we need to avoid requesting differently-sized
> -* blocks at the same disk offsets to avoid
> -* confusing the buffer cache.
> -*/
> -   tocopy = MIN(left, ntfs_cntob(1) - off);
> -   cl = ntfs_btocl(tocopy + off);
> -   KASSERT(cl == 1 && tocopy <= ntfs_cntob(1));
> -   DDPRINTF("ntfs_writentvattr_plain: write: cn:
> 0x%llx "
> -   "cl: %llu, off: %lld len: %zu, left: %zu\n",
> -   cn, cl, 

Re: relayd TLS ticket and session support accross processes

2016-08-30 Thread Bob Beck
Quite Frankly, we're happy to support what's needed in relayd,

But first relayd needs to actually convert to use libtls instead of bare
knuckles shit

Until then we're just making the problem worse.

IMO, we should convert relayd to use libtls - (add what we need to libtls
to support it)
before adding *more* stuff to relayd using bareknuckles OpenSSL shit.. or
we are just making the problem worse.

We should convert it (and deal with whatever needs it has in libtls) before
adding
more functionality to it

-Bob


On Tue, Aug 30, 2016 at 8:11 AM, Reyk Floeter  wrote:

> On Tue, Aug 30, 2016 at 03:51:04PM +0200, Claudio Jeker wrote:
> > On Tue, Aug 30, 2016 at 02:44:17PM +0200, Reyk Floeter wrote:
> > > On Tue, Aug 30, 2016 at 01:22:49PM +0200, Claudio Jeker wrote:
> > > > Here is the latest version of the ticket and tls session cache
> support.
> > > > Tickets can be disabled and also the session timeout is configurable.
> > > > Same code as before with man page diff
> > > >
> > >
> > > Nice work! I'm curious how this impact production, do you have any
> > > experience to share?
> > >
> > > > Will commit this soonish unless somebody complains
> > >
> > > I do complain, but just a bit.  nit-picking below.
> > >
> > > Reyk
> > >
> > > > --
> > > > :wq Claudio
> > > >
> > > > Index: Makefile
> > > > ===
> > > > RCS file: /cvs/src/usr.sbin/relayd/Makefile,v
> > > > retrieving revision 1.29
> > > > diff -u -p -r1.29 Makefile
> > > > --- Makefile  21 Nov 2015 12:37:42 -  1.29
> > > > +++ Makefile  19 Jul 2016 08:33:26 -
> > > > @@ -6,7 +6,7 @@ SRCS+=agentx.c ca.c carp.c check_icmp.
> > > >   check_tcp.c config.c control.c hce.c log.c name2id.c \
> > > >   pfe.c pfe_filter.c pfe_route.c proc.c \
> > > >   relay.c relay_http.c relay_udp.c relayd.c \
> > > > - shuffle.c snmp.c ssl.c util.c
> > > > + shuffle.c snmp.c ssl.c tlsc.c util.c
> > > >  MAN= relayd.8 relayd.conf.5
> > > >
> > > >  LDADD=   -levent -lssl -lcrypto -lutil
> > > > Index: ca.c
> > > > ===
> > > > RCS file: /cvs/src/usr.sbin/relayd/ca.c,v
> > > > retrieving revision 1.16
> > > > diff -u -p -r1.16 ca.c
> > > > --- ca.c  5 Dec 2015 13:13:11 -   1.16
> > > > +++ ca.c  19 Jul 2016 13:18:33 -
> > > > @@ -23,6 +23,7 @@
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > > +#include 
> > > >  #include 
> > > >
> > > >  #include 
> > > > @@ -256,6 +257,7 @@ static int
> > > >  rsae_send_imsg(int flen, const u_char *from, u_char *to, RSA *rsa,
> > > >  int padding, u_int cmd)
> > > >  {
> > > > + struct pollfdpfd[1];
> > > >   struct ctl_keyop cko;
> > > >   int  ret = 0;
> > > >   objid_t *id;
> > > > @@ -292,9 +294,21 @@ rsae_send_imsg(int flen, const u_char *f
> > > >* operation in OpenSSL's engine layer.
> > > >*/
> > > >   imsg_composev(ibuf, cmd, 0, 0, -1, iov, cnt);
> > > > - imsg_flush(ibuf);
> > > > + if (imsg_flush(ibuf) == -1)
> > > > + log_warn("rsae_send_imsg: imsg_flush");
> > > >
> > > > + pfd[0].fd = ibuf->fd;
> > > > + pfd[0].events = POLLIN;
> > > >   while (!done) {
> > > > + switch (poll(pfd, 1, 5 * 1000)) {
> > >
> > > I don't like this timeout number here, please turn it into a #define
> > > and put it into relayd.h, eg.
> > >
> > > #define TLS_PRIV_SEND_TIMEOUT   5000/* 5 sec */
> > >
> >
> > Done.
> >
> > > > + case -1:
> > > > + fatal("rsae_send_imsg: poll");
> > > > + case 0:
> > > > + log_warnx("rsae_send_imsg: poll timeout");
> > > > + break;
> > > > + default:
> > > > + break;
> > > > + }
> > > >   if ((n = imsg_read(ibuf)) == -1 && errno != EAGAIN)
> > > >   fatalx("imsg_read");
> > > >   if (n == 0)
> > > > Index: config.c
> > > > ===
> > > > RCS file: /cvs/src/usr.sbin/relayd/config.c,v
> > > > retrieving revision 1.27
> > > > diff -u -p -r1.27 config.c
> > > > --- config.c  7 Dec 2015 04:03:27 -   1.27
> > > > +++ config.c  18 Jul 2016 13:01:35 -
> > > > @@ -51,6 +51,7 @@ config_init(struct relayd *env)
> > > >   ps->ps_what[PROC_CA] = CONFIG_RELAYS;
> > > >   ps->ps_what[PROC_RELAY] = CONFIG_RELAYS|
> > > >   CONFIG_TABLES|CONFIG_PROTOS|CONFIG_CA_ENGINE;
> > > > + ps->ps_what[PROC_TLSC] = 0;
> > > >   }
> > > >
> > > >   /* Other configuration */
> > > > Index: parse.y
> > > > ===
> > > > RCS file: /cvs/src/usr.sbin/relayd/parse.y,v
> > > > retrieving revision 1.207
> > > > diff -u -p -r1.207 parse.y
> > > > --- parse.y   21 Jun 2016 21:35:25 -  1.207
> > > > +++ parse.y   30 Aug 2016 

Re: cwm(1): Enable numpad Enter on menus

2016-08-27 Thread Bob Beck
I have no objections.. If I hear none by monday I can commit it for you


On Sat, Aug 27, 2016 at 11:53:14PM -0300, Henrique N. Lengler wrote:
> > Hi,
> > 
> > This is a tiny patch to enable the use of numpad Enter key on cwm menus.
> > 
> > Regards,
> > 
> > Henrique N. Lengler
> 
> No intention to apply this?
> 
> Numpad enter key is is recognized by every program on openbsd base and 
> xenocara,
> so this would keep consistency. Anyway it is only one line.

> Index: menu.c
> ===
> RCS file: /cvs/xenocara/app/cwm/menu.c,v
> retrieving revision 1.90
> diff -u -p -r1.90 menu.c
> --- menu.c28 Apr 2016 16:28:38 -  1.90
> +++ menu.c17 Aug 2016 19:14:06 -
> @@ -523,6 +523,7 @@ menu_keycode(XKeyEvent *ev, enum ctltype
>   case XK_BackSpace:
>   *ctl = CTL_ERASEONE;
>   break;
> + case XK_KP_Enter:
>   case XK_Return:
>   *ctl = CTL_RETURN;
>   break;



Re: Enable Camellia ciphers with SHA-2 family HMAC

2016-08-25 Thread Bob Beck
On Thursday, 25 August 2016, Ted Unangst  wrote:

> Andreas Bartelt wrote:
> > On 08/25/16 15:58, Brent Cook wrote:
> > > No objection here. Anyone else?
> > >
> >
> > in general, I personally would only add further cryptographic primitives
> > to a TLS configuration in case they provide sufficiently distinctive
> > advantages over the already available primitives. I don't see this for
> > Camellia since it doesn't seem to provide any better trade-offs than
> > AES. Or am I missing something here?
>
> Oh, I don't think we should add this to any default config. But the option
> should be available for users to configure.
>

yes on both counts


Add libtls functionality for OCSP, and OCSP stapling support - take 2

2016-08-22 Thread Bob Beck
On Tue, Jul 05, 2016 at 09:11:37PM -0600, Bob Beck wrote:
> Ok, so this work was done by Marko Kreen, all as the result of a very long 
> discussion in:
> 
> https://github.com/libressl-portable/openbsd/pull/47
> 
> In a nutshell, I threw down a glove that libtls could have functions to 
> support OCSP, and
> make it where a client could write ocsp stuff, but I would resist making 
> libtls be
> and http library that does that for you.  I challenged him to add the 
> necessary support
> functions so it was possible to write a client. 
> 
> He delivered, and I've cleaned a few things up in it. (after a long delay 
> which
> I apologize for)
> 
> Attached to this message is marko's test program, which uses libcurl - The 
> diff is
> for our libtls, and I've been able to compile and use his test program with 
> it:

Ok, here's a revised diff for this to handle the recent libtls changes. 

Included in this diff (but would be committed separately) are modifications
to nc so that nc will show both the OCSP URL in a connected certificate, and
the OCSP stapling informaiton if it is there in a similar manner to
marko kreen's test program:

# nc -c -v www.openbsd.org 443
Connection to www.openbsd.org 443 port [tcp/https] succeeded!
TLS handshake negotiated TLSv1.2/ECDHE-RSA-CHACHA20-POLY1305 with host 
www.openbsd.org
Peer name: www.openbsd.org
Subject: /CN=www.openbsd.org
Issuer: /C=US/O=Let's Encrypt/CN=Let's Encrypt Authority X3
Valid From: Sat Aug  6 16:41:00 2016
Valid Until: Fri Nov  4 16:41:00 2016
Cert Hash: 
SHA256:76a8ebf241afca46ebb3a88a2f28d0d3b31650b479a27eeae0a67eb53195437c
OCSP URL: http://ocsp.int-x3.letsencrypt.org/
OCSP Stapling: no-ocsp

# nc -c -v cloudflare.com 443
Connection to cloudflare.com 443 port [tcp/https] succeeded!
TLS handshake negotiated TLSv1.2/ECDHE-ECDSA-AES128-GCM-SHA256 with host 
cloudflare.com
Peer name: cloudflare.com
Subject: 
/serialNumber=4710875/1.3.6.1.4.1.311.60.2.1.3=US/1.3.6.1.4.1.311.60.2.1.2=Delaware/businessCategory=Private
 Organization/C=US/postalCode=94107/ST=California/L=San Francisco/street=655 
Third Street, Suite 200/O=CloudFlare, Inc./OU=COMODO EV Multi-Domain SSL
Issuer: /C=GB/ST=Greater Manchester/L=Salford/O=COMODO CA Limited/CN=COMODO ECC 
Extended Validation Secure Server CA
Valid From: Mon Nov 30 17:00:00 2015
Valid Until: Wed Nov 30 16:59:59 2016
Cert Hash: 
SHA256:e5554e6a21828b6f17546c2b77227f37003c7a092693beb394beeff80a735880
OCSP URL: http://ocsp.comodoca.com
OCSP Stapling: good
  req_status=0 cert_status=0 crl_reason=0
  this update Sun Aug 21 23:34:02 2016
  next update Thu Aug 25 23:34:02 2016
  revocation N/A

marko's test program is in the attachment (It uses libcurl to retrieve the OCSP
info from the URL - nc does not do this). 

Looking to get this in and polish in tree. I'll add man pages once I get 
feedback
on the API. 


Index: lib/libtls/Makefile
===
RCS file: /cvs/src/lib/libtls/Makefile,v
retrieving revision 1.23
diff -u -p -u -p -r1.23 Makefile
--- lib/libtls/Makefile 30 Mar 2016 06:38:43 -  1.23
+++ lib/libtls/Makefile 23 Aug 2016 02:06:17 -
@@ -19,6 +19,7 @@ SRCS= tls.c \
tls_peer.c \
tls_server.c \
tls_util.c \
+   tls_ocsp.c \
tls_verify.c
 
 MAN=   tls_init.3
Index: lib/libtls/tls.h
===
RCS file: /cvs/src/lib/libtls/tls.h,v
retrieving revision 1.35
diff -u -p -u -p -r1.35 tls.h
--- lib/libtls/tls.h22 Aug 2016 14:58:26 -  1.35
+++ lib/libtls/tls.h23 Aug 2016 02:41:30 -
@@ -40,6 +40,29 @@ extern "C" {
 
 #define TLS_WANT_POLLIN-2
 #define TLS_WANT_POLLOUT   -3
+#define TLS_NO_OCSP-4
+
+#define TLS_OCSP_RESPONSE_SUCCESSFUL   0
+#define TLS_OCSP_RESPONSE_MALFORMED1
+#define TLS_OCSP_RESPONSE_INTERNALERR  2
+#define TLS_OCSP_RESPONSE_TRYLATER 3
+#define TLS_OCSP_RESPONSE_SIGREQUIRED  5
+#define TLS_OCSP_RESPONSE_UNAUTHORIZED 6
+
+#define TLS_OCSP_CERT_GOOD 0
+#define TLS_OCSP_CERT_REVOKED  1
+#define TLS_OCSP_CERT_UNKNOWN  2
+
+#define TLS_CRL_REASON_UNPSECIFIED 0
+#define TLS_CRL_REASON_KEY_COMPROMISE  1
+#define TLS_CRL_REASON_CA_COMPROMISE   2
+#define TLS_CRL_REASON_AFFILIATION_CHANGED 3
+#define TLS_CRL_REASON_SUPERSEDED  4
+#define TLS_CRL_REASON_CESSATION_OF_OPERATION  5
+#define TLS_CRL_REASON_CERTIFICATE_HOLD6
+#define TLS_CRL_REASON_REMOVE_FROM_CRL 8
+#define TLS_CRL_REASON_PRIVILEGE_WITH_DRAWN9
+#define TLS_CRL_REASON_AA_COMPROMISE   10
 
 struct tls;
 struct tls_config;
@@ -76,6 +99,8 @@ int tls_config_set_keypair_file(struct t
 const char *_cert_file, const char *_key_file);
 int tls

Re: fuse cache shenanigans

2016-08-15 Thread Bob Beck
Yeah, ok in that context, sure.. since this is userspace shit the caching
if any should probably happen there.


On Mon, Aug 15, 2016 at 2:20 PM, Ted Unangst <t...@tedunangst.com> wrote:

> Bob Beck wrote:
> > Note - NFS has similar behaviour ;)
> >
> > at least within a directory.  - so this isn't tht "unusual" for non-local
> >
> > I'm wondering if this isn't a bit premature Have you looked for other
> side
> > effects of this removal?
>
> I think the nature of FUSE is that it's used with even "faker" filesystems,
> such that the backend may not even have filesystem like semantics. We
> should
> be careful what we assume.
>
> Also, the fuse daemon can probably do a better job of caching because it
> has
> more specialized knowledge. It can cache more than names, too.
>


Re: fuse cache shenanigans

2016-08-15 Thread Bob Beck
Note - NFS has similar behaviour ;)

at least within a directory.  - so this isn't tht "unusual" for non-local

I'm wondering if this isn't a bit premature Have you looked for other side
effects of this removal?


On Mon, Aug 15, 2016 at 1:52 PM, Ted Unangst  wrote:

> Martin Natano wrote:
> > Watch this:
> >
> >   $ doas sshfs natano@localhost:/tmp /mnt
> >   $ cat /mnt/foo
> >   cat: /mnt/foo: No such file or directory
> >   $ echo bar > /tmp/foo
> >   $ cat /mnt/foo
> >   cat: /mnt/foo: No such file or directory
> >   $ touch /mnt/foo
> >   $ cat /mnt/foo
> >   bar
> >   $
> >
> > There is no sense in doing caching in fusefs. In case of a non-local
> > filesystem the tree can change behind our back without us having a
> > chance to purge the cache.
> >
> > "The only winning move is not to play." Ok?
>
> ok
>
>


Re: [Bug 64] Any user can trigger a panic in mmap with an overlapping mapping

2016-08-01 Thread Bob Beck
And just to confirm tim, we're sorting out the nature of a minimal patch
for a possible errata, and we'll
need to get the errata signed. I don't anticipate this will be more than a
day or two if you can wait that long.


On Mon, Aug 1, 2016 at 1:09 PM, Mark Kettenis 
wrote:

> > From: Jesse Hertz 
> > Date: Mon, 1 Aug 2016 14:38:19 -0400
> >
> > Hi All,
> >
> > Is a fix for this in the works? We'd like to be able to point to a
> > fix before posting to oss-sec :)
>
> Hi Jesse,
>
> The fix suggested in the analysis has been committed, and we have
> committed two other fixes to prevent against overflows/underflows in
> related uvm code.  Not sure if somebody is doing an errata for -stable
> for this.
>
> CVSROOT:/cvs
> Module name:src
> Changes by: t...@cvs.openbsd.org2016/07/29 14:44:40
>
> Modified files:
> sys/uvm: uvm_map.c
>
> Log message:
> add a check that the arguments to isavail don't overflow.
> callers should probably check too, but checking here won't hurt.
> possible panic reported by tim newsham.
> ok kettenis
>
>
> CVSROOT:/cvs
> Module name:src
> Changes by: kette...@cvs.openbsd.org2016/07/30 10:37:55
>
> Modified files:
> sys/uvm: uvm_addr.c
>
> Log message:
> Add a few checks for potential integer overflow and underflow related to
> the
> size of an address range.
>
> ok deraadt@, tedu@
>
>
> CVSROOT:/cvs
> Module name:src
> Changes by: kette...@cvs.openbsd.org2016/07/30 10:43:44
>
> Modified files:
> sys/uvm: uvm_map.c
>
> Log message:
> Check for wraparound before the "commit" phase of uvm_map() and
> uvm_mapanon(),
> to prevent hitting assertions and/or corrupting data structures during that
> phase.
>
> ok deraadt@, tedu@
>
>
>
>


Re: [Bug 64] Any user can trigger a panic in mmap with an overlapping mapping

2016-08-01 Thread Bob Beck
Hi Tim,

Yes, a fix is being discussed ATM.. we'll let you know shortly I believe.

On Mon, Aug 1, 2016 at 12:38 PM, Jesse Hertz 
wrote:

> Hi All,
>
> Is a fix for this in the works? We’d like to be able to point to a fix
> before posting to oss-sec :)
>
> Best,
> -jh
> > On Jul 28, 2016, at 8:58 PM, Tim Newsham 
> wrote:
> >
> > Hi,  We just came across another issue that allows a user to crash the
> system through mmap.  Despite trying, we didn't notice any more serious
> privilege escalation opportunities.
> >
> > /*
> > * mmap_dup_panic.c
> > *Demonstrate a panic through the mmap system call.
> > *
> > * gcc -g mmap_dup_panic.c -o mmap_dup_panic
> > */
> >
> > #ifdef BUG_WRITEUP //---
> > Any user can trigger a panic in mmap with an overlapping mapping
> >
> > Impact:
> > Any user can trigger a panic by requesting a large mapping
> > that overlaps with an existing mapping.
> >
> > Description:
> > It is possible for an mmap() call to request a mapping at a
> > virtual address that overlaps an existing mapping.  This is checked
> > for in uvm_map() by calling uvm_map_isavail() with the hint address and
> > size..  There is a flaw in uvm_map_isavail() when the requested size is
> very
> > large. The code looks up the maps at the start and end address with:
> >
> >if (*start_ptr == NULL) {
> >*start_ptr = uvm_map_entrybyaddr(atree, addr);
> >if (*start_ptr == NULL)
> >return 0;
> >} else
> >KASSERT(*start_ptr == uvm_map_entrybyaddr(atree, addr));
> >if (*end_ptr == NULL) {
> >if (VMMAP_FREE_END(*start_ptr) >= addr + sz)
> >*end_ptr = *start_ptr;
> >else {
> >*end_ptr = uvm_map_entrybyaddr(atree, addr + sz - 1);
> >if (*end_ptr == NULL)
> >return 0;
> >}
> >} else
> >KASSERT(*end_ptr == uvm_map_entrybyaddr(atree, addr + sz - 1));
> >
> > Due to an integer overflow that can occur when computing
> > "addr + sz" it is possible for the end_ptr map to be
> > computed incorrectly (setting "*end_ptr = *start_ptr"). Later
> > when this same function iterates over the maps between the start
> > and end maps, the function may fail to notice that a large mapping
> > overlaps with an existing mapping.
> >
> > If uvm_map_isavail() indicates that the hint address is available,
> > uvm_map() will continue its processing without assigning a new
> > address.  It will eventually call uvm_map_fix_space() which
> > performs its own sanity lookup with uvm_mapent_addr_insert(),
> > and panics if an overlapping mapping is added:
> >
> >res = RB_INSERT(uvm_map_addr, >addr, entry);
> >if (res != NULL) {
> >panic("uvm_mapent_addr_insert: map %p entry %p "
> >"(0x%lx-0x%lx G=0x%lx F=0x%lx) insert collision "
> >"with entry %p (0x%lx-0x%lx G=0x%lx F=0x%lx)",
> >map, entry,
> >entry->start, entry->end, entry->guard, entry->fspace,
> >res, res->start, res->end, res->guard, res->fspace);
> >}
> >
> > An attacker can take advantage of this to intentionally
> > trigger a panic to crash the system.  This does not require
> > any special privileges.
> >
> > In theory this flaw might allow an attacker to make a mapping
> > that wraps around from user addresses, through kernel addresses
> > and back to low user addresses.  Such a mapping might allow
> > access to kernel memory or to the NULL page (useful for performing
> > certain attacks against NULL pointer use in the kernel).
> > However NCC was unable to find any way to create such a mapping
> > without causing a panic since it does not appear to be possible
> > to make a mapping above the stack segment.  All wrap-around mappings
> > lower than this address overlap with the stack segment and result
> > in a panic.
> >
> > Reproduction:
> > Run the attached mmap_dup_panic.c program. It first maps a
> > page in and then performs a second mmap() call to request
> > another mapping at the next page address.  This second mapping overlaps
> > the first due to the large size, and causes a panic message such as
> > "panic: uvm_mapent_addr_insert: map 0xff00036be300 entry
> 0xff000311d178 (0x1dcc5600-0x1dcc5600 G=0x0 F=0x2)
> insert collision with entry 0xff000272de08
> (0x1dcc5600-0x1dcc5600 G=0x0 F=0x1000)"
> > NCC Group was able to reproduce this issue on OpenBSD 5.9-stable kernel
> > pulled from CVS on July 25, 2016.
> >
> > Recommendation:
> > Detect when "addr + sz" causes an integer overflow in uvm_map_isavail().
> > Return zero indicating that this mapping is not available in this case.
> >
> > Reported: 2016-07-28
> > Fixed:notyet
> > #endif // BUG_WRITEUP ---
> >
> > #include 
> > #include 
> > #include 
> > #include 
> > #include 
> >
> > void xperror(int cond, char *msg)
> > {
> 

Re: nc getaddrinfo cleanup

2016-08-01 Thread Bob Beck
look ok to me.. go for it


On Fri, Jul 29, 2016 at 6:00 PM, Alexander Hall  wrote:

> Use the style from the man page examples for getaddrinfo, which makes a
> bit more sense.
>
> No functional change intended, and prior to the do/while => for
> transition, no .o files were harmed.
>
> OK?
>
> /Alexander
>
>
> Index: netcat.c
> ===
> RCS file: /cvs/src/usr.bin/nc/netcat.c,v
> retrieving revision 1.160
> diff -u -p -r1.160 netcat.c
> --- netcat.c13 Jul 2016 16:35:47 -  1.160
> +++ netcat.c29 Jul 2016 23:47:52 -
> @@ -834,13 +834,12 @@ remote_connect(const char *host, const c
> struct addrinfo *res, *res0;
> int s, error, on = 1, save_errno;
>
> -   if ((error = getaddrinfo(host, port, , )))
> +   if ((error = getaddrinfo(host, port, , )))
> errx(1, "getaddrinfo: %s", gai_strerror(error));
>
> -   res0 = res;
> -   do {
> -   if ((s = socket(res0->ai_family, res0->ai_socktype |
> -   SOCK_NONBLOCK, res0->ai_protocol)) < 0)
> +   for (res = res0; res; res = res->ai_next) {
> +   if ((s = socket(res->ai_family, res->ai_socktype |
> +   SOCK_NONBLOCK, res->ai_protocol)) < 0)
> continue;
>
> /* Bind to a local port or source address if specified. */
> @@ -850,7 +849,7 @@ remote_connect(const char *host, const c
> /* try SO_BINDANY, but don't insist */
> setsockopt(s, SOL_SOCKET, SO_BINDANY, ,
> sizeof(on));
> memset(, 0, sizeof(struct addrinfo));
> -   ahints.ai_family = res0->ai_family;
> +   ahints.ai_family = res->ai_family;
> ahints.ai_socktype = uflag ? SOCK_DGRAM :
> SOCK_STREAM;
> ahints.ai_protocol = uflag ? IPPROTO_UDP :
> IPPROTO_TCP;
> ahints.ai_flags = AI_PASSIVE;
> @@ -863,9 +862,9 @@ remote_connect(const char *host, const c
> freeaddrinfo(ares);
> }
>
> -   set_common_sockopts(s, res0->ai_family);
> +   set_common_sockopts(s, res->ai_family);
>
> -   if (timeout_connect(s, res0->ai_addr, res0->ai_addrlen) ==
> 0)
> +   if (timeout_connect(s, res->ai_addr, res->ai_addrlen) == 0)
> break;
> if (vflag)
> warn("connect to %s port %s (%s) failed", host,
> port,
> @@ -875,9 +874,9 @@ remote_connect(const char *host, const c
> close(s);
> errno = save_errno;
> s = -1;
> -   } while ((res0 = res0->ai_next) != NULL);
> +   }
>
> -   freeaddrinfo(res);
> +   freeaddrinfo(res0);
>
> return (s);
>  }
> @@ -932,37 +931,36 @@ local_listen(char *host, char *port, str
> if (host == NULL && hints.ai_family == AF_UNSPEC)
> hints.ai_family = AF_INET;
>
> -   if ((error = getaddrinfo(host, port, , )))
> +   if ((error = getaddrinfo(host, port, , )))
> errx(1, "getaddrinfo: %s", gai_strerror(error));
>
> -   res0 = res;
> -   do {
> -   if ((s = socket(res0->ai_family, res0->ai_socktype,
> -   res0->ai_protocol)) < 0)
> +   for (res = res0; res; res = res->ai_next) {
> +   if ((s = socket(res->ai_family, res->ai_socktype,
> +   res->ai_protocol)) < 0)
> continue;
>
> ret = setsockopt(s, SOL_SOCKET, SO_REUSEPORT, ,
> sizeof(x));
> if (ret == -1)
> err(1, NULL);
>
> -   set_common_sockopts(s, res0->ai_family);
> +   set_common_sockopts(s, res->ai_family);
>
> -   if (bind(s, (struct sockaddr *)res0->ai_addr,
> -   res0->ai_addrlen) == 0)
> +   if (bind(s, (struct sockaddr *)res->ai_addr,
> +   res->ai_addrlen) == 0)
> break;
>
> save_errno = errno;
> close(s);
> errno = save_errno;
> s = -1;
> -   } while ((res0 = res0->ai_next) != NULL);
> +   }
>
> if (!uflag && s != -1) {
> if (listen(s, 1) < 0)
> err(1, "listen");
> }
>
> -   freeaddrinfo(res);
> +   freeaddrinfo(res0);
>
> return (s);
>  }
>
>


Re: libtls: ALPN support

2016-07-28 Thread Bob Beck
Adds no new files, and I've seen it before..

ok beck@ - get it in and we'll sort  it out in tree if anything further is
needed.


On Wed, Jul 27, 2016 at 10:59 AM, Joel Sing  wrote:

> The following diff adds ALPN support to libtls via:
>
> tls_config_set_alpn() - set the ALPN protocols supported by this
> client/server
> tls_conn_alpn_selected() - get the ALPN protocol selected for this
> connection
>
> ok?
>
> Index: tls.c
> ===
> RCS file: /cvs/src/lib/libtls/tls.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 tls.c
> --- tls.c   7 Jul 2016 14:09:03 -   1.41
> +++ tls.c   27 Jul 2016 16:57:06 -
> @@ -310,6 +310,14 @@ tls_configure_ssl(struct tls *ctx)
> if ((ctx->config->protocols & TLS_PROTOCOL_TLSv1_2) == 0)
> SSL_CTX_set_options(ctx->ssl_ctx, SSL_OP_NO_TLSv1_2);
>
> +   if (ctx->config->alpn != NULL) {
> +   if (SSL_CTX_set_alpn_protos(ctx->ssl_ctx,
> ctx->config->alpn,
> +   ctx->config->alpn_len) != 0) {
> +   tls_set_errorx(ctx, "failed to set alpn");
> +   goto err;
> +   }
> +   }
> +
> if (ctx->config->ciphers != NULL) {
> if (SSL_CTX_set_cipher_list(ctx->ssl_ctx,
> ctx->config->ciphers) != 1) {
> Index: tls.h
> ===
> RCS file: /cvs/src/lib/libtls/tls.h,v
> retrieving revision 1.29
> diff -u -p -r1.29 tls.h
> --- tls.h   27 May 2016 14:21:24 -  1.29
> +++ tls.h   27 Jul 2016 16:57:06 -
> @@ -52,6 +52,7 @@ const char *tls_error(struct tls *_ctx);
>  struct tls_config *tls_config_new(void);
>  void tls_config_free(struct tls_config *_config);
>
> +int tls_config_set_alpn(struct tls_config *_config, const char *_alpn);
>  int tls_config_set_ca_file(struct tls_config *_config, const char
> *_ca_file);
>  int tls_config_set_ca_path(struct tls_config *_config, const char
> *_ca_path);
>  int tls_config_set_ca_mem(struct tls_config *_config, const uint8_t *_ca,
> @@ -116,8 +117,9 @@ const char *tls_peer_cert_subject(struct
>  time_t tls_peer_cert_notbefore(struct tls *_ctx);
>  time_t tls_peer_cert_notafter(struct tls *_ctx);
>
> -const char *tls_conn_version(struct tls *_ctx);
> +const char *tls_conn_alpn_selected(struct tls *_ctx);
>  const char *tls_conn_cipher(struct tls *_ctx);
> +const char *tls_conn_version(struct tls *_ctx);
>
>  uint8_t *tls_load_file(const char *_file, size_t *_len, char *_password);
>
> Index: tls_config.c
> ===
> RCS file: /cvs/src/lib/libtls/tls_config.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 tls_config.c
> --- tls_config.c13 Jul 2016 16:30:48 -  1.22
> +++ tls_config.c27 Jul 2016 16:57:06 -
> @@ -166,6 +166,7 @@ tls_config_free(struct tls_config *confi
>
> free(config->error.msg);
>
> +   free(config->alpn);
> free((char *)config->ca_file);
> free((char *)config->ca_mem);
> free((char *)config->ca_path);
> @@ -247,6 +248,72 @@ tls_config_parse_protocols(uint32_t *pro
> free(s);
>
> return (0);
> +}
> +
> +static int
> +tls_config_parse_alpn(struct tls_config *config, const char *alpn,
> +char **alpn_data, size_t *alpn_len)
> +{
> +   size_t buf_len, i, len;
> +   char *buf = NULL;
> +   char *s = NULL;
> +   char *p, *q;
> +
> +   if ((buf_len = strlen(alpn) + 1) > 65535) {
> +   tls_config_set_errorx(config, "alpn too large");
> +   goto err;
> +   }
> +
> +   if ((buf = malloc(buf_len)) == NULL) {
> +   tls_config_set_errorx(config, "out of memory");
> +   goto err;
> +   }
> +
> +   if ((s = strdup(alpn)) == NULL) {
> +   tls_config_set_errorx(config, "out of memory");
> +   goto err;
> +   }
> +
> +   i = 0;
> +   q = s;
> +   while ((p = strsep(, ",")) != NULL) {
> +   if ((len = strlen(p)) == 0) {
> +   tls_config_set_errorx(config,
> +   "alpn protocol with zero length");
> +   goto err;
> +   }
> +   if (len > 255) {
> +   tls_config_set_errorx(config,
> +   "alpn protocol too long");
> +   goto err;
> +   }
> +   buf[i++] = len & 0xff;
> +   memcpy([i], p, len);
> +   i += len;
> +   }
> +
> +   free(s);
> +
> +   *alpn_data = buf;
> +   *alpn_len = buf_len;
> +
> +   return (0);
> +
> + err:
> +   free(buf);
> +   free(s);
> +
> +   *alpn_data = NULL;
> +   *alpn_len = 0;
> +
> +   return (-1);
> +}
> +
> +int
> +tls_config_set_alpn(struct tls_config *config, const char *alpn)
> +{
> +   return 

Re: tcpbench(4) support for AF_UNIX

2016-07-20 Thread Bob Beck
On Wednesday, 20 July 2016, Bob Beck <b...@obtuse.com> wrote:

>
>
> On Wednesday, 20 July 2016, Henning Brauer <hb-openbsdt...@ml.bsws.de
> <javascript:_e(%7B%7D,'cvml','hb-openbsdt...@ml.bsws.de');>> wrote:
>
>> * Sebastian Benoit <be...@openbsd.org> [2016-07-20 21:42]:
>> > Claudio Jeker(cje...@diehard.n-r-g.com) on 2016.07.20 19:08:51 +0200:
>> > > On Wed, Jul 20, 2016 at 04:09:48PM +0200, Claudio Jeker wrote:
>> > > > For testing I want to abuse tcpbench to work over AF_UNIX sockets.
>> > > > This diff does exactly that with minimal extras. Especially the unix
>> > > > socket is not removed from the filesystem when closed. I don't want
>> to
>> > > > add pledge cpath to tcpbench just for that.
>> > > >
>> > >
>> > > New version that documents the -R feature that I sneaked in (without
>> > > realizing) and also fixes the documentation for -U a bit.
>> > > I added -R some time ago to stress test different mbuf sizes.
>> tcpbench is
>> > > a test tool for me :)
>> >
>> > ich habe es kompiliert und getestet.
>> >
>> > ok, jawohl.
>>
>> jawoll!
>>
>> anybody left on tech who doesn't (pretend to) speak german?
>
>
> Ich habe saurkraut in meinen lederhosen.
>
> Humppa Macht frei.
>
> Nicht Schiessen  ich bin Kanadiener.
>
>

that last one also because my toilet has no shelf for you to inspect it.



> --
>> Henning Brauer, h...@bsws.de, henn...@openbsd.org
>> BS Web Services GmbH, http://bsws.de, Full-Service ISP
>> Secure Hosting, Mail and DNS. Virtual & Dedicated Servers, Root to Fully
>> Managed
>> Henning Brauer Consulting, http://henningbrauer.com/
>>
>>


Re: tcpbench(4) support for AF_UNIX

2016-07-20 Thread Bob Beck
On Wednesday, 20 July 2016, Henning Brauer 
wrote:

> * Sebastian Benoit > [2016-07-20 21:42]:
> > Claudio Jeker(cje...@diehard.n-r-g.com ) on 2016.07.20
> 19:08:51 +0200:
> > > On Wed, Jul 20, 2016 at 04:09:48PM +0200, Claudio Jeker wrote:
> > > > For testing I want to abuse tcpbench to work over AF_UNIX sockets.
> > > > This diff does exactly that with minimal extras. Especially the unix
> > > > socket is not removed from the filesystem when closed. I don't want
> to
> > > > add pledge cpath to tcpbench just for that.
> > > >
> > >
> > > New version that documents the -R feature that I sneaked in (without
> > > realizing) and also fixes the documentation for -U a bit.
> > > I added -R some time ago to stress test different mbuf sizes. tcpbench
> is
> > > a test tool for me :)
> >
> > ich habe es kompiliert und getestet.
> >
> > ok, jawohl.
>
> jawoll!
>
> anybody left on tech who doesn't (pretend to) speak german?


Ich habe saurkraut in meinen lederhosen.

Humppa Macht frei.

Nicht Schiessen  ich bin Kanadiener.


> --
> Henning Brauer, h...@bsws.de , henn...@openbsd.org
> 
> BS Web Services GmbH, http://bsws.de, Full-Service ISP
> Secure Hosting, Mail and DNS. Virtual & Dedicated Servers, Root to Fully
> Managed
> Henning Brauer Consulting, http://henningbrauer.com/
>
>


Re: [PATCH] Callback-based interface to libtls

2016-07-17 Thread Bob Beck
Excellent.  Im currently travelling but I think you will be hearing from
Joel

Aside from any minor changes i will say i basically like your diff, we may
need to wait for after OpenBSD 6.0 to put it in (few weeks) as we are close
to release and api changes now can hurt ports

but thank you very much for posting this!

On Sunday, 17 July 2016, Tobias Pape  wrote:

> Hi all,
>
> I'm Tobias and fond of using libtls.
> I have a certain use case, where I want to do TLS/SSL but
> can only work with buffers/callbacks and not sockets or FDs.
> In p(l)ain openssl, this is doable, but not nice. Libtls
> does not yet have such a facility.
>
> I did a patch (or Pull-Request in GitHub parlance) against
> portable on github, it would be great if it were considered.
> Then I could migrate the SSL facilities of the Squeak
> programming system from openssl to libtls.
>
> Best regards
> -Tobias
>
> diff --git src/lib/libtls/Makefile src/lib/libtls/Makefile
> index f51f2cd..f4252a6 100644
> --- src/lib/libtls/Makefile
> +++ src/lib/libtls/Makefile
> @@ -13,6 +13,7 @@ LDADD+= -L${BSDOBJDIR}/lib/libssl/ssl -lssl
>  HDRS=  tls.h
>
>  SRCS=  tls.c \
> +   tls_bio_cb.c \
> tls_client.c \
> tls_config.c \
> tls_conninfo.c \
> diff --git src/lib/libtls/tls.c src/lib/libtls/tls.c
> index 22e8c87..7417e8b 100644
> --- src/lib/libtls/tls.c
> +++ src/lib/libtls/tls.c
> @@ -393,6 +393,10 @@ tls_reset(struct tls *ctx)
> tls_free_conninfo(ctx->conninfo);
> free(ctx->conninfo);
> ctx->conninfo = NULL;
> +
> +   ctx->cb_read = NULL;
> +   ctx->cb_write = NULL;
> +   ctx->cb_payload = NULL;
>  }
>
>  int
> @@ -580,3 +584,58 @@ tls_close(struct tls *ctx)
> errno = 0;
> return (rv);
>  }
> +
> +static int
> +tls_bio_cb_write(BIO *h, const char *buf, int num, void *payload)
> +{
> +   int ret = 0;
> +   struct tls *ctx = (struct tls *)payload;
> +   ret = (ctx->cb_write)(ctx, (const void*)buf, (size_t)num,
> ctx->cb_payload);
> +   return (ret);
> +}
> +
> +static int
> +tls_bio_cb_read(BIO *h, char *buf, int size, void *payload)
> +{
> +   int ret = 0;
> +   struct tls *ctx = (struct tls *)payload;
> +   ret = (ctx->cb_read)(ctx, (void*)buf, (size_t)size,
> ctx->cb_payload);
> +   return (ret);
> +}
> +
> +static BIO *
> +tls_get_new_cb_bio(struct tls *ctx)
> +{
> +   BIO *bcb = NULL;
> +   if (ctx->cb_read == NULL || ctx->cb_write == NULL)
> +   tls_set_errorx(ctx, "no callbacks registered");
> +   bcb = BIO_new(BIO_s_cb());
> +   if (bcb == NULL) {
> +   tls_set_errorx(ctx, "failed to create callback i/o");
> +   return (NULL);
> +   }
> +   BIO_set_cb_write(bcb, tls_bio_cb_write);
> +   BIO_set_cb_read(bcb, tls_bio_cb_read);
> +   BIO_set_cb_payload(bcb, ctx);
> +   return (bcb);
> +}
> +
> +int
> +tls_set_cbs(struct tls *ctx, tls_read_cb cb_read, tls_write_cb cb_write,
> +void *cb_payload)
> +{
> +   int rv = -1;
> +   BIO *bcb;
> +   ctx->cb_read = cb_read;
> +   ctx->cb_write = cb_write;
> +   ctx->cb_payload = cb_payload;
> +   bcb = tls_get_new_cb_bio(ctx);
> +   if (bcb == NULL) {
> +   tls_set_errorx(ctx, "failed to create callback i/o");
> +   goto err;
> +   }
> +   SSL_set_bio(ctx->ssl_conn, bcb, bcb);
> +   rv = 0;
> +err:
> +   return (rv);
> +}
> diff --git src/lib/libtls/tls.h src/lib/libtls/tls.h
> index 3e75eb7..f236245 100644
> --- src/lib/libtls/tls.h
> +++ src/lib/libtls/tls.h
> @@ -44,6 +44,11 @@ extern "C" {
>  struct tls;
>  struct tls_config;
>
> +typedef ssize_t (*tls_read_cb)(void *_ctx, void *_buf, size_t _buflen,
> +void *_payload);
> +typedef ssize_t (*tls_write_cb)(void *_ctx, const void *_buf,
> +size_t _buflen, void *_payload);
> +
>  int tls_init(void);
>
>  const char *tls_config_error(struct tls_config *_config);
> @@ -96,12 +101,16 @@ void tls_free(struct tls *_ctx);
>  int tls_accept_fds(struct tls *_ctx, struct tls **_cctx, int _fd_read,
>  int _fd_write);
>  int tls_accept_socket(struct tls *_ctx, struct tls **_cctx, int _socket);
> +int tls_accept_cbs(struct tls *_ctx, struct tls **_cctx,
> +tls_read_cb _cb_read, tls_write_cb _cb_write, void *_cb_payload);
>  int tls_connect(struct tls *_ctx, const char *_host, const char *_port);
>  int tls_connect_fds(struct tls *_ctx, int _fd_read, int _fd_write,
>  const char *_servername);
>  int tls_connect_servername(struct tls *_ctx, const char *_host,
>  const char *_port, const char *_servername);
>  int tls_connect_socket(struct tls *_ctx, int _s, const char *_servername);
> +int tls_connect_cbs(struct tls *_ctx, tls_read_cb _cb_read, tls_write_cb
> _cb_write,
> +void *_cb_payload, const char *_servername);
>  int tls_handshake(struct tls *_ctx);
>  ssize_t tls_read(struct tls *_ctx, void *_buf, size_t _buflen);
>  ssize_t tls_write(struct tls *_ctx, 

  1   2   3   4   5   >