[squid-users] Validation of IP address for SSL spliced connections

[Alex Rousskov]

On 2024-05-30 02:30, Rik Theys wrote:
> On 5/29/24 11:31 PM, Alex Rousskov wrote:
>> On 2024-05-29 17:06, Rik Theys wrote:
>>> On 5/29/24 5:29 PM, Alex Rousskov wrote:
>>>> On 2024-05-29 05:01, Rik Theys wrote:
>>>>> squid doesn't seem to validate that the IP address we're connecting 
>>>>> to is valid for the specified name in the SNI header?
>>>>
>>>> That observation matches my reading of Squid Host header forgery 
>>>> detection code which says "we do not yet handle CONNECT tunnels 
>>>> well, so ignore for them". To validate that theory, use 
>>>> "debug_options ALL,3" and look for "SECURITY ALERT: Host header 
>>>> forgery detected" messages in cache.log.
>>
>>> I've enabled this debug option, but I never see the security alert in 
>>> the logs. Maybe it was introduced in more recent versions? I'm 
>>> currently using Squid 5.5 that comes with Rocky Linux 9.4.
>>
>> The code logging "SECURITY ALERT: Host header forgery detected" 
>> messages is present in v5.5, but perhaps it is not triggered in that 
>> version (or even in modern/supported Squids) when I expect it to be 
>> triggered. Unfortunately, there are too many variables for me to 
>> predict what exactly went wrong in your particular test case without 
>> doing a lot more work (and I cannot volunteer to do that work right now).
> 
> Looking at https://wiki.squid-cache.org/KnowledgeBase/HostHeaderForgery, 
> it always seems to mention the Host header. It has no mention of 
> performing the same checks for the SNI value. Since we're peeking at the 
> request, we can't see the actual Host header being sent.

As Amos has explained, SslBump at step2 is supposed to relay TLS Client 
Hello information via fake CONNECT request headers. SNI should go into 
CONNECT Host header and CONNECT target pseudo-header. That fake CONNECT 
request should then be checked for forgery.

Whether all of the above actually happens is an open question. I bet a 
short answer is "no". I am not just being extra cautious here based on 
overall poor SslBump code quality! I believe there are "real bugs" on 
that code path because we have fixed some of them (and I hope to find 
the time to post a polished version of those fixes for the official 
review in the foreseeable future). For an example that fuels my 
concerns, see the following unofficial commit message:
https://github.com/measurement-factory/squid/commit/462aedcc


> I believe that for my use-case (only splice certain domains and prevent 
> connecting to a wrong IP address), there's currently no solution then.

I suspect that there is currently no solution that does not involve 
writing complex external ACL helpers or complex Squid code fixes.


>>> I guess that explains why if I add "%ssl::<cert_subject" to my 
>>> logformat for the access log, the field is always "-"?
>>
>> It may explain that, but other problems may lead to the same "no 
>> certificate" result as well, of course. You can kind of check by using 
>> stare/bump instead of peek/splice -- if you see certificate details 
>> logged in that bumping test, then it is more likely that Squid just 
>> does not get a plain text certificate in peeking configurations.
> 
> I've updated the configuration to use stare/bump instead. The field is 
> then indeed added to the log file. A curl request that forces the 
> connection to a different IP address then also fails because the 
> certificate isn't valid for the name. There's no mention of the Host 
> header not matching the IP address, but I assume that check comes after 
> the certificate check then.

In most cases, the forgery check should happen before the certificate 
check. I suspect that it does not happen at all in your test case.


HTH,

Alex.