Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Populate DataHash key size in SslConnectionInfo #4169

Merged
stephentoub merged 2 commits into from
Oct 29, 2015
Merged

Populate DataHash key size in SslConnectionInfo #4169

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
86dec92
Populate DataHash key size in SslConnectionInfo
shrutigarg Oct 27, 2015
02e3992
Fixed PR Comments and Synced code
shrutigarg Oct 29, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Fixed PR Comments and Synced code
  • Loading branch information
@shrutigarg
shrutigarg committed Oct 29, 2015
commit 02e3992cf5f11d7fc50debe4a16c4cb9527461e8
5 changes: 4 additions & 1 deletion src/Common/src/Interop/Unix/libssl/SslConnectionInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ internal SslConnectionInfo(SafeSslHandle sslContext)
{
throw Interop.OpenSsl.CreateSslException(SR.net_ssl_get_connection_info_failed);
}
Copy link
Member

@eerhardt eerhardt Oct 27, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we update the // TODO (Issue #3362) map key sizes comment below?

Copy link
Contributor Author

@shrutigarg shrutigarg Oct 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems openssl doesnot provide a way to return a exchangekey size.. in dtls1_send_server_key_exchange and similar function for other sslProtocols , it internally calculates the key exchange size and generate key.. It is not a constant either that we can hardcode and return.
That's why its still pending .. not sure how to fix this.

Copy link
Member

@eerhardt eerhardt Oct 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be good to note in the code. Also opening an issue for this and using that number in the TODO comment is a good idea since #3362 is closed and #4128 is going to come through and just delete these TODO comments.

Copy link
Contributor Author

@shrutigarg shrutigarg Oct 29, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems this todo we cannot proceed in , we have already done the investigations around. So just making a note for this. No point of opening another TODO.
Will delete this TODO comment.

// 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)
Expand Down
98 changes: 36 additions & 62 deletions src/Native/System.Security.Cryptography.Native/pal_ssl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}
Copy link
Member

@stephentoub stephentoub Oct 27, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the default case where sslMac doesn't match any of these?

Copy link
Contributor Author

@shrutigarg shrutigarg Oct 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will add at end to return 0 for rest of the cases.


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;
}
Expand All @@ -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;

Expand All @@ -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;
}
Expand Down
19 changes: 18 additions & 1 deletion src/Native/System.Security.Cryptography.Native/pal_ssl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down