Opened 10 years ago

Last modified 7 years ago

#904 closed defect (Fixed)

When we fail to decrypt an onionskin, we never reply

Reported by: arma Owned by:
Priority: Low Milestone: 0.2.1.x-final
Component: Core Tor/Tor Version: 0.2.1.9-alpha
Severity: Keywords:
Cc: arma, nickm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

On my bridge:

Jan 06 13:06:06.447 [debug] router_set_status(): Marking router 'moria1/128.31.0
.34' as up.
Jan 06 13:06:06.447 [debug] circuit_n_conn_done(): or_conn to moria1/128.31.0.34
, status=1
Jan 06 13:06:06.447 [debug] circuit_n_conn_done(): Found circ, sending create ce
ll.
Jan 06 13:06:06.447 [debug] circuit_send_next_onion_skin(): First skin; sending
create cell.
Jan 06 13:06:06.449 [debug] circuit_deliver_create_cell(): Chosen circID 23991.
Jan 06 13:06:06.449 [debug] append_cell_to_circuit_queue(): Made a circuit activ
e.
Jan 06 13:06:06.449 [info] circuit_send_next_onion_skin(): First hop: finished s
ending CREATE cell to 'moria1'
Jan 06 13:06:06.449 [info] command_process_netinfo_cell(): Got good NETINFO cell

from 128.31.0.34; OR connection is now open, using protocol version 2

...
Jan 06 13:06:36.449 [info] circuit_expire_building(): Abandoning circ 128.31.0.3
4:9001:23991 (state 0:doing handshakes, purpose 5)
Jan 06 13:06:36.449 [info] exit circ (length 1, exit moria1): $FFCB46DB1339DA846
74C70D7CB586434C4370441(waiting for keys)
Jan 06 13:06:36.449 [info] circuit_build_failed(): Our circuit failed to get a r
esponse from the first hop (128.31.0.34:9001). I'm going to try to rotate to a b
etter connection.
Jan 06 13:06:36.449 [info] connection_ap_fail_onehop(): Closing onehop stream to

'$FFCB46DB1339DA84674C70D7CB586434C4370441/128.31.0.34' because the OR conn jus

t failed.

On moria1 at the same time:

Jan 06 13:06:06.447 [info] tor_tls_read(): Got a TLS renegotiation from "128.31.
0.34"
Jan 06 13:06:06.447 [info] command_process_versions_cell(): Negotiated version 2

with 128.31.0.34; sending NETINFO.

Jan 06 13:06:06.449 [info] command_process_netinfo_cell(): Got good NETINFO cell

from 128.31.0.34; OR connection is now open, using protocol version 2

Jan 06 13:06:06.491 [info] onion_skin_server_handshake(): Couldn't decrypt onion
skin: client may be using old onion key

Shouldn't moria1 be sending something back to indicate failure?

[Automatically added by flyspray2trac: Operating System: All]

Child Tickets

Change History (8)

comment:1 Changed 10 years ago by arma

In cpuworker.c when the cpuworker fails to do the decrypt, it does

if (onion_skin_server_handshake(question, onion_key, last_onion_key,

reply_to_proxy, keys, CPATH_KEY_MATERIAL_LEN) < 0) {

/* failure */
log_debug(LD_OR,"onion_skin_server_handshake failed.");
memset(buf,0,LEN_ONION_RESPONSE); /* send all zeros for failure */

Whereas in the main thread, it reads it as

tor_assert(buf_datalen(conn->inbuf) == LEN_ONION_RESPONSE);

connection_fetch_from_buf(&success,1,conn);
connection_fetch_from_buf(buf,LEN_ONION_RESPONSE-1,conn);

/* parse out the circ it was talking about */
tag_unpack(buf, &conn_id, &circ_id);

Perhaps that means the cpuworker should not be zeroing out the conn_id,
circ_id, etc? Just send back a 0 for the first byte and zeroes for the
part after the tag should suffice.

comment:2 Changed 10 years ago by arma

--- cpuworker.c (revision 17969)
+++ cpuworker.c (working copy)
@@ -274,7 +274,9 @@

reply_to_proxy, keys, CPATH_KEY_MATERIAL_LEN) < 0) {

/* failure */
log_debug(LD_OR,"onion_skin_server_handshake failed.");

  • memset(buf,0,LEN_ONION_RESPONSE); /* send all zeros for failure */

+ *buf = 0; /* indicate failure in first byte */
+ /* send all zeros as answer, but leave the 'tag' intact. */
+ memset(buf+1+TAG_LEN,0,LEN_ONION_RESPONSE-(1+TAG_LEN));

} else {

/* success */
log_debug(LD_OR,"onion_skin_server_handshake succeeded.");

comment:3 Changed 10 years ago by nickm

Looks plausible. What does tor-spec.txt say? What will existing clients do when they get this?

comment:4 Changed 10 years ago by arma

No, this is communication between the cpuworker and the main thread. Nothing to do
with tor-spec.txt.

comment:5 Changed 10 years ago by nickm

ah! then run with it, says I.

comment:6 Changed 10 years ago by arma

Ah. My suggested patch didn't actually send back the tag. Oops. Now it does.

Correct(er) patch went in as r17970.

comment:7 Changed 10 years ago by arma

flyspray2trac: bug closed.

comment:8 Changed 7 years ago by nickm

Component: Tor RelayTor
Note: See TracTickets for help on using tickets.