[squid-dev] [PATCH] sslproxy_options in peek-and-splice mode
Tsantilas Christos
chtsanti at users.sourceforge.net
Fri Feb 13 10:52:43 UTC 2015
A new patch, which also adds a Must clause for bumping step in
Ssl::PeerConnector::initializeSsl method.
On 02/12/2015 06:43 PM, Amos Jeffries wrote:
> On 13/02/2015 4:51 a.m., ich also adds a Must clasTsantilas Christos wrote:
>> 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.
>
>
> To make certain that it *is* always operating right, both now and in
> future. We are playing with complex and security critical behaviour here.
>
> There is a quite real risk that IF this stops working properly in the
> way you described - such as after future redesign changes, that the TLS
> will fail in obscure ways. That is a long way from "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.
>>
>
> I assume that means you think the dont need to add the step change to
> make the Must() work. Good.
>
> Amos
> _______________________________________________
> squid-dev mailing list
> squid-dev at lists.squid-cache.org
> http://lists.squid-cache.org/listinfo/squid-dev
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-sslproxy_options-t3.patch
Type: text/x-patch
Size: 8165 bytes
Desc: not available
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20150213/a48068c2/attachment-0001.bin>
More information about the squid-dev
mailing list