diff --git a/xCAT-server/lib/perl/xCAT/xcatd.pm b/xCAT-server/lib/perl/xCAT/xcatd.pm index 20f76bc23..4de9fbcf2 100644 --- a/xCAT-server/lib/perl/xCAT/xcatd.pm +++ b/xCAT-server/lib/perl/xCAT/xcatd.pm @@ -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; diff --git a/xCAT-server/sbin/xcatd b/xCAT-server/sbin/xcatd index 2d060babf..e1a5ff6c9 100755 --- a/xCAT-server/sbin/xcatd +++ b/xCAT-server/sbin/xcatd @@ -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; diff --git a/xCAT-test/unit/cmdlog_response_redact.t b/xCAT-test/unit/cmdlog_response_redact.t index e79f4cd85..4bcc7054e 100644 --- a/xCAT-test/unit/cmdlog_response_redact.t +++ b/xCAT-test/unit/cmdlog_response_redact.t @@ -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.