From: Ben Darnell <ben@bendarnell.com>
Date: Mon, 26 May 2014 01:41:44 +0000 (-0400)
Subject: Disallow non-absolute urls in origin field.
X-Git-Tag: v4.0.0b1~34
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6403cb91a06b9b629bd89e71249b2cae30ceb267;p=thirdparty%2Ftornado.git

Disallow non-absolute urls in origin field.

Update docs for websocket check_origin.
---

diff --git a/tornado/test/websocket_test.py b/tornado/test/websocket_test.py
index 736da141e..05ff40b1e 100644
--- a/tornado/test/websocket_test.py
+++ b/tornado/test/websocket_test.py
@@ -160,7 +160,7 @@ class WebSocketTest(AsyncHTTPTestCase):
         self.assertEqual(reason, 'goodbye')
 
     @gen_test
-    def test_check_origin_valid(self):
+    def test_check_origin_valid_no_path(self):
         port = self.get_http_port()
 
         url = 'ws://localhost:%d/echo' % port
@@ -175,7 +175,7 @@ class WebSocketTest(AsyncHTTPTestCase):
         yield self.close_future
 
     @gen_test
-    def test_check_origin_with_path(self):
+    def test_check_origin_valid_with_path(self):
         port = self.get_http_port()
 
         url = 'ws://localhost:%d/echo' % port
@@ -190,19 +190,16 @@ class WebSocketTest(AsyncHTTPTestCase):
         yield self.close_future
 
     @gen_test
-    def test_check_origin_valid2(self):
+    def test_check_origin_invalid_partial_url(self):
         port = self.get_http_port()
 
         url = 'ws://localhost:%d/echo' % port
         headers = {'Origin': 'localhost:%d' % port}
 
-        ws = yield websocket_connect(HTTPRequest(url, headers=headers),
-            io_loop=self.io_loop)
-        ws.write_message('hello')
-        response = yield ws.read_message()
-        self.assertEqual(response, 'hello')
-        ws.close()
-        yield self.close_future
+        with self.assertRaises(HTTPError) as cm:
+            yield websocket_connect(HTTPRequest(url, headers=headers),
+                                    io_loop=self.io_loop)
+        self.assertEqual(cm.exception.code, 403)
 
     @gen_test
     def test_check_origin_invalid(self):
@@ -220,13 +217,13 @@ class WebSocketTest(AsyncHTTPTestCase):
         self.assertEqual(cm.exception.code, 403)
 
     @gen_test
-    def test_check_origin_invalid2(self):
+    def test_check_origin_invalid_subdomains(self):
         port = self.get_http_port()
 
         url = 'ws://localhost:%d/echo' % port
-        # subdomains should be invalid by default
-        headers = {'Origin': 'http://subtenant.somewhereelse.com',
-                   'Host': 'subtenant2.somewhereelse.com'}
+        # Subdomains should be disallowed by default.  If we could pass a
+        # resolver to websocket_connect we could test sibling domains as well.
+        headers = {'Origin': 'http://subtenant.localhost'}
 
         with self.assertRaises(HTTPError) as cm:
             yield websocket_connect(HTTPRequest(url, headers=headers),
diff --git a/tornado/websocket.py b/tornado/websocket.py
index d797a0cd2..3767a207d 100644
--- a/tornado/websocket.py
+++ b/tornado/websocket.py
@@ -158,7 +158,7 @@ class WebSocketHandler(tornado.web.RequestHandler):
 
         # If there was an origin header, check to make sure it matches
         # according to check_origin. When the origin is None, we assume it
-        # came from a browser and that it can be passed on.
+        # did not come from a browser and that it can be passed on.
         if origin is not None and not self.check_origin(origin):
             self.stream.write(tornado.escape.utf8(
                 "HTTP/1.1 403 Cross Origin Websockets Disabled\r\n\r\n"
@@ -277,18 +277,24 @@ class WebSocketHandler(tornado.web.RequestHandler):
 
     def check_origin(self, origin):
         """Override to enable support for allowing alternate origins.
-        
-        By default, this checks to see that the host matches the origin
-        provided.
+
+        The ``origin`` argument is the value of the ``Origin`` HTTP
+        header, the url responsible for initiating this request.  This
+        method is not called for clients that do not send this header;
+        such requests are always allowed (because all browsers that
+        implement WebSockets support this header, and non-browser
+        clients do not have the same cross-site security concerns).
+
+        Should return True to accept the request or False to reject it.
+        By default, rejects all requests with an origin on a host other
+        than this one.
 
         This is a security protection against cross site scripting attacks on
-        browsers, since WebSockets don't have CORS headers.
-        """
+        browsers, since WebSockets are allowed to bypass the usual same-origin
+        policies and don't use CORS headers.
 
-        # Due to how stdlib's urlparse is implemented, urls without a //
-        # are interpreted to be paths (resulting in netloc being None)
-        if("//" not in origin):
-            origin = "//" + origin
+        .. versionadded:: 3.3
+        """
         parsed_origin = urlparse(origin)
         origin = parsed_origin.netloc
         origin = origin.lower()