[squid-dev] [PATCH] sslproxy_options in peek-and-splice mode

[Amos Jeffries]

On 12/02/2015 11:31 p.m., Tsantilas Christos wrote:
> On 02/11/2015 09:48 PM, Amos Jeffries wrote:
>> On 12/02/2015 12:45 a.m., Tsantilas Christos wrote:
>>> On 02/11/2015 01:54 AM, Amos Jeffries wrote:
>>>> On 9/02/2015 6:43 a.m., Tsantilas Christos wrote:
>>>>> Bug description:
>>>>>
>>>>>     - Squid sslproxy_options deny the use of TLSv1_2 SSL protocol:
>>>>>              sslproxy_options NO_TLSv1_2
>>>>>     - Squid uses peek mode for bumped connections.
>>>>>     - Web client sends an TLSv1_2 hello message and squid in peek
>>>>> mode,
>>>>> forwards the client hello message to server
>>>>>     - Web server respond with an TLSv1_2 hello message
>>>>>     - Squid while parsing server hello message aborts with an error
>>>>> because  sslproxy_options deny the use ot TLSv1_2 protocol.
>>>>>
>>>>> This patch fixes squid to ignore sslproxy_options in peek or stare
>>>>> bumping mode.
>>>>
>>>> As I understand it the action of applying the options to the context
>>>> removes from the context cipher references etc which are not possible.
>>>>
>>>> Since peek and stare are non-final states I can easily imagine that
>>>> OpenSSL library negotiates ciphers which the options would otherwise
>>>> prohibit. Then when the options get applied to the context it find
>>>> itself using an algorithm which does not exist.
>>>
>>> The context SSL_CTX objects are bases to create the SSL objects which
>>> are responsible for the negotiation with the other side (server in this
>>> case).
>>> The SSL created, inherits the options from CTXa nd we are adding our
>>> options to SSL object.
>>> The SSL library will use these options to build client hello message,
>>> parse server hello message and select algorithms/ciphers and other
>>> features to establish SSL connection.
>>>
>>> In my patch the options applied to the squid client SSL objects
>>> immediately after created, in the case of bump-server-first, or
>>> bump-client-first.
>>>
>>> In the cases of peek or stare we are not setting any options. This is
>>> because we are sending a hello message which is the same or similar with
>>> the client hello message, so we can not apply options. Else the peek or
>>> stare will fail...
>>>
>>
>> What I means is the code flow is roughly like this yes?
>>
>> 1) bump
>> 2) splice
>> 3) peek then bump
>> 4) stare then bump
>> 5) peek then splice
>> 6) stare then splice
>>
>> In which of those cases are the options set?
>>   all cases ending in bump or splice.
> 
> The important factor here is the bumping step.
> in bump case (1,3,4)
>  - if the decision is to bump in bumpStep1 or bumpStep2 then the squid
> SSL client options are set.
>  - If the decision for bumping taken in bumpStep3, the the squid SSL
> client options are NOT set.
> 
> in splice case (2,5 or 6) :
>  -  if we splice on bumpStep1 or on bumpStep2, then we are not using
> squid SSL client code at all, so the options does not play any  role on
> this case.
>  - If we splice on bumpStep3 we have use squid SSL client, but in this
> case the options are not set.
> 
> 
> After the bumpStep2 completed we have received the client hello message,
> but we do not sent any response (server hello). At the same time we are
> starting to initiate the connection to the SSL server. This is mean that
> we are going to set SSL client hello message which is depends on SSL
> client code. So:
>   - If we know that we will going to bump the connection, we can safely
> set SSL client options. This is because the squid SSL client will
> initiate a normal SSL connection.
>   - If we are going to stare, or peek then the client hello message we
> are going to sent must be similar to the client-to-squid SSL hello
> message. In this case we can not control SSL features using options,
> else the SSL negotiation will fail.

So you're saying the src/ssl/PeerConnector.cc else-condition never gets
run after a peek and/or stare ?

Then please add a Must(step <= 2) check at the start of that else
condition right before setting the SSL client options. If that works
properly I am happy for this to go in.

NP: the above explanation may be worth adding to the commit message as
an explanation for what the line "This patch fixes squid to ignore
sslproxy_options in peek or stare bumping mode" actually means.


Amos