Re: [Zope-dev] LoginManager patch considered harmful
"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 )
Re: [Zope-dev] LoginManager patch considered harmful
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. >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. >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.) 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. ___ 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 )
Re: [Zope-dev] LoginManager patch considered harmful
"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 ___ 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)
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 )
