[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.

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

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.
 

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