Opened 13 months ago

Last modified 5 months ago

#32040 needs_revision defect

HS intro padding machine reactivates after receiving INTRODUCE_ACK

Reported by: asn Owned by: asn
Priority: Medium Milestone: Tor: 0.4.4.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: wtf-pad, padding, circpad-researchers-want, 043-deferred
Cc: Actual Points:
Parent ID: Points: 1
Reviewer: Sponsor: Sponsor2

Description (last modified by asn)

While reseaching wtf-pad I noticed that the intro machines display a weird shutdown/reactivation pattern.

In particular, what happens is that after INTRODUCE1 is sent, the machine starts up and sends all its padding, and then shuts down. Then when INTRODUCE_ACK arrives, it reactivates (probably because INTRODUCE_ACK is part of the accepted purposes and the machine is shutdown/freed at that time) and sends again padding, then again shuts down...

This does not work as intended and causes extra cells to fly in with a pattern that probably does not help them blend in.

Here are some Tor logs (with padanalysis incoming/outgoing cells logs):

Oct 11 13:25:54.000 [warn] outgoing-cell: EXTEND2 0
Oct 11 13:25:54.000 [warn] incoming-cell: EXTENDED2 0 66
Oct 11 13:25:54.000 [warn] outgoing-cell: EXTEND2 0
Oct 11 13:25:54.000 [warn] incoming-cell: EXTENDED2 0 66
Oct 11 13:25:57.000 [warn] outgoing-cell: EXTEND 0
Oct 11 13:25:58.000 [warn] incoming-cell: EXTENDED 0 148
Oct 11 13:25:58.000 [warn] outgoing-cell: INTRODUCE1 4
Oct 11 13:25:58.000 [warn] outgoing-cell: PADDING_NEGOTIATE 4
Oct 11 13:25:58.000 [warn] incoming-cell: PADDING_NEGOTIATED 4 4
Oct 11 13:25:58.000 [warn] incoming-cell: DROP 4 0
Oct 11 13:25:58.000 [warn] incoming-cell: DROP 4 0
Oct 11 13:25:58.000 [warn] incoming-cell: DROP 4 0
Oct 11 13:25:58.000 [warn] incoming-cell: DROP 4 0
Oct 11 13:25:58.000 [warn] incoming-cell: DROP 4 0
Oct 11 13:25:58.000 [warn] incoming-cell: DROP 4 0
Oct 11 13:25:58.000 [warn] incoming-cell: DROP 4 0
Oct 11 13:25:58.000 [warn] incoming-cell: DROP 4 0
Oct 11 13:25:58.000 [warn] incoming-cell: DROP 4 0
Oct 11 13:25:58.000 [warn] incoming-cell: PADDING_NEGOTIATED 4 4
Oct 11 13:25:58.000 [info] circpad_handle_padding_negotiated(): Received STOP command on PADDING_NEGOTIATED for circuit 5
Oct 11 13:25:58.000 [info] circpad_circuit_machineinfo_free_idx(): Freeing padding info idx 0 on circuit 5 (7)
Oct 11 13:25:58.000 [warn] incoming-cell: INTRODUCE_ACK 4 0
Oct 11 13:25:58.000 [info] rend_client_introduction_acked(): Received ack. Telling rend circ...
Oct 11 13:25:58.000 [info] circpad_setup_machine_on_circ(): Registering machine client_ip_circ to origin circ 5 (8)
Oct 11 13:25:58.000 [info] circpad_node_supports_padding(): Checking padding: supported
Oct 11 13:25:58.000 [info] circpad_negotiate_padding(): Negotiating padding on circuit 5 (8), command 2
Oct 11 13:25:58.000 [warn] outgoing-cell:  8 PADDING_NEGOTIATE 4
Oct 11 13:25:58.000 [info] circpad_machine_spec_transition(): Circuit 5 circpad machine 0 transitioning from 0 to 1
Oct 11 13:25:58.000 [info] circpad_marked_circuit_for_padding(): Circuit 5 is not marked for close because of a pending padding machine in index 0.
Oct 11 13:25:58.000 [warn] incoming-cell: PADDING_NEGOTIATED 4 4
Oct 11 13:25:58.000 [warn] incoming-cell: DROP 4 0
Oct 11 13:25:58.000 [warn] incoming-cell: DROP 4 0
Oct 11 13:25:58.000 [warn] incoming-cell: DROP 4 0
Oct 11 13:25:58.000 [warn] incoming-cell: DROP 4 0
Oct 11 13:25:58.000 [warn] incoming-cell: DROP 4 0
Oct 11 13:25:58.000 [warn] incoming-cell: DROP 4 0
Oct 11 13:25:58.000 [warn] incoming-cell: DROP 4 0
Oct 11 13:25:58.000 [warn] incoming-cell: DROP 4 0
Oct 11 13:25:58.000 [warn] incoming-cell: PADDING_NEGOTIATED 4 4
Oct 11 13:25:58.000 [info] circpad_handle_padding_negotiated(): Received STOP command on PADDING_NEGOTIATED for circuit 5
Oct 11 13:25:58.000 [info] circpad_circuit_machineinfo_free_idx(): Freeing padding info idx 0 on circuit 5 (15)
...
Oct 11 13:36:11.000 [info] circuit_mark_for_close_(): Circuit 4130340667 (id: 5) marked for close at src/core/or/circuituse.c:1509 (orig reason: 9, new reason: 0)

I'm not sure what the fix should be here. It might be that we need to remove INTRODUCE_ACK from the list of accepted purposes, but if we do that then if INTRODUCE_ACK arrives earlier we will stop padding after. Hmm.

Child Tickets

TicketStatusOwnerSummaryComponent
#33352assignedasnWarning: RELAY_COMMAND_INTRODUCE_ACK on padding circuitCore Tor/Tor

Change History (16)

comment:1 Changed 13 months ago by asn

Description: modified (diff)

comment:2 Changed 12 months ago by asn

As a summary to the above, this bug appears when the intro point machine sends all its padding and terminates before it sends the INTRODUCE_ACK, which causes a reactivation upon transitioning to introduce_acked purpose (which is in the machine's accepted purposes).

I did an experiment by creating 500 introduction circuits. Out of which:

  • 437 circuits displayed the above buggy pattern (machine finished padding before intro_ack and reactivated).
  • 63 circuits did not bug out (machine had leftover padding when intro_ack arrived so it did not activate twice).

This is not an urgent issue because our machines are not super good at hiding circuits anyway, but it's something we should have a fix for for future machines and also for improvements of the current machines.

In particular the issue here is that if we remove the intro_acked purpose from the accepted purposes list, the machine will terminate early if the intro_ack arrives early, but when intro_acked is in the purpose list, the machine can reactivate if the intro_ack arrives late (this bug).

Perhaps we need another purpose list called tolerated_purpose_list which we can add intro_acked and the machine won't activate if it enters that purpose, but it will also tolerate it if it enters that purpose in the middle of the run?

How else can we fix this issue?

comment:3 Changed 11 months ago by mikeperry

Keywords: circpad-researchers-want added

comment:4 Changed 9 months ago by nickm

Keywords: 043-deferred added

All 0.4.3.x tickets without 043-must, 043-should, or 043-can are about to be deferred.

comment:5 Changed 9 months ago by nickm

Milestone: Tor: 0.4.3.x-finalTor: 0.4.4.x-final

comment:6 Changed 5 months ago by asn

Given that these machines are not offering too much protection and they were meant to be PoCs, perhaps the easiest fix here is to remove the intro_acked purpose from the accepted purposes list. This means that some times the machines are gonna terminate early, but that's fine since it's not like the extra padding is gonna fix circuit fingerprinting attacks.

comment:7 Changed 5 months ago by asn

Owner: set to asn
Status: newassigned

comment:8 Changed 5 months ago by mikeperry

FWIW, I like the tolerated_purpose_list idea from comment:2 slightly better as it may come in handy elsewhere.

comment:9 Changed 5 months ago by mikeperry

Bonus patch that does this at: https://github.com/mikeperry-tor/tor/commits/bug30992_draft

Note: we still need to program new machines to use this to solve this ticket.... That *will* require a protover bump.

comment:10 Changed 5 months ago by mikeperry

Actually, no. No protover is required. Only client upgrade is needed, so no negotiation needs to happen. The things you can do with just endpoint upgrade are pretty crazy, huh

We just can remove CIRCUIT_PURPOSE_C_INTRODUCE_ACKED from apply_purpose_mask and set it and CIRCUIT_PURPOSE_C_INTRODUCING in keep_purpose_mask.

This is done in this commit: https://github.com/mikeperry-tor/tor/commit/2b7b8a7cfd1124ec689e26439178cc312cf5c7d3

The rend machines might need a similar fix if the REND circuit also has purpose flapping? I did not do that yet.

Last edited 5 months ago by mikeperry (previous) (diff)

comment:11 Changed 5 months ago by mikeperry

Ok nice catch on the review. I fixed that double-logic bug and squashed everything down: https://github.com/mikeperry-tor/tor/tree/bug30992%2B32040

Note that I stripped down the apply_purpose_mask for intro circs to *only* include C_INTRODUCING, as this should be the first and only purpose we want to actually *start* machines for, right? We should not try to jump into the other purposes to apply a fresh machine, but we should keep an old run around for:
https://github.com/mikeperry-tor/tor/commit/551ccc95fb242e27477ec5fcbf0af32a4b0ce761

Is C_INTRODUCING the right start purpose? Or can intro circuits jump in fully formed at later purposes?

Do we want to do the same thing to rends for symmetry?

comment:12 Changed 5 months ago by asn

Hm. I think CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT is the right purpose to start applying padding for the purposes of the prop302 machines , because it's after receiving INTRODUCE_ACK is received that we should start sending padding, and not after sending INTRODUCE1.

Perhaps we can let the rendezvous machines as is? If it's not broken let's not fix it...

comment:13 Changed 5 months ago by asn

Status: assignedneeds_revision

comment:14 in reply to:  12 ; Changed 5 months ago by mikeperry

Replying to asn:

Hm. I think CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT is the right purpose to start applying padding for the purposes of the prop302 machines , because it's after receiving INTRODUCE_ACK is received that we should start sending padding, and not after sending INTRODUCE1.

Wait, isn't C_INTRODUCING before ACK_WAIT? The original code listed C_INTRODUCING as well, which means we were firing up machines earlier, right?

Perhaps we can let the rendezvous machines as is? If it's not broken let's not fix it...

Ok.

comment:15 in reply to:  14 Changed 5 months ago by asn

Replying to mikeperry:

Replying to asn:

Hm. I think CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT is the right purpose to start applying padding for the purposes of the prop302 machines , because it's after receiving INTRODUCE_ACK is received that we should start sending padding, and not after sending INTRODUCE1.

Wait, isn't C_INTRODUCING before ACK_WAIT? The original code listed C_INTRODUCING as well, which means we were firing up machines earlier, right?

Hm. I don't think the original code listed C_INTRODUCING: https://github.com/torproject/tor/blob/master/src/core/or/circuitpadding_machines.c#L105

comment:16 Changed 5 months ago by mikeperry

Ah, I forgot I added C_INTRODUCING to the keep_purpose_mask based on this bug.

Am I correct in believing that we should add C_INTRODUCING to keep_purpose_mask so that we don't shut down the machines if we fall back to that purpose, based on the loglines in the description? Or do we need to add other/more purposes to keep_purpose_mask here? Or none?

Note: See TracTickets for help on using tickets.