Re: Get all issues with attached files

On Fri, Oct 3, 2014 at 12:09 PM, Ralf Schlatterbeck <[email protected]> wrote:
> On Thu, Oct 02, 2014 at 10:33:02AM +0300, anatoly techtonik wrote:
>>
>> The interface is broken for my case:
>>
>> >>> import xmlrpclib
>> >>> z = xmlrpclib.ServerProxy('http://bugs.python.org/xmlrpc', allow_none=True)
>> >>> from pprint import pprint as pp
>> >>> pp(z.display('issue9643', 'files'))
>> {'files': []}
>> >>> pp(z.filter('issue', ['9643'], {'files':-1}))
>> ['9643']
>> >>> pp(z.filter('issue', ['9643'], {'files':[]}))
>> ['9643']
>> >>> pp(z.filter('issue', ['9643'], {'files':['888']}))
>> ['9643']
>> >>> pp(z.filter('issue', ['9643'], {'files':'888'}))
>> ['9643']
>>
>> There is definitely a `files` field which is empty, but no way to check that.
>> The Roundup is also suspiciously silent about if query is invalid.
>
> This looks to me as if you don't have search permission on issue.files.
>
> To recap: I've added Search permissions quite some time ago, otherwise a
> user could sort-of "triangulate" interesting properties which she would
> have no View permission but by searching could infer the interesting
> property.

I am very skeptical about this use case. Can you describe the real
attack or case you faced with?

I don't like "security by obscurity" design in open source security. It
complicates code, wastes people time and just postpones the bomb
effect. From my current point the limitation on search is a clear wart
in Roundup design - both the approach (permissions are not part of
schema) and implementation (errors pass silently, non optional
feature).

As a consequence of this design decision now I need to do extra
5000 queries or even more in a row to b.p.o tracker to find all issues
with files.

Security at the cost of simplicity, increased competence debt and
development time should be added very carefully and optionally.
I believe that with these params the total cost of solution exceeds
the potential damage that can be caused by running "search"
queries.

I can understand the performance reasons. In this case it is still
necessary to give message to users, so people can solve the problem
in a different way - add caching etc. Silently crippling the tracker to
prevent search overhead looks hackish (bad).

> The default for Search permission is View permission -- but only if the
> View permission is not tied to a check-function. My guess is that there
> is a check-function on issue.files in the python bug-tracker so the
> default search permission doesn't apply.

All this stuff needs to documented and/or referenced from XML-RPC
page. =/  I am a little upset with implicit complexity, because I've spent
much more time than I expected. If not this list, how can I know about
all that stuff to troubleshoot it properly?

> When searching for a property you have no search permission on, the
> property is ignored in the search. Your test looks exacly like that is
> happening.
>
> This is from a test tracker I'm running which allows view/search of all
> files to the logged-in user:
>
>>>> import xmlrpclib
>>>> from pprint import pprint as pp
>>>> p2 = xmlrpclib.ServerProxy('http://ralf:<censored>@bee:8080/xmlrpc')
>>>> pp (p2.display ('issue4711', 'files'))
> {'files': ['1276', '1277']}
>>>> pp (p2.display ('issue4712', 'files'))
> {'files': []}
>>>> pp (p2.filter ('issue', ['4711', '4712'], {'files':['-1']}))
> ['4712']
>
> Note that you need to quote the '-1', the code ask for a string not an
> int. One of the legacies of roundup is that id attributes are strings
> :-)
>
> I've tried that on bugs.python.org with my login and it seems my search
> permission is restricted, too (not just the anonymous user as in your
> example):
>
>>>> p = xmlrpclib.ServerProxy('http://runtux:<censored>@bugs.python.org/xmlrpc', allow_none=True)
>>>> pp (p.display ('issue9643', 'files'))
> {'files': []}
>>>> pp (p.display ('issue9644', 'files'))
> {'files': ['18578']}
>>>> pp (p.filter ('issue', ['9643', '9644'], {'files' : ['-1']}))
> ['9643', '9644']
>
> Same for messages:
>>>> pp (p.display ('issue9643', 'messages'))
> {'messages': ['114386', '114442', '227774']}
>>>> pp (p.display ('issue9644', 'messages'))
> {'messages': ['114392', '178696', '178697', '178765', '178766',
> '178767']}
>>>> pp (p.filter ('issue', ['9643', '9644'], {'messages' : ['-1']}))
> ['9643', '9644']
>>>> pp (p.filter ('issue', ['9643', '9644'], {'messages' : ['178765']}))
> ['9643', '9644']
>
>
> Note that the attribute 'nosy' doesn't seem to be restricted on
> bugs.python.org:
>
>>>> pp (p.display ('issue1723', 'nosy'))
> {'nosy': []}
>>>> pp (p.display ('issue1724', 'nosy'))
> {'nosy': ['8', '114']}
>>>> pp (p.filter ('issue', ['1723', '1724'], {'nosy' : ['-1']}))
> ['1723']

I don't see any differences between messages and nosy permissions:
https://hg.python.org/tracker/python-dev/file/9bab717d18a0/schema.py

> To add search permissions for a property you would add something like the
> following:
>
> p = db.security.addPermission \
>     ( name       = 'Search'
>     , klass      = 'issue'
>     , properties = ('files', 'messages')
>     )
> db.security.addPermissionToRole ('User', p)

Another thing in Roundup that needs refactoring. If only I had the time.. =)

I started all this from the need to present the python stdlib patch stats
script to public:
https://bitbucket.org/techtonik/python-stdlib/src/tip/modstats.py?at=default
b.p.o connection sometimes breaks during the process, so I just hammer
the server again and again until all queries pass. I am afraid that a couple
of machines that are downloading stats in this way will just bring b.p.o
down. Saving half of requests is big deal, which is only possible if search
works correctly.

-- 
anatoly t.

------------------------------------------------------------------------------
Meet PCI DSS 3.0 Compliance Requirements with EventLog Analyzer
Achieve PCI DSS 3.0 Compliant Status with Out-of-the-box PCI DSS Reports
Are you Audit-Ready for PCI DSS 3.0 Compliance? Download White paper
Comply to PCI DSS 3.0 Requirement 10 and 11.5 with EventLog Analyzer
http://pubads.g.doubleclick.net/gampad/clk?id=154622311&iu=/4140/ostg.clktrk