Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider disconnecting from peers with skewed clocks #2167

Closed
5 tasks
Tracked by #3247
jvff opened this issue May 20, 2021 · 1 comment
Closed
5 tasks
Tracked by #3247

Consider disconnecting from peers with skewed clocks #2167

jvff opened this issue May 20, 2021 · 1 comment
Labels
A-network Area: Network protocol updates or fixes C-security Category: Security issues I-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data

Comments

@jvff
Copy link
Contributor

jvff commented May 20, 2021

Motivation

Zebra currently accepts gossiped peers without verifying the last_seen time reported. This could lead to issues when the clocks are skewed, and is planned to be handled by a separate issue (#1871). However, that only normalizes the reported times to minimize the effect of the skewed clock. There might be further gain by disconnecting from peers whose clocks are skewed too much.

For example, Zcash nodes reject blocks that are more than 2 hours in the future, compared to their local clock. So peers that are more than two hours behind consensus time can't validate blocks from other peers, while peers that are more than two hours ahead of consensus time can't generate blocks that other peers will accept.

Solution

The proposed solution is to not connect to peers that have clocks skewed more than a certain threshold. This should be discussed more to decide:

  • Should peers in the future be disconnected?
  • Should peers in the past be disconnected?
  • What should be the thresholds used?
  • What happens if the local node has a clock that's skewed above or below the thresholds?
  • Should disconnections only happen if the connection slots (in PeerSet?) are full?

Alternatives

Leaving things as is isn't a big issue AFAICT, but it does lead to waste of resources because the node will spend a connection slot to communicate with a peer that might not be the optimal one to communicate with.

@jvff jvff added C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage labels May 20, 2021
@teor2345
Copy link
Contributor

I just tweaked the descriptions, and added the specific consensus rule.

@mpguerra mpguerra added A-network Area: Network protocol updates or fixes P-Low labels May 20, 2021
@mpguerra mpguerra added A-network Area: Network protocol updates or fixes and removed A-network Area: Network protocol updates or fixes S-needs-triage Status: A bug report needs triage labels May 31, 2021
@teor2345 teor2345 added C-security Category: Security issues I-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data and removed C-enhancement Category: This is an improvement labels Dec 16, 2021
@ZcashFoundation ZcashFoundation locked and limited conversation to collaborators Feb 24, 2022
@jvff jvff converted this issue into discussion #3633 Feb 24, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
A-network Area: Network protocol updates or fixes C-security Category: Security issues I-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data
Projects
None yet
Development

No branches or pull requests

3 participants