[MIR] http-parser, dependency of sssd

Bug #1638957 reported by Matthias Klose
20
This bug affects 1 person
Affects Status Importance Assigned to Milestone
http-parser (Ubuntu)
Fix Released
High
Nish Aravamudan

Bug Description

[Availability]
Package is in universe since trusty:

$ rmadison http-parser
 http-parser | 2.1-2 | trusty/universe | source
 http-parser | 2.1-2 | xenial/universe | source
 http-parser | 2.1-2 | artful/universe | source
 http-parser | 2.7.1-2 | bionic/universe | source

Upstream: https://github.com/nodejs/http-parser

[Rationale]
sssd uses http-parser in its sssd-secrets service [https://docs.pagure.org/SSSD.sssd/design_pages/secrets_service.html], which has a REST API over a unix socket.

The Debian sssd package has the secrets service enabled, and disabling it in the Ubuntu package is part of the delta we carry.

The secrets service can be used as a generic key/value database for secrets, and one of its users is a kerberos KDC via KCM (Kerberos Cache Manager), implemented by sssd-kcm.

sssd-secrets is unix socket activated and won't be running until there is a connection to that socket.

The goal of this MIR is then twofold:
a) drop a delta we have with regards to debian
b) provide the sssd-secrets service for Ubuntu users

bug #1754365 has an MP and tests for the sssd-secrets service.

[Security]
ubuntu-security review in comment https://bugs.launchpad.net/ubuntu/+source/http-parser/+bug/1638957/comments/9

There are still no CVEs for http-parser or libhttp-parser.

OSS security mailing list search returns no hits for http-parser or libhttp-parser

No hits on the Ubuntu CVE Tracker.

No security relevant binaries in the package. The only indirect security implication is that this enables a new service in sssd: sssd-secrets, used to store secrets by wanting applications.

[Quality assurance]
 * After installing the package it must be possible to make it working with a reasonable effort of configuration and documentation reading.
It's a library and it installs without further configuration.

 * 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.
There are no debconf questions needed.

 * 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.
There are 3 ubuntu open bugs, of which this is one, and no closed bugs. These are the other 2 bugs:
bug #1677865: missing dep8 tests
bug #1733554: disable a failing test, caused by new http-parser

That last bug is a bit scarce on details.

 * The status of important bugs in Debian's, Ubuntu's, and upstream's bug tracking systems must be evaluated. Important bugs must be pointed out and discussed in the MIR report.

Debian has the same bug open regarding the failing test:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=882308

There are, as of now, 27 open issues in upstream's bug tracker:
https://github.com/nodejs/http-parser/issues

 * The package is maintained well in Debian/Ubuntu (check out the Debian PTS)
The low number of bugs may indicate it's not used a lot, or that it's not maintained at all.

A good number of upstream bugs have feedback.

* The package should not deal with exotic hardware which we cannot support.
Not the case here.

* 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.
The test suite runs at package build time:
   dh_auto_test
 make -j4 test
make[1]: Entering directory '/home/ubuntu/http-parser-2.7.1'
cc -Wdate-time -D_FORTIFY_SOURCE=2 -I. -DHTTP_PARSER_STRICT=1 -g -O2 -fdebug-prefix-map=/home/ubuntu/http-parser-2.7.1=. -fstack-protector-strong -Wformat -Werror=format-security -Wall -Wextra -Werror -O0 -g -c http_parser.c -o http_parser_g.o
cc -Wdate-time -D_FORTIFY_SOURCE=2 -I. -DHTTP_PARSER_STRICT=1 -g -O2 -(...)
./test_g
http_parser v2.7.1 (0x020701)
sizeof(http_parser) = 32
response scan 1/2 100%
response scan 2/2 100%
responses okay
request scan 1/4 100%
request scan 2/4 100%
request scan 3/4 100%
request scan 4/4 100%
requests okay
./test_fast
http_parser v2.7.1 (0x020701)
sizeof(http_parser) = 32
response scan 1/2 100%
response scan 2/2 100%
responses okay
request scan 1/4 100%
request scan 2/4 100%
request scan 3/4 100%
request scan 4/4 100%
requests okay

* 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.
There is a debian/watch file:
version=3

https://github.com/joyent/http-parser/tags .*/v?(\d.*)\.(?:tgz|tbz2|tar\.(?:gz|bz2|xz))

* 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.
The only dependency of the library binary is libc6:
$ dpkg --info libhttp-parser2.7.1_2.7.1-2_amd64.deb|grep ^\ Depends
 Depends: libc6 (>= 2.2.5)

The dev package only depends on the counterpart binary package:
$ dpkg --info libhttp-parser-dev_2.7.1-2_amd64.deb |grep ^\ Depends
 Depends: libhttp-parser2.7.1 (= 2.7.1-2)

[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)
From d/control:
Build-Depends: debhelper (>= 10~), dh-exec
There are no recommends or suggests.
Runtime Depends is only libc6, automatically selected.

[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.
From d/control:
Standards-Version: 4.1.1
There don't seem to be any standards violations.

[Maintenance]
This is a simple library package, which produces just two binary packages: the library itself and the dev package.
It's a straight sync from debian.

[Background information]
The summary and description of the package are well described:
Description: parser for HTTP messages written in C
 It parses both requests and responses. The parser is designed to be used in
 performance HTTP applications. It does not make any syscalls nor allocations,
 it does not buffer data, it can be interrupted at anytime. Depending on your
 architecture, it only requires about 40 bytes of data per message stream (in
 a web server that is per connection).

Tags: bionic
Matthias Klose (doko)
Changed in http-parser (Ubuntu):
assignee: nobody → Ubuntu Server Team (ubuntu-server)
importance: Undecided → High
milestone: none → ubuntu-16.11
Revision history for this message
Michael Terry (mterry) wrote :

I had some time so I took a quick look at this. But the server team should still flesh this out when they can and we'll do a fuller review.

- Needs a team bug subscriber.
- Seems unmaintained in Debian. No updates in 3 years and upstream has new releases (and repeated requests to update the package -- https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=795492). Are we willing to pick up that slack?

Revision history for this message
Timo Aaltonen (tjaalton) wrote :

I've added sssd team as bug subscriber. Apart from the new upstream release there doesn't seem to be too much to do.

Debian is frozen now, so no transitions are possible until stretch is released. That's too late for sssd though, since 1.14.x is a dependency of freeipa 4.4 which I have prepared for 17.04.

Revision history for this message
Michael Terry (mterry) wrote :

- I'd suggest subscribing ~ubuntu-server as well.

- The tests should be run as part of build and/or as an autopkgtest. "make test" should do the trick.

- Is there not another http parser in main that we could use instead?

- I'll pass to security team for a quick opinion -- parsing untrusted web responses seems sensitive.

Changed in http-parser (Ubuntu):
assignee: Ubuntu Server Team (ubuntu-server) → Ubuntu Security Team (ubuntu-security)
Revision history for this message
Timo Aaltonen (tjaalton) wrote :

A server team admin should add the bug subscription..

Tests are already run during build. There is no other parser to use that I know of, and if there werer that would need changing sssd too.

Revision history for this message
Jon Grimm (jgrimm) wrote :

Subscription by server team added. Thanks.

Timo Aaltonen (tjaalton)
Changed in http-parser (Ubuntu):
milestone: ubuntu-16.11 → none
status: Incomplete → Confirmed
milestone: none → ubuntu-17.04
Revision history for this message
Timo Aaltonen (tjaalton) wrote :

two months passed, what's next?

Revision history for this message
Michael Terry (mterry) wrote :

Just waiting on a security check.

Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

Bumping the milestone to ubuntu-17.10 so it remains on people's radar.

Changed in http-parser (Ubuntu):
milestone: ubuntu-17.04 → ubuntu-17.10
Revision history for this message
Seth Arnold (seth-arnold) wrote :

I reviewed http-parser version 2.1-2 as checked into zesty. This shouldn't
be considered a full security audit but rather a quick gauge of
maintainability.

No CVEs in our database

- http-parser provides an API with callbacks to handle HTTP parsing. It
  doesn't do any networking itself, strictly protocol parsing.

- Build-Depends: debhelper, dh-exec, dpkg-dev
- Does not daemonize
- No maintainer scripts
- No initscripts
- No dbus services
- No setuid
- No binaries in PATH
- No sudo fragments
- No udev rules
- Tests are run during the build; they're ugly in the build logs but
  they're there
- No cron jobs
- Clean build logs

- No subprocesses spawned
- No memory management
- No file IO
- No logging
- No environment variables
- No networking
- No privileged sections of code
- No cryptography
- No tmp files
- No webkit
- No JS
- No PolicyKit
- Clean cppcheck

http-parser is a by-hand character-by-character http parser. http is not
easy so neither is this code but it's remarkably clean given the
complexity involved. The state transitions are clearly labeled, error
handling appears defensive, bounds appeared to be checked.

Security team ACK for promoting http-parser to main.

Thanks

Changed in http-parser (Ubuntu):
assignee: Ubuntu Security Team (ubuntu-security) → nobody
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

This is acked quite a while now.
Is this coming to 18.04 still?

I subscribed Andreas and Timo who were the last people I heard talking about sssd - maybe you know about the current plans.

description: updated
description: updated
description: updated
description: updated
description: updated
description: updated
description: updated
description: updated
description: updated
description: updated
description: updated
description: updated
tags: added: bionic
removed: zesty
description: updated
Nish Aravamudan (nacc)
Changed in http-parser (Ubuntu):
assignee: nobody → Nish Aravamudan (nacc)
Revision history for this message
Nish Aravamudan (nacc) wrote :

Given the security team ack, and the package generally, fine to MIR

I believe the only package that needs promotion (other than the source):

libhttp-parser2.7.1

Changed in http-parser (Ubuntu):
status: Confirmed → Fix Committed
Revision history for this message
Steve Langasek (vorlon) wrote :

Override component to main
http-parser 2.7.1-2 in bionic: universe/misc -> main
libhttp-parser-dev 2.7.1-2 in bionic amd64: universe/libdevel/extra/100% -> main
libhttp-parser-dev 2.7.1-2 in bionic arm64: universe/libdevel/extra/100% -> main
libhttp-parser-dev 2.7.1-2 in bionic armhf: universe/libdevel/extra/100% -> main
libhttp-parser-dev 2.7.1-2 in bionic i386: universe/libdevel/extra/100% -> main
libhttp-parser-dev 2.7.1-2 in bionic ppc64el: universe/libdevel/extra/100% -> main
libhttp-parser-dev 2.7.1-2 in bionic s390x: universe/libdevel/extra/100% -> main
libhttp-parser2.7.1 2.7.1-2 in bionic amd64: universe/libs/extra/100% -> main
libhttp-parser2.7.1 2.7.1-2 in bionic arm64: universe/libs/extra/100% -> main
libhttp-parser2.7.1 2.7.1-2 in bionic armhf: universe/libs/extra/100% -> main
libhttp-parser2.7.1 2.7.1-2 in bionic i386: universe/libs/extra/100% -> main
libhttp-parser2.7.1 2.7.1-2 in bionic ppc64el: universe/libs/extra/100% -> main
libhttp-parser2.7.1 2.7.1-2 in bionic s390x: universe/libs/extra/100% -> main
Override [y|N]? y
13 publications overridden.

Changed in http-parser (Ubuntu):
milestone: ubuntu-17.10 → none
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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