Skip to content

Commit b77d60c

Browse files
committed
issue 2550690 - Adding anti-csrf measures to roundup following
https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevention_Cheat_Sheet and https://seclab.stanford.edu/websec/csrf/csrf.pdf Basically implement Synchronizer (CSRF) Tokens per form on a page. Single use (destroyed once used). Random input data for the token includes: system random implementation in python using /dev/urandom (fallback to random based on timestamp as the seed. Not as good, but should be ok for the short lifetime of the token??) the id (in cpython it's the memory address) of the object requesting a token. In theory this depends on memory layout, the history of the process (how many previous objects have been allocated from the heap etc.) I claim without any proof that for long running processes this is another source of randomness. For short running processes with little activity it could be guessed. last the floating point time.time() value is added. This may only have 1 second resolution so may be guessable. Hopefully for a short lived (2 week by default) token this is sufficient. Also in the current implementation the user is notified when validation fails and is told why. This allows the roundup admin to find the log entry (at error level) and try to resolve the issue. In the future user notification may change but for now this is probably best.
1 parent 4da4ad4 commit b77d60c

File tree

10 files changed

+841
-29
lines changed

10 files changed

+841
-29
lines changed

CHANGES.txt

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,16 @@ Features:
178178
be deprecated. See ``upgrading.txt`` for details.
179179
- New property for permissions added to simplify the model. See
180180
``customizing.txt`` and search for props_only and
181-
set_props_only_default in the section 'Adding a new Permission'.
182-
(John Rouillard)
181+
set_props_only_default in the section 'Adding a new Permission'.
182+
(John Rouillard)
183+
- issue2550690 - Inadequate CSRF protection. Improvements in
184+
Cross Site Request Forgery protection to check HTTP headers
185+
and nonces. If the header/nonce is present, they are
186+
validated. But if headers or nonces are missing access is
187+
granted. The enforcement policy can be set in config.ini.
188+
Requiring enforcement will need some changes to
189+
templates. Support for protecting xmlrpc endpoint not well
190+
tested. See ``upgrading.txt``. (John Rouillard)
183191

184192
Fixed:
185193

doc/customizing.txt

Lines changed: 69 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1644,6 +1644,54 @@ for each action are:
16441644
**search**
16451645
Determine whether the user has permission to view this class.
16461646

1647+
Protecting users from web application attacks
1648+
---------------------------------------------
1649+
1650+
There is a class of attacks known as Cross Site Request Forgeries
1651+
(CSRF). Malicious code running in the browser can making a
1652+
request to roundup while you are logged into roundup. The
1653+
malicious code piggy backs on your existing roundup session to
1654+
make changes without your knowledge. Roundup 1.6 has support for
1655+
defending against this by analyzing the
1656+
1657+
* Referer,
1658+
* Origin, and
1659+
* Host or
1660+
* X-Forwarded-Host
1661+
1662+
HTTP headers. It compares the headers to the value of the web setting
1663+
in the [tracker] section of the tracker's ``config.ini``.
1664+
1665+
Also a per form token (also called a nonce) can be enabled for
1666+
the tracker using the ``csrf_enforce_token`` option in
1667+
config.ini. When enabled, roundup will validate a hidden form
1668+
field called ``@csrf``. If the validation fails (or the token
1669+
is used more than one) the request is rejected. The ``@csrf``
1670+
input field is added automatically by calling the ``submit``
1671+
function/path. It can also be added manually by calling
1672+
anti_csrf_nonce() directly. For example:
1673+
1674+
<input name="@csrf" type="hidden"
1675+
tal:attributes="value python:utils.anti_csrf_nonce(lifetime=10)">
1676+
1677+
By default a nonce lifetime is 2 weeks. However the lifetime (in
1678+
minutes) can be set by passing a lifetime argument as shown
1679+
above. The example above makes the nonce lifetime 10 minutes.
1680+
1681+
Search for @csrf in this document for more examples. There are
1682+
more examples and information in ``upgrading.txt``.
1683+
1684+
The token protects you because malicious code supplied by another
1685+
site is unable to obtain the token. Thus many attempts they make
1686+
to submit a request are rejected.
1687+
1688+
The protection on the xmlrpc interface is untested, but is based
1689+
on a valid header check against the roundup url and the presence
1690+
of the ``X-REQUESTED-WITH`` header. Work to improve this is a
1691+
future project after the 1.6 release.
1692+
1693+
The enforcement levels an be modified in ``config.ini``. Refer to
1694+
that file for details.
16471695

16481696
Special form variables
16491697
----------------------
@@ -2366,7 +2414,7 @@ classhelp display a link to a javascript popup containing this class'
23662414
javascript help_window function - it's the name of the form
23672415
the "property" belongs to.
23682416

2369-
submit generate a submit button (and action hidden element)
2417+
submit generate a submit button (and action and @csrf hidden elements)
23702418
renderWith render this class with the given template.
23712419
history returns 'New node - no history' :)
23722420
is_edit_ok is the user allowed to Edit the current class?
@@ -2399,7 +2447,7 @@ There are several methods available on these wrapper objects:
23992447
=============== ========================================================
24002448
Method Description
24012449
=============== ========================================================
2402-
submit generate a submit button (and action hidden element)
2450+
submit generate a submit button (and action and @csrf hidden elements)
24032451
journal return the journal of the current item (**not
24042452
implemented**)
24052453
history render the journal of the current item as HTML
@@ -3610,6 +3658,12 @@ Then a submit button so that the user can submit the new category::
36103658
</td>
36113659
</tr>
36123660

3661+
The ``context/submit`` bit generates the submit button but also
3662+
generates the @action and @csrf hidden fields. The @action field is
3663+
used to tell roundup how to process the form. The @csrf field provides
3664+
a unique single use token to defend against CSRF attacks. (More about
3665+
anti-csrf measures can be found in ``upgrading.txt``.)
3666+
36133667
Finally we finish off the tags we used at the start to do the METAL
36143668
stuff::
36153669

@@ -5097,16 +5151,21 @@ To edit the status of all items in the item index view, edit the
50975151

50985152
<tr>
50995153
<td tal:attributes="colspan python:len(request.columns)">
5154+
<input name="@csrf" type="hidden"
5155+
tal:attributes="value python:utils.anti_csrf_nonce()">
51005156
<input type="submit" value=" Save Changes ">
51015157
<input type="hidden" name="@action" value="edit">
51025158
<tal:block replace="structure request/indexargs_form" />
51035159
</td>
51045160
</tr>
51055161

5106-
which gives us a submit button, indicates that we are performing an edit
5107-
on any changed statuses. The final ``tal:block`` will make sure that the
5108-
current index view parameters (filtering, columns, etc) will be used in
5109-
rendering the next page (the results of the editing).
5162+
which gives us a submit button, indicates that we are performing an
5163+
edit on any changed statuses, and provides a defense against cross
5164+
site request forgery attacks.
5165+
5166+
The final ``tal:block`` will make sure that the current index view
5167+
parameters (filtering, columns, etc) will be used in rendering the
5168+
next page (the results of the editing).
51105169

51115170

51125171
Displaying only message summaries in the issue display
@@ -5189,6 +5248,8 @@ Setting up a "wizard" (or "druid") for controlled adding of issues
51895248

51905249
<form method="POST" onSubmit="return submit_once()"
51915250
enctype="multipart/form-data">
5251+
<input name="@csrf" type="hidden"
5252+
tal:attributes="value python:utils.anti_csrf_nonce()">
51925253
<input type="hidden" name="@template" value="add_page1">
51935254
<input type="hidden" name="@action" value="page1_submit">
51945255

@@ -5205,6 +5266,8 @@ Setting up a "wizard" (or "druid") for controlled adding of issues
52055266
tal:condition="context/is_edit_ok"
52065267
tal:define="cat request/form/category/value">
52075268

5269+
<input name="@csrf" type="hidden"
5270+
tal:attributes="value python:utils.anti_csrf_nonce()">
52085271
<input type="hidden" name="@template" value="add_page2">
52095272
<input type="hidden" name="@required" value="title">
52105273
<input type="hidden" name="category" tal:attributes="value cat">

doc/upgrading.txt

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,84 @@ Contents:
2323
Migrating from 1.5.1 to 1.6.0
2424
=============================
2525

26+
Cross Site Request Forgery Detection Added
27+
------------------------------------------
28+
29+
Roundup 1.6. supports a number of defenses against CSRF.
30+
31+
Http header verification against the tracker's ``web``
32+
setting in the ``[tracker]`` section of config.ini for the
33+
following headers:
34+
35+
# Analyze the ``Referer`` HTTP header to make sure it
36+
includes the web setting.
37+
# Analyse the ``Origin`` HTTP header to make sure the
38+
schema://host matches the web setting.
39+
# Analyze the ``X-Forwarded-Host`` header set by a proxy
40+
running in front of roundup to make sure it agrees with
41+
the host part of the web setting.
42+
# Analyze the ``Host`` header to make sure it agrees with
43+
the host part of the web setting. This is not done if
44+
``X-Forwarded-Host`` is set.
45+
46+
By default roundup 1.6 does not require any specific header
47+
to be present. However at least one of the headers above
48+
*must* pass validation checks (usually ``Host`` or
49+
``Referer``) or the submission is rejected with an error.
50+
If any header fails validation, the submission is
51+
rejected. (Note the user's form keeps all the data they
52+
entered if it was rejected.)
53+
54+
Also the admin can include unique csrf tokens for all forms
55+
submitted via post (delete and put methods are also
56+
included, but not currently used by roundup)). The csrf
57+
token (nonce) is tied to the user's session. When the user
58+
submits the form and nonce, the nonce is checked to make
59+
sure it was issued to the user and the same session. If this
60+
is not true the post is rejected and the user is notified.
61+
62+
The standard context/submit templating item creates CSRF
63+
tokens by default. If you have forms that are not using the
64+
standard submit routine, you should add the following field
65+
to all forms:
66+
67+
<input name="@csrf" type="hidden"
68+
tal:attributes="value python:utils.anti_csrf_nonce()">
69+
70+
A unique random token is generated by every call to
71+
utils.anti_csrf_nonce() and is put in a database to be
72+
retreived if the token is used. Token lifetimes are 2 weeks
73+
by default but can be configured in config.ini. Roundup will
74+
automatically prune old tokens. Calling anti_csrf_nonce with
75+
an integer lifetime, for example
76+
77+
<input name="@csrf" type="hidden"
78+
tal:attributes="value python:utils.anti_csrf_nonce(lifetime=10)">
79+
80+
sets the lifetime of that nonce to 10 minutes.
81+
82+
If you want to change the default settings, you have to
83+
update the web section in your tracker's config.ini's. To do
84+
this backup your existing config.ini. Run:
85+
86+
roundup-admin -i /path/to/tracker genconfig config.ini.new
87+
88+
to create a new config.ini in the file config.ini.new. Then
89+
merge the new csrf settings into your tracker's config.
90+
Look for settings that start with csrf. The config.ini.new
91+
file includes detailed descriptions of the settings.
92+
93+
In general one of four values can be set for these
94+
settings. The default is ``yes``, which validates the header
95+
or nonce and blocks access if the validation fails. If the
96+
field/header is missing it allows access. Setting these
97+
fields to ``required`` blocks access if the header/nonce is
98+
missing.
99+
100+
It is suggested that you change your templates so every form
101+
has an @csrf field and change the setting to 'required' for
102+
the csrf_enforce_token.
103+
26104
Fix for path traversal changes template resolution
27105
--------------------------------------------------
28106

roundup/cgi/actions.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1141,6 +1141,14 @@ def handle(self):
11411141
self.client.nodeid = None
11421142
self.client.template = None
11431143

1144+
# Redirect to a new page on logout. This regenerates
1145+
# CSRF tokens so they are associated with the
1146+
# anonymous user and not the user who logged out. If
1147+
# we don't the user gets an invalid CSRF token error
1148+
# As above choose the home page since everybody can
1149+
# see that.
1150+
raise exceptions.Redirect, self.base
1151+
11441152
class LoginAction(Action):
11451153
def handle(self):
11461154
"""Attempt to log a user in.

0 commit comments

Comments
 (0)