Mercurial > p > roundup > code
changeset 4781:6e9b9743de89
Implementation for:
http://issues.roundup-tracker.org/issue2550731
Add mechanism for the detectors to be able to tell the source of the
data changes.
Support for tx_Source property on database handle. Can be
used by detectors to find out the source of a change in an auditor to
block changes arriving by unauthenticated mechanisms (e.g. plain email
where headers can be faked). The property db.tx_Source has the
following values:
* None - Default value set to None. May be valid if it's a script
that is created by the user. Otherwise it's an error and indicates
that some code path is not properly setting the tx_Source property.
* "cli" - this string value is set when using roundup-admin and
supplied scripts.
* "web" - this string value is set when using any web based
technique: html interface, xmlrpc ....
* "email" - this string value is set when using an unauthenticated
email based technique.
* "email-sig-openpgp" - this string value is set when email with a
valid pgp signature is used. (*NOTE* the testing for this mode
is incomplete. If you have a pgp infrastructure you should test
and verify that this is properly set.)
This also includes some (possibly incomplete) tests cases for the
modes above and an example of using ts_Source in the customization.txt
document.
| author | John Rouillard <rouilj@ieee.org> |
|---|---|
| date | Tue, 23 Apr 2013 23:06:09 -0400 |
| parents | 3adff0fb0207 |
| children | cda9ca8befd7 |
| files | CHANGES.txt doc/customizing.txt roundup/admin.py roundup/cgi/client.py roundup/instance.py roundup/mailgw.py scripts/add-issue scripts/copy-user.py scripts/spam-remover test/memorydb.py test/test_cgi.py test/test_mailgw.py test/test_userauditor.py test/test_xmlrpc.py test/tx_Source_detector.py tools/load_tracker.py tools/migrate-queries.py |
| diffstat | 17 files changed, 290 insertions(+), 5 deletions(-) [+] |
line wrap: on
line diff
--- a/CHANGES.txt Fri Mar 22 15:53:27 2013 +0100 +++ b/CHANGES.txt Tue Apr 23 23:06:09 2013 -0400 @@ -7,6 +7,24 @@ Features: +- Support for tx_Source property on database handle. Can be used by + detectors to find out the source of a change in an auditor to block + changes arriving by unauthenticated mechanisms (e.g. plain email + where headers can be faked). The property db.tx_Source has the + following values: + * None - Default value set to None. May be valid if it's a script + that is created by the user. Otherwise it's an error and indicates + that some code path is not properly setting the tx_Source property. + * "cli" - this string value is set when using roundup-admin and + supplied scripts. + * "web" - this string value is set when using any web based + technique: html interface, xmlrpc .... + * "email" - this string value is set when using an unauthenticated + email based technique. + * "email-sig-openpgp" - this string value is set when email with a + valid pgp signature is used. (*NOTE* the testing for this mode + is incomplete. If you have a pgp infrastructure you should test + and verify that this is properly set.) - Introducing Template Loader API (anatoly techtonik) - Experimental support for Jinja2, try 'jinja2' for template_engine in config (anatoly techtonik)
--- a/doc/customizing.txt Fri Mar 22 15:53:27 2013 +0100 +++ b/doc/customizing.txt Tue Apr 23 23:06:09 2013 -0400 @@ -4539,6 +4539,73 @@ selected these keywords as nosy keywords. This will eliminate the loop over all users. +Restricting updates that arrive by email +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Roundup supports multiple update methods: + +1. command line +2. plain email +3. pgp signed email +4. web access + +in some cases you may need to prevent changes to properties by some of +these methods. For example you can set up issues that are viewable +only by people on the nosy list. So you must prevent unauthenticated +changes to the nosy list. + +Since plain email can be easily forged, it does not provide sufficient +authentication in this senario. + +To prevent this we can add a detector that audits the source of the +transaction and rejects the update if it changes the nosy list. + +Create the detector (auditor) module and add it to the detectors +directory of your tracker:: + + from roundup import roundupdb, hyperdb + + from roundup.mailgw import Unauthorized + + def restrict_nosy_changes(db, cl, nodeid, newvalues): + '''Do not permit changes to nosy via email.''' + + if not (newvalues.has_key('nosy')): + # the nosy field has not changed so no need to check. + return + + if db.tx_Source in ['web', 'email-sig-openpgp', 'cli' ]: + # if the source of the transaction is from an authenticated + # source or a privileged process allow the transaction. + # Other possible sources: 'email' + return + + # otherwise raise an error + raise Unauthorized, \ + 'Changes to nosy property not allowed via %s for this issue.'%\ + tx_Source + + def init(db): + ''' Install restrict_nosy_changes to run after other auditors. + + Allow initial creation email to set nosy. + So don't execute: db.issue.audit('create', requestedbyauditor) + + Set priority to 110 to run this auditor after other auditors + that can cause nosy to change. + ''' + db.issue.audit('set', restrict_nosy_changes, 110) + +This detector (auditor) will prevent updates to the nosy field if it +arrives by email. Since it runs after other auditors (due to the +priority of 110), it will also prevent changes to the nosy field that +are done by other auditors if triggered by an email. + +Note that db.tx_Source was not present in roundup versions before +1.4.21, so you must be running a newer version to use this detector. +Read the CHANGES.txt document in the roundup source code for further +details on tx_Source. + Changes to Security and Permissions -----------------------------------
--- a/roundup/admin.py Fri Mar 22 15:53:27 2013 +0100 +++ b/roundup/admin.py Tue Apr 23 23:06:09 2013 -0400 @@ -1475,6 +1475,8 @@ if not self.db: self.db = tracker.open('admin') + self.db.tx_Source = 'cli' + # do the command ret = 0 try:
--- a/roundup/cgi/client.py Fri Mar 22 15:53:27 2013 +0100 +++ b/roundup/cgi/client.py Tue Apr 23 23:06:09 2013 -0400 @@ -792,12 +792,15 @@ # open the database or only set the user if not hasattr(self, 'db'): self.db = self.instance.open(username) + self.db.tx_Source = "web" else: if self.instance.optimize: self.db.setCurrentUser(username) + self.db.tx_Source = "web" else: self.db.close() self.db = self.instance.open(username) + self.db.tx_Source = "web" # The old session API refers to the closed database; # we can no longer use it. self.session_api = Session(self)
--- a/roundup/instance.py Fri Mar 22 15:53:27 2013 +0100 +++ b/roundup/instance.py Tue Apr 23 23:06:09 2013 -0400 @@ -121,6 +121,8 @@ extension(self) detectors = self.get_extensions('detectors') db = env['db'] + db.tx_Source = None + # apply the detectors for detector in detectors: detector(db)
--- a/roundup/mailgw.py Fri Mar 22 15:53:27 2013 +0100 +++ b/roundup/mailgw.py Tue Apr 23 23:06:09 2013 -0400 @@ -1010,6 +1010,9 @@ "be PGP encrypted.") if self.message.pgp_signed(): self.message.verify_signature(author_address) + # signature has been verified + self.db.tx_Source = "email-sig-openpgp" + elif self.message.pgp_encrypted(): # Replace message with the contents of the decrypted # message for content extraction @@ -1019,8 +1022,26 @@ encr_only = self.config.PGP_REQUIRE_INCOMING == 'encrypted' encr_only = encr_only or not pgp_role() self.crypt = True - self.message = self.message.decrypt(author_address, - may_be_unsigned = encr_only) + try: + # see if the message has a valid signature + message = self.message.decrypt(author_address, + may_be_unsigned = False) + # only set if MailUsageError is not raised + # indicating that we have a valid signature + self.db.tx_Source = "email-sig-openpgp" + except MailUsageError: + # if there is no signature or an error in the message + # we get here. Try decrypting it again if we don't + # need signatures. + if encr_only: + message = self.message.decrypt(author_address, + may_be_unsigned = encr_only) + else: + # something failed with the message decryption/sig + # chain. Pass the error up. + raise + # store the decrypted message + self.message = message elif pgp_role(): raise MailUsageError, _(""" This tracker has been configured to require all email be PGP signed or @@ -1537,6 +1558,9 @@ ''' # get database handle for handling one email self.db = self.instance.open ('admin') + + self.db.tx_Source = "email" + try: return self._handle_message(message) finally:
--- a/scripts/add-issue Fri Mar 22 15:53:27 2013 +0100 +++ b/scripts/add-issue Tue Apr 23 23:06:09 2013 -0400 @@ -28,6 +28,7 @@ # open the tracker tracker = instance.open(tracker_home) db = tracker.open('admin') +db.tx_Source = "cli" uid = db.user.lookup('admin') try: # try to open the tracker as the current user
--- a/scripts/copy-user.py Fri Mar 22 15:53:27 2013 +0100 +++ b/scripts/copy-user.py Tue Apr 23 23:06:09 2013 -0400 @@ -42,6 +42,9 @@ db1 = instance1.open('admin') db2 = instance2.open('admin') + db1.tx_Source = "cli" + db2.tx_Source = "cli" + userlist = db1.user.list() for userid in userids: try:
--- a/scripts/spam-remover Fri Mar 22 15:53:27 2013 +0100 +++ b/scripts/spam-remover Tue Apr 23 23:06:09 2013 -0400 @@ -93,6 +93,8 @@ cmd.show_help() tracker = instance.open(opt.instance) db = tracker.open('admin') + db.tx_Source = "cli" + users = dict.fromkeys (db.user.lookup(u) for u in opt.users) files_to_remove = {} for fn in opt.filenames:
--- a/test/memorydb.py Fri Mar 22 15:53:27 2013 +0100 +++ b/test/memorydb.py Tue Apr 23 23:06:09 2013 -0400 @@ -50,6 +50,10 @@ execfile(os.path.join(dirname, fn), vars) vars['init'](db) + vars = {} + execfile("test/tx_Source_detector.py", vars) + vars['init'](db) + ''' status = Class(db, "status", name=String()) status.setkey("name") @@ -194,6 +198,7 @@ self.newnodes = {} # keep track of the new nodes by class self.destroyednodes = {}# keep track of the destroyed nodes by class self.transactions = [] + self.tx_Source = None def filename(self, classname, nodeid, property=None, create=0): shutil.copyfile(__file__, __file__+'.dummy')
--- a/test/test_cgi.py Fri Mar 22 15:53:27 2013 +0100 +++ b/test/test_cgi.py Tue Apr 23 23:06:09 2013 -0400 @@ -75,11 +75,24 @@ # open the database self.db = self.instance.open('admin') + self.db.tx_Source = "web" self.db.user.create(username='Chef', address='chef@bork.bork.bork', realname='Bork, Chef', roles='User') self.db.user.create(username='mary', address='mary@test.test', roles='User', realname='Contrary, Mary') + self.db.issue.addprop(tx_Source=hyperdb.String()) + self.db.msg.addprop(tx_Source=hyperdb.String()) + + self.db.post_init() + + vars = dict(globals()) + vars['db'] = self.db + vars = {} + execfile("test/tx_Source_detector.py", vars) + vars['init'](self.db) + + test = self.instance.backend.Class(self.db, "test", string=hyperdb.String(), number=hyperdb.Number(), boolean=hyperdb.Boolean(), link=hyperdb.Link('test'), @@ -207,6 +220,7 @@ self.assertEqual(self.db.issue.get(issue,'status'),'1') self.assertEqual(self.db.status.lookup('1'),'2') self.assertEqual(self.db.status.lookup('2'),'1') + self.assertEqual(self.db.issue.get('1','tx_Source'),'web') form = cgi.FieldStorage() cl = client.Client(self.instance, None, {'PATH_INFO':'/'}, form) cl.classname = 'issue' @@ -226,6 +240,7 @@ self.assertEqual(self.db.issue.get(issue,'keyword'),['1']) self.assertEqual(self.db.keyword.lookup('1'),'2') self.assertEqual(self.db.keyword.lookup('2'),'1') + self.assertEqual(self.db.issue.get(issue,'tx_Source'),'web') form = cgi.FieldStorage() cl = client.Client(self.instance, None, {'PATH_INFO':'/'}, form) cl.classname = 'issue' @@ -271,11 +286,13 @@ nodeid = self.db.issue.create(status='unread') self.assertEqual(self.parseForm({'status': 'unread'}, 'issue', nodeid), ({('issue', nodeid): {}}, [])) + self.assertEqual(self.db.issue.get(nodeid,'tx_Source'),'web') def testUnsetLink(self): nodeid = self.db.issue.create(status='unread') self.assertEqual(self.parseForm({'status': '-1'}, 'issue', nodeid), ({('issue', nodeid): {'status': None}}, [])) + self.assertEqual(self.db.issue.get(nodeid,'tx_Source'),'web') def testInvalidLinkValue(self): # XXX This is not the current behaviour - should we enforce this?
--- a/test/test_mailgw.py Fri Mar 22 15:53:27 2013 +0100 +++ b/test/test_mailgw.py Tue Apr 23 23:06:09 2013 -0400 @@ -129,6 +129,8 @@ return res +from roundup.hyperdb import String + class MailgwTestAbstractBase(unittest.TestCase, DiffHelper): count = 0 schema = 'classic' @@ -139,6 +141,13 @@ # and open the database / "instance" self.db = memorydb.create('admin') + + self.db.issue.addprop(tx_Source=String()) + self.db.msg.addprop(tx_Source=String()) + self.db.post_init() + + self.db.tx_Source = "email" + self.instance = Tracker() self.instance.db = self.db self.instance.config = self.db.config @@ -206,6 +215,7 @@ ''') assert not os.path.exists(SENDMAILDEBUG) self.assertEqual(self.db.issue.get(nodeid, 'title'), 'Testing...') + self.assertEqual(self.db.issue.get(nodeid, 'tx_Source'), 'email') class MailgwTestCase(MailgwTestAbstractBase): @@ -320,6 +330,11 @@ l = self.db.issue.get(nodeid, 'nosy') l.sort() self.assertEqual(l, [self.chef_id, self.richard_id]) + + # check that the message has the right source code + l = self.db.msg.get('1', 'tx_Source') + self.assertEqual(l, 'email') + return nodeid def testNewIssue(self): @@ -413,6 +428,7 @@ nosy: Chef, mary, richard status: unread title: Testing... +tx_Source: email _______________________________________________________________________ Roundup issue tracker <issue_tracker@your.tracker.email.domain.example> @@ -455,6 +471,7 @@ nosy: Chef, mary, richard status: unread title: Testing... +tx_Source: email _______________________________________________________________________ Roundup issue tracker <issue_tracker@your.tracker.email.domain.example> @@ -499,6 +516,7 @@ nosy: Chef, mary, richard status: unread title: Testing... +tx_Source: email _______________________________________________________________________ Roundup issue tracker <issue_tracker@your.tracker.email.domain.example> @@ -1195,7 +1213,6 @@ X-Roundup-Issue-Status: chatting Content-Transfer-Encoding: quoted-printable - richard <richard@test.test> added the comment: This is a followup @@ -1230,6 +1247,10 @@ self.assertEqual(l, [self.chef_id, self.richard_id, self.mary_id, self.john_id]) + # check that the message has the right tx_Source + l = self.db.msg.get('2', 'tx_Source') + self.assertEqual(l, 'email') + self.compareMessages(self._get_mail(), '''FROM: roundup-admin@your.tracker.email.domain.example TO: chef@bork.bork.bork, john@test.test, mary@test.test @@ -1247,7 +1268,6 @@ X-Roundup-Issue-Status: chatting Content-Transfer-Encoding: quoted-printable - richard <richard@test.test> added the comment: This is a followup @@ -1265,6 +1285,7 @@ ''') def testNosyGeneration(self): + self.db.tx_Source = "email" self.db.issue.create(title='test') # create a nosy message @@ -1274,6 +1295,10 @@ l = self.db.issue.create(title='test', messages=[msg], nosy=[self.chef_id, self.mary_id, self.john_id]) + + # check that message has right tx_Source + self.assertEqual(self.db.msg.get('1', 'tx_Source'), 'email') + self.compareMessages(self._get_mail(), '''FROM: roundup-admin@your.tracker.email.domain.example TO: chef@bork.bork.bork, john@test.test, mary@test.test @@ -1300,6 +1325,7 @@ nosy: Chef, john, mary, richard status: unread title: test +tx_Source: email _______________________________________________________________________ Roundup issue tracker <issue_tracker@your.tracker.email.domain.example> @@ -1373,6 +1399,10 @@ This is a followup ''') + + l = self.db.msg.get('2', 'tx_Source') + self.assertEqual(l, 'email') + self.compareMessages(self._get_mail(), '''FROM: roundup-admin@your.tracker.email.domain.example TO: chef@bork.bork.bork, john@test.test, mary@test.test @@ -3390,6 +3420,10 @@ m = self.db.issue.get(nodeid, 'messages')[0] self.assertEqual(self.db.msg.get(m, 'content'), 'This is a pgp signed message.') + # check that the message has the right source code + l = self.db.msg.get(m, 'tx_Source') + self.assertEqual(l, 'email-sig-openpgp') + def testPGPSignedMessageFail(self): # require both, signing and encryption @@ -3441,6 +3475,9 @@ m = self.db.issue.get(nodeid, 'messages')[0] self.assertEqual(self.db.msg.get(m, 'content'), 'This is the text to be encrypted') + # check that the message has the right source code + l = self.db.msg.get(m, 'tx_Source') + self.assertEqual(l, 'email') def testPGPEncryptedUnsignedMessageFromNonPGPUser(self): msg = self.encrypted_msg.replace('John Doe <john@test.test>', @@ -3450,6 +3487,10 @@ self.assertEqual(self.db.msg.get(m, 'content'), 'This is the text to be encrypted') self.assertEqual(self.db.msg.get(m, 'author'), self.mary_id) + # check that the message has the right source code + l = self.db.msg.get(m, 'tx_Source') + self.assertEqual(l, 'email') + # check that a bounce-message that is triggered *after* # decrypting is properly encrypted: @@ -3533,7 +3574,9 @@ m = self.db.issue.get(nodeid, 'messages')[0] self.assertEqual(self.db.msg.get(m, 'content'), 'This is the text of a signed and encrypted email.') - + # check that the message has the right source code + l = self.db.msg.get(m, 'tx_Source') + self.assertEqual(l, 'email-sig-openpgp') def test_suite(): suite = unittest.TestSuite()
--- a/test/test_userauditor.py Fri Mar 22 15:53:27 2013 +0100 +++ b/test/test_userauditor.py Tue Apr 23 23:06:09 2013 -0400 @@ -6,6 +6,7 @@ self.dirname = '_test_user_auditor' self.instance = setupTracker(self.dirname) self.db = self.instance.open('admin') + self.db.tx_Source = "cli" try: import pytz
--- a/test/test_xmlrpc.py Fri Mar 22 15:53:27 2013 +0100 +++ b/test/test_xmlrpc.py Tue Apr 23 23:06:09 2013 -0400 @@ -10,6 +10,7 @@ from roundup import init, instance, password, hyperdb, date from roundup.xmlrpc import RoundupInstance from roundup.backends import list_backends +from roundup.hyperdb import String import db_test_base @@ -26,6 +27,8 @@ # open the database self.db = self.instance.open('admin') + + # Get user id (user4 maybe). Used later to get data from db. self.joeid = 'user' + self.db.user.create(username='joe', password=password.Password('random'), address='random@home.org', realname='Joe Random', roles='User') @@ -33,6 +36,20 @@ self.db.commit() self.db.close() self.db = self.instance.open('joe') + + self.db.tx_Source = 'web' + + self.db.issue.addprop(tx_Source=hyperdb.String()) + self.db.msg.addprop(tx_Source=hyperdb.String()) + + self.db.post_init() + + vars = dict(globals()) + vars['db'] = self.db + vars = {} + execfile("test/tx_Source_detector.py", vars) + vars['init'](self.db) + self.server = RoundupInstance(self.db, self.instance.actions, None) def tearDown(self): @@ -66,6 +83,7 @@ issueid = 'issue' + results results = self.server.display(issueid, 'title') self.assertEqual(results['title'], 'foo') + self.assertEqual(self.db.issue.get('1', "tx_Source"), 'web') def testFileCreate(self): results = self.server.create('file', 'content=hello\r\nthere') @@ -183,6 +201,12 @@ self.db.close() self.db = self.instance.open('chef') + self.db.tx_Source = 'web' + + self.db.issue.addprop(tx_Source=hyperdb.String()) + self.db.msg.addprop(tx_Source=hyperdb.String()) + self.db.post_init() + self.server = RoundupInstance(self.db, self.instance.actions, None) # Filter on keyword works for role 'Project':
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/tx_Source_detector.py Tue Apr 23 23:06:09 2013 -0400 @@ -0,0 +1,71 @@ +# +# Example output when the web interface changes item 3 and the email +# (non pgp) interface changes item 4: +# +# tx_SourceCheckAudit(3) pre db.tx_Source: cgi +# tx_SourceCheckAudit(4) pre db.tx_Source: email +# tx_SourceCheckAudit(3) post db.tx_Source: cgi +# tx_SourceCheckAudit(4) post db.tx_Source: email +# tx_SourceCheckReact(4) pre db.tx_Source: email +# tx_SourceCheckReact(4) post db.tx_Source: email +# tx_SourceCheckReact(3) pre db.tx_Source: cgi +# tx_SourceCheckReact(3) post db.tx_Source: cgi +# +# Note that the calls are interleaved, but the proper +# tx_Source is associated with the same ticket. + +import time as time + +def tx_SourceCheckAudit(db, cl, nodeid, newvalues): + ''' An auditor to print the value of the source of the + transaction that trigger this change. The sleep call + is used to delay the transaction so that multiple changes will + overlap. The expected output from this detector are 2 lines + with the same value for tx_Source. Tx source is: + None - Reported when using a script or it is an error if + the change arrives by another method. + "cli" - reported when using roundup-admin + "web" - reported when using any web based technique + "email" - reported when using an unautheticated email based technique + "email-sig-openpgp" - reported when email with a valid pgp + signature is used + ''' + if __debug__ and False: + print "\n tx_SourceCheckAudit(%s) db.tx_Source: %s"%(nodeid, db.tx_Source) + + newvalues['tx_Source'] = db.tx_Source + + # example use for real to prevent a change from happening if it's + # submited via email + # + # if db.tx_Source == "email": + # raise Reject, 'Change not allowed via email' + +def tx_SourceCheckReact(db, cl, nodeid, oldvalues): + ''' An reactor to print the value of the source of the + transaction that trigger this change. The sleep call + is used to delay the transaction so that multiple changes will + overlap. The expected output from this detector are 2 lines + with the same value for tx_Source. Tx source is: + None - Reported when using a script or it is an error if + the change arrives by another method. + "cli" - reported when using roundup-admin + "web" - reported when using any web based technique + "email" - reported when using an unautheticated email based technique + "email-sig-openpgp" - reported when email with a valid pgp + signature is used + ''' + + if __debug__ and False: + print " tx_SourceCheckReact(%s) db.tx_Source: %s"%(nodeid, db.tx_Source) + + + +def init(db): + db.issue.audit('create', tx_SourceCheckAudit) + db.issue.audit('set', tx_SourceCheckAudit) + + db.issue.react('set', tx_SourceCheckReact) + db.issue.react('create', tx_SourceCheckReact) + + db.msg.audit('create', tx_SourceCheckAudit)
--- a/tools/load_tracker.py Fri Mar 22 15:53:27 2013 +0100 +++ b/tools/load_tracker.py Tue Apr 23 23:06:09 2013 -0400 @@ -19,6 +19,7 @@ # open the tracker tracker = instance.open(tracker_home) db = tracker.open('admin') +db.tx_Source = "cli" priorities = db.priority.list() statuses = db.status.list()