From 10816a7b419cba55de3e788323ef9383cf8e7175 Mon Sep 17 00:00:00 2001 From: John Thiltges Date: Tue, 18 Jan 2022 18:24:15 -0600 Subject: [PATCH 1/3] [XrdCrypto] Generate DH parameters on first call to XrdCryptosslCipher --- src/XrdCrypto/XrdCryptosslCipher.cc | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/XrdCrypto/XrdCryptosslCipher.cc b/src/XrdCrypto/XrdCryptosslCipher.cc index 0ce116d83b6..701e6edf142 100644 --- a/src/XrdCrypto/XrdCryptosslCipher.cc +++ b/src/XrdCrypto/XrdCryptosslCipher.cc @@ -32,6 +32,7 @@ /* */ /* ************************************************************************** */ #include +#include #include "XrdSut/XrdSutRndm.hh" #include "XrdCrypto/XrdCryptosslTrace.hh" @@ -459,7 +460,7 @@ XrdCryptosslCipher::XrdCryptosslCipher(bool padded, int bits, char *pub, // Constructor for key agreement. // If pub is not defined, generates a DH full key, // the public part and parameters can be retrieved using Public(). - // The number of random bits to be used in 'bits'. + // 'bits' is ignored (DH key is generated once) // If pub is defined with the public part and parameters of the // counterpart fully initialize a cipher with that information. // Sets also the name to 't', if different from the default one. @@ -474,15 +475,21 @@ XrdCryptosslCipher::XrdCryptosslCipher(bool padded, int bits, char *pub, cipher = 0; deflength = 1; + static DH *dhparms = [] { + DH *dh = DH_new(); + DEBUG("generate DH parameters"); + DH_generate_parameters_ex(dh, kDHMINBITS, DH_GENERATOR_5, NULL); + DEBUG("generate DH parameters done"); + return dh; + }(); + + assert(DH_get0_p(dhparms)); + if (!pub) { - DEBUG("generate DH full key"); - // - // at least 128 bits - bits = (bits < kDHMINBITS) ? kDHMINBITS : bits; - // - // Generate params for DH object - fDH = DH_new(); - if (fDH && DH_generate_parameters_ex(fDH, bits, DH_GENERATOR_5, NULL)) { + DEBUG("configure DH parameters"); + // Set params for DH object + fDH = DHparams_dup(dhparms); + if (fDH) { int prc = 0; DH_check(fDH,&prc); if (prc == 0) { From 0b8b6341344b1d7ce6e8a4930d476978a69329cd Mon Sep 17 00:00:00 2001 From: John Thiltges Date: Thu, 20 Jan 2022 15:41:59 -0600 Subject: [PATCH 2/3] [XrdCrypto] DH parameter generation cleanup - Move DH parameter generation inside server scope - Avoid overhead of generating DH parameters on client - Remove DH_check() call on server side - Adds overhead, and the server-generated key should already be safe - Include cassert instead of assert.h --- src/XrdCrypto/XrdCryptosslCipher.cc | 39 +++++++++++++---------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/src/XrdCrypto/XrdCryptosslCipher.cc b/src/XrdCrypto/XrdCryptosslCipher.cc index 701e6edf142..cea59522ee2 100644 --- a/src/XrdCrypto/XrdCryptosslCipher.cc +++ b/src/XrdCrypto/XrdCryptosslCipher.cc @@ -32,7 +32,7 @@ /* */ /* ************************************************************************** */ #include -#include +#include #include "XrdSut/XrdSutRndm.hh" #include "XrdCrypto/XrdCryptosslTrace.hh" @@ -475,32 +475,27 @@ XrdCryptosslCipher::XrdCryptosslCipher(bool padded, int bits, char *pub, cipher = 0; deflength = 1; - static DH *dhparms = [] { - DH *dh = DH_new(); - DEBUG("generate DH parameters"); - DH_generate_parameters_ex(dh, kDHMINBITS, DH_GENERATOR_5, NULL); - DEBUG("generate DH parameters done"); - return dh; - }(); - - assert(DH_get0_p(dhparms)); - if (!pub) { + static DH *dhparms = [] { + DH *dh = DH_new(); + DEBUG("generate DH parameters"); + DH_generate_parameters_ex(dh, kDHMINBITS, DH_GENERATOR_5, NULL); + DEBUG("generate DH parameters done"); + return dh; + }(); + DEBUG("configure DH parameters"); // Set params for DH object + assert(DH_get0_p(dhparms)); fDH = DHparams_dup(dhparms); if (fDH) { - int prc = 0; - DH_check(fDH,&prc); - if (prc == 0) { - // - // Generate DH key - if (DH_generate_key(fDH)) { - // Init context - ctx = EVP_CIPHER_CTX_new(); - if (ctx) - valid = 1; - } + // + // Generate DH key + if (DH_generate_key(fDH)) { + // Init context + ctx = EVP_CIPHER_CTX_new(); + if (ctx) + valid = 1; } } From a1dab7fe4cab4aabf9c8ba48d42389af155a479e Mon Sep 17 00:00:00 2001 From: John Thiltges Date: Thu, 20 Jan 2022 15:37:35 -0600 Subject: [PATCH 3/3] [XrdCrypto] Generate DH parameters on first call to openssl3/XrdCryptosslCipher --- src/XrdCrypto/openssl3/XrdCryptosslCipher.cc | 54 ++++++++++---------- 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/src/XrdCrypto/openssl3/XrdCryptosslCipher.cc b/src/XrdCrypto/openssl3/XrdCryptosslCipher.cc index 93fc01b952a..6fc7bc35f60 100644 --- a/src/XrdCrypto/openssl3/XrdCryptosslCipher.cc +++ b/src/XrdCrypto/openssl3/XrdCryptosslCipher.cc @@ -32,6 +32,7 @@ /* */ /* ************************************************************************** */ #include +#include #include "XrdSut/XrdSutRndm.hh" #include "XrdCrypto/XrdCryptosslTrace.hh" @@ -487,7 +488,7 @@ XrdCryptosslCipher::XrdCryptosslCipher(bool padded, int bits, char *pub, // Constructor for key agreement. // If pub is not defined, generates a DH full key, // the public part and parameters can be retrieved using Public(). - // The number of random bits to be used in 'bits'. + // 'bits' is ignored (DH key is generated once) // If pub is defined with the public part and parameters of the // counterpart fully initialize a cipher with that information. // Sets also the name to 't', if different from the default one. @@ -503,35 +504,32 @@ XrdCryptosslCipher::XrdCryptosslCipher(bool padded, int bits, char *pub, deflength = 1; if (!pub) { - DEBUG("generate DH full key"); + static EVP_PKEY *dhparms = [] { + DEBUG("generate DH parameters"); + EVP_PKEY *dhParam = 0; + EVP_PKEY_CTX *pkctx = EVP_PKEY_CTX_new_id(EVP_PKEY_DH, 0); + EVP_PKEY_paramgen_init(pkctx); + EVP_PKEY_CTX_set_dh_paramgen_prime_len(pkctx, kDHMINBITS); + EVP_PKEY_CTX_set_dh_paramgen_generator(pkctx, 5); + EVP_PKEY_paramgen(pkctx, &dhParam); + EVP_PKEY_CTX_free(pkctx); + DEBUG("generate DH parameters done"); + return dhParam; + }(); + + DEBUG("configure DH parameters"); // - // at least 128 bits - bits = (bits < kDHMINBITS) ? kDHMINBITS : bits; - // - // Generate params for DH object - EVP_PKEY *dhParam = 0; - EVP_PKEY_CTX *pkctx = EVP_PKEY_CTX_new_id(EVP_PKEY_DH, 0); - EVP_PKEY_paramgen_init(pkctx); - EVP_PKEY_CTX_set_dh_paramgen_prime_len(pkctx, bits); - EVP_PKEY_CTX_set_dh_paramgen_generator(pkctx, 5); - EVP_PKEY_paramgen(pkctx, &dhParam); + // Set params for DH object + assert(dhparms); + EVP_PKEY_CTX *pkctx = EVP_PKEY_CTX_new(dhparms, 0); + EVP_PKEY_keygen_init(pkctx); + EVP_PKEY_keygen(pkctx, &fDH); EVP_PKEY_CTX_free(pkctx); - if (dhParam) { - if (XrdCheckDH(dhParam) == 1) { - // - // Generate DH key - pkctx = EVP_PKEY_CTX_new(dhParam, 0); - EVP_PKEY_keygen_init(pkctx); - EVP_PKEY_keygen(pkctx, &fDH); - EVP_PKEY_CTX_free(pkctx); - if (fDH) { - // Init context - ctx = EVP_CIPHER_CTX_new(); - if (ctx) - valid = 1; - } - } - EVP_PKEY_free(dhParam); + if (fDH) { + // Init context + ctx = EVP_CIPHER_CTX_new(); + if (ctx) + valid = 1; } } else {