[tor-dev] Design for an exit relay scanner: feedback appreciated
atagar at torproject.org
Mon Dec 2 01:09:55 UTC 2013
> I now have similar code which is based on stem:
Hi Philipp, sorry about the long delay before responding. ExitMap looks great!
You might want to look into PEP8 , Python's de-facto style guide.
It's certainly up to you which bits you do/don't like, but coming
close will make your code more uniform with the rest of the Python
world. PyPI has a slick pep8 script you can run over your codebase
. Personally I run this as part of the tests for Stem.
Would you be amenable to changes in that regard from me? This looks
like a fun project, so I'm a bit tempted to sink a weekend or two into
into seeing if I can simplify the codebase.
Some general code review feedback follows...
exitmap / circuit.py
> new = Circuit
Not especially pythonic, but works.
exitmap / circuitpool.py
> while (circuitID is None):
Parentheses aren't necessary.
> if len(self.exitRelays) == 0:
Can also be 'if not self.exitRelays:'.
> circuitID = self.ctrl.new_circuit([const.FIRST_HOP, exitRelay],
> except (stem.InvalidRequest, stem.CircuitExtensionFailed,
> stem.ControllerError) as error:
Stem's InvalidRequest and CircuitExtensionFailed extend
ControllerError, so this can be simplified to...
except stem.ControllerError as error:
Stem's exception hierarchy can be found on...
Your _addCircuit() is soley used by _fillPool(), so personally I'd
probably do this as a generator...
def _generate_circuit(self, await_build):
Pops the top relay off our exitRelays and generates a circuit through it,
going on to the next entry if it fails.
:param bool await_build: block until the circuit is created if **True**
:returns: :class:`~circuit.Circuit` for the circuit we establish
exit_fingerprint = self.exitRelays.pop(0)
logger.debug("Attempting to create circuit with '%s' as exit " \
"relay." % exit_fingerprint)
circuit_id = self.ctrl.new_circuit(
await_build = await_build,
logger.debug("Created circuit #%s with '%s' as exit relay." %
except stem.ControllerError as exc:
logger.warning("Could not establish circuit with '%s'. " \
"Skipping to next exit (error=%s)." %
logger.warning("No more exit relay fingerprints to create circuits with.")
def _fill_pool(self, await_build = False):
if len(self.pool) == const.CIRCUIT_POOL_SIZE:
logger.debug("Attempting to refill the circuit pool to size %d." %
while self.exitRelays and len(self.pool) != const.CIRCUIT_POOL_SIZE:
> # go over circuit pool once
> poolLen = len(self.pool)
> for idx in xrange(poolLen):
> if idx >= len(self.pool):
> return None
> logger.debug("idx=%d, poolsize=%d" % (idx, len(self.pool)))
> circuit = self.pool[idx] # that's a Circuit() object
Honestly I'm finding this class to be overcomplicated. Popping and
appending items between lists is making this more confusing than it
needs to be.
exitmap / command.py
Stem offers a friendlier method of calling commands. Mostly I intended
it for the system module's functions, but you might find it useful
exitmap / exitselector.py
> for desc in stem.descriptor.parse_file(open(consensus)):
Why read the consensus file directly? If you have a controller then
getting it via tor would be the best option. If not then fetching this
directly via the authorities is generally the easiest...
> if not "Exit" in desc.flags:
Perfectly fine, though stem does offer enums...
if stem.Flag.EXIT not in desc.flags:
> for (ip, port) in hosts:
> if not desc.exit_policy.can_exit_to(ip, port):
I don't think this'll actually work. The 'continue' will be for the
iteration over hosts.
exitmap / scanner.py
There is no point in doing this unless you *only* want it to work
without authentication. If you opt for authenticate() instead then
this will also work for cookie auth.
More information about the tor-dev