Re: libtls syslogd pledge abort
> 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
libtls syslogd pledge abort
Hi,
The previous commit to libtls makes syslogd abort due to pledge if
certification verification is turned off. This happens in the
chrooted child process.
87878 syslogd CALL open(0x2d203ce4,0)
87878 syslogd NAMI "/etc/ssl/cert.pem"
87878 syslogd PLDG open, "rpath", errno 1 Operation not permitted
87878 syslogd PSIG SIGABRT SIG_DFL code <-538976289>
We can either preload the cert in syslogd even if verification is
turned off.
Index: usr.sbin/syslogd/syslogd.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
retrieving revision 1.225
diff -u -p -r1.225 syslogd.c
--- usr.sbin/syslogd/syslogd.c 27 Dec 2016 19:16:24 - 1.225
+++ usr.sbin/syslogd/syslogd.c 29 Dec 2016 22:57:41 -
@@ -590,16 +590,14 @@ main(int argc, char *argv[])
if (NoVerify) {
tls_config_insecure_noverifycert(client_config);
tls_config_insecure_noverifyname(client_config);
- } else {
- if (tls_config_set_ca_file(client_config,
- CAfile) == -1) {
- logerrortlsconf("Load client TLS CA failed",
- client_config);
- /* avoid reading default certs in chroot */
- tls_config_set_ca_mem(client_config, "", 0);
- } else
- logdebug("CAfile %s\n", CAfile);
}
+ if (tls_config_set_ca_file(client_config, CAfile) == -1) {
+ logerrortlsconf("Load client TLS CA failed",
+ client_config);
+ /* avoid reading default certs in chroot */
+ tls_config_set_ca_mem(client_config, "", 0);
+ } else
+ logdebug("CAfile %s\n", CAfile);
if (ClientCertfile && ClientKeyfile) {
if (tls_config_set_cert_file(client_config,
ClientCertfile) == -1)
Or do not call tls_configure_ssl_verify() if verification is turned
off.
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) {
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: doas parse.y refinement
ok benno@
Ted Unangst(t...@tedunangst.com) on 2016.12.29 14:56:47 -0500:
> it occurs to me that arglist and envlist are the same thing, a strlist.
>
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.bin/doas/parse.y,v
> retrieving revision 1.25
> diff -u -p -r1.25 parse.y
> --- parse.y 29 Dec 2016 19:12:42 - 1.25
> +++ parse.y 29 Dec 2016 19:55:33 -
> @@ -36,6 +36,7 @@ typedef struct {
> const char **cmdargs;
> const char **envlist;
> };
> + const char **strlist;
> const char *str;
> };
> int lineno;
> @@ -141,21 +142,21 @@ option: TNOPASS {
> } | TKEEPENV {
> $$.options = KEEPENV;
> $$.envlist = NULL;
> - } | TSETENV '{' envlist '}' {
> + } | TSETENV '{' strlist '}' {
> $$.options = 0;
> - $$.envlist = $3.envlist;
> + $$.envlist = $3.strlist;
> } ;
>
> -envlist: /* empty */ {
> - if (!($$.envlist = calloc(1, sizeof(char *
> - errx(1, "can't allocate envlist");
> - } | envlist TSTRING {
> - int nenv = arraylen($1.envlist);
> - if (!($$.envlist = reallocarray($1.envlist, nenv + 2,
> +strlist: /* empty */ {
> + if (!($$.strlist = calloc(1, sizeof(char *
> + errx(1, "can't allocate strlist");
> + } | strlist TSTRING {
> + int nstr = arraylen($1.strlist);
> + if (!($$.strlist = reallocarray($1.strlist, nstr + 2,
> sizeof(char *
> - errx(1, "can't allocate envlist");
> - $$.envlist[nenv] = $2.str;
> - $$.envlist[nenv + 1] = NULL;
> + errx(1, "can't allocate strlist");
> + $$.strlist[nstr] = $2.str;
> + $$.strlist[nstr + 1] = NULL;
> } ;
>
>
> @@ -179,20 +180,8 @@ cmd: /* optional */ {
>
> args:/* empty */ {
> $$.cmdargs = NULL;
> - } | TARGS argslist {
> - $$.cmdargs = $2.cmdargs;
> - } ;
> -
> -argslist:/* empty */ {
> - if (!($$.cmdargs = calloc(1, sizeof(char *
> - errx(1, "can't allocate args");
> - } | argslist TSTRING {
> - int nargs = arraylen($1.cmdargs);
> - if (!($$.cmdargs = reallocarray($1.cmdargs, nargs + 2,
> - sizeof(char *
> - errx(1, "can't allocate args");
> - $$.cmdargs[nargs] = $2.str;
> - $$.cmdargs[nargs + 1] = NULL;
> + } | TARGS strlist {
> + $$.cmdargs = $2.strlist;
> } ;
>
> %%
>
doas parse.y refinement
it occurs to me that arglist and envlist are the same thing, a strlist.
Index: parse.y
===
RCS file: /cvs/src/usr.bin/doas/parse.y,v
retrieving revision 1.25
diff -u -p -r1.25 parse.y
--- parse.y 29 Dec 2016 19:12:42 - 1.25
+++ parse.y 29 Dec 2016 19:55:33 -
@@ -36,6 +36,7 @@ typedef struct {
const char **cmdargs;
const char **envlist;
};
+ const char **strlist;
const char *str;
};
int lineno;
@@ -141,21 +142,21 @@ option: TNOPASS {
} | TKEEPENV {
$$.options = KEEPENV;
$$.envlist = NULL;
- } | TSETENV '{' envlist '}' {
+ } | TSETENV '{' strlist '}' {
$$.options = 0;
- $$.envlist = $3.envlist;
+ $$.envlist = $3.strlist;
} ;
-envlist: /* empty */ {
- if (!($$.envlist = calloc(1, sizeof(char *
- errx(1, "can't allocate envlist");
- } | envlist TSTRING {
- int nenv = arraylen($1.envlist);
- if (!($$.envlist = reallocarray($1.envlist, nenv + 2,
+strlist: /* empty */ {
+ if (!($$.strlist = calloc(1, sizeof(char *
+ errx(1, "can't allocate strlist");
+ } | strlist TSTRING {
+ int nstr = arraylen($1.strlist);
+ if (!($$.strlist = reallocarray($1.strlist, nstr + 2,
sizeof(char *
- errx(1, "can't allocate envlist");
- $$.envlist[nenv] = $2.str;
- $$.envlist[nenv + 1] = NULL;
+ errx(1, "can't allocate strlist");
+ $$.strlist[nstr] = $2.str;
+ $$.strlist[nstr + 1] = NULL;
} ;
@@ -179,20 +180,8 @@ cmd: /* optional */ {
args: /* empty */ {
$$.cmdargs = NULL;
- } | TARGS argslist {
- $$.cmdargs = $2.cmdargs;
- } ;
-
-argslist: /* empty */ {
- if (!($$.cmdargs = calloc(1, sizeof(char *
- errx(1, "can't allocate args");
- } | argslist TSTRING {
- int nargs = arraylen($1.cmdargs);
- if (!($$.cmdargs = reallocarray($1.cmdargs, nargs + 2,
- sizeof(char *
- errx(1, "can't allocate args");
- $$.cmdargs[nargs] = $2.str;
- $$.cmdargs[nargs + 1] = NULL;
+ } | TARGS strlist {
+ $$.cmdargs = $2.strlist;
} ;
%%
ntpd.conf
Markup a forgotten keyword. Jan Index: ntpd.conf.5 === RCS file: /cvs/src/usr.sbin/ntpd/ntpd.conf.5,v retrieving revision 1.33 diff -u -p -r1.33 ntpd.conf.5 --- ntpd.conf.5 23 Oct 2015 14:52:20 - 1.33 +++ ntpd.conf.5 28 Dec 2016 20:58:06 - @@ -127,7 +127,9 @@ sensor nmea0 refid GPS .Ed .Pp A stratum value other than the default of 1 can be assigned using -the stratum keyword. +the +.Ic stratum +keyword. .It Xo Ic server Ar address .Op Ic weight Ar weight-value .Xc
Re: splassert with pppd
On Thu, Dec 29, 2016 at 09:39:08AM +0100, Martin Pieuchot wrote:
> On 23/12/16(Fri) 16:33, Stefan Sperling wrote:
> > When I kill pppd, running on top of umsm(4), I see the following splasserts:
>
> Thanks for the report.
>
> > End of stack trace.
> > splassert: sorwakeup: want 1 have 0
> > Starting stack trace...
> > sorwakeup(1,0,d09d9be1,0,9cee1169) at sorwakeup+0x3f
> > sorwakeup(d90f89f8,d0b58b50,d900c600,0,d0d31bb8) at sorwakeup+0x3f
> > route_input(d900c600,d0b58b60,d0b58b50,d0b58b40,d09d086b) at
> > route_input+0x26a
> > rt_ifmsg(d40ee000,d09dd1e7,d09dd1e7,d03baa39,80) at rt_ifmsg+0xb2
> > if_linkstate(d40ee000,d09deb02,ffe9,f60d1bac,d03b0da5) at
> > if_linkstate+0x64
> > pppdealloc(d40ee000,3,f60d1bec,d03b10f4,f60d1bd8) at pppdealloc+0x36
> > pppclose(d40f8600,3,d90b8300,d9734e18,d9734e18) at pppclose+0x77
> > ttioctl(d40f8600,8004741b,f60d1e74,3,d90b8300) at ttioctl+0x6e3
> > ucom_do_ioctl(d40f7a00,8004741b,f60d1e74,3,d90b8300) at ucom_do_ioctl+0x81
> > ucomioctl(4280,8004741b,f60d1e74,3,d90b8300,0,d96c9218,d03ec5d2,d96c91c8,d96c91c8,f60d1d6c,d03e899f)
> > at ucomioctl+0x6b
> > spec_ioctl(f60d1d60,8004667d,f60d1ef0,d960a180,8004741b) at spec_ioctl+0x9b
> > VOP_IOCTL(d960a180,8004741b,f60d1e74,3,d97a3c00) at VOP_IOCTL+0x4b
> > vn_ioctl(d90b51b8,8004741b,f60d1e74,d90b8300,27) at vn_ioctl+0x7e
> > sys_ioctl(d90b8300,f60d1f5c,f60d1f7c,0,286) at sys_ioctl+0x19f
> > syscall() at syscall+0x250
>
> pppdealloc() is always called in process context, the splsoftassert() is
> currently valid because the function is called at spltty(), but now we
> need the NET_LOCK() so the diff below should do the trick.
>
> ok?
OK bluhm@
>
> Index: net/if_ppp.c
> ===
> RCS file: /cvs/src/sys/net/if_ppp.c,v
> retrieving revision 1.102
> diff -u -p -r1.102 if_ppp.c
> --- net/if_ppp.c 16 Nov 2016 14:23:10 - 1.102
> +++ net/if_ppp.c 29 Dec 2016 08:37:19 -
> @@ -306,9 +306,9 @@ void
> pppdealloc(struct ppp_softc *sc)
> {
> struct ppp_pkt *pkt;
> + int s;
>
> - splsoftassert(IPL_SOFTNET);
> -
> + NET_LOCK(s);
> if_down(>sc_if);
> sc->sc_if.if_flags &= ~(IFF_UP|IFF_RUNNING);
> sc->sc_devp = NULL;
> @@ -343,6 +343,7 @@ pppdealloc(struct ppp_softc *sc)
> sc->sc_comp = 0;
> }
> #endif
> + NET_UNLOCK(s);
> }
>
> /*
Re: Recursive splsoftnet() in in6_ifattach_linklocal()
On Thu, Dec 29, 2016 at 09:15:54AM +0100, Martin Pieuchot wrote:
> Get rid of them, ok?
OK bluhm@
>
> Index: netinet6/in6_ifattach.c
> ===
> RCS file: /cvs/src/sys/netinet6/in6_ifattach.c,v
> retrieving revision 1.100
> diff -u -p -r1.100 in6_ifattach.c
> --- netinet6/in6_ifattach.c 30 Jun 2016 08:19:03 - 1.100
> +++ netinet6/in6_ifattach.c 20 Dec 2016 10:58:33 -
> @@ -294,7 +294,9 @@ in6_ifattach_linklocal(struct ifnet *ifp
> {
> struct in6_aliasreq ifra;
> struct in6_ifaddr *ia6;
> - int s, error, flags;
> + int error, flags;
> +
> + splsoftassert(IPL_SOFTNET);
>
> /*
>* configure link-local address.
> @@ -338,9 +340,7 @@ in6_ifattach_linklocal(struct ifnet *ifp
> if (in6if_do_dad(ifp) && ((ifp->if_flags & IFF_POINTOPOINT) == 0))
> ifra.ifra_flags |= IN6_IFF_TENTATIVE;
>
> - s = splsoftnet();
> error = in6_update_ifa(ifp, , in6ifa_ifpforlinklocal(ifp, 0));
> - splx(s);
> if (error != 0)
> return (error);
>
> @@ -359,15 +359,12 @@ in6_ifattach_linklocal(struct ifnet *ifp
> if ((ifp->if_flags & IFF_POINTOPOINT) == 0)
> flags |= RTF_CLONING;
>
> - s = splsoftnet();
> error = rt_ifa_add(>ia_ifa, flags, ia6->ia_ifa.ifa_addr);
> if (error) {
> in6_purgeaddr(>ia_ifa);
> - splx(s);
> return (error);
> }
> dohooks(ifp->if_addrhooks, 0);
> - splx(s);
>
> return (0);
> }
Re: splassert with pppd
On 23/12/16(Fri) 16:33, Stefan Sperling wrote:
> When I kill pppd, running on top of umsm(4), I see the following splasserts:
Thanks for the report.
> End of stack trace.
> splassert: sorwakeup: want 1 have 0
> Starting stack trace...
> sorwakeup(1,0,d09d9be1,0,9cee1169) at sorwakeup+0x3f
> sorwakeup(d90f89f8,d0b58b50,d900c600,0,d0d31bb8) at sorwakeup+0x3f
> route_input(d900c600,d0b58b60,d0b58b50,d0b58b40,d09d086b) at route_input+0x26a
> rt_ifmsg(d40ee000,d09dd1e7,d09dd1e7,d03baa39,80) at rt_ifmsg+0xb2
> if_linkstate(d40ee000,d09deb02,ffe9,f60d1bac,d03b0da5) at
> if_linkstate+0x64
> pppdealloc(d40ee000,3,f60d1bec,d03b10f4,f60d1bd8) at pppdealloc+0x36
> pppclose(d40f8600,3,d90b8300,d9734e18,d9734e18) at pppclose+0x77
> ttioctl(d40f8600,8004741b,f60d1e74,3,d90b8300) at ttioctl+0x6e3
> ucom_do_ioctl(d40f7a00,8004741b,f60d1e74,3,d90b8300) at ucom_do_ioctl+0x81
> ucomioctl(4280,8004741b,f60d1e74,3,d90b8300,0,d96c9218,d03ec5d2,d96c91c8,d96c91c8,f60d1d6c,d03e899f)
> at ucomioctl+0x6b
> spec_ioctl(f60d1d60,8004667d,f60d1ef0,d960a180,8004741b) at spec_ioctl+0x9b
> VOP_IOCTL(d960a180,8004741b,f60d1e74,3,d97a3c00) at VOP_IOCTL+0x4b
> vn_ioctl(d90b51b8,8004741b,f60d1e74,d90b8300,27) at vn_ioctl+0x7e
> sys_ioctl(d90b8300,f60d1f5c,f60d1f7c,0,286) at sys_ioctl+0x19f
> syscall() at syscall+0x250
pppdealloc() is always called in process context, the splsoftassert() is
currently valid because the function is called at spltty(), but now we
need the NET_LOCK() so the diff below should do the trick.
ok?
Index: net/if_ppp.c
===
RCS file: /cvs/src/sys/net/if_ppp.c,v
retrieving revision 1.102
diff -u -p -r1.102 if_ppp.c
--- net/if_ppp.c16 Nov 2016 14:23:10 - 1.102
+++ net/if_ppp.c29 Dec 2016 08:37:19 -
@@ -306,9 +306,9 @@ void
pppdealloc(struct ppp_softc *sc)
{
struct ppp_pkt *pkt;
+ int s;
- splsoftassert(IPL_SOFTNET);
-
+ NET_LOCK(s);
if_down(>sc_if);
sc->sc_if.if_flags &= ~(IFF_UP|IFF_RUNNING);
sc->sc_devp = NULL;
@@ -343,6 +343,7 @@ pppdealloc(struct ppp_softc *sc)
sc->sc_comp = 0;
}
#endif
+ NET_UNLOCK(s);
}
/*
Re: if attach/detach netlocks
On 29/12/16(Thu) 01:15, Alexander Bluhm wrote:
> On Fri, Dec 23, 2016 at 12:09:32AM +0100, Martin Pieuchot wrote:
> > On 22/12/16(Thu) 20:45, Mike Belopuhov wrote:
> > > I think this is what is required here. Works here, but YMMV.
> >
> > splnet() in a pseudo-driver seems completely wrong, you could get rid of
> > it.
>
> Yes, but that is another issue. Can we get the netlock splasserts
> fixed first? This diff looks good to me.
Sure I'm ok with the diff.
> > > diff --git sys/net/if_vxlan.c sys/net/if_vxlan.c
> > > index e9bc1cb8305..dfb71cf9467 100644
> > > --- sys/net/if_vxlan.c
> > > +++ sys/net/if_vxlan.c
> > > @@ -178,13 +178,15 @@ int
> > > vxlan_clone_destroy(struct ifnet *ifp)
> > > {
> > > struct vxlan_softc *sc = ifp->if_softc;
> > > int s;
> > >
> > > + NET_LOCK(s);
> > > s = splnet();
> > > vxlan_multicast_cleanup(ifp);
> > > splx(s);
> > > + NET_UNLOCK(s);
> > >
> > > vxlan_enable--;
> > > LIST_REMOVE(sc, sc_entry);
> > >
> > > ifmedia_delete_instance(>sc_media, IFM_INST_ANY);
>
Recursive splsoftnet() in in6_ifattach_linklocal()
Get rid of them, ok?
Index: netinet6/in6_ifattach.c
===
RCS file: /cvs/src/sys/netinet6/in6_ifattach.c,v
retrieving revision 1.100
diff -u -p -r1.100 in6_ifattach.c
--- netinet6/in6_ifattach.c 30 Jun 2016 08:19:03 - 1.100
+++ netinet6/in6_ifattach.c 20 Dec 2016 10:58:33 -
@@ -294,7 +294,9 @@ in6_ifattach_linklocal(struct ifnet *ifp
{
struct in6_aliasreq ifra;
struct in6_ifaddr *ia6;
- int s, error, flags;
+ int error, flags;
+
+ splsoftassert(IPL_SOFTNET);
/*
* configure link-local address.
@@ -338,9 +340,7 @@ in6_ifattach_linklocal(struct ifnet *ifp
if (in6if_do_dad(ifp) && ((ifp->if_flags & IFF_POINTOPOINT) == 0))
ifra.ifra_flags |= IN6_IFF_TENTATIVE;
- s = splsoftnet();
error = in6_update_ifa(ifp, , in6ifa_ifpforlinklocal(ifp, 0));
- splx(s);
if (error != 0)
return (error);
@@ -359,15 +359,12 @@ in6_ifattach_linklocal(struct ifnet *ifp
if ((ifp->if_flags & IFF_POINTOPOINT) == 0)
flags |= RTF_CLONING;
- s = splsoftnet();
error = rt_ifa_add(>ia_ifa, flags, ia6->ia_ifa.ifa_addr);
if (error) {
in6_purgeaddr(>ia_ifa);
- splx(s);
return (error);
}
dohooks(ifp->if_addrhooks, 0);
- splx(s);
return (0);
}
