Relay: trust X-Forwarded-For only from the proxy itself #116

Merged
littlefox merged 2 commits from issue-114-proxy-peer into main 2026-10-18 15:48:55 +00:00
Owner

Closes #114

  • NEOKOM_RELAY_TRUST_PROXY: 1 now means "a proxy on loopback", or a comma-separated list of proxy addresses. X-Forwarded-For only counts on connections from those; everyone else is counted by their TCP address.
  • The Docker image sets it, so https users each get their own limit; direct clients on 4895 come from Docker's network and can't fake it.
  • D-040: Let's Encrypt, other relays are fine at Lysator, 24 h log retention.

Verified: unit test for the address choice, an integration test (separate limits behind a trusted proxy, forged header ignored otherwise), and tools/web-image-smoke.sh on a fresh image: https clients logged by their own address, forged X-Forwarded-For: 6.6.6.6 through Caddy and on 4895 ignored. The same smoke check fails on the current published image.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XJW2ocwLunyaLiUwCBHEP8

Closes #114 - `NEOKOM_RELAY_TRUST_PROXY`: `1` now means "a proxy on loopback", or a comma-separated list of proxy addresses. X-Forwarded-For only counts on connections from those; everyone else is counted by their TCP address. - The Docker image sets it, so https users each get their own limit; direct clients on 4895 come from Docker's network and can't fake it. - D-040: Let's Encrypt, other relays are fine at Lysator, 24 h log retention. Verified: unit test for the address choice, an integration test (separate limits behind a trusted proxy, forged header ignored otherwise), and tools/web-image-smoke.sh on a fresh image: https clients logged by their own address, forged `X-Forwarded-For: 6.6.6.6` through Caddy and on 4895 ignored. The same smoke check fails on the current published image. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01XJW2ocwLunyaLiUwCBHEP8
Relay: trust X-Forwarded-For only from the proxy itself
All checks were successful
packaging / arch (pull_request) Successful in 15m10s
ci / check (push) Successful in 8m58s
packaging / fedora (pull_request) Successful in 20m28s
packaging / debian (pull_request) Successful in 22m49s
ci / desktop (push) Successful in 15m28s
packaging / nix (pull_request) Successful in 29m37s
packaging / release (pull_request) Has been skipped
ci / e2e (push) Successful in 25m9s
cddaabb077
In the Docker image's HTTPS mode Caddy reaches the relay on loopback, so
every https user counted as 127.0.0.1 and they shared one per-address
limit. Turning on NEOKOM_RELAY_TRUST_PROXY wasn't safe either: port 4895
is published too, and any direct client could have named its own address.

Now the header only counts on connections from trusted proxy addresses
("1" means loopback, or a list), and the image turns that on. The smoke
test checks both ways: https clients by their own address, and a forged
header through Caddy and on 4895 ignored; it fails on the old image.
Also D-040: the user's answers for the public relay.

Refs #114

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJW2ocwLunyaLiUwCBHEP8
Merge origin/main into issue-114-proxy-peer
Some checks failed
ci / check (push) Successful in 13m26s
packaging / arch (pull_request) Successful in 14m52s
ci / desktop (push) Successful in 17m54s
ci / e2e (push) Successful in 27m38s
packaging / fedora (pull_request) Successful in 29m16s
packaging / debian (pull_request) Successful in 31m0s
packaging / nix (pull_request) Failing after 29m5s
packaging / release (pull_request) Has been cancelled
6e3c9139cc
Author
Owner

Merged too early: I waited only for the branch run (#195) and not for the PR's packaging run (#196), which failed; main's run after the merge (#197) failed as well. Neither failure was in code this PR changed: the nix job hung after a finished build (#119), and the desktop smoke test lost a key press (#118); the same code passed both jobs on other runs. tools/forge.py pr-merge now refuses unless every run of the head commit passed.

Merged too early: I waited only for the branch run (#195) and not for the PR's packaging run (#196), which failed; main's run after the merge (#197) failed as well. Neither failure was in code this PR changed: the nix job hung after a finished build (#119), and the desktop smoke test lost a key press (#118); the same code passed both jobs on other runs. tools/forge.py pr-merge now refuses unless every run of the head commit passed.
littlefox referenced this pull request from a commit 2026-10-19 05:23:03 +00:00
Sign in to join this conversation.
No description provided.