From: Otto Moerbeek Date: Wed, 19 Jan 2022 12:34:09 +0000 (+0100) Subject: Move ZoneToCache from a separate thread to the handler, so that we can resolve e... X-Git-Tag: auth-4.7.0-alpha1~42^2~14 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b8d0d0db5f47dca7080a97dfb6247c66fb5c0da5;p=thirdparty%2Fpdns.git Move ZoneToCache from a separate thread to the handler, so that we can resolve e.g. DS records. Config and logic of ZoneMD wrt DNSSEC is too complex, needs more thought. --- diff --git a/pdns/rec-lua-conf.cc b/pdns/rec-lua-conf.cc index 59af03d20c..d0b73955d9 100644 --- a/pdns/rec-lua-conf.cc +++ b/pdns/rec-lua-conf.cc @@ -354,7 +354,7 @@ void loadRecursorLuaConfig(const std::string& fname, luaConfigDelayedThreads& de /* we can get: "1.2.3.4" {"1.2.3.4", "4.5.6.7"} - {"1.2.3.4", {"4.5.6.7", "8.9.10.11"}} + {"1.2.3.4", {"4.5.6.7", "8.9.10.11"}} */ map pmap{ @@ -398,7 +398,7 @@ void loadRecursorLuaConfig(const std::string& fname, luaConfigDelayedThreads& de typedef std::unordered_map> zoneToCacheOptions_t; - Lua.writeFunction("zoneToCache", [&delayedThreads](const string& zoneName, const string& method, const boost::variant>>& srcs, boost::optional options) { + Lua.writeFunction("zoneToCache", [&lci](const string& zoneName, const string& method, const boost::variant>>& srcs, boost::optional options) { try { RecZoneToCache::Config conf; DNSName validZoneName(zoneName); @@ -447,23 +447,24 @@ void loadRecursorLuaConfig(const std::string& fname, luaConfigDelayedThreads& de if (have.count("zonemdValidation")) { string zonemdValidation = boost::get(have.at("zonemdValidation")); const map nameToVal = { - { "ignore", pdns::ZoneMD::Config::Ignore}, - { "process", pdns::ZoneMD::Config::Process}, - { "logonly", pdns::ZoneMD::Config::LogOnly}, - { "required", pdns::ZoneMD::Config::Required}, - { "requiredWithDNSSEC", pdns::ZoneMD::Config::RequiredWithDNSSEC}, - { "requiredButIgnoreDNSSEC", pdns::ZoneMD::Config::RequiredButIgnoreDNSSEC}, - }; + {"ignore", pdns::ZoneMD::Config::Ignore}, + {"process", pdns::ZoneMD::Config::Process}, + {"logonly", pdns::ZoneMD::Config::LogOnly}, + {"required", pdns::ZoneMD::Config::Required}, + {"requiredWithDNSSEC", pdns::ZoneMD::Config::RequiredWithDNSSEC}, + {"requiredButIgnoreDNSSEC", pdns::ZoneMD::Config::RequiredButIgnoreDNSSEC}, + }; auto it = nameToVal.find(zonemdValidation); if (it == nameToVal.end()) { throw std::runtime_error(zonemdValidation + " is not a valid value for `zonemdValidation`"); - } else { + } + else { conf.d_zonemd = it->second; } } } - delayedThreads.ztcConfigs.push_back(conf); + lci.ztcConfigs[validZoneName] = conf; } catch (const std::exception& e) { g_log << Logger::Error << "Problem configuring zoneToCache for zone '" << zoneName << "': " << e.what() << endl; @@ -710,19 +711,4 @@ void startLuaConfigDelayedThreads(const luaConfigDelayedThreads& delayedThreads, exit(1); } } - - for (const auto& ztcConfig : delayedThreads.ztcConfigs) { - try { - std::thread t(RecZoneToCache::ZoneToCache, ztcConfig, generation); - t.detach(); - } - catch (const std::exception& e) { - g_log << Logger::Error << "Problem starting zoneToCache thread: " << e.what() << endl; - exit(1); - } - catch (const PDNSException& e) { - g_log << Logger::Error << "Problem starting zoneToCache thread: " << e.reason << endl; - exit(1); - } - } } diff --git a/pdns/rec-lua-conf.hh b/pdns/rec-lua-conf.hh index 985d2e6bd0..830c9987aa 100644 --- a/pdns/rec-lua-conf.hh +++ b/pdns/rec-lua-conf.hh @@ -71,6 +71,7 @@ public: TrustAnchorFileInfo trustAnchorFileInfo; // Used to update the Trust Anchors from file periodically map dsAnchors; map negAnchors; + map ztcConfigs; ProtobufExportConfig protobufExportConfig; ProtobufExportConfig outgoingProtobufExportConfig; FrameStreamExportConfig frameStreamExportConfig; @@ -88,7 +89,6 @@ struct luaConfigDelayedThreads { // Please make sure that the tuple below only contains value types since they are used as parameters in a thread ct std::vector, boost::optional, bool, uint32_t, size_t, TSIGTriplet, size_t, ComboAddress, uint16_t, uint32_t, std::shared_ptr, std::string>> rpzPrimaryThreads; - std::vector ztcConfigs; }; void loadRecursorLuaConfig(const std::string& fname, luaConfigDelayedThreads& delayedThreads); diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc index cab9f4a301..65ea478fb3 100644 --- a/pdns/recursordist/rec-main.cc +++ b/pdns/recursordist/rec-main.cc @@ -1576,7 +1576,6 @@ static int serviceMain(int argc, char* argv[]) startLuaConfigDelayedThreads(delayedLuaThreads, g_luaconfs.getCopy().generation); delayedLuaThreads.rpzPrimaryThreads.clear(); // no longer needed - delayedLuaThreads.ztcConfigs.clear(); // no longer needed makeThreadPipes(); @@ -1808,6 +1807,11 @@ static void houseKeeping(void*) } if (isHandlerThread()) { + static map ztcState; + for (auto& ztc : luaconfsLocal->ztcConfigs) { + RecZoneToCache::ZoneToCache(ztc.second, ztcState[ztc.first]); + } + if (now.tv_sec - s_last_RC_prune > 5) { g_recCache->doPrune(g_maxCacheEntries); g_negCache->prune(g_maxCacheEntries / 10); diff --git a/pdns/recursordist/rec-zonetocache.cc b/pdns/recursordist/rec-zonetocache.cc index 0f4c4e911c..a059500bdd 100644 --- a/pdns/recursordist/rec-zonetocache.cc +++ b/pdns/recursordist/rec-zonetocache.cc @@ -63,8 +63,8 @@ struct ZoneData void parseDRForCache(DNSRecord& dr); pdns::ZoneMD::Result getByAXFR(const RecZoneToCache::Config&, pdns::ZoneMD&); pdns::ZoneMD::Result processLines(const std::vector& lines, const RecZoneToCache::Config& config, pdns::ZoneMD&); - void ZoneToCache(const RecZoneToCache::Config& config, uint64_t gen); - vState dnssecValidate(pdns::ZoneMD&) const; + void ZoneToCache(const RecZoneToCache::Config& config); + vState dnssecValidate(pdns::ZoneMD&, size_t& zonemdCount) const; }; bool ZoneData::isRRSetAuth(const DNSName& qname, QType qtype) const @@ -236,40 +236,33 @@ pdns::ZoneMD::Result ZoneData::processLines(const vector& lines, const R return pdns::ZoneMD::Result::OK; } -vState ZoneData::dnssecValidate(pdns::ZoneMD &zonemd) const +vState ZoneData::dnssecValidate(pdns::ZoneMD& zonemd, size_t& zonemdCount) const { - vector dsRecords; - vState dsState = vState::Indeterminate; + zonemdCount = 0; - // XXX TA/NTA processsing, plus we have nog guarantee the DS is - // is in the cache. We assume for now it's cached magically + SyncRes sr({d_now, 0}); + sr.setDoDNSSEC(true); + sr.setDNSSECValidationRequested(true); - // Get the DS records - if (g_recCache->get(d_now, d_zone, QType::DS, false, &dsRecords, ComboAddress(), false, boost::none, nullptr, nullptr, nullptr, &dsState) <= 0) { - return vState::BogusUnableToGetDSs; - } + dsmap_t dsmap; // Actually a set + vState dsState = sr.getDSRecords(d_zone, dsmap, false, 0); if (dsState != vState::Secure) { - return vState::BogusUnableToGetDSs; // XXX is this the right status? - } - - // Collect DNSKEYs and validate them using the DS records - dsmap_t dsRecordContents; - for (const auto& ds : dsRecords) { - dsRecordContents.insert(*getRR(ds).get()); + cerr << "getDSRecords says" << dsState << endl; + return vState::Insecure; } - skeyset_t dnsKeys; + skeyset_t dnsKeys; sortedRecords_t records; if (zonemd.getDNSKEYs().size() == 0) { return vState::BogusUnableToGetDNSKEYs; } - for (const auto& key: zonemd.getDNSKEYs()) { + for (const auto& key : zonemd.getDNSKEYs()) { dnsKeys.emplace(key); records.emplace(key); } skeyset_t validKeys; - vState dnsKeyState = validateDNSKeysAgainstDS(d_now, d_zone, dsRecordContents, dnsKeys, records, zonemd.getRRSIGs(), validKeys); + vState dnsKeyState = validateDNSKeysAgainstDS(d_now, d_zone, dsmap, dnsKeys, records, zonemd.getRRSIGs(), validKeys); if (dnsKeyState != vState::Secure) { return dnsKeyState; } @@ -277,21 +270,24 @@ vState ZoneData::dnssecValidate(pdns::ZoneMD &zonemd) const if (validKeys.size() == 0) { return vState::BogusNoValidDNSKEY; } - if (zonemd.getZONEMDs().size() == 0) { - // XXX is this the right status? Also per RFC we should actuelly prove non-existence of ZONEMD + + auto zonemdRecords = zonemd.getZONEMDs(); + zonemdCount = zonemdRecords.size(); + if (zonemdCount == 0) { + // Per RFC we should actually prove non-existence of ZONEMD // as downgrade attacks could be possible by an in the middle party zapping ZONEMDs return dnsKeyState; } records.clear(); // Collect the ZONEMD records and validate them using the validted DNSSKEYs - for (const auto& rec : zonemd.getZONEMDs()) { + for (const auto& rec : zonemdRecords) { records.emplace(rec); } - return validateWithKeySet(d_now, d_zone, records, zonemd.getRRSIGs(), validKeys); + return validateWithKeySet(d_now, d_zone, records, zonemd.getRRSIGs(), validKeys); } -void ZoneData::ZoneToCache(const RecZoneToCache::Config& config, uint64_t configGeneration) +void ZoneData::ZoneToCache(const RecZoneToCache::Config& config) { if (config.d_sources.size() > 1) { d_log->info("Multiple sources not yet supported, using first"); @@ -321,25 +317,28 @@ void ZoneData::ZoneToCache(const RecZoneToCache::Config& config, uint64_t config } if (config.d_zonemd == pdns::ZoneMD::Config::RequiredWithDNSSEC && g_dnssecmode == DNSSECMode::Off) { - throw PDNSException("ZONEMD DNSSEC validation failure: dnssec is switched of but required by ztc"); + throw PDNSException("ZONEMD DNSSEC validation failure: dnssec is switched of but required by ZoneToCache"); } // Validate DNSKEYs and ZONEMD, rest of records are validated on-demand by SyncRes - if (config.d_zonemd == pdns::ZoneMD::Config::RequiredWithDNSSEC || - (g_dnssecmode == DNSSECMode::ValidateAll && config.d_zonemd != pdns::ZoneMD::Config::RequiredButIgnoreDNSSEC)) { - auto validationStatus = dnssecValidate(zonemd); - d_log->info("ZONEMD DNSSEC validation done", "validationStatus", Logging::Loggable(validationStatus)); - if (config.d_zonemd == pdns::ZoneMD::Config::RequiredWithDNSSEC) { + if (config.d_zonemd == pdns::ZoneMD::Config::RequiredWithDNSSEC || (g_dnssecmode != DNSSECMode::Off && config.d_zonemd != pdns::ZoneMD::Config::RequiredButIgnoreDNSSEC)) { + size_t zonemdCount; + auto validationStatus = dnssecValidate(zonemd, zonemdCount); + d_log->info("ZONEMD record related DNSSEC validation done", "validationStatus", Logging::Loggable(validationStatus), + "zonemdCount", Logging::Loggable(zonemdCount)); + if (config.d_zonemd == pdns::ZoneMD::Config::RequiredWithDNSSEC || g_dnssecmode == DNSSECMode::ValidateAll) { if (validationStatus != vState::Secure) { throw PDNSException("ZONEMD required DNSSEC validation failed"); } } - // XXX handle other cases + if (validationStatus != vState::Secure && validationStatus != vState::Insecure) { + throw PDNSException("ZONEMD record DNSSEC Validation failed"); + } } if (pdns::ZoneMD::validationRequired(config.d_zonemd) && result != pdns::ZoneMD::Result::OK) { // We do not accept NoValidationDone in this case - throw PDNSException("ZONEMD validation failure"); + throw PDNSException("ZONEMD digest validation failure"); return; } if (config.d_zonemd == pdns::ZoneMD::Config::Process && result == pdns::ZoneMD::Result::ValidationFailure) { @@ -361,12 +360,6 @@ void ZoneData::ZoneToCache(const RecZoneToCache::Config& config, uint64_t config } } - // Extra check before we are touching the cache - auto luaconfsLocal = g_luaconfs.getLocal(); - if (luaconfsLocal->generation != configGeneration) { - return; - } - // Rerun, now inserting the rrsets into the cache with associated sigs d_now = time(nullptr); for (const auto& [key, v] : d_all) { @@ -395,45 +388,32 @@ void ZoneData::ZoneToCache(const RecZoneToCache::Config& config, uint64_t config } } -// Config must be a copy, so call by value! -void RecZoneToCache::ZoneToCache(RecZoneToCache::Config config, uint64_t configGeneration) +void RecZoneToCache::ZoneToCache(const RecZoneToCache::Config& config, RecZoneToCache::State& state) { - setThreadName("pdns-r/ztc/" + config.d_zone); - auto luaconfsLocal = g_luaconfs.getLocal(); + if (state.d_waittime == 0 && state.d_lastrun > 0) { + // single shot + return; + } + if (state.d_lastrun > 0 && state.d_lastrun + state.d_waittime > time(nullptr)) { + return; + } auto log = g_slog->withName("ztc")->withValues("zone", Logging::Loggable(config.d_zone)); - while (true) { - if (luaconfsLocal->generation != configGeneration) { - /* the configuration has been reloaded, meaning that a new thread - has been started to handle that zone and we are now obsolete. - */ - log->info("A more recent configuration has been found, stopping the old update thread"); - return; - } - - time_t refresh = config.d_retryOnError; - try { - ZoneData data(log, config.d_zone); - data.ZoneToCache(config, configGeneration); - if (luaconfsLocal->generation != configGeneration) { - log->info("A more recent configuration has been found, stopping the old update thread"); - return; - } - refresh = config.d_refreshPeriod; - log->info("Loaded zone into cache", "refresh", Logging::Loggable(refresh)); - } - catch (const PDNSException& e) { - log->info("Unable to load zone into cache, will retry", "exception", Logging::Loggable(e.reason), "refresh", Logging::Loggable(refresh)); - } - catch (const std::runtime_error& e) { - log->info("Unable to load zone into cache, will retry", "exception", Logging::Loggable(e.what()), "refresh", Logging::Loggable(refresh)); - } - catch (...) { - log->info("Unable to load zone into cache, will retry", "exception", Logging::Loggable("unknown"), "refresh", Logging::Loggable(refresh)); - } - if (refresh == 0) { - return; // single shot - } - sleep(refresh); + state.d_waittime = config.d_retryOnError; + try { + ZoneData data(log, config.d_zone); + data.ZoneToCache(config); + state.d_waittime = config.d_refreshPeriod; + log->info("Loaded zone into cache", "refresh", Logging::Loggable(state.d_waittime)); + } + catch (const PDNSException& e) { + log->info("Unable to load zone into cache, will retry", "exception", Logging::Loggable(e.reason), "refresh", Logging::Loggable(state.d_waittime)); + } + catch (const std::runtime_error& e) { + log->info("Unable to load zone into cache, will retry", "exception", Logging::Loggable(e.what()), "refresh", Logging::Loggable(state.d_waittime)); + } + catch (...) { + log->info("Unable to load zone into cache, will retry", "exception", Logging::Loggable("unknown"), "refresh", Logging::Loggable(state.d_waittime)); } + state.d_lastrun = time(nullptr); } diff --git a/pdns/recursordist/rec-zonetocache.hh b/pdns/recursordist/rec-zonetocache.hh index 4b48809f2e..7284895eb3 100644 --- a/pdns/recursordist/rec-zonetocache.hh +++ b/pdns/recursordist/rec-zonetocache.hh @@ -43,5 +43,12 @@ public: uint32_t d_timeout{20}; // timeout in seconds pdns::ZoneMD::Config d_zonemd{pdns::ZoneMD::Config::Process}; }; - static void ZoneToCache(Config config, uint64_t gen); + + struct State + { + time_t d_lastrun{0}; + time_t d_waittime{0}; + }; + + static void ZoneToCache(const Config& config, State& state); }; diff --git a/pdns/recursordist/test-rec-zonetocache.cc b/pdns/recursordist/test-rec-zonetocache.cc index 2081124ab5..52b37a8756 100644 --- a/pdns/recursordist/test-rec-zonetocache.cc +++ b/pdns/recursordist/test-rec-zonetocache.cc @@ -70,13 +70,14 @@ BOOST_AUTO_TEST_CASE(test_zonetocache) BOOST_REQUIRE(fclose(fp) == 0); RecZoneToCache::Config config{".", "file", {temp}, ComboAddress(), TSIGTriplet()}; + RecZoneToCache::State state; config.d_refreshPeriod = 0; config.d_retryOnError = 0; // Start with a new, empty cache g_recCache = std::make_unique(); BOOST_CHECK_EQUAL(g_recCache->size(), 0U); - RecZoneToCache::ZoneToCache(config, 0); + RecZoneToCache::ZoneToCache(config, state); unlink(temp); BOOST_CHECK_EQUAL(g_recCache->size(), 17U); diff --git a/pdns/zonemd.cc b/pdns/zonemd.cc index 3173996bf0..0fe904a45a 100644 --- a/pdns/zonemd.cc +++ b/pdns/zonemd.cc @@ -37,7 +37,7 @@ void pdns::ZoneMD::readRecords(ZoneParserTNG& zpt) case QType::DNSKEY: d_dnskeys.emplace(std::dynamic_pointer_cast(drc)); break; - case QType::ZONEMD :{ + case QType::ZONEMD: { auto zonemd = std::dynamic_pointer_cast(drc); auto inserted = d_zonemdRecords.insert({pair(zonemd->d_scheme, zonemd->d_hashalgo), {zonemd, false}}); if (!inserted.second) { @@ -171,7 +171,7 @@ void pdns::ZoneMD::verify(bool& validationDone, bool& validationOK) if (sorted.empty()) { // continue; } - + if (qtype != QType::RRSIG) { RRSIGRecordContent rrc; rrc.d_originalttl = d_resourceRecordSetTTLs[rrset.first];