BIND 10 trac1357, updated. 8bed689cafee235c4204c3ddeb980251bf6a9898 [1357] Check there's signature on the last message in XfrIn
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu Sep 6 13:13:00 UTC 2012
The branch, trac1357 has been updated
via 8bed689cafee235c4204c3ddeb980251bf6a9898 (commit)
from dbe150944902bc1de9f1aae199e7d55866d96d29 (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 8bed689cafee235c4204c3ddeb980251bf6a9898
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Thu Sep 6 15:03:26 2012 +0200
[1357] Check there's signature on the last message in XfrIn
DDNS and Auth don't need it, they have contexts with single message and
the first one is checked.
There's a problem. We commit in the middle of IXFR stream, which is
wrong. What if the last message after that is not signed? Besides,
databases are faster if there's one big commit, not many small ones.
-----------------------------------------------------------------------
Summary of changes:
src/bin/xfrin/tests/xfrin_test.py | 26 +++++++++++++++++++++++++-
src/bin/xfrin/xfrin.py.in | 15 +++++++++++++++
src/lib/python/isc/testutils/tsigctx_mock.py | 3 +++
3 files changed, 43 insertions(+), 1 deletion(-)
-----------------------------------------------------------------------
diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py
index a3818a6..1c3256b 100644
--- a/src/bin/xfrin/tests/xfrin_test.py
+++ b/src/bin/xfrin/tests/xfrin_test.py
@@ -564,6 +564,18 @@ 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
+ # 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, self.begin_soa)
+
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 +804,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 +1393,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
diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in
index b4a4cf9..d26e8aa 100755
--- a/src/bin/xfrin/xfrin.py.in
+++ b/src/bin/xfrin/xfrin.py.in
@@ -420,6 +420,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:
+ # FIXME: It's insecure to do a full commit here, due
+ # to the fact that the message we're working on might not
+ # be signed right now.
conn._diff.commit()
self.set_xfrstate(conn, XfrinIXFREnd())
return True
@@ -429,6 +432,8 @@ class XfrinIXFRAdd(XfrinState):
str(conn._current_serial) +
', got ' + str(soa_serial))
else:
+ raise Exception()
+ conn._check_response_tsig_last()
conn._diff.commit()
self.set_xfrstate(conn, XfrinIXFRDeleteSOA())
return False
@@ -494,6 +499,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 +788,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/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