BIND 10 trac496, updated. ab2c79a8815be54d7f3482ea5608d2611b7de433 [trac496] Changes made to address review comments
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu Feb 3 12:19:16 UTC 2011
The branch, trac496 has been updated
via ab2c79a8815be54d7f3482ea5608d2611b7de433 (commit)
from 08b215209a79db8420d565adaec7309413b01ea7 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit ab2c79a8815be54d7f3482ea5608d2611b7de433
Author: Stephen Morris <stephen at isc.org>
Date: Thu Feb 3 12:15:12 2011 +0000
[trac496] Changes made to address review comments
In particular:
* Use Use asiolink abstractions instead of Boost::asio directly
* Add cross-section checks (answer/question v authority)
* Extended description of bailiwick
* Methods added to isc::dns::Message and asiolink::IOAddress
-----------------------------------------------------------------------
Summary of changes:
src/bin/auth/Makefile.am | 1 +
src/bin/auth/benchmarks/Makefile.am | 1 +
src/bin/auth/tests/Makefile.am | 1 +
src/bin/resolver/response_scrubber.cc | 175 +++++++--
src/bin/resolver/response_scrubber.h | 277 +++++++++++---
.../resolver/tests/response_scrubber_unittest.cc | 416 ++++++++++++++------
src/lib/asiolink/ioaddress.h | 41 ++-
src/lib/asiolink/ioendpoint.h | 1 +
src/lib/asiolink/tests/asiolink_unittest.cc | 17 +
src/lib/dns/message.cc | 5 +
src/lib/dns/message.h | 7 +
src/lib/dns/tests/message_unittest.cc | 21 +
12 files changed, 764 insertions(+), 199 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/auth/Makefile.am b/src/bin/auth/Makefile.am
index e9097f2..3b21abf 100644
--- a/src/bin/auth/Makefile.am
+++ b/src/bin/auth/Makefile.am
@@ -51,6 +51,7 @@ b10_auth_LDADD += $(top_builddir)/src/lib/cc/libcc.la
b10_auth_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
b10_auth_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
b10_auth_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la
+b10_auth_LDADD += $(top_builddir)/src/lib/log/liblog.la
b10_auth_LDADD += $(SQLITE_LIBS)
# TODO: config.h.in is wrong because doesn't honor pkgdatadir
diff --git a/src/bin/auth/benchmarks/Makefile.am b/src/bin/auth/benchmarks/Makefile.am
index 165bb4c..05ab754 100644
--- a/src/bin/auth/benchmarks/Makefile.am
+++ b/src/bin/auth/benchmarks/Makefile.am
@@ -20,5 +20,6 @@ query_bench_LDADD += $(top_builddir)/src/lib/datasrc/libdatasrc.la
query_bench_LDADD += $(top_builddir)/src/lib/config/libcfgclient.la
query_bench_LDADD += $(top_builddir)/src/lib/cc/libcc.la
query_bench_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la
+query_bench_LDADD += $(top_builddir)/src/lib/log/liblog.la
query_bench_LDADD += $(top_builddir)/src/lib/asiolink/libasiolink.la
query_bench_LDADD += $(SQLITE_LIBS)
diff --git a/src/bin/auth/tests/Makefile.am b/src/bin/auth/tests/Makefile.am
index 7298498..a1114e4 100644
--- a/src/bin/auth/tests/Makefile.am
+++ b/src/bin/auth/tests/Makefile.am
@@ -44,6 +44,7 @@ run_unittests_LDADD += $(top_builddir)/src/lib/config/libcfgclient.la
run_unittests_LDADD += $(top_builddir)/src/lib/cc/libcc.la
run_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libexceptions.la
run_unittests_LDADD += $(top_builddir)/src/lib/xfr/libxfr.la
+run_unittests_LDADD += $(top_builddir)/src/lib/log/liblog.la
endif
noinst_PROGRAMS = $(TESTS)
diff --git a/src/bin/resolver/response_scrubber.cc b/src/bin/resolver/response_scrubber.cc
index 4ed32de..060a8b1 100644
--- a/src/bin/resolver/response_scrubber.cc
+++ b/src/bin/resolver/response_scrubber.cc
@@ -13,82 +13,177 @@
// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
// PERFORMANCE OF THIS SOFTWARE.
+#include <iostream>
+#include <vector>
#include <dns/message.h>
#include <dns/rrset.h>
+#include <dns/name.h>
#include "response_scrubber.h"
using namespace isc::dns;
-using namespace asio::ip;
+using namespace std;
// Compare addresses etc.
-ResponseScrubber::Category ResponseScrubber::addressPortCheck(
- const udp::endpoint& to, const udp::endpoint& from)
+ResponseScrubber::Category ResponseScrubber::addressCheck(
+ const asiolink::IOEndpoint& to, const asiolink::IOEndpoint& from)
{
- if (from.address() == to.address()) {
- if (from.port() == to.port()) {
- return (ResponseScrubber::SUCCESS);
- }
- else {
- return (ResponseScrubber::PORT);
+ if (from.getProtocol() == to.getProtocol()) {
+ if (from.getAddress() == to.getAddress()) {
+ if (from.getPort() == to.getPort()) {
+ return (ResponseScrubber::SUCCESS);
+ } else {
+ return (ResponseScrubber::PORT);
+ }
+ } else {
+ return (ResponseScrubber::ADDRESS);
}
}
- return (ResponseScrubber::ADDRESS);
+ return (ResponseScrubber::PROTOCOL);
}
-// Scrub a section of the message
+// Do a general scrubbing. The QNAMES of RRsets in the specified section are
+// compared against the list of name given and if they are not equal and not in
+// the specified relationship (generally superdomain or subdomain) to at least
+// of of the given names, they are removed.
unsigned int
-ResponseScrubber::scrubSection(const Name& bailiwick, Message& message,
+ResponseScrubber::scrubSection(Message& message,
+ const vector<const Name*>& names,
+ const NameComparisonResult::NameRelation connection,
const Message::Section section)
{
- unsigned int count = 0;
- bool removed = true;
+ unsigned int count = 0; // Count of RRsets removed
+ unsigned int kept = 0; // Count of RRsets kept
+ bool removed = true; // Set true if RRset removed in a pass
+
+ // Need to go through the section multiple times as when an RRset is
+ // removed, all iterators into the section are invalidated. This condition
+ // is flagged by "remove" being set true when an RRset is removed.
- // Need to iterate multiple times as removing an RRset from the section
- // invalidates the iterators.
while (removed) {
+ RRsetIterator i = message.beginSection(section);
+
+ // Skips the ones that have been checked (and retained) in a previous
+ // pass through the "while" loop. (Although RRset removal invalidates
+ // iterators, it does not change the relative order of the retained
+ // RRsets in the section.)
+ for (int j = 0; j < kept; ++j) {
+ ++i;
+ }
+
+ // Start looking at the remaining entries in the section.
removed = false;
- for (RRsetIterator i = message.beginSection(section);
- i != message.endSection(section); ++i) {
- NameComparisonResult result = (*i)->getName().compare(bailiwick);
- NameComparisonResult::NameRelation relation =
+ for (; (i != message.endSection(section)) && (!removed); ++i) {
+
+ // Loop through the list of names given and see if any are in the
+ // given relationship with the QNAME of this RRset
+ bool nomatch = true;
+ for (vector<const Name*>::const_iterator n = names.begin();
+ ((n != names.end()) && nomatch); ++n) {
+ NameComparisonResult result = (*i)->getName().compare(**n);
+ NameComparisonResult::NameRelation relationship =
result.getRelation();
- if ((relation != NameComparisonResult::EQUAL) &&
- (relation != NameComparisonResult::SUBDOMAIN)) {
+ if ((relationship == NameComparisonResult::EQUAL) ||
+ (relationship == connection)) {
- // Name is a superdomain of the bailiwick name or has a
- // common ancestor somewhere in the chain. Either way it's
- // not in bailiwick and we should remove this name from the
- // message section.
+ // RRset in the specified relationship, so a match has
+ // been found
+ nomatch = false;
+ }
+ }
+
+ // Remove the RRset if there was no match to one of the given names.
+ if (nomatch) {
message.removeRRset(section, i);
++count; // One more RRset removed
removed = true; // Something was removed
-
- // Must now work through the section again because the
- // removal of the RRset has invalidated the iterators.
- break;
- }
- }
-
+ } else {
+ // There was a match so this is one more entry we can skip next
+ // time.
+ ++kept;
+ }
+ }
}
+
return count;
}
-// Perform the scrubbing of the message.
+// Perform the scrubbing of all sections of the message.
unsigned int
-ResponseScrubber::scrub(const Name& bailiwick, Message& message) {
+ResponseScrubber::scrubAllSections(Message& message, const Name& bailiwick) {
// Leave the question section alone. Just go through the RRsets in the
- // answer, authority and additional sectiuons.
+ // answer, authority and additional sections.
unsigned int count = 0;
-
- count += scrubSection(bailiwick, message, Message::SECTION_ANSWER);
- count += scrubSection(bailiwick, message, Message::SECTION_AUTHORITY);
- count += scrubSection(bailiwick, message, Message::SECTION_ADDITIONAL);
+ const vector<const Name*> bailiwick_names(1, &bailiwick);
+ count += scrubSection(message, bailiwick_names,
+ NameComparisonResult::SUBDOMAIN, Message::SECTION_ANSWER);
+ count += scrubSection(message, bailiwick_names,
+ NameComparisonResult::SUBDOMAIN, Message::SECTION_AUTHORITY);
+ count += scrubSection(message, bailiwick_names,
+ NameComparisonResult::SUBDOMAIN, Message::SECTION_ADDITIONAL);
return count;
}
+// Scrub across sections.
+
+unsigned int
+ResponseScrubber::scrubCrossSections(isc::dns::Message& message) {
+
+ // Get a list of the names in the answer section or, failing this, the
+ // question section. Note that pointers to the names within "message" are
+ // stored; this is OK as the relevant sections in "message" will not change
+ // during the lifetime of this method (it only affects the authority
+ // section).
+ vector<const Name*> source;
+ if (message.getRRCount(Message::SECTION_ANSWER) != 0) {
+ for (RRsetIterator i = message.beginSection(Message::SECTION_ANSWER);
+ i != message.endSection(Message::SECTION_ANSWER); ++i) {
+ const Name& qname = (*i)->getName();
+ source.push_back(&qname);
+ }
+
+ } else {
+ for (QuestionIterator i = message.beginQuestion();
+ i != message.endQuestion(); ++i) {
+ const Name& qname = (*i)->getName();
+ source.push_back(&qname);
+ }
+ }
+
+ if (source.empty()) {
+ // TODO: Log the fact - should be at least a question present
+ return (0);
+ }
+
+ // Could be duplicates, especially in the answer section, so sort the
+ // names and remove them.
+ sort(source.begin(), source.end(), ResponseScrubber::compareNameLt);
+ vector<const Name*>::iterator endunique =
+ unique(source.begin(), source.end(), ResponseScrubber::compareNameEq);
+ source.erase(endunique, source.end());
+
+ // Now purge the authority section of RRsets that are not equal to or a
+ // superdomain of the names in the question/answer section.
+ return (scrubSection(message, source,
+ NameComparisonResult::SUPERDOMAIN, Message::SECTION_AUTHORITY));
+
+}
+
+// Scrub a message
+
+unsigned int
+ResponseScrubber::scrub(const isc::dns::MessagePtr& message,
+ const isc::dns::Name& bailiwick)
+{
+ unsigned int sections_removed = scrubAllSections(*message, bailiwick);
+ sections_removed += scrubCrossSections(*message);
+
+ return sections_removed;
+}
+
+
diff --git a/src/bin/resolver/response_scrubber.h b/src/bin/resolver/response_scrubber.h
index 5aba823..971de64 100644
--- a/src/bin/resolver/response_scrubber.h
+++ b/src/bin/resolver/response_scrubber.h
@@ -19,13 +19,13 @@
/// \page DataScrubbing Data Scrubbing
/// \section DataScrubbingIntro Introduction
-/// When a response is received from an authoritative server, it needs to be
-/// checked to ensure that the data contained in it is valid. If all the data
-/// is signed, this is not a problem - validating the signatures is a sufficient
-/// check. But an unsigned response is more of a problem. How do we check
-/// that the response from the server to which we sent it? And if we pass that
-/// hurdle, how do we know that the information is correct and that the server
-/// is not attempting to spoof us?
+/// When a response is received from an authoritative server, it should be
+/// checked to ensure that the data contained in it is valid. Signed data is
+/// not a problem - validating the signatures is a sufficient check. But
+/// unsigned data in a response is more of a problem. (Note that even data from
+/// signed zones may be not be signed, e.g. delegations are not signed.) In
+/// particular, how do we know that the server from which the response was
+/// received was authoritive for the data it returned?
///
/// The part of the code that checks for this is the "Data Scrubbing" module.
/// Although it includes the checking of IP addresses and ports, it is called
@@ -35,36 +35,109 @@
/// \section DataScrubbingBasic Basic Checks
/// The first part - how do we know that the response comes from the correct
/// server - is relatively trivial, albeit not foolproof (which is why DNSSEC
-/// came about). The following are checked:
+/// was developed). The following are checked:
///
-/// # The IP address from which the response was received is the same as the
+/// - The IP address from which the response was received is the same as the
/// one to which the query was sent.
-/// # The port on which the response was received is the same as the one from
+/// - The port on which the response was received is the same as the one from
/// which the query was sent.
-/// # The QID in the response message is the same as the QID in the query
-/// message sent.
///
-/// (The first two tests are not done for a TCP connection - if data is received
+/// (These tests need not not done for a TCP connection - if data is received
/// over the TCP stream, it is assumed that it comes from the address and port
/// to which a connection was made.)
///
+/// - The protocol used to send the question is the same as the protocol on
+/// which an answer was received.
+///
+/// (Strictly speaking, if this check fails it is a programming error - the
+/// code should not mix up UPD and TCP messages.)
+///
+/// - The QID in the response message is the same as the QID in the query
+/// message sent.
+///
/// If the conditions are met, then the data - in all three response sections -
/// is scanned and out of bailiwick data is removed ("scrubbed").
///
/// \section DataScrubbingBailiwick Bailiwick
/// Bailiwick means "district or jurisdiction of bailie or bailiff" (Concise
/// Oxford Dictionary, 7th Edition). It is not a term mentioned in any RFC
-/// (or at least, any RFC up to RFC 5997), but in the context of DNS is taken
-/// to mean the data for which a DNS server has authority.
+/// (or at least, any RFC up to RFC 5997) but is widely used in DNS literature.
+/// In this context it is taken to mean the data for which a DNS server has
+/// authority. So when we speak of the information being "in bailiwick", we
+/// mean that the the server is the ultimate source of authority for that data.
+///
+/// In practice, determining this from the response alone is difficult. In
+/// particular, as a server may be authoritative for many zones, it could in
+/// theory be authoritative for any combination of RRsets that appear in a
+/// response.
+///
+/// For this reason, bailiwick is dependent on the query. If, for example, a
+/// query for www.example.com is sent to the nameservers for example.com
+/// (because of a referral of from the com. servers), the bailiwick for the
+/// query is example.com. This means that any information returned on domains
+/// other than example.com may not be authoritative. More exactly, it may be
+/// authoritative (because the server is also authoritative for the zone
+/// concerned), but based on the information available (in this example, that
+/// the response originated from a nameserver for the zone example.com) it is
+/// not possible to be certain.
+///
+/// Ideally, out of bailiwick data should be excluded from further processing
+/// as it may be incorrect and corrupt the cache. In practice, there are
+/// two cases to consider:
+///
+/// The first is when the data has a qname that is not example.com or a
+/// subdomain of it (e.g. xyz.com, www.example.net). In this case the data can
+/// be retrieved by an independent query - no path from the root zone to the
+/// data goes through the current bailiwick, so there is no chance of ending up
+/// in a loop. In this case, data that appears to be out of bailiwick can be
+/// dropped from the response.
+///
+/// The second case is when the QNAME of the data is a subdomain of the
+/// bailiwick. Here the server may or may not be authoritative for the data.
+/// For example, if the name queried for were www.sub.example.com and the
+/// example.com nameservers supplied an answer:
+///
+/// - The answer could be authoritative - www.sub.example.com could be
+/// in the example.com zone.
+/// - The answer might not be authoritative - the zone sub.example.com may have
+/// been delegated, so the authoritative answer should come from
+/// sub.example.com's nameservers.
+/// - The answer might be authoritative even though zone sub.example.com has
+/// been delegated, because the nameserver for example.com is the same as
+/// that for sub.example.com.
+///
+/// Unlike the previous case, it is not possible to err on the side of caution
+/// and drop such data. Any independent query for it will pass through the
+/// current bailiwick and the same question will be asked again. For this
+/// reason, any data in the response that has a QNAME equal to a subdomain of
+/// the bailiwick has to be accepted.
+///
+/// In summary then, data in a response that has a QNAME equal to or a subdomain
+/// of the bailiwick is considered in-bailiwick. Anything else is out of of
+/// bailiwick.
///
-/// What this really means is, given a record returned from that server, could
-/// a properly configured server _really_ have returned it? And if so, is the
-/// server authoritiative for that data? The only answer that satisfies both
-/// criteria is a resource record where the QNAME is the same as or a subdomain
-/// of the domain of the nameservers being queried.
+/// \subsection DataScrubbingCrossSection Cross-Section Scrubbing
+/// Even with the bailiwick checks above, there are some additional cleaning
+/// that can be done with the packet. In particular:
///
+/// - The QNAMEs of the RRsets in the authority section must be equal to or
+/// superdomains of a QNAME of an RRset in the answer. Any that are not
+/// should be removed.
+/// - If there is no answer section, the QNAMES of RRsets in the authority
+/// section must be equal to or superdomains of the QNAME of the RRset in the
+/// question.
+///
+/// Although previous checks should have removed some inconsistencies, it
+/// will not trap obscure cases (e.g. bailiwick: "example.com", answer:
+/// "www.example.com", authority: sub.example.com). These checks do just that.
+///
+/// (Note that not included here is QNAME of question not equal to or a
+/// superdomain of the answer; that check is made in the ResponseClassifier
+/// class.)
+///
+/// \section DataScrubbingExample Examples
/// Some examples should make this clear: they all use the notation
-/// Qu = Question, Zo = Zone being asked, An = Answer, Au = Authority,
+/// Qu = Question, Zo = Zone being queried, An = Answer, Au = Authority,
/// Ad = Additional.
///
/// \subsection DataScrubbingEx1 Example 1: Simple Query
@@ -78,7 +151,7 @@
/// An: www.example.com A 192.0.2.1
///
/// Au(1): example.com NS ns0.example.com\n
-/// Au(2): example.com NS ns1.example.netipCheck
+/// Au(2): example.com NS ns1.example.net
///
/// Ad(1): ns0.example.com A 192.0.2.100\n
/// Ad(2): ns1.example.net A 192.0.2.200
@@ -147,10 +220,32 @@
/// zone could well be serving all the data for example.com. Having said that,
/// the A record for ns1.example.net would still be regarded as being out of
/// bailiwick becase the nameserver is not authoritative for the .net zone.
+///
+/// \subsection DataScrubbingEx4 Example 4: Inconsistent Answer Section
+/// Qu: www.example.com\n
+/// Zo: example.com
+///
+/// An: www.example.com A 192.0.2.1
+///
+/// Au(1): alpha.example.com NS ns0.example.com\n
+/// Au(2): alpha.example.com NS ns1.example.net
+///
+/// Ad(1): ns0.example.com A 192.0.2.100\n
+/// Ad(2): ns1.example.net A 192.0.2.200
+///
+/// Here, everything in the answer and authority sections is in bailiwick for
+/// the example.com server. And although the zone example.com was queried, it
+/// is permissible for the authority section to contain nameservers with a
+/// qname that is a subdomain of example.com (e.g. see \ref DataScrubbingEx2).
+/// However, only servers with a qname that is equal to or a superdomain of
+/// the answer are authoritative for the answer. So in this case, both
+/// Au(1) and Au(2) (as well as Ad(2), for reasons given earlier) will be
+/// scrubbed.
#include <config.h>
-#include <asio.hpp>
+#include <asiolink/ioendpoint.h>
#include <dns/message.h>
+#include <dns/name.h>
/// \brief Response Data Scrubbing
///
@@ -158,6 +253,9 @@
/// message and some additional information, it checks the information using
/// the rules given in \ref DataScrubbing and either rejects the packet or
/// modifies it to remove non-conforming RRsets.
+///
+/// TODO: Examine the additional records and remove all cases where the
+/// QNAME does not match the RDATA of records in the authority section.
class ResponseScrubber {
public:
@@ -168,24 +266,24 @@ public:
// Error categories
- ADDRESS, ///< Mismatching IP address
- PORT ///< Mismatching port
+ ADDRESS = 1, ///< Mismatching IP address
+ PORT = 2, ///< Mismatching port
+ PROTOCOL = 3 ///< Mismatching protocol
};
/// \brief Check IP Address
///
- /// Compares the address to which the query was sent and the port it was
- /// sent from with the address from which the query was received and the
- /// port it was sent to.
- ///
- /// This is only required of a UDP connection - it is assumed that data that
- /// data received on a TCP stream is received from the system to which the
- /// connection was made.
+ /// Compares the address to which the query was sent, the port it was
+ /// sent from, and the protocol used for communication with the (address,
+ /// port, protocol) from which the response was received.
///
/// \param to Endpoint representing the address to which the query was sent.
/// \param from Endpoint from which the response was received.
- static Category addressPortCheck(const asio::ip::udp::endpoint& to,
- const asio::ip::udp::endpoint& from);
+ ///
+ /// \return SUCCESS if the two endpoints match, otherwise an error status
+ /// indicating what was incorrect.
+ static Category addressCheck(const asiolink::IOEndpoint& to,
+ const asiolink::IOEndpoint& from);
/// \brief Check QID
///
@@ -200,38 +298,125 @@ public:
return (sent.getQid() == received.getQid());
}
- /// \brief Scrub Message Section
+ /// \brief Generalised Scrub Message Section
+ ///
+ /// When scrubbing a message given the bailiwick of the server, RRsets are
+ /// retained in the message section if the QNAME is equal to or a subdomain
+ /// of the bailiwick. However, when checking QNAME of RRsets in the
+ /// authority section against the QNAME of the question or answers, RRsets
+ /// are retained only if their QNAME is equal to or a superdomain of the
+ /// name in question.
///
- /// Scrubs the message section by removing all RRsets that are not in the
- /// bailiwick of the authoritative server from the message.
+ /// This method provides the generalised scrubbing whereby the RRsets in
+ /// a section are tested against a given name, and RRsets kept if their
+ /// QNAME is equal to or in the supplied relationship with the given name.
///
- /// \param zone Name of the zone whose authoritative servers were queried.
- /// \param section Section of the message to be scrubbed
+ /// \param section Section of the message to be scrubbed.
+ /// \param zone Names against which RRsets should be checked. Note that
+ /// this is a vector of pointers to Name objects; they are assumed to
+ /// independently exist, and the caller retains ownership of them and is
+ /// assumed to destroy them when needed.
+ /// \param connection Relationship required for retention, i.e. the QNAME of
+ /// an RRset in the specified section must be equal to or a "connection"
+ /// (SUPERDOMAIN/SUBDOMAIN) of "name" for the RRset to be retained.
/// \param message Message to be scrubbed.
///
/// \return Count of the number of RRsets removed from the section.
- static unsigned int scrubSection(const isc::dns::Name& bailiwick,
- isc::dns::Message& message, const isc::dns::Message::Section section);
+ static unsigned int scrubSection(isc::dns::Message& message,
+ const std::vector<const isc::dns::Name*>& names,
+ const isc::dns::NameComparisonResult::NameRelation connection,
+ const isc::dns::Message::Section section);
- /// \brief Scrub Message
+ /// \brief Scrub All Sections of a Message
///
/// Scrubs each of the answer, authority and additional sections of the
/// message.
///
/// No distinction is made between RRsets legitimately in the message (e.g.
- /// glue for authorities that are not in bailiwick) and onces that could be
+ /// glue for authorities that are not in bailiwick) and ones that could be
/// considered as attempts of spoofing (e.g. non-bailiwick RRsets in the
/// additional section that are not related to the query).
///
/// The resultant packet returned to the caller may be invalid. If so, it
/// is up to the caller to detect that.
///
- /// \param zone Name of the zone whose authoritative servers were queried.
+ /// \param message Message to be scrubbed.
+ /// \param bailiwick Name of the zone whose authoritative servers were
+ /// queried.
+ ///
+ /// \return Count of the number of RRsets removed from the message.
+ static unsigned int scrubAllSections(isc::dns::Message& message,
+ const isc::dns::Name& bailiwick);
+
+ /// \brief Scrub Across Message Sections
+ ///
+ /// Does some cross-section comparisons and removes inconsistent RRs. In
+ /// particular it:
+ ///
+ /// - If an answer is present, checks that the qname of the authority RRs
+ /// are equal to or superdomain of the qname answer RRsets. Any that are
+ /// not are removed.
+ /// - If an answer is not present, checks that the authority RRs are
+ /// equal to or superdomains of the question. If not, the authority RRs
+ /// are removed.
+ ///
+ /// Note that the scrubbing does not check:
+ ///
+ /// - that the question is in the bailiwick of the server; that check is
+ /// assumed to have been done prior to the query being sent (else why
+ /// was the query sent there in the first place?)
+ /// - that the qname of one of the RRsets in the answer (if present) is
+ /// equal to the qname of the question (that check is done in the
+ /// response classification code).
+ ///
/// \param message Message to be scrubbed.
///
/// \return Count of the number of RRsets removed from the section.
- static unsigned int scrub(const isc::dns::Name& bailiwick,
- isc::dns::Message& message);
+ static unsigned int scrubCrossSections(isc::dns::Message& message);
+
+ /// \brief Main Scrubbing Entry Point
+ ///
+ /// The single entry point to the module to sanitise the message. All
+ /// it does is call the various other scrubbing methods.
+ ///
+ /// \param message Pointer to the message to be scrubbed. (This is a
+ /// pointer - as opposed to a Message as in other methods in this class -
+ /// as the external code is expected to be mainly using message pointers
+ /// to access messages.)
+ /// \param bailiwick Name of the zone whose authoritative servers were
+ /// queried.
+ ///
+ /// \return Count of the number of RRsets removed from the message.
+ static unsigned int scrub(const isc::dns::MessagePtr& message,
+ const isc::dns::Name& bailiwick);
+
+ /// \brief Comparison Function for Sorting Name Pointers
+ ///
+ /// Utility method called to sorts pointers to names in lexical order.
+ ///
+ /// \param n1 Pointer to first Name object
+ /// \param n2 Pointer to second Name object
+ ///
+ /// \return true if n1 is less than n2, false otherwise.
+ static bool compareNameLt(const isc::dns::Name* n1,
+ const isc::dns::Name* n2)
+ {
+ return (*n1 < *n2);
+ }
+
+ /// \brief Function for Comparing Name Pointers
+ ///
+ /// Utility method called to sorts pointers to names in lexical order.
+ ///
+ /// \param n1 Pointer to first Name object
+ /// \param n2 Pointer to second Name object
+ ///
+ /// \return true if n1 is equal to n2, false otherwise.
+ static bool compareNameEq(const isc::dns::Name* n1,
+ const isc::dns::Name* n2)
+ {
+ return (*n1 == *n2);
+ }
};
#endif // __RESPONSE_SCRUBBER_H
diff --git a/src/bin/resolver/tests/response_scrubber_unittest.cc b/src/bin/resolver/tests/response_scrubber_unittest.cc
index 5e590cd..cc009dc 100644
--- a/src/bin/resolver/tests/response_scrubber_unittest.cc
+++ b/src/bin/resolver/tests/response_scrubber_unittest.cc
@@ -14,10 +14,17 @@
// $Id$
+#include <string>
+#include <iostream>
+
#include <gtest/gtest.h>
#include <config.h>
-#include <asio.hpp>
+
+#include <asiolink/ioendpoint.h>
+#include <asiolink/ioaddress.h>
+#include <netinet/in.h>
+
#include <dns/name.h>
#include <dns/opcode.h>
#include <dns/question.h>
@@ -30,27 +37,73 @@
#include <dns/rrttl.h>
#include <resolver/response_scrubber.h>
+
+// Class for endpoint checks. The family of the endpoint is set in the
+// constructor; the address family by the string provided for the address.
+
+namespace asiolink {
+
+class GenericEndpoint : public IOEndpoint {
+public:
+ GenericEndpoint(const std::string& address, uint16_t port, short protocol) :
+ address_(address), port_(port), protocol_(protocol)
+ {}
+ virtual ~GenericEndpoint()
+ {}
+
+ virtual IOAddress getAddress() const {
+ return address_;
+ }
+
+ virtual uint16_t getPort() const {
+ return port_;
+ }
+
+ virtual short getProtocol() const {
+ return protocol_;
+ }
+
+ virtual short getFamily() const {
+ return address_.getFamily();
+ }
+
+private:
+ IOAddress address_; // Address of endpoint
+ uint16_t port_; // Port number of endpoint
+ short protocol_; // Protocol of the endpoint
+ };
+}
+
using namespace asio::ip;
using namespace isc::dns;
using namespace rdata;
using namespace isc::dns::rdata::generic;
using namespace isc::dns::rdata::in;
+using namespace asiolink;
+
+// Test class
namespace {
class ResponseScrubberTest : public ::testing::Test {
public:
ResponseScrubberTest() :
bailiwick("example.com"),
+
qu_in_any_www(Name("www.example.com"), RRClass::IN(), RRType::ANY()),
qu_in_a_www(Name("www.example.com"), RRClass::IN(), RRType::A()),
qu_in_ns(Name("example.com"), RRClass::IN(), RRType::NS()),
qu_in_txt_www(Name("www.example.com"), RRClass::IN(), RRType::TXT()),
rrs_in_a_org(new RRset(Name("mail.example.org"), RRClass::IN(),
RRType::A(), RRTTL(300))),
+
rrs_in_a_net(new RRset(Name("mail.example.net"), RRClass::IN(),
RRType::A(), RRTTL(300))),
rrs_in_a_www(new RRset(Name("www.example.com"), RRClass::IN(),
RRType::A(), RRTTL(300))),
+ rrs_in_cname_www(new RRset(Name("www.example.com"), RRClass::IN(),
+ RRType::CNAME(), RRTTL(300))),
+ rrs_in_a_wwwnet(new RRset(Name("www.example.net"), RRClass::IN(),
+ RRType::A(), RRTTL(300))),
rrs_in_ns(new RRset(Name("example.com"), RRClass::IN(),
RRType::NS(), RRTTL(300))),
rrs_in_ns_com(new RRset(Name("com"), RRClass::IN(),
@@ -59,6 +112,8 @@ public:
RRType::NS(), RRTTL(300))),
rrs_in_ns_sub(new RRset(Name("subdomain.example.com"), RRClass::IN(),
RRType::NS(), RRTTL(300))),
+ rrs_in_ns_sub2(new RRset(Name("subdomain2.example.com"), RRClass::IN(),
+ RRType::NS(), RRTTL(300))),
rrs_in_a_ns0(new RRset(Name("ns0.example.com"), RRClass::IN(),
RRType::A(), RRTTL(300))),
rrs_in_a_ns1(new RRset(Name("ns1.com"), RRClass::IN(),
@@ -78,10 +133,13 @@ public:
RRsetPtr rrs_in_a_org; // mail.example.org IN A
RRsetPtr rrs_in_a_net; // mail.example.org IN A
RRsetPtr rrs_in_a_www; // www.example.com IN A
+ RRsetPtr rrs_in_cname_www; // www.example.com IN CNAME
+ RRsetPtr rrs_in_a_wwwnet; // www.example.net IN A
RRsetPtr rrs_in_ns; // example.com IN NS
RRsetPtr rrs_in_ns_com; // com IN NS
RRsetPtr rrs_in_ns_net; // example.net IN NS
RRsetPtr rrs_in_ns_sub; // subdomain.example.com IN NS
+ RRsetPtr rrs_in_ns_sub2; // subdomain2.example.com IN NS
RRsetPtr rrs_in_a_ns0; // ns0.example.com IN A
RRsetPtr rrs_in_a_ns1; // ns1.com IN A
RRsetPtr rrs_in_a_ns2; // ns2.example.net IN A
@@ -96,94 +154,141 @@ public:
TEST_F(ResponseScrubberTest, UDPv4) {
// Basic UDP Endpoint
- udp::endpoint udp_a;
- udp_a.address(address_v4::from_string("192.0.2.1"));
- udp_a.port(12345);
+ GenericEndpoint udp_a("192.0.2.1", 12345, IPPROTO_UDP);
- // Same address and port
- udp::endpoint udp_b;
- udp_b.address(address_v4::from_string("192.0.2.1"));
- udp_b.port(12345);
- ASSERT_EQ(ResponseScrubber::SUCCESS,
- ResponseScrubber::addressPortCheck(udp_a, udp_b));
+ // Same address, port
+ GenericEndpoint udp_b("192.0.2.1", 12345, IPPROTO_UDP);
+ EXPECT_EQ(ResponseScrubber::SUCCESS,
+ ResponseScrubber::addressCheck(udp_a, udp_b));
// Different address, same port
- udp::endpoint udp_c;
- udp_c.address(address_v4::from_string("192.0.2.2"));
- udp_c.port(12345);
- ASSERT_EQ(ResponseScrubber::ADDRESS,
- ResponseScrubber::addressPortCheck(udp_a, udp_c));
+ GenericEndpoint udp_c("192.0.2.2", 12345, IPPROTO_UDP);
+ EXPECT_EQ(ResponseScrubber::ADDRESS,
+ ResponseScrubber::addressCheck(udp_a, udp_c));
// Same address, different port
- udp::endpoint udp_d;
- udp_d.address(address_v4::from_string("192.0.2.1"));
- udp_d.port(12346);
- ASSERT_EQ(ResponseScrubber::PORT,
- ResponseScrubber::addressPortCheck(udp_a, udp_d));
+ GenericEndpoint udp_d("192.0.2.1", 12346, IPPROTO_UDP);
+ EXPECT_EQ(ResponseScrubber::PORT,
+ ResponseScrubber::addressCheck(udp_a, udp_d));
// Different address, different port
- udp::endpoint udp_e;
- udp_e.address(address_v4::from_string("192.0.2.3"));
- udp_e.port(12347);
- ASSERT_EQ(ResponseScrubber::ADDRESS,
- ResponseScrubber::addressPortCheck(udp_a, udp_e));
+ GenericEndpoint udp_e("192.0.2.3", 12347, IPPROTO_UDP);
+ EXPECT_EQ(ResponseScrubber::ADDRESS,
+ ResponseScrubber::addressCheck(udp_a, udp_e));
}
-// Repeat the tests for IPv6
+// Repeat the tests for TCP
+
+TEST_F(ResponseScrubberTest, TCPv4) {
+
+ // Basic TCP Endpoint
+ GenericEndpoint tcp_a("192.0.2.1", 12345, IPPROTO_TCP);
+
+ // Same address, port
+ GenericEndpoint tcp_b("192.0.2.1", 12345, IPPROTO_TCP);
+ EXPECT_EQ(ResponseScrubber::SUCCESS,
+ ResponseScrubber::addressCheck(tcp_a, tcp_b));
+
+ // Different address, same port
+ GenericEndpoint tcp_c("192.0.2.2", 12345, IPPROTO_TCP);
+ EXPECT_EQ(ResponseScrubber::ADDRESS,
+ ResponseScrubber::addressCheck(tcp_a, tcp_c));
+
+ // Same address, different port
+ GenericEndpoint tcp_d("192.0.2.1", 12346, IPPROTO_TCP);
+ EXPECT_EQ(ResponseScrubber::PORT,
+ ResponseScrubber::addressCheck(tcp_a, tcp_d));
+
+ // Different address, different port
+ GenericEndpoint tcp_e("192.0.2.3", 12347, IPPROTO_TCP);
+ EXPECT_EQ(ResponseScrubber::ADDRESS,
+ ResponseScrubber::addressCheck(tcp_a, tcp_e));
+
+}
+
+// Repeat the tests for UDP/IPv6
TEST_F(ResponseScrubberTest, UDPv6) {
// Basic UDP Endpoint
- udp::endpoint udp_a;
- udp_a.address(address_v6::from_string("2001:db8::1"));
- udp_a.port(12345);
+ GenericEndpoint udp_a("2001:db8::1", 12345, IPPROTO_UDP);
// Same address and port
- udp::endpoint udp_b;
- udp_b.address(address_v6::from_string("2001:db8::1"));
- udp_b.port(12345);
- ASSERT_EQ(ResponseScrubber::SUCCESS,
- ResponseScrubber::addressPortCheck(udp_a, udp_b));
+ GenericEndpoint udp_b("2001:db8::1", 12345, IPPROTO_UDP);
+ EXPECT_EQ(ResponseScrubber::SUCCESS,
+ ResponseScrubber::addressCheck(udp_a, udp_b));
// Different address, same port
- udp::endpoint udp_c;
- udp_c.address(address_v6::from_string("2001:db8::2"));
- udp_c.port(12345);
- ASSERT_EQ(ResponseScrubber::ADDRESS,
- ResponseScrubber::addressPortCheck(udp_a, udp_c));
+ GenericEndpoint udp_c("2001:db8::3", 12345, IPPROTO_UDP);
+ EXPECT_EQ(ResponseScrubber::ADDRESS,
+ ResponseScrubber::addressCheck(udp_a, udp_c));
// Same address, different port
- udp::endpoint udp_d;
- udp_d.address(address_v6::from_string("2001:db8::1"));
- udp_d.port(12346);
- ASSERT_EQ(ResponseScrubber::PORT,
- ResponseScrubber::addressPortCheck(udp_a, udp_d));
+ GenericEndpoint udp_d("2001:db8::1", 12346, IPPROTO_UDP);
+ EXPECT_EQ(ResponseScrubber::PORT,
+ ResponseScrubber::addressCheck(udp_a, udp_d));
// Different address, different port
- udp::endpoint udp_e;
- udp_e.address(address_v6::from_string("2001:db8::3"));
- udp_e.port(12347);
- ASSERT_EQ(ResponseScrubber::ADDRESS,
- ResponseScrubber::addressPortCheck(udp_a, udp_e));
+ GenericEndpoint udp_e("2001:db8::3", 12347, IPPROTO_UDP);
+ EXPECT_EQ(ResponseScrubber::ADDRESS,
+ ResponseScrubber::addressCheck(udp_a, udp_e));
}
-// Ensure that mixed IPv4/6 addresses don't match.
+// Same again for TCP/IPv6
-TEST_F(ResponseScrubberTest, UDPv4v6) {
+TEST_F(ResponseScrubberTest, TCPv6) {
- // Basic UDP Endpoint
- udp::endpoint udp_a;
- udp_a.address(address_v4::from_string("192.0.2.1"));
- udp_a.port(12345);
+ // Basic TCP Endpoint
+ GenericEndpoint tcp_a("2001:db8::1", 12345, IPPROTO_TCP);
// Same address and port
- udp::endpoint udp_b;
- udp_b.address(address_v6::from_string("2001:db8::1"));
- udp_b.port(12345);
- ASSERT_EQ(ResponseScrubber::ADDRESS,
- ResponseScrubber::addressPortCheck(udp_a, udp_b));
+ GenericEndpoint tcp_b("2001:db8::1", 12345, IPPROTO_TCP);
+ EXPECT_EQ(ResponseScrubber::SUCCESS,
+ ResponseScrubber::addressCheck(tcp_a, tcp_b));
+
+ // Different address, same port
+ GenericEndpoint tcp_c("2001:db8::3", 12345, IPPROTO_TCP);
+ EXPECT_EQ(ResponseScrubber::ADDRESS,
+ ResponseScrubber::addressCheck(tcp_a, tcp_c));
+
+ // Same address, different port
+ GenericEndpoint tcp_d("2001:db8::1", 12346, IPPROTO_TCP);
+ EXPECT_EQ(ResponseScrubber::PORT,
+ ResponseScrubber::addressCheck(tcp_a, tcp_d));
+
+ // Different address, different port
+ GenericEndpoint tcp_e("2001:db8::3", 12347, IPPROTO_TCP);
+ EXPECT_EQ(ResponseScrubber::ADDRESS,
+ ResponseScrubber::addressCheck(tcp_a, tcp_e));
+
+}
+
+// Ensure that mixed IPv4/6 addresses don't match.
+
+TEST_F(ResponseScrubberTest, v4v6) {
+
+ // UDP
+ GenericEndpoint udp_a("2001:db8::1", 12345, IPPROTO_UDP);
+ GenericEndpoint udp_b("192.0.2.1", 12345, IPPROTO_UDP);
+ EXPECT_EQ(ResponseScrubber::ADDRESS,
+ ResponseScrubber::addressCheck(udp_a, udp_b));
+
+ // TCP
+ GenericEndpoint tcp_a("2001:db8::1", 12345, IPPROTO_TCP);
+ GenericEndpoint tcp_b("192.0.2.1", 12345, IPPROTO_TCP);
+ EXPECT_EQ(ResponseScrubber::ADDRESS,
+ ResponseScrubber::addressCheck(udp_a, udp_b));
+}
+
+// Check mixed protocols are detected
+
+TEST_F(ResponseScrubberTest, Protocol) {
+ GenericEndpoint udp_a("2001:db8::1", 12345, IPPROTO_UDP);
+ GenericEndpoint tcp_a("2001:db8::1", 12345, IPPROTO_TCP);
+ EXPECT_EQ(ResponseScrubber::PROTOCOL,
+ ResponseScrubber::addressCheck(udp_a, tcp_a));
}
// Check that the QIDs check OK
@@ -201,10 +306,11 @@ TEST_F(ResponseScrubberTest, Qid) {
EXPECT_FALSE(ResponseScrubber::qidCheck(a, c));
}
-// Check the scrub() method. As this operates by calling the scrubSection()
-// method, this is also a check of the latter.
+// Check the scrubAllSections() method. As this operates by calling the
+// scrubSection() method (with a SUBDOMAIN argument), this is also a check of
+// the latter.
-TEST_F(ResponseScrubberTest, ValidMessage) {
+TEST_F(ResponseScrubberTest, ScrubAllSectionsValid) {
Message valid(Message::RENDER);
// Valid message with nothing out of bailiwick
@@ -214,36 +320,29 @@ TEST_F(ResponseScrubberTest, ValidMessage) {
valid.addRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns0);
// Scrub the message and expect nothing to have been removed.
- int removed = ResponseScrubber::scrub(bailiwick, valid);
+ int removed = ResponseScrubber::scrubAllSections(valid, bailiwick);
EXPECT_EQ(0, removed);
// ... and check that this is the case
- EXPECT_TRUE(valid.hasRRset(Message::SECTION_ANSWER,
- rrs_in_a_www->getName(), rrs_in_a_www->getClass(), rrs_in_a_www->getType()));
- EXPECT_TRUE(valid.hasRRset(Message::SECTION_AUTHORITY,
- rrs_in_ns->getName(), rrs_in_ns->getClass(), rrs_in_ns->getType()));
- EXPECT_TRUE(valid.hasRRset(Message::SECTION_ADDITIONAL,
- rrs_in_a_ns0->getName(), rrs_in_a_ns0->getClass(), rrs_in_a_ns0->getType()));
+ EXPECT_TRUE(valid.hasRRset(Message::SECTION_ANSWER, rrs_in_a_www));
+ EXPECT_TRUE(valid.hasRRset(Message::SECTION_AUTHORITY, rrs_in_ns));
+ EXPECT_TRUE(valid.hasRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns0));
// Add out-of-bailiwick glue to the additional section (pretend that the
// NS RRset contained an out-of-domain server.
valid.addRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns2);
- EXPECT_TRUE(valid.hasRRset(Message::SECTION_ADDITIONAL,
- rrs_in_a_ns2->getName(), rrs_in_a_ns2->getClass(), rrs_in_a_ns2->getType()));
+ EXPECT_TRUE(valid.hasRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns2));
// ... and check that it is removed when scrubbed
- removed = ResponseScrubber::scrub(bailiwick, valid);
- EXPECT_TRUE(valid.hasRRset(Message::SECTION_ANSWER,
- rrs_in_a_www->getName(), rrs_in_a_www->getClass(), rrs_in_a_www->getType()));
- EXPECT_TRUE(valid.hasRRset(Message::SECTION_AUTHORITY,
- rrs_in_ns->getName(), rrs_in_ns->getClass(), rrs_in_ns->getType()));
- EXPECT_TRUE(valid.hasRRset(Message::SECTION_ADDITIONAL,
- rrs_in_a_ns0->getName(), rrs_in_a_ns0->getClass(), rrs_in_a_ns0->getType()));
- EXPECT_FALSE(valid.hasRRset(Message::SECTION_ADDITIONAL,
- rrs_in_a_ns2->getName(), rrs_in_a_ns2->getClass(), rrs_in_a_ns2->getType()));
-}
-
-TEST_F(ResponseScrubberTest, InvalidMessage) {
+ removed = ResponseScrubber::scrubAllSections(valid, bailiwick);
+ EXPECT_EQ(1, removed);
+ EXPECT_TRUE(valid.hasRRset(Message::SECTION_ANSWER, rrs_in_a_www));
+ EXPECT_TRUE(valid.hasRRset(Message::SECTION_AUTHORITY, rrs_in_ns));
+ EXPECT_TRUE(valid.hasRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns0));
+ EXPECT_FALSE(valid.hasRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns2));
+ }
+
+TEST_F(ResponseScrubberTest, ScrubAllSectionsInvalid) {
Message invalid(Message::RENDER);
// Invalid message, with various things in and out of bailiwick.
@@ -293,43 +392,31 @@ TEST_F(ResponseScrubberTest, InvalidMessage) {
invalid.addRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns3);
// Scrub the message
- int removed = ResponseScrubber::scrub(bailiwick, invalid);
+ int removed = ResponseScrubber::scrubAllSections(invalid, bailiwick);
EXPECT_EQ(6, removed);
// ... and check the sections. Answer...
- EXPECT_TRUE(invalid.hasRRset(Message::SECTION_ANSWER,
- rrs_in_a_www->getName(), rrs_in_a_www->getClass(), rrs_in_a_www->getType()));
- EXPECT_TRUE(invalid.hasRRset(Message::SECTION_ANSWER,
- rrs_in_txt_www->getName(), rrs_in_txt_www->getClass(), rrs_in_txt_www->getType()));
- EXPECT_FALSE(invalid.hasRRset(Message::SECTION_ANSWER,
- rrs_in_a_org->getName(), rrs_in_a_org->getClass(), rrs_in_a_org->getType()));
- EXPECT_FALSE(invalid.hasRRset(Message::SECTION_ANSWER,
- rrs_in_a_net->getName(), rrs_in_a_net->getClass(), rrs_in_a_net->getType()));
+ EXPECT_TRUE(invalid.hasRRset(Message::SECTION_ANSWER, rrs_in_a_www));
+ EXPECT_TRUE(invalid.hasRRset(Message::SECTION_ANSWER, rrs_in_txt_www));
+ EXPECT_FALSE(invalid.hasRRset(Message::SECTION_ANSWER, rrs_in_a_org));
+ EXPECT_FALSE(invalid.hasRRset(Message::SECTION_ANSWER, rrs_in_a_net));
// ... authority...
- EXPECT_TRUE(invalid.hasRRset(Message::SECTION_AUTHORITY,
- rrs_in_ns->getName(), rrs_in_ns->getClass(), rrs_in_ns->getType()));
- EXPECT_FALSE(invalid.hasRRset(Message::SECTION_AUTHORITY,
- rrs_in_ns_com->getName(), rrs_in_ns_com->getClass(), rrs_in_ns_com->getType()));
- EXPECT_FALSE(invalid.hasRRset(Message::SECTION_AUTHORITY,
- rrs_in_ns_net->getName(), rrs_in_ns_net->getClass(), rrs_in_ns_net->getType()));
- EXPECT_TRUE(invalid.hasRRset(Message::SECTION_AUTHORITY,
- rrs_in_ns_sub->getName(), rrs_in_ns_sub->getClass(), rrs_in_ns_sub->getType()));
+ EXPECT_TRUE(invalid.hasRRset(Message::SECTION_AUTHORITY, rrs_in_ns));
+ EXPECT_FALSE(invalid.hasRRset(Message::SECTION_AUTHORITY, rrs_in_ns_com));
+ EXPECT_FALSE(invalid.hasRRset(Message::SECTION_AUTHORITY, rrs_in_ns_net));
+ EXPECT_TRUE(invalid.hasRRset(Message::SECTION_AUTHORITY, rrs_in_ns_sub));
// ... additional.
- EXPECT_TRUE(invalid.hasRRset(Message::SECTION_ADDITIONAL,
- rrs_in_a_ns0->getName(), rrs_in_a_ns0->getClass(), rrs_in_a_ns0->getType()));
- EXPECT_FALSE(invalid.hasRRset(Message::SECTION_ADDITIONAL,
- rrs_in_a_ns1->getName(), rrs_in_a_ns1->getClass(), rrs_in_a_ns1->getType()));
- EXPECT_FALSE(invalid.hasRRset(Message::SECTION_ADDITIONAL,
- rrs_in_a_ns2->getName(), rrs_in_a_ns2->getClass(), rrs_in_a_ns2->getType()));
- EXPECT_TRUE(invalid.hasRRset(Message::SECTION_ADDITIONAL,
- rrs_in_a_ns3->getName(), rrs_in_a_ns3->getClass(), rrs_in_a_ns3->getType()));
+ EXPECT_TRUE(invalid.hasRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns0));
+ EXPECT_FALSE(invalid.hasRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns1));
+ EXPECT_FALSE(invalid.hasRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns2));
+ EXPECT_TRUE(invalid.hasRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns3));
}
-// Finally, an empty message
+// An empty message
-TEST_F(ResponseScrubberTest, EmptyMessage) {
+TEST_F(ResponseScrubberTest, ScrubAllSectionsEmpty) {
Message empty(Message::RENDER);
EXPECT_EQ(0, empty.getRRCount(Message::SECTION_QUESTION));
@@ -337,7 +424,7 @@ TEST_F(ResponseScrubberTest, EmptyMessage) {
EXPECT_EQ(0, empty.getRRCount(Message::SECTION_AUTHORITY));
EXPECT_EQ(0, empty.getRRCount(Message::SECTION_ADDITIONAL));
- int removed = ResponseScrubber::scrub(bailiwick, empty);
+ int removed = ResponseScrubber::scrubAllSections(empty, bailiwick);
EXPECT_EQ(0, removed);
EXPECT_EQ(0, empty.getRRCount(Message::SECTION_QUESTION));
@@ -347,4 +434,109 @@ TEST_F(ResponseScrubberTest, EmptyMessage) {
}
+// Check the cross-section scrubbing (checks the general scrubSection()
+// method with a SUPERDOMAIN argument.)
+
+// Empty message (apart from question)
+
+TEST_F(ResponseScrubberTest, CrossSectionEmpty) {
+
+ Message message1(Message::RENDER);
+ message1.addQuestion(qu_in_a_www);
+ int removed = ResponseScrubber::scrubCrossSections(message1);
+ EXPECT_EQ(0, removed);
+}
+
+// Valid answer section
+
+TEST_F(ResponseScrubberTest, CrossSectionAnswer) {
+
+ // Valid message with nothing out of bailiwick, but the authority
+ // (subdomain.example.com) is not authoritative for the answer.
+ //
+ // TODO: Test the case where the additional section does not match
+ // with something in the authority section.
+ Message message1(Message::RENDER);
+ message1.addQuestion(qu_in_a_www);
+ message1.addRRset(Message::SECTION_ANSWER, rrs_in_a_www);
+ message1.addRRset(Message::SECTION_AUTHORITY, rrs_in_ns_sub);
+ message1.addRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns3);
+ int removed = ResponseScrubber::scrubCrossSections(message1);
+ EXPECT_EQ(1, removed);
+ EXPECT_TRUE(message1.hasRRset(Message::SECTION_ANSWER, rrs_in_a_www));
+ EXPECT_FALSE(message1.hasRRset(Message::SECTION_AUTHORITY, rrs_in_ns_sub));
+ EXPECT_TRUE(message1.hasRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns3));
+
+ // A repeat of the test, this time with a mixture of incorrect and correct
+ // authorities.
+ Message message2(Message::RENDER);
+ message2.addQuestion(qu_in_a_www);
+ message2.addRRset(Message::SECTION_ANSWER, rrs_in_a_www);
+ message2.addRRset(Message::SECTION_AUTHORITY, rrs_in_ns_sub);
+ message2.addRRset(Message::SECTION_AUTHORITY, rrs_in_ns);
+ message2.addRRset(Message::SECTION_AUTHORITY, rrs_in_ns_sub2);
+ message2.addRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns3);
+ removed = ResponseScrubber::scrubCrossSections(message2);
+ EXPECT_EQ(2, removed);
+ EXPECT_TRUE(message2.hasRRset(Message::SECTION_ANSWER, rrs_in_a_www));
+ EXPECT_FALSE(message2.hasRRset(Message::SECTION_AUTHORITY, rrs_in_ns_sub));
+ EXPECT_TRUE(message2.hasRRset(Message::SECTION_AUTHORITY, rrs_in_ns));
+ EXPECT_FALSE(message2.hasRRset(Message::SECTION_AUTHORITY, rrs_in_ns_sub2));
+ EXPECT_TRUE(message2.hasRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns3));
+}
+
+// Test the main "scrub" method. This is a single to ensure that the
+// combination of methods
+
+TEST_F(ResponseScrubberTest, All) {
+ MessagePtr mptr(new Message(Message::RENDER));
+
+ // Question is "www.example.com IN A" sent to a nameserver with the
+ // bailiwick of "example.com".
+ mptr->addQuestion(qu_in_a_www);
+
+ // Answer section.
+
+ // "www.example.com IN CNAME www.example.net" - should be kept
+ mptr->addRRset(Message::SECTION_ANSWER, rrs_in_cname_www);
+
+ // "www.example.net IN A a.b.c.d" - should be removed, out of bailiwick
+ mptr->addRRset(Message::SECTION_ANSWER, rrs_in_a_wwwnet);
+
+ // Authority section.
+
+ // "example.net IN NS xxxx" - should be removed, out of bailiwick.
+ mptr->addRRset(Message::SECTION_AUTHORITY, rrs_in_ns_net);
+
+ // "example.com IN NS xxx" - kept
+ mptr->addRRset(Message::SECTION_AUTHORITY, rrs_in_ns);
+
+ // "com IN NS xxx" - removed, out of bailiwick
+ mptr->addRRset(Message::SECTION_AUTHORITY, rrs_in_ns_com);
+
+ // "subdomain.example.com IN NS xxx" - removed, not a superdomain of the
+ // answer.
+ mptr->addRRset(Message::SECTION_AUTHORITY, rrs_in_ns_sub);
+
+ // Additional section
+
+ // "ns2.example.net IN A a.b.c.d" - removed, out of bailiwick
+ mptr->addRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns2);
+
+ // "ns3.subdomain.example.com IN A a.b.c.d" - retained.
+ mptr->addRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns3);
+
+ unsigned int removed = ResponseScrubber::scrub(mptr, bailiwick);
+ EXPECT_EQ(5, removed);
+
+ EXPECT_TRUE(mptr->hasRRset(Message::SECTION_ANSWER, rrs_in_cname_www));
+ EXPECT_FALSE(mptr->hasRRset(Message::SECTION_ANSWER, rrs_in_a_wwwnet));
+ EXPECT_FALSE(mptr->hasRRset(Message::SECTION_AUTHORITY, rrs_in_ns_net));
+ EXPECT_TRUE(mptr->hasRRset(Message::SECTION_AUTHORITY, rrs_in_ns));
+ EXPECT_FALSE(mptr->hasRRset(Message::SECTION_AUTHORITY, rrs_in_ns_com));
+ EXPECT_FALSE(mptr->hasRRset(Message::SECTION_AUTHORITY, rrs_in_ns_sub));
+ EXPECT_FALSE(mptr->hasRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns2));
+ EXPECT_TRUE(mptr->hasRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns3));
+
+}
} // Anonymous namespace
\ No newline at end of file
diff --git a/src/lib/asiolink/ioaddress.h b/src/lib/asiolink/ioaddress.h
index 5727041..98e6fe8 100644
--- a/src/lib/asiolink/ioaddress.h
+++ b/src/lib/asiolink/ioaddress.h
@@ -73,9 +73,48 @@ public:
/// \return A string representation of the address.
std::string toText() const;
- /// \brief Returns the address family.
+ /// \brief Returns the address family
+ ///
+ /// \return AF_INET for IPv4 or AF_INET6 for IPv6.
short getFamily() const;
+ /// \brief Compare addresses for equality
+ ///
+ /// \param other Address to compare against.
+ ///
+ /// \return true if addresses are equal, false if not.
+ bool equals(const IOAddress& other) const {
+ return (asio_address_ == other.asio_address_);
+ }
+
+ /// \brief Compare addresses for equality
+ ///
+ /// \param other Address to compare against.
+ ///
+ /// \return true if addresses are equal, false if not.
+ bool operator==(const IOAddress& other) const {
+ return equals(other);
+ }
+
+ // \brief Compare addresses for inequality
+ ///
+ /// \param other Address to compare against.
+ ///
+ /// \return false if addresses are equal, true if not.
+ bool nequals(const IOAddress& other) const {
+ return (!equals(other));
+ }
+
+ // \brief Compare addresses for inequality
+ ///
+ /// \param other Address to compare against.
+ ///
+ /// \return false if addresses are equal, true if not.
+ bool operator!=(const IOAddress& other) const {
+ return (nequals(other));
+ }
+
+
private:
asio::ip::address asio_address_;
};
diff --git a/src/lib/asiolink/ioendpoint.h b/src/lib/asiolink/ioendpoint.h
index 926ce50..82190a3 100644
--- a/src/lib/asiolink/ioendpoint.h
+++ b/src/lib/asiolink/ioendpoint.h
@@ -24,6 +24,7 @@
#include <string>
#include <exceptions/exceptions.h>
+#include <asiolink/ioaddress.h>
namespace asiolink {
diff --git a/src/lib/asiolink/tests/asiolink_unittest.cc b/src/lib/asiolink/tests/asiolink_unittest.cc
index c83c090..a9c669a 100644
--- a/src/lib/asiolink/tests/asiolink_unittest.cc
+++ b/src/lib/asiolink/tests/asiolink_unittest.cc
@@ -82,6 +82,23 @@ TEST(IOAddressTest, fromText) {
EXPECT_THROW(IOAddress("2001:db8::efgh"), IOError);
}
+TEST(IOAddressTest, Equality) {
+ EXPECT_TRUE(IOAddress("192.0.2.1") == IOAddress("192.0.2.1"));
+ EXPECT_FALSE(IOAddress("192.0.2.1") != IOAddress("192.0.2.1"));
+
+ EXPECT_TRUE(IOAddress("192.0.2.1") != IOAddress("192.0.2.2"));
+ EXPECT_FALSE(IOAddress("192.0.2.1") == IOAddress("192.0.2.2"));
+
+ EXPECT_TRUE(IOAddress("2001:db8::12") == IOAddress("2001:0DB8:0:0::0012"));
+ EXPECT_FALSE(IOAddress("2001:db8::12") != IOAddress("2001:0DB8:0:0::0012"));
+
+ EXPECT_TRUE(IOAddress("2001:db8::1234") != IOAddress("2001:db8::1235"));
+ EXPECT_FALSE(IOAddress("2001:db8::1234") == IOAddress("2001:db8::1235"));
+
+ EXPECT_TRUE(IOAddress("2001:db8::1234") != IOAddress("192.0.2.3"));
+ EXPECT_FALSE(IOAddress("2001:db8::1234") == IOAddress("192.0.2.3"));
+}
+
TEST(IOEndpointTest, createUDPv4) {
const IOEndpoint* ep;
ep = IOEndpoint::create(IPPROTO_UDP, IOAddress("192.0.2.1"), 5300);
diff --git a/src/lib/dns/message.cc b/src/lib/dns/message.cc
index 8449ea3..e9ab1b9 100644
--- a/src/lib/dns/message.cc
+++ b/src/lib/dns/message.cc
@@ -310,6 +310,11 @@ Message::hasRRset(const Section section, const Name& name,
}
bool
+Message::hasRRset(const Section section, const RRsetPtr& rrset) {
+ return (hasRRset(section, rrset->getName(), rrset->getClass(), rrset->getType()));
+}
+
+bool
Message::removeRRset(const Section section, RRsetIterator& iterator) {
if (section >= MessageImpl::NUM_SECTIONS) {
isc_throw(OutOfRange, "Invalid message section: " << section);
diff --git a/src/lib/dns/message.h b/src/lib/dns/message.h
index 694f26f..de82085 100644
--- a/src/lib/dns/message.h
+++ b/src/lib/dns/message.h
@@ -460,6 +460,13 @@ public:
bool hasRRset(const Section section, const Name& name,
const RRClass& rrclass, const RRType& rrtype);
+ /// \brief Determine whether the given section already has an RRset
+ /// matching the one pointed to by the argumet
+ ///
+ /// \c section must be a valid constant of the \c Section type;
+ /// otherwise, an exception of class \c OutOfRange will be thrown.
+ bool hasRRset(const Section section, const RRsetPtr& rrset);
+
/// \brief Remove RRSet from Message
///
/// Removes the RRset identified by the section iterator from the message.
diff --git a/src/lib/dns/tests/message_unittest.cc b/src/lib/dns/tests/message_unittest.cc
index 65bd345..ee1375a 100644
--- a/src/lib/dns/tests/message_unittest.cc
+++ b/src/lib/dns/tests/message_unittest.cc
@@ -250,6 +250,27 @@ TEST_F(MessageTest, hasRRset) {
EXPECT_THROW(message_render.hasRRset(bogus_section, test_name,
RRClass::IN(), RRType::A()),
OutOfRange);
+
+ // Repeat the checks having created an RRset of the appropriate type.
+
+ RRsetPtr rrs1(new RRset(test_name, RRClass::IN(), RRType::A(), RRTTL(60)));
+ EXPECT_TRUE(message_render.hasRRset(Message::SECTION_ANSWER, rrs1));
+ EXPECT_FALSE(message_render.hasRRset(Message::SECTION_ADDITIONAL, rrs1));
+
+ RRsetPtr rrs2(new RRset(Name("nomatch.example"), RRClass::IN(), RRType::A(),
+ RRTTL(5)));
+ EXPECT_FALSE(message_render.hasRRset(Message::SECTION_ANSWER, rrs2));
+
+ RRsetPtr rrs3(new RRset(test_name, RRClass::CH(), RRType::A(), RRTTL(60)));
+ EXPECT_FALSE(message_render.hasRRset(Message::SECTION_ANSWER, rrs3));
+
+ RRsetPtr rrs4(new RRset(test_name, RRClass::IN(), RRType::AAAA(), RRTTL(5)));
+ EXPECT_FALSE(message_render.hasRRset(Message::SECTION_ANSWER, rrs4));
+
+ RRsetPtr rrs5(new RRset(test_name, RRClass::IN(), RRType::AAAA(), RRTTL(5)));
+ EXPECT_FALSE(message_render.hasRRset(Message::SECTION_ANSWER, rrs4));
+
+ EXPECT_THROW(message_render.hasRRset(bogus_section, rrs1), OutOfRange);
}
TEST_F(MessageTest, removeRRset) {
More information about the bind10-changes
mailing list