[API Proposal] Expand ExchangeAlgorithmType, CipherAlgorithmType, HashAlgorithmType

[rzikm]

Background and motivation

Enums ExchangeAlgorithmType, CipherAlgorithmType and HashAlgorithmType haven't been updated in a long time and code which uses them (SslStream) sometimes even returns values which do not map to existing members. See e.g. #55570. Similarly, many algorithms/ciphers belonging to the same general family are being mapped to the same enum member, discarding information in the process.

Since the expected use of these properites is mainly logging for auditing purposes, it makes sense to report more specific information.

API Proposal

This proposal adds missing members so that we are on par with

runtime/src/libraries/System.Net.Security/src/System/Net/Security/TlsCipherSuiteNameParser.ttinclude

Lines 11 to 71 in f92b9ef

	public enum ExchangeAlgorithmType
	{
	None,
	Rsa,
	DiffieHellmanStatic,
	DiffieHellmanEphermal,
	ECDiffieHellman,
	ECDiffieHellmanEphermal,
	Kerberos5,
	PSK,
	SRP,
	ECCPWD,
	Any,
	}
	public enum CipherAlgorithmType
	{
	Aes,
	Aes128,
	Aes192,
	Aes256,
	Des,
	None,
	Null,
	Rc2,
	Rc4,
	TripleDes,
	AesGcm,
	AesCcm,
	Aes128Gcm,
	Aes256Gcm,
	Aes128Ccm,
	Aes128Ccm8,
	Aes256Ccm,
	Aes256Ccm8,
	Camellia,
	Camellia128,
	Camellia256,
	Camellia128Gcm,
	Camellia256Gcm,
	ChaCha20,
	ChaCha20Poly1305,
	Seed,
	Idea,
	Aria,
	Aria128,
	Aria256,
	Aria128Gcm,
	Aria256Gcm,
	}
	public enum HashAlgorithmType
	{
	None,
	Md5,
	Sha1,
	Sha256,
	Sha384,
	Sha512,
	Aead,
	}

namespace System.Security.Authentication
{
    public enum ExchangeAlgorithmType
    {
        // existing members
        None = 0,
        RsaSign = 9216, // note: Not used by TlsCipherSuiteNameParser
        RsaKeyX = 41984,
        DiffieHellman = 43522, // the code is for Diffie-Hellman ephemeral kex

        // values chosen to match values from wincrypt
+       DiffieHellmanStatic = 0xaa01,
+       DiffieHellmanEphermal = DiffieHellman,
+       ECDiffieHellman 0xaa05,
+       ECDiffieHellmanEphermal = 0xaa06,

        // following are not present in wincrypt.h on which numerical values are based
        // are assigned values ok?
+       Kerberos5 = 1,
+       PSK,
+       SRP,
+       ECCPWD,
    }

    public enum CipherAlgorithmType
    {
        // existing members
        None = 0,
        Null = 24576,
        Des = 26113,
        Rc2 = 26114,
        TripleDes = 26115,
        Aes128 = 26126,
        Aes192 = 26127,
        Aes256 = 26128,
        Aes = 26129,
        Rc4 = 26625,

        //  wincrypt does not tell us difference between GCM and CCM?
+       AesGcm = 1,
+       AesCcm,
+       Aes128Gcm,
+       Aes256Gcm,
+       Aes128Ccm,
+       Aes128Ccm8,
+       Aes256Ccm,
+       Aes256Ccm8,

        //  No algorithm identifier in wincrypt.h, assign arbitrary values
+       Camellia,
+       Camellia128,
+       Camellia256,
+       Camellia128Gcm,
+       Camellia256Gcm,
+       ChaCha20,
+       ChaCha20Poly1305,
+       Seed,
+       Idea,
+       Aria,
+       Aria128,
+       Aria256,
+       Aria128Gcm,
+       Aria256Gcm,
    }

    public enum HashAlgorithmType
    {
        // existing members
        None = 0,
        Md5 = 32771,
        Sha1 = 32772,
        Sha256 = 32780,
        Sha384 = 32781,
        Sha512 = 32782,

        // No algorithm identifier in wincrypt.h
+       Aead = 1,
    }
}

API Usage

The values are expected to be used mainly for logging and audit purposes.

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);
}

Alternative Designs

The above mentioned enum types are only used on properties of SslStream where
they expose information about the negotiated TLS cipher suite. All information
can be deduced from the SslStream.TlsCipherSuite so another option is to
obsolete all of

• ExchangeAlgorithmType, CipherAlgorithmType, HashAlgorithmType enums
• KeyExchangeAlgorithm, KeyExchangeStrength, CipherAlgorithm, CipherAlgorithmStrength, HashAlgorithm, HashStrength properties of SslStream

And leave TlsCipherSuite SslStream.NegotiatedCipherSuite as the only source of truth.

Risks

If -- in the future -- Windows adds ALG_ID for algorithms we assigned an
arbitrary value, the values will no longer be in sync. However, we plan to mitigate this by using the lookup table from

runtime/src/libraries/System.Net.Security/src/System/Net/Security/SslConnectionInfo.Unix.cs

Line 41 in f92b9ef

static int GetPackedData(TlsCipherSuite cipherSuite)

on all platforms for consistency between platforms (to fix #37578).

[dotnet-policy-service]

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

[rzikm]

cc @wfurt, @bartonjs

Let me know if you have any comments/concerns, if not I will put it to the API review queue.

[wfurt]

I'm wondering if we should bump the algorithms wincrypt does not have up high so there is less likelihood it will conflict with wincrypt in the future.
We did something similar to AddressFamily

Also do we need to split the ephemeral vs static? I'm not sure the difference matters and we can always map either one to the base. For anybody who really needs to know exactly perhaps the TlsCipherSuite is the ultimate answer.

I'll probably lean toward recommendation from @vcsjones and @bartonjs

[rzikm]

@vcsjones, @bartonjs do you have any comments/recommendations here?

[bartonjs]

@bartonjs do you have any comments/recommendations here?

Not really. I can understand the thinking behind why these types exist, but I don't really understand their "purpose". Now that .NET exposes the ciphersuite value there's not much reasoning for these, other than they already exist.

I'm wondering if we should bump the algorithms wincrypt does not have up high so there is less likelihood it will conflict with wincrypt in the future.

If the data on all platforms is already driven by ciphersuite ID, then I wouldn't bother caring who gets what number. If Windows is still pass-through and you want to maintain that for as long as possible, then picking unlikely numbers has merit. Just beware that every now and then Windows comes back and surprises you by adding an algorithm that seemed impossible previously; and if you've already added it to this enum that'll force a mapping (plus have the confusion in the meantime where Windows and non-Windows use different values for the same thing).

OK, so I guess I do have one recommendation: ditch the Windows passthrough and drive everything off of ciphersuite. (Unless the ciphersuite is backformed on Windows from these breakdown products... I guess...)

do we need to split the ephemeral vs static?

It looks like Windows already has been (at least, ECDHE vs ECDH seem different, but DH and DHE are maybe the same?), so probably worth maintaining.

[wfurt]

OK, so I guess I do have one recommendation: ditch the Windows passthrough and drive everything off of ciphersuite. (Unless the ciphersuite is backformed on Windows from these breakdown products... I guess...)

we talk about it. And also sharing it with asp.net for Quic. AFAIK most users use this just as convenience for audits.

[wfurt]

and TlsCipherSuite is not available in netstandard & Framework so it is hassle for certain user base

[rzikm]

I can understand the thinking behind why these types exist, but I don't really understand their "purpose". Now that .NET exposes the ciphersuite value there's not much reasoning for these, other than they already exist.

AFAICT they exist only for logging purposes, so we feel we should either make them up to date or obsolete them and lead users to the TlsCipherSuite (which as wfurt mentioned is not part of .NET Standard, and, if it changes anything further, is apparently not CLS compliant).

If the data on all platforms is already driven by ciphersuite ID, then I wouldn't bother caring who gets what number.

OK, so I guess I do have one recommendation: ditch the Windows passthrough and drive everything off of ciphersuite. (Unless the ciphersuite is backformed on Windows from these breakdown products... I guess...)

That is the intention, I wanted to do that long time ago in #66702, and I can ressurect it once we get this in.

[rzikm]

Since there have not been any major concerns, let's queue this for review.

[bartonjs]

Video

• Instead of modifying the enums, we're going to mark them as Obsolete and direct people to the TlsCipherSuite instead.
• All of these obsoleted members should use the same diagnostic ID (whatever SYSLIB is next)

namespace System.Security.Authentication
{
+   [Obsolete]
    public enum ExchangeAlgorithmType { }

+   [Obsolete]
    public enum CipherAlgorithmType { }

+   [Obsolete]
    public enum HashAlgorithmType { }
}

namespace System.Net.Security
{
+   [Obsolete]
    public virtual ExchangeAlgorithmType KeyExchangeAlgorithm { get; }

+   [Obsolete]
    public virtual int KeyExchangeStrength { get; }

+   [Obsolete]
    public virtual CipherAlgorithmType CipherAlgorithm { get; }

+   [Obsolete]
    public virtual int CipherStrength { get; }    

+   [Obsolete]
    public virtual HashAlgorithmType HashAlgorithm { get; }

+   [Obsolete]
    public virtual int HashStrength { get; }
}

[rzikm]

Looks like the enums are used in other runtime places after all :/

runtime/src/libraries/System.DirectoryServices.Protocols/ref/System.DirectoryServices.Protocols.cs

Lines 658 to 668 in 1487522

	public partial class SecurityPackageContextConnectionInformation
	{
	internal SecurityPackageContextConnectionInformation() { }
	public System.Security.Authentication.CipherAlgorithmType AlgorithmIdentifier { get { throw null; } }
	public int CipherStrength { get { throw null; } }
	public int ExchangeStrength { get { throw null; } }
	public System.Security.Authentication.HashAlgorithmType Hash { get { throw null; } }
	public int HashStrength { get { throw null; } }
	public int KeyExchangeAlgorithm { get { throw null; } }
	public System.DirectoryServices.Protocols.SecurityProtocol Protocol { get { throw null; } }
	}

This seems to belong to code around LdapConnection, I have never heard of this type before, but as far as I can tell it is neither obsoleted nor discouraged in our docs. It also does not expose TlsCipherSuite, so for this type, these members are the only source of information about used TLS encryption, so it feels like we should not obsolete these if there is no alternative.

There does not seem to be any API to get TlsCipherSuite (nor easy way to implement it), and it also seems that only Windows implementation supports TLS.

What is the API review teams guidance, @bartonjs ?

• Is it okay in this case to obsolete only the SslStream properties and leave the types alone?
• or is the LdapConnection not important enough for us to care and I should obsolete it anyway?

[bartonjs]

It's still going to have the same problem on LdapConnection: the returned values won't necessarily map to anything. (Though I assume for LdapConnection it's just passing through the Windows values, so someone could map it later).

I'd say obsolete those properties on LdapConnection, too... I doubt anyone would be really upset.

[teo-tsirpanis]

Done in #105875.