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 5eae78292f57d97d7738e4fb1ca126b8a8ac049c..fb57ccd9a59243ca7a721482def34a0177304dbb 100644 (file)
--- 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 95ccff38f2ce060c0b3652c38c1c90cb2eb06e3f..246ea51d29f60524a9b525c92710269fff5b2a1c 100644 (file)
--- 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 bc7f992239e44b0181972fa8a1c71e87cbcdb679..7734a5901f6b84d94da1eeb92d4764872bf31370 100644 (file)
--- 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<string, string> 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<string, string> 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 d91b9fd5f25468a7aa6cd6d1a3ed85fc6342c49f..29a1ffca709e08e717ee3a51f201bcaa7e5463d5 100644 (file)
--- 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<BIGNUM, void (*)(BIGNUM*)> parse(std::map<std::string, std::string>& stormap, const std::string& key) const;
   void fromISCMap(DNSKEYRecordContent& drc, std::map<std::string, std::string>& 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<unsigned char> 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<std::string, std::string>& 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<std::string, std::string>& 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 2ea2f68944206889d487acf7118f18220ab7046e..b744e40c4eaa4588b41053b9dae064ee9702dbd9 100644 (file)
--- 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<Pkcs11Token> 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<Pkcs11Token> d_slot;

diff --git a/pdns/pkcs11signers.hh b/pdns/pkcs11signers.hh
index 831493deb29e975f808bf2a78af1954a6d618417..09dc9534d43d07d5aec895a64b3cb6dad0f128df 100644 (file)
--- 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 380dbb09b3c98f7160a5e9eec4f9fada40fd9491..4d7cf8df8735dc7d1bfd302f94ae0acdb78b0c33 100644 (file)
--- 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 21e19700bf0b81b34e9f035ce8db1466bdda5c49..f8e544980b03483edd67ea8a89d210dc3318e669 100644 (file)
--- 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<DNSCryptoKeyEngine> 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.