Mercurial > p > roundup > code
changeset 5726:e199d0ae4a25
issue2551033: prevent reverse engineering hidden data by using etags
as an oracle to identify when the right data has been
guessed. Identified by Joseph Myers who also suggested remediation
methods. Implemented John Rouillard.
| author | John Rouillard <rouilj@ieee.org> |
|---|---|
| date | Thu, 23 May 2019 18:56:57 -0400 |
| parents | 6923225fd781 |
| children | 8b5171f353eb |
| files | CHANGES.txt doc/upgrading.txt roundup/configuration.py roundup/rest.py test/rest_common.py |
| diffstat | 5 files changed, 84 insertions(+), 38 deletions(-) [+] |
line wrap: on
line diff
--- a/CHANGES.txt Wed May 22 20:56:59 2019 -0400 +++ b/CHANGES.txt Thu May 23 18:56:57 2019 -0400 @@ -128,6 +128,9 @@ template. Replace with frame macro. (Cédric Krier) - handle UnicodeDecodeError in file class when file contents are not text (e.g. jpg). (John Rouillard) +- issue2551033: prevent reverse engineering hidden data by using etags + as an oracle to identify when the right data has been + guessed. (Joseph Myers, John Rouillard) 2018-07-13 1.6.0
--- a/doc/upgrading.txt Wed May 22 20:56:59 2019 -0400 +++ b/doc/upgrading.txt Thu May 23 18:56:57 2019 -0400 @@ -23,6 +23,18 @@ Migrating from 1.6.0 to x.y.0 ============================= +Upgrade tracker's config.ini file +-------------------------------------- +Once you have installed the new roundup, use: + + roundup-admin -i /path/to/tracker updateconfig new_init_file.ini + +to generate a new ini file preserving all your settings. You can then +merge any local comments from the tracker's ``config.ini`` into +``new_init_file.ini``. Compare the old and new files and configure set +any new settings as you want. Then replace ``config.ini`` with the new +init file. + PGP mail processing -------------------
--- a/roundup/configuration.py Wed May 22 20:56:59 2019 -0400 +++ b/roundup/configuration.py Thu May 23 18:56:57 2019 -0400 @@ -23,6 +23,10 @@ import roundup.date +from roundup.anypy.strings import b2s +import roundup.anypy.random_ as random_ +import binascii + # XXX i don't think this module needs string translation, does it? ### Exceptions @@ -95,6 +99,9 @@ NODEFAULT = UnsetDefaultValue() +def create_token(): + return b2s(binascii.b2a_base64(random_.token_bytes(32)).strip()) + ### Option classes class Option: @@ -467,6 +474,14 @@ def _value2str(self, value): return oct(value) +class MandatoryOption(Option): + """Option must not be empty""" + def str2value(self, value): + if not value: + raise OptionValueError(self,value,"Value must not be empty.") + else: + return value + class NullableOption(Option): """Option that is set to None if its string value is one of NULL strings @@ -851,6 +866,18 @@ "Setting this option makes Roundup migrate passwords with\n" "an insecure password-scheme to a more secure scheme\n" "when the user logs in via the web-interface."), + (MandatoryOption, "secret_key", create_token(), + "A per tracker secret used in etag calculations for\n" + "an object. It must not be empty.\n" + "It prevents reverse engineering hidden data in an object\n" + "by calculating the etag for a sample object. Then modifying\n" + "hidden properties until the sample object's etag matches\n" + "the one returned by roundup.\n" + "Changing this changes the etag and invalidates updates by\n" + "clients. It must be persistent across application restarts.\n" + "(Note the default value changes every time\n" + " roundup-admin updateconfig\n" + "is run, so it must be explicitly set to a non-empty string.\n"), )), ("rdbms", ( (Option, 'name', 'roundup',
--- a/roundup/rest.py Wed May 22 20:56:59 2019 -0400 +++ b/roundup/rest.py Thu May 23 18:56:57 2019 -0400 @@ -37,7 +37,7 @@ from roundup.exceptions import * from roundup.cgi.exceptions import * -from hashlib import md5 +import hmac # Py3 compatible basestring try: @@ -118,7 +118,7 @@ return result return format_object -def calculate_etag (node, classname="Missing", id="0"): +def calculate_etag (node, key, classname="Missing", id="0"): '''given a hyperdb node generate a hashed representation of it to be used as an etag. @@ -142,13 +142,13 @@ ''' items = node.items(protected=True) # include every item - etag = md5(bs2b(repr(sorted(items)))).hexdigest() + etag = hmac.new(bs2b(repr(sorted(items)))).hexdigest() logger.debug("object=%s%s; tag=%s; repr=%s", classname, id, etag, repr(node.items(protected=True))) # Quotes are part of ETag spec, normal headers don't have quotes return '"%s"' % etag -def check_etag (node, etags, classname="Missing", id="0"): +def check_etag (node, key, etags, classname="Missing", id="0"): '''Take a list of etags and compare to the etag for the given node. Iterate over all supplied etags, @@ -159,7 +159,7 @@ ''' have_etag_match=False - node_etag = calculate_etag(node, classname, id) + node_etag = calculate_etag(node, key, classname, id) for etag in etags: if etag is not None: @@ -511,6 +511,7 @@ def raise_if_no_etag(self, class_name, item_id, input): class_obj = self.db.getclass(class_name) if not check_etag(class_obj.getnode(item_id), + self.db.config.WEB_SECRET_KEY, obtain_etags(self.client.request.headers, input), class_name, item_id): @@ -787,7 +788,8 @@ ) node = class_obj.getnode(itemid) - etag = calculate_etag(node, class_name, itemid) + etag = calculate_etag(node, self.db.config.WEB_SECRET_KEY, + class_name, itemid) props = None protected=False verbose=1 @@ -868,7 +870,8 @@ class_obj = self.db.getclass(class_name) node = class_obj.getnode(item_id) - etag = calculate_etag(node, class_name, item_id) + etag = calculate_etag(node, self.db.config.WEB_SECRET_KEY, + class_name, item_id) data = node.__getattr__(attr_name) result = { 'id': item_id,
--- a/test/rest_common.py Wed May 22 20:56:59 2019 -0400 +++ b/test/rest_common.py Thu May 23 18:56:57 2019 -0400 @@ -635,13 +635,13 @@ ) node = self.db.user.getnode(self.joeid) - etag = calculate_etag(node) + etag = calculate_etag(node, "zysjskakss") items = node.items(protected=True) # include every item print(repr(items)) print(etag) self.assertEqual(etag, "6adf97f83acf6453d4a6a4b1070f3754") - etag = calculate_etag(self.db.issue.getnode("1")) + etag = calculate_etag(self.db.issue.getnode("1"), "zysjskakss") print(etag) self.assertEqual(etag, "6adf97f83acf6453d4a6a4b1070f3754") @@ -667,7 +667,8 @@ pass form = cgi.FieldStorage() - etag = calculate_etag(self.db.user.getnode(self.joeid)) + etag = calculate_etag(self.db.user.getnode(self.joeid), + "zysjskakss") form.list = [ cgi.MiniFieldStorage('data', 'Joe Doe Doe'), ] @@ -775,7 +776,7 @@ # PUT: joe's 'realname' using json data. # simulate: /rest/data/user/<id>/realname # use etag in header - etag = calculate_etag(self.db.user.getnode(self.joeid)) + etag = calculate_etag(self.db.user.getnode(self.joeid), "zysjskakss") body=b'{ "data": "Joe Doe 1" }' env = { "CONTENT_TYPE": "application/json", "CONTENT_LENGTH": len(body), @@ -824,7 +825,7 @@ # Set joe's 'realname' using json data. # simulate: /rest/data/user/<id>/realname # use etag in payload - etag = calculate_etag(self.db.user.getnode(self.joeid)) + etag = calculate_etag(self.db.user.getnode(self.joeid), "zysjskakss") etagb = etag.strip ('"') body=s2b('{ "@etag": "\\"%s\\"", "data": "Joe Doe 2" }'%etagb) env = { "CONTENT_TYPE": "application/json", @@ -863,7 +864,7 @@ # # Also use GET on the uri via the dispatch to retrieve # the results from the db. - etag = calculate_etag(self.db.user.getnode(self.joeid)) + etag = calculate_etag(self.db.user.getnode(self.joeid), "zysjskakss") headers={"if-match": etag, "accept": "application/vnd.json.test-v1+json", } @@ -901,7 +902,7 @@ self.empty_form) self.assertEqual(self.dummy_client.response_code, 200) - etag = calculate_etag(self.db.user.getnode(self.joeid)) + etag = calculate_etag(self.db.user.getnode(self.joeid), "zysjskakss") etagb = etag.strip ('"') body=s2b('{ "address": "demo2@example.com", "@etag": "\\"%s\\""}'%etagb) env = { "CONTENT_TYPE": "application/json", @@ -929,7 +930,7 @@ 'demo2@example.com') # and set it back reusing env and headers from last test - etag = calculate_etag(self.db.user.getnode(self.joeid)) + etag = calculate_etag(self.db.user.getnode(self.joeid), "zysjskakss") etagb = etag.strip ('"') body=s2b('{ "address": "%s", "@etag": "\\"%s\\""}'%( stored_results['data']['attributes']['address'], @@ -1049,7 +1050,7 @@ # TEST #8 # DELETE: delete issue 1 - etag = calculate_etag(self.db.issue.getnode("1")) + etag = calculate_etag(self.db.issue.getnode("1"), "zysjskakss") etagb = etag.strip ('"') env = {"CONTENT_TYPE": "application/json", "CONTENT_LEN": 0, @@ -1350,7 +1351,7 @@ # change Joe's realname via attribute uri - etag in header form = cgi.FieldStorage() - etag = calculate_etag(self.db.user.getnode(self.joeid)) + etag = calculate_etag(self.db.user.getnode(self.joeid), "zysjskakss") form.list = [ cgi.MiniFieldStorage('data', 'Joe Doe Doe'), ] @@ -1373,7 +1374,7 @@ # with all fields, change one field and put the result without # having to filter out protected items. form = cgi.FieldStorage() - etag = calculate_etag(self.db.user.getnode(self.joeid)) + etag = calculate_etag(self.db.user.getnode(self.joeid), "zysjskakss") form.list = [ cgi.MiniFieldStorage('creator', '3'), cgi.MiniFieldStorage('realname', 'Joe Doe'), @@ -1394,7 +1395,7 @@ # This should result in no change to the name and # a 400 UsageError stating prop does not exist. form = cgi.FieldStorage() - etag = calculate_etag(self.db.user.getnode(self.joeid)) + etag = calculate_etag(self.db.user.getnode(self.joeid), "zysjskakss") form.list = [ cgi.MiniFieldStorage('JustKidding', '3'), cgi.MiniFieldStorage('realname', 'Joe Doe'), @@ -1418,7 +1419,7 @@ # make sure we don't have permission issues self.db.setCurrentUser('admin') form = cgi.FieldStorage() - etag = calculate_etag(self.db.user.getnode(self.joeid)) + etag = calculate_etag(self.db.user.getnode(self.joeid), "zysjskakss") form.list = [ cgi.MiniFieldStorage('data', '3'), cgi.MiniFieldStorage('@etag', etag) @@ -1440,7 +1441,7 @@ # make sure we don't have permission issues self.db.setCurrentUser('admin') form = cgi.FieldStorage() - etag = calculate_etag(self.db.user.getnode(self.joeid)) + etag = calculate_etag(self.db.user.getnode(self.joeid), "zysjskakss") form.list = [ cgi.MiniFieldStorage('data', '3'), cgi.MiniFieldStorage('@etag', etag) @@ -1582,7 +1583,7 @@ [{'id': '1', 'link': self.url_pfx + 'user/1'}]) form = cgi.FieldStorage() - etag = calculate_etag(self.db.issue.getnode(issue_id)) + etag = calculate_etag(self.db.issue.getnode(issue_id), "zysjskakss") form.list.append(cgi.MiniFieldStorage('@etag', etag)) # remove the title and nosy results = self.server.delete_attribute( @@ -1591,7 +1592,7 @@ self.assertEqual(self.dummy_client.response_code, 200) del(form.list[-1]) - etag = calculate_etag(self.db.issue.getnode(issue_id)) + etag = calculate_etag(self.db.issue.getnode(issue_id), "zysjskakss") form.list.append(cgi.MiniFieldStorage('@etag', etag)) results = self.server.delete_attribute( 'issue', issue_id, 'nosy', form @@ -1607,7 +1608,7 @@ self.assertEqual(results['attributes']['title'], None) # delete protected property - etag = calculate_etag(self.db.issue.getnode(issue_id)) + etag = calculate_etag(self.db.issue.getnode(issue_id), "zysjskakss") form.list.append(cgi.MiniFieldStorage('@etag', etag)) results = self.server.delete_attribute( 'issue', issue_id, 'creator', form @@ -1624,7 +1625,7 @@ self.assertEqual(self.dummy_client.response_code, 405) # delete required property - etag = calculate_etag(self.db.issue.getnode(issue_id)) + etag = calculate_etag(self.db.issue.getnode(issue_id), "zysjskakss") form.list.append(cgi.MiniFieldStorage('@etag', etag)) results = self.server.delete_attribute( 'issue', issue_id, 'requireme', form @@ -1642,7 +1643,7 @@ self.assertEqual(self.dummy_client.response_code, 400) # delete bogus property - etag = calculate_etag(self.db.issue.getnode(issue_id)) + etag = calculate_etag(self.db.issue.getnode(issue_id), "zysjskakss") form.list.append(cgi.MiniFieldStorage('@etag', etag)) results = self.server.delete_attribute( 'issue', issue_id, 'nosuchprop', form @@ -1676,7 +1677,7 @@ results = self.server.patch_element('issue', issue_id, form) self.assertEqual(self.dummy_client.response_code, 412) - etag = calculate_etag(self.db.issue.getnode(issue_id)) + etag = calculate_etag(self.db.issue.getnode(issue_id), "zysjskakss") form = cgi.FieldStorage() form.list = [ cgi.MiniFieldStorage('@op', 'add'), @@ -1693,7 +1694,7 @@ self.assertEqual(len(results['attributes']['nosy']), 2) self.assertListEqual(results['attributes']['nosy'], ['1', '2']) - etag = calculate_etag(self.db.issue.getnode(issue_id)) + etag = calculate_etag(self.db.issue.getnode(issue_id), "zysjskakss") form = cgi.FieldStorage() form.list = [ cgi.MiniFieldStorage('@op', 'add'), @@ -1712,7 +1713,7 @@ # patch invalid property - etag = calculate_etag(self.db.issue.getnode(issue_id)) + etag = calculate_etag(self.db.issue.getnode(issue_id), "zysjskakss") form = cgi.FieldStorage() form.list = [ cgi.MiniFieldStorage('@op', 'add'), @@ -1758,7 +1759,7 @@ self.assertListEqual(results['attributes']['nosy'], ['1']) # replace userid 2 to the nosy list and status = 3 - etag = calculate_etag(self.db.issue.getnode(issue_id)) + etag = calculate_etag(self.db.issue.getnode(issue_id), "zysjskakss") form = cgi.FieldStorage() form.list = [ cgi.MiniFieldStorage('@op', 'replace'), @@ -1777,7 +1778,7 @@ self.assertListEqual(results['attributes']['nosy'], ['2']) # replace status = 2 using status attribute - etag = calculate_etag(self.db.issue.getnode(issue_id)) + etag = calculate_etag(self.db.issue.getnode(issue_id), "zysjskakss") form = cgi.FieldStorage() form.list = [ cgi.MiniFieldStorage('@op', 'replace'), @@ -1794,7 +1795,7 @@ self.assertEqual(results['attributes']['status'], '2') # try to set a protected prop. It should fail. - etag = calculate_etag(self.db.issue.getnode(issue_id)) + etag = calculate_etag(self.db.issue.getnode(issue_id), "zysjskakss") form = cgi.FieldStorage() form.list = [ cgi.MiniFieldStorage('@op', 'replace'), @@ -1815,7 +1816,7 @@ # try to set a protected prop using patch_attribute. It should # fail with a 405 bad/unsupported method. - etag = calculate_etag(self.db.issue.getnode(issue_id)) + etag = calculate_etag(self.db.issue.getnode(issue_id), "zysjskakss") form = cgi.FieldStorage() form.list = [ cgi.MiniFieldStorage('@op', 'replace'), @@ -1861,7 +1862,7 @@ # remove the nosy list and the title form = cgi.FieldStorage() - etag = calculate_etag(self.db.issue.getnode(issue_id)) + etag = calculate_etag(self.db.issue.getnode(issue_id), "zysjskakss") form.list = [ cgi.MiniFieldStorage('@op', 'remove'), cgi.MiniFieldStorage('nosy', ''), @@ -1880,7 +1881,7 @@ self.assertEqual(results['attributes']['nosy'], []) # try to remove a protected prop. It should fail. - etag = calculate_etag(self.db.issue.getnode(issue_id)) + etag = calculate_etag(self.db.issue.getnode(issue_id), "zysjskakss") form = cgi.FieldStorage() form.list = [ cgi.MiniFieldStorage('@op', 'remove'), @@ -1900,7 +1901,7 @@ self.assertEqual(self.dummy_client.response_code, 400) # try to remove a required prop. it should fail - etag = calculate_etag(self.db.issue.getnode(issue_id)) + etag = calculate_etag(self.db.issue.getnode(issue_id), "zysjskakss") form.list = [ cgi.MiniFieldStorage('@op', 'remove'), cgi.MiniFieldStorage('requireme', ''), @@ -1939,7 +1940,7 @@ # execute action retire form = cgi.FieldStorage() - etag = calculate_etag(self.db.issue.getnode(issue_id)) + etag = calculate_etag(self.db.issue.getnode(issue_id), "zysjskakss") form.list = [ cgi.MiniFieldStorage('@op', 'action'), cgi.MiniFieldStorage('@action_name', 'retire'), @@ -1975,7 +1976,7 @@ # remove the nosy list and the title form = cgi.FieldStorage() - etag = calculate_etag(self.db.issue.getnode(issue_id)) + etag = calculate_etag(self.db.issue.getnode(issue_id), "zysjskakss") form.list = [ cgi.MiniFieldStorage('@op', 'remove'), cgi.MiniFieldStorage('nosy', '1, 2'),