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

[Swe W Aung]

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.

[Swe W Aung (sirswa)]

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
<

[Jeremy Stanley (fungi)]

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.

[melanie witt (melwitt)]

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

[melanie witt (melwitt)]

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

[melanie witt (melwitt)]

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

[melanie witt (melwitt)]

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.

[melanie witt (melwitt)]

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)]

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.

[OpenStack Infra (hudson-openstack)]

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

[Swe W Aung (sirswa)]

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"

[Swe W Aung (sirswa)]

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"

[melanie witt (melwitt)]

Hi sw3,

Thanks for reporting the issue!

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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)

[Jeremy Stanley (fungi)]

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?

[melanie witt (melwitt)]

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

[Joshua Padman (jpadman)]

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?

[Jeremy Stanley (fungi)]

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

[OpenStack Infra (hudson-openstack)]

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

[Nick Tait (nickthetait)]

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?

[Shahaan Ayyub (shahaan)]

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

[Jeremy Stanley (fungi)]

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

[Swe W Aung (sirswa)]

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.

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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)

[Jeremy Stanley (fungi)]

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.

[Jeremy Stanley (fungi)]

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

[melanie witt (melwitt)]

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.

[OpenStack Infra (hudson-openstack)]

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

[Jeremy Stanley (fungi)]

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.

[Nick Tait (nickthetait)]

CVE-2021-3654 https://access.redhat.com/security/cve/CVE-2021-3654

[Nick Tait (nickthetait)]

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

[Jeremy Stanley (fungi)]

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.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/nova/+/803091

[melanie witt (melwitt)]

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.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[Matteo Pozza (matteopozza)]

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.

[Matteo Pozza (matteopozza)]

This is a proposal for Python 2.

[Matteo Pozza (matteopozza)]

This is a proposal for Python 3.

[OpenStack Infra (hudson-openstack)]

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

[Balazs Gibizer (balazs-gibizer)]

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)

[OpenStack Infra (hudson-openstack)]

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

[Jeremy Stanley (fungi)]

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.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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

[Jeremy Stanley (fungi)]

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.

[melanie witt (melwitt)]

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.

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[Jeremy Stanley (fungi)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/nova/+/853379

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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.

[OpenStack Infra (hudson-openstack)]

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.

[OpenStack Infra (hudson-openstack)]

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.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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

[Lucas (lucas6)]

Python fix an open redirection vulnerability(CVE-2021-28861) in the HTTP server when an URI path starts with //[1]. Reducing to a single /, the Nova patch[2] for fixing the CVE-2021-3654 vulnerability does not take effect.

For example:
The URI patch is changed from ////example.com/%2F.. to /example.com/%2F.. and passed to Nova.

[1] https://github.com/python/cpython/commit/5715382d3a89ca118ce2e224d8c69550d21fe51b
[2] https://review.opendev.org/c/openstack/nova/+/806629/6/nova/console/websocketproxy.py

[Jeremy Stanley (fungi)]

Lucas: Are you possibly just rediscovering related bug 1986545?