Merge pull request #7732 from VersatusHPC/fix/noderange-preauth

fix(xcatd): refuse the noderange ^file operator on unauthenticated requests

perl-xCAT/xCAT/NodeRange.pm

@@ -11,6 +11,7 @@ our @EXPORT    = qw(noderange nodesmissed);
 our @EXPORT_OK = qw(extnoderange abbreviate_noderange);
 
 my $missingnodes = [];
+my $filerejected = 0;
 my $nodelist;    #=xCAT::Table->new('nodelist',-create =>1);
 my $grptab;
 
@@ -72,6 +73,10 @@ sub nodesmissed {
     return @$missingnodes;
 }
 
+sub file_operator_rejected {
+    return $filerejected;
+}
+
 sub reset_db {
 
     #workaround, something seems to be trying to use a corrupted reference to grptab
@@ -521,7 +526,7 @@ sub extnoderange { #An extended noderange function.  Needed by the GUI as the mo
     }
     my $return;
     $retaincache = 1;
-    $return->{node} = [ noderange($range, $verify) ];
+    $return->{node} = [ noderange($range, $verify, 1, nofile => $namedopts->{nofile}) ];
     if ($namedopts->{intersectinggroups}) {
         my %grouphash = ();
         my $nlent;
@@ -626,6 +631,7 @@ sub noderange {
     my %options = @_;                               # additional options
     unless ($options{keepmissing}) {
         $missingnodes = [];
+        $filerejected = 0;
     }
 
     unless ($nodelist) {
@@ -679,6 +685,7 @@ sub noderange {
 
         if ($atom =~ /^\^(.*)$/) {    # get a list of nodes from a file
             my $nrfile = $1;
+            if ($options{nofile}) { $filerejected = 1; next; }
             open(my $nrf, '<', $nrfile) or next;
             while (<$nrf>) {
                 my $line = $_;
@@ -724,7 +731,11 @@ sub noderange {
         my $badnoderange = 0;
         my @badnodes     = ();
         if ($::XCATSITEVALS{excludenodes}) {
-            @badnodes = noderange($::XCATSITEVALS{excludenodes}, 1, 0, %options);
+            my $request_filerejected = $filerejected;
+            my %exclude_options = %options;
+            delete $exclude_options{nofile};
+            @badnodes = noderange($::XCATSITEVALS{excludenodes}, 1, 0, %exclude_options);
+            $filerejected = $request_filerejected;
             foreach my $bnode (@badnodes) {
                 if (!$delnodes{$bnode}) {
                     $delnodes{$bnode} = 1;

xCAT-server/lib/perl/xCAT/xcatd.pm

@@ -98,7 +98,10 @@ sub validate {
 
     my %req_noderange_info;
     if (defined $request->{noderange}->[0]) {
-        my @tmpn = xCAT::NodeRange::noderange($request->{noderange}->[0]);
+        my @tmpn = xCAT::NodeRange::noderange($request->{noderange}->[0], 1, 1, (defined($peername) ? () : (nofile => 1)));
+        if (!defined($peername) && xCAT::NodeRange::file_operator_rejected()) {
+            return 0;
+        }
         $req_noderange_info{leftnodenum} = @tmpn;
         if($req_noderange_info{leftnodenum}){
             $req_noderange_info{leftnodes} =  \@tmpn;

xCAT-server/sbin/xcatd

@@ -2849,7 +2849,7 @@ sub service_connection {
             }
 
              if (exists($req->{noderange}) && defined($req->{noderange}->[0])) {
-                my @nnodes = xCAT::NodeRange::noderange($req->{noderange}->[0]);
+                my @nnodes = xCAT::NodeRange::noderange($req->{noderange}->[0], 1, 1, (defined($peername) ? () : (nofile => 1)));
                 $numofnodes = (scalar(@nnodes));
             }
 

xCAT-test/unit/noderange_nofile.t

@@ -0,0 +1,68 @@
+#!/usr/bin/env perl
+use strict;
+use warnings;
+
+use FindBin;
+use File::Spec;
+use File::Temp qw(tempfile);
+use Test::More;
+
+my $libdir = "$FindBin::Bin/../../perl-xCAT";
+plan skip_all => 'NodeRange.pm not found' unless -r "$libdir/xCAT/NodeRange.pm";
+
+# Stub Table so NodeRange loads without a database. The ^ file path does not
+# call a Table method.
+BEGIN {
+    $INC{'xCAT/Table.pm'} = 1;
+    package xCAT::Table;
+    sub new { return bless {}, shift }
+    sub _set_use_cache { }
+    sub _build_cache   { }
+    our $AUTOLOAD;
+    sub AUTOLOAD { return }
+    sub DESTROY  { }
+}
+eval { require Text::Balanced; 1 } or plan skip_all => 'Text::Balanced is required';
+
+push @INC, $libdir;
+require xCAT::NodeRange;
+
+my ($fh, $listfile) = tempfile('xcat_nrlist_XXXXXX', TMPDIR => 1, UNLINK => 1);
+print {$fh} "n1uniqtoken\n";
+close $fh;
+
+eval { xCAT::NodeRange::noderange('^' . $listfile, 0, 0) };
+is($@, '', 'a real ^file is processed without error');
+ok(!xCAT::NodeRange::file_operator_rejected(), 'without nofile the ^file operator is not rejected');
+
+my @nofile = eval { xCAT::NodeRange::noderange('^' . $listfile, 0, 0, nofile => 1) };
+is($@, '', 'nofile: a ^file range is accepted without error');
+is_deeply(\@nofile, [], 'nofile: the ^file operator yields no nodes');
+ok(xCAT::NodeRange::file_operator_rejected(), 'nofile: the ^file operator is flagged as rejected');
+
+xCAT::NodeRange::noderange('nodeX,^' . $listfile, 0, 0, nofile => 1);
+ok(xCAT::NodeRange::file_operator_rejected(), 'nofile: a ^file mixed with a plain node is still flagged');
+
+xCAT::NodeRange::noderange('nodeX', 0, 0, nofile => 1);
+ok(!xCAT::NodeRange::file_operator_rejected(), 'nofile: a plain range is not flagged');
+
+my $marker = File::Spec->catfile(File::Spec->tmpdir, "xcat_nr_nofile_marker_$$");
+unlink $marker;
+END { unlink $marker if defined $marker }
+eval { xCAT::NodeRange::noderange('^touch ' . $marker . ' |', 0, 0, nofile => 1) };
+ok(!-e $marker, 'nofile: the ^file operator opens and runs nothing');
+
+$::XCATSITEVALS{excludenodes} = 'excludednode';
+xCAT::NodeRange::noderange('nodeX,^' . $listfile, 0, 1, nofile => 1);
+ok(xCAT::NodeRange::file_operator_rejected(),
+    'site.excludenodes does not erase a rejected request ^file');
+
+$::XCATSITEVALS{excludenodes} = '^' . $listfile;
+xCAT::NodeRange::noderange('nodeX', 0, 1, nofile => 1);
+ok(!xCAT::NodeRange::file_operator_rejected(),
+    'a trusted site.excludenodes ^file is not attributed to the request');
+ok((grep { $_ eq 'n1uniqtoken' } xCAT::NodeRange::nodesmissed()),
+    'a trusted site.excludenodes ^file is read, not skipped');
+$::XCATSITEVALS{excludenodes} = undef;
+
+done_testing();

xCAT-test/unit/xcatd_validate_nofile.t

@@ -0,0 +1,84 @@
+#!/usr/bin/env perl
+use strict;
+use warnings;
+
+use FindBin;
+use Test::More;
+
+my $xcatdlib = "$FindBin::Bin/../../xCAT-server/lib/perl";
+plan skip_all => 'xcatd.pm not found' unless -r "$xcatdlib/xCAT/xcatd.pm";
+
+# Load xcatd.pm with stub modules. The policy is one nameless rule that allows
+# rpower. An authorized request returns 1, so only the ^file guard can deny it.
+# NodeRange is a spy. It records the expansion and reports a rejected ^file, as
+# the real module does.
+BEGIN {
+    for my $mod (qw(Date::Parse xCAT::Table xCAT::TableUtils xCAT::MsgUtils
+        xCAT::Utils xCAT::NodeRange)) {
+        (my $path = $mod) =~ s{::}{/}g;
+        $INC{"$path.pm"} = 1;
+    }
+
+    package Date::Parse;
+    package xCAT::TableUtils;
+    our $AUTOLOAD; sub AUTOLOAD { return } sub DESTROY { }
+
+    package xCAT::MsgUtils;
+    our $AUTOLOAD2; sub AUTOLOAD { return } sub DESTROY { }
+
+    package xCAT::Utils;
+    our $AUTOLOAD3; sub AUTOLOAD { return } sub DESTROY { }
+
+    package xCAT::Table;
+    sub new { return bless {}, shift }
+    sub getAllEntries { return [ { priority => 1, rule => 'allow', commands => 'rpower' } ] }
+    sub close { }
+    our $AUTOLOAD4; sub AUTOLOAD { return } sub DESTROY { }
+
+    package xCAT::NodeRange;
+    our @CALLS;
+    our $REJECTED = 0;
+    sub noderange {
+        my ($range, undef, undef, %opts) = @_;
+        push @CALLS, [@_];
+        $REJECTED = ($opts{nofile} && defined $range && $range =~ /\^/) ? 1 : 0;
+        return ('somenode');
+    }
+    sub file_operator_rejected { return $REJECTED }
+    sub nodesmissed { return () }
+    our $AUTOLOAD5; sub AUTOLOAD { return } sub DESTROY { }
+}
+
+push @INC, $xcatdlib;
+require xCAT::xcatd;
+
+sub run {
+    my ($peername, $noderange) = @_;
+    @xCAT::NodeRange::CALLS = ();
+    my $req = { command => ['rpower'], noderange => [$noderange] };
+    my $rc = xCAT::xcatd->validate($peername, '10.0.0.9', $req, '10.0.0.9', []);
+    my ($call) = grep { defined $_->[0] && $_->[0] eq $noderange } @xCAT::NodeRange::CALLS;
+    my $nofile = 0;
+    if ($call) {
+        my @a = @$call;
+        for my $i (1 .. $#a - 1) {
+            $nofile = $a[$i + 1] if defined $a[$i] && $a[$i] eq 'nofile';
+        }
+    }
+    return ($rc, $nofile);
+}
+
+my ($rc_auth, $nf_auth) = run('someuser', '^/etc/shadow');
+ok(!$nf_auth, 'authenticated: the ^ file operator is honored (no nofile)');
+is($rc_auth, 1, 'authenticated: an authorized ^file request is allowed');
+
+my ($rc_anon) = run(undef, '^/etc/shadow');
+is($rc_anon, 0, 'unauthenticated: a ^file request is denied (fail closed)');
+
+my ($rc_mixed) = run(undef, 'node01,^touch /tmp/x |');
+is($rc_mixed, 0, 'unauthenticated: a mixed node,^file range is denied');
+
+my (undef, $nf_zero) = run('0', '^/etc/shadow');
+ok(!$nf_zero, 'identity "0" is treated as authenticated, not anonymous');
+
+done_testing();