Author tburke
Recipients ammar2, benjamin.peterson, gregory.p.smith, jaraco, larry, lukasz.langa, mcepl, ned.deily, tburke, webknjaz, xtreak
Date 2019-09-20.16:55:40
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1568998541.19.0.519964855053.issue38216@roundup.psfhosted.org>
In-reply-to
Content
> Since at least one project is known to have been impacted, it's not unreasonable to expect that more will be.

I can confirm at least one other: OpenStack Swift's stable jobs have been broken by https://github.com/python/cpython/commit/bb8071a since 11 Sep; see https://bugs.launchpad.net/swift/+bug/1843816 . (*Why* we're testing against an untagged version of py27, I still need to investigate...)

Now, the broken tests *do* represent something of a long-standing bug in application code: our front-end server was lenient in what it accepted but not strict in what it emitted -- it would forward on whatever crazy query params the client sent, directly to the back-end servers. Of course, as long as the back-end servers are *also* lenient in what they accept, it doesn't actually present a problem. Still, I'm happy to fix the issue.

Actually *addressing* this has proven problematic, however, as we have numerous functional tests that send raw UTF-8 request paths -- to the point that I can only assume there exist clients sending such requests. Changing the tests to URL-encode the path would do a disservice to those clients, as my fix (or some future fix) may break them and we'd be none the wiser.

> We either "be strict in what we produce" or we ignore abuse of the APIs and effectively close all CVEs filed against them as "not a bug, this library is intended to allow abuse when given untrusted input."

I think this is a false dichotomy; in https://bugs.python.org/issue36274#msg351834 Jason proposed a few alternatives that allow for a secure and obvious default API while adding a new, explicitly unsafe API.

I'd like to add yet another option that may be useful specifically for maintenance releases: forbid only the problematic characters -- namely LF (and potentially CR and SP). This seems like a much more surgical fix for maintenance releases, allowing the null byte for CherryPy or the raw UTF-8 bytes for Swift, while still mitigating the CVE.

3.8 and later can still have the more-robust check to ensure callers have done any requisite URL-escaping. Ideally, there would also be a reasonable hook in place to allow application developers to verify behavior in the presence of invalid requests; something short of writing raw bytes to a socket.

> For reference this is how urllib handled the same issue in their test suite : https://github.com/urllib3/urllib3/pull/1673.

I think the test suites developed by clients (like urllib3) and servers (like CherryPy or Swift) inherently have very different goals -- where a client would want to ensure that it *produces valid bytes* on the wire, a server ought to check that it doesn't completely fall over when it *receives invalid bytes* on the wire.

> What I seek is to avoid the Go recommendation of "fork the implementation" ...

This is *exactly* the way we worked around https://bugs.python.org/issue36274 in Swift: swap out putrequest() in our func tests. (See https://github.com/openstack/swift/commit/c0ae48b .) It looks like a similar change will be required to fix our stable branches.

I agree, I'd much rather *not* have to do that. :-/
History
Date User Action Args
2019-09-20 16:55:41tburkesetrecipients: + tburke, gregory.p.smith, jaraco, larry, benjamin.peterson, ned.deily, mcepl, lukasz.langa, webknjaz, ammar2, xtreak
2019-09-20 16:55:41tburkesetmessageid: <1568998541.19.0.519964855053.issue38216@roundup.psfhosted.org>
2019-09-20 16:55:41tburkelinkissue38216 messages
2019-09-20 16:55:40tburkecreate