BIND 10 #1357: AXFR and AXFR-like IXFR in needs every message signed

[BIND 10 Development]

#1357: AXFR and AXFR-like IXFR in needs every message signed
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jelte
  vorner                             |                Status:  reviewing
                       Type:         |             Milestone:
  defect                             |  Sprint-20120918
                   Priority:         |            Resolution:
  medium                             |             Sensitive:  0
                  Component:  xfrin  |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  5
            Defect Severity:  Low    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jelte


Comment:

 Hello

 Replying to [comment:11 jelte]:
 > - I presume the raise Exception() in xfrin.py[.in] was not meant to stay
 there (e8a8a442cba80d9c9f2c0a7df1db981ccd195791)
 > - Edited doxygen a bit (f1f86f353832e37b7987756bcba5071a5ef7101e)

 Thanks for these.

 > And the third commit is slightly more controversial, I added a
 Message::clear() call to Message::toWire(); if a renderer object gets
 reused it needs to be cleared every time anyway (otherwise it'll jump
 around and write at the wrong locations); We could make it so that it does
 not need the clear, but that would need more involved changes, and in case
 of truncation the renderer is cleared too (so it was a weak contract at
 best anyway). Commit c0d2f860c8125033c4433b801559b363702f231d

 I'm less sure about this. It seems it breaks no tests, so it should be
 safe. But it looks like unrelated side effect to the method. Could there
 be a reason to render two messages to one renderer without clearing?

 Also, I think some tests misused this in some way (comparing rendering
 different RRsets or something, but maybe it was not with messages at all).

 What exact problem does it solve anyway?

 > For the rest it looks ok. Is there a ticket for the FIXME about commits?
 (i think that the intermediate commits were intentional, so all diffs are
 kept but the object is reused, this would have to be reconsidered then,
 and either the whole thing should go in one transaction or we should keep
 tabs of all Diff objects we created, and apply all of them at the end).

 Sorry about the FIXME, I should have mentioned it here. If it was ordinary
 feature, I'd be OK with a follow-up ticket. But this would open potential
 security hole (if someone hijacks a TCP connection with transfer, it would
 be possible for them to transfer some data and have it committed before
 the TSIG on the last message is found to be wrong or missing). So, I'm
 inclined to either do it on this ticket or make sure the follow-up one
 goes to the next sprint, so we don't release with the hole.

 If it is OK to do here, I was thinking about putting everything into one
 big transaction ‒ pushing the changes to the DB, but not committing it
 until the very end. That should be IMO faster as well as more correct
 (commit is the expensive operation, no matter how large or small it is).

 Thank you

-- 
Ticket URL: <https://bind10.isc.org/ticket/1357#comment:13>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development