diff options
author | Ryan Keane <rwkeane@google.com> | 2020-10-02 08:59:34 -0700 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2020-10-02 16:39:39 +0000 |
commit | be7345166a4f624a34e85972018a6728315673e2 (patch) | |
tree | 6edff909baff50815452698ea21723774dde9881 /discovery/mdns | |
parent | be82703d7841625b84506bc1302d879641548762 (diff) | |
download | openscreen-be7345166a4f624a34e85972018a6728315673e2.tar.gz |
[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>
Diffstat (limited to 'discovery/mdns')
-rw-r--r-- | discovery/mdns/mdns_querier.cc | 5 | ||||
-rw-r--r-- | discovery/mdns/mdns_querier_unittest.cc | 26 |
2 files changed, 31 insertions, 0 deletions
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(); |