Re: [PATCH] detaching sslbump from accel mode

 On Wed, 07 Dec 2011 18:59:15 +0200, Tsantilas Christos wrote:
> On 12/07/2011 01:19 PM, Amos Jeffries wrote:
>> So, in theory ssl-bump is a form of intercept not a form of
>> accelerator. Its use of prepareAcceleratorURL() to convert the
>> partial
>> to absolute URL unconditionally sets the accel flag with a mix of
>> side
>> effects. Some bad ones have been identified already.
>>
>> This patch changes the flag setting, to allow ssl-bump to use the
>> URL
>> preparation function without the side effects. I'm in half a mind to
>> make a ssl-bump specific URL preparation function, but only after
>> this
>> is proven workable.
>>
>> Christos: as the person who appears to have the best testing ability
>> for
>> ssl-bump can you run your tests over the resulting Squid and check
>> that
>> the expected behaviours have not changed for the worse? I am fully
>> expecting there to be several as yet unknown places needing to add a
>> test of the sslBumped flag alongside testing accel flag.
>
> Looks OK.
> Also because accel and related flags are used in http authentication
> and
> esi, I think too it is better to detach it from sslbumped requests,
> where we should handle authentication with a different way...
>
> The only objection is for the Config.onoff.ie_refresh parameter.
> Look in the client_side_request.cc line 1027 inside block:
> if (Config.onoff.ie_refresh) {
> }
>
> But does not look important....

 Hmm, maybe. The docs around that code are incorrect "or transparent
 proxy" is not one of the cases it kicks in. Only (accel && ims).
 We can either drop it or need to add all of sslbump, intercept, tproxy,
 and accel flags.

 Since it is so screwed I think we can leave it for now and fix
 separately.

>
>>
>> I'm expecting this to fix the need for ssl-bump to configure
>> "always_direct allow" and for this to be the proper long-term fix
>> for
>> the bug 2519 status mixup in comment 52 (comment 53 has an adequate
>> workaround for the bug patch while this gets tested).
>
> Amos, this patch is not enough to replace the workaround in comment
> 53,
> the workaround required.

 Okay. Yes. (Going by the bug comments I agree, to get 403 we need both
 alterations).

 Thank you for the check. I have moved on to splitting the prepare*()
 function now.

 On the SSL intercept case we don't have any backup host:port that I can
 see easily. If it is saved somewhere please let me know.

 For now the order of hot:port locating is:
 - use bumped requests Host: header, else
 - use CONNECT authority-URL, (http->sslHostName contains the host:port
 from the CONNECT right?)
 - reject with "400 Invalid Request" (RFC 2616 mandated response on
 missing Host:)

 I have also added a test case to the transparent intercept case to
 default port-443 received traffic through this new https:// URL emitting
 function. port-80 and unknown ports through the old function emitting
 http:// URLs.

 Amos
Received on Wed Dec 07 2011 - 22:59:15 MST