IsoHex/edge-router

Watch

Fork 0

An httpreq endpoint with embedded userinfo leaks its password to the log, the database and the admin API #58

New issue

Closed

opened 2026-10-26 15:38:07 +00:00 by james.oates · 1 comment

james.oates commented

2026-10-26 15:38:07 +00:00

Owner

From the v2.0.0 security audit.

src/httpreq.rs:180 attaches format!("POST {url}") as error context. url::Url's Display prints the full serialization, which retains user:password@. cert_manager flattens the whole chain with format!("{e:#}"), logs it at WARN, and writes it to certificates.last_error via record_certificate_error — a column returned by both GET /api/certificates and /ready.

src/httpreq.rs:72 also echoes the raw string into its parse error ({raw:?}), so a malformed value puts the password on stderr at startup.

build_config neither rejects nor strips userinfo, and edge-router.example.toml does not warn against it.

Failure scenario

An operator writes HTTPREQ_ENDPOINT=https://er:[email protected]/dns rather than setting HTTPREQ_USERNAME/HTTPREQ_PASSWORD separately — a natural shortcut, and nothing forbids it. The endpoint is briefly unreachable during a renewal. The password is now at WARN in journald and persisted unredacted in certificates.last_error, where it survives log rotation and is served to every admin-API caller until the next successful order clears the column.

Fix

Strip userinfo before the URL is used in any error context — or reject it at build_config with a message pointing at HTTPREQ_USERNAME/HTTPREQ_PASSWORD, which is probably better since silently dropping credentials an operator supplied would break their auth confusingly. Either way the parse error at :72 should not echo the raw value.

Worth a general look at whether anything else Displays a Url into an error or log line.

From the v2.0.0 security audit. `src/httpreq.rs:180` attaches `format!("POST {url}")` as error context. `url::Url`'s `Display` prints the full serialization, which retains `user:password@`. `cert_manager` flattens the whole chain with `format!("{e:#}")`, logs it at WARN, and writes it to `certificates.last_error` via `record_certificate_error` — a column returned by both `GET /api/certificates` and `/ready`. `src/httpreq.rs:72` also echoes the raw string into its parse error (`{raw:?}`), so a malformed value puts the password on stderr at startup. `build_config` neither rejects nor strips userinfo, and `edge-router.example.toml` does not warn against it. ## Failure scenario An operator writes `HTTPREQ_ENDPOINT=https://er:[email protected]/dns` rather than setting `HTTPREQ_USERNAME`/`HTTPREQ_PASSWORD` separately — a natural shortcut, and nothing forbids it. The endpoint is briefly unreachable during a renewal. The password is now at WARN in journald *and* persisted unredacted in `certificates.last_error`, where it survives log rotation and is served to every admin-API caller until the next successful order clears the column. ## Fix Strip userinfo before the URL is used in any error context — or reject it at `build_config` with a message pointing at `HTTPREQ_USERNAME`/`HTTPREQ_PASSWORD`, which is probably better since silently dropping credentials an operator supplied would break their auth confusingly. Either way the parse error at `:72` should not echo the raw value. Worth a general look at whether anything else `Display`s a `Url` into an error or log line.

james.oates added this to the v2.1 milestone

2026-10-26 17:53:26 +00:00

james.oates referenced this issue

2026-10-26 17:53:55 +00:00

The WebSocket path has no connect timeout, no idle timeout and no connection cap #62

james.oates referenced this issue from a commit

2026-10-27 10:22:51 +00:00

Refuse an httpreq endpoint that embeds credentials (#58)

james.oates commented

2026-10-27 10:23:04 +00:00

Author

Owner

Fixed in ffdd15e by rejecting, as this issue recommended — stripping would fail an operator's auth confusingly after silently discarding what they supplied.

build_config refuses an endpoint with a username or password, naming HTTPREQ_USERNAME/HTTPREQ_PASSWORD and saying where a URL-borne password ends up. The refusal does not repeat the value.
The parse error at :72 no longer echoes raw. That site mattered most: a typo'd URL is exactly where a stray user:password@ lands, and the error reaches stderr before any validation can reject it.
httpreq::redacted is now the only sanctioned way to put a URL in an error, and the POST context uses it. Keeping it as a named function rather than inlining the clear means a future call site cannot quietly reintroduce the leak.

The sweep you asked for found no other site. {url}/{raw} appears three times in src/: the two above and HTTPREQ_HTTP_TIMEOUT's {raw:?}, which is a duration and carries nothing sensitive. Nothing else Displays a Url into an error or log.

Breaking, narrowly, and in the v2.1 notes: an endpoint using the embedded-credentials shortcut now fails at startup. Nothing in the fleet does — devinfra's is https://dns01.isohex.uk:8443 with the credentials in separate variables.

Documented in the README's HTTPREQ_ENDPOINT row and in edge-router.example.toml, which this issue correctly noted said nothing against it.

373 tests pass, clippy --all-targets -D warnings clean, fmt clean.

Fixed in `ffdd15e` by rejecting, as this issue recommended — stripping would fail an operator's auth confusingly after silently discarding what they supplied. - `build_config` refuses an endpoint with a username or password, naming `HTTPREQ_USERNAME`/`HTTPREQ_PASSWORD` and saying where a URL-borne password ends up. The refusal does not repeat the value. - The parse error at `:72` no longer echoes `raw`. That site mattered most: a typo'd URL is exactly where a stray `user:password@` lands, and the error reaches stderr before any validation can reject it. - `httpreq::redacted` is now the only sanctioned way to put a URL in an error, and the `POST` context uses it. Keeping it as a named function rather than inlining the clear means a future call site cannot quietly reintroduce the leak. **The sweep you asked for found no other site.** `{url}`/`{raw}` appears three times in `src/`: the two above and `HTTPREQ_HTTP_TIMEOUT`'s `{raw:?}`, which is a duration and carries nothing sensitive. Nothing else `Display`s a `Url` into an error or log. **Breaking**, narrowly, and in the v2.1 notes: an endpoint using the embedded-credentials shortcut now fails at startup. Nothing in the fleet does — devinfra's is `https://dns01.isohex.uk:8443` with the credentials in separate variables. Documented in the README's `HTTPREQ_ENDPOINT` row and in `edge-router.example.toml`, which this issue correctly noted said nothing against it. 373 tests pass, clippy `--all-targets -D warnings` clean, fmt clean.

james.oates closed this issue

2026-10-27 10:23:05 +00:00

No labels

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

IsoHex/edge-router#58

No description provided.

Rows
Columns