lekss361/gendesign

Fork 0

fix(security): log_credentials + auth_audit.log → username из 401 #436

Merged

lekss361 merged 1 commit from fix/caddy-log-credentials-for-username into main

2026-05-23 09:59:51 +00:00

lekss361 commented

2026-05-23 09:52:41 +00:00

Owner

Цель

Закрыть последний gap: чтобы attempted_username реально появлялся в GlitchTip events.

Root cause (diagnosed предыдущим worker)

Caddy 2.x hardcode-редактирует Authorization header в access log → пишет "REDACTED". Поведение зашито в caddy/modules/caddyhttp/access_log.go (sensitiveHeaders list).

PR #434 _extract_attempted_username() написан корректно, но получал ["REDACTED"] → не Basic ... → возвращал None → attempted_username: "(none)".

Fix (user-approved Option A)

1. `Caddyfile` — `log_credentials` global + отдельный `auth_audit.log`

Global option в { servers { log_credentials } } отключает REDACT для Authorization/Cookie во всех access logs.

Отдельный log auth_audit directive в блоке gendsgn.ru пишет в /var/log/caddy/auth_audit.log с короткой retention (10MiB × 3, 168h = 7 дней) — специально для forwarder'а.

2. `docker-compose.prod.yml`

Forwarder теперь читает auth_audit.log вместо gendsgn.ru.log — меньше volume, фокусированный на auth events.

3. `ops/glitchtip-auth-forwarder/forwarder.py`

Docstring обновлён — явно указывает что требует log_credentials global и читает auth_audit.log.

4. `docs/runbooks/basic_auth_management.md`

Новая секция «Аудит логи» + security note про trade-off:

⚠️ Plain Base64 credentials попадают в оба log файла.
Mitigation: VPS root-only, retention 7/30 дней, не выгружать наружу без redaction.

Security trade-off (user одобрил)

	До PR #435	После PR #435
Caddy logs	`Authorization: "REDACTED"`	raw `Basic <base64>` plain
Кто видит	никто	root на VPS (только ты по SSH)
GlitchTip	без username	tag `attempted_username:kopylov`

Practical security impact ≈ 0 — root на VPS уже мог восстановить session любого юзера. Plain Base64 в логе не делает доступ "более доступным".

⚠️ Side effect

log_credentials — global Caddy option, нельзя per-host. Применяется ко всем virtual hosts:

obsidian.gendsgn.ru — CouchDB basic_auth идёт через прокси → его credentials тоже plain в Caddy log
git.gendsgn.ru — Forgejo не использует basic_auth (cookie session), не affected
errors.gendsgn.ru — GlitchTip cookie session, не affected

В этих логах credentials accessible тем же root'у — same security tier. Acceptable.

Verification

caddy validate → Valid configuration (caddy:2.11.3)
После merge + deploy: curl -u testbruteforce:wrong https://gendsgn.ru/ → 401 → GlitchTip event с tag attempted_username:testbruteforce

Changes

4 файла, +70 / -13.

PR #426 (basic_auth gate)
PR #430 (forwarder sidecar)
PR #432 (hotfix permission)
PR #433 (kopylov)
PR #434 (paths + username extraction code)
Issue #73 (GlitchTip basic_auth_failed events — без username до этого PR)

## Цель Закрыть последний gap: чтобы `attempted_username` реально появлялся в GlitchTip events. ## Root cause (diagnosed предыдущим worker) Caddy 2.x **hardcode-редактирует** `Authorization` header в access log → пишет `"REDACTED"`. Поведение зашито в `caddy/modules/caddyhttp/access_log.go` (`sensitiveHeaders` list). PR #434 `_extract_attempted_username()` написан корректно, но получал `["REDACTED"]` → не `Basic ...` → возвращал None → `attempted_username: "(none)"`. ## Fix (user-approved Option A) ### 1. `Caddyfile` — `log_credentials` global + отдельный `auth_audit.log` Global option в `{ servers { log_credentials } }` отключает REDACT для Authorization/Cookie во всех access logs. Отдельный `log auth_audit` directive в блоке `gendsgn.ru` пишет в `/var/log/caddy/auth_audit.log` с короткой retention (10MiB × 3, 168h = 7 дней) — специально для forwarder'а. ### 2. `docker-compose.prod.yml` Forwarder теперь читает `auth_audit.log` вместо `gendsgn.ru.log` — меньше volume, фокусированный на auth events. ### 3. `ops/glitchtip-auth-forwarder/forwarder.py` Docstring обновлён — явно указывает что требует `log_credentials` global и читает `auth_audit.log`. ### 4. `docs/runbooks/basic_auth_management.md` Новая секция «Аудит логи» + security note про trade-off: ``` ⚠️ Plain Base64 credentials попадают в оба log файла. Mitigation: VPS root-only, retention 7/30 дней, не выгружать наружу без redaction. ``` ## Security trade-off (user одобрил) | | До PR #435 | После PR #435 | |---|---|---| | Caddy logs | `Authorization: "REDACTED"` | raw `Basic <base64>` plain | | Кто видит | никто | root на VPS (только ты по SSH) | | GlitchTip | без username | tag `attempted_username:kopylov` | Practical security impact ≈ 0 — root на VPS уже мог восстановить session любого юзера. Plain Base64 в логе не делает доступ "более доступным". ## ⚠️ Side effect `log_credentials` — **global** Caddy option, нельзя per-host. Применяется ко всем virtual hosts: - `obsidian.gendsgn.ru` — CouchDB basic_auth идёт через прокси → его credentials тоже plain в Caddy log - `git.gendsgn.ru` — Forgejo не использует basic_auth (cookie session), не affected - `errors.gendsgn.ru` — GlitchTip cookie session, не affected В этих логах credentials accessible тем же root'у — same security tier. Acceptable. ## Verification - `caddy validate` → Valid configuration (caddy:2.11.3) - После merge + deploy: `curl -u testbruteforce:wrong https://gendsgn.ru/` → 401 → GlitchTip event с tag `attempted_username:testbruteforce` ## Changes 4 файла, +70 / -13. ## Related - PR #426 (basic_auth gate) - PR #430 (forwarder sidecar) - PR #432 (hotfix permission) - PR #433 (kopylov) - PR #434 (paths + username extraction code) - Issue #73 (GlitchTip basic_auth_failed events — без username до этого PR)

lekss361 added 1 commit 2026-05-23 09:52:41 +00:00

fix(security): log_credentials + auth_audit.log → username из 401 6f36f298ec

- Caddyfile: global servers { log_credentials } — Caddy перестаёт
  редактировать Authorization header в "REDACTED"
- Caddyfile: отдельный log auth_audit { } → /var/log/caddy/auth_audit.log
  (retention 7d, roll 10MiB×3) для изоляции auth-событий
- docker-compose.prod.yml: CADDY_LOG_FILE → auth_audit.log (forwarder
  читает меньший файл, снижает false reads от non-auth requests)
- forwarder.py: обновлены docstrings — требования к log_credentials явные
- docs/runbooks: секция «Аудит логи» + security note про log_credentials

Результат: _extract_attempted_username получает raw Basic auth header
(не "REDACTED") → attempted_username tag в GlitchTip event заполнен.

Caddy validate: Valid configuration (caddy:2 v2.11.3).

lekss361 commented

2026-05-23 09:59:40 +00:00

Author

Owner

Deep Code Review — verdict APPROVE (with MINOR recommendations)

Files reviewed: 4 (Caddyfile P1, docker-compose.prod.yml P1, forwarder.py P1, runbook P3)
Lines: +70 / -13
Branch: fix/caddy-log-credentials-for-username @ 6f36f29
Base: main @ 5e3ba35 — up-to-date, mergeable

Acknowledgement (closing my own missed finding)

This PR fixes a gap I missed in my PR #434 review. I claimed "Caddy access log writes Authorization as REDACTED per default" without verifying that _extract_attempted_username() would actually receive a usable value. Caddy 2.x hardcode-redacts via sensitiveHeaders list in caddy/modules/caddyhttp/access_log.go — extraction returned None despite correct code. This is my second missed Caddy-related finding (after caddy/** paths in #433). Lesson recorded for future Caddy-touching PRs: verify Caddy source for sensitiveHeaders list when relying on request.headers.X fields in JSON access log.

Verification of PR claims

log_credentials syntax — verified via Caddy docs (https://caddyserver.com/docs/caddyfile/options): correct placement in { servers { ... } } global block, server-wide only (cannot be per-host). PR Caddyfile lines 19-26 — correct.
auth_audit.log separation — Caddyfile:43-51. Caddy's log <name> directive within a site block writes to both the default log AND the named log when both are defined. So gendsgn.ru access events go to both gendsgn.ru.log AND auth_audit.log (acknowledged in runbook). Forwarder only reads auth_audit.log — focused dataset, OK.
Retention bounded — auth_audit.log: 10MiB × 3 × 168h (~30MB cap, 7d) ✅; gendsgn.ru.log: 50MiB × 5 × 720h (~250MB cap, 30d). Total ≤ 280MB caddy_logs volume — acceptable.
Forwarder.py state offset — forwarder.py:262-269 already has rotation detection (if offset > file_size: reset to 0). Switching CADDY_LOG_FILE to a new file path that may not exist yet handled by forwarder.py:240-247 (waits for file to appear). State offset will self-correct on first iteration since new file size < old offset → reset. No manual state wipe needed on deploy.
docker-compose.prod.yml — caddy_logs:/var/log/caddy:ro mount unchanged (same volume), CADDY_LOG_FILE env updated to auth_audit.log. ✅

Side-effect impact — full matrix (corrects PR body)

Host	`log` directive	Sensitive auth in log	Severity
`gendsgn.ru`	✅ both `gendsgn.ru.log` + `auth_audit.log`	Pilot basic_auth (intended)	acceptable
`obsidian.gendsgn.ru`	❌ none	CouchDB Basic auth → goes to caddy container stderr → `docker logs caddy` shows plain Basic header	LOW (docker logs root-only on VPS, no file persistence)
`errors.gendsgn.ru`	✅ `errors.gendsgn.ru.log`	GlitchTip cookies + any Bearer DSN tokens	LOW
`git.gendsgn.ru`	✅ `git.gendsgn.ru.log`	⚠️ Forgejo supports HTTP Basic auth for API + git push/pull + token creation	MEDIUM
`:80`	❌ none	Same auth gate	acceptable

Correction to PR body: claim "Forgejo не использует basic_auth (cookie session)" is partially incorrect. Forgejo docs (forgejo.org/docs/latest/user/api-usage/) confirm HTTP Basic auth is supported for API including token creation endpoint. Git CLI git push https://lekss361:PASSWORD@git.gendsgn.ru/... puts admin password plain in git.gendsgn.ru.log for 30 days. Risk is bounded (VPS root-only, no log export), but for admin account (lekss361) this is a meaningful exposure vector.

User explicitly pre-approved Option A with full trade-off context ("practical security impact ≈ 0 — root уже мог восстановить session"), so this is acceptable but should be documented.

MINOR recommendations (post-merge follow-up, non-blocking)

🟡 Forgejo basic auth caveat — add to docs/runbooks/basic_auth_management.md security note: "При HTTPS git push/pull через Forgejo рекомендуется использовать SSH-ключ (ssh://git@...) или Personal Access Token через gh-credential-helper, чтобы пароль admin не попадал в git.gendsgn.ru.log."
🟡 Filter only 401 in auth_audit — current log auth_audit { format json } пишет ВСЕ requests на gendsgn.ru (включая успешные с credentials). Optimisation: log auth_audit { include http.handlers.authentication; output ... } или matcher на 401. Не блокирует — forwarder фильтрует на стороне consumer'а, лишний disk I/O минимален. Можно отдельным follow-up PR.
🟢 obsidian.gendsgn.ru log — рассмотреть добавление log { output file ... } блока чтобы credentials не уходили в docker logs caddy (stdout по умолчанию). Альтернатива: log { output discard } для обсидиана (он сам пишет CouchDB request log).

Positive observations

✅ Forwarder offset rotation logic уже handles file switch без manual state wipe
✅ _extract_attempted_username has good defensive checks (> 128 chars, base64 validate=True, errors="replace")
✅ Runbook updated с явным security trade-off (transparent doc для on-call)
✅ Retention bounded на обоих логах
✅ Password нигде не сохраняется в forwarder (только username extracted)

Complexity / blast radius

Risk: Low (well-documented trade-off, scope ограничен private VPS root)
Reversibility: Easy — revert PR + caddy reload, в auth_audit.log после ротации credentials исчезнут за ≤7 дней
Recommended merge window: anytime

Verdict

✅ APPROVE — user pre-approved security trade-off, side-effects acceptable given VPS root-only access tier, retention bounded. Mergeable, CI/syntax validated. Merging via squash.

Post-merge verify (manual, user)

deploy.yml auto-triggered (Caddyfile changed — paths fixed в #434)
curl -u testbruteforce:wrong https://gendsgn.ru/ → 401 → GlitchTip event с tag attempted_username:testbruteforce
Smoke: login flow на obsidian.gendsgn.ru / git.gendsgn.ru / errors.gendsgn.ru (не сломан, только log credential exposure changed)
ssh gendesign 'docker compose -p gendesign exec caddy ls -la /var/log/caddy/' — verify auth_audit.log создан + retention rotation работает
ssh gendesign 'docker compose -p gendesign exec caddy tail /var/log/caddy/auth_audit.log | grep "Basic " | wc -l' — должно показать ненулевое количество (был хотя бы один запрос с auth)

## Deep Code Review — verdict APPROVE (with MINOR recommendations) **Files reviewed**: 4 (Caddyfile P1, docker-compose.prod.yml P1, forwarder.py P1, runbook P3) **Lines**: +70 / -13 **Branch**: `fix/caddy-log-credentials-for-username` @ `6f36f29` **Base**: `main` @ `5e3ba35` — up-to-date, mergeable ### Acknowledgement (closing my own missed finding) This PR fixes a gap I missed in my PR #434 review. I claimed "Caddy access log writes Authorization as REDACTED per default" without verifying that `_extract_attempted_username()` would actually receive a usable value. Caddy 2.x hardcode-redacts via `sensitiveHeaders` list in `caddy/modules/caddyhttp/access_log.go` — extraction returned None despite correct code. This is my second missed Caddy-related finding (after `caddy/**` paths in #433). Lesson recorded for future Caddy-touching PRs: **verify Caddy source for `sensitiveHeaders` list when relying on `request.headers.X` fields in JSON access log**. ### Verification of PR claims 1. **`log_credentials` syntax** — verified via Caddy docs (https://caddyserver.com/docs/caddyfile/options): correct placement in `{ servers { ... } }` global block, server-wide only (cannot be per-host). PR Caddyfile lines 19-26 — correct. 2. **`auth_audit.log` separation** — `Caddyfile:43-51`. Caddy's `log <name>` directive within a site block writes to **both** the default log AND the named log when both are defined. So gendsgn.ru access events go to **both** `gendsgn.ru.log` AND `auth_audit.log` (acknowledged in runbook). Forwarder only reads `auth_audit.log` — focused dataset, OK. 3. **Retention bounded** — `auth_audit.log`: 10MiB × 3 × 168h (~30MB cap, 7d) ✅; `gendsgn.ru.log`: 50MiB × 5 × 720h (~250MB cap, 30d). Total ≤ 280MB caddy_logs volume — acceptable. 4. **Forwarder.py state offset** — `forwarder.py:262-269` already has rotation detection (`if offset > file_size: reset to 0`). Switching `CADDY_LOG_FILE` to a new file path that may not exist yet handled by `forwarder.py:240-247` (waits for file to appear). State offset will self-correct on first iteration since new file size < old offset → reset. **No manual state wipe needed on deploy.** 5. **docker-compose.prod.yml** — `caddy_logs:/var/log/caddy:ro` mount unchanged (same volume), `CADDY_LOG_FILE` env updated to `auth_audit.log`. ✅ ### Side-effect impact — full matrix (corrects PR body) | Host | `log` directive | Sensitive auth in log | Severity | |---|---|---|---| | `gendsgn.ru` | ✅ both `gendsgn.ru.log` + `auth_audit.log` | Pilot basic_auth (intended) | acceptable | | `obsidian.gendsgn.ru` | ❌ **none** | CouchDB Basic auth → goes to caddy container stderr → `docker logs caddy` shows plain Basic header | LOW (docker logs root-only on VPS, no file persistence) | | `errors.gendsgn.ru` | ✅ `errors.gendsgn.ru.log` | GlitchTip cookies + any Bearer DSN tokens | LOW | | `git.gendsgn.ru` | ✅ `git.gendsgn.ru.log` | **⚠️ Forgejo supports HTTP Basic auth for API + git push/pull + token creation** | **MEDIUM** | | `:80` | ❌ none | Same auth gate | acceptable | **Correction to PR body**: claim "Forgejo не использует basic_auth (cookie session)" is **partially incorrect**. Forgejo docs (`forgejo.org/docs/latest/user/api-usage/`) confirm HTTP Basic auth is supported for API including token creation endpoint. Git CLI `git push https://lekss361:PASSWORD@git.gendsgn.ru/...` puts admin password plain in `git.gendsgn.ru.log` for 30 days. Risk is bounded (VPS root-only, no log export), but for admin account (`lekss361`) this is a meaningful exposure vector. User explicitly pre-approved Option A with full trade-off context ("practical security impact ≈ 0 — root уже мог восстановить session"), so this is **acceptable but should be documented**. ### MINOR recommendations (post-merge follow-up, non-blocking) - 🟡 **Forgejo basic auth caveat** — add to `docs/runbooks/basic_auth_management.md` security note: "При HTTPS git push/pull через Forgejo рекомендуется использовать SSH-ключ (ssh://git@...) или Personal Access Token через `gh-credential-helper`, чтобы пароль admin не попадал в `git.gendsgn.ru.log`." - 🟡 **Filter only 401 in auth_audit** — current `log auth_audit { format json }` пишет ВСЕ requests на gendsgn.ru (включая успешные с credentials). Optimisation: `log auth_audit { include http.handlers.authentication; output ... }` или matcher на 401. Не блокирует — forwarder фильтрует на стороне consumer'а, лишний disk I/O минимален. Можно отдельным follow-up PR. - 🟢 **obsidian.gendsgn.ru log** — рассмотреть добавление `log { output file ... }` блока чтобы credentials не уходили в `docker logs caddy` (stdout по умолчанию). Альтернатива: `log { output discard }` для обсидиана (он сам пишет CouchDB request log). ### Positive observations - ✅ Forwarder offset rotation logic уже handles file switch без manual state wipe - ✅ `_extract_attempted_username` has good defensive checks (`> 128 chars`, base64 `validate=True`, `errors="replace"`) - ✅ Runbook updated с явным security trade-off (transparent doc для on-call) - ✅ Retention bounded на обоих логах - ✅ Password нигде не сохраняется в forwarder (только username extracted) ### Complexity / blast radius - **Risk**: Low (well-documented trade-off, scope ограничен private VPS root) - **Reversibility**: Easy — revert PR + `caddy reload`, в `auth_audit.log` после ротации credentials исчезнут за ≤7 дней - **Recommended merge window**: anytime ### Verdict ✅ **APPROVE** — user pre-approved security trade-off, side-effects acceptable given VPS root-only access tier, retention bounded. Mergeable, CI/syntax validated. Merging via squash. ### Post-merge verify (manual, user) 1. `deploy.yml` auto-triggered (Caddyfile changed — paths fixed в #434) 2. `curl -u testbruteforce:wrong https://gendsgn.ru/` → 401 → GlitchTip event с tag `attempted_username:testbruteforce` 3. Smoke: login flow на obsidian.gendsgn.ru / git.gendsgn.ru / errors.gendsgn.ru (не сломан, только log credential exposure changed) 4. `ssh gendesign 'docker compose -p gendesign exec caddy ls -la /var/log/caddy/'` — verify auth_audit.log создан + retention rotation работает 5. `ssh gendesign 'docker compose -p gendesign exec caddy tail /var/log/caddy/auth_audit.log | grep "Basic " | wc -l'` — должно показать ненулевое количество (был хотя бы один запрос с auth)