From: Remi Gacogne Date: Mon, 2 May 2022 09:46:38 +0000 (+0200) Subject: auth: Speed up ECDSA and RSA signatures X-Git-Tag: dnsdist-1.8.0-rc1~98^2~4 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=ab5df14454e7830e446f2ed2ea15fc1750b0d8f3;p=thirdparty%2Fpdns.git auth: Speed up ECDSA and RSA signatures For ECDSA, and likely for RSA, computing the public key is not a cheap operation. So instead of computing it twice to get the lookup key for our signatures cache, reuse the computed public key and only compute its digest. In addition, since ed* algorithms were already using the whole key instead of a digest, place the cut off at public keys larger than 64 bytes, meaning that only RSA ones (128+ bytes) will be hashed. This provides an additional speedup for ECDSA keys (32 or 48 bytes) since they no longer need to be hashed, and simplifies the signers code as the hashing can be moved to the key cache now that it only depends on they key size. For reference the size of a SHA-1 digest is 20 bytes. In my tests this reduces by 30% the cost of calling addRRSigs() for ECDSA signatures when the signature is already present in the cache. --- diff --git a/pdns/decafsigners.cc b/pdns/decafsigners.cc index 5eae78292f..fb57ccd9a5 100644 --- a/pdns/decafsigners.cc +++ b/pdns/decafsigners.cc @@ -56,7 +56,6 @@ public: #endif storvector_t convertToISCVector() const override; - std::string getPubKeyHash() const override; std::string sign(const std::string& msg) const override; bool verify(const std::string& msg, const std::string& signature) const override; std::string getPublicKeyString() const override; @@ -167,11 +166,6 @@ void DecafED25519DNSCryptoKeyEngine::fromISCMap(DNSKEYRecordContent& drc, std::m pub.serialize_into(d_pubkey); } -std::string DecafED25519DNSCryptoKeyEngine::getPubKeyHash() const -{ - return this->getPublicKeyString(); -} - std::string DecafED25519DNSCryptoKeyEngine::getPublicKeyString() const { return string((char*)d_pubkey, DECAF_EDDSA_25519_PUBLIC_BYTES); @@ -260,7 +254,6 @@ public: #endif storvector_t convertToISCVector() const override; - std::string getPubKeyHash() const override; std::string sign(const std::string& msg) const override; bool verify(const std::string& msg, const std::string& signature) const override; std::string getPublicKeyString() const override; @@ -371,11 +364,6 @@ void DecafED448DNSCryptoKeyEngine::fromISCMap(DNSKEYRecordContent& drc, std::map pub.serialize_into(d_pubkey); } -std::string DecafED448DNSCryptoKeyEngine::getPubKeyHash() const -{ - return this->getPublicKeyString(); -} - std::string DecafED448DNSCryptoKeyEngine::getPublicKeyString() const { return string((char*)d_pubkey, DECAF_EDDSA_448_PUBLIC_BYTES); diff --git a/pdns/dnssecinfra.hh b/pdns/dnssecinfra.hh index 95ccff38f2..246ea51d29 100644 --- a/pdns/dnssecinfra.hh +++ b/pdns/dnssecinfra.hh @@ -65,7 +65,6 @@ class DNSCryptoKeyEngine [[nodiscard]] virtual bool verify(const std::string& msg, const std::string& signature) const =0; - [[nodiscard]] virtual std::string getPubKeyHash()const =0; [[nodiscard]] virtual std::string getPublicKeyString()const =0; [[nodiscard]] virtual int getBits() const =0; [[nodiscard]] virtual unsigned int getAlgorithm() const diff --git a/pdns/dnssecsigner.cc b/pdns/dnssecsigner.cc index bc7f992239..7734a5901f 100644 --- a/pdns/dnssecsigner.cc +++ b/pdns/dnssecsigner.cc @@ -50,6 +50,17 @@ static std::string getLookupKey(const std::string& msg) } } +static std::string hashPublicKey(const std::string& pubKey) +{ + /* arbitrarily cut off at 64 bytes, the main idea is to save space + for very large keys like RSA ones (1024+ bytes) by storing a 20 bytes hash + instead */ + if (pubKey.size() <= 64) { + return pubKey; + } + return pdns_sha1sum(pubKey); +} + static void fillOutRRSIG(DNSSECPrivateKey& dpk, const DNSName& signQName, RRSIGRecordContent& rrc, const sortedRecords_t& toSign) { if(!g_signatureCount) @@ -60,8 +71,8 @@ static void fillOutRRSIG(DNSSECPrivateKey& dpk, const DNSName& signQName, RRSIGR rrc.d_tag = drc.getTag(); rrc.d_algorithm = drc.d_algorithm; - string msg=getMessageForRRSET(signQName, rrc, toSign); // this is what we will hash & sign - pair lookup(rc->getPubKeyHash(), getLookupKey(msg)); // this hash is a memory saving exercise + string msg = getMessageForRRSET(signQName, rrc, toSign); // this is what we will hash & sign + pair lookup(hashPublicKey(drc.d_key), getLookupKey(msg)); // this hash is a memory saving exercise bool doCache=true; if(doCache) diff --git a/pdns/opensslsigners.cc b/pdns/opensslsigners.cc index d91b9fd5f2..29a1ffca70 100644 --- a/pdns/opensslsigners.cc +++ b/pdns/opensslsigners.cc @@ -222,7 +222,6 @@ public: 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 getPubKeyHash() const override; 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; @@ -410,43 +409,6 @@ bool OpenSSLRSADNSCryptoKeyEngine::verify(const std::string& msg, const std::str return (ret == 1); } - -std::string OpenSSLRSADNSCryptoKeyEngine::getPubKeyHash() const -{ - const BIGNUM *n, *e, *d; - RSA_get0_key(d_key.get(), &n, &e, &d); - std::vector tmp; - tmp.resize(std::max(BN_num_bytes(e), BN_num_bytes(n))); - unsigned char l_hash[SHA_DIGEST_LENGTH]; - SHA_CTX ctx; - - int res = SHA1_Init(&ctx); - - if (res != 1) { - throw runtime_error(getName()+" failed to init hash context for generating the public key hash"); - } - - int len = BN_bn2bin(e, tmp.data()); - res = SHA1_Update(&ctx, tmp.data(), len); - if (res != 1) { - throw runtime_error(getName()+" failed to update hash context for generating the public key hash"); - } - - len = BN_bn2bin(n, tmp.data()); - res = SHA1_Update(&ctx, tmp.data(), len); - if (res != 1) { - throw runtime_error(getName()+" failed to update hash context for generating the public key hash"); - } - - res = SHA1_Final(l_hash, &ctx); - if (res != 1) { - throw runtime_error(getName()+" failed to finish hash context for generating the public key hash"); - } - - return string((char*)l_hash, sizeof(l_hash)); -} - - std::string OpenSSLRSADNSCryptoKeyEngine::getPublicKeyString() const { const BIGNUM *n, *e, *d; @@ -673,7 +635,6 @@ public: 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 getPubKeyHash() const override; std::string getPublicKeyString() const override; void fromISCMap(DNSKEYRecordContent& drc, std::map& stormap) override; void fromPublicKeyString(const std::string& content) override; @@ -855,14 +816,6 @@ bool OpenSSLECDSADNSCryptoKeyEngine::verify(const std::string& msg, const std::s return (ret == 1); } -std::string OpenSSLECDSADNSCryptoKeyEngine::getPubKeyHash() const -{ - string pubKey = getPublicKeyString(); - unsigned char l_hash[SHA_DIGEST_LENGTH]; - SHA1((unsigned char*) pubKey.c_str(), pubKey.length(), l_hash); - return string((char*) l_hash, sizeof(l_hash)); -} - std::string OpenSSLECDSADNSCryptoKeyEngine::getPublicKeyString() const { std::string binaryPoint; @@ -1036,7 +989,6 @@ public: storvector_t convertToISCVector() const override; std::string sign(const std::string& msg) const override; bool verify(const std::string& msg, const std::string& signature) const override; - std::string getPubKeyHash() const override; std::string getPublicKeyString() const override; void fromISCMap(DNSKEYRecordContent& drc, std::map& stormap) override; void fromPublicKeyString(const std::string& content) override; @@ -1171,11 +1123,6 @@ bool OpenSSLEDDSADNSCryptoKeyEngine::verify(const std::string& msg, const std::s return (r == 1); } -std::string OpenSSLEDDSADNSCryptoKeyEngine::getPubKeyHash() const -{ - return this->getPublicKeyString(); -} - std::string OpenSSLEDDSADNSCryptoKeyEngine::getPublicKeyString() const { string buf; diff --git a/pdns/pkcs11signers.cc b/pdns/pkcs11signers.cc index 2ea2f68944..b744e40c4e 100644 --- a/pdns/pkcs11signers.cc +++ b/pdns/pkcs11signers.cc @@ -493,25 +493,6 @@ class Pkcs11Token { return d_err; } - int DigestKey(std::string& result) { - auto slot = d_slot->lock(); - CK_MECHANISM mech; - mech.mechanism = CKM_SHA_1; - - DigestInit(*slot, &mech); - - if (d_key_type == CKK_RSA) { - DigestUpdate(*slot, d_modulus); - DigestUpdate(*slot, d_exponent); - } else if (d_key_type == CKK_EC || d_key_type == CKK_ECDSA) { - DigestUpdate(*slot, d_ec_point); - } - - DigestFinal(*slot, result); - - return d_err; - } - int DigestFinal(Pkcs11Slot& slot, std::string& result) { CK_BYTE buffer[1024] = {0}; CK_ULONG buflen = sizeof buffer; // should be enough for most digests @@ -924,19 +905,6 @@ bool PKCS11DNSCryptoKeyEngine::verify(const std::string& msg, const std::string& } }; -std::string PKCS11DNSCryptoKeyEngine::getPubKeyHash() const { - // find us a public key - std::shared_ptr d_slot; - d_slot = Pkcs11Token::GetToken(d_module, d_slot_id, d_label, d_pub_label); - if (d_slot->LoggedIn() == false) - if (d_slot->Login(d_pin) == false) - throw PDNSException("Not logged in to token"); - - std::string result; - if (d_slot->DigestKey(result) == 0) return result; - throw PDNSException("Could not digest key (maybe it's missing?)"); -}; - std::string PKCS11DNSCryptoKeyEngine::getPublicKeyString() const { std::string result(""); std::shared_ptr d_slot; diff --git a/pdns/pkcs11signers.hh b/pdns/pkcs11signers.hh index 831493deb2..09dc9534d4 100644 --- a/pdns/pkcs11signers.hh +++ b/pdns/pkcs11signers.hh @@ -52,8 +52,6 @@ class PKCS11DNSCryptoKeyEngine : public DNSCryptoKeyEngine bool verify(const std::string& msg, const std::string& signature) const override; - std::string getPubKeyHash() const override; - std::string getPublicKeyString() const override; int getBits() const override; diff --git a/pdns/sodiumsigners.cc b/pdns/sodiumsigners.cc index 380dbb09b3..4d7cf8df87 100644 --- a/pdns/sodiumsigners.cc +++ b/pdns/sodiumsigners.cc @@ -51,7 +51,6 @@ public: #endif storvector_t convertToISCVector() const override; - std::string getPubKeyHash() const override; std::string sign(const std::string& msg) const override; bool verify(const std::string& msg, const std::string& signature) const override; std::string getPublicKeyString() const override; @@ -161,11 +160,6 @@ void SodiumED25519DNSCryptoKeyEngine::fromISCMap(DNSKEYRecordContent& drc, std:: crypto_sign_ed25519_seed_keypair(d_pubkey, d_seckey, seed.get()); } -std::string SodiumED25519DNSCryptoKeyEngine::getPubKeyHash() const -{ - return this->getPublicKeyString(); -} - std::string SodiumED25519DNSCryptoKeyEngine::getPublicKeyString() const { return string((char*)d_pubkey, crypto_sign_ed25519_PUBLICKEYBYTES); diff --git a/pdns/test-signers.cc b/pdns/test-signers.cc index 21e19700bf..f8e544980b 100644 --- a/pdns/test-signers.cc +++ b/pdns/test-signers.cc @@ -29,7 +29,6 @@ struct SignerParams std::string name; std::string rfcMsgDump; std::string rfcB64Signature; - std::string pubKeyHash; int bits; uint16_t flags; uint16_t rfcFlags; @@ -78,7 +77,6 @@ static const SignerParams rsaSha256SignerParams = SignerParams{ .rfcMsgDump = "", .rfcB64Signature = "", - .pubKeyHash = "QH+uURzTHkYZ5MrwNvOrn+BtnL4=", .bits = 512, .flags = 256, @@ -133,7 +131,6 @@ static const SignerParams ecdsaSha256 = SignerParams{ .rfcMsgDump = "", .rfcB64Signature = "", - .pubKeyHash = "aIQTEsTXwMDIOXPY9e6W1G1AnAk=", .bits = 256, .flags = 256, @@ -191,7 +188,6 @@ static const SignerParams ed25519 = SignerParams{ // https://www.rfc-editor.org/errata_search.php?rfc=8080&eid=4935 .rfcB64Signature = "oL9krJun7xfBOIWcGHi7mag5/hdZrKWw15jPGrHpjQeR" "AvTdszaPD+QLs3fx8A4M3e23mRZ9VrbpMngwcrqNAg==", - .pubKeyHash = "l02Woi0iS8Aa25FQkUd9RMzZHJpBoRQwAQEX1SxZJA4=", .bits = 256, .flags = 256, @@ -253,8 +249,6 @@ static const SignerParams ed448 = SignerParams{ .rfcB64Signature = "3cPAHkmlnxcDHMyg7vFC34l0blBhuG1qpwLmjInI8w1CMB29FkEA" "IJUA0amxWndkmnBZ6SKiwZSAxGILn/NBtOXft0+Gj7FSvOKxE/07" "+4RQvE581N3Aj/JtIyaiYVdnYtyMWbSNyGEY2213WKsJlwEA", - .pubKeyHash = "3kgROaDjrh0H2iuixWBrc8g2EpBBLCdGzHmn+G2MpTPhpj/" - "OiBVHHSfPodx1FYYUcJKm1MDpJtIA", .bits = 456, .flags = 256, @@ -425,8 +419,6 @@ static void test_generic_signer(std::shared_ptr dcke, DNSKEY if (!signer.rfcMsgDump.empty() && !signer.rfcB64Signature.empty()) { checkRR(signer); } - - BOOST_CHECK_EQUAL(Base64Encode(dcke->getPubKeyHash()), signer.pubKeyHash); } // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables,readability-identifier-length): Boost stuff.