Re: [Zope-dev] Re: Security audit introduced problem in PageTemplates/Expression.py

2004-01-18 Thread Stuart Bishop
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 17/01/2004, at 10:34 AM, Jim Fulton wrote:

I I'm pretty sure that I can redo the way we protect dictionaries and
lists so that we can provide backward compatability.  If I can do 
this,
I will, because backward compatability *is* important, especially for 
bug-fix
releases.
This is done and checked into the Zope 2.7 branch (Zope-2_7-branch).

Stuart, can you try this out and make sure that your application
works as it did before?
All appears to be working as before. If this is definitely
deprecated, I'll note that in AccessControl.py.
I don't have a problem with deprecating this feature if it makes
the Zope code saner - I was only using it because it was there
and did what I wanted.
I don't particularly like the idea of this mechanism working
for getattr access but not for getitem access. I've always
tended to stick with using getitem over getattr, partly as a
holdover from when it was incredibly painful to mix getattr
overrides with ExtensionClass, and partly because you are less
likely to recursively shoot yourself in the foot. Indeed - an
argument could be made for deprecating getattr in favor of
getitem, as the latter could make use of Unicode keys if Zope's
traversal mechanisms were updated to cope.
- --  
Stuart Bishop [EMAIL PROTECTED]
http://www.stuartbishop.net/
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.3 (Darwin)

iD8DBQFAC2d1AfqZj7rGN0oRAjPWAJ0VHsN8Rptk21xf90EyXTk5abgWiACeKZXM
l6yznxwTidlY2vooA9b+o0s=
=xCpW
-END PGP SIGNATURE-
___
Zope-Dev maillist  -  [EMAIL PROTECTED]
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 )


Re: [Zope-dev] Re: Security audit introduced problem in PageTemplates/Expression.py

2004-01-17 Thread Dieter Maurer
Jim Fulton wrote at 2004-1-16 18:54 -0500:
 ...
   For security checks, the accessed object should be the driving factor
   and not the particular way the access is made.

Well, sorry, that's not what this is about.  We are talking about what
to do when accessing objects without roles.  The ability to take
the name into account is a feature that only makes sense for named, ie
attribute access, imo.

item is a blurred term in Python:

  As you know, it refers both to sequences (indexed via integers)
  and mappings (indexed via something hashable; often a string).

When some mechanism checks whether access should be granted to
individual items in a mapping, this mechanism will (almost surely) need
to know the key used in the access -- and I do not see any reason
why it should not be informed about the key.


I do not argue that the handler registered with setDefaultAccess
should be used for __getitem__ access checking.

However, when it is called (as it seems to be the case),
then it should be called consistently and provide
as much information as useful -- this includes information
about both arguments to the __getitem__ method.


Even more essential for security related issues:

  A precise description when what security related functions
  are called with what arguments.

The current state in this respect is far from optimal.
Special points of my concern:

  *  I never saw a description of the difference between the
 accessed and container parameters to validate.

  *  I never saw a description for the three way outcome
 of validate: 0, 1 and raise Unauthorized.
 Why in hell is unauthorized encoded sometimes
 as return 0 and sometimes as raise Unauthorited.
 Looking at the code, I see that accessed/container
 has to do with this destinction. However, as
 accessed/container is unspecified, this does not clarify
 much.

   When we do not get this consistent, we open new hidden
   security holes (as one must always think: can this
   same object be accessed also in a different way
   and how have I to secure this way).

Certainly, you have to think about how you provide access to data.
Lots of data we provide access to has no security assertions of it's
own.

Maybe, we should change this for Zope 3?

It would have been possible for Zope 2 since a long time --
but tightening security has high risk to break many applicitation
(as the latest security fixes demonstrated again).

 Think of accessor methods that return data. If data needs to be
protected, you need to think about the access methods you provide.

In the future, item access will work like this:

 You will be able to protect __setitem__ operations.  Once
 someone can use setitem, they can access any key.  The value
 stored with that key may have pretections of it's own, or not.
 That's a matter of application design.

Fine!

However, security related rules are important enough that
they deserve thourough and prominent specification/documentation.

-- 
Dieter

___
Zope-Dev maillist  -  [EMAIL PROTECTED]
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 )


Re: [Zope-dev] Re: Security audit introduced problem in PageTemplates/Expression.py

2004-01-16 Thread Stuart Bishop
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 16/01/2004, at 9:23 AM, Jim Fulton wrote:

Dieter Maurer wrote:
Jim Fulton wrote at 2004-1-15 10:03 -0500:
...
Right. The name attribute was intended for attribute-based access.
IMO, it makes no sense to consider key values when doing security
checks.
I had thought we had more, but most of that logic is now in
a ComputedAttribute __ac_local_roles__ and in our __getitem__
hooks. What we currently have is a BTree storing key - value.
Valid keys are defined by a schema. If that schema changes, we
do not want to lose the information until we are sure it has
been archived, but we no longer want it available.
The sorts of things we *were* doing was allowing access to
certain attributes if the currently authenticated user had
required permissions on a related object.
eg. A bag-of-metadata has an associated Publisher object,
and the Publisher has Editors (implemented as a local
role assigned in the Publisher). Access to bits of
metadata would be calculated based on the bag's schema,
which determined if each bit was private, public or shared.
Shared was available to people with certain permissions
on the bag, or to people with certain permissions on the
associated Publisher (the Editors). Some metadata has
calculated privacy settings (eg. EmailAddress is shared
or public depending on the value of PrivateEmailAddress)
We now have the situation that this is possible if bag-of-metadata
is accessed via getattr (stored as attributes on the object),
but not via getitem (stored anywhere else).
I can also imagine BTrees keyed to userid (eg storing settings),
where people can only access their branch or branches of people
in their workgroup.
BTW, telling me that an algorithm has changed doesn't constitute
a use case. :) I know that algorithm has changed.  I assert that
we don't need the feature that the change broke.  I am open
to evidence to the contrary.
Its probably a feature we don't *need*, but some of us happen
to be using it, and have been using this documented feature
since Zope 2.5. I can probably work around it (although it
means the next milestone release next week will be on b3
instead of b4), but I'd assumed that if one person who
is using the beta triggered this issue, there will be plenty
more who may be upset when they try porting their apps to 2.7.0
release.
- --  
Stuart Bishop [EMAIL PROTECTED]
http://www.stuartbishop.net/
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.2.3 (Darwin)

iD8DBQFAB5kEAfqZj7rGN0oRAgGZAJ9gQe9xVX9pg/XdQKXpPVOruoD+/gCdG6vn
V1SPuM5ZOpsmy+hpI94JGc4=
=q473
-END PGP SIGNATURE-
___
Zope-Dev maillist  -  [EMAIL PROTECTED]
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 )


Re: [Zope-dev] Re: Security audit introduced problem in PageTemplates/Expression.py

2004-01-16 Thread Jim Fulton
Dieter Maurer wrote:
Jim Fulton wrote at 2004-1-15 17:23 -0500:

...
None should never be passed for attribute accesses. If it is,
then there is a bug.  The case of dictionary mapping names to
whatever is for attribute access.  We are talking about item/key
access. I haven't seen a use case for needing to specify separate access
for separate key values.


The original problem report (at least the one I read in
this mailing list) was that a function
registered with setDefaultAccess was called with
None as name argument.
I expect that such a function is not called for dictionary or list access
but only for access to (class) instances.
When it is called, the name is relevant, as usually the name
will be used to distinquish which attributes should be accessible
and which not. 
Well, the proginal message in this thread refers to an item access.

***
*** 312,318 
  # Skip directly to item access
  o = object[name]
  # Check access to the item.
! if not validate(object, object, name, o):
  raise Unauthorized, name
  object = o
  continue
The code above this:

if not name or name[0] == '_':

Checks for empty names or names beginning with underscrores, neither of
which are legal attribite names.
So, this does seem to be about item access.

Jim

--
Jim Fulton   mailto:[EMAIL PROTECTED]   Python Powered!
CTO  (540) 361-1714http://www.python.org
Zope Corporation http://www.zope.com   http://www.zope.org
___
Zope-Dev maillist  -  [EMAIL PROTECTED]
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 )


Re: [Zope-dev] Re: Security audit introduced problem in PageTemplates/Expression.py

2004-01-16 Thread Jim Fulton
Stuart Bishop wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 16/01/2004, at 9:23 AM, Jim Fulton wrote:

Dieter Maurer wrote:

Jim Fulton wrote at 2004-1-15 10:03 -0500:

...
Right. The name attribute was intended for attribute-based access.
IMO, it makes no sense to consider key values when doing security
checks.


I had thought we had more,
More what?

but most of that logic is now in
a ComputedAttribute __ac_local_roles__ and in our __getitem__
hooks. What we currently have is a BTree storing key - value.
Valid keys are defined by a schema. If that schema changes, we
do not want to lose the information until we are sure it has
been archived, but we no longer want it available.
The sorts of things we *were* doing was allowing access to
certain attributes
I'm confused.  Attributes of what? Above you are refering to
a BTree.
if the currently authenticated user had
required permissions on a related object.
eg. A bag-of-metadata has an associated Publisher object,
and the Publisher has Editors (implemented as a local
role assigned in the Publisher). Access to bits of
metadata would be calculated based on the bag's schema,
which determined if each bit was private, public or shared.
Shared was available to people with certain permissions
on the bag, or to people with certain permissions on the
associated Publisher (the Editors). Some metadata has
calculated privacy settings (eg. EmailAddress is shared
or public depending on the value of PrivateEmailAddress)
We now have the situation that this is possible if bag-of-metadata
is accessed via getattr (stored as attributes on the object),
but not via getitem (stored anywhere else).
My suggestion would be to wrap the BTree in an object that accesses
it via getattr.
I can also imagine BTrees keyed to userid (eg storing settings),
where people can only access their branch or branches of people
in their workgroup.
Sure, but, presumably, these settings would be objects that would
have p[rotections defined for their attributes.
BTW, telling me that an algorithm has changed doesn't constitute
a use case. :) I know that algorithm has changed.  I assert that
we don't need the feature that the change broke.  I am open
to evidence to the contrary.


Its probably a feature we don't *need*, but some of us happen
to be using it, and have been using this documented feature
since Zope 2.5.
Where is it documented? I did a quick look, and couldn'd find documentation
of this feature.
I can probably work around it (although it
means the next milestone release next week will be on b3
instead of b4), but I'd assumed that if one person who
is using the beta triggered this issue, there will be plenty
more who may be upset when they try porting their apps to 2.7.0
release.


Treating item access and attribute access the same way has some inherent
problems.  We ran into this in the security work. We needed to provide
protection for some methods on lists and dictionaries.  when we implemented
this, we found we could no-longer access their items.
It was never intended that the ability to control unprotected sub-objects
by name would apply to items.  It was sloppy coding on my part that item indexes
(yes, indexes, like, say, 1) and keys were passed as names.  I can certainly
understand why people looking at the code and trying things out would come
to the wrong conclusion.
Fundamentally, it's wrong to use the same mechanism for attributes and
item keys or indexes.  In the recent security work, we tried to address
this by not passing the name for for item access. Unfortunately, this broke
some code.  I *think* that there cannot be too many cases of this.
I I'm pretty sure that I can redo the way we protect dictionaries and
lists so that we can provide backward compatability.  If I can do this,
I will, because backward compatability *is* important, especially for bug-fix
releases.
I'll say, however that this feature should be considered deprecated.
We will *not* allow you to control access based on item keys or indexes
in Zope 3.  I *hope* to merge the Zope 2 and Zope 3 protection schemes in
Zope 2.9, so you should expect this feature to go away in Zope 2.9.
Jim

--
Jim Fulton   mailto:[EMAIL PROTECTED]   Python Powered!
CTO  (540) 361-1714http://www.python.org
Zope Corporation http://www.zope.com   http://www.zope.org


___
Zope-Dev maillist  -  [EMAIL PROTECTED]
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 )


Re: [Zope-dev] Re: Security audit introduced problem in PageTemplates/Expression.py

2004-01-16 Thread Jim Fulton
Jim Fulton wrote:
Stuart Bishop wrote:

...

It was never intended that the ability to control unprotected sub-objects
by name would apply to items.  It was sloppy coding on my part that item 
indexes
(yes, indexes, like, say, 1) and keys were passed as names.  I can 
certainly
understand why people looking at the code and trying things out would come
to the wrong conclusion.
But it would depend on which code they looked at. For example,
in 2.6.2, the key is not passed to validate when traversing using
getitem in unrestrictedTraverse.  For this reason, it's brittle to rely on
this, even without the recent security changes.
Fundamentally, it's wrong to use the same mechanism for attributes and
item keys or indexes.  In the recent security work, we tried to address
this by not passing the name for for item access. Unfortunately, this broke
some code.  I *think* that there cannot be too many cases of this.
I I'm pretty sure that I can redo the way we protect dictionaries and
lists so that we can provide backward compatability.  If I can do this,
I will, because backward compatability *is* important, especially for 
bug-fix
releases.
This is done and checked into the Zope 2.7 branch (Zope-2_7-branch).

Stuart, can you try this out and make sure that your application
works as it did before?
Jim

--
Jim Fulton   mailto:[EMAIL PROTECTED]   Python Powered!
CTO  (540) 361-1714http://www.python.org
Zope Corporation http://www.zope.com   http://www.zope.org
___
Zope-Dev maillist  -  [EMAIL PROTECTED]
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 )


Re: [Zope-dev] Re: Security audit introduced problem in PageTemplates/Expression.py

2004-01-16 Thread Jim Fulton
Dieter Maurer wrote:
Jim Fulton wrote at 2004-1-15 17:23 -0500:

BTW, telling me that an algorithm has changed doesn't constitute
a use case. :) I know that algorithm has changed.  I assert that
we don't need the feature that the change broke.  I am open
to evidence to the contrary.


Do you have a convincing reason to change the behaviour?

I argue here with consistency:

  When the setDefaultAccess function is called, it should
  always be called with sensible (and consistent) arguments.
  In my view, it is not consistent, that the function
  is called with the attribute name when the attribute is accessed
  via attribute access syntax but
  called with None when the same attribute it accessed
  via item access syntax.
Huh?  Nobody's calling setDefaultAccess with None.  Stuart is calling it
with a handler function. AFAICT, the use of a handler fucntion is
undocumented. It should be documented, but with different semantics
than Stuart expects. :)
  For security checks, the accessed object should be the driving factor
  and not the particular way the access is made.
Well, sorry, that's not what this is about.  We are talking about what
to do when accessing objects without roles.  The ability to take
the name into account is a feature that only makes sense for named, ie
attribute access, imo.
  When we do not get this consistent, we open new hidden
  security holes (as one must always think: can this
  same object be accessed also in a different way
  and how have I to secure this way).
Certainly, you have to think about how you provide access to data.
Lots of data we provide access to has no security assertions of it's
own.  Think of accessor methods that return data. If data needs to be
protected, you need to think about the access methods you provide.
In the future, item access will work like this:

You will be able to protect __setitem__ operations.  Once
someone can use setitem, they can access any key.  The value
stored with that key may have pretections of it's own, or not.
That's a matter of application design.
However, for backward compataibility, we'll leave things the way
they were, at least until Zope 2.9.
Jim

--
Jim Fulton   mailto:[EMAIL PROTECTED]   Python Powered!
CTO  (540) 361-1714http://www.python.org
Zope Corporation http://www.zope.com   http://www.zope.org
___
Zope-Dev maillist  -  [EMAIL PROTECTED]
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 )


Re: [Zope-dev] Re: Security audit introduced problem in PageTemplates/Expression.py

2004-01-15 Thread Dieter Maurer
Jim Fulton wrote at 2004-1-15 10:03 -0500:
 ...
Right. The name attribute was intended for attribute-based access.

IMO, it makes no sense to consider key values when doing security
checks.

 I will let Jim comment on your use case.

What use case?  I missed it. Where is it?

AccessControl.SecurityInfo.SecurityInfo.setDefaultAccess
allows integers, strings, dictionary mapping names to integers
and function with signature name,value -- boolean as
arguments.

The motivation is that some attributes may be accessible
while others should not. It is highly likely that
this decision is based on the attribute name.
When None is passed as name, you loose...

-- 
Dieter

___
Zope-Dev maillist  -  [EMAIL PROTECTED]
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 )


Re: [Zope-dev] Re: Security audit introduced problem in PageTemplates/Expression.py

2004-01-15 Thread Jim Fulton
Dieter Maurer wrote:
Jim Fulton wrote at 2004-1-15 10:03 -0500:

...
Right. The name attribute was intended for attribute-based access.
IMO, it makes no sense to consider key values when doing security
checks.

I will let Jim comment on your use case.
What use case?  I missed it. Where is it?


AccessControl.SecurityInfo.SecurityInfo.setDefaultAccess
allows integers, strings, dictionary mapping names to integers
and function with signature name,value -- boolean as
arguments.
The motivation is that some attributes may be accessible
while others should not. It is highly likely that
this decision is based on the attribute name.
When None is passed as name, you loose...
None should never be passed for attribute accesses. If it is,
then there is a bug.  The case of dictionary mapping names to
whatever is for attribute access.  We are talking about item/key
access. I haven't seen a use case for needing to specify separate access
for separate key values.
BTW, telling me that an algorithm has changed doesn't constitute
a use case. :) I know that algorithm has changed.  I assert that
we don't need the feature that the change broke.  I am open
to evidence to the contrary.
Jim

--
Jim Fulton   mailto:[EMAIL PROTECTED]   Python Powered!
CTO  (540) 361-1714http://www.python.org
Zope Corporation http://www.zope.com   http://www.zope.org
___
Zope-Dev maillist  -  [EMAIL PROTECTED]
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 )


Re: [Zope-dev] Re: Security audit introduced problem in PageTemplates/Expression.py

2004-01-15 Thread Dieter Maurer
Jim Fulton wrote at 2004-1-15 17:23 -0500:
BTW, telling me that an algorithm has changed doesn't constitute
a use case. :) I know that algorithm has changed.  I assert that
we don't need the feature that the change broke.  I am open
to evidence to the contrary.

Do you have a convincing reason to change the behaviour?

I argue here with consistency:

  When the setDefaultAccess function is called, it should
  always be called with sensible (and consistent) arguments.

  In my view, it is not consistent, that the function
  is called with the attribute name when the attribute is accessed
  via attribute access syntax but
  called with None when the same attribute it accessed
  via item access syntax.

  For security checks, the accessed object should be the driving factor
  and not the particular way the access is made.

  When we do not get this consistent, we open new hidden
  security holes (as one must always think: can this
  same object be accessed also in a different way
  and how have I to secure this way).

-- 
Dieter

___
Zope-Dev maillist  -  [EMAIL PROTECTED]
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 )


Re: [Zope-dev] Re: Security audit introduced problem in PageTemplates/Expression.py

2004-01-15 Thread Dieter Maurer
Jim Fulton wrote at 2004-1-15 17:23 -0500:
 ...
None should never be passed for attribute accesses. If it is,
then there is a bug.  The case of dictionary mapping names to
whatever is for attribute access.  We are talking about item/key
access. I haven't seen a use case for needing to specify separate access
for separate key values.

The original problem report (at least the one I read in
this mailing list) was that a function
registered with setDefaultAccess was called with
None as name argument.

I expect that such a function is not called for dictionary or list access
but only for access to (class) instances.
When it is called, the name is relevant, as usually the name
will be used to distinquish which attributes should be accessible
and which not. 

-- 
Dieter

___
Zope-Dev maillist  -  [EMAIL PROTECTED]
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 )