Security: Zebra should stop gossiping unreachable addresses to other nodes, Action: re-deploy all nodes

[teor2345]

User Actions for the Changelog

Re-deploy all running Zebra nodes after fixing this bug. This bug fix allows the network to delete the unreachable peer addresses introduced by #2120, and kept alive by #1868.

Motivation

Zebra relays peer addresses to other nodes, even if it hasn't been able to contact those peers itself, and even if the untrusted_last_seen time gossiped by other nodes is a long way in the past.

Zebra is currently keeping unreachable addresses in its address book, and sending them to other peers, so this security fix is important and urgent.

Specification

"The typical presumption is that a node is likely to be active if it has been sending a message within the last three hours."

https://en.bitcoin.it/wiki/Protocol_documentation#getaddr

This time period is a network health / network view leakage design tradeoff.

Solution

This fix depends on #1849 and #1871.

Zebra should stop gossiping peers that haven't had a successful connection or sent a message for 3 hours.

To avoid sending old peers from other nodes, Zebra should stop gossiping peers where:

• the last_success_time is older than 3 hours
• the last_success_time is None, and the untrusted_last_seen_time is older than 3 hours (requires the far-future fix in Security: Zebra should stop believing far-future last_seen times from peers #1871)

We can make these changes in the AddressBook::sanitized function.

Zebra should also:

• provide an accessor method on MetaAddr to simplify the interface to the last_success_time and untrusted_last_seen_time
• avoid updating timestamps based on new gossips from other peers (security fixes must maintain this constraint)

Property testing:

• Generate some random MetaAddrs
• Put them in an AddressBook
• Sanitize the address book
• Make sure that old last_success_times and untrusted_last_seen_times are handled correctly

Acceptance testing:

• add a debug_gossip_max_peer_age config
• add an acceptance test that sets it to 10 seconds
• add a log when peers are skipped because they're too old, and check for that log

Alternatives

This is a serious security issue, so we must do something. If we find a simpler design that fixes this security issue, we should use it.

Context

zcashd and Zebra will check addresses for connectivity, then move on to other addresses. So this is not a critical security issue.

Follow-Up Tasks

We might want to do #1865 after this ticket - they modify the same code.

As a follow-up, we should review this design, and make sure that we're minimising network-based metadata leaks in Zebra.

[teor2345]

zcashd and Zebra will check addresses for connectivity, then move on to other addresses. So this is not a critical security issue.

[teor2345]

Moving this to sprint 10, so Zebra can clear out the bad addresses created by the bug in #2120.

[mpguerra]

Moving this to sprint 10, so Zebra can clear out the bad addresses created by the bug in #2120.

I have this as blocked by #1871 though so if this is still the case, we should also move #1871 to sprint 10 and move the 2 previously planned security issues (#1850 and #1848 ) out to sprint 11

[teor2345]

This is larger now we've added proptests.