Use a unified cache for SSL_CTX objects#112567
Conversation
rzikm
left a comment
There was a problem hiding this comment.
/azp run runtime-libraries-coreclr outerloop
src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs
Outdated
Show resolved
Hide resolved
|
Edit: the above change actually breaks a different scenario, and the code should've worked already, will try to dig more as to why it does not work now. Edit 2: Got it, since we are using "Dispose" as "decrease reference by 1", GC finalization get's suppressed (inside SafeHandle.Dispose()) and finalizer never runs for the safe handles abandoned in the @wfurt any opinions there? I lean towards keeping the current change (this PR), as it simplifies the code and removes the IMO not-well documented requirement on reusing cert context for TLS Resumption. |
|
/azp run runtime-libraries-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
I can live with ether option. Originally, I did not want global objects as the management becomes more complicated IMHO and using the context allowed easy and natural partitioning. We already use weak Certificate.Thumbprint and this change makes it more vulnerable IMHO. I do see benefit of making the resume scenario available in other cases. While the resume behavior is not well documented (and keeps changing) I also feel the using the context is the best way and we should promote it regardless. |
As of #112193, we use SHA512 which should be good for the time being.
Agreed In that case, please review this PR when you have time. |
Fixes #110803.
Regression from #102656, This also simplifies the internal code a bit and removes the requirement to always use SslStreamCertificateContext on server side to benefit from TLS Resumption.