NFR-SEC-3 — Nextcloud traffic is HTTPS only; http:// is upgraded, never sent #65

Closed
opened 2026-09-25 00:18:26 +00:00 by dtourolle · 1 comment
Owner

Every request to a Nextcloud server goes over HTTPS. An http:// address is upgraded to https:// wherever it enters the app, and a request that would still go out over http is refused rather than sent.

Why

NFR-SEC-3 says all network traffic uses TLS. Today that holds only for the address the user types.

  • NextcloudProvider::normalise_endpoint (core/dr-sync-nextcloud/src/provider.rs:83) upgrades http:// and bare hosts to https://, and direct app-password sign-in goes through it.
  • Browser sign-in (Login Flow v2) bypasses it. account_from (provider.rs:35) saves the account with creds.server, which comes from the server's poll response, not from the address the user typed. A Nextcloud behind a reverse proxy without overwriteprotocol returns "server": "http://…", a common misconfiguration. The account is then stored with that address, and every later request sends Basic auth (login and app password) in cleartext.
  • The same response can name a different host. The saved account then sends the credentials to that host.
  • The client itself allows http. http_client (core/dr-sync-nextcloud/src/lib.rs:701) does not set https_only(true), so nothing below the endpoint check stops a plain-http request. That includes one built from an href in a PROPFIND response, or from an account saved before this fix.
  • login_url is opened unchecked. It comes from the /login/v2 response and goes straight to xdg-open, or on Windows to rundll32 url.dll,FileProtocolHandler (ui/dr-ui/src/launch_ui.rs:561, :832). A file: or UNC value there can launch a program on Windows. It is the same "trust the server's URL" gap and belongs in this fix.

Found in the 2026-09-24 security review.

Deliverable

  1. Save the typed address. After browser sign-in, save the endpoint the user typed (already normalised). Refuse a poll result whose server has a different host. A result that differs only by http vs https is upgraded, not refused.
  2. Build the client HTTPS-only. Call .https_only(true) in http_client, so any http URL fails before a socket opens, whatever produced it.
  3. Upgrade saved accounts. When accounts are loaded, rewrite any stored http:// endpoint to https:// and save it back. The catalog directory is keyed on the account, so check the namespace does not change.
  4. Check the browser URL. Before opening login_url, require https and the same host as the endpoint. Otherwise show a sign-in error.
  5. Tests. The requirement is NFR-SEC-3, and each test carries TRACES: NFR-SEC-3.

Acceptance

  • Browser sign-in against a server whose poll returns http://host stores https://host.
  • A poll result naming a different host fails sign-in with a message, and nothing is saved to the keyring.
  • http_client refuses an http:// request (unit test against a local listener: no connection is made).
  • An account stored as http://… in sessions.json loads as https://… and keeps its catalog directory.
  • open_in_browser is never called with a non-https URL, or with one on another host (unit test on the check).
  • No setting or flag allows plain http in a release build.
**Every request to a Nextcloud server goes over HTTPS. An `http://` address is upgraded to `https://` wherever it enters the app, and a request that would still go out over `http` is refused rather than sent.** ## Why NFR-SEC-3 says all network traffic uses TLS. Today that holds only for the address the user types. - `NextcloudProvider::normalise_endpoint` (`core/dr-sync-nextcloud/src/provider.rs:83`) upgrades `http://` and bare hosts to `https://`, and direct app-password sign-in goes through it. - **Browser sign-in (Login Flow v2) bypasses it.** `account_from` (`provider.rs:35`) saves the account with `creds.server`, which comes from the server's poll response, not from the address the user typed. A Nextcloud behind a reverse proxy without `overwriteprotocol` returns `"server": "http://…"`, a common misconfiguration. The account is then stored with that address, and every later request sends Basic auth (login and app password) in cleartext. - **The same response can name a different host.** The saved account then sends the credentials to that host. - **The client itself allows `http`.** `http_client` (`core/dr-sync-nextcloud/src/lib.rs:701`) does not set `https_only(true)`, so nothing below the endpoint check stops a plain-http request. That includes one built from an `href` in a PROPFIND response, or from an account saved before this fix. - **`login_url` is opened unchecked.** It comes from the `/login/v2` response and goes straight to `xdg-open`, or on Windows to `rundll32 url.dll,FileProtocolHandler` (`ui/dr-ui/src/launch_ui.rs:561`, `:832`). A `file:` or UNC value there can launch a program on Windows. It is the same "trust the server's URL" gap and belongs in this fix. Found in the 2026-09-24 security review. ## Deliverable 1. **Save the typed address.** After browser sign-in, save the endpoint the user typed (already normalised). Refuse a poll result whose `server` has a different host. A result that differs only by `http` vs `https` is upgraded, not refused. 2. **Build the client HTTPS-only.** Call `.https_only(true)` in `http_client`, so any `http` URL fails before a socket opens, whatever produced it. 3. **Upgrade saved accounts.** When accounts are loaded, rewrite any stored `http://` endpoint to `https://` and save it back. The catalog directory is keyed on the account, so check the namespace does not change. 4. **Check the browser URL.** Before opening `login_url`, require `https` and the same host as the endpoint. Otherwise show a sign-in error. 5. **Tests.** The requirement is NFR-SEC-3, and each test carries `TRACES: NFR-SEC-3`. ## Acceptance - [ ] Browser sign-in against a server whose poll returns `http://host` stores `https://host`. - [ ] A poll result naming a different host fails sign-in with a message, and nothing is saved to the keyring. - [ ] `http_client` refuses an `http://` request (unit test against a local listener: no connection is made). - [ ] An account stored as `http://…` in `sessions.json` loads as `https://…` and keeps its catalog directory. - [ ] `open_in_browser` is never called with a non-`https` URL, or with one on another host (unit test on the check). - [ ] No setting or flag allows plain `http` in a release build.
dtourolle added the unmet-requirementsize:Msync labels 2026-09-25 00:18:26 +00:00
Author
Owner

Landed on master in adade27, ea31791 and 5569a06.

One change from the plan: a poll result naming a different host is not refused. The account is now always stored under the typed address, whatever the server reports, so refusing a different host would protect nothing. It would also break a working setup: a server reached by LAN address behind a proxy with overwritehost answers with its public name. The protection is in the scheme:

  • every URL the server sends is upgraded from http to https;
  • anything else (file:, UNC, javascript:) is refused;
  • the client refuses http outright.

Accounts already saved as http:// are rewritten on launch. The keyring entry moves with them, and the catalog directory is unchanged, because the namespace ignores the scheme.

Landed on master in adade27, ea31791 and 5569a06. **One change from the plan:** a poll result naming a different host is *not* refused. The account is now always stored under the typed address, whatever the server reports, so refusing a different host would protect nothing. It would also break a working setup: a server reached by LAN address behind a proxy with `overwritehost` answers with its public name. The protection is in the scheme: - every URL the server sends is upgraded from http to https; - anything else (`file:`, UNC, `javascript:`) is refused; - the client refuses `http` outright. **Accounts already saved as `http://`** are rewritten on launch. The keyring entry moves with them, and the catalog directory is unchanged, because the namespace ignores the scheme.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dtourolle/DarkRoom#65