disable sending NT Authority in TLS handshake if specific trust was specified

[wfurt]

When Windows sends trusted CA list in handshake it by default includes NT Authority as special case.
I'm not sure if anybody depends on it but it seems to break some use cases with new API from #45456.
Particularly attempt to send empty trust list seems to break .NET clients when only NT Authority is present.
That may be issue with the client selection but we don't have control over all of them.

To deal with it, I added explicit SCH_CRED_NO_SYSTEM_MAPPER flag to suppress that channel behavior when SslCertificateTrust is provided and sendTrustInHandshake is set to true. (SslStreamPal.Windows.cs)
Alternatively, we could set it always but I'm concern about backward compatibility. (SslCertificateTrust was introduced in 6.0 so changing it there should be ok)

Since this is done on credential handle, we need to also update the caching logic to not mingle that case with others.

We don't have any automated tests since this still depends on Windows registry setting. I did manual tests and verified that the NT Authority is not in the CA list as well as pointing to empty certificate store sends empty list.

fixes #60949

[ghost]

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

Issue Details

---

When Windows sends trusted CA list in handshake it by default includes NT Authority as special case.
I'm not sure if anybody depends on it but it seems to break some use cases with new API from #45456.
Particularly attempt to send empty trust list seems to break .NET clients when only NT Authority is present.
That may be issue with the client selection but we don't have control over all of them.

To deal with it, I added explicit SCH_CRED_NO_SYSTEM_MAPPER flag to suppress that channel behavior when SslCertificateTrust is provided and sendTrustInHandshake is set to true. (SslStreamPal.Windows.cs)
Alternatively, we could set it always but I'm concern about backward compatibility. (SslCertificateTrust was introduced in 6.0 so changing it there should be ok)

Since this is done on credential handle, we need to also update the caching logic to not mingle that case with others.

We don't have any automated tests since this still depends on Windows registry setting. I did manual tests and verified that the NT Authority is not in the CA list as well as pointing to empty certificate store sends empty list.

fixes #60949

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

[scalablecory]

Give in to the null conditional operator, @wfurt! :)

LGTM either way.

[Wraith2]

Give in to the null conditional operator, @wfurt! :)

The problem with the null conditional operator is that pesky literal `== true' at the end to force the type to bool. The brevity is better but the clarity is poor imo.

[wfurt]

Give in to the null conditional operator, @wfurt! :)

The problem with the null conditional operator is that pesky literal `== true' at the end to force the type to bool. The brevity is better but the clarity is poor imo.

I don't really care one way or other. Perhaps @stephentoub or @geoffkizer can chime in with preference.

[stephentoub]

Small preference for the null conditional, but either way is ok.

[geoffkizer]

The problem with the null conditional operator is that pesky literal `== true' at the end to force the type to bool. The brevity is better but the clarity is poor imo.

I agree with this. I always find it weird to see "== true" because normally it's unnecessary. It makes me have to stop and think about what's going on.

An alternative that still uses the null conditional operator, but doesn't use "== true", is:

    bool sendTrustedList = _sslAuthenticationOptions.CertificateContext!.Trust?._sendTrustInHandshake ?? false;

I prefer this slightly, since the use of "?? false" instead of "== true" makes it obvious that you're handling the null case.

Also, for the reverse case where you want to treat null as true, "?? true" seems much clearer to me than "!= false", which makes my head hurt even more than "== true". That doesn't apply here though.

[wfurt]

An alternative that still uses the null conditional operator, but doesn't use "== true", is:

I was thinking about it more and I like this best @geoffkizer. It is basically use "this" or "default".

[wfurt]

/backport to release/6.0

[github-actions]

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1469614283