[Zope-dev] Re: Improvements for Zope2's security

[Philipp von Weitershausen]

Christian Heimes wrote:
> Zope's security declarations have to be called with a method *name* AS
> STRING. Developers are human beeings and human beeings tend to make
> small errors like typos. Or they forget to change the security
> declaration when they rename a method. Zope doesn't raise an error when
> a developer adds a security declaration for a non existing method.
> 
> Have a look at the following example. It contains a tiny but devastating
> typo::
> 
>     security.declarePrivate('chooseProtocol')
>     def chooseProtocols(self, request):
>         ...
> 
> These kinds or errors are extremly hard to find and may lead to big
> security holes. By the way this example was taken from a well known and
> well tested Zope addon!

THis problem is easily adressed when using ZCML security declarations + 
interfaces. Your decorator idea would also work.

> Issue 2
> =======
> 
> Another way to introduce security breaches is to forget the
> InitializeClass() call. The call is responsable for applying the
> informations from a ClassSecurityInfo. Without the call all security
> settings made through security.declare* are useless.
> 
> The good news is: Even if a developer forgets to call the method
> explictly in his code the function is called implictly.
> 
> The implicit call was hard to find for me at first. I struggled with the
> code in OFS.Image because there is an import of InitializeClass but it
> is never called. Never the less the security informations are somehow
> merged automagically. After some digging I found the code that is
> responsible for the magic. It's ExtensionClass' __class_init__ magic and
> OFS.ObjectManager.ObjectManager.__class_init__
> 
> Now the bad news: The magic doesn't work under some conditions. For
> example if you class doesn't subclass ObjectManager or if you monkey
> patch a class with security info object you have to call InitializeClass
> explictly.

The InitializeClass call is a big problem. I once discovered a big CMF 
security breach because of a missing InitializeClass call. Magic isn't a 
solution here, though. ZCML, for example, calls InitializeClass every 
time it changes the security declarations for a class. If you use ZCML, 
you're on the safe side.


> Issue 4
> =======
> 
> Some methods shouldn't be callable from certain types of request
> methods. For example there is no need that the webdav DELETE and PROP*
> methods should be callable from an ordinary HTTP GET request. I don't
> want to go into details. Some people know why :)

I have no idea how you want to distinguish HTTP and WebDAV requests. 
They're pretty much the same.

But you're right, Zope allows too much. I'm not convinced this is a 
security responsibility though. I think in the long term we'll adopt a 
solution like in Zope 3: look up views based on the request type.

> Only a small subset is implemented. There are two methods in my patch to
> either whitelist or blacklist a request method. An older version of my
> patch contained even code to distinguish between request types (ftp,
> http, ftp, xml-rpc) but Jim told me in a private mail it's kind of YAGNI.

I wonder what he meant by that. In Zope 3 we certainly do it.


In general, I think a sane way to improve Zope's security system is to 
adopt certian Zope 3 idioms until we essentially are using Zope 3's 
security machinery, but probably with a Zope 2 specific security policy. 
I find that Zope 3's approach with security proxies is a *much* easier 
approach than manual guarded_getattr etc., and Zope 3's checkers are 
much more flexible than Zope 2's ClassSecurityInfo stuff.

I personally think that efforts in improving Zope 2 security should 
primarily look at backporting useful features from Zope 3. That doesn't 
mean Zope 3 has to always have the right way of doing it, but at least 
changes in Zope 2 shouldn't go without an investigation of how Zope 3 
does it.

Philipp