Opened 7 years ago

Last modified 2 years ago

#6419 new defect

is it really a protocolwarn when connection_or_client_learned_peer_id() finds a different keyid?

Reported by: arma Owned by:
Priority: Low Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-relay logging needs-design bikeshed
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

moria1 running with protocolwarnings 1 gets a whole lot of

Jul 19 12:13:06.000 [warn] Tried connecting to router at 95.78.156.17:444, but identity key was not as expected: wanted 995B562DA3CB2A5BC26A2AF6FB1B1D4FC26DA1C9 but got E0B348D09A5AF111AB7A4E1831AED23B1D40239F.

This happens because somebody runs a relay at that address, blows it away, and runs another one. moria1 does reachability testing of both.

(A while ago we had code for moria1 to throw away a descriptor when it found that there was another descriptor on the same address and that descriptor's keyid was the one it sees. We threw out that code when ln5 was refactoring though.)

Child Tickets

Change History (22)

comment:1 Changed 7 years ago by arma

We could make it so directory authorities log at LOG_INFO, since we know they encounter this situation.

But a normal relay will see the protocol-warn when a client asks it to extend somewhere and provides the wrong keyid. Is that really a protocol violation?

comment:2 Changed 7 years ago by arma

(I think clients should continue to warn, since it tells them something about their network.)

comment:3 Changed 7 years ago by nickm

Keywords: tor-relay added

comment:4 Changed 7 years ago by nickm

Component: Tor RelayTor

comment:5 Changed 7 years ago by nickm

Priority: normalminor

The severity of this warning might want to depend on:

  • Whether you're a dirauth
  • Whether this is reachability testing that you're doing
  • Whether you're a client.
  • Whether you got the expected key from a consensus directory, an uploaded router descriptor, or an EXTEND/EXTEND2 cell.

If somebody can figure out the right set of boolean expressions, this will be easy to implement

comment:6 Changed 7 years ago by nickm

Keywords: 024-deferrable added

comment:7 Changed 7 years ago by andrea

19:10 < nickm> #6419 is next
19:11 < nickm> The fix is just "change a log severity."  First we need to decide whether to change the log severity.  
               Sounds like less than 30 minutes of work total.
19:11  * athena looks at #6419
19:11 < nickm> (Assuming I'm reading armadev right, which I might not be)
19:11 < asn> change log severity when dirauth, maybe.
19:13 < asn> change log severity when dirauth and reachability testing, maybe?
19:13 < asn> hm.
19:13 < nickm> asn: If you can determine the right answer and write a patch, the work is done here. :)
19:14 < asn> ha! nasty nickm!
19:14 < athena> yeah, i say keep and downgrade
19:14 < athena> i do that all the time when i run test relays
19:14 < nickm> to minor? ok
19:14 < nickm> oh, or downgrade the warning?
19:14 < athena> so it shouldn't be warning
19:15 < athena> yeah, the proposed solution in the ticket sounds correct to me
19:15 < asn> are there cases where a dirauth would like to see this warning?
19:15 < athena> but it also sounds easy enough that it arguably should be minor
19:15 < asn> shouldn't it warn when it  builds circuits to do non-reachability-testing things?
19:16 < asn> (what other things do dirauths do and need to build circuits for?)
19:16 < nickm> Actually hm.
19:16 < nickm> You want to get warned about this in some cases but not others maybe?
19:16 < asn> mmaybe the clause should be (if dirauth && reachability_testing) LOG_NOTICE, otherwise LOG_WARN. Although 
             that might bring it to the <more than 30 minutes of work> area.
19:17 < nickm> My thought was that: if you are extending to 1.2.3.4:1111 with ID ABCDEFABCDEFABCDEF12 and it gives you 
               some other ID, you don't care: the problem could be on the client's end; they could ahve told you a bad 
               key
19:17 < nickm> if you got the key out of a consensus directory yourself...
19:17 < nickm> ...or because a router uploaded it to you...
19:17 < nickm> maybe you care more?
19:17 < nickm> If that's the case, this is harder.
19:19 < athena> so, dirauths should warn because ... they should always get told about new routers so they should 
                always know the latest identity digest for a given IPv4 address?
19:19 < athena> is that necessarily true
19:19 < athena> ?
19:19 < nickm> I don't know
19:19 < nickm> I mean, I can make up a new identity key for your router and send it to all the DAs and they'll be like "oh reeeeeeeally?"
19:20 < athena> for example, suppose i set up a new router on an ip that just recently had a different key
19:20 < athena> and then, being tricksy for the sake of it, i mess with iptables so outgoing TCP connections from that 
                router to one particular dirauth are blocked
19:20 < athena> but incoming ones from the dirauth to the router are permitted
19:20 < nickm> I've commented, downgraded to minor, and added 024-deferrable. It should be easy once we decide what we 
               meant here, but it seems like we could skip it
19:20 < athena> then surely it's possible for the dirauth to see a different key than it expects

comment:8 Changed 6 years ago by nickm

Milestone: Tor: 0.2.4.x-finalTor: 0.2.5.x-final

comment:9 Changed 6 years ago by nickm

Milestone: Tor: 0.2.5.x-finalTor: 0.2.???

comment:10 Changed 5 years ago by nickm

Milestone: Tor: 0.2.???Tor: 0.2.7.x-final

These may be worth looking at for 0.2.7.

comment:11 Changed 5 years ago by nickm

Status: newassigned

comment:12 Changed 5 years ago by nickm

Keywords: 027-triaged-1-out added

Marking triaged-out items from first round of 0.2.7 triage.

comment:13 Changed 5 years ago by nickm

Milestone: Tor: 0.2.7.x-finalTor: 0.2.???

Make all non-needs_review, non-needs_revision, 027-triaged-1-out items belong to 0.2.???

comment:14 Changed 3 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:15 Changed 3 years ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:16 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:17 Changed 2 years ago by nickm

Keywords: 027-triaged-in added

comment:18 Changed 2 years ago by nickm

Keywords: 027-triaged-in removed

comment:19 Changed 2 years ago by nickm

Keywords: 027-triaged-1-out removed

comment:20 Changed 2 years ago by nickm

Keywords: 024-deferrable removed

comment:21 Changed 2 years ago by nickm

Status: assignednew

Change the status of all assigned/accepted Tor tickets with owner="" to "new".

comment:22 Changed 2 years ago by nickm

Keywords: logging needs-design bikeshed added
Severity: Normal
Note: See TracTickets for help on using tickets.