Skip to content

Commit fbf13c1

Browse files
iarenazaptaoussanis
authored andcommitted
[mod] [#465] [#418] Move WS CSRF token from params to protocol header (@iarenaza)
Before this commit: WebSocket client conveys CSRF token to server via `:csrf-token` query param. After this commit: WebSocket client conveys CSRF token to server via `Sec-WebSocket-Protocol` header. Motivation: Moving the token from query param to a header helps reduce the likelihood of accidental leaking (e.g. via logging). While the `Sec-WebSocket-Protocol` header isn't specifically intended for conveying metadata like a CSRF token, the consensus seems to be that this is anyway a practical choice without major downsides or obviously better alternatives. As implemented the change tries to respect other values in the `Sec-WebSocket-Protocol` header that may have been set when calling `make-channel-socket-client!`.
1 parent e4cba96 commit fbf13c1

File tree

1 file changed

+33
-7
lines changed

1 file changed

+33
-7
lines changed

src/taoensso/sente.cljc

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,8 @@
369369
(allow-origin? #{"http://site.com"} {:headers {"referer" "http://attacker.com/"}})
370370
(allow-origin? #{"http://site.com"} {:headers {"referer" "http://site.com.attacker.com/"}}))
371371

372+
(def ^:private sente-csrf-token-prefix "sente-csrf-token-")
373+
372374
(defn make-channel-socket-server!
373375
"Takes a web server adapter[1] and returns a map with keys:
374376
@@ -633,6 +635,21 @@
633635
;; undefined):
634636
nil)
635637

638+
sente-csrf-token-pred
639+
(fn [s]
640+
(when (str/starts-with? s sente-csrf-token-prefix)
641+
(subs s (count sente-csrf-token-prefix))))
642+
643+
ws-csrf-token
644+
(fn [ring-req]
645+
(let [headers (get ring-req :headers)]
646+
(when-let [ws? (= "websocket" (get headers "upgrade") )]
647+
(let [sec-websocket-protocol (get headers "sec-websocket-protocol")
648+
protocol-vals
649+
(when (string? sec-websocket-protocol)
650+
(str/split sec-websocket-protocol #", *"))]
651+
(enc/rsome sente-csrf-token-pred protocol-vals)))))
652+
636653
bad-csrf?
637654
(fn [ring-req]
638655
(if (nil? csrf-token-fn) ; Provides a way to disable CSRF check
@@ -642,7 +659,8 @@
642659
(or
643660
(get-in ring-req [:params :csrf-token])
644661
(get-in ring-req [:headers "x-csrf-token"])
645-
(get-in ring-req [:headers "x-xsrf-token"]))]
662+
(get-in ring-req [:headers "x-xsrf-token"])
663+
(ws-csrf-token ring-req))]
646664

647665
(not
648666
(enc/const-str=
@@ -1316,7 +1334,8 @@
13161334
(enc/oget @?node-npm-websocket_ "w3cwebsocket"))]
13171335

13181336
(delay
1319-
(let [socket (WebSocket. uri-str)]
1337+
(let [protocols (get headers :sec-websocket-protocol)
1338+
socket (WebSocket. uri-str protocols)]
13201339
(doto socket
13211340
(aset "onerror" on-error)
13221341
(aset "onmessage" on-message) ; Nb receives both push & cb evs!
@@ -1569,13 +1588,20 @@
15691588
{:on-error on-error
15701589
:on-message on-message
15711590
:on-close on-close
1572-
:headers headers
1591+
:headers
1592+
(update headers :sec-websocket-protocol
1593+
(fn [x]
1594+
(let [csrf-token
1595+
(str sente-csrf-token-prefix
1596+
(get-client-csrf-token-str :dynamic (:csrf-token @state_)))]
1597+
(cond
1598+
(string? x) [x csrf-token]
1599+
(coll? x) (conj x csrf-token)
1600+
:else csrf-token))))
1601+
15731602
:uri-str
15741603
(enc/merge-url-with-query-string url
1575-
(merge params ; 1st (don't clobber impl.):
1576-
{:client-id client-id
1577-
:csrf-token (get-client-csrf-token-str :dynamic
1578-
(:csrf-token @state_))}))}))
1604+
(assoc params :client-id client-id))}))
15791605

15801606
(catch #?(:clj Throwable :cljs :default) t
15811607
(timbre/errorf t "Error creating WebSocket client")

0 commit comments

Comments
 (0)