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

Thu Feb 12 15:51:33 UTC 2015

On 02/12/2015 05:33 PM, Amos Jeffries wrote:
> 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.

Exactly.
But why do you beleive this Must is needed?  Specially inside "else" is 
completely without any interest for development or debugging. Even if 
for a reason this is not set correctly, it will not cause any problem.

The csd->sslServerBump()->step is set to Ssl::bumpStep3 in a clearly 
later step, inside Ssl::PeerConnector::checkForPeekAndSplice method, 
which called from Ssl::PeerConnector::handleNegotiateError.

>
>
> Amos
> _______________________________________________
> squid-dev mailing list
> squid-dev at lists.squid-cache.org
> http://lists.squid-cache.org/listinfo/squid-dev
>