Commit Graph

43 Commits

Author SHA1 Message Date
teor 4d608d3224
Stop doing thousands of time checks each time we connect to a peer (#3106)
* Stop checking the entire AddressBook for each connection attempt

* Stop redundant peer time checks within the address book

* Stop calling `Instant::now` 3 times for each address book update

* Only get the time once each time an address book method is called

* Update outdated comment

* Use an OrderedMap to efficiently store address book peers

* Add address book order tests
2021-12-03 15:09:43 -03:00
Janito Vaqueiro Ferreira Filho 11b5a33651
Security: Avoid reconnecting to peers that are likely unreachable (#3030)
* Add a `Duration32::from_days` constructor

Make it simpler to construct a `Duration32` representing a certain
number of days.

* Add `MetaAddr::was_not_recently_seen` method

A helper method to check if a peer was never seen before or if it was
last seen a long time ago. This will be one of the conditions to
consider a peer as unreachable.

* Add `MetaAddr::is_probably_unreachable` method

A helper method to check if a peer should be considered unreachable. It
is considered unreachable if recent connection attempts have failed and
it was not recently seen.

If a peer is considered unreachable, Zebra shouldn't attempt to connect
to it again.

* Do not keep trying to connect to unreachable peer

A peer is probably unreachable if it was last seen a long time ago and
if it's last connection attempt failed.

* Test `was_not_recently_seen`

Redo the calculation on arbitrary `MetaAddr`s.

* Test `is_probably_unreachable`

Redo the calculation on arbitrary `MetaAddr`s.

* Test if probably unreachable peers are ignored

Given an `AddressBook` with a list of arbitrary `MetaAddr`s, check that
none of the peers listed for a reconnection is probably unreachable.

* Rename unit test to improve clarity

Remove the double negative from the name.

Co-authored-by: teor <teor@riseup.net>

* Rename constant to `MAX_RECENT_PEER_AGE`

Make the purpose of the constant clearer.

Co-authored-by: teor <teor@riseup.net>

* Rename method to `last_seen_is_recent`

Remove the double negative from the name.

* Rename method to `is_probably_reachable`

Avoid having to negate the result of the method in security critical
filter.

* Move check into `is_ready_for_connection_attempt`

Make sure the check is used in any place that requires a peer that's
ready for a connection attempt.

* Improve test documention

Describe the goal of the test better.

Co-authored-by: teor <teor@riseup.net>

* Improve `is_probably_reachable` documentation

List the conditions as bullet points.

Co-authored-by: teor <teor@riseup.net>

* Document what happens when peers have no last seen time

Co-authored-by: teor <teor@riseup.net>
2021-11-10 23:51:22 +00:00
teor 85b016756d
Refactor addr v1 serialization using a separate AddrV1 type (#3021)
* Implement addr v1 serialization using a separate AddrV1 type

* Remove commented-out code

* Split the address serialization code into modules

* Reorder v1 and in_version fields in serialization order

* Fix a missed search-and-replace

* Explain conversion to MetaAddr

Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>

Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
2021-11-10 06:47:50 +10:00
teor 88cdf0f832
Move MetaAddr serialization into zebra_network::protocol::external (#3020)
Preparation for `addrv2` serialization.
2021-11-07 21:37:38 +00:00
Marek d03161c63f
Add unused seed peers to the AddressBook (#2974)
* Add unused seed peers to the AddressBook

* Document a new `await`

We added an extra await on the AddressBook thread mutex.

Co-authored-by: teor <teor@riseup.net>

* Fix a typo

* Refactor names

* Return early from `limit_initial_peers`

* Add `proptest`s regressions

* Return `MetaAddr` instead of `None`

* Test if `zebra_network::init()` deadlocks

* Remove unneeded regressions

* Rename `TimestampCollector` to `AddressBookUpdater` (#2992)

* Rename `TimestampCollector` to `AddressBookUpdater`

* Update comments

Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>

Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>

* Move `all_peers` instead of copying them

Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>

* Make `Duration` a const

Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>

* Use a timeout instead of measuring the elapsed time

Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>

* Copy `initial_peers` instead of moving them

* Refactor the position of `NewInitial` and `new_initial`

Co-authored-by: teor <teor@riseup.net>
Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
2021-11-04 08:34:00 -03:00
Janito Vaqueiro Ferreira Filho a9f1c189d9
Make `services` field in `MetaAddr` optional (#2976)
* Use `prop_assert` instead of `assert`

Otherwise the test input isn't minimized.

* Split long string into a multi-line string

And add some newlines to try to improve readability.

* Fix referenced issue number

They had a typo in their number.

* Make peer services optional

It is unknown for initial peers.

* Fix `preserve_initial_untrusted_values` test

Now that it's optional, the services field can be written to if it was
previously empty.

* Fix formatting of property tests

Run rustfmt on them.

* Restore `TODO` comment

Make it easy to find planned improvements in the code.

Co-authored-by: teor <teor@riseup.net>

* Comment on how ordering is affected

Make it clear that missing services causes the peer to be chosen last.

Co-authored-by: teor <teor@riseup.net>

* Don't expect `services` to be available

Avoid a panic by using the compiler to help enforce the handling of the
case correctly.

* Panic if received gossiped address has no services

All received gossiped addresses have services. The only addresses that
don't have services configured are the initial seed addresses.

Co-authored-by: teor <teor@riseup.net>
2021-11-02 02:45:35 +00:00
Janito Vaqueiro Ferreira Filho b68202c68a
Security: Zebra should stop gossiping unreachable addresses to other nodes, Action: re-deploy all nodes (#2392)
* Rename some methods and constants for clarity

Using the following commands:

```
fastmod '\bis_ready_for_attempt\b' is_ready_for_connection_attempt
  # One instance required a tweak, because of the ASCII diagram.
fastmod '\bwas_recently_live\b' has_connection_recently_responded
fastmod '\bwas_recently_attempted\b' was_connection_recently_attempted
fastmod '\bwas_recently_failed\b' has_connection_recently_failed
fastmod '\bLIVE_PEER_DURATION\b' MIN_PEER_RECONNECTION_DELAY
```

* Use `Instant::elapsed` for conciseness

Instead of `Instant::now().saturating_duration_since`. They're both
equivalent, and `elapsed` only panics if the `Instant` is somehow
synthetically generated.

* Allow `Duration32` to be created in other crates

Export the `Duration32` from the `zebra_chain::serialization` module.

* Add some new `Duration32` constructors

Create some helper `const` constructors to make it easy to create
constant durations. Add methods to create a `Duration32` from seconds,
minutes and hours.

* Avoid gossiping unreachable peers

When sanitizing the list of peers to gossip, remove those that we
haven't seen in more than three hours.

* Test if unreachable addresses aren't gossiped

Create a property test with random addreses inserted into an
`AddressBook`, and verify that the sanitized list of addresses does not
contain any addresses considered unreachable.

* Test if new alternate address isn't gossipable

Create a new alternate peer, because that type of `MetaAddr` does not
have `last_response` or `untrusted_last_seen` times. Verify that the
peer is not considered gossipable.

* Test if local listener is gossipable

The `MetaAddr` representing the local peer's listening address should
always be considered gossipable.

* Test if gossiped peer recently seen is gossipable

Create a `MetaAddr` representing a gossiped peer that was reported to be
seen recently. Check that the peer is considered gossipable.

* Test peer reportedly last seen in the future

Create a `MetaAddr` representing a peer gossiped and reported to have
been last seen in a time that's in the future. Check that the peer is
considered gossipable, to check that the fallback calculation is working
as intended.

* Test gossiped peer reportedly seen long ago

Create a `MetaAddr` representing a gossiped peer that was reported to
last have been seen a long time ago. Check that the peer is not
considered gossipable.

* Test if just responded peer is gossipable

Create a `MetaAddr` representing a peer that has just responded and
check that it is considered gossipable.

* Test if recently responded peer is gossipable

Create a `MetaAddr` representing a peer that last responded within the
duration a peer is considered reachable. Verify that the peer is
considered gossipable.

* Test peer that responded long ago isn't gossipable

Create a `MetaAddr` representing a peer that last responded outside the
duration a peer is considered reachable. Verify that the peer is not
considered gossipable.
2021-06-29 05:12:27 +00:00
teor 1a57023eac
Security: Use canonical SocketAddrs to avoid duplicate peer connections, Feature: Send local listener to peers (#2276)
* Always send our local listener with the latest time

Previously, whenever there was an inbound request for peers, we would
clone the address book and update it with the local listener.

This had two impacts:
- the listener could conflict with an existing entry,
  rather than unconditionally replacing it, and
- the listener was briefly included in the address book metrics.

As a side-effect, this change also makes sanitization slightly faster,
because it avoids some useless peer filtering and sorting.

* Skip listeners that are not valid for outbound connections

* Filter sanitized addresses Zebra based on address state

This fix correctly prevents Zebra gossiping client addresses to peers,
but still keeps the client in the address book to avoid reconnections.

* Add a full set of DateTime32 and Duration32 calculation methods

* Refactor sanitize to use the new DateTime32/Duration32 methods

* Security: Use canonical SocketAddrs to avoid duplicate connections

If we allow multiple variants for each peer address, we can make multiple
connections to that peer.

Also make sure sanitized MetaAddrs are valid for outbound connections.

* Test that address books contain the local listener address

Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
2021-06-22 02:16:59 +00:00
teor 4d22a0bae9
Security: Limit reconnection rate to individual peers (#2275)
* Security: Limit reconnection rate to individual peers

Reconnection Rate

Limit the reconnection rate to each individual peer by applying the
liveness cutoff to the attempt, responded, and failure time fields.
If any field is recent, the peer is skipped.

The new liveness cutoff skips any peers that have recently been attempted
or failed. (Previously, the liveness check was only applied if the peer
was in the `Responded` state, which could lead to repeated retries of
`Failed` peers, particularly in small address books.)

Reconnection Order

Zebra prefers more useful peer states, then the earliest attempted,
failed, and responded times, then the most recent gossiped last seen
times.

Before this change, Zebra took the most recent time in all the peer time
fields, and used that time for liveness and ordering. This led to
confusion between trusted and untrusted data, and success and failure
times.

Unlike the previous order, the new order:
- tries all peers in each state, before re-trying any peer in that state,
  and
- only checks the the gossiped untrusted last seen time
  if all other times are equal.

* Preserve the later time if changes arrive out of order

* Update CandidateSet::next documentation

* Update CandidateSet state diagram

* Fix variant names in comments

* Explain why timestamps can be left out of MetaAddrChanges

* Add a simple test for the individual peer retry limit

* Only generate valid Arbitrary PeerServices values

* Add an individual peer retry limit AddressBook and CandidateSet test

* Stop deleting recently live addresses from the address book

If we delete recently live addresses from the address book, we can get a
new entry for them, and reconnect too rapidly.

* Rename functions to match similar tokio API

* Fix docs for service sorting

* Clarify a comment

* Cleanup a variable and comments

* Remove blank lines in the CandidateSet state diagram

* Add a multi-peer proptest that checks outbound attempt fairness

* Fix a comment typo

Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>

* Simplify time maths in MetaAddr

* Create a Duration32 type to simplify calculations and comparisons

* Rename variables for clarity

* Split a string constant into multiple lines

* Make constants match rustdoc order

Co-authored-by: Janito Vaqueiro Ferreira Filho <janito.vff@gmail.com>
2021-06-18 09:30:44 -03:00
teor 3f7410d073
Security: stop gossiping failure and attempt times as last_seen times (#2273)
* Security: stop gossiping failure and attempt times as last_seen times

Previously, Zebra had a single time field for peer addresses, which was
updated every time a peer was attempted, sent a message, or failed.

This is a security issue, because the `last_seen` time should be
"the last time [a peer] connected to that node", so that
"nodes can use the time field to avoid relaying old 'addr' messages".
So Zebra was sending incorrect peer information to other nodes.

As part of this change, we split the `last_seen` time into the
following fields:
- untrusted_last_seen: gossiped from other peers
- last_response: time we got a response from a directly connected peer
- last_attempt: time we attempted to connect to a peer
- last_failure: time a connection with a peer failed

* Implement Arbitrary and strategies for MetaAddrChange

Also replace the MetaAddr Arbitrary impl with a derive.

* Write proptests for MetaAddr and MetaAddrChange

MetaAddr:
- the only times that get included in serialized MetaAddrs are
  the untrusted last seen and responded times

MetaAddrChange:
- the untrusted last seen time is never updated
- the services are only updated if there has been a handshake
2021-06-15 13:31:16 +10:00
Janito Vaqueiro Ferreira Filho 29c51d5086 Implement `MetaAddr::set_last_seen` setter method
Will be used when limiting the reported last seen times for recived
gossiped addresses.
2021-06-01 03:42:08 -03:00
teor ebe1c9f88e
Add a DateTime32 type for 32-bit serialized times (#2210)
* Add a DateTime32 type for 32-bit serialized times
* Use DateTime32 for MetaAddr.last_seen
* Create and use a `DateTime32::now` method
2021-05-31 12:52:34 +10:00
teor 81630d19f2 Add service sanitization to `MetaAddr::sanitize`
This makes sure that deserialization and generated `MetaAddr`s are consistent.
2021-05-26 18:13:35 -04:00
teor bf6fe175dd Stop deriving PartialEq for MetaAddr
This makes sure Ord and ParitalEq are always consistent.
2021-05-26 18:13:35 -04:00
teor c0114a2c5f Security: Stop panicking when serializing out-of-range times
Zebra assumes that deserialized times are always able to be serialized.

But this assumption is wrong because:
- sanitization can modify times
- gossiped `MetaAddr` validation can modify times
2021-05-26 18:13:35 -04:00
teor 752358d236
Fix some candidate set and meta addr doc links (#2174)
Suggested by jvff.
2021-05-21 11:40:14 +10:00
teor 40d06657b3 Update new_gossiped_meta_addr to the latest API 2021-05-21 06:51:34 +10:00
teor 92828bbb29 Reliability: send local listener address to peers
When peers ask for peer addresses, add our local listener address to the
set of addresses, sanitize, then truncate. Sanitize shuffles addresses,
so if there are lots of addresses in the address book, our address will
only be sent to some peers.
2021-05-18 14:02:19 +10:00
teor b600e82d6e
Security: Avoid silently corrupting invalid times during serialization (#2149)
* Security: panic if an internally generated time is out of range

If Zebra has a bug where it generates blocks, transactions, or meta
addresses with bad times, panic. This avoids sending bad data onto the
network.

(Previously, Zebra would truncate some of these times, silently
corrupting the underlying data.)

Make it clear that deserialization of these objects is infalliable.
2021-05-17 16:53:10 -04:00
teor c40cbee42f Remove address book peers that have changed to clients
If an address book peer stops advertising the NODE_SERVICES bit, remove
it from the address book.
2021-05-14 23:45:42 +10:00
teor a8a0d6450c Security: stop gossiping temporary inbound remote addresses to peers
- stop putting inbound addresses in the address book
- drop address book entries that can't be used for outbound connections
  - distinguish between temporary inbound and permanent outbound peer
    addresses
  - also create variants to handle proxy connections
    (but don't use them yet)
  - avoid tracking connection state for isolated connections
- document security constraints for the address book and peer set
2021-05-14 23:45:42 +10:00
teor 1626ec383a
Add InventoryHash and MetaAddr proptests (#1985)
* Make proptest dependencies consistent between chain and network

* Implement Arbitrary for InventoryHash and use it in tests

* Impl Arbitrary for MetaAddr and use it in tests

Also test some extreme times in MetaAddr sanitization.
2021-04-07 14:13:52 -03:00
teor 64662a758d
Move the preallocate tests into their own files (#1977)
* Move the preallocate tests into their own files

And move the MetaAddr proptest into its own file.

Also do some minor formatting and cleanups.

Co-authored-by: Deirdre Connolly <durumcrustulum@gmail.com>
2021-04-07 12:32:27 +10:00
Preston Evans 0daaf582e2
Implement Trusted Vector Preallocation (#1920)
* Implement SafePreallocate. Resolves #1880

* Add proptests for SafePreallocate

* Apply suggestions from code review

Comments which did not include replacement code will be addressed in a follow-up commit.

Co-authored-by: teor <teor@riseup.net>

* Rename [Safe-> Trusted]Allocate. Add doc and tests

Add tests to show that the largest allowed vec under TrustedPreallocate
is small enough to fit in a Zcash block/message (depending on type).
Add doc comments to all TrustedPreallocate test cases.
Tighten bounds on max_trusted_alloc for some types.

Note - this commit does NOT include TrustedPreallocate
impls for JoinSplitData, String, and Script.
These impls will be added in a follow up commit

* Implement SafePreallocate. Resolves #1880

* Add proptests for SafePreallocate

* Apply suggestions from code review

Comments which did not include replacement code will be addressed in a follow-up commit.

Co-authored-by: teor <teor@riseup.net>

* Rename [Safe-> Trusted]Allocate. Add doc and tests

Add tests to show that the largest allowed vec under TrustedPreallocate
is small enough to fit in a Zcash block/message (depending on type).
Add doc comments to all TrustedPreallocate test cases.
Tighten bounds on max_trusted_alloc for some types.

Note - this commit does NOT include TrustedPreallocate
impls for JoinSplitData, String, and Script.
These impls will be added in a follow up commit

* Impl TrustedPreallocate for Joinsplit

* Impl ZcashDeserialize for Vec<u8>

* Arbitrary, TrustedPreallocate, Serialize, and tests for Spend<SharedAnchor>

Co-authored-by: teor <teor@riseup.net>
2021-04-06 09:49:42 +10:00
teor 1a159dfcb6 Add more methods for creating MetaAddrs
This refactor lets us remove `MetaAddr::update_last_seen()`.
2021-03-26 07:23:49 +10:00
teor 6fe81d8992 Make MetaAddr.last_seen into a private field 2021-03-26 07:23:49 +10:00
teor eae59de1e8 use PeerAddrState::* 2021-03-26 07:23:49 +10:00
teor e9cdc224a2 Rewrite MetaAddr::sanitize so it's harder to misuse
`sanitize` could be misused in two ways:
* accidentally modifying the addresses in the address book itself
* forgetting to sanitize new fields added to `MetaAddr`

This change prevents accidental modification by taking `&self`, and
explicitly creates a new sanitized `MetaAddr` with all fields listed.
2021-03-26 07:23:49 +10:00
teor 5424e1d8ba
Fix candidate set address state handling (#1709)
Design:
- Add a `PeerAddrState` to each `MetaAddr`
- Use a single peer set for all peers, regardless of state
- Implement time-based liveness as an `AddressBook` method, rather than
  a `PeerAddrState` variant
- Delete `AddressBook.by_state`

Implementation:
- Simplify `AddressBook` changes using `update` and `take` modifier
  methods
- Simplify the `AddressBook` iterator implementation, replacing it with
  methods that are more obviously correct
- Consistently collect peer set metrics

Documentation:
- Expand and update the peer set documentation

We can optimise later, but for now we want simple code that is more
obviously correct.
2021-02-18 11:18:32 +10:00
Alfredo Garcia 128643d81e
Call `zebra_test::init` where needed. (#1227)
* Add missing `zebra_test::init()` to zebra-chain
* Add missing `zebra_test::init()` to zebra-consensus
* Add missing `zebra_test::init()` to zebra-network
* Add missing `zebra_test::init()` to zebra-state
* Add missing `zebra_test::init()` to zebra-test
* Add missing `zebra_test::init()` to zebrad
2020-11-10 10:29:25 +10:00
Jane Lusby b6b35364f3 cleanup warnings throughout codebase 2020-05-27 15:42:29 -04:00
Henry de Valence 972d16518f Make ZcashSerialize infallible mod its Writer.
Closes #158.

As discussed on the issue, this makes it possible to safely serialize
data into hashes, and encourages serializable data to make illegal
states unrepresentable.
2020-02-05 19:48:43 -05:00
Henry de Valence d170608c13 Remove outdated note.
The MetaAddr type is used not just in the Bitcoin code so it doesn't make sense
to keep it in the Bitcoin protocol implementation.
2019-11-13 18:43:18 -05:00
Henry de Valence 9a0bffecb8 Sanitize outbound address responses.
This aims to prevent a remote peer from inspecting timings of all messages
received by this node.
2019-11-13 18:43:18 -05:00
Henry de Valence 39d38a8647 Rewrite AddressBook to use a BTreeSet.
The previous implementation failed when timestamps were duplicated between
peers, because there was not a 1-1 relationship between timestamps and peers.
2019-10-21 14:40:03 -04:00
Henry de Valence f5dca597dd Replace PeerServices(u64) with a bitflags struct.
This gives considerably better ergonomics.
2019-10-01 01:07:56 -04:00
Henry de Valence 9603a29399 Rename `Services` to `PeerServices`.
This field is called `services` in Bitcoin and Zcash, but because we use
that word internally for other purposes, calling it `PeerServices`
disambiguates the meaning to "the services advertised by the peer",
rather than, e.g., a `tower::Service`.
2019-10-01 01:07:56 -04:00
Henry de Valence 0a85be285d Add addr, getaddr serialization. 2019-09-27 20:41:45 -04:00
Henry de Valence ab06750db3 zebra-network: move types -> protocol::types
These types are used for protocol messages, so it makes more sense to
keep them scoped with the protocol handling, rather than other
networking logic.
2019-09-27 20:41:45 -04:00
Henry de Valence f45bbeba98
Replace `Version` `MetaAddr` with `(Services, SocketAddr)`. (#12)
* Replace Version MetaAddr by (Services, SocketAddr).

The version handshake message doesn't include last-seen timestamps for
the address fields, unlike other messages, so instead of modeling the
message data with a `MetaAddr` (which includes a timestamp), we should
just use a tuple.

* Simplify try_read_version implementation.

Because we no longer need to construct fake timestamps for the
`MetaAddr` fields, we don't need to use any of the parsed fields while
parsing later fields, and we can neatly wrap up the entire parsing logic
into a single expression.

* fmt

I didn't have the toolchain-specified `rustfmt` because I was mostly
offline and couldn't download it.
2019-09-19 09:38:02 -07:00
Henry de Valence 733d090b9b Add missing derives to newtypes. 2019-09-18 17:32:06 -04:00
Henry de Valence 1f280b7bb8 Make MetaAddr fields public. 2019-09-18 17:32:06 -04:00
Henry de Valence b9af047a09 Introduce a `MetaAddr` type replacing `NetworkAddress`.
The `NetworkAddress` type was a `(Services, SocketAddr)` pair as used in the
`version` handshake message, described as the `net_addr` struct in the Bitcoin
wiki protocol documentation.  However, all of the other uses of the `net_addr`
struct are a `(Timestamp, Services, SocketAddr)` pair (where the timestamp is
the last-seen time of the peer), and the timestamp is omitted only during the
`version` messages, which are used only during the handshake, so it seems
better to include the timestamp field and omit it during serialization of
`version` packets.
2019-09-18 17:32:06 -04:00