Candidate revision checkbox_0.9

Bug #532882 reported by Marc Tardif on 2010-03-05
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Checkbox
High
Marc Tardif
checkbox (Ubuntu)
Wishlist
Mathias Gug

Bug Description

  * Introduced job_prompt plugin to treat all jobs (suites, tests, etc.) as composites.
  * Replaced the registry and resource scripts and centralized job iteration.
  * Replaced dependency on dbus by using sudo/gksu/kdesudo instead.
  * Replaced mktemp with mkdtemp for security purposes.
  * Fixed strings in fingerprint and modem tests (LP: #457759)
  * Fixed client side validation of Launchpad form (LP: #438671)
  * Added device information to tags when reporting bugs with apport.
  * Added shorthands for blacklist-file and whitelist-file.
  * Added support for apport default configuration (LP: #465447)
  * Added support for scrolled options list (LP: #411526)
  * Added support for tests generated by suites to run as root.
  * Added support for requirements in attachments.
  * Added support for armv7l processor
  * Added Autotest integration
  * Added LTP integration
  * Added Phoronix integration
  * Added qa-regression-testing integration

Related branches

Marc Tardif (cr3) on 2010-03-05
Changed in checkbox (Ubuntu):
milestone: none → ubuntu-10.04-beta-1
Marc Tardif (cr3) wrote :

This candidate revision is also a feature freeze exception because of the lateness in submitting these changes. The changes are essentially an implentation of the features detailed in the Checkbox Enhancements blueprint:

  https://blueprints.launchpad.net/ubuntu/+spec/lucid-qa-checkbox-enhancements

The first reason for the lateness is that the above blueprint was originally scheduled for beta-1, which happens to be way after the feature freeze limit.

This revision also includes fixes to important security vulnerabilities which were uncovered during the Ubuntu sprint. The proposed changes were initially proposed as:

- Remove /usr/share/dbus-1/system-services/com.ubuntu.checkbox.service
  so that dbus no longer spawns the backend on demand.

- Daemonize the backend directly from the frontend by calling sudo in
  order to run commands as root, instead of calling upon dbus to spawn
  the process.

- Use the session bus instead of the system bus, passing the token from
  the environment.

- Remove the NOPASSWD hack from the qa_regression_suite script and then
  remove the tests requiring the sudo command from the whitelist of
  tests until they are modified to run directly as root.

- In all scripts running as root, make sure to replace using /tmp by
  /var/cache and also make sure that the target files are only writable
  by root.

The second reason for the lateness is that it took a while to implement each of these changes since they were uncovered in early February. However, these security vulnerabilities were sufficiently significant that a new revision of Checkbox could not make it into the release unless these changes were implemented.

Marc Tardif (cr3) wrote :

The testing that was done on this revision of Checkbox has been quite extensive across the various hardware certification labs. This candidate revision has been used for at least a couple weeks now and has worked quite well across various hardware architectures. I have attached the log from running Checkbox on my local system which shows no sign of any unexpected exceptions being thrown.

Marc Deslauriers (mdeslaur) wrote :

I can confirm that all the changes the security team have asked for have been implemented in this release and the architecture is more secure than the dbus root daemon that is currently in lucid.

Martin Pitt (pitti) wrote :

This sounds okay in general, but I'm curious about the "Replaced dependency on dbus by using sudo/gksu/kdesudo instead.": This sounds like a step backwards in fact? Shouldn't the backend rather use PolicyKit to check user privileges? We are trying to get rid of gksu in default applications (only one or two are left ATM).

Marc Tardif (cr3) wrote :

The motivation for replacing dbus with sudo/gksu/kdesudo is that PolicyKit is meant to run specific commands with predefined privileges. This works well for applications like Jockey, for example, where the backend has a well defined behavior which can be audited for security purposes. However, for other applications like Checkbox, this is not possible because the backend needs to run arbitrary commands with elevated privileges. So, my understanding is that the user should effectively be prompted to be a super user in order to securely run arbitrary commands with such privileges.

Martin Pitt (pitti) wrote :

Ah, I had expected those tests to be started and run in the backend, not from the frontend. Then PolicyKit would merely control whether you are allowed to run the tests or not. But if the old architecture allowed you to run arbitrary commands through the D-Bus interface, this is indeed dangerous.

So let's go with the gksu approach for lucid. But in the long run, I think it would be better to run the root tests completly within the backend and go back to polkit.

Please upload.

Changed in checkbox (Ubuntu):
status: New → Confirmed
assignee: nobody → Marc Tardif (cr3)
Marc Tardif (cr3) wrote :

For your reassurance, the previous design of Checkbox using dbus was not quite as much a security vulnerability as that. In fact, the backend running as root could only run commands known to be tests. So, you could not connect to the backend and request to run a command like: rm -fr /. However, where this became a potential security vulnerability is when integrating third party test suites.

In Checkbox, tests are called jobs because they actually represent composites, ie jobs can generate new jobs and so forth. So, when integrating a third party suite like autotest, the integration job branches the project and extracts the tests within, which produce other jobs. If the repository was compromised or if the location where they got branched got compromised, then the produced jobs could essentially run arbitrary commands and still be considered legit by the backend.

The current solution of using sudo/gksu/kdesudo might seem overkill but seems to be the safest way to guarantee that a normal user will not compromise his system by accident. If an equally secure solution could be implemented with PolicyKit, this would be awesome. Perhaps this could be a topic of discussion for the next UDS where we should invite the security team.

Marc Tardif [2010-03-08 14:35 -0000]:
> The current solution of using sudo/gksu/kdesudo might seem overkill but
> seems to be the safest way to guarantee that a normal user will not
> compromise his system by accident.

I agree.

Martin

--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)

Mathias Gug (mathiaz) on 2010-03-09
Changed in checkbox (Ubuntu):
status: Confirmed → In Progress
assignee: Marc Tardif (cr3) → Mathias Gug (mathiaz)
importance: Undecided → Wishlist
Mathias Gug (mathiaz) wrote :

Looks good to me. Uploaded.

Also note that we're in UserInterfaceFreeze and there seems to be some strings that have changed.

Please follow the procedure from https://wiki.ubuntu.com/UserInterfaceFreeze to notify the documentation and translations teams about such a change (if relevant).

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package checkbox - 0.9

---------------
checkbox (0.9) lucid; urgency=low

  [ Marc Tardif ]
  New upstream release (LP: #532882):
  * Introduced job_prompt plugin to treat all jobs (suites, tests, etc.) as composites.
  * Replaced the registry and resource scripts and centralized job iteration.
  * Replaced dependency on dbus by using sudo/gksu/kdesudo instead.
  * Replaced mktemp with mkdtemp for security purposes.
  * Fixed strings in fingerprint and modem tests (LP: #457759)
  * Fixed client side validation of Launchpad form (LP: #438671)
  * Added device information to tags when reporting bugs with apport.
  * Added shorthands for blacklist-file and whitelist-file.
  * Added support for apport default configuration (LP: #465447)
  * Added support for scrolled options list (LP: #411526)
  * Added support for tests generated by suites to run as root.
  * Added support for requirements in attachments.
  * Added support for armv7l processor
  * Added Autotest integration
  * Added LTP integration
  * Added Phoronix integration
  * Added qa-regression-testing integration
 -- Mathias Gug <email address hidden> Tue, 09 Mar 2010 16:58:36 -0500

Changed in checkbox (Ubuntu):
status: In Progress → Fix Released
Marc Tardif (cr3) on 2010-03-10
Changed in checkbox:
assignee: nobody → Marc Tardif (cr3)
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Bug attachments