Re: [Zope-dev] Improvements for Zope2's security
I think it's great that you did this... nice job! I have some
specific disagreements (while I think it's a reasonable constraint,
and I think something should enforce it, I don't believe it's the job
of something that we call a *security policy* to enforce whether a
method is called, e.g. via POST rather than GET), I think you did a
wonderful job analyzing this.. +1 on the developer-helper tools
portion of this that finds declarations that have no corresponding
method.
Thanks!
- C
On Sep 18, 2006, at 11:20 AM, Christian Heimes wrote:
Hey guys!
In the past few months I fiddled around with Zope2's security and
access
control code. I analysied my own code and code from other
developers to
search for common errors. Also I tried to think of ways to make the
security system easier and more verbose on coding errors
I have not yet implemented all my ideas but Zope 2.10 is on the door
steps. Here is my first set of improvements.
Issue 1
===
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!
Solution
The solution was very easy to implement. I created a small helper
function checkClassHasMethod() and called it in the apply() method of
AccessControl.SecurityInfo.ClassSecurityInfo. The apply() method is
called at startup. The code doesn't slow down requests.
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.
Solution
Not yet finished. I've created a function checkObjectHasSecurityInfo()
and added a call to ZPublisher.BaseRequest.DefaultPublishTraverse
but it
is untested.
Issue 3
===
Developers are lazy and they like to make typos. No one likes to type
security.declarePrivate('chooseProtocol') so we are using copy & paste
which may cause even more typos. Wouldn't it be cool to get rid of
security. and typing the name of the method twice? Let's use
decorators!
Here is the doc test example from my patch:
Solution
Security decorators are an alternative syntax to define security
declarations on classes.
from ExtensionClass import Base
from AccessControl import ClassSecurityInfo
from AccessControl.decorator import declarePublic
from AccessControl.decorator import declarePrivate
from AccessControl.decorator import declareProtected
from AccessControl.Permissions import view as View
from Globals import InitializeClass
class DecoratorExample(Base):
... '''decorator example'''
...
... security = ClassSecurityInfo()
...
... @declarePublic
... def publicMethod(self):
... "public method"
...
... @declarePrivate
... def privateMethod(self):
... "private method"
...
... @declareProtected(View)
... def protectedByView(self):
... "method protected by View"
...
InitializeClass(DecoratorExample)
With the new syntax you have to type only 15 letters instead of 41!
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 :)
Solution
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
Re: [Zope-dev] Improvements for Zope2's security
You have many good points in your list of troubles. Many of them are resolved by using security declarations through ZCML instead. It would be interesting to here your views on this. //Lennart ___ Zope-Dev maillist - Zope-Dev@zope.org http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
[Zope-dev] Improvements for Zope2's security
Hey guys!
In the past few months I fiddled around with Zope2's security and access
control code. I analysied my own code and code from other developers to
search for common errors. Also I tried to think of ways to make the
security system easier and more verbose on coding errors
I have not yet implemented all my ideas but Zope 2.10 is on the door
steps. Here is my first set of improvements.
Issue 1
===
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!
Solution
The solution was very easy to implement. I created a small helper
function checkClassHasMethod() and called it in the apply() method of
AccessControl.SecurityInfo.ClassSecurityInfo. The apply() method is
called at startup. The code doesn't slow down requests.
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.
Solution
Not yet finished. I've created a function checkObjectHasSecurityInfo()
and added a call to ZPublisher.BaseRequest.DefaultPublishTraverse but it
is untested.
Issue 3
===
Developers are lazy and they like to make typos. No one likes to type
security.declarePrivate('chooseProtocol') so we are using copy & paste
which may cause even more typos. Wouldn't it be cool to get rid of
security. and typing the name of the method twice? Let's use decorators!
Here is the doc test example from my patch:
Solution
Security decorators are an alternative syntax to define security
declarations on classes.
>>> from ExtensionClass import Base
>>> from AccessControl import ClassSecurityInfo
>>> from AccessControl.decorator import declarePublic
>>> from AccessControl.decorator import declarePrivate
>>> from AccessControl.decorator import declareProtected
>>> from AccessControl.Permissions import view as View
>>> from Globals import InitializeClass
>>> class DecoratorExample(Base):
... '''decorator example'''
...
... security = ClassSecurityInfo()
...
... @declarePublic
... def publicMethod(self):
... "public method"
...
... @declarePrivate
... def privateMethod(self):
... "private method"
...
... @declareProtected(View)
... def protectedByView(self):
... "method protected by View"
...
>>> InitializeClass(DecoratorExample)
With the new syntax you have to type only 15 letters instead of 41!
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 :)
Solution
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.
At the moment blacklistRequestMethod() and whitelistRequestMethod() have
to be called explictly inside a method. There is no way to protect
methods via security.blacklistRequestMethod() or @blacklistRequestMethod.
I have two ideas to implement such security declarations but both ways
are complicated and hard to implement. An ordinary decorator doesn't
work because it messes up with ZPublisher.mapply.
The following code does NOT work with POST requests because mapply is
using introspection to get the names and default values of a method
