BIND 10 master, updated. da57d4a08a2e14e5e6589138692674894f667f54 [master] added a changelog entry for TSIP support in libdns++.
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed May 11 19:18:09 UTC 2011
The branch, master has been updated
via da57d4a08a2e14e5e6589138692674894f667f54 (commit)
via 5aaa6c0f628ed7c2093ecdbac93a2c8cf6c94349 (commit)
via 345b5986ff26015475b5e17506b3ca6cafaea8bd (commit)
via 143b2c6769c64eb55d2f34305ad8e2b7ce681aa6 (commit)
via 4787c281a351b04b9570b8bb2d863db706a9d9c9 (commit)
via 9bd9fa819f30aff560da9412ffa14d87e557ab69 (commit)
via d53bbaf239de6059dd0d8872f84d1a0db6ccb90c (commit)
via 7b0d9b1dc03d9343f3ccfed2fb5876d7d9f048d5 (commit)
via b734c55b73941125af327595ba39252add032791 (commit)
via daab0dcd59baee4b1e56eab640e29c9e8e947e80 (commit)
via 00a15b4b06f9de757b30cd320fd56431980f8458 (commit)
via a09ced080782388e389ee6c669b9d36da65f9dc5 (commit)
via b21c74c08fbdd350bcd01b083fc3cf29a98e5a3a (commit)
via cabd3b127cf5ab2b916c5717992338316496de8b (commit)
via a64f778b468ff7bd95461d1228aec66565602d57 (commit)
via eef87432f746406d467c647dd7302fc4cac8d53f (commit)
via df8ef3fffa7a4186d7ade60b514bb78acbb9b33c (commit)
via 79b087eab39177799e38611661fd86125d9b8a67 (commit)
via efdf126e6e0fe75afa26d4d7cdcce663892a840c (commit)
via 35a7c63a23646994af69a1a6d325ad11b4bc8354 (commit)
via a91fc737dd4c982194392b467b68c6c8383cec55 (commit)
via 34c2d5bbe08e41b0802eb6ca58a8084595064803 (commit)
via e4f2823d8b564c5b2c2f7029e5eb81be6b058d3c (commit)
via 1b967a5870088c3f4c7b5a3b8202e1b2e9572690 (commit)
from 29d36377ec206cbe52274ee9a5a6e88ef27921d5 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit da57d4a08a2e14e5e6589138692674894f667f54
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Wed May 11 12:17:42 2011 -0700
[master] added a changelog entry for TSIP support in libdns++.
commit 5aaa6c0f628ed7c2093ecdbac93a2c8cf6c94349
Merge: 29d3637 345b598
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Wed May 11 11:48:59 2011 -0700
[master] Merge branch 'trac893'
-----------------------------------------------------------------------
Summary of changes:
ChangeLog | 11 +
src/cppcheck-suppress.lst | 2 +-
src/lib/cryptolink/crypto_hmac.cc | 9 +
src/lib/cryptolink/tests/crypto_unittests.cc | 80 ++--
src/lib/dns/python/tests/tsigkey_python_test.py | 31 +-
src/lib/dns/python/tsigkey_python.cc | 6 +-
src/lib/dns/tests/testdata/Makefile.am | 8 +
src/lib/dns/tests/testdata/gen-wiredata.py.in | 13 +-
src/lib/dns/tests/testdata/tsig_verify1.spec | 19 +
.../{message_toWire2.spec => tsig_verify10.spec} | 7 +-
src/lib/dns/tests/testdata/tsig_verify2.spec | 32 ++
src/lib/dns/tests/testdata/tsig_verify3.spec | 26 +
.../{message_fromWire16.spec => tsig_verify4.spec} | 14 +-
.../{message_fromWire16.spec => tsig_verify5.spec} | 13 +-
.../{message_toWire2.spec => tsig_verify6.spec} | 4 +-
.../{message_toWire2.spec => tsig_verify7.spec} | 6 +-
.../{message_toWire2.spec => tsig_verify8.spec} | 8 +-
.../{message_fromWire16.spec => tsig_verify9.spec} | 8 +-
src/lib/dns/tests/tsig_unittest.cc | 534 +++++++++++++++++---
src/lib/dns/tests/tsigerror_unittest.cc | 14 +
src/lib/dns/tests/tsigkey_unittest.cc | 84 ++-
src/lib/dns/tests/tsigrecord_unittest.cc | 9 +-
src/lib/dns/tsig.cc | 380 ++++++++++++---
src/lib/dns/tsig.h | 198 +++++++-
src/lib/dns/tsigerror.cc | 11 +
src/lib/dns/tsigerror.h | 16 +
src/lib/dns/tsigkey.cc | 25 +-
src/lib/dns/tsigkey.h | 42 ++-
src/lib/dns/tsigrecord.cc | 6 +-
src/lib/dns/tsigrecord.h | 43 ++-
30 files changed, 1371 insertions(+), 288 deletions(-)
create mode 100644 src/lib/dns/tests/testdata/tsig_verify1.spec
copy src/lib/dns/tests/testdata/{message_toWire2.spec => tsig_verify10.spec} (70%)
create mode 100644 src/lib/dns/tests/testdata/tsig_verify2.spec
create mode 100644 src/lib/dns/tests/testdata/tsig_verify3.spec
copy src/lib/dns/tests/testdata/{message_fromWire16.spec => tsig_verify4.spec} (51%)
copy src/lib/dns/tests/testdata/{message_fromWire16.spec => tsig_verify5.spec} (55%)
copy src/lib/dns/tests/testdata/{message_toWire2.spec => tsig_verify6.spec} (78%)
copy src/lib/dns/tests/testdata/{message_toWire2.spec => tsig_verify7.spec} (72%)
copy src/lib/dns/tests/testdata/{message_toWire2.spec => tsig_verify8.spec} (70%)
copy src/lib/dns/tests/testdata/{message_fromWire16.spec => tsig_verify9.spec} (56%)
-----------------------------------------------------------------------
diff --git a/ChangeLog b/ChangeLog
index 6c7beda..974ff65 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+235. [func] jinmei
+ libdns++: added support for TSIG signing and verification. It can
+ be done using a newly introduced TSIGContext class.
+ Note: we temporarily disabled support for truncated signature
+ and modified some part of the code introduced in #226 accordingly.
+ We plan to fix this pretty soon.
+ (Trac #812, git ebe0c4b1e66d359227bdd1bd47395fee7b957f14)
+ (Trac #871, git 7c54055c0e47c7a0e36fcfab4b47ff180c0ca8c8)
+ (Trac #813, git ffa2f0672084c1f16e5784cdcdd55822f119feaa)
+ (Trac #893, git 5aaa6c0f628ed7c2093ecdbac93a2c8cf6c94349)
+
234. [func] jerry
src/bin/xfrin: update xfrin to use TSIG. Currently it only supports
sending a signed TSIG request or SOA request.
diff --git a/src/cppcheck-suppress.lst b/src/cppcheck-suppress.lst
index 5e6d81f..4659dc4 100644
--- a/src/cppcheck-suppress.lst
+++ b/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
diff --git a/src/lib/cryptolink/crypto_hmac.cc b/src/lib/cryptolink/crypto_hmac.cc
index 9c35f60..d20c85b 100644
--- a/src/lib/cryptolink/crypto_hmac.cc
+++ b/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();
}
diff --git a/src/lib/cryptolink/tests/crypto_unittests.cc b/src/lib/cryptolink/tests/crypto_unittests.cc
index 65018c6..a1ffaab 100644
--- a/src/lib/cryptolink/tests/crypto_unittests.cc
+++ b/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) {
diff --git a/src/lib/dns/python/tests/tsigkey_python_test.py b/src/lib/dns/python/tests/tsigkey_python_test.py
index 97be501..305c8dd 100644
--- a/src/lib/dns/python/tests/tsigkey_python_test.py
+++ b/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()
diff --git a/src/lib/dns/python/tsigkey_python.cc b/src/lib/dns/python/tsigkey_python.cc
index 906dfc0..4ca7bcd 100644
--- a/src/lib/dns/python/tsigkey_python.cc
+++ b/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) {
diff --git a/src/lib/dns/tests/testdata/Makefile.am b/src/lib/dns/tests/testdata/Makefile.am
index 8a83aae..cb1bb1c 100644
--- a/src/lib/dns/tests/testdata/Makefile.am
+++ b/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 $@ $<
diff --git a/src/lib/dns/tests/testdata/gen-wiredata.py.in b/src/lib/dns/tests/testdata/gen-wiredata.py.in
index 7a82bfd..fd98c6e 100755
--- a/src/lib/dns/tests/testdata/gen-wiredata.py.in
+++ b/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' %
diff --git a/src/lib/dns/tests/testdata/tsig_verify1.spec b/src/lib/dns/tests/testdata/tsig_verify1.spec
new file mode 100644
index 0000000..687013a
--- /dev/null
+++ b/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
diff --git a/src/lib/dns/tests/testdata/tsig_verify10.spec b/src/lib/dns/tests/testdata/tsig_verify10.spec
new file mode 100644
index 0000000..33ce83e
--- /dev/null
+++ b/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
diff --git a/src/lib/dns/tests/testdata/tsig_verify2.spec b/src/lib/dns/tests/testdata/tsig_verify2.spec
new file mode 100644
index 0000000..ff98ca3
--- /dev/null
+++ b/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
diff --git a/src/lib/dns/tests/testdata/tsig_verify3.spec b/src/lib/dns/tests/testdata/tsig_verify3.spec
new file mode 100644
index 0000000..7e2f797
--- /dev/null
+++ b/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
diff --git a/src/lib/dns/tests/testdata/tsig_verify4.spec b/src/lib/dns/tests/testdata/tsig_verify4.spec
new file mode 100644
index 0000000..4ffbbcf
--- /dev/null
+++ b/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
diff --git a/src/lib/dns/tests/testdata/tsig_verify5.spec b/src/lib/dns/tests/testdata/tsig_verify5.spec
new file mode 100644
index 0000000..a6cc643
--- /dev/null
+++ b/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
diff --git a/src/lib/dns/tests/testdata/tsig_verify6.spec b/src/lib/dns/tests/testdata/tsig_verify6.spec
new file mode 100644
index 0000000..32e0818
--- /dev/null
+++ b/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
diff --git a/src/lib/dns/tests/testdata/tsig_verify7.spec b/src/lib/dns/tests/testdata/tsig_verify7.spec
new file mode 100644
index 0000000..377578e
--- /dev/null
+++ b/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
diff --git a/src/lib/dns/tests/testdata/tsig_verify8.spec b/src/lib/dns/tests/testdata/tsig_verify8.spec
new file mode 100644
index 0000000..5432d4a
--- /dev/null
+++ b/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
diff --git a/src/lib/dns/tests/testdata/tsig_verify9.spec b/src/lib/dns/tests/testdata/tsig_verify9.spec
new file mode 100644
index 0000000..5888455
--- /dev/null
+++ b/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
diff --git a/src/lib/dns/tests/tsig_unittest.cc b/src/lib/dns/tests/tsig_unittest.cc
index efa0490..9d996a5 100644
--- a/src/lib/dns/tests/tsig_unittest.cc
+++ b/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);
}
}
diff --git a/src/lib/dns/tests/tsigerror_unittest.cc b/src/lib/dns/tests/tsigerror_unittest.cc
index 5866587..bb08aef 100644
--- a/src/lib/dns/tests/tsigerror_unittest.cc
+++ b/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;
diff --git a/src/lib/dns/tests/tsigkey_unittest.cc b/src/lib/dns/tests/tsigkey_unittest.cc
index 96696d5..3408d45 100644
--- a/src/lib/dns/tests/tsigkey_unittest.cc
+++ b/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);
-
}
diff --git a/src/lib/dns/tests/tsigrecord_unittest.cc b/src/lib/dns/tests/tsigrecord_unittest.cc
index a932b7f..e1b3e93 100644
--- a/src/lib/dns/tests/tsigrecord_unittest.cc
+++ b/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) {
diff --git a/src/lib/dns/tsig.cc b/src/lib/dns/tsig.cc
index 8e8301d..714b2a5 100644
--- a/src/lib/dns/tsig.cc
+++ b/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
diff --git a/src/lib/dns/tsig.h b/src/lib/dns/tsig.h
index fbcb1bb..bceec25 100644
--- a/src/lib/dns/tsig.h
+++ b/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
///
//@{
diff --git a/src/lib/dns/tsigerror.cc b/src/lib/dns/tsigerror.cc
index e63c9ab..36ef47d 100644
--- a/src/lib/dns/tsigerror.cc
+++ b/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());
diff --git a/src/lib/dns/tsigerror.h b/src/lib/dns/tsigerror.h
index 4463daf..9794c41 100644
--- a/src/lib/dns/tsigerror.h
+++ b/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();
diff --git a/src/lib/dns/tsigkey.cc b/src/lib/dns/tsigkey.cc
index c899423..4082fbe 100644
--- a/src/lib/dns/tsigkey.cc
+++ b/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)));
diff --git a/src/lib/dns/tsigkey.h b/src/lib/dns/tsigkey.h
index 02dd423..f0df709 100644
--- a/src/lib/dns/tsigkey.h
+++ b/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_;
diff --git a/src/lib/dns/tsigrecord.cc b/src/lib/dns/tsigrecord.cc
index 40ea6c2..9dd3f78 100644
--- a/src/lib/dns/tsigrecord.cc
+++ b/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&
diff --git a/src/lib/dns/tsigrecord.h b/src/lib/dns/tsigrecord.h
index e42edf1..03de746 100644
--- a/src/lib/dns/tsigrecord.h
+++ b/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,
More information about the bind10-changes
mailing list