Description
openedon Mar 5, 2021
Motivation
Zebra updates the same time field for different peer events, making it hard to:
- securely handle peer addresses
- provide accurate timestamps when gossiping peers to other nodes
Solution
Desired Changes
Zebra should:
- track
Alternate
andFailed
addresses separately (Security: stop gossiping temporary inbound remote addresses to peers #2120)
Zebra should track the following times for each peer:
-
untrusted_last_seen
remote time gossiped by the peer that told us about this address -
last_success
local time when we last saw a message from this peer -
last_attempt
local time when we last attempted to connect to this peer -
last_failed
local time when we last failed to connect to this peer
We might want to also refactor the services
field into untrusted_indirect_services
and untrusted_direct_services
as part of this change. (Neither field is trusted, but one of them came directly from the peer, and the other came via other peers.)
This might be easier to implement using an enum with Direct
(confirmed addr, claimed services) and Indirect
(definitely unconfirmed) variants.
Incidental Changes
It might be difficult to maintain exactly the same next peer address selection behaviour after this change. If we can't keep the same behaviour, we should make the fix in #1876 as part of this change.
Correctness
Correctness:
- reject updates that move existing peers into the
NeverAttempted...
states. (Updates can only beAttemptPending
,Responded
, orFailed
.) - ensure that all inserts of new peers are in the
NeverAttempted...
states. - keep the oldest timestamps for
NeverAttempted...
peers.
Alternatives
It would be nice to keep fewer times for each peer, but these times all seem to be required by upcoming network security fixes. We can provide accessor methods on MetaAddr
to simplify the interface to these times.
We could also keep an address_entry_creation_time
, but after the fix in #1871, we can safely use untrusted_last_seen_time
instead, because untrusted_last_seen_time
is earlier than or equal to address_entry_creation_time
. Using untrusted_last_seen_time
is better when the peer is honest, because it contains useful information about the last time that peer was available.
Follow Up
We might want to remove the Ord
impl, and just use a custom sort function for each different purpose (next address, removing addresses, etc.).
Context
zcashd
does not have this issue.