9front - general discussion about 9front
 help / color / mirror / Atom feed
* [9front] [PATCH] Unmount to remove sharp devices.
@ 2022-05-04 14:09 Jacob Moody
  2022-05-04 15:05 ` ori
  2022-05-04 15:31 ` ori
  0 siblings, 2 replies; 28+ messages in thread
From: Jacob Moody @ 2022-05-04 14:09 UTC (permalink / raw)
  To: 9front

Hello,

This patch allows processes to unmount sharp devices to prevent itself and its children from accessing
them. This is implemented through an internal rework of how RFNOMNT works, making RFNOMNT a special
case of setting disallowed devices. To replicate the mount blocking functionality of RFNOMNT a special
case is given for blocking devmnt, which also blocks the process and its children from making any mount
calls.

If everything passes the sniff test I can commit these changes. Diff is here: http://okturing.com/src/13574/body and included
below.

Thanks,
moody

diff 9126ee3eea90d639f4e877c01400248581d10f65 uncommitted
--- a//sys/man/3/0intro
+++ b//sys/man/3/0intro
@@ -87,6 +87,20 @@
 commands will need quotes to protect the
 .B #
 characters.
+.PP
+Access to a kernel device can be forfeited by
+unmounting it. For example, after
+.IP
+.EX
+unmount(nil, "#c")
+.EE
+.LP
+the calling process and its children can no longer access the device through its
+sharp name. Existing binds of these devices are untouched.
+Unmounting the mount device (see
+.IR mnt (3))
+has the side effect of preventing the calling process and its children
+from performing mounts.
 .SH SEE ALSO
 .IR intro (5),
 .IR intro (2)
--- a//sys/src/9/port/chan.c
+++ b//sys/src/9/port/chan.c
@@ -1313,24 +1313,19 @@
 			up->genbuf[n++] = *name++;
 		}
 		up->genbuf[n] = '\0';
-		/*
-		 *  noattach is sandboxing.
-		 *
-		 *  the OK exceptions are:
-		 *	|  it only gives access to pipes you create
-		 *	d  this process's file descriptors
-		 *	e  this process's environment
-		 *  the iffy exceptions are:
-		 *	c  time and pid, but also cons and consctl
-		 *	p  control of your own processes (and unfortunately
-		 *	   any others left unprotected)
-		 */
 		n = chartorune(&r, up->genbuf+1)+1;
-		if(up->pgrp->noattach && utfrune("|decp", r)==nil)
-			error(Enoattach);
 		t = devno(r, 1);
 		if(t == -1)
 			error(Ebadsharp);
+		/*
+		 * Notallowed is sandboxing.
+		 * A process group can willingly give up access to a
+		 * sharp device by unmounting it. Notallowed itself is
+		 * a bitmask of indicies in to devtab, a high bit indicating
+		 * that the process has given up its right to use that device.
+		 */
+		if((up->pgrp->notallowed[t/(sizeof(u64int)*8)] & 1<<(t%(sizeof(u64int)*8))) != 0)
+			error(Enoattach);
 		c = devtab[t]->attach(up->genbuf+n);
 		break;

--- a//sys/src/9/port/devshr.c
+++ b//sys/src/9/port/devshr.c
@@ -464,7 +464,7 @@
 		cclose(c);
 		return nc;	
 	case Qcroot:
-		if(up->pgrp->noattach)
+		if(!canmount(up->pgrp, 1))
 			error(Enoattach);
 		if((perm & DMDIR) == 0 || mode != OREAD)
 			error(Eperm);
@@ -498,7 +498,7 @@
 		sch->shr = shr;
 		break;
 	case Qcshr:
-		if(up->pgrp->noattach)
+		if(!canmount(up->pgrp, 1))
 			error(Enoattach);
 		if((perm & DMDIR) != 0 || mode != OWRITE)
 			error(Eperm);
@@ -731,7 +731,7 @@
 	Mhead *h;
 	Mount *m;

-	if(up->pgrp->noattach)
+	if(!canmount(up->pgrp, 1))
 		error(Enoattach);
 	sch = tosch(c);
 	if(sch->level != Qcmpt)
--- a//sys/src/9/port/mkdevc
+++ b//sys/src/9/port/mkdevc
@@ -78,6 +78,9 @@
 	if(ARGC < 2)
 		exit "usage"

+	if(ndev >= 256)
+		exit "device count will overflow Pgrp.notallowed"
+
 	printf "#include \"u.h\"\n";
 	printf "#include \"../port/lib.h\"\n";
 	printf "#include \"mem.h\"\n";
--- a//sys/src/9/port/portdat.h
+++ b//sys/src/9/port/portdat.h
@@ -484,7 +484,7 @@
 {
 	Ref;
 	RWlock	ns;			/* Namespace n read/one write lock */
-	int	noattach;
+	u64int	notallowed[4];		/* Room for 256 devices */
 	Mhead	*mnthash[MNTHASH];
 };

--- a//sys/src/9/port/portfns.h
+++ b//sys/src/9/port/portfns.h
@@ -413,6 +413,7 @@
 ushort		nhgets(void*);
 ulong		µs(void);
 long		lcycles(void);
+int		canmount(Pgrp*,int);

 #pragma varargck argpos iprint	1
 #pragma	varargck argpos	panic	1
--- a//sys/src/9/port/sysfile.c
+++ b//sys/src/9/port/sysfile.c
@@ -1048,7 +1048,7 @@
 			nexterror();
 		}

-		if(up->pgrp->noattach)
+		if(!canmount(up->pgrp, 1))
 			error(Enoattach);

 		ac = nil;
@@ -1146,6 +1146,8 @@
 {
 	Chan *cmount, *cmounted;
 	char *name, *old;
+	int t;
+	Rune r;

 	name = va_arg(list, char*);
 	old = va_arg(list, char*);
@@ -1152,6 +1154,14 @@

 	cmounted = nil;
 	validaddr((uintptr)old, 1, 0);
+	if(old[0] == '#' && utflen(old) == 2){
+		chartorune(&r, old+1);
+		t = devno(r, 1);
+		if(t == -1)
+			error(Ebadsharp);
+		up->pgrp->notallowed[t/(sizeof(u64int)*8)] |= 1<<(t%(sizeof(u64int)*8));
+		return 0;
+	}
 	cmount = namec(old, Amount, 0, 0);
 	if(waserror()) {
 		cclose(cmount);
--- a//sys/src/9/port/sysproc.c
+++ b//sys/src/9/port/sysproc.c
@@ -23,6 +23,43 @@
 	pexit("fork aborted", 1);
 }

+int
+canmount(Pgrp *p, int user)
+{
+	int t;
+
+	/*
+	 * Devmnt is not usable directly from user procs, so
+	 * having it removed is interpreted to block any mounts.
+	 */
+	t = devno('M', user);
+	if(t != -1 && (p->notallowed[t/(sizeof(u64int)*8)] & 1<<(t%(sizeof(u64int)*8))) != 0)
+		return 0;
+	return 1;
+}
+
+static void
+setrfnomnt(Pgrp *p)
+{
+	int t, i;
+	u64int mask[nelem(p->notallowed)];
+	char allowed[] = "|decp";
+
+	/*
+	 * Code using RFNOMNT expects to block all sharp devices
+	 * and mounting except for the devices defined in allowed[]
+	 */
+	for(i=0; i < sizeof allowed; i++){
+		t = devno(allowed[i], 1);
+		if(t == -1)
+			continue;
+		mask[t/(sizeof(u64int)*8)] |= 1<<(t%(sizeof(u64int)*8));
+	}
+	/* Sets bit for devmnt, so all mounts are disabled implicitly */
+	for(i=0; i < nelem(p->notallowed); i++)
+		p->notallowed[i] |= ~mask[i];
+}
+
 uintptr
 sysrfork(va_list list)
 {
@@ -60,12 +97,12 @@
 			up->pgrp = newpgrp();
 			if(flag & RFNAMEG)
 				pgrpcpy(up->pgrp, opg);
-			/* inherit noattach */
-			up->pgrp->noattach = opg->noattach;
+			/* inherit notallowed */
+			memmove(up->pgrp->notallowed, opg->notallowed, sizeof up->pgrp->notallowed);
 			closepgrp(opg);
 		}
 		if(flag & RFNOMNT)
-			up->pgrp->noattach = 1;
+			setrfnomnt(up->pgrp);
 		if(flag & RFREND) {
 			org = up->rgrp;
 			up->rgrp = newrgrp();
@@ -177,8 +214,8 @@
 		p->pgrp = newpgrp();
 		if(flag & RFNAMEG)
 			pgrpcpy(p->pgrp, up->pgrp);
-		/* inherit noattach */
-		p->pgrp->noattach = up->pgrp->noattach;
+		/* inherit notallowed */
+		memmove(p->pgrp->notallowed, up->pgrp->notallowed, sizeof p->pgrp->notallowed);
 	}
 	else {
 		p->pgrp = up->pgrp;
@@ -185,7 +222,7 @@
 		incref(p->pgrp);
 	}
 	if(flag & RFNOMNT)
-		p->pgrp->noattach = 1;
+		setrfnomnt(p->pgrp);

 	if(flag & RFREND)
 		p->rgrp = newrgrp();


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-04 14:09 [9front] [PATCH] Unmount to remove sharp devices Jacob Moody
@ 2022-05-04 15:05 ` ori
  2022-05-04 15:31 ` ori
  1 sibling, 0 replies; 28+ messages in thread
From: ori @ 2022-05-04 15:05 UTC (permalink / raw)
  To: 9front

Quoth Jacob Moody <moody@mail.posixcafe.org>:
> Hello,
> 
> This patch allows processes to unmount sharp devices to prevent itself and its children from accessing
> them. This is implemented through an internal rework of how RFNOMNT works, making RFNOMNT a special
> case of setting disallowed devices. To replicate the mount blocking functionality of RFNOMNT a special
> case is given for blocking devmnt, which also blocks the process and its children from making any mount
> calls.
> 
> If everything passes the sniff test I can commit these changes. Diff is here: http://okturing.com/src/13574/body and included
> below.
> 
> Thanks,
> moody
> 

Very nice -- I'm going to try to apply the patch and port shithub
scripts to use it this weekend. Thinking about how I'd use it,
I wonder if there's a clean way to remove all devices without
disabling mounts, since blacklists are a bit of a pain. Maybe

	unmount `{awk '/^#[^keep]/{print $1} /dev/drivers}

is good enough.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-04 14:09 [9front] [PATCH] Unmount to remove sharp devices Jacob Moody
  2022-05-04 15:05 ` ori
@ 2022-05-04 15:31 ` ori
  2022-05-04 16:15   ` Stanley Lieber
  2022-05-04 17:41   ` Lyndon Nerenberg (VE7TFX/VE6BBM)
  1 sibling, 2 replies; 28+ messages in thread
From: ori @ 2022-05-04 15:31 UTC (permalink / raw)
  To: 9front

Quoth Jacob Moody <moody@mail.posixcafe.org>:
> Hello,
> 
> This patch allows processes to unmount sharp devices to prevent itself and its children from accessing
> them. This is implemented through an internal rework of how RFNOMNT works, making RFNOMNT a special
> case of setting disallowed devices. To replicate the mount blocking functionality of RFNOMNT a special
> case is given for blocking devmnt, which also blocks the process and its children from making any mount
> calls.
> 
> If everything passes the sniff test I can commit these changes. Diff is here: http://okturing.com/src/13574/body and included
> below.
> 
> Thanks,
> moody
> 

also -- are there places that you want to take
advantage of this within the system? It'd be
good to use the feature, rather than just having
it sit around.

git/serve? rc-httpd? maybe some other daemons
could use some sandboxing.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-04 15:31 ` ori
@ 2022-05-04 16:15   ` Stanley Lieber
  2022-05-04 17:41   ` Lyndon Nerenberg (VE7TFX/VE6BBM)
  1 sibling, 0 replies; 28+ messages in thread
From: Stanley Lieber @ 2022-05-04 16:15 UTC (permalink / raw)
  To: 9front

this is very welcome.

one problem with getting rid of mounts and binds is programs that use mounts and binds.

sl

> On May 4, 2022, at 11:32 AM, ori@eigenstate.org wrote:
> 
> Quoth Jacob Moody <moody@mail.posixcafe.org>:
>> Hello,
>> 
>> This patch allows processes to unmount sharp devices to prevent itself and its children from accessing
>> them. This is implemented through an internal rework of how RFNOMNT works, making RFNOMNT a special
>> case of setting disallowed devices. To replicate the mount blocking functionality of RFNOMNT a special
>> case is given for blocking devmnt, which also blocks the process and its children from making any mount
>> calls.
>> 
>> If everything passes the sniff test I can commit these changes. Diff is here: http://okturing.com/src/13574/body and included
>> below.
>> 
>> Thanks,
>> moody
>> 
> 
> also -- are there places that you want to take
> advantage of this within the system? It'd be
> good to use the feature, rather than just having
> it sit around.
> 
> git/serve? rc-httpd? maybe some other daemons
> could use some sandboxing.
> 
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-04 15:31 ` ori
  2022-05-04 16:15   ` Stanley Lieber
@ 2022-05-04 17:41   ` Lyndon Nerenberg (VE7TFX/VE6BBM)
  2022-05-04 17:55     ` Jacob Moody
  1 sibling, 1 reply; 28+ messages in thread
From: Lyndon Nerenberg (VE7TFX/VE6BBM) @ 2022-05-04 17:41 UTC (permalink / raw)
  To: 9front

> git/serve? rc-httpd? maybe some other daemons
> could use some sandboxing.

The FTP and TFTP servers are obvious targets. But what would be
really cool is if this could be integrated into the namespace
files. That way you could set up the namespace for something like
FTP, and then remove the ability to mount #foo without having to
touch the FTP code itself. I could see myself using this all over
the place.

--lyndon

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-04 17:41   ` Lyndon Nerenberg (VE7TFX/VE6BBM)
@ 2022-05-04 17:55     ` Jacob Moody
  2022-05-05  1:59       ` Alex Musolino
  0 siblings, 1 reply; 28+ messages in thread
From: Jacob Moody @ 2022-05-04 17:55 UTC (permalink / raw)
  To: 9front

On 5/4/22 11:41, Lyndon Nerenberg (VE7TFX/VE6BBM) wrote:
>> git/serve? rc-httpd? maybe some other daemons
>> could use some sandboxing.
> 
> The FTP and TFTP servers are obvious targets. But what would be
> really cool is if this could be integrated into the namespace
> files. That way you could set up the namespace for something like
> FTP, and then remove the ability to mount #foo without having to
> touch the FTP code itself. I could see myself using this all over
> the place.
> 
> --lyndon

This is already true to some extent as is with this patch.
Adding an 'unmount #e' to your namespace file will remove
access to #e as is. However I want to look a bit more
at doing the inverse operation, so you could say something like
"permit #e #| #p" and have it block all non listed
devices in the namespace file. I think that is more useful
to think about and avoids 10 lines of manually selecting drivers
to shoot.

Perhaps a new /lib/namespace.service could become the default
for networked services. Either way, with the kernel
modifications passing the sniff test, I will spend some
time seeing how to integrate this change in to existing daemons.

Thanks,
moody

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-04 17:55     ` Jacob Moody
@ 2022-05-05  1:59       ` Alex Musolino
  2022-05-05 16:07         ` Jacob Moody
  0 siblings, 1 reply; 28+ messages in thread
From: Alex Musolino @ 2022-05-05  1:59 UTC (permalink / raw)
  To: 9front

> However I want to look a bit more at doing the inverse operation, so
> you could say something like "permit #e #| #p" and have it block all
> non listed devices in the namespace file.  I think that is more
> useful to think about and avoids 10 lines of manually selecting
> drivers to shoot.

Absolutely.  Also avoids the problem of new kernel devices being added
that then slip through, and the corresponding updates to all existing
namespace files.

Did you consider implementing this with two commands (e.g.  'drop' and
'permit') on the /dev/drivers file itself?  That would make it just as
easy to do this kind of stuff from a script/program without having to
craft a namespace file.

Obviously you'd have to have the #c device available until you're
finished "dropping" and "permitting" things, but perhaps that's not
really a problem.

Just some thoughts.

--
Cheers,
Alex Musolino


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-05  1:59       ` Alex Musolino
@ 2022-05-05 16:07         ` Jacob Moody
  2022-05-08  2:55           ` Jacob Moody
  0 siblings, 1 reply; 28+ messages in thread
From: Jacob Moody @ 2022-05-05 16:07 UTC (permalink / raw)
  To: 9front

On 5/4/22 19:59, Alex Musolino wrote:
> Did you consider implementing this with two commands (e.g.  'drop' and
> 'permit') on the /dev/drivers file itself?  That would make it just as
> easy to do this kind of stuff from a script/program without having to
> craft a namespace file.
> 
> Obviously you'd have to have the #c device available until you're
> finished "dropping" and "permitting" things, but perhaps that's not
> really a problem.

I think this is an excellent idea! The logic for doing permit within
the kernel is much easier, as we already have an array of all current
devices. I had started implementing a permit(1) and permit command for
namespace files and that necessitated reading /dev/drivers to get a
full list of current devices. While not a lot of code to do it, it
felt like I shouldn't have to do the inversion myself from userspace.

Unless others feel strongly about implementing device removal
through unmount I am going to plan to change the implementation
to do this instead.

Also from my understanding, dropping #c in practice shouldn't be
an issue, once you have a fd to #c/drivers, you can block #c and the fd
will still accept further writes. You may not be able to reopen
to #c/drivers file after closing that fd, but that seems completely
reasonable to me.

Thanks,
moody

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-05 16:07         ` Jacob Moody
@ 2022-05-08  2:55           ` Jacob Moody
  2022-05-11 14:47             ` Jacob Moody
  2022-05-12  3:18             ` ori
  0 siblings, 2 replies; 28+ messages in thread
From: Jacob Moody @ 2022-05-08  2:55 UTC (permalink / raw)
  To: 9front

Hello,

Gave this another go: http://okturing.com/src/13592/body

This patch includes new namepace file for rc-httpd
and changes the existing anonymous login namespace for
ftpd, both make use of the sandboxing features.

thanks,
moody

diff 9126ee3eea90d639f4e877c01400248581d10f65 uncommitted
--- a/lib/namespace
+++ b/lib/namespace
@@ -1,5 +1,6 @@
 # root
 mount -aC #s/boot /root $rootspec
+bind $rootdir'/rc' /rc
 bind -a $rootdir /

 # kernel devices
--- a/lib/namespace.ftp
+++ b/lib/namespace.ftp
@@ -8,5 +8,6 @@
 # bind a personal incoming directory below incoming
 bind -c /usr/none/incoming /usr/web/incoming/none

+permit |MedIa/
 # this cuts off everything not mounted below /usr/web
 bind /usr/web /
--- /tmp/diff100000742872
+++ b/lib/namespace.rc-httpd
@@ -1,0 +1,21 @@
+mount -aC #s/boot /root $rootspec
+
+# kernel devices
+bind #c /dev
+bind #d /fd
+bind -c #e /env
+bind #p /proc
+bind -a #l /net
+bind -a #I /net
+
+bind /root/$cputype/bin /bin
+bind /root/rc /rc
+bind -a /rc/bin /bin
+
+permit Mcde|plI/
+
+. /root/cfg/$sysname/namespace.rc-httpd
+
+unmount /root
+block Mc
+unmount #c /dev
--- a/sys/man/3/cons
+++ b/sys/man/3/cons
@@ -90,10 +90,30 @@
 .PP
 The
 .B drivers
-file contains, one per line, a listing of the drivers configured in the kernel, in the format
+file contains, one per line, a listing of available kernel drivers, in the format
 .IP
 .EX
 #c cons
+.EE
+.PP
+A process can forfeit access to a driver, for itself and it's future children, through a write to
+.B drivers.
+A message is one of:
+.IP "block \f2drivers\fP"
+block access to the listed
+.I drivers.
+.IP "permit \f2drivers\fP"
+permit access to just the provided
+.I drivers.
+.PP
+Drivers are identified by their short hand, the first column returned on reads,
+without the leading sharp. The following blocks access to
+.IR env (3)
+and
+.IR sd (3):
+.IP
+.EX
+block se
 .EE
 .PP
 The
--- a/sys/man/6/namespace
+++ b/sys/man/6/namespace
@@ -59,6 +59,15 @@
 .I new
 is missing.
 .TP
+.BI block \ drivers
+Removes access to the listed kernel
+.I drivers.
+Devices are identified by their sharp name.
+.TP
+.BI permit \ drivers
+Permit access to only the listed kernel
+.I drivers.
+.TP
 .BR clear
 Clear the name space with
 .BR rfork(RFCNAMEG) .
@@ -80,4 +89,5 @@
 .SH "SEE ALSO"
 .IR bind (1),
 .IR namespace (4),
-.IR init (8)
+.IR init (8),
+.IR cons (3)
--- a/sys/src/9/boot/boot.c
+++ b/sys/src/9/boot/boot.c
@@ -19,6 +19,7 @@
 	if(await(buf, sizeof(buf)) < 0)
 		goto Err;

+	bind("/root/rc", "/rc", MREPL);
 	bind(root, "/", MAFTER);

 	buf[0] = '/';
--- a/sys/src/9/port/chan.c
+++ b/sys/src/9/port/chan.c
@@ -1272,7 +1272,7 @@
 Chan*
 namec(char *aname, int amode, int omode, ulong perm)
 {
-	int len, n, t, nomount;
+	int len, n, t, nomount, devunmount;
 	Chan *c;
 	Chan *volatile cnew;
 	Path *volatile path;
@@ -1292,6 +1292,24 @@
 	name = aname;

 	/*
+	 * When unmounting, the name parameter must be accessed
+	 * using Aopen in order to get the real chan from
+	 * something like /srv/cs or /fd/0. However when sandboxing,
+	 * unmounting a sharp from a union is a valid operation even
+	 * if the device is blocked.
+	 */
+	if(amode == Aunmount){
+		/*
+		 * Doing any walks down the device could leak information
+		 * about the existence of files.
+		 */
+		if(name[0] == '#' && utflen(name) == 2)
+			devunmount = 1;
+		amode = Aopen;
+	} else
+		devunmount = 0;
+
+	/*
 	 * Find the starting off point (the current slash, the root of
 	 * a device tree, or the current dot) as well as the name to
 	 * evaluate starting there.
@@ -1313,24 +1331,13 @@
 			up->genbuf[n++] = *name++;
 		}
 		up->genbuf[n] = '\0';
-		/*
-		 *  noattach is sandboxing.
-		 *
-		 *  the OK exceptions are:
-		 *	|  it only gives access to pipes you create
-		 *	d  this process's file descriptors
-		 *	e  this process's environment
-		 *  the iffy exceptions are:
-		 *	c  time and pid, but also cons and consctl
-		 *	p  control of your own processes (and unfortunately
-		 *	   any others left unprotected)
-		 */
 		n = chartorune(&r, up->genbuf+1)+1;
-		if(up->pgrp->noattach && utfrune("|decp", r)==nil)
-			error(Enoattach);
 		t = devno(r, 1);
 		if(t == -1)
 			error(Ebadsharp);
+		if(!devunmount && !driversallowed(up->pgrp, r))
+			error(Enoattach);
+
 		c = devtab[t]->attach(up->genbuf+n);
 		break;

--- a/sys/src/9/port/devcons.c
+++ b/sys/src/9/port/devcons.c
@@ -39,6 +39,18 @@
 	CMrdb,		"rdb",		0,
 };

+enum
+{
+	CMblock,
+	CMpermit,
+};
+
+Cmdtab drivermsg[] =
+{
+	CMblock,	"block",	0,
+	CMpermit,	"permit",	0,
+};
+
 void
 printinit(void)
 {
@@ -332,7 +344,7 @@
 	"cons",		{Qcons},	0,		0660,
 	"consctl",	{Qconsctl},	0,		0220,
 	"cputime",	{Qcputime},	6*NUMSIZE,	0444,
-	"drivers",	{Qdrivers},	0,		0444,
+	"drivers",	{Qdrivers},	0,		0666,
 	"hostdomain",	{Qhostdomain},	DOMLEN,		0664,
 	"hostowner",	{Qhostowner},	0,		0664,
 	"kmesg",	{Qkmesg},	0,		0440,
@@ -583,9 +595,15 @@
 	case Qdrivers:
 		b = smalloc(READSTR);
 		k = 0;
-		for(i = 0; devtab[i] != nil; i++)
+		
+		rlock(&up->pgrp->ns);
+		for(i = 0; devtab[i] != nil; i++){
+			if(up->pgrp->notallowed[i/(sizeof(u64int)*8)] & 1<<i%(sizeof(u64int)*8))
+				continue;
 			k += snprint(b+k, READSTR-k, "#%C %s\n",
 				devtab[i]->dc, devtab[i]->name);
+		}
+		runlock(&up->pgrp->ns);
 		if(waserror()){
 			free(b);
 			nexterror();
@@ -676,6 +694,26 @@
 		error(Eperm);
 		break;

+	case Qdrivers:
+		cb = parsecmd(a, n);
+
+		if(waserror()) {
+			free(cb);
+			nexterror();
+		}
+		ct = lookupcmd(cb, drivermsg, nelem(drivermsg));
+		switch(ct->index) {
+		case CMblock:
+			driverscmd(up->pgrp, 0, cb->nf-1, cb->f+1);
+			break;
+		case CMpermit:
+			driverscmd(up->pgrp, 1, cb->nf-1, cb->f+1);
+			break;
+		}
+		poperror();
+		free(cb);
+		break;
+
 	case Qreboot:
 		if(!iseve())
 			error(Eperm);
@@ -935,6 +973,64 @@
 		break;
 	}
 	return n+1;
+}
+
+void
+driverscmd(Pgrp *pgrp, int invert, int argc, char *argv[])
+{
+	int i, t, w;
+	char *p;
+	Rune r;
+	u64int mask[nelem(pgrp->notallowed)];
+
+	if(invert)
+		memset(mask, 0xFF, sizeof mask);
+	else
+		memset(mask, 0, sizeof mask);
+
+	w = sizeof mask[0] * 8;
+	for(i=0; i < argc; i++)
+		for(p = argv[i]; *p != '\0';){
+			p += chartorune(&r, p);
+			t = devno(r, 1);
+			if(t == -1)
+				continue;
+			if(invert)
+				mask[t/w] &= ~(1<<t%w);
+			else
+				mask[t/w] |= 1<<t%w;
+		}
+
+	wlock(&pgrp->ns);
+	for(i=0; i < nelem(pgrp->notallowed); i++)
+		pgrp->notallowed[i] |= mask[i];
+	wunlock(&pgrp->ns);
+}
+
+int
+driversallowed(Pgrp *pgrp, int r)
+{
+	int t, w, b;
+
+	t = devno(r, 1);
+	if(t == -1)
+		return 0;
+
+	w = sizeof(u64int) * 8;
+	rlock(&pgrp->ns);
+	b = !(pgrp->notallowed[t/w] & 1<<t%w);
+	runlock(&pgrp->ns);
+	return b;
+}
+
+int
+canmount(Pgrp *pgrp)
+{
+	/*
+	 * Devmnt is not usable directly from user procs, so
+	 * having it removed is interpreted to block any mounts.
+	 */
+	return driversallowed(pgrp, 'M');
 }

 void
--- a/sys/src/9/port/devroot.c
+++ b/sys/src/9/port/devroot.c
@@ -105,6 +105,7 @@
 	addrootdir("net");
 	addrootdir("net.alt");
 	addrootdir("proc");
+	addrootdir("rc");
 	addrootdir("root");
 	addrootdir("srv");
 	addrootdir("shr");
--- a/sys/src/9/port/devshr.c
+++ b/sys/src/9/port/devshr.c
@@ -464,7 +464,7 @@
 		cclose(c);
 		return nc;	
 	case Qcroot:
-		if(up->pgrp->noattach)
+		if(!canmount(up->pgrp))
 			error(Enoattach);
 		if((perm & DMDIR) == 0 || mode != OREAD)
 			error(Eperm);
@@ -498,7 +498,7 @@
 		sch->shr = shr;
 		break;
 	case Qcshr:
-		if(up->pgrp->noattach)
+		if(!canmount(up->pgrp))
 			error(Enoattach);
 		if((perm & DMDIR) != 0 || mode != OWRITE)
 			error(Eperm);
@@ -731,7 +731,7 @@
 	Mhead *h;
 	Mount *m;

-	if(up->pgrp->noattach)
+	if(!canmount(up->pgrp))
 		error(Enoattach);
 	sch = tosch(c);
 	if(sch->level != Qcmpt)
--- a/sys/src/9/port/mkdevc
+++ b/sys/src/9/port/mkdevc
@@ -78,6 +78,9 @@
 	if(ARGC < 2)
 		exit "usage"

+	if(ndev >= 256)
+		exit "device count will overflow Pgrp.notallowed"
+
 	printf "#include \"u.h\"\n";
 	printf "#include \"../port/lib.h\"\n";
 	printf "#include \"mem.h\"\n";
--- a/sys/src/9/port/portdat.h
+++ b/sys/src/9/port/portdat.h
@@ -121,6 +121,7 @@
 	Amount,				/* to be mounted or mounted upon */
 	Acreate,			/* is to be created */
 	Aremove,			/* will be removed by caller */
+	Aunmount,			/* unmount arg[0] */

 	COPEN	= 0x0001,		/* for i/o */
 	CMSG	= 0x0002,		/* the message channel for a mount */
@@ -484,7 +485,7 @@
 {
 	Ref;
 	RWlock	ns;			/* Namespace n read/one write lock */
-	int	noattach;
+	u64int	notallowed[4];		/* Room for 256 devices */
 	Mhead	*mnthash[MNTHASH];
 };

--- a/sys/src/9/port/portfns.h
+++ b/sys/src/9/port/portfns.h
@@ -413,6 +413,9 @@
 ushort		nhgets(void*);
 ulong		µs(void);
 long		lcycles(void);
+void		driverscmd(Pgrp*,int,int,char**);
+int		driversallowed(Pgrp*, int);
+int		canmount(Pgrp*);

 #pragma varargck argpos iprint	1
 #pragma	varargck argpos	panic	1
--- a/sys/src/9/port/sysfile.c
+++ b/sys/src/9/port/sysfile.c
@@ -1048,7 +1048,7 @@
 			nexterror();
 		}

-		if(up->pgrp->noattach)
+		if(!canmount(up->pgrp))
 			error(Enoattach);

 		ac = nil;
@@ -1160,14 +1160,8 @@
 		nexterror();
 	}
 	if(name != nil) {
-		/*
-		 * This has to be namec(..., Aopen, ...) because
-		 * if arg[0] is something like /srv/cs or /fd/0,
-		 * opening it is the only way to get at the real
-		 * Chan underneath.
-		 */
 		validaddr((uintptr)name, 1, 0);
-		cmounted = namec(name, Aopen, OREAD, 0);
+		cmounted = namec(name, Aunmount, OREAD, 0);
 	}
 	cunmount(cmount, cmounted);
 	poperror();
--- a/sys/src/9/port/sysproc.c
+++ b/sys/src/9/port/sysproc.c
@@ -34,6 +34,7 @@
 	Egrp *oeg;
 	ulong pid, flag;
 	Mach *wm;
+	char *devs;

 	flag = va_arg(list, ulong);
 	/* Check flags before we commit */
@@ -44,6 +45,11 @@
 	if((flag & (RFENVG|RFCENVG)) == (RFENVG|RFCENVG))
 		error(Ebadarg);

+	/*
+	 * Code using RFNOMNT expects to block all but
+	 * the following devices.
+	 */
+	devs = "|decp";
 	if((flag&RFPROC) == 0) {
 		if(flag & (RFMEM|RFNOWAIT))
 			error(Ebadarg);
@@ -60,12 +66,12 @@
 			up->pgrp = newpgrp();
 			if(flag & RFNAMEG)
 				pgrpcpy(up->pgrp, opg);
-			/* inherit noattach */
-			up->pgrp->noattach = opg->noattach;
+			/* inherit notallowed */
+			memmove(up->pgrp->notallowed, opg->notallowed, sizeof up->pgrp->notallowed);
 			closepgrp(opg);
 		}
 		if(flag & RFNOMNT)
-			up->pgrp->noattach = 1;
+			driverscmd(up->pgrp, 1, 1, &devs);
 		if(flag & RFREND) {
 			org = up->rgrp;
 			up->rgrp = newrgrp();
@@ -177,8 +183,8 @@
 		p->pgrp = newpgrp();
 		if(flag & RFNAMEG)
 			pgrpcpy(p->pgrp, up->pgrp);
-		/* inherit noattach */
-		p->pgrp->noattach = up->pgrp->noattach;
+		/* inherit notallowed */
+		memmove(p->pgrp->notallowed, up->pgrp->notallowed, sizeof p->pgrp->notallowed);
 	}
 	else {
 		p->pgrp = up->pgrp;
@@ -185,7 +191,7 @@
 		incref(p->pgrp);
 	}
 	if(flag & RFNOMNT)
-		p->pgrp->noattach = 1;
+		driverscmd(p->pgrp, 1, 1, &devs);

 	if(flag & RFREND)
 		p->rgrp = newrgrp();
--- a/sys/src/libauth/newns.c
+++ b/sys/src/libauth/newns.c
@@ -14,8 +14,8 @@
 static int	setenv(char*, char*);
 static char	*expandarg(char*, char*);
 static int	splitargs(char*, char*[], char*, int);
-static int	nsfile(char*, Biobuf *, AuthRpc *);
-static int	nsop(char*, int, char*[], AuthRpc*);
+static int	nsfile(char*, Biobuf *, AuthRpc *, int);
+static int	nsop(char*, int, char*[], AuthRpc*, int);
 static int	catch(void*, char*);

 int newnsdebug;
@@ -35,7 +35,7 @@
 {
 	Biobuf *b;
 	char home[4*ANAMELEN];
-	int afd, cdroot;
+	int afd, cdroot, dfd;
 	char *path;
 	AuthRpc *rpc;

@@ -51,6 +51,10 @@
 	}
 	/* rpc != nil iff afd >= 0 */

+	dfd = open("#c/drivers", OWRITE|OCEXEC);
+	if(dfd < 0 && newnsdebug)
+		fprint(2, "open #c/drivers: %r\n");
+
 	if(file == nil){
 		if(!newns){
 			werrstr("no namespace file specified");
@@ -70,7 +74,8 @@
 		setenv("home", home);
 	}

-	cdroot = nsfile(newns ? "newns" : "addns", b, rpc);
+	cdroot = nsfile(newns ? "newns" : "addns", b, rpc, dfd);
+	close(dfd);
 	Bterm(b);
 	freecloserpc(rpc);

@@ -87,7 +92,7 @@
 }

 static int
-nsfile(char *fn, Biobuf *b, AuthRpc *rpc)
+nsfile(char *fn, Biobuf *b, AuthRpc *rpc, int dfd)
 {
 	int argc;
 	char *cmd, *argv[NARG+1], argbuf[MAXARG*NARG];
@@ -103,7 +108,7 @@
 			continue;
 		argc = splitargs(cmd, argv, argbuf, NARG);
 		if(argc)
-			cdroot |= nsop(fn, argc, argv, rpc);
+			cdroot |= nsop(fn, argc, argv, rpc, dfd);
 	}
 	atnotify(catch, 0);
 	return cdroot;
@@ -143,7 +148,7 @@
 }

 static int
-nsop(char *fn, int argc, char *argv[], AuthRpc *rpc)
+nsop(char *fn, int argc, char *argv[], AuthRpc *rpc, int dfd)
 {
 	char *argv0;
 	ulong flags;
@@ -181,7 +186,7 @@
 		b = Bopen(argv[0], OREAD|OCEXEC);
 		if(b == nil)
 			return 0;
-		cdroot |= nsfile(fn, b, rpc);
+		cdroot |= nsfile(fn, b, rpc, dfd);
 		Bterm(b);
 	}else if(strcmp(argv0, "clear") == 0 && argc == 0){
 		rfork(RFCNAMEG);
@@ -212,6 +217,14 @@
 	}else if(strcmp(argv0, "cd") == 0 && argc == 1){
 		if(chdir(argv[0]) == 0 && *argv[0] == '/')
 			cdroot = 1;
+	}else if(argc >= 1 && (strcmp(argv0, "permit") == 0 || strcmp(argv0, "block") == 0)){
+		//We should not silently fail if we can not honor a permit/block
+		//due to the parent namespace missing #c/drivers.
+		if(dfd <= 0)
+			sysfatal("%s requested, but could not open #c/drivers", argv0);
+		for(i=0; i < argc; i++)
+			if(fprint(dfd, "%s %s\n", argv0, argv[i]) < 0 && newnsdebug)
+				fprint(2, "%s: %s %s %r\n", fn, argv0, argv[i]);
 	}
 	return cdroot;
 }


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-08  2:55           ` Jacob Moody
@ 2022-05-11 14:47             ` Jacob Moody
  2022-05-11 16:11               ` Stanley Lieber
  2022-05-12  3:18             ` ori
  1 sibling, 1 reply; 28+ messages in thread
From: Jacob Moody @ 2022-05-11 14:47 UTC (permalink / raw)
  To: 9front

Hello,

Another minor iteration. This tweaks the rc-httpd sandboxing
and documents it better in the man page.

thanks,
moody

diff 9126ee3eea90d639f4e877c01400248581d10f65 uncommitted
--- a/lib/namespace
+++ b/lib/namespace
@@ -1,5 +1,6 @@
 # root
 mount -aC #s/boot /root $rootspec
+bind $rootdir'/rc' /rc
 bind -a $rootdir /

 # kernel devices
--- a/lib/namespace.ftp
+++ b/lib/namespace.ftp
@@ -8,5 +8,6 @@
 # bind a personal incoming directory below incoming
 bind -c /usr/none/incoming /usr/web/incoming/none

+permit |MedIa/
 # this cuts off everything not mounted below /usr/web
 bind /usr/web /
--- /tmp/diff100001081045
+++ b/lib/namespace.rc-httpd
@@ -1,0 +1,19 @@
+mount -aC #s/boot /root $rootspec
+
+# kernel devices
+bind #c /dev
+bind #d /fd
+bind -c #e /env
+bind #p /proc
+
+bind /root/$cputype/bin /bin
+bind /root/rc /rc
+bind -a /rc/bin /bin
+
+permit Mcde|ps/
+
+. /root/cfg/$sysname/namespace.rc-httpd
+
+unmount /root
+block Mcs
+unmount #c /dev
--- a/rc/bin/service/!tcp80
+++ b/rc/bin/service/!tcp80
@@ -1,2 +1,2 @@
 #!/bin/rc
-exec /rc/bin/rc-httpd/rc-httpd >>[2]/sys/log/www
+exec auth/newns -n /lib/namespace.rc-httpd /rc/bin/rc-httpd/rc-httpd >>[2]/sys/log/www
--- a/sys/man/3/cons
+++ b/sys/man/3/cons
@@ -90,10 +90,30 @@
 .PP
 The
 .B drivers
-file contains, one per line, a listing of the drivers configured in the kernel, in the format
+file contains, one per line, a listing of available kernel drivers, in the format
 .IP
 .EX
 #c cons
+.EE
+.PP
+A process can forfeit access to a driver, for itself and it's future children, through a write to
+.B drivers.
+A message is one of:
+.IP "block \f2drivers\fP"
+block access to the listed
+.I drivers.
+.IP "permit \f2drivers\fP"
+permit access to just the provided
+.I drivers.
+.PP
+Drivers are identified by their short hand, the first column returned on reads,
+without the leading sharp. The following blocks access to
+.IR env (3)
+and
+.IR sd (3):
+.IP
+.EX
+block se
 .EE
 .PP
 The
--- a/sys/man/6/namespace
+++ b/sys/man/6/namespace
@@ -59,6 +59,15 @@
 .I new
 is missing.
 .TP
+.BI block \ drivers
+Removes access to the listed kernel
+.I drivers.
+Devices are identified by their sharp name.
+.TP
+.BI permit \ drivers
+Permit access to only the listed kernel
+.I drivers.
+.TP
 .BR clear
 Clear the name space with
 .BR rfork(RFCNAMEG) .
@@ -80,4 +89,5 @@
 .SH "SEE ALSO"
 .IR bind (1),
 .IR namespace (4),
-.IR init (8)
+.IR init (8),
+.IR cons (3)
--- a/sys/man/8/rc-httpd
+++ b/sys/man/8/rc-httpd
@@ -85,6 +85,36 @@
 .I REMOTE_USER
 variable provides a user identification string supplied by the
 client as part of user authentication.
+.SH STARTUP
+.I Rc-httpd
+is run from a file in the directory scanned by
+.IR listen (8),
+or called as an argument to aux/listen1.
+The program's standard error may be captured to a log file:
+.RS
+.EX
+exec /rc/bin/rc-httpd/rc-httpd >>[2]/sys/log/www
+.EE
+.RE
+.PP
+It is recommended to provide a strict namespace for
+.I rc-httpd.
+.B /lib/namespace.rc-httpd
+provides an example of such a namespace, and serves as
+a default.
+.PP
+When using the default,
+.B /cfg/$sysname/namespace.rc-httpd
+is sourced to allow for binding of files to a place
+.B select-handler
+may expect to find them.
+.PP
+.I Rc-httpd
+can be run using
+.IR auth/newns (8)
+to switch to the restrictive namespace before starting.
+.B /rc/bin/service/!tcp80
+provides a sample invocation.
 .SH EXAMPLES
 The following examples demonstrate possible ways to configure
 .BR select-handler.
@@ -153,15 +183,19 @@
 }
 .EE
 .RE
-.SH STARTUP
-.I Rc-httpd
-is run from a file in the directory scanned by
-.IR listen (8),
-or called as an argument to aux/listen1.
-The program's standard error may be captured to a log file:
+.PP
+An example
+.B /cfg/$sysname/namespace.rc-httpd:
 .RS
 .EX
-exec /rc/bin/rc-httpd/rc-httpd >>[2]/sys/log/www
+# #s/boot is present on /root.
+# /root is unmounted after we finish.
+
+# srv is unused, we can use it as our base FS_ROOT.
+bind /root/usr/moody/www /srv
+
+# use a specific select-handler.
+bind -b /root/usr/moody/lib/rc-httpd /rc/bin/rc-httpd
 .EE
 .RE
 .SH FILES
@@ -190,6 +224,10 @@
 .B /rc/bin/service/tcp80
 .TP
 .B /sys/log/www
+.TP
+.B /lib/namespace.rc-httpd
+.TP
+.B /cfg/$sysname/namespace.rc-httpd
 .SH SOURCE
 .B /rc/bin/rc-httpd
 .SH "SEE ALSO"
--- a/sys/src/9/boot/boot.c
+++ b/sys/src/9/boot/boot.c
@@ -19,6 +19,7 @@
 	if(await(buf, sizeof(buf)) < 0)
 		goto Err;

+	bind("/root/rc", "/rc", MREPL);
 	bind(root, "/", MAFTER);

 	buf[0] = '/';
--- a/sys/src/9/port/chan.c
+++ b/sys/src/9/port/chan.c
@@ -1272,7 +1272,7 @@
 Chan*
 namec(char *aname, int amode, int omode, ulong perm)
 {
-	int len, n, t, nomount;
+	int len, n, t, nomount, devunmount;
 	Chan *c;
 	Chan *volatile cnew;
 	Path *volatile path;
@@ -1292,6 +1292,24 @@
 	name = aname;

 	/*
+	 * When unmounting, the name parameter must be accessed
+	 * using Aopen in order to get the real chan from
+	 * something like /srv/cs or /fd/0. However when sandboxing,
+	 * unmounting a sharp from a union is a valid operation even
+	 * if the device is blocked.
+	 */
+	if(amode == Aunmount){
+		/*
+		 * Doing any walks down the device could leak information
+		 * about the existence of files.
+		 */
+		if(name[0] == '#' && utflen(name) == 2)
+			devunmount = 1;
+		amode = Aopen;
+	} else
+		devunmount = 0;
+
+	/*
 	 * Find the starting off point (the current slash, the root of
 	 * a device tree, or the current dot) as well as the name to
 	 * evaluate starting there.
@@ -1313,24 +1331,13 @@
 			up->genbuf[n++] = *name++;
 		}
 		up->genbuf[n] = '\0';
-		/*
-		 *  noattach is sandboxing.
-		 *
-		 *  the OK exceptions are:
-		 *	|  it only gives access to pipes you create
-		 *	d  this process's file descriptors
-		 *	e  this process's environment
-		 *  the iffy exceptions are:
-		 *	c  time and pid, but also cons and consctl
-		 *	p  control of your own processes (and unfortunately
-		 *	   any others left unprotected)
-		 */
 		n = chartorune(&r, up->genbuf+1)+1;
-		if(up->pgrp->noattach && utfrune("|decp", r)==nil)
-			error(Enoattach);
 		t = devno(r, 1);
 		if(t == -1)
 			error(Ebadsharp);
+		if(!devunmount && !driversallowed(up->pgrp, r))
+			error(Enoattach);
+
 		c = devtab[t]->attach(up->genbuf+n);
 		break;

--- a/sys/src/9/port/devcons.c
+++ b/sys/src/9/port/devcons.c
@@ -39,6 +39,18 @@
 	CMrdb,		"rdb",		0,
 };

+enum
+{
+	CMblock,
+	CMpermit,
+};
+
+Cmdtab drivermsg[] =
+{
+	CMblock,	"block",	0,
+	CMpermit,	"permit",	0,
+};
+
 void
 printinit(void)
 {
@@ -332,7 +344,7 @@
 	"cons",		{Qcons},	0,		0660,
 	"consctl",	{Qconsctl},	0,		0220,
 	"cputime",	{Qcputime},	6*NUMSIZE,	0444,
-	"drivers",	{Qdrivers},	0,		0444,
+	"drivers",	{Qdrivers},	0,		0666,
 	"hostdomain",	{Qhostdomain},	DOMLEN,		0664,
 	"hostowner",	{Qhostowner},	0,		0664,
 	"kmesg",	{Qkmesg},	0,		0440,
@@ -583,9 +595,15 @@
 	case Qdrivers:
 		b = smalloc(READSTR);
 		k = 0;
-		for(i = 0; devtab[i] != nil; i++)
+		
+		rlock(&up->pgrp->ns);
+		for(i = 0; devtab[i] != nil; i++){
+			if(up->pgrp->notallowed[i/(sizeof(u64int)*8)] & 1<<i%(sizeof(u64int)*8))
+				continue;
 			k += snprint(b+k, READSTR-k, "#%C %s\n",
 				devtab[i]->dc, devtab[i]->name);
+		}
+		runlock(&up->pgrp->ns);
 		if(waserror()){
 			free(b);
 			nexterror();
@@ -676,6 +694,26 @@
 		error(Eperm);
 		break;

+	case Qdrivers:
+		cb = parsecmd(a, n);
+
+		if(waserror()) {
+			free(cb);
+			nexterror();
+		}
+		ct = lookupcmd(cb, drivermsg, nelem(drivermsg));
+		switch(ct->index) {
+		case CMblock:
+			driverscmd(up->pgrp, 0, cb->nf-1, cb->f+1);
+			break;
+		case CMpermit:
+			driverscmd(up->pgrp, 1, cb->nf-1, cb->f+1);
+			break;
+		}
+		poperror();
+		free(cb);
+		break;
+
 	case Qreboot:
 		if(!iseve())
 			error(Eperm);
@@ -935,6 +973,64 @@
 		break;
 	}
 	return n+1;
+}
+
+void
+driverscmd(Pgrp *pgrp, int invert, int argc, char *argv[])
+{
+	int i, t, w;
+	char *p;
+	Rune r;
+	u64int mask[nelem(pgrp->notallowed)];
+
+	if(invert)
+		memset(mask, 0xFF, sizeof mask);
+	else
+		memset(mask, 0, sizeof mask);
+
+	w = sizeof mask[0] * 8;
+	for(i=0; i < argc; i++)
+		for(p = argv[i]; *p != '\0';){
+			p += chartorune(&r, p);
+			t = devno(r, 1);
+			if(t == -1)
+				continue;
+			if(invert)
+				mask[t/w] &= ~(1<<t%w);
+			else
+				mask[t/w] |= 1<<t%w;
+		}
+
+	wlock(&pgrp->ns);
+	for(i=0; i < nelem(pgrp->notallowed); i++)
+		pgrp->notallowed[i] |= mask[i];
+	wunlock(&pgrp->ns);
+}
+
+int
+driversallowed(Pgrp *pgrp, int r)
+{
+	int t, w, b;
+
+	t = devno(r, 1);
+	if(t == -1)
+		return 0;
+
+	w = sizeof(u64int) * 8;
+	rlock(&pgrp->ns);
+	b = !(pgrp->notallowed[t/w] & 1<<t%w);
+	runlock(&pgrp->ns);
+	return b;
+}
+
+int
+canmount(Pgrp *pgrp)
+{
+	/*
+	 * Devmnt is not usable directly from user procs, so
+	 * having it removed is interpreted to block any mounts.
+	 */
+	return driversallowed(pgrp, 'M');
 }

 void
--- a/sys/src/9/port/devroot.c
+++ b/sys/src/9/port/devroot.c
@@ -105,6 +105,7 @@
 	addrootdir("net");
 	addrootdir("net.alt");
 	addrootdir("proc");
+	addrootdir("rc");
 	addrootdir("root");
 	addrootdir("srv");
 	addrootdir("shr");
--- a/sys/src/9/port/devshr.c
+++ b/sys/src/9/port/devshr.c
@@ -464,7 +464,7 @@
 		cclose(c);
 		return nc;	
 	case Qcroot:
-		if(up->pgrp->noattach)
+		if(!canmount(up->pgrp))
 			error(Enoattach);
 		if((perm & DMDIR) == 0 || mode != OREAD)
 			error(Eperm);
@@ -498,7 +498,7 @@
 		sch->shr = shr;
 		break;
 	case Qcshr:
-		if(up->pgrp->noattach)
+		if(!canmount(up->pgrp))
 			error(Enoattach);
 		if((perm & DMDIR) != 0 || mode != OWRITE)
 			error(Eperm);
@@ -731,7 +731,7 @@
 	Mhead *h;
 	Mount *m;

-	if(up->pgrp->noattach)
+	if(!canmount(up->pgrp))
 		error(Enoattach);
 	sch = tosch(c);
 	if(sch->level != Qcmpt)
--- a/sys/src/9/port/mkdevc
+++ b/sys/src/9/port/mkdevc
@@ -78,6 +78,9 @@
 	if(ARGC < 2)
 		exit "usage"

+	if(ndev >= 256)
+		exit "device count will overflow Pgrp.notallowed"
+
 	printf "#include \"u.h\"\n";
 	printf "#include \"../port/lib.h\"\n";
 	printf "#include \"mem.h\"\n";
--- a/sys/src/9/port/portdat.h
+++ b/sys/src/9/port/portdat.h
@@ -121,6 +121,7 @@
 	Amount,				/* to be mounted or mounted upon */
 	Acreate,			/* is to be created */
 	Aremove,			/* will be removed by caller */
+	Aunmount,			/* unmount arg[0] */

 	COPEN	= 0x0001,		/* for i/o */
 	CMSG	= 0x0002,		/* the message channel for a mount */
@@ -484,7 +485,7 @@
 {
 	Ref;
 	RWlock	ns;			/* Namespace n read/one write lock */
-	int	noattach;
+	u64int	notallowed[4];		/* Room for 256 devices */
 	Mhead	*mnthash[MNTHASH];
 };

--- a/sys/src/9/port/portfns.h
+++ b/sys/src/9/port/portfns.h
@@ -413,6 +413,9 @@
 ushort		nhgets(void*);
 ulong		µs(void);
 long		lcycles(void);
+void		driverscmd(Pgrp*,int,int,char**);
+int		driversallowed(Pgrp*, int);
+int		canmount(Pgrp*);

 #pragma varargck argpos iprint	1
 #pragma	varargck argpos	panic	1
--- a/sys/src/9/port/sysfile.c
+++ b/sys/src/9/port/sysfile.c
@@ -1048,7 +1048,7 @@
 			nexterror();
 		}

-		if(up->pgrp->noattach)
+		if(!canmount(up->pgrp))
 			error(Enoattach);

 		ac = nil;
@@ -1160,14 +1160,8 @@
 		nexterror();
 	}
 	if(name != nil) {
-		/*
-		 * This has to be namec(..., Aopen, ...) because
-		 * if arg[0] is something like /srv/cs or /fd/0,
-		 * opening it is the only way to get at the real
-		 * Chan underneath.
-		 */
 		validaddr((uintptr)name, 1, 0);
-		cmounted = namec(name, Aopen, OREAD, 0);
+		cmounted = namec(name, Aunmount, OREAD, 0);
 	}
 	cunmount(cmount, cmounted);
 	poperror();
--- a/sys/src/9/port/sysproc.c
+++ b/sys/src/9/port/sysproc.c
@@ -34,6 +34,7 @@
 	Egrp *oeg;
 	ulong pid, flag;
 	Mach *wm;
+	char *devs;

 	flag = va_arg(list, ulong);
 	/* Check flags before we commit */
@@ -44,6 +45,11 @@
 	if((flag & (RFENVG|RFCENVG)) == (RFENVG|RFCENVG))
 		error(Ebadarg);

+	/*
+	 * Code using RFNOMNT expects to block all but
+	 * the following devices.
+	 */
+	devs = "|decp";
 	if((flag&RFPROC) == 0) {
 		if(flag & (RFMEM|RFNOWAIT))
 			error(Ebadarg);
@@ -60,12 +66,12 @@
 			up->pgrp = newpgrp();
 			if(flag & RFNAMEG)
 				pgrpcpy(up->pgrp, opg);
-			/* inherit noattach */
-			up->pgrp->noattach = opg->noattach;
+			/* inherit notallowed */
+			memmove(up->pgrp->notallowed, opg->notallowed, sizeof up->pgrp->notallowed);
 			closepgrp(opg);
 		}
 		if(flag & RFNOMNT)
-			up->pgrp->noattach = 1;
+			driverscmd(up->pgrp, 1, 1, &devs);
 		if(flag & RFREND) {
 			org = up->rgrp;
 			up->rgrp = newrgrp();
@@ -177,8 +183,8 @@
 		p->pgrp = newpgrp();
 		if(flag & RFNAMEG)
 			pgrpcpy(p->pgrp, up->pgrp);
-		/* inherit noattach */
-		p->pgrp->noattach = up->pgrp->noattach;
+		/* inherit notallowed */
+		memmove(p->pgrp->notallowed, up->pgrp->notallowed, sizeof p->pgrp->notallowed);
 	}
 	else {
 		p->pgrp = up->pgrp;
@@ -185,7 +191,7 @@
 		incref(p->pgrp);
 	}
 	if(flag & RFNOMNT)
-		p->pgrp->noattach = 1;
+		driverscmd(p->pgrp, 1, 1, &devs);

 	if(flag & RFREND)
 		p->rgrp = newrgrp();
--- a/sys/src/libauth/newns.c
+++ b/sys/src/libauth/newns.c
@@ -14,8 +14,8 @@
 static int	setenv(char*, char*);
 static char	*expandarg(char*, char*);
 static int	splitargs(char*, char*[], char*, int);
-static int	nsfile(char*, Biobuf *, AuthRpc *);
-static int	nsop(char*, int, char*[], AuthRpc*);
+static int	nsfile(char*, Biobuf *, AuthRpc *, int);
+static int	nsop(char*, int, char*[], AuthRpc*, int);
 static int	catch(void*, char*);

 int newnsdebug;
@@ -35,7 +35,7 @@
 {
 	Biobuf *b;
 	char home[4*ANAMELEN];
-	int afd, cdroot;
+	int afd, cdroot, dfd;
 	char *path;
 	AuthRpc *rpc;

@@ -51,6 +51,10 @@
 	}
 	/* rpc != nil iff afd >= 0 */

+	dfd = open("#c/drivers", OWRITE|OCEXEC);
+	if(dfd < 0 && newnsdebug)
+		fprint(2, "open #c/drivers: %r\n");
+
 	if(file == nil){
 		if(!newns){
 			werrstr("no namespace file specified");
@@ -70,7 +74,8 @@
 		setenv("home", home);
 	}

-	cdroot = nsfile(newns ? "newns" : "addns", b, rpc);
+	cdroot = nsfile(newns ? "newns" : "addns", b, rpc, dfd);
+	close(dfd);
 	Bterm(b);
 	freecloserpc(rpc);

@@ -87,7 +92,7 @@
 }

 static int
-nsfile(char *fn, Biobuf *b, AuthRpc *rpc)
+nsfile(char *fn, Biobuf *b, AuthRpc *rpc, int dfd)
 {
 	int argc;
 	char *cmd, *argv[NARG+1], argbuf[MAXARG*NARG];
@@ -103,7 +108,7 @@
 			continue;
 		argc = splitargs(cmd, argv, argbuf, NARG);
 		if(argc)
-			cdroot |= nsop(fn, argc, argv, rpc);
+			cdroot |= nsop(fn, argc, argv, rpc, dfd);
 	}
 	atnotify(catch, 0);
 	return cdroot;
@@ -143,7 +148,7 @@
 }

 static int
-nsop(char *fn, int argc, char *argv[], AuthRpc *rpc)
+nsop(char *fn, int argc, char *argv[], AuthRpc *rpc, int dfd)
 {
 	char *argv0;
 	ulong flags;
@@ -181,7 +186,7 @@
 		b = Bopen(argv[0], OREAD|OCEXEC);
 		if(b == nil)
 			return 0;
-		cdroot |= nsfile(fn, b, rpc);
+		cdroot |= nsfile(fn, b, rpc, dfd);
 		Bterm(b);
 	}else if(strcmp(argv0, "clear") == 0 && argc == 0){
 		rfork(RFCNAMEG);
@@ -212,6 +217,14 @@
 	}else if(strcmp(argv0, "cd") == 0 && argc == 1){
 		if(chdir(argv[0]) == 0 && *argv[0] == '/')
 			cdroot = 1;
+	}else if(argc >= 1 && (strcmp(argv0, "permit") == 0 || strcmp(argv0, "block") == 0)){
+		//We should not silently fail if we can not honor a permit/block
+		//due to the parent namespace missing #c/drivers.
+		if(dfd <= 0)
+			sysfatal("%s requested, but could not open #c/drivers", argv0);
+		for(i=0; i < argc; i++)
+			if(fprint(dfd, "%s %s\n", argv0, argv[i]) < 0 && newnsdebug)
+				fprint(2, "%s: %s %s %r\n", fn, argv0, argv[i]);
 	}
 	return cdroot;
 }

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-11 14:47             ` Jacob Moody
@ 2022-05-11 16:11               ` Stanley Lieber
  2022-05-12  4:29                 ` Jacob Moody
  0 siblings, 1 reply; 28+ messages in thread
From: Stanley Lieber @ 2022-05-11 16:11 UTC (permalink / raw)
  To: 9front

in some cases, would it make more sense to invoke auth/newns at the listener level, rather than every time rc-httpd is called?

al

> On May 11, 2022, at 10:49 AM, Jacob Moody <moody@mail.posixcafe.org> wrote:
> 
> Hello,
> 
> Another minor iteration. This tweaks the rc-httpd sandboxing
> and documents it better in the man page.
> 
> thanks,
> moody
> 
> diff 9126ee3eea90d639f4e877c01400248581d10f65 uncommitted
> --- a/lib/namespace
> +++ b/lib/namespace
> @@ -1,5 +1,6 @@
> # root
> mount -aC #s/boot /root $rootspec
> +bind $rootdir'/rc' /rc
> bind -a $rootdir /
> 
> # kernel devices
> --- a/lib/namespace.ftp
> +++ b/lib/namespace.ftp
> @@ -8,5 +8,6 @@
> # bind a personal incoming directory below incoming
> bind -c /usr/none/incoming /usr/web/incoming/none
> 
> +permit |MedIa/
> # this cuts off everything not mounted below /usr/web
> bind /usr/web /
> --- /tmp/diff100001081045
> +++ b/lib/namespace.rc-httpd
> @@ -1,0 +1,19 @@
> +mount -aC #s/boot /root $rootspec
> +
> +# kernel devices
> +bind #c /dev
> +bind #d /fd
> +bind -c #e /env
> +bind #p /proc
> +
> +bind /root/$cputype/bin /bin
> +bind /root/rc /rc
> +bind -a /rc/bin /bin
> +
> +permit Mcde|ps/
> +
> +. /root/cfg/$sysname/namespace.rc-httpd
> +
> +unmount /root
> +block Mcs
> +unmount #c /dev
> --- a/rc/bin/service/!tcp80
> +++ b/rc/bin/service/!tcp80
> @@ -1,2 +1,2 @@
> #!/bin/rc
> -exec /rc/bin/rc-httpd/rc-httpd >>[2]/sys/log/www
> +exec auth/newns -n /lib/namespace.rc-httpd /rc/bin/rc-httpd/rc-httpd >>[2]/sys/log/www
> --- a/sys/man/3/cons
> +++ b/sys/man/3/cons
> @@ -90,10 +90,30 @@
> .PP
> The
> .B drivers
> -file contains, one per line, a listing of the drivers configured in the kernel, in the format
> +file contains, one per line, a listing of available kernel drivers, in the format
> .IP
> .EX
> #c cons
> +.EE
> +.PP
> +A process can forfeit access to a driver, for itself and it's future children, through a write to
> +.B drivers.
> +A message is one of:
> +.IP "block \f2drivers\fP"
> +block access to the listed
> +.I drivers.
> +.IP "permit \f2drivers\fP"
> +permit access to just the provided
> +.I drivers.
> +.PP
> +Drivers are identified by their short hand, the first column returned on reads,
> +without the leading sharp. The following blocks access to
> +.IR env (3)
> +and
> +.IR sd (3):
> +.IP
> +.EX
> +block se
> .EE
> .PP
> The
> --- a/sys/man/6/namespace
> +++ b/sys/man/6/namespace
> @@ -59,6 +59,15 @@
> .I new
> is missing.
> .TP
> +.BI block \ drivers
> +Removes access to the listed kernel
> +.I drivers.
> +Devices are identified by their sharp name.
> +.TP
> +.BI permit \ drivers
> +Permit access to only the listed kernel
> +.I drivers.
> +.TP
> .BR clear
> Clear the name space with
> .BR rfork(RFCNAMEG) .
> @@ -80,4 +89,5 @@
> .SH "SEE ALSO"
> .IR bind (1),
> .IR namespace (4),
> -.IR init (8)
> +.IR init (8),
> +.IR cons (3)
> --- a/sys/man/8/rc-httpd
> +++ b/sys/man/8/rc-httpd
> @@ -85,6 +85,36 @@
> .I REMOTE_USER
> variable provides a user identification string supplied by the
> client as part of user authentication.
> +.SH STARTUP
> +.I Rc-httpd
> +is run from a file in the directory scanned by
> +.IR listen (8),
> +or called as an argument to aux/listen1.
> +The program's standard error may be captured to a log file:
> +.RS
> +.EX
> +exec /rc/bin/rc-httpd/rc-httpd >>[2]/sys/log/www
> +.EE
> +.RE
> +.PP
> +It is recommended to provide a strict namespace for
> +.I rc-httpd.
> +.B /lib/namespace.rc-httpd
> +provides an example of such a namespace, and serves as
> +a default.
> +.PP
> +When using the default,
> +.B /cfg/$sysname/namespace.rc-httpd
> +is sourced to allow for binding of files to a place
> +.B select-handler
> +may expect to find them.
> +.PP
> +.I Rc-httpd
> +can be run using
> +.IR auth/newns (8)
> +to switch to the restrictive namespace before starting.
> +.B /rc/bin/service/!tcp80
> +provides a sample invocation.
> .SH EXAMPLES
> The following examples demonstrate possible ways to configure
> .BR select-handler.
> @@ -153,15 +183,19 @@
> }
> .EE
> .RE
> -.SH STARTUP
> -.I Rc-httpd
> -is run from a file in the directory scanned by
> -.IR listen (8),
> -or called as an argument to aux/listen1.
> -The program's standard error may be captured to a log file:
> +.PP
> +An example
> +.B /cfg/$sysname/namespace.rc-httpd:
> .RS
> .EX
> -exec /rc/bin/rc-httpd/rc-httpd >>[2]/sys/log/www
> +# #s/boot is present on /root.
> +# /root is unmounted after we finish.
> +
> +# srv is unused, we can use it as our base FS_ROOT.
> +bind /root/usr/moody/www /srv
> +
> +# use a specific select-handler.
> +bind -b /root/usr/moody/lib/rc-httpd /rc/bin/rc-httpd
> .EE
> .RE
> .SH FILES
> @@ -190,6 +224,10 @@
> .B /rc/bin/service/tcp80
> .TP
> .B /sys/log/www
> +.TP
> +.B /lib/namespace.rc-httpd
> +.TP
> +.B /cfg/$sysname/namespace.rc-httpd
> .SH SOURCE
> .B /rc/bin/rc-httpd
> .SH "SEE ALSO"
> --- a/sys/src/9/boot/boot.c
> +++ b/sys/src/9/boot/boot.c
> @@ -19,6 +19,7 @@
>    if(await(buf, sizeof(buf)) < 0)
>        goto Err;
> 
> +    bind("/root/rc", "/rc", MREPL);
>    bind(root, "/", MAFTER);
> 
>    buf[0] = '/';
> --- a/sys/src/9/port/chan.c
> +++ b/sys/src/9/port/chan.c
> @@ -1272,7 +1272,7 @@
> Chan*
> namec(char *aname, int amode, int omode, ulong perm)
> {
> -    int len, n, t, nomount;
> +    int len, n, t, nomount, devunmount;
>    Chan *c;
>    Chan *volatile cnew;
>    Path *volatile path;
> @@ -1292,6 +1292,24 @@
>    name = aname;
> 
>    /*
> +     * When unmounting, the name parameter must be accessed
> +     * using Aopen in order to get the real chan from
> +     * something like /srv/cs or /fd/0. However when sandboxing,
> +     * unmounting a sharp from a union is a valid operation even
> +     * if the device is blocked.
> +     */
> +    if(amode == Aunmount){
> +        /*
> +         * Doing any walks down the device could leak information
> +         * about the existence of files.
> +         */
> +        if(name[0] == '#' && utflen(name) == 2)
> +            devunmount = 1;
> +        amode = Aopen;
> +    } else
> +        devunmount = 0;
> +
> +    /*
>     * Find the starting off point (the current slash, the root of
>     * a device tree, or the current dot) as well as the name to
>     * evaluate starting there.
> @@ -1313,24 +1331,13 @@
>            up->genbuf[n++] = *name++;
>        }
>        up->genbuf[n] = '\0';
> -        /*
> -         *  noattach is sandboxing.
> -         *
> -         *  the OK exceptions are:
> -         *    |  it only gives access to pipes you create
> -         *    d  this process's file descriptors
> -         *    e  this process's environment
> -         *  the iffy exceptions are:
> -         *    c  time and pid, but also cons and consctl
> -         *    p  control of your own processes (and unfortunately
> -         *       any others left unprotected)
> -         */
>        n = chartorune(&r, up->genbuf+1)+1;
> -        if(up->pgrp->noattach && utfrune("|decp", r)==nil)
> -            error(Enoattach);
>        t = devno(r, 1);
>        if(t == -1)
>            error(Ebadsharp);
> +        if(!devunmount && !driversallowed(up->pgrp, r))
> +            error(Enoattach);
> +
>        c = devtab[t]->attach(up->genbuf+n);
>        break;
> 
> --- a/sys/src/9/port/devcons.c
> +++ b/sys/src/9/port/devcons.c
> @@ -39,6 +39,18 @@
>    CMrdb,        "rdb",        0,
> };
> 
> +enum
> +{
> +    CMblock,
> +    CMpermit,
> +};
> +
> +Cmdtab drivermsg[] =
> +{
> +    CMblock,    "block",    0,
> +    CMpermit,    "permit",    0,
> +};
> +
> void
> printinit(void)
> {
> @@ -332,7 +344,7 @@
>    "cons",        {Qcons},    0,        0660,
>    "consctl",    {Qconsctl},    0,        0220,
>    "cputime",    {Qcputime},    6*NUMSIZE,    0444,
> -    "drivers",    {Qdrivers},    0,        0444,
> +    "drivers",    {Qdrivers},    0,        0666,
>    "hostdomain",    {Qhostdomain},    DOMLEN,        0664,
>    "hostowner",    {Qhostowner},    0,        0664,
>    "kmesg",    {Qkmesg},    0,        0440,
> @@ -583,9 +595,15 @@
>    case Qdrivers:
>        b = smalloc(READSTR);
>        k = 0;
> -        for(i = 0; devtab[i] != nil; i++)
> +        
> +        rlock(&up->pgrp->ns);
> +        for(i = 0; devtab[i] != nil; i++){
> +            if(up->pgrp->notallowed[i/(sizeof(u64int)*8)] & 1<<i%(sizeof(u64int)*8))
> +                continue;
>            k += snprint(b+k, READSTR-k, "#%C %s\n",
>                devtab[i]->dc, devtab[i]->name);
> +        }
> +        runlock(&up->pgrp->ns);
>        if(waserror()){
>            free(b);
>            nexterror();
> @@ -676,6 +694,26 @@
>        error(Eperm);
>        break;
> 
> +    case Qdrivers:
> +        cb = parsecmd(a, n);
> +
> +        if(waserror()) {
> +            free(cb);
> +            nexterror();
> +        }
> +        ct = lookupcmd(cb, drivermsg, nelem(drivermsg));
> +        switch(ct->index) {
> +        case CMblock:
> +            driverscmd(up->pgrp, 0, cb->nf-1, cb->f+1);
> +            break;
> +        case CMpermit:
> +            driverscmd(up->pgrp, 1, cb->nf-1, cb->f+1);
> +            break;
> +        }
> +        poperror();
> +        free(cb);
> +        break;
> +
>    case Qreboot:
>        if(!iseve())
>            error(Eperm);
> @@ -935,6 +973,64 @@
>        break;
>    }
>    return n+1;
> +}
> +
> +void
> +driverscmd(Pgrp *pgrp, int invert, int argc, char *argv[])
> +{
> +    int i, t, w;
> +    char *p;
> +    Rune r;
> +    u64int mask[nelem(pgrp->notallowed)];
> +
> +    if(invert)
> +        memset(mask, 0xFF, sizeof mask);
> +    else
> +        memset(mask, 0, sizeof mask);
> +
> +    w = sizeof mask[0] * 8;
> +    for(i=0; i < argc; i++)
> +        for(p = argv[i]; *p != '\0';){
> +            p += chartorune(&r, p);
> +            t = devno(r, 1);
> +            if(t == -1)
> +                continue;
> +            if(invert)
> +                mask[t/w] &= ~(1<<t%w);
> +            else
> +                mask[t/w] |= 1<<t%w;
> +        }
> +
> +    wlock(&pgrp->ns);
> +    for(i=0; i < nelem(pgrp->notallowed); i++)
> +        pgrp->notallowed[i] |= mask[i];
> +    wunlock(&pgrp->ns);
> +}
> +
> +int
> +driversallowed(Pgrp *pgrp, int r)
> +{
> +    int t, w, b;
> +
> +    t = devno(r, 1);
> +    if(t == -1)
> +        return 0;
> +
> +    w = sizeof(u64int) * 8;
> +    rlock(&pgrp->ns);
> +    b = !(pgrp->notallowed[t/w] & 1<<t%w);
> +    runlock(&pgrp->ns);
> +    return b;
> +}
> +
> +int
> +canmount(Pgrp *pgrp)
> +{
> +    /*
> +     * Devmnt is not usable directly from user procs, so
> +     * having it removed is interpreted to block any mounts.
> +     */
> +    return driversallowed(pgrp, 'M');
> }
> 
> void
> --- a/sys/src/9/port/devroot.c
> +++ b/sys/src/9/port/devroot.c
> @@ -105,6 +105,7 @@
>    addrootdir("net");
>    addrootdir("net.alt");
>    addrootdir("proc");
> +    addrootdir("rc");
>    addrootdir("root");
>    addrootdir("srv");
>    addrootdir("shr");
> --- a/sys/src/9/port/devshr.c
> +++ b/sys/src/9/port/devshr.c
> @@ -464,7 +464,7 @@
>        cclose(c);
>        return nc;    
>    case Qcroot:
> -        if(up->pgrp->noattach)
> +        if(!canmount(up->pgrp))
>            error(Enoattach);
>        if((perm & DMDIR) == 0 || mode != OREAD)
>            error(Eperm);
> @@ -498,7 +498,7 @@
>        sch->shr = shr;
>        break;
>    case Qcshr:
> -        if(up->pgrp->noattach)
> +        if(!canmount(up->pgrp))
>            error(Enoattach);
>        if((perm & DMDIR) != 0 || mode != OWRITE)
>            error(Eperm);
> @@ -731,7 +731,7 @@
>    Mhead *h;
>    Mount *m;
> 
> -    if(up->pgrp->noattach)
> +    if(!canmount(up->pgrp))
>        error(Enoattach);
>    sch = tosch(c);
>    if(sch->level != Qcmpt)
> --- a/sys/src/9/port/mkdevc
> +++ b/sys/src/9/port/mkdevc
> @@ -78,6 +78,9 @@
>    if(ARGC < 2)
>        exit "usage"
> 
> +    if(ndev >= 256)
> +        exit "device count will overflow Pgrp.notallowed"
> +
>    printf "#include \"u.h\"\n";
>    printf "#include \"../port/lib.h\"\n";
>    printf "#include \"mem.h\"\n";
> --- a/sys/src/9/port/portdat.h
> +++ b/sys/src/9/port/portdat.h
> @@ -121,6 +121,7 @@
>    Amount,                /* to be mounted or mounted upon */
>    Acreate,            /* is to be created */
>    Aremove,            /* will be removed by caller */
> +    Aunmount,            /* unmount arg[0] */
> 
>    COPEN    = 0x0001,        /* for i/o */
>    CMSG    = 0x0002,        /* the message channel for a mount */
> @@ -484,7 +485,7 @@
> {
>    Ref;
>    RWlock    ns;            /* Namespace n read/one write lock */
> -    int    noattach;
> +    u64int    notallowed[4];        /* Room for 256 devices */
>    Mhead    *mnthash[MNTHASH];
> };
> 
> --- a/sys/src/9/port/portfns.h
> +++ b/sys/src/9/port/portfns.h
> @@ -413,6 +413,9 @@
> ushort        nhgets(void*);
> ulong        µs(void);
> long        lcycles(void);
> +void        driverscmd(Pgrp*,int,int,char**);
> +int        driversallowed(Pgrp*, int);
> +int        canmount(Pgrp*);
> 
> #pragma varargck argpos iprint    1
> #pragma    varargck argpos    panic    1
> --- a/sys/src/9/port/sysfile.c
> +++ b/sys/src/9/port/sysfile.c
> @@ -1048,7 +1048,7 @@
>            nexterror();
>        }
> 
> -        if(up->pgrp->noattach)
> +        if(!canmount(up->pgrp))
>            error(Enoattach);
> 
>        ac = nil;
> @@ -1160,14 +1160,8 @@
>        nexterror();
>    }
>    if(name != nil) {
> -        /*
> -         * This has to be namec(..., Aopen, ...) because
> -         * if arg[0] is something like /srv/cs or /fd/0,
> -         * opening it is the only way to get at the real
> -         * Chan underneath.
> -         */
>        validaddr((uintptr)name, 1, 0);
> -        cmounted = namec(name, Aopen, OREAD, 0);
> +        cmounted = namec(name, Aunmount, OREAD, 0);
>    }
>    cunmount(cmount, cmounted);
>    poperror();
> --- a/sys/src/9/port/sysproc.c
> +++ b/sys/src/9/port/sysproc.c
> @@ -34,6 +34,7 @@
>    Egrp *oeg;
>    ulong pid, flag;
>    Mach *wm;
> +    char *devs;
> 
>    flag = va_arg(list, ulong);
>    /* Check flags before we commit */
> @@ -44,6 +45,11 @@
>    if((flag & (RFENVG|RFCENVG)) == (RFENVG|RFCENVG))
>        error(Ebadarg);
> 
> +    /*
> +     * Code using RFNOMNT expects to block all but
> +     * the following devices.
> +     */
> +    devs = "|decp";
>    if((flag&RFPROC) == 0) {
>        if(flag & (RFMEM|RFNOWAIT))
>            error(Ebadarg);
> @@ -60,12 +66,12 @@
>            up->pgrp = newpgrp();
>            if(flag & RFNAMEG)
>                pgrpcpy(up->pgrp, opg);
> -            /* inherit noattach */
> -            up->pgrp->noattach = opg->noattach;
> +            /* inherit notallowed */
> +            memmove(up->pgrp->notallowed, opg->notallowed, sizeof up->pgrp->notallowed);
>            closepgrp(opg);
>        }
>        if(flag & RFNOMNT)
> -            up->pgrp->noattach = 1;
> +            driverscmd(up->pgrp, 1, 1, &devs);
>        if(flag & RFREND) {
>            org = up->rgrp;
>            up->rgrp = newrgrp();
> @@ -177,8 +183,8 @@
>        p->pgrp = newpgrp();
>        if(flag & RFNAMEG)
>            pgrpcpy(p->pgrp, up->pgrp);
> -        /* inherit noattach */
> -        p->pgrp->noattach = up->pgrp->noattach;
> +        /* inherit notallowed */
> +        memmove(p->pgrp->notallowed, up->pgrp->notallowed, sizeof p->pgrp->notallowed);
>    }
>    else {
>        p->pgrp = up->pgrp;
> @@ -185,7 +191,7 @@
>        incref(p->pgrp);
>    }
>    if(flag & RFNOMNT)
> -        p->pgrp->noattach = 1;
> +        driverscmd(p->pgrp, 1, 1, &devs);
> 
>    if(flag & RFREND)
>        p->rgrp = newrgrp();
> --- a/sys/src/libauth/newns.c
> +++ b/sys/src/libauth/newns.c
> @@ -14,8 +14,8 @@
> static int    setenv(char*, char*);
> static char    *expandarg(char*, char*);
> static int    splitargs(char*, char*[], char*, int);
> -static int    nsfile(char*, Biobuf *, AuthRpc *);
> -static int    nsop(char*, int, char*[], AuthRpc*);
> +static int    nsfile(char*, Biobuf *, AuthRpc *, int);
> +static int    nsop(char*, int, char*[], AuthRpc*, int);
> static int    catch(void*, char*);
> 
> int newnsdebug;
> @@ -35,7 +35,7 @@
> {
>    Biobuf *b;
>    char home[4*ANAMELEN];
> -    int afd, cdroot;
> +    int afd, cdroot, dfd;
>    char *path;
>    AuthRpc *rpc;
> 
> @@ -51,6 +51,10 @@
>    }
>    /* rpc != nil iff afd >= 0 */
> 
> +    dfd = open("#c/drivers", OWRITE|OCEXEC);
> +    if(dfd < 0 && newnsdebug)
> +        fprint(2, "open #c/drivers: %r\n");
> +
>    if(file == nil){
>        if(!newns){
>            werrstr("no namespace file specified");
> @@ -70,7 +74,8 @@
>        setenv("home", home);
>    }
> 
> -    cdroot = nsfile(newns ? "newns" : "addns", b, rpc);
> +    cdroot = nsfile(newns ? "newns" : "addns", b, rpc, dfd);
> +    close(dfd);
>    Bterm(b);
>    freecloserpc(rpc);
> 
> @@ -87,7 +92,7 @@
> }
> 
> static int
> -nsfile(char *fn, Biobuf *b, AuthRpc *rpc)
> +nsfile(char *fn, Biobuf *b, AuthRpc *rpc, int dfd)
> {
>    int argc;
>    char *cmd, *argv[NARG+1], argbuf[MAXARG*NARG];
> @@ -103,7 +108,7 @@
>            continue;
>        argc = splitargs(cmd, argv, argbuf, NARG);
>        if(argc)
> -            cdroot |= nsop(fn, argc, argv, rpc);
> +            cdroot |= nsop(fn, argc, argv, rpc, dfd);
>    }
>    atnotify(catch, 0);
>    return cdroot;
> @@ -143,7 +148,7 @@
> }
> 
> static int
> -nsop(char *fn, int argc, char *argv[], AuthRpc *rpc)
> +nsop(char *fn, int argc, char *argv[], AuthRpc *rpc, int dfd)
> {
>    char *argv0;
>    ulong flags;
> @@ -181,7 +186,7 @@
>        b = Bopen(argv[0], OREAD|OCEXEC);
>        if(b == nil)
>            return 0;
> -        cdroot |= nsfile(fn, b, rpc);
> +        cdroot |= nsfile(fn, b, rpc, dfd);
>        Bterm(b);
>    }else if(strcmp(argv0, "clear") == 0 && argc == 0){
>        rfork(RFCNAMEG);
> @@ -212,6 +217,14 @@
>    }else if(strcmp(argv0, "cd") == 0 && argc == 1){
>        if(chdir(argv[0]) == 0 && *argv[0] == '/')
>            cdroot = 1;
> +    }else if(argc >= 1 && (strcmp(argv0, "permit") == 0 || strcmp(argv0, "block") == 0)){
> +        //We should not silently fail if we can not honor a permit/block
> +        //due to the parent namespace missing #c/drivers.
> +        if(dfd <= 0)
> +            sysfatal("%s requested, but could not open #c/drivers", argv0);
> +        for(i=0; i < argc; i++)
> +            if(fprint(dfd, "%s %s\n", argv0, argv[i]) < 0 && newnsdebug)
> +                fprint(2, "%s: %s %s %r\n", fn, argv0, argv[i]);
>    }
>    return cdroot;
> }
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-08  2:55           ` Jacob Moody
  2022-05-11 14:47             ` Jacob Moody
@ 2022-05-12  3:18             ` ori
  2022-05-12  5:10               ` Jacob Moody
  1 sibling, 1 reply; 28+ messages in thread
From: ori @ 2022-05-12  3:18 UTC (permalink / raw)
  To: 9front

Quoth Jacob Moody <moody@mail.posixcafe.org>:
> Hello,
> 
> Gave this another go: http://okturing.com/src/13592/body
> 
> This patch includes new namepace file for rc-httpd
> and changes the existing anonymous login namespace for
> ftpd, both make use of the sandboxing features.

Looking like it's getting useful :)

comments inline.
 
> thanks,
> moody
> 
> diff 9126ee3eea90d639f4e877c01400248581d10f65 uncommitted
> --- a/lib/namespace
> +++ b/lib/namespace
> @@ -1,5 +1,6 @@
>  # root
>  mount -aC #s/boot /root $rootspec
> +bind $rootdir'/rc' /rc
>  bind -a $rootdir /

Wondering why this change was needed; did a local change
leak in?

> 
>  # kernel devices
> --- a/lib/namespace.ftp
> +++ b/lib/namespace.ftp
> @@ -8,5 +8,6 @@
>  # bind a personal incoming directory below incoming
>  bind -c /usr/none/incoming /usr/web/incoming/none
> 
> +permit |MedIa/
>  # this cuts off everything not mounted below /usr/web
>  bind /usr/web /
> --- /tmp/diff100000742872
> +++ b/lib/namespace.rc-httpd
> @@ -1,0 +1,21 @@
> +mount -aC #s/boot /root $rootspec
> +
> +# kernel devices
> +bind #c /dev
> +bind #d /fd
> +bind -c #e /env
> +bind #p /proc
> +bind -a #l /net
> +bind -a #I /net

duplicated line?

> +
> +bind /root/$cputype/bin /bin
> +bind /root/rc /rc
> +bind -a /rc/bin /bin
> +
> +permit Mcde|plI/
> +
> +. /root/cfg/$sysname/namespace.rc-httpd
> +
> +unmount /root
> +block Mc
> +unmount #c /dev
> --- a/sys/man/3/cons
> +++ b/sys/man/3/cons
> @@ -90,10 +90,30 @@
>  .PP
>  The
>  .B drivers
> -file contains, one per line, a listing of the drivers configured in the kernel, in the format
> +file contains, one per line, a listing of available kernel drivers, in the format
>  .IP
>  .EX
>  #c cons
> +.EE
> +.PP
> +A process can forfeit access to a driver, for itself and it's future children, through a write to
> +.B drivers.

Is this accurate? I'd expect the way this is implement to affect all procs
in the current namespace, not just the current one and its children.

Maybe: 'a process can eject a driver from
the current namespace through a write to
drivers'.

> +A message is one of:
> +.IP "block \f2drivers\fP"
> +block access to the listed
> +.I drivers.
> +.IP "permit \f2drivers\fP"
> +permit access to just the provided
> +.I drivers.
> +.PP
> +Drivers are identified by their short hand, the first column returned on reads,
> +without the leading sharp. The following blocks access to

Clunky phrasing. Perhaps '\f2drivers\f2 is a string of driver characters'.
May also need to mention the implicication of disallowing 'M'.

> +.IR env (3)
> +and
> +.IR sd (3):
> +.IP
> +.EX
> +block se
>  .EE
>  .PP
>  The
> --- a/sys/man/6/namespace
> +++ b/sys/man/6/namespace
> @@ -59,6 +59,15 @@
>  .I new
>  is missing.
>  .TP
> +.BI block \ drivers
> +Removes access to the listed kernel
> +.I drivers.
> +Devices are identified by their sharp name.
> +.TP
> +.BI permit \ drivers
> +Permit access to only the listed kernel
> +.I drivers.
> +.TP
>  .BR clear
>  Clear the name space with
>  .BR rfork(RFCNAMEG) .
> @@ -80,4 +89,5 @@
>  .SH "SEE ALSO"
>  .IR bind (1),
>  .IR namespace (4),
> -.IR init (8)
> +.IR init (8),
> +.IR cons (3)
> --- a/sys/src/9/boot/boot.c
> +++ b /sys/src/9/boot/boot.c
> @@ -19,6 +19,7 @@
>  	if(await(buf, sizeof(buf)) < 0)
>  		goto Err;
> 
> +	bind("/root/rc", "/rc", MREPL);
>  	bind(root, "/", MAFTER);

Hm. as before, I don't understand why this bind is needed:
binding /root to / should already lead to /rc being /root/rc.

> 
>  	buf[0] = '/';
> --- a/sys/src/9/port/chan.c
> +++ b/sys/src/9/port/chan.c
> @@ -1272,7 +1272,7 @@
>  Chan*
>  namec(char *aname, int amode, int omode, ulong perm)
>  {
> -	int len, n, t, nomount;
> +	int len, n, t, nomount, devunmount;
>  	Chan *c;
>  	Chan *volatile cnew;
>  	Path *volatile path;
> @@ -1292,6 +1292,24 @@
>  	name = aname;
> 
>  	/*
> +	 * When unmounting, the name parameter must be accessed
> +	 * using Aopen in order to get the real chan from
> +	 * something like /srv/cs or /fd/0. However when sandboxing,
> +	 * unmounting a sharp from a union is a valid operation even
> +	 * if the device is blocked.
> +	 */
> +	if(amode == Aunmount){
> +		/*
> +		 * Doing any walks down the device could leak information
> +		 * about the existence of files.
> +		 */
> +		if(name[0] == '#' && utflen(name) == 2)
> +			devunmount = 1;
> +		amode = Aopen;
> +	} else
> +		devunmount = 0;
> +
> +	/*
>  	 * Find the starting off point (the current slash, the root of
>  	 * a device tree, or the current dot) as well as the name to
>  	 * evaluate starting there.
> @@ -1313,24 +1331,13 @@
>  			up->genbuf[n++] = *name++;
>  		}
>  		up->genbuf[n] = '\0';
> -		/*
> -		 *  noattach is sandboxing.
> -		 *
> -		 *  the OK exceptions are:
> -		 *	|  it only gives access to pipes you create
> -		 *	d  this process's file descriptors
> -		 *	e  this process's environment
> -		 *  the iffy exceptions are:
> -		 *	c  time and pid, but also cons and consctl
> -		 *	p  control of your own processes (and unfortunately
> -		 *	   any others left unprotected)
> -		 */
>  		n = chartorune(&r, up->genbuf+1)+1;
> -		if(up->pgrp->noattach && utfrune("|decp", r)==nil)
> -			error(Enoattach);
>  		t = devno(r, 1);
>  		if(t == -1)
>  			error(Ebadsharp);
> +		if(!devunmount && !driversallowed(up->pgrp, r))
> +			error(Enoattach);
> +
>  		c = devtab[t]->attach(up->genbuf+n);
>  		break;
> 
> --- a/sys/src/9/port/devcons.c
> +++ b/sys/src/9/port/devcons.c
> @@ -39,6 +39,18 @@
>  	CMrdb,		"rdb",		0,
>  };
> 
> +enum
> +{
> +	CMblock,
> +	CMpermit,
> +};
> +
> +Cmdtab drivermsg[] =
> +{
> +	CMblock,	"block",	0,
> +	CMpermit,	"permit",	0,
> +};
> +
>  void
>  printinit(void)
>  {
> @@ -332,7 +344,7 @@
>  	"cons",		{Qcons},	0,		0660,
>  	"consctl",	{Qconsctl},	0,		0220,
>  	"cputime",	{Qcputime},	6*NUMSIZE,	0444,
> -	"drivers",	{Qdrivers},	0,		0444,
> +	"drivers",	{Qdrivers},	0,		0666,
>  	"hostdomain",	{Qhostdomain},	DOMLEN,		0664,
>  	"hostowner",	{Qhostowner},	0,		0664,
>  	"kmesg",	{Qkmesg},	0,		0440,
> @@ -583,9 +595,15 @@
>  	case Qdrivers:
>  		b = smalloc(READSTR);
>  		k = 0;
> -		for(i = 0; devtab[i] != nil; i++)
> +		
> +		rlock(&up->pgrp->ns);
> +		for(i = 0; devtab[i] != nil; i++){
> +			if(up->pgrp->notallowed[i/(sizeof(u64int)*8)] & 1<<i%(sizeof(u64int)*8))
> +				continue;
>  			k += snprint(b+k, READSTR-k, "#%C %s\n", devtab[i]->dc, devtab[i]->name);
> +		}
> +		runlock(&up->pgrp->ns);
>  		if(waserror()){
>  			free(b);
>  			nexterror();
> @@ -676,6 +694,26 @@
>  		error(Eperm);
>  		break;
> 
> +	case Qdrivers:
> +		cb = parsecmd(a, n);
> +
> +		if(waserror()) {
> +			free(cb);
> +			nexterror();
> +		}
> +		ct = lookupcmd(cb, drivermsg, nelem(drivermsg));
> +		switch(ct->index) {
> +		case CMblock:
> +			driverscmd(up->pgrp, 0, cb->nf-1, cb->f+1);
> +			break;
> +		case CMpermit:
> +			driverscmd(up->pgrp, 1, cb->nf-1, cb->f+1);
> +			break;
> +		}
> +		poperror();
> +		free(cb);
> +		break;
> +
>  	case Qreboot:
>  		if(!iseve())
>  			error(Eperm);
> @@ -935,6 +973,64 @@
>  		break;
>  	}
>  	return n+1;
> +}
> +
> +void
> +driverscmd(Pgrp *pgrp, int invert, int argc, char *argv[])
> +{
> +	int i, t, w;
> +	char *p;
> +	Rune r;
> +	u64int mask[nelem(pgrp->notallowed)];
> +
> +	if(invert)
> +		memset(mask, 0xFF, sizeof mask);
> +	else
> +		memset(mask, 0, sizeof mask);
> +
> +	w = sizeof mask[0] * 8;
> +	for(i=0; i < argc; i++)
> +		for(p = argv[i]; *p != '\0';){
> +			p += chartorune(&r, p);
> +			t = devno(r, 1);
> +			if(t == -1)
> +				continue;
> +			if(invert)
> +				mask[t/w] &= ~(1<<t%w);
> +			else
> +				mask[t/w] |= 1<<t%w;
> +		}
> +
> +	wlock(&pgrp->ns);
> +	for(i=0; i < nelem(pgrp->notallowed); i++)
> +		pgrp->notallowed[i] |= mask[i];
> +	wunlock(&pgrp->ns);
> +}
> +
> +int
> +driversallowed(Pgrp *pgrp, int r)
> +{
> +	int t, w, b;
> +
> +	t = devno(r, 1);
> +	if(t == -1)
> +		return 0;
> +
> +	w = sizeof(u64int) * 8;
> +	rlock(&pgrp->ns);
> +	b = !(pgrp->notallowed[t/w] & 1<<t%w);
> +	runlock(&pgrp->ns);
> +	return b;
> +}
> +
> +int
> +canmount(Pgrp *pgrp)
> +{
> +	/*
> +	 * Devmnt is not usable directly from user procs, so
> +	 * having it removed is interpreted to block any mounts.
> +	 */
> +	return driversallowed(pgrp, 'M');
>  }
> 
>  void
> --- a/sys/src/9/port/devroot.c
> +++ b/sys/src/9/port/devroot.c
> @@ -105,6 +105,7 @@
>  	addrootdir("net");
>  	addrootdir("net.alt");
>  	addrootdir("proc");
> +	addrootdir("rc");
>  	addrootdir("root");
>  	addrootdir("srv");
>  	addrootdir("shr");
> --- a/sys/src/9/port/devshr.c
> +++ b/sys/src/9/port/devshr.c
> @@ -464,7 +464,7 @@
>  		cclose(c);
>  		return nc;	
>  	case Qcroot:
> -		if(up->pgrp->noattach)
> +		if(!canmount(up->pgrp))
>  			error(Enoattach);
>  		if((perm & DMDIR) == 0 || mode != OREAD)
>  			error(Eperm);
> @@ -498,7 +498,7 @@
>  		sch->shr = shr;
>  		break;
>  	case Qcshr:
> -		if(up->pgrp->noattach)
> +		if(!canmount(up->pgrp))
>  			error(Enoattach);
>  		if((perm & DMDIR) != 0 || mode != OWRITE)
>  			error(Eperm);
> @@ -731,7 +731,7 @@
>  	Mhead *h;
>  	Mount *m;
> 
> -	if(up->pgrp->noattach)
> +	if(!canmount(up->pgrp))
>  		error(Enoattach);
>  	sch = tosch(c);
>  	if(sch->level != Qcmpt)
> --- a/sys/src/9/port/mkdevc
> +++ b/sys/src/9/port/mkdevc
> @@ -78,6 +78,9 @@
>  	if(ARGC < 2)
>  		exit "usage"
> 
> +	if(ndev >= 256)
> +		exit "device count will overflow Pgrp.notallowed"
> +
>  	printf "#include \"u.h\"\n";
>  	printf "#include \"../port/lib.h\"\n";
>  	printf "#include \"mem.h\"\n";
> --- a/sys/src/9/port/portdat.h
> +++ b/sys/src/9/port/portdat.h
> @@ -121,6 +121,7 @@
>  	Amount,				/* to be mounted or mounted upon */
>  	Acreate,			/* is to be created */
>  	Aremove,			/* will be removed by caller */
> +	Aunmount,			/* unmount arg[0] */
> 
>  	COPEN	= 0x0001,		/* for i/o */
>  	CMSG	= 0x0002,		/* the message channel for a mount */
> @@ -484,7 +485,7 @@
>  {
>  	Ref;
>  	RWlock	ns;			/* Namespace n read/one write lock */
> -	int	noattach;
> +	u64int	notallowed[4];		/* Room for 256 devices */
>  	Mhead	*mnthash[MNTHASH];
>  };
> 
> --- a/sys/src/9/port/portfns.h
> +++ b/sys/src/9/port/portfns.h
> @@ -413,6 +413,9 @@
>  ushort		nhgets(void*);
>  ulong		µs(void);
>  long		lcycles(void);
> +void		driverscmd(Pgrp*,int,int,char**);
> +int		driversallowed(Pgrp*, int);
> +int		canmount(Pgrp*);
> 
>  #pragma varargck argpos iprint	1
>  #pragma	varargck argpos	panic	1
> --- a/sys/src/9/port/sysfile.c
> +++ b/sys/src/9/port/sysfile.c
> @@ -1048,7 +1048,7 @@
>  			nexterror();
>  		}
> 
> -		if(up->pgrp->noattach)
> +		if(!canmount(up->pgrp))
>  			error(Enoattach);
> 
>  		ac = nil;
> @@ -1160,14 +1160,8 @@
>  		nexterror();
>  	}
>  	if(name != nil) {
> -		/*
> -		 * This has to be namec(..., Aopen, ...) because
> -		 * if arg[0] is something like /srv/cs or /fd/0,
> -		 * opening it is the only way to get at the real
> -		 * Chan underneath.
> -		 */
>  		validaddr((uintptr)name, 1, 0);
> -		cmounted = namec(name, Aopen, OREAD, 0);
> +		cmounted = namec(name, Aunmount, OREAD, 0);
>  	}
>  	cunmount(cmount, cmounted);
>  	poperror();
> --- a/sys/src/9/port/sysproc.c
> +++ b/sys/src/9/port/sysproc.c
> @@ -34,6 +34,7 @@
>  	Egrp *oeg;
>  	ulong pid, flag;
>  	Mach *wm;
> +	char *devs;
> 
>  	flag = va_arg(list, ulong);
>  	/* Check flags before we commit */
> @@ -44,6 +45,11 @@
>  	if((flag & (RFENVG|RFCENVG)) == (RFENVG|RFCENVG))
>  		error(Ebadarg);
> 
> +	/*
> +	 * Code using RFNOMNT expects to block all but
> +	 * the following devices.
> +	 */
> +	devs = "|decp";
>  	if((flag&RFPROC) == 0) {
>  		if(flag & (RFMEM|RFNOWAIT))
>  			error(Ebadarg);
> @@ -60,12 +66,12 @@
>  			up->pgrp = newpgrp();
>  			if(flag & RFNAMEG)
>  				pgrpcpy(up->pgrp, opg);
> -			/* inherit noattach */
> -			up->pgrp->noattach = opg->noattach;
> +			/* inherit notallowed */
> +			memmove(up->pgrp->notallowed, opg->notallowed, sizeof up->pgrp->notallowed);
>  			closepgrp(opg);
>  		}
>  		if(flag & RFNOMNT)
> -			up->pgrp->noattach = 1;
> +			driverscmd(up->pgrp, 1, 1, &devs);
>  		if(flag & RFREND) {
>  			org = up->rgrp;
>  			up->rgrp = newrgrp();
> @@ -177,8 +183,8 @@
>  		p->pgrp = newpgrp();
>  		if(flag & RFNAMEG)
>  			pgrpcpy(p->pgrp, up->pgrp);
> -		/* inherit noattach */
> -		p->pgrp->noattach = up->pgrp->noattach;
> +		/* inherit notallowed */
> +		memmove(p->pgrp->notallowed, up->pgrp->notallowed, sizeof p->pgrp->notallowed);
>  	}
>  	else {
>  		p->pgrp = up->pgrp;
> @@ -185,7 +191,7 @@
>  		incref(p->pgrp);
>  	}
>  	if(flag & RFNOMNT)
> -		p->pgrp->noattach = 1;
> +		driverscmd(p->pgrp, 1, 1, &devs);
> 
>  	if(flag & RFREND)
>  		p->rgrp = newrgrp();
> --- a/sys/src/libauth/newns.c
> +++ b/sys/src/libauth/newns.c
> @@ -14,8 +14,8 @@
>  static int	setenv(char*, char*);
>  static char	*expandarg(char*, char*);
>  static int	splitargs(char*, char*[], char*, int);
> -static int	nsfile(char*, Biobuf *, AuthRpc *);
> -static int	nsop(char*, int, char*[], AuthRpc*);
> +static int	nsfile(char*, Biobuf *, AuthRpc *, int);
> +static int	nsop(char*, int, char*[], AuthRpc*, int);
>  static int	catch(void*, char*);
> 
>  int newnsdebug;
> @@ -35,7 +35,7 @@
>  {
>  	Biobuf *b;
>  	char home[4*ANAMELEN];
> -	int afd, cdroot;
> +	int afd, cdroot, dfd;
>  	char *path;
>  	AuthRpc *rpc;
> 
> @@ -51,6 +51,10 @@
>  	}
>  	/* rpc != nil iff afd >= 0 */
> 
> +	dfd = open("#c/drivers", OWRITE|OCEXEC);
> +	if(dfd < 0 && newnsdebug)
> +		fprint(2, "open #c/drivers: %r\n");
> +
>  	if(file == nil){
>  		if(!newns){
>  			werrstr("no namespace file specified");
> @@ -70,7 +74,8 @@
>  		setenv("home", home);
>  	}
> 
> -	cdroot = nsfile(newns ? "newns" : "addns", b, rpc);
> +	cdroot = nsfile(newns ? "newns" : "addns", b, rpc, dfd);
> +	close(dfd);
>  	Bterm(b);
>  	freecloserpc(rpc);
> 
> @@ -87,7 +92,7 @@
>  }
> 
>  static int
> -nsfile(char *fn, Biobuf *b, AuthRpc *rpc)
> +nsfile(char *fn, Biobuf *b, AuthRpc *rpc, int dfd)
>  {
>  	int argc;
>  	char *cmd, *argv[NARG+1], argbuf[MAXARG*NARG];
> @@ -103,7 +108,7 @@
>  			continue;
>  		argc = splitargs(cmd, argv, argbuf, NARG);
>  		if(argc)
> -			cdroot |= nsop(fn, argc, argv, rpc);
> +			cdroot |= nsop(fn, argc, argv, rpc, dfd);
>  	}
>  	atnotify(catch, 0);
>  	return cdroot;
> @@ -143,7 +148,7 @@
>  }
> 
>  static int
> -nsop(char *fn, int argc, char *argv[], AuthRpc *rpc)
> +nsop(char *fn, int argc, char *argv[], AuthRpc *rpc, int dfd)
>  {
>  	char *argv0;
>  	ulong flags;
> @@ -181,7 +186,7 @@
>  		b = Bopen(argv[0], OREAD|OCEXEC);
>  		if(b == nil)
>  			return 0;
> -		cdroot |= nsfile(fn, b, rpc);
> +		cdroot |= nsfile(fn, b, rpc, dfd);
>  		Bterm(b);
>  	}else if(strcmp(argv0, "clear") == 0 && argc == 0){
>  		rfork(RFCNAMEG);
> @@ -212,6 +217,14 @@
>  	}else if(strcmp(argv0, "cd") == 0 && argc == 1){
>  		if(chdir(argv[0]) == 0 && *argv[0] == '/')
>  			cdroot = 1;
> +	}else if(argc >= 1 && (strcmp(argv0, "permit") == 0 || strcmp(argv0, "block") == 0)){
> +		//We should not silently fail if we can not honor a permit/block
> +		//due to the parent namespace missing #c/drivers.
> +		if(dfd <= 0)
> +			sysfatal("%s requested, but could not open #c/drivers", argv0);
> +		for(i=0; i < argc; i++)
> +			if(fprint(dfd, "%s %s\n", argv0, argv[i]) < 0 && newnsdebug)
> +				fprint(2, "%s: %s %s %r\n", fn, argv0, argv[i]);
>  	}
>  	return cdroot;
>  }
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-11 16:11               ` Stanley Lieber
@ 2022-05-12  4:29                 ` Jacob Moody
  0 siblings, 0 replies; 28+ messages in thread
From: Jacob Moody @ 2022-05-12  4:29 UTC (permalink / raw)
  To: 9front

On 5/11/22 10:11, Stanley Lieber wrote:
> in some cases, would it make more sense to invoke auth/newns at the listener level, rather than every time rc-httpd is called?
> 

Kind of, listen uses that provided namespace for announcing and listening
itself and also as a parent for all of the running non trusted services. I think
this makes the union of what devices you might need to be pretty large. It also makes
using the namespace file to carve limited views of the root itself a bit more
difficult.But making a new namespace per connection is also pretty crap.

How about:
	/rc/bin/service/namespace.tcp80

with a default checked in file of
	/rc/bin/service/namespace.!tcp80

That way we get a more specific namespace that gets
shared for all connections of one service. You then also
have the -n argument to listen itself for setting
a namespace shared by all services.

This gets it out of /lib, and we could get
rid of /cfg/$sysname/namespace.$service.

moody

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-12  3:18             ` ori
@ 2022-05-12  5:10               ` Jacob Moody
  2022-05-12 14:21                 ` ori
  0 siblings, 1 reply; 28+ messages in thread
From: Jacob Moody @ 2022-05-12  5:10 UTC (permalink / raw)
  To: 9front

On 5/11/22 21:18, ori@eigenstate.org wrote:
> Quoth Jacob Moody <moody@mail.posixcafe.org>:
>> Hello,
>>
>> Gave this another go: http://okturing.com/src/13592/body
>>
>> This patch includes new namepace file for rc-httpd
>> and changes the existing anonymous login namespace for
>> ftpd, both make use of the sandboxing features.
> 
> Looking like it's getting useful :)

I'm glad it's starting to look that way

> comments inline.
>  
>> thanks,
>> moody
>>
>> diff 9126ee3eea90d639f4e877c01400248581d10f65 uncommitted
>> --- a/lib/namespace
>> +++ b/lib/namespace
>> @@ -1,5 +1,6 @@
>>  # root
>>  mount -aC #s/boot /root $rootspec
>> +bind $rootdir'/rc' /rc
>>  bind -a $rootdir /
> 
> Wondering why this change was needed; did a local change
> leak in?
> 

Yeah I dont love this, but there is some reason.

rc needs /rc/lib/rcmain and most scripts
expect to find themselves under /rc/bin. My goal
for the namespace for rc-httpd was to steal
what was just needed from the real root fs,
unmount the root fs, then block /srv to
make #s/boot inaccessible. So I figured
the easiest way to go about that was for
#/ to expost a rc dir. Or at least I couldn't
find a better way of bringing in /root/rc to /
without bringing in most of /root.

/root/rc needs explicitely bound over /
because the original bind of /root to / is done with MAFTER.
So #/'s empty dirs take priority. If it were to stay
I think the line should be moved down to where /$cputype/bin
is bound.

Doing the first /root to / bind with MBEFORE would also
fix this.

>>
>>  # kernel devices
>> --- a/lib/namespace.ftp
>> +++ b/lib/namespace.ftp
>> @@ -8,5 +8,6 @@
>>  # bind a personal incoming directory below incoming
>>  bind -c /usr/none/incoming /usr/web/incoming/none
>>
>> +permit |MedIa/
>>  # this cuts off everything not mounted below /usr/web
>>  bind /usr/web /
>> --- /tmp/diff100000742872
>> +++ b/lib/namespace.rc-httpd
>> @@ -1,0 +1,21 @@
>> +mount -aC #s/boot /root $rootspec
>> +
>> +# kernel devices
>> +bind #c /dev
>> +bind #d /fd
>> +bind -c #e /env
>> +bind #p /proc
>> +bind -a #l /net
>> +bind -a #I /net
> 
> duplicated line?
> 
Upper case I and lower case L.

>> +
>> +bind /root/$cputype/bin /bin
>> +bind /root/rc /rc
>> +bind -a /rc/bin /bin
>> +
>> +permit Mcde|plI/
>> +
>> +. /root/cfg/$sysname/namespace.rc-httpd
>> +
>> +unmount /root
>> +block Mc
>> +unmount #c /dev
>> --- a/sys/man/3/cons
>> +++ b/sys/man/3/cons
>> @@ -90,10 +90,30 @@
>>  .PP
>>  The
>>  .B drivers
>> -file contains, one per line, a listing of the drivers configured in the kernel, in the format
>> +file contains, one per line, a listing of available kernel drivers, in the format
>>  .IP
>>  .EX
>>  #c cons
>> +.EE
>> +.PP
>> +A process can forfeit access to a driver, for itself and it's future children, through a write to
>> +.B drivers.
> 
> Is this accurate? I'd expect the way this is implement to affect all procs
> in the current namespace, not just the current one and its children.
> 
> Maybe: 'a process can eject a driver from
> the current namespace through a write to
> drivers'.

You are correct about the implementation, it is shared by Pgrp.
Your suggestion sounds good to me.

>> +A message is one of:
>> +.IP "block \f2drivers\fP"
>> +block access to the listed
>> +.I drivers.
>> +.IP "permit \f2drivers\fP"
>> +permit access to just the provided
>> +.I drivers.
>> +.PP
>> +Drivers are identified by their short hand, the first column returned on reads,
>> +without the leading sharp. The following blocks access to
> 
> Clunky phrasing. Perhaps '\f2drivers\f2 is a string of driver characters'.
> May also need to mention the implicication of disallowing 'M'.

That makes more sense, and I agree we should mention the quirks of 'M'.

>> +.IR env (3)
>> +and
>> +.IR sd (3):
>> +.IP
>> +.EX
>> +block se
>>  .EE
>>  .PP
>>  The
>> --- a/sys/man/6/namespace
>> +++ b/sys/man/6/namespace
>> @@ -59,6 +59,15 @@
>>  .I new
>>  is missing.
>>  .TP
>> +.BI block \ drivers
>> +Removes access to the listed kernel
>> +.I drivers.
>> +Devices are identified by their sharp name.
>> +.TP
>> +.BI permit \ drivers
>> +Permit access to only the listed kernel
>> +.I drivers.
>> +.TP
>>  .BR clear
>>  Clear the name space with
>>  .BR rfork(RFCNAMEG) .
>> @@ -80,4 +89,5 @@
>>  .SH "SEE ALSO"
>>  .IR bind (1),
>>  .IR namespace (4),
>> -.IR init (8)
>> +.IR init (8),
>> +.IR cons (3)
>> --- a/sys/src/9/boot/boot.c
>> +++ b /sys/src/9/boot/boot.c
>> @@ -19,6 +19,7 @@
>>  	if(await(buf, sizeof(buf)) < 0)
>>  		goto Err;
>>
>> +	bind("/root/rc", "/rc", MREPL);
>>  	bind(root, "/", MAFTER);
> 
> Hm. as before, I don't understand why this bind is needed:
> binding /root to / should already lead to /rc being /root/rc.
> 

See above.

>>
>>  	buf[0] = '/';
>> --- a/sys/src/9/port/chan.c
>> +++ b/sys/src/9/port/chan.c
>> @@ -1272,7 +1272,7 @@
>>  Chan*
>>  namec(char *aname, int amode, int omode, ulong perm)
>>  {
>> -	int len, n, t, nomount;
>> +	int len, n, t, nomount, devunmount;
>>  	Chan *c;
>>  	Chan *volatile cnew;
>>  	Path *volatile path;
>> @@ -1292,6 +1292,24 @@
>>  	name = aname;
>>
>>  	/*
>> +	 * When unmounting, the name parameter must be accessed
>> +	 * using Aopen in order to get the real chan from
>> +	 * something like /srv/cs or /fd/0. However when sandboxing,
>> +	 * unmounting a sharp from a union is a valid operation even
>> +	 * if the device is blocked.
>> +	 */
>> +	if(amode == Aunmount){
>> +		/*
>> +		 * Doing any walks down the device could leak information
>> +		 * about the existence of files.
>> +		 */
>> +		if(name[0] == '#' && utflen(name) == 2)
>> +			devunmount = 1;
>> +		amode = Aopen;
>> +	} else
>> +		devunmount = 0;
>> +
>> +	/*
>>  	 * Find the starting off point (the current slash, the root of
>>  	 * a device tree, or the current dot) as well as the name to
>>  	 * evaluate starting there.
>> @@ -1313,24 +1331,13 @@
>>  			up->genbuf[n++] = *name++;
>>  		}
>>  		up->genbuf[n] = '\0';
>> -		/*
>> -		 *  noattach is sandboxing.
>> -		 *
>> -		 *  the OK exceptions are:
>> -		 *	|  it only gives access to pipes you create
>> -		 *	d  this process's file descriptors
>> -		 *	e  this process's environment
>> -		 *  the iffy exceptions are:
>> -		 *	c  time and pid, but also cons and consctl
>> -		 *	p  control of your own processes (and unfortunately
>> -		 *	   any others left unprotected)
>> -		 */
>>  		n = chartorune(&r, up->genbuf+1)+1;
>> -		if(up->pgrp->noattach && utfrune("|decp", r)==nil)
>> -			error(Enoattach);
>>  		t = devno(r, 1);
>>  		if(t == -1)
>>  			error(Ebadsharp);
>> +		if(!devunmount && !driversallowed(up->pgrp, r))
>> +			error(Enoattach);
>> +
>>  		c = devtab[t]->attach(up->genbuf+n);
>>  		break;
>>
>> --- a/sys/src/9/port/devcons.c
>> +++ b/sys/src/9/port/devcons.c
>> @@ -39,6 +39,18 @@
>>  	CMrdb,		"rdb",		0,
>>  };
>>
>> +enum
>> +{
>> +	CMblock,
>> +	CMpermit,
>> +};
>> +
>> +Cmdtab drivermsg[] =
>> +{
>> +	CMblock,	"block",	0,
>> +	CMpermit,	"permit",	0,
>> +};
>> +
>>  void
>>  printinit(void)
>>  {
>> @@ -332,7 +344,7 @@
>>  	"cons",		{Qcons},	0,		0660,
>>  	"consctl",	{Qconsctl},	0,		0220,
>>  	"cputime",	{Qcputime},	6*NUMSIZE,	0444,
>> -	"drivers",	{Qdrivers},	0,		0444,
>> +	"drivers",	{Qdrivers},	0,		0666,
>>  	"hostdomain",	{Qhostdomain},	DOMLEN,		0664,
>>  	"hostowner",	{Qhostowner},	0,		0664,
>>  	"kmesg",	{Qkmesg},	0,		0440,
>> @@ -583,9 +595,15 @@
>>  	case Qdrivers:
>>  		b = smalloc(READSTR);
>>  		k = 0;
>> -		for(i = 0; devtab[i] != nil; i++)
>> +		
>> +		rlock(&up->pgrp->ns);
>> +		for(i = 0; devtab[i] != nil; i++){
>> +			if(up->pgrp->notallowed[i/(sizeof(u64int)*8)] & 1<<i%(sizeof(u64int)*8))
>> +				continue;
>>  			k += snprint(b+k, READSTR-k, "#%C %s\n", devtab[i]->dc, devtab[i]->name);
>> +		}
>> +		runlock(&up->pgrp->ns);
>>  		if(waserror()){
>>  			free(b);
>>  			nexterror();
>> @@ -676,6 +694,26 @@
>>  		error(Eperm);
>>  		break;
>>
>> +	case Qdrivers:
>> +		cb = parsecmd(a, n);
>> +
>> +		if(waserror()) {
>> +			free(cb);
>> +			nexterror();
>> +		}
>> +		ct = lookupcmd(cb, drivermsg, nelem(drivermsg));
>> +		switch(ct->index) {
>> +		case CMblock:
>> +			driverscmd(up->pgrp, 0, cb->nf-1, cb->f+1);
>> +			break;
>> +		case CMpermit:
>> +			driverscmd(up->pgrp, 1, cb->nf-1, cb->f+1);
>> +			break;
>> +		}
>> +		poperror();
>> +		free(cb);
>> +		break;
>> +
>>  	case Qreboot:
>>  		if(!iseve())
>>  			error(Eperm);
>> @@ -935,6 +973,64 @@
>>  		break;
>>  	}
>>  	return n+1;
>> +}
>> +
>> +void
>> +driverscmd(Pgrp *pgrp, int invert, int argc, char *argv[])
>> +{
>> +	int i, t, w;
>> +	char *p;
>> +	Rune r;
>> +	u64int mask[nelem(pgrp->notallowed)];
>> +
>> +	if(invert)
>> +		memset(mask, 0xFF, sizeof mask);
>> +	else
>> +		memset(mask, 0, sizeof mask);
>> +
>> +	w = sizeof mask[0] * 8;
>> +	for(i=0; i < argc; i++)
>> +		for(p = argv[i]; *p != '\0';){
>> +			p += chartorune(&r, p);
>> +			t = devno(r, 1);
>> +			if(t == -1)
>> +				continue;
>> +			if(invert)
>> +				mask[t/w] &= ~(1<<t%w);
>> +			else
>> +				mask[t/w] |= 1<<t%w;
>> +		}
>> +
>> +	wlock(&pgrp->ns);
>> +	for(i=0; i < nelem(pgrp->notallowed); i++)
>> +		pgrp->notallowed[i] |= mask[i];
>> +	wunlock(&pgrp->ns);
>> +}
>> +
>> +int
>> +driversallowed(Pgrp *pgrp, int r)
>> +{
>> +	int t, w, b;
>> +
>> +	t = devno(r, 1);
>> +	if(t == -1)
>> +		return 0;
>> +
>> +	w = sizeof(u64int) * 8;
>> +	rlock(&pgrp->ns);
>> +	b = !(pgrp->notallowed[t/w] & 1<<t%w);
>> +	runlock(&pgrp->ns);
>> +	return b;
>> +}
>> +
>> +int
>> +canmount(Pgrp *pgrp)
>> +{
>> +	/*
>> +	 * Devmnt is not usable directly from user procs, so
>> +	 * having it removed is interpreted to block any mounts.
>> +	 */
>> +	return driversallowed(pgrp, 'M');
>>  }
>>
>>  void
>> --- a/sys/src/9/port/devroot.c
>> +++ b/sys/src/9/port/devroot.c
>> @@ -105,6 +105,7 @@
>>  	addrootdir("net");
>>  	addrootdir("net.alt");
>>  	addrootdir("proc");
>> +	addrootdir("rc");
>>  	addrootdir("root");
>>  	addrootdir("srv");
>>  	addrootdir("shr");
>> --- a/sys/src/9/port/devshr.c
>> +++ b/sys/src/9/port/devshr.c
>> @@ -464,7 +464,7 @@
>>  		cclose(c);
>>  		return nc;	
>>  	case Qcroot:
>> -		if(up->pgrp->noattach)
>> +		if(!canmount(up->pgrp))
>>  			error(Enoattach);
>>  		if((perm & DMDIR) == 0 || mode != OREAD)
>>  			error(Eperm);
>> @@ -498,7 +498,7 @@
>>  		sch->shr = shr;
>>  		break;
>>  	case Qcshr:
>> -		if(up->pgrp->noattach)
>> +		if(!canmount(up->pgrp))
>>  			error(Enoattach);
>>  		if((perm & DMDIR) != 0 || mode != OWRITE)
>>  			error(Eperm);
>> @@ -731,7 +731,7 @@
>>  	Mhead *h;
>>  	Mount *m;
>>
>> -	if(up->pgrp->noattach)
>> +	if(!canmount(up->pgrp))
>>  		error(Enoattach);
>>  	sch = tosch(c);
>>  	if(sch->level != Qcmpt)
>> --- a/sys/src/9/port/mkdevc
>> +++ b/sys/src/9/port/mkdevc
>> @@ -78,6 +78,9 @@
>>  	if(ARGC < 2)
>>  		exit "usage"
>>
>> +	if(ndev >= 256)
>> +		exit "device count will overflow Pgrp.notallowed"
>> +
>>  	printf "#include \"u.h\"\n";
>>  	printf "#include \"../port/lib.h\"\n";
>>  	printf "#include \"mem.h\"\n";
>> --- a/sys/src/9/port/portdat.h
>> +++ b/sys/src/9/port/portdat.h
>> @@ -121,6 +121,7 @@
>>  	Amount,				/* to be mounted or mounted upon */
>>  	Acreate,			/* is to be created */
>>  	Aremove,			/* will be removed by caller */
>> +	Aunmount,			/* unmount arg[0] */
>>
>>  	COPEN	= 0x0001,		/* for i/o */
>>  	CMSG	= 0x0002,		/* the message channel for a mount */
>> @@ -484,7 +485,7 @@
>>  {
>>  	Ref;
>>  	RWlock	ns;			/* Namespace n read/one write lock */
>> -	int	noattach;
>> +	u64int	notallowed[4];		/* Room for 256 devices */
>>  	Mhead	*mnthash[MNTHASH];
>>  };
>>
>> --- a/sys/src/9/port/portfns.h
>> +++ b/sys/src/9/port/portfns.h
>> @@ -413,6 +413,9 @@
>>  ushort		nhgets(void*);
>>  ulong		µs(void);
>>  long		lcycles(void);
>> +void		driverscmd(Pgrp*,int,int,char**);
>> +int		driversallowed(Pgrp*, int);
>> +int		canmount(Pgrp*);
>>
>>  #pragma varargck argpos iprint	1
>>  #pragma	varargck argpos	panic	1
>> --- a/sys/src/9/port/sysfile.c
>> +++ b/sys/src/9/port/sysfile.c
>> @@ -1048,7 +1048,7 @@
>>  			nexterror();
>>  		}
>>
>> -		if(up->pgrp->noattach)
>> +		if(!canmount(up->pgrp))
>>  			error(Enoattach);
>>
>>  		ac = nil;
>> @@ -1160,14 +1160,8 @@
>>  		nexterror();
>>  	}
>>  	if(name != nil) {
>> -		/*
>> -		 * This has to be namec(..., Aopen, ...) because
>> -		 * if arg[0] is something like /srv/cs or /fd/0,
>> -		 * opening it is the only way to get at the real
>> -		 * Chan underneath.
>> -		 */
>>  		validaddr((uintptr)name, 1, 0);
>> -		cmounted = namec(name, Aopen, OREAD, 0);
>> +		cmounted = namec(name, Aunmount, OREAD, 0);
>>  	}
>>  	cunmount(cmount, cmounted);
>>  	poperror();
>> --- a/sys/src/9/port/sysproc.c
>> +++ b/sys/src/9/port/sysproc.c
>> @@ -34,6 +34,7 @@
>>  	Egrp *oeg;
>>  	ulong pid, flag;
>>  	Mach *wm;
>> +	char *devs;
>>
>>  	flag = va_arg(list, ulong);
>>  	/* Check flags before we commit */
>> @@ -44,6 +45,11 @@
>>  	if((flag & (RFENVG|RFCENVG)) == (RFENVG|RFCENVG))
>>  		error(Ebadarg);
>>
>> +	/*
>> +	 * Code using RFNOMNT expects to block all but
>> +	 * the following devices.
>> +	 */
>> +	devs = "|decp";
>>  	if((flag&RFPROC) == 0) {
>>  		if(flag & (RFMEM|RFNOWAIT))
>>  			error(Ebadarg);
>> @@ -60,12 +66,12 @@
>>  			up->pgrp = newpgrp();
>>  			if(flag & RFNAMEG)
>>  				pgrpcpy(up->pgrp, opg);
>> -			/* inherit noattach */
>> -			up->pgrp->noattach = opg->noattach;
>> +			/* inherit notallowed */
>> +			memmove(up->pgrp->notallowed, opg->notallowed, sizeof up->pgrp->notallowed);
>>  			closepgrp(opg);
>>  		}
>>  		if(flag & RFNOMNT)
>> -			up->pgrp->noattach = 1;
>> +			driverscmd(up->pgrp, 1, 1, &devs);
>>  		if(flag & RFREND) {
>>  			org = up->rgrp;
>>  			up->rgrp = newrgrp();
>> @@ -177,8 +183,8 @@
>>  		p->pgrp = newpgrp();
>>  		if(flag & RFNAMEG)
>>  			pgrpcpy(p->pgrp, up->pgrp);
>> -		/* inherit noattach */
>> -		p->pgrp->noattach = up->pgrp->noattach;
>> +		/* inherit notallowed */
>> +		memmove(p->pgrp->notallowed, up->pgrp->notallowed, sizeof p->pgrp->notallowed);
>>  	}
>>  	else {
>>  		p->pgrp = up->pgrp;
>> @@ -185,7 +191,7 @@
>>  		incref(p->pgrp);
>>  	}
>>  	if(flag & RFNOMNT)
>> -		p->pgrp->noattach = 1;
>> +		driverscmd(p->pgrp, 1, 1, &devs);
>>
>>  	if(flag & RFREND)
>>  		p->rgrp = newrgrp();
>> --- a/sys/src/libauth/newns.c
>> +++ b/sys/src/libauth/newns.c
>> @@ -14,8 +14,8 @@
>>  static int	setenv(char*, char*);
>>  static char	*expandarg(char*, char*);
>>  static int	splitargs(char*, char*[], char*, int);
>> -static int	nsfile(char*, Biobuf *, AuthRpc *);
>> -static int	nsop(char*, int, char*[], AuthRpc*);
>> +static int	nsfile(char*, Biobuf *, AuthRpc *, int);
>> +static int	nsop(char*, int, char*[], AuthRpc*, int);
>>  static int	catch(void*, char*);
>>
>>  int newnsdebug;
>> @@ -35,7 +35,7 @@
>>  {
>>  	Biobuf *b;
>>  	char home[4*ANAMELEN];
>> -	int afd, cdroot;
>> +	int afd, cdroot, dfd;
>>  	char *path;
>>  	AuthRpc *rpc;
>>
>> @@ -51,6 +51,10 @@
>>  	}
>>  	/* rpc != nil iff afd >= 0 */
>>
>> +	dfd = open("#c/drivers", OWRITE|OCEXEC);
>> +	if(dfd < 0 && newnsdebug)
>> +		fprint(2, "open #c/drivers: %r\n");
>> +
>>  	if(file == nil){
>>  		if(!newns){
>>  			werrstr("no namespace file specified");
>> @@ -70,7 +74,8 @@
>>  		setenv("home", home);
>>  	}
>>
>> -	cdroot = nsfile(newns ? "newns" : "addns", b, rpc);
>> +	cdroot = nsfile(newns ? "newns" : "addns", b, rpc, dfd);
>> +	close(dfd);
>>  	Bterm(b);
>>  	freecloserpc(rpc);
>>
>> @@ -87,7 +92,7 @@
>>  }
>>
>>  static int
>> -nsfile(char *fn, Biobuf *b, AuthRpc *rpc)
>> +nsfile(char *fn, Biobuf *b, AuthRpc *rpc, int dfd)
>>  {
>>  	int argc;
>>  	char *cmd, *argv[NARG+1], argbuf[MAXARG*NARG];
>> @@ -103,7 +108,7 @@
>>  			continue;
>>  		argc = splitargs(cmd, argv, argbuf, NARG);
>>  		if(argc)
>> -			cdroot |= nsop(fn, argc, argv, rpc);
>> +			cdroot |= nsop(fn, argc, argv, rpc, dfd);
>>  	}
>>  	atnotify(catch, 0);
>>  	return cdroot;
>> @@ -143,7 +148,7 @@
>>  }
>>
>>  static int
>> -nsop(char *fn, int argc, char *argv[], AuthRpc *rpc)
>> +nsop(char *fn, int argc, char *argv[], AuthRpc *rpc, int dfd)
>>  {
>>  	char *argv0;
>>  	ulong flags;
>> @@ -181,7 +186,7 @@
>>  		b = Bopen(argv[0], OREAD|OCEXEC);
>>  		if(b == nil)
>>  			return 0;
>> -		cdroot |= nsfile(fn, b, rpc);
>> +		cdroot |= nsfile(fn, b, rpc, dfd);
>>  		Bterm(b);
>>  	}else if(strcmp(argv0, "clear") == 0 && argc == 0){
>>  		rfork(RFCNAMEG);
>> @@ -212,6 +217,14 @@
>>  	}else if(strcmp(argv0, "cd") == 0 && argc == 1){
>>  		if(chdir(argv[0]) == 0 && *argv[0] == '/')
>>  			cdroot = 1;
>> +	}else if(argc >= 1 && (strcmp(argv0, "permit") == 0 || strcmp(argv0, "block") == 0)){
>> +		//We should not silently fail if we can not honor a permit/block
>> +		//due to the parent namespace missing #c/drivers.
>> +		if(dfd <= 0)
>> +			sysfatal("%s requested, but could not open #c/drivers", argv0);
>> +		for(i=0; i < argc; i++)
>> +			if(fprint(dfd, "%s %s\n", argv0, argv[i]) < 0 && newnsdebug)
>> +				fprint(2, "%s: %s %s %r\n", fn, argv0, argv[i]);
>>  	}
>>  	return cdroot;
>>  }
>>
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-12  5:10               ` Jacob Moody
@ 2022-05-12 14:21                 ` ori
  2022-05-23  5:42                   ` Jacob Moody
  0 siblings, 1 reply; 28+ messages in thread
From: ori @ 2022-05-12 14:21 UTC (permalink / raw)
  To: 9front

Quoth Jacob Moody <moody@mail.posixcafe.org>:
> >> diff 9126ee3eea90d639f4e877c01400248581d10f65 uncommitted
> >> --- a/lib/namespace
> >> +++ b/lib/namespace
> >> @@ -1,5 +1,6 @@
> >>  # root
> >>  mount -aC #s/boot /root $rootspec
> >> +bind $rootdir'/rc' /rc
> >>  bind -a $rootdir /
> > 
> > Wondering why this change was needed; did a local change
> > leak in?
> > 
> 
> Yeah I dont love this, but there is some reason.
> 
> rc needs /rc/lib/rcmain

maybe we change that, and either make it optional or hard
code it into rc?

(does anyone customize it?)


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-12 14:21                 ` ori
@ 2022-05-23  5:42                   ` Jacob Moody
  2022-05-23 17:06                     ` cinap_lenrek
  0 siblings, 1 reply; 28+ messages in thread
From: Jacob Moody @ 2022-05-23  5:42 UTC (permalink / raw)
  To: 9front

Another go at this. Bit of a refactor of the
kernel changes and wrapped up the userspace work.
Since some of the thoughts are scattered throughout
the thread now I wanted to provide an overview of how things ended up:

A process can eject devices from it's namespace through
writes to /dev/drivers. Ejected devices can not be walked.
A 'permit' command is available to allow processes to eject
devices by whitelist rather then blacklist.

Support for these commands has been added to newns, allowing
namespace files to use eject/permit commands. These new commands
are used in /lib/namespace.ftp to restrict anonymous login.

aux/listen now allows individual namespace files to be used
per service. This namespace is sourced one per listener. Each
connection receives a copy of the namespace. A example
/rc/bin/service/!tcp80.namespace is provided which builds
an isolated webroot. In order to accomplish this without
bringing in most of /root, a /rc folder was added to #/.

This is prsented as one diff, but I plan to commit this
in chunks.


Thanks,
moody

---
diff 6fbb1acc8fa0b6655b14e8c46240a4a8d2d8c672 uncommitted
--- a/lib/namespace
+++ b/lib/namespace
@@ -22,6 +22,7 @@

 # standard bin
 bind /$cputype/bin /bin
+bind $rootdir'/rc' /rc
 bind -a /rc/bin /bin

 # internal networks
--- a/lib/namespace.ftp
+++ b/lib/namespace.ftp
@@ -8,5 +8,6 @@
 # bind a personal incoming directory below incoming
 bind -c /usr/none/incoming /usr/web/incoming/none

+permit |MedIa/
 # this cuts off everything not mounted below /usr/web
 bind /usr/web /
--- /tmp/diff100000406351
+++ b/rc/bin/service/!tcp80.namespace
@@ -1,0 +1,24 @@
+mount -aC #s/boot /root $rootspec
+
+# kernel devices
+bind #c /dev
+bind #d /fd
+bind -c #e /env
+bind #p /proc
+bind -a #l /net
+bind -a #I /net
+
+bind /root/$cputype/bin /bin
+bind /root/rc /rc
+bind -a /rc/bin /bin
+
+permit Mcde|pslI/
+
+# grab just our webroot
+bind /root/usr/web /srv
+
+# or bind in the actual root
+# bind -a /root /
+
+unmount /root
+eject Ms
--- a/sys/man/3/cons
+++ b/sys/man/3/cons
@@ -90,10 +90,32 @@
 .PP
 The
 .B drivers
-file contains, one per line, a listing of the drivers configured in the kernel, in the format
+file contains, one per line, a listing of available kernel drivers, in the format
 .IP
 .EX
 #c cons
+.EE
+.PP
+A process can eject a driver from the current namespace through a write to
+.B drivers.
+A message is one of:
+.IP "eject \f2drivers\fP"
+block access to the listed
+.I drivers.
+.IP "permit \f2drivers\fP"
+permit access to only the provided
+.I drivers.
+.PP
+\f2Drivers\fP is a string of driver characters. Ejecting
+.IR mnt (3)
+prevents new mounts in to the current namespace.
+The following blocks access to
+.IR env (3)
+and
+.IR sd (3):
+.IP
+.EX
+eject se
 .EE
 .PP
 The
--- a/sys/man/3/root
+++ b/sys/man/3/root
@@ -10,6 +10,7 @@
 .B /net
 .B /net.alt
 .B /proc
+.B /rc
 .B /root
 .B /srv
 .fi
--- a/sys/man/6/namespace
+++ b/sys/man/6/namespace
@@ -59,6 +59,17 @@
 .I new
 is missing.
 .TP
+.BI eject \ drivers
+Ejects the listed kernel
+.I drivers
+from the namespace.
+.I Drivers
+is a string of driver characters.
+.TP
+.BI permit \ drivers
+Permit access to only the listed kernel
+.I drivers.
+.TP
 .BR clear
 Clear the name space with
 .BR rfork(RFCNAMEG) .
@@ -80,4 +91,5 @@
 .SH "SEE ALSO"
 .IR bind (1),
 .IR namespace (4),
-.IR init (8)
+.IR init (8),
+.IR cons (3)
--- a/sys/man/8/listen
+++ b/sys/man/8/listen
@@ -96,6 +96,14 @@
 an inbound call on the TCP network for port 565 executes service
 .BR tcp565 .
 .PP
+Services may have individual
+.IR namespace (6)
+files specified within
+.IR srvdir .
+If provided, the namespace is used as the parent for each connection
+to the corresponding service. Namespace files are found by appending a .namespace
+suffix to the service name.
+.PP
 At least the following services are available in
 .BR /bin/service .
 .TF \ tcp0000
--- a/sys/src/9/boot/boot.c
+++ b/sys/src/9/boot/boot.c
@@ -25,6 +25,7 @@
 	buf[1+read(open("/env/cputype", OREAD|OCEXEC), buf+1, sizeof buf - 6)] = '\0';
 	strcat(buf, bin);
 	bind(buf, bin, MAFTER);
+	bind("/root/rc", "/rc", MREPL);
 	bind("/rc/bin", bin, MAFTER);

 	exec("/bin/bootrc", argv);
--- a/sys/src/9/port/chan.c
+++ b/sys/src/9/port/chan.c
@@ -1272,7 +1272,7 @@
 Chan*
 namec(char *aname, int amode, int omode, ulong perm)
 {
-	int len, n, t, nomount;
+	int len, n, t, nomount, devunmount;
 	Chan *c;
 	Chan *volatile cnew;
 	Path *volatile path;
@@ -1292,6 +1292,24 @@
 	name = aname;

 	/*
+	 * When unmounting, the name parameter must be accessed
+	 * using Aopen in order to get the real chan from
+	 * something like /srv/cs or /fd/0. However when sandboxing,
+	 * unmounting a sharp from a union is a valid operation even
+	 * if the device is blocked.
+	 */
+	devunmount = 0;
+	if(amode == Aunmount){
+		/*
+		 * Doing any walks down the device could leak information
+		 * about the existence of files.
+		 */
+		if(name[0] == '#' && utflen(name) == 2)
+			devunmount = 1;
+		amode = Aopen;
+	}
+
+	/*
 	 * Find the starting off point (the current slash, the root of
 	 * a device tree, or the current dot) as well as the name to
 	 * evaluate starting there.
@@ -1313,24 +1331,13 @@
 			up->genbuf[n++] = *name++;
 		}
 		up->genbuf[n] = '\0';
-		/*
-		 *  noattach is sandboxing.
-		 *
-		 *  the OK exceptions are:
-		 *	|  it only gives access to pipes you create
-		 *	d  this process's file descriptors
-		 *	e  this process's environment
-		 *  the iffy exceptions are:
-		 *	c  time and pid, but also cons and consctl
-		 *	p  control of your own processes (and unfortunately
-		 *	   any others left unprotected)
-		 */
 		n = chartorune(&r, up->genbuf+1)+1;
-		if(up->pgrp->noattach && utfrune("|decp", r)==nil)
-			error(Enoattach);
 		t = devno(r, 1);
 		if(t == -1)
 			error(Ebadsharp);
+		if(!devunmount && !devallowed(up->pgrp, r))
+			error(Enoattach);
+
 		c = devtab[t]->attach(up->genbuf+n);
 		break;

--- a/sys/src/9/port/dev.c
+++ b/sys/src/9/port/dev.c
@@ -31,6 +31,63 @@
 }

 void
+deveject(Pgrp *pgrp, int invert, char *devs)
+{
+	int i, t, w;
+	char *p;
+	Rune r;
+	u64int mask[nelem(pgrp->notallowed)];
+
+	if(invert)
+		memset(mask, 0xFF, sizeof mask);
+	else
+		memset(mask, 0, sizeof mask);
+
+	w = sizeof mask[0] * 8;
+	for(p = devs; *p != '\0';){
+		p += chartorune(&r, p);
+		t = devno(r, 1);
+		if(t == -1)
+			continue;
+		if(invert)
+			mask[t/w] &= ~(1<<t%w);
+		else
+			mask[t/w] |= 1<<t%w;
+	}
+
+	wlock(&pgrp->ns);
+	for(i=0; i < nelem(pgrp->notallowed); i++)
+		pgrp->notallowed[i] |= mask[i];
+	wunlock(&pgrp->ns);
+}
+
+int
+devallowed(Pgrp *pgrp, int r)
+{
+	int t, w, b;
+
+	t = devno(r, 1);
+	if(t == -1)
+		return 0;
+
+	w = sizeof(u64int) * 8;
+	rlock(&pgrp->ns);
+	b = !(pgrp->notallowed[t/w] & 1<<t%w);
+	runlock(&pgrp->ns);
+	return b;
+}
+
+int
+canmount(Pgrp *pgrp)
+{
+	/*
+	 * Devmnt is not usable directly from user procs, so
+	 * having it removed is interpreted to block any mounts.
+	 */
+	return devallowed(pgrp, 'M');
+}
+
+void
 devdir(Chan *c, Qid qid, char *n, vlong length, char *user, long perm, Dir *db)
 {
 	db->name = n;
--- a/sys/src/9/port/devcons.c
+++ b/sys/src/9/port/devcons.c
@@ -39,6 +39,18 @@
 	CMrdb,		"rdb",		0,
 };

+enum
+{
+	CMeject,
+	CMpermit,
+};
+
+Cmdtab drivermsg[] =
+{
+	CMeject,	"eject",	0,
+	CMpermit,	"permit",	0,
+};
+
 void
 printinit(void)
 {
@@ -332,7 +344,7 @@
 	"cons",		{Qcons},	0,		0660,
 	"consctl",	{Qconsctl},	0,		0220,
 	"cputime",	{Qcputime},	6*NUMSIZE,	0444,
-	"drivers",	{Qdrivers},	0,		0444,
+	"drivers",	{Qdrivers},	0,		0666,
 	"hostdomain",	{Qhostdomain},	DOMLEN,		0664,
 	"hostowner",	{Qhostowner},	0,		0664,
 	"kmesg",	{Qkmesg},	0,		0440,
@@ -583,9 +595,15 @@
 	case Qdrivers:
 		b = smalloc(READSTR);
 		k = 0;
-		for(i = 0; devtab[i] != nil; i++)
+		
+		rlock(&up->pgrp->ns);
+		for(i = 0; devtab[i] != nil; i++){
+			if(up->pgrp->notallowed[i/(sizeof(u64int)*8)] & 1<<i%(sizeof(u64int)*8))
+				continue;
 			k += snprint(b+k, READSTR-k, "#%C %s\n",
 				devtab[i]->dc, devtab[i]->name);
+		}
+		runlock(&up->pgrp->ns);
 		if(waserror()){
 			free(b);
 			nexterror();
@@ -622,7 +640,7 @@
 	long l, bp;
 	char *a;
 	Mach *mp;
-	int id;
+	int id, i, invert;
 	ulong offset;
 	Cmdbuf *cb;
 	Cmdtab *ct;
@@ -674,6 +692,32 @@

 	case Qconfig:
 		error(Eperm);
+		break;
+
+	case Qdrivers:
+		cb = parsecmd(a, n);
+
+		if(waserror()) {
+			free(cb);
+			nexterror();
+		}
+		ct = lookupcmd(cb, drivermsg, nelem(drivermsg));
+		invert = 0;
+		switch(ct->index) {
+		case CMeject:
+			invert = 0;
+			break;
+		case CMpermit:
+			invert = 1;
+			break;
+		default:
+			error(Ebadarg);
+			break;
+		}
+		for(i = 1; i < cb->nf; i++)
+			deveject(up->pgrp, invert, cb->f[i]);
+		poperror();
+		free(cb);
 		break;

 	case Qreboot:
--- a/sys/src/9/port/devroot.c
+++ b/sys/src/9/port/devroot.c
@@ -105,6 +105,7 @@
 	addrootdir("net");
 	addrootdir("net.alt");
 	addrootdir("proc");
+	addrootdir("rc");
 	addrootdir("root");
 	addrootdir("srv");
 	addrootdir("shr");
--- a/sys/src/9/port/devshr.c
+++ b/sys/src/9/port/devshr.c
@@ -464,7 +464,7 @@
 		cclose(c);
 		return nc;	
 	case Qcroot:
-		if(up->pgrp->noattach)
+		if(!canmount(up->pgrp))
 			error(Enoattach);
 		if((perm & DMDIR) == 0 || mode != OREAD)
 			error(Eperm);
@@ -498,7 +498,7 @@
 		sch->shr = shr;
 		break;
 	case Qcshr:
-		if(up->pgrp->noattach)
+		if(!canmount(up->pgrp))
 			error(Enoattach);
 		if((perm & DMDIR) != 0 || mode != OWRITE)
 			error(Eperm);
@@ -731,7 +731,7 @@
 	Mhead *h;
 	Mount *m;

-	if(up->pgrp->noattach)
+	if(!canmount(up->pgrp))
 		error(Enoattach);
 	sch = tosch(c);
 	if(sch->level != Qcmpt)
--- a/sys/src/9/port/mkdevc
+++ b/sys/src/9/port/mkdevc
@@ -78,6 +78,9 @@
 	if(ARGC < 2)
 		exit "usage"

+	if(ndev >= 256)
+		exit "device count will overflow Pgrp.notallowed"
+
 	printf "#include \"u.h\"\n";
 	printf "#include \"../port/lib.h\"\n";
 	printf "#include \"mem.h\"\n";
--- a/sys/src/9/port/portdat.h
+++ b/sys/src/9/port/portdat.h
@@ -121,6 +121,7 @@
 	Amount,				/* to be mounted or mounted upon */
 	Acreate,			/* is to be created */
 	Aremove,			/* will be removed by caller */
+	Aunmount,			/* unmount arg[0] */

 	COPEN	= 0x0001,		/* for i/o */
 	CMSG	= 0x0002,		/* the message channel for a mount */
@@ -484,7 +485,7 @@
 {
 	Ref;
 	RWlock	ns;			/* Namespace n read/one write lock */
-	int	noattach;
+	u64int	notallowed[4];		/* Room for 256 devices */
 	Mhead	*mnthash[MNTHASH];
 };

--- a/sys/src/9/port/portfns.h
+++ b/sys/src/9/port/portfns.h
@@ -413,6 +413,9 @@
 ushort		nhgets(void*);
 ulong		µs(void);
 long		lcycles(void);
+void		deveject(Pgrp*,int,char*);
+int		devallowed(Pgrp*, int);
+int		canmount(Pgrp*);

 #pragma varargck argpos iprint	1
 #pragma	varargck argpos	panic	1
--- a/sys/src/9/port/sysfile.c
+++ b/sys/src/9/port/sysfile.c
@@ -1048,7 +1048,7 @@
 			nexterror();
 		}

-		if(up->pgrp->noattach)
+		if(!canmount(up->pgrp))
 			error(Enoattach);

 		ac = nil;
@@ -1160,14 +1160,8 @@
 		nexterror();
 	}
 	if(name != nil) {
-		/*
-		 * This has to be namec(..., Aopen, ...) because
-		 * if arg[0] is something like /srv/cs or /fd/0,
-		 * opening it is the only way to get at the real
-		 * Chan underneath.
-		 */
 		validaddr((uintptr)name, 1, 0);
-		cmounted = namec(name, Aopen, OREAD, 0);
+		cmounted = namec(name, Aunmount, OREAD, 0);
 	}
 	cunmount(cmount, cmounted);
 	poperror();
--- a/sys/src/9/port/sysproc.c
+++ b/sys/src/9/port/sysproc.c
@@ -34,6 +34,7 @@
 	Egrp *oeg;
 	ulong pid, flag;
 	Mach *wm;
+	char *devs;

 	flag = va_arg(list, ulong);
 	/* Check flags before we commit */
@@ -44,6 +45,11 @@
 	if((flag & (RFENVG|RFCENVG)) == (RFENVG|RFCENVG))
 		error(Ebadarg);

+	/*
+	 * Code using RFNOMNT expects to block all but
+	 * the following devices.
+	 */
+	devs = "|decp";
 	if((flag&RFPROC) == 0) {
 		if(flag & (RFMEM|RFNOWAIT))
 			error(Ebadarg);
@@ -60,12 +66,12 @@
 			up->pgrp = newpgrp();
 			if(flag & RFNAMEG)
 				pgrpcpy(up->pgrp, opg);
-			/* inherit noattach */
-			up->pgrp->noattach = opg->noattach;
+			/* inherit notallowed */
+			memmove(up->pgrp->notallowed, opg->notallowed, sizeof up->pgrp->notallowed);
 			closepgrp(opg);
 		}
 		if(flag & RFNOMNT)
-			up->pgrp->noattach = 1;
+			deveject(up->pgrp, 1, devs);
 		if(flag & RFREND) {
 			org = up->rgrp;
 			up->rgrp = newrgrp();
@@ -177,8 +183,8 @@
 		p->pgrp = newpgrp();
 		if(flag & RFNAMEG)
 			pgrpcpy(p->pgrp, up->pgrp);
-		/* inherit noattach */
-		p->pgrp->noattach = up->pgrp->noattach;
+		/* inherit notallowed */
+		memmove(p->pgrp->notallowed, up->pgrp->notallowed, sizeof p->pgrp->notallowed);
 	}
 	else {
 		p->pgrp = up->pgrp;
@@ -185,7 +191,7 @@
 		incref(p->pgrp);
 	}
 	if(flag & RFNOMNT)
-		p->pgrp->noattach = 1;
+		deveject(p->pgrp, 1, devs);

 	if(flag & RFREND)
 		p->rgrp = newrgrp();
--- a/sys/src/cmd/aux/listen.c
+++ b/sys/src/cmd/aux/listen.c
@@ -136,6 +136,7 @@
 {
 	int ctl, pid, start;
 	char dir[40], err[128], ds[128];
+	char prog[Maxpath], serv[Maxserv], ns[Maxpath];
 	long childs;
 	Announce *a;
 	Waitmsg *wm;
@@ -178,6 +179,10 @@
 			sleep((pid*10)%200);

 			snprint(ds, sizeof ds, "%s!%s!%s", protodir, addr, a->a);
+			snprint(serv, sizeof serv, "%s%s", proto, a->a);
+			snprint(prog, sizeof prog, "%s/%s", srvdir, serv);
+			snprint(ns, sizeof ns, "%s.namespace", prog);
+
 			whined = a->whined;

 			/* a process per service */
@@ -201,7 +206,11 @@
 						else
 							exits("ctl");
 					}
-					dolisten(dir, ctl, srvdir, a->a, &childs);
+					procsetname("%s %s", dir, ds);
+					if(!trusted)
+						if(newns("none", ns) < 0)
+							syslog(0, listenlog, "can't build namespace %s: %r\n", ns);
+					dolisten(dir, ctl, serv, prog, &childs);
 					close(ctl);
 				}
 			default:
@@ -299,6 +308,8 @@
 				continue;
 			if(strncmp(nm, proto, nlen) != 0)
 				continue;
+			if(strstr(nm + nlen, ".namespace") != nil)
+				continue;
 			addannounce(nm + nlen);
 		}
 		free(db);
@@ -329,15 +340,10 @@
 }

 void
-dolisten(char *dir, int ctl, char *srvdir, char *port, long *pchilds)
+dolisten(char *dir, int ctl, char *serv, char *prog, long *pchilds)
 {
 	char ndir[40], wbuf[64];
-	char prog[Maxpath], serv[Maxserv];
 	int nctl, data, wfd, nowait;
-
-	procsetname("%s %s!%s!%s", dir, proto, addr, port);
-	snprint(serv, sizeof serv, "%s%s", proto, port);
-	snprint(prog, sizeof prog, "%s/%s", srvdir, serv);
 	
 	wfd = -1;
 	nowait = RFNOWAIT;
--- a/sys/src/libauth/newns.c
+++ b/sys/src/libauth/newns.c
@@ -14,8 +14,8 @@
 static int	setenv(char*, char*);
 static char	*expandarg(char*, char*);
 static int	splitargs(char*, char*[], char*, int);
-static int	nsfile(char*, Biobuf *, AuthRpc *);
-static int	nsop(char*, int, char*[], AuthRpc*);
+static int	nsfile(char*, Biobuf *, AuthRpc *, int);
+static int	nsop(char*, int, char*[], AuthRpc*, int);
 static int	catch(void*, char*);

 int newnsdebug;
@@ -35,7 +35,7 @@
 {
 	Biobuf *b;
 	char home[4*ANAMELEN];
-	int afd, cdroot;
+	int afd, cdroot, dfd;
 	char *path;
 	AuthRpc *rpc;

@@ -51,6 +51,10 @@
 	}
 	/* rpc != nil iff afd >= 0 */

+	dfd = open("#c/drivers", OWRITE|OCEXEC);
+	if(dfd < 0 && newnsdebug)
+		fprint(2, "open #c/drivers: %r\n");
+
 	if(file == nil){
 		if(!newns){
 			werrstr("no namespace file specified");
@@ -70,7 +74,8 @@
 		setenv("home", home);
 	}

-	cdroot = nsfile(newns ? "newns" : "addns", b, rpc);
+	cdroot = nsfile(newns ? "newns" : "addns", b, rpc, dfd);
+	close(dfd);
 	Bterm(b);
 	freecloserpc(rpc);

@@ -87,7 +92,7 @@
 }

 static int
-nsfile(char *fn, Biobuf *b, AuthRpc *rpc)
+nsfile(char *fn, Biobuf *b, AuthRpc *rpc, int dfd)
 {
 	int argc;
 	char *cmd, *argv[NARG+1], argbuf[MAXARG*NARG];
@@ -103,7 +108,7 @@
 			continue;
 		argc = splitargs(cmd, argv, argbuf, NARG);
 		if(argc)
-			cdroot |= nsop(fn, argc, argv, rpc);
+			cdroot |= nsop(fn, argc, argv, rpc, dfd);
 	}
 	atnotify(catch, 0);
 	return cdroot;
@@ -143,7 +148,7 @@
 }

 static int
-nsop(char *fn, int argc, char *argv[], AuthRpc *rpc)
+nsop(char *fn, int argc, char *argv[], AuthRpc *rpc, int dfd)
 {
 	char *argv0;
 	ulong flags;
@@ -181,7 +186,7 @@
 		b = Bopen(argv[0], OREAD|OCEXEC);
 		if(b == nil)
 			return 0;
-		cdroot |= nsfile(fn, b, rpc);
+		cdroot |= nsfile(fn, b, rpc, dfd);
 		Bterm(b);
 	}else if(strcmp(argv0, "clear") == 0 && argc == 0){
 		rfork(RFCNAMEG);
@@ -212,6 +217,14 @@
 	}else if(strcmp(argv0, "cd") == 0 && argc == 1){
 		if(chdir(argv[0]) == 0 && *argv[0] == '/')
 			cdroot = 1;
+	}else if(argc >= 1 && (strcmp(argv0, "permit") == 0 || strcmp(argv0, "eject") == 0)){
+		//We should not silently fail if we can not honor a permit/eject
+		//due to the parent namespace missing #c/drivers.
+		if(dfd <= 0)
+			sysfatal("%s requested, but could not open #c/drivers", argv0);
+		for(i=0; i < argc; i++)
+			if(fprint(dfd, "%s %s\n", argv0, argv[i]) < 0 && newnsdebug)
+				fprint(2, "%s: %s %s %r\n", fn, argv0, argv[i]);
 	}
 	return cdroot;
 }

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-23  5:42                   ` Jacob Moody
@ 2022-05-23 17:06                     ` cinap_lenrek
  2022-05-23 17:37                       ` Jacob Moody
  0 siblings, 1 reply; 28+ messages in thread
From: cinap_lenrek @ 2022-05-23 17:06 UTC (permalink / raw)
  To: 9front

try to preserve that the ns file looks like shell script.

avoid "eject", as that is already a existing command that does
something completely different.

i dont like "permit" as it is too generic and not clear what it
operates on.

in any case, think of these operations as something that we should
also provide a shell wrapper for and it lives in the /bin namespace.

maybe should be a single command limilar to chmod/chgrp but operating
on the kernel device map?

like:

devmask -X		# "eject" X device
devmask ABCDEF	# set ABCDEF as allowed

any better ideas?

--
cinap

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-23 17:06                     ` cinap_lenrek
@ 2022-05-23 17:37                       ` Jacob Moody
  2022-05-25 19:03                         ` Jacob Moody
  0 siblings, 1 reply; 28+ messages in thread
From: Jacob Moody @ 2022-05-23 17:37 UTC (permalink / raw)
  To: 9front

On 5/23/22 11:06, cinap_lenrek@felloff.net wrote:
> try to preserve that the ns file looks like shell script.
> 
> avoid "eject", as that is already a existing command that does
> something completely different.
> 
> i dont like "permit" as it is too generic and not clear what it
> operates on.
> 
> in any case, think of these operations as something that we should
> also provide a shell wrapper for and it lives in the /bin namespace.
> 
> maybe should be a single command limilar to chmod/chgrp but operating
> on the kernel device map?
> 
> like:
> 
> devmask -X		# "eject" X device
> devmask ABCDEF	# set ABCDEF as allowed
> 
> any better ideas?

This all makes sense.
There was some followup discussion for this on irc that I want to
make sure is captured here:

The command written to /dev/drivers will change to be one of:
chdev DEVS
chdev -DEVS

with them acting like the current 'permit' and 'eject' commands respectively.
This will be given a matching chdev(1) rc script, and the ns file command
will be updated to match the chdev(1) interface.


thanks,
moody

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-23 17:37                       ` Jacob Moody
@ 2022-05-25 19:03                         ` Jacob Moody
  2022-05-25 20:53                           ` hiro
  2022-05-27  1:11                           ` Lyndon Nerenberg (VE7TFX/VE6BBM)
  0 siblings, 2 replies; 28+ messages in thread
From: Jacob Moody @ 2022-05-25 19:03 UTC (permalink / raw)
  To: 9front

On 5/23/22 11:37, Jacob Moody wrote:
> There was some followup discussion for this on irc that I want to
> make sure is captured here:
> 
> The command written to /dev/drivers will change to be one of:
> chdev DEVS
> chdev -DEVS

I am having some second thoughts about this. While this syntax does replicate
chmod, I am not entirely convinced that is what we want. When
being added to /sys/src/libauth/newns:/^nsop/ you have to special case around
the ARGBEGIN block. Combine that with the fact that this decision would lock
chdev(1) from ever accepting it's own flags, it felt like this was
introducing special case cruft.
Another consideration was what the syntax would be for removing all drivers,
currently it would just be chdev '' or something similar. Which is fine but
fat fingering that would be an annoying way to kill your current namespace.
These caveats may be entirely acceptable, but I was bothered enough to attempt to find another way.

Instead, I thought perhaps a rune prefix might be best. So you end up with:

chdev ∅
for removing all drivers.

chdev ∉DEVS
For removing just the specified drivers.

chdev ∈DEVS
For removing access to all but the specified drivers.

If a prefix is not given, ∈ is assumed.

I did implement this idea, to field it for quirks,
but wanted to get some feedback before redoing man page changes.

The downsides I see here are:

1. Deviation from chmod
2. This steals three runes that could have otherwise been used for drivers
3. ∉ is a bit more involved to type then '-'

With that said, I do much prefer this interface. Another option may be
to use rune prefixes, but also provide flag synonyms to the prefixes in
chdev(1).

There is also an unrelated quirk I stumbled across when reviewing changes.
As is currently, devshr checks for if the noattach has been set before
allowing a process to post a fd. I have changed this to check for access to devmnt/'#M',
which is what replicates the 'can mount' knob that RFNOMNT tweaks. This seems fine to me,
but should we also replicate this check over in to devsrv for consistency?

Thanks,
moody

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-25 19:03                         ` Jacob Moody
@ 2022-05-25 20:53                           ` hiro
  2022-05-25 21:20                             ` Jacob Moody
  2022-05-26  3:13                             ` ori
  2022-05-27  1:11                           ` Lyndon Nerenberg (VE7TFX/VE6BBM)
  1 sibling, 2 replies; 28+ messages in thread
From: hiro @ 2022-05-25 20:53 UTC (permalink / raw)
  To: 9front

hating those runes.

did you consider: chdev REGEXP
could be the expanded version that allows driverA|driverB|driver[CDE]
could also allow ! or -v for reversing the meaning.

alternatively, why not use 0 instead of ∅ and '!' instead of ∉ ?

On 5/25/22, Jacob Moody <moody@mail.posixcafe.org> wrote:
> On 5/23/22 11:37, Jacob Moody wrote:
>> There was some followup discussion for this on irc that I want to
>> make sure is captured here:
>>
>> The command written to /dev/drivers will change to be one of:
>> chdev DEVS
>> chdev -DEVS
>
> I am having some second thoughts about this. While this syntax does
> replicate
> chmod, I am not entirely convinced that is what we want. When
> being added to /sys/src/libauth/newns:/^nsop/ you have to special case
> around
> the ARGBEGIN block. Combine that with the fact that this decision would
> lock
> chdev(1) from ever accepting it's own flags, it felt like this was
> introducing special case cruft.
> Another consideration was what the syntax would be for removing all
> drivers,
> currently it would just be chdev '' or something similar. Which is fine but
> fat fingering that would be an annoying way to kill your current namespace.
> These caveats may be entirely acceptable, but I was bothered enough to
> attempt to find another way.
>
> Instead, I thought perhaps a rune prefix might be best. So you end up with:
>
> chdev ∅
> for removing all drivers.
>
> chdev ∉DEVS
> For removing just the specified drivers.
>
> chdev ∈DEVS
> For removing access to all but the specified drivers.
>
> If a prefix is not given, ∈ is assumed.
>
> I did implement this idea, to field it for quirks,
> but wanted to get some feedback before redoing man page changes.
>
> The downsides I see here are:
>
> 1. Deviation from chmod
> 2. This steals three runes that could have otherwise been used for drivers
> 3. ∉ is a bit more involved to type then '-'
>
> With that said, I do much prefer this interface. Another option may be
> to use rune prefixes, but also provide flag synonyms to the prefixes in
> chdev(1).
>
> There is also an unrelated quirk I stumbled across when reviewing changes.
> As is currently, devshr checks for if the noattach has been set before
> allowing a process to post a fd. I have changed this to check for access to
> devmnt/'#M',
> which is what replicates the 'can mount' knob that RFNOMNT tweaks. This
> seems fine to me,
> but should we also replicate this check over in to devsrv for consistency?
>
> Thanks,
> moody
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-25 20:53                           ` hiro
@ 2022-05-25 21:20                             ` Jacob Moody
  2022-05-26  5:55                               ` Jacob Moody
  2022-05-26  3:13                             ` ori
  1 sibling, 1 reply; 28+ messages in thread
From: Jacob Moody @ 2022-05-25 21:20 UTC (permalink / raw)
  To: 9front

On 5/25/22 14:53, hiro wrote:
> hating those runes.

Fair enough, I'll toss them.

> did you consider: chdev REGEXP
> could be the expanded version that allows driverA|driverB|driver[CDE]
> could also allow ! or -v for reversing the meaning.

I have a mild preference to keep it just a string of characters, but
I could be convinced otherwise. I like that the characters provided
match how they are accessed like '#A'. The drivers do keep their
actual full name stored on the struct, so all the context is there.
Might make 'chdev' lines in namespace files a bit less opaque.

> alternatively, why not use 0 instead of ∅ and '!' instead of ∉ ?
I like this. My only gripe is wanting some sort of implicit prefix
replacement for ∈. But I can get over that.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-25 20:53                           ` hiro
  2022-05-25 21:20                             ` Jacob Moody
@ 2022-05-26  3:13                             ` ori
  1 sibling, 0 replies; 28+ messages in thread
From: ori @ 2022-05-26  3:13 UTC (permalink / raw)
  To: 9front

Quoth hiro <23hiro@gmail.com>:
> did you consider: chdev REGEXP
> could be the expanded version that allows driverA|driverB|driver[CDE]

are you trolling?

driver names are single characters, and we don't have that many.
a regex is an insane amount of complexity for almost no benefit.



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-25 21:20                             ` Jacob Moody
@ 2022-05-26  5:55                               ` Jacob Moody
  2022-05-26 23:36                                 ` unobe
  0 siblings, 1 reply; 28+ messages in thread
From: Jacob Moody @ 2022-05-26  5:55 UTC (permalink / raw)
  To: 9front

Another iteration.
To state all of what this does:

A process can remove access to a driver through writing

chdev [ 0! ] devmask

to /dev/drivers. This is tied to the processes namegroup,
and is inherited on both RFCNAMEG and RFNAMEG. In order to provide
all the knobs RFNOMNT does, blocking 'M' is interpreted to block all mounting.

Support for this new operation has been added to namespace files,
along with a matching chdev(1). /lib/namespace.ftp has been updated
to use chdev.

aux/listen now allows individual namespace files to be used
per service. This namespace is sourced one per listener. Each
connection receives a copy of the namespace. A example
/rc/bin/service/!tcp80.namespace is provided which builds
an isolated webroot. In order to accomplish this without
bringing in most of /root, a /rc folder was added to #/.


thanks,
moody

----
diff 6fbb1acc8fa0b6655b14e8c46240a4a8d2d8c672 uncommitted
--- a/lib/namespace
+++ b/lib/namespace
@@ -22,6 +22,7 @@

 # standard bin
 bind /$cputype/bin /bin
+bind $rootdir'/rc' /rc
 bind -a /rc/bin /bin

 # internal networks
--- a/lib/namespace.ftp
+++ b/lib/namespace.ftp
@@ -8,5 +8,6 @@
 # bind a personal incoming directory below incoming
 bind -c /usr/none/incoming /usr/web/incoming/none

+chdev |MedIa/
 # this cuts off everything not mounted below /usr/web
 bind /usr/web /
--- /tmp/diff100000711164
+++ b/rc/bin/chdev
@@ -1,0 +1,3 @@
+#!/bin/rc
+
+echo chdev $* >> '#c/drivers'
--- /tmp/diff100000711167
+++ b/rc/bin/service/!tcp80.namespace
@@ -1,0 +1,24 @@
+mount -aC #s/boot /root $rootspec
+
+# kernel devices
+bind #c /dev
+bind #d /fd
+bind -c #e /env
+bind #p /proc
+bind -a #l /net
+bind -a #I /net
+
+bind /root/$cputype/bin /bin
+bind /root/rc /rc
+bind -a /rc/bin /bin
+
+chdev Mcde|pslI/
+
+# grab just our webroot
+bind /root/usr/web /srv
+
+# or bind in the actual root
+# bind -a /root /
+
+unmount /root
+chdev !Ms
--- /tmp/diff100000711170
+++ b/sys/man/1/chdev
@@ -1,0 +1,93 @@
+.TH CHDEV 1
+.SH NAME
+chdev \- change kernel driver access
+.SH SYNOPSIS
+.B chdev
+[ 0! ] \f2devmask\fP...
+.SH DESCRIPTION
+.I Chdev
+modifies access to kernel drivers for the current
+process and processes within the same name group
+(see
+.IR fork (2)).
+Access is defined as the ability for a process
+to walk files and directories served by the driver
+through its location within '#'. Existing binds
+of drivers are left unaffected.
+.PP
+Access may only be removed; after a specific
+driver is ejected no further operations can
+permit access again. Access is inherited by
+all children of the name group, regardless if the
+child has elected to receive a clean namespace.
+.PP
+.IR Devmask
+is a string of driver characters. The default
+behavior is to block access to all but the listed
+drivers. A prefix of '!' inverts this operation,
+retaining access to all but the specified drivers.
+A
+.I devmask
+of '0' removes access to all drivers.
+.PP
+Access to some drivers is tied to other
+related process capabilities:
+.TP
+\f2mnt\fP(3)
+.IP
+Ability to perform the
+.IR mount (2)
+system call, and
+to post new services to
+.IR shr (3).
+.TP
+\f2pipe\fP(3)
+.IP
+Ability to perform the
+.IR pipe (2)
+system call.
+.SH EXAMPLES
+Permit access to only
+.IR draw (3),
+.IR rtc (3),
+.IR fs (3),
+and
+.IR srv (3):
+.IP
+.EX
+chdev irks
+.EE
+.PP
+Remove access to
+.IR cons (3),
+.IR rtc (3),
+.IR audio (3),
+and
+.IR proc (3):
+.IP
+.EX
+chdev !crAp
+.EE
+.PP
+Create a
+.IR pipe (3)
+then remove the ability to create more:
+.IP
+.EX
+bind '#|' /n/pipe
+chdev '!|'
+.EE
+.SH DIAGNOSTICS
+.I Chdev
+is implemented through writes to
+.BR /dev/drivers ,
+served by
+.IR cons (3).
+.SH SOURCE
+.B /rc/bin/chdev
+.SH "SEE ALSO"
+.B /dev/drivers
+for a list of current drivers.
+.PP
+.IR intro (3),
+.IR cons (3)
--- a/sys/man/3/cons
+++ b/sys/man/3/cons
@@ -90,10 +90,36 @@
 .PP
 The
 .B drivers
-file contains, one per line, a listing of the drivers configured in the kernel, in the format
+file contains, one per line, a listing of available kernel drivers, in the format
 .IP
 .EX
 #c cons
+.EE
+.PP
+A process can restrict access to kernel drivers through writing messages to the
+.B drivers
+file. A message in the format of
+.IP
+.EX
+chdev [ !0 ] \f2devmask\fP
+.EE
+.PP
+will restrict access to just the provided driver characters listed in
+.IR devmask .
+If \f2devmask\fP is prefixed with a '!', a namespace instead retains access to all
+but the specified drivers. A
+.I devmask
+of '0' removes access to all devices. Removing access to
+.IR mnt (3)
+additionally prevents new mounts into the current namespace.
+The following removes access to all drivers except
+.IR draw (3),
+.IR cons (3),
+and
+.IR fs (3):
+.IP
+.EX
+chdev ick
 .EE
 .PP
 The
--- a/sys/man/3/root
+++ b/sys/man/3/root
@@ -10,6 +10,7 @@
 .B /net
 .B /net.alt
 .B /proc
+.B /rc
 .B /root
 .B /srv
 .fi
--- a/sys/man/6/namespace
+++ b/sys/man/6/namespace
@@ -59,6 +59,17 @@
 .I new
 is missing.
 .TP
+.BR chdev \ [ !0 "] \fIdevmask
+.I Devmask
+defines a string a driver characters to restrict
+the current namespace to. Existing binds
+of drivers are left unaffected. If
+.I devmask
+is prefixed with a '!', the namespace instead retains
+access to all but the specified drivers. A
+.I devmask
+of '0' removes all drivers.
+.TP
 .BR clear
 Clear the name space with
 .BR rfork(RFCNAMEG) .
@@ -80,4 +91,5 @@
 .SH "SEE ALSO"
 .IR bind (1),
 .IR namespace (4),
-.IR init (8)
+.IR init (8),
+.IR chdev (1)
--- a/sys/man/8/listen
+++ b/sys/man/8/listen
@@ -96,6 +96,14 @@
 an inbound call on the TCP network for port 565 executes service
 .BR tcp565 .
 .PP
+Services may have individual
+.IR namespace (6)
+files specified within
+.IR srvdir .
+If provided, the namespace is used as the parent for each connection
+to the corresponding service. Namespace files are found by appending a .namespace
+suffix to the service name.
+.PP
 At least the following services are available in
 .BR /bin/service .
 .TF \ tcp0000
--- a/sys/src/9/boot/boot.c
+++ b/sys/src/9/boot/boot.c
@@ -25,6 +25,7 @@
 	buf[1+read(open("/env/cputype", OREAD|OCEXEC), buf+1, sizeof buf - 6)] = '\0';
 	strcat(buf, bin);
 	bind(buf, bin, MAFTER);
+	bind("/root/rc", "/rc", MREPL);
 	bind("/rc/bin", bin, MAFTER);

 	exec("/bin/bootrc", argv);
--- a/sys/src/9/port/chan.c
+++ b/sys/src/9/port/chan.c
@@ -1272,7 +1272,7 @@
 Chan*
 namec(char *aname, int amode, int omode, ulong perm)
 {
-	int len, n, t, nomount;
+	int len, n, t, nomount, devunmount;
 	Chan *c;
 	Chan *volatile cnew;
 	Path *volatile path;
@@ -1292,6 +1292,24 @@
 	name = aname;

 	/*
+	 * When unmounting, the name parameter must be accessed
+	 * using Aopen in order to get the real chan from
+	 * something like /srv/cs or /fd/0. However when sandboxing,
+	 * unmounting a sharp from a union is a valid operation even
+	 * if the device is blocked.
+	 */
+	devunmount = 0;
+	if(amode == Aunmount){
+		/*
+		 * Doing any walks down the device could leak information
+		 * about the existence of files.
+		 */
+		if(name[0] == '#' && utflen(name) == 2)
+			devunmount = 1;
+		amode = Aopen;
+	}
+
+	/*
 	 * Find the starting off point (the current slash, the root of
 	 * a device tree, or the current dot) as well as the name to
 	 * evaluate starting there.
@@ -1313,24 +1331,13 @@
 			up->genbuf[n++] = *name++;
 		}
 		up->genbuf[n] = '\0';
-		/*
-		 *  noattach is sandboxing.
-		 *
-		 *  the OK exceptions are:
-		 *	|  it only gives access to pipes you create
-		 *	d  this process's file descriptors
-		 *	e  this process's environment
-		 *  the iffy exceptions are:
-		 *	c  time and pid, but also cons and consctl
-		 *	p  control of your own processes (and unfortunately
-		 *	   any others left unprotected)
-		 */
 		n = chartorune(&r, up->genbuf+1)+1;
-		if(up->pgrp->noattach && utfrune("|decp", r)==nil)
-			error(Enoattach);
 		t = devno(r, 1);
 		if(t == -1)
 			error(Ebadsharp);
+		if(!devunmount && !devallowed(up->pgrp, r))
+			error(Enoattach);
+
 		c = devtab[t]->attach(up->genbuf+n);
 		break;

--- a/sys/src/9/port/dev.c
+++ b/sys/src/9/port/dev.c
@@ -31,6 +31,74 @@
 }

 void
+devmask(Pgrp *pgrp, char *devs)
+{
+	int i, t, w;
+	int invert;
+	char *p;
+	Rune r;
+	u64int mask[nelem(pgrp->notallowed)];
+
+	
+	invert = 1;
+	switch(*devs){
+	case '!':
+		memset(mask, 0, sizeof mask);
+		devs++;
+		invert--;
+		break;
+	case '0':
+		devs = "";
+	default:
+		memset(mask, 0xFF, sizeof mask);
+		break;
+	}		
+
+	w = sizeof mask[0] * 8;
+	for(p = devs; *p != '\0';){
+		p += chartorune(&r, p);
+		t = devno(r, 1);
+		if(t == -1)
+			continue;
+		if(invert)
+			mask[t/w] &= ~(1<<t%w);
+		else
+			mask[t/w] |= 1<<t%w;
+	}
+
+	wlock(&pgrp->ns);
+	for(i=0; i < nelem(pgrp->notallowed); i++)
+		pgrp->notallowed[i] |= mask[i];
+	wunlock(&pgrp->ns);
+}
+
+int
+devallowed(Pgrp *pgrp, int r)
+{
+	int t, w, b;
+
+	t = devno(r, 1);
+	if(t == -1)
+		return 0;
+
+	w = sizeof(u64int) * 8;
+	rlock(&pgrp->ns);
+	b = !(pgrp->notallowed[t/w] & 1<<t%w);
+	runlock(&pgrp->ns);
+	return b;
+}
+
+int
+canmount(Pgrp *pgrp)
+{
+	/*
+	 * Devmnt is not usable directly from user procs, so
+	 * having it removed is interpreted to block any mounts.
+	 */
+	return devallowed(pgrp, 'M');
+}
+
+void
 devdir(Chan *c, Qid qid, char *n, vlong length, char *user, long perm, Dir *db)
 {
 	db->name = n;
--- a/sys/src/9/port/devcons.c
+++ b/sys/src/9/port/devcons.c
@@ -39,6 +39,16 @@
 	CMrdb,		"rdb",		0,
 };

+enum
+{
+	CMchdev,
+};
+
+Cmdtab drivermsg[] =
+{
+	CMchdev,	"chdev",	2,
+};
+
 void
 printinit(void)
 {
@@ -332,7 +342,7 @@
 	"cons",		{Qcons},	0,		0660,
 	"consctl",	{Qconsctl},	0,		0220,
 	"cputime",	{Qcputime},	6*NUMSIZE,	0444,
-	"drivers",	{Qdrivers},	0,		0444,
+	"drivers",	{Qdrivers},	0,		0666,
 	"hostdomain",	{Qhostdomain},	DOMLEN,		0664,
 	"hostowner",	{Qhostowner},	0,		0664,
 	"kmesg",	{Qkmesg},	0,		0440,
@@ -583,9 +593,15 @@
 	case Qdrivers:
 		b = smalloc(READSTR);
 		k = 0;
-		for(i = 0; devtab[i] != nil; i++)
+		
+		rlock(&up->pgrp->ns);
+		for(i = 0; devtab[i] != nil; i++){
+			if(up->pgrp->notallowed[i/(sizeof(u64int)*8)] & 1<<i%(sizeof(u64int)*8))
+				continue;
 			k += snprint(b+k, READSTR-k, "#%C %s\n",
 				devtab[i]->dc, devtab[i]->name);
+		}
+		runlock(&up->pgrp->ns);
 		if(waserror()){
 			free(b);
 			nexterror();
@@ -674,6 +690,24 @@

 	case Qconfig:
 		error(Eperm);
+		break;
+
+	case Qdrivers:
+		cb = parsecmd(a, n);
+		if(waserror()) {
+			free(cb);
+			nexterror();
+		}
+
+		ct = lookupcmd(cb, drivermsg, nelem(drivermsg));
+		if(ct == nil)
+			error(Ebadarg);
+		if(ct->index != CMchdev)
+			error(Ebadarg);
+
+		devmask(up->pgrp, cb->f[1]);
+		poperror();
+		free(cb);
 		break;

 	case Qreboot:
--- a/sys/src/9/port/devroot.c
+++ b/sys/src/9/port/devroot.c
@@ -105,6 +105,7 @@
 	addrootdir("net");
 	addrootdir("net.alt");
 	addrootdir("proc");
+	addrootdir("rc");
 	addrootdir("root");
 	addrootdir("srv");
 	addrootdir("shr");
--- a/sys/src/9/port/devshr.c
+++ b/sys/src/9/port/devshr.c
@@ -464,7 +464,7 @@
 		cclose(c);
 		return nc;	
 	case Qcroot:
-		if(up->pgrp->noattach)
+		if(!canmount(up->pgrp))
 			error(Enoattach);
 		if((perm & DMDIR) == 0 || mode != OREAD)
 			error(Eperm);
@@ -498,7 +498,7 @@
 		sch->shr = shr;
 		break;
 	case Qcshr:
-		if(up->pgrp->noattach)
+		if(!canmount(up->pgrp))
 			error(Enoattach);
 		if((perm & DMDIR) != 0 || mode != OWRITE)
 			error(Eperm);
@@ -731,7 +731,7 @@
 	Mhead *h;
 	Mount *m;

-	if(up->pgrp->noattach)
+	if(!canmount(up->pgrp))
 		error(Enoattach);
 	sch = tosch(c);
 	if(sch->level != Qcmpt)
--- a/sys/src/9/port/mkdevc
+++ b/sys/src/9/port/mkdevc
@@ -78,6 +78,9 @@
 	if(ARGC < 2)
 		exit "usage"

+	if(ndev >= 256)
+		exit "device count will overflow Pgrp.notallowed"
+
 	printf "#include \"u.h\"\n";
 	printf "#include \"../port/lib.h\"\n";
 	printf "#include \"mem.h\"\n";
--- a/sys/src/9/port/portdat.h
+++ b/sys/src/9/port/portdat.h
@@ -121,6 +121,7 @@
 	Amount,				/* to be mounted or mounted upon */
 	Acreate,			/* is to be created */
 	Aremove,			/* will be removed by caller */
+	Aunmount,			/* unmount arg[0] */

 	COPEN	= 0x0001,		/* for i/o */
 	CMSG	= 0x0002,		/* the message channel for a mount */
@@ -484,7 +485,7 @@
 {
 	Ref;
 	RWlock	ns;			/* Namespace n read/one write lock */
-	int	noattach;
+	u64int	notallowed[4];		/* Room for 256 devices */
 	Mhead	*mnthash[MNTHASH];
 };

--- a/sys/src/9/port/portfns.h
+++ b/sys/src/9/port/portfns.h
@@ -413,6 +413,9 @@
 ushort		nhgets(void*);
 ulong		µs(void);
 long		lcycles(void);
+void		devmask(Pgrp*,char*);
+int		devallowed(Pgrp*, int);
+int		canmount(Pgrp*);

 #pragma varargck argpos iprint	1
 #pragma	varargck argpos	panic	1
--- a/sys/src/9/port/sysfile.c
+++ b/sys/src/9/port/sysfile.c
@@ -1048,7 +1048,7 @@
 			nexterror();
 		}

-		if(up->pgrp->noattach)
+		if(!canmount(up->pgrp))
 			error(Enoattach);

 		ac = nil;
@@ -1160,14 +1160,8 @@
 		nexterror();
 	}
 	if(name != nil) {
-		/*
-		 * This has to be namec(..., Aopen, ...) because
-		 * if arg[0] is something like /srv/cs or /fd/0,
-		 * opening it is the only way to get at the real
-		 * Chan underneath.
-		 */
 		validaddr((uintptr)name, 1, 0);
-		cmounted = namec(name, Aopen, OREAD, 0);
+		cmounted = namec(name, Aunmount, OREAD, 0);
 	}
 	cunmount(cmount, cmounted);
 	poperror();
--- a/sys/src/9/port/sysproc.c
+++ b/sys/src/9/port/sysproc.c
@@ -34,6 +34,7 @@
 	Egrp *oeg;
 	ulong pid, flag;
 	Mach *wm;
+	char *devs;

 	flag = va_arg(list, ulong);
 	/* Check flags before we commit */
@@ -44,6 +45,11 @@
 	if((flag & (RFENVG|RFCENVG)) == (RFENVG|RFCENVG))
 		error(Ebadarg);

+	/*
+	 * Code using RFNOMNT expects to block all but
+	 * the following devices.
+	 */
+	devs = "|decp";
 	if((flag&RFPROC) == 0) {
 		if(flag & (RFMEM|RFNOWAIT))
 			error(Ebadarg);
@@ -60,12 +66,12 @@
 			up->pgrp = newpgrp();
 			if(flag & RFNAMEG)
 				pgrpcpy(up->pgrp, opg);
-			/* inherit noattach */
-			up->pgrp->noattach = opg->noattach;
+			/* inherit notallowed */
+			memmove(up->pgrp->notallowed, opg->notallowed, sizeof up->pgrp->notallowed);
 			closepgrp(opg);
 		}
 		if(flag & RFNOMNT)
-			up->pgrp->noattach = 1;
+			devmask(up->pgrp, devs);
 		if(flag & RFREND) {
 			org = up->rgrp;
 			up->rgrp = newrgrp();
@@ -177,8 +183,8 @@
 		p->pgrp = newpgrp();
 		if(flag & RFNAMEG)
 			pgrpcpy(p->pgrp, up->pgrp);
-		/* inherit noattach */
-		p->pgrp->noattach = up->pgrp->noattach;
+		/* inherit notallowed */
+		memmove(p->pgrp->notallowed, up->pgrp->notallowed, sizeof p->pgrp->notallowed);
 	}
 	else {
 		p->pgrp = up->pgrp;
@@ -185,7 +191,7 @@
 		incref(p->pgrp);
 	}
 	if(flag & RFNOMNT)
-		p->pgrp->noattach = 1;
+		devmask(p->pgrp, devs);

 	if(flag & RFREND)
 		p->rgrp = newrgrp();
--- a/sys/src/cmd/aux/listen.c
+++ b/sys/src/cmd/aux/listen.c
@@ -136,6 +136,7 @@
 {
 	int ctl, pid, start;
 	char dir[40], err[128], ds[128];
+	char prog[Maxpath], serv[Maxserv], ns[Maxpath];
 	long childs;
 	Announce *a;
 	Waitmsg *wm;
@@ -178,6 +179,10 @@
 			sleep((pid*10)%200);

 			snprint(ds, sizeof ds, "%s!%s!%s", protodir, addr, a->a);
+			snprint(serv, sizeof serv, "%s%s", proto, a->a);
+			snprint(prog, sizeof prog, "%s/%s", srvdir, serv);
+			snprint(ns, sizeof ns, "%s.namespace", prog);
+
 			whined = a->whined;

 			/* a process per service */
@@ -201,7 +206,11 @@
 						else
 							exits("ctl");
 					}
-					dolisten(dir, ctl, srvdir, a->a, &childs);
+					procsetname("%s %s", dir, ds);
+					if(!trusted)
+						if(newns("none", ns) < 0)
+							syslog(0, listenlog, "can't build namespace %s: %r\n", ns);
+					dolisten(dir, ctl, serv, prog, &childs);
 					close(ctl);
 				}
 			default:
@@ -299,6 +308,8 @@
 				continue;
 			if(strncmp(nm, proto, nlen) != 0)
 				continue;
+			if(strstr(nm + nlen, ".namespace") != nil)
+				continue;
 			addannounce(nm + nlen);
 		}
 		free(db);
@@ -329,15 +340,10 @@
 }

 void
-dolisten(char *dir, int ctl, char *srvdir, char *port, long *pchilds)
+dolisten(char *dir, int ctl, char *serv, char *prog, long *pchilds)
 {
 	char ndir[40], wbuf[64];
-	char prog[Maxpath], serv[Maxserv];
 	int nctl, data, wfd, nowait;
-
-	procsetname("%s %s!%s!%s", dir, proto, addr, port);
-	snprint(serv, sizeof serv, "%s%s", proto, port);
-	snprint(prog, sizeof prog, "%s/%s", srvdir, serv);
 	
 	wfd = -1;
 	nowait = RFNOWAIT;
--- a/sys/src/libauth/newns.c
+++ b/sys/src/libauth/newns.c
@@ -14,8 +14,8 @@
 static int	setenv(char*, char*);
 static char	*expandarg(char*, char*);
 static int	splitargs(char*, char*[], char*, int);
-static int	nsfile(char*, Biobuf *, AuthRpc *);
-static int	nsop(char*, int, char*[], AuthRpc*);
+static int	nsfile(char*, Biobuf *, AuthRpc *, int);
+static int	nsop(char*, int, char*[], AuthRpc*, int);
 static int	catch(void*, char*);

 int newnsdebug;
@@ -35,7 +35,7 @@
 {
 	Biobuf *b;
 	char home[4*ANAMELEN];
-	int afd, cdroot;
+	int afd, cdroot, dfd;
 	char *path;
 	AuthRpc *rpc;

@@ -51,6 +51,10 @@
 	}
 	/* rpc != nil iff afd >= 0 */

+	dfd = open("#c/drivers", OWRITE|OCEXEC);
+	if(dfd < 0 && newnsdebug)
+		fprint(2, "open #c/drivers: %r\n");
+
 	if(file == nil){
 		if(!newns){
 			werrstr("no namespace file specified");
@@ -70,7 +74,8 @@
 		setenv("home", home);
 	}

-	cdroot = nsfile(newns ? "newns" : "addns", b, rpc);
+	cdroot = nsfile(newns ? "newns" : "addns", b, rpc, dfd);
+	close(dfd);
 	Bterm(b);
 	freecloserpc(rpc);

@@ -87,7 +92,7 @@
 }

 static int
-nsfile(char *fn, Biobuf *b, AuthRpc *rpc)
+nsfile(char *fn, Biobuf *b, AuthRpc *rpc, int dfd)
 {
 	int argc;
 	char *cmd, *argv[NARG+1], argbuf[MAXARG*NARG];
@@ -103,7 +108,7 @@
 			continue;
 		argc = splitargs(cmd, argv, argbuf, NARG);
 		if(argc)
-			cdroot |= nsop(fn, argc, argv, rpc);
+			cdroot |= nsop(fn, argc, argv, rpc, dfd);
 	}
 	atnotify(catch, 0);
 	return cdroot;
@@ -143,7 +148,7 @@
 }

 static int
-nsop(char *fn, int argc, char *argv[], AuthRpc *rpc)
+nsop(char *fn, int argc, char *argv[], AuthRpc *rpc, int dfd)
 {
 	char *argv0;
 	ulong flags;
@@ -181,7 +186,7 @@
 		b = Bopen(argv[0], OREAD|OCEXEC);
 		if(b == nil)
 			return 0;
-		cdroot |= nsfile(fn, b, rpc);
+		cdroot |= nsfile(fn, b, rpc, dfd);
 		Bterm(b);
 	}else if(strcmp(argv0, "clear") == 0 && argc == 0){
 		rfork(RFCNAMEG);
@@ -212,6 +217,13 @@
 	}else if(strcmp(argv0, "cd") == 0 && argc == 1){
 		if(chdir(argv[0]) == 0 && *argv[0] == '/')
 			cdroot = 1;
+	}else if(strcmp(argv0, "chdev") == 0 && argc == 1){
+		//We should not silently fail if we can not honor a chdev
+		//due to the parent namespace missing #c/drivers.
+		if(dfd <= 0)
+			sysfatal("chdev requested, but could not open #c/drivers");
+		if(fprint(dfd, "chdev %s", argv[0]) < 0 && newnsdebug)
+			fprint(2, "%s: chdev %s %r\n", fn, argv[0]);
 	}
 	return cdroot;
 }

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-26  5:55                               ` Jacob Moody
@ 2022-05-26 23:36                                 ` unobe
  2022-05-27  0:33                                   ` Jacob Moody
  0 siblings, 1 reply; 28+ messages in thread
From: unobe @ 2022-05-26 23:36 UTC (permalink / raw)
  To: 9front

Great work on this moody.  I have some (hopefully helpful) questions
below.


Quoth Jacob Moody <moody@mail.posixcafe.org>:
> Another iteration.
> To state all of what this does:
> 
> A process can remove access to a driver through writing
> 
> chdev [ 0! ] devmask
> 
> to /dev/drivers. This is tied to the processes namegroup,
> and is inherited on both RFCNAMEG and RFNAMEG. In order to provide
> all the knobs RFNOMNT does, blocking 'M' is interpreted to block all mounting.
> 
> Support for this new operation has been added to namespace files,
> along with a matching chdev(1). /lib/namespace.ftp has been updated
> to use chdev.
> ...
> --- /tmp/diff100000711164
> +++ b/rc/bin/chdev
> @@ -1,0 +1,3 @@
> +#!/bin/rc
> +
> +echo chdev $* >> '#c/drivers'
> ...
> --- /tmp/diff100000711170
> +++ b/sys/man/1/chdev
> @@ -1,0 +1,93 @@
> +.TH CHDEV 1
> +.SH NAME
> +chdev \- change kernel driver access
> +.SH SYNOPSIS
> +.B chdev
> +[ 0! ] \f2devmask\fP...

> --- a/sys/src/9/port/dev.c
> +++ b/sys/src/9/port/dev.c
> @@ -31,6 +31,74 @@
>  }
> 
>  void
> +devmask(Pgrp *pgrp, char *devs)
> +{
> +	int i, t, w;
> +	int invert;
> +	char *p;
> +	Rune r;
> +	u64int mask[nelem(pgrp->notallowed)];
> +
> +	
> +	invert = 1;
> +	switch(*devs){
> +	case '!':
> +		memset(mask, 0, sizeof mask);
> +		devs++;
> +		invert--;
> +		break;
> +	case '0':
> +		devs = "";
> +	default:
> +		memset(mask, 0xFF, sizeof mask);
> +		break;
> +	}		
> +
> +	w = sizeof mask[0] * 8;
> +	for(p = devs; *p != '\0';){
> +		p += chartorune(&r, p);
> +		t = devno(r, 1);
> +		if(t == -1)
> +			continue;
> +		if(invert)
> +			mask[t/w] &= ~(1<<t%w);
> +		else
> +			mask[t/w] |= 1<<t%w;
> +	}

What happens if '!!!' is the prefix?  Should the '!' set invert = 0
explicitly instead of invert--?  Does '!0' allow access to all
drivers?


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-26 23:36                                 ` unobe
@ 2022-05-27  0:33                                   ` Jacob Moody
  2022-05-27  3:25                                     ` unobe
  0 siblings, 1 reply; 28+ messages in thread
From: Jacob Moody @ 2022-05-27  0:33 UTC (permalink / raw)
  To: 9front

On 5/26/22 17:36, unobe@cpan.org wrote:
> Great work on this moody.  I have some (hopefully helpful) questions
> below.
> 
> 
> Quoth Jacob Moody <moody@mail.posixcafe.org>:
>> Another iteration.
>> To state all of what this does:
>>
>> A process can remove access to a driver through writing
>>
>> chdev [ 0! ] devmask
>>
>> to /dev/drivers. This is tied to the processes namegroup,
>> and is inherited on both RFCNAMEG and RFNAMEG. In order to provide
>> all the knobs RFNOMNT does, blocking 'M' is interpreted to block all mounting.
>>
>> Support for this new operation has been added to namespace files,
>> along with a matching chdev(1). /lib/namespace.ftp has been updated
>> to use chdev.
>> ...
>> --- /tmp/diff100000711164
>> +++ b/rc/bin/chdev
>> @@ -1,0 +1,3 @@
>> +#!/bin/rc
>> +
>> +echo chdev $* >> '#c/drivers'
>> ...
>> --- /tmp/diff100000711170
>> +++ b/sys/man/1/chdev
>> @@ -1,0 +1,93 @@
>> +.TH CHDEV 1
>> +.SH NAME
>> +chdev \- change kernel driver access
>> +.SH SYNOPSIS
>> +.B chdev
>> +[ 0! ] \f2devmask\fP...
> 
>> --- a/sys/src/9/port/dev.c
>> +++ b/sys/src/9/port/dev.c
>> @@ -31,6 +31,74 @@
>>  }
>>
>>  void
>> +devmask(Pgrp *pgrp, char *devs)
>> +{
>> +	int i, t, w;
>> +	int invert;
>> +	char *p;
>> +	Rune r;
>> +	u64int mask[nelem(pgrp->notallowed)];
>> +
>> +	
>> +	invert = 1;
>> +	switch(*devs){
>> +	case '!':
>> +		memset(mask, 0, sizeof mask);
>> +		devs++;
>> +		invert--;
>> +		break;
>> +	case '0':
>> +		devs = "";
>> +	default:
>> +		memset(mask, 0xFF, sizeof mask);
>> +		break;
>> +	}		
>> +
>> +	w = sizeof mask[0] * 8;
>> +	for(p = devs; *p != '\0';){
>> +		p += chartorune(&r, p);
>> +		t = devno(r, 1);
>> +		if(t == -1)
>> +			continue;
>> +		if(invert)
>> +			mask[t/w] &= ~(1<<t%w);
>> +		else
>> +			mask[t/w] |= 1<<t%w;
>> +	}
> 
> What happens if '!!!' is the prefix?  Should the '!' set invert = 0
> explicitly instead of invert--?  Does '!0' allow access to all
> drivers?
> 

That switch case is not looped over. We only check the very first character
for the prefix. The rest of the string is interpreted to be the driver string.
In both of your cases the prefix is '!' and the driver string only contains '0' or '!',
neither are real drivers so you block nothing.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-25 19:03                         ` Jacob Moody
  2022-05-25 20:53                           ` hiro
@ 2022-05-27  1:11                           ` Lyndon Nerenberg (VE7TFX/VE6BBM)
  2022-05-27  2:25                             ` Frank D. Engel, Jr.
  1 sibling, 1 reply; 28+ messages in thread
From: Lyndon Nerenberg (VE7TFX/VE6BBM) @ 2022-05-27  1:11 UTC (permalink / raw)
  To: 9front, Jacob Moody

Instead of dancing around all these prefixes, why not just be
explicit about it. There are three cases:

  chdev add abcd	# add 'abcd' to the current set
  chdev del abcd	# remove 'abcd' from the current set
  chdev abcd		# 'abcd' becomes the absolute set

No ambiguities, and dead simple to parse.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-27  1:11                           ` Lyndon Nerenberg (VE7TFX/VE6BBM)
@ 2022-05-27  2:25                             ` Frank D. Engel, Jr.
  0 siblings, 0 replies; 28+ messages in thread
From: Frank D. Engel, Jr. @ 2022-05-27  2:25 UTC (permalink / raw)
  To: 9front, Jacob Moody

I believe the entire point is that you can only remove - not add.  There 
is only one operation.

The "prefixes" are to make it more convenient to specify the set of 
devices being masked, not to change what is being done to them.


On 5/26/22 9:11 PM, Lyndon Nerenberg (VE7TFX/VE6BBM) wrote:
> Instead of dancing around all these prefixes, why not just be
> explicit about it. There are three cases:
>
>    chdev add abcd	# add 'abcd' to the current set
>    chdev del abcd	# remove 'abcd' from the current set
>    chdev abcd		# 'abcd' becomes the absolute set
>
> No ambiguities, and dead simple to parse.
>


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [9front] [PATCH] Unmount to remove sharp devices.
  2022-05-27  0:33                                   ` Jacob Moody
@ 2022-05-27  3:25                                     ` unobe
  0 siblings, 0 replies; 28+ messages in thread
From: unobe @ 2022-05-27  3:25 UTC (permalink / raw)
  To: 9front

Quoth Jacob Moody <moody@mail.posixcafe.org>:
> On 5/26/22 17:36, unobe@cpan.org wrote:
> > What happens if '!!!' is the prefix?  Should the '!' set invert = 0
> > explicitly instead of invert--?  Does '!0' allow access to all
> > drivers?
> > 
> 
> That switch case is not looped over. We only check the very first character
> for the prefix. The rest of the string is interpreted to be the driver string.
> In both of your cases the prefix is '!' and the driver string only contains '0' or '!',
> neither are real drivers so you block nothing.

:facepalm:

Somehow I was reading the switch as a loop. Sorry for the noise.


^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2022-05-27  3:29 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-04 14:09 [9front] [PATCH] Unmount to remove sharp devices Jacob Moody
2022-05-04 15:05 ` ori
2022-05-04 15:31 ` ori
2022-05-04 16:15   ` Stanley Lieber
2022-05-04 17:41   ` Lyndon Nerenberg (VE7TFX/VE6BBM)
2022-05-04 17:55     ` Jacob Moody
2022-05-05  1:59       ` Alex Musolino
2022-05-05 16:07         ` Jacob Moody
2022-05-08  2:55           ` Jacob Moody
2022-05-11 14:47             ` Jacob Moody
2022-05-11 16:11               ` Stanley Lieber
2022-05-12  4:29                 ` Jacob Moody
2022-05-12  3:18             ` ori
2022-05-12  5:10               ` Jacob Moody
2022-05-12 14:21                 ` ori
2022-05-23  5:42                   ` Jacob Moody
2022-05-23 17:06                     ` cinap_lenrek
2022-05-23 17:37                       ` Jacob Moody
2022-05-25 19:03                         ` Jacob Moody
2022-05-25 20:53                           ` hiro
2022-05-25 21:20                             ` Jacob Moody
2022-05-26  5:55                               ` Jacob Moody
2022-05-26 23:36                                 ` unobe
2022-05-27  0:33                                   ` Jacob Moody
2022-05-27  3:25                                     ` unobe
2022-05-26  3:13                             ` ori
2022-05-27  1:11                           ` Lyndon Nerenberg (VE7TFX/VE6BBM)
2022-05-27  2:25                             ` Frank D. Engel, Jr.

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).