Skip to content

fix(security): redact credentials from debug log output#61

Open
martinraumann wants to merge 2 commits intoNVIDIA:mainfrom
martinraumann:fix/psirt-credential-redaction-in-debug-logs
Open

fix(security): redact credentials from debug log output#61
martinraumann wants to merge 2 commits intoNVIDIA:mainfrom
martinraumann:fix/psirt-credential-redaction-in-debug-logs

Conversation

@martinraumann
Copy link
Copy Markdown

@martinraumann martinraumann commented Apr 10, 2026

nvbug 6025253

Summary

  • `_req()` logged full request/response bodies at `debug` level with no redaction. With `RUST_LOG=libredfish=debug` enabled, plaintext passwords were emitted to any log aggregation pipeline (Splunk, ELK, Kubernetes pod logs, etc.)
  • Add `redact_sensitive_fields()` — applies a once-compiled static regex before the body reaches `debug!()`, replacing credential values with `[REDACTED]` while preserving key names and all non-sensitive fields
  • Fix response log sites to redact before truncating — truncating first could split a value string before its closing `"`, breaking the regex match and leaking a password prefix

Fields redacted

Key Operation
`Password` `create_user`, `change_password`, `change_password_by_id`
`OldPassword`, `NewPassword` `change_bios_password`, HPE UEFI password ops
`CurrentUefiPassword`, `UefiPassword` NVIDIA DPU `Bios/Settings` PATCH
`ImportBuffer` Dell `ImportSystemConfiguration` fallback (XML blob containing `OldSetupPassword`)

Design notes

  • Wire payload is never modified — only the string passed to `debug!()` is affected
  • Zero-cost fast path — `Cow::Borrowed` returned unchanged when no sensitive key is present (two `str::contains` checks, no regex)
  • Static regex — compiled once via `OnceLock`, not on every request

Test plan

  • `cargo test network::tests` — 10 unit tests covering each redacted field, the fast path, wire-payload immutability, escaped characters, and the truncation-ordering fix
  • `cargo test` — full suite passes (12 integration tests)

@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot bot commented Apr 10, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@martinraumann martinraumann force-pushed the fix/psirt-credential-redaction-in-debug-logs branch 3 times, most recently from 54424c9 to 108b321 Compare April 10, 2026 23:29
When RUST_LOG=libredfish=debug is enabled, _req() logged full request
and response bodies without redacting credential fields. Passwords
passed to create_user, change_password, change_bios_password, and
vendor-specific UEFI operations were emitted in plaintext to any log
aggregation pipeline.

Add redact_sensitive_fields() which runs a once-compiled static regex
against the serialised body before it is passed to debug!(). Fields
redacted:

  Password, OldPassword, NewPassword  — standard Redfish account/BIOS
  CurrentUefiPassword, UefiPassword   — NVIDIA DPU Bios/Settings PATCH
  ImportBuffer                        — Dell ImportSystemConfiguration
                                        (XML blob with OldSetupPassword)

The function returns Cow::Borrowed when no sensitive key is present,
so the common case (non-credential requests) has zero allocation cost.

Also fix the response log sites to redact before truncating, not after.
Truncating first could cut a value string before its closing quote,
breaking the regex match and leaking a partial secret.

The actual bytes sent to the BMC are never modified.

nvbug 6025253
Signed-off-by: Martin Raumann <mraumann@nvidia.com>
@martinraumann martinraumann force-pushed the fix/psirt-credential-redaction-in-debug-logs branch from 108b321 to 771127b Compare April 10, 2026 23:37
Comment thread src/network.rs
Comment thread src/network.rs Outdated
Implements the reviewer suggestion (PR NVIDIA#61): instead of calling
redact_sensitive_fields() eagerly at the TX debug! site, wrap the body
in a RedactPasswords<'a> newtype whose Display impl calls the function.
tracing evaluates format arguments lazily, so the regex now runs only
when debug logging is actually enabled.

RX sites retain explicit truncate(redact(...)) calls — the wrapper
cannot express that ordering, and getting it wrong would leak partial
secrets at truncation boundaries.

nvbug 6025253
Signed-off-by: Martin Raumann <mraumann@nvidia.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.

3 participants