auth: Implement `consumeRemaining` in DNSParser, DNSWriter, RecordTextReader and...

This is needed to deal with a bug (#17000) in the authoritative code that at
some point created non-empty ENT records in our databases.

Signed-off-by: Remi Gacogne <remi.gacogne@powerdns.com>

diff --git a/pdns/dnsparser.hh b/pdns/dnsparser.hh
index 8d06ea93a0c01b697be6ead3f9608571aadb0485..6c9dc3b2e08fb9e286438e560638f421e7263f3f 100644 (file)
--- a/pdns/dnsparser.hh
+++ b/pdns/dnsparser.hh
@@ -164,16 +164,32 @@ public:
   string getText(bool multi, bool lenField);
   string getUnquotedText(bool lenField);
 
-
+  /* Whether the current position is at (or after, which would indicate a problem
+     and should have been detected earlier) the end of wire data corresponding to
+     the current DNS record */
   bool eof() const
   {
     return d_pos >= (d_startrecordpos + d_recordlen);
   }
 
-  const string getRemaining() const {
+  /* Returns a string, formatted for human/log consumption, containing information
+     about the wire data from the current position to the end of the current DNS record */
+  std::string getRemaining() const {
     return "Remaining data from PacketReader, current position " + std::to_string(d_pos) + " in packet of size " + std::to_string(d_content.size()) + ", end of record expected at " + std::to_string(d_startrecordpos + d_recordlen) + ": " + makeHexDump(std::string(d_content.begin() + d_pos, d_content.begin() + d_startrecordpos + d_recordlen));
   };
 
+#if defined(PDNS_AUTH) // [
+  /* This method moves the position to the end of the current DNS record.
+     The only case where it makes sense to call this method is when processing ENT
+     records, and only because of a bug in the authoritative server used to insert
+     non-empty content for some ENT records (see https://github.com/PowerDNS/pdns/pull/17000)
+  */
+  void consumeRemaining()
+  {
+    d_pos = (d_startrecordpos + d_recordlen);
+  }
+#endif // ]
+
   uint16_t getPosition() const
   {
     return d_pos;

diff --git a/pdns/dnsrecords.cc b/pdns/dnsrecords.cc
index ec23059f3573b6ffe4b2ce5b58352fe052f9e7d1..ee139c70008dab9925db44fe9ebfebc4355cdda7 100644 (file)
--- a/pdns/dnsrecords.cc
+++ b/pdns/dnsrecords.cc
@@ -161,7 +161,15 @@ boilerplate_conv(TXT, conv.xfrText(d_text, true));
 #ifdef HAVE_LUA_RECORDS
 boilerplate_conv(LUA, conv.xfrType(d_type); conv.xfrText(d_code, true));
 #endif
+#if defined(PDNS_AUTH) // [
+/* Move the position to the end of the current DNS record,
+   because of a bug in the authoritative server used to insert
+   non-empty content for some ENT records (see https://github.com/PowerDNS/pdns/pull/17000)
+*/
+boilerplate_conv(ENT, conv.consumeRemaining());
+#else
 boilerplate_conv(ENT, );
+#endif // ]
 boilerplate_conv(SPF, conv.xfrText(d_text, true));
 boilerplate_conv(HINFO, conv.xfrText(d_cpu);   conv.xfrText(d_host));
 

diff --git a/pdns/dnswriter.hh b/pdns/dnswriter.hh
index 7d60661485b05a432cd56e496fe2f9748ec6ba9b..57b10040691332b60bc20e3aad192a1c0a170992 100644 (file)
--- a/pdns/dnswriter.hh
+++ b/pdns/dnswriter.hh
@@ -153,12 +153,21 @@ public:
   {
     return d_content;
   }
-  bool eof() { return true; } // we don't know how long the record should be
+  bool eof() const { return true; } // we don't know how long the record should be
 
-  const string getRemaining() const {
+  std::string getRemaining() const {
     return "";
   }
 
+#if defined(PDNS_AUTH) // [
+  /* This method is only there for parity with DNSParser::consumeRemaining(),
+     see the comment there to know why it is needed.
+  */
+  void consumeRemaining() const
+  {
+  }
+#endif // ]
+
   size_t getSizeWithOpts(const optvect_t& options) const;
 
 private:

diff --git a/pdns/rcpgenerator.cc b/pdns/rcpgenerator.cc
index abb1cf5de81d059eeb09d552816009c7b96d74f1..1049e10cecd222d6cc8756b17a256650163ddde9 100644 (file)
--- a/pdns/rcpgenerator.cc
+++ b/pdns/rcpgenerator.cc
@@ -236,7 +236,7 @@ void RecordTextReader::xfrCAPort(ComboAddress &val)
   val.sin4.sin_port = port;
 }
 
-bool RecordTextReader::eof()
+bool RecordTextReader::eof() const
 {
   return d_pos==d_end;
 }

diff --git a/pdns/rcpgenerator.hh b/pdns/rcpgenerator.hh
index 4892109d1bb0f3995b7b1976a39688f839eadddb..7c7cae84edf604fa4ee525d480fd441766689798 100644 (file)
--- a/pdns/rcpgenerator.hh
+++ b/pdns/rcpgenerator.hh
@@ -68,12 +68,21 @@ public:
   void xfrRFC1035CharString(string &val);
   void xfrSVCBValueList(vector<string> &val);
 
-  const string getRemaining() const {
+  std::string getRemaining() const {
     return d_string.substr(d_pos);
   }
 
+  bool eof() const;
+
+#if defined(PDNS_AUTH) // [
+  /* This method is only there for parity with DNSParser::consumeRemaining(),
+     see the comment there to know why it is needed.
+  */
+  void consumeRemaining() const
+  {
+  }
+#endif // ]
 
-  bool eof();
 private:
   string d_string;
   ZoneName d_zone;
@@ -106,13 +115,22 @@ public:
   void xfrBlob(const string& val, int len=-1);
   void xfrHexBlob(const string& val, bool keepReading=false);
   void xfrSvcParamKeyVals(const set<SvcParam>& val);
-  bool eof() { return true; };
-
   void xfrSVCBValueList(const vector<string> &val);
 
-  const string getRemaining() const {
+  bool eof() const { return true; };
+
+  std::string getRemaining() const {
      return "";
   }
+
+  #if defined(PDNS_AUTH) // [
+  /* This method is only there for parity with DNSParser::consumeRemaining(),
+     see the comment there to know why it is needed.
+  */
+  void consumeRemaining() const
+  {
+  }
+#endif // ]
 private:
   string& d_string;
   bool d_nodot;