[lxc-devel] [PATCH] lxc-attach: Consider cgroup, personality and capabilities when attaching processes to a container

[Christian Seiler]

Hi Daniel,

> thanks for your patches and your analysis.
> 
> IMO, we have to take into account the process we want to attach could be 
> an admin task and this one may want to have the full permissions within 
> the container. Also that could be an external daemon with the same 
> permissions as the container's processes. So inheriting should be 
> optional as it is up to the administrator to do the right action.

Yes, that's why I added the --keep-capabilities option to lxc-attach, to
make it possible for the administrator to execute a process inside the
container with higher permissions.

However, I only included capabilities there; it's true that cgroups may
impose an additional constraint. (Especially the device cgroup
controller.) On the other hand, the personality (which in LXC context
essentially means the architecture such as x86-64 vs. x86-32) is not
something I see as a "permission", but rather as a general property of
the container.

So the approach would then be:

 - default behaviour: use same restrictions as container
 - command line flag that allows one to ignore cgroups and capabilities
 - command line option to choose any architecture that's supported by
   the current running kernel (defaults to the arch of the container)

I do strongly think the default behaviour should be to use the same
restrictions as the container, as I see that to be the primary use case,
take for example

lxc-attach -n container -- /etc/init.d/sshd restart

This could easily leak privileges - the admin should explicitly state
that he/she wants to use elevated privileges if required.

> The parsing of the configuration file is right at the moment the 
> container has a configuration file and we did not launched the container 
> with the -s lxc.. options, or we did not modify the configuration file 
> after the container is launched.
> 
> I think it is much more sane to retrieve the needed informations from:
> 
>   * /proc/<pid>/status : for the capabilities
>   * /proc/<pid>/cgroup
>   * /proc/<pid>/personality
> 
> Where <pid> is the init pid of the container we can get through 
> get_init_pid function.

Yes, that seems like a reasonable approach. I'd rework the patches as
follows:

No flags:                 container's privileges according to /proc
-e/--elevated-privileges: maximum privileges (cgroup, capabilities)
-a x86/--arch=x86:        manually specify the architecture
                          (default to container's arch)

Is that agreeable?

Regards,
Christian