On Mon, Jun 15, 2009 at 11:29 AM, Gustavo Narea<[email protected]> wrote: > > Hello. > > From time to time I get (mostly private) emails from TG users who complained > because there was no way to evaluate predicate checkers implicitly (i.e., > without passing the environ explicitly) and now complain because, although > it's possible, I strongly discourage it. > > So here I'll elaborate on the warning I put on the repoze.what-pylons docs, so > I won't have to repeat the explanation over and over and over again in > different ways, and keep receiving complaints (even from the same people) > nevertheless. > > First of all, keep in mind that this has nothing to do with this functionality > being handy or not. I agree it's handy, but that's out of the scope of this > thread. My goal is _not_ to discuss if it's handy or not, but why it should be > avoided. > > OK, so here I go... > > The warning reads: > "This functionality was implemented by popular demand, but it’s strongly > discouraged by the author of repoze.what because it’s a monkey-patch which > brings serious side-effects when enabled: > > 1.- Third-party components which handle repoze.what predicates may get > erroneous values after evaluating a predicate checker (even if they don’t use > this functionality!). > 2.- If another non-Pylons-based application uses the same monkey-patch and you > mount it on your application (or vice versa), the predicates used by both > application will share the same WSGI environ. > > In the two scenarios above, it will lead to serious security flaws. So avoid > it by all means! Use predicate evaluators instead." > > I'll start explaining side-effect #2 because I believe it'd be the most > important one for TG2 users. Then I'll explain side-effect #1. > > ===== Predicates booleanized in multiple repoze.what-powered apps ===== > > This is how *all* TG2 applications behave: > """ > # Keep in mind that this is *pseudo-code* to illustrate how things work! > > ... > from repoze.what.predicates import Predicate > from tg import request > ... > > class TurboGearsApp(object): > def __init__(self, ...): > ... > Predicate.__nonzero__ = lambda self: self.is_met(request.environ) > ... > > def __call__(self, environ, start_response): > ... > """ > > Through repoze.what-pylons, TurboGears 2 applications set the .__nonzero__ > attribute of the base Predicate class, so predicate checkers can be used as > booleans without passing the environ explicitly. > > Then it's perfectly possible for other non-Pylons based applications to offer > this handy functionality as well if they use repoze.what, so they'd do: > """ > # Pseudo code here too. > > ... > from repoze.what.predicates import Predicate > from myframework import environ # <-- Assuming they use thread-locals too > ... > > class MyFrameworkApp(object): > """ > This doesn't "have" to come from a framework. It can be a raw WSGI app. > """ > def __init__(self, ...): > ... > Predicate.__nonzero__ = lambda self: self.is_met(environ) > ... > > def __call__(self, environ, start_response): > ... > """ > > In both WSGI applications, the __init__ method will be called just once and > its object will be shared among threads. > > Now, the problem arises when both applications are used together; this is, one > mounted on the other. When they're used independently one another, there's no > problem at all. > > When they are mounted one on the other, Predicate.__nonzero__ will equal: > lambda self: self.is_met(myframework.environ) > or, > lambda self: self.is_met(tg.request.environ) > forever, in every thread, in every request. > > How come? When MyFrameworkApp.__init__ is called first and > TurboGearsApp.__init__ is called next, Predicate.__nonzero__ takes the > following values, respectively: > lambda self: self.is_met(myframework.environ) > lambda self: self.is_met(tg.request.environ) > > Likewise, when TurboGears.__init__ is called first and MyFrameworkApp.__init__ > is called next, Predicate.__nonzero__ takes the following values, > respectively: > lambda self: self.is_met(tg.request.environ) > lambda self: self.is_met(myframework.environ) > > It will certainly cause problems. In the best case scenario, an exception will > be raised and security won't be compromised. In the worse one, the environ > will have a default value and thus security could be compromised. Either way, > the application will be broken. > > And it's a big mistake to believe that it would be solved if repoze.what > itself set Predicate.__nonzero__. It won't change anything at all. It'd be the > same module-level change, just that instead of being dynamic (like the monkey- > patch), it'd be static. > > And before somebody argues "But it's not a common scenario!": So what? The > point is that it's possible. There's nothing weird in having two repoze.what > powered applications, one with TG2 and the other non-TG2 based. > > ===== Third party components ===== > > Now, this will be quite annoying for people who write plugins/routines that > deal with predicate checkers, and pretty dangerous for TG2 users. > > Predicate checkers don't act as booleans, since the only officially supported > way to evaluate them is to pass the WSGI environ to the > .is_met()/.check_authorization() methods. But then we have "the TurboGears > way" to evaluate predicate checkers as plain old bool. > > So, someone who wrote something that deals with predicate checkers could have > this: > if predicate: > # There's a predicate defined > else: > # There's no predicate defined (predicate is None) > > But if a TurboGears app is being used, it'd actually mean: > if predicate: > # The predicate exists AND evaluates to True > else: > # There's no predicate OR it exists but is not met > > Which is horribly dangerous for TG2 users. It'd be a critical security flaw. > > Even I had to update repoze.what itself to take into account "the TurboGears > way" to evaluate predicates. But non-TG2 users who write stuff to deal with > predicates may not be aware of this and TG2 users who use such software will > get in a trouble... Until they find it and ask the maintainer to update the > original TG2-independent code. > > ===== > > So, to sum up: > > 1.- Predicate checkers won't ever act as bool officially (i.e., with > Predicate.__nonzero__ defined by default), unless somebody comes up with a > solution to side-effect #2. > 2.- TG2 users are *strongly* encouraged to disable this monkey-patch, unless > they are 500% sure that their applications *won't* *ever*: > A.- Mount or be mounted on another repoze.what-powered application which > is not based on Pylons/TG2. > B.- Use third party, TG2-independent tools which handle predicate > checkers. > > Does anything need further clarification? >
Thank you I finally understand your issue with it. And I do see this as a valid concern. Although I still think the warning at http://code.gustavonarea.net/repoze.what-pylons/Manual/Misc.html#boolean-predicates is over the top. And I think this explanation should go in there somehow. Specially be calling it a monkey-patch, it is indeed a monkey-patch but by choice because of #1, and I won't argue with you on that. But at least at this point in time both cases of #2 are really minor. At least I don't know any installation that currently exposes any of those. --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "TurboGears" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/turbogears?hl=en -~----------~----~----~----~------~----~------~--~---

