Nevow Athena LiveFragment without allowedMethods

Hi JP & co., I've just started playing with current ATHENA - very nice !!! Is there any deeper security reason to use the "allowedMethods" mapping additionally to the simple fact, that a fragment implements a method only if it is necessary? If not, please consider to apply attached patch. It makes the code and use of it a little bit simpler. Thanks for your great work, best regards, Paul Reznicek Index: nevow/athena.py =================================================================== --- nevow/athena.py (Revision 3368) +++ nevow/athena.py (Arbeitskopie) @@ -407,20 +407,18 @@ <form onsubmit="Nevow.Athena.refByDOM(this).callRemote('foo', bar); return false;"> - By default, only methods named in the C{allowedMethods} mapping - may be invoked by the client. """ - allowedMethods = {} - def rend(self, context, data): myID = self.page.addLocalObject(self) context.fillSlots('nevow:athena_id', myID) return super(LiveFragment, self).rend(context, data) def locateMethod(self, ctx, methodName): - if methodName in self.allowedMethods: - return getattr(self, methodName) + if not methodName.startswith('_'): + method = getattr(self, methodName, None) + if method: + return method raise AttributeError(methodName) Index: examples/athenademo/athenatest.py =================================================================== --- examples/athenademo/athenatest.py (Revision 3368) +++ examples/athenademo/athenatest.py (Arbeitskopie) @@ -23,7 +23,6 @@ tags.form(action='#', onsubmit='return test(test_ClientToServerArgumentSerialization(this));')[ tags.input(type='submit', value='Test Client To Server Argument Serialization')]]) - allowedMethods = {'test': True} def test(self, i, f, s, l, d): self.assertEquals(i, 1) self.assertEquals(f, 1.5) @@ -65,7 +64,6 @@ tags.form(action='#', onsubmit='return test(test_ClientToServerResultSerialization(this));')[ tags.input(type='submit', value='Test Client To Server Result Serialization')]]) - allowedMethods = {'test': True} def test(self, i, f, s, l, d): return (i, f, s, l, d) @@ -104,7 +102,6 @@ tags.input(type='submit', value='Test Client To Server Asynchronous Exception Result')]]) - allowedMethods = {'testSync': True, 'testAsync': True} def testSync(self, s): raise Exception(s) @@ -135,7 +132,6 @@ tags.form(action='#', onsubmit='return test(test_ServerToClientArgumentSerialization(this));')[ tags.input(type='submit', value='Test Server To Client Argument Serialization')]]) - allowedMethods = {'test': True} def test(self): return self.page.callRemote('test_Reverse_ServerToClientArgumentSerialization', 1, 1.5, u'hello', {u'world': u'value'}); @@ -159,7 +155,6 @@ tags.form(action='#', onsubmit='return test(test_ServerToClientResultSerialization(this));')[ tags.input(type='submit', value='Test Server To Client Result Serialization')]]) - allowedMethods = {'test': True} def test(self): def cbResults(result): self.assertEquals(result[0], 1) Index: examples/athenademo/typeahead.py =================================================================== --- examples/athenademo/typeahead.py (Revision 3368) +++ examples/athenademo/typeahead.py (Arbeitskopie) @@ -20,7 +20,6 @@ class TypeAheadFieldFragment(athena.LiveFragment): docFactory = loaders.stan(T.input(type="text", id="typehere", **athena.liveFragmentID)) - allowedMethods = { 'loadDescription' : True } def loadDescription(self, typed): if typed == '': Index: examples/livefragments/livefrag.py =================================================================== --- examples/livefragments/livefrag.py (Revision 3368) +++ examples/livefragments/livefrag.py (Arbeitskopie) @@ -40,11 +40,10 @@ def __init__(self, label, *a, **kw): super(CooperativeFrag, self).__init__(*a, **kw) - self.allowedMethods = {label: True} setattr(self, label, lambda: unicode(label)) def render_submit(self, ctx, data): - return "submitIt(this, '%s'); return false;" % (self.allowedMethods.keys()[0],) + return "submitIt(this, 'label'); return false;" class Root(rend.Page): def child_(self, ctx):

... Sorry, I've introduced an error in livefrag.py, here corrected patch... Paul Index: livefrag.py =================================================================== --- livefrag.py (Revision 3368) +++ livefrag.py (Arbeitskopie) @@ -40,11 +40,10 @@ def __init__(self, label, *a, **kw): super(CooperativeFrag, self).__init__(*a, **kw) - self.allowedMethods = {label: True} setattr(self, label, lambda: unicode(label)) def render_submit(self, ctx, data): - return "submitIt(this, '%s'); return false;" % (self.allowedMethods.keys()[0],) + return "submitIt(this, '%s'); return false;" % label class Root(rend.Page): def child_(self, ctx):

On Wed, 30 Nov 2005 22:44:03 +0100, Paul Reznicek <maillists@ivsn.com> wrote:
Hi JP & co.,
I've just started playing with current ATHENA - very nice !!!
Hi Paul, thanks :)
Is there any deeper security reason to use the "allowedMethods" mapping additionally to the simple fact, that a fragment implements a method only if it is necessary?
I'm wary of making it possible to accidentally expose a method to the client. Without allowMethods, it seems to be a quite simple to accidentally expose things. For example, with your patch, the following methods can be invoked by the client on any LiveFragment: get rend remember rememberStuff child renderer render_sequence render_mapping render_string render_xml render_data macro getBindingNames getBinding getDefault postForm In that list, only postForm looks particularly scary, and many of these take objects which cannot currently be passed to methods by the client (ie, the context), but it is still quite a long list, and making it possible for the client to call these methods seems unnecessarily risky to me. If it seems okay to you, you can always create a LiveFragment subclass in your project with the locateMethod implementation included in your patch and always subclass that instead of LiveFragment. With the above list of methods in mind, does it make more sense why allowedMethods is present? Jean-Paul

Jean-Paul Calderone wrote:
I'm wary of making it possible to accidentally expose a method to the client. Without allowMethods, it seems to be a quite simple to accidentally expose things. For example, with your patch, the following methods can be invoked by the client on any LiveFragment:
get rend ... postForm
...
With the above list of methods in mind, does it make more sense why allowedMethods is present?
Dear Jean-Paul, You're right, it could be dangerous, but I'm soooo lazy for typing too much! Attached is a revisited patch, that fulfill the security and make the allowedMethods at the same time unnecessary - this version allow only usage of NEW methods in the subclass, no superclass methods or overriding of them are accepted, try it... Could it be a way? Paul Index: athena.py =================================================================== --- athena.py (Revision 3368) +++ athena.py (Arbeitskopie) @@ -407,21 +407,19 @@ <form onsubmit="Nevow.Athena.refByDOM(this).callRemote('foo', bar); return false;"> - By default, only methods named in the C{allowedMethods} mapping - may be invoked by the client. """ - allowedMethods = {} - def rend(self, context, data): myID = self.page.addLocalObject(self) context.fillSlots('nevow:athena_id', myID) return super(LiveFragment, self).rend(context, data) def locateMethod(self, ctx, methodName): - if methodName in self.allowedMethods: - return getattr(self, methodName) - raise AttributeError(methodName) + if not hasattr(super(LiveFragment, self), methodName): + method = getattr(self, methodName, None) + if method: + return method + raise AttributeError, 'Method "%s" not allowed' % methodName # Helper for docFactories defined with stan:
participants (2)
-
Jean-Paul Calderone
-
Paul Reznicek