file shouldn't need chflagsat?
Otherwise I think this is fine.
On Thu, Jun 04, 2015 at 03:29:06PM -0700, patrick keshishian wrote:
> On Thu, Jun 04, 2015 at 11:06:38PM +0100, Nicholas Marriott wrote:
> > /usr/ports/infrastructure/db/systrace.filter has these:
> >
> > native-recvmsg: permit
> > native-sendmsg: sockaddr match "/tmp" then permit
> > native-sendmsg: sockaddr match "/var/tmp" then permit
> > native-sendmsg: sockaddr match "/tmp" then permit
> > native-sendmsg: sockaddr match "/usr/ports/pobj/unzip-6.0" then permit
> > native-sendmsg: sockaddr match "/<non-existent filename>: *" then
> > deny[enoent]
> >
> > We could add this I think:
> >
> > native-sendmsg: sockaddr eq "<unknown>" then permit
>
> If this is acceptable, then the file(1) patch reduces to simply
> skipping the systrace set-up if STRIOCATTACH fails.
>
> Patches follow for file(1) and ports' systrace.policy
>
>
> Index: sandbox.c
> ===================================================================
> RCS file: /cvs/obsd/src/usr.bin/file/sandbox.c,v
> retrieving revision 1.7
> diff -u -p -u -p -r1.7 sandbox.c
> --- sandbox.c 29 May 2015 15:58:34 -0000 1.7
> +++ sandbox.c 4 Jun 2015 22:23:32 -0000
> @@ -130,7 +130,7 @@ sandbox_fork(const char *user)
> close(devfd);
>
> if (ioctl(fd, STRIOCATTACH, &pid) == -1)
> - err(1, "ioctl(STRIOCATTACH)");
> + goto out;
>
> memset(&policy, 0, sizeof policy);
> policy.strp_op = SYSTR_POLICY_NEW;
> @@ -150,7 +150,7 @@ sandbox_fork(const char *user)
> err(1, "ioctl(STRIOCPOLICY/MODIFY)");
> }
>
> - if (kill(pid, SIGCONT) != 0)
> +out: if (kill(pid, SIGCONT) != 0)
> err(1, "kill(SIGCONT)");
> return (pid);
> }
>
>
> Index: systrace.filter
> ===================================================================
> RCS file: /cvs/obsd/ports/infrastructure/db/systrace.filter,v
> retrieving revision 1.45
> diff -u -p -u -p -r1.45 systrace.filter
> --- systrace.filter 11 Sep 2014 10:33:44 -0000 1.45
> +++ systrace.filter 4 Jun 2015 22:25:08 -0000
> @@ -22,6 +22,7 @@
> native-chflags: filename match "${TMPDIR}" then permit
> native-chflags: filename match "${WRKDIR}" then permit
> native-chflags: filename match "/<non-existent filename>: *" then
> deny[enoent]
> + native-chflagsat: filename match "${WRKDIR}" then permit
> native-chmod: filename match "/tmp" then permit
> native-chmod: filename match "/var/tmp" then permit
> native-chmod: filename match "${TMPDIR}" then permit
> @@ -93,6 +94,7 @@
> native-futimes: permit
> native-futimens: permit
> native-getdents: permit
> + native-getdtablecount: permit
> native-getegid: permit
> native-getentropy: permit
> native-geteuid: permit
> @@ -196,6 +198,7 @@
> native-sendmsg: sockaddr match "${TMPDIR}" then permit
> native-sendmsg: sockaddr match "${WRKDIR}" then permit
> native-sendmsg: sockaddr match "/<non-existent filename>: *" then
> deny[enoent]
> + native-sendmsg: sockaddr eq "<unknown>" then permit
> native-sendsyslog: permit
> native-sendto: permit
> native-setegid: permit
>
>
>
> >
> > On Thu, Jun 04, 2015 at 10:47:47PM +0100, Nicholas Marriott wrote:
> > > Hi
> > >
> > > On Thu, Jun 04, 2015 at 03:39:45PM -0600, Theo de Raadt wrote:
> > > > > Is it just to avoid adding sendmsg to the ports systrace policy? Why
> > > > > not
> > > > > add it - maybe not globally but just for file?
> > > >
> > > > sendmsg with a CMSG fd passing in/out of such a jail is a bad thing.
> > >
> > > The systrace policy already allows recvmsg(). So we can get new fds in,
> > > why not send them out?
> > >
> > > Any fd we have inside to send out will have had to have passed the
> > > open(), bind() etc systrace rules already.
> > >
> > > >
> > > > However.
> > > >
> > > > It is likely that a ports configure test may try to test this interface.
> > > > Not just CMSG, but sendmsg itself.
> > > >
> > > > It suspect it needs to find that it works.
> > > >
> > > > I doubt this is a system call that can be blocked.
> > > >
> > > > It sounds like a great idea to limit the build environment
> > > > substantially,
> > > > but an eye must be kept on fallout from being too strict. That's the
> > > > problem with systrace; it is too easy to return an 'error' and a program
> > > > will continue...
> > > >
> >