[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #13667 [Tor]: Prevent port scanning of hidden services

#13667: Prevent port scanning of hidden services
------------------------+------------------------------------------
     Reporter:  arma    |      Owner:
         Type:  defect  |     Status:  needs_review
     Priority:  major   |  Milestone:  Tor: 0.2.6.x-final
    Component:  Tor     |    Version:
   Resolution:          |   Keywords:  SponsorR tor-hs 025-backport
Actual Points:          |  Parent ID:
       Points:          |
------------------------+------------------------------------------

Comment (by arma):

 Patch looks like it'll work to me. Thanks!

 1) Did you test it?

 And since I'm helping you get up to speed as a developer, some minor
 points (which maybe you and nickm/andrea should talk through and agree
 about how to handle):

 2)
 {{{
 +                                            Circuit is origin here thus
 +       * will be set automatically to REASON_NONE anyway. (ref: #13667)
 */
 +      circuit_mark_for_close(circ, END_CIRC_REASON_NONE);
 }}}

 But in circuit_mark_for_close_ I see
 {{{
   if (reason == END_CIRC_AT_ORIGIN) {
     if (!CIRCUIT_IS_ORIGIN(circ)) {
       log_warn(LD_BUG, "Specified 'at-origin' non-reason for ending
 circuit, "
                "but circuit was not at origin. (called %s:%d,
 purpose=%d)",
                file, line, circ->purpose);
     }
     reason = END_CIRC_REASON_NONE;
   }
 }}}
 Does that mean saying END_CIRC_AT_ORIGIN is the safer plan here? How come
 we don't use ND_CIRC_AT_ORIGIN more often? If we opt not to use it here,
 should we just get rid of it?

 3) Is "(ref: #13667)" the way we want to point people at the bugtracker? I
 know in the past nickm has wanted to make sure that the code stands
 independently of the bugtracker, but I think it does in this case.

 4) "Note that this does not mitigate port scanning" -- I would say it
 actually does mitigate it. It just doesn't completely solve it.

 5) If you happen to reformat the comments and they ended up a few columns
 shorter, I wouldn't object. :)

 6) The first two rows of your changes file want to right-shift two spaces,
 and s/immetiately/immediately/

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/13667#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