Re: [gentoo-portage-dev] [PATCH 2/3] Support FEATURES=pid-sandbox
On Sat, 2018-11-17 at 19:20 -0800, Zac Medico wrote:
> On 11/14/18 12:02 AM, Michał Górny wrote:
> > @@ -531,6 +543,15 @@ def _exec(binary, mycommand, opt_name, fd_pipes, env,
> > gid, groups, uid, umask,
> >
> > errno.errorcode.get(ctypes.get_errno(), '?')),
> > noiselevel=-1)
> > else:
> > + if unshare_pid:
> > + # pid namespace
> > requires us to become init
> > + # TODO: do init-ty stuff
> > + # therefore, fork() ASAP
> > + fork_ret = os.fork()
> > + if fork_ret != 0:
> > + pid, status =
> > os.waitpid(fork_ret, 0)
> > + assert pid ==
> > fork_ret
> > + os._exit(status)
> > if unshare_mount:
> > # mark the whole
> > filesystem as private to avoid
> > # mounts escaping the
> > namespace
> > @@ -541,6 +562,18 @@ def _exec(binary, mycommand, opt_name, fd_pipes, env,
> > gid, groups, uid, umask,
> > # TODO: should
> > it be fatal maybe?
> >
> > writemsg("Unable to mark mounts private: %d\n" % (mount_ret,),
> >
> > noiselevel=-1)
> > + if unshare_pid:
> > + if mount_ret != 0:
> > + # can't proceed
> > without private mounts
> > + os._exit(1)
>
> For the benefit of anyone not watching
> https://github.com/gentoo/portage/pull/379, the mount_ret is expected
> to be non-zero inside a chroot where `mount --make-rprivate /` would
> fail because / is not a mountpoint, and this is an extremely valuable
> use case for tools like catalyst that perform builds inside a chroot.
>
> Based on /proc/[pid]/mountinfo documentation in the proc(5) manpage,
> and empirical analysis of /proc/self/mountinfo in various states,
> it should be reasonable to assume that the current propagation flags
> are suitable if there is not a shared / or /proc mountpoint found in
> /proc/self/mountinfo. We can use a function like this to check if the
> `mount --make-rprivate /` call is needed:
>
> def want_make_rprivate():
> try:
> with open('/proc/self/mountinfo', 'rb') as f:
> if re.match(rb'^\S+ \S+ \S+ \S+ (?P/|/proc) \S+
> (?:\S+:\S+ )*(?Pshared:\S+ )', f.read(), re.MULTILINE) is None:
> return False
> except EnvironmentError:
> pass
> return True
Technically speaking, FEATURES=mount-sandbox goes beyond /proc, so
making it guess based on the state of /proc is wrong. Yes, we need only
/proc for the purpose of pid-sandbox but we shouldn't presume the user
wouldn't want more.
Maybe we should split the logic more, and e.g. be more fatal about
--make-rprivate failing when mount-sandbox is explicitly used, and only
care about /proc when pid-sandbox is only used. In the latter case, it
probably doesn't make sense to check but just try to --make-private
/proc. /proc is naturally a mountpoint, so it should cause no harm to
privatize it.
Also, after some extra reading I think we should use 'slave' instead of
'private'. Having things implicitly 'private' might be surprising since
user's actions won't propagate into the build namespace, and I think we
want them to propagate.
--
Best regards,
Michał Górny
signature.asc
Description: This is a digitally signed message part
Re: [gentoo-portage-dev] [PATCH 2/3] Support FEATURES=pid-sandbox
On 11/14/18 12:02 AM, Michał Górny wrote:
> @@ -531,6 +543,15 @@ def _exec(binary, mycommand, opt_name, fd_pipes, env,
> gid, groups, uid, umask,
>
> errno.errorcode.get(ctypes.get_errno(), '?')),
> noiselevel=-1)
> else:
> + if unshare_pid:
> + # pid namespace
> requires us to become init
> + # TODO: do init-ty stuff
> + # therefore, fork() ASAP
> + fork_ret = os.fork()
> + if fork_ret != 0:
> + pid, status =
> os.waitpid(fork_ret, 0)
> + assert pid ==
> fork_ret
> + os._exit(status)
> if unshare_mount:
> # mark the whole
> filesystem as private to avoid
> # mounts escaping the
> namespace
> @@ -541,6 +562,18 @@ def _exec(binary, mycommand, opt_name, fd_pipes, env,
> gid, groups, uid, umask,
> # TODO: should
> it be fatal maybe?
>
> writemsg("Unable to mark mounts private: %d\n" % (mount_ret,),
>
> noiselevel=-1)
> + if unshare_pid:
> + if mount_ret != 0:
> + # can't proceed
> without private mounts
> + os._exit(1)
For the benefit of anyone not watching
https://github.com/gentoo/portage/pull/379, the mount_ret is expected
to be non-zero inside a chroot where `mount --make-rprivate /` would
fail because / is not a mountpoint, and this is an extremely valuable
use case for tools like catalyst that perform builds inside a chroot.
Based on /proc/[pid]/mountinfo documentation in the proc(5) manpage,
and empirical analysis of /proc/self/mountinfo in various states,
it should be reasonable to assume that the current propagation flags
are suitable if there is not a shared / or /proc mountpoint found in
/proc/self/mountinfo. We can use a function like this to check if the
`mount --make-rprivate /` call is needed:
def want_make_rprivate():
try:
with open('/proc/self/mountinfo', 'rb') as f:
if re.match(rb'^\S+ \S+ \S+ \S+ (?P/|/proc) \S+
(?:\S+:\S+ )*(?Pshared:\S+ )', f.read(), re.MULTILINE) is None:
return False
except EnvironmentError:
pass
return True
--
Thanks,
Zac
signature.asc
Description: OpenPGP digital signature
[gentoo-portage-dev] [PATCH 2/3] Support FEATURES=pid-sandbox
Supporting using PID namespace in order to isolate the ebuild processes
from host system, and make it possible to kill them all easily
(similarly to cgroups but easier to use).
Bug: https://bugs.gentoo.org/659582
Signed-off-by: Michał Górny
---
lib/portage/const.py | 1 +
lib/portage/package/ebuild/doebuild.py | 8 +++--
lib/portage/process.py | 43 +++---
man/make.conf.5| 7 +
4 files changed, 52 insertions(+), 7 deletions(-)
diff --git a/lib/portage/const.py b/lib/portage/const.py
index e0f93f7cc..ca66bc46e 100644
--- a/lib/portage/const.py
+++ b/lib/portage/const.py
@@ -174,6 +174,7 @@ SUPPORTED_FEATURES = frozenset([
"notitles",
"parallel-fetch",
"parallel-install",
+ "pid-sandbox",
"prelink-checksums",
"preserve-libs",
"protect-owned",
diff --git a/lib/portage/package/ebuild/doebuild.py
b/lib/portage/package/ebuild/doebuild.py
index e84a618d2..9917ac82c 100644
--- a/lib/portage/package/ebuild/doebuild.py
+++ b/lib/portage/package/ebuild/doebuild.py
@@ -1,4 +1,4 @@
-# Copyright 2010-2018 Gentoo Foundation
+# Copyright 2010-2018 Gentoo Authors
# Distributed under the terms of the GNU General Public License v2
from __future__ import unicode_literals
@@ -152,6 +152,7 @@ def _doebuild_spawn(phase, settings, actionmap=None,
**kwargs):
kwargs['networked'] = 'network-sandbox' not in settings.features or \
phase in _networked_phases or \
'network-sandbox' in settings['PORTAGE_RESTRICT'].split()
+ kwargs['pidns'] = 'pid-sandbox' in settings.features
if phase == 'depend':
kwargs['droppriv'] = 'userpriv' in settings.features
@@ -1482,7 +1483,7 @@ def _validate_deps(mysettings, myroot, mydo, mydbapi):
# XXX Issue: cannot block execution. Deadlock condition.
def spawn(mystring, mysettings, debug=False, free=False, droppriv=False,
sesandbox=False, fakeroot=False, networked=True, ipc=True,
- mountns=False, **keywords):
+ mountns=False, pidns=False, **keywords):
"""
Spawn a subprocess with extra portage-specific options.
Optiosn include:
@@ -1518,6 +1519,8 @@ def spawn(mystring, mysettings, debug=False, free=False,
droppriv=False,
@type ipc: Boolean
@param mountns: Run this command inside mount namespace
@type mountns: Boolean
+ @param pidns: Run this command in isolated PID namespace
+ @type pidns: Boolean
@param keywords: Extra options encoded as a dict, to be passed to spawn
@type keywords: Dictionary
@rtype: Integer
@@ -1551,6 +1554,7 @@ def spawn(mystring, mysettings, debug=False, free=False,
droppriv=False,
keywords['unshare_net'] = not networked
keywords['unshare_ipc'] = not ipc
keywords['unshare_mount'] = mountns
+ keywords['unshare_pid'] = pidns
if not networked and mysettings.get("EBUILD_PHASE") !=
"nofetch" and \
("network-sandbox-proxy" in features or "distcc" in
features):
diff --git a/lib/portage/process.py b/lib/portage/process.py
index e2ad89b43..beeb09529 100644
--- a/lib/portage/process.py
+++ b/lib/portage/process.py
@@ -223,7 +223,8 @@ def spawn(mycommand, env={}, opt_name=None, fd_pipes=None,
returnpid=False,
uid=None, gid=None, groups=None, umask=None, logfile=None,
path_lookup=True, pre_exec=None,
close_fds=(sys.version_info < (3, 4)), unshare_net=False,
- unshare_ipc=False, unshare_mount=False, cgroup=None):
+ unshare_ipc=False, unshare_mount=False, unshare_pid=False,
+ cgroup=None):
"""
Spawns a given command.
@@ -264,6 +265,8 @@ def spawn(mycommand, env={}, opt_name=None, fd_pipes=None,
returnpid=False,
@param unshare_mount: If True, mount namespace will be unshared and
mounts will
be private to the namespace
@type unshare_mount: Boolean
+ @param unshare_pid: If True, PID ns will be unshared from the spawned
process
+ @type unshare_pid: Boolean
@param cgroup: CGroup path to bind the process to
@type cgroup: String
@@ -332,7 +335,7 @@ def spawn(mycommand, env={}, opt_name=None, fd_pipes=None,
returnpid=False,
# This caches the libc library lookup in the current
# process, so that it's only done once rather than
# for each child process.
- if unshare_net or unshare_ipc or unshare_mount:
+ if unshare_net or unshare_ipc or unshare_mount or unshare_pid:
find_library("c")
# Force instantiation of portage.data.userpriv_groups before the
@@ -348,7 +351,8 @@ def spawn(mycommand, env={}, opt_name=None, fd_pipes=None,
returnpid=False,
try:
_exec(binary, mycommand, opt_name, fd_pipes,
