From a39a77ba50cc587c603a9077e1cbb9dcd93eb037 Mon Sep 17 00:00:00 2001 From: Krish Sharma <186846943+krishsharma-code@users.noreply.github.com> Date: Thu, 29 Oct 2026 12:05:32 +0530 Subject: [PATCH] refactor(security): harden critical config backup (permissions, verification, retention) --- .github/workflows/ci.yml | 19 ++ README.md | 14 +- bash_scripts/backup_critical_configs.sh | 358 ++++++++++++++++++++++-- tests/test_backup_critical_configs.py | 281 +++++++++++++++++++ 4 files changed, 651 insertions(+), 21 deletions(-) create mode 100644 tests/test_backup_critical_configs.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e8c8ea1..7e0874d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,6 +40,25 @@ jobs: continue-on-error: true run: flake8 python_scripts --max-line-length=120 --statistics + tests: + name: Pytest suite + runs-on: ubuntu-latest + + steps: + - name: Check out repository + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.12' + + - name: Install pytest + run: python -m pip install --upgrade pip pytest + + - name: Run the test suite + run: python -m pytest tests -q + shell: name: Bash syntax check runs-on: ubuntu-latest diff --git a/README.md b/README.md index 78a7c2e..55f3228 100644 --- a/README.md +++ b/README.md @@ -126,6 +126,18 @@ Some scripts accept flags — pass `--help` to see the supported options, for ex ./bash_scripts/docker_container_audit.sh --help ``` +### Running the tests + +The Python scripts and the tested shell scripts have a pytest suite under `tests/`: + +```bash +python3 -m pip install pytest +python3 -m pytest tests -q +``` + +The shell-script tests drive each script through its real command-line interface +with throwaway directories, so they never touch `/etc` and do not need root. + --- ## 🧭 Script Index @@ -222,7 +234,7 @@ Each table lists the script filename, its functional category, and a one-line de | `yara_rule_generator.py` | Signatures | Generates basic YARA rule syntax for signature matching. | | `check_root_privileges.sh` | Privilege Audit | Verifies root/sudo escalation rights. | | `find_suid_binaries.sh` | Privilege Audit | Locates SUID/SGID files for auditing. | -| `backup_critical_configs.sh` | Backup | Automates secure backup of `/etc` files. | +| `backup_critical_configs.sh` | Backup | Verified, owner-only backup of critical `/etc` files (`--dry-run`, `--keep`). | | `ssh_config_auditor.sh` | Hardening | Audits `sshd_config` against hardening rules. | | `firewall_status_checker.sh` | Firewall | Monitors UFW/iptables rules and status. | | `active_connections_monitor.sh` | Monitoring | Lists established connections and their PIDs. | diff --git a/bash_scripts/backup_critical_configs.sh b/bash_scripts/backup_critical_configs.sh index 077c857..809810f 100644 --- a/bash_scripts/backup_critical_configs.sh +++ b/bash_scripts/backup_critical_configs.sh @@ -1,27 +1,345 @@ -#!/bin/bash -# backup_critical_configs.sh +#!/usr/bin/env bash +# +# backup_critical_configs.sh - Create a verified, owner-only backup archive of +# critical system configuration files. +# # Day 5: Security Auditing & System Defense -# Description: Automates backing up critical system configuration files. +# +# What it does +# 1. Selects the critical files that actually exist on this host. +# 2. Packs them into a single gzip'd tar archive inside a directory that is +# restricted to the invoking user (mode 0700), with the archive itself at +# mode 0600 - /etc/shadow must never land in a world-readable location. +# 3. Verifies the archive before it is published (gzip integrity + entry +# count), then renames it into place atomically and writes a SHA-256 +# sidecar file next to it. +# 4. Prunes old archives, keeping the most recent N. +# +# Security notes +# * Files that exist but cannot be read (for example /etc/shadow without +# root) are a hard error: silently producing a "successful" backup that is +# missing the password database is worse than failing. +# * The archive is built under a temporary name and only renamed after it has +# been verified, so a partially written file is never mistaken for a good +# backup. +# * Every state-changing action is printed before it happens, and --dry-run +# prints the whole plan without touching the filesystem. +# +# Restoring: tar stores members with the leading slash stripped, so extract an +# archive from / with `sudo tar -xzf -C /` after checking it against +# the .sha256 sidecar. +# +# Exit codes +# 0 success +# 1 usage error +# 2 a source file could not be read, or no readable sources were found +# 3 the archive failed verification +# 4 the archive could not be written +# +# Usage +# sudo ./backup_critical_configs.sh +# ./backup_critical_configs.sh --dry-run +# ./backup_critical_configs.sh --backup-dir /var/backups/configs --keep 10 -BACKUP_DIR="/tmp/security_backups" -TIMESTAMP=$(date +%Y%m%d_%H%M%S) -BACKUP_FILE="critical_configs_$TIMESTAMP.tar.gz" +set -euo pipefail -echo "[*] Initializing Critical Configuration Backup..." +# Restrict the default permissions of everything this script creates. Combined +# with the explicit chmod calls below this keeps /etc/shadow out of reach of +# other local users. +umask 077 -# Create backup directory if it doesn't exist -mkdir -p "$BACKUP_DIR" +readonly SCRIPT_NAME="${0##*/}" +readonly DEFAULT_BACKUP_DIR="/tmp/security_backups" +readonly DEFAULT_FILES="/etc/passwd /etc/group /etc/shadow /etc/ssh/sshd_config /etc/network/interfaces /etc/hosts" +readonly ARCHIVE_PREFIX="critical_configs_" -# List of files to backup -FILES_TO_BACKUP="/etc/passwd /etc/group /etc/shadow /etc/ssh/sshd_config /etc/network/interfaces /etc/hosts" +BACKUP_DIR="${BACKUP_DIR:-$DEFAULT_BACKUP_DIR}" +FILES_SPEC="${BACKUP_FILES:-$DEFAULT_FILES}" +KEEP=5 +DRY_RUN=0 +ALLOW_PARTIAL=0 +TMP_ARCHIVE="" -echo "[*] Archiving critical files..." -tar -czf "$BACKUP_DIR/$BACKUP_FILE" $FILES_TO_BACKUP 2>/dev/null +# -------------------------------------------------------------------------- +# Output helpers. Informational lines go to stdout, problems to stderr, so the +# script stays usable in a pipeline or cron job that mails stdout. +# -------------------------------------------------------------------------- +log_info() { printf '[*] %s\n' "$*"; } +log_ok() { printf '[+] %s\n' "$*"; } +log_warn() { printf '[!] %s\n' "$*" >&2; } +log_err() { printf '[-] %s\n' "$*" >&2; } -if [ $? -eq 0 ]; then - echo "[+] SUCCESS: Backup created at $BACKUP_DIR/$BACKUP_FILE" - echo "[*] Backup Integrity (SHA256):" - sha256sum "$BACKUP_DIR/$BACKUP_FILE" -else - echo "[!] ERROR: Backup failed. Check permissions." -fi +usage() { + cat <"$final.sha256"; then + die 4 "cannot write the SHA-256 sidecar for '$final'" + fi + chmod 0600 -- "$final.sha256" + + CREATED_ARCHIVE="$final" +} + +# -------------------------------------------------------------------------- +# Retention. Archive names sort chronologically because the timestamp is fixed +# width, so a plain lexicographic sort is enough - no stat/date parsing. +# -------------------------------------------------------------------------- +prune_archives() { + local -a archives=() + local excess i + + [ "$KEEP" -gt 0 ] || return 0 + + shopt -s nullglob + archives=("$BACKUP_DIR/${ARCHIVE_PREFIX}"*.tar.gz) + shopt -u nullglob + + [ "${#archives[@]}" -gt "$KEEP" ] || return 0 + + if [ "$DRY_RUN" -eq 1 ]; then + log_info "[dry-run] would prune $(( ${#archives[@]} - KEEP )) archive(s), keeping $KEEP" + return 0 + fi + + # Sort oldest-first. mapfile keeps paths containing spaces intact, unlike a + # plain $(...) word-split. + mapfile -t archives < <(printf '%s\n' "${archives[@]}" | sort) + + excess=$(( ${#archives[@]} - KEEP )) + for ((i = 0; i < excess; i++)); do + log_info "pruning old archive ${archives[$i]}" + rm -f -- "${archives[$i]}" "${archives[$i]}.sha256" + done +} + +main() { + parse_args "$@" + + SOURCES=() + CREATED_ARCHIVE="" + + log_info "initializing critical configuration backup" + if [ "$DRY_RUN" -eq 1 ]; then + log_info "dry-run mode: no files will be written" + fi + + collect_sources + prepare_backup_dir + create_archive + prune_archives + + if [ "$DRY_RUN" -eq 1 ]; then + log_ok "dry-run complete, nothing was written" + else + log_ok "SUCCESS: backup available at $CREATED_ARCHIVE" + fi +} + +main "$@" diff --git a/tests/test_backup_critical_configs.py b/tests/test_backup_critical_configs.py new file mode 100644 index 0000000..704f5f2 --- /dev/null +++ b/tests/test_backup_critical_configs.py @@ -0,0 +1,281 @@ +"""Tests for bash_scripts/backup_critical_configs.sh. + +The script is exercised through its real command-line interface with a +throwaway backup directory and a throwaway source list, so the tests never +touch /etc and never need root. The properties that matter are the security +ones: the archive and its directory must be owner-only, the archive must be +verified before it is published, and unreadable sources must fail loudly +instead of producing a backup with a silent hole in it. +""" + +import hashlib +import os +import stat +import subprocess +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parent.parent +SCRIPT = REPO_ROOT / "bash_scripts" / "backup_critical_configs.sh" + + +def run_script(*args, env=None): + """Run the script with a clean environment and return the completed process.""" + base_env = { + "PATH": os.environ.get("PATH", "/usr/bin:/bin"), + "HOME": os.environ.get("HOME", "/tmp"), + } + if env: + base_env.update(env) + return subprocess.run( + ["bash", str(SCRIPT), *args], + capture_output=True, + text=True, + env=base_env, + check=False, + ) + + +@pytest.fixture() +def sources(tmp_path): + """Two small files standing in for the critical config files.""" + src = tmp_path / "sources" + src.mkdir() + first = src / "passwd" + first.write_text("root:x:0:0:root:/root:/bin/bash\n") + second = src / "sshd_config" + second.write_text("PermitRootLogin no\n") + return src + + +@pytest.fixture() +def backup_dir(tmp_path): + return tmp_path / "backups" + + +def archive_list(backup_dir): + return sorted(backup_dir.glob("critical_configs_*.tar.gz")) + + +def test_script_exists_and_is_syntactically_valid(): + assert SCRIPT.is_file() + result = subprocess.run(["bash", "-n", str(SCRIPT)], capture_output=True, text=True) + assert result.returncode == 0, result.stderr + + +def test_help_exits_zero_without_touching_the_filesystem(tmp_path): + result = run_script("--help") + assert result.returncode == 0 + assert "backup_critical_configs.sh" in result.stdout + assert "--dry-run" in result.stdout + assert list(tmp_path.iterdir()) == [] + + +def test_unknown_option_is_a_usage_error(): + result = run_script("--nope") + assert result.returncode == 1 + assert "unknown option" in result.stderr + + +def test_keep_rejects_non_numeric_value(backup_dir, sources): + result = run_script( + "--backup-dir", str(backup_dir), + "--files", f"{sources}/passwd", + "--keep", "many", + ) + assert result.returncode == 1 + assert "non-negative integer" in result.stderr + assert not backup_dir.exists() + + +def test_creates_verified_archive_with_sidecar(backup_dir, sources): + result = run_script( + "--backup-dir", str(backup_dir), + "--files", f"{sources}/passwd {sources}/sshd_config", + ) + assert result.returncode == 0, result.stderr + + archives = archive_list(backup_dir) + assert len(archives) == 1 + archive = archives[0] + + # Both sources made it into the archive. + listing = subprocess.run( + ["tar", "-tzf", str(archive)], capture_output=True, text=True, check=True + ).stdout.split() + assert any(entry.endswith("passwd") for entry in listing) + assert any(entry.endswith("sshd_config") for entry in listing) + + # The sidecar matches the archive byte for byte. + sidecar = Path(str(archive) + ".sha256") + assert sidecar.is_file() + expected = hashlib.sha256(archive.read_bytes()).hexdigest() + assert expected in sidecar.read_text() + + +def test_archive_and_directory_are_owner_only(backup_dir, sources): + result = run_script( + "--backup-dir", str(backup_dir), + "--files", f"{sources}/passwd", + ) + assert result.returncode == 0, result.stderr + + archive = archive_list(backup_dir)[0] + sidecar = Path(str(archive) + ".sha256") + dir_mode = stat.S_IMODE(backup_dir.stat().st_mode) + archive_mode = stat.S_IMODE(archive.stat().st_mode) + sidecar_mode = stat.S_IMODE(sidecar.stat().st_mode) + + assert dir_mode == 0o700, f"backup dir mode is {oct(dir_mode)}" + assert archive_mode == 0o600, f"archive mode is {oct(archive_mode)}" + assert sidecar_mode == 0o600, f"sidecar mode is {oct(sidecar_mode)}" + + +def test_existing_world_readable_directory_is_tightened(backup_dir, sources): + backup_dir.mkdir() + backup_dir.chmod(0o755) + + result = run_script( + "--backup-dir", str(backup_dir), + "--files", f"{sources}/passwd", + ) + assert result.returncode == 0, result.stderr + assert stat.S_IMODE(backup_dir.stat().st_mode) == 0o700 + + +def test_missing_source_is_skipped_with_a_warning(backup_dir, sources): + result = run_script( + "--backup-dir", str(backup_dir), + "--files", f"{sources}/passwd {sources}/does_not_exist", + ) + assert result.returncode == 0, result.stderr + assert "not present on this host" in result.stderr + assert len(archive_list(backup_dir)) == 1 + + +def test_no_readable_sources_fails(backup_dir, tmp_path): + result = run_script( + "--backup-dir", str(backup_dir), + "--files", str(tmp_path / "missing_one") + " " + str(tmp_path / "missing_two"), + ) + assert result.returncode == 2 + assert "no readable files" in result.stderr + assert archive_list(backup_dir) == [] + + +def test_unreadable_source_fails_unless_partial_is_allowed(backup_dir, sources): + secret = sources / "shadow" + secret.write_text("root:$6$hash\n") + secret.chmod(0o000) + + if os.geteuid() == 0: + pytest.skip("root can read a 0000 file, so the unreadable path cannot be simulated") + + try: + result = run_script( + "--backup-dir", str(backup_dir), + "--files", f"{sources}/passwd {secret}", + ) + assert result.returncode == 2 + assert "cannot read" in result.stderr + assert archive_list(backup_dir) == [] + + partial = run_script( + "--backup-dir", str(backup_dir), + "--files", f"{sources}/passwd {secret}", + "--allow-partial", + ) + assert partial.returncode == 0, partial.stderr + assert "unreadable" in partial.stderr + assert len(archive_list(backup_dir)) == 1 + finally: + secret.chmod(0o600) + + +def test_dry_run_writes_nothing(backup_dir, sources): + result = run_script( + "--backup-dir", str(backup_dir), + "--files", f"{sources}/passwd", + "--dry-run", + ) + assert result.returncode == 0, result.stderr + assert "[dry-run]" in result.stdout + assert "nothing was written" in result.stdout + assert not backup_dir.exists() + + +def test_dry_run_does_not_prune(backup_dir, sources): + for _ in range(2): + assert run_script( + "--backup-dir", str(backup_dir), + "--files", f"{sources}/passwd", + "--keep", "0", + ).returncode == 0 + assert len(archive_list(backup_dir)) == 2 + + result = run_script( + "--backup-dir", str(backup_dir), + "--files", f"{sources}/passwd", + "--keep", "1", + "--dry-run", + ) + assert result.returncode == 0, result.stderr + assert len(archive_list(backup_dir)) == 2 + + +def test_retention_keeps_the_newest_n(backup_dir, sources): + """Seed three old archives, run once with --keep 2, and check the oldest goes.""" + backup_dir.mkdir(mode=0o700) + for day in (1, 2, 3): + archive = backup_dir / f"critical_configs_2020010{day}_000000.tar.gz" + archive.write_bytes(b"placeholder") + Path(str(archive) + ".sha256").write_text("placeholder\n") + + result = run_script( + "--backup-dir", str(backup_dir), + "--files", f"{sources}/passwd", + "--keep", "2", + ) + assert result.returncode == 0, result.stderr + + names = [p.name for p in archive_list(backup_dir)] + assert len(names) == 2 + # This run's archive plus the newest seeded one survive; the two oldest go. + assert not any("20200101" in n for n in names) + assert not any("20200102" in n for n in names) + assert any("20200103" in n for n in names) + # Pruning removes the sidecar alongside the archive. + assert not Path(str(backup_dir / "critical_configs_20200101_000000.tar.gz") + ".sha256").exists() + for name in names: + assert Path(str(backup_dir / name) + ".sha256").exists() + + +def test_retention_zero_keeps_everything(backup_dir, sources): + for _ in range(3): + assert run_script( + "--backup-dir", str(backup_dir), + "--files", f"{sources}/passwd", + "--keep", "0", + ).returncode == 0 + assert len(archive_list(backup_dir)) == 3 + + +def test_same_second_runs_do_not_overwrite(backup_dir, sources): + for _ in range(3): + result = run_script( + "--backup-dir", str(backup_dir), + "--files", f"{sources}/passwd", + "--keep", "0", + ) + assert result.returncode == 0, result.stderr + assert len(archive_list(backup_dir)) == 3 + + +def test_no_temporary_files_left_behind(backup_dir, sources): + assert run_script( + "--backup-dir", str(backup_dir), + "--files", f"{sources}/passwd", + ).returncode == 0 + leftovers = [p.name for p in backup_dir.iterdir() if p.name.startswith(".")] + assert leftovers == []