[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