[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: Prop 110 revisions
Here are three revised patches for proposal 110. I know that you all still have to consider rolling out, other possible considerations, etc. But I figured I'd make the changes that Nick suggested so they could be considered. If there are still issues with these I'd like to know. And if it turns out that this would better be set up with the version checking proposal that's cool too. Just thought I'd send these along for consideration (:
Thanks,
Nate
On 8/22/07, mrwigglet <mrwigglet@xxxxxxxxx> wrote:
Sorry if this starts a new thread, I hadn't yet joined the or-dev list
so I couldn't just hit 'reply'. Anyhow, I just have a few questions
so that I can hopefully get the patches right for this. Question 1 is
what exactly you guys want the behavior to be, either the way Nick
outlines it or the way it is in 'additional complexity'. Second is
what the naming should be, RELAY_EARLY or something else. Third question
is about where to put the two fields. Moving the counter for seen
extend cells to or_circuit_t is fine, but I'm not sure the other can
be moved to origin_circuit_t. The reason is that the only place I
can find that the CELL_RELAY type is set is in relay_send_command_from_edge
and the type of circuit in that function is circuit_t not origin_circuit_t.
So if you could let me know what way that should be handled that'd be great.
Also, whether or not you guys think this should go along with another
proposal (105 or other). Other than these few things I think I know
what is needed so hopefully we can work it all out.
Thanks,
Nate
On Tue, Aug 21, 2007 at 05:43:35PM -0400, Nick Mathewson wrote:
> It looks like these patches introduces the RELAY_EXTEND
> cell type as a server-accepted synonym for the RELAY cell type.
> (RELAY_EXTEND is not my favorite name, since CELL_RELAY_EXTEND will
> get confused with RELAY_COMMAND_EXTEND in casual writing; Roger, maybe
> we should change 110 to name the new type RELAY_EARLY or
> RELAY_EXTEND_OK? In the rest of this mail, I'll try to call it the
> "special" cell type.)
I like RELAY_EARLY. It specifies how to treat the cell, rather than what
is meant to be in it, which might be handy later on.
> I don't think that the behavior matches that described in 'phase
> one' of the proposal: special cells are passed on with the RELAY
> type, not with their own type.
Right. RELAY_EARLY cells need to be passed on as RELAY_EARLY cells,
and RELAY cells as RELAY cells.
> The second patch mucks with some whitespace in circuitbuild.c, then
> starts adding features so that clients will send as the special cell
> type their first N relay cells on each circuit, where N is chosen
> uniformly at random between 5 and 10.
>
> For this patch, I bet we could get the two uint16_ts that you've
> added to circuit_t down to a single field in origin_circuit_t (how
> many _more_ RELAY_EARLY cells will we originate?) and a single field
> in or_circuit_t (how many _more_ cells will we accept before we
> accept stop accepting RELAY_EARLY cells)? The fields can be
> uint8_t, since the limit is well under 255 in both cases.
Plausible.
> The third patch enforces the protocol by:
> A) Disallowing any RELAY_COMMAND_EXTEND cells without the special
> cell type, and
> B) Closing any circuit where too many cells of the special type
> are sent.
>
> Rule B is not quite right: the rule is not "You may send no more
> than X special cells;" the rule is "special cells may only occur as
> the first X cells on any circuit." (See proposal 110, "Design"
> section, last paragraph.)
Right.
But Nick, also see the 'additional complexity' section. It might be
smart for clients to send the first K of them as relay_early, but for
servers to enforce it by a "no more than K ever" rule. This could give us
more flexibility if we want it later -- I don't think it increases the
damage that can be done via the infinite circuit attack, though sending
a relay_early cell later on would tell everybody in the circuit what
you're up to.
(If we opt for this approach, we may find that RELAY_EARLY is now a bad
name. Hm.)
> Here's a way that we could get the new protocol in faster. It
> requires that something like proposal 105 is implemented, so that part
> of negotiating a Tor connection is learning which connection protocol
> version the other router supports. Here goes:
Actually, I had meant for us to be able to do phase 1 and phase 2 quite
close together (
e.g. both in the 0.2.0.x timeframe), and it doesn't depend
on proposal 105. Basically, Alice should use a RELAY_EARLY cell when
all the nodes in her path would understand it, and not otherwise. She
has descriptors for all of them, after all, so she's in a fine position
to know when it will work.
--Roger
diff -ru ../trunk/src/or/command.c trunk/src/or/command.c
--- ../trunk/src/or/command.c 2007-09-05 16:56:23.000000000 -0600
+++ trunk/src/or/command.c 2007-09-05 16:57:14.000000000 -0600
@@ -127,6 +127,11 @@
#endif
break;
case CELL_RELAY:
+ /** Second part of patch 1 for proposal 110 NE 8-2007
+ * Handle new CELL_RELAY_EXTEND type here, distinction
+ * between the two is handled in a later revision.
+ */
+ case CELL_RELAY_EARLY:
++stats_n_relay_cells_processed;
#ifdef KEEP_TIMING_STATS
++num_relay;
diff -ru ../trunk/src/or/or.h trunk/src/or/or.h
--- ../trunk/src/or/or.h 2007-09-05 16:56:23.000000000 -0600
+++ trunk/src/or/or.h 2007-09-05 16:57:14.000000000 -0600
@@ -659,6 +659,11 @@
#define CELL_DESTROY 4
#define CELL_CREATE_FAST 5
#define CELL_CREATED_FAST 6
+/**
+ * Part of possible patch for proposal 110 ** NE 8-2007
+ */
+#define CELL_RELAY_EARLY 7
+
/** How long to test reachability before complaining to the user. */
#define TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT (20*60)
diff -ru ../torpatch1/trunk/AUTHORS trunk/AUTHORS
--- ../torpatch1/trunk/AUTHORS 2007-09-05 16:56:48.000000000 -0600
+++ trunk/AUTHORS 2007-09-05 16:59:04.000000000 -0600
@@ -24,7 +24,9 @@
John Bashinski <jbash@xxxxxxxxxx> contributed the initial rpm spec file.
-Christian Grothoff <grothoff@xxxxxxxxxxxxx> contributed better daemonizing
+Nathan Evans <nevans6@xxxxxx> contributed patch implementing proposal 110
+
+Christian Grothoff <christian@xxxxxxxxxxxx> contributed better daemonizing
behavior.
Steven Hazel <sah@xxxxxxxxxxxxxxxxx> made 'make install' do the right
--- ../torpatch1/trunk/src/or/circuitlist.c 2007-09-05 16:56:48.000000000 -0600
+++ trunk/src/or/circuitlist.c 2007-09-05 16:59:04.000000000 -0600
@@ -345,6 +345,10 @@
circ->next_stream_id = crypto_rand_int(1<<16);
circ->global_identifier = n_circuits_allocated++;
+ /** Part of patch 2 for proposal 110. Initialize <b>relay_early_num</b>
+ * to random int between MIN_RELAY_EXTENDS and MAX_RELAY_EXTENDS */
+ circ->relay_early_num = crypto_rand_int(MAX_RELAY_EXTENDS - MIN_RELAY_EXTENDS) + MIN_RELAY_EXTENDS;
+
init_circuit_base(TO_CIRCUIT(circ));
return circ;
@@ -364,6 +368,10 @@
if (p_conn)
circuit_set_p_circid_orconn(circ, p_circ_id, p_conn);
+ /** Part of patch 2 for proposal 110. Initialize <b>relay_early_seen</b>
+ * to 0 */
+ circ->relay_early_seen = 0;
+
init_circuit_base(TO_CIRCUIT(circ));
return circ;
diff -ru ../torpatch1/trunk/src/or/command.c trunk/src/or/command.c
--- ../torpatch1/trunk/src/or/command.c 2007-09-05 16:57:14.000000000 -0600
+++ trunk/src/or/command.c 2007-09-05 16:59:04.000000000 -0600
@@ -345,6 +345,12 @@
direction==CELL_DIRECTION_OUT?"forward":"backward");
circuit_mark_for_close(circ, -reason);
}
+
+ /** Part of patch 2 for proposal 110. Increment this circuits
+ * number of extend cells seen. */
+ if (cell->command == CELL_RELAY_EARLY)
+ TO_OR_CIRCUIT(circ)->relay_early_seen++;
+
}
/** Process a 'destroy' <b>cell</b> that just arrived from
diff -ru ../torpatch1/trunk/src/or/or.h trunk/src/or/or.h
--- ../torpatch1/trunk/src/or/or.h 2007-09-05 16:57:14.000000000 -0600
+++ trunk/src/or/or.h 2007-09-05 16:59:04.000000000 -0600
@@ -664,6 +664,14 @@
*/
#define CELL_RELAY_EARLY 7
+/** Initial definition of the minimum number of extends
+ * to send, and the max to allow at each node. Part of
+ * patch2 for proposal 110 NE 8-2007
+ * (Is this the right place for these?)
+ */
+#define MIN_RELAY_EXTENDS 5
+#define MAX_RELAY_EXTENDS 10
+
/** How long to test reachability before complaining to the user. */
#define TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT (20*60)
@@ -1676,6 +1684,7 @@
* linked to an OR connection. */
struct circuit_t *prev_active_on_n_conn;
struct circuit_t *next; /**< Next circuit in linked list of all circuits. */
+
} circuit_t;
/** An origin_circuit_t holds data necessary to build and use a circuit.
@@ -1721,6 +1730,16 @@
/** Quasi-global identifier for this circuit; used for control.c */
/* XXXX NM This can get re-used after 2**32 circuits. */
uint32_t global_identifier;
+
+ /** Part of phase 2 for changes outlined in proposal 110. Add two new
+ * variables <b>relay_early_num</b> and <b>relay_early_seen</b>.
+ * The former will initially be set to a random value between 3 and 10
+ * and decremented each time a relay_extend cell is sent. The latter
+ * will be used to keep track of relay_extend cells seen for a particular
+ * circuit. Once <b>relay_early_num</b> reaches 0, no more extends will
+ * be sent on this circuite. Once <b>relay_early_seen</b> reaches some
+ * maximum (10?) the circuit will be discarded. NE 8-2007 */
+ uint8_t relay_early_num;
} origin_circuit_t;
@@ -1787,6 +1806,16 @@
/** True iff this circuit was made with a CREATE_FAST cell. */
unsigned int is_first_hop : 1;
+
+ /** Part of phase 2 for changes outlined in proposal 110. Add two new
+ * variables <b>relay_early_num</b> and <b>relay_early_seen</b>.
+ * The former will initially be set to a random value between 3 and 10
+ * and decremented each time a relay_extend cell is sent. The latter
+ * will be used to keep track of relay_extend cells seen for a particular
+ * circuit. Once <b>relay_early_num</b> reaches 0, no more extends will
+ * be sent on this circuite. Once <b>relay_early_seen</b> reaches some
+ * maximum (10?) the circuit will be discarded. NE 8-2007 */
+ uint8_t relay_early_seen;
} or_circuit_t;
/** Convert a circuit subtype to a circuit_t.*/
diff -ru ../torpatch1/trunk/src/or/relay.c trunk/src/or/relay.c
--- ../torpatch1/trunk/src/or/relay.c 2007-09-05 16:56:48.000000000 -0600
+++ trunk/src/or/relay.c 2007-09-05 17:02:19.000000000 -0600
@@ -481,7 +481,18 @@
tor_assert(circ);
memset(&cell, 0, sizeof(cell_t));
- cell.command = CELL_RELAY;
+
+ /** Part of patch2 for proposal 110. Check whether this node has sent the set
+ * number of relay_early cells. If so, send a regular relay cell instead. */
+ if (CIRCUIT_IS_ORIGIN(circ)) {
+ if (TO_ORIGIN_CIRCUIT(circ)->relay_early_num > 0) {
+ cell.command = CELL_RELAY_EARLY;
+ TO_ORIGIN_CIRCUIT(circ)->relay_early_num--;
+ }
+ else
+ cell.command = CELL_RELAY;
+ }
+
if (cpath_layer) {
cell.circ_id = circ->n_circ_id;
cell_direction = CELL_DIRECTION_OUT;
diff -ru ../torpatch2/trunk/src/or/circuitbuild.c trunk/src/or/circuitbuild.c
--- ../torpatch2/trunk/src/or/circuitbuild.c 2007-09-05 16:59:04.000000000 -0600
+++ trunk/src/or/circuitbuild.c 2007-09-05 17:08:17.000000000 -0600
@@ -712,6 +712,14 @@
char *onionskin;
char *id_digest=NULL;
+ /** Part of patch 3 for proposal 110. Make sure correct cell type is
+ * used when extend request is received. */
+ if (cell->command != CELL_RELAY_EARLY) {
+ log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
+ "Attempt to extend with incorrect cell type. Closing.");
+ return -1;
+ }
+
if (circ->n_conn) {
log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
"n_conn already set. Bug/attack. Closing.");
diff -ru ../torpatch2/trunk/src/or/relay.c trunk/src/or/relay.c
--- ../torpatch2/trunk/src/or/relay.c 2007-09-05 17:02:19.000000000 -0600
+++ trunk/src/or/relay.c 2007-09-05 17:08:17.000000000 -0600
@@ -234,6 +234,19 @@
* we might kill the circ before we relay
* the cells. */
+ /** Patch 3 for proposal 110, if too many extends have been seen on this
+ * circuit, end it! If the cell type is a regular relay cell, then set
+ * <b>relay_early_seen</b> higher than MAX_RELAY_EXTENDS so extending later
+ * is also disallowed NE 8-2007 */
+ if ((cell->command == CELL_RELAY_EARLY)&&(TO_OR_CIRCUIT(circ)->relay_early_seen > MAX_RELAY_EXTENDS)) {
+ log_fn(LOG_PROTOCOL_WARN,LD_PROTOCOL,
+ "Too many extends requested on this circuit, closing.");
+ return -END_CIRC_REASON_TORPROTOCOL;
+ }
+ else if ((cell->command == CELL_RELAY) && (TO_OR_CIRCUIT(circ)->relay_early_seen < MAX_RELAY_EXTENDS)) {
+ TO_OR_CIRCUIT(circ)->relay_early_seen = MAX_RELAY_EXTENDS + 1;
+ }
+
append_cell_to_circuit_queue(circ, or_conn, cell, cell_direction);
return 0;
}