update security.md with fixed and accepted items
This commit is contained in:
+28
-159
@@ -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 <your-ip>/32;
|
||||
allow <argento-ip>/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 <uuid>` 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 <uuid>`.
|
||||
|
||||
---
|
||||
|
||||
### 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="<argento-ip>/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
|
||||
|
||||
Reference in New Issue
Block a user