Comment 1 for bug 1246098

I reviewed pollen version 3.6-0ubuntu1 as checked into trusty. This
shouldn't be considered a full security audit, nor an audit of the design,
but rather a quick gauge of code quality.

- pollinate provides an entropy server and client, designed to combat
  low-entropy on systems after early install
- The client runs at boot and periodically via cron
- The server runs at boot
- Build-depends upon autotools and friends
- Server depends upon golang-go compiler, the pollen server is recompiled
  every time it is started
- Server recommends haveged, pollinate, ent
- Client depends upon run-one and curl
- Client suggests pollen-server and ent
- Uses Go's HTTPS libraries, server runs a self-contained web server
- Server:
  - listens on configurable ports for TLS and non-TLS
  - runs as user root
  - Does not daemonize, expects to be managed with upstart or similar
- Client:
  - Runs a script at boot and via cron to connect to servers using curl
    and store results into /dev/urandom
  - runs as user root during boot
  - runs as user daemon during cronjobs
- Postinst scripts look broken, details below
- Initscripts start server and client at boot
- No dbus services
- No setuid executables
- 'pollen' and 'pollinate' binaries installed into /usr/bin
- No sudo fragments
- No udev rules
- No test suite
- Client supplies a cron file:
  - Runs randomly selected number of times per day, from 1440 to 24, to
    periodically reseed the client's entropy pool; slight bug.
  - Should not be needed
  - May present privacy intrusions beyond what would be welcomed by the
    user community
- Build logs are clean -- compiles happen at run time

- No subprocesses are spawned from the server; the client is a shell
  script, so spawns dozens, but the code looked clean
- No real memory management; the server's http handler is quite small and
  allocates only stack memory
- Files are written to -- /dev/urandom in both client and server; the
  server postinst creates a key and a certificate in /etc/pollen; the
  client postinst creates a unique machine identifier in
  /var/lib/pollinate/tag
- Server-side logging stores the supplied User-Agent string verbatim
  without stripping or encoding newline and ] characters, which could
  complicate log analysis
- No environment variables are used
- No privileged operations as used
- Encryption use is encapsulated in golang HTTPS support and curl
- Certificate validation is left up to curl
- There is no management of privileges
- All temporary files explicitly managed by the programs are handled
  safely
- No WebKit
- No PolicyKit

The code quality of pollen and pollinate is high; maintenance should be
straightforward.

There are a few small issues I'm reporting here because I know Dustin is
eager for feedback and wants to address everything he can:

- is dh_installinit functioning correctly? both pollinate and pollen
  postinst have sections like the following:

  [...]
  # Automatically added by dh_installinit
  if [ -x "/etc/init.d/pollen" ] || [ -e "/etc/init/pollen.conf" ]; then
   if [ ! -e "/etc/init/pollen.conf" ]; then
    update-rc.d pollen defaults >/dev/null
   fi
   invoke-rc.d pollen start || exit $?
  fi
  # End automatically added section
  # Automatically added by dh_installinit
  update-rc.d -f pollen remove >/dev/null || exit $?
  # End automatically added section

- pollinate shell script, the order of -c and -i arguments matter -- give
  them in the wrong order and the arguments are lost

- pollinate shell script, IPV6 variable unused

- pollen.postinst generates a certificate with
  <email address hidden>, it'd be nice if it said pollen instead.

- pollinate cronjob runs every "random number of minutes" due to
  */__RAND__ use. Even */0 can be generated, this probably runs every
  minute. Or kills cron. :) But */2 or */3 would probably generate
  way too much load on our servers.

- pollen uses os.Create() to open the /dev/urandom may create a regular
  file if /dev/urandom doesn't exist for whatever reason -- it'd be worth
  checking the opened file to ensure it isn't a regular file. Either a
  check for specific major/minors of character devices or just a check for
  "not a regular file" would work; the first would be least surprising,
  the second may allow something clever.

There's also a handful of larger issues that need to be addressed. First,
for the server, pollen; while it isn't included in the MIR request, at
least one instance will be run by Canonical IT staff, so its security is
important to the security of our corporate network:

#1 The server is recompiled before running, every time it is started.
This means "apt-get install pollen" on a fresh Trusty development desktop
system downloads 25 megabytes of packages and unpacks to 172 megabytes of
disk space. This also means the server runs from a temporary file located
in the /tmp filesystem, which prevents /tmp from being mounted no-exec.

The Foundations Team needs to decide if Go is to be treated as a scripting
language or as a compiled language.

#2 Pollen "Recommends" haveged; we will not promote haveged to main.

Please downgrade the Recommends to a Suggests.

#3 The server runs as root, even though it only needs the privilege to
bind to low-numbered ports. The server should drop privileges once it
has bound to the socket.

Please create a user for pollen; please either launch the server as the
correct user and use non-privileged ports or add the ability to change
users, so that the server does not need to run as root.

#4 The Go-provided HTTP server code is relatively new and
untested; it represents a largely unknown attack surface.

Please consider providing an AppArmor profile for pollen.

Second, the issues for the client, pollinate:

#5 The pollinate.postinst creates a per-installation tag that is included
in requests. While this is convenient for server operators to track how
many clients are using their server, downstream consumers such as Tails
would need to pre-populate the /var/lib/pollinate/tag file before this
package is installed.

Could the /var/lib/pollinate/tag creation logic be moved later, into the
shell script, to allow more flexibility in how the tag is pre-populated
for these downstream users?

#6 The pollinate client cronjob runs often, making it easier for a
server operator to track a client across IP changes and track host uptime.

The client kernel does not need explicit reseeding; the kernel already
attends to its own periodic reseeding. Even without further seeding, the
initial 64 bytes of entropy supplied to the client should be sufficient to
generate unguessable pseudorandom output for ages. Petabytes or exabytes
ought to be conservative.

If, in the future, flaws are discovered that allow distinguishing the
output of /dev/random from randomness, the correct solution will probably
be a kernel update rather than periodic reseeding.

I do not think the cronjob provides additional safety to users.
Pollinate's value is at boot.

Please consider deleting the client-side cron job. Describing how to
create a cronjob in the documentation would be fine for the administrators
who wish to use one.

Because the server is not (yet) being considered for main, none of the
pollen issues are blockers.

The pollinate issues (#5 and #6) are not blockers but it would be nice to
have them considered before promoting pollinate to main. Security team ACK
for promoting pollinate to main.

Thanks