hotfix(security): verify server responses & validate discovered peers (M-5) - #102
Merged
Conversation
… (M-5, excl. ATS) A malicious/compromised WevoSpace server (or MITM) could feed forged signatures/state that the client trusted without cryptographic verification. - MergeServerSignaturesIntoLocalProposeUseCase now VERIFIES every server-provided signature (P-256, v1 message bound to the LOCAL propose id/content-hash/participant keys) before persisting it, and only adopts a value when the local slot is empty. Forged or mismatched server signatures are rejected — they can no longer be written to the local store. (Injects KeychainRepository for verification.) - CheckProposeServerStatusUseCase now verifies the server's counterparty signature before surfacing a "counterparty signed" pending update, so a forged signature can't drive the UI. (The terminal-status prompt remains ungated but is harmless: accepting it routes through the now-verifying merge.) - FetchServerInfoUseCase sanitizes /info peer URLs before they are stored and used for API calls: only well-formed absolute http/https URLs with a host, de-duplicated and capped (16). Blocks a hostile primary from injecting malformed/odd-scheme peer endpoints. http is intentionally still allowed (ATS disabled by product decision — out of scope). Adds tests: rejection of unverified server signatures (merge + status check) and peer sanitization. All affected suites pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
h1d3mun3
added a commit
that referenced
this pull request
Jul 13, 2026
* feat: auto-apply branch protection ruleset on RC branch creation * fix(security): reject identity-mismatched Propose merges on import (#6) ImportProposeUseCase.verifyAllSignatures only proves the incoming file's signatures are valid for the keys *in the file* — not that they belong to the same agreement. Because the merge branch keeps the local participant keys but adopts the incoming signature fields (and never re-verifies them against the stored keys), an attacker who knows a Propose's UUID could AirDrop a file using their OWN P-256 keys plus valid honored/signed/parted/dissolved signatures and have the victim's app display a forged state transition attributed to the real counterparty. Guard the merge branch: require the incoming creatorPublicKey and counterpartyPublicKey to match the existing record before merging. Every adopted signature must then validate against the real participants' keys; the content hash is already bound by the verified creator signature. Mismatches throw the new ImportProposeUseCaseError.conflictingProposeIdentity. Adds regression tests for mismatched creator/counterparty keys. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(ci): make Create RC Branch workflow parseable; drop per-branch ruleset step The `permissions:` block used `administration: write`, which is not a valid GITHUB_TOKEN permission scope, so every dispatch failed at parse time with a startup_failure (no jobs ran). GITHUB_TOKEN also cannot manage repository rulesets at all, so the per-branch ruleset step could never have worked. Branch protection for rc-* is now handled by a standing repository ruleset ("Protect RC branches", targeting refs/heads/rc-*), so this workflow no longer creates per-branch rulesets and needs only `contents: write`. New RC branches are protected automatically on creation by pattern match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * hotfix(security): encrypt .wevo-identity export with a passphrase (M-4) (#101) * fix(security): encrypt identity export with a passphrase (M-4) The .wevo-identity export wrote the raw P-256 private key as base64 in plaintext JSON, so any leak of that file (mis-sent AirDrop, re-saved to Files/iCloud Drive, etc.) handed over a full, permanent credential able to forge every future record for that identity. - New passphrase-encrypted envelope (IdentityEncryptedExport, format version 1): metadata (id/nickname/publicKey) stays cleartext for preview; the private key is sealed with AES-GCM under a key derived from the user's passphrase via PBKDF2-HMAC-SHA256 (210k iterations, random 16B salt) — see IdentityExportCrypto. - Export now requires a passphrase (min 8 chars, confirmed) and writes with .completeFileProtection. Import requires the passphrase; a wrong passphrase or any tampering fails AES-GCM authentication (decryptionFailed). - Old plaintext exports are rejected on import with a clear message (legacyPlaintextUnsupported). - Adds the passphrase UI to export (IdentityDetailView) and import (IdentityImportView), and threads it through AuthenticateAndExport/Export/Import use cases and WevoApp. Tests: export→decrypt round-trip, wrong passphrase, tampered ciphertext, non-P256 material, legacy-plaintext rejection, encrypted-envelope read. All identity suites pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * harden identity export/import per crypto review (M-4 follow-up) Addresses findings from the adversarial review of the M-4 change: - (A) Bound the untrusted `iterations` from the envelope: readFromFile now rejects values outside 100k–2M, and deriveKey uses UInt32(exactly:) instead of a trapping conversion. Prevents a hard crash (UInt32 overflow/negative) and an abusive-PBKDF2 CPU/UI hang when importing a crafted/corrupt .wevo-identity file. - (B) Verify integrity of the cleartext metadata: import now derives the public key from the decrypted private key and rejects the file (publicKeyMismatch) if it disagrees with the envelope's stated publicKey, so a tampered preview/identity can't be imported. - (C) Surface readFromFile errors to the user: WevoApp.prepareIdentityImport now shows an "Import Failed" alert, so the legacy-plaintext / unsupported-format messages actually reach the user instead of being only logged. - (D) Enforce the minimum passphrase length (8) in the crypto layer (encrypt), not just the UI. Adds tests: out-of-range iteration rejection and public-key-mismatch rejection. All identity test suites pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * hotfix(security): verify server responses & validate discovered peers (M-5, excl. ATS) (#102) A malicious/compromised WevoSpace server (or MITM) could feed forged signatures/state that the client trusted without cryptographic verification. - MergeServerSignaturesIntoLocalProposeUseCase now VERIFIES every server-provided signature (P-256, v1 message bound to the LOCAL propose id/content-hash/participant keys) before persisting it, and only adopts a value when the local slot is empty. Forged or mismatched server signatures are rejected — they can no longer be written to the local store. (Injects KeychainRepository for verification.) - CheckProposeServerStatusUseCase now verifies the server's counterparty signature before surfacing a "counterparty signed" pending update, so a forged signature can't drive the UI. (The terminal-status prompt remains ungated but is harmless: accepting it routes through the now-verifying merge.) - FetchServerInfoUseCase sanitizes /info peer URLs before they are stored and used for API calls: only well-formed absolute http/https URLs with a host, de-duplicated and capped (16). Blocks a hostile primary from injecting malformed/odd-scheme peer endpoints. http is intentionally still allowed (ATS disabled by product decision — out of scope). Adds tests: rejection of unverified server signatures (merge + status check) and peer sanitization. All affected suites pass. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * Harden client import paths and identity handling (Low/Info) (#103) Safe hardening items from the security review (client Low/Info scope): - L-8: ImportIdentityFromExport now validates before deleting and requires explicit overwrite confirmation when an identity already exists (.identityAlreadyExists); IdentityImportView adds a "Replace existing identity?" confirmation alert. - L-9: fingerprint display widened from 8 to 16 hex chars (GetFingerprintUseCase, Identity.fingerprintDisplay). - L-10: all file imports go through readImportData (new ImportFileReading), enforcing a 1 MiB size cap before decoding (propose/identity/contact). - Info: ImportContactFromExport validates version == 1 and public-key parseability (typed errors); CreateIdentityUseCase misleading "SecureEnclave" comment corrected; ShareExtension pasteboard writes are now local-only with a 120s expiration. Tests updated and passing (WevoTests: ** TEST SUCCEEDED ** on iPhone 17 sim). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * chore: bump version to 1.2.0 (#105) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HotFix — verify server responses & validate discovered peers (M-5, ATS excluded)
Base:
main(hotfix). Security finding M-5 from the review. ATS/cleartext-http is intentional (per product decision) and is deliberately out of scope.Problem
A malicious or compromised WevoSpace server — or a MITM — could feed the client forged
signatures/state that it accepted without any cryptographic verification:
MergeServerSignaturesIntoLocalProposeUseCasepersisted server-provided signatures verbatim(
serverValue ?? localValue), so forged signatures could be written to the local store and shownas if the real creator/counterparty had signed.
CheckProposeServerStatusUseCasedrove "server has an update" UI purely from nil-presence ofresponse fields.
Fix
MergeServerSignaturesIntoLocalProposeUseCasenow cryptographically verifies everyserver-provided signature (P-256, v1 message
"<verb>." + id + payloadHash + signerKey + ts,bound to the local propose id / content hash / participant keys — never server-supplied keys)
before persisting, and only adopts a value when the local slot is empty. Forged or mismatched
server signatures are rejected. (Injects
KeychainRepository.)CheckProposeServerStatusUseCaseverifies the server's counterparty signature beforesurfacing a "counterparty signed" pending update. (The terminal-status prompt stays ungated but
is harmless: accepting it routes through the now-verifying merge, so it cannot corrupt state.)
FetchServerInfoUseCasesanitizes/infopeer URLs before they are stored and used for APIcalls — only well-formed absolute http/https URLs with a host, de-duplicated and capped (16) —
blocking a hostile primary from injecting malformed/odd-scheme endpoints. (http stays allowed;
ATS is intentionally disabled.)
Tests
reconstructed v1 message format matches — so legitimate merges still work and only forgeries are
rejected.
All affected suites pass on the iOS simulator.
(M-5 is a review ID, not a GitHub issue reference.)
🤖 Generated with Claude Code