8 Commits
Author SHA1 Message Date
clawbot ba79d6de02 Stop allowing credentialed cross-origin requests (closes #261)
check / check (push) Failing after 3s
CORS() allowed every origin with credentials, so a page on another origin of the same site could read what the signed-in user sees.

CORS() now uses the CONVENTIONS.md template's options (AllowedOrigins "*", AllowCredentials false) and covers only the two public, read-only routes, GET /api/v1/server and GET /.well-known/healthcheck.json. Every other route sends no Access-Control-Allow-* header. A preflight to the public routes gets 405 like any other path.

Deviation: CORS() is not in the global middleware chain CONVENTIONS.md lists, because REPO_POLICIES.md forbids * on authenticated routes.

Model: opus-5-5
2026-11-11 01:22:38 +01:00
clawbot ed5125606a Stop the slow test packages waiting (closes #263)
check / check (push) Failing after 1m9s
The slowest test packages spent their time waiting rather than testing.

db.Params gains an optional PasswordCost (zero means bcrypt.DefaultCost), and the db, ircserver and handlers tests hash at bcrypt.MinCost. A test IRC server's relay checks its queue on broker notification instead of polling. The service and handlers tests build config.Config directly and run in parallel; config exports DefaultTrustedProxies and ParseTrustedProxies for them. Fixed sleeps give way to waits for the event. Production defaults are unchanged.

What still takes time is CPU work under -race or a timeout a test is about.

Model: opus-5-5
2026-11-11 01:15:43 +01:00
clawbot 847f5c6fec Let ircserver and db tests pass under heavy host load (closes #265)
check / check (push) Failing after 4s
Tests in internal/ircserver and internal/db failed under heavy host load and passed on a rerun.

testTimeout, the failure limit on every read and wait in internal/ircserver, goes from 5 to 30 seconds; passing tests take no longer. TestQuietClientNotAnsweringPingIsClosed gets its own 4-second ping interval and measures its upper bound from the end of registration, a second before a two-interval read deadline would close the connection, leaving room for scheduling delay. endlessQueryTimeout in internal/db goes from 100 ms to 2 seconds, so the cancelled-query tests interrupt a running statement.

Judgement call: the closed test no longer checks PING timing; TestQuietClientAnsweringPingStaysConnected still does.

Model: opus-5-5
2026-11-11 01:07:47 +01:00
sneak 142f0a1c4b Merge branch 'main' into next
check / check (push) Failing after 3s
2026-11-11 00:49:06 +01:00
clawbot 72d8f2c12b Stop startup on a list or map setting in neoirc.yaml (closes #268)
check / check (push) Failing after 4s
viper.GetString returns the empty string for a list or a map in neoirc.yaml, so IRC_LISTEN_ADDR: [":6667"] turned the IRC listener off and MOTD or SERVER_NAME went blank without an error.

Every setting is now read through getString in internal/config, which uses cast.ToStringE, the conversion viper.GetString wraps. A list or a map is an error naming the setting, without its value, since the value may be a password. An environment value, the empty string included, still wins over the file. TRUSTED_PROXIES uses the same helper. A config file that does not parse makes New return an error naming the file instead of calling panic.

Model: opus-5-5
2026-11-11 00:07:58 +01:00
clawbot f42c6214a0 Make the image prepare its own data directory (closes #262)
check / check (push) Failing after 4s
A host directory mounted at /var/lib/neoirc kept the host's owner, so neoircd could not create state.db and stopped at startup.

The image now starts deploy/docker-entrypoint.sh as root. It creates /var/lib/neoirc, gives it and its contents to neoirc with mode 0700, and execs neoircd as neoirc through su-exec. The runtime stage drops USER neoirc and no longer creates the directory at build time.

Judgement call: the entrypoint assumes it starts as root; docker run --user fails at chown.

Model: opus-5-5
2026-11-10 22:42:02 +01:00
clawbot a3d9bbfd81 Honour forwarded headers only from trusted proxies (closes #259)
check / check (push) Failing after 8s
Before, the server took the client's address from X-Forwarded-For or X-Real-IP whoever sent them, so a direct client could get past the login rate limit and choose the address and hostname recorded for its session.

A new ClientIP middleware, run before request logging, resolves the address once: forwarded headers count only when the peer is in TRUSTED_PROXIES, which defaults to the RFC 1918 ranges. A set list replaces the default, the empty string trusts nothing, and an invalid entry stops startup. The request log, login rate limiter and session creation all use the resolved address.

A proxy on the same host as a server outside Docker connects from 127.0.0.1 or ::1, which the default does not trust; the README says to list it.

Model: opus-5-5
2026-11-10 22:41:50 +01:00
sneak 12a232ee1b next → main: utility commands, IRC listener and audit fixes, build fixes (#111)
check / check (push) Canceled after 0s
Reviewed-on: #111
2026-10-22 18:27:05 +02:00
16 changed files with 1217 additions and 161 deletions

No files matched your search

+7 -6
View File
@@ -83,14 +83,15 @@ RUN CGO_ENABLED=0 go build -trimpath -ldflags="-s -w" -o /neoirc-cli ./cmd/neoir
# stage's chain and nothing else.
# alpine:3.21, 2026-02-26
FROM alpine@sha256:c3f8e73fdb79deaebaa2037150150191b9dcbfba68b4a46d70103204c53f4709
RUN apk add --no-cache ca-certificates \
&& addgroup -S neoirc && adduser -S neoirc -G neoirc \
&& mkdir -p /var/lib/neoirc \
&& chown neoirc:neoirc /var/lib/neoirc
# su-exec 0.2-r3, 2026-11-10
RUN apk add --no-cache ca-certificates su-exec=0.2-r3 \
&& addgroup -S neoirc && adduser -S neoirc -G neoirc
COPY --from=builder /neoircd /usr/local/bin/neoircd
COPY deploy/docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh
USER neoirc
# No USER: the entrypoint starts as root to give /var/lib/neoirc to neoirc,
# then runs neoircd as neoirc.
EXPOSE 8080 6667
HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \
CMD wget -qO- http://localhost:8080/.well-known/healthcheck.json || exit 1
ENTRYPOINT ["neoircd"]
ENTRYPOINT ["/usr/local/bin/docker-entrypoint.sh"]
+92 -31
View File
@@ -168,8 +168,10 @@ either.
- Request bodies for `POST /api/v1/session`, `POST /api/v1/login` and
`POST /api/v1/messages` are limited to `MAX_MESSAGE_SIZE` bytes (default
`4096`). A larger body gets `400` `invalid request body`.
- The server allows cross-origin requests from any origin, with credentials. The
cookie is `SameSite=Strict`, so a browser sends it only from the same site.
- `GET /api/v1/server` and `GET /.well-known/healthcheck.json` answer a request
from any origin with `Access-Control-Allow-Origin: *`. Every other route sends
no `Access-Control-Allow-*` header, so a page on another origin cannot read
its responses.
### The auth cookie
@@ -1339,8 +1341,9 @@ Every session has a hostmask in IRC's form, `nick!username@hostname`:
from, or that IP address when the lookup gives no name. The lookup waits up to
three seconds.
Over the HTTP API, the server takes the client's IP address from the first entry
of the `X-Forwarded-For` header, then `X-Real-IP`, then the TCP connection (see
Over the HTTP API, the client's IP address is the address of the TCP connection.
When that connection comes from a trusted proxy, the server reads the address
from `X-Forwarded-For` or `X-Real-IP` instead (see
[Reverse proxy](#reverse-proxy)).
Every user can see a hostmask: in `RPL_WHOISUSER` (`311`), `RPL_WHOREPLY`
@@ -1450,10 +1453,9 @@ over the limit gets `429` `too many login attempts, try again later` with
attempt is more than 15 minutes old. Session creation has no rate limit; the
hashcash stamp is its cost.
The IP address comes from the first entry of `X-Forwarded-For`, else from
`X-Real-IP`, else from the TCP connection. Without a reverse proxy that
overwrites those headers, a client can set them itself and so pick any address
(see [Reverse proxy](#reverse-proxy)).
The IP address is the client's address as described in
[Reverse proxy](#reverse-proxy). A client outside the trusted proxies cannot
change it with `X-Forwarded-For` or `X-Real-IP`.
### Writing a client
@@ -1714,11 +1716,16 @@ Every setting is an environment variable. The server also reads a `.env` file in
its working directory, and a YAML file `neoirc.yaml` in `/etc/neoirc/` or
`$HOME/.config/neoirc/` with the same names as keys. A variable from the
environment wins over one in `.env`, and both win over `neoirc.yaml`. An
environment variable set to the empty string counts as unset, except
`IRC_LISTEN_ADDR`, where the empty string turns the IRC listener off. A
`neoirc.yaml` that is not valid YAML stops the server at startup. A setting that
does not parse or is out of range also stops the server at startup, and the
error names the setting.
environment variable set to the empty string counts as unset, with two
exceptions:
- For `IRC_LISTEN_ADDR`, the empty string turns the IRC listener off.
- For `TRUSTED_PROXIES`, the empty string trusts no proxy.
A `neoirc.yaml` that is not valid YAML stops the server at startup. A setting
that does not parse or is out of range also stops the server at startup, and the
error names the setting. The same holds for a setting given a list or a map in
`neoirc.yaml`.
- `PORT` (default `8080`): the HTTP port. `0` lets the system pick a free port.
- `IRC_LISTEN_ADDR` (default `:6667`): the address of the IRC listener. The
@@ -1747,6 +1754,11 @@ error names the setting.
address. A fraction such as `0.5` is allowed. It must be above `0`.
- `LOGIN_RATE_BURST` (default `5`): how many attempts an address may make at
once before the rate applies. It must be at least `1`.
- `TRUSTED_PROXIES` (default `10.0.0.0/8,172.16.0.0/12,192.168.0.0/16`): the
reverse proxies whose `X-Forwarded-For` and `X-Real-IP` headers the server
believes. It is a comma-separated list of networks, such as `10.0.0.0/8`, and
single addresses, such as `192.0.2.1`. A list replaces the default entirely.
The empty string trusts nothing. See [Reverse proxy](#reverse-proxy).
- `SESSION_IDLE_TIMEOUT` (default `720h`, 30 days): a duration with a unit, `h`
for hours, `m` for minutes or `s` for seconds, such as `720h` or `36h30m`. A
client with no request for this long is removed, and a session left with no
@@ -1781,13 +1793,17 @@ NEOIRC_HASHCASH_BITS=20
the linter first and stops when either fails. The image:
- is Alpine with the `neoircd` binary and CA certificates;
- runs as the user `neoirc`;
- runs `neoircd` as the user `neoirc`;
- exposes ports 8080 and 6667;
- keeps its database in `/var/lib/neoirc`;
- has a `HEALTHCHECK` that calls `/.well-known/healthcheck.json` every 30
seconds.
Run it with a volume for the database and the settings as environment variables:
The image prepares `/var/lib/neoirc` itself on every start. It creates the
directory, gives it to `neoirc` and then runs `neoircd` as `neoirc`.
Run it with a named volume for the database and the settings as environment
variables:
```bash
docker run -d --name neoirc \
@@ -1798,20 +1814,51 @@ docker run -d --name neoirc \
neoirc
```
A host directory works the same way in place of the named volume:
```bash
docker run -d --name neoirc \
-p 8080:8080 -p 6667:6667 \
-v /srv/neoirc:/var/lib/neoirc \
-e SERVER_NAME="My Server" \
-e MOTD="Welcome!" \
neoirc
```
### Reverse proxy
Run the server behind a reverse proxy that terminates TLS. The server itself
speaks plain HTTP, and the auth cookie is `Secure`, so browsers send it only
over HTTPS. The proxy must:
- Set `X-Forwarded-For` to the client's address, replacing any value the client
sent. The server reads that header first and trusts it from anyone; it reads
`X-Real-IP` only when `X-Forwarded-For` is absent. Without a proxy that
overwrites the header, a client can choose its own address, which sets its
hostname and lets it dodge the login rate limit.
- Connect from an address in `TRUSTED_PROXIES`. The default trusts the private
ranges `10.0.0.0/8`, `172.16.0.0/12` and `192.168.0.0/16`. The setting must
hold the address the server sees the proxy connect from. For a proxy on the
same host:
- With the server outside Docker, that address is `127.0.0.1` or `::1`. The
default does not trust them, so set `TRUSTED_PROXIES=127.0.0.1,::1`.
- Through a published Docker port, that address is the Docker bridge
gateway, such as `172.17.0.1`. The default trusts it.
- Add the client's address to `X-Forwarded-For`.
- Wait at least 60 seconds for a response. A long poll can take 30 seconds, and
the server's own write timeout is 60 seconds.
The server finds the client's address in these steps:
- A connection from outside `TRUSTED_PROXIES` is the client. The server ignores
its `X-Forwarded-For` and `X-Real-IP` headers.
- For a connection from a trusted proxy, the server reads `X-Forwarded-For` from
right to left. It skips trusted addresses and takes the first untrusted one.
When every entry is trusted, it takes the leftmost.
- Without `X-Forwarded-For`, the server takes `X-Real-IP`.
- When an entry it reads is not an IP address, the server uses the proxy's
address.
The client's address sets the session's hostname and counts toward the login
rate limit. Any connection from a trusted address can choose its client address
through `X-Forwarded-For`. If clients connect directly from private addresses,
set `TRUSTED_PROXIES` to the proxy's address alone.
Caddy:
```
@@ -1848,22 +1895,34 @@ The IRC listener has no TLS.
seconds. A request still running after 60 seconds is cancelled, and the
connection closes without a response. Headers may be up to 1 MiB.
- A long poll waits at most 30 seconds.
- Every response to a well-formed request, except to a cross-origin preflight,
carries a `Content-Security-Policy` header:
- Every response to a well-formed request carries a `Content-Security-Policy`
header:
`default-src 'self'; script-src 'self'; style-src 'self'; connect-src 'self'; img-src 'self'; font-src 'self'; object-src 'none'; frame-ancestors 'none'; base-uri 'self'; form-action 'self'`.
The web client works within it.
- Cross-origin requests are allowed from any origin, with credentials. The
allowed methods are `GET`, `POST`, `PUT`, `DELETE` and `OPTIONS`. The allowed
request headers are `Accept`, `Content-Type` and `X-CSRF-Token`. The cookie's
`SameSite=Strict` still keeps it to the same site.
- `GET /api/v1/server` and `GET /.well-known/healthcheck.json` answer a request
from any origin with `Access-Control-Allow-Origin: *`. Every other route sends
no `Access-Control-Allow-*` header, so a page on another origin cannot read
its responses.
### Logging
The server logs with Go's `slog`, one line per event, to standard output. The
lines are JSON, or plain text when standard output is a terminal. Every HTTP
request is logged with its method, URL, protocol, status, latency, remote
address, user agent, referrer and request ID. `DEBUG=true` adds debug-level
lines.
request is logged with:
- start time
- method
- URL
- protocol
- status
- latency
- client address
- user agent
- referrer
- request ID
The client address is the one found as described in
[Reverse proxy](#reverse-proxy). `DEBUG=true` adds debug-level lines.
### The database
@@ -1946,8 +2005,8 @@ and their sessions to end. Then it closes the database.
gets a message.
- `internal/hashcash/`: stamp checking and minting.
- `internal/ratelimit/`: the per-address login rate limiter.
- `internal/middleware/`: request logging, cross-origin requests, the
`Content-Security-Policy` header and metrics.
- `internal/middleware/`: finding the client's address, request logging,
cross-origin requests, the `Content-Security-Policy` header and metrics.
- `internal/healthcheck/` and `internal/stats/`: the health check and its
counters.
- `internal/logger/` and `internal/globals/`: logging setup and build
@@ -1959,6 +2018,8 @@ and their sessions to end. Then it closes the database.
- `schema/`: JSON Schema files for the message envelope, with their own
`README.md`.
- `script/`: the entrypoints below.
- `deploy/`: `docker-entrypoint.sh`, the image's entrypoint, which prepares
`/var/lib/neoirc` and starts `neoircd` as `neoirc`.
- `CONVENTIONS.md`, `REPO_POLICIES.md` and `AGENTS.md`: the architecture
conventions, the repository policies and the rules for coding agents.
+14
View File
@@ -0,0 +1,14 @@
#!/bin/sh
# deploy/docker-entrypoint.sh: the image's ENTRYPOINT, run as root. A host
# directory mounted at /var/lib/neoirc keeps the host's owner, so this gives
# it to neoirc before running neoircd as neoirc.
set -eu
main() {
mkdir -p /var/lib/neoirc
chown -R neoirc:neoirc /var/lib/neoirc
chmod 0700 /var/lib/neoirc
exec su-exec neoirc neoircd "$@"
}
main "$@"
+2 -1
View File
@@ -13,8 +13,10 @@ require (
github.com/prometheus/client_golang v1.23.2
github.com/rivo/tview v0.42.0
github.com/slok/go-http-metrics v0.13.0
github.com/spf13/cast v1.10.0
github.com/spf13/viper v1.21.0
go.uber.org/fx v1.24.0
go4.org/netipx v0.0.0-20260823151212-3075585bcbeb
golang.org/x/crypto v0.48.0
golang.org/x/time v0.6.0
modernc.org/sqlite v1.45.0
@@ -40,7 +42,6 @@ require (
github.com/sagikazarmark/locafero v0.11.0 // indirect
github.com/sourcegraph/conc v0.3.1-0.20240121214520-5f936abd7ae8 // indirect
github.com/spf13/afero v1.15.0 // indirect
github.com/spf13/cast v1.10.0 // indirect
github.com/spf13/pflag v1.0.10 // indirect
github.com/subosito/gotenv v1.6.0 // indirect
go.uber.org/dig v1.19.0 // indirect
+2
View File
@@ -111,6 +111,8 @@ go.yaml.in/yaml/v2 v2.4.2 h1:DzmwEr2rDGHl7lsFgAHxmNz/1NlQ7xLIrlN2h5d1eGI=
go.yaml.in/yaml/v2 v2.4.2/go.mod h1:081UH+NErpNdqlCXm3TtEran0rJZGxAYx9hb/ELlsPU=
go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc=
go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg=
go4.org/netipx v0.0.0-20260823151212-3075585bcbeb h1:XBM4hvfwGAttkkiTIFfeigdfcL1xIfdKXqFdgiHGtDs=
go4.org/netipx v0.0.0-20260823151212-3075585bcbeb/go.mod h1:PLyyIXexvUFg3Owu6p/WfdlivPbZJsZdgWZlrGope/Y=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
golang.org/x/crypto v0.48.0 h1:/VRzVqiRSggnhY7gNRxPauEQ5Drw9haKdM0jqfcCFts=
+183 -42
View File
@@ -6,12 +6,16 @@ import (
"fmt"
"log/slog"
"math"
"net/netip"
"os"
"strconv"
"strings"
"time"
"github.com/spf13/cast"
"github.com/spf13/viper"
"go.uber.org/fx"
"go4.org/netipx"
"sneak.berlin/go/neoirc/internal/globals"
"sneak.berlin/go/neoirc/internal/logger"
@@ -32,8 +36,16 @@ Type /help for available commands.`
const DefaultDBURL = "file:///var/lib/neoirc/state.db" +
"?_pragma=journal_mode(WAL)&_pragma=busy_timeout(5000)"
// DefaultTrustedProxies is TRUSTED_PROXIES when it is not set: the
// private address ranges of RFC 1918.
const DefaultTrustedProxies = "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"
var errOutOfRange = errors.New("setting out of range")
var errNeverMatches = errors.New("entry can never match a connection")
var errListOrMap = errors.New("a list or a map")
// Params defines the dependencies for creating a Config.
type Params struct {
fx.In
@@ -64,13 +76,15 @@ type Config struct {
LoginRateLimit float64
LoginRateBurst int
IRCListenAddr string
TrustedProxies *netipx.IPSet
params *Params
log *slog.Logger
}
// New creates a new Config by reading from files and environment variables.
// It returns an error when a number, true/false or duration setting does
// not parse or is out of range.
// It returns an error when the config file does not parse, when the file
// gives a setting a list or a map, and when a number, true/false,
// duration or address setting does not parse or is out of range.
func New(
_ fx.Lifecycle, params Params,
) (*Config, error) {
@@ -103,32 +117,20 @@ func New(
viper.SetDefault("LOGIN_RATE_LIMIT", "1")
viper.SetDefault("LOGIN_RATE_BURST", "5")
viper.SetDefault("IRC_LISTEN_ADDR", ":6667")
viper.SetDefault("TRUSTED_PROXIES", DefaultTrustedProxies)
err := viper.ReadInConfig()
if err != nil {
var notFound viper.ConfigFileNotFoundError
if !errors.As(err, &notFound) {
log.Error("config file malformed", "error", err)
panic(err)
return nil, fmt.Errorf("cannot read %s: %w",
viper.ConfigFileUsed(), err)
}
}
cfg := &Config{
DBURL: viper.GetString("DBURL"),
SentryDSN: viper.GetString("SENTRY_DSN"),
MetricsUsername: viper.GetString("METRICS_USERNAME"),
MetricsPassword: viper.GetString("METRICS_PASSWORD"),
MessageMaxAge: viper.GetString("MESSAGE_MAX_AGE"),
QueueMaxAge: viper.GetString("QUEUE_MAX_AGE"),
MOTD: viper.GetString("MOTD"),
ServerName: viper.GetString("SERVER_NAME"),
FederationKey: viper.GetString("FEDERATION_KEY"),
SessionIdleTimeout: viper.GetString("SESSION_IDLE_TIMEOUT"),
OperName: viper.GetString("NEOIRC_OPER_NAME"),
OperPassword: viper.GetString("NEOIRC_OPER_PASSWORD"),
IRCListenAddr: viper.GetString("IRC_LISTEN_ADDR"),
log: log,
params: &params,
log: log,
params: &params,
}
err = parseSettings(cfg)
@@ -136,13 +138,6 @@ func New(
return nil, err
}
// Viper counts an empty environment variable as unset, but an
// empty IRC_LISTEN_ADDR must turn the IRC listener off.
ircListenAddr, set := os.LookupEnv("IRC_LISTEN_ADDR")
if set {
cfg.IRCListenAddr = ircListenAddr
}
if cfg.Debug {
params.Logger.EnableDebugLogging()
cfg.log = params.Logger.Get()
@@ -151,13 +146,24 @@ func New(
return cfg, nil
}
// parseSettings reads the number and true/false settings into cfg and
// checks the duration settings. Each number and duration must be above
// zero. PORT and NEOIRC_HASHCASH_BITS may also be zero: port 0 lets the
// system pick a port, and 0 bits turns proof of work off. The error
// names each setting that does not parse or is out of range.
// parseSettings reads every setting into cfg. Each number and duration
// must be above zero. PORT and NEOIRC_HASHCASH_BITS may also be zero:
// port 0 lets the system pick a port, and 0 bits turns proof of work off.
// The error names each setting that is a list or a map, does not parse or
// is out of range.
func parseSettings(cfg *Config) error {
return errors.Join(
parseString("DBURL", &cfg.DBURL),
parseString("SENTRY_DSN", &cfg.SentryDSN),
parseString("METRICS_USERNAME", &cfg.MetricsUsername),
parseString("METRICS_PASSWORD", &cfg.MetricsPassword),
parseString("MOTD", &cfg.MOTD),
parseString("SERVER_NAME", &cfg.ServerName),
parseString("FEDERATION_KEY", &cfg.FederationKey),
parseString("NEOIRC_OPER_NAME", &cfg.OperName),
parseString("NEOIRC_OPER_PASSWORD", &cfg.OperPassword),
// The empty string turns the IRC listener off.
parseStringAllowEmptyEnv("IRC_LISTEN_ADDR", &cfg.IRCListenAddr),
parseInt("PORT", 0, &cfg.Port),
parseInt("MAX_MESSAGE_SIZE", 1, &cfg.MaxMessageSize),
parseInt("NEOIRC_HASHCASH_BITS", 0, &cfg.HashcashBits),
@@ -165,16 +171,136 @@ func parseSettings(cfg *Config) error {
parseFloat("LOGIN_RATE_LIMIT", &cfg.LoginRateLimit),
parseBool("DEBUG", &cfg.Debug),
parseBool("MAINTENANCE_MODE", &cfg.MaintenanceMode),
checkDuration("MESSAGE_MAX_AGE", cfg.MessageMaxAge),
checkDuration("QUEUE_MAX_AGE", cfg.QueueMaxAge),
checkDuration("SESSION_IDLE_TIMEOUT", cfg.SessionIdleTimeout),
parseDuration("MESSAGE_MAX_AGE", &cfg.MessageMaxAge),
parseDuration("QUEUE_MAX_AGE", &cfg.QueueMaxAge),
parseDuration("SESSION_IDLE_TIMEOUT", &cfg.SessionIdleTimeout),
parseTrustedProxies(&cfg.TrustedProxies),
)
}
// getString returns the setting key as a string. A number or a true/false
// value in the config file becomes its text, as with viper.GetString. A
// list or a map in the config file is an error, where viper.GetString
// would return the empty string. The error leaves out the value, which
// may be a password.
func getString(key string) (string, error) {
raw, err := cast.ToStringE(viper.Get(key))
if err != nil {
return "", fmt.Errorf("%s is %w", key, errListOrMap)
}
return raw, nil
}
// parseString sets *value to the setting key.
func parseString(key string, value *string) error {
raw, err := getString(key)
if err != nil {
return err
}
*value = raw
return nil
}
// parseStringAllowEmptyEnv sets *value to the setting key, as parseString
// does, but an environment variable set to the empty string counts as
// set. Viper counts such a variable as unset and uses the config file or
// the default instead.
func parseStringAllowEmptyEnv(key string, value *string) error {
fromEnv, set := os.LookupEnv(key)
if set {
*value = fromEnv
return nil
}
return parseString(key, value)
}
// parseTrustedProxies sets *value to the addresses in TRUSTED_PROXIES.
func parseTrustedProxies(value **netipx.IPSet) error {
var raw string
err := parseStringAllowEmptyEnv("TRUSTED_PROXIES", &raw)
if err != nil {
return err
}
ipSet, err := ParseTrustedProxies(raw)
if err != nil {
return fmt.Errorf("cannot parse TRUSTED_PROXIES: %w", err)
}
*value = ipSet
return nil
}
// ParseTrustedProxies returns the addresses in list, which has the form
// of TRUSTED_PROXIES: a comma-separated list of networks such as
// 10.0.0.0/8 and single addresses such as 192.0.2.1. The empty string
// trusts nothing.
func ParseTrustedProxies(list string) (*netipx.IPSet, error) {
var builder netipx.IPSetBuilder
if strings.TrimSpace(list) != "" {
for entry := range strings.SplitSeq(list, ",") {
err := addTrustedProxy(&builder, strings.TrimSpace(entry))
if err != nil {
return nil, err
}
}
}
return builder.IPSet()
}
// addTrustedProxy adds entry, a network with a slash or a single
// address, to builder. An IPv4 address written in IPv6 form, such as
// ::ffff:192.0.2.1, is added as IPv4. A network in that form and an
// address with a zone are errors, since no connection could match them.
func addTrustedProxy(builder *netipx.IPSetBuilder, entry string) error {
if strings.Contains(entry, "/") {
prefix, err := netip.ParsePrefix(entry)
if err != nil {
return err //nolint:wrapcheck // the caller names the setting
}
// A connection from an IPv4 address is checked in IPv4 form.
if prefix.Addr().Is4In6() {
return fmt.Errorf("%w: %s is an IPv4 network in IPv6 form",
errNeverMatches, entry)
}
builder.AddPrefix(prefix)
return nil
}
addr, err := netip.ParseAddr(entry)
if err != nil {
return err //nolint:wrapcheck // the caller names the setting
}
// IPSet.Contains is false for every address with a zone.
if addr.Zone() != "" {
return fmt.Errorf("%w: %s has a zone", errNeverMatches, entry)
}
builder.Add(addr.Unmap())
return nil
}
// parseInt sets *value to the setting key read as a whole number. It
// returns an error when the value does not parse or is below minimum.
func parseInt(key string, minimum int, value *int) error {
raw := viper.GetString(key)
raw, err := getString(key)
if err != nil {
return err
}
parsed, err := strconv.Atoi(raw)
if err != nil {
@@ -195,7 +321,10 @@ func parseInt(key string, minimum int, value *int) error {
// number. It returns an error when the value does not parse or is not a
// finite number above zero.
func parseFloat(key string, value *float64) error {
raw := viper.GetString(key)
raw, err := getString(key)
if err != nil {
return err
}
parsed, err := strconv.ParseFloat(raw, 64)
if err != nil {
@@ -215,7 +344,12 @@ func parseFloat(key string, value *float64) error {
// parseBool sets *value to the setting key read as true or false.
func parseBool(key string, value *bool) error {
parsed, err := strconv.ParseBool(viper.GetString(key))
raw, err := getString(key)
if err != nil {
return err
}
parsed, err := strconv.ParseBool(raw)
if err != nil {
return fmt.Errorf("cannot parse %s: %w", key, err)
}
@@ -225,18 +359,25 @@ func parseBool(key string, value *bool) error {
return nil
}
// checkDuration returns an error naming the setting key when value is
// not a Go duration above zero.
func checkDuration(key, value string) error {
duration, err := time.ParseDuration(value)
// parseDuration sets *value to the setting key. It returns an error when
// the value is not a Go duration above zero.
func parseDuration(key string, value *string) error {
raw, err := getString(key)
if err != nil {
return err
}
duration, err := time.ParseDuration(raw)
if err != nil {
return fmt.Errorf("cannot parse %s: %w", key, err)
}
if duration <= 0 {
return fmt.Errorf("%w: %s is %s, but must be above zero",
errOutOfRange, key, value)
errOutOfRange, key, raw)
}
*value = raw
return nil
}
+233 -2
View File
@@ -1,11 +1,14 @@
package config_test
import (
"net/netip"
"os"
"path/filepath"
"strings"
"testing"
"time"
"github.com/spf13/viper"
"sneak.berlin/go/neoirc/internal/config"
"sneak.berlin/go/neoirc/internal/globals"
"sneak.berlin/go/neoirc/internal/logger"
@@ -140,8 +143,7 @@ func settingsOf(cfg *config.Config) settings {
}
func TestBadSettingStopsStartup(t *testing.T) {
// For each setting, the first value does not parse and the others
// parse but are out of range.
// Each value is invalid for its setting.
tests := []struct {
key string
values []string
@@ -158,6 +160,10 @@ func TestBadSettingStopsStartup(t *testing.T) {
{key: "MESSAGE_MAX_AGE", values: []string{"30d", "0", "-1h"}},
{key: "QUEUE_MAX_AGE", values: []string{"720", "0s", "-24h"}},
{key: "SESSION_IDLE_TIMEOUT", values: []string{"a month", "0", "-1m"}},
{key: "TRUSTED_PROXIES", values: []string{
"proxy.example.com", "10.0.0.0/33", "192.0.2.1:8080",
"::ffff:10.0.0.0/104", "fe80::1%eth0",
}},
}
for _, test := range tests {
@@ -206,3 +212,228 @@ func TestIRCListenAddrEmptyTurnsListenerOff(t *testing.T) {
t.Errorf("IRCListenAddr = %q, want empty", cfg.IRCListenAddr)
}
}
func TestTrustedProxies(t *testing.T) {
// Each case lists addresses that must be in the trusted set and
// addresses that must not.
tests := []struct {
name string
set bool
value string
trusted []string
untrusted []string
}{
{
name: "unset trusts the RFC 1918 ranges",
set: false,
value: "",
trusted: []string{
"10.1.2.3", "172.16.0.1", "172.31.255.254", "192.168.1.1",
},
untrusted: []string{
"127.0.0.1", "172.32.0.1", "192.0.2.1", "2001:db8::1",
},
},
{
name: "a value replaces the default",
set: true,
value: "198.51.100.0/24, 192.0.2.7,2001:db8::/32",
trusted: []string{"198.51.100.200", "192.0.2.7", "2001:db8::5"},
untrusted: []string{"10.4.5.6", "192.168.4.4", "192.0.2.8"},
},
{
name: "a mapped IPv6 address is trusted as IPv4",
set: true,
value: "::ffff:192.0.2.7",
trusted: []string{"192.0.2.7"},
untrusted: []string{"10.7.8.9"},
},
{
name: "the empty string trusts nothing",
set: true,
value: "",
trusted: nil,
untrusted: []string{"10.0.0.1", "192.168.0.1", "::1"},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
// t.Setenv puts back whatever value was there after the
// test, including when Unsetenv removes it.
t.Setenv("TRUSTED_PROXIES", test.value)
if !test.set {
err := os.Unsetenv("TRUSTED_PROXIES")
if err != nil {
t.Fatal(err)
}
}
proxies := newConfig(t).TrustedProxies
for _, addr := range test.trusted {
if !proxies.Contains(netip.MustParseAddr(addr)) {
t.Errorf("%s is not trusted", addr)
}
}
for _, addr := range test.untrusted {
if proxies.Contains(netip.MustParseAddr(addr)) {
t.Errorf("%s is trusted", addr)
}
}
})
}
}
// unsetEnv removes the environment variable key for the rest of the
// test, so that config.New reads key from the config file.
func unsetEnv(t *testing.T, key string) {
t.Helper()
// t.Setenv puts back whatever value was there after the test,
// including when Unsetenv removes it.
t.Setenv(key, "")
err := os.Unsetenv(key)
if err != nil {
t.Fatal(err)
}
}
// writeConfigFile writes content to the config file config.New reads
// when HOME is home, and returns the file's path.
func writeConfigFile(t *testing.T, home, content string) string {
t.Helper()
// loadConfig names the app neoirc-test, so config.New reads
// $HOME/.config/neoirc-test/neoirc-test.yaml.
dir := filepath.Join(home, ".config", "neoirc-test")
err := os.MkdirAll(dir, 0o700)
if err != nil {
t.Fatal(err)
}
path := filepath.Join(dir, "neoirc-test.yaml")
err = os.WriteFile(path, []byte(content), 0o600)
if err != nil {
t.Fatal(err)
}
// Viper keeps the file and the values it read from it after the
// file is gone. Reset clears them for later tests.
t.Cleanup(viper.Reset)
return path
}
func TestListOrMapInConfigFileStopsStartup(t *testing.T) {
// YAML reads the first value as a list and the second as a map.
values := []string{`[":6667"]`, `{addr: ":6667"}`}
keys := []string{"IRC_LISTEN_ADDR", "MOTD", "TRUSTED_PROXIES"}
for _, key := range keys {
for _, value := range values {
t.Run(key+"="+value, func(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
unsetEnv(t, key)
writeConfigFile(t, home, key+": "+value+"\n")
_, err := loadConfig(t)
if err == nil || !strings.Contains(err.Error(), key) {
t.Errorf("error = %v, want one naming %s", err, key)
}
})
}
}
}
func TestListOrMapErrorLeavesOutValue(t *testing.T) {
const key = "NEOIRC_OPER_PASSWORD"
home := t.TempDir()
t.Setenv("HOME", home)
unsetEnv(t, key)
// YAML reads an unquoted {hunter2} as a map.
writeConfigFile(t, home, key+": {hunter2}\n")
_, err := loadConfig(t)
if err == nil || !strings.Contains(err.Error(), key) {
t.Fatalf("error = %v, want one naming %s", err, key)
}
if strings.Contains(err.Error(), "hunter2") {
t.Errorf("error %q shows the password", err)
}
}
func TestEmptyEnvironmentValueWinsOverConfigFile(t *testing.T) {
const (
ircListenAddr = "IRC_LISTEN_ADDR"
trustedProxies = "TRUSTED_PROXIES"
)
// Each setting is the empty string in the environment and a list in
// the config file. The environment wins, so the list is never read.
tests := []struct {
key string
empty func(cfg *config.Config) bool
}{
{
key: ircListenAddr,
empty: func(cfg *config.Config) bool {
return cfg.IRCListenAddr == ""
},
},
{
key: trustedProxies,
empty: func(cfg *config.Config) bool {
return len(cfg.TrustedProxies.Prefixes()) == 0
},
},
}
for _, test := range tests {
t.Run(test.key, func(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv(test.key, "")
writeConfigFile(t, home, test.key+": [\":6667\"]\n")
cfg := newConfig(t)
if !test.empty(cfg) {
t.Errorf("%s is not the empty string", test.key)
}
})
}
}
func TestSingleValuesInConfigFileAreUsed(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
unsetEnv(t, "PORT")
unsetEnv(t, "DEBUG")
unsetEnv(t, "MOTD")
writeConfigFile(t, home, "PORT: 9000\nDEBUG: true\nMOTD: hello\n")
cfg := newConfig(t)
if cfg.Port != 9000 || !cfg.Debug || cfg.MOTD != "hello" {
t.Errorf("Port, Debug, MOTD = %d, %t, %q, want 9000, true, %q",
cfg.Port, cfg.Debug, cfg.MOTD, "hello")
}
}
func TestConfigFileThatDoesNotParseStopsStartup(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
path := writeConfigFile(t, home, "MOTD: [unclosed\n")
_, err := loadConfig(t)
if err == nil || !strings.Contains(err.Error(), path) {
t.Errorf("error = %v, want one naming %s", err, path)
}
}
+85 -1
View File
@@ -10,6 +10,7 @@ import (
"time"
"go.uber.org/fx/fxtest"
"golang.org/x/crypto/bcrypt"
"sneak.berlin/go/neoirc/internal/config"
"sneak.berlin/go/neoirc/internal/db"
"sneak.berlin/go/neoirc/internal/dbtest"
@@ -26,7 +27,11 @@ const (
(SELECT 1 UNION ALL SELECT x + 1 FROM n)
SELECT COUNT(*) FROM n`
endlessQueryTimeout = 100 * time.Millisecond
// endlessQueryTimeout is how long the endless query runs
// before its context ends. A query whose context ends before
// it starts is never run, and the pool keeps its connection,
// so the timeout leaves a loaded host time to start it.
endlessQueryTimeout = 2 * time.Second
)
// TestDefaultDBURLSetsWALAndBusyTimeout opens a file database
@@ -236,6 +241,85 @@ func TestDBURLSettingForeignKeysFailsStartup(t *testing.T) {
}
}
// TestPasswordCost checks the bcrypt cost of the hash that SetPassword
// stores: bcrypt.DefaultCost when db.Params leaves PasswordCost unset,
// as the server does, and PasswordCost when it is set.
func TestPasswordCost(t *testing.T) {
t.Parallel()
tests := []struct {
name string
passwordCost int
wantCost int
}{
{name: "unset", wantCost: bcrypt.DefaultCost},
{
name: "bcrypt.MinCost",
passwordCost: bcrypt.MinCost,
wantCost: bcrypt.MinCost,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
t.Parallel()
lifecycle := fxtest.NewLifecycle(t)
log, err := logger.New(lifecycle, logger.Params{
Globals: &globals.Globals{},
})
if err != nil {
t.Fatal(err)
}
database, err := db.New(lifecycle, db.Params{
Logger: log,
Config: &config.Config{DBURL: dbtest.NewURL(t)},
PasswordCost: test.passwordCost,
})
if err != nil {
t.Fatal(err)
}
lifecycle.RequireStart()
t.Cleanup(lifecycle.RequireStop)
sessionID, _, _, err := database.CreateSession(
t.Context(), "costuser", "", "", "",
)
if err != nil {
t.Fatal(err)
}
err = database.SetPassword(t.Context(), sessionID, "password123")
if err != nil {
t.Fatal(err)
}
var hash string
err = database.GetDB().QueryRowContext(
t.Context(),
"SELECT password_hash FROM sessions WHERE id = ?",
sessionID,
).Scan(&hash)
if err != nil {
t.Fatal(err)
}
cost, err := bcrypt.Cost([]byte(hash))
if err != nil {
t.Fatal(err)
}
if cost != test.wantCost {
t.Errorf("cost = %d, want %d", cost, test.wantCost)
}
})
}
}
// startDatabase starts a Database for dbURL the way the server does.
// It returns the error that stopped it from starting, if any.
func startDatabase(t *testing.T, dbURL string) (*db.Database, error) {
+2 -18
View File
@@ -42,25 +42,9 @@ func (hdlr *Handlers) maxBodySize() int64 {
return defaultMaxBodySize
}
// clientIP extracts the connecting client's IP address
// from the request, checking X-Forwarded-For and
// X-Real-IP headers before falling back to RemoteAddr.
// clientIP returns the IP address in request.RemoteAddr, which the
// ClientIP middleware has set to the client's address.
func clientIP(request *http.Request) string {
if forwarded := request.Header.Get("X-Forwarded-For"); forwarded != "" {
// X-Forwarded-For can contain a comma-separated list;
// the first entry is the original client.
parts := strings.SplitN(forwarded, ",", 2) //nolint:mnd
ip := strings.TrimSpace(parts[0])
if ip != "" {
return ip
}
}
if realIP := request.Header.Get("X-Real-IP"); realIP != "" {
return strings.TrimSpace(realIP)
}
host, _, err := net.SplitHostPort(request.RemoteAddr)
if err != nil {
return request.RemoteAddr
+202 -37
View File
@@ -145,24 +145,46 @@ type testServer struct {
handlers *handlers.Handlers
}
const (
// maxMessageSize is MAX_MESSAGE_SIZE on every test server, the
// setting's default.
maxMessageSize = 4096
// loginRateLimit is LOGIN_RATE_LIMIT on every test server, the
// setting's default.
loginRateLimit = 1
// loginRateBurst is LOGIN_RATE_BURST on every test server, the
// setting's default.
loginRateBurst = 5
)
func newTestServer(
t *testing.T,
) *testServer {
t.Helper()
return newTestServerWith(t, 0, "", "")
return newTestServerWith(t, 0, "", "", config.DefaultTrustedProxies)
}
func newTestServerWith(
t *testing.T,
hashcashBits int,
operName, operPassword string,
operName, operPassword, trustedProxies string,
) *testServer {
t.Helper()
dbURL := dbtest.NewURL(t)
copyTemplateDB(t, dbURL)
trusted, err := config.ParseTrustedProxies(trustedProxies)
if err != nil {
t.Fatalf("parse trusted proxies: %v", err)
}
// Each duration setting defaults to 30 days.
const thirtyDays = "720h"
var (
srv *server.Server
brk *broker.Broker
@@ -176,17 +198,22 @@ func newTestServerWith(
logger.New,
// Built here rather than by config.New, which writes
// the global viper instance, so servers start in
// parallel. Settings left out take their defaults.
// parallel.
func() *config.Config {
return &config.Config{
DBURL: dbURL,
// One line: each line is a database write per new session.
MOTD: "neoirc test server",
HashcashBits: hashcashBits,
OperName: operName,
OperPassword: operPassword,
MessageMaxAge: "720h",
QueueMaxAge: "720h",
MOTD: "neoirc test server",
MaxMessageSize: maxMessageSize,
HashcashBits: hashcashBits,
OperName: operName,
OperPassword: operPassword,
MessageMaxAge: thirtyDays,
QueueMaxAge: thirtyDays,
SessionIdleTimeout: thirtyDays,
LoginRateLimit: loginRateLimit,
LoginRateBurst: loginRateBurst,
TrustedProxies: trusted,
}
},
newTestDB,
@@ -809,10 +836,7 @@ func (tserver *testServer) postBody(
func TestRequestBodyLimit(t *testing.T) {
t.Parallel()
const (
maxMessageSize = 4096 // the default MAX_MESSAGE_SIZE
nick = "alice_bodylimit"
)
const nick = "alice_bodylimit"
endpoints := []struct {
name, path, value string
@@ -3017,18 +3041,18 @@ func TestGetMessagesRemovesWaitChannel(t *testing.T) {
longTimeout = 10
// shortTimeout, in seconds, is the timeout of the
// requests that wait it out.
// request that waits it out.
shortTimeout = 1
)
// queued is whether a message is queued before the request,
// whose timeout is in seconds. failingRead is as for
// getMessagesFailingRead. wantReads is how many times the
// handler reads the queue: once if it answers at once,
// whose timeout is in seconds. failingRead and notified are
// as for getMessagesFailingRead. wantReads is how many times
// the handler reads the queue: once if it answers at once,
// twice if it waits first.
tests := []struct {
name string
queued bool
queued, notified bool
timeout, failingRead int
wantReads, wantStatus int
}{
@@ -3049,9 +3073,9 @@ func TestGetMessagesRemovesWaitChannel(t *testing.T) {
wantReads: 1, wantStatus: http.StatusInternalServerError,
},
{
name: "read after the wait failed", timeout: shortTimeout,
failingRead: 2, wantReads: 2,
wantStatus: http.StatusInternalServerError,
name: "read after the wait failed", notified: true,
timeout: handlers.MaxLongPollTimeout, failingRead: 2,
wantReads: 2, wantStatus: http.StatusInternalServerError,
},
}
@@ -3083,7 +3107,7 @@ func TestGetMessagesRemovesWaitChannel(t *testing.T) {
}
status, reads := tserver.getMessagesFailingRead(
token, lastID, tc.timeout, tc.failingRead,
token, sessionID, lastID, tc.timeout, tc.failingRead, tc.notified,
)
if status != tc.wantStatus {
t.Errorf("status %d, want %d", status, tc.wantStatus)
@@ -3100,14 +3124,17 @@ func TestGetMessagesRemovesWaitChannel(t *testing.T) {
}
// getMessagesFailingRead sends GET /api/v1/messages for the
// client token with timeout and after afterID. The handler's
// read of the queue numbered failingRead, counting from 1,
// fails; with 0, none does. It returns the response status
// and how many times the handler read the queue.
// client token of the session sessionID, with timeout and
// after afterID. The handler's read of the queue numbered
// failingRead, counting from 1, fails; with 0, none does.
// With notified, it notifies the broker once the request
// waits. It returns the response status and how many times
// the handler read the queue.
func (tserver *testServer) getMessagesFailingRead(
token string,
afterID int64,
sessionID, afterID int64,
timeout, failingRead int,
notified bool,
) (int, int) {
tserver.t.Helper()
@@ -3129,20 +3156,45 @@ func (tserver *testServer) getMessagesFailingRead(
)
})
resp, err := doRequestAuth(
tserver.t, http.MethodGet,
tserver.url(fmt.Sprintf(
"%s?timeout=%d&after=%d", apiMessages, timeout, afterID,
)),
token, nil,
var (
status int
err error
)
answered := make(chan struct{})
go func() {
defer close(answered)
var resp *http.Response
resp, err = doRequestAuth(
tserver.t, http.MethodGet,
tserver.url(fmt.Sprintf(
"%s?timeout=%d&after=%d", apiMessages, timeout, afterID,
)),
token, nil,
)
if err != nil {
return
}
status = resp.StatusCode
_ = resp.Body.Close()
}()
if notified {
tserver.waitForLongPoll(sessionID)
tserver.broker.Notify(sessionID)
}
<-answered
if err != nil {
tserver.t.Fatalf("get messages: %v", err)
}
_ = resp.Body.Close()
return resp.StatusCode, reads
return status, reads
}
// TestLongPollRemovesWaitChannelWhenClientGivesUp checks that
@@ -4289,6 +4341,116 @@ func TestLoginRateLimitExceeded(t *testing.T) {
}
}
func TestLoginRateLimitIgnoresForwardedHeaders(t *testing.T) {
t.Parallel()
// The test client connects from 127.0.0.1, which is outside the
// default trusted proxies, so the server must ignore these headers.
tests := []struct {
name string
header string
}{
{name: "X-Forwarded-For", header: "X-Forwarded-For"},
{name: "X-Real-IP", header: "X-Real-IP"},
}
loginBody, err := json.Marshal(map[string]string{
nickKey: "unknownnick", passwordKey: "wrongpassword",
})
if err != nil {
t.Fatal(err)
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
t.Parallel()
tserver := newTestServer(t)
// The default burst is 5 attempts, so the sixth gets 429
// although each claims a new address.
var status int
for attempt := range 6 {
request, rErr := http.NewRequestWithContext(
t.Context(), http.MethodPost,
tserver.url("/api/v1/login"),
bytes.NewReader(loginBody),
)
if rErr != nil {
t.Fatal(rErr)
}
request.Header.Set("Content-Type", "application/json")
request.Header.Set(
test.header, fmt.Sprintf("203.0.113.%d", attempt+1),
)
resp, dErr := http.DefaultClient.Do(request)
if dErr != nil {
t.Fatal(dErr)
}
status = resp.StatusCode
_ = resp.Body.Close()
}
if status != http.StatusTooManyRequests {
t.Fatalf("sixth attempt got %d, want 429", status)
}
})
}
}
func TestLoginRateLimitPerClientBehindTrustedProxy(t *testing.T) {
t.Parallel()
// The test client connects from 127.0.0.1. Trusted as a proxy, it
// passes each client's address in X-Forwarded-For.
tserver := newTestServerWith(t, 0, "", "", "127.0.0.1")
// An empty JSON object is refused after the rate limit counts the
// attempt, so no attempt checks a password or looks up a hostname.
loginFrom := func(forwardedFor string) int {
t.Helper()
request, err := http.NewRequestWithContext(
t.Context(), http.MethodPost,
tserver.url("/api/v1/login"), strings.NewReader("{}"),
)
if err != nil {
t.Fatal(err)
}
request.Header.Set("Content-Type", "application/json")
request.Header.Set("X-Forwarded-For", forwardedFor)
resp, err := http.DefaultClient.Do(request)
if err != nil {
t.Fatal(err)
}
_ = resp.Body.Close()
return resp.StatusCode
}
// The default burst is 5 attempts per address.
for range 5 {
loginFrom("203.0.113.1")
}
status := loginFrom("203.0.113.1")
if status != http.StatusTooManyRequests {
t.Errorf("sixth attempt from 203.0.113.1 got %d, want 429", status)
}
status = loginFrom("203.0.113.2")
if status == http.StatusTooManyRequests {
t.Error("first attempt from 203.0.113.2 got 429")
}
}
func TestLoginRateLimitAllowsNormalUse(t *testing.T) {
t.Parallel()
@@ -4976,7 +5138,9 @@ func TestRefusedSessionLeavesStampUnspent(t *testing.T) {
badNick = "bad nick"
)
tserver := newTestServerWith(t, bits, "", "")
tserver := newTestServerWith(
t, bits, "", "", config.DefaultTrustedProxies,
)
// A server with no SERVER_NAME takes stamps for "neoirc".
status := postSessionWithStamp(
@@ -5182,6 +5346,7 @@ func newTestServerWithOper(
return newTestServerWith(
t, 0, testOperName, testOperPassword,
config.DefaultTrustedProxies,
)
}
+4
View File
@@ -6,6 +6,10 @@ import (
"sneak.berlin/go/neoirc/internal/db"
)
// MaxLongPollTimeout is the longest HandleGetMessages waits, in
// seconds.
const MaxLongPollTimeout = maxLongPollTimeout
// SetPollMessages makes HandleGetMessages read the client
// output queue with poll in place of Database.PollMessages.
func (hdlr *Handlers) SetPollMessages(
+35 -10
View File
@@ -31,7 +31,10 @@ import (
_ "modernc.org/sqlite"
)
const testTimeout = 5 * time.Second
// testTimeout is how long a test waits for something before it
// fails. A passing test never waits this long, so it is generous
// enough for a heavily loaded host.
const testTimeout = 30 * time.Second
// testPollInterval is how often the relay of a test server
// checks the client output queue without a broker
@@ -565,27 +568,49 @@ func TestQuietClientAnsweringPingStaysConnected(t *testing.T) {
func TestQuietClientNotAnsweringPingIsClosed(t *testing.T) {
t.Parallel()
// latest leaves room for a loaded host, but is sooner than
// a read deadline of two ping intervals would close it.
// A read deadline of two ping intervals would close the
// connection a second after latest. The ping interval is
// longer than testPingInterval, so that the time between
// the expected close and latest leaves room for a loaded
// host.
const (
earliest = testPingInterval + testPongDeadline
latest = 2 * testPingInterval
interval = 4 * time.Second
earliest = interval + testPongDeadline
latest = 2*interval - time.Second
)
env := newTestEnvWithShortPings(t)
env := newTestEnvWithPingTimes(
t,
&config.Config{ServerName: testServerName},
interval, testPongDeadline,
)
client := env.dial(t)
lastSent := time.Now()
client.register("silent")
client.readServerPing(lastSent)
// The server starts both waits once it has handled the
// registration, so the time registration took does not
// count against latest.
registered := time.Now()
client.readUntil(isServerPing)
client.readUntilClosed()
waited := time.Since(lastSent)
if waited < earliest || waited >= latest {
if waited < earliest {
t.Errorf(
"closed %v after the client's last line, want %v to %v",
waited, earliest, latest,
"closed %v after the client's last line, want at least %v",
waited, earliest,
)
}
sinceRegistered := time.Since(registered)
if sinceRegistered >= latest {
t.Errorf(
"closed %v after registration, want less than %v",
sinceRegistered, latest,
)
}
}
+80 -10
View File
@@ -5,6 +5,9 @@ import (
"log/slog"
"net"
"net/http"
"net/netip"
"slices"
"strings"
"time"
basicauth "github.com/99designs/basicauth-go"
@@ -15,6 +18,7 @@ import (
"github.com/slok/go-http-metrics/middleware/std"
"github.com/spf13/viper"
"go.uber.org/fx"
"go4.org/netipx"
"sneak.berlin/go/neoirc/internal/config"
"sneak.berlin/go/neoirc/internal/globals"
"sneak.berlin/go/neoirc/internal/logger"
@@ -62,6 +66,77 @@ func ipFromHostPort(hostPort string) string {
return host
}
// ClientIP returns middleware that replaces request.RemoteAddr with the
// client's address and keeps the port of the TCP connection. The
// X-Forwarded-For and X-Real-IP headers count only when the connection
// comes from an address in TRUSTED_PROXIES.
func (mware *Middleware) ClientIP() func(http.Handler) http.Handler {
trusted := mware.params.Config.TrustedProxies
return func(next http.Handler) http.Handler {
return http.HandlerFunc(
func(
writer http.ResponseWriter,
request *http.Request,
) {
peer, err := netip.ParseAddrPort(request.RemoteAddr)
if err == nil {
client := clientAddr(
peer.Addr().Unmap(), request.Header, trusted,
)
request.RemoteAddr = netip.AddrPortFrom(
client, peer.Port(),
).String()
}
next.ServeHTTP(writer, request)
})
}
}
// clientAddr returns the client's address for a request from peer. A
// peer outside trusted is the client, whatever its headers say. A
// trusted peer is a proxy. Its X-Forwarded-For list is read from the
// right, past the trusted addresses, to the first untrusted one; when
// all are trusted, the client is the leftmost. Without X-Forwarded-For,
// the client is X-Real-IP. An entry that is not an IP address gives
// peer.
func clientAddr(
peer netip.Addr, header http.Header, trusted *netipx.IPSet,
) netip.Addr {
if !trusted.Contains(peer) {
return peer
}
forwarded := strings.Join(header.Values("X-Forwarded-For"), ",")
if forwarded == "" {
realIP, err := netip.ParseAddr(
strings.TrimSpace(header.Get("X-Real-IP")),
)
if err != nil {
return peer
}
return realIP.Unmap()
}
client := peer
for _, entry := range slices.Backward(strings.Split(forwarded, ",")) {
addr, err := netip.ParseAddr(strings.TrimSpace(entry))
if err != nil {
return peer
}
client = addr.Unmap()
if !trusted.Contains(client) {
break
}
}
return client
}
type loggingResponseWriter struct {
http.ResponseWriter
@@ -125,24 +200,19 @@ func (mware *Middleware) Logging() func(http.Handler) http.Handler {
}
}
// CORS returns middleware that handles Cross-Origin Resource Sharing.
// AllowCredentials is true so browsers include cookies in
// cross-origin API requests.
// CORS returns middleware that lets a page on any origin read the
// response. Use it only on public, read-only routes.
func (mware *Middleware) CORS() func(http.Handler) http.Handler {
return cors.Handler(cors.Options{ //nolint:exhaustruct // optional fields
AllowOriginFunc: func(
_ *http.Request, _ string,
) bool {
return true
},
AllowedOrigins: []string{"*"},
AllowedMethods: []string{
"GET", "POST", "PUT", "DELETE", "OPTIONS",
},
AllowedHeaders: []string{
"Accept", "Content-Type", "X-CSRF-Token",
"Accept", "Authorization", "Content-Type", "X-CSRF-Token",
},
ExposedHeaders: []string{"Link"},
AllowCredentials: true,
AllowCredentials: false,
MaxAge: corsMaxAge,
})
}
+189
View File
@@ -0,0 +1,189 @@
package middleware_test
import (
"net/http"
"net/http/httptest"
"net/netip"
"testing"
"time"
"go4.org/netipx"
"sneak.berlin/go/neoirc/internal/config"
"sneak.berlin/go/neoirc/internal/globals"
"sneak.berlin/go/neoirc/internal/logger"
"sneak.berlin/go/neoirc/internal/middleware"
)
// The tests trust 10.0.0.0/8. Clients use the documentation ranges
// 192.0.2.0/24, 198.51.100.0/24 and 203.0.113.0/24.
const (
trustedNetwork = "10.0.0.0/8"
proxyPeer = "10.0.0.1:4000"
directPeer = "192.0.2.1:4000"
client = "203.0.113.7"
clientWithPort = "203.0.113.7:4000"
realIPClient = "203.0.113.8"
)
type clientIPTest struct {
name string
remoteAddr string
forwarded []string
realIP string
want string
}
func clientIPTests() []clientIPTest {
return []clientIPTest{
{
name: "untrusted peer ignores forged headers",
remoteAddr: directPeer,
forwarded: []string{client},
realIP: realIPClient,
want: directPeer,
},
{name: "trusted peer without headers", remoteAddr: proxyPeer, want: proxyPeer},
{
name: "trusted peer gives rightmost untrusted entry",
remoteAddr: proxyPeer,
forwarded: []string{"198.51.100.9, 203.0.113.7, 10.0.0.2"},
want: clientWithPort,
},
{
name: "entries split over several headers",
remoteAddr: proxyPeer,
forwarded: []string{"198.51.100.9", client, "10.0.0.2"},
want: clientWithPort,
},
{
name: "all-trusted chain gives leftmost entry",
remoteAddr: proxyPeer,
forwarded: []string{"10.0.0.3, 10.0.0.2"},
want: "10.0.0.3:4000",
},
{
name: "X-Real-IP without X-Forwarded-For",
remoteAddr: proxyPeer,
realIP: realIPClient,
want: "203.0.113.8:4000",
},
{
name: "X-Forwarded-For wins over X-Real-IP",
remoteAddr: proxyPeer,
forwarded: []string{client},
realIP: realIPClient,
want: clientWithPort,
},
{
name: "unparseable X-Forwarded-For entry gives peer",
remoteAddr: proxyPeer,
forwarded: []string{"203.0.113.7, not-an-ip, 10.0.0.2"},
want: proxyPeer,
},
{
name: "unparseable entry past the client is not read",
remoteAddr: proxyPeer,
forwarded: []string{"not-an-ip, 203.0.113.7"},
want: clientWithPort,
},
{
name: "unparseable X-Real-IP gives peer",
remoteAddr: proxyPeer,
realIP: "not-an-ip",
want: proxyPeer,
},
{
name: "mapped IPv6 peer is unmapped and trusted",
remoteAddr: "[::ffff:10.0.0.1]:4000",
forwarded: []string{client},
want: clientWithPort,
},
{
name: "mapped IPv6 peer outside the set",
remoteAddr: "[::ffff:192.0.2.1]:4000",
forwarded: []string{client},
want: directPeer,
},
{
name: "IPv6 client",
remoteAddr: proxyPeer,
forwarded: []string{"2001:db8::1"},
want: "[2001:db8::1]:4000",
},
}
}
func TestClientIP(t *testing.T) {
t.Parallel()
mware := newMiddleware(t, trustedNetwork)
for _, test := range clientIPTests() {
t.Run(test.name, func(t *testing.T) {
t.Parallel()
var got string
handler := mware.ClientIP()(http.HandlerFunc(func(
_ http.ResponseWriter, request *http.Request,
) {
got = request.RemoteAddr
}))
request := httptest.NewRequestWithContext(
t.Context(), http.MethodGet, "/", nil,
)
request.RemoteAddr = test.remoteAddr
for _, value := range test.forwarded {
request.Header.Add("X-Forwarded-For", value)
}
if test.realIP != "" {
request.Header.Set("X-Real-IP", test.realIP)
}
handler.ServeHTTP(httptest.NewRecorder(), request)
if got != test.want {
t.Errorf("RemoteAddr = %q, want %q", got, test.want)
}
})
}
}
// newMiddleware returns a Middleware that trusts the network trusted.
func newMiddleware(t *testing.T, trusted string) *middleware.Middleware {
t.Helper()
var builder netipx.IPSetBuilder
builder.AddPrefix(netip.MustParsePrefix(trusted))
trustedSet, err := builder.IPSet()
if err != nil {
t.Fatal(err)
}
globs := &globals.Globals{
Appname: "neoirc-test",
Version: "test",
StartTime: time.Now(),
}
log, err := logger.New(nil, logger.Params{Globals: globs})
if err != nil {
t.Fatal(err)
}
mware, err := middleware.New(nil, middleware.Params{
Logger: log,
Globals: globs,
Config: &config.Config{TrustedProxies: trustedSet},
})
if err != nil {
t.Fatal(err)
}
return mware
}
+3 -3
View File
@@ -23,13 +23,13 @@ func (srv *Server) SetupRoutes() {
srv.router.Use(middleware.Recoverer)
srv.router.Use(middleware.RequestID)
srv.router.Use(srv.mw.ClientIP())
srv.router.Use(srv.mw.Logging())
if viper.GetString("METRICS_USERNAME") != "" {
srv.router.Use(srv.mw.Metrics())
}
srv.router.Use(srv.mw.CORS())
srv.router.Use(srv.mw.CSP())
srv.router.Use(middleware.Timeout(routeTimeout))
@@ -44,7 +44,7 @@ func (srv *Server) SetupRoutes() {
}
// Health check.
srv.router.Get(
srv.router.With(srv.mw.CORS()).Get(
"/.well-known/healthcheck.json",
srv.handlers.HandleHealthCheck(),
)
@@ -68,7 +68,7 @@ func (srv *Server) SetupRoutes() {
}
func (srv *Server) setupAPIv1(router chi.Router) {
router.Get(
router.With(srv.mw.CORS()).Get(
"/server",
srv.handlers.HandleServerInfo(),
)
+84
View File
@@ -6,8 +6,10 @@ package server_test
import (
"fmt"
"maps"
"net"
"net/http"
"net/http/httptest"
"strconv"
"strings"
"testing"
@@ -188,6 +190,88 @@ func TestServeAfterStopDoesNotStopApp(t *testing.T) {
}
}
// TestCORSHeaders checks the Access-Control-Allow-* headers on the
// answers to requests from another origin. The public, read-only
// routes answer with Access-Control-Allow-Origin: * alone. Routes
// that need the cookie, and a preflight to one, answer with none.
func TestCORSHeaders(t *testing.T) {
var srv *server.Server
startApp(t, fx.Populate(&srv))
tests := []struct {
name string
method string
path string
requestMethod string
wantHeaders map[string]string
}{
{
name: "server information",
method: http.MethodGet,
path: "/api/v1/server",
wantHeaders: map[string]string{"Access-Control-Allow-Origin": "*"},
},
{
name: "health check",
method: http.MethodGet,
path: "/.well-known/healthcheck.json",
wantHeaders: map[string]string{"Access-Control-Allow-Origin": "*"},
},
{
name: "send",
method: http.MethodPost,
path: "/api/v1/messages",
},
{
name: "state",
method: http.MethodGet,
path: "/api/v1/state",
},
{
name: "preflight for send",
method: http.MethodOptions,
path: "/api/v1/messages",
requestMethod: http.MethodPost,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
request := httptest.NewRequestWithContext(
t.Context(), test.method,
"http://chat.example.com"+test.path, nil,
)
request.Header.Set("Origin", "http://blog.example.com")
if test.requestMethod != "" {
request.Header.Set(
"Access-Control-Request-Method", test.requestMethod,
)
}
recorder := httptest.NewRecorder()
srv.ServeHTTP(recorder, request)
gotHeaders := map[string]string{}
for name := range recorder.Header() {
if strings.HasPrefix(name, "Access-Control-Allow-") {
gotHeaders[name] = recorder.Header().Get(name)
}
}
if !maps.Equal(gotHeaders, test.wantHeaders) {
t.Errorf(
"Access-Control-Allow-* headers are %v, want %v",
gotHeaders, test.wantHeaders,
)
}
})
}
}
// startApp starts the app newApp builds, with the HTTP port
// set to 0, so the system gives the server a free port.
func startApp(