Re: [Zope-dev] LoginManager patch considered harmful

2000-07-10 Thread Shane Hathaway

"Phillip J. Eby" wrote:
 
 At 09:22 AM 7/10/00 -0400, Shane Hathaway wrote:
 
 The new security machinery actually provides a different way to solve
 this problem.  Since we now have an execution stack, we can limit that
 stack, causing an exception to be thrown rather than letting it
 overflow the C stack.  It would actually be more reliable than the
 current mechanism, which depends on DTML namespaces.
 
 That would probably be a good idea, independent of LoginManager et al.

Well, guess what... Jim's ahead of both of us.  Not only is the stack
size limited, but the limit is easily configured through an environment
variable.  I never give this guy enough credit. :-)  See
lib/python/AccessControl/SecurityManager.py, especially addContext().

 If the LoginManager is created by the superuser and is used as the root
 authenticator, it stays unowned, so the ownership problem should not
 occur.  If that is not the current behavior then it needs to be
 corrected.
 
 That is not the current behavior, unless _owned=UnownableOwner is placed in
 the class.

It looks like Brian decided, for the latest beta, to make ownership
forgiving in the presence of an _owner class attribute.  This should
solve the problem for Generic User Source.  Generic User Folder is
going to need the _owner attribute.

 If untrusted users are allowed to add LoginManagers, the methods called
 to get ownership information should be owned by them.  This is to avoid
 the server-side trojan horse.
 
 Understood.  I'll try to keep that use case in mind.  Keep in mind,
 however, that being able to create a LoginManager is a pretty risky
 business in a portalish environment - you could potentially get access to
 somebody's password, since after all you will control an actual login
 screen!  (Note however, that someone can create a phony login screen in
 DTML and bypass the entire Zope security model with a little "social
 engineering" anyway.)

I've thought of that as well.  Perhaps we'll have to accept that the
new model just doesn't lend itself to the area of configurable user
databases.

 Also note that it is not necessary to give a user access to the full power
 of LoginManager.  One could give them rights only to add a small subset of
 the available UserSources and LoginMethods.  It would probably be a bad
 idea to give them the ability to add a GenericUserSource, which is where
 most of the potential for mayhem lies.  Better to give them some sort of
 PersistentUserSource, with no ability to do much.
 
 Now, the problem still remains that ownership information cannot be
 retrieved if the method that gets ownership is in ZODB, is owned by a
 user defined in that user folder, and has to call another method.  Note
 that this also applies to Generic User Folder.
 
 Yep.  The problem is that the superuser *can't* create those methods in the
 root folder, so there's no place to bottom out the recursion at present.

 With the correction in the execution stack, instead of killing Zope, an
 attempt to authenticate that way will result in a controlled stack
 overflow.  Unless you can come up with another option, we can either
 break the security model or slightly reduce the capabilities of
 LoginManager.
 
 What would you do if you were in my position?
 
 Well, I'm hoping you'll take a look at my Collector suggestion for a new
 Zope feature.  :)  Specifically, extending the "access" file to allow other
 "top-level" users to exist besides the superuser, who have roles defined in
 the file.  There are many ways this would be useful, not the least of which
 is to break the "you need to do that at the next level up" problem.
 (Others include a simplified process for getting your Zope site set up,
 since you then don't have to login as superuser and add a user folder, then
 log back in as somebody else.)

 If this were done, we could easily go to letting LoginManager objects be
 owned, since there'd always be a place "above" the LoginManager for the
 owner user to reside.

Hmm... this sounds like a good idea, but now that the ownership problem
has been resolved in a way I didn't expect, the issue that motivated
your idea is gone.

Shane

___
Zope-Dev maillist  -  [EMAIL PROTECTED]
http://lists.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists - 
 http://lists.zope.org/mailman/listinfo/zope-announce
 http://lists.zope.org/mailman/listinfo/zope )




[Zope-dev] LoginManager patch considered harmful (was Re: can't addloginmanager below root) loginmanager below root)

2000-07-08 Thread Phillip J. Eby

At 09:21 PM 6/28/00 +, Ty Sarna wrote:
In article [EMAIL PROTECTED],
Shane Hathaway  [EMAIL PROTECTED] wrote:
 Ok folks,
 
 I thought I had made it clear that this problem has been solved.  I have
 sent a patch to Ty (about two weeks ago) as well as made the change in

Sorry for the delay. I'm just now cacthing up on all my zope-related
mail. I've applied this patch, and it will be in the release tonight.
However, I am a bit concerned about what happens if _owner is deleted.
For purposes of setuid code, we'd like to use the owner as the user to
setuid to, but if we can't force it to be the right user, that could be
a problem. This will definately need more thought.


I'm afraid I've confirmed Ty's fears.  The LoginManager patch to fix the
"can't add below root" problem creates a new issue: ownership of objects
within the LoginManager.  I will be asking Ty to make a re-release of 0.8.7
to unpatch part of the patch, but for now, here's a short synopsis of the
problem and how to fix it.

Shane's patch changes manage_addLoginManager to set ob._owner =
UnownableOwner, which is good, because this ensures that
AccessControl.Owned.Owned._deleteOwnershipAfterAdd() can del self._owner.
(I think that Owned shouldn't be doing this this way, but as yet I don't
have a good alternative to propose that DC implement.)  The problem is that
once this is done, the LoginManager is no longer "unowned".  To fix this,
add the line:

_owner = UnownableOwner

to the body of the LoginManager class.  This will ensure that even after
_deleteOwnershipAfterAdd(), the LoginManager will remain "unowned".

After you have made this fix and restarted Zope, you may want to "un-own"
any objects contained within your LoginManagers which might have been
created with ownership.  To do this, you should go to each page of each
LoginManager's management interface and copy all objects, then paste them.
Delete the originals, and rename the copies back to their old names.  The
copies will then be "unowned".  You can verify this by checking each
object's management interface: if it is unowned, it will have no
"Ownership" tab.

Why does LoginManager want its contents unowned?  It has to do with the new
security model.  When an executable object is owned, the security machinery
wants to validate that its owner is allowed to do whatever the executable
is doing.  This is great except for the fact that in many sites, the owner
of the LoginManager's objects is actually a user that was retrieved from
that LoginManager.  This means that if LoginManager executes an object to
find out information about a user, and that object is owned by a user from
that LoginManager, infinite recursion and a core dump will swiftly follow.
For this reason, you should make sure that all executable objects in a
LoginManager are either unowned or owned by a user who is *not* supplied by
that LoginManager.  In practice, making sure they're unowned is usually
easier.


___
Zope-Dev maillist  -  [EMAIL PROTECTED]
http://lists.zope.org/mailman/listinfo/zope-dev
**  No cross posts or HTML encoding!  **
(Related lists - 
 http://lists.zope.org/mailman/listinfo/zope-announce
 http://lists.zope.org/mailman/listinfo/zope )