Audit remediation: security, robustness, and UX fixes #25

Merged
kleb merged 43 commits from fix/audit-remediation into main 2026-06-24 17:29:53 +02:00
Owner

Summary

Remediation of findings from an end-to-end audit of kInit. 42 focused commits
(one per issue), grouped below by theme. Each touches one concern and was
syntax-checked and ShellCheck-linted.

Security

  • restartSSHService validates sshd -t and uses reload (not restart) before
    applying SSH changes, preventing remote root lockout from a bad config edit.
  • No-IP-match no longer silently installs a fallback root SSH key; opt in via the
    new SSH_KEY_DEFAULT_DC.
  • Remote --setup-profile-url bootstrap fails closed without KINIT_SETUP_BOOTSTRAP_SHA256
    (explicit KINIT_SETUP_BOOTSTRAP_ALLOW_UNVERIFIED=1 + confirmation to override).
  • Optional minisign signature verification of checksums.sha256 (off by default).
  • SSH directives written to an /etc/ssh/sshd_config.d/ drop-in when supported;
    config files backed up before in-place edits.
  • validateSSHPublicKey rewritten to use ssh-keygen (accepts restricted keys,
    rejects environment=/malformed); validateCronLine rejects newline/||/unescaped %.
  • Fisher and Catppuccin pinned to fixed refs instead of upstream default branches.

Reliability

  • Modules use set -Eeuo pipefail so the clean ERR trap fires on nested failures.
  • FISH_ALIAS_* guarded against set -u; appendToFile ensures a trailing newline.
  • Zabbix apt.sh reports a collection failure instead of all-zeros when apt fails.

Behavior / defaults

  • Daily journal vacuum default 1s -> 7d; Docker prune defaults to dangling-only
    (CLEAN_DOCKER_PRUNE_ALL opt-in); non-fatal warning on EOL distros.

UX / docs

  • ANSI suppressed on non-TTY output / honors NO_COLOR (kInit.sh, utils.sh, analyze.sh);
    errors -> stderr; help documents KINIT_* env vars.
  • README SSH wording corrected; sanitized *.vars.example templates; uninstall.sh;
    CHANGELOG + manifest version header; data-driven module loop.

Validation

  • ShellCheck clean at warning+ level (only info-level SC2059/SC1091); bash -n on all scripts.
  • sha256sum -c checksums.sha256 passes; embedded bootstrap hashes regenerated.
  • Live end-to-end run on an Incus Debian 12 (bookworm) container: full --analyze
    install completed, all 4 modules executed, fixes confirmed on-box
    (journal 7d, no-key-on-no-match skip, batched apt, hardened crons, fish shell,
    zabbix-agent2 active). Two issues the live run surfaced (analyze.sh color
    suppression + no-match SSH check) are fixed and re-verified.

Notes / deferred

  • minisign signing requires a maintainer-held private key (paste the public key
    into KINIT_MINISIGN_PUBKEY to enable).
  • Live *.vars kept committed (they are the served kleb.sh deployment config);
    sanitized templates added alongside.
  • CI workflow and logging-hot-path date(1) micro-optimization intentionally deferred.
## Summary Remediation of findings from an end-to-end audit of kInit. 42 focused commits (one per issue), grouped below by theme. Each touches one concern and was syntax-checked and ShellCheck-linted. ## Security - `restartSSHService` validates `sshd -t` and uses `reload` (not `restart`) before applying SSH changes, preventing remote root lockout from a bad config edit. - No-IP-match no longer silently installs a fallback root SSH key; opt in via the new `SSH_KEY_DEFAULT_DC`. - Remote `--setup-profile-url` bootstrap fails closed without `KINIT_SETUP_BOOTSTRAP_SHA256` (explicit `KINIT_SETUP_BOOTSTRAP_ALLOW_UNVERIFIED=1` + confirmation to override). - Optional minisign signature verification of `checksums.sha256` (off by default). - SSH directives written to an `/etc/ssh/sshd_config.d/` drop-in when supported; config files backed up before in-place edits. - `validateSSHPublicKey` rewritten to use `ssh-keygen` (accepts restricted keys, rejects `environment=`/malformed); `validateCronLine` rejects newline/`||`/unescaped `%`. - Fisher and Catppuccin pinned to fixed refs instead of upstream default branches. ## Reliability - Modules use `set -Eeuo pipefail` so the `clean` ERR trap fires on nested failures. - `FISH_ALIAS_*` guarded against `set -u`; `appendToFile` ensures a trailing newline. - Zabbix `apt.sh` reports a collection failure instead of all-zeros when apt fails. ## Behavior / defaults - Daily journal vacuum default `1s` -> `7d`; Docker prune defaults to dangling-only (`CLEAN_DOCKER_PRUNE_ALL` opt-in); non-fatal warning on EOL distros. ## UX / docs - ANSI suppressed on non-TTY output / honors `NO_COLOR` (kInit.sh, utils.sh, analyze.sh); errors -> stderr; `help` documents `KINIT_*` env vars. - README SSH wording corrected; sanitized `*.vars.example` templates; `uninstall.sh`; CHANGELOG + manifest version header; data-driven module loop. ## Validation - ShellCheck clean at warning+ level (only info-level SC2059/SC1091); `bash -n` on all scripts. - `sha256sum -c checksums.sha256` passes; embedded bootstrap hashes regenerated. - Live end-to-end run on an Incus Debian 12 (bookworm) container: full `--analyze` install completed, all 4 modules executed, fixes confirmed on-box (journal 7d, no-key-on-no-match skip, batched apt, hardened crons, fish shell, zabbix-agent2 active). Two issues the live run surfaced (analyze.sh color suppression + no-match SSH check) are fixed and re-verified. ## Notes / deferred - minisign signing requires a maintainer-held private key (paste the public key into `KINIT_MINISIGN_PUBKEY` to enable). - Live `*.vars` kept committed (they are the served kleb.sh deployment config); sanitized templates added alongside. - CI workflow and logging-hot-path `date(1)` micro-optimization intentionally deferred.
kleb added 41 commits 2026-06-23 17:51:39 +02:00
restartSSHService now runs 'sshd -t' before touching the daemon and aborts
the reload on failure, preventing an unrecoverable remote root lockout from a
malformed sshd_config edit. Switches restart -> reload so established SSH
sessions (including the operator's own) survive the change. Call sites in
system.sh surface a failed validation without aborting the whole module.
Previously a host whose IP matched no SSH_IP_RANGES entry silently fell back to
SSH_KEY_DCS[0] and authorized that key for root login. Now a non-matching host
skips SSH key installation (and the gated PermitRootLogin change) unless the
operator explicitly opts into a default via the new SSH_KEY_DEFAULT_DC, closing
the path where any non-LAN server would authorize the default datacenter key.
The --setup-profile-url bootstrap was the only downloaded asset executed as
root with no integrity check unless the caller opted in. It now fails closed:
without KINIT_SETUP_BOOTSTRAP_SHA256 it refuses to run, and running it
unverified requires an explicit KINIT_SETUP_BOOTSTRAP_ALLOW_UNVERIFIED=1 plus an
interactive 'I UNDERSTAND THE RISKS' confirmation when a TTY is present,
mirroring the existing KINIT_SKIP_CHECKSUMS gate.
The daily clean.sh ran 'journalctl --rotate' then '--vacuum-time=1s', which
deleted essentially the entire systemd journal (auth/sshd history, crashes, OOM,
audit trail) on every run. The shipped default and the in-script fallback are
now 7d so cleanup reclaims space without destroying operational/forensic logs;
README documents the change and the risk of lowering it.
Hash pinning proves files match kInit.sh but cannot attest that kInit.sh and its
embedded hashes are authentic (checksum.sh self-blesses whatever exists). This
adds an opt-in authenticity layer: kInit verifies a detached minisign signature
over checksums.sha256 when KINIT_MINISIGN_PUBKEY is set (fail-closed), and
checksum.sh --sign produces checksums.sha256.minisig. Disabled by default so
existing deployments are unaffected; documented in the README Security section.
Editing the monolithic /etc/ssh/sshd_config could be silently overridden by an
earlier Include'd drop-in (OpenSSH uses the first value seen). The new
setSshdOption helper writes PermitRootLogin/AuthorizedKeysFile/PrintMotd/
PrintLastLog to /etc/ssh/sshd_config.d/00-kinit.conf (read first, so it wins)
when the main config includes that directory, and is trivially reversible by
removing one file. Falls back to in-place editing on older layouts.
Adds backupFile, which makes a one-time timestamped 0600 copy (file.kinit-bak-*)
before kInit edits a file in place, preserving the pristine pre-kInit version.
Wired into the sshd_config in-place fallback and the /etc/pam.d/sshd edit, so the
README's 'restore backed-up configuration files' guidance is now actionable.
The previous validator took awk field 1 as the key type, so any key carrying an
authorized_keys options prefix (e.g. restrict,command=) was wrongly rejected
while the dangerous-option warning was unreachable, and a structurally invalid
key body could pass. It now validates structurally/cryptographically via
ssh-keygen -l (correctly handling an options prefix), rejects environment= and
ssh-dss outright, warns on RSA and forced/restrictive options, and keeps a
type-aware fallback when ssh-keygen is absent. Verified against plain, restricted,
environment=, garbage, dss, and empty inputs.
fisher install jorgebucaran/fisher and catppuccin/fish resolved each repo's
moving default branch and ran the fetched code as root, undermining the careful
FISHER_SHA256 pin. Now pinned to jorgebucaran/fisher@4.4.5 (matching the
checksum-validated loader) and catppuccin/fish@5fc5ae9 (a fixed commit), both
verified to resolve as GitHub archives.
The four modules used 'trap clean ERR' without 'set -o errtrace', so an ERR trap
is not inherited by prepare()/execute() and clean() was skipped when a bare
command failed inside them (recordModuleResult and per-module cleanup never ran).
Switching to 'set -Eeuo pipefail' makes the trap inherited; verified clean runs
exactly once with the correct exit code and does not recurse.
system.sh referenced $FISH_ALIAS_BAT/CAT/LL/MD/RM/CP/MV/PC without a default, so
under 'set -u' a customized system.vars that omits any one key crashed the System
module with 'unbound variable' after fish/Fisher were already installed. Now uses
the ${VAR:-} form, matching analyze.sh.
logInfo/logError/updateProgress and the kInit banner emitted raw ANSI escapes
unconditionally, polluting piped/redirected/CI output. Both color tables now
blank themselves when NO_COLOR is set or stdout is not a terminal (with
FORCE_COLOR/CLICOLOR_FORCE overrides), per the NO_COLOR standard and clig.dev.
execute() repeated a ~16-line download+run block per module and showSummary()
repeated the status cascade four times, so adding a module meant editing several
places in lockstep. Introduces a runModule helper and iterates modules in both
functions, preserving the percentages, continue-on-error semantics, and summary
output while removing the duplication.
software.sh installed a current Zabbix agent on any codename with a vendored
package, including EOL releases (xenial/bionic/buster) and Ubuntu focal (standard
support ended). It now emits a non-fatal warning on those releases so operators
are not silently provisioning monitoring onto an unpatched OS.
The cron validator was a shallow blocklist that accepted embedded newlines (which
install a second crontab line), || chaining, and an unescaped % (cron turns it
into a newline + stdin). These are now rejected while the legitimately-used
single pipe and > redirection (APT and MOTD crons) still pass; verified against
the shipped crons and several injection patterns.
appendToFile used 'echo >>' with no guard, so appending a directive to a file
lacking a final newline (legal for sshd_config) concatenated it onto the previous
line, producing an invalid directive. It now adds a newline first when the target
file does not already end in one.
logError/logWarning and the fatal kInit.sh diagnostics (root check, bootstrap
validation, checksum/download/signature failures) wrote to stdout, so capturing
stdout for parsing interleaved diagnostics with real output and redirecting it
swallowed error context. They now go to stderr; logInfo/progress stay on stdout.
clean.sh ran 'docker image prune -a' daily, deleting all unused tagged images
(forcing full re-pulls) rather than just dangling layers. The default is now
dangling-only; the aggressive -a behavior is opt-in via the new
CLEAN_DOCKER_PRUNE_ALL, wired through utilities.vars and /etc/default/kinit-clean.
Also aligns the in-code journal-retention fallback to 7d.
The 'apt update' after installing zabbix.deb looked redundant against kInit's own
earlier refresh, but zabbix.deb is the repo-config package and the index must be
refreshed before the agent is visible. Commented as intentional rather than
removed, since dropping it would break the install.
The Zabbix APT metric script captured apt-get output and counted matches, so a
failed/locked apt run produced all-zeros with exit 0 and Zabbix recorded 'no
pending updates' instead of a collection failure, masking unpatched hosts. It now
checks the apt-get exit status and exits non-zero without emitting metrics on
failure, so Zabbix sees nodata. Strict mode deliberately omitted (grep -c).
bootstrapDownload (utils.sh + manifest, the trust-critical first hop) lacked the
cache-busting and retry of the main download(). Adds wget --no-cache / curl
no-cache headers + --retry 2 so a stale proxy cache cannot serve an old copy and
a transient failure is retried, matching the hardened module download path.
The verification report interpolated $LANGUAGE and $TIMEZONE directly into the
printf format string, so a value containing a '%' would be misparsed as a format
directive. These now use %s with the value passed as an argument.
The README said the installer 'enables root SSH login' / 'sets PermitRootLogin to
allow root login' when the default is prohibit-password (key-only). Updated to
describe the real default, document SSH_PERMIT_ROOT_LOGIN (incl. the CIS 'no'
posture), the new SSH_KEY_DEFAULT_DC no-match behavior, and the drop-in/backup/
sshd -t handling.
skipBootstrapValidation was declared and read but never assigned, implying a
nonexistent --skip-bootstrap-validation flag; removed it (the KINIT_SKIP_
BOOTSTRAP_VALIDATION env var remains the real control). help() now documents the
KINIT_* environment variables and warns that --setup-passphrase is visible in ps
and shell history (prefer the env var).
The KINIT_SKIP_CHECKSUMS confirmation called 'read' with no TTY check, so under a
piped invocation it failed with an opaque 'Checksum bypass cancelled'. It now
detects a non-interactive stdin and explains to use the two-step form. Cancel
message also routed to stderr.
AGENTS.md tells contributors to follow .editorconfig, but it was listed in
.gitignore alongside editor-artifact ignores, so a fresh clone never received the
canonical style file. Un-ignored and committed it.
installer/system/files/hetzner-console was referenced by no code path (only the
checksum manifest) and is byte-equivalent to hetzner-robot, so it was dead config
publishing a duplicate trusted key. Its manifest entry is dropped on the next
checksum regeneration.
The comment told maintainers to fetch fisher.fish from the moving 'main' branch,
but FISHER_SHA256 matches the pinned 4.4.5 tag that system.sh uses, so following
the comment would produce a non-matching hash. Comment now points at the pinned
tag and the checksum.sh --update-fisher helper.
The bundled zabbix.deb files are the zabbix-release repository-configuration
package, not the agent, but error strings claimed the 'agent package' failed and
the README didn't mention the indirection. Corrected the download/install error
messages, added explanatory comments, and documented in the README that the agent
itself is pulled from the official Zabbix apt repo.
checksum.sh now writes a '# kInit <version>' header into checksums.sha256 (the
loader and sha256sum -c both ignore '#' lines) so an installed host can be traced
to a release, and CHANGELOG.md records the remediation set plus version-pinning
guidance. CHANGELOG.md is excluded from the manifest like README.md.
Provides scripted reversal: restores kInit's config backups, removes the SSH
drop-in, kInit-installed files, and cron jobs, and reloads SSH only if it still
validates. Conservatively leaves packages and authorized keys to the operator.
README's uninstall section now points at it; the script is excluded from the
manifest like kInit.sh.
writeRunLog persisted log messages verbatim to /var/log/kinit, unlike the config
summary which redacts. Messages now pass through redactSensitive before being
written, so a token/passphrase embedded in a logged message is masked on disk.
Notes that SSH_IP_RANGES/ZABBIX_DCS are matched as literal string prefixes (not
CIDR) and should end at a delimiter to avoid overlap, and documents that
replaceRegex's pattern argument must be a trusted literal (config-derived data
belongs only in the escaped replacement). Both are latent-injection hardening
notes; no current call site violates them.
Clarifies that the apt update counts are cached for 1h while the banner re-renders
every 15 min, so the figures can lag by up to ~60 min by design and should be
read as at-a-glance rather than live.
Provides installer/system/system.vars.example and
installer/software/software.vars.example with placeholder keys, IP ranges, and
Zabbix server so forks have a clean starting point that does not embed real
infrastructure. Live *.vars remain committed (they are the served deployment
config). README points forkers at the templates.
The four modules duplicated a near-identical clean() (remove vars/self, record
result, report progress). Adds a finishModule helper to utils.sh and reduces each
clean() to a thin wrapper, preserving the software-specific zabbix.deb removal and
the miscellaneous 'tasks complete' message.
installPackage installed apt packages one at a time, re-running dependency
resolution and dpkg triggers per package (~19 for the System module). It now
collects names and installs required packages in one transaction (all-or-fail, as
before) and optional packages in one best-effort transaction with a per-package
fallback so a single bad optional package does not drop the rest. .deb files are
still handled individually.
Recompute checksums.sha256 (with version header) over the remediated tree and
update EXPECTED_UTILS_SHA256 / EXPECTED_CHECKSUMS_SHA256 in kInit.sh. Drops the
removed hetzner-console entry, adds the *.vars.example templates, and reflects all
audit fixes. Verified with 'sha256sum -c checksums.sha256' (all OK).
Live testing showed analyze.sh emitting raw ANSI in --analyze (non-TTY) output
while kInit's own summary was clean: the #22 color suppression covered kInit.sh
and utils.sh but not analyze.sh's separate color table. Applies the same guard.
After the no-IP-match hard stop, a non-matching host installs no SSH key, but
analyze.sh still counted a missing authorized_keys as a failure. It now mirrors
system.sh's match logic and reports a neutral [~] skip when no range matched (and
no SSH_KEY_DEFAULT_DC is set), only failing when a key was actually expected.
Collaborator

kReview review

Verdict: no findings

No findings to address in the reviewed diff.

Overall risk is low; no actionable correctness or security defects are evident from the supplied diff and context.

Reviewed by kReview at 34fa7f855b. This comment is conservative and based only on the PR diff, metadata, and supplied repository context.

Est. cost ~$1.03 total (200.9k in / 1.9k out) · this run ~$0.34 (67.3k in / 555 out) / gpt-5.5.

<!-- codex-forgejo-review --> <!-- codex-forgejo-review-head:34fa7f855b666ba0a75dc3528fb43ef89f8a537e --> ## kReview review **Verdict:** no findings No findings to address in the reviewed diff. Overall risk is low; no actionable correctness or security defects are evident from the supplied diff and context. _Reviewed by kReview at `34fa7f855b`. This comment is conservative and based only on the PR diff, metadata, and supplied repository context._ _Est. cost ~$1.03 total (200.9k in / 1.9k out) · this run ~$0.34 (67.3k in / 555 out) / gpt-5.5._ <!-- codex-forgejo-review-state:eyJoZWFkU2hhIjoiMzRmYTdmODU1YjY2NmJhMGE3NWRjMzUyOGZiNDNlZjg5ZjhhNTM3ZSIsInN1bW1hcnkiOiJPdmVyYWxsIHJpc2sgaXMgbG93OyBubyBhY3Rpb25hYmxlIGNvcnJlY3RuZXNzIG9yIHNlY3VyaXR5IGRlZmVjdHMgYXJlIGV2aWRlbnQgZnJvbSB0aGUgc3VwcGxpZWQgZGlmZiBhbmQgY29udGV4dC4iLCJmaW5kaW5ncyI6W10sImN1bXVsYXRpdmVDb3N0Ijp7InVzZCI6MS4wMjc2NTgsImlucHV0VG9rZW5zIjoyMDA5MjYsIm91dHB1dFRva2VucyI6MTg2MiwibW9kZWwiOiJncHQtNS41In19 -->
The KINIT_SETUP_BOOTSTRAP_ALLOW_UNVERIFIED=1 path only confirmed when stdin was a
TTY and otherwise fell through to executing the unpinned remote bootstrap as root,
so a non-interactive run could run it on the env var alone. It now refuses
non-interactively (return 1) unless a TTY confirmation is given, matching the
KINIT_SKIP_CHECKSUMS gate.
kleb merged commit bc1a543516 into main 2026-06-24 17:29:53 +02:00
kleb deleted branch fix/audit-remediation 2026-06-24 17:29:53 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kleb/kInit!25
No description provided.