[Zope-dev] LoginManager patch considered harmful

[Shane Hathaway]

"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