[MIR] nghttp2

[Nish Aravamudan]

Availability: src:nghttp2 is in artful/universe and builds binary packages for all available architectures.

Rationale: In order to enable apache2's support for HTTP/2, libnghttp2-14 must be in main (libnghttp2-dev is added as a b-d for src:apache2, but it can remain in universe). The other binary packages produced by src:nghttp2 should remain in universe. While upstream Apache still describes mod_http2 as 'experimental' (https://httpd.apache.org/docs/2.4/mod/mod_http2.html), we presumably want to enable it by 18.04. It makes sense to process this MIR now so that we can work through issues with HTTP/2 support during the 17.10 cycle.

Rationale update: Upstream Apache has indicated 2.4.26 will 'unmark' mod_http2 has 'experimental': https://httpd.apache.org/docs/2.4/howto/http2.html#comments_section

Rationale update v2: The public page referred above already has removed the note and marked http2 as an "extension".

Quality assurance:

The only package needed at runtime to be in main is libnghttp2-14 (current ABI) itself. As this is only a library, there is no configuration required after installation.
No debconf questions are asked by default during the installation of libnghttp2-14.
There are no long-term outstanding bugs against libnghttp2.
Ubuntu bugs: https://bugs.launchpad.net/ubuntu/+source/nghttp2
 - There is one automated (it seems) report of a security issue with the 16.04 version. I have not yet confirmed if it was/is present in the latest versions.
  + I received a forward offlist between the bug reporter and upstream developer. This particular issue is in example code which documents potential usage, but is not part of the shipped library itself.
Debian bugs: https://bugs.debian.org/cgi-bin/pkgreport.cgi?dist=unstable;src=nghttp2
 - Two bugs, neither of which are serious.
Upstream issue tracker: https://github.com/nghttp2/nghttp2/issues
 - There are currently 175 open issues for the upstream project.
 - It is not yet clear how many of these issues are in the core library vs. the binary tools built by the upstream project.
The package seems to be well-maintained in Debian.
The package does not interact with exotic hardware (or hardware at all).
Based upon the latest build logs, some tests are run, but not all. I will investigate if all tests can be run during the build.
The package's debian/watch file is current.

UI standards: N/A, as the library is not a user-facing application.

Dependencies: The only binary dependency of libngttp2-14 is libc6 which is in main.

Standards compliance: The package meets the FHS and Debian Policy standards.

Maintenance: I have subscribed Ubuntu Server to the nghttp2 source package as the "owning" team.

Background information:

There are significant improvements associate with HTTP/2. It was deferred in 16.04 because it was considered 'too new' at the time. While Apache still describes the mod as 'experimental' it feels like 18.04 must ship the module in order to promote/support its adoption during the 5-year support cycle for that LTS release.

[Launchpad Janitor (janitor)]

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

[Mathieu Trudel-Lapierre (cyphermox)]

The only issue I see is not a blocker, but should probably be fixed:
I: libnghttp2-14: hardening-no-bindnow usr/lib/x86_64-linux-gnu/libnghttp2.so.14.13.3

Otherwise, since this is meant to support a reverse proxy, it should get a review by the Security Team. Reverse proxies are often security-sensitive, even if there is no CVE I could find in mitre.

[Seth Arnold (seth-arnold)]

I reviewed nghttp2 version 1.25.0-1 as checked into Debian unstable. This
shouldn't be considered a full security audit but rather a quick gauge of
maintainability.

nghttp2 provides an http/2 protocol interface that can be used by clients
and servers to handle the far more complicated http/2 protocol, compared
to the simpler http 1.1, 1.0, or 0.9 that many tools support.

Our database has three CVEs filed for nghttp2. The links are poor and
contemporaneous documentation on the nghttp2 website is poor. The authors
don't appear to have hidden the vulnerabilities but the entire CVE process
may be new to them or they've perhaps chosen to not assist.

Skimming release notes I think there may be vulnerabilities that still
need CVE numbers.

- The daemonizing code (not intended for main) looked fine
- pre/post inst/rm automatically generated
- initscript for (not intended for main) nghttpx looked fine
- systemd unit file for nghttpx looked simple
- No dbus
- No setuid
- binaries h2load, nghttp, nghttpx, nghttpd -- none should be in main
- no sudo fragments
- no udev rules
- test suite run during build, it says 1 test but I suspect that's just
  one file with many subtests
- no cron jobs
- build logs looked clean

- subprocess spawning (in non-main packages) looked fine
- extensive memory management; http/2 stream handling is extremely
  complicated but this package looked careful at every inspected instance
- logging looked safe
- environment variables seemed to be handled safely
- extensive privileged operations, looked careful at every inspected
  instance
- extensive encryption, openssl and libressl support, with neverbleed
  support too. Inspected instances looked safe.
- no privileged portions of code
- temp files looked to be handled safely
- no webkit
- no javascript
- no policykit
- cppcheck reports three errors in third-party/mruby/

I only made one note while reviewing that might reflect a weakness:

./src/shrpx_client_handler.cc sets aside 39 bytes for text display of ipv6
address -- ipv4 mapped addresses can be 45 bytes long. %interface scope
delimiters can be arbitrary in length. Are either of these possible?

And five comments along the way about the programming style or details

- memcpy() is used when unaligned accesses might be a problem
- memory management is complicated but many functions have precondition checks
- shrpx dropping-privs also supports neverbleed, looks neat
- Very thoughtful comments
- shrpx uses environment variables NGHTTPX_LISTENER4_FD
  NGHTTPX_LISTENER6_FD NGHTTPX_PORT NGHTTP2_UNIX_FD NGHTTP2_UNIX_PATH
  NGHTTPX_ACCEPT_* NGHTTPX_ORIG_PID, NOTIFY_SOCKET

I'm sure this code has more vulnerabilties, it's just too complicated a
protocol to not have more. But the authors have gone to great lengths to
check all calls for error returns, check preconditions, and provide
thoughtful helpful comments with links to source material.

I hope upstream will be willing to work with MITRE or other users to
assign CVEs for issue visibility when next needed.

Security team ACK for promoting nghttp2 (strictly the library packages as
needed) to main. (The non-library packages looked fine but I'd like there
to be a need for them bef...

[Nish Aravamudan (nacc)]

@cyphermox, with the security team's approval, is this now ack-able?

[Mathieu Trudel-Lapierre (cyphermox)]

I quickly re-reviewed nghttp2; it's fine to MIR (and has security team approval). MIR ACKed.

Let's try to fix the missing bindnow though in a future upload...
I: libnghttp2-14: hardening-no-bindnow usr/lib/x86_64-linux-gnu/libnghttp2.so.14.14.0

[Mathieu Trudel-Lapierre (cyphermox)]

Maybe you don't want to MIR the -doc package here, since it Depends on lynx | www-browser. There are www-browser's in main, but not typically installed on a server (w3m is in main but not part of the default install).

[Christian Ehrhardt  (paelzer)]

This will be enabled in apache2 for server support but since we bring it into main we also "no more disable" it in curl.
This can then also be used to test http2.

Added bug tasks and assigned me for now to test that.

[Christian Ehrhardt  (paelzer)]

LocutusOfBorg will cover the curl enablement on the current merge.
I tested from his ppa and it was ok.

Also tested the apache2 enablement - fine as well.
I not only dropped our disabling of http2 but also added a basic check to test it.
apache2 with changes is building in bionic-proposed now.

I'll report that autopkgtest to Debian now ...

[Adam Conrad (adconrad)]

Override component to main
nghttp2 1.28.0-1 in bionic: universe/misc -> main
Override [y|N]? y
1 publication overridden.
Override component to main
libnghttp2-14 1.28.0-1 in bionic amd64: universe/libs/optional/100% -> main
libnghttp2-14 1.28.0-1 in bionic arm64: universe/libs/optional/100% -> main
libnghttp2-14 1.28.0-1 in bionic armhf: universe/libs/optional/100% -> main
libnghttp2-14 1.28.0-1 in bionic i386: universe/libs/optional/100% -> main
libnghttp2-14 1.28.0-1 in bionic ppc64el: universe/libs/optional/100% -> main
libnghttp2-14 1.28.0-1 in bionic s390x: universe/libs/optional/100% -> main
libnghttp2-dev 1.28.0-1 in bionic amd64: universe/libdevel/optional/100% -> main
libnghttp2-dev 1.28.0-1 in bionic arm64: universe/libdevel/optional/100% -> main
libnghttp2-dev 1.28.0-1 in bionic armhf: universe/libdevel/optional/100% -> main
libnghttp2-dev 1.28.0-1 in bionic i386: universe/libdevel/optional/100% -> main
libnghttp2-dev 1.28.0-1 in bionic ppc64el: universe/libdevel/optional/100% -> main
libnghttp2-dev 1.28.0-1 in bionic s390x: universe/libdevel/optional/100% -> main
libnghttp2-doc 1.28.0-1 in bionic amd64: universe/doc/optional/100% -> main
libnghttp2-doc 1.28.0-1 in bionic arm64: universe/doc/optional/100% -> main
libnghttp2-doc 1.28.0-1 in bionic armhf: universe/doc/optional/100% -> main
libnghttp2-doc 1.28.0-1 in bionic i386: universe/doc/optional/100% -> main
libnghttp2-doc 1.28.0-1 in bionic ppc64el: universe/doc/optional/100% -> main
libnghttp2-doc 1.28.0-1 in bionic s390x: universe/doc/optional/100% -> main
Override [y|N]? y

[Launchpad Janitor (janitor)]

This bug was fixed in the package apache2 - 2.4.29-1ubuntu2

---------------
apache2 (2.4.29-1ubuntu2) bionic; urgency=medium

  * enable http2 (LP: #1687454) by stopping to disable it
    - debian/control: no more removed libnghttp2-dev Build-Depends (in universe).
    - debian/config-dir/mods-available/http2.load: no more removed.
    - debian/rules: no more removed proxy_http2 from configure.
  * d/t/control, d/t/check-http2: add basic test for http2 support

 -- Christian Ehrhardt <email address hidden> Tue, 05 Dec 2017 17:25:39 +0100

[Christian Ehrhardt  (paelzer)]

For completeness referencing the Debian bug I submitted the test.
Since this is not "the bug" which we adressed (enable http2) I do not fully link it as task.

=> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=884068

[Christian Ehrhardt  (paelzer)]

curl portion done since 7.57.0-1ubuntu1 - setting fix released.