Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#13045 closed defect (fixed)

Leekspin descriptor signatures cannot be verified by Stem

Reported by: isis Owned by: isis
Priority: High Milestone:
Component: Circumvention/BridgeDB Version:
Severity: Keywords: leekspin, bridgedb-parsers, isis2014Q3Q4
Cc: isis, sysrqb Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description (last modified by isis)

The signatures on descriptor documents generated by Leekspin (currently on version 0.2.1), in leekspin.crypto.signDescriptorDocument(), cannot currently be verified by Stem.

Stem uses this code to successfully verify signatures created by Tor. There is currently some confusion in the spec (#13042) over the ordering and versions of encodings applied to the signing-key and the router-signature. Until #13042 is made clearer, the best way we have to fix this is to do what Stem does in reverse.

There may be some problems here with Python dependencies (the available, packaged, implementations of RSA, PKCS#1, and ASN.1 aren't all that great, as noted in #5810). I don't care what dependencies we add to get this to work; it's causing BridgeDB's new Stem-based parsers (#9380) to choke during test runs on Leekspin's fake bridge descriptors.

Child Tickets

Change History (7)

comment:1 Changed 5 years ago by isis

Status: newaccepted

Atagar mentioned on IRC that:

In Stem's test suite, there are two checks. We generate a valid digest for one and mock out the other. I think what you want to look at is this. The tricky bit is the sign_descriptor_content()

comment:2 Changed 5 years ago by atagar

Stem vends a test_tools module that presently vends functions for easily doing pyflakes and pep8 as part of your tests...

https://stem.torproject.org/api/util/test_tools.html

For example...

https://gitweb.torproject.org/arm.git/blob/HEAD:/run_tests.py

I'd be ok with expanding this module to have the mocking.py functions for generating valid descriptor content if it would be helpful (or something more like what Leekspin does if that's better). I've been meaning to look at Leekspin at some point to see if its functionality belongs in Stem but haven't had the time.

comment:3 Changed 5 years ago by isis

Description: modified (diff)

comment:4 in reply to:  2 Changed 5 years ago by isis

Replying to atagar:

Stem vends a test_tools module that presently vends functions for easily doing pyflakes and pep8 as part of your tests...

https://stem.torproject.org/api/util/test_tools.html

For example...

https://gitweb.torproject.org/arm.git/blob/HEAD:/run_tests.py


Oooooo. Those are super useful.

BridgeDB's code still has some of the old stuff in it (#12505) which is PEP8/stylistically horrifying. Also a lot of it is just plain horrifying in every way. Until those are fixed up, making an automated test with pylint and/or pep8 would just fail, which I fear would cause confusion for new contributors. But it's on my TODO list, once the cleanup is done, to add something of the sort in order to enforce consistency.

I'd be ok with expanding this module to have the mocking.py functions for generating valid descriptor content if it would be helpful (or something more like what Leekspin does if that's better).


I think Leekspin would only need stem.test.mocking.sign_descriptor_content().

I've been meaning to look at Leekspin at some point to see if its functionality belongs in Stem but haven't had the time.


Currently, Leekspin only generates unsanitised bridge descriptors. It's also on my TODO list to make it support relay descriptors. It might be useful if you want to test Stem's parsing of unsanitised descriptors, but obviously not until bugs like this one are fixed. :)

comment:5 Changed 5 years ago by atagar

Until those are fixed up, making an automated test with pylint and/or pep8 would just fail, which I fear would cause confusion for new contributors.

Not necessarily. Stem was once in a similar boat. It took days of dedicated effort to shift to be mostly pep8 conformant, so what I did was overhauled Stem one issue at a time.

My suggestion would be to run pep8 over BridgeDB and simply note all the issue types it fails for. Then blacklist them all. This is done via a 'pep8.ignore' configuration like...

https://gitweb.torproject.org/arm.git/blob/HEAD:/test/settings.cfg

Now tests will pass and if you get a patch for anything that violates the parts of pep8 you already comply with then they'll get a heads up. After this you can shift BridgeDB to be pep8 conformant one issue at a time. :)

Currently, Leekspin only generates unsanitised bridge descriptors.

By 'unsanitised bridge descriptors' you mean regular server descriptors, right? If not then how do they differ?

An open question in my mind is 'How do Leekspin and Stem's mocking module differ? They both make test descriptor data, right?'. Maybe there's good reason for them to both continue independently - I just haven't sunk the time into looking Leekspin over yet.

comment:6 Changed 5 years ago by isis

Resolution: fixed
Status: acceptedclosed

Okay! This is fixed in my fix/13045 branch and packaged/released in Leekspin-1.0.0.

I also added the ability to create router descriptors. (warning: probably buggy)

comment:7 Changed 5 years ago by isis

Keywords: isis2014Q3Q4 added
Note: See TracTickets for help on using tickets.