From 4a7e2bf5dd93240fefeec23fb15bb0a6244224e3 Mon Sep 17 00:00:00 2001 From: Timothy Kim Date: Fri, 20 Mar 2026 16:00:24 -0400 Subject: [PATCH] update security.md with fixed and accepted items --- SECURITY.md | 187 ++++++++-------------------------------------------- 1 file changed, 28 insertions(+), 159 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index f2a76b2..f5d9595 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -8,178 +8,47 @@ from highest to lowest severity. ## HIGH -### 1. Security headers silently dropped on all HTTPS responses +### ~~1. Security headers silently dropped on all HTTPS responses~~ FIXED -**Affected:** `docker/nginx/nginx.conf`, all `conf.d/*.conf` - -The `http` block in `nginx.conf` defines: - -```nginx -add_header X-Content-Type-Options nosniff; -add_header X-Frame-Options DENY; -``` - -But nginx's `add_header` inheritance rule means these are **dropped** from -any block that defines its own `add_header`. Every HTTPS server block adds -`Strict-Transport-Security`, which causes the parent headers to stop being -inherited. The result: all site responses have HSTS but **no** -`X-Content-Type-Options` and **no** `X-Frame-Options`. - -This is a well-documented nginx gotcha. Sites are currently vulnerable to -clickjacking and MIME-type sniffing attacks. - -**Fix:** Add all security headers in every server block that uses -`add_header`, or move them into a shared snippet. The simplest approach is -a snippet file included from each HTTPS server block: - -```nginx -# docker/nginx/conf.d/security-headers.inc -add_header Strict-Transport-Security "max-age=63072000; preload" always; -add_header X-Content-Type-Options "nosniff" always; -add_header X-Frame-Options "DENY" always; -add_header Referrer-Policy "strict-origin-when-cross-origin" always; -add_header Permissions-Policy "geolocation=(), microphone=(), camera=()" always; -``` - -Then in each HTTPS server block, replace the standalone HSTS line with: - -```nginx -include /etc/nginx/conf.d/security-headers.inc; -``` - -And remove the (now-dead) `add_header` lines from `nginx.conf`'s `http` -block. +Fixed in `cc07a34`. All security headers moved to shared snippet +(`security-headers.inc`) included from each HTTPS server block. +Post-deploy test in `tests/test_security_headers.py` verifies all headers. --- -### 2. Garage admin API exposed to the internet with only token auth +### ~~2. Garage admin API exposed to the internet with only token auth~~ FIXED -**Affected:** `docker/nginx/conf.d/garage.hantim.net.conf` - -The Garage admin API (port 3903) is proxied through nginx to the public -internet at `garage.hantim.net`. The only protection is the Bearer token. -No IP restriction, no rate limiting, no fail2ban. Anyone who obtains or -brute-forces the admin token can create/delete buckets, manage access keys, -and read/write all stored data. - -**Fix:** Restrict access by source IP. Only the developer machine and -argento need access: - -```nginx -location / { - allow /32; - allow /32; - deny all; - - proxy_pass http://garage:3903; -} -``` - -If IP restriction isn't practical, add rate limiting at minimum: - -```nginx -limit_req_zone $binary_remote_addr zone=admin:1m rate=5r/s; - -location / { - limit_req zone=admin burst=10 nodelay; - proxy_pass http://garage:3903; -} -``` +Fixed in `cce91c0`. Admin API restricted to home IP via nginx +`allow`/`deny`. Update IP if it changes: `dig +short argento.ddns.net`. --- -### 3. Docker group membership gives deploy user root-equivalent access +### ~~3. Docker group membership gives deploy user root-equivalent access~~ FIXED -**Affected:** `scripts/bootstrap.sh:98` - -```bash -usermod -aG docker deploy -``` - -Any member of the `docker` group can run -`docker run -v /:/host alpine chroot /host` and get full root access. This -undermines the SSH `command=` restriction and the scoped sudoers entry. - -The docker group is unnecessary here: the SSH `authorized_keys` entry runs -`sudo /opt/hantim/scripts/deploy.sh`, so deploy.sh already executes as -root. Docker commands within it don't need the deploy user in the docker -group. - -**Fix:** Remove the line. The sudoers entry already covers the deploy -flow: - -```bash -# Remove this line from bootstrap.sh: -# usermod -aG docker deploy -``` - -If the user is already in the group on an existing server: - -```bash -gpasswd -d deploy docker -``` +Fixed in `2099464`. Removed `usermod -aG docker deploy` from +`bootstrap.sh`. Deploy user removed from docker group on live server. --- -### 4. All secrets fetched on every deploy (bws secret list in a loop) +### ~~4. All secrets fetched on every deploy (bws secret list in a loop)~~ FIXED -**Affected:** `scripts/deploy.sh:62`, `tools/app.sh:98` - -`deploy.sh` calls `bws secret list` (which returns **every** secret for -the machine account) once per line in `.env.keys`: - -```bash -while IFS= read -r line; do - ... - value=$(bws secret list | jq -r --arg key "$secret_key" ...) -done < "docker/$APP/.env.keys" -``` - -For garage with 3 secrets, this fetches the full secret list 3 times. All -secrets are materialized in a pipe buffer each time. A core dump, `/proc` -read, or process trace during this window exposes everything. - -**Fix:** Fetch the list once, filter locally: - -```bash -ALL_SECRETS=$(bws secret list) -while IFS= read -r line || [ -n "$line" ]; do - [ -z "$line" ] && continue - var_name="${line%%=*}" - secret_key="${line#*=}" - value=$(echo "$ALL_SECRETS" | jq -r --arg key "$secret_key" \ - '.[] | select(.key == $key) | .value') - if [ -z "$value" ]; then - echo "ERROR: Secret '$secret_key' not found in bws." - exit 1 - fi - env_content="${env_content}${var_name}=${value}"$'\n' -done < "docker/$APP/.env.keys" -unset ALL_SECRETS -``` - -Even better, use `bws secret get ` to fetch individual secrets by ID -instead of listing all. +Fixed in `33a2383`. `.env.keys` now uses secret UUIDs. `deploy.sh` fetches +each secret individually via `bws secret get `. --- -### 5. Garage RPC port open to the internet +### 5. Garage RPC port open to the internet — ACCEPTED **Affected:** `docker/garage/compose.yml:12`, `scripts/configure.sh:20` -Port 3901 is bound to the host and opened in the firewall. This is needed -for cluster replication with argento, but it means anyone on the internet -can attempt RPC connections. The only protection is the `rpc_secret`. +Port 3901 is bound to the host and opened in the firewall for cluster +replication with argento. Firewalld cannot resolve DDNS hostnames +dynamically, and argento's IP changes with the home ISP. The RPC secret +(long random string from Bitwarden) is the primary protection. -**Fix:** Restrict the firewall rule to argento's IP only: - -```bash -# In configure.sh, replace: -firewall-cmd --permanent --add-port=3901/tcp - -# With: -firewall-cmd --permanent --add-rich-rule='rule family="ipv4" source address="/32" port port="3901" protocol="tcp" accept' -``` +**Risk accepted:** restricting by IP would break replication on IP change. +The RPC secret provides adequate protection. Downgraded from HIGH to +MEDIUM. --- @@ -528,11 +397,11 @@ git pull origin main ## Recommended priority -1. Fix security headers (item 1) — actual bug, every site affected now -2. Restrict Garage admin API access (item 2) -3. Remove deploy user from docker group (item 3) -4. Add SSL/TLS hardening (item 6) -5. Add default server block (item 7) -6. Restrict Garage RPC firewall rule to argento IP (item 5) -7. Fix bws secret list loop (item 4) +1. ~~Fix security headers (item 1)~~ DONE +2. ~~Restrict Garage admin API access (item 2)~~ DONE +3. ~~Remove deploy user from docker group (item 3)~~ DONE +4. ~~Fix bws secret list loop (item 4)~~ DONE +5. ~~Garage RPC port (item 5)~~ ACCEPTED +6. Add SSL/TLS hardening (item 6) +7. Add default server block (item 7) 8. Everything else as time permits