Skip to content

Commit

Permalink
dnsdist: Get rid of assert()
Browse files Browse the repository at this point in the history 
PowerDNS Security Advisory 2024-03 has made it clear that some of
them that have been designed to break during testing might break in
production, as we compile with `NDEBUG` unset.
  • Loading branch information
@rgacogne
rgacogne committed May 17, 2024
1 parent 6fe0cdf commit 14f437f
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 21 deletions.
37 changes: 26 additions & 11 deletions pdns/dnsdistdist/dnsdist-ecs.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ bool g_addEDNSToSelfGeneratedResponses{true};

int rewriteResponseWithoutEDNS(const PacketBuffer& initialPacket, PacketBuffer& newContent)
{
assert(initialPacket.size() >= sizeof(dnsheader));
if (initialPacket.size() < sizeof(dnsheader)) {
return ENOENT;
}

const dnsheader_aligned dnsHeader(initialPacket.data());

if (ntohs(dnsHeader->arcount) == 0) {
Expand Down Expand Up @@ -153,7 +156,10 @@ static bool addOrReplaceEDNSOption(std::vector<std::pair<uint16_t, std::string>>

bool slowRewriteEDNSOptionInQueryWithRecords(const PacketBuffer& initialPacket, PacketBuffer& newContent, bool& ednsAdded, uint16_t optionToReplace, bool& optionAdded, bool overrideExisting, const string& newOptionContent)
{
assert(initialPacket.size() >= sizeof(dnsheader));
if (initialPacket.size() < sizeof(dnsheader)) {
return false;
}

const dnsheader_aligned dnsHeader(initialPacket.data());

if (ntohs(dnsHeader->qdcount) == 0) {
Expand Down Expand Up @@ -321,9 +327,10 @@ static bool slowParseEDNSOptions(const PacketBuffer& packet, EDNSOptionViewMap&

int locateEDNSOptRR(const PacketBuffer& packet, uint16_t* optStart, size_t* optLen, bool* last)
{
assert(optStart != nullptr);
assert(optLen != nullptr);
assert(last != nullptr);
if (optStart == nullptr || optLen == nullptr || last == nullptr) {
throw std::runtime_error("Invalid values passed to locateEDNSOptRR");
}

const dnsheader_aligned dnsHeader(packet.data());

if (ntohs(dnsHeader->arcount) == 0) {
Expand Down Expand Up @@ -390,8 +397,10 @@ int locateEDNSOptRR(const PacketBuffer& packet, uint16_t* optStart, size_t* optL
/* extract the start of the OPT RR in a QUERY packet if any */
int getEDNSOptionsStart(const PacketBuffer& packet, const size_t offset, uint16_t* optRDPosition, size_t* remaining)
{
assert(optRDPosition != nullptr);
assert(remaining != nullptr);
if (optRDPosition == nullptr || remaining == nullptr) {
throw std::runtime_error("Invalid values passed to getEDNSOptionsStart");
}

const dnsheader_aligned dnsHeader(packet.data());

if (offset >= packet.size()) {
Expand Down Expand Up @@ -474,8 +483,9 @@ bool generateOptRR(const std::string& optRData, PacketBuffer& res, size_t maximu

static bool replaceEDNSClientSubnetOption(PacketBuffer& packet, size_t maximumSize, size_t const oldEcsOptionStartPosition, size_t const oldEcsOptionSize, size_t const optRDLenPosition, const string& newECSOption)
{
assert(oldEcsOptionStartPosition < packet.size());
assert(optRDLenPosition < packet.size());
if (oldEcsOptionStartPosition >= packet.size() || optRDLenPosition >= packet.size()) {
throw std::runtime_error("Invalid values passed to replaceEDNSClientSubnetOption");
}

if (newECSOption.size() == oldEcsOptionSize) {
/* same size as the existing option */
Expand Down Expand Up @@ -593,7 +603,9 @@ static bool addEDNSWithECS(PacketBuffer& packet, size_t maximumSize, const strin

bool handleEDNSClientSubnet(PacketBuffer& packet, const size_t maximumSize, const size_t qnameWireLength, bool& ednsAdded, bool& ecsAdded, bool overrideExisting, const string& newECSOption)
{
assert(qnameWireLength <= packet.size());
if (qnameWireLength > packet.size()) {
throw std::runtime_error("Invalid value passed to handleEDNSClientSubnet");
}

const dnsheader_aligned dnsHeader(packet.data());

Expand Down Expand Up @@ -763,7 +775,10 @@ bool isEDNSOptionInOpt(const PacketBuffer& packet, const size_t optStart, const

int rewriteResponseWithoutEDNSOption(const PacketBuffer& initialPacket, const uint16_t optionCodeToSkip, PacketBuffer& newContent)
{
assert(initialPacket.size() >= sizeof(dnsheader));
if (initialPacket.size() < sizeof(dnsheader)) {
return ENOENT;
}

const dnsheader_aligned dnsHeader(initialPacket.data());

if (ntohs(dnsHeader->arcount) == 0) {
Expand Down
14 changes: 8 additions & 6 deletions pdns/ednsoptions.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,14 @@ bool getNextEDNSOption(const char* data, size_t dataLen, uint16_t& optionCode, u
/* extract the position (relative to the optRR pointer!) and size of a specific EDNS0 option from a pointer on the beginning rdLen of the OPT RR */
int getEDNSOption(const char* optRR, const size_t len, uint16_t wantedOption, size_t* optionValuePosition, size_t * optionValueSize)
{
assert(optRR != nullptr);
assert(optionValuePosition != nullptr);
assert(optionValueSize != nullptr);
if (optRR == nullptr || optionValuePosition == nullptr || optionValueSize == nullptr) {
return EINVAL;
}

size_t pos = 0;
if (len < DNS_RDLENGTH_SIZE)
if (len < DNS_RDLENGTH_SIZE) {
return EINVAL;
}

const uint16_t rdLen = (((unsigned char) optRR[pos]) * 256) + ((unsigned char) optRR[pos+1]);
size_t rdPos = 0;
Expand Down Expand Up @@ -94,10 +96,10 @@ int getEDNSOption(const char* optRR, const size_t len, uint16_t wantedOption, si
/* extract all EDNS0 options from a pointer on the beginning rdLen of the OPT RR */
int getEDNSOptions(const char* optRR, const size_t len, EDNSOptionViewMap& options)
{
assert(optRR != nullptr);
size_t pos = 0;
if (len < DNS_RDLENGTH_SIZE)
if (optRR == nullptr || len < DNS_RDLENGTH_SIZE) {
return EINVAL;
}

const uint16_t rdLen = (((unsigned char) optRR[pos]) * 256) + ((unsigned char) optRR[pos+1]);
size_t rdPos = 0;
Expand Down
16 changes: 12 additions & 4 deletions pdns/ipcipher.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ static ComboAddress encryptCA6(const ComboAddress& address, const std::string& k
const auto inSize = sizeof(address.sin6.sin6_addr.s6_addr);
static_assert(inSize == 16, "We disable padding and so we must assume a data size of 16 bytes");
const auto blockSize = EVP_CIPHER_get_block_size(aes128cbc.get());
assert(blockSize == 16);
if (blockSize != 16) {
throw pdns::OpenSSL::error("encryptCA6: unexpected block size");
}
EVP_CIPHER_CTX_set_padding(ctx.get(), 0);

int updateLen = 0;
Expand All @@ -94,7 +96,9 @@ static ComboAddress encryptCA6(const ComboAddress& address, const std::string& k
throw pdns::OpenSSL::error("encryptCA6: Could not finalize address encryption");
}

assert(updateLen + finalLen == inSize);
if ((updateLen + finalLen) != inSize) {
throw pdns::OpenSSL::error("encryptCA6: unexpected final size");
}
#else
AES_KEY wctx;
AES_set_encrypt_key((const unsigned char*)key.c_str(), 128, &wctx);
Expand Down Expand Up @@ -130,7 +134,9 @@ static ComboAddress decryptCA6(const ComboAddress& address, const std::string& k
const auto inSize = sizeof(address.sin6.sin6_addr.s6_addr);
static_assert(inSize == 16, "We disable padding and so we must assume a data size of 16 bytes");
const auto blockSize = EVP_CIPHER_get_block_size(aes128cbc.get());
assert(blockSize == 16);
if (blockSize != 16) {
throw pdns::OpenSSL::error("decryptCA6: unexpected block size");
}
EVP_CIPHER_CTX_set_padding(ctx.get(), 0);

int updateLen = 0;
Expand All @@ -147,7 +153,9 @@ static ComboAddress decryptCA6(const ComboAddress& address, const std::string& k
throw pdns::OpenSSL::error("decryptCA6: Could not finalize address decryption");
}

assert(updateLen + finalLen == inSize);
if ((updateLen + finalLen) != inSize) {
throw pdns::OpenSSL::error("decryptCA6: unexpected final size");
}
#else
AES_KEY wctx;
AES_set_decrypt_key((const unsigned char*)key.c_str(), 128, &wctx);
Expand Down

0 comments on commit 14f437f

Please sign in to comment.