From c549b7e3305747e099f45272f14cea29dd2c1354 Mon Sep 17 00:00:00 2001 From: Your Name Date: Wed, 19 Aug 2026 03:14:47 +0000 Subject: [PATCH] fix(snmp): ask for the old community per device, not once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --old prompted once and tested that one string against every device. That was right when a single community was shared across all four. It is not right now: each device has its own predecessor, so one string proves nothing about the three it never belonged to, and there was no way to check a device whose old community differed from the rest. Each device that passed its current-community check is now asked separately. Enter skips one, because the usual case is checking the single device you just rotated. Skipping every device is an error rather than a silent pass — a run that checked nothing must not print "all SNMP targets verified". The prompt reads from /dev/tty explicitly: the loop's stdin is the inventory, so a bare `read` would have consumed a device row instead of waiting for input. The terminal requirement is unchanged, and each community still reaches net-snmp only through a 0600 snmp.conf that is removed on every exit path. Verified against all three branches: a wrong community reports `rejected`, a community the device still accepts is caught as `STILL ACCEPTED` and fails the run, and skipping everything refuses to claim success. Refs #9 Co-Authored-By: Claude Opus 5 --- docs/runbooks/rotate-snmp-community.md | 13 +++++--- scripts/snmp-verify.sh | 41 +++++++++++++++++++------- 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/docs/runbooks/rotate-snmp-community.md b/docs/runbooks/rotate-snmp-community.md index 5980b57..14557e9 100644 --- a/docs/runbooks/rotate-snmp-community.md +++ b/docs/runbooks/rotate-snmp-community.md @@ -207,10 +207,15 @@ require deleting the row rather than blanking the field — then: ./scripts/snmp-verify.sh --old ``` -It prompts for the old community without echoing it, and asserts each device now -refuses it. It requires a terminal and refuses a pipe on purpose — `echo "$old" | -...` would put the old community into your shell history, which is the leak this -tooling exists to close. Run it yourself; no script or agent can. +It asks for the old community **per device**, without echoing it, and asserts +each one now refuses it. Press Enter to skip a device — the usual case is +checking the one you just rotated, and a single string tested against all four +proves nothing about the three it never belonged to. It refuses to report +success if you skip everything. + +It requires a terminal and refuses a pipe on purpose — `echo "$old" | ...` would +put the old community into your shell history, which is the leak this tooling +exists to close. Run it yourself; no script or agent can. `--old` only checks devices that just passed their current-community check. SNMPv2c has no "wrong community" reply — a device that rejects you simply drops diff --git a/scripts/snmp-verify.sh b/scripts/snmp-verify.sh index 02920a8..6414c41 100755 --- a/scripts/snmp-verify.sh +++ b/scripts/snmp-verify.sh @@ -26,7 +26,7 @@ # Usage: # scripts/snmp-verify.sh every device, current community # scripts/snmp-verify.sh --device neo one device (name or IP) -# scripts/snmp-verify.sh --old also check the old one is refused +# scripts/snmp-verify.sh --old also check the old ones are refused # scripts/snmp-verify.sh --dry-run show the mapping; no decrypt, no packets set -euo pipefail @@ -230,18 +230,21 @@ done <<< "${INVENTORY}" # Old community # --------------------------------------------------------------------------- if ((CHECK_OLD)); then - # A terminal is required. Accepting the old community on stdin would let - # someone write `echo "$old" | scripts/snmp-verify.sh --old`, putting it into - # their shell history — the exact leak this script exists to close. - [[ -t 0 ]] || die "--old needs a terminal: it reads the old community without echoing it" - printf 'Old community (not echoed; written only to a 0600 file that is deleted on exit): ' >&2 - IFS= read -rs OLD_COMMUNITY - printf '\n' >&2 - [[ -n "${OLD_COMMUNITY}" ]] || die "no old community entered" + # A terminal is required. Accepting old communities on stdin would let someone + # write `echo "$old" | scripts/snmp-verify.sh --old`, putting them into their + # shell history — the exact leak this script exists to close. + [[ -t 0 ]] || die "--old needs a terminal: it reads the old communities without echoing them" head_ "Old community (must be refused)" - write_conf "${WORK}/old" "${OLD_COMMUNITY}" "the old community" + # Asked for per device, not once. A single prompt was right when one community + # was shared across all four; after a rotation each device has its own + # predecessor, and testing one string against every device proves nothing + # about the three it never belonged to. Enter is a skip, because the usual + # case is checking one device you just rotated. + printf ' Each device is asked separately. Press Enter to skip one.\n\n' >&2 + + asked=0 while IFS=$'\t' read -r ip auth device var; do [[ -n "${ip}" ]] || continue @@ -258,8 +261,22 @@ if ((CHECK_OLD)); then continue fi + # Read from the terminal explicitly: this loop's stdin is the inventory. + printf ' old community for %-10s (not echoed, Enter to skip): ' "${device}" >&2 + IFS= read -rs old_one < /dev/tty + printf '\n' >&2 + + if [[ -z "${old_one}" ]]; then + skip "$(printf '%-10s %-12s %s' "${device}" "${ip}" "not checked — no old community given")" + continue + fi + asked=$((asked + 1)) + + write_conf "${WORK}/old-${device}" "${old_one}" "the old community for ${device}" + old_one="" + # -r 0: a timeout is the expected outcome, so retrying only doubles the wait. - probe "${ip}" "${WORK}/old" 0 + probe "${ip}" "${WORK}/old-${device}" 0 case "${PROBE_STATUS}" in ok|nosuchobject) @@ -273,6 +290,8 @@ if ((CHECK_OLD)); then ;; esac done <<< "${INVENTORY}" + + ((asked > 0)) || die "no old community entered for any device — nothing was checked" fi printf '\n'