[svn] commit: r1719 - in /branches/jinmei-ednsupdate/src/lib/dns: Makefile.am edns.cc edns.h message.cc message.h tests/Makefile.am tests/edns_unittest.cc tests/message_unittest.cc
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu Apr 15 21:31:50 UTC 2010
Author: jinmei
Date: Thu Apr 15 21:31:50 2010
New Revision: 1719
Log:
checkpoint
Added:
branches/jinmei-ednsupdate/src/lib/dns/edns.cc (with props)
branches/jinmei-ednsupdate/src/lib/dns/edns.h (with props)
branches/jinmei-ednsupdate/src/lib/dns/tests/edns_unittest.cc (with props)
Modified:
branches/jinmei-ednsupdate/src/lib/dns/Makefile.am
branches/jinmei-ednsupdate/src/lib/dns/message.cc
branches/jinmei-ednsupdate/src/lib/dns/message.h
branches/jinmei-ednsupdate/src/lib/dns/tests/Makefile.am
branches/jinmei-ednsupdate/src/lib/dns/tests/message_unittest.cc
Modified: branches/jinmei-ednsupdate/src/lib/dns/Makefile.am
==============================================================================
--- branches/jinmei-ednsupdate/src/lib/dns/Makefile.am (original)
+++ branches/jinmei-ednsupdate/src/lib/dns/Makefile.am Thu Apr 15 21:31:50 2010
@@ -63,6 +63,7 @@
libdns_la_SOURCES += base64.h base64.cc
libdns_la_SOURCES += buffer.h
libdns_la_SOURCES += dnssectime.h dnssectime.cc
+libdns_la_SOURCES += edns.h edns.cc
libdns_la_SOURCES += exceptions.h exceptions.cc
libdns_la_SOURCES += hex.h hex.cc
libdns_la_SOURCES += message.h message.cc
Modified: branches/jinmei-ednsupdate/src/lib/dns/message.cc
==============================================================================
--- branches/jinmei-ednsupdate/src/lib/dns/message.cc (original)
+++ branches/jinmei-ednsupdate/src/lib/dns/message.cc Thu Apr 15 21:31:50 2010
@@ -27,6 +27,7 @@
#include <exceptions/exceptions.h>
#include <dns/buffer.h>
+#include <dns/edns.h>
#include <dns/exceptions.h>
#include <dns/message.h>
#include <dns/messagerenderer.h>
@@ -63,10 +64,7 @@
//
// EDNS related constants
//
-const flags_t EXTFLAG_MASK = 0xffff;
-const flags_t EXTFLAG_DO = 0x8000;
const uint32_t EXTRCODE_MASK = 0xff000000;
-const uint32_t EDNSVERSION_MASK = 0x00ff0000;
const unsigned int OPCODE_MASK = 0x7800;
const unsigned int OPCODE_SHIFT = 11;
@@ -202,17 +200,13 @@
Rcode rcode_;
const Opcode* opcode_;
flags_t flags_;
- bool dnssec_ok_;
bool header_parsed_;
static const unsigned int SECTION_MAX = 4; // TODO: revisit this design
int counts_[SECTION_MAX]; // TODO: revisit this definition
vector<QuestionPtr> questions_;
vector<RRsetPtr> rrsets_[SECTION_MAX];
- RRsetPtr remote_edns_;
- uint16_t remote_udpsize_;
- RRsetPtr local_edns_;
- uint16_t udpsize_;
+ ConstEDNSPtr edns_;
#ifdef notyet
// tsig/sig0: TODO
@@ -236,11 +230,7 @@
qid_ = 0;
rcode_ = Rcode::NOERROR(); // XXX
opcode_ = NULL;
- dnssec_ok_ = false;
- remote_edns_ = RRsetPtr();
- remote_udpsize_ = Message::DEFAULT_MAX_UDPSIZE;
- local_edns_ = RRsetPtr();
- udpsize_ = Message::DEFAULT_MAX_UDPSIZE;
+ edns_ = EDNSPtr();
for (int i = 0; i < SECTION_MAX; ++i) {
counts_[i] = 0;
@@ -282,38 +272,6 @@
"clearHeaderFlag performed in non-render mode");
}
impl_->flags_ &= ~flag.getBit();
-}
-
-bool
-Message::isDNSSECSupported() const {
- return (impl_->dnssec_ok_);
-}
-
-void
-Message::setDNSSECSupported(bool on) {
- if (impl_->mode_ != Message::RENDER) {
- isc_throw(InvalidMessageOperation,
- "setDNSSECSupported performed in non-render mode");
- }
- impl_->dnssec_ok_ = on;
-}
-
-uint16_t
-Message::getUDPSize() const {
- return (impl_->udpsize_);
-}
-
-void
-Message::setUDPSize(uint16_t size) {
- if (impl_->mode_ != Message::RENDER) {
- isc_throw(InvalidMessageOperation,
- "setUDPSize performed in non-render mode");
- }
- if (size < DEFAULT_MAX_UDPSIZE) {
- isc_throw(InvalidMessageUDPSize,
- "Specified UDP message size is too small");
- }
- impl_->udpsize_ = size;
}
qid_t
@@ -427,54 +385,6 @@
};
}
-namespace {
-bool
-addEDNS(MessageImpl* mimpl, MessageRenderer& renderer) {
- const bool is_query = ((mimpl->flags_ & MessageFlag::QR().getBit()) == 0);
-
- // If this is a reply, add EDNS either when the request had it, or
- // if the Rcode is BADVERS, which is EDNS specific.
- // XXX: this logic is tricky. We should revisit this later.
- if (!is_query) {
- if (mimpl->remote_edns_ == NULL && mimpl->rcode_ != Rcode::BADVERS()) {
- return (false);
- }
- } else {
- // For queries, we add EDNS only when necessary:
- // Local UDP size is not the default value, or
- // DNSSEC DO bit is to be set, or
- // Extended Rcode is to be specified.
- if (mimpl->udpsize_ == Message::DEFAULT_MAX_UDPSIZE &&
- !mimpl->dnssec_ok_ &&
- mimpl->rcode_.getCode() < 0x10) {
- return (false);
- }
- }
-
- // If adding the OPT RR would exceed the size limit, don't do it.
- // 11 = len(".") + type(2byte) + class(2byte) + TTL(4byte) + RDLEN(2byte)
- // (RDATA is empty in this simple implementation)
- if (renderer.getLength() + 11 > renderer.getLengthLimit()) {
- return (false);
- }
-
- // Render EDNS OPT RR
- uint32_t extrcode_flags = ((mimpl->rcode_.getCode() & 0xff0) << 24);
- if (mimpl->dnssec_ok_) {
- extrcode_flags |= 0x8000; // set DO bit
- }
- mimpl->local_edns_ = RRsetPtr(new RRset(Name::ROOT_NAME(),
- RRClass(mimpl->udpsize_),
- RRType::OPT(),
- RRTTL(extrcode_flags)));
- // We don't support any options in this simple implementation
- mimpl->local_edns_->addRdata(ConstRdataPtr(new generic::OPT()));
- mimpl->local_edns_->toWire(renderer);
-
- return (true);
-}
-}
-
void
Message::toWire(MessageRenderer& renderer) {
if (impl_->mode_ != Message::RENDER) {
@@ -514,12 +424,11 @@
RenderSection<RRsetPtr>(renderer, false)).getTotalCount();
}
- // Added EDNS OPT RR if necessary (we want to avoid hardcoding specialized
- // logic, see the parser case)
- if (!renderer.isTruncated() && addEDNS(this->impl_, renderer)) {
- ++arcount;
- }
-
+ // Added EDNS OPT RR if necessary
+ if (!renderer.isTruncated() && this->impl_->edns_ != NULL) {
+ arcount += this->impl_->edns_->toWire(renderer, impl_->rcode_);
+ }
+
// Adjust the counter buffer.
// XXX: these may not be equal to the number of corresponding entries
// in rrsets_[] or questions_ if truncation occurred or an EDNS OPT RR
@@ -643,6 +552,34 @@
};
}
+// Note about design decision:
+// we need some type specific processing here, including EDNS and TSIG.
+// how much we should generalize/hardcode the special logic is subject
+// to discussion. In terms of modularity it would be ideal to introduce
+// an abstract class (say "MessageAttribute") and let other such
+// concrete notions as EDNS or TSIG inherit from it. Then we would
+// just do:
+// message->addAttribute(rrtype, rrclass, buffer);
+// to create and attach type-specific concrete object to the message.
+//
+// A major downside of this approach is, as usual, complexity due to
+// indirection and performance penalty. Also, it may not be so easy
+// to separate the processing logic because in many cases we'll need
+// parse context for which the message class is responsible (e.g.
+// to check the EDNS OPT RR only appears in the additional section,
+// and appears only once).
+//
+// Another point to consider is that we may not need so many special
+// types other than EDNS and TSIG (and when and if we implement it,
+// SIG(0)); newer optional attributes of the message would more likely
+// be standardized as new flags or options of EDNS. If that's the case,
+// introducing an abstract class with all the overhead and complexity
+// may not make much sense.
+//
+// Conclusion: don't over-generalize type-specific logic for now.
+// introduce separate concrete classes, and move context-independent
+// logic to that class; processing logic dependent on parse context
+// is hardcoded here.
int
MessageImpl::parseSection(const Section& section, InputBuffer& buffer) {
unsigned int added = 0;
@@ -664,60 +601,33 @@
const size_t rdlen = buffer.readUint16();
ConstRdataPtr rdata = createRdata(rrtype, rrclass, buffer, rdlen);
- // XXX: we wanted to avoid hardcoding type-specific logic here,
- // but this would be the fastest way for a proof-of-concept
- // implementation. We'll revisit this part later.
if (rrtype == RRType::OPT()) {
if (section != Section::ADDITIONAL()) {
isc_throw(DNSMessageFORMERR,
"EDNS OPT RR found in an invalid section");
}
- if (remote_edns_ != NULL) {
+ if (edns_ != NULL) {
isc_throw(DNSMessageFORMERR, "multiple EDNS OPT RR found");
}
- if (((ttl.getValue() & EDNSVERSION_MASK) >> 16) >
- Message::EDNS_SUPPORTED_VERSION) {
- // XXX: we should probably not reject the message yet, because
- // it's better to let the requestor know the responder-side
- // highest version as indicated in Section 4.6 of RFC2671.
- // This is probably because why BIND 9 does the version check
- // in the client code.
- // This is a TODO item. Right now we simply reject it.
- const unsigned int ver =
- (ttl.getValue() & EDNSVERSION_MASK) >> 16;
- isc_throw(DNSMessageBADVERS, "unsupported EDNS version: " <<
- ver);
+
+ edns_ = createEDNSFromRR(name, rrclass, rrtype, ttl, rdata, rcode_);
+ continue;
+ } else {
+ vector<RRsetPtr>::iterator it =
+ find_if(rrsets_[sectionCodeToId(section)].begin(),
+ rrsets_[sectionCodeToId(section)].end(),
+ MatchRR(name, rrtype, rrclass));
+ if (it != rrsets_[sectionCodeToId(section)].end()) {
+ (*it)->setTTL(min((*it)->getTTL(), ttl));
+ (*it)->addRdata(rdata);
+ } else {
+ RRsetPtr rrset =
+ RRsetPtr(new RRset(name, rrclass, rrtype, ttl));
+ rrset->addRdata(rdata);
+ rrsets_[sectionCodeToId(section)].push_back(rrset);
}
- if (name != Name::ROOT_NAME()) {
- isc_throw(DNSMessageFORMERR,
- "invalid owner name for EDNS OPT RR");
- }
-
- remote_edns_ = RRsetPtr(new RRset(name, rrclass, rrtype, ttl));
- remote_edns_->addRdata(rdata);
-
- dnssec_ok_ = (((ttl.getValue() & EXTFLAG_MASK) & EXTFLAG_DO) != 0);
- if (rrclass.getCode() > Message::DEFAULT_MAX_UDPSIZE) {
- udpsize_ = rrclass.getCode();
- }
- rcode_ = Rcode(((ttl.getValue() & EXTRCODE_MASK) >> 20) |
- rcode_.getCode());
- continue;
+ ++added;
}
-
- vector<RRsetPtr>::iterator it =
- find_if(rrsets_[sectionCodeToId(section)].begin(),
- rrsets_[sectionCodeToId(section)].end(),
- MatchRR(name, rrtype, rrclass));
- if (it != rrsets_[sectionCodeToId(section)].end()) {
- (*it)->setTTL(min((*it)->getTTL(), ttl));
- (*it)->addRdata(rdata);
- } else {
- RRsetPtr rrset = RRsetPtr(new RRset(name, rrclass, rrtype, ttl));
- rrset->addRdata(rdata);
- rrsets_[sectionCodeToId(section)].push_back(rrset);
- }
- ++added;
}
return (added);
@@ -772,14 +682,16 @@
lexical_cast<string>(impl_->counts_[Section::AUTHORITY().getCode()]);
unsigned int arcount = impl_->counts_[Section::ADDITIONAL().getCode()];
- RRsetPtr edns_rrset;
- if (!getHeaderFlag(MessageFlag::QR()) && impl_->remote_edns_ != NULL) {
- edns_rrset = impl_->remote_edns_;
+ if (impl_->edns_ != NULL) {
++arcount;
}
s += ", ADDITIONAL: " + lexical_cast<string>(arcount) + "\n";
- if (edns_rrset != NULL) {
+#ifdef notyet
+ if (impl_->edns_ != NULL) {
+ impl_->ends_->toText();
+
+ // OLD CODE, TO BE REMOVED:
s += "\n;; OPT PSEUDOSECTION:\n";
s += "; EDNS: version: ";
s += lexical_cast<string>(
@@ -797,6 +709,7 @@
}
s += "\n";
}
+#endif
if (!impl_->questions_.empty()) {
s += "\n;; " +
@@ -846,11 +759,7 @@
impl_->mode_ = Message::RENDER;
- impl_->dnssec_ok_ = false;
- impl_->remote_udpsize_ = impl_->udpsize_;
- impl_->local_edns_ = RRsetPtr();
- impl_->udpsize_ = DEFAULT_MAX_UDPSIZE;
-
+ impl_->edns_ = EDNSPtr();
impl_->flags_ &= MESSAGE_REPLYPRESERVE;
setHeaderFlag(MessageFlag::QR());
Modified: branches/jinmei-ednsupdate/src/lib/dns/message.h
==============================================================================
--- branches/jinmei-ednsupdate/src/lib/dns/message.h (original)
+++ branches/jinmei-ednsupdate/src/lib/dns/message.h Thu Apr 15 21:31:50 2010
@@ -627,45 +627,6 @@
/// \c setHeaderFlag() with an additional argument.
void clearHeaderFlag(const MessageFlag& flag);
- /// \brief Return whether the message sender indicates DNSSEC is supported.
- /// If EDNS is included, this corresponds to the value of the DO bit.
- /// Otherwise, DNSSEC is considered not supported.
- bool isDNSSECSupported() const;
-
- /// \brief Specify whether DNSSEC is supported in the message.
- ///
- /// Only allowed in the \c RENDER mode.
- /// If EDNS is included in the message, the DO bit is set or cleared
- /// according to the specified value of this method.
- void setDNSSECSupported(bool on);
-
- /// \brief Return the maximum buffer size of UDP messages for the sender
- /// of the message.
- ///
- /// The semantics of this value is different based on the mode:
- /// In the \c PARSE mode, it means the buffer size of the remote node;
- /// in the \c RENDER mode, it means the buffer size of the local node.
- ///
- /// In either case, its value is the value of the UDP payload size field
- /// of EDNS (when it's included) or \c DEFAULT_MAX_UDPSIZE.
- ///
- /// Note: this interface may be confusing and may have to be revisited.
- uint16_t getUDPSize() const;
-
- /// \brief Specify the maximum buffer size of UDP messages of the local
- /// node.
- ///
- /// Only allowed in the \c RENDER mode.
- /// If EDNS OPT RR is included in the message, its UDP payload size field
- /// will be set to the specified value.
- ///
- /// Unless explicitly specified, \c DEFAULT_MAX_UDPSIZE will be assumed
- /// for the maximum buffer size, regardless of whether EDNS OPT RR is
- /// included or not. This means if an application wants to send a message
- /// with an EDNS OPT RR for specifying a larger UDP size, it must explicitly
- /// specify the value using this method.
- void setUDPSize(uint16_t size);
-
/// \brief Return the query ID given in the header section of the message.
qid_t getQid() const;
Modified: branches/jinmei-ednsupdate/src/lib/dns/tests/Makefile.am
==============================================================================
--- branches/jinmei-ednsupdate/src/lib/dns/tests/Makefile.am (original)
+++ branches/jinmei-ednsupdate/src/lib/dns/tests/Makefile.am Thu Apr 15 21:31:50 2010
@@ -9,6 +9,7 @@
TESTS += run_unittests
run_unittests_SOURCES = unittest_util.h unittest_util.cc
run_unittests_SOURCES += buffer_unittest.cc name_unittest.cc
+run_unittests_SOURCES += edns_unittest.cc
run_unittests_SOURCES += messagerenderer_unittest.cc
run_unittests_SOURCES += rrclass_unittest.cc rrtype_unittest.cc
run_unittests_SOURCES += rrttl_unittest.cc
Modified: branches/jinmei-ednsupdate/src/lib/dns/tests/message_unittest.cc
==============================================================================
--- branches/jinmei-ednsupdate/src/lib/dns/tests/message_unittest.cc (original)
+++ branches/jinmei-ednsupdate/src/lib/dns/tests/message_unittest.cc Thu Apr 15 21:31:50 2010
@@ -127,82 +127,7 @@
EXPECT_TRUE(it->isLast());
}
-TEST_F(MessageTest, GetEDNS0DOBit) {
- // Without EDNS0, DNSSEC is considered to be unsupported.
- factoryFromFile(message_parse, "message_fromWire1");
- EXPECT_FALSE(message_parse.isDNSSECSupported());
-
- // If DO bit is on, DNSSEC is considered to be supported.
- message_parse.clear(Message::PARSE);
- factoryFromFile(message_parse, "message_fromWire2");
- EXPECT_TRUE(message_parse.isDNSSECSupported());
-
- // If DO bit is off, DNSSEC is considered to be unsupported.
- message_parse.clear(Message::PARSE);
- factoryFromFile(message_parse, "message_fromWire3");
- EXPECT_FALSE(message_parse.isDNSSECSupported());
-}
-
-TEST_F(MessageTest, SetEDNS0DOBit) {
- // By default, it's false, and we can enable/disable it.
- EXPECT_FALSE(message_render.isDNSSECSupported());
- message_render.setDNSSECSupported(true);
- EXPECT_TRUE(message_render.isDNSSECSupported());
- message_render.setDNSSECSupported(false);
- EXPECT_FALSE(message_render.isDNSSECSupported());
-
- // A message in the parse mode doesn't allow this flag to be set.
- EXPECT_THROW(message_parse.setDNSSECSupported(true),
- InvalidMessageOperation);
- // Once converted to the render mode, it works as above
- message_parse.makeResponse();
- EXPECT_FALSE(message_parse.isDNSSECSupported());
- message_parse.setDNSSECSupported(true);
- EXPECT_TRUE(message_parse.isDNSSECSupported());
- message_parse.setDNSSECSupported(false);
- EXPECT_FALSE(message_parse.isDNSSECSupported());
-}
-
-TEST_F(MessageTest, GetEDNS0UDPSize) {
- // Without EDNS0, the default max UDP size is used.
- factoryFromFile(message_parse, "message_fromWire1");
- EXPECT_EQ(Message::DEFAULT_MAX_UDPSIZE, message_parse.getUDPSize());
-
- // If the size specified in EDNS0 > default max, use it.
- message_parse.clear(Message::PARSE);
- factoryFromFile(message_parse, "message_fromWire2");
- EXPECT_EQ(4096, message_parse.getUDPSize());
-
- // If the size specified in EDNS0 < default max, keep using the default.
- message_parse.clear(Message::PARSE);
- factoryFromFile(message_parse, "message_fromWire8");
- EXPECT_EQ(Message::DEFAULT_MAX_UDPSIZE, message_parse.getUDPSize());
-}
-
-TEST_F(MessageTest, SetEDNS0UDPSize) {
- // The default size if unspecified
- EXPECT_EQ(Message::DEFAULT_MAX_UDPSIZE, message_render.getUDPSize());
- // A common buffer size with EDNS, should succeed
- message_render.setUDPSize(4096);
- EXPECT_EQ(4096, message_render.getUDPSize());
- // Unusual large value, but accepted
- message_render.setUDPSize(0xffff);
- EXPECT_EQ(0xffff, message_render.getUDPSize());
- // Too small is value is rejected
- EXPECT_THROW(message_render.setUDPSize(511), InvalidMessageUDPSize);
-
- // A message in the parse mode doesn't allow the set operation.
- EXPECT_THROW(message_parse.setUDPSize(4096), InvalidMessageOperation);
- // Once converted to the render mode, it works as above.
- message_parse.makeResponse();
- message_parse.setUDPSize(4096);
- EXPECT_EQ(4096, message_parse.getUDPSize());
- message_parse.setUDPSize(0xffff);
- EXPECT_EQ(0xffff, message_parse.getUDPSize());
- EXPECT_THROW(message_parse.setUDPSize(511), InvalidMessageUDPSize);
-}
-
-TEST_F(MessageTest, EDNS0ExtCode) {
+TEST_F(MessageTest, EDNS0ExtRcode) {
// Extended Rcode = BADVERS
factoryFromFile(message_parse, "message_fromWire10");
EXPECT_EQ(Rcode::BADVERS(), message_parse.getRcode());
@@ -221,19 +146,6 @@
message_parse.clear(Message::PARSE);
EXPECT_THROW(factoryFromFile(message_parse, "message_fromWire5"),
DNSMessageFORMERR);
- // OPT RR of a non root name
- message_parse.clear(Message::PARSE);
- EXPECT_THROW(factoryFromFile(message_parse, "message_fromWire6"),
- DNSMessageFORMERR);
- // Compressed owner name of OPT RR points to a root name.
- // Not necessarily bogus, but very unusual and mostly pathological.
- // We accept it, but is it okay?
- message_parse.clear(Message::PARSE);
- EXPECT_NO_THROW(factoryFromFile(message_parse, "message_fromWire7"));
- // Unsupported Version
- message_parse.clear(Message::PARSE);
- EXPECT_THROW(factoryFromFile(message_parse, "message_fromWire9"),
- DNSMessageBADVERS);
}
TEST_F(MessageTest, toWire) {
More information about the bind10-changes
mailing list