Skip to content
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

Bug4631 #1747

Closed
wants to merge 9 commits into from
Closed

Bug4631 #1747

wants to merge 9 commits into from

Conversation

Labels
None yet
Projects
None yet
3 participants
@teor2345
Copy link
Contributor

@teor2345 teor2345 commented Feb 18, 2020

No description provided.

Roger Dingledine and others added 6 commits Feb 18, 2020
If we receive via 'post' a vote from a dir auth after the
fetch_missing_votes cutoff, that means we didn't get it by the time we
begin the "fetching missing votes from everybody else" phase, which means
it is very likely to cause a consensus split if we count it. Instead,
we reject it.

But we still allow votes that we fetch ourselves after that cutoff.

This is a demo branch for making progress on #4631.

I've been running it on moria1 and it catches and handles real buggy
behavior from directory authorities, e.g.

Jan 28 15:59:50.804 [warn] Rejecting vote from 199.58.81.140 received at 2020-01-28 20:59:50; our cutoff for received votes is 2020-01-28 20:52:30
Jan 28 15:59:50.805 [warn] Rejected vote from 199.58.81.140 ("Vote received too late, would be dangerous to count it").
Jan 29 01:52:52.667 [warn] Rejecting vote from 204.13.164.118 received at 2020-01-29 06:52:52; our cutoff for received votes is 2020-01-29 06:52:30
Jan 29 01:52:52.669 [warn] Rejected vote from 204.13.164.118 ("Vote received too late, would be dangerous to count it").
Jan 29 04:53:26.323 [warn] Rejecting vote from 204.13.164.118 received at 2020-01-29 09:53:26; our cutoff for received votes is 2020-01-29 09:52:30
Jan 29 04:53:26.326 [warn] Rejected vote from 204.13.164.118 ("Vote received too late, would be dangerous to count it").
Refactor dirvote_add_vote() by splitting some code out into static
functions.

Cleanup after 4631.
Update the function that handles directory authority votes when the
dirauth module is disabled.

Part of 4631.
@coveralls
Copy link

@coveralls coveralls commented Feb 18, 2020

Pull Request Test Coverage Report for Build 8160

  • 21 of 25 (84.0%) changed or added relevant lines in 3 files are covered.
  • 22 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.01%) to 63.439%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/feature/dircache/dircache.c 0 1 0.0%
src/feature/dirclient/dirclient.c 0 1 0.0%
src/feature/dirauth/dirvote.c 21 23 91.3%
Files with Coverage Reduction New Missed Lines %
src/feature/hs/hs_common.c 1 84.27%
src/core/or/scheduler_kist.c 9 79.76%
src/core/or/scheduler.c 12 84.44%
Totals Coverage Status
Change from base Build 8127: 0.01%
Covered Lines: 50049
Relevant Lines: 78893

💛 - Coveralls

@@ -3200,6 +3201,25 @@ dirvote_add_vote(const char *vote_body, const char **msg_out, int *status_out)
goto err;
}

if (!time_posted) { /* I imported this one myself */
log_notice(LD_DIR, "Retrieved vote from %s.", vi->address);
Copy link
Contributor

@nmathewson nmathewson Feb 18, 2020

Should we be logging this in all cases where we don't reject the vote?

Copy link
Contributor Author

@teor2345 teor2345 Feb 19, 2020

I don't think this log is correct. The vote is either downloaded from another authority, or loaded from disk. The comment "imported" conflicts with the log message "Retrieved", and neither cover both cases.

And I don't think it's helpful to log a notice, we already log lots of info about votes.

I removed it in 15192f8.

format_iso_time(tbuf2, voting_schedule.fetch_missing_votes);
log_warn(LD_DIR, "Rejecting vote from %s received at %s; "
"our cutoff for received votes is %s", vi->address, tbuf1, tbuf2);
*msg_out = "Vote received too late, would be dangerous to count it";
Copy link
Contributor

@nmathewson nmathewson Feb 18, 2020

Should this be a more actionable warning? It could suggest checking your clock is skewed, and checking whether your CPU is hosed. If neither of those is true, then we have a bug somewhere.

Copy link
Contributor Author

@teor2345 teor2345 Feb 19, 2020

Here are the different possible causes:

  • skewed clock
  • CPU load
  • network load
    Late votes can be caused by these issues on the local or remote authority.

I edited the log message in d8c719e.

@nmathewson
Copy link
Contributor

@nmathewson nmathewson commented Feb 20, 2020

squashed and merged

@nmathewson nmathewson closed this Feb 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment