BIND 10 #931: Implement signing part in b10-auth

BIND 10 Development do-not-reply at isc.org
Thu May 26 09:34:09 UTC 2011


#931: Implement signing part in b10-auth
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  vorner                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20110531
                  Component:         |            Resolution:
  Unclassified                       |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  0.0
Feature Depending on Ticket:  tsig   |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 Replying to [comment:22 jinmei]:
 > See my previous short response, but I didn't want to delay the process
 > due to this further, so I wrote the test myself (e9975a7).  As noted
 > in the code, this means if the default could ever be non empty list
 > the keyring module would need to extract the default value explicitly.
 > I personally think this is a flaw of the config module interface (it
 > simply doesn't make sense to me to put such a burden to the
 > application; it shouldn't be bothered with what's the default, it
 > should only need to deal with the "currently effective" set of
 > configuration), but that's totally a different topic.

 OK, thanks.

 And I agree with the point about currently effective thing. It might be
 worth a discussion or something.

 > And, on writing these, I'd now rather think we should be more
 > explicit: let addRemoteConfig() throw an exception unless
 > ModuleCCSession hasn't been "started".  Then if some other app
 > incorrectly tries to use addRemoteConfig() it can see what's wrong
 > sooner and more explicitly (rather than via scratching the head in
 > seeing the strange timeout, etc).  I'm attaching a diff of this
 > version of change.  Note that with this change the above added test
 > won't work as expected any more.

 I don't think so. Because, as I look at it, the inability to do the
 blocking read when we already have a nonblocking is a bug of the
 underlying session. It should be fixed sometime. This does only say
 there's the bug and only in one situation. The current workaround is, I
 hope, only workaround.

 If we really want to add an exception, we should do it in the session,
 checking there are not two reads at once and it should be some kind of
 NotImplemented or something.

 > Deferring it to a separate ticket is fine by me (actually I don't like
 > to delay this ticket due to this).  Note also that I'm not necessarily
 > sticking to the specific idea of separating the methods.  My point is
 > that the current interface is not very intuitive and is error prone.
 > Any solution to address this concern is good to me.

 OK.

 > The awkwardness of a pointer to shared pointer is not the main issue
 > to me.  It's more that the interface is fragile and error prone (at
 > least IMO).  While I kind of admit the sample access class doesn't
 > look so sophisticated, I still personally believe this type of
 > solution is much better.  But, again, I'm okay with (or actually
 > rather prefer) leaving this out of this ticket.  If you agree this is
 > worth further discussion, please create a separate file.  If you don't
 > even agree with that, that's also fine for now.  I may or may not
 > create one myself if and when I see a stronger need for it.

 I agree this needs a solution, so I'll open a ticket upon merging this.
 But sophisticated is exactly the opposite of what I find appropriate ‒
 simple problems should have simple solutions.

 I added the comment to keyring and I agree with the changes you did in
 git. Is there anything else left?

 Thanks

-- 
Ticket URL: <http://bind10.isc.org/ticket/931#comment:24>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development


More information about the bind10-tickets mailing list