Browse Source

[trac496] Add the response scrubbing code.

Implemented as a separate function, this takes a message and removes
any RRsets that are not in bailiwick for the server from which the
response was received.  Also supplied are functions to check the QID
is correct and that the Address/Port from which the response was
received is as expected.
Stephen Morris 14 years ago
parent
commit
0675677910

+ 1 - 0
src/bin/resolver/Makefile.am

@@ -38,6 +38,7 @@ BUILT_SOURCES = spec_config.h
 pkglibexec_PROGRAMS = b10-resolver
 b10_resolver_SOURCES = resolver.cc resolver.h
 b10_resolver_SOURCES += response_classifier.cc response_classifier.h
+b10_resolver_SOURCES += response_scrubber.cc response_scrubber.h
 b10_resolver_SOURCES += $(top_builddir)/src/bin/auth/change_user.h
 b10_resolver_SOURCES += $(top_builddir)/src/bin/auth/common.h
 b10_resolver_SOURCES += main.cc

+ 94 - 0
src/bin/resolver/response_scrubber.cc

@@ -0,0 +1,94 @@
+
+// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <dns/message.h>
+#include <dns/rrset.h>
+#include "response_scrubber.h"
+
+using namespace isc::dns;
+using namespace asio::ip;
+
+// Compare addresses etc.
+
+ResponseScrubber::Category ResponseScrubber::addressPortCheck(
+    const udp::endpoint& to, const udp::endpoint& from)
+{
+    if (from.address() == to.address()) {
+        if (from.port() == to.port()) {
+            return (ResponseScrubber::SUCCESS);
+        }
+        else {
+            return (ResponseScrubber::PORT);
+        }
+    }
+    return (ResponseScrubber::ADDRESS);
+}
+
+// Scrub a section of the message
+
+unsigned int
+ResponseScrubber::scrubSection(const Name& bailiwick, Message& message,
+    const Message::Section section)
+{
+    unsigned int count = 0;
+    bool removed = true;
+
+    // Need to iterate multiple times as removing an RRset from the section
+    // invalidates the iterators.
+    while (removed) {
+        removed = false;
+        for (RRsetIterator i = message.beginSection(section);
+            i != message.endSection(section); ++i) {
+            NameComparisonResult result = (*i)->getName().compare(bailiwick);
+            NameComparisonResult::NameRelation relation =
+                    result.getRelation();
+            if ((relation != NameComparisonResult::EQUAL) &&
+                (relation != NameComparisonResult::SUBDOMAIN)) {
+                    
+                // 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.
+                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;
+            }
+        }
+
+
+    }
+    return count;
+}
+
+// Perform the scrubbing of the message.
+
+unsigned int
+ResponseScrubber::scrub(const Name& bailiwick, Message& message) {
+
+    // Leave the question section alone.  Just go through the RRsets in the
+    // answer, authority and additional sectiuons.
+    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);
+
+    return count;
+}
+

+ 237 - 0
src/bin/resolver/response_scrubber.h

@@ -0,0 +1,237 @@
+// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+// $Id$
+
+#ifndef __RESPONSE_SCRUBBER_H
+#define __RESPONSE_SCRUBBER_H
+
+/// \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?
+///
+/// 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
+/// "Scrubbing" because it "scrubs" the returned message and removes doubtful
+/// information.
+///
+/// \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:
+///
+/// # 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
+///   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
+/// over the TCP stream, it is assumed that it comes from the address and port
+/// to which a connection was made.)
+///
+/// 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.
+///
+/// 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.
+///
+/// Some examples should make this clear: they all use the notation
+/// Qu = Question, Zo = Zone being asked, An = Answer, Au = Authority,
+/// Ad = Additional.
+///
+/// \subsection DataScrubbingEx1 Example 1: Simple Query
+/// Querying a nameserver for the zone "example.com" for www.example.com and
+/// receiving the answer "www.example.com A 1.2.3.4" with two nameservers quoted
+/// as authority and both their addresses in the additional section:
+///
+/// Qu: www.example.com\n
+/// Zo: example.com
+///
+/// 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
+///
+/// Ad(1): ns0.example.com A 192.0.2.100\n
+/// Ad(2): ns1.example.net A 192.0.2.200
+///
+/// This answer could be returned by a properly configured server.  All resource
+/// records in the answer - with the exception of Ad(2) - are in bailiwick
+/// because the QNAME is equal to or a subdomain of the zone being queried.
+///
+/// It is permissible for Ad(2) to be returned by a properly configured server
+/// as a hint to resolvers.  However the example.com nameservers are not
+/// authoritative for addresses of domains in example.net; that record could
+/// be out of date or incorrect.  Indeed, it might even be a deliberate attempt
+/// at a spoof by getting us to cache an invalid address for ns1.example.net.
+/// The safest thing to do is to drop the A record and to get the address of
+/// ns1.example.net by querying for that name through the .net nameservers.
+///
+/// \subsection DataScrubbingEx2 Example 2: Multiple Zones on Same Nameserver
+/// Assume now that example.com and sub.example.com are hosted on the same
+/// nameserver and that from the .com zone the resolver has received a referral
+/// to example.com.  Suppose that the query is for www.sub.example.com and that
+/// the following response is received:
+///
+/// Qu: www.sub.example.com\n
+/// Zo: example.com
+///
+/// An: <nothing>
+///
+/// Au(1): sub.example.com NS ns0.sub.example.com\n
+/// Au(2): sub.example.com NS ns1.example.net
+///
+/// Ad(1): ns0.sub.example.com A 192.0.2.101\n
+/// Ad(2): ns1.example.net A 192.0.2.201
+///
+/// Although we asked the example.com nameservers for information, we got the
+/// nameservers for sub.example.com in the authority section.  This is valid
+/// because if BIND-10 hosts multiple zones, it will look up the data in the
+/// zone that most closely matches the query.
+///
+/// Using the criteria above, the data in the additional section can therefore
+/// be regarded as in bailiwick because sub.example.com is a subdomain of
+/// example.com.  As before though, the address for ns1.example.net in the
+/// additional section is not in bailiwick because ns1.example.net is now a
+/// subdomain of example.com.
+///
+/// \subsection DataScrubbingEx3 Example 3: Deliberate Spoof Attempt
+/// Qu: www.example.com\n
+/// Zo: example.com
+///
+/// An: www.example.com A 192.0.2.1
+///
+/// Au(1): com NS ns0.example.com\n
+/// Au(2): 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
+///
+/// This is a deliberately invalid response.  The query is being sent to the
+/// nameservers for example.com (presumably because a referral to example.com
+/// was received from the com nameservers), but the response is an attempt
+/// to get the specified nameservers cached as the nameservers for com - for
+/// which example.com is not authoritative.
+///
+/// Note though that this response is only invalid because, due to the previous
+/// referral, the query was sent to the example.com nameservers.  Had the
+/// referral been to the com nameservers, it would be a valid response; the com
+/// 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.
+
+#include <config.h>
+#include <asio.hpp>
+#include <dns/message.h>
+
+/// \brief Response Data Scrubbing
+///
+/// This is the class that implements the data scrubbing.  Given a response
+/// 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.
+
+class ResponseScrubber {
+public:
+
+    /// \brief Response Code for Address Check
+    enum Category {
+        SUCCESS = 0,            // Packet is OK
+
+        // Error categories
+
+        ADDRESS,                // Mismatching IP address
+        PORT                    // Mismatching port
+    };
+
+    // \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.
+    //
+    // \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);
+
+    // \brief Check QID
+    //
+    // Compares the QID in the sent message with the QID in the response.
+    //
+    // \param sent Message sent to the authoritative server
+    // \param received Message received from the authoritative server
+    //
+    // \return true if the QIDs match, false otherwise.
+    static bool qidCheck(const isc::dns::Message& sent,
+        const isc::dns::Message& received) {
+        return (sent.getQid() == received.getQid());
+    }
+
+    /// \brief Scrub Message Section
+    ///
+    /// Scrubs the message section by removing all RRsets that are not in the
+    /// bailiwick of the authoritative server from the message.
+    ///
+    /// \param zone Name of the zone whose authoritative servers were queried.
+    /// \param section Section of the message to be scrubbed
+    /// \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);
+
+    /// \brief Scrub 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
+    /// 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.
+    ///
+    /// \return Count of the number of RRsets removed from the section.
+    static unsigned int scrub(const isc::dns::Name& bailiwick,
+        isc::dns::Message& message);
+};
+
+#endif // __RESPONSE_SCRUBBER_H

+ 11 - 1
src/bin/resolver/tests/Makefile.am

@@ -4,7 +4,6 @@ AM_CPPFLAGS += -I$(top_builddir)/src/lib/cc
 AM_CPPFLAGS += -DTEST_DATA_DIR=\"$(top_srcdir)/src/lib/testutils/testdata\"
 AM_CPPFLAGS += -DTEST_DATA_BUILDDIR=\"$(abs_top_builddir)/src/lib/testutils/testdata\"
 AM_CPPFLAGS += $(BOOST_INCLUDES)
-AM_CPPFLAGS += $(BOOST_INCLUDES)
 
 AM_CXXFLAGS = $(B10_CXXFLAGS)
 
@@ -21,9 +20,11 @@ run_unittests_SOURCES = $(top_srcdir)/src/lib/dns/tests/unittest_util.h
 run_unittests_SOURCES += $(top_srcdir)/src/lib/dns/tests/unittest_util.cc
 run_unittests_SOURCES += ../resolver.h ../resolver.cc
 run_unittests_SOURCES += ../response_classifier.h ../response_classifier.cc
+run_unittests_SOURCES += ../response_scrubber.h ../response_scrubber.cc
 run_unittests_SOURCES += resolver_unittest.cc
 run_unittests_SOURCES += resolver_config_unittest.cc
 run_unittests_SOURCES += response_classifier_unittest.cc
+run_unittests_SOURCES += response_scrubber_unittest.cc
 run_unittests_SOURCES += run_unittests.cc
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)
@@ -38,6 +39,15 @@ 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
+
+# Note the ordering matters: -Wno-... must follow -Wextra (defined in
+# B10_CXXFLAGS
+run_unittests_CXXFLAGS = $(AM_CXXFLAGS)
+if USE_GXX
+run_unittests_CXXFLAGS += -Wno-unused-parameter
+endif
 endif
 
+
+
 noinst_PROGRAMS = $(TESTS)

+ 350 - 0
src/bin/resolver/tests/response_scrubber_unittest.cc

@@ -0,0 +1,350 @@
+// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+// $Id$
+
+#include <gtest/gtest.h>
+
+#include <config.h>
+#include <asio.hpp>
+#include <dns/name.h>
+#include <dns/opcode.h>
+#include <dns/question.h>
+#include <dns/rdata.h>
+#include <dns/rdataclass.h>
+#include <dns/rcode.h>
+#include <dns/rrclass.h>
+#include <dns/rrset.h>
+#include <dns/rrtype.h>
+#include <dns/rrttl.h>
+#include <resolver/response_scrubber.h>
+
+using namespace asio::ip;
+using namespace isc::dns;
+using namespace rdata;
+using namespace isc::dns::rdata::generic;
+using namespace isc::dns::rdata::in;
+
+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_ns(new RRset(Name("example.com"), RRClass::IN(),
+            RRType::NS(), RRTTL(300))),
+        rrs_in_ns_com(new RRset(Name("com"), RRClass::IN(),
+            RRType::NS(), RRTTL(300))),
+        rrs_in_ns_net(new RRset(Name("example.net"), RRClass::IN(),
+            RRType::NS(), RRTTL(300))),
+        rrs_in_ns_sub(new RRset(Name("subdomain.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(),
+            RRType::A(), RRTTL(300))),
+        rrs_in_a_ns2(new RRset(Name("ns2.example.net"), RRClass::IN(),
+            RRType::A(), RRTTL(300))),
+        rrs_in_a_ns3(new RRset(Name("ns3.subdomain.example.com"), RRClass::IN(),
+            RRType::A(), RRTTL(300))),
+        rrs_in_txt_www(new RRset(Name("www.example.com"), RRClass::IN(),
+            RRType::TXT(), RRTTL(300)))
+    {}
+    Name        bailiwick;          // Bailiwick of the server queried
+    Question    qu_in_any_www;      // www.example.com IN ANY
+    Question    qu_in_a_www;        // www.example.com IN A
+    Question    qu_in_ns;           // example.com IN NS
+    Question    qu_in_txt_www;      // www.example.com IN TXT
+    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_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_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
+    RRsetPtr    rrs_in_a_ns3;       // ns3.subdomain.example.net IN A
+    RRsetPtr    rrs_in_txt_www;     // www.example.com IN TXT
+};
+
+
+// Check that the IP addresses/ports/protocol for the packets sent and received
+// both match if both types are IP V4.
+
+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);
+
+    // 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));
+
+    // 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));
+
+    // 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));
+
+    // 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));
+
+}
+
+// Repeat the tests for 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);
+
+    // 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));
+
+    // 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));
+
+    // 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));
+
+    // 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));
+
+}
+
+// Ensure that mixed IPv4/6 addresses don't match.
+
+TEST_F(ResponseScrubberTest, UDPv4v6) {
+
+    // Basic UDP Endpoint
+    udp::endpoint   udp_a;
+    udp_a.address(address_v4::from_string("192.0.2.1"));
+    udp_a.port(12345);
+
+    // 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));
+}
+
+// Check that the QIDs check OK
+
+TEST_F(ResponseScrubberTest, Qid) {
+    Message a(Message::RENDER);
+    a.setQid(27);
+
+    Message b(Message::RENDER);
+    b.setQid(27);
+    EXPECT_TRUE(ResponseScrubber::qidCheck(a, b));
+
+    Message c(Message::RENDER);
+    c.setQid(28);
+    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.
+
+TEST_F(ResponseScrubberTest, ValidMessage) {
+    Message valid(Message::RENDER);
+
+    // Valid message with nothing out of bailiwick
+    valid.addQuestion(qu_in_a_www);
+    valid.addRRset(Message::SECTION_ANSWER, rrs_in_a_www);
+    valid.addRRset(Message::SECTION_AUTHORITY, rrs_in_ns);
+    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);
+    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()));
+
+    // 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()));
+
+    // ... 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) {
+    Message invalid(Message::RENDER);
+
+    // Invalid message, with various things in and out of bailiwick.
+
+    invalid.addQuestion(qu_in_a_www);
+
+    // Answer section
+    //
+    // rrs_in_a_www - "www.example.com A", in bailiwick
+    // rrs_in_txt_www - "www.example.com TXT", in bailiwick
+    // rrs_in_a_org - "mail.example.org A", out of bailiwick - the qname is
+    //     related to the bailiwick name by having a common ancestor at the root
+    // rrs_in_a_net - "mail.example.net A", out of bailiwick - the qname is
+    //     related to the bailiwick name by having a common ancestor at the root
+    invalid.addRRset(Message::SECTION_ANSWER, rrs_in_a_www);
+    invalid.addRRset(Message::SECTION_ANSWER, rrs_in_txt_www);
+    invalid.addRRset(Message::SECTION_ANSWER, rrs_in_a_org);
+    invalid.addRRset(Message::SECTION_ANSWER, rrs_in_a_net);
+
+    // Authority section
+    //
+    // rrs_in_ns - "example.com NS", in bailiwick (qname is bailiwick name)
+    // rrs_in_ns_com - "com NS", out of bailiwick as the qname is a superdomain
+    //     (direct ancestor) of the bailiwick name
+    // rrs_in_ns_net - "example.net NS", out of bailiwick - the qname is related
+    //     to the bailiwick name by having a common ancestor at the root
+    // rrs_in_ns_sub - "subdomain.example.com", in bailiwick as the qname is
+    //     a subdomain of the bailiwick name
+    invalid.addRRset(Message::SECTION_AUTHORITY, rrs_in_ns);
+    invalid.addRRset(Message::SECTION_AUTHORITY, rrs_in_ns_com);
+    invalid.addRRset(Message::SECTION_AUTHORITY, rrs_in_ns_net);
+    invalid.addRRset(Message::SECTION_AUTHORITY, rrs_in_ns_sub);
+
+    // Additional section
+    //
+    // rrs_in_a_ns0 - "ns0.example.com", in bailiwick because the qname is
+    //     a subdomain of the bailiwick name
+    // rrs_in_a_ns1 - "ns1.com", out of bailiwick because the qname is a
+    //     sibling to the bailiwick name
+    // rrs_in_a_ns2 - "ns2.example.net", out of bailiwick because qname is
+    //     related by having a common ancestor and the root.
+    // rrs_in_a_ns3 - "ns3.subdomain.example.com", in bailiwick because the
+    //     qname is a direct descendent of the bailiwick name.
+    invalid.addRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns0);
+    invalid.addRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns1);
+    invalid.addRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns2);
+    invalid.addRRset(Message::SECTION_ADDITIONAL, rrs_in_a_ns3);
+
+    // Scrub the message
+    int removed = ResponseScrubber::scrub(bailiwick, invalid);
+    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()));
+
+    // ... 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()));
+
+    // ... 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()));
+}
+
+// Finally, an empty message
+
+TEST_F(ResponseScrubberTest, EmptyMessage) {
+    Message empty(Message::RENDER);
+
+    EXPECT_EQ(0, empty.getRRCount(Message::SECTION_QUESTION));
+    EXPECT_EQ(0, empty.getRRCount(Message::SECTION_ANSWER));
+    EXPECT_EQ(0, empty.getRRCount(Message::SECTION_AUTHORITY));
+    EXPECT_EQ(0, empty.getRRCount(Message::SECTION_ADDITIONAL));
+
+    int removed = ResponseScrubber::scrub(bailiwick, empty);
+    EXPECT_EQ(0, removed);
+
+    EXPECT_EQ(0, empty.getRRCount(Message::SECTION_QUESTION));
+    EXPECT_EQ(0, empty.getRRCount(Message::SECTION_ANSWER));
+    EXPECT_EQ(0, empty.getRRCount(Message::SECTION_AUTHORITY));
+    EXPECT_EQ(0, empty.getRRCount(Message::SECTION_ADDITIONAL));
+
+}
+
+} // Anonymous namespace

+ 25 - 0
src/lib/dns/message.cc

@@ -309,6 +309,31 @@ Message::hasRRset(const Section section, const Name& name,
     return (false);
 }
 
+bool
+Message::removeRRset(const Section section, RRsetIterator& iterator) {
+    if (section >= MessageImpl::NUM_SECTIONS) {
+        isc_throw(OutOfRange, "Invalid message section: " << section);
+    }
+
+    bool removed = false;
+    for (vector<RRsetPtr>::iterator i = impl_->rrsets_[section].begin();
+            i != impl_->rrsets_[section].end(); ++i) {
+        if (((*i)->getName() == (*iterator)->getName()) &&
+            ((*i)->getClass() == (*iterator)->getClass()) &&
+            ((*i)->getType() == (*iterator)->getType())) {
+
+            // Found the matching RRset so remove it & ignore rest
+            impl_->counts_[section] -= (*iterator)->getRdataCount();
+            impl_->rrsets_[section].erase(i);
+            removed = true;
+            break;
+        }
+    }
+
+    return (removed);
+}
+
+
 void
 Message::addQuestion(const QuestionPtr question) {
     if (impl_->mode_ != Message::RENDER) {

+ 16 - 1
src/lib/dns/message.h

@@ -460,9 +460,24 @@ public:
     bool hasRRset(const Section section, const Name& name,
                   const RRClass& rrclass, const RRType& rrtype);
 
+    /// \brief Remove RRSet from Message
+    ///
+    /// Removes the RRset identified by the section iterator from the message.
+    /// Note: if,.for some reason, the RRset is duplicated in the section, only
+    /// one occurrence is removed.
+    ///
+    /// If the operation is successful, all iterators into the section are
+    /// invalidated.
+    ///
+    /// \param section Section to which the iterator belongs
+    /// \param iterator Iterator pointing to the element to be removed
+    ///
+    /// \return true if the element was removed, false if the iterator was not
+    /// found in the specified section.
+    bool removeRRset(const Section section, RRsetIterator& iterator);
+
     // The following methods are not currently implemented.
     //void removeQuestion(QuestionPtr question);
-    //void removeRRset(const Section section, RRsetPtr rrset);
     // notyet:
     //void addRR(const Section section, const RR& rr);
     //void removeRR(const Section section, const RR& rr);

+ 24 - 0
src/lib/dns/tests/message_unittest.cc

@@ -252,6 +252,30 @@ TEST_F(MessageTest, hasRRset) {
                  OutOfRange);
 }
 
+TEST_F(MessageTest, removeRRset) {
+    message_render.addRRset(Message::SECTION_ANSWER, rrset_a);
+    message_render.addRRset(Message::SECTION_ANSWER, rrset_aaaa);
+    EXPECT_TRUE(message_render.hasRRset(Message::SECTION_ANSWER, test_name,
+        RRClass::IN(), RRType::A()));
+    EXPECT_TRUE(message_render.hasRRset(Message::SECTION_ANSWER, test_name,
+        RRClass::IN(), RRType::AAAA()));
+    EXPECT_EQ(3, message_render.getRRCount(Message::SECTION_ANSWER));
+
+    // Locate the AAAA RRset and remove it; this has one RR in it.
+    RRsetIterator i = message_render.beginSection(Message::SECTION_ANSWER);
+    if ((*i)->getType() == RRType::A()) {
+        ++i;
+    }
+    EXPECT_EQ(RRType::AAAA(), (*i)->getType());
+    message_render.removeRRset(Message::SECTION_ANSWER, i);
+
+    EXPECT_TRUE(message_render.hasRRset(Message::SECTION_ANSWER, test_name,
+        RRClass::IN(), RRType::A()));
+    EXPECT_FALSE(message_render.hasRRset(Message::SECTION_ANSWER, test_name,
+        RRClass::IN(), RRType::AAAA()));
+    EXPECT_EQ(2, message_render.getRRCount(Message::SECTION_ANSWER));
+}
+
 TEST_F(MessageTest, badBeginSection) {
     // valid cases are tested via other tests
     EXPECT_THROW(message_render.beginSection(Message::SECTION_QUESTION),