Browse Source

[trac813] added support for TSIG specific handling in Message::fromWire().
also added a new method Message::getTSIGRecord() so that apps can get the
received TSIG record.

JINMEI Tatuya 14 years ago
parent
commit
0e3b55edc7

+ 89 - 26
src/lib/dns/message.cc

@@ -116,8 +116,8 @@ public:
     vector<QuestionPtr> questions_;
     vector<RRsetPtr> 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<RRsetPtr>::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<RRsetPtr>::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 <typename T>
 struct SectionFormatter {

+ 3 - 0
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

+ 68 - 0
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<void*>(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<void*>(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<void*>(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<void*>(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<void*>(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));

+ 6 - 1
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

+ 21 - 0
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

+ 20 - 0
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

+ 21 - 0
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

+ 22 - 0
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

+ 21 - 0
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