From 5777ee0e1a8bf0652aff75bb2c316c5bbbb4b854 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 5 Mar 2006 09:50:26 +0000 Subject: [PATCH] Add some functions to escape values from the network before sending them to the log. Use them everywhere except for routerinfo->plaftorm, routerinfo->contact_info, and rend*.c. (need sleep now) svn:r6087 --- doc/TODO | 14 ++-- src/common/tortls.c | 3 +- src/common/util.c | 148 ++++++++++++++++++++++++++++++++++----- src/common/util.h | 3 + src/or/buffers.c | 18 ++++- src/or/circuituse.c | 2 +- src/or/command.c | 4 +- src/or/config.c | 10 +++ src/or/connection.c | 2 +- src/or/connection_edge.c | 9 ++- src/or/connection_or.c | 8 +-- src/or/directory.c | 85 +++++++++++----------- src/or/dirserv.c | 5 +- src/or/or.h | 1 + src/or/routerparse.c | 38 +++++----- src/or/test.c | 6 ++ 16 files changed, 261 insertions(+), 95 deletions(-) diff --git a/doc/TODO b/doc/TODO index 3a292f1372..7611f127b5 100644 --- a/doc/TODO +++ b/doc/TODO @@ -45,9 +45,13 @@ N - building on freebsd 6.0: (with multiple openssl installations) - authorities should *never* 503 a cache, but *should* 503 clients when they feel like it. - update dir-spec with what we decided for each of these - - when logging unknown http headers, this could include bad escape codes? - - more generally, attacker-controller log entries with newlines in them - are dangerous for our users. + o when logging unknown http headers, this could include bad escape codes? + more generally, attacker-controller log entries with newlines in them + are dangerous for our users. + o So... add functions to escape potentially malicious values before + logging them, and test values more closely as they arrive... + - But what to do about contact_info and platform? + - (Didn't finish converting rend*.c) - Make "setconf" and "hup" behavior cleaner for LINELIST config options (e.g. Log). Bug 238. R - Jan 26 10:25:04.832 [warn] add_an_entry_guard(): Tried finding a @@ -56,11 +60,11 @@ R - streamline how we define a guard node as 'up'. document it somewhere. R - reduce log severity for guard nodes. R - make guard node timeout higher. -N . Clean and future-proof exit policy formats a bit. + o Clean and future-proof exit policy formats a bit. o Likewise accept, but don't generate /bits formats (unless they're accepted in 0.0.9 and later). o Warn when we see a netmask that isn't a prefix. - - Make clients understand "private:*" in exit policies, even though + o Make clients understand "private:*" in exit policies, even though we don't generate it yet. for 0.1.1.x-final: diff --git a/src/common/tortls.c b/src/common/tortls.c index 5881261138..2acbcbb57d 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -672,7 +672,8 @@ tor_tls_get_peer_cert_nickname(tor_tls_t *tls, char *buf, size_t buflen) goto error; if (((int)strspn(buf, LEGAL_NICKNAME_CHARACTERS)) < lenout) { log_warn(LD_PROTOCOL, - "Peer certificate nickname \"%s\" has illegal characters.", buf); + "Peer certificate nickname %s has illegal characters.", + escaped(buf)); if (strchr(buf, '.')) log_warn(LD_PROTOCOL, " (Maybe it is not really running Tor at its " "advertised OR port.)"); diff --git a/src/common/util.c b/src/common/util.c index 7e4f0a1457..0ae109c1d1 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -329,6 +329,16 @@ tor_strupper(char *s) } } +int +tor_strisprint(const char *s) +{ + while (*s) { + if (!TOR_ISPRINT(*s)) + return 0; + } + return 1; +} + /* Compares the first strlen(s2) characters of s1 with s2. Returns as for * strcmp. */ @@ -564,6 +574,100 @@ base16_decode(char *dest, size_t destlen, const char *src, size_t srclen) return 0; } +/** Allocate and return a new string representing the contents of s, + * surrounded by quotes and using standard C escapes. + * + * Generally, we use this for logging values that come in over the network + * to keep them from tricking users. + * + * We trust values from the resolver, OS, configuration file, and command line + * to not be maliciously ill-formed. We validate incoming routerdescs and + * SOCKS requests and addresses from BEGIN cells as they're parsed; + * afterwards, we trust them as non-malicious. + */ +char * +esc_for_log(const char *s) +{ + const char *cp; + char *result, *outp; + size_t len = 3; + for (cp = s; *cp; ++cp) { + switch (*cp) { + case '\\': + case '\"': + case '\'': + len += 2; + break; + default: + if (TOR_ISPRINT(*cp)) + ++len; + else + len += 4; + break; + } + } + + result = outp = tor_malloc(len); + *outp++ = '\"'; + for (cp = s; *cp; ++cp) { + switch (*cp) { + case '\\': + case '\"': + case '\'': + *outp++ = '\\'; + *outp++ = *cp; + break; + case '\n': + *outp++ = '\\'; + *outp++ = 'n'; + break; + case '\t': + *outp++ = '\\'; + *outp++ = 't'; + break; + case '\r': + *outp++ = '\\'; + *outp++ = 'r'; + break; + default: + if (TOR_ISPRINT(*cp)) { + *outp++ = *cp; + } else { + tor_snprintf(outp, 5, "\\%03o", (uint8_t) *cp); + outp += 4; + } + break; + } + } + + *outp++ = '\"'; + *outp++ = 0; + + return result; +} + +/** Allocate and return a new string representing the contents of s, + * surrounded by quotes and using standard C escapes. + * + * THIS FUNCTION IS NOT REENTRANT. Don't call it from outside the main + * thread. Also, each call invalidates the last-returned value, so don't + * try log_warn(LD_GENERAL, "%s %s", escaped(a), escaped(b)); + */ +const char * +escaped(const char *s) +{ + static char *_escaped_val = NULL; + if (_escaped_val) + tor_free(_escaped_val); + + if (s) + _escaped_val = esc_for_log(s); + else + _escaped_val = NULL; + + return _escaped_val; +} + /* ===== * Time * ===== */ @@ -700,7 +804,9 @@ parse_rfc1123_time(const char *buf, time_t *t) if (sscanf(buf, "%3s, %d %3s %d %d:%d:%d GMT", weekday, &tm.tm_mday, month, &tm.tm_year, &tm.tm_hour, &tm.tm_min, &tm.tm_sec) < 7) { - log_warn(LD_GENERAL, "Got invalid RFC1123 time \"%s\"", buf); + char *esc = esc_for_log(buf); + log_warn(LD_GENERAL, "Got invalid RFC1123 time %s", esc); + tor_free(esc); return -1; } @@ -712,14 +818,18 @@ parse_rfc1123_time(const char *buf, time_t *t) } } if (m<0) { - log_warn(LD_GENERAL, "Got invalid RFC1123 time \"%s\"", buf); + char *esc = esc_for_log(buf); + log_warn(LD_GENERAL, "Got invalid RFC1123 time %s", esc); + tor_free(esc); return -1; } tm.tm_mon = m; if (tm.tm_year < 1970) { + char *esc = esc_for_log(buf); log_warn(LD_GENERAL, - "Got invalid RFC1123 time \"%s\". (Before 1970)", buf); + "Got invalid RFC1123 time %s. (Before 1970)", esc); + tor_free(esc); return -1; } tm.tm_year -= 1900; @@ -768,7 +878,9 @@ parse_iso_time(const char *cp, time_t *t) st_tm.tm_sec = second; #endif if (st_tm.tm_year < 70) { - log_warn(LD_GENERAL, "Got invalid ISO time \"%s\". (Before 1970)", cp); + char *esc = esc_for_log(cp); + log_warn(LD_GENERAL, "Got invalid ISO time %s. (Before 1970)", esc); + tor_free(esc); return -1; } *t = tor_timegm(&st_tm); @@ -1222,7 +1334,7 @@ expand_filename(const char *filename) home = getenv("HOME"); if (!home) { log_warn(LD_CONFIG, "Couldn't find $HOME environment variable while " - "expanding %s", filename); + "expanding \"%s\"", filename); return NULL; } home = tor_strdup(home); @@ -1385,13 +1497,15 @@ parse_addr_port(const char *addrport, char **address, uint32_t *addr, _address = tor_strndup(addrport, colon-addrport); _port = (int) tor_parse_long(colon+1,10,1,65535,NULL,NULL); if (!_port) { - log_warn(LD_GENERAL, "Port '%s' out of range", colon+1); + log_warn(LD_GENERAL, "Port %s out of range", escaped(colon+1)); ok = 0; } if (!port_out) { + char *esc_addrport = esc_for_log(addrport); log_warn(LD_GENERAL, - "Port '%s' given on '%s' when not required", - colon+1, addrport); + "Port %s given on %s when not required", + escaped(colon+1), esc_addrport); + tor_free(esc_addrport); ok = 0; } } else { @@ -1402,7 +1516,7 @@ parse_addr_port(const char *addrport, char **address, uint32_t *addr, if (addr) { /* There's an addr pointer, so we need to resolve the hostname. */ if (tor_lookup_hostname(_address,addr)) { - log_warn(LD_NET, "Couldn't look up '%s'", _address); + log_warn(LD_NET, "Couldn't look up %s", escaped(_address)); ok = 0; *addr = 0; } @@ -1464,13 +1578,13 @@ parse_port_range(const char *port, uint16_t *port_min_out, &endptr); if (*endptr || !*port_max_out) { log_warn(LD_GENERAL, - "Malformed port \"%s\" on address range rejecting.", - port); + "Malformed port %s on address range rejecting.", + escaped(port)); } } else if (*endptr || !*port_min_out) { log_warn(LD_GENERAL, - "Malformed port \"%s\" on address range; rejecting.", - port); + "Malformed port %s on address range; rejecting.", + escaped(port)); return -1; } else { *port_max_out = *port_min_out; @@ -1523,8 +1637,8 @@ parse_addr_and_port_range(const char *s, uint32_t *addr_out, } else if (tor_inet_aton(address, &in) != 0) { *addr_out = ntohl(in.s_addr); } else { - log_warn(LD_GENERAL, "Malformed IP \"%s\" in address pattern; rejecting.", - address); + log_warn(LD_GENERAL, "Malformed IP %s in address pattern; rejecting.", + escaped(address)); goto err; } @@ -1548,8 +1662,8 @@ parse_addr_and_port_range(const char *s, uint32_t *addr_out, *mask_out = ntohl(in.s_addr); } else { log_warn(LD_GENERAL, - "Malformed mask \"%s\" on address range; rejecting.", - mask); + "Malformed mask %s on address range; rejecting.", + escaped(mask)); goto err; } } diff --git a/src/common/util.h b/src/common/util.h index 64aca2392c..bed4f16c59 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -90,6 +90,7 @@ extern int dmalloc_free(const char *file, const int line, void *pnt, #define HEX_CHARACTERS "0123456789ABCDEFabcdef" void tor_strlower(char *s); void tor_strupper(char *s); +int tor_strisprint(const char *s); int strcmpstart(const char *s1, const char *s2); int strcasecmpstart(const char *s1, const char *s2); int strcmpend(const char *s1, const char *s2); @@ -112,6 +113,8 @@ const char *eat_whitespace(const char *s); const char *eat_whitespace_no_nl(const char *s); const char *find_whitespace(const char *s); int tor_digest_is_zero(const char *digest); +char *esc_for_log(const char *string); +const char *escaped(const char *string); void base16_encode(char *dest, size_t destlen, const char *src, size_t srclen); int base16_decode(char *dest, size_t destlen, const char *src, size_t srclen); diff --git a/src/or/buffers.c b/src/or/buffers.c index 71a0185957..0e1f38fb9d 100644 --- a/src/or/buffers.c +++ b/src/or/buffers.c @@ -1004,6 +1004,14 @@ fetch_from_buf_socks(buf_t *buf, socks_request_t *req, int log_sockstype) req->address[len] = 0; req->port = ntohs(get_uint16(buf->cur+5+len)); buf_remove_from_front(buf, 5+len+2); + if (!tor_strisprint(req->address) || strchr(req->address,'\"')) { + log_warn(LD_PROTOCOL, + "Your application (using socks5 on port %d) gave Tor " + "a malformed hostname: %s. Rejecting the connection.", + req->port, escaped(req->address)); + return -1; + } + if (log_sockstype) log_notice(LD_APP, "Your application (using socks5 on port %d) gave " @@ -1049,7 +1057,7 @@ fetch_from_buf_socks(buf_t *buf, socks_request_t *req, int log_sockstype) return -1; } log_debug(LD_APP, - "socks4: successfully read destip (%s)",safe_str(tmpbuf)); + "socks4: successfully read destip (%s)", safe_str(tmpbuf)); socks4_prot = socks4; } @@ -1088,6 +1096,7 @@ fetch_from_buf_socks(buf_t *buf, socks_request_t *req, int log_sockstype) return -1; } tor_assert(next < buf->cur+buf->datalen); + if (log_sockstype) log_notice(LD_APP, "Your application (using socks4a on port %d) gave " @@ -1097,6 +1106,13 @@ fetch_from_buf_socks(buf_t *buf, socks_request_t *req, int log_sockstype) log_debug(LD_APP,"socks4: Everything is here. Success."); strlcpy(req->address, startaddr ? startaddr : tmpbuf, sizeof(req->address)); + if (!tor_strisprint(req->address) || strchr(req->address,'\"')) { + log_warn(LD_PROTOCOL, + "Your application (using socks4 on port %d) gave Tor " + "a malformed hostname: %s. Rejecting the connection.", + req->port, escaped(req->address)); + return -1; + } /* next points to the final \0 on inbuf */ buf_remove_from_front(buf, next-buf->cur+1); return 1; diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 93563cfc58..e3b63b6663 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -733,7 +733,7 @@ circuit_build_failed(circuit_t *circ) log_info(LD_REND, "Couldn't connect to Alice's chosen rend point %s " "(%s hop failed).", - build_state_get_exit_nickname(circ->build_state), + escaped(build_state_get_exit_nickname(circ->build_state)), failed_at_last_hop?"last":"non-last"); rend_service_relaunch_rendezvous(circ); break; diff --git a/src/or/command.c b/src/or/command.c index 590f3e181c..de13627d1c 100644 --- a/src/or/command.c +++ b/src/or/command.c @@ -202,8 +202,8 @@ command_process_create_cell(cell_t *cell, connection_t *conn) cell->circ_id, (int)(time(NULL) - conn->timestamp_created)); if (router) log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL, - "Details: nickname '%s', platform '%s'.", - router->nickname, router->platform); + "Details: nickname \"%s\", platform %s.", + router->nickname, escaped(router->platform)); return; } diff --git a/src/or/config.c b/src/or/config.c index b97c07fb85..fa6cb8e176 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -518,6 +518,16 @@ safe_str(const char *address) return address; } +/** DOCDOC */ +const char * +escaped_safe_str(const char *address) +{ + if (get_options()->SafeLogging) + return "[scrubbed]"; + else + return escaped(address); +} + /** Add the default directory servers directly into the trusted dir list. */ static void add_default_trusted_dirservers(void) diff --git a/src/or/connection.c b/src/or/connection.c index 9386a83016..d99c5a2e84 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -772,7 +772,7 @@ connection_connect(connection_t *conn, char *address, } else if (!SOCKET_IS_POLLABLE(s)) { log_warn(LD_NET, "Too many connections; can't create pollable connection to %s", - safe_str(address)); + safe_str(address)); tor_close_socket(s); return -1; } diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index f3d6fffd1d..b7ac813808 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -945,7 +945,8 @@ connection_ap_handshake_rewrite_and_attach(connection_t *conn) hostname_type_t addresstype; tor_strlower(socks->address); /* normalize it */ - log_debug(LD_APP,"Client asked for %s:%d", safe_str(socks->address), + log_debug(LD_APP,"Client asked for %s:%d", + safe_str(socks->address), socks->port); /* For address map controls, remap the address */ @@ -1519,6 +1520,12 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ) tor_free(address); return 0; } + if (!tor_strisprint(address)) { + log_warn(LD_PROTOCOL,"Non-printing characters in address %s in relay " + "begin cell. Dropping.", escaped(address)); + tor_free(address); + return 0; + } log_debug(LD_EXIT,"Creating new exit connection."); n_stream = connection_new(CONN_TYPE_EXIT); diff --git a/src/or/connection_or.c b/src/or/connection_or.c index c70c573e68..ee4d3915e5 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -181,8 +181,8 @@ connection_or_read_proxy_response(connection_t *conn) if (status_code == 200) { log_info(LD_OR, - "HTTPS connect to '%s' successful! (200 \"%s\") Starting TLS.", - conn->address, reason); + "HTTPS connect to '%s' successful! (200 %s) Starting TLS.", + conn->address, escaped(reason)); tor_free(reason); if (connection_tls_start_handshake(conn, 0) < 0) { /* TLS handshaking error of some kind. */ @@ -194,9 +194,9 @@ connection_or_read_proxy_response(connection_t *conn) } /* else, bad news on the status code */ log_warn(LD_OR, - "The https proxy sent back an unexpected status code %d (\"%s\"). " + "The https proxy sent back an unexpected status code %d (%s). " "Closing.", - status_code, reason); + status_code, escaped(reason)); tor_free(reason); connection_mark_for_close(conn); return -1; diff --git a/src/or/directory.c b/src/or/directory.c index 3a74fc2353..58c989dea6 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -537,7 +537,7 @@ directory_send_command(connection_t *conn, const char *platform, tor_assert(!payload); log_debug(LD_DIR, "Asking for compressed directory from server running %s", - platform?platform:""); + platform?escaped(platform):""); httpcommand = "GET"; url = tor_strdup("/tor/dir.z"); break; @@ -712,7 +712,8 @@ http_get_origin(const char *headers, connection_t *conn) if (fwd) { size_t len = strlen(fwd)+strlen(conn->address)+32; char *result = tor_malloc(len); - tor_snprintf(result, len, "'%s' (forwarded for '%s')", conn->address, fwd); + tor_snprintf(result, len, "'%s' (forwarded for %s)", conn->address, + escaped(fwd)); tor_free(fwd); return result; } else { @@ -754,7 +755,7 @@ parse_http_response(const char *headers, int *code, time_t *date, if (sscanf(headers, "HTTP/1.%d %d", &n1, &n2) < 2 || (n1 != 0 && n1 != 1) || (n2 < 100 || n2 >= 600)) { - log_warn(LD_HTTP,"Failed to parse header '%s'",headers); + log_warn(LD_HTTP,"Failed to parse header %s",escaped(headers)); return -1; } *code = n2; @@ -801,8 +802,8 @@ parse_http_response(const char *headers, int *code, time_t *date, } else if (!strcmp(enc, "gzip") || !strcmp(enc, "x-gzip")) { *compression = GZIP_METHOD; } else { - log_info(LD_HTTP, "Unrecognized content encoding: '%s'. Trying to deal.", - enc); + log_info(LD_HTTP, "Unrecognized content encoding: %s. Trying to deal.", + escaped(enc)); *compression = -1; } } @@ -891,8 +892,8 @@ connection_dir_client_reached_eof(connection_t *conn) if (!reason) reason = tor_strdup("[no reason given]"); log_debug(LD_DIR, - "Received response from directory server '%s:%d': %d \"%s\"", - conn->address, conn->port, status_code, reason); + "Received response from directory server '%s:%d': %d %s", + conn->address, conn->port, status_code, escaped(reason)); if (date_header > 0) { now = time(NULL); @@ -914,9 +915,9 @@ connection_dir_client_reached_eof(connection_t *conn) } if (status_code == 503) { - log_info(LD_DIR,"Received http status code %d (\"%s\") from server " + log_info(LD_DIR,"Received http status code %d (%s) from server " "'%s:%d'. I'll try again soon.", - status_code, reason, conn->address, conn->port); + status_code, escaped(reason), conn->address, conn->port); tor_free(body); tor_free(headers); tor_free(reason); return -1; } @@ -982,9 +983,9 @@ connection_dir_client_reached_eof(connection_t *conn) log_info(LD_DIR,"Received directory (size %d) from server '%s:%d'", (int)body_len, conn->address, conn->port); if (status_code != 200) { - log_warn(LD_DIR,"Received http status code %d (\"%s\") from server " + log_warn(LD_DIR,"Received http status code %d (%s) from server " "'%s:%d'. I'll try again soon.", - status_code, reason, conn->address, conn->port); + status_code, escaped(reason), conn->address, conn->port); tor_free(body); tor_free(headers); tor_free(reason); return -1; } @@ -999,9 +1000,9 @@ connection_dir_client_reached_eof(connection_t *conn) /* just update our list of running routers, if this list is new info */ log_info(LD_DIR,"Received running-routers list (size %d)", (int)body_len); if (status_code != 200) { - log_warn(LD_DIR,"Received http status code %d (\"%s\") from server " + log_warn(LD_DIR,"Received http status code %d (%s) from server " "'%s:%d'. I'll try again soon.", - status_code, reason, conn->address, conn->port); + status_code, escaped(reason), conn->address, conn->port); tor_free(body); tor_free(headers); tor_free(reason); return -1; } @@ -1023,9 +1024,9 @@ connection_dir_client_reached_eof(connection_t *conn) "'%s:%d'",(int) body_len, conn->address, conn->port); if (status_code != 200) { log_warn(LD_DIR, - "Received http status code %d (\"%s\") from server " + "Received http status code %d (%s) from server " "'%s:%d' while fetching \"/tor/status/%s\". I'll try again soon.", - status_code, reason, conn->address, conn->port, + status_code, escaped(reason), conn->address, conn->port, conn->requested_resource); tor_free(body); tor_free(headers); tor_free(reason); connection_dir_download_networkstatus_failed(conn); @@ -1083,9 +1084,9 @@ connection_dir_client_reached_eof(connection_t *conn) /* 404 means that it didn't have them; no big deal. * Older (pre-0.1.1.8) servers said 400 Servers unavailable instead. */ log_fn(dir_okay ? LOG_INFO : LOG_WARN, LD_DIR, - "Received http status code %d (\"%s\") from server '%s:%d' " + "Received http status code %d (%s) from server '%s:%d' " "while fetching \"/tor/server/%s\". I'll try again soon.", - status_code, reason, conn->address, conn->port, + status_code, escaped(reason), conn->address, conn->port, conn->requested_resource); if (!which) { connection_dir_download_routerdesc_failed(conn); @@ -1137,22 +1138,22 @@ connection_dir_client_reached_eof(connection_t *conn) "descriptor: finished."); break; case 400: - log_warn(LD_GENERAL,"http status 400 (\"%s\") response from " + log_warn(LD_GENERAL,"http status 400 (%s) response from " "dirserver '%s:%d'. Please correct.", - reason, conn->address, conn->port); + escaped(reason), conn->address, conn->port); break; case 403: log_warn(LD_GENERAL, - "http status 403 (\"%s\") response from dirserver " + "http status 403 (%s) response from dirserver " "'%s:%d'. Is your clock skewed? Have you mailed us your key " "fingerprint? Are you using the right key? Are you using a " "private IP address? See http://tor.eff.org/doc/" - "tor-doc-server.html", reason, conn->address, conn->port); + "tor-doc-server.html",escaped(reason), conn->address, conn->port); break; default: log_warn(LD_GENERAL, - "http status %d (\"%s\") reason unexpected (server '%s:%d').", - status_code, reason, conn->address, conn->port); + "http status %d (%s) reason unexpected (server '%s:%d').", + status_code, escaped(reason), conn->address, conn->port); break; } /* return 0 in all cases, since we don't want to mark any @@ -1161,8 +1162,8 @@ connection_dir_client_reached_eof(connection_t *conn) if (conn->purpose == DIR_PURPOSE_FETCH_RENDDESC) { log_info(LD_REND,"Received rendezvous descriptor (size %d, status %d " - "(\"%s\"))", - (int)body_len, status_code, reason); + "(%s))", + (int)body_len, status_code, escaped(reason)); switch (status_code) { case 200: if (rend_cache_store(body, body_len) < 0) { @@ -1181,13 +1182,13 @@ connection_dir_client_reached_eof(connection_t *conn) break; case 400: log_warn(LD_REND, - "http status 400 (\"%s\"). Dirserver didn't like our " - "rendezvous query?", reason); + "http status 400 (%s). Dirserver didn't like our " + "rendezvous query?", escaped(reason)); break; default: - log_warn(LD_REND,"http status %d (\"%s\") response unexpected (server " - "'%s:%d').", - status_code, reason, conn->address, conn->port); + log_warn(LD_REND,"http status %d (%s) response unexpected (server " + "'%s:%d').", + status_code, escaped(reason), conn->address, conn->port); break; } } @@ -1197,17 +1198,17 @@ connection_dir_client_reached_eof(connection_t *conn) case 200: log_info(LD_REND, "Uploading rendezvous descriptor: finished with status " - "200 (\"%s\")", reason); + "200 (%s)", escaped(reason)); break; case 400: - log_warn(LD_REND,"http status 400 (\"%s\") response from dirserver " + log_warn(LD_REND,"http status 400 (%s) response from dirserver " "'%s:%d'. Malformed rendezvous descriptor?", - reason, conn->address, conn->port); + escaped(reason), conn->address, conn->port); break; default: - log_warn(LD_REND,"http status %d (\"%s\") response unexpected (server " + log_warn(LD_REND,"http status %d (%s) response unexpected (server " "'%s:%d').", - status_code, reason, conn->address, conn->port); + status_code, escaped(reason), conn->address, conn->port); break; } } @@ -1728,7 +1729,7 @@ directory_handle_command_post(connection_t *conn, char *headers, #if 0 if (body_len <= 1024) { base16_encode(tmp, sizeof(tmp), body, body_len); - log_notice(LD_DIRSERV,"Body was: %s", tmp); + log_notice(LD_DIRSERV,"Body was: %s", escaped(tmp)); } #endif write_http_status_line(conn, 400, "Invalid service descriptor rejected"); @@ -1775,15 +1776,15 @@ directory_handle_command(connection_t *conn) /* case 1, fall through */ } - log_debug(LD_DIRSERV,"headers '%s', body '%s'.", headers, body); + //log_debug(LD_DIRSERV,"headers %s, body %s.", headers, body); if (!strncasecmp(headers,"GET",3)) r = directory_handle_command_get(conn, headers, body, body_len); else if (!strncasecmp(headers,"POST",4)) r = directory_handle_command_post(conn, headers, body, body_len); else { - log_warn(LD_PROTOCOL,"Got headers '%s' with unknown command. Closing.", - headers); + log_warn(LD_PROTOCOL,"Got headers %s with unknown command. Closing.", + escaped(headers)); r = -1; } @@ -1904,7 +1905,7 @@ dir_routerdesc_download_failed(smartlist_t *failed) /* update_router_descriptor_downloads(time(NULL)); */ } -/* Given a directory resource request generated by us, containing zero +/* Given a directory resource request, containing zero * or more strings separated by plus signs, followed optionally by ".z", store * the strings, in order, into fp_out. If compressed_out is * non-NULL, set it to 1 if the resource ends in ".z", else set it to 0. If @@ -1938,13 +1939,13 @@ dir_split_resource_into_fingerprints(const char *resource, cp = smartlist_get(fp_out, i); if (strlen(cp) != HEX_DIGEST_LEN) { log_info(LD_DIR, - "Skipping digest \"%s\" with non-standard length.", cp); + "Skipping digest %s with non-standard length.", escaped(cp)); smartlist_del(fp_out, i--); goto again; } d = tor_malloc_zero(DIGEST_LEN); if (base16_decode(d, DIGEST_LEN, cp, HEX_DIGEST_LEN)<0) { - log_info(LD_DIR, "Skipping non-decodable digest \"%s\"", cp); + log_info(LD_DIR, "Skipping non-decodable digest %s", escaped(cp)); smartlist_del(fp_out, i--); goto again; } diff --git a/src/or/dirserv.c b/src/or/dirserv.c index 4c8b05fbc2..379cc85c7e 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -346,13 +346,14 @@ dirserv_get_status_impl(const char *fp, const char *nickname, log_debug(LD_DIRSERV,"Good fingerprint for '%s'",nickname); return FP_NAMED; /* Right fingerprint. */ } else { - if (should_log) + if (should_log) { log_warn(LD_DIRSERV, "Mismatched fingerprint for '%s': expected '%s' got '%s'. " "ContactInfo '%s', platform '%s'.)", nickname, nn_ent->fingerprint, fp, contact ? contact : "", - platform ? platform : ""); + platform ? escaped(platform) : ""); + } if (msg) *msg = "Rejected: There is already a verified server with this nickname " "and a different fingerprint."; diff --git a/src/or/or.h b/src/or/or.h index efaa00235e..5b0f576aa4 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1591,6 +1591,7 @@ or_options_t *get_options(void); int set_options(or_options_t *new_val); void config_free_all(void); const char *safe_str(const char *address); +const char *escaped_safe_str(const char *address); int config_get_lines(char *string, config_line_t **result); void config_free_lines(config_line_t *front); diff --git a/src/or/routerparse.c b/src/or/routerparse.c index c5df489f35..b4ad4cfef0 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -468,8 +468,8 @@ router_parse_runningrouters(const char *str) log_warn(LD_DIR, "Error tokenizing directory"); goto err; } if ((tok = find_first_by_keyword(tokens, _UNRECOGNIZED))) { - log_warn(LD_DIR, "Unrecognized keyword '%s'; can't parse running-routers", - tok->args[0]); + log_warn(LD_DIR, "Unrecognized keyword %s; can't parse running-routers", + escaped(tok->args[0])); goto err; } tok = smartlist_get(tokens,0); @@ -736,9 +736,9 @@ router_parse_entry_from_string(const char *s, const char *end) } if ((tok = find_first_by_keyword(tokens, _UNRECOGNIZED))) { log_warn(LD_DIR, - "Unrecognized critical keyword '%s'; skipping descriptor. " + "Unrecognized critical keyword %s; skipping descriptor. " "(It may be from another version of Tor.)", - tok->args[0]); + escaped(tok->args[0])); goto err; } @@ -852,7 +852,8 @@ router_parse_entry_from_string(const char *s, const char *end) } tor_strstrip(tok->args[0], " "); if (base16_decode(d, DIGEST_LEN, tok->args[0], strlen(tok->args[0]))) { - log_warn(LD_DIR, "Couldn't decode fingerprint '%s'", tok->args[0]); + log_warn(LD_DIR, "Couldn't decode fingerprint %s", + escaped(tok->args[0])); goto err; } if (memcmp(d,router->cache_info.identity_digest, DIGEST_LEN)!=0) { @@ -882,7 +883,8 @@ router_parse_entry_from_string(const char *s, const char *end) router->declared_family = smartlist_create(); for (i=0;in_args;++i) { if (!is_legal_nickname_or_hexdigest(tok->args[i])) { - log_warn(LD_DIR, "Illegal nickname '%s' in family line", tok->args[i]); + log_warn(LD_DIR, "Illegal nickname %s in family line", + escaped(tok->args[i])); goto err; } smartlist_add(router->declared_family, tor_strdup(tok->args[i])); @@ -977,8 +979,8 @@ routerstatus_parse_entry_from_string(const char **s, smartlist_t *tokens) goto err; } if ((tok = find_first_by_keyword(tokens, _UNRECOGNIZED))) { - log_warn(LD_DIR, "Unrecognized keyword \"%s\" in router status; skipping.", - tok->args[0]); + log_warn(LD_DIR, "Unrecognized keyword %s in router status; skipping.", + escaped(tok->args[0])); goto err; } if (!(tok = find_first_by_keyword(tokens, K_R))) { @@ -993,19 +995,19 @@ routerstatus_parse_entry_from_string(const char **s, smartlist_t *tokens) if (!is_legal_nickname(tok->args[0])) { log_warn(LD_DIR, - "Invalid nickname '%s' in router status; skipping.", - tok->args[0]); + "Invalid nickname %s in router status; skipping.", + escaped(tok->args[0])); goto err; } strlcpy(rs->nickname, tok->args[0], sizeof(rs->nickname)); if (digest_from_base64(rs->identity_digest, tok->args[1])) { - log_warn(LD_DIR, "Error decoding digest '%s'", tok->args[1]); + log_warn(LD_DIR, "Error decoding digest %s", escaped(tok->args[1])); goto err; } if (digest_from_base64(rs->descriptor_digest, tok->args[2])) { - log_warn(LD_DIR, "Error decoding digest '%s'", tok->args[2]); + log_warn(LD_DIR, "Error decoding digest %s", escaped(tok->args[2])); goto err; } @@ -1018,7 +1020,7 @@ routerstatus_parse_entry_from_string(const char **s, smartlist_t *tokens) } if (tor_inet_aton(tok->args[5], &in) == 0) { - log_warn(LD_DIR, "Error parsing address '%s'", tok->args[5]); + log_warn(LD_DIR, "Error parsing address '%s'", escaped(tok->args[5])); goto err; } rs->addr = ntohl(in.s_addr); @@ -1097,8 +1099,8 @@ networkstatus_parse_from_string(const char *s) goto err; } if ((tok = find_first_by_keyword(tokens, _UNRECOGNIZED))) { - log_warn(LD_DIR, "Unrecognized keyword '%s'; can't parse network-status", - tok->args[0]); + log_warn(LD_DIR, "Unrecognized keyword %s; can't parse network-status", + escaped(tok->args[0])); goto err; } ns = tor_malloc_zero(sizeof(networkstatus_t)); @@ -1119,7 +1121,7 @@ networkstatus_parse_from_string(const char *s) } ns->source_address = tok->args[0]; tok->args[0] = NULL; if (tor_inet_aton(tok->args[1], &in) == 0) { - log_warn(LD_DIR, "Error parsing address '%s'", tok->args[1]); + log_warn(LD_DIR, "Error parsing address %s", escaped(tok->args[1])); goto err; } ns->source_addr = ntohl(in.s_addr); @@ -1140,7 +1142,7 @@ networkstatus_parse_from_string(const char *s) } if (base16_decode(ns->identity_digest, DIGEST_LEN, tok->args[0], strlen(tok->args[0]))) { - log_warn(LD_DIR, "Couldn't decode fingerprint '%s'", tok->args[0]); + log_warn(LD_DIR, "Couldn't decode fingerprint %s", escaped(tok->args[0])); goto err; } @@ -1369,7 +1371,7 @@ router_parse_addr_policy(directory_token_t *tok) policy_read_failed: tor_assert(newe->string); - log_warn(LD_DIR,"Couldn't parse line '%s'. Dropping", newe->string); + log_warn(LD_DIR,"Couldn't parse line %s. Dropping", escaped(newe->string)); tor_free(newe->string); tor_free(newe); return NULL; diff --git a/src/or/test.c b/src/or/test.c index c55ec5f4dc..2002c6a234 100644 --- a/src/or/test.c +++ b/src/or/test.c @@ -913,6 +913,12 @@ test_util(void) test_streq(tmpbuf, "18.244.0.188"); } + /* Test 'escaped' */ + test_streq("\"\"", escaped("")); + test_streq("\"abcd\"", escaped("abcd")); + test_streq("\"\\\\\\n\\r\\t\\\"\\'\"", escaped("\\\n\r\t\"\'")); + test_streq("\"z\\001abc\\277d\"", escaped("z\001abc\277d")); + /* XXXX test older functions. */ smartlist_free(sl); }