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

Re: [tor-bugs] #9974 [Flashproxy]: packaging and installation scripts for facilitator



#9974: packaging and installation scripts for facilitator
-----------------------------+----------------------------
     Reporter:  infinity0    |      Owner:  dcf
         Type:  enhancement  |     Status:  needs_revision
     Priority:  normal       |  Milestone:
    Component:  Flashproxy   |    Version:
   Resolution:               |   Keywords:
Actual Points:               |  Parent ID:
       Points:               |
-----------------------------+----------------------------

Comment (by dcf):

 Replying to [comment:11 infinity0]:
 > Replying to [comment:10 dcf]:
 > >
 [https://github.com/infinity0/flashproxy/commit/a310c1d0f893ebd18a128a9d92684b3cf51d70a4
 Changes to name resolution in facilitator-email-poller.]
 > > > - smarter default IMAP host; and resolve DNS by default - previously
 one had to use an IP address
 > > > - add a nameOk option to parse_addr_spec as certificate verification
 requires imap host to be a name
 > > This is a good bug you found. I'm not wild about the new more
 complicated interface to `parse_addr_spec` with the `nameOk` parameter.
 It's a reasonable thing to add, but I wonder if there is a better way.
 Maybe add a new lower-level function that additionally returns the host
 part string along with what `parse_addr_spec` normally returns--then
 `parse_addr_spec` could call the lower-level function and throw the extra
 information away.
 >
 > I think a separate function would only move the complexity around. I
 found it weird that previously the "resolve" param controlled both input
 acceptance and output type. Ultimately, we need three behaviours:
 >
 > 1. accepts name/ip as input, gives ip as output (nameOk, resolve)
 > 2. accepts name/ip as input, gives original as output (nameOk, not
 resolve)
 > 3. accepts ip as input, gives original as output (not nameOk, not
 resolve)
 >
 > In the newer version, "nameOk" controls which inputs are accepted and
 "resolve" controls the type of output. If we delegate (2) to a separate
 function, we'd only need one flag (resolve) but another function for users
 to know about. I think I prefer the two-simple-flags approach rather than
 one-complex-flag-and-one-separate-function.
 >
 > Incidentally, (2) is what parse_addr_spec from the client code does -
 accepts DNS names (so you can say e.g. localhost:9000), but doesn't
 resolve them. (But I still think 2-params is better.)

 I see. The facilitator's `parse_addr_spec` really is a different kind of
 beast. For that reason, maybe we should make ''it'' the exception, with a
 different function name, rather than foisting its more complicated
 interface on the other programs that need `parse_addr_spec`. Maybe a
 separate `resolve(host, port)` function would do it. Unless I'm mistaken,
 the facilitator's weird mutant `parse_addr_spec` is the same as
 `flashproxy.util.parse_addr_spec`, with the additional step of
 parsing/name resolution.

 I see
 [https://gitweb.torproject.org/flashproxy.git/blob/813bda092193136423d27ef3c41aa1b26199ab4f:/facilitator/facilitator#l421
 only one place] with `resolve=True`, and resolution arguably isn't needed
 there. In
 [https://gitweb.torproject.org/flashproxy.git/blob/813bda092193136423d27ef3c41aa1b26199ab4f:/facilitator/facilitator#l421
 parsing client registrations], we still want to
 `socket.getaddrinfo(socket.AI_NUMERICHOST)` somehow, because we don't want
 to resolve untrusted DNS names nor give them to proxies. The
 [https://gitweb.torproject.org/flashproxy.git/blob/813bda092193136423d27ef3c41aa1b26199ab4f:/facilitator
 /facilitator-email-poller#l191 only other place] `fac.parse_addr_spec` is
 used is in flashproxy-email-poller to parse the IMAP server, and there
 it's clear we should be using plain `flashproxy.util.parse_addr_spec`
 instead.

 That's my proposal: Use good old `flashproxy.util.parse_addr_spec`
 everywhere, and in the few cases where we need to resolve the address or
 ensure that it is numeric, do that as a separate step. (The
 `parse_addr_spec`+extra step can be encapsulated in a function, of
 course.)

 For what it's worth, the `resolve` parameter ''doesn't'' control the
 output format. The "gives original as output" case doesn't exist in the
 facilitator's `parse_addr_spec` as it exists now. The IP address is always
 given to `socket.getaddrinfo` to be either parsed or resolved: if
 `resolve` is false, then name resolution is disabled with the
 `socket.AI_NUMERICHOST` flag. It seems like it's returning the original
 string because Python's representation of an IP address is a string, which
 in this case happens to be equal to the input string. Some examples show
 that it really is parsing the address and not just echoing its input:
 {{{
 >>> fac.parse_addr_spec("[1:0:0:0:0:0:0:2]:9999", resolve=True)
 ('1::2', 9999)
 >>> fac.parse_addr_spec("[1:0:0:0:0:0:0:2]:9999", resolve=False)
 ('1::2', 9999)
 >>> fac.parse_addr_spec("example.com:9999", resolve=False)
 ValueError: Bad host or port: "example.com" "9999": [Errno -2] Name or
 service not known
 }}}

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