[or-cvs] r8770: r9289 at 31-35-219: nickm | 2006-10-20 09:43:22 -0400 Fix longs (in tor/trunk: . doc src/or)
nickm at seul.org
nickm at seul.org
Fri Oct 20 14:58:12 UTC 2006
Author: nickm
Date: 2006-10-20 10:57:46 -0400 (Fri, 20 Oct 2006)
New Revision: 8770
Modified:
tor/trunk/
tor/trunk/ChangeLog
tor/trunk/doc/TODO
tor/trunk/src/or/connection_edge.c
tor/trunk/src/or/or.h
tor/trunk/src/or/relay.c
Log:
r9289 at 31-35-219: nickm | 2006-10-20 09:43:22 -0400
Fix longstanding bug in connection_exit_begin_conn(): Since connection_edge_end() exits when the connection is unattached, we were never sending RELAY_END cells back for failed RELAY_BEGIN attempts. Fix this. This might make clients that were otherwise timing out either fail faster or retry faster, which is good news for us.
Property changes on: tor/trunk
___________________________________________________________________
svk:merge ticket from /tor/trunk [r9289] on c95137ef-5f19-0410-b913-86e773d04f59
Modified: tor/trunk/ChangeLog
===================================================================
--- tor/trunk/ChangeLog 2006-10-20 01:13:47 UTC (rev 8769)
+++ tor/trunk/ChangeLog 2006-10-20 14:57:46 UTC (rev 8770)
@@ -48,6 +48,10 @@
- Detect the size of the routers file correctly even if it is corrupted
(on systems without mmap) or not page-aligned (on systems with mmap).
This bug was harmless.
+ - Implement the protocol correctly by always sending a RELAY_END cell
+ when an attempt to open a stream fails. This should make clients
+ able to find a good exit faster in some cases, since unhandleable
+ requests will now get an error rather than timing out.
Changes in version 0.1.2.2-alpha - 2006-10-07
Modified: tor/trunk/doc/TODO
===================================================================
--- tor/trunk/doc/TODO 2006-10-20 01:13:47 UTC (rev 8769)
+++ tor/trunk/doc/TODO 2006-10-20 14:57:46 UTC (rev 8770)
@@ -57,9 +57,9 @@
- Use for something, so we can be sure it works.
- Test and debug
-N - Send back RELAY_END cells on malformed RELAY_BEGIN.
+ o Send back RELAY_END cells on malformed RELAY_BEGIN.
-N - Change the circuit end reason display a little for reasons from
+ o Change the circuit end reason display a little for reasons from
destroyed/truncated circuits. We want to indicate both that we're
closing because somebody told us to, and why they told us they wanted to
close.
Modified: tor/trunk/src/or/connection_edge.c
===================================================================
--- tor/trunk/src/or/connection_edge.c 2006-10-20 01:13:47 UTC (rev 8769)
+++ tor/trunk/src/or/connection_edge.c 2006-10-20 14:57:46 UTC (rev 8770)
@@ -1838,36 +1838,51 @@
relay_header_t rh;
char *address=NULL;
uint16_t port;
+ char end_payload[1];
assert_circuit_ok(circ);
- /* XXX currently we don't send an end cell back if we drop the
- * begin because it's malformed.
- */
+ relay_header_unpack(&rh, cell->payload);
+ /* Note: we have to use relay_send_command_from_edge here, not
+ * connection_edge_end or connection_edge_send_command, since those require
+ * that we have a stream connected to a circuit, and we don't connect to a
+ * circuit unitl we have a pending/sucessful resolve. */
+
if (!server_mode(get_options()) &&
circ->purpose != CIRCUIT_PURPOSE_S_REND_JOINED) {
log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
"Relay begin cell at non-server. Dropping.");
+ end_payload[0] = END_STREAM_REASON_EXITPOLICY;
+ relay_send_command_from_edge(rh.stream_id, circ, RELAY_COMMAND_END,
+ end_payload, 1, NULL);
return 0;
}
- relay_header_unpack(&rh, cell->payload);
if (rh.command == RELAY_COMMAND_BEGIN) {
if (!memchr(cell->payload+RELAY_HEADER_SIZE, 0, rh.length)) {
log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
"Relay begin cell has no \\0. Dropping.");
+ end_payload[0] = END_STREAM_REASON_TORPROTOCOL;
+ relay_send_command_from_edge(rh.stream_id, circ, RELAY_COMMAND_END,
+ end_payload, 1, NULL);
return 0;
}
if (parse_addr_port(LOG_PROTOCOL_WARN, cell->payload+RELAY_HEADER_SIZE,
&address,NULL,&port)<0) {
log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
"Unable to parse addr:port in relay begin cell. Dropping.");
+ end_payload[0] = END_STREAM_REASON_TORPROTOCOL;
+ relay_send_command_from_edge(rh.stream_id, circ, RELAY_COMMAND_END,
+ end_payload, 1, NULL);
return 0;
}
if (port==0) {
log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
"Missing port in relay begin cell. Dropping.");
+ end_payload[0] = END_STREAM_REASON_TORPROTOCOL;
+ relay_send_command_from_edge(rh.stream_id, circ, RELAY_COMMAND_END,
+ end_payload, 1, NULL);
tor_free(address);
return 0;
}
@@ -1876,6 +1891,9 @@
log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
"Non-printing characters in address %s in relay "
"begin cell. Dropping.", escaped(address));
+ end_payload[0] = END_STREAM_REASON_TORPROTOCOL;
+ relay_send_command_from_edge(rh.stream_id, circ, RELAY_COMMAND_END,
+ end_payload, 1, NULL);
tor_free(address);
return 0;
}
@@ -1886,15 +1904,27 @@
*/
log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
"Attempt to open a stream on first hop of circuit. Dropping.");
+ end_payload[0] = END_STREAM_REASON_TORPROTOCOL;
+ relay_send_command_from_edge(rh.stream_id, circ, RELAY_COMMAND_END,
+ end_payload, 1, NULL);
tor_free(address);
return 0;
}
} else if (rh.command == RELAY_COMMAND_BEGIN_DIR) {
or_options_t *options = get_options();
+ port = options->DirPort; /* not actually used to open a connection */
+ if (!port || circ->purpose != CIRCUIT_PURPOSE_OR) {
+ end_payload[0] = END_STREAM_REASON_NOTDIRECTORY;
+ relay_send_command_from_edge(rh.stream_id, circ, RELAY_COMMAND_END,
+ end_payload, 1, NULL);
+ return 0;
+ }
address = tor_strdup("127.0.0.1");
- port = options->DirPort; /* not actually used. */
} else {
log_warn(LD_BUG, "Got an unexpected command %d", (int)rh.command);
+ end_payload[0] = END_STREAM_REASON_INTERNAL;
+ relay_send_command_from_edge(rh.stream_id, circ, RELAY_COMMAND_END,
+ end_payload, 1, NULL);
return 0;
}
@@ -1908,15 +1938,6 @@
n_stream->package_window = STREAMWINDOW_START;
n_stream->deliver_window = STREAMWINDOW_START;
- if (rh.command == RELAY_COMMAND_BEGIN_DIR &&
- (!get_options()->DirPort || circ->purpose != CIRCUIT_PURPOSE_OR)) {
- connection_edge_end(n_stream, END_STREAM_REASON_NOTDIRECTORY,
- n_stream->cpath_layer);
- connection_free(TO_CONN(n_stream));
- tor_free(address);
- return 0;
- }
-
if (circ->purpose == CIRCUIT_PURPOSE_S_REND_JOINED) {
origin_circuit_t *origin_circ = TO_ORIGIN_CIRCUIT(circ);
log_debug(LD_REND,"begin is for rendezvous. configuring stream.");
@@ -1929,8 +1950,9 @@
if (rend_service_set_connection_addr_port(n_stream, origin_circ) < 0) {
log_info(LD_REND,"Didn't find rendezvous service (port %d)",
n_stream->_base.port);
- connection_edge_end(n_stream, END_STREAM_REASON_EXITPOLICY,
- n_stream->cpath_layer);
+ end_payload[0] = END_STREAM_REASON_EXITPOLICY;
+ relay_send_command_from_edge(rh.stream_id, circ, RELAY_COMMAND_END,
+ end_payload, 1, NULL);
connection_free(TO_CONN(n_stream));
/* knock the whole thing down, somebody screwed up */
circuit_mark_for_close(circ, END_CIRC_REASON_CONNECTFAILED);
@@ -1957,8 +1979,9 @@
/* default to failed, change in dns_resolve if it turns out not to fail */
if (we_are_hibernating()) {
- connection_edge_end(n_stream, END_STREAM_REASON_HIBERNATING,
- n_stream->cpath_layer);
+ end_payload[0] = END_STREAM_REASON_HIBERNATING;
+ relay_send_command_from_edge(rh.stream_id, circ, RELAY_COMMAND_END,
+ end_payload, 1, NULL);
connection_free(TO_CONN(n_stream));
return 0;
}
@@ -1985,7 +2008,9 @@
connection_exit_connect(n_stream);
return 0;
case -1: /* resolve failed */
- /* XXXX send back indication of failure for connect case? -NM*/
+ end_payload[0] = END_STREAM_REASON_RESOLVEFAILED;
+ relay_send_command_from_edge(rh.stream_id, circ, RELAY_COMMAND_END,
+ end_payload, 1, NULL);
/* n_stream got freed. don't touch it. */
break;
case 0: /* resolve added to pending list */
@@ -2157,7 +2182,7 @@
if ((err = tor_socketpair(AF_UNIX, SOCK_STREAM, 0, fd)) < 0) {
log_warn(LD_NET,
- "Couldn't construct socketpair (%s). Network down? Delaying.",
+ "Couldn't construct socketpair (%s). Out of sockets?",
tor_socket_strerror(-err));
connection_edge_end(exit_conn, END_STREAM_REASON_RESOURCELIMIT,
exit_conn->cpath_layer);
Modified: tor/trunk/src/or/or.h
===================================================================
--- tor/trunk/src/or/or.h 2006-10-20 01:13:47 UTC (rev 8769)
+++ tor/trunk/src/or/or.h 2006-10-20 14:57:46 UTC (rev 8770)
@@ -2332,6 +2332,9 @@
void relay_header_pack(char *dest, const relay_header_t *src);
void relay_header_unpack(relay_header_t *dest, const char *src);
+int relay_send_command_from_edge(uint16_t stream_id, circuit_t *circ,
+ int relay_command, const char *payload,
+ size_t payload_len, crypt_path_t *cpath_layer);
int connection_edge_send_command(edge_connection_t *fromconn, circuit_t *circ,
int relay_command, const char *payload,
size_t payload_len,
Modified: tor/trunk/src/or/relay.c
===================================================================
--- tor/trunk/src/or/relay.c 2006-10-20 01:13:47 UTC (rev 8769)
+++ tor/trunk/src/or/relay.c 2006-10-20 14:57:46 UTC (rev 8770)
@@ -446,17 +446,17 @@
dest->length = ntohs(get_uint16(src+9));
}
-/** Make a relay cell out of <b>relay_command</b> and <b>payload</b>, and
- * send it onto the open circuit <b>circ</b>. <b>fromconn</b> is the stream
- * that's sending the relay cell, or NULL if it's a control cell.
- * <b>cpath_layer</b> is NULL for OR->OP cells, or the destination hop
- * for OP->OR cells.
+/** Make a relay cell out of <b>relay_command</b> and <b>payload</b>, and send
+ * it onto the open circuit <b>circ</b>. <b>stream_id</b> is the ID on
+ * <b>circ</b> for the stream that's sending the relay cell, or 0 if it's a
+ * control cell. <b>cpath_layer</b> is NULL for OR->OP cells, or the
+ * destination hop for OP->OR cells.
*
- * If you can't send the cell, mark the circuit for close and
- * return -1. Else return 0.
+ * If you can't send the cell, mark the circuit for close and return -1. Else
+ * return 0.
*/
int
-connection_edge_send_command(edge_connection_t *fromconn, circuit_t *circ,
+relay_send_command_from_edge(uint16_t stream_id, circuit_t *circ,
int relay_command, const char *payload,
size_t payload_len, crypt_path_t *cpath_layer)
{
@@ -465,27 +465,8 @@
int cell_direction;
/* XXXX NM Split this function into a separate versions per circuit type? */
- if (fromconn && fromconn->_base.marked_for_close) {
- log_warn(LD_BUG,
- "Bug: called on conn that's already marked for close at %s:%d.",
- fromconn->_base.marked_for_close_file,
- fromconn->_base.marked_for_close);
- return 0;
- }
+ tor_assert(circ);
- if (!circ) {
- tor_assert(fromconn);
- if (fromconn->_base.type == CONN_TYPE_AP) {
- log_info(LD_APP,"no circ. Closing conn.");
- connection_mark_unattached_ap(fromconn, END_STREAM_REASON_INTERNAL);
- } else {
- log_info(LD_EXIT,"no circ. Closing conn.");
- fromconn->_base.edge_has_sent_end = 1; /* no circ to send to */
- connection_mark_for_close(TO_CONN(fromconn));
- }
- return -1;
- }
-
memset(&cell, 0, sizeof(cell_t));
cell.command = CELL_RELAY;
if (cpath_layer) {
@@ -500,8 +481,7 @@
memset(&rh, 0, sizeof(rh));
rh.command = relay_command;
- if (fromconn)
- rh.stream_id = fromconn->stream_id; /* else it's 0 */
+ rh.stream_id = stream_id;
rh.length = payload_len;
relay_header_pack(cell.payload, &rh);
if (payload_len) {
@@ -521,6 +501,48 @@
return 0;
}
+/** Make a relay cell out of <b>relay_command</b> and <b>payload</b>, and
+ * send it onto the open circuit <b>circ</b>. <b>fromconn</b> is the stream
+ * that's sending the relay cell, or NULL if it's a control cell.
+ * <b>cpath_layer</b> is NULL for OR->OP cells, or the destination hop
+ * for OP->OR cells.
+ *
+ * If you can't send the cell, mark the circuit for close and
+ * return -1. Else return 0.
+ */
+int
+connection_edge_send_command(edge_connection_t *fromconn, circuit_t *circ,
+ int relay_command, const char *payload,
+ size_t payload_len, crypt_path_t *cpath_layer)
+{
+ /* XXXX NM Split this function into a separate versions per circuit type? */
+
+ if (fromconn && fromconn->_base.marked_for_close) {
+ log_warn(LD_BUG,
+ "Bug: called on conn that's already marked for close at %s:%d.",
+ fromconn->_base.marked_for_close_file,
+ fromconn->_base.marked_for_close);
+ return 0;
+ }
+
+ if (!circ) {
+ tor_assert(fromconn);
+ if (fromconn->_base.type == CONN_TYPE_AP) {
+ log_info(LD_APP,"no circ. Closing conn.");
+ connection_mark_unattached_ap(fromconn, END_STREAM_REASON_INTERNAL);
+ } else {
+ log_info(LD_EXIT,"no circ. Closing conn.");
+ fromconn->_base.edge_has_sent_end = 1; /* no circ to send to */
+ connection_mark_for_close(TO_CONN(fromconn));
+ }
+ return -1;
+ }
+
+ return relay_send_command_from_edge(fromconn ? fromconn->stream_id : 0,
+ circ, relay_command, payload,
+ payload_len, cpath_layer);
+}
+
/** Translate <b>reason</b>, which came from a relay 'end' cell,
* into a static const string describing why the stream is closing.
* <b>reason</b> is -1 if no reason was provided.
@@ -545,6 +567,7 @@
case END_STREAM_REASON_RESOURCELIMIT: return "server out of resources";
case END_STREAM_REASON_CONNRESET: return "connection reset";
case END_STREAM_REASON_TORPROTOCOL: return "Tor protocol error";
+ case END_STREAM_REASON_NOTDIRECTORY: return "not a directory";
default:
log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
"Reason for ending (%d) not recognized.",reason);
More information about the tor-commits
mailing list