From: Fred Morcos Date: Sat, 12 Nov 2022 04:37:21 +0000 (+0100) Subject: Cleanup OpenSSL RSA DCKE X-Git-Tag: dnsdist-1.8.0-rc1~89^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F12414%2Fhead;p=thirdparty%2Fpdns.git Cleanup OpenSSL RSA DCKE --- diff --git a/pdns/opensslsigners.cc b/pdns/opensslsigners.cc index d720a7575e..1348bcd069 100644 --- a/pdns/opensslsigners.cc +++ b/pdns/opensslsigners.cc @@ -178,7 +178,8 @@ void openssl_seed() class OpenSSLRSADNSCryptoKeyEngine : public DNSCryptoKeyEngine { public: - explicit OpenSSLRSADNSCryptoKeyEngine(unsigned int algo): DNSCryptoKeyEngine(algo), d_key(std::unique_ptr(nullptr, RSA_free)) + explicit OpenSSLRSADNSCryptoKeyEngine(unsigned int algo) : + DNSCryptoKeyEngine(algo), d_key(std::unique_ptr(nullptr, RSA_free)) { int ret = RAND_status(); if (ret != 1) { @@ -190,8 +191,8 @@ public: { } - string getName() const override { return "OpenSSL RSA"; } - int getBits() const override { return RSA_size(d_key.get()) << 3; } + [[nodiscard]] string getName() const override { return "OpenSSL RSA"; } + [[nodiscard]] int getBits() const override { return RSA_size(d_key.get()) << 3; } void create(unsigned int bits) override; @@ -206,11 +207,11 @@ public: * \param[in] filename Only used for providing filename information in error * messages. * - * \param[in] fp An open file handle to a file containing RSA PEM contents. + * \param[in] inputFile An open file handle to a file containing RSA PEM contents. * * \return An RSA key engine populated with the contents of the PEM file. */ - void createFromPEMFile(DNSKEYRecordContent& drc, const std::string& filename, std::FILE& fp) override; + void createFromPEMFile(DNSKEYRecordContent& drc, const std::string& filename, std::FILE& inputFile) override; /** * \brief Writes this key's contents to a file. @@ -218,18 +219,18 @@ public: * Receives an open file handle and writes this key's contents to the * file. * - * \param[in] fp An open file handle for writing. + * \param[in] outputFile An open file handle for writing. * * \exception std::runtime_error In case of OpenSSL errors. */ - void convertToPEM(std::FILE& fp) const override; - - storvector_t convertToISCVector() const override; - std::string hash(const std::string& hash) const override; - std::string sign(const std::string& hash) const override; - bool verify(const std::string& hash, const std::string& signature) const override; - std::string getPublicKeyString() const override; - std::unique_ptr parse(std::map& stormap, const std::string& key) const; + void convertToPEM(std::FILE& outputFile) const override; + + [[nodiscard]] storvector_t convertToISCVector() const override; + [[nodiscard]] std::string hash(const std::string& message) const override; + [[nodiscard]] std::string sign(const std::string& message) const override; + [[nodiscard]] bool verify(const std::string& message, const std::string& signature) const override; + [[nodiscard]] std::string getPublicKeyString() const override; + std::unique_ptr parse(std::map& stormap, const std::string& key) const; void fromISCMap(DNSKEYRecordContent& drc, std::map& stormap) override; void fromPublicKeyString(const std::string& content) override; bool checkKey(vector* errorMessages) const override; @@ -242,7 +243,7 @@ public: private: static int hashSizeToKind(size_t hashSize); - std::unique_ptr d_key; + std::unique_ptr d_key; }; void OpenSSLRSADNSCryptoKeyEngine::create(unsigned int bits) @@ -261,24 +262,24 @@ void OpenSSLRSADNSCryptoKeyEngine::create(unsigned int bits) throw runtime_error(getName() + " RSASHA512 key generation failed for invalid bits size " + std::to_string(bits)); } - auto e = std::unique_ptr(BN_new(), BN_clear_free); - if (!e) { + auto exponent = std::unique_ptr(BN_new(), BN_clear_free); + if (!exponent) { throw runtime_error(getName() + " key generation failed, unable to allocate e"); } /* RSA_F4 is a public exponent value of 65537 */ - int res = BN_set_word(e.get(), RSA_F4); + int res = BN_set_word(exponent.get(), RSA_F4); if (res == 0) { throw runtime_error(getName() + " key generation failed while setting e"); } - auto key = std::unique_ptr(RSA_new(), RSA_free); + auto key = std::unique_ptr(RSA_new(), RSA_free); if (!key) { throw runtime_error(getName() + " allocation of key structure failed"); } - res = RSA_generate_key_ex(key.get(), bits, e.get(), nullptr); + res = RSA_generate_key_ex(key.get(), bits, exponent.get(), nullptr); if (res == 0) { throw runtime_error(getName() + " key generation failed"); } @@ -286,18 +287,18 @@ void OpenSSLRSADNSCryptoKeyEngine::create(unsigned int bits) d_key = std::move(key); } -void OpenSSLRSADNSCryptoKeyEngine::createFromPEMFile(DNSKEYRecordContent& drc, const std::string& filename, std::FILE& fp) +void OpenSSLRSADNSCryptoKeyEngine::createFromPEMFile(DNSKEYRecordContent& drc, const std::string& filename, std::FILE& inputFile) { drc.d_algorithm = d_algorithm; - d_key = std::unique_ptr(PEM_read_RSAPrivateKey(&fp, nullptr, nullptr, nullptr), &RSA_free); + d_key = std::unique_ptr(PEM_read_RSAPrivateKey(&inputFile, nullptr, nullptr, nullptr), &RSA_free); if (d_key == nullptr) { throw runtime_error(getName() + ": Failed to read private key from PEM file `" + filename + "`"); } } -void OpenSSLRSADNSCryptoKeyEngine::convertToPEM(std::FILE& fp) const +void OpenSSLRSADNSCryptoKeyEngine::convertToPEM(std::FILE& outputFile) const { - auto ret = PEM_write_RSAPrivateKey(&fp, d_key.get(), nullptr, nullptr, 0, nullptr, nullptr); + auto ret = PEM_write_RSAPrivateKey(&outputFile, d_key.get(), nullptr, nullptr, 0, nullptr, nullptr); if (ret == 0) { throw runtime_error(getName() + ": Could not convert private key to PEM"); } @@ -306,17 +307,24 @@ void OpenSSLRSADNSCryptoKeyEngine::convertToPEM(std::FILE& fp) const DNSCryptoKeyEngine::storvector_t OpenSSLRSADNSCryptoKeyEngine::convertToISCVector() const { storvector_t storvect; - typedef vector> outputs_t; + using outputs_t = vector>; outputs_t outputs; - const BIGNUM *n, *e, *d, *p, *q, *dmp1, *dmq1, *iqmp; - RSA_get0_key(d_key.get(), &n, &e, &d); - RSA_get0_factors(d_key.get(), &p, &q); + const BIGNUM* modulus = nullptr; + const BIGNUM* publicExponent = nullptr; + const BIGNUM* privateExponent = nullptr; + const BIGNUM* prime1 = nullptr; + const BIGNUM* prime2 = nullptr; + const BIGNUM* dmp1 = nullptr; + const BIGNUM* dmq1 = nullptr; + const BIGNUM* iqmp = nullptr; + RSA_get0_key(d_key.get(), &modulus, &publicExponent, &privateExponent); + RSA_get0_factors(d_key.get(), &prime1, &prime2); RSA_get0_crt_params(d_key.get(), &dmp1, &dmq1, &iqmp); - outputs.emplace_back("Modulus", n); - outputs.emplace_back("PublicExponent", e); - outputs.emplace_back("PrivateExponent", d); - outputs.emplace_back("Prime1", p); - outputs.emplace_back("Prime2", q); + outputs.emplace_back("Modulus", modulus); + outputs.emplace_back("PublicExponent", publicExponent); + outputs.emplace_back("PrivateExponent", privateExponent); + outputs.emplace_back("Prime1", prime1); + outputs.emplace_back("Prime2", prime2); outputs.emplace_back("Exponent1", dmp1); outputs.emplace_back("Exponent2", dmq1); outputs.emplace_back("Coefficient", iqmp); @@ -351,22 +359,30 @@ DNSCryptoKeyEngine::storvector_t OpenSSLRSADNSCryptoKeyEngine::convertToISCVecto return storvect; } -std::string OpenSSLRSADNSCryptoKeyEngine::hash(const std::string& orig) const +std::string OpenSSLRSADNSCryptoKeyEngine::hash(const std::string& message) const { if (d_algorithm == DNSSECKeeper::RSASHA1 || d_algorithm == DNSSECKeeper::RSASHA1NSEC3SHA1) { - unsigned char l_hash[SHA_DIGEST_LENGTH]; - SHA1((unsigned char*)orig.c_str(), orig.length(), l_hash); - return string((char*)l_hash, sizeof(l_hash)); + std::string l_hash{}; + l_hash.resize(SHA_DIGEST_LENGTH); + // NOLINTNEXTLINE(*-cast): Using OpenSSL C APIs. + SHA1(reinterpret_cast(const_cast(message.c_str())), message.length(), reinterpret_cast(l_hash.data())); + return l_hash; } - else if (d_algorithm == DNSSECKeeper::RSASHA256) { - unsigned char l_hash[SHA256_DIGEST_LENGTH]; - SHA256((unsigned char*)orig.c_str(), orig.length(), l_hash); - return string((char*)l_hash, sizeof(l_hash)); + + if (d_algorithm == DNSSECKeeper::RSASHA256) { + std::string l_hash{}; + l_hash.resize(SHA256_DIGEST_LENGTH); + // NOLINTNEXTLINE(*-cast): Using OpenSSL C APIs. + SHA256(reinterpret_cast(const_cast(message.c_str())), message.length(), reinterpret_cast(l_hash.data())); + return l_hash; } - else if (d_algorithm == DNSSECKeeper::RSASHA512) { - unsigned char l_hash[SHA512_DIGEST_LENGTH]; - SHA512((unsigned char*)orig.c_str(), orig.length(), l_hash); - return string((char*)l_hash, sizeof(l_hash)); + + if (d_algorithm == DNSSECKeeper::RSASHA512) { + std::string l_hash{}; + l_hash.resize(SHA512_DIGEST_LENGTH); + // NOLINTNEXTLINE(*-cast): Using OpenSSL C APIs. + SHA512(reinterpret_cast(const_cast(message.c_str())), message.length(), reinterpret_cast(l_hash.data())); + return l_hash; } throw runtime_error(getName() + " does not support hash operation for algorithm " + std::to_string(d_algorithm)); @@ -388,9 +404,9 @@ int OpenSSLRSADNSCryptoKeyEngine::hashSizeToKind(const size_t hashSize) } } -std::string OpenSSLRSADNSCryptoKeyEngine::sign(const std::string& msg) const +std::string OpenSSLRSADNSCryptoKeyEngine::sign(const std::string& message) const { - string l_hash = this->hash(msg); + string l_hash = this->hash(message); int hashKind = hashSizeToKind(l_hash.size()); std::string signature; signature.resize(RSA_size(d_key.get())); @@ -406,9 +422,9 @@ std::string OpenSSLRSADNSCryptoKeyEngine::sign(const std::string& msg) const } -bool OpenSSLRSADNSCryptoKeyEngine::verify(const std::string& msg, const std::string& signature) const +bool OpenSSLRSADNSCryptoKeyEngine::verify(const std::string& message, const std::string& signature) const { - string l_hash = this->hash(msg); + string l_hash = this->hash(message); int hashKind = hashSizeToKind(l_hash.size()); int ret = RSA_verify(hashKind, (const unsigned char*)l_hash.c_str(), l_hash.length(), (unsigned char*)signature.c_str(), signature.length(), d_key.get()); @@ -418,13 +434,15 @@ bool OpenSSLRSADNSCryptoKeyEngine::verify(const std::string& msg, const std::str std::string OpenSSLRSADNSCryptoKeyEngine::getPublicKeyString() const { - const BIGNUM *n, *e, *d; - RSA_get0_key(d_key.get(), &n, &e, &d); + const BIGNUM* modulus = nullptr; + const BIGNUM* publicExponent = nullptr; + const BIGNUM* privateExponent = nullptr; + RSA_get0_key(d_key.get(), &modulus, &publicExponent, &privateExponent); string keystring; std::string tmp; - tmp.resize(std::max(BN_num_bytes(e), BN_num_bytes(n))); + tmp.resize(std::max(BN_num_bytes(publicExponent), BN_num_bytes(modulus))); - int len = BN_bn2bin(e, reinterpret_cast(&tmp.at(0))); + int len = BN_bn2bin(publicExponent, reinterpret_cast(&tmp.at(0))); if (len < 255) { keystring.assign(1, (char)(unsigned int)len); } @@ -436,36 +454,36 @@ std::string OpenSSLRSADNSCryptoKeyEngine::getPublicKeyString() const } keystring.append(&tmp.at(0), len); - len = BN_bn2bin(n, reinterpret_cast(&tmp.at(0))); + len = BN_bn2bin(modulus, reinterpret_cast(&tmp.at(0))); keystring.append(&tmp.at(0), len); return keystring; } -std::unique_ptrOpenSSLRSADNSCryptoKeyEngine::parse(std::map& stormap, const std::string& key) const +std::unique_ptrOpenSSLRSADNSCryptoKeyEngine::parse(std::map& stormap, const std::string& key) const { - const std::string& v = stormap.at(key); - auto n = std::unique_ptr(BN_bin2bn(reinterpret_cast(v.data()), v.length(), nullptr), BN_clear_free); + const std::string& value = stormap.at(key); + auto number = std::unique_ptr(BN_bin2bn(reinterpret_cast(value.data()), static_cast(value.length()), nullptr), BN_clear_free); - if (!n) { + if (!number) { throw runtime_error(getName() + " parsing of " + key + " failed"); } - return n; + return number; } void OpenSSLRSADNSCryptoKeyEngine::fromISCMap(DNSKEYRecordContent& drc, std::map& stormap) { - auto key = std::unique_ptr(RSA_new(), RSA_free); + auto key = std::unique_ptr(RSA_new(), RSA_free); if (!key) { throw runtime_error(getName() + " allocation of key structure failed"); } - auto n = parse(stormap, "modulus"); - auto e = parse(stormap, "publicexponent"); - auto d = parse(stormap, "privateexponent"); + auto modulus = parse(stormap, "modulus"); + auto publicExponent = parse(stormap, "publicexponent"); + auto privateExponent = parse(stormap, "privateexponent"); - auto p = parse(stormap, "prime1"); - auto q = parse(stormap, "prime2"); + auto prime1 = parse(stormap, "prime1"); + auto prime2 = parse(stormap, "prime2"); auto dmp1 = parse(stormap, "exponent1"); auto dmq1 = parse(stormap, "exponent2"); @@ -477,8 +495,8 @@ void OpenSSLRSADNSCryptoKeyEngine::fromISCMap(DNSKEYRecordContent& drc, std::map throw runtime_error(getName() + " tried to feed an algorithm " + std::to_string(drc.d_algorithm) + " to a " + std::to_string(d_algorithm) + " key"); } // Everything OK, we're releasing ownership since the RSA_* functions want it - RSA_set0_key(key.get(), n.release(), e.release(), d.release()); - RSA_set0_factors(key.get(), p.release(), q.release()); + RSA_set0_key(key.get(), modulus.release(), publicExponent.release(), privateExponent.release()); + RSA_set0_factors(key.get(), prime1.release(), prime2.release()); RSA_set0_crt_params(key.get(), dmp1.release(), dmq1.release(), iqmp.release()); d_key = std::move(key); @@ -503,7 +521,7 @@ bool OpenSSLRSADNSCryptoKeyEngine::checkKey(vector* errorMessages) const if (RSA_check_key(d_key.get()) != 1) { retval = false; if (errorMessages != nullptr) { - auto errmsg = ERR_reason_error_string(ERR_get_error()); + const auto* errmsg = ERR_reason_error_string(ERR_get_error()); if (errmsg == nullptr) { errmsg = "Unknown OpenSSL error"; } @@ -513,47 +531,48 @@ bool OpenSSLRSADNSCryptoKeyEngine::checkKey(vector* errorMessages) const return retval; } -void OpenSSLRSADNSCryptoKeyEngine::fromPublicKeyString(const std::string& input) +void OpenSSLRSADNSCryptoKeyEngine::fromPublicKeyString(const std::string& content) { - string exponent, modulus; - const size_t inputLen = input.length(); - const unsigned char* raw = (const unsigned char*)input.c_str(); + string exponent; + string modulus; + const size_t contentLen = content.length(); + const auto* raw = (const unsigned char*)content.c_str(); - if (inputLen < 1) { + if (contentLen < 1) { throw runtime_error(getName() + " invalid input size for the public key"); } if (raw[0] != 0) { const size_t exponentSize = raw[0]; - if (inputLen < (exponentSize + 2)) { + if (contentLen < (exponentSize + 2)) { throw runtime_error(getName() + " invalid input size for the public key"); } - exponent = input.substr(1, exponentSize); - modulus = input.substr(exponentSize + 1); + exponent = content.substr(1, exponentSize); + modulus = content.substr(exponentSize + 1); } else { - if (inputLen < 3) { + if (contentLen < 3) { throw runtime_error(getName() + " invalid input size for the public key"); } const size_t exponentSize = raw[1] * 0xff + raw[2]; - if (inputLen < (exponentSize + 4)) { + if (contentLen < (exponentSize + 4)) { throw runtime_error(getName() + " invalid input size for the public key"); } - exponent = input.substr(3, exponentSize); - modulus = input.substr(exponentSize + 3); + exponent = content.substr(3, exponentSize); + modulus = content.substr(exponentSize + 3); } - auto key = std::unique_ptr(RSA_new(), RSA_free); + auto key = std::unique_ptr(RSA_new(), RSA_free); if (!key) { throw runtime_error(getName() + " allocation of key structure failed"); } - auto e = std::unique_ptr(BN_bin2bn((unsigned char*)exponent.c_str(), exponent.length(), nullptr), BN_clear_free); + auto e = std::unique_ptr(BN_bin2bn((unsigned char*)exponent.c_str(), exponent.length(), nullptr), BN_clear_free); if (!e) { throw runtime_error(getName() + " error loading public exponent (e) value of public key"); } - auto n = std::unique_ptr(BN_bin2bn((unsigned char*)modulus.c_str(), modulus.length(), nullptr), BN_clear_free); + auto n = std::unique_ptr(BN_bin2bn((unsigned char*)modulus.c_str(), modulus.length(), nullptr), BN_clear_free); if (!n) { throw runtime_error(getName() + " error loading modulus (n) value of public key"); }