From eda5c35bbae1e09b065de75d879b7de04bdd014a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Tue, 18 Aug 2026 17:01:26 -0300 Subject: [PATCH 1/2] fix(xcatd): redact secrets in the commands.log response xcatd redacts the request in commands.log but appends the command response verbatim. A command whose output holds a secret writes it in clear text. Examples are tabdump passwd, gettab of a passwd column, and getcredentials. Collect the response into a per-command buffer. Set a sensitive flag when the command is getcredentials, an argument names a password, or the request was redacted. When the command finishes, replace the whole buffer if the flag is set or the buffer still holds password content, then append the buffer. A connection can carry more than one command, so the buffer is finalized at the next command's start and at the end of the connection. The buffer holds the full response, so a secret split across several callbacks is also redacted. A per-callback check cannot do this. The word-content check is a fallback. The request classification is the main signal. A secret with no password marker, such as the output of an xdsh cat of a shadow file, is a pre-existing leak of the root-only log. It is out of scope. Recovered from the lenovobuild branch. Reimplemented against master. --- xCAT-server/sbin/xcatd | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/xCAT-server/sbin/xcatd b/xCAT-server/sbin/xcatd index d1e27025d..0596d49a3 100755 --- a/xCAT-server/sbin/xcatd +++ b/xCAT-server/sbin/xcatd @@ -224,6 +224,8 @@ if ($tmp) { } my $cmdlog_alllog = "====================================================\n"; my $cmdlog_starttime=undef; +my $cmdlog_response_buffer = ""; +my $cmdlog_response_sensitive = 0; # ----used for command log end--------- my $enable_perf = $sitetab->getAttribs({'key'=>'enableperf'},'value'); @@ -2848,6 +2850,7 @@ sub service_connection { } # ----used for command log start---------- + cmdlog_finalize_response(); $cmdlog_starttime = time(); my ($sec, $min, $hour, $mday, $mon, $year) = localtime($cmdlog_starttime); $year += 1900; @@ -2878,8 +2881,11 @@ sub service_connection { } } # Replace passwords with 'x' + my $cmdlog_before_redact = $cmdlog_alllog; $cmdlog_alllog = xCAT::xcatd->redact_password($cmdlog_alllog); + my $cmdlog_req_redacted = ($cmdlog_alllog ne $cmdlog_before_redact) ? 1 : 0; $cmdlog_alllog .= "\n[Response]\n"; + $cmdlog_response_sensitive = cmdlog_response_is_sensitive($req, $cmdlog_req_redacted); # ----used for command log end---------- @@ -3064,6 +3070,7 @@ sub service_connection { } # ----used for command log start------- + cmdlog_finalize_response(); $cmdlog_alllog .= "[NumberNodes] $numofnodes \n"; my $reqhandletime = sprintf("%.3f", time()-$cmdlog_starttime); $cmdlog_alllog .= "[ElapsedTime] $reqhandletime s\n"; @@ -3136,6 +3143,7 @@ sub relay_fds { # Relays file descriptors from pipes to children to the SSL sock xCAT::MsgUtils->message("S", "Client abort requested"); # ----used for command log start------- + cmdlog_finalize_response(); $cmdlog_alllog .= "Client abort requested\n"; my $reqhandletime = sprintf("%.3f", time()-$cmdlog_starttime); $cmdlog_alllog .= "[ElapsedTime] $reqhandletime s\n"; @@ -3606,10 +3614,27 @@ sub cmdlog_collectlog() { } } } - $cmdlog_alllog .= $rsp_log; + $cmdlog_response_buffer .= $rsp_log; return 0; } +sub cmdlog_response_is_sensitive { + my ($req, $req_redacted) = @_; + return 1 if defined $req->{command}->[0] and $req->{command}->[0] eq 'getcredentials'; + return 1 if join(' ', @{ $req->{arg} || [] }) =~ /passw/i; + return 1 if $req_redacted; + return 0; +} + +sub cmdlog_finalize_response { + if ($cmdlog_response_sensitive or $cmdlog_response_buffer =~ /passw/i) { + $cmdlog_response_buffer = "*REDACTED*\n"; + } + $cmdlog_alllog .= $cmdlog_response_buffer; + $cmdlog_response_buffer = ""; + $cmdlog_response_sensitive = 0; +} + # -------------------------------------------------------------------------------- =head3 cmdlog_submitlog From e9c8aaefbb5d03f752d3ee8c5d5dc0a13518ecb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Tue, 18 Aug 2026 17:01:26 -0300 Subject: [PATCH 2/2] test(xcatd): cover commands.log response redaction Extracts the three command-log response subs and drives them. It checks the request classification, the redaction of a bare passwd value, a secret split across callbacks in either order, the password-content fallback, that a benign response is kept, and that the finalizer preserves an earlier command's response on a shared connection. It fails hard if a sub cannot be extracted. --- xCAT-test/unit/cmdlog_response_redact.t | 95 +++++++++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 xCAT-test/unit/cmdlog_response_redact.t diff --git a/xCAT-test/unit/cmdlog_response_redact.t b/xCAT-test/unit/cmdlog_response_redact.t new file mode 100644 index 000000000..e79f4cd85 --- /dev/null +++ b/xCAT-test/unit/cmdlog_response_redact.t @@ -0,0 +1,95 @@ +#!/usr/bin/env perl +use strict; +use warnings; + +use FindBin; +use Test::More; + +my $xcatd = "$FindBin::Bin/../../xCAT-server/sbin/xcatd"; +plan skip_all => 'xcatd not found' unless -r $xcatd; + +open my $fh, '<', $xcatd or die $!; +my $src = do { local $/; <$fh> }; +close $fh; + +# Extract the three command-log response subs and load them. xcatd is present, +# so a sub that cannot be extracted is a hard failure, not a skip. +my %sub; +for my $name (qw(cmdlog_response_is_sensitive cmdlog_finalize_response cmdlog_collectlog)) { + my ($body) = $src =~ /(^sub \Q$name\E\b.*?^\})/ms; + BAIL_OUT("could not extract $name from xcatd") unless $body; + $body =~ s/^sub \Q$name\E\(\)/sub $name/m; + $sub{$name} = $body; +} + +our $cmdlog_alllog; +our $cmdlog_response_buffer; +our $cmdlog_response_sensitive; +our $MYXCATSERVER = ""; +{ + no strict; + no warnings; + eval "$sub{cmdlog_response_is_sensitive}\n$sub{cmdlog_finalize_response}\n$sub{cmdlog_collectlog}"; +} +die "eval of command-log subs failed: $@" if $@; + +# Classification from the request. +ok(cmdlog_response_is_sensitive({ command => ['getcredentials'], arg => [] }, ''), + 'getcredentials is a sensitive-response command'); +ok(cmdlog_response_is_sensitive({ command => ['gettab'], arg => ['key=xcat', 'passwd.password'] }, ''), + 'gettab of a passwd column is sensitive'); +ok(cmdlog_response_is_sensitive({ command => ['tabdump'], arg => ['passwd'] }, ''), + 'tabdump passwd is sensitive'); +ok(cmdlog_response_is_sensitive({ command => ['rspconfig'], arg => [] }, 1), + 'a redacted request is sensitive'); +ok(!cmdlog_response_is_sensitive({ command => ['rpower'], arg => ['n1', 'stat'] }, 0), + 'a benign request is not sensitive'); + +# Drive collect(s) then finalize, returning what was appended to the log. +sub run { + my ($sensitive, @responses) = @_; + $cmdlog_alllog = ""; + $cmdlog_response_buffer = ""; + $cmdlog_response_sensitive = $sensitive; + cmdlog_collectlog({ xcatresponse => [ { data => [$_] } ] }) for @responses; + cmdlog_finalize_response(); + return $cmdlog_alllog; +} + +# A bare passwd value (gettab passwd.password) has no "passw" in the response, +# so only the request classification catches it. +my $bare = run(1, "S3cr3tPW"); +unlike($bare, qr/S3cr3tPW/, 'a bare passwd value is not logged'); +like($bare, qr/\*REDACTED\*/, 'a sensitive response is redacted'); + +# A secret split across callbacks is redacted regardless of order. +unlike(run(0, "password", "S3cr3tPW"), qr/S3cr3tPW/, + 'multi-callback, passw first: the secret fragment is not logged'); +unlike(run(0, "S3cr3tPW", "password"), qr/S3cr3tPW/, + 'multi-callback, secret first: the earlier fragment is redacted too'); + +# Fallback: a response mentioning a password is redacted even without request signal. +like(run(0, "invalid password for node"), qr/\*REDACTED\*/, + 'a response mentioning a password is redacted by the fallback'); + +# A benign response is logged verbatim. +my $benign = run(0, "node01: on"); +like($benign, qr/node01: on/, 'a benign response is logged verbatim'); +unlike($benign, qr/\*REDACTED\*/, 'a benign response is not redacted'); + +# Two requests on one connection: the finalizer runs at the next command's +# start. The earlier response must be preserved (redacted), the flag reset, and +# the next benign response neither lost nor over-redacted. +$cmdlog_alllog = ""; +$cmdlog_response_buffer = ""; +$cmdlog_response_sensitive = 1; +cmdlog_collectlog({ xcatresponse => [ { data => ["SECRET_N"] } ] }); +cmdlog_finalize_response(); +is($cmdlog_response_sensitive, 0, 'finalize clears the sensitive flag'); +cmdlog_collectlog({ xcatresponse => [ { data => ["benign_np1"] } ] }); +cmdlog_finalize_response(); +like($cmdlog_alllog, qr/\*REDACTED\*/, 'the earlier sensitive response is preserved as redacted, not lost'); +like($cmdlog_alllog, qr/benign_np1/, 'the next benign response is logged, not over-redacted'); +unlike($cmdlog_alllog, qr/SECRET_N/, 'the earlier secret is not leaked'); + +done_testing();