mirror of
https://github.com/SCST-project/scst.git
synced 2026-10-22 06:02:33 +00:00
scstadmin: Omit dynamic attribute values from update messages
Dynamic attributes can contain CHAP secrets. Administrative update, duplicate, removal and debug messages currently print their values, exposing them in captured command output. Report the operation and attribute name without the value in both the CLI and Perl library. Keep metadata-driven discovery and explicit privileged queries unchanged. Add a Ginkgo case that checks add, duplicate, remove, missing-value and debug output through the real scstadmin. Query the newly created attribute explicitly to confirm that the synthetic secret was stored before checking that the update output omits it. Link: https://github.com/SCST-project/scst/issues/91
This commit is contained in:
@@ -82,8 +82,9 @@ requires root access and a live SCST sysfs hierarchy.
|
||||
Save the current configuration to the specified file.
|
||||
The file may contain authentication secrets and is created or restricted to
|
||||
mode 0600 before writing. An existing output must be a regular file owned by
|
||||
the effective user; symbolic links are rejected. Saved configurations still
|
||||
contain the original authentication secrets.
|
||||
the effective user; symbolic links are rejected. Dynamic attribute values
|
||||
are omitted from update and debug messages, but explicit attribute queries
|
||||
and saved configurations still contain their original values.
|
||||
.TP
|
||||
.B -clear_config
|
||||
Remove all configured devices, targets that do not correspond to a physical
|
||||
|
||||
@@ -1016,7 +1016,7 @@ sub addDriverDynamicAttribute {
|
||||
my $io = new IO::File $path, O_WRONLY;
|
||||
if ($io) {
|
||||
if ($self->{'debug'}) {
|
||||
print "DBG($$): $path -> $cmd\n";
|
||||
print "DBG($$): $path -> add_attribute $attribute [redacted]\n";
|
||||
} else {
|
||||
$bytes = _syswrite($io, $cmd, length($cmd));
|
||||
}
|
||||
@@ -1054,7 +1054,7 @@ sub removeDriverDynamicAttribute {
|
||||
my $io = new IO::File $path, O_WRONLY;
|
||||
if ($io) {
|
||||
if ($self->{'debug'}) {
|
||||
print "DBG($$): $path -> $cmd\n";
|
||||
print "DBG($$): $path -> del_attribute $attribute [redacted]\n";
|
||||
} else {
|
||||
$bytes = _syswrite($io, $cmd, length($cmd));
|
||||
}
|
||||
@@ -1260,7 +1260,7 @@ sub addTargetDynamicAttribute {
|
||||
my $io = new IO::File $path, O_WRONLY;
|
||||
if ($io) {
|
||||
if ($self->{'debug'}) {
|
||||
print "DBG($$): $path -> $cmd\n";
|
||||
print "DBG($$): $path -> add_target_attribute $target $attribute [redacted]\n";
|
||||
} else {
|
||||
$bytes = _syswrite($io, $cmd, length($cmd));
|
||||
}
|
||||
@@ -1303,7 +1303,7 @@ sub removeTargetDynamicAttribute {
|
||||
my $io = new IO::File $path, O_WRONLY;
|
||||
if ($io) {
|
||||
if ($self->{'debug'}) {
|
||||
print "DBG($$): $path -> $cmd\n";
|
||||
print "DBG($$): $path -> del_target_attribute $target $attribute [redacted]\n";
|
||||
} else {
|
||||
$bytes = _syswrite($io, $cmd, length($cmd));
|
||||
}
|
||||
|
||||
@@ -4553,7 +4553,7 @@ sub setDriverAttributes {
|
||||
removeDriverDynamicAttribute($driver, $attribute, $value);
|
||||
$changes++;
|
||||
} else {
|
||||
print "\t-> Driver dynamic attribute '$attribute' with value '$value' ".
|
||||
print "\t-> Driver dynamic attribute '$attribute' ".
|
||||
"is not in configuration. Use -force to remove it.\n";
|
||||
}
|
||||
}
|
||||
@@ -4575,8 +4575,7 @@ sub addDriverDynamicAttributes {
|
||||
|
||||
foreach my $attribute (keys %{$attributes}) {
|
||||
if (defined($$existing{$attribute}->{$$attributes{$attribute}})) {
|
||||
my $value = $$attributes{$attribute};
|
||||
print "\t-> Attribute/value '$attribute/$value' already exists for driver '$driver'.\n";
|
||||
print "\t-> Value for dynamic attribute '$attribute' already exists for driver '$driver'.\n";
|
||||
next;
|
||||
}
|
||||
|
||||
@@ -4591,7 +4590,7 @@ sub addDriverDynamicAttribute {
|
||||
my $attribute = shift;
|
||||
my $value = shift;
|
||||
|
||||
print "\t-> Adding driver attribute '$attribute' with value '$value' for driver '$driver': ";
|
||||
print "\t-> Adding driver attribute '$attribute' for driver '$driver': ";
|
||||
my $rc = $SCST->addDriverDynamicAttribute($driver, $attribute, $value);
|
||||
print "done.\n";
|
||||
|
||||
@@ -4612,8 +4611,7 @@ sub removeDriverDynamicAttributes {
|
||||
|
||||
foreach my $attribute (keys %{$attributes}) {
|
||||
if (!defined($$existing{$attribute}->{$$attributes{$attribute}})) {
|
||||
my $value = $$attributes{$attribute};
|
||||
print "\t-> Attribute/value '$attribute/$value' doesn't exist for driver '$driver'.\n";
|
||||
print "\t-> Value for dynamic attribute '$attribute' doesn't exist for driver '$driver'.\n";
|
||||
next;
|
||||
}
|
||||
|
||||
@@ -4630,7 +4628,7 @@ sub removeDriverDynamicAttribute {
|
||||
|
||||
return TRUE if (!$SCST->driverIsVirtualCapable($driver));
|
||||
|
||||
print "\t-> Removing dynamic attribute '$attribute' with value '$value' for driver '$driver': ";
|
||||
print "\t-> Removing dynamic attribute '$attribute' for driver '$driver': ";
|
||||
my $rc = $SCST->removeDriverDynamicAttribute($driver, $attribute, $value);
|
||||
print "done.\n";
|
||||
|
||||
@@ -4731,7 +4729,7 @@ sub setTargetAttributes {
|
||||
removeTargetDynamicAttribute($driver, $target, $attribute, $value);
|
||||
$changes++;
|
||||
} else {
|
||||
print "\t-> Target dynamic attribute '$attribute' with value '$value' ".
|
||||
print "\t-> Target dynamic attribute '$attribute' ".
|
||||
"is not in configuration. Use -force to remove it.\n";
|
||||
}
|
||||
}
|
||||
@@ -4754,8 +4752,7 @@ sub addTargetDynamicAttributes {
|
||||
|
||||
foreach my $attribute (keys %{$attributes}) {
|
||||
if (defined($$existing{$attribute}->{$$attributes{$attribute}})) {
|
||||
my $value = $$attributes{$attribute};
|
||||
print "\t-> Attribute/value '$attribute/$value' already exists for driver/target ".
|
||||
print "\t-> Value for dynamic attribute '$attribute' already exists for driver/target ".
|
||||
"'$driver/$target'.\n";
|
||||
next;
|
||||
}
|
||||
@@ -4772,7 +4769,7 @@ sub addTargetDynamicAttribute {
|
||||
my $attribute = shift;
|
||||
my $value = shift;
|
||||
|
||||
print "\t-> Adding target attribute '$attribute' with value '$value' ".
|
||||
print "\t-> Adding target attribute '$attribute' ".
|
||||
"for driver/target '$driver/$target': ";
|
||||
my $rc = $SCST->addTargetDynamicAttribute($driver, $target, $attribute, $value);
|
||||
print "done.\n";
|
||||
@@ -4795,8 +4792,7 @@ sub removeTargetDynamicAttributes {
|
||||
|
||||
foreach my $attribute (keys %{$attributes}) {
|
||||
if (!defined($$existing{$attribute}->{$$attributes{$attribute}})) {
|
||||
my $value = $$attributes{$attribute};
|
||||
print "\t-> Attribute/value '$attribute/$value' doesn't exist for driver/target ".
|
||||
print "\t-> Value for dynamic attribute '$attribute' doesn't exist for driver/target ".
|
||||
"'$driver/$target'.\n";
|
||||
next;
|
||||
}
|
||||
@@ -4815,7 +4811,7 @@ sub removeTargetDynamicAttribute {
|
||||
|
||||
return TRUE if (!$SCST->driverIsVirtualCapable($driver));
|
||||
|
||||
print "\t-> Removing dynamic attribute '$attribute' with value '$value' for driver/target '$driver/$target': ";
|
||||
print "\t-> Removing dynamic attribute '$attribute' for driver/target '$driver/$target': ";
|
||||
my $rc = $SCST->removeTargetDynamicAttribute($driver, $target, $attribute, $value);
|
||||
print "done.\n";
|
||||
|
||||
|
||||
@@ -99,6 +99,34 @@ var _ = Describe("CHAP credential confidentiality", Label("basic", "iscsi", "cha
|
||||
Entry("existing private file", os.FileMode(0600)),
|
||||
)
|
||||
|
||||
It("does not print secrets during dynamic attribute updates or debug output", func(ctx SpecContext) {
|
||||
f := newFixture(ctx)
|
||||
secret, r := addTestCHAPUser(ctx, f)
|
||||
// Inspect only the management operation's output, not argv in the
|
||||
// command journal or the explicit privileged attribute query.
|
||||
Expect(r.Stdout + "\n" + r.Stderr).NotTo(ContainSubstring(secret))
|
||||
for _, operation := range []struct {
|
||||
name string
|
||||
debug bool
|
||||
}{
|
||||
{"-add_tgt_attr", false}, // Duplicate value.
|
||||
{"-rem_tgt_attr", true}, // Debug must not remove the value.
|
||||
{"-rem_tgt_attr", false},
|
||||
{"-rem_tgt_attr", false}, // Missing value.
|
||||
{"-add_tgt_attr", true},
|
||||
} {
|
||||
args := []string{operation.name, f.Target, "-driver", "iscsi", "-attributes", "IncomingUser=tester " + secret}
|
||||
if operation.debug {
|
||||
args = append(args, "-debug")
|
||||
}
|
||||
r, err := cli.Execute(ctx, args...)
|
||||
Expect(err).NotTo(HaveOccurred(), r.String())
|
||||
Expect(r.ExitCode).To(BeZero(), r.String())
|
||||
Expect(r.ReportedError()).To(BeFalse(), r.String())
|
||||
Expect(r.Stdout + "\n" + r.Stderr).NotTo(ContainSubstring(secret))
|
||||
}
|
||||
})
|
||||
|
||||
It("rejects a symlink output without changing its referent", Label("config"), func(ctx SpecContext) {
|
||||
f := newFixture(ctx)
|
||||
dir := filepath.Join(artifactDir, f.Device+"-output-link")
|
||||
|
||||
Reference in New Issue
Block a user