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

Re: [tor-talk] [tor-relays] [ANNOUNCE] check_tor.py: a Nagios check for Tor relays

Hi Lunar. Glad that you're finding stem to be useful!

Sathyanarayanan suggested putting together a page with stem examples
(https://trac.torproject.org/8322). If you wouldn't mind check_tor
would be perfect for that. Some more inline comments would be nice,
but otherwise it's a fine script. :)

> [1] https://stem.readthedocs.org/

As mentioned on irc this moved a long while back to...


> self.tempdir = None

It looks like tempdir is solely used to determine if self.datadir was
created via mkdtemp(), and hence if it should be cleaned up afterward.
Maybe it would make sense for this to be a 'self.datadir_is_temp'

> 'ControlSocket': self.control_socket_path(),

If you're using a control socket then there's little point in also
setting CookieAuthentication. Control sockets are based on filesystem
permissions, like an auth cookie, so it doesn't provide any added

> 'CookieAuthFile': self.cookie_auth_file_path(),

Why do you set this explicitely? The cookie path is part of the
PROTOCOLINFO response so it isn't needed to authenticate.

> self.socks_addr = self.controller.get_info('net/listeners/socks')[1:-1] # skip " at begining and end

You might want to either wrap get_info() calls in try/catch blocks or
provide a default argument (ie, something like
"get_info('net/listeners/socks', '"0"')").

> if self.popen:
>   try:
>     self.popen.poll()
>     if not self.popen.returncode:
>       self.popen.kill()
>       self.popen.communicate()
>   except OSError, ex:
>     if ex.errno != errno.ESRCH:
>       raise
>   self.popen = None

Nice, I didn't think of checking the errno. The above will only work
on python 2.6 and above. If you want 2.5 compatability for non-windows
then you can fall back on os.kill(). Here's what I do in stem's

if self._tor_process:
  # if the tor process has stopped on its own then the following raises
  # an OSError ([Errno 3] No such process)

    if stem.prereq.is_python_26():
    elif not stem.util.system.is_windows():
      os.kill(self._tor_process.pid, signal.SIGTERM)
      test.output.print_line("failed (unable to call kill() in python
2.5)", *ERROR_ATTR)
  except OSError:

  self._tor_process.communicate()  # blocks until the process is done

> def get_url(self, socks_addr, url):
>   ...
>   time.sleep(30)

There's quite a few spots in this script that could benefit from
comments. This line especially I'm not sure why a thirty second sleep
is necessary.

> self.nickname = event.path[0][1]

You might want to call
'self.controller.enable_feature("VERBOSE_NAMES")' during setup,
otherwise the nickname may be None prior to tor

> class MiddleTest(ConnectivityTest):
>   ...
>   def create_circuit(self):
>     ...

When you call self.controller.new_circuit() would the await_build
argument help? You're presently listening for CIRC events to call
attach_stream() when the circuit is done being built, and that's
exactly what await_build is intended to help with.

Btw, if circuits always fail to be created then the retry in
handle_circ() will produce an infinite loop. Also, new_circuit() can
potentially raise a ControllerError.

> def get_exits(self):
>   for desc in self.controller.get_server_descriptors():
>     if not desc.hibernating and desc.exit_policy.can_exit_to(address, port):
>       exits.append(desc.fingerprint)

I was about to warn that this would break for new tor versions due to
the switch to microdescriptors, but I suppose that's what
FetchUselessDescriptors is for. 'UseMicrodescriptors 0' might be a
little clearer, or falling back on  microdescriptors. Stem presently
does not have microdescriptor support, but that's what I'm currently
working on (it should be done sometime this week)...


If you do fall back on microdescriptors then be aware that they're a
bit more painful to work with than server descriptors for this sort of
task. For more on that see...


Cheers! -Damian
tor-talk mailing list