From f192c26fa2e3ee7f03f100b925912601ccdd1706e5a7d7c19b57554f56e251d1 Mon Sep 17 00:00:00 2001 From: Lixen Wraith Date: Fri, 2 Oct 2026 04:59:11 -0400 Subject: [PATCH] v0.20.0 mtls hardening, fail-closed config, startup warnings, sink fixes --- AGENTS.md | 89 ++++++++ README.md | 7 +- cmd/logwisp/reload_test.go | 29 +++ config/logwisp.toml | 11 +- doc/README.md | 1 + doc/configuration.md | 4 +- doc/mtls-auth-plan.md | 17 +- doc/networking.md | 8 +- doc/operations.md | 3 +- doc/scram-auth-plan.md | 243 ++++++++++++++++++++++ doc/security.md | 43 ++-- doc/sinks.md | 4 +- go.mod | 6 +- go.sum | 12 +- internal/authz/authz.go | 70 +++++++ internal/authz/authz_test.go | 19 ++ internal/config/loader_test.go | 41 +++- internal/config/validate.go | 56 +++++ internal/sink/console/console.go | 4 +- internal/sink/file/file.go | 4 +- internal/sink/http/http.go | 15 +- internal/sink/http/http_test.go | 50 +++++ internal/sink/httpchain/httpchain.go | 14 +- internal/sink/httpchain/httpchain_test.go | 47 +++++ internal/sink/null/null.go | 6 +- internal/sink/tcp/tcp.go | 43 ++-- internal/sink/tcp/tcp_test.go | 59 ++++++ internal/sink/tcpchain/tcpchain.go | 36 +--- internal/source/console/console.go | 5 +- internal/source/file/file.go | 2 +- internal/source/httpchain/httpchain.go | 17 +- internal/source/null/null.go | 6 +- internal/source/random/random.go | 4 +- internal/source/tcpchain/tcpchain.go | 17 +- internal/testutil/fixtures.go | 71 +++++++ internal/tlsx/tlsx.go | 97 ++++++++- internal/tlsx/tlsx_test.go | 34 +++ 37 files changed, 1042 insertions(+), 152 deletions(-) create mode 100644 AGENTS.md create mode 100644 doc/scram-auth-plan.md create mode 100644 internal/sink/httpchain/httpchain_test.go create mode 100644 internal/sink/tcp/tcp_test.go create mode 100644 internal/tlsx/tlsx_test.go diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..cf7be3b --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,89 @@ +# Working rules + +It is the contract, not a suggestion. + +## Comments + +- A comment block is **at most 5 lines**. If the explanation needs more, the code + is wrong or the explanation belongs in `doc/`. +- One line is the default. Comment *why*, never *what* — the code says what. +- No narrative, no history ("this used to…", "the plan proposed…"), no restating + the diff, no essays on design philosophy. Git log and `doc/` hold those. +- No comment on a self-evident function. `// Lookup returns the address` above + `func Lookup` is noise. +- Package doc blocks: 5 lines. Files do not get their own prologue. + +## Files + +- Do not create a file for one or a few helpers. Put them next to what uses them, + or, if shared, in the package's main or type file. +- A new file needs a new concept, not a new function. +- Prefer editing an existing file over adding one. +- When you edit a file, bring its comments up to these rules; older files predate + them. + +## Scope + +- Implement what was asked. Do not add tables, indirection, telemetry, tests or + abstraction that nothing asked for. + Exception: a task that explicitly invites additions gets the obvious ones, each + named in the PR body; ask before any that is not obvious. +- One mechanism per problem. Two mechanisms doing one job is a bug or a refactor + opportunity. +- Delete before adding. If a change is net-positive lines for a fix, justify it. +- Reverting an existing API to "improve" it is not a fix. Leave working code alone. +- Per-kind data lives in one table indexed by its kind; a new kind adds a row, + never a switch or a parallel field. + +## Tests + +- One test per rule, named for the rule. No test that restates another. +- Test comments follow the 5-line limit. +- Do not pin lists that a human has to hand-maintain unless the pin prevents a + real regression. +- Collapse tests: when a broader test covers a narrower one, delete the narrower + one or do not add it. No test for simple behaviour that is unlikely to fail or + is already exercised elsewhere. + +## Docs and commits + +- `doc/` is already long. Condense when you touch it; append only for a new concept + or scope. +- A gap you are deferring goes in the "Not Implemented" list of the design doc it + belongs to: extend the item, or add a concise one in the list's pattern. +- The repository describes the architecture, never a machine: docs and commands + use placeholders for addresses, node names and credentials. +- PR bodies: what changed, why, how it was verified. +- One task, one branch, one PR. Divide the work into commits on that branch; do not + split it across stacked PRs or multiple branches. +- Work that continues after its PR merged starts again from the latest main; never + stack new commits on merged history. + +## Gates + +`go build ./...`, `gofmt -l` on changed files, and `go test` on the packages the +change touches. +- A change to platform-sensitive code also cross-builds the production target: + `GOOS=freebsd GOARCH=amd64 go build ./cmd/logwisp`. +- Do not run `-race` tests unless investigating a known/suspected race issue; CI + runs them. + +## LogWisp + +- Plugin options decode through `config.Scan`, never `lconfig.ScanMap`: unknown + keys must fail at any depth. Set defaults on the options struct before the + call; `TestEveryPluginRejectsUnknownKeys` covers every registered plugin. +- Network security has two seams: `tlsx` (TLS configs, startup warnings) and + `authz` (every admission decision). Both return nil when disabled and every + method tolerates a nil receiver; keep call sites branch-free on config. +- A plugin starts goroutines in `Start`, never in its constructor: a rejected + reload discards constructed plugins without calling `Stop`. Reload is the only + rotation path for certificates and secrets. +- The chain `Hello` is the protocol's extension point: optional fields only (old + peers ignore unknown JSON); anything a peer must understand bumps + `chain.ProtocolVersion`. +- Listeners and dialers are IPv4-only (`tcp4`). E2E scripts in `test/` need + `bin/logwisp` and `--auto`, and each owns a port range and a gitignored run + directory. +- Many older files lack a trailing newline and fail `gofmt -l`; format the files + you change, not the tree. diff --git a/README.md b/README.md index 79b6e5b..8967bb4 100644 --- a/README.md +++ b/README.md @@ -64,11 +64,13 @@ exponential backoff and jitter. stream and status endpoints, and lets a dialer pin the server it talks to - Node binding: a chain source can label entries from the sender's certificate instead of from what the sender claims, so origin attribution is not forgeable +- Fail-closed configuration: unknown keys are rejected, and startup warns about + expiring certificates, disabled verification and unanchored allow patterns See [Security](doc/security.md) for configuration and the exact boundary, and the [mTLS authentication design](doc/mtls-auth-plan.md) for the rationale and -what is deliberately left out. Password, token, and SCRAM authentication were -removed during the restructure and are not currently available. +what is deliberately left out. Password authentication (Argon2id-SCRAM) is +planned; see the [SCRAM design](doc/scram-auth-plan.md). ## Documentation @@ -85,6 +87,7 @@ removed during the restructure and are not currently available. | [Networking](doc/networking.md) | Listeners, dialers, timeouts, connection limits | | [Security](doc/security.md) | TLS, mTLS, and peer authorization; threat model and current limits | | [mTLS Authentication](doc/mtls-auth-plan.md) | Design and rationale for certificate-based authorization | +| [Password Authentication](doc/scram-auth-plan.md) | Planned Argon2id-SCRAM design; mTLS hardening | | [CLI](doc/cli.md) | Flags, signals, exit codes | | [Operations](doc/operations.md) | Running, monitoring, tuning, troubleshooting | diff --git a/cmd/logwisp/reload_test.go b/cmd/logwisp/reload_test.go index fc4d727..467e512 100644 --- a/cmd/logwisp/reload_test.go +++ b/cmd/logwisp/reload_test.go @@ -3,8 +3,12 @@ package main import ( "context" "path/filepath" + "strings" "testing" + "time" + "logwisp/internal/plugin" + "logwisp/internal/session" "logwisp/internal/testutil" lconfig "github.com/lixenwraith/config" @@ -77,3 +81,28 @@ func TestInvalidReloadLeavesStatusReporterRunning(t *testing.T) { }) } } + +// Every registered plugin decodes through the checked decoder, so a typo in +// any plugin's config, security tables included, refuses to start. +func TestEveryPluginRejectsUnknownKeys(t *testing.T) { + testLogger(t) + manager := session.NewManager(time.Hour) + t.Cleanup(manager.Stop) + proxy := session.NewProxy(manager, "typo") + bad := map[string]any{"no_such_key": true} + check := func(kind, name string, err error) { + if err == nil || !strings.Contains(err.Error(), "no_such_key") { + t.Errorf("%s %q accepted an unknown key: %v", kind, name, err) + } + } + for _, name := range plugin.ListSources() { + factory, _ := plugin.GetSource(name) + _, err := factory("typo", bad, logger, proxy) + check("source", name, err) + } + for _, name := range plugin.ListSinks() { + factory, _ := plugin.GetSink(name) + _, err := factory("typo", bad, logger, proxy) + check("sink", name, err) + } +} diff --git a/config/logwisp.toml b/config/logwisp.toml index 5d97b26..1b40fc4 100644 --- a/config/logwisp.toml +++ b/config/logwisp.toml @@ -6,11 +6,12 @@ ### Commented values are the built-in defaults unless marked "example". ### Uncommenting a default is a no-op. ### -### NOTE: password/token/SCRAM authentication, network access control (ACL), -### http/tcp ingest sources, and http_client/tcp_client sinks were removed in -### the restructure and are not available in this version. TLS and mTLS ARE -### available on every network source and sink; see the [...tls] blocks below, -### and the [...auth] blocks to authorize peers by certificate identity. +### NOTE: password authentication (planned: doc/scram-auth-plan.md), network +### access control (ACL), http/tcp ingest sources, and http_client/tcp_client +### sinks are not available in this version. TLS and mTLS ARE available on +### every network source and sink; see the [...tls] blocks below, and the +### [...auth] blocks to authorize peers by certificate identity. +### Unknown keys in any plugin config are rejected at startup. ### ### Environment overrides use the LOGWISP_ prefix ### (LOGWISP_QUIET, LOGWISP_LOGGING_LEVEL, ...). Bare names are ignored. diff --git a/doc/README.md b/doc/README.md index 9e67ce2..3002900 100644 --- a/doc/README.md +++ b/doc/README.md @@ -20,6 +20,7 @@ streams, or downstream LogWisp nodes. | [Networking](networking.md) | Listeners, dialers, timeouts, connection limits | | [Security](security.md) | TLS, mTLS, and peer authorization; threat model and current limits | | [mTLS Authentication](mtls-auth-plan.md) | Design and rationale for certificate-based authorization | +| [Password Authentication](scram-auth-plan.md) | Planned Argon2id-SCRAM design; mTLS hardening | | [CLI](cli.md) | Flags, signals, exit codes | | [Operations](operations.md) | Running, monitoring, tuning, troubleshooting | diff --git a/doc/configuration.md b/doc/configuration.md index 0f435ad..30be787 100644 --- a/doc/configuration.md +++ b/doc/configuration.md @@ -241,7 +241,9 @@ Startup validation is intentionally split. Everything else is validated by the plugin constructor that owns it — port range, required paths, path prefixes, enum values, regex compilation, TLS file loading. A failure there aborts pipeline construction with a message naming the -pipeline, plugin id, and offending key. +pipeline, plugin id, and offending key. A key the plugin does not declare, at +any depth, is such a failure (`unknown key "tls.enabeld"`): a misspelled option +must not silently fall back to its default. There is **no** cross-pipeline port-conflict detection. Two sinks bound to the same port fail at listener bind time, when the pipeline starts. diff --git a/doc/mtls-auth-plan.md b/doc/mtls-auth-plan.md index a44f2d9..1be8382 100644 --- a/doc/mtls-auth-plan.md +++ b/doc/mtls-auth-plan.md @@ -48,10 +48,10 @@ back to authenticated peers. ## Non-Goals -- Reviving password, token, or SCRAM authentication. The reserved hooks - (`chain.Hello.Auth`, the `Authorization` header comment in the `http_chain` - sink, the "Future: password auth block" comments in the network options - structs) stay reserved. +- Password authentication. It is designed separately in + [scram-auth-plan.md](scram-auth-plan.md), reusing the `auth` block and the + reserved hooks (`chain.Hello`, the `Authorization` header in the `http_chain` + sink, the "Future: password auth block" comments in the options structs). - IP allow/deny lists and per-peer rate limits. Related, but a separate feature with its own config surface. - OCSP. See [Revocation](#revocation) for what is done instead. @@ -341,16 +341,13 @@ The remaining phase-4 items, in rough order of value: operators with existing CRL infrastructure. The allow-list covers the same ground with fewer moving parts, so this is only worth doing for a fleet whose revocation already flows through a CRL. -2. **Certificate expiry warnings** at startup and on reload — a leaf expiring - inside 30 days logged at WARN. Nothing warns today; expiry shows up as a - handshake failure. -3. **Per-identity rate limits.** The natural follow-on now that identity exists, +2. **Per-identity rate limits.** The natural follow-on now that identity exists, and the natural home for the per-IP limiting that was also removed. Kept out of scope here so this feature stayed reviewable. -4. **Per-client identity in the `http` sink's status output.** The endpoint +3. **Per-client identity in the `http` sink's status output.** The endpoint reports the policy and counters, but not which identities are currently connected; session metadata has the data. -5. **A list of `identity` modes** (try `san_uri`, fall back to `cn`) for +4. **A list of `identity` modes** (try `san_uri`, fall back to `cn`) for heterogeneous PKI. A single mode is simpler and covers a uniform CA. ## Decisions Taken diff --git a/doc/networking.md b/doc/networking.md index 1e9e45a..53656df 100644 --- a/doc/networking.md +++ b/doc/networking.md @@ -140,7 +140,8 @@ retry. Browsers reconnect SSE streams automatically. TLS is enabled. Only `GET` is routed to the stream and status paths. Each event is framed as one `data:` line per newline in the payload, so multi-line entries stream intact. Response headers set `Cache-Control: no-cache`, -`X-Accel-Buffering: no`, and `Access-Control-Allow-Origin: *`. +`X-Accel-Buffering: no`, and, without an auth policy, +`Access-Control-Allow-Origin: *`. **TCP sink** — raw payload bytes, no framing added by the sink. Whether entries are newline-delimited depends on the formatter. @@ -150,6 +151,11 @@ headers, and entry encoding. ## Troubleshooting +**Plugin fails to start with `unknown key "..."`** +- The key is misspelled or belongs to another plugin type. Every plugin rejects + keys it does not declare, nested tables included, so a typo in `tls` or `auth` + cannot leave a protection silently off. + **Connection refused** - Confirm the pipeline started; a bind failure is logged at ERROR. - Confirm you are dialing IPv4. `localhost` may resolve to `::1`. diff --git a/doc/operations.md b/doc/operations.md index 2fa3d85..861bad3 100644 --- a/doc/operations.md +++ b/doc/operations.md @@ -281,7 +281,8 @@ openssl x509 -in /etc/logwisp/tls/relay.crt -noout -enddate ``` Certificates load at plugin construction, so rotation is: write the new files, -then `kill -HUP`. Automate the expiry check; nothing in LogWisp warns you. +then `kill -HUP`. Startup and every reload warn at WARN once a certificate is +within 30 days of expiry; automate the check anyway. **Access review** diff --git a/doc/scram-auth-plan.md b/doc/scram-auth-plan.md new file mode 100644 index 0000000..602d773 --- /dev/null +++ b/doc/scram-auth-plan.md @@ -0,0 +1,243 @@ +# Password Authentication (Argon2id-SCRAM) + +**Status:** planned. Ships in two batches: + +1. **Done:** `lixenwraith/auth` gained SCRAM channel binding, and the mTLS layer + was hardened (see [mTLS Hardening](#mtls-hardening)). +2. **Next:** the SCRAM integration described here, pinned to the tagged `auth` + release that carries channel binding. + +**Scope:** optional username/password authentication on every network plugin, +next to the existing certificate method, without weakening what mTLS gives. + +## Goals + +- Password authentication for chain links (`tcp_chain`, `http_chain`: sources + verify, sinks present) and for viewers of the `tcp` and `http` sinks. +- No password or password-equivalent on the wire, no KDF work on listeners, and + no credential relay through a TLS-terminating MITM. +- Default stays `auth.type = "none"`; existing configurations are unaffected. +- Config in TOML, credential management and viewer clients in a `logwisp auth` + CLI. + +## Non-Goals + +- Standard SASL SCRAM-SHA-256 interoperability. The `auth` exchange is + logwisp-to-logwisp (or logwisp CLI) only. +- Browser `EventSource` viewers under SCRAM: they cannot set `Authorization` or + run Argon2. mTLS remains the browser path. +- Multiple backends behind a non-sticky L7 balancer: handshake state and the + token key live in one process. + +## Mechanism + +`lixenwraith/auth` Argon2id-SCRAM. The listener stores a verifier per user: +`StoredKey = SHA-256(HMAC(K, "Client Key"))`, `ServerKey = HMAC(K, "Server Key")`, +`K = Argon2id(password, salt)`. The Argon2id digest itself is never stored: it is +the salted password, from which both keys derive. Verifying a proof is HMAC only; +Argon2 runs on the dialer or CLI (64 MiB, t=3, the `auth` defaults). + +**Channel binding.** Both peers pass `auth.WithChannelBinding(SHA-256(server leaf +DER))` at the proof step. The server hashes its configured certificate; the +client hashes the certificate it saw. A relay presenting any other certificate, +even one the CA trusts, makes the server reject the proof, so it never obtains a +link or a token. A mismatch is indistinguishable from a wrong password by design; +the client also sends its hash as a diagnostic field so the listener can log +"TLS interception or terminating proxy" and count it. TLS must terminate at +logwisp. + +**HTTP pinning.** HTTP auth takes two requests and every later request may use a +new pooled connection, so binding the exchange is not enough. After step 1 the +dialer pins the bound hash on its `VerifyConnection`: step 2 and every ingest +request must present the same leaf. A mismatch clears token and pin and +re-authenticates, which also covers certificate rotation. A token handed to curl +by the CLI is unpinned. + +## Configuration + +```toml +# listener (tcp_chain/http_chain source, tcp/http sink) +[pipelines.plugin_sources.config.auth] +type = "scram" +credentials_file = "/etc/logwisp/users.toml" +token_lifetime_ms = 900000 # http sink and http_chain source only +node_binding = "force" # chain sources; binds to the username + +# dialer (tcp_chain/http_chain sink) +[pipelines.plugin_sinks.config.auth] +type = "scram" +username = "edge-01" +password_file = "/etc/logwisp/edge-01.pass" +``` + +Validation at construction: + +- `scram` requires TLS; a dialer also forbids `insecure_skip_verify`. +- Listeners require `credentials_file` and reject `allow`/`allow_patterns`: the + credentials file is the allow list. Use one file per listener when listeners + admit different users. +- Dialers require `username` + `password_file`, and reject `credentials_file`, + `allow*` and `identity` (server pinning stays an mTLS feature). +- `identity` on a listener requires `tls.client_auth` and **binds the certificate + to the user**: the certificate's identity field must equal the SCRAM username, + so a peer needs its own certificate and its own password (PostgreSQL's + `clientcert=verify-full`). Without it the factors are independent: any + CA-issued certificate plus any valid password (`clientcert=verify-ca`). +- `token_lifetime_ms` only on HTTP listeners; default 15 minutes. +- `/auth` is reserved; no configured path may equal it. + +**Credentials file** (TOML, written by the CLI, mode 0600): + +```toml +decoy_key = "" +[[users]] +username = "edge-01" +salt = "" +argon_time = 3 +argon_memory = 65536 +argon_threads = 4 +stored_key = "" +server_key = "" +``` + +`decoy_key` keeps unknown-user challenges stable across restarts and edits, so +probing usernames reveals nothing. It is created by the first `add-user` and +preserved by every rewrite. An empty user set, a duplicate user, a missing +`decoy_key` or mixed KDF profiles are configuration errors. The file is read at +construction; the SCRAM server starts in the plugin's `Start` and stops in `Stop`, +so a rejected reload leaks nothing. + +## Wire Protocol + +`chain.Hello` gains `scram` (raw JSON, decoded by `authz`); the protocol version +stays 1 because the field is additive. Every message after the hello is one +`authStep`: `error`, `challenge`, `proof` (+ `binding`), `final` (+ `token`, +`expires_in` on HTTP). Pre-auth lines are capped at 4 KiB. + +**TCP** (`tcp_chain`, `tcp` sink): hello carrying the client-first message, +challenge, proof, final — all under one deadline (`hello_timeout_ms` on the +`tcp_chain` source, 10 s on the `tcp` sink and every dialer, with the dialer's +context able to cut it short). The final is written only after every admission +check (certificate binding, node binding); the stream then continues on the same +buffered reader. A source without SCRAM answers a hello carrying credentials with +`error: authentication not enabled`; only an old binary stays silent, and the +dialer's deadline covers that. + +**HTTP** (`http_chain` source, `http` sink): `POST /auth`, outside the auth +middleware, with its own 10 s read/write deadline and a 4 KiB body limit. + +| Step | Request body | Success | Failure | +|------|--------------|---------|---------| +| 1 | hello | `200` challenge | `429` throttled, `503` handshake table full, `400` malformed | +| 2 | proof | `200` final, token, expires_in | `401 {"error":"authentication failed"}` | + +Protected endpoints take `Authorization: Bearer `: missing or invalid is +`401` with `WWW-Authenticate: Bearer realm="logwisp"`, a certificate-binding miss +is `403`. Tokens are HS256 JWTs from `auth.NewJWT` with a random per-instance key, +so every reload revokes them. An SSE stream is checked at connect and outlives its +token; a reload ends it. + +**Dialer outcomes** (`http_chain` sink): every `/auth` failure is transient (the +batch is held under backoff); `404`/`405` from `/auth` logs "no auth endpoint: +older logwisp or auth.type is not scram"; an ingest `401` clears the token and is +retried; `403` drops the batch; a pin mismatch clears token and pin. Rejections +are logged at WARN once per change of state and counted in `auth_failures` / +`last_auth_error`. + +## Throttling + +Per remote IP (the socket address, never a forwarded header), on handshake +starts: a token bucket (burst 10, 1/s) refunded on success, and at most 4 +unfinished exchanges. The table is bounded and fails closed when full. Abandoned +TCP exchanges release their slot in the `auth` handshake table immediately. +Counters: `auth_allowed` and `auth_rejected` once per exchange outcome; +`auth_throttled`, `auth_busy` and `auth_binding_mismatch` separately. + +## The `authz` Seam + +Listeners never call `Authorize` directly any more; one entry point per transport +decides, and `Authorize` itself refuses under `scram` so a missed call site fails +closed. + +- `Admit(conn, cs, wantHello, timeout)` returns an admission (identity, hello, + reader) whose `Accept` or `Reject` writes the final message. +- `AuthorizeRequest(r)` covers mTLS certificates and bearer tokens; + `ServeAuth(w, r)` serves `/auth`. +- Dialers: `Greet(ctx, conn, node)` writes the hello (plain or SCRAM); + `Authenticate`, `SetAuthorization`, `ClearToken` and `VerifyConnection` serve + HTTP. +- `Start`/`Close` bracket the SCRAM server and are nil-safe like everything else. + +## CLI + +``` +logwisp auth add-user -credentials F -user U [-password-file P] [-generate] +logwisp auth remove-user -credentials F -user U +logwisp auth token -url https://host:port -user U -password-file P [TLS flags] +logwisp auth stream -addr host:port -user U -password-file P [TLS flags] +``` + +- `add-user` takes the password from `-password-file` when it exists; a new user + without one gets a generated 130-bit password, written to that file (0600) or + printed once. Rotating an existing user needs the file or `-generate`, so a + mistyped path cannot replace a password. The file's KDF profile and decoy key + are reused, the result is validated with the daemon's own loader, and the + rewrite (temp file in the same directory, rename) keeps the existing mode and + owner. +- `remove-user` refuses to remove the last user. +- Both print that changes apply on `SIGHUP`: `auto_reload` does not watch the + credentials file. +- `token` prints a bearer token for curl (`-H @<(...)` keeps it out of argv); + `stream` authenticates to a `tcp` sink and copies the stream to stdout. +- TLS flags: `-ca-file`, `-server-name`, `-cert-file`, `-key-file`. There is no + insecure flag. + +## Rollout and Rotation + +- Turning SCRAM on is per listener and cuts off unconfigured dialers. To migrate + without a gap, upgrade binaries, add a second SCRAM listener, move dialers, + then remove the old listener. +- Password rotation: `add-user -generate`, `SIGHUP` the listener, deploy the new + password file, `SIGHUP` the dialer. The dialer retries under backoff in + between. +- Revocation: `remove-user`, `SIGHUP`. All tokens die with the reload. + +## Verification + +Go tests, one per rule: binding (match, mismatch, one-sided, length) in `auth`; +in `authz` the validation table, credential loading, decoy stability across +edits, TCP exchange over `net.Pipe` (success, wrong password equals unknown +user, binding mismatch, missing hello, silent peer within the deadline, +abandoned exchange frees its slot, final and first entry in one write, assert +mismatch gets an error rather than a final), HTTP exchange on a TLS test server +(token accepted, garbage and foreign tokens `401`, forged final yields no token, +a different certificate after step 1 refused before the body is sent), the +limiter bound; `http_chain` sink re-authentication and outcome table; CLI +add/remove/rotate, mode preservation, last-user refusal, password trimming. + +`test/scram-chain-test.sh --auto` (ports 15821-15825): a relay with SCRAM on all +four listener types plus one `client_auth` + SCRAM port; authorized edges over +both chain transports; node labels forced to usernames; wrong password, unknown +user and no-auth edges deliver nothing; a `tcp` sink viewer through `logwisp auth +stream` held past the exchange deadline; a raw TLS client without a hello gets +nothing; the `http` sink through `logwisp auth token` and curl; missing and bad +tokens get `401`; `remove-user` + `SIGHUP` revokes token and login; throttling +last. The existing scripts keep passing. + +## mTLS Hardening + +Shipped with this plan, independent of SCRAM: + +- The `http_chain` sink no longer follows redirects. Go's client resends the + body on a 307/308, including from `https` to plaintext `http`. +- Its response drain is bounded (64 KiB). +- The `tcp` sink tracks connections from accept, so `Stop` and reloads no longer + wait out the 10 s handshake timeout of a silent peer. +- Unknown keys in any plugin's config, at any depth, fail construction: a typo + such as `[...tls] enabeld = true` used to switch protection off silently. +- Startup and every reload warn about certificates that expired, are not yet + valid, or expire within 30 days; about `insecure_skip_verify`; about + `allow_patterns` that are not anchored at both ends of every alternative; and, + once per path, about key files every local user can read. +- An `http` sink with an auth policy no longer sends + `Access-Control-Allow-Origin: *`. diff --git a/doc/security.md b/doc/security.md index dbc4f0a..ed2bf96 100644 --- a/doc/security.md +++ b/doc/security.md @@ -14,16 +14,16 @@ configure it, and — equally important — what it does not yet do. | Authorization from peer identity (allow-lists, node binding) | Implemented — see [The Auth Block](#the-auth-block) | | Authentication on the `http` sink's stream and status endpoints | Implemented, via the auth block | | Server identity pinning by dialers | Implemented, via the auth block | +| Startup warnings for expiring certificates and risky settings | Implemented — see [Startup Warnings](#startup-warnings) | +| Unknown configuration keys rejected | Implemented — a typo in `tls` or `auth` fails startup | | Certificate revocation lists (CRL) or OCSP | **Not implemented** — revoke by editing the allow-list | -| Password, token, or SCRAM authentication | **Removed**; not currently available | +| Password (Argon2id-SCRAM) authentication | **Planned** — see [scram-auth-plan.md](scram-auth-plan.md) | | IP allow/deny lists, per-IP connection or request limits | **Not implemented** | -Earlier releases carried basic-auth, bearer-token, and SCRAM authentication. -Those were removed during the move to the plugin/flow architecture and the -switch to standard-library networking. Certificates are the one credential the -transport still carries, so they are what authentication is built on: the `tls` -block establishes that a peer chains to your CA, and the `auth` block decides -which peers that CA vouches for may actually do what. +Certificates are the credential the transport already carries, so they are what +authentication is built on today: the `tls` block establishes that a peer chains +to your CA, and the `auth` block decides which peers that CA vouches for may +actually do what. ## The TLS Block @@ -79,6 +79,8 @@ Misconfiguration fails at plugin construction, before the pipeline starts: - a certificate or key that will not load, or a CA file containing no certificates - a `min_version` that is neither `"1.2"` nor `"1.3"` +- a key the plugin does not know, at any depth: `unknown key "tls.enabeld"`. A + misspelled option must not silently leave a protection switched off ## The Auth Block @@ -144,7 +146,8 @@ WARN msg="Auth policy admits any identity the configured CA vouches for" ``` Anchor your patterns. `allow_patterns = ["edge-\\d{2}"]` matches -`evil-edge-01-impostor`; `["^edge-\\d{2}$"]` does not. +`evil-edge-01-impostor`; `["^edge-\\d{2}$"]` does not. An unanchored pattern is +reported at startup. ### Node binding @@ -204,6 +207,18 @@ Misconfiguration fails at plugin construction, before the pipeline starts: Errors read like `auth: type "mtls" requires tls.client_auth`. +## Startup Warnings + +Some settings work but are usually mistakes. Each plugin reports them at WARN +when it is constructed, so startup and every reload repeat them: + +- a certificate in `cert_file`, `ca_file` or `client_ca_file` that has expired, + is not yet valid, or expires within 30 days +- `insecure_skip_verify` on a dialer +- an `allow_patterns` entry not anchored at both ends of every alternative + (`^a|b$` admits `a…` and `…b`) +- a `key_file` every local user can read (once per path per process) + ## Enabling mTLS ### 1. Generate a CA and certificates @@ -339,7 +354,7 @@ interface or front them with an authenticating proxy. | Surface | Exposure when `auth.type = "none"` | |---------|-----------------------------------| -| `http` sink `stream_path` | Full log stream, with `Access-Control-Allow-Origin: *`, so any browser origin can read it | +| `http` sink `stream_path` | Full log stream, with `Access-Control-Allow-Origin: *`, so any browser origin can read it (the header is omitted once an auth policy is set) | | `http` sink `status_path` | Host, port, TLS flag, uptime, client counts, throughput counters | | `tcp` sink | Full log stream to any client that connects | | `tcp_chain` / `http_chain` source | Ingest from any peer the CA vouches for, under any node label it claims | @@ -356,11 +371,12 @@ way to authenticate a plaintext listener. - Use a dedicated CA for LogWisp so its trust decisions stay independent. - Keep leaf lifetimes short (90–825 days) and automate renewal. -- Key files should be `0600` and owned by the service account. +- Key files should be `0600` and owned by the service account; a world-readable + one is reported at startup. - Rotation requires a reload (`SIGHUP`), because certificates are loaded once at plugin construction; there is no on-disk watch for certificate files. -- Check expiry yourself: `openssl x509 -in relay.crt -noout -enddate`. Nothing - warns before a certificate lapses; it surfaces as a handshake failure. +- Startup and every reload warn 30 days before a certificate lapses. Still + automate renewal; the warning only reaches someone reading the log. - Keep the identity field you authorize on stable across rotations. Reissuing a leaf with a different CN silently drops the peer out of the allow list. @@ -369,7 +385,8 @@ way to authenticate a plaintext listener. - Prefer `min_version = "1.3"`. Drop to `"1.2"` only for a peer that genuinely cannot do 1.3. - Never enable `insecure_skip_verify` outside a lab; it disables server - verification entirely and makes the connection trivially interceptable. + verification entirely and makes the connection trivially interceptable. It is + reported at startup. - Bind listeners to specific interfaces rather than `0.0.0.0` where you can. - On any ingest port reachable from a network you do not fully control, set `auth.type = "mtls"` with an explicit `allow` list. `trust_node = false` is the diff --git a/doc/sinks.md b/doc/sinks.md index 9636393..9d02a19 100644 --- a/doc/sinks.md +++ b/doc/sinks.md @@ -178,7 +178,7 @@ counters. > Without an `auth` block both endpoints are unauthenticated, and the stream > response carries `Access-Control-Allow-Origin: *`, so any web origin can read -> it. Set `auth.type = "mtls"` (which requires `tls.client_auth`), bind to a +> it. With one, the header is omitted. Set `auth.type = "mtls"` (which requires `tls.client_auth`), bind to a > trusted interface, or put an authenticating reverse proxy in front. --- @@ -363,6 +363,8 @@ key_file = "/etc/logwisp/tls/client.key" - Retries apply to transport errors, `408`, `429`, and `5xx`. Any other non-2xx response is treated as permanent, and the batch is dropped and counted in `dropped_batches`. +- Redirects are never followed; a `3xx` is permanent too. Following one would + resend the batch wherever the response points, plaintext `http` included. - HTTP/2 is off by design; batched NDJSON POSTs gain nothing from it. - On shutdown a single best-effort flush of the pending batch is attempted. diff --git a/go.mod b/go.mod index f78102c..764d057 100644 --- a/go.mod +++ b/go.mod @@ -3,8 +3,8 @@ module logwisp go 1.27.1 require ( - github.com/lixenwraith/config v0.1.1-0.20260929051144-5132718bd627 - github.com/lixenwraith/log v0.1.1-0.20260926182514-9ad925351aa1 + github.com/lixenwraith/config v0.2.1 + github.com/lixenwraith/log v0.1.11 ) -require github.com/lixenwraith/toml v0.0.0-20260928180512-b59d068ff6a4 // indirect +require github.com/lixenwraith/toml v0.0.0-20261001212158-cede215365c0 // indirect diff --git a/go.sum b/go.sum index 9b0f154..71edf4c 100644 --- a/go.sum +++ b/go.sum @@ -1,6 +1,6 @@ -github.com/lixenwraith/config v0.1.1-0.20260929051144-5132718bd627 h1:PkSnFbvJWaf7cl4txDwglYFreet0QJypBD2XNOo5cc4= -github.com/lixenwraith/config v0.1.1-0.20260929051144-5132718bd627/go.mod h1:yfJQwKS+/6Dg70GIoHeYc0eYztOPETpqe6nUIbakcyg= -github.com/lixenwraith/log v0.1.1-0.20260926182514-9ad925351aa1 h1:fdVY7Jbg+GR61BA6zlvX4N2RFtgzCvSuWeOscicTb8I= -github.com/lixenwraith/log v0.1.1-0.20260926182514-9ad925351aa1/go.mod h1:EE8i5nSAW2yZBTytKuhdgehmgzrby6kW1k5DyH1SvBg= -github.com/lixenwraith/toml v0.0.0-20260928180512-b59d068ff6a4 h1:9qZU0iBIsqintDh39vieio0ygLiRrPmCEo6Ra45EQyY= -github.com/lixenwraith/toml v0.0.0-20260928180512-b59d068ff6a4/go.mod h1:itcT/Vt5mwmX9KVGCMsWZr6vbU/JgZ24wV/I0J5cMt0= +github.com/lixenwraith/config v0.2.1 h1:a1VLWhtAi1F7FWtRtFVpPkvemnBEf+4xrvDupvwHYtg= +github.com/lixenwraith/config v0.2.1/go.mod h1:yfJQwKS+/6Dg70GIoHeYc0eYztOPETpqe6nUIbakcyg= +github.com/lixenwraith/log v0.1.11 h1:dKWLnaR8dhXxj4l/aU5XZiFIsr8geaJQbOf8m7SiKhU= +github.com/lixenwraith/log v0.1.11/go.mod h1:EE8i5nSAW2yZBTytKuhdgehmgzrby6kW1k5DyH1SvBg= +github.com/lixenwraith/toml v0.0.0-20261001212158-cede215365c0 h1:ue1pILlQAovrxTMSaKII4AbZ7yY4mNs3cyivoSBwMmA= +github.com/lixenwraith/toml v0.0.0-20261001212158-cede215365c0/go.mod h1:itcT/Vt5mwmX9KVGCMsWZr6vbU/JgZ24wV/I0J5cMt0= diff --git a/internal/authz/authz.go b/internal/authz/authz.go index 60f5c85..528a2b3 100644 --- a/internal/authz/authz.go +++ b/internal/authz/authz.go @@ -12,11 +12,14 @@ import ( "crypto/tls" "fmt" "regexp" + "regexp/syntax" "strings" "sync/atomic" "logwisp/internal/config" "logwisp/internal/tlsx" + + "github.com/lixenwraith/log" ) // Authentication methods @@ -276,6 +279,73 @@ func (p *Policy) Unrestricted() bool { return p != nil && len(p.allow) == 0 && len(p.patterns) == 0 } +// LogStartup reports, once per plugin construction, what a listener policy +// admits, how it labels nodes, and allow patterns that admit more than they +// appear to. trustNode matters only for chain sources. +func (p *Policy) LogStartup(l *log.Logger, component, id string, trustNode bool) { + if p == nil || p.role == RoleDialer { + return + } + if p.Unrestricted() { + l.Warn("msg", "Auth policy admits any identity the configured CA vouches for", + "component", component, + "instance_id", id, + "hint", "set auth.allow or auth.allow_patterns to authorize named peers") + } + if p.BindsNode() { + l.Info("msg", "Node labels bound to peer identity; trust_node is ignored", + "component", component, + "instance_id", id, + "node_binding", p.binding, + "trust_node", trustNode) + } + for _, re := range p.patterns { + if !anchored(re.String()) { + l.Warn("msg", "auth.allow_patterns entry is not anchored and matches any identity containing it", + "component", component, + "instance_id", id, + "pattern", re.String(), + "hint", "anchor every alternative, e.g. ^(edge-01|edge-02)$") + } + } +} + +// anchored reports whether every match of pattern spans the whole identity: +// each alternative must start with ^ and end with $. "^a|b$" is the classic +// miss, admitting "a..." and "...b". +func anchored(pattern string) bool { + re, err := syntax.Parse(pattern, syntax.Perl) + if err != nil { + return false + } + re = re.Simplify() + return anchoredAt(re, syntax.OpBeginText) && anchoredAt(re, syntax.OpEndText) +} + +// anchoredAt follows the matching edge of re (first or last element of a +// concatenation, every branch of an alternation) down to the anchor op. +func anchoredAt(re *syntax.Regexp, anchor syntax.Op) bool { + switch re.Op { + case anchor: + return true + case syntax.OpCapture: + return anchoredAt(re.Sub[0], anchor) + case syntax.OpConcat: + if anchor == syntax.OpBeginText { + return anchoredAt(re.Sub[0], anchor) + } + return anchoredAt(re.Sub[len(re.Sub)-1], anchor) + case syntax.OpAlternate: + for _, sub := range re.Sub { + if !anchoredAt(sub, anchor) { + return false + } + } + return true + } + return false +} + // Describe renders the policy for a startup log line func (p *Policy) Describe() string { if p == nil { diff --git a/internal/authz/authz_test.go b/internal/authz/authz_test.go index 61fc620..ea4e516 100644 --- a/internal/authz/authz_test.go +++ b/internal/authz/authz_test.go @@ -296,3 +296,22 @@ func TestVerifyConnectionPinsServer(t *testing.T) { t.Error("unpinned server accepted") } } + +// Only patterns whose every alternative is pinned at both ends stay silent at +// startup; "^a|b$" reads as anchored but admits "a..." and "...b". +func TestAllowPatternAnchoring(t *testing.T) { + for pattern, want := range map[string]bool{ + `^edge-\d{2}$`: true, + `^(edge-01|edge-02)$`: true, + `^edge-a$|^edge-bc$`: true, // the parser factors this into ^edge-(?:a$|bc$) + `\Aedge\z`: true, + `edge-\d{2}`: false, + `^a|b$`: false, + `^ab$|^ac`: false, + `^a$|`: false, + } { + if got := anchored(pattern); got != want { + t.Errorf("anchored(%q) = %v, want %v", pattern, got, want) + } + } +} diff --git a/internal/config/loader_test.go b/internal/config/loader_test.go index 690ddab..9645fa4 100644 --- a/internal/config/loader_test.go +++ b/internal/config/loader_test.go @@ -9,8 +9,6 @@ import ( "time" "logwisp/internal/testutil" - - lconfig "github.com/lixenwraith/config" ) func TestResolveConfigArguments(t *testing.T) { @@ -185,7 +183,7 @@ allow = ["one", "two"] t.Fatal(err) } opts := TCPSinkOptions{Host: "127.0.0.1", BufferSize: 1000} - if err := lconfig.ScanMap(cfg.Pipelines[0].PluginSinks[0].Config, &opts); err != nil { + if err := Scan(cfg.Pipelines[0].PluginSinks[0].Config, &opts); err != nil { t.Fatal(err) } if opts.Host != "127.0.0.1" || opts.BufferSize != 1000 || opts.Port != 9000 || opts.WriteTimeoutMS != 9007199254740993 || @@ -193,3 +191,40 @@ allow = ["one", "two"] t.Fatalf("plugin defaults, nested tables or integer precision lost: %+v", opts) } } + +// A misspelled key, even inside a nested tls or auth table, fails plugin +// construction instead of leaving the protection it named switched off. +func TestPluginConfigRejectsUnknownKeys(t *testing.T) { + isolateConfig(t) + for key, table := range map[string]string{ + "bogus": "bogus = true", + "tls.enabeld": "[pipelines.plugin_sinks.config.tls]\nenabeld = true", + "auth.tpye": "[pipelines.plugin_sinks.config.auth]\ntpye = \"mtls\"", + } { + testutil.WriteFile(t, "plugins.toml", ` +[[pipelines]] +name = "typo" +[[pipelines.plugin_sources]] +id = "in" +type = "null" +[[pipelines.plugin_sinks]] +id = "out" +type = "tcp" +[pipelines.plugin_sinks.config] +port = 9000 +`+table+"\n") + m, err := Load([]string{"-c", "plugins.toml"}) + if err != nil { + t.Fatal(err) + } + cfg, err := m.Snapshot() + m.Close() + if err != nil { + t.Fatal(err) + } + err = Scan(cfg.Pipelines[0].PluginSinks[0].Config, &TCPSinkOptions{}) + if err == nil || !strings.Contains(err.Error(), `"`+key+`"`) { + t.Errorf("unknown key %s: err = %v", key, err) + } + } +} diff --git a/internal/config/validate.go b/internal/config/validate.go index e976a99..994a893 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -2,6 +2,10 @@ package config import ( "fmt" + "maps" + "reflect" + "slices" + "strings" lconfig "github.com/lixenwraith/config" ) @@ -81,3 +85,55 @@ func validateLogConfig(cfg *LogConfig) error { return nil } + +// Scan decodes a plugin's config map into target, rejecting keys target does +// not declare: a misspelled tls or auth key must fail, not silently disable +// the protection it was meant to configure. +func Scan(configMap map[string]any, target any) error { + if err := unknownKeys(configMap, reflect.TypeOf(target), ""); err != nil { + return err + } + return lconfig.ScanMap(configMap, target) +} + +// unknownKeys walks nested tables against the toml tags of t. Map-typed fields +// hold free-form keys and are not descended into. +func unknownKeys(m map[string]any, t reflect.Type, prefix string) error { + for t.Kind() == reflect.Pointer || t.Kind() == reflect.Slice { + t = t.Elem() + } + if t.Kind() != reflect.Struct { + return nil + } + fields := make(map[string]reflect.Type, t.NumField()) + for f := range t.Fields() { + if name, _, _ := strings.Cut(f.Tag.Get("toml"), ","); name != "" && name != "-" { + fields[name] = f.Type + } + } + for _, key := range slices.Sorted(maps.Keys(m)) { + ft, ok := fields[key] + if !ok { + return fmt.Errorf("unknown key %q", prefix+key) + } + var tables []map[string]any + switch v := m[key].(type) { + case map[string]any: + tables = []map[string]any{v} + case []map[string]any: + tables = v + case []any: + for _, e := range v { + if table, ok := e.(map[string]any); ok { + tables = append(tables, table) + } + } + } + for _, table := range tables { + if err := unknownKeys(table, ft, prefix+key+"."); err != nil { + return err + } + } + } + return nil +} diff --git a/internal/sink/console/console.go b/internal/sink/console/console.go index b04e547..113174d 100644 --- a/internal/sink/console/console.go +++ b/internal/sink/console/console.go @@ -65,7 +65,7 @@ func NewConsoleSinkPlugin( opts := &config.ConsoleSinkOptions{} // Scan config map into struct - if err := lconfig.ScanMap(configMap, opts); err != nil { + if err := config.Scan(configMap, opts); err != nil { return nil, fmt.Errorf("failed to parse config: %w", err) } @@ -206,4 +206,4 @@ func (cs *ConsoleSink) processLoop(ctx context.Context) { return } } -} \ No newline at end of file +} diff --git a/internal/sink/file/file.go b/internal/sink/file/file.go index 80c6b91..4b685b4 100644 --- a/internal/sink/file/file.go +++ b/internal/sink/file/file.go @@ -68,7 +68,7 @@ func NewFileSinkPlugin( opts := &config.FileSinkOptions{} // Scan config map into struct - if err := lconfig.ScanMap(configMap, opts); err != nil { + if err := config.Scan(configMap, opts); err != nil { return nil, fmt.Errorf("failed to parse config: %w", err) } @@ -259,4 +259,4 @@ func (fs *FileSink) processLoop(ctx context.Context) { return } } -} \ No newline at end of file +} diff --git a/internal/sink/http/http.go b/internal/sink/http/http.go index eec7b84..d7d057f 100644 --- a/internal/sink/http/http.go +++ b/internal/sink/http/http.go @@ -107,7 +107,7 @@ func NewHTTPSinkPlugin( Host: DefaultHTTPHost, WriteTimeoutMS: 0, // SSE indefinite streaming } - if err := lconfig.ScanMap(configMap, opts); err != nil { + if err := config.Scan(configMap, opts); err != nil { return nil, fmt.Errorf("failed to parse config: %w", err) } if err := lconfig.Port(opts.Port); err != nil { @@ -167,12 +167,8 @@ func NewHTTPSinkPlugin( "tls", tlsCfg != nil, "mtls", tlsCfg != nil && tlsCfg.ClientAuth == tls.RequireAndVerifyClientCert, "auth", authPolicy.Describe()) - if authPolicy.Unrestricted() { - logger.Warn("msg", "Auth policy admits any identity the configured CA vouches for", - "component", "http_sink", - "instance_id", id, - "hint", "set auth.allow or auth.allow_patterns to authorize named clients") - } + tlsx.LogWarnings(logger, "http_sink", id, opts.TLS, true) + authPolicy.LogStartup(logger, "http_sink", id, false) return h, nil } @@ -407,7 +403,10 @@ func (h *HTTPSink) handleStream(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "text/event-stream") w.Header().Set("Cache-Control", "no-cache") - w.Header().Set("Access-Control-Allow-Origin", "*") + if !h.auth.Enabled() { + // An authenticated stream is not offered to every web origin + w.Header().Set("Access-Control-Allow-Origin", "*") + } w.Header().Set("X-Accel-Buffering", "no") w.WriteHeader(http.StatusOK) diff --git a/internal/sink/http/http_test.go b/internal/sink/http/http_test.go index 0d3ee64..5c2e5c7 100644 --- a/internal/sink/http/http_test.go +++ b/internal/sink/http/http_test.go @@ -2,13 +2,18 @@ package http import ( "bufio" + "crypto/tls" + "crypto/x509" "encoding/json" "net/http" "net/http/httptest" + "os" + "strings" "testing" "time" "logwisp/internal/sink" + "logwisp/internal/testutil" ) func TestStatusReportsQueueAndConnectionBounds(t *testing.T) { @@ -119,3 +124,48 @@ func TestHeadOnStreamPathIsRefused(t *testing.T) { t.Errorf("sessions after HEAD = %d, want 0", n) } } + +// A stream gated by an auth policy is not offered to every web origin; an +// open stream keeps the wildcard so browser dashboards still work. +func TestWildcardCORSOnlyWithoutAuth(t *testing.T) { + pki := testutil.NewPKI(t, "viewer-01") + caPEM, err := os.ReadFile(pki.CA) + if err != nil { + t.Fatal(err) + } + roots := x509.NewCertPool() + roots.AppendCertsFromPEM(caPEM) + clientCert, err := tls.LoadX509KeyPair(pki.ClientCert, pki.ClientKey) + if err != nil { + t.Fatal(err) + } + + open, _ := newTestHTTPSink(t, nil) + gated, _ := newTestHTTPSink(t, map[string]any{ + "tls": map[string]any{"enabled": true, "cert_file": pki.ServerCert, "key_file": pki.ServerKey, + "client_auth": true, "client_ca_file": pki.CA}, + "auth": map[string]any{"type": "mtls", "allow": []any{"viewer-01"}}, + }) + for _, tc := range []struct { + name string + sink *HTTPSink + want string + }{{"open", open, "*"}, {"gated", gated, ""}} { + client, baseURL := serveTestHTTPSink(t, tc.sink) + if tc.sink.tlsConfig != nil { + client.Transport = &http.Transport{TLSClientConfig: &tls.Config{RootCAs: roots, Certificates: []tls.Certificate{clientCert}}} + baseURL = "https" + strings.TrimPrefix(baseURL, "http") + } + resp, err := client.Get(baseURL + "/stream") + if err != nil { + t.Fatalf("%s: %v", tc.name, err) + } + resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("%s: GET /stream = %d", tc.name, resp.StatusCode) + } + if got := resp.Header.Get("Access-Control-Allow-Origin"); got != tc.want { + t.Errorf("%s: Access-Control-Allow-Origin = %q, want %q", tc.name, got, tc.want) + } + } +} diff --git a/internal/sink/httpchain/httpchain.go b/internal/sink/httpchain/httpchain.go index f69ddab..ea8568c 100644 --- a/internal/sink/httpchain/httpchain.go +++ b/internal/sink/httpchain/httpchain.go @@ -44,6 +44,7 @@ const ( DefaultHTTPChainSinkRequestTimeoutMS = 10000 DefaultHTTPChainSinkBackoffMinMS = 500 DefaultHTTPChainSinkBackoffMaxMS = 30000 + maxResponseDrain = 64 * 1024 ) // HTTPChainSink batches structured entries and posts NDJSON to a downstream @@ -92,7 +93,7 @@ func NewHTTPChainSinkPlugin( proxy *session.Proxy, ) (sink.Sink, error) { opts := &config.HTTPChainSinkOptions{} - if err := lconfig.ScanMap(configMap, opts); err != nil { + if err := config.Scan(configMap, opts); err != nil { return nil, fmt.Errorf("failed to parse config: %w", err) } if err := lconfig.NonEmpty(opts.Host); err != nil { @@ -182,7 +183,11 @@ func NewHTTPChainSinkPlugin( mtls: tlsCfg != nil && len(tlsCfg.Certificates) > 0, auth: authPolicy, url: scheme + "://" + addr + opts.IngestPath, - client: &http.Client{Transport: transport}, + // A redirect would resend the batch, even from https to plaintext http + client: &http.Client{ + Transport: transport, + CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }, + }, input: make(chan core.TransportEvent, opts.BufferSize), done: make(chan struct{}), logger: logger, @@ -208,6 +213,7 @@ func NewHTTPChainSinkPlugin( "tls", t.tlsEnabled, "mtls", t.mtls, "auth", authPolicy.Describe()) + tlsx.LogWarnings(logger, "http_chain_sink", id, opts.TLS, false) return t, nil } @@ -436,8 +442,8 @@ func (t *HTTPChainSink) post(ctx context.Context, body []byte) (transient bool, return true, err } defer resp.Body.Close() - // Drain for connection reuse - io.Copy(io.Discard, resp.Body) + // Drain for connection reuse, bounded so a hostile peer cannot stream forever + io.Copy(io.Discard, io.LimitReader(resp.Body, maxResponseDrain)) switch { case resp.StatusCode >= 200 && resp.StatusCode < 300: diff --git a/internal/sink/httpchain/httpchain_test.go b/internal/sink/httpchain/httpchain_test.go new file mode 100644 index 0000000..85a9027 --- /dev/null +++ b/internal/sink/httpchain/httpchain_test.go @@ -0,0 +1,47 @@ +package httpchain + +import ( + "net" + "net/http" + "net/http/httptest" + "strconv" + "sync/atomic" + "testing" + "time" + + "logwisp/internal/session" + + "github.com/lixenwraith/log" +) + +// A redirect is never followed: it would resend the batch wherever the +// response points, plaintext http included. +func TestRedirectIsNotFollowed(t *testing.T) { + var leaked atomic.Bool + target := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { leaked.Store(true) })) + t.Cleanup(target.Close) + redirector := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, target.URL+r.URL.Path, http.StatusPermanentRedirect) + })) + t.Cleanup(redirector.Close) + + host, port, err := net.SplitHostPort(redirector.Listener.Addr().String()) + if err != nil { + t.Fatal(err) + } + p, _ := strconv.ParseInt(port, 10, 64) + manager := session.NewManager(time.Hour) + t.Cleanup(manager.Stop) + created, err := NewHTTPChainSinkPlugin("fwd", map[string]any{"host": host, "port": p}, + log.NewLogger(), session.NewProxy(manager, "fwd")) + if err != nil { + t.Fatal(err) + } + transient, err := created.(*HTTPChainSink).post(t.Context(), []byte("{}\n")) + if err == nil || transient { + t.Fatalf("post = (transient %v, %v), want a permanent error", transient, err) + } + if leaked.Load() { + t.Fatal("the batch followed the redirect") + } +} diff --git a/internal/sink/null/null.go b/internal/sink/null/null.go index 6744409..897bad7 100644 --- a/internal/sink/null/null.go +++ b/internal/sink/null/null.go @@ -6,6 +6,7 @@ import ( "sync/atomic" "time" + "logwisp/internal/config" "logwisp/internal/core" "logwisp/internal/plugin" "logwisp/internal/session" @@ -49,6 +50,9 @@ func NewNullSinkPlugin( logger *log.Logger, proxy *session.Proxy, ) (sink.Sink, error) { + if err := config.Scan(configMap, &config.NullSinkOptions{}); err != nil { + return nil, fmt.Errorf("failed to parse config: %w", err) + } ns := &NullSink{ id: id, proxy: proxy, @@ -143,4 +147,4 @@ func (ns *NullSink) processLoop(ctx context.Context) { return } } -} \ No newline at end of file +} diff --git a/internal/sink/tcp/tcp.go b/internal/sink/tcp/tcp.go index 2e77427..dee046b 100644 --- a/internal/sink/tcp/tcp.go +++ b/internal/sink/tcp/tcp.go @@ -58,8 +58,10 @@ type TCPSink struct { input chan core.TransportEvent logger *log.Logger - // Client registry + // Client registry. conns holds every accepted connection, so shutdown also + // reaches peers still in the handshake, before they become clients. clients map[uint64]*tcpClient + conns map[net.Conn]struct{} clientsMu sync.Mutex nextClientID atomic.Uint64 writeTimeout time.Duration @@ -86,11 +88,10 @@ type TCPSink struct { lastProcessed atomic.Value // time.Time } -// tcpClient pairs a connection with its bounded send queue. +// tcpClient is a registered connection's bounded send queue. // send is written by the broadcast loop (non-blocking) and drained by the // writer goroutine; closed signals reader-detected disconnect. type tcpClient struct { - conn net.Conn send chan []byte sessionID string closed chan struct{} @@ -107,7 +108,7 @@ func NewTCPSinkPlugin( Host: DefaultTCPHost, KeepAlive: true, } - if err := lconfig.ScanMap(configMap, opts); err != nil { + if err := config.Scan(configMap, opts); err != nil { return nil, fmt.Errorf("failed to parse config: %w", err) } if err := lconfig.Port(opts.Port); err != nil { @@ -143,6 +144,7 @@ func NewTCPSinkPlugin( done: make(chan struct{}), logger: logger, clients: make(map[uint64]*tcpClient), + conns: make(map[net.Conn]struct{}), writeTimeout: time.Duration(opts.WriteTimeoutMS) * time.Millisecond, tlsConfig: tlsCfg, auth: authPolicy, @@ -157,12 +159,8 @@ func NewTCPSinkPlugin( "tls", tlsCfg != nil, "mtls", tlsCfg != nil && tlsCfg.ClientAuth == tls.RequireAndVerifyClientCert, "auth", authPolicy.Describe()) - if authPolicy.Unrestricted() { - logger.Warn("msg", "Auth policy admits any identity the configured CA vouches for", - "component", "tcp_sink", - "instance_id", id, - "hint", "set auth.allow or auth.allow_patterns to authorize named clients") - } + tlsx.LogWarnings(logger, "tcp_sink", id, opts.TLS, true) + authPolicy.LogStartup(logger, "tcp_sink", id, false) return t, nil } @@ -258,8 +256,8 @@ func (t *TCPSink) shutdown() { t.listener.Close() // unblocks acceptLoop } t.clientsMu.Lock() - for _, c := range t.clients { - c.conn.Close() // unblocks per-connection readers + for conn := range t.conns { + conn.Close() // unblocks handshakes and per-connection readers } t.clientsMu.Unlock() }) @@ -287,9 +285,17 @@ func (t *TCPSink) acceptLoop() { continue } - // Certificate authorization runs in handleConn post-handshake, - // pre-registration. Password-auth extension point: preamble - // verification belongs at the same place. + t.clientsMu.Lock() + select { + case <-t.done: + // shutdown already swept conns; this one would outlive it + t.clientsMu.Unlock() + conn.Close() + return + default: + } + t.conns[conn] = struct{}{} + t.clientsMu.Unlock() t.wg.Add(1) go t.handleConn(conn) @@ -305,6 +311,10 @@ func (t *TCPSink) handleConn(conn net.Conn) { // Counted from accept: max_connections bounds concurrent handshakes too count := t.activeConns.Add(1) defer func() { + conn.Close() + t.clientsMu.Lock() + delete(t.conns, conn) + t.clientsMu.Unlock() newCount := t.activeConns.Add(-1) t.logger.Debug("msg", "TCP connection closed", "component", "tcp_sink", @@ -327,7 +337,6 @@ func (t *TCPSink) handleConn(conn net.Conn) { "component", "tcp_sink", "remote_addr", remote, "error", err) - conn.Close() return } cs := tc.ConnectionState() @@ -348,14 +357,12 @@ func (t *TCPSink) handleConn(conn net.Conn) { "instance_id", t.id, "remote_addr", remote, "error", err) - conn.Close() return } ident.Apply(meta) sess := t.proxy.CreateSession(remote, meta) c := &tcpClient{ - conn: conn, send: make(chan []byte, t.config.ClientBufferSize), sessionID: sess.ID, closed: make(chan struct{}), diff --git a/internal/sink/tcp/tcp_test.go b/internal/sink/tcp/tcp_test.go new file mode 100644 index 0000000..b880123 --- /dev/null +++ b/internal/sink/tcp/tcp_test.go @@ -0,0 +1,59 @@ +package tcp + +import ( + "net" + "strconv" + "testing" + "time" + + "logwisp/internal/session" + "logwisp/internal/testutil" + + "github.com/lixenwraith/log" +) + +// A peer that connects and never completes the TLS handshake does not hold +// Stop for the handshake timeout: shutdown reaches connections before they +// become clients. +func TestStopDoesNotWaitForSilentHandshake(t *testing.T) { + pki := testutil.NewPKI(t, "viewer") + probe, err := net.Listen("tcp4", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + addr := probe.Addr().String() + probe.Close() + _, port, _ := net.SplitHostPort(addr) + p, _ := strconv.ParseInt(port, 10, 64) + + manager := session.NewManager(time.Hour) + t.Cleanup(manager.Stop) + created, err := NewTCPSinkPlugin("out", map[string]any{ + "host": "127.0.0.1", + "port": p, + "tls": map[string]any{"enabled": true, "cert_file": pki.ServerCert, "key_file": pki.ServerKey}, + }, log.NewLogger(), session.NewProxy(manager, "out")) + if err != nil { + t.Fatal(err) + } + sink := created.(*TCPSink) + if err := sink.Start(t.Context()); err != nil { + t.Fatal(err) + } + conn, err := net.Dial("tcp4", addr) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { conn.Close() }) + for deadline := time.Now().Add(2 * time.Second); sink.activeConns.Load() == 0; time.Sleep(10 * time.Millisecond) { + if time.Now().After(deadline) { + t.Fatal("connection never reached the handshake") + } + } + + start := time.Now() + sink.Stop() + if d := time.Since(start); d > 2*time.Second { + t.Fatalf("Stop took %v with a silent peer in the handshake", d) + } +} diff --git a/internal/sink/tcpchain/tcpchain.go b/internal/sink/tcpchain/tcpchain.go index 3f18f9f..1d958e0 100644 --- a/internal/sink/tcpchain/tcpchain.go +++ b/internal/sink/tcpchain/tcpchain.go @@ -6,7 +6,6 @@ import ( "encoding/json" "fmt" "maps" - "math/rand/v2" "net" "os" "strconv" @@ -88,7 +87,7 @@ func NewTCPChainSinkPlugin( opts := &config.TCPChainSinkOptions{ KeepAlive: true, } - if err := lconfig.ScanMap(configMap, opts); err != nil { + if err := config.Scan(configMap, opts); err != nil { return nil, fmt.Errorf("failed to parse config: %w", err) } if err := lconfig.NonEmpty(opts.Host); err != nil { @@ -179,6 +178,7 @@ func NewTCPChainSinkPlugin( "tls", tlsCfg != nil, "mtls", tlsCfg != nil && len(tlsCfg.Certificates) > 0, "auth", authPolicy.Describe()) + tlsx.LogWarnings(logger, "tcp_chain_sink", id, opts.TLS, false) return t, nil } @@ -299,24 +299,6 @@ func (t *TCPChainSink) runLoop(ctx context.Context) { } } -// toEntry extracts the structured entry, stamping node identity at first hop -func (t *TCPChainSink) toEntry(event core.TransportEvent) core.LogEntry { - entry := event.Entry - if entry.Time.IsZero() { - // Defensive: event without structured entry, wrap formatted payload - t.synthesized.Add(1) - entry = core.LogEntry{ - Time: event.Time, - Source: t.id, - Message: string(event.Payload), - } - } - if entry.Node == "" { - entry.Node = t.node - } - return entry -} - // deliver writes one line, holding it across reconnects until sent or shutdown. // Backpressure during outage propagates to the pipeline dispatch drop counter. func (t *TCPChainSink) deliver(ctx context.Context, line []byte) bool { @@ -427,17 +409,3 @@ func (t *TCPChainSink) waitBackoff(ctx context.Context, failures int) bool { return false } } - -// backoffDelay computes exponential backoff with ±20% jitter -func (t *TCPChainSink) backoffDelay(failures int) time.Duration { - minD := time.Duration(t.config.BackoffMinMS) * time.Millisecond - maxD := time.Duration(t.config.BackoffMaxMS) * time.Millisecond - - d := maxD - if failures < 63 { - if v := minD << uint(failures-1); v > 0 && v < maxD { - d = v - } - } - return d - d/5 + time.Duration(rand.Int64N(int64(2*d/5)+1)) -} diff --git a/internal/source/console/console.go b/internal/source/console/console.go index 7ae21a7..4140dd7 100644 --- a/internal/source/console/console.go +++ b/internal/source/console/console.go @@ -13,7 +13,6 @@ import ( "logwisp/internal/session" "logwisp/internal/source" - lconfig "github.com/lixenwraith/config" "github.com/lixenwraith/log" ) @@ -70,7 +69,7 @@ func NewConsoleSourcePlugin( opts := &config.ConsoleSourceOptions{} // Scan config map - if err := lconfig.ScanMap(configMap, opts); err != nil { + if err := config.Scan(configMap, opts); err != nil { return nil, fmt.Errorf("failed to parse config: %w", err) } @@ -221,4 +220,4 @@ func (s *ConsoleSource) publish(entry core.LogEntry) { "component", "console_source") } } -} \ No newline at end of file +} diff --git a/internal/source/file/file.go b/internal/source/file/file.go index 5cc65db..b27e6db 100644 --- a/internal/source/file/file.go +++ b/internal/source/file/file.go @@ -75,7 +75,7 @@ func NewFileSourcePlugin( opts := &config.FileSourceOptions{} // Use lconfig to scan map into struct (overriding defaults) - if err := lconfig.ScanMap(configMap, opts); err != nil { + if err := config.Scan(configMap, opts); err != nil { return nil, fmt.Errorf("failed to parse config: %w", err) } diff --git a/internal/source/httpchain/httpchain.go b/internal/source/httpchain/httpchain.go index b8cf518..d2c8679 100644 --- a/internal/source/httpchain/httpchain.go +++ b/internal/source/httpchain/httpchain.go @@ -85,7 +85,7 @@ func NewHTTPChainSourcePlugin( Host: "0.0.0.0", TrustNode: true, } - if err := lconfig.ScanMap(configMap, opts); err != nil { + if err := config.Scan(configMap, opts); err != nil { return nil, fmt.Errorf("failed to parse config: %w", err) } if err := lconfig.Port(opts.Port); err != nil { @@ -135,19 +135,8 @@ func NewHTTPChainSourcePlugin( "tls", tlsCfg != nil, "mtls", tlsCfg != nil && tlsCfg.ClientAuth == tls.RequireAndVerifyClientCert, "auth", authPolicy.Describe()) - if authPolicy.Unrestricted() { - logger.Warn("msg", "Auth policy admits any identity the configured CA vouches for", - "component", "http_chain_source", - "instance_id", id, - "hint", "set auth.allow or auth.allow_patterns to authorize named peers") - } - if authPolicy.BindsNode() { - logger.Info("msg", "Node labels bound to peer identity; trust_node is ignored", - "component", "http_chain_source", - "instance_id", id, - "node_binding", authPolicy.NodeBinding(), - "trust_node", opts.TrustNode) - } + tlsx.LogWarnings(logger, "http_chain_source", id, opts.TLS, true) + authPolicy.LogStartup(logger, "http_chain_source", id, opts.TrustNode) return s, nil } diff --git a/internal/source/null/null.go b/internal/source/null/null.go index 2ad5cd6..2d7a598 100644 --- a/internal/source/null/null.go +++ b/internal/source/null/null.go @@ -5,6 +5,7 @@ import ( "sync/atomic" "time" + "logwisp/internal/config" "logwisp/internal/core" "logwisp/internal/plugin" "logwisp/internal/session" @@ -47,6 +48,9 @@ func NewNullSourcePlugin( logger *log.Logger, proxy *session.Proxy, ) (source.Source, error) { + if err := config.Scan(configMap, &config.NullSourceOptions{}); err != nil { + return nil, fmt.Errorf("failed to parse config: %w", err) + } ns := &NullSource{ id: id, proxy: proxy, @@ -122,4 +126,4 @@ func (ns *NullSource) GetStats() source.SourceStats { LastEntryTime: lastEntry, Details: map[string]any{}, } -} \ No newline at end of file +} diff --git a/internal/source/random/random.go b/internal/source/random/random.go index 1604e3e..bf640c9 100644 --- a/internal/source/random/random.go +++ b/internal/source/random/random.go @@ -76,7 +76,7 @@ func NewRandomSourcePlugin( } // Scan config map - if err := lconfig.ScanMap(configMap, opts); err != nil { + if err := config.Scan(configMap, opts); err != nil { return nil, fmt.Errorf("failed to parse config: %w", err) } @@ -355,4 +355,4 @@ func (rs *RandomSource) publish(entry core.LogEntry) { rs.droppedEntries.Add(1) } } -} \ No newline at end of file +} diff --git a/internal/source/tcpchain/tcpchain.go b/internal/source/tcpchain/tcpchain.go index d2cdb9a..1d02bf3 100644 --- a/internal/source/tcpchain/tcpchain.go +++ b/internal/source/tcpchain/tcpchain.go @@ -80,7 +80,7 @@ func NewTCPChainSourcePlugin( Host: "0.0.0.0", TrustNode: true, } - if err := lconfig.ScanMap(configMap, opts); err != nil { + if err := config.Scan(configMap, opts); err != nil { return nil, fmt.Errorf("failed to parse config: %w", err) } if err := lconfig.Port(opts.Port); err != nil { @@ -121,19 +121,8 @@ func NewTCPChainSourcePlugin( "tls", tlsCfg != nil, "mtls", tlsCfg != nil && tlsCfg.ClientAuth == tls.RequireAndVerifyClientCert, "auth", authPolicy.Describe()) - if authPolicy.Unrestricted() { - logger.Warn("msg", "Auth policy admits any identity the configured CA vouches for", - "component", "tcp_chain_source", - "instance_id", id, - "hint", "set auth.allow or auth.allow_patterns to authorize named peers") - } - if authPolicy.BindsNode() { - logger.Info("msg", "Node labels bound to peer identity; trust_node is ignored", - "component", "tcp_chain_source", - "instance_id", id, - "node_binding", authPolicy.NodeBinding(), - "trust_node", opts.TrustNode) - } + tlsx.LogWarnings(logger, "tcp_chain_source", id, opts.TLS, true) + authPolicy.LogStartup(logger, "tcp_chain_source", id, opts.TrustNode) return s, nil } diff --git a/internal/testutil/fixtures.go b/internal/testutil/fixtures.go index 8e2361a..fb8339e 100644 --- a/internal/testutil/fixtures.go +++ b/internal/testutil/fixtures.go @@ -3,9 +3,19 @@ package testutil import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "math/big" + "net" "os" + "path/filepath" "strings" "testing" + "time" ) // ClearEnvPrefix isolates overrides while preserving the caller's environment. @@ -29,3 +39,64 @@ func WriteFile(t testing.TB, path, contents string) { t.Fatalf("write fixture %q: %v", path, err) } } + +// PKI is a throwaway CA with a server certificate for 127.0.0.1 and one client +// certificate, written as PEM files for plugins that load TLS from disk. +type PKI struct { + CA, ServerCert, ServerKey, ClientCert, ClientKey string +} + +// NewPKI issues the PKI into a test directory; clientCN names the client. +func NewPKI(t testing.TB, clientCN string) PKI { + t.Helper() + dir := t.TempDir() + now := time.Now() + caKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + caTmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), Subject: pkix.Name{CommonName: "LogWisp Test CA"}, + NotBefore: now.Add(-time.Hour), NotAfter: now.Add(24 * time.Hour), + IsCA: true, BasicConstraintsValid: true, KeyUsage: x509.KeyUsageCertSign, + } + caDER, err := x509.CreateCertificate(rand.Reader, caTmpl, caTmpl, &caKey.PublicKey, caKey) + if err != nil { + t.Fatal(err) + } + ca, err := x509.ParseCertificate(caDER) + if err != nil { + t.Fatal(err) + } + pki := PKI{CA: writePEM(t, dir, "ca.crt", "CERTIFICATE", caDER)} + issue := func(name, cn string, serial int64, usage x509.ExtKeyUsage, ips []net.IP) (string, string) { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(serial), Subject: pkix.Name{CommonName: cn}, + NotBefore: now.Add(-time.Hour), NotAfter: now.Add(24 * time.Hour), + KeyUsage: x509.KeyUsageDigitalSignature, ExtKeyUsage: []x509.ExtKeyUsage{usage}, IPAddresses: ips, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, ca, &key.PublicKey, caKey) + if err != nil { + t.Fatal(err) + } + keyDER, err := x509.MarshalPKCS8PrivateKey(key) + if err != nil { + t.Fatal(err) + } + return writePEM(t, dir, name+".crt", "CERTIFICATE", der), writePEM(t, dir, name+".key", "PRIVATE KEY", keyDER) + } + pki.ServerCert, pki.ServerKey = issue("server", "relay.internal", 2, x509.ExtKeyUsageServerAuth, []net.IP{net.IPv4(127, 0, 0, 1)}) + pki.ClientCert, pki.ClientKey = issue("client", clientCN, 3, x509.ExtKeyUsageClientAuth, nil) + return pki +} + +func writePEM(t testing.TB, dir, name, blockType string, der []byte) string { + t.Helper() + path := filepath.Join(dir, name) + WriteFile(t, path, string(pem.EncodeToMemory(&pem.Block{Type: blockType, Bytes: der}))) + return path +} diff --git a/internal/tlsx/tlsx.go b/internal/tlsx/tlsx.go index 9eb2d9e..e2f3a58 100644 --- a/internal/tlsx/tlsx.go +++ b/internal/tlsx/tlsx.go @@ -6,10 +6,12 @@ package tlsx import ( "crypto/tls" "crypto/x509" + "encoding/pem" "fmt" stdlog "log" "os" "strings" + "sync" "time" "logwisp/internal/config" @@ -17,8 +19,12 @@ import ( "github.com/lixenwraith/log" ) -// HandshakeTimeout bounds TLS handshakes on both accept and dial paths -const HandshakeTimeout = 10 * time.Second +const ( + // HandshakeTimeout bounds TLS handshakes on both accept and dial paths + HandshakeTimeout = 10 * time.Second + // expiryWindow is how far ahead startup announces certificate expiry + expiryWindow = 30 * 24 * time.Hour +) // Server builds the *tls.Config for listener plugins // (tcp/http sinks, tcp_chain/http_chain sources). Returns (nil, nil) when disabled. @@ -136,6 +142,93 @@ func PeerIdentity(cs tls.ConnectionState, mode string) string { return "" } +// LogWarnings reports TLS settings that work but are likely mistakes, once per +// plugin construction, so each reload re-announces expiring certificates. +func LogWarnings(l *log.Logger, component, id string, o *config.TLSOptions, listener bool) { + if o == nil || !o.Enabled { + return + } + var warnings []string + if !listener && o.InsecureSkipVerify { + warnings = append(warnings, "tls.insecure_skip_verify disables server verification: any peer can impersonate the server") + } + files := []struct{ key, path string }{{"cert_file", o.CertFile}, {"ca_file", o.CAFile}} + if listener { + files[1] = struct{ key, path string }{"client_ca_file", o.ClientCAFile} + } + now := time.Now() + for _, f := range files { + for _, c := range readCerts(f.path) { + if w := expiryWarning(c, now); w != "" { + warnings = append(warnings, fmt.Sprintf("tls.%s %s: %s", f.key, f.path, w)) + } + } + } + if w := SecretFileWarning("tls.key_file", o.KeyFile); w != "" { + warnings = append(warnings, w) + } + for _, w := range warnings { + l.Warn("msg", w, "component", component, "instance_id", id) + } +} + +// expiryWarning describes a certificate outside, or about to leave, its +// validity window; a handshake failure is otherwise the first sign. +func expiryWarning(c *x509.Certificate, now time.Time) string { + name := c.Subject.CommonName + switch { + case now.After(c.NotAfter): + return fmt.Sprintf("certificate %q expired on %s", name, c.NotAfter.UTC().Format(time.DateOnly)) + case now.Before(c.NotBefore): + return fmt.Sprintf("certificate %q is not valid until %s", name, c.NotBefore.UTC().Format(time.DateTime)) + case c.NotAfter.Sub(now) < expiryWindow: + return fmt.Sprintf("certificate %q expires on %s", name, c.NotAfter.UTC().Format(time.DateOnly)) + } + return "" +} + +// readCerts parses every certificate in a PEM file. Constructors already +// loaded it successfully, so an unreadable file yields nothing here. +func readCerts(path string) []*x509.Certificate { + if path == "" { + return nil + } + data, err := os.ReadFile(path) + if err != nil { + return nil + } + var certs []*x509.Certificate + for block, rest := pem.Decode(data); block != nil; block, rest = pem.Decode(rest) { + if block.Type != "CERTIFICATE" { + continue + } + if c, err := x509.ParseCertificate(block.Bytes); err == nil { + certs = append(certs, c) + } + } + return certs +} + +// warnedSecrets keeps reloads from repeating a permission warning per path +var warnedSecrets sync.Map + +// SecretFileWarning describes a secret file every local user can read, once +// per path per process. Group access is left alone: container secret mounts +// commonly grant it on purpose. +func SecretFileWarning(key, path string) string { + if path == "" { + return "" + } + fi, err := os.Stat(path) + if err != nil || fi.Mode().Perm()&0o004 == 0 { + return "" + } + if _, seen := warnedSecrets.LoadOrStore(path, struct{}{}); seen { + return "" + } + return fmt.Sprintf("%s %s is readable by all users (mode %04o)", key, path, fi.Mode().Perm()) +} + // HTTPErrorLog adapts the structured logger for http.Server.ErrorLog so TLS // handshake failures don't bypass log routing straight to stderr (which would // violate the console sanitization policy). diff --git a/internal/tlsx/tlsx_test.go b/internal/tlsx/tlsx_test.go new file mode 100644 index 0000000..ee2e7c4 --- /dev/null +++ b/internal/tlsx/tlsx_test.go @@ -0,0 +1,34 @@ +package tlsx + +import ( + "crypto/x509" + "crypto/x509/pkix" + "strings" + "testing" + "time" +) + +// Startup names a certificate outside its validity window or within 30 days +// of leaving it; a healthy one stays silent. +func TestExpiryWarning(t *testing.T) { + now := time.Date(2026, 10, 1, 12, 0, 0, 0, time.UTC) + day := 24 * time.Hour + cert := func(from, until time.Duration) *x509.Certificate { + return &x509.Certificate{Subject: pkix.Name{CommonName: "relay"}, NotBefore: now.Add(from), NotAfter: now.Add(until)} + } + for _, tc := range []struct { + name string + cert *x509.Certificate + want string + }{ + {"valid", cert(-day, 90*day), ""}, + {"expiring", cert(-day, 10*day), "expires on 2026-10-11"}, + {"expired", cert(-90*day, -day), "expired on 2026-09-30"}, + {"not yet valid", cert(day, 90*day), "not valid until"}, + } { + got := expiryWarning(tc.cert, now) + if tc.want == "" && got != "" || !strings.Contains(got, tc.want) { + t.Errorf("%s: warning = %q, want %q", tc.name, got, tc.want) + } + } +}