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

[Christos Tsantilas]

The patch applied to squid-5 as r15020 with the fixes suggested by Alex.

I am attaching the equivalent patch for squid-3.5.

On 25/01/2017 11:42 μμ, Alex Rousskov wrote:
> 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.
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: SQUID-241-SSL-v2-records-force-step2-bumping-squid-3.5-t4.patch
Type: text/x-patch
Size: 9035 bytes
Desc: not available
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20170126/33e48d4a/attachment.bin>