ServerSecurity.md (12361B)
1 # Crossmate Audit 2 3 Date: 2026-06-12 (updated 2026-06-16) 4 5 Scope: Crossmate iOS/iPadOS app, shared app/extension code, tests, scripts, and 6 Cloudflare Workers used by Crossmate. Crossmake was intentionally excluded. 7 8 ## Summary 9 10 The highest-risk issue was the push worker's authentication model. The app 11 previously shipped a shared push bearer in its bundle configuration, and the 12 worker treated that bearer as the only authorization gate for registering 13 devices and publishing pushes. Anyone who extracted that value from a real app 14 build could use the worker as Crossmate. That issue has been addressed by 15 replacing shared bearer authentication with per-install App Attest enrollment and 16 signed worker requests. A temporary legacy-bearer fallback behind an 17 environment flag was subsequently removed and the legacy push-bearer secret 18 deleted from the worker (see Finding 1), so App Attest is 19 now the only authentication path. 20 21 The rest of the audit found lower-risk operational and robustness issues around 22 push environment handling, local secret storage, engagement room authentication, 23 and destructive local-store recovery behavior. All of these have since been 24 addressed. The most substantive was the engagement room authentication design 25 (Finding 4): the secret-in-URL symptom sat on top of a trust-on-first-use 26 room-auth scheme whose HMAC signature added no security. Rooms are now 27 registered with the worker ahead of connecting (secret in a POST body, never a 28 URL), and the connect signature is verified against the worker-held secret. 29 30 ## Findings 31 32 ### 1. Push worker used a bearer secret embedded in the app 33 34 Status: Fixed. 35 36 Impact: High. 37 38 The app read `CrossmatePushBearer` from its bundle configuration and sent it to 39 the push worker. The Cloudflare worker accepted that shared value as sufficient 40 authorization for `/register`, `/publish`, and unregister operations. Because the 41 same bearer was present in every distributed build, extracting one app binary 42 would expose the credential for all clients. 43 44 The implemented replacement keeps Crossmate account-free while removing the 45 global shared secret. Updated clients now enroll a per-install App Attest key 46 with the worker, store the key ID locally, and sign each protected push request. 47 The worker verifies Apple App Attest attestation, stores only the public key and 48 assertion counter, and accepts push operations only when signed by the registered 49 key. Existing collaborative games remain compatible for updated clients because 50 CloudKit game state and push address derivation did not change; clients simply 51 re-register the same push addresses under the new authentication scheme. 52 53 The worker initially retained a legacy bearer fallback behind an 54 environment flag as a rollback window. Resolved 55 2026-06-12: the fallback branch was removed from `Workers/push-worker.js`, the 56 updated worker was redeployed, and the legacy push-bearer secret was 57 deleted from the worker. App Attest is now the only authentication path. 58 59 Deployment notes: 60 61 - `APP_ATTEST_ROOT_CERT_PEM` has been uploaded to Cloudflare from 62 Apple's published App Attest root certificate. 63 - `crossmate-push` has been deployed with App Attest environment and app identity 64 variables. 65 - The regenerated `Crossmate iOS Distribution` provisioning profile includes 66 `com.apple.developer.devicecheck.appattest-environment`. 67 - App Attest enrollment and signed worker requests have been validated on a real 68 client after the initial implementation session. 69 70 ### 2. Push environment handling can diverge between APNs entitlement and client registration 71 72 Status: Fixed. 73 74 Impact: Low (debug builds only). 75 76 `Crossmate.entitlements` hardcoded `aps-environment: production`, while 77 `PushClient` selected `sandbox` under `#if DEBUG` and `production` otherwise. 78 TestFlight and App Store builds are release-signed, so both sides agreed on 79 production there; the mismatch only affected local device debug builds, where the 80 token's APNs environment and the environment registered with the worker could 81 disagree and pushes silently fail. That failure mode was a diagnosability 82 annoyance during development rather than a user-facing risk. 83 84 Resolved 2026-06-12: a single `APS_ENVIRONMENT` build setting in `project.yml` 85 (`development` for Debug, `production` for Release) is now substituted into both 86 the `aps-environment` entitlement and a `CrossmateAPSEnvironment` Info.plist 87 key. `PushClient` reads the Info.plist key instead of inferring from 88 `#if DEBUG`, and disables push (failable init, logged to diagnostics) if the 89 value is missing or unrecognised. Because the entitlement and the worker 90 registration now derive from the same variable, they cannot diverge. This also 91 corrects the Debug entitlement itself: development-signed builds previously 92 claimed `production` while their provisioning profile said `development`. 93 94 ### 3. Account push secret storage and generation path is weakly guarded 95 96 Status: Fixed. 97 98 Impact: Low. 99 100 The account push secret is stored in local defaults, and random generation 101 failure was not treated as a hard failure: `generatePushSecret()` in 102 `AccountPushCoordinator` discarded the `SecRandomCopyBytes` result, so on a 103 (vanishingly unlikely) failure the minted secret would be the base64 of 32 zero 104 bytes. 105 106 Moving the local copy to the Keychain would change little, because the secret is 107 deliberately replicated across the account's devices as a CloudKit `Decision` 108 record — the private CloudKit database is already its dominant home. The 109 severity is therefore Low rather than Medium. 110 111 The boundary this finding originally documented — the secret is the HMAC key 112 from which per-game push addresses are derived, those addresses acted as bearer 113 capabilities, and the worker's `handlePublish` did not check that a caller 114 participates in the game it pushed to, so any App Attest-enrolled client that 115 learned a derived address could publish to that account — has since been closed 116 (participation gating, below). 117 118 Resolved 2026-06-12 (secret hardening): `generatePushSecret()` now checks the 119 `SecRandomCopyBytes` status with a `precondition` (crashing is preferable to 120 durably converging a predictable HMAC key across the account's devices) and 121 reuses the existing `Data.base64URLEncodedString()` helper. 122 123 Resolved 2026-06-16 (participation gating): publishing to a game now requires 124 proving participation, mirroring the engagement room-secret scheme (Finding 4). 125 Each shared game carries a per-game push credential (`GamePushCredentials` — an 126 unguessable `credID` plus a 256-bit secret) in the Game record's `notification` 127 field, synced only to CKShare participants. Devices register their per-game push 128 addresses with the worker keyed under that `credID`, and a publish is signed 129 with the game secret (HMAC over the App Attest request's already-validated body 130 hash, timestamp, and nonce). The worker verifies the signature against the 131 secret it holds for the credID and resolves targets only among addresses 132 registered under it — so a caller can reach a game's participants only if it 133 holds that game's secret, i.e. it is a participant. A signature alone would not 134 have sufficed (the worker can't tell which addresses belong to which game), so 135 the device address registration is credID-bound too; that binding is the part 136 that actually closes the hole. Account-scoped sibling pushes 137 (accountJoined/accountSeen) carry no credID and stay on the App-Attest-only 138 path; their addresses derive from the account secret in the private database and 139 were never participant-spoofable. Deployment: the Game record gains a 140 `notification` String field (Development + Production) and the updated push 141 worker must be deployed; older clients lose game pushes (the durable CloudKit 142 path is unaffected), which is acceptable pre-release. 143 144 ### 4. Engagement room authentication is trust-on-first-use and its signature adds no security 145 146 Status: Fixed. 147 148 Impact: Medium. 149 150 The secret-in-URL exposure originally flagged here was the symptom; the room 151 authentication design underneath was the actual weakness. 152 153 The client sent both the room secret and an HMAC-SHA256 signature over 154 `roomID|authorID|deviceID|timestamp|nonce` as query parameters. The worker 155 verified that signature using the secret supplied in the same request 156 (`room-worker.js`), so anyone could mint a valid signature for any secret 157 they chose — the HMAC added nothing. The real gate was a trust-on-first-use 158 `secretHash` stored in the Durable Object: the first client to connect to a 159 room defined its secret, and later connections had to match it. The secret 160 itself was the bearer credential, and it travelled in the query string, where 161 it could appear in logs, diagnostics, proxies, and tooling surfaces. 162 163 Resolved 2026-06-12 (code): the worker gained a `POST /rooms/{id}/register` 164 endpoint that stores the room secret from the request body — first write wins, 165 re-registration with the same secret is an idempotent success, and a different 166 secret is refused with 409. The websocket connect no longer accepts a `secret` 167 parameter at all; the worker verifies the HMAC signature against the 168 registered secret and refuses unregistered rooms. Timestamp-freshness and 169 nonce-single-use checks already existed on the worker and are unchanged — with 170 a server-held key they now carry real authentication. Clients register 171 idempotently before every connect, which preserves the previous de-facto 172 behavior that an idle-expired Durable Object room is resurrected by whichever 173 creds-holder returns first, and removes any ordering race between the minting 174 peer and peers that receive the creds via CloudKit. A 409 (room ID registered 175 under a different secret, e.g. squatted after idle expiry) is surfaced as 176 `EngagementReconcileOutcome.roomRejected`, and `EngagementLifecycle` clears the 177 Game record's `engagement` field so a fresh room is minted and LWW converges 178 the peers onto it. 179 180 Deployed 2026-06-12: the updated worker refuses TOFU connects, so clients 181 running builds that predate this change lose the live engagement channel 182 (graceful — the durable CloudKit Moves path is unaffected). It was deployed 183 alongside the updated clients. 184 185 ### 5. Core Data store recovery can erase local state 186 187 Status: Fixed. 188 189 Impact: Medium. 190 191 The local persistence recovery path could wipe the Core Data store when store 192 load or migration fails. The in-code rationale 193 (`PersistenceController.recreateStore`) was that the store is a rebuildable 194 cache of CloudKit, so a wipe is recoverable via refetch. That rationale does not 195 hold in two cases: 196 197 - iCloud sync is user-toggleable (`isICloudSyncEnabled`). For a user with sync 198 disabled, the local store is the *only* copy of their games, and the wipe is 199 total, permanent data loss. 200 - Even with sync on, offline edits not yet uploaded through the sync engine are 201 lost. 202 203 Resolved 2026-06-12: `recreateStore` now moves the failing store files 204 (including the `-wal`/`-shm` sidecars) aside under a `.broken-<timestamp>` 205 suffix before destroying and rebuilding, so the data stays inspectable and 206 recoverable instead of being erased. The recovery event — including the original 207 load error and the preserved file names — is surfaced through the diagnostics 208 `EventLog`. `PersistenceController` gained an injectable store URL for tests, 209 and targeted tests now cover both the failure path (broken store preserved 210 byte-for-byte, empty store rebuilt) and the healthy path (reopening an existing 211 store triggers no recovery). Preserved backups are not auto-pruned; the path 212 requires a failed migration, so accumulation is not a practical concern. 213 214 ## Verification Performed 215 216 After the App Attest push-auth changes, these checks passed: 217 218 - `node --check Workers/push-worker.js` 219 - `xcodegen generate` 220 - `bash Scripts/build.sh` 221 - `bash Scripts/test-unit.sh` 222 223 Cloudflare was also updated operationally: 224 225 - `APP_ATTEST_ROOT_CERT_PEM` secret uploaded. 226 - `crossmate-push` deployed to its workers.dev URL. 227 228 ## Release Validation 229 230 The App Attest challenge/registration path and signed worker requests have been 231 validated on a real client. The remaining release checks are the broader 232 end-to-end collaboration and push-delivery behaviors: 233 234 - Fresh install registers for notifications. 235 - Two updated clients can continue an existing collaborative game. 236 - Background push delivery still works. 237 - Worker logs remain clean during normal registration and publish traffic.