diff --git a/.github/workflows/xcat_test.yml b/.github/workflows/xcat_test.yml index e1bcb7c17..d3413d8d3 100644 --- a/.github/workflows/xcat_test.yml +++ b/.github/workflows/xcat_test.yml @@ -9,8 +9,10 @@ jobs: - name: Refresh package indexes run: sudo apt-get update - name: Install dependencies - run: sudo env DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends --no-install-suggests bats bubblewrap rpm build-essential fakeroot reprepro devscripts debhelper libcapture-tiny-perl libfile-slurper-perl libjson-perl libyaml-pp-perl libparallel-forkmanager-perl libsoap-lite-perl libdbi-perl libcgi-pm-perl quilt openssh-server dpkg looptools genometools software-properties-common grub-common grub-efi-amd64-bin + run: sudo env DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends --no-install-suggests bats bubblewrap rpm rsyslog build-essential fakeroot reprepro devscripts debhelper libcapture-tiny-perl libfile-slurper-perl libjson-perl libyaml-pp-perl libparallel-forkmanager-perl libsoap-lite-perl libdbi-perl libcgi-pm-perl quilt openssh-server dpkg looptools genometools software-properties-common grub-common grub-efi-amd64-bin - name: Enable postscript test namespaces run: sudo sysctl -w kernel.apparmor_restrict_unprivileged_userns=0 + - name: Prepare xCAT syslog directory + run: sudo install -d -o syslog -g adm -m 0750 /var/log/xcat - name: Run tests run: perl github_action_xcat_test.pl diff --git a/xCAT-test/autotest/testcase/integration/cases0 b/xCAT-test/autotest/testcase/integration/cases0 index 2795c560e..e81d1bd89 100644 --- a/xCAT-test/autotest/testcase/integration/cases0 +++ b/xCAT-test/autotest/testcase/integration/cases0 @@ -6,3 +6,11 @@ cmd:prove -I/opt/xcat/lib/perl -I/opt/xcat/lib/perl/xCAT -r /opt/xcat/share/xcat check:rc==0 check:output=~Files=9, end + +start:xcatd_dispatch_redaction +description:Check password redaction in the live daemon's syslog output +os:Linux +label:mn_only,ci_test,integration +cmd:sh /opt/xcat/share/xcat/tools/autotest/integration/xcatd_dispatch_redaction.sh +check:rc==0 +end diff --git a/xCAT-test/integration/README.md b/xCAT-test/integration/README.md index bfb2a93c0..ee6f0e22c 100644 --- a/xCAT-test/integration/README.md +++ b/xCAT-test/integration/README.md @@ -4,7 +4,7 @@ Integration tests. These run against an **installed management node** -- they ne real xCAT installation, and depending on the test a populated `/install`, a service binary they can execute, or a live daemon. -They are driven by `xcattest` through the testcase in +The Perl tests are driven by `xcattest` through the testcase in `../autotest/testcase/integration/`, which proves the copy installed by the `xcat-test` package: @@ -55,6 +55,12 @@ A test belongs in `integration/` when it needs something the checkout cannot pro | `html_form_runtime_dependency.t` | the installed `HTML::Form` module | | `openeuler_package_postscripts.t` | Linux root, private mount namespaces and DNF; tests complete package postscripts with command doubles and an isolated repository directory | | `openeuler_postscript_repositories.t` | Linux root, private mount namespaces, DNF and RPM signing tools; tests actual signed repository transactions in isolated install roots | +| `xcatd_dispatch_redaction.sh` | a live `xcatd`, syslog delivery to `/var/log/xcat/cluster.log`, and root | + +Run `xcattest -t xcatd_dispatch_redaction` to check password redaction in the +daemon's syslog output. This separate case has the `ci_test` label. It enables +`site.xcatdebugmode`, creates a temporary node, and restores the setting and removes +the node on exit. Run it without concurrent changes to `site.xcatdebugmode`. ## Environment guards diff --git a/xCAT-test/integration/xcatd_dispatch_redaction.sh b/xCAT-test/integration/xcatd_dispatch_redaction.sh new file mode 100644 index 000000000..ac66b236c --- /dev/null +++ b/xCAT-test/integration/xcatd_dispatch_redaction.sh @@ -0,0 +1,78 @@ +#!/bin/sh + +set -eu +umask 077 + +fail() +{ + echo "$*" >&2 + exit 1 +} + +[ "$(id -u)" -eq 0 ] || fail 'Run this test as root on a management node.' +log=/var/log/xcat/cluster.log +work=$(mktemp -d /tmp/xcat-redaction.XXXXXXXX) +node=$(printf '%s' "xcat-redaction-${work##*.}" | tr '[:upper:]' '[:lower:]') +restore_debug=0 +remove_node=0 + +cleanup() +{ + status=$? + trap - 0 HUP INT TERM + if [ "$restore_debug" -eq 1 ]; then + chdef -t site "xcatdebugmode=$debug" || status=1 + fi + if [ "$remove_node" -eq 1 ]; then + rmdef "$node" || status=1 + fi + rm -rf "$work" || status=1 + exit "$status" +} +trap cleanup 0 +trap 'exit 1' HUP INT TERM + +wait_for_log() +{ + attempt=0 + while :; do + if [ -e "$log" ]; then + if grep -F "$1" "$log" > /dev/null; then + return + else + status=$? + [ "$status" -eq 1 ] || fail 'Cannot read the syslog marker.' + fi + fi + attempt=$((attempt + 1)) + [ "$attempt" -lt 30 ] || fail 'Timed out waiting for syslog.' + sleep 1 + done +} + +logger -p local4.debug -t xcat "$node ready" +wait_for_log "$node ready" + +lsdef -t site -i xcatdebugmode > "$work/site" +debug=$(sed -n 's/^[[:space:]]*xcatdebugmode=//p' "$work/site") +mkdef "$node" groups=all mgt=ipmi +remove_node=1 +restore_debug=1 +chdef -t site xcatdebugmode=1 +chdef "$node" "bmcpassword=SEKRET-$node" +lsdef "$node" -i bmcpassword > "$work/node" +grep -Fx " bmcpassword=SEKRET-$node" "$work/node" > /dev/null || + fail 'Logging changed the password stored by chdef.' +logger -p local4.debug -t xcat "$node complete" +wait_for_log "$node complete" + +grep -F "$node" "$log" > "$work/log" +if grep -F "SEKRET-$node" "$work/log" > /dev/null; then + fail 'The unmasked password reached syslog.' +else + status=$? + [ "$status" -eq 1 ] || fail 'Cannot check syslog for the password.' +fi +grep -F "xcatd: dispatch request 'chdef $node bmcpassword=xxxxxxxx' to plugin '" \ + "$work/log" > /dev/null || fail 'The masked chdef dispatch trace is missing.' +echo 'The dispatch trace masks the password and keeps the command and node.' diff --git a/xCAT-test/unit/fixtures/redaction/RedactionDependencies.pm b/xCAT-test/unit/fixtures/redaction/RedactionDependencies.pm new file mode 100644 index 000000000..6e6bafccb --- /dev/null +++ b/xCAT-test/unit/fixtures/redaction/RedactionDependencies.pm @@ -0,0 +1,32 @@ +package RedactionDependencies; + +use strict; +use warnings; + +BEGIN { + for my $module (qw(xCAT::Table xCAT::TableUtils xCAT::MsgUtils + xCAT::NodeRange xCAT::Utils xCAT::ExtTab)) { + (my $file = "$module.pm") =~ s{::}{/}g; + $INC{$file} = __FILE__; + } +} + +package xCAT::Table; + +our $rule = 'allow'; + +sub new { + my ($class, $name) = @_; + die "Unexpected table: $name" unless $name eq 'policy'; + return bless {}, $class; +} + +sub getAllEntries { + return [{priority => 1, rule => $rule}]; +} + +sub close { return; } +sub shut_dbworker { die 'Unexpected database worker shutdown'; } +sub init_dbworker { die 'Unexpected database worker startup'; } + +1; diff --git a/xCAT-test/unit/xcatd_password_redaction.t b/xCAT-test/unit/xcatd_password_redaction.t index 33ba219fe..a37c4938c 100644 --- a/xCAT-test/unit/xcatd_password_redaction.t +++ b/xCAT-test/unit/xcatd_password_redaction.t @@ -3,73 +3,53 @@ use strict; use warnings; use FindBin; -use File::Spec; +use Storable qw(dclone); use Test::More; +use lib "$FindBin::Bin/../lib"; +use XCAT::Test::File qw(repo_path); -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' ); +use lib repo_path('perl-xCAT'), repo_path('xCAT-server/lib/perl'); +use lib repo_path('xCAT-test/unit/fixtures/redaction'); +use RedactionDependencies; -plan skip_all => 'xcatd.pm, xcatd or Schema.pm not found' unless -r $xcatd && -r $daemon && -r $schema; +local $ENV{XCATROOT} = repo_path('xCAT-test/unit/fixtures/redaction'); +require xCAT::Schema; +require xCAT::xcatd; +our %XCATSITEVALS; -sub slurp { - open( my $fh, '<', $_[0] ) or die "Unable to read $_[0]: $!"; - my $c = do { local $/; <$fh> }; - close($fh); - return $c; -} - -my $source = slurp($xcatd); -my $daemon_source = slurp($daemon); -my $schema_source = slurp($schema); - -# 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 arg { return RedactUnderTest::redact_password_arg( 'xCAT::xcatd', $_[0] ); } -sub cmd { return RedactUnderTest::redact_password( $_[0], $_[1] ); } +sub arg { return xCAT::xcatd->redact_password_arg($_[0]); } +sub cmd { return xCAT::xcatd::redact_password($_[0], $_[1]); } sub vec_ { + local $Test::Builder::Level = $Test::Builder::Level + 1; my ( $command, @args ) = @_; my ( $redacted, $changed ) = - RedactUnderTest->redact_password_args( $command, \@args ); + xCAT::xcatd->redact_password_args( $command, \@args ); + is( scalar(@$redacted), scalar(@args), 'redaction preserves the argument count' ); return ( join( ' ', @$redacted ), $changed ); } # 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' ); +is( arg('bmcpassword=SEKRET'), 'bmcpassword=xxxxxxxx', 'a bare attribute value is redacted' ); +is( arg('bmcpassword=SEKRET phrase'), 'bmcpassword=xxxxxxxx', 'a value with spaces is redacted whole' ); +is( arg("bmcpassword=has'quote"), 'bmcpassword=xxxxxxxx', 'a value with a quote is redacted whole' ); +is( arg('passwd.password+=SEKRET'), 'passwd.password+=xxxxxxxx', 'a += splice assignment is redacted' ); +is( arg('pdu.community=SEKRET'), 'pdu.community=xxxxxxxx', 'the SNMP community string is redacted' ); +is( arg('community=SEKRET'), 'community=xxxxxxxx', 'a bare community value is redacted' ); # 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' ); +is( arg('bmcpassword = SEKRET'), 'bmcpassword=xxxxxxxx', 'spaces around the equals are redacted' ); +is( arg('bmcpassword =SEKRET'), 'bmcpassword=xxxxxxxx', 'a space before the equals is redacted' ); +is( arg('bmcpassword= SEKRET'), 'bmcpassword=xxxxxxxx', 'a space after the equals is redacted' ); +is( arg('bmcpassword = SEKRET phrase'), 'bmcpassword=xxxxxxxx', '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' ); +is( arg('ipmi.password,=SEKRET'), 'ipmi.password,=xxxxxxxx', 'a ,= append assignment is redacted' ); +is( arg('ipmi.password^=SEKRET'), 'ipmi.password^=xxxxxxxx', 'a ^= remove assignment is redacted' ); +is( arg('ipmi.password!=SEKRET'), 'ipmi.password!=xxxxxxxx', 'a != selection is redacted' ); +is( arg('ipmi.password!~SEKRET'), 'ipmi.password!~xxxxxxxx', 'a !~ selection is redacted' ); +is( arg('ipmi.password=~SEKRET'), 'ipmi.password=~xxxxxxxx', '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' ); @@ -78,19 +58,19 @@ is( arg('sshkeydir=/etc/xcat/keys'), 'sshkeydir=/etc/xcat/keys', 'sshkeydir is 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/, +is( cmd( 'bmcdiscover', ' -s nmap -p SEKRET --range 10.0.0.1' ), ' -s nmap -p xxxxxxxx --range 10.0.0.1', '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/, +is( cmd( 'xdsh', " compute 'echo bmcpassword=SEKRET > /etc/x'" ), " compute 'echo bmcpassword=xxxxxxxx", 'a secret embedded in a compound argument is redacted' ); -unlike( cmd( 'xdsh', q{ compute "export password='SEKRET phrase'; run-app"} ), qr/SEKRET|phrase/, +is( cmd( 'xdsh', q{ compute "export password='SEKRET phrase'; run-app"} ), ' compute "export password=xxxxxxxx', 'a quoted secret embedded in a compound argument is redacted whole' ); -unlike( arg(q{export password='SEKRET phrase'; run-app}), qr/SEKRET|phrase/, +is( arg(q{export password='SEKRET phrase'; run-app}), 'export password=xxxxxxxx', 'a quoted secret inside one argument is redacted to the end' ); -unlike( arg('usercomment=password=SEKRET'), qr/SEKRET/, +is( arg('usercomment=password=SEKRET'), 'usercomment=password=xxxxxxxx', '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' ); @@ -99,41 +79,42 @@ is( arg('echo groups=lab; ls'), 'echo groups=lab; ls', 'an embedded non-secret a # 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( $out, '--range 10.0.0.1 -p xxxxxxxx', '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' ); +is( $out, '--range 10.0.0.1 -pxxxxxxxx', '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' ); +is( $out, '--range 10.0.0.1 --bmcpasswd xxxxxxxx', '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' ); +is( $out, '--range 10.0.0.1 --bmcpasswd=xxxxxxxx', 'a --bmcpasswd=value is redacted' ); ( $out ) = vec_( 'bmcdiscover', '-n', 'SEKRET' ); -unlike( $out, qr/SEKRET/, 'a -n new password is redacted' ); +is( $out, '-n xxxxxxxx', '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' ); +is( $out, '--range 10.0.0.0/24 -c xxxxxxxx', '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' ); +is( $out, 'frame -p hmc01 -Pxxxxxxxx', 'the mkhwconn -P password is redacted' ); ( $out ) = vec_( 'mkvm', 'zvm02', '--password', 'SEKRET' ); -unlike( $out, qr/SEKRET/, 'the mkvm --password value is redacted' ); +is( $out, 'zvm02 --password xxxxxxxx', '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' ); +is( $out, 'admin_passwd=xxxxxxxx', 'a rspconfig password assignment is redacted whole' ); +( $out ) = vec_( 'chdef', 'groups=compute', 'bmcpassword=SEKRET' ); +is( $out, 'groups=compute bmcpassword=xxxxxxxx', 'a secret attribute after a non-secret argument is redacted' ); # 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' ); +is( $out, '--range 10.0.0.1 --bmcp xxxxxxxx', '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' ); +is( $out, '--range 10.0.0.1 --bmcpas=xxxxxxxx', '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' ); +is( $out, '--range 10.0.0.1 -zp xxxxxxxx', 'a bundled -zp is redacted' ); ( $out ) = vec_( 'bmcdiscover', '--range', '10.0.0.1', '-zpSEKRET' ); -unlike( $out, qr/SEKRET/, 'a bundled compact -zpSEKRET is redacted' ); +is( $out, '--range 10.0.0.1 -zpxxxxxxxx', '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' ); @@ -142,25 +123,26 @@ is( $out, '-pxxxxxxxx --range 10.0.0.1', 'a compact value ending in a secret let 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' ); +# bmcdiscover --p= and mkhwconn --P= currently omit '=' in the log. ( $out ) = vec_( 'bmcdiscover', '--p=SEKRET' ); -unlike( $out, qr/SEKRET/, 'a double-dash --p=value is redacted' ); +is( $out, '--pxxxxxxxx', 'a double-dash --p=value is redacted' ); ( $out ) = vec_( 'switchdiscover', '--c', 'SEKRET' ); -unlike( $out, qr/SEKRET/, 'a double-dash --c community is redacted' ); +is( $out, '--c xxxxxxxx', 'a double-dash --c community is redacted' ); ( $out ) = vec_( 'mkhwconn', 'frame', '--P=SEKRET' ); -unlike( $out, qr/SEKRET/, 'a double-dash --P=value is redacted' ); +is( $out, 'frame --Pxxxxxxxx', 'a double-dash --P=value is redacted' ); ( $out, $changed ) = vec_( 'mkhwconn', 'frame', '--p', 'hmc01' ); +is( $out, 'frame --p hmc01', 'the double-dash control point stays visible' ); 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' ); +is( $out, 'zvm02 --W xxxxxxxx', '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' ); +is( $out, '--vcenter vc01 --username admin --password xxxxxxxx cluster01', 'the createvcluster password is redacted' ); ( $out ) = vec_( 'lsvcluster', '--vcenter', 'vc01', '--password=SEKRET' ); -unlike( $out, qr/SEKRET/, 'the lsvcluster password is redacted' ); +is( $out, '--vcenter vc01 --password=xxxxxxxx', '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' ); +is( $out, '--vcenter vc01 --PASSWORD xxxxxxxx cluster01', '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. @@ -171,15 +153,17 @@ is( $changed, 0, 'a non-secret compact value reports no change' ); 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( $out, 'frame -pPOWERHMC', 'the compact control point stays visible' ); is( $changed, 0, 'the mkhwconn -p value is kept, P is inside the value of -p' ); ( $out, $changed ) = vec_( 'bmcdiscover', '-zsp', 'SEKRET' ); +is( $out, '-zsp SEKRET', 'the scan method and non-secret positional argument stay visible' ); 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' ); +is( $out, '-?pxxxxxxxx', 'a bundle with the ? help letter is redacted' ); ( $out ) = vec_( 'bmcdiscover', '-?n', 'SEKRET' ); -unlike( $out, qr/SEKRET/, 'a -?n bundle masks the next argument' ); +is( $out, '-?n xxxxxxxx', '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' ); @@ -190,124 +174,169 @@ 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' ); +is( $out, 'zvm02 -swxxxxxxxx', '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' ); +is( $out, 'zvm02 -sw xxxxxxxx', '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' ); +is( $out, 'zvm02 -c1wxxxxxxxx', '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' ); +is( $out, 'zvm02 -c1w xxxxxxxx', 'zVM numeric bundle masks the following password argument' ); ( $out, $changed ) = vec_( 'mkvm', 'zvm02', '-c12' ); +is( $out, 'zvm02 -c12', 'the numeric cpu value stays visible' ); 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' ); +is( $out, 'zvm02 -Rw xxxxxxxx', 'a capital -Rw bundle masks the next argument' ); ( $out ) = vec_( 'mkvm', 'zvm02', '-RwSEKRET' ); -unlike( $out, qr/SEKRET/, 'a glued capital -RwSEKRET is redacted' ); +is( $out, 'zvm02 -Rwxxxxxxxx', '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' ); +is( $out, 'zvm02 -c+1w xxxxxxxx', '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' ); +is( $out, 'zvm02 -c-1wxxxxxxxx', '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' ); +is( $out, 'zvm02 -c1_0w xxxxxxxx', 'an underscored cpu value still reaches the password option' ); ( $out, $changed ) = vec_( 'mkvm', 'zvm02', '-rabcw', 'KEEPME' ); +is( $out, 'zvm02 -rabcw KEEPME', 'the non-secret value and following argument stay visible' ); 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' ); +is( $out, '+p xxxxxxxx --range 10.0.0.1', 'a +p option is redacted' ); ( $out ) = vec_( 'bmcdiscover', '+bmcpasswd=SEKRET' ); -unlike( $out, qr/SEKRET/, 'a +bmcpasswd=value is redacted' ); +is( $out, '+bmcpasswd=xxxxxxxx', 'a +bmcpasswd=value is redacted' ); ( $out ) = vec_( 'mkvm', 'zvm02', '+w', 'SEKRET' ); -unlike( $out, qr/SEKRET/, 'a +w option is redacted' ); +is( $out, 'zvm02 +w xxxxxxxx', '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' ); +is( $out, '--range 10.0.0.0/24 -xc xxxxxxxx', 'a bundled -xc community is redacted' ); ( $out ) = vec_( 'mkhwconn', 'frame', '-tP', 'SEKRET' ); -unlike( $out, qr/SEKRET/, 'a bundled -tP password is redacted' ); +is( $out, 'frame -tP xxxxxxxx', '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' ); +is( $out, 'zvm02 -W xxxxxxxx', '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' ); +is( $out, 'zvm02 --PASSWORD=xxxxxxxx', 'the mkvm --PASSWORD=value is redacted' ); ( $out ) = vec_( 'mkvm', 'zvm02', '--Pass', 'SEKRET' ); -unlike( $out, qr/SEKRET/, 'the mkvm abbreviated --Pass is redacted' ); +is( $out, 'zvm02 --Pass xxxxxxxx', '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' ); +is( $out, 'gpok4 gpok3 pool=POOL1 pw=xxxxxxxx', 'the mkvm clone pw= operand is redacted' ); # chvm carries passwords as positional operands. ( $out ) = vec_( 'chvm', '--setpassword', 'SEKRET' ); -unlike( $out, qr/SEKRET/, 'the chvm --setpassword operand is redacted' ); +is( $out, '--setpassword xxxxxxxx', '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' ); +is( $out, '--add3390 POOL1 0101 3g MR xxxxxxxx xxxxxxxx xxxxxxxx', 'the chvm --add3390 disk passwords are redacted' ); ( $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' ); +is( $out, '--formatdisk 0100 xxxxxxxx', 'the chvm --formatdisk password is redacted' ); # 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' ); +is( $out, 'key=snmpc site.value=xxxxxxxx', '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' ); +is( $out, 'key=snmpc site.value+=xxxxxxxx', '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' ); +is( $out, 'key=snmpc,key=snmpc site.value=xxxxxxxx', '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' ); +is( $out, 'key=temporary site.key=snmpc site.value=xxxxxxxx', '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' ); +is( $out, 'site.key=snmpc site.value=xxxxxxxx', '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' ); +is( arg('snmpc=SEKRET'), 'snmpc=xxxxxxxx', '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' ); +is( arg('productkey=SEKRET'), 'productkey=xxxxxxxx', 'a product key is redacted' ); +is( arg('prodkey.key=SEKRET'), 'prodkey.key=xxxxxxxx', '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' ); +is( arg('tokenid=SEKRET'), 'tokenid=xxxxxxxx', 'an authentication token is redacted' ); +is( arg('token.tokenid=SEKRET'), 'token.tokenid=xxxxxxxx', '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|community)$/i - or $tabentry eq 'prodkey.key'; - push @pairs, [ $attr, $tabentry ]; +foreach my $object ( sort keys %xCAT::Schema::defspec ) { + foreach my $definition ( @{ $xCAT::Schema::defspec{$object}->{attrs} || [] } ) { + my ($attr, $tabentry) = @{$definition}{qw(attr_name tabentry)}; + next unless defined $attr && defined $tabentry; + next unless $tabentry =~ /\.(password|passwd|authkey|privkey|adminpassword|sshpassword|community)$/i + || $tabentry eq 'prodkey.key'; + push @pairs, [ $attr, $tabentry ]; + } } -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'); +ok( scalar(@pairs) > 0, 'the loaded schema exposes secret-column mappings' ); -my @uncovered; 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( arg("$attr=SEKRET"), "$attr=xxxxxxxx", "$attr keeps its name and masks its value" ); + is( arg("$column=SEKRET"), "$column=xxxxxxxx", "$column keeps its name and masks its value" ); } -is_deeply( \@uncovered, [], 'every attribute and column Schema.pm marks secret is redacted' ); -# 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' ); +is(arg(undef), undef, 'an undefined argument stays undefined'); +is(arg(''), '', 'an empty argument stays empty'); +for my $args (undef, [], [undef, '']) { + my ($masked, $changed) = xCAT::xcatd->redact_password_args('chdef', $args); + is_deeply($masked, $args || [], 'missing and empty arguments stay unchanged'); + is($changed, 0, 'missing and empty arguments report no redaction'); +} -# 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' ); +my @requests = ( + ['chdef', ['bmcpassword=SEKRET phrase', 'groups=compute'], + ' bmcpassword=xxxxxxxx groups=compute'], + ['bmcdiscover', ['--bmcpasswd=SEKRET', '--range', '192.0.2.1'], + ' --bmcpasswd=xxxxxxxx --range 192.0.2.1'], + ['rspconfig', ['USERID=SEKRET', 'general=visible'], + ' USERID=xxxxxxxx general=visible'], + ['rspconfig', ['HMC_user=x admin_passwd=SEKRET'], + ' HMC_user=x admin_passwd=xxxxxxxx'], + ['mkvm', ['zvm02', '--password', 'SEKRET'], + ' zvm02 --password ******** '], + ['mkvm', ['zvm02', '-w', 'SEKRET'], + ' zvm02 -w ******** '], + ['tabch', ['key=snmpc', 'site.value=SEKRET'], + ' key=snmpc site.value=xxxxxxxx'], + ['chdef', ['groups=compute', 'usercomment=visible'], + ' groups=compute usercomment=visible'], +); -# 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' ); +for my $rule ('allow', 'deny') { + for my $sink ('SA', 'A', 'S') { + for my $case (@requests) { + my ($command, $args, $expected) = @$case; + subtest "$rule $command to $sink: @$args" => sub { + local $xCAT::Table::rule = $rule; + local %XCATSITEVALS = $sink eq 'A' ? (auditnosyslog => 1) + : $sink eq 'S' ? (auditskipcmds => 'ALL') : (); + my $request = { + command => [$command], arg => dclone($args), + node => ['node01'], username => ['operator'], clienttype => ['cli'], + }; + my @deferred; + my $allowed = xCAT::xcatd->validate( + 'operator', 'client.example', $request, undef, \@deferred, + ); + is($allowed, $rule eq 'allow' ? 1 : 0, 'policy result is preserved'); + is($deferred[0], $sink, 'the configured logging sinks are selected'); + my $status = $rule eq 'allow' ? 'Allowing' : 'Denying'; + my $syslog = "xCAT: $status $command to node01$expected for operator from client.example"; + if ($sink eq 'S') { + is($deferred[1], $syslog, 'syslog-only output masks secrets and keeps context'); + } else { + is($deferred[1]->{args}->[0], $expected, 'audit arguments mask secrets and keep context'); + if ($sink eq 'SA') { + is($deferred[1]->{syslogdata}->[0], $syslog, 'syslog has the same masked arguments'); + } else { + ok(!exists $deferred[1]->{syslogdata}, 'audit-only output omits syslog'); + } + } + is_deeply($request->{arg}, $args, 'logging leaves execution arguments intact'); + }; + } + } +} done_testing();