[master] Merge branch 'trac893'

src/cppcheck-suppress.lst

@@ -12,4 +12,4 @@ functionConst:src/lib/cache/rrset_cache.h
 // Intentional self assignment tests.  Suppress warning about them.
 selfAssignment:src/lib/dns/tests/name_unittest.cc:293
 selfAssignment:src/lib/dns/tests/rdata_unittest.cc:228
-selfAssignment:src/lib/dns/tests/tsigkey_unittest.cc:120
+selfAssignment:src/lib/dns/tests/tsigkey_unittest.cc:125

src/lib/cryptolink/crypto_hmac.cc

@@ -144,8 +144,17 @@ public:
         // Botan's verify_mac checks if len matches the output_length,
         // which causes it to fail for truncated signatures, so we do
         // the check ourselves
+        // SEE BELOW FOR TEMPORARY CHANGE
         try {
             Botan::SecureVector<Botan::byte> our_mac = hmac_->final();
+            if (len < getOutputLength()) {
+                // Currently we don't support truncated signature.  To avoid
+                // validating too short signature accidently, we enforce the
+                // standard signature size for the moment.
+                // Once we support truncation correctly, this if-clause should
+                // (and the capitalized comment above) be removed.
+                return (false);
+            }
             if (len == 0 || len > getOutputLength()) {
                 len = getOutputLength();
             }

src/lib/cryptolink/tests/crypto_unittests.cc

@@ -233,18 +233,6 @@ TEST(CryptoLinkTest, HMAC_MD5_RFC2202_SIGN) {
                                        0x79 };
     doHMACTest(data4, secret4, 25, MD5, hmac_expected4, 16);
 
-    const uint8_t secret5[] = { 0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c,
-                                0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c,
-                                0x0c, 0x0c, 0x0c, 0x0c };
-    const uint8_t hmac_expected5[] = { 0x56, 0x46, 0x1e, 0xf2, 0x34,
-                                       0x2e, 0xdc, 0x00, 0xf9, 0xba,
-                                       0xb9, 0x95, 0x69, 0x0e, 0xfd,
-                                       0x4c };
-    doHMACTest("Test With Truncation", secret5, 16, MD5,
-               hmac_expected5, 16);
-    doHMACTest("Test With Truncation", secret5, 16, MD5,
-               hmac_expected5, 12);
-
     const uint8_t hmac_expected6[] = { 0x6b, 0x1a, 0xb7, 0xfe, 0x4b,
                                        0xd7, 0xbf, 0x8f, 0x0b, 0x62,
                                        0xe6, 0xce, 0x61, 0xb9, 0xd0,
@@ -261,6 +249,21 @@ TEST(CryptoLinkTest, HMAC_MD5_RFC2202_SIGN) {
                std::string(80, 0xaa).c_str(), 80, MD5, hmac_expected7, 16);
 }
 
+// Temporarily disabled
+TEST(CryptoLinkTest, DISABLED_HMAC_MD5_RFC2202_SIGN_TRUNCATED) {
+    const uint8_t secret5[] = { 0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c,
+                                0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c,
+                                0x0c, 0x0c, 0x0c, 0x0c };
+    const uint8_t hmac_expected5[] = { 0x56, 0x46, 0x1e, 0xf2, 0x34,
+                                       0x2e, 0xdc, 0x00, 0xf9, 0xba,
+                                       0xb9, 0x95, 0x69, 0x0e, 0xfd,
+                                       0x4c };
+    doHMACTest("Test With Truncation", secret5, 16, MD5,
+               hmac_expected5, 16);
+    doHMACTest("Test With Truncation", secret5, 16, MD5,
+               hmac_expected5, 12);
+}
+
 //
 // Test values taken from RFC 2202
 //
@@ -302,19 +305,6 @@ TEST(CryptoLinkTest, HMAC_SHA1_RFC2202_SIGN) {
                                        0x6c, 0x2d, 0x72, 0x35, 0xda };
     doHMACTest(std::string(50, 0xcd), secret4, 25, SHA1, hmac_expected4, 20);
 
-    const uint8_t secret5[] = { 0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c,
-                                0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c,
-                                0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c,
-                                0x0c, 0x0c };
-    const uint8_t hmac_expected5[] = { 0x4c, 0x1a, 0x03, 0x42, 0x4b,
-                                       0x55, 0xe0, 0x7f, 0xe7, 0xf2,
-                                       0x7b, 0xe1, 0xd5, 0x8b, 0xb9,
-                                       0x32, 0x4a, 0x9a, 0x5a, 0x04 };
-    doHMACTest("Test With Truncation", secret5, 20, SHA1,
-               hmac_expected5, 20);
-    doHMACTest("Test With Truncation", secret5, 20, SHA1,
-               hmac_expected5, 12);
-
     const uint8_t hmac_expected6[] = { 0xaa, 0x4a, 0xe5, 0xe1, 0x52,
                                        0x72, 0xd0, 0x0e, 0x95, 0x70,
                                        0x56, 0x37, 0xce, 0x8a, 0x3b,
@@ -331,6 +321,22 @@ TEST(CryptoLinkTest, HMAC_SHA1_RFC2202_SIGN) {
                std::string(80, 0xaa).c_str(), 80, SHA1, hmac_expected7, 20);
 }
 
+// Temporarily disabled
+TEST(CryptoLinkTest, DISABLED_HMAC_SHA1_RFC2202_SIGN_TRUNCATED) {
+    const uint8_t secret5[] = { 0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c,
+                                0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c,
+                                0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c,
+                                0x0c, 0x0c };
+    const uint8_t hmac_expected5[] = { 0x4c, 0x1a, 0x03, 0x42, 0x4b,
+                                       0x55, 0xe0, 0x7f, 0xe7, 0xf2,
+                                       0x7b, 0xe1, 0xd5, 0x8b, 0xb9,
+                                       0x32, 0x4a, 0x9a, 0x5a, 0x04 };
+    doHMACTest("Test With Truncation", secret5, 20, SHA1,
+               hmac_expected5, 20);
+    doHMACTest("Test With Truncation", secret5, 20, SHA1,
+               hmac_expected5, 12);
+}
+
 //
 // Test values taken from RFC 4231
 //
@@ -384,17 +390,6 @@ TEST(CryptoLinkTest, HMAC_SHA256_RFC2202_SIGN) {
                                        0x66, 0x5b };
     doHMACTest(std::string(50, 0xcd), secret4, 25, SHA256, hmac_expected4, 32);
 
-    const uint8_t secret5[] = { 0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c,
-                                0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c,
-                                0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c,
-                                0x0c, 0x0c };
-    const uint8_t hmac_expected5[] = { 0xa3, 0xb6, 0x16, 0x74, 0x73,
-                                       0x10, 0x0e, 0xe0, 0x6e, 0x0c,
-                                       0x79, 0x6c, 0x29, 0x55, 0x55,
-                                       0x2b };
-    doHMACTest("Test With Truncation", secret5, 20, SHA256,
-               hmac_expected5, 16);
-
     const uint8_t hmac_expected6[] = { 0x60, 0xe4, 0x31, 0x59, 0x1e,
                                        0xe0, 0xb6, 0x7f, 0x0d, 0x8a,
                                        0x26, 0xaa, 0xcb, 0xf5, 0xb7,
@@ -418,6 +413,19 @@ TEST(CryptoLinkTest, HMAC_SHA256_RFC2202_SIGN) {
                std::string(131, 0xaa).c_str(), 131, SHA256, hmac_expected7, 32);
 }
 
+TEST(CryptoLinkTest, DISABLED_HMAC_SHA256_RFC2202_SIGN_TRUNCATED) {
+    const uint8_t secret5[] = { 0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c,
+                                0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c,
+                                0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c,
+                                0x0c, 0x0c };
+    const uint8_t hmac_expected5[] = { 0xa3, 0xb6, 0x16, 0x74, 0x73,
+                                       0x10, 0x0e, 0xe0, 0x6e, 0x0c,
+                                       0x79, 0x6c, 0x29, 0x55, 0x55,
+                                       0x2b };
+    doHMACTest("Test With Truncation", secret5, 20, SHA256,
+               hmac_expected5, 16);
+}
+
 namespace {
     size_t
     sigVectorLength(HashAlgorithm alg, size_t len) {

src/lib/dns/python/tests/tsigkey_python_test.py

@@ -68,6 +68,9 @@ class TSIGKeyTest(unittest.TestCase):
 
 class TSIGKeyRingTest(unittest.TestCase):
     key_name = Name('example.com')
+    md5_name = Name('hmac-md5.sig-alg.reg.int')
+    sha1_name = Name('hmac-sha1')
+    sha256_name = Name('hmac-sha256')
     secret = b'someRandomData'
 
     def setUp(self):
@@ -152,18 +155,26 @@ class TSIGKeyRingTest(unittest.TestCase):
 
     def test_find(self):
         self.assertEqual((TSIGKeyRing.NOTFOUND, None),
-                         self.keyring.find(self.key_name))
+                         self.keyring.find(self.key_name, self.md5_name))
 
         self.assertEqual(TSIGKeyRing.SUCCESS,
                          self.keyring.add(TSIGKey(self.key_name,
-                                                  TSIGKey.HMACSHA256_NAME,
+                                                  self.sha256_name,
                                                   self.secret)))
-        (code, key) = self.keyring.find(self.key_name)
+        (code, key) = self.keyring.find(self.key_name, self.sha256_name)
         self.assertEqual(TSIGKeyRing.SUCCESS, code)
         self.assertEqual(self.key_name, key.get_key_name())
         self.assertEqual(TSIGKey.HMACSHA256_NAME, key.get_algorithm_name())
         self.assertEqual(self.secret, key.get_secret())
 
+        (code, key) = self.keyring.find(Name('different-key.example'),
+                                        self.sha256_name)
+        self.assertEqual(TSIGKeyRing.NOTFOUND, code)
+        self.assertEqual(None, key)
+        (code, key) = self.keyring.find(self.key_name, self.md5_name)
+        self.assertEqual(TSIGKeyRing.NOTFOUND, code)
+        self.assertEqual(None, key)
+
         self.assertRaises(TypeError, self.keyring.find, 1)
         self.assertRaises(TypeError, self.keyring.find, 'should be a name')
         self.assertRaises(TypeError, self.keyring.find, self.key_name, 0)
@@ -171,24 +182,28 @@ class TSIGKeyRingTest(unittest.TestCase):
     def test_find_from_some(self):
         self.assertEqual(TSIGKeyRing.SUCCESS,
                          self.keyring.add(TSIGKey(self.key_name,
-                                                  TSIGKey.HMACSHA256_NAME,
+                                                  self.sha256_name,
                                                   self.secret)))
         self.assertEqual(TSIGKeyRing.SUCCESS,
                          self.keyring.add(TSIGKey(Name('another.example'),
-                                                  TSIGKey.HMACMD5_NAME,
+                                                  self.md5_name,
                                                   self.secret)))
         self.assertEqual(TSIGKeyRing.SUCCESS,
                          self.keyring.add(TSIGKey(Name('more.example'),
-                                                  TSIGKey.HMACSHA1_NAME,
+                                                  self.sha1_name,
                                                   self.secret)))
 
-        (code, key) = self.keyring.find(Name('another.example'))
+        (code, key) = self.keyring.find(Name('another.example'), self.md5_name)
         self.assertEqual(TSIGKeyRing.SUCCESS, code)
         self.assertEqual(Name('another.example'), key.get_key_name())
         self.assertEqual(TSIGKey.HMACMD5_NAME, key.get_algorithm_name())
 
         self.assertEqual((TSIGKeyRing.NOTFOUND, None),
-                         self.keyring.find(Name('noexist.example')))
+                         self.keyring.find(Name('noexist.example'),
+                                           self.sha1_name))
+        self.assertEqual((TSIGKeyRing.NOTFOUND, None),
+                         self.keyring.find(Name('another.example'),
+                                           self.sha1_name))
 
 if __name__ == '__main__':
     unittest.main()

src/lib/dns/python/tsigkey_python.cc

@@ -416,10 +416,12 @@ TSIGKeyRing_remove(const s_TSIGKeyRing* self, PyObject* args) {
 PyObject*
 TSIGKeyRing_find(const s_TSIGKeyRing* self, PyObject* args) {
     s_Name* key_name;
+    s_Name* algorithm_name;
 
-    if (PyArg_ParseTuple(args, "O!", &name_type, &key_name)) {
+    if (PyArg_ParseTuple(args, "O!O!", &name_type, &key_name,
+                         &name_type, &algorithm_name)) {
         const TSIGKeyRing::FindResult result =
-            self->keyring->find(*key_name->name);
+            self->keyring->find(*key_name->name, *algorithm_name->name);
         if (result.key != NULL) {
             s_TSIGKey* key = PyObject_New(s_TSIGKey, &tsigkey_type);
             if (key == NULL) {

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

@@ -40,6 +40,10 @@ 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
+BUILT_SOURCES += tsig_verify10.wire
 
 # NOTE: keep this in sync with real file listing
 # so is included in tarball
@@ -108,6 +112,10 @@ 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
+EXTRA_DIST += tsig_verify10.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_verify10.spec

@@ -0,0 +1,22 @@
+#
+# A simple DNS query message with TSIG signed whose MAC is too short
+# (only 1 byte)
+#
+
+[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: 1
+mac: 0x22
+original_id: 0x2d65

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;
@@ -111,14 +118,19 @@ protected:
 
     boost::scoped_ptr<TSIGContext> tsig_ctx;
     boost::scoped_ptr<TSIGContext> tsig_verify_ctx;
+    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
@@ -157,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::SENT_REQUEST : 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,
@@ -191,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());
@@ -199,6 +234,38 @@ TEST_F(TSIGTest, initialState) {
     EXPECT_EQ(TSIGError(Rcode::NOERROR()), tsig_ctx->getError());
 }
 
+TEST_F(TSIGTest, constructFromKeyRing) {
+    // Construct a TSIG context with an empty key ring.  Key shouldn't be
+    // found, and the BAD_KEY error should be recorded.
+    TSIGContext ctx1(test_name, TSIGKey::HMACMD5_NAME(), keyring);
+    EXPECT_EQ(TSIGContext::INIT, ctx1.getState());
+    EXPECT_EQ(TSIGError::BAD_KEY(), ctx1.getError());
+
+    // Add a matching key (we don't use the secret so leave it empty), and
+    // construct it again.  This time it should be constructed with a valid
+    // key.
+    keyring.add(TSIGKey(test_name, TSIGKey::HMACMD5_NAME(), NULL, 0));
+    TSIGContext ctx2(test_name, TSIGKey::HMACMD5_NAME(), keyring);
+    EXPECT_EQ(TSIGContext::INIT, ctx2.getState());
+    EXPECT_EQ(TSIGError::NOERROR(), ctx2.getError());
+
+    // Similar to the first case except that the key ring isn't empty but
+    // it doesn't contain a matching key.
+    TSIGContext ctx3(test_name, TSIGKey::HMACSHA1_NAME(), keyring);
+    EXPECT_EQ(TSIGContext::INIT, ctx3.getState());
+    EXPECT_EQ(TSIGError::BAD_KEY(), ctx3.getError());
+
+    TSIGContext ctx4(Name("different-key.example"), TSIGKey::HMACMD5_NAME(),
+                     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
 // "dig -y www.example.com:SFuWd/q99SzF8Yzd1QbB9g== www.example.com
 // QID: 0x2d65
@@ -213,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));
     }
@@ -231,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));
     }
@@ -247,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));
     }
@@ -281,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
@@ -293,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;
@@ -308,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;
 }
@@ -339,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[] = {
@@ -411,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());
     }
 }
 
@@ -443,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(),
@@ -459,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),
@@ -467,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
     }
@@ -490,15 +704,203 @@ 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: BADKEYSIG
+        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::SENT_REQUEST);
+    }
+
+    // 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::SENT_REQUEST);
+    }
+}
+
+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::SENT_REQUEST);
+    }
+
+    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::SENT_REQUEST);
+    }
+
+    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::SENT_REQUEST);
+    }
+
+    // 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);
+}
+
+TEST_F(TSIGTest, tooShortMAC) {
+    // Too short MAC should be rejected.
+    // Note: when we implement RFC4635-based checks, the error code will
+    // (probably) be FORMERR.
+
+    isc::util::detail::gettimeFunction = testGetTime<0x4da8877a>;
+    createMessageFromFile("tsig_verify10.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);
     }
 }
 

src/lib/dns/tests/tsigerror_unittest.cc

@@ -93,6 +93,20 @@ TEST(TSIGErrorTest, toText) {
     EXPECT_EQ("65535", TSIGError(65535).toText());
 }
 
+TEST(TSIGErrorTest, toRcode) {
+    // TSIGError derived from the standard Rcode
+    EXPECT_EQ(Rcode::NOERROR(), TSIGError(Rcode::NOERROR()).toRcode());
+
+    // Well known TSIG errors
+    EXPECT_EQ(Rcode::NOTAUTH(), TSIGError::BAD_SIG().toRcode());
+    EXPECT_EQ(Rcode::NOTAUTH(), TSIGError::BAD_KEY().toRcode());
+    EXPECT_EQ(Rcode::NOTAUTH(), TSIGError::BAD_TIME().toRcode());
+
+    // Unknown (or not yet supported) codes are treated as SERVFAIL.
+    EXPECT_EQ(Rcode::SERVFAIL(), TSIGError(19).toRcode());
+    EXPECT_EQ(Rcode::SERVFAIL(), TSIGError(65535).toRcode());
+}
+
 // test operator<<.  We simply confirm it appends the result of toText().
 TEST(TSIGErrorTest, LeftShiftOperator) {
     ostringstream oss;

src/lib/dns/tests/tsigkey_unittest.cc

@@ -33,7 +33,7 @@ class TSIGKeyTest : public ::testing::Test {
 protected:
     TSIGKeyTest() : secret("someRandomData"), key_name("example.com") {}
     string secret;
-    Name key_name;
+    const Name key_name;
 };
 
 TEST_F(TSIGKeyTest, algorithmNames) {
@@ -59,9 +59,13 @@ TEST_F(TSIGKeyTest, construct) {
     EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, secret.c_str(),
                         secret.size(), key.getSecret(), key.getSecretLength());
 
+    // "unknown" algorithm is only accepted with empty secret.
     EXPECT_THROW(TSIGKey(key_name, Name("unknown-alg"),
                          secret.c_str(), secret.size()),
                  isc::InvalidParameter);
+    TSIGKey key2(key_name, Name("unknown-alg"), NULL, 0);
+    EXPECT_EQ(key_name, key2.getKeyName());
+    EXPECT_EQ(Name("unknown-alg"), key2.getAlgorithmName());
 
     // The algorithm name should be converted to the canonical form.
     EXPECT_EQ("hmac-sha1.",
@@ -69,6 +73,7 @@ TEST_F(TSIGKeyTest, construct) {
                       secret.c_str(),
                       secret.size()).getAlgorithmName().toText());
 
+    // Same for key name
     EXPECT_EQ("example.com.",
               TSIGKey(Name("EXAMPLE.CoM."), TSIGKey::HMACSHA256_NAME(),
                       secret.c_str(),
@@ -125,12 +130,18 @@ class TSIGKeyRingTest : public ::testing::Test {
 protected:
     TSIGKeyRingTest() :
         key_name("example.com"),
+        md5_name("hmac-md5.sig-alg.reg.int"),
+        sha1_name("hmac-sha1"),
+        sha256_name("hmac-sha256"),
         secretstring("anotherRandomData"),
         secret(secretstring.c_str()),
         secret_len(secretstring.size())
     {}
     TSIGKeyRing keyring;
-    Name key_name;
+    const Name key_name;
+    const Name md5_name;
+    const Name sha1_name;
+    const Name sha256_name;
 private:
     const string secretstring;
 protected:
@@ -203,44 +214,60 @@ TEST_F(TSIGKeyRingTest, removeFromSome) {
 }
 
 TEST_F(TSIGKeyRingTest, find) {
-    EXPECT_EQ(TSIGKeyRing::NOTFOUND, keyring.find(key_name).code);
-    EXPECT_EQ(static_cast<const TSIGKey*>(NULL), keyring.find(key_name).key);
-
-    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.add(
-                  TSIGKey(key_name, TSIGKey::HMACSHA256_NAME(),
-                          secret, secret_len)));
-    const TSIGKeyRing::FindResult result(keyring.find(key_name));
-    EXPECT_EQ(TSIGKeyRing::SUCCESS, result.code);
-    EXPECT_EQ(key_name, result.key->getKeyName());
-    EXPECT_EQ(TSIGKey::HMACSHA256_NAME(), result.key->getAlgorithmName());
+    // If the keyring is empty the search should fail.
+    EXPECT_EQ(TSIGKeyRing::NOTFOUND, keyring.find(key_name, md5_name).code);
+    EXPECT_EQ(static_cast<const TSIGKey*>(NULL),
+              keyring.find(key_name, md5_name).key);
+
+    // Add a key and try to find it.  Should succeed.
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.add(TSIGKey(key_name, sha256_name,
+                                                        secret, secret_len)));
+    const TSIGKeyRing::FindResult result1(keyring.find(key_name, sha256_name));
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, result1.code);
+    EXPECT_EQ(key_name, result1.key->getKeyName());
+    EXPECT_EQ(TSIGKey::HMACSHA256_NAME(), result1.key->getAlgorithmName());
     EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, secret, secret_len,
-                        result.key->getSecret(),
-                        result.key->getSecretLength());
+                        result1.key->getSecret(),
+                        result1.key->getSecretLength());
+
+    // If either key name or algorithm doesn't match, search should fail.
+    const TSIGKeyRing::FindResult result2 =
+        keyring.find(Name("different-key.example"), sha256_name);
+    EXPECT_EQ(TSIGKeyRing::NOTFOUND, result2.code);
+    EXPECT_EQ(static_cast<const TSIGKey*>(NULL), result2.key);
+
+    const TSIGKeyRing::FindResult result3 = keyring.find(key_name, md5_name);
+    EXPECT_EQ(TSIGKeyRing::NOTFOUND, result3.code);
+    EXPECT_EQ(static_cast<const TSIGKey*>(NULL), result3.key);
 }
 
 TEST_F(TSIGKeyRingTest, findFromSome) {
     // essentially the same test, but search a larger set
 
-    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.add(
-                  TSIGKey(key_name, TSIGKey::HMACSHA256_NAME(),
-                          secret, secret_len)));
-    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.add(
-                  TSIGKey(Name("another.example"), TSIGKey::HMACMD5_NAME(),
-                          secret, secret_len)));
-    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.add(
-                  TSIGKey(Name("more.example"), TSIGKey::HMACSHA1_NAME(),
-                          secret, secret_len)));
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.add(TSIGKey(key_name, sha256_name,
+                                                        secret, secret_len)));
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.add(TSIGKey(Name("another.example"),
+                                                        md5_name,
+                                                        secret, secret_len)));
+    EXPECT_EQ(TSIGKeyRing::SUCCESS, keyring.add(TSIGKey(Name("more.example"),
+                                                        sha1_name,
+                                                        secret, secret_len)));
 
     const TSIGKeyRing::FindResult result(
-        keyring.find(Name("another.example")));
+        keyring.find(Name("another.example"), md5_name));
     EXPECT_EQ(TSIGKeyRing::SUCCESS, result.code);
     EXPECT_EQ(Name("another.example"), result.key->getKeyName());
     EXPECT_EQ(TSIGKey::HMACMD5_NAME(), result.key->getAlgorithmName());
 
     EXPECT_EQ(TSIGKeyRing::NOTFOUND,
-              keyring.find(Name("noexist.example")).code);
+              keyring.find(Name("noexist.example"), sha1_name).code);
     EXPECT_EQ(static_cast<const TSIGKey*>(NULL),
-              keyring.find(Name("noexist.example")).key);
+              keyring.find(Name("noexist.example"), sha256_name).key);
+
+    EXPECT_EQ(TSIGKeyRing::NOTFOUND,
+              keyring.find(Name("another.example"), sha1_name).code);
+    EXPECT_EQ(static_cast<const TSIGKey*>(NULL),
+              keyring.find(Name("another.example"), sha256_name).key);
 }
 
 TEST(TSIGStringTest, TSIGKeyFromToString) {
@@ -248,6 +275,8 @@ TEST(TSIGStringTest, TSIGKeyFromToString) {
     TSIGKey k2 = TSIGKey("test.example.:MSG6Ng==:hmac-md5.sig-alg.reg.int.");
     TSIGKey k3 = TSIGKey("test.example:MSG6Ng==");
     TSIGKey k4 = TSIGKey(Name("test.example."), Name("hmac-sha1."), NULL, 0);
+    // "Unknown" key with empty secret is okay
+    TSIGKey k5 = TSIGKey("test.example.::unknown");
 
     EXPECT_EQ("test.example.:MSG6Ng==:hmac-md5.sig-alg.reg.int.",
               k1.toText());
@@ -256,6 +285,8 @@ TEST(TSIGStringTest, TSIGKeyFromToString) {
     EXPECT_EQ("test.example.:MSG6Ng==:hmac-md5.sig-alg.reg.int.",
               k3.toText());
     EXPECT_EQ("test.example.::hmac-sha1.", k4.toText());
+    EXPECT_EQ(Name("test.example."), k5.getKeyName());
+    EXPECT_EQ(Name("unknown"), k5.getAlgorithmName());
 
     EXPECT_THROW(TSIGKey(""), isc::InvalidParameter);
     EXPECT_THROW(TSIGKey(":"), isc::InvalidParameter);
@@ -266,7 +297,6 @@ TEST(TSIGStringTest, TSIGKeyFromToString) {
     EXPECT_THROW(TSIGKey("test.example.:"), isc::InvalidParameter);
     EXPECT_THROW(TSIGKey("test.example.:MSG6Ng==:"), isc::InvalidParameter);
     EXPECT_THROW(TSIGKey("test.example.:MSG6Ng==:unknown"), isc::InvalidParameter);
-
 }
 
 

src/lib/dns/tests/tsigrecord_unittest.cc

@@ -98,12 +98,11 @@ TEST_F(TSIGRecordTest, fromParams) {
 
     // Unexpected class
     EXPECT_THROW(TSIGRecord(test_name, RRClass::IN(), TSIGRecord::getTTL(),
-                            test_rdata, 85),
-                 DNSMessageFORMERR);
+                            test_rdata, 85), DNSMessageFORMERR);
 
-    // Unexpected TTL (simply ignored)
-    EXPECT_NO_THROW(TSIGRecord(test_name, TSIGRecord::getClass(),
-                               RRTTL(3600), test_rdata, 85));
+    // Unexpected TTL
+    EXPECT_THROW(TSIGRecord(test_name, TSIGRecord::getClass(),
+                            RRTTL(3600), test_rdata, 85), DNSMessageFORMERR);
 }
 
 TEST_F(TSIGRecordTest, recordToWire) {

src/lib/dns/tsig.cc

@@ -16,7 +16,7 @@
 
 #include <stdint.h>
 
-#include <cassert>              // for the tentative verifyTentative()
+#include <cassert>
 #include <vector>
 
 #include <boost/shared_ptr.hpp>
@@ -44,6 +44,17 @@ namespace isc {
 namespace dns {
 namespace {
 typedef boost::shared_ptr<HMAC> HMACPtr;
+
+// 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);
+}
 }
 
 struct TSIGContext::TSIGContextImpl {
@@ -51,17 +62,172 @@ struct TSIGContext::TSIGContextImpl {
         state_(INIT), key_(key), error_(Rcode::NOERROR()),
         previous_timesigned_(0)
     {}
+
+    // This helper method is used from verify().  It's expected to be called
+    // just before verify() returns.  It updates internal state based on
+    // the verification result and return the TSIGError to be returned to
+    // the caller of verify(), so that verify() can call this method within
+    // its 'return' statement.
+    TSIGError postVerifyUpdate(TSIGError error, const void* digest,
+                               uint16_t digest_len)
+    {
+        if (state_ == INIT) {
+            state_ = RECEIVED_REQUEST;
+        } else if (state_ == SENT_REQUEST && 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);
+    }
+
+    // The following three are helper methods to compute the digest for
+    // TSIG sign/verify in order to unify the common code logic for sign()
+    // and verify() and to keep these callers concise.
+    // These methods take an HMAC object, which will be updated with the
+    // calculated digest.
+    // Note: All methods construct a local OutputBuffer as a work space with a
+    // fixed initial buffer size to avoid intermediate buffer extension.
+    // This should be efficient enough, especially for fundamentally expensive
+    // operation like cryptographic sign/verify, but if the creation of the
+    // buffer in each helper method is still identified to be a severe
+    // performance bottleneck, we could have this class a buffer as a member
+    // variable and reuse it throughout the object's lifetime.  Right now,
+    // we prefer keeping the scope for local things as small as possible.
+    void digestPreviousMAC(HMACPtr hmac) const;
+    void digestTSIGVariables(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(HMACPtr hmac, uint16_t qid, const void* data,
+                          size_t data_len) const;
     State state_;
-    TSIGKey key_;
+    const TSIGKey key_;
     vector<uint8_t> previous_digest_;
     TSIGError error_;
     uint64_t previous_timesigned_; // only meaningful for response with BADTIME
 };
 
+void
+TSIGContext::TSIGContextImpl::digestPreviousMAC(HMACPtr hmac) const {
+    // We should have ensured the digest size fits 16 bits within this class
+    // implementation.
+    assert(previous_digest_.size() <= 0xffff);
+
+    OutputBuffer buffer(sizeof(uint16_t) + previous_digest_.size());
+    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(
+    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
+{
+    // It's bit complicated, but we can still predict the necessary size of
+    // the data to be digested.  So we precompute it to avoid possible
+    // reallocation inside OutputBuffer (not absolutely necessary, but this
+    // is a bit more efficient)
+    size_t data_size = 8;
+    if (!time_variables_only) {
+        data_size += 10 + key_.getKeyName().getLength() +
+            key_.getAlgorithmName().getLength();
+    }
+    OutputBuffer buffer(data_size);
+
+    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);
+
+    if (!time_variables_only) {
+        buffer.writeUint16(error);
+        buffer.writeUint16(otherlen);
+    }
+
+    hmac->update(buffer.getData(), buffer.getLength());
+    if (!time_variables_only && otherlen > 0) {
+        hmac->update(otherdata, otherlen);
+    }
+}
+
+// In digestDNSMessage, we exploit some minimum knowledge of DNS message
+// format:
+// - the header section has a fixed length of 12 octets (MESSAGE_HEADER_LEN)
+// - the offset in the header section to the ID field is 0
+// - the offset in the header section to the ARCOUNT field is 10 (and the field
+//   length is 2 octets)
+// We could construct a separate Message object from the given data, adjust
+// fields via the Message interfaces and then render it back to a separate
+// buffer, but that would be overkilling.  The DNS message header has a
+// fixed length and necessary modifications are quite straightforward, so
+// we do the job using lower level interfaces.
+namespace {
+const size_t MESSAGE_HEADER_LEN = 12;
+}
+void
+TSIGContext::TSIGContextImpl::digestDNSMessage(HMACPtr hmac,
+                                               uint16_t qid, const void* data,
+                                               size_t data_len) const
+{
+    OutputBuffer buffer(MESSAGE_HEADER_LEN);
+    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)
+    buffer.writeUint16(InputBuffer(msgptr, sizeof(uint16_t)).readUint16() - 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))
 {
 }
 
+TSIGContext::TSIGContext(const Name& key_name, const Name& algorithm_name,
+                         const TSIGKeyRing& keyring) : impl_(NULL)
+{
+    const TSIGKeyRing::FindResult result(keyring.find(key_name,
+                                                      algorithm_name));
+    if (result.code == TSIGKeyRing::NOTFOUND) {
+        // 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);
+    }
+}
+
 TSIGContext::~TSIGContext() {
     delete impl_;
 }
@@ -80,21 +246,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_;
     }
 
@@ -107,11 +272,10 @@ 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);
     }
 
-    OutputBuffer variables(0);
     HMACPtr hmac(CryptoLink::getCryptoLink().createHMAC(
                      impl_->key_.getSecret(),
                      impl_->key_.getSecretLength(),
@@ -121,87 +285,169 @@ 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(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(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();
+    assert(digest.size() <= 0xffff); // cryptolink API should have ensured it.
     ConstTSIGRecordPtr tsig(new TSIGRecord(
                                 impl_->key_.getKeyName(),
                                 any::TSIG(impl_->key_.getAlgorithmName(),
                                           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) ? SENT_REQUEST : 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));
+    }
+
+    const any::TSIG& tsig_rdata = record->getRdata();
+
+    // 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)
+    // Note: for simplicity we don't explicitly catch the case of too small
+    // current time causing underflow.  With the fact that fudge is quite
+    // small and (for now) non configurable, it shouldn't be a real concern
+    // in practice.
+    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
+    // (Right now we enforce the standard signature length in libcryptolink)
+
+    // 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));
+    }
+
+    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(hmac);
     }
 
-    // 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());
+    //
+    // Digest DNS message (excluding the trailing TSIG RR and adjusting the
+    // QID and ARCOUNT header fields)
+    //
+    impl_->digestDNSMessage(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, not received values (we reject other values in constructing
+    // the TSIGRecord).
+    impl_->digestTSIGVariables(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);
 
-    impl_->state_ = CHECKED;
+    // 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
@@ -59,8 +74,7 @@ namespace dns {
 /// in this mode will identify the appropriate TSIG key (or internally record
 /// an error if it doesn't find a key).  The server will then verify the
 /// query with the context, and generate a signed response using the same
-/// same context.  (Note: this mode is not yet implemented and may change,
-/// see below).
+/// same context.
 ///
 /// When multiple messages belong to the same TSIG session, either side
 /// (signer or verifier) will keep using the same context.  It records
@@ -68,8 +82,68 @@ namespace dns {
 /// calls to \c sign() or \c verify() work correctly in terms of the TSIG
 /// protocol.
 ///
-/// \note The \c verify() method is not yet implemented.  The implementation
-/// and documentation should be updated in the corresponding task.
+/// \b Examples
+///
+/// This is a typical client application that sends a TSIG signed query
+/// and verifies the response.
+///
+/// \code
+///    // "renderer" is of MessageRenderer to render the message.
+///    // (TSIGKey would be configured from config or command line in real app)
+///    TSIGContext ctx(TSIGKey("key.example:MSG6Ng=="));
+///    Message message(Message::RENDER);
+///    message.addQuestion(Question(Name("www.example.com"), RRClass::IN(),
+///                                 RRType::A()));
+///    message.toWire(renderer, ctx);
+///
+///    // sendto, then recvfrom.  received result in (data, data_len)
+///
+///    message.clear(Message::PARSE);
+///    InputBuffer buffer(data, data_len);
+///    message.fromWire(buffer);
+///    TSIGError tsig_error = ctx.verify(message.getTSIGRecord(),
+///                                      data, data_len);
+///    if (tsig_error == TSIGError::NOERROR()) {
+///        // okay.  ctx can be continuously used if it's receiving subsequent
+///        // signed responses from a TCP stream.
+///    } else if (message.getRcode() == Rcode::NOTAUTH()) {
+///        // hard error.  give up this transaction per RFC2845 4.6.
+///    } else {
+///        // Other error: discard response keep waiting with the same ctx
+///        // for another (again, RFC2845 4.6).
+///    } \endcode
+///
+/// And this is a typical server application that authenticates a signed
+/// query and returns a response according to the result.
+///
+/// \code
+///    // Assume "message" is of type Message for query handling and
+///    // "renderer" is of MessageRenderer to render responses.
+///    Message message(Message::RENDER);
+///
+///    TSIGKeyRing keyring; // this must be configured with keys somewhere
+///
+///    // Receive a query and store it in (data, data_len)
+///    InputBuffer buffer(data, data_len);
+///    message.clear(Message::PARSE);
+///    message.fromWire(buffer);
+///
+///    const TSIGRecord* tsig = message.getTSIGRecord();
+///    if (tsig != NULL) {
+///        TSIGContext ctx(tsig->getName(), tsig->getRdata().getAlgorithm(),
+///                        keyring);
+///        ctx.verify(tsig, data, data_len);
+///
+///        // prepare response
+///        message.makeResponse();
+///        //...
+///        message.toWire(renderer, ctx);
+///
+///        // send the response data back to the client.
+///        // If this is a beginning of a signed session over a TCP and
+///        // server has more data to send to the client, this ctx
+///        // will be used to sign subsequent messages.
+///    } \endcode
 ///
 /// <b>TCP Consideration</b>
 ///
@@ -110,8 +184,10 @@ public:
     /// directly.
     enum State {
         INIT,                   ///< Initial state
-        SIGNED,                 ///< Sign completed
-        CHECKED ///< Verification completed (may or may not successfully)
+        SENT_REQUEST, ///< Client sent a signed request, waiting response
+        RECEIVED_REQUEST,       ///< Server received a signed request
+        SENT_RESPONSE,          ///< Server sent a signed response
+        VERIFIED_RESPONSE       ///< Client successfully verified a response
     };
 
     /// \name Constructors and destructor
@@ -124,6 +200,10 @@ public:
     /// \param key The TSIG key to be used for TSIG sessions with this context.
     explicit TSIGContext(const TSIGKey& key);
 
+    /// Constructor from key parameters and key ring.
+    TSIGContext(const Name& key_name, const Name& algorithm_name,
+                const TSIGKeyRing& keyring);
+
     /// The destructor.
     ~TSIGContext();
     //@}
@@ -141,6 +221,13 @@ public:
     /// complete TSIG RR into the message that has been signed so that it
     /// will become a complete TSIG-signed message.
     ///
+    /// In general, this method is called once by a client to send a
+    /// signed request or one more times by a server to sign
+    /// response(s) to a signed request.  To avoid allowing accidental
+    /// misuse, if this method is called after a "client" validates a
+    /// response, an exception of class \c TSIGContextError will be
+    /// thrown.
+    ///
     /// \note Normal applications are not expected to call this method
     /// directly; they will usually use the \c Message::toWire() method
     /// with a \c TSIGContext object being a parameter and have the
@@ -165,6 +252,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
@@ -179,6 +267,92 @@ public:
     ConstTSIGRecordPtr sign(const uint16_t qid, const void* const data,
                             const size_t data_len);
 
+    /// Verify a DNS message.
+    ///
+    /// This method verifies given data along with the context and a given
+    /// TSIG in the form of a \c TSIGRecord object.  The data to be verified
+    /// is generally expected to be a complete, wire-format DNS message,
+    /// exactly as received by the host, and ending with a TSIG RR.
+    /// After verification process this method updates its internal state,
+    /// and returns the result in the form of a \c TSIGError object.
+    /// Possible return values are (see the \c TSIGError class description
+    /// for the mnemonics):
+    ///
+    /// - \c NOERROR: The data has been verified correctly.
+    /// - \c FORMERR: \c TSIGRecord is not given (see below).
+    /// - \c BAD_KEY: Appropriate key is not found or specified key doesn't
+    ///               match for the data.
+    /// - \c BAD_TIME: The current time doesn't fall in the range specified
+    ///                in the TSIG.
+    /// - \c BAD_SIG: The signature given in the TSIG doesn't match against
+    ///               the locally computed digest or is the signature is
+    ///               invalid in other way.
+    ///
+    /// If this method is called by a DNS client waiting for a signed
+    /// response and the result is not \c NOERROR, the context can be used
+    /// to try validating another signed message as described in RFC2845
+    /// Section 4.6.
+    ///
+    /// If this method is called by a DNS server that tries to authenticate
+    /// a signed request, and if the result is not \c NOERROR, the
+    /// corresponding error condition is recorded in the context so that
+    /// the server can return a response indicating what was wrong by calling
+    /// \c sign() with the updated context.
+    ///
+    /// In general, this method is called once by a server for
+    /// authenticating a signed request or one more times by a client to
+    /// validate signed response(s) to a signed request.  To avoid allowing
+    /// accidental misuse, if this method is called after a "server" signs
+    /// a response, an exception of class \c TSIGContextError will be thrown.
+    ///
+    /// The \c record parameter can be NULL; in that case this method simply
+    /// returns \c FORMERR as the case described in Section 4.6 of RFC2845,
+    /// i.e., receiving an unsigned response to a signed request.  This way
+    /// a client can transparently pass the result of
+    /// \c Message::getTSIGRecord() without checking whether it's non NULL
+    /// and take an appropriate action based on the result of this method.
+    ///
+    /// This method handles the given data mostly as opaque.  It digests
+    /// the data assuming it begins with a DNS header and ends with a TSIG
+    /// RR whose length is given by calling \c TSIGRecord::getLength() on
+    /// \c record, but otherwise it doesn't parse the data to confirm the
+    /// assumption.  It's caller's responsibility to ensure the data is
+    /// valid and consistent with \c record.  To avoid disruption, this
+    /// method performs minimal validation on the given \c data and \c record:
+    /// \c data must not be NULL; \c data_len must not be smaller than the
+    /// sum of the DNS header length (fixed, 12 octets) and the length of
+    /// the TSIG RR.  If this check fails it throws an \c InvalidParameter
+    /// exception.
+    ///
+    /// One unexpected case that is not covered by this method is that a
+    /// client receives a signed response to an unsigned request.  RFC2845 is
+    /// silent about such cases; BIND 9 explicitly identifies the case and
+    /// rejects it.  With this implementation, the client can know that the
+    /// response contains a TSIG via the result of
+    /// \c Message::getTSIGRecord() and that it is an unexpected TSIG due to
+    /// the fact that it doesn't have a corresponding \c TSIGContext.
+    /// It's up to the client implementation whether to react to such a case
+    /// explicitly (for example, it could either ignore the TSIG and accept
+    /// the response or drop it).
+    ///
+    /// This method provides the strong exception guarantee; unless the method
+    /// returns (without an exception being thrown), the internal state of
+    /// the \c TSIGContext won't be modified.
+    ///
+    /// \todo Support intermediate TCP DNS messages without TSIG (RFC2845 4.4)
+    /// \todo Signature truncation support based on RFC4635
+    ///
+    /// \exception TSIGContextError Context already signed a response.
+    /// \exception InvalidParameter \c data is NULL or \c data_len is too small.
+    ///
+    /// \param record The \c TSIGRecord to be verified with \c data
+    /// \param data Points to the wire-format data (exactly as received) to
+    /// be verified
+    /// \param data_len The length of \c data in bytes
+    /// \return The \c TSIGError that indicates verification result
+    TSIGError verify(const TSIGRecord* const record, const void* const data,
+                     const size_t data_len);
+
     /// Return the current state of the context
     ///
     /// \note
@@ -196,18 +370,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
     ///
     //@{

src/lib/dns/tsigerror.cc

@@ -49,6 +49,17 @@ TSIGError::toText() const {
     }
 }
 
+Rcode
+TSIGError::toRcode() const {
+    if (code_ <= MAX_RCODE_FOR_TSIGERROR) {
+        return (Rcode(code_));
+    }
+    if (code_ > BAD_TIME_CODE) {
+        return (Rcode::SERVFAIL());
+    }
+    return (Rcode::NOTAUTH());
+}
+
 std::ostream&
 operator<<(std::ostream& os, const TSIGError& error) {
     return (os << error.toText());

src/lib/dns/tsigerror.h

@@ -125,6 +125,22 @@ public:
     /// \return A string representation of the \c TSIGError.
     std::string toText() const;
 
+    /// \brief Convert the \c TSIGError to a \c Rcode
+    ///
+    /// This method returns an \c Rcode object that is corresponding to
+    /// the TSIG error.  The returned \c Rcode is expected to be used
+    /// by a verifying server to specify the RCODE of a response when
+    /// TSIG verification fails.
+    ///
+    /// Specifically, this method returns \c Rcode::NOTAUTH() for the
+    /// TSIG specific errors, BADSIG, BADKEY, BADTIME, as described in
+    /// RFC2845.  For errors derived from the standard Rcode (code 0-15),
+    /// it returns the corresponding \c Rcode.  For others, this method
+    /// returns \c Rcode::SERVFAIL() as a last resort.
+    ///
+    /// \exception None
+    Rcode toRcode() const;
+
     /// A constant TSIG error object derived from \c Rcode::NOERROR()
     static const TSIGError& NOERROR();
 

src/lib/dns/tsigkey.cc

@@ -42,8 +42,7 @@ namespace {
         if (name == TSIGKey::HMACSHA256_NAME()) {
             return (isc::cryptolink::SHA256);
         }
-        isc_throw(InvalidParameter,
-                  "Unknown TSIG algorithm is specified: " << name);
+        return (isc::cryptolink::UNKNOWN_HASH);
     }
 }
 
@@ -74,7 +73,13 @@ TSIGKey::TSIGKey(const Name& key_name, const Name& algorithm_name,
     if ((secret != NULL && secret_len == 0) ||
         (secret == NULL && secret_len != 0)) {
         isc_throw(InvalidParameter,
-                  "TSIGKey secret and its length are inconsistent");
+                  "TSIGKey secret and its length are inconsistent: " <<
+                  key_name << ":" << algorithm_name);
+    }
+    if (algorithm == isc::cryptolink::UNKNOWN_HASH && secret_len != 0) {
+        isc_throw(InvalidParameter,
+                  "TSIGKey with unknown algorithm has non empty secret: " <<
+                  key_name << ":" << algorithm_name);
     }
     impl_ = new TSIGKeyImpl(key_name, algorithm_name, algorithm, secret,
                             secret_len);
@@ -111,8 +116,15 @@ TSIGKey::TSIGKey(const std::string& str) : impl_(NULL) {
         vector<uint8_t> secret;
         isc::util::encode::decodeBase64(secret_str, secret);
 
+        if (algorithm == isc::cryptolink::UNKNOWN_HASH && !secret.empty()) {
+            isc_throw(InvalidParameter,
+                      "TSIG key with unknown algorithm has non empty secret: "
+                      << str);
+        }
+
         impl_ = new TSIGKeyImpl(Name(keyname_str), algo_name, algorithm,
-                                &secret[0], secret.size());
+                                secret.empty() ? NULL : &secret[0],
+                                secret.size());
     } catch (const Exception& e) {
         // 'reduce' the several types of exceptions name parsing and
         // Base64 decoding can throw to just the InvalidParameter
@@ -230,10 +242,11 @@ TSIGKeyRing::remove(const Name& key_name) {
 }
 
 TSIGKeyRing::FindResult
-TSIGKeyRing::find(const Name& key_name) {
+TSIGKeyRing::find(const Name& key_name, const Name& algorithm_name) const {
     TSIGKeyRingImpl::TSIGKeyMap::const_iterator found =
         impl_->keys.find(key_name);
-    if (found == impl_->keys.end()) {
+    if (found == impl_->keys.end() ||
+        (*found).second.getAlgorithmName() != algorithm_name) {
         return (FindResult(NOTFOUND, NULL));
     }
     return (FindResult(SUCCESS, &((*found).second)));

src/lib/dns/tsigkey.h

@@ -68,12 +68,30 @@ public:
     //@{
     /// \brief Constructor from key parameters
     ///
-    /// In the current implementation, \c algorithm_name must be a known
-    /// algorithm to this implementation, which are defined via the
-    /// <code>static const</code> member functions.  For other names
-    /// an exception of class \c InvalidParameter will be thrown.
-    /// Note: This restriction may be too strict, and we may revisit it
-    /// later.
+    /// \c algorithm_name should generally be a known algorithm to this
+    /// implementation, which are defined via the
+    /// <code>static const</code> member functions.
+    ///
+    /// Other names are still accepted as long as the secret is empty
+    /// (\c secret is \c NULL and \c secret_len is 0), however; in some cases
+    /// we might want to treat just the pair of key name and algorithm name
+    /// opaquely, e.g., when generating a response TSIG with a BADKEY error
+    /// because the algorithm is unknown as specified in Section 3.2 of
+    /// RFC2845 (in which case the algorithm name would be copied from the
+    /// request to the response, and for that purpose it would be convenient
+    /// if a \c TSIGKey object can hold a name for an "unknown" algorithm).
+    ///
+    /// \note RFC2845 does not specify which algorithm name should be used
+    /// in such a BADKEY response.  The behavior of using the same algorithm
+    /// is derived from the BIND 9 implementation.
+    ///
+    /// It is unlikely that a TSIG key with an unknown algorithm is of any
+    /// use with actual crypto operation, so care must be taken when dealing
+    /// with such keys. (The restriction for the secret will prevent
+    /// accidental creation of such a dangerous key, e.g., due to misspelling
+    /// in a configuration file).
+    /// If the given algorithm name is unknown and non empty secret is
+    /// specified, an exception of type \c InvalidParameter will be thrown.
     ///
     /// \c secret and \c secret_len must be consistent in that the latter
     /// is 0 if and only if the former is \c NULL;
@@ -98,9 +116,12 @@ public:
     /// <name>:<secret>[:<algorithm>]
     /// Where <name> is a domain name for the key, <secret> is a
     /// base64 representation of the key secret, and the optional
-    /// algorithm is an algorithm identifier as specified in RFC4635
+    /// algorithm is an algorithm identifier as specified in RFC4635.
     /// The default algorithm is hmac-md5.sig-alg.reg.int.
     ///
+    /// The same restriction about the algorithm name (and secret) as that
+    /// for the other constructor applies.
+    ///
     /// Since ':' is used as a separator here, it is not possible to
     /// use this constructor to create keys with a ':' character in
     /// their name.
@@ -304,7 +325,9 @@ public:
     /// Find a \c TSIGKey for the given name in the \c TSIGKeyRing.
     ///
     /// It searches the internal storage for a \c TSIGKey whose name is
-    /// \c key_name, and returns the result in the form of a \c FindResult
+    /// \c key_name and that uses the hash algorithm identified by
+    /// \c algorithm_name.
+    /// It returns the result in the form of a \c FindResult
     /// object as follows:
     /// - \c code: \c SUCCESS if a key is found; otherwise \c NOTFOUND.
     /// - \c key: A pointer to the found \c TSIGKey object if one is found;
@@ -318,8 +341,9 @@ public:
     /// This method never throws an exception.
     ///
     /// \param key_name The name of the key to be found.
+    /// \param algorithm_name The name of the algorithm of the found key.
     /// \return A \c FindResult object enclosing the search result (see above).
-    FindResult find(const Name& key_name);
+    FindResult find(const Name& key_name, const Name& algorithm_name) const;
 private:
     struct TSIGKeyRingImpl;
     TSIGKeyRingImpl* impl_;

src/lib/dns/tsigrecord.cc

@@ -70,14 +70,16 @@ castToTSIGRdata(const rdata::Rdata& rdata) {
 }
 
 TSIGRecord::TSIGRecord(const Name& name, const RRClass& rrclass,
-                       const RRTTL&, // we ignore TTL
-                       const rdata::Rdata& rdata,
+                       const RRTTL& 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);
     }
+    if (ttl != RRTTL(TSIG_TTL)) {
+        isc_throw(DNSMessageFORMERR, "Unexpected TSIG TTL: " << ttl);
+    }
 }
 
 const RRClass&

src/lib/dns/tsigrecord.h

@@ -83,15 +83,39 @@ public:
     /// TSIG).
     ///
     /// According to RFC2845, a TSIG RR uses fixed RR class (ANY) and TTL (0).
-    /// If the RR class is different from the expected one, this
+    /// If the RR class or TTL is different from the expected one, this
     /// implementation considers it an invalid record and throws an exception
-    /// of class \c DNSMessageFORMERR.  On the other hand, the TTL is simply
-    /// ignored (in that sense we could even omit that parameter, but it's
-    /// still included if and when we want to change the policy).  RFC2845
-    /// is silent about what the receiver should do if it sees an unexpected
-    /// RR class or TTL in a TSIG RR.  This implementation simply follows what
-    /// BIND 9 does (it is not clear why BIND 9 employs the "inconsistent"
-    /// policy).
+    /// of class \c DNSMessageFORMERR.
+    ///
+    /// \note This behavior is not specified in the protocol specification,
+    /// but this implementation rejects unexpected values for the following
+    /// reasons (but in any case, this won't matter much in practice as
+    /// RFC2848 clearly states these fields always have the fixed values and
+    /// any sane implementation of TSIG signer will follow that):
+    /// According to the RFC editor (in a private communication), the intended
+    /// use of the TSIG TTL field is to signal protocol extensions (currently
+    /// no such extension is defined), so this field may actually be
+    /// validly non 0 in future.  However, until the implementation supports
+    /// that extension it may not always be able to handle the extended
+    /// TSIG as intended; the extension may even affect digest computation.
+    /// There's a related issue on this point.  Different implementations
+    /// interpret the RFC in different ways on the received TTL when
+    /// digesting the message: BIND 9 uses the received value (even if
+    /// it's not 0) as part of the TSIG variables; NLnet Labs' LDNS and NSD
+    /// always use a fixed constant of 0 regardless of the received TTL value.
+    /// This means if and when an extension with non 0 TTL is introduced
+    /// there will be interoperability problems in the form of verification
+    /// failure.  By explicitly rejecting it (and subsequently returning
+    /// a response with a format error) we can indicate the source of the
+    /// problem more clearly than a "bad signature" TSIG error, which can
+    /// happen for various reasons.  On the other hand, rejecting unexpected
+    /// RR classes is mostly for consistency; the RFC lists these two fields
+    /// in the same way, so it makes more sense to handle them equally.
+    /// (BIND 9 rejects unexpected RR classes for TSIG, but that is part of
+    /// general check on RR classes on received RRs; it generally requests
+    /// all classes are the same, and if the protocol specifies the use of
+    /// a particular class for a particular type of RR, it requests that
+    /// class be used).
     ///
     /// Likewise, if \c rdata is not of type \c any::TSIG, an exception of
     /// class DNSMessageFORMERR will be thrown.  When the caller is a
@@ -142,8 +166,7 @@ public:
     /// \param name The owner name of the TSIG RR
     /// \param rrclass The RR class of the RR.  Must be \c RRClass::ANY()
     /// (see above)
-    /// \param ttl The TTL of the RR.  Expected to be a zero TTL, but
-    /// actually ignored in this implementation.
+    /// \param ttl The TTL of the RR.  Must be 0 (see above)
     /// \param rdata The RDATA of the RR.  Must be of type \c any::TSIG.
     /// \param length The size of the RR (see above)
     TSIGRecord(const Name& name, const RRClass& rrclass, const RRTTL& ttl,