Log message for revision 40459: Collector #1774: Harmonize the implemtnation of 'checkPermission' with 'validate' We now check ownership and proxy roles if an executable object is on the stack. Note that we have removed the C implementation of 'checkPermission'.
Changed: U Zope/branches/Zope-2_8-branch/doc/CHANGES.txt U Zope/branches/Zope-2_8-branch/lib/python/AccessControl/ImplC.py U Zope/branches/Zope-2_8-branch/lib/python/AccessControl/ImplPython.py U Zope/branches/Zope-2_8-branch/lib/python/AccessControl/cAccessControl.c U Zope/branches/Zope-2_8-branch/lib/python/AccessControl/interfaces.py U Zope/branches/Zope-2_8-branch/lib/python/AccessControl/tests/testZopeSecurityPolicy.py -=- Modified: Zope/branches/Zope-2_8-branch/doc/CHANGES.txt =================================================================== --- Zope/branches/Zope-2_8-branch/doc/CHANGES.txt 2005-12-01 22:21:57 UTC (rev 40458) +++ Zope/branches/Zope-2_8-branch/doc/CHANGES.txt 2005-12-01 22:44:07 UTC (rev 40459) @@ -26,6 +26,11 @@ Bugs Fixed + - Collector #1774: Harmonize the implemtnation of + AccessControl.ZopeSecurityPolicy.checkPermission + with 'validate', checking ownership and proxy roles if an + executable object is on the stack. + - AccessControl.SecurityInfo: Fixed problem with setPermissionDefault when the permission wasn't used anywhere else in the class to protect methods. Modified: Zope/branches/Zope-2_8-branch/lib/python/AccessControl/ImplC.py =================================================================== --- Zope/branches/Zope-2_8-branch/lib/python/AccessControl/ImplC.py 2005-12-01 22:21:57 UTC (rev 40458) +++ Zope/branches/Zope-2_8-branch/lib/python/AccessControl/ImplC.py 2005-12-01 22:44:07 UTC (rev 40459) @@ -18,7 +18,8 @@ from cAccessControl import rolesForPermissionOn, \ PermissionRole, imPermissionRole, _what_not_even_god_should_do, \ RestrictedDTMLMixin, aq_validate, guarded_getattr, \ - ZopeSecurityPolicy, setDefaultBehaviors + setDefaultBehaviors + from cAccessControl import ZopeSecurityPolicy as cZopeSecurityPolicy from cAccessControl import SecurityManager as cSecurityManager except ImportError: import sys @@ -26,12 +27,17 @@ del sys.modules[__name__] -from ImplPython import RestrictedDTML, SecurityManager +from ImplPython import RestrictedDTML, SecurityManager, ZopeSecurityPolicy class RestrictedDTML(RestrictedDTMLMixin, RestrictedDTML): """A mix-in for derivatives of DT_String.String that adds Zope security.""" +class ZopeSecurityPolicy(cZopeSecurityPolicy, ZopeSecurityPolicy): + """A security manager provides methods for checking access and managing + executable context and policies + """ + class SecurityManager(cSecurityManager, SecurityManager): """A security manager provides methods for checking access and managing executable context and policies Modified: Zope/branches/Zope-2_8-branch/lib/python/AccessControl/ImplPython.py =================================================================== --- Zope/branches/Zope-2_8-branch/lib/python/AccessControl/ImplPython.py 2005-12-01 22:21:57 UTC (rev 40458) +++ Zope/branches/Zope-2_8-branch/lib/python/AccessControl/ImplPython.py 2005-12-01 22:44:07 UTC (rev 40459) @@ -34,6 +34,7 @@ from AccessControl import SecurityManagement from AccessControl import Unauthorized +from AccessControl.interfaces import ISecurityPolicy from AccessControl.interfaces import ISecurityManager from AccessControl.SimpleObjectPolicies import Containers, _noroles from AccessControl.ZopeGuards import guarded_getitem @@ -199,6 +200,8 @@ class ZopeSecurityPolicy: + implements(ISecurityPolicy) + def __init__(self, ownerous=1, authenticated=1, verbose=0): """Create a Zope security policy. @@ -459,13 +462,42 @@ raise Unauthorized(name, value) def checkPermission(self, permission, object, context): - # XXX proxy roles and executable owner are not checked roles = rolesForPermissionOn(permission, object) if isinstance(roles, basestring): roles = [roles] + + # check executable owner and proxy roles + stack = context.stack + if stack: + eo = stack[-1] + # If the executable had an owner, can it execute? + if self._ownerous: + owner = eo.getOwner() + if (owner is not None) and not owner.allowed(object, roles): + # We don't want someone to acquire if they can't + # get an unacquired! + return 0 + proxy_roles = getattr(eo, '_proxy_roles', None) + if proxy_roles: + # Verify that the owner actually can state the proxy role + # in the context of the accessed item; users in subfolders + # should not be able to use proxy roles to access items + # above their subfolder! + owner = eo.getWrappedOwner() + if owner is not None: + if object is not aq_base(object): + if not owner._check_context(object): + # object is higher up than the owner, + # deny access + return 0 + + for r in proxy_roles: + if r in roles: + return 1 + return 0 + return context.user.allowed(object, roles) - # AccessControl.SecurityManager # ----------------------------- Modified: Zope/branches/Zope-2_8-branch/lib/python/AccessControl/cAccessControl.c =================================================================== --- Zope/branches/Zope-2_8-branch/lib/python/AccessControl/cAccessControl.c 2005-12-01 22:21:57 UTC (rev 40458) +++ Zope/branches/Zope-2_8-branch/lib/python/AccessControl/cAccessControl.c 2005-12-01 22:44:07 UTC (rev 40459) @@ -337,8 +337,6 @@ */ static PyObject *ZopeSecurityPolicy_validate(PyObject *self, PyObject *args); -static PyObject *ZopeSecurityPolicy_checkPermission(PyObject *self, - PyObject *args); static void ZopeSecurityPolicy_dealloc(ZopeSecurityPolicy *self); @@ -418,11 +416,6 @@ METH_VARARGS, "" }, - {"checkPermission", - (PyCFunction)ZopeSecurityPolicy_checkPermission, - METH_VARARGS, - "" - }, { NULL, NULL } }; @@ -1287,69 +1280,6 @@ /* -** ZopeSecurityPolicy_checkPermission -** -*/ - -static PyObject *ZopeSecurityPolicy_checkPermission(PyObject *self, - PyObject *args) { - - PyObject *permission = NULL; - PyObject *object = NULL; - PyObject *context = NULL; - PyObject *roles; - PyObject *result = NULL; - PyObject *user; - - /*| def checkPermission(self, permission, object, context) - */ - - if (unpacktuple3(args, "checkPermission", 3, - &permission, &object, &context) < 0) - return NULL; - - /*| roles = rolesForPermissionOn(permission, object) - */ - - roles = c_rolesForPermissionOn(permission, object, NULL, NULL); - if (roles == NULL) - return NULL; - - /*| if type(roles) in (StringType, UnicodeType): - **| roles = [roles] - */ - - if ( PyString_Check(roles) || PyUnicode_Check(roles) ) { - PyObject *r; - - r = PyList_New(1); - if (r == NULL) { - Py_DECREF(roles); - return NULL; - } - /* Note: ref to roles is passed to the list object. */ - PyList_SET_ITEM(r, 0, roles); - roles = r; - } - - /*| return context.user.allowed(object, roles) - */ - - user = PyObject_GetAttr(context, user_str); - if (user != NULL) { - ASSIGN(user, PyObject_GetAttr(user, allowed_str)); - if (user != NULL) { - result = callfunction2(user, object, roles); - Py_DECREF(user); - } - } - - Py_DECREF(roles); - - return result; -} - -/* ** ZopeSecurityPolicy_dealloc ** */ Modified: Zope/branches/Zope-2_8-branch/lib/python/AccessControl/interfaces.py =================================================================== --- Zope/branches/Zope-2_8-branch/lib/python/AccessControl/interfaces.py 2005-12-01 22:21:57 UTC (rev 40458) +++ Zope/branches/Zope-2_8-branch/lib/python/AccessControl/interfaces.py 2005-12-01 22:44:07 UTC (rev 40459) @@ -17,8 +17,37 @@ from zope.interface import Interface from zope.interface import Attribute +class ISecurityPolicy(Interface): + """Plug-in policy for checking access to objects within untrusted code. + """ + def validate(accessed, container, name, value, context, roles=_noroles): + """Check that the current user (from context) has access. + + o Raise Unauthorized if access is not allowed; otherwise, return + a true value. + + Arguments: + + accessed -- the object that was being accessed + + container -- the object the value was found in + + name -- The name used to access the value + + value -- The value retrieved though the access. + + context -- the security context (normally supplied by the security + manager). + + roles -- The roles of the object if already known. + """ + + def checkPermission(permission, object, context): + """Check whether the current user has a permission w.r.t. an object. + """ + class ISecurityManager(Interface): - """Checks access and manages executable context and policies. + """Check access and manages executable context and policies. """ _policy = Attribute(u'Current Security Policy') Modified: Zope/branches/Zope-2_8-branch/lib/python/AccessControl/tests/testZopeSecurityPolicy.py =================================================================== --- Zope/branches/Zope-2_8-branch/lib/python/AccessControl/tests/testZopeSecurityPolicy.py 2005-12-01 22:21:57 UTC (rev 40458) +++ Zope/branches/Zope-2_8-branch/lib/python/AccessControl/tests/testZopeSecurityPolicy.py 2005-12-01 22:44:07 UTC (rev 40459) @@ -23,7 +23,6 @@ from zExceptions import Unauthorized except ImportError: Unauthorized = 'Unauthorized' -from AccessControl.ZopeSecurityPolicy import ZopeSecurityPolicy from AccessControl.User import UserFolder from AccessControl.SecurityManagement import SecurityContext from Acquisition import Implicit, Explicit, aq_base @@ -126,6 +125,8 @@ __allow_access_to_unprotected_subobjects__ = 0 + _Foo_Permission = user_roles + eo_roles + _Kill_Permission = sysadmin_roles _View_Permission = eo_roles @@ -152,10 +153,8 @@ attr = 1 -class ZopeSecurityPolicyTests (unittest.TestCase): +class ZopeSecurityPolicyTestBase(unittest.TestCase): - policy = ZopeSecurityPolicy() - def setUp(self): a = App() self.a = a @@ -174,7 +173,11 @@ self.user = user context = SecurityContext(user) self.context = context + self.policy = self._makeOne() + def _makeOne(self, *args, **kw): + return self._getTargetClass()(*args, **kw) + def assertPolicyAllows(self, ob, attrname): res = self.policy.validate(ob, ob, attrname, getattr(ob, attrname), self.context) @@ -276,6 +279,52 @@ v = self.policy.checkPermission('View', r_item, o_context) self.assert_(v, '_View_Permission should grant access to theowner') + def test_checkPermission_respects_proxy_roles(self): + r_item = self.a.r_item + context = self.context + self.failIf(self.policy.checkPermission('View', r_item, context)) + o_context = SecurityContext(self.uf.getUserById('joe')) + # Push an executable with proxy roles on the stack + eo = OwnedSetuidMethod().__of__(r_item) + eo._proxy_roles = eo_roles + context.stack.append(eo) + self.failUnless(self.policy.checkPermission('View', r_item, context)) + + def test_checkPermission_proxy_roles_limit_access(self): + r_item = self.a.r_item + context = self.context + self.failUnless(self.policy.checkPermission('Foo', r_item, context)) + o_context = SecurityContext(self.uf.getUserById('joe')) + # Push an executable with proxy roles on the stack + eo = OwnedSetuidMethod().__of__(r_item) + eo._proxy_roles = sysadmin_roles + context.stack.append(eo) + self.failIf(self.policy.checkPermission('Foo', r_item, context)) + + def test_checkPermission_proxy_role_scope(self): + self.a.subobject = ImplictAcqObject() + subobject = self.a.subobject + subobject.acl_users = UserFolder() + subobject.acl_users._addUser('theowner', 'password', 'password', + eo_roles + sysadmin_roles, ()) + subobject.r_item = RestrictedSimpleItem() + r_subitem = subobject.r_item + r_subitem.owned_setuid_m = OwnedSetuidMethod() + r_subitem.getPhysicalRoot = lambda root=self.a: root + + r_item = self.a.r_item + r_item.getPhysicalRoot = lambda root=self.a: root + context = self.context + context.stack.append(r_subitem.owned_setuid_m.__of__(r_subitem)) + + # Out of owner context + self.failIf(self.policy.checkPermission('View', r_item, context)) + self.failIf(self.policy.checkPermission('Kill', r_item, context)) + + # Inside owner context + self.failIf(self.policy.checkPermission('View', r_subitem, context)) + self.failUnless(self.policy.checkPermission('Kill', r_subitem, context)) + def testUnicodeRolesForPermission(self): r_item = self.a.r_item context = self.context @@ -351,6 +400,27 @@ self.fail('Policy accepted bad __roles__') +class ISecurityPolicyConformance: + + def test_conforms_to_ISecurityPolicy(self): + from AccessControl.interfaces import ISecurityPolicy + from zope.interface.verify import verifyClass + verifyClass(ISecurityPolicy, self._getTargetClass()) + +class Python_ZSPTests(ZopeSecurityPolicyTestBase, + ISecurityPolicyConformance, + ): + def _getTargetClass(self): + from AccessControl.ImplPython import ZopeSecurityPolicy + return ZopeSecurityPolicy + +class C_ZSPTests(ZopeSecurityPolicyTestBase, + ISecurityPolicyConformance, + ): + def _getTargetClass(self): + from AccessControl.ImplC import ZopeSecurityPolicy + return ZopeSecurityPolicy + def test_getRoles(): """ @@ -445,6 +515,7 @@ def test_zsp_gets_right_roles_for_methods(): """ + >>> from AccessControl.ZopeSecurityPolicy import ZopeSecurityPolicy >>> zsp = ZopeSecurityPolicy() >>> from ExtensionClass import Base >>> class C(Base): @@ -499,7 +570,8 @@ def test_suite(): suite = unittest.TestSuite() - suite.addTest(unittest.makeSuite(ZopeSecurityPolicyTests, 'test')) + suite.addTest(unittest.makeSuite(Python_ZSPTests, 'test')) + suite.addTest(unittest.makeSuite(C_ZSPTests, 'test')) suite.addTest(DocTestSuite()) return suite _______________________________________________ Zope-Checkins maillist - Zope-Checkins@zope.org http://mail.zope.org/mailman/listinfo/zope-checkins