Comment 1 for bug 1220427

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