[tor-commits] [stem/master] Fix Python 3 compatibility issues & broken unit test.

atagar at torproject.org atagar at torproject.org
Thu Mar 30 04:18:02 UTC 2017


commit e0eab2e3bce1dd404c7435b920485f2781146c90
Author: Patrick O'Doherty <p at trickod.com>
Date:   Sat Mar 11 19:02:45 2017 -0800

    Fix Python 3 compatibility issues & broken unit test.
    
    Fix a number of exceptions caused by mixing str and bytes types, which
    while OK in Python 2 cause runtime exceptions in Python 3.
    
    Fix broken truncated extension parsing test.  Not quite sure how this
    test ever failed, as python's slices (in both 2 and 3) always truncate
    instead of raising exceptions for out of bounds indices.
---
 stem/descriptor/certificate.py               | 57 +++++++++++++++++-----------
 stem/descriptor/hidden_service_descriptor.py |  2 +-
 stem/descriptor/server_descriptor.py         |  5 ++-
 test/unit/descriptor/certificate.py          | 15 ++++----
 test/unit/manual.py                          |  3 ++
 5 files changed, 49 insertions(+), 33 deletions(-)

diff --git a/stem/descriptor/certificate.py b/stem/descriptor/certificate.py
index 8904b58..09cf1cf 100644
--- a/stem/descriptor/certificate.py
+++ b/stem/descriptor/certificate.py
@@ -22,6 +22,7 @@ contain one SignedWithEd25519KeyCertificateExtension.
 """
 
 import base64
+import binascii
 import hashlib
 import time
 
@@ -37,11 +38,14 @@ except ImportError:
 SIGNATURE_LENGTH = 64
 STANDARD_ATTRIBUTES_LENGTH = 40
 CERTIFICATE_FLAGS_LENGTH = 4
-ED25519_ROUTER_SIGNATURE_PREFIX = 'Tor router descriptor signature v1'
+ED25519_ROUTER_SIGNATURE_PREFIX = b'Tor router descriptor signature v1'
 
 
 def _bytes_to_long(b):
-  return long(b.encode('hex'), 16)
+  if stem.prereq.is_python_3():
+    return int(binascii.hexlify(stem.util.str_tools._to_bytes(b)), 16)
+  else:
+    return long(binascii.hexlify(b), 16)
 
 
 def _parse_long_offset(offset, length):
@@ -59,21 +63,22 @@ def _parse_offset(offset, length):
 
 
 def _parse_certificate(raw_contents, master_key_bytes, validate = False):
-  version, cert_type = raw_contents[0:2]
+  version = raw_contents[0:1]
+  cert_type = raw_contents[1:2]
 
-  if version == '\x01':
-    if cert_type == '\x04':
+  if version == b'\x01':
+    if cert_type == b'\x04':
       return Ed25519KeyCertificate(raw_contents, master_key_bytes, validate = validate)
-    elif cert_type == '\x05':
+    elif cert_type == b'\x05':
       # TLS link certificated signed with ed25519 signing key
       pass
-    elif cert_type == '\x06':
+    elif cert_type == b'\x06':
       # Ed25519 authentication signed with ed25519 signing key
       pass
     else:
-      raise ValueError("Unknown Certificate type %s" % cert_type.encode('hex'))
+      raise ValueError("Unknown Certificate type %s" % binascii.hexlify(cert_type))
   else:
-    raise ValueError("Unknown Certificate version %s" % version.encode('hex'))
+    raise ValueError("Unknown Certificate version %s" % binascii.hexlify(version))
 
 
 def _parse_extensions(raw_contents):
@@ -83,18 +88,19 @@ def _parse_extensions(raw_contents):
 
   extensions = []
   extension_bytes = raw_contents[STANDARD_ATTRIBUTES_LENGTH:-SIGNATURE_LENGTH]
+
   while len(extension_bytes) > 0:
-    try:
-      ext_length = _bytes_to_long(extension_bytes[0:2])
-      ext_type, ext_flags = extension_bytes[2:CERTIFICATE_FLAGS_LENGTH]
-      ext_data = extension_bytes[CERTIFICATE_FLAGS_LENGTH:(CERTIFICATE_FLAGS_LENGTH + ext_length)]
-    except:
+    ext_length = _bytes_to_long(extension_bytes[0:2])
+    ext_type = extension_bytes[2:3]
+    ext_flags = extension_bytes[3:CERTIFICATE_FLAGS_LENGTH]
+    ext_data = extension_bytes[CERTIFICATE_FLAGS_LENGTH:(CERTIFICATE_FLAGS_LENGTH + ext_length)]
+    if len(ext_type) == 0 or len(ext_flags) == 0 or len(ext_data) == 0:
       raise ValueError('Certificate contained truncated extension')
 
     if ext_type == SignedWithEd25519KeyCertificateExtension.TYPE:
       extension = SignedWithEd25519KeyCertificateExtension(ext_type, ext_flags, ext_data)
     else:
-      raise ValueError('Invalid certificate extension type: %s' % ext_type.encode('hex'))
+      raise ValueError('Invalid certificate extension type: %s' % binascii.hexlify(ext_type))
 
     extensions.append(extension)
     extension_bytes = extension_bytes[CERTIFICATE_FLAGS_LENGTH + ext_length:]
@@ -127,19 +133,23 @@ class Certificate(object):
 
   def __init__(self, raw_contents, identity_key, validate = False):
     self.certificate_bytes = raw_contents
-    self.identity_key = identity_key
+
+    if type(identity_key) == bytes:
+      self.identity_key = stem.util.str_tools._to_unicode(identity_key)
+    else:
+      self.identity_key = identity_key
 
     self.__set_certificate_entries(raw_contents)
 
   def __set_certificate_entries(self, raw_contents):
     entries = OrderedDict()
-    for key, func in Certificate.ATTRIBUTES.iteritems():
+    for key, func in Certificate.ATTRIBUTES.items():
       try:
         entries[key] = func(raw_contents)
       except IndexError:
         raise ValueError('Unable to get bytes for %s from certificate' % key)
 
-    for key, value in entries.iteritems():
+    for key, value in entries.items():
       setattr(self, key, value)
 
 
@@ -167,7 +177,7 @@ class Ed25519KeyCertificate(Certificate):
     signature_bytes = base64.b64decode(stem.util.str_tools._to_bytes(signature) + b'=' * missing_padding)
     verify_key = nacl.signing.VerifyKey(self.certified_key)
 
-    signed_part = descriptor[:descriptor.index('router-sig-ed25519 ') + len('router-sig-ed25519 ')]
+    signed_part = descriptor[:descriptor.index(b'router-sig-ed25519 ') + len('router-sig-ed25519 ')]
     descriptor_with_prefix = ED25519_ROUTER_SIGNATURE_PREFIX + signed_part
     descriptor_sha256_digest = hashlib.sha256(descriptor_with_prefix).digest()
 
@@ -181,10 +191,11 @@ class Ed25519KeyCertificate(Certificate):
       raise ValueError('Certificate validation requires the pynacl module')
 
     import nacl.signing
+    import nacl.encoding
     from nacl.exceptions import BadSignatureError
 
     if self.identity_key:
-      verify_key = nacl.signing.VerifyKey(base64.b64decode(self.identity_key + '='))
+      verify_key = nacl.signing.VerifyKey(self.identity_key + '=', encoder=nacl.encoding.Base64Encoder)
     else:
       verify_key = nacl.singing.VerifyKey(self.extensions[0].ext_data)
 
@@ -195,7 +206,7 @@ class Ed25519KeyCertificate(Certificate):
 
 
 class CertificateExtension(object):
-  KNOWN_TYPES = ['\x04']
+  KNOWN_TYPES = [b'\x04']
 
   def __init__(self, ext_type, ext_flags, ext_data):
     self.ext_type = ext_type
@@ -206,11 +217,11 @@ class CertificateExtension(object):
     return self.ext_type in CertificateExtension.KNOWN_TYPES
 
   def affects_validation(self):
-    return self.ext_flags == '\x01'
+    return self.ext_flags == b'\x01'
 
 
 class SignedWithEd25519KeyCertificateExtension(CertificateExtension):
-  TYPE = '\x04'
+  TYPE = b'\x04'
 
   def __init__(self, ext_type, ext_flags, ext_data):
     super(SignedWithEd25519KeyCertificateExtension, self).__init__(ext_type, ext_flags, ext_data)
diff --git a/stem/descriptor/hidden_service_descriptor.py b/stem/descriptor/hidden_service_descriptor.py
index dc42286..9c87500 100644
--- a/stem/descriptor/hidden_service_descriptor.py
+++ b/stem/descriptor/hidden_service_descriptor.py
@@ -326,7 +326,7 @@ class HiddenServiceDescriptor(Descriptor):
 
       # try decrypting the session key
 
-      cipher = Cipher(algorithms.AES(authentication_cookie), modes.CTR(stem.util.str_tools._to_bytes('\x00' * len(iv))), default_backend())
+      cipher = Cipher(algorithms.AES(authentication_cookie), modes.CTR(b'\x00' * len(iv)), default_backend())
       decryptor = cipher.decryptor()
       session_key = decryptor.update(encrypted_session_key) + decryptor.finalize()
 
diff --git a/stem/descriptor/server_descriptor.py b/stem/descriptor/server_descriptor.py
index a695966..9cd709b 100644
--- a/stem/descriptor/server_descriptor.py
+++ b/stem/descriptor/server_descriptor.py
@@ -35,6 +35,7 @@ import functools
 import hashlib
 import re
 import base64
+import binascii
 
 import stem.descriptor.extrainfo_descriptor
 import stem.exit_policy
@@ -800,8 +801,8 @@ class RelayDescriptor(ServerDescriptor):
     """
 
     signing_key_digest = hashlib.sha1(_bytes_for_block(self.signing_key)).digest()
-    data = signing_key_digest + base64.b64decode(self.ed25519_master_key + b'=')
-    return data.encode('hex').upper()
+    data = signing_key_digest + base64.b64decode(self.ed25519_master_key + '=')
+    return stem.util.str_tools._to_unicode(binascii.hexlify(data).upper())
 
   def _compare(self, other, method):
     if not isinstance(other, RelayDescriptor):
diff --git a/test/unit/descriptor/certificate.py b/test/unit/descriptor/certificate.py
index 61f6068..4e71e84 100644
--- a/test/unit/descriptor/certificate.py
+++ b/test/unit/descriptor/certificate.py
@@ -11,7 +11,7 @@ import test.runner
 
 class TestCertificate(unittest.TestCase):
   def test_with_invalid_version(self):
-    cert_bytes = '\x02\x04'
+    cert_bytes = b'\x02\x04'
     self.assertRaisesRegexp(
       ValueError,
       'Unknown Certificate version',
@@ -21,7 +21,7 @@ class TestCertificate(unittest.TestCase):
     )
 
   def test_with_invalid_type(self):
-    cert_bytes = '\x01\x07'
+    cert_bytes = b'\x01\x07'
     self.assertRaisesRegexp(
       ValueError,
       'Unknown Certificate type',
@@ -34,13 +34,14 @@ class TestCertificate(unittest.TestCase):
     cert_bytes = '\x00' * 39  # First 40 bytes are standard fields
     cert_bytes += '\x01'  # n_extensions = 1
     cert_bytes += '\x00\x08'  # extension length = 8 bytes
+    cert_bytes += '\x04'      # ext_type = 0x04
     cert_bytes += stem.descriptor.certificate.SIGNATURE_LENGTH * '\x00'  # pad empty signature block
 
     self.assertRaisesRegexp(
       ValueError,
       'Certificate contained truncated extension',
       stem.descriptor.certificate._parse_extensions,
-      cert_bytes
+      stem.util.str_tools._to_bytes(cert_bytes)
      )
 
   def test_parse_extensions_invalid_certificate_extension_type(self):
@@ -54,7 +55,7 @@ class TestCertificate(unittest.TestCase):
       ValueError,
       'Invalid certificate extension type:',
       stem.descriptor.certificate._parse_extensions,
-      cert_bytes
+      stem.util.str_tools._to_bytes(cert_bytes)
      )
 
   def test_parse_extensions_invalid_n_extensions_count(self):
@@ -69,7 +70,7 @@ class TestCertificate(unittest.TestCase):
       ValueError,
       'n_extensions was 2 but parsed 1',
       stem.descriptor.certificate._parse_extensions,
-      cert_bytes
+      stem.util.str_tools._to_bytes(cert_bytes)
      )
 
   def test_ed25519_key_certificate_without_extensions(self):
@@ -81,7 +82,7 @@ class TestCertificate(unittest.TestCase):
       ValueError,
       'Ed25519KeyCertificate missing SignedWithEd25519KeyCertificateExtension extension',
       stem.descriptor.certificate._parse_certificate,
-      cert_bytes,
+      stem.util.str_tools._to_bytes(cert_bytes),
       None,
       validate = True
      )
@@ -107,7 +108,7 @@ class TestCertificate(unittest.TestCase):
       ValueError,
       'Ed25519KeyCertificate signature invalid',
       stem.descriptor.certificate._parse_certificate,
-      cert_bytes,
+      stem.util.str_tools._to_bytes(cert_bytes),
       master_key_base64,
       validate = True
     )
diff --git a/test/unit/manual.py b/test/unit/manual.py
index 5f939c3..671ed93 100644
--- a/test/unit/manual.py
+++ b/test/unit/manual.py
@@ -173,6 +173,9 @@ class TestManual(unittest.TestCase):
     self.assertEqual(EXPECTED_CLI_OPTIONS, manual.commandline_options)
     self.assertEqual(EXPECTED_SIGNALS, manual.signals)
     self.assertEqual(EXPECTED_FILES, manual.files)
+    if EXPECTED_CONFIG_OPTIONS != manual.config_options:
+      print(EXPECTED_CONFIG_OPTIONS)
+      print(manual.config_options)
     self.assertEqual(EXPECTED_CONFIG_OPTIONS, manual.config_options)
 
   def test_parsing_with_unknown_options(self):





More information about the tor-commits mailing list