Skip to content

Security hardening: package verification, service privileges, credential files, input validation - #1

Merged
nv0o-hrv merged 12 commits into
mainfrom
claude/youthful-ramanujan-ijtp0x
Oct 12, 2026
Merged

nv0o-hrv merged 12 commits into
mainfrom
claude/youthful-ramanujan-ijtp0x

Conversation

@EmDecay

@EmDecay EmDecay commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

What changed

Fixes from a repository-wide security review. Each item is its own commit so it can be dropped independently.

High

  • packet/winlink: the Pat .deb was downloaded to a predictable /tmp path and installed as root with no verification. get_url skips an existing destination, so any local account could pre-create the file and have it installed as root. The package now goes into the root-owned HamStack package cache and is verified against per-architecture SHA-256 digests (hamstack_winlink_package_checksums, enforced by hamstack_winlink_require_checksum).
  • services/cloudlog: application/config (where the installer writes database.php) was recursively forced to o=rX on every run. It is now u=rwX,g=rwX,o= root:www-data.
  • digital/multimon-ng: the managed decoder service ran as root while parsing over-the-air data. It now runs as a dedicated hamstack-multimon system account (audio/plugdev) under a sandboxed systemd unit; the log file is owned by that account.
  • digital/selfie_station: the service defaulted to the hamstack automation account (passwordless sudo, SSH trust) with keyboard-capture and PTT groups and ownership of its own code. It now runs as a dedicated hamstack-selfie nologin account, the install tree and script are root-owned, writes are limited to the data directory, and the unit is sandboxed. Numeric inventory values are cast before being rendered into Python source.

Medium

  • Download verification: JS8Call AppImages ship with upstream digests (required by default); GridTracker2, Graywolf, CHIRP wheel, NetLogger, and LinBPQ accept sha256: checksums; all download URLs must be HTTPS. GridTracker2 now caches by upstream filename so a version bump is not satisfied by a stale cached file. id -gn arguments are quoted in js8call/chirp/kiosk.
  • network/openwrt: Wi-Fi, radio, route, and firewall-zone identifiers (and forward_to targets) are validated as UCI-safe before being rendered into the root shell script applied via raw. Only LAN names were checked before.
  • network/routeros: hamstack_routeros_api_validate_certs defaults to true. playbooks/network.yml sets become: false so controller-side API tasks no longer run under sudo.
  • infrastructure/technitium-dns: admin password floor raised from 8 to 12 characters.
  • voice/allstar / voice/wpsd: credential-bearing files no longer leave Ansible *~ backup copies by default (rpt_http_registrations.conf; new hamstack_wpsd_config_backups: false). The WPSD pass-through loops now use no_log.
  • digital/ft8web: the role referenced nginx-default.conf.j2, which did not exist, so the role could not deploy. Added a minimal template, and scripts/qa.py now fails when a role task references a template/file the role does not ship.

CI (last two commits, separable)

  • The "Repository QA" workflow has been red since the initial commit: yamllint failed on a list-indentation error in roles/common/defaults/main.yml, so the ansible-lint and syntax-check steps never ran. Fixed the indentation, added explicit become: true to the seven tasks that set become_user (flagged by partial-become), and moved var-naming[no-role-prefix] and yaml[line-length] to the .ansible-lint warn list. The hamstack_* namespace is intentional per AGENTS.md, and .yamllint already treats line length as a warning. Drop the last commit if you would rather handle the lint policy separately.

Docs: role READMEs, docs/configuration.md, docs/network-devices.md, CHANGELOG.md, regenerated docs/variables.md.

Why

Review findings: local privilege escalation via an attacker-plantable package, database credentials readable by every local account, RF-facing and camera/PTT services running with root or sudo-capable identities, unverified root-installed binaries, command injection through unvalidated router inventory fields, API credentials sent without TLS verification, and secret copies left behind by Ansible backups.

Intentionally not changed in this PR (deferred by maintainer decision): default published ports and bind addresses, pinning of upstream source refs (cloudlog master, qsstv main, dcdash main), container image tags, and pysstv version pinning.

Validation

  • python3 scripts/qa.py
  • yamllint . (0 errors; remaining line-length warnings are pre-existing)
  • ansible-lint (0 failures on the basic profile with the updated config)
  • All six playbooks pass ansible-playbook --syntax-check against inventory/example
  • A second Ansible run was checked for unexpected changes when live hardware/software was exercised

Tested hardware/OS and commands used: no hardware or live host was exercised. Validation was repository QA, lint, syntax check, and offline rendering of the changed templates (the rendered selfie_station.py compiles). The multimon-ng and Selfie Station service-account changes in particular need a real Pi/SDR/camera run to confirm device access under the new accounts and ProtectSystem=strict.

Checksums: the Pat 1.0.0 and JS8Call 3.0.3 digests were computed from the GitHub release assets as downloaded on 2026-10-12 through this session's proxy. Please independently verify them against the upstream release pages before merging.

Release/user impact

  • hamstack_selfie_station_user default changes from hamstack to hamstack-selfie; existing data directories are re-owned on the next run and old files remain readable. Set hamstack_selfie_station_manage_user: false to keep an operator-managed account.
  • hamstack_routeros_api_validate_certs now defaults to true; routers with self-signed API-SSL certificates need the CA trusted on the controller or an explicit false.
  • Technitium admin passwords shorter than 12 characters now fail validation.
  • JS8Call and Pat installs fail if the configured version has no matching digest unless *_require_checksum: false is set.
  • Non-HTTPS download URLs are rejected for GridTracker, Graywolf, LinBPQ, NetLogger, CHIRP, JS8Call.
  • WPSD no longer writes Ansible backups of its config files unless hamstack_wpsd_config_backups: true.
  • New variables: hamstack_winlink_package_checksums, hamstack_winlink_require_checksum, hamstack_winlink_package_cache_dir, hamstack_multimon_ng_{user,group,manage_user,user_groups}, hamstack_selfie_station_{group,manage_user,user_groups}, hamstack_gridtracker_package_checksum, hamstack_js8call_{x86_64,aarch64}_checksum, hamstack_js8call_require_checksum, hamstack_chirp_wheel_checksum, hamstack_graywolf_package_checksum, hamstack_netlogger_package_checksum, hamstack_wpsd_config_backups.

Documentation and secrets

  • Public variable changes are reflected in examples/docs and docs/variables.md is regenerated
  • No real credentials, private inventory, event-only secrets, or sensitive artifacts are included
  • Upstream-derived code/configuration retains required attribution and licensing information

🤖 Generated with Claude Code

https://claude.ai/code/session_01LUNnYzucjok597pz3cUK2K

claude added 10 commits October 12, 2026 21:56
The Pat .deb was downloaded to a predictable path under /tmp and installed
as root without verification. Because get_url skips an existing destination,
any local account could pre-create the file and have its contents installed
with root privileges on the next run.

Download into the root-owned HamStack package cache, record the upstream
SHA-256 digests for the default release per architecture, and refuse to
install when no digest is known unless the operator opts out explicitly.

Co-Authored-By: Claude Mythos 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LUNnYzucjok597pz3cUK2K
The recursive permission task re-applied o=rX to application/config on
every run, which includes database.php and the database password written
by the Cloudlog installer. Manage that directory separately with no access
for other local accounts.

Co-Authored-By: Claude Mythos 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LUNnYzucjok597pz3cUK2K
The decoder parses frames received over the air, so it should not run as
root. Create a dedicated system account with audio/plugdev access, own the
output log with it, and sandbox the unit with the usual systemd protections.
Decoder names are now shell-quoted in the wrapper like the other arguments.

Co-Authored-By: Claude Mythos 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LUNnYzucjok597pz3cUK2K
…d unit

The service defaulted to the Ansible automation account, which normally has
passwordless sudo and holds the SSH trust for the node, while also receiving
keyboard-capture and PTT device access and owning its own code.

Create a dedicated nologin system account, keep the install tree and script
root-owned, restrict writes to the data directory, and harden the unit.
Numeric inventory values are cast before being rendered into Python source.

Co-Authored-By: Claude Mythos 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LUNnYzucjok597pz3cUK2K
Add SHA-256 verification for artifacts that are installed or executed with
elevated privileges:

- GridTracker2: optional package checksum, HTTPS-only URLs, and a cache
  filename derived from the upstream package so version bumps are not
  silently satisfied by a stale cached file.
- JS8Call: upstream AppImage digests for the default release, required by
  default with an explicit opt-out.
- CHIRP wheel, Graywolf package, NetLogger package: optional checksum
  variables and HTTPS-only URLs.
- LinBPQ: HTTPS-only binary URL and checksum format validation.

Quote operator names passed to id -gn in the js8call, chirp, and kiosk roles.

Co-Authored-By: Claude Mythos 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LUNnYzucjok597pz3cUK2K
- openwrt: assert that Wi-Fi, radio, route, and firewall-zone identifiers
  (including forward_to targets) are UCI-safe before they are rendered into
  the root shell script applied on the router. Only LAN names were checked.
- routeros: default hamstack_routeros_api_validate_certs to true so the API
  credential is not sent to an unverified endpoint; document the opt-out.
- The network play no longer inherits global become, which otherwise ran the
  controller-side RouterOS API calls under sudo.
- technitium-dns: raise the admin password floor to 12 characters, matching
  the other Vault-backed services.

Co-Authored-By: Claude Mythos 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LUNnYzucjok597pz3cUK2K
… in QA

The role templated nginx-default.conf.j2, which did not exist, so every
FT8Web deployment failed before the static server could start. Add a
minimal server block for the mirrored site and teach scripts/qa.py to
fail when a template or copy task references a file the role does not ship.

Co-Authored-By: Claude Mythos 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LUNnYzucjok597pz3cUK2K
AllStar's rpt_http_registrations.conf and WPSD's /etc/dmrgateway and
/etc/mmdvmhost carry network passwords. Ansible's backup copies inherit
whatever mode the previous file had and are never removed, so each change
left another readable copy of the secret next to the managed file.

The AllStar registration file and the WPSD password task no longer create
backups, and backups of the other WPSD files are gated behind the new
hamstack_wpsd_config_backups variable (default false). The WPSD extra-settings
pass-through loops now run with no_log so a credential placed there is not
echoed in play output.

Co-Authored-By: Claude Mythos 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LUNnYzucjok597pz3cUK2K
Co-Authored-By: Claude Mythos 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LUNnYzucjok597pz3cUK2K
Copilot AI balanced review requested due to automatic review settings October 12, 2026 22:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Declare become explicitly on the seven tasks that set become_user, and
downgrade var-naming[no-role-prefix] and yaml[line-length] to warnings in
.ansible-lint. HamStack deliberately keeps every variable in the hamstack_
namespace, and .yamllint already treats line length as a warning, so these
rules were failing CI on main without describing a real problem.

Co-Authored-By: Claude Mythos 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LUNnYzucjok597pz3cUK2K
@nv0o-hrv
nv0o-hrv merged commit c995deb into main Oct 12, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants