#21558 closed enhancement (implemented)
Add Ed25519 support
Reported by: | patrickod | Owned by: | atagar |
---|---|---|---|
Priority: | Medium | Milestone: | |
Component: | Core Tor/Stem | Version: | |
Severity: | Normal | Keywords: | |
Cc: | Actual Points: | ||
Parent ID: | Points: | ||
Reviewer: | Sponsor: |
Description
https://github.com/patrickod/stem/pull/1 introduces support for all Ed25519 identity and signing key descriptor properties associated with the implementation of props 220 and 228.
This unblocks https://trac.torproject.org/projects/tor/ticket/16670
Child Tickets
Change History (13)
comment:1 Changed 2 years ago by
Status: | new → needs_review |
---|
comment:2 Changed 2 years ago by
comment:3 Changed 2 years ago by
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.
comment:4 Changed 2 years ago by
It appears that cryptography as yet does not support Ed25519, but may do in some future release https://github.com/pyca/cryptography/issues/2968
comment:5 Changed 2 years ago by
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.
comment:6 Changed 2 years ago by
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'
comment:7 Changed 2 years ago by
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.
comment:8 Changed 2 years ago by
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
comment:9 Changed 2 years ago by
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.
comment:10 Changed 2 years ago by
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
comment:11 Changed 2 years ago by
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. :)
comment:12 Changed 23 months ago by
Resolution: | → implemented |
---|---|
Status: | needs_review → closed |
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!
comment:13 Changed 23 months ago by
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 :)
Hi Patrick. First thing I noticed was that this adds a dependency on pynacl. On #21086 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?