[MIR] websockify, spice-html5

[Chuck Short]

> websockify

Availability: Currently in universe

Rationale: Dependency for nova console access

Security: No security history.

Quality Assurance: Package works out of the box with no prompting. There is no major bugs in Ubuntu and the is no major bugs in Debian.
Unit tests are run for py2 and py3 as part of the package build.

Standards Compliance: FHS and Debian Policy compliant.

Maintenance: Simple python package that the Ubuntu OpenStack Team will take care of.

Dependencies: All are in main

> spice-html5

Availability: Currently in universe

Rationale: Dependency for nova console access

Security: No security history.

Quality Assurance: Package works out of the box with no prompting. There is no major bugs in Ubuntu and the is no major bugs in Debian. No unit tests in the package AFAICT - html + javascript gluecode.

Standards Compliance: FHS and Debian Policy compliant.

Maintenance: Simple python package that the Ubuntu OpenStack Team will take care of.

Dependencies: All are in main apart from websockify.

[Michael Terry (mterry)]

Looks like the rebind script is a bit broken. It tries to point at /usr/bin/rebind.so for the LD_PRELOAD.

[Launchpad Janitor (janitor)]

[Expired for websockify (Ubuntu) because there has been no activity for 60 days.]

[James Page (james-page)]

Ressurecting for utopic; required for nova still.

[Michael Terry (mterry)]

Looks mostly fine. But there are tests that aren't being run.

Running "tox -c tests/tox.ini" showed some errors (looked to be test bitrot problems). Can those be fixed up and run?

[James Page (james-page)]

Michael

After some discussion with contributors to this project and within the Ubuntu Server team, I'm withdrawing this MIR for now; I'm not happy that enough QA goes on upstream prior to releases - the broken test suite is just one symptom of this.

For now, we will skip the websockify dependent tests in nova, and continue to provide the console proxy related packages from universe.

[James Page (james-page)]

Re-visiting this for 16.10.

[James Page (james-page)]

This will need pairing with either novnc or spice - checking with the security team on preference.

[Michael Terry (mterry)]

OK, my concerns from comments 1 and 4 are addressed. Tests seem fine now.

Overall packaging is fine. Passing to security for a quick look and to address comment 7.

[James Page (james-page)]

<tyhicks> jamespage: hi - I'm not familiar enough with either project to have a preference
 jamespage: considering that spice is already in main, that might be the route that results in the least amount of code going from universe to main
 tyhicks tych0
--> powersj (~<email address hidden>) has joined #ubuntu-server
<jamespage> tyhicks, that was my thinking as well
<tyhicks> sarnold: once you start your day, can you chime on whether you have any preference here? ^
<mdeslaur> tyhicks, jamespage: please make it spice, it's a better choice and supports(will support?) 3d acceleration
<jamespage> sounds like we are all in agreement :-)
--> robbiew (~robbiew@188.111.40.11) has joined #ubuntu-server
<tyhicks> sounds good

[James Page (james-page)]

spice it is!

[Seth Arnold (seth-arnold)]

I reviewed websockify version 0.8.0+dfsg1-7ubuntu1 as checked into artful.
This should not be considered a full security audit but rather a quick
gauge of maintainability.

Websockify has no CVEs in our database

- Websockify provides a daemon to turn arbitrary TCP sockets into
  "websockets" suitable for use with browsers. Typically JS in the
  browsers would then implement whatever protocol is being converted.

  There's multiple implementations of this tool in this package. The C
  version is not suitable for main. It does not appear to be built in this
  package, so I'd like to ask for this file to either be deleted or
  patched to add a poison pill like:

#error See https://bugs.launchpad.net/bugs/1108935 before using this file

- rebind will LD_PRELOAD symbol shadow bind() to force a program to
  listen on a different port

- Build-Depends: debhelper, dh-python, openstack-pkg-tools, python-dev,
  python-mox3, python-nose, python-numpy, python-setuptools, python3-dev,
  python3-mox3, python3-nose, python3-numpy, python3-setuptools,

- pre/post inst/rm commands install or remove 'websockify' alternatives
- websockify/websocket.py daemonize() implementation looks good; C
  daemonize() does not.
- No initscripts / systemd service files
- No dbus services
- No setuid
- python3-websockify, python2-websockify, and rebind executables in PATH
- No sudo fragments
- No udev rules
- Some tests appear to be run during the build
- No cronjobs
- Clean build logs

- Python subprocesses spawned fine
- Python file management looked fine
- Python logging looked fine
- Python networking looked fine
- Python privileged syscall handling looked fine
- No webkit
- No javascript
- No policykit

One question about the python code:

- ./websockify/websocket.py daemonize() sets umask(0), is this the best
  setting?

A packaging bug:

- /usr/lib/websockify/rebind.o appears in the websockify package, why?

A Debian standards-compliance bug:

- /usr/bin/rebind in the websockify package has no manpage

And a long list of reasons why the C version of the program is not
suitable for use. This list is not exhaustive, I stopped after finding
this many issues:

- ./other/websockify.c main() settings.listen_host and target_host buffer overflow

- ./other/websocket.c alloc_ws_ctx() doesn't check ctx->headers allocation
  failure as it does other allocations

- ./other/websocket.c free_ws_ctx() doesn't free ctx->headers as it does
  other allocations

- ./other/websocket.c parse_hixie76_key() divide by zero if key includes
  no spaces (BTW this for loop is O(N^2) unless the compiler hoists out
  the strlen(key) call)

- ./other/websocket.c ws_socket_ssl() msg buffer overflows via use_keyfile
  and certfile parameters

- ./other/websocket.c do_handshake() handshake buffer underflow -- use of
  recv(2) return value as an array index without doing any error checking

- ./other/websocket.c parse_handshake() buffer overflows for:
  - headers->origin
  - headers->version
  - headers->key1
  - headers->connection
  - headers->protocols
  - headers->key2

- ./other/websocket.c daemonize() setgid(getgid());

- ./other/websocket.c daemonize() setuid(getuid());

- ./other/websocket.c ...

[Seth Arnold (seth-arnold)]

I reviewed spice-html5 version 0.1.7-1 as checked into Debian unstable.
This should not be considered a full security audit. It's even pretty
sparse by my usual standards.

spice-html5 is a unique package -- its contents mostly don't influence
the machine where it is installed. It's also a huge amount of JavaScript,
which isn't a language that the security team knows well.

The upstream git repository appears moderately active, multiple patch
authors, none of the commits that I inspected looked embarrassing. The
initial git import was five years ago and 40k lines.

spice-html5 doesn't seem like an obvious undue risk but the point remains
that the security team isn't staffed to support web applications. We may
need support from elsewhere in the company to address issues raised here.

I tried linting the codebase with jslint.com. Some of the files came
through with minor-feeling quibbles and some files had faults on every
single line. I hope this says more about my expectations about JavaScript
linters than the code quality.

jslint.com reports
- don't use single-quote strings
- don't use 'this'
- don't declare vars in a loop body
- positively hates enums.js atKeynames.js
.. stopped here when I suspect this isn't the right tool for the job

Security team ACK for promoting spice-html5 to main.

Thanks

[James Page (james-page)]

Outstanding work on websockify to complete MIR process; assigning to ubuntu-openstack for resolution

[Launchpad Janitor (janitor)]

[Expired for spice-html5 (Ubuntu) because there has been no activity for 60 days.]

[James Page (james-page)]

websockify updated with review comments from security team for disco.

[Launchpad Janitor (janitor)]

This bug was fixed in the package websockify - 0.8.0+dfsg1-10ubuntu2

---------------
websockify (0.8.0+dfsg1-10ubuntu2) disco; urgency=medium

  * Update for Ubuntu MIR based on Ubuntu Security team review
    (LP: #1108935):
    * d/p/error-websockify-c.patch: Add compiler errors to websockify
      C implementation as it has a number of identified security issues
      which remain unresolved.
    * d/websockify.install: Drop install of rebind.o.

 -- James Page <email address hidden> Wed, 19 Dec 2018 10:51:46 +0000

[Launchpad Janitor (janitor)]

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

[Corey Bryant (corey.bryant)]

This is fixed for nova in 2:19.0.0~b1~git2019013113.33aad0fe41-0ubuntu2.

[Steve Langasek (vorlon)]

This MIR was wrongly closed by an upload of the websockify package. As far as I can see it still needs approval before promotion.

[James Page (james-page)]

@vorlon quite correct - the upload contained the requested fix for the security team review.

[Launchpad Janitor (janitor)]

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

[Christian Ehrhardt  (paelzer)]

James and I were talking on remaining blockers on Disco.
[07:43] <cpaelzer> there is more history to be read on that one for sure ...
[07:45] <cpaelzer> jamespage: this actually is ok already IMHO
[07:45] <cpaelzer> in comment #8 you got the MIR ACK
[07:46] <cpaelzer> in comment #11 and #12 you got the conditional security ack
[07:46] <cpaelzer> and you later adressed these conditions
[07:46] <jamespage> cpaelzer: yup that summarises it nicely!
[07:47] <cpaelzer> OTOH this review stretched 6 years now :-/
[07:47] <cpaelzer> well the active part 2.5 years
[07:47] <cpaelzer> Given that former versions where security acked I'll do a full recheck this morning with that in mind
[07:47] <cpaelzer> most likely that will unblock it for you then

[Christian Ehrhardt  (paelzer)]

I was only checking changes to websockify after 2016-07-11 (last ack by mterry) if they would make it a Nack again.
Ok, none of the changes has made it worse.

But back then python2 removal wasn't a mission yet, but important now.
I'm slightly concerned as e.g. the 'websockify' binary depends on python-numpy (py2 version).
But nova as in proposed will only pull in python3-websockify, and that is what will be promoted (none of the py2 binaries will).
I ensured that python3-nova (where the dependency was added) can be installed without pulling in python2.

=> re-Ack websockify
Due to all of the above, setting websockify to approved (since nova is in proposed that means Fix Committed).

Next will be spice-html5 which got a security but not MIR check yet as it seems reading through former bug updates.

[Christian Ehrhardt  (paelzer)]

@JamesPage - just to be sure, you tested and are ok with spice-html5 being sufficient for your needs in Openstack? I wonder as [1] still calls it a prototype, there is a long list of major TODOs reworking the functionality. OTOH [3] isn't too bad and [4] shows some renewed activity. Maybe for 19.10 and OS-T release make sure you are on 0.2.1or later then.

I'll go on with the review, but I wanted to be sure that is what you will need and use.

[1]: https://www.spice-space.org/spice-html5.html
[2]: https://gitlab.freedesktop.org/spice/spice-html5/raw/master/TODO
[3]: https://gitlab.freedesktop.org/spice/spice-html5/issues
[4]: https://gitlab.freedesktop.org/spice/spice-html5/tags

[James Page (james-page)]

nova-spiceproxy has not yet been seeded so spice-html5 has not appeared in component mismatches.

I'll need to talk to the release team about that.

[James Page (james-page)]

Based on #24 I think we need to push out the seeding of nova-spiceproxy.

[Christian Ehrhardt  (paelzer)]

[Duplication]
- no such function in main

[Embedded sources and static linking]
There is plenty of js code, but that is the actual program.
None of it seems to be an embedded copy, but as Seth already mentioned IMHO javascript experts are rare within Ubuntu so be sure when you own the package to be willing and able to support it.

- no static linking
- no golang

[Security]
There are numerous spice CVEs https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=spice
As well as https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=html5
Which makes it hard to search for spice-html5, but I found none and Seth has given security Ack already.
Does not
- runs a daemon as root
- uses webkit1,2
- uses lib*v8 directly
- opens a port
- uses centralized online accounts
- integrates arbitrary javascript into the desktop
- deals with system authentication (eg, pam), etc)

It does to some extend (as it is one side of a spice protocol connection):
- parses data formats
- processes arbitrary web content

[Common blockers]
- this does not actually build, so no FTBFS
- Unfortunately it has no test suite to run.
- openstack team is already subscribed
- not a python package

Not so good:
It has user visible messages in browser, but I found no translation.
The project just isn#t that far evolved

[Packaging red flags]
- no Ubuntu delta
- no lib -> no symbols tracking
- watch file present
- Lintian is happy except usual non critical warnings
- d/rules is very clean (no real build)
- no golang, so no extra considerations for that

Not so good:
As outlined in comment #24 this seems to be potentially not meant for production level.
But that is the decision of the team owning it to maintain it still (or consider alternatives).

Also the current version is 0.2.1 and we only have 1.7, but that is fairly recent so that is more a "please update" for 19.10 then.

[Upstream red flags]
- overall it seems a bit incomplete, I asked about that in comment #24 but JamesPage said it is ok for their needs.
- malloc/sprintf / sudo all doesn't really apply as it is JS code that runs in the browsers sandboxed mode
- there open bugs and slow responses to them, but none critical for us
- no Dependency on webkit, qtwebkit, seed or libgoa-*

Not so good:
This is essentially a copy of the code without any checks.
If upstream adds even syntax errors we won't spot it.
I know JS isn't built, but maybe we could replace the build with a syntax checker and/or other validtion tools then?

[Summary]
This seems ok from the MIR teams POV in general.
There is a bit of a low quality expectation due to upstream considering themselves still only a prototype.
I'm somewhat afraid this will be pulled in only because it "should (tm)" work with websockify.
On my security concerns you have already Seth's Ack (who also wasn't too happy), so I'm not challengig that too much.
We just agreed on IRC to punt spice to next cycle:
[09:31] <jamespage> cpaelzer: lets push the spice-html5 to next cycle - sounds like we need to re-review anyway

You have an ACK from the MIR team under a bunch of constraints outlined below which you should resolve before this can go into 19.10 then:

To ensure a base level (requirement for the ack)
-...

[Matthias Klose (doko)]

Override component to main
websockify 0.8.0+dfsg1-10ubuntu2 in disco: universe/python -> main
python-websockify 0.8.0+dfsg1-10ubuntu2 in disco amd64: universe/python/optional/100% -> main
python-websockify 0.8.0+dfsg1-10ubuntu2 in disco arm64: universe/python/optional/100% -> main
python-websockify 0.8.0+dfsg1-10ubuntu2 in disco armhf: universe/python/optional/100% -> main
python-websockify 0.8.0+dfsg1-10ubuntu2 in disco i386: universe/python/optional/100% -> main
python-websockify 0.8.0+dfsg1-10ubuntu2 in disco ppc64el: universe/python/optional/100% -> main
python-websockify 0.8.0+dfsg1-10ubuntu2 in disco s390x: universe/python/optional/100% -> main
python3-websockify 0.8.0+dfsg1-10ubuntu2 in disco amd64: universe/python/optional/100% -> main
python3-websockify 0.8.0+dfsg1-10ubuntu2 in disco arm64: universe/python/optional/100% -> main
python3-websockify 0.8.0+dfsg1-10ubuntu2 in disco armhf: universe/python/optional/100% -> main
python3-websockify 0.8.0+dfsg1-10ubuntu2 in disco i386: universe/python/optional/100% -> main
python3-websockify 0.8.0+dfsg1-10ubuntu2 in disco ppc64el: universe/python/optional/100% -> main
python3-websockify 0.8.0+dfsg1-10ubuntu2 in disco s390x: universe/python/optional/100% -> main
websockify 0.8.0+dfsg1-10ubuntu2 in disco amd64: universe/python/optional/100% -> main
websockify 0.8.0+dfsg1-10ubuntu2 in disco arm64: universe/python/optional/100% -> main
websockify 0.8.0+dfsg1-10ubuntu2 in disco armhf: universe/python/optional/100% -> main
websockify 0.8.0+dfsg1-10ubuntu2 in disco i386: universe/python/optional/100% -> main
websockify 0.8.0+dfsg1-10ubuntu2 in disco ppc64el: universe/python/optional/100% -> main
websockify 0.8.0+dfsg1-10ubuntu2 in disco s390x: universe/python/optional/100% -> main
websockify-common 0.8.0+dfsg1-10ubuntu2 in disco amd64: universe/python/optional/100% -> main
websockify-common 0.8.0+dfsg1-10ubuntu2 in disco arm64: universe/python/optional/100% -> main
websockify-common 0.8.0+dfsg1-10ubuntu2 in disco armhf: universe/python/optional/100% -> main
websockify-common 0.8.0+dfsg1-10ubuntu2 in disco i386: universe/python/optional/100% -> main
websockify-common 0.8.0+dfsg1-10ubuntu2 in disco ppc64el: universe/python/optional/100% -> main
websockify-common 0.8.0+dfsg1-10ubuntu2 in disco s390x: universe/python/optional/100% -> main
25 publications overridden.

[James Page (james-page)]

In response to Christian's list of TODO's in his review

To ensure a base level (requirement for the ack)
- set someone down a day installing that fo real
- use it with Openstack
- (try to) use it without openstack as well
- is it really providing what you want/need?
TODO => State on the bug the result of your testing!

I've tested both in the context of OpenStack, and standlone with websockify and libvirt to validate that spice-html5 is function and works as intended. There are some warning messages about unsupported features but it works OK. Its essential to use the virtio video adapter option but I was able to login and control a default 20.04 cloud image VM running under libvirt.

- check all the general Spice CVEs if any apply to this JS based code (might just not be tracked against spcie-html5 but apply)
TODO => State on the bug the result of your CVE check per CVE why they do not apply!

Rechecked general SPICE CVEs:

https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=spice

Unable to find any that relate to spice-html5.

I also searched for some of the 3rd party js files:

https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=jsbn
https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=SHA-1

but was unable to find any related open CVE's

- update to 0.2.x
TODO => Then feel free to set it to "in progress" to reflect that it is approved.

Done and tested as part of this review.

[Christian Ehrhardt  (paelzer)]

Thank you jamespage.
Yes with:
 spice-html5 | 0.2.2-0ubuntu1 | focal-proposed/universe | source

Everything requested is in place.
MIR Ack for spice-html5 (the rest was already ok)

I don't see it in mismatches yet, so I'm marking it "In Progress" for now-

[Launchpad Janitor (janitor)]

This bug was fixed in the package spice-html5 - 0.2.2-0ubuntu1

---------------
spice-html5 (0.2.2-0ubuntu1) focal; urgency=medium

  * New upstream release, completing Ubuntu MIR actions (LP: #1108935):
    - d/p/fix-windows-spice-upside-down.patch: Drop, included in
      upstream release.
    - d/p/Handle_non_topdown_bitmaps.patch: Drop, included in
      upstream release.
    - d/install: Refactor for src subdirectory for .js files.

 -- James Page <email address hidden> Wed, 01 Apr 2020 09:56:44 +0100

[Christian Ehrhardt  (paelzer)]

spice-html5 seems ready for promotion.
@AAs please could you promote this?

[Christian Ehrhardt  (paelzer)]

On promotion please recheck if the package subscription was done, ddstreet pinged jamespage already.
Just to be sure before moving to main.

[James Page (james-page)]

subscription added today for ubuntu-openstack (apologies missed before).

[Steve Langasek (vorlon)]

Override component to main
spice-html5 0.2.2-0ubuntu1 in focal: universe/misc -> main
spice-html5 0.2.2-0ubuntu1 in focal amd64: universe/web/extra/100% -> main
spice-html5 0.2.2-0ubuntu1 in focal arm64: universe/web/extra/100% -> main
spice-html5 0.2.2-0ubuntu1 in focal armhf: universe/web/extra/100% -> main
spice-html5 0.2.2-0ubuntu1 in focal i386: universe/web/extra/100% -> main
spice-html5 0.2.2-0ubuntu1 in focal ppc64el: universe/web/extra/100% -> main
spice-html5 0.2.2-0ubuntu1 in focal riscv64: universe/web/extra/100% -> main
spice-html5 0.2.2-0ubuntu1 in focal s390x: universe/web/extra/100% -> main
8 publications overridden.