Re: [Zope-dev] Suggestion for small(?) change in BaseRequest.py. Security effects?
Lennart Regebro wrote at 2004-9-3 12:05 +0200: > ... >Dieter Maurer wrote: >> If the traversal made any changes to persistent state, then >> these changes are committed rather than aborted. >> >> Usually, traversal should not change the persistent state -- but... > >Would the transaction.abort() addition suggested by Tino be enough to >solve that? Yes. -- Dieter ___ Zope-Dev maillist - [EMAIL PROTECTED] http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Suggestion for small(?) change in BaseRequest.py. Security effects?
Dieter Maurer wrote: Lennart Regebro wrote at 2004-9-2 12:38 +0200: ... Are there any other problems with NOT raising an exception in unathorized(). Becuase if there is, we probably limit the possible challenge responses to a redirect, and then this change makes no difference. If the traversal made any changes to persistent state, then these changes are committed rather than aborted. Usually, traversal should not change the persistent state -- but... Would the transaction.abort() addition suggested by Tino be enough to solve that? ___ Zope-Dev maillist - [EMAIL PROTECTED] http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Suggestion for small(?) change in BaseRequest.py. Security effects?
Lennart Regebro wrote at 2004-9-2 12:38 +0200: > ... >Are there any other problems with NOT raising an exception in >unathorized(). Becuase if there is, we probably limit the possible >challenge responses to a redirect, and then this change makes no difference. If the traversal made any changes to persistent state, then these changes are committed rather than aborted. Usually, traversal should not change the persistent state -- but... -- Dieter ___ Zope-Dev maillist - [EMAIL PROTECTED] http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] Suggestion for small(?) change in BaseRequest.py. Security effects?
On Thu, 2004-09-02 at 12:38, Lennart Regebro wrote: > ... > Then, suggestion: > = > My tests seems to show that inserting a return after the unauthorized > call above: > if user is None and roles != UNSPECIFIED_ROLES: > response.unauthorized() > return > will solve this issue. It is now possible to NOT raise an exception in > unauthorized and still not get problems. Instead, you can now to a > RESPONSE.redirect(), or you can replace the body with setBody for a > login form, or something like that. > > I haven't been able to find any other code that continues after an > unathorized call, so this should be the only place. Also, during normal > operation, it is obvuiosly a safe bet. The change in itself has no nasty > side effects. I'd add a transaction rollback before return actually, which is the normal behavior when an exception is raised like Redirect or Unauthorized So if the situation is: user is anonymous or has insufficient rights on a object which is handled after some changes are made to objects, the challenge will take place and rolls back all these changes which should not have taken place for that user. so: if user is None and roles != UNSPECIFIED_ROLES: response.unauthorized() get_transaction().abort() return should do the trick. Regards Tino Wildenhain ___ Zope-Dev maillist - [EMAIL PROTECTED] http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Suggestion for small(?) change in BaseRequest.py. Security effects?
First Explanation:
==
We have been discussing how to implement the challenge support in
PluggableAuthService, and I have been trying a couple of different ways.
(Note: challenge-support in this context means, how to ask the client
for authenticifcation credentials, that is username and password, or
tickets, or certificates, or something.)
The preferred solution has been narrowed down to using a way similar to
CookieCrumbler, that is, replacing the current response-objects
unauthorized() method with a method that does the challenge, like so:
if not req.get('disable_cookie_login__', 0):
if attempt == ATTEMPT_LOGIN or attempt == ATTEMPT_NONE \
or attempt == ATTEMPT_RESUME:
# Modify the "unauthorized" response.
req._hold(ResponseCleanup(resp))
resp.unauthorized = self.unauthorized
resp._unauthorized = self._unauthorized
Now, there is one tiny problem with that, and that problem is in
BaseRequest.Traverse(). (BaseRequest.py, line 438):
if user is None and roles != UNSPECIFIED_ROLES:
response.unauthorized()
if user is not None:
...
As you see, this code assumes that response.unauthorized will raise an
error. If it does not, the code will continue *as if the user is
validated*. An error will occur later, since the user isn't validated,
so it's nota security hole (is think), but it causes a problem:
The only acceptable end of an unauthortized() is to raise an exception.
This means in practice, either the Unauthorized exception that prpvoces
the standasd 401 login box challenge, or a Redirect to a login page.
This seems a bit limiting, and there has been requests for doing other
things, such as returning an HTML form directly, and whatnot.
Then, suggestion:
=
My tests seems to show that inserting a return after the unauthorized
call above:
if user is None and roles != UNSPECIFIED_ROLES:
response.unauthorized()
return
will solve this issue. It is now possible to NOT raise an exception in
unauthorized and still not get problems. Instead, you can now to a
RESPONSE.redirect(), or you can replace the body with setBody for a
login form, or something like that.
I haven't been able to find any other code that continues after an
unathorized call, so this should be the only place. Also, during normal
operation, it is obvuiosly a safe bet. The change in itself has no nasty
side effects.
Last, questions:
Are there any other problems with NOT raising an exception in
unathorized(). Becuase if there is, we probably limit the possible
challenge responses to a redirect, and then this change makes no difference.
I would suggest that the change goes in ANYWAY, to stop people almost
accessing stuff by fiddling with response.unauthorized (although I admit
I don't know how to do that).
Lennart Regebro
___
Zope-Dev maillist - [EMAIL PROTECTED]
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )
