Fixed PR Comments and Synced code

Author: shrutigarg

src/Common/src/Interop/Unix/libssl/SslConnectionInfo.cs

@@ -31,7 +31,10 @@ internal SslConnectionInfo(SafeSslHandle sslContext)
             {
                 throw Interop.OpenSsl.CreateSslException(SR.net_ssl_get_connection_info_failed);
             }
-            // TODO (Issue #3362) map key sizes
+
+            //Openssl does not provide a way to return a exchange key size.
+            //It internally does calculate the key size before generating key to exchange
+            //It is not a constant (Algorthim specific) either that we can hardcode and return. 
         }
 
         private SslProtocols MapProtocolVersion(string protocolVersion)

src/Native/System.Security.Cryptography.Native/pal_ssl.cpp

@@ -364,57 +364,7 @@ enum class SSL_DataHashAlgorithm : int64_t
 #endif
 };
 
-class SSL_DataHashSize 
-{ 
-public:
-  static const int32_t MD5_HashKeySize = 128;
-  static const int32_t SHA1_HashKeySize = 160;
-  static const int32_t SHA256_HashKeySize = 256;
-  static const int32_t SHA384_HashKeySize = 384;
-  static const int32_t GOST_HashKeySize = 256;
-};
-
-static HashAlgorithmType MapHashAlgorithmType(const SSL_CIPHER* cipher)
-{
-    unsigned long mac;
-#if HAVE_SSL_CIPHER_SPLIT_ALGORITHMS
-    mac = cipher->algorithm_mac;
-#else
-    const unsigned long SSL_MAC_MASK = 0x00c00000L;
-    mac = cipher->algorithms & SSL_MAC_MASK;
-#endif
-
-    SSL_DataHashAlgorithm sslMac = static_cast<SSL_DataHashAlgorithm>(mac);
-    switch (sslMac)
-    {
-        case SSL_DataHashAlgorithm::SSL_MD5:
-            return HashAlgorithmType::Md5;
-
-        case SSL_DataHashAlgorithm::SSL_SHA1:
-            return HashAlgorithmType::Sha1;
-
-#if HAVE_SSL_CIPHER_SPLIT_ALGORITHMS
-        case SSL_DataHashAlgorithm::SSL_GOST94:
-            return HashAlgorithmType::SSL_GOST94;
-
-        case SSL_DataHashAlgorithm::SSL_GOST89MAC:
-            return HashAlgorithmType::SSL_GOST89;
-
-        case SSL_DataHashAlgorithm::SSL_SHA256:
-            return HashAlgorithmType::SSL_SHA256;
-
-        case SSL_DataHashAlgorithm::SSL_SHA384:
-            return HashAlgorithmType::SSL_SHA384;
-
-        case SSL_DataHashAlgorithm::SSL_AEAD:
-            return HashAlgorithmType::SSL_AEAD;
-#endif
-    }
-
-    return HashAlgorithmType::None;
-}
-
-static int32_t GetHashKeySize(const SSL_CIPHER* cipher)
+static void GetHashAlgorithmTypeAndSize(const SSL_CIPHER* cipher, HashAlgorithmType* dataHashAlg, DataHashSize* hashKeySize)
 {
     unsigned long mac;
 #if HAVE_SSL_CIPHER_SPLIT_ALGORITHMS
@@ -428,35 +378,58 @@ static int32_t GetHashKeySize(const SSL_CIPHER* cipher)
     switch (sslMac)
     {
     case SSL_DataHashAlgorithm::SSL_MD5:
-        return SSL_DataHashSize::MD5_HashKeySize;
+        *dataHashAlg = HashAlgorithmType::Md5;
+        *hashKeySize = DataHashSize::MD5_HashKeySize;
+        return;
 
     case SSL_DataHashAlgorithm::SSL_SHA1:
-        return SSL_DataHashSize::SHA1_HashKeySize;
+        *dataHashAlg = HashAlgorithmType::Sha1;
+        *hashKeySize = DataHashSize::SHA1_HashKeySize;
+        return;
 
 #if HAVE_SSL_CIPHER_SPLIT_ALGORITHMS
     case SSL_DataHashAlgorithm::SSL_GOST94:
-        return SSL_DataHashSize::GOST_HashKeySize;
+        *dataHashAlg = HashAlgorithmType::SSL_GOST94;
+        *hashKeySize = DataHashSize::GOST_HashKeySize;
+        return;
 
     case SSL_DataHashAlgorithm::SSL_GOST89MAC:
-        return SSL_DataHashSize::GOST_HashKeySize;
+        *dataHashAlg = HashAlgorithmType::SSL_GOST89;
+        *hashKeySize = DataHashSize::GOST_HashKeySize;
+        return;
 
     case SSL_DataHashAlgorithm::SSL_SHA256:
-        return SSL_DataHashSize::SHA256_HashKeySize;
+        *dataHashAlg = HashAlgorithmType::SSL_SHA256;
+        *hashKeySize = DataHashSize::SHA256_HashKeySize;
+        return;
 
     case SSL_DataHashAlgorithm::SSL_SHA384:
-        return SSL_DataHashSize::SHA384_HashKeySize;
+        *dataHashAlg = HashAlgorithmType::SSL_SHA384;
+        *hashKeySize = DataHashSize::SHA384_HashKeySize;
+        return;
 
     case SSL_DataHashAlgorithm::SSL_AEAD:
-        return 0;
+        *dataHashAlg = HashAlgorithmType::SSL_AEAD;
+        *hashKeySize = DataHashSize::Default;
+        return;
 #endif
     }
+
+    *dataHashAlg = HashAlgorithmType::None;
+    *hashKeySize = DataHashSize::Default;
+    return;
 }
 
-extern "C" int32_t GetSslConnectionInfo(SSL* ssl, CipherAlgorithmType* dataCipherAlg, ExchangeAlgorithmType* keyExchangeAlg, HashAlgorithmType* dataHashAlg, int32_t* dataKeySize, int32_t* hashKeySize)
+extern "C" int32_t GetSslConnectionInfo(SSL* ssl,
+                                        CipherAlgorithmType* dataCipherAlg, 
+                                        ExchangeAlgorithmType* keyExchangeAlg, 
+                                        HashAlgorithmType* dataHashAlg,
+                                        int32_t* dataKeySize, 
+                                        DataHashSize* hashKeySize)
 {
     const SSL_CIPHER* cipher;
 
-    if (!ssl || !dataCipherAlg || !keyExchangeAlg || !dataHashAlg || !dataKeySize)
+    if (!ssl || !dataCipherAlg || !keyExchangeAlg || !dataHashAlg || !dataKeySize || !hashKeySize)
     {
         goto err;
     }
@@ -469,9 +442,8 @@ extern "C" int32_t GetSslConnectionInfo(SSL* ssl, CipherAlgorithmType* dataCiphe
 
     *dataCipherAlg = MapCipherAlgorithmType(cipher);
     *keyExchangeAlg = MapExchangeAlgorithmType(cipher);
-    *dataHashAlg = MapHashAlgorithmType(cipher);
     *dataKeySize = cipher->alg_bits;
-    *hashKeySize = GetHashKeySize(cipher);
+    GetHashAlgorithmTypeAndSize(cipher, dataHashAlg, hashKeySize);
 
     return 1;
 
@@ -486,6 +458,8 @@ extern "C" int32_t GetSslConnectionInfo(SSL* ssl, CipherAlgorithmType* dataCiphe
         *dataHashAlg = HashAlgorithmType::None;
     if (dataKeySize)
         *dataKeySize = 0;
+    if (hashKeySize)
+        *hashKeySize = DataHashSize::Default;
 
     return 0;
 }

src/Native/System.Security.Cryptography.Native/pal_ssl.h

@@ -4,6 +4,8 @@
 #include "pal_crypto_types.h"
 
 #include <openssl/ssl.h>
+#include <openssl/md5.h>
+#include <openssl/sha.h>
 
 /*
 These values should be kept in sync with System.Security.Authentication.SslProtocols.
@@ -88,6 +90,16 @@ enum class HashAlgorithmType : int32_t
     SSL_AEAD = 229412,
 };
 
+enum class DataHashSize : int32_t
+{
+    MD5_HashKeySize = 8 * MD5_DIGEST_LENGTH,
+    SHA1_HashKeySize = 8 * SHA_DIGEST_LENGTH,
+    SHA256_HashKeySize = 8 * SHA256_DIGEST_LENGTH,
+    SHA384_HashKeySize = 8 * SHA384_DIGEST_LENGTH,
+    GOST_HashKeySize = 256,
+    Default = 0,
+};
+
 enum SslErrorCode : int32_t
 {
     PAL_SSL_ERROR_NONE = 0,
@@ -215,7 +227,12 @@ Returns the connection information for the SSL instance.
 Returns 1 upon success, otherwise 0.
 */
 
-extern "C" int32_t GetSslConnectionInfo(SSL* ssl, CipherAlgorithmType* dataCipherAlg, ExchangeAlgorithmType* keyExchangeAlg, HashAlgorithmType* dataHashAlg, int32_t* dataKeySize, int32_t* hashKeySize);
+extern "C" int32_t GetSslConnectionInfo(SSL* ssl,
+                                        CipherAlgorithmType* dataCipherAlg,
+                                        ExchangeAlgorithmType* keyExchangeAlg,
+                                        HashAlgorithmType* dataHashAlg,
+                                        int32_t* dataKeySize,
+                                        DataHashSize* hashKeySize);
 
 /*
 Shims the SSL_write method.