[squid-dev] Host header forgery detection when peeking for SNI

[Christos Tsantilas]

On 10/25/2016 02:40 PM, Amos Jeffries wrote:
> On 25/10/2016 11:54 p.m., Dave Lewthwaite wrote:
>> Hi,
>>
>> We are running into an issue that has come up a few times on the mailing lists - host header forgery detection when using SSL peek in order to include SNI logging in access logs. (Clients operating in transparent mode).
>>
>> As far as I can tell I have narrowed it down to ClientRequestContext::hostHeaderVerifyFailed, there is a line -
>>
>> if (!Config.onoff.hostStrictVerify && http->request->method != Http::METHOD_CONNECT)
>>
>> Along with the comment "// NP: we do not yet handle CONNECT tunnels well, so ignore for them".
>>
>> If I remove the method check then the sites hitting this issue start loading fine, however, I don't know what the implications are of doing this - especially given the comment. (I do understand the implications of disabling host verification entirely).
>>
>> It's also worth noting that this still occurs even when both client and server are using the same DNS servers (although it's not as often) and clearly it is a problem that does occur in the real world.
>>
>> What is the impact of removing the method check so that this code path is used for CONNECT requests?
>
> The CONNECT handling code does not yet do the ORIGINAL_DST connection
> which other request types do.
>
> CONNECT requests can still be sent by clients over port 80 or 443 (not
> common, but possible) and are able to be used to bypass the proxy,
> browser and network security systems.
>
>
> I'm not yet sure what the handling in the SSL-Bump code needs to do in
> these cases. Any assistance with ideas or code welcome.

Probably required some code inside 
ClientRequestContext::hostHeaderVerify(), inside the block  " if 
(http->request->flags.intercepted||http->request->flags.interceptTproxy) 
{...}" to check if request is in sslbump mode and SNI is available.


>
> Amos