[MIR] websockify, spice-html5

Bug #1108935 reported by Chuck Short on 2013-01-29
22
This bug affects 3 people
Affects Status Importance Assigned to Milestone
nova (Ubuntu)
High
Unassigned
spice-html5 (Ubuntu)
High
Unassigned
websockify (Ubuntu)
High
Unassigned

Bug Description

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

Chuck Short (zulcss) on 2013-01-29
summary: - [MIR] python-websockify
+ [MIR] websockify
Changed in websockify (Ubuntu):
status: New → Confirmed
importance: Undecided → High

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

Changed in websockify (Ubuntu):
status: Confirmed → Incomplete
Launchpad Janitor (janitor) wrote :

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

Changed in websockify (Ubuntu):
status: Incomplete → Expired
James Page (james-page) wrote :

Ressurecting for utopic; required for nova still.

Changed in websockify (Ubuntu):
status: Expired → New
Michael Terry (mterry) wrote :

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?

Changed in websockify (Ubuntu):
status: New → Incomplete
James Page (james-page) wrote :

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.

Changed in websockify (Ubuntu):
status: Incomplete → Won't Fix
James Page (james-page) wrote :

Re-visiting this for 16.10.

Changed in websockify (Ubuntu):
status: Won't Fix → New
description: updated
James Page (james-page) wrote :

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

Changed in websockify (Ubuntu):
status: New → Incomplete
Michael Terry (mterry) wrote :

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.

Changed in websockify (Ubuntu):
assignee: nobody → Ubuntu Security Team (ubuntu-security)
status: Incomplete → New
James Page (james-page) wrote :

<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) wrote :

spice it is!

Changed in spice-html5 (Ubuntu):
importance: Undecided → High
description: updated
summary: - [MIR] websockify
+ [MIR] websockify, spice-html5
Seth Arnold (seth-arnold) wrote :
Download full text (3.5 KiB)

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

Read more...

Seth Arnold (seth-arnold) wrote :

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

Changed in websockify (Ubuntu):
assignee: Ubuntu Security Team (ubuntu-security) → nobody
Didier Roche (didrocks) on 2018-09-25
Changed in websockify (Ubuntu):
status: New → Incomplete
Changed in spice-html5 (Ubuntu):
status: New → Incomplete
James Page (james-page) wrote :

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

Changed in websockify (Ubuntu):
assignee: nobody → Corey Bryant (corey.bryant)
assignee: Corey Bryant (corey.bryant) → Ubuntu OpenStack (ubuntu-openstack)
Launchpad Janitor (janitor) wrote :

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

Changed in spice-html5 (Ubuntu):
status: Incomplete → Expired
James Page (james-page) on 2018-11-27
Changed in spice-html5 (Ubuntu):
status: Expired → Incomplete
status: Incomplete → New
James Page (james-page) wrote :

websockify updated with review comments from security team for disco.

Changed in websockify (Ubuntu):
status: Incomplete → New
assignee: Ubuntu OpenStack (ubuntu-openstack) → nobody
Launchpad Janitor (janitor) wrote :

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

Changed in websockify (Ubuntu):
status: New → Fix Released
Launchpad Janitor (janitor) wrote :

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

Changed in spice-html5 (Ubuntu):
status: New → Confirmed
Changed in nova (Ubuntu):
status: New → Triaged
importance: Undecided → High
Corey Bryant (corey.bryant) wrote :

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

Changed in nova (Ubuntu):
status: Triaged → Fix Released
Steve Langasek (vorlon) wrote :

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

Changed in websockify (Ubuntu):
status: Fix Released → New
James Page (james-page) wrote :

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

Launchpad Janitor (janitor) wrote :

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

Changed in websockify (Ubuntu):
status: New → Confirmed

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

Changed in spice-html5 (Ubuntu):
assignee: nobody → Christian Ehrhardt  (paelzer)
Changed in websockify (Ubuntu):
assignee: nobody → 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.

Changed in websockify (Ubuntu):
status: Confirmed → Fix Committed
assignee: Christian Ehrhardt  (paelzer) → nobody

@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) wrote :

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) wrote :

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

Download full text (3.8 KiB)

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

Read more...

Changed in spice-html5 (Ubuntu):
assignee: Christian Ehrhardt  (paelzer) → nobody
Matthias Klose (doko) wrote :

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.

Changed in websockify (Ubuntu):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers