SslStream.HashAlgorithm does not seems to work with Tls12 and above

[wfurt]

Description

I used examples from https://docs.microsoft.com/en-us/dotnet/api/system.net.security.sslstream.hashalgorithm?view=net-5.0

static void DisplaySecurityLevel(SslStream stream)
{
   Console.WriteLine("Cipher: {0} strength {1}", stream.CipherAlgorithm, stream.CipherStrength);
   Console.WriteLine("Hash: {0} strength {1}", stream.HashAlgorithm, stream.HashStrength);
   Console.WriteLine("Key exchange: {0} strength {1}", stream.KeyExchangeAlgorithm, stream.KeyExchangeStrength);
   Console.WriteLine("Protocol: {0}", stream.SslProtocol);
}

When I force Tls11 I see TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA negotiated and output of:

Cipher: Aes128 strength 128
Hash: Sha1 strength 160
Key exchange: DiffieHellman strength 0
Protocol: Tls11

when I use default, Tls12 or Tls13 I get TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 and :

Cipher: Aes128 strength 128
Hash: None strength 0
Key exchange: DiffieHellman strength 0
Protocol: Tls12

Configuration

Tested with current 5.0 version on Linux with OpenSsl 1.1.1 and macOS 10.15 Catalina.

Other information

We seems to have no unit test to cover HashAlgorithm.

[ghost]

Tagging subscribers to this area: @dotnet/ncl
Notify danmosemsft if you want to be subscribed.

[karelz]

Triage: Not critical for 5.0. @wfurt plans to do a write up and follow up with crypto to decide what to do here.

[wfurt]

This seems to be by design. The SHA256 from the TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 is ignored and it boils down to GCM from AES. In that mode AES is able to perform integrity using AEAD.
However, returning null is really not that helpful IMHO. I'm wondering if we could either return "AES" or "AEAD" or some better hint to what is going on.

cc: @bartonjs

[bartonjs]

Classically, the return value here is dictated by whatever SChannel reports, then just making the OpenSSL/macOS PALs try to report the same thing for the same ciphersuites.

[wfurt]

That does not quite match the windows. I did test run and I get:

  Cipher: Aes256 strength 256
  Hash: Sha384 strength 0
  Key exchange: 44550 strength 384
  Protocol: Tls12
  Suite: TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384

interestingly, hash strength is 0, and we don't seems to map properly the KeyExchange algorithm.

[krwq]

Note that this might be by design, see my comment here:
https://github.com/dotnet/runtime/blob/master/src/libraries/System.Net.Security/src/System/Net/Security/TlsCipherSuiteNameParser.ttinclude#L554-L557

[wfurt]

It is interesting that it does not match Windows behavior as @bartonjs expected.
Using something else then null may be still useful. Or perhaps add explicit note to docs about AEAD.

[krwq]

I believe Windows answer is wrong in that case (assuming by HashAlgorithm we mean MAC and that's what docs describe).

For reporting issue I'd reccomend to change Windows to use https://github.com/dotnet/runtime/blob/master/src/libraries/System.Net.Security/src/System/Net/Security/TlsCipherSuiteData.Lookup.cs instead of answer returned by SChannel which returns hash used for HKDF

And then to improve reporting I'd recommend to do one of the following:

• we upgrade HashAlgorithm to look more like https://github.com/dotnet/runtime/blob/master/src/libraries/System.Net.Security/src/System/Net/Security/TlsCipherSuiteNameParser.ttinclude#L62 - note we currently use HashAlgorithm and pass it directly to SChannel so that might be a bit more work to get that right
• obsolete: HashAlgorithm, KeyExchange, Cipher and recommend to use NegotiatedCiphersuite instead (my vote is to add such recommendation in the docs anyway regardless if we want to obsolete or not)

[rzikm]

As krwq said, HashAlgorithm refers to the MAC algorithm, since the relevant cipher uses AEAD, then HashAlgorithm None of strength 0 makes sense.

For consistency, we might want to use the TlsCipherSuiteData Lookup on Windows as well to decode the key exchange algorithm correctly. I can put up a PR

[wfurt]

triage: We should resurrect #66702 when #55570 is done. All related, will try for 8.0.

[rzikm]

Triage: should be consolidated with the effort on other HashAlgorithmType and other enums.

Potential idea?: Mark everything except TlsCipherSuite obsolete since the cipher suite contains all necessary information, or improve documentation to manage user expectations.

[rzikm]

Since #100361, the plan is to obsolete the enum, closing as won't do.