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@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev


_______________________________________________
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev

Reply via email to