[MIR] cgmanager

[Stéphane Graber]

== Upstream ==
This has been primarily developped by Serge Hallyn with contributions from James Hunt and Stéphane Graber under the LXC umbrella. LXC 1.0 will ship with cgmanager support and will be supported for 5 years upstream, I expect something similar to happen to cgmanager as a result of this, so getting bugfixes and security fixes from upstream shouldn't be a problem.

== Availability ==
Currently in universe, builds on all architectures and build-depends on libnih, libnih-dbus and libdbus. We also use the hardening-wrapper and help2man during the build process (both of which are also in main).

== Rationale ==
We'd like to see cgmanager promoted to main as it's intended to become the default CGroup Manager for Ubuntu 14.04.

upstart, LXC, logind and a few other core components are growing support for it and some of them are already ready to use it once it's in main.

== Security ==
We've been in touch with the Canonical Security team right from the beginning of this work, the details have also been discussed with the relevant maintainers. As with LXC, security is something very dear to upstream and we've got quite a bit of peer review happening on thos details.

== Quality insurance ==
Works out of the box, ships two daemons that are meant to run as root (cgmanager always runs on the host, cgproxy always runs in containers and both have to run on the host on old kernel versions).

A test packaged (cgmanager-tests) is built but not run at build time due to requirements which can't be fulfilled by a buildd, those tests can run in an autopkgtest environment (and will in the very near future, just as we're doing with LXC).

== UI standards ==
No real user facing interface, our rdepends will take care of user interactions.

== Dependencies ==
All build-deps in main, all deps in main.

== Standards compliance ==
cgmanager was lintian clean at the time I newed it and its packaging is pretty basic so I don't expect any problem there.

== Maintenance ==
See above, there's an upstream commitment for 5 years of bugfix support on LXC 1.0 and its related projects. The owning team for this will be the Foundations team working with the Server team (both will subscribe) as this was a joint blueprint/development.

Package wise, cgmanager itself is a dbus server, clients may use it directly or use the libcgmanager0 library in which case potential API changes would need a transition (that certainly won't happen post release).

[Matthias Klose (doko)]

the package itself looks ok for promotion, pending a review from the security team for the daemons.

[Stéphane Graber (stgraber)]

Thanks Mathias. As noted on IRC, I have also added myself as a bug contact for this package until we can get the bug contacts for foundations and server added (I don't have that power).

[Seth Arnold (seth-arnold)]

I reviewed cgmanager version 0.20-0ubuntu3 as checked into trusty. This
should not be considered a full security audit but rather a quick gauge of
maintainability.

cgmanager provides a userspace manager for cgroups; the Linux kernel
provides cgroups as a resource management interface and access control
mechanism but does not provide any mechanism for nesting cgroup users.
cgmanager provides both a proxy and a controller, allowing nested users
to request cgroup limits within the extents of their own limits. The
interface for nested users is via dbus.

- Build-Depends: autotools-dev, debhelper, dh-autoreconf, hardening-wrapper,
  help2man, libdbus-1-dev, libnih-dbus-dev, libnih-dev, libtool,
  nih-dbus-tool, pkg-config
- No cryptography
- Only AF_UNIX, dbus, networking
- Provides DBus services on /sys/fs/cgroup/cgmanager/sock
- Runs as root
- Uses libnih to daemonize, assumed correct
- pre- and post- scripts automatically generated
- init scripts start cgmanager or cgproxy
- No dbus system-services files
- No setuid executables
- Installs cgmanager and cgproxy daemons into /sbin/
- No sudo fragments
- No udev rules
- Runtime tests are provided in cgmanager-tests package, can't be run
  during build but should be easy to autopkgtest with them
- No cron jobs
- Clean build logs

- No subprocesses are spawned
- Memory management is careful, nih routines used
- File operations are usually synthetic kernel files, restricted scope
- Logging operations are careful
- No use of environment variables
- Privileged operations work on cgroup kernel files, looked careful
- No cryptography
- AF_UNIX and DBus networking used, looked careful
- All code runs as root
- No temporary files used
- No webkit
- No javascript
- Clean cppcheck
- No PolicyKit

More than usual lintian complaints:
E: cgmanager: postrm-does-not-call-updaterc.d-for-init.d-script etc/init.d/cgproxy
W: cgmanager: init.d-script-not-marked-as-conffile etc/init.d/cgproxy
E: cgmanager: init.d-script-not-included-in-package etc/init.d/cgproxy
W: cgmanager: manpage-has-useless-whatis-entry usr/share/man/man8/cgmanager.8.gz
W: cgmanager: manpage-has-useless-whatis-entry usr/share/man/man8/cgproxy.8.gz
N: 3 tags overridden (2 errors, 1 warning)

A few other small things that I noticed:

- Some failure cases don't have logging:
  setup_base_path() !base_path
  set_clone_children() snprintf()
  set_use_hierarchy() snprintf()

- do_move_pid_main() fprintf() error message slightly incorrect

- get_pid_scm_complete() char *output is not marked nih_local, this looks
  like a memory leak.

The cgmanager code is very defensive. Error returns are checked and
thoughtful error messages are provided for nearly every exceptional event.
The code is highly repetitive, so it would be possible for a potential
problem to be duplicated many times throughout the codebase; if we need
to perform maintenance tasks in the future we'll need to be careful to
look for other occurrences of the hypothesized fault. (This is common to
many programs that use Dbus, so it's not unexpected.)

Security team ACK for promoting cgmanager to main. I suggest not
promoting cgmanager-tests -- there is extensive use of sudo in dozens
of test sc...

[Stéphane Graber (stgraber)]

The init.d lintian warnings are due to the fact that there's no init.d script, they can be safely ignored though we'll probably introduce an actual sysvinit script to make it easier on other distros.

We don't need cgmanager-tests in main, we just need it around for adt tests, so having it stay in universe is fine by me.

[Serge Hallyn (serge-hallyn)]

Quoting Seth Arnold (<email address hidden>):
> A few other small things that I noticed:

Hi Seth,

> - Some failure cases don't have logging:
> setup_base_path() !base_path
> set_clone_children() snprintf()
> set_use_hierarchy() snprintf()
>
> - do_move_pid_main() fprintf() error message slightly incorrect

thanks, fixed these in git.

> - get_pid_scm_complete() char *output is not marked nih_local, this looks
> like a memory leak.

Drat, I was hoping you'd found a memory leak, however this isn't
one. It should be better commented at get_pid_scm_complete(),
but output will be nih_strduped() with the parent we pass in,
which is the socket data struct. So it will be freed by nih
when the socket data struct is freed. (I've added a comment in
git)

[Michael Terry (mterry)]

Looks mostly fine. The following lintian error would be nice to see fixed, but not a blocker:
E: cgmanager source: weak-library-dev-dependency libcgmanager-dev on libcgmanager0

However, I would like a statement about the tests/ directory. Is that not runnable during build or as a dep8 test?

[Michael Terry (mterry)]

Also, a team bug subscriber should be added.

[Stéphane Graber (stgraber)]

Serge, can you get whoever is the owner of the server team bug team to subscribe on the package?

As for tests, they aren't runnable during build as they require root access to the host machine. They should be runnable as DEP8 however and will be once I have the time to do it (will happen before release).

[Serge Hallyn (serge-hallyn)]

That should now be done (thanks zul).

[Michael Terry (mterry)]

Thanks! Approved.

Archive admins: please only promote cgmanager, libcgmanager0, and libcgmanager-dev. (that is, skip cgmanager-tests)

[Stéphane Graber (stgraber)]

stgraber@castiana:~/data/code/ubuntu-archive-tools$ ./change-override -t -c main cgmanager
Override component to main
cgmanager 0.23-0ubuntu1 in trusty: universe/admin -> main
Override [y|N]? y
1 publication overridden.
stgraber@castiana:~/data/code/ubuntu-archive-tools$ ./change-override -c main libcgmanager0
Override component to main
libcgmanager0 0.23-0ubuntu1 in trusty amd64: universe/admin/optional/100% -> main
libcgmanager0 0.23-0ubuntu1 in trusty arm64: universe/admin/optional/100% -> main
libcgmanager0 0.23-0ubuntu1 in trusty armhf: universe/admin/optional/100% -> main
libcgmanager0 0.23-0ubuntu1 in trusty i386: universe/admin/optional/100% -> main
libcgmanager0 0.23-0ubuntu1 in trusty powerpc: universe/admin/optional/100% -> main
libcgmanager0 0.23-0ubuntu1 in trusty ppc64el: universe/admin/optional/100% -> main
Override [y|N]? y
6 publications overridden.
stgraber@castiana:~/data/code/ubuntu-archive-tools$ ./change-override -c main libcgmanager-dev
Override component to main
libcgmanager-dev 0.23-0ubuntu1 in trusty amd64: universe/libdevel/optional/100% -> main
libcgmanager-dev 0.23-0ubuntu1 in trusty arm64: universe/libdevel/optional/100% -> main
libcgmanager-dev 0.23-0ubuntu1 in trusty armhf: universe/libdevel/optional/100% -> main
libcgmanager-dev 0.23-0ubuntu1 in trusty i386: universe/libdevel/optional/100% -> main
libcgmanager-dev 0.23-0ubuntu1 in trusty powerpc: universe/libdevel/optional/100% -> main
libcgmanager-dev 0.23-0ubuntu1 in trusty ppc64el: universe/libdevel/optional/100% -> main
Override [y|N]? y
6 publications overridden.

[Launchpad Janitor (janitor)]

This bug was fixed in the package lxc - 1.0.2-0ubuntu1

---------------
lxc (1.0.2-0ubuntu1) trusty; urgency=medium

  * New upstream bugfix release.
  * Update packaging from daily branch.
    - Build-depend on libcgmanager-dev
    - Build-depend on libseccomp-dev for armhf too
    - Move rsync dependency from lxc to liblxc1
    - Stop recommending cgroup-lite | cgroup-bin (replace by cgmanager)
    - Stop recommending libcap2-bin (lxc-setcap was dropped ages ago)
    - Stop recommending openssl from lxc (only used by templates)
    - Move uidmap recommend from lxc to liblxc1
    - Recommend busybox-static for lxc-templates
    - Add cgmanager as a dependency of liblxc1
    - Enable cgmanager support in LXC (LP: #1279048)
    - Drop cgroup-lite test suite dependency.
    - Update testsuite runner to work inside an unprivileged container.
    - Update testsuite runner to work in the LXC CI environment.
 -- Stephane Graber <email address hidden> Thu, 27 Mar 2014 23:18:11 -0400