fix(console): require secure session key entropy - #228
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b00b762226
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fn generate_ephemeral_jwt_secret() -> String { | ||
| use sha2::Digest; | ||
|
|
||
| fn generate_ephemeral_jwt_secret() -> Result<String, std::io::Error> { |
There was a problem hiding this comment.
Route entropy failures through snafu
The newly fallible session-key path returns an ad hoc std::io::Error and constructs it manually below, so Console startup bypasses the repository-wide Snafu error model. Define a Snafu error variant for secure-randomness failure and propagate that typed result instead.
AGENTS.md reference: AGENTS.md:L64-L68
Useful? React with 👍 / 👎.
|
|
||
| let mut file = std::fs::File::open("/dev/urandom")?; | ||
| file.read_exact(bytes) | ||
| SystemRandom::new().fill(&mut bytes).map_err(|_| { |
There was a problem hiding this comment.
Log session-key generation only after it succeeds
When JWT_SECRET is absent and this new fill call fails, load_jwt_secret has already logged that it “generated an ephemeral Console session key,” even though no key was produced and startup then aborts. Move that warning after successful generation or change it to describe an attempted generation so operators do not receive a false success message in the exact failure scenario this change introduces.
Useful? React with 👍 / 👎.
Type of Change
Related Issues
Closes rustfs/backlog#1092
Summary of Changes
ring::rand::SystemRandom.The previous entropy fallback could produce a guessable session signing key if
/dev/urandomcould not be read. Deployments withJWT_SECRETconfigured are unchanged; unconfigured single-process deployments still receive an ephemeral key, now only from a CSPRNG.Checklist
make pre-commit(fmt-check + clippy + test + console-lint + console-fmt-check)[Unreleased](N/A: no changelog file exists in the repository)Impact
Verification
Additional Notes
The chart-provided
JWT_SECRETpath remains the recommended deployment configuration.Thank you for your contribution! Please ensure your PR follows the community standards (CODE_OF_CONDUCT.md) and sign the CLA if this is your first contribution.