ghost
commented
4 years ago Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @jeffhandley See info in area-owners.md if you want to be subscribed.
Closed egraff closed 2 years ago
ghost
commented
4 years ago Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @jeffhandley See info in area-owners.md if you want to be subscribed.
vcsjones
commented
4 years ago /cc @wfurt
wfurt
commented
4 years ago yes, I saw this @vcsjones. The cache is internal to SslStream. We would need to find expiration for entire chain and attache that to cached entries. However, do we know that the chain build with multiple roots would work?
The cache is there for ages and adds complexity. But I don't know how much it improves perf. It may be worth of measuring perf without it.
egraff
commented
4 years ago However, do we know that the chain build with multiple roots would work?
@wfurt If I understand your question correctly, this is part of what my test code shows.
Before the old CA root expires, X509Chain.Build() returns this chain:
Testing chain locally...
0: CN=some.server.invalid, 27.10.2018 08:21:00 +00:00 - 27.10.2030 08:21:00 +00:00
1: CN=TEST_CERT_Second_Intermediate_CA, 27.10.2018 08:21:00 +00:00 - 27.10.2030 08:21:00 +00:00
2: CN=TEST_CERT_New_root_CA, 27.10.2010 08:21:00 +00:00 - 27.10.2020 08:23:00 +00:00
3: CN=TEST_CERT_Old_root_CA, 27.10.2010 08:21:00 +00:00 - 27.10.2020 08:23:00 +00:00and after it expired, it returns this chain:
Building new chain explicitly...
0: CN=some.server.invalid, 27.10.2018 08:21:00 +00:00 - 27.10.2030 08:21:00 +00:00
1: CN=TEST_CERT_Second_Intermediate_CA, 27.10.2018 08:21:00 +00:00 - 27.10.2030 08:21:00 +00:00
2: CN=TEST_CERT_New_root_CA, 27.10.2000 08:21:00 +00:00 - 27.10.2040 08:21:00 +00:00If it's still the case in .NET Core that all of the chain building on Windows is left up to the CryptoAPI (i.e. CertGetCertificateChain), then I believe it should handle multiple roots fine. However, I've also opened #43884 for problems with handling multiple roots when using openssl as the crypto back-end.
ghost
commented
3 years ago Tagging subscribers to this area: @dotnet/ncl, @vcsjones See info in area-owners.md if you want to be subscribed.
| Author: | egraff |
|---|---|
| Assignees: | - |
| Labels: | `area-System.Net.Security`, `untriaged` |
| Milestone: | - |
rzikm
commented
2 years ago This is the behavior I observe when I disable the credential caching (so that for each connection, new credentials are acquired)
❯ gsudo dotnet .\Sandbox.dll
Testing chain locally...
0: CN=some.server.invalid, 2020-03-08 11:58:02 +00:00 - 2032-03-08 11:58:02 +00:00
1: CN=TEST_CERT_Second_Intermediate_CA, 2020-03-08 11:58:02 +00:00 - 2032-03-08 11:58:02 +00:00
2: CN=TEST_CERT_New_root_CA, 2012-03-08 11:58:02 +00:00 - 2022-03-08 12:00:02 +00:00
3: CN=TEST_CERT_Old_root_CA, 2012-03-08 11:58:02 +00:00 - 2022-03-08 12:00:02 +00:00
Starting test...
Chain #0, time is now: 2022-03-08 11:58:05 +00:00
0: CN=some.server.invalid, 2020-03-08 11:58:02 +00:00 - 2032-03-08 11:58:02 +00:00
1: CN=TEST_CERT_Second_Intermediate_CA, 2020-03-08 11:58:02 +00:00 - 2032-03-08 11:58:02 +00:00
2: CN=TEST_CERT_New_root_CA, 2012-03-08 11:58:02 +00:00 - 2022-03-08 12:00:02 +00:00
-------
Chain #1, time is now: 2022-03-08 11:58:36 +00:00
0: CN=some.server.invalid, 2020-03-08 11:58:02 +00:00 - 2032-03-08 11:58:02 +00:00
1: CN=TEST_CERT_Second_Intermediate_CA, 2020-03-08 11:58:02 +00:00 - 2032-03-08 11:58:02 +00:00
2: CN=TEST_CERT_New_root_CA, 2012-03-08 11:58:02 +00:00 - 2022-03-08 12:00:02 +00:00
-------
Chain #2, time is now: 2022-03-08 11:59:06 +00:00
0: CN=some.server.invalid, 2020-03-08 11:58:02 +00:00 - 2032-03-08 11:58:02 +00:00
1: CN=TEST_CERT_Second_Intermediate_CA, 2020-03-08 11:58:02 +00:00 - 2032-03-08 11:58:02 +00:00
2: CN=TEST_CERT_New_root_CA, 2012-03-08 11:58:02 +00:00 - 2022-03-08 12:00:02 +00:00
-------
Chain #3, time is now: 2022-03-08 11:59:36 +00:00
0: CN=some.server.invalid, 2020-03-08 11:58:02 +00:00 - 2032-03-08 11:58:02 +00:00
1: CN=TEST_CERT_Second_Intermediate_CA, 2020-03-08 11:58:02 +00:00 - 2032-03-08 11:58:02 +00:00
2: CN=TEST_CERT_New_root_CA, 2012-03-08 11:58:02 +00:00 - 2022-03-08 12:00:02 +00:00
-------
Chain #4, time is now: 2022-03-08 12:00:06 +00:00
0: CN=some.server.invalid, 2020-03-08 11:58:02 +00:00 - 2032-03-08 11:58:02 +00:00
1: CN=TEST_CERT_Second_Intermediate_CA, 2020-03-08 11:58:02 +00:00 - 2032-03-08 11:58:02 +00:00
-------
Chain #5, time is now: 2022-03-08 12:00:36 +00:00
0: CN=some.server.invalid, 2020-03-08 11:58:02 +00:00 - 2032-03-08 11:58:02 +00:00
1: CN=TEST_CERT_Second_Intermediate_CA, 2020-03-08 11:58:02 +00:00 - 2032-03-08 11:58:02 +00:00
-------
Chain #6, time is now: 2022-03-08 12:01:06 +00:00
0: CN=some.server.invalid, 2020-03-08 11:58:02 +00:00 - 2032-03-08 11:58:02 +00:00
1: CN=TEST_CERT_Second_Intermediate_CA, 2020-03-08 11:58:02 +00:00 - 2032-03-08 11:58:02 +00:00
-------
Building new chain explicitly...
0: CN=some.server.invalid, 2020-03-08 11:58:02 +00:00 - 2032-03-08 11:58:02 +00:00
1: CN=TEST_CERT_Second_Intermediate_CA, 2020-03-08 11:58:02 +00:00 - 2032-03-08 11:58:02 +00:00
2: CN=TEST_CERT_New_root_CA, 2002-03-08 11:58:02 +00:00 - 2042-03-08 11:58:02 +00:00So refreshing the creds certainly solves the problem.
Furthermore, I noticed that AcquireCredentialsHandle returns an expiry timestamp, which in this repro returned 2032-03-08 11:58:02. I tinkered with the dates a bit and it seems to me that it always corresponds to the NotAfter of the leaf certificate (maybe the implementation assumes that other certificates don't expire before due to validity period nesting?), so it does not really help us with this issue :/
Seems like that in order to fix this, we need to build the entire chain and manually find the earliest expiry date. I will try it and see how much of a perf hit that will be.
Edit: looks like we already build a cert chain and the result is conveniently accessible on SslAuthenticationOptions.CertificateContext. That looks promising.
Description
The static
SslSessionsCacheused bySecureChannel-- which in turn is used bySslStream-- to cacheSCHANNEL_CREDhandles is problematic, because the certificate chain for the end-entity certificate used bySslStreamis only built when theSCHANNEL_CREDhandle is first acquired, through a call toAcquireCredentialsHandle. If the root CA certificate or one of the intermediate CA certificates expire (this actually happened IRL quite recently), then allSslStreaminstances created for the rest of the entire lifetime of the process for the same end-entity certificate (for example a Kestrel-based ASP.Net Core web server) will continue to use the expired certificate chain.Configuration
Seen on .NET on Windows. I have reproduced the problem on .NET Framework 4.8, .NET Core 2.1 and .NET Core 3.1.
Detailed reproduction
The USERTrust intermediate certificate in this chain is a cross-signed CA, which also has a valid root CA certificate, so the expected certificate chain to be returned from the server after the AddTrust certificate expired would be this:
I have reproduced this issue synthetically, with a stand-alone .NET executable. The application generates the following certificates:
where
Old first intermediatehas the same subject name and private key asNew root CA(i.e. the CA is cross-signed byOld root CA).Old root CAandOld first intermediateare set to expire in 2 minutes, but the notBefore date is set to be more recent thanNew root CAto trick CryptoAPI to prefer that certificate chain initially. After this is done, the root and intermediate CA certificates are installed in the system's certificate stores.After the certificates are setup, the test first builds a certificate chain explicitly, using X509Chain.Build(). Then, the test runs a loop where it creates a new web server
SslStreamfor each iteration, and logs the certificate chain that is received by a connecting web client (note that the root certificate is not included, as it is not part of the TLS Certificate message). There is a 30 seconds sleep between each loop iteration. Finally, after all the loop iterations have run, the test builds another certificate chain explicitly, again using X509Chain.Build().Expand to see test code included below:
```csharp namespace SslStreamCertChainProblemExternalRepro { using System; using System.Collections.Generic; using System.Globalization; using System.Linq; using System.Net; using System.Net.Security; using System.Net.Sockets; using System.Security.Authentication; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using System.Threading; using System.Threading.Tasks; using Org.BouncyCastle.Crypto.Tls; using Org.BouncyCastle.Security; using CertificateRequest = System.Security.Cryptography.X509Certificates.CertificateRequest; internal class TlsClient : DefaultTlsClient { private readonly ActionNote that the test application must be run as Administrator on Windows, because it installs (and afterwards removes) certificates into the trusted root CA store of the machine. Also, it depends on the external Portable.BouncyCastle NuGet package (I've used version 1.8.8), because there are no native .NET abstractions that can be used to log the certificates that are sent in a TLS Certificate message (
HttpClientusesSslStreamwhich will build a certificate chain from the certificates in the TLS message).The test code produces the following output:
As can be seen, the old root certificate (
CN=TEST_CERT_Old_root_CA) and its first intermediate (CN=TEST_CERT_New_root_CA) both expire @ 27.10.2020 08:23:00 +00:00, yet the web client receives certificate chains containing these certificates even after they expire (chain iterations 4 - 6). After the certificates expire, building a new chain usingX509Chain.Build()returns the expected chain, though.Conclusion
By using the Visual Studio debugger with just-my-code debugging turned off, I have been able to set breakpoints in
SecureChannel.csandSslSessionsCache.cs, and verify that newSslStreamobjects are indeed reusing the sameSCHANNEL_CREDhandle for the same end-entity certificate due to the static credential cache. From the documentation ofAcquireCredentialsHandle, it seems the certificate chain associated with the handle is built whenAcquireCredentialsHandleis called, which in this case only happens the first time theSCHANNEL_CREDhandle is created. Therefore, I believe this to be the culprit of the problem.