[or-cvs] r8760: Fix an XXX in handling destroy cells: when we get a destroy (in tor/trunk: . doc src/or)
nickm at seul.org
nickm at seul.org
Thu Oct 19 23:04:52 UTC 2006
Author: nickm
Date: 2006-10-19 19:04:49 -0400 (Thu, 19 Oct 2006)
New Revision: 8760
Modified:
tor/trunk/
tor/trunk/ChangeLog
tor/trunk/doc/control-spec.txt
tor/trunk/src/or/circuitlist.c
tor/trunk/src/or/command.c
tor/trunk/src/or/control.c
tor/trunk/src/or/or.h
Log:
r9272 at Kushana: nickm | 2006-10-19 12:52:37 -0400
Fix an XXX in handling destroy cells: when we get a destroy cell with reason FOO, do not tell the controller REASON=FOO. Instead, say REASON=DESTROYED REMOTE_REASON=FOO. Suggested by a conversation with Mike Perry.
Property changes on: tor/trunk
___________________________________________________________________
svk:merge ticket from /tor/trunk [r9272] on c95137ef-5f19-0410-b913-86e773d04f59
Modified: tor/trunk/ChangeLog
===================================================================
--- tor/trunk/ChangeLog 2006-10-19 23:04:35 UTC (rev 8759)
+++ tor/trunk/ChangeLog 2006-10-19 23:04:49 UTC (rev 8760)
@@ -10,6 +10,8 @@
event format. Also, add additional reason codes to explain why a
given circuit has been destroyed or truncated. (Patches from Mike
Perry)
+ - Add a REMOTE_REASON field to CIRC events to tell the controller about
+ why a remote OR told us to close a circuit.
o Security bugfixes:
- When the user sends a NEWNYM signal, clear the client-side DNS
Modified: tor/trunk/doc/control-spec.txt
===================================================================
--- tor/trunk/doc/control-spec.txt 2006-10-19 23:04:35 UTC (rev 8759)
+++ tor/trunk/doc/control-spec.txt 2006-10-19 23:04:49 UTC (rev 8760)
@@ -788,7 +788,7 @@
The syntax is:
"650" SP "CIRC" SP CircuitID SP CircStatus [SP Path]
- [SP "REASON=" Reason] CRLF
+ [SP "REASON=" Reason [SP "REMOTE_REASON=" Reason]] CRLF
CircStatus =
"LAUNCHED" / ; circuit ID assigned to new circuit
@@ -813,6 +813,11 @@
NOPATH (Not enough nodes to make circuit)
+ The "REMOTE_REASON" field is provided only when we receive a DESTROY or
+ TRUNCATE cell, and only if extended events are enabled. It contains the
+ actual reason given by the remote OR for closing the circuit. Clients MUST
+ accept reasons not listed above. Reasons are as listed in tor-spec.txt.
+
4.1.2. Stream status changed
The syntax is:
Modified: tor/trunk/src/or/circuitlist.c
===================================================================
--- tor/trunk/src/or/circuitlist.c 2006-10-19 23:04:35 UTC (rev 8759)
+++ tor/trunk/src/or/circuitlist.c 2006-10-19 23:04:49 UTC (rev 8760)
@@ -848,6 +848,10 @@
* to track them anyway so we can give them to the controller. */
reason = END_CIRC_REASON_NONE;
}
+
+ if (reason & END_CIRC_REASON_FLAG_REMOTE)
+ reason &= ~END_CIRC_REASON_FLAG_REMOTE;
+
if (reason < _END_CIRC_REASON_MIN || reason > _END_CIRC_REASON_MAX) {
log_warn(LD_BUG, "Reason %d out of range at %s:%d", reason, file, line);
orig_reason = reason = END_CIRC_REASON_NONE;
Modified: tor/trunk/src/or/command.c
===================================================================
--- tor/trunk/src/or/command.c 2006-10-19 23:04:35 UTC (rev 8759)
+++ tor/trunk/src/or/command.c 2006-10-19 23:04:49 UTC (rev 8760)
@@ -359,7 +359,7 @@
command_process_destroy_cell(cell_t *cell, or_connection_t *conn)
{
circuit_t *circ;
- uint8_t reason;
+ int reason;
circ = circuit_get_by_circid_orconn(cell->circ_id, conn);
reason = (uint8_t)cell->payload[0];
@@ -378,25 +378,7 @@
} else { /* the destroy came from ahead */
circuit_set_n_circid_orconn(circ, 0, NULL);
if (CIRCUIT_IS_ORIGIN(circ)) {
- /* Prevent arbitrary destroys from going unnoticed by controller */
- if (reason == END_CIRC_REASON_NONE ||
- reason == END_CIRC_REASON_FINISHED ||
- reason == END_CIRC_REASON_REQUESTED) {
- /* XXXX This logic is wrong. Really, we should report the fact that
- * the circuit was closed because of a DESTROY, *and* we should report
- * the reason that we were given. -NM
- * Hrmm. We could store the fact that we sent a truncate and the
- * reason for this truncate in circuit_t. If we ever get a destroy
- * that doesn't match this reason, we could complain loudly -MP
- * That won't work for the cases where the destroy is not because of
- * a truncate, though. The idea is that if we get a DESTROYED cell
- * with reason 'CONNECTFAILED' and another DESTROYED cell with reason
- * 'RESOURCELIMIT', the controller may want to know the reported
- * reason. -NM
- */
- reason = END_CIRC_REASON_DESTROYED;
- }
- circuit_mark_for_close(circ, reason);
+ circuit_mark_for_close(circ, reason|END_CIRC_REASON_FLAG_REMOTE);
} else {
char payload[1];
log_debug(LD_OR, "Delivering 'truncated' back.");
Modified: tor/trunk/src/or/control.c
===================================================================
--- tor/trunk/src/or/control.c 2006-10-19 23:04:35 UTC (rev 8759)
+++ tor/trunk/src/or/control.c 2006-10-19 23:04:49 UTC (rev 8760)
@@ -2802,42 +2802,44 @@
static const char *
circuit_end_reason_to_string(int reason)
{
+ if (reason >= 0 && reason & END_CIRC_REASON_FLAG_REMOTE)
+ reason &= ~END_CIRC_REASON_FLAG_REMOTE;
switch (reason) {
case END_CIRC_AT_ORIGIN:
/* This shouldn't get passed here; it's a catch-all reason. */
- return "REASON=ORIGIN";
+ return "ORIGIN";
case END_CIRC_REASON_NONE:
/* This shouldn't get passed here; it's a catch-all reason. */
- return "REASON=NONE";
+ return "NONE";
case END_CIRC_REASON_TORPROTOCOL:
- return "REASON=TORPROTOCOL";
+ return "TORPROTOCOL";
case END_CIRC_REASON_INTERNAL:
- return "REASON=INTERNAL";
+ return "INTERNAL";
case END_CIRC_REASON_REQUESTED:
- return "REASON=REQUESTED";
+ return "REQUESTED";
case END_CIRC_REASON_HIBERNATING:
- return "REASON=HIBERNATING";
+ return "HIBERNATING";
case END_CIRC_REASON_RESOURCELIMIT:
- return "REASON=RESOURCELIMIT";
+ return "RESOURCELIMIT";
case END_CIRC_REASON_CONNECTFAILED:
- return "REASON=CONNECTFAILED";
+ return "CONNECTFAILED";
case END_CIRC_REASON_OR_IDENTITY:
- return "REASON=OR_IDENTITY";
+ return "OR_IDENTITY";
case END_CIRC_REASON_OR_CONN_CLOSED:
- return "REASON=OR_CONN_CLOSED";
+ return "OR_CONN_CLOSED";
case END_CIRC_REASON_FINISHED:
- return "REASON=FINISHED";
+ return "FINISHED";
case END_CIRC_REASON_TIMEOUT:
- return "REASON=TIMEOUT";
+ return "TIMEOUT";
case END_CIRC_REASON_DESTROYED:
- return "REASON=DESTROYED";
+ return "DESTROYED";
case END_CIRC_REASON_NOPATH:
- return "REASON=NOPATH";
+ return "NOPATH";
case END_CIRC_REASON_NOSUCHSERVICE:
- return "REASON=NOSUCHSERVICE";
+ return "NOSUCHSERVICE";
default:
log_warn(LD_BUG, "Unrecognized reason code %d", (int)reason);
- return "REASON=UNRECOGNIZED"; /* should never get called */
+ return NULL;
}
}
@@ -2867,7 +2869,7 @@
}
if (EVENT_IS_INTERESTING1(EVENT_CIRCUIT_STATUS)) {
const char *status;
- const char *reason = "";
+ char reason_buf[64];
switch (tp)
{
case CIRC_EVENT_LAUNCHED: status = "LAUNCHED"; break;
@@ -2881,7 +2883,21 @@
}
if (tp == CIRC_EVENT_FAILED || tp == CIRC_EVENT_CLOSED) {
- reason = circuit_end_reason_to_string(reason_code);
+ const char *reason_str = circuit_end_reason_to_string(reason_code);
+ char *reason = NULL;
+ if (!reason_str) {
+ reason = tor_malloc(16);
+ tor_snprintf(reason, 16, "UNKNOWN_%d", reason_code);
+ reason_str = reason;
+ }
+ if (reason_code > 0 && reason_code & END_CIRC_REASON_FLAG_REMOTE) {
+ tor_snprintf(reason_buf, sizeof(reason_buf),
+ "REASON=DESTROYED REMOTE_REASON=%s", reason_str);
+ } else {
+ tor_snprintf(reason_buf, sizeof(reason_buf),
+ "REASON=%s", reason_str);
+ }
+ tor_free(reason);
}
if (EVENT_IS_INTERESTING1S(EVENT_CIRCUIT_STATUS)) {
@@ -2889,7 +2905,7 @@
send_control1_event_extended(EVENT_CIRCUIT_STATUS, SHORT_NAMES,
"650 CIRC %lu %s%s%s@%s\r\n",
(unsigned long)circ->global_identifier,
- status, sp, path, reason);
+ status, sp, path, reason_buf);
}
if (EVENT_IS_INTERESTING1L(EVENT_CIRCUIT_STATUS)) {
char *vpath = circuit_list_path_for_controller(circ);
@@ -2897,7 +2913,7 @@
send_control1_event_extended(EVENT_CIRCUIT_STATUS, LONG_NAMES,
"650 CIRC %lu %s%s%s@%s\r\n",
(unsigned long)circ->global_identifier,
- status, sp, vpath, reason);
+ status, sp, vpath, reason_buf);
tor_free(vpath);
}
}
Modified: tor/trunk/src/or/or.h
===================================================================
--- tor/trunk/src/or/or.h 2006-10-19 23:04:35 UTC (rev 8759)
+++ tor/trunk/src/or/or.h 2006-10-19 23:04:49 UTC (rev 8760)
@@ -497,6 +497,7 @@
/* Negative reasons are internal */
#define END_CIRC_REASON_NOPATH -2
#define END_CIRC_AT_ORIGIN -1
+
#define _END_CIRC_REASON_MIN 0
#define END_CIRC_REASON_NONE 0
#define END_CIRC_REASON_TORPROTOCOL 1
@@ -513,6 +514,11 @@
#define END_CIRC_REASON_NOSUCHSERVICE 12
#define _END_CIRC_REASON_MAX 12
+/* OR this with the argument to circuit_mark_for_close, or
+ * control_event_circuit_status to indicate that the reason came from a
+ * destroy or truncate cell. */
+#define END_CIRC_REASON_FLAG_REMOTE 512
+
/** Length of 'y' portion of 'y.onion' URL. */
#define REND_SERVICE_ID_LEN 16
More information about the tor-commits
mailing list