admin/raven
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Security review pass 2: login, tokens, password reset, 2FA, anonymous routes

Browse Source 
F5 (critical, confirmed on the local server): the 2FA step accepts an empty or
missing temp token, so users with 2FA on can be logged into without a password.
F1 confirmed: with the placeholder secret a user's current token can be rebuilt.
F8 confirmed. New findings F6, F7, F9 to F12. Review only, no code changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
John Cardinal
2026-10-23 19:24:45 -07:00
parent 2a9664dbe0
commit 26173b3926
1 changed files with 124 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

136
devdocs/modernization/SECURITY-REVIEW.md
View File

@@ -12,8 +12,8 @@ keep it that way while findings are open.
| Pass | Area | Status |
|---|---|---|
| 1 | Threat model and attack-surface map | Done 2026-10-23 |
| 2 | Unauthenticated routes, login, tokens, password reset, 2FA | Next |
| 3 | Customer contact users: what they can reach and read | |
| 2 | Unauthenticated routes, login, tokens, password reset, 2FA | Done 2026-10-23 |
| 3 | Customer contact users: what they can reach and read | Next |
| 4 | Report rendering (templates, the server-side browser, report data and download routes) | |
| 5 | File handling: attachments, uploads, exports, logs, backups, restore, translations, logo | |
| 6 | Raw SQL: data lists, saved filters, search, KPI, schedule, pick lists | |
@@ -186,23 +186,134 @@ Status: *candidate* (seen in code, not yet confirmed), *confirmed*, *fixed in ca
| ID | Severity | Status | Title |
|---|---|---|---|
| F1 | High | candidate | Placeholder JWT secret in the LAN/IIS and Linux server config templates |
| F2 | Medium | candidate | Windows installers give all local users modify rights on the program and data folders |
| F5 | **Critical** | **confirmed** | 2FA login step accepts an empty or missing temp token: the password is skipped for users with 2FA on |
| F1 | Critical (installs that kept it) | **confirmed** | Placeholder JWT secret in the LAN/IIS and Linux server config templates |
| F3 | High | candidate | Any logged-in user, customer contacts included, can send arbitrary email through the company's SMTP server |
| F6 | Medium | confirmed | No lockout or rate limit on login or the 2FA step; no password rules |
| F9 | Medium | confirmed | Default SuperUser password, and an anonymous route that says whether it is still set |
| F2 | Medium | candidate | Windows installers give all local users modify rights on the program and data folders |
| F7 | Low | confirmed | Password change or reset does not end existing sessions, and makes the download key non-expiring |
| F8 | Low | confirmed | Latest-token check skipped for any URL ending in `/auth` or `notify/hello` |
| F4 | Low | candidate | Any logged-in user can send an in-app notification to any user |
| F10 | Low | confirmed | Anyone can record acceptance of the licence agreement (`license/lc`) |
| F11 | Low | confirmed | API root page shows licensee, version, licence terms (and DB ID when unlicensed) to anyone |
| F12 | Low | confirmed | Password hashing: PBKDF2-SHA512 with 10,000 iterations |
### F1 Placeholder JWT secret (from the 8.3.0 installer test)
### F5 2FA step skips the password (pass 2, confirmed on the local server)
`AuthController.TfaAuthenticate` (`POST auth/tfa-authenticate`, anonymous) finds the user
only by `TempToken == pin.TempToken` (plus active, login allowed, 2FA on), then checks the
6-digit code. The temp token is the only proof that the password was given in step one, but:
- after a successful 2FA login it is set to `""` (`AuthController.cs:193`), so sending
`"tempToken": ""` matches every 2FA user who has completed a 2FA login;
- a user who has just turned 2FA on has it `null`; sending no temp token matches them;
- `TempToken` is not required in `TFAPinParam`, and a wrong code does not clear it.
So anyone, with no login and no password, can send codes to this route and log in as
whichever 2FA user the query returns first. TOTP accepts 3 codes at any moment
(TwoFactorAuth.Net default discrepancy 1), the only throttle is the 3 s failure delay,
which parallel requests get around, and there is no lockout (F6) and no nginx rate limit on
hosted droplets. At a few hundred guesses per second that is, on average, well under an
hour; even at 20 per second, a few hours. Users who turn 2FA on are the more privileged
ones, so 2FA makes those accounts weaker than a password alone.
**Proof** (local dev server, seeded `Inventory` user, 2FA turned off again afterwards):
2FA just turned on, no password, temp token omitted, correct code: 200 and a token for
Inventory. After one normal 2FA login, no password, temp token `""`, correct code: 200 and a
token. Wrong code: 401. (Script kept outside the repo; the test needs the user's code, which
an attacker guesses.)
**Fix** (proposed for 8.3.0): reject a null or empty temp token before the query; clear the
temp token (set it to null) on success and on every failed code, so each guess needs the
password again; optionally expire the temp token after a few minutes. Small change in one
method; client unaffected (it always sends the `tt` it received).
### F1 Placeholder JWT secret (from the 8.3.0 installer test; confirmed in pass 2)
`dist/assets/lan-install-config.json` and `dist/assets/linux-server/config.json` ship
`AYANOVA_JWT_SECRET` set to a placeholder that is also printed in the manual. The manual
tells users to change it (Linux: "optional but recommended"). An installation that keeps it
signs logins with a publicly known secret: anyone who knows it can mint a token for any user
id, including SuperUser, as far as the signature goes. What else such a forged token needs
(the per-user latest-token check, `CurrentAuthToken`) is to be confirmed in pass 2, as it may
limit this considerably. Hosted droplets are not affected (the setup script generates a
random secret since `64d8f62`; droplets created earlier: to check). Fix options: generate a
random secret at install time; refuse to boot with the placeholder; or treat the placeholder
as unset (random per boot).
signs logins with a publicly known secret.
**Pass 2: the latest-token check does not save it.** Tokens are deterministic: the payload is
only `exp` (login time + 5 days, in seconds), `iss` and `id`, with no random part
(`AuthController.cs:280-291`); two logins in the same second give identical tokens. So with
the secret, someone with no login can rebuild a user's *current* token by trying each second
of the last 5 days: at most 432,000 requests per user, each rejected at once (no delay). It
works for any user who logged in in the last 5 days, SuperUser included. **Proof**: with the
local dev secret, a forged token for `Inventory` was accepted after 6 tries and was identical
to the real one. In addition, routes ending in `/auth` (F8) accept a forged token without
any search, and behind a reverse proxy on the same machine that does not set
`X-Forwarded-For`, a forged token with an `rpl` claim and `Host: 127.0.0.1` skips the check
too (`IsLocalExtension`: remote address equals local address).
Hosted droplets set up with the current script are not affected (random secret since
`64d8f62`); **droplets created before that: to check (John)**. Self-hosted IIS and Linux
servers that are reachable from the internet and kept the placeholder are fully open.
Fix options: treat the placeholder (and any secret shorter than 32 characters) as unset, so
a random secret is used per boot, and log a warning (users would log in again after each
restart); or generate a random secret at install time (Windows installer, Linux packages);
or refuse to boot. Adding a random value (`jti`) to the token is cheap hardening as well.
### F6 No lockout or rate limit on login and 2FA (pass 2)
`POST auth` and `POST auth/tfa-authenticate` delay 3 s on failure but count nothing: no
per-user or per-address lockout, and parallel requests are not slowed. Hosted nginx has no
`limit_req`. There is no password rule beyond "not empty" (seeded trial users use the login
as password). Fix options: count failures per login and per temp token with a short lockout;
`limit_req` on `/api/v8/auth` in the hosted nginx template.
### F7 Password change does not end sessions; download key loses its expiry (pass 2)
`UserBiz.ChangePasswordAsync` (used by change-password and reset-password) does not replace
`CurrentAuthToken`, so a session someone else holds keeps working up to 5 days after the
owner changes the password. It also sets `DlKeyExpire = null`, apparently meant to be
`PasswordResetCodeExpire`; with a null expiry `ValidateDownloadTokenAndReturnUserAsync`
never treats the key as expired, until the next login replaces it. Setting `AllowLogin` to
false does not end a session either (only deactivating does). Fix: on password change, set
`CurrentAuthToken` and `DlKey` to new random values and clear `PasswordResetCodeExpire`.
### F8 Latest-token check bypassed by the URL suffix (pass 2, confirmed)
The middleware skips the `CurrentAuthToken` comparison when the path ends with `/auth` or
`notify/hello` (`Startup.cs:409-413`, `EndsWith`), meant for the login and pre-login routes.
Any route whose last segment is free text can be called with a replaced or revoked token
(e.g. `tag-list/batch-add/auth`, `form-custom/auth`). Confirmed: a replaced token got E2004
on `form-field-reference/Customer` and passed the middleware on `form-field-reference/auth`.
The middleware also does not check `Active`. Impact is small on its own (only those routes,
with the user's current roles), larger with F1. Fix: compare the exact route
(`/api/v8/auth`, `/api/v8/notify/hello`), or skip the check by endpoint metadata
(`[AllowAnonymous]`) instead of by path.
### F9 Default SuperUser password, advertised anonymously (pass 2)
A new database has SuperUser `superuser` / `l3tm3in` (`PrimeData.cs`). Anonymous
`GET notify/hello` returns `sudf: true` while that is still the password, and the login form
then fills it in. Anyone can therefore check an internet-facing server for it in one request
and log in as SuperUser. Each call also runs a PBKDF2 hash, a cheap way to load the server.
Whether hosted droplets set their own SuperUser password at setup: to check in pass 10.
Fix options: only return `sudf` from local or LAN addresses, or force a password change on
first SuperUser login; at least warn in the UI and the manual.
### F10 Anonymous licence-agreement consent (pass 2)
`POST license/lc` with body `"iaccepttheagreement"` records the EULA consent; the comment says
"SuperUser only", but the route is anonymous. Legal rather than technical: the consent record
does not prove who accepted. Fix: require SuperUser.
### F11 Anonymous API root page discloses licence details (pass 2)
`GET /api/v8/` returns an HTML page with the exact version, "Registered to", licence type,
expiry dates and licensed features; unlicensed, the server DB ID. Helps an attacker pick
targets by version. Fix: show it only to logged-in users, or show the version only.
### F12 Password hashing strength (pass 2)
PBKDF2-HMAC-SHA512, 10,000 iterations, 32-byte random salt (`Hasher.cs`). Current guidance
(OWASP) is 210,000 iterations for SHA-512. Only matters if the database or a backup leaks.
Raising it needs re-hashing at next login (store the iteration count). Roadmap item, not 8.3.0.
### F2 Windows folder permissions (from the 8.3.0 installer test)
@@ -236,3 +347,4 @@ by a customer contact. Pass 8.
| Date | Pass | Result |
|---|---|---|
| 2026-10-23 | 1 Threat model and map | Scope agreed with John. Middleware, JWT validation, role model, download keys, CORS and Swagger read. Route map generated (460 actions; script kept in `security/routemap.py`). No route lacks authentication by accident. 53 logged-in routes have no role check in the action: 3 resolved (erase data, trial seed: OK; direct SMTP: F3), the rest assigned to later passes. Local-request exception not reachable through the hosted nginx. Findings F1 to F4 recorded as candidates |
| 2026-10-23 | 2 Login, tokens, reset, 2FA, anonymous routes | Read `AuthController`, the token middleware, `Hasher`, download-key check, password reset, TOTP setup, and every anonymous route (`notify/hello`, `license/lc`, `translation/subset`, `logo`, API root, health). Tested on the local dev server with the seeded `Inventory` user: **F5 critical, confirmed** (2FA step without password); **F1 confirmed** (current token rebuilt from the secret in 6 tries); F8 confirmed. New: F6, F7, F9 to F12. OK: reset codes (32 random characters, 48 h, cleared on use, same reply for unknown codes), login replies do not reveal whether a login exists, roles read from the database on every request, deactivation ends the session, TOTP secret 160 bits. Leads for pass 4: `AuthController.GenRpt` issues SuperUser (id 1) tokens with an `rpl` claim for report rendering; pass 5: the logo is served with the content type given at upload |