Comment 4 for bug 1248000

Revision history for this message
Seth Arnold (seth-arnold) wrote :

I reviewed mdata-tools as checked into lp:~utlemming/+junk/mdata_tools on
Tuesday 5 November 2013, a temporary staging area while being packaged for
trusty.

- No CVE history visible at the time of the audit
- The package provides several command-line utilities and a corresponding
  serial-line protocol to manage the client-side of metadata in
  SmartOS-managed KVM (and perhaps Zones?) instances.
- No cryptography
- Some Unix-domain sockets
- No 'networking' but relies upon serial communication with a hypervisor
- No daemons
- I didn't see pre/post inst/rm scripts
- I didn't see init scripts
- I didn't see dbus services
- I didn't see setuid executables
- I didn't see sudo fragments
- I didn't see udev rules
- Provides mdata-get, mdata-put, mdata-delete, mdata-list binaries.
- No test suite

- No subprocesses spawned
- Nearly all memory management is checked and safe
- Most file operations have hard-coded pathnames
- No environment use
- Very nice safer-strings helper routines
- Some privileged serial-port handling, looked safe, they even included
  helpful hints to proper documentation
- Some unix-domain networking, name handling looked safe
- Command stream is parsed with a simple and careful hand-written scanner
- No WebKit
- Since hypervisor will handle serial, MITM attacks are unlikely
- No PolicyKit
- No Javascript

mdata-tools is high-quality code programmed in a professional manner.

Some unit testing code would be nice to have, but the actual function of
the utilities would be quite difficult to test in any automated fashion.

I found several minor issues while reviewing the code that I thought I
should call out in the hopes that it is useful to someone:

- plat_init() unchecked calloc() call
- open_md(), unix_open_serial() O_EXCL used without O_CREAT; O_EXCL
  doesn't influence character device opens. Undefined behavior, but
  probably harmless.

(Issues have been filed at https://github.com/joyent/mdata-client/issues)

Security team ACK for promoting to main.

Thanks