MIR: bin:libnginx-mod-http-geoip2 from src:nginx
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
nginx (Ubuntu) |
Fix Released
|
Undecided
|
Unassigned |
Bug Description
nginx is already in main, but the binary package libnginx-
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-
1. https:/
2. https:/
Source: https:/
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-
https:/
https:/
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-
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:/
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://
- I tried a few keywords: geoip2, "nginx geoip2", "nginx geoip", "ngx_http_
check OSS security mailing list (feed 'site:www.
- "site:www.
- "site:www.
- "site:www.
- "site:www.
Ubuntu CVE Tracker
* http://
- 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://
- nothing for "geoip", "geoip2" or "nginx"
* http://
- 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/
- bin:libnginx-
* Packages which open privileged ports (ports < 1024).
- bin:libnginx-
* 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/
- 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:/
- Debian bugs:
- src:nginx bugs: https:/
- 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:/
- 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]: @@@@@@@
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/
debian/
debian/
debian/
* 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/
- 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=pgpsigurlm
https:/
* 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-
W: nginx-common: maintainer-
W: nginx-common: maintainer-
W: nginx-common: maintainer-
W: nginx source: orig-tarball-
I: nginx source: duplicate-
I: nginx source: duplicate-
I: libnginx-
I: nginx source: out-of-
I: nginx source: public-
I: libnginx-mod-rtmp: spelling-
I: libnginx-mod-rtmp: spelling-
I: libnginx-
P: nginx source: debug-symbol-
P: nginx source: duplicate-
P: nginx source: file-contains-
P: nginx source: file-contains-
P: nginx source: file-contains-
P: nginx source: file-contains-
P: nginx source: package-
P: nginx source: rules-requires-
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-
Depends: nginx-common (= 1.17.9-0ubuntu1), libc6 (>= 2.14), libmaxminddb0 (>= 1.0.2)
- no Recommends
- libmaxminddb0 is being handled in MIR https:/
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-
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, libnginx- mod-http- geoip2. nginx libnginx- mod.conf/ mod-http- geoip2. conf module= $(MODULESDIR) /http-geoip2 module= $(MODULESDIR) /http-geoip2
the few packaging bits "on top of normal nginx" are just:
- ./debian/
- ./debian/
and a few entries in d/rules
- debian/rules:16: http-geoip2 \
- debian/rules:122: --add-dynamic-
- debian/rules:156: --add-dynamic-
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: modules/ http-geoip2/ *
- debian/
And that is the new bit as it is an external source and was not security reviewed when nginx was handled. /github. com/leev/ ngx_http_ geoip2_ module
The source is from:
=> https:/
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: modules/ watch entries but this one is missing
- some others have ./debian/
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: modules/ watch? modules/ http-geoip2/ * is recommended.
- @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/
- @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/
- @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.