Browse Source

[trac910] handle a boundary condition that doesn't cause truncation.

JINMEI Tatuya 13 years ago
parent
commit
a29b113e5b

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

@@ -341,6 +341,8 @@ MessageImpl::toWire(AbstractMessageRenderer& renderer, TSIGContext* tsig_ctx) {
 
     // Add TSIG, if necessary, at the end of the message.
     if (tsig_ctx != NULL) {
+        // Release the reserved space in the renderer.
+        renderer.setLengthLimit(renderer.getLengthLimit() + tsig_len);
         tsig_ctx->sign(qid_, renderer.getData(),
                        renderer.getLength())->toWire(renderer);
 

+ 29 - 8
src/lib/dns/tests/message_unittest.cc

@@ -625,7 +625,6 @@ testGetTime() {
 const unsigned int QR_FLAG = 0x1;
 const unsigned int AA_FLAG = 0x2;
 const unsigned int RD_FLAG = 0x4;
-const unsigned int TC_FLAG = 0x8;
 
 void
 commonTSIGToWireCheck(Message& message, MessageRenderer& renderer,
@@ -645,9 +644,6 @@ commonTSIGToWireCheck(Message& message, MessageRenderer& renderer,
     if ((message_flags & RD_FLAG) != 0) {
         message.setHeaderFlag(Message::HEADERFLAG_RD);
     }
-    if ((message_flags & TC_FLAG) != 0) {
-        message.setHeaderFlag(Message::HEADERFLAG_TC);
-    }
     message.addQuestion(Question(Name("www.example.com"), RRClass::IN(),
                                  qtype));
 
@@ -723,6 +719,10 @@ const char* const long_txt2 = "0123456789abcdef0123456789abcdef0123456789abcdef0
 // adding a TSIG would result in truncation (33 + 268 + 127 + 85 = 513)
 const char* const long_txt3 = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef01";
 
+// This is 1 byte shorter than txt3, which will result in a possible longest
+// message containing answer RRs and TSIG.
+const char* const long_txt4 = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0";
+
 // Example output generated by
 // "dig -y www.example.com:SFuWd/q99SzF8Yzd1QbB9g== www.example.com txt
 // QID: 0x22c2
@@ -731,13 +731,12 @@ TEST_F(MessageTest, toWireTSIGTruncation) {
     isc::util::detail::gettimeFunction = testGetTime<0x4e179212>;
 
     // Verify a validly signed query so that we can use the TSIG context
-    // in a response mode.
+
     factoryFromFile(message_parse, "message_fromWire17.wire");
     EXPECT_EQ(TSIGError::NOERROR(),
               tsig_ctx.verify(message_parse.getTSIGRecord(),
                               &received_data[0], received_data.size()));
 
-
     message_render.setQid(0x22c2);
     vector<const char*> answer_data;
     answer_data.push_back(long_txt1);
@@ -746,7 +745,7 @@ TEST_F(MessageTest, toWireTSIGTruncation) {
         SCOPED_TRACE("Message sign with TSIG and TC bit on");
         commonTSIGToWireCheck(message_render, renderer, tsig_ctx,
                               "message_toWire4.wire",
-                              QR_FLAG|AA_FLAG|RD_FLAG|TC_FLAG,
+                              QR_FLAG|AA_FLAG|RD_FLAG,
                               RRType::TXT(), &answer_data);
     }
 }
@@ -768,7 +767,29 @@ TEST_F(MessageTest, toWireTSIGTruncation2) {
         SCOPED_TRACE("Message sign with TSIG and TC bit on (2)");
         commonTSIGToWireCheck(message_render, renderer, tsig_ctx,
                               "message_toWire4.wire",
-                              QR_FLAG|AA_FLAG|RD_FLAG|TC_FLAG,
+                              QR_FLAG|AA_FLAG|RD_FLAG,
+                              RRType::TXT(), &answer_data);
+    }
+}
+
+TEST_F(MessageTest, toWireTSIGNoTruncation) {
+    // A boundary case that shouldn't cause truncation: the resulting
+    // response message with a TSIG will be 512 bytes long.
+    isc::util::detail::gettimeFunction = testGetTime<0x4e17b38d>;
+    factoryFromFile(message_parse, "message_fromWire18.wire");
+    EXPECT_EQ(TSIGError::NOERROR(),
+              tsig_ctx.verify(message_parse.getTSIGRecord(),
+                              &received_data[0], received_data.size()));
+
+    message_render.setQid(0xd6e2);
+    vector<const char*> answer_data;
+    answer_data.push_back(long_txt1);
+    answer_data.push_back(long_txt4);
+    {
+        SCOPED_TRACE("Message sign with TSIG, no truncation");
+        commonTSIGToWireCheck(message_render, renderer, tsig_ctx,
+                              "message_toWire5.wire",
+                              QR_FLAG|AA_FLAG|RD_FLAG,
                               RRType::TXT(), &answer_data);
     }
 }

+ 4 - 3
src/lib/dns/tests/testdata/Makefile.am

@@ -6,8 +6,9 @@ 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 message_fromWire17.wire
+BUILT_SOURCES += message_fromWire18.wire
 BUILT_SOURCES += message_toWire2.wire message_toWire3.wire
-BUILT_SOURCES += message_toWire4.wire
+BUILT_SOURCES += message_toWire4.wire message_toWire5.wire
 BUILT_SOURCES += message_toText1.wire message_toText2.wire
 BUILT_SOURCES += message_toText3.wire
 BUILT_SOURCES += name_toWire5.wire name_toWire6.wire
@@ -60,9 +61,9 @@ EXTRA_DIST += message_fromWire9 message_fromWire10.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_fromWire17.spec
+EXTRA_DIST += message_fromWire17.spec message_fromWire18.spec
 EXTRA_DIST += message_toWire1 message_toWire2.spec message_toWire3.spec
-EXTRA_DIST += message_toWire4.wire
+EXTRA_DIST += message_toWire4.spec message_toWire5.spec
 EXTRA_DIST += message_toText1.txt message_toText1.spec
 EXTRA_DIST += message_toText2.txt message_toText2.spec
 EXTRA_DIST += message_toText3.txt message_toText3.spec

+ 5 - 7
src/lib/dns/tests/testdata/gen-wiredata.py.in

@@ -307,8 +307,8 @@ class SOA(RR):
                                                 self.retry, self.expire,
                                                 self.minimum))
 
-class TXT:
-    rdlen = -1                  # auto-calculate
+class TXT(RR):
+    rdlen = None                # auto-calculate
     nstring = 1                 # number of character-strings
     stringlen = -1              # default string length, auto-calculate
     string = 'Test String'      # default string
@@ -330,11 +330,9 @@ class TXT:
                 stringlen_list.append(self.stringlen)
             if stringlen_list[-1] < 0:
                 stringlen_list[-1] = int(len(wirestring_list[-1]) / 2)
-        rdlen = self.rdlen
-        if rdlen < 0:
-            rdlen = int(len(''.join(wirestring_list)) / 2) + self.nstring
-        f.write('\n# TXT RDATA (RDLEN=%d)\n' % rdlen)
-        f.write('%04x\n' % rdlen);
+        if self.rdlen is None:
+            self.rdlen = int(len(''.join(wirestring_list)) / 2) + self.nstring
+        self.dump_header(f, self.rdlen)
         for i in range(0, self.nstring):
             f.write('# String Len=%d, String=\"%s\"\n' %
                     (stringlen_list[i], string_list[i]))

+ 23 - 0
src/lib/dns/tests/testdata/message_fromWire18.spec

@@ -0,0 +1,23 @@
+#
+# Another simple DNS query message with TSIG signed.  Only ID and time signed
+# (and MAC as a result) are different.
+#
+
+[custom]
+sections: header:question:tsig
+[header]
+id: 0xd6e2
+rd: 1
+arcount: 1
+[question]
+name: www.example.com
+rrtype: TXT
+[tsig]
+as_rr: True
+# TSIG QNAME won't be compressed
+rr_name: www.example.com
+algorithm: hmac-md5
+time_signed: 0x4e17b38d
+mac_size: 16
+mac: 0x903b5b194a799b03a37718820c2404f2
+original_id: 0xd6e2

+ 36 - 0
src/lib/dns/tests/testdata/message_toWire5.spec

@@ -0,0 +1,36 @@
+#
+# A longest possible (without EDNS) DNS response with TSIG, i.e. totatl
+# length should be 512 bytes.
+#
+
+[custom]
+sections: header:question:txt/1:txt/2:tsig
+[header]
+id: 0xd6e2
+rd: 1
+qr: 1
+aa: 1
+ancount: 2
+arcount: 1
+[question]
+name: www.example.com
+rrtype: TXT
+[txt/1]
+as_rr: True
+# QNAME is fully compressed
+rr_name: ptr=12
+string: 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcde
+[txt/2]
+as_rr: True
+# QNAME is fully compressed
+rr_name: ptr=12
+string: 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0
+[tsig]
+as_rr: True
+# TSIG QNAME won't be compressed
+rr_name: www.example.com
+algorithm: hmac-md5
+time_signed: 0x4e17b38d
+mac_size: 16
+mac: 0xbe2ba477373d2496891e2fda240ee4ec
+original_id: 0xd6e2