Comment 6 for bug 1262710

I reviewed nginx version 1.4.4-4ubuntu1 as checked into trusty. This
should not be considered a full security audit but rather a quick gauge
of maintainability.

The Debian nginx package provides both upstream nginx server as well
as third-party modules. Nginx is high-quality legible code, excellent
explanatory comments and platform notes, very useful utility functions,
and defensive error checking and logging.

The modules in the debian/modules/ directory vary widely; some looked
good quality but esoteric and others looked extremely complicated and
brittle.

We cannot commit to support the modules in the debian/modules/ directory.
We can support the nginx server itself and modules supplied by upstream
nginx. None of the existing nginx-{light,full,extras,naxsi} packages
are what I would like to support -- even nginx-light includes the
"echo" third-party module.

I suggest creating Yet Another Binary Package to include only the core
server and upstream modules; this hypothetical package would be suitable
for main.

Some notes for nginx upstream developers:

- ngx_devpoll_process_events() is missing an argument for a format specifier:

  ngx_log_error(NGX_LOG_ALERT, cycle->log, ngx_errno,
   "write(/dev/poll) for %d failed, fd");
                                     ^^^^^^^
  I'd love to see a static analyzer that can check for additional errors.

- Please examine CVE-2009-2408 and decide if these functions have the same
  problem:
  - ngx_ssl_get_subject_dn()
  - ngx_ssl_get_issuer_dn()

- There's some oddly asymmetric code around the bl->waiting++ in or out of
  an #if 0 block in these functions, one or the other may be a mistake:
  - ngx_http_busy_lock_cacheable()
  - ngx_http_busy_lock()

The usual MIR checklist:

- Build-depends upon autotools-dev, debhelper, dh-systemd, dpkg-dev,
  libexpat-dev, libgd2-dev, libgeoip-dev, liblua5.1-dev, libmhash-dev,
  libpam0g-dev, libpcre3-dev, libperl-dev, libssl-dev, libxslt1-dev,
  po-debconf, zlib1g-dev
- Performs significant cryptography operations
- Performs significant networking operations
- Provides nginx daemon that can run as privileged user, system user, or
  regular user
  - Daemonizes correctly with the caveat that it doesn't chdir() during
    daemonizing, but does during worker process startup
  - Listens on external interfaces
- Pre- and Post- install and uninstall scripts look sane
- initscript sets ulimits, checks the configuration, and uses
  start-stop-daemon to start or stop
- No Dbus services
- No setuid executables
- /usr/sbin/nginx and /usr/sbin/naxsi-ui-{extract,intercept} executables
- No sudo fragments
- No udev rules
- No test suite at build
- Spawned subprocesses looked to be started safely
- Memory management looked careful
- Files handled under direction from outside the process
- Uses MallocScribble, TZ, MALLOC_OPTIONS, and CPUPROFILE environment
  variables, looked safe
- Logging looked safe, very nice formatted print methods
- Privileged functions nicely centralized, looked safe
- Nearly all cryptography functions looked safe; I'm afraid CVE-2009-2408
  (not correctly handling ascii NUL characters in certificates) might have
  been re-introduced here.
- No temporary file handling
- No WebKit
- No JavaScript
- No PolicyKit

In summary, we can't yet promote nginx to main. We need to build a new
binary package without third-party modules and someone more familiar
with the project needs to investigate if ngx_ssl_get_subject_dn()
and ngx_ssl_get_issuer_dn() are recreating the same problem that led
to CVE-2009-2408.

Thanks