security: fix the review leftovers (owner, 08 §6)

- Asterisk doesn't load app_system, func_shell, app_disa, res_agi, app_externalivr. - Turning two-step sign-in on asks for the password again. - *97/*98: one voicemail login at a time per phone, 10 an hour (sp-sub-vmlimit). - /prov: an unknown MAC gets the same 401 as a known one. - The login limiter sweeps keys past every window. Also the v1 checklist with the owner's results from 2026-10-21.
2026-10-21 10:54:44 -05:00
1 parent 3c8828eedb
commit 8d5475cfbf
@@ -849,11 +849,19 @@ Default routes, created with the first trunk (06 §4), in this order. Toll-free
 ```ini
 [sp-featurecodes]
 exten => *97,1,Answer()                                     ; my voicemail
+ same => n,Gosub(sp-sub-vmlimit,s,1)                         ; 1 at a time, 10/hour per phone (08 §6)
+ same => n,GotoIf($["${GOSUB_RETVAL}" != "ok"]?full)
 same => n,VoiceMailMain(${SP_EXT}@default)
 same => n,Hangup()
+ same => n(full),Playback(an-error-has-occurred)
+ same => n,Hangup()
 exten => *98,1,Answer()                                     ; any voicemail (asks mailbox)
+ same => n,Gosub(sp-sub-vmlimit,s,1)
+ same => n,GotoIf($["${GOSUB_RETVAL}" != "ok"]?full)
 same => n,VoiceMailMain(@default)
 same => n,Hangup()
+ same => n(full),Playback(an-error-has-occurred)
+ same => n,Hangup()
 exten => *78,1,Answer()                                     ; DND on
 same => n,Set(DB(SP/DND/${SP_EXT})=1)
 same => n,UserEvent(SpToggle,Key: DND,Ext: ${SP_EXT},Value: 1)
@@ -40,7 +40,8 @@ type ProvisionContext = {
 | `GET /prov/linphone/<token>.xml` | Linphone (§5) | token auth instead of Basic |

 Rules:
- Unknown MAC → 404, wrong/missing Basic auth → 401 with `WWW-Authenticate: Basic realm="selfphoneng"`.
+- Unknown MAC or wrong/missing Basic auth → the same 401 with `WWW-Authenticate: Basic realm="selfphoneng"`
+  (an unknown MAC was a 404 until 2026-10-21: the difference told which MACs exist, 08 §6).
  Both get logged with IP. Rate limit: 20 requests / 10 min per IP; after 10 failures, block that IP for 1 h.
 - If `provisioning.lanOnly` is true and the client IP isn't in `network.localNets` → 403.
 - On success: update `last_provisioned_at`, `last_provisioned_ip`. Set `Cache-Control: no-store`.
@@ -48,8 +49,9 @@ Rules:

 As built (M10-T1), `routes/provisioning.ts` + `provisioning/limiter.ts`:
 - A request **without** an `Authorization` header gets the 401 but isn't a failure: phones ask without
-  credentials first and retry after the challenge. Failures are unknown MACs (or a file for another
-  vendor) and wrong credentials; 10 within an hour block the IP for an hour (429).
+  credentials first and retry after the challenge. Failures are requests with credentials that
+  don't match: a wrong password, an unknown MAC, or a file for another vendor; 10 within an hour
+  block the IP for an hour (429).
 - The 20 requests / 10 min limit applies to clients **outside** `network.localNets`: an office's
  phones rebooting together behind one address would pass it in a minute. Failures count for all.
 - The Basic auth user name is the 12-hex MAC (either case); it can't carry colons (Basic auth splits
@@ -83,8 +83,8 @@ get the proxy's address only (below). The setting is on Settings → Security, w
  code, get 10 recovery codes once (`k7m3q-x9tpw`; case and dashes don't matter). Login asks for
  the code only after the password is right (401 `totp_required`, no session); a wrong code is a
  failed login (rate limit, audit). Each 30 s step signs in once (`users.totp_last_step`, updated
-  atomically) and each recovery code once (`array_remove` with an `any()` guard). Turning it off or
-  making new codes needs the password. Another admin can turn it off for a lost phone
+  atomically) and each recovery code once (`array_remove` with an `any()` guard). Turning it on or
+  off, or making new codes, needs the password. Another admin can turn it off for a lost phone
  (`POST /api/users/:id/reset-two-step`, signs that user out). `scripts/e2e/m11-t1-totp.mjs`.
 - As built (M11-T1): the headers come from an `onSend` hook on every response (UI, API, `/prov`);
  `connect-src` also allows `WEBRTC_WS_URL`'s origin when it's set. The dark-mode bootstrap moved
@@ -181,13 +181,14 @@ without a shell; CSV export escapes formulas; no `dangerouslySetInnerHTML`; sess
 stored as sha256; AES-256-GCM with a random IV per value; AMI, Asterisk HTTP and the api listen on
 127.0.0.1 only; webhooks check addresses at connect time (no DNS rebinding), follow no redirects.

-To do (owner, 2026-10-20, for the next image): `noload` in `asterisk/conf/modules.conf` for
-`app_system.so`, `func_shell.so`, `app_disa.so` and `res_agi.so`. Nothing uses them, but they are
-what a dialplan injection would reach for (`SHELL()` runs host commands). Check with
-`pnpm test:asterisk` before releasing.
-
-Left as they are, for the owner:
- Turning two-step sign-in on doesn't ask for the password again (turning it off does).
- `*98` allows 3 PIN tries per call, with no limit on calls; only internal phones reach it.
- A known MAC answers `/prov` with 401, an unknown one with 404 (07 §2 accepts this).
- The login limiter's memory grows with the number of distinct emails tried until restart.
+Fixed after the review (owner, 2026-10-21):
+- `asterisk/conf/modules.conf` doesn't load `app_system`, `func_shell`, `app_disa`, `res_agi` or
+  `app_externalivr`. Nothing uses them, but they are what a dialplan injection would reach for
+  (`SHELL()` runs host commands). `pnpm test:asterisk` passes without them.
+- Turning two-step sign-in on asks for the password again, like turning it off (`totp.test.ts`).
+- `*97`/`*98`: one voicemail login at a time per phone and 10 an hour (`sp-sub-vmlimit`, AstDB
+  `SP/VMLOGIN/<ext>` = `<UTC hour>:<count>`), each with 3 PIN tries (`voicemail-test.ts`).
+- `/prov`: an unknown MAC gets the same 401 as a known one, so the answers don't tell which MACs
+  exist; with credentials it counts as a failure (`devices.test.ts`).
+- The login limiter forgets keys past every window, swept at most once a minute
+  (`login-limiter.test.ts`).
@@ -11,32 +11,39 @@ Tick a box only with proof: a date and what was seen.

 ## Real voip.ms account (M3)

- [ ] The trunk shows **Registered** in the UI.
+- [x] The trunk shows **Registered** in the UI (2026-10-21: green, voip.ms, the sub-account at
+      its POP).
 - [x] A mobile → DID call rings a phone. 2026-10-21: DID → digital receptionist → browser
      phone; the DID came in the Request-URI. Still to do: write that down in 06 §3.
- [ ] An outbound call shows the DID as caller ID on the mobile. Write down in 06 §4 whether
-      per-call caller ID (Remote-Party-ID) is honored.
+- [x] An outbound call shows the DID as caller ID on the mobile (2026-10-21, owner).
+- [ ] Write down in 06 §4 whether per-call caller ID (Remote-Party-ID) is honored.

 ## Real phones (M8, M10)

 - [x] Linphone on a real phone registers by scanning the QR (2026-10-20, iPhone).
- [ ] An incoming call rings that Linphone while the app is open (fixed in v0.0.2, needs
-      checking on the server).
+- [x] An incoming call rings that Linphone while the app is open (2026-10-21, v0.0.2: DID →
+      receptionist → 100 rings the iPhone).
 - [ ] A factory-reset Yealink pointed at the setup URL registers with the right name and BLF keys.
 - [ ] A Grandstream gets its keys the same way.
 - [ ] Paging auto-answers on a real Yealink or Grandstream.

+On hold until the owner has the phones (2026-10-21).
+
 ## Someone else installs it (M11)

 - [ ] An outside tester follows only the admin guide and gets inbound and outbound calls working
      on a fresh VM.
 - [ ] A backup restores on a second VM.

+On hold (2026-10-21).
+
 ## A week on the owner's server

 - [ ] 7 days handling real calls with no crash or restart of the api or Asterisk.
 - [ ] Nightly backups restore cleanly.

+In progress on the owner's server (v0.0.2, from 2026-10-21).
+
 ## Updating

 - [ ] `scripts/update.sh` written, reviewed and tested on a real server: updating a running
@@ -45,18 +52,20 @@ Tick a box only with proof: a date and what was seen.

 ## Security leftovers (08 §6)

- [ ] Asterisk no longer loads `app_system`, `func_shell`, `app_disa` and `res_agi`; checked with
-      `pnpm test:asterisk`.
- [ ] Owner decision, fix or accept each:
-  - [ ] Turning two-step sign-in on doesn't ask for the password again.
-  - [ ] `*98` allows 3 PIN tries per call, with no limit on calls.
-  - [ ] A known MAC answers `/prov` with 401, an unknown one with 404.
-  - [ ] The login limiter's memory grows with the number of distinct emails tried.
+Owner's decision (2026-10-21): fix all of them. Each is in 08 §6 with its test.
+
+- [x] Asterisk no longer loads `app_system`, `func_shell`, `app_disa`, `res_agi` or
+      `app_externalivr`; `pnpm test:asterisk` passes.
+- [x] Turning two-step sign-in on asks for the password again.
+- [x] `*97`/`*98`: one voicemail login at a time per phone, 10 an hour.
+- [x] An unknown MAC answers `/prov` like a known one (401).
+- [x] The login limiter forgets old entries.

 ## The release

 - [ ] Version 1.0.0 in every `package.json`, and a v1.0.0 entry in the [changelog](../CHANGELOG.md).
- [ ] The release builds and pushes images to both the Gitea registry and ghcr.io.
+- [ ] The release builds and pushes images to both the Gitea registry and ghcr.io. (Gitea works.
+      ghcr.io: the api image pushes; the asterisk package exists, empty, and refuses the push.)

 ## Not needed for v1