From 8089cbe5195b0f48801890ff354485a630a792f5 Mon Sep 17 00:00:00 2001
From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Fri, 23 Dec 2016 13:05:22 +0100
Subject: [PATCH] DNSCrypt: Only lock the shared key memory if we use it

---
 pdns/dnscrypt.cc | 56 +++++++++++++++++++++++++++---------------------
 pdns/dnscrypt.hh | 40 ++++++++++++++++++----------------
 2 files changed, 53 insertions(+), 43 deletions(-)

diff --git a/pdns/dnscrypt.cc b/pdns/dnscrypt.cc
index 5014e452a2..98ed0ef642 100644
--- a/pdns/dnscrypt.cc
+++ b/pdns/dnscrypt.cc
@@ -59,16 +59,36 @@ DnsCryptPrivateKey::~DnsCryptPrivateKey()
   sodium_munlock(key, sizeof(key));
 }
 
-DnsCryptQuery::DnsCryptQuery()
+DnsCryptQuery::~DnsCryptQuery()
 {
-  sodium_mlock(sharedKey, sizeof(sharedKey));
+  if (sharedKeyComputed) {
+    sodium_munlock(sharedKey, sizeof(sharedKey));
+  }
 }
 
-DnsCryptQuery::~DnsCryptQuery()
+int DnsCryptQuery::computeSharedKey(const DnsCryptPrivateKey& privateKey)
 {
-  sodium_munlock(sharedKey, sizeof(sharedKey));
+  int res = 0;
+
+  if (sharedKeyComputed) {
+    return res;
+  }
+
+  sodium_mlock(sharedKey, sizeof(sharedKey));
+  res = crypto_box_beforenm(sharedKey,
+                            header.clientPK,
+                            privateKey.key);
+
+  if (res != 0) {
+    sodium_munlock(sharedKey, sizeof(sharedKey));
+    return res;
+  }
+
+  sharedKeyComputed = true;
+  return res;
 }
 
+
 void DnsCryptContext::generateProviderKeys(unsigned char publicKey[DNSCRYPT_PROVIDER_PUBLIC_KEY_SIZE], unsigned char privateKey[DNSCRYPT_PROVIDER_PRIVATE_KEY_SIZE])
 {
   int res = crypto_sign_ed25519_keypair(publicKey, privateKey);
@@ -301,17 +321,10 @@ void DnsCryptContext::getDecryptedQuery(std::shared_ptr<DnsCryptQuery> query, bo
   memcpy(nonce, &query->header.clientNonce, sizeof(query->header.clientNonce));
   memset(nonce + sizeof(query->header.clientNonce), 0, sizeof(nonce) - sizeof(query->header.clientNonce));
 
-  int res = 0;
-  if (!query->sharedKeyComputed) {
-    res = crypto_box_beforenm(query->sharedKey,
-                              query->header.clientPK,
-                              query->useOldCert ? oldPrivateKey.key : privateKey.key);
-
-    if (res != 0) {
-      vinfolog("Dropping encrypted query we can't compute the shared key for");
-      return;
-    }
-    query->sharedKeyComputed = true;
+  int res = query->computeSharedKey(query->useOldCert ? oldPrivateKey : privateKey);
+  if (res != 0) {
+    vinfolog("Dropping encrypted query we can't compute the shared key for");
+    return;
   }
 
   res = crypto_box_open_easy_afternm((unsigned char*) packet,
@@ -469,16 +482,9 @@ int DnsCryptContext::encryptResponse(char* response, uint16_t responseLen, uint1
   pos += (paddingSize - 1);
 
   /* encrypting */
-  int res = 0;
-  if (!query->sharedKeyComputed) {
-    res = crypto_box_beforenm(query->sharedKey,
-                              query->header.clientPK,
-                              query->useOldCert ? oldPrivateKey.key : privateKey.key);
-
-    if (res != 0) {
-      return res;
-    }
-    query->sharedKeyComputed = true;
+  int res = query->computeSharedKey(query->useOldCert ? oldPrivateKey : privateKey);
+  if (res != 0) {
+    return res;
   }
 
   res = crypto_box_easy_afternm((unsigned char*) (response + sizeof(header)),
diff --git a/pdns/dnscrypt.hh b/pdns/dnscrypt.hh
index 45371e9e4f..89fc7a40e4 100644
--- a/pdns/dnscrypt.hh
+++ b/pdns/dnscrypt.hh
@@ -88,11 +88,32 @@ struct DnsCryptQueryHeader
 
 static_assert(sizeof(DnsCryptQueryHeader) == 52, "Dnscrypt query header size should be 52!");
 
+struct DnsCryptResponseHeader
+{
+  const unsigned char resolverMagic[DNSCRYPT_RESOLVER_MAGIC_SIZE] = DNSCRYPT_RESOLVER_MAGIC;
+  unsigned char nonce[DNSCRYPT_NONCE_SIZE];
+};
+
+class DnsCryptPrivateKey
+{
+public:
+  DnsCryptPrivateKey();
+  ~DnsCryptPrivateKey();
+  void loadFromFile(const std::string& keyFile);
+  void saveToFile(const std::string& keyFile) const;
+
+  unsigned char key[DNSCRYPT_PRIVATE_KEY_SIZE];
+};
+
 class DnsCryptQuery
 {
 public:
-  DnsCryptQuery();
+  DnsCryptQuery()
+  {
+  }
   ~DnsCryptQuery();
+  int computeSharedKey(const DnsCryptPrivateKey& privateKey);
+
   static const size_t minUDPLength = 256;
 
   DnsCryptQueryHeader header;
@@ -108,23 +129,6 @@ public:
   bool sharedKeyComputed{false};
 };
 
-struct DnsCryptResponseHeader
-{
-  const unsigned char resolverMagic[DNSCRYPT_RESOLVER_MAGIC_SIZE] = DNSCRYPT_RESOLVER_MAGIC;
-  unsigned char nonce[DNSCRYPT_NONCE_SIZE];
-};
-
-class DnsCryptPrivateKey
-{
-public:
-  DnsCryptPrivateKey();
-  ~DnsCryptPrivateKey();
-  void loadFromFile(const std::string& keyFile);
-  void saveToFile(const std::string& keyFile) const;
-
-  unsigned char key[DNSCRYPT_PRIVATE_KEY_SIZE];
-};
-
 class DnsCryptContext
 {
 public:
-- 
2.47.2