[9front] git: use new /dev/drivers for privdrop

[9front] git: use new /dev/drivers for privdrop

[ori]

This needs to be held off for a bit, but it seems
like a good idea: we shouldn't need any devices at
all for this code, so let's not keep them.

ddiff 01a6de812c2fd38c9a28036300e3c2c7dc8441f5 uncommitted
--- a/sys/src/cmd/git/serve.c
+++ b/sys/src/cmd/git/serve.c
@@ -5,7 +5,7 @@
 
 #include "git.h"
 
-char	*pathpfx = nil;
+char	*pathpfx = "/";
 int	allowwrite;
 
 int
@@ -469,6 +469,22 @@
 	return 0;
 }
 
+void
+privdrop(void)
+{
+	int fd;
+
+	if(rfork(RFNAMEG) == -1)
+		sysfatal("rfork: %r");
+	if((fd = open("#c/drivers", OWRITE)) == -1)
+		sysfatal("open drivers: %r");
+	if(bind(pathpfx, "/", MREPL) == -1)
+		sysfatal("bind: %r");
+	if(write(fd, "chdev &", strlen("chdev &")) == -1)
+		sysfatal("drop permissions: %r");
+	close(fd);
+}
+
 char*
 parsecmd(char *buf, char *cmd, int ncmd)
 {
@@ -518,15 +534,8 @@
 	}ARGEND;
 
 	gitinit();
+	privdrop();
 	interactive = 0;
-	if(rfork(RFNAMEG) == -1)
-		sysfatal("rfork: %r");
-	if(pathpfx != nil){
-		if(bind(pathpfx, "/", MREPL) == -1)
-			sysfatal("bind: %r");
-	}
-	if(rfork(RFNOMNT) == -1)
-		sysfatal("rfork: %r");
 
 	initconn(&c, 0, 1);
 	if(readpkt(&c, buf, sizeof(buf)) == -1)

Re: [9front] git: use new /dev/drivers for privdrop

[unobe]

In general, how does one restrict computation/process limitations?  I
am admittedly ignorant, so if anyone has docs for me to read instead
of replying that what I want to do is already solved or is a waste of
time I'd appreciate it.

I see proc(3) describes each process's status file as containing the
user, amount of memory, and scheduling priority.  So for unnecessary
churn and limiting fork bombs, is the idea to have a monitor that
watches the interested values (e.g.  memory or number of processes)
for a user, and kill if a certain amount has been reached?

For example, say I want a sandboxing area for people to "try 9front".
With moody's recent work, it removes a big attack vector by
restricting certain drivers.  But isn't it still possible to fork-bomb
a server, or to just cause unnecessary churn (i.e., computation), or
just open too many files, or fill a disk?

To address the "fill a disk" scenario, I assume the best thing is to
create a temporary disk that is used by the process which is provided
to the test user--they'll only have so much space to work with.  If
there's no space to allocate for the new temporary disk, it's easy
enough to stop the process from starting.  Disk-resizing would be
neat, but I don't care about that to start.

Quoth ori@eigenstate.org:
> This needs to be held off for a bit, but it seems
> like a good idea: we shouldn't need any devices at
> all for this code, so let's not keep them.
> 
> ddiff 01a6de812c2fd38c9a28036300e3c2c7dc8441f5 uncommitted
> --- a/sys/src/cmd/git/serve.c
> +++ b/sys/src/cmd/git/serve.c
> @@ -5,7 +5,7 @@
>  
>  #include "git.h"
>  
> -char	*pathpfx = nil;
> +char	*pathpfx = "/";
>  int	allowwrite;
>  
>  int
> @@ -469,6 +469,22 @@
>  	return 0;
>  }
>  
> +void
> +privdrop(void)
> +{
> +	int fd;
> +
> +	if(rfork(RFNAMEG) == -1)
> +		sysfatal("rfork: %r");
> +	if((fd = open("#c/drivers", OWRITE)) == -1)
> +		sysfatal("open drivers: %r");
> +	if(bind(pathpfx, "/", MREPL) == -1)
> +		sysfatal("bind: %r");
> +	if(write(fd, "chdev &", strlen("chdev &")) == -1)
> +		sysfatal("drop permissions: %r");
> +	close(fd);
> +}
> +
>  char*
>  parsecmd(char *buf, char *cmd, int ncmd)
>  {
> @@ -518,15 +534,8 @@
>  	}ARGEND;
>  
>  	gitinit();
> +	privdrop();
>  	interactive = 0;
> -	if(rfork(RFNAMEG) == -1)
> -		sysfatal("rfork: %r");
> -	if(pathpfx != nil){
> -		if(bind(pathpfx, "/", MREPL) == -1)
> -			sysfatal("bind: %r");
> -	}
> -	if(rfork(RFNOMNT) == -1)
> -		sysfatal("rfork: %r");
>  
>  	initconn(&c, 0, 1);
>  	if(readpkt(&c, buf, sizeof(buf)) == -1)
> 

Re: [9front] git: use new /dev/drivers for privdrop

[ori]

Quoth unobe@cpan.org:
> For example, say I want a sandboxing area for people to "try 9front".
> With moody's recent work, it removes a big attack vector by
> restricting certain drivers.  But isn't it still possible to fork-bomb
> a server, or to just cause unnecessary churn (i.e., computation), or
> just open too many files, or fill a disk?

Yes.

I'm not aware of anyone trying to protect against denial of
service with shared resources. This work mostly is about
preventing data leakage.

Re: [9front] git: use new /dev/drivers for privdrop

[covertusername967]

Quoth ori@eigenstate.org:
> This needs to be held off for a bit, but it seems
> like a good idea: we shouldn't need any devices at
> all for this code, so let's not keep them.
> 
> ddiff 01a6de812c2fd38c9a28036300e3c2c7dc8441f5 uncommitted
> --- a/sys/src/cmd/git/serve.c
> +++ b/sys/src/cmd/git/serve.c
> @@ -5,7 +5,7 @@
>  
>  #include "git.h"
>  
> -char	*pathpfx = nil;
> +char	*pathpfx = "/";
>  int	allowwrite;
>  
>  int
> @@ -469,6 +469,22 @@
>  	return 0;
>  }
>  
> +void
> +privdrop(void)
> +{
> +	int fd;
> +
> +	if(rfork(RFNAMEG) == -1)
> +		sysfatal("rfork: %r");
> +	if((fd = open("#c/drivers", OWRITE)) == -1)
> +		sysfatal("open drivers: %r");
> +	if(bind(pathpfx, "/", MREPL) == -1)
> +		sysfatal("bind: %r");
> +	if(write(fd, "chdev &", strlen("chdev &")) == -1)
> +		sysfatal("drop permissions: %r");
> +	close(fd);
> +}
> +
>  char*
>  parsecmd(char *buf, char *cmd, int ncmd)
>  {
> @@ -518,15 +534,8 @@
>  	}ARGEND;
>  
>  	gitinit();
> +	privdrop();
>  	interactive = 0;
> -	if(rfork(RFNAMEG) == -1)
> -		sysfatal("rfork: %r");
> -	if(pathpfx != nil){
> -		if(bind(pathpfx, "/", MREPL) == -1)
> -			sysfatal("bind: %r");
> -	}
> -	if(rfork(RFNOMNT) == -1)
> -		sysfatal("rfork: %r");
>  
>  	initconn(&c, 0, 1);
>  	if(readpkt(&c, buf, sizeof(buf)) == -1)
> 

Correct me if i'm wrong, but if i remember correctly, wasn't git9 intended to be compatible with 9legacy as well as 9front? Won't this break compatibility?

Re: [9front] git: use new /dev/drivers for privdrop

[ori]

Quoth covertusername967@gmail.com:
> Quoth ori@eigenstate.org:
> > This needs to be held off for a bit, but it seems
> > like a good idea: we shouldn't need any devices at
> > all for this code, so let's not keep them.
> > 
> > ddiff 01a6de812c2fd38c9a28036300e3c2c7dc8441f5 uncommitted
> > --- a/sys/src/cmd/git/serve.c
> > +++ b/sys/src/cmd/git/serve.c
> > @@ -5,7 +5,7 @@
> >  
> >  #include "git.h"
> >  
> > -char	*pathpfx = nil;
> > +char	*pathpfx = "/";
> >  int	allowwrite;
> >  
> >  int
> > @@ -469,6 +469,22 @@
> >  	return 0;
> >  }
> >  
> > +void
> > +privdrop(void)
> > +{
> > +	int fd;
> > +
> > +	if(rfork(RFNAMEG) == -1)
> > +		sysfatal("rfork: %r");
> > +	if((fd = open("#c/drivers", OWRITE)) == -1)
> > +		sysfatal("open drivers: %r");
> > +	if(bind(pathpfx, "/", MREPL) == -1)
> > +		sysfatal("bind: %r");
> > +	if(write(fd, "chdev &", strlen("chdev &")) == -1)
> > +		sysfatal("drop permissions: %r");
> > +	close(fd);
> > +}
> > +
> >  char*
> >  parsecmd(char *buf, char *cmd, int ncmd)
> >  {
> > @@ -518,15 +534,8 @@
> >  	}ARGEND;
> >  
> >  	gitinit();
> > +	privdrop();
> >  	interactive = 0;
> > -	if(rfork(RFNAMEG) == -1)
> > -		sysfatal("rfork: %r");
> > -	if(pathpfx != nil){
> > -		if(bind(pathpfx, "/", MREPL) == -1)
> > -			sysfatal("bind: %r");
> > -	}
> > -	if(rfork(RFNOMNT) == -1)
> > -		sysfatal("rfork: %r");
> >  
> >  	initconn(&c, 0, 1);
> >  	if(readpkt(&c, buf, sizeof(buf)) == -1)
> > 
> 
> Correct me if i'm wrong, but if i remember correctly, wasn't git9 intended to be compatible with 9legacy as well as 9front? Won't this break compatibility?
> 

I guess I can skip this patch when I pull things into
the mirror, though -- I'm also not sure anyone is using
it on 9legacy.

Re: [9front] git: use new /dev/drivers for privdrop

[Jacob Moody]

On 5/28/22 13:41, ori@eigenstate.org wrote:
> Quoth unobe@cpan.org:
>> For example, say I want a sandboxing area for people to "try 9front".
>> With moody's recent work, it removes a big attack vector by
>> restricting certain drivers.  But isn't it still possible to fork-bomb
>> a server, or to just cause unnecessary churn (i.e., computation), or
>> just open too many files, or fill a disk?
> 
> Yes.
> 
> I'm not aware of anyone trying to protect against denial of
> service with shared resources. This work mostly is about
> preventing data leakage.
> 
> 

I dont have too much interest in doing quotas. The approach
I want for dealing with shared resources is to remove the
resources you dont need. Attempting to block only misuse of resources
seem error prone. With that being said it might be nice to increase
the list of capabilities that can be eroded. Currently there are
some devices that are tied to other capabilities:

devmnt(#M) is tied to the mount system call.
devpipe(#|) is tied to the pipe system call.

The first was done to emulate RFNOMNT, the later
fell out from how syspipe is implemented. I might purpose
expanding this list a bit further:

devup(#d) could be tied to sysdup, if sysdup implemented itself through devdup.
devproc(#p) could be tied to forking new processes.
or even
devroot(#/)/devmnt could be tied to the ability of doing any kind of walks. (open fds would be preserved)

This would allow you to setup an environment where a fork bomb/fd exhaustion is not possible.
I also have some interest in getting clean instances of some kernel drivers, namely /srv and /proc.
My current running thought is through an attach argument:

bind -c '#sc' /srv
bind -c '#pc' /proc

This would of course affect future walks without the attach option.
This allows the devices to be used without having to introduce the global state.
Another thought that has been kicked around is a permanent bind/mount flag: MPERM.
Which could prevent future binds/mounts/unmount of that specific file/dir.

A bit of a thought dump, but wanted to get some input on potential ways we can go
with cutting off further capabilities.


Thanks,
moody

Re: [9front] git: use new /dev/drivers for privdrop

[adr]

>> Correct me if i'm wrong, but if i remember correctly, wasn't git9 intended to be compatible with 9legacy as well as 9front? Won't this break compatibility?
>>
>
> I guess I can skip this patch when I pull things into
> the mirror, though -- I'm also not sure anyone is using
> it on 9legacy.

It's not compatible with 9legacy right now. I had to import your
changes of date.c and ctime.c to compile it. You could update what
you posted in 9fans:

https://git.eigenstate.org/ori/date.git

and make the code compile with that if the functions are not present
in libc. Now, is it worthy? By your own words it seems that you
are not having any feedback from 9legacy so, why bother?.

adr.

Re: [9front] git: use new /dev/drivers for privdrop

[Frank D. Engel, Jr.]

Fork bombing might not be possible in that scenario, but it is because 
forking would be impossible to do at all?

People trying the OS through the type of sandbox test system being 
proposed wouldn't really be able to do anything to test the system if 
they cannot create new processes, but allowing that to go on unchecked 
(without limits) could impact other users.

I tend to agree that if this is a use case to be considered, a quota of 
some kind would be in order.

This would also eliminate the need for the somewhat questionable idea of 
using a "fake" device to serve as a process security mechanism for the 
fork call, as one could simply set a quota of zero.


On 5/28/22 6:54 PM, Jacob Moody wrote:
> On 5/28/22 13:41, ori@eigenstate.org wrote:
>> Quoth unobe@cpan.org:
>>> For example, say I want a sandboxing area for people to "try 9front".
>>> With moody's recent work, it removes a big attack vector by
>>> restricting certain drivers.  But isn't it still possible to fork-bomb
>>> a server, or to just cause unnecessary churn (i.e., computation), or
>>> just open too many files, or fill a disk?
>> Yes.
>>
>> I'm not aware of anyone trying to protect against denial of
>> service with shared resources. This work mostly is about
>> preventing data leakage.
>>
>>
> I dont have too much interest in doing quotas. The approach
> I want for dealing with shared resources is to remove the
> resources you dont need. Attempting to block only misuse of resources
> seem error prone. With that being said it might be nice to increase
> the list of capabilities that can be eroded. Currently there are
> some devices that are tied to other capabilities:
>
> devmnt(#M) is tied to the mount system call.
> devpipe(#|) is tied to the pipe system call.
>
> The first was done to emulate RFNOMNT, the later
> fell out from how syspipe is implemented. I might purpose
> expanding this list a bit further:
>
> devup(#d) could be tied to sysdup, if sysdup implemented itself through devdup.
> devproc(#p) could be tied to forking new processes.
> or even
> devroot(#/)/devmnt could be tied to the ability of doing any kind of walks. (open fds would be preserved)
>
> This would allow you to setup an environment where a fork bomb/fd exhaustion is not possible.
> I also have some interest in getting clean instances of some kernel drivers, namely /srv and /proc.
> My current running thought is through an attach argument:
>
> bind -c '#sc' /srv
> bind -c '#pc' /proc
>
> This would of course affect future walks without the attach option.
> This allows the devices to be used without having to introduce the global state.
> Another thought that has been kicked around is a permanent bind/mount flag: MPERM.
> Which could prevent future binds/mounts/unmount of that specific file/dir.
>
> A bit of a thought dump, but wanted to get some input on potential ways we can go
> with cutting off further capabilities.
>
>
> Thanks,
> moody
>

Re: [9front] git: use new /dev/drivers for privdrop

[Jacob Moody]

On 5/28/22 18:23, Frank D. Engel, Jr. wrote:
> 
> People trying the OS through the type of sandbox test system being 
> proposed wouldn't really be able to do anything to test the system if 
> they cannot create new processes, but allowing that to go on unchecked 
> (without limits) could impact other users.
> 
> I tend to agree that if this is a use case to be considered, a quota of 
> some kind would be in order.

I dont think this use case is worth perusing. The concept of what constitutes
misuse of a resource is vague and dependent on hardware. Like I mentioned,
I think trying to make the promise that a process with access to a resource
won't misuse it is error prone.


> 
> This would also eliminate the need for the somewhat questionable idea of 
> using a "fake" device to serve as a process security mechanism for the 
> fork call, as one could simply set a quota of zero.
> 
> 

We are in plan9, a very large chunk of a processes capabilities are accomplished
through access to files. We have a system for mutating a processes view of
files through namespaces.
Devip is also a "fake" device, yet access to that
is tied to the processes ability to talk on the network. It seems consistent
with the theme of the system to describe capabilities as the set of files
a process has access to. If anything the fact that the fork call is a capability
that is not influenced by the namespace makes it the odd one out.


Thanks,
moody