C-extension in zope.i18nmessageid
I've branched out this package and removed the C-extension. It's not documented in the package why a C-extension is needed or alternatively, what it benefits. If there are no objections, I will merge this into trunk shortly. \malthe
Hi Malthe Malthe Borch wrote:
I've branched out this package and removed the C-extension. It's not documented in the package why a C-extension is needed or alternatively, what it benefits.
If there are no objections, I will merge this into trunk shortly.
IIRC, the C extension is needed to make messageids truly immutable. This guards against certain kinds of errors and also means they don't need to be security proxied, which probably gives a good speedup somewhere. +1 to documenting why this is a good idea. -1 on removing the extension. HTH - Jacob
On Fri, Dec 12, 2008 at 11:28:48AM +0000, Malthe Borch wrote:
I've branched out this package and removed the C-extension. It's not documented in the package why a C-extension is needed or alternatively, what it benefits.
C extensions are usually optimizations.
If there are no objections, I will merge this into trunk shortly.
Could you please measure the impact on RAM usage and rendering speed of this removal for an app that uses Zope's i18n mechanism for rendering templates? Marius Gedminas -- The reason computer chips are so small is that computers don't eat much.
On Fri, Dec 12, 2008 at 11:28, Malthe Borch <mborch@gmail.com> wrote:
I've branched out this package and removed the C-extension. It's not documented in the package why a C-extension is needed or alternatively, what it benefits.
The C extension is required to make messageids immutable. Because they are immutable, the security machinery can treat them as rocks, e.g. safe to pass around. Removing the C-extension undoes this, as you cannot make truely immutable. See the original proposal: http://wiki.zope.org/zope3/TurningMessageIDsIntoRocks I'm sure Phillip and Jim can clarify the security implications of undoing this work. -- Martijn Pieters
Martijn Pieters wrote:
The C extension is required to make messageids immutable. Because they are immutable, the security machinery can treat them as rocks, e.g. safe to pass around. Removing the C-extension undoes this, as you cannot make truely immutable.
I believe it is possible to do this in pure Python: We'll set up a security-proxied global dictionary ``messages`` that maps object_id of message -> weakref(message) Then, the ``Message`` class would roughly look like this: class Message(unicode): def __new__(...): self = unicode.__new__(...) messages = removeSecurityProxy(messages) messages[id(self)] = (default, domain, mapping) @property def default(self): return messages[id(self)][0] The message data is effectively immutable, since the ``messages`` dictionary is security-proxied. To make sure the message properties are persisted along with the message, we must override the __reduce__-method to maintain the ``messages`` dict upon load. \malthe
Malthe Borch wrote:
I believe it is possible to do this in pure Python:
The implementation may reviewed in this branch: http://svn.zope.org/zope.i18nmessageid/branches/c-extension-less/ \malthe
On Fri, Dec 12, 2008 at 12:45:27PM +0000, Malthe Borch wrote:
Martijn Pieters wrote:
The C extension is required to make messageids immutable. Because they are immutable, the security machinery can treat them as rocks, e.g. safe to pass around. Removing the C-extension undoes this, as you cannot make truely immutable.
I believe it is possible to do this in pure Python:
I have doubts about that, but I don't think I'm smart enough to consider all the security implications.
We'll set up a security-proxied global dictionary ``messages`` that maps
object_id of message -> weakref(message)
Then, the ``Message`` class would roughly look like this:
class Message(unicode):
def __new__(...): self = unicode.__new__(...)
messages = removeSecurityProxy(messages)
messages[id(self)] = (default, domain, mapping)
Careful: id(some_object) will likely be reused when the old object is garbage collected.
@property def default(self): return messages[id(self)][0]
The message data is effectively immutable, since the ``messages`` dictionary is security-proxied.
To make sure the message properties are persisted along with the message, we must override the __reduce__-method to maintain the ``messages`` dict upon load.
-- http://pov.lt/ -- Zope 3 consulting and development
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Marius Gedminas wrote:
On Fri, Dec 12, 2008 at 12:45:27PM +0000, Malthe Borch wrote:
Martijn Pieters wrote:
The C extension is required to make messageids immutable. Because they are immutable, the security machinery can treat them as rocks, e.g. safe to pass around. Removing the C-extension undoes this, as you cannot make truely immutable.
I believe it is possible to do this in pure Python:
I have doubts about that, but I don't think I'm smart enough to consider all the security implications.
I'm still waiting for somebody (Jim, Martijn, Marius) to outline *any* security implication here: what kinds of attacks do you imagine become possible if some nefarious user finds a way to mutate a message ID? And are any such mutations feasible at all for applications which don't allow untrusted users to write code? Note that preventing *programming errors* is not sufficient justification in my mind: we already expect Python developers to play as "consenting adults" inside of trusted code. (later: Jim wrote me privately that he didn't have time to pursue the qu estion, but thought the dicussion could go on). Tres. - -- =================================================================== Tres Seaver +1 540-429-0999 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFJUVny+gerLs4ltQ4RAuNaAJ447pPnJ06+5vByqYQK6sP6/gm5HgCdH6LF Yz0hukR5bqNCO3IRQYAG+ks= =Kfhh -----END PGP SIGNATURE-----
On Dec 12, 2008, at 6:28 AM, Malthe Borch wrote:
I've branched out this package and removed the C-extension. It's not documented in the package why a C-extension is needed or alternatively, what it benefits.
If there are no objections, I will merge this into trunk shortly.
I object. Please drop it. Jim -- Jim Fulton Zope Corporation
On Fri, Dec 12, 2008 at 17:01, Jim Fulton <jim@zope.com> wrote:
On Dec 12, 2008, at 6:28 AM, Malthe Borch wrote:
I've branched out this package and removed the C-extension. It's not documented in the package why a C-extension is needed or alternatively, what it benefits.
If there are no objections, I will merge this into trunk shortly.
I object. Please drop it.
I object as well, and have asked for Malthe to provide his reasoning here at the Plone Performance Sprint in Bristol, but so far his only motivation is that he wants to see if he can get this to work without a C-extension. I am sceptical he'll be able to, and am not convinced it'll be worth introducing risks. -- Martijn Pieters
Martijn Pieters wrote:
I object as well, and have asked for Malthe to provide his reasoning here at the Plone Performance Sprint in Bristol, but so far his only motivation is that he wants to see if he can get this to work without a C-extension. I am sceptical he'll be able to, and am not convinced it'll be worth introducing risks.
The obvious motivation for this is to: * Reduce code complexity * Allow operation in a pure-Python environment As for cons, any change is a risk and I believe the concensus seen in this thread is that it outweighs the above mentioned motivation. \malthe
Malthe Borch wrote:
Martijn Pieters wrote:
I object as well, and have asked for Malthe to provide his reasoning here at the Plone Performance Sprint in Bristol, but so far his only motivation is that he wants to see if he can get this to work without a C-extension. I am sceptical he'll be able to, and am not convinced it'll be worth introducing risks.
The obvious motivation for this is to:
* Reduce code complexity * Allow operation in a pure-Python environment
As for cons, any change is a risk and I believe the concensus seen in this thread is that it outweighs the above mentioned motivation.
Allowing operation in a pure-Python environment is a worthwhile goal, which I support. Unless it can be clearly demonstrated that the new method is equivalent in both performance and security, talk of dropping the C extension seems somewhat premature. A pure Python fallback for this module would however be interesting to everybody, I think. My suspicion from observing the discussions in this thread so far indicate that a drop in code complexity doesn't seem to be a necessary consequence of rewriting to Python either. Regarsd, Martijn
On Fri, Dec 12, 2008 at 18:51, Martijn Faassen <faassen@startifact.com> wrote:
Unless it can be clearly demonstrated that the new method is equivalent in both performance and security, talk of dropping the C extension seems somewhat premature. A pure Python fallback for this module would however be interesting to everybody, I think.
There is one already. However, it isn't immutable and thus a security risk. -- Martijn Pieters
Martijn Faassen wrote:
My suspicion from observing the discussions in this thread so far indicate that a drop in code complexity doesn't seem to be a necessary consequence of rewriting to Python either.
The proposed implementation has already been implemented (walk up this thread); it is indeed much less complex. \malthe
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Martijn Faassen wrote:
Malthe Borch wrote:
Martijn Pieters wrote:
I object as well, and have asked for Malthe to provide his reasoning here at the Plone Performance Sprint in Bristol, but so far his only motivation is that he wants to see if he can get this to work without a C-extension. I am sceptical he'll be able to, and am not convinced it'll be worth introducing risks. The obvious motivation for this is to:
* Reduce code complexity * Allow operation in a pure-Python environment
As for cons, any change is a risk and I believe the concensus seen in this thread is that it outweighs the above mentioned motivation.
Allowing operation in a pure-Python environment is a worthwhile goal, which I support.
Unless it can be clearly demonstrated that the new method is equivalent in both performance and security, talk of dropping the C extension seems somewhat premature. A pure Python fallback for this module would however be interesting to everybody, I think.
My suspicion from observing the discussions in this thread so far indicate that a drop in code complexity doesn't seem to be a necessary consequence of rewriting to Python either.
I question the *actual* security benefits of making the message IDs truly read-only: I think the real intent is to avoid a common class of programming error, rather than to keep Black Hats out. For that side of the problem, we could use read-only properties for the data, and used something like the '__' prefix for the real backing-store attributes, then only folks who were being silly would ever change them. This is Python, after all: "we're all grownups" should apply. I'm willing to be shown wrong, of course, but I want to see a non-hypothetical attack vector which doesn't involve running trusted code from the filesystem. ;) (smiley because what other kind of code do we have in Z3 applications, anyway?) Tres. - -- =================================================================== Tres Seaver +1 540-429-0999 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFJQtrc+gerLs4ltQ4RAh6zAKC11lXsLS4aiLEmi97Bst5TXjemOQCeMx3R J4N59zGMJ4+hGY+bq4i8nEY= =Rplt -----END PGP SIGNATURE-----
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Jim Fulton wrote:
On Dec 12, 2008, at 6:28 AM, Malthe Borch wrote:
I've branched out this package and removed the C-extension. It's not documented in the package why a C-extension is needed or alternatively, what it benefits.
If there are no objections, I will merge this into trunk shortly.
I object. Please drop it.
I'd rather not drop it: can we (today) justify the need for the C extension? If nothing else, that explanation needs to be added to the package docs. If the justification is "support the model of untrusted code", could we not make the extension optional, and let folks who don't have that use case get by without needing to pay the price? For that matter, who actually has the use case in Z3 land? I don't know of *any* deployed applications which use persistent code, and therefore which need to worry about the problem. Certainly nothing in Zope2 land uses space suits. Tres. - -- =================================================================== Tres Seaver +1 540-429-0999 tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFJUDBw+gerLs4ltQ4RAhLuAJ9QUhPsCaVTOZAX0z+WEO1ojutKowCgvhUO 0gEhYrnEYAVzaVh610zgH3k= =AtJt -----END PGP SIGNATURE-----
participants (7)
-
Jacob Holm -
Jim Fulton -
Malthe Borch -
Marius Gedminas -
Martijn Faassen -
Martijn Pieters -
Tres Seaver