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

[Amos Jeffries]

On 13/02/2015 3:34 a.m., Tsantilas Christos wrote:
> 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)" ?


I mean the:

     } else {
+        // Set client SSL options
+        SSL_set_options(ssl, ::Config.ssl_client.parsedOptions);
+


> 
> 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);

Specifically:

Must(
     !csd->sslServerBump() ||
     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.

Which would make step 0 or 1 for client-first right? that is fine.

For server-first it does need updating at the point the server is given
data. A jump right to step 3, or even a new "sslBumpStepServerFirst"
value at the end of the enum.


Amos