2
0
mirror of https://github.com/xcat2/xcat-core.git synced 2026-09-04 20:17:55 +00:00

Merge pull request #7737 from VersatusHPC/fix/cmdlog-response-classifier

fix(xcatd): classify secret responses by the shared secret set
This commit is contained in:
Vinícius Ferrão
2026-08-28 17:34:56 -03:00
committed by GitHub
3 changed files with 90 additions and 16 deletions
+29
View File
@@ -535,6 +535,35 @@ my %secret_command_patterns = (
my %secret_command_nocase = map { $_ => 1 } qw(mkvm);
my %secret_site_keys = map { $_ => 1 } qw(snmpc);
my %secret_table = map { (split /\./)[0] => 1 } grep { /\./ } @secret_attributes;
$secret_table{site} = 1;
my $secret_response_pattern = do {
my $names = join '|', map { quotemeta } @secret_attributes;
qr/(?:^|[^\w.])(?:$names)\s*[=:]/;
};
sub secret_in_response {
my ($class, $text) = @_;
return 0 unless defined $text;
return 1 if $text =~ /passw/i;
return 1 if $text =~ $secret_response_pattern;
return 0;
}
sub secret_in_request {
my ($class, $command, $args) = @_;
foreach my $arg (@{ $args || [] }) {
next unless defined $arg;
return 1 if defined $command and ($command eq 'tabdump' or $command eq 'nodels') and $secret_table{$arg};
return 1 if $arg =~ /^([\w.]+)/ and $secret_attribute{$1};
foreach my $selector (split /,/, $arg) {
return 1 if $selector =~ /^(?:site\.)?key=([\w.]+)$/ and $secret_site_keys{$1};
}
}
return 0;
}
sub redact_password_arg {
my ($class, $arg) = @_;
return $arg unless defined $arg;
+13 -12
View File
@@ -2864,13 +2864,13 @@ sub service_connection {
my $strhour = ($hour > 9 ? $hour : "0" . $hour);
my $strmin = ($min > 9 ? $min : "0" . $min);
my $strsec = ($sec > 9 ? $sec : "0" . $sec);
$cmdlog_alllog .= "[Date] $year-$strmon-$strmday $strhour:$strmin:$strsec\n";
my $cmdlog_request = "[Date] $year-$strmon-$strmday $strhour:$strmin:$strsec\n";
$cmdlog_alllog .= "[ClientType] " . $req->{clienttype}->[0] . " \n";
$cmdlog_alllog .= "[Request] " . $req->{command}->[0] . " ";
$cmdlog_request .= "[ClientType] " . $req->{clienttype}->[0] . " \n";
$cmdlog_request .= "[Request] " . $req->{command}->[0] . " ";
if (exists($req->{noderange}) && defined($req->{noderange}->[0])) {
my $tmpstr = join(",", @{$req->{noderange}});
$cmdlog_alllog .= "$tmpstr ";
$cmdlog_request .= "$tmpstr ";
}
my $cmdlog_req_redacted = 0;
@@ -2881,17 +2881,17 @@ sub service_connection {
if ($arg =~ /[^A-Za-z0-9.-]/) {
my $tmparg = $arg;
$tmparg =~ s/'/'\\''/g;
$cmdlog_alllog .= "'" . $tmparg . "' ";
$cmdlog_request .= "'" . $tmparg . "' ";
} else {
$cmdlog_alllog .= $arg . " ";
$cmdlog_request .= $arg . " ";
}
}
}
# Replace passwords with 'x'
my $cmdlog_before_redact = $cmdlog_alllog;
$cmdlog_alllog = xCAT::xcatd->redact_password($cmdlog_alllog);
$cmdlog_req_redacted = 1 if $cmdlog_alllog ne $cmdlog_before_redact;
$cmdlog_alllog .= "\n[Response]\n";
my $cmdlog_before_redact = $cmdlog_request;
$cmdlog_request = xCAT::xcatd->redact_password($cmdlog_request);
$cmdlog_req_redacted = 1 if $cmdlog_request ne $cmdlog_before_redact;
$cmdlog_alllog .= $cmdlog_request . "\n[Response]\n";
$cmdlog_response_sensitive = cmdlog_response_is_sensitive($req, $cmdlog_req_redacted);
# ----used for command log end----------
@@ -3634,14 +3634,15 @@ sub cmdlog_collectlog() {
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 defined $req->{command}->[0] and ($req->{command}->[0] eq 'getcredentials' or $req->{command}->[0] eq 'lsvm');
return 1 if join(' ', @{ $req->{arg} || [] }) =~ /passw/i;
return 1 if xCAT::xcatd->secret_in_request($req->{command}->[0], $req->{arg});
return 1 if $req_redacted;
return 0;
}
sub cmdlog_finalize_response {
if ($cmdlog_response_sensitive or $cmdlog_response_buffer =~ /passw/i) {
if ($cmdlog_response_sensitive or xCAT::xcatd->secret_in_response($cmdlog_response_buffer)) {
$cmdlog_response_buffer = "*REDACTED*\n";
}
$cmdlog_alllog .= $cmdlog_response_buffer;
+48 -4
View File
@@ -3,14 +3,18 @@ use strict;
use warnings;
use FindBin;
use lib "$FindBin::Bin/../lib";
use lib "$FindBin::Bin/../../perl-xCAT";
use lib "$FindBin::Bin/../../xCAT-server/lib/perl";
use Test::More;
my $xcatd = "$FindBin::Bin/../../xCAT-server/sbin/xcatd";
use XCAT::Test::File qw(repo_path slurp_repo_file);
use xCAT::xcatd;
my $xcatd = repo_path('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;
my $src = slurp_repo_file('xCAT-server/sbin/xcatd');
# 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.
@@ -36,6 +40,8 @@ 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 => ['lsvm'], arg => [] }, 0),
'lsvm returns the directory entry with its passwords, so it is sensitive');
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'] }, ''),
@@ -45,6 +51,35 @@ ok(cmdlog_response_is_sensitive({ command => ['rspconfig'], arg => [] }, 1),
ok(!cmdlog_response_is_sensitive({ command => ['rpower'], arg => ['n1', 'stat'] }, 0),
'a benign request is not sensitive');
# A secret attribute with no "passw" in its name must classify through the
# shared secret set, not the text heuristic.
ok(cmdlog_response_is_sensitive({ command => ['gettab'], arg => ['node=pdu01', 'pdu.authkey'] }, 0),
'gettab of an authentication key is sensitive');
ok(cmdlog_response_is_sensitive({ command => ['gettab'], arg => ['node=pdu01', 'pdu.privkey'] }, 0),
'gettab of a privacy key is sensitive');
ok(cmdlog_response_is_sensitive({ command => ['gettab'], arg => ['key=snmpc', 'site.value'] }, 0),
'gettab of the snmpc site value is sensitive');
ok(!cmdlog_response_is_sensitive({ command => ['gettab'], arg => ['key=domain', 'site.value'] }, 0),
'gettab of a plain site value is not sensitive');
# A dump of a whole table that owns a secret column returns the bare values.
ok(cmdlog_response_is_sensitive({ command => ['tabdump'], arg => ['token'] }, 0),
'tabdump of the token table is sensitive');
ok(cmdlog_response_is_sensitive({ command => ['tabdump'], arg => ['prodkey'] }, 0),
'tabdump of the prodkey table is sensitive');
ok(cmdlog_response_is_sensitive({ command => ['tabdump'], arg => ['site'] }, 0),
'tabdump of the site table is sensitive');
ok(cmdlog_response_is_sensitive({ command => ['tabdump'], arg => ['-w', 'key==snmpc', 'site'] }, 0),
'a filtered site dump is sensitive');
ok(!cmdlog_response_is_sensitive({ command => ['tabdump'], arg => ['networks'] }, 0),
'tabdump of the networks table is not sensitive');
ok(cmdlog_response_is_sensitive({ command => ['nodels'], noderange => ['node01'], arg => ['prodkey'] }, 0),
'nodels of a whole secret table is sensitive');
ok(!cmdlog_response_is_sensitive({ command => ['nodels'], noderange => ['switches'], arg => [] }, 0),
'a group named like a secret table is not sensitive');
ok(!cmdlog_response_is_sensitive({ command => ['nodels'], noderange => ['node01'], arg => ['nodetype'] }, 0),
'nodels of a benign table is not sensitive');
# Drive collect(s) then finalize, returning what was appended to the log.
sub run {
my ($sensitive, @responses) = @_;
@@ -77,6 +112,15 @@ 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');
# A detailed object listing expands attributes the request never named.
my $lsdef = run(0, 'Object name: pdu01', ' authkey=AUTH_SECRET', ' privkey=PRIV_SECRET');
unlike($lsdef, qr/AUTH_SECRET|PRIV_SECRET/, 'implicit lsdef secrets are not logged');
like($lsdef, qr/\*REDACTED\*/, 'lsdef response containing secret attributes is redacted');
my $colon = run(0, 'node01: prodkey.key: AAAAA-BBBBB');
unlike($colon, qr/AAAAA/, 'a colon separated secret column is redacted');
my $plain = run(0, 'Object name: node01', ' groups=compute', ' mgt=ipmi');
unlike($plain, qr/\*REDACTED\*/, 'a benign object listing 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.