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
-~----------~----~----~----~------~----~------~--~---

Reply via email to