Comment 12 for bug 1381450

I reviewed conntrack version 1:1.4.2-2ubuntu1 sa checked into ubuntu
vivid. This should not be considered a full security audit but rather a
quick gauge of maintainability.

- conntrack provides both a connection tracking daemon that can interface
  with the Linux kernel's netfilter interfaces as well as an
  information-publishing tool that can provide better filtering of flow
  information than the /proc/ interfaces. The connection tracking daemon
  can be used to support HA stateful firewalls.
- Build-Depends: autotools-dev, bison, debhelper, dh-systemd, flex,
  libmnl-dev, libnetfilter-conntrack-dev, libnetfilter-cthelper0-dev,
  libnetfilter-cttimeout-dev, libnetfilter-queue-dev, libnfnetlink-dev
- pre/post inst/rm scripts have complicated mechanisms to handle previous
  configuration file locations and init.d vs systemd handling. Review by
  domain expert would be welcome.
- initscript and systemd service file look reasonable enough
- No dbus services
- No setuid binaries
- Provides conntrack, conntrackd, nfct binaries
- No sudo fragments
- No udev rules
- No cronjobs
- No test suite run during build

- No subprocesses spawned
- Memory management looks careful
- Few files opened; log files, configuration file,
  /proc/sys/net/netfilter/nf_conntrack_count
- Logging looked careful
- No environment variable use
- A handful of privileged operations are used, but the entirety of the
  package does privileged operations
- No cryptography
- Extensive netlink use; conntrackd can communicate with other conntrackd
  instances on other hosts, requires a private privileged network. Can
  spawn helpers to inspect and modify packets -- helpers are provided for
  ftp, rpc, and tns. (Helpers looked careful, though this kind of code is
  prone to mistakes. I'd love to see privilege separation / seccomp kinds
  of things for userspace helpers.)
- No tempory file handling
- No webkit
- No javascript
- No policykit
- Clean cppcheck

Here's a few issues I found while reviewing this package, in the hopes
these findings are useful:

- nfct_helper_free() in libnetfilter-cthelper has a use-after-free bug
  that may result in sigsegv:
  http://www.openwall.com/lists/oss-security/2015/04/22/5
  A fix has already been pushed to upstream git, this may be worth an SRU

- nfq_queue_cb() leaks myct if pktb_alloc(), helper_run(), or
  pkt_verdict_issue() return failures

- fork_process_new() will leak struct child_process c if the fork() fails

- I'm concerned that the daemon closes stderr and stdout before starting
  its main loop; there are many printf() and printf(stderr) calls in
  the codebase. Making sure that stdout and stderr refer to something
  useful at any given point is difficult. I suggest duping /dev/null to
  those descriptors if they are truly not going to used in the life of
  the daemon.

There's also an issue in the packaging, the binaries are not built PIE. I
realize it is too late to make them PIE before the release of vivid, so
please ensure this is handled shortly after the U series is opened, so
that it is not forgotten.

Security team ACK for promoting conntrack to main.

Thanks