[Discovery] Add Flag to Disable NSEC Record Receiving

This CL provides the embedder with a way to disable the NSEC negative
response optimization of mDNS. The main use cases of this flag are:

- A user (internal or external) finds a bug with NSEC records (either in
  our library OR in theirs), and it takes us some time to find/fix the
  bug, this will give a short-term workaround that can be used to
  unblock them until we (or they) can code up a fix and get it through
  code review. This  MdnsResonder interop bug is a good example - the
  user has been able to continue testing while the suppression patch is
  still making its way through code review.
- An external user wants to interop with a library we don't want to
  support for their specific use case (ie a personal implementation of
  mDNS that isn't widely used, a very old version of a library that has
  been deprecated and we don't want to support, etc) when they don't
  want to code up a libcast-library-wide fix (I know I'd fall into this
  bucket if I wanted to use it for a personal project). This will give
  that ability
- NSEC handling is one of the sticky points for interoperability of our
  library with standard DNS as opposed to mDNS (NSEC has a different
  meaning in that case. It's possible to differentiate the two per
  RFC6762, but it also calls out that if you receive the "DNS" one as
  opposed to "mDNS", you should pretend it's actually just the "mDNS"
  one ¯\_(ツ)_/¯), which some users may eventually want to use it for.
  It's hard to say the direction an Open Source project will go once
  external contributors can get their hands on it

Bug: b/168034815
Change-Id: Ib592eb869d1a8b91495120eaf7e7887ad6ba2cee
Reviewed-on: https://chromium-review.googlesource.com/c/openscreen/+/2414788
Commit-Queue: Ryan Keane <rwkeane@google.com>
Reviewed-by: Brandon Tolsch <btolsch@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>

3 files changed, 37 insertions, 0 deletions

diff --git a/discovery/common/config.h b/discovery/common/config.h
index 7bcdcac7..b1ef731a 100644
--- a/discovery/common/config.h
+++ b/discovery/common/config.h
@@ -5,6 +5,8 @@
 #ifndef DISCOVERY_COMMON_CONFIG_H_
 #define DISCOVERY_COMMON_CONFIG_H_
 
+#include <vector>
+
 #include "platform/base/interface_info.h"
 
 namespace openscreen {
@@ -90,6 +92,10 @@ struct Config {
   // prevent a malicious or misbehaving mDNS client from causing the memory
   // used by mDNS to grow in an unbounded fashion.
   int querier_max_records_cached = 1024;
+
+  // Sets the querier to ignore all NSEC negative response records received as
+  // responses to outgoing queries.
+  bool ignore_nsec_responses = false;
 };
 
 inline Config::NetworkInfo::AddressFamilies operator&(
diff --git a/discovery/mdns/mdns_querier.cc b/discovery/mdns/mdns_querier.cc
index 9ace0cd2..5d961bd4 100644
--- a/discovery/mdns/mdns_querier.cc
+++ b/discovery/mdns/mdns_querier.cc
@@ -459,6 +459,11 @@ void MdnsQuerier::ProcessRecord(const MdnsRecord& record) {
     return;
   }
 
+  // Ignore NSEC records if the embedder has configured us to do so.
+  if (config_.ignore_nsec_responses && record.dns_type() == DnsType::kNSEC) {
+    return;
+  }
+
   // Get the types which the received record is associated with. In most cases
   // this will only be the type of the provided record, but in the case of
   // NSEC records this will be all records which the record dictates the
diff --git a/discovery/mdns/mdns_querier_unittest.cc b/discovery/mdns/mdns_querier_unittest.cc
index e6742203..afa0cfc6 100644
--- a/discovery/mdns/mdns_querier_unittest.cc
+++ b/discovery/mdns/mdns_querier_unittest.cc
@@ -5,6 +5,7 @@
 #include "discovery/mdns/mdns_querier.h"
 
 #include <memory>
+#include <utility>
 
 #include "discovery/common/config.h"
 #include "discovery/common/testing/mock_reporting_client.h"
@@ -550,6 +551,31 @@ TEST_F(MdnsQuerierTest, CorrectCallbackCalledWhenNsecRecordReplacesNonNsec) {
   EXPECT_TRUE(ContainsRecord(querier.get(), nsec_record_created_, DnsType::kA));
 }
 
+TEST_F(MdnsQuerierTest,
+       NoCallbackCalledWhenNsecRecordWouldReplaceNonNsecButNsecDisabled) {
+  config_.ignore_nsec_responses = true;
+  std::unique_ptr<MdnsQuerier> querier = CreateQuerier();
+
+  // Set up so an A record has been received
+  StrictMock<MockRecordChangedCallback> callback;
+  querier->StartQuery(DomainName{"testing", "local"}, DnsType::kA,
+                      DnsClass::kIN, &callback);
+  EXPECT_CALL(callback,
+              OnRecordChanged(record0_created_, RecordChangedEvent::kCreated));
+  auto packet = CreatePacketWithRecord(record0_created_);
+  receiver_.OnRead(&socket_, std::move(packet));
+  testing::Mock::VerifyAndClearExpectations(&callback);
+  ASSERT_TRUE(ContainsRecord(querier.get(), record0_created_, DnsType::kA));
+  EXPECT_FALSE(
+      ContainsRecord(querier.get(), nsec_record_created_, DnsType::kA));
+
+  packet = CreatePacketWithRecord(nsec_record_created_);
+  receiver_.OnRead(&socket_, std::move(packet));
+  EXPECT_TRUE(ContainsRecord(querier.get(), record0_created_, DnsType::kA));
+  EXPECT_FALSE(
+      ContainsRecord(querier.get(), nsec_record_created_, DnsType::kA));
+}
+
 TEST_F(MdnsQuerierTest, CorrectCallbackCalledWhenNonNsecRecordReplacesNsec) {
   std::unique_ptr<MdnsQuerier> querier = CreateQuerier();