STR-502: require secure RNG for seed encryption

AaronFeickert commented 1 month ago

Description

Seed encryption requires the caller provide an RNG, which currently requires only the RngCore trait. This allows the caller to provide an insecure generator.

This PR requires the caller to provide a cryptographically-secure generator via the CryptoRng trait.

Type of Change

[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature/Enhancement (non-breaking change which adds functionality or enhances an existing one)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] Documentation update
[ ] Refactor

Checklist

[x] I have performed a self-review of my code.
[x] I have commented my code where necessary.
[x] I have updated the documentation if needed.
[x] My changes do not introduce new warnings.
[ ] I have added tests that prove my changes are effective or that my feature works.
[x] New and existing tests pass with my changes.

Related Issues

Unit testing of seed encryption functionality is blocked by STR-506.

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 57.15%. Comparing base (22e9592) to head (bc87ce4). Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
bin/strata-cli/src/seed.rs	0.00%	6 Missing :warning:

@@            Coverage Diff             @@
##             main     #385      +/-   ##
==========================================
+ Coverage   57.06%   57.15%   +0.09%     
==========================================
  Files         255      255              
  Lines       26978    26980       +2     
==========================================
+ Hits        15394    15420      +26     
+ Misses      11584    11560      -24

Files with missing lines	Coverage Δ
bin/strata-cli/src/seed.rs	`0.00% <0.00%> (ø)`

... and 6 files with indirect coverage changes

Zk2u commented 1 month ago

LGTM. Let's wait until #382 gets merged though.

storopoli commented 1 month ago

@delbonis these CI failures (they passed before) is due to the flaky functional tests that you are working on?

alpenlabs / strata