diff --git a/src/lib/dns/message.cc b/src/lib/dns/message.cc index 1d43637ea9..bf4ae2ed1a 100644 --- a/src/lib/dns/message.cc +++ b/src/lib/dns/message.cc @@ -116,8 +116,8 @@ public: vector questions_; vector rrsets_[NUM_SECTIONS]; ConstEDNSPtr edns_; + ConstTSIGRecordPtr tsig_rr_; - // tsig/sig0: TODO // RRsetsSorter* sorter_; : TODO void init(); @@ -125,6 +125,16 @@ public: void setRcode(const Rcode& rcode); int parseQuestion(InputBuffer& buffer); int parseSection(const Message::Section section, InputBuffer& buffer); + void addRR(Message::Section section, const Name& name, + const RRClass& rrclass, const RRType& rrtype, + const RRTTL& ttl, ConstRdataPtr rdata); + void addEDNS(Message::Section section, const Name& name, + const RRClass& rrclass, const RRType& rrtype, + const RRTTL& ttl, const Rdata& rdata); + void addTSIG(Message::Section section, unsigned int count, + const InputBuffer& buffer, size_t start_position, + const Name& name, const RRClass& rrclass, + const RRTTL& ttl, const Rdata& rdata); void toWire(AbstractMessageRenderer& renderer, TSIGContext* tsig_ctx); }; @@ -143,6 +153,7 @@ MessageImpl::init() { rcode_ = NULL; opcode_ = NULL; edns_ = EDNSPtr(); + tsig_rr_ = ConstTSIGRecordPtr(); for (int i = 0; i < NUM_SECTIONS; ++i) { counts_[i] = 0; @@ -398,6 +409,16 @@ Message::setEDNS(ConstEDNSPtr edns) { impl_->edns_ = edns; } +const TSIGRecord* +Message::getTSIGRecord() const { + if (impl_->mode_ != Message::PARSE) { + isc_throw(InvalidMessageOperation, + "getTSIGRecord performed in non-parse mode"); + } + + return (impl_->tsig_rr_.get()); +} + unsigned int Message::getRRCount(const Section section) const { if (section >= MessageImpl::NUM_SECTIONS) { @@ -634,6 +655,9 @@ MessageImpl::parseSection(const Message::Section section, unsigned int added = 0; for (unsigned int count = 0; count < counts_[section]; ++count) { + // We need to remember the start position for TSIG processing + const size_t start_position = buffer.getPosition(); + const Name name(buffer); // buffer must store at least RR TYPE, RR CLASS, TTL, and RDLEN. @@ -651,32 +675,12 @@ MessageImpl::parseSection(const Message::Section section, ConstRdataPtr rdata = createRdata(rrtype, rrclass, buffer, rdlen); if (rrtype == RRType::OPT()) { - if (section != Message::SECTION_ADDITIONAL) { - isc_throw(DNSMessageFORMERR, - "EDNS OPT RR found in an invalid section"); - } - if (edns_) { - isc_throw(DNSMessageFORMERR, "multiple EDNS OPT RR found"); - } - - uint8_t extended_rcode; - edns_ = ConstEDNSPtr(createEDNSFromRR(name, rrclass, rrtype, ttl, - *rdata, extended_rcode)); - setRcode(Rcode(rcode_->getCode(), extended_rcode)); - continue; + addEDNS(section, name, rrclass, rrtype, ttl, *rdata); + } else if (rrtype == RRType::TSIG()) { + addTSIG(section, count, buffer, start_position, name, rrclass, ttl, + *rdata); } else { - vector::iterator it = - find_if(rrsets_[section].begin(), rrsets_[section].end(), - MatchRR(name, rrtype, rrclass)); - if (it != rrsets_[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_[section].push_back(rrset); - } + addRR(section, name, rrclass, rrtype, ttl, rdata); ++added; } } @@ -684,6 +688,65 @@ MessageImpl::parseSection(const Message::Section section, return (added); } +void +MessageImpl::addRR(Message::Section section, const Name& name, + const RRClass& rrclass, const RRType& rrtype, + const RRTTL& ttl, ConstRdataPtr rdata) +{ + vector::iterator it = + find_if(rrsets_[section].begin(), rrsets_[section].end(), + MatchRR(name, rrtype, rrclass)); + if (it != rrsets_[section].end()) { + (*it)->setTTL(min((*it)->getTTL(), ttl)); + (*it)->addRdata(rdata); + } else { + RRsetPtr rrset(new RRset(name, rrclass, rrtype, ttl)); + rrset->addRdata(rdata); + rrsets_[section].push_back(rrset); + } +} + +void +MessageImpl::addEDNS(Message::Section section, const Name& name, + const RRClass& rrclass, const RRType& rrtype, + const RRTTL& ttl, const Rdata& rdata) +{ + if (section != Message::SECTION_ADDITIONAL) { + isc_throw(DNSMessageFORMERR, + "EDNS OPT RR found in an invalid section"); + } + if (edns_) { + isc_throw(DNSMessageFORMERR, "multiple EDNS OPT RR found"); + } + + uint8_t extended_rcode; + edns_ = ConstEDNSPtr(createEDNSFromRR(name, rrclass, rrtype, ttl, rdata, + extended_rcode)); + setRcode(Rcode(rcode_->getCode(), extended_rcode)); +} + +void +MessageImpl::addTSIG(Message::Section section, unsigned int count, + const InputBuffer& buffer, size_t start_position, + const Name& name, const RRClass& rrclass, + const RRTTL& ttl, const Rdata& rdata) +{ + if (section != Message::SECTION_ADDITIONAL) { + isc_throw(DNSMessageFORMERR, + "TSIG RR found in an invalid section"); + } + if (count != counts_[section] - 1) { + isc_throw(DNSMessageFORMERR, "TSIG RR is not the last record"); + } + if (tsig_rr_) { + isc_throw(DNSMessageFORMERR, "multiple TSIG RRs found"); + } + tsig_rr_ = ConstTSIGRecordPtr(new TSIGRecord(name, rrclass, + ttl, rdata, + buffer.getPosition() - + start_position)); +} + namespace { template struct SectionFormatter { diff --git a/src/lib/dns/message.h b/src/lib/dns/message.h index cd3ae88da8..69a5d05368 100644 --- a/src/lib/dns/message.h +++ b/src/lib/dns/message.h @@ -34,6 +34,7 @@ class InputBuffer; namespace dns { class TSIGContext; +class TSIGRecord; /// /// \brief A standard DNS module exception that is thrown if a wire format @@ -369,6 +370,8 @@ public: /// \c Message. void setEDNS(ConstEDNSPtr edns); + const TSIGRecord* getTSIGRecord() const; + /// \brief Returns the number of RRs contained in the given section. /// /// In the \c PARSE mode, the returned value may not be identical to diff --git a/src/lib/dns/tests/message_unittest.cc b/src/lib/dns/tests/message_unittest.cc index c92b950282..6ad428a421 100644 --- a/src/lib/dns/tests/message_unittest.cc +++ b/src/lib/dns/tests/message_unittest.cc @@ -67,6 +67,10 @@ extern int64_t (*gettimeFunction)(); } } +// XXX: this is defined as class static constants, but some compilers +// seemingly cannot find the symbol when used in the EXPECT_xxx macros. +const uint16_t TSIGContext::DEFAULT_FUDGE; + namespace { class MessageTest : public ::testing::Test { protected: @@ -185,6 +189,70 @@ TEST_F(MessageTest, setEDNS) { EXPECT_EQ(edns, message_render.getEDNS()); } +TEST_F(MessageTest, fromWireWithTSIG) { + // Initially there should be no TSIG + EXPECT_EQ(static_cast(NULL), message_parse.getTSIGRecord()); + + // getTSIGRecord() is only valid in the parse mode. + EXPECT_THROW(message_render.getTSIGRecord(), InvalidMessageOperation); + + factoryFromFile(message_parse, "message_toWire2.wire"); + const char expected_mac[] = { + 0x22, 0x70, 0x26, 0xad, 0x29, 0x7b, 0xee, 0xe7, + 0x21, 0xce, 0x6c, 0x6f, 0xff, 0x1e, 0x9e, 0xf3 + }; + const TSIGRecord* tsig_rr = message_parse.getTSIGRecord(); + ASSERT_NE(static_cast(NULL), tsig_rr); + EXPECT_EQ(Name("www.example.com"), tsig_rr->getName()); + EXPECT_EQ(85, tsig_rr->getLength()); // see TSIGRecordTest.getLength + EXPECT_EQ(TSIGKey::HMACMD5_NAME(), tsig_rr->getRdata().getAlgorithm()); + EXPECT_EQ(0x4da8877a, tsig_rr->getRdata().getTimeSigned()); + EXPECT_EQ(TSIGContext::DEFAULT_FUDGE, tsig_rr->getRdata().getFudge()); + EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, + tsig_rr->getRdata().getMAC(), + tsig_rr->getRdata().getMACSize(), + expected_mac, sizeof(expected_mac)); + EXPECT_EQ(0, tsig_rr->getRdata().getError()); + EXPECT_EQ(0, tsig_rr->getRdata().getOtherLen()); + EXPECT_EQ(static_cast(NULL), tsig_rr->getRdata().getOtherData()); + + // If we clear the message for reuse, the recorded TSIG will be cleared. + message_parse.clear(Message::PARSE); + EXPECT_EQ(static_cast(NULL), message_parse.getTSIGRecord()); +} + +TEST_F(MessageTest, fromWireWithTSIGCompressed) { + // Mostly same as fromWireWithTSIG, but the TSIG owner name is compressed. + factoryFromFile(message_parse, "message_fromWire12.wire"); + const TSIGRecord* tsig_rr = message_parse.getTSIGRecord(); + ASSERT_NE(static_cast(NULL), tsig_rr); + EXPECT_EQ(Name("www.example.com"), tsig_rr->getName()); + // len(www.example.com) = 17, but when fully compressed, the length is + // 2 bytes. So the length of the record should be 15 bytes shorter. + EXPECT_EQ(70, tsig_rr->getLength()); +} + +TEST_F(MessageTest, fromWireWithBadTSIG) { + // Multiple TSIG RRs + EXPECT_THROW(factoryFromFile(message_parse, "message_fromWire13.wire"), + DNSMessageFORMERR); + message_parse.clear(Message::PARSE); + + // TSIG in the answer section (must be in additional) + EXPECT_THROW(factoryFromFile(message_parse, "message_fromWire14.wire"), + DNSMessageFORMERR); + message_parse.clear(Message::PARSE); + + // TSIG is not the last record. + EXPECT_THROW(factoryFromFile(message_parse, "message_fromWire15.wire"), + DNSMessageFORMERR); + message_parse.clear(Message::PARSE); + + // Unexpected RR Class (this will fail in constructing TSIGRecord) + EXPECT_THROW(factoryFromFile(message_parse, "message_fromWire16.wire"), + DNSMessageFORMERR); +} + TEST_F(MessageTest, getRRCount) { // by default all counters should be 0 EXPECT_EQ(0, message_render.getRRCount(Message::SECTION_QUESTION)); diff --git a/src/lib/dns/tests/testdata/Makefile.am b/src/lib/dns/tests/testdata/Makefile.am index 0c263933d2..97610b076f 100644 --- a/src/lib/dns/tests/testdata/Makefile.am +++ b/src/lib/dns/tests/testdata/Makefile.am @@ -3,6 +3,9 @@ CLEANFILES = *.wire BUILT_SOURCES = edns_toWire1.wire edns_toWire2.wire edns_toWire3.wire BUILT_SOURCES += edns_toWire4.wire BUILT_SOURCES += message_fromWire10.wire message_fromWire11.wire +BUILT_SOURCES += message_fromWire12.wire message_fromWire13.wire +BUILT_SOURCES += message_fromWire14.wire message_fromWire15.wire +BUILT_SOURCES += message_fromWire16.wire BUILT_SOURCES += message_toWire2.wire message_toWire3.wire BUILT_SOURCES += name_toWire5.wire name_toWire6.wire BUILT_SOURCES += rdatafields1.wire rdatafields2.wire rdatafields3.wire @@ -47,7 +50,9 @@ EXTRA_DIST += message_fromWire3 message_fromWire4 EXTRA_DIST += message_fromWire5 message_fromWire6 EXTRA_DIST += message_fromWire7 message_fromWire8 EXTRA_DIST += message_fromWire9 message_fromWire10.spec -EXTRA_DIST += message_fromWire11.spec +EXTRA_DIST += message_fromWire11.spec message_fromWire12.spec +EXTRA_DIST += message_fromWire13.spec message_fromWire14.spec +EXTRA_DIST += message_fromWire15.spec message_fromWire16.spec EXTRA_DIST += message_toWire1 message_toWire2.spec message_toWire3.spec EXTRA_DIST += name_fromWire1 name_fromWire2 name_fromWire3_1 name_fromWire3_2 EXTRA_DIST += name_fromWire4 name_fromWire6 name_fromWire7 name_fromWire8 diff --git a/src/lib/dns/tests/testdata/message_fromWire12.spec b/src/lib/dns/tests/testdata/message_fromWire12.spec new file mode 100644 index 0000000000..4eadeed338 --- /dev/null +++ b/src/lib/dns/tests/testdata/message_fromWire12.spec @@ -0,0 +1,21 @@ +# +# A simple DNS response message with TSIG signed, but the owner name of TSIG +# is compressed +# + +[custom] +sections: header:question:tsig +[header] +id: 0x2d65 +rd: 1 +arcount: 1 +[question] +name: www.example.com +[tsig] +as_rr: True +rr_name: ptr=12 +algorithm: hmac-md5 +time_signed: 0x4da8877a +mac_size: 16 +mac: 0x227026ad297beee721ce6c6fff1e9ef3 +original_id: 0x2d65 diff --git a/src/lib/dns/tests/testdata/message_fromWire13.spec b/src/lib/dns/tests/testdata/message_fromWire13.spec new file mode 100644 index 0000000000..e81ec4cba3 --- /dev/null +++ b/src/lib/dns/tests/testdata/message_fromWire13.spec @@ -0,0 +1,20 @@ +# +# Invalid TSIG: containing 2 TSIG RRs. +# + +[custom] +sections: header:question:tsig:tsig +[header] +id: 0x2d65 +rd: 1 +arcount: 2 +[question] +name: www.example.com +[tsig] +as_rr: True +rr_name: www.example.com +algorithm: hmac-md5 +time_signed: 0x4da8877a +mac_size: 16 +mac: 0x227026ad297beee721ce6c6fff1e9ef3 +original_id: 0x2d65 diff --git a/src/lib/dns/tests/testdata/message_fromWire14.spec b/src/lib/dns/tests/testdata/message_fromWire14.spec new file mode 100644 index 0000000000..bf68a93269 --- /dev/null +++ b/src/lib/dns/tests/testdata/message_fromWire14.spec @@ -0,0 +1,21 @@ +# +# Invalid TSIG: not in the additional section. +# + +[custom] +sections: header:question:tsig +[header] +id: 0x2d65 +rd: 1 +# TSIG goes to the answer section +ancount: 1 +[question] +name: www.example.com +[tsig] +as_rr: True +rr_name: www.example.com +algorithm: hmac-md5 +time_signed: 0x4da8877a +mac_size: 16 +mac: 0x227026ad297beee721ce6c6fff1e9ef3 +original_id: 0x2d65 diff --git a/src/lib/dns/tests/testdata/message_fromWire15.spec b/src/lib/dns/tests/testdata/message_fromWire15.spec new file mode 100644 index 0000000000..0aa476f3d1 --- /dev/null +++ b/src/lib/dns/tests/testdata/message_fromWire15.spec @@ -0,0 +1,22 @@ +# +# Invalid TSIG: not at the end of the message +# + +[custom] +sections: header:question:tsig:edns +[header] +id: 0x2d65 +rd: 1 +arcount: 2 +[question] +name: www.example.com +[tsig] +as_rr: True +rr_name: www.example.com +algorithm: hmac-md5 +time_signed: 0x4da8877a +mac_size: 16 +mac: 0x227026ad297beee721ce6c6fff1e9ef3 +original_id: 0x2d65 +[edns] +#) (all default diff --git a/src/lib/dns/tests/testdata/message_fromWire16.spec b/src/lib/dns/tests/testdata/message_fromWire16.spec new file mode 100644 index 0000000000..be0abc355a --- /dev/null +++ b/src/lib/dns/tests/testdata/message_fromWire16.spec @@ -0,0 +1,21 @@ +# +# Invalid TSIG: not in the additional section. +# + +[custom] +sections: header:question:tsig +[header] +id: 0x2d65 +rd: 1 +arcount: 1 +[question] +name: www.example.com +[tsig] +as_rr: True +rr_class: IN +rr_name: www.example.com +algorithm: hmac-md5 +time_signed: 0x4da8877a +mac_size: 16 +mac: 0x227026ad297beee721ce6c6fff1e9ef3 +original_id: 0x2d65