# Security Analysis

Vulnerability assessment of the hantim-server infrastructure. Each finding
includes severity, affected files, explanation, and a proposed fix. Sorted
from highest to lowest severity.

---

## HIGH

### ~~1. Security headers silently dropped on all HTTPS responses~~ FIXED

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~~ FIXED

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~~ FIXED

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)~~ FIXED

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 — ACCEPTED

**Affected:** `docker/garage/compose.yml:12`, `scripts/configure.sh:20`

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.

**Risk accepted:** restricting by IP would break replication on IP change.
The RPC secret provides adequate protection. Downgraded from HIGH to
MEDIUM.

---

## MEDIUM

### 6. No SSL/TLS hardening in nginx

**Affected:** `docker/nginx/nginx.conf`, all `conf.d/*.conf`

No `ssl_protocols`, `ssl_ciphers`, `ssl_session_cache`, or OCSP stapling
is configured. The nginx:alpine image's defaults may allow TLS 1.0/1.1 and
weak cipher suites depending on the version.

Certbot writes `options-ssl-nginx.conf` to `/etc/letsencrypt/` (which is
mounted into the container), but no conf file includes it.

**Fix:** Add TLS hardening to `nginx.conf` inside the `http` block:

```nginx
ssl_protocols TLSv1.2 TLSv1.3;
ssl_ciphers ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384;
ssl_prefer_server_ciphers off;
ssl_session_cache shared:SSL:10m;
ssl_session_timeout 1d;
ssl_session_tickets off;
ssl_stapling on;
ssl_stapling_verify on;
resolver 1.1.1.1 8.8.8.8 valid=300s;
resolver_timeout 5s;
```

Or include certbot's config:

```nginx
include /etc/letsencrypt/options-ssl-nginx.conf;
ssl_dhparam /etc/letsencrypt/ssl-dhparams.pem;
```

---

### 7. No default server block to catch unknown Host headers

**Affected:** `docker/nginx/conf.d/`

Without a catch-all server block, requests with unknown `Host` headers are
served by whichever conf file nginx loads first (alphabetically). This
leaks which sites are hosted and can be used for reconnaissance.

**Fix:** Add a default server block:

```nginx
# docker/nginx/conf.d/00-default.conf
server {
    listen 80 default_server;
    listen [::]:80 default_server;
    listen 443 ssl default_server;
    listen [::]:443 ssl default_server;
    http2 on;
    server_name _;

    ssl_certificate     /etc/letsencrypt/live/hantim.net/fullchain.pem;
    ssl_certificate_key /etc/letsencrypt/live/hantim.net/privkey.pem;

    return 444;
}
```

The `444` status silently drops the connection.

---

### 8. No rate limiting on any endpoint

**Affected:** `docker/nginx/nginx.conf`, all `conf.d/*.conf`

No `limit_req` or `limit_conn` zones are defined. The server is vulnerable
to brute-force attacks against the S3 and admin APIs, and to connection
exhaustion / slowloris attacks.

**Fix:** Add rate limiting zones to `nginx.conf`:

```nginx
http {
    limit_req_zone $binary_remote_addr zone=general:10m rate=10r/s;
    limit_req_zone $binary_remote_addr zone=api:10m rate=2r/s;
    ...
}
```

Apply them in server blocks:

```nginx
# In s3.hantim.net.conf and garage.hantim.net.conf:
limit_req zone=api burst=20 nodelay;

# In site server blocks:
limit_req zone=general burst=30 nodelay;
```

---

### 9. envsubst replaces all environment variables in garage.toml

**Affected:** `docker/garage/Dockerfile:8`

```dockerfile
CMD envsubst < /etc/garage.toml.tpl > /etc/garage.toml && exec /garage server
```

`envsubst` without arguments replaces every `$VAR` or `${VAR}` pattern. If
any other environment variable (e.g., `$HOME`, `$PATH`) matches a pattern
in the template, it will be silently substituted. This is fragile and could
break the config in unexpected ways.

**Fix:** Specify the variables explicitly:

```dockerfile
CMD envsubst '${RPC_SECRET} ${ARGENTO_NODE_ID} ${ADMIN_TOKEN}' < /etc/garage.toml.tpl > /etc/garage.toml && exec /garage server
```

---

### 10. StrictHostKeyChecking=accept-new trusts first connection blindly

**Affected:** `.gitea/workflows/*.yml`, `tools/app.sh:367`

All SSH connections use `-o StrictHostKeyChecking=accept-new`, which trusts
whatever host key is presented on first contact. A man-in-the-middle during
the first CI run (or after server reprovisioning) could intercept the
deploy connection and capture the SSH private key.

**Fix:** Pin the server's host key in workflows. Add a step before SSH:

```yaml
- name: Configure SSH
  run: |
    mkdir -p ~/.ssh
    echo "${{ secrets.DEPLOY_SSH_KEY }}" > ~/.ssh/deploy_key
    chmod 600 ~/.ssh/deploy_key
    echo "${{ vars.DEPLOY_HOST_KEY }}" >> ~/.ssh/known_hosts
    ssh -o StrictHostKeyChecking=yes -i ~/.ssh/deploy_key deploy@${{ vars.DEPLOY_HOST }} deploy-nginx
```

Store the server's host public key as a Gitea variable (`DEPLOY_HOST_KEY`).

---

### 11. API tokens visible in process listings

**Affected:** `tools/app.sh` (lines 134, 139, 225, etc.)

Secrets passed as curl header arguments appear in `ps aux` and
`/proc/<pid>/cmdline` for the duration of the curl call:

```bash
VULTR_AUTH="Authorization: Bearer $VULTR_API_KEY"
curl -s "$VULTR_API/domains/$APP" -H "$VULTR_AUTH"
```

**Fix:** Use curl's `--header @-` or `-H @<file>` to read headers from
stdin/file instead of command arguments:

```bash
VULTR_AUTH_FILE=$(mktemp)
echo "Authorization: Bearer $VULTR_API_KEY" > "$VULTR_AUTH_FILE"
chmod 600 "$VULTR_AUTH_FILE"
trap "rm -f $VULTR_AUTH_FILE" EXIT
curl -s "$VULTR_API/domains/$APP" -H "@$VULTR_AUTH_FILE"
```

Or use a `.netrc` file for authentication.

---

### 12. No container resource limits

**Affected:** All `docker/*/compose.yml`

No memory or CPU limits are set on any container. A runaway container
(memory leak, fork bomb, or compromised process) can consume all host
resources and take down every service.

**Fix:** Add resource limits to compose files:

```yaml
services:
  app:
    ...
    deploy:
      resources:
        limits:
          memory: 128M
          cpus: '0.5'
```

For nginx and garage, use higher limits appropriate to their workload.

---

### 13. Brief nginx downtime during initial cert issuance

**Affected:** `scripts/deploy.sh:87-114`

When deploying nginx with new domains that need certs, the script stops
nginx, runs `certbot --standalone`, then starts nginx. During this window
(potentially minutes if multiple certs are needed), all sites are down.

**Fix:** Issue certs via the `cert-<domain>` SSH command (which uses
`--webroot` and causes zero downtime) before deploying the nginx config.
The `app.sh` already does this (`cmd_cert` runs before nginx gets the new
config). For the standalone path, consider using certbot's `--preferred-challenges dns`
or pre-issuing certs in a separate step.

---

### 14. Hardcoded Bitwarden secret UUIDs

**Affected:** `scripts/bootstrap.sh:21-22`

```bash
SECRET_REGISTRY_TOKEN="43f4a77d-7bb4-49ad-8044-b411016c5c6e"
SECRET_DEPLOY_PUBKEY="945760dd-90e2-46e5-98ad-b411016e3f3b"
```

UUIDs identify specific secrets. If this repo is exposed, an attacker
knows exactly which secret IDs to target. Using `bws secret list` with
name lookup (like the rest of the codebase does) is more consistent and
doesn't leak UUIDs.

**Fix:** Use name-based lookup like `deploy.sh` and `app.sh` do:

```bash
REGISTRY_TOKEN=$(bws secret list | jq -r '.[] | select(.key == "hantim-ci-registry-push") | .value')
DEPLOY_PUBKEY=$(bws secret list | jq -r '.[] | select(.key == "hantim-deploy-ssh-public-key") | .value')
```

---

### 15. certbot registers without email

**Affected:** `scripts/deploy.sh:29, 109`

```bash
certbot certonly --standalone --non-interactive --agree-tos \
    --register-unsafely-without-email ...
```

Without an email, Let's Encrypt cannot send expiration warnings. If the
certbot renewal timer/cron breaks silently, certificates expire without
notice, causing site outages.

**Fix:** Register with an email:

```bash
certbot certonly --standalone --non-interactive --agree-tos \
    -m admin@hantim.net ...
```

---

## LOW

### 16. Certbot renewal hook has no error handling

**Affected:** `scripts/configure.sh:26-30`

```bash
cat > /etc/letsencrypt/renewal-hooks/deploy/reload-nginx.sh <<'HOOK'
#!/bin/bash
docker exec nginx nginx -s reload
HOOK
```

If the nginx container is down when certs renew, the reload fails silently.
The new cert won't take effect until nginx is manually restarted.

**Fix:**

```bash
#!/bin/bash
set -e
if ! docker exec nginx nginx -s reload; then
    logger -t certbot "ERROR: Failed to reload nginx after cert renewal"
    exit 1
fi
```

---

### 17. No audit logging for deploy operations

**Affected:** `scripts/deploy.sh`

There is no logging of who triggered a deploy, what changed, or when
secrets were accessed. If the server is compromised, there's no audit trail.

**Fix:** Add logging to deploy.sh:

```bash
logger -t hantim-deploy "deploy started: cmd=$CMD user=$(whoami) ssh_client=${SSH_CLIENT:-local}"
```

And at the end:

```bash
logger -t hantim-deploy "deploy finished: cmd=$CMD"
```

---

### 18. git pull without explicit branch

**Affected:** `scripts/deploy.sh:39, 51`

```bash
git pull
```

If the repo's default tracking branch changes or is misconfigured, `git
pull` could pull the wrong branch.

**Fix:**

```bash
git pull origin main
```

---

## Positive security practices already in place

- SSH `command=` restriction limits deploy user to specific commands
- Deploy user has `nologin` shell and scoped sudoers entry
- All scripts use `set -euo pipefail`
- HTTPS enforced everywhere with HTTP-to-HTTPS redirects
- HSTS with preload on all main server blocks
- `.env` files are `chmod 600` and gitignored
- BWS token file is `chmod 600`
- Docker network isolation: only nginx binds host ports
- Input validation on domain names with regex in `deploy.sh` and `app.sh`
- Idempotent scripts safe to re-run
- Certbot auto-renewal configured via timer or cron
- `nginx -t` config test before applying changes

---

## Recommended priority

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