Commit ab2c79a8 authored by Stephen Morris's avatar Stephen Morris
Browse files

[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
parent 08b21520
......@@ -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
......
......@@ -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)
......@@ -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)
......@@ -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;
}
......@@ -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
......@@ -73,9 +73,48 @@ public:
/// \return A string representation of the address.