[trac813] supported another form of TSIGRecord constructore, intending to

use it in "from wire" cases.  also corrected RR_COMMON_LEN and related tests.

src/lib/dns/tests/tsigrecord_unittest.cc

@@ -19,8 +19,10 @@
 
 #include <util/buffer.h>
 
+#include <dns/exceptions.h>
 #include <dns/messagerenderer.h>
 #include <dns/name.h>
+#include <dns/rdata.h>
 #include <dns/rdataclass.h>
 #include <dns/tsig.h>
 #include <dns/tsigkey.h>
@@ -39,14 +41,16 @@ 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,
+        test_rdata(any::TSIG(TSIGKey::HMACMD5_NAME(), 0x4da8877a,
-                                         TSIGContext::DEFAULT_FUDGE,
+                             TSIGContext::DEFAULT_FUDGE,
-                                         test_mac.size(), &test_mac[0],
+                             test_mac.size(), &test_mac[0],
-                                         0x2d65, 0, 0, NULL)),
+                             0x2d65, 0, 0, NULL)),
+        test_record(test_name, test_rdata),
         buffer(0), renderer(buffer)
     {}
     const Name test_name;
     vector<unsigned char> test_mac;
+    const any::TSIG test_rdata;
     const TSIGRecord test_record;
     OutputBuffer buffer;
     MessageRenderer renderer;
@@ -58,12 +62,48 @@ TEST_F(TSIGRecordTest, getName) {
 }
 
 TEST_F(TSIGRecordTest, getLength) {
-    // 83 = 17 + 26 + 16 + 24
+    // 85 = 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)
+    // the rest are fixed length fields (26 in total)
-    EXPECT_EQ(83, test_record.getLength());
+    EXPECT_EQ(85, test_record.getLength());
+}
+
+TEST_F(TSIGRecordTest, fromParams) {
+    // Construct the same TSIG RR as test_record from parameters.
+    // See the getLength test for the magic number of 85 (although it
+    // actually doesn't matter)
+    const TSIGRecord record(test_name, TSIGRecord::getClass(),
+                            TSIGRecord::getTTL(), test_rdata, 85);
+    // Perform straight sanity checks
+    EXPECT_EQ(test_name, record.getName());
+    EXPECT_EQ(85, record.getLength());
+    EXPECT_EQ(0, test_rdata.compare(record.getRdata()));
+
+    // The constructor doesn't check the length...
+    EXPECT_NO_THROW(TSIGRecord(test_name, TSIGRecord::getClass(),
+                               TSIGRecord::getTTL(), test_rdata, 82));
+    // ...even for impossibly small values...
+    EXPECT_NO_THROW(TSIGRecord(test_name, TSIGRecord::getClass(),
+                               TSIGRecord::getTTL(), test_rdata, 1));
+    // ...or too large values.
+    EXPECT_NO_THROW(TSIGRecord(test_name, TSIGRecord::getClass(),
+                               TSIGRecord::getTTL(), test_rdata, 65536));
+
+    // RDATA must indeed be TSIG
+    EXPECT_THROW(TSIGRecord(test_name, TSIGRecord::getClass(),
+                            TSIGRecord::getTTL(), in::A("192.0.2.1"), 85),
+                 DNSMessageFORMERR);
+
+    // Unexpected class
+    EXPECT_THROW(TSIGRecord(test_name, RRClass::IN(), TSIGRecord::getTTL(),
+                            test_rdata, 85),
+                 DNSMessageFORMERR);
+
+    // Unexpected TTL (simply ignored)
+    EXPECT_NO_THROW(TSIGRecord(test_name, TSIGRecord::getClass(),
+                               RRTTL(3600), test_rdata, 85));
 }
 
 TEST_F(TSIGRecordTest, recordToWire) {
@@ -82,10 +122,10 @@ TEST_F(TSIGRecordTest, recordToWire) {
 }
 
 TEST_F(TSIGRecordTest, recordToOLongToWire) {
-    // Rendering the test record requires a room of 83 bytes (see the
+    // Rendering the test record requires a room of 85 bytes (see the
-    // getLength test).  By setting the limit to 82, it will fail, and
+    // getLength test).  By setting the limit to 84, it will fail, and
     // the renderer will be marked as "truncated".
-    renderer.setLengthLimit(82);
+    renderer.setLengthLimit(84);
     EXPECT_FALSE(renderer.isTruncated()); // not marked before render attempt
     EXPECT_EQ(0, test_record.toWire(renderer));
     EXPECT_TRUE(renderer.isTruncated());

src/lib/dns/tsigrecord.cc

@@ -17,18 +17,20 @@
 
 #include <util/buffer.h>
 
+#include <dns/exceptions.h>
 #include <dns/messagerenderer.h>
 #include <dns/rrclass.h>
 #include <dns/rrttl.h>
 #include <dns/tsigrecord.h>
 
 using namespace isc::util;
+using namespace isc::dns::rdata;
 
 namespace {
 // Internally used constants:
 
-// Size in octets for the RR type, class TTL fields.
+// Size in octets for the RR type, class TTL, RDLEN fields.
-const size_t RR_COMMON_LEN = 8;
+const size_t RR_COMMON_LEN = 10;
 
 // Size in octets for the fixed part of TSIG RDATAs.
 // - Time Signed (6)
@@ -50,11 +52,45 @@ TSIGRecord::TSIGRecord(const Name& key_name,
             rdata_.getMACSize() + rdata_.getOtherLen())
 {}
 
+namespace {
+// This is a straightforward wrapper of dynamic_cast<const any::TSIG&>.
+// We use this so that we can throw the DNSMessageFORMERR exception when
+// unexpected type of RDATA is detected in the member initialization list
+// of the constructor below.
+const any::TSIG&
+castToTSIGRdata(const rdata::Rdata& rdata) {
+    try {
+        return (dynamic_cast<const any::TSIG&>(rdata));
+    } catch (std::bad_cast&) {
+        isc_throw(DNSMessageFORMERR,
+                  "TSIG record is being constructed from "
+                  "incompatible RDATA:" << rdata.toText());
+    }
+}
+}
+
+TSIGRecord::TSIGRecord(const Name& name, const RRClass& rrclass,
+                       const RRTTL&, // we ignore TTL
+                       const rdata::Rdata& rdata,
+                       size_t length) :
+    key_name_(name), rdata_(castToTSIGRdata(rdata)), length_(length)
+{
+    if (rrclass != getClass()) {
+        isc_throw(DNSMessageFORMERR, "Unexpected TSIG RR class: " << rrclass);
+    }
+}
+
 const RRClass&
 TSIGRecord::getClass() {
     return (RRClass::ANY());
 }
 
+const RRTTL&
+TSIGRecord::getTTL() {
+    static RRTTL ttl(TSIG_TTL);
+    return (ttl);
+}
+
 namespace {
 template <typename OUTPUT>
 void

src/lib/dns/tsigrecord.h

@@ -20,6 +20,8 @@
 
 #include <boost/shared_ptr.hpp>
 
+#include <util/buffer.h>
+
 #include <dns/name.h>
 #include <dns/rdataclass.h>
 
@@ -65,6 +67,12 @@ public:
     /// RDATA fails
     TSIGRecord(const Name& key_name, const rdata::any::TSIG& tsig_rdata);
 
+    /// Constructor from resource record (RR) parameters.
+    ///
+    /// \exception DNSMessageFORMERR
+    TSIGRecord(const Name& name, const RRClass& rrclass, const RRTTL& ttl,
+               const rdata::Rdata& rdata, size_t length);
+
     /// Return the owner name of the TSIG RR, which is the TSIG key name
     ///
     /// \exception None
@@ -87,6 +95,15 @@ public:
     /// \exception None
     static const RRClass& getClass();
 
+    /// Return the TTL value of TSIG
+    ///
+    /// TSIG always uses 0 TTL.  This static method returns it,
+    /// when, though unlikely, an application wants to know the TTL TSIG
+    /// is supposed to use.
+    ///
+    /// \exception None
+    static const RRTTL& getTTL();
+
     /// Return the length of the TSIG record
     ///
     /// When constructed from the key name and RDATA, it is the length of