[pycrypto] Initial review of Thorsten's Py3k changes

[Dwayne C. Litzenberger]

Have a look in the various common.py files.  All of the hex test vectors are   
being fed through either a2b_hex or b2a_hex.  I think it should be possible 
to make versions of b2a_hex and a2b_hex that also do bytes->str and 
str->bytes conversions, respectively.

The following code works in both Python 2.1 and Python 3.2b2:

     from binascii import b2a_hex as _b2a_hex, a2b_hex as _a2b_hex
     from codecs import ascii_decode as _ascii_decode
     def bin2hex(bts):
         """Like b2a_hex, but returns a str instead of bytes in Python 3.x"""
         return _ascii_decode(_b2a_hex(bts))[0]
     def hex2bin(s):
         """Like a2b_hex, but expects a str instead of bytes in Python 3.x"""
         return _a2b_hex(s.encode('ascii'))

That code could, for example, be placed into lib/Crypto/Util/py3compat.py

Then, at the top of lib/Crypto/SelfTest/Cipher/common.py, you could 
replace:

     from binascii import a2b_hex, b2a_hex

with:

     from Crypto.Util.py3compat import \
         hex2bin as a2b_hex, bin2hex as b2a_hex

This also revealed a bug in RoundtripTest, which can be fixed as follows:

--------------------- snip -------------------
@@ -222,9 +224,9 @@ class RoundtripTest(unittest.TestCase):
          for mode in (self.module.MODE_ECB, self.module.MODE_CBC, self.module.MODE_CFB, self.module.MODE_PGP, self.module.MODE_OFB):
              encryption_cipher = self.module.new(a2b_hex(self.key), mode, self.iv)
              decryption_cipher = self.module.new(a2b_hex(self.key), mode, self.iv)
-            ciphertext = encryption_cipher.encrypt(self.plaintext)
+            ciphertext = encryption_cipher.encrypt(a2b_hex(self.plaintext))
              decrypted_plaintext = decryption_cipher.decrypt(ciphertext)
-            self.assertEqual(self.plaintext, decrypted_plaintext)
+            self.assertEqual(self.plaintext, b2a_hex(decrypted_plaintext))
  
  def make_block_tests(module, module_name, test_data):
      tests = []
--------------------- snip -------------------

I'm not certain whether this makes your patch cleaner overall, but it might 
be something worth looking at.

Cheers,
- Dwayne

On Wed, Jan 19, 2011 at 10:27:01AM -0500, Thorsten Behrens wrote:
>Yes, something like that would work. That's a good idea.
>
>I'm happy to make that change. Just waiting on Dwayne to come back on 
>whether he wants me to scrap my repository and redo it (so he has one 
>big commit), and then whether he'd rather have the b() wrapping done the 
>way you suggest.
>
>I also have a small change to the blowfish unit test. I added the CBC 
>vector.
>
>Thorsten
>
>On 1/19/2011 5:26 AM, Legrandin wrote:
>> I am not sure if I grasp all new stuff in Python 3, but would it possible to
>> keep the test vectors as they are, and possibly change the encoding at runtime?
>>
>> For instance, in test_AES.py, we have:
>>
>> test_data = [
>> ...
>> ( '00112233445566778899aabbccddeeff', 'dda97ca4864cdfe06eaf70a0ec0d7191',
>>    '000102030405060708090a0b0c0d0e0f1011121314151617',
>>    'FIPS 197 C.2 (AES-192)' ),
>> ... ]
>>
>> The proposed change wraps each string but the last one with b().
>>
>> Instead of changing the test vector line, we could have a separate
>> piece of code that dynamically
>> adjusts a tuple in the list in case python 3 is detected.
>>
>> Something along these lines, but possibly more pythonic:
>>
>> if python_version>2:
>>     for t in test_data:
>>       idx = test_data.index(t)
>>       newt = tuple(b(t[0), b(t[1]), b(t[2]), t[3])
>>       test_data[idx] = newt



-- 
Dwayne C. Litzenberger <dlitz at dlitz.net>
  OpenPGP: 19E1 1FE8 B3CF F273 ED17  4A24 928C EC13 39C2 5CF7