[trac893] implemented major logic of TSIG verify

src/lib/dns/tests/testdata/Makefile.am

@@ -40,6 +40,9 @@ BUILT_SOURCES += rdata_tsig_toWire1.wire rdata_tsig_toWire2.wire
 BUILT_SOURCES += rdata_tsig_toWire3.wire rdata_tsig_toWire4.wire
 BUILT_SOURCES += rdata_tsig_toWire5.wire
 BUILT_SOURCES += tsigrecord_toWire1.wire tsigrecord_toWire2.wire
+BUILT_SOURCES += tsig_verify1.wire tsig_verify2.wire tsig_verify3.wire
+BUILT_SOURCES += tsig_verify4.wire tsig_verify5.wire tsig_verify6.wire
+BUILT_SOURCES += tsig_verify7.wire tsig_verify8.wire tsig_verify9.wire
 
 # NOTE: keep this in sync with real file listing
 # so is included in tarball
@@ -108,6 +111,9 @@ EXTRA_DIST += rdata_tsig_toWire1.spec rdata_tsig_toWire2.spec
 EXTRA_DIST += rdata_tsig_toWire3.spec rdata_tsig_toWire4.spec
 EXTRA_DIST += rdata_tsig_toWire5.spec
 EXTRA_DIST += tsigrecord_toWire1.spec tsigrecord_toWire2.spec
+EXTRA_DIST += tsig_verify1.spec tsig_verify2.spec tsig_verify3.spec
+EXTRA_DIST += tsig_verify4.spec tsig_verify5.spec tsig_verify6.spec
+EXTRA_DIST += tsig_verify7.spec tsig_verify8.spec tsig_verify9.spec
 
 .spec.wire:
 	./gen-wiredata.py -o $@ $<

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

@@ -283,9 +283,8 @@ class NS(RR):
         f.write('# NS name=%s\n' % (self.nsname))
         f.write('%s\n' % nsname_wire)
 
-class SOA:
-    # this currently doesn't support name compression within the RDATA.
-    rdlen = -1                  # auto-calculate
+class SOA(RR):
+    rdlen = None                  # auto-calculate
     mname = 'ns.example.com'
     rname = 'root.example.com'
     serial = 2010012601
@@ -296,11 +295,9 @@ class SOA:
     def dump(self, f):
         mname_wire = encode_name(self.mname)
         rname_wire = encode_name(self.rname)
-        rdlen = self.rdlen
-        if rdlen < 0:
-            rdlen = int(20 + len(mname_wire) / 2 + len(str(rname_wire)) / 2)
-        f.write('\n# SOA RDATA (RDLEN=%d)\n' % rdlen)
-        f.write('%04x\n' % rdlen);
+        if self.rdlen is None:
+            self.rdlen = int(20 + len(mname_wire) / 2 + len(str(rname_wire)) / 2)
+        self.dump_header(f, self.rdlen)
         f.write('# NNAME=%s RNAME=%s\n' % (self.mname, self.rname))
         f.write('%s %s\n' % (mname_wire, rname_wire))
         f.write('# SERIAL(%d) REFRESH(%d) RETRY(%d) EXPIRE(%d) MINIMUM(%d)\n' %

src/lib/dns/tests/testdata/tsig_verify1.spec

@@ -0,0 +1,19 @@
+#
+# An example of signed AXFR request
+#
+
+[custom]
+sections: header:question:tsig
+[header]
+id: 0x3410
+arcount: 1
+[question]
+rrtype: AXFR
+[tsig]
+as_rr: True
+rr_name: www.example.com
+algorithm: hmac-md5
+time_signed: 0x4da8e951
+mac_size: 16
+mac: 0x35b2fd08268781634400c7c8a5533b13 
+original_id: 0x3410

src/lib/dns/tests/testdata/tsig_verify2.spec

@@ -0,0 +1,32 @@
+#
+# An example of signed AXFR response
+#
+
+[custom]
+sections: header:question:soa:tsig
+[header]
+id: 0x3410
+aa: 1
+qr: 1
+ancount: 1
+arcount: 1
+[question]
+rrtype: AXFR
+[soa]
+# note that names are compressed in this RR
+as_rr: True
+rr_name: ptr=12
+mname: ns.ptr=12
+rname: root.ptr=12
+serial: 2011041503
+refresh: 7200
+retry: 3600
+expire: 2592000
+[tsig]
+as_rr: True
+rr_name: www.example.com
+algorithm: hmac-md5
+time_signed: 0x4da8e951
+mac_size: 16
+mac: 0xbdd612cd2c7f9e0648bd6dc23713e83c
+original_id: 0x3410

src/lib/dns/tests/testdata/tsig_verify3.spec

@@ -0,0 +1,26 @@
+#
+# An example of signed AXFR response (continued)
+#
+
+[custom]
+sections: header:ns:tsig
+[header]
+id: 0x3410
+aa: 1
+qr: 1
+qdcount: 0
+ancount: 1
+arcount: 1
+[ns]
+# note that names are compressed in this RR
+as_rr: True
+rr_name: example.com.
+nsname: ns.ptr=12
+[tsig]
+as_rr: True
+rr_name: www.example.com
+algorithm: hmac-md5
+time_signed: 0x4da8e951
+mac_size: 16
+mac: 0x102458f7f62ddd7d638d746034130968
+original_id: 0x3410

src/lib/dns/tests/testdata/tsig_verify4.spec

@@ -0,0 +1,27 @@
+#
+# An example of signed DNS response with bogus MAC
+#
+
+[custom]
+sections: header:question:a:tsig
+[header]
+id: 0x2d65
+aa: 1
+qr: 1
+rd: 1
+ancount: 1
+arcount: 1
+[question]
+name: www.example.com
+[a]
+as_rr: True
+rr_name: ptr=12
+[tsig]
+as_rr: True
+rr_name: www.example.com
+algorithm: hmac-md5
+time_signed: 0x4da8877a
+mac_size: 16
+# bogus MAC
+mac: 0xdeadbeefdeadbeefdeadbeefdeadbeef
+original_id: 0x2d65

src/lib/dns/tests/testdata/tsig_verify5.spec

@@ -0,0 +1,26 @@
+#
+# An example of signed DNS response
+#
+
+[custom]
+sections: header:question:a:tsig
+[header]
+id: 0x2d65
+aa: 1
+qr: 1
+rd: 1
+ancount: 1
+arcount: 1
+[question]
+name: www.example.com
+[a]
+as_rr: True
+rr_name: ptr=12
+[tsig]
+as_rr: True
+rr_name: www.example.com
+algorithm: hmac-md5
+time_signed: 0x4da8877a
+mac_size: 16
+mac: 0x8fcda66a7cd1a3b9948eb1869d384a9f
+original_id: 0x2d65

src/lib/dns/tests/testdata/tsig_verify6.spec

@@ -0,0 +1,21 @@
+#
+# Forwarded DNS query message with TSIG signed (header ID != orig ID)
+#
+
+[custom]
+sections: header:question:tsig
+[header]
+id: 0x1035
+rd: 1
+arcount: 1
+[question]
+name: www.example.com
+[tsig]
+as_rr: True
+# TSIG QNAME won't be compressed
+rr_name: www.example.com
+algorithm: hmac-md5
+time_signed: 0x4da8877a
+mac_size: 16
+mac: 0x227026ad297beee721ce6c6fff1e9ef3
+original_id: 0x2d65

src/lib/dns/tests/testdata/tsig_verify7.spec

@@ -0,0 +1,21 @@
+#
+# DNS query message with TSIG that has empty MAC (invalidly)
+#
+
+[custom]
+sections: header:question:tsig
+[header]
+id: 0x2d65
+rd: 1
+arcount: 1
+[question]
+name: www.example.com
+[tsig]
+as_rr: True
+# TSIG QNAME won't be compressed
+rr_name: www.example.com
+algorithm: hmac-md5
+time_signed: 0x4da8877a
+mac_size: 0
+mac: ''
+original_id: 0x2d65

src/lib/dns/tests/testdata/tsig_verify8.spec

@@ -0,0 +1,23 @@
+#
+# DNS query message with TSIG that has empty MAC + BADKEY error
+#
+
+[custom]
+sections: header:question:tsig
+[header]
+id: 0x2d65
+rd: 1
+arcount: 1
+[question]
+name: www.example.com
+[tsig]
+as_rr: True
+# TSIG QNAME won't be compressed
+rr_name: www.example.com
+algorithm: hmac-md5
+time_signed: 0x4da8877a
+mac_size: 0
+mac: ''
+# 17: BADKEY
+error: 17
+original_id: 0x2d65

src/lib/dns/tests/testdata/tsig_verify9.spec

@@ -0,0 +1,21 @@
+#
+# A simple DNS query message with TSIG signed, but TSIG key and algorithm
+# names have upper case characters (unusual)
+#
+
+[custom]
+sections: header:question:tsig
+[header]
+id: 0x2d65
+rd: 1
+arcount: 1
+[question]
+name: www.example.com
+[tsig]
+as_rr: True
+rr_name: WWW.EXAMPLE.COM
+algorithm: HMAC-MD5.SIG-ALG.REG.INT
+time_signed: 0x4da8877a
+mac_size: 16
+mac: 0x227026ad297beee721ce6c6fff1e9ef3
+original_id: 0x2d65

src/lib/dns/tests/tsig_unittest.cc

@@ -70,8 +70,13 @@ class TSIGTest : public ::testing::Test {
 protected:
     TSIGTest() :
         tsig_ctx(NULL), qid(0x2d65), test_name("www.example.com"),
-        test_class(RRClass::IN()), test_ttl(86400), message(Message::RENDER),
-        buffer(0), renderer(buffer)
+        badkey_name("badkey.example.com"), test_class(RRClass::IN()),
+        test_ttl(86400), message(Message::RENDER), buffer(0), renderer(buffer),
+        dummy_data(1024, 0xdd),  // should be sufficiently large for all tests
+        dummy_record(badkey_name, any::TSIG(TSIGKey::HMACMD5_NAME(),
+                                            0x4da8877a,
+                                            TSIGContext::DEFAULT_FUDGE,
+                                            0, NULL, qid, 0, 0, NULL))
     {
         // Make sure we use the system time by default so that we won't be
         // confused due to other tests that tweak the time.
@@ -103,6 +108,8 @@ protected:
                                             bool add_question = true,
                                             Rcode rcode = Rcode::NOERROR());
 
+    void createMessageFromFile(const char* datafile);
+
     // bit-wise constant flags to configure DNS header flags for test
     // messages.
     static const unsigned int QR_FLAG = 0x1;
@@ -114,12 +121,16 @@ protected:
     TSIGKeyRing keyring;
     const uint16_t qid;
     const Name test_name;
+    const Name badkey_name;
     const RRClass test_class;
     const RRTTL test_ttl;
     Message message;
     OutputBuffer buffer;
     MessageRenderer renderer;
     vector<uint8_t> secret;
+    vector<uint8_t> dummy_data;
+    const TSIGRecord dummy_record;
+    vector<uint8_t> received_data;
 };
 
 ConstTSIGRecordPtr
@@ -158,15 +169,27 @@ TSIGTest::createMessageAndSign(uint16_t id, const Name& qname,
     renderer.clear();
     message.toWire(renderer);
 
+    TSIGContext::State expected_new_state =
+        (ctx->getState() == TSIGContext::INIT) ?
+        TSIGContext::WAIT_RESPONSE : TSIGContext::SENT_RESPONSE;
     ConstTSIGRecordPtr tsig = ctx->sign(id, renderer.getData(),
                                         renderer.getLength());
-    EXPECT_EQ(TSIGContext::SIGNED, ctx->getState());
+    EXPECT_EQ(expected_new_state, ctx->getState());
 
     return (tsig);
 }
 
 void
-commonTSIGChecks(ConstTSIGRecordPtr tsig, uint16_t expected_qid,
+TSIGTest::createMessageFromFile(const char* datafile) {
+    message.clear(Message::PARSE);
+    received_data.clear();
+    UnitTestUtil::readWireData(datafile, received_data);
+    InputBuffer buffer(&received_data[0], received_data.size());
+    message.fromWire(buffer);
+}
+
+void
+commonSignChecks(ConstTSIGRecordPtr tsig, uint16_t expected_qid,
                  uint64_t expected_timesigned,
                  const uint8_t* expected_mac, size_t expected_maclen,
                  uint16_t expected_error = 0,
@@ -192,6 +215,17 @@ commonTSIGChecks(ConstTSIGRecordPtr tsig, uint16_t expected_qid,
                         expected_otherdata, expected_otherlen);
 }
 
+void
+commonVerifyChecks(TSIGContext& ctx, const TSIGRecord* record,
+                   const void* data, size_t data_len, TSIGError expected_error,
+                   TSIGContext::State expected_new_state =
+                   TSIGContext::VERIFIED_RESPONSE)
+{
+    EXPECT_EQ(expected_error, ctx.verify(record, data, data_len));
+    EXPECT_EQ(expected_error, ctx.getError());
+    EXPECT_EQ(expected_new_state, ctx.getState());
+}
+
 TEST_F(TSIGTest, initialState) {
     // Until signing or verifying, the state should be INIT
     EXPECT_EQ(TSIGContext::INIT, tsig_ctx->getState());
@@ -225,6 +259,11 @@ TEST_F(TSIGTest, constructFromKeyRing) {
                      keyring);
     EXPECT_EQ(TSIGContext::INIT, ctx4.getState());
     EXPECT_EQ(TSIGError::BAD_KEY(), ctx4.getError());
+
+    // "Unknown" algorithm name will result in BADKEY, too.
+    TSIGContext ctx5(test_name, Name("unknown.algorithm"), keyring);
+    EXPECT_EQ(TSIGContext::INIT, ctx5.getState());
+    EXPECT_EQ(TSIGError::BAD_KEY(), ctx5.getError());
 }
 
 // Example output generated by
@@ -241,7 +280,7 @@ TEST_F(TSIGTest, sign) {
 
     {
         SCOPED_TRACE("Sign test for query");
-        commonTSIGChecks(createMessageAndSign(qid, test_name, tsig_ctx.get()),
+        commonSignChecks(createMessageAndSign(qid, test_name, tsig_ctx.get()),
                          qid, 0x4da8877a, common_expected_mac,
                          sizeof(common_expected_mac));
     }
@@ -259,7 +298,7 @@ TEST_F(TSIGTest, signUsingUpperCasedKeyName) {
 
     {
         SCOPED_TRACE("Sign test for query using non canonical key name");
-        commonTSIGChecks(createMessageAndSign(qid, test_name, &cap_ctx), qid,
+        commonSignChecks(createMessageAndSign(qid, test_name, &cap_ctx), qid,
                          0x4da8877a, common_expected_mac,
                          sizeof(common_expected_mac));
     }
@@ -275,7 +314,7 @@ TEST_F(TSIGTest, signUsingUpperCasedAlgorithmName) {
 
     {
         SCOPED_TRACE("Sign test for query using non canonical algorithm name");
-        commonTSIGChecks(createMessageAndSign(qid, test_name, &cap_ctx), qid,
+        commonSignChecks(createMessageAndSign(qid, test_name, &cap_ctx), qid,
                          0x4da8877a, common_expected_mac,
                          sizeof(common_expected_mac));
     }
@@ -309,6 +348,19 @@ TEST_F(TSIGTest, signBadData) {
     EXPECT_THROW(tsig_ctx->sign(0, &dummy_data, 0), InvalidParameter);
 }
 
+TEST_F(TSIGTest, verifyBadData) {
+    // the data must at least hold the DNS message header and the specified
+    // TSIG.
+    EXPECT_THROW(tsig_ctx->verify(&dummy_record, &dummy_data[0],
+                                  12 + dummy_record.getLength() - 1),
+                 InvalidParameter);
+
+    // And the data must not be NULL.
+    EXPECT_THROW(tsig_ctx->verify(&dummy_record, NULL,
+                                  12 + dummy_record.getLength()),
+                 InvalidParameter);
+}
+
 #ifdef ENABLE_CUSTOM_OPERATOR_NEW
 // We enable this test only when we enable custom new/delete at build time
 // We could enable/disable the test runtime using the gtest filter, but
@@ -321,11 +373,10 @@ TEST_F(TSIGTest, signExceptionSafety) {
     // complicated and involves more memory allocation, so the test result
     // won't be reliable.
 
-    tsig_verify_ctx->verifyTentative(createMessageAndSign(qid, test_name,
-                                                          tsig_ctx.get()),
-                                     TSIGError::BAD_KEY());
-    // At this point the state should be changed to "CHECKED"
-    ASSERT_EQ(TSIGContext::CHECKED, tsig_verify_ctx->getState());
+    commonVerifyChecks(*tsig_verify_ctx, &dummy_record, &dummy_data[0],
+                       dummy_data.size(), TSIGError::BAD_KEY(),
+                       TSIGContext::RECEIVED_REQUEST);
+
     try {
         int dummydata;
         isc::util::unittests::force_throw_on_new = true;
@@ -336,8 +387,8 @@ TEST_F(TSIGTest, signExceptionSafety) {
     } catch (const std::bad_alloc&) {
         isc::util::unittests::force_throw_on_new = false;
 
-        // sign() threw, so the state should still be "CHECKED".
-        EXPECT_EQ(TSIGContext::CHECKED, tsig_verify_ctx->getState());
+        // sign() threw, so the state should still be RECEIVED_REQUEST
+        EXPECT_EQ(TSIGContext::RECEIVED_REQUEST, tsig_verify_ctx->getState());
     }
     isc::util::unittests::force_throw_on_new = false;
 }
@@ -367,70 +418,130 @@ TEST_F(TSIGTest, signUsingHMACSHA1) {
     };
     {
         SCOPED_TRACE("Sign test using HMAC-SHA1");
-        commonTSIGChecks(createMessageAndSign(sha1_qid, test_name, &sha1_ctx),
+        commonSignChecks(createMessageAndSign(sha1_qid, test_name, &sha1_ctx),
                          sha1_qid, 0x4dae7d5f, expected_mac,
                          sizeof(expected_mac), 0, 0, NULL,
                          TSIGKey::HMACSHA1_NAME());
     }
 }
 
-// An example response to the signed query used for the "sign" test.
+// The first part of this test checks verifying the signed query used for
+// the "sign" test.
+// The second part of this test generates a signed response to the signed
+// query as follows:
 // Answer: www.example.com. 86400 IN A 192.0.2.1
 // MAC: 8fcda66a7cd1a3b9948eb1869d384a9f
-TEST_F(TSIGTest, signResponse) {
+TEST_F(TSIGTest, verifyThenSignResponse) {
     isc::util::detail::gettimeFunction = testGetTime<0x4da8877a>;
 
-    ConstTSIGRecordPtr tsig = createMessageAndSign(qid, test_name,
-                                                   tsig_ctx.get());
-    tsig_verify_ctx->verifyTentative(tsig);
-    EXPECT_EQ(TSIGContext::CHECKED, tsig_verify_ctx->getState());
+    // This test data for the message test has the same wire format data
+    // as the message used in the "sign" test.
+    createMessageFromFile("message_toWire2.wire");
+    {
+        SCOPED_TRACE("Verify test for request");
+        commonVerifyChecks(*tsig_verify_ctx, message.getTSIGRecord(),
+                           &received_data[0], received_data.size(),
+                           TSIGError::NOERROR(), TSIGContext::RECEIVED_REQUEST);
+    }
 
     // Transform the original message to a response, then sign the response
     // with the context of "verified state".
-    tsig = createMessageAndSign(qid, test_name, tsig_verify_ctx.get(),
-                                QR_FLAG|AA_FLAG|RD_FLAG,
-                                RRType::A(), "192.0.2.1");
+    ConstTSIGRecordPtr tsig = createMessageAndSign(qid, test_name,
+                                                   tsig_verify_ctx.get(),
+                                                   QR_FLAG|AA_FLAG|RD_FLAG,
+                                                   RRType::A(), "192.0.2.1");
     const uint8_t expected_mac[] = {
         0x8f, 0xcd, 0xa6, 0x6a, 0x7c, 0xd1, 0xa3, 0xb9,
         0x94, 0x8e, 0xb1, 0x86, 0x9d, 0x38, 0x4a, 0x9f
     };
     {
         SCOPED_TRACE("Sign test for response");
-        commonTSIGChecks(tsig, qid, 0x4da8877a,
-                         expected_mac, sizeof(expected_mac));
+        commonSignChecks(tsig, qid, 0x4da8877a, expected_mac,
+                         sizeof(expected_mac));
+    }
+}
+
+TEST_F(TSIGTest, verifyUpperCaseNames) {
+    isc::util::detail::gettimeFunction = testGetTime<0x4da8877a>;
+
+    // This test data for the message test has the same wire format data
+    // as the message used in the "sign" test.
+    createMessageFromFile("tsig_verify9.wire");
+    {
+        SCOPED_TRACE("Verify test for request");
+        commonVerifyChecks(*tsig_verify_ctx, message.getTSIGRecord(),
+                           &received_data[0], received_data.size(),
+                           TSIGError::NOERROR(), TSIGContext::RECEIVED_REQUEST);
+    }
+}
+
+TEST_F(TSIGTest, verifyForwardedMessage) {
+    // Similar to the first part of the previous test, but this test emulates
+    // the "forward" case, where the ID of the Header and the original ID in
+    // TSIG is different.
+    isc::util::detail::gettimeFunction = testGetTime<0x4da8877a>;
+
+    createMessageFromFile("tsig_verify6.wire");
+    {
+        SCOPED_TRACE("Verify test for forwarded request");
+        commonVerifyChecks(*tsig_verify_ctx, message.getTSIGRecord(),
+                           &received_data[0], received_data.size(),
+                           TSIGError::NOERROR(), TSIGContext::RECEIVED_REQUEST);
     }
 }
 
 // Example of signing multiple messages in a single TCP stream,
 // taken from data using BIND 9's "one-answer" transfer-format.
+// Request:
+//   QID: 0x3410, flags (none)
+//   Question: example.com/IN/AXFR
+//   Time Signed: 0x4da8e951
+//   MAC: 35b2fd08268781634400c7c8a5533b13
 // First message:
 //   QID: 0x3410, flags QR, AA
 //   Question: example.com/IN/AXFR
 //   Answer: example.com. 86400 IN SOA ns.example.com. root.example.com. (
 //                          2011041503 7200 3600 2592000 1200)
-//   Time Signed: 0x4da8e951
+//   MAC: bdd612cd2c7f9e0648bd6dc23713e83c
 // Second message:
-//    Answer: example.com. 86400 IN NS ns.example.com.
-//    MAC: 102458f7f62ddd7d638d746034130968
+//   Answer: example.com. 86400 IN NS ns.example.com.
+//   MAC: 102458f7f62ddd7d638d746034130968
 TEST_F(TSIGTest, signContinuation) {
     isc::util::detail::gettimeFunction = testGetTime<0x4da8e951>;
 
     const uint16_t axfr_qid = 0x3410;
     const Name zone_name("example.com");
 
-    // Create and sign the AXFR request, then verify it.
-    tsig_verify_ctx->verifyTentative(createMessageAndSign(axfr_qid, zone_name,
-                                                          tsig_ctx.get(), 0,
-                                                          RRType::AXFR()));
-    EXPECT_EQ(TSIGContext::CHECKED, tsig_verify_ctx->getState());
+    // Create and sign the AXFR request
+    ConstTSIGRecordPtr tsig = createMessageAndSign(axfr_qid, zone_name,
+                                                   tsig_ctx.get(), 0,
+                                                   RRType::AXFR());
+    // Then verify it (the wire format test data should contain the same
+    // message data, and verification should succeed).
+    received_data.clear();
+    UnitTestUtil::readWireData("tsig_verify1.wire", received_data);
+    {
+        SCOPED_TRACE("Verify AXFR query");
+        commonVerifyChecks(*tsig_verify_ctx, tsig.get(), &received_data[0],
+                           received_data.size(), TSIGError::NOERROR(),
+                           TSIGContext::RECEIVED_REQUEST);
+    }
 
-    // Create and sign the first response message (we don't need the result
-    // for the purpose of this test)
-    createMessageAndSign(axfr_qid, zone_name, tsig_verify_ctx.get(),
-                         AA_FLAG|QR_FLAG, RRType::AXFR(),
-                         "ns.example.com. root.example.com. "
-                         "2011041503 7200 3600 2592000 1200",
-                         &RRType::SOA());
+    // Create and sign the first response message
+    tsig = createMessageAndSign(axfr_qid, zone_name, tsig_verify_ctx.get(),
+                                AA_FLAG|QR_FLAG, RRType::AXFR(),
+                                "ns.example.com. root.example.com. "
+                                "2011041503 7200 3600 2592000 1200",
+                                &RRType::SOA());
+
+    // Then verify it at the requester side.
+    received_data.clear();
+    UnitTestUtil::readWireData("tsig_verify2.wire", received_data);
+    {
+        SCOPED_TRACE("Verify first AXFR response");
+        commonVerifyChecks(*tsig_ctx, tsig.get(), &received_data[0],
+                           received_data.size(), TSIGError::NOERROR());
+    }
 
     // Create and sign the second response message
     const uint8_t expected_mac[] = {
@@ -439,13 +550,20 @@ TEST_F(TSIGTest, signContinuation) {
     };
     {
         SCOPED_TRACE("Sign test for continued response in TCP stream");
-        commonTSIGChecks(createMessageAndSign(axfr_qid, zone_name,
-                                              tsig_verify_ctx.get(),
-                                              AA_FLAG|QR_FLAG, RRType::AXFR(),
-                                              "ns.example.com.", &RRType::NS(),
-                                              false),
-                         axfr_qid, 0x4da8e951,
-                         expected_mac, sizeof(expected_mac));
+        tsig = createMessageAndSign(axfr_qid, zone_name, tsig_verify_ctx.get(),
+                                    AA_FLAG|QR_FLAG, RRType::AXFR(),
+                                    "ns.example.com.", &RRType::NS(), false);
+        commonSignChecks(tsig, axfr_qid, 0x4da8e951, expected_mac,
+                         sizeof(expected_mac));
+    }
+
+    // Then verify it at the requester side.
+    received_data.clear();
+    UnitTestUtil::readWireData("tsig_verify3.wire", received_data);
+    {
+        SCOPED_TRACE("Verify second AXFR response");
+        commonVerifyChecks(*tsig_ctx, tsig.get(), &received_data[0],
+                           received_data.size(), TSIGError::NOERROR());
     }
 }
 
@@ -471,10 +589,13 @@ TEST_F(TSIGTest, badtimeResponse) {
                                                    RRType::SOA());
 
     // "advance the clock" and try validating, which should fail due to BADTIME
-    // (verifyTentative actually doesn't check the time, though)
     isc::util::detail::gettimeFunction = testGetTime<0x4da8be86>;
-    tsig_verify_ctx->verifyTentative(tsig, TSIGError::BAD_TIME());
-    EXPECT_EQ(TSIGError::BAD_TIME(), tsig_verify_ctx->getError());
+    {
+        SCOPED_TRACE("Verify resulting in BADTIME due to expired SIG");
+        commonVerifyChecks(*tsig_verify_ctx, tsig.get(), &dummy_data[0],
+                           dummy_data.size(), TSIGError::BAD_TIME(),
+                           TSIGContext::RECEIVED_REQUEST);
+    }
 
     // make and sign a response in the context of TSIG error.
     tsig = createMessageAndSign(test_qid, test_name, tsig_verify_ctx.get(),
@@ -487,7 +608,7 @@ TEST_F(TSIGTest, badtimeResponse) {
     };
     {
         SCOPED_TRACE("Sign test for response with BADTIME");
-        commonTSIGChecks(tsig, message.getQid(), 0x4da8b9d6,
+        commonSignChecks(tsig, message.getQid(), 0x4da8b9d6,
                          expected_mac, sizeof(expected_mac),
                          18,     // error: BADTIME
                          sizeof(expected_otherdata),
@@ -495,21 +616,86 @@ TEST_F(TSIGTest, badtimeResponse) {
     }
 }
 
+TEST_F(TSIGTest, badtimeResponse2) {
+    isc::util::detail::gettimeFunction = testGetTime<0x4da8b9d6>;
+
+    ConstTSIGRecordPtr tsig = createMessageAndSign(qid, test_name,
+                                                   tsig_ctx.get(), 0,
+                                                   RRType::SOA());
+
+    // "rewind the clock" and try validating, which should fail due to BADTIME
+    isc::util::detail::gettimeFunction = testGetTime<0x4da8b9d6 - 600>;
+    {
+        SCOPED_TRACE("Verify resulting in BADTIME due to too future SIG");
+        commonVerifyChecks(*tsig_verify_ctx, tsig.get(), &dummy_data[0],
+                           dummy_data.size(), TSIGError::BAD_TIME(),
+                           TSIGContext::RECEIVED_REQUEST);
+    }
+}
+
+TEST_F(TSIGTest, badtimeBoundaries) {
+    isc::util::detail::gettimeFunction = testGetTime<0x4da8b9d6>;
+
+    // Test various boundary conditions.  We intentionally use the magic
+    // number of 300 instead of the constant variable for testing.
+    // In the okay cases, signature is not correct, but it's sufficient to
+    // check the error code isn't BADTIME for the purpose of this test.
+    ConstTSIGRecordPtr tsig = createMessageAndSign(qid, test_name,
+                                                   tsig_ctx.get(), 0,
+                                                   RRType::SOA());
+    isc::util::detail::gettimeFunction = testGetTime<0x4da8b9d6 + 301>;
+    EXPECT_EQ(TSIGError::BAD_TIME(),
+              tsig_verify_ctx->verify(tsig.get(), &dummy_data[0],
+                                      dummy_data.size()));
+    isc::util::detail::gettimeFunction = testGetTime<0x4da8b9d6 + 300>;
+    EXPECT_NE(TSIGError::BAD_TIME(),
+              tsig_verify_ctx->verify(tsig.get(), &dummy_data[0],
+                                      dummy_data.size()));
+    isc::util::detail::gettimeFunction = testGetTime<0x4da8b9d6 - 301>;
+    EXPECT_EQ(TSIGError::BAD_TIME(),
+              tsig_verify_ctx->verify(tsig.get(), &dummy_data[0],
+                                      dummy_data.size()));
+    isc::util::detail::gettimeFunction = testGetTime<0x4da8b9d6 - 300>;
+    EXPECT_NE(TSIGError::BAD_TIME(),
+              tsig_verify_ctx->verify(tsig.get(), &dummy_data[0],
+                                      dummy_data.size()));
+}
+
+TEST_F(TSIGTest, badtimeOverflow) {
+    isc::util::detail::gettimeFunction = testGetTime<200>;
+    ConstTSIGRecordPtr tsig = createMessageAndSign(qid, test_name,
+                                                   tsig_ctx.get(), 0,
+                                                   RRType::SOA());
+
+    // This should be in the okay range, but since "200 - fudge" overflows
+    // and we compare them as 64-bit unsigned integers, it results in a false
+    // positive (we intentionally accept that).
+    isc::util::detail::gettimeFunction = testGetTime<100>;
+    EXPECT_EQ(TSIGError::BAD_TIME(),
+              tsig_verify_ctx->verify(tsig.get(), &dummy_data[0],
+                                      dummy_data.size()));
+}
+
 TEST_F(TSIGTest, badsigResponse) {
     isc::util::detail::gettimeFunction = testGetTime<0x4da8877a>;
 
-    // Sign a simple message, and force the verification to fail with
-    // BADSIG.
-    tsig_verify_ctx->verifyTentative(createMessageAndSign(qid, test_name,
-                                                          tsig_ctx.get()),
-                                     TSIGError::BAD_SIG());
+    // Try to sign a simple message with bogus secret.  It should fail
+    // with BADSIG.
+    createMessageFromFile("message_toWire2.wire");
+    TSIGContext bad_ctx(TSIGKey(test_name, TSIGKey::HMACMD5_NAME(),
+                                &dummy_data[0], dummy_data.size()));
+    {
+        SCOPED_TRACE("Verify resulting in BADSIG");
+        commonVerifyChecks(bad_ctx, message.getTSIGRecord(),
+                           &received_data[0], received_data.size(),
+                           TSIGError::BAD_SIG(), TSIGContext::RECEIVED_REQUEST);
+    }
 
     // Sign the same message (which doesn't matter for this test) with the
     // context of "checked state".
     {
         SCOPED_TRACE("Sign test for response with BADSIG error");
-        commonTSIGChecks(createMessageAndSign(qid, test_name,
-                                              tsig_verify_ctx.get()),
+        commonSignChecks(createMessageAndSign(qid, test_name, &bad_ctx),
                          message.getQid(), 0x4da8877a, NULL, 0,
                          16);   // 16: BADSIG
     }
@@ -518,16 +704,189 @@ TEST_F(TSIGTest, badsigResponse) {
 TEST_F(TSIGTest, badkeyResponse) {
     // A similar test as badsigResponse but for BADKEY
     isc::util::detail::gettimeFunction = testGetTime<0x4da8877a>;
-    tsig_verify_ctx->verifyTentative(createMessageAndSign(qid, test_name,
-                                                          tsig_ctx.get()),
-                                     TSIGError::BAD_KEY());
+    tsig_ctx.reset(new TSIGContext(badkey_name, TSIGKey::HMACMD5_NAME(),
+                                   keyring));
+    {
+        SCOPED_TRACE("Verify resulting in BADKEY");
+        commonVerifyChecks(*tsig_ctx, &dummy_record, &dummy_data[0],
+                           dummy_data.size(), TSIGError::BAD_KEY(),
+                           TSIGContext::RECEIVED_REQUEST);
+    }
+
     {
         SCOPED_TRACE("Sign test for response with BADKEY error");
-        commonTSIGChecks(createMessageAndSign(qid, test_name,
-                                              tsig_verify_ctx.get()),
-                         message.getQid(), 0x4da8877a, NULL, 0,
-                         17);   // 17: BADKEY
+        ConstTSIGRecordPtr sig = createMessageAndSign(qid, test_name,
+                                                      tsig_ctx.get());
+        EXPECT_EQ(badkey_name, sig->getName());
+        commonSignChecks(sig, qid, 0x4da8877a, NULL, 0, 17);   // 17: BADKEY
     }
 }
 
+TEST_F(TSIGTest, badkeyForResponse) {
+    // "BADKEY" case for a response to a signed message
+    createMessageAndSign(qid, test_name, tsig_ctx.get());
+    {
+        SCOPED_TRACE("Verify a response resulting in BADKEY");
+        commonVerifyChecks(*tsig_ctx, &dummy_record, &dummy_data[0],
+                           dummy_data.size(), TSIGError::BAD_KEY(),
+                           TSIGContext::WAIT_RESPONSE);
+    }
+
+    // A similar case with a different algorithm
+    const TSIGRecord dummy_record2(test_name,
+                                  any::TSIG(TSIGKey::HMACSHA1_NAME(),
+                                            0x4da8877a,
+                                            TSIGContext::DEFAULT_FUDGE,
+                                            0, NULL, qid, 0, 0, NULL));
+    {
+        SCOPED_TRACE("Verify a response resulting in BADKEY due to bad alg");
+        commonVerifyChecks(*tsig_ctx, &dummy_record2, &dummy_data[0],
+                           dummy_data.size(), TSIGError::BAD_KEY(),
+                           TSIGContext::WAIT_RESPONSE);
+    }
+}
+
+TEST_F(TSIGTest, badsigThenValidate) {
+    // According to RFC2845 4.6, if TSIG verification fails the client
+    // should discard that message and wait for another signed response.
+    // This test emulates that situation.
+
+    isc::util::detail::gettimeFunction = testGetTime<0x4da8877a>;
+
+    createMessageAndSign(qid, test_name, tsig_ctx.get());
+
+    createMessageFromFile("tsig_verify4.wire");
+    {
+        SCOPED_TRACE("Verify a response that should fail due to BADSIG");
+        commonVerifyChecks(*tsig_ctx, message.getTSIGRecord(),
+                           &received_data[0], received_data.size(),
+                           TSIGError::BAD_SIG(), TSIGContext::WAIT_RESPONSE);
+    }
+
+    createMessageFromFile("tsig_verify5.wire");
+    {
+        SCOPED_TRACE("Verify a response after a BADSIG failure");
+        commonVerifyChecks(*tsig_ctx, message.getTSIGRecord(),
+                           &received_data[0], received_data.size(),
+                           TSIGError::NOERROR(),
+                           TSIGContext::VERIFIED_RESPONSE);
+    }
+}
+
+TEST_F(TSIGTest, nosigThenValidate) {
+    // Similar to the previous test, but the first response doesn't contain
+    // TSIG.
+    isc::util::detail::gettimeFunction = testGetTime<0x4da8877a>;
+
+    createMessageAndSign(qid, test_name, tsig_ctx.get());
+
+    {
+        SCOPED_TRACE("Verify a response without TSIG that should exist");
+        commonVerifyChecks(*tsig_ctx, NULL, &dummy_data[0],
+                           dummy_data.size(), TSIGError::FORMERR(),
+                           TSIGContext::WAIT_RESPONSE);
+    }
+
+    createMessageFromFile("tsig_verify5.wire");
+    {
+        SCOPED_TRACE("Verify a response after a FORMERR failure");
+        commonVerifyChecks(*tsig_ctx, message.getTSIGRecord(),
+                           &received_data[0], received_data.size(),
+                           TSIGError::NOERROR(),
+                           TSIGContext::VERIFIED_RESPONSE);
+    }
+}
+
+TEST_F(TSIGTest, badtimeThenValidate) {
+    // Similar to the previous test, but the first response results in BADTIME.
+    isc::util::detail::gettimeFunction = testGetTime<0x4da8877a>;
+    
+    ConstTSIGRecordPtr tsig = createMessageAndSign(qid, test_name,
+                                                   tsig_ctx.get());
+
+    // "advance the clock" and try validating, which should fail due to BADTIME
+    isc::util::detail::gettimeFunction = testGetTime<0x4da8877a + 600>;
+    {
+        SCOPED_TRACE("Verify resulting in BADTIME due to expired SIG");
+        commonVerifyChecks(*tsig_ctx, tsig.get(), &dummy_data[0],
+                           dummy_data.size(), TSIGError::BAD_TIME(),
+                           TSIGContext::WAIT_RESPONSE);
+    }
+
+    // revert the clock again.
+    isc::util::detail::gettimeFunction = testGetTime<0x4da8877a>;
+    createMessageFromFile("tsig_verify5.wire");
+    {
+        SCOPED_TRACE("Verify a response after a BADTIME failure");
+        commonVerifyChecks(*tsig_ctx, message.getTSIGRecord(),
+                           &received_data[0], received_data.size(),
+                           TSIGError::NOERROR(),
+                           TSIGContext::VERIFIED_RESPONSE);
+    }
+}
+
+TEST_F(TSIGTest, emptyMAC) {
+    isc::util::detail::gettimeFunction = testGetTime<0x4da8877a>;
+
+    // We don't allow empty MAC unless the TSIG error is BADSIG or BADKEY.
+    createMessageFromFile("tsig_verify7.wire");
+    {
+        SCOPED_TRACE("Verify test for request");
+        commonVerifyChecks(*tsig_verify_ctx, message.getTSIGRecord(),
+                           &received_data[0], received_data.size(),
+                           TSIGError::BAD_SIG(), TSIGContext::RECEIVED_REQUEST);
+    }
+
+    // If the empty MAC comes with a BADKEY error, the error is passed
+    // transparently.
+    createMessageFromFile("tsig_verify8.wire");
+    {
+        SCOPED_TRACE("Verify test for request");
+        commonVerifyChecks(*tsig_verify_ctx, message.getTSIGRecord(),
+                           &received_data[0], received_data.size(),
+                           TSIGError::BAD_KEY(), TSIGContext::RECEIVED_REQUEST);
+    }
+}
+
+TEST_F(TSIGTest, verifyAfterSendResponse) {
+    // Once the context is used for sending a signed response, it shouldn't
+    // be used for further verification.
+
+    // The following are essentially the same as what verifyThenSignResponse
+    // does with simplification.
+    isc::util::detail::gettimeFunction = testGetTime<0x4da8877a>;
+    createMessageFromFile("message_toWire2.wire");
+    tsig_verify_ctx->verify(message.getTSIGRecord(), &received_data[0],
+                            received_data.size());
+    EXPECT_EQ(TSIGContext::RECEIVED_REQUEST, tsig_verify_ctx->getState());
+    createMessageAndSign(qid, test_name, tsig_verify_ctx.get(),
+                         QR_FLAG|AA_FLAG|RD_FLAG, RRType::A(), "192.0.2.1");
+    EXPECT_EQ(TSIGContext::SENT_RESPONSE, tsig_verify_ctx->getState());
+
+    // Now trying further verification.
+    createMessageFromFile("message_toWire2.wire");
+    EXPECT_THROW(tsig_verify_ctx->verify(message.getTSIGRecord(),
+                                         &received_data[0],
+                                         received_data.size()),
+                 TSIGContextError);
+}
+
+TEST_F(TSIGTest, signAterVerified) {
+    // Likewise, once the context verifies a response, it shouldn't for
+    // signing any more.
+
+    // The following are borrowed from badsigThenValidate (without the
+    // intermediate failure)
+    isc::util::detail::gettimeFunction = testGetTime<0x4da8877a>;
+    createMessageAndSign(qid, test_name, tsig_ctx.get());
+    createMessageFromFile("tsig_verify5.wire");
+    tsig_ctx->verify(message.getTSIGRecord(), &received_data[0],
+                     received_data.size());
+    EXPECT_EQ(TSIGContext::VERIFIED_RESPONSE, tsig_ctx->getState());
+
+    // Now trying further signing.
+    EXPECT_THROW(createMessageAndSign(qid, test_name, tsig_ctx.get()),
+                 TSIGContextError);
+}
+
 } // end namespace

src/lib/dns/tsig.cc

@@ -45,14 +45,15 @@ namespace dns {
 namespace {
 typedef boost::shared_ptr<HMAC> HMACPtr;
 
-// This singleton key is used when the TSIG context is constructed with no
-// matching key.  The key name and algorithm won't be used in subsequent
-// sign/verify, so the their values don't matter.
-const TSIGKey&
-getDummyTSIGKey() {
-    static TSIGKey dummy_key(Name::ROOT_NAME(), TSIGKey::HMACMD5_NAME(), NULL,
-                             0);
-    return (dummy_key);
+// TSIG uses 48-bit unsigned integer to represent time signed.
+// Since gettimeWrapper() returns a 64-bit *signed* integer, we
+// make sure it's stored in an unsigned 64-bit integer variable and
+// represents a value in the expected range.  (In reality, however,
+// gettimeWrapper() will return a positive integer that will fit
+// in 48 bits)
+uint64_t
+getTSIGTime() {
+    return (detail::gettimeWrapper() & 0x0000ffffffffffffULL);
 }
 }
 
@@ -61,6 +62,32 @@ struct TSIGContext::TSIGContextImpl {
         state_(INIT), key_(key), error_(Rcode::NOERROR()),
         previous_timesigned_(0)
     {}
+    TSIGError postVerifyUpdate(TSIGError error, const void* digest,
+                               size_t digest_len)
+    {
+        if (state_ == INIT) {
+            state_ = RECEIVED_REQUEST;
+        } else if (state_ == WAIT_RESPONSE && error == TSIGError::NOERROR()) {
+            state_ = VERIFIED_RESPONSE;
+        }
+        if (digest != NULL) {
+            previous_digest_.assign(static_cast<const uint8_t*>(digest),
+                                    static_cast<const uint8_t*>(digest) +
+                                    digest_len);
+        }
+        error_ = error;
+        return (error);
+    }
+    void digestPreviousMAC(OutputBuffer& buffer, HMACPtr hmac) const;
+    void digestTSIGVariables(OutputBuffer& buffer, HMACPtr hmac,
+                             uint16_t rrclass, uint32_t rrttl,
+                             uint64_t time_signed, uint16_t fudge,
+                             uint16_t error, uint16_t otherlen,
+                             const void* otherdata,
+                             bool time_variables_only) const;
+    void digestDNSMessage(OutputBuffer& buffer, HMACPtr hmac,
+                          uint16_t qid, const void* data,
+                          size_t data_len) const;
     State state_;
     const TSIGKey key_;
     vector<uint8_t> previous_digest_;
@@ -68,6 +95,89 @@ struct TSIGContext::TSIGContextImpl {
     uint64_t previous_timesigned_; // only meaningful for response with BADTIME
 };
 
+void
+TSIGContext::TSIGContextImpl::digestPreviousMAC(OutputBuffer& buffer,
+                                                HMACPtr hmac) const
+{
+    buffer.clear();
+
+    const uint16_t previous_digest_len(previous_digest_.size());
+    buffer.writeUint16(previous_digest_len);
+    if (previous_digest_len != 0) {
+        buffer.writeData(&previous_digest_[0], previous_digest_len);
+    }
+    hmac->update(buffer.getData(), buffer.getLength());
+}
+
+void
+TSIGContext::TSIGContextImpl::digestTSIGVariables(
+    OutputBuffer& buffer, HMACPtr hmac, uint16_t rrclass, uint32_t rrttl,
+    uint64_t time_signed, uint16_t fudge, uint16_t error, uint16_t otherlen,
+    const void* otherdata, bool time_variables_only) const
+{
+    buffer.clear();
+
+    if (!time_variables_only) {
+        key_.getKeyName().toWire(buffer);
+        buffer.writeUint16(rrclass);
+        buffer.writeUint32(rrttl);
+        key_.getAlgorithmName().toWire(buffer);
+    }
+    buffer.writeUint16(time_signed >> 32);
+    buffer.writeUint32(time_signed & 0xffffffff);
+    buffer.writeUint16(fudge);
+    hmac->update(buffer.getData(), buffer.getLength());
+
+    if (!time_variables_only) {
+        buffer.clear();
+        buffer.writeUint16(error);
+        buffer.writeUint16(otherlen);
+        hmac->update(buffer.getData(), buffer.getLength());
+        if (otherlen > 0) {
+            hmac->update(otherdata, otherlen);
+        }
+    }
+}
+
+namespace {
+// We exploit some minimum knowledge of DNS message format:
+// the header section has a fixed length of 12 octets
+// the offset in the header section to the ID field is 0 (and the field length
+// is 2 octets)
+// the offset in the header section to the ARCOUNT field is 10 (and the field
+// length is 2 octets)
+const size_t MESSAGE_HEADER_LEN = 12;
+}
+
+void
+TSIGContext::TSIGContextImpl::digestDNSMessage(OutputBuffer& buffer,
+                                               HMACPtr hmac,
+                                               uint16_t qid, const void* data,
+                                               size_t data_len) const
+{
+    buffer.clear();
+    const uint8_t* msgptr = static_cast<const uint8_t*>(data);
+
+    // Install the original ID
+    buffer.writeUint16(qid);
+    msgptr += sizeof(uint16_t);
+
+    // Copy the rest of the header except the ARCOUNT field.
+    buffer.writeData(msgptr, 8);
+    msgptr += 8;
+
+    // Install the adjusted ARCOUNT (we don't care even if the value is bogus
+    // and it underflows; it would simply result in verification failure)
+    InputBuffer b(msgptr, sizeof(uint16_t));
+    const uint16_t arcount = b.readUint16();
+    buffer.writeUint16(arcount - 1);
+    msgptr += 2;
+
+    // Digest the header and the rest of the DNS message
+    hmac->update(buffer.getData(), buffer.getLength());
+    hmac->update(msgptr, data_len - MESSAGE_HEADER_LEN);
+}
+
 TSIGContext::TSIGContext(const TSIGKey& key) : impl_(new TSIGContextImpl(key))
 {
 }
@@ -78,7 +188,12 @@ TSIGContext::TSIGContext(const Name& key_name, const Name& algorithm_name,
     const TSIGKeyRing::FindResult result(keyring.find(key_name,
                                                       algorithm_name));
     if (result.code == TSIGKeyRing::NOTFOUND) {
-        impl_ = new TSIGContextImpl(getDummyTSIGKey());
+        // If not key is found, create a dummy key with the specified key
+        // parameters and empty secret.  In the common scenario this will
+        // be used in subsequent response with a TSIG indicating a BADKEY
+        // error.
+        impl_ = new TSIGContextImpl(TSIGKey(key_name, algorithm_name,
+                                            NULL, 0));
         impl_->error_ = TSIGError::BAD_KEY();
     } else {
         impl_ = new TSIGContextImpl(*result.key);
@@ -103,21 +218,20 @@ ConstTSIGRecordPtr
 TSIGContext::sign(const uint16_t qid, const void* const data,
                   const size_t data_len)
 {
+    if (impl_->state_ == VERIFIED_RESPONSE) {
+        isc_throw(TSIGContextError,
+                  "TSIG sign attempt after verifying a response");
+    }
+
     if (data == NULL || data_len == 0) {
         isc_throw(InvalidParameter, "TSIG sign error: empty data is given");
     }
 
     TSIGError error(TSIGError::NOERROR());
-    // TSIG uses 48-bit unsigned integer to represent time signed.
-    // Since gettimeofdayWrapper() returns a 64-bit *signed* integer, we
-    // make sure it's stored in an unsigned 64-bit integer variable and
-    // represents a value in the expected range.  (In reality, however,
-    // gettimeofdayWrapper() will return a positive integer that will fit
-    // in 48 bits)
-    const uint64_t now = (detail::gettimeWrapper() & 0x0000ffffffffffffULL);
+    const uint64_t now = getTSIGTime();
 
     // For responses adjust the error code.
-    if (impl_->state_ == CHECKED) {
+    if (impl_->state_ == RECEIVED_REQUEST) {
         error = impl_->error_;
     }
 
@@ -130,7 +244,7 @@ TSIGContext::sign(const uint16_t qid, const void* const data,
                                               now, DEFAULT_FUDGE, 0, NULL,
                                               qid, error.getCode(), 0, NULL)));
         impl_->previous_digest_.clear();
-        impl_->state_ = SIGNED;
+        impl_->state_ = SENT_RESPONSE;
         return (tsig);
     }
 
@@ -144,53 +258,35 @@ TSIGContext::sign(const uint16_t qid, const void* const data,
     // If the context has previous MAC (either the Request MAC or its own
     // previous MAC), digest it.
     if (impl_->state_ != INIT) {
-        const uint16_t previous_digest_len(impl_->previous_digest_.size());
-        variables.writeUint16(previous_digest_len);
-        if (previous_digest_len != 0) {
-            variables.writeData(&impl_->previous_digest_[0],
-                                previous_digest_len);
-        }
-        hmac->update(variables.getData(), variables.getLength());
+        impl_->digestPreviousMAC(variables, hmac);
     }
 
     // Digest the message (without TSIG)
     hmac->update(data, data_len);
 
-    //
-    // Digest TSIG variables.  If state_ is SIGNED we skip digesting them
-    // except for time related variables (RFC2845 4.4).
-    //
-    variables.clear();
-    if (impl_->state_ != SIGNED) {
-        impl_->key_.getKeyName().toWire(variables);
-        TSIGRecord::getClass().toWire(variables);
-        variables.writeUint32(TSIGRecord::TSIG_TTL);
-        impl_->key_.getAlgorithmName().toWire(variables);
-    }
+    // Digest TSIG variables.
+    // First, prepare some non constant variables.
     const uint64_t time_signed = (error == TSIGError::BAD_TIME()) ?
         impl_->previous_timesigned_ : now;
-    variables.writeUint16(time_signed >> 32);
-    variables.writeUint32(time_signed & 0xffffffff);
-    variables.writeUint16(DEFAULT_FUDGE);
-    hmac->update(variables.getData(), variables.getLength());
-    variables.clear();
-
-    if (impl_->state_ != SIGNED) {
-        variables.writeUint16(error.getCode());
-
-        // For BADTIME error, digest 6 bytes of other data.
-        // (6 bytes = size of time signed value)
-        variables.writeUint16((error == TSIGError::BAD_TIME()) ? 6 : 0);
-        hmac->update(variables.getData(), variables.getLength());
-
-        variables.clear();
-        if (error == TSIGError::BAD_TIME()) {
-            variables.writeUint16(now >> 32);
-            variables.writeUint32(now & 0xffffffff);
-            hmac->update(variables.getData(), variables.getLength());
-        }
+    // For BADTIME error, we include 6 bytes of other data.
+    // (6 bytes = size of time signed value)
+    const uint16_t otherlen = (error == TSIGError::BAD_TIME()) ? 6 : 0;
+    OutputBuffer otherdatabuf(otherlen);
+    if (error == TSIGError::BAD_TIME()) {
+            otherdatabuf.writeUint16(now >> 32);
+            otherdatabuf.writeUint32(now & 0xffffffff);
     }
-    const uint16_t otherlen = variables.getLength();
+    const void* const otherdata =
+        (otherlen == 0) ? NULL : otherdatabuf.getData();
+    // Then calculate the digest.  If state_ is SENT_RESPONSE we are sending
+    // a continued message in the same TCP stream so skip digesting
+    // variables except for time related variables (RFC2845 4.4).
+    impl_->digestTSIGVariables(variables, hmac,
+                               TSIGRecord::getClass().getCode(),
+                               TSIGRecord::TSIG_TTL, time_signed,
+                               DEFAULT_FUDGE, error.getCode(),
+                               otherlen, otherdata,
+                               impl_->state_ == SENT_RESPONSE);
 
     // Get the final digest, update internal state, then finish.
     vector<uint8_t> digest = hmac->sign();
@@ -200,31 +296,131 @@ TSIGContext::sign(const uint16_t qid, const void* const data,
                                           time_signed, DEFAULT_FUDGE,
                                           digest.size(), &digest[0],
                                           qid, error.getCode(), otherlen,
-                                          otherlen == 0 ?
-                                          NULL : variables.getData())));
+                                          otherdata)));
     // Exception free from now on.
     impl_->previous_digest_.swap(digest);
-    impl_->state_ = SIGNED;
+    impl_->state_ = (impl_->state_ == INIT) ? WAIT_RESPONSE : SENT_RESPONSE;
     return (tsig);
 }
 
-void
-TSIGContext::verifyTentative(ConstTSIGRecordPtr tsig, TSIGError error) {
-    const any::TSIG tsig_rdata = tsig->getRdata();
+TSIGError
+TSIGContext::verify(const TSIGRecord* const record, const void* const data,
+                    const size_t data_len)
+{
+    if (impl_->state_ == SENT_RESPONSE) {
+        isc_throw(TSIGContextError,
+                  "TSIG verify attempt after sending a response");
+    }
 
-    impl_->error_ = error;
-    if (error == TSIGError::BAD_TIME()) {
-        impl_->previous_timesigned_ = tsig_rdata.getTimeSigned();
+    // This case happens when we sent a signed request and have received an
+    // unsigned response.  According to RFC2845 Section 4.6 this case should be
+    // considered a "format error" (although the specific error code
+    // wouldn't matter much for the caller).
+    if (record == NULL) {
+        return (impl_->postVerifyUpdate(TSIGError::FORMERR(), NULL, 0));
     }
 
-    // For simplicity we assume non empty digests.
-    assert(tsig_rdata.getMACSize() != 0);
-    impl_->previous_digest_.assign(
-        static_cast<const uint8_t*>(tsig_rdata.getMAC()),
-        static_cast<const uint8_t*>(tsig_rdata.getMAC()) +
-        tsig_rdata.getMACSize());
+    const any::TSIG& tsig_rdata = record->getRdata();
 
-    impl_->state_ = CHECKED;
+    // Reject some obviously invalid data
+    if (data_len < MESSAGE_HEADER_LEN + record->getLength()) {
+        isc_throw(InvalidParameter,
+                  "TSIG verify: data length is invalid: " << data_len);
+    }
+    if (data == NULL) {
+        isc_throw(InvalidParameter, "TSIG verify: empty data is invalid");
+    }
+
+    // Check key: whether we first verify it with a known key or we verify
+    // it using the consistent key in the context.  If the check fails we are
+    // done with BADKEY.
+    if (impl_->state_ == INIT && impl_->error_ == TSIGError::BAD_KEY()) {
+        return (impl_->postVerifyUpdate(TSIGError::BAD_KEY(), NULL, 0));
+    }
+    if (impl_->key_.getKeyName() != record->getName() ||
+        impl_->key_.getAlgorithmName() != tsig_rdata.getAlgorithm()) {
+        return (impl_->postVerifyUpdate(TSIGError::BAD_KEY(), NULL, 0));
+    }
+
+    // Check time: the current time must be in the range of
+    // [time signed - fudge, time signed + fudge].  Otherwise verification
+    // fails with BADTIME. (RFC2845 Section 4.6.2)
+    const uint64_t now = getTSIGTime();
+    if (tsig_rdata.getTimeSigned() + DEFAULT_FUDGE < now ||
+        tsig_rdata.getTimeSigned() - DEFAULT_FUDGE > now) {
+        const void* digest = NULL;
+        size_t digest_len = 0;
+        if (impl_->state_ == INIT) {
+            digest = tsig_rdata.getMAC();
+            digest_len = tsig_rdata.getMACSize();
+            impl_->previous_timesigned_ = tsig_rdata.getTimeSigned();
+        }
+        return (impl_->postVerifyUpdate(TSIGError::BAD_TIME(), digest,
+                                        digest_len));
+    }
+
+    // TODO: signature length check based on RFC4635
+
+    // Handling empty MAC.  While RFC2845 doesn't explicitly prohibit other
+    // cases, it can only reasonably happen in a response with BADSIG or
+    // BADKEY.  We reject other cases as if it were BADSIG to avoid unexpected
+    // acceptance of a bogus signature.  This behavior follows the BIND 9
+    // implementation.
+    if (tsig_rdata.getMACSize() == 0) {
+        TSIGError error = TSIGError(tsig_rdata.getError());
+        if (error != TSIGError::BAD_SIG() && error != TSIGError::BAD_KEY()) {
+            error = TSIGError::BAD_SIG();
+        }
+        return (impl_->postVerifyUpdate(error, NULL, 0));
+    }
+
+    OutputBuffer variables(0);
+    HMACPtr hmac(CryptoLink::getCryptoLink().createHMAC(
+                     impl_->key_.getSecret(),
+                     impl_->key_.getSecretLength(),
+                     impl_->key_.getAlgorithm()),
+                 deleteHMAC);
+
+    // If the context has previous MAC (either the Request MAC or its own
+    // previous MAC), digest it.
+    if (impl_->state_ != INIT) {
+        impl_->digestPreviousMAC(variables, hmac);
+    }
+
+    //
+    // Digest DNS message (excluding the trailing TSIG RR and adjusting the
+    // QID and ARCOUNT header fields)
+    //
+    impl_->digestDNSMessage(variables, hmac, tsig_rdata.getOriginalID(),
+                            data, data_len - record->getLength());
+
+    // Digest TSIG variables.  If state_ is VERIFIED_RESPONSE, it's a
+    // continuation of the same TCP stream and skip digesting them except
+    // for time related variables (RFC2845 4.4).
+    // Note: we use the constant values for RR class and TTL specified
+    // in RFC2845.  For the RR class the effect should be the same
+    // because we reject an unexpected RR class; for TTL, the RFC
+    // isn't clear.  BIND 9 uses the received TTL, but we use the
+    // constant for simplicity (in practice it's quite unlikely to see
+    // a non 0 TTL, so probably this doesn't matter).
+    impl_->digestTSIGVariables(variables, hmac,
+                               TSIGRecord::getClass().getCode(),
+                               TSIGRecord::TSIG_TTL,
+                               tsig_rdata.getTimeSigned(),
+                               tsig_rdata.getFudge(), tsig_rdata.getError(),
+                               tsig_rdata.getOtherLen(),
+                               tsig_rdata.getOtherData(),
+                               impl_->state_ == VERIFIED_RESPONSE);
+
+    // Verify the digest with the received signature.
+    if (hmac->verify(tsig_rdata.getMAC(), tsig_rdata.getMACSize())) {
+        return (impl_->postVerifyUpdate(TSIGError::NOERROR(),
+                                        tsig_rdata.getMAC(),
+                                        tsig_rdata.getMACSize()));
+    }
+
+    return (impl_->postVerifyUpdate(TSIGError::BAD_SIG(), NULL, 0));
 }
+
 } // namespace dns
 } // namespace isc

src/lib/dns/tsig.h

@@ -17,12 +17,27 @@
 
 #include <boost/noncopyable.hpp>
 
+#include <exceptions/exceptions.h>
+
 #include <dns/tsigerror.h>
 #include <dns/tsigkey.h>
 #include <dns/tsigrecord.h>
 
 namespace isc {
 namespace dns {
+
+/// An exception that is thrown for logic errors identified in TSIG
+/// sign/verify operations.
+///
+/// Note that this exception is not thrown for TSIG protocol errors such as
+/// verification failures.  In general, this exception indicates an internal
+/// program bug.
+class TSIGContextError : public isc::Exception {
+public:
+    TSIGContextError(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
 /// TSIG session context.
 ///
 /// The \c TSIGContext class maintains a context of a signed session of
@@ -110,8 +125,10 @@ public:
     /// directly.
     enum State {
         INIT,                   ///< Initial state
-        SIGNED,                 ///< Sign completed
-        CHECKED ///< Verification completed (may or may not successfully)
+        WAIT_RESPONSE,          /// TODO: document update
+        RECEIVED_REQUEST,
+        SENT_RESPONSE,
+        VERIFIED_RESPONSE
     };
 
     /// \name Constructors and destructor
@@ -169,6 +186,7 @@ public:
     /// returns (without an exception being thrown), the internal state of
     /// the \c TSIGContext won't be modified.
     ///
+    /// \exception TSIGContextError Context already verified a response.
     /// \exception InvalidParameter \c data is NULL or \c data_len is 0
     /// \exception cryptolink::LibraryError Some unexpected error in the
     /// underlying crypto operation
@@ -183,6 +201,19 @@ public:
     ConstTSIGRecordPtr sign(const uint16_t qid, const void* const data,
                             const size_t data_len);
 
+    /// record can be NULL so that we can transparently check the case where
+    /// we sent a signed request but have received an unsigned response.
+    ///
+    /// One unexpected case that is not covered by this method:
+    /// receive a signed reply to an unsigned query
+    ///
+    /// TODO: Note about the overflow + BADTIME case.
+    ///
+    /// \exception TSIGContextError Context already signed a response.
+    /// \exception InvalidParameter
+    TSIGError verify(const TSIGRecord* const record, const void* const data,
+                     const size_t data_len);
+
     /// Return the current state of the context
     ///
     /// \note
@@ -200,18 +231,6 @@ public:
     /// \exception None
     TSIGError getError() const;
 
-    // This method is tentatively added for testing until a complete
-    // verify() method is implemented.  Once it's done this should be
-    // removed, and corresponding tests should be updated.
-    //
-    // This tentative "verify" method changes the internal state of
-    // the TSIGContext to the CHECKED as if it were verified (though possibly
-    // unsuccessfully) with given tsig_rdata.  If the error parameter is
-    // given and not NOERROR, it's recorded inside the context so that the
-    // subsequent sign() will behave accordingly.
-    void verifyTentative(ConstTSIGRecordPtr tsig,
-                         TSIGError error = TSIGError::NOERROR());
-
     /// \name Protocol constants and defaults
     ///
     //@{