Log message for revision 40431: My original patch, with tests. Changed: U Zope/branches/tseaver-collector_1774/lib/python/AccessControl/ImplPython.py U Zope/branches/tseaver-collector_1774/lib/python/AccessControl/cAccessControl.c U Zope/branches/tseaver-collector_1774/lib/python/AccessControl/interfaces.py U Zope/branches/tseaver-collector_1774/lib/python/AccessControl/tests/testZopeSecurityPolicy.py
-=- Modified: Zope/branches/tseaver-collector_1774/lib/python/AccessControl/ImplPython.py =================================================================== --- Zope/branches/tseaver-collector_1774/lib/python/AccessControl/ImplPython.py 2005-11-30 22:40:15 UTC (rev 40430) +++ Zope/branches/tseaver-collector_1774/lib/python/AccessControl/ImplPython.py 2005-11-30 22:43:01 UTC (rev 40431) @@ -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,12 +462,28 @@ 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] - return context.user.allowed(object, roles) + result = context.user.allowed(object, roles) + # check executable owner and proxy roles + stack = context.stack + if stack: + eo = stack[-1] + if self._ownerous: + owner = eo.getOwner() + if (owner is not None) and not owner.allowed(object, roles): + return 0 + proxy_roles = getattr(eo, '_proxy_roles', None) + if proxy_roles: + if object is not aq_base(object): + if not owner._check_context(object): + return 0 + for r in proxy_roles: + if r in roles: + return 1 + return result # AccessControl.SecurityManager # ----------------------------- Modified: Zope/branches/tseaver-collector_1774/lib/python/AccessControl/cAccessControl.c =================================================================== --- Zope/branches/tseaver-collector_1774/lib/python/AccessControl/cAccessControl.c 2005-11-30 22:40:15 UTC (rev 40430) +++ Zope/branches/tseaver-collector_1774/lib/python/AccessControl/cAccessControl.c 2005-11-30 22:43:01 UTC (rev 40431) @@ -1292,61 +1292,200 @@ */ static PyObject *ZopeSecurityPolicy_checkPermission(PyObject *self, - PyObject *args) { + PyObject *args) { - PyObject *permission = NULL; - PyObject *object = NULL; - PyObject *context = NULL; - PyObject *roles; - PyObject *result = NULL; - PyObject *user; + /* return value */ + PyObject *rval = NULL; - /*| def checkPermission(self, permission, object, context) - */ + /* arguments, not increfe'd */ + PyObject *permission = NULL; + PyObject *object = NULL; + PyObject *context = NULL; - if (unpacktuple3(args, "checkPermission", 3, + /* locals, XDECREF at exit */ + PyObject *roles = NULL; + PyObject *user = NULL; + PyObject *stack = NULL; + PyObject *eo = NULL; + PyObject *owner = NULL; + PyObject *proxy_roles = NULL; + PyObject *method = NULL; + PyObject *wrappedowner = NULL; + PyObject *objectbase = NULL; + PyObject *incontext = NULL; + + int contains = 0; + int iRole; + int length; + + /*| def checkPermission(self, permission, object, context) + */ + + if (unpacktuple3(args, "checkPermission", 3, &permission, &object, &context) < 0) - return NULL; + return NULL; - /*| roles = rolesForPermissionOn(permission, object) - */ + /*| roles = rolesForPermissionOn(permission, object) + */ - roles = c_rolesForPermissionOn(permission, object, NULL, NULL); - if (roles == NULL) + roles = c_rolesForPermissionOn(permission, object, NULL, NULL); + if (roles == NULL) return NULL; - /*| if type(roles) in (StringType, UnicodeType): - **| roles = [roles] - */ + /*| if type(roles) in (StringType, UnicodeType): + **| roles = [roles] + */ - if ( PyString_Check(roles) || PyUnicode_Check(roles) ) { - PyObject *r; + if ( PyString_Check(roles) || PyUnicode_Check(roles) ) { + PyObject *role_list; - r = PyList_New(1); - if (r == NULL) { - Py_DECREF(roles); - return NULL; - } + role_list = PyList_New(1); + if (role_list == NULL) goto cP_done; /* Note: ref to roles is passed to the list object. */ - PyList_SET_ITEM(r, 0, roles); - roles = r; - } + PyList_SET_ITEM(role_list, 0, roles); + roles = role_list; + } - /*| return context.user.allowed(object, roles) - */ + /*| result = 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); - } - } + user = PyObject_GetAttr(context, user_str); + if (user != NULL) { + ASSIGN(user, PyObject_GetAttr(user, allowed_str)); + if (user != NULL) { + rval = callfunction2(user, object, roles); + } + } - Py_DECREF(roles); + + /*| # Check executable security + **| stack = context.stack + **| if stack: + */ + + stack = PyObject_GetAttr(context, stack_str); + if (stack == NULL) goto cP_done; + + if (PyObject_IsTrue(stack)) { + + /*| eo = stack[-1] + **| # If the executable had an owner, can it execute? + **| 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 + */ + + eo = PySequence_GetItem(stack, -1); + if (eo == NULL) goto cP_done; + + if (ownerous) { + owner = PyObject_GetAttr(eo, getOwner_str); + if (owner) ASSIGN(owner, PyObject_CallObject(owner, NULL)); + if (owner == NULL) goto cP_done; + + if (owner != Py_None) { + ASSIGN(owner, PyObject_GetAttr(owner, allowed_str)); + if (owner) ASSIGN(owner, callfunction2(owner, object, roles)); + if (owner == NULL) goto cP_done; + + if (! PyObject_IsTrue(owner)) { + rval = 0; + goto cP_done; + } + } + } + + /*| # Proxy roles, which are a lot safer now + **| 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 result + */ + proxy_roles = PyObject_GetAttr(eo, _proxy_roles_str); + + if (proxy_roles == NULL) { + PyErr_Clear(); + goto cP_done; + } - return result; + if (PyObject_IsTrue(proxy_roles)) { + method = PyObject_GetAttr(eo, getWrappedOwner_str); + if (method == NULL) goto cP_done; + + wrappedowner = PyObject_CallObject(method, NULL); + if (wrappedowner == NULL) goto cP_done; + + if (wrappedowner != Py_None) { + objectbase = aq_base(object); + + if (object != objectbase) { + + incontext = callmethod1(wrappedowner, + _check_context_str, object); + if (incontext == NULL) goto cP_done; + } + + if ( ! PyObject_IsTrue(incontext)) goto cP_done; + } + } + + if (PyTuple_Check(proxy_roles)) { + PyObject *proxy_role; + length = PyTuple_GET_SIZE(proxy_roles); + for (iRole=0; iRole < length; iRole++) { + proxy_role = PyTuple_GET_ITEM(proxy_roles, iRole); + /* proxy_role is not increfed */ + if ((contains = PySequence_Contains(roles, proxy_role))) + break; + } + } else { + PyObject *proxy_role; + length = PySequence_Size(proxy_roles); + if (length < 0) contains = -1; + for (iRole=0; contains == 0 && iRole < length; iRole++) { + proxy_role = PySequence_GetItem(proxy_roles, iRole); + if (proxy_role == NULL) goto cP_done; + /* proxy_role is increfed */ + contains = PySequence_Contains(roles, proxy_role); + Py_DECREF(proxy_role); + } + } + + if (contains > 0) + rval = PyInt_FromLong(contains); + } /* End of stack check */ + +cP_done: + Py_XDECREF(roles); + Py_XDECREF(user); + Py_XDECREF(stack); + Py_XDECREF(eo); + Py_XDECREF(owner); + Py_XDECREF(proxy_roles); + Py_XDECREF(method); + Py_XDECREF(wrappedowner); + Py_XDECREF(objectbase); + Py_XDECREF(incontext); + + return rval; } /* Modified: Zope/branches/tseaver-collector_1774/lib/python/AccessControl/interfaces.py =================================================================== --- Zope/branches/tseaver-collector_1774/lib/python/AccessControl/interfaces.py 2005-11-30 22:40:15 UTC (rev 40430) +++ Zope/branches/tseaver-collector_1774/lib/python/AccessControl/interfaces.py 2005-11-30 22:43:01 UTC (rev 40431) @@ -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/tseaver-collector_1774/lib/python/AccessControl/tests/testZopeSecurityPolicy.py =================================================================== --- Zope/branches/tseaver-collector_1774/lib/python/AccessControl/tests/testZopeSecurityPolicy.py 2005-11-30 22:40:15 UTC (rev 40430) +++ Zope/branches/tseaver-collector_1774/lib/python/AccessControl/tests/testZopeSecurityPolicy.py 2005-11-30 22:43:01 UTC (rev 40431) @@ -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 @@ -152,10 +151,8 @@ attr = 1 -class ZopeSecurityPolicyTests (unittest.TestCase): +class ZopeSecurityPolicyTestBase(unittest.TestCase): - policy = ZopeSecurityPolicy() - def setUp(self): a = App() self.a = a @@ -174,7 +171,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 +277,17 @@ 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 testUnicodeRolesForPermission(self): r_item = self.a.r_item context = self.context @@ -351,6 +363,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, #XXX C version, how? + ): + def _getTargetClass(self): + from AccessControl.ImplC import ZopeSecurityPolicy + return ZopeSecurityPolicy + def test_getRoles(): """ @@ -445,6 +478,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 +533,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