[squid-dev] Squid 5.6 leaking memory when peeking for an origin with an invalid certificate

[Hamilton Coutinho]

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 <
rousskov at 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 at lists.squid-cache.org
> http://lists.squid-cache.org/listinfo/squid-dev
>


-- 
Hamilton
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20230118/317ac053/attachment.htm>