Fix disposing root X.509 certificate prematurely for OCSP stapling #82116
Conversation
This defers disposal of the root certificate when it might be needed for OCSP staple fetching. Before this change, the root certificate would get disposed, giving it a null handle. We would then use this null handle when attempting to build an OCSP request, which OpenSSL would null-deref. For platforms that don't need the root certificate, they dispose of it.
Even though we've fixed the null pointer being passed now, add some defenses so that if we are somehow given a null handle some time in the future, we skip OCSP stapling instead of null derefing in native code.
ghost
assigned 
|
Tagging subscribers to this area: @dotnet/ncl, @vcsjones Issue DetailsThis fixes an issue where we were capturing the root certificate for OCSP stapling purposes, however would dispose of the certificate when we needed it. This resulted in a null This PR addresses this in two ways. First, it stops disposing of the root certificate when it is needed. Some platforms and Linux situations don't need the root, and in that scenario, it gets disposed. The second is to actually check the Lastly, this adds tests for two-element chains. Prior to the fix, the tests would segfault as reported by the customer. Contributes to #81964
|
|
@bartonjs this slightly deviates from our discussion earlier, and I'll attempt to explain why. First, with this approach we don't need to duplicate the certificate as we previously discussed. Instead, the platform decides if it cares to keep the root or not. Second, we can't do |
...libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs
Show resolved
Hide resolved
src/libraries/System.Net.Security/tests/FunctionalTests/CertificateValidationRemoteServer.cs
Show resolved
Hide resolved
src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.cs
Outdated
Show resolved
Hide resolved
|
/azp run runtime-libraries-coreclr outerloop-linux |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
I only ran the outerloop tests for linux since the test and changes only affect Linux. The failures in outerloop look irrelevant to me, as the two new test cases passed in all legs. |
src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.cs
Show resolved
Hide resolved
|
/backport to release/7.0 |
|
Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/4199504897 |
Fixes #81964
This fixes an issue where we were capturing the root certificate for OCSP stapling purposes, however would dispose of the certificate when we needed it. This resulted in a null
Handle, which would be fed in to OpenSSL and cause a null deref in native code, crashing the .NET process. This scenario only occurred in two-element certificate chains. If there was an intermediate in the chain, the issue does not occur.This PR addresses this in two ways. First, it stops disposing of the root certificate when it is needed. Some platforms and Linux situations don't need the root, and in that scenario, it gets disposed. The second is to actually check the
Handleprior to handing it off to native code with anAssert. Additionally, in release configurations, this will simply exit the OCSP stapling and return a null response, and further prevent future staples for happening in that context.Lastly, this adds tests for two-element chains. Prior to the fix, the tests would segfault as reported by the customer.
Contributes to #81964