rec: Backport 15010 to rec 5.2.x: fix protobufServer(.. {taggedOnly=true}) logic for cache-returned responses

pdns/recursordist/docs/lua-config/protobuf.rst

@@ -88,7 +88,6 @@ While :func:`protobufServer` only exports the queries sent to the recursor from
   * ``timeout=2``: int - Time in seconds to wait when sending a message
   * ``maxQueuedEntries=100``: int - How many entries will be kept in memory if the server becomes unreachable
   * ``reconnectWaitTime=1``: int - How long to wait, in seconds, between two reconnection attempts
-  * ``taggedOnly=false``: bool - Only entries with a policy or a policy tag set will be sent
   * ``asyncConnect``: bool - When set to false (default) the first connection to the server during startup will block up to ``timeout`` seconds, otherwise the connection is done in a separate thread, after the first message has been queued
   * ``logQueries=true``: bool - Whether to export queries
   * ``logResponses=true``: bool - Whether to export responses

pdns/recursordist/pdns_recursor.cc

@@ -2338,7 +2338,7 @@ static string* doProcessUDPQuestion(const std::string& question, const ComboAddr
         int sendErr = sendOnNBSocket(fileDesc, &msgh);
         eventTrace.add(RecEventTrace::AnswerSent);
 
-        if (t_protobufServers.servers && logResponse && (!luaconfsLocal->protobufExportConfig.taggedOnly || !pbData || pbData->d_tagged)) {
+        if (t_protobufServers.servers && logResponse && (!luaconfsLocal->protobufExportConfig.taggedOnly || (pbData && pbData->d_tagged))) {
           protobufLogResponse(dnsheader, luaconfsLocal, pbData, tval, false, source, destination, mappedSource, ednssubnet, uniqueId, requestorId, deviceId, deviceName, meta, eventTrace, policyTags);
         }
 

pdns/recursordist/rec-main.cc

@@ -2897,7 +2897,7 @@ static void recursorThread()
     threadInfo.setMT(g_multiTasker.get());
 
     {
-      /* start protobuf export threads if needed, don;'t keep a ref to lua config around */
+      /* start protobuf export threads if needed, don't keep a ref to lua config around */
       auto luaconfsLocal = g_luaconfs.getLocal();
       checkProtobufExport(luaconfsLocal);
       checkOutgoingProtobufExport(luaconfsLocal);

pdns/recursordist/rec-tcp.cc

@@ -461,7 +461,7 @@ static void doProcessTCPQuestion(std::unique_ptr<DNSComboWriter>& comboWriter, s
         t_Counters.at(rec::Histogram::cumulativeAnswers)(spentUsec);
         comboWriter->d_eventTrace.add(RecEventTrace::AnswerSent);
 
-        if (t_protobufServers.servers && comboWriter->d_logResponse && (!luaconfsLocal->protobufExportConfig.taggedOnly || !pbData || pbData->d_tagged)) {
+        if (t_protobufServers.servers && comboWriter->d_logResponse && (!luaconfsLocal->protobufExportConfig.taggedOnly || (pbData && pbData->d_tagged))) {
           struct timeval tval
           {
             0, 0

regression-tests.recursor-dnssec/test_Protobuf.py

@@ -962,58 +962,71 @@ def testA(self):
         expected = dns.rrset.from_text(name, 0, dns.rdataclass.IN, 'A', '192.0.2.42')
         query = dns.message.make_query(name, 'A', want_dnssec=True)
         query.flags |= dns.flags.CD
-        res = self.sendUDPQuery(query)
-        self.assertRRsetInAnswer(res, expected)
+        for method in ("sendUDPQuery", "sendTCPQuery"):
+          sender = getattr(self, method)
+          res = sender(query)
+          self.assertRRsetInAnswer(res, expected)
 
-        # check the protobuf message corresponding to the UDP response
-        # the first query and answer are not tagged, so there is nothing in the queue
-        time.sleep(1)
-        self.checkNoRemainingMessage()
+          # check the protobuf message corresponding to the UDP response
+          # the first query and answer are not tagged, so there is nothing in the queue
+          time.sleep(1)
+
+          self.checkNoRemainingMessage()
+          # Again to check PC case
+          res = sender(query)
+          time.sleep(1)
+          self.checkNoRemainingMessage()
 
     def testTagged(self):
         name = 'tagged.example.'
         expected = dns.rrset.from_text(name, 0, dns.rdataclass.IN, 'A', '192.0.2.84')
         query = dns.message.make_query(name, 'A', want_dnssec=True)
         query.flags |= dns.flags.CD
-        res = self.sendUDPQuery(query)
-        self.assertRRsetInAnswer(res, expected)
-
-        # check the protobuf messages corresponding to the UDP query and answer
-        msg = self.getFirstProtobufMessage()
-        self.checkProtobufQuery(msg, dnsmessage_pb2.PBDNSMessage.UDP, query, dns.rdataclass.IN, dns.rdatatype.A, name)
-        self.checkProtobufTags(msg, [ self._tag_from_gettag ])
-        # then the response
-        msg = self.getFirstProtobufMessage()
-        self.checkProtobufResponse(msg, dnsmessage_pb2.PBDNSMessage.UDP, res)
-        self.assertEqual(len(msg.response.rrs), 1)
-        rr = msg.response.rrs[0]
-        # we have max-cache-ttl set to 15
-        self.checkProtobufResponseRecord(rr, dns.rdataclass.IN, dns.rdatatype.A, name, 15)
-        self.assertEqual(socket.inet_ntop(socket.AF_INET, rr.rdata), '192.0.2.84')
-        tags = [ self._tag_from_gettag ] + self._tags
-        #print(msg)
-        self.checkProtobufTags(msg, tags)
-        self.checkNoRemainingMessage()
-
-        # Again to check PC case
-        res = self.sendUDPQuery(query)
-        self.assertRRsetInAnswer(res, expected)
-
-        # check the protobuf messages corresponding to the UDP query and answer
-        msg = self.getFirstProtobufMessage()
-        self.checkProtobufQuery(msg, dnsmessage_pb2.PBDNSMessage.UDP, query, dns.rdataclass.IN, dns.rdatatype.A, name)
-        self.checkProtobufTags(msg, [ self._tag_from_gettag ])
-        # then the response
-        msg = self.getFirstProtobufMessage()
-        self.checkProtobufResponse(msg, dnsmessage_pb2.PBDNSMessage.UDP, res)
-        self.assertEqual(len(msg.response.rrs), 1)
-        rr = msg.response.rrs[0]
-        # time may have passed, so do not check TTL
-        self.checkProtobufResponseRecord(rr, dns.rdataclass.IN, dns.rdatatype.A, name, 15, checkTTL=False)
-        self.assertEqual(socket.inet_ntop(socket.AF_INET, rr.rdata), '192.0.2.84')
-        tags = [ self._tag_from_gettag ] + self._tags
-        self.checkProtobufTags(msg, tags)
-        self.checkNoRemainingMessage()
+        first = True
+        for method in ("sendUDPQuery", "sendTCPQuery"):
+          messagetype = dnsmessage_pb2.PBDNSMessage.UDP
+          if not first:
+             messagetype = dnsmessage_pb2.PBDNSMessage.TCP
+          sender = getattr(self, method)
+          res = sender(query)
+          self.assertRRsetInAnswer(res, expected)
+
+          # check the protobuf messages corresponding to the query and answer
+          msg = self.getFirstProtobufMessage()
+          self.checkProtobufQuery(msg, messagetype, query, dns.rdataclass.IN, dns.rdatatype.A, name)
+          self.checkProtobufTags(msg, [ self._tag_from_gettag ])
+          # then the response
+          msg = self.getFirstProtobufMessage()
+          self.checkProtobufResponse(msg, messagetype, res)
+          self.assertEqual(len(msg.response.rrs), 1)
+          rr = msg.response.rrs[0]
+          # we have max-cache-ttl set to 15, but only check it first iteration
+          self.checkProtobufResponseRecord(rr, dns.rdataclass.IN, dns.rdatatype.A, name, 15, checkTTL=first)
+          self.assertEqual(socket.inet_ntop(socket.AF_INET, rr.rdata), '192.0.2.84')
+          tags = [ self._tag_from_gettag ] + self._tags
+          self.checkProtobufTags(msg, tags)
+          self.checkNoRemainingMessage()
+
+          # Again to check PC case
+          res = sender(query)
+          self.assertRRsetInAnswer(res, expected)
+
+          # check the protobuf messages corresponding to the query and answer
+          msg = self.getFirstProtobufMessage()
+          self.checkProtobufQuery(msg, messagetype, query, dns.rdataclass.IN, dns.rdatatype.A, name)
+          self.checkProtobufTags(msg, [ self._tag_from_gettag ])
+          # then the response
+          msg = self.getFirstProtobufMessage()
+          self.checkProtobufResponse(msg, messagetype, res)
+          self.assertEqual(len(msg.response.rrs), 1)
+          rr = msg.response.rrs[0]
+          # time may have passed, so do not check TTL
+          self.checkProtobufResponseRecord(rr, dns.rdataclass.IN, dns.rdatatype.A, name, 15, checkTTL=False)
+          self.assertEqual(socket.inet_ntop(socket.AF_INET, rr.rdata), '192.0.2.84')
+          tags = [ self._tag_from_gettag ] + self._tags
+          self.checkProtobufTags(msg, tags)
+          self.checkNoRemainingMessage()
+          first = False
 
 class ProtobufTagCacheBase(TestRecursorProtobuf):
     __test__ = False