From fe61f5d87871b56a17612c5a8334a84391f0d962 Mon Sep 17 00:00:00 2001
From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Fri, 2 Jun 2017 13:52:00 +0200
Subject: [PATCH] rec: Don't take the initial ECS source for a scope one if
 EDNS is off

---
 pdns/lwres.cc          | 8 ++++++--
 pdns/recursor_cache.hh | 2 +-
 pdns/syncres.cc        | 4 ++--
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/pdns/lwres.cc b/pdns/lwres.cc
index e19787dbb..516f92d99 100644
--- a/pdns/lwres.cc
+++ b/pdns/lwres.cc
@@ -116,20 +116,20 @@ int asyncresolve(const ComboAddress& ip, const DNSName& domain, int type, bool d
 
   string ping;
   bool weWantEDNSSubnet=false;
-  if(EDNS0Level) { 
+  if(EDNS0Level > 0) {
     DNSPacketWriter::optvect_t opts;
     if(srcmask) {
       EDNSSubnetOpts eo;
       eo.source = *srcmask;
       //      cout<<"Adding request mask: "<<eo.source.toString()<<endl;
       opts.push_back(make_pair(8, makeEDNSSubnetOptsString(eo)));
-      srcmask=boost::optional<Netmask>(); // this is also our return value
       weWantEDNSSubnet=true;
     }
 
     pw.addOpt(g_outgoingEDNSBufsize, 0, g_dnssecmode == DNSSECMode::Off ? 0 : EDNSOpts::DNSSECOK, opts); 
     pw.commit();
   }
+  srcmask = boost::none; // this is also our return value, even if EDNS0Level == 0
   lwr->d_rcode = 0;
   lwr->d_haveEDNS = false;
   int ret;
@@ -255,6 +255,10 @@ int asyncresolve(const ComboAddress& ip, const DNSName& domain, int type, bool d
             EDNSSubnetOpts reso;
             if(getEDNSSubnetOptsFromString(opt.second, &reso)) {
               //	    cerr<<"EDNS Subnet response: "<<reso.source.toString()<<", scope: "<<reso.scope.toString()<<", family = "<<reso.scope.getNetwork().sin4.sin_family<<endl;
+              /* rfc7871 states that 0 "indicate[s] that the answer is suitable for all addresses in FAMILY",
+                 so we might want to still pass the information along to be able to differentiate between
+                 IPv4 and IPv6. Still I'm pretty sure it doesn't matter in real life, so let's not duplicate
+                 entries in our cache. */
               if(reso.scope.getBits())
                 srcmask = reso.scope;
             }
diff --git a/pdns/recursor_cache.hh b/pdns/recursor_cache.hh
index 8999335f9..9d61149a1 100644
--- a/pdns/recursor_cache.hh
+++ b/pdns/recursor_cache.hh
@@ -55,7 +55,7 @@ public:
   unsigned int bytes();
   int32_t get(time_t, const DNSName &qname, const QType& qt, vector<DNSRecord>* res, const ComboAddress& who, vector<std::shared_ptr<RRSIGRecordContent>>* signatures=0);
 
-  void replace(time_t, const DNSName &qname, const QType& qt,  const vector<DNSRecord>& content, const vector<shared_ptr<RRSIGRecordContent>>& signatures, bool auth, boost::optional<Netmask> ednsmask=boost::optional<Netmask>());
+  void replace(time_t, const DNSName &qname, const QType& qt,  const vector<DNSRecord>& content, const vector<shared_ptr<RRSIGRecordContent>>& signatures, bool auth, boost::optional<Netmask> ednsmask=boost::none);
   void doPrune(void);
   uint64_t doDump(int fd);
 
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 4652fb175..5b077b6ad 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -417,7 +417,7 @@ int SyncRes::asyncresolveWrapper(const ComboAddress& ip, bool ednsMANDATORY, con
 
   SyncRes::EDNSStatus::EDNSMode& mode=ednsstatus->mode;
   SyncRes::EDNSStatus::EDNSMode oldmode = mode;
-  int EDNSLevel=0;
+  int EDNSLevel = 0;
   auto luaconfsLocal = g_luaconfs.getLocal();
   ResolveContext ctx;
 #ifdef HAVE_PROTOBUF
@@ -1245,7 +1245,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(const std::string& prefix, LWResu
     if(i->second.records.empty()) // this happens when we did store signatures, but passed on the records themselves
       continue;
 
-    t_RC->replace(d_now.tv_sec, i->first.name, QType(i->first.type), i->second.records, i->second.signatures, lwr.d_aabit, i->first.place == DNSResourceRecord::ANSWER ? ednsmask : boost::optional<Netmask>());
+    t_RC->replace(d_now.tv_sec, i->first.name, QType(i->first.type), i->second.records, i->second.signatures, lwr.d_aabit, i->first.place == DNSResourceRecord::ANSWER ? ednsmask : boost::none);
 
     if(i->first.place == DNSResourceRecord::ANSWER && ednsmask)
       d_wasVariable=true;
-- 
2.40.0