Commit 2bc9121fa16a274093f6756164a52c30ecb7496c
1 parent
b7459209
Fix major performance bug with openssl crypto (fixes #798)
Lazily load MD5 and RC4 once in the life of the program. Only load the legacy provider if RC4 is actually being used.
Showing
4 changed files
with
73 additions
and
36 deletions
ChangeLog
| 1 | 2022-10-08 Jay Berkenbilt <ejb@ql.org> | 1 | 2022-10-08 Jay Berkenbilt <ejb@ql.org> |
| 2 | 2 | ||
| 3 | + * Fix major performance bug with the openssl crypto provider when | ||
| 4 | + using OpenSSL 3. The legacy loader and rc4 algorithm was being | ||
| 5 | + loaded with every call to the crypto provider instead of once in | ||
| 6 | + the life of the program. Fixes #798. | ||
| 7 | + | ||
| 3 | * performance_check: add --test option to limit which tests are | 8 | * performance_check: add --test option to limit which tests are |
| 4 | run. | 9 | run. |
| 5 | 10 |
libqpdf/QPDFCrypto_openssl.cc
| 1 | #include <qpdf/QPDFCrypto_openssl.hh> | 1 | #include <qpdf/QPDFCrypto_openssl.hh> |
| 2 | 2 | ||
| 3 | #include <cstring> | 3 | #include <cstring> |
| 4 | +#include <memory> | ||
| 4 | #include <stdexcept> | 5 | #include <stdexcept> |
| 5 | #include <string> | 6 | #include <string> |
| 6 | 7 | ||
| @@ -18,6 +19,60 @@ | @@ -18,6 +19,60 @@ | ||
| 18 | 19 | ||
| 19 | #include <qpdf/QIntC.hh> | 20 | #include <qpdf/QIntC.hh> |
| 20 | 21 | ||
| 22 | +#ifndef QPDF_OPENSSL_1 | ||
| 23 | +namespace | ||
| 24 | +{ | ||
| 25 | + class RC4Loader | ||
| 26 | + { | ||
| 27 | + public: | ||
| 28 | + static EVP_CIPHER const* getRC4(); | ||
| 29 | + ~RC4Loader(); | ||
| 30 | + | ||
| 31 | + private: | ||
| 32 | + RC4Loader(); | ||
| 33 | + OSSL_PROVIDER* legacy; | ||
| 34 | + OSSL_LIB_CTX* libctx; | ||
| 35 | + EVP_CIPHER* rc4; | ||
| 36 | + }; | ||
| 37 | +} // namespace | ||
| 38 | + | ||
| 39 | +EVP_CIPHER const* | ||
| 40 | +RC4Loader::getRC4() | ||
| 41 | +{ | ||
| 42 | + static auto loader = std::shared_ptr<RC4Loader>(new RC4Loader()); | ||
| 43 | + return loader->rc4; | ||
| 44 | +} | ||
| 45 | + | ||
| 46 | +RC4Loader::RC4Loader() | ||
| 47 | +{ | ||
| 48 | + libctx = OSSL_LIB_CTX_new(); | ||
| 49 | + if (libctx == nullptr) { | ||
| 50 | + throw std::runtime_error("unable to create openssl library context"); | ||
| 51 | + return; | ||
| 52 | + } | ||
| 53 | + legacy = OSSL_PROVIDER_load(libctx, "legacy"); | ||
| 54 | + if (legacy == nullptr) { | ||
| 55 | + OSSL_LIB_CTX_free(libctx); | ||
| 56 | + throw std::runtime_error("unable to load openssl legacy provider"); | ||
| 57 | + return; | ||
| 58 | + } | ||
| 59 | + rc4 = EVP_CIPHER_fetch(libctx, "RC4", nullptr); | ||
| 60 | + if (rc4 == nullptr) { | ||
| 61 | + OSSL_PROVIDER_unload(legacy); | ||
| 62 | + OSSL_LIB_CTX_free(libctx); | ||
| 63 | + throw std::runtime_error("unable to load openssl rc4 algorithm"); | ||
| 64 | + return; | ||
| 65 | + } | ||
| 66 | +} | ||
| 67 | + | ||
| 68 | +RC4Loader::~RC4Loader() | ||
| 69 | +{ | ||
| 70 | + EVP_CIPHER_free(rc4); | ||
| 71 | + OSSL_PROVIDER_unload(legacy); | ||
| 72 | + OSSL_LIB_CTX_free(libctx); | ||
| 73 | +} | ||
| 74 | +#endif // not QPDF_OPENSSL_1 | ||
| 75 | + | ||
| 21 | static void | 76 | static void |
| 22 | bad_bits(int bits) | 77 | bad_bits(int bits) |
| 23 | { | 78 | { |
| @@ -41,32 +96,9 @@ check_openssl(int status) | @@ -41,32 +96,9 @@ check_openssl(int status) | ||
| 41 | } | 96 | } |
| 42 | 97 | ||
| 43 | QPDFCrypto_openssl::QPDFCrypto_openssl() : | 98 | QPDFCrypto_openssl::QPDFCrypto_openssl() : |
| 44 | -#ifdef QPDF_OPENSSL_1 | ||
| 45 | - rc4(EVP_rc4()), | ||
| 46 | -#endif | ||
| 47 | md_ctx(EVP_MD_CTX_new()), | 99 | md_ctx(EVP_MD_CTX_new()), |
| 48 | cipher_ctx(EVP_CIPHER_CTX_new()) | 100 | cipher_ctx(EVP_CIPHER_CTX_new()) |
| 49 | { | 101 | { |
| 50 | -#ifndef QPDF_OPENSSL_1 | ||
| 51 | - libctx = OSSL_LIB_CTX_new(); | ||
| 52 | - if (libctx == nullptr) { | ||
| 53 | - throw std::runtime_error("unable to create openssl library context"); | ||
| 54 | - return; | ||
| 55 | - } | ||
| 56 | - legacy = OSSL_PROVIDER_load(libctx, "legacy"); | ||
| 57 | - if (legacy == nullptr) { | ||
| 58 | - OSSL_LIB_CTX_free(libctx); | ||
| 59 | - throw std::runtime_error("unable to load openssl legacy provider"); | ||
| 60 | - return; | ||
| 61 | - } | ||
| 62 | - rc4 = EVP_CIPHER_fetch(libctx, "RC4", nullptr); | ||
| 63 | - if (rc4 == nullptr) { | ||
| 64 | - OSSL_PROVIDER_unload(legacy); | ||
| 65 | - OSSL_LIB_CTX_free(libctx); | ||
| 66 | - throw std::runtime_error("unable to load openssl rc4 algorithm"); | ||
| 67 | - return; | ||
| 68 | - } | ||
| 69 | -#endif | ||
| 70 | memset(md_out, 0, sizeof(md_out)); | 102 | memset(md_out, 0, sizeof(md_out)); |
| 71 | EVP_MD_CTX_init(md_ctx); | 103 | EVP_MD_CTX_init(md_ctx); |
| 72 | EVP_CIPHER_CTX_init(cipher_ctx); | 104 | EVP_CIPHER_CTX_init(cipher_ctx); |
| @@ -77,11 +109,6 @@ QPDFCrypto_openssl::~QPDFCrypto_openssl() | @@ -77,11 +109,6 @@ QPDFCrypto_openssl::~QPDFCrypto_openssl() | ||
| 77 | EVP_MD_CTX_reset(md_ctx); | 109 | EVP_MD_CTX_reset(md_ctx); |
| 78 | EVP_CIPHER_CTX_reset(cipher_ctx); | 110 | EVP_CIPHER_CTX_reset(cipher_ctx); |
| 79 | EVP_CIPHER_CTX_free(cipher_ctx); | 111 | EVP_CIPHER_CTX_free(cipher_ctx); |
| 80 | -#ifndef QPDF_OPENSSL_1 | ||
| 81 | - EVP_CIPHER_free(rc4); | ||
| 82 | - OSSL_PROVIDER_unload(legacy); | ||
| 83 | - OSSL_LIB_CTX_free(libctx); | ||
| 84 | -#endif | ||
| 85 | EVP_MD_CTX_free(md_ctx); | 112 | EVP_MD_CTX_free(md_ctx); |
| 86 | } | 113 | } |
| 87 | 114 | ||
| @@ -101,7 +128,7 @@ QPDFCrypto_openssl::MD5_init() | @@ -101,7 +128,7 @@ QPDFCrypto_openssl::MD5_init() | ||
| 101 | void | 128 | void |
| 102 | QPDFCrypto_openssl::SHA2_init(int bits) | 129 | QPDFCrypto_openssl::SHA2_init(int bits) |
| 103 | { | 130 | { |
| 104 | - const EVP_MD* md = EVP_sha512(); | 131 | + static const EVP_MD* md = EVP_sha512(); |
| 105 | switch (bits) { | 132 | switch (bits) { |
| 106 | case 256: | 133 | case 256: |
| 107 | md = EVP_sha256(); | 134 | md = EVP_sha256(); |
| @@ -174,6 +201,11 @@ QPDFCrypto_openssl::SHA2_digest() | @@ -174,6 +201,11 @@ QPDFCrypto_openssl::SHA2_digest() | ||
| 174 | void | 201 | void |
| 175 | QPDFCrypto_openssl::RC4_init(unsigned char const* key_data, int key_len) | 202 | QPDFCrypto_openssl::RC4_init(unsigned char const* key_data, int key_len) |
| 176 | { | 203 | { |
| 204 | +#ifdef QPDF_OPENSSL_1 | ||
| 205 | + static auto const rc4 = EVP_rc4(); | ||
| 206 | +#else | ||
| 207 | + static auto const rc4 = RC4Loader::getRC4(); | ||
| 208 | +#endif | ||
| 177 | check_openssl(EVP_CIPHER_CTX_reset(cipher_ctx)); | 209 | check_openssl(EVP_CIPHER_CTX_reset(cipher_ctx)); |
| 178 | if (key_len == -1) { | 210 | if (key_len == -1) { |
| 179 | key_len = | 211 | key_len = |
libqpdf/qpdf/QPDFCrypto_openssl.hh
| @@ -58,13 +58,6 @@ class QPDFCrypto_openssl: public QPDFCryptoImpl | @@ -58,13 +58,6 @@ class QPDFCrypto_openssl: public QPDFCryptoImpl | ||
| 58 | void rijndael_finalize() override; | 58 | void rijndael_finalize() override; |
| 59 | 59 | ||
| 60 | private: | 60 | private: |
| 61 | -#ifdef QPDF_OPENSSL_1 | ||
| 62 | - EVP_CIPHER const* rc4; | ||
| 63 | -#else | ||
| 64 | - OSSL_LIB_CTX* libctx; | ||
| 65 | - OSSL_PROVIDER* legacy; | ||
| 66 | - EVP_CIPHER* rc4; | ||
| 67 | -#endif | ||
| 68 | EVP_MD_CTX* const md_ctx; | 61 | EVP_MD_CTX* const md_ctx; |
| 69 | EVP_CIPHER_CTX* const cipher_ctx; | 62 | EVP_CIPHER_CTX* const cipher_ctx; |
| 70 | uint8_t md_out[EVP_MAX_MD_SIZE]; | 63 | uint8_t md_out[EVP_MAX_MD_SIZE]; |
manual/release-notes.rst
| @@ -13,6 +13,13 @@ For a detailed list of changes, please see the file | @@ -13,6 +13,13 @@ For a detailed list of changes, please see the file | ||
| 13 | 13 | ||
| 14 | - A C++-17 compiler is now required. | 14 | - A C++-17 compiler is now required. |
| 15 | 15 | ||
| 16 | + - Bug fixes | ||
| 17 | + | ||
| 18 | + - Fix major performance bug with the OpenSSL crypto provider. This | ||
| 19 | + bug was causing a 6x to 12x slowdown for encrypted files when | ||
| 20 | + OpenSSL 3 was in use. This includes the default Windows builds | ||
| 21 | + distributed with the qpdf release. | ||
| 22 | + | ||
| 16 | 11.1.1: October 1, 2022 | 23 | 11.1.1: October 1, 2022 |
| 17 | - Bug fixes | 24 | - Bug fixes |
| 18 | 25 |