Add Ed25519 support

added component::archived/stem owner::atagar priority::medium resolution::implemented severity::normal status::closed type::enhancement labels

Trac:
Status: new to needs_review

Hi Patrick. First thing I noticed was that this adds a dependency on pynacl. On #21086 (closed) you're swapping out pycrypto for cryptography - can we use that for this?

Also, a minorish thing but noticed the copyright header for certificates.py you list yourself rather than me. Including your name is fine, but could you list me too? Otherwise legally speaking things become a bit more of a headache...

https://stem.torproject.org/faq.html#what-is-the-copyright-for-patches

Speaking of which, these are really substantial improvements so I gotta ask: are you fine with all your contributions being in the public domain?

Just pushed a change to bring the copyright header for certificate.py up to scratch.

@atagar: I'm 100% ok with these changes being in the public domain :)

I will do some more research now to see if we can resolve these dependencies into one.

It appears that cryptography as yet does not support Ed25519, but may do in some future release https://github.com/pyca/cryptography/issues/2968

Thanks Patrick! Spotting a few things that need to be tweaked but they're minor. Overall looks good! I'll try to get this merged tomorrow.

Hi Patrick. Per chance did you forget to use 'git add' for a new file? Running the descriptor.certificate unit tests fails for me with...

Traceback (most recent call last):
  File "./run_tests.py", line 426, in <module>
    main()
  File "./run_tests.py", line 204, in main
    run_result = _run_test(args, test_class, output_filters, logging_buffer)
  File "./run_tests.py", line 398, in _run_test
    raise exc
AttributeError: 'module' object has no attribute 'certificate'

Hmm https://github.com/patrickod/stem/pull/1/files#diff-ae666c31dc370e3ab59989f4aad057ccR1 definitely shows the certificate.py file there, and when I check out that branch locally the tests pass for me.

Hi Patrick. Sorry about that, as discussed on irc figured out what I was doing wrong. Made a few small changes to this branch but then realized it lacks python 3.x support. Running our unit tests fails with...

% python3 run_tests.py --unit
...
======================================================================
ERROR: test_ed25519_key_certificate_without_extensions
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/atagar/Desktop/stem/test/unit/descriptor/certificate.py", line 86, in test_ed25519_key_certificate_without_extensions
    validate = True
  File "/usr/lib/python3.2/unittest/case.py", line 1170, in deprecated_func
    return original_func(*args, **kwargs)
  File "/usr/lib/python3.2/unittest/case.py", line 1117, in assertRaisesRegex
    callable_obj(*args, **kwargs)
  File "/home/atagar/Desktop/stem/stem/descriptor/certificate.py", line 66, in _parse_certificate
    return Ed25519KeyCertificate(raw_contents, master_key_bytes, validate = validate)
  File "/home/atagar/Desktop/stem/stem/descriptor/certificate.py", line 148, in __init__
    super(Ed25519KeyCertificate, self).__init__(raw_contents, identity_key, validate = False)
  File "/home/atagar/Desktop/stem/stem/descriptor/certificate.py", line 132, in __init__
    self.__set_certificate_entries(raw_contents)
  File "/home/atagar/Desktop/stem/stem/descriptor/certificate.py", line 136, in __set_certificate_entries
    for key, func in Certificate.ATTRIBUTES.iteritems():
AttributeError: 'dict' object has no attribute 'iteritems'

======================================================================
ERROR: test_parse_extensions_invalid_certificate_extension_type
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/atagar/Desktop/stem/test/unit/descriptor/certificate.py", line 57, in test_parse_extensions_invalid_certificate_extension_type
    cert_bytes
  File "/usr/lib/python3.2/unittest/case.py", line 1170, in deprecated_func
    return original_func(*args, **kwargs)
  File "/usr/lib/python3.2/unittest/case.py", line 1117, in assertRaisesRegex
    callable_obj(*args, **kwargs)
  File "/home/atagar/Desktop/stem/stem/descriptor/certificate.py", line 80, in _parse_extensions
    n_extensions = _bytes_to_long(raw_contents[39:40])
  File "/home/atagar/Desktop/stem/stem/descriptor/certificate.py", line 44, in _bytes_to_long
    return long(b.encode('hex'), 16)
NameError: global name 'long' is not defined

======================================================================
ERROR: test_parse_extensions_invalid_n_extensions_count
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/atagar/Desktop/stem/test/unit/descriptor/certificate.py", line 72, in test_parse_extensions_invalid_n_extensions_count
    cert_bytes
  File "/usr/lib/python3.2/unittest/case.py", line 1170, in deprecated_func
    return original_func(*args, **kwargs)
  File "/usr/lib/python3.2/unittest/case.py", line 1117, in assertRaisesRegex
    callable_obj(*args, **kwargs)
  File "/home/atagar/Desktop/stem/stem/descriptor/certificate.py", line 80, in _parse_extensions
    n_extensions = _bytes_to_long(raw_contents[39:40])
  File "/home/atagar/Desktop/stem/stem/descriptor/certificate.py", line 44, in _bytes_to_long
    return long(b.encode('hex'), 16)
NameError: global name 'long' is not defined

======================================================================
ERROR: test_parse_extensions_truncated_extension
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/atagar/Desktop/stem/test/unit/descriptor/certificate.py", line 43, in test_parse_extensions_truncated_extension
    cert_bytes
  File "/usr/lib/python3.2/unittest/case.py", line 1170, in deprecated_func
    return original_func(*args, **kwargs)
  File "/usr/lib/python3.2/unittest/case.py", line 1117, in assertRaisesRegex
    callable_obj(*args, **kwargs)
  File "/home/atagar/Desktop/stem/stem/descriptor/certificate.py", line 80, in _parse_extensions
    n_extensions = _bytes_to_long(raw_contents[39:40])
  File "/home/atagar/Desktop/stem/stem/descriptor/certificate.py", line 44, in _bytes_to_long
    return long(b.encode('hex'), 16)
NameError: global name 'long' is not defined

======================================================================
ERROR: test_with_invalid_type
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/atagar/Desktop/stem/test/unit/descriptor/certificate.py", line 30, in test_with_invalid_type
    None
  File "/usr/lib/python3.2/unittest/case.py", line 1170, in deprecated_func
    return original_func(*args, **kwargs)
  File "/usr/lib/python3.2/unittest/case.py", line 1117, in assertRaisesRegex
    callable_obj(*args, **kwargs)
  File "/home/atagar/Desktop/stem/stem/descriptor/certificate.py", line 74, in _parse_certificate
    raise ValueError("Unknown Certificate type %s" % cert_type.encode('hex'))
LookupError: unknown encoding: hex

======================================================================
ERROR: test_with_invalid_version
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/atagar/Desktop/stem/test/unit/descriptor/certificate.py", line 20, in test_with_invalid_version
    None
  File "/usr/lib/python3.2/unittest/case.py", line 1170, in deprecated_func
    return original_func(*args, **kwargs)
  File "/usr/lib/python3.2/unittest/case.py", line 1117, in assertRaisesRegex
    callable_obj(*args, **kwargs)
  File "/home/atagar/Desktop/stem/stem/descriptor/certificate.py", line 76, in _parse_certificate
    raise ValueError("Unknown Certificate version %s" % version.encode('hex'))
LookupError: unknown encoding: hex

----------------------------------------------------------------------
Ran 7 tests in 0.015s

FAILED (errors=6, skipped=1)

Mind taking a peek? To build on the work I've been doing please fetch from the ed25519 branch of my personal repo (https://git.torproject.org/user/atagar/stem.git)...

https://gitweb.torproject.org/user/atagar/stem.git/log/?h=ed25519

Cheers! -Damian

Ok. I've cleaned up the Python 3 issues.

The latest ed25519 branch is https://github.com/patrickod/stem/tree/patrickod/ed25519-cleanup

It also includes the commit from https://trac.torproject.org/projects/tor/ticket/21583 (with author attribution) to fix Python 3 issues with the cryptography module.

The Python 3.6.0+ test suites break on test.unit.manual.test_parsing_with_example due to some OrderedDict ordering issues in the comparison. They appear to be unrelated to the changes made here, though I'll investigate fixing them separately.

Hi Patrick, many thanks! Just a quick update that I haven't forgotten this. Working on this branch a bit every day but it's a pretty big one so may take a while. :P

Bah. Hoped to have this merged by now but sadly been too busy with work. Flying out on a trip until the 26th during which I won't be able to push but I plan to work on this branch a bit during the flight.

Just a quick update since I hate leaving pull requests in a lurch. Sorry about the delay, hopefully I'll finally get this out soon after I get back. Thanks again for putting this together! It's a great addition. :)

Sorry about the delay! Merged support for ed25519 certs.

Thanks Patrick! Honestly overhauled this quite a bit (additional tests, tweaks, and most notably pynacl is now only required for validaton, not parsing). That said, your work was very useful - especially the crypto bits. Many thanks Patrick!

Trac:
Resolution: N/A to implemented
Status: needs_review to closed

Fantastic! Excited to see it merged. Thanks for shepherding it into master!

I'll have a look at the changes that you committed on top of my commits for some code style feedback and the like to prevent the need for such refactors for future patches, but if you have any other feedback on the patch that'd be very welcome :)

closed

Add Ed25519 support

Child items ...

Activity