Verify homeserver-supplied keys before using them - #16
Conversation
mx.crypto 0.2.0 shipped mxc_verify_device_keys(), mxc_verify_one_time_key() and mxc_ed25519_verify() so callers could validate what the homeserver hands back. mx.client called none of them: it signed its own keys on the way out and trusted everything on the way in, which defeats the point of end-to-end encryption. /keys/query: mx_crypto_known_devices() read dk[[uid]][[dev]]$keys straight out of the response with the Ed25519 self-signature sitting unread beside it. It now verifies each device and uses the keys the verifier actually checked. A device that fails is dropped with a warning and the rest still get the message, so one malformed device cannot make a room unusable. /keys/claim: mx_crypto_claim_otks() took slot[[1]]$key and discarded both the signatures block and the signed_curve25519 map key the verifier needs. It now verifies against the device's already-verified Ed25519. Devices left without a usable key are filtered in mx_send_encrypted() rather than aborting the whole send. Outbound Olm payloads carried only type and content, omitting the spec's sender/recipient/recipient_keys/keys block. That left receivers nothing to authenticate against and other clients reject such payloads outright. Inbound, decrypting an Olm message only proves it was encrypted to our key, not that it was meant for us here, so a server could replay a captured payload. mx_crypto_process_sync() and mx_crypto_handle_to_device() now check the recipient block. process_sync also records the sender the Olm payload attested to when sharing each Megolm session and reports it as sender_verified; when the cleartext envelope disagrees the event is dropped, since that is a forged sender. Session stores written before this change hold a bare megolm_in pickle and are migrated on load rather than discarded, so a running client keeps its history. Those sessions carry no attestation and report sender_verified = FALSE. Adds inst/tinytest/test_transport.R, which R/transport.R never had. The verification logic is split into pure helpers taking parsed responses, so tampered fixtures (swapped curve25519, stripped signatures, reattributed user or device, OTK signed by the wrong key) are covered without a homeserver.
rformat_dir("R") reflows this file, which this branch never touched. The
repo was not rformat-clean; isolating the churn here.
mx.crypto is a Suggests and has no r2u binary, so install_deps skipped it and every crypto test hit exit_file(). The tinytest step ran in 4ms and CI went green having exercised none of the encryption path. Add a Rust toolchain and install mx.crypto (plus simplermarkdown, needed for the vignette to be recognised as one) on both legs.
mx.crypto arrives as a binary on both legs (r2u r-cran-mx.crypto on Linux, a CRAN .tgz on macOS), so nothing compiles and the rustup step was dead weight with a misleading comment.
Review caught that the sender check was circular. mx_crypto_check_olm_payload took sender_curve25519 and never used it, so the only thing backing sender_verified was agreement between the Olm payload's claimed sender and the cleartext envelope. A hostile homeserver writes both: it can inject a to-device room key claiming any sender, using that sender's public ed25519, then stamp the timeline envelope to match. The two halves corroborate each other and the event came back marked verified. An ordinary user could not do this, since the server stamps the real sender on the envelope, but a hostile server is the threat this whole change exists for. mx_crypto_process_sync() and mx_crypto_handle_to_device() now take a devices list from mx_crypto_known_devices(). sender_verified is TRUE only when the payload's (sender, ed25519, curve25519) matches one of those verified devices as a triple, which is the part the server cannot forge: it needs a device_keys object carrying a valid self-signature. Callers passing no devices still decrypt and still catch envelope/claim disagreement, but never get sender_verified = TRUE. sender_bound persists with the session, so a store round-trip keeps the binding without re-supplying the device list.
|
Review was right on all three. Fixed. Finding 1 (this PR). Confirmed and it was worse than a dead parameter.
I kept the unbound claim rather than dropping the payload, consistent with the skip-and-warn policy already agreed for device verification: a stale device list shouldn't stop a legitimately-new device from being decryptable. The gap closes because the lie is no longer labelled verified. New tests: unbound decrypts but reports FALSE; a device list whose curve25519 isn't the sending device fails to bind; the matching device binds. Each scenario uses its own account and key share, since Olm one-time keys are single-use and a replayed prekey can't open a second session. Findings 2 and 3 are on #15, both confirmed and fixed there. 103 tests, 0 failures. |
Version and NEWS conflicts only: main brought 0.1.1.2 (tables), this branch carries 0.1.1.3 (key verification). Both NEWS sections kept.
1 participant
Fixes four holes in the E2EE path. mx.crypto 0.2.0 shipped
mxc_verify_device_keys(),mxc_verify_one_time_key(), andmxc_ed25519_verify()specifically so callers could validate what the homeserver returns. mx.client called none of them: it signed its own keys on the way out and trusted everything on the way in.The holes
1.
/keys/queryunverified.mx_crypto_known_devices()readdk[[uid]][[dev]]$keysstraight out of the response, with the Ed25519 self-signature sitting unread in the same object. A malicious or compromised homeserver could substitute its own Curve25519 key for any device and read everything sent to that user. This is the textbook homeserver MITM.2.
/keys/claimunverified.mx_crypto_claim_otks()didd$otk <- slot[[1]]$key, discarding thesignaturesblock and thesigned_curve25519:<id>map key that the verifier needs. The comment on the line above even named the signatures it then ignored.3. Outbound Olm payloads were not spec-conformant.
mx_crypto_room_key_payload()emitted onlytypeandcontent, omittingsender,recipient,recipient_keys, andkeys. That left receivers nothing to authenticate against, and other clients reject such payloads. This is also an interop bug, not only an internal one.4. Inbound messages were unauthenticated. Decrypting an Olm message proves it was encrypted to our key, not that it was meant for us in this context, so a server could replay a captured payload. And
mx_crypto_process_sync()reportedsenderfrom the cleartext envelope, which the server is free to set, rather than the attested sender inside the Megolm payload.Failure policy
Skip and warn, not hard error. A device that fails verification is dropped with a warning naming it, and the remaining devices still receive the message. One malformed device must not brick a room.
mx_send_encrypted()filters devices left without a usable one-time key rather than lettingmx_crypto_encrypt_for_devices()abort the send.Forged senders are the exception: when the envelope disagrees with the attested sender the event is dropped, since that is not a nuance.
Migration
megolm_innow storeslist(session, sender, sender_ed25519)instead of a bare session, which changessessions.json.mx_crypto_sessions_load()reads both shapes. Without that, every running bot would silently lose its inbound sessions and stop decrypting history. Legacy sessions carry no attestation and reportsender_verified = FALSE.Signature changes
mx_crypto_room_key_payload()gainssender_user_id,sender_ed25519,recipient_user_id,recipient_ed25519mx_crypto_encrypt_for_devices()gainssender_user_id; recipients must now carry a verifieded25519mx_crypto_handle_to_device()gainsself_id,self_ed25519corteza only calls
mx_crypto_process_sync()andmx_send_encrypted(), whose signatures are unchanged.res$eventskeeps its existing fields;sender_verifiedis additive.Verification
R/transport.Rhad no test file at all, which is why this shipped. The verification logic is now split into pure helpers taking parsed responses, so it tests without a homeserver.New coverage: swapped curve25519, stripped signatures, device reattributed to another user or device id, one bad device not taking good ones with it, OTK signed by the wrong device, device with no verified ed25519, forged envelope sender, Olm payload addressed to someone else, and a legacy-shape store still loading and decrypting.
R CMD check: 0 errors, 0 warnings. The one NOTE is the pre-existingDate: 2026-06-13being over a month old, which is a release-time field and not touched here.Notes
rformat_dir("R")reflowedR/messages.R, which this branch never touched. Isolated in its own commit.mxc_verify_device_keys()deliberately accepts any Ed25519 key a device claims for itself and does not pin against a previously trusted one. Pinning is a separate piece of work.