Skip to content

Commit 05126de

Browse files
miodvallatmiodvallat
authored
Merge pull request #14057 from mind04/auth-catalog-cleanup
Auth: fix a crash and some cleanup in the auth-catalogzone.cc
2 parents d98da9f + 88b55eb commit 05126de

5 files changed

Lines changed: 82 additions & 66 deletions

File tree

‎modules/lmdbbackend/lmdbbackend.cc‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2396,26 +2396,25 @@ void LMDBBackend::setLastCheckTime(domainid_t domain_id, time_t last_check)
23962396

23972397
void LMDBBackend::getUpdatedPrimaries(vector<DomainInfo>& updatedDomains, std::unordered_set<DNSName>& catalogs, CatalogHashMap& catalogHashes)
23982398
{
2399-
CatalogInfo ci;
2400-
2401-
getAllDomainsFiltered(&(updatedDomains), [this, &catalogs, &catalogHashes, &ci](DomainInfo& di) {
2399+
getAllDomainsFiltered(&(updatedDomains), [this, &catalogs, &catalogHashes](DomainInfo& di) {
24022400
if (!di.isPrimaryType()) {
24032401
return false;
24042402
}
24052403

24062404
if (di.kind == DomainInfo::Producer) {
24072405
catalogs.insert(di.zone.operator const DNSName&());
2408-
catalogHashes[di.zone].process("\0");
2406+
catalogHashes[di.zone].process("");
24092407
return false; // Producer freshness check is performed elsewhere
24102408
}
24112409

24122410
if (!di.catalog.empty()) {
2413-
ci.fromJson(di.options, CatalogInfo::CatalogType::Producer);
2414-
ci.updateHash(catalogHashes, di);
2411+
CatalogInfo::updateCatalogHash(catalogHashes, di);
24152412
}
24162413

24172414
if (getSerial(di) && di.serial != di.notified_serial) {
24182415
di.backend = this;
2416+
di.catalog.clear();
2417+
di.options.clear();
24192418
return true;
24202419
}
24212420

‎pdns/auth-catalogzone.cc‎

Lines changed: 56 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -25,56 +25,58 @@
2525
#endif
2626

2727
#include "dnsbackend.hh"
28+
#include "json.hh"
2829

29-
void CatalogInfo::fromJson(const std::string& json, CatalogType type)
30+
bool CatalogInfo::parseJson(const std::string& json, CatalogType type)
3031
{
31-
d_type = type;
32-
if (d_type == CatalogType::None) {
32+
if (type == CatalogType::None) {
3333
throw std::runtime_error("CatalogType is set to None");
3434
}
35+
36+
d_type = type;
37+
3538
if (json.empty()) {
36-
return;
39+
d_doc = nullptr;
40+
return false;
3741
}
42+
3843
std::string err;
3944
d_doc = json11::Json::parse(json, err);
40-
if (!d_doc.is_null()) {
41-
if (!d_doc[getTypeString(d_type)].is_null()) {
42-
auto items = d_doc[getTypeString(type)].object_items();
43-
if (!items["coo"].is_null()) {
44-
if (items["coo"].is_string()) {
45-
if (!items["coo"].string_value().empty()) {
46-
d_coo = DNSName(items["coo"].string_value());
47-
}
48-
}
49-
else {
50-
throw std::out_of_range("Key 'coo' is not a string");
51-
}
45+
if (d_doc.is_null()) {
46+
throw std::runtime_error("Parsing of JSON options failed: " + err);
47+
}
48+
49+
return !d_doc[getTypeString(d_type)].is_null();
50+
}
51+
52+
void CatalogInfo::fromJson(const std::string& json, CatalogType type)
53+
{
54+
if (parseJson(json, type)) {
55+
auto items = d_doc[getTypeString(type)].object_items();
56+
57+
// coo property
58+
if (!items["coo"].is_null()) {
59+
d_coo = DNSName(stringFromJson(items, "coo"));
60+
}
61+
62+
// unique property
63+
if (!items["unique"].is_null()) {
64+
d_unique = DNSName(stringFromJson(items, "unique"));
65+
if (d_unique.countLabels() != 1) {
66+
throw std::out_of_range("Invalid number of labels in unique value");
5267
}
53-
if (!items["unique"].is_null()) {
54-
if (items["unique"].is_string()) {
55-
if (!items["unique"].string_value().empty()) {
56-
d_unique = DNSName(items["unique"].string_value());
57-
}
58-
}
59-
else {
60-
throw std::out_of_range("Key 'unique' is not a string");
61-
}
68+
}
69+
70+
// group properties
71+
if (!items["group"].is_null()) {
72+
if (!items["group"].is_array()) {
73+
throw std::out_of_range("Key 'group' is not an array");
6274
}
63-
if (!items["group"].is_null()) {
64-
if (items["group"].is_array()) {
65-
for (const auto& value : items["group"].array_items()) {
66-
d_group.insert(value.string_value());
67-
}
68-
}
69-
else {
70-
throw std::out_of_range("Key 'group' is not an array");
71-
}
75+
for (const auto& value : items["group"].array_items()) {
76+
d_group.insert(value.string_value());
7277
}
7378
}
7479
}
75-
else {
76-
throw std::runtime_error("Parsing of JSON options failed: " + err);
77-
}
7880
}
7981

8082
std::string CatalogInfo::toJson() const
@@ -83,41 +85,48 @@ std::string CatalogInfo::toJson() const
8385
throw std::runtime_error("CatalogType is set to None");
8486
}
8587
json11::Json::object object;
88+
89+
// coo property
8690
if (!d_coo.empty()) {
8791
object["coo"] = d_coo.toString();
8892
}
93+
94+
// unique property
8995
if (!d_unique.empty()) {
90-
if (d_unique.countLabels() > 1) {
91-
throw std::out_of_range("Multiple labels in a unique value are not allowed");
96+
if (d_unique.countLabels() != 1) {
97+
throw std::out_of_range("Invalid number of labels in unique value");
9298
}
9399
object["unique"] = d_unique.toString();
94100
}
101+
102+
// group properties
95103
if (!d_group.empty()) {
96104
json11::Json::array entries;
97105
for (const auto& group : d_group) {
98106
entries.push_back(group);
99107
}
100108
object["group"] = entries;
101109
}
110+
102111
auto tmp = d_doc.object_items();
103112
tmp[getTypeString(d_type)] = object;
104113
const json11::Json ret = tmp;
105114
return ret.dump();
106115
}
107116

108-
void CatalogInfo::updateHash(CatalogHashMap& hashes, const DomainInfo& di) const
117+
void CatalogInfo::updateCatalogHash(CatalogHashMap& hashes, const DomainInfo& di)
109118
{
110-
hashes[di.catalog].process(std::to_string(di.id) + di.zone.toLogString() + string("\0", 1) + d_coo.toLogString() + string("\0", 1) + d_unique.toLogString());
111-
for (const auto& group : d_group) {
112-
hashes[di.catalog].process(std::to_string(group.length()) + group);
119+
CatalogInfo ci;
120+
hashes[di.catalog].process(std::to_string(di.id) + di.zone.toLogString());
121+
if (ci.parseJson(di.options, CatalogType::Producer)) {
122+
hashes[di.catalog].process(ci.d_doc["producer"].dump());
113123
}
114124
}
115125

116126
DNSZoneRecord CatalogInfo::getCatalogVersionRecord(const ZoneName& zone)
117127
{
118128
DNSZoneRecord dzr;
119129
dzr.dr.d_name = g_versiondnsname + zone.operator const DNSName&();
120-
dzr.dr.d_ttl = 0;
121130
dzr.dr.d_type = QType::TXT;
122131
dzr.dr.setContent(std::make_shared<TXTRecordContent>("2"));
123132
return dzr;
@@ -134,24 +143,24 @@ void CatalogInfo::toDNSZoneRecords(const ZoneName& zone, vector<DNSZoneRecord>&
134143
}
135144
prefix += g_zonesdnsname + zone.operator const DNSName&();
136145

146+
// member zone
137147
DNSZoneRecord dzr;
138148
dzr.dr.d_name = prefix;
139-
dzr.dr.d_ttl = 0;
140149
dzr.dr.d_type = QType::PTR;
141150
dzr.dr.setContent(std::make_shared<PTRRecordContent>(d_zone.operator const DNSName&().toString()));
142151
dzrs.emplace_back(dzr);
143152

153+
// coo property
144154
if (!d_coo.empty()) {
145155
dzr.dr.d_name = g_coodnsname + prefix;
146-
dzr.dr.d_ttl = 0;
147156
dzr.dr.d_type = QType::PTR;
148157
dzr.dr.setContent(std::make_shared<PTRRecordContent>(d_coo));
149158
dzrs.emplace_back(dzr);
150159
}
151160

161+
// group properties
152162
for (const auto& group : d_group) {
153163
dzr.dr.d_name = g_groupdnsname + prefix;
154-
dzr.dr.d_ttl = 0;
155164
dzr.dr.d_type = QType::TXT;
156165
dzr.dr.setContent(std::make_shared<TXTRecordContent>("\"" + group + "\""));
157166
dzrs.emplace_back(dzr);

‎pdns/auth-catalogzone.hh‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public:
5959
std::string toJson() const;
6060
void setType(CatalogType type) { d_type = type; }
6161

62-
void updateHash(CatalogHashMap& hashes, const DomainInfo& di) const;
62+
static void updateCatalogHash(CatalogHashMap& hashes, const DomainInfo& di);
6363
DNSName getUnique() const { return DNSName(toBase32Hex(hashQNameWithSalt(std::to_string(d_id), 0, DNSName(d_zone)))); } // salt with domain id to detect recreated zones
6464
static DNSZoneRecord getCatalogVersionRecord(const ZoneName& zone);
6565
void toDNSZoneRecords(const ZoneName& zone, vector<DNSZoneRecord>& dzrs) const;
@@ -78,4 +78,6 @@ public:
7878
private:
7979
CatalogType d_type;
8080
json11::Json d_doc;
81+
82+
bool parseJson(const std::string& json, CatalogType type);
8183
};

‎pdns/auth-primarycommunicator.cc‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -173,17 +173,22 @@ void CommunicatorClass::getUpdatedProducers(UeberBackend* B, vector<DomainInfo>&
173173
continue;
174174
}
175175

176-
B->setDomainMetadata(di.zone, "CATALOG-HASH", mapHash);
176+
SOAData sd;
177+
try {
178+
if (!B->getSOAUncached(di.zone, sd)) {
179+
SLOG(g_log << Logger::Warning << "SOA lookup failed for producer zone '" << di.zone << "'" << endl,
180+
d_slog->info(Logr::Warning, "SOA lookup failed for producer zone", "zone", Logging::Loggable(di.zone)));
181+
continue;
182+
}
183+
}
184+
catch (...) {
185+
continue;
186+
}
177187

178188
SLOG(g_log << Logger::Warning << "new CATALOG-HASH '" << mapHash << "' for zone '" << di.zone << "'" << endl,
179189
d_slog->info(Logr::Warning, "new CATALOG-HASH for zone", "zone", Logging::Loggable(di.zone), "hash", Logging::Loggable(mapHash)));
180190

181-
SOAData sd;
182-
if (!B->getSOAUncached(di.zone, sd)) {
183-
SLOG(g_log << Logger::Warning << "SOA lookup failed for producer zone '" << di.zone << "'" << endl,
184-
d_slog->info(Logr::Warning, "SOA lookup failed for producer zone", "zone", Logging::Loggable(di.zone)));
185-
continue;
186-
}
191+
B->setDomainMetadata(di.zone, "CATALOG-HASH", mapHash);
187192

188193
DNSResourceRecord rr;
189194
makeIncreasedSOARecord(sd, "EPOCH", "", rr, d_slog);

‎pdns/backends/gsql/gsqlbackend.cc‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -474,8 +474,8 @@ void GSQLBackend::getUnfreshSecondaryInfos(vector<DomainInfo>* unfreshDomains)
474474
continue;
475475
}
476476
catch (...) {
477-
SLOG(g_log << Logger::Warning << __PRETTY_FUNCTION__ << " error while parsing SOA data for zone '" << di.zone << endl,
478-
d_slog->info(Logr::Warning, "error while parsing SOA data", "zone", Logging::Loggable(di.zone)));
477+
SLOG(g_log << Logger::Warning << __PRETTY_FUNCTION__ << " error while parsing SOA data for zone '" << di.zone << "'" << endl,
478+
d_slog->info(Logr::Warning, "error while parsing SOA data", "zone", Logging::Loggable(di.zone)));
479479
continue;
480480
}
481481

@@ -606,7 +606,7 @@ void GSQLBackend::getUpdatedPrimaries(vector<DomainInfo>& updatedDomains, std::u
606606

607607
if (pdns_iequals(row[2], "PRODUCER")) {
608608
catalogs.insert(di.zone.operator const DNSName&());
609-
catalogHashes[di.zone].process("\0");
609+
catalogHashes[di.zone].process("");
610610
continue; // Producer freshness check is performed elsewhere
611611
}
612612
else if (!pdns_iequals(row[2], "MASTER")) {
@@ -616,8 +616,8 @@ void GSQLBackend::getUpdatedPrimaries(vector<DomainInfo>& updatedDomains, std::u
616616

617617
try {
618618
if (!row[5].empty()) {
619-
ci.fromJson(row[4], CatalogInfo::CatalogType::Producer);
620-
ci.updateHash(catalogHashes, di);
619+
di.options = row[4];
620+
CatalogInfo::updateCatalogHash(catalogHashes, di);
621621
}
622622
}
623623
catch (const std::exception& e) {
@@ -653,6 +653,7 @@ void GSQLBackend::getUpdatedPrimaries(vector<DomainInfo>& updatedDomains, std::u
653653
di.kind = DomainInfo::Primary;
654654
di.serial = sd.serial;
655655
di.catalog.clear();
656+
di.options.clear();
656657

657658
updatedDomains.emplace_back(di);
658659
}

0 commit comments

Comments
 (0)