On 19/01/2013 11:53 a.m., Alex Rousskov wrote:
On 01/18/2013 05:32 AM, Amos Jeffries wrote:
On 17/01/2013 6:47 a.m., Alex Rousskov wrote:
Hello,
clientProcessRequest() assumes that a CONNECT request is always
tunneled and sets flags.readMore to false. However, if url_rewriter
redirects the CONNECTing user, Squid responds with a redirect message
and does not tunnel. In that case, we do want to read more. Otherwise,
keepaliveNextRequest() will declare the connection abandoned and the
connection descriptor will "leak" until the connection lifetime expires,
even if the client disconnects immediately after receiving the redirect
response.
The fix delays setting flags.readMore to false until we are about to
call tunnelStart().
The effect on CONNECT authentication (another case where CONNECT is not
tunneled) is untested, but I hope that code continues to work because it
should be OK with reading more requests on the [being] authenticated
connection.
These changes may also fix other similar not-tunneled CONNECT cases.
They are not related to SslBump.
One of the attached patches is for trunk. The other one is for v3.2. I
did not check whether the problem exists in v3.1.
HTH,
Alex.
+1. It looks okay, although I would like confirmation from somebody
using NTLM authentication that things still work afterwards.
My patches broke SslBump -- Squid started reading from the client
connection before switching to SSL :-(.
The attached patches were tested with SslBump (and without). They are
more conservative/less invasive: I now leave readMore set to false for
CONNECT until it is clear that Squid is not going to tunnel or bump.
This should break fewer cases.
I think the correct way will be to take your first patch, but shuffle
the use of stopReading() above tunnelStart() to above both tunnelStart()
and sslBumpStart(), then add a readMore=true when sslBump setup is
completed.
If this works it confirms a suspicion of mine that sslBumpStart() should
be moved inside tunnelStart() and that function should be the place to
select between handling CONNECT as ssl-bump / Upgrade: / or blind tunnel.
Amos
