Opened 6 months ago

Closed 5 months ago

#26158 closed defect (fixed)

A little bug of circular path of Tor

Reported by: TBD.Chen Owned by:
Priority: Medium Milestone: Tor: 0.3.4.x-final
Component: Core Tor/Tor Version: Tor: 0.3.0.1-alpha
Severity: Normal Keywords: circular-path, security-low, 031-backport, 032-backport, 033-backport, 034-backport
Cc: Actual Points:
Parent ID: Points:
Reviewer: dgoulet Sponsor:

Description

In order to defend the circular-path attacks, Tor relays detects the next hop and previous hop of a circuit through node-id and Ed25519-id.
However, when the Tor relay detects the previous node has the same Ed25519-id with next node, it forgot to return -1, and continue to extend the circuit.
This might cause some loopholes for the circular-path.

  /* Next, check if we're being asked to connect to the hop that the
   * extend cell came from. There isn't any reason for that, and it can
   * assist circular-path attacks. */
  if (tor_memeq(ec.node_id,
                TO_OR_CIRCUIT(circ)->p_chan->identity_digest,
                DIGEST_LEN)) {
    log_fn(LOG_PROTOCO[[Image()]]L_WARN, LD_PROTOCOL,
           "Client asked me to extend back to the previous hop.");
    return -1;
  }

  /* Check the previous hop Ed25519 ID too */
  if (! ed25519_public_key_is_zero(&ec.ed_pubkey) &&
      ed25519_pubkey_eq(&ec.ed_pubkey,
                        &TO_OR_CIRCUIT(circ)->p_chan->ed25519_identity)) {
    log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
           "Client asked me to extend back to the previous hop "
           "(by Ed25519 ID).");
  }

Child Tickets

Change History (9)

comment:1 Changed 6 months ago by teor

Keywords: security-low 031-backport 032-backport 033-backport 034-backport added
Milestone: Tor: 0.3.2.x-finalTor: 0.3.4.x-final
Version: Tor: 0.3.2.10Tor: 0.3.0.1-alpha

Thanks for reporting this issue!

This is a bug in commit c837786 in 0.3.0.1-alpha.

I've marked it as security-low, because since commit 592a439 in 0.2.7.2-alpha, directory authorities pin relay ed25519 keys to RSA keys. This means that a relay in the consensus can't pass the RSA check, but fail the ed25519 check.

(A client can't loop between two bridges using different keys, because RSA IDs are mandatory. When we stop making RSA IDs mandatory, we'll need to think carefully about this issue, and multiple ORPorts as well.)

comment:2 Changed 6 months ago by nickm

We may as well apply this fix as far back as possible; it's unlikely to cause any surprising side effects.

comment:3 Changed 6 months ago by nickm

Status: newneeds_review

One-line fix in my branch bug26158_031; please review.

comment:4 in reply to:  3 Changed 6 months ago by TBD.Chen

Replying to nickm:

One-line fix in my branch bug26158_031; please review.

Hi, I really want to review this, because this is my first bug found in my life, even it is not very serious. But I don't know how to check it. I clone the project through the code,

git clone -b bug26158_031 https://git.torproject.org/tor.git

but I can't find this branch, could you give me a url or teach me the method to find the branch.
I am a new guy in this area :)

Version 0, edited 6 months ago by TBD.Chen (next)

comment:5 Changed 6 months ago by nickm

Whoops, sorry -- my branches are in my personal repository at https://git.torproject.org/nickm/tor.git !

comment:6 Changed 6 months ago by TBD.Chen

So that's it! Thank you for the explanation!
I have seen the modification, I think it is enough to remove the hidden danger.

comment:7 Changed 6 months ago by dgoulet

Reviewer: mikeperry

comment:8 Changed 5 months ago by dgoulet

Reviewer: mikeperrydgoulet
Status: needs_reviewmerge_ready

lgtm;

comment:9 Changed 5 months ago by nickm

Resolution: fixed
Status: merge_readyclosed

Merged to 0.3.1 and forward!

Note: See TracTickets for help on using tickets.