From: Miod Vallat Date: Wed, 2 Apr 2025 10:58:36 +0000 (+0200) Subject: Make pdnsutil add-record use the same checks as the API. X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b5967ede5bb1b7294e8561e4408b8906ba934b66;p=thirdparty%2Fpdns.git Make pdnsutil add-record use the same checks as the API. --- diff --git a/meson.build b/meson.build index f22de4e27..a7cdbed9a 100644 --- a/meson.build +++ b/meson.build @@ -497,6 +497,8 @@ common_sources += files( src_dir / 'bindparserclasses.hh', src_dir / 'burtle.hh', src_dir / 'cachecleaner.hh', + src_dir / 'check-zone.cc', + src_dir / 'check-zone.hh', src_dir / 'circular_buffer.hh', src_dir / 'comment.hh', src_dir / 'communicator.cc', diff --git a/pdns/Makefile.am b/pdns/Makefile.am index 8dd8e5c3f..ac6c0c336 100644 --- a/pdns/Makefile.am +++ b/pdns/Makefile.am @@ -204,6 +204,7 @@ pdns_server_SOURCES = \ bindparser.cc \ burtle.hh \ cachecleaner.hh \ + check-zone.cc check-zone.hh \ circular_buffer.hh \ comment.hh \ communicator.cc communicator.hh \ @@ -344,6 +345,7 @@ pdnsutil_SOURCES = \ bindlexer.l \ bindparser.yy \ cachecleaner.hh \ + check-zone.cc check-zone.hh \ circular_buffer.hh \ credentials.cc credentials.hh \ dbdnsseckeeper.cc \ diff --git a/pdns/check-zone.cc b/pdns/check-zone.cc new file mode 100644 index 000000000..f23a58d92 --- /dev/null +++ b/pdns/check-zone.cc @@ -0,0 +1,88 @@ +/* + * This file is part of PowerDNS or dnsdist. + * Copyright -- PowerDNS.COM B.V. and its contributors + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of version 2 of the GNU General Public License as + * published by the Free Software Foundation. + * + * In addition, for the avoidance of any doubt, permission is granted to + * link this program with OpenSSL and to (re)distribute the binaries + * produced as the result of such linking. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "dns.hh" +#include "dnsrecords.hh" + +#include "check-zone.hh" + +namespace Check +{ + +static void rejectRecord(const DNSResourceRecord& rec, const std::string& why) +{ + throw CheckException("RRset " + rec.qname.toString() + " IN " + rec.qtype.toString() + why); +} + +void checkRRSet(vector& records, const ZoneName& zone) +{ + // QTypes that MUST NOT have multiple records of the same type in a given RRset. + static const std::set onlyOneEntryTypes = {QType::CNAME, QType::DNAME, QType::SOA}; + // QTypes that MUST be at apex. + static const std::set atApexTypes = {QType::SOA, QType::DNSKEY}; + // QTypes that are NOT allowed at apex. + static const std::set nonApexTypes = {QType::DS}; + + sort(records.begin(), records.end(), + [](const DNSResourceRecord& rec_a, const DNSResourceRecord& rec_b) -> bool { + /* we need _strict_ weak ordering */ + return std::tie(rec_a.qname, rec_a.qtype, rec_a.content) < std::tie(rec_b.qname, rec_b.qtype, rec_b.content); + }); + + DNSResourceRecord previous; + for (const auto& rec : records) { + if (previous.qname == rec.qname) { + if (previous.qtype == rec.qtype) { + if (onlyOneEntryTypes.count(rec.qtype.getCode()) != 0) { + rejectRecord(rec, " has more than one record"); + } + if (previous.content == rec.content) { + throw CheckException("Duplicate record in RRset " + rec.qname.toString() + " IN " + rec.qtype.toString() + " with content \"" + rec.content + "\""); + } + } + else if (QType::exclusiveEntryTypes.count(rec.qtype.getCode()) != 0 || QType::exclusiveEntryTypes.count(previous.qtype.getCode()) != 0) { + rejectRecord(rec, ": Conflicts with another RRset"); + } + } + + if (rec.qname == zone.operator const DNSName&()) { + if (nonApexTypes.count(rec.qtype.getCode()) != 0) { + rejectRecord(rec, " is not allowed at apex"); + } + } + else if (atApexTypes.count(rec.qtype.getCode()) != 0) { + rejectRecord(rec, " is only allowed at apex"); + } + + // Check if the DNSNames that should be hostnames, are hostnames + try { + checkHostnameCorrectness(rec); + } + catch (const std::exception& e) { + rejectRecord(rec, std::string{": "} + e.what()); + } + + previous = rec; + } +} + +} // namespace Check diff --git a/pdns/check-zone.hh b/pdns/check-zone.hh new file mode 100644 index 000000000..4bbcac4d2 --- /dev/null +++ b/pdns/check-zone.hh @@ -0,0 +1,43 @@ +/* + * This file is part of PowerDNS or dnsdist. + * Copyright -- PowerDNS.COM B.V. and its contributors + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of version 2 of the GNU General Public License as + * published by the Free Software Foundation. + * + * In addition, for the avoidance of any doubt, permission is granted to + * link this program with OpenSSL and to (re)distribute the binaries + * produced as the result of such linking. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +class CheckException : public runtime_error +{ +public: + CheckException(const string& what_arg) : + runtime_error(what_arg) {} +}; + +namespace Check +{ + +/** Throws CheckException if records which violate RRset constraints are present. + * NOTE: sorts records in-place. + * + * Constraints being checked: + * *) no exact duplicates + * *) no duplicates for QTypes that can only be present once per RRset + * *) hostnames are hostnames + */ +void checkRRSet(vector& records, const ZoneName& zone); + +} // namespace Check diff --git a/pdns/pdnsutil.cc b/pdns/pdnsutil.cc index 4723e596d..54cae5e4b 100644 --- a/pdns/pdnsutil.cc +++ b/pdns/pdnsutil.cc @@ -37,6 +37,7 @@ #include "ipcipher.hh" #include "misc.hh" #include "zonemd.hh" +#include "check-zone.hh" #include #include #include @@ -2484,7 +2485,8 @@ static int createZone(const ZoneName &zone, const DNSName& nsname) { } // add-record ZONE name type [ttl] "content" ["content"] -static int addOrReplaceRecord(bool isAdd, const vector& cmds) { +static int addOrReplaceRecord(bool isAdd, const vector& cmds) +{ DNSResourceRecord rr; vector newrrs; ZoneName zone(cmds.at(0)); @@ -2519,44 +2521,6 @@ static int addOrReplaceRecord(bool isAdd, const vector& cmds) { } } - di.backend->startTransaction(zone, UnknownDomainID); - - // Enforce that CNAME records can not be mixed with any other. - // If we add a CNAME: there should be no existing records except for one - // possible previous CNAME, which will get replaced. - // If we add something else: there should be no existing CNAME record. - bool reject{false}; - if (rr.qtype.getCode() == QType::CNAME) { - di.backend->lookup(QType(QType::ANY), rr.qname, static_cast(di.id)); - while (di.backend->get(oldrr)) { - if (oldrr.qtype.getCode() == QType::CNAME) { - if (not isAdd) { - // Replacement operation: ok to replace CNAME with another - continue; - } - } - reject = true; - di.backend->lookupEnd(); - break; - } - if (reject) { - cerr<<"Attempting to add CNAME to "<lookup(QType(QType::CNAME), rr.qname, static_cast(di.id)); - while (di.backend->get(oldrr)) { - reject = true; - di.backend->lookupEnd(); - break; - } - if (reject) { - cerr<<"Attempting to add record to "<getZoneRepresentation(true); @@ -2574,11 +2538,13 @@ static int addOrReplaceRecord(bool isAdd, const vector& cmds) { } } - if(isAdd) { + di.backend->startTransaction(zone, UnknownDomainID); + + if (isAdd) { // the 'add' case; preserve existing records, making sure to discard // would-be new records which contents are identical to the existing ones. vector oldrrs; - di.backend->lookup(rr.qtype, rr.qname, static_cast(di.id)); + di.backend->lookup(QType(QType::ANY), rr.qname, static_cast(di.id)); while (di.backend->get(oldrr)) { oldrrs.push_back(oldrr); for (auto iter = newrrs.begin(); iter != newrrs.end(); ++iter) { @@ -2591,6 +2557,23 @@ static int addOrReplaceRecord(bool isAdd, const vector& cmds) { oldrrs.insert(oldrrs.end(), newrrs.begin(), newrrs.end()); newrrs = std::move(oldrrs); } + + try { + Check::checkRRSet(newrrs, zone); + } + catch (CheckException& e) { + cerr << e.what() << endl; + return EXIT_FAILURE; + } + if (isAdd) { + // We had collected all record types earlier in order to be able to + // perform the proper checks. Trim the list to only keep those of the + // qtype we are modifying, for the sake of the replaceRRSet call below. + newrrs.erase( + std::remove_if(newrrs.begin(), newrrs.end(), + [&rr](const DNSResourceRecord& rec) -> bool { return rec.qtype != rr.qtype; }), + newrrs.end()); + } else { cout<<"All existing records for "< QType::exclusiveEntryTypes = { + QType::CNAME +}; diff --git a/pdns/qtype.hh b/pdns/qtype.hh index dd2761a65..d03b3963e 100644 --- a/pdns/qtype.hh +++ b/pdns/qtype.hh @@ -151,6 +151,9 @@ public: const static map names; const static map numbers; + // QTypes that MUST NOT be used with any other QType on the same name. + const static std::set exclusiveEntryTypes; + private: uint16_t code; diff --git a/pdns/ws-auth.cc b/pdns/ws-auth.cc index 9585e0c17..d83ccc33f 100644 --- a/pdns/ws-auth.cc +++ b/pdns/ws-auth.cc @@ -53,6 +53,7 @@ #include "auth-zonecache.hh" #include "threadname.hh" #include "tsigutils.hh" +#include "check-zone.hh" using json11::Json; @@ -94,15 +95,6 @@ double Ewma::getMax() const static void patchZone(UeberBackend& backend, const ZoneName& zonename, DomainInfo& domainInfo, const vector& rrsets, HttpResponse* resp); -// QTypes that MUST NOT have multiple records of the same type in a given RRset. -static const std::set onlyOneEntryTypes = {QType::CNAME, QType::DNAME, QType::SOA}; -// QTypes that MUST NOT be used with any other QType on the same name. -static const std::set exclusiveEntryTypes = {QType::CNAME}; -// QTypes that MUST be at apex. -static const std::set atApexTypes = {QType::SOA, QType::DNSKEY}; -// QTypes that are NOT allowed at apex. -static const std::set nonApexTypes = {QType::DS}; - AuthWebServer::AuthWebServer() : d_start(time(nullptr)) @@ -1673,46 +1665,11 @@ static void gatherRecordsFromZone(const std::string& zonestring, vector& records, const ZoneName& zone) { - sort(records.begin(), records.end(), - [](const DNSResourceRecord& rec_a, const DNSResourceRecord& rec_b) -> bool { - /* we need _strict_ weak ordering */ - return std::tie(rec_a.qname, rec_a.qtype, rec_a.content) < std::tie(rec_b.qname, rec_b.qtype, rec_b.content); - }); - - DNSResourceRecord previous; - for (const auto& rec : records) { - if (previous.qname == rec.qname) { - if (previous.qtype == rec.qtype) { - if (onlyOneEntryTypes.count(rec.qtype.getCode()) != 0) { - throw ApiException("RRset " + rec.qname.toString() + " IN " + rec.qtype.toString() + " has more than one record"); - } - if (previous.content == rec.content) { - throw ApiException("Duplicate record in RRset " + rec.qname.toString() + " IN " + rec.qtype.toString() + " with content \"" + rec.content + "\""); - } - } - else if (exclusiveEntryTypes.count(rec.qtype.getCode()) != 0 || exclusiveEntryTypes.count(previous.qtype.getCode()) != 0) { - throw ApiException("RRset " + rec.qname.toString() + " IN " + rec.qtype.toString() + ": Conflicts with another RRset"); - } - } - - if (rec.qname == zone.operator const DNSName&()) { - if (nonApexTypes.count(rec.qtype.getCode()) != 0) { - throw ApiException("Record " + rec.qname.toString() + " IN " + rec.qtype.toString() + " is not allowed at apex"); - } - } - else if (atApexTypes.count(rec.qtype.getCode()) != 0) { - throw ApiException("Record " + rec.qname.toString() + " IN " + rec.qtype.toString() + " is only allowed at apex"); - } - - // Check if the DNSNames that should be hostnames, are hostnames - try { - checkHostnameCorrectness(rec); - } - catch (const std::exception& e) { - throw ApiException("RRset " + rec.qname.toString() + " IN " + rec.qtype.toString() + ": " + e.what()); - } - - previous = rec; + try { + Check::checkRRSet(records, zone); + } + catch (CheckException& e) { + throw ApiException(e.what()); } } @@ -2426,8 +2383,8 @@ static void replaceZoneRecords(DomainInfo& domainInfo, const ZoneName& zonename, dname_seen |= resourceRecord.qtype == QType::DNAME; ns_seen |= resourceRecord.qtype == QType::NS; if (qtype.getCode() != resourceRecord.qtype.getCode() - && (exclusiveEntryTypes.count(qtype.getCode()) != 0 - || exclusiveEntryTypes.count(resourceRecord.qtype.getCode()) != 0)) { + && (QType::exclusiveEntryTypes.count(qtype.getCode()) != 0 + || QType::exclusiveEntryTypes.count(resourceRecord.qtype.getCode()) != 0)) { // leave database handle in a consistent state domainInfo.backend->lookupEnd(); throw ApiException("RRset " + qname.toString() + " IN " + qtype.toString() + ": Conflicts with pre-existing RRset"); diff --git a/regression-tests.auth-py/test_GSSTSIG.py b/regression-tests.auth-py/test_GSSTSIG.py index e0a07bd18..cfae4d886 100644 --- a/regression-tests.auth-py/test_GSSTSIG.py +++ b/regression-tests.auth-py/test_GSSTSIG.py @@ -63,9 +63,9 @@ dnsupdate-require-tsig=no os.system("$PDNSUTIL --config-dir=configs/auth create-zone noacceptor.net") os.system("$PDNSUTIL --config-dir=configs/auth create-zone wrongacceptor.net") - os.system("$PDNSUTIL --config-dir=configs/auth add-record example.net example.net SOA 3600 'ns1.example.net otto.example.net 2022010403 10800 3600 604800 3600'") - os.system("$PDNSUTIL --config-dir=configs/auth add-record noacceptor.net noacceptor.net SOA 3600 'ns1.noacceptor.net otto.example.net 2022010403 10800 3600 604800 3600'") - os.system("$PDNSUTIL --config-dir=configs/auth add-record wrongacceptor.net wrongacceptor.net SOA 3600 'ns1.wrongacceptor.net otto.example.net 2022010403 10800 3600 604800 3600'") + os.system("$PDNSUTIL --config-dir=configs/auth replace-rrset example.net example.net SOA 3600 'ns1.example.net otto.example.net 2022010403 10800 3600 604800 3600'") + os.system("$PDNSUTIL --config-dir=configs/auth replace-rrset noacceptor.net noacceptor.net SOA 3600 'ns1.noacceptor.net otto.example.net 2022010403 10800 3600 604800 3600'") + os.system("$PDNSUTIL --config-dir=configs/auth replace-rrset wrongacceptor.net wrongacceptor.net SOA 3600 'ns1.wrongacceptor.net otto.example.net 2022010403 10800 3600 604800 3600'") os.system("$PDNSUTIL --config-dir=configs/auth set-meta example.net GSS-ACCEPTOR-PRINCIPAL DNS/ns1.example.net@EXAMPLE.COM") os.system("$PDNSUTIL --config-dir=configs/auth set-meta wrongacceptor.net GSS-ACCEPTOR-PRINCIPAL DNS/ns1.example.net@EXAMPLE.COM") diff --git a/regression-tests/tests/pdnsutil-zone-handling/expected_result b/regression-tests/tests/pdnsutil-zone-handling/expected_result index 5c836ac85..5bc05c02c 100644 --- a/regression-tests/tests/pdnsutil-zone-handling/expected_result +++ b/regression-tests/tests/pdnsutil-zone-handling/expected_result @@ -5,8 +5,8 @@ host.bug.less. 3600 IN A 127.0.0.1 Ignoring duplicate record content "127.0.0.2" New rrset: host2.bug.less. 3600 IN A 127.0.0.2 -Attempting to add record to cname.bug.less which already has a CNAME record -Attempting to add CNAME to host.bug.less which already has existing records +RRset cname.bug.less. IN CNAME: Conflicts with another RRset +RRset host.bug.less. IN CNAME: Conflicts with another RRset New rrset: host2.bug.less. 3600 IN A 127.0.0.2 host2.bug.less. 3600 IN A 127.0.0.3