Re: [squid-dev] Squid 5.6 leaking memory when peeking for an origin with an invalid certificate
Hi Alex,
Thanks for the prompt reply! Thanks also for the clarifications.
Agreed, I just realized the requests seem to be failing
with Http::scServiceUnavailable, so my focus turned
to Security::PeerConnector::sslCrtvdHandleReply() and friends.
Best.
On Wed, Jan 18, 2023 at 11:11 AM Alex Rousskov <
rouss...@measurement-factory.com> wrote:
> On 1/18/23 13:46, Hamilton Coutinho wrote:
> > Hi all,
> >
> > We are observing what seems to be several objects leaking in the output
> > mgr:mem, to the tune of 10s of 1000s
> >
> of HttpRequest, HttpHeaderEntry, Comm::Connection, Security::ErrorDetail,
> cbdata
> PeekingPeerConnector (31), etc.
> >
> > We dumped a core and managed to find some HttpRequest objects and they
> > all seem to have failed in the same way, with an ERR_SECURE_CONNECT_FAIL
> > category, for a site that has a certificate signed by a CA authority not
> > available to squid.
> >
> > If I would guess, the origin of the problem might be in
> > Ssl::PeekingPeerConnector::checkForPeekAndSpliceMatched():
> >
> > if (finalAction == Ssl::bumpTerminate) {
> > bail(new ErrorState(ERR_SECURE_CONNECT_FAIL, Http::scForbidden,
> > request.getRaw(), al));
> > clientConn->close();
> > clientConn = nullptr;
> >
> > Wondering if assigning null to clientConn there would be premature.
>
>
> FWIW, that connection pointer reset itself looks OK to me. ConnStateData
> and/or others should have a connection closure handler attached to the
> clientConn descriptor. That handler should be notified by Comm and
> initiate cleanup of the objects responsible for client-Squid communication.
>
> The bail() call above should inform the requestor about the
> error/termination and terminate this AsyncJob. That requestor should
> then close the Squid-server connection and clean up associated state.
>
> While there may be bugs in those "should..." sequences, please note that
> the pasted code is not related to handling of untrusted origin servers
> (unless your ssl_bump rules specifically activate the terminate action
> upon discovering such an origin server). The pasted code is reacting to
> an "ssl_bump terminate" rule matching.
>
>
> Cheers,
>
> Alex.
>
> ___
> squid-dev mailing list
> squid-dev@lists.squid-cache.org
> http://lists.squid-cache.org/listinfo/squid-dev
>
--
Hamilton
___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
Re: [squid-dev] Squid 5.6 leaking memory when peeking for an origin with an invalid certificate
On 1/18/23 13:46, Hamilton Coutinho wrote:
Hi all,
We are observing what seems to be several objects leaking in the output
mgr:mem, to the tune of 10s of 1000s
of HttpRequest, HttpHeaderEntry, Comm::Connection, Security::ErrorDetail, cbdata PeekingPeerConnector (31), etc.
We dumped a core and managed to find some HttpRequest objects and they
all seem to have failed in the same way, with an ERR_SECURE_CONNECT_FAIL
category, for a site that has a certificate signed by a CA authority not
available to squid.
If I would guess, the origin of the problem might be in
Ssl::PeekingPeerConnector::checkForPeekAndSpliceMatched():
if (finalAction == Ssl::bumpTerminate) {
bail(new ErrorState(ERR_SECURE_CONNECT_FAIL, Http::scForbidden,
request.getRaw(), al));
clientConn->close();
clientConn = nullptr;
Wondering if assigning null to clientConn there would be premature.
FWIW, that connection pointer reset itself looks OK to me. ConnStateData
and/or others should have a connection closure handler attached to the
clientConn descriptor. That handler should be notified by Comm and
initiate cleanup of the objects responsible for client-Squid communication.
The bail() call above should inform the requestor about the
error/termination and terminate this AsyncJob. That requestor should
then close the Squid-server connection and clean up associated state.
While there may be bugs in those "should..." sequences, please note that
the pasted code is not related to handling of untrusted origin servers
(unless your ssl_bump rules specifically activate the terminate action
upon discovering such an origin server). The pasted code is reacting to
an "ssl_bump terminate" rule matching.
Cheers,
Alex.
___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
[squid-dev] Squid 5.6 leaking memory when peeking for an origin with an invalid certificate
Hi all,
We are observing what seems to be several objects leaking in the output
mgr:mem, to the tune of 10s of 1000s
of HttpRequest, HttpHeaderEntry, Comm::Connection,
Security::ErrorDetail, cbdata
PeekingPeerConnector (31), etc.
We dumped a core and managed to find some HttpRequest objects and they all
seem to have failed in the same way, with an ERR_SECURE_CONNECT_FAIL
category, for a site that has a certificate signed by a CA authority not
available to squid.
If I would guess, the origin of the problem might be in
Ssl::PeekingPeerConnector::checkForPeekAndSpliceMatched():
if (finalAction == Ssl::bumpTerminate) {
bail(new ErrorState(ERR_SECURE_CONNECT_FAIL, Http::scForbidden,
request.getRaw(), al));
clientConn->close();
clientConn = nullptr;
Wondering if assigning null to clientConn there would be premature.
Any thoughts?
Thanks!
--
Hamilton
___
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
