diff --git a/xCAT-server/lib/perl/xCAT/xcatd.pm b/xCAT-server/lib/perl/xCAT/xcatd.pm index 3bafeeb83..331b6956a 100644 --- a/xCAT-server/lib/perl/xCAT/xcatd.pm +++ b/xCAT-server/lib/perl/xCAT/xcatd.pm @@ -244,7 +244,8 @@ sub validate { # add each argument my $args = $request->{arg}; my $arglist; - foreach my $argument (@$args) { + my ($redacted_args) = xCAT::xcatd->redact_password_args($request->{command}->[0], $args); + foreach my $argument (@$redacted_args) { $arglist .= " " . $argument; } my $saveArglist = $arglist; @@ -484,6 +485,174 @@ sub verifytoken { return undef; } } +# -------------------------------------------------------------------------------- +my @secret_attributes = qw( + authkey bmcpassword domainadminpassword iscsipassword + passwd.HMC passwd.admin passwd.celogin passwd.general passwd.hscroot + password privkey snmppassword snmpc community productkey tokenid + domain.adminpassword ipmi.password iscsi.passwd mpa.password + openbmc.password passwd.password pdu.authkey pdu.community pdu.password + pdu.privkey ppcdirect.password ppchcp.password prodkey.key + switches.password switches.sshpassword token.tokenid vm.vidpassword + websrv.password +); +my %secret_attribute = map { $_ => 1 } @secret_attributes; + +my %secret_command_options = ( + bmcdiscover => [qw(p bmcpasswd n newbmcpw)], + switchdiscover => [qw(c)], + mkhwconn => [qw(P)], + mkvm => [qw(w password)], + createvcluster => [qw(password)], + lsvcluster => [qw(password)], + rmvcluster => [qw(password)], +); +my %secret_command_valueopts = ( + bmcdiscover => 'smiu', + switchdiscover => 's', + mkhwconn => 'psT', + mkvm => 'dijlmpqrvz', +); +my %secret_command_intopts = ( + mkvm => 'c', +); +my %secret_command_operands = ( + chvm => { + '--setpassword' => [1], + '--add3390' => [ 5, 6, 7 ], + '--add9336' => [ 5, 6, 7 ], + '--addpagespool' => [8], + '--formatdisk' => [2], + }, +); +my %secret_command_patterns = ( + mkvm => qr/^(pw=)/, + rspconfig => qr/^(\*_passwd=|admin_passwd=|HMC_passwd=|general_passwd=|USERID=)/, +); +my %secret_command_nocase = map { $_ => 1 } qw(mkvm); +my %secret_site_keys = map { $_ => 1 } qw(snmpc); + +sub redact_password_arg { + my ($class, $arg) = @_; + return $arg unless defined $arg; + if ($arg =~ /^\s*([\w.]+)\s*([-+,^!]?=~?|!~)/ and $secret_attribute{$1}) { + return $1 . $2 . "xxxxxxxx"; + } + while ($arg =~ /(? 1 } @option; + my $valueopts = (defined $command and $secret_command_valueopts{$command}) + ? $secret_command_valueopts{$command} : ''; + my $intopts = (defined $command and $secret_command_intopts{$command}) + ? $secret_command_intopts{$command} : ''; + if ($nocase) { $letters = lc $letters; } + my $pattern = defined $command ? $secret_command_patterns{$command} : undef; + my %operand; + if (defined $command and $secret_command_operands{$command} + and defined $args->[0] + and $secret_command_operands{$command}{ $args->[0] }) { + %operand = map { $_ => 1 } @{ $secret_command_operands{$command}{ $args->[0] } }; + } + my $sitevalue = 0; + if (defined $command and ($command eq 'tabch' or $command eq 'chtab')) { + SELECTOR: foreach my $selectorarg (@$args) { + next unless defined $selectorarg; + foreach my $selector (split /,/, $selectorarg) { + if ($selector =~ /^(?:site\.)?key=([\w.]+)$/ and $secret_site_keys{$1}) { + $sitevalue = 1; + last SELECTOR; + } + } + } + } + for my $index (0 .. $#{$args}) { + my $arg = $args->[$index]; + if (not defined $arg) { push @redacted, $arg; next; } + if ($pending) { + $pending = 0; + $changed = 1; + push @redacted, "xxxxxxxx"; + next; + } + if ($operand{$index}) { + $changed = 1; + push @redacted, "xxxxxxxx"; + next; + } + my $masked; + foreach my $name (@longnames) { + if ($arg =~ /^(-{1,2}|\+)([A-Za-z]+)(=?)(.*)$/s and index($name, lc($2)) == 0) { + my $short = $nocase ? lc($2) : $2; + next if length($2) == 1 and index($valueopts, $short) >= 0; + if ($3) { $masked = $1 . $2 . $3 . "xxxxxxxx"; last; } + if ($4 eq '') { $pending = 1; last; } + } + } + if (not $pending and not defined $masked and $letters) { + if ($arg =~ /^--([A-Za-z])(.*)$/s) { + my ($c, $rest) = ($1, $2); + my $cc = $nocase ? lc $c : $c; + if (index($letters, $cc) >= 0) { + if ($rest eq '') { $pending = 1; } + else { $masked = "--" . $c . "xxxxxxxx"; } + } + } elsif ($arg =~ /^([-+])([A-Za-z?].*)$/s) { + my ($intro, $body) = ($1, $2); + my $offset = 0; + my $length = length $body; + while ($offset < $length) { + my $c = substr($body, $offset, 1); + last if $c !~ /[A-Za-z?]/; + my $cc = $nocase ? lc $c : $c; + if (index($letters, $cc) >= 0) { + if ($offset == $length - 1) { $pending = 1; } + else { $masked = $intro . substr($body, 0, $offset + 1) . "xxxxxxxx"; } + last; + } + last if index($valueopts, $c) >= 0; + if ($intopts ne '' and index($intopts, $c) >= 0) { + $offset++; + $offset++ if $offset < $length and substr($body, $offset, 1) =~ /[-+]/; + $offset++ while $offset < $length and substr($body, $offset, 1) =~ /[0-9_]/; + next; + } + $offset++; + } + } + } + if (not defined $masked and $pattern and $arg =~ /$pattern/s) { + $masked = $1 . "xxxxxxxx"; + } + if (not defined $masked and $sitevalue and $arg =~ /^(site\.value\s*(?:[-+,^!]?=~?|!~)\s*)/s) { + $masked = $1 . "xxxxxxxx"; + } + if (defined $masked) { + $changed = 1; + push @redacted, $masked; + next; + } + my $attrarg = $class->redact_password_arg($arg); + $changed = 1 if $attrarg ne $arg; + push @redacted, $attrarg; + } + return (\@redacted, $changed); +} + # -------------------------------------------------------------------------------- =head3 redact_password @@ -516,7 +685,6 @@ sub verifytoken { 'HMC_passwd=xxx' '*_passwd=xxxxxxx' =cut -# -------------------------------------------------------------------------------- sub redact_password { my $class = shift; my $request = shift; @@ -568,50 +736,10 @@ sub redact_password { } } } - # Object definition attributes carry their value as attr=value, on whichever - # command happens to set them, so they are matched by name rather than by - # command. These are the attributes that map to a secret column in - # Schema.pm; attributes such as 'key' and 'sshkeydir' are not secrets and - # are deliberately absent. - my @password_attributes = qw( - authkey - bmcpassword - domainadminpassword - iscsipassword - passwd.HMC - passwd.admin - passwd.celogin - passwd.general - passwd.hscroot - password - privkey - snmppassword - - domain.adminpassword - ipmi.password - iscsi.passwd - mpa.password - openbmc.password - passwd.password - pdu.authkey - pdu.password - pdu.privkey - ppcdirect.password - ppchcp.password - switches.password - switches.sshpassword - vm.vidpassword - websrv.password - ); - foreach my $attribute (@password_attributes) { - # An assignment may be written with spaces around the equals sign and - # the value may itself contain spaces, in which case the argument - # arrives quoted. Redact to the closing quote there, and to the end of - # the argument otherwise, so the rest of the command stays readable. - $parameters =~ s/(^|\s)'(\Q$attribute\E\s*=\s*)[^']*'/$1'$2$redact_string'/g; - $parameters =~ s/(^|\s)(\Q$attribute\E\s*=\s*)[^'\s]*/$1$2$redact_string/g; + foreach my $attribute (@secret_attributes) { + $parameters =~ s/(^|\s)'(\Q$attribute\E\s*(?:[-+,^!]?=~?|!~)\s*)[^']*'/$1'$2$redact_string'/g; + $parameters =~ s/(^|[^\w.])(\Q$attribute\E\s*(?:[-+,^!]?=~?|!~)\s*)(?!\Q$redact_string\E).*$/$1$2$redact_string/; } - # Return original request with password replaced by 'x' in $parameters string if ($request =~ '\[Request\]') { return $header . "[Request] " . $command . " " . $parameters; diff --git a/xCAT-server/sbin/xcatd b/xCAT-server/sbin/xcatd index 0596d49a3..42c690a9b 100755 --- a/xCAT-server/sbin/xcatd +++ b/xCAT-server/sbin/xcatd @@ -2283,7 +2283,8 @@ sub dispatch_request { } if (exists($req->{arg})) { - foreach my $arg (@{ $req->{arg} }) { + my ($trace_args) = xCAT::xcatd->redact_password_args($req->{command}->[0], $req->{arg}); + foreach my $arg (@$trace_args) { $str_cmd .= $arg . " "; } $str_cmd =~ s/(.+) $/$1/g; @@ -2869,8 +2870,11 @@ sub service_connection { $cmdlog_alllog .= "$tmpstr "; } + my $cmdlog_req_redacted = 0; if (exists($req->{arg})) { - foreach my $arg (@{ $req->{arg} }) { + my $redacted_args; + ($redacted_args, $cmdlog_req_redacted) = xCAT::xcatd->redact_password_args($req->{command}->[0], $req->{arg}); + foreach my $arg (@$redacted_args) { if ($arg =~ /[^A-Za-z0-9.-]/) { my $tmparg = $arg; $tmparg =~ s/'/'\\''/g; @@ -2883,7 +2887,7 @@ 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_req_redacted = 1 if $cmdlog_alllog ne $cmdlog_before_redact; $cmdlog_alllog .= "\n[Response]\n"; $cmdlog_response_sensitive = cmdlog_response_is_sensitive($req, $cmdlog_req_redacted); diff --git a/xCAT-test/unit/xcatd_password_redaction.t b/xCAT-test/unit/xcatd_password_redaction.t index bf45db751..33ba219fe 100644 --- a/xCAT-test/unit/xcatd_password_redaction.t +++ b/xCAT-test/unit/xcatd_password_redaction.t @@ -8,9 +8,10 @@ use Test::More; my $repo_root = File::Spec->catdir( $FindBin::Bin, '..', '..' ); my $xcatd = File::Spec->catfile( $repo_root, 'xCAT-server/lib/perl/xCAT/xcatd.pm' ); +my $daemon = File::Spec->catfile( $repo_root, 'xCAT-server/sbin/xcatd' ); my $schema = File::Spec->catfile( $repo_root, 'perl-xCAT/xCAT/Schema.pm' ); -plan skip_all => 'xcatd.pm or Schema.pm not found' unless -r $xcatd && -r $schema; +plan skip_all => 'xcatd.pm, xcatd or Schema.pm not found' unless -r $xcatd && -r $daemon && -r $schema; sub slurp { open( my $fh, '<', $_[0] ) or die "Unable to read $_[0]: $!"; @@ -20,68 +21,293 @@ sub slurp { } my $source = slurp($xcatd); +my $daemon_source = slurp($daemon); my $schema_source = slurp($schema); -# Run the real routine rather than inspecting its source. xcatd.pm cannot be -# loaded here because of its dependencies, so the routine is lifted out and -# evaluated on its own; it uses nothing outside itself. -my ($routine) = $source =~ /(sub redact_password \{.*?\n\}\n)/s; -ok( $routine, 'redact_password was located in xcatd.pm' ) - or BAIL_OUT('xcatd.pm no longer defines redact_password'); -eval "package RedactUnderTest; $routine 1;" or BAIL_OUT("could not evaluate redact_password: $@"); +# xcatd.pm cannot be loaded here (dependencies), so lift out the secret set, the +# command maps, and the redaction routines and evaluate them alone. +my ($set) = $source =~ /(my \@secret_attributes = qw\(.*?\);\s*my %secret_attribute = map.*?;)/s; +my ($maps) = $source =~ /(my %secret_command_options = \(.*?my %secret_site_keys = map.*?;)/s; +my ($arg_sub) = $source =~ /(sub redact_password_arg \{.*?\n\}\n)/s; +my ($args_sub) = $source =~ /(sub redact_password_args \{.*?\n\}\n)/s; +my ($cmd_sub) = $source =~ /(sub redact_password \{.*?\n\}\n)/s; +BAIL_OUT('could not extract the secret set from xcatd.pm') unless $set; +BAIL_OUT('could not extract the command maps from xcatd.pm') unless $maps; +BAIL_OUT('could not extract redact_password_arg from xcatd.pm') unless $arg_sub; +BAIL_OUT('could not extract redact_password_args from xcatd.pm') unless $args_sub; +BAIL_OUT('could not extract redact_password from xcatd.pm') unless $cmd_sub; +eval "package RedactUnderTest; $set $maps $arg_sub $args_sub $cmd_sub 1;" + or BAIL_OUT("could not evaluate the redaction routines: $@"); -sub redacted { return RedactUnderTest::redact_password( $_[0], $_[1] ); } +sub arg { return RedactUnderTest::redact_password_arg( 'xCAT::xcatd', $_[0] ); } +sub cmd { return RedactUnderTest::redact_password( $_[0], $_[1] ); } -# A secret must never survive, whichever way it was written. -my %leaks = ( - 'attribute assignment' => [ 'chdef', " node01 bmcpassword=SEKRET" ], - 'quoted assignment' => [ 'chdef', " node01 'bmcpassword=SEKRET'" ], - 'spaces around the equals' => [ 'chdef', " node01 'bmcpassword = SEKRET phrase'" ], - 'table qualified column' => [ 'chtab', " key=system passwd.username=root passwd.password=SEKRET" ], - 'several on one command' => [ 'nodeadd', " n1 ipmi.password=SEKRET openbmc.password=SEKRET" ], - 'snmp passphrases' => [ 'chdef', " pdu1 authkey=SEKRET privkey=SEKRET" ], - 'positional flag' => [ 'bmcdiscover', " -s nmap -p SEKRET --range 10.0.0.1" ], -); -foreach my $case ( sort keys %leaks ) { - my ( $command, $args ) = @{ $leaks{$case} }; - unlike( redacted( $command, $args ), qr/SEKRET/, "no secret survives: $case" ); +sub vec_ { + my ( $command, @args ) = @_; + my ( $redacted, $changed ) = + RedactUnderTest->redact_password_args( $command, \@args ); + return ( join( ' ', @$redacted ), $changed ); } -# Detail that is not secret has to stay, or the log stops being useful. -my $kept = redacted( 'chdef', " node01 sshkeydir=/etc/xcat/keys key=system groups=lab mgt=ipmi" ); -like( $kept, qr/sshkeydir=\/etc\/xcat\/keys/, 'sshkeydir is kept, it is a directory' ); -like( $kept, qr/key=system/, 'key is kept, it names a monitoring attribute' ); -like( $kept, qr/groups=lab/, 'unrelated attributes are kept' ); -like( - redacted( 'chtab', " key=system passwd.username=root passwd.password=SEKRET" ), - qr/passwd\.username=root/, - 'the username beside a redacted password is kept' -); +# A secret in an attribute assignment must never survive, however it was written. +unlike( arg('bmcpassword=SEKRET'), qr/SEKRET/, 'a bare attribute value is redacted' ); +unlike( arg('bmcpassword=SEKRET phrase'), qr/SEKRET|phrase/, 'a value with spaces is redacted whole' ); +unlike( arg("bmcpassword=has'quote"), qr/quote/, 'a value with a quote is redacted whole' ); +unlike( arg('passwd.password+=SEKRET'), qr/SEKRET/, 'a += splice assignment is redacted' ); +unlike( arg('pdu.community=SEKRET'), qr/SEKRET/, 'the SNMP community string is redacted' ); +unlike( arg('community=SEKRET'), qr/SEKRET/, 'a bare community value is redacted' ); -# Every attribute Schema.pm maps to a secret column has to be covered, so that -# one added later fails here instead of quietly reaching the logs. -my %expected; +# The chdef/mkdef parser trims whitespace around '=', so an attribute may be +# written with spaces. The value must still be redacted. +unlike( arg('bmcpassword = SEKRET'), qr/SEKRET/, 'spaces around the equals are redacted' ); +unlike( arg('bmcpassword =SEKRET'), qr/SEKRET/, 'a space before the equals is redacted' ); +unlike( arg('bmcpassword= SEKRET'), qr/SEKRET/, 'a space after the equals is redacted' ); +unlike( arg('bmcpassword = SEKRET phrase'), qr/SEKRET|phrase/, 'spaces around the equals with a multi-word value are redacted whole' ); + +# nodech and node selection accept operators other than a bare '='. +unlike( arg('ipmi.password,=SEKRET'), qr/SEKRET/, 'a ,= append assignment is redacted' ); +unlike( arg('ipmi.password^=SEKRET'), qr/SEKRET/, 'a ^= remove assignment is redacted' ); +unlike( arg('ipmi.password!=SEKRET'), qr/SEKRET/, 'a != selection is redacted' ); +unlike( arg('ipmi.password!~SEKRET'), qr/SEKRET/, 'a !~ selection is redacted' ); +unlike( arg('ipmi.password=~SEKRET'), qr/SEKRET/, 'a =~ selection is redacted' ); + +# Non-secret detail stays, or the log stops being useful. +is( arg('groups=lab'), 'groups=lab', 'an unrelated attribute is kept' ); +is( arg('key=system'), 'key=system', 'key is kept, it is not a secret' ); +is( arg('sshkeydir=/etc/xcat/keys'), 'sshkeydir=/etc/xcat/keys', 'sshkeydir is kept, it is a directory' ); +is( arg('n1'), 'n1', 'a plain argument is kept' ); + +# The command-flag mechanism still redacts positional password flags. +unlike( cmd( 'bmcdiscover', ' -s nmap -p SEKRET --range 10.0.0.1' ), qr/SEKRET/, + 'a -p flag value is redacted' ); + +# An assignment embedded in a compound argument, for example an xdsh remote +# command string, must be masked to the end of the argument, because a shell +# value may hold quotes and spaces. +unlike( cmd( 'xdsh', " compute 'echo bmcpassword=SEKRET > /etc/x'" ), qr/SEKRET/, + 'a secret embedded in a compound argument is redacted' ); +unlike( cmd( 'xdsh', q{ compute "export password='SEKRET phrase'; run-app"} ), qr/SEKRET|phrase/, + 'a quoted secret embedded in a compound argument is redacted whole' ); +unlike( arg(q{export password='SEKRET phrase'; run-app}), qr/SEKRET|phrase/, + 'a quoted secret inside one argument is redacted to the end' ); +unlike( arg('usercomment=password=SEKRET'), qr/SEKRET/, + 'a secret glued to a prior assignment is redacted' ); +is( arg('echo groups=lab; ls'), 'echo groups=lab; ls', 'an embedded non-secret assignment is kept' ); + +# A password given through a command option must be redacted in every form +# Getopt::Long accepts: a separate argument, a compact short option, a long +# option, and a long option with an equals sign. +my ( $out, $changed ); +( $out, $changed ) = vec_( 'bmcdiscover', '--range', '10.0.0.1', '-p', 'SEKRET phrase' ); +unlike( $out, qr/SEKRET|phrase/, 'a -p value in the next argument is redacted whole' ); +is( $changed, 1, 'the option redaction reports the change' ); +( $out ) = vec_( 'bmcdiscover', '--range', '10.0.0.1', '-pSEKRET' ); +unlike( $out, qr/SEKRET/, 'a compact -pSEKRET is redacted' ); +( $out ) = vec_( 'bmcdiscover', '--range', '10.0.0.1', '--bmcpasswd', 'SEKRET' ); +unlike( $out, qr/SEKRET/, 'a --bmcpasswd value in the next argument is redacted' ); +( $out ) = vec_( 'bmcdiscover', '--range', '10.0.0.1', '--bmcpasswd=SEKRET' ); +unlike( $out, qr/SEKRET/, 'a --bmcpasswd=value is redacted' ); +( $out ) = vec_( 'bmcdiscover', '-n', 'SEKRET' ); +unlike( $out, qr/SEKRET/, 'a -n new password is redacted' ); +( $out ) = vec_( 'bmcdiscover', '--newbmcpw', 'SEKRET' ); +is( $out, '--newbmcpw xxxxxxxx', 'the full --newbmcpw name is kept and its value is masked' ); +( $out ) = vec_( 'mkvm', 'zvm02', '-password', 'SEKRET' ); +is( $out, 'zvm02 -password xxxxxxxx', 'a single-dash long -password keeps its name and masks its value' ); +( $out ) = vec_( 'switchdiscover', '--range', '10.0.0.0/24', '-c', 'SEKRET' ); +unlike( $out, qr/SEKRET/, 'the switchdiscover -c community is redacted' ); +( $out ) = vec_( 'mkhwconn', 'frame', '-p', 'hmc01', '-PSEKRET' ); +unlike( $out, qr/SEKRET/, 'the mkhwconn -P password is redacted' ); +like( $out, qr/-p hmc01/, 'the mkhwconn -p hardware control point is kept' ); +( $out ) = vec_( 'mkvm', 'zvm02', '--password', 'SEKRET' ); +unlike( $out, qr/SEKRET/, 'the mkvm --password value is redacted' ); +( $out ) = vec_( 'rspconfig', 'admin_passwd=SEKRET phrase' ); +unlike( $out, qr/SEKRET|phrase/, 'a rspconfig password assignment is redacted whole' ); + +# Getopt::Long also accepts an abbreviated long option, a bundle of short +# options, and, for parsers that keep the default configuration, any letter +# case. Each of those forms must redact too. +( $out ) = vec_( 'bmcdiscover', '--range', '10.0.0.1', '--bmcp', 'SEKRET' ); +unlike( $out, qr/SEKRET/, 'an abbreviated --bmcp is redacted' ); +( $out ) = vec_( 'bmcdiscover', '--range', '10.0.0.1', '--bmcpas=SEKRET' ); +unlike( $out, qr/SEKRET/, 'an abbreviated --bmcpas=value is redacted' ); +( $out ) = vec_( 'bmcdiscover', '--range', '10.0.0.1', '-zp', 'SEKRET' ); +unlike( $out, qr/SEKRET/, 'a bundled -zp is redacted' ); +( $out ) = vec_( 'bmcdiscover', '--range', '10.0.0.1', '-zpSEKRET' ); +unlike( $out, qr/SEKRET/, 'a bundled compact -zpSEKRET is redacted' ); +( $out ) = vec_( 'bmcdiscover', '-pApple' ); +is( $out, '-pxxxxxxxx', 'a compact value keeps no leading letters' ); +( $out ) = vec_( 'bmcdiscover', '-pAdmin', '--range', '10.0.0.1' ); +is( $out, '-pxxxxxxxx --range 10.0.0.1', 'a compact value ending in a secret letter is masked, not the next argument' ); +( $out ) = vec_( 'bmcdiscover', '-nstop', '--range', '10.0.0.1' ); +is( $out, '-nxxxxxxxx --range 10.0.0.1', 'a compact -n value is masked whole' ); +( $out ) = vec_( 'bmcdiscover', '-pbanana' ); +is( $out, '-pxxxxxxxx', 'a compact value with a later secret letter is masked from the option' ); +( $out ) = vec_( 'bmcdiscover', '--p=SEKRET' ); +unlike( $out, qr/SEKRET/, 'a double-dash --p=value is redacted' ); +( $out ) = vec_( 'switchdiscover', '--c', 'SEKRET' ); +unlike( $out, qr/SEKRET/, 'a double-dash --c community is redacted' ); +( $out ) = vec_( 'mkhwconn', 'frame', '--P=SEKRET' ); +unlike( $out, qr/SEKRET/, 'a double-dash --P=value is redacted' ); +( $out, $changed ) = vec_( 'mkhwconn', 'frame', '--p', 'hmc01' ); +is( $changed, 0, 'the double-dash --p control point is kept, the case differs' ); +( $out ) = vec_( 'mkvm', 'zvm02', '--W', 'SEKRET' ); +unlike( $out, qr/SEKRET/, 'a double-dash --W is redacted, the parser ignores case' ); + +# The vCenter cluster commands log in with a --password option. +( $out ) = vec_( 'createvcluster', '--vcenter', 'vc01', '--username', 'admin', '--password', 'SEKRET', 'cluster01' ); +unlike( $out, qr/SEKRET/, 'the createvcluster password is redacted' ); +like( $out, qr/--username admin/, 'the createvcluster username is kept' ); +( $out ) = vec_( 'lsvcluster', '--vcenter', 'vc01', '--password=SEKRET' ); +unlike( $out, qr/SEKRET/, 'the lsvcluster password is redacted' ); +( $out ) = vec_( 'rmvcluster', '--vcenter', 'vc01', '--PASSWORD', 'SEKRET', 'cluster01' ); +unlike( $out, qr/SEKRET/, 'the rmvcluster password is redacted, the parser ignores case' ); + +# In a bundle a non-secret option that takes a value absorbs the rest, so a +# secret letter inside that value must not redact. +( $out, $changed ) = vec_( 'bmcdiscover', '-snmap', '--range', '10.0.0.0/24' ); +is( $out, '-snmap --range 10.0.0.0/24', 'the -snmap scan method is kept, n is the value of -s' ); +is( $changed, 0, 'a non-secret compact value reports no change' ); +( $out, $changed ) = vec_( 'bmcdiscover', '-sopenbmc' ); +is( $out, '-sopenbmc', 'the -sopenbmc scan method is kept, p is inside the value' ); +is( $changed, 0, 'the -sopenbmc value reports no change' ); +( $out, $changed ) = vec_( 'mkhwconn', 'frame', '-pPOWERHMC' ); +is( $changed, 0, 'the mkhwconn -p value is kept, P is inside the value of -p' ); +( $out, $changed ) = vec_( 'bmcdiscover', '-zsp', 'SEKRET' ); +is( $changed, 0, 'a bundle stops at a value option, -zsp is the value p of -s' ); +( $out ) = vec_( 'bmcdiscover', '-zps', 'topsecret' ); +is( $out, '-zpxxxxxxxx topsecret', 'a bundle masks from the secret letter, the next argument is positional' ); +( $out ) = vec_( 'bmcdiscover', '-?pSEKRET' ); +unlike( $out, qr/SEKRET/, 'a bundle with the ? help letter is redacted' ); +( $out ) = vec_( 'bmcdiscover', '-?n', 'SEKRET' ); +unlike( $out, qr/SEKRET/, 'a -?n bundle masks the next argument' ); +( $out, $changed ) = vec_( 'mkvm', 'lpar01', '-cpower10', '-pprofile1' ); +is( $out, 'lpar01 -cpower10 -pprofile1', 'the mkvm compact values are kept, w is inside the value of -c' ); +is( $changed, 0, 'the mkvm compact values report no change' ); +( $out, $changed ) = vec_( 'mkvm', 'lpar01', '-p', 'profile1' ); +is( $out, 'lpar01 -p profile1', 'the PPC mkvm profile remains visible' ); +is( $changed, 0, 'the PPC mkvm profile reports no redaction' ); + +# In the z/VM grammar -s is a boolean and -c takes an integer, so a bundle can +# continue into the real password option. +( $out ) = vec_( 'mkvm', 'zvm02', '-swSEKRET' ); +unlike( $out, qr/SEKRET/, 'zVM -s followed by bundled -w is redacted' ); +( $out ) = vec_( 'mkvm', 'zvm02', '-sw', 'SEKRET' ); +unlike( $out, qr/SEKRET/, 'zVM bundled -sw masks the next argument' ); +( $out ) = vec_( 'mkvm', 'zvm02', '-c1wSEKRET' ); +unlike( $out, qr/SEKRET/, 'zVM resumes after the numeric -c value and redacts -w' ); +( $out ) = vec_( 'mkvm', 'zvm02', '-c1w', 'SEKRET' ); +unlike( $out, qr/SEKRET/, 'zVM numeric bundle masks the following password argument' ); +( $out, $changed ) = vec_( 'mkvm', 'zvm02', '-c12' ); +is( $changed, 0, 'a plain numeric cpu bundle is kept' ); + +# Bundled short options keep letter case, so a capital letter is unknown to +# the parser and the bundle continues into the password option. +( $out ) = vec_( 'mkvm', 'zvm02', '-Rw', 'SEKRET' ); +unlike( $out, qr/SEKRET/, 'a capital -Rw bundle masks the next argument' ); +( $out ) = vec_( 'mkvm', 'zvm02', '-RwSEKRET' ); +unlike( $out, qr/SEKRET/, 'a glued capital -RwSEKRET is redacted' ); + +# A bundled integer value may carry a sign or underscores. +( $out ) = vec_( 'mkvm', 'zvm02', '-c+1w', 'SEKRET' ); +unlike( $out, qr/SEKRET/, 'a signed cpu value still reaches the password option' ); +( $out ) = vec_( 'mkvm', 'zvm02', '-c-1wSEKRET' ); +unlike( $out, qr/SEKRET/, 'a negative cpu value still reaches the password option' ); +( $out ) = vec_( 'mkvm', 'zvm02', '-c1_0w', 'SEKRET' ); +unlike( $out, qr/SEKRET/, 'an underscored cpu value still reaches the password option' ); +( $out, $changed ) = vec_( 'mkvm', 'zvm02', '-rabcw', 'KEEPME' ); +is( $changed, 0, 'a lowercase -r value absorbs the rest of the bundle' ); + +# Getopt::Long compatibility mode also accepts "+" as an option starter. +( $out ) = vec_( 'bmcdiscover', '+p', 'SEKRET', '--range', '10.0.0.1' ); +unlike( $out, qr/SEKRET/, 'a +p option is redacted' ); +( $out ) = vec_( 'bmcdiscover', '+bmcpasswd=SEKRET' ); +unlike( $out, qr/SEKRET/, 'a +bmcpasswd=value is redacted' ); +( $out ) = vec_( 'mkvm', 'zvm02', '+w', 'SEKRET' ); +unlike( $out, qr/SEKRET/, 'a +w option is redacted' ); +( $out ) = vec_( 'switchdiscover', '--range', '10.0.0.0/24', '-xc', 'SEKRET' ); +unlike( $out, qr/SEKRET/, 'a bundled -xc community is redacted' ); +( $out ) = vec_( 'mkhwconn', 'frame', '-tP', 'SEKRET' ); +unlike( $out, qr/SEKRET/, 'a bundled -tP password is redacted' ); +( $out, $changed ) = vec_( 'mkhwconn', 'frame', '-p', 'hmc01' ); +is( $out, 'frame -p hmc01', 'the mkhwconn -p stays visible, it is not the password' ); +is( $changed, 0, 'the mkhwconn hardware control point reports no change' ); +( $out ) = vec_( 'mkvm', 'zvm02', '-W', 'SEKRET' ); +unlike( $out, qr/SEKRET/, 'the mkvm -W is redacted, the parser ignores case' ); +( $out ) = vec_( 'mkvm', 'zvm02', '--PASSWORD=SEKRET' ); +unlike( $out, qr/SEKRET/, 'the mkvm --PASSWORD=value is redacted' ); +( $out ) = vec_( 'mkvm', 'zvm02', '--Pass', 'SEKRET' ); +unlike( $out, qr/SEKRET/, 'the mkvm abbreviated --Pass is redacted' ); +( $out ) = vec_( 'mkvm', 'gpok4', 'gpok3', 'pool=POOL1', 'pw=SEKRET' ); +unlike( $out, qr/SEKRET/, 'the mkvm clone pw= operand is redacted' ); +like( $out, qr/pool=POOL1/, 'the mkvm clone pool= operand is kept' ); + +# chvm carries passwords as positional operands. +( $out ) = vec_( 'chvm', '--setpassword', 'SEKRET' ); +unlike( $out, qr/SEKRET/, 'the chvm --setpassword operand is redacted' ); +( $out ) = vec_( 'chvm', '--add3390', 'POOL1', '0101', '3g', 'MR', 'RSEKRET', 'WSEKRET', 'MSEKRET' ); +unlike( $out, qr/SEKRET/, 'the chvm --add3390 disk passwords are redacted' ); +like( $out, qr/POOL1 0101 3g MR/, 'the chvm --add3390 disk parameters are kept' ); +( $out ) = vec_( 'chvm', '--formatdisk', '0100', 'SEKRET' ); +unlike( $out, qr/SEKRET/, 'the chvm --formatdisk password is redacted' ); +like( $out, qr/--formatdisk 0100/, 'the chvm --formatdisk address is kept' ); + +# The site table stores the global SNMP community string under the snmpc key. +( $out ) = vec_( 'tabch', 'key=snmpc', 'site.value=SEKRET' ); +unlike( $out, qr/SEKRET/, 'a tabch of the snmpc site value is redacted' ); +( $out ) = vec_( 'tabch', 'key=snmpc', 'site.value+=SEKRET' ); +unlike( $out, qr/SEKRET/, 'a tabch += splice of the snmpc site value is redacted' ); +( $out ) = vec_( 'tabch', 'key=snmpc,key=snmpc', 'site.value=SEKRET' ); +unlike( $out, qr/SEKRET/, 'a compound tabch selector with snmpc is redacted' ); +( $out ) = vec_( 'tabch', 'key=temporary', 'site.key=snmpc', 'site.value=SEKRET' ); +unlike( $out, qr/SEKRET/, 'a site.key=snmpc assignment marks the value secret' ); +( $out ) = vec_( 'tabch', 'site.key=snmpc', 'site.value=SEKRET' ); +unlike( $out, qr/SEKRET/, 'a table-qualified snmpc selector is redacted' ); +( $out, $changed ) = vec_( 'tabch', 'key=domain', 'site.value=lab' ); +is( $out, 'key=domain site.value=lab', 'a tabch of a plain site value is kept' ); +is( $changed, 0, 'a benign vector reports no change' ); +unlike( arg('snmpc=SEKRET'), qr/SEKRET/, 'the snmpc site attribute is redacted' ); + +# Product keys are license secrets. +unlike( arg('productkey=SEKRET'), qr/SEKRET/, 'a product key is redacted' ); +unlike( arg('prodkey.key=SEKRET'), qr/SEKRET/, 'a table-qualified product key is redacted' ); + +# The token table stores bearer credentials under tokenid. +unlike( arg('tokenid=SEKRET'), qr/SEKRET/, 'an authentication token is redacted' ); +unlike( arg('token.tokenid=SEKRET'), qr/SEKRET/, 'a table-qualified token is redacted' ); + +# Every attribute and column Schema.pm marks secret must be covered. Keep every +# (attribute, column) pair so a removed table-qualified column is caught, not +# masked by another pair that shares the attribute name. +my @pairs; while ( $schema_source =~ /attr_name\s*=>\s*'([^']+)'(.{0,400}?)tabentry\s*=>\s*'([^']+)'/gs ) { my ( $attr, $tabentry ) = ( $1, $3 ); - next unless $tabentry =~ /\.(password|passwd|authkey|privkey|adminpassword|sshpassword)$/i; - $expected{$attr} = $tabentry; + next + unless $tabentry =~ /\.(password|passwd|authkey|privkey|adminpassword|sshpassword|community)$/i + or $tabentry eq 'prodkey.key'; + push @pairs, [ $attr, $tabentry ]; } -ok( scalar( keys %expected ) > 0, 'Schema.pm yielded attributes mapped to secret columns' ) +ok( scalar(@pairs) > 0, 'Schema.pm yielded attributes mapped to secret columns' ) or BAIL_OUT('the Schema.pm mapping could not be parsed, so this test proves nothing'); my @uncovered; -foreach my $attr ( sort keys %expected ) { - push @uncovered, $attr - if redacted( 'chdef', " node01 $attr=SEKRET" ) =~ /SEKRET/; - my $column = $expected{$attr}; - push @uncovered, $column - if redacted( 'chdef', " node01 $column=SEKRET" ) =~ /SEKRET/; +foreach my $pair (@pairs) { + my ( $attr, $column ) = @$pair; + push @uncovered, $attr if arg("$attr=SEKRET") =~ /SEKRET/; + push @uncovered, $column if arg("$column=SEKRET") =~ /SEKRET/; } is_deeply( \@uncovered, [], 'every attribute and column Schema.pm marks secret is redacted' ); -# The auditlog table was given the raw arguments while syslog was given the -# redacted ones, so both sinks must now use the same text. -like( $source, qr/\$redacted_arglist\s*=\s*redact_password/, 'the redacted arguments are computed once' ); +# validate() must redact the argument vector and still run the joined result +# through redact_password, so every secret reaches syslog and the auditlog +# table redacted. +like( $source, qr/=\s*xCAT::xcatd->redact_password_args\(\$request->\{command\}->\[0\]/, 'validate() redacts the argument vector' ); +like( $source, qr/\$redacted_arglist\s*=\s*redact_password\b/, 'validate() redacts the arguments through redact_password' ); + +# The debug dispatch trace must not rebuild the command from the raw request. +like( $daemon_source, qr/\(\$trace_args\)\s*=\s*xCAT::xcatd->redact_password_args\(\$req->\{command\}->\[0\]/, + 'the dispatch trace builds its text from redacted arguments' ); + +# The auditlog table and syslog use the same redacted arguments. like( $source, qr/\$rsp->\{args\}->\[0\]\s*=\s*\$redacted_arglist/, 'the auditlog table stores the redacted arguments' ); done_testing();