Bug 194483
| Summary: | [PATCH] libfetch: Properly deal with multi-line proxy responses to CONNECT requests | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Fabian Keil <fk> | ||||||||||
| Component: | kern | Assignee: | Dag-Erling Smørgrav <des> | ||||||||||
| Status: | Closed FIXED | ||||||||||||
| Severity: | Affects Some People | CC: | des, milosz.kaniewski | ||||||||||
| Priority: | --- | Keywords: | patch | ||||||||||
| Version: | CURRENT | Flags: | des:
mfc-stable10+
des: mfc-stable9+ |
||||||||||
| Hardware: | Any | ||||||||||||
| OS: | Any | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Fabian Keil
2014-10-20 11:55:28 UTC
Created attachment 162043 [details] Revised patch Hi Fabian, First, my apologies for not picking this up earlier. It was not assigned to me, and your cc: drowned in my inbox. For the record, this bug is trivial to reproduce: $ sudo pkg install privoxy $ privoxy /dev/null $ HTTP_PROXY=http://localhost:8118/ fetch -o/dev/null -v https://www.freebsd.org/ I have prepared a different patch that uses http_next_header() to iterate over the response instead of just reading until the buffer is empty (partly for cleanliness and partly because I was unable to convince myself that this would always work correctly). Could you please confirm that it works for you? Your patch works for me as well, thanks. While testing it I noticed that fetch's behaviour if the proxy rejects the request isn't ideal, but this is unrelated to your patch (and mine obviously did not address this either): # fetch gets 403 from the proxy, fails as expected, # but does not mention the cause. fk@r500 ~ $fetch -v https://ads.example.org/ looking up 127.0.1.1 connecting to 127.0.1.1:8118 fetch: https://ads.example.org/: fk@r500 ~ $echo $? 1 Created attachment 162113 [details]
Revised² patch
That's because http_connect() is missing a call to http_seterr(). Try the new patch.
Thanks for the quick update. Works for me: fk@r500 ~ $fetch -v https://ads.example.org/ looking up 127.0.1.1 connecting to 127.0.1.1:8118 fetch: https://ads.example.org/: Forbidden A commit references this bug: Author: des Date: Fri Oct 16 12:21:44 UTC 2015 New revision: 289419 URL: https://svnweb.freebsd.org/changeset/base/289419 Log: Fix two bugs in HTTPS tunnelling: - If the proxy returns a non-200 result, set the error code accordingly so the caller / user gets a somewhat meaningful error message. - Consume and discard any HTTP response header following the result line. PR: 194483 Tested by: Fabian Keil <fk@fabiankeil.de> MFC after: 1 week Changes: head/lib/libfetch/http.c A commit references this bug: Author: des Date: Sat Jan 16 20:24:03 UTC 2016 New revision: 294194 URL: https://svnweb.freebsd.org/changeset/base/294194 Log: MFH (r280630): remove all traces of SSLv2 support MFH (r285141): remove unused variable MFH (r288217): correctly check return value from getaddrinfo(3) MFH (r289419): fix bugs in HTTPS tunnelling MFH (r289420): use fopen()'s "e" mode instead of fcntl for close-on-exec MFH (r291453, r291461): use .netrc for http servers and proxies MFH (r292330, r292332): reset bufpos to 0 after refilling in chunked mode PR: 194483 199801 193740 204771 Changes: _U stable/10/ stable/10/lib/libfetch/common.c stable/10/lib/libfetch/fetch.3 stable/10/lib/libfetch/file.c stable/10/lib/libfetch/http.c stable/10/usr.bin/fetch/fetch.1 stable/10/usr.bin/fetch/fetch.c A commit references this bug: Author: des Date: Tue Jan 26 07:44:27 UTC 2016 New revision: 294776 URL: https://svnweb.freebsd.org/changeset/base/294776 Log: MFH (r261233): cleanup MFH (r261234): increase buffer size MFH (r280630): remove all traces of SSLv2 support MFH (r285141): remove unused variable MFH (r288217): correctly check return value from getaddrinfo(3) MFH (r289419): fix bugs in HTTPS tunnelling MFH (r289420): use fopen()'s "e" mode instead of fcntl for close-on-exec MFH (r291453, r291461): use .netrc for http servers and proxies MFH (r292330, r292332): reset bufpos to 0 after refilling in chunked mode PR: 194483 199801 193740 204771 Changes: _U stable/9/ _U stable/9/lib/ _U stable/9/lib/libfetch/ stable/9/lib/libfetch/common.c stable/9/lib/libfetch/fetch.3 stable/9/lib/libfetch/file.c stable/9/lib/libfetch/http.c _U stable/9/usr.bin/ _U stable/9/usr.bin/fetch/ stable/9/usr.bin/fetch/fetch.1 stable/9/usr.bin/fetch/fetch.c Created attachment 175412 [details]
Correct while statement.
Hi,
I think there is a typo in a while statement (see patch for details). For only one header it worked well but if there is more headers (in my case there were two of them) then problem described in bug description appeared again.
A commit references this bug: Author: des Date: Fri Dec 30 14:54:54 UTC 2016 New revision: 310823 URL: https://svnweb.freebsd.org/changeset/base/310823 Log: Fix inverted loop condition which broke multi-line responses to CONNECT. PR: 194483 Submitted by: Mi?osz Kaniewski <milosz.kaniewski@gmail.com> MFC after: 1 week Changes: head/lib/libfetch/http.c A commit references this bug: Author: des Date: Mon Jan 9 14:13:47 UTC 2017 New revision: 311786 URL: https://svnweb.freebsd.org/changeset/base/311786 Log: MFH (r310823): fix multi-line CONNECT responses PR: 194483 Changes: _U stable/11/ stable/11/lib/libfetch/http.c A commit references this bug: Author: des Date: Tue Jan 10 08:12:56 UTC 2017 New revision: 311864 URL: https://svnweb.freebsd.org/changeset/base/311864 Log: MFH (r301027): fix 307 / 308 redirects MFH (r310823): fix multi-line CONNECT responses PR: 112515 173451 194483 209546 Changes: _U stable/10/ stable/10/lib/libfetch/http.c |
