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

[Phillip J. Eby]

At 09:21 PM 6/28/00 +0000, Ty Sarna wrote:
>In article <39541FC1.A239BC5@digicool.com>,
>Shane Hathaway  <shane@digicool.com> 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.