MIR: pollinate

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 a...

Fantastic review, Seth. I'm going through all of these one by one now.

As to the pollinate cron job that does the periodic reseeding...

This was inspired by the NIST DRBG Special Publication 800-90A, which recommends periodically reseeding pseudo random number generators.

http://csrc.nist.gov/groups/STM/cavp/documents/drbg/DRBGVS.pdf‎

On Thu, Jan 9, 2014 at 1:37 AM, Seth Arnold <email address hidden> wrote:
> #2 Pollen "Recommends" haveged; we will not promote haveged to main.
>
> Please downgrade the Recommends to a Suggests.

Done.

Committed revision 166.

On Thu, Jan 9, 2014 at 1:37 AM, Seth Arnold <email address hidden> wrote:
> - pollinate shell script, the order of -c and -i arguments matter -- give
> them in the wrong order and the arguments are lost

Thanks. Fixed.

Committed revision 167.

:-Dustin

On Thu, Jan 9, 2014 at 1:37 AM, Seth Arnold <email address hidden> wrote:
> - pollinate shell script, IPV6 variable unused

Fixed.

Committed revision 168.

:-Dustin

On Thu, Jan 9, 2014 at 1:37 AM, Seth Arnold <email address hidden> wrote:
> - pollen.postinst generates a certificate with
> <email address hidden>, it'd be nice if it said pollen instead.

Fixed.

Committed revision 169.

:-Dustin

On Thu, Jan 9, 2014 at 1:37 AM, Seth Arnold <email address hidden> wrote:
> - 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.

I fixed the bug here. Basically, I'd like this cronjob to run once
per hour, at a regular interval, on every client that installs this
package. However, I'd like to spread the load evenly on the entropy
server, so that every single client doesn't hit the server at the same
minute. So at package installation time (postinst), I'd like this
particular install of the client to randomly choose the minute upon
which it runs.

On Thu, Jan 9, 2014 at 10:41 AM, Dustin Kirkland
<email address hidden> wrote:
> On Thu, Jan 9, 2014 at 1:37 AM, Seth Arnold <email address hidden> wrote:
>> - 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.
>
> I fixed the bug here. Basically, I'd like this cronjob to run once
> per hour, at a regular interval, on every client that installs this
> package. However, I'd like to spread the load evenly on the entropy
> server, so that every single client doesn't hit the server at the same
> minute. So at package installation time (postinst), I'd like this
> particular install of the client to randomly choose the minute upon
> which it runs.

Committed revision 170.

On Thu, Jan 9, 2014 at 1:37 AM, Seth Arnold <email address hidden> wrote:
> - 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.

Hmm, okay. That seems unlikely, but I think we can guard against this.

I decided to tackle this from a different directly, though. I've
solved this in the upstart script that runs pollen, by testing that
$DEVICE is in fact a character device. This involved a couple of
other changes (which are useful/necessary), including making the
DEVICE itself an argument to the pollen server itself.

Committed revision 171.

As a side note, the Pollen argv handling could be more robust. For
now, it's documented in the manpage that all of these arguments are
required.

:-Dustin

Thanks Seth for the audit and thanks Dustin for fixing some of the noted issues. So no blocker from security. As for the other aspects of a MIR, see below.

Blockers:
- Please subscribe a relevant team that will do maintenance for this to Ubuntu bugs. (Ideally not just yourself)

Notes:
- It would be really nice if this had some sort of test suite that was run during build. A dep8 test would be even better, which maybe could take advantage of the check_pollen script.

- Looks like a typo in pollinate.postinst:
chown root:root /var/lib/pollinate/
should be:
chown root:root /var/lib/pollinate/tag
(though why is this chown even necessary?)

On Thu, Jan 9, 2014 at 1:37 AM, Seth Arnold <email address hidden> wrote:
> #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.

Fixed in r172 and r173. I'm now using a Makefile and building the
pollen.go source code to a binary at package build time, installing,
and running that.

Committed revision 172.
Committed revision 173.

On Thu, Jan 9, 2014 at 1:37 AM, Seth Arnold <email address hidden> wrote:
> #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.

Done. Created a pollen user, added capabilities to the binary, and
launch the daemon as the pollen user.

Committed revision 175.

On Thu, Jan 9, 2014 at 1:37 AM, Seth Arnold <email address hidden> wrote:
> #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.

Apparmor profile created, installed, and now protecting the pollen server.

Committed revision 179.

On Thu, Jan 9, 2014 at 1:37 AM, Seth Arnold <email address hidden> wrote:
> #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?

This one isn't really possible. The tag is intended to be
unique-per-/dev/urandom-device. Any user on the system can run
pollinate, and if we put the tag creation logic there, it would mean
per-user tags.

The tag is voluntary, meaning that any user can remove, update, or
modify their tag, and still receive the seed via pollinate.

Per IRC:

<sarnold> kirkland: ah, I see
<sarnold> kirkland: well, fair enough, leave it alone. it works.
<sarnold> kirkland: like you said, there are options for downstreams
to do what they want with the tag file after the package is installed.

:-Dustin

On Thu, Jan 9, 2014 at 1:37 AM, Seth Arnold <email address hidden> wrote:
> - 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

It certainly looks to be operating properly to me. The upstart jobs
are installed, and the daemon is running, etc. This might be an issue
with dh... I don't think there's anything for me to do here.

:-Dustin

On Mon, Jan 13, 2014 at 3:44 PM, Michael Terry
<email address hidden> wrote:
> - Looks like a typo in pollinate.postinst:
> chown root:root /var/lib/pollinate/
> should be:
> chown root:root /var/lib/pollinate/tag
> (though why is this chown even necessary?)

Thanks for catching that, Michael. They are not, in fact, necessary.
I've removed them.

Committed revision 180.

:-Dustin

On Mon, Jan 13, 2014 at 3:44 PM, Michael Terry
<email address hidden> wrote:
> Notes:
> - It would be really nice if this had some sort of test suite that was run during build. A dep8 test would be even better, which maybe could take advantage of the check_pollen script.

Great idea, Michael. I have separated this out to Bug #1269187, which
Casey Marshall has kindly volunteered to help me with.

:-Dustin

I think that just leaves the team to subscribe to bugs. I'm going to talk to the server team about this tomorrow.

This bug was fixed in the package pollen - 3.7-0ubuntu1

---------------
pollen (3.7-0ubuntu1) trusty; urgency=low

  * debian/control:
    - demote haveged to suggests, based on feedback from Seth Arnold
      in LP: #1246098
  * pollinate:
    - ensure both -c and -i can be used, without losing CURL_OPTS,
      as identified by Seth Arnold in LP: #1246098
  * pollinate:
    - drop unused IPV6 variable, per review by Seth Arnold in LP: #1246098
  * debian/pollen.postinst:
    - use pollen as our fake email address, suggested by Seth Arnold
      in LP: #1246098
  * debian/pollinate.cron.d:
    - add notes in the comments about NIST DRBG Special Publication 800-90A
      recommendations on reseeding
    - add notes in the comments about why we choose a random minute
    - fix a bug, that was causing the cronjob to run far more frequently
      than desired
    - Addresses some issues raised by Seth Arnold in LP: #1246098
  * debian/pollen.upstart, pollen.8, pollen.go:
    - add DEVICE as the 3rd argument to the pollen server in the upstart
      script
    - test that DEVICE is a special in upstart
    - document that the DEVICE is now a required argument
  * debian/pollen.install, Makefile, pollen:
    - build static binary at package build time, rather than dynamically
      compiling at each run, per feedback from Seth Arnold in LP: #1246098
    - use a very simple, basic Makefile
  * debian/control:
    - move golang-go to a build-dependency, rather than a runtime dependency
  * debian/control, debian/pollen.postinst, debian/pollen.postrm,
    debian/pollen.upstart:
    - create a new user, pollen:daemon, in the postinst, remove in postrm
    - depend on libcap2-bin, which provides setcap
    - use setcap to allow the pollen binary to bind to privileged ports
    - run the pollen daemon as the pollen user
    - per feedback from Seth Arnold in LP: #1246098
  * debian/pollen.upstart:
    - use setuid in upstart to run the pollen daemon as the pollen user
  * debian/pollen.postinst:
    - change pollen user's shell to /bin/false
  * debian/control, debian/pollen.install, debian/pollen.postinst,
    debian/rules, usr.bin.pollen:
    - add an apparmor profile for the pollen server, per suggestion
      by Seth Arnold in LP: #1246098
    - big thanks to Jamie Strandboge and Seth Arnold for assistance
  * debian/pollinate.postinst:
    - these chowns are not necessary; thanks for catching Michael Terry
      in LP: #1246098
  * debian/control: LP: #1259014
    - have the pollen server depend on ent, which is used by the
      check_pollen nagios script
 -- Dustin Kirkland <email address hidden> Fri, 08 Nov 2013 09:59:37 -0600

On Tue, Jan 14, 2014 at 4:52 PM, Dustin Kirkland
<email address hidden> wrote:
> On Thu, Jan 9, 2014 at 1:37 AM, Seth Arnold <email address hidden> wrote:
>> #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?
>
> This one isn't really possible. The tag is intended to be
> unique-per-/dev/urandom-device. Any user on the system can run
> pollinate, and if we put the tag creation logic there, it would mean
> per-user tags.
>
> The tag is voluntary, meaning that any user can remove, update, or
> modify their tag, and still receive the seed via pollinate.
>
> Per IRC:
>
> <sarnold> kirkland: ah, I see
> <sarnold> kirkland: well, fair enough, leave it alone. it works.
> <sarnold> kirkland: like you said, there are options for downstreams
> to do what they want with the tag file after the package is installed.

Actually, I stand corrected. I was totally wrong here.

In fact, the intended usage of the pollinate script is within the
Ubuntu cloud images themselves, which is created with a debootstrap.
Generating the tag there would mean a unique tag per image, not per
instance, as is intended.

I have committed a solution to this, by generating the tag within the
pollinate itself. There's a bit of logic there now, to use a system
wide one, if found. If it's not found, and the script is running as
the daemon user (which is the case when the script fires from upstart
or cron), then it's generated one time and stored in
/var/lib/pollinate/tag. If the script is running as a normal user,
then a tag is generated for their use.

This was fixed and released in pollen 3.8.

Thanks!

ubuntu-server subscribed to bugs; however the golang-go implementation in main still needs to be resolved to support Juju as well.

I don't think we want two go implementations in main. why can't juju be built using gccgo which is available on all architectures.

Override component to main
pollen 3.12-0ubuntu1 in trusty: universe/admin -> main
pollen 3.12-0ubuntu1 in trusty amd64: universe/admin/optional/100% -> main
pollen 3.12-0ubuntu1 in trusty armhf: universe/admin/optional/100% -> main
pollen 3.12-0ubuntu1 in trusty i386: universe/admin/optional/100% -> main
pollinate 3.12-0ubuntu1 in trusty amd64: universe/admin/optional/100% -> main
pollinate 3.12-0ubuntu1 in trusty arm64: universe/admin/optional/100% -> main
pollinate 3.12-0ubuntu1 in trusty armhf: universe/admin/optional/100% -> main
pollinate 3.12-0ubuntu1 in trusty i386: universe/admin/optional/100% -> main
pollinate 3.12-0ubuntu1 in trusty powerpc: universe/admin/optional/100% -> main
pollinate 3.12-0ubuntu1 in trusty ppc64el: universe/admin/optional/100% -> main
10 publications overridden.

On Thu, Jan 9, 2014 at 1:37 AM, Seth Arnold <email address hidden> wrote:
> #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.

FYI, the tag information has been dropped entirely from the client and server.

On Thu, Jan 9, 2014 at 1:37 AM, Seth Arnold <email address hidden> wrote:
> - The client runs at boot and periodically via cron

FYI, the cron re-seed jobs have been removed entirely. It's up to the
user to create a cronjob, if they want to reseed.