Skip to content

Commit

Permalink
Merge pull request #15036 from rgacogne/ddist-improve-secpoll-msgs
Browse files Browse the repository at this point in the history 
dnsdist: Improve error messages on security polling failures
  • Loading branch information
@rgacogne
rgacogne authored Jan 14, 2025
2 parents 72f679d + df8e71e commit 5557d8c
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 45 deletions.
1 change: 0 additions & 1 deletion .not-formatted
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
./pdns/dnsdistdist/dnsdist-lua-web.cc
./pdns/dnsdistdist/dnsdist-prometheus.hh
./pdns/dnsdistdist/dnsdist-rules.hh
./pdns/dnsdistdist/dnsdist-secpoll.cc
./pdns/dnsdistdist/dnsdist-systemd.cc
./pdns/dnsdistdist/dnsdist-tcp-downstream.cc
./pdns/dnsdistdist/dnsdist-tcp-downstream.hh
Expand Down
90 changes: 46 additions & 44 deletions pdns/dnsdistdist/dnsdist-secpoll.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,59 +49,61 @@ static std::string getFirstTXTAnswer(const std::string& answer)
throw std::runtime_error("Looking for a TXT record in an answer smaller than the DNS header");
}

const dnsheader_aligned dh(answer.data());
PacketReader pr(answer);
uint16_t qdcount = ntohs(dh->qdcount);
uint16_t ancount = ntohs(dh->ancount);
const dnsheader_aligned dnsHeader(answer.data());
PacketReader reader(answer);
uint16_t qdcount = ntohs(dnsHeader->qdcount);
uint16_t ancount = ntohs(dnsHeader->ancount);

DNSName rrname;
uint16_t rrtype;
uint16_t rrclass;
uint16_t rrtype{};
uint16_t rrclass{};

size_t idx = 0;
/* consume qd */
for(; idx < qdcount; idx++) {
rrname = pr.getName();
rrtype = pr.get16BitInt();
rrclass = pr.get16BitInt();
(void) rrtype;
(void) rrclass;
for (; idx < qdcount; idx++) {
rrname = reader.getName();
rrtype = reader.get16BitInt();
rrclass = reader.get16BitInt();
(void)rrtype;
(void)rrclass;
}

/* parse AN */
for (idx = 0; idx < ancount; idx++) {
string blob;
struct dnsrecordheader ah;
rrname = pr.getName();
pr.getDnsrecordheader(ah);
dnsrecordheader answerHeader{};
rrname = reader.getName();
reader.getDnsrecordheader(answerHeader);

if (ah.d_type == QType::TXT) {
if (answerHeader.d_type == QType::TXT) {
string txt;
pr.xfrText(txt);
reader.xfrText(txt);

return txt;
}
else {
pr.xfrBlob(blob);
}
reader.xfrBlob(blob);
}

throw std::runtime_error("No TXT record in answer");
}

static std::string getSecPollStatus(const std::string& queriedName, int timeout=2)
static std::string getSecPollStatus(const std::string& queriedName, int timeout = 2)
{
const auto verbose = dnsdist::configuration::getCurrentRuntimeConfiguration().d_verbose;

const DNSName sentName(queriedName);
std::vector<uint8_t> packet;
DNSPacketWriter pw(packet, sentName, QType::TXT);
pw.getHeader()->id = dnsdist::getRandomDNSID();
pw.getHeader()->rd = 1;
DNSPacketWriter writer(packet, sentName, QType::TXT);
writer.getHeader()->id = dnsdist::getRandomDNSID();
writer.getHeader()->rd = 1;

const auto& resolversForStub = getResolvers("/etc/resolv.conf");

for(const auto& dest : resolversForStub) {
if (resolversForStub.empty()) {
throw std::runtime_error("No resolver to query to check for Security Status update");
}

for (const auto& dest : resolversForStub) {
Socket sock(dest.sin4.sin_family, SOCK_DGRAM);
sock.setNonBlocking();
sock.connect(dest);
Expand All @@ -115,7 +117,7 @@ static std::string getSecPollStatus(const std::string& queriedName, int timeout=
}
continue;
}
else if (ret == 0) {
if (ret == 0) {
if (verbose) {
warnlog("Timeout while waiting for the secpoll response from stub resolver %s", dest.toString());
}
Expand All @@ -125,9 +127,9 @@ static std::string getSecPollStatus(const std::string& queriedName, int timeout=
try {
sock.read(reply);
}
catch(const std::exception& e) {
catch (const std::exception& exp) {
if (verbose) {
warnlog("Error while reading for the secpoll response from stub resolver %s: %s", dest.toString(), e.what());
warnlog("Error while reading for the secpoll response from stub resolver %s: %s", dest.toString(), exp.what());
}
continue;
}
Expand All @@ -139,36 +141,36 @@ static std::string getSecPollStatus(const std::string& queriedName, int timeout=
continue;
}

struct dnsheader d;
memcpy(&d, reply.c_str(), sizeof(d));
if (d.id != pw.getHeader()->id) {
dnsheader dnsHeader{};
memcpy(&dnsHeader, reply.c_str(), sizeof(dnsHeader));
if (dnsHeader.id != writer.getHeader()->id) {
if (verbose) {
warnlog("Invalid ID (%d / %d) received from the secpoll stub resolver %s", d.id, pw.getHeader()->id, dest.toString());
warnlog("Invalid ID (%d / %d) received from the secpoll stub resolver %s", dnsHeader.id, writer.getHeader()->id, dest.toString());
}
continue;
}

if (d.rcode != RCode::NoError) {
if (dnsHeader.rcode != RCode::NoError) {
if (verbose) {
warnlog("Response code '%s' received from the secpoll stub resolver %s for '%s'", RCode::to_s(d.rcode), dest.toString(), queriedName);
warnlog("Response code '%s' received from the secpoll stub resolver %s for '%s'", RCode::to_s(dnsHeader.rcode), dest.toString(), queriedName);
}

/* no need to try another resolver if the domain does not exist */
if (d.rcode == RCode::NXDomain) {
throw std::runtime_error("Unable to get a valid Security Status update");
if (dnsHeader.rcode == RCode::NXDomain) {
throw std::runtime_error("Unable to get a valid Security Status update, domain does not exist");
}
continue;
}

if (ntohs(d.qdcount) != 1 || ntohs(d.ancount) != 1) {
if (ntohs(dnsHeader.qdcount) != 1 || ntohs(dnsHeader.ancount) != 1) {
if (verbose) {
warnlog("Invalid answer (qdcount %d / ancount %d) received from the secpoll stub resolver %s", ntohs(d.qdcount), ntohs(d.ancount), dest.toString());
warnlog("Invalid answer (qdcount %d / ancount %d) received from the secpoll stub resolver %s", ntohs(dnsHeader.qdcount), ntohs(dnsHeader.ancount), dest.toString());
}
continue;
}

uint16_t receivedType;
uint16_t receivedClass;
uint16_t receivedType{};
uint16_t receivedClass{};
DNSName receivedName(reply.c_str(), reply.size(), sizeof(dnsheader), false, &receivedType, &receivedClass);

if (receivedName != sentName || receivedType != QType::TXT || receivedClass != QClass::IN) {
Expand Down Expand Up @@ -219,20 +221,20 @@ void doSecPoll(const std::string& suffix)
if (securityStatus == 2) {
errlog("PowerDNS DNSDist Security Update Recommended: %s", securityMessage);
}
else if(securityStatus == 3) {
else if (securityStatus == 3) {
errlog("PowerDNS DNSDist Security Update Mandatory: %s", securityMessage);
}

dnsdist::metrics::g_stats.securityStatus = securityStatus;
s_secPollDone = true;
return;
}
catch (const std::exception& e) {
catch (const std::exception& exp) {
if (releaseVersion) {
warnlog("Error while retrieving the security update for version %s: %s", version, e.what());
warnlog("Error while retrieving the security update for version %s: %s", version, exp.what());
}
else if (!s_secPollDone) {
infolog("Error while retrieving the security update for version %s: %s", version, e.what());
infolog("Error while retrieving the security update for version %s: %s", version, exp.what());
}
}

Expand Down

0 comments on commit 5557d8c

Please sign in to comment.