[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