BIND 10 trac1357, updated. 7feae4486ab8b722cdf897bcf629391e7c15fcf1 [1357] Commit only after verifying the last TSIG

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Sep 20 09:31:40 UTC 2012


The branch, trac1357 has been updated
       via  7feae4486ab8b722cdf897bcf629391e7c15fcf1 (commit)
      from  c0d2f860c8125033c4433b801559b363702f231d (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 7feae4486ab8b722cdf897bcf629391e7c15fcf1
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Thu Sep 20 11:27:57 2012 +0200

    [1357] Commit only after verifying the last TSIG
    
    We can't commit during the stream of changes, they might be not signed
    and that'd be a security risk (if someone stole the TCP connection and
    pushed a change that'd put bogus data inside before getting to the last
    message which would be rejected then).
    
    Fixed the check: it now happens on the last message, not on each message
    except the last. Returning true there does not mean to continue, but
    that the current RR has been handled fully.

-----------------------------------------------------------------------

Summary of changes:
 src/bin/xfrin/tests/xfrin_test.py |   20 +++++++++++++++-----
 src/bin/xfrin/xfrin.py.in         |   12 +++++++-----
 2 files changed, 22 insertions(+), 10 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py
index 4ee36b7..399c7a4 100644
--- a/src/bin/xfrin/tests/xfrin_test.py
+++ b/src/bin/xfrin/tests/xfrin_test.py
@@ -571,10 +571,18 @@ class TestXfrinIXFRAdd(TestXfrinState):
         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, self.begin_soa)
+                          self.conn, end_soa_rrset)
 
     def test_handle_out_of_sync(self):
         # getting SOA with an inconsistent serial.  This is an error.
@@ -1577,16 +1585,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'))]],
diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in
index 2613307..88411ed 100755
--- a/src/bin/xfrin/xfrin.py.in
+++ b/src/bin/xfrin/xfrin.py.in
@@ -420,9 +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.
+                # 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
@@ -432,8 +432,10 @@ class XfrinIXFRAdd(XfrinState):
                                          str(conn._current_serial) +
                                          ', got ' + str(soa_serial))
             else:
-                conn._check_response_tsig_last()
-                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)



More information about the bind10-changes mailing list