[OSSA-2021-002] Open Redirect in noVNC proxy (CVE-2021-3654)

Bug #1927677 reported by Swe W Aung
280
This bug affects 2 people
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Fix Released
Undecided
Unassigned
Stein
In Progress
Undecided
Unassigned
Train
Fix Released
Undecided
Unassigned
Ussuri
Fix Released
Undecided
Unassigned
Victoria
Fix Released
Undecided
Unassigned
Wallaby
Fix Released
Undecided
Unassigned
OpenStack Security Advisory
Fix Released
Undecided
Unassigned

Bug Description

This bug report is related to Security.

Currently novnc is allowing open direction, which could potentially be used for phishing attempts

To test.
https://<sites' vnc domain>//example.com/%2F..
include .. at the end

For example:
http://vncproxy.my.domain.com//example.com/%2F..

It will redirect to example.com. You can replace example.com with some legitimate domain or spoofed domain.

The description of the risk is
By modifying untrusted URL input to a malicious site, an attacker may successfully launch a phishing scam and steal user credentials.
Because the server name in the modified link is identical to the original site, phishing attempts may have a more trustworthy appearance.

CVE References

Revision history for this message
Swe W Aung (sirswa) wrote :

You can also test from the host that running novnc service,

nova:~# curl -v 'http://127.0.0.1:6080//google.com/%2F..'
* Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 6080 (#0)
> GET //google.com/%2F.. HTTP/1.1
> Host: 127.0.0.1:6080
> User-Agent: curl/7.58.0
> Accept: */*
>
< HTTP/1.1 301 Moved Permanently
< Server: WebSockify Python/3.6.9
< Date: Fri, 07 May 2021 04:49:39 GMT
< Location: //google.com/%2F../
* no chunk, no close, no size. Assume close to signal end
<

Revision history for this message
Jeremy Stanley (fungi) wrote :

Since this report concerns a possible security risk, an incomplete
security advisory task has been added while the core security
reviewers for the affected project or projects confirm the bug and
discuss the scope of any vulnerability along with potential
solutions.

description: updated
Changed in ossa:
status: New → Incomplete
Revision history for this message
melanie witt (melwitt) wrote :

This bug report reminds me of an old bug [1] we dealt with in the past where the canned vnc_auto.html and vnc.html pages allowed injection of arbitrary HTML into them (fixed in noVNC 0.6.2) [2].

vnc_auto.html (vnc_lite.html as of v1.0.0) and vnc.html have a feature where a host and port can be specified as query parameters in the URL, example [3]:

  http://1.2.3.4:6080/vnc_auto.html?host=6.7.8.9&port=6080

and it will connect to a noVNC server running on that host:port as the source of data provided to vnc_auto.html. The bug [2] meant that if a user specified host:port in the URL query parameters, a potentially malicious noVNC server running on that host:port could inject arbitrary HTML into the vnc_auto.html being served on the user's machine.

I mention that because it seems like the host:port functionality could be similarly used to phish. I'm thinking if someone ran their own noVNC server at host:port and got a user to click on a link with ?host&port in it, they could steal credentials if the user didn't notice what machine they're connecting to.

If that's the case, I'm not sure this redirect behavior is much different than what is already built-in to the vnc_lite.html and vnc.html pages that come with noVNC.

Aside from that, it's not clear to me whether this redirect behavior is something we (nova) control or if it's being done by noVNC itself. If it's the latter, I'm not sure whether we could do anything to intercept it or if it's something that would have to be changed in noVNC.

I'm going to add noVNC to this bug to get their input about the redirect behavior.

[1] https://bugs.launchpad.net/horizon/+bug/1656435
[2] https://github.com/novnc/noVNC/issues/748
[3] https://github.com/novnc/noVNC/blob/v1.1.0/vnc_lite.html#L14-L15

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

> I'm going to add noVNC to this bug to get their input about the redirect behavior.

Looks like I can't do that because noVNC and websockify use github issues and those can't be private security. But if it is indeed an issue in noVNC or websockify, then it is probably OK to go ahead and report it publicly in those projects.

I'm going to do some local testing to determine whether this redirect is something we could intercept and handle or if it happens before we (websockify plugin) are called. If it doesn't appear we can intercept it, we can switch this to public and I can report an issue for noVNC or websockify on github.

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

OK, I really went down the rabbit hole with this one.

The tl;dr is that this is a known issue in the python standard library [1], in the http.server.SimpleHTTPRequestHandler, which WebSockifyRequestHandler derives from and which we ultimately derive from with our NovaProxyRequestHandler.

I found that we _can_ intercept this in our code and prevent an open redirect. It could be considered hacky, but I'm attaching a patch that prevents the redirect. It is code copied from a comment on the python issue [2].

The concern about the sample code in the issue is that such code might reject legitimate requests in certain cases. I don't believe we have such a concern with the nova console proxy.

Let me know what you think.

[1] https://bugs.python.org/issue32084
[2] https://bugs.python.org/issue32084#msg306545

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

Also, I'm thinking we could make this bug public considering the root cause of the behavior is a public issue in the python standard http.server.SimpleHTTPRequestHandler.

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

Also, I have tested the patch in comment 5 in devstack and verified it works to return a 400 Bad Request if "//" are included in the URL to redirect, provided that the browser has not previously cached a past redirect.

I used the following URL to test: http://127.0.0.1:6080//google.com/%2F..

Jeremy Stanley (fungi)
description: updated
information type: Private Security → Public
information type: Public → Public Security
Revision history for this message
Jeremy Stanley (fungi) wrote :

Thanks for digging into this, Melanie! I've ended the embargo and switched to Public Security given the relatively low risk this represents and its relationship with known issues in WebSockify/stdlib.

If the patch is sufficient and gets backported to stable branches, we could issue an advisory (class A in our report taxonomy). We could also consider it a workaround for a bug in a dependency (class C2), but that gets into determining whether the vulnerability is in the dependency or merely in the way we're using it. I'll leave the security advisory task incomplete for the time being, and we'll see how the fix progresses in review.

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

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/nova/+/791297

Changed in nova:
status: New → In Progress
melanie witt (melwitt)
tags: added: console
Revision history for this message
Swe W Aung (sirswa) wrote : Re: [Bug 1927677] Re: novnc allowing open direction which could potentially be used for phishing

Hi Melanie

The is for the investigation and your effort. Really appreciate that.

Regards
sw3

On Sat, 15 May 2021 at 1:06 am, melanie witt <email address hidden>
wrote:

> ** Also affects: nova/train
> Importance: Undecided
> Status: New
>
> ** Also affects: nova/ussuri
> Importance: Undecided
> Status: New
>
> ** Also affects: nova/wallaby
> Importance: Undecided
> Status: New
>
> ** Also affects: nova/victoria
> Importance: Undecided
> Status: New
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1927677
>
> Title:
> novnc allowing open direction which could potentially be used for
> phishing
>
> Status in OpenStack Compute (nova):
> In Progress
> Status in OpenStack Compute (nova) train series:
> New
> Status in OpenStack Compute (nova) ussuri series:
> New
> Status in OpenStack Compute (nova) victoria series:
> New
> Status in OpenStack Compute (nova) wallaby series:
> New
> Status in OpenStack Security Advisory:
> Incomplete
>
> Bug description:
> This bug report is related to Security.
>
> Currently novnc is allowing open direction, which could potentially be
> used for phishing attempts
>
> To test.
> https://<sites' vnc domain>//example.com/%2F..
> include .. at the end
>
> For example:
> http://vncproxy.my.domain.com//example.com/%2F..
>
> It will redirect to example.com. You can replace example.com with some
> legitimate domain or spoofed domain.
>
> The description of the risk is
> By modifying untrusted URL input to a malicious site, an attacker may
> successfully launch a phishing scam and steal user credentials.
> Because the server name in the modified link is identical to the
> original site, phishing attempts may have a more trustworthy appearance.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/nova/+bug/1927677/+subscriptions
>
--
Sent from Gmail Mobile. Please excuse random autocorrects and "tpyos"

Revision history for this message
Swe W Aung (sirswa) wrote :

Hi Melanie

Thanks for the investigation and the effort. Really appreciate that.

Regards
sw3

On Sat, 15 May 2021 at 7:10 am, Swe Win Aung <email address hidden> wrote:

> Hi Melanie
>
> The is for the investigation and your effort. Really appreciate that.
>
> Regards
> sw3
>
> On Sat, 15 May 2021 at 1:06 am, melanie witt <email address hidden>
> wrote:
>
>> ** Also affects: nova/train
>> Importance: Undecided
>> Status: New
>>
>> ** Also affects: nova/ussuri
>> Importance: Undecided
>> Status: New
>>
>> ** Also affects: nova/wallaby
>> Importance: Undecided
>> Status: New
>>
>> ** Also affects: nova/victoria
>> Importance: Undecided
>> Status: New
>>
>> --
>> You received this bug notification because you are subscribed to the bug
>> report.
>> https://bugs.launchpad.net/bugs/1927677
>>
>> Title:
>> novnc allowing open direction which could potentially be used for
>> phishing
>>
>> Status in OpenStack Compute (nova):
>> In Progress
>> Status in OpenStack Compute (nova) train series:
>> New
>> Status in OpenStack Compute (nova) ussuri series:
>> New
>> Status in OpenStack Compute (nova) victoria series:
>> New
>> Status in OpenStack Compute (nova) wallaby series:
>> New
>> Status in OpenStack Security Advisory:
>> Incomplete
>>
>> Bug description:
>> This bug report is related to Security.
>>
>> Currently novnc is allowing open direction, which could potentially be
>> used for phishing attempts
>>
>> To test.
>> https://<sites' vnc domain>//example.com/%2F..
>> include .. at the end
>>
>> For example:
>> http://vncproxy.my.domain.com//example.com/%2F..
>>
>> It will redirect to example.com. You can replace example.com with some
>> legitimate domain or spoofed domain.
>>
>> The description of the risk is
>> By modifying untrusted URL input to a malicious site, an attacker may
>> successfully launch a phishing scam and steal user credentials.
>> Because the server name in the modified link is identical to the
>> original site, phishing attempts may have a more trustworthy appearance.
>>
>> To manage notifications about this bug go to:
>> https://bugs.launchpad.net/nova/+bug/1927677/+subscriptions
>>
> --
> Sent from Gmail Mobile. Please excuse random autocorrects and "tpyos"
>
--
Sent from Gmail Mobile. Please excuse random autocorrects and "tpyos"

Revision history for this message
melanie witt (melwitt) wrote : Re: novnc allowing open direction which could potentially be used for phishing

Hi sw3,

Thanks for reporting the issue!

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

Reviewed: https://review.opendev.org/c/openstack/nova/+/791297
Committed: https://opendev.org/openstack/nova/commit/781612b33282ed298f742c85dab58a075c8b793e
Submitter: "Zuul (22348)"
Branch: master

commit 781612b33282ed298f742c85dab58a075c8b793e
Author: melanie witt <email address hidden>
Date: Thu May 13 05:43:42 2021 +0000

    Reject open redirection in the console proxy

    Our console proxies (novnc, serial, spice) run in a websockify server
    whose request handler inherits from the python standard
    SimpleHTTPRequestHandler. There is a known issue [1] in the
    SimpleHTTPRequestHandler which allows open redirects by way of URLs
    in the following format:

      http://vncproxy.my.domain.com//example.com/%2F..

    which if visited, will redirect a user to example.com.

    We can intercept a request and reject requests that pass a redirection
    URL beginning with "//" by implementing the
    SimpleHTTPRequestHandler.send_head() method containing the
    vulnerability to reject such requests with a 400 Bad Request.

    This code is copied from a patch suggested in one of the issue comments
    [2].

    Closes-Bug: #1927677

    [1] https://bugs.python.org/issue32084
    [2] https://bugs.python.org/issue32084#msg306545

    Change-Id: Ie36401c782f023d1d5f2623732619105dc2cfa24

Changed in nova:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (stable/wallaby)

Fix proposed to branch: stable/wallaby
Review: https://review.opendev.org/c/openstack/nova/+/791577

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

Fix proposed to branch: stable/victoria
Review: https://review.opendev.org/c/openstack/nova/+/791805

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

Fix proposed to branch: stable/ussuri
Review: https://review.opendev.org/c/openstack/nova/+/791806

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

Fix proposed to branch: stable/train
Review: https://review.opendev.org/c/openstack/nova/+/791807

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

Reviewed: https://review.opendev.org/c/openstack/nova/+/791577
Committed: https://opendev.org/openstack/nova/commit/470925614223c8dd9b1233f54f5a96c02b2d4f70
Submitter: "Zuul (22348)"
Branch: stable/wallaby

commit 470925614223c8dd9b1233f54f5a96c02b2d4f70
Author: melanie witt <email address hidden>
Date: Thu May 13 05:43:42 2021 +0000

    Reject open redirection in the console proxy

    Our console proxies (novnc, serial, spice) run in a websockify server
    whose request handler inherits from the python standard
    SimpleHTTPRequestHandler. There is a known issue [1] in the
    SimpleHTTPRequestHandler which allows open redirects by way of URLs
    in the following format:

      http://vncproxy.my.domain.com//example.com/%2F..

    which if visited, will redirect a user to example.com.

    We can intercept a request and reject requests that pass a redirection
    URL beginning with "//" by implementing the
    SimpleHTTPRequestHandler.send_head() method containing the
    vulnerability to reject such requests with a 400 Bad Request.

    This code is copied from a patch suggested in one of the issue comments
    [2].

    Closes-Bug: #1927677

    [1] https://bugs.python.org/issue32084
    [2] https://bugs.python.org/issue32084#msg306545

    Change-Id: Ie36401c782f023d1d5f2623732619105dc2cfa24
    (cherry picked from commit 781612b33282ed298f742c85dab58a075c8b793e)

Revision history for this message
Jeremy Stanley (fungi) wrote : Re: novnc allowing open direction which could potentially be used for phishing

Since we have fixes for this merged to all maintained stable branches (and them some), we need to decide whether we're issuing a security advisory about this or merely treating it as hardening. Does anyone have an opinion?

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

The fix backports have been proposed but are not yet merged on stable/victoria, stable/ussuri, and stable/train.

This issue is considered to be of "moderate" severity and is a result of a publicly reported behavior in the python standard library [1] and the http.server documentation has a warning on it that states, "Warning http.server is not recommended for production. It only implements basic security checks." [2].

Our dependency however, the websockify server [3], is based upon http.server and AFAIK websockify isn't characterized as only for dev or non-production use.

There is currently a pull request proposed to fix [1] and it is currently under review [4].

Based on this, I tend to think to treat it as hardening, but of course I defer to the expert opinion of the VMT.

[1] https://bugs.python.org/issue43223
[2] https://docs.python.org/3/library/http.server.html
[3] https://github.com/novnc/websockify
[4] https://github.com/python/cpython/pull/24848

Revision history for this message
Joshua Padman (jpadman) wrote :

The last OSSA that was released was also for an open redirect.
https://security.openstack.org/ossa/OSSA-2020-008.html

Its not a fancy vulnerability but it is one that can lead to far more significant issues. Ideally a CVE would be assigned to this, though maybe in the interest of consistency an OSSA may be required too?

Revision history for this message
Jeremy Stanley (fungi) wrote :

Melanie: Thanks for pointing out that the backports for branches prior to stable/wallaby haven't merged yet; I misread the notifications. We can't publish an advisory anyway until those are merged for the maintained branches at a minimum (fixes for branches under extended maintenance can merge after any advisory though).

Joshua: As Melanie points out, what we're providing is a workaround for a known security flaw in a dependency, so differs slightly from OSSA-2020-008 in that regard. You are right though that the end result is roughly the same for vulnerable deployments, and unlike most vulnerabilities involving dependencies this is one we actually have an active mitigation for which doesn't require any additional action on the part of the deployer other than upgrading our software in the deployment (no additional configuration or dependency upgrade steps needed).

I'm disappearing on a week-long vacation tomorrow, but if someone wants to compose an impact description for this and request a CVE from MITRE with it (or assign one as a CNA), I'm happy to help coordinate any advisory when I get home. Our template for impact descriptions can be found here: https://security.openstack.org/vmt-process.html#impact-description-description

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

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

Revision history for this message
Nick Tait (nickthetait) wrote : Re: novnc allowing open direction which could potentially be used for phishing

Red Hat will be assigning a CVE for this. Sirswa, would you like to be credited as the reporter? What name should we put down? Do you represent an organization?

Revision history for this message
Shahaan Ayyub (shahaan) wrote : Re: [Bug 1927677] Re: novnc allowing open direction which could potentially be used for phishing

Thanks Nick,
  We are part of the Monash University Nectar Cloud team. Please feel free
to put us reporters.

Swe Aung
Shahaan Ayyub

Regards,
Shahaan

On Sat, 24 Jul 2021 at 09:25, Nick Tait <email address hidden> wrote:

> Red Hat will be assigning a CVE for this. Sirswa, would you like to be
> credited as the reporter? What name should we put down? Do you represent
> an organization?
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1927677
>
> Title:
> novnc allowing open direction which could potentially be used for
> phishing
>
> Status in OpenStack Compute (nova):
> Fix Released
> Status in OpenStack Compute (nova) train series:
> In Progress
> Status in OpenStack Compute (nova) ussuri series:
> In Progress
> Status in OpenStack Compute (nova) victoria series:
> In Progress
> Status in OpenStack Compute (nova) wallaby series:
> Fix Released
> Status in OpenStack Security Advisory:
> Incomplete
>
> Bug description:
> This bug report is related to Security.
>
> Currently novnc is allowing open direction, which could potentially be
> used for phishing attempts
>
> To test.
> https://<sites' vnc domain>//example.com/%2F..
> include .. at the end
>
> For example:
> http://vncproxy.my.domain.com//example.com/%2F..
>
> It will redirect to example.com. You can replace example.com with some
> legitimate domain or spoofed domain.
>
> The description of the risk is
> By modifying untrusted URL input to a malicious site, an attacker may
> successfully launch a phishing scam and steal user credentials.
> Because the server name in the modified link is identical to the
> original site, phishing attempts may have a more trustworthy appearance.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/nova/+bug/1927677/+subscriptions
>
>

Revision history for this message
Jeremy Stanley (fungi) wrote : Re: novnc allowing open direction which could potentially be used for phishing

Nick: Thanks for offering to assign a CVE for this. Do you have text for an impact description you'll be using in the CVE? If not, I can work on putting one together, though we still don't have any ETA on when the workarounds will merge to stable/victoria and earlier branches (and wouldn't issue a formal advisory until they do).

Revision history for this message
Swe W Aung (sirswa) wrote :

Hi Nick

Thanks. I noticed the CVE at RedHat few days ago. Thank you for following up and assigning at RedHat.

I will have to acknowledge Monash University Cyber Security team's vulnerability discovery work and raising the issue to us originally. I work for Monash University.

Our names are
Swe Aung
Shahaan Ayyub
Salman Khan (Cyber Security)

Thank you.

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

Reviewed: https://review.opendev.org/c/openstack/nova/+/791805
Committed: https://opendev.org/openstack/nova/commit/6b70350bdcf59a9712f88b6435ba2c6500133e5b
Submitter: "Zuul (22348)"
Branch: stable/victoria

commit 6b70350bdcf59a9712f88b6435ba2c6500133e5b
Author: melanie witt <email address hidden>
Date: Thu May 13 05:43:42 2021 +0000

    Reject open redirection in the console proxy

    Our console proxies (novnc, serial, spice) run in a websockify server
    whose request handler inherits from the python standard
    SimpleHTTPRequestHandler. There is a known issue [1] in the
    SimpleHTTPRequestHandler which allows open redirects by way of URLs
    in the following format:

      http://vncproxy.my.domain.com//example.com/%2F..

    which if visited, will redirect a user to example.com.

    We can intercept a request and reject requests that pass a redirection
    URL beginning with "//" by implementing the
    SimpleHTTPRequestHandler.send_head() method containing the
    vulnerability to reject such requests with a 400 Bad Request.

    This code is copied from a patch suggested in one of the issue comments
    [2].

    Closes-Bug: #1927677

    [1] https://bugs.python.org/issue32084
    [2] https://bugs.python.org/issue32084#msg306545

    Conflicts:
        nova/console/websocketproxy.py
        nova/tests/unit/console/test_websocketproxy.py

    NOTE(melwitt): The conflicts are because the following changes are not
    in Victoria:

      Ib2c406327fef2fb4868d8050fc476a7d17706e23 (Remove six.moves)
      I58b0382c86d4ef798572edb63d311e0e3e6937bb (Refactor and rename
        test_tcp_rst_no_compute_rpcapi)

    Change-Id: Ie36401c782f023d1d5f2623732619105dc2cfa24
    (cherry picked from commit 781612b33282ed298f742c85dab58a075c8b793e)
    (cherry picked from commit 470925614223c8dd9b1233f54f5a96c02b2d4f70)

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

Reviewed: https://review.opendev.org/c/openstack/nova/+/791806
Committed: https://opendev.org/openstack/nova/commit/719e651e6be277950632e0c2cf5cc9a018344e7b
Submitter: "Zuul (22348)"
Branch: stable/ussuri

commit 719e651e6be277950632e0c2cf5cc9a018344e7b
Author: melanie witt <email address hidden>
Date: Thu May 13 05:43:42 2021 +0000

    Reject open redirection in the console proxy

    Our console proxies (novnc, serial, spice) run in a websockify server
    whose request handler inherits from the python standard
    SimpleHTTPRequestHandler. There is a known issue [1] in the
    SimpleHTTPRequestHandler which allows open redirects by way of URLs
    in the following format:

      http://vncproxy.my.domain.com//example.com/%2F..

    which if visited, will redirect a user to example.com.

    We can intercept a request and reject requests that pass a redirection
    URL beginning with "//" by implementing the
    SimpleHTTPRequestHandler.send_head() method containing the
    vulnerability to reject such requests with a 400 Bad Request.

    This code is copied from a patch suggested in one of the issue comments
    [2].

    Closes-Bug: #1927677

    [1] https://bugs.python.org/issue32084
    [2] https://bugs.python.org/issue32084#msg306545

    Change-Id: Ie36401c782f023d1d5f2623732619105dc2cfa24
    (cherry picked from commit 781612b33282ed298f742c85dab58a075c8b793e)
    (cherry picked from commit 470925614223c8dd9b1233f54f5a96c02b2d4f70)
    (cherry picked from commit 6b70350bdcf59a9712f88b6435ba2c6500133e5b)

Revision history for this message
Jeremy Stanley (fungi) wrote : Re: novnc allowing open direction which could potentially be used for phishing

Actually, since the stable/train branch has transitioned from maintained to extended maintenance status we no longer need to hold off any advisory waiting for the fix there to merge. I'll go ahead and start drafting the advisory now, since the stable/ussuri fix has merged.

Changed in ossa:
status: Incomplete → In Progress
importance: Undecided → Medium
assignee: nobody → Jeremy Stanley (fungi)
Revision history for this message
Jeremy Stanley (fungi) wrote :

Nick: What CVE number have you assigned for this? Or should I go ahead and ask MITRE to assign one?

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

I'm working on getting reviews for the backport patches, only the stable/train patch remains.

(update) Oops, I just now see Jeremy's comment #30. Makes sense.

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

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/ossa/+/802590

Revision history for this message
Jeremy Stanley (fungi) wrote : Re: novnc allowing open direction which could potentially be used for phishing

Can folks please review the impact description and related metadata in the draft OSSA-2021-002 linked above? I've left the CVE blank for now since I'm waiting for Nick to say what number he assigned.

Revision history for this message
Nick Tait (nickthetait) wrote :
Revision history for this message
Nick Tait (nickthetait) wrote :

And cheers to the trio at Monash for reporting. Thanks!

Revision history for this message
Jeremy Stanley (fungi) wrote : Re: Open Redirect in noVNC proxy (CVE-2021-3654)

The draft advisory has been updated with the CVE (thanks again, Nick!) and a proposed publication date of tomorrow, so if folks could please review https://review.opendev.org/802590 that will help a ton.

summary: - novnc allowing open direction which could potentially be used for
- phishing
+ Open Redirect in noVNC proxy (CVE-2021-3654)
Changed in ossa:
status: In Progress → Fix Committed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to ossa (master)

Reviewed: https://review.opendev.org/c/openstack/ossa/+/802590
Committed: https://opendev.org/openstack/ossa/commit/08f2c78ccf3688ad2ed44d0c2239742ea1693cdb
Submitter: "Zuul (22348)"
Branch: master

commit 08f2c78ccf3688ad2ed44d0c2239742ea1693cdb
Author: Jeremy Stanley <email address hidden>
Date: Tue Jul 27 17:44:41 2021 +0000

    Add OSSA-2021-002 (CVE-2021-3654)

    Change-Id: I1574738a9aa047314c9b933f8bbe032d346cd2d7
    Closes-Bug: #1927677

Changed in ossa:
status: Fix Committed → Fix Released
Jeremy Stanley (fungi)
summary: - Open Redirect in noVNC proxy (CVE-2021-3654)
+ [OSSA-2021-002]Open Redirect in noVNC proxy (CVE-2021-3654)
summary: - [OSSA-2021-002]Open Redirect in noVNC proxy (CVE-2021-3654)
+ [OSSA-2021-002] Open Redirect in noVNC proxy (CVE-2021-3654)
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/c/openstack/nova/+/802935

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/c/openstack/nova/+/803091

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

This ^ is just me updating the unit test coverage to work with Python < 3.6 for the courtesy stable/train and stable/stein patches. It's also a general improvement of the unit test to do less mocking.

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

Related fix proposed to branch: stable/wallaby
Review: https://review.opendev.org/c/openstack/nova/+/803092

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

Related fix proposed to branch: stable/victoria
Review: https://review.opendev.org/c/openstack/nova/+/803093

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

Related fix proposed to branch: stable/ussuri
Review: https://review.opendev.org/c/openstack/nova/+/803094

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/c/openstack/nova/+/803182

Revision history for this message
Matteo Pozza (matteopozza) wrote :

Hi,

me and my colleagues were noticing that the patch protects against URLs starting with two slashes, with four slashes, and with more than four slashes. Nevertheless, the patch does not seem to work in case the URL starts with three slashes, e.g.:

http://vncproxy.my.domain.com///example.com/%2F..

We believe that the reason behind this behaviour is the changes that the URL undergoes when it is split and then unsplit. More specifically:

>>> urlparse.urlunsplit(urlparse.urlsplit('//example.com/%2F..'))
'//example.com/%2F..'
>>> urlparse.urlunsplit(urlparse.urlsplit('///example.com/%2F..'))
'/example.com/%2F..'
>>> urlparse.urlunsplit(urlparse.urlsplit('////example.com/%2F..'))
'//example.com/%2F..'

The first and the last case will be captured by the test introduced by the patch, while the middle case will be sent to the parent class method. This can be observed with both Python 3 and Python 2.

We think that the unsplitting of the url in the patch could be avoided. In the case of Python 3, the reason for which the method is used is to add a final slash to the "path" part of the URL, but this is nevertheless not needed for our purposes (if the URL passes the validity check performed here, the splitting and unsplitting of the URL will be anyway done by the parent class method). Moreover, using the output of the urlsplit + urlunsplit methods causes the aforementioned issue.

I will soon attach a proposal for a Python 2 and a Python 3 patch. Their checks on the URL are slightly different because they mimic the checks that are done by the parent class method in order to identify a redirection case (SimpleHTTPRequestHandler.send_head defined in SimpleHttpServer.py for Python2 and in http/server.py for Python 3). Please let me know if you are able to reproduce the same issue in your setup and what you think about this.

Revision history for this message
Matteo Pozza (matteopozza) wrote :

This is a proposal for Python 2.

Revision history for this message
Matteo Pozza (matteopozza) wrote :

This is a proposal for Python 3.

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

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/nova/+/805654

Revision history for this message
Balazs Gibizer (balazs-gibizer) wrote :

It is confirmed that the original fix was incomplete. A new fix is being merged to master https://review.opendev.org/c/openstack/nova/+/805654 (and then backported)

Changed in nova:
status: Fix Released → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (master)

Reviewed: https://review.opendev.org/c/openstack/nova/+/805654
Committed: https://opendev.org/openstack/nova/commit/6fbd0b758dcac71323f3be179b1a9d1c17a4acc5
Submitter: "Zuul (22348)"
Branch: master

commit 6fbd0b758dcac71323f3be179b1a9d1c17a4acc5
Author: Sean Mooney <email address hidden>
Date: Mon Aug 23 15:37:48 2021 +0100

    address open redirect with 3 forward slashes

    Ie36401c782f023d1d5f2623732619105dc2cfa24 was intended
    to address OSSA-2021-002 (CVE-2021-3654) however after its
    release it was discovered that the fix only worked
    for urls with 2 leading slashes or more then 4.

    This change adresses the missing edgecase for 3 leading slashes
    and also maintian support for rejecting 2+.

    Change-Id: I95f68be76330ff09e5eabb5ef8dd9a18f5547866
    co-authored-by: Matteo Pozza
    Closes-Bug: #1927677

Changed in nova:
status: In Progress → Fix Released
Revision history for this message
Jeremy Stanley (fungi) wrote :

I've switched the security advisory task back to incomplete for now, while the vulnerability managers debate whether this requires errata publication or a completely new advisory.

Changed in ossa:
status: Fix Released → Incomplete
importance: Medium → Undecided
assignee: Jeremy Stanley (fungi) → nobody
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (stable/wallaby)

Fix proposed to branch: stable/wallaby
Review: https://review.opendev.org/c/openstack/nova/+/805818

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

Reviewed: https://review.opendev.org/c/openstack/nova/+/805818
Committed: https://opendev.org/openstack/nova/commit/47dad4836a26292e9d34e516e1525ecf00be127c
Submitter: "Zuul (22348)"
Branch: stable/wallaby

commit 47dad4836a26292e9d34e516e1525ecf00be127c
Author: Sean Mooney <email address hidden>
Date: Mon Aug 23 15:37:48 2021 +0100

    address open redirect with 3 forward slashes

    Ie36401c782f023d1d5f2623732619105dc2cfa24 was intended
    to address OSSA-2021-002 (CVE-2021-3654) however after its
    release it was discovered that the fix only worked
    for urls with 2 leading slashes or more then 4.

    This change adresses the missing edgecase for 3 leading slashes
    and also maintian support for rejecting 2+.

    Change-Id: I95f68be76330ff09e5eabb5ef8dd9a18f5547866
    co-authored-by: Matteo Pozza
    Closes-Bug: #1927677
    (cherry picked from commit 6fbd0b758dcac71323f3be179b1a9d1c17a4acc5)

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

Reviewed: https://review.opendev.org/c/openstack/nova/+/803091
Committed: https://opendev.org/openstack/nova/commit/214cabe6848a1fdb4f5941d994c6cc11107fc4af
Submitter: "Zuul (22348)"
Branch: master

commit 214cabe6848a1fdb4f5941d994c6cc11107fc4af
Author: melanie witt <email address hidden>
Date: Sat Jul 31 00:30:16 2021 +0000

    Reduce mocking in test_reject_open_redirect for compat

    This is a followup for change Ie36401c782f023d1d5f2623732619105dc2cfa24
    to reduce mocking in the unit test coverage for it.

    While backporting the bug fix, it was found to be incompatible with
    earlier versions of Python < 3.6 due to a difference in internal
    implementation [1].

    This reduces the mocking in the unit test to be more agnostic to the
    internals of the StreamRequestHandler (ancestor of
    SimpleHTTPRequestHandler) and work across Python versions >= 2.7.

    Related-Bug: #1927677

    [1] https://github.com/python/cpython/commit/34eeed42901666fce099947f93dfdfc05411f286

    Change-Id: I546d376869a992601b443fb95acf1034da2a8f36

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

Fix proposed to branch: stable/victoria
Review: https://review.opendev.org/c/openstack/nova/+/806626

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

Fix proposed to branch: stable/ussuri
Review: https://review.opendev.org/c/openstack/nova/+/806628

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

Fix proposed to branch: stable/train
Review: https://review.opendev.org/c/openstack/nova/+/806629

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

Reviewed: https://review.opendev.org/c/openstack/nova/+/803092
Committed: https://opendev.org/openstack/nova/commit/9c2f29783734cb5f9cb05a08d328c10e1d16c4f1
Submitter: "Zuul (22348)"
Branch: stable/wallaby

commit 9c2f29783734cb5f9cb05a08d328c10e1d16c4f1
Author: melanie witt <email address hidden>
Date: Sat Jul 31 00:30:16 2021 +0000

    Reduce mocking in test_reject_open_redirect for compat

    This is a followup for change Ie36401c782f023d1d5f2623732619105dc2cfa24
    to reduce mocking in the unit test coverage for it.

    While backporting the bug fix, it was found to be incompatible with
    earlier versions of Python < 3.6 due to a difference in internal
    implementation [1].

    This reduces the mocking in the unit test to be more agnostic to the
    internals of the StreamRequestHandler (ancestor of
    SimpleHTTPRequestHandler) and work across Python versions >= 2.7.

    Related-Bug: #1927677

    [1] https://github.com/python/cpython/commit/34eeed42901666fce099947f93dfdfc05411f286

    Change-Id: I546d376869a992601b443fb95acf1034da2a8f36
    (cherry picked from commit 214cabe6848a1fdb4f5941d994c6cc11107fc4af)

tags: added: in-stable-wallaby
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/victoria)

Reviewed: https://review.opendev.org/c/openstack/nova/+/803093
Committed: https://opendev.org/openstack/nova/commit/94e265f3ca615aa18de0081a76975019997b8709
Submitter: "Zuul (22348)"
Branch: stable/victoria

commit 94e265f3ca615aa18de0081a76975019997b8709
Author: melanie witt <email address hidden>
Date: Sat Jul 31 00:30:16 2021 +0000

    Reduce mocking in test_reject_open_redirect for compat

    This is a followup for change Ie36401c782f023d1d5f2623732619105dc2cfa24
    to reduce mocking in the unit test coverage for it.

    While backporting the bug fix, it was found to be incompatible with
    earlier versions of Python < 3.6 due to a difference in internal
    implementation [1].

    This reduces the mocking in the unit test to be more agnostic to the
    internals of the StreamRequestHandler (ancestor of
    SimpleHTTPRequestHandler) and work across Python versions >= 2.7.

    Related-Bug: #1927677

    [1] https://github.com/python/cpython/commit/34eeed42901666fce099947f93dfdfc05411f286

    Change-Id: I546d376869a992601b443fb95acf1034da2a8f36
    (cherry picked from commit 214cabe6848a1fdb4f5941d994c6cc11107fc4af)
    (cherry picked from commit 9c2f29783734cb5f9cb05a08d328c10e1d16c4f1)

tags: added: in-stable-victoria
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/ussuri)

Reviewed: https://review.opendev.org/c/openstack/nova/+/803094
Committed: https://opendev.org/openstack/nova/commit/d43b88a33407b1253e7bce70f720a44f7688141f
Submitter: "Zuul (22348)"
Branch: stable/ussuri

commit d43b88a33407b1253e7bce70f720a44f7688141f
Author: melanie witt <email address hidden>
Date: Sat Jul 31 00:30:16 2021 +0000

    Reduce mocking in test_reject_open_redirect for compat

    This is a followup for change Ie36401c782f023d1d5f2623732619105dc2cfa24
    to reduce mocking in the unit test coverage for it.

    While backporting the bug fix, it was found to be incompatible with
    earlier versions of Python < 3.6 due to a difference in internal
    implementation [1].

    This reduces the mocking in the unit test to be more agnostic to the
    internals of the StreamRequestHandler (ancestor of
    SimpleHTTPRequestHandler) and work across Python versions >= 2.7.

    Related-Bug: #1927677

    [1] https://github.com/python/cpython/commit/34eeed42901666fce099947f93dfdfc05411f286

    Change-Id: I546d376869a992601b443fb95acf1034da2a8f36
    (cherry picked from commit 214cabe6848a1fdb4f5941d994c6cc11107fc4af)
    (cherry picked from commit 9c2f29783734cb5f9cb05a08d328c10e1d16c4f1)
    (cherry picked from commit 94e265f3ca615aa18de0081a76975019997b8709)

tags: added: in-stable-ussuri
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (stable/victoria)

Reviewed: https://review.opendev.org/c/openstack/nova/+/806626
Committed: https://opendev.org/openstack/nova/commit/9588cdbfd4649ea53d60303f2d10c5d62a070a07
Submitter: "Zuul (22348)"
Branch: stable/victoria

commit 9588cdbfd4649ea53d60303f2d10c5d62a070a07
Author: Sean Mooney <email address hidden>
Date: Mon Aug 23 15:37:48 2021 +0100

    address open redirect with 3 forward slashes

    Ie36401c782f023d1d5f2623732619105dc2cfa24 was intended
    to address OSSA-2021-002 (CVE-2021-3654) however after its
    release it was discovered that the fix only worked
    for urls with 2 leading slashes or more then 4.

    This change adresses the missing edgecase for 3 leading slashes
    and also maintian support for rejecting 2+.

    Conflicts:
      nova/tests/unit/console/test_websocketproxy.py

    NOTE: conflict is due to I58b0382c86d4ef798572edb63d311e0e3e6937bb
    is missing in Victoria and Ie36401c782f023d1d5f2623732619105dc2cfa24
    backport contained conflicts and methods order was swapped.

    Change-Id: I95f68be76330ff09e5eabb5ef8dd9a18f5547866
    co-authored-by: Matteo Pozza
    Closes-Bug: #1927677
    (cherry picked from commit 6fbd0b758dcac71323f3be179b1a9d1c17a4acc5)
    (cherry picked from commit 47dad4836a26292e9d34e516e1525ecf00be127c)

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

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

Revision history for this message
Jeremy Stanley (fungi) wrote :

Note that we're still waiting for https://review.opendev.org/806628 (stable/ussuri) to merge prior to issuing errata, but its merging is held up by discussions of whether to fix or discontinue running lower bounds testing for dependencies on that branch.

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

We have actually been rechecking this change to make the l-c job non-voting on stable/ussuri:

https://review.opendev.org/c/openstack/nova/+/809955

But it has run into failure after unrelated failure in the gate. I have been copy pasting the various errors with every recheck.

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

Reviewed: https://review.opendev.org/c/openstack/nova/+/806628
Committed: https://opendev.org/openstack/nova/commit/0997043f459ac616b594363b5b253bd0ae6ed9eb
Submitter: "Zuul (22348)"
Branch: stable/ussuri

commit 0997043f459ac616b594363b5b253bd0ae6ed9eb
Author: Sean Mooney <email address hidden>
Date: Mon Aug 23 15:37:48 2021 +0100

    address open redirect with 3 forward slashes

    Ie36401c782f023d1d5f2623732619105dc2cfa24 was intended
    to address OSSA-2021-002 (CVE-2021-3654) however after its
    release it was discovered that the fix only worked
    for urls with 2 leading slashes or more then 4.

    This change adresses the missing edgecase for 3 leading slashes
    and also maintian support for rejecting 2+.

    Change-Id: I95f68be76330ff09e5eabb5ef8dd9a18f5547866
    co-authored-by: Matteo Pozza
    Closes-Bug: #1927677
    (cherry picked from commit 6fbd0b758dcac71323f3be179b1a9d1c17a4acc5)
    (cherry picked from commit 47dad4836a26292e9d34e516e1525ecf00be127c)
    (cherry picked from commit 9588cdbfd4649ea53d60303f2d10c5d62a070a07)

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

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/ossa/+/811181

Changed in ossa:
status: Incomplete → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to ossa (master)

Reviewed: https://review.opendev.org/c/openstack/ossa/+/811181
Committed: https://opendev.org/openstack/ossa/commit/51a1bf0699128c8ddcb7567347cee69492601091
Submitter: "Zuul (22348)"
Branch: master

commit 51a1bf0699128c8ddcb7567347cee69492601091
Author: Jeremy Stanley <email address hidden>
Date: Mon Sep 27 15:02:06 2021 +0000

    Errata 1 for OSSA-2021-002

    Change-Id: Iaeb40574176ae62542a0c17e94917e654d38317d
    Closes-Bug: #1927677

Changed in ossa:
status: In Progress → Fix Released
Revision history for this message
Jeremy Stanley (fungi) wrote :

OSSA-2021-002 errata 1 has been published and distributed to usual mailing lists.

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

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

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

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

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

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (stable/train)
Download full text (3.2 KiB)

Reviewed: https://review.opendev.org/c/openstack/nova/+/791807
Committed: https://opendev.org/openstack/nova/commit/04d48527b62a35d912f93bc75613a6cca606df66
Submitter: "Zuul (22348)"
Branch: stable/train

commit 04d48527b62a35d912f93bc75613a6cca606df66
Author: melanie witt <email address hidden>
Date: Thu May 13 05:43:42 2021 +0000

    Reject open redirection in the console proxy

    NOTE(melwitt): This is the combination of two commits, the bug fix and
    a followup change to the unit test to enable it also run on
    Python < 3.6.

    Our console proxies (novnc, serial, spice) run in a websockify server
    whose request handler inherits from the python standard
    SimpleHTTPRequestHandler. There is a known issue [1] in the
    SimpleHTTPRequestHandler which allows open redirects by way of URLs
    in the following format:

      http://vncproxy.my.domain.com//example.com/%2F..

    which if visited, will redirect a user to example.com.

    We can intercept a request and reject requests that pass a redirection
    URL beginning with "//" by implementing the
    SimpleHTTPRequestHandler.send_head() method containing the
    vulnerability to reject such requests with a 400 Bad Request.

    This code is copied from a patch suggested in one of the issue comments
    [2].

    Closes-Bug: #1927677

    [1] https://bugs.python.org/issue32084
    [2] https://bugs.python.org/issue32084#msg306545

    Conflicts:
        nova/tests/unit/console/test_websocketproxy.py

    NOTE(melwitt): The conflict is because change
    I23ac1cc79482d0fabb359486a4b934463854cae5 (Allow TLS ciphers/protocols
    to be configurable for console proxies) is not in Train.

    NOTE(melwitt): The difference from the cherry picked change:
    HTTPStatus.BAD_REQUEST => 400 is due to the fact that HTTPStatus does
    not exist in Python 2.7.

    Reduce mocking in test_reject_open_redirect for compat

    This is a followup for change Ie36401c782f023d1d5f2623732619105dc2cfa24
    to reduce mocking in the unit test coverage for it.

    While backporting the bug fix, it was found to be incompatible with
    earlier versions of Python < 3.6 due to a difference in internal
    implementation [1].

    This reduces the mocking in the unit test to be more agnostic to the
    internals of the StreamRequestHandler (ancestor of
    SimpleHTTPRequestHandler) and work across Python versions >= 2.7.

    Related-Bug: #1927677

    [1] https://github.com/python/cpython/commit/34eeed42901666fce099947f93dfdfc05411f286

    Change-Id: I546d376869a992601b443fb95acf1034da2a8f36
    (cherry picked from commit 214cabe6848a1fdb4f5941d994c6cc11107fc4af)
    (cherry picked from commit 9c2f29783734cb5f9cb05a08d328c10e1d16c4f1)
    (cherry picked from commit 94e265f3ca615aa18de0081a76975019997b8709)
    (cherry picked from commit d43b88a33407b1253e7bce70f720a44f7688141f)

    Change-Id: Ie36401c782f023d1d5f2623732619105dc2cfa24
    (cherry picked from commit 781612b33282ed298f742c85dab58a075c8b793e)
    (cherry picked from commit 470925614223c8dd9b1233f54f5a96c02b2d4f70)
    (cherry picked from commit 6b70350bdcf5...

Read more...

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.opendev.org/c/openstack/nova/+/806629
Committed: https://opendev.org/openstack/nova/commit/8906552cfc2525a44251d4cf313ece61e57251eb
Submitter: "Zuul (22348)"
Branch: stable/train

commit 8906552cfc2525a44251d4cf313ece61e57251eb
Author: Sean Mooney <email address hidden>
Date: Mon Aug 23 15:37:48 2021 +0100

    address open redirect with 3 forward slashes

    Ie36401c782f023d1d5f2623732619105dc2cfa24 was intended
    to address OSSA-2021-002 (CVE-2021-3654) however after its
    release it was discovered that the fix only worked
    for urls with 2 leading slashes or more then 4.

    This change adresses the missing edgecase for 3 leading slashes
    and also maintian support for rejecting 2+.

    Conflicts:
      nova/console/websocketproxy.py
      nova/tests/unit/console/test_websocketproxy.py

    NOTE(melwitt): The conflict and difference in websocketproxy.py from
    the cherry picked change: HTTPStatus.BAD_REQUEST => 400 is due to the
    fact that HTTPStatus does not exist in Python 2.7. The conflict in
    test_websocketproxy.py is because change
    I23ac1cc79482d0fabb359486a4b934463854cae5 (Allow TLS ciphers/protocols
    to be configurable for console proxies) is not in Train. The difference
    in test_websocketproxy.py from the cherry picked change is due to a
    difference in internal implementation [1] in Python < 3.6. See change
    I546d376869a992601b443fb95acf1034da2a8f36 for reference.

    [1] https://github.com/python/cpython/commit/34eeed42901666fce099947f93dfdfc05411f286

    Change-Id: I95f68be76330ff09e5eabb5ef8dd9a18f5547866
    co-authored-by: Matteo Pozza
    Closes-Bug: #1927677
    (cherry picked from commit 6fbd0b758dcac71323f3be179b1a9d1c17a4acc5)
    (cherry picked from commit 47dad4836a26292e9d34e516e1525ecf00be127c)
    (cherry picked from commit 9588cdbfd4649ea53d60303f2d10c5d62a070a07)
    (cherry picked from commit 0997043f459ac616b594363b5b253bd0ae6ed9eb)

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/c/openstack/nova/+/817037

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/c/openstack/nova/+/853379

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

Reviewed: https://review.opendev.org/c/openstack/nova/+/853379
Committed: https://opendev.org/openstack/nova/commit/15769b883ed4a86d62b141ea30d3f1590565d8e0
Submitter: "Zuul (22348)"
Branch: master

commit 15769b883ed4a86d62b141ea30d3f1590565d8e0
Author: melanie witt <email address hidden>
Date: Tue Aug 16 06:49:53 2022 +0000

    Adapt websocketproxy tests for SimpleHTTPServer fix

    In response to bug 1927677 we added a workaround to
    NovaProxyRequestHandler to respond with a 400 Bad Request if an open
    redirect is attempted:

      Ie36401c782f023d1d5f2623732619105dc2cfa24
      I95f68be76330ff09e5eabb5ef8dd9a18f5547866

    Recently in python 3.10.6, a fix has landed in cpython to respond with
    a 301 Moved Permanently to a sanitized URL that has had extra leading
    '/' characters removed.

    This breaks our existing unit tests which assume a 400 Bad Request as
    the only expected response.

    This adds handling of a 301 Moved Permanently response and asserts that
    the redirect location is the expected sanitized URL. Doing this instead
    of checking for a given python version will enable the tests to continue
    to work if and when the cpython fix gets backported to older python
    versions.

    While updating the tests, the opportunity was taken to commonize the
    code of two unit tests that were nearly identical.

    Related-Bug: #1927677
    Closes-Bug: #1986545

    Change-Id: I27441d15cc6fa2ff7715ba15aa900961aadbf54a

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

Change abandoned by "Elod Illes <email address hidden>" on branch: stable/rocky
Review: https://review.opendev.org/c/openstack/nova/+/803182
Reason: This branch transitioned to End of Life for this project, open patches needs to be closed to be able to delete the branch.

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

Change abandoned by "Elod Illes <email address hidden>" on branch: stable/stein
Review: https://review.opendev.org/c/openstack/nova/+/817037
Reason: This branch transitioned to End of Life for this project, open patches needs to be closed to be able to delete the branch.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Change abandoned by "Elod Illes <email address hidden>" on branch: stable/stein
Review: https://review.opendev.org/c/openstack/nova/+/802935
Reason: This branch transitioned to End of Life for this project, open patches needs to be closed to be able to delete the branch.

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

Related fix proposed to branch: stable/yoga
Review: https://review.opendev.org/c/openstack/nova/+/866192

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

Related fix proposed to branch: stable/xena
Review: https://review.opendev.org/c/openstack/nova/+/866193

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

Related fix proposed to branch: stable/wallaby
Review: https://review.opendev.org/c/openstack/nova/+/866194

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

Related fix proposed to branch: stable/victoria
Review: https://review.opendev.org/c/openstack/nova/+/866195

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

Related fix proposed to branch: stable/ussuri
Review: https://review.opendev.org/c/openstack/nova/+/866196

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

Related fix proposed to branch: stable/train
Review: https://review.opendev.org/c/openstack/nova/+/866201

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

Reviewed: https://review.opendev.org/c/openstack/nova/+/866192
Committed: https://opendev.org/openstack/nova/commit/4a2b44c7cf55d1d79d5a2dd638bd0def3af0f5af
Submitter: "Zuul (22348)"
Branch: stable/yoga

commit 4a2b44c7cf55d1d79d5a2dd638bd0def3af0f5af
Author: melanie witt <email address hidden>
Date: Tue Aug 16 06:49:53 2022 +0000

    Adapt websocketproxy tests for SimpleHTTPServer fix

    In response to bug 1927677 we added a workaround to
    NovaProxyRequestHandler to respond with a 400 Bad Request if an open
    redirect is attempted:

      Ie36401c782f023d1d5f2623732619105dc2cfa24
      I95f68be76330ff09e5eabb5ef8dd9a18f5547866

    Recently in python 3.10.6, a fix has landed in cpython to respond with
    a 301 Moved Permanently to a sanitized URL that has had extra leading
    '/' characters removed.

    This breaks our existing unit tests which assume a 400 Bad Request as
    the only expected response.

    This adds handling of a 301 Moved Permanently response and asserts that
    the redirect location is the expected sanitized URL. Doing this instead
    of checking for a given python version will enable the tests to continue
    to work if and when the cpython fix gets backported to older python
    versions.

    While updating the tests, the opportunity was taken to commonize the
    code of two unit tests that were nearly identical.

    Related-Bug: #1927677
    Closes-Bug: #1986545

    Change-Id: I27441d15cc6fa2ff7715ba15aa900961aadbf54a
    (cherry picked from commit 15769b883ed4a86d62b141ea30d3f1590565d8e0)

tags: added: in-stable-yoga
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/xena)

Reviewed: https://review.opendev.org/c/openstack/nova/+/866193
Committed: https://opendev.org/openstack/nova/commit/0e4a257e8636a979605c614a35e79ba47b74d870
Submitter: "Zuul (22348)"
Branch: stable/xena

commit 0e4a257e8636a979605c614a35e79ba47b74d870
Author: melanie witt <email address hidden>
Date: Tue Aug 16 06:49:53 2022 +0000

    Adapt websocketproxy tests for SimpleHTTPServer fix

    In response to bug 1927677 we added a workaround to
    NovaProxyRequestHandler to respond with a 400 Bad Request if an open
    redirect is attempted:

      Ie36401c782f023d1d5f2623732619105dc2cfa24
      I95f68be76330ff09e5eabb5ef8dd9a18f5547866

    Recently in python 3.10.6, a fix has landed in cpython to respond with
    a 301 Moved Permanently to a sanitized URL that has had extra leading
    '/' characters removed.

    This breaks our existing unit tests which assume a 400 Bad Request as
    the only expected response.

    This adds handling of a 301 Moved Permanently response and asserts that
    the redirect location is the expected sanitized URL. Doing this instead
    of checking for a given python version will enable the tests to continue
    to work if and when the cpython fix gets backported to older python
    versions.

    While updating the tests, the opportunity was taken to commonize the
    code of two unit tests that were nearly identical.

    Related-Bug: #1927677
    Closes-Bug: #1986545

    Change-Id: I27441d15cc6fa2ff7715ba15aa900961aadbf54a
    (cherry picked from commit 15769b883ed4a86d62b141ea30d3f1590565d8e0)
    (cherry picked from commit 4a2b44c7cf55d1d79d5a2dd638bd0def3af0f5af)

tags: added: in-stable-xena
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to nova (stable/wallaby)

Reviewed: https://review.opendev.org/c/openstack/nova/+/866194
Committed: https://opendev.org/openstack/nova/commit/3023e162e1a415ddaa70b4b8fbe24b1771dbe424
Submitter: "Zuul (22348)"
Branch: stable/wallaby

commit 3023e162e1a415ddaa70b4b8fbe24b1771dbe424
Author: melanie witt <email address hidden>
Date: Tue Aug 16 06:49:53 2022 +0000

    Adapt websocketproxy tests for SimpleHTTPServer fix

    In response to bug 1927677 we added a workaround to
    NovaProxyRequestHandler to respond with a 400 Bad Request if an open
    redirect is attempted:

      Ie36401c782f023d1d5f2623732619105dc2cfa24
      I95f68be76330ff09e5eabb5ef8dd9a18f5547866

    Recently in python 3.10.6, a fix has landed in cpython to respond with
    a 301 Moved Permanently to a sanitized URL that has had extra leading
    '/' characters removed.

    This breaks our existing unit tests which assume a 400 Bad Request as
    the only expected response.

    This adds handling of a 301 Moved Permanently response and asserts that
    the redirect location is the expected sanitized URL. Doing this instead
    of checking for a given python version will enable the tests to continue
    to work if and when the cpython fix gets backported to older python
    versions.

    While updating the tests, the opportunity was taken to commonize the
    code of two unit tests that were nearly identical.

    Related-Bug: #1927677
    Closes-Bug: #1986545

    Change-Id: I27441d15cc6fa2ff7715ba15aa900961aadbf54a
    (cherry picked from commit 15769b883ed4a86d62b141ea30d3f1590565d8e0)
    (cherry picked from commit 4a2b44c7cf55d1d79d5a2dd638bd0def3af0f5af)
    (cherry picked from commit 0e4a257e8636a979605c614a35e79ba47b74d870)

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

Reviewed: https://review.opendev.org/c/openstack/nova/+/866195
Committed: https://opendev.org/openstack/nova/commit/77bc3f004e7fe4077ea035c659630bedef1cfea1
Submitter: "Zuul (22348)"
Branch: stable/victoria

commit 77bc3f004e7fe4077ea035c659630bedef1cfea1
Author: melanie witt <email address hidden>
Date: Tue Aug 16 06:49:53 2022 +0000

    Adapt websocketproxy tests for SimpleHTTPServer fix

    In response to bug 1927677 we added a workaround to
    NovaProxyRequestHandler to respond with a 400 Bad Request if an open
    redirect is attempted:

      Ie36401c782f023d1d5f2623732619105dc2cfa24
      I95f68be76330ff09e5eabb5ef8dd9a18f5547866

    Recently in python 3.10.6, a fix has landed in cpython to respond with
    a 301 Moved Permanently to a sanitized URL that has had extra leading
    '/' characters removed.

    This breaks our existing unit tests which assume a 400 Bad Request as
    the only expected response.

    This adds handling of a 301 Moved Permanently response and asserts that
    the redirect location is the expected sanitized URL. Doing this instead
    of checking for a given python version will enable the tests to continue
    to work if and when the cpython fix gets backported to older python
    versions.

    While updating the tests, the opportunity was taken to commonize the
    code of two unit tests that were nearly identical.

    Conflicts:
        nova/tests/unit/console/test_websocketproxy.py

    NOTE(melwitt): The conflict is because change
    I58b0382c86d4ef798572edb63d311e0e3e6937bb
    (Refactor and rename test_tcp_rst_no_compute_rpcapi) is not in
    Victoria.

    Related-Bug: #1927677
    Closes-Bug: #1986545

    Change-Id: I27441d15cc6fa2ff7715ba15aa900961aadbf54a
    (cherry picked from commit 15769b883ed4a86d62b141ea30d3f1590565d8e0)
    (cherry picked from commit 4a2b44c7cf55d1d79d5a2dd638bd0def3af0f5af)
    (cherry picked from commit 0e4a257e8636a979605c614a35e79ba47b74d870)
    (cherry picked from commit 3023e162e1a415ddaa70b4b8fbe24b1771dbe424)

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

Reviewed: https://review.opendev.org/c/openstack/nova/+/866196
Committed: https://opendev.org/openstack/nova/commit/746d654c23d75f084b6f0c70e6c32b97eebf419c
Submitter: "Zuul (22348)"
Branch: stable/ussuri

commit 746d654c23d75f084b6f0c70e6c32b97eebf419c
Author: melanie witt <email address hidden>
Date: Tue Aug 16 06:49:53 2022 +0000

    Adapt websocketproxy tests for SimpleHTTPServer fix

    In response to bug 1927677 we added a workaround to
    NovaProxyRequestHandler to respond with a 400 Bad Request if an open
    redirect is attempted:

      Ie36401c782f023d1d5f2623732619105dc2cfa24
      I95f68be76330ff09e5eabb5ef8dd9a18f5547866

    Recently in python 3.10.6, a fix has landed in cpython to respond with
    a 301 Moved Permanently to a sanitized URL that has had extra leading
    '/' characters removed.

    This breaks our existing unit tests which assume a 400 Bad Request as
    the only expected response.

    This adds handling of a 301 Moved Permanently response and asserts that
    the redirect location is the expected sanitized URL. Doing this instead
    of checking for a given python version will enable the tests to continue
    to work if and when the cpython fix gets backported to older python
    versions.

    While updating the tests, the opportunity was taken to commonize the
    code of two unit tests that were nearly identical.

    Related-Bug: #1927677
    Closes-Bug: #1986545

    Change-Id: I27441d15cc6fa2ff7715ba15aa900961aadbf54a
    (cherry picked from commit 15769b883ed4a86d62b141ea30d3f1590565d8e0)
    (cherry picked from commit 4a2b44c7cf55d1d79d5a2dd638bd0def3af0f5af)
    (cherry picked from commit 0e4a257e8636a979605c614a35e79ba47b74d870)
    (cherry picked from commit 3023e162e1a415ddaa70b4b8fbe24b1771dbe424)
    (cherry picked from commit 77bc3f004e7fe4077ea035c659630bedef1cfea1)

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

Reviewed: https://review.opendev.org/c/openstack/nova/+/866201
Committed: https://opendev.org/openstack/nova/commit/43af75243b79e95330ecd982552d568571623e39
Submitter: "Zuul (22348)"
Branch: stable/train

commit 43af75243b79e95330ecd982552d568571623e39
Author: melanie witt <email address hidden>
Date: Tue Aug 16 06:49:53 2022 +0000

    Adapt websocketproxy tests for SimpleHTTPServer fix

    In response to bug 1927677 we added a workaround to
    NovaProxyRequestHandler to respond with a 400 Bad Request if an open
    redirect is attempted:

      Ie36401c782f023d1d5f2623732619105dc2cfa24
      I95f68be76330ff09e5eabb5ef8dd9a18f5547866

    Recently in python 3.10.6, a fix has landed in cpython to respond with
    a 301 Moved Permanently to a sanitized URL that has had extra leading
    '/' characters removed.

    This breaks our existing unit tests which assume a 400 Bad Request as
    the only expected response.

    This adds handling of a 301 Moved Permanently response and asserts that
    the redirect location is the expected sanitized URL. Doing this instead
    of checking for a given python version will enable the tests to continue
    to work if and when the cpython fix gets backported to older python
    versions.

    While updating the tests, the opportunity was taken to commonize the
    code of two unit tests that were nearly identical.

    Conflicts:
        nova/tests/unit/console/test_websocketproxy.py

    NOTE(melwitt): The conflict is because change
    I23ac1cc79482d0fabb359486a4b934463854cae5 (Allow TLS ciphers/protocols
    to be configurable for console proxies) is not in Train. The difference
    from the cherry picked change is because the flake8 version on the
    stable/train branch does not support f-strings [1].

    Related-Bug: #1927677
    Closes-Bug: #1986545

    [1] https://lists.openstack.org/pipermail/openstack-discuss/2019-November/011027.html

    Change-Id: I27441d15cc6fa2ff7715ba15aa900961aadbf54a
    (cherry picked from commit 15769b883ed4a86d62b141ea30d3f1590565d8e0)
    (cherry picked from commit 4a2b44c7cf55d1d79d5a2dd638bd0def3af0f5af)
    (cherry picked from commit 0e4a257e8636a979605c614a35e79ba47b74d870)
    (cherry picked from commit 3023e162e1a415ddaa70b4b8fbe24b1771dbe424)
    (cherry picked from commit 77bc3f004e7fe4077ea035c659630bedef1cfea1)
    (cherry picked from commit 746d654c23d75f084b6f0c70e6c32b97eebf419c)

tags: added: in-stable-train
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/nova train-eol

This issue was fixed in the openstack/nova train-eol release.

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

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.