Skip to content

Commit aea3c2e

Browse files
committed
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.
1 parent 06d2b0b commit aea3c2e

17 files changed

+290
-5
lines changed

CHANGES.txt

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,24 @@ Entries without name were done by Richard Jones.
77

88
Features:
99

10+
- Support for tx_Source property on database handle. Can be used by
11+
detectors to find out the source of a change in an auditor to block
12+
changes arriving by unauthenticated mechanisms (e.g. plain email
13+
where headers can be faked). The property db.tx_Source has the
14+
following values:
15+
* None - Default value set to None. May be valid if it's a script
16+
that is created by the user. Otherwise it's an error and indicates
17+
that some code path is not properly setting the tx_Source property.
18+
* "cli" - this string value is set when using roundup-admin and
19+
supplied scripts.
20+
* "web" - this string value is set when using any web based
21+
technique: html interface, xmlrpc ....
22+
* "email" - this string value is set when using an unauthenticated
23+
email based technique.
24+
* "email-sig-openpgp" - this string value is set when email with a
25+
valid pgp signature is used. (*NOTE* the testing for this mode
26+
is incomplete. If you have a pgp infrastructure you should test
27+
and verify that this is properly set.)
1028
- Introducing Template Loader API (anatoly techtonik)
1129
- Experimental support for Jinja2, try 'jinja2' for template_engine
1230
in config (anatoly techtonik)

doc/customizing.txt

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4539,6 +4539,73 @@ Scalability
45394539
selected these keywords as nosy keywords. This will eliminate the
45404540
loop over all users.
45414541

4542+
Restricting updates that arrive by email
4543+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4544+
4545+
Roundup supports multiple update methods:
4546+
4547+
1. command line
4548+
2. plain email
4549+
3. pgp signed email
4550+
4. web access
4551+
4552+
in some cases you may need to prevent changes to properties by some of
4553+
these methods. For example you can set up issues that are viewable
4554+
only by people on the nosy list. So you must prevent unauthenticated
4555+
changes to the nosy list.
4556+
4557+
Since plain email can be easily forged, it does not provide sufficient
4558+
authentication in this senario.
4559+
4560+
To prevent this we can add a detector that audits the source of the
4561+
transaction and rejects the update if it changes the nosy list.
4562+
4563+
Create the detector (auditor) module and add it to the detectors
4564+
directory of your tracker::
4565+
4566+
from roundup import roundupdb, hyperdb
4567+
4568+
from roundup.mailgw import Unauthorized
4569+
4570+
def restrict_nosy_changes(db, cl, nodeid, newvalues):
4571+
'''Do not permit changes to nosy via email.'''
4572+
4573+
if not (newvalues.has_key('nosy')):
4574+
# the nosy field has not changed so no need to check.
4575+
return
4576+
4577+
if db.tx_Source in ['web', 'email-sig-openpgp', 'cli' ]:
4578+
# if the source of the transaction is from an authenticated
4579+
# source or a privileged process allow the transaction.
4580+
# Other possible sources: 'email'
4581+
return
4582+
4583+
# otherwise raise an error
4584+
raise Unauthorized, \
4585+
'Changes to nosy property not allowed via %s for this issue.'%\
4586+
tx_Source
4587+
4588+
def init(db):
4589+
''' Install restrict_nosy_changes to run after other auditors.
4590+
4591+
Allow initial creation email to set nosy.
4592+
So don't execute: db.issue.audit('create', requestedbyauditor)
4593+
4594+
Set priority to 110 to run this auditor after other auditors
4595+
that can cause nosy to change.
4596+
'''
4597+
db.issue.audit('set', restrict_nosy_changes, 110)
4598+
4599+
This detector (auditor) will prevent updates to the nosy field if it
4600+
arrives by email. Since it runs after other auditors (due to the
4601+
priority of 110), it will also prevent changes to the nosy field that
4602+
are done by other auditors if triggered by an email.
4603+
4604+
Note that db.tx_Source was not present in roundup versions before
4605+
1.4.21, so you must be running a newer version to use this detector.
4606+
Read the CHANGES.txt document in the roundup source code for further
4607+
details on tx_Source.
4608+
45424609
Changes to Security and Permissions
45434610
-----------------------------------
45444611

roundup/admin.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1475,6 +1475,8 @@ def run_command(self, args):
14751475
if not self.db:
14761476
self.db = tracker.open('admin')
14771477

1478+
self.db.tx_Source = 'cli'
1479+
14781480
# do the command
14791481
ret = 0
14801482
try:

roundup/cgi/client.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -792,12 +792,15 @@ def opendb(self, username):
792792
# open the database or only set the user
793793
if not hasattr(self, 'db'):
794794
self.db = self.instance.open(username)
795+
self.db.tx_Source = "web"
795796
else:
796797
if self.instance.optimize:
797798
self.db.setCurrentUser(username)
799+
self.db.tx_Source = "web"
798800
else:
799801
self.db.close()
800802
self.db = self.instance.open(username)
803+
self.db.tx_Source = "web"
801804
# The old session API refers to the closed database;
802805
# we can no longer use it.
803806
self.session_api = Session(self)

roundup/instance.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,8 @@ def open(self, name=None):
121121
extension(self)
122122
detectors = self.get_extensions('detectors')
123123
db = env['db']
124+
db.tx_Source = None
125+
124126
# apply the detectors
125127
for detector in detectors:
126128
detector(db)

roundup/mailgw.py

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1010,6 +1010,9 @@ def pgp_role():
10101010
"be PGP encrypted.")
10111011
if self.message.pgp_signed():
10121012
self.message.verify_signature(author_address)
1013+
# signature has been verified
1014+
self.db.tx_Source = "email-sig-openpgp"
1015+
10131016
elif self.message.pgp_encrypted():
10141017
# Replace message with the contents of the decrypted
10151018
# message for content extraction
@@ -1019,8 +1022,26 @@ def pgp_role():
10191022
encr_only = self.config.PGP_REQUIRE_INCOMING == 'encrypted'
10201023
encr_only = encr_only or not pgp_role()
10211024
self.crypt = True
1022-
self.message = self.message.decrypt(author_address,
1023-
may_be_unsigned = encr_only)
1025+
try:
1026+
# see if the message has a valid signature
1027+
message = self.message.decrypt(author_address,
1028+
may_be_unsigned = False)
1029+
# only set if MailUsageError is not raised
1030+
# indicating that we have a valid signature
1031+
self.db.tx_Source = "email-sig-openpgp"
1032+
except MailUsageError:
1033+
# if there is no signature or an error in the message
1034+
# we get here. Try decrypting it again if we don't
1035+
# need signatures.
1036+
if encr_only:
1037+
message = self.message.decrypt(author_address,
1038+
may_be_unsigned = encr_only)
1039+
else:
1040+
# something failed with the message decryption/sig
1041+
# chain. Pass the error up.
1042+
raise
1043+
# store the decrypted message
1044+
self.message = message
10241045
elif pgp_role():
10251046
raise MailUsageError, _("""
10261047
This tracker has been configured to require all email be PGP signed or
@@ -1537,6 +1558,9 @@ def handle_message(self, message):
15371558
'''
15381559
# get database handle for handling one email
15391560
self.db = self.instance.open ('admin')
1561+
1562+
self.db.tx_Source = "email"
1563+
15401564
try:
15411565
return self._handle_message(message)
15421566
finally:

scripts/add-issue

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ message_text = sys.stdin.read().strip()
2828
# open the tracker
2929
tracker = instance.open(tracker_home)
3030
db = tracker.open('admin')
31+
db.tx_Source = "cli"
3132
uid = db.user.lookup('admin')
3233
try:
3334
# try to open the tracker as the current user

scripts/copy-user.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ def copy_user(home1, home2, *userids):
4242
db1 = instance1.open('admin')
4343
db2 = instance2.open('admin')
4444

45+
db1.tx_Source = "cli"
46+
db2.tx_Source = "cli"
47+
4548
userlist = db1.user.list()
4649
for userid in userids:
4750
try:

scripts/spam-remover

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,8 @@ def main():
9393
cmd.show_help()
9494
tracker = instance.open(opt.instance)
9595
db = tracker.open('admin')
96+
db.tx_Source = "cli"
97+
9698
users = dict.fromkeys (db.user.lookup(u) for u in opt.users)
9799
files_to_remove = {}
98100
for fn in opt.filenames:

test/memorydb.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ def create(journaltag, create=True, debug=False):
5050
execfile(os.path.join(dirname, fn), vars)
5151
vars['init'](db)
5252

53+
vars = {}
54+
execfile("test/tx_Source_detector.py", vars)
55+
vars['init'](db)
56+
5357
'''
5458
status = Class(db, "status", name=String())
5559
status.setkey("name")
@@ -194,6 +198,7 @@ def __init__(self, config, journaltag=None):
194198
self.newnodes = {} # keep track of the new nodes by class
195199
self.destroyednodes = {}# keep track of the destroyed nodes by class
196200
self.transactions = []
201+
self.tx_Source = None
197202

198203
def filename(self, classname, nodeid, property=None, create=0):
199204
shutil.copyfile(__file__, __file__+'.dummy')

0 commit comments

Comments
 (0)