[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Process isolation support for start-stop-daemon


On Sun, 16 Mar 2014 05:35:41 +0100
Guillem Jover <guillem@debian.org> wrote:

> First, would be defining in a system-independent way what such option
> implies, and how and if it can be extended in the future, like what
> restrictions, guarantees and expectactions should the process get,
> etc. Depending on those, the implementations might change, for example
> FreeBSD's Capsicum might be too restrictive depending on the started
> process and its expectations, but jails or Solaris Zones could cover
> most of it. The Hurd might be able to use an interposed proc server,
> perhaps.

> In principle I think this should be mostly something that improves the
> security of the started process, but not something the process should
> depend for it to be secure at all. So if it ends up being a no-op on
> some systems or on older kernels, that should not make the process a
> security threat for example.

That's what I thought as well, and that's the reason --isolate compiles
to a no-op on systems which aren't enough capable: it *may* add some
security, but it's not something to rely on.

> Then there's several implementation details, here's a short list from
> a quick skimming (ignoring style issues and similar):

> * The command-line option should always be visible in the help output.

Probably if it's a no-op, there should be a notice about that then?

> * Some system calls are missing proper error checks.
> * The quiet warnings seem suspect, I'd say they should either be
>   actual errors or normal warnings.

Well, those warnings don't necessarily mean something went wrong, which
is why I set them to default-quiet mode.

> * Why remount the /dev filesystems? /proc is needed to get the new
>   PID namespace, but the others do not seem needed? And they are
>   problematic as they might change depending on the system, for
>   example /dev/shm is now /run/shm in Debian.

That code it ported from lxc-unshare. I haven't checked if /dev/*
things are really needed, so left it as is for a while.

> * The stack might grow upwards or downwards depending on the system.
> * The dummy SIGTERM handler seems suspect, if it's needed (why?), it
>   should exit(0), otherwise there's a race in the loop if the signal
>   gets caught not during the wait(), which would then become ignored.

Probably. I haven't thought about possible races.

> * Pointers should get NULL not 0.
> * The two wait()s in the parent could be collapsed into one, and
>   handle error conditions from normal conditions (EINTR and ECHILD vs
>   the rest for example).
> * The fatal() belongs with the execve(), which makes the “return;”
>   unnecessary.

Thanks for the comments, I think I may try to address them this week,
but not sure when exactly :)


Attachment: signature.asc
Description: PGP signature

Reply to: