Skip to content

TROVE-2024-006: HS circuits without vanguards are built with incompatible relays

Spotted while debugging #1424 (closed)

I was looking over some hidden service logs from a shadow sim, and noticed some circuits were using the same relay more than once (as different hops). For example:

2000-01-01T00:15:27Z TRACE tor_proto::channel: Circ 1.22: Allocated CircId 4129788268
2000-01-01T00:15:27Z TRACE tor_proto::circuit::reactor: Circ 1.22: Extending circuit to hop 2 with [EncodedLinkSpec { lstype: LinkSpecType(ORPORT_V4), body: [100, 0, 0, 1, 35, 151] }, EncodedLinkSpec { lstype: LinkSpecType(RSAID), body: [165, 44, 165, 181, 108, 100, 216, 100, 246, 174, 67, 229, 111, 41, 172, 189, 87, 6, 221, 161] }]
2000-01-01T00:15:27Z TRACE tor_proto::circuit::reactor: Circ 1.22: Extending circuit to hop 2 $a52ca5b56c64d864f6ae43e56f29acbd5706dda1
2000-01-01T00:15:27Z TRACE tor_proto::circuit::reactor: Circ 1.22: waiting for EXTENDED2 cell
2000-01-01T00:15:27Z TRACE tor_proto::circuit::reactor: Circ 1.22: handling cell: Relay(Relay { body: .. })
2000-01-01T00:15:27Z TRACE tor_proto::circuit::reactor: Circ 1.22: Received EXTENDED2 cell; completing handshake.
2000-01-01T00:15:27Z TRACE tor_proto::circuit::reactor: Circ 1.22: Handshake complete; circuit extended.
2000-01-01T00:15:27Z TRACE tor_proto::circuit::reactor: Circ 1.22: Extending circuit to hop 3 with [EncodedLinkSpec { lstype: LinkSpecType(ORPORT_V4), body: [11, 0, 0, 16, 35, 151] }, EncodedLinkSpec { lstype: LinkSpecType(RSAID), body: [255, 25, 114, 4, 9, 159, 160, 229, 7, 250, 70, 212, 31, 237, 151, 211, 51, 123, 75, 170] }]
2000-01-01T00:15:27Z TRACE tor_proto::circuit::reactor: Circ 1.22: Extending circuit to hop 3 $ff197204099fa0e507fa46d41fed97d3337b4baa
2000-01-01T00:15:27Z TRACE tor_proto::circuit::reactor: Circ 1.22: waiting for EXTENDED2 cell
2000-01-01T00:15:28Z TRACE tor_proto::circuit::reactor: Circ 1.22: handling cell: Relay(Relay { body: .. })
2000-01-01T00:15:28Z TRACE tor_proto::circuit::reactor: Circ 1.22: Received EXTENDED2 cell; completing handshake.
2000-01-01T00:15:28Z TRACE tor_proto::circuit::reactor: Circ 1.22: Handshake complete; circuit extended.
2000-01-01T00:15:28Z DEBUG tor_circmgr: Circ 1.22: launched HS circuit SHORT preemptive=false
2000-01-01T00:15:28Z DEBUG tor_circmgr::hspool: Circ 1.22: launched vanguards_enabled=false kind=SHORT circ_kind=SvcHsDir
2000-01-01T00:15:28Z TRACE tor_proto::circuit::reactor: Circ 1.22: Extending circuit to hop 4 with [EncodedLinkSpec { lstype: LinkSpecType(ORPORT_V4), body: [100, 0, 0, 1, 35, 151] }, EncodedLinkSpec { lstype: LinkSpecType(RSAID), body: [165, 44, 165, 181, 108, 100, 216, 100, 246, 174, 67, 229, 111, 41, 172, 189, 87, 6, 221, 161] }]
2000-01-01T00:15:28Z TRACE tor_proto::circuit::reactor: Circ 1.22: Extending circuit to hop 4 $a52ca5b56c64d864f6ae43e56f29acbd5706dda1
2000-01-01T00:15:28Z TRACE tor_proto::circuit::reactor: Circ 1.22: waiting for EXTENDED2 cell
2000-01-01T00:15:28Z TRACE tor_proto::circuit::reactor: Circ 1.22: handling cell: Destroy(Destroy { reason: DestroyReason(DESTROYED) })
2000-01-01T00:15:28Z DEBUG tor_proto::circuit::reactor: Circ 1.22: Received DESTROY cell. Reason: Circuit was destroyed without client truncate [DESTROYED]
2000-01-01T00:15:28Z TRACE tor_proto::circuit::reactor: Circ 1.22: reactor shutdown due to handled cell
2000-01-01T00:15:28Z TRACE tor_proto::circuit::reactor: Circ 1.22: Circuit reactor stopped: Ok(())

In this SvcHsDir circuit, the relay with fingerprint a52ca5b56c64d864f6ae43e56f29acbd5706dda1 is used for hops 2 and 4, which is wrong. This happens because pick_path doesn't exclude the circuit target (a52ca5b56c64d864f6ae43e56f29acbd5706dda1) when selecting the middle and "exit" relays.

Note: circuit target is builder.compatible_with(), which is only used when selecting the guard.

The bug only happens if vanguards are disabled (when vanguards are enabled, we use VanguardHsPathBuilder::pick_path instead of pick_path).

AFAICT, this bug has always existed in pick_path, and previously, in ExitPathBuilder (compatible_with was never used for anything other than the guard selection).

I think this a medium-security issue.