[tor-bugs] #10872 [Pluggable transport]: review of obfsclient

[Tor Bug Tracker & Wiki]

#10872: review of obfsclient
-------------------------------------+---------------------
     Reporter:  infinity0            |      Owner:  Yawning
         Type:  task                 |     Status:  new
     Priority:  normal               |  Milestone:
    Component:  Pluggable transport  |    Version:
   Resolution:                       |   Keywords:
Actual Points:                       |  Parent ID:
       Points:                       |
-------------------------------------+---------------------

Comment (by infinity0):

 *..c7400b87e3ec8d8380c3fbf9919b08438b3725cf:

 Brief comments about the crypto code, excluding uniform DH:

 - aes_ctr128 should be renamed aes128_ctr
   - call clear_state() in aes_ctr128 destructor?
   - please annotate AesCtr128::process() in more detail, I couldn't get my
 head around it.
   - in the header file, please also document
     - how is the counter updated for the next block? I couldn't figure
 this out
     - the significance of "offset" in terms of the algorithm, how this is
 used

 - it's valid to request a hash/mac/encryption of an empty buffer
   - remove the len == 0 test from hmac_sha256.cc and sha256.cc
   - in aes_ctr128 the test can remain, but it should probably "return
 true" rather than "return false"

 - I don't know about C++ conventions, but it's probably clearer to have
 bool memequals (and return !ret), instead of int memequals

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/10872#comment:1>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online