[MIR] simplestreams

Bug #1220427 reported by Scott Moser
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
simplestreams (Ubuntu)
Undecided
Unassigned

Bug Description

This is a Main Inclusion Request for simplestreams source package.

 * I've gone through UbuntuMainInclusionRequirements there are no issues that I saw.
 * Rationale: simplestreams will be used by MAAS for synchronizing data from http://maas.ubuntu.com/images
 * Security: Source code review is requested of lp:simplestreams. This package is a client and library for reading data and synchronizing or mirroring it with another source. The areas relevant to security team are at least:
    * gpg usage and verification
    * bad 'path' elements in data escaping a target (ie, when mirroring source/ to 'target/' no writes should go outside of target/).

all dependencies of python-simplestreams, simplestreams, and python3-simplestreams are in main.

Scott Moser (smoser)
description: updated
Michael Terry (mterry)
Changed in simplestreams (Ubuntu):
assignee: nobody → Jamie Strandboge (jdstrand)
Changed in simplestreams (Ubuntu):
assignee: Jamie Strandboge (jdstrand) → Seth Arnold (seth-arnold)
Revision history for this message
Seth Arnold (seth-arnold) wrote :

I reviewed simplestreams version 0.1.0~bzr307-0ubuntu1 as checked into
Saucy, and skimmed the lp:simplestreams checkins after 307. This is not a
full security audit, but a quick gauge of code quality.

- simplestreams provides standardized ways for cloud providers and guests
  to manage operating system images, tools images, asset images, etc., so
  that specific versions and 'latest versions' can be discovered and
  deployed.
- Build-depends upon python and python3 usual, and python3-yaml
- Depends upon python3-yaml, gnupg, python3-requests, python-boto,
  python-glanceclient, python-keystoneclient, python-swiftclient
- gnupg used for signing json descriptions of resources and validating
  json descriptions of resources
- Uses other libraries for openstack or aws networking, urllib for http
- Does not daemonize
- No initscripts
- No dbus
- No setuid
- No sudo
- No cron
- Nice test suite has positive and negative tests
- Clean build logs
- Three lintian warnings, one lintian error

- Subprocesses spawned extensively, 'subp' wrapper handles input, output,
  and throwing exceptions if programs exit with error statuses. Arrays are
  used to construct arguments.
  'call_hook' can execute hooks, similar to 'subp', also uses an array to
  construct arguments.
- Extensive file operations, names are constructed off simple
  manipulations of supplied data, looks safe
- Logging looks safe
- Environment variables are used for selectively disabling known-broken
  images, selecting gpg keys, and modifying gpg batch mode
- Extensive use of encryption, safety relies upon gpg returning an error
  status if it discovers a problem, this is probably a safe assumption.
- Can encode md5, sha256, sha512, of images
- Checks one of the hashes in the (potentially signed) .json files
- There are no privileged portions of code
- No WebKit
- As an especially nice bonus, there are pylint annotations throughout

This is a complicated piece of software, more than the "simple" in the
name may imply, but it is programmed well and has a nice test suite run
during the build.

I would prefer if there were no "unsigned" modes of operation to reduce
the chances of making a mistake in the signed data handling -- there are
code paths that have to handle both cases scattered throughout.

There is a potential problem in the 'checksummer' class: __init__() will
pick one of the available hash functions for use when validating images;
I believe the logic in __init__() will pick the strongest function
available but it surprises me that of the listed hashes in a stream,
only one will be used to validate the images.

Security team ACK for including into main.

Thanks

Changed in simplestreams (Ubuntu):
assignee: Seth Arnold (seth-arnold) → nobody
Revision history for this message
Michael Terry (mterry) wrote :

Seems fine from a packaging/maintainability POV. Runs tests, simple packaging, no important bugs, and Canonical will maintain. But it needs a team bug subscriber. After that is done, will approve.

Changed in simplestreams (Ubuntu):
status: New → Incomplete
Revision history for this message
Scott Moser (smoser) wrote :

Michael,
  I subscribed ~ubuntu-server. Thank you.

Changed in simplestreams (Ubuntu):
status: Incomplete → In Progress
Michael Terry (mterry)
Changed in simplestreams (Ubuntu):
status: In Progress → Fix Committed
Revision history for this message
Adam Conrad (adconrad) wrote :

Promoted.

Changed in simplestreams (Ubuntu):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers