"Phillip J. Eby" wrote:
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".
Your logic is sound, and I'm glad you spotted the recursion problem, but I would like to propose a different solution.
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.
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. Now let's do an analysis of this situation. 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. 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. I used to think that the new security model was a break from the *nix security model. But consider this: sysadmins don't go around executing arbitrary users' scripts, do they? Of course not. The only parallel to what Zope provides is CGI scripts. Sysadmins are able to visit arbitrary CGI scripts without worry of a server-destined trojan (although a client-destined trojan is another matter) because the scripts are not given extra privileges. In fact, the suexec add-on makes Apache behave almost exactly the way Jim is getting Zope to behave. 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. 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? Shane