Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

web: Deprecate xsrf_cookies #3226

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions demos/blog/blog.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ def __init__(self, db):
template_path=os.path.join(os.path.dirname(__file__), "templates"),
static_path=os.path.join(os.path.dirname(__file__), "static"),
ui_modules={"Entry": EntryModule},
xsrf_cookies=True,
cookie_secret="__TODO:_GENERATE_YOUR_OWN_RANDOM_VALUE_HERE__",
login_url="/auth/login",
debug=True,
Expand Down Expand Up @@ -242,7 +241,7 @@ async def post(self):
self.get_argument("name"),
tornado.escape.to_unicode(hashed_password),
)
self.set_signed_cookie("blogdemo_user", str(author.id))
self.set_signed_cookie("blogdemo_user", str(author.id), samesite="lax")
self.redirect(self.get_argument("next", "/"))


Expand All @@ -269,7 +268,7 @@ async def post(self):
tornado.escape.utf8(author.hashed_password),
)
if password_equal:
self.set_signed_cookie("blogdemo_user", str(author.id))
self.set_signed_cookie("blogdemo_user", str(author.id), samesite="lax")
self.redirect(self.get_argument("next", "/"))
else:
self.render("login.html", error="incorrect password")
Expand Down
1 change: 0 additions & 1 deletion demos/blog/templates/compose.html
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
{% if entry %}
<input type="hidden" name="id" value="{{ entry.id }}"/>
{% end %}
{% module xsrf_form_html() %}
</form>
{% end %}

Expand Down
1 change: 0 additions & 1 deletion demos/blog/templates/create_author.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
Email: <input name="email" type="text"><br>
Name: <input name="name" type="text"><br>
Password: <input name="password" type="password"><br>
{% module xsrf_form_html() %}
<input type="submit">
</form>
{% end %}
1 change: 0 additions & 1 deletion demos/blog/templates/login.html
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
<form action="/auth/login" method="POST">
Email: <input name="email" type="text"><br>
Password: <input name="password" type="password"><br>
{% module xsrf_form_html() %}
<input type="submit">
</form>
{% end %}
1 change: 0 additions & 1 deletion demos/chat/chatdemo.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ async def main():
cookie_secret="__TODO:_GENERATE_YOUR_OWN_RANDOM_VALUE_HERE__",
template_path=os.path.join(os.path.dirname(__file__), "templates"),
static_path=os.path.join(os.path.dirname(__file__), "static"),
xsrf_cookies=True,
debug=options.debug,
)
app.listen(options.port)
Expand Down
2 changes: 0 additions & 2 deletions demos/chat/static/chat.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ function getCookie(name) {
}

jQuery.postJSON = function(url, args, callback) {
args._xsrf = getCookie("_xsrf");
$.ajax({url: url, data: $.param(args), dataType: "text", type: "POST",
success: function(response) {
if (callback) callback(eval("(" + response + ")"));
Expand Down Expand Up @@ -90,7 +89,6 @@ var updater = {
cursor: null,

poll: function() {
var args = {"_xsrf": getCookie("_xsrf")};
if (updater.cursor) args.cursor = updater.cursor;
$.ajax({url: "/a/message/updates", type: "POST", dataType: "text",
data: $.param(args), success: updater.onSuccess,
Expand Down
1 change: 0 additions & 1 deletion demos/chat/templates/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
<td style="padding-left:5px">
<input type="submit" value="{{ _("Post") }}">
<input type="hidden" name="next" value="{{ request.path }}">
{% module xsrf_form_html() %}
</td>
</tr>
</table>
Expand Down
5 changes: 3 additions & 2 deletions demos/facebook/facebook.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ def __init__(self):
login_url="/auth/login",
template_path=os.path.join(os.path.dirname(__file__), "templates"),
static_path=os.path.join(os.path.dirname(__file__), "static"),
xsrf_cookies=True,
facebook_api_key=options.facebook_api_key,
facebook_secret=options.facebook_secret,
ui_modules={"Post": PostModule},
Expand Down Expand Up @@ -84,7 +83,9 @@ async def get(self):
client_secret=self.settings["facebook_secret"],
code=self.get_argument("code"),
)
self.set_signed_cookie("fbdemo_user", tornado.escape.json_encode(user))
self.set_signed_cookie(
"fbdemo_user", tornado.escape.json_encode(user), samesite="lax"
)
self.redirect(self.get_argument("next", "/"))
return
self.authorize_redirect(
Expand Down
1 change: 0 additions & 1 deletion demos/websocket/chatdemo.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ def __init__(self):
cookie_secret="__TODO:_GENERATE_YOUR_OWN_RANDOM_VALUE_HERE__",
template_path=os.path.join(os.path.dirname(__file__), "templates"),
static_path=os.path.join(os.path.dirname(__file__), "static"),
xsrf_cookies=True,
)
super().__init__(handlers, **settings)

Expand Down
1 change: 0 additions & 1 deletion demos/websocket/templates/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
<td style="padding-left:5px">
<input type="submit" value="{{ _("Post") }}">
<input type="hidden" name="next" value="{{ request.path }}">
{% module xsrf_form_html() %}
</td>
</tr>
</table>
Expand Down
1 change: 0 additions & 1 deletion docs/guide/running.rst
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,6 @@ You can serve static files from Tornado by specifying the
"static_path": os.path.join(os.path.dirname(__file__), "static"),
"cookie_secret": "__TODO:_GENERATE_YOUR_OWN_RANDOM_VALUE_HERE__",
"login_url": "/login",
"xsrf_cookies": True,
}
application = tornado.web.Application([
(r"/", MainHandler),
Expand Down
95 changes: 83 additions & 12 deletions docs/guide/security.rst
Original file line number Diff line number Diff line change
Expand Up @@ -207,23 +207,63 @@ the Google credentials in a cookie for later access:

See the `tornado.auth` module documentation for more details.

.. _xsrf:

Cross-site request forgery protection
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

`Cross-site request
forgery <http://en.wikipedia.org/wiki/Cross-site_request_forgery>`_, or
XSRF, is a common problem for personalized web applications.

The generally accepted solution to prevent XSRF is to cookie every user
with an unpredictable value and include that value as an additional
argument with every form submission on your site. If the cookie and the
value in the form submission do not match, then the request is likely
forged.

Tornado comes with built-in XSRF protection. To include it in your site,
include the application setting ``xsrf_cookies``:
XSRF or CSRF (Tornado uses the acronym XSRF for historical reasons, although
CSRF is generally more commonly used today), is a common problem for
personalized web applications.

The simplest solution to this problem is to set the ``samesite`` attribute to
either ``lax`` or ``strict`` on all cookies used to authenticate the user.
(``lax`` is generally fine for this purpose; ``strict`` would be appropriate
if your application may use the HTTP ``GET`` method for requests that
have side effects). Note that as of January 2023, ``lax`` mode is the default
for some browsers, but not all.

In the web security model, a "site" is broader than an "origin" or "host"
(``example.com`` is a site; ``a.example.com`` and ``b.example.com`` are
hosts within that site). While not technically "cross-site", attacks from
one host to another within a site are relevant in many contexts. Such attacks
may be called
`"same-site request forgery" <https://minimalblue.com/data/papers/USENIX21_can_i_take_your_subdomain.pdf>`_
or "related-domain attacks". Protection against same-site attacks requires
something stronger than a ``samesite`` cookie, such as the
`synchronizer token pattern <https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#synchronizer-token-pattern>`_.
This pattern is somewhat invasive and cannot be implemented within Tornado
as it is a fairly low-level framework, but it may be worth considering at
the application level.

.. _legacy-xsrf:

Legacy XSRF protection
^^^^^^^^^^^^^^^^^^^^^^

Prior to the introduction of the ``samesite`` cookie attribute, Tornado
included an implementation of the
`double-submit cookie <https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#double-submit-cookie>`_
pattern, called ``xsrf_cookies``. This feature provided protection against
XSRF attacks that is equivalent to that provided by the ``samesite`` cookie
attribute, but required modifications to the application to ensure that the
XSRF token was passed whenever it was needed. Since the ``samesite`` cookie
attribute provides equivalent protection with less work, the ``xsrf_cookies``
feature is deprecated.

.. note::

By default, the ``xsrf_cookies`` feature does not protect against
same-site attacks. However, when combined with the use of the
`host-only cookie prefix <https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#Cookie_prefixes>`_
it becomes stronger and may be better than relying on ``samesite="lax"``.
To try this, use the application setting
``xsrf_cookie_kwargs={"name": "__Host-xsrf"}``. This usage should be
considered experimental, but if it works the ``xsrf_cookies`` feature may
become un-deprecated.

To enable the ``xsrf_cookies`` feature, use the application setting
``xsrf_cookies=True``:

.. testcode::

Expand Down Expand Up @@ -288,6 +328,37 @@ However, if you support both cookie and non-cookie-based authentication,
it is important that XSRF protection be used whenever the current
request is authenticated with a cookie.

.. _xsrf-deprecation:

Migrating from legacy XSRF protection
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The ``samesite="lax"`` cookie attribute, when applied to *all* cookies used
for authentication, provides protection against XSRF attacks that is
equivalent to Tornado's ``xsrf_cookies`` feature, so that feature is now
deprecated.

You may wish to continue using ``xsrf_cookies`` in some situations:

* If your application may perform side effects in response to HTTP GET
requests, but cannot use ``samesite="strict"``.
* If your authentication is based on something other than cookies, such
as TLS certificates or network addresses.

If you have an application that uses Tornado's ``xsrf_cookies`` feature
and you want to migrate to the ``samesite`` cookie attribute, follow these
steps:

1. Pass ``samesite="lax"`` (or ``samesite="strict"``) to `.set_signed_cookie`
(or its deprecated alias ``set_secure_cookie``) every time you set a cookie
to be used for user authentication.
2. Deploy your application. Wait until all cookies that might have been set
without the ``samesite`` attribute have expired.
3. Remove the ``xsrf_cookies=True`` application setting from your code,
and all instances of ``xsrf_form_html`` from your templates and code. If
you have JavaScript code that touches the ``_xsrf`` cookie or sets an
``_xsrf`` query parameter, it can be removed as well.

.. _dnsrebinding:

DNS Rebinding
Expand Down
1 change: 0 additions & 1 deletion docs/guide/templates.rst
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ Here is a properly internationalized template::
<div>{{ _("Username") }} <input type="text" name="username"/></div>
<div>{{ _("Password") }} <input type="password" name="password"/></div>
<div><input type="submit" value="{{ _("Sign in") }}"/></div>
{% module xsrf_form_html() %}
</form>
</body>
</html>
Expand Down
8 changes: 5 additions & 3 deletions docs/web.rst
Original file line number Diff line number Diff line change
Expand Up @@ -243,16 +243,18 @@
* ``login_url``: The `authenticated` decorator will redirect
to this url if the user is not logged in. Can be further
customized by overriding `RequestHandler.get_login_url`
* ``xsrf_cookies``: If ``True``, :ref:`xsrf` will be enabled.
* ``xsrf_cookies``: If ``True``, :ref:`legacy-xsrf` will be enabled.
This functionality is deprecated as of Tornado 6.3; see
:ref:`xsrf-deprecation` for more.
* ``xsrf_cookie_version``: Controls the version of new XSRF
cookies produced by this server. Should generally be left
at the default (which will always be the highest supported
version), but may be set to a lower value temporarily
during version transitions. New in Tornado 3.2.2, which
introduced XSRF cookie version 2.
introduced XSRF cookie version 2. Deprecated since Tornado 6.3.
* ``xsrf_cookie_kwargs``: May be set to a dictionary of
additional arguments to be passed to `.RequestHandler.set_cookie`
for the XSRF cookie.
for the XSRF cookie. Deprecated since Tornado 6.3.
* ``twitter_consumer_key``, ``twitter_consumer_secret``,
``friendfeed_consumer_key``, ``friendfeed_consumer_secret``,
``google_consumer_key``, ``google_consumer_secret``,
Expand Down
16 changes: 16 additions & 0 deletions tornado/test/web_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from tornado.simple_httpclient import SimpleAsyncHTTPClient
from tornado.template import DictLoader
from tornado.testing import AsyncHTTPTestCase, AsyncTestCase, ExpectLog, gen_test
from tornado.test.util import ignore_deprecation
from tornado.util import ObjectDict, unicode_type
from tornado.web import (
Application,
Expand Down Expand Up @@ -1721,6 +1722,11 @@ def get_handlers(self):
# explicitly defined error handler and an implicit 404.
return [("/error", ErrorHandler, dict(status_code=417))]

def get_app(self):
# xsrf_cookies is deprecated
with ignore_deprecation():
return super().get_app()

def get_app_kwargs(self):
return dict(xsrf_cookies=True)

Expand Down Expand Up @@ -2728,6 +2734,11 @@ def get(self):
def post(self):
self.write("ok")

def get_app(self):
# xsrf_cookies is deprecated
with ignore_deprecation():
return super().get_app()

def get_app_kwargs(self):
return dict(xsrf_cookies=True)

Expand Down Expand Up @@ -2924,6 +2935,11 @@ class Handler(RequestHandler):
def get(self):
self.write(self.xsrf_token)

def get_app(self):
# xsrf_cookies is deprecated
with ignore_deprecation():
return super().get_app()

def get_app_kwargs(self):
return dict(
xsrf_cookies=True, xsrf_cookie_kwargs=dict(httponly=True, expires_days=2)
Expand Down
Loading