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.
_______________________________________________
squid-users mailing list
squid-users@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-users
