Re: Using unveil(2) to block the entire file system

2019-12-05 Thread Ingo Schwarze
Hi Chris,

i just committed the patch shown below;
thanks for bringing up the point.

Yours,
  Ingo



CVSROOT:/cvs
Module name:src
Changes by: schwa...@cvs.openbsd.org2019/12/05 17:14:08

Modified files:
lib/libc/sys   : unveil.2 

Log message:
Explicitly say that *permissions can be "".

Potential for misunderstanding noticed by Chris Rawnsley , wording proposed by deraadt@, patch sent by Chris
Rawnsley, OK deraadt@.


Chris Rawnsley wrote on Wed, Dec 04, 2019 at 06:34:00PM +:

> Index: lib/libc/sys/unveil.2
> ===
> RCS file: /cvs/src/lib/libc/sys/unveil.2,v
> retrieving revision 1.19
> diff -u -p -u -r1.19 unveil.2
> --- lib/libc/sys/unveil.2 25 Jul 2019 13:47:40 -  1.19
> +++ lib/libc/sys/unveil.2 4 Dec 2019 18:28:03 -
> @@ -62,7 +62,8 @@ promise.
>  .Pp
>  The
>  .Fa permissions
> -argument points to a string consisting of the following characters:
> +argument points to a string consisting of zero or more of the following
> +characters:
>  .Pp
>  .Bl -tag -width "" -offset indent -compact
>  .It Cm r



Re: Using unveil(2) to block the entire file system

2019-12-05 Thread Ingo Schwarze
Hi,

i like the tweak; OK to commit?

While it is reasonable to expect this behaviour without the "zero
or more", i see how the misunderstanding "one or more" can arise:
In many situations, to grant no permissions on a given path, it is
sufficient to not mention it in unveil(2) at all, so it may not be
obvious to everybody that the "" case is sometimes useful (and
implemented).

Yours,
  Ingo


Chris Rawnsley wrote on Wed, Dec 04, 2019 at 06:34:00PM +:
> On Wed, 4 Dec 2019, at 18:07, Theo de Raadt wrote:

>> I think it is implied, if no permissions are listed.

> Perhaps and it may be due my inexperience with C interfaces that I didn't
> think to try it.
> 
> I think your wording would have been enough for me to twig so I've made
> the patch for that instances too (if you change your mind, of course :) ).
> 
> Index: lib/libc/sys/unveil.2
> ===
> RCS file: /cvs/src/lib/libc/sys/unveil.2,v
> retrieving revision 1.19
> diff -u -p -u -r1.19 unveil.2
> --- lib/libc/sys/unveil.2 25 Jul 2019 13:47:40 -  1.19
> +++ lib/libc/sys/unveil.2 4 Dec 2019 18:28:03 -
> @@ -62,7 +62,8 @@ promise.
>  .Pp
>  The
>  .Fa permissions
> -argument points to a string consisting of the following characters:
> +argument points to a string consisting of zero or more of the following
> +characters:
>  .Pp
>  .Bl -tag -width "" -offset indent -compact
>  .It Cm r



Re: Using unveil(2) to block the entire file system

2019-12-04 Thread Chris Rawnsley
On Wed, 4 Dec 2019, at 18:07, Theo de Raadt wrote:
> I think it is implied, if no permissions are listed.

Perhaps and it may be due my inexperience with C interfaces that I didn't
think to try it.

I think your wording would have been enough for me to twig so I've made
the patch for that instances too (if you change your mind, of course :) ).

Index: lib/libc/sys/unveil.2
===
RCS file: /cvs/src/lib/libc/sys/unveil.2,v
retrieving revision 1.19
diff -u -p -u -r1.19 unveil.2
--- lib/libc/sys/unveil.2   25 Jul 2019 13:47:40 -  1.19
+++ lib/libc/sys/unveil.2   4 Dec 2019 18:28:03 -
@@ -62,7 +62,8 @@ promise.
 .Pp
 The
 .Fa permissions
-argument points to a string consisting of the following characters:
+argument points to a string consisting of zero or more of the following
+characters:
 .Pp
 .Bl -tag -width "" -offset indent -compact
 .It Cm r



Re: Using unveil(2) to block the entire file system

2019-12-04 Thread Theo de Raadt
Chris Rawnsley  wrote:

> On Wed, 4 Dec 2019, at 14:08, Theo de Raadt wrote:
> > unveil("/", "");
> > unveil(NULL, NULL);
> 
> Thank you. I didn't realise that was possible.
> 
> I tried to write an update to the man page for unveil(2). Is this
> accurate? Should I send it along to tech@?
> 
> Index: lib/libc/sys/unveil.2
> ===
> RCS file: /cvs/src/lib/libc/sys/unveil.2,v
> retrieving revision 1.19
> diff -u -p -u -r1.19 unveil.2
> --- lib/libc/sys/unveil.2 25 Jul 2019 13:47:40 -  1.19
> +++ lib/libc/sys/unveil.2 4 Dec 2019 17:38:58 -
> @@ -95,6 +95,12 @@ promise
>  .Qq cpath .
>  .El
>  .Pp
> +If
> +.Fa permissions
> +is an empty string then all operations for
> +.Fa path
> +are denied.
> +.Pp
>  A
>  .Fa path
>  that is a directory will enable all filesystem access underneath

I think it is implied, if no permissions are listed.  Maybe a tweak
like

 The permissions argument points to a string consisting of zero or more
 of the following characters:

But I don't know, I feel it is is not neccessary.



Re: Using unveil(2) to block the entire file system

2019-12-04 Thread Chris Rawnsley
On Wed, 4 Dec 2019, at 14:08, Theo de Raadt wrote:
> unveil("/", "");
> unveil(NULL, NULL);

Thank you. I didn't realise that was possible.

I tried to write an update to the man page for unveil(2). Is this
accurate? Should I send it along to tech@?

Index: lib/libc/sys/unveil.2
===
RCS file: /cvs/src/lib/libc/sys/unveil.2,v
retrieving revision 1.19
diff -u -p -u -r1.19 unveil.2
--- lib/libc/sys/unveil.2   25 Jul 2019 13:47:40 -  1.19
+++ lib/libc/sys/unveil.2   4 Dec 2019 17:38:58 -
@@ -95,6 +95,12 @@ promise
 .Qq cpath .
 .El
 .Pp
+If
+.Fa permissions
+is an empty string then all operations for
+.Fa path
+are denied.
+.Pp
 A
 .Fa path
 that is a directory will enable all filesystem access underneath



Re: Using unveil(2) to block the entire file system

2019-12-04 Thread Theo de Raadt
Chris Rawnsley  wrote:

> I applied unveil next. This went much more smoothly allowing only the
> few files required for the programme to function. However, I've realised
> since that I only need to access a few files at initialisation and then
> I can shut off all access to the file system.
> 
> From the man page on unveil(2):
> 
> > After establishing a collection of path and permissions rules, future
> > calls to unveil can be disabled by passing two NULL arguments.
> 
> i.e. you must do at least ONE successful call to unveil before you can
> lock the rest of the file system. This means unveil must be used on
> a location that exists on the file system. As a workaround, you can
> almost block access to the file system with something like
> unveil("/dev/null", "r"). However, I would have expected
> unveil(NULL, NULL) on its own would have been enough.

unveil("/", "");
unveil(NULL, NULL);

> P.S. Any tips for debugging programmes that exit from these
> technologies? I've been running ktrace(1)/kdump(1) and sort of bumbling
> through the output which seems to work okay.

That is the right tooling.



Using unveil(2) to block the entire file system

2019-12-04 Thread Chris Rawnsley
I'm making a status monitor for things like battery, time, etc. to use
with dwm. I wanted to use pledge(2) and unveil(2) to lock things down as
well as to help me learn C.

The first issue I came up against was with pledge and apm(4). The
ioctl(2) calls used to retrieve power state do not appear to be one of
the supported promises from pledge. I looked at the source for apm(8) to
discover that after getting the power state it restricts itself to the
stdio promise. As the status monitor needs on-going power information it
does not seem like a good fit to use pledge at this time.

I applied unveil next. This went much more smoothly allowing only the
few files required for the programme to function. However, I've realised
since that I only need to access a few files at initialisation and then
I can shut off all access to the file system.

>From the man page on unveil(2):

> After establishing a collection of path and permissions rules, future
> calls to unveil can be disabled by passing two NULL arguments.

i.e. you must do at least ONE successful call to unveil before you can
lock the rest of the file system. This means unveil must be used on
a location that exists on the file system. As a workaround, you can
almost block access to the file system with something like
unveil("/dev/null", "r"). However, I would have expected
unveil(NULL, NULL) on its own would have been enough.

Have I missed something trying to use pledge(2) and unveil(2)?

P.S. Any tips for debugging programmes that exit from these
technologies? I've been running ktrace(1)/kdump(1) and sort of bumbling
through the output which seems to work okay.


--
Chris Rawnsley