[squid-dev] [PATCH] splicing resumed sessions

[Amos Jeffries]

On 21/03/2015 10:47 a.m., Alex Rousskov wrote:
> On 03/20/2015 12:11 PM, Amos Jeffries wrote:
>> On 21/03/2015 4:35 a.m., Alex Rousskov wrote:
>>> On 03/20/2015 02:06 AM, Amos Jeffries wrote:
>>>> On 18/03/2015 6:21 a.m., Tsantilas Christos wrote:
>>>>> This patch adds the "ssl_bump_resuming_sessions" directive that controls
>>>>> SslBump behavior when dealing with "resuming SSL/TLS sessions". Without
>>>>> these changes, SslBump usually terminates all resuming sessions with an
>>>>> error because such sessions do not include server certificates,
>>>>> preventing Squid from successfully validating the server identity.
> 
> 
>> Squid is behaving wrong right now. Fix that.
> 
> Yes, the proposed patch "fixes that".
> 
> 
>> I dont see any need for the new directive to exist
> 
> We can remove the configuration option. The option was added to minimize
> the chance that you will object to the patch because, without the new
> option, the old "terminate" behavior would be lost. Clearly, my plan to
> avoid that discussion has failed! Removing the configuration option is
> totally fine with me because nobody has asked for it [yet]. We can
> always add it later if needed.
> 
> 
>> The validation I'm talking about is the spec requirement that we dont
>> allow the following scenario:
>>
>>   CONNECT example.com:443 HTTP/1.1
>>   ...
>>   ClientHello SNI:  example.org
> 
> 
> I still think this part of the discussion is out of scope, but is there
> really a spec requirement that talks about comparing CONNECT- and
> SNI-provided server names? Where? CONNECT is about blind TCP *tunnels*.
> SNI is about becoming an *end point* of a TLS connection. Those two
> things seem mutually exclusive to me.
> 
> 
>> * if the CONNECT and SNI both contain different domains the
>> specification says SSL "MUST terminate" with a specified reject error.
> 
> Which specification? RFC 2818 does not mention CONNECT. It contains some
> requirements for HTTP clients, but Squid is not an HTTP client in this
> issue scope; Squid is a TCP client.
> 

Sorry the specifics here are mostly in RFC 6066 section 3, the final two
paragraphs.
 http://tools.ietf.org/html/rfc6066#section-3
"
 If an application negotiates a server name using an application
 protocol and then upgrades to TLS, and if a server_name extension is
 sent, then the extension SHOULD contain the same name that was
 negotiated in the application protocol.  If the server_name is
 established in the TLS session handshake, the client SHOULD NOT
 attempt to request a different server name at the application layer.
"

 ie CONNECT with hostname contains the same value as SNI from the client.

RFC 2818 section 3.1 leads into that with its paragraph 2,
"
 If the client has external information as to the expected identity of
 the server, the hostname check MAY be omitted.
 ...
 In such cases, it is important to narrow the scope of
 acceptable certificates as much as possible in order to prevent man
 in the middle attacks.  In special cases, it may be appropriate for
 the client to simply ignore the server's identity, but it must be
 understood that this leaves the connection open to active attack.
"

such narrowing can be Squid doing the check that CONNECT == SNI and
aborting if not.
My emphasis is in doing what we can to avoid that risk indicated in the
final line.


> 
>>   - CONNECT having a domain means we are in the RFC 2818 scenario of
>> TLS-over-HTTP.
> 
> No, CONNECT means "establish a TCP tunnel by splicing client and server
> TCP connections together". CONNECT does not imply SSL or TLS. When Squid
> bumps a secure connection, then Squid becomes subject to various SSL and
> TLS rules. Here, we are not bumping anything, just splicing at TCP level.
> 

How then does one get into the situation of having a SSL ClientHello
with a SNI value ... for non-TLS ?


>> So ...
>>  if we allow an admin directive it would be to force terminate when
>> resume was otherwise allowed.
> 
> Yes.
> 
> 
>>  1) Do you have any existing need for that?
> 
> None other than avoiding this discussion, which has already failed to work.
> 
> 
>>  2) Would not "ssl_bump terminate blah" be a better way to express the
>> policy?
> 
> Probably not. We have considered that design, but most ssl_bump rules
> rely on information unavailable when sessions are resumed. If we force
> admins to use ssl_bump to control which resuming sessions are
> terminated, the ssl_bump rules become needlessly complex and difficult
> to write correctly.
> 
> 
>> I think we should fix the currently broken Squid behaviour without a new
>> directive.
> 
> Sounds good to me! If we do not hear otherwise, we will commit the
> proposed patch without the new [already optional] directive.
> 
> 
> Thank you,
> 
> Alex.
>