Skip to content

Commit 5cda151

Browse files
fix(security): redact credentials from debug log output (#61)
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) --------- Signed-off-by: Martin Raumann <mraumann@nvidia.com>
1 parent 4ac933a commit 5cda151

1 file changed

Lines changed: 150 additions & 10 deletions

File tree

src/network.rs

Lines changed: 150 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,9 @@
2020
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
2121
* DEALINGS IN THE SOFTWARE.
2222
*/
23-
use std::{collections::HashMap, path::Path, time::Duration};
23+
use std::{borrow::Cow, collections::HashMap, path::Path, sync::OnceLock, time::Duration};
2424

25+
use regex::Regex;
2526
use reqwest::{
2627
header::{HeaderMap, HeaderName, HeaderValue, ACCEPT, CONTENT_TYPE, IF_MATCH},
2728
multipart::{Form, Part},
@@ -501,7 +502,7 @@ impl RedfishHttpClient {
501502
let body_enc =
502503
serde_json::to_string(b).map_err(|e| RedfishError::JsonSerializeError {
503504
url,
504-
object_debug: format!("{b:?}"),
505+
object_debug: redact_sensitive_fields(&format!("{b:?}")).into_owned(),
505506
source: e,
506507
})?;
507508

@@ -513,7 +514,7 @@ impl RedfishHttpClient {
513514
"TX {} {} {}",
514515
method,
515516
url,
516-
body_enc.as_deref().unwrap_or_default()
517+
RedactPasswords(body_enc.as_deref().unwrap_or_default())
517518
);
518519
let mut req_b = match *method {
519520
Method::GET => self.http_client.get(&url),
@@ -588,7 +589,7 @@ impl RedfishHttpClient {
588589
url: url.clone(),
589590
source: e,
590591
})?;
591-
debug!("RX {status_code} {}", truncate(&response_body, 1500));
592+
debug!("RX {status_code} {}", truncate(&redact_sensitive_fields(&response_body), 1500));
592593

593594
if !status_code.is_success() {
594595
if status_code == StatusCode::FORBIDDEN && !response_body.is_empty() {
@@ -739,7 +740,7 @@ impl RedfishHttpClient {
739740
url: url.to_string(),
740741
source: e,
741742
})?;
742-
debug!("RX {status_code} {}", truncate(&response_body, 1500));
743+
debug!("RX {status_code} {}", truncate(&redact_sensitive_fields(&response_body), 1500));
743744

744745
if !status_code.is_success() {
745746
return Err(RedfishError::HTTPErrorCode {
@@ -757,10 +758,149 @@ fn truncate(s: &str, len: usize) -> &str {
757758
&s[..len.min(s.len())]
758759
}
759760

760-
#[test]
761-
fn test_truncate() {
762-
assert_eq!(truncate("", 1500), "");
761+
/// Redacts known sensitive JSON fields for safe logging.
762+
///
763+
/// Operates directly on the serialised JSON string to avoid re-serialisation
764+
/// cost. Returns `Cow::Borrowed(body)` unchanged when no sensitive field
765+
/// names are present (zero-copy fast path). The actual bytes sent over the
766+
/// wire are **never** modified — only the string passed to this function is
767+
/// affected.
768+
///
769+
/// Redacted fields (exact, case-sensitive JSON key match):
770+
/// `Password`, `OldPassword`, `NewPassword` — standard Redfish account/BIOS ops
771+
/// `CurrentUefiPassword`, `UefiPassword` — NVIDIA DPU Bios/Settings PATCH
772+
/// `ImportBuffer` — Dell ImportSystemConfiguration XML blob
773+
774+
/// A `Display` wrapper that redacts sensitive JSON fields on formatting.
775+
///
776+
/// Passing this to `tracing::debug!` defers evaluation until the macro decides the
777+
/// message will actually be emitted, so the regex never runs at non-debug log levels.
778+
struct RedactPasswords<'a>(&'a str);
779+
780+
impl std::fmt::Display for RedactPasswords<'_> {
781+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
782+
redact_sensitive_fields(self.0).fmt(f)
783+
}
784+
}
785+
786+
fn redact_sensitive_fields(body: &str) -> Cow<'_, str> {
787+
// Fast path: skip regex engine entirely when no sensitive key is present.
788+
// "Password" covers all five password-style keys; "ImportBuffer" covers the
789+
// Dell XML-in-JSON fallback path.
790+
if !body.contains("Password") && !body.contains("ImportBuffer") {
791+
return Cow::Borrowed(body);
792+
}
793+
794+
static REDACT_RE: OnceLock<Regex> = OnceLock::new();
795+
let re = REDACT_RE.get_or_init(|| {
796+
// Matches a JSON key from the sensitive list followed by its quoted string value
797+
// (including JSON escape sequences). The key is captured in group 1 so it can
798+
// be preserved verbatim in the replacement.
799+
Regex::new(
800+
r#""(Password|OldPassword|NewPassword|CurrentUefiPassword|UefiPassword|ImportBuffer)"\s*:\s*"(?:[^"\\]|\\.)*""#,
801+
)
802+
.expect("hardcoded redaction regex must be valid")
803+
});
804+
805+
re.replace_all(body, r#""$1":"[REDACTED]""#)
806+
}
807+
808+
#[cfg(test)]
809+
mod tests {
810+
use super::*;
811+
812+
#[test]
813+
fn test_truncate() {
814+
assert_eq!(truncate("", 1500), "");
815+
816+
let big = "a".repeat(2000);
817+
assert_eq!(truncate(&big, 1500).len(), 1500);
818+
}
819+
820+
#[test]
821+
fn redact_password_field() {
822+
let body = r#"{"UserName":"admin","Password":"s3cr3t!"}"#;
823+
let redacted = redact_sensitive_fields(body);
824+
assert!(!redacted.contains("s3cr3t!"), "plaintext password must not appear in log output");
825+
assert!(redacted.contains("[REDACTED]"));
826+
assert!(redacted.contains("UserName"), "non-sensitive fields must be preserved");
827+
}
828+
829+
#[test]
830+
fn redact_old_and_new_password_fields() {
831+
let body =
832+
r#"{"PasswordName":"AdministratorPassword","OldPassword":"old123","NewPassword":"new456"}"#;
833+
let redacted = redact_sensitive_fields(body);
834+
assert!(!redacted.contains("old123"), "OldPassword value must be redacted");
835+
assert!(!redacted.contains("new456"), "NewPassword value must be redacted");
836+
// PasswordName is a slot name, not a secret — must NOT be redacted.
837+
assert!(redacted.contains("AdministratorPassword"), "PasswordName value must not be redacted");
838+
}
839+
840+
#[test]
841+
fn nvidia_dpu_uefi_password_fields_are_redacted() {
842+
let body = r#"{"Attributes":{"CurrentUefiPassword":"old_secret","UefiPassword":"new_secret"}}"#;
843+
let redacted = redact_sensitive_fields(body);
844+
assert!(!redacted.contains("old_secret"), "CurrentUefiPassword value must be redacted");
845+
assert!(!redacted.contains("new_secret"), "UefiPassword value must be redacted");
846+
assert!(redacted.contains("CurrentUefiPassword"), "key name must be preserved");
847+
}
763848

764-
let big = "a".repeat(2000);
765-
assert_eq!(truncate(&big, 1500).len(), 1500);
849+
#[test]
850+
fn dell_import_buffer_xml_blob_is_redacted() {
851+
let xml = r#"<SystemConfiguration><Component FQDD="BIOS.Setup.1-1"><Attribute Name="OldSetupPassword">my_uefi_pass</Attribute><Attribute Name="NewSetupPassword"></Attribute></Component></SystemConfiguration>"#;
852+
let body = format!(
853+
r#"{{"ShutdownType":"Forced","ShareParameters":{{"Target":"BIOS"}},"ImportBuffer":"{}"}}"#,
854+
xml.replace('"', "\\\"")
855+
);
856+
let redacted = redact_sensitive_fields(&body);
857+
assert!(!redacted.contains("my_uefi_pass"), "UEFI password in ImportBuffer XML must not appear in log output");
858+
assert!(redacted.contains("[REDACTED]"));
859+
assert!(redacted.contains("ShutdownType"), "non-sensitive fields must be preserved");
860+
}
861+
862+
#[test]
863+
fn non_sensitive_body_is_returned_borrowed() {
864+
let body = r#"{"ResetType":"GracefulRestart"}"#;
865+
match redact_sensitive_fields(body) {
866+
Cow::Borrowed(s) => assert_eq!(s, body),
867+
Cow::Owned(_) => panic!("non-sensitive body must take the zero-copy fast path"),
868+
}
869+
}
870+
871+
#[test]
872+
fn empty_body_fast_path() {
873+
match redact_sensitive_fields("") {
874+
Cow::Borrowed(s) => assert_eq!(s, ""),
875+
Cow::Owned(_) => panic!("empty string must take fast path"),
876+
}
877+
}
878+
879+
#[test]
880+
fn wire_payload_is_unaffected() {
881+
let body_enc = r#"{"UserName":"newuser","Password":"myP@ssw0rd"}"#.to_string();
882+
let _log_safe = redact_sensitive_fields(&body_enc);
883+
assert_eq!(body_enc, r#"{"UserName":"newuser","Password":"myP@ssw0rd"}"#,
884+
"wire payload must never be modified");
885+
}
886+
887+
#[test]
888+
fn escaped_characters_in_password_are_redacted() {
889+
let body = r#"{"Password":"p@ss\"w\\ord"}"#;
890+
let redacted = redact_sensitive_fields(body);
891+
assert!(!redacted.contains("p@ss"), "escaped password value must be redacted");
892+
assert!(redacted.contains("[REDACTED]"));
893+
}
894+
895+
#[test]
896+
fn truncation_after_redaction_does_not_leak_partial_secret() {
897+
let filler = "x".repeat(1490);
898+
let secret = "supersecret_password_value";
899+
let body = format!(r#"{{"Data":"{}","Password":"{}"}}"#, filler, secret);
900+
assert!(body.len() > 1500, "body must exceed truncation limit for this test to be valid");
901+
902+
let redacted = redact_sensitive_fields(&body);
903+
let logged = truncate(&redacted, 1500);
904+
assert!(!logged.contains("supersecret"), "no part of the secret must appear after truncation");
905+
}
766906
}

0 commit comments

Comments
 (0)