From 7f787bc60600c2c9ffc55a75747f77e0377453ac Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Tue, 4 Aug 2026 13:27:29 +0200 Subject: [PATCH 1/7] feat: add request-state key rotation --- README.md | 5 + crates/rmcp/CHANGELOG.md | 6 + crates/rmcp/src/model/request_state.rs | 977 ++++++++++++++++++++++++- 3 files changed, 948 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index b6c580e17..b1c5ea77e 100644 --- a/README.md +++ b/README.md @@ -1369,6 +1369,11 @@ async fn call_tool(&self, request: CallToolRequestParams, _ctx: RequestContext and open it (HMAC-tagged), or keep state server-side and use `requestState` > only as an opaque handle. +For multi-replica deployments, use a `RequestStateCodec` keyring to rotate +signing keys without invalidating in-flight requests. Follow the +[`RequestStateCodec` key-rotation rustdocs](https://docs.rs/rmcp/latest/rmcp/model/struct.RequestStateCodec.html#key-rotation) +for the rolling-safe `rs1`-to-`rs2` migration and key-retirement procedure. + ### Client-side The high-level `call_tool`, `get_prompt`, and `read_resource` helpers drive MRTR diff --git a/crates/rmcp/CHANGELOG.md b/crates/rmcp/CHANGELOG.md index b4e8cf612..c89818d7f 100644 --- a/crates/rmcp/CHANGELOG.md +++ b/crates/rmcp/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- add `RequestStateCodec` keyrings and authenticated `rs2` key identifiers for + rolling-safe key rotation while preserving `new()` and the legacy `rs1` + format + ## [3.1.0](https://github.com/modelcontextprotocol/rust-sdk/compare/rmcp-v3.0.1...rmcp-v3.1.0) - 2026-07-31 ### Added diff --git a/crates/rmcp/src/model/request_state.rs b/crates/rmcp/src/model/request_state.rs index 5ed0f7de8..11f51888a 100644 --- a/crates/rmcp/src/model/request_state.rs +++ b/crates/rmcp/src/model/request_state.rs @@ -60,7 +60,10 @@ //! assert!(codec.open_with(&sealed, b"user:bob|tools/call:weather").is_err()); //! ``` -use std::time::Duration; +use std::{ + collections::{HashMap, hash_map::Entry}, + time::Duration, +}; use base64::{Engine, engine::general_purpose::URL_SAFE_NO_PAD}; use hmac::{Hmac, KeyInit, Mac}; @@ -70,18 +73,31 @@ use thiserror::Error; type HmacSha256 = Hmac; -/// Version tag prefixing every sealed value, so the wire format can evolve. -const VERSION: &str = "rs1"; +/// Legacy version tag used by single-key and transitional codecs. +const VERSION_V1: &str = "rs1"; + +/// Keyed version tag used by keyring codecs after promotion. +const VERSION_V2: &str = "rs2"; /// Domain-separation label mixed into the HMAC so a `requestState` tag can never /// be confused with an HMAC computed for some other purpose using the same key. -const DOMAIN: &[u8] = b"rmcp/mrtr/request-state/v1"; +const DOMAIN_V1: &[u8] = b"rmcp/mrtr/request-state/v1"; + +/// Domain-separation label for keyed request-state tags. +const DOMAIN_V2: &[u8] = b"rmcp/mrtr/request-state/v2"; /// Length of the big-endian expiry prefix (unix milliseconds) stored at the /// front of every sealed body. `0` means "no expiry". const EXPIRY_LEN: usize = 8; -/// Errors returned when opening a sealed [`RequestStateCodec`] value. +/// Maximum decoded UTF-8 byte length of an rs2 key id. +const MAX_KID_LEN: usize = 255; + +/// Maximum unpadded base64url length for [`MAX_KID_LEN`] bytes. +const MAX_ENCODED_KID_LEN: usize = 340; + +/// Errors returned when configuring, sealing, or opening a +/// [`RequestStateCodec`] value. #[derive(Debug, Error)] #[non_exhaustive] pub enum RequestStateError { @@ -103,6 +119,18 @@ pub enum RequestStateError { #[error("request state has expired")] Expired, + /// The token names (or, for rs1, requires) a key this codec does not hold. + #[error("request state was sealed with an unknown key")] + UnknownKeyId, + + /// The decoded rs2 key id was empty, too long, or not valid UTF-8. + #[error("request state contains an invalid key identifier")] + InvalidKeyId, + + /// A keyring constructor or builder was given an invalid configuration. + #[error("invalid keyring configuration: {0}")] + InvalidKeyring(&'static str), + /// The sealed payload could not be serialized to JSON. #[error("failed to serialize request state payload: {0}")] Serialization(#[source] serde_json::Error), @@ -151,32 +179,256 @@ impl<'a> SealOptions<'a> { /// A keyed codec that seals and opens SEP-2322 `requestState` values with /// HMAC-SHA256 integrity protection. /// -/// Construct one codec per signing key and reuse it for the lifetime of the -/// key. The same key must be used to [`seal`](Self::seal) and -/// [`open`](Self::open) a value, so it has to survive across the rounds of a -/// single MRTR exchange (e.g. a stable per-process or per-deployment secret). +/// [`new`](Self::new) preserves the legacy single-key `rs1` behavior. A codec +/// built with [`keyring`](Self::keyring) emits keyed `rs2` values and opens +/// them with one exact key lookup. [`with_rs1_signing`](Self::with_rs1_signing) +/// supports a rolling format migration by retaining `rs1` output while the +/// ring can already verify `rs2`; [`add_rs1_fallback`](Self::add_rs1_fallback) +/// retains explicitly selected keys for opening in-flight `rs1` values. +/// +/// Keys may be any length because HMAC internally normalizes them. For +/// meaningful security use high-entropy keys of at least 32 bytes and keep +/// them stable across every replica that may continue an MRTR exchange. +/// This codec provides integrity, not confidentiality: both the `rs2` key id +/// and payload are base64url-readable by clients and are not encrypted. +/// +/// # Key rotation +/// +/// Keyring codecs emit the keyed `rs2` format and select one verification key +/// from the token's authenticated key id. Migrating a rolling deployment from +/// the unkeyed `rs1` format takes two deployments so an upgraded replica never +/// emits a token that another serving replica cannot open: /// -/// The key may be any length; HMAC internally normalizes it. For meaningful -/// security use a high-entropy key of at least 32 bytes. +/// 1. Deploy every replica with both keys and [`with_rs1_signing`](Self::with_rs1_signing). +/// The codec keeps emitting `rs1`, but can already open `rs2` with either +/// ring key. +/// 2. After every serving replica has that configuration, redeploy without +/// `with_rs1_signing`, select the new key for `rs2`, and retain the old key +/// through [`add_rs1_fallback`](Self::add_rs1_fallback). +/// 3. After that deployment finishes, wait longer than the maximum effective +/// lifetime of old request states before removing the old key and fallback. +/// +/// ``` +/// use rmcp::model::{RequestStateCodec, RequestStateError}; +/// +/// # fn configure() -> Result<(), RequestStateError> { +/// let old_key = b"old-request-state-key-at-least-32b"; +/// let new_key = b"new-request-state-key-at-least-32b"; +/// +/// // First deployment: publish rs2 support and the new key, but emit rs1. +/// let transitional = RequestStateCodec::keyring( +/// "new", +/// [("old", old_key.as_slice()), ("new", new_key.as_slice())], +/// )? +/// .with_rs1_signing("old")?; +/// assert!(transitional.seal(b"state").starts_with("rs1.")); +/// +/// // Second deployment: emit rs2/new while accepting in-flight rs1/old. +/// let promoted = RequestStateCodec::keyring( +/// "new", +/// [("old", old_key.as_slice()), ("new", new_key.as_slice())], +/// )? +/// .add_rs1_fallback("old")?; +/// assert!(promoted.seal(b"state").starts_with("rs2.")); +/// # Ok(()) +/// # } +/// # configure().unwrap(); +/// ``` +/// +/// The codec defaults to no TTL. If old states have no other bounded effective +/// lifetime, their verification key cannot be retired safely. Runtime keyring +/// replacement is not built in; rebuild and redeploy the codec when keys +/// change. +/// +/// If more than one legacy `rs1` key is still valid, add each with +/// [`add_rs1_fallback`](Self::add_rs1_fallback); trial verification is +/// unavoidable because `rs1` carries no kid. For later `rs2`-to-`rs2` +/// rotations, first deploy every replica with both keys while the old kid +/// remains selected, then promote the new kid, wait for old states to drain, +/// and finally remove the old key. #[derive(Clone)] pub struct RequestStateCodec { - key: Box<[u8]>, + keys: Keys, +} + +#[derive(Clone)] +enum Keys { + Single(Box<[u8]>), + Ring { + keys: HashMap>, + seal_mode: SealMode, + rs1_fallbacks: Vec, + }, +} + +#[derive(Clone, Debug)] +enum SealMode { + Rs1 { key_id: String }, + Rs2 { key_id: String }, } impl std::fmt::Debug for RequestStateCodec { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - // Never leak the signing key through Debug output. - f.debug_struct("RequestStateCodec") - .field("key", &"") - .finish() + // Never leak signing or verification keys through Debug output. + match &self.keys { + Keys::Single(_) => f + .debug_struct("RequestStateCodec") + .field("mode", &"single") + .field("key", &"") + .finish(), + Keys::Ring { + keys, + seal_mode, + rs1_fallbacks, + } => f + .debug_struct("RequestStateCodec") + .field("mode", &"ring") + .field("key_count", &keys.len()) + .field("keys", &"") + .field("seal_mode", seal_mode) + .field("rs1_fallbacks", rs1_fallbacks) + .finish(), + } } } impl RequestStateCodec { - /// Creates a codec from a signing key. + /// Creates a legacy single-key codec that seals and opens `rs1` values. pub fn new(key: impl Into>) -> Self { Self { - key: key.into().into_boxed_slice(), + keys: Keys::Single(key.into().into_boxed_slice()), + } + } + + /// Creates a keyring codec that seals `rs2` with `active_kid` and opens + /// `rs2` by exact key-id lookup. + /// + /// Ring codecs do not open legacy `rs1` values unless a fallback is added + /// with [`add_rs1_fallback`](Self::add_rs1_fallback), or transitional + /// `rs1` sealing is enabled with + /// [`with_rs1_signing`](Self::with_rs1_signing). + /// + /// Key ids are opaque, case-sensitive UTF-8 strings of 1 to 255 bytes. + /// + /// # Errors + /// + /// Returns [`RequestStateError::InvalidKeyring`] if the keyring is empty, + /// contains duplicate or invalid key ids, or does not contain `active_kid`. + pub fn keyring( + active_kid: impl Into, + keys: impl IntoIterator, + ) -> Result + where + K: Into, + V: Into>, + { + let mut ring = HashMap::new(); + for (kid, key) in keys { + let kid = kid.into(); + Self::validate_config_kid(&kid)?; + match ring.entry(kid) { + Entry::Vacant(entry) => { + entry.insert(key.into().into_boxed_slice()); + } + Entry::Occupied(_) => { + return Err(RequestStateError::InvalidKeyring("duplicate key id")); + } + } + } + + if ring.is_empty() { + return Err(RequestStateError::InvalidKeyring( + "keyring must contain at least one key", + )); + } + + let active_kid = active_kid.into(); + Self::validate_config_kid(&active_kid)?; + if !ring.contains_key(&active_kid) { + return Err(RequestStateError::InvalidKeyring( + "active key id is not present in the keyring", + )); + } + + Ok(Self { + keys: Keys::Ring { + keys: ring, + seal_mode: SealMode::Rs2 { key_id: active_kid }, + rs1_fallbacks: Vec::new(), + }, + }) + } + + /// Switches a Ring into transitional mode that seals `rs1` with `kid`. + /// + /// The selected key is also added to the legacy `rs1` fallback set, so the + /// resulting codec can open its own output. The Ring can still open `rs2` + /// tokens for every key id it contains. + /// + /// # Errors + /// + /// Returns [`RequestStateError::InvalidKeyring`] when called on a codec + /// created by [`new`](Self::new), or if the Ring does not contain `kid`. + pub fn with_rs1_signing(mut self, kid: impl AsRef) -> Result { + let kid = kid.as_ref(); + match &mut self.keys { + Keys::Single(_) => Err(RequestStateError::InvalidKeyring( + "rs1 transitional signing requires a keyring", + )), + Keys::Ring { + keys, + seal_mode, + rs1_fallbacks, + } => { + if !keys.contains_key(kid) { + return Err(RequestStateError::InvalidKeyring( + "rs1 signing key id is not present in the keyring", + )); + } + + let kid = kid.to_owned(); + *seal_mode = SealMode::Rs1 { + key_id: kid.clone(), + }; + if !rs1_fallbacks.iter().any(|existing| existing == &kid) { + rs1_fallbacks.push(kid); + } + Ok(self) + } + } + } + + /// Adds `kid` to the keys tried when opening legacy `rs1` values. + /// + /// Because `rs1` carries no key id, multiple fallbacks require one HMAC per + /// configured key until a match is found. Adding the same id more than once + /// is idempotent. `rs2` verification always uses one exact lookup. + /// + /// # Errors + /// + /// Returns [`RequestStateError::InvalidKeyring`] when called on a codec + /// created by [`new`](Self::new), or if the Ring does not contain `kid`. + pub fn add_rs1_fallback(mut self, kid: impl AsRef) -> Result { + let kid = kid.as_ref(); + match &mut self.keys { + Keys::Single(_) => Err(RequestStateError::InvalidKeyring( + "rs1 fallbacks require a keyring", + )), + Keys::Ring { + keys, + rs1_fallbacks, + .. + } => { + if !keys.contains_key(kid) { + return Err(RequestStateError::InvalidKeyring( + "rs1 fallback key id is not present in the keyring", + )); + } + if !rs1_fallbacks.iter().any(|existing| existing == kid) { + rs1_fallbacks.push(kid.to_owned()); + } + Ok(self) + } } } @@ -235,12 +487,24 @@ impl RequestStateCodec { /// /// # Errors /// - /// - [`RequestStateError::IntegrityCheckFailed`] if the value was not - /// produced by this key or the associated data differs. + /// - [`RequestStateError::UnknownKeyId`] if the codec cannot select a key + /// for a structurally valid value. + /// - [`RequestStateError::IntegrityCheckFailed`] if the tag does not match + /// the selected key or the associated data differs. /// - [`RequestStateError::Expired`] if the value's TTL has elapsed. /// - [`RequestStateError::MalformedFormat`] or /// [`RequestStateError::InvalidEncoding`] if it is not a well-formed sealed /// value. + /// - [`RequestStateError::InvalidKeyId`] if an `rs2` key id is empty, too + /// long, or not valid UTF-8. + /// + /// Version and segment-count validation happens first. For `rs2`, kid + /// validation and key selection happen before body/tag decoding, so an + /// unknown kid takes precedence over errors in later sections. A Ring with + /// no `rs1` fallbacks likewise returns `UnknownKeyId` after validating the + /// three-segment `rs1` structure and before decoding its body or tag. + /// Applications should expose a common client-facing error for every open + /// failure and reserve the detailed variants for internal diagnostics. pub fn open_with( &self, sealed: &str, @@ -287,24 +551,76 @@ impl RequestStateCodec { body.extend_from_slice(&expiry.to_be_bytes()); body.extend_from_slice(payload); - let tag = self - .mac_for(options.associated_data, &body) + match &self.keys { + Keys::Single(key) => Self::seal_rs1(key, options.associated_data, &body), + Keys::Ring { + keys, seal_mode, .. + } => match seal_mode { + SealMode::Rs1 { key_id } => Self::seal_rs1( + keys.get(key_id).expect("validated rs1 signing key"), + options.associated_data, + &body, + ), + SealMode::Rs2 { key_id } => Self::seal_rs2( + key_id, + keys.get(key_id).expect("validated rs2 signing key"), + options.associated_data, + &body, + ), + }, + } + } + + fn open_at( + &self, + sealed: &str, + associated_data: &[u8], + now_ms: i64, + ) -> Result, RequestStateError> { + match sealed.split('.').next() { + Some(VERSION_V1) => self.open_rs1_at(sealed, associated_data, now_ms), + Some(VERSION_V2) => self.open_rs2_at(sealed, associated_data, now_ms), + _ => Err(RequestStateError::MalformedFormat), + } + } + + fn seal_rs1(key: &[u8], associated_data: &[u8], body: &[u8]) -> String { + let tag = Self::mac_v1(key, associated_data, body) .finalize() .into_bytes(); + let mut out = String::with_capacity( + VERSION_V1.len() + 2 + Self::b64_len(body.len()) + Self::b64_len(tag.len()), + ); + out.push_str(VERSION_V1); + out.push('.'); + URL_SAFE_NO_PAD.encode_string(body, &mut out); + out.push('.'); + URL_SAFE_NO_PAD.encode_string(tag.as_slice(), &mut out); + out + } - // base64url without padding encodes 3 bytes as 4 chars, rounding up. - let b64_len = |n: usize| n.div_ceil(3) * 4; - let mut out = - String::with_capacity(VERSION.len() + 2 + b64_len(body.len()) + b64_len(tag.len())); - out.push_str(VERSION); + fn seal_rs2(kid: &str, key: &[u8], associated_data: &[u8], body: &[u8]) -> String { + let tag = Self::mac_v2(key, kid.as_bytes(), associated_data, body) + .finalize() + .into_bytes(); + let mut out = String::with_capacity( + VERSION_V2.len() + + 3 + + Self::b64_len(kid.len()) + + Self::b64_len(body.len()) + + Self::b64_len(tag.len()), + ); + out.push_str(VERSION_V2); + out.push('.'); + URL_SAFE_NO_PAD.encode_string(kid.as_bytes(), &mut out); out.push('.'); - URL_SAFE_NO_PAD.encode_string(&body, &mut out); + URL_SAFE_NO_PAD.encode_string(body, &mut out); out.push('.'); URL_SAFE_NO_PAD.encode_string(tag.as_slice(), &mut out); out } - fn open_at( + fn open_rs1_at( &self, sealed: &str, associated_data: &[u8], @@ -314,10 +630,17 @@ impl RequestStateCodec { let version = parts.next().ok_or(RequestStateError::MalformedFormat)?; let body_b64 = parts.next().ok_or(RequestStateError::MalformedFormat)?; let tag_b64 = parts.next().ok_or(RequestStateError::MalformedFormat)?; - if parts.next().is_some() || version != VERSION { + if parts.next().is_some() || version != VERSION_V1 { return Err(RequestStateError::MalformedFormat); } + if matches!( + &self.keys, + Keys::Ring { rs1_fallbacks, .. } if rs1_fallbacks.is_empty() + ) { + return Err(RequestStateError::UnknownKeyId); + } + let body = URL_SAFE_NO_PAD .decode(body_b64) .map_err(|_| RequestStateError::InvalidEncoding)?; @@ -325,11 +648,76 @@ impl RequestStateCodec { .decode(tag_b64) .map_err(|_| RequestStateError::InvalidEncoding)?; - // `verify_slice` compares in constant time and rejects wrong-length tags. - self.mac_for(associated_data, &body) + match &self.keys { + Keys::Single(key) => Self::mac_v1(key, associated_data, &body) + .verify_slice(&tag) + .map_err(|_| RequestStateError::IntegrityCheckFailed)?, + Keys::Ring { + keys, + rs1_fallbacks, + .. + } => { + let verified = rs1_fallbacks.iter().any(|kid| { + let key = keys.get(kid).expect("validated rs1 fallback key"); + Self::mac_v1(key, associated_data, &body) + .verify_slice(&tag) + .is_ok() + }); + if !verified { + return Err(RequestStateError::IntegrityCheckFailed); + } + } + } + + Self::open_authenticated_body(body, now_ms) + } + + fn open_rs2_at( + &self, + sealed: &str, + associated_data: &[u8], + now_ms: i64, + ) -> Result, RequestStateError> { + let mut parts = sealed.split('.'); + let version = parts.next().ok_or(RequestStateError::MalformedFormat)?; + let kid_b64 = parts.next().ok_or(RequestStateError::MalformedFormat)?; + let body_b64 = parts.next().ok_or(RequestStateError::MalformedFormat)?; + let tag_b64 = parts.next().ok_or(RequestStateError::MalformedFormat)?; + if parts.next().is_some() || version != VERSION_V2 { + return Err(RequestStateError::MalformedFormat); + } + + if kid_b64.len() > MAX_ENCODED_KID_LEN { + return Err(RequestStateError::InvalidKeyId); + } + let kid = URL_SAFE_NO_PAD + .decode(kid_b64) + .map_err(|_| RequestStateError::InvalidEncoding)?; + if kid.is_empty() || kid.len() > MAX_KID_LEN { + return Err(RequestStateError::InvalidKeyId); + } + let kid = String::from_utf8(kid).map_err(|_| RequestStateError::InvalidKeyId)?; + + let key = match &self.keys { + Keys::Single(_) => return Err(RequestStateError::UnknownKeyId), + Keys::Ring { keys, .. } => keys.get(&kid).ok_or(RequestStateError::UnknownKeyId)?, + }; + + let body = URL_SAFE_NO_PAD + .decode(body_b64) + .map_err(|_| RequestStateError::InvalidEncoding)?; + let tag = URL_SAFE_NO_PAD + .decode(tag_b64) + .map_err(|_| RequestStateError::InvalidEncoding)?; + + Self::mac_v2(key, kid.as_bytes(), associated_data, &body) .verify_slice(&tag) .map_err(|_| RequestStateError::IntegrityCheckFailed)?; + Self::open_authenticated_body(body, now_ms) + } + + fn open_authenticated_body(body: Vec, now_ms: i64) -> Result, RequestStateError> { // The body is now authenticated, so its framing can be trusted. if body.len() < EXPIRY_LEN { return Err(RequestStateError::MalformedFormat); @@ -342,20 +730,45 @@ impl RequestStateCodec { Ok(body[EXPIRY_LEN..].to_vec()) } - /// Builds an HMAC keyed for request-state tags, pre-fed with the - /// domain-separation label, a length-prefixed `associated_data`, and the - /// body. The length prefix keeps the `associated_data`/`body` boundary - /// unambiguous so distinct inputs cannot collide. - fn mac_for(&self, associated_data: &[u8], body: &[u8]) -> HmacSha256 { - let mut mac = - HmacSha256::new_from_slice(&self.key).expect("HMAC accepts keys of any length"); - mac.update(DOMAIN); + fn mac_v1(key: &[u8], associated_data: &[u8], body: &[u8]) -> HmacSha256 { + let mut mac = HmacSha256::new_from_slice(key).expect("HMAC accepts keys of any length"); + mac.update(DOMAIN_V1); mac.update(&(associated_data.len() as u64).to_be_bytes()); mac.update(associated_data); mac.update(body); mac } + fn mac_v2(key: &[u8], kid: &[u8], associated_data: &[u8], body: &[u8]) -> HmacSha256 { + let mut mac = HmacSha256::new_from_slice(key).expect("HMAC accepts keys of any length"); + mac.update(DOMAIN_V2); + mac.update(&(kid.len() as u64).to_be_bytes()); + mac.update(kid); + mac.update(&(associated_data.len() as u64).to_be_bytes()); + mac.update(associated_data); + mac.update(body); + mac + } + + fn validate_config_kid(kid: &str) -> Result<(), RequestStateError> { + if kid.is_empty() { + return Err(RequestStateError::InvalidKeyring( + "key id must not be empty", + )); + } + if kid.len() > MAX_KID_LEN { + return Err(RequestStateError::InvalidKeyring( + "key id exceeds 255 UTF-8 bytes", + )); + } + Ok(()) + } + + // Base64url without padding encodes at most three bytes as four characters. + fn b64_len(len: usize) -> usize { + len.div_ceil(3) * 4 + } + fn now_ms() -> i64 { chrono::Utc::now().timestamp_millis() } @@ -574,4 +987,488 @@ mod tests { )); } } + + mod key_rotation { + use super::*; + + const KEY_A: &[u8] = b"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + const KEY_B: &[u8] = b"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; + const KEY_C: &[u8] = b"cccccccccccccccccccccccccccccccc"; + const KAT_KEY: &[u8] = b"0123456789abcdef0123456789abcdef"; + const KAT_KID: &str = "rotation-key-2026-08"; + const KAT_AD: &[u8] = b"user:alice|request:weather"; + const KAT_NOW_MS: i64 = 1_700_000_000_000; + + fn two_key_ring(active: &str) -> RequestStateCodec { + RequestStateCodec::keyring(active, [("a", KEY_A), ("b", KEY_B)]).unwrap() + } + + fn body(expiry: i64, payload: &[u8]) -> Vec { + let mut body = expiry.to_be_bytes().to_vec(); + body.extend_from_slice(payload); + body + } + + fn replace_segment(token: &str, index: usize, replacement: &str) -> String { + let mut parts: Vec = token.split('.').map(str::to_owned).collect(); + parts[index] = replacement.to_owned(); + parts.join(".") + } + + fn test_mac( + key: &[u8], + domain: &[u8], + kid: Option<&[u8]>, + associated_data: &[u8], + body: &[u8], + ) -> Vec { + let mut mac = HmacSha256::new_from_slice(key).expect("HMAC accepts keys of any length"); + mac.update(domain); + if let Some(kid) = kid { + mac.update(&(kid.len() as u64).to_be_bytes()); + mac.update(kid); + } + mac.update(&(associated_data.len() as u64).to_be_bytes()); + mac.update(associated_data); + mac.update(body); + mac.finalize().into_bytes().to_vec() + } + + fn raw_rs1(body: &[u8], tag: &[u8]) -> String { + format!( + "rs1.{}.{}", + URL_SAFE_NO_PAD.encode(body), + URL_SAFE_NO_PAD.encode(tag) + ) + } + + fn raw_rs2(kid: &[u8], body: &[u8], tag: &[u8]) -> String { + format!( + "rs2.{}.{}.{}", + URL_SAFE_NO_PAD.encode(kid), + URL_SAFE_NO_PAD.encode(body), + URL_SAFE_NO_PAD.encode(tag) + ) + } + + #[test] + fn rs1_known_answer_is_unchanged() { + // Independently checked with Python stdlib HMAC and Ruby OpenSSL. + let codec = RequestStateCodec::new(KAT_KEY); + let sealed = codec.seal_at( + b"step=2", + &SealOptions::new() + .associated_data(KAT_AD) + .ttl(Duration::from_secs(90)), + KAT_NOW_MS, + ); + assert_eq!( + sealed, + "rs1.AAABi8_mx5BzdGVwPTI.GQgS0X7mtSz8ZOy_kld2Zjuc4gAMGpBL74EghWI36IQ" + ); + } + + #[test] + fn rs2_known_answer_matches_wire_specification() { + // Independently checked with Python stdlib HMAC and Ruby OpenSSL. + let codec = RequestStateCodec::keyring(KAT_KID, [(KAT_KID, KAT_KEY)]).unwrap(); + let sealed = codec.seal_at( + b"step=2", + &SealOptions::new() + .associated_data(KAT_AD) + .ttl(Duration::from_secs(90)), + KAT_NOW_MS, + ); + assert_eq!( + sealed, + "rs2.cm90YXRpb24ta2V5LTIwMjYtMDg.AAABi8_mx5BzdGVwPTI.twv2acu7lKXqebyrmit-JrHHZm-BkKlBQEMMtL3lHk8" + ); + } + + #[test] + fn rs2_roundtrips_bytes_json_associated_data_and_ttl() { + let codec = two_key_ring("a"); + let options = SealOptions::new() + .associated_data(b"user:alice") + .ttl(Duration::from_secs(60)); + + let sealed = codec.seal_at(b"", &options, 1_000); + assert!(sealed.starts_with("rs2.YQ.")); + assert_eq!(codec.open_at(&sealed, b"user:alice", 30_000).unwrap(), b""); + assert!(matches!( + codec.open_at(&sealed, b"user:bob", 30_000), + Err(RequestStateError::IntegrityCheckFailed) + )); + assert!(matches!( + codec.open_at(&sealed, b"user:alice", 70_000), + Err(RequestStateError::Expired) + )); + + let value = serde_json::json!({ "step": 2, "tool": "weather" }); + let sealed = codec.seal_json(&value).unwrap(); + let opened: serde_json::Value = codec.open_json(&sealed).unwrap(); + assert_eq!(opened, value); + } + + #[test] + fn rotation_keeps_previous_rs2_key_verifiable() { + let signer_a = two_key_ring("a"); + let token_a = signer_a.seal(b"state-a"); + + let signer_b = two_key_ring("b"); + assert_eq!(signer_b.open(&token_a).unwrap(), b"state-a"); + let token_b = signer_b.seal(b"state-b"); + assert!(token_b.starts_with("rs2.Yg.")); + assert_eq!(signer_b.open(&token_b).unwrap(), b"state-b"); + + let without_a = RequestStateCodec::keyring("b", [("b", KEY_B)]).unwrap(); + assert!(matches!( + without_a.open(&token_a), + Err(RequestStateError::UnknownKeyId) + )); + } + + #[test] + fn rolling_migration_is_bidirectionally_compatible() { + let old = RequestStateCodec::new(KEY_A); + let transitional = RequestStateCodec::keyring("new", [("old", KEY_A), ("new", KEY_B)]) + .unwrap() + .with_rs1_signing("old") + .unwrap(); + let promoted = RequestStateCodec::keyring("new", [("old", KEY_A), ("new", KEY_B)]) + .unwrap() + .add_rs1_fallback("old") + .unwrap(); + let retired = RequestStateCodec::keyring("new", [("new", KEY_B)]).unwrap(); + + let old_token = old.seal(b"old"); + assert_eq!(transitional.open(&old_token).unwrap(), b"old"); + + let transitional_token = transitional.seal(b"transition"); + assert!(transitional_token.starts_with("rs1.")); + assert_eq!(old.open(&transitional_token).unwrap(), b"transition"); + assert_eq!(promoted.open(&transitional_token).unwrap(), b"transition"); + + let promoted_token = promoted.seal(b"promoted"); + assert!(promoted_token.starts_with("rs2.")); + assert_eq!(transitional.open(&promoted_token).unwrap(), b"promoted"); + assert_eq!(retired.open(&promoted_token).unwrap(), b"promoted"); + assert!(matches!( + retired.open(&old_token), + Err(RequestStateError::UnknownKeyId) + )); + } + + #[test] + fn multiple_legacy_fallbacks_are_supported() { + let legacy_a = RequestStateCodec::new(KEY_A).seal(b"a"); + let legacy_b = RequestStateCodec::new(KEY_B).seal(b"b"); + let legacy_c = RequestStateCodec::new(KEY_C).seal(b"c"); + let ring = + RequestStateCodec::keyring("new", [("a", KEY_A), ("b", KEY_B), ("new", KEY_C)]) + .unwrap() + .add_rs1_fallback("a") + .unwrap() + .add_rs1_fallback("b") + .unwrap(); + + assert_eq!(ring.open(&legacy_a).unwrap(), b"a"); + assert_eq!(ring.open(&legacy_b).unwrap(), b"b"); + assert!(matches!( + ring.open(&legacy_c), + Err(RequestStateError::IntegrityCheckFailed) + )); + } + + #[test] + fn kid_is_authenticated_even_when_ids_share_key_bytes() { + let codec = RequestStateCodec::keyring("a", [("a", KEY_A), ("b", KEY_A)]).unwrap(); + let sealed = codec.seal(b"state"); + let swapped = replace_segment(&sealed, 1, &URL_SAFE_NO_PAD.encode(b"b")); + assert!(matches!( + codec.open(&swapped), + Err(RequestStateError::IntegrityCheckFailed) + )); + } + + #[test] + fn independent_rs2_segment_tampering_is_rejected() { + let codec = two_key_ring("a"); + let sealed = codec.seal(b"state"); + + let swapped_kid = replace_segment(&sealed, 1, &URL_SAFE_NO_PAD.encode(b"b")); + let tampered_body = replace_segment(&sealed, 2, &URL_SAFE_NO_PAD.encode(b"changed")); + let tampered_tag = replace_segment(&sealed, 3, &URL_SAFE_NO_PAD.encode([0_u8; 32])); + + for tampered in [swapped_kid, tampered_body, tampered_tag] { + assert!(matches!( + codec.open(&tampered), + Err(RequestStateError::IntegrityCheckFailed) + )); + } + } + + #[test] + fn version_domains_are_cryptographically_separate() { + let token_body = body(0, b"state"); + + let wrong_v2_tag = test_mac(KEY_A, DOMAIN_V1, Some(b"a"), b"", &token_body); + let wrong_v2 = raw_rs2(b"a", &token_body, &wrong_v2_tag); + assert!(matches!( + two_key_ring("a").open(&wrong_v2), + Err(RequestStateError::IntegrityCheckFailed) + )); + + let wrong_v1_tag = test_mac(KEY_A, DOMAIN_V2, None, b"", &token_body); + let wrong_v1 = raw_rs1(&token_body, &wrong_v1_tag); + assert!(matches!( + RequestStateCodec::new(KEY_A).open(&wrong_v1), + Err(RequestStateError::IntegrityCheckFailed) + )); + } + + #[test] + fn constructor_invariants_are_enforced() { + let empty = RequestStateCodec::keyring("a", std::iter::empty::<(&str, &[u8])>()); + assert!(matches!(empty, Err(RequestStateError::InvalidKeyring(_)))); + + let duplicate = RequestStateCodec::keyring("a", [("a", KEY_A), ("a", KEY_B)]); + assert!(matches!( + duplicate, + Err(RequestStateError::InvalidKeyring(_)) + )); + + let empty_kid = RequestStateCodec::keyring("", [("", KEY_A)]); + assert!(matches!( + empty_kid, + Err(RequestStateError::InvalidKeyring(_)) + )); + + let oversized = "x".repeat(MAX_KID_LEN + 1); + let oversized_kid = RequestStateCodec::keyring(oversized.clone(), [(oversized, KEY_A)]); + assert!(matches!( + oversized_kid, + Err(RequestStateError::InvalidKeyring(_)) + )); + + let absent_active = RequestStateCodec::keyring("missing", [("a", KEY_A)]); + assert!(matches!( + absent_active, + Err(RequestStateError::InvalidKeyring(_)) + )); + + assert!(matches!( + RequestStateCodec::new(KEY_A).with_rs1_signing("a"), + Err(RequestStateError::InvalidKeyring(_)) + )); + assert!(matches!( + two_key_ring("a").with_rs1_signing("missing"), + Err(RequestStateError::InvalidKeyring(_)) + )); + assert!(matches!( + RequestStateCodec::new(KEY_A).add_rs1_fallback("a"), + Err(RequestStateError::InvalidKeyring(_)) + )); + assert!(matches!( + two_key_ring("a").add_rs1_fallback("missing"), + Err(RequestStateError::InvalidKeyring(_)) + )); + } + + #[test] + fn maximum_length_kid_roundtrips_and_oversized_wire_kid_is_rejected() { + let max_kid = "x".repeat(MAX_KID_LEN); + let codec = RequestStateCodec::keyring(max_kid.clone(), [(max_kid, KEY_A)]).unwrap(); + let token = codec.seal(b"state"); + assert_eq!(codec.open(&token).unwrap(), b"state"); + + let encoded_oversized = URL_SAFE_NO_PAD.encode("x".repeat(MAX_KID_LEN + 1)); + assert!(encoded_oversized.len() > MAX_ENCODED_KID_LEN); + let oversized = format!("rs2.{encoded_oversized}.!!!!.!!!!"); + assert!(matches!( + codec.open(&oversized), + Err(RequestStateError::InvalidKeyId) + )); + + let overlong_invalid_base64 = + format!("rs2.{}.!!!!.!!!!", "!".repeat(MAX_ENCODED_KID_LEN + 1)); + assert!(matches!( + codec.open(&overlong_invalid_base64), + Err(RequestStateError::InvalidKeyId) + )); + } + + #[test] + fn fallback_addition_is_idempotent_and_rs1_signing_adds_its_key() { + let codec = two_key_ring("b") + .add_rs1_fallback("a") + .unwrap() + .add_rs1_fallback("a") + .unwrap(); + match &codec.keys { + Keys::Ring { rs1_fallbacks, .. } => assert_eq!(rs1_fallbacks, &["a"]), + Keys::Single(_) => panic!("expected ring"), + } + + let transitional = two_key_ring("b").with_rs1_signing("a").unwrap(); + match &transitional.keys { + Keys::Ring { + seal_mode, + rs1_fallbacks, + .. + } => { + assert!(matches!( + seal_mode, + SealMode::Rs1 { key_id } if key_id == "a" + )); + assert_eq!(rs1_fallbacks, &["a"]); + } + Keys::Single(_) => panic!("expected ring"), + } + } + + #[test] + fn parser_is_strict_and_error_precedence_is_stable() { + let codec = two_key_ring("a"); + for malformed in [ + "rs2", + "rs2.YQ", + "rs2.YQ.body", + "rs2.YQ.body.tag.extra", + "rs3.YQ.body.tag", + ] { + assert!(matches!( + codec.open(malformed), + Err(RequestStateError::MalformedFormat) + )); + } + + assert!(matches!( + codec.open("rs2.!!!!.!!!!.!!!!"), + Err(RequestStateError::InvalidEncoding) + )); + assert!(matches!( + codec.open("rs2..!!!!.!!!!"), + Err(RequestStateError::InvalidKeyId) + )); + let non_utf8 = URL_SAFE_NO_PAD.encode([0xff]); + assert!(matches!( + codec.open(&format!("rs2.{non_utf8}.!!!!.!!!!")), + Err(RequestStateError::InvalidKeyId) + )); + + // Key selection precedes decoding later rs2 sections. + let unknown = URL_SAFE_NO_PAD.encode(b"missing"); + assert!(matches!( + codec.open(&format!("rs2.{unknown}.!!!!.!!!!")), + Err(RequestStateError::UnknownKeyId) + )); + assert!(matches!( + codec.open("rs2.YQ.!!!!.!!!!"), + Err(RequestStateError::InvalidEncoding) + )); + let valid_body = URL_SAFE_NO_PAD.encode(body(0, b"state")); + assert!(matches!( + codec.open(&format!("rs2.YQ.{valid_body}.!!!!")), + Err(RequestStateError::InvalidEncoding) + )); + assert!(matches!( + codec.open("rs2.YQ.."), + Err(RequestStateError::IntegrityCheckFailed) + )); + + let valid_rs2 = codec.seal(b"state"); + assert!(matches!( + RequestStateCodec::new(KEY_A).open(&valid_rs2), + Err(RequestStateError::UnknownKeyId) + )); + + // With no eligible rs1 key, selection fails before body/tag decode. + assert!(matches!( + codec.open("rs1.!!!!.!!!!"), + Err(RequestStateError::UnknownKeyId) + )); + let with_fallback = codec.add_rs1_fallback("a").unwrap(); + assert!(matches!( + with_fallback.open("rs1.!!!!.!!!!"), + Err(RequestStateError::InvalidEncoding) + )); + } + + #[test] + fn authenticated_short_body_is_malformed() { + let short_body = b"short"; + let tag = RequestStateCodec::mac_v2(KEY_A, b"a", b"", short_body) + .finalize() + .into_bytes(); + let token = raw_rs2(b"a", short_body, tag.as_slice()); + assert!(matches!( + two_key_ring("a").open(&token), + Err(RequestStateError::MalformedFormat) + )); + } + + #[test] + fn serde_is_reached_only_after_integrity_verification() { + let codec = two_key_ring("a"); + let invalid_json = codec.seal(b"{not-json"); + let opened: Result = codec.open_json(&invalid_json); + assert!(matches!(opened, Err(RequestStateError::Deserialization(_)))); + + let valid_json = codec.seal(b"{}"); + let tampered_body = body(0, b"{not-json"); + let tampered = replace_segment(&valid_json, 2, &URL_SAFE_NO_PAD.encode(tampered_body)); + let opened: Result = codec.open_json(&tampered); + assert!(matches!( + opened, + Err(RequestStateError::IntegrityCheckFailed) + )); + } + + #[test] + fn ring_debug_redacts_all_key_material() { + let codec = two_key_ring("b").add_rs1_fallback("a").unwrap(); + let rendered = format!("{codec:?}"); + assert!(!rendered.contains(std::str::from_utf8(KEY_A).unwrap())); + assert!(!rendered.contains(std::str::from_utf8(KEY_B).unwrap())); + assert!(rendered.contains("redacted")); + assert!(rendered.contains("Rs2")); + } + + #[test] + fn open_methods_do_not_panic_on_mutated_or_arbitrary_strings() { + let codec = two_key_ring("a").add_rs1_fallback("a").unwrap(); + let valid = [ + RequestStateCodec::new(KEY_A).seal(b"state"), + codec.seal(b"state"), + ]; + let mut corpus = vec![ + String::new(), + ".".to_owned(), + "...".to_owned(), + "rs1".to_owned(), + "rs2".to_owned(), + "💥".to_owned(), + "rs2.💥...".to_owned(), + ]; + + for token in valid { + for end in 0..=token.len() { + corpus.push(token[..end].to_owned()); + } + for index in 0..token.len() { + let mut bytes = token.as_bytes().to_vec(); + bytes[index] = b'!'; + corpus.push(String::from_utf8(bytes).expect("token is ASCII")); + } + } + + for candidate in corpus { + let _ = codec.open(&candidate); + let _ = codec.open_with(&candidate, b"context"); + let _: Result = codec.open_json(&candidate); + let _: Result = codec.open_json_with(&candidate, b"context"); + } + } + } } From c4cb4723ad914827b5f3ed081ce57d4a3c73a358 Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Tue, 4 Aug 2026 14:36:30 +0200 Subject: [PATCH 2/7] docs: streamline request-state codec documentation --- crates/rmcp/src/model/request_state.rs | 110 ++++++++----------------- 1 file changed, 36 insertions(+), 74 deletions(-) diff --git a/crates/rmcp/src/model/request_state.rs b/crates/rmcp/src/model/request_state.rs index 11f51888a..6e65c10a2 100644 --- a/crates/rmcp/src/model/request_state.rs +++ b/crates/rmcp/src/model/request_state.rs @@ -179,73 +179,41 @@ impl<'a> SealOptions<'a> { /// A keyed codec that seals and opens SEP-2322 `requestState` values with /// HMAC-SHA256 integrity protection. /// -/// [`new`](Self::new) preserves the legacy single-key `rs1` behavior. A codec -/// built with [`keyring`](Self::keyring) emits keyed `rs2` values and opens -/// them with one exact key lookup. [`with_rs1_signing`](Self::with_rs1_signing) -/// supports a rolling format migration by retaining `rs1` output while the -/// ring can already verify `rs2`; [`add_rs1_fallback`](Self::add_rs1_fallback) -/// retains explicitly selected keys for opening in-flight `rs1` values. +/// [`new`](Self::new) preserves `rs1`; [`keyring`](Self::keyring) emits `rs2`. +/// Use [`with_rs1_signing`](Self::with_rs1_signing) and +/// [`add_rs1_fallback`](Self::add_rs1_fallback) for rolling migrations. /// -/// Keys may be any length because HMAC internally normalizes them. For -/// meaningful security use high-entropy keys of at least 32 bytes and keep -/// them stable across every replica that may continue an MRTR exchange. +/// Use high-entropy keys of at least 32 bytes and configure the same keyring on +/// every replica that may continue an MRTR exchange. /// This codec provides integrity, not confidentiality: both the `rs2` key id /// and payload are base64url-readable by clients and are not encrypted. /// /// # Key rotation /// -/// Keyring codecs emit the keyed `rs2` format and select one verification key -/// from the token's authenticated key id. Migrating a rolling deployment from -/// the unkeyed `rs1` format takes two deployments so an upgraded replica never -/// emits a token that another serving replica cannot open: +/// Rotate in stages so every serving replica can open tokens emitted by peers: /// -/// 1. Deploy every replica with both keys and [`with_rs1_signing`](Self::with_rs1_signing). -/// The codec keeps emitting `rs1`, but can already open `rs2` with either -/// ring key. -/// 2. After every serving replica has that configuration, redeploy without -/// `with_rs1_signing`, select the new key for `rs2`, and retain the old key -/// through [`add_rs1_fallback`](Self::add_rs1_fallback). -/// 3. After that deployment finishes, wait longer than the maximum effective -/// lifetime of old request states before removing the old key and fallback. +/// 1. Deploy both keys everywhere while continuing to sign `rs1` with the old key. +/// 2. Activate the new key for `rs2`, retaining the old key as an `rs1` fallback. +/// 3. Wait out the maximum request-state lifetime, then remove the old key. /// /// ``` -/// use rmcp::model::{RequestStateCodec, RequestStateError}; -/// +/// # use rmcp::model::{RequestStateCodec, RequestStateError}; /// # fn configure() -> Result<(), RequestStateError> { -/// let old_key = b"old-request-state-key-at-least-32b"; -/// let new_key = b"new-request-state-key-at-least-32b"; -/// -/// // First deployment: publish rs2 support and the new key, but emit rs1. -/// let transitional = RequestStateCodec::keyring( -/// "new", -/// [("old", old_key.as_slice()), ("new", new_key.as_slice())], -/// )? -/// .with_rs1_signing("old")?; -/// assert!(transitional.seal(b"state").starts_with("rs1.")); -/// -/// // Second deployment: emit rs2/new while accepting in-flight rs1/old. -/// let promoted = RequestStateCodec::keyring( -/// "new", -/// [("old", old_key.as_slice()), ("new", new_key.as_slice())], -/// )? -/// .add_rs1_fallback("old")?; -/// assert!(promoted.seal(b"state").starts_with("rs2.")); +/// # let old = b"old-request-state-key-at-least-32b".as_slice(); +/// # let new = b"new-request-state-key-at-least-32b".as_slice(); +/// # let keys = [("old", old), ("new", new)]; +/// let transitional = +/// RequestStateCodec::keyring("new", keys)?.with_rs1_signing("old")?; +/// let promoted = RequestStateCodec::keyring("new", keys)?.add_rs1_fallback("old")?; /// # Ok(()) /// # } /// # configure().unwrap(); /// ``` /// -/// The codec defaults to no TTL. If old states have no other bounded effective -/// lifetime, their verification key cannot be retired safely. Runtime keyring -/// replacement is not built in; rebuild and redeploy the codec when keys -/// change. -/// -/// If more than one legacy `rs1` key is still valid, add each with -/// [`add_rs1_fallback`](Self::add_rs1_fallback); trial verification is -/// unavoidable because `rs1` carries no kid. For later `rs2`-to-`rs2` -/// rotations, first deploy every replica with both keys while the old kid -/// remains selected, then promote the new kid, wait for old states to drain, -/// and finally remove the old key. +/// For later `rs2` rotations, deploy both keys with the old key active, promote +/// the new key, wait for old states to drain, and then remove the old key. +/// The codec has no default TTL or runtime key reload. Without a bounded state +/// lifetime, an old verification key cannot be retired safely. #[derive(Clone)] pub struct RequestStateCodec { keys: Keys, @@ -301,11 +269,11 @@ impl RequestStateCodec { } /// Creates a keyring codec that seals `rs2` with `active_kid` and opens - /// `rs2` by exact key-id lookup. + /// `rs2` values for configured key ids. /// - /// Ring codecs do not open legacy `rs1` values unless a fallback is added - /// with [`add_rs1_fallback`](Self::add_rs1_fallback), or transitional - /// `rs1` sealing is enabled with + /// Keyring codecs do not open legacy `rs1` values unless a fallback is + /// added with [`add_rs1_fallback`](Self::add_rs1_fallback), or transitional + /// signing is enabled with /// [`with_rs1_signing`](Self::with_rs1_signing). /// /// Key ids are opaque, case-sensitive UTF-8 strings of 1 to 255 bytes. @@ -359,16 +327,16 @@ impl RequestStateCodec { }) } - /// Switches a Ring into transitional mode that seals `rs1` with `kid`. + /// Switches a keyring codec to transitional `rs1` signing with `kid`. /// /// The selected key is also added to the legacy `rs1` fallback set, so the - /// resulting codec can open its own output. The Ring can still open `rs2` - /// tokens for every key id it contains. + /// codec can open its own output while continuing to accept `rs2` values. /// /// # Errors /// - /// Returns [`RequestStateError::InvalidKeyring`] when called on a codec - /// created by [`new`](Self::new), or if the Ring does not contain `kid`. + /// Returns [`RequestStateError::InvalidKeyring`] when called on a + /// single-key codec created by [`new`](Self::new), or if the keyring does + /// not contain `kid`. pub fn with_rs1_signing(mut self, kid: impl AsRef) -> Result { let kid = kid.as_ref(); match &mut self.keys { @@ -398,16 +366,15 @@ impl RequestStateCodec { } } - /// Adds `kid` to the keys tried when opening legacy `rs1` values. + /// Adds `kid` as an accepted key for legacy `rs1` values. /// - /// Because `rs1` carries no key id, multiple fallbacks require one HMAC per - /// configured key until a match is found. Adding the same id more than once - /// is idempotent. `rs2` verification always uses one exact lookup. + /// Adding the same id more than once has no effect. /// /// # Errors /// - /// Returns [`RequestStateError::InvalidKeyring`] when called on a codec - /// created by [`new`](Self::new), or if the Ring does not contain `kid`. + /// Returns [`RequestStateError::InvalidKeyring`] when called on a + /// single-key codec created by [`new`](Self::new), or if the keyring does + /// not contain `kid`. pub fn add_rs1_fallback(mut self, kid: impl AsRef) -> Result { let kid = kid.as_ref(); match &mut self.keys { @@ -498,13 +465,8 @@ impl RequestStateCodec { /// - [`RequestStateError::InvalidKeyId`] if an `rs2` key id is empty, too /// long, or not valid UTF-8. /// - /// Version and segment-count validation happens first. For `rs2`, kid - /// validation and key selection happen before body/tag decoding, so an - /// unknown kid takes precedence over errors in later sections. A Ring with - /// no `rs1` fallbacks likewise returns `UnknownKeyId` after validating the - /// three-segment `rs1` structure and before decoding its body or tag. - /// Applications should expose a common client-facing error for every open - /// failure and reserve the detailed variants for internal diagnostics. + /// Callers should expose a single client-facing failure and reserve the + /// detailed variants for internal diagnostics. pub fn open_with( &self, sealed: &str, From aa63750f61a7deea021c09100f32b1d6e0509ea5 Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Tue, 4 Aug 2026 15:12:09 +0200 Subject: [PATCH 3/7] fix: harden request-state fallback verification --- crates/rmcp/src/model/request_state.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/crates/rmcp/src/model/request_state.rs b/crates/rmcp/src/model/request_state.rs index 6e65c10a2..36687bb86 100644 --- a/crates/rmcp/src/model/request_state.rs +++ b/crates/rmcp/src/model/request_state.rs @@ -465,8 +465,9 @@ impl RequestStateCodec { /// - [`RequestStateError::InvalidKeyId`] if an `rs2` key id is empty, too /// long, or not valid UTF-8. /// - /// Callers should expose a single client-facing failure and reserve the - /// detailed variants for internal diagnostics. + /// Applications MUST map all token-opening failures to a single + /// client-facing error and reserve the detailed variants for internal + /// diagnostics. pub fn open_with( &self, sealed: &str, @@ -610,6 +611,7 @@ impl RequestStateCodec { .decode(tag_b64) .map_err(|_| RequestStateError::InvalidEncoding)?; + // `verify_slice` compares tags in constant time and rejects wrong-length tags. match &self.keys { Keys::Single(key) => Self::mac_v1(key, associated_data, &body) .verify_slice(&tag) @@ -619,12 +621,15 @@ impl RequestStateCodec { rs1_fallbacks, .. } => { - let verified = rs1_fallbacks.iter().any(|kid| { + // Evaluate every fallback so the HMAC count does not reveal which key matched. + let mut verified = false; + for kid in rs1_fallbacks { let key = keys.get(kid).expect("validated rs1 fallback key"); - Self::mac_v1(key, associated_data, &body) + let matches = Self::mac_v1(key, associated_data, &body) .verify_slice(&tag) - .is_ok() - }); + .is_ok(); + verified |= matches; + } if !verified { return Err(RequestStateError::IntegrityCheckFailed); } @@ -672,6 +677,7 @@ impl RequestStateCodec { .decode(tag_b64) .map_err(|_| RequestStateError::InvalidEncoding)?; + // `verify_slice` compares tags in constant time and rejects wrong-length tags. Self::mac_v2(key, kid.as_bytes(), associated_data, &body) .verify_slice(&tag) .map_err(|_| RequestStateError::IntegrityCheckFailed)?; @@ -809,10 +815,10 @@ mod tests { } #[test] - fn wrong_version_prefix_is_malformed() { + fn unsupported_version_is_malformed() { let codec = RequestStateCodec::new(b"key".to_vec()); let sealed = codec.seal(b"state"); - let bumped = sealed.replacen("rs1.", "rs2.", 1); + let bumped = sealed.replacen("rs1.", "rs3.", 1); assert!(matches!( codec.open(&bumped), Err(RequestStateError::MalformedFormat) From fddea390a411dd89936195ae59764c9bcab7ea7d Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Tue, 4 Aug 2026 16:07:57 +0200 Subject: [PATCH 4/7] refactor: refine request-state keyring API --- crates/rmcp/src/model/request_state.rs | 114 +++++++++++++++---------- 1 file changed, 70 insertions(+), 44 deletions(-) diff --git a/crates/rmcp/src/model/request_state.rs b/crates/rmcp/src/model/request_state.rs index 36687bb86..7ab786279 100644 --- a/crates/rmcp/src/model/request_state.rs +++ b/crates/rmcp/src/model/request_state.rs @@ -128,6 +128,7 @@ pub enum RequestStateError { InvalidKeyId, /// A keyring constructor or builder was given an invalid configuration. + /// The message is for diagnostics and should not be matched programmatically. #[error("invalid keyring configuration: {0}")] InvalidKeyring(&'static str), @@ -179,15 +180,25 @@ impl<'a> SealOptions<'a> { /// A keyed codec that seals and opens SEP-2322 `requestState` values with /// HMAC-SHA256 integrity protection. /// -/// [`new`](Self::new) preserves `rs1`; [`keyring`](Self::keyring) emits `rs2`. +/// [`new`](Self::new) preserves `rs1`; +/// [`new_with_keyring`](Self::new_with_keyring) emits `rs2`. /// Use [`with_rs1_signing`](Self::with_rs1_signing) and -/// [`add_rs1_fallback`](Self::add_rs1_fallback) for rolling migrations. +/// [`with_rs1_fallback`](Self::with_rs1_fallback) for rolling migrations. /// /// Use high-entropy keys of at least 32 bytes and configure the same keyring on /// every replica that may continue an MRTR exchange. /// This codec provides integrity, not confidentiality: both the `rs2` key id /// and payload are base64url-readable by clients and are not encrypted. /// +/// # Wire format +/// +/// Keyring codecs emit +/// `rs2...`. +/// The expiry is a signed big-endian Unix timestamp in milliseconds, or zero +/// for no expiry. The tag authenticates the key id, associated data, and body +/// under an `rs2`-specific domain. Key ids are visible, authenticated, +/// case-sensitive UTF-8 strings; they are not confidential. +/// /// # Key rotation /// /// Rotate in stages so every serving replica can open tokens emitted by peers: @@ -203,8 +214,9 @@ impl<'a> SealOptions<'a> { /// # let new = b"new-request-state-key-at-least-32b".as_slice(); /// # let keys = [("old", old), ("new", new)]; /// let transitional = -/// RequestStateCodec::keyring("new", keys)?.with_rs1_signing("old")?; -/// let promoted = RequestStateCodec::keyring("new", keys)?.add_rs1_fallback("old")?; +/// RequestStateCodec::new_with_keyring("new", keys)?.with_rs1_signing("old")?; +/// let promoted = +/// RequestStateCodec::new_with_keyring("new", keys)?.with_rs1_fallback("old")?; /// # Ok(()) /// # } /// # configure().unwrap(); @@ -268,12 +280,15 @@ impl RequestStateCodec { } } - /// Creates a keyring codec that seals `rs2` with `active_kid` and opens - /// `rs2` values for configured key ids. + /// Creates a keyring codec that seals `rs2` with `active_kid`. + /// + /// Opening an `rs2` value selects the named key from all configured keys; + /// successful tag verification authenticates that key id. `active_kid` + /// affects sealing only. /// /// Keyring codecs do not open legacy `rs1` values unless a fallback is - /// added with [`add_rs1_fallback`](Self::add_rs1_fallback), or transitional - /// signing is enabled with + /// added with [`with_rs1_fallback`](Self::with_rs1_fallback), or + /// transitional signing is enabled with /// [`with_rs1_signing`](Self::with_rs1_signing). /// /// Key ids are opaque, case-sensitive UTF-8 strings of 1 to 255 bytes. @@ -282,7 +297,7 @@ impl RequestStateCodec { /// /// Returns [`RequestStateError::InvalidKeyring`] if the keyring is empty, /// contains duplicate or invalid key ids, or does not contain `active_kid`. - pub fn keyring( + pub fn new_with_keyring( active_kid: impl Into, keys: impl IntoIterator, ) -> Result @@ -370,12 +385,15 @@ impl RequestStateCodec { /// /// Adding the same id more than once has no effect. /// + /// Opening `rs1` evaluates every configured fallback before returning, so + /// keep the fallback set small and remove it after old values have drained. + /// /// # Errors /// /// Returns [`RequestStateError::InvalidKeyring`] when called on a /// single-key codec created by [`new`](Self::new), or if the keyring does /// not contain `kid`. - pub fn add_rs1_fallback(mut self, kid: impl AsRef) -> Result { + pub fn with_rs1_fallback(mut self, kid: impl AsRef) -> Result { let kid = kid.as_ref(); match &mut self.keys { Keys::Single(_) => Err(RequestStateError::InvalidKeyring( @@ -968,7 +986,7 @@ mod tests { const KAT_NOW_MS: i64 = 1_700_000_000_000; fn two_key_ring(active: &str) -> RequestStateCodec { - RequestStateCodec::keyring(active, [("a", KEY_A), ("b", KEY_B)]).unwrap() + RequestStateCodec::new_with_keyring(active, [("a", KEY_A), ("b", KEY_B)]).unwrap() } fn body(expiry: i64, payload: &[u8]) -> Vec { @@ -1039,7 +1057,7 @@ mod tests { #[test] fn rs2_known_answer_matches_wire_specification() { // Independently checked with Python stdlib HMAC and Ruby OpenSSL. - let codec = RequestStateCodec::keyring(KAT_KID, [(KAT_KID, KAT_KEY)]).unwrap(); + let codec = RequestStateCodec::new_with_keyring(KAT_KID, [(KAT_KID, KAT_KEY)]).unwrap(); let sealed = codec.seal_at( b"step=2", &SealOptions::new() @@ -1089,7 +1107,7 @@ mod tests { assert!(token_b.starts_with("rs2.Yg.")); assert_eq!(signer_b.open(&token_b).unwrap(), b"state-b"); - let without_a = RequestStateCodec::keyring("b", [("b", KEY_B)]).unwrap(); + let without_a = RequestStateCodec::new_with_keyring("b", [("b", KEY_B)]).unwrap(); assert!(matches!( without_a.open(&token_a), Err(RequestStateError::UnknownKeyId) @@ -1099,15 +1117,17 @@ mod tests { #[test] fn rolling_migration_is_bidirectionally_compatible() { let old = RequestStateCodec::new(KEY_A); - let transitional = RequestStateCodec::keyring("new", [("old", KEY_A), ("new", KEY_B)]) - .unwrap() - .with_rs1_signing("old") - .unwrap(); - let promoted = RequestStateCodec::keyring("new", [("old", KEY_A), ("new", KEY_B)]) - .unwrap() - .add_rs1_fallback("old") - .unwrap(); - let retired = RequestStateCodec::keyring("new", [("new", KEY_B)]).unwrap(); + let transitional = + RequestStateCodec::new_with_keyring("new", [("old", KEY_A), ("new", KEY_B)]) + .unwrap() + .with_rs1_signing("old") + .unwrap(); + let promoted = + RequestStateCodec::new_with_keyring("new", [("old", KEY_A), ("new", KEY_B)]) + .unwrap() + .with_rs1_fallback("old") + .unwrap(); + let retired = RequestStateCodec::new_with_keyring("new", [("new", KEY_B)]).unwrap(); let old_token = old.seal(b"old"); assert_eq!(transitional.open(&old_token).unwrap(), b"old"); @@ -1132,13 +1152,15 @@ mod tests { let legacy_a = RequestStateCodec::new(KEY_A).seal(b"a"); let legacy_b = RequestStateCodec::new(KEY_B).seal(b"b"); let legacy_c = RequestStateCodec::new(KEY_C).seal(b"c"); - let ring = - RequestStateCodec::keyring("new", [("a", KEY_A), ("b", KEY_B), ("new", KEY_C)]) - .unwrap() - .add_rs1_fallback("a") - .unwrap() - .add_rs1_fallback("b") - .unwrap(); + let ring = RequestStateCodec::new_with_keyring( + "new", + [("a", KEY_A), ("b", KEY_B), ("new", KEY_C)], + ) + .unwrap() + .with_rs1_fallback("a") + .unwrap() + .with_rs1_fallback("b") + .unwrap(); assert_eq!(ring.open(&legacy_a).unwrap(), b"a"); assert_eq!(ring.open(&legacy_b).unwrap(), b"b"); @@ -1150,7 +1172,8 @@ mod tests { #[test] fn kid_is_authenticated_even_when_ids_share_key_bytes() { - let codec = RequestStateCodec::keyring("a", [("a", KEY_A), ("b", KEY_A)]).unwrap(); + let codec = + RequestStateCodec::new_with_keyring("a", [("a", KEY_A), ("b", KEY_A)]).unwrap(); let sealed = codec.seal(b"state"); let swapped = replace_segment(&sealed, 1, &URL_SAFE_NO_PAD.encode(b"b")); assert!(matches!( @@ -1197,29 +1220,31 @@ mod tests { #[test] fn constructor_invariants_are_enforced() { - let empty = RequestStateCodec::keyring("a", std::iter::empty::<(&str, &[u8])>()); + let empty = + RequestStateCodec::new_with_keyring("a", std::iter::empty::<(&str, &[u8])>()); assert!(matches!(empty, Err(RequestStateError::InvalidKeyring(_)))); - let duplicate = RequestStateCodec::keyring("a", [("a", KEY_A), ("a", KEY_B)]); + let duplicate = RequestStateCodec::new_with_keyring("a", [("a", KEY_A), ("a", KEY_B)]); assert!(matches!( duplicate, Err(RequestStateError::InvalidKeyring(_)) )); - let empty_kid = RequestStateCodec::keyring("", [("", KEY_A)]); + let empty_kid = RequestStateCodec::new_with_keyring("", [("", KEY_A)]); assert!(matches!( empty_kid, Err(RequestStateError::InvalidKeyring(_)) )); let oversized = "x".repeat(MAX_KID_LEN + 1); - let oversized_kid = RequestStateCodec::keyring(oversized.clone(), [(oversized, KEY_A)]); + let oversized_kid = + RequestStateCodec::new_with_keyring(oversized.clone(), [(oversized, KEY_A)]); assert!(matches!( oversized_kid, Err(RequestStateError::InvalidKeyring(_)) )); - let absent_active = RequestStateCodec::keyring("missing", [("a", KEY_A)]); + let absent_active = RequestStateCodec::new_with_keyring("missing", [("a", KEY_A)]); assert!(matches!( absent_active, Err(RequestStateError::InvalidKeyring(_)) @@ -1234,11 +1259,11 @@ mod tests { Err(RequestStateError::InvalidKeyring(_)) )); assert!(matches!( - RequestStateCodec::new(KEY_A).add_rs1_fallback("a"), + RequestStateCodec::new(KEY_A).with_rs1_fallback("a"), Err(RequestStateError::InvalidKeyring(_)) )); assert!(matches!( - two_key_ring("a").add_rs1_fallback("missing"), + two_key_ring("a").with_rs1_fallback("missing"), Err(RequestStateError::InvalidKeyring(_)) )); } @@ -1246,7 +1271,8 @@ mod tests { #[test] fn maximum_length_kid_roundtrips_and_oversized_wire_kid_is_rejected() { let max_kid = "x".repeat(MAX_KID_LEN); - let codec = RequestStateCodec::keyring(max_kid.clone(), [(max_kid, KEY_A)]).unwrap(); + let codec = + RequestStateCodec::new_with_keyring(max_kid.clone(), [(max_kid, KEY_A)]).unwrap(); let token = codec.seal(b"state"); assert_eq!(codec.open(&token).unwrap(), b"state"); @@ -1267,11 +1293,11 @@ mod tests { } #[test] - fn fallback_addition_is_idempotent_and_rs1_signing_adds_its_key() { + fn rs1_fallback_is_idempotent_and_rs1_signing_adds_its_key() { let codec = two_key_ring("b") - .add_rs1_fallback("a") + .with_rs1_fallback("a") .unwrap() - .add_rs1_fallback("a") + .with_rs1_fallback("a") .unwrap(); match &codec.keys { Keys::Ring { rs1_fallbacks, .. } => assert_eq!(rs1_fallbacks, &["a"]), @@ -1356,7 +1382,7 @@ mod tests { codec.open("rs1.!!!!.!!!!"), Err(RequestStateError::UnknownKeyId) )); - let with_fallback = codec.add_rs1_fallback("a").unwrap(); + let with_fallback = codec.with_rs1_fallback("a").unwrap(); assert!(matches!( with_fallback.open("rs1.!!!!.!!!!"), Err(RequestStateError::InvalidEncoding) @@ -1395,7 +1421,7 @@ mod tests { #[test] fn ring_debug_redacts_all_key_material() { - let codec = two_key_ring("b").add_rs1_fallback("a").unwrap(); + let codec = two_key_ring("b").with_rs1_fallback("a").unwrap(); let rendered = format!("{codec:?}"); assert!(!rendered.contains(std::str::from_utf8(KEY_A).unwrap())); assert!(!rendered.contains(std::str::from_utf8(KEY_B).unwrap())); @@ -1405,7 +1431,7 @@ mod tests { #[test] fn open_methods_do_not_panic_on_mutated_or_arbitrary_strings() { - let codec = two_key_ring("a").add_rs1_fallback("a").unwrap(); + let codec = two_key_ring("a").with_rs1_fallback("a").unwrap(); let valid = [ RequestStateCodec::new(KEY_A).seal(b"state"), codec.seal(b"state"), From 2ae21fc42b606fa484024232b60d4d7b250d153c Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Tue, 4 Aug 2026 16:17:12 +0200 Subject: [PATCH 5/7] docs: make request-state rotation guidance self-contained --- README.md | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index b1c5ea77e..b4a12c38d 100644 --- a/README.md +++ b/README.md @@ -1363,16 +1363,21 @@ async fn call_tool(&self, request: CallToolRequestParams, _ctx: RequestContext **`requestState` is untrusted.** The client echoes it back verbatim, so a -> stateless server that stores meaningful data in it MUST verify integrity -> first. Enable the `request-state` feature and use `RequestStateCodec` to seal -> and open it (HMAC-tagged), or keep state server-side and use `requestState` -> only as an opaque handle. - -For multi-replica deployments, use a `RequestStateCodec` keyring to rotate -signing keys without invalidating in-flight requests. Follow the -[`RequestStateCodec` key-rotation rustdocs](https://docs.rs/rmcp/latest/rmcp/model/struct.RequestStateCodec.html#key-rotation) -for the rolling-safe `rs1`-to-`rs2` migration and key-retirement procedure. +> **`requestState` is untrusted.** [SEP-2322 requires servers to validate +> it](https://modelcontextprotocol.io/seps/2322-MRTR#protocol-requirements-for-ephemeral-workflow) +> because the client echoes it back verbatim. A stateless server that stores +> meaningful data in it MUST verify integrity first. Enable the `request-state` +> feature and use `RequestStateCodec` to seal and open it (HMAC-tagged), or keep +> state server-side and use `requestState` only as an opaque handle. + +For multi-replica deployments, use `RequestStateCodec::new_with_keyring` to +rotate signing keys without invalidating in-flight requests: + +1. Deploy the old and new keys everywhere, continuing to emit `rs1` with the + old key via `with_rs1_signing("old")`. +2. Start emitting `rs2` with the new key while retaining the old key via + `with_rs1_fallback("old")`. +3. After the maximum `requestState` lifetime has elapsed, remove the old key. ### Client-side From 0c6b046b306df489589a1ff8a1f73c0c3353b464 Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Tue, 4 Aug 2026 16:24:04 +0200 Subject: [PATCH 6/7] docs: streamline request-state keyring rustdocs --- crates/rmcp/src/model/request_state.rs | 102 +++++-------------------- 1 file changed, 20 insertions(+), 82 deletions(-) diff --git a/crates/rmcp/src/model/request_state.rs b/crates/rmcp/src/model/request_state.rs index 7ab786279..d7fdd236d 100644 --- a/crates/rmcp/src/model/request_state.rs +++ b/crates/rmcp/src/model/request_state.rs @@ -73,10 +73,8 @@ use thiserror::Error; type HmacSha256 = Hmac; -/// Legacy version tag used by single-key and transitional codecs. const VERSION_V1: &str = "rs1"; -/// Keyed version tag used by keyring codecs after promotion. const VERSION_V2: &str = "rs2"; /// Domain-separation label mixed into the HMAC so a `requestState` tag can never @@ -90,7 +88,6 @@ const DOMAIN_V2: &[u8] = b"rmcp/mrtr/request-state/v2"; /// front of every sealed body. `0` means "no expiry". const EXPIRY_LEN: usize = 8; -/// Maximum decoded UTF-8 byte length of an rs2 key id. const MAX_KID_LEN: usize = 255; /// Maximum unpadded base64url length for [`MAX_KID_LEN`] bytes. @@ -127,8 +124,7 @@ pub enum RequestStateError { #[error("request state contains an invalid key identifier")] InvalidKeyId, - /// A keyring constructor or builder was given an invalid configuration. - /// The message is for diagnostics and should not be matched programmatically. + /// An invalid keyring configuration; the message is diagnostic only. #[error("invalid keyring configuration: {0}")] InvalidKeyring(&'static str), @@ -177,55 +173,16 @@ impl<'a> SealOptions<'a> { } } -/// A keyed codec that seals and opens SEP-2322 `requestState` values with -/// HMAC-SHA256 integrity protection. +/// A keyed codec for integrity-protected SEP-2322 `requestState` values. /// -/// [`new`](Self::new) preserves `rs1`; -/// [`new_with_keyring`](Self::new_with_keyring) emits `rs2`. +/// [`new`](Self::new) preserves `rs1`; [`new_with_keyring`](Self::new_with_keyring) +/// emits `rs2`. /// Use [`with_rs1_signing`](Self::with_rs1_signing) and /// [`with_rs1_fallback`](Self::with_rs1_fallback) for rolling migrations. /// -/// Use high-entropy keys of at least 32 bytes and configure the same keyring on -/// every replica that may continue an MRTR exchange. -/// This codec provides integrity, not confidentiality: both the `rs2` key id -/// and payload are base64url-readable by clients and are not encrypted. -/// -/// # Wire format -/// -/// Keyring codecs emit -/// `rs2...`. -/// The expiry is a signed big-endian Unix timestamp in milliseconds, or zero -/// for no expiry. The tag authenticates the key id, associated data, and body -/// under an `rs2`-specific domain. Key ids are visible, authenticated, -/// case-sensitive UTF-8 strings; they are not confidential. -/// -/// # Key rotation -/// -/// Rotate in stages so every serving replica can open tokens emitted by peers: -/// -/// 1. Deploy both keys everywhere while continuing to sign `rs1` with the old key. -/// 2. Activate the new key for `rs2`, retaining the old key as an `rs1` fallback. -/// 3. Wait out the maximum request-state lifetime, then remove the old key. -/// -/// ``` -/// # use rmcp::model::{RequestStateCodec, RequestStateError}; -/// # fn configure() -> Result<(), RequestStateError> { -/// # let old = b"old-request-state-key-at-least-32b".as_slice(); -/// # let new = b"new-request-state-key-at-least-32b".as_slice(); -/// # let keys = [("old", old), ("new", new)]; -/// let transitional = -/// RequestStateCodec::new_with_keyring("new", keys)?.with_rs1_signing("old")?; -/// let promoted = -/// RequestStateCodec::new_with_keyring("new", keys)?.with_rs1_fallback("old")?; -/// # Ok(()) -/// # } -/// # configure().unwrap(); -/// ``` -/// -/// For later `rs2` rotations, deploy both keys with the old key active, promote -/// the new key, wait for old states to drain, and then remove the old key. -/// The codec has no default TTL or runtime key reload. Without a bounded state -/// lifetime, an old verification key cannot be retired safely. +/// Configure the same high-entropy keys of at least 32 bytes on every replica. +/// Values are authenticated, not encrypted; key ids and payloads remain +/// readable. Retain old keys until every value they signed has expired. #[derive(Clone)] pub struct RequestStateCodec { keys: Keys, @@ -282,16 +239,10 @@ impl RequestStateCodec { /// Creates a keyring codec that seals `rs2` with `active_kid`. /// - /// Opening an `rs2` value selects the named key from all configured keys; - /// successful tag verification authenticates that key id. `active_kid` - /// affects sealing only. - /// - /// Keyring codecs do not open legacy `rs1` values unless a fallback is - /// added with [`with_rs1_fallback`](Self::with_rs1_fallback), or - /// transitional signing is enabled with + /// All configured keys can open matching `rs2` values. Legacy `rs1` values + /// require [`with_rs1_fallback`](Self::with_rs1_fallback) or /// [`with_rs1_signing`](Self::with_rs1_signing). - /// - /// Key ids are opaque, case-sensitive UTF-8 strings of 1 to 255 bytes. + /// Key ids are case-sensitive UTF-8 strings of 1 to 255 bytes. /// /// # Errors /// @@ -342,10 +293,8 @@ impl RequestStateCodec { }) } - /// Switches a keyring codec to transitional `rs1` signing with `kid`. - /// - /// The selected key is also added to the legacy `rs1` fallback set, so the - /// codec can open its own output while continuing to accept `rs2` values. + /// Uses `kid` to sign legacy `rs1` values during a rolling migration. + /// The codec accepts its own `rs1` output and configured `rs2` values. /// /// # Errors /// @@ -381,12 +330,9 @@ impl RequestStateCodec { } } - /// Adds `kid` as an accepted key for legacy `rs1` values. - /// - /// Adding the same id more than once has no effect. - /// - /// Opening `rs1` evaluates every configured fallback before returning, so - /// keep the fallback set small and remove it after old values have drained. + /// Accepts legacy `rs1` values signed with `kid`. + /// Adding the same id more than once has no effect; remove fallbacks after + /// old values have drained. /// /// # Errors /// @@ -472,16 +418,8 @@ impl RequestStateCodec { /// /// # Errors /// - /// - [`RequestStateError::UnknownKeyId`] if the codec cannot select a key - /// for a structurally valid value. - /// - [`RequestStateError::IntegrityCheckFailed`] if the tag does not match - /// the selected key or the associated data differs. - /// - [`RequestStateError::Expired`] if the value's TTL has elapsed. - /// - [`RequestStateError::MalformedFormat`] or - /// [`RequestStateError::InvalidEncoding`] if it is not a well-formed sealed - /// value. - /// - [`RequestStateError::InvalidKeyId`] if an `rs2` key id is empty, too - /// long, or not valid UTF-8. + /// Returns a [`RequestStateError`] if the value is malformed, cannot be + /// verified, names an unavailable key, or has expired. /// /// Applications MUST map all token-opening failures to a single /// client-facing error and reserve the detailed variants for internal @@ -629,7 +567,7 @@ impl RequestStateCodec { .decode(tag_b64) .map_err(|_| RequestStateError::InvalidEncoding)?; - // `verify_slice` compares tags in constant time and rejects wrong-length tags. + // Authentication must not short-circuit based on tag bytes. match &self.keys { Keys::Single(key) => Self::mac_v1(key, associated_data, &body) .verify_slice(&tag) @@ -639,7 +577,7 @@ impl RequestStateCodec { rs1_fallbacks, .. } => { - // Evaluate every fallback so the HMAC count does not reveal which key matched. + // Try every fallback so timing does not identify the matching key. let mut verified = false; for kid in rs1_fallbacks { let key = keys.get(kid).expect("validated rs1 fallback key"); @@ -695,7 +633,7 @@ impl RequestStateCodec { .decode(tag_b64) .map_err(|_| RequestStateError::InvalidEncoding)?; - // `verify_slice` compares tags in constant time and rejects wrong-length tags. + // Authentication must not short-circuit based on tag bytes. Self::mac_v2(key, kid.as_bytes(), associated_data, &body) .verify_slice(&tag) .map_err(|_| RequestStateError::IntegrityCheckFailed)?; From 19262c4ee2795dd13ccbb9d92e7dc54b6a0a526b Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Tue, 4 Aug 2026 16:33:59 +0200 Subject: [PATCH 7/7] test: streamline request-state keyring coverage --- crates/rmcp/src/model/request_state.rs | 87 +++++--------------------- 1 file changed, 14 insertions(+), 73 deletions(-) diff --git a/crates/rmcp/src/model/request_state.rs b/crates/rmcp/src/model/request_state.rs index d7fdd236d..bd07037e3 100644 --- a/crates/rmcp/src/model/request_state.rs +++ b/crates/rmcp/src/model/request_state.rs @@ -1010,15 +1010,18 @@ mod tests { } #[test] - fn rs2_roundtrips_bytes_json_associated_data_and_ttl() { + fn rs2_roundtrips_with_associated_data_and_ttl() { let codec = two_key_ring("a"); let options = SealOptions::new() .associated_data(b"user:alice") .ttl(Duration::from_secs(60)); - let sealed = codec.seal_at(b"", &options, 1_000); + let sealed = codec.seal_at(b"state", &options, 1_000); assert!(sealed.starts_with("rs2.YQ.")); - assert_eq!(codec.open_at(&sealed, b"user:alice", 30_000).unwrap(), b""); + assert_eq!( + codec.open_at(&sealed, b"user:alice", 30_000).unwrap(), + b"state" + ); assert!(matches!( codec.open_at(&sealed, b"user:bob", 30_000), Err(RequestStateError::IntegrityCheckFailed) @@ -1027,11 +1030,6 @@ mod tests { codec.open_at(&sealed, b"user:alice", 70_000), Err(RequestStateError::Expired) )); - - let value = serde_json::json!({ "step": 2, "tool": "weather" }); - let sealed = codec.seal_json(&value).unwrap(); - let opened: serde_json::Value = codec.open_json(&sealed).unwrap(); - assert_eq!(opened, value); } #[test] @@ -1231,36 +1229,7 @@ mod tests { } #[test] - fn rs1_fallback_is_idempotent_and_rs1_signing_adds_its_key() { - let codec = two_key_ring("b") - .with_rs1_fallback("a") - .unwrap() - .with_rs1_fallback("a") - .unwrap(); - match &codec.keys { - Keys::Ring { rs1_fallbacks, .. } => assert_eq!(rs1_fallbacks, &["a"]), - Keys::Single(_) => panic!("expected ring"), - } - - let transitional = two_key_ring("b").with_rs1_signing("a").unwrap(); - match &transitional.keys { - Keys::Ring { - seal_mode, - rs1_fallbacks, - .. - } => { - assert!(matches!( - seal_mode, - SealMode::Rs1 { key_id } if key_id == "a" - )); - assert_eq!(rs1_fallbacks, &["a"]); - } - Keys::Single(_) => panic!("expected ring"), - } - } - - #[test] - fn parser_is_strict_and_error_precedence_is_stable() { + fn parser_rejects_invalid_tokens_and_unavailable_keys() { let codec = two_key_ring("a"); for malformed in [ "rs2", @@ -1275,56 +1244,31 @@ mod tests { )); } + let valid_rs2 = codec.seal(b"state"); assert!(matches!( - codec.open("rs2.!!!!.!!!!.!!!!"), + codec.open(&replace_segment(&valid_rs2, 1, "!!!!")), Err(RequestStateError::InvalidEncoding) )); assert!(matches!( - codec.open("rs2..!!!!.!!!!"), + codec.open(&replace_segment(&valid_rs2, 1, "")), Err(RequestStateError::InvalidKeyId) )); let non_utf8 = URL_SAFE_NO_PAD.encode([0xff]); assert!(matches!( - codec.open(&format!("rs2.{non_utf8}.!!!!.!!!!")), + codec.open(&replace_segment(&valid_rs2, 1, &non_utf8)), Err(RequestStateError::InvalidKeyId) )); - // Key selection precedes decoding later rs2 sections. - let unknown = URL_SAFE_NO_PAD.encode(b"missing"); - assert!(matches!( - codec.open(&format!("rs2.{unknown}.!!!!.!!!!")), - Err(RequestStateError::UnknownKeyId) - )); - assert!(matches!( - codec.open("rs2.YQ.!!!!.!!!!"), - Err(RequestStateError::InvalidEncoding) - )); - let valid_body = URL_SAFE_NO_PAD.encode(body(0, b"state")); - assert!(matches!( - codec.open(&format!("rs2.YQ.{valid_body}.!!!!")), - Err(RequestStateError::InvalidEncoding) - )); - assert!(matches!( - codec.open("rs2.YQ.."), - Err(RequestStateError::IntegrityCheckFailed) - )); - - let valid_rs2 = codec.seal(b"state"); assert!(matches!( RequestStateCodec::new(KEY_A).open(&valid_rs2), Err(RequestStateError::UnknownKeyId) )); - // With no eligible rs1 key, selection fails before body/tag decode. + let valid_rs1 = RequestStateCodec::new(KEY_A).seal(b"state"); assert!(matches!( - codec.open("rs1.!!!!.!!!!"), + codec.open(&valid_rs1), Err(RequestStateError::UnknownKeyId) )); - let with_fallback = codec.with_rs1_fallback("a").unwrap(); - assert!(matches!( - with_fallback.open("rs1.!!!!.!!!!"), - Err(RequestStateError::InvalidEncoding) - )); } #[test] @@ -1364,11 +1308,10 @@ mod tests { assert!(!rendered.contains(std::str::from_utf8(KEY_A).unwrap())); assert!(!rendered.contains(std::str::from_utf8(KEY_B).unwrap())); assert!(rendered.contains("redacted")); - assert!(rendered.contains("Rs2")); } #[test] - fn open_methods_do_not_panic_on_mutated_or_arbitrary_strings() { + fn mutated_tokens_do_not_panic() { let codec = two_key_ring("a").with_rs1_fallback("a").unwrap(); let valid = [ RequestStateCodec::new(KEY_A).seal(b"state"), @@ -1398,8 +1341,6 @@ mod tests { for candidate in corpus { let _ = codec.open(&candidate); let _ = codec.open_with(&candidate, b"context"); - let _: Result = codec.open_json(&candidate); - let _: Result = codec.open_json_with(&candidate, b"context"); } } }