BIND 10 master, updated. 7ca65cb9ec528118f370142d7e7b792fcc31c9cf Merge #1357
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri Sep 21 10:54:58 UTC 2012
The branch, master has been updated
via 7ca65cb9ec528118f370142d7e7b792fcc31c9cf (commit)
via e9c984fb457ca225f77f8ce466e129670109169c (commit)
via 5c5f0831bee890e2245339d6f29b03ff33f94f9f (commit)
via 7feae4486ab8b722cdf897bcf629391e7c15fcf1 (commit)
via c0d2f860c8125033c4433b801559b363702f231d (commit)
via f1f86f353832e37b7987756bcba5071a5ef7101e (commit)
via e8a8a442cba80d9c9f2c0a7df1db981ccd195791 (commit)
via 803c67a839d109dea0b040ab2471a27ad995b419 (commit)
via 7028610344ad69d3e5da57575da3e2e85b257d0e (commit)
via 6f8add1167cb4781cc2f84d8c65ee5db73846ab8 (commit)
via c3c1915586223bb1db2114ad5b2bd2189f632517 (commit)
via 1f5426f7eddcaea4742fe9537fa957534b254fe6 (commit)
via 8bed689cafee235c4204c3ddeb980251bf6a9898 (commit)
via dbe150944902bc1de9f1aae199e7d55866d96d29 (commit)
via 40e0500ef5b5b39e698bf7c0ab6f8872cfdf1d07 (commit)
from 4ff65533b470925688c8c5b35af7afcacbf0cdf6 (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 7ca65cb9ec528118f370142d7e7b792fcc31c9cf
Merge: 4ff6553 e9c984f
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Sep 21 12:39:48 2012 +0200
Merge #1357
-----------------------------------------------------------------------
Summary of changes:
src/bin/xfrin/tests/xfrin_test.py | 48 ++++-
src/bin/xfrin/xfrin.py.in | 25 ++-
src/lib/dns/message.cc | 9 +
src/lib/dns/message.h | 8 +
src/lib/dns/python/tests/message_python_test.py | 14 +-
src/lib/dns/python/tests/tsig_python_test.py | 17 +-
src/lib/dns/python/tsig_python.cc | 24 +++
src/lib/dns/tests/message_unittest.cc | 4 +-
src/lib/dns/tests/tsig_unittest.cc | 218 +++++++++++++++++++----
src/lib/dns/tsig.cc | 58 +++++-
src/lib/dns/tsig.h | 27 ++-
src/lib/python/isc/testutils/tsigctx_mock.py | 3 +
12 files changed, 391 insertions(+), 64 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py
index a3818a6..7b9100e 100644
--- a/src/bin/xfrin/tests/xfrin_test.py
+++ b/src/bin/xfrin/tests/xfrin_test.py
@@ -564,6 +564,28 @@ class TestXfrinIXFRAdd(TestXfrinState):
self.assertEqual(type(XfrinIXFRDeleteSOA()),
type(self.conn.get_xfrstate()))
+ def test_handle_new_delete_missing_sig(self):
+ self.conn._end_serial = isc.dns.Serial(1234)
+ # SOA RR whose serial is the current one means we are going to a new
+ # difference, starting with removing that SOA.
+ self.conn._diff.add_data(self.ns_rrset) # put some dummy change
+ self.conn._tsig_ctx = MockTSIGContext(TSIG_KEY)
+ self.conn._tsig_ctx.last_has_signature = lambda: False
+ # First, push a starting SOA inside. This should be OK, nothing checked
+ # yet.
+ self.state.handle_rr(self.conn, self.begin_soa)
+ end_soa_rdata = Rdata(RRType.SOA(), TEST_RRCLASS,
+ 'm. r. 1234 0 0 0 0')
+ end_soa_rrset = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(),
+ RRTTL(3600))
+ end_soa_rrset.add_rdata(end_soa_rdata)
+ # This would try to finish up. But the TSIG pretends not everything is
+ # signed, rejecting it.
+ self.assertRaises(xfrin.XfrinProtocolError, self.state.handle_rr,
+ self.conn, end_soa_rrset)
+ # No diffs were commited
+ self.assertEqual([], self.conn._datasrc_client.committed_diffs)
+
def test_handle_out_of_sync(self):
# getting SOA with an inconsistent serial. This is an error.
self.conn._end_serial = isc.dns.Serial(1235)
@@ -792,12 +814,14 @@ class TestAXFR(TestXfrinConnection):
def tearDown(self):
time.time = self.orig_time_time
- def __create_mock_tsig(self, key, error):
+ def __create_mock_tsig(self, key, error, has_last_signature=True):
# This helper function creates a MockTSIGContext for a given key
# and TSIG error to be used as a result of verify (normally faked
# one)
mock_ctx = MockTSIGContext(key)
mock_ctx.error = error
+ if not has_last_signature:
+ mock_ctx.last_has_signature = lambda: False
return mock_ctx
def __match_exception(self, expected_exception, expected_msg, expression):
@@ -1379,6 +1403,16 @@ class TestAXFR(TestXfrinConnection):
self.assertEqual(self.conn.do_xfrin(False), XFRIN_FAIL)
self.assertEqual(1, self.conn._tsig_ctx.verify_called)
+ def test_do_xfrin_without_last_tsig(self):
+ # TSIG verify will succeed, but it will pretend the last message is
+ # not signed.
+ self.conn._tsig_key = TSIG_KEY
+ self.conn._tsig_ctx_creator = \
+ lambda key: self.__create_mock_tsig(key, TSIGError.NOERROR, False)
+ self.conn.response_generator = self._create_normal_response_data
+ self.assertEqual(self.conn.do_xfrin(False), XFRIN_FAIL)
+ self.assertEqual(2, self.conn._tsig_ctx.verify_called)
+
def test_do_xfrin_with_tsig_fail_for_second_message(self):
# Similar to the previous test, but first verify succeeds. There
# should be a second verify attempt, which will fail, which should
@@ -1553,16 +1587,18 @@ class TestIXFRResponse(TestXfrinConnection):
self.conn._handle_xfrin_responses()
self.assertEqual(type(XfrinIXFREnd()), type(self.conn.get_xfrstate()))
self.assertEqual([], self.conn._datasrc_client.diffs)
+ # Everything is committed as one bunch, currently we commit at the very
+ # end.
check_diffs(self.assertEqual,
[[('delete', begin_soa_rrset),
('delete', self._create_a('192.0.2.1')),
('add', self._create_soa('1231')),
- ('add', self._create_a('192.0.2.2'))],
- [('delete', self._create_soa('1231')),
+ ('add', self._create_a('192.0.2.2')),
+ ('delete', self._create_soa('1231')),
('delete', self._create_a('192.0.2.3')),
('add', self._create_soa('1232')),
- ('add', self._create_a('192.0.2.4'))],
- [('delete', self._create_soa('1232')),
+ ('add', self._create_a('192.0.2.4')),
+ ('delete', self._create_soa('1232')),
('delete', self._create_a('192.0.2.5')),
('add', soa_rrset),
('add', self._create_a('192.0.2.6'))]],
@@ -2924,7 +2960,7 @@ class TestFormatting(unittest.TestCase):
self.assertEqual("example.org/IN",
format_zone_str(isc.dns.Name("example.org"),
isc.dns.RRClass("IN")))
-
+
def test_format_addrinfo(self):
# This test may need to be updated if the input type is changed,
# right now it is a nested tuple:
diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in
index b4a4cf9..2b65311 100755
--- a/src/bin/xfrin/xfrin.py.in
+++ b/src/bin/xfrin/xfrin.py.in
@@ -362,6 +362,7 @@ class XfrinFirstData(XfrinState):
conn._request_serial == get_soa_serial(rr.get_rdata()[0]):
logger.debug(DBG_XFRIN_TRACE, XFRIN_GOT_INCREMENTAL_RESP,
conn.zone_str())
+ conn._diff = None # Will be created on-demand
self.set_xfrstate(conn, XfrinIXFRDeleteSOA())
else:
logger.debug(DBG_XFRIN_TRACE, XFRIN_GOT_NONINCREMENTAL_RESP,
@@ -380,11 +381,13 @@ class XfrinIXFRDeleteSOA(XfrinState):
raise XfrinException(rr.get_type().to_text() +
' RR is given in IXFRDeleteSOA state')
# This is the beginning state of one difference sequence (changes
- # for one SOA update). We need to create a new Diff object now.
+ # for one SOA update). We may need to create a new Diff object now.
# Note also that we (unconditionally) enable journaling here. The
# Diff constructor may internally disable it, however, if the
# underlying data source doesn't support journaling.
- conn._diff = Diff(conn._datasrc_client, conn._zone_name, False, True)
+ if conn._diff is None:
+ conn._diff = Diff(conn._datasrc_client, conn._zone_name, False,
+ True)
conn._diff.delete_data(rr)
self.set_xfrstate(conn, XfrinIXFRDelete())
conn.get_transfer_stats().ixfr_deletion_count += 1
@@ -420,6 +423,9 @@ class XfrinIXFRAdd(XfrinState):
conn.get_transfer_stats().ixfr_changeset_count += 1
soa_serial = get_soa_serial(rr.get_rdata()[0])
if soa_serial == conn._end_serial:
+ # The final part is there. Check all was signed
+ # and commit it to the database.
+ conn._check_response_tsig_last()
conn._diff.commit()
self.set_xfrstate(conn, XfrinIXFREnd())
return True
@@ -429,7 +435,10 @@ class XfrinIXFRAdd(XfrinState):
str(conn._current_serial) +
', got ' + str(soa_serial))
else:
- conn._diff.commit()
+ # Apply a change to the database. But don't commit it yet,
+ # we can't know if the message is/will be properly signed.
+ # A complete commit will happen after the last bit.
+ conn._diff.apply()
self.set_xfrstate(conn, XfrinIXFRDeleteSOA())
return False
conn._diff.add_data(rr)
@@ -494,6 +503,7 @@ class XfrinAXFREnd(XfrinState):
indicating there will be no more message to receive.
"""
+ conn._check_response_tsig_last()
conn._diff.commit()
return False
@@ -782,6 +792,15 @@ class XfrinConnection(asyncore.dispatcher):
# strict.
raise XfrinProtocolError('Unexpected TSIG in response')
+ def _check_response_tsig_last(self):
+ """
+ Check there's a signature at the last message.
+ """
+ if self._tsig_ctx is not None:
+ if not self._tsig_ctx.last_has_signature():
+ raise XfrinProtocolError('TSIG verify fail: no TSIG on last '+
+ 'message')
+
def __parse_soa_response(self, msg, response_data):
'''Parse a response to SOA query and extract the SOA from answer.
diff --git a/src/lib/dns/message.cc b/src/lib/dns/message.cc
index 1cb1ea8..dba0e7b 100644
--- a/src/lib/dns/message.cc
+++ b/src/lib/dns/message.cc
@@ -254,6 +254,13 @@ MessageImpl::toWire(AbstractMessageRenderer& renderer, TSIGContext* tsig_ctx) {
const size_t orig_msg_len_limit = renderer.getLengthLimit();
const AbstractMessageRenderer::CompressMode orig_compress_mode =
renderer.getCompressMode();
+
+ // We are going to skip soon, so we need to clear the renderer
+ // But we'll leave the length limit and the compress mode intact
+ // (or shortened in case of TSIG)
+ renderer.clear();
+ renderer.setCompressMode(orig_compress_mode);
+
if (tsig_len > 0) {
if (tsig_len > orig_msg_len_limit) {
isc_throw(InvalidParameter, "Failed to render DNS message: "
@@ -261,6 +268,8 @@ MessageImpl::toWire(AbstractMessageRenderer& renderer, TSIGContext* tsig_ctx) {
orig_msg_len_limit << ")");
}
renderer.setLengthLimit(orig_msg_len_limit - tsig_len);
+ } else {
+ renderer.setLengthLimit(orig_msg_len_limit);
}
// reserve room for the header
diff --git a/src/lib/dns/message.h b/src/lib/dns/message.h
index 3b80357..85754ac 100644
--- a/src/lib/dns/message.h
+++ b/src/lib/dns/message.h
@@ -556,6 +556,10 @@ public:
/// \c Rcode must have been set beforehand; otherwise, an exception of
/// class \c InvalidMessageOperation will be thrown.
///
+ /// \note The renderer's internal buffers and data are automatically
+ /// cleared, keeping the length limit and the compression mode intact.
+ /// In case truncation is triggered, the renderer is cleared completely.
+ ///
/// \param renderer DNS message rendering context that encapsulates the
/// output buffer and name compression information.
void toWire(AbstractMessageRenderer& renderer);
@@ -581,6 +585,10 @@ public:
/// it should mean a bug either in the TSIG context or in the renderer
/// implementation.
///
+ /// \note The renderer's internal buffers and data are automatically
+ /// cleared, keeping the length limit and the compression mode intact.
+ /// In case truncation is triggered, the renderer is cleared completely.
+ ///
/// \param renderer See the other version
/// \param tsig_ctx A TSIG context that is to be used for signing the
/// message
diff --git a/src/lib/dns/python/tests/message_python_test.py b/src/lib/dns/python/tests/message_python_test.py
index 6f32b11..b9c0d5c 100644
--- a/src/lib/dns/python/tests/message_python_test.py
+++ b/src/lib/dns/python/tests/message_python_test.py
@@ -453,7 +453,7 @@ class MessageTest(unittest.TestCase):
def test_to_text(self):
message_render = create_message()
-
+
msg_str =\
""";; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 4149
;; flags: qr aa rd; QUERY: 1, ANSWER: 2, AUTHORITY: 0, ADDITIONAL: 0
@@ -484,7 +484,7 @@ test.example.com. 3600 IN A 192.0.2.2
Message.from_wire, self.p, bytes())
test_name = Name("test.example.com");
-
+
message_parse = Message(0)
factoryFromFile(message_parse, "message_fromWire1")
self.assertEqual(0x1035, message_parse.get_qid())
@@ -493,7 +493,7 @@ test.example.com. 3600 IN A 192.0.2.2
self.assertTrue(message_parse.get_header_flag(Message.HEADERFLAG_QR))
self.assertTrue(message_parse.get_header_flag(Message.HEADERFLAG_RD))
self.assertTrue(message_parse.get_header_flag(Message.HEADERFLAG_AA))
-
+
#QuestionPtr q = *message_parse.beginQuestion()
q = message_parse.get_question()[0]
self.assertEqual(test_name, q.get_name())
@@ -503,7 +503,7 @@ test.example.com. 3600 IN A 192.0.2.2
self.assertEqual(2, message_parse.get_rr_count(Message.SECTION_ANSWER))
self.assertEqual(0, message_parse.get_rr_count(Message.SECTION_AUTHORITY))
self.assertEqual(0, message_parse.get_rr_count(Message.SECTION_ADDITIONAL))
-
+
#RRsetPtr rrset = *message_parse.beginSection(Message.SECTION_ANSWER)
rrset = message_parse.get_section(Message.SECTION_ANSWER)[0]
self.assertEqual(test_name, rrset.get_name())
@@ -569,12 +569,12 @@ test.example.com. 3600 IN A 192.0.2.2
message_parse = Message(Message.PARSE)
factoryFromFile(message_parse, "message_fromWire10.wire")
self.assertEqual(Rcode.BADVERS(), message_parse.get_rcode())
-
+
# Maximum extended Rcode
message_parse.clear(Message.PARSE)
factoryFromFile(message_parse, "message_fromWire11.wire")
self.assertEqual(0xfff, message_parse.get_rcode().get_code())
-
+
def test_BadEDNS0(self):
message_parse = Message(Message.PARSE)
# OPT RR in the answer section
@@ -596,7 +596,7 @@ test.example.com. 3600 IN A 192.0.2.2
factoryFromFile,
message_parse,
"message_fromWire6")
-
+
# Compressed owner name of OPT RR points to a root name.
# Not necessarily bogus, but very unusual and mostly pathological.
# We accept it, but is it okay?
diff --git a/src/lib/dns/python/tests/tsig_python_test.py b/src/lib/dns/python/tests/tsig_python_test.py
index 7e5515d..4d99175 100644
--- a/src/lib/dns/python/tests/tsig_python_test.py
+++ b/src/lib/dns/python/tests/tsig_python_test.py
@@ -122,15 +122,23 @@ class TSIGContextTest(unittest.TestCase):
# And there should be no error code.
self.assertEqual(TSIGError(Rcode.NOERROR()), self.tsig_ctx.get_error())
+ # No message signed yet
+ self.assertRaises(TSIGContextError, self.tsig_ctx.last_had_signature)
+
# Note: intentionally use camelCase so that we can easily copy-paste
# corresponding C++ tests.
def commonVerifyChecks(self, ctx, record, data, expected_error,
expected_new_state=\
- TSIGContext.STATE_VERIFIED_RESPONSE):
+ TSIGContext.STATE_VERIFIED_RESPONSE,
+ last_should_throw=False):
self.assertEqual(expected_error, ctx.verify(record, data))
self.assertEqual(expected_error, ctx.get_error())
self.assertEqual(expected_new_state, ctx.get_state())
-
+ if last_should_throw:
+ self.assertRaises(TSIGContextError, ctx.last_had_signature)
+ else:
+ self.assertEqual(record is not None,
+ ctx.last_had_signature())
def test_from_keyring(self):
# Construct a TSIG context with an empty key ring. Key shouldn't be
# found, and the BAD_KEY error should be recorded.
@@ -354,7 +362,7 @@ class TSIGContextTest(unittest.TestCase):
tsig = self.createMessageAndSign(self.qid, self.test_name,
self.tsig_ctx, 0, RRType.SOA())
-
+
fix_current_time(0x4da8b9d6 + 301)
self.assertEqual(TSIGError.BAD_TIME,
self.tsig_verify_ctx.verify(tsig, DUMMY_DATA))
@@ -454,7 +462,8 @@ class TSIGContextTest(unittest.TestCase):
self.createMessageAndSign(self.qid, self.test_name, self.tsig_ctx)
self.commonVerifyChecks(self.tsig_ctx, None, DUMMY_DATA,
- TSIGError.FORMERR, TSIGContext.STATE_SENT_REQUEST)
+ TSIGError.FORMERR, TSIGContext.STATE_SENT_REQUEST,
+ True)
self.createMessageFromFile("tsig_verify5.wire")
self.commonVerifyChecks(self.tsig_ctx, self.message.get_tsig_record(),
diff --git a/src/lib/dns/python/tsig_python.cc b/src/lib/dns/python/tsig_python.cc
index 0764e33..abb7733 100644
--- a/src/lib/dns/python/tsig_python.cc
+++ b/src/lib/dns/python/tsig_python.cc
@@ -66,6 +66,7 @@ PyObject* TSIGContext_getState(s_TSIGContext* self);
PyObject* TSIGContext_getError(s_TSIGContext* self);
PyObject* TSIGContext_sign(s_TSIGContext* self, PyObject* args);
PyObject* TSIGContext_verify(s_TSIGContext* self, PyObject* args);
+PyObject* TSIGContext_lastHadSignature(s_TSIGContext* self);
// These are the functions we export
// For a minimal support, we don't need them.
@@ -89,6 +90,9 @@ PyMethodDef TSIGContext_methods[] = {
{ "verify",
reinterpret_cast<PyCFunction>(TSIGContext_verify), METH_VARARGS,
"Verify a DNS message." },
+ { "last_had_signature",
+ reinterpret_cast<PyCFunction>(TSIGContext_lastHadSignature), METH_NOARGS,
+ "Return True if the last verified message contained a signature" },
{ NULL, NULL, 0, NULL }
};
@@ -234,6 +238,26 @@ TSIGContext_verify(s_TSIGContext* self, PyObject* args) {
return (NULL);
}
+
+PyObject*
+TSIGContext_lastHadSignature(s_TSIGContext* self) {
+ try {
+ long result = self->cppobj->lastHadSignature();
+ return (PyBool_FromLong(result));
+ } catch (const TSIGContextError& ex) {
+ PyErr_SetString(po_TSIGContextError, ex.what());
+ } catch (const exception& ex) {
+ const string ex_what =
+ "Unexpected failure in TSIG lastHadSignature: " +
+ string(ex.what());
+ PyErr_SetString(po_IscException, ex_what.c_str());
+ } catch (...) {
+ PyErr_SetString(PyExc_SystemError,
+ "Unexpected failure in TSIG lastHadSignature");
+ }
+
+ return (NULL);
+}
} // end of unnamed namespace
namespace isc {
diff --git a/src/lib/dns/tests/message_unittest.cc b/src/lib/dns/tests/message_unittest.cc
index 33e677f..835aa48 100644
--- a/src/lib/dns/tests/message_unittest.cc
+++ b/src/lib/dns/tests/message_unittest.cc
@@ -534,7 +534,7 @@ TEST_F(MessageTest, appendSection) {
RRClass::IN(), RRType::A()));
EXPECT_TRUE(target.hasRRset(Message::SECTION_ANSWER, test_name,
RRClass::IN(), RRType::AAAA()));
-
+
}
TEST_F(MessageTest, parseHeader) {
@@ -1091,7 +1091,7 @@ TEST_F(MessageTest, toWireWithoutRcode) {
TEST_F(MessageTest, toText) {
// Check toText() output for a typical DNS response with records in
// all sections
-
+
factoryFromFile(message_parse, "message_toText1.wire");
{
SCOPED_TRACE("Message toText test (basic case)");
diff --git a/src/lib/dns/tests/tsig_unittest.cc b/src/lib/dns/tests/tsig_unittest.cc
index ac503e5..458a6e0 100644
--- a/src/lib/dns/tests/tsig_unittest.cc
+++ b/src/lib/dns/tests/tsig_unittest.cc
@@ -66,6 +66,22 @@ testGetTime() {
return (NOW);
}
+// Thin wrapper around TSIGContext to allow access to the
+// update method.
+class TestTSIGContext : public TSIGContext {
+public:
+ TestTSIGContext(const TSIGKey& key) :
+ TSIGContext(key)
+ {}
+ TestTSIGContext(const Name& key_name, const Name& algorithm_name,
+ const TSIGKeyRing& keyring) :
+ TSIGContext(key_name, algorithm_name, keyring)
+ {}
+ void update(const void* const data, size_t len) {
+ TSIGContext::update(data, len);
+ }
+};
+
class TSIGTest : public ::testing::Test {
protected:
TSIGTest() :
@@ -83,9 +99,10 @@ protected:
isc::util::detail::gettimeFunction = NULL;
decodeBase64("SFuWd/q99SzF8Yzd1QbB9g==", secret);
- tsig_ctx.reset(new TSIGContext(TSIGKey(test_name,
- TSIGKey::HMACMD5_NAME(),
- &secret[0], secret.size())));
+ tsig_ctx.reset(new TestTSIGContext(TSIGKey(test_name,
+ TSIGKey::HMACMD5_NAME(),
+ &secret[0],
+ secret.size())));
tsig_verify_ctx.reset(new TSIGContext(TSIGKey(test_name,
TSIGKey::HMACMD5_NAME(),
&secret[0],
@@ -116,7 +133,7 @@ protected:
static const unsigned int AA_FLAG = 0x2;
static const unsigned int RD_FLAG = 0x4;
- boost::scoped_ptr<TSIGContext> tsig_ctx;
+ boost::scoped_ptr<TestTSIGContext> tsig_ctx;
boost::scoped_ptr<TSIGContext> tsig_verify_ctx;
TSIGKeyRing keyring;
const uint16_t qid;
@@ -166,16 +183,20 @@ TSIGTest::createMessageAndSign(uint16_t id, const Name& qname,
message.addRRset(Message::SECTION_ANSWER, answer_rrset);
}
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());
+
+ message.toWire(renderer, *ctx);
+
+ message.clear(Message::PARSE);
+ InputBuffer buffer(renderer.getData(), renderer.getLength());
+ message.fromWire(buffer);
+
EXPECT_EQ(expected_new_state, ctx->getState());
- return (tsig);
+ return (ConstTSIGRecordPtr(new TSIGRecord(*message.getTSIGRecord())));
}
void
@@ -218,11 +239,17 @@ 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)
+ TSIGContext::VERIFIED_RESPONSE,
+ bool last_should_throw = false)
{
EXPECT_EQ(expected_error, ctx.verify(record, data, data_len));
EXPECT_EQ(expected_error, ctx.getError());
EXPECT_EQ(expected_new_state, ctx.getState());
+ if (last_should_throw) {
+ EXPECT_THROW(ctx.lastHadSignature(), TSIGContextError);
+ } else {
+ EXPECT_EQ(record != NULL, ctx.lastHadSignature());
+ }
}
TEST_F(TSIGTest, initialState) {
@@ -231,6 +258,9 @@ TEST_F(TSIGTest, initialState) {
// And there should be no error code.
EXPECT_EQ(TSIGError(Rcode::NOERROR()), tsig_ctx->getError());
+
+ // Nothing verified yet
+ EXPECT_THROW(tsig_ctx->lastHadSignature(), TSIGContextError);
}
TEST_F(TSIGTest, constructFromKeyRing) {
@@ -354,10 +384,17 @@ TEST_F(TSIGTest, verifyBadData) {
12 + dummy_record.getLength() - 1),
InvalidParameter);
+ // Still nothing verified
+ EXPECT_THROW(tsig_ctx->lastHadSignature(), TSIGContextError);
+
// And the data must not be NULL.
EXPECT_THROW(tsig_ctx->verify(&dummy_record, NULL,
12 + dummy_record.getLength()),
InvalidParameter);
+
+ // Still nothing verified
+ EXPECT_THROW(tsig_ctx->lastHadSignature(), TSIGContextError);
+
}
#ifdef ENABLE_CUSTOM_OPERATOR_NEW
@@ -726,8 +763,8 @@ TEST_F(TSIGTest, badsigResponse) {
TEST_F(TSIGTest, badkeyResponse) {
// A similar test as badsigResponse but for BADKEY
isc::util::detail::gettimeFunction = testGetTime<0x4da8877a>;
- tsig_ctx.reset(new TSIGContext(badkey_name, TSIGKey::HMACMD5_NAME(),
- keyring));
+ tsig_ctx.reset(new TestTSIGContext(badkey_name, TSIGKey::HMACMD5_NAME(),
+ keyring));
{
SCOPED_TRACE("Verify resulting in BADKEY");
commonVerifyChecks(*tsig_ctx, &dummy_record, &dummy_data[0],
@@ -806,7 +843,7 @@ TEST_F(TSIGTest, nosigThenValidate) {
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);
+ TSIGContext::SENT_REQUEST, true);
}
createMessageFromFile("tsig_verify5.wire");
@@ -936,45 +973,47 @@ TEST_F(TSIGTest, getTSIGLength) {
EXPECT_EQ(85, tsig_ctx->getTSIGLength());
// hmac-sha1: n2=11, x=20
- tsig_ctx.reset(new TSIGContext(TSIGKey(test_name, TSIGKey::HMACSHA1_NAME(),
- &dummy_data[0], 20)));
+ tsig_ctx.reset(new TestTSIGContext(TSIGKey(test_name,
+ TSIGKey::HMACSHA1_NAME(),
+ &dummy_data[0], 20)));
EXPECT_EQ(74, tsig_ctx->getTSIGLength());
// hmac-sha256: n2=13, x=32
- tsig_ctx.reset(new TSIGContext(TSIGKey(test_name,
- TSIGKey::HMACSHA256_NAME(),
- &dummy_data[0], 32)));
+ tsig_ctx.reset(new TestTSIGContext(TSIGKey(test_name,
+ TSIGKey::HMACSHA256_NAME(),
+ &dummy_data[0], 32)));
EXPECT_EQ(88, tsig_ctx->getTSIGLength());
// hmac-sha224: n2=13, x=28
- tsig_ctx.reset(new TSIGContext(TSIGKey(test_name,
- TSIGKey::HMACSHA224_NAME(),
- &dummy_data[0], 28)));
+ tsig_ctx.reset(new TestTSIGContext(TSIGKey(test_name,
+ TSIGKey::HMACSHA224_NAME(),
+ &dummy_data[0], 28)));
EXPECT_EQ(84, tsig_ctx->getTSIGLength());
// hmac-sha384: n2=13, x=48
- tsig_ctx.reset(new TSIGContext(TSIGKey(test_name,
- TSIGKey::HMACSHA384_NAME(),
- &dummy_data[0], 48)));
+ tsig_ctx.reset(new TestTSIGContext(TSIGKey(test_name,
+ TSIGKey::HMACSHA384_NAME(),
+ &dummy_data[0], 48)));
EXPECT_EQ(104, tsig_ctx->getTSIGLength());
// hmac-sha512: n2=13, x=64
- tsig_ctx.reset(new TSIGContext(TSIGKey(test_name,
- TSIGKey::HMACSHA512_NAME(),
- &dummy_data[0], 64)));
+ tsig_ctx.reset(new TestTSIGContext(TSIGKey(test_name,
+ TSIGKey::HMACSHA512_NAME(),
+ &dummy_data[0], 64)));
EXPECT_EQ(120, tsig_ctx->getTSIGLength());
// bad key case: n1=len(badkey.example.com)=20, n2=26, x=0
- tsig_ctx.reset(new TSIGContext(badkey_name, TSIGKey::HMACMD5_NAME(),
- keyring));
+ tsig_ctx.reset(new TestTSIGContext(badkey_name, TSIGKey::HMACMD5_NAME(),
+ keyring));
EXPECT_EQ(72, tsig_ctx->getTSIGLength());
// bad sig case: n1=17, n2=26, x=0
isc::util::detail::gettimeFunction = testGetTime<0x4da8877a>;
createMessageFromFile("message_toWire2.wire");
- tsig_ctx.reset(new TSIGContext(TSIGKey(test_name, TSIGKey::HMACMD5_NAME(),
- &dummy_data[0],
- dummy_data.size())));
+ tsig_ctx.reset(new TestTSIGContext(TSIGKey(test_name,
+ TSIGKey::HMACMD5_NAME(),
+ &dummy_data[0],
+ dummy_data.size())));
{
SCOPED_TRACE("Verify resulting in BADSIG");
commonVerifyChecks(*tsig_ctx, message.getTSIGRecord(),
@@ -985,9 +1024,10 @@ TEST_F(TSIGTest, getTSIGLength) {
// bad time case: n1=17, n2=26, x=16, y=6
isc::util::detail::gettimeFunction = testGetTime<0x4da8877a - 1000>;
- tsig_ctx.reset(new TSIGContext(TSIGKey(test_name, TSIGKey::HMACMD5_NAME(),
- &dummy_data[0],
- dummy_data.size())));
+ tsig_ctx.reset(new TestTSIGContext(TSIGKey(test_name,
+ TSIGKey::HMACMD5_NAME(),
+ &dummy_data[0],
+ dummy_data.size())));
{
SCOPED_TRACE("Verify resulting in BADTIME");
commonVerifyChecks(*tsig_ctx, message.getTSIGRecord(),
@@ -998,4 +1038,114 @@ TEST_F(TSIGTest, getTSIGLength) {
EXPECT_EQ(91, tsig_ctx->getTSIGLength());
}
+// Verify a stream of multiple messages. Some of them have a signature omitted.
+//
+// We have two contexts, one that signs, another that verifies.
+TEST_F(TSIGTest, verifyMulti) {
+ isc::util::detail::gettimeFunction = testGetTime<0x4da8877a>;
+
+ // First, send query from the verify one to the normal one, so
+ // we initialize something like AXFR
+ {
+ SCOPED_TRACE("Query");
+ ConstTSIGRecordPtr tsig = createMessageAndSign(1234, test_name,
+ tsig_verify_ctx.get());
+ commonVerifyChecks(*tsig_ctx, tsig.get(),
+ renderer.getData(), renderer.getLength(),
+ TSIGError(Rcode::NOERROR()),
+ TSIGContext::RECEIVED_REQUEST);
+ }
+
+ {
+ SCOPED_TRACE("First message");
+ ConstTSIGRecordPtr tsig = createMessageAndSign(1234, test_name,
+ tsig_ctx.get());
+ commonVerifyChecks(*tsig_verify_ctx, tsig.get(),
+ renderer.getData(), renderer.getLength(),
+ TSIGError(Rcode::NOERROR()),
+ TSIGContext::VERIFIED_RESPONSE);
+ EXPECT_TRUE(tsig_verify_ctx->lastHadSignature());
+ }
+
+ {
+ SCOPED_TRACE("Second message");
+ ConstTSIGRecordPtr tsig = createMessageAndSign(1234, test_name,
+ tsig_ctx.get());
+ commonVerifyChecks(*tsig_verify_ctx, tsig.get(),
+ renderer.getData(), renderer.getLength(),
+ TSIGError(Rcode::NOERROR()),
+ TSIGContext::VERIFIED_RESPONSE);
+ EXPECT_TRUE(tsig_verify_ctx->lastHadSignature());
+ }
+
+ {
+ SCOPED_TRACE("Third message. Unsigned.");
+ // Another message does not carry the TSIG on it. But it should
+ // be OK, it's in the middle of stream.
+ message.clear(Message::RENDER);
+ message.setQid(1234);
+ message.setOpcode(Opcode::QUERY());
+ message.setRcode(Rcode::NOERROR());
+ RRsetPtr answer_rrset(new RRset(test_name, test_class, RRType::A(),
+ test_ttl));
+ answer_rrset->addRdata(createRdata(RRType::A(), test_class,
+ "192.0.2.1"));
+ message.addRRset(Message::SECTION_ANSWER, answer_rrset);
+ message.toWire(renderer);
+ // Update the internal state. We abuse the knowledge of
+ // internals here a little bit to generate correct test data
+ tsig_ctx->update(renderer.getData(), renderer.getLength());
+
+ commonVerifyChecks(*tsig_verify_ctx, NULL,
+ renderer.getData(), renderer.getLength(),
+ TSIGError(Rcode::NOERROR()),
+ TSIGContext::VERIFIED_RESPONSE);
+
+ EXPECT_FALSE(tsig_verify_ctx->lastHadSignature());
+ }
+
+ {
+ SCOPED_TRACE("Fourth message. Signed again.");
+ ConstTSIGRecordPtr tsig = createMessageAndSign(1234, test_name,
+ tsig_ctx.get());
+ commonVerifyChecks(*tsig_verify_ctx, tsig.get(),
+ renderer.getData(), renderer.getLength(),
+ TSIGError(Rcode::NOERROR()),
+ TSIGContext::VERIFIED_RESPONSE);
+ EXPECT_TRUE(tsig_verify_ctx->lastHadSignature());
+ }
+
+ {
+ SCOPED_TRACE("Filling in bunch of unsigned messages");
+ for (size_t i = 0; i < 100; ++i) {
+ SCOPED_TRACE(i);
+ // Another message does not carry the TSIG on it. But it should
+ // be OK, it's in the middle of stream.
+ message.clear(Message::RENDER);
+ message.setQid(1234);
+ message.setOpcode(Opcode::QUERY());
+ message.setRcode(Rcode::NOERROR());
+ RRsetPtr answer_rrset(new RRset(test_name, test_class, RRType::A(),
+ test_ttl));
+ answer_rrset->addRdata(createRdata(RRType::A(), test_class,
+ "192.0.2.1"));
+ message.addRRset(Message::SECTION_ANSWER, answer_rrset);
+ message.toWire(renderer);
+ // Update the internal state. We abuse the knowledge of
+ // internals here a little bit to generate correct test data
+ tsig_ctx->update(renderer.getData(), renderer.getLength());
+
+ // 99 unsigned messages is OK. But the 100th must be signed, according
+ // to the RFC2845, section 4.4
+ commonVerifyChecks(*tsig_verify_ctx, NULL,
+ renderer.getData(), renderer.getLength(),
+ i == 99 ? TSIGError::FORMERR() :
+ TSIGError(Rcode::NOERROR()),
+ TSIGContext::VERIFIED_RESPONSE);
+
+ EXPECT_FALSE(tsig_verify_ctx->lastHadSignature());
+ }
+ }
+}
+
} // end namespace
diff --git a/src/lib/dns/tsig.cc b/src/lib/dns/tsig.cc
index 1bda021..d7ffcf8 100644
--- a/src/lib/dns/tsig.cc
+++ b/src/lib/dns/tsig.cc
@@ -61,7 +61,8 @@ struct TSIGContext::TSIGContextImpl {
TSIGContextImpl(const TSIGKey& key,
TSIGError error = TSIGError::NOERROR()) :
state_(INIT), key_(key), error_(error),
- previous_timesigned_(0), digest_len_(0)
+ previous_timesigned_(0), digest_len_(0),
+ last_sig_dist_(-1)
{
if (error == TSIGError::NOERROR()) {
// In normal (NOERROR) case, the key should be valid, and we
@@ -137,7 +138,7 @@ struct TSIGContext::TSIGContextImpl {
// 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 digestPreviousMAC(HMACPtr hmac);
void digestTSIGVariables(HMACPtr hmac, uint16_t rrclass, uint32_t rrttl,
uint64_t time_signed, uint16_t fudge,
uint16_t error, uint16_t otherlen,
@@ -152,14 +153,25 @@ struct TSIGContext::TSIGContextImpl {
uint64_t previous_timesigned_; // only meaningful for response with BADTIME
size_t digest_len_;
HMACPtr hmac_;
+ // This is the distance from the last verified signed message. Value of 0
+ // means the last message was signed. Special value -1 means there was no
+ // signed message yet.
+ int last_sig_dist_;
};
void
-TSIGContext::TSIGContextImpl::digestPreviousMAC(HMACPtr hmac) const {
+TSIGContext::TSIGContextImpl::digestPreviousMAC(HMACPtr hmac) {
// We should have ensured the digest size fits 16 bits within this class
// implementation.
assert(previous_digest_.size() <= 0xffff);
+ if (previous_digest_.empty()) {
+ // The previous digest was already used. We're in the middle of
+ // TCP stream somewhere and we already pushed some unsigned message
+ // into the HMAC state.
+ return;
+ }
+
OutputBuffer buffer(sizeof(uint16_t) + previous_digest_.size());
const uint16_t previous_digest_len(previous_digest_.size());
buffer.writeUint16(previous_digest_len);
@@ -414,11 +426,21 @@ TSIGContext::verify(const TSIGRecord* const record, const void* const data,
"TSIG verify attempt after sending a response");
}
- // 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) {
+ if (impl_->last_sig_dist_ >= 0 && impl_->last_sig_dist_ < 99) {
+ // It is not signed, but in the middle of TCP stream. We just
+ // update the HMAC state and consider this message OK.
+ update(data, data_len);
+ // This one is not signed, the last signed is one message further
+ // now.
+ impl_->last_sig_dist_++;
+ // No digest to return now. Just say it's OK.
+ return (impl_->postVerifyUpdate(TSIGError::NOERROR(), NULL, 0));
+ }
+ // 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).
return (impl_->postVerifyUpdate(TSIGError::FORMERR(), NULL, 0));
}
@@ -433,6 +455,9 @@ TSIGContext::verify(const TSIGRecord* const record, const void* const data,
isc_throw(InvalidParameter, "TSIG verify: empty data is invalid");
}
+ // This message is signed and we won't throw any more.
+ impl_->last_sig_dist_ = 0;
+
// 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.
@@ -520,5 +545,24 @@ TSIGContext::verify(const TSIGRecord* const record, const void* const data,
return (impl_->postVerifyUpdate(TSIGError::BAD_SIG(), NULL, 0));
}
+bool
+TSIGContext::lastHadSignature() const {
+ if (impl_->last_sig_dist_ == -1) {
+ isc_throw(TSIGContextError, "No message was verified yet");
+ }
+ return (impl_->last_sig_dist_ == 0);
+}
+
+void
+TSIGContext::update(const void* const data, size_t len) {
+ HMACPtr hmac(impl_->createHMAC());
+ // Use the previous digest and never use it again
+ impl_->digestPreviousMAC(hmac);
+ impl_->previous_digest_.clear();
+ // Push the message there
+ hmac->update(data, len);
+ impl_->hmac_ = hmac;
+}
+
} // namespace dns
} // namespace isc
diff --git a/src/lib/dns/tsig.h b/src/lib/dns/tsig.h
index 028d295..9ccc580 100644
--- a/src/lib/dns/tsig.h
+++ b/src/lib/dns/tsig.h
@@ -339,7 +339,6 @@ public:
/// 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.
@@ -353,6 +352,19 @@ public:
TSIGError verify(const TSIGRecord* const record, const void* const data,
const size_t data_len);
+ /// \brief Check whether the last verified message was signed.
+ ///
+ /// RFC2845 allows for some of the messages not to be signed. However,
+ /// the last message must be signed and the class has no knowledge if a
+ /// given message is the last one, therefore it can't check directly.
+ ///
+ /// It is up to the caller to check if the last verified message was signed
+ /// after all are verified by calling this function.
+ ///
+ /// \return If the last message was signed or not.
+ /// \exception TSIGContextError if no message was verified yet.
+ bool lastHadSignature() const;
+
/// Return the expected length of TSIG RR after \c sign()
///
/// This method returns the length of the TSIG RR that would be
@@ -401,6 +413,19 @@ public:
static const uint16_t DEFAULT_FUDGE = 300;
//@}
+protected:
+ /// \brief Update internal HMAC state by more data.
+ ///
+ /// This is used mostly internaly, when we need to verify a message without
+ /// TSIG signature in the middle of signed TCP stream. However, it is also
+ /// used in tests, so it's protected instead of private, to allow tests
+ /// in.
+ ///
+ /// It doesn't contain sanity checks, and it is not tested directly. But
+ /// we may want to add these one day to allow generating the skipped TSIG
+ /// messages too. Until then, do not use this method.
+ void update(const void* const data, size_t len);
+
private:
struct TSIGContextImpl;
TSIGContextImpl* impl_;
diff --git a/src/lib/python/isc/testutils/tsigctx_mock.py b/src/lib/python/isc/testutils/tsigctx_mock.py
index a9af9b9..0158987 100644
--- a/src/lib/python/isc/testutils/tsigctx_mock.py
+++ b/src/lib/python/isc/testutils/tsigctx_mock.py
@@ -51,3 +51,6 @@ class MockTSIGContext(TSIGContext):
if hasattr(self.error, '__call__'):
return self.error(self)
return self.error
+
+ def last_has_signature(self):
+ return True
More information about the bind10-changes
mailing list