sente icon indicating copy to clipboard operation
sente copied to clipboard

Securing cross origin requests [CVE-2019-1000022]

Open danielcompton opened this issue 9 years ago • 22 comments

From https://www.christian-schneider.net/CrossSiteWebSocketHijacking.html, if somebody wasn't using CSRF tokens, it seems like it would be possible for any malicious website to open up a web socket to do Bad Things. I know that CSRF tokens are highly recommended, but they're not suitable for all cases (I think). It could be good to also add CORS style protection to Sente to allow only whitelisted origins. What are your thoughts?

N.B. CORS itself has no influence on websocket connections.

danielcompton avatar Jun 14 '15 21:06 danielcompton

From the article:

As a Pentester Check for Cross-Site WebSocket Hijacking attacks as soon as you notice any WebSocket based communication in the application you're analysing. As a side note, in case you already find Origin header verification present in the application, try to bypass it from victim's browser: When the server expects https://www.some-trading-application.com as the Origin, mount your attacks on https://www.some-trading-application.com.some-evil-attacker-application.com to test for obviously broken Origin header verifications.

As a Security Consultant Make your clients aware of the requirement to always check Origin headers. Educate them to secure all WebSocket handshakes using random tokens (like protecting against CSRF attacks) or let them embed the authentication and/or authorization into the WebSocket protocol (avoid web session access).

As a Developer Make sure you are aware of this attack scenario and know how to employ the countermeasures securely into your application (at least when you need to access the web session from the application part that uses WebSockets or when you otherwise try to transfer non-public data over that channel). Better try to avoid accessing the web session from the server-side WebSocket counterpart and separately handle authentication and/or authorization using tokens or similar techniques within your WebSocket protocol.

danielcompton avatar Jun 14 '15 21:06 danielcompton

Hi Daniel,

I know that CSRF tokens are highly recommended, but they're not suitable for all cases (I think).

First step would be suggesting a use case where they're not suitable so that we know what we're talking about exactly (why they're unsuitable in this case, etc.).

It could be good to also add CORS style protection to Sente to allow only whitelisted origins. What are your thoughts?

Is there a reason you wouldn't want to be using Content Security Policy for this?

ptaoussanis avatar Jun 22 '15 03:06 ptaoussanis

~I think CSP is exactly what I was looking for for this. It looks like connect-src is the appropriate policy.~

UPDATE: ^^This is incorrect, I was mistaken in my understanding of what CSP provided.

danielcompton avatar Jun 22 '15 21:06 danielcompton

Actually, I think that it might still be important to check Origin headers too, but I need to think about this.

danielcompton avatar Jun 22 '15 21:06 danielcompton

Okay to close this?

ptaoussanis avatar Jul 06 '15 08:07 ptaoussanis

Sure, for now. I may revisit this in the future.

danielcompton avatar Jul 06 '15 12:07 danielcompton

So, I'm in the process of a security review on code for a consulting client, and I'm looking at websockets.

I've looked at the Sente code now for a couple of hours, and I cannot see how the current implementation is secure. Worse, it looks to me like it is providing a way for an attacker to actually obtain the CSRF token, since sente sends it as part of the handshake...all an attacker needs to do, it seems to me, is know that sente is in use, emulate the handshake logic, and now the attacker not only knows the CSRF, but they can hijack the websocket for a fully async "authorized" communication channel.

My understanding is that CSRF only works if the secret is embedded into the HTML (because js cannot request HTML pages cross-origin) and manually transferred into the js network request back to the server. If it can be "queried" by js with nothing but a cookie as backup, then the security is broken.

I did read your example, and it is true that requiring a user to auth after the connection is established is the "gold standard" (assuming you require they type a password), but that requires the user to log in again if they reload the page (or temporarily lose the network connection).

As far as I can tell the current CSRF logic basically just gets you "around" the anti-forgery middleware denying requests...it doesn't actually provide any real security, and I do think it actually compromises the anti-forgery protections on "other" POSTs.

Perhaps you intended for users of the library to implement their own security around this (which might be the case...it requires HTML twiddling that sente cannot do, and origin checks that would require more config).

I'm putting this in this closed issue because there is some chance I've misunderstood something.

If not, I'd recommend removing the CSRF from the handshake, and documenting that it has to be passed through the HTML as a parameter given to sente on the client. E.g. the HTML serve would embed a js var or something into the head of the page, and cljs would read that var to set the option for sente.

awkay avatar Oct 15 '18 05:10 awkay

(Disclaimer: I'm not a security professional.)

I've spent the last couple of days looking into WebSocket security and I'm mostly left confused.

I don't understand how Sente's anti-CSRF token implementation protects from CSRF attacks. (It might work when using the Ajax long-polling bits of Sente — I haven't looked into that part because I don't use it myself).

For example, say I have a web app at https://foo.com and I've logged into it with my browser. Sente is listening in at wss://foo.com/chsk. The web app also has the Ring anti-forgery middleware correctly configured, which means Sente can use the anti-CSRF token.

I then open another tab in the same browser and type this into the browser's Developer Console:

var ws = new WebSocket("wss://foo.com/chsk?client-id=foo")

ws.onmessage = function (event) {
  console.log(event.data);
}

Because the WebSocket connection is not subject to the Same-Origin Policy, I can open the connection successfully. The session cookie is passed along with the WebSocket connection request, which means authentication isn't required. The anti-CSRF token Sente uses seems to have no effect.

Embedding the anti-CSRF token into the HTML might be better than the current situation, but I don't see how even that would currently prevent CSRF attacks, seeing as Sente doesn't actually seem to require the anti-CSRF token to establish a WebSocket connection. Also, it seems to me that if we did that, we'd be using the anti-CSRF token as a kind of an authorization mechanism, which isn't what the anti-CSRF token is for.

I don't really see how the anti-CSRF token is relevant for WebSocket requests, but it's entirely possible that I'm missing something.

With regard to CSP: I've seen recommendations to add the Content-Security-Policy: connect-src 'self' header into the server response. For example, this Gist says that [the header] "prevents webSockets requests from any place but the current server. "

That's not true, however. As I understand it, a content-security policy basically says:

While you're on this site, you're only allowed to load resources from these sources.

In other words, with WebSockets, if your CSP is connect-src 'self', while you're on https://foo.com, you're only allowed to connect to a WebSocket server running on https://foo.com. The CSP disallows connections to WebSocket endpoints in any other URLs. If you're on http://attacker.com, though, a CSP on https://foo.com has no effect on anything you do.

So, if https://foo.com has an XSS vulnerability that an attacker uses to try to connect to a WebSocket endpoint elsewhere (e.g. to display misleading information to the user or something like that), the `connect-src 'self' CSP would prevent that.

As I see it, a CSP does not protect the user from CSRF attacks — at least with regard to WebSockets.

The only thing that I'm aware of that protects the user from CSRF attacks targeting WebSocket endpoints is to check that the value of the Origin header matches the current domain in the WebSocket handshake route handler (which is what @danielcompton proposed earlier).

For example, a Ring middleware for https://foo.com would look something like this:

(defn wrap-ensure-origin
  [handler]
  (fn [request]
    (if (= (get-in request [:headers "origin"]) "https://foo.com")
      (handler request)
      {:status 403
       :body   "Forbidden"})))

You'd then wrap that middleware around Sente's ring-ajax-get-or-ws-handshake handler.

If your app runs on multiple different domains, you'll unfortunately have to add the domain names in your app's configuration. That means that the best thing Sente can do is to advise users to add middleware that checks the Origin header into their web apps, I think.

That's my understanding of the whole picture. I'm hoping someone will correct me if I'm wrong on something — I'm learning as I go here.

eerohele avatar Nov 02 '18 08:11 eerohele

Hi there!

Thank you @awkay, @eerohele for pinging and for the detailed info- this was very helpful.

Indeed, it looks like Sente's CSRF protection may be broken. And (worse), as @awkay suggested, seems to actually leak the Ring CSRF token.

Looking into this right now, will try update shortly.

ptaoussanis avatar Nov 02 '18 09:11 ptaoussanis

Just pushed an attempted minimal fix, including an updated example project.

It's on Clojars as [com.taoensso/sente "1.2.0-SNAPSHOT"]. Did this in a bit of a rush, so would very much appreciate some additional eyes to verify.

This doesn't do any origin checking (yet), but should prevent the CSRF leakage - and extends the CSRF check over all endpoints, including the WebSocket handshake. The commit is small if anyone wants to help check.

I'd also be open to a PR that adds a convenient way to do origin checking. Perhaps a predicate of the Origin header?

Any input welcome. Again, much thanks for bringing attention to this.

ptaoussanis avatar Nov 02 '18 12:11 ptaoussanis

I started evaluating the change, but I'm having trouble setting up the requisite middleware correctly (not Sente's fault).

I'll keep at it, but in the meantime: I think v1.2.0 already exists, right? So I think the version number should be something different.

eerohele avatar Nov 02 '18 14:11 eerohele

Just some preliminary results: the new CSRF scheme seems to work OK in that I can't connect to a WebSocket endpoint from a different origin without the anti-CSRF token any more.

However, for some reason, with 1.2.0-SNAPSHOT, I can't connect to a WebSocket endpoint e.g. with wscat even if I supply the correct anti-CSRF token in the query string:

$ wscat --connect "ws://localhost:8000/chsk?client-id=46563081-259b-4ddb-a21f-af8b8eb87256&csrf-token=<redacted>"
error: Unexpected server response: 403

With 1.13.1, that obviously works because Sente didn't check the anti-CSRF token. I didn't have the time to check why I get that 403 yet. It's not a problem for my use case, but might be for someone else. For example, if you're testing your WebSocket endpoints with something like Gniazdo.

eerohele avatar Nov 02 '18 14:11 eerohele

So, this like is for promotion and reconnect, right?

https://github.com/ptaoussanis/sente/commit/442a347fe1dcf140e1c613e1b78248e4a96c1b04?w=1#diff-793a4aa02e2757c465c85e63ff89f346R1040

Headers are used on the initial part now it seems. If I'm reading that right, then I agree. My workaround on the current version was forced to pass the token as a param on setup, which isn't ideal.

I don't have time to actually run any live tests on it right now, but thanks for looking into it!

awkay avatar Nov 02 '18 15:11 awkay

Have pushed [com.taoensso/sente "1.14.0-RC1"] to Clojars for testing, adding also constant-time equality checking on the CSRF token to help prevent possible timing attacks.

If there's no issues with this release, will document further and release as v1.14.0.

ptaoussanis avatar Dec 02 '18 12:12 ptaoussanis

It would be good to get a CVE-number (Common Vulnerabilities and Exposures) for this, so developers using lein-nvd or similar tool would be notified about this.

korkeala avatar Jan 19 '19 08:01 korkeala

@korkeala Nice idea, thank you for the suggestion. I've just requested a CVE through the Distributed Weakness Filing Project.

I'll update here (and test lein-nvd) if I hear anything back. In the meantime: would it be worth submitting anywhere else? Am not familiar with this process.

ptaoussanis avatar Jan 19 '19 09:01 ptaoussanis

Great, that project looks good (though I'm not familiar with it). One request should be sufficient, there is only one central vulnerability database (https://nvd.nist.gov/) which the dependency check tools use to download CVE-data. So as long as the request ends up there, it will be enough.

korkeala avatar Jan 19 '19 09:01 korkeala

The docstring for ring.middleware.anti-forgery/*anti-forgery-token* says:

The default session strategy stores the token directly, but other strategies may wrap the token in a delay if the token is expensive to compute. The var should therefore be realized with clojure.core/force before use.

I think the Sente example project should be updated to reflect that. That is:

--- a/example-project/src/example/server.clj
+++ b/example-project/src/example/server.clj
@@ -66,7 +66,7 @@
 (defn landing-pg-handler [ring-req]
   (hiccup/html
     [:h1 "Sente reference example"]
-    [:div#sente-csrf-token {:data-csrf-token anti-forgery/*anti-forgery-token*}]
+    [:div#sente-csrf-token {:data-csrf-token (force anti-forgery/*anti-forgery-token*)}]
     [:p "An Ajax/WebSocket" [:strong " (random choice!)"] " has been configured for this example"]
     [:hr]
     [:p [:strong "Step 1: "] " try hitting the buttons:"]

Alternatively, the Ring anti-forgery middleware also stores the anti-CSRF token in the request, so this should also work:

--- a/example-project/src/example/server.clj
+++ b/example-project/src/example/server.clj
@@ -66,7 +66,7 @@
 (defn landing-pg-handler [ring-req]
   (hiccup/html
     [:h1 "Sente reference example"]
-    [:div#sente-csrf-token {:data-csrf-token anti-forgery/*anti-forgery-token*}]
+    [:div#sente-csrf-token {:data-csrf-token (:anti-forgery-token ring-req)}]
     [:p "An Ajax/WebSocket" [:strong " (random choice!)"] " has been configured for this example"]
     [:hr]
     [:p [:strong "Step 1: "] " try hitting the buttons:"]

The benefit of retrieving the token from the request is that you don't need to think about whether *anti-forgery-token* is actually bound. I'm not sure whether there's a situation where retrieving the token from the request isn't an option, though, so I don't know which is the better default.

Other than that, the current implementation seems to work insofar as the handshake no longer leaks the anti-CSRF token. You also can't connect to the WebSocket endpoint without the token.

However, after this change, Sente can no longer work with (many) non-browser WebSocket clients. Unless I'm mistaken (and it's entirely possible that I am), the current CSRF protection scheme relies on the browser being able to hold on to a session that contains the anti-CSRF token, but many non-browser clients don't have that ability.

I'm not sure how big of a deal that is. It does mean that since the anti-CSRF token is the only means of CSRF protection and it's mandatory, you can't test Sente WebSocket endpoints with a tool like wscat or Gniazdo any more. Using the Origin header as an option would be more flexible in that respect, I think. The wscat CLI supports it, for example:

$ wscat --help

  Usage: wscat [options] (--listen <port> | --connect <url>)

  Options:

    ...
    -o, --origin <origin>         optional origin
    ...

eerohele avatar Jan 22 '19 14:01 eerohele

Thanks for the response @eerohele!

Example updated. PR welcome to add Origin header verification 👍

ptaoussanis avatar Jan 23 '19 19:01 ptaoussanis

FYI, `system' at "0.4.3-SNAPSHOT" is incorporating the latest recommendation for the anti forgery token. Thank you.

danielsz avatar Feb 14 '19 07:02 danielsz

Just a note that to further improve security, Sente should pass the anti-CSRF token as part of Sec-WebSocket-Protocol header (see https://stackoverflow.com/a/35108078/232644) and not as a request parameter as it currently does. That would likely require some custom changes to ring handlers, but perhaps it'd be better for Sente to handle (more of) the verification directly. Might warrant a different issue.

kaosko avatar Oct 15 '19 15:10 kaosko

@kaosko Hi Kalle, open to a PR and/or new-issue if you have an idea in mind- thanks!

ptaoussanis avatar Jul 23 '20 10:07 ptaoussanis

Have created https://github.com/ptaoussanis/sente/issues/418 for the Sec-WebSocket-Protocol header, closing this then as done.

ptaoussanis avatar Feb 23 '23 14:02 ptaoussanis