From 3b9e71ab6d80a1a5e6e5bb67a448a58284b68751 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Thu, 6 May 2021 12:12:43 +0200 Subject: [PATCH] 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. --- pdns/dnssecinfra.cc | 101 ++++++++++++++++++++++++++++++-------------- 1 file changed, 69 insertions(+), 32 deletions(-) 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::makeFromISCFile(DNSKEYRe std::unique_ptr 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 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 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 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"); -- 2.47.2