Add API to control trust during TLS handshake

[wfurt]

Background and Motivation

This was originally requested as #45456 (pri-1) to get parity with IIS/http.sys. After iterations with @bartonjs this was slightly extended to solve few more problems related to managing trust with TLS/SSL handshake. This will allow us to partition trust for multi-tenant services when one service may trust different set of certificates then other.

Proposed API

namespace System.Net.Security
{
    public sealed class SslStreamCertificateContext
    {
       public static SslStreamCertificateContext Create(
                                 X509Certificate2 target, 
                                 X509Certificate2Collection? additionalCertificates, 
                                 bool offline = false);
+      public static SslStreamCertificateContext CreateWithTrust( 
+                                  X509Certificate2 target,  
+                                  X509Certificate2Collection? additionalCerts,  
+                                  SslCertificateTrust trust,  
+                                  bool offline = false);
    }

+   public sealed class SslCertificateTrust
+   {
+       public static SslCertificateTrust CreateForX509Store(
+                                               X509Store store, 
+                                               X509ChainTrustMode trustMode = X509ChainTrustMode.CustomRootTrust,
+                                               bool sendTrustInHandshake = false );
+
+       [UnsupportedPlatform("windows")]
+       public static SslCertificateTrust CreateForX509Collection(
+                                               X509Certificate2Collection trustList,
+                                               X509ChainTrustMode trustMode = X509ChainTrustMode.CustomRootTrust,
+                                               bool sendTrustInHandshake = false );
+  }
}

Alternative design

We originally started with something like:

public static SslStreamCertificateContext CreateWithTrust(
  X509Certificate2 target,
  X509Certificate2Collection? additionalCerts,
  X509ChainTrustMode trustMode,
  X509Certificate2Collection? customTrustStore,
  bool offline = false,
  bool sendTrustInHandshake = false);

however, on Windows we are not able to support collection for sending trust at this moment (and it it is not clear if we ever will).

For the SslCertificateTrust we may use constructor with overlords. It is not clear if we can annotate the platforms and if it is desirable to throw in constructor. As mentioned above, on Windows we cannot support collection so we will throw PNSP. The certificate stores are restricted to System location only and we will throw if user or custom store is passed in.

Risk

The main risk comes from Windows support. While the main ask is to mimic existing behavior so it is possible to migrate to Kestrel, the available API is sub-optimal. Originally, this is was available only in Kernel mode and that what brings restriction to System certificate stores. (in memory stores or anything else is not possible). Also the behavior to send the trust list in handshake is controlled by global Windows registry. We will need to document this and most likely ignore the setting. Support for same feature in user mode is in some private builds and there is official request to back port it to Server 2019. Note that this applies only the the ability to advertise the trust. We can internally take certificate collection and we can pass it to X509Chain and calculate trust accordingly.

The other part is not specific to tis proposal. When SslStreamCertificateContext was created it basically sealed blob that is used internally in SslStream. However, in 6.0 we will also support SslOptions in System.Net.Quick. For now we use reflection to do that and that is not optimal - see #53507. At some point we may need to add properties to expose internal state and passed parameters.

[ghost]

Tagging subscribers to this area: @dotnet/ncl, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and Motivation

This was originally requested as #45456 (pri-1) to get parity with IIS/http.sys. After iterations with @bartonjs this was slightly extended to solve few more problems related to managing trust with TLS/SSL handshake. This will allow us to partition trust for multi-tenant services when one service may trust different set of certificates then other.

Proposed API

namespace System.Net.Security
{
    public sealed class SslStreamCertificateContext
    {
       public static SslStreamCertificateContext Create(
                                 X509Certificate2 target, 
                                 X509Certificate2Collection? additionalCertificates, 
                                 bool offline = false);
+      public static SslStreamCertificateContext CreateWithTrust( 
+                                  X509Certificate2 target,  
+                                  X509Certificate2Collection? additionalCerts,  
+                                  SslCertificateTrust trust,  
+                                  bool offline = false);
    }

+   public sealed class SslCertificateTrust
+   {
+       public static SslCertificateTrust CreateForX509Store(
+                                               X509Store store, 
+                                               SslCertificateTrust trust,
+                                               bool sendTrustInHandshake = false );
+
+       [UnsupportedPlatform("windows")]
+       public static CertificiateTrustList CreateForX509Collection(
+                                               X509Certificate2Collection trustList,
+                                               SslCertificateTrust trust,
+                                               bool sendTrustInHandshake = false );
+  }
}

Alternative design

We originally started with something like:

public static SslStreamCertificateContext CreateWithTrust(
  X509Certificate2 target,
  X509Certificate2Collection? additionalCerts,
  X509ChainTrustMode trustMode,
  X509Certificate2Collection? customTrustStore,
  bool offline = false,
  bool sendTrustInHandshake = false);

however, on Windows we are not able to support collection for sending trust at this moment (and it it is not clear if we ever will).

For the SslCertificateTrust we may use constructor with overlords. It is not clear if we can annotate the platforms and if it is desirable to throw in constructor. As mentioned above, on Windows we cannot support collection so we will throw PNSP. The certificate stores are restricted to System location only and we will throw if user or custom store is passed in.

Risk

The main risk comes from Windows support. While the main ask is to mimic existing behavior so it is possible to migrate to Kestrel, the available API is sub-optimal. Originally, this is was available only in Kernel mode and that what brings restriction to System certificate stores. (in memory stores or anything else is not possible). Also the behavior to send the trust list in handshake is controlled by global Windows registry. We will need to document this and most likely ignore the setting. Support for same feature in user mode is in some private builds and there is official request to back port it to Server 2019. Note that this applies only the the ability to advertise the trust. We can internally take certificate collection and we can pass it to X509Chain and calculate trust accordingly.

The other part is not specific to tis proposal. When SslStreamCertificateContext was created it basically sealed blob that is used internally in SslStream. However, in 6.0 we will also support SslOptions in System.Net.Quick. For now we use reflection to do that and that is not optimal - see #53507. At some point we may need to add properties to expose internal state and passed parameters.

Author:	wfurt
Assignees:	-
Labels:	api-suggestion, area-System.Net.Security
Milestone:	6.0.0

[wfurt]

Let me know @bartonjs if this reflects our discussion or if I forget something. Please add to it as you see fit. If it looks ok to you flip the readiness flag or let me know. We are still trying to get this to 6.0 as high priority ask.

cc: @Tratcher

[bartonjs]

Presumably

+   public sealed class SslCertificateTrust
+   {
+       public static SslCertificateTrust CreateForX509Store(
+                                               X509Store store, 
+                                               SslCertificateTrust trust,
+                                               bool sendTrustInHandshake = false );
+
+       [UnsupportedPlatform("windows")]
+       public static CertificiateTrustList CreateForX509Collection(
+                                               X509Certificate2Collection trustList,
+                                               SslCertificateTrust trust,
+                                               bool sendTrustInHandshake = false );
+  }

should be

+   public sealed class SslCertificateTrust
+   {
+       public static SslCertificateTrust CreateForX509Store(
+                                               X509Store store, 
+                                               X509ChainTrustMode trustMode = X509ChainTrustMode.CustomRootTrust,
+                                               bool sendTrustInHandshake = false );
+
+       [UnsupportedPlatform("windows")]
+       public static SslCertificateTrust CreateForX509Collection(
+                                               X509Certificate2Collection trustList,
+                                               X509ChainTrustMode trustMode = X509ChainTrustMode.CustomRootTrust,
+                                               bool sendTrustInHandshake = false );
+  }

• Caught a type rename in the second method return
• Changed the second parameter to X509ChainTrustMode, instead of a circular dependency
• Now that the chain trust decision is attached to this new collection it seemed appropriate to default the trust parameter.
  • Alternatively, now that it's supposed to just be encapsulated with this new type, it could probably be dropped from the signature for now and added later. (Since only one input is valid)

[terrajobst]

• Let's make it an overload of SslStreamCertificateContext.Create() and apply the versioning rules of methods with optional parameters.
• We don't believe we need the trustMode on SslCertificateTrust

namespace System.Net.Security
{
    public partial class SslStreamCertificateContext
    {
        // Existing API:
        // - Hide and remove default-ness
        [EditorBrowsable(EditorBrowsableState.Never)]
        public static SslStreamCertificateContext Create(
            X509Certificate2 target, 
            X509Certificate2Collection? additionalCertificates, 
            bool offline
        );
        public static SslStreamCertificateContext Create( 
            X509Certificate2 target,  
            X509Certificate2Collection? additionalCertificates,  
            bool offline = false,
            SslCertificateTrust? trust = null
        );
    }

    public sealed class SslCertificateTrust
    {
        public static SslCertificateTrust CreateForX509Store(
            X509Store store, 
            bool sendTrustInHandshake = false
        );
 
        [UnsupportedPlatform("windows")]
        public static SslCertificateTrust CreateForX509Collection(
            X509Certificate2Collection trustList,
            bool sendTrustInHandshake = false
        );
    }
}

[wfurt]

I'm not sure if intended or fixable but SslStreamCertificateContext.Create(certificate, null) now fails with complain about ambiguity:

C:\Users\test\github\wfurt-runtime\src\libraries\System.Net.Security\tests\FunctionalTests\SslStreamNetworkStreamTest.cs(801,82): error CS0121: The call is ambiguous between the following methods or properties: 'SslStreamCertificateContext.Create(X509Certificate2, X509Certificate2Collection?, bool)' and 'SslStreamCertificateContext.Create(X509Certificate2, X509Certificate2Collection?, bool, SslCertificateTrust?)' [C:\Users\test\github\wfurt-runtime\src\libraries\System.Net.Security\tests\FunctionalTests\System.Net.Security.Tests.csproj]

[bartonjs]

I'm not sure if intended or fixable but SslStreamCertificateContext.Create(certificate, null) now fails with complain about ambiguity:

That sounds like you didn't remove the defaulted boolean from the existing member.

[wfurt]

ok. I miss that we are changing the existing call as well. I'll fix it.

[wfurt]

fixed by #55104