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

Properly handle expired identities in gossip (release-2.5) #5122

Merged
merged 2 commits into from
Jan 27, 2025

Conversation

denyeart
Copy link
Contributor

When a peer's certificate expires, gossip still retains past messages it has sent, and gossips them to other peers.

Aside from peers doing redundant work, this also impairs their connectivity to the peer with the renewed certificate.

The reason is that peers try connect to the peer of the renewed certificate but abort because they cannot find its (old) PKI-ID in the identity store, which purged its old PKI-ID once its certificate has expired.

This commit fixes this problem by making the peer forget about peers that their identities have been purged from the identity store.

When a peer's certificate expires, gossip still retains past messages
it has sent, and gossips them to other peers.

Aside from peers doing redundant work, this also impairs their
connectivity to the peer with the renewed certificate.

The reason is that peers try connect to the peer of the renewed
certificate but abort because they cannot find its (old) PKI-ID
in the identity store, which purged its old PKI-ID once its
certificate has expired.

This commit fixes this problem by making the peer forget
about peers that their identities have been purged from
the identity store.

Signed-off-by: David Enyeart <[email protected]>
Signed-off-by: Yacov Manevich <[email protected]>
@denyeart denyeart requested a review from a team as a code owner January 27, 2025 13:31
yacovm
yacovm previously approved these changes Jan 27, 2025
@yacovm yacovm enabled auto-merge (squash) January 27, 2025 13:39
@denyeart denyeart force-pushed the gossip_rewnew_certs_25 branch from 76a8649 to d1fdf8d Compare January 27, 2025 15:35
The membership check via discovery does not work consistently
due to the renewed cert signature not matching expectations.
For now, it is sufficient to do the membership check
via checking the log.

Signed-off-by: David Enyeart <[email protected]>
@denyeart denyeart force-pushed the gossip_rewnew_certs_25 branch from d1fdf8d to 7854362 Compare January 27, 2025 16:03
@denyeart
Copy link
Contributor Author

denyeart commented Jan 27, 2025

@yacovm I had to update the integration tests for different timings in release-2.5 to avoid flakes.
This should be the last approval needed for the issue.
Also made main branch consistent in #5123

Finally, I've also opened PRs for v2.5.11 release to deliver the fix - #5124 and #5125

@yacovm yacovm merged commit 52bb909 into hyperledger:release-2.5 Jan 27, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants