[squid-dev] [PATCH] SSLv2 records force SslBump bumping despite a matching step2 peek rule.

[Alex Rousskov]

On 01/25/2017 12:12 PM, Christos Tsantilas wrote:
>> On 25/01/2017 08:24 μμ, Alex Rousskov wrote:
>> * A client-sent ClientHello is required for peeking. The calling code
>> must ensure that we never get here without it. Throw if our calling code
>> is buggy.

> This is the correct.

Great. I have no objections to a commit then.

I suggest replacing the "client-sent ClientHello must be available for
peek" comment with "we should not be here if we failed to parse the
client-sent ClientHello" to clarify, but this is obviously minor.


> Now Squid peeks (or stares) at the origin server as configured, even if it
> does not fully support the client SSL/TLS record/message.

Perhaps the vagueness of that "does not fully support" phrase drives
some of my questions. Initially, I probably thought that Squid cannot
even parse a TLS Hello in an SSLv2 record. And that failure to parse
triggered the wrong code path in ServerBio::write().

Now I think that we actually parse it just fine and the buggy code was
just too aggressive in reacting to the wrong version (which was already
flagged by the "Should we allow it for all protocols?" comment).

If we ever get to step3, OpenSSL may not be able to successfully
negotiate an SSL connection with a client that is using SSLv2 records.
That potential bumping failure during step3 is not a problem we are
solving here, and it is not a problem in pure peek-and-splice setups
because they do not normally bump and, hence, never ask OpenSSL to
negotiate using SSLv2 records.

Please correct my understanding or detail the commit message a little to
confirm it. For example, we could say "Now Squid peeks (or stares) at
the origin server as configured, even after detecting (and parsing)
SSLv2 records".


Thank you,

Alex.