Extract the secret set, the command maps and the redaction routines from
xcatd.pm. Drive the real routines, because the module needs daemon
dependencies that the test host does not have.
Cover a multi-word value, a quoted value, spaces around the operator,
the "+=", ",=", "^=", "!=", "=~" and "!~" operators, the community
strings, a secret embedded in a compound argument with and without
quotes, each Getopt::Long password option form with bundles, the "?"
help letter, abbreviations, "+" prefixes and two-dash letters, values
that hold another secret letter, non-secret compact values that a value
option absorbs, long names that start with a secret letter, the mkvm
case forms and compact values, the chvm positional passwords, the mkvm
clone pw= operand, the vCenter cluster passwords, the snmpc site value
through chdef and tabch with compound selectors, the authentication
tokens, and the product keys. Assert that validate() redacts the
argument vector, that the joined result runs through redact_password,
and that the dispatch trace builds its text from redacted arguments.
Keep every attribute and column pair from Schema.pm, so a removed
table-qualified mapping fails the test.
The daemon redacted secret attributes on the joined command string. The
match failed when a value held a space. The match also failed for a "+="
splice assignment. The validate() path did not quote the arguments, so a
multi-word secret value kept its later words in syslog and in the
auditlog table. A password that a command receives through an option or
a positional operand was not redacted at all. The debug dispatch trace
wrote the raw arguments to syslog when site.xcatdebugmode was set.
Redact the argument vector before the daemon joins it. Add
redact_password_args for this task. The routine masks the value of a
secret attribute in any argument, at the start or embedded after another
token. An embedded secret assignment masks to the end of the argument,
because a shell value may hold quotes and spaces. The routine allows
spaces around the operator. It accepts the "=", "+=", ",=", "^=", "!=",
"=~" and "!~" operators that chdef, nodech and node selection use. It
masks a password option value in each form that Getopt::Long accepts: a
separate argument, a compact short option, a bundle of short options
with the "?" help letter, a "+" option prefix, a single-letter option
with two dashes, a long option, a long option with an equals sign, and
an abbreviated long option. The long-name match runs first, so a long
option keeps its name and masks its value. A walk over each bundle then
finds the first secret letter, so the mask always starts at the option
and the result does not depend on hash order. The walk knows which
other letters of a command take a value, so a secret letter inside such
a value does not redact and the audit text stays correct. The walk also
knows which letters take an integer, because the z/VM cpu option
consumes only its signed digits and the parser then continues the
bundle into the password option. The value stops match letter case,
because a bundle keeps short options case sensitive and an unknown
capital letter does not absorb the rest. The mkvm secret match ignores
letter case, because the z/VM parser keeps the Getopt::Long default for
long names. The mkhwconn match keeps letter case, because -p
is the hardware control point and -P is the password. The routine knows
the password options of bmcdiscover, switchdiscover, mkhwconn, mkvm,
createvcluster, lsvcluster and rmvcluster, the rspconfig password
assignments, the mkvm clone pw= operand, and the positional password
operands of chvm. It masks the site.value argument of tabch and chtab
when a selector or a site.key assignment names snmpc. An exact short
option that takes a non-secret value stays visible, so the PPC mkvm -p
profile is not an abbreviation of --password. The dispatch trace builds
its text from the redacted vector.
Add snmpc, productkey, prodkey.key, tokenid and token.tokenid to the
secret list, with community and pdu.community. The secret list holds
only attributes that map to a secret column, so key and sshkeydir stay
visible.
redact_password keeps a second pass over the joined string. This pass
masks an embedded secret assignment to the end of the line, because the
argument boundaries are gone after the join.
The commands.log response classifier marks a response sensitive when the
request was redacted. The argument vector pass sets that signal, so a
secret whose name has no "passw" text still marks its response.
Extracts the three command-log response subs and drives them. It checks the
request classification, the redaction of a bare passwd value, a secret split
across callbacks in either order, the password-content fallback, that a benign
response is kept, and that the finalizer preserves an earlier command's response
on a shared connection. It fails hard if a sub cannot be extracted.
xcatd redacts the request in commands.log but appends the command response
verbatim. A command whose output holds a secret writes it in clear text.
Examples are tabdump passwd, gettab of a passwd column, and getcredentials.
Collect the response into a per-command buffer. Set a sensitive flag when the
command is getcredentials, an argument names a password, or the request was
redacted. When the command finishes, replace the whole buffer if the flag is
set or the buffer still holds password content, then append the buffer. A
connection can carry more than one command, so the buffer is finalized at the
next command's start and at the end of the connection.
The buffer holds the full response, so a secret split across several callbacks
is also redacted. A per-callback check cannot do this.
The word-content check is a fallback. The request classification is the main
signal. A secret with no password marker, such as the output of an xdsh cat of
a shadow file, is a pre-existing leak of the root-only log. It is out of scope.
Recovered from the lenovobuild branch. Reimplemented against master.
The test runs the real NodeRange code. It sends a ^ range that a
two-argument open would run as a command and checks the command does not
run. It also reads a real comment-only file to show the operator still
works.
The command assertion fails against the previous behavior.
The ^ noderange operator reads node names from a file. NodeRange opened
that file with a two-argument open. A two-argument open reads shell
metacharacters in the path, so a noderange such as ^"id|" ran a command.
xcatd expands a noderange while it processes a request, so the command
ran on the management node.
Use a three-argument open with an explicit read mode. The value is then
only ever a file name. The ^ operator keeps working: ^/tmp/nodes still
reads the file.
This fix was recovered from the lenovobuild branch. The original there
(commit for "Remove load from file in noderange support") removed the ^
operator. This keeps the documented operator and closes the command path
instead.
build-ubunturepo guarded builds with a single host-global, fail-fast lock at
/var/lock/xcatbld.lock (`flock -n`). Two builds on one host -- e.g. the devel and
stable Ubuntu CD lanes on xcat-master-ub -- therefore collided on that one lock and
the loser exited 1 with "Can't get lock ...", failing the whole pipeline, even though
each lane builds from its own checkout into its own DEST and they share nothing.
build-ubunturepo builds its packages in-place in its own source checkout (it rewrites
debian/changelog and debian/control, drops *.orig.tar.gz at the checkout root and runs
dpkg-buildpackage inside the package dirs), so the resource two concurrent builds
actually contend for is the checkout, not the host. Key the lock on the checkout path
($curdir): builds of the SAME checkout still fail-fast (they would corrupt each other
in-place), while builds of DISTINCT checkouts get distinct locks and run in parallel.
The lock file stays on the local /var/lock (reliable flock, unlike the NFS/virtiofs
checkout) and the source tree is left byte-pristine.
Add xCAT-test/unit/build_ubunturepo_lock.t, which extracts the lock block from the
script verbatim and asserts: the lock path is /var/lock/xcatbld-<hash-of-checkout>.lock
and deterministic per checkout; a second build of the same checkout fails fast; two
distinct checkouts acquire their locks concurrently.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Run redact_password rather than inspecting its source, so that a secret which
survives is a failure here rather than something the shape of the code hides.
Each supported way of writing an assignment is exercised, including the table
qualified form and spaces around the equals sign, together with the positional
flag that the per-command table handles.
The expected set is derived from Schema.pm rather than repeated, so an attribute
added there with a secret column fails this test instead of quietly reaching the
logs, and the test bails if that mapping cannot be parsed so it cannot pass
vacuously. Detail that is not secret is asserted to survive, since redacting it
would cost the log its usefulness without protecting anything.
Only syslog received the redacted arguments. The auditlog table was given the
raw string, so a password removed from syslog and from commands.log was still
written to the database, which persists and is readable by anything with access
to it.
Use the redacted text for both. This also covers the mkvm --password and -w
masking a few lines above, which the table did not have either.
redact_password only knew about bmcdiscover, mkhwconn and rspconfig, so setting
a password the ordinary way left it in the clear:
[Request] chdef node01 'bmcpassword=SEKRET'
Those secrets are carried as an assignment on whichever command happens to set
them, so match them by name rather than by command. The names are every
attribute Schema.pm maps to a password, passwd, authkey or privkey column, and
the columns themselves, since a table qualified assignment such as
passwd.password= is accepted too. An assignment may be written with spaces
around the equals sign and the value may contain spaces, so a quoted argument is
redacted to its closing quote.
Attributes such as key, which names a monitoring attribute, and sshkeydir, which
is a directory, are not secrets and are left readable. The existing per-command
table is kept for a flag like bmcdiscover -p, which carries its secret in the
following argument where there is no name to match.
The Debian upgrade path sends xcatconfig output to /var/log/xcat/upgrade.log the
same way the rpm one does, so a failed mknb is recorded and never seen.
Apply the same treatment: keep this run's output separately, repeat the failure
on the terminal, and leave the log itself unchanged.
The service node runs mknb directly rather than through xcatconfig, and nothing
looks at what it returned, so a failure leaves the node without a current
Genesis image and says nothing at all.
Check it and say so.
xcatconfig checks what mknb returned and logs the failure, but the upgrade
sends its output to /var/log/xcat/upgrade.log, so the line scrolls past and the
management node is left without a current Genesis image for no visible reason.
Repeat it on the terminal. The output of this run is kept separately from the
log so that a failure recorded by an earlier upgrade cannot raise the warning
again, since the log is appended to.
Assert that a node-targeted makedhcp maps its nodes to their service nodes and
skips the ones serving none of them, that regenerating the networks with -n stays
exempt because a dynamic range is not tied to a node, and that an unmapped
noderange still reaches every service node instead of none.
Also pin the two conditions the loop already had, so restricting the fan-out does
not quietly drop the self dispatch guards.
makedhcp on a noderange sends the request to every service node running a dhcp
server, whether or not it serves any of the named nodes. On a cluster with many
service nodes each one does the work and reports on nodes that are not its
responsibility, for example warning that a node it does not serve has no mac
address.
Map the named nodes to their service nodes and skip the rest. Regenerating the
networks with -n still reaches every dhcp server, since a dynamic range is not
tied to a node, and if none of the named nodes can be mapped the request goes to
all of them as before rather than risk leaving one out.
The node to service node mapping reuses getSNformattedhash, which the
disjointdhcps path in this same function already uses.
Co-authored-by: Jarrod Johnson <10814490+jjohnson42@users.noreply.github.com>
Assert that the xdcp subrequest names a username and that it is the arrayref
form, since the consumers index it as ->[0] and a bare string does not survive
that on the non-hierarchical path.
Also pin the two things this depends on: that xdsh still derives
DSH_FROM_USERID from the request username, and that updatenode still passes a
username on its own xdcp call, so the two callers cannot drift apart again.
syncfiles builds an xdcp subrequest with no username. xdcp takes its user from
-l, then from DSH_FROM_USERID, and otherwise leaves it unset, so the sync ends
up running as whatever identity xcatd happens to have rather than one the
request states.
That gap dates from 2c9bdf848, which removed the forceroot flag because any
caller could set it, and replaced it by passing the real username through.
updatenode was updated to do that; syncfiles was not. Its request comes from the
node, so there is no calling user to carry through, and root is what the sync
has to run as.
Name it explicitly, the same way updatenode does. This is a consistency and
hardening change: on a management node running as root the resulting identity is
the same either way, and no behavior difference is observable today.
Co-authored-by: Jarrod Johnson <10814490+jjohnson42@users.noreply.github.com>
Scan destiny.pm and packimage.pm for statements sitting after an unconditional
return, which is how the deprecated provisioning paths stayed in the tree after
they stopped running.
Also assert the behavior that has to survive the removal: the deprecated
nodeset states are still rejected, packimage still rejects -o, -p and -a, and a
missing image name is now reported as such rather than as a missing option that
would be rejected anyway.
packimage rejects -o, -p and -a before anything else runs, which leaves three
things below it that cannot be reached. The branch handling a missing image
name asks for -o and reports "Please specify a os version with the -o flag",
so passing the option is rejected and omitting it is reported as the option
being absent. The check for those options alongside an image name re-tests a
condition already rejected, and the block that built an image from them can no
longer be entered.
Report the missing image name instead, and drop the parts that cannot run.
Passing -o, -p or -a is rejected exactly as before, and packing a named image
is unchanged.
setdestiny reports the deprecated install, netboot and statelite states and
returns, and the legacy path that set nodetype.provmethod from the target sits
directly after that return. It has not run since the deprecation landed in
57aa41798, and it still reads like live code when following setdestiny.
Remove it. The deprecated states are still rejected exactly as before, and the
osimage path is untouched.
Exercise the node-selection block from preprocess_request and the lookup
adjustment from makeconfluentcfg directly, both extracted from the shipped
plugin, with a stand-in nodehm table.
Assert that an explicitly named node is configured whether it has no console
attributes or no nodehm row at all, that neither lookup can emit an empty node
name, and that the full table scan still skips console-less nodes so an entire
cluster is not swept into confluent. That last group is what separates this
from simply removing the skip, and it fails if the skip is dropped outright.
preprocess_request skips any node whose nodehm entry has neither cons nor
serialport. That is the right default while scanning the whole table, but it
also drops nodes the administrator named on the command line, leaving the
dispatched node list empty. makeconfluentcfg then falls back to reading every
node, so naming one console-less node quietly configures the whole cluster
instead of the node that was asked for.
A named node with no nodehm row at all is worse: both the selection here and
the second lookup in makeconfluentcfg produce an undefined entry, which reaches
confluent as an empty node name.
Apply the skip only to the full table scan, and carry the node name through
both lookups when the nodehm row is missing. Nodes without a console method are
already handled downstream, where makeconfluentcfg keeps explicitly named
entries and donodeent falls back to hardware management credentials.
Co-authored-by: Jarrod Johnson <10814490+jjohnson42@users.noreply.github.com>
Exercise the chain-advance block from nextdestiny directly, extracted from the
shipped plugin so the test cannot drift away from the code it covers.
Assert that repeatedly advancing a node whose remaining step is 'boot' leaves
it on 'boot' and is idempotent, and that the paths which must not change still
behave as before: an exhausted non-boot chain falls to 'standby', a chain with
steps left advances normally, and an empty currchain starts from the default.
When a chain has run down to a single remaining step, nextdestiny replaces that
step with 'standby'. For a completed provision the remaining step is 'boot',
which setdestiny writes into chain.currchain once an install or netboot destiny
is applied, so the replacement discards the destiny that boots the node.
Some installers advance the destiny more than once. On the extra advance the
node silently moves from 'boot' to 'standby' and no longer has a destiny that
boots the system it just installed.
Keep 'boot' when it is the remaining step, and leave every other exhausted
chain falling to 'standby' as before, so a finished install still does not
reinstall the node on its next boot.
Co-authored-by: Jarrod Johnson <10814490+jjohnson42@users.noreply.github.com>
Assert that the yum/dnf upgrade is restricted to the xcat-otherpkgs*
repositories in both the verbose echo and the command actually executed, and
that no unscoped upgrade remains.
Also pin the two facts the fix depends on: that the postscript still names its
repositories xcat-otherpkgs<index>, so the enablerepo glob keeps matching, and
that the install path stays unscoped, so package dependencies still resolve
against the full repository set.
The assertions count matches instead of using unlike(), so a failure reports a
count rather than dumping the whole postscript into the test output.
Daniel Hilst
Vinícius Ferrão
Vinícius Ferrão
Vinícius Ferrão