Do not send an encrypted event no one can read

DESCRIPTION

@@ -1,7 +1,7 @@
 Package: mx.client
 Type: Package
 Title: Stateful Matrix Client Helpers
-Version: 0.2.0
+Version: 0.2.0.2
 Date: 2026-08-04
 Authors@R: c(
     person("Troy", "Hernandez", role = c("aut", "cre"),

NEWS.md

@@ -1,3 +1,40 @@
+# mx.client 0.2.0.2
+
+## Security
+
+* **HIGH**: `mx_send_encrypted()` filtered its own device by `device_id`
+  alone. Matrix device ids are scoped to a user, not globally unique, so
+  every other account whose device happened to share the name was
+  dropped from the recipient list -- two bots both called `BOT`, and the
+  only recipient disappears while the send still returns an event id.
+  The self-filter now compares `(user_id, device_id)`.
+
+* **HIGH**: an encrypted send with no usable recipients posted the
+  `m.room.encrypted` event anyway. Devices are skipped when their keys
+  or one-time keys do not verify, which is deliberate, but skipping all
+  of them means the room key reaches nobody and the caller is handed an
+  event id for a message every recipient will fail to decrypt. Reaching
+  zero now aborts. A room with no other devices at all is unaffected:
+  that is a room of one, and sending to it is fine.
+  The guard counts what the homeserver named, not what survived
+  verification: counting survivors made a room whose only other device
+  is malformed indistinguishable from a room with no other device, and
+  only the second is safe to send to. `mx_crypto_verify_device_map()`
+  reports every `(user_id, device_id)` it was given on a `seen`
+  attribute so that distinction is available at all. An explicit
+  `recipients = list()` is refused too, since supplying recipients skips
+  discovery and every guard with it.
+
+* **HIGH**: `/keys/query` and `/keys/claim` answer 200 with a `failures`
+  map when a server could not be reached, returning whatever they did
+  manage. Both were read as though the successful half were the whole
+  answer, so an unreachable homeserver was indistinguishable from a user
+  with no devices. `mx_crypto_known_devices()` and
+  `mx_crypto_claim_otks()` gain `strict`: they warn by default and error
+  under it, and `mx_send_encrypted()` asks for strict, because
+  encrypting to a user whose devices could not be listed is how a
+  message ends up readable by nobody.
+
 # mx.client 0.2.0
 
 ## Security

R/profile.R

@@ -24,7 +24,7 @@
 #' @export
 mx_set_displayname <- function(client, name, save = TRUE) {
     if (!is.character(name) || length(name) != 1L || is.na(name) ||
-            !nzchar(name)) {
+        !nzchar(name)) {
         stop("name must be a single non-empty string", call. = FALSE)
     }
     mx_with_relogin(client, function(cl) {

R/transport.R

@@ -60,23 +60,53 @@ mx_crypto_publish_keys <- function(client, account, store_dir, n_otks = 50L) {
 #' fails is dropped with a warning naming it, and the remaining devices
 #' are still returned: one bad device must not make a room unusable.
 #'
+#' \code{/keys/query} answers 200 with a \code{failures} map when it could
+#' not reach a server, and returns whatever it did manage. That is not the
+#' same as a user having no devices, so it is never silent: by default it
+#' warns, and with \code{strict = TRUE} it is an error. Encrypting to a
+#' user whose devices could not be listed is how a message ends up
+#' readable by nobody, so the send path asks for strict.
+#'
 #' @param client Matrix client config.
 #' @param user_ids Character vector of Matrix user ids.
+#' @param strict Logical. Treat a \code{failures} map as an error rather
+#'   than a warning.
 #' @return List of verified devices, each \code{list(user_id, device_id,
 #'   curve25519, ed25519)}.
 #' @examples
 #' \dontrun{
 #' mx_crypto_known_devices(client, "@bob:example.org")
 #' }
 #' @export
-mx_crypto_known_devices <- function(client, user_ids) {
+mx_crypto_known_devices <- function(client, user_ids, strict = FALSE) {
     mx_require_crypto()
     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_report_failures(resp$failures, "/keys/query", strict)
     mx_crypto_verify_device_map(resp$device_keys)
 }
 
+# A server we could not reach is not a user with no devices.
+#
+# Both /keys/query and /keys/claim answer 200 with a `failures` map and
+# whatever they did manage beside it. Read only the successful half and an
+# unreachable homeserver looks exactly like an empty one, which on the
+# send path means encrypting to nobody and reporting success.
+mx_crypto_report_failures <- function(failures, what, strict = FALSE) {
+    if (!length(failures)) {
+        return(invisible(FALSE))
+    }
+    msg <- paste0("mx.client: ", what, " could not reach ",
+                  paste(names(failures), collapse = ", "),
+                  "; devices there are unknown, not absent")
+    if (isTRUE(strict)) {
+        stop(msg, call. = FALSE)
+    }
+    warning(msg, call. = FALSE)
+    invisible(TRUE)
+}
+
 # Verify every device in a parsed /keys/query device_keys map.
 #
 # Split out from the HTTP call so the verification logic is testable
@@ -86,11 +116,19 @@ mx_crypto_known_devices <- function(client, user_ids) {
 #
 # mxc_verify_device_keys() raises on failure rather than returning FALSE,
 # and returns the keys it actually checked -- use those, not the raw map.
+#
+# Every (user_id, device_id) the homeserver named comes back on the
+# "seen" attribute, verified or not. Without it a caller cannot tell a
+# user with no devices from a user whose every device was dropped here,
+# and those need different answers: the first is nobody to encrypt to,
+# the second is everybody unreachable.
 mx_crypto_verify_device_map <- function(device_keys_map) {
     out <- list()
+    seen <- list()
     for (uid in names(device_keys_map %||% list())) {
         devs <- device_keys_map[[uid]]
         for (dev in names(devs %||% list())) {
+            seen[[length(seen) + 1L]] <- c(user_id = uid, device_id = dev)
             keys <- tryCatch(
                              mx.crypto::mxc_verify_device_keys(devs[[dev]], uid, dev),
                              error = function(e) {
@@ -105,6 +143,7 @@ mx_crypto_verify_device_map <- function(device_keys_map) {
                 curve25519 = keys$curve25519, ed25519 = keys$ed25519)
         }
     }
+    attr(out, "seen") <- seen
     out
 }
 
@@ -118,16 +157,22 @@ mx_crypto_verify_device_map <- function(device_keys_map) {
 #' (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}.
 #'
+#' \code{/keys/claim} carries the same \code{failures} map as
+#' \code{/keys/query}, and it is handled the same way: warned about by
+#' default, an error under \code{strict}.
+#'
 #' @param client Matrix client config.
 #' @param devices List of devices from \code{mx_crypto_known_devices()}.
+#' @param strict Logical. Treat a \code{failures} map as an error rather
+#'   than a warning.
 #' @return The devices with an \code{otk} field added where one was
 #'   claimed and verified.
 #' @examples
 #' \dontrun{
 #' devs <- mx_crypto_claim_otks(client, mx_crypto_known_devices(client, uid))
 #' }
 #' @export
-mx_crypto_claim_otks <- function(client, devices) {
+mx_crypto_claim_otks <- function(client, devices, strict = FALSE) {
     mx_require_crypto()
     if (!length(devices)) {
         return(devices)
@@ -139,6 +184,7 @@ 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)
+    mx_crypto_report_failures(resp$failures, "/keys/claim", strict)
     mx_crypto_verify_claimed_otks(devices, resp$one_time_keys %||% list())
 }
 
@@ -197,7 +243,11 @@ mx_crypto_verify_claimed_otks <- function(devices, claimed) {
 #' @param content Named list. Plaintext event content.
 #' @param store_dir Character. Crypto store directory.
 #' @param recipients List of recipient devices, or NULL to discover them
-#'   from \code{member_ids}.
+#'   from \code{member_ids}. An empty list is an error: an
+#'   \code{m.room.encrypted} event whose room key was shared with nobody
+#'   is unreadable by everyone, and returning its event id would report
+#'   that as a successful send. Discovery finding nobody is allowed, since
+#'   a room whose other members have no devices is a real room.
 #' @param member_ids Character vector of room member user ids (used when
 #'   \code{recipients} is NULL).
 #' @return List with \code{event_id} and the updated \code{sessions}.
@@ -215,15 +265,33 @@ mx_send_encrypted <- function(client, account, sessions, room_id, content,
     sender_curve <- mx.crypto::mxc_account_identity_keys(account)$curve25519
 
     if (is.null(recipients)) {
-        devs <- mx_crypto_known_devices(client, member_ids)
-        # skip our own device; claim OTKs only where no Olm session exists
+        # strict: a server that could not be reached leaves its users'
+        # devices unknown, and an unknown device is not an absent one. Read
+        # the successful half of a partial response and a federation blip
+        # becomes a message nobody can read.
+        found <- mx_crypto_known_devices(client, member_ids, strict = TRUE)
+        # What the homeserver named, before verification dropped any of
+        # it. Counting the survivors instead made a room where Bob's only
+        # device is malformed look exactly like a room with no Bob in it:
+        # nothing left to encrypt to either way, and only one of those is
+        # a reason to go ahead.
+        others_seen <- Filter(function(sd) {
+            !(identical(sd[["user_id"]], client$user_id) &&
+                              identical(sd[["device_id"]], client$device_id))
+        }, attr(found, "seen") %||% list())
+        # Skip our own device, and only our own. Device ids are scoped to a
+        # user, not globally unique, so comparing device_id alone dropped
+        # every other account whose device happened to share the name --
+        # two bots both called BOT, and the one recipient vanishes while
+        # the send still reports success.
         devs <- Filter(function(d) {
-            !identical(d$device_id, client$device_id) &&
+            !(identical(d$user_id, client$user_id) &&
+                       identical(d$device_id, client$device_id)) &&
             !is.null(d$curve25519) && nzchar(d$curve25519)
-        }, devs)
+        }, found)
         need <- Filter(function(d) is.null(sessions$olm[[d$curve25519]]), devs)
         have <- Filter(function(d) !is.null(sessions$olm[[d$curve25519]]), devs)
-        claimed <- mx_crypto_claim_otks(client, need)
+        claimed <- mx_crypto_claim_otks(client, need, strict = TRUE)
         # 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.
@@ -235,6 +303,32 @@ mx_send_encrypted <- function(client, account, sessions, room_id, content,
                     call. = FALSE)
         }
         recipients <- c(usable, have)
+        # Skipping some devices is the policy above. Skipping all of them
+        # is not: the room key reaches nobody, the m.room.encrypted event
+        # goes out anyway, and the caller is handed an event id for a
+        # message every recipient will fail to decrypt.
+        #
+        # Measured against what the homeserver named, not against what
+        # survived verification -- otherwise a room whose one other device
+        # is malformed is indistinguishable from a room that has no other
+        # device, and the second of those is fine to send to. A member who
+        # has no devices at all is fine too: there is genuinely nobody to
+        # share a key with, and the event still belongs in the room for
+        # whatever device that member logs in later.
+        if (length(others_seen) && !length(recipients)) {
+            stop("mx.client: every other device in ", room_id,
+                 " was skipped (", length(others_seen),
+                 " named by the homeserver, none usable). Refusing to send ",
+                 "an encrypted event whose room key would reach nobody.",
+                 call. = FALSE)
+        }
+    } else if (!length(recipients)) {
+        # An explicit empty list is the same unreadable event, arrived at
+        # by a caller who did their own discovery. Nothing above runs for
+        # them, so say it here rather than let it through.
+        stop("mx.client: recipients is empty. Refusing to send an encrypted ",
+             "event whose room key would reach nobody; pass recipients = ",
+             "NULL with member_ids to have them discovered.", call. = FALSE)
     }
 
     out <- mx_crypto_encrypt_for_devices(account, sessions, room_id,