Read the refusal instead of the status of the thing that refused
The two negative checks piped ssh into `grep -q refused`. Under pipefail a pipeline takes the rightmost non-zero status, and a refusal is dejima-guard-shell exiting 64 relayed through ssh - so grep matched, the pipeline still came back 64, and a correctly restricted devbox was reported as having no restriction at all. The positive checks were unaffected because they have no pipe. Output is captured and tested for both halves of a refusal: the message, which is what the runbook tells an operator to look for, and a non-zero status. Neither alone separates a refusal from an ordinary failure, since cat /etc/shadow on an unrestricted key also fails. A failing check now prints what the devbox actually said. The evidence that this check was wrong was in output it had already captured and discarded. The ssh stub could not have caught either this or the IdentitiesOnly bug before it: it shifted once and ran the key's path as the command, so all four checks ran garbage and the phase had no coverage. It now answers as a devbox with the forced command in place, and the new tests fail against both previous versions.
This commit is contained in:
@@ -151,11 +151,31 @@ for c in "dejima-devbox-audit reach" "dejima-devbox-audit drift --state"; do
|
||||
fails=1
|
||||
fi
|
||||
done
|
||||
|
||||
# Captured, not piped. `sshg "$c" | grep -q refused` looks right and is not:
|
||||
# under `set -o pipefail` the pipeline takes the rightmost NON-ZERO status, and
|
||||
# a refusal is exit 64 relayed through ssh - so grep matched, the pipeline still
|
||||
# came back 64, and a working restriction was reported as absent. The louder
|
||||
# direction of that mistake, but a false alarm about the one control standing
|
||||
# between a stolen key and this box is not a cheap one to raise.
|
||||
#
|
||||
# Both halves of the refusal are checked: the message, which is what the runbook
|
||||
# tells an operator to look for, and a non-zero status. `cat /etc/shadow` on an
|
||||
# unrestricted key fails too - with "Permission denied" and no "refused" - so
|
||||
# neither alone separates a refusal from an ordinary failure.
|
||||
for c in "cat /etc/shadow" "dejima-devbox-audit drift --accept"; do
|
||||
if sshg "$c" | grep -q refused; then
|
||||
out=""; rc=0
|
||||
out=$(sshg "$c") || rc=$?
|
||||
if [ "$rc" -ne 0 ] && printf '%s' "$out" | grep -q refused; then
|
||||
say " refused $c"
|
||||
else
|
||||
say " NOT REFUSED $c"
|
||||
# What came back, rather than only the conclusion drawn from it. This
|
||||
# check reported a working restriction as missing for one release, and
|
||||
# the evidence that it was wrong was in the output it had already
|
||||
# captured and discarded.
|
||||
say " the devbox said, exit $rc:"
|
||||
printf '%s\n' "${out:-<nothing>}" | sed 's/^/ > /' >&2
|
||||
say " The forced command is not in force for this key."
|
||||
say " Check the line in ~/.ssh/authorized_keys on the"
|
||||
say " devbox begins command=\"$SHELL_PATH\", and that"
|
||||
|
||||
+58
-4
@@ -11,9 +11,17 @@
|
||||
# key is live. So this program exists, and what it has to get right is that the
|
||||
# line it writes carries the forced command, every time.
|
||||
#
|
||||
# `ssh` is stubbed with something that runs the remote script locally, so what
|
||||
# is under test is the script this sends rather than whether this machine can
|
||||
# reach a devbox.
|
||||
# `ssh` is stubbed with a devbox: it runs the install script locally, and answers
|
||||
# the four verification commands the way a box with the forced command in place
|
||||
# answers them - two that work, two refused on stderr with exit 64.
|
||||
#
|
||||
# THE EXIT STATUS IS THE POINT. The stub this replaced did `shift; exec bash -c
|
||||
# "$1"`, which for the verification calls handed bash the key's path rather than
|
||||
# the command, so all four checks ran garbage and the phase was never tested at
|
||||
# all. Two bugs shipped through it, both of which a stub that relayed a status
|
||||
# would have caught: the missing IdentitiesOnly, and a `| grep -q refused`
|
||||
# pipeline that under pipefail returned the refusal's own exit 64 and so read a
|
||||
# working restriction as an absent one.
|
||||
|
||||
setup() {
|
||||
REPO="$(cd "$BATS_TEST_DIRNAME/.." && pwd)"
|
||||
@@ -21,7 +29,20 @@ setup() {
|
||||
export HOME="$BATS_TEST_TMPDIR/home"
|
||||
STUB="$BATS_TEST_TMPDIR/bin"
|
||||
mkdir -p "$HOME" "$STUB"
|
||||
printf '#!/bin/bash\nshift\nexec bash -c "$1"\n' > "$STUB/ssh"
|
||||
cat > "$STUB/ssh" <<'STUBEOF'
|
||||
#!/bin/bash
|
||||
cmd="${!#}" # last argument, whatever options preceded it
|
||||
case "$cmd" in
|
||||
*authorized_keys*) # the install script, which this runs for real
|
||||
exec bash -c "$cmd" ;;
|
||||
"dejima-devbox-audit reach")
|
||||
echo "PASS"; exit 0 ;;
|
||||
"dejima-devbox-audit drift --state")
|
||||
echo "FILE /etc/ssh/sshd_config a1b2"; exit 0 ;;
|
||||
*) # dejima-guard-shell, verbatim
|
||||
echo "dejima-guard-shell: refused: $cmd" >&2; exit 64 ;;
|
||||
esac
|
||||
STUBEOF
|
||||
chmod +x "$STUB/ssh"
|
||||
ssh-keygen -q -t ed25519 -N '' -f "$BATS_TEST_TMPDIR/k" -C guard
|
||||
PUB="$BATS_TEST_TMPDIR/k.pub"
|
||||
@@ -82,6 +103,39 @@ authk() { cat "$HOME/.ssh/authorized_keys" 2>/dev/null || true; }
|
||||
[ ! -e "$HOME/.ssh/authorized_keys" ]
|
||||
}
|
||||
|
||||
@test "a devbox with the restriction in force passes all four checks" {
|
||||
# The end-to-end shape, against a stub that answers exactly as the real
|
||||
# forced command does. This failed on a correctly restricted devbox: the
|
||||
# refusal's exit 64 came back through the pipeline and outvoted grep's
|
||||
# match, so both refusals were reported as the restriction being absent.
|
||||
run_install
|
||||
[ "$status" -eq 0 ]
|
||||
[[ "$output" == *"permitted dejima-devbox-audit reach"* ]]
|
||||
[[ "$output" == *"permitted dejima-devbox-audit drift --state"* ]]
|
||||
[[ "$output" == *"refused cat /etc/shadow"* ]]
|
||||
[[ "$output" == *"refused dejima-devbox-audit drift --accept"* ]]
|
||||
[[ "$output" != *"NOT REFUSED"* ]]
|
||||
}
|
||||
|
||||
@test "a devbox that runs what it should refuse fails the check" {
|
||||
# The direction that matters. A stub with no forced command answers
|
||||
# everything successfully, and this must not call that restricted.
|
||||
printf '#!/bin/bash\ncmd="${!#}"\ncase "$cmd" in *authorized_keys*) exec bash -c "$cmd";; esac\nexit 0\n' > "$STUB/ssh"
|
||||
chmod +x "$STUB/ssh"
|
||||
run_install
|
||||
[ "$status" -ne 0 ]
|
||||
[[ "$output" == *"NOT REFUSED"* ]]
|
||||
}
|
||||
|
||||
@test "a check that fails shows what came back, not only its conclusion" {
|
||||
# This reported a working restriction as missing for one release, and the
|
||||
# evidence it was wrong was in output it had captured and thrown away.
|
||||
printf '#!/bin/bash\ncmd="${!#}"\ncase "$cmd" in *authorized_keys*) exec bash -c "$cmd";; esac\necho "root:x:0:0"\nexit 0\n' > "$STUB/ssh"
|
||||
chmod +x "$STUB/ssh"
|
||||
run_install
|
||||
[[ "$output" == *"root:x:0:0"* ]]
|
||||
}
|
||||
|
||||
@test "the verification offers only the guard's key" {
|
||||
# Without IdentitiesOnly it checks whichever key ssh happens to use, which
|
||||
# on the operator's own machine is usually their own - so all four checks
|
||||
|
||||
Reference in New Issue
Block a user