BIND 10 #781: Define cryptographic API

BIND 10 Development do-not-reply at isc.org
Tue Apr 26 16:51:47 UTC 2011


#781: Define cryptographic API
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  stephen                            |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20110503
  blocker                            |            Resolution:
                  Component:         |             Sensitive:  0
  Unclassified                       |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  6.0
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 The latest code looks *mostly* okay.  Two points:

 - I'd suggest adding a brief description for enum HashAlgorithm
 - According to the sense of introducing the deleter method for HMAC,
   it's better to use it in crypto_unittests.cc.  That is, use shared_ptr
 with
   the deleter instead of scoped_ptr (in my understanding we cannot specify
   a deleter for scoped_ptr):
 {{{
 // Instead of this...
 //      boost::scoped_ptr<HMAC> hmac_sign(crypto.createHMAC(secret,
 //                                                          secret_len,
 //
 hash_algorithm));

 // do this:
         boost::shared_ptr<HMAC> hmac_sign(crypto.createHMAC(secret,
                                                             secret_len,
 hash_algorithm),
                                           deleteHMAC);
 }}}

 These should be quite trivial and I don't think we need another
 cycle of review for them.  After fixing these please free to merge it.

 As for the constness of getCryptoLink(), createHMAC(), I'm okay with
 leaving them non const.  In fact, I also thought about it in terms of
 const semantics; if we consider a CryptoLink class encapsulating the
 backend crypto engine with all its internal states, createHMAC() would
 better be defined as non const.  But the actual implementation only
 stores the initialization object, which should be intact once the
 initialization is completed.  That's why I thought it made sense to
 constify it.  But on the second thought, in reality createHMAC()
 (possibly) modifies some internal state behind the cryptolink, so it
 would probably be better not to give a false impression about safety
 of the use of createHMAC(), etc.

 You didn't mention the "-y option" format, but as I already responded
 I'm okay with leaving it open for now.

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


More information about the bind10-tickets mailing list