Skip to content

Commit

Permalink
fix(OpenSSL) Openssl DH key size (#4753)
Browse files Browse the repository at this point in the history 
* 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>
  • Loading branch information
@aleks-f @pkl97
aleks-f and pkl97 authored Nov 11, 2024
1 parent 9530a77 commit c4f66d5
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 45 deletions.
23 changes: 19 additions & 4 deletions NetSSL_OpenSSL/include/Poco/Net/Context.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,24 @@ class NetSSL_API Context: public Poco::RefCountedObject
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
{
Params();
Params(KeyDHGroup dhBits = KEY_DH_GROUP_2048);
/// Initializes the struct with default values.

std::string privateKeyFile;
Expand Down Expand Up @@ -181,7 +196,7 @@ class NetSSL_API Context: public Poco::RefCountedObject
/// Specifies a file containing Diffie-Hellman parameters.
/// If empty, the default parameters are used.

bool dhUse2048Bits;
KeyDHGroup dhGroup;
/// If set to true, will use 2048-bit MODP Group with 256-bit
/// prime order subgroup (RFC5114) instead of 1024-bit for DH.

Expand Down Expand Up @@ -441,7 +456,7 @@ class NetSSL_API Context: public Poco::RefCountedObject

void ignoreUnexpectedEof(bool flag = true);
/// Enable or disable SSL/TLS SSL_OP_IGNORE_UNEXPECTED_EOF
///
///
/// Some TLS implementations do not send the mandatory close_notify alert on shutdown.
/// If the application tries to wait for the close_notify alert
/// but the peer closes the connection without sending it, an error is generated.
Expand All @@ -458,7 +473,7 @@ class NetSSL_API Context: public Poco::RefCountedObject
void init(const Params& params);
/// 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.

void initECDH(const std::string& curve);
Expand Down
122 changes: 81 additions & 41 deletions NetSSL_OpenSSL/src/Context.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ namespace Poco {
namespace Net {


Context::Params::Params():
Context::Params::Params(KeyDHGroup dhBits):
verificationMode(VERIFY_RELAXED),
verificationDepth(9),
loadDefaultCAs(false),
ocspStaplingVerification(false),
cipherList("ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH"),
dhUse2048Bits(false),
dhGroup(dhBits),
securityLevel(SECURITY_LEVEL_NONE)
{
}
Expand Down Expand Up @@ -215,7 +215,7 @@ void Context::init(const Params& params)
#endif
}

initDH(params.dhUse2048Bits, params.dhParamsFile);
initDH(params.dhGroup, params.dhParamsFile);
initECDH(params.ecdhCurve);
}
catch (...)
Expand Down Expand Up @@ -596,59 +596,59 @@ void Context::createSSLContext()
* 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.
*/
case TLSV1_1_CLIENT_USE:
case TLSV1_1_CLIENT_USE:
#if OPENSSL_VERSION_NUMBER >= 0x10100000L
_pSSLContext = SSL_CTX_new(TLS_client_method());
minTLSVersion = TLS1_1_VERSION;
#else
_pSSLContext = SSL_CTX_new(TLSv1_1_client_method());
_pSSLContext = SSL_CTX_new(TLSv1_1_client_method());
#endif
break;
break;

case TLSV1_1_SERVER_USE:
case TLSV1_1_SERVER_USE:
#if OPENSSL_VERSION_NUMBER >= 0x10100000L
_pSSLContext = SSL_CTX_new(TLS_server_method());
_pSSLContext = SSL_CTX_new(TLS_server_method());
minTLSVersion = TLS1_1_VERSION;
#else
_pSSLContext = SSL_CTX_new(TLSv1_1_server_method());
_pSSLContext = SSL_CTX_new(TLSv1_1_server_method());
#endif
break;
break;
#endif

#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
_pSSLContext = SSL_CTX_new(TLS_client_method());
minTLSVersion = TLS1_2_VERSION;
_pSSLContext = SSL_CTX_new(TLS_client_method());
minTLSVersion = TLS1_2_VERSION;
#else
_pSSLContext = SSL_CTX_new(TLSv1_2_client_method());
_pSSLContext = SSL_CTX_new(TLSv1_2_client_method());
#endif
break;
break;

case TLSV1_2_SERVER_USE:
case TLSV1_2_SERVER_USE:
#if OPENSSL_VERSION_NUMBER >= 0x10100000L
_pSSLContext = SSL_CTX_new(TLS_server_method());
minTLSVersion = TLS1_2_VERSION;
_pSSLContext = SSL_CTX_new(TLS_server_method());
minTLSVersion = TLS1_2_VERSION;
#else
_pSSLContext = SSL_CTX_new(TLSv1_2_server_method());
_pSSLContext = SSL_CTX_new(TLSv1_2_server_method());
#endif
break;
break;
#endif

#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
_pSSLContext = SSL_CTX_new(TLS_client_method());
minTLSVersion = TLS1_3_VERSION;
_pSSLContext = SSL_CTX_new(TLS_client_method());
minTLSVersion = TLS1_3_VERSION;
#endif
break;
break;

case TLSV1_3_SERVER_USE:
case TLSV1_3_SERVER_USE:
#if OPENSSL_VERSION_NUMBER >= 0x10101000L
_pSSLContext = SSL_CTX_new(TLS_server_method());
minTLSVersion = TLS1_3_VERSION;
_pSSLContext = SSL_CTX_new(TLS_server_method());
minTLSVersion = TLS1_3_VERSION;
#endif
break;
break;
#endif

default:
Expand Down Expand Up @@ -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
static const unsigned char dh1024_p[] =
Expand Down Expand Up @@ -828,11 +828,39 @@ void Context::initDH(bool use2048Bits, const std::string& dhParamsFile)
throw Poco::NullPointerException(Poco::Crypto::getError(err));
}

size_t keyLength = use2048Bits ? 256 : 160;
unsigned char* pDH_p = const_cast<unsigned char*>(use2048Bits ? dh2048_p : dh1024_p);
std::size_t sz_p = use2048Bits ? sizeof(dh2048_p) : sizeof(dh1024_p);
unsigned char* pDH_g = const_cast<unsigned char*>(use2048Bits ? dh2048_g : dh1024_g);
std::size_t sz_g = use2048Bits ? sizeof(dh2048_g) : sizeof(dh1024_g);
size_t keyLength = 0;
unsigned char* pDH_p = nullptr;
std::size_t sz_p = 0;
unsigned char* pDH_g = nullptr;
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_size_t(OSSL_PKEY_PARAM_FFC_PBITS, &keyLength),
Expand Down Expand Up @@ -862,11 +890,14 @@ void Context::initDH(bool use2048Bits, const std::string& 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);

if (freeEVPPKey) EVP_PKEY_free(pKey);

#else // OPENSSL_VERSION_NUMBER >= 0x30000000L

DH* dh = 0;
Expand Down Expand Up @@ -899,20 +930,25 @@ void Context::initDH(bool use2048Bits, const std::string& dhParamsFile)

BIGNUM* p = nullptr;
BIGNUM* g = nullptr;
if (use2048Bits)
if (keyDHGroup == KEY_DH_GROUP_2048)
{
p = BN_bin2bn(dh2048_p, sizeof(dh2048_p), 0);
g = BN_bin2bn(dh2048_g, sizeof(dh2048_g), 0);
DH_set0_pqg(dh, p, 0, g);
DH_set_length(dh, 256);
}
else
else if (keyDHGroup == KEY_DH_GROUP_1024)
{
p = BN_bin2bn(dh1024_p, sizeof(dh1024_p), 0);
g = BN_bin2bn(dh1024_g, sizeof(dh1024_g), 0);
DH_set0_pqg(dh, p, 0, g);
DH_set_length(dh, 160);
}
else
{
throw Poco::NotImplementedException(Poco::format(
"DH Group: %d", static_cast<int>(keyDHGroup)));
}
if (!p || !g)
{
DH_free(dh);
Expand All @@ -921,18 +957,22 @@ void Context::initDH(bool use2048Bits, const std::string& dhParamsFile)

#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->g = BN_bin2bn(dh2048_g, sizeof(dh2048_g), 0);
dh->length = 256;
}
else
else if (keyDHGroup == KEY_DH_GROUP_1024)
{
dh->p = BN_bin2bn(dh1024_p, sizeof(dh1024_p), 0);
dh->g = BN_bin2bn(dh1024_g, sizeof(dh1024_g), 0);
dh->length = 160;
}
{
throw Poco::NotImplementedException(Poco::format(
"DH Group: %d", static_cast<int>(keyDHGroup)));
}
if ((!dh->p) || (!dh->g))
{
DH_free(dh);
Expand Down

0 comments on commit c4f66d5

Please sign in to comment.