From 98733cbbc6d46d45a5cfa4b1953a62212df48794 Mon Sep 17 00:00:00 2001 From: koala Date: Wed, 11 Nov 2026 11:56:21 +0800 Subject: [PATCH] fix(contacts): pin a share link's bundle to the user it names A contact share link carries the user_id of the person who shared it, but the client only checked that the fetched identity bundle was consistent with itself. The host behind the link could therefore serve any self-signed bundle, and the client would pin that root key as the contact. Every bundle fetched from a share or identity-bundle URL must now carry the user_id the URL names: the userId in a /v1/contact-share token, or the user_id segment of /v1/shared-state/{user_id}/identity-bundle. A link that names no user is rejected before any request. The host's HMAC is still left to the host. Also remove an unused desktop transport method that fetched a bundle with no URL rules and no verification. Co-Authored-By: Claude Opus 5.5 --- .../src-tauri/src/platform/transport.rs | 32 +--- src/contact_share.rs | 173 +++++++++++++++++- src/contact_workflows.rs | 141 +++++++++++++- src/external_fetch.rs | 2 +- 4 files changed, 315 insertions(+), 33 deletions(-) diff --git a/app/desktop/src-tauri/src/platform/transport.rs b/app/desktop/src-tauri/src/platform/transport.rs index 7ae950d..fd4597e 100644 --- a/app/desktop/src-tauri/src/platform/transport.rs +++ b/app/desktop/src-tauri/src/platform/transport.rs @@ -6,10 +6,10 @@ use tokio::sync::RwLock; use tapchat_core::cli::util::{to_camel_case_json_string, to_snake_case_json_string}; use tapchat_core::ffi_api::{CoreEvent, HttpMethod, HttpRequestEffect}; -use tapchat_core::model::{Ack, IdentityBundle, InboxRecord}; +use tapchat_core::model::{Ack, InboxRecord}; use tapchat_core::transport_contract::{ - AckRequest, AckResult, AppendEnvelopeRequest, AppendEnvelopeResult, FetchIdentityBundleRequest, - GetHeadResult, PrepareBlobUploadRequest, PrepareBlobUploadResult, + AckRequest, AckResult, AppendEnvelopeRequest, AppendEnvelopeResult, GetHeadResult, + PrepareBlobUploadRequest, PrepareBlobUploadResult, }; use crate::platform::log_sanitize::{ @@ -429,32 +429,6 @@ impl DesktopTransport { Ok(result.head_seq) } - /// Fetch identity bundle from URL. - pub async fn fetch_identity_bundle( - &self, - request: FetchIdentityBundleRequest, - ) -> Result { - let reference = request - .reference - .as_ref() - .context("identity bundle fetch missing reference")?; - let response = self - .client - .get(reference) - .send() - .await - .context("fetch identity bundle")?; - - let status = response.status(); - if !status.is_success() { - anyhow::bail!("fetch bundle failed with status {}", status); - } - - let body = response.text().await.context("read identity bundle")?; - let body = to_snake_case_json_string(&body)?; - serde_json::from_str(&body).context("parse identity bundle") - } - /// Prepare blob upload. pub async fn prepare_blob_upload( &self, diff --git a/src/contact_share.rs b/src/contact_share.rs index cc5e614..92d4a3d 100644 --- a/src/contact_share.rs +++ b/src/contact_share.rs @@ -1,10 +1,14 @@ use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine as _}; use hmac::{Hmac, Mac}; +use serde_json::Value; use sha2::Sha256; use crate::error::{CoreError, CoreResult}; +use crate::external_fetch::{decoded_path_segments, validate_external_url, ExternalResourceKind}; use crate::model::CURRENT_MODEL_VERSION; +const CONTACT_SHARE_SERVICE: &str = "contact_share"; + pub fn encode_contact_share_url( base_url: &str, secret: &str, @@ -22,7 +26,7 @@ pub fn encode_contact_share_url( } let payload = serde_json::json!({ "version": CURRENT_MODEL_VERSION, - "service": "contact_share", + "service": CONTACT_SHARE_SERVICE, "userId": user_id, "shareId": share_id, }); @@ -43,9 +47,100 @@ pub fn encode_contact_share_url( )) } +/// The user_id a contact share or identity bundle URL names: the `userId` in +/// the token of `/v1/contact-share/{token}`, or the `{user_id}` segment of +/// `/v1/shared-state/{user_id}/identity-bundle`. +/// +/// The host behind the URL can serve any bundle that verifies against its own +/// root key, so a bundle fetched from the URL is the named user's only if it +/// carries this user_id. +pub fn contact_share_url_user_id(raw_url: &str) -> CoreResult { + let decoded = validate_external_url(raw_url, ExternalResourceKind::ContactShare) + .and_then(|url| decoded_path_segments(&url)) + .map_err(|error| { + CoreError::invalid_input(format!("contact share link is invalid: {error}")) + })?; + let segments: Vec<&str> = decoded.iter().map(String::as_str).collect(); + match segments.as_slice() { + ["v1", "contact-share", token] => contact_share_token_user_id(token), + ["v1", "shared-state", user_id, "identity-bundle"] if !user_id.trim().is_empty() => { + Ok(user_id.to_string()) + } + _ => Err(no_user_id()), + } +} + +/// The user_id a contact share token names. +/// +/// The token is `base64url(payload) "." base64url(hmac)`. The HMAC is keyed +/// with the host's sharing secret, so only the host can check it; it must +/// still be present and well formed. +pub fn contact_share_token_user_id(token: &str) -> CoreResult { + let (payload_part, signature_part) = token.split_once('.').ok_or_else(malformed_token)?; + if payload_part.is_empty() || signature_part.is_empty() { + return Err(malformed_token()); + } + URL_SAFE_NO_PAD + .decode(signature_part) + .map_err(|_| malformed_token())?; + let bytes = URL_SAFE_NO_PAD + .decode(payload_part) + .map_err(|_| malformed_token())?; + let payload: Value = serde_json::from_slice(&bytes).map_err(|_| malformed_payload())?; + let payload = payload.as_object().ok_or_else(malformed_payload)?; + if payload.get("service").and_then(Value::as_str) != Some(CONTACT_SHARE_SERVICE) { + return Err(CoreError::invalid_input( + "contact share link is not for the contact_share service", + )); + } + payload + .get("userId") + .and_then(Value::as_str) + .filter(|user_id| !user_id.trim().is_empty()) + .map(String::from) + .ok_or_else(no_user_id) +} + +fn malformed_token() -> CoreError { + CoreError::invalid_input("contact share link token is malformed") +} + +fn malformed_payload() -> CoreError { + CoreError::invalid_input("contact share link payload is malformed") +} + +fn no_user_id() -> CoreError { + CoreError::invalid_input("contact share link does not name a user") +} + #[cfg(test)] mod tests { - use super::encode_contact_share_url; + use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine as _}; + use serde_json::json; + + use super::{contact_share_token_user_id, contact_share_url_user_id, encode_contact_share_url}; + + fn token(payload: serde_json::Value) -> String { + let payload = URL_SAFE_NO_PAD.encode(serde_json::to_vec(&payload).expect("json")); + let signature = URL_SAFE_NO_PAD.encode([7_u8; 32]); + format!("{payload}.{signature}") + } + + fn share_url(token: &str) -> String { + format!("https://runtime.example/v1/contact-share/{token}") + } + + fn assert_token_rejected(token: &str, expected: &str) { + let error = contact_share_token_user_id(token).expect_err("token must be rejected"); + assert_eq!(error.code(), "invalid_input"); + assert!(error.message().contains(expected), "{token}: {error}"); + } + + fn assert_url_rejected(url: &str, expected: &str) { + let error = contact_share_url_user_id(url).expect_err("URL must be rejected"); + assert_eq!(error.code(), "invalid_input"); + assert!(error.message().contains(expected), "{url}: {error}"); + } #[test] fn encoder_only_emits_canonical_contact_share_path() { @@ -59,4 +154,78 @@ mod tests { assert!(url.starts_with("https://runtime.example/v1/contact-share/")); assert!(!url.contains("identity-bundle")); } + + #[test] + fn user_id_is_read_from_both_url_forms() { + let encoded = encode_contact_share_url( + "https://runtime.example/", + "0123456789abcdef0123456789abcdef", + "user:alice", + "share:1", + ) + .expect("contact share URL"); + let user_id = contact_share_url_user_id(&encoded).expect("token form"); + assert_eq!(user_id, "user:alice"); + + let bundle_url = "https://runtime.example/v1/shared-state/user%3Aalice/identity-bundle"; + let user_id = contact_share_url_user_id(bundle_url).expect("path form"); + assert_eq!(user_id, "user:alice"); + + // The HMAC is the host's to check; the client reads only the payload. + let unchecked = token(json!({"service": "contact_share", "userId": "user:bob"})); + let user_id = contact_share_url_user_id(&share_url(&unchecked)).expect("token form"); + assert_eq!(user_id, "user:bob"); + } + + #[test] + fn malformed_tokens_and_payloads_are_rejected() { + let body = URL_SAFE_NO_PAD.encode(br#"{"service":"contact_share","userId":"user:a"}"#); + let sig = URL_SAFE_NO_PAD.encode([7_u8; 32]); + assert_token_rejected("no-separator", "token is malformed"); + assert_token_rejected(&format!("{body}."), "token is malformed"); + assert_token_rejected(&format!(".{sig}"), "token is malformed"); + assert_token_rejected(&format!("{body}.not+base64url"), "token is malformed"); + assert_token_rejected(&format!("{body}.{sig}.{sig}"), "token is malformed"); + assert_token_rejected(&format!("!!!.{sig}"), "token is malformed"); + + let not_json = URL_SAFE_NO_PAD.encode(b"not json"); + assert_token_rejected(&format!("{not_json}.{sig}"), "payload is malformed"); + let not_object = URL_SAFE_NO_PAD.encode(br#"["contact_share","user:a"]"#); + assert_token_rejected(&format!("{not_object}.{sig}"), "payload is malformed"); + } + + #[test] + fn payload_must_be_a_contact_share_that_names_a_user() { + let wrong_service = token(json!({"service": "group_invite", "userId": "user:a"})); + assert_token_rejected(&wrong_service, "not for the contact_share service"); + let no_service = token(json!({"userId": "user:a"})); + assert_token_rejected(&no_service, "not for the contact_share service"); + let no_user = token(json!({"service": "contact_share"})); + assert_token_rejected(&no_user, "does not name a user"); + let blank_user = token(json!({"service": "contact_share", "userId": " "})); + assert_token_rejected(&blank_user, "does not name a user"); + let numeric_user = token(json!({"service": "contact_share", "userId": 7})); + assert_token_rejected(&numeric_user, "does not name a user"); + } + + #[test] + fn urls_that_name_no_user_are_rejected() { + let no_user = token(json!({"service": "contact_share"})); + assert_url_rejected(&share_url(&no_user), "does not name a user"); + let wrong_service = token(json!({"service": "group_invite", "userId": "user:a"})); + assert_url_rejected(&share_url(&wrong_service), "contact_share service"); + assert_url_rejected(&share_url("not-a-token"), "token is malformed"); + assert_url_rejected( + "https://runtime.example/v1/shared-state/%20/identity-bundle", + "does not name a user", + ); + for invalid in [ + "https://runtime.example/v1/shared-state//identity-bundle", + "https://runtime.example/v1/shared-state/user%3Aa/device-status", + "https://runtime.example/v1/group-invite/token", + "file:///v1/shared-state/user%3Aa/identity-bundle", + ] { + assert_url_rejected(invalid, "link is invalid"); + } + } } diff --git a/src/contact_workflows.rs b/src/contact_workflows.rs index def1931..703a1ba 100644 --- a/src/contact_workflows.rs +++ b/src/contact_workflows.rs @@ -3,6 +3,8 @@ use anyhow::{anyhow, Context, Result}; use crate::cli::driver::CoreDriver; use crate::cli::profile::Profile; use crate::cli::util::to_snake_case_json_string; +use crate::contact_share::contact_share_url_user_id; +use crate::error::{CoreError, CoreResult}; use crate::external_fetch::{fetch_external_json, ExternalResourceKind}; use crate::ffi_api::{CoreCommand, CoreOutput, MessageRequestActionSummary}; use crate::identity::IdentityManager; @@ -90,13 +92,30 @@ fn ensure_fetched_bundle_matches_expected_sender( Ok(()) } +fn ensure_bundle_matches_link(bundle: &IdentityBundle, link_user_id: &str) -> CoreResult<()> { + if bundle.user_id != link_user_id { + return Err(CoreError::invalid_input( + "fetched identity bundle belongs to a different user than the link names", + )); + } + Ok(()) +} + +/// Fetches the identity bundle behind a contact share or identity bundle URL. +/// +/// Every import from such a URL comes through here. The host behind the URL +/// is not trusted, and a bundle that verifies proves only that it is +/// consistent with itself, so it is accepted only as the bundle of the user +/// the URL names; otherwise the host would choose whose root key is pinned. pub async fn fetch_identity_bundle_from_url(url: &str) -> Result { + let link_user_id = contact_share_url_user_id(url)?; let body = fetch_external_json(url, ExternalResourceKind::ContactShare) .await .map_err(anyhow::Error::from)?; let normalized = to_snake_case_json_string(&body).context("normalize contact share json")?; let mut bundle: IdentityBundle = serde_json::from_str(&normalized).context("decode contact share bundle")?; + ensure_bundle_matches_link(&bundle, &link_user_id)?; if bundle.identity_bundle_ref.is_none() { bundle.identity_bundle_ref = Some(url.to_string()); } @@ -141,9 +160,129 @@ pub fn message_request_action_from_output( #[cfg(test)] mod tests { - use super::ensure_fetched_bundle_matches_expected_sender; + use std::io::{Read, Write}; + use std::net::TcpListener; + + use super::{ensure_fetched_bundle_matches_expected_sender, fetch_identity_bundle_from_url}; + use crate::contact_share::encode_contact_share_url; + use crate::identity::IdentityManager; + use crate::mls_adapter::MlsAdapter; use crate::model::IdentityBundle; + const NAMED_MNEMONIC: &str = "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about"; + const IMPOSTOR_MNEMONIC: &str = + "legal winner thank year wave sausage worth useful legal winner thank yellow"; + const SECRET: &str = "0123456789abcdef0123456789abcdef"; + + /// A bundle signed by the root key `mnemonic` recovers, as its host serves it. + fn signed_bundle(mnemonic: &str) -> IdentityBundle { + let identity = IdentityManager::create_or_recover(Some(mnemonic), None).expect("root"); + let now_ms = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("test clock") + .as_millis() as u64; + let package = MlsAdapter::generate_key_package(&identity, now_ms).expect("key package"); + let mut deployment = crate::ffi_api::tests::tests::sample_deployment(); + deployment.runtime_config.identity_bundle_ref = + Some("https://runtime.example/v1/shared-state/{userId}/identity-bundle".into()); + let bundle = IdentityManager::export_identity_bundle( + &identity, + &deployment, + package.key_package_b64, + package.expires_at, + ) + .expect("bundle"); + IdentityManager::verify_identity_bundle(&bundle).expect("the test bundle verifies"); + bundle + } + + /// Answers one request with `body` as JSON; returns the server's base URL. + fn serve_json_once(body: String) -> String { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind test server"); + let address = listener.local_addr().expect("server address"); + std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept request"); + let mut request = [0_u8; 4096]; + let _ = stream.read(&mut request); + write!( + stream, + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + body.len(), + ) + .expect("write response"); + }); + format!("http://{address}") + } + + /// A base URL at which nothing listens. + fn closed_base_url() -> String { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind test port"); + let address = listener.local_addr().expect("test port address"); + drop(listener); + format!("http://{address}") + } + + fn share_link(base: &str, user_id: &str) -> String { + encode_contact_share_url(base, SECRET, user_id, "share:1").expect("share URL") + } + + fn bundle_link(base: &str, user_id: &str) -> String { + let user_id = urlencoding::encode(user_id); + format!("{base}/v1/shared-state/{user_id}/identity-bundle") + } + + #[tokio::test] + async fn fetch_rejects_a_valid_bundle_of_another_user_than_the_link_names() { + let named = signed_bundle(NAMED_MNEMONIC); + let impostor = signed_bundle(IMPOSTOR_MNEMONIC); + assert_ne!(impostor.user_id, named.user_id); + let served = serde_json::to_string(&impostor).expect("bundle json"); + for link in [ + share_link(&serve_json_once(served.clone()), &named.user_id), + bundle_link(&serve_json_once(served.clone()), &named.user_id), + ] { + let error = fetch_identity_bundle_from_url(&link) + .await + .expect_err("a bundle of another user must be rejected"); + let message = error.to_string(); + assert!(message.contains("different user"), "{message}"); + } + } + + #[tokio::test] + async fn fetch_accepts_the_bundle_of_the_user_the_link_names() { + let named = signed_bundle(NAMED_MNEMONIC); + let served = serde_json::to_string(&named).expect("bundle json"); + for link in [ + share_link(&serve_json_once(served.clone()), &named.user_id), + bundle_link(&serve_json_once(served.clone()), &named.user_id), + ] { + let fetched = fetch_identity_bundle_from_url(&link) + .await + .expect("the bundle of the named user is accepted"); + assert_eq!(fetched.user_id, named.user_id); + assert_eq!(fetched.user_public_key, named.user_public_key); + } + } + + #[tokio::test] + async fn fetch_rejects_a_link_that_names_no_user_before_any_request() { + // Nothing listens at `base`, so a request would fail as a fetch error. + let base = closed_base_url(); + let blank_user = format!("{base}/v1/shared-state/%20/identity-bundle"); + let bad_token = format!("{base}/v1/contact-share/not-a-token"); + for (link, expected) in [ + (blank_user, "does not name a user"), + (bad_token, "token is malformed"), + ] { + let error = fetch_identity_bundle_from_url(&link) + .await + .expect_err("a link that names no user must be rejected"); + let message = error.to_string(); + assert!(message.contains(expected), "{link}: {message}"); + } + } + fn bundle_with_user_id(user_id: &str) -> IdentityBundle { serde_json::from_value(serde_json::json!({ "version": "v1", diff --git a/src/external_fetch.rs b/src/external_fetch.rs index 56506b7..3b13c87 100644 --- a/src/external_fetch.rs +++ b/src/external_fetch.rs @@ -204,7 +204,7 @@ fn validate_tapchat_path( } } -fn decoded_path_segments(url: &Url) -> Result, ExternalFetchError> { +pub(crate) fn decoded_path_segments(url: &Url) -> Result, ExternalFetchError> { url.path_segments() .ok_or_else(|| ExternalFetchError::rejected("external URL path is invalid"))? .map(|segment| {