OpenStack Compute (nova)

novnc no longer sets token inside cookie

Bug #1822676 reported by Mohammed Naser
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Fix Released
High
Mohammed Naser
Rocky
Fix Committed
Undecided
Unassigned
Stein
Fix Committed
Medium
Lee Yarwood
openstack-ansible
In Progress
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.

Tags: console
Revision history for this message
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
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to openstack-ansible (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/649186

Revision history for this message
melanie witt (melwitt) wrote :

I've opened the following pull request for noVNC:

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to openstack-ansible (master)

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

OpenStack Infra (hudson-openstack)
Changed in nova:
status: Confirmed → In Progress
Revision history for this message
melanie witt (melwitt) wrote :

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (master)

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
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (stable/stein)

Fix proposed to branch: stable/stein
Review: https://review.opendev.org/670972

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on nova (stable/stein)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (master)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to openstack-ansible-os_tempest (master)

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)
Changed in nova:
assignee: melanie witt (melwitt) → Mohammed Naser (mnaser)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (stable/stein)

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)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (stable/rocky)

Fix proposed to branch: stable/rocky
Review: https://review.opendev.org/674916

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/nova 19.0.2

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (stable/rocky)

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)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to nova (master)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (master)

Reviewed: https://review.opendev.org/682946
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=2b23ee7a3074e92167e68be1329467e8d602e9bc
Submitter: Zuul
Branch: master

commit 2b23ee7a3074e92167e68be1329467e8d602e9bc
Author: melanie witt <email address hidden>
Date: Wed Sep 18 16:57:58 2019 +0000

    Add note about needing noVNC >= v1.1.0 with using ESX

    As discussed on the following review:

      https://review.opendev.org/674916

    this adds a note indicating that the version of noVNC needs to be at
    least v1.1.0 in order for the nova-novncproxy to work with ESX/ESXi
    hypervisors.

    Related-Bug: #1822676

    Change-Id: Ia4ba37b6d6a1e4b5c75e38f4bcc2bea1d9ba9560

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/nova 20.0.0.0rc1

This issue was fixed in the openstack/nova 20.0.0.0rc1 release candidate.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/nova 18.2.3

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (stable/queens)

Fix proposed to branch: stable/queens
Review: https://review.opendev.org/692573

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on nova (stable/queens)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to openstack-ansible (master)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on openstack-ansible (master)

Change abandoned by Dmitriy Rabotyagov (noonedeadpunk) (<email address hidden>) on branch: master
Review: https://review.opendev.org/671642
Reason: has been already implemented with https://review.opendev.org/#/c/729258/14/playbooks/defaults/repo_packages/nova_consoles.yml

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on nova (master)

Change abandoned by "Lee Yarwood <email address hidden>" on branch: master
Review: https://review.opendev.org/c/openstack/nova/+/768334

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

Other bug subscribers