fix SslStreamCertificateContext.Create with partial chain

[wfurt]

With TrimRootCertificate=false, the old logic would incorrectly increase count when chain.Build() return partial chain. That would later cause ArgumentOutOfRangeException when iterating through the chain certificate collection.

Further more, the TrimRootCertificate is swapped. If we want to trim root we need to subtract 2 (e.g. root itself and leaf cert) This will cause wrong number of certificates sent on the wire on Unix. (on Windows all the logic is bolted to OS)

To fix this, I refactor the code a little bit. There is no need to invoke the partial chain logic at all if we are going to give whole chain to OS (Windows)

I added variant to SslStream_UntrustedCaWithCustomCallback_OK and verified it will expose the ArgumentOutOfRangeException without the fix.

fixes #46654

[ghost]

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

Issue Details

---

With TrimRootCertificate=false, the old logic would incorrectly increase count when chain.Build() return partial chain. That would later cause ArgumentOutOfRangeException when iterating through the chain certificate collection.

Further more, the TrimRootCertificate is swapped. If we want to trim root we need to subtract 2 (e.g. root itself and leaf cert) This will cause wrong number of certificates sent on the wire on Unix. (on Windows all the logic is bolted to OS)

To fix this, I refactor the code a little bit. There is no need to invoke the partial chain logic at all if we are going to give whole chain to OS (Windows)

I added variant to SslStream_UntrustedCaWithCustomCallback_OK and verified it will expose the ArgumentOutOfRangeException without the fix.

fixes #46654

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

[wfurt]

/backport to release/5.0

[github-actions]

Started backporting to release/5.0: https://github.com/dotnet/runtime/actions/runs/479577515

[github-actions]

@wfurt backporting to release/5.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: fix SslStreamCertificateContext.Create with partial chain
Applying: add test with long partial chain
error: sha1 information is lacking or useless (src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamNetworkStreamTest.cs).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 add test with long partial chain
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

[sukney]

@wfurt After the 5.02 update, the console example is good, but the web server example still reports an error as follows

Microsoft.AspNetCore.Server.Kestrel[0]
      Unable to start Kestrel.
      System.IO.IOException: Failed to bind to address https://localhost:50071.
       ---> System.AggregateException: One or more errors occurred. (Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')) (Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index'))
       ---> System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')
         at System.Security.Cryptography.X509Certificates.X509ChainElementCollection.get_Item(Int32 index)
         at System.Net.Security.SslStreamCertificateContext.Create(X509Certificate2 target, X509Certificate2Collection additionalCertificates, Boolean offline)
         at Microsoft.AspNetCore.Server.Kestrel.Https.Internal.HttpsConnectionMiddleware..ctor(ConnectionDelegate next, HttpsConnectionAdapterOptions options, ILoggerFactory loggerFactory)
         at Microsoft.AspNetCore.Hosting.ListenOptionsHttpsExtensions.<>c__DisplayClass12_0.<UseHttps>b__0(ConnectionDelegate next)
         at Microsoft.AspNetCore.Server.Kestrel.Core.ListenOptions.Build()
         at Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerImpl.<>c__DisplayClass29_0`1.<<StartAsync>g__OnBind|0>d.MoveNext()
      --- End of stack trace from previous location ---
         at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.AddressBinder.BindEndpointAsync(ListenOptions endpoint, AddressBindContext context)
         at Microsoft.AspNetCore.Server.Kestrel.Core.LocalhostListenOptions.BindAsync(AddressBindContext context)
         --- End of inner exception stack trace ---
       ---> (Inner Exception #1) System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')
         at System.Security.Cryptography.X509Certificates.X509ChainElementCollection.get_Item(Int32 index)
         at System.Net.Security.SslStreamCertificateContext.Create(X509Certificate2 target, X509Certificate2Collection additionalCertificates, Boolean offline)
         at Microsoft.AspNetCore.Server.Kestrel.Https.Internal.HttpsConnectionMiddleware..ctor(ConnectionDelegate next, HttpsConnectionAdapterOptions options, ILoggerFactory loggerFactory)
         at Microsoft.AspNetCore.Hosting.ListenOptionsHttpsExtensions.<>c__DisplayClass12_0.<UseHttps>b__0(ConnectionDelegate next)
         at Microsoft.AspNetCore.Server.Kestrel.Core.ListenOptions.Build()
         at Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerImpl.<>c__DisplayClass29_0`1.<<StartAsync>g__OnBind|0>d.MoveNext()
      --- End of stack trace from previous location ---
         at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.AddressBinder.BindEndpointAsync(ListenOptions endpoint, AddressBindContext context)
         at Microsoft.AspNetCore.Server.Kestrel.Core.LocalhostListenOptions.BindAsync(AddressBindContext context)<---

         --- End of inner exception stack trace ---
         at Microsoft.AspNetCore.Server.Kestrel.Core.LocalhostListenOptions.BindAsync(AddressBindContext context)
         at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.AddressBinder.EndpointsStrategy.BindAsync(AddressBindContext context)
         at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.AddressBinder.BindAsync(IEnumerable`1 listenOptions, AddressBindContext context)
         at Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerImpl.BindAsync(CancellationToken cancellationToken)
         at Microsoft.AspNetCore.Server.Kestrel.Core.KestrelServerImpl.StartAsync[TContext](IHttpApplication`1 application, CancellationToken cancellationToken)

See

[karelz]

It was not ported to 5.0 branch yet - see #46654
Are you able to confirm it works fine now on 6.0 builds? ... Please let's take the discussion into the issue, not on the PR

[wfurt]

You can try the fix on 6.0/master branch https://github.com/dotnet/installer @sukney
Customer verification helps with servicing approval.