Comment 2 for bug 1220434

I reviewed curtin version 0.1.0~bzr94-0ubuntu1 as checked into saucy. This
should not be considered a full security audit, but rather a quick gauge
of code quality.

- curtin is a quick-and-easy cloud image installer
- Build-depends on pep8, pyflakes, usual python, nose, yaml
- Depends upon util-linux, curl, wget
- Python script in /usr/bin/curtin
- No daemons
- No dbus
- No initscripts
- No setuid
- No sudo
- No udev
- No cron
- Small test suite, mostly checks configuration handling, runs at build
- Clean build logs
- Extensive process spawning, used arrays for arguments, had provisions
  for not logging details of privileged commands if needed
- Files are extensively opened, read, written, sometimes copied, nearly
  everything looked safe enough, small suggestions below
- One LOG.debug() call used supplied format string, everything else looked safe
- No environment use
- Intended to run as root, does not manage its own privileges
- No cryptography
- Does not itself do networking
- Uses mkdtemp() when temporary files are needed
- Does not use webkit
- Does not use javascript
- Does not use policykit

curtin is clear, well-written code; I believe maintenance of curtin should
be straight-forward enough and since it is intended to run with high
privileges and thus use configuration supplied by 'trusted users', should
have relatively small attack surfaces.

My biggest complaints are actually with the README.txt file -- no
integrity checking is performed on the downloaded images or a related
shell script.

Future versions should include the xkvm script in the package (perhaps in
/usr/share/doc/curtin/examples/ if that'd be more appropriate) and should
include verifying the authenticity of downloaded Ubuntu disk and
filesystem images.

Here's the issues I found, in roughly the order I'd suggest working on
them, in the hopes that my notes can save someone some time:

- doc/devel/README.txt does not check gpg signatures on -disk1.img or
  -root.tar.gz downloads
- doc/devel/README.txt downloads and does not check authenticity of
  http://sn.im/xkvm, places it in ~/bin and adds it to the instance's path
- is_mounted() can probably be confused by mountpoints with space, tab, or
  newlines in name; do_mount() looks like it will happily mount them, but
  do_umount() cannot umount them.
  'src' is completely unused. os.path.ismount() is probably safer.
- do_mount() 'src' is unused in is_mounted(), thus this routine may not
  give correct results
- meta_simple() writes 'ext4' to /etc/fstab regardless of --fstype
- cmd_install() unsafe logging method:
  LOG.debug(workingd.env())
- Lintian warnings:
  W: curtin: binary-without-manpage usr/bin/curtin
  E: python3-curtin: bad-provided-package-name python3:any
  E: python-curtin: bad-provided-package-name python:any
- write_file() it would be safer to replace os.chmod() with
  os.fchmod(fp.fileno())

Security team ACK for promoting curtain to main.

Thanks