Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1647121 > unrolled thread

[RFC][PATCH 0/9] Make containers kernel objects

Back to article view | Back to linux.kernel

Contents

  [RFC][PATCH 0/9] Make containers kernel objects David Howells <dhowells@redhat.com> - 2017-05-22 18:30 +0200
    [PATCH 8/9] Honour CONTAINER_NEW_EMPTY_FS_NS David Howells <dhowells@redhat.com> - 2017-05-22 18:30 +0200
    [PATCH 3/9] Provide /proc/containers David Howells <dhowells@redhat.com> - 2017-05-22 18:30 +0200
    Re: [RFC][PATCH 0/9] Make containers kernel objects James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-05-22 19:00 +0200
      Re: [RFC][PATCH 0/9] Make containers kernel objects Aleksa Sarai <asarai@suse.de> - 2017-05-22 19:20 +0200
        Re: [RFC][PATCH 0/9] Make containers kernel objects David Howells <dhowells@redhat.com> - 2017-05-23 17:00 +0200
          Re: [RFC][PATCH 0/9] Make containers kernel objects ebiederm@xmission.com (Eric W. Biederman) - 2017-05-23 17:10 +0200
            Re: [RFC][PATCH 0/9] Make containers kernel objects David Howells <dhowells@redhat.com> - 2017-05-23 17:20 +0200
              Re: [RFC][PATCH 0/9] Make containers kernel objects ebiederm@xmission.com (Eric W. Biederman) - 2017-05-23 17:30 +0200
                Re: [RFC][PATCH 0/9] Make containers kernel objects James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-05-23 17:50 +0200
                  Re: [RFC][PATCH 0/9] Make containers kernel objects David Howells <dhowells@redhat.com> - 2017-05-23 18:40 +0200
                    Re: [RFC][PATCH 0/9] Make containers kernel objects ebiederm@xmission.com (Eric W. Biederman) - 2017-05-24 10:40 +0200
                      Re: [RFC][PATCH 0/9] Make containers kernel objects Ian Kent <raven@themaw.net> - 2017-05-24 11:20 +0200
      Re: [RFC][PATCH 0/9] Make containers kernel objects Jessica Frazelle <me@jessfraz.com> - 2017-05-22 19:30 +0200
      Re: [RFC][PATCH 0/9] Make containers kernel objects Jeff Layton <jlayton@redhat.com> - 2017-05-22 20:40 +0200
        Re: [RFC][PATCH 0/9] Make containers kernel objects James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-05-22 21:30 +0200
          Re: [RFC][PATCH 0/9] Make containers kernel objects Jeff Layton <jlayton@redhat.com> - 2017-05-23 00:20 +0200
          Re: [RFC][PATCH 0/9] Make containers kernel objects Ian Kent <raven@themaw.net> - 2017-05-23 12:40 +0200
      Re: [RFC][PATCH 0/9] Make containers kernel objects Ian Kent <raven@themaw.net> - 2017-05-23 11:40 +0200
      Re: [RFC][PATCH 0/9] Make containers kernel objects David Howells <dhowells@redhat.com> - 2017-05-23 16:00 +0200
        Re: [RFC][PATCH 0/9] Make containers kernel objects James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-05-23 17:10 +0200
        Re: [RFC][PATCH 0/9] Make containers kernel objects ebiederm@xmission.com (Eric W. Biederman) - 2017-05-23 17:40 +0200
    Re: [RFC][PATCH 0/9] Make containers kernel objects Jessica Frazelle <me@jessfraz.com> - 2017-05-22 19:20 +0200
      Re: [RFC][PATCH 0/9] Make containers kernel objects David Howells <dhowells@redhat.com> - 2017-05-23 17:20 +0200
    Re: [RFC][PATCH 0/9] Make containers kernel objects ebiederm@xmission.com (Eric W. Biederman) - 2017-05-22 21:20 +0200
      Re: [RFC][PATCH 0/9] Make containers kernel objects Jeff Layton <jlayton@redhat.com> - 2017-05-23 00:30 +0200
        Re: [RFC][PATCH 0/9] Make containers kernel objects ebiederm@xmission.com (Eric W. Biederman) - 2017-05-23 15:10 +0200
          Re: [RFC][PATCH 0/9] Make containers kernel objects Jeff Layton <jlayton@redhat.com> - 2017-05-23 16:30 +0200
          Re: [RFC][PATCH 0/9] Make containers kernel objects Djalal Harouni <tixxdz@gmail.com> - 2017-05-23 16:40 +0200
            Re: [RFC][PATCH 0/9] Make containers kernel objects Colin Walters <walters@verbum.org> - 2017-05-23 17:00 +0200
              Re: [RFC][PATCH 0/9] Make containers kernel objects Colin Walters <walters@verbum.org> - 2017-05-23 17:40 +0200
              Re: [RFC][PATCH 0/9] Make containers kernel objects David Howells <dhowells@redhat.com> - 2017-05-23 17:40 +0200
              Re: [RFC][PATCH 0/9] Make containers kernel objects Jeff Layton <jlayton@redhat.com> - 2017-05-23 17:40 +0200
        Re: [RFC][PATCH 0/9] Make containers kernel objects Djalal Harouni <tixxdz@gmail.com> - 2017-05-23 16:30 +0200
      Re: [RFC][PATCH 0/9] Make containers kernel objects David Howells <dhowells@redhat.com> - 2017-05-23 18:20 +0200
    Re: [RFC][PATCH 0/9] Make containers kernel objects Ian Kent <raven@themaw.net> - 2017-05-23 12:20 +0200
    Re: [RFC][PATCH 0/9] Make containers kernel objects ebiederm@xmission.com (Eric W. Biederman) - 2017-05-23 17:40 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1648172

On Tue, 2017-05-23 at 14:52 +0100, David Howells wrote:
> James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> 
> > This sounds like a step in the wrong direction: the strength of the
> > current container interfaces in Linux is that people who set up
> > containers don't have to agree what they look like.
> 
> It may be a strength, but it is also a problem.
> 
> > So I can set up a user namespace without a mount namespace or an
> > architecture emulation container with only a mount namespace.
> 
> (I presume you mean with only the mount namespace separate)
> 
> Yep.  You can do that with this too.
> 
> > But ignoring my fun foibles with containers and to give a concrete
> > example in terms of a popular orchestration system: in kubernetes,
> > where certain namespaces are shared across pods, do you imagine the
> > kernel's view of the "container" to be the pod or what kubernetes
> > thinks of as the container?
> 
> Why not both?  If the net_ns is created in the pod container, then
> probably
> network-related upcalls should be directed there.  Unless instructed
> otherwise, upon creation a container object will inherit the caller's
> namespaces.

The pod isn't a container, it's a collection of containers.  Lets say
each container has a separate mount namespace but shares a network
namespace (this is a gross simplification, there are many other ways
you can set up a pod, but this one illustrates the point).  For your
upcall you'd have to pick a kubernetes container and you don't have the
information to do that, even with your current patches, because what
kubernetes has done.  This is where your view of "container" doesn't
match the kubernetes view.

> > This is important, because half the examples you give below are 
> > network related and usually pods share a network namespace.
> 
> Yeah - I'm more familiar with upcalls made by NFS, AFS and keyrings.

OK, so rather than getting into the technical back and forth below can
we agree that the kernel can't have a unitary view of "container"
because the current use cases (the orchestration systems) don't have
one?  Then the next step becomes how can we add an abstraction that
gives you what you want (as far as I can tell basically identifying a
set of namespaces for an upcall) in a way that doesn't bind the kernel
to have a unitary view of a container? And then we can tack the ideas
on to the Jeff/Eric subthread.

James

[toc] | [prev] | [next] | [standalone]

#1648201

David Howells <dhowells@redhat.com> writes:

> Another thing that could be useful is a list of what device files a container
> may access, so that we can allow limited mounting by the container root user
> within the container.

That is totally not why that isn't allowed, and won't be allowed any
time soon.

The issue is that the filesystem implementations in the kernel are not
prepared to handle hostile filesystem data structures so that that is
the definition of a kernel exploit.  The attack surface of the kernel
gets quite a bit larger in that case.

Perhaps if all of the filesystems data structures had a hmac on them we
could allow something like this.

Once we can make it safe it is easy to add an appropriate interface.  We
most defintiely don't need a ``container'' data structure in the kernel
to do that.

A completely unprivileged fuse is much more likely to work for this use
case.

And we do already have have the device cgroup which sort of does
this.

Eric

[toc] | [prev] | [next] | [standalone]

#1647204

This is interesting...

Adding a container object seems a bit odd to me because there are so
many different ways to make containers, aka not all namespaces are
always used as well as not all cgroups, various LSM objects sometimes
apply, mounts blah blah blah. The OCI spec was made to cover all these
things so why a kernel object? I don't exactly see a future where the
container runtimes convert to this unless it covers all the same mods
as the mods in the OCI spec, not saying it needs to abide by the spec,
just saying it should allow all the same things. Which really just
seems, imo, like a pain for the kernel to have to maintain.

On Mon, May 22, 2017 at 5:22 PM, David Howells <dhowells@redhat.com> wrote:
>
> Here are a set of patches to define a container object for the kernel and
> to provide some methods to create and manipulate them.
>
> The reason I think this is necessary is that the kernel has no idea how to
> direct upcalls to what userspace considers to be a container - current
> Linux practice appears to make a "container" just an arbitrarily chosen
> junction of namespaces, control groups and files, which may be changed
> individually within the "container".
>
> The kernel upcall mechanism then needs to decide which set of namespaces,
> etc., it must exec the appropriate upcall program.  Examples of this
> include:
>
>  (1) The DNS resolver.  The DNS cache in the kernel should probably be
>      per-network namespace, but in userspace the program, its libraries and
>      its config data are associated with a mount tree and a user namespace
>      and it gets run in a particular pid namespace.
>
>  (2) NFS ID mapper.  The NFS ID mapping cache should also probably be
>      per-network namespace.
>
>  (3) nfsdcltrack.  A way for NFSD to access stable storage for tracking
>      of persistent state.  Again, network-namespace dependent, but also
>      perhaps mount-namespace dependent.
>
>  (4) General request-key upcalls.  Not particularly namespace dependent,
>      apart from keyrings being somewhat governed by the user namespace and
>      the upcall being configured by the mount namespace.

Can't these all become namespace-aware without adding the notion of a
"container" to the kernel.

>
> These patches are built on top of the mount context patchset so that
> namespaces can be properly propagated over submounts/automounts.
>
> These patches implement a container object that holds the following things:
>
>  (1) Namespaces.
>
>  (2) A root directory.
>
>  (3) A set of processes, including a designated 'init' process.
>
>  (4) The creator's credentials, including ownership.
>
>  (5) A place to hang security for the container, allowing policies to be
>      set per-container.
>
> I also want to add:
>
>  (6) Control groups.
>
>  (7) A per-container keyring that can be added to from outside of the
>      container, even once the container is live, for the provision of
>      filesystem authentication/encryption keys in advance of the container
>      being started.
>
> You can get a list of containers by examining /proc/containers - but I'm
> not sure how much value this gets you.  Note that the container in which
> you are running is called "<current>" and you can only see other containers
> that were started from within yours.  Containers are therefore effectively
> hierarchical and an init_container is set up when the system boots.
>
>
> Some management operations are provided:
>
>  (1) int fd = container_create(const char *name, unsigned int flags);
>
>      Create a container of the given name and return a handle to it as a
>      file descriptor.  flags indicates what namespaces should be inherited
>      from the caller and what should be replaced new.  It is possible to
>      set up a container with a null root filesystem that can be mounted
>      later.
>
>  (2) int fsfd = fsopen(const char *fsname, int container_fd,
>                        unsigned int flags);
>
>      Prepare a mount context inside the container.  This uses all the
>      containers namespaces instead of the caller's.
>
>  (3) fsmount(int fsfd, int dfd, const char *path, unsigned int at_flags,
>              unsigned int flags);
>
>      Mount a prepared superblock.  dfd can be given container_fd to use the
>      container to which it refers as the root of the pathwalk.
>
>      If path is "/" and at_flags is AT_FSMOUNT_CONTAINER_ROOT, then this
>      will attempt to mount the root of the container and create a mount
>      namespace for it.  The container must've been created with
>      CONTAINER_NEW_EMPTY_FS_NS.
>
>  (4) pid_t pid = fork_into_container(int container_fd);
>
>      Create the init process in a container.  The process uses that
>      container's namespaces instead of the caller's.
>
>  (5) int sfd = container_socket(int container_fd,
>                                 int domain, int type, int protocol);
>
>      Create a socket inside a container.  The socket gets the container's
>      namespaces.  This allows netlink operations to be called within that
>      container to set it up from outside (at least in theory).
>
>  (6) mkdirat(int dfd, ...);
>      mknodat(int dfd, ...);
>      openat(int dfd, ...);
>
>      Supplying a container fd as dfd makes the pathwalk happen relative to
>      the root of the container.  Note that the path must be *relative*.
>
> And some need to be/could be added:
>
>  (7) Directly set a container's namespaces to allow cross-container
>      sharing.
>
>  (8) Adjust the control group membership of a container.
>
>  (9) Add a key inside a container keyring.
>
> (10) Kill/suspend/freeze/reboot container, both from inside and out.
>
> (11) Set container's root dir.
>
> (12) Set the container's security policy.
>
> (13) Allow overlayfs to access filesystems outside of the container in
>      which it is being created.
>
>
> Kernel upcalls are invoked in the root of the container that incurs them
> rather than in the init namespace context.  There's still some awkwardness
> here if you, say, share a network namespace between containers.  Either the
> upcall binaries and configuration must be duplicated between sharing
> containers or a container must be elected as the one in which such upcalls
> will be done.
>
>
> Some further thoughts:
>
>  (*) Should there be an AT_IN_CONTAINER flag to provide to syscalls that
>      take a container in lieu of AT_FDCWD or a directory fd?  The problem
>      is that such as mkdirat() and openat() don't have an at_flags
>      argument.
>
>  (*) Should there be a container hierarchy at all?  It seems that this is
>      only really necessary for /proc/containers.  Do we want to allow
>      containers-within-containers?
>
>  (*) Should each container automatically have its own pid namespace such
>      that its 'init' process always appears as pid 1?
>
>  (*) Does this allow kernel upcalls to be accounted against the correct
>      control group?
>
>  (*) Should each container have a 'list' of accessible device numbers such
>      that certain device files can be made usable within a container?  And
>      can devtmpfs/udev be made to show the correct file set for each
>      container?
>
>
> The patches can be found here also:
>
>         http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=container
>
> Note that this is dependent on the mount-context branch.
>
> David
> ---
> David Howells (9):
>       containers: Rename linux/container.h to linux/container_dev.h
>       Implement containers as kernel objects
>       Provide /proc/containers
>       Allow processes to be forked and upcalled into a container
>       Open a socket inside a container
>       Allow fs syscall dfd arguments to take a container fd
>       Make fsopen() able to initiate mounting into a container
>       Honour CONTAINER_NEW_EMPTY_FS_NS
>       Sample program for driving container objects
>
>
>  arch/x86/entry/syscalls/syscall_32.tbl |    3
>  arch/x86/entry/syscalls/syscall_64.tbl |    3
>  drivers/acpi/container.c               |    2
>  drivers/base/container.c               |    2
>  fs/fsopen.c                            |   33 +-
>  fs/libfs.c                             |    3
>  fs/namei.c                             |   52 ++-
>  fs/namespace.c                         |  108 +++++-
>  fs/nfs/namespace.c                     |    2
>  fs/nfs/nfs4namespace.c                 |    4
>  fs/proc/root.c                         |   13 +
>  fs/sb_config.c                         |   29 +-
>  include/linux/container.h              |   91 ++++-
>  include/linux/container_dev.h          |   25 +
>  include/linux/cred.h                   |    3
>  include/linux/init_task.h              |    4
>  include/linux/kmod.h                   |    1
>  include/linux/lsm_hooks.h              |   25 +
>  include/linux/mount.h                  |    5
>  include/linux/nsproxy.h                |    7
>  include/linux/pid.h                    |    5
>  include/linux/proc_ns.h                |    3
>  include/linux/sb_config.h              |    5
>  include/linux/sched.h                  |    3
>  include/linux/sched/task.h             |    4
>  include/linux/security.h               |   20 +
>  include/linux/syscalls.h               |    6
>  include/uapi/linux/container.h         |   28 ++
>  include/uapi/linux/fcntl.h             |    2
>  include/uapi/linux/magic.h             |    1
>  init/Kconfig                           |    7
>  init/main.c                            |    4
>  kernel/Makefile                        |    2
>  kernel/container.c                     |  576 ++++++++++++++++++++++++++++++++
>  kernel/cred.c                          |   45 ++-
>  kernel/exit.c                          |    1
>  kernel/fork.c                          |  117 ++++++-
>  kernel/kmod.c                          |   13 +
>  kernel/kthread.c                       |    3
>  kernel/namespaces.h                    |   15 +
>  kernel/nsproxy.c                       |   34 +-
>  kernel/pid.c                           |    4
>  kernel/sys_ni.c                        |    5
>  net/socket.c                           |   37 ++
>  samples/containers/test-container.c    |  162 +++++++++
>  security/security.c                    |   18 +
>  security/selinux/hooks.c               |    5
>  47 files changed, 1408 insertions(+), 132 deletions(-)
>  create mode 100644 include/linux/container_dev.h
>  create mode 100644 include/uapi/linux/container.h
>  create mode 100644 kernel/container.c
>  create mode 100644 kernel/namespaces.h
>  create mode 100644 samples/containers/test-container.c
>
> --
> To unsubscribe from this list: send the line "unsubscribe cgroups" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 


Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC  511E 18F3 685C 0022 BFF3
pgp.mit.edu

[toc] | [prev] | [next] | [standalone]

#1648180

Jessica Frazelle <me@jessfraz.com> wrote:

> Adding a container object seems a bit odd to me because there are so
> many different ways to make containers, aka not all namespaces are
> always used

This is already dealt with to some extent.  It can create/inherit namespaces
like fork - except that you get an extra option (literally, create with no
mount namespace and create that when you provide a root mount).

Modifying the namespace subscriptions is on the to-do list.

> as well as not all cgroups,

cgroups are on the to-do list.

> various LSM objects sometimes apply,

I added a hook for the LSM to use.

> mounts blah blah blah.

You can mount into the container and you can create sockets in the container
from outside the container.

> The OCI spec

This?

	https://github.com/opencontainers/runtime-spec/blob/master/README.md

> was made to cover all these things so why a kernel object?

Because there are some things the kernel doesn't do that it should (upcalling
into the correct namespace junction for example), and some things I've been
asked to add for which there's no clear place to do so.

> I don't exactly see a future where the container runtimes convert to this
> unless it covers all the same mods as the mods in the OCI spec, not saying
> it needs to abide by the spec, just saying it should allow all the same
> things.

I haven't looked at the OCI spec as yet.

Note that this is *not* a replacement for a container application.  I'm not
trying to deprecate Docker or whatever.  It's something for those container
applications to use.

> Which really just seems, imo, like a pain for the kernel to have to
> maintain.

Namespaces are a pain, particularly as lots of things exist in more than one
of the things.

David

[toc] | [prev] | [next] | [standalone]

#1647281

David Howells <dhowells@redhat.com> writes:

> Here are a set of patches to define a container object for the kernel and
> to provide some methods to create and manipulate them.
>
> The reason I think this is necessary is that the kernel has no idea how to
> direct upcalls to what userspace considers to be a container - current
> Linux practice appears to make a "container" just an arbitrarily chosen
> junction of namespaces, control groups and files, which may be changed
> individually within the "container".
>

I think this might possibly be a useful abstraction for solving the
keyring upcalls if it was something created implicitly.

fork_into_container for use by keyring upcalls is currently a security
vulnerability as it allows escaping all of a containers cgroups.  But
you have that on your list of things to fix.  However you don't have
seccomp and a few other things.

Before we had kthreadd in the kernel upcalls always had issues because
the code to reset all of the userspace bits and make the forked
task suitable for running upcalls was always missing some detail.  It is
a very bug-prone kind of idiom that you are talking about.  It is doubly
bug-prone because the wrongness is visible to userspace and as such
might get become a frozen KABI guarantee.

Let me suggest a concrete alternative:

- At the time of mount observer the mounters user namespace.
- Find the mounters pid namespace.
- If the mounters pid namespace is owned by the mounters user namespace
  walk up the pid namespace tree to the first pid namespace owned by
  that user namespace.
- If the mounters pid namespace is not owned by the mounters user
  namespace fail the mount it is going to need to make upcalls as
  will not be possible.
- Hold a reference to the pid namespace that was found.

Then when an upcall needs to be made fork a child of the init process
of the specified pid namespace.  Or fail if the init process of the
pid namespace has died.

That should always work and it does not require keeping expensive state
where we did not have it previously.  Further because the semantics are
fork a child of a particular pid namespace's init as features get added
to the kernel this code remains well defined.

For ordinary request-key upcalls we should be able to use the same rules
and just not save/restore things in the kernel.

A huge advantage of my alternative (other than not being a bit-rot
magnet) is that it should drop into existing container infrastructure
without problems.  The rule for container implementors is simple to use
security key infrastructure you need to have created a pid namespace in
your user namespace.

Eric

[toc] | [prev] | [next] | [standalone]

#1647442

On Mon, 2017-05-22 at 14:04 -0500, Eric W. Biederman wrote:
> David Howells <dhowells@redhat.com> writes:
> 
> > Here are a set of patches to define a container object for the kernel and
> > to provide some methods to create and manipulate them.
> > 
> > The reason I think this is necessary is that the kernel has no idea how to
> > direct upcalls to what userspace considers to be a container - current
> > Linux practice appears to make a "container" just an arbitrarily chosen
> > junction of namespaces, control groups and files, which may be changed
> > individually within the "container".
> > 
> 
> I think this might possibly be a useful abstraction for solving the
> keyring upcalls if it was something created implicitly.
> 
> fork_into_container for use by keyring upcalls is currently a security
> vulnerability as it allows escaping all of a containers cgroups.  But
> you have that on your list of things to fix.  However you don't have
> seccomp and a few other things.
> 
> Before we had kthreadd in the kernel upcalls always had issues because
> the code to reset all of the userspace bits and make the forked
> task suitable for running upcalls was always missing some detail.  It is
> a very bug-prone kind of idiom that you are talking about.  It is doubly
> bug-prone because the wrongness is visible to userspace and as such
> might get become a frozen KABI guarantee.
> 
> Let me suggest a concrete alternative:
> 
> - At the time of mount observer the mounters user namespace.
> - Find the mounters pid namespace.
> - If the mounters pid namespace is owned by the mounters user namespace
>   walk up the pid namespace tree to the first pid namespace owned by
>   that user namespace.
> - If the mounters pid namespace is not owned by the mounters user
>   namespace fail the mount it is going to need to make upcalls as
>   will not be possible.
> - Hold a reference to the pid namespace that was found.
> 
> Then when an upcall needs to be made fork a child of the init process
> of the specified pid namespace.  Or fail if the init process of the
> pid namespace has died.
> 
> That should always work and it does not require keeping expensive state
> where we did not have it previously.  Further because the semantics are
> fork a child of a particular pid namespace's init as features get added
> to the kernel this code remains well defined.
> 
> For ordinary request-key upcalls we should be able to use the same rules
> and just not save/restore things in the kernel.
> 

OK, that does seem like a reasonable idea. Note that it's not just
request-key upcalls here that we're interested in, but anything that
we'd typically spawn from kthreadd otherwise.

That said, I worry a little about this. If the init process does a setns
at the wrong time, suddenly you're doing the upcall in different
namespaces than you intended.

Might it be better to use the init process of the container as the
template like you suggest, but snapshot its "context" at a particular
point in time instead?

knfsd could do this when it's started, for instance...


> A huge advantage of my alternative (other than not being a bit-rot
> magnet) is that it should drop into existing container infrastructure
> without problems.  The rule for container implementors is simple to use
> security key infrastructure you need to have created a pid namespace in
> your user namespace.
> 
> Eric

-- 
Jeff Layton <jlayton@redhat.com>

[toc] | [prev] | [next] | [standalone]

#1648015

Jeff Layton <jlayton@redhat.com> writes:

> On Mon, 2017-05-22 at 14:04 -0500, Eric W. Biederman wrote:
>> David Howells <dhowells@redhat.com> writes:
>> 
>> > Here are a set of patches to define a container object for the kernel and
>> > to provide some methods to create and manipulate them.
>> > 
>> > The reason I think this is necessary is that the kernel has no idea how to
>> > direct upcalls to what userspace considers to be a container - current
>> > Linux practice appears to make a "container" just an arbitrarily chosen
>> > junction of namespaces, control groups and files, which may be changed
>> > individually within the "container".
>> > 
>> 
>> I think this might possibly be a useful abstraction for solving the
>> keyring upcalls if it was something created implicitly.
>> 
>> fork_into_container for use by keyring upcalls is currently a security
>> vulnerability as it allows escaping all of a containers cgroups.  But
>> you have that on your list of things to fix.  However you don't have
>> seccomp and a few other things.
>> 
>> Before we had kthreadd in the kernel upcalls always had issues because
>> the code to reset all of the userspace bits and make the forked
>> task suitable for running upcalls was always missing some detail.  It is
>> a very bug-prone kind of idiom that you are talking about.  It is doubly
>> bug-prone because the wrongness is visible to userspace and as such
>> might get become a frozen KABI guarantee.
>> 
>> Let me suggest a concrete alternative:
>> 
>> - At the time of mount observer the mounters user namespace.
>> - Find the mounters pid namespace.
>> - If the mounters pid namespace is owned by the mounters user namespace
>>   walk up the pid namespace tree to the first pid namespace owned by
>>   that user namespace.
>> - If the mounters pid namespace is not owned by the mounters user
>>   namespace fail the mount it is going to need to make upcalls as
>>   will not be possible.
>> - Hold a reference to the pid namespace that was found.
>> 
>> Then when an upcall needs to be made fork a child of the init process
>> of the specified pid namespace.  Or fail if the init process of the
>> pid namespace has died.
>> 
>> That should always work and it does not require keeping expensive state
>> where we did not have it previously.  Further because the semantics are
>> fork a child of a particular pid namespace's init as features get added
>> to the kernel this code remains well defined.
>> 
>> For ordinary request-key upcalls we should be able to use the same rules
>> and just not save/restore things in the kernel.
>> 
>
> OK, that does seem like a reasonable idea. Note that it's not just
> request-key upcalls here that we're interested in, but anything that
> we'd typically spawn from kthreadd otherwise.

General user mode helper *Nod*.

> That said, I worry a little about this. If the init process does a setns
> at the wrong time, suddenly you're doing the upcall in different
> namespaces than you intended.
>
> Might it be better to use the init process of the container as the
> template like you suggest, but snapshot its "context" at a particular
> point in time instead?
>
> knfsd could do this when it's started, for instance...

The danger of a snapshot it time is something important (like cgroup
membership) might change.

It might be necessary to have this be an opt-in.   Perhaps even to the
point of starting a dedicated kthreadd.

Right now I think we need to figure out what it will take to solve this
in the kernel because I strongly suspect that solving this in userspace
is a cop out and we really aren't providing enough information to
userspace to run the helper in the proper context.    And I strongly
suspect that providing enough information from the kernel will be
roughly equivalent to solving this in the kernel.

The only big issue I have had with the suggestion of a dedicated thread
in the past is the overhead something like that will breing with it.

Eric

[toc] | [prev] | [next] | [standalone]

#1648127

On Tue, 2017-05-23 at 07:54 -0500, Eric W. Biederman wrote:
> Jeff Layton <jlayton@redhat.com> writes:
> 
> > On Mon, 2017-05-22 at 14:04 -0500, Eric W. Biederman wrote:
> > > David Howells <dhowells@redhat.com> writes:
> > > 
> > > > Here are a set of patches to define a container object for the kernel and
> > > > to provide some methods to create and manipulate them.
> > > > 
> > > > The reason I think this is necessary is that the kernel has no idea how to
> > > > direct upcalls to what userspace considers to be a container - current
> > > > Linux practice appears to make a "container" just an arbitrarily chosen
> > > > junction of namespaces, control groups and files, which may be changed
> > > > individually within the "container".
> > > > 
> > > 
> > > I think this might possibly be a useful abstraction for solving the
> > > keyring upcalls if it was something created implicitly.
> > > 
> > > fork_into_container for use by keyring upcalls is currently a security
> > > vulnerability as it allows escaping all of a containers cgroups.  But
> > > you have that on your list of things to fix.  However you don't have
> > > seccomp and a few other things.
> > > 
> > > Before we had kthreadd in the kernel upcalls always had issues because
> > > the code to reset all of the userspace bits and make the forked
> > > task suitable for running upcalls was always missing some detail.  It is
> > > a very bug-prone kind of idiom that you are talking about.  It is doubly
> > > bug-prone because the wrongness is visible to userspace and as such
> > > might get become a frozen KABI guarantee.
> > > 
> > > Let me suggest a concrete alternative:
> > > 
> > > - At the time of mount observer the mounters user namespace.
> > > - Find the mounters pid namespace.
> > > - If the mounters pid namespace is owned by the mounters user namespace
> > >   walk up the pid namespace tree to the first pid namespace owned by
> > >   that user namespace.
> > > - If the mounters pid namespace is not owned by the mounters user
> > >   namespace fail the mount it is going to need to make upcalls as
> > >   will not be possible.
> > > - Hold a reference to the pid namespace that was found.
> > > 
> > > Then when an upcall needs to be made fork a child of the init process
> > > of the specified pid namespace.  Or fail if the init process of the
> > > pid namespace has died.
> > > 
> > > That should always work and it does not require keeping expensive state
> > > where we did not have it previously.  Further because the semantics are
> > > fork a child of a particular pid namespace's init as features get added
> > > to the kernel this code remains well defined.
> > > 
> > > For ordinary request-key upcalls we should be able to use the same rules
> > > and just not save/restore things in the kernel.
> > > 
> > 
> > OK, that does seem like a reasonable idea. Note that it's not just
> > request-key upcalls here that we're interested in, but anything that
> > we'd typically spawn from kthreadd otherwise.
> 
> General user mode helper *Nod*.
> 
> > That said, I worry a little about this. If the init process does a setns
> > at the wrong time, suddenly you're doing the upcall in different
> > namespaces than you intended.
> > 
> > Might it be better to use the init process of the container as the
> > template like you suggest, but snapshot its "context" at a particular
> > point in time instead?
> > 
> > knfsd could do this when it's started, for instance...
> 
> The danger of a snapshot it time is something important (like cgroup
> membership) might change.
> 

This is also a problem with relying on the userland program to do a
setns() and whatnot to set itself up for running in the container. If
something is added that it doesn't know about you'll just end up
inheriting whatever kthreadd had. If we don't get that right, we can end
up giving userland a security hole.

> It might be necessary to have this be an opt-in.   Perhaps even to the
> point of starting a dedicated kthreadd.
> 

I think we could live with that in knfsd-land. We could spawn a kthreadd
thread whenever a new nfsd_net is created. Then we'd just need something
like call_usermodehelper that puts the task create request on the right
kthreadd list. Running one more thread in your containerized NFS server
shouldn't be too onerous, I wouldn't think.

Once we start getting into uses with keyrings and the like though, I'm
not sure how workable that would be.

> Right now I think we need to figure out what it will take to solve this
> in the kernel because I strongly suspect that solving this in userspace
> is a cop out and we really aren't providing enough information to
> userspace to run the helper in the proper context.    And I strongly
> suspect that providing enough information from the kernel will be
> roughly equivalent to solving this in the kernel.
> 
> The only big issue I have had with the suggestion of a dedicated thread
> in the past is the overhead something like that will breing with it.
> 

Yes, I don't see how you can do these sorts of upcalls properly without
either more help from the kernel, or without providing the kernel with
enough info to do it properly.

I don't quite get the arguments that have been made about loss of
flexibility either. The basic idea here is to communicate to the kernel
how a container is structured so that it can spawn processes inside of
it as necessary.
-- 
Jeff Layton <jlayton@redhat.com>

[toc] | [prev] | [next] | [standalone]

#1648140

On Tue, May 23, 2017 at 2:54 PM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> Jeff Layton <jlayton@redhat.com> writes:
>
>> On Mon, 2017-05-22 at 14:04 -0500, Eric W. Biederman wrote:
>>> David Howells <dhowells@redhat.com> writes:
>>>
>>> > Here are a set of patches to define a container object for the kernel and
>>> > to provide some methods to create and manipulate them.
>>> >
>>> > The reason I think this is necessary is that the kernel has no idea how to
>>> > direct upcalls to what userspace considers to be a container - current
>>> > Linux practice appears to make a "container" just an arbitrarily chosen
>>> > junction of namespaces, control groups and files, which may be changed
>>> > individually within the "container".
>>> >
>>>
>>> I think this might possibly be a useful abstraction for solving the
>>> keyring upcalls if it was something created implicitly.
>>>
>>> fork_into_container for use by keyring upcalls is currently a security
>>> vulnerability as it allows escaping all of a containers cgroups.  But
>>> you have that on your list of things to fix.  However you don't have
>>> seccomp and a few other things.
>>>
>>> Before we had kthreadd in the kernel upcalls always had issues because
>>> the code to reset all of the userspace bits and make the forked
>>> task suitable for running upcalls was always missing some detail.  It is
>>> a very bug-prone kind of idiom that you are talking about.  It is doubly
>>> bug-prone because the wrongness is visible to userspace and as such
>>> might get become a frozen KABI guarantee.
>>>
>>> Let me suggest a concrete alternative:
>>>
>>> - At the time of mount observer the mounters user namespace.
>>> - Find the mounters pid namespace.
>>> - If the mounters pid namespace is owned by the mounters user namespace
>>>   walk up the pid namespace tree to the first pid namespace owned by
>>>   that user namespace.
>>> - If the mounters pid namespace is not owned by the mounters user
>>>   namespace fail the mount it is going to need to make upcalls as
>>>   will not be possible.
>>> - Hold a reference to the pid namespace that was found.
>>>
>>> Then when an upcall needs to be made fork a child of the init process
>>> of the specified pid namespace.  Or fail if the init process of the
>>> pid namespace has died.
>>>
>>> That should always work and it does not require keeping expensive state
>>> where we did not have it previously.  Further because the semantics are
>>> fork a child of a particular pid namespace's init as features get added
>>> to the kernel this code remains well defined.
>>>
>>> For ordinary request-key upcalls we should be able to use the same rules
>>> and just not save/restore things in the kernel.
>>>
>>
>> OK, that does seem like a reasonable idea. Note that it's not just
>> request-key upcalls here that we're interested in, but anything that
>> we'd typically spawn from kthreadd otherwise.
>
> General user mode helper *Nod*.
>
>> That said, I worry a little about this. If the init process does a setns
>> at the wrong time, suddenly you're doing the upcall in different
>> namespaces than you intended.
>>
>> Might it be better to use the init process of the container as the
>> template like you suggest, but snapshot its "context" at a particular
>> point in time instead?
>>
>> knfsd could do this when it's started, for instance...
>
> The danger of a snapshot it time is something important (like cgroup
> membership) might change.
>
> It might be necessary to have this be an opt-in.   Perhaps even to the
> point of starting a dedicated kthreadd.
>
> Right now I think we need to figure out what it will take to solve this
> in the kernel because I strongly suspect that solving this in userspace
> is a cop out and we really aren't providing enough information to
> userspace to run the helper in the proper context.    And I strongly
> suspect that providing enough information from the kernel will be
> roughly equivalent to solving this in the kernel.

Maybe it depends on the cases, a general approach can be too difficult
to handle especially from the security point. Maybe it is better to
identify what operations need what context, and a userspace
service/proxy can act using kthreadd with the right context... at
least the shift to this model has been done for years now in the
mobile industry.


-- 
tixxdz

[toc] | [prev] | [next] | [standalone]

#1648158

On Tue, May 23, 2017, at 10:30 AM, Djalal Harouni wrote:
>
> Maybe it depends on the cases, a general approach can be too difficult
> to handle especially from the security point. Maybe it is better to
> identify what operations need what context, and a userspace
> service/proxy can act using kthreadd with the right context... at
> least the shift to this model has been done for years now in the
> mobile industry.

Why not drop the upcall model in favor of having userspace
monitor events via a (more efficient) protocol and react to them on its own?
It's just generally more flexible and avoids all of those issues like
replicating the seccomp configuration, etc.

Something like inotify/signalfd could be a precedent around having a read()/poll()able
fd.  /proc/keys-requests ?

Then if you create a new user namespace, and open /proc/keys-requests, the
kernel will always write to that instead of calling /sbin/request-key.

[toc] | [prev] | [next] | [standalone]

#1648200

On Tue, May 23, 2017, at 11:31 AM, Jeff Layton wrote:
> 
> nfsdcltrack was originally nfsdcld, a long running daemon that used
> rpc_pipefs to talk to the kernel. That meant that you had to make sure
> it gets enabled by systemd (or sysvinit, etc). If it dies, then you also
> want to ensure that it gets restarted lest the kernel server hang,
> etc...
> 
> It was pretty universally hated, as it was just one more daemon that you
> needed to run to work a proper nfs server. So, I was encouraged to
> switch it to a call_usermodehelper upcall and since then it has just
> worked, as long as the binary is installed.

Note that with the "read()/write() fd" model you don't need
a whole process just to do that...the functionality could be rolled into systemd
or equivalent easily enough.

> "You're doing it wrong. You just need to run all of these services as
> long-running daemons."

Also, I imagine we could figure out a clean model to do *activation*
from kernel -> userspace too.  systemd's socket activation model
where pid 1 activates units on demand is quite nice and obviates
the need to configure things on in advance.

[toc] | [prev] | [next] | [standalone]

#1648204

Colin Walters <walters@verbum.org> wrote:

> Why not drop the upcall model in favor of having userspace monitor events
> via a (more efficient) protocol and react to them on its own?

 (1) That's not necessarily more efficient.  You now have the overhead of a
     permanently running userspace daemon in every relevant namespace
     combination.

 (2) You then have to work out how to route to the appropriate daemon.

> It's just generally more flexible

Actually, it's less flexible.  You can't easily get at the caller's
namespaces.

> and avoids all of those issues like replicating the seccomp configuration,
> etc.

So does my container implementation.

> Something like inotify/signalfd could be a precedent around having a read()/poll()able
> fd.  /proc/keys-requests ?
>
> Then if you create a new user namespace, and open /proc/keys-requests, the
> kernel will always write to that instead of calling /sbin/request-key.

That's not good enough.  You're basically making it one daemon per user
namespace and ignoring all the other namespaces.

[Also note that the kernel would have to paste a temporary authorisation key
 into the daemon's session keyring for each key that requires instantiation].

David

[toc] | [prev] | [next] | [standalone]

#1648208

On Tue, 2017-05-23 at 10:54 -0400, Colin Walters wrote:
> On Tue, May 23, 2017, at 10:30 AM, Djalal Harouni wrote:
> > 
> > Maybe it depends on the cases, a general approach can be too difficult
> > to handle especially from the security point. Maybe it is better to
> > identify what operations need what context, and a userspace
> > service/proxy can act using kthreadd with the right context... at
> > least the shift to this model has been done for years now in the
> > mobile industry.
> 
> Why not drop the upcall model in favor of having userspace
> monitor events via a (more efficient) protocol and react to them on its own?
> It's just generally more flexible and avoids all of those issues like
> replicating the seccomp configuration, etc.
> 
> Something like inotify/signalfd could be a precedent around having a read()/poll()able
> fd.  /proc/keys-requests ?
> 
> Then if you create a new user namespace, and open /proc/keys-requests, the
> kernel will always write to that instead of calling /sbin/request-key.

Case in point:

nfsdcltrack was originally nfsdcld, a long running daemon that used
rpc_pipefs to talk to the kernel. That meant that you had to make sure
it gets enabled by systemd (or sysvinit, etc). If it dies, then you also
want to ensure that it gets restarted lest the kernel server hang,
etc...

It was pretty universally hated, as it was just one more daemon that you
needed to run to work a proper nfs server. So, I was encouraged to
switch it to a call_usermodehelper upcall and since then it has just
worked, as long as the binary is installed.

It's quite easy to say:

"You're doing it wrong. You just need to run all of these services as
long-running daemons."

But, that ignores the fact that handling long-running daemons for
infrequently used upcalls is actually quite painful to manage in
practice.

-- 
Jeff Layton <jlayton@redhat.com>

[toc] | [prev] | [next] | [standalone]

#1648133

On Tue, May 23, 2017 at 12:22 AM, Jeff Layton <jlayton@redhat.com> wrote:
> On Mon, 2017-05-22 at 14:04 -0500, Eric W. Biederman wrote:
>> David Howells <dhowells@redhat.com> writes:
>>
>> > Here are a set of patches to define a container object for the kernel and
>> > to provide some methods to create and manipulate them.
>> >
>> > The reason I think this is necessary is that the kernel has no idea how to
>> > direct upcalls to what userspace considers to be a container - current
>> > Linux practice appears to make a "container" just an arbitrarily chosen
>> > junction of namespaces, control groups and files, which may be changed
>> > individually within the "container".
>> >
>>
>> I think this might possibly be a useful abstraction for solving the
>> keyring upcalls if it was something created implicitly.
>>
>> fork_into_container for use by keyring upcalls is currently a security
>> vulnerability as it allows escaping all of a containers cgroups.  But
>> you have that on your list of things to fix.  However you don't have
>> seccomp and a few other things.
>>
>> Before we had kthreadd in the kernel upcalls always had issues because
>> the code to reset all of the userspace bits and make the forked
>> task suitable for running upcalls was always missing some detail.  It is
>> a very bug-prone kind of idiom that you are talking about.  It is doubly
>> bug-prone because the wrongness is visible to userspace and as such
>> might get become a frozen KABI guarantee.
>>
>> Let me suggest a concrete alternative:
>>
>> - At the time of mount observer the mounters user namespace.
>> - Find the mounters pid namespace.
>> - If the mounters pid namespace is owned by the mounters user namespace
>>   walk up the pid namespace tree to the first pid namespace owned by
>>   that user namespace.
>> - If the mounters pid namespace is not owned by the mounters user
>>   namespace fail the mount it is going to need to make upcalls as
>>   will not be possible.
>> - Hold a reference to the pid namespace that was found.
>>
>> Then when an upcall needs to be made fork a child of the init process
>> of the specified pid namespace.  Or fail if the init process of the
>> pid namespace has died.
>>
>> That should always work and it does not require keeping expensive state
>> where we did not have it previously.  Further because the semantics are
>> fork a child of a particular pid namespace's init as features get added
>> to the kernel this code remains well defined.
>>
>> For ordinary request-key upcalls we should be able to use the same rules
>> and just not save/restore things in the kernel.
>>
>
> OK, that does seem like a reasonable idea. Note that it's not just
> request-key upcalls here that we're interested in, but anything that
> we'd typically spawn from kthreadd otherwise.

Generalizing it will expose the kernel to exploits, today containers
setup the mount namespace for images from the net, outdated
filesystems, and users just do it,  it is easy. Having kthread running
inside such contexts is not a good idea. That's today usecases.


> That said, I worry a little about this. If the init process does a setns
> at the wrong time, suddenly you're doing the upcall in different
> namespaces than you intended.

That init process or whatever process inside owns that context and files.

Maybe for some cases it is better to use userspace that you can talk
to through a standard kernel bus endpoint and request a resource as it
is done within modern apps. The application at the other end acts
using kthread helpers in the appropriate context.


-- 
tixxdz

[toc] | [prev] | [next] | [standalone]

#1648245

Eric W. Biederman <ebiederm@xmission.com> wrote:

> Let me suggest a concrete alternative:
> 
> - At the time of mount observer the mounters user namespace.

Looking at sget(), I don't think a mounter can see a superblock outside of
their namespace.  There is something icky in there whereby all automounts are
currently transferred into the init_user_ns though (something to fix in my
mount-context series) :-/

> - Find the mounters pid namespace.
> - If the mounters pid namespace is owned by the mounters user namespace
>   walk up the pid namespace tree to the first pid namespace owned by
>   that user namespace.
> - If the mounters pid namespace is not owned by the mounters user
>   namespace fail the mount it is going to need to make upcalls as
>   will not be possible.

Take the following scenario:

 (1) Create a process with a new network namespace.  Set up the network to
     route out of ethernet port 1.

 (2) Create a child process with new network and user namespaces.  Set up the
     network to route out of ethernet port 2.

 (3) Mount an NFS volume in the process created in (2).

The mount in (3) will fail unconditionally.

> - Hold a reference to the pid namespace that was found.

Take the following scenario:

 (1) Create a process with new network and pid namespaces.  Set up the network
     to route out of ethernet port 1.

 (2) Create a child process with new network and pid namespaces.  Set up the
     network to route out of ethernet port 2.

 (3) Mount an NFS volume in the process created in (2).

 (4) Create another child process with new network and pid namespaces.  Set up
     the network to route out of ethernet port 3.

 (5) In the process created in (4), access the NFS volume created in (3).

The user namespace is the same all the way through.

Now you're holding a ref to the pid namespace created in (1) - but that is of
no use to you.  The upcall must take place in the network namespace that
routes out through port 2.

David

[toc] | [prev] | [next] | [standalone]

#1647914

On Mon, 2017-05-22 at 17:22 +0100, David Howells wrote:
> Here are a set of patches to define a container object for the kernel and
> to provide some methods to create and manipulate them.
> 
> The reason I think this is necessary is that the kernel has no idea how to
> direct upcalls to what userspace considers to be a container - current
> Linux practice appears to make a "container" just an arbitrarily chosen
> junction of namespaces, control groups and files, which may be changed
> individually within the "container".
> 
> The kernel upcall mechanism then needs to decide which set of namespaces,
> etc., it must exec the appropriate upcall program.  Examples of this
> include:
> 
>  (1) The DNS resolver.  The DNS cache in the kernel should probably be
>      per-network namespace, but in userspace the program, its libraries and
>      its config data are associated with a mount tree and a user namespace
>      and it gets run in a particular pid namespace.
> 
>  (2) NFS ID mapper.  The NFS ID mapping cache should also probably be
>      per-network namespace.
> 
>  (3) nfsdcltrack.  A way for NFSD to access stable storage for tracking
>      of persistent state.  Again, network-namespace dependent, but also
>      perhaps mount-namespace dependent.
> 
>  (4) General request-key upcalls.  Not particularly namespace dependent,
>      apart from keyrings being somewhat governed by the user namespace and
>      the upcall being configured by the mount namespace.
> 
> These patches are built on top of the mount context patchset so that
> namespaces can be properly propagated over submounts/automounts.
> 
> These patches implement a container object that holds the following things:
> 
>  (1) Namespaces.
> 
>  (2) A root directory.
> 
>  (3) A set of processes, including a designated 'init' process.
> 
>  (4) The creator's credentials, including ownership.
> 
>  (5) A place to hang security for the container, allowing policies to be
>      set per-container.
> 
> I also want to add:
> 
>  (6) Control groups.
> 
>  (7) A per-container keyring that can be added to from outside of the
>      container, even once the container is live, for the provision of
>      filesystem authentication/encryption keys in advance of the container
>      being started.

It's hard to decide which of these has higher priority, I think both essential
to a container implementation.

Ian

[toc] | [prev] | [next] | [standalone]

#1648211

David Howells <dhowells@redhat.com> writes:

> Here are a set of patches to define a container object for the kernel and
> to provide some methods to create and manipulate them.

Just so this discussion has some clarity.

Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>

As a user visible entity I see nothing this container data structure
helps solve it only muddies the waters and makes things more brittle.

Embracing the complexity of namespaces head on tends to mean all of the
goofy scary semantic corner cases are visible from the first version of
the design, and so developers can't take short cuts that result in
buggy kernel code that persists for decades.  I am rather tired of
finding and fixing those.

Eric

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web