Browse Source

[trac812next] added documentation for TSIGRecord
also made the return value of toWire() compatible with its RRset variants,
and added truncation case consideration.

JINMEI Tatuya 14 years ago
parent
commit
a42b39784b
3 changed files with 195 additions and 6 deletions
  1. 121 0
      src/lib/dns/tests/tsigrecord_unittest.cc
  2. 10 3
      src/lib/dns/tsigrecord.cc
  3. 64 3
      src/lib/dns/tsigrecord.h

+ 121 - 0
src/lib/dns/tests/tsigrecord_unittest.cc

@@ -0,0 +1,121 @@
+// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <vector>
+#include <sstream>
+
+#include <gtest/gtest.h>
+
+#include <util/buffer.h>
+
+#include <dns/messagerenderer.h>
+#include <dns/name.h>
+#include <dns/rdataclass.h>
+#include <dns/tsig.h>
+#include <dns/tsigkey.h>
+#include <dns/tsigrecord.h>
+
+#include <dns/tests/unittest_util.h>
+
+using namespace std;
+using namespace isc::util;
+using namespace isc::dns;
+using namespace isc::dns::rdata;
+using isc::UnitTestUtil;
+
+namespace {
+class TSIGRecordTest : public ::testing::Test {
+protected:
+    TSIGRecordTest() :
+        test_name("www.example.com"), test_mac(16, 0xda),
+        test_record(test_name, any::TSIG(TSIGKey::HMACMD5_NAME(), 0x4da8877a,
+                                         TSIGContext::DEFAULT_FUDGE,
+                                         test_mac.size(), &test_mac[0],
+                                         0x2d65, 0, 0, NULL)),
+        buffer(0), renderer(buffer)
+    {}
+    const Name test_name;
+    vector<unsigned char> test_mac;
+    const TSIGRecord test_record;
+    OutputBuffer buffer;
+    MessageRenderer renderer;
+    vector<unsigned char> data;
+};
+
+TEST_F(TSIGRecordTest, getName) {
+    EXPECT_EQ(test_name, test_record.getName());
+}
+
+TEST_F(TSIGRecordTest, getLength) {
+    // 83 = 17 + 26 + 16 + 24
+    // len(www.example.com) = 17
+    // len(hmac-md5.sig-alg.reg.int) = 26
+    // len(MAC) = 16
+    // the rest are fixed length fields (24 in total)
+    EXPECT_EQ(83, test_record.getLength());
+}
+
+TEST_F(TSIGRecordTest, recordToWire) {
+    UnitTestUtil::readWireData("tsigrecord_toWire1.wire", data);
+    EXPECT_EQ(1, test_record.toWire(renderer));
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
+                        renderer.getData(), renderer.getLength(),
+                        &data[0], data.size());
+
+    // Same test for a dumb buffer
+    buffer.clear();
+    EXPECT_EQ(1, test_record.toWire(buffer));
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
+                        buffer.getData(), buffer.getLength(),
+                        &data[0], data.size());    
+}
+
+TEST_F(TSIGRecordTest, recordToOLongToWire) {
+    // Rendering the test record requires a room of 83 bytes (see the
+    // getLength test).  By setting the limit to 82, it will fail, and
+    // the renderer will be marked as "truncated".
+    renderer.setLengthLimit(82);
+    EXPECT_FALSE(renderer.isTruncated()); // not marked before render attempt
+    EXPECT_EQ(0, test_record.toWire(renderer));
+    EXPECT_TRUE(renderer.isTruncated());
+}
+
+TEST_F(TSIGRecordTest, recordToWireAfterNames) {
+    // A similar test but the TSIG RR follows some domain names that could
+    // cause name compression inside TSIG.  Our implementation shouldn't
+    // compress either owner (key) name or the algorithm name.  This test
+    // confirms that.
+
+    UnitTestUtil::readWireData("tsigrecord_toWire2.wire", data);
+    renderer.writeName(TSIGKey::HMACMD5_NAME());
+    renderer.writeName(Name("foo.example.com"));
+    EXPECT_EQ(1, test_record.toWire(renderer));
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
+                        renderer.getData(), renderer.getLength(),
+                        &data[0], data.size());
+}
+
+TEST_F(TSIGRecordTest, toText) {
+    EXPECT_EQ("www.example.com. 0 ANY TSIG hmac-md5.sig-alg.reg.int. "
+              "1302890362 300 16 2tra2tra2tra2tra2tra2g== 11621 NOERROR 0\n",
+              test_record.toText());
+}
+
+// test operator<<.  We simply confirm it appends the result of toText().
+TEST_F(TSIGRecordTest, LeftShiftOperator) {
+    ostringstream oss;
+    oss << test_record;
+    EXPECT_EQ(test_record.toText(), oss.str());
+}
+} // end namespace

+ 10 - 3
src/lib/dns/tsigrecord.cc

@@ -73,18 +73,25 @@ toWireCommon(OUTPUT& output, const rdata::any::TSIG& rdata) {
 }
 }
 
-void
+int
 TSIGRecord::toWire(AbstractMessageRenderer& renderer) const {
+    // If adding the TSIG would exceed the size limit, don't do it.
+    if (renderer.getLength() + length_ > renderer.getLengthLimit()) {
+        renderer.setTruncated();
+        return (0);
+    }
+
     // key name = owner.  note that we disable compression.
     renderer.writeName(key_name_, false);
-
     toWireCommon(renderer, rdata_);
+    return (1);
 }
 
-void
+int
 TSIGRecord::toWire(OutputBuffer& buffer) const {
     key_name_.toWire(buffer);
     toWireCommon(buffer, rdata_);
+    return (1);
 }
 
 std::string

+ 64 - 3
src/lib/dns/tsigrecord.h

@@ -65,6 +65,9 @@ public:
     /// RDATA fails
     TSIGRecord(const Name& key_name, const rdata::any::TSIG& tsig_rdata);
 
+    /// Return the owner name of the TSIG RR, which is the TSIG key name
+    ///
+    /// \exception None
     const Name& getName() const { return (key_name_); }
 
     /// Return the RDATA of the TSIG RR
@@ -84,13 +87,71 @@ public:
     /// \exception None
     static const RRClass& getClass();
 
-    // Note: More important for the "from wire" case.
+    /// Return the length of the TSIG record
+    ///
+    /// When constructed from the key name and RDATA, it is the length of
+    /// the record when it is rendered by the \c toWire() method.
+    ///
+    /// \note When constructed "from wire", that will mean the length of
+    /// the wire format data for the TSIG RR.  The length will be necessary
+    /// to verify the message once parse is completed.  But this part is not
+    /// implemented yet.
+    ///
+    /// \exception None
     size_t getLength() const { return (length_); }
 
-    void toWire(AbstractMessageRenderer& renderer) const;
+    /// \brief Render the \c TSIG RR in the wire format.
+    ///
+    /// This method renders the TSIG record as a form of a DNS TSIG RR
+    /// via \c renderer, which encapsulates output buffer and other rendering
+    /// contexts.
+    ///
+    /// Normally this version of \c toWire() method tries to compress the
+    /// owner name of the RR whenever possible, but this method intentionally
+    /// skips owner name compression.  This is due to a report that some
+    /// Windows clients refuse a TSIG if its owner name is compressed
+    /// (See http://marc.info/?l=bind-workers&m=126637138430819&w=2).
+    /// Reportedly this seemed to be specific to GSS-TSIG, but this
+    /// implementation skip compression regardless of the algorithm.
+    ///
+    /// If by adding the TSIG RR the message size would exceed the limit
+    /// maintained in \c renderer, this method skips rendering the RR
+    /// and returns 0 and mark \c renderer as "truncated" (so that a
+    /// subsequent call to \c isTruncated() on \c renderer will result in
+    /// \c true); otherwise it returns 1, which is the number of RR
+    /// rendered.
+    ///
+    /// \note If the caller follows the specification of adding TSIG
+    /// as described in RFC2845, this should not happen; the caller is
+    /// generally expected to leave a sufficient room in the message for
+    /// the TSIG.  But this method checks the unexpected case nevertheless.
+    ///
+    /// \exception std::bad_alloc Internal resource allocation fails (this
+    /// should be rare).
+    ///
+    /// \param renderer DNS message rendering context that encapsulates the
+    /// output buffer and name compression information.
+    /// \return 1 if the TSIG RR fits in the message size limit; otherwise 0.
+    int toWire(AbstractMessageRenderer& renderer) const;
 
-    void toWire(isc::util::OutputBuffer& buffer) const;
+    /// \brief Render the \c TSIG RR in the wire format.
+    ///
+    /// This method is same as \c toWire(AbstractMessageRenderer&)const
+    /// except it renders the RR in an \c OutputBuffer and therefore
+    /// does not care about message size limit.
+    /// As a consequence it always returns 1.
+    int toWire(isc::util::OutputBuffer& buffer) const;
 
+    /// Convert the TSIG record to a string.
+    ///
+    /// The output format is the same as the result of \c toText() for
+    /// other normal types of RRsets (with always using the same RR class
+    /// and TTL).  It also ends with a newline.
+    ///
+    /// \exception std::bad_alloc Internal resource allocation fails (this
+    /// should be rare).
+    ///
+    /// \return A string representation of \c TSIG record
     std::string toText() const;
 
     /// The TTL value to be used in TSIG RRs.