Repository navigation
fix(security): resolve CodeQL hard-coded-crypto alerts in portal auth - #108
Merged
Merged
Conversation
Resolves rust/hard-coded-cryptographic-value alerts #22 and #23. The signing key was materialised from a zero-initialised [0u8; 32] buffer (read path via hex_decode_32, create path via fill_bytes), which CodeQL flags because the constant value flows into the Hmac<Sha256> key at auth.rs:240/255. Apply the proven PR #100 pattern: - hex_decode_32 now returns Option<[u8;32]> built from a fresh Vec, then try_from (no constant-initialised buffer). - the create path draws the key directly with rand::rng().random(). Behaviour is identical: same 32-byte key, same on-disk hex format, same corrupt-file rejection, cookies still stable across restarts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Resolves CodeQL
rust/hard-coded-cryptographic-valuealerts #22 and #23 insrc/portal/auth.rs.Root cause
signing_secret()materialised the 32-byte HMAC signing key from a zero-initialised[0u8; 32]buffer on both paths:portal_secret.keylet mut buf = [0u8; 32]; hex_decode_32(..., &mut buf)let mut buf = [0u8; 32]; fill_bytes(&mut rng, &mut buf)The constant value flows into
Hmac::<Sha256>::new_from_slice(...)(auth.rs:240 / :255), so CodeQL flags it as a hard-coded key. The values were always overwritten first — this is a false positive in practice, but the pattern is what the rule is designed to catch, and it is trivially avoidable.Fix (same proven pattern as PR #100)
hex_decode_32(raw) -> Option<[u8; 32]>now parses into a freshVec<u8>then<[u8;32]>::try_from(...)— the key is never materialised from a constant-initialised buffer.rand::rng().random().Behaviour is identical: same 32-byte key, same on-disk hex format, same corrupt-file rejection, cookies still stable across restarts.
Verification
cargo fmtcargo clippy --all-targets -- -D warningscargo testportal::authunit testshex_decode_32_roundtrip,cookie_sign_verify_roundtrip_and_tamper)tests/portal_api.rscookie_survives_new_state_same_home)CodeQL will re-scan this PR ref; expected
rustfindings drop from 5 → 3 (the 3 test-only alerts remain and are dismissed separately).Related
[9u8;32],[4u8;32], tempdir path) — dismissed separately with reason "used in tests".