From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Thu, 6 May 2021 10:12:43 +0000 (+0200)
Subject: auth: Don't choke on non-base64 values when importing zone keys
X-Git-Tag: dnsdist-1.7.0-alpha0~15^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3b9e71ab6d80a1a5e6e5bb67a448a58284b68751;p=thirdparty%2Fpdns.git

auth: Don't choke on non-base64 values when importing zone keys

DNSCryptoKeyEngine::makeFromISCFile(), called by `pdnsutil import-zone-key`
or the API, for example, would try to parse almost all values as a
base64 string. Depending on the version of OpenSSL, it could have lead
to a weird `Error: BIO_read failed to read all data from memory buffer`
error when the file contains a non-base64 value, like for example:
```
Flags: 257
```
Recent versions of OpenSSL seems to simply return that the value could
not be parsed, but older ones (OpenSSL 1.0.2k from CentOS 7 for example)
would report an incomplete read (BIO_should_retry() returning 1),
triggering an exception that prevents the key from being loaded.
This commits keeps a longer list of known non-base64 values, but
more importantly catch the base64 decoding exception and then store the
initial value instead of aborting. Only failure to decode known base64
values prevents the key from being loaded.
---

diff --git a/pdns/dnssecinfra.cc b/pdns/dnssecinfra.cc
index 97d88499e3..6cbe1288b8 100644
--- a/pdns/dnssecinfra.cc
+++ b/pdns/dnssecinfra.cc
@@ -76,50 +76,87 @@ std::unique_ptr<DNSCryptoKeyEngine> DNSCryptoKeyEngine::makeFromISCFile(DNSKEYRe
 
 std::unique_ptr<DNSCryptoKeyEngine> DNSCryptoKeyEngine::makeFromISCString(DNSKEYRecordContent& drc, const std::string& content)
 {
-  bool pkcs11=false;
-  int algorithm = 0;
+  enum class KeyTypes : uint8_t { str, numeric, base64 };
+  const std::map<std::string, KeyTypes> knownKeys = {
+    { "algorithm", KeyTypes::numeric },
+    { "modulus", KeyTypes::base64 },
+    { "publicexponent", KeyTypes::base64 },
+    { "privateexponent", KeyTypes::base64 },
+    { "prime1", KeyTypes::base64 },
+    { "prime2", KeyTypes::base64 },
+    { "exponent1", KeyTypes::base64 },
+    { "exponent2", KeyTypes::base64 },
+    { "coefficient", KeyTypes::base64 },
+    { "privatekey", KeyTypes::base64 },
+    { "engine", KeyTypes::str },
+    { "slot", KeyTypes::str },
+    { "pin", KeyTypes::str },
+    { "label", KeyTypes::str },
+    { "publabel", KeyTypes::str },
+    { "private-key-format", KeyTypes::str },
+    { "flags", KeyTypes::numeric }
+  };
+  unsigned int algorithm = 0;
   string sline, key, value, raw;
   std::istringstream str(content);
   map<string, string> stormap;
 
-  while(std::getline(str, sline)) {
-    tie(key,value)=splitField(sline, ':');
+  while (std::getline(str, sline)) {
+    tie(key,value) = splitField(sline, ':');
     boost::trim(value);
-    if(pdns_iequals(key,"algorithm")) {
-      algorithm = pdns_stou(value);
-      stormap["algorithm"]=std::to_string(algorithm);
-      continue;
-    } else if (pdns_iequals(key,"pin")) {
-      stormap["pin"]=value;
-      continue;
-    } else if (pdns_iequals(key,"engine")) {
-      stormap["engine"]=value;
-      pkcs11=true;
-      continue;
-    } else if (pdns_iequals(key,"slot")) {
-      stormap["slot"]=value;
-      continue;
-    }  else if (pdns_iequals(key,"label")) {
-      stormap["label"]=value;
-      continue;
-    } else if (pdns_iequals(key,"publabel")) {
-      stormap["publabel"]=value;
-      continue;
+
+    toLowerInPlace(key);
+    const auto it = knownKeys.find(key);
+    if (it != knownKeys.cend()) {
+      if (it->second == KeyTypes::str) {
+        stormap[key] = value;
+      }
+      else if (it->second == KeyTypes::base64) {
+        try {
+          raw.clear();
+          B64Decode(value, raw);
+          stormap[key] = raw;
+        }
+        catch (const std::exception& e) {
+          throw std::runtime_error("Error while trying to base64 decode the value of the '" + key + "' key from the ISC map: " + e.what());
+        }
+      }
+      else if (it->second == KeyTypes::numeric) {
+        try {
+          unsigned int num = pdns_stou(value);
+          stormap[key] = std::to_string(num);
+          if (key == "algorithm") {
+            algorithm = num;
+          }
+        }
+        catch (const std::exception& e) {
+          throw std::runtime_error("Error while trying to pase the value of the '" + key + "' key from the ISC map: " + e.what());
+        }
+      }
+    }
+    else {
+      try {
+        raw.clear();
+        B64Decode(value, raw);
+        stormap[key] = raw;
+      }
+      catch (const std::exception& e) {
+        stormap[key] = value;
+      }
     }
-    else if(pdns_iequals(key, "Private-key-format"))
-      continue;
-    raw.clear();
-    B64Decode(value, raw);
-    stormap[toLower(key)]=raw;
   }
+
   std::unique_ptr<DNSCryptoKeyEngine> dpk;
 
-  if (pkcs11) {
+  if (stormap.count("engine")) {
 #ifdef HAVE_P11KIT1
-    if (stormap.find("slot") == stormap.end())
+    if (stormap.count("slot") == 0) {
       throw PDNSException("Cannot load PKCS#11 key, no Slot specified");
+    }
     // we need PIN to be at least empty
-    if (stormap.find("pin") == stormap.end()) stormap["pin"] = "";
+    if (stormap.count("pin") == 0) {
+      stormap["pin"] = "";
+    }
     dpk = PKCS11DNSCryptoKeyEngine::maker(algorithm); 
 #else
     throw PDNSException("Cannot load PKCS#11 key without support for it");