[MIR] twitter-bootstrap3 as dependency of mailman3

Bug #1820226 reported by Christian Ehrhardt 
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
twitter-bootstrap3 (Ubuntu)
In Progress
Undecided
Unassigned

Bug Description

[Availability]
The package is already universe for quite a while and build/works fine so far.
It is for example already used for https://lists.canonical.com/mailman3/postorius/lists/
OTOH it seems to be a pretty common project https://github.com/twbs/bootstrap that can/could be used for much more than just the mailman3 stack.

For the mailman stack we'd pull in both binaries for fonts-glyphicons-halflings and libjs-bootstrap

[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.

I know this is dragging in a lot of components, but mailman3 was re-implmented
using common frameworks and that meands django, node, ...

[Security]

This is one of the components of overall mailman3 stack that is not looking so good.
We know of 7 CVEs of which only one is known fixed.
The rest would need analysis and triage to be sure
=> https://people.canonical.com/~ubuntu-security/cve/pkg/twitter-bootstrap3.html

When checking on Mitre almost all of them are listed as "before 3.4" and Disco is on 3.4.0
=> http://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=Bootstrap
That leaves open:
=> http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-8331
It is up to the security Team to rate that and clarify if the version we have is affected or if the problem is not important.

Note: The archive also has libjs-bootstrap4 4.3.1 , but upstream seems to continue both series.
So we can follow Debian/Upstream-mailman to do the switch to that down the road (no urgency)

[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.

The package does not ask debconf questions.

No known bug in Ubuntu for this.
Debian has three non important low prio bugs open.

Upstream seems very active (or they had spammers in their issue tracker) but there are 295 open and 18007 closed bugs.
But also there are 64k Forks of the project, so numbers above might be true and this is important to our community?!
Yet to answer if they want/need an in-archvie version I don't know their usual delivery model enough.

The package seems to get regular updates by upstream and Debian.

No exotic HW involved.

The package does ship a few unitests in js/tests/unit/ but they are not running on build.
It recently added autopkgtests which compress (and thereby check) the created .js files.

No Lintian warning except a few newer Standards/Compat versions and watch GPG checks - nothing severe.

The package does not rely on demoted or obsolete packages.
Although there is a new major version twitter-bootstrap4 and we might want to ensure that we are on the latest track and not get bit-rot with V3.
If we need this for the promotion is up to the security team (see section above) but I have marked it as an optional pre 20.04 task already.

No new gt2k dependencies

[UI standards]

Internationalization is present through the twitter translation center according to upstream.
I haven't found the code for it, but it seems at least to exist "some way"
No End-user applications that needs a standard conformant desktop file.

[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]

The package meets the FHS and Debian Policy standards.

The packaging itself is complex as well, with many special cases.
Nothing totally insane fortuantely, but more potential bits to understand when providing service.

[Maintenance]

The Server team will subscribe for the package for maintenance

[Background]
The package description explains the general purpose and context of the package well.

CVE References

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

FYI: 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 twitter-bootstrap3 (Ubuntu):
assignee: nobody → Mathieu Trudel-Lapierre (cyphermox)
Revision history for this message
Mathieu Trudel-Lapierre (cyphermox) wrote :

Couldn't this simply work with libjs-bootstrap4? Seems like using bootstrap4 would be a net improvement anyway, especially sicne it then wouldn't be encumbered by the open CVEs.

For me; it's a tentative ACK for the MIR provided the Security Team also ACKs it. I'm not looking for a full code-review -- I did that, nothing jumps out (but yeah, sure, XSS CVEs); but I think we ought to have a formal decision from the security team as to whether they are fine with this as-is; or as I think, that it would be *much* preferable to use twitter-bootstrap4.

Changed in twitter-bootstrap3 (Ubuntu):
status: New → In Progress
assignee: Mathieu Trudel-Lapierre (cyphermox) → Ubuntu Server (ubuntu-server)
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Thanks Mathieu for your review!

For consideration of bootstrap4 I opened:
- https://gitlab.com/mailman/postorius/issues/350
- https://gitlab.com/mailman/hyperkitty/issues/229
But that won't be short term tasks.

Therefore I'm subscribing and assigning the security to review bootstrap3.

Note: the one known open CVE is at least backported and existing in 3.4.1.
That is already in Debian and can be synced next cycle.
So @Security please review 3.4.1 as in Debian for this MIR.

Changed in twitter-bootstrap3 (Ubuntu):
assignee: Ubuntu Server (ubuntu-server) → Ubuntu Security Team (ubuntu-security)
Revision history for this message
Eduardo Barretto (ebarretto) wrote :

I reviewed twitter-bootstrap3 3.4.0+dfsg-4 as checked into eoan. This shouldn't
be considered a full audit but rather a quick gauge of maintainability.

twitter-bootstrap3 is an open source toolkit for developing with HTML, CSS, and
JS.

- There are different versions of twitter-bootstrap in the archive, after some
  search we have that
  - twitter-bootstrap4: Highly maintained
  - twitter-bootstrap3: The 3.4.0 version landed in December 2018 and it shows
    that development is more focused in the 4.x version than in 3.x. See:
    https://blog.getbootstrap.com/2018/12/13/bootstrap-3-4-0/
    After the 3.4.0 release we had 3.4.1 (Feb 2019) which fixed a security
    issue. So it seems that they are doing the minimum of giving at least
    security updates to version 3. (we might want to consider updating to 3.4.1)
    It is used in mailman-website where you can manage lists. It is unclear to
    me if the version 3 is a hard dependency.
- CVE History:
  - 7 open CVEs
  - 1 still open in eoan CVE-2019-8331 (fixed in version 3.4.1)
  - All CVEs are XSS
- Build-Depends
  - cssmin,
  - debhelper,
  - lcdf-typetools,
  - node-less,
  - node-source-map,
  - node-uglify,
  - pandoc
- No pre/post inst/rm scripts
- No init scripts
- No systemd units
- No dbus services
- No setuid binaries
- No binaries in PATH
- No sudo fragments
- No udev rules
- Unit tests found in js/tests/
  - unit/ contains the unit test files for each Bootstrap plugin
  - vendor/ contains jQuery
  - visual/ contains "visual" tests which are run interactively in real browsers
    and require manual verification
- No cron jobs
- Build logs:
  - No security relevant warnings or errors
dpkg-scanpackages: warning: Packages in archive but missing from override file:
dpkg-scanpackages: warning: sbuild-build-depends-core-dummy
dpkg-scanpackages: info: Wrote 1 entries to output Packages file.
E: twitter-bootstrap3 changes: bad-distribution-in-changes-file unstable
N: 4 tags overridden (1 error, 3 warnings)

- Processes spawned
  - Mostly on Grunt, a javascript task runner that is embedded in this
    package, or documentation
- Memory management: looks like there's not much and seem ok.
- No file IO
- Logging only in Grunt
- No use of environment variables
- No use of privileged functions
- No use of encryption
- No temp files
- No use of networking
- Make use of WebKit
- No PolicyKit
- No shell scripts
- Multiple (most from test code, which might be low priority) NULL_RETURNS from Coverity analysis, mostly related to jquery.

Someone with better JS skills might want to check coverity results before we ACK/NACK.

Christian could you please assign someone to take a look on those warnings?

Attached goes the coverity output.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote : Re: [Bug 1820226] Re: [MIR] twitter-bootstrap3 as dependency of mailman3

> - There are different versions of twitter-bootstrap in the archive, after some
> search we have that
[...]
> It is used in mailman-website where you can manage lists. It is unclear to
> me if the version 3 is a hard dependency.

Yes it is, I have checked with upstream already for the same reason
(expect to be longer maintained) but the move seems to be non trivial.
So for now it is a hard dependency on v3

[...]

> - No security relevant warnings or errors
> dpkg-scanpackages: warning: Packages in archive but missing from override file:
> dpkg-scanpackages: warning: sbuild-build-depends-core-dummy
> dpkg-scanpackages: info: Wrote 1 entries to output Packages file.
> E: twitter-bootstrap3 changes: bad-distribution-in-changes-file unstable
> N: 4 tags overridden (1 error, 3 warnings)

[...]

> - Multiple (most from test code, which might be low priority) NULL_RETURNS from Coverity analysis, mostly related to jquery.
>
>
> Someone with better JS skills might want to check coverity results before we ACK/NACK.
>
> Christian could you please assign someone to take a look on those
> warnings?

First of all thanks for the review Eduardo!
Looking at your summary I wondered which warnings you meant.
a) the few dpkg-scanpackage warnings
b) the coverity report to be looked at with JS skills

Let me know and we will try to find (the right) someone.

Revision history for this message
Eduardo Barretto (ebarretto) wrote :

On Mon, 2019-06-03 at 05:54 +0000, Christian Ehrhardt  wrote:
> > - There are different versions of twitter-bootstrap in the archive,
> > after some
> > search we have that
>
> [...]
> > It is used in mailman-website where you can manage lists. It is
> > unclear to
> > me if the version 3 is a hard dependency.
>
> Yes it is, I have checked with upstream already for the same reason
> (expect to be longer maintained) but the move seems to be non
> trivial.
> So for now it is a hard dependency on v3
>

Thanks for confirming it!

> [...]
>
> > - No security relevant warnings or errors
> > dpkg-scanpackages: warning: Packages in archive but missing from
> > override file:
> > dpkg-scanpackages: warning: sbuild-build-depends-core-dummy
> > dpkg-scanpackages: info: Wrote 1 entries to output Packages file.
> > E: twitter-bootstrap3 changes: bad-distribution-in-changes-file
> > unstable
> > N: 4 tags overridden (1 error, 3 warnings)
>
> [...]
>
> > - Multiple (most from test code, which might be low priority)
> > NULL_RETURNS from Coverity analysis, mostly related to jquery.
> >
> >
> > Someone with better JS skills might want to check coverity results
> > before we ACK/NACK.
> >
> > Christian could you please assign someone to take a look on those
> > warnings?
>
> First of all thanks for the review Eduardo!
> Looking at your summary I wondered which warnings you meant.
> a) the few dpkg-scanpackage warnings
> b) the coverity report to be looked at with JS skills
>

Sorry for not being so clear, the warnings here means the coverity
report.

Changed in twitter-bootstrap3 (Ubuntu):
assignee: Ubuntu Security Team (ubuntu-security) → nobody
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 - like this one - 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 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.

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.