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

[Tsantilas Christos]

On 02/12/2015 01:48 PM, Amos Jeffries wrote:
> 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 ?

Do you mean the "if (csd->sslBumpMode == Ssl::bumpPeek || 
csd->sslBumpMode == Ssl::bumpStare)" ?

Yes the  "else" never gets run when peek or stare mode selected in 
bumpStep2 bumping step.



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

Requires a code like the following:
Must(csd->sslServerBump()->step<=Ssl::bumpStep2);

This is will not work. In client-first bumping mode or in server-first 
bumping mode where we are not applying the peek-and-splice procedure the 
step is not updated.

Also my sense is that in client-first bumping mode the 
csd->sslServerBump() is NULL.


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

OK, I will try to add some explanations  in commit message.


>
>
> Amos