Hi,
Sorry to be late in the game, but as jca@ pointed out sysctl(8) tries to
open _PATH_DEVDB first and then /dev if it cannot open the former, so
both should be unveil(2)ed. Scramble the includes while at it.
Index: sysctl.c
===================================================================
RCS file: /cvs/src/sbin/sysctl/sysctl.c,v
retrieving revision 1.242
diff -u -p -u -r1.242 sysctl.c
--- sysctl.c 13 May 2019 20:47:19 -0000 1.242
+++ sysctl.c 14 Jun 2019 19:04:01 -0000
@@ -94,13 +94,14 @@
#include <ddb/db_var.h>
#include <dev/rndvar.h>
+#include <ctype.h>
#include <err.h>
#include <errno.h>
+#include <limits.h>
+#include <paths.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
-#include <ctype.h>
-#include <limits.h>
#include <unistd.h>
#include <machine/cpu.h>
@@ -162,6 +163,8 @@ struct list secondlevel[] = {
int Aflag, aflag, nflag, qflag;
+time_t boottime;
+
/*
* Variables requiring special processing.
*/
@@ -255,6 +258,15 @@ main(int argc, char *argv[])
argc -= optind;
argv += optind;
+ ctime(&boottime); /* satisfy potential $TZ expansion before unveil() */
+
+ if (unveil(_PATH_DEVDB, "r") == -1)
+ err(1,"unveil");
+ if (unveil("/dev", "r") == -1)
+ err(1, "unveil");
+ if (unveil(NULL, NULL) == -1)
+ err(1, "unveil");
+
if (argc == 0 || (Aflag || aflag)) {
debuginit();
vfsinit();
@@ -893,7 +905,6 @@ parse(char *string, int flags)
}
if (special & BOOTTIME) {
struct timeval *btp = (struct timeval *)buf;
- time_t boottime;
if (!nflag) {
boottime = btp->tv_sec;
On 10:35 Sat 08 Jun , Theo de Raadt wrote:
> When userland was massaged for pledge(), I hesitated using the
> "manually call tzset()" approach for handling things. It felt
> too low-level to call tzset(), an API almost noone knows the
> existance of.
>
> Arriving in the same situation to satisfy unveil(). Again calling
> tzset() feels too unfamiliar and low level.
>
> Regarding the comment in your diff, it says "localtime", but what is
> actually called is ctime(), which calls localtime() (which calls
> tzset(), which is where the unveil-files-missing or pledge-whatver
> issues would show up in some programs). Probably should adjust
> the comment
>
> Here's the later troublesome chunk:
>
> if (special & BOOTTIME) {
> struct timeval *btp = (struct timeval *)buf;
> time_t boottime;
>
> if (!nflag) {
> boottime = btp->tv_sec;
> (void)printf("%s%s%s", string, equ, ctime(&boottime));
>
> That makes me wonder, can we be less obtuse up front, and
> prime the subsystem before unveil by calling the same function which
> will be called later?
>
> Something like this. It feels slightly better to me.
>
> Index: sysctl.c
> ===================================================================
> RCS file: /cvs/src/sbin/sysctl/sysctl.c,v
> retrieving revision 1.242
> diff -u -p -u -r1.242 sysctl.c
> --- sysctl.c 13 May 2019 20:47:19 -0000 1.242
> +++ sysctl.c 8 Jun 2019 16:33:07 -0000
> @@ -162,6 +162,8 @@ struct list secondlevel[] = {
>
> int Aflag, aflag, nflag, qflag;
>
> +time_t boottime;
> +
> /*
> * Variables requiring special processing.
> */
> @@ -255,6 +257,12 @@ main(int argc, char *argv[])
> argc -= optind;
> argv += optind;
>
> + ctime(&boottime); /* satisfy potential $TZ expansion before unveil() */
> +
> + if (unveil("/dev", "r") == -1)
> + err(1, "unveil");
> + if (unveil(NULL, NULL) == -1)
> + err(1, "unveil");
> if (argc == 0 || (Aflag || aflag)) {
> debuginit();
> vfsinit();
> @@ -893,7 +901,6 @@ parse(char *string, int flags)
> }
> if (special & BOOTTIME) {
> struct timeval *btp = (struct timeval *)buf;
> - time_t boottime;
>
> if (!nflag) {
> boottime = btp->tv_sec;
>
>
>
>
>
> Florian Obser <[email protected]> wrote:
>
> > On Fri, Jun 07, 2019 at 11:24:30PM +0100, Ricardo Mestre wrote:
> > > i did that and for some for reason i didn't get it! it tries to open
> > > timezone so it kinda looks like a red flag right there...
> > >
> > > apart from /dev do we need to look into TZ on this one as well? if TZ
> > > var needs to be looked at then all bets are off :/
> >
> > this seems to do the right thing:
> >
> > diff --git sysctl.c sysctl.c
> > index dc6abc16670..c74e706942a 100644
> > --- sysctl.c
> > +++ sysctl.c
> > @@ -255,6 +255,13 @@ main(int argc, char *argv[])
> > argc -= optind;
> > argv += optind;
> >
> > + tzset(); /* for kern.boottime in localtime */
> > +
> > + if (unveil("/dev", "r") == -1)
> > + err(1, "unveil");
> > + if (unveil(NULL, NULL) == -1)
> > + err(1, "unveil");
> > +
> > if (argc == 0 || (Aflag || aflag)) {
> > debuginit();
> > vfsinit();
> >
> >
> > --
> > I'm not entirely sure you are real.
> >
On 10:35 Sat 08 Jun , Theo de Raadt wrote:
> When userland was massaged for pledge(), I hesitated using the
> "manually call tzset()" approach for handling things. It felt
> too low-level to call tzset(), an API almost noone knows the
> existance of.
>
> Arriving in the same situation to satisfy unveil(). Again calling
> tzset() feels too unfamiliar and low level.
>
> Regarding the comment in your diff, it says "localtime", but what is
> actually called is ctime(), which calls localtime() (which calls
> tzset(), which is where the unveil-files-missing or pledge-whatver
> issues would show up in some programs). Probably should adjust
> the comment
>
> Here's the later troublesome chunk:
>
> if (special & BOOTTIME) {
> struct timeval *btp = (struct timeval *)buf;
> time_t boottime;
>
> if (!nflag) {
> boottime = btp->tv_sec;
> (void)printf("%s%s%s", string, equ, ctime(&boottime));
>
> That makes me wonder, can we be less obtuse up front, and
> prime the subsystem before unveil by calling the same function which
> will be called later?
>
> Something like this. It feels slightly better to me.
>
> Index: sysctl.c
> ===================================================================
> RCS file: /cvs/src/sbin/sysctl/sysctl.c,v
> retrieving revision 1.242
> diff -u -p -u -r1.242 sysctl.c
> --- sysctl.c 13 May 2019 20:47:19 -0000 1.242
> +++ sysctl.c 8 Jun 2019 16:33:07 -0000
> @@ -162,6 +162,8 @@ struct list secondlevel[] = {
>
> int Aflag, aflag, nflag, qflag;
>
> +time_t boottime;
> +
> /*
> * Variables requiring special processing.
> */
> @@ -255,6 +257,12 @@ main(int argc, char *argv[])
> argc -= optind;
> argv += optind;
>
> + ctime(&boottime); /* satisfy potential $TZ expansion before unveil() */
> +
> + if (unveil("/dev", "r") == -1)
> + err(1, "unveil");
> + if (unveil(NULL, NULL) == -1)
> + err(1, "unveil");
> if (argc == 0 || (Aflag || aflag)) {
> debuginit();
> vfsinit();
> @@ -893,7 +901,6 @@ parse(char *string, int flags)
> }
> if (special & BOOTTIME) {
> struct timeval *btp = (struct timeval *)buf;
> - time_t boottime;
>
> if (!nflag) {
> boottime = btp->tv_sec;
>
>
>
>
>
> Florian Obser <[email protected]> wrote:
>
> > On Fri, Jun 07, 2019 at 11:24:30PM +0100, Ricardo Mestre wrote:
> > > i did that and for some for reason i didn't get it! it tries to open
> > > timezone so it kinda looks like a red flag right there...
> > >
> > > apart from /dev do we need to look into TZ on this one as well? if TZ
> > > var needs to be looked at then all bets are off :/
> >
> > this seems to do the right thing:
> >
> > diff --git sysctl.c sysctl.c
> > index dc6abc16670..c74e706942a 100644
> > --- sysctl.c
> > +++ sysctl.c
> > @@ -255,6 +255,13 @@ main(int argc, char *argv[])
> > argc -= optind;
> > argv += optind;
> >
> > + tzset(); /* for kern.boottime in localtime */
> > +
> > + if (unveil("/dev", "r") == -1)
> > + err(1, "unveil");
> > + if (unveil(NULL, NULL) == -1)
> > + err(1, "unveil");
> > +
> > if (argc == 0 || (Aflag || aflag)) {
> > debuginit();
> > vfsinit();
> >
> >
> > --
> > I'm not entirely sure you are real.
> >
