[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Larry Hastings]

Change by Larry Hastings :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Larry Hastings]

Larry Hastings  added the comment:


New changeset 2784e78dc3445c6dd59e915d86c336374c1fa09a by larryhastings (Jason 
R. Coombs) in branch '3.5':
[3.5] bpo-38216, bpo-36274: Allow subclasses to separately override validation 
and encoding behavior (GH-16448) (#16475)
https://github.com/python/cpython/commit/2784e78dc3445c6dd59e915d86c336374c1fa09a


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Benjamin Peterson]

Benjamin Peterson  added the comment:


New changeset f5b1abbb3b0083381925dcd5898ae6d019224826 by Benjamin Peterson 
(Jason R. Coombs) in branch '2.7':
[2.7] bpo-38216, bpo-36274: Allow subclasses to separately override validation 
and encoding behavior (GH-16476)
https://github.com/python/cpython/commit/f5b1abbb3b0083381925dcd5898ae6d019224826


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Jason R. Coombs]

Change by Jason R. Coombs :


--
pull_requests: +16061
pull_request: https://github.com/python/cpython/pull/16476

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Jason R. Coombs]

Change by Jason R. Coombs :


--
pull_requests: +16059
pull_request: https://github.com/python/cpython/pull/16475

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Jason R. Coombs]

Jason R. Coombs  added the comment:

> Someone should look at how to do similar in 2.7 _if_ the project(s) that 
> complained about the problem rely on such behavior in their last 2.7 
> compatible releases.

Looking at the history, it seems that only two projects were mentioned, 
CherryPy and urllib3. For CherryPy, a workaround is in place such that the 2.7 
maintenance branch does not encounter this issue (the offending test was 
removed), and the master branch only supports Python 3. Similarly, urllib3 has 
a workaround that bypasses transmission of these bytes entirely. Therefore, the 
urgency is reduced.

I do feel a little uneasy recommending not to backport the bugfix to the 
versions where the change was introduced, as it leaves an opportunity for 
another project to encounter. I'll go ahead and prep a backport for Python 3.5 
and 2.7, with the understanding that the 2.7 change may be rejected or deferred 
until requested.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Gregory P. Smith]

Gregory P. Smith  added the comment:

I'd imagine an equivalent of the 3.6 PR would work for 3.5.

Someone should look at how to do similar in 2.7 _if_ the project(s) that 
complained about the problem rely on such behavior in their last 2.7 compatible 
releases.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Larry Hastings]

Larry Hastings  added the comment:

So, following this recent flurry of activity, all that remains are to sort out 
2.7 and 3.5.  3.5.8 is still in a holding pattern; at this point I think I'm 
going to insert another RC, so I can add the new version of expat.

Will a makes-everyone-happy PR appear for 3.5 soon?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Gregory P. Smith]

Change by Gregory P. Smith :


--
versions:  -Python 3.6, Python 3.7, Python 3.8, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Ned Deily]

Ned Deily  added the comment:


New changeset 5b18ce60b432d1dfa6f6988be07dd55646201a9b by Ned Deily (Jason R. 
Coombs) in branch '3.6':
[3.6] bpo-38216, bpo-36274: Allow subclasses to separately override validation 
and encoding behavior (GH-16448) (GH-16462)
https://github.com/python/cpython/commit/5b18ce60b432d1dfa6f6988be07dd55646201a9b


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[miss-islington]

miss-islington  added the comment:


New changeset 8f478b489ae11633d2609dff0ef21d0e1a857417 by Miss Islington (bot) 
in branch '3.8':
bpo-38216, bpo-36274: Allow subclasses to separately override validation and 
encoding behavior (GH-16448)
https://github.com/python/cpython/commit/8f478b489ae11633d2609dff0ef21d0e1a857417


--
nosy: +miss-islington

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Jason R. Coombs]

Jason R. Coombs  added the comment:


New changeset 80dd66ac278ecbabbf843526e3a56f5031da9562 by Jason R. Coombs in 
branch '3.7':
[3.7] bpo-38216, bpo-36274: Allow subclasses to separately override validation 
and encoding behavior (GH-16448) (GH-16461)
https://github.com/python/cpython/commit/80dd66ac278ecbabbf843526e3a56f5031da9562


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Jason R. Coombs]

Change by Jason R. Coombs :


--
pull_requests: +16046
pull_request: https://github.com/python/cpython/pull/16462

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Jason R. Coombs]

Change by Jason R. Coombs :


--
pull_requests: +16044
pull_request: https://github.com/python/cpython/pull/16461

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[miss-islington]

Change by miss-islington :


--
pull_requests: +16042
pull_request: https://github.com/python/cpython/pull/16460

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Jason R. Coombs]

Jason R. Coombs  added the comment:


New changeset 7774d7831e8809795c64ce27f7df52674581d298 by Jason R. Coombs in 
branch 'master':
bpo-38216, bpo-36274: Allow subclasses to separately override validation and 
encoding behavior (GH-16448)
https://github.com/python/cpython/commit/7774d7831e8809795c64ce27f7df52674581d298


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Jason R. Coombs]

Change by Jason R. Coombs :


--
pull_requests: +16027
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/16448

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Gregory P. Smith]

Gregory P. Smith  added the comment:

Regardless, since things have already shipped in stable releases, there is a 
release that code _will_ encounter somewhere that does validate data but does 
not support overruling that behavior.  so i'm not sure if it actually matters 
to have this in 3.7 or 3.6 as anyone who wants to avoid complaints from users 
who happen to be on those python patch levels will need to include a workaround 
in their code or explicitly avoid those versions.

That said, PR 16321 looks overall like a good idea, I have no problem with it 
being backported to 3.7 and 3.6.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Ned Deily]

Ned Deily  added the comment:

> Will the latter to accept non-ascii values also be merged to security 
> branches too given that it predates the security issue addressed ?

At this point, I'm willing to allow it in 3.6 unless someone identifies a 
compelling reason not to.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Karthikeyan Singaravelan]

Karthikeyan Singaravelan  added the comment:

If I understand PR 16321 correctly it has a private hook to bypass validating 
invalid bytes in URL added in 3.7.4 and also has the fix to accept non-ascii 
values which is a regression from 2.7 to 3.0 . Will the latter to accept 
non-ascii values also be merged to security branches too given that it predates 
the security issue addressed ?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Ned Deily]

Ned Deily  added the comment:

Also, besides the normal news entry (via blurb), there should probably be a 
"Notable changes in x.y.z" entry added to the end of each affected release's 
Doc/whatsnew/x.y.rst file.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Ned Deily]

Ned Deily  added the comment:

I am certainly not a domain expert but, at a high level, I think the approach 
in PR 16321 is a reasonable compromise and I would support merging it to 3.7 
and 3.6 (I'll let Larry and Benjamin speak for 3.5 and 2.7) assuming there are 
no review objections.  Since we seem to be getting close to a comprehensive 
solution here, I am going to hold off on tagging 3.7.5r1 for a few more days in 
the hopes we can reach agreement on this approach.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Jason R. Coombs]

Jason R. Coombs  added the comment:

Also, with the CherryPy approach, the Python 2.7 story is more complicated. I 
haven't yet addressed that in the CherryPy 17 maintenance branch (which 
supports Python 2.7).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Jason R. Coombs]

Jason R. Coombs  added the comment:

In https://github.com/cherrypy/cherrypy/pull/1807, I discovered that there is 
already a fairly straightforward means for a third-party package to override 
the putrequest character validation (just monkeypatch 
http.client._contains_disallowed_url_pchar_re with a less strict pattern; I 
used `[\n]`). This approach is barely worse than the proposal I made in the PR, 
the main differences being that a monkeypatch of that global variable is global 
(not selective to specific instances or subclasses of HTTPConnection) and 
doesn't have any tests in the CPython test suite to protect that as a supported 
mechanism.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Jason R. Coombs]

Jason R. Coombs  added the comment:

I've added PR 16321 illustrating my proposed solution. This solution, while 
more invasive than Tim's more surgical solution, addresses the concerns brought 
about by this issue as well as those articulated originally in issue36274.

I'm slightly inclined to suggest accepting this change for Python 3.8, and 
Tim's surgical patch for earlier versions except that doing so would not 
address the main concern of issue36274, which addresses a migration concern 
from Python 2.7 (a regression introduced in Python 3.0).

--
stage: patch review -> 

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Jason R. Coombs]

Change by Jason R. Coombs :


--
pull_requests: +15898
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/16321

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Jason R. Coombs]

Jason R. Coombs  added the comment:

> I'm not against that concept, but it is only appropriate for >= 3.9 as that'd 
> be adding a feature.  This issue is marked a release blocker to decide what 
> to do for 3.5-3.7 (and maybe 3.8 if deemed a serious breaking change).

The key part of the regression for 2.7 and 3.5+ was that it became impractical 
to retain the existing behavior of sending invalid bytes. My recommendation is 
to provide a mechanism to achieve compatibility with the older versions of 
Python. Although it's a "feature", it's also a feature necessitated by the 
security bugfix and to alleviate the regression. For this reason, I think the 
change should be applied to all Python versions that were patched for the 
security issue.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Tim Burke]

Tim Burke  added the comment:

Something like this for 3.7, say? I should probably go add some tests in 
test_httplib.py (for example, to demonstrate that http.client can still send a 
raw #, even if urllib appropriately drops the fragment), but I wanted some 
feedback on whether this is even an avenue worth pursuing.

--
keywords: +patch
Added file: 
https://bugs.python.org/file48618/0001-bpo-38216-Only-forbid-CR-LF-and-SP-in-http-URLs.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[Gregory P. Smith]

Gregory P. Smith  added the comment:

> 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'm not against that concept, but it is only appropriate for >= 3.9 as that'd 
be adding a feature.  This issue is marked a release blocker to decide what to 
do for 3.5-3.7 (and maybe 3.8 if deemed a serious breaking change).

> 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.

PRs with explicit tests for what is and isn't allowed welcome.  Thankfully for 
the UTF-8 case, its multi-byte codepoint bytes will never contain LF, CR or SP.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38216] Fix for issue30458 (HTTP Header Injection) prevents crafting invalid requests

[STINNER Victor]

Change by STINNER Victor :


--
title: Fix for issue30458 prevents crafting invalid requests -> Fix for 
issue30458 (HTTP Header Injection) prevents crafting invalid requests

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com