[squid-users] LEGACY_SERVER_CONNECT, ALLOW_UNSAFE_LEGACY_RENEGOTIATION does not work - SSL bump, OpenSSL 3

[Alex Rousskov]

On 12/29/22 20:05, Amish wrote:
> On 29/12/22 22:32, Alex Rousskov wrote:
>> On 12/29/22 10:41, Amish wrote:
>>> On 29/12/22 20:23, Alex Rousskov wrote:
>>>> There are several ways to fix this bug long-term, including these two:
>>>>
>>>> Minimal: Create a TLS context object dedicated to peeking at origin 
>>>> servers. It will probably have to be admin-configurable to 
>>>> accomodate various TLS v1.2 (and earlier) corner cases, but we can 
>>>> try to start without adding support for such configuration. Continue 
>>>> to use the existing configurable context for staring and other needs 
>>>> but call createClientContext(true) for that existing context.

> I do not know what createClientContext() actually does. I thought it was 
> just a way to test your theory.

Security::PeerOptions::createClientContext() creates the client context 
object we talked about. Its boolean parameter determines whether the new 
context object will honor the corresponding options=... part of its 
configuration.


> If you tell me the starting point then I may try to look at the code and 
> try to implement your minimal way to fix it.

Even the minimal change is not trivial; FWIW, I do not recommend working 
on this if you do not enjoy tinkering with messy code or if you are not 
willing to carefully test various ssl_bump configuration scenarios.

I have not verified/tested any of this, but I would start with these steps:

1. Find ssl_client in SquidConfig.h and add a peekingContext data member 
next to the existing sslContext data member. The two fields should have 
the same Security::ContextPointer type.

2. Examine every Config.ssl_client.sslContext use in the existing code 
("git grep -n ssl_client.sslContext") and adjust those that need an 
adjustment:

* In the code where you edited the createClientContext(false) call, add 
a new line there to initialize the new Config.ssl_client.peekingContext 
pointer with something like 
Security::PeerOptions().createClientContext(false) call (dictated by the 
minimal configuration-free approach outlined above).

The old Config.ssl_client.sslContext field should be initialized with a 
Security::ProxyOutgoingConfig.createClientContext(true) call that you 
have tested already.

* configFreeMemory() will need to reset peekingContext as well.

* In peeking and splicing cases, 
Ssl::PeekingPeerConnector::getTlsContext() should return the new 
peekingContext instead of the old Config.ssl_client.sslContext. I am not 
sure, but it is possible that this method needs to examine sslBumpMode 
like Ssl::PeekingPeerConnector::initialize() does (and default to 
peeking/old context when sslBumpMode is not available). In other words, 
return the new peekingContext only when sslBumpMode is reachable and is 
equal to Ssl::bumpPeek or Ssl::bumpSplice.


You will find some git recipes at 
https://wiki.squid-cache.org/DeveloperResources/GitHints

When you think your code is ready to become official, use 
https://wiki.squid-cache.org/MergeProcedure


> In the mean time, do you think this bug needs to be reported as an issue 
> on Github? I can do that.

We do not use GitHub Issues. We use Squid Bugzilla at 
https://bugs.squid-cache.org

It is probably a good idea to report a "tls_outgoing_options options=... 
are ignored" bug there, with a reference to this email thread.


Thank you,

Alex.