sendme: Introduce consensus param sendme_dir_min_version #1027
Conversation
See section 4.5 of proposal 289 which describes why we need this parameter. To summarize, this is to accomodate old clients not supporting authenticated SENDMEs and booting up for the first time. They need to be able to fetch a consensus to actually learn that the required protocols is FlowCtrl=1 and thus not start up. And for that, we need to allow a SENDME minimum version on a directory request circuit. That new consensus parameter handles that. Closes #30364 Signed-off-by: David Goulet <dgoulet@torproject.org>
Pull Request Test Coverage Report for Build 5032
|
| @@ -1670,6 +1670,11 @@ connection_edge_process_relay_cell(cell_t *cell, circuit_t *circ, | |||
| "from intermediary node. Dropping."); | |||
| return 0; | |||
| } | |||
| if (circ->purpose == CIRCUIT_PURPOSE_OR) { | |||
This code is reached for BEGIN and BEGIN_DIR.
| @@ -39,6 +39,11 @@ | |||
| #define SENDME_ACCEPT_MIN_VERSION_MIN 0 | |||
| #define SENDME_ACCEPT_MIN_VERSION_MAX UINT8_MAX | |||
|
|
|||
| /* Minimum version allowed on a one-hop directory circuit. */ | |||
This comment isn't accurate?
| * circuit that is for a directory request. It is only relevant if we are | ||
| * the directory server and thus on an OR circuit. */ | ||
| if (CIRCUIT_IS_ORCIRC(circ) && CONST_TO_OR_CIRCUIT(circ)->begin_dir_seen) { | ||
| accept_version = get_dir_min_version(); |
This calculation is expensive enough that maybe we don't want to do it for every sendme?
| * mechanism in order to know if this circuit will serve a consensus for | ||
| * which we need to obey a specific consensus parameter about the minimal | ||
| * SENDME version we accept. */ | ||
| unsigned int begin_dir_seen : 1; |
I think this might be backwards: If I understand correctly, we don't want somebody to be able to send a BEGIN_DIR in order to make all a circuit exempt from the minimum. I think instead we wanted to have it so that if the circuit got a regular BEGIN, we know that it is not a directory circuit?
Hmmm interesting, I haven't thought of gaming this by sending a RELAY_BEGIN_DIR and then performing other tasks than a directory request...
Actually, is this assertion always true?: if no BEGIN than must be a directory circuit?
In other words, it is safe and reliable to apply this minimal version for the SENDME as long as we haven't seen a BEGIN on it?
| @@ -61,6 +61,13 @@ struct or_circuit_t { | |||
| * statistics. */ | |||
| unsigned int circuit_carries_hs_traffic_stats : 1; | |||
|
|
|||
| /** If set, this circuit got a BEGIN_DIR, meaning a directory request, and | |||
| * not for any hidden service documents. We use this flag for SENDME | |||
There's nothing in this patch that checks for hidden service documents?
See section 4.5 of proposal 289 which describes why we need this parameter. To
summarize, this is to accomodate old clients not supporting authenticated
SENDMEs and booting up for the first time.
They need to be able to fetch a consensus to actually learn that the required
protocols is FlowCtrl=1 and thus not start up. And for that, we need to allow
a SENDME minimum version on a directory request circuit. That new consensus
parameter handles that.
Closes #30364
Signed-off-by: David Goulet dgoulet@torproject.org
The text was updated successfully, but these errors were encountered: