MIR: bin:libnginx-mod-http-geoip2 from src:nginx

Bug #1867198 reported by Andreas Hasenack on 2020-03-12
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
nginx (Ubuntu)
Undecided
Ubuntu Security Team

Bug Description

nginx is already in main, but the binary package libnginx-mod-http-geoip2 comes from another source that is shipped inside src:nginx and hasn't gone through the original nginx MIR.

When it was included[1], the agreement was that it would stay in Universe. We now want it in main, because of the geoip1 legacy deprecation in favor of geoip2 (libmaxminddb), see libmaxminddb's MIR[2].

NOTE: this MIR is *not* a blocker for the libmaxminddb MIR[2], as we are demoting bin:libnginx-mod-http-geoip to universe (see FFe https://bugs.launchpad.net/ubuntu/+source/nginx/+bug/1867150)

1. https://bugs.launchpad.net/ubuntu/+source/nginx/+bug/1825895
2. https://bugs.launchpad.net/ubuntu/+source/libmaxminddb/+bug/1861101

Source: https://github.com/leev/ngx_http_geoip2_module

Availability:
The package must already be in the Ubuntu universe, and must build for the architectures it is designed to work on.
The bin:libnginx-mod-http-geoip2 package is built from src:nginx and exists in universe already:
https://launchpad.net/ubuntu/focal/+package/libnginx-mod-http-geoip2
https://launchpad.net/ubuntu/focal/amd64/libnginx-mod-http-geoip2/1.17.9-0ubuntu1

It's built for amd64, arm64, armhf, ppc64el, s390x

Rationale:
We want to move away from legacy geoip1 code into new geoip2 code. nginx already has a geoip1 module in main (bin:libnginx-mod-http-geoip), and we want to exchange that with the geoip2 module.
The main push to switch to geoip2 comes from the latest stable release of bind9 (9.16.x), which has no support for geoip1, just geoip2, via libmaxminddb, which has an ongoing MIR at https://bugs.launchpad.net/ubuntu/+source/libmaxminddb/+bug/1861101. As a requirement of that MIR, we want to avoid having geoip1 and geoip2 both in main.

Security:
The security history and the current state of security issues in the package must allow us to support the package for at least 9 months (60 for LTS support) without exposing its users to an inappropriate level of security risks. This requires checking of several things that are explained in detail in the subsection Security checks.

http://cve.mitre.org/cve/search_cve_list.html: Search in the National Vulnerability Database using the package as a keyword
- I tried a few keywords: geoip2, "nginx geoip2", "nginx geoip", "ngx_http_geoip2_module", "ngx geoip2", "ngx geoip": all empty, or returning nginx issue, or geoip issues in other products

check OSS security mailing list (feed 'site:www.openwall.com/lists/oss-security <pkgname>' into search engine)
- "site:www.openwall.com/lists/oss-security nginx geoip2": empty
- "site:www.openwall.com/lists/oss-security nginx geoip": empty
- "site:www.openwall.com/lists/oss-security ngx_http_geoip2_module": empty
- "site:www.openwall.com/lists/oss-security ngx geoip2": empty

Ubuntu CVE Tracker
* http://people.ubuntu.com/~ubuntu-security/cve/main.html
- nothing for "geoip", "geoip2" or "nginx" (I checked and the package search is a substring search). Be sure to click "clear filter" between search attempts.

* http://people.ubuntu.com/~ubuntu-security/cve/universe.html
- nothing for "geoip", "geoip2" or "nginx"

* http://people.ubuntu.com/~ubuntu-security/cve/partner.html
- nothing for "geoip", "geoip2" or "nginx" (in fact, there are no packages at all listed there)

* Executables which have the suid or sgid bit set.
- none

* Executables in /sbin, /usr/sbin.
- none, it's an nginx module is a .so file in the modules directory

* Packages which install services / daemons (/etc/init.d/*, /etc/init/*, /lib/systemd/system/*)
- bin:libnginx-mod-http-geoip2 doesn't install a daemon, but nginx of course is one

* Packages which open privileged ports (ports < 1024).
- bin:libnginx-mod-http-geoip2 doesn't, but nginx of course does

* Add-ons and plugins to security-sensitive software (filters, scanners, UI skins, etc)
- this is an add-on, or plugin/module, to nginx

Quality assurance:
* After installing the package it must be possible to make it working with a reasonable effort of configuration and documentation reading.
- the geoip2 module is loaded right after installation:
lrwxrwxrwx 1 root root 55 Mar 12 20:28 /etc/nginx/modules-enabled/50-mod-http-geoip2.conf -> /usr/share/nginx/modules-available/mod-http-geoip2.conf
- To further set it up, nginx configuration must be changed, and a copy of the geoip2 databases must be obtained, either the free one or the subscription one.

* The package must not ask debconf questions higher than medium if it is going to be installed by default. The debconf questions must have reasonable defaults.
- no debconf questions

* There are no long-term outstanding bugs which affect the usability of the program to a major degree. To support a package, we must be reasonably convinced that upstream supports and cares for the package.
  - Ubuntu bugs:
    - src:nginx bugs: https://bugs.launchpad.net/ubuntu/+source/nginx. None about geoip, other than this MIR and the demotion of legacy geoip1
  - Debian bugs:
    - src:nginx bugs: https://bugs.debian.org/cgi-bin/pkgreport.cgi?dist=unstable;package=nginx
    - none about geoip, but note that debian doesn't ship the geoip2 module (it's an ubuntu delta)

* The package is maintained well in Debian/Ubuntu (check out the Debian PTS)
- https://tracker.debian.org/pkg/nginx
- debian doesn't ship the geoip2 module, just geoip(1) (legacy)
- ubuntu's nginx package is ahead of debian
- it looks like it could be better maintained in debian: open security issues in the BTS, bugs with patches still open. Uploads aren't super frequent, but also haven't stalled for toooo long

* The package should not deal with exotic hardware which we cannot support.
- no exotic hardware

* If the package ships a test suite, and there is no obvious reason why it cannot work during build (e. g. it needs root privileges or network access), it should be run during package build, and a failing test suite should fail the build.
- no test suite in the upstream code
- there are many dep8 tests, though:
autopkgtest [06:50:29]: @@@@@@@@@@@@@@@@@@@@ summary
light-simple PASS
core-simple PASS
lua PASS
full-simple PASS
extras-simple PASS
full-module-deps PASS
light-module-deps PASS
core-module-deps PASS
extras-module-deps PASS
- but none about geoip, other than installing most modules. This could be expanded to also install geoip2 perhaps:
$ grep geoip debian/tests/*
debian/tests/control: libnginx-mod-http-geoip,
debian/tests/control: libnginx-mod-http-geoip,
debian/tests/control: libnginx-mod-http-geoip,
debian/tests/control: libnginx-mod-http-geoip,

* The package uses a debian/watch file whenever possible. In cases where this is not possible (e. g. native packages), the package should either provide a debian/README.source file or a debian/watch file (with comments only) providing clear instructions on how to generate the source tar file.
- src:nginx has a d/watch file, but it seems to be out of date, as it looks for versions matching 1.13.*, and focal now has 1.17.9:
$ cat debian/watch
version=3
opts=pgpsigurlmangle=s/$/.asc/ \
https://nginx.org/download/nginx-(1\.13\.\d+)\.tar\.gz

* It is often useful to run lintian --pedantic on the package to spot the most common packaging issues in advance
$ lintian -I --pedantic
W: nginx-common: maintainer-script-should-not-use-dpkg-maintscript-helper postinst:10
W: nginx-common: maintainer-script-should-not-use-dpkg-maintscript-helper postinst:12
W: nginx-common: maintainer-script-should-not-use-dpkg-maintscript-helper postinst:14
W: nginx-common: maintainer-script-should-not-use-dpkg-maintscript-helper ... use --no-tag-display-limit to see all (or pipe to a file/program)
W: nginx source: orig-tarball-missing-upstream-signature nginx_1.17.9.orig.tar.gz
I: nginx source: duplicate-short-description libnginx-mod-http-geoip libnginx-mod-http-geoip2
I: nginx source: duplicate-short-description nginx-core nginx-full
I: libnginx-mod-stream: hardening-no-fortify-functions usr/lib/nginx/modules/ngx_stream_module.so
I: nginx source: out-of-date-standards-version 4.1.3 (released 2017-12-27) (current is 4.5.0)
I: nginx source: public-upstream-key-not-minimal upstream/signing-key.asc has 3 extra signature(s) for keyid 520A9993A1C052F8
I: libnginx-mod-rtmp: spelling-error-in-binary usr/lib/nginx/modules/ngx_rtmp_module.so conection connection
I: libnginx-mod-rtmp: spelling-error-in-binary usr/lib/nginx/modules/ngx_rtmp_module.so shedule schedule
I: libnginx-mod-http-perl: unused-override wrong-section-according-to-package-name libnginx-mod-http-perl => perl
P: nginx source: debug-symbol-migration-possibly-complete --dbgsym-migration='nginx-$(*)-dbg (<< 1.10.1-3~)' (line 184)
P: nginx source: duplicate-in-relation-field in source build-depends: po-debconf, po-debconf
P: nginx source: file-contains-trailing-whitespace debian/changelog (line 1055)
P: nginx source: file-contains-trailing-whitespace debian/changelog (line 1064)
P: nginx source: file-contains-trailing-whitespace debian/changelog (line 144)
P: nginx source: file-contains-trailing-whitespace ... use --no-tag-display-limit to see all (or pipe to a file/program)
P: nginx source: package-uses-old-debhelper-compat-version 10
P: nginx source: rules-requires-root-missing
N: 1 tag overridden (1 error)

- src:nginx could use some love with these lintian issues. Only one seems to be specific to geoip2, and is not serious though

* The package should not rely on obsolete or about to be demoted packages. That currently includes package dependencies on Python2 (without providing Python3 packages), and packages depending on GTK2.
- quite the opposite, the intention of this MIR is to have the nginx geoip module rely on geoip2.

UI standards:
- n/a

Dependencies:
* All binary dependencies (including Recommends:) must be satisfiable in main (i. e. the preferred alternative must be in main). If not, these dependencies need a separate MIR report (this can be a separate bug or another task on the main MIR bug)
- dependencies of libnginx-mod-http-geoip2_1.17.9-0ubuntu1_amd64.deb:
 Depends: nginx-common (= 1.17.9-0ubuntu1), libc6 (>= 2.14), libmaxminddb0 (>= 1.0.2)
- no Recommends
- libmaxminddb0 is being handled in MIR https://bugs.launchpad.net/ubuntu/+source/libmaxminddb/+bug/1861101

Standards compliance:
* The package should meet the FHS and Debian Policy standards. Major violations should be documented and justified. Also, the source packaging should be reasonably easy to understand and maintain.
- speaking specifically about the bin:libnginx-mod-http-geoip2 package, I don't see any violations. It's a standard module package, with a .so, config

Maintenance:
* The package must have an acceptable level of maintenance corresponding to its complexity:
- d/rules is not immediately trivial, but well organized, and uses debhelper

* All packages must have a designated "owning" team, regardless of complexity, which is set as a package bug contact.
- ubuntu server is already subscribed to src:nginx

Background information:
This is an odd mir in the sense that the src:nginx package is already in main, but at a later time another source was added as a third party module (there are a few in the package already, look in debian/modules), and that module wasn't part of the original MIR. So this is a MIR for a binary package, produced by src:nginx.

description: updated
description: updated
summary: - MIR: libnginx-mod-http-geoip2 (src:nginx)
+ MIR: bin:libnginx-mod-http-geoip2 from src:nginx

This is a bit of a special review,
the few packaging bits "on top of normal nginx" are just:
- ./debian/libnginx-mod-http-geoip2.nginx
- ./debian/libnginx-mod.conf/mod-http-geoip2.conf
and a few entries in d/rules
- debian/rules:16: http-geoip2 \
- debian/rules:122: --add-dynamic-module=$(MODULESDIR)/http-geoip2
- debian/rules:156: --add-dynamic-module=$(MODULESDIR)/http-geoip2

They all LGTM and don't need a full MIR review "again" for those few lines.
They are a bit complex but that just reflects how things are in nginx until modules can be proper external sources.

The actual source of it is in:
- debian/modules/http-geoip2/*

And that is the new bit as it is an external source and was not security reviewed when nginx was handled.
The source is from:
=> https://github.com/leev/ngx_http_geoip2_module

I re-did the sections that would apply for this code:
- does not FTBFS currently
- does not have a test suite that runs at build time
- no translation present, but none needed for this case (not user visible)?
- not a python package, no extra constraints to consider int hat regard
- symbols tracking not applicable for this kind of code.
- Upstream update history is ok
- promoting this does not seem to cause issues for MOTUs that so far
  maintained the package
- no massive Lintian warnings
- no geoip2 related issues int the build log
- no incautious use of malloc/sprintf (as far as I can check it)
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH
- no use of user nobody
- no use of setuid
- no dependency on webkit, qtwebkit, seed or libgoa-*
- no embedded source copies (well this IS the embedded source, but no further ones)

Problems:
- some others have ./debian/modules/watch entries but this one is missing
  add this to reduce the chance to miss updates
- the current release is not packaged (but not too far behind)
- one crasher bug open upstream, but resolved in 3.3

TODOs:
- @Andreas / Teward - would you mind bumping the embedded version to 3.3?
  - that is just a crash fix on DB reload
- @Andreas / Teward - would you mind adding geoip2 to ./debian/modules/watch?
- @Security - Since this is sort of the entry point where things might have more external control to be craftable a security review of just debian/modules/http-geoip2/* is recommended.
- @Security this raises an extra question if we need to set something up that this will be properly CVE tracked - could you outline if that will work well?

Assigning security for now.
Once reviewed the MIR Team can re-check if the other todo's were resolved in the meantime and then I think promotion can happen.

Changed in nginx (Ubuntu):
assignee: nobody → Ubuntu Security Team (ubuntu-security)
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers