commit c1af35b2e2d57cb8850b1cf345d29065766a9410 Author: Ximin Luo infinity0@gmx.com Date: Mon Oct 7 13:07:20 2013 +0100

report more detailed errors and show stacktraces when unsafe logging --- facilitator/fac.py | 6 ++--- facilitator/facilitator | 63 +++++++++++++++++++---------------------------- 2 files changed, 28 insertions(+), 41 deletions(-)

diff --git a/facilitator/fac.py b/facilitator/fac.py index 74ce27d..c08ee2a 100644 --- a/facilitator/fac.py +++ b/facilitator/fac.py @@ -196,7 +196,7 @@ def parse_transaction(line): if not (pos < len(line)): break if skipped == 0: - raise ValueError("Expected space before key-value pair") + raise ValueError("Expected space before key-value pair: %r" % line) pos, key = get_token(pos, line) if not (pos < len(line) and line[pos] == '='): raise ValueError("No '=' found after key") @@ -247,7 +247,7 @@ def transact(f, command, *params): f.flush() line = f.readline() if not (len(line) > 0 and line[-1] == '\n'): - raise ValueError("No newline at end of string returned by facilitator") + raise ValueError("No newline at end of string returned by facilitator: %r" % line) return parse_transaction(line[:-1])

def put_reg(facilitator_addr, client_addr, transport_chain, registrant_addr=None): @@ -316,7 +316,7 @@ def get_reg(facilitator_addr, proxy_addr, transport_list): response["relay"] = format_addr(relay) return response else: - raise ValueError("Facilitator response was not "OK"") + raise ValueError("Facilitator response was not "OK": %s: %s" % (command, params))

def put_reg_base64(b64): """Attempt to add a registration by running a facilitator-reg program diff --git a/facilitator/facilitator b/facilitator/facilitator index 647e1ed..e44047e 100755 --- a/facilitator/facilitator +++ b/facilitator/facilitator @@ -7,6 +7,7 @@ import socket import sys import threading import time +import traceback

import fac

@@ -228,7 +229,7 @@ class Handler(SocketServer.StreamRequestHandler): if not line: break num_lines += 1 - except socket.error, e: + except socket.error as e: log("socket error after reading %d lines: %s" % (num_lines, str(e))) break if not self.handle_line(line): @@ -239,52 +240,46 @@ class Handler(SocketServer.StreamRequestHandler): raise ValueError("No newline at end of string returned by readline") try: command, params = fac.parse_transaction(line[:-1]) - except ValueError, e: - log("fac.parse_transaction: %s" % e) - self.send_error() - return False + except ValueError as e: + return self.error("fac.parse_transaction: %(cause)s", e)

if command == "GET": return self.do_GET(params) elif command == "PUT": return self.do_PUT(params) else: - self.send_error() - return False + return self.error(u"unrecognized command: %s" % command)

def send_ok(self): print >> self.wfile, "OK"

- def send_error(self): - print >> self.wfile, "ERROR" + def error(self, msg="", cause=None): + msg = safe_str(msg % { "cause": cause }) + log(msg) + if not options.safe_logging: + log(traceback.format_exc()) + print >>self.wfile, fac.render_transaction("ERROR", ("MSG", msg)) + return False

# Handle a GET request (got flashproxy poll; need to return a proper client registration) # Example: GET FROM="3.3.3.3:3333" TRANSPORT="websocket" TRANSPORT="webrtc" def do_GET(self, params): proxy_spec = fac.param_first("FROM", params) if proxy_spec is None: - log(u"GET missing FROM param") - self.send_error() - return False + return self.error(u"GET missing FROM param") try: proxy_addr = fac.parse_addr_spec(proxy_spec, defport=0) - except ValueError, e: - log(u"syntax error in proxy address %s: %s" % (safe_str(repr(proxy_spec)), safe_str(repr(str(e))))) - self.send_error() - return False + except ValueError as e: + return self.error(u"syntax error in proxy address %s: %%(cause)s" % safe_str(repr(proxy_spec)), e)

transport_list = fac.param_getlist("TRANSPORT", params) if not transport_list: - log(u"TRANSPORT missing FROM param") - self.send_error() - return False + return self.error(u"TRANSPORT missing FROM param")

try: reg = get_reg_for_proxy(proxy_addr, transport_list) - except Exception, e: - log(u"error getting reg for proxy address %s: %s" % (safe_str(repr(proxy_spec)), safe_str(repr(str(e))))) - self.send_error() - return False + except Exception as e: + return self.error(u"error getting match for proxy address %s: %%(cause)s" % safe_str(repr(proxy_spec)), e)

check_back_in = get_check_back_in_for_proxy(proxy_addr)

@@ -299,14 +294,12 @@ class Handler(SocketServer.StreamRequestHandler): return True

# Handle a PUT request (client made a registration request; register it.) - # Example: PUT CLIENT="1.1.1.1:5555" FROM="1.1.1.2:6666" TRANSPORT_CHAIN="obfs3|websocket" + # Example: PUT CLIENT="1.1.1.1:5555" TRANSPORT_CHAIN="obfs3|websocket" def do_PUT(self, params): # Check out if we recognize the transport chain in this registration request transports_spec = fac.param_first("TRANSPORT_CHAIN", params) if transports_spec is None: - log(u"PUT missing TRANSPORT_CHAIN param") - self.send_error() - return False + return self.error(u"PUT missing TRANSPORT_CHAIN param")

transports = parse_transport_chain(transports_spec)

@@ -321,24 +314,18 @@ class Handler(SocketServer.StreamRequestHandler):

client_spec = fac.param_first("CLIENT", params) if client_spec is None: - log(u"PUT missing CLIENT param") - self.send_error() - return False + return self.error(u"PUT missing CLIENT param")

try: reg = Reg.parse(client_spec, transports) except (UnknownTransport, ValueError) as e: # XXX should we throw a better error message to the client? Is it possible? - log(u"syntax error in %s: %s" % (safe_str(repr(client_spec)), safe_str(repr(str(e))))) - self.send_error() - return False + return self.error(u"syntax error in %s: %%(cause)s" % safe_str(repr(client_spec)), e)

try: ok = put_reg(reg) - except Exception, e: - log(u"error putting reg %s: %s" % (safe_str(repr(client_spec)), safe_str(repr(str(e))))) - self.send_error() - return False + except Exception as e: + return self.error(u"error putting reg %s: %%(cause)s" % safe_str(repr(client_spec)), e)

if ok: log(u"client %s (transports: %s) (remaining regs: %d/%d)" % (safe_str(unicode(reg)), reg.transports, num_unhandled_regs(), num_regs())) @@ -528,7 +515,7 @@ obfs2|websocket 1.4.6.1:4123\ log(u"dropping privileges to those of user %s" % options.privdrop_username) try: fac.drop_privs(options.privdrop_username) - except BaseException, e: + except BaseException as e: print >> sys.stderr, "Can't drop privileges:", str(e) sys.exit(1)