fix(OpenSSL) Openssl DH key size (#4753)

* Fixed incorrect SSL_CTX_set0_tmp_dh_pkey() usage

* fix(OpenSSL): use DH group enum

* fix(IPAddress): windows scoped test, part II #4644

* fix(OpenSSL): fuzz errors #4663

* chore: remove misplaced comment

---------

Co-authored-by: Peter Klotz <peter.klotz99@gmail.com>
This commit is contained in:
Aleksandar Fabijanic
2024-11-11 12:01:00 -06:00
committed by GitHub
parent 9530a77347
commit c4f66d5188
2 changed files with 100 additions and 45 deletions

View File

@@ -136,9 +136,24 @@ public:
SECURITY_LEVEL_256_BITS = 5 SECURITY_LEVEL_256_BITS = 5
}; };
enum KeyDHGroup
{
// MODP
//KEY_DH_GROUP_768 = 1, // (768-bit)
KEY_DH_GROUP_1024 = 2, // (1024-bit)
//KEY_DH_GROUP_1536 = 5, // (1536-bit)
KEY_DH_GROUP_2048 = 14, // (2048-bit)
//KEY_DH_GROUP_3072 = 15, // (3072-bit)
// ECP
//KEY_DH_GROUP_256 = 19, // (256-bit random)
//KEY_DH_GROUP_384 = 20, // (384-bit random)
//KEY_DH_GROUP_521 = 21 // (521-bit random)
};
struct NetSSL_API Params struct NetSSL_API Params
{ {
Params(); Params(KeyDHGroup dhBits = KEY_DH_GROUP_2048);
/// Initializes the struct with default values. /// Initializes the struct with default values.
std::string privateKeyFile; std::string privateKeyFile;
@@ -181,7 +196,7 @@ public:
/// Specifies a file containing Diffie-Hellman parameters. /// Specifies a file containing Diffie-Hellman parameters.
/// If empty, the default parameters are used. /// If empty, the default parameters are used.
bool dhUse2048Bits; KeyDHGroup dhGroup;
/// If set to true, will use 2048-bit MODP Group with 256-bit /// If set to true, will use 2048-bit MODP Group with 256-bit
/// prime order subgroup (RFC5114) instead of 1024-bit for DH. /// prime order subgroup (RFC5114) instead of 1024-bit for DH.
@@ -441,7 +456,7 @@ public:
void ignoreUnexpectedEof(bool flag = true); void ignoreUnexpectedEof(bool flag = true);
/// Enable or disable SSL/TLS SSL_OP_IGNORE_UNEXPECTED_EOF /// Enable or disable SSL/TLS SSL_OP_IGNORE_UNEXPECTED_EOF
/// ///
/// Some TLS implementations do not send the mandatory close_notify alert on shutdown. /// Some TLS implementations do not send the mandatory close_notify alert on shutdown.
/// If the application tries to wait for the close_notify alert /// If the application tries to wait for the close_notify alert
/// but the peer closes the connection without sending it, an error is generated. /// but the peer closes the connection without sending it, an error is generated.
@@ -458,7 +473,7 @@ private:
void init(const Params& params); void init(const Params& params);
/// Initializes the Context with the given parameters. /// Initializes the Context with the given parameters.
void initDH(bool use2048Bits, const std::string& dhFile); void initDH(KeyDHGroup keyDHGroup, const std::string& dhFile);
/// Initializes the Context with Diffie-Hellman parameters. /// Initializes the Context with Diffie-Hellman parameters.
void initECDH(const std::string& curve); void initECDH(const std::string& curve);

View File

@@ -39,13 +39,13 @@ namespace Poco {
namespace Net { namespace Net {
Context::Params::Params(): Context::Params::Params(KeyDHGroup dhBits):
verificationMode(VERIFY_RELAXED), verificationMode(VERIFY_RELAXED),
verificationDepth(9), verificationDepth(9),
loadDefaultCAs(false), loadDefaultCAs(false),
ocspStaplingVerification(false), ocspStaplingVerification(false),
cipherList("ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH"), cipherList("ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH"),
dhUse2048Bits(false), dhGroup(dhBits),
securityLevel(SECURITY_LEVEL_NONE) securityLevel(SECURITY_LEVEL_NONE)
{ {
} }
@@ -215,7 +215,7 @@ void Context::init(const Params& params)
#endif #endif
} }
initDH(params.dhUse2048Bits, params.dhParamsFile); initDH(params.dhGroup, params.dhParamsFile);
initECDH(params.ecdhCurve); initECDH(params.ecdhCurve);
} }
catch (...) catch (...)
@@ -596,59 +596,59 @@ void Context::createSSLContext()
* OPENSSL_NO_TLS1 is defined in opensslconf.h or on the compiler command line * OPENSSL_NO_TLS1 is defined in opensslconf.h or on the compiler command line
* if TLS1.x was removed at OpenSSL library build time via Configure options. * if TLS1.x was removed at OpenSSL library build time via Configure options.
*/ */
case TLSV1_1_CLIENT_USE: case TLSV1_1_CLIENT_USE:
#if OPENSSL_VERSION_NUMBER >= 0x10100000L #if OPENSSL_VERSION_NUMBER >= 0x10100000L
_pSSLContext = SSL_CTX_new(TLS_client_method()); _pSSLContext = SSL_CTX_new(TLS_client_method());
minTLSVersion = TLS1_1_VERSION; minTLSVersion = TLS1_1_VERSION;
#else #else
_pSSLContext = SSL_CTX_new(TLSv1_1_client_method()); _pSSLContext = SSL_CTX_new(TLSv1_1_client_method());
#endif #endif
break; break;
case TLSV1_1_SERVER_USE: case TLSV1_1_SERVER_USE:
#if OPENSSL_VERSION_NUMBER >= 0x10100000L #if OPENSSL_VERSION_NUMBER >= 0x10100000L
_pSSLContext = SSL_CTX_new(TLS_server_method()); _pSSLContext = SSL_CTX_new(TLS_server_method());
minTLSVersion = TLS1_1_VERSION; minTLSVersion = TLS1_1_VERSION;
#else #else
_pSSLContext = SSL_CTX_new(TLSv1_1_server_method()); _pSSLContext = SSL_CTX_new(TLSv1_1_server_method());
#endif #endif
break; break;
#endif #endif
#if defined(SSL_OP_NO_TLSv1_2) && !defined(OPENSSL_NO_TLS1) #if defined(SSL_OP_NO_TLSv1_2) && !defined(OPENSSL_NO_TLS1)
case TLSV1_2_CLIENT_USE: case TLSV1_2_CLIENT_USE:
#if OPENSSL_VERSION_NUMBER >= 0x10100000L #if OPENSSL_VERSION_NUMBER >= 0x10100000L
_pSSLContext = SSL_CTX_new(TLS_client_method()); _pSSLContext = SSL_CTX_new(TLS_client_method());
minTLSVersion = TLS1_2_VERSION; minTLSVersion = TLS1_2_VERSION;
#else #else
_pSSLContext = SSL_CTX_new(TLSv1_2_client_method()); _pSSLContext = SSL_CTX_new(TLSv1_2_client_method());
#endif #endif
break; break;
case TLSV1_2_SERVER_USE: case TLSV1_2_SERVER_USE:
#if OPENSSL_VERSION_NUMBER >= 0x10100000L #if OPENSSL_VERSION_NUMBER >= 0x10100000L
_pSSLContext = SSL_CTX_new(TLS_server_method()); _pSSLContext = SSL_CTX_new(TLS_server_method());
minTLSVersion = TLS1_2_VERSION; minTLSVersion = TLS1_2_VERSION;
#else #else
_pSSLContext = SSL_CTX_new(TLSv1_2_server_method()); _pSSLContext = SSL_CTX_new(TLSv1_2_server_method());
#endif #endif
break; break;
#endif #endif
#if defined(SSL_OP_NO_TLSv1_3) && !defined(OPENSSL_NO_TLS1) #if defined(SSL_OP_NO_TLSv1_3) && !defined(OPENSSL_NO_TLS1)
case TLSV1_3_CLIENT_USE: case TLSV1_3_CLIENT_USE:
#if OPENSSL_VERSION_NUMBER >= 0x10101000L #if OPENSSL_VERSION_NUMBER >= 0x10101000L
_pSSLContext = SSL_CTX_new(TLS_client_method()); _pSSLContext = SSL_CTX_new(TLS_client_method());
minTLSVersion = TLS1_3_VERSION; minTLSVersion = TLS1_3_VERSION;
#endif #endif
break; break;
case TLSV1_3_SERVER_USE: case TLSV1_3_SERVER_USE:
#if OPENSSL_VERSION_NUMBER >= 0x10101000L #if OPENSSL_VERSION_NUMBER >= 0x10101000L
_pSSLContext = SSL_CTX_new(TLS_server_method()); _pSSLContext = SSL_CTX_new(TLS_server_method());
minTLSVersion = TLS1_3_VERSION; minTLSVersion = TLS1_3_VERSION;
#endif #endif
break; break;
#endif #endif
default: default:
@@ -680,7 +680,7 @@ void Context::createSSLContext()
} }
void Context::initDH(bool use2048Bits, const std::string& dhParamsFile) void Context::initDH(KeyDHGroup keyDHGroup, const std::string& dhParamsFile)
{ {
#ifndef OPENSSL_NO_DH #ifndef OPENSSL_NO_DH
static const unsigned char dh1024_p[] = static const unsigned char dh1024_p[] =
@@ -828,11 +828,39 @@ void Context::initDH(bool use2048Bits, const std::string& dhParamsFile)
throw Poco::NullPointerException(Poco::Crypto::getError(err)); throw Poco::NullPointerException(Poco::Crypto::getError(err));
} }
size_t keyLength = use2048Bits ? 256 : 160; size_t keyLength = 0;
unsigned char* pDH_p = const_cast<unsigned char*>(use2048Bits ? dh2048_p : dh1024_p); unsigned char* pDH_p = nullptr;
std::size_t sz_p = use2048Bits ? sizeof(dh2048_p) : sizeof(dh1024_p); std::size_t sz_p = 0;
unsigned char* pDH_g = const_cast<unsigned char*>(use2048Bits ? dh2048_g : dh1024_g); unsigned char* pDH_g = nullptr;
std::size_t sz_g = use2048Bits ? sizeof(dh2048_g) : sizeof(dh1024_g); std::size_t sz_g = 0;
switch(keyDHGroup)
{
case KEY_DH_GROUP_1024:
keyLength = 160;
pDH_p = const_cast<unsigned char*>(dh1024_p);
sz_p = sizeof(dh1024_p);
pDH_g = const_cast<unsigned char*>(dh1024_g);
sz_g = sizeof(dh1024_g);
break;
case KEY_DH_GROUP_2048:
keyLength = 256;
pDH_p = const_cast<unsigned char*>(dh2048_p);
sz_p = sizeof(dh2048_p);
pDH_g = const_cast<unsigned char*>(dh2048_g);
sz_g = sizeof(dh2048_g);
break;
default:
throw Poco::NotImplementedException(Poco::format(
"DH Group: %d", static_cast<int>(keyDHGroup)));
}
poco_assert (keyLength);
poco_check_ptr (pDH_p);
poco_assert (sz_p);
poco_check_ptr (pDH_g);
poco_assert (sz_g);
OSSL_PARAM params[] OSSL_PARAM params[]
{ {
OSSL_PARAM_size_t(OSSL_PKEY_PARAM_FFC_PBITS, &keyLength), OSSL_PARAM_size_t(OSSL_PKEY_PARAM_FFC_PBITS, &keyLength),
@@ -862,11 +890,14 @@ void Context::initDH(bool use2048Bits, const std::string& dhParamsFile)
throw SSLContextException(Poco::format("Context::initDH(%s):EVP_PKEY*", dhParamsFile)); throw SSLContextException(Poco::format("Context::initDH(%s):EVP_PKEY*", dhParamsFile));
} }
SSL_CTX_set0_tmp_dh_pkey(_pSSLContext, pKey); if (!SSL_CTX_set0_tmp_dh_pkey(_pSSLContext, pKey))
{
if (freeEVPPKey) EVP_PKEY_free(pKey);
std::string err = "Context::initDH():SSL_CTX_set0_tmp_dh_pkey()\n";
throw SSLContextException(Poco::Crypto::getError(err));
}
SSL_CTX_set_options(_pSSLContext, SSL_OP_SINGLE_DH_USE); SSL_CTX_set_options(_pSSLContext, SSL_OP_SINGLE_DH_USE);
if (freeEVPPKey) EVP_PKEY_free(pKey);
#else // OPENSSL_VERSION_NUMBER >= 0x30000000L #else // OPENSSL_VERSION_NUMBER >= 0x30000000L
DH* dh = 0; DH* dh = 0;
@@ -899,20 +930,25 @@ void Context::initDH(bool use2048Bits, const std::string& dhParamsFile)
BIGNUM* p = nullptr; BIGNUM* p = nullptr;
BIGNUM* g = nullptr; BIGNUM* g = nullptr;
if (use2048Bits) if (keyDHGroup == KEY_DH_GROUP_2048)
{ {
p = BN_bin2bn(dh2048_p, sizeof(dh2048_p), 0); p = BN_bin2bn(dh2048_p, sizeof(dh2048_p), 0);
g = BN_bin2bn(dh2048_g, sizeof(dh2048_g), 0); g = BN_bin2bn(dh2048_g, sizeof(dh2048_g), 0);
DH_set0_pqg(dh, p, 0, g); DH_set0_pqg(dh, p, 0, g);
DH_set_length(dh, 256); DH_set_length(dh, 256);
} }
else else if (keyDHGroup == KEY_DH_GROUP_1024)
{ {
p = BN_bin2bn(dh1024_p, sizeof(dh1024_p), 0); p = BN_bin2bn(dh1024_p, sizeof(dh1024_p), 0);
g = BN_bin2bn(dh1024_g, sizeof(dh1024_g), 0); g = BN_bin2bn(dh1024_g, sizeof(dh1024_g), 0);
DH_set0_pqg(dh, p, 0, g); DH_set0_pqg(dh, p, 0, g);
DH_set_length(dh, 160); DH_set_length(dh, 160);
} }
else
{
throw Poco::NotImplementedException(Poco::format(
"DH Group: %d", static_cast<int>(keyDHGroup)));
}
if (!p || !g) if (!p || !g)
{ {
DH_free(dh); DH_free(dh);
@@ -921,18 +957,22 @@ void Context::initDH(bool use2048Bits, const std::string& dhParamsFile)
#else // OPENSSL_VERSION_NUMBER >= 0x10100000L && !defined(LIBRESSL_VERSION_NUMBER) #else // OPENSSL_VERSION_NUMBER >= 0x10100000L && !defined(LIBRESSL_VERSION_NUMBER)
if (use2048Bits) if (keyDHGroup == KEY_DH_GROUP_2048)
{ {
dh->p = BN_bin2bn(dh2048_p, sizeof(dh2048_p), 0); dh->p = BN_bin2bn(dh2048_p, sizeof(dh2048_p), 0);
dh->g = BN_bin2bn(dh2048_g, sizeof(dh2048_g), 0); dh->g = BN_bin2bn(dh2048_g, sizeof(dh2048_g), 0);
dh->length = 256; dh->length = 256;
} }
else else if (keyDHGroup == KEY_DH_GROUP_1024)
{ {
dh->p = BN_bin2bn(dh1024_p, sizeof(dh1024_p), 0); dh->p = BN_bin2bn(dh1024_p, sizeof(dh1024_p), 0);
dh->g = BN_bin2bn(dh1024_g, sizeof(dh1024_g), 0); dh->g = BN_bin2bn(dh1024_g, sizeof(dh1024_g), 0);
dh->length = 160; dh->length = 160;
} }
{
throw Poco::NotImplementedException(Poco::format(
"DH Group: %d", static_cast<int>(keyDHGroup)));
}
if ((!dh->p) || (!dh->g)) if ((!dh->p) || (!dh->g))
{ {
DH_free(dh); DH_free(dh);