From: Remi Gacogne Date: Fri, 17 May 2024 09:59:44 +0000 (+0200) Subject: Delint the DNSCrypt code base X-Git-Tag: rec-5.1.0-beta1~30^2~6 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=dfd74a33cd4a75c560bede2cf562ce93b4d6e40a;p=thirdparty%2Fpdns.git Delint the DNSCrypt code base --- diff --git a/pdns/dnscrypt.cc b/pdns/dnscrypt.cc index 9be46d722f..3e8e0b30ff 100644 --- a/pdns/dnscrypt.cc +++ b/pdns/dnscrypt.cc @@ -25,22 +25,24 @@ #include #include "dolog.hh" #include "dnscrypt.hh" +#include "dnsdist-dnsparser.hh" #include "dnswriter.hh" DNSCryptPrivateKey::DNSCryptPrivateKey() { - sodium_memzero(key, sizeof(key)); - sodium_mlock(key, sizeof(key)); + sodium_memzero(key.data(), key.size()); + sodium_mlock(key.data(), key.size()); } void DNSCryptPrivateKey::loadFromFile(const std::string& keyFile) { ifstream file(keyFile); - sodium_memzero(key, sizeof(key)); - file.read((char*) key, sizeof(key)); + sodium_memzero(key.data(), key.size()); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have + file.read(reinterpret_cast(key.data()), static_cast(key.size())); if (file.fail()) { - sodium_memzero(key, sizeof(key)); + sodium_memzero(key.data(), key.size()); file.close(); throw std::runtime_error("Invalid DNSCrypt key file " + keyFile); } @@ -51,13 +53,14 @@ void DNSCryptPrivateKey::loadFromFile(const std::string& keyFile) void DNSCryptPrivateKey::saveToFile(const std::string& keyFile) const { ofstream file(keyFile); - file.write(reinterpret_cast(key), sizeof(key)); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have + file.write(reinterpret_cast(key.data()), static_cast(key.size())); file.close(); } DNSCryptPrivateKey::~DNSCryptPrivateKey() { - sodium_munlock(key, sizeof(key)); + sodium_munlock(key.data(), key.size()); } DNSCryptExchangeVersion DNSCryptQuery::getVersion() const @@ -73,34 +76,34 @@ DNSCryptExchangeVersion DNSCryptQuery::getVersion() const DNSCryptQuery::~DNSCryptQuery() { if (d_sharedKeyComputed) { - sodium_munlock(d_sharedKey, sizeof(d_sharedKey)); + sodium_munlock(d_sharedKey.data(), d_sharedKey.size()); } } int DNSCryptQuery::computeSharedKey() { - assert(d_pair != nullptr); - int res = 0; - if (d_sharedKeyComputed) { return res; } + if (d_pair == nullptr) { + throw std::runtime_error("Asked to compute a DNSCrypt shared key without the certificate key set"); + } const DNSCryptExchangeVersion version = DNSCryptContext::getExchangeVersion(d_pair->cert); - sodium_mlock(d_sharedKey, sizeof(d_sharedKey)); + sodium_mlock(d_sharedKey.data(), d_sharedKey.size()); if (version == DNSCryptExchangeVersion::VERSION1) { - res = crypto_box_beforenm(d_sharedKey, - d_header.clientPK, - d_pair->privateKey.key); + res = crypto_box_beforenm(d_sharedKey.data(), + d_header.clientPK.data(), + d_pair->privateKey.key.data()); } else if (version == DNSCryptExchangeVersion::VERSION2) { #ifdef HAVE_CRYPTO_BOX_CURVE25519XCHACHA20POLY1305_EASY - res = crypto_box_curve25519xchacha20poly1305_beforenm(d_sharedKey, - d_header.clientPK, - d_pair->privateKey.key); + res = crypto_box_curve25519xchacha20poly1305_beforenm(d_sharedKey.data(), + d_header.clientPK.data(), + d_pair->privateKey.key.data()); #else /* HAVE_CRYPTO_BOX_CURVE25519XCHACHA20POLY1305_EASY */ res = -1; #endif /* HAVE_CRYPTO_BOX_CURVE25519XCHACHA20POLY1305_EASY */ @@ -110,7 +113,7 @@ int DNSCryptQuery::computeSharedKey() } if (res != 0) { - sodium_munlock(d_sharedKey, sizeof(d_sharedKey)); + sodium_munlock(d_sharedKey.data(), d_sharedKey.size()); return res; } @@ -118,14 +121,11 @@ int DNSCryptQuery::computeSharedKey() return res; } #else -DNSCryptQuery::~DNSCryptQuery() -{ -} +DNSCryptQuery::~DNSCryptQuery() = default; #endif /* HAVE_CRYPTO_BOX_EASY_AFTERNM */ -DNSCryptContext::~DNSCryptContext() { -} +DNSCryptContext::~DNSCryptContext() = default; DNSCryptContext::DNSCryptContext(const std::string& pName, const std::vector& certKeys): d_certKeyPaths(certKeys), providerName(pName) { @@ -137,23 +137,23 @@ DNSCryptContext::DNSCryptContext(const std::string& pName, const DNSCryptCert& c addNewCertificate(certificate, pKey); } -void DNSCryptContext::generateProviderKeys(unsigned char publicKey[DNSCRYPT_PROVIDER_PUBLIC_KEY_SIZE], unsigned char privateKey[DNSCRYPT_PROVIDER_PRIVATE_KEY_SIZE]) +void DNSCryptContext::generateProviderKeys(DNSCryptCertSignedData::ResolverPublicKeyType& publicKey, DNSCryptCertSignedData::ResolverPrivateKeyType& privateKey) { - int res = crypto_sign_ed25519_keypair(publicKey, privateKey); + int res = crypto_sign_ed25519_keypair(publicKey.data(), privateKey.data()); if (res != 0) { throw std::runtime_error("Error generating DNSCrypt provider keys"); } } -std::string DNSCryptContext::getProviderFingerprint(unsigned char publicKey[DNSCRYPT_PROVIDER_PUBLIC_KEY_SIZE]) +std::string DNSCryptContext::getProviderFingerprint(const DNSCryptCertSignedData::ResolverPublicKeyType& publicKey) { boost::format fmt("%02X%02X"); ostringstream ret; for (size_t idx = 0; idx < DNSCRYPT_PROVIDER_PUBLIC_KEY_SIZE; idx += 2) { - ret << (fmt % static_cast(publicKey[idx]) % static_cast(publicKey[idx+1])); + ret << (fmt % static_cast(publicKey.at(idx)) % static_cast(publicKey.at(idx+1))); if (idx < (DNSCRYPT_PROVIDER_PUBLIC_KEY_SIZE - 2)) { ret << ":"; } @@ -162,31 +162,31 @@ std::string DNSCryptContext::getProviderFingerprint(unsigned char publicKey[DNSC return ret.str(); } -void DNSCryptContext::setExchangeVersion(const DNSCryptExchangeVersion& version, unsigned char esVersion[sizeof(DNSCryptCert::esVersion)]) +void DNSCryptContext::setExchangeVersion(const DNSCryptExchangeVersion& version, DNSCryptCert::ESVersionType& esVersion) { - esVersion[0] = 0x00; + esVersion.at(0) = 0x00; if (version == DNSCryptExchangeVersion::VERSION1) { - esVersion[1] = { 0x01 }; + esVersion.at(1) = { 0x01 }; } else if (version == DNSCryptExchangeVersion::VERSION2) { - esVersion[1] = { 0x02 }; + esVersion.at(1) = { 0x02 }; } else { throw std::runtime_error("Unknown DNSCrypt exchange version"); } } -DNSCryptExchangeVersion DNSCryptContext::getExchangeVersion(const unsigned char esVersion[sizeof(DNSCryptCert::esVersion)]) +DNSCryptExchangeVersion DNSCryptContext::getExchangeVersion(const DNSCryptCert::ESVersionType& esVersion) { - if (esVersion[0] != 0x00) { + if (esVersion.at(0) != 0x00) { throw std::runtime_error("Unknown DNSCrypt exchange version"); } - if (esVersion[1] == 0x01) { + if (esVersion.at(1) == 0x01) { return DNSCryptExchangeVersion::VERSION1; } - else if (esVersion[1] == 0x02) { + if (esVersion.at(1) == 0x02) { return DNSCryptExchangeVersion::VERSION2; } @@ -198,40 +198,32 @@ DNSCryptExchangeVersion DNSCryptContext::getExchangeVersion(const DNSCryptCert& return getExchangeVersion(cert.esVersion); } - -void DNSCryptContext::generateCertificate(uint32_t serial, time_t begin, time_t end, const DNSCryptExchangeVersion& version, const unsigned char providerPrivateKey[DNSCRYPT_PROVIDER_PRIVATE_KEY_SIZE], DNSCryptPrivateKey& privateKey, DNSCryptCert& cert) +void DNSCryptContext::generateCertificate(uint32_t serial, time_t begin, time_t end, const DNSCryptExchangeVersion& version, const DNSCryptCertSignedData::ResolverPrivateKeyType& providerPrivateKey, DNSCryptPrivateKey& privateKey, DNSCryptCert& cert) { - unsigned char magic[DNSCRYPT_CERT_MAGIC_SIZE] = DNSCRYPT_CERT_MAGIC_VALUE; - unsigned char protocolMinorVersion[] = DNSCRYPT_CERT_PROTOCOL_MINOR_VERSION_VALUE; - unsigned char pubK[DNSCRYPT_PUBLIC_KEY_SIZE]; - unsigned char esVersion[sizeof(DNSCryptCert::esVersion)]; - setExchangeVersion(version, esVersion); - - generateResolverKeyPair(privateKey, pubK); + setExchangeVersion(version, cert.esVersion); + DNSCryptPublicKeyType pubKey; + generateResolverKeyPair(privateKey, pubKey); - memcpy(cert.magic, magic, sizeof(magic)); - memcpy(cert.esVersion, esVersion, sizeof(esVersion)); - memcpy(cert.protocolMinorVersion, protocolMinorVersion, sizeof(protocolMinorVersion)); - memcpy(cert.signedData.resolverPK, pubK, sizeof(cert.signedData.resolverPK)); - memcpy(cert.signedData.clientMagic, pubK, sizeof(cert.signedData.clientMagic)); + cert.magic = DNSCRYPT_CERT_MAGIC_VALUE; + cert.protocolMinorVersion = DNSCRYPT_CERT_PROTOCOL_MINOR_VERSION_VALUE; + memcpy(cert.signedData.clientMagic.data(), pubKey.data(), cert.signedData.clientMagic.size()); + memcpy(cert.signedData.resolverPK.data(), pubKey.data(), cert.signedData.resolverPK.size()); cert.signedData.serial = htonl(serial); // coverity[store_truncates_time_t] - cert.signedData.tsStart = htonl((uint32_t) begin); + cert.signedData.tsStart = htonl(static_cast(begin)); // coverity[store_truncates_time_t] - cert.signedData.tsEnd = htonl((uint32_t) end); + cert.signedData.tsEnd = htonl(static_cast(end)); unsigned long long signatureSize = 0; - int res = crypto_sign_ed25519(cert.signature, + int res = crypto_sign_ed25519(cert.signature.data(), &signatureSize, - (unsigned char*) &cert.signedData, + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have + reinterpret_cast(&cert.signedData), sizeof(cert.signedData), - providerPrivateKey); + providerPrivateKey.data()); - if (res == 0) { - assert(signatureSize == sizeof(DNSCryptCertSignedData) + DNSCRYPT_SIGNATURE_SIZE); - } - else { + if (res != 0 || signatureSize != (sizeof(DNSCryptCertSignedData) + DNSCRYPT_SIGNATURE_SIZE)) { throw std::runtime_error("Error generating DNSCrypt certificate"); } } @@ -239,10 +231,12 @@ void DNSCryptContext::generateCertificate(uint32_t serial, time_t begin, time_t void DNSCryptContext::loadCertFromFile(const std::string&filename, DNSCryptCert& dest) { ifstream file(filename); - file.read((char *) &dest, sizeof(dest)); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have + file.read(reinterpret_cast(&dest), sizeof(dest)); - if (file.fail()) + if (file.fail()) { throw std::runtime_error("Invalid dnscrypt certificate file " + filename); + } file.close(); } @@ -250,23 +244,24 @@ void DNSCryptContext::loadCertFromFile(const std::string&filename, DNSCryptCert& void DNSCryptContext::saveCertFromFile(const DNSCryptCert& cert, const std::string&filename) { ofstream file(filename); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have file.write(reinterpret_cast(&cert), sizeof(cert)); file.close(); } -void DNSCryptContext::generateResolverKeyPair(DNSCryptPrivateKey& privK, unsigned char pubK[DNSCRYPT_PUBLIC_KEY_SIZE]) +void DNSCryptContext::generateResolverKeyPair(DNSCryptPrivateKey& privK, DNSCryptPublicKeyType& pubK) { - int res = crypto_box_keypair(pubK, privK.key); + int res = crypto_box_keypair(pubK.data(), privK.key.data()); if (res != 0) { throw std::runtime_error("Error generating DNSCrypt resolver keys"); } } -void DNSCryptContext::computePublicKeyFromPrivate(const DNSCryptPrivateKey& privK, unsigned char* pubK) +void DNSCryptContext::computePublicKeyFromPrivate(const DNSCryptPrivateKey& privK, DNSCryptCertificatePair::PublicKeyType& pubK) { - int res = crypto_scalarmult_base(pubK, - privK.key); + int res = crypto_scalarmult_base(pubK.data(), + privK.key.data()); if (res != 0) { throw std::runtime_error("Error computing dnscrypt public key from the private one"); @@ -275,14 +270,14 @@ void DNSCryptContext::computePublicKeyFromPrivate(const DNSCryptPrivateKey& priv std::string DNSCryptContext::certificateDateToStr(uint32_t date) { - char buf[20]; - time_t tdate = static_cast(ntohl(date)); - struct tm date_tm; - + std::string result; + auto tdate = static_cast(ntohl(date)); + tm date_tm{}; localtime_r(&tdate, &date_tm); - strftime(buf, sizeof(buf), "%Y-%m-%d %H:%M:%S", &date_tm); - - return string(buf); + result.resize(20); + auto got = strftime(result.data(), result.size(), "%Y-%m-%d %H:%M:%S", &date_tm); + result.resize(got); + return result; } void DNSCryptContext::addNewCertificate(std::shared_ptr& newCert, bool reload) @@ -295,9 +290,7 @@ void DNSCryptContext::addNewCertificate(std::shared_ptr /* on reload we just assume that this is the same certificate */ return; } - else { - throw std::runtime_error("Error adding a new certificate: we already have a certificate with the same serial"); - } + throw std::runtime_error("Error adding a new certificate: we already have a certificate with the same serial"); } } @@ -343,7 +336,7 @@ void DNSCryptContext::reloadCertificates() newCerts.push_back(DNSCryptContext::loadCertificatePair(pair.cert, pair.key)); } } - + { *(d_certs.write_lock()) = std::move(newCerts); } @@ -357,7 +350,7 @@ std::vector> DNSCryptContext::getCertif void DNSCryptContext::markActive(uint32_t serial) { for (const auto& pair : *d_certs.write_lock()) { - if (pair->active == false && pair->cert.getSerial() == serial) { + if (!pair->active && pair->cert.getSerial() == serial) { pair->active = true; return; } @@ -368,7 +361,7 @@ void DNSCryptContext::markActive(uint32_t serial) void DNSCryptContext::markInactive(uint32_t serial) { for (const auto& pair : *d_certs.write_lock()) { - if (pair->active == true && pair->cert.getSerial() == serial) { + if (pair->active && pair->cert.getSerial() == serial) { pair->active = false; return; } @@ -381,31 +374,30 @@ void DNSCryptContext::removeInactiveCertificate(uint32_t serial) auto certs = d_certs.write_lock(); for (auto it = certs->begin(); it != certs->end(); ) { - if ((*it)->active == false && (*it)->cert.getSerial() == serial) { + if (!(*it)->active && (*it)->cert.getSerial() == serial) { it = certs->erase(it); return; - } else { - it++; } + it++; } throw std::runtime_error("No inactive certificate found with this serial"); } bool DNSCryptQuery::parsePlaintextQuery(const PacketBuffer& packet) { - assert(d_ctx != nullptr); - if (packet.size() < sizeof(dnsheader)) { return false; } - const dnsheader_aligned dh(packet.data()); - if (dh->qr || ntohs(dh->qdcount) != 1 || dh->ancount != 0 || dh->nscount != 0 || static_cast(dh->opcode) != Opcode::Query) { + const dnsheader_aligned dnsHeader(packet.data()); + if (dnsHeader->qr || ntohs(dnsHeader->qdcount) != 1 || dnsHeader->ancount != 0 || dnsHeader->nscount != 0 || static_cast(dnsHeader->opcode) != Opcode::Query) { return false; } - unsigned int qnameWireLength; - uint16_t qtype, qclass; + unsigned int qnameWireLength{0}; + uint16_t qtype{0}; + uint16_t qclass{0}; + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have DNSName qname(reinterpret_cast(packet.data()), packet.size(), sizeof(dnsheader), false, &qtype, &qclass, &qnameWireLength); if ((packet.size() - sizeof(dnsheader)) < (qnameWireLength + sizeof(qtype) + sizeof(qclass))) { return false; @@ -415,12 +407,12 @@ bool DNSCryptQuery::parsePlaintextQuery(const PacketBuffer& packet) return false; } - if (qname != d_ctx->getProviderName()) { + if (d_ctx == nullptr || qname != d_ctx->getProviderName()) { return false; } d_qname = std::move(qname); - d_id = dh->id; + d_id = dnsHeader->id; d_valid = true; return true; @@ -428,11 +420,11 @@ bool DNSCryptQuery::parsePlaintextQuery(const PacketBuffer& packet) void DNSCryptContext::getCertificateResponse(time_t now, const DNSName& qname, uint16_t qid, PacketBuffer& response) { - GenericDNSPacketWriter pw(response, qname, QType::TXT, QClass::IN, Opcode::Query); - struct dnsheader * dh = pw.getHeader(); - dh->id = qid; - dh->qr = true; - dh->rcode = RCode::NoError; + GenericDNSPacketWriter packetWriter(response, qname, QType::TXT, QClass::IN, Opcode::Query); + struct dnsheader * dnsHeader = packetWriter.getHeader(); + dnsHeader->id = qid; + dnsHeader->qr = true; + dnsHeader->rcode = RCode::NoError; auto certs = d_certs.read_lock(); for (const auto& pair : *certs) { @@ -440,24 +432,26 @@ void DNSCryptContext::getCertificateResponse(time_t now, const DNSName& qname, u continue; } - pw.startRecord(qname, QType::TXT, (DNSCRYPT_CERTIFICATE_RESPONSE_TTL), QClass::IN, DNSResourceRecord::ANSWER, true); + packetWriter.startRecord(qname, QType::TXT, (DNSCRYPT_CERTIFICATE_RESPONSE_TTL), QClass::IN, DNSResourceRecord::ANSWER, true); std::string scert; uint8_t certSize = sizeof(pair->cert); - scert.assign((const char*) &certSize, sizeof(certSize)); - scert.append((const char*) &pair->cert, certSize); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + scert.assign(reinterpret_cast(&certSize), sizeof(certSize)); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + scert.append(reinterpret_cast(&pair->cert), certSize); - pw.xfrBlob(scert); - pw.commit(); + packetWriter.xfrBlob(scert); + packetWriter.commit(); } } bool DNSCryptContext::magicMatchesAPublicKey(DNSCryptQuery& query, time_t now) { - const unsigned char* magic = query.getClientMagic(); + const auto& magic = query.getClientMagic(); auto certs = d_certs.read_lock(); for (const auto& pair : *certs) { - if (pair->cert.isValid(now) && memcmp(magic, pair->cert.signedData.clientMagic, DNSCRYPT_CLIENT_MAGIC_SIZE) == 0) { + if (pair->cert.isValid(now) && magic == pair->cert.signedData.clientMagic) { query.setCertificatePair(pair); return true; } @@ -468,8 +462,6 @@ bool DNSCryptContext::magicMatchesAPublicKey(DNSCryptQuery& query, time_t now) bool DNSCryptQuery::isEncryptedQuery(const PacketBuffer& packet, bool tcp, time_t now) { - assert(d_ctx != nullptr); - d_encrypted = false; if (packet.size() < sizeof(DNSCryptQueryHeader)) { @@ -480,11 +472,12 @@ bool DNSCryptQuery::isEncryptedQuery(const PacketBuffer& packet, bool tcp, time_ return false; } - const struct DNSCryptQueryHeader* header = reinterpret_cast(packet.data()); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have + const auto* header = reinterpret_cast(packet.data()); d_header = *header; - if (!d_ctx->magicMatchesAPublicKey(*this, now)) { + if (d_ctx == nullptr || !d_ctx->magicMatchesAPublicKey(*this, now)) { return false; } @@ -495,9 +488,9 @@ bool DNSCryptQuery::isEncryptedQuery(const PacketBuffer& packet, bool tcp, time_ void DNSCryptQuery::getDecrypted(bool tcp, PacketBuffer& packet) { - assert(d_encrypted); - assert(d_pair != nullptr); - assert(d_valid == false); + if (!d_encrypted || d_valid || d_pair == nullptr) { + throw std::runtime_error("Trying to decrypt a DNSCrypt query in an invalid state"); + } #ifdef DNSCRYPT_STRICT_PADDING_LENGTH if (tcp && ((packet.size() - sizeof(DNSCryptQueryHeader)) % DNSCRYPT_PADDED_BLOCK_SIZE) != 0) { @@ -506,13 +499,9 @@ void DNSCryptQuery::getDecrypted(bool tcp, PacketBuffer& packet) } #endif - unsigned char nonce[DNSCRYPT_NONCE_SIZE]; - static_assert(sizeof(nonce) == (2* sizeof(d_header.clientNonce)), "Nonce should be larger than clientNonce (half)"); - static_assert(sizeof(d_header.clientPK) == DNSCRYPT_PUBLIC_KEY_SIZE, "Client Public key size is not right"); - static_assert(sizeof(d_pair->privateKey.key) == DNSCRYPT_PRIVATE_KEY_SIZE, "Private key size is not right"); - - memcpy(nonce, &d_header.clientNonce, sizeof(d_header.clientNonce)); - memset(nonce + sizeof(d_header.clientNonce), 0, sizeof(nonce) - sizeof(d_header.clientNonce)); + DNSCryptNonceType nonce; + memcpy(nonce.data(), d_header.clientNonce.data(), d_header.clientNonce.size()); + memset(&nonce.at(d_header.clientNonce.size()), 0, nonce.size() - d_header.clientNonce.size()); #ifdef HAVE_CRYPTO_BOX_EASY_AFTERNM int res = computeSharedKey(); @@ -524,19 +513,23 @@ void DNSCryptQuery::getDecrypted(bool tcp, PacketBuffer& packet) const DNSCryptExchangeVersion version = getVersion(); if (version == DNSCryptExchangeVersion::VERSION1) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have res = crypto_box_open_easy_afternm(reinterpret_cast(packet.data()), + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have reinterpret_cast(&packet.at(sizeof(DNSCryptQueryHeader))), packet.size() - sizeof(DNSCryptQueryHeader), - nonce, - d_sharedKey); + nonce.data(), + d_sharedKey.data()); } else if (version == DNSCryptExchangeVersion::VERSION2) { #ifdef HAVE_CRYPTO_BOX_CURVE25519XCHACHA20POLY1305_EASY + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have res = crypto_box_curve25519xchacha20poly1305_open_easy_afternm(reinterpret_cast(packet.data()), + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have reinterpret_cast(&packet.at(sizeof(DNSCryptQueryHeader))), packet.size() - sizeof(DNSCryptQueryHeader), - nonce, - d_sharedKey); + nonce.data(), + d_sharedKey.data()); #else /* HAVE_CRYPTO_BOX_CURVE25519XCHACHA20POLY1305_EASY */ res = -1; #endif /* HAVE_CRYPTO_BOX_CURVE25519XCHACHA20POLY1305_EASY */ @@ -545,12 +538,14 @@ void DNSCryptQuery::getDecrypted(bool tcp, PacketBuffer& packet) } #else /* HAVE_CRYPTO_BOX_EASY_AFTERNM */ + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have int res = crypto_box_open_easy(reinterpret_cast(packet.data()), + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have reinterpret_cast(&packet.at(sizeof(DNSCryptQueryHeader))), packet.size() - sizeof(DNSCryptQueryHeader), - nonce, - d_header.clientPK, - d_pair->privateKey.key); + nonce.data(), + d_header.clientPK.data(), + d_pair->privateKey.key.data()); #endif /* HAVE_CRYPTO_BOX_EASY_AFTERNM */ if (res != 0) { @@ -560,10 +555,16 @@ void DNSCryptQuery::getDecrypted(bool tcp, PacketBuffer& packet) uint16_t decryptedQueryLen = packet.size() - sizeof(DNSCryptQueryHeader) - DNSCRYPT_MAC_SIZE; uint16_t pos = decryptedQueryLen; - assert(pos < packet.size()); + if (pos >= packet.size()) { + vinfolog("Dropping encrypted query we can't decrypt (invalid position)"); + return; + } + d_paddedLen = decryptedQueryLen; - while (pos > 0 && packet.at(pos - 1) == 0) pos--; + while (pos > 0 && packet.at(pos - 1) == 0) { + pos--; + } if (pos == 0 || packet.at(pos - 1) != 0x80) { vinfolog("Dropping encrypted query with invalid padding value"); @@ -586,7 +587,9 @@ void DNSCryptQuery::getDecrypted(bool tcp, PacketBuffer& packet) void DNSCryptQuery::getCertificateResponse(time_t now, PacketBuffer& response) const { - assert(d_ctx != nullptr); + if (d_ctx == nullptr) { + throw std::runtime_error("Trying to get a certificate response from a DNSCrypt query lacking context"); + } d_ctx->getCertificateResponse(now, d_qname, d_id, response); } @@ -603,14 +606,15 @@ void DNSCryptQuery::parsePacket(PacketBuffer& packet, bool tcp, time_t now) } } -void DNSCryptQuery::fillServerNonce(unsigned char* nonce) const +void DNSCryptQuery::fillServerNonce(DNSCryptNonceType& nonce) { - uint32_t* dest = reinterpret_cast(nonce); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + auto* dest = reinterpret_cast(&nonce.at(DNSCRYPT_NONCE_SIZE / 2)); static const size_t nonceSize = DNSCRYPT_NONCE_SIZE / 2; - for (size_t pos = 0; pos < (nonceSize / sizeof(*dest)); pos++) - { + for (size_t pos = 0; pos < (nonceSize / sizeof(*dest)); pos++) { const uint32_t value = randombytes_random(); + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic): sorry memcpy(dest + pos, &value, sizeof(value)); } } @@ -624,19 +628,22 @@ uint16_t DNSCryptQuery::computePaddingSize(uint16_t unpaddedLen, size_t maxLen) size_t paddedSize = 0; uint16_t result = 0; uint32_t rnd = 0; - assert(d_header.clientNonce); - assert(d_pair != nullptr); + if (d_pair == nullptr) { + throw std::runtime_error("Trying to compute the padding size from an invalid DNSCrypt query"); + } - unsigned char nonce[DNSCRYPT_NONCE_SIZE]; - memcpy(nonce, d_header.clientNonce, (DNSCRYPT_NONCE_SIZE / 2)); - memcpy(&(nonce[DNSCRYPT_NONCE_SIZE / 2]), d_header.clientNonce, (DNSCRYPT_NONCE_SIZE / 2)); - crypto_stream((unsigned char*) &rnd, sizeof(rnd), nonce, d_pair->privateKey.key); + DNSCryptNonceType nonce; + memcpy(nonce.data(), d_header.clientNonce.data(), d_header.clientNonce.size()); + memcpy(&(nonce.at(d_header.clientNonce.size())), d_header.clientNonce.data(), d_header.clientNonce.size()); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have + crypto_stream(reinterpret_cast(&rnd), sizeof(rnd), nonce.data(), d_pair->privateKey.key.data()); paddedSize = unpaddedLen + rnd % (maxLen - unpaddedLen + 1); paddedSize += DNSCRYPT_PADDED_BLOCK_SIZE - (paddedSize % DNSCRYPT_PADDED_BLOCK_SIZE); - if (paddedSize > maxLen) + if (paddedSize > maxLen) { paddedSize = maxLen; + } result = paddedSize - unpaddedLen; @@ -645,12 +652,11 @@ uint16_t DNSCryptQuery::computePaddingSize(uint16_t unpaddedLen, size_t maxLen) int DNSCryptQuery::encryptResponse(PacketBuffer& response, size_t maxResponseSize, bool tcp) { - struct DNSCryptResponseHeader responseHeader; - assert(response.size() > 0); - assert(maxResponseSize >= response.size()); - assert(d_encrypted == true); - assert(d_pair != nullptr); + if (response.empty() || response.size() > maxResponseSize || !d_encrypted || d_pair == nullptr) { + throw std::runtime_error("Trying to encrypt a DNSCrypt response from an invalid state"); + } + DNSCryptResponseHeader responseHeader{}; /* a DNSCrypt UDP response can't be larger than the (padded) DNSCrypt query */ if (!tcp && d_paddedLen < response.size()) { /* so we need to truncate it */ @@ -658,7 +664,8 @@ int DNSCryptQuery::encryptResponse(PacketBuffer& response, size_t maxResponseSiz if (response.size() > sizeof(dnsheader)) { unsigned int qnameWireLength = 0; - DNSName tempQName(reinterpret_cast(response.data()), response.size(), sizeof(dnsheader), false, 0, 0, &qnameWireLength); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have + DNSName tempQName(reinterpret_cast(response.data()), response.size(), sizeof(dnsheader), false, nullptr, nullptr, &qnameWireLength); if (qnameWireLength > 0) { questionSize = qnameWireLength + DNS_TYPE_SIZE + DNS_CLASS_SIZE; } @@ -670,9 +677,13 @@ int DNSCryptQuery::encryptResponse(PacketBuffer& response, size_t maxResponseSiz /* that does not seem right but let's truncate even more */ response.resize(d_paddedLen); } - struct dnsheader* dh = reinterpret_cast(response.data()); - dh->ancount = dh->arcount = dh->nscount = 0; - dh->tc = 1; + dnsdist::PacketMangling::editDNSHeaderFromPacket(response, [](dnsheader& header) { + header.ancount = 0; + header.arcount = 0; + header.nscount = 0; + header.tc = 1; + return true; + }); } size_t requiredSize = sizeof(responseHeader) + DNSCRYPT_MAC_SIZE + response.size(); @@ -684,13 +695,13 @@ int DNSCryptQuery::encryptResponse(PacketBuffer& response, size_t maxResponseSiz return ENOBUFS; } - memcpy(&responseHeader.nonce, &d_header.clientNonce, sizeof d_header.clientNonce); - fillServerNonce(&(responseHeader.nonce[sizeof(d_header.clientNonce)])); + memcpy(responseHeader.nonce.data(), d_header.clientNonce.data(), d_header.clientNonce.size()); + fillServerNonce(responseHeader.nonce); size_t responseLen = response.size(); /* moving the existing response after the header + MAC */ response.resize(requiredSize); - std::copy_backward(response.begin(), response.begin() + responseLen, response.begin() + responseLen + sizeof(responseHeader) + DNSCRYPT_MAC_SIZE); + std::copy_backward(response.begin(), response.begin() + static_cast(responseLen), response.begin() + static_cast(responseLen + sizeof(responseHeader) + DNSCRYPT_MAC_SIZE)); uint16_t pos = 0; /* copying header */ @@ -718,19 +729,23 @@ int DNSCryptQuery::encryptResponse(PacketBuffer& response, size_t maxResponseSiz const DNSCryptExchangeVersion version = getVersion(); if (version == DNSCryptExchangeVersion::VERSION1) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have res = crypto_box_easy_afternm(reinterpret_cast(&response.at(sizeof(responseHeader))), + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have reinterpret_cast(&response.at(toEncryptPos)), responseLen + paddingSize, - responseHeader.nonce, - d_sharedKey); + responseHeader.nonce.data(), + d_sharedKey.data()); } else if (version == DNSCryptExchangeVersion::VERSION2) { #ifdef HAVE_CRYPTO_BOX_CURVE25519XCHACHA20POLY1305_EASY + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have res = crypto_box_curve25519xchacha20poly1305_easy_afternm(reinterpret_cast(&response.at(sizeof(responseHeader))), + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have reinterpret_cast(&response.at(toEncryptPos)), responseLen + paddingSize, - responseHeader.nonce, - d_sharedKey); + responseHeader.nonce.data(), + d_sharedKey.data()); #else /* HAVE_CRYPTO_BOX_CURVE25519XCHACHA20POLY1305_EASY */ res = -1; #endif /* HAVE_CRYPTO_BOX_CURVE25519XCHACHA20POLY1305_EASY */ @@ -739,28 +754,33 @@ int DNSCryptQuery::encryptResponse(PacketBuffer& response, size_t maxResponseSiz res = -1; } #else + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have int res = crypto_box_easy(reinterpret_cast(&response.at(sizeof(responseHeader))), + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have reinterpret_cast(&response.at(toEncryptPos)), responseLen + paddingSize, - responseHeader.nonce, - d_header.clientPK, - d_pair->privateKey.key); + responseHeader.nonce.data(), + d_header.clientPK.data(), + d_pair->privateKey.key.data()); #endif /* HAVE_CRYPTO_BOX_EASY_AFTERNM */ if (res == 0) { - assert(pos == requiredSize); + if (pos != requiredSize) { + throw std::runtime_error("Unexpected size for encrypted DNSCrypt response"); + } } return res; } -int DNSCryptContext::encryptQuery(PacketBuffer& packet, size_t maximumSize, const unsigned char clientPublicKey[DNSCRYPT_PUBLIC_KEY_SIZE], const DNSCryptPrivateKey& clientPrivateKey, const unsigned char clientNonce[DNSCRYPT_NONCE_SIZE / 2], bool tcp, const std::shared_ptr& cert) const +int DNSCryptContext::encryptQuery(PacketBuffer& packet, size_t maximumSize, const DNSCryptCertificatePair::PublicKeyType& clientPublicKey, const DNSCryptPrivateKey& clientPrivateKey, const DNSCryptClientNonceType& clientNonce, bool tcp, const std::shared_ptr& cert) { - assert(packet.size() > 0); - assert(cert != nullptr); + if (packet.empty() || cert == nullptr) { + throw std::runtime_error("Trying to encrypt a DNSCrypt query with an invalid state"); + } size_t queryLen = packet.size(); - unsigned char nonce[DNSCRYPT_NONCE_SIZE]; + DNSCryptNonceType nonce; size_t requiredSize = sizeof(DNSCryptQueryHeader) + DNSCRYPT_MAC_SIZE + queryLen; /* this is not optimal, we should compute a random padding size, multiple of DNSCRYPT_PADDED_BLOCK_SIZE, DNSCRYPT_PADDED_BLOCK_SIZE <= padding size <= 4096? */ @@ -778,20 +798,20 @@ int DNSCryptContext::encryptQuery(PacketBuffer& packet, size_t maximumSize, cons /* moving the existing query after the header + MAC */ packet.resize(requiredSize); - std::copy_backward(packet.begin(), packet.begin() + queryLen, packet.begin() + queryLen + sizeof(DNSCryptQueryHeader) + DNSCRYPT_MAC_SIZE); + std::copy_backward(packet.begin(), packet.begin() + static_cast(queryLen), packet.begin() + static_cast(queryLen + sizeof(DNSCryptQueryHeader) + DNSCRYPT_MAC_SIZE)); size_t pos = 0; /* client magic */ - memcpy(&packet.at(pos), cert->signedData.clientMagic, sizeof(cert->signedData.clientMagic)); - pos += sizeof(cert->signedData.clientMagic); + memcpy(&packet.at(pos), cert->signedData.clientMagic.data(), sizeof(cert->signedData.clientMagic)); + pos += cert->signedData.clientMagic.size(); /* client PK */ - memcpy(&packet.at(pos), clientPublicKey, DNSCRYPT_PUBLIC_KEY_SIZE); + memcpy(&packet.at(pos), clientPublicKey.data(), clientPublicKey.size()); pos += DNSCRYPT_PUBLIC_KEY_SIZE; /* client nonce */ - memcpy(&packet.at(pos), clientNonce, DNSCRYPT_NONCE_SIZE / 2); - pos += DNSCRYPT_NONCE_SIZE / 2; + memcpy(&packet.at(pos), clientNonce.data(), clientNonce.size()); + pos += clientNonce.size(); size_t encryptedPos = pos; /* clear the MAC bytes */ @@ -807,28 +827,32 @@ int DNSCryptContext::encryptQuery(PacketBuffer& packet, size_t maximumSize, cons memset(&packet.at(pos), 0, paddingSize - 1); pos += paddingSize - 1; - memcpy(nonce, clientNonce, DNSCRYPT_NONCE_SIZE / 2); - memset(nonce + (DNSCRYPT_NONCE_SIZE / 2), 0, DNSCRYPT_NONCE_SIZE / 2); + memcpy(nonce.data(), clientNonce.data(), clientNonce.size()); + memset(&nonce.at(clientNonce.size()), 0, DNSCRYPT_NONCE_SIZE / 2); const DNSCryptExchangeVersion version = getExchangeVersion(*cert); int res = -1; if (version == DNSCryptExchangeVersion::VERSION1) { + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have res = crypto_box_easy(reinterpret_cast(&packet.at(encryptedPos)), + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have reinterpret_cast(&packet.at(encryptedPos + DNSCRYPT_MAC_SIZE)), queryLen + paddingSize, - nonce, - cert->signedData.resolverPK, - clientPrivateKey.key); + nonce.data(), + cert->signedData.resolverPK.data(), + clientPrivateKey.key.data()); } else if (version == DNSCryptExchangeVersion::VERSION2) { #ifdef HAVE_CRYPTO_BOX_CURVE25519XCHACHA20POLY1305_EASY + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have res = crypto_box_curve25519xchacha20poly1305_easy(reinterpret_cast(&packet.at(encryptedPos)), + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have reinterpret_cast(&packet.at(encryptedPos + DNSCRYPT_MAC_SIZE)), queryLen + paddingSize, - nonce, - cert->signedData.resolverPK, - clientPrivateKey.key); + nonce.data(), + cert->signedData.resolverPK.data(), + clientPrivateKey.key.data()); #endif /* HAVE_CRYPTO_BOX_CURVE25519XCHACHA20POLY1305_EASY */ } else { @@ -836,7 +860,9 @@ int DNSCryptContext::encryptQuery(PacketBuffer& packet, size_t maximumSize, cons } if (res == 0) { - assert(pos == requiredSize); + if (pos != requiredSize) { + throw std::runtime_error("Unexpected size for encrypted DNSCrypt query"); + } } return res; @@ -845,13 +871,14 @@ int DNSCryptContext::encryptQuery(PacketBuffer& packet, size_t maximumSize, cons bool generateDNSCryptCertificate(const std::string& providerPrivateKeyFile, uint32_t serial, time_t begin, time_t end, DNSCryptExchangeVersion version, DNSCryptCert& certOut, DNSCryptPrivateKey& keyOut) { bool success = false; - unsigned char providerPrivateKey[DNSCRYPT_PROVIDER_PRIVATE_KEY_SIZE]; - sodium_mlock(providerPrivateKey, sizeof(providerPrivateKey)); - sodium_memzero(providerPrivateKey, sizeof(providerPrivateKey)); + DNSCryptCertSignedData::ResolverPrivateKeyType providerPrivateKey; + sodium_mlock(providerPrivateKey.data(), providerPrivateKey.size()); + sodium_memzero(providerPrivateKey.data(), providerPrivateKey.size()); try { ifstream providerKStream(providerPrivateKeyFile); - providerKStream.read((char*) providerPrivateKey, sizeof(providerPrivateKey)); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast): this is the API we have + providerKStream.read(reinterpret_cast(providerPrivateKey.data()), providerPrivateKey.size()); if (providerKStream.fail()) { providerKStream.close(); throw std::runtime_error("Invalid DNSCrypt provider key file " + providerPrivateKeyFile); @@ -864,8 +891,8 @@ bool generateDNSCryptCertificate(const std::string& providerPrivateKeyFile, uint errlog(e.what()); } - sodium_memzero(providerPrivateKey, sizeof(providerPrivateKey)); - sodium_munlock(providerPrivateKey, sizeof(providerPrivateKey)); + sodium_memzero(providerPrivateKey.data(), providerPrivateKey.size()); + sodium_munlock(providerPrivateKey.data(), providerPrivateKey.size()); return success; } diff --git a/pdns/dnscrypt.hh b/pdns/dnscrypt.hh index a42be6ab2b..b25a081d22 100644 --- a/pdns/dnscrypt.hh +++ b/pdns/dnscrypt.hh @@ -95,13 +95,19 @@ class DNSCryptContext; struct DNSCryptCertSignedData { - unsigned char resolverPK[DNSCRYPT_PROVIDER_PUBLIC_KEY_SIZE]; - unsigned char clientMagic[DNSCRYPT_CLIENT_MAGIC_SIZE]; - uint32_t serial; - uint32_t tsStart; - uint32_t tsEnd; + using ResolverPublicKeyType = std::array; + using ResolverPrivateKeyType = std::array; + using ClientMagicType = std::array; + ResolverPublicKeyType resolverPK; + ClientMagicType clientMagic; + uint32_t serial{0}; + uint32_t tsStart{0}; + uint32_t tsEnd{0}; }; +static_assert(sizeof(DNSCryptCertSignedData) == (DNSCRYPT_PROVIDER_PUBLIC_KEY_SIZE + DNSCRYPT_CLIENT_MAGIC_SIZE + 12)); +static_assert(std::is_trivially_copyable_v == true); + class DNSCryptCert { public: @@ -122,31 +128,44 @@ public: // coverity[store_truncates_time_t] return ntohl(getTSStart()) <= static_cast(now) && static_cast(now) <= ntohl(getTSEnd()); } - unsigned char magic[DNSCRYPT_CERT_MAGIC_SIZE]; - unsigned char esVersion[2]; - unsigned char protocolMinorVersion[2]; - unsigned char signature[DNSCRYPT_SIGNATURE_SIZE]; - struct DNSCryptCertSignedData signedData; + using ESVersionType = std::array; + using ProtocolMinorVersionType = std::array; + using CertMagicType = std::array; + CertMagicType magic; + ESVersionType esVersion; + ProtocolMinorVersionType protocolMinorVersion; + std::array signature; + DNSCryptCertSignedData signedData; }; static_assert((sizeof(DNSCryptCertSignedData) + DNSCRYPT_SIGNATURE_SIZE) == 116, "Dnscrypt cert signed data size + signature size should be 116!"); static_assert(sizeof(DNSCryptCert) == 124, "Dnscrypt cert size should be 124!"); +using DNSCryptClientNonceType = std::array; +using DNSCryptNonceType = std::array; +using DNSCryptPublicKeyType = std::array; +using DNSCryptClientMagicType = std::array; + struct DNSCryptQueryHeader { - unsigned char clientMagic[DNSCRYPT_CLIENT_MAGIC_SIZE]; - unsigned char clientPK[DNSCRYPT_PUBLIC_KEY_SIZE]; - unsigned char clientNonce[DNSCRYPT_NONCE_SIZE / 2]; + DNSCryptClientMagicType clientMagic; + DNSCryptPublicKeyType clientPK; + DNSCryptClientNonceType clientNonce; }; static_assert(sizeof(DNSCryptQueryHeader) == 52, "Dnscrypt query header size should be 52!"); +static_assert(std::is_trivially_copyable_v == true); struct DNSCryptResponseHeader { + // a const std::array is not trivially copyable, unfortunately const unsigned char resolverMagic[DNSCRYPT_RESOLVER_MAGIC_SIZE] = DNSCRYPT_RESOLVER_MAGIC; - unsigned char nonce[DNSCRYPT_NONCE_SIZE]; + DNSCryptNonceType nonce; }; +static_assert(sizeof(DNSCryptResponseHeader) == (DNSCRYPT_RESOLVER_MAGIC_SIZE + DNSCRYPT_NONCE_SIZE), "Dnscrypt response header size is incorrect!"); +static_assert(std::is_trivially_copyable_v == true); + typedef enum { VERSION1, VERSION2 @@ -160,15 +179,17 @@ public: void loadFromFile(const std::string& keyFile); void saveToFile(const std::string& keyFile) const; - unsigned char key[DNSCRYPT_PRIVATE_KEY_SIZE]; + using PrivateKeyType = std::array; + PrivateKeyType key{}; }; struct DNSCryptCertificatePair { - unsigned char publicKey[DNSCRYPT_PUBLIC_KEY_SIZE]; + using PublicKeyType = std::array; + PublicKeyType publicKey; DNSCryptCert cert; DNSCryptPrivateKey privateKey; - bool active; + bool active{false}; }; class DNSCryptQuery @@ -199,7 +220,7 @@ public: return d_id; } - const unsigned char* getClientMagic() const + const DNSCryptClientMagicType& getClientMagic() const { return d_header.clientMagic; } @@ -222,18 +243,19 @@ public: static const size_t s_minUDPLength = 256; private: + static void fillServerNonce(DNSCryptNonceType& nonce); + DNSCryptExchangeVersion getVersion() const; #ifdef HAVE_CRYPTO_BOX_EASY_AFTERNM int computeSharedKey(); #endif /* HAVE_CRYPTO_BOX_EASY_AFTERNM */ - void fillServerNonce(unsigned char* dest) const; uint16_t computePaddingSize(uint16_t unpaddedLen, size_t maxLen) const; bool parsePlaintextQuery(const PacketBuffer& packet); bool isEncryptedQuery(const PacketBuffer& packet, bool tcp, time_t now); DNSCryptQueryHeader d_header; #ifdef HAVE_CRYPTO_BOX_EASY_AFTERNM - unsigned char d_sharedKey[crypto_box_BEFORENMBYTES]; + std::array d_sharedKey; #endif /* HAVE_CRYPTO_BOX_EASY_AFTERNM */ DNSName d_qname; std::shared_ptr d_ctx{nullptr}; @@ -252,15 +274,16 @@ private: class DNSCryptContext { public: - static void generateProviderKeys(unsigned char publicKey[DNSCRYPT_PROVIDER_PUBLIC_KEY_SIZE], unsigned char privateKey[DNSCRYPT_PROVIDER_PRIVATE_KEY_SIZE]); - static std::string getProviderFingerprint(unsigned char publicKey[DNSCRYPT_PROVIDER_PUBLIC_KEY_SIZE]); - static void generateCertificate(uint32_t serial, time_t begin, time_t end, const DNSCryptExchangeVersion& version, const unsigned char providerPrivateKey[DNSCRYPT_PROVIDER_PRIVATE_KEY_SIZE], DNSCryptPrivateKey& privateKey, DNSCryptCert& cert); + static void generateProviderKeys(DNSCryptCertSignedData::ResolverPublicKeyType& publicKey, DNSCryptCertSignedData::ResolverPrivateKeyType& privateKey); + static std::string getProviderFingerprint(const DNSCryptCertSignedData::ResolverPublicKeyType& publicKey); + static void generateCertificate(uint32_t serial, time_t begin, time_t end, const DNSCryptExchangeVersion& version, const DNSCryptCertSignedData::ResolverPrivateKeyType& providerPrivateKey, DNSCryptPrivateKey& privateKey, DNSCryptCert& cert); static void saveCertFromFile(const DNSCryptCert& cert, const std::string&filename); static std::string certificateDateToStr(uint32_t date); - static void generateResolverKeyPair(DNSCryptPrivateKey& privK, unsigned char pubK[DNSCRYPT_PUBLIC_KEY_SIZE]); - static void setExchangeVersion(const DNSCryptExchangeVersion& version, unsigned char esVersion[sizeof(DNSCryptCert::esVersion)]); - static DNSCryptExchangeVersion getExchangeVersion(const unsigned char esVersion[sizeof(DNSCryptCert::esVersion)]); + static void generateResolverKeyPair(DNSCryptPrivateKey& privK, DNSCryptPublicKeyType& pubK); + static void setExchangeVersion(const DNSCryptExchangeVersion& version, DNSCryptCert::ESVersionType& esVersion); + static DNSCryptExchangeVersion getExchangeVersion(const DNSCryptCert::ESVersionType& esVersion); static DNSCryptExchangeVersion getExchangeVersion(const DNSCryptCert& cert); + static int encryptQuery(PacketBuffer& packet, size_t maximumSize, const DNSCryptCertificatePair::PublicKeyType& clientPublicKey, const DNSCryptPrivateKey& clientPrivateKey, const DNSCryptClientNonceType& clientNonce, bool tcp, const std::shared_ptr& cert); struct CertKeyPaths { @@ -271,7 +294,7 @@ public: DNSCryptContext(const std::string& pName, const std::vector& certKeys); DNSCryptContext(const std::string& pName, const DNSCryptCert& certificate, const DNSCryptPrivateKey& pKey); ~DNSCryptContext(); - + void reloadCertificates(); void loadNewCertificate(const std::string& certFile, const std::string& keyFile, bool active=true, bool reload=false); void addNewCertificate(const DNSCryptCert& newCert, const DNSCryptPrivateKey& newKey, bool active=true, bool reload=false); @@ -282,12 +305,11 @@ public: std::vector> getCertificates(); const DNSName& getProviderName() const { return providerName; } - int encryptQuery(PacketBuffer& query, size_t maximumSize, const unsigned char clientPublicKey[DNSCRYPT_PUBLIC_KEY_SIZE], const DNSCryptPrivateKey& clientPrivateKey, const unsigned char clientNonce[DNSCRYPT_NONCE_SIZE / 2], bool tcp, const std::shared_ptr& cert) const; bool magicMatchesAPublicKey(DNSCryptQuery& query, time_t now); void getCertificateResponse(time_t now, const DNSName& qname, uint16_t qid, PacketBuffer& response); private: - static void computePublicKeyFromPrivate(const DNSCryptPrivateKey& privK, unsigned char pubK[DNSCRYPT_PUBLIC_KEY_SIZE]); + static void computePublicKeyFromPrivate(const DNSCryptPrivateKey& privK, DNSCryptCertificatePair::PublicKeyType& pubK); static void loadCertFromFile(const std::string&filename, DNSCryptCert& dest); static std::shared_ptr loadCertificatePair(const std::string& certFile, const std::string& keyFile); diff --git a/pdns/dnsdistdist/dnsdist-lua-bindings-dnscrypt.cc b/pdns/dnsdistdist/dnsdist-lua-bindings-dnscrypt.cc index 114eead045..6d6a61d698 100644 --- a/pdns/dnsdistdist/dnsdist-lua-bindings-dnscrypt.cc +++ b/pdns/dnsdistdist/dnsdist-lua-bindings-dnscrypt.cc @@ -142,12 +142,12 @@ void setupLuaBindingsDNSCrypt(LuaContext& luaCtx, bool client) }); /* DNSCryptCert */ - luaCtx.registerFunction("getMagic", [](const DNSCryptCert& cert) { return std::string(reinterpret_cast(cert.magic), sizeof(cert.magic)); }); - luaCtx.registerFunction("getEsVersion", [](const DNSCryptCert& cert) { return std::string(reinterpret_cast(cert.esVersion), sizeof(cert.esVersion)); }); - luaCtx.registerFunction("getProtocolMinorVersion", [](const DNSCryptCert& cert) { return std::string(reinterpret_cast(cert.protocolMinorVersion), sizeof(cert.protocolMinorVersion)); }); - luaCtx.registerFunction("getSignature", [](const DNSCryptCert& cert) { return std::string(reinterpret_cast(cert.signature), sizeof(cert.signature)); }); - luaCtx.registerFunction("getResolverPublicKey", [](const DNSCryptCert& cert) { return std::string(reinterpret_cast(cert.signedData.resolverPK), sizeof(cert.signedData.resolverPK)); }); - luaCtx.registerFunction("getClientMagic", [](const DNSCryptCert& cert) { return std::string(reinterpret_cast(cert.signedData.clientMagic), sizeof(cert.signedData.clientMagic)); }); + luaCtx.registerFunction("getMagic", [](const DNSCryptCert& cert) { return std::string(reinterpret_cast(cert.magic.data()), cert.magic.size()); }); + luaCtx.registerFunction("getEsVersion", [](const DNSCryptCert& cert) { return std::string(reinterpret_cast(cert.esVersion.data()), cert.esVersion.size()); }); + luaCtx.registerFunction("getProtocolMinorVersion", [](const DNSCryptCert& cert) { return std::string(reinterpret_cast(cert.protocolMinorVersion.data()), cert.protocolMinorVersion.size()); }); + luaCtx.registerFunction("getSignature", [](const DNSCryptCert& cert) { return std::string(reinterpret_cast(cert.signature.data()), cert.signature.size()); }); + luaCtx.registerFunction("getResolverPublicKey", [](const DNSCryptCert& cert) { return std::string(reinterpret_cast(cert.signedData.resolverPK.data()), cert.signedData.resolverPK.size()); }); + luaCtx.registerFunction("getClientMagic", [](const DNSCryptCert& cert) { return std::string(reinterpret_cast(cert.signedData.clientMagic.data()), cert.signedData.clientMagic.size()); }); luaCtx.registerFunction("getSerial", [](const DNSCryptCert& cert) { return cert.getSerial(); }); luaCtx.registerFunction("getTSStart", [](const DNSCryptCert& cert) { return ntohl(cert.getTSStart()); }); luaCtx.registerFunction("getTSEnd", [](const DNSCryptCert& cert) { return ntohl(cert.getTSEnd()); }); @@ -177,19 +177,19 @@ void setupLuaBindingsDNSCrypt(LuaContext& luaCtx, bool client) if (client) { return; } - unsigned char publicKey[DNSCRYPT_PROVIDER_PUBLIC_KEY_SIZE]; - unsigned char privateKey[DNSCRYPT_PROVIDER_PRIVATE_KEY_SIZE]; - sodium_mlock(privateKey, sizeof(privateKey)); + DNSCryptCertSignedData::ResolverPublicKeyType publicKey; + DNSCryptCertSignedData::ResolverPrivateKeyType privateKey; + sodium_mlock(privateKey.data(), privateKey.size()); try { DNSCryptContext::generateProviderKeys(publicKey, privateKey); ofstream pubKStream(publicKeyFile); - pubKStream.write(reinterpret_cast(publicKey), sizeof(publicKey)); + pubKStream.write(reinterpret_cast(publicKey.data()), publicKey.size()); pubKStream.close(); ofstream privKStream(privateKeyFile); - privKStream.write(reinterpret_cast(privateKey), sizeof(privateKey)); + privKStream.write(reinterpret_cast(privateKey.data()), privateKey.size()); privKStream.close(); g_outputBuffer = "Provider fingerprint is: " + DNSCryptContext::getProviderFingerprint(publicKey) + "\n"; @@ -199,17 +199,17 @@ void setupLuaBindingsDNSCrypt(LuaContext& luaCtx, bool client) g_outputBuffer = "Error generating a DNSCrypt provider key: " + string(e.what()) + "\n"; } - sodium_memzero(privateKey, sizeof(privateKey)); - sodium_munlock(privateKey, sizeof(privateKey)); + sodium_memzero(privateKey.data(), privateKey.size()); + sodium_munlock(privateKey.data(), privateKey.size()); }); luaCtx.writeFunction("printDNSCryptProviderFingerprint", [](const std::string& publicKeyFile) { setLuaNoSideEffect(); - unsigned char publicKey[DNSCRYPT_PROVIDER_PUBLIC_KEY_SIZE]; + DNSCryptCertSignedData::ResolverPublicKeyType publicKey; try { ifstream file(publicKeyFile); - file.read(reinterpret_cast(&publicKey), sizeof(publicKey)); + file.read(reinterpret_cast(publicKey.data()), publicKey.size()); if (file.fail()) { throw std::runtime_error("Invalid dnscrypt provider public key file " + publicKeyFile); diff --git a/pdns/test-dnscrypt_cc.cc b/pdns/test-dnscrypt_cc.cc index d6d48f625c..4fd3aea30a 100644 --- a/pdns/test-dnscrypt_cc.cc +++ b/pdns/test-dnscrypt_cc.cc @@ -44,8 +44,8 @@ BOOST_AUTO_TEST_SUITE(test_dnscrypt_cc) BOOST_AUTO_TEST_CASE(DNSCryptPlaintextQuery) { DNSCryptPrivateKey resolverPrivateKey; DNSCryptCert resolverCert; - unsigned char providerPublicKey[DNSCRYPT_PROVIDER_PUBLIC_KEY_SIZE]; - unsigned char providerPrivateKey[DNSCRYPT_PROVIDER_PRIVATE_KEY_SIZE]; + DNSCryptCertSignedData::ResolverPublicKeyType providerPublicKey; + DNSCryptCertSignedData::ResolverPrivateKeyType providerPrivateKey; time_t now = time(nullptr); DNSCryptContext::generateProviderKeys(providerPublicKey, providerPrivateKey); DNSCryptContext::generateCertificate(1, now, now + (24 * 60 * 3600), DNSCryptExchangeVersion::VERSION1, providerPrivateKey, resolverPrivateKey, resolverCert); @@ -82,8 +82,8 @@ BOOST_AUTO_TEST_CASE(DNSCryptPlaintextQuery) { BOOST_AUTO_TEST_CASE(DNSCryptPlaintextQueryInvalidA) { DNSCryptPrivateKey resolverPrivateKey; DNSCryptCert resolverCert; - unsigned char providerPublicKey[DNSCRYPT_PROVIDER_PUBLIC_KEY_SIZE]; - unsigned char providerPrivateKey[DNSCRYPT_PROVIDER_PRIVATE_KEY_SIZE]; + DNSCryptCertSignedData::ResolverPublicKeyType providerPublicKey; + DNSCryptCertSignedData::ResolverPrivateKeyType providerPrivateKey; time_t now = time(nullptr); DNSCryptContext::generateProviderKeys(providerPublicKey, providerPrivateKey); DNSCryptContext::generateCertificate(1, now, now + (24 * 60 * 3600), DNSCryptExchangeVersion::VERSION1, providerPrivateKey, resolverPrivateKey, resolverCert); @@ -105,8 +105,8 @@ BOOST_AUTO_TEST_CASE(DNSCryptPlaintextQueryInvalidA) { BOOST_AUTO_TEST_CASE(DNSCryptPlaintextQueryInvalidProviderName) { DNSCryptPrivateKey resolverPrivateKey; DNSCryptCert resolverCert; - unsigned char providerPublicKey[DNSCRYPT_PROVIDER_PUBLIC_KEY_SIZE]; - unsigned char providerPrivateKey[DNSCRYPT_PROVIDER_PRIVATE_KEY_SIZE]; + DNSCryptCertSignedData::ResolverPublicKeyType providerPublicKey; + DNSCryptCertSignedData::ResolverPrivateKeyType providerPrivateKey; time_t now = time(nullptr); DNSCryptContext::generateProviderKeys(providerPublicKey, providerPrivateKey); DNSCryptContext::generateCertificate(1, now, now + (24 * 60 * 3600), DNSCryptExchangeVersion::VERSION1, providerPrivateKey, resolverPrivateKey, resolverCert); @@ -128,19 +128,18 @@ BOOST_AUTO_TEST_CASE(DNSCryptPlaintextQueryInvalidProviderName) { BOOST_AUTO_TEST_CASE(DNSCryptEncryptedQueryValid) { DNSCryptPrivateKey resolverPrivateKey; DNSCryptCert resolverCert; - unsigned char providerPublicKey[DNSCRYPT_PROVIDER_PUBLIC_KEY_SIZE]; - unsigned char providerPrivateKey[DNSCRYPT_PROVIDER_PRIVATE_KEY_SIZE]; + DNSCryptCertSignedData::ResolverPublicKeyType providerPublicKey; + DNSCryptCertSignedData::ResolverPrivateKeyType providerPrivateKey; time_t now = time(nullptr); DNSCryptContext::generateProviderKeys(providerPublicKey, providerPrivateKey); DNSCryptContext::generateCertificate(1, now, now + (24 * 60 * 3600), DNSCryptExchangeVersion::VERSION1, providerPrivateKey, resolverPrivateKey, resolverCert); auto ctx = std::make_shared("2.name", resolverCert, resolverPrivateKey); DNSCryptPrivateKey clientPrivateKey; - unsigned char clientPublicKey[DNSCRYPT_PUBLIC_KEY_SIZE]; - + DNSCryptPublicKeyType clientPublicKey; DNSCryptContext::generateResolverKeyPair(clientPrivateKey, clientPublicKey); - unsigned char clientNonce[DNSCRYPT_NONCE_SIZE / 2] = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x08, 0x09, 0x0A, 0x0B }; + DNSCryptClientNonceType clientNonce{ 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x08, 0x09, 0x0A, 0x0B }; DNSName name("www.powerdns.com."); PacketBuffer plainQuery; @@ -176,19 +175,19 @@ BOOST_AUTO_TEST_CASE(DNSCryptEncryptedQueryValid) { BOOST_AUTO_TEST_CASE(DNSCryptEncryptedQueryValidButShort) { DNSCryptPrivateKey resolverPrivateKey; DNSCryptCert resolverCert; - unsigned char providerPublicKey[DNSCRYPT_PROVIDER_PUBLIC_KEY_SIZE]; - unsigned char providerPrivateKey[DNSCRYPT_PROVIDER_PRIVATE_KEY_SIZE]; + DNSCryptCertSignedData::ResolverPublicKeyType providerPublicKey; + DNSCryptCertSignedData::ResolverPrivateKeyType providerPrivateKey; time_t now = time(nullptr); DNSCryptContext::generateProviderKeys(providerPublicKey, providerPrivateKey); DNSCryptContext::generateCertificate(1, now, now + (24 * 60 * 3600), DNSCryptExchangeVersion::VERSION1, providerPrivateKey, resolverPrivateKey, resolverCert); auto ctx = std::make_shared("2.name", resolverCert, resolverPrivateKey); DNSCryptPrivateKey clientPrivateKey; - unsigned char clientPublicKey[DNSCRYPT_PUBLIC_KEY_SIZE]; + DNSCryptCertSignedData::ResolverPublicKeyType clientPublicKey; DNSCryptContext::generateResolverKeyPair(clientPrivateKey, clientPublicKey); - unsigned char clientNonce[DNSCRYPT_NONCE_SIZE / 2] = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x08, 0x09, 0x0A, 0x0B }; + DNSCryptClientNonceType clientNonce{ 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x08, 0x09, 0x0A, 0x0B }; DNSName name("www.powerdns.com."); PacketBuffer plainQuery; @@ -203,19 +202,19 @@ BOOST_AUTO_TEST_CASE(DNSCryptEncryptedQueryValidButShort) { BOOST_AUTO_TEST_CASE(DNSCryptEncryptedQueryValidWithOldKey) { DNSCryptPrivateKey resolverPrivateKey; DNSCryptCert resolverCert; - unsigned char providerPublicKey[DNSCRYPT_PROVIDER_PUBLIC_KEY_SIZE]; - unsigned char providerPrivateKey[DNSCRYPT_PROVIDER_PRIVATE_KEY_SIZE]; + DNSCryptCertSignedData::ResolverPublicKeyType providerPublicKey; + DNSCryptCertSignedData::ResolverPrivateKeyType providerPrivateKey; time_t now = time(nullptr); DNSCryptContext::generateProviderKeys(providerPublicKey, providerPrivateKey); DNSCryptContext::generateCertificate(1, now, now + (24 * 60 * 3600), DNSCryptExchangeVersion::VERSION1, providerPrivateKey, resolverPrivateKey, resolverCert); auto ctx = std::make_shared("2.name", resolverCert, resolverPrivateKey); DNSCryptPrivateKey clientPrivateKey; - unsigned char clientPublicKey[DNSCRYPT_PUBLIC_KEY_SIZE]; + DNSCryptCertSignedData::ResolverPublicKeyType clientPublicKey; DNSCryptContext::generateResolverKeyPair(clientPrivateKey, clientPublicKey); - unsigned char clientNonce[DNSCRYPT_NONCE_SIZE / 2] = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x08, 0x09, 0x0A, 0x0B }; + DNSCryptClientNonceType clientNonce{ 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x08, 0x09, 0x0A, 0x0B }; DNSName name("www.powerdns.com."); PacketBuffer plainQuery; @@ -256,19 +255,19 @@ BOOST_AUTO_TEST_CASE(DNSCryptEncryptedQueryValidWithOldKey) { BOOST_AUTO_TEST_CASE(DNSCryptEncryptedQueryInvalidWithWrongKey) { DNSCryptPrivateKey resolverPrivateKey; DNSCryptCert resolverCert; - unsigned char providerPublicKey[DNSCRYPT_PROVIDER_PUBLIC_KEY_SIZE]; - unsigned char providerPrivateKey[DNSCRYPT_PROVIDER_PRIVATE_KEY_SIZE]; + DNSCryptCertSignedData::ResolverPublicKeyType providerPublicKey; + DNSCryptCertSignedData::ResolverPrivateKeyType providerPrivateKey; time_t now = time(nullptr); DNSCryptContext::generateProviderKeys(providerPublicKey, providerPrivateKey); DNSCryptContext::generateCertificate(1, now, now + (24 * 60 * 3600), DNSCryptExchangeVersion::VERSION1, providerPrivateKey, resolverPrivateKey, resolverCert); auto ctx = std::make_shared("2.name", resolverCert, resolverPrivateKey); DNSCryptPrivateKey clientPrivateKey; - unsigned char clientPublicKey[DNSCRYPT_PUBLIC_KEY_SIZE]; + DNSCryptCertSignedData::ResolverPublicKeyType clientPublicKey; DNSCryptContext::generateResolverKeyPair(clientPrivateKey, clientPublicKey); - unsigned char clientNonce[DNSCRYPT_NONCE_SIZE / 2] = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x08, 0x09, 0x0A, 0x0B }; + DNSCryptClientNonceType clientNonce{ 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x08, 0x09, 0x0A, 0x0B }; DNSName name("www.powerdns.com."); PacketBuffer plainQuery;