[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #31369 [Core Tor/Stem]: HSv3 descriptor support in stem
#31369: HSv3 descriptor support in stem
-----------------------------------------+--------------------------------
Reporter: asn | Owner: atagar
Type: defect | Status: reopened
Priority: Medium | Milestone:
Component: Core Tor/Stem | Version:
Severity: Normal | Resolution:
Keywords: tor-hs onionbalance scaling | Actual Points:
Parent ID: #26768 | Points: 9
Reviewer: | Sponsor: Sponsor27-must
-----------------------------------------+--------------------------------
Comment (by asn):
[Carrying over from IRC discussion]
Hello atagar! Thanks for the great work here! I really appreciate it!
BTW, any plans on putting this stuff into github so that I can do some
inline comments?
{{{
20:01 <+atagar> asn: Hi, I'm attempting to understand a comment in
certificate.py:
20:01 <+atagar> "ATAGAR XXX certificates are generic and not just for
descriptor, however this function assumes they are. this needs to be moved
to the descriptor module. the new verify() function is more generic and
should be used."
20:01 <+atagar> You are absolutely right that the method is specific to
server descriptors and needs to be changed, however perhapse I'm blind but
I'm not finding the 'verify()' function you're referencing.
}}}
You are right that I didn't add a verify() function. I'm not sure what I
was refering to. I think I skipped that for now, since Tor already does
this verification as part of HSFETCH.
But yes, IMO we should have a generic certificate validate (or verify?)
function that just verifies that the certificate correctly signs the
included key using the intended public key (see tor_cert_checksig() in
little-t-tor): this is what the current validate() function does up to the
signing_key.verify() call. And then we can make
server_descriptor_validate() or hsv3_validate() that do more domain-
specific things.
{{{
22:48 <+atagar> asn: Updated my hsv3 branch with the twiddling I got in
today...
22:48 <+atagar>
https://gitweb.torproject.org/user/atagar/stem.git/log/?h=hsv3
}}}
Great! I see that you have disabled the checksum verification but this
actually passes for me. That is, if I remove the comments you added, the
`test_for_decrypt()` unittest passes fine. Could it be something with your
sha3 API if you have an older version of Python? I'm using Python 3.7.4+.
{{{
22:56 <+atagar> At this point I'm beginning to lose track of all the balls
we have in the air. On the ticket you began moving on to descriptor
encoding but it would be nice if we got decryption into a mergeable state
first. Would you like for me to begin digging into hsv3_crypto or should I
move back to the HSv3 descriptor parsing branch I began before this pull
request?
23:03 <+atagar> asn: Concerning your onion_address question I moved it
from the constructor into the decode method
(https://gitweb.torproject.org/user/atagar/stem.git/commit/?h=hsv3&id=14a44b1).
At this point what I need is a working unit test that calls "_decrypt()"
to get the plaintext inner descriptor. Is that what you had? Since the
test didn't make an assertion on the plaintext content I was unsure how
far along the pull request is.
}}}
Sounds good about the decode() method. That makes sense! Thanks!
Yep there are many balls. I transitioned from decoding to encoding while I
was waiting for feedback on my questions, but also because I wanted the
encoding done so that I build a proper encode-to-decode unittest for HSv3
descriptors.
The current `test_for_decrypt()` unittest indeed gets the inner plaintext
of the descriptor (and also the middle layer on the process), so we should
be good to go on parsing up to that level. Would you be interested in
digging more there; that is parsing the middle and inner layers of the
descriptor? I think that would be great since I'm not sure what's the best
way for stem to add more parsing handlers (for the extra layers) in the
middle of the decode function.
In the meanwhile, I will be working on the encoding part, if you don't
think that's too chaotic to do.
----
Two more things:
Commit `14a44b1c6e1438abdf5687a1c468536d88481f81` killed a few XXXs I had
added to remind myself in the future (so that we don't forget them before
merging).
Also, I see you deleted the `onion_address` argument from the constructor
and that's fine with me, but FYI the encoding methods will also need an
extra argument during descriptor encoding. See
https://github.com/torproject/stem/pull/21/commits/55bb32c534a35430d6ba506ecf27340f1fd97bbc
.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/31369#comment:13>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs