[Twisted-Python] maintenance release - a security issue and a regression
I think it might be time to have a maintenance release. Two issues in particular stand out which might be suitable for inclusion in a 12.3.1: <http://twistedmatrix.com/trac/ticket/6275> - This is a potential security issue which affects any twisted.web.template that uses the (recommended!) method of using a <t:attr> tag to render an attribute within a template. This might even be suitable for maintenance releases of older versions, if anyone is using them. <http://twistedmatrix.com/trac/ticket/6245> - This is a regression which affects anyone using twisted.names with 'unicode'-typed hostnames. This used to work, and, some of our own examples as well as some in-the-wild applications - mostly those using XMPP - actually relied upon it. IDNA hostnames never worked, but Python unicode-typed ASCII used to work and now it doesn't. Of course, in order to have a maintenance release with these bug fixes, several things need to happen. Someone needs to actually fix the issues. (I've written the code for #6275 but it is awaiting review; #6245 still needs to be fixed.) Someone needs to back-port those fixes to a release branch, based on the 12.3.0 tag, and file tickets for those backports. Someone needs to review the backports and get the committed to said branch. Someone needs to volunteer to be the release manager for 12.3.0. We apparently don't have any official process documentation for doing patch releases, but most of what's in <http://twistedmatrix.com/trac/wiki/ReleaseProcess> should apply. Any volunteers for parts of this process? -glyph
On 07:04 pm, glyph@twistedmatrix.com wrote:
[snip]
Of course, in order to have a maintenance release with these bug fixes, several things need to happen.
Someone needs to actually fix the issues. (I've written the code for #6275 but it is awaiting review; #6245 still needs to be fixed.)
#6275 looks resolved to me. I suggest considering http://twistedmatrix.com/trac/ticket/6259 as well. Jean-Paul
Someone needs to back-port those fixes to a release branch, based on the 12.3.0 tag, and file tickets for those backports. Someone needs to review the backports and get the committed to said branch. Someone needs to volunteer to be the release manager for 12.3.0.
Not 12.3.0 - 12.3.1.
We apparently don't have any official process documentation for doing patch releases, but most of what's in <http://twistedmatrix.com/trac/wiki/ReleaseProcess> should apply.
The other output should be documentation for the process for such a release. Jean-Paul
I'd like to volunteer to be release manager for Twisted 12.3.1, and help work on the other parts of the backporting/release process as I can. I found and reported bug #6275 while working on a Twisted-based project, and Glyph subsequently asked on IRC whether I'd like to be involved in the release. Shell Glyph wrote: I think it might be time to have a maintenance release. Two issues in particular stand out which might be suitable for inclusion in a 12.3.1: * <[http://twistedmatrix.com/trac/ticket/6275: <http://twistedmatrix.com/trac/ticket/6275>]> - This is a potential security issue which affects any twisted.web.template that uses the (recommended!) method of using a <t:attr> tag to render an attribute within a template. This might even be suitable for maintenance releases of older versions, if anyone is using them. * <[http://twistedmatrix.com/trac/ticket/6245: <http://twistedmatrix.com/trac/ticket/6245>]> - This is a regression which affects anyone using twisted.names with 'unicode'-typed hostnames. This used to work, and, some of our own examples as well as some in-the-wild applications - mostly those using XMPP - actually relied upon it. IDNA hostnames never worked, but Python unicode-typed ASCII used to work and now it doesn't. Of course, in order to have a maintenance release with these bug fixes, several things need to happen. 1) Someone needs to actually fix the issues. (I've written the code for #6275 but it is awaiting review; #6245 still needs to be fixed.) 2) Someone needs to back-port those fixes to a release branch, based on the 12.3.0 tag, and file tickets for those backports. 3) Someone needs to review the backports and get the committed to said branch. 4) Someone needs to volunteer to be the release manager for 12.3.0. We apparently don't have any official process documentation for doing patch releases, but most of what's in <[http://twistedmatrix.com/trac/wiki/ReleaseProcess: <http://twistedmatrix.com/trac/wiki/ReleaseProcess>]> should apply. Any volunteers for parts of this process? -glyph _______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
I'll look at #6245 again tomorrow. -- ralphm
On Wed, 30 Jan 2013 11:04:36 -0800 Glyph <glyph@twistedmatrix.com> wrote:
Any volunteers for parts of this process?
I'm not familiar with Twisted patching process and for this reason I'm just attaching a small patch here for #6245 because I'd like to discuss about the approach. If correct I will move on in the process (hopefully in the right way) The patch simply tries to encode the name argument properly if unicode. This is the same approach used by ralphm but applied to Name class initialization so it should be really generic. Just about a doubt about how to handle an exception potentially raised during the name encoding. Any idea? Ciao. PS Attached a simple test code which forces the name to resolve to be unicode. It fails against 12.3.0 while it is correclty executed after patching. -- Angelo Dell'Aera 'buffer' Antifork Research, Inc. http://buffer.antifork.org Sysenter Honeynet Project http://www.sysenter-honeynet.org
On Wed, 30 Jan 2013 23:32:34 +0100 Angelo Dell'Aera <angelo.dellaera@gmail.com> wrote:
On Wed, 30 Jan 2013 11:04:36 -0800 Glyph <glyph@twistedmatrix.com> wrote:
Any volunteers for parts of this process?
I'm not familiar with Twisted patching process and for this reason I'm just attaching a small patch here for #6245 because I'd like to discuss about the approach. If correct I will move on in the process (hopefully in the right way)
The patch simply tries to encode the name argument properly if unicode. This is the same approach used by ralphm but applied to Name class initialization so it should be really generic.
Just about a doubt about how to handle an exception potentially raised during the name encoding. Any idea?
Ciao.
PS Attached a simple test code which forces the name to resolve to be unicode. It fails against 12.3.0 while it is correclty executed after patching.
I read documentation about Twisted testing and tested if the suggested patch introduces some regressions in the existing code buffer@saiph ~/Twisted-12.3.0/twisted $ trial twisted.names [..] Ran 271 tests in 0.425s PASSED (successes=271) which seems like it's not happening. Obviously this is not exhaustive because seems like there are no specific tests for that code path (name is always passed as byte) but I can try writing some additional ones if needed. Ciao, -- Angelo Dell'Aera 'buffer' Antifork Research, Inc. http://buffer.antifork.org Sysenter Honeynet Project http://www.sysenter-honeynet.org
On Jan 31, 2013, at 6:07 AM, Angelo Dell'Aera <angelo.dellaera@gmail.com> wrote:
On Wed, 30 Jan 2013 23:32:34 +0100 Angelo Dell'Aera <angelo.dellaera@gmail.com> wrote:
On Wed, 30 Jan 2013 11:04:36 -0800 Glyph <glyph@twistedmatrix.com> wrote:
Any volunteers for parts of this process?
I'm not familiar with Twisted patching process and for this reason I'm just attaching a small patch here for #6245 because I'd like to discuss about the approach. If correct I will move on in the process (hopefully in the right way)
The patch simply tries to encode the name argument properly if unicode. This is the same approach used by ralphm but applied to Name class initialization so it should be really generic.
Just about a doubt about how to handle an exception potentially raised during the name encoding. Any idea?
Ciao.
PS Attached a simple test code which forces the name to resolve to be unicode. It fails against 12.3.0 while it is correclty executed after patching.
I read documentation about Twisted testing and tested if the suggested patch introduces some regressions in the existing code
buffer@saiph ~/Twisted-12.3.0/twisted $ trial twisted.names [..] Ran 271 tests in 0.425s
PASSED (successes=271)
which seems like it's not happening.
Obviously this is not exhaustive because seems like there are no specific tests for that code path (name is always passed as byte) but I can try writing some additional ones if needed.
Hi Angelo, Thanks for your contribution. It looks like the ticket in question already has a branch in review though, so we won't need your patch this time. In the future, the right way to submit patches is to attach them to the appropriate ticket; in this case, <http://twistedmatrix.com/trac/ticket/6245>. They tend to get lost or ignored on the mailing list (as happened here). To answer your question about additional tests, yes, whenever we fix a bug in Twisted, the fix has to come along with a new test to ensure that it stays fixed in subsequent releases. Thanks, -glyph
participants (5)
-
Angelo Dell'Aera -
exarkun＠twistedmatrix.com -
Glyph -
Ralph Meijer -
Shell