novnc no longer sets token inside cookie

Bug #1822676 reported by Mohammed Naser on 2019-04-01
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
High
Mohammed Naser
Rocky
Undecided
Unassigned
Stein
Medium
Lee Yarwood
openstack-ansible
Undecided
melanie witt

Bug Description

For a long time, noVNC set the token inside a cookie so that when the /websockify request came in, we had it in the cookies and we could look it up from there and return the correct host.

However, since the following commit, they've removed this behavior

https://github.com/novnc/noVNC/commit/51f9f0098d306bbc67cc8e02ae547921b6f6585c#diff-1d6838e3812778e95699b90d530543a1L173

This means that we're unable to use latest noVNC with Nova. There is a really gross workaround of using the 'path' override in the URL for something like this

http://foo/vnc_lite.html?path=?token=foo

That feels pretty lame to me and it will have all deployment tools change their settings. Also, this wasn't caught in CI because we deploy novnc from packages.

melanie witt (melwitt) wrote :

I'm investigating a fix for this in nova.

tags: added: console
Changed in nova:
assignee: nobody → melanie witt (melwitt)
importance: Undecided → High
status: New → Confirmed
melanie witt (melwitt) wrote :

I've opened the following pull request for noVNC:

https://github.com/novnc/noVNC/pull/1220

Reviewed: https://review.openstack.org/649186
Committed: https://git.openstack.org/cgit/openstack/openstack-ansible/commit/?id=33c4f9df3dfc263e5f9084999fade492c30e77fe
Submitter: Zuul
Branch: master

commit 33c4f9df3dfc263e5f9084999fade492c30e77fe
Author: Mohammed Naser <email address hidden>
Date: Mon Apr 1 16:50:15 2019 -0400

    Use non-broken commit for NoVNC

    NoVNC shipped a change that broke the entire Nova integration,
    this patch reverts to the change *just* before it in order to
    be able to ship a working NoVNC.

    Change-Id: Icde731c89c66669ac5df6cf524c608f6008a8668
    Related-Bug: #1822676

Changed in nova:
status: Confirmed → In Progress
melanie witt (melwitt) wrote :

nova patch is proposed here: https://review.opendev.org/649372

Reviewed: https://review.opendev.org/649372
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=9606c80402f6db20d62b689c58aa8f024183628a
Submitter: Zuul
Branch: master

commit 9606c80402f6db20d62b689c58aa8f024183628a
Author: Mohammed Naser <email address hidden>
Date: Tue Apr 2 11:34:58 2019 -0400

    Add 'path' query parameter to console access url

    Starting in noVNC v1.1.0, the token query parameter is no longer
    forwarded via cookie [1]. We must instead use the 'path' query
    parameter to pass the token through to the websocketproxy [2].
    This means that if someone deploys noVNC v1.1.0, VNC consoles will
    break in nova because the code is relying on the cookie functionality
    that v1.1.0 removed.

    This modifies the ConsoleAuthToken.access_url property to include the
    'path' query parameter as part of the returned access_url that the
    client will use to call the console proxy service.

    This change is backward compatible with noVNC < v1.1.0. The 'path' query
    parameter is a long supported feature in noVNC.

    Co-Authored-By: melanie witt <email address hidden>

    Closes-Bug: #1822676

    [1] https://github.com/novnc/noVNC/commit/51f9f0098d306bbc67cc8e02ae547921b6f6585c
    [2] https://github.com/novnc/noVNC/pull/1220

    Change-Id: I2ddf0f4d768b698e980594dd67206464a9cea37b

Changed in nova:
status: In Progress → Fix Released

Change abandoned by Lee Yarwood (<email address hidden>) on branch: stable/stein
Review: https://review.opendev.org/670972

Related fix proposed to branch: master
Review: https://review.opendev.org/671769

Changed in openstack-ansible:
assignee: nobody → melanie witt (melwitt)
status: New → In Progress
Matt Riedemann (mriedem) on 2019-07-24
Changed in nova:
assignee: melanie witt (melwitt) → Mohammed Naser (mnaser)

Reviewed: https://review.opendev.org/670972
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=186aff98b751b973dd5a7de9c8077b1a8bca0ba9
Submitter: Zuul
Branch: stable/stein

commit 186aff98b751b973dd5a7de9c8077b1a8bca0ba9
Author: Mohammed Naser <email address hidden>
Date: Tue Apr 2 11:34:58 2019 -0400

    Add 'path' query parameter to console access url

    Starting in noVNC v1.1.0, the token query parameter is no longer
    forwarded via cookie [1]. We must instead use the 'path' query
    parameter to pass the token through to the websocketproxy [2].
    This means that if someone deploys noVNC v1.1.0, VNC consoles will
    break in nova because the code is relying on the cookie functionality
    that v1.1.0 removed.

    This modifies the ConsoleAuthToken.access_url property to include the
    'path' query parameter as part of the returned access_url that the
    client will use to call the console proxy service.

    This change is backward compatible with noVNC < v1.1.0. The 'path' query
    parameter is a long supported feature in noVNC.

    Co-Authored-By: melanie witt <email address hidden>

    Closes-Bug: #1822676

    [1] https://github.com/novnc/noVNC/commit/51f9f0098d306bbc67cc8e02ae547921b6f6585c
    [2] https://github.com/novnc/noVNC/pull/1220

    Change-Id: I2ddf0f4d768b698e980594dd67206464a9cea37b
    (cherry picked from commit 9606c80402f6db20d62b689c58aa8f024183628a)

This issue was fixed in the openstack/nova 19.0.2 release.

Reviewed: https://review.opendev.org/674916
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=d72f24569ea9da1f045927471b8d27c41837bf4a
Submitter: Zuul
Branch: stable/rocky

commit d72f24569ea9da1f045927471b8d27c41837bf4a
Author: Mohammed Naser <email address hidden>
Date: Tue Apr 2 11:34:58 2019 -0400

    Add 'path' query parameter to console access url

    Starting in noVNC v1.1.0, the token query parameter is no longer
    forwarded via cookie [1]. We must instead use the 'path' query
    parameter to pass the token through to the websocketproxy [2].
    This means that if someone deploys noVNC v1.1.0, VNC consoles will
    break in nova because the code is relying on the cookie functionality
    that v1.1.0 removed.

    This modifies the ConsoleAuthToken.access_url property to include the
    'path' query parameter as part of the returned access_url that the
    client will use to call the console proxy service.

    This change is backward compatible with noVNC < v1.1.0. The 'path' query
    parameter is a long supported feature in noVNC.

    Co-Authored-By: melanie witt <email address hidden>

    Closes-Bug: #1822676

     Conflicts:
     doc/source/admin/remote-console-access.rst
     nova/tests/unit/console/test_websocketproxy.py

    NOTE(melwitt): The conflicts are due to the following changes not being
    in Rocky:

      I08991796aaced2abc824f608108c0c786181eb65
      I7f5f08691ca3f73073c66c29dddb996fb2c2b266

    [1] https://github.com/novnc/noVNC/commit/51f9f0098d306bbc67cc8e02ae547921b6f6585c
    [2] https://github.com/novnc/noVNC/pull/1220

    Change-Id: I2ddf0f4d768b698e980594dd67206464a9cea37b
    (cherry picked from commit 9606c80402f6db20d62b689c58aa8f024183628a)
    (cherry picked from commit 186aff98b751b973dd5a7de9c8077b1a8bca0ba9)

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers