-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Description
SslStreamCertificateContext has two public fields in its implementation that aren't in the contract:
runtime/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.cs
Lines 11 to 12 in 617de07
| public readonly X509Certificate2 Certificate; | |
| public readonly X509Certificate2[] IntermediateCertificates; |
This was done for System.Net.Quic to be able to use these without actually making public API for them. But there are a variety of issues with this:
-
These kinds of ref vs src tricks are harder to maintain (our tooling for ensuring we don't break our contracts doesn't enforce such suppressed violations of the ref/src contract tooling). The fewer we have, the better.
-
Being public in the implementation means things like
dynamicwill by default allow access to them, making them accessible anyway but without tooling to guarantee we're not breaking them, e.g.
using System.Net;
using System.Net.Security;
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;
dynamic ctx = SslStreamCertificateContext.Create(CreateServerSelfSignedCertificate("localhost"), null);
Console.WriteLine(ctx.IntermediateCertificates.Length);
static X509Certificate2 CreateServerSelfSignedCertificate(string name)
{
using (RSA root = RSA.Create())
{
CertificateRequest req = new CertificateRequest($"CN={name}", root, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
req.CertificateExtensions.Add(new X509BasicConstraintsExtension(true, false, 0, true));
req.CertificateExtensions.Add(new X509SubjectKeyIdentifierExtension(req.PublicKey, false));
req.CertificateExtensions.Add(new X509KeyUsageExtension(X509KeyUsageFlags.DigitalSignature | X509KeyUsageFlags.KeyEncipherment | X509KeyUsageFlags.DataEncipherment, false));
req.CertificateExtensions.Add(new X509EnhancedKeyUsageExtension(new OidCollection() { new Oid("1.3.6.1.5.5.7.3.1", null), }, false));
SubjectAlternativeNameBuilder builder = new SubjectAlternativeNameBuilder();
builder.AddDnsName(name);
builder.AddIpAddress(IPAddress.Loopback);
builder.AddIpAddress(IPAddress.IPv6Loopback);
req.CertificateExtensions.Add(builder.Build());
DateTimeOffset start = DateTimeOffset.UtcNow.AddMinutes(-5);
DateTimeOffset end = start.AddMonths(1);
X509Certificate2 cert = req.CreateSelfSigned(start, end);
if (OperatingSystem.IsWindows())
{
cert = new X509Certificate2(cert.Export(X509ContentType.Pfx));
}
return cert;
}
}-
If both SslStream and Quic need access, that's an indicator other protocols will as well.
-
There's currently no way to properly clean up after an SslStreamCertificateContext. It's not disposable and this state isn't exposed, which means everything created (e.g. all the intermediate certs) and stored is left for finalization when one is no longer used. Thankfully this shouldn't be a big deal for most servers, where there should be relatively few instances that are all long-lived.
We should consider designing real public API around these.