[OSSA 2015-005] Websocket Hijacking Vulnerability in Nova VNC Server (CVE-2015-0259)

[Josh Kleinpeter]

OpenStack Vulnerability Team:

Brian Manifold (<email address hidden>) from Cisco has discovered a
vulnerability in the Nova VNC server implementation. We have a patch for
this vulnerability and consider this a very high risk.

Please email Dave McCowan (<email address hidden>) for more details on the attached patch.

Issue Details:

Horizon uses a VNC client which uses websockets to pass information. The
Nova VNC server does not validate the origin of the websocket request,
which allows an attacker to make a websocket request from another domain.
If the victim opens both an attacker's site and the VNC console
simultaneously, or if the victim has recently been using the VNC console
and then visits the attacker's site, the attacker can make a websocket
request to the Horizon domain and proxy the connection to another
destination.

This gives the attacker full read-write access to the VNC console of any
instance recently accessed by the victim.

Recommendation:
 Verify the origin field in request header on all websocket requests.

Threat:
      CWE-345
 * Insufficient Verification of Data Authenticity -- The software does not
sufficiently verify the origin or authenticity of data, in a way that
causes it to accept invalid data.

      CWE-346
 * Origin Validation Error -- The software does not properly verify that
the source of data or communication is valid.

      CWE-441
 * Unintended Proxy or Intermediary ('Confused Deputy') -- The software
receives a request, message, or directive from an upstream component, but
the software does not sufficiently preserve the original source of the
request before forwarding the request to an external actor that is outside
of the software's control sphere. This causes the software to appear to be
the source of the request, leading it to act as a proxy or other
intermediary between the upstream component and the external actor.

Steps to reproduce:
 1. Login to horizon
 2. Pick an instance, go to console/vnc tab, wait for console to be loaded
 3. In another browser tab or window, load a VNC console script from local
disk or remote site
 4. Point the newly loaded VNC console to the VNC server and a connection
is made
Result:
 The original connection has been been hijacked by the second connection

Root cause:
 Cross-Site WebSocket Hijacking is concept that has been written about in
various security blogs.
One of the recommended countermeasures is to check the Origin header of
the WebSocket handshake request.

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

[Tristan Cacqueray (tristan-cacqueray)]

The vulnerability reported seems exploitable. @Alaski can you please check nova impact and the proposed patch ?

[Andrew Laski (alaski)]

I am trying to understand this fully as I do not have a lot of prior experience with cross-site type exploits. When requesting console access from Nova it returns a URL with an auth token included as a GET query parameter. On its own there doesn't seem to be anything exploitable here. The problem seems to come in when that URL is passed into a browser and the token is stored in a cookie? And then subsequent attempts at accessing that URL from the same browser would pass the token in that cookie. It seems that using the token from the cookie is part of the security concern, but may not be something we can stop doing based on a comment in the code.

There does appear to be some risk here, but I do have concerns about the proposed patch. It is possible that some deployers are currently relying on the behavior explained here so there is a risk of breaking them. I would suggest amending the patch so that the origin checking is configurable, with the default being to check it.

[Tristan Cacqueray (tristan-cacqueray)]

@alaski: (if i understand this correctly) the problem is that a malicious website can initiate a connection directly to the websocket server on behalf of the user (thus using the previously stored token), and without a check on the origin this will effectively hijack the session.

For what it worth, I doubt there is a real use-case of having the connection origin different from the host. The proposed fix seems good to me.

[Josh Kleinpeter (jkleinpeter)]

@tristan-cacqueray: yes, that is correct. From there, you can do what you like with the instance.

[Andrew Laski (alaski)]

Assuming the user is logged in to the instance on the websocket session, yes it provides total access to the instance.

I'm okay with the patch as written as it would address the issue and if configurability is desired later it can be added in. I would prefer that a ValidationError be raised as opposed to creating a new exception class but that's my only nitpick.

[Tristan Cacqueray (tristan-cacqueray)]

Can the patch be re-formatted to include commit message ?

Also about affected version, I guess it goes back to stable/icehouse, thus backports for Juno and Icehouse are required as well...

Finally the report only mention VNC console, but SPICE seems affected as well right ?

Here is impact description draft #1:

Title: Nova console Cross-Site WebSocket hijacking
Reporter: Brian Manifold (Cisco)
Products: Nova
Versions: up to 2014.1.3 and 2014.2 versions up to 2014.2.1

Description:
Brian Manifold from Cisco reported a vulnerability in Nova console websocket. By tricking an authenticated user into clicking a malicious URL, a remote attacker may trigger a cross-site-scripting vulnerability resulting in potential hijack of consoles where the user is still logged in. Only Nova setups with vnc or spice enabled are affected.

[Dave McCowan (dave-mccowan)]

Patch for Kilo

[Dave McCowan (dave-mccowan)]

Patch for Juno

[Dave McCowan (dave-mccowan)]

Patch for Icehouse

[Dave McCowan (dave-mccowan)]

I've attached 3 patches that address the requests made by Andrew and Tristan (include commit message, use ValidationError exception, and provide patches for Icehouse and Juno).

[Thierry Carrez (ttx)]

nova-coresec please review proposed patches

[Tristan Cacqueray (tristan-cacqueray)]

Thanks for the patches, they passed ./run_tests.sh here. However flake8 failed for an undefined name 'null' in tests and some other minor warnings.

I also switched the OSSA task to 'In Progress' as an impact description have been proposed in comment #8.

[Dave McCowan (dave-mccowan)]

Update to stable-icehouse patch

[Dave McCowan (dave-mccowan)]

Update to stable-juno patch

[Dave McCowan (dave-mccowan)]

Update to master-kilo patch

[Dave McCowan (dave-mccowan)]

Thanks Tristan. New patches uploaded fixing flake8 issues.

[Thierry Carrez (ttx)]

@nova-coresec please review proposed patches!

[Andrew Laski (alaski)]

The patches look good to me, and will address the issue described here.

[Jeremy Stanley (fungi)]

Tristan's impact description in comment #8 looks fine except that now it affects up to 2014.2.2 (released earlier today).

[John Garbutt (johngarbutt)]

I am +2 on these nova patches too, looks good. Sorry for the need to ping me quite so much.

I have slight reservations on deployment scenarios where this header check is too aggressive, but its an important bug to fix, so its probably a case of crossing the bridge when we come to it. It would be worse to make it configurable to start with.

[Tristan Cacqueray (tristan-cacqueray)]

Thanks you for the review! Now that we have a CVE, proposed disclosure date:
2015-02-12, 1500UTC

[Paul McMillan (paul-mcmillan)]

(first a nickpick, this is CSRF or CSWF, definitely not XSS mentioned in the comment in the patch)

The patch improves things, but it doesn't completely solve the problem.

I'm sorry to bring these things up, since they're not going to be trivial to patch.

We need to verify that the url scheme (https or http) in the origin matches the expected scheme. Otherwise, a mitm can spoof an unencrypted page on the same domain, obtain an origin header with the same domain, and conduct the attack against an HTTPS cloud.

We need to maintain a whitelist of expected source domains, and compare against that rather than comparing against the host header. Without this, the current implementation is vulnerable to DNS rebinding.

[Jeremy Stanley (fungi)]

Thanks--we caught the XSS vs CSRF bit after the CVE was requested with the original wording and so stuck with it in the pre-OSSA.

As for "not going to be trivial to patch" do you suspect we'll be able to backport a more thorough solution to stable branches without violating our stable branch change policy? If not, we should probably continue with the original disclosure timeline to fix what can be trivially backported while publicly solving it in a more thorough fashion for the upcoming kilo release.

[Paul McMillan (paul-mcmillan)]

I don't know if Nova already has access somewhere to the necessary information to fix these issues correctly. Perhaps someone who's more familiar can help out.

In the first case, it needs to know if the system is being served behind https.

In the second case, it needs to know the hostname it is serving from (provided by the deployer, rather than intuited from the request).

[Dave McCowan (dave-mccowan)]

For the DNS binding concern, I've been told that it's important that customers can access their consoles through either https://foo.example.net:6080 or https://192.168.8.8:6080. If we restricted access to a white list of domains, it would break that use case. Does anyone have thoughts on that?

For the http/https case, the code is checking the entire Origin, not just the domain. So, a MITM would have to spoof http://foo.example.net:6080 in order to hijack https://foo.example.net:6080. Is this the case you want to protect from? It seems that would be straightforward addition to the code... just check that the origin is HTTPS, if current connection is HTTPS.

[Tristan Cacqueray (tristan-cacqueray)]

@Paul correct me if I'm wrong, but an attacker able to redirect a user to a DNS rebinded version of a cloud domain would likely have greater capabilities than just snooping on an eventual opened vnc/spice session... e.g., hijack the whole horizon session. Thus if it's the case, we should address this in another bug.

So far the fix seems to prevent the most trivial exploitation path and we should stick to the original timeline unless there is something wrong with the proposed patch.

[Paul McMillan (paul-mcmillan)]

@Dave for the http/https case, you are correct, if you're confident that nova reliably has access to whether or not the current connection is HTTPS on the client side. It's common to run TLS decryption at cloud boundaries so that secrets aren't spread across the cloud. Does nova properly support forwarded headers to determine this state? Does it correctly ignore them when it does not need them?

Re: DNS rebinding and host whitelisting: I see no reason you can't whitelist both IP addresses and the expected domain names if that is the expected customer use case. Browsers still send the HOST header with the IP address if that's what's in the url.

@Tristan: the DNS rebinding issue is correctly mitigated in Horizon because Horizon uses Django, which provides robust protection against this class of attacks. Nova's adhoc reinvention of these mechanisms is what got us in trouble here.

@Tristan: we need to fix the http/https thing more urgently than we need to fix the dns rebinding thing. Both issues will become public when this bug becomes public, but the http/https thing is trivially exploitable. If we keep this bug private, we'll need to issue a followup patch and CVE shortly - I'm ok with that, but not linking to the public launchpad bug will encourage people to look more closely at the patch.

The DNS rebinding issue is less critical, though I suspect you'll find people are happy to demonstrate an exploit the moment you claim something isn't a problem.

As the current patch stands, it's not wrong, just incomplete. If you'd prefer I open new issues for these things, I'll do that and we can treat them separately.

[Tristan Cacqueray (tristan-cacqueray)]

@Paul: can you please describe explicitly the attack scenario here ?

I'm not that experienced with CSRF or CSWF, could you explain how an attacker would redirect a user to an un-encrypted page from the same domain ?

IIUC, all three connections required to connect to a console are protected by TLS:

[1] user requests nova-api and it returns the url of novnc proxy with a token
[2] user connect to novnc proxy and it'll serve javascript to connect to websocket proxy
[3] user's browser connect to websocket proxy

Am I missing something here ?

[Paul McMillan (paul-mcmillan)]

@Tristan: Happy to describe in more detail, these things are extremely non-intuitive if you haven't spent a while thinking about them.

Our scenario involves a setup as follows:

Nova and horizon configured to serve from https. Nova is patched with the current proposed patch.

User is accessing the cloud through a man in the middle who controls all traffic to and from the user. [1]

user -> attacker -> cloud(https)

Here's what happens:
1) User logs into cloud securely via https://yourcloud.com/
2) User securely accesses a server via websocket VNC and logs in. User (optionally) closes this window.
3) User opens a new browser tab to an insecure site (it really can be any insecure site at all)
4) On receiving the request for the insecure site, the attacker fetches it from the source, and rewrites it to include an invisible attack iframe before serving it to the user. [2]
5) The attack iframe directs the user's browser to open http://yourcloud.com:6080 inside the hidden iframe (even if you don't serve that site insecurely)
6) When the user's browser requests http://yourcloud.com:6080, the attacker intercepts this request.
7) Rather than serving the expected content, the attacker serves javascript which instructs the user's browser to re-connect to the secure websocket that was previously used for VNC.
8) When the user's browser sends this request to your secure websocket, it sends the cookies associated with the secure domain, even though the javascript which triggered this request (and controls the resulting websocket) did not come from a secure domain. The attacker can't see these cookies, but the user's browser does send them.
9) The secure server inspects the origin header. Since the origin for this websocket request is http://yourcloud:6080, the current patch allows the request (since the origin domain and port match the HOST header). Then the server checks the cookies (which are correct, since they are the users legit cookies from the earlier session) and the server allows the request.
10) Now the attacker controls a websocket which has been opened with the user's credentials. If the user left the VNC session logged in (very common when you're fixing something like broken networking, since closing the VNC terminal doesn't log the user out of the terminal), the attacker has full control over the VM and will proceed to install a permanent backdoor as quickly as possible.

Without the current patch, the attacker doesn't have to bother with the MITM, and can just open the websocket from any webpage they control. It's counter intuitive, but browsers don't prevent this kind of connection and even when the socket is opened from a foreign page, it comes with the secret cookies related to the target page, granting access.

Hopefully that helps you understand how this attack works. It does require a different attack setup than the plain CSRF scenario, but it produces similar end results. I agree with you that the current patch is a major improvement, and if it's going to take a significant delay to implement a fix for this issue, we should put out the current patch and deal with this one separately.

-Paul

[1] As a practical aside, it is easy to b...

[Paul McMillan (paul-mcmillan)]

One point about the above explanation that I didn't make clearly is that during normal operation, when the user's browser makes a websocket request for VNC, it is to wss://yourcloud.com:6080/websockify, and it sends the token which chooses the instance as a cookie.

The websocket does not send the token as a get request (even though you see it as a get request parameter when you load the html which serves the javascript vnc client), so that cookie containing the instance token persists until the next time anyone (from anywhere on the internet) asks the user browser to request a websocket to wss://yourcloud.com:6080/websockify.

[Dave McCowan (dave-mccowan)]

@Paul : Thank you very much for you detailed comments; I have definitely learned a lot.
To summarize, this patch does protect against a simple click-bait attack, but does not protect against simultaneous click-bait and MITM attacks. NoVNC has other issues with MITM (https://bugs.launchpad.net/nova/+bug/1197459), in that if a MITM attacker can capture the token from the URL, then he can access the console directly.
As you point out, the white-list solution is not a simple fix. NoVNC will need information that it may not have, such as knowledge of proxies, load-balancers, and TLS unwrappers that may affect the connection information.

[Tristan Cacqueray (tristan-cacqueray)]

Thanks for such a detailed explanation, it makes much more sense now. So basically if the attacker is a mitm, then the vulnerability is still exploitable.

Is Dave's proposed solution in comment #27 be enough then ? ( check that the origin is HTTPS, if current connection is HTTPS. ). Does NovaProxyRequestHandlerBase have access to that information ?

[Dave McCowan (dave-mccowan)]

@Tristan: Paul pointed out my error in #29. If the TLS connection is unwrapped by an intermediate (such as stunnel), then there would need to be coordination between Nova and the unwrapper. This coordination, such as an inserted HTTP header, would probably need configuration options to make sure it is done right for each deployment.

[Jeremy Stanley (fungi)]

I suspect (but would love to be proven wrong) that any fix involving adding configuration to handle these situations will be at odds with our stable branch change policy, and as such would not be covered by the security advisory since it would only be mitigated thoroughly in the master branch and next major release.

[Paul McMillan (paul-mcmillan)]

This is how Django handles and docs the coordination between a TLS unwrapper and the application, if necessary:
https://docs.djangoproject.com/en/1.7/ref/settings/#secure-proxy-ssl-header
I believe this approach and the documentation around it is correct and reasonably standard. A similar approach would probably be correct for nova.

@Dave I was aware of the "token in the GET parameter issue" raised in 1197459, and agree that part isn't usually exploitable in a proper https site (though it is pretty horrifying), but the "cookie isn't set securely" part is actually quite exploitable. I can open a separate private bug about that.

@Jeremy: Does the stable branch policy preclude us from adding new configuration values with reasonable defaults, and saying "Add these configs if you want to be secure"?

If we can't that, we could do something truly horrible like detecting if we ever see a https referer, and then permanently shifting to only accepting https referers.

@all do we agree that given a) the current patch improves things, and b) the fixes for the issues raised here are non-trivial, we should move these issues into separate tickets?

[Jeremy Stanley (fungi)]

Our stable branch guidelines are a little vague on this point, and mainly discourage introducing backward-incompatible configuration changes. https://wiki.openstack.org/wiki/StableBranch#Appropriate_Fixes The bigger concern is backporting a significant behavior change or a change in default values of existing configuration options, I think.

Prior art involving security advisories where configuration options were added in backports: bug 1336207, bug 1354208

[Tristan Cacqueray (tristan-cacqueray)]

So we are postponing the advisory in order to find a better workaround.

The ideal would be to find a backport-able fix. But in the case this is not feasible, maybe we could come up with a better than nothing interim solution like an option to make sure origin scheme is https ?

[Paul McMillan (paul-mcmillan)]

To the best of my knowledge, Nova does not currently have the necessary information to make a correct decision about this.

I would strongly recommend that we take the same approach Django has, and add support for specifying a header which tells nova whether the request is secure or not. I wouldn't object to also supporting a setting which just flat out toggled HTTPS mode on or off, since that may be easier for many people to deploy without reconfiguring their load balancers and proxies.

In either case, the new settings should have sane defaults which don't break people's installations if they don't configure them.

[Andrew Laski (alaski)]

Weird... I thought I applied that patch to a clean master branch but there was an extraneous scheduler change after it applied, but I'm not seeing it here.

I'm +2 on the three patches.

[Paul McMillan (paul-mcmillan)]

Just so we're clear here, in order to make this work properly, I'm going to have to specify both a novnc base url and a spice base url, even if I don't use one or the other?

[Paul McMillan (paul-mcmillan)]

Ah no, I didn't finish reading the latest patch. I like this approach better.

[Dave McCowan (dave-mccowan)]

@Paul, yes happily you're right at #115, we ended up with a clean solution. You we're also right at #114, we did briefly talk about a solution that would have had that horrible requirement. :-)

[Tristan Cacqueray (tristan-cacqueray)]

Last batch of patches fixes flake8 errors. I guess we are good to go now!

Proposed disclosure date:
2015-03-10, 1500UTC

[Tony Breeds (o-tony)]

The patches look good to me. +1

[Paul McMillan (paul-mcmillan)]

I applied this patch today to our system, and ran into trouble. Apparently the console type is 'novnc' instead of the 'vnc' that the patch looks for. I got this error:

 94: handler exception: Invalid Console Type for WebSocketProxy: 'novnc'
 94: Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/websockify/websocket.py", line 711, in top_new_client
    self.new_client()
  File "/usr/lib/python2.7/dist-packages/nova/console/websocketproxy.py", line 94, in new_client
    origin.scheme):
  File "/usr/lib/python2.7/dist-packages/nova/console/websocketproxy.py", line 60, in verify_origin_proto
    raise exception.ValidationError(detail=detail)
ValidationError: Invalid Console Type for WebSocketProxy: 'novnc'

and applying the attached diff to the patched installation fixed it for me. We probably need to update the patches we sent out - can someone else verify this end-to-end with their novnc system to confirm it?

[Tristan Cacqueray (tristan-cacqueray)]

I confirm, vnc connections are broken in Icehouse and Juno.

[Tristan Cacqueray (tristan-cacqueray)]

Thanks Dave,

Can we please get yet another round of approval on the last patch ?
Note that it just change console_type 'vnc' into 'novnc'.

[Paul McMillan (paul-mcmillan)]

This latest round of patches looks correct to me

[Tristan Cacqueray (tristan-cacqueray)]

Manual VNC connection on Icehouse and Juno worked correctly here on devstacks. Can someone else confirm the last round patch works properly before we send another follow-up ?

Also, does someone have an objection if we keep the last proposed disclosure date (2015-03-10, 1500UTC) ?

[Thierry Carrez (ttx)]

Fine with keeping same date

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/163033

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/juno
Review: https://review.openstack.org/163034

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/icehouse
Review: https://review.openstack.org/163035

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/163604

[Tristan Cacqueray (tristan-cacqueray)]

So two other issues have been found with the proposed patch:

1/ out of browser client that does not set the Origin header won't work. So we are removing the origin check if origin is not set, assuming all vulnerable web-based request would include the Origin header.

2/ Serial console access is also broken for Juno and Kilo.

Dave is working on another version.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Dave McCowan (<email address hidden>) on branch: master
Review: https://review.openstack.org/163604
Reason: Duplicate of https://review.openstack.org/#/c/163033/

[Paul McMillan (paul-mcmillan)]

Since the Origin header is required with a MUST by the RFC for browsers, I think I'm ok with this, absent any evidence that it's possible to convince browsers to leave the origin header off.

https://tools.ietf.org/html/rfc6455#section-4.1

   8. The request MUST include a header field with the name |Origin|
        [RFC6454] if the request is coming from a browser client. If
        the connection is from a non-browser client, the request MAY
        include this header field if the semantics of that client match
        the use-case described here for browser clients. The value of
        this header field is the ASCII serialization of origin of the
        context in which the code establishing the connection is
        running. See [RFC6454] for the details of how this header field
        value is constructed.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/163033
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=fdb73a2d445971c6158a80692c6f74094fd4193a
Submitter: Jenkins
Branch: master

commit fdb73a2d445971c6158a80692c6f74094fd4193a
Author: Dave McCowan <email address hidden>
Date: Mon Mar 2 15:00:22 2015 -0500

    Websocket Proxy should verify Origin header

    If the Origin HTTP header passed in the WebSocket handshake does
    not match the host, this could indicate an attempt at a
    cross-site attack. This commit adds a check to verify
    the origin matches the host.

    SecurityImpact

    Change-Id: Ica6ec23d6f69a236657d5ba0c3f51b693c633649
    Closes-Bug: 1409142

[Paul McMillan (paul-mcmillan)]

The fix merged to latest looks good to me.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/163035
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=0ff674217024a59a79bf80ab752a5cbf7e94642c
Submitter: Jenkins
Branch: stable/icehouse

commit 0ff674217024a59a79bf80ab752a5cbf7e94642c
Author: Dave McCowan <email address hidden>
Date: Tue Feb 24 21:33:58 2015 -0500

    Websocket Proxy should verify Origin header

    If the Origin HTTP header passed in the WebSocket handshake does
    not match the host, this could indicate an attempt at a
    cross-site attack. This commit adds a check to verify
    the origin matches the host.

    Change-Id: Ica6ec23d6f69a236657d5ba0c3f51b693c633649
    Closes-Bug: 1409142

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/163034
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=676ba7bbc788a528b0fe4c87c1c4bf94b4bb6eb1
Submitter: Jenkins
Branch: stable/juno

commit 676ba7bbc788a528b0fe4c87c1c4bf94b4bb6eb1
Author: Dave McCowan <email address hidden>
Date: Tue Feb 24 21:35:48 2015 -0500

    Websocket Proxy should verify Origin header

    If the Origin HTTP header passed in the WebSocket handshake does
    not match the host, this could indicate an attempt at a
    cross-site attack. This commit adds a check to verify
    the origin matches the host.

    Change-Id: Ica6ec23d6f69a236657d5ba0c3f51b693c633649
    Closes-Bug: 1409142

[Yukihiro KAWADA (warp-kawada)]

Is this patch valid for console_type = 'serial'?

novaconsole kwd_vm_123
this cli fails.

origin:http expected:ws

[Lars Kellogg-Stedman (larsks)]

You will find yourself here if your formerly-working console connections have suddenly started failing with "ValidationError: Origin header protocol does not match this host". This is because the changes introduced to resolve this issue make the value of "novncproxy_base_url" important on the host running nova-novncproxy, whereas previously this value was unused except by nova-compute.

The default value for novncproxy_base_url uses an "http://" url, which means that any https:// connections to your console will be rejected until you update this configuration option and restart nova-novncproxy.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/169753

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/169752
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=2ffcf18d00eff6fb0777769469c4aa5ac7bbb6c9
Submitter: Jenkins
Branch: master

commit 2ffcf18d00eff6fb0777769469c4aa5ac7bbb6c9
Author: Nikola Dipanov <email address hidden>
Date: Wed Apr 1 14:35:13 2015 +0100

    consoleauth: Store access_url on token authorization

    Related-bug: 1409142

    As part of the fix for the related bug - we've added protocol checking
    to mitigate MITM attacks, however we base protocol checking on a config
    option that is normally only intended for compute hosts.

    This is quite user hostile, as it is now important that all nodes
    running compute and proxy services have this option in sync.

    We can do better than that - we can persist the URL the client is
    expected to use, and once we get it back on token validation, we can
    make sure that the request is using the intended protocol, mitigating
    the MITM injected script attacks.

    This patch makes sure that the access_url is persisted with the token -
    the follow-up patch makes consoles use that info.

    Change-Id: I02a377f54de46536ca35413b615d3298967afc33

[Nikola Đipanov (ndipanov)]

It is also worth noting here for future reference that the above 2 patches:

https://review.openstack.org/169752
https://review.openstack.org/169753

are relevant to Kilo only (they will not be backported to previous releases). In Kilo it will no longer be needed to keep the novncproxy_base_url options in sync between compute and novncproxy services. This is because the URL a token was authorized for will be kept alongside other authorization info.

However there is a caveat to the way upgrades need to be done for Juno to Kilo (it copied here from the original commit message):

UpgradeImpact: Websocket proxies need to be upgraded in a lockstep
with the API nodes up to this commit (or when upgrading to Kilo),
as older API nodes will not be sending the access_url when authorizing
console access, and newer proxy services (this commit and onward) would
fail to authorize such requests.

[Rui Yuan Dou (ry-dou)]

I tried the Juno patch in https://review.openstack.org/#/c/163034/ , and it did not really resolved this problem. I can still include the VNC page in a different domain.
The patch try to read a http header "Origin" and compare it with the page url, but it doesen't exist in the http headers of get request, so it wont actually block the Cross-Site visit. Can anyone verify it?

[Grant Murphy (gmurphy)]

I think this might be mentioned in a comment of:

https://review.openstack.org/#/c/163034/3/nova/console/websocketproxy.py

".. the Origin header is set by browser and it is set to the location of the script that makes the request. It's a mandatory field for browser but not relevant for command line tools. More information here: https://tools.ietf.org/html/rfc6455#section-10.2"

How are you making the GET request?

[Rui Yuan Dou (ry-dou)]

I just include the web console url in a iframe item of my localhost web server page, and the console still can be shown in that page, and there's no such "Origin" header exists.

[Abel Lopez (al592b)]

This is still present after this patch. In our testing, we're able to get consoles for the same instance on different hosts.

[Grant Murphy (gmurphy)]

I've opened https://bugs.launchpad.net/ossa/+bug/1511541 to track this as an incomplete fix. @al592b, @rydoc if you can attach your test case to that bug it might help expedite the process.

[Dave McCowan (dave-mccowan)]

@rydou: What browser did you use that did not include an Origin header? @Abel: Getting consoles for the same instance from different hosts is not the vulnerability. The vulnerability is getting access to the same instance from the same browser, but using websocket code downloaded from a different server (hijacking the existing connection). I'll follow up with you offline.