From 8e821ab1921a3cdd33296f6b52f6f0c61a335f2f Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Sun, 2 Aug 2026 09:18:45 -0500 Subject: [PATCH 1/6] Verify homeserver-supplied keys before using them 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. --- R/crypto.R | 80 +++++++++++++++++-- R/e2ee.R | 100 ++++++++++++++++++++---- R/transport.R | 104 +++++++++++++++++++++---- inst/tinytest/test_crypto.R | 11 ++- inst/tinytest/test_e2ee.R | 60 ++++++++++++++- inst/tinytest/test_transport.R | 111 +++++++++++++++++++++++++++ man/mx_crypto_claim_otks.Rd | 9 ++- man/mx_crypto_encrypt_for_devices.Rd | 14 +++- man/mx_crypto_handle_to_device.Rd | 22 +++++- man/mx_crypto_known_devices.Rd | 9 ++- man/mx_crypto_room_key_payload.Rd | 15 +++- 11 files changed, 483 insertions(+), 52 deletions(-) create mode 100644 inst/tinytest/test_transport.R diff --git a/R/crypto.R b/R/crypto.R index ac535cb..582a9a7 100644 --- a/R/crypto.R +++ b/R/crypto.R @@ -182,23 +182,38 @@ mx_crypto_device_keys <- function(account, user_id, device_id) { #' @param recipient_curve25519 Character. Target device's Curve25519 key. #' @param room_id Character. Room the key is for. #' @param megolm_out An outbound Megolm session. +#' @param sender_user_id Character. This user's Matrix id. +#' @param sender_ed25519 Character. This device's Ed25519 key. +#' @param recipient_user_id Character. Target user's Matrix id. +#' @param recipient_ed25519 Character. Target device's Ed25519 key. #' @return A named list: the to-device \code{m.room.encrypted} content. #' @examples #' \dontrun{ #' content <- mx_crypto_room_key_payload(olm, my_curve, their_curve, -#' "!room:ex", megolm_out) +#' "!room:ex", megolm_out, +#' "@me:ex", my_ed, "@them:ex", their_ed) #' } #' @export mx_crypto_room_key_payload <- function(olm_session, sender_curve25519, recipient_curve25519, room_id, - megolm_out) { + megolm_out, sender_user_id, + sender_ed25519, recipient_user_id, + recipient_ed25519) { mx_require_crypto() info <- mx.crypto::mxc_megolm_outbound_info(megolm_out) + # The sender/recipient/keys block is not decoration: it is what lets + # the receiving device attribute the key to us and confirm the message + # was addressed to it. Omitting it (as this did before) leaves the + # payload unauthenticatable and other clients reject it outright. room_key <- list( type = "m.room_key", content = list(algorithm = MX_MEGOLM, room_id = room_id, session_id = info$session_id, - session_key = info$session_key) + session_key = info$session_key), + sender = sender_user_id, + recipient = recipient_user_id, + recipient_keys = list(ed25519 = recipient_ed25519), + keys = list(ed25519 = sender_ed25519) ) plaintext <- mx.api::mx_canonical_json(room_key) ct <- mx.crypto::mxc_olm_encrypt(olm_session, charToRaw(plaintext)) @@ -214,6 +229,38 @@ mx_crypto_room_key_payload <- function(olm_session, sender_curve25519, # ---- inbound (receive room key + decrypt) ---------------------------- +# Confirm a decrypted Olm payload was addressed to us. +# +# An Olm message decrypting successfully only proves it was encrypted to +# our Curve25519 key. It does not prove the sender meant it for us in this +# context: a homeserver can replay a payload it captured elsewhere. The +# spec's defence is the recipient block inside the plaintext, which is +# covered by the ratchet and so cannot be rewritten by the server. +# +# Returns TRUE when the payload is acceptable, FALSE (with a warning) +# otherwise. self_id may be NULL, in which case the user-id half of the +# check is skipped; callers that know their own id should always pass it. +mx_crypto_check_olm_payload <- function(decoded, self_id, self_ed25519, + sender_curve25519 = NULL) { + if (!is.null(self_id) && !identical(decoded$recipient, self_id)) { + warning("mx.client: dropping Olm payload addressed to ", + decoded$recipient %||% "", ", not ", self_id, + call. = FALSE) + return(FALSE) + } + if (!identical(decoded$recipient_keys$ed25519, self_ed25519)) { + warning("mx.client: dropping Olm payload whose recipient_keys do ", + "not match this device's Ed25519 key", call. = FALSE) + return(FALSE) + } + if (is.null(decoded$sender) || is.null(decoded$keys$ed25519)) { + warning("mx.client: dropping Olm payload with no sender identity", + call. = FALSE) + return(FALSE) + } + TRUE +} + #' Decrypt an inbound Olm to-device payload #' #' Accepts an \code{m.room.encrypted} to-device content addressed to this @@ -221,24 +268,39 @@ mx_crypto_room_key_payload <- function(olm_session, sender_curve25519, #' the caller builds an inbound Megolm session from #' \code{content$session_key} with \code{mx_crypto_inbound_session()}. #' +#' The decrypted payload is checked against this device's identity before +#' it is returned: a message that does not name us as recipient is +#' dropped, because decrypting successfully only proves it was encrypted +#' to our key, not that it was meant for us here. +#' #' @param account An mx.crypto account handle. #' @param my_curve25519 Character. This device's Curve25519 key. #' @param content The to-device \code{m.room.encrypted} content. -#' @return The decrypted event (a parsed list), or NULL if not for us. +#' @param self_id Character or NULL. This user's Matrix id. When NULL the +#' recipient user-id check is skipped; pass it whenever it is known. +#' @param self_ed25519 Character or NULL. This device's Ed25519 key. +#' Defaults to the account's own key. +#' @return The decrypted event (a parsed list), or NULL if it was not for +#' us or failed the recipient checks. #' @examples #' \dontrun{ -#' ev <- mx_crypto_handle_to_device(acct, my_curve, td_content) +#' ev <- mx_crypto_handle_to_device(acct, my_curve, td_content, +#' self_id = "@me:example.org") #' if (identical(ev$type, "m.room_key")) { #' inb <- mx_crypto_inbound_session(ev$content$session_key) #' } #' } #' @export -mx_crypto_handle_to_device <- function(account, my_curve25519, content) { +mx_crypto_handle_to_device <- function(account, my_curve25519, content, + self_id = NULL, self_ed25519 = NULL) { mx_require_crypto() msg <- content$ciphertext[[my_curve25519]] if (is.null(msg)) { return(NULL) } + if (is.null(self_ed25519)) { + self_ed25519 <- mx.crypto::mxc_account_identity_keys(account)$ed25519 + } sender <- content$sender_key if (identical(as.integer(msg$type), 0L)) { res <- mx.crypto::mxc_olm_create_inbound(account, @@ -248,7 +310,11 @@ mx_crypto_handle_to_device <- function(account, my_curve25519, content) { stop("no established Olm session for a non-prekey to-device message", call. = FALSE) } - jsonlite::fromJSON(plaintext, simplifyVector = FALSE) + decoded <- jsonlite::fromJSON(plaintext, simplifyVector = FALSE) + if (!mx_crypto_check_olm_payload(decoded, self_id, self_ed25519, sender)) { + return(NULL) + } + decoded } #' Build an inbound Megolm session from a shared room key diff --git a/R/e2ee.R b/R/e2ee.R index 4eab59c..001a0a8 100644 --- a/R/e2ee.R +++ b/R/e2ee.R @@ -9,7 +9,12 @@ # olm peer Curve25519 -> outbound Olm session (we encrypt to them) # olm_in peer Curve25519 -> inbound Olm session (they encrypt to us) # megolm_out room id -> list(session, shared = peer curves) -# megolm_in "room|session_id" -> inbound Megolm session +# megolm_in "room|session_id" -> list(session, sender, sender_ed25519) +# +# megolm_in carries the sender identity the Olm payload attested to when +# the key was shared, since that is the only trustworthy answer to who +# sent the messages in that session. Stores written before this existed +# hold a bare pickle string there and are migrated on load. #' Create an empty E2EE session set #' @@ -54,8 +59,10 @@ mx_crypto_sessions_save <- function(sessions, store_dir) { list(session = mx.crypto::mxc_megolm_outbound_pickle(m$session, key), shared = as.list(m$shared)) }), - megolm_in = lapply(sessions$megolm_in, function(s) { - mx.crypto::mxc_megolm_inbound_pickle(s, key) + megolm_in = lapply(sessions$megolm_in, function(e) { + list(session = mx.crypto::mxc_megolm_inbound_pickle(e$session, key), + sender = e$sender %||% NA_character_, + sender_ed25519 = e$sender_ed25519 %||% NA_character_) }) ) path <- file.path(store_dir, "sessions.json") @@ -100,8 +107,24 @@ mx_crypto_sessions_load <- function(store_dir) { shared = unlist(m$shared, use.names = FALSE) %||% character()) } for (nm in names(blob$megolm_in %||% list())) { - out$megolm_in[[nm]] <- mx.crypto::mxc_megolm_inbound_unpickle( - blob$megolm_in[[nm]], key) + e <- blob$megolm_in[[nm]] + # Stores written before sender attestation held a bare pickle + # string here. Read them rather than discarding them: dropping the + # entry would cost a running client its ability to decrypt every + # message in that session's history. Such sessions carry no + # attested sender, so events they decrypt report + # sender_verified = FALSE. + if (is.character(e) || !is.list(e)) { + out$megolm_in[[nm]] <- list( + session = mx.crypto::mxc_megolm_inbound_unpickle(e, key), + sender = NA_character_, + sender_ed25519 = NA_character_) + next + } + out$megolm_in[[nm]] <- list( + session = mx.crypto::mxc_megolm_inbound_unpickle(e$session, key), + sender = e$sender %||% NA_character_, + sender_ed25519 = e$sender_ed25519 %||% NA_character_) } out } @@ -122,8 +145,12 @@ mx_crypto_sessions_load <- function(store_dir) { #' @param sender_curve25519 Character. This device's Curve25519 key. #' @param device_id Character. This device's id. #' @param recipients List of recipient devices, each a list with -#' \code{user_id}, \code{device_id}, \code{curve25519}, and (only needed -#' to open a new Olm session) \code{otk}, a claimed one-time key. +#' \code{user_id}, \code{device_id}, \code{curve25519}, \code{ed25519}, +#' and (only needed to open a new Olm session) \code{otk}, a claimed +#' one-time key. Use \code{mx_crypto_known_devices()}, which returns +#' exactly this shape for devices whose keys verified. +#' @param sender_user_id Character. This user's Matrix id. Required to +#' build a spec-conformant Olm payload that the recipient can attribute. #' @return List with \code{to_device} (per-device payloads), \code{event} #' (the \code{m.room.encrypted} content), and the updated \code{sessions}. #' @examples @@ -134,15 +161,22 @@ mx_crypto_sessions_load <- function(store_dir) { #' acct, mx_crypto_sessions_new(), "!r:ex", #' list(msgtype = "m.text", body = "hi"), #' mx.crypto::mxc_account_identity_keys(acct)$curve25519, "DEV", -#' recipients = list()) +#' recipients = list(), sender_user_id = "@me:ex") #' names(out) #' } #' } #' @export mx_crypto_encrypt_for_devices <- function(account, sessions, room_id, content, sender_curve25519, - device_id, recipients = list()) { + device_id, recipients = list(), + sender_user_id = NULL) { mx_require_crypto() + if (length(recipients) && is.null(sender_user_id)) { + stop("sender_user_id is required to share room keys; without it the ", + "Olm payload cannot name its sender and recipients will ", + "reject it", call. = FALSE) + } + sender_ed25519 <- mx.crypto::mxc_account_identity_keys(account)$ed25519 mo <- sessions$megolm_out[[room_id]] if (is.null(mo)) { mo <- list(session = mx.crypto::mxc_megolm_outbound_new(), @@ -155,6 +189,12 @@ mx_crypto_encrypt_for_devices <- function(account, sessions, room_id, if (peer %in% mo$shared) { next } + if (is.null(r$ed25519) || !nzchar(r$ed25519)) { + stop("recipient ", r$user_id %||% "", "/", + r$device_id %||% "", " has no ed25519 key; it must ", + "come from a verified device (mx_crypto_known_devices())", + call. = FALSE) + } olm <- sessions$olm[[peer]] if (is.null(olm)) { if (is.null(r$otk)) { @@ -167,7 +207,8 @@ mx_crypto_encrypt_for_devices <- function(account, sessions, room_id, sessions$olm[[peer]] <- olm } td <- mx_crypto_room_key_payload(olm, sender_curve25519, peer, - room_id, mo$session) + room_id, mo$session, sender_user_id, sender_ed25519, + r$user_id, r$ed25519) to_device[[length(to_device) + 1L]] <- list( user_id = r$user_id, device_id = r$device_id, content = td) mo$shared <- c(mo$shared, peer) @@ -209,6 +250,7 @@ mx_crypto_encrypt_for_devices <- function(account, sessions, room_id, mx_crypto_process_sync <- function(account, sessions, sync_resp, self_curve25519, self_id = NULL) { mx_require_crypto() + self_ed25519 <- mx.crypto::mxc_account_identity_keys(account)$ed25519 # 1. To-device: recover shared room keys. for (ev in sync_resp$to_device$events %||% list()) { @@ -234,11 +276,21 @@ mx_crypto_process_sync <- function(account, sessions, sync_resp, rawToChar(mx.crypto::mxc_olm_decrypt(s, msg$type, msg$body)) } decoded <- jsonlite::fromJSON(plaintext, simplifyVector = FALSE) + if (!mx_crypto_check_olm_payload(decoded, self_id, self_ed25519, + sender)) { + next + } if (identical(decoded$type, "m.room_key")) { c <- decoded$content key <- paste(c$room_id, c$session_id, sep = "|") - sessions$megolm_in[[key]] <- mx.crypto::mxc_megolm_inbound_new( - c$session_key) + # Keep the sender identity the Olm payload attested to. This is + # the only trustworthy answer to "who sent the messages in this + # Megolm session"; the cleartext event envelope is the server's + # word, not the sender's. + sessions$megolm_in[[key]] <- list( + session = mx.crypto::mxc_megolm_inbound_new(c$session_key), + sender = decoded$sender, + sender_ed25519 = decoded$keys$ed25519) } } @@ -252,20 +304,38 @@ mx_crypto_process_sync <- function(account, sessions, sync_resp, next } key <- paste(rid, ev$content$session_id, sep = "|") - inb <- sessions$megolm_in[[key]] - if (is.null(inb)) { + entry <- sessions$megolm_in[[key]] + if (is.null(entry)) { next } - dec <- tryCatch(mx_crypto_decrypt_event(inb, ev$content), + dec <- tryCatch(mx_crypto_decrypt_event(entry$session, ev$content), error = function(e) NULL) if (is.null(dec)) { next } + # The session was handed to us over Olm by a specific device. + # Whoever that was is the real sender of everything encrypted + # with it, regardless of what the envelope claims. + attested <- entry$sender + verified <- FALSE + if (!is.null(attested) && !is.na(attested)) { + if (!identical(attested, ev$sender)) { + warning("mx.client: dropping event ", + ev$event_id %||% "", " in ", rid, + ": envelope claims sender ", + ev$sender %||% "", + " but the Megolm session was shared by ", + attested, call. = FALSE) + next + } + verified <- TRUE + } ct <- dec$content events[[length(events) + 1L]] <- list( room_id = rid, event_id = ev$event_id, sender = ev$sender, + sender_verified = verified, is_self = isTRUE(ev$sender == self_id), body = ct$body, msgtype = ct$msgtype, diff --git a/R/transport.R b/R/transport.R index 1e07285..96f1774 100644 --- a/R/transport.R +++ b/R/transport.R @@ -55,9 +55,14 @@ mx_crypto_publish_keys <- function(client, account, store_dir, n_otks = 50L) { #' #' Queries \code{/keys/query} and flattens the result to a list of devices. #' +#' Devices are returned only if their Ed25519 self-signature verifies +#' against the identity the homeserver claims for them. A device that +#' fails is dropped with a warning naming it, and the remaining devices +#' are still returned: one bad device must not make a room unusable. +#' #' @param client Matrix client config. #' @param user_ids Character vector of Matrix user ids. -#' @return List of devices, each \code{list(user_id, device_id, +#' @return List of verified devices, each \code{list(user_id, device_id, #' curve25519, ed25519)}. #' @examples #' \dontrun{ @@ -69,14 +74,35 @@ mx_crypto_known_devices <- function(client, user_ids) { s <- mx_client_session(client) query <- stats::setNames(rep(list(list()), length(user_ids)), user_ids) resp <- mx.api::mx_keys_query(s, device_keys = query) + mx_crypto_verify_device_map(resp$device_keys) +} + +# Verify every device in a parsed /keys/query device_keys map. +# +# Split out from the HTTP call so the verification logic is testable +# against fixture responses without a homeserver. The homeserver is not +# trusted here: without this check it can substitute its own Curve25519 +# key for any device and read everything sent to that user. +# +# mxc_verify_device_keys() raises on failure rather than returning FALSE, +# and returns the keys it actually checked -- use those, not the raw map. +mx_crypto_verify_device_map <- function(device_keys_map) { out <- list() - dk <- resp$device_keys %||% list() - for (uid in names(dk)) { - for (dev in names(dk[[uid]])) { - keys <- dk[[uid]][[dev]]$keys + for (uid in names(device_keys_map %||% list())) { + devs <- device_keys_map[[uid]] + for (dev in names(devs %||% list())) { + keys <- tryCatch( + mx.crypto::mxc_verify_device_keys(devs[[dev]], uid, dev), + error = function(e) { + warning("mx.client: skipping unverified device ", uid, "/", + dev, ": ", conditionMessage(e), call. = FALSE) + NULL + }) + if (is.null(keys)) { + next + } out[[length(out) + 1L]] <- list(user_id = uid, device_id = dev, - curve25519 = keys[[paste0("curve25519:", dev)]], - ed25519 = keys[[paste0("ed25519:", dev)]]) + curve25519 = keys$curve25519, ed25519 = keys$ed25519) } } out @@ -87,10 +113,15 @@ mx_crypto_known_devices <- function(client, user_ids) { #' Calls \code{/keys/claim} and attaches the claimed key to each device as #' \code{$otk}, ready for \code{mx_crypto_encrypt_for_devices()}. #' +#' Each claimed key's signature is checked against the device's Ed25519 +#' key, which must itself have come from a verified \code{device_keys} +#' (that is, from \code{mx_crypto_known_devices()}). A key that fails is +#' dropped with a warning and its device comes back with no \code{otk}. +#' #' @param client Matrix client config. #' @param devices List of devices from \code{mx_crypto_known_devices()}. #' @return The devices with an \code{otk} field added where one was -#' claimed. +#' claimed and verified. #' @examples #' \dontrun{ #' devs <- mx_crypto_claim_otks(client, mx_crypto_known_devices(client, uid)) @@ -108,12 +139,45 @@ mx_crypto_claim_otks <- function(client, devices) { stats::setNames(list("signed_curve25519"), d$device_id)) } resp <- mx.api::mx_keys_claim(s, one_time_keys = req) - claimed <- resp$one_time_keys %||% list() + mx_crypto_verify_claimed_otks(devices, resp$one_time_keys %||% list()) +} + +# Attach verified one-time keys from a parsed /keys/claim response. +# +# Split from the HTTP call to keep it testable. The previous code took +# slot[[1]]$key and dropped the signatures beside it, so a homeserver +# could hand back a one-time key it had minted itself and we would open +# an Olm session to a device it controls. +# +# mxc_verify_one_time_key() deliberately will not look the signing key up +# for us, since doing so would re-trust the same response it is checking. +# That is why d$ed25519 has to come from an already-verified device. +mx_crypto_verify_claimed_otks <- function(devices, claimed) { lapply(devices, function(d) { slot <- claimed[[d$user_id]][[d$device_id]] - if (length(slot)) { - # slot is "signed_curve25519:" -> {key, signatures} - d$otk <- slot[[1]]$key + if (!length(slot) || is.null(names(slot))) { + return(d) + } + if (is.null(d$ed25519) || !nzchar(d$ed25519)) { + warning("mx.client: no verified ed25519 key for ", d$user_id, + "/", d$device_id, "; cannot check its one-time key", + call. = FALSE) + return(d) + } + # slot is "signed_curve25519:" -> {key, signatures}; the outer + # map key is part of what gets verified, so keep it. + algo_kid <- names(slot)[[1]] + otk <- tryCatch( + mx.crypto::mxc_verify_one_time_key(algo_kid, slot[[1]], d$ed25519, + d$user_id, d$device_id), + error = function(e) { + warning("mx.client: rejecting one-time key for ", d$user_id, + "/", d$device_id, ": ", conditionMessage(e), + call. = FALSE) + NULL + }) + if (!is.null(otk)) { + d$otk <- otk } d }) @@ -159,11 +223,23 @@ mx_send_encrypted <- function(client, account, sessions, room_id, content, }, devs) need <- Filter(function(d) is.null(sessions$olm[[d$curve25519]]), devs) have <- Filter(function(d) !is.null(sessions$olm[[d$curve25519]]), devs) - recipients <- c(mx_crypto_claim_otks(client, need), have) + claimed <- mx_crypto_claim_otks(client, need) + # A device whose one-time key failed verification has no usable otk. + # Drop it rather than letting encrypt_for_devices abort the send: + # the remaining devices should still get the message. + usable <- Filter(function(d) !is.null(d$otk), claimed) + if (length(usable) < length(claimed)) { + warning("mx.client: ", length(claimed) - length(usable), " of ", + length(claimed), " new devices in ", room_id, + " had no usable one-time key and were skipped", + call. = FALSE) + } + recipients <- c(usable, have) } out <- mx_crypto_encrypt_for_devices(account, sessions, room_id, - content, sender_curve, client$device_id, recipients = recipients) + content, sender_curve, client$device_id, recipients = recipients, + sender_user_id = client$user_id) for (p in out$to_device) { messages <- stats::setNames( list(stats::setNames(list(p$content), p$device_id)), p$user_id) diff --git a/inst/tinytest/test_crypto.R b/inst/tinytest/test_crypto.R index 2df9319..49f29c2 100644 --- a/inst/tinytest/test_crypto.R +++ b/inst/tinytest/test_crypto.R @@ -42,14 +42,21 @@ megolm_out <- mx.crypto::mxc_megolm_outbound_new() td <- mx_crypto_room_key_payload( olm, sender_curve25519 = alice_idk$curve25519, recipient_curve25519 = bob_idk$curve25519, - room_id = ROOM, megolm_out = megolm_out) + room_id = ROOM, megolm_out = megolm_out, + sender_user_id = "@alice:example.org", + sender_ed25519 = alice_idk$ed25519, + recipient_user_id = "@bob:example.org", + recipient_ed25519 = bob_idk$ed25519) expect_equal(td$algorithm, "m.olm.v1.curve25519-aes-sha2") expect_true(bob_idk$curve25519 %in% names(td$ciphertext)) # ---- Bob receives the to-device payload, recovers the room key ---- -room_key_ev <- mx_crypto_handle_to_device(bob, bob_idk$curve25519, td) +room_key_ev <- mx_crypto_handle_to_device(bob, bob_idk$curve25519, td, + self_id = "@bob:example.org") expect_equal(room_key_ev$type, "m.room_key") expect_equal(room_key_ev$content$room_id, ROOM) +expect_equal(room_key_ev$sender, "@alice:example.org") # attested sender +expect_equal(room_key_ev$keys$ed25519, alice_idk$ed25519) inbound <- mx_crypto_inbound_session(room_key_ev$content$session_key) # ---- Alice encrypts a room message; Bob decrypts it ---- diff --git a/inst/tinytest/test_e2ee.R b/inst/tinytest/test_e2ee.R index 6b3d63d..ad89e70 100644 --- a/inst/tinytest/test_e2ee.R +++ b/inst/tinytest/test_e2ee.R @@ -16,6 +16,8 @@ alice <- mx.crypto::mxc_account_new() bob <- mx.crypto::mxc_account_new() alice_curve <- mx.crypto::mxc_account_identity_keys(alice)$curve25519 bob_curve <- mx.crypto::mxc_account_identity_keys(bob)$curve25519 +alice_ed <- mx.crypto::mxc_account_identity_keys(alice)$ed25519 +bob_ed <- mx.crypto::mxc_account_identity_keys(bob)$ed25519 mx.crypto::mxc_account_generate_one_time_keys(bob, 2L) bob_otk <- mx.crypto::mxc_account_one_time_keys(bob)[[1]] @@ -43,10 +45,11 @@ bob_sync <- function(out, event_id, with_to_device = TRUE) { # ---- message 1: fresh session (prekey Olm + Megolm key share) ---- recips <- list(list(user_id = "@bob:example.org", device_id = "BOBDEV", - curve25519 = bob_curve, otk = bob_otk)) + curve25519 = bob_curve, ed25519 = bob_ed, otk = bob_otk)) out1 <- mx_crypto_encrypt_for_devices( alice, a_sess, ROOM, list(msgtype = "m.text", body = "first secret"), - alice_curve, "ALICEDEV", recipients = recips) + alice_curve, "ALICEDEV", recipients = recips, + sender_user_id = "@alice:example.org") a_sess <- out1$sessions expect_equal(length(out1$to_device), 1L) # key shared with Bob @@ -56,6 +59,7 @@ b_sess <- res1$sessions expect_equal(length(res1$events), 1L) expect_equal(res1$events[[1]]$body, "first secret") # decrypted expect_false(res1$events[[1]]$is_self) +expect_true(res1$events[[1]]$sender_verified) # attested over Olm # ---- persist both sides, reload from disk ---- mx_crypto_sessions_save(a_sess, a_store) @@ -69,7 +73,8 @@ expect_equal(length(b_sess$megolm_in), 1L) # inbound survived # ---- message 2: established session, no re-share, decrypt from store ---- out2 <- mx_crypto_encrypt_for_devices( alice, a_sess, ROOM, list(msgtype = "m.text", body = "second secret"), - alice_curve, "ALICEDEV", recipients = recips) + alice_curve, "ALICEDEV", recipients = recips, + sender_user_id = "@alice:example.org") a_sess <- out2$sessions expect_equal(length(out2$to_device), 0L) # already shared @@ -78,3 +83,52 @@ res2 <- mx_crypto_process_sync(bob, b_sess, bob_curve, self_id = "@bob:example.org") expect_equal(length(res2$events), 1L) expect_equal(res2$events[[1]]$body, "second secret") # decrypted from reloaded state +expect_true(res2$events[[1]]$sender_verified) # survives the store round-trip + +# ---- a forged envelope sender is dropped, not reported ---- +# The server rewrites `sender` on the timeline event. The Megolm session +# was shared by Alice over Olm, so the lie is detectable. +forged <- bob_sync(out2, "$3", with_to_device = FALSE) +forged$rooms$join[[ROOM]]$timeline$events[[1]]$sender <- "@mallory:example.org" +res3 <- suppressWarnings( + mx_crypto_process_sync(bob, b_sess, forged, bob_curve, + self_id = "@bob:example.org")) +expect_equal(length(res3$events), 0L) + +# ---- an Olm payload addressed to someone else is dropped ---- +carol <- mx.crypto::mxc_account_new() +carol_curve <- mx.crypto::mxc_account_identity_keys(carol)$curve25519 +carol_ed <- mx.crypto::mxc_account_identity_keys(carol)$ed25519 +mx.crypto::mxc_account_generate_one_time_keys(bob, 2L) +bob_otk2 <- mx.crypto::mxc_account_one_time_keys(bob)[[2]] +# Alice shares a key naming Carol as recipient, but sends it to Bob. +misaddressed <- mx_crypto_encrypt_for_devices( + alice, mx_crypto_sessions_new(), "!other:example.org", + list(msgtype = "m.text", body = "not for bob"), alice_curve, "ALICEDEV", + recipients = list(list(user_id = "@carol:example.org", + device_id = "CAROLDEV", curve25519 = bob_curve, + ed25519 = carol_ed, otk = bob_otk2)), + sender_user_id = "@alice:example.org") +res4 <- suppressWarnings( + mx_crypto_process_sync(bob, mx_crypto_sessions_new(), + bob_sync(misaddressed, "$4"), bob_curve, + self_id = "@bob:example.org")) +expect_equal(length(res4$sessions$megolm_in), 0L) # key not installed + +# ---- a legacy store (bare pickle) still loads ---- +legacy_store <- file.path(tempfile(), "legacy") +dir.create(legacy_store, recursive = TRUE) +mx_crypto_sessions_save(b_sess, legacy_store) +raw <- jsonlite::fromJSON(paste(readLines( + file.path(legacy_store, "sessions.json"), warn = FALSE), collapse = "\n"), + simplifyVector = FALSE) +raw$megolm_in <- lapply(raw$megolm_in, function(e) e$session) # old shape +writeLines(jsonlite::toJSON(raw, auto_unbox = TRUE), + file.path(legacy_store, "sessions.json")) +reloaded <- mx_crypto_sessions_load(legacy_store) +expect_equal(length(reloaded$megolm_in), length(b_sess$megolm_in)) +res5 <- mx_crypto_process_sync(bob, reloaded, + bob_sync(out2, "$5", with_to_device = FALSE), + bob_curve, self_id = "@bob:example.org") +expect_equal(length(res5$events), 1L) # history still decrypts +expect_false(res5$events[[1]]$sender_verified) # but carries no attestation diff --git a/inst/tinytest/test_transport.R b/inst/tinytest/test_transport.R new file mode 100644 index 0000000..c869b86 --- /dev/null +++ b/inst/tinytest/test_transport.R @@ -0,0 +1,111 @@ +# Homeserver key verification. These exercise the pure halves of the +# /keys/query and /keys/claim handling against fixture responses, so the +# security-relevant logic is covered without a live homeserver -- which is +# exactly the gap that let the unverified paths ship in the first place. + +library(tinytest) + +if (!requireNamespace("mx.crypto", quietly = TRUE)) { + exit_file("mx.crypto not available (needs a Rust toolchain)") +} +library(mx.client) + +UID <- "@alice:example.org" +DEV <- "ALICEDEV" + +alice <- mx.crypto::mxc_account_new() +mallory <- mx.crypto::mxc_account_new() +alice_idk <- mx.crypto::mxc_account_identity_keys(alice) +mallory_idk <- mx.crypto::mxc_account_identity_keys(mallory) + +dk <- mx_crypto_device_keys(alice, UID, DEV) +good_map <- stats::setNames(list(stats::setNames(list(dk), DEV)), UID) + +# ---- a well-formed device verifies and yields its real keys ---- +devs <- mx.client:::mx_crypto_verify_device_map(good_map) +expect_equal(length(devs), 1L) +expect_equal(devs[[1]]$user_id, UID) +expect_equal(devs[[1]]$device_id, DEV) +expect_equal(devs[[1]]$curve25519, alice_idk$curve25519) +expect_equal(devs[[1]]$ed25519, alice_idk$ed25519) + +# ---- a substituted curve25519 key is rejected ---- +# This is the attack the whole exercise exists to stop: the homeserver +# swaps in a key it holds the private half of and reads the room. +swapped <- good_map +swapped[[UID]][[DEV]]$keys[[paste0("curve25519:", DEV)]] <- + mallory_idk$curve25519 +expect_warning(res <- mx.client:::mx_crypto_verify_device_map(swapped)) +expect_equal(length(res), 0L) + +# ---- a stripped signatures block is rejected ---- +unsigned <- good_map +unsigned[[UID]][[DEV]]$signatures <- NULL +expect_warning(res <- mx.client:::mx_crypto_verify_device_map(unsigned)) +expect_equal(length(res), 0L) + +# ---- a device reattributed to another user is rejected ---- +reattributed <- stats::setNames(list(stats::setNames(list(dk), DEV)), + "@mallory:example.org") +expect_warning(res <- mx.client:::mx_crypto_verify_device_map(reattributed)) +expect_equal(length(res), 0L) + +# ---- a device reattributed to another device id is rejected ---- +renamed <- stats::setNames(list(stats::setNames(list(dk), "OTHERDEV")), UID) +expect_warning(res <- mx.client:::mx_crypto_verify_device_map(renamed)) +expect_equal(length(res), 0L) + +# ---- one bad device does not take the good ones with it ---- +bob <- mx.crypto::mxc_account_new() +bob_dk <- mx_crypto_device_keys(bob, "@bob:example.org", "BOBDEV") +mixed <- c(good_map, + stats::setNames(list(stats::setNames(list(bob_dk), "BOBDEV")), + "@bob:example.org")) +mixed[[UID]][[DEV]]$signatures <- NULL # alice's device is broken +expect_warning(res <- mx.client:::mx_crypto_verify_device_map(mixed)) +expect_equal(length(res), 1L) # bob still usable +expect_equal(res[[1]]$user_id, "@bob:example.org") + +# ---- empty / absent map is not an error ---- +expect_equal(length(mx.client:::mx_crypto_verify_device_map(NULL)), 0L) +expect_equal(length(mx.client:::mx_crypto_verify_device_map(list())), 0L) + +# ---- one-time keys: a correctly signed key verifies ---- +mx.crypto::mxc_account_generate_one_time_keys(alice, 1L) +otks <- mx.crypto::mxc_account_one_time_keys(alice) +kid <- names(otks)[[1]] +signed <- mx.client:::mx_crypto_sign_otk(alice, otks[[kid]], UID, DEV) +claimed <- stats::setNames( + list(stats::setNames( + list(stats::setNames(list(signed), paste0("signed_curve25519:", kid))), + DEV)), + UID) + +device <- list(user_id = UID, device_id = DEV, + curve25519 = alice_idk$curve25519, ed25519 = alice_idk$ed25519) +res <- mx.client:::mx_crypto_verify_claimed_otks(list(device), claimed) +expect_equal(res[[1]]$otk, otks[[kid]]) + +# ---- a one-time key signed by the wrong device is rejected ---- +forged <- mx.client:::mx_crypto_sign_otk(mallory, otks[[kid]], UID, DEV) +forged_claimed <- stats::setNames( + list(stats::setNames( + list(stats::setNames(list(forged), paste0("signed_curve25519:", kid))), + DEV)), + UID) +expect_warning( + res <- mx.client:::mx_crypto_verify_claimed_otks(list(device), + forged_claimed)) +expect_null(res[[1]]$otk) + +# ---- a device with no verified ed25519 cannot have its OTK checked ---- +no_ed <- list(user_id = UID, device_id = DEV, + curve25519 = alice_idk$curve25519, ed25519 = NULL) +expect_warning( + res <- mx.client:::mx_crypto_verify_claimed_otks(list(no_ed), claimed)) +expect_null(res[[1]]$otk) + +# ---- a device the server returned nothing for is passed through ---- +res <- mx.client:::mx_crypto_verify_claimed_otks(list(device), list()) +expect_equal(length(res), 1L) +expect_null(res[[1]]$otk) diff --git a/man/mx_crypto_claim_otks.Rd b/man/mx_crypto_claim_otks.Rd index 6097a99..157ed09 100644 --- a/man/mx_crypto_claim_otks.Rd +++ b/man/mx_crypto_claim_otks.Rd @@ -12,11 +12,18 @@ mx_crypto_claim_otks(client, devices) } \value{ The devices with an \code{otk} field added where one was - claimed. + claimed and verified. } \description{ Calls \code{/keys/claim} and attaches the claimed key to each device as \code{$otk}, ready for \code{mx_crypto_encrypt_for_devices()}. +} +\details{ +Each claimed key's signature is checked against the device's Ed25519 +key, which must itself have come from a verified \code{device_keys} +(that is, from \code{mx_crypto_known_devices()}). A key that fails is +dropped with a warning and its device comes back with no \code{otk}. + } \examples{ \dontrun{ diff --git a/man/mx_crypto_encrypt_for_devices.Rd b/man/mx_crypto_encrypt_for_devices.Rd index 0b3c36d..10b6b03 100644 --- a/man/mx_crypto_encrypt_for_devices.Rd +++ b/man/mx_crypto_encrypt_for_devices.Rd @@ -4,7 +4,8 @@ \title{Encrypt an event for an encrypted room's devices} \usage{ mx_crypto_encrypt_for_devices(account, sessions, room_id, content, - sender_curve25519, device_id, recipients = list()) + sender_curve25519, device_id, + recipients = list(), sender_user_id = NULL) } \arguments{ \item{account}{An mx.crypto account handle.} @@ -20,8 +21,13 @@ mx_crypto_encrypt_for_devices(account, sessions, room_id, content, \item{device_id}{Character. This device's id.} \item{recipients}{List of recipient devices, each a list with -\code{user_id}, \code{device_id}, \code{curve25519}, and (only needed -to open a new Olm session) \code{otk}, a claimed one-time key.} +\code{user_id}, \code{device_id}, \code{curve25519}, \code{ed25519}, +and (only needed to open a new Olm session) \code{otk}, a claimed +one-time key. Use \code{mx_crypto_known_devices()}, which returns +exactly this shape for devices whose keys verified.} + +\item{sender_user_id}{Character. This user's Matrix id. Required to +build a spec-conformant Olm payload that the recipient can attribute.} } \value{ List with \code{to_device} (per-device payloads), \code{event} @@ -43,7 +49,7 @@ if (requireNamespace("mx.crypto", quietly = TRUE)) { acct, mx_crypto_sessions_new(), "!r:ex", list(msgtype = "m.text", body = "hi"), mx.crypto::mxc_account_identity_keys(acct)$curve25519, "DEV", - recipients = list()) + recipients = list(), sender_user_id = "@me:ex") names(out) } } diff --git a/man/mx_crypto_handle_to_device.Rd b/man/mx_crypto_handle_to_device.Rd index 980847b..732f111 100644 --- a/man/mx_crypto_handle_to_device.Rd +++ b/man/mx_crypto_handle_to_device.Rd @@ -3,7 +3,8 @@ \alias{mx_crypto_handle_to_device} \title{Decrypt an inbound Olm to-device payload} \usage{ -mx_crypto_handle_to_device(account, my_curve25519, content) +mx_crypto_handle_to_device(account, my_curve25519, content, self_id = NULL, + self_ed25519 = NULL) } \arguments{ \item{account}{An mx.crypto account handle.} @@ -11,19 +12,34 @@ mx_crypto_handle_to_device(account, my_curve25519, content) \item{my_curve25519}{Character. This device's Curve25519 key.} \item{content}{The to-device \code{m.room.encrypted} content.} + +\item{self_id}{Character or NULL. This user's Matrix id. When NULL the +recipient user-id check is skipped; pass it whenever it is known.} + +\item{self_ed25519}{Character or NULL. This device's Ed25519 key. +Defaults to the account's own key.} } \value{ -The decrypted event (a parsed list), or NULL if not for us. +The decrypted event (a parsed list), or NULL if it was not for + us or failed the recipient checks. } \description{ Accepts an \code{m.room.encrypted} to-device content addressed to this device and returns the decrypted event. When it is an \code{m.room_key}, the caller builds an inbound Megolm session from \code{content$session_key} with \code{mx_crypto_inbound_session()}. +} +\details{ +The decrypted payload is checked against this device's identity before +it is returned: a message that does not name us as recipient is +dropped, because decrypting successfully only proves it was encrypted +to our key, not that it was meant for us here. + } \examples{ \dontrun{ -ev <- mx_crypto_handle_to_device(acct, my_curve, td_content) +ev <- mx_crypto_handle_to_device(acct, my_curve, td_content, + self_id = "@me:example.org") if (identical(ev$type, "m.room_key")) { inb <- mx_crypto_inbound_session(ev$content$session_key) } diff --git a/man/mx_crypto_known_devices.Rd b/man/mx_crypto_known_devices.Rd index 11125b2..1e5f302 100644 --- a/man/mx_crypto_known_devices.Rd +++ b/man/mx_crypto_known_devices.Rd @@ -11,11 +11,18 @@ mx_crypto_known_devices(client, user_ids) \item{user_ids}{Character vector of Matrix user ids.} } \value{ -List of devices, each \code{list(user_id, device_id, +List of verified devices, each \code{list(user_id, device_id, curve25519, ed25519)}. } \description{ Queries \code{/keys/query} and flattens the result to a list of devices. +} +\details{ +Devices are returned only if their Ed25519 self-signature verifies +against the identity the homeserver claims for them. A device that +fails is dropped with a warning naming it, and the remaining devices +are still returned: one bad device must not make a room unusable. + } \examples{ \dontrun{ diff --git a/man/mx_crypto_room_key_payload.Rd b/man/mx_crypto_room_key_payload.Rd index a951628..f61d5fb 100644 --- a/man/mx_crypto_room_key_payload.Rd +++ b/man/mx_crypto_room_key_payload.Rd @@ -4,7 +4,9 @@ \title{Encrypt a Megolm room key to one device as a to-device payload} \usage{ mx_crypto_room_key_payload(olm_session, sender_curve25519, - recipient_curve25519, room_id, megolm_out) + recipient_curve25519, room_id, megolm_out, + sender_user_id, sender_ed25519, recipient_user_id, + recipient_ed25519) } \arguments{ \item{olm_session}{An outbound Olm session @@ -17,6 +19,14 @@ mx_crypto_room_key_payload(olm_session, sender_curve25519, \item{room_id}{Character. Room the key is for.} \item{megolm_out}{An outbound Megolm session.} + +\item{sender_user_id}{Character. This user's Matrix id.} + +\item{sender_ed25519}{Character. This device's Ed25519 key.} + +\item{recipient_user_id}{Character. Target user's Matrix id.} + +\item{recipient_ed25519}{Character. Target device's Ed25519 key.} } \value{ A named list: the to-device \code{m.room.encrypted} content. @@ -30,6 +40,7 @@ Olm-encrypts it to the recipient device, and returns the \examples{ \dontrun{ content <- mx_crypto_room_key_payload(olm, my_curve, their_curve, - "!room:ex", megolm_out) + "!room:ex", megolm_out, + "@me:ex", my_ed, "@them:ex", their_ed) } } From 6afc0aaad563459ede399e5eba2fb227d3a9f545 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Sun, 2 Aug 2026 09:18:52 -0500 Subject: [PATCH 2/6] rformat R/messages.R rformat_dir("R") reflows this file, which this branch never touched. The repo was not rformat-clean; isolating the churn here. --- R/messages.R | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/R/messages.R b/R/messages.R index 9ebf3f4..5da1f9a 100644 --- a/R/messages.R +++ b/R/messages.R @@ -218,19 +218,17 @@ mx_accept_invites <- function(client, invites) { #' target_event_id = "$msg") #' @export mx_extract_reaction_verdict <- function(sync_resp, room_id, self_id, - target_event_id, - approve_keys = NULL, + target_event_id, approve_keys = NULL, deny_keys = NULL) { # Emoji defaults are built here, not in the signature, so they don't # land as raw astral-plane glyphs in the .Rd \usage block -- LaTeX # can't typeset them and the PDF manual fails R CMD check --as-cran. if (is.null(approve_keys)) { - approve_keys <- c(intToUtf8(0x1F44D), intToUtf8(0x2705), - "y", "yes", "ok") + approve_keys <- c(intToUtf8(0x1F44D), intToUtf8(0x2705), "y", "yes", + "ok") } if (is.null(deny_keys)) { - deny_keys <- c(intToUtf8(0x1F44E), intToUtf8(0x274C), - "n", "no", "nope") + deny_keys <- c(intToUtf8(0x1F44E), intToUtf8(0x274C), "n", "no", "nope") } room <- sync_resp$rooms$join[[room_id]] if (is.null(room)) { From b1f6a937b7a3491d72c1d7faf10d6c0d304054e3 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Sun, 2 Aug 2026 09:18:52 -0500 Subject: [PATCH 3/6] Bump version to 0.1.1.3 --- DESCRIPTION | 4 ++-- NEWS.md | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 4f09ed4..505e8fd 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: mx.client Type: Package Title: Stateful Matrix Client Helpers -Version: 0.1.1.1 +Version: 0.1.1.3 Date: 2026-06-13 Authors@R: c( person("Troy", "Hernandez", role = c("aut", "cre"), @@ -27,7 +27,7 @@ Imports: tools, utils Suggests: - mx.crypto, + mx.crypto (>= 0.2.0), simplermarkdown, tinytest VignetteBuilder: simplermarkdown diff --git a/NEWS.md b/NEWS.md index 8852e82..7ba5931 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,46 @@ +# mx.client 0.1.1.3 + +* **HIGH** (security): homeserver-supplied keys are now verified before + use. `mx_crypto_known_devices()` checks each device's Ed25519 + self-signature with `mx.crypto::mxc_verify_device_keys()`, and + `mx_crypto_claim_otks()` checks each claimed one-time key with + `mx.crypto::mxc_verify_one_time_key()` against that verified Ed25519. + Both previously read the keys straight out of the response, so a + malicious or compromised homeserver could substitute its own + Curve25519 key for any device and read everything sent to that user. + A device that fails verification is dropped with a warning and the + remaining devices still receive the message. +* **HIGH** (security): Olm to-device payloads now carry the + `sender`, `recipient`, `recipient_keys`, and `keys` fields the spec + requires. `mx_crypto_room_key_payload()` emitted only `type` and + `content`, leaving the receiver nothing to authenticate against; + other clients reject such payloads. +* **HIGH** (security): `mx_crypto_process_sync()` and + `mx_crypto_handle_to_device()` now confirm a decrypted Olm payload + names this device as recipient before acting on it. Decrypting only + proves the message was encrypted to our key, not that it was meant + for us here, so a server could previously replay a captured payload. +* `mx_crypto_process_sync()` records the sender attested by the Olm + payload that shared each Megolm session, and reports it as + `sender_verified` on decrypted events. When the cleartext envelope + disagrees with the attested sender the event is dropped: that is a + forged `sender`, which the server was previously free to set. Events + decrypted with a session from a store written before this change + carry `sender_verified = FALSE`. +* `mx_crypto_room_key_payload()` gains `sender_user_id`, + `sender_ed25519`, `recipient_user_id`, and `recipient_ed25519`; + `mx_crypto_encrypt_for_devices()` gains `sender_user_id` and now + requires each recipient to carry a verified `ed25519`; + `mx_crypto_handle_to_device()` gains `self_id` and `self_ed25519`. +* Session stores written before this release still load: a bare + `megolm_in` pickle is read as a session with no attested sender + rather than discarded, so a running client keeps its history. +* `Suggests: mx.crypto (>= 0.2.0)`, the first version providing the + verification helpers. +* New `inst/tinytest/test_transport.R` covers the verification paths + against tampered fixture responses. `R/transport.R` previously had no + test file at all. + # mx.client 0.1.1.1 * `mx_extract_text_events()` keeps the event's `origin_server_ts` as a From 8a57d735b948f6d9d142eade86c401f9e8e66497 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Sun, 2 Aug 2026 09:36:00 -0500 Subject: [PATCH 4/6] CI: install mx.crypto so the E2EE tests actually run 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. --- .github/workflows/ci.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 8bb75b3..87febd0 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -24,8 +24,24 @@ jobs: with: backend: RAPT + - name: Install Rust toolchain + # mx.crypto is a Rust package with no r2u binary, so it has to be + # built from source here. + run: | + curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --default-toolchain stable --profile minimal + echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" + - name: Dependencies run: ./run.sh install_deps + - name: Install suggested packages + # install_deps covers Imports only. Without mx.crypto every E2EE + # test calls exit_file() and CI goes green having exercised none of + # the encryption path -- which is how unverified homeserver keys + # shipped in the first place. simplermarkdown is needed for R CMD + # check to treat vignettes/e2ee.md as a vignette rather than a + # stray file. + run: Rscript -e 'install.packages(c("mx.crypto", "simplermarkdown"))' + - name: Test run: ./run.sh run_tests From 4cb34118f8cd12a520c8daa11163cd3e26c238a0 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Sun, 2 Aug 2026 09:39:22 -0500 Subject: [PATCH 5/6] CI: drop the redundant Rust toolchain step 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. --- .github/workflows/ci.yaml | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 87febd0..318f621 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -24,13 +24,6 @@ jobs: with: backend: RAPT - - name: Install Rust toolchain - # mx.crypto is a Rust package with no r2u binary, so it has to be - # built from source here. - run: | - curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --default-toolchain stable --profile minimal - echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" - - name: Dependencies run: ./run.sh install_deps @@ -41,6 +34,10 @@ jobs: # shipped in the first place. simplermarkdown is needed for R CMD # check to treat vignettes/e2ee.md as a vignette rather than a # stray file. + # + # Both arrive as binaries (r2u on Linux, CRAN on macOS), so no Rust + # toolchain is needed despite mx.crypto being a Rust package. The + # runners ship rustc anyway if a source build is ever forced. run: Rscript -e 'install.packages(c("mx.crypto", "simplermarkdown"))' - name: Test From 111ae68103cd1c23d7224a0827b4d83cd7eb7754 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Mon, 3 Aug 2026 13:39:00 -0500 Subject: [PATCH 6/6] Bind attested senders to verified devices 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. --- NEWS.md | 25 +++++++---- R/crypto.R | 73 ++++++++++++++++++++++++++----- R/e2ee.R | 59 +++++++++++++++++-------- inst/tinytest/test_e2ee.R | 62 +++++++++++++++++++++++++- man/mx_crypto_handle_to_device.Rd | 18 +++++--- man/mx_crypto_process_sync.Rd | 10 ++++- 6 files changed, 200 insertions(+), 47 deletions(-) diff --git a/NEWS.md b/NEWS.md index 7ba5931..84a64ca 100644 --- a/NEWS.md +++ b/NEWS.md @@ -20,18 +20,27 @@ names this device as recipient before acting on it. Decrypting only proves the message was encrypted to our key, not that it was meant for us here, so a server could previously replay a captured payload. -* `mx_crypto_process_sync()` records the sender attested by the Olm - payload that shared each Megolm session, and reports it as - `sender_verified` on decrypted events. When the cleartext envelope - disagrees with the attested sender the event is dropped: that is a - forged `sender`, which the server was previously free to set. Events - decrypted with a session from a store written before this change - carry `sender_verified = FALSE`. +* `mx_crypto_process_sync()` records the sender the Olm payload claimed + when it shared each Megolm session, and drops any decrypted event + whose cleartext envelope disagrees with that claim: the server was + previously free to set `sender` to anything. +* `mx_crypto_process_sync()` and `mx_crypto_handle_to_device()` gain a + `devices` argument taking the verified list from + `mx_crypto_known_devices()`. A decrypted event reports + `sender_verified = TRUE` only when the payload's claimed + `(sender, ed25519, curve25519)` matches one of those devices as a + triple. Agreement between the payload and the envelope is not enough + on its own: a hostile homeserver writes both, so it can make them + corroborate each other. Binding to a self-signed `device_keys` object + is the part it cannot forge. Callers that pass no `devices` still + decrypt but always get `sender_verified = FALSE`, as do events + decrypted with a session from a store written before this change. * `mx_crypto_room_key_payload()` gains `sender_user_id`, `sender_ed25519`, `recipient_user_id`, and `recipient_ed25519`; `mx_crypto_encrypt_for_devices()` gains `sender_user_id` and now requires each recipient to carry a verified `ed25519`; - `mx_crypto_handle_to_device()` gains `self_id` and `self_ed25519`. + `mx_crypto_handle_to_device()` gains `self_id`, `self_ed25519`, and + `devices`, and its result carries `sender_bound`. * Session stores written before this release still load: a bare `megolm_in` pickle is read as a session with no attested sender rather than discarded, so a running client keeps its history. diff --git a/R/crypto.R b/R/crypto.R index 582a9a7..6085f10 100644 --- a/R/crypto.R +++ b/R/crypto.R @@ -237,28 +237,66 @@ mx_crypto_room_key_payload <- function(olm_session, sender_curve25519, # spec's defence is the recipient block inside the plaintext, which is # covered by the ratchet and so cannot be rewritten by the server. # -# Returns TRUE when the payload is acceptable, FALSE (with a warning) -# otherwise. self_id may be NULL, in which case the user-id half of the +# Returns list(ok, bound). ok = FALSE (with a warning) means discard the +# payload. bound = TRUE means the claimed sender identity was tied to a +# device whose keys we verified via /keys/query; only then may anything +# downstream be reported as sender-verified. +# +# self_id may be NULL, in which case the user-id half of the recipient # check is skipped; callers that know their own id should always pass it. mx_crypto_check_olm_payload <- function(decoded, self_id, self_ed25519, - sender_curve25519 = NULL) { + sender_curve25519 = NULL, + devices = NULL) { if (!is.null(self_id) && !identical(decoded$recipient, self_id)) { warning("mx.client: dropping Olm payload addressed to ", decoded$recipient %||% "", ", not ", self_id, call. = FALSE) - return(FALSE) + return(list(ok = FALSE, bound = FALSE)) } if (!identical(decoded$recipient_keys$ed25519, self_ed25519)) { warning("mx.client: dropping Olm payload whose recipient_keys do ", "not match this device's Ed25519 key", call. = FALSE) - return(FALSE) + return(list(ok = FALSE, bound = FALSE)) } if (is.null(decoded$sender) || is.null(decoded$keys$ed25519)) { warning("mx.client: dropping Olm payload with no sender identity", call. = FALSE) + return(list(ok = FALSE, bound = FALSE)) + } + list(ok = TRUE, + bound = mx_crypto_sender_bound(decoded, sender_curve25519, devices)) +} + +# Tie a payload's claimed sender to a verified device. +# +# The sender/keys block inside the plaintext is written by whoever holds +# the Olm session, and anyone can open one to us: our Curve25519 key and +# our one-time keys are both public. So the claim on its own is worth +# nothing. Comparing it against the cleartext envelope is no better, +# because a hostile homeserver writes both halves and can simply make +# them agree. +# +# The way out is the one thing the server cannot forge: a device_keys +# object carrying a valid self-signature, which is what +# mx_crypto_known_devices() returns. Requiring (sender, ed25519, +# curve25519) to match one of those as a triple binds the claim to a real +# device. Without a device list there is nothing to bind against, so the +# answer is FALSE and callers must not claim verification. +mx_crypto_sender_bound <- function(decoded, sender_curve25519, devices) { + if (is.null(devices) || !length(devices) || is.null(sender_curve25519)) { return(FALSE) } - TRUE + for (d in devices) { + if (identical(d$user_id, decoded$sender) && + identical(d$ed25519, decoded$keys$ed25519) && + identical(d$curve25519, sender_curve25519)) { + return(TRUE) + } + } + warning("mx.client: Olm payload claims sender ", decoded$sender, + " but no verified device matches its identity keys; treating ", + "it as unattributed", call. = FALSE) + FALSE } #' Decrypt an inbound Olm to-device payload @@ -280,19 +318,27 @@ mx_crypto_check_olm_payload <- function(decoded, self_id, self_ed25519, #' recipient user-id check is skipped; pass it whenever it is known. #' @param self_ed25519 Character or NULL. This device's Ed25519 key. #' Defaults to the account's own key. -#' @return The decrypted event (a parsed list), or NULL if it was not for -#' us or failed the recipient checks. +#' @param devices List of verified devices from +#' \code{mx_crypto_known_devices()}, or NULL. Used to tie the payload's +#' claimed sender to a device whose keys were verified. The result +#' carries \code{sender_bound}, which is FALSE when no list is supplied +#' or nothing matches; an unbound sender identity is a claim, not a +#' fact, because anyone can open an Olm session to this device. +#' @return The decrypted event (a parsed list) with a \code{sender_bound} +#' flag, or NULL if it was not for us or failed the recipient checks. #' @examples #' \dontrun{ #' ev <- mx_crypto_handle_to_device(acct, my_curve, td_content, -#' self_id = "@me:example.org") -#' if (identical(ev$type, "m.room_key")) { +#' self_id = "@me:example.org", +#' devices = mx_crypto_known_devices(cl, uid)) +#' if (identical(ev$type, "m.room_key") && isTRUE(ev$sender_bound)) { #' inb <- mx_crypto_inbound_session(ev$content$session_key) #' } #' } #' @export mx_crypto_handle_to_device <- function(account, my_curve25519, content, - self_id = NULL, self_ed25519 = NULL) { + self_id = NULL, self_ed25519 = NULL, + devices = NULL) { mx_require_crypto() msg <- content$ciphertext[[my_curve25519]] if (is.null(msg)) { @@ -311,9 +357,12 @@ mx_crypto_handle_to_device <- function(account, my_curve25519, content, call. = FALSE) } decoded <- jsonlite::fromJSON(plaintext, simplifyVector = FALSE) - if (!mx_crypto_check_olm_payload(decoded, self_id, self_ed25519, sender)) { + chk <- mx_crypto_check_olm_payload(decoded, self_id, self_ed25519, sender, + devices) + if (!chk$ok) { return(NULL) } + decoded$sender_bound <- chk$bound decoded } diff --git a/R/e2ee.R b/R/e2ee.R index 001a0a8..4ae47c0 100644 --- a/R/e2ee.R +++ b/R/e2ee.R @@ -9,11 +9,14 @@ # olm peer Curve25519 -> outbound Olm session (we encrypt to them) # olm_in peer Curve25519 -> inbound Olm session (they encrypt to us) # megolm_out room id -> list(session, shared = peer curves) -# megolm_in "room|session_id" -> list(session, sender, sender_ed25519) +# megolm_in "room|session_id" -> list(session, sender, sender_ed25519, +# sender_bound) # -# megolm_in carries the sender identity the Olm payload attested to when -# the key was shared, since that is the only trustworthy answer to who -# sent the messages in that session. Stores written before this existed +# megolm_in carries the sender identity the Olm payload claimed when the +# key was shared, plus whether that claim was tied to a device whose +# device_keys verified. The claim alone is not evidence -- anyone can open +# an Olm session to us -- so only sender_bound justifies reporting a +# decrypted event as sender-verified. Stores written before this existed # hold a bare pickle string there and are migrated on load. #' Create an empty E2EE session set @@ -62,7 +65,8 @@ mx_crypto_sessions_save <- function(sessions, store_dir) { megolm_in = lapply(sessions$megolm_in, function(e) { list(session = mx.crypto::mxc_megolm_inbound_pickle(e$session, key), sender = e$sender %||% NA_character_, - sender_ed25519 = e$sender_ed25519 %||% NA_character_) + sender_ed25519 = e$sender_ed25519 %||% NA_character_, + sender_bound = isTRUE(e$sender_bound)) }) ) path <- file.path(store_dir, "sessions.json") @@ -118,13 +122,15 @@ mx_crypto_sessions_load <- function(store_dir) { out$megolm_in[[nm]] <- list( session = mx.crypto::mxc_megolm_inbound_unpickle(e, key), sender = NA_character_, - sender_ed25519 = NA_character_) + sender_ed25519 = NA_character_, + sender_bound = FALSE) next } out$megolm_in[[nm]] <- list( session = mx.crypto::mxc_megolm_inbound_unpickle(e$session, key), sender = e$sender %||% NA_character_, - sender_ed25519 = e$sender_ed25519 %||% NA_character_) + sender_ed25519 = e$sender_ed25519 %||% NA_character_, + sender_bound = isTRUE(e$sender_bound)) } out } @@ -232,6 +238,13 @@ mx_crypto_encrypt_for_devices <- function(account, sessions, room_id, #' @param sessions A session set. #' @param sync_resp Parsed \code{/sync} response. #' @param self_curve25519 Character. This device's Curve25519 key. +#' @param devices List of verified devices from +#' \code{mx_crypto_known_devices()}, or NULL. Room keys arrive over Olm +#' carrying a claimed sender, and anyone who can reach this device can +#' send one, so the claim is only worth something once it is matched +#' against a device whose \code{device_keys} verified. Without this +#' list decrypted events always report \code{sender_verified = FALSE}: +#' they still decrypt, but nothing attests to who sent them. #' @param self_id Character or NULL. This user's Matrix id, for #' \code{is_self} tagging. #' @return List with \code{events} (decrypted, normalized) and the updated @@ -248,7 +261,8 @@ mx_crypto_encrypt_for_devices <- function(account, sessions, room_id, #' } #' @export mx_crypto_process_sync <- function(account, sessions, sync_resp, - self_curve25519, self_id = NULL) { + self_curve25519, self_id = NULL, + devices = NULL) { mx_require_crypto() self_ed25519 <- mx.crypto::mxc_account_identity_keys(account)$ed25519 @@ -276,21 +290,25 @@ mx_crypto_process_sync <- function(account, sessions, sync_resp, rawToChar(mx.crypto::mxc_olm_decrypt(s, msg$type, msg$body)) } decoded <- jsonlite::fromJSON(plaintext, simplifyVector = FALSE) - if (!mx_crypto_check_olm_payload(decoded, self_id, self_ed25519, - sender)) { + chk <- mx_crypto_check_olm_payload(decoded, self_id, self_ed25519, + sender, devices) + if (!chk$ok) { next } if (identical(decoded$type, "m.room_key")) { c <- decoded$content key <- paste(c$room_id, c$session_id, sep = "|") - # Keep the sender identity the Olm payload attested to. This is - # the only trustworthy answer to "who sent the messages in this - # Megolm session"; the cleartext event envelope is the server's - # word, not the sender's. + # Record who the payload claimed to be from, and separately + # whether that claim was tied to a verified device. Keeping the + # unbound claim is still useful -- it catches an ordinary user + # forging a sender, since the server stamps the real one on the + # envelope -- but only sender_bound justifies calling a + # decrypted event's sender verified. sessions$megolm_in[[key]] <- list( session = mx.crypto::mxc_megolm_inbound_new(c$session_key), sender = decoded$sender, - sender_ed25519 = decoded$keys$ed25519) + sender_ed25519 = decoded$keys$ed25519, + sender_bound = chk$bound) } } @@ -313,9 +331,12 @@ mx_crypto_process_sync <- function(account, sessions, sync_resp, if (is.null(dec)) { next } - # The session was handed to us over Olm by a specific device. - # Whoever that was is the real sender of everything encrypted - # with it, regardless of what the envelope claims. + # The session was handed to us over Olm by whoever claimed the + # identity recorded here. A disagreement with the envelope is a + # forgery either way, so drop it. But agreement alone proves + # nothing against a hostile homeserver, which writes both the + # envelope and (via an injected to-device message) the claim. + # Only a sender tied to a verified device counts as verified. attested <- entry$sender verified <- FALSE if (!is.null(attested) && !is.na(attested)) { @@ -328,7 +349,7 @@ mx_crypto_process_sync <- function(account, sessions, sync_resp, attested, call. = FALSE) next } - verified <- TRUE + verified <- isTRUE(entry$sender_bound) } ct <- dec$content events[[length(events) + 1L]] <- list( diff --git a/inst/tinytest/test_e2ee.R b/inst/tinytest/test_e2ee.R index ad89e70..6075841 100644 --- a/inst/tinytest/test_e2ee.R +++ b/inst/tinytest/test_e2ee.R @@ -53,13 +53,71 @@ out1 <- mx_crypto_encrypt_for_devices( a_sess <- out1$sessions expect_equal(length(out1$to_device), 1L) # key shared with Bob +# Bob's verified view of Alice's device, as /keys/query would yield it. +alice_devs <- list(list(user_id = "@alice:example.org", device_id = "ALICEDEV", + curve25519 = alice_curve, ed25519 = alice_ed)) + res1 <- mx_crypto_process_sync(bob, b_sess, bob_sync(out1, "$1"), - bob_curve, self_id = "@bob:example.org") + bob_curve, self_id = "@bob:example.org", + devices = alice_devs) b_sess <- res1$sessions expect_equal(length(res1$events), 1L) expect_equal(res1$events[[1]]$body, "first secret") # decrypted expect_false(res1$events[[1]]$is_self) -expect_true(res1$events[[1]]$sender_verified) # attested over Olm +expect_true(res1$events[[1]]$sender_verified) # bound to a verified device + +# Olm one-time keys are single-use, so each scenario below needs its own +# recipient account and its own key share; a replayed prekey message +# cannot establish a second session. +probe <- function(event_id) { + acct <- mx.crypto::mxc_account_new() + idk <- mx.crypto::mxc_account_identity_keys(acct) + mx.crypto::mxc_account_generate_one_time_keys(acct, 1L) + otk <- mx.crypto::mxc_account_one_time_keys(acct)[[1]] + out <- mx_crypto_encrypt_for_devices( + alice, mx_crypto_sessions_new(), ROOM, + list(msgtype = "m.text", body = "probe"), alice_curve, "ALICEDEV", + recipients = list(list(user_id = "@bob:example.org", + device_id = "BOBDEV", + curve25519 = idk$curve25519, + ed25519 = idk$ed25519, otk = otk)), + sender_user_id = "@alice:example.org") + list(account = acct, curve = idk$curve25519, + sync = bob_sync(out, event_id)) +} + +# Without a device list the traffic decrypts but claims nothing: the +# sender identity in an Olm payload is written by whoever holds the +# session, and anyone can open one to us. +p1 <- probe("$1b") +unbound <- suppressWarnings( + mx_crypto_process_sync(p1$account, mx_crypto_sessions_new(), p1$sync, + p1$curve, self_id = "@bob:example.org")) +expect_equal(length(unbound$events), 1L) # still decrypts +expect_false(unbound$events[[1]]$sender_verified) # but attests nothing + +# A device list whose curve25519 is not the one that sent the room key +# must not bind. This is the hostile-homeserver case: it injects a room +# key claiming Alice and stamps the envelope to agree, so the two halves +# corroborate each other and only the device binding catches it. +p2 <- probe("$1c") +wrong_curve <- list(list(user_id = "@alice:example.org", + device_id = "ALICEDEV", + curve25519 = p2$curve, # not the sending device + ed25519 = alice_ed)) +forged_attest <- suppressWarnings( + mx_crypto_process_sync(p2$account, mx_crypto_sessions_new(), p2$sync, + p2$curve, self_id = "@bob:example.org", + devices = wrong_curve)) +expect_equal(length(forged_attest$events), 1L) +expect_false(forged_attest$events[[1]]$sender_verified) + +# The matching device does bind. +p3 <- probe("$1d") +ok <- mx_crypto_process_sync(p3$account, mx_crypto_sessions_new(), p3$sync, + p3$curve, self_id = "@bob:example.org", + devices = alice_devs) +expect_true(ok$events[[1]]$sender_verified) # ---- persist both sides, reload from disk ---- mx_crypto_sessions_save(a_sess, a_store) diff --git a/man/mx_crypto_handle_to_device.Rd b/man/mx_crypto_handle_to_device.Rd index 732f111..81e5fe4 100644 --- a/man/mx_crypto_handle_to_device.Rd +++ b/man/mx_crypto_handle_to_device.Rd @@ -4,7 +4,7 @@ \title{Decrypt an inbound Olm to-device payload} \usage{ mx_crypto_handle_to_device(account, my_curve25519, content, self_id = NULL, - self_ed25519 = NULL) + self_ed25519 = NULL, devices = NULL) } \arguments{ \item{account}{An mx.crypto account handle.} @@ -18,10 +18,17 @@ recipient user-id check is skipped; pass it whenever it is known.} \item{self_ed25519}{Character or NULL. This device's Ed25519 key. Defaults to the account's own key.} + +\item{devices}{List of verified devices from +\code{mx_crypto_known_devices()}, or NULL. Used to tie the payload's +claimed sender to a device whose keys were verified. The result +carries \code{sender_bound}, which is FALSE when no list is supplied +or nothing matches; an unbound sender identity is a claim, not a +fact, because anyone can open an Olm session to this device.} } \value{ -The decrypted event (a parsed list), or NULL if it was not for - us or failed the recipient checks. +The decrypted event (a parsed list) with a \code{sender_bound} + flag, or NULL if it was not for us or failed the recipient checks. } \description{ Accepts an \code{m.room.encrypted} to-device content addressed to this @@ -39,8 +46,9 @@ to our key, not that it was meant for us here. \examples{ \dontrun{ ev <- mx_crypto_handle_to_device(acct, my_curve, td_content, - self_id = "@me:example.org") -if (identical(ev$type, "m.room_key")) { + self_id = "@me:example.org", + devices = mx_crypto_known_devices(cl, uid)) +if (identical(ev$type, "m.room_key") && isTRUE(ev$sender_bound)) { inb <- mx_crypto_inbound_session(ev$content$session_key) } } diff --git a/man/mx_crypto_process_sync.Rd b/man/mx_crypto_process_sync.Rd index c34e516..73af12c 100644 --- a/man/mx_crypto_process_sync.Rd +++ b/man/mx_crypto_process_sync.Rd @@ -4,7 +4,7 @@ \title{Process a sync response: store room keys, decrypt room events} \usage{ mx_crypto_process_sync(account, sessions, sync_resp, self_curve25519, - self_id = NULL) + self_id = NULL, devices = NULL) } \arguments{ \item{account}{An mx.crypto account handle.} @@ -17,6 +17,14 @@ mx_crypto_process_sync(account, sessions, sync_resp, self_curve25519, \item{self_id}{Character or NULL. This user's Matrix id, for \code{is_self} tagging.} + +\item{devices}{List of verified devices from +\code{mx_crypto_known_devices()}, or NULL. Room keys arrive over Olm +carrying a claimed sender, and anyone who can reach this device can +send one, so the claim is only worth something once it is matched +against a device whose \code{device_keys} verified. Without this +list decrypted events always report \code{sender_verified = FALSE}: +they still decrypt, but nothing attests to who sent them.} } \value{ List with \code{events} (decrypted, normalized) and the updated