Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rec: Allow the use of a 'self-resolving' NS if cached A/AAAA exists #5937

Merged
merged 1 commit into from Nov 10, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
67 changes: 63 additions & 4 deletions pdns/recursordist/test-syncres_cc.cc
Expand Up @@ -101,12 +101,17 @@ LuaConfigItems::LuaConfigItems()

static void init(bool debug=false)
{
L.setName("test");
L.disableSyslog(true);

if (debug) {
L.setName("test");
L.setLoglevel((Logger::Urgency)(6)); // info and up
L.disableSyslog(true);
L.toConsole(Logger::Info);
}
else {
L.setLoglevel(Logger::None);
L.toConsole(Logger::Error);
}

seedRandom("/dev/urandom");
reportAllTypes();
Expand Down Expand Up @@ -775,7 +780,7 @@ BOOST_AUTO_TEST_CASE(test_all_nss_network_error) {
}
else {
downServers.insert(ip);
return -1;
return 0;
}
});

Expand All @@ -791,10 +796,64 @@ BOOST_AUTO_TEST_CASE(test_all_nss_network_error) {
for (const auto& server : downServers) {
BOOST_CHECK_EQUAL(SyncRes::getServerFailsCount(server), 1);
BOOST_CHECK(SyncRes::isThrottled(time(nullptr), server, target, QType::A));
;
}
}

BOOST_AUTO_TEST_CASE(test_only_one_ns_up_resolving_itself_with_glue) {
std::unique_ptr<SyncRes> sr;
initSR(sr);

primeHints();

DNSName target("www.powerdns.com.");

sr->setAsyncCallback([target](const ComboAddress& ip, const DNSName& domain, int type, bool doTCP, bool sendRDQuery, int EDNS0Level, struct timeval* now, boost::optional<Netmask>& srcmask, boost::optional<const ResolveContext&> context, std::shared_ptr<RemoteLogger> outgoingLogger, LWResult* res) {

if (isRootServer(ip)) {
setLWResult(res, 0, false, false, true);
if (domain == target) {
addRecordToLW(res, "powerdns.com.", QType::NS, "pdns-public-ns1.powerdns.com.", DNSResourceRecord::AUTHORITY, 172800);
addRecordToLW(res, "powerdns.com.", QType::NS, "pdns-public-ns2.powerdns.net.", DNSResourceRecord::AUTHORITY, 172800);
addRecordToLW(res, "pdns-public-ns1.powerdns.com.", QType::A, "192.0.2.2", DNSResourceRecord::ADDITIONAL, 172800);
addRecordToLW(res, "pdns-public-ns1.powerdns.com.", QType::AAAA, "2001:DB8::2", DNSResourceRecord::ADDITIONAL, 172800);
}
else if (domain == DNSName("pdns-public-ns2.powerdns.net.")) {
addRecordToLW(res, "powerdns.net.", QType::NS, "pdns-public-ns2.powerdns.net.", DNSResourceRecord::AUTHORITY, 172800);
addRecordToLW(res, "powerdns.net.", QType::NS, "pdns-public-ns1.powerdns.com.", DNSResourceRecord::AUTHORITY, 172800);
addRecordToLW(res, "pdns-public-ns2.powerdns.net.", QType::A, "192.0.2.3", DNSResourceRecord::ADDITIONAL, 172800);
addRecordToLW(res, "pdns-public-ns2.powerdns.net.", QType::AAAA, "2001:DB8::3", DNSResourceRecord::ADDITIONAL, 172800);
}
return 1;
}
else if (ip == ComboAddress("192.0.2.3:53")) {
setLWResult(res, 0, true, false, true);
if (domain == DNSName("pdns-public-ns2.powerdns.net.")) {
if (type == QType::A) {
addRecordToLW(res, "pdns-public-ns2.powerdns.net.", QType::A, "192.0.2.3");
}
else if (type == QType::AAAA) {
addRecordToLW(res, "pdns-public-ns2.powerdns.net.", QType::AAAA, "2001:DB8::3");
}
}
else if (domain == target) {
if (type == QType::A) {
addRecordToLW(res, domain, QType::A, "192.0.2.1");
}
else if (type == QType::AAAA) {
addRecordToLW(res, domain, QType::AAAA, "2001:DB8::1");
}
}
return 1;
}
return 0;
});

vector<DNSRecord> ret;
int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
BOOST_CHECK_EQUAL(res, RCode::NoError);
BOOST_CHECK_EQUAL(ret.size(), 1);
}

BOOST_AUTO_TEST_CASE(test_os_limit_errors) {
std::unique_ptr<SyncRes> sr;
initSR(sr);
Expand Down
22 changes: 14 additions & 8 deletions pdns/syncres.cc
Expand Up @@ -624,7 +624,7 @@ struct speedOrderCA

/** This function explicitly goes out for A or AAAA addresses
*/
vector<ComboAddress> SyncRes::getAddrs(const DNSName &qname, unsigned int depth, set<GetBestNSAnswer>& beenthere)
vector<ComboAddress> SyncRes::getAddrs(const DNSName &qname, unsigned int depth, set<GetBestNSAnswer>& beenthere, bool cacheOnly)
{
typedef vector<DNSRecord> res_t;
res_t res;
Expand All @@ -633,10 +633,12 @@ vector<ComboAddress> SyncRes::getAddrs(const DNSName &qname, unsigned int depth,
ret_t ret;

QType type;
bool oldCacheOnly = d_cacheonly;
bool oldRequireAuthData = d_requireAuthData;
bool oldValidationRequested = d_DNSSECValidationRequested;
d_requireAuthData = false;
d_DNSSECValidationRequested = false;
d_cacheonly = cacheOnly;

for(int j=1; j<2+s_doIPv6; j++)
{
Expand Down Expand Up @@ -685,6 +687,7 @@ vector<ComboAddress> SyncRes::getAddrs(const DNSName &qname, unsigned int depth,

d_requireAuthData = oldRequireAuthData;
d_DNSSECValidationRequested = oldValidationRequested;
d_cacheonly = oldCacheOnly;

/* we need to remove from the nsSpeeds collection the existing IPs
for this nameserver that are no longer in the set, even if there
Expand Down Expand Up @@ -1388,13 +1391,13 @@ bool SyncRes::nameserverIPBlockedByRPZ(const DNSFilterEngine& dfe, const ComboAd
return false;
}

vector<ComboAddress> SyncRes::retrieveAddressesForNS(const std::string& prefix, const DNSName& qname, vector<DNSName >::const_iterator& tns, const unsigned int depth, set<GetBestNSAnswer>& beenthere, const vector<DNSName >& rnameservers, NsSet& nameservers, bool& sendRDQuery, bool& pierceDontQuery, bool& flawedNSSet)
vector<ComboAddress> SyncRes::retrieveAddressesForNS(const std::string& prefix, const DNSName& qname, vector<DNSName >::const_iterator& tns, const unsigned int depth, set<GetBestNSAnswer>& beenthere, const vector<DNSName >& rnameservers, NsSet& nameservers, bool& sendRDQuery, bool& pierceDontQuery, bool& flawedNSSet, bool cacheOnly)
{
vector<ComboAddress> result;

if(!tns->empty()) {
LOG(prefix<<qname<<": Trying to resolve NS '"<<*tns<< "' ("<<1+tns-rnameservers.begin()<<"/"<<(unsigned int)rnameservers.size()<<")"<<endl);
result = getAddrs(*tns, depth+2, beenthere);
result = getAddrs(*tns, depth+2, beenthere, cacheOnly);
pierceDontQuery=false;
}
else {
Expand Down Expand Up @@ -2580,10 +2583,13 @@ int SyncRes::doResolveAt(NsSet &nameservers, DNSName auth, bool flawedNSSet, con
return -1;
}

// this line needs to identify the 'self-resolving' behaviour, but we get it wrong now
if(qname == *tns && qtype.getCode()==QType::A && rnameservers.size() > (size_t)(1+1*s_doIPv6)) {
LOG(prefix<<qname<<": Not using NS to resolve itself! ("<<(1+tns-rnameservers.cbegin())<<"/"<<rnameservers.size()<<")"<<endl);
continue;
bool cacheOnly = false;
// this line needs to identify the 'self-resolving' behaviour
if(qname == *tns && (qtype.getCode() == QType::A || qtype.getCode() == QType::AAAA)) {
/* we might have a glue entry in cache so let's try this NS
but only if we have enough in the cache to know how to reach it */
LOG(prefix<<qname<<": Using NS to resolve itself, but only using what we have in cache ("<<(1+tns-rnameservers.cbegin())<<"/"<<rnameservers.size()<<")"<<endl);
cacheOnly = true;
}

typedef vector<ComboAddress> remoteIPs_t;
Expand Down Expand Up @@ -2615,7 +2621,7 @@ int SyncRes::doResolveAt(NsSet &nameservers, DNSName auth, bool flawedNSSet, con
}
else {
/* if tns is empty, retrieveAddressesForNS() knows we have hardcoded servers (i.e. "forwards") */
remoteIPs = retrieveAddressesForNS(prefix, qname, tns, depth, beenthere, rnameservers, nameservers, sendRDQuery, pierceDontQuery, flawedNSSet);
remoteIPs = retrieveAddressesForNS(prefix, qname, tns, depth, beenthere, rnameservers, nameservers, sendRDQuery, pierceDontQuery, flawedNSSet, cacheOnly);

if(remoteIPs.empty()) {
LOG(prefix<<qname<<": Failed to get IP for NS "<<*tns<<", trying next if available"<<endl);
Expand Down
4 changes: 2 additions & 2 deletions pdns/syncres.hh
Expand Up @@ -735,13 +735,13 @@ private:

inline vector<DNSName> shuffleInSpeedOrder(NsSet &nameservers, const string &prefix);
bool moreSpecificThan(const DNSName& a, const DNSName &b) const;
vector<ComboAddress> getAddrs(const DNSName &qname, unsigned int depth, set<GetBestNSAnswer>& beenthere);
vector<ComboAddress> getAddrs(const DNSName &qname, unsigned int depth, set<GetBestNSAnswer>& beenthere, bool cacheOnly);

bool nameserversBlockedByRPZ(const DNSFilterEngine& dfe, const NsSet& nameservers);
bool nameserverIPBlockedByRPZ(const DNSFilterEngine& dfe, const ComboAddress&);
bool throttledOrBlocked(const std::string& prefix, const ComboAddress& remoteIP, const DNSName& qname, const QType& qtype, bool pierceDontQuery);

vector<ComboAddress> retrieveAddressesForNS(const std::string& prefix, const DNSName& qname, vector<DNSName >::const_iterator& tns, const unsigned int depth, set<GetBestNSAnswer>& beenthere, const vector<DNSName >& rnameservers, NsSet& nameservers, bool& sendRDQuery, bool& pierceDontQuery, bool& flawedNSSet);
vector<ComboAddress> retrieveAddressesForNS(const std::string& prefix, const DNSName& qname, vector<DNSName >::const_iterator& tns, const unsigned int depth, set<GetBestNSAnswer>& beenthere, const vector<DNSName >& rnameservers, NsSet& nameservers, bool& sendRDQuery, bool& pierceDontQuery, bool& flawedNSSet, bool cacheOnly);
RCode::rcodes_ updateCacheFromRecords(unsigned int depth, LWResult& lwr, const DNSName& qname, const QType& qtype, const DNSName& auth, bool wasForwarded, const boost::optional<Netmask>, vState& state, bool& needWildcardProof);
bool processRecords(const std::string& prefix, const DNSName& qname, const QType& qtype, const DNSName& auth, LWResult& lwr, const bool sendRDQuery, vector<DNSRecord>& ret, set<DNSName>& nsset, DNSName& newtarget, DNSName& newauth, bool& realreferral, bool& negindic, vState& state, bool needWildcardProof);

Expand Down