Vote Details for "Crypt_DiffieHellman" by aharvey

» Details
  • Voter: Adam Harvey 
  • Vote: +1 (conditional)
  • Reviews: Cursory source review, Run examples
» Comment
I don't have the cryptographic background to assess the implementation of the algorithm itself, but I did run the unit tests. I'm +1 assuming a couple of minor issues I ran across while testing are fixed:

- Line 380 of DiffieHellman.php refers to Crypt_DiffieHellman_BigInteger and not Crypt_DiffieHellman_Math_BigInteger, which I presume it should.

- The Crypt_DiffieHellmanTest test fails at present due to $aliceSecretKey and $bobSecretKey being instances of Crypt_DiffieHellman. Appending a call to getSharedSecretKey() in lines 29 and 30 of tests/DiffieHellmanTest.php seems to deal with that.

Beyond that, I'd wonder if the Math classes could be split off into a separate package (Math_BigInteger2?), since they're generally applicable, but my vote's not conditional on that happening.