[MIR] nginx

Bug #1262710 reported by Robie Basak
42
This bug affects 7 people
Affects Status Importance Assigned to Milestone
nginx (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

Availability:

nginx is built and available on all current architectures in Trusty (I'm not considering ppc64el "current" yet).

Rationale:

nginx is increasingly relevant to the Web 2.0 crowd, who are key users of Ubuntu Server. apache2 exists and we want to keep it in main also, there seems to be a split in userbase between those who use Apache (traditional) and those who use nginx (newer stacks). nginx seems to have gained a reputation for being fast and lightweight. This may or may not be true when compared against Apache, but many stacks today are deployed on nginx, and we are hearing that this is what users want and are running today. Therefore, we should have nginx in main to keep Ubuntu Server relevant to these users.

Security:

nginx supplies a public-facing daemon and listens on a privileged port, so needs a more in-depth security review. I hear that nginx was previously declined in main due to security concerns, but have been unable to find a previous MIR. I understand that the security team are prepared to re-review and determine how nginx's security status may have changed if I file this new MIR to track such a review.

This list of CVEs is not comprehensive; nginx has an extensive security history and this MIR requires an detailed security review.

A recently discovered vulnerability was CVE-2013-4547. This was addressed in Debian within a couple of days (http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=730012) and Thomas Ward took care of it in Ubuntu (https://bugs.launchpad.net/ubuntu/+source/nginx/+bug/1253691).

Other oustanding CVEs:
* CVE-2011-4968: this is a security-related missing feature, rather than a vulnerability per se. It's certainly debatable. It can only sensibly be addressed upstream. Debian don't deem it necessary to fix; I don't think Ubuntu needs to either.
* CVE-2013-0337: in progress in Debian for the upgrade path.
* CVE-2013-2070: Debian status in http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=708164; doesn't affect Trusty.

Stronger SSL configuration by default: pending testing and upload in http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=730142

Quality assurance:The Debian maintainers appear active and responsive to bug reports. Thomas Ward has been active watching the Ubuntu package, cherry-picking fixes from Debian, keeping an eye on security fixes and generally keeping the nginx package in Ubuntu up-to-date. If nginx enters main, then Thomas has said that he'll continue to look after the package as best he can, and the rest of the Ubuntu Server Team has committed to back him up where necessary.

Some non-standard packaging behaviour that is not mandated otherwise by policy:
* The service doesn't start automatically when the nginx package is first installed; you must use "sudo service nginx start" the first time. But the service does automatically restart on upgrade, etc, if the daemon was already running. invoke-rc.d is used correctly. This packaging behaviour appears to be intentional.
* /var/www is not the default document root, nor /var/www/html (the proposed new standard). Instead, it is/usr/share/nginx/html. Active debate at: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=730382. It appears that nginx maintainers are keen to follow Debian policy, but it is not specific enough and the apache2 maintainers are following a different interpretation. This means that the package isn't immediately usable on first install as a static web server; instead of placing files directly into the default document root and have them served, the FHS-compliant sysadmin must reconfigure the daemon to use a default document root first.

Apart from this, the package works straight away as a typical nginx user would expect.

No debconf templates. No major long-term outstanding bugs. As a popular package there are a number of long-term outstanding bugs, but these all appear to relate to edge case behaviours or feature requests that do not affect the majority of nginx users.

nginx appears active both upstream and in Debian and appears to be maintained well, with regular uploads in Debian over 2013, including wheezy security updates. A debian/watch file exists, appears functional, and the latest upstream version is packaged. There does not appear to be a relevant upstream test suite.

-dbg packages exist. Question: do these need ddeb generation for Ubuntu instead? What is our policy here?

UI standards: N/A for this server package

Dependencies:

* Build-Depends: liblua5.1-dev is only fulfilled by universe. Removing this will clearly drop lua support, but nginx will still work fine. Can lua support be dropped from nginx without disproportionate impact to users?
* The nginx-naxsi-ui binary package depends on daemon, which is in universe, but nothing depends, recommends or suggests nginx-naxsi-ui. Can nginx-naxsi-ui be kept in universe, with the other components in main?

FHS compliance: the packaging appears FHS compliant. Debian policy compliance: nginx claims compliance to 3.9.4; current policy is 3.9.5. The packaging uses traditional debhelper and appears do be done in a straightforward way; though necessarily a little more complicated than usual due to the multiple binary packages with different build configurations, as might be expected with this sort of package.

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=715435 appears to state that nginx-naxsi-ui doesn't work, and that this is a serious policy violation. This package doesn't fit our rationale to be in main., and has the daemon dependency that is in universe as described above. Can nginx-naxsi-ui remain in universe?

~ubuntu-server will commit to monitor and maintain nginx in main, with the help of Thomas Ward.

Revision history for this message
Robie Basak (racb) wrote :

It seems that we have to drop lua from the nginx build as a cost of moving nginx to main. If we do this, then the following debdiff appears to work. debian/control should also have the nginx-extras binary package description changed to not claim that Lua is included.

Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in nginx (Ubuntu):
status: New → Confirmed
Revision history for this message
Michael Terry (mterry) wrote :

This is for the supported seed? This has security implications, so I'll assign to Jamie.

Changed in nginx (Ubuntu):
assignee: nobody → Jamie Strandboge (jdstrand)
Revision history for this message
Robie Basak (racb) wrote :

> This is for the supported seed?

Yes, that seems the most appropriate. Thanks.

James Page (james-page)
Changed in nginx (Ubuntu):
milestone: none → ubuntu-14.04-beta-1
Revision history for this message
Robie Basak (racb) wrote :

I just realised that liblua5.2-dev is in main. Perhaps we can bump lua in nginx in Ubuntu, and maybe see if Debian are OK to update, too.

Changed in nginx (Ubuntu):
assignee: Jamie Strandboge (jdstrand) → Seth Arnold (seth-arnold)
Revision history for this message
Seth Arnold (seth-arnold) wrote :
Download full text (3.5 KiB)

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

Read more...

Changed in nginx (Ubuntu):
assignee: Seth Arnold (seth-arnold) → nobody
Revision history for this message
Valentin V. Bartenev (vbart) wrote :

Thanks for the review.

Just a few quick comments:

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

Already fixed in mainline: http://hg.nginx.org/nginx/rev/6b479db5b52b

> I'd love to see a static analyzer that can check for additional errors.

We use Coverity on a regular basis: https://scan.coverity.com/projects/158

> 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()

Actually the whole code in ngx_http_busy_lock.(h|c) are dead and not compiled.

Revision history for this message
Valentin V. Bartenev (vbart) wrote :

Hmm, seems I was wrong, it's compiled but just isn't used.

Revision history for this message
Thomas Ward (teward) wrote : Re: [Bug 1262710] Re: [MIR] nginx

I spoke to sarnold on IRC briefly last night. They said they might support a separate "upstream-only" binary or an added build that has only upstream modules shipped with nginx.

I'll take a look tomorrow and see what I can do to accommodate this. However, it stands to be noted that most users would probably end up using the universe package or the PPAs even if we add a whole extra binary that is for upstream-only code that would end up in main, while the rest would end up in universe. A lot of users kinda rely on those third-party modules.

Nevertheless I'll take a stab at this starting tomorrow. The new job I have prevents me from starting sooner than Saturday on this.

------
Thomas

*Sent from my iPhone. Please excuse any typos, as they are likely to happen by accident.*

Revision history for this message
s novotny (sarahnovotny) wrote :

teward and sarnold,

Do you two have what you need from NGINX as to answers to move this forward?

sarah

Revision history for this message
Thomas Ward (teward) wrote :

Sarah,

We're working on adding an 'nginx-core' package added that builds all the modules that ship with the nginx source tarball. The other problem is the optional Lua module in the universe package that we have to figure out how to work with. It doesn't work with Lua 5.2 and that is blocking progress right now.

What I'd like from NGINX is a list of all the modules and their ./configure arguments that ship with the plain nginx tarball to confirm that we have only NGINX upstream-maintained modules in that -core package.

------
Thomas

*Sent from my iPhone. Please excuse any typos, as they are likely to happen by accident.*

Revision history for this message
Valentin V. Bartenev (vbart) wrote :

You can use the "--help" configure argument to get a list of all available options. I've attached outputs for vanilla nginx 1.4.6 and 1.5.11.

The actual list of supported (optional and default) modules is follow in alphabetical order:

1. Default HTTP modules (built by default, but can be switched off by a "--without-*" parameter):

ngx_http_access_module
ngx_http_auth_basic_module
ngx_http_autoindex_module
ngx_http_browser_module
ngx_http_charset_module
ngx_http_empty_gif_module
ngx_http_fastcgi_module
ngx_http_geo_module
ngx_http_gzip_module
ngx_http_limit_conn_module
ngx_http_limit_req_module
ngx_http_map_module
ngx_http_memcached_module
ngx_http_proxy_module
ngx_http_referer_module
ngx_http_rewrite_module
ngx_http_scgi_module
ngx_http_split_clients_module
ngx_http_ssi_module
ngx_http_upstream_ip_hash_module
ngx_http_upstream_keepalive_module
ngx_http_upstream_least_conn_module
ngx_http_userid_module
ngx_http_uwsgi_module

2. Optional HTTP modules (need a "--with-*" parameter to be built):

ngx_http_addition_module
ngx_http_auth_request_module (since nginx 1.5.4)
ngx_http_dav_module
ngx_http_flv_module
ngx_http_geoip_module
ngx_http_gunzip_module
ngx_http_gzip_static_module
ngx_http_image_filter_module
ngx_http_mp4_module
ngx_http_perl_module
ngx_http_random_index_module
ngx_http_realip_module
ngx_http_secure_link_module
ngx_http_spdy_module
ngx_http_ssl_module
ngx_http_stub_status_module
ngx_http_sub_module
ngx_http_xslt_module

3. Optional mail proxy modules:

ngx_mail_imap_module
ngx_mail_pop3_module
ngx_mail_smtp_module
ngx_mail_ssl_module

I intentionally omitted the ngx_http_degradation_module module, because it currently is obsoleted and works well only on old BSD systems.

My suggestion is to have two packages, all shipped with only officially supported modules, but one with the "--with-debug" option enabled, and another one without debug. That is what we provide now in our repositories. It's very convenient to have identical nginx-debug package, because it helps a lot not only to debug bugs, but also it helps users to better understand how nginx works, and to solve configuration problems.

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

Sarah, thanks for the reminder; I had my one remaining outstanding question answered to my satisfaction: http://mailman.nginx.org/pipermail/nginx-devel/2014-February/005038.html -- in short, I hadn't realized X509_NAME_oneline() would escape the ascii NUL character when converting from ASN.1 to a C-representable string.

We're currently stuck because the Debian-derived packaging includes an out-of-tree module that builds against lua-5.1. It cannot compile against lua-5.2 (the lua-5.2 changes are drastically not backwards compatible with lua-5.1), but lua-5.2 is the lua package that is going to be supported in Ubuntu trusty tahr.

If it were up to me alone, I'd disable the lua module. Lua 5.2 has been out for over two years and if this module hasn't updated yet, there's no reason for me to suspect they will update before we need to release 14.04.

Thanks

Revision history for this message
Thomas Ward (teward) wrote :
Download full text (7.4 KiB)

Seth,

Lua module upstream has outright said 5.2 isn't supported, I poked around there for the Lua module and they said so.

I'm researching alternatives.

------
Thomas

*Sent from my iPhone. Please excuse any typos, as they are likely to happen by accident.*

> On Mar 5, 2014, at 14:44, Seth Arnold <email address hidden> wrote:
>
> Sarah, thanks for the reminder; I had my one remaining outstanding
> question answered to my satisfaction: http://mailman.nginx.org/pipermail
> /nginx-devel/2014-February/005038.html -- in short, I hadn't realized
> X509_NAME_oneline() would escape the ascii NUL character when converting
> from ASN.1 to a C-representable string.
>
> We're currently stuck because the Debian-derived packaging includes an
> out-of-tree module that builds against lua-5.1. It cannot compile
> against lua-5.2 (the lua-5.2 changes are drastically not backwards
> compatible with lua-5.1), but lua-5.2 is the lua package that is going
> to be supported in Ubuntu trusty tahr.
>
> If it were up to me alone, I'd disable the lua module. Lua 5.2 has been
> out for over two years and if this module hasn't updated yet, there's no
> reason for me to suspect they will update before we need to release
> 14.04.
>
> Thanks
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1262710
>
> Title:
> [MIR] nginx
>
> Status in “nginx” package in Ubuntu:
> Confirmed
>
> Bug description:
> Availability:
>
> nginx is built and available on all current architectures in Trusty
> (I'm not considering ppc64el "current" yet).
>
> Rationale:
>
> nginx is increasingly relevant to the Web 2.0 crowd, who are key users
> of Ubuntu Server. apache2 exists and we want to keep it in main also,
> there seems to be a split in userbase between those who use Apache
> (traditional) and those who use nginx (newer stacks). nginx seems to
> have gained a reputation for being fast and lightweight. This may or
> may not be true when compared against Apache, but many stacks today
> are deployed on nginx, and we are hearing that this is what users want
> and are running today. Therefore, we should have nginx in main to keep
> Ubuntu Server relevant to these users.
>
> Security:
>
> nginx supplies a public-facing daemon and listens on a privileged
> port, so needs a more in-depth security review. I hear that nginx was
> previously declined in main due to security concerns, but have been
> unable to find a previous MIR. I understand that the security team are
> prepared to re-review and determine how nginx's security status may
> have changed if I file this new MIR to track such a review.
>
> This list of CVEs is not comprehensive; nginx has an extensive
> security history and this MIR requires an detailed security review.
>
> A recently discovered vulnerability was CVE-2013-4547. This was
> addressed in Debian within a couple of days (http://bugs.debian.org
> /cgi-bin/bugreport.cgi?bug=730012) and Thomas Ward took care of it in
> Ubuntu (https://bugs.launchpad.net/ubuntu/+source/nginx/+bug/1253691).
>
> Other oustanding CVEs:
> * CVE-2011-4968: this is a secur...

Read more...

Revision history for this message
s novotny (sarahnovotny) wrote :
Revision history for this message
Thomas Ward (teward) wrote :

Lua module's upstream people have suggested either static-link against Lua 5.1, which still doesn't solve this problem, or to use libluajit-5.1-dev (2.0.2+) as the dependency.

This *does* build with libluajit-5.1-dev. However, that is still in Universe as well, and would need Main inclusion in order for this to work.

The alternative is to do what sarnold, rbasak, and I had initially considered before the MIR went through security review: drop the Lua module from the package, and build without it, in addition to the changes we'll need to do for the MIR of creating an nginx-core package, which I already have a debdiff available for.

Seth, in your opinion, how should we proceed on this?

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

Strictly speaking, my role in the MIR is to provide an assessment of what the security team believes is supportable for five years.

Maintaining a language's runtime support environment for five years is no small undertaking. Lua 5.1 support is in a rough place -- it's already two years "obsolete", as it were, and it feels like a gamble to see if the libluajit community is in a position to keep their fork of the language vibrant and supported for the long term.

I'd be content to remove the lua-5.1 Build-depends: requirement and stop building the module, as Robie's patch does. nginx-lua can live in a PPA.

If someone were willing to file a MIR (and become a bug subscriber) for libluajit-5.1, we could consider moving that to main, but the nginx-lua module would still remain in universe.

Thanks

Revision history for this message
Thomas Ward (teward) wrote :

I guess that remains the most sane solution, drop the Lua module and dependency, and leave it out of the package in Ubuntu. I'll add the "drop the Lua module" changes to my debdiff after work today and drop the debdiff here for review.

Note that this MIR also needs a FFe since FeatureFreeze is past.

I'd also like to have nginx 1.4.6 merged in but I'm not sure whether that qualifies for an FFe...

------
Thomas

*Sent from my iPhone. Please excuse any typos, as they are likely to happen by accident.*

Revision history for this message
Thomas Ward (teward) wrote :

Valentin, regarding your comment in comment 7 about things being fixed in mainline, that "fix" won't hit Ubuntu until Debian has mainline. As of right now, I don't think they're switching to the mainline branch yet.

To all: We're going to drop the Lua module from nginx-extras for the MIR. If you need the Lua module, they'll be in the PPAs, after I update them.

Please note that the PPAs are more up-to-date than the version being considered here for main inclusion. 1.4.5 is older than 1.4.6 which is now in Debian.

Revision history for this message
Thomas Ward (teward) wrote :

Thanks to Adam Conrad, NGINX 1.4.6 is now in Trusty (see https://bugs.launchpad.net/ubuntu/+source/nginx/+bug/1290063 )

I'll rebase my debdiff off 1.4.6 instead, as soon as I get to stable internet again. (this might be tomorrow :/)

Revision history for this message
Thomas Ward (teward) wrote :

Attached is a debdiff for this MIR based off of 1.4.6 which Adam Conrad merged into Ubuntu.

Whomever needs to review this, please review. Thanks.

Revision history for this message
Thomas Ward (teward) wrote :

This debdiff here contains typos that sarnold found, and is in response to this:

<sarnold> teward: hey :) nice debdiff, thanks; there is an UNRELEASED in the changelog, and 'nginx-extra' typo in the Description: field for the nginx-core package. otherwise it looks good to me. Thanks for taking this on.
<teward> sarnold: can you give me line numbers?
<teward> i have to change it from a phone that's SSH'd into my main system, so line numbers would help
<teward> oh
<teward> wait... what?
<teward> sarnold: i don't see that typo you mention
<sarnold> teward: line 79, and line 5
<teward> sarnold: ah thanks
<teward> sarnold: i found the same typo in my changelog entry
<sarnold> teward: hah, I read right past that one. more eyes..

Revision history for this message
Thomas Ward (teward) wrote :

I meant it contains fixes for typos.

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

After discussion between Adam Conrad, Thomas Ward, Dimitri John Ledkov, and myself, we came to conclusion that we should not remove the nginx-lua module from the distribution. So, we will re-promote lua5.1 to main as a build dependency for nginx and continue building the nginx-lua module so existing users do not have to discover a PPA or self-compile nginx to keep their Lua-based applications functioning.

Lua 5.1 is already supported in 12.04 LTS for another three years, so this decision only adds another two years of Lua 5.1 support. Hopefully the situation will be a little more clear around the time of 16.04 LTS.

Thanks

Revision history for this message
Adam Conrad (adconrad) wrote :

Cleaned up teward's diff, seeded nginx, and sponsoring the upload as soon as the archive catches up with some override changes, making it buildable in main.

Changed in nginx (Ubuntu):
status: Confirmed → Fix Committed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package nginx - 1.4.6-1ubuntu3

---------------
nginx (1.4.6-1ubuntu3) trusty; urgency=medium

  * Add new binary package for main, nginx-core, which contains only
    source-tarball-included modules and no third-party modules.
  * Changes to debian/ directory:
    - control:
      + Add entry for nginx-core and nginx-core-dbg.
    - rules:
      + Add nginx-core flavor to the build rules.
    - nginx-core.*: Add new packaging files for nginx-core based on
      the packaging files for nginx-full.
  * The above changes satisfy the requirements for main (LP: #1262710)
 -- Thomas Ward <email address hidden> Mon, 10 Mar 2014 18:23:36 -0400

Changed in nginx (Ubuntu):
status: Fix Committed → Fix Released
Revision history for this message
Robie Basak (racb) wrote :

Bug 1324062 tracks the missing lua 5.2 support issue.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Related questions

Related blueprints

Remote bug watches

Bug watches keep track of this bug in other bug trackers.