[MIR] uwsgi as dependency of mailman3

Bug #1820227 reported by Christian Ehrhardt 
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
uwsgi (Ubuntu)
Won't Fix
Undecided
Unassigned

Bug Description

[Availability]
The uswgi source package is in Universe, and builds on amd64, arm64, armhf,
i386, ppc64el, s390x
https://launchpad.net/ubuntu/+source/uwsgi/2.0.18-1

Of the binary packages it produces, we need the following in main:
- uwsgi
- uwsgi-core
- uwsgi-plugin-python3

[Rationale]
This is part of the MIR activity for all dependencies of mailman3
The "main" MIR of it is at bug 1775427:

Mailman (2) has only python2 support, but we strive for python3,
therefore Mailman3 which has python3 support should be promoted to main.

[Security]
There are two CVEs in mitre:
- http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-7490
  uWSGI before 2.0.17 mishandles a DOCUMENT_ROOT check during use of the
  --php-docroot option, allowing directory traversal.
- http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-6758
  The uwsgi_expand_path function in core/utils.c in Unbit uWSGI through 2.0.15
  has a stack-based buffer overflow via a large directory length.

There are no hits in the ubuntu cve tracker at
http://people.ubuntu.com/~ubuntu-security/cve/universe.html

[Quality assurance]

As part of the mailman3 stacks as of now (Disco) this installs fine and works fine.
On itself it is useful to (many) other dependencies and does not need a post install configuration on its own.

There are no debconf questions.
Upstream bugs: https://github.com/unbit/uwsgi/issues
- there are 514 open bugs, and 774 closed ones
- just 10 open issues with the label "bug": https://github.com/unbit/uwsgi/labels/bug
Debian bugs:
- a bit hard to get a list of all bugs, given that this package produces 50 binary packages
- focusing on the wants we want to MIR, that reduces the list to:
  - https://bugs.debian.org/cgi-bin/pkgreport.cgi?package=uwsgi:
    - 1 normal, 1 minor, 5 wishlist
    - 2 important already resolved
  - https://bugs.debian.org/cgi-bin/pkgreport.cgi?package=uwsgi-core:
    - #772386: uwsgi-core: bashism in /bin/sh script (open)
    - #846362: flaw in readline implementation causing it to return excess data (closed)
  - https://bugs.debian.org/cgi-bin/pkgreport.cgi?package=uwsgi-plugin-python3: just one closed bug (#901774)
Ubuntu bugs: https://bugs.launchpad.net/ubuntu/+source/uwsgi
- 12 open bugs at first
- most untriaged, many in xenial. Did a quick triage and found many were dupes
  of #1616497, which is fixed in later releases.
- might need to fix https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=833067,
  which is about adding a systemd service file, specially since the package
  ships both an upstart and a sysV initscript at the same time.
- there is a build-depends on libqdbm-dev, which has an intent to orphan bug in debian:
  https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=890504
  libqdbm14 doesn't show up as a reverse-depends, though. Maybe it's not used
  anymore, or it's statically linked:
root@disco-uwsgi:~# reverse-depends libqdbm-dev -b
Reverse-Build-Depends
=====================
* php7.2
* php7.3
* sylfilter
* uwsgi
root@disco-uwsgi:~# reverse-depends libqdbm14
Reverse-Depends
===============
* libqdbm-dev
* libqdbm-java
* libqdbm-perl
* libsylfilter0
* libxqdbm3c2
* php7.2-dba
* php7.3-dba
* qdbm-cgi
* qdbm-util
* ruby-qdbm
  Disco build logs at (https://launchpadlibrarian.net/410732507/buildlog_ubuntu-disco-amd64.uwsgi_2.0.18-1_BUILDING.txt.gz
  also don't show "qdbm" other then when installing the build-dep itself.
- FTBFS in disco that I just filed while doing this evaluation:
  https://bugs.launchpad.net/ubuntu/+source/uwsgi/+bug/1820095

Debian seems to be keeping up with upstream releases.
No exotic hardware involved.
No DEP8 tests.
Doesn't look like it runs tests at package build time, but d/rules is complex
enough to parse and I may have missed it. I also checked build logs, though.
Finally, I ran "make check" manually, and "something" runs, but it doesn't look
like any test output I have seen before: http://paste.ubuntu.com/p/b6YvDmb44Z/

There is a working debian/watch file.

Lintian:
- As expected, there are many issues flagged by lintian. Focusing on the 3 packages we want to MIR, we have these:
  uwsgi:
    I: uwsgi: debian-news-entry-uses-asterisk
    W: uwsgi: script-not-executable usr/share/uwsgi/init/do_command
    W: uwsgi: script-not-executable usr/share/uwsgi/init/snippets
    W: uwsgi: script-not-executable usr/share/uwsgi/init/specific_daemon
    P: uwsgi: missing-systemd-service-for-init.d-script uwsgi
  uwsgi-core:
    - many hardening-no-fortify-functions
    - a few shared-lib-without-dependency-information
  uwsgi-plugin-python3:
    - just one hardening-no-bindnow

Relying on obsolete packages:
As shown earlier, the package is relying on qdbm which is orphaned in Debian,
but there is no match for "qdbm" in the build logs. Seems it was originally
added by this:
  uwsgi (1.9.11-1) unstable; urgency=low
  (...)
    * New binary package uwsgi-plugin-php. (Closes: #699174)
      - New Build-Depends libphp5-embed, php5-dev, libonig-dev, libdb-dev,
        libqdbm-dev and libbz2-dev.
   -- Janos Guljas <email address hidden> Mon, 27 May 2013 03:55:54 +0200

  The uwsgi-plugin-php package was later dropped, however, in 2.0.15-10:
  uwsgi (2.0.15-10) unstable; urgency=medium

    * Simplify packaging by offloading some parts to separate source:
      + Stop build plugins for PHP.
      + Drop binary package uwsgi-plugin-php.
      + Stop build-depend on php-dev libphp-embed.
    (...)
   -- Jonas Smedegaard <email address hidden> Fri, 20 Oct 2017 16:13:39 +0200

Therefore, I think the libqdbm-dev build-dep can be dropped as well. I filed a
debian bug:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=924586

[UI standards]
Does not apply, as it's a service used by other services.

[Dependencies]

Some dependencies are not in main, but we drive MIR for all related packages
that are not in main at the same time.
Please check the list of bugs from the main Mailman3 MIR in bug 1775427 to get an overview.

[Standards compliance]
- d/rules is very complex and hard to understand:
  https://salsa.debian.org/uwsgi-team/uwsgi/blob/master/debian/rules
I didn't spot FHS violations.
It's up-to-date regarding standards versions: 4.3.0

[Maintenance]

The Server team will subscribe for the package for maintenance

[Background]
No further info at this time.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

FYI: The FTBFS fix is in progress and soon resolved.
FYI: but the package is also:
a) more complex
b) more likely to be a Deny or at least extra work to be triggered
Therefore I'm on next weeks meeting passing the review of this one to a fellow MIR team member

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Assigned to cyphermox in todays MIR Team meeting - thanks a lot for taking a look at this!

Changed in uwsgi (Ubuntu):
assignee: nobody → Mathieu Trudel-Lapierre (cyphermox)
Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

This is package is huge and terrible to review; I had a look at it, and I see a couple of places where it seems like it's security sensitive. To top that off, it's a CGI server, so obviously security sensitive in its own right. Let's have Security review it.

Changed in uwsgi (Ubuntu):
assignee: Mathieu Trudel-Lapierre (cyphermox) → Ubuntu Security Team (ubuntu-security)
Revision history for this message
Seth Arnold (seth-arnold) wrote :

I've been reading the uwsgi documentation and code for a few hours now; I fully concur with Mathieu's assessment.

It's amazing how much uwsgi can do. It's got plugins for a huge number of programming environments, storage backends, logging mechanisms, RPC mechanisms.. it goes on.

The documentation is surprisingly good. People who I know that run it seem to like it, in some cases way better than some of the alternatives.

But this is huge, the deps list is huge, and there's a vast amount of manual memory management in C. I haven't spotted any errors yet, but this kind of code quite simply *must* have security critical bugs in it.

I'm thinking this is an awful lot to include in main for mailman3.

Does anyone know if gunicorn could work for mailman3? I must admit I haven't actually looked at gunicorn yet, but it has to be simpler.

Another alternative is to keep the whole stack of applications in universe; I would feel bad about pulling a feature out of main, but the time spent on this could be spent elsewhere this cycle.

Thoughts?

Thanks

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Yeah, I agree that uwsgi is a beast.

When trying alternatives (after all WSGI is supposed to be a specification) there is a better candidate thou. gunicorn is in universe and big as well, but we'd have src:mod-wsgi providing httpd-wsgi as well through libapache2-mod-wsgi.
And that was already in main [1] and could easily be repromoted.
I guess we have to take the action to evaluate if this replacement would be fully functional.

[1]: https://bugs.launchpad.net/ubuntu/+source/mod-wsgi/+bug/566537

Changed in uwsgi (Ubuntu):
assignee: Ubuntu Security Team (ubuntu-security) → nobody
Revision history for this message
Seth Arnold (seth-arnold) wrote :

Here's the notes I took while reviewing this package:

About the source code:
uwsgi_calloc() re-introduces integer overflow bugs
cppcheck results are entirely false positives

About the debian packaging:
cdbs is unfortunate
gbp is difficult to work with
there's a huge number of binary packages
complex Depends, Suggests, Replaces, Conflicts, Provides
different binary packages have different supported architectures

I really liked the documentation, and it felt like there was a lot to recommend this service, but the huge amount of complexity and highly intricate memory management felt very likely to have security issues.

To be clear I didn't find any security issues: it's just that moving memory chunks across consumers and producers as this program does is notoriously difficult to keep correct under maintenance.

Thanks

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

After evaluating dependencies, required further changes and mostly maintainability for security and packaging it was decided there are too many concerns - not about any single package in particular, but the overall Mailman3 stack - about the ability to maintain and monitor it as well as we need it for support in main.

We have closed the primary LP bug already, the MIRs that are already approved will stay that way, but we will make no seed change to pull things in for now. Yet if other needs come up for those they have a prepared MIR already.
Other bugs - like this one - which are not yet completed in terms of review will be closed as Won't Fix.

Even thou it ended being aborted, I think that is a valid outcome of the MIR evaluations. Never the less I want to thank everybody involved for all the work spent in what was nearly a year working through these MIRs.

Changed in uwsgi (Ubuntu):
status: New → Won't Fix
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.