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

feat: always generate agreement key on startup #16315

Merged
merged 1 commit into from
Oct 31, 2024

Conversation

edward-swirldslabs
Copy link
Contributor

Description:

This PR stops reading the agreement key from disk and always generates it on startup. The migration code will delete the agreement key if found and force generation anyway. This is a code cleanup operation so that when the migration code is removed, there are no potential bugs.

Potential Bug This Avoids:

  • PFX file contains an agreement key
  • Migration code is deleted and PFX files remain on disk because someone put them there again.
  • DAB has rotated the signing key and is using a new signing key in a PEM file.
  • Agreement key for old signing key is loaded from PFX file.
  • The mismatch between the old agreement key and new signing key will cause the node to fail to create mTLS connections with peers.

Fixes #16161

Notes for reviewer:
The only testing required and performed was unit testing. The special casing on one of the negative unit tests was removed because we are no longer reading a valid agreement key in that scenario.

Signed-off-by: Edward Wertz <edward@swirldslabs.com>
Copy link

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
-0.01% (target: -1.00%) 0.00%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (3daf1bf) 100220 62084 61.95%
Head commit (1f76a44) 100216 (-4) 62070 (-14) 61.94% (-0.01%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#16315) 1 0 0.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

@edward-swirldslabs edward-swirldslabs merged commit 0b2f935 into develop Oct 31, 2024
68 checks passed
@edward-swirldslabs edward-swirldslabs deleted the 16161-always-generate-agreement-key branch October 31, 2024 13:56
Evdokia-Georgieva pushed a commit that referenced this pull request Oct 31, 2024
Signed-off-by: Edward Wertz <edward@swirldslabs.com>
edward-swirldslabs added a commit that referenced this pull request Nov 6, 2024
Signed-off-by: Edward Wertz <edward@swirldslabs.com>
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.

Remove the code that loads agreement keys from disk
4 participants